Re: [Intel-gfx] [PATCH 5/8] drm/i915: Move the "recompress on activate" to a central place

From: Jani Nikula <jani.nikula@linux.intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 5/8] drm/i915: Move the "recompress on activate" to a central place
Date: Wed, 21 Apr 2021 16:17:10 +0300	[thread overview]
Message-ID: <87czun93qh.fsf@intel.com> (raw)
In-Reply-To: <20210414022309.30898-6-ville.syrjala@linux.intel.com>

On Wed, 14 Apr 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> On ILK+ we current do a nuke right after activating FBC. If my
> memory isn't playing tricks on me this is actially required if
> FBC didn't stay disabled for a full frame. In that case the
> deactivate+reactivate may not invalidate the cfb. I'd have to
> double chekc to be sure though.
>
> So let's keep the nuke, and just extend it backwards to cover
> all the platforms by doing it a bit higher up.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Does what it says on the box, and the change overall seems logical.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index fb8c0872a2b7..8165bdb6f921 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -212,16 +212,16 @@ static void i965_fbc_recompress(struct drm_i915_private *dev_priv)
>  /* This function forces a CFB recompression through the nuke operation. */
>  static void snb_fbc_recompress(struct drm_i915_private *dev_priv)
>  {
> -	struct intel_fbc *fbc = &dev_priv->fbc;
> -
> -	trace_intel_fbc_nuke(fbc->crtc);
> -
>  	intel_de_write(dev_priv, MSG_FBC_REND_STATE, FBC_REND_NUKE);
>  	intel_de_posting_read(dev_priv, MSG_FBC_REND_STATE);
>  }
>  
>  static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
>  {
> +	struct intel_fbc *fbc = &dev_priv->fbc;
> +
> +	trace_intel_fbc_nuke(fbc->crtc);
> +
>  	if (DISPLAY_VER(dev_priv) >= 6)
>  		snb_fbc_recompress(dev_priv);
>  	else if (DISPLAY_VER(dev_priv) >= 4)
> @@ -274,8 +274,6 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
>  		       params->fence_y_offset);
>  	/* enable it... */
>  	intel_de_write(dev_priv, ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
> -
> -	intel_fbc_recompress(dev_priv);
>  }
>  
>  static void ilk_fbc_deactivate(struct drm_i915_private *dev_priv)
> @@ -348,8 +346,6 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
>  		dpfc_ctl |= FBC_CTL_FALSE_COLOR;
>  
>  	intel_de_write(dev_priv, ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
> -
> -	intel_fbc_recompress(dev_priv);
>  }
>  
>  static bool intel_fbc_hw_is_active(struct drm_i915_private *dev_priv)
> @@ -418,6 +414,7 @@ static void intel_fbc_activate(struct drm_i915_private *dev_priv)
>  	drm_WARN_ON(&dev_priv->drm, !mutex_is_locked(&fbc->lock));
>  
>  	intel_fbc_hw_activate(dev_priv);
> +	intel_fbc_recompress(dev_priv);
>  
>  	fbc->no_fbc_reason = NULL;
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx