Re: [PATCH v2 09/18] drm/i915: Support for extra alignment for tiled surfaces

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Jan 25, 2016 at 06:24:05PM +0100, Daniel Vetter wrote:
> On Wed, Jan 20, 2016 at 09:05:30PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > SKL+ needs >4K alignment for tiled surfaces, so make
> > intel_compute_page_offset() handle it.
> > 
> > The way we do it is first we compute the closest tile boundary
> > as before, and then figure out how many tiles we need to go
> > to reach the desired alignment. The difference in the offset
> > is then added into the x/y offsets.
> > 
> > v2: Be less confusing wrt. units (pixels vs. bytes) (Daniel)
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 47 ++++++++++++++++++++++++++++++++----
> >  1 file changed, 42 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index bda3224021b2..a102fabce5b4 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2459,6 +2459,35 @@ static void intel_unpin_fb_obj(struct drm_framebuffer *fb,
> >  }
> >  
> >  /*
> > + * Adjust the tile offset by moving the difference into
> > + * the x/y offsets.
> > + *
> > + * Input tile dimensions and pitch must already be
> > + * rotated to match x and y, and in pixel units.
> > + */
> > +static void intel_adjust_tile_offset(int *x, int *y,
> > +				     unsigned int tile_width,
> > +				     unsigned int tile_height,
> > +				     unsigned int tile_size,
> > +				     unsigned int pitch_tiles,
> > +				     unsigned int old_offset,
> > +				     unsigned int new_offset)
> > +{
> > +	unsigned int tiles;
> > +
> > +	WARN_ON(old_offset & (tile_size - 1));
> > +	WARN_ON(new_offset & (tile_size - 1));
> > +	WARN_ON(new_offset > old_offset);
> > +
> > +	tiles = (old_offset - new_offset) / tile_size;
> > +	if (tiles == 0)
> > +		return;
> > +
> > +	*y += tiles / pitch_tiles * tile_height;
> > +	*x += tiles % pitch_tiles * tile_width;
> > +}
> > +
> > +/*
> >   * Computes the linear offset to the base tile and adjusts
> >   * x, y. bytes per pixel is assumed to be a power-of-two.
> >   *
> > @@ -2473,6 +2502,12 @@ u32 intel_compute_tile_offset(struct drm_i915_private *dev_priv,
> >  			      unsigned int pitch,
> >  			      unsigned int rotation)
> >  {
> > +	unsigned int offset, alignment;
> > +
> > +	alignment = intel_surf_alignment(dev_priv, fb_modifier);
> > +	if (alignment)
> > +		alignment--;
> 
> Still voting for offset_aligned = ALIGN(offset, ...) per
> 
> http://article.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/72294
> 
> (at the very bottom, with the r-b for this patch).

And as I replied there, ALIGN doesn't do what we want.

> 
> > +
> >  	if (fb_modifier != DRM_FORMAT_MOD_NONE) {
> >  		unsigned int tile_size, tile_width, tile_height;
> >  		unsigned int tile_rows, tiles, pitch_tiles;
> > @@ -2494,16 +2529,18 @@ u32 intel_compute_tile_offset(struct drm_i915_private *dev_priv,
> >  		tiles = *x / tile_width;
> >  		*x %= tile_width;
> >  
> > -		return (tile_rows * pitch_tiles + tiles) * tile_size;
> > -	} else {
> > -		unsigned int alignment = intel_linear_alignment(dev_priv) - 1;
> > -		unsigned int offset;
> > +		offset = (tile_rows * pitch_tiles + tiles) * tile_size;
> 
> pitch_tiles seems undefined here, does this compile?

Yes. Diff just likes to make patches hard to read.

> -Daniel
> 
> >  
> > +		intel_adjust_tile_offset(x, y, tile_width, tile_height,
> > +					 tile_size, pitch_tiles,
> > +					 offset, offset & ~alignment);
> > +	} else {
> >  		offset = *y * pitch + *x * cpp;
> >  		*y = (offset & alignment) / pitch;
> >  		*x = ((offset & alignment) - *y * pitch) / cpp;
> > -		return offset & ~alignment;
> >  	}
> > +
> > +	return offset & ~alignment;
> >  }
> >  
> >  static int i9xx_format_to_fourcc(int format)
> > -- 
> > 2.4.10
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

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