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

[Andi Shyti <andi.shyti@linux.intel.com>]

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?

At this point I guess the warning should be removed from
everywhere (i.e.  only those RC6 and RPS interfaces that are
duplicated/multiplexed).

We shouldn't be supposed to need more usage of multiplexed
interfaces in the future (maybe just rc6 enable, but I don't see
it really necessary).

Thanks,
Andi