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

[Stéphane Marchesin]

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?

Stéphane

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