Re: [Intel-gfx] [PATCH v5 5/7] drm/i915/gt: Create per-tile RC6 sysfs interface

Tvrtko Ursulin
2022-02-18T09:13:02Z

On 17/02/2022 15:53, Andi Shyti wrote:
> Hi Tvrtko,
>
>>> Now tiles have their own sysfs interfaces under the gt/
>>> directory. Because RC6 is a property that can be configured on a
>>> tile basis, then each tile should have its own interface
>>>
>>> The new sysfs structure will have a similar layout for the 4 tile
>>> case:
>>>
>>> /sys/.../card0
>>> ├── gt
>>> │   ├── gt0
>>> │   │   ├── id
>>> │   │   ├── rc6_enable
>>> │   │   ├── rc6_residency_ms
>>> . . .
>>> . . .
>>> . .
>>> │   └── gtN
>>> │   ├── id
>>> │   ├── rc6_enable
>>> │   ├── rc6_residency_ms
>>> │ .
>>> │ .
>>> │
>>> └── power/ -+
>>> ├── rc6_enable | Original interface
>>> ├── rc6_residency_ms +-> kept as existing ABI;
>>> . | it multiplexes over
>>> . | the GTs
>>> -+
>>>
>>> The existing interfaces have been kept in their original location
>>> to preserve the existing ABI. They act on all the GTs: when
>>> reading they provide the average value from all the GTs.
>>
>> Average feels very odd to me. I'd ask if we can get away providing an errno
>> instead? Or tile zero data?
>
> Real multiplexing would be providing something when reading and
> when writing. The idea of average came while revieweing with
> Chris the write multiplexing. Indeed it makes sense to provide
> some common value, but I don't know how useful it can be to the
> user (still if the user needs any average).
>
> Joonas, Chris... any idea?
>
>> Case in point, and please correct me if I am wrong, legacy rc6_enable
>> returns tile zero, while residency returns average.
>
> As the interface is done now, the rc6_enable is just returning
> whether the gpu (i.e. i915, not gt) supports RC6 or not. I think
> there is a patch later.
>
>> Even the deprecated message gets logged with every access right?
>>
>> Btw is the deperecated message limited to multi-tile platforms (can't see
>> that it is) and what is the plan for that?
>
> yes, at this point the message would need to be removed and I
> forgot to do it.

Maybe it is correct to have it, I don't know at this point. Is the plan
to remove the warning everywhere, or only have it on multi-tile
platforms, or new platforms? And/or remove legacy files after a while on
all platforms, or just new ones?

Regards,

Tvrtko