Re: [PATCH] drm/i915: Only fence tiled region of object.

[Daniel Vetter <daniel@ffwll.ch>]

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@ghs.com>
> > > > > > > > Signed-off-by: Bob Paauwe <bob.j.paauwe@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. 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.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx