On Wed, 2022-05-11 at 20:14 +0200, Rafael J. Wysocki wrote:
> On Tue, May 10, 2022 at 8:22 PM Srinivas Pandruvada
> <srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote:
> >
> > With the new OS handshake introduced with the commit: "c7ff29763989
> > ("thermal: int340x: Update OS policy capability handshake")",
> > thermal zone mode "enabled" doesn't work in the same way as the
> > legacy
> > handshake. The mode "enabled" fails with -EINVAL using new
> > handshake.
> >
> > To address this issue, when the new OS UUID mask is set:
> > - When mode is "enabled", return 0 as the firmware already has the
> > latest policy mask.
> > - When mode is "disabled", update the firmware with UUID mask of
> > zero.
> > In this way firmware can take control of the thermal control. Also
> > reset the OS UUID mask. This allows user space to update with new
> > set of policies.
> >
> > Fixes: c7ff29763989 ("thermal: int340x: Update OS policy capability
> > handshake")
> > Signed-off-by: Srinivas Pandruvada
> > <srinivas.pandruvada@xxxxxxxxxxxxxxx>
> > Cc: stable@xxxxxxxxxxxxxxx
>
> This is not -stable material yet.
I thought it will wait for 5.19 merge window.
>
> > ---
> > update:
> > Added Fixes tag
> >
> >
[...]
> > + if (priv->os_uuid_mask) {
> > + if (!enabled) {
> > + priv->os_uuid_mask = 0;
> > + result = set_os_uuid_mask(priv,
> > priv->os_uuid_mask);
>
> This change worries me a bit, because it means replaying an already
> established _OSC handshake which shouldn't be done by the spec.
>
I checked with the firmware team. The _OSC changes dynamically is
validated and is recommended when enable/disable user space thermal
control.
Looking at ACPI Spec
"OSPM may evaluate _OSC multiple times to indicate changes in OSPM
capability to the device but this may be precluded by specific device
requirements"
> But I suppose you have tested this?
I tested on TigerLake system.
>
> > + }
> > +
[...]
>
> Patch applied as 5.18-rc material, but I've removed some unneeded
> parens from the new code, so please double check the result in
> bleeding-edge.
I tested the patch from bleeding edge.
Works fine.
Thanks,
Srinivas
>
> Thanks!
