Re: [Intel-gfx] [PATCH 1/2] drm/i915/pmu: Turn off the timer to sample frequencies when GT is parked

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 24/05/2023 22:46, Dixit, Ashutosh wrote:
> On Wed, 24 May 2023 02:12:31 -0700, Andrzej Hajda wrote:
>>
> 
> Hi Andrzej,
> 
>> On 23.05.2023 17:19, Ashutosh Dixit wrote:
>>> pmu_needs_timer() keeps the timer running even when GT is parked,
>>> ostensibly to sample requested/actual frequencies. However
>>> frequency_sample() has the following:
>>>
>>> 	/* Report 0/0 (actual/requested) frequency while parked. */
>>> 	if (!intel_gt_pm_get_if_awake(gt))
>>> 		return;
>>>
>>> The above code prevents frequencies to be sampled while the GT is
>>> parked. So we might as well turn off the sampling timer itself in this
>>> case and save CPU cycles/power.
>>>
>>> v2: Instead of turning freq bits off, return false, since no counters will
>>>       run after this change when GT is parked (Tvrtko)
>>>
>>> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
>>> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/i915_pmu.c | 12 +++++-------
>>>    1 file changed, 5 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
>>> index a814583e19fd7..b47d890d4ada1 100644
>>> --- a/drivers/gpu/drm/i915/i915_pmu.c
>>> +++ b/drivers/gpu/drm/i915/i915_pmu.c
>>> @@ -144,6 +144,10 @@ static bool pmu_needs_timer(struct i915_pmu *pmu, bool gpu_active)
>>> 	struct drm_i915_private *i915 = container_of(pmu, typeof(*i915), pmu);
>>> 	u32 enable;
>>>    +	/* When GPU is idle, at present no counters need to run */
>>> +	if (!gpu_active)
>>> +		return false;
>>> +
>>
>> What is then purpose of calling pmu_needs_timer with 2nd arg false?
>> Why not just replace all occurrences of pmu_needs_timer(.., false) with
>> false? And remove the 2nd argument.
> 
> OK, this didn't seem unreasonable so I went ahead and made this change in
> Patch v3. Copying Tvrtko too in case he prefers v2 for any reason. Please
> review.

It is all fine by me (the latest version and all)!

Regards,

Tvrtko