diff mbox series

[2/4] drm/i915/fbc: Fix nuke for pre-snb platforms

Message ID 20200702153723.24327-3-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: FBC fixes | expand

Commit Message

Ville Syrjälä July 2, 2020, 3:37 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The MSG_FBC_REND_STATE register only exists on snb+. For older
platforms (would also work for snb+) we can simply rewite DSPSURF
to trigger a flip nuke.

While generally RMW is considered harmful we'll use it here for
simplicity. And since FBC doesn't exist in i830 we don't have to
worry about the DSPSURF double buffering hardware fails present
on that platform.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_fbc.c | 34 +++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

Comments

Souza, Jose July 2, 2020, 11:02 p.m. UTC | #1 
On Thu, 2020-07-02 at 18:37 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The MSG_FBC_REND_STATE register only exists on snb+. For older
> platforms (would also work for snb+) we can simply rewite DSPSURF
> to trigger a flip nuke.
> 
> While generally RMW is considered harmful we'll use it here for
> simplicity. And since FBC doesn't exist in i830 we don't have to
> worry about the DSPSURF double buffering hardware fails present
> on that platform.

Did not found a explicit statement about writing DSPSURF will nuke compressed buffer but that makes sense, also checked that MSG_FBC_REND_STATE do not
exist this older platforms.

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 34 +++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index d30c2a389294..036546ce8db8 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -187,8 +187,30 @@ static bool g4x_fbc_is_active(struct drm_i915_private *dev_priv)
>  	return intel_de_read(dev_priv, DPFC_CONTROL) & DPFC_CTL_EN;
>  }
>  
> +static void i8xx_fbc_recompress(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
> +	enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
> +
> +	spin_lock_irq(&dev_priv->uncore.lock);
> +	intel_de_write_fw(dev_priv, DSPADDR(i9xx_plane),
> +			  intel_de_read_fw(dev_priv, DSPADDR(i9xx_plane)));
> +	spin_unlock_irq(&dev_priv->uncore.lock);
> +}
> +
> +static void i965_fbc_recompress(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
> +	enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
> +
> +	spin_lock_irq(&dev_priv->uncore.lock);
> +	intel_de_write_fw(dev_priv, DSPSURF(i9xx_plane),
> +			  intel_de_read_fw(dev_priv, DSPSURF(i9xx_plane)));
> +	spin_unlock_irq(&dev_priv->uncore.lock);
> +}
> +
>  /* This function forces a CFB recompression through the nuke operation. */
> -static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
> +static void snb_fbc_recompress(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_fbc *fbc = &dev_priv->fbc;
>  
> @@ -198,6 +220,16 @@ static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
>  	intel_de_posting_read(dev_priv, MSG_FBC_REND_STATE);
>  }
>  
> +static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
> +{
> +	if (INTEL_GEN(dev_priv) >= 6)
> +		snb_fbc_recompress(dev_priv);
> +	else if (INTEL_GEN(dev_priv) >= 4)
> +		i965_fbc_recompress(dev_priv);
> +	else
> +		i8xx_fbc_recompress(dev_priv);
> +}
> +
>  static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
Ville Syrjälä July 3, 2020, 11:45 a.m. UTC | #2 
On Thu, Jul 02, 2020 at 11:02:05PM +0000, Souza, Jose wrote:
> On Thu, 2020-07-02 at 18:37 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The MSG_FBC_REND_STATE register only exists on snb+. For older
> > platforms (would also work for snb+) we can simply rewite DSPSURF
> > to trigger a flip nuke.
> > 
> > While generally RMW is considered harmful we'll use it here for
> > simplicity. And since FBC doesn't exist in i830 we don't have to
> > worry about the DSPSURF double buffering hardware fails present
> > on that platform.
> 
> Did not found a explicit statement about writing DSPSURF will nuke compressed buffer but that makes sense,

Flip nuke is certainly a thing, but flipping to the same address I think
might be somewhat undefined. IIRC I did test this however and it worked
just fine.

> also checked that MSG_FBC_REND_STATE do not
> exist this older platforms.
> 
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> 
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_fbc.c | 34 +++++++++++++++++++++++-
> >  1 file changed, 33 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> > index d30c2a389294..036546ce8db8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > @@ -187,8 +187,30 @@ static bool g4x_fbc_is_active(struct drm_i915_private *dev_priv)
> >  	return intel_de_read(dev_priv, DPFC_CONTROL) & DPFC_CTL_EN;
> >  }
> >  
> > +static void i8xx_fbc_recompress(struct drm_i915_private *dev_priv)
> > +{
> > +	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
> > +	enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
> > +
> > +	spin_lock_irq(&dev_priv->uncore.lock);
> > +	intel_de_write_fw(dev_priv, DSPADDR(i9xx_plane),
> > +			  intel_de_read_fw(dev_priv, DSPADDR(i9xx_plane)));
> > +	spin_unlock_irq(&dev_priv->uncore.lock);
> > +}
> > +
> > +static void i965_fbc_recompress(struct drm_i915_private *dev_priv)
> > +{
> > +	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
> > +	enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
> > +
> > +	spin_lock_irq(&dev_priv->uncore.lock);
> > +	intel_de_write_fw(dev_priv, DSPSURF(i9xx_plane),
> > +			  intel_de_read_fw(dev_priv, DSPSURF(i9xx_plane)));
> > +	spin_unlock_irq(&dev_priv->uncore.lock);
> > +}
> > +
> >  /* This function forces a CFB recompression through the nuke operation. */
> > -static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
> > +static void snb_fbc_recompress(struct drm_i915_private *dev_priv)
> >  {
> >  	struct intel_fbc *fbc = &dev_priv->fbc;
> >  
> > @@ -198,6 +220,16 @@ static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
> >  	intel_de_posting_read(dev_priv, MSG_FBC_REND_STATE);
> >  }
> >  
> > +static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
> > +{
> > +	if (INTEL_GEN(dev_priv) >= 6)
> > +		snb_fbc_recompress(dev_priv);
> > +	else if (INTEL_GEN(dev_priv) >= 4)
> > +		i965_fbc_recompress(dev_priv);
> > +	else
> > +		i8xx_fbc_recompress(dev_priv);
> > +}
> > +
> >  static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
> >  {
> >  	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index d30c2a389294..036546ce8db8 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -187,8 +187,30 @@  static bool g4x_fbc_is_active(struct drm_i915_private *dev_priv)
 	return intel_de_read(dev_priv, DPFC_CONTROL) & DPFC_CTL_EN;
 }
 
+static void i8xx_fbc_recompress(struct drm_i915_private *dev_priv)
+{
+	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
+	enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
+
+	spin_lock_irq(&dev_priv->uncore.lock);
+	intel_de_write_fw(dev_priv, DSPADDR(i9xx_plane),
+			  intel_de_read_fw(dev_priv, DSPADDR(i9xx_plane)));
+	spin_unlock_irq(&dev_priv->uncore.lock);
+}
+
+static void i965_fbc_recompress(struct drm_i915_private *dev_priv)
+{
+	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
+	enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
+
+	spin_lock_irq(&dev_priv->uncore.lock);
+	intel_de_write_fw(dev_priv, DSPSURF(i9xx_plane),
+			  intel_de_read_fw(dev_priv, DSPSURF(i9xx_plane)));
+	spin_unlock_irq(&dev_priv->uncore.lock);
+}
+
 /* This function forces a CFB recompression through the nuke operation. */
-static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
+static void snb_fbc_recompress(struct drm_i915_private *dev_priv)
 {
 	struct intel_fbc *fbc = &dev_priv->fbc;
 
@@ -198,6 +220,16 @@  static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
 	intel_de_posting_read(dev_priv, MSG_FBC_REND_STATE);
 }
 
+static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
+{
+	if (INTEL_GEN(dev_priv) >= 6)
+		snb_fbc_recompress(dev_priv);
+	else if (INTEL_GEN(dev_priv) >= 4)
+		i965_fbc_recompress(dev_priv);
+	else
+		i8xx_fbc_recompress(dev_priv);
+}
+
 static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
 {
 	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;