[v9,7/7] drm/i915/tgl: Add DC3CO counter in i915_dmc_info

Submitted by Anshuamn Gupta on Sept. 26, 2019, 2:56 p.m.

Details

Message ID 20190926145621.9090-8-anshuman.gupta@intel.com
State New
Headers show
Series "DC3CO Support for TGL" ( rev: 12 ) in Intel GFX

Browsing this patch as part of:
"DC3CO Support for TGL" rev 12 in Intel GFX
<< prev patch [7/7] next patch >>

Commit Message

Anshuamn Gupta Sept. 26, 2019, 2:56 p.m.
Adding DC3CO counter in i915_dmc_info debugfs will be
useful for DC3CO validation.
DMC firmware uses DMC_DEBUG3 register as DC3CO counter
register on TGL, as per B.Specs DMC_DEBUG3 is general
purpose register.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 6 ++++++
 drivers/gpu/drm/i915/i915_reg.h     | 2 ++
 2 files changed, 8 insertions(+)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index b5b449a88cf1..8a16bbd31212 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2407,6 +2407,12 @@  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));
 
+	/*
+	 * TGL DMC f/w uses DMC_DEBUG3 register for DC3CO counter.
+	 */
+	if (IS_TIGERLAKE(dev_priv))
+		seq_printf(m, "DC3CO count: %d\n", I915_READ(DMC_DEBUG3));
+
 	if (INTEL_GEN(dev_priv) >= 12) {
 		dc5_reg = TGL_DMC_DEBUG_DC5_COUNT;
 		dc6_reg = TGL_DMC_DEBUG_DC6_COUNT;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3ee9720af207..af810f6ed652 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7263,6 +7263,8 @@  enum {
 #define TGL_DMC_DEBUG_DC5_COUNT	_MMIO(0x101084)
 #define TGL_DMC_DEBUG_DC6_COUNT	_MMIO(0x101088)
 
+#define DMC_DEBUG3		_MMIO(0x101090)
+
 /* interrupts */
 #define DE_MASTER_IRQ_CONTROL   (1 << 31)
 #define DE_SPRITEB_FLIP_DONE    (1 << 29)

Comments

On Thu, Sep 26, 2019 at 08:26:21PM +0530, Anshuman Gupta wrote:
> Adding DC3CO counter in i915_dmc_info debugfs will be
> useful for DC3CO validation.
> DMC firmware uses DMC_DEBUG3 register as DC3CO counter
> register on TGL, as per B.Specs DMC_DEBUG3 is general
> purpose register.
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 6 ++++++
>  drivers/gpu/drm/i915/i915_reg.h     | 2 ++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index b5b449a88cf1..8a16bbd31212 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2407,6 +2407,12 @@ 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));
>  
> +	/*
> +	 * TGL DMC f/w uses DMC_DEBUG3 register for DC3CO counter.
> +	 */

The above is obvious from the code itself, so we don't need a comment
for it. Please also consider removing all other comments in the patchset
that state only what is obvious from the code.

> +	if (IS_TIGERLAKE(dev_priv))

The above should match the check in get_allowed_dc_mask().

> +		seq_printf(m, "DC3CO count: %d\n", I915_READ(DMC_DEBUG3));
> +
>  	if (INTEL_GEN(dev_priv) >= 12) {
>  		dc5_reg = TGL_DMC_DEBUG_DC5_COUNT;
>  		dc6_reg = TGL_DMC_DEBUG_DC6_COUNT;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 3ee9720af207..af810f6ed652 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7263,6 +7263,8 @@ enum {
>  #define TGL_DMC_DEBUG_DC5_COUNT	_MMIO(0x101084)
>  #define TGL_DMC_DEBUG_DC6_COUNT	_MMIO(0x101088)
>  
> +#define DMC_DEBUG3		_MMIO(0x101090)
> +
>  /* interrupts */
>  #define DE_MASTER_IRQ_CONTROL   (1 << 31)
>  #define DE_SPRITEB_FLIP_DONE    (1 << 29)
> -- 
> 2.21.0
>
On 2019-09-27 at 19:38:49 +0300, Imre Deak wrote:
> On Thu, Sep 26, 2019 at 08:26:21PM +0530, Anshuman Gupta wrote:
> > Adding DC3CO counter in i915_dmc_info debugfs will be
> > useful for DC3CO validation.
> > DMC firmware uses DMC_DEBUG3 register as DC3CO counter
> > register on TGL, as per B.Specs DMC_DEBUG3 is general
> > purpose register.
> > 
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Animesh Manna <animesh.manna@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 6 ++++++
> >  drivers/gpu/drm/i915/i915_reg.h     | 2 ++
> >  2 files changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index b5b449a88cf1..8a16bbd31212 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2407,6 +2407,12 @@ 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));
> >  
> > +	/*
> > +	 * TGL DMC f/w uses DMC_DEBUG3 register for DC3CO counter.
> > +	 */
> 
> The above is obvious from the code itself, so we don't need a comment
> for it. Please also consider removing all other comments in the patchset
> that state only what is obvious from the code.
DMC_DEBUG3 is a DMC general purpose register, B.Specs doesn't specify
it as DC3CO counter unlike DC5 and DC6, that is why i have added
this comment. Shall i remove this comment considering DMC_DEBUG3 
as general purpose register?
> 
> > +	if (IS_TIGERLAKE(dev_priv))
> 
> The above should match the check in get_allowed_dc_mask().
IS_TIGERLAKE is being checked for the same reason as TGL
DMC is using DMC_DEBUG3 for DC3CO counter. It may not be true
for other Gen12 platfrom.
Thanks,
Anshuman Gupta.  
> 
> > +		seq_printf(m, "DC3CO count: %d\n", I915_READ(DMC_DEBUG3));
> > +
> >  	if (INTEL_GEN(dev_priv) >= 12) {
> >  		dc5_reg = TGL_DMC_DEBUG_DC5_COUNT;
> >  		dc6_reg = TGL_DMC_DEBUG_DC6_COUNT;
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 3ee9720af207..af810f6ed652 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7263,6 +7263,8 @@ enum {
> >  #define TGL_DMC_DEBUG_DC5_COUNT	_MMIO(0x101084)
> >  #define TGL_DMC_DEBUG_DC6_COUNT	_MMIO(0x101088)
> >  
> > +#define DMC_DEBUG3		_MMIO(0x101090)
> > +
> >  /* interrupts */
> >  #define DE_MASTER_IRQ_CONTROL   (1 << 31)
> >  #define DE_SPRITEB_FLIP_DONE    (1 << 29)
> > -- 
> > 2.21.0
> >
On Fri, Sep 27, 2019 at 10:27:44PM +0530, Anshuman Gupta wrote:
> On 2019-09-27 at 19:38:49 +0300, Imre Deak wrote:
> > On Thu, Sep 26, 2019 at 08:26:21PM +0530, Anshuman Gupta wrote:
> > > Adding DC3CO counter in i915_dmc_info debugfs will be
> > > useful for DC3CO validation.
> > > DMC firmware uses DMC_DEBUG3 register as DC3CO counter
> > > register on TGL, as per B.Specs DMC_DEBUG3 is general
> > > purpose register.
> > > 
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Cc: Imre Deak <imre.deak@intel.com>
> > > Cc: Animesh Manna <animesh.manna@intel.com>
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_debugfs.c | 6 ++++++
> > >  drivers/gpu/drm/i915/i915_reg.h     | 2 ++
> > >  2 files changed, 8 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index b5b449a88cf1..8a16bbd31212 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -2407,6 +2407,12 @@ 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));
> > >  
> > > +	/*
> > > +	 * TGL DMC f/w uses DMC_DEBUG3 register for DC3CO counter.
> > > +	 */
> > 
> > The above is obvious from the code itself, so we don't need a comment
> > for it. Please also consider removing all other comments in the patchset
> > that state only what is obvious from the code.
> DMC_DEBUG3 is a DMC general purpose register, B.Specs doesn't specify
> it as DC3CO counter unlike DC5 and DC6, that is why i have added
> this comment. Shall i remove this comment considering DMC_DEBUG3 
> as general purpose register?

Imo that's an issue in bspec, can you file a ticket there? The comment
could explain what the problem is, the above is already obvious from the
code. Something like:

"NOTE: DMC_DEBUG3 is a general purpose reg, its specification is yet to
be finalized in Bspec for the DC3co counter use."

> > 
> > > +	if (IS_TIGERLAKE(dev_priv))
> > 
> > The above should match the check in get_allowed_dc_mask().
> IS_TIGERLAKE is being checked for the same reason as TGL
> DMC is using DMC_DEBUG3 for DC3CO counter. It may not be true
> for other Gen12 platfrom.

Should we enable then DC3co, if we won't be able to debug it?
In any case DMC_DEBUG3 exists for all GEN12 platforms, so I think we can
safely read it.

> Thanks,
> Anshuman Gupta.  
> > 
> > > +		seq_printf(m, "DC3CO count: %d\n", I915_READ(DMC_DEBUG3));
> > > +
> > >  	if (INTEL_GEN(dev_priv) >= 12) {
> > >  		dc5_reg = TGL_DMC_DEBUG_DC5_COUNT;
> > >  		dc6_reg = TGL_DMC_DEBUG_DC6_COUNT;
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index 3ee9720af207..af810f6ed652 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -7263,6 +7263,8 @@ enum {
> > >  #define TGL_DMC_DEBUG_DC5_COUNT	_MMIO(0x101084)
> > >  #define TGL_DMC_DEBUG_DC6_COUNT	_MMIO(0x101088)
> > >  
> > > +#define DMC_DEBUG3		_MMIO(0x101090)
> > > +
> > >  /* interrupts */
> > >  #define DE_MASTER_IRQ_CONTROL   (1 << 31)
> > >  #define DE_SPRITEB_FLIP_DONE    (1 << 29)
> > > -- 
> > > 2.21.0
> > >