[Intel-gfx] [PATCH] drm/i915/pmu: Check actual RC6 status

[Tvrtko Ursulin]

On 01/04/2021 11:24, Tamminen, Eero T wrote:
> Hi,
> 
> On Thu, 2021-04-01 at 05:54 -0400, Rodrigo Vivi wrote:
>> On Thu, Apr 01, 2021 at 10:38:11AM +0100, Tvrtko Ursulin wrote:
> ...
>>> I think it is possible to argue both ways.
>>>
>>> 1)
>>> HAS_RC6 means hardware has RC6 so if we view PMU as very low level
>>> we can
>>> say always export it.
>>>
>>> If i915 had to turn it off (rc6->supported == false) due firmware or
>>> GVT-g,
>>> then we could say reporting zero RC6 is accurate in that sense. Only
>>> the
>>> reason "why it is zero" is missing for PMU users.
>>>
>>> 2)
>>> Or if we go with this patch we could say that presence of the PMU
>>> metric
>>> means RC6 is active and enabled, while absence means it is either
>>> not
>>> supported due platform (or firmware) or how the platform is getting
>>> used
>>> (GVT-g).
>>>
>>
>> yeap, these 2 cases described well my mental conflict...
>>
>>> So I think patch is a bit better. I don't see it is adding more
>>> confusion.
>>
>> As I said on the other patch I have no strong position on which is
>> better,
>> but if you and Eero feel that this works better for the current case,
>> let's do it...
> 
> IMHO seeing case 1) i.e. zero RC6 could be slightly better from user
> point of view than not seeing RC6 at all, because:
> 
> A) user then knows that GPU is not entering RC6, and
> 
> B) then the question is why it's not going to RC6 => one can see from
> sysfs that it has been disabled
> 
> 
> Whereas in case 2), the question is why there's no RC6 info, and user
> doesn't know whether GPU is suspended or not (i.e. why GPU power
> consumption is higher than expected).  It would help if i-g-t could show
> e.g. "RC6 OFF" in that case.

So many options.. :)

It can be handle on the "presentation" layer (intel_gpu_top). If we go 
with this patch but different errnos it could indeed distinguish and 
either not show RC6 or say "RC6 OFF".

If we go with the other patch 
(https://patchwork.freedesktop.org/patch/426589/?series=88580&rev=1) 
then intel_gpu_top could really still do the same by looking at 
/sys/class/drm/card0/power/rc6_enable.

So strictly no i915 patch is even needed to provide clarity in 
intel_gpu_top.

But still one of those two i915 patches is required to improve how 
low-level Perf/PMU RC6 counter gets exposed (or not exposed). I don't 
have a strong preference which one to take either. :)

Regards,

Tvrtko