[Intel-gfx] [PATCH 3/4] drm/i915/hwmon: Add HWMON power sensor support

[Guenter Roeck]

On Mon, Jun 20, 2022 at 11:41:41PM -0700, Dixit, Ashutosh wrote:
> On Mon, 20 Jun 2022 13:58:49 -0700, Guenter Roeck wrote:
> >
> 
> Hi Guenter, Thanks for taking a look.
> 
> > > diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> > > index 24c4b7477d51..945f472dd4a2 100644
> > > --- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> > > +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> > > @@ -5,3 +5,23 @@ Contact:	dri-devel at lists.freedesktop.org
> > >   Description:	RO. Current Voltage in millivolt.
> > >			Only supported for particular Intel i915 graphics
> > > platforms.
> > > +
> > > +What:		/sys/devices/.../hwmon/hwmon<i>/power1_max
> > > +Date:		June 2022
> > > +KernelVersion:	5.19
> > > +Contact:	dri-devel at lists.freedesktop.org
> > > +Description:	RW. Card reactive sustained  (PL1/Tau) power limit in microwatts.
> > > +
> > > +		The power controller will throttle the operating frequency
> > > +		if the power averaged over a window (typically seconds)
> > > +		exceeds this limit.
> > > +
> > > +		Only supported for particular Intel i915 graphics platforms.
> > > +
> > > +What:		/sys/devices/.../hwmon/hwmon<i>/power1_max_default
> >
> > I don't immediately see the reason for not using the standard power1_cap
> > attribute, which is described as
> >
> >		If power use rises above this limit, the
> >                 system should take action to reduce power use.
> >
> > and pretty much matches the description above.
> 
> Sorry I believe you are referring to the description above which is for the
> standard power1_max attribute (as we have used it). The non-standard
> attribute is power1_max_default the description for which is below ("Card
> default power limit (default TDP setting)").
> 

If you use power1_max to limit power consumption if exceeded, power1_cap
might have been more appropriate.

> > > +Date:		June 2022
> > > +KernelVersion:	5.19
> > > +Contact:	dri-devel at lists.freedesktop.org
> > > +Description:	RO. Card default power limit (default TDP setting).
> 
> Actually we do not want to use custom hwmon attributes as far as
> possible and are looking for some guidance on which standard attributes
> which we should use instead.
> 
You could possibly have used power1_rated_max instead of power1_max_default.

> These are the power attributes we are interested in: the two above and
> another one which will come in a future patch:
> 
> 1. PL1 (RW)
> 
>    https://www.hardwaretimes.com/intel-10th-gen-cpu-power-consumption-explained-pl1-pl2-and-tau/
> 
> 2. TDP (RO)
> 
>    https://en.wikipedia.org/wiki/Thermal_design_power
> 
Not sure I understand the difference between 'default TDP' (RO),
'TDP' (RO), and PL1.

> 3. Tau (RW)
> 
>    https://www.hardwaretimes.com/intel-10th-gen-cpu-power-consumption-explained-pl1-pl2-and-tau/
> 
> Would you be able to suggest if there are standard hwmon attributes which
> we would be able to use for these three? We also want to use the read/write
> permissions as shown above.
> 

There are a number of standard power attributes available to set or report
limits (cap, cap_max, cap_min, max, crit, rated_min, rated_max). I would
suggest to pick from that list whatever you think fits best. I don't have
a recommendation for Tau.

Either case, when reporting power, make sure you don't hit any of the
security issues which caused power reporting to be deleted for other CPUs.
Restricting read access to hwmon attributes for non-privileged users is not
acceptable.

Thanks,
Guenter