[PATCH] drm/i915: Implement fbc_status "Compressing" info for all platforms

[PATCH] drm/i915: Implement fbc_status "Compressing" info for all platforms

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The number of compressed segments has been available ever since
FBC2 was introduced in g4x, it just moved from the STATUS register
into STATUS2 on IVB.

For FBC1 if we really wanted the number of compressed segments we'd
have to trawl through the tags, but in this case since the code just
uses the number of compressed segments as an indicator whether
compression has occurred we can just check the state of the
COMPRESSING and COMPRESSED bits. IIRC the hardware will try to
periodically recompress all uncompressed lines even if they haven't
changed and the COMPRESSED bit will be cleared while the compressor
is running, so just checking the COMPRESSED bit might not give us
the right answer. Hence it seems better to check for both
COMPRESSED and COMPRESSING as that should tell us that the
compressor is at least trying to do something.

While at it move the IVB+ register define to the right place, unify
the naming convention of the compressed segment count masks, and
fix up the mask for g4x.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 22 ++++++++++++++++------
 drivers/gpu/drm/i915/i915_reg.h     | 10 +++++-----
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 3b088685a553..126f1e8a9d0b 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1670,12 +1670,22 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
 		seq_printf(m, "FBC disabled: %s\n",
 			   dev_priv->fbc.no_fbc_reason);
 
-	if (intel_fbc_is_active(dev_priv) && INTEL_GEN(dev_priv) >= 7) {
-		uint32_t mask = INTEL_GEN(dev_priv) >= 8 ?
-				BDW_FBC_COMPRESSION_MASK :
-				IVB_FBC_COMPRESSION_MASK;
-		seq_printf(m, "Compressing: %s\n",
-			   yesno(I915_READ(FBC_STATUS2) & mask));
+	if (intel_fbc_is_active(dev_priv)) {
+		u32 mask;
+
+		if (INTEL_GEN(dev_priv) >= 8)
+			mask = I915_READ(ILK_DPFC_STATUS2) & BDW_DPFC_COMP_SEG_MASK;
+		else if (INTEL_GEN(dev_priv) >= 7)
+			mask = I915_READ(ILK_DPFC_STATUS2) & IVB_DPFC_COMP_SEG_MASK;
+		else if (INTEL_GEN(dev_priv) >= 5)
+			mask = I915_READ(ILK_DPFC_STATUS) & ILK_DPFC_COMP_SEG_MASK;
+		else if (IS_G4X(dev_priv))
+			mask = I915_READ(DPFC_STATUS) & DPFC_COMP_SEG_MASK;
+		else
+			mask = I915_READ(FBC_STATUS) & (FBC_STAT_COMPRESSING |
+							FBC_STAT_COMPRESSED);
+
+		seq_printf(m, "Compressing: %s\n", yesno(mask));
 	}
 
 	mutex_unlock(&dev_priv->fbc.lock);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 231ee86625cd..23bdc079575c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2507,10 +2507,6 @@ enum skl_disp_power_wells {
 #define FBC_FENCE_OFF		_MMIO(0x3218) /* BSpec typo has 321Bh */
 #define FBC_TAG(i)		_MMIO(0x3300 + (i) * 4)
 
-#define FBC_STATUS2			_MMIO(0x43214)
-#define  IVB_FBC_COMPRESSION_MASK	0x7ff
-#define  BDW_FBC_COMPRESSION_MASK	0xfff
-
 #define FBC_LL_SIZE		(1536)
 
 #define FBC_LLC_READ_CTRL	_MMIO(0x9044)
@@ -2539,7 +2535,7 @@ enum skl_disp_power_wells {
 #define   DPFC_INVAL_SEG_SHIFT  (16)
 #define   DPFC_INVAL_SEG_MASK	(0x07ff0000)
 #define   DPFC_COMP_SEG_SHIFT	(0)
-#define   DPFC_COMP_SEG_MASK	(0x000003ff)
+#define   DPFC_COMP_SEG_MASK	(0x000007ff)
 #define DPFC_STATUS2		_MMIO(0x3214)
 #define DPFC_FENCE_YOFF		_MMIO(0x3218)
 #define DPFC_CHICKEN		_MMIO(0x3224)
@@ -2553,6 +2549,10 @@ enum skl_disp_power_wells {
 #define   DPFC_RESERVED		(0x1FFFFF00)
 #define ILK_DPFC_RECOMP_CTL	_MMIO(0x4320c)
 #define ILK_DPFC_STATUS		_MMIO(0x43210)
+#define  ILK_DPFC_COMP_SEG_MASK	0x7ff
+#define ILK_DPFC_STATUS2	_MMIO(0x43214)
+#define  IVB_DPFC_COMP_SEG_MASK	0x7ff
+#define  BDW_DPFC_COMP_SEG_MASK	0xfff
 #define ILK_DPFC_FENCE_YOFF	_MMIO(0x43218)
 #define ILK_DPFC_CHICKEN	_MMIO(0x43224)
 #define   ILK_DPFC_DISABLE_DUMMY0 (1<<8)
-- 
2.10.2

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

✓ Fi.CI.BAT: success for drm/i915: Implement fbc_status "Compressing" info for all platforms

[Patchwork]

== Series Details ==

Series: drm/i915: Implement fbc_status "Compressing" info for all platforms
URL   : https://patchwork.freedesktop.org/series/25176/
State : success

== Summary ==

Series 25176v1 drm/i915: Implement fbc_status "Compressing" info for all platforms
https://patchwork.freedesktop.org/api/1.0/series/25176/revisions/1/mbox/

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:447s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:435s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:580s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:508s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:487s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:483s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:432s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:410s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:416s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:499s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:467s
fi-kbl-7500u     total:278  pass:255  dwarn:5   dfail:0   fail:0   skip:18  time:463s
fi-kbl-7560u     total:278  pass:263  dwarn:5   dfail:0   fail:0   skip:10  time:568s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:457s
fi-skl-6700hq    total:278  pass:239  dwarn:0   dfail:1   fail:17  skip:21  time:427s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:470s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:504s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:436s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:532s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:400s

2c9abf8ec88cf4b3d3735976bb0ff7a4991946b2 drm-tip: 2017y-06m-01d-13h-29m-05s UTC integration manifest
c5201c2 drm/i915: Implement fbc_status "Compressing" info for all platforms

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4860/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Implement fbc_status "Compressing" info for all platforms

[Paulo Zanoni]

Em Qui, 2017-06-01 às 21:02 +0300, ville.syrjala@linux.intel.com
escreveu:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The number of compressed segments has been available ever since
> FBC2 was introduced in g4x, it just moved from the STATUS register
> into STATUS2 on IVB.
> 
> For FBC1 if we really wanted the number of compressed segments we'd
> have to trawl through the tags, but in this case since the code just
> uses the number of compressed segments as an indicator whether
> compression has occurred we can just check the state of the
> COMPRESSING and COMPRESSED bits. IIRC the hardware will try to
> periodically recompress all uncompressed lines even if they haven't
> changed and the COMPRESSED bit will be cleared while the compressor
> is running, so just checking the COMPRESSED bit might not give us
> the right answer. Hence it seems better to check for both
> COMPRESSED and COMPRESSING as that should tell us that the
> compressor is at least trying to do something.
> 
> While at it move the IVB+ register define to the right place, unify
> the naming convention of the compressed segment count masks, and
> fix up the mask for g4x.
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 22 ++++++++++++++++------
>  drivers/gpu/drm/i915/i915_reg.h     | 10 +++++-----
>  2 files changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 3b088685a553..126f1e8a9d0b 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1670,12 +1670,22 @@ static int i915_fbc_status(struct seq_file
> *m, void *unused)
>  		seq_printf(m, "FBC disabled: %s\n",
>  			   dev_priv->fbc.no_fbc_reason);
>  
> -	if (intel_fbc_is_active(dev_priv) && INTEL_GEN(dev_priv) >=
> 7) {
> -		uint32_t mask = INTEL_GEN(dev_priv) >= 8 ?
> -				BDW_FBC_COMPRESSION_MASK :
> -				IVB_FBC_COMPRESSION_MASK;
> -		seq_printf(m, "Compressing: %s\n",
> -			   yesno(I915_READ(FBC_STATUS2) & mask));
> +	if (intel_fbc_is_active(dev_priv)) {
> +		u32 mask;

Someone should sed our whole driver so it either fully uses u32 or
uint32_t.


> +
> +		if (INTEL_GEN(dev_priv) >= 8)
> +			mask = I915_READ(ILK_DPFC_STATUS2) &
> BDW_DPFC_COMP_SEG_MASK;
> +		else if (INTEL_GEN(dev_priv) >= 7)
> +			mask = I915_READ(ILK_DPFC_STATUS2) &
> IVB_DPFC_COMP_SEG_MASK;
> +		else if (INTEL_GEN(dev_priv) >= 5)
> +			mask = I915_READ(ILK_DPFC_STATUS) &
> ILK_DPFC_COMP_SEG_MASK;
> +		else if (IS_G4X(dev_priv))
> +			mask = I915_READ(DPFC_STATUS) &
> DPFC_COMP_SEG_MASK;
> +		else
> +			mask = I915_READ(FBC_STATUS) &
> (FBC_STAT_COMPRESSING |
> +							FBC_STAT_COM
> PRESSED);
> +
> +		seq_printf(m, "Compressing: %s\n", yesno(mask));
>  	}
>  
>  	mutex_unlock(&dev_priv->fbc.lock);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 231ee86625cd..23bdc079575c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2507,10 +2507,6 @@ enum skl_disp_power_wells {
>  #define FBC_FENCE_OFF		_MMIO(0x3218) /* BSpec typo has
> 321Bh */
>  #define FBC_TAG(i)		_MMIO(0x3300 + (i) * 4)
>  
> -#define FBC_STATUS2			_MMIO(0x43214)
> -#define  IVB_FBC_COMPRESSION_MASK	0x7ff
> -#define  BDW_FBC_COMPRESSION_MASK	0xfff
> -
>  #define FBC_LL_SIZE		(1536)
>  
>  #define FBC_LLC_READ_CTRL	_MMIO(0x9044)
> @@ -2539,7 +2535,7 @@ enum skl_disp_power_wells {
>  #define   DPFC_INVAL_SEG_SHIFT  (16)
>  #define   DPFC_INVAL_SEG_MASK	(0x07ff0000)
>  #define   DPFC_COMP_SEG_SHIFT	(0)
> -#define   DPFC_COMP_SEG_MASK	(0x000003ff)
> +#define   DPFC_COMP_SEG_MASK	(0x000007ff)
>  #define DPFC_STATUS2		_MMIO(0x3214)
>  #define DPFC_FENCE_YOFF		_MMIO(0x3218)
>  #define DPFC_CHICKEN		_MMIO(0x3224)
> @@ -2553,6 +2549,10 @@ enum skl_disp_power_wells {
>  #define   DPFC_RESERVED		(0x1FFFFF00)
>  #define ILK_DPFC_RECOMP_CTL	_MMIO(0x4320c)
>  #define ILK_DPFC_STATUS		_MMIO(0x43210)
> +#define  ILK_DPFC_COMP_SEG_MASK	0x7ff
> +#define ILK_DPFC_STATUS2	_MMIO(0x43214)

I'm fine with using ILK names for something that appeared on ILK, but I
can't find ILK_DPFC_STATUS2 on my ILK docs. It seems to me that STATUS2
only appeared on IVB, and it's named FBC_STATUS2 there.

If that's the case, ideally we should call it FBC_STATUS2, because
that's the name. If you really insist, I could very reluctantly go with
IVB_DPFC_STATUS2, although I really don't like the idea of going with a
name that's nowhere in our docs. We should really only keep it as
ILK_DPFC_STATUS2 if the register indeed was added on ILK (and in this
case, please give me pointers to the appropriate doc).

For the gen5+ part of the patch, if you rename the STATUS2 register,
consider it Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>.

For the gen 2 - 4 part, I really didn't check the docs. But FBC is
disabled by default on these platforms anyway and assumed to be broken,
so: Acked-by: Paulo Zanoni <paulo.r.zanoni@intel.com>.



> +#define  IVB_DPFC_COMP_SEG_MASK	0x7ff
> +#define  BDW_DPFC_COMP_SEG_MASK	0xfff
>  #define ILK_DPFC_FENCE_YOFF	_MMIO(0x43218)
>  #define ILK_DPFC_CHICKEN	_MMIO(0x43224)
>  #define   ILK_DPFC_DISABLE_DUMMY0 (1<<8)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Implement fbc_status "Compressing" info for all platforms

[Gabriel Krisman Bertazi]

ville.syrjala@linux.intel.com writes:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The number of compressed segments has been available ever since
> FBC2 was introduced in g4x, it just moved from the STATUS register
> into STATUS2 on IVB.
>
> For FBC1 if we really wanted the number of compressed segments we'd
> have to trawl through the tags, but in this case since the code just
> uses the number of compressed segments as an indicator whether
> compression has occurred we can just check the state of the
> COMPRESSING and COMPRESSED bits. IIRC the hardware will try to
> periodically recompress all uncompressed lines even if they haven't
> changed and the COMPRESSED bit will be cleared while the compressor
> is running, so just checking the COMPRESSED bit might not give us
> the right answer. Hence it seems better to check for both
> COMPRESSED and COMPRESSING as that should tell us that the
> compressor is at least trying to do something.
>
> While at it move the IVB+ register define to the right place, unify
> the naming convention of the compressed segment count masks, and
> fix up the mask for g4x.
>

Hi Ville,

Feel free to add

Tested-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk> # SNB

-- 
Gabriel Krisman Bertazi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Implement fbc_status "Compressing" info for all platforms

[Jani Nikula]

On Mon, 05 Jun 2017, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> Em Qui, 2017-06-01 às 21:02 +0300, ville.syrjala@linux.intel.com
> escreveu:
>> +		u32 mask;
>
> Someone should sed our whole driver so it either fully uses u32 or
> uint32_t.

Kernel fixed size typedefs should be preferred over the C typedefs.

BR,
Jani.

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

Re: [PATCH] drm/i915: Implement fbc_status "Compressing" info for all platforms

[Ville Syrjälä]

On Mon, Jun 05, 2017 at 05:23:17PM -0300, Paulo Zanoni wrote:
> Em Qui, 2017-06-01 às 21:02 +0300, ville.syrjala@linux.intel.com
> escreveu:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The number of compressed segments has been available ever since
> > FBC2 was introduced in g4x, it just moved from the STATUS register
> > into STATUS2 on IVB.
> > 
> > For FBC1 if we really wanted the number of compressed segments we'd
> > have to trawl through the tags, but in this case since the code just
> > uses the number of compressed segments as an indicator whether
> > compression has occurred we can just check the state of the
> > COMPRESSING and COMPRESSED bits. IIRC the hardware will try to
> > periodically recompress all uncompressed lines even if they haven't
> > changed and the COMPRESSED bit will be cleared while the compressor
> > is running, so just checking the COMPRESSED bit might not give us
> > the right answer. Hence it seems better to check for both
> > COMPRESSED and COMPRESSING as that should tell us that the
> > compressor is at least trying to do something.
> > 
> > While at it move the IVB+ register define to the right place, unify
> > the naming convention of the compressed segment count masks, and
> > fix up the mask for g4x.
> > 
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 22 ++++++++++++++++------
> >  drivers/gpu/drm/i915/i915_reg.h     | 10 +++++-----
> >  2 files changed, 21 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 3b088685a553..126f1e8a9d0b 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -1670,12 +1670,22 @@ static int i915_fbc_status(struct seq_file
> > *m, void *unused)
> >  		seq_printf(m, "FBC disabled: %s\n",
> >  			   dev_priv->fbc.no_fbc_reason);
> >  
> > -	if (intel_fbc_is_active(dev_priv) && INTEL_GEN(dev_priv) >=
> > 7) {
> > -		uint32_t mask = INTEL_GEN(dev_priv) >= 8 ?
> > -				BDW_FBC_COMPRESSION_MASK :
> > -				IVB_FBC_COMPRESSION_MASK;
> > -		seq_printf(m, "Compressing: %s\n",
> > -			   yesno(I915_READ(FBC_STATUS2) & mask));
> > +	if (intel_fbc_is_active(dev_priv)) {
> > +		u32 mask;
> 
> Someone should sed our whole driver so it either fully uses u32 or
> uint32_t.
> 
> 
> > +
> > +		if (INTEL_GEN(dev_priv) >= 8)
> > +			mask = I915_READ(ILK_DPFC_STATUS2) &
> > BDW_DPFC_COMP_SEG_MASK;
> > +		else if (INTEL_GEN(dev_priv) >= 7)
> > +			mask = I915_READ(ILK_DPFC_STATUS2) &
> > IVB_DPFC_COMP_SEG_MASK;
> > +		else if (INTEL_GEN(dev_priv) >= 5)
> > +			mask = I915_READ(ILK_DPFC_STATUS) &
> > ILK_DPFC_COMP_SEG_MASK;
> > +		else if (IS_G4X(dev_priv))
> > +			mask = I915_READ(DPFC_STATUS) &
> > DPFC_COMP_SEG_MASK;
> > +		else
> > +			mask = I915_READ(FBC_STATUS) &
> > (FBC_STAT_COMPRESSING |
> > +							FBC_STAT_COM
> > PRESSED);
> > +
> > +		seq_printf(m, "Compressing: %s\n", yesno(mask));
> >  	}
> >  
> >  	mutex_unlock(&dev_priv->fbc.lock);
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 231ee86625cd..23bdc079575c 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2507,10 +2507,6 @@ enum skl_disp_power_wells {
> >  #define FBC_FENCE_OFF		_MMIO(0x3218) /* BSpec typo has
> > 321Bh */
> >  #define FBC_TAG(i)		_MMIO(0x3300 + (i) * 4)
> >  
> > -#define FBC_STATUS2			_MMIO(0x43214)
> > -#define  IVB_FBC_COMPRESSION_MASK	0x7ff
> > -#define  BDW_FBC_COMPRESSION_MASK	0xfff
> > -
> >  #define FBC_LL_SIZE		(1536)
> >  
> >  #define FBC_LLC_READ_CTRL	_MMIO(0x9044)
> > @@ -2539,7 +2535,7 @@ enum skl_disp_power_wells {
> >  #define   DPFC_INVAL_SEG_SHIFT  (16)
> >  #define   DPFC_INVAL_SEG_MASK	(0x07ff0000)
> >  #define   DPFC_COMP_SEG_SHIFT	(0)
> > -#define   DPFC_COMP_SEG_MASK	(0x000003ff)
> > +#define   DPFC_COMP_SEG_MASK	(0x000007ff)
> >  #define DPFC_STATUS2		_MMIO(0x3214)
> >  #define DPFC_FENCE_YOFF		_MMIO(0x3218)
> >  #define DPFC_CHICKEN		_MMIO(0x3224)
> > @@ -2553,6 +2549,10 @@ enum skl_disp_power_wells {
> >  #define   DPFC_RESERVED		(0x1FFFFF00)
> >  #define ILK_DPFC_RECOMP_CTL	_MMIO(0x4320c)
> >  #define ILK_DPFC_STATUS		_MMIO(0x43210)
> > +#define  ILK_DPFC_COMP_SEG_MASK	0x7ff
> > +#define ILK_DPFC_STATUS2	_MMIO(0x43214)
> 
> I'm fine with using ILK names for something that appeared on ILK, but I
> can't find ILK_DPFC_STATUS2 on my ILK docs. It seems to me that STATUS2
> only appeared on IVB, and it's named FBC_STATUS2 there.

Hmm. Indeed it looks like it's not documented for ILK/SNB. But I
suspect it's actually there since g4x had it already. But I guess
I could rename it to IVB_FBC_STATUS2 or something like that just
to match the docs.

> 
> If that's the case, ideally we should call it FBC_STATUS2, because
> that's the name. If you really insist, I could very reluctantly go with
> IVB_DPFC_STATUS2, although I really don't like the idea of going with a
> name that's nowhere in our docs. We should really only keep it as
> ILK_DPFC_STATUS2 if the register indeed was added on ILK (and in this
> case, please give me pointers to the appropriate doc).
> 
> For the gen5+ part of the patch, if you rename the STATUS2 register,
> consider it Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>.
> 
> For the gen 2 - 4 part, I really didn't check the docs. But FBC is
> disabled by default on these platforms anyway and assumed to be broken,
> so: Acked-by: Paulo Zanoni <paulo.r.zanoni@intel.com>.
> 
> 
> 
> > +#define  IVB_DPFC_COMP_SEG_MASK	0x7ff
> > +#define  BDW_DPFC_COMP_SEG_MASK	0xfff
> >  #define ILK_DPFC_FENCE_YOFF	_MMIO(0x43218)
> >  #define ILK_DPFC_CHICKEN	_MMIO(0x43224)
> >  #define   ILK_DPFC_DISABLE_DUMMY0 (1<<8)

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

[PATCH v2] drm/i915: Implement fbc_status "Compressing" info for all platforms

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The number of compressed segments has been available ever since
FBC2 was introduced in g4x, it just moved from the STATUS register
into STATUS2 on IVB.

For FBC1 if we really wanted the number of compressed segments we'd
have to trawl through the tags, but in this case since the code just
uses the number of compressed segments as an indicator whether
compression has occurred we can just check the state of the
COMPRESSING and COMPRESSED bits. IIRC the hardware will try to
periodically recompress all uncompressed lines even if they haven't
changed and the COMPRESSED bit will be cleared while the compressor
is running, so just checking the COMPRESSED bit might not give us
the right answer. Hence it seems better to check for both
COMPRESSED and COMPRESSING as that should tell us that the
compressor is at least trying to do something.

While at it move the IVB+ register define to the right place, unify
the naming convention of the compressed segment count masks, and
fix up the mask for g4x.

v2: s/ILK_DPFC_STATUS2/IVB_FBC_STATUS2/ (Paulo)

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Tested-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk> # SNB
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> # ilk+
Acked-by: Paulo Zanoni <paulo.r.zanoni@intel.com> # pre-ilk
---
 drivers/gpu/drm/i915/i915_debugfs.c | 22 ++++++++++++++++------
 drivers/gpu/drm/i915/i915_reg.h     | 10 +++++-----
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 3b088685a553..8fdb911344b3 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1670,12 +1670,22 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
 		seq_printf(m, "FBC disabled: %s\n",
 			   dev_priv->fbc.no_fbc_reason);
 
-	if (intel_fbc_is_active(dev_priv) && INTEL_GEN(dev_priv) >= 7) {
-		uint32_t mask = INTEL_GEN(dev_priv) >= 8 ?
-				BDW_FBC_COMPRESSION_MASK :
-				IVB_FBC_COMPRESSION_MASK;
-		seq_printf(m, "Compressing: %s\n",
-			   yesno(I915_READ(FBC_STATUS2) & mask));
+	if (intel_fbc_is_active(dev_priv)) {
+		u32 mask;
+
+		if (INTEL_GEN(dev_priv) >= 8)
+			mask = I915_READ(IVB_FBC_STATUS2) & BDW_FBC_COMP_SEG_MASK;
+		else if (INTEL_GEN(dev_priv) >= 7)
+			mask = I915_READ(IVB_FBC_STATUS2) & IVB_FBC_COMP_SEG_MASK;
+		else if (INTEL_GEN(dev_priv) >= 5)
+			mask = I915_READ(ILK_DPFC_STATUS) & ILK_DPFC_COMP_SEG_MASK;
+		else if (IS_G4X(dev_priv))
+			mask = I915_READ(DPFC_STATUS) & DPFC_COMP_SEG_MASK;
+		else
+			mask = I915_READ(FBC_STATUS) & (FBC_STAT_COMPRESSING |
+							FBC_STAT_COMPRESSED);
+
+		seq_printf(m, "Compressing: %s\n", yesno(mask));
 	}
 
 	mutex_unlock(&dev_priv->fbc.lock);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1329420f4a1e..ac0bf2364efa 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2507,10 +2507,6 @@ enum skl_disp_power_wells {
 #define FBC_FENCE_OFF		_MMIO(0x3218) /* BSpec typo has 321Bh */
 #define FBC_TAG(i)		_MMIO(0x3300 + (i) * 4)
 
-#define FBC_STATUS2			_MMIO(0x43214)
-#define  IVB_FBC_COMPRESSION_MASK	0x7ff
-#define  BDW_FBC_COMPRESSION_MASK	0xfff
-
 #define FBC_LL_SIZE		(1536)
 
 #define FBC_LLC_READ_CTRL	_MMIO(0x9044)
@@ -2539,7 +2535,7 @@ enum skl_disp_power_wells {
 #define   DPFC_INVAL_SEG_SHIFT  (16)
 #define   DPFC_INVAL_SEG_MASK	(0x07ff0000)
 #define   DPFC_COMP_SEG_SHIFT	(0)
-#define   DPFC_COMP_SEG_MASK	(0x000003ff)
+#define   DPFC_COMP_SEG_MASK	(0x000007ff)
 #define DPFC_STATUS2		_MMIO(0x3214)
 #define DPFC_FENCE_YOFF		_MMIO(0x3218)
 #define DPFC_CHICKEN		_MMIO(0x3224)
@@ -2553,6 +2549,10 @@ enum skl_disp_power_wells {
 #define   DPFC_RESERVED		(0x1FFFFF00)
 #define ILK_DPFC_RECOMP_CTL	_MMIO(0x4320c)
 #define ILK_DPFC_STATUS		_MMIO(0x43210)
+#define  ILK_DPFC_COMP_SEG_MASK	0x7ff
+#define IVB_FBC_STATUS2		_MMIO(0x43214)
+#define  IVB_FBC_COMP_SEG_MASK	0x7ff
+#define  BDW_FBC_COMP_SEG_MASK	0xfff
 #define ILK_DPFC_FENCE_YOFF	_MMIO(0x43218)
 #define ILK_DPFC_CHICKEN	_MMIO(0x43224)
 #define   ILK_DPFC_DISABLE_DUMMY0 (1<<8)
-- 
2.13.0

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

✓ Fi.CI.BAT: success for drm/i915: Implement fbc_status "Compressing" info for all platforms (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Implement fbc_status "Compressing" info for all platforms (rev2)
URL   : https://patchwork.freedesktop.org/series/25176/
State : success

== Summary ==

Series 25176v2 drm/i915: Implement fbc_status "Compressing" info for all platforms
https://patchwork.freedesktop.org/api/1.0/series/25176/revisions/2/mbox/

Test kms_busy:
        Subgroup basic-flip-default-b:
                dmesg-warn -> PASS       (fi-skl-6700hq) fdo#101144 +2
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                fail       -> PASS       (fi-skl-6700hq) fdo#101154 +7

fdo#101144 https://bugs.freedesktop.org/show_bug.cgi?id=101144
fdo#101154 https://bugs.freedesktop.org/show_bug.cgi?id=101154

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:448s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:431s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:574s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:509s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:489s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:478s
fi-glk-2a        total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:592s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:431s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:411s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:418s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:495s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:475s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:471s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:579s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:463s
fi-skl-6700hq    total:278  pass:239  dwarn:0   dfail:1   fail:17  skip:21  time:427s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:468s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:498s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:438s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:532s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:407s

30d3326ecb407cad6c03ef6a6d3805c70ba9f0a9 drm-tip: 2017y-06m-05d-15h-21m-50s UTC integration manifest
d3909e9 drm/i915: Implement fbc_status "Compressing" info for all platforms

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4887/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Implement fbc_status "Compressing" info for all platforms

[Ville Syrjälä]

On Tue, Jun 06, 2017 at 03:43:18PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The number of compressed segments has been available ever since
> FBC2 was introduced in g4x, it just moved from the STATUS register
> into STATUS2 on IVB.
> 
> For FBC1 if we really wanted the number of compressed segments we'd
> have to trawl through the tags, but in this case since the code just
> uses the number of compressed segments as an indicator whether
> compression has occurred we can just check the state of the
> COMPRESSING and COMPRESSED bits. IIRC the hardware will try to
> periodically recompress all uncompressed lines even if they haven't
> changed and the COMPRESSED bit will be cleared while the compressor
> is running, so just checking the COMPRESSED bit might not give us
> the right answer. Hence it seems better to check for both
> COMPRESSED and COMPRESSING as that should tell us that the
> compressor is at least trying to do something.
> 
> While at it move the IVB+ register define to the right place, unify
> the naming convention of the compressed segment count masks, and
> fix up the mask for g4x.
> 
> v2: s/ILK_DPFC_STATUS2/IVB_FBC_STATUS2/ (Paulo)
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Tested-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk> # SNB
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> # ilk+
> Acked-by: Paulo Zanoni <paulo.r.zanoni@intel.com> # pre-ilk

Pushed to dinq. Thanks for the review and testing.

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