Re: [PATCH 16/33] drm/i915: Convert fence computations to use vma directly

[Chris Wilson <chris@chris-wilson.co.uk>]

On Tue, Aug 09, 2016 at 01:27:31PM +0300, Joonas Lahtinen wrote:
> On su, 2016-08-07 at 15:45 +0100, Chris Wilson wrote:
> > Lookup the GGTT vma once for the object assigned to the fence, and then
> > derive everything from that vma.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_fence.c | 55 +++++++++++++++++------------------
> >  1 file changed, 26 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c b/drivers/gpu/drm/i915/i915_gem_fence.c
> > index 9e8173fe2a09..60749cd23f20 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_fence.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_fence.c
> > @@ -85,22 +85,19 @@ static void i965_write_fence_reg(struct drm_device *dev, int reg,
> >  	POSTING_READ(fence_reg_lo);
> >  
> >  	if (obj) {
> > -		u32 size = i915_gem_obj_ggtt_size(obj);
> > +		struct i915_vma *vma = i915_gem_obj_to_ggtt(obj);
> >  		unsigned int tiling = i915_gem_object_get_tiling(obj);
> >  		unsigned int stride = i915_gem_object_get_stride(obj);
> > -		uint64_t val;
> > +		u64 size = vma->node.size;
> > +		u32 row_size = stride * (tiling == I915_TILING_Y ? 32 : 8);
> > +		u64 val;
> >  
> >  		/* Adjust fence size to match tiled area */
> > -		if (tiling != I915_TILING_NONE) {
> > -			uint32_t row_size = stride *
> > -				(tiling == I915_TILING_Y ? 32 : 8);
> > -			size = (size / row_size) * row_size;
> > -		}
> > +		size = size / row_size * row_size;
> 
> There's a macro for this, it's called rounddown().
> 
> >  
> > -		val = (uint64_t)((i915_gem_obj_ggtt_offset(obj) + size - 4096) &
> > -				 0xfffff000) << 32;
> > -		val |= i915_gem_obj_ggtt_offset(obj) & 0xfffff000;
> > -		val |= (uint64_t)((stride / 128) - 1) << fence_pitch_shift;
> > +		val = ((vma->node.start + size - 4096) & 0xfffff000) << 32;
> > +		val |= vma->node.start & 0xfffff000;
> > +		val |= (u64)((stride / 128) - 1) << fence_pitch_shift;
> 
> This was rather magicy before, but it could be much better. The rest
> are less so. Can be added to TODO.

> With above converted to rounddown()

The code is just silly. The correct fix is to expand the GTT vma to fit
the fence and not reduce the fence.  rounddown is fine as until it is
fixed.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx