| Message ID | 20190926145621.9090-8-anshuman.gupta@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DC3CO Support for TGL | expand |
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 > > >
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)
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(+)