[Intel-gfx] [PATCH v2] drm/i915/skl: Consider plane rotation when calculating stride in skl_do_mmio_flip

[Ville Syrjälä]

On Tue, Oct 20, 2015 at 04:11:34PM +0100, Tvrtko Ursulin wrote:
> 
> On 20/10/15 16:08, Ville Syrjälä wrote:
> > On Tue, Oct 20, 2015 at 03:54:11PM +0100, Tvrtko Ursulin wrote:
> >> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >>
> >> Previously rotation was ignored and wrong stride programmed
> >> into the plane registers resulting in a corrupt image on screen.
> >>
> >> v2: Do not access potentialy old plane state at flip time,
> >>      but store the rotation value at the time of queing the flip.
> >>      (Ville)
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >> Testcase: igt/kms_rotation_crc/primary-rotation-90-flip-stress (SKL)
> >> Cc: Sonika Jindal <sonika.jindal at intel.com>
> >> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/intel_display.c | 30 +++++++++++++++++++-----------
> >>   drivers/gpu/drm/i915/intel_drv.h     |  1 +
> >>   2 files changed, 20 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index 1fc1d240d3c1..9cb29086971a 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -11091,13 +11091,14 @@ static bool use_mmio_flip(struct intel_engine_cs *ring,
> >>   }
> >>
> >>   static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
> >> +			     unsigned int rotation,
> >>   			     struct intel_unpin_work *work)
> >>   {
> >>   	struct drm_device *dev = intel_crtc->base.dev;
> >>   	struct drm_i915_private *dev_priv = dev->dev_private;
> >>   	struct drm_framebuffer *fb = intel_crtc->base.primary->fb;
> >>   	const enum pipe pipe = intel_crtc->pipe;
> >> -	u32 ctl, stride;
> >> +	u32 ctl, stride, tile_height;
> >>
> >>   	ctl = I915_READ(PLANE_CTL(pipe, 0));
> >>   	ctl &= ~PLANE_CTL_TILED_MASK;
> >> @@ -11121,9 +11122,16 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
> >>   	 * The stride is either expressed as a multiple of 64 bytes chunks for
> >>   	 * linear buffers or in number of tiles for tiled buffers.
> >>   	 */
> >> -	stride = fb->pitches[0] /
> >> -		 intel_fb_stride_alignment(dev, fb->modifier[0],
> >> -					   fb->pixel_format);
> >> +	if (intel_rotation_90_or_270(rotation)) {
> >> +		/* stride = Surface height in tiles */
> >> +		tile_height = intel_tile_height(dev, fb->pixel_format,
> >> +						fb->modifier[0], 0);
> >> +		stride = DIV_ROUND_UP(fb->height, tile_height);
> >> +	} else {
> >> +		stride = fb->pitches[0] /
> >> +				intel_fb_stride_alignment(dev, fb->modifier[0],
> >> +							  fb->pixel_format);
> >> +	}
> >>
> >>   	/*
> >>   	 * Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
> >> @@ -11181,7 +11189,7 @@ static void intel_do_mmio_flip(struct intel_mmio_flip *mmio_flip)
> >>   	intel_pipe_update_start(crtc);
> >>
> >>   	if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
> >> -		skl_do_mmio_flip(crtc, work);
> >> +		skl_do_mmio_flip(crtc, mmio_flip->rotation, work);
> >>   	else
> >>   		/* use_mmio_flip() retricts MMIO flips to ilk+ */
> >>   		ilk_do_mmio_flip(crtc, work);
> >> @@ -11208,10 +11216,8 @@ static void intel_mmio_flip_work_func(struct work_struct *work)
> >>
> >>   static int intel_queue_mmio_flip(struct drm_device *dev,
> >>   				 struct drm_crtc *crtc,
> >> -				 struct drm_framebuffer *fb,
> >> -				 struct drm_i915_gem_object *obj,
> >> -				 struct intel_engine_cs *ring,
> >> -				 uint32_t flags)
> >> +				 unsigned int rotation,
> >> +				 struct drm_i915_gem_object *obj)
> >>   {
> >>   	struct intel_mmio_flip *mmio_flip;
> >>
> >> @@ -11222,6 +11228,7 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
> >>   	mmio_flip->i915 = to_i915(dev);
> >>   	mmio_flip->req = i915_gem_request_reference(obj->last_write_req);
> >>   	mmio_flip->crtc = to_intel_crtc(crtc);
> >> +	mmio_flip->rotation = rotation;
> >
> > Could just do the crtc->primary->state->rotation thing here and avoid
> > having to pass the rotation from the caller.
> 
> Oh that would indeed be nicer. I'll do a v3.
> 
> > But the patch looks OK anyway:
> > Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Can I carry over the r-b to v3?

Yes.

-- 
Ville Syrjälä
Intel OTC