Re: [PATCH 5/6] drm/i915/reset: Handle the display vs. GPU reset deadlock using a custom dma-fence

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Apr 13, 2026 at 11:35:23AM +0200, Christian König wrote:
> On 4/13/26 11:11, Ville Syrjälä wrote:
> >>>> I think something is missing in my picture how that is supposed to work.
> >>>
> >>> The problem stems from the fact that on old platforms a GPU reset
> >>> also resets the display hardware,
> >>
> >> Which is true for at least AMD GPUs and I think pretty much everybody else as well, but that wasn't so much of a problem so far.
> >>
> >>> and to do that safely we need:
> >>> 1. shut down display
> >>> 2. perform the GPU reset
> >>> 3. restore the display hardware to its orignal state
> >>
> >> Mhm, I've recently talked with Michel about it and we confirmed that this is perfectly possible without issues. Adding Michel as well.
> >>
> >>> We just do that with essentially with a normal atomic commit.
> >>
> >> I think that is the source of the problem.
> >>
> >> I'm not an expert on that topic but amdgpu and tons of other drivers seem to just use drm_atomic_helper_shutdown() for that.
> > 
> > drm_atomic_helper_shutdown() is definitely not the thing to use
> > for this as it would clobber the stored kms state, leaving everything
> > permanently disabled. The drm_atomic_helper_commit_duplicated_state()
> > stuff i915 uses is the correct thing here.
> > 
> > But for this problem it doesn't even matter which gets used. Either
> > would get equally stuck behind a previous atomic commit waiting for
> > its fences.
> > 
> >>
> >> What is i915 doing differently?
> > 
> > I see zero code for any display reset stuff in any other driver. If
> > amdgpu does anything it must be something completely custom, hidden
> > somewhere deep.
> 
> The display is just fully reset by any MODE1 reset, you don't need to do anything special for that.

You can't just ignore the fact that there may be a display hardware
reprogramming already happening in parallel. Failing to follow the
correct programming sequence is a recipe for even hard system hangs.

> 
> Restoring the display after the reset is either not an atomic commit at all or done by an async worker after the reset completed.

Everything is an atomic commit. No one is going to implement a
complete second modeset codepath just for resets.

> 
> >>> But a 
> >>> previous atomic commit may already be waiting for a fence, which
> >>> won't signal until the GPU reset happens, and the GPU reset is now
> >>> waiting for that previous atomic commit to finish so that it can do
> >>> its own atomic commit. In order to break the deadlock we need to
> >>> abort the fence waits in the atomic commit, and that's what this
> >>> "reset fence" achieves.
> >>
> >> As far as I can see that approach looks strongly like a no-go.
> >>
> >> You essentially have a lock inversion here and it is documented that it should *never* be resolved by a timeout, the approach you take now is not much better.
> > 
> > Unless someone wants to add some kind of extra abort mechanism to
> > dma_fence_wait*() then I think this is probably the best solution.
> > And given this is only a thing for one driver on old hardware,
> > adding extra stuff to dma_fence_wait*() doesn't really seem worth
> > the hassle.
> 
> Well adding something to dma_fence_wait() would be a no-go as well.
> 
> The point is you should *NOT* wait for an atomic commit to finish from your GPU reset at all. Any GPU reset which waits on a DMA-fence in one way or another is a really big bug.
> 
> That is documented and Simona even came up with tons of lockdep annotation to make sure that drivers comply to that.
> 
> I'm deeply surprised that a driver like i915 actually tries something like that. And as DMA-buf maintainer I must bluntly NAK that since it goes against documented rules for implementing drivers.
> 
> I mean what can be is that this isn't really solved with the atomic mode setting helpers, but since multiple drivers use it it shouldn't be anything special at all.

There is zero support in the atomic design for reordering commits,
which is pretty much what would be needed. A generic solution would
more or less involve a full redesign/rewrite of atomic.

-- 
Ville Syrjälä
Intel