Re: [Intel-gfx] [PATCH 04/12] drm/i915/fbc: Fix nuke for pre-snb platforms

2020-05-04 Thread Ville Syrjälä
On Fri, May 01, 2020 at 06:18:18PM -0700, Matt Roper wrote:
> On Wed, Apr 29, 2020 at 01:10:26PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > The MSG_FBC_REND_STATE register only exists on snb+. For older
> 
> I only find this register in the bspec for HSW+.  Is the spec incomplete
> or am I looking in the wrong place?

The docs are a bit of a mess around this area. IIRC this rcs nuke
workaround was documented for ivb+ (presumably due to ppgtt).
I thinka the bltter counterpart (part of the BCS_ECOSKPD dance)
was documented for SNB as well which implies the register is there
and working. Also the fact that the code works does confirm that.

We're not really following much of the documented stuff for 
FBC since we basically don't use the hardware tracking all.
So the value of the docs is mostly in finding the right bits
to cause nukes and turn off hw tracking as much as possible.

> 
> It's a bit hard to review these changes for older platforms since there
> doesn't really seem to be much FBC/DPFC documentation at all in the
> bspec until we get to BDW and beyond.  The only explicit mention I can
> find of nuke-on-flip for older platforms is a SNB-specific bit in
> FBC_CTL that disables that behavior.  Do you have other documents that
> clarify that this will indeed work farther back?

gen2-gen4 bspec has slightly better docs on FBC compared to more recent
platforms. Sadly I've never been able to find a way to trigger a nuke
explicitly, hence we resort to (ab)using a flip nuke.

> 
> 
> Matt
> 
> > 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ä 
> > ---
> >  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 613ab499d42e..983224e07eaf 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > @@ -188,8 +188,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 = _priv->fbc.params;
> > +   enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
> > +
> > +   spin_lock_irq(_priv->uncore.lock);
> > +   intel_de_write_fw(dev_priv, DSPADDR(i9xx_plane),
> > + intel_de_read_fw(dev_priv, DSPADDR(i9xx_plane)));
> > +   spin_unlock_irq(_priv->uncore.lock);
> > +}
> > +
> > +static void i965_fbc_recompress(struct drm_i915_private *dev_priv)
> > +{
> > +   struct intel_fbc_reg_params *params = _priv->fbc.params;
> > +   enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
> > +
> > +   spin_lock_irq(_priv->uncore.lock);
> > +   intel_de_write_fw(dev_priv, DSPSURF(i9xx_plane),
> > + intel_de_read_fw(dev_priv, DSPSURF(i9xx_plane)));
> > +   spin_unlock_irq(_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 = _priv->fbc;
> >  
> > @@ -199,6 +221,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 = _priv->fbc.params;
> > -- 
> > 2.24.1
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation
> (916) 356-2795

-- 
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 04/12] drm/i915/fbc: Fix nuke for pre-snb platforms

2020-05-01 Thread Matt Roper
On Wed, Apr 29, 2020 at 01:10:26PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> The MSG_FBC_REND_STATE register only exists on snb+. For older

I only find this register in the bspec for HSW+.  Is the spec incomplete
or am I looking in the wrong place?

It's a bit hard to review these changes for older platforms since there
doesn't really seem to be much FBC/DPFC documentation at all in the
bspec until we get to BDW and beyond.  The only explicit mention I can
find of nuke-on-flip for older platforms is a SNB-specific bit in
FBC_CTL that disables that behavior.  Do you have other documents that
clarify that this will indeed work farther back?


Matt

> 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ä 
> ---
>  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 613ab499d42e..983224e07eaf 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -188,8 +188,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 = _priv->fbc.params;
> + enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
> +
> + spin_lock_irq(_priv->uncore.lock);
> + intel_de_write_fw(dev_priv, DSPADDR(i9xx_plane),
> +   intel_de_read_fw(dev_priv, DSPADDR(i9xx_plane)));
> + spin_unlock_irq(_priv->uncore.lock);
> +}
> +
> +static void i965_fbc_recompress(struct drm_i915_private *dev_priv)
> +{
> + struct intel_fbc_reg_params *params = _priv->fbc.params;
> + enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
> +
> + spin_lock_irq(_priv->uncore.lock);
> + intel_de_write_fw(dev_priv, DSPSURF(i9xx_plane),
> +   intel_de_read_fw(dev_priv, DSPSURF(i9xx_plane)));
> + spin_unlock_irq(_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 = _priv->fbc;
>  
> @@ -199,6 +221,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 = _priv->fbc.params;
> -- 
> 2.24.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 04/12] drm/i915/fbc: Fix nuke for pre-snb platforms

2020-04-29 Thread Ville Syrjala
From: Ville Syrjälä 

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ä 
---
 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 613ab499d42e..983224e07eaf 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -188,8 +188,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 = _priv->fbc.params;
+   enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
+
+   spin_lock_irq(_priv->uncore.lock);
+   intel_de_write_fw(dev_priv, DSPADDR(i9xx_plane),
+ intel_de_read_fw(dev_priv, DSPADDR(i9xx_plane)));
+   spin_unlock_irq(_priv->uncore.lock);
+}
+
+static void i965_fbc_recompress(struct drm_i915_private *dev_priv)
+{
+   struct intel_fbc_reg_params *params = _priv->fbc.params;
+   enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
+
+   spin_lock_irq(_priv->uncore.lock);
+   intel_de_write_fw(dev_priv, DSPSURF(i9xx_plane),
+ intel_de_read_fw(dev_priv, DSPSURF(i9xx_plane)));
+   spin_unlock_irq(_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 = _priv->fbc;
 
@@ -199,6 +221,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 = _priv->fbc.params;
-- 
2.24.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx