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

[Andi Shyti]

Hi Tvrtko and Joonas,

> > > > > 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.
> 
> I was thinking that in conjunction with deprecated log message it wouldn't
> be wrong - I mean if the route take was to eventually retire the legacy
> files altogether.

that's a good point... do we want to treat the legacy interfaces
as an error or do we want to make them a feature? As the
discussion is turning those interfaces are becoming a feature.
But what are we going to do with the coming interfaces?

E.g. in the future we will have the rc6_enable/disable that can
be a command, so that we will add the "_store" interface per
tile. What are we going to do with the above interfaces? Are we
going to add a multiplexed command as well?

> > 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.
>  So we have MIN, AVG or SUM, or errno, or remove the file (which is just a
> different kind of errno?) to choose from. :)

in this case it would just be MIN and MAX. At the end we have
here only two types of interface: frequencies and residency_ms.
For the first type we would use 'max', for the second 'min'.

But the question holds in case we want keep adding interfaces to
the above directories.

Andi