Re: [PATCH 1/5] drm/i915: Always pin the fence for scanout on gen2/3 and primary planes

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/5] drm/i915: Always pin the fence for scanout on gen2/3 and primary planes
Date: Fri, 17 Nov 2017 15:17:53 +0200	[thread overview]
Message-ID: <20171117131753.GH10981@intel.com> (raw)
In-Reply-To: <151086636808.26104.16701598731599378514@mail.alporthouse.com>

On Thu, Nov 16, 2017 at 09:06:08PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2017-11-16 19:14:47)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The current code is trying to be lazy with fences on scanout buffers.
> > That looks broken for several reasons:
> > * gen2/3 always need a fence for tiled scanout
> 
> Which it already gets. All gen2-gen4 are given a fenceable vma.

Fenceable yes, but we still allow the fence_pin() to fail so
there's no guarantee that we actually have a fence on the vma
AFAICS.

I did managed to trigger an oops in the FBC code on my i85x where
the vma didn't have a fence. I actually don't know how it managed
to scan out anything in that case. Maybe it didn't and I just wasn't
looking at the screen when it failed. 

It's also slightly odd that it even got that far as the FBC code has
a vma->fence check earlier. My current thinking is that we didn't
have a fence when we were supposed to pin it, and then when FBC did
its check a fence happened to be present, and later on the fence
disappeared once more. Either that or FBC was being enabled when
the fb was no longer pinned, which would be a clear bug in itself.

> 
> > * the unpin doesn't know whether we pinned the fence or not so it
> >   may unpin something we don't own
> 
> Then track it correctly.

Wasn't convinced that it's worth it. After this series on most platforms
we would have just the one plane that would need a fence. And the total
number of fences being 32 on modern platforms it seems somewhat
unlikely to me that we couldn't get the fence here. I've not tested that
theory though.

Although I guess for the 90/270 case we really would need to track the
other vma and its fence correctly.

> 
> > * FBC GTT tracking needs a fence (not sure we have proper fallback
> >   for when there is no fence)
> 
> Debatable as whether that is worth forcing a fence; the argument being
> that you don't want to stall your flip upon eviction which is the
> situation you are already in if you didn't get a fence in the first
> place.

My thinking was again that it's very unlikely that would can't get a
fence.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx