Re: [PATCH 08/12] drm/i915: Allow calling intel_adjust_tile_offset() multiple times

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

On Wed, Aug 10, 2016 at 11:48:56AM +0200, Daniel Vetter wrote:
> On Wed, Aug 10, 2016 at 12:23:17PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Minimize the resulting X coordinate after intel_adjust_tile_offset() is
> > done with it's offset adjustment. This allows calling
> > intel_adjust_tile_offset() multiple times in case we need to adjust
> > the offset several times.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Do we check anywhere that y doesn't eventually overflow? Anyway, this
> looks good.

Hmm. I was thinking it's limited by the fb size, but only the plane src
coordinate part is limited like that. The part coming from fb->offsets[]
is extra, so I guess the total might end up too big even for 32 bits.
And the hardware for sure has even lower number of bits available for
the x/y offsets, so we should definitely add some extra checks for
this.

I have a feeling I had some checks like this at some point, but either I
imagined it, or I mistakenly threw them out, or they're in some random
branch I've misplaced.

> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index f4b45625703e..40c809593ae9 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2330,6 +2330,7 @@ static u32 intel_adjust_tile_offset(int *x, int *y,
> >  				    u32 old_offset,
> >  				    u32 new_offset)
> >  {
> > +	unsigned int pitch_pixels = pitch_tiles * tile_width;
> >  	unsigned int tiles;
> >  
> >  	WARN_ON(old_offset & (tile_size - 1));
> > @@ -2341,6 +2342,10 @@ static u32 intel_adjust_tile_offset(int *x, int *y,
> >  	*y += tiles / pitch_tiles * tile_height;
> >  	*x += tiles % pitch_tiles * tile_width;
> >  
> > +	/* minimize x in case it got needlessly big */
> > +	*y += *x / pitch_pixels * tile_height;
> > +	*x %= pitch_pixels;
> > +
> >  	return new_offset;
> >  }
> >  
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://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
https://lists.freedesktop.org/mailman/listinfo/intel-gfx