Re: [Intel-gfx] [PATCH 4/6] drm/i915/debugfs: Add perf_limit_reasons in debugfs

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Tue, 06 Sep 2022 07:13:03 -0700, Rodrigo Vivi wrote:
>

Copying author.

> On Fri, Sep 02, 2022 at 04:53:00PM -0700, Ashutosh Dixit wrote:
> > From: Tilak Tangudu <tilak.tangudu@intel.com>
> >
> > Add perf_limit_reasons in debugfs. Unlike the lower 16 perf_limit_reasons
> > status bits, the upper 16 log bits remain set until cleared, thereby
> > ensuring the throttling occurrence is not missed. The clear fop clears
> > the upper 16 log bits, the get fop gets all 32 log and status bits.
> >
> > Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 27 +++++++++++++++++++
> >  drivers/gpu/drm/i915/i915_reg.h               |  1 +
> >  2 files changed, 28 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> > index 108b9e76c32e..5c95cba5e5df 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> > @@ -655,6 +655,32 @@ static bool rps_eval(void *data)
> >
> >  DEFINE_INTEL_GT_DEBUGFS_ATTRIBUTE(rps_boost);
> >
> > +static int perf_limit_reasons_get(void *data, u64 *val)
> > +{
> > +	struct intel_gt *gt = data;
> > +	intel_wakeref_t wakeref;
> > +
> > +	with_intel_runtime_pm(gt->uncore->rpm, wakeref)
> > +		*val = intel_uncore_read(gt->uncore, GT0_PERF_LIMIT_REASONS);
> > +
> > +	return 0;
> > +}
> > +
> > +static int perf_limit_reasons_clear(void *data, u64 val)
> > +{
> > +	struct intel_gt *gt = data;
> > +	intel_wakeref_t wakeref;
> > +
> > +	/* Clear the upper 16 log bits, the lower 16 status bits are read-only */
> > +	with_intel_runtime_pm(gt->uncore->rpm, wakeref)
> > +		intel_uncore_rmw(gt->uncore, GT0_PERF_LIMIT_REASONS,
> > +				 GT0_PERF_LIMIT_REASONS_LOG_MASK, 0);
> > +
> > +	return 0;
> > +}
> > +DEFINE_SIMPLE_ATTRIBUTE(perf_limit_reasons_fops, perf_limit_reasons_get,
> > +			perf_limit_reasons_clear, "%llu\n");
> > +
> >  void intel_gt_pm_debugfs_register(struct intel_gt *gt, struct dentry *root)
> >  {
> >	static const struct intel_gt_debugfs_file files[] = {
> > @@ -664,6 +690,7 @@ void intel_gt_pm_debugfs_register(struct intel_gt *gt, struct dentry *root)
> >		{ "forcewake_user", &forcewake_user_fops, NULL},
> >		{ "llc", &llc_fops, llc_eval },
> >		{ "rps_boost", &rps_boost_fops, rps_eval },
> > +		{ "perf_limit_reasons", &perf_limit_reasons_fops, NULL },
> >	};
> >
> >	intel_gt_debugfs_register_files(root, files, ARRAY_SIZE(files), gt);
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 5e6239864c35..10126995e1f6 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -1802,6 +1802,7 @@
> >  #define   POWER_LIMIT_4_MASK		REG_BIT(9)
> >  #define   POWER_LIMIT_1_MASK		REG_BIT(11)
> >  #define   POWER_LIMIT_2_MASK		REG_BIT(12)
> > +#define   GT0_PERF_LIMIT_REASONS_LOG_MASK REG_GENMASK(31, 16)
>
> Is this valid for all platforms?
> What does the bits are really telling us?
> Could we expand the reasons? The previous bits we know exactly
> what kind of limits we are dealing of, but with this combined
> one without any explanation I'm afraid this will bring more
> confusion than help. We will get bugged by many folks trying
> to debug this out there when bit 13, for instance, is set.
> "What does bit 13 mean?" will be a recurrent question with
> only a tribal knowledge kind of answer.
>
> >
> >  #define CHV_CLK_CTL1			_MMIO(0x101100)
> >  #define VLV_CLK_CTL2			_MMIO(0x101104)
> > --
> > 2.34.1
> >