Re: [PATCH 9/9] drm/i915: fix FBC for cases where crtc->base.y is non-zero

[Daniel Vetter <daniel@ffwll.ch>]

Pulled in the entire series to dinq, thanks.
-Daniel

On Mon, Sep 21, 2015 at 05:04:49PM +0300, Ville Syrjälä wrote:
> On Mon, Sep 14, 2015 at 03:20:03PM -0300, Paulo Zanoni wrote:
> > I only tested this on BDW and SKL, but since the register description
> > is the same ever since gen4, let's assume that all gens take the same
> > register format. If that's not true, then hopefully someone will
> > bisect a bug to this patch and we'll fix it.
> > 
> > Notice that the wrong fence offset register just means that the
> > hardware tracking will be wrong.
> > 
> > Testcases:
> >  - igt/kms_frontbuffer_tracking/fbc-1p-primscrn-pri-shrfb-draw-mmap-gtt
> >  - igt/kms_frontbuffer_tracking/fbc-2p-primscrn-pri-shrfb-draw-mmap-gtt
> > 
> > v2:
> >   - Add intel_crtc->adjusted_{x,y} so this code can work independently
> >     of intel_gen4_compute_page_offset(). (Ville).
> >   - This version also works on SKL.
> > 
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c |  9 +++++++++
> >  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
> >  drivers/gpu/drm/i915/intel_fbc.c     | 25 ++++++++++++++++++++-----
> >  3 files changed, 31 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index fc00867..c27a636 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2730,6 +2730,9 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
> >  			(intel_crtc->config->pipe_src_w - 1) * pixel_size;
> >  	}
> >  
> > +	intel_crtc->adjusted_x = x;
> > +	intel_crtc->adjusted_y = y;
> > +
> >  	I915_WRITE(reg, dspcntr);
> >  
> >  	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
> > @@ -2830,6 +2833,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
> >  		}
> >  	}
> >  
> > +	intel_crtc->adjusted_x = x;
> > +	intel_crtc->adjusted_y = y;
> > +
> >  	I915_WRITE(reg, dspcntr);
> >  
> >  	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
> > @@ -3082,6 +3088,9 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
> >  	}
> >  	plane_offset = y_offset << 16 | x_offset;
> >  
> > +	intel_crtc->adjusted_x = x_offset;
> > +	intel_crtc->adjusted_y = y_offset;
> > +
> >  	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
> >  	I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset);
> >  	I915_WRITE(PLANE_SIZE(pipe, 0), plane_size);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 02a755a..6054bcb 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -537,6 +537,8 @@ struct intel_crtc {
> >  	 * gen4+ this only adjusts up to a tile, offsets within a tile are
> >  	 * handled in the hw itself (with the TILEOFF register). */
> >  	unsigned long dspaddr_offset;
> > +	int adjusted_x;
> > +	int adjusted_y;
> >  
> >  	struct drm_i915_gem_object *cursor_bo;
> >  	uint32_t cursor_addr;
> > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> > index a3a97f2..2319dc5 100644
> > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > @@ -41,6 +41,19 @@
> >  #include "intel_drv.h"
> >  #include "i915_drv.h"
> >  
> > +/*
> > + * In some platforms where the CRTC's x:0/y:0 coordinates doesn't match the
> > + * frontbuffer's x:0/y:0 coordinates we lie to the hardware about the plane's
> > + * origin so the x and y offsets can actually fit the registers. As a
> > + * consequence, the fence doesn't really start exactly at the display plane
> > + * address we program because it starts at the real start of the buffer, so we
> > + * have to take this into consideration here.
> > + */
> > +static unsigned int get_crtc_fence_y_offset(struct intel_crtc *crtc)
> > +{
> > +	return crtc->base.y - crtc->adjusted_y;
> > +}
> 
> We may want to move over to using the plane state here in the future.
> But for now it looks OK, so:
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> 
> > +
> >  static void i8xx_fbc_disable(struct drm_i915_private *dev_priv)
> >  {
> >  	u32 fbc_ctl;
> > @@ -97,7 +110,7 @@ static void i8xx_fbc_enable(struct intel_crtc *crtc)
> >  		fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE;
> >  		fbc_ctl2 |= FBC_CTL_PLANE(crtc->plane);
> >  		I915_WRITE(FBC_CONTROL2, fbc_ctl2);
> > -		I915_WRITE(FBC_FENCE_OFF, crtc->base.y);
> > +		I915_WRITE(FBC_FENCE_OFF, get_crtc_fence_y_offset(crtc));
> >  	}
> >  
> >  	/* enable it... */
> > @@ -135,7 +148,7 @@ static void g4x_fbc_enable(struct intel_crtc *crtc)
> >  		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
> >  	dpfc_ctl |= DPFC_CTL_FENCE_EN | obj->fence_reg;
> >  
> > -	I915_WRITE(DPFC_FENCE_YOFF, crtc->base.y);
> > +	I915_WRITE(DPFC_FENCE_YOFF, get_crtc_fence_y_offset(crtc));
> >  
> >  	/* enable it... */
> >  	I915_WRITE(DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
> > @@ -177,6 +190,7 @@ static void ilk_fbc_enable(struct intel_crtc *crtc)
> >  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> >  	u32 dpfc_ctl;
> >  	int threshold = dev_priv->fbc.threshold;
> > +	unsigned int y_offset;
> >  
> >  	dev_priv->fbc.enabled = true;
> >  
> > @@ -200,7 +214,8 @@ static void ilk_fbc_enable(struct intel_crtc *crtc)
> >  	if (IS_GEN5(dev_priv))
> >  		dpfc_ctl |= obj->fence_reg;
> >  
> > -	I915_WRITE(ILK_DPFC_FENCE_YOFF, crtc->base.y);
> > +	y_offset = get_crtc_fence_y_offset(crtc);
> > +	I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset);
> >  	I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID);
> >  	/* enable it... */
> >  	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
> > @@ -208,7 +223,7 @@ static void ilk_fbc_enable(struct intel_crtc *crtc)
> >  	if (IS_GEN6(dev_priv)) {
> >  		I915_WRITE(SNB_DPFC_CTL_SA,
> >  			   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
> > -		I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->base.y);
> > +		I915_WRITE(DPFC_CPU_FENCE_OFFSET, y_offset);
> >  	}
> >  
> >  	intel_fbc_nuke(dev_priv);
> > @@ -288,7 +303,7 @@ static void gen7_fbc_enable(struct intel_crtc *crtc)
> >  
> >  	I915_WRITE(SNB_DPFC_CTL_SA,
> >  		   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
> > -	I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->base.y);
> > +	I915_WRITE(DPFC_CPU_FENCE_OFFSET, get_crtc_fence_y_offset(crtc));
> >  
> >  	intel_fbc_nuke(dev_priv);
> >  
> > -- 
> > 2.5.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx