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

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

Hi Matt,

> > 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. The
> > deprecated interface needs for the CONFIG_SYSFS_DEPRECATED_V2
> > flag in order to be generated.
> > 
> > The new sysfs structure will have a similar layout for the 4 tile
> > case:
> > 
> > /sys/.../card0
> >          ├── gt
> >          │   ├── gt0
> >          │   │   ├── id
> >          │   │   ├── rc6_enable
> >          │   │   ├── rc6_residency_ms
> >          │   │   ├── rps_act_freq_mhz
> >          │   │   ├── rps_boost_freq_mhz
> >          │   │   ├── rps_cur_freq_mhz
> >          │   │   ├── rps_max_freq_mhz
> >          │   │   ├── rps_min_freq_mhz
> >          │   │   ├── rps_RP0_freq_mhz
> >          │   │   ├── rps_RP1_freq_mhz
> >          │   │   └── rps_RPn_freq_mhz
> > 	 .   .
> > 	 .   .
> > 	 .   .
> >          │   └── gt3
> >          │       ├── id
> >          │       ├── rc6_enable
> >          │       ├── rc6_residency_ms
> >          │       ├── rps_act_freq_mhz
> >          │       ├── rps_boost_freq_mhz
> >          │       ├── rps_cur_freq_mhz
> >          │       ├── rps_max_freq_mhz
> >          │       ├── rps_min_freq_mhz
> >          │       ├── rps_RP0_freq_mhz
> >          │       ├── rps_RP1_freq_mhz
> >          │       └── rps_RPn_freq_mhz
> >          ├── gt_act_freq_mhz   -+
> >          ├── gt_boost_freq_mhz  |
> >          ├── gt_cur_freq_mhz    |    Original interface
> >          ├── gt_max_freq_mhz    +─-> kept as existing ABI;
> >          ├── gt_min_freq_mhz    |    it points to gt0/
> >          ├── gt_RP0_freq_mhz    |
> >          └── gt_RP1_freq_mhz    |
> >          └── gt_RPn_freq_mhz   -+
> > 
> > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Even though this is a pretty natural extension of sysfs entries we
> already have in the driver today, it's still technically "new ABI" so I
> think we need a link to the IGT changes that exercise it to make sure
> it's working as expected.

this patch should unblock the level0 sysman for DG1 and DG2,
currently being upstreamed, and this will be handled by Sujaritha
in the next days.

Besides there a patch that is adding an hwmon interface to i915
that depends on this one.

> Likewise we need a link to the real userspace consumer that is expecting
> to use this (does some of the Level0 stuff use these knobs?).  If this
> is one of those really rare cases where we never expect any specific
> userspace software to use the ABI, but the intent is instead to let
> sysadmins turn the knobs manually with custom shell scripts or whatever,
> that would need to be justified in the commit message too.

I think with the first multitile platform coming (next series)
this will be essential to control the power of multitile devices,
either manually or with tools, otherwise we will be able only to
access tile 0.

I can add a note in the commit message.

Andi