[Intel-gfx] [PATCH 2/2] drm/i915: repin bound framebuffers on resume

[Daniel Vetter]

On Fri, Jun 14, 2013 at 9:12 PM, Stéphane Marchesin
<marcheu at chromium.org> wrote:
> On Wed, Jun 12, 2013 at 3:06 PM, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
>> On Wed, 12 Jun 2013 00:48:25 +0100
>> Chris Wilson <chris at chris-wilson.co.uk> wrote:
>>
>>> On Tue, Jun 11, 2013 at 04:01:21PM -0700, Stéphane Marchesin wrote:
>>> >
>>> > On Tue, Jun 11, 2013 at 3:57 PM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
>>> > > On Tue, Jun 11, 2013 at 03:49:27PM -0700, Stéphane Marchesin wrote:
>>> > >> During suspend all fences are reset, including their pin_count which
>>> > >> is reset to 0. However a framebuffer can be bound across
>>> > >> suspend/resume, which means that when the buffer is unbound after
>>> > >> resume, the pin count for the buffer will be negative. Since the
>>> > >> fence pin count is now negative when available and zero when in use,
>>> > >> the buffer's fence will get recycled when the fence is in use which
>>> > >> is the opposite of what we want. The visible effect is that since the
>>> > >> fence is recycled the tiling mode goes away while the buffer is being
>>> > >> displayed and we get lines/screens of garbage.
>>> > >>
>>> > >> To fix this, we repin the fences for all bound fbs on resume, which
>>> > >> ensures the pin count is right.
>>> > >
>>> > > Yikes. So why do we not just keep the fences alive during suspend (not
>>> > > touching their pin_count), and then just iterate over the list of fences
>>> > > rewriting the register as required upon resume? That would seem less
>>> > > error prone than trying to reconstruct the lost pin_count.
>>> >
>>> > I suspect they'd need to be saved/restored at the hw level as well,
>>> > which AFAICS isn't happening today...
>>>
>>> Ugh, I introduced this bug 30 months ago - saved by the VT switch on
>>> resume. But we can restore the fences from dev_priv->fence_regs...
>>> Actually we have a very similar problem after a GPU reset where we
>>> should restore fences for pinned objects (i.e. the scanout). The patch
>>> to fix both looks fairly straightforward.
>>
>> To be clear, this only affects gen3 right?  For gen4+ we don't need the
>> fences for scanout since we have a bit in the plane control...
>
> Yup I've only ever seen the issue on gen3.
>
> Anyway, what should we do about this? Should I make another patch
> where I save/restore the fence regs instead?

drm-intel-fixes has already the improved patch from Chris,
drm-intel-next-queued has a patch to add a WARN so we'll catch this
much quicker next time around.
-Daniel

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch