[Intel-gfx] [PATCH] drm/i915/rc6: GTC6_RESIDENCY_{LSB, MSB} Residency counter support

[Tvrtko Ursulin]

On 27/09/2022 07:48, Gupta, Anshuman wrote:
> 
>> -----Original Message-----
>> From: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
>> Sent: Monday, September 26, 2022 9:35 PM
>> To: Gupta, Anshuman <anshuman.gupta at intel.com>; intel-
>> gfx at lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [PATCH] drm/i915/rc6: GTC6_RESIDENCY_{LSB, MSB}
>> Residency counter support
>>
>>
>> On 26/09/2022 09:45, Anshuman Gupta wrote:
>>> Adding support in drpc show debugfs to print the GT RPM Unit RC6
>>> residency. This GTC6_RESIDENCY_{LSB, MSB} will only increment when GT
>>> will be RC6. Therefore these register will get reset at RC6 exit and
>>> will start incrementing on next RC6 entry.
>>>
>>> BSpec: 64977
>>> Signed-off-by: Anshuman Gupta <anshuman.gupta at intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c |  5 +++++
>>>    drivers/gpu/drm/i915/gt/intel_gt_regs.h       |  5 +++++
>>>    drivers/gpu/drm/i915/gt/intel_rc6.c           | 19 +++++++++++++++++++
>>>    drivers/gpu/drm/i915/gt/intel_rc6.h           |  1 +
>>>    4 files changed, 30 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 10f680dbd7b62..59b6cc49464e9 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
>>> @@ -195,6 +195,11 @@ static int gen6_drpc(struct seq_file *m)
>>>    	print_rc6_res(m, "RC6 \"Locked to RPn\" residency since boot:",
>>>    		      GEN6_GT_GFX_RC6_LOCKED);
>>>    	print_rc6_res(m, "RC6 residency since boot:", GEN6_GT_GFX_RC6);
>>> +
>>> +	if (GRAPHICS_VER(i915) >= 12)
>>> +		seq_printf(m, "GT RC6 RPM Unit Residency since last RC6 exit:
>> 0x%llx\n",
>>> +			   intel_rc6_rpm_unit_residency(&gt->rc6));
>>> +
>>>    	print_rc6_res(m, "RC6+ residency since boot:", GEN6_GT_GFX_RC6p);
>>>    	print_rc6_res(m, "RC6++ residency since boot:", GEN6_GT_GFX_RC6pp);
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>>> b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>>> index 7f79bbf978284..7715d0aeffc9d 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>>> @@ -8,6 +8,11 @@
>>>
>>>    #include "i915_reg_defs.h"
>>>
>>> +/* GT RPM RC6 counter */
>>> +#define GEN12_GT_GFX_RC6_LSB			_MMIO(0xC20)
>>> +#define GEN12_GT_GFX_RC6_MSB			_MMIO(0xC24)
>>> +#define   GEN12_GT_GFX_RC6_MSB_MASK		REG_GENMASK(23, 0)
>>> +
>>>    /* RPM unit config (Gen8+) */
>>>    #define RPM_CONFIG0				_MMIO(0xd00)
>>>    #define   GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_SHIFT	3
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c
>>> b/drivers/gpu/drm/i915/gt/intel_rc6.c
>>> index f8d0523f4c18e..ee830c4027542 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
>>> @@ -816,6 +816,25 @@ u64 intel_rc6_residency_us(struct intel_rc6 *rc6,
>> i915_reg_t reg)
>>>    	return DIV_ROUND_UP_ULL(intel_rc6_residency_ns(rc6, reg), 1000);
>>>    }
>>>
>>> +u64 intel_rc6_rpm_unit_residency(struct intel_rc6 *rc6) {
>>> +	struct drm_i915_private *i915 = rc6_to_i915(rc6);
>>> +	struct intel_gt *gt = rc6_to_gt(rc6);
>>> +	intel_wakeref_t wakeref;
>>> +	u64 lsb, msb, counter;
>>> +
>>> +	with_intel_runtime_pm(gt->uncore->rpm, wakeref) {
>>> +		lsb = intel_uncore_read(gt->uncore, GEN12_GT_GFX_RC6_LSB);
>>> +		msb = intel_uncore_read(gt->uncore,
>> GEN12_GT_GFX_RC6_MSB);
>>> +	}
>>> +
>>> +	drm_dbg(&i915->drm, "GT RC6 MSB=0x%x LSB=0x%x\n", (u32) msb,
>> (u32) lsb);
>>> +	msb = REG_FIELD_GET(GEN12_GT_GFX_RC6_MSB_MASK, (u32)msb);
>>> +	counter = msb << 32 | lsb;
>>
>> What about wrap?
> Wrap is not practically possible here, as this is 56 bit counter and this will get reset on each rc6 exit.

I primarily was thinking about LSB wrap, see the repeat of the loop 
intel_uncore_read64_2x32 does to handle it. But as you say increment 
period is unknown...

>> I guess you can't use intel_uncore_read64_2x32 because there is something
>> present in bits 31-24?
>>
>> Anyway, what is the unit here and why it is useful to put this in debugfs (together
>> with drm_dbg)? (Considering the value restarts on each
>> RC6 entry.)
> I will remove the drm_dbg.
> This can be useful to know about rc6 exit from debugfs.
> 
> Actual frequency this counter is ticking is not really known from spec.
> I am still trying to figuring out that. Currently these are just raw count from reg.

... IMO too many unknowns to justify adding this. Yes it's only debugfs 
but (rapid) polling on a debugfs register to notice some internal state 
transitions feels over the top. Would intel_reg read perhaps be enough 
to whatever use case you have in mind?

Regards,

Tvrtko