Re: [Intel-xe] [PATCH v2 0/6] Add HWMON support for DGFX

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Fri, Jul 14, 2023 at 03:26:49PM -0700, Guenter Roeck wrote:
> On 7/14/23 13:21, Rodrigo Vivi wrote:
> [ ... ]
> 
> > Hi Guenter,
> > 
> > First of all sorry for jumping late here. I'm totally with you here and we should
> > definitely only use the new API. For standard entries that will definitely
> > reduce the code size.
> > 
> > So, since we are talking about reducing code here, and looking to other DRM
> > drivers, and thinking about the needs on this new Xe driver, I'm wondering
> > if you would consider accepting 'frequency' as a standard hwmon attribute.
> > 
> > We would need it to be RW so we could use to put freq requests as well,
> > and possibly different types/domains and even throttle reasons on top.
> > 
> > So we could then try to unify all the drm drivers in a common drm-hwmon
> > layer putting an end in all abuses and deprecated users.
> > 
> > But before moving fwd with any proposal I'd like to hear your thoughts on
> > this 'frequency' block as standard attribute.
> > 
> 
> I really don't see how this would fit under "hardware monitoring".
> Making it writable would be even worse - this is most definitely not a limit but
> an actual value. The notion of limit actually shows that it is not a good fit as
> a monitoring attribute: I can not conceive the notion of a "maximum" or "minimum"
> frequency limit, or an "under" or "over" frequency.

how's that different from the voltage/pwm/current/etc min, max, critical RW limits
already existent?

> 
> If this is about thermal control/management, you might want to consider registering
> with devfreq and the thermal subsystem (see devfreq_cooling_register() and
> friends for reference).

yeap, it looks like devfreq is a good candidate for the unification. It is just
sad that it is not as robust and flexible as hwmon infrastructure.

> 
> Thanks,
> Guenter
>