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

[Imre Deak]

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.

--Imre

> Or maybe it reject set_tiling if it didn't match (which breaks abi). I
> was always working under the assumption that the fence never direct
> access out of it's assigned range, just that it corrupts them. 
> 

> But that's obviously wrong, so we need to fix this.
> -Daniel
> 
> > 
> > > 
> > > Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> > > Cc: stable at vger.kernel.org
> > > 
> > > Thanks, Daniel
> > > 
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_gem.c | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > > index 67550ac..c9acbfa 100644
> > > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > > @@ -3173,6 +3173,13 @@ static void i965_write_fence_reg(struct drm_device *dev, int reg,
> > > >  		u32 size = i915_gem_obj_ggtt_size(obj);
> > > >  		uint64_t val;
> > > >  
> > > > +		/* Adjust fence size to match tiled area */
> > > > +		if (obj->tiling_mode != I915_TILING_NONE) {
> > > > +			uint32_t row_size = obj->stride *
> > > > +				(obj->tiling_mode == I915_TILING_Y ? 32 : 8);
> > > > +			size = (size / row_size) * row_size;
> > > > +		}
> > > > +
> > > >  		val = (uint64_t)((i915_gem_obj_ggtt_offset(obj) + size - 4096) &
> > > >  				 0xfffff000) << 32;
> > > >  		val |= i915_gem_obj_ggtt_offset(obj) & 0xfffff000;
> > > > -- 
> > > > 2.1.0
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx at lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > 
> > > -- 
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC
>