From: Hans de Goede <hdegoede@redhat.com>
To: "Limonciello, Mario" <mario.limonciello@amd.com>,
Shyam Sundar S K <Shyam-sundar.S-k@amd.com>,
markgross@kernel.org
Cc: platform-driver-x86@vger.kernel.org, Patil.Reddy@amd.com,
Mark Pearson <markpearson@lenovo.com>
Subject: Re: [PATCH v1 13/15] platform/x86/amd/pmf: Handle AMT and CQL events for Auto mode
Date: Thu, 28 Jul 2022 19:46:40 +0200 [thread overview]
Message-ID: <81fae6a4-4859-bc15-4edc-2c4df953cc12@redhat.com> (raw)
In-Reply-To: <f351a921-3b7f-d8c9-10e5-816f83f22775@amd.com>
Hi,
On 7/28/22 16:38, Limonciello, Mario wrote:
>
>>>> 1. If I understand things right, then on ThinkPads /sys/firmware/apci/platform_profile
>>>> will be registered by thinkpad_acpi. But in version 1 of this patchset nothing is
>>>> stopping the amd-pmf code from registering /sys/firmware/apci/platform_profile if
>>>> the amd-pmf module gets loaded first. So if the intend is for it to always be owned
>>>> by thinkpad_acpi then the amd-pmf code must check for this and not even try to
>>>> register its platform_profile support. We cannot rely on module ordering ensuring
>>>> that thinkpad_acpi registers first and then amd-pmf will get an -EBUSY error,
>>>> since there are no module load ordering guarantees.
>>>
>>> This was my thought initially too while this was being developed, but actually there is some nuance here that is non-obvious. The platform profile registering code in amd-pmf will examine bits set in the BIOS to decide whether or not to export platform profile support. In Lenovo platforms that support thinkpad_acpi these bits are not set. So platform profile support ONLY comes from thinkpad-acpi in those platforms.
>>
>> Right, Shyam mentioned this in another part of the thread. As I
>> mentioned there IHMO it would still be good to check this in the driver
>> though. To catch cases where a BIOS for some reasons advertises an
>> unexpected combination of features.
>>
>>>> 2. So when the thinkpad_acpi platform_profile is set to balanced, then it will
>>>> enable AMT and then the periodically run workqueue function from amd-pmf
>>>> will do its AMT thing. But what when the thinkpad_acpi platform_profile is
>>>> set to low-power or performance. Should the amd-pmf code then apply the static
>>>> slider settings for low-power/performance which it has read from the ACPI
>>>> tables? Or will the ACPI/EC code on thinkpads take care of this themselves ?
>>>>
>>>
>>> When thinkpad_acpi changes platform profile then a BIOS event goes through and amd-pmf receives that and will run based on the event.
>>
>> Hmm, I don't remember seeing anything for this in the patches. Actually this
>> reminds me that the code should probably reschedule (using mod_delayed_work)
>> the work to run immediately after a BIOS event, rather then waiting for
>> the next normally scheduled run.
>>
>> But even then I don't remember seeing any code related to catching
>> platform-profile changes done outside amd-pmf... ?
>
> It's not a platform profile change - it's an ACPI event.
>
> When a user changes a platform profile then thinkpad_acpi will see whether it's balanced or not. When changing to/from balanced thinkpad_acpi sends an AMT event. amd-pmf reacts to said AMT event.
>
> This is the code you're looking for (in this specific patch):
>
> +static void apmf_event_handler(acpi_handle handle, u32 event, void *data)
> +{
> + struct amd_pmf_dev *pmf_dev = data;
> + struct apmf_if *apmf_if = pmf_dev->apmf_if;
> + int ret;
> +
> + if (apmf_if->func.sbios_requests) {
> + struct apmf_sbios_req req;
> +
> + ret = apmf_get_sbios_requests(apmf_if, &req);
> + if (ret) {
> + dev_err(pmf_dev->dev, "Failed to get SBIOS requests:%d\n", ret);
> + return;
> + }
> + if (req.pending_req & BIT(APMF_AMT_NOTIFICATION)) {
> + pr_debug("PMF: AMT is supported and notifications %s\n",
> + req.amt_event ? "Enabled" : "Disabled");
> + if (req.amt_event)
> + pmf_dev->is_amt_event = true;
> + else
> + pmf_dev->is_amt_event = !!req.amt_event;
> + }
> +
> + if (req.pending_req & BIT(APMF_CQL_NOTIFICATION)) {
> + pr_debug("PMF: CQL is supported and notifications %s\n",
> + req.cql_event ? "Enabled" : "Disabled");
> + if (req.cql_event)
> + pmf_dev->is_cql_event = true;
> + else
> + pmf_dev->is_cql_event = !!req.cql_event;
> +
> + /* update the target mode information */
> + amd_pmf_update_2_cql(pmf_dev);
> + }
> + }
> +}
> +
Right this is the AMT on/off path that bit I understand.
This happens when switching to / away from balanced mode.
My question is what does the equivalent of these lines:
+ amd_pmf_send_cmd(dev, SET_SPL, false, config_store.prop[src][idx].spl, NULL);
+ amd_pmf_send_cmd(dev, SET_FPPT, false, config_store.prop[src][idx].fppt, NULL);
+ amd_pmf_send_cmd(dev, SET_SPPT, false, config_store.prop[src][idx].sppt, NULL);
+ amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false,
+ config_store.prop[src][idx].sppt_apu_only, NULL);
+ amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false,
+ config_store.prop[src][idx].stt_min, NULL);
+ amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
+ config_store.prop[src][idx].stt_skin_temp[STT_TEMP_APU], NULL);
+ amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
+ config_store.prop[src][idx].stt_skin_temp[STT_TEMP_HS2], NULL);
When the profile is switched (by userspace, or through the hotkeys on
the laptop) to low-power or to performance mode ?
Regards,
Hans
next prev parent reply other threads:[~2022-07-28 17:46 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-12 14:58 [PATCH v1 00/15] platform/x86/amd/pmf: Introduce AMD PMF Driver Shyam Sundar S K
2022-07-12 14:58 ` [PATCH v1 01/15] ACPI: platform_profile: Add support for notification chains Shyam Sundar S K
2022-07-12 15:03 ` Limonciello, Mario
2022-07-27 13:24 ` Hans de Goede
2022-07-27 20:38 ` Hans de Goede
2022-07-12 14:58 ` [PATCH v1 02/15] platform/x86/amd/pmf: Add support for PMF core layer Shyam Sundar S K
2022-07-27 13:57 ` Hans de Goede
2022-07-12 14:58 ` [PATCH v1 03/15] platform/x86/amd/pmf: Add support for PMF APCI layer Shyam Sundar S K
2022-07-27 13:57 ` Hans de Goede
2022-07-12 14:58 ` [PATCH v1 04/15] platform/x86/amd/pmf: Add support SPS PMF feature Shyam Sundar S K
2022-07-27 19:29 ` Hans de Goede
2022-07-27 20:26 ` Hans de Goede
2022-07-12 14:58 ` [PATCH v1 05/15] platform/x86/amd/pmf: Add debugfs information Shyam Sundar S K
2022-07-27 19:50 ` Hans de Goede
2022-07-12 14:58 ` [PATCH v1 06/15] platform/x86/amd/pmf: Add heartbeat signal support Shyam Sundar S K
2022-07-27 19:53 ` Hans de Goede
2022-07-12 14:58 ` [PATCH v1 07/15] platform/x86/amd/pmf: Add fan control support Shyam Sundar S K
2022-07-27 20:11 ` Hans de Goede
2022-07-12 14:58 ` [PATCH v1 08/15] platform/x86/amd/pmf: Get performance metrics from PMFW Shyam Sundar S K
2022-07-27 20:36 ` Hans de Goede
2022-07-12 14:58 ` [PATCH v1 09/15] platform/x86/amd/pmf: Add support for CnQF Shyam Sundar S K
2022-07-27 20:51 ` Hans de Goede
2022-07-27 21:00 ` Hans de Goede
2022-07-12 14:58 ` [PATCH v1 10/15] platform/x86/amd/pmf: Add sysfs to toggle CnQF Shyam Sundar S K
2022-07-27 20:52 ` Hans de Goede
2022-07-27 21:12 ` Hans de Goede
2022-07-12 14:58 ` [PATCH v1 11/15] Documentation/ABI/testing/sysfs-amd-pmf: Add ABI doc for AMD PMF Shyam Sundar S K
2022-07-27 20:52 ` Hans de Goede
2022-07-12 14:58 ` [PATCH v1 12/15] platform/x86/amd/pmf: Add support for Auto mode feature Shyam Sundar S K
2022-07-27 21:22 ` Hans de Goede
2022-07-28 12:57 ` Shyam Sundar S K
2022-07-28 13:15 ` Hans de Goede
2022-07-12 14:58 ` [PATCH v1 13/15] platform/x86/amd/pmf: Handle AMT and CQL events for Auto mode Shyam Sundar S K
2022-07-27 21:33 ` Hans de Goede
2022-07-27 21:44 ` Hans de Goede
2022-07-27 21:46 ` Hans de Goede
2022-07-27 23:52 ` Limonciello, Mario
2022-07-28 13:03 ` Hans de Goede
2022-07-28 13:43 ` Limonciello, Mario
2022-07-28 14:09 ` Hans de Goede
2022-07-28 14:38 ` Limonciello, Mario
2022-07-28 17:46 ` Hans de Goede [this message]
2022-07-28 18:06 ` Limonciello, Mario
2022-07-28 18:17 ` Hans de Goede
2022-07-28 21:01 ` Limonciello, Mario
2022-07-29 11:03 ` Hans de Goede
2022-07-29 15:43 ` Limonciello, Mario
2022-07-29 17:40 ` Shyam Sundar S K
2022-07-29 17:59 ` Hans de Goede
2022-08-01 10:29 ` Shyam Sundar S K
2022-08-01 11:08 ` Hans de Goede
2022-07-12 14:58 ` [PATCH v1 14/15] platform/x86/amd/pmf: Force load driver on older supported platforms Shyam Sundar S K
2022-07-27 21:40 ` Hans de Goede
2022-07-12 14:58 ` [PATCH v1 15/15] MAINTAINERS: Add AMD PMF driver entry Shyam Sundar S K
2022-07-27 21:41 ` Hans de Goede
2022-07-28 17:44 ` Shyam Sundar S K
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=81fae6a4-4859-bc15-4edc-2c4df953cc12@redhat.com \
--to=hdegoede@redhat.com \
--cc=Patil.Reddy@amd.com \
--cc=Shyam-sundar.S-k@amd.com \
--cc=mario.limonciello@amd.com \
--cc=markgross@kernel.org \
--cc=markpearson@lenovo.com \
--cc=platform-driver-x86@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).