On 7/29/2022 06:03, Hans de Goede wrote:
So as for the AMT mode, since that is Lenovo only, I guess that means
that there is no need to do call amd_pmf_update_slider() when AMT
is being disabled since at this point the firmware will have
already set the values.
Yeah, Shyam made this modification for v2 to make sure that code path isn't called unless static slider was set in the BIOS.
But this code path is only hit when AMT / auto mode is available and
when that is true then the static slider should never be set in the BIOS
so the whole amd_pmf_update_slider() call on AMT disable can simply
be dropped AFAICT.
The reason to leave it in place but guarded like this is for validation
of the feature behaves properly from AMD internal systems AMD test BIOS.
It can be used to prove out something works properly without needing
to include extra drivers and software.
Actually this seems to mean that we must ensure that the AMD-PMF
code stops touching these settings as soon as the event is received.
Which would imply killing the periodic work when an AMT off event
is received from within the event handling and then restating it
when AMT is on (and making sure the work being queued or not state
matches the AMT on/off state at driver probe time) ?
At first glance this seems plausible, but actually I think it should stay as is because CQL thermals can be set at any time (that's like a lap mode sensor event from thinkpad_acpi). Even when AMT is turned off, you may want the CQL thermal profile set accordingly.
So the CQL code is to handle lapmode when AMT is active. But I would
expect the firmware to update the power-limits, etc. for lapmode itself
when in performance mode. >
The amd_pmf_update_2_cql() function only does things when
config_store.current_mode == AUTO_PERFORMANCE (or AUTO_PERFORMANCE_ON_LAP)
And that reflects the last mode selected by the auto/AMT mode code, not
the mode actual set by thinkpad_acpi so if the last auto selected mode
was balanced and then AMT gets disabled because thinkpad_acpi switches
to performance mode, then on CQL events after the switch amd_pmf_update_2_cql()
will not do anything.
To me it seems that when AMT is off the AMD-PMF code should not touch
the power-limits, etc. at all and thus it should also always ignore
CQL events when AMT is off.
This assumes that the firmware takes care of udating the limits for
on lap / off lap when thinkpad_acpi's profile is set to performance.
Where does this assumption come from? I guess that's how it's done on
Lenovo's Intel systems?
AMT and CQL is a new feature on Lenovo AMD systems, this is the way that
it's supposed to be done here.
If thinkpad_acpi does not do this then the AMD-PMF code should
check what mode has been selected by the thinkpad_acpi code in
amd_pmf_update_2_cql() when AMT is off.
It is up to the firmware (and thinkpad_acpi) to decide when to send
the CQL events.
Shyam - any comments here?
