Re: [PATCH 12/16] drm/i915: apply WaFbcAsynchFlipDisableFbcQueue earlier

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 12/16] drm/i915: apply WaFbcAsynchFlipDisableFbcQueue earlier
Date: Fri, 28 Aug 2015 20:45:13 +0300	[thread overview]
Message-ID: <20150828174513.GF5176@intel.com> (raw)
In-Reply-To: <1439588061-18064-13-git-send-email-paulo.r.zanoni@intel.com>

On Fri, Aug 14, 2015 at 06:34:17PM -0300, Paulo Zanoni wrote:
> The spec says the register should have that value for the entire time
> that FBC is enabled, so apply the WA before we enable FBC.
> 
> Notice that we also have this WA for ILK/SNB, but it is implemented at
> init_clock_gating(). I could move the IVB/HSW/BDW WA code to
> init_clock_gating() too, but since we recently had some complaints
> about WAs not staying after being set, I'm going to play safe and keep
> this here for now.

Yeah I think we tried to optimize things by applying the w/a only when
absolutely needed, on the theory that it might save some power when FBC
is disabled. But I have no idea if that's true or not. Oh and we don't
actually undo the w/a after disalbing FBC which pretty mucch defeats the
purpose of the optimizaton.

Would be better to do it the same way for all platforms I think. Either
in init_clock_gating, or we should do and undo the w/a around FBC
enable/disable.

In the meantime this patch seems fine, so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_fbc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 9dee0b5..b76c19f 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -293,8 +293,6 @@ static void gen7_fbc_enable(struct intel_crtc *crtc)
>  	if (dev_priv->fbc.false_color)
>  		dpfc_ctl |= FBC_CTL_FALSE_COLOR;
>  
> -	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
> -
>  	if (IS_IVYBRIDGE(dev_priv)) {
>  		/* WaFbcAsynchFlipDisableFbcQueue:ivb */
>  		I915_WRITE(ILK_DISPLAY_CHICKEN1,
> @@ -307,6 +305,8 @@ static void gen7_fbc_enable(struct intel_crtc *crtc)
>  			   HSW_FBCQ_DIS);
>  	}
>  
> +	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
> +
>  	I915_WRITE(SNB_DPFC_CTL_SA,
>  		   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
>  	I915_WRITE(DPFC_CPU_FENCE_OFFSET, get_crtc_fence_y_offset(crtc));
> -- 
> 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