Re: [PATCH] [intel-gfx] drm/i915/csr Added DC5 and DC6 counter register for ICL in debugfs entry.

From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Jyoti Yadav <jyoti.r.yadav@intel.com>
Cc: intel-gfx@lists.freedesktop.org, chris.p.wilson@intel.com
Subject: Re: [PATCH] [intel-gfx] drm/i915/csr Added DC5 and DC6 counter register for ICL in debugfs entry.
Date: Fri, 5 Oct 2018 10:29:49 -0700	[thread overview]
Message-ID: <20181005172949.GD2315@intel.com> (raw)
In-Reply-To: <1538712146-31325-1-git-send-email-jyoti.r.yadav@intel.com>

On Fri, Oct 05, 2018 at 12:02:26AM -0400, Jyoti Yadav wrote:
> DC5 and DC6 counter register tells about residency of DC5 and DC6.
> These registers are same for SKL and ICL.
> 
> v2 : Remove csr_version check.
>      Added generic check regarding DC counters for  Gen9 onwards. (Rodrigo)
> v3 : Simplified gen checks. (Chris)
> v4 : Simplified "if" ladder for multiple gens.
> 
> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 9 ++++-----
>  drivers/gpu/drm/i915/i915_reg.h     | 1 +
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index a5265c2..738f8c7 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2897,15 +2897,14 @@ static int i915_dmc_info(struct seq_file *m, void *unused)
>  	seq_printf(m, "version: %d.%d\n", CSR_VERSION_MAJOR(csr->version),
>  		   CSR_VERSION_MINOR(csr->version));
>  
> -	if (IS_KABYLAKE(dev_priv) ||
> -	    (IS_SKYLAKE(dev_priv) && csr->version >= CSR_VERSION(1, 6))) {
> +	if (IS_BROXTON(dev_priv)) {
> +		seq_printf(m, "DC3 -> DC5 count: %d\n",
> +			   I915_READ(BXT_CSR_DC3_DC5_COUNT));
> +	} else if (IS_GEN(dev_priv, 9, 11)) {
>  		seq_printf(m, "DC3 -> DC5 count: %d\n",
>  			   I915_READ(SKL_CSR_DC3_DC5_COUNT));
>  		seq_printf(m, "DC5 -> DC6 count: %d\n",
>  			   I915_READ(SKL_CSR_DC5_DC6_COUNT));
> -	} else if (IS_BROXTON(dev_priv) && csr->version >= CSR_VERSION(1, 4)) {
> -		seq_printf(m, "DC3 -> DC5 count: %d\n",
> -			   I915_READ(BXT_CSR_DC3_DC5_COUNT));
>  	}
>  
>  out:
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8534f88..573d5f3 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6985,6 +6985,7 @@ enum {
>  /* MMIO address range for CSR program (0x80000 - 0x82FFF) */
>  #define CSR_MMIO_START_RANGE	0x80000
>  #define CSR_MMIO_END_RANGE	0x8FFFF
> +/* DC3_DC5 count and DC5_DC6 count registers are same for SKL and ICL */

This comment is incorrect... it is same for skl, kbl, cfl, cnl, whl, aml,
and icl.

But it is also useless because the code is clear already.

And not needed, because "SKL_" prefix shows already
started on SKL, but can be used for SKL+ or not...

Also it has a risk of getting outdated and forgotten.

So, with this removed:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Sorry for not spotting this earlier.

>  #define SKL_CSR_DC3_DC5_COUNT	_MMIO(0x80030)
>  #define SKL_CSR_DC5_DC6_COUNT	_MMIO(0x8002C)
>  #define BXT_CSR_DC3_DC5_COUNT	_MMIO(0x80038)
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx