Hi Mario
On 6/3/22 13:14, Limonciello, Mario wrote:
> [AMD Official Use Only - General]
>
>
>
>> -----Original Message-----
>> From: Mark Pearson <markpearson@xxxxxxxxxx>
>> Sent: Friday, June 3, 2022 12:02
>> To: markpearson@xxxxxxxxxx
>> Cc: hdegoede@xxxxxxxxxx; markgross@xxxxxxxxxx; platform-driver-
>> x86@xxxxxxxxxxxxxxx; Limonciello, Mario <Mario.Limonciello@xxxxxxx>
>> Subject: [PATCH 3/4] platform/x86: thinkpad-acpi: Add support for hotkey
>> 0x131a
>>
>> On some AMD platforms if you press FN+T it will toggle whether automatic
>> mode transitions are active.
>>
>> Recognize this keycode and use it to toggle AMT.
>>
>> Co-developed-by: Mario Limonciello <mario.limonciello@xxxxxxx>
>> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
>> Signed-off-by: Mark Pearson <markpearson@xxxxxxxxxx>
>> ---
>> drivers/platform/x86/thinkpad_acpi.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/platform/x86/thinkpad_acpi.c
>> b/drivers/platform/x86/thinkpad_acpi.c
>> index 2df290cee0a1..f11866225ef3 100644
>> --- a/drivers/platform/x86/thinkpad_acpi.c
>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>> @@ -159,6 +159,7 @@ enum tpacpi_hkey_event_t {
>> TP_HKEY_EV_VOL_DOWN = 0x1016, /* Volume down or
>> unmute */
>> TP_HKEY_EV_VOL_MUTE = 0x1017, /* Mixer output
>> mute */
>> TP_HKEY_EV_PRIVACYGUARD_TOGGLE = 0x130f, /* Toggle priv.guard
>> on/off */
>> + TP_HKEY_EV_AMT_TOGGLE = 0x131a, /* Toggle AMT
>> on/off */
>>
>> /* Reasons for waking up from S3/S4 */
>> TP_HKEY_EV_WKUP_S3_UNDOCK = 0x2304, /* undock
>> requested, S3 */
>> @@ -3735,6 +3736,7 @@ static bool hotkey_notify_extended_hotkey(const
>> u32 hkey)
>>
>> switch (hkey) {
>> case TP_HKEY_EV_PRIVACYGUARD_TOGGLE:
>> + case TP_HKEY_EV_AMT_TOGGLE:
>> tpacpi_driver_event(hkey);
>> return true;
>> }
>> @@ -11038,6 +11040,15 @@ static void tpacpi_driver_event(const unsigned int
>> hkey_event)
>> if (changed)
>>
>> drm_privacy_screen_call_notifier_chain(lcdshadow_dev);
>> }
>> + if (hkey_event == TP_HKEY_EV_AMT_TOGGLE) {
>> + /* If we're enabling AMT we need to force balanced mode */
>> + if (!dytc_amt_active)
>> + /* This will also set AMT mode enabled */
>> + dytc_profile_set(NULL,
>> PLATFORM_PROFILE_BALANCED);
>> + else
>> + dytc_control_amt(!dytc_amt_active);
>
> I missed this while we were making the series, but a fresh set of eyes tells me
> shouldn't dytc_control_amt(..) run in either case - not just in the "else" case?
>
> * If AMT is not active and you press the key (to activate it) you
> should switch to balanced mode "and" turn it on.
> * If AMT is active and you press the key (to deactivate it) you're already in
> balanced mode, so you should just turn it off.
The dytc_profile_set for balanced mode will also enable AMT (if it's
supported) so I didn't want to call it twice.
Mark
