[Intel-gfx] [PATCH] drm/i915: Only fence tiled region of object.

[Chris Wilson]

On Fri, Dec 19, 2014 at 11:15:59AM +0100, Daniel Vetter wrote:
> On Fri, Dec 19, 2014 at 09:17:40AM +0000, Chris Wilson wrote:
> > On Fri, Dec 19, 2014 at 11:05:36AM +0200, Imre Deak wrote:
> > > On Fri, 2014-12-19 at 08:26 +0000, Chris Wilson wrote:
> > > > On Fri, Dec 19, 2014 at 12:14:00AM +0200, Imre Deak wrote:
> > > > > On Thu, 2014-12-18 at 22:19 +0100, Daniel Vetter wrote:
> > > > > > On Thu, Dec 18, 2014 at 11:04:11PM +0200, Ville Syrjälä wrote:
> > > > > > > On Thu, Dec 18, 2014 at 09:37:37PM +0100, Daniel Vetter wrote:
> > > > > > > > On Thu, Dec 18, 2014 at 09:51:26AM -0800, Bob Paauwe wrote:
> > > > > > > > > When creating a fence for a tiled object, only fence the area that
> > > > > > > > > makes up the actual tiles.  The object may be larger than the tiled
> > > > > > > > > area and if we allow those extra addresses to be fenced, they'll
> > > > > > > > > get converted to addresses beyond where the object is mapped. This
> > > > > > > > > opens up the possiblity of writes beyond the end of object.
> > > > > > > > > 
> > > > > > > > > To prevent this, we adjust the size of the fence to only encompass
> > > > > > > > > the area that makes up the actual tiles.  The extra space is considered
> > > > > > > > > un-tiled and now behaves as if it was a linear object.
> > > > > > > > > 
> > > > > > > > > Testcase: igt/gem_tiled_fence_overflow
> > > > > > > > > Reported-by: Dan Hettena <danh at ghs.com>
> > > > > > > > > Signed-off-by: Bob Paauwe <bob.j.paauwe at intel.com>
> > > > > > > > 
> > > > > > > > Presuming this indeed blows up (I didn't try your test) this is one for
> > > > > > > > Jani.
> > > > > > > 
> > > > > > > Hmm. Wasn't this problem discussed a few years ago already? My recollection
> > > > > > > is that Imre had patches but you said you don't care about the problem.
> > > > > > 
> > > > > > Imre had patches iirc to resize the allocation , which would have caused
> > > > > > major havoc with moving stuff around I think.
> > > > > 
> > > > > It did that only for old GENs where the POT fence size constraint gives
> > > > > you no other choice to fix this issue. On new HW I also rounded down the
> > > > > fence size.
> > > > 
> > > > If we were to be consistent, then we would pad in the GTT so that no
> > > > other object fitted in the full fenced region.
> > > 
> > > Yes, I did that. In v2 I changed this (based on your feedback) so the
> > > padding happens only on old GENs with the POT constraint, since on new
> > > GENs we can instead reduce the fence region size. The end of the buffer
> > > couldn't contain even a single whole pixel line, so would display
> > > incorrectly anyway.
> > 
> > Maybe they allocated one very large object for a mipmap, each level of
> > varying pitches but uploading through a single fence with a single
> > pitch, and so carefully wrote the trailing levels to account for the
> > different tile row size (but it knew the individual pages would be
> > correct).
> > 
> > Technically reducing the fenced region size is an ABI change... :p
> 
> Well the last incomplete tile-row couldn't ever really be used anyway
> since writes just go somewhere.

They go to a fully deterministic page, so it could be used by a
masochistic user.

> So I don't think this is a user-observable
> ABI change. And it's simpler than enlarging the tiled gtt size on gen4+,
> which might result in some more gtt thrashing. So I only want to do that
> if we really need it.

I would have accepted that not even libva has ever done anything like
this, and so the only known use has been in buggy code.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre