Re: [PATCH] drm/i915: Use pixel size for computing linear offsets into a sprite

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

On Tue, Jan 08, 2013 at 11:59:57AM +0100, Daniel Vetter wrote:
> On Wed, Dec 19, 2012 at 12:14:22PM +0000, Chris Wilson wrote:
> > This fixes an original bug in the sprite code that miscomputed the
> > source offset into a linear YUV packed framebuffer, that was magnified
> > into an oops with
> > 
> > commit 5a35e99e8162d6820013a56ad15ea8bf937af5a6
> > Author: Damien Lespiau <damien.lespiau@intel.com>
> > Date:   Fri Oct 26 18:20:12 2012 +0100
> > 
> >     drm/i915: adjust sprite base address
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.com>
> > Cc: Damien Lespiau <damien.lespiau@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_sprite.c |   10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index 3e05d79..fef3cfb 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -120,11 +120,10 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
> >  	I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]);
> >  	I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
> >  
> > -	linear_offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
> > +	linear_offset = y * fb->pitches[0] + x * pixel_size;
> 
> Shouldn't we round x to an even offset for packed yuv? Maybe even using
> drm_format_horz_chroma_subsampling ... The idea being that a generic
> client can't really know the resolution for our source offsets.

Yeah either we round, or we just reject the operation. Both options have
some merit, so I think we need some kind of user controlled knob to select
the mode.

The "proper clipping" patch in the atomic branch rounds to nearest
macropixel, and it also handles the scaling limitations in a fuzzy
manner. I still need to split that stuff out, and then we need to
figure out how the user is supposed to select the rounding mode
(a per plane property perhaps, or just a global flag?).

-- 
Ville Syrjälä
Intel OTC