Em Qui, 2017-06-01 às 21:02 +0300, ville.syrjala@xxxxxxxxxxxxxxx
escreveu:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxx>
> Cc: Gabriel Krisman Bertazi <krisman@xxxxxxxxxxxxxxx>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxx>.
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@xxxxxxxxx>.
> +#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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
