* [PATCH 1/2] platform/x86/amd/pmf: Add new ACPI ID AMDI0107
@ 2024-07-23 13:24 Shyam Sundar S K
2024-07-23 13:24 ` [PATCH 2/2] platform/x86/amd/pmf: Fix to Update HPD Data When ALS is Disabled Shyam Sundar S K
2024-07-30 12:38 ` [PATCH 1/2] platform/x86/amd/pmf: Add new ACPI ID AMDI0107 Ilpo Järvinen
0 siblings, 2 replies; 6+ messages in thread
From: Shyam Sundar S K @ 2024-07-23 13:24 UTC (permalink / raw)
To: hdegoede, ilpo.jarvinen
Cc: platform-driver-x86, Patil.Reddy, Shyam Sundar S K
Add new ACPI ID AMDI0107 used by upcoming AMD platform to the PMF
supported list of devices.
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index 2d6e2558863c..8f1f719befa3 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -41,6 +41,7 @@
#define AMD_CPU_ID_RMB 0x14b5
#define AMD_CPU_ID_PS 0x14e8
#define PCI_DEVICE_ID_AMD_1AH_M20H_ROOT 0x1507
+#define PCI_DEVICE_ID_AMD_1AH_M60H_ROOT 0x1122
#define PMF_MSG_DELAY_MIN_US 50
#define RESPONSE_REGISTER_LOOP_MAX 20000
@@ -249,6 +250,7 @@ static const struct pci_device_id pmf_pci_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_RMB) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_PS) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M20H_ROOT) },
+ { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_ROOT) },
{ }
};
@@ -382,6 +384,7 @@ static const struct acpi_device_id amd_pmf_acpi_ids[] = {
{"AMDI0102", 0},
{"AMDI0103", 0},
{"AMDI0105", 0},
+ {"AMDI0107", 0},
{ }
};
MODULE_DEVICE_TABLE(acpi, amd_pmf_acpi_ids);
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] platform/x86/amd/pmf: Fix to Update HPD Data When ALS is Disabled
2024-07-23 13:24 [PATCH 1/2] platform/x86/amd/pmf: Add new ACPI ID AMDI0107 Shyam Sundar S K
@ 2024-07-23 13:24 ` Shyam Sundar S K
2024-07-29 11:55 ` Ilpo Järvinen
2024-07-30 12:38 ` [PATCH 1/2] platform/x86/amd/pmf: Add new ACPI ID AMDI0107 Ilpo Järvinen
1 sibling, 1 reply; 6+ messages in thread
From: Shyam Sundar S K @ 2024-07-23 13:24 UTC (permalink / raw)
To: hdegoede, ilpo.jarvinen
Cc: platform-driver-x86, Patil.Reddy, Shyam Sundar S K
If the Ambient Light Sensor (ALS) is disabled, the current code in the PMF
driver does not query for Human Presence Detection (HPD) data in
amd_pmf_get_sensor_info(). As a result, stale HPD data is used by PMF-TA
to evaluate policy conditions, leading to unexpected behavior in the policy
output actions.
To resolve this issue, modify the PMF driver to query HPD data
independently of ALS.
With this change, amd_pmf_get_sensor_info() now returns void instead of
int.
Fixes: cedecdba60f4 ("platform/x86/amd/pmf: Get ambient light information from AMD SFH driver")
Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/spc.c | 33 +++++++++++++++---------------
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
index a3dec14c3004..d9e60d63553c 100644
--- a/drivers/platform/x86/amd/pmf/spc.c
+++ b/drivers/platform/x86/amd/pmf/spc.c
@@ -150,7 +150,7 @@ static int amd_pmf_get_slider_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_
return 0;
}
-static int amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
+static void amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
{
struct amd_sfh_info sfh_info;
int ret;
@@ -160,26 +160,25 @@ static int amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_
if (!ret)
in->ev_info.ambient_light = sfh_info.ambient_light;
else
- return ret;
+ dev_dbg(dev->dev, "ALS is not enabled\n");
/* get HPD data */
ret = amd_get_sfh_info(&sfh_info, MT_HPD);
- if (ret)
- return ret;
-
- switch (sfh_info.user_present) {
- case SFH_NOT_DETECTED:
- in->ev_info.user_present = 0xff; /* assume no sensors connected */
- break;
- case SFH_USER_PRESENT:
- in->ev_info.user_present = 1;
- break;
- case SFH_USER_AWAY:
- in->ev_info.user_present = 0;
- break;
+ if (!ret) {
+ switch (sfh_info.user_present) {
+ case SFH_NOT_DETECTED:
+ in->ev_info.user_present = 0xff; /* assume no sensors connected */
+ break;
+ case SFH_USER_PRESENT:
+ in->ev_info.user_present = 1;
+ break;
+ case SFH_USER_AWAY:
+ in->ev_info.user_present = 0;
+ break;
+ }
+ } else {
+ dev_dbg(dev->dev, "HPD is not enabled\n");
}
-
- return 0;
}
void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] platform/x86/amd/pmf: Fix to Update HPD Data When ALS is Disabled
2024-07-23 13:24 ` [PATCH 2/2] platform/x86/amd/pmf: Fix to Update HPD Data When ALS is Disabled Shyam Sundar S K
@ 2024-07-29 11:55 ` Ilpo Järvinen
2024-07-30 14:27 ` Shyam Sundar S K
0 siblings, 1 reply; 6+ messages in thread
From: Ilpo Järvinen @ 2024-07-29 11:55 UTC (permalink / raw)
To: Shyam Sundar S K; +Cc: Hans de Goede, platform-driver-x86, Patil.Reddy
On Tue, 23 Jul 2024, Shyam Sundar S K wrote:
> If the Ambient Light Sensor (ALS) is disabled, the current code in the PMF
> driver does not query for Human Presence Detection (HPD) data in
> amd_pmf_get_sensor_info(). As a result, stale HPD data is used by PMF-TA
> to evaluate policy conditions, leading to unexpected behavior in the policy
> output actions.
>
> To resolve this issue, modify the PMF driver to query HPD data
> independently of ALS.
>
> With this change, amd_pmf_get_sensor_info() now returns void instead of
> int.
>
> Fixes: cedecdba60f4 ("platform/x86/amd/pmf: Get ambient light information from AMD SFH driver")
> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> drivers/platform/x86/amd/pmf/spc.c | 33 +++++++++++++++---------------
> 1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> index a3dec14c3004..d9e60d63553c 100644
> --- a/drivers/platform/x86/amd/pmf/spc.c
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -150,7 +150,7 @@ static int amd_pmf_get_slider_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_
> return 0;
> }
>
> -static int amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
> +static void amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
> {
> struct amd_sfh_info sfh_info;
> int ret;
> @@ -160,26 +160,25 @@ static int amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_
> if (!ret)
> in->ev_info.ambient_light = sfh_info.ambient_light;
> else
> - return ret;
> + dev_dbg(dev->dev, "ALS is not enabled\n");
>
> /* get HPD data */
> ret = amd_get_sfh_info(&sfh_info, MT_HPD);
> - if (ret)
> - return ret;
> -
> - switch (sfh_info.user_present) {
> - case SFH_NOT_DETECTED:
> - in->ev_info.user_present = 0xff; /* assume no sensors connected */
> - break;
> - case SFH_USER_PRESENT:
> - in->ev_info.user_present = 1;
> - break;
> - case SFH_USER_AWAY:
> - in->ev_info.user_present = 0;
> - break;
> + if (!ret) {
> + switch (sfh_info.user_present) {
> + case SFH_NOT_DETECTED:
> + in->ev_info.user_present = 0xff; /* assume no sensors connected */
> + break;
> + case SFH_USER_PRESENT:
> + in->ev_info.user_present = 1;
> + break;
> + case SFH_USER_AWAY:
> + in->ev_info.user_present = 0;
> + break;
> + }
> + } else {
> + dev_dbg(dev->dev, "HPD is not enabled\n");
Is it okay to leave in->ev_info.user_present as 0 this case? Should it be
set to same as with SFH_NOT_DETECTED?
...I now realize your print out doesn't cover that case at all and
user_present is not used for anything else than debug printing.
--
i.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] platform/x86/amd/pmf: Fix to Update HPD Data When ALS is Disabled
2024-07-29 11:55 ` Ilpo Järvinen
@ 2024-07-30 14:27 ` Shyam Sundar S K
0 siblings, 0 replies; 6+ messages in thread
From: Shyam Sundar S K @ 2024-07-30 14:27 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: Hans de Goede, platform-driver-x86, Patil.Reddy
On 7/29/2024 17:25, Ilpo Järvinen wrote:
> On Tue, 23 Jul 2024, Shyam Sundar S K wrote:
>
>> If the Ambient Light Sensor (ALS) is disabled, the current code in the PMF
>> driver does not query for Human Presence Detection (HPD) data in
>> amd_pmf_get_sensor_info(). As a result, stale HPD data is used by PMF-TA
>> to evaluate policy conditions, leading to unexpected behavior in the policy
>> output actions.
>>
>> To resolve this issue, modify the PMF driver to query HPD data
>> independently of ALS.
>>
>> With this change, amd_pmf_get_sensor_info() now returns void instead of
>> int.
>>
>> Fixes: cedecdba60f4 ("platform/x86/amd/pmf: Get ambient light information from AMD SFH driver")
>> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
>> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>> drivers/platform/x86/amd/pmf/spc.c | 33 +++++++++++++++---------------
>> 1 file changed, 16 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
>> index a3dec14c3004..d9e60d63553c 100644
>> --- a/drivers/platform/x86/amd/pmf/spc.c
>> +++ b/drivers/platform/x86/amd/pmf/spc.c
>> @@ -150,7 +150,7 @@ static int amd_pmf_get_slider_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>> return 0;
>> }
>>
>> -static int amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
>> +static void amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
>> {
>> struct amd_sfh_info sfh_info;
>> int ret;
>> @@ -160,26 +160,25 @@ static int amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>> if (!ret)
>> in->ev_info.ambient_light = sfh_info.ambient_light;
>> else
>> - return ret;
>> + dev_dbg(dev->dev, "ALS is not enabled\n");
>>
>> /* get HPD data */
>> ret = amd_get_sfh_info(&sfh_info, MT_HPD);
>> - if (ret)
>> - return ret;
>> -
>> - switch (sfh_info.user_present) {
>> - case SFH_NOT_DETECTED:
>> - in->ev_info.user_present = 0xff; /* assume no sensors connected */
>> - break;
>> - case SFH_USER_PRESENT:
>> - in->ev_info.user_present = 1;
>> - break;
>> - case SFH_USER_AWAY:
>> - in->ev_info.user_present = 0;
>> - break;
>> + if (!ret) {
>> + switch (sfh_info.user_present) {
>> + case SFH_NOT_DETECTED:
>> + in->ev_info.user_present = 0xff; /* assume no sensors connected */
>> + break;
>> + case SFH_USER_PRESENT:
>> + in->ev_info.user_present = 1;
>> + break;
>> + case SFH_USER_AWAY:
>> + in->ev_info.user_present = 0;
>> + break;
>> + }
>> + } else {
>> + dev_dbg(dev->dev, "HPD is not enabled\n");
>
> Is it okay to leave in->ev_info.user_present as 0 this case? Should it be
> set to same as with SFH_NOT_DETECTED?
>
Valid point. Realized that the PMF TA only accepts a boolean and it
only looks for information on if the user is present or not.
I have adjusted this behavior in v2. Please take a look.
> ...I now realize your print out doesn't cover that case at all and
> user_present is not used for anything else than debug printing.
>
entire populated "ev_info" is passed to the PMF TA to evaluate the
policy conditions. So, its just not limited to debug message.
Thanks,
Shyam
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] platform/x86/amd/pmf: Add new ACPI ID AMDI0107
2024-07-23 13:24 [PATCH 1/2] platform/x86/amd/pmf: Add new ACPI ID AMDI0107 Shyam Sundar S K
2024-07-23 13:24 ` [PATCH 2/2] platform/x86/amd/pmf: Fix to Update HPD Data When ALS is Disabled Shyam Sundar S K
@ 2024-07-30 12:38 ` Ilpo Järvinen
2024-07-30 12:47 ` Shyam Sundar S K
1 sibling, 1 reply; 6+ messages in thread
From: Ilpo Järvinen @ 2024-07-30 12:38 UTC (permalink / raw)
To: Shyam Sundar S K; +Cc: Hans de Goede, platform-driver-x86, Patil.Reddy
On Tue, 23 Jul 2024, Shyam Sundar S K wrote:
> Add new ACPI ID AMDI0107 used by upcoming AMD platform to the PMF
> supported list of devices.
>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
This patch 1 applied to review-ilpo (the other is still pending with an
outstanding comment/question).
--
i.
> drivers/platform/x86/amd/pmf/core.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index 2d6e2558863c..8f1f719befa3 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -41,6 +41,7 @@
> #define AMD_CPU_ID_RMB 0x14b5
> #define AMD_CPU_ID_PS 0x14e8
> #define PCI_DEVICE_ID_AMD_1AH_M20H_ROOT 0x1507
> +#define PCI_DEVICE_ID_AMD_1AH_M60H_ROOT 0x1122
>
> #define PMF_MSG_DELAY_MIN_US 50
> #define RESPONSE_REGISTER_LOOP_MAX 20000
> @@ -249,6 +250,7 @@ static const struct pci_device_id pmf_pci_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_RMB) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_PS) },
> { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M20H_ROOT) },
> + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_ROOT) },
> { }
> };
>
> @@ -382,6 +384,7 @@ static const struct acpi_device_id amd_pmf_acpi_ids[] = {
> {"AMDI0102", 0},
> {"AMDI0103", 0},
> {"AMDI0105", 0},
> + {"AMDI0107", 0},
> { }
> };
> MODULE_DEVICE_TABLE(acpi, amd_pmf_acpi_ids);
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] platform/x86/amd/pmf: Add new ACPI ID AMDI0107
2024-07-30 12:38 ` [PATCH 1/2] platform/x86/amd/pmf: Add new ACPI ID AMDI0107 Ilpo Järvinen
@ 2024-07-30 12:47 ` Shyam Sundar S K
0 siblings, 0 replies; 6+ messages in thread
From: Shyam Sundar S K @ 2024-07-30 12:47 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: Hans de Goede, platform-driver-x86, Patil.Reddy
On 7/30/2024 18:08, Ilpo Järvinen wrote:
> On Tue, 23 Jul 2024, Shyam Sundar S K wrote:
>
>> Add new ACPI ID AMDI0107 used by upcoming AMD platform to the PMF
>> supported list of devices.
>>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>
> This patch 1 applied to review-ilpo (the other is still pending with an
> outstanding comment/question).
>
Thank you Ilpo. I have looked at your remarks on patch 2/2. Shall send
a new revision in some time.
Thanks,
Shyam
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-07-30 14:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-23 13:24 [PATCH 1/2] platform/x86/amd/pmf: Add new ACPI ID AMDI0107 Shyam Sundar S K
2024-07-23 13:24 ` [PATCH 2/2] platform/x86/amd/pmf: Fix to Update HPD Data When ALS is Disabled Shyam Sundar S K
2024-07-29 11:55 ` Ilpo Järvinen
2024-07-30 14:27 ` Shyam Sundar S K
2024-07-30 12:38 ` [PATCH 1/2] platform/x86/amd/pmf: Add new ACPI ID AMDI0107 Ilpo Järvinen
2024-07-30 12:47 ` Shyam Sundar S K
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox