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.zan...@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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to