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

Andi Shyti
2022-02-22T08:57:14Z

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