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

[Joonas Lahtinen]

Quoting Andi Shyti (2022-02-17 17:53:58)
> 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
> > >           \u251c\u2500\u2500 gt
> > >           \u2502   \u251c\u2500\u2500 gt0
> > >           \u2502   \u2502   \u251c\u2500\u2500 id
> > >           \u2502   \u2502   \u251c\u2500\u2500 rc6_enable
> > >           \u2502   \u2502   \u251c\u2500\u2500 rc6_residency_ms
> > >           .   .   .
> > >           .   .   .
> > >           .   .
> > >           \u2502   \u2514\u2500\u2500 gtN
> > >           \u2502       \u251c\u2500\u2500 id
> > >           \u2502       \u251c\u2500\u2500 rc6_enable
> > >           \u2502       \u251c\u2500\u2500 rc6_residency_ms
> > >           \u2502       .
> > >           \u2502       .
> > >           \u2502
> > >           \u2514\u2500\u2500 power/                -+
> > >                \u251c\u2500\u2500 rc6_enable        |    Original interface
> > >                \u251c\u2500\u2500 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?

Tile zero data is always wrong, in my opinion. If we have round-robin
scaling workloads like some media cases, part of the system load might
just disappear when it goes to tile 1.

> 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).

I think all read/write controls like min/max/boost_freq should return
an error from the global interface if all the tiles don't return same
value. Write will always overwrite per-tile values.

When we have frequency readbacks without control, returning MAX() across
tiles would be the logical thing. The fact that parts of the hardware can
be clocked lower when one part is fully utilized is the "new feature".

After that we're only really left with the rc6_residency_ms. And that is
the tough one. I'm inclined that MIN() across tiles would be the right
answer. If you are fully utilizing a single tile, you should be able to
see it.

This all would be what feels natural for an user who has their setup
tuned for single-tile device. And would allow simple round-robing
balancing across the tiles in somewhat coherent manner.

Regards, Joonas

> 
> 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.
> 
> > It's a tough problem, no easy answers even after all this time. :D
> 
> yeah! quite hard to get it conceptually right!
> 
> Thanks Tvrtko,
> Andi