diff mbox

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

Message ID 1439588061-18064-4-git-send-email-paulo.r.zanoni@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zanoni, Paulo R Aug. 14, 2015, 9:34 p.m. UTC 
I only tested this on BDW, 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

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_fbc.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

Comments

Ville Syrjala Aug. 28, 2015, 2:30 p.m. UTC | #1 
On Fri, Aug 14, 2015 at 06:34:08PM -0300, Paulo Zanoni wrote:
> I only tested this on BDW, 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
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_fbc.c | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index fa9b004..9ffa7dc 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -64,6 +64,20 @@ static void i8xx_fbc_disable(struct drm_i915_private *dev_priv)
>  	DRM_DEBUG_KMS("disabled FBC\n");
>  }
>  
> +static unsigned int get_crtc_fence_y_offset(struct intel_crtc *crtc)
> +{
> +	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> +	struct drm_framebuffer *fb = crtc->base.primary->fb;
> +	unsigned int tile_height;
> +
> +	tile_height = intel_tile_height(dev_priv->dev, fb->pixel_format,
> +					fb->modifier[0]);
> +
> +	/* The value we want is the line number of the first tile that contains
> +	 * the first vertical line of the CRTC. */
> +	return crtc->base.y - (crtc->base.y % tile_height);

As mentioned before, I believe this depends on an implementtion detail
of intel_gen4_compute_page_offset(). I think it would be nicer if the
callers of intel_gen4_compute_page_offset() would store the correct
fbc y offset already somewhere under intel_crtc, so that you could
just look it up here. That would safeguard this code from any changes in
intel_gen4_compute_page_offset().

> +}
> +
>  static void i8xx_fbc_enable(struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> @@ -97,7 +111,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 +149,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 +191,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 +215,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 +224,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 +304,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.4.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index fa9b004..9ffa7dc 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -64,6 +64,20 @@  static void i8xx_fbc_disable(struct drm_i915_private *dev_priv)
 	DRM_DEBUG_KMS("disabled FBC\n");
 }
 
+static unsigned int get_crtc_fence_y_offset(struct intel_crtc *crtc)
+{
+	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+	struct drm_framebuffer *fb = crtc->base.primary->fb;
+	unsigned int tile_height;
+
+	tile_height = intel_tile_height(dev_priv->dev, fb->pixel_format,
+					fb->modifier[0]);
+
+	/* The value we want is the line number of the first tile that contains
+	 * the first vertical line of the CRTC. */
+	return crtc->base.y - (crtc->base.y % tile_height);
+}
+
 static void i8xx_fbc_enable(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
@@ -97,7 +111,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 +149,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 +191,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 +215,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 +224,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 +304,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);