Re: [Intel-gfx] [PATCH v3 2/2] drm/i915/gt: make a gt sysfs group and move power management files

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

Hi Joonas,

> > > The GT has its own properties and in sysfs they should be grouped
> > > in the 'gt/' directory.
> > > 
> > > Create a 'gt/' directory in sysfs which will contain gt0...gtN
> > > directories related to each tile configured in the GPU. Move the
> > > power management files inside those directories.
> > > 
> > > The previous power management files are kept in their original
> > > root directory to avoid breaking the ABI. They point to the tile
> > > '0' and a warning message is printed whenever accessed to.
> 
> This is wrong. They should act as multiplexers to all the tiles.
> 
> Needs to be fixed before merging.

I have a patch for this and I planned to send it later. I have
even been asked to split this one in more chunks as the review is
a bit difficult.

> > > The
> > > deprecated interface needs for the CONFIG_SYSFS_DEPRECATED_V2
> > > flag in order to be generated.
> > 
> > CONFIG_SYSFS_DEPRECATED_V2 idea was abandoned, no? This patch at least 
> > does not appear to contain it so please update the commit message to 
> > reflect current state.
> > 
> > Adding Joonas to help address the question of how strict are userspace 
> > requirements for sysfs additions. Personally sysadmin use sounds fine to 
> > me, although it needs to be mentioned/documented as Matt requested, but 
> > I fear it may not be enough to upstream. Is Level0 at the stage where we 
> > can upstream for it I am also not sure.
> 
> Sysadmin usage is fine for the simple interfaces that can truly be used
> from the command line. This patch seems to just expose the existing
> interface in per-tile manner, so should not be a concern.

This will definitely help this patch (series) to get in, but I
my understanding is that Level0 is a bit behind for upstreaming.

> However, the controls not under gt directories, need to be converted to
> apply to all tiles. (I've definitely given that feedback multiple
> times). Otherwise it will be very unexpected to the end user when what
> previously applied to whole device would only apply to part of it.

It's not forgotten :)

> Regards, Joonas

Thank you,
Andi