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

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

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