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

[Jesse Barnes]

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...

Or are we failing to fault on a previously mapped scanout too?  If so,
we'd need to cover more than just scanout here.

-- 
Jesse Barnes, Intel Open Source Technology Center