Re: [PATCH 2/2] platform/x86/amd/pmf: Get SRA sensor data from AMD SFH driver

[Shyam Sundar S K <Shyam-sundar.S-k@amd.com>]

On 12/12/2024 21:56, Mario Limonciello wrote:
> On 12/12/2024 09:19, Shyam Sundar S K wrote:
>> The AMD SFH driver includes APIs to export SRA sensor data. This
>> data is
>> utilized by the AMD PMF driver to transmit SRA data to the PMF TA,
>> enabling the AMD PMF driver to implement the output actions
>> specified by
>> the PMF TA.
>>
>> 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/pmf.h | 18 ++++++++++-
>>   drivers/platform/x86/amd/pmf/spc.c | 51 ++++++++++++++++++++++++++
>> ++++
>>   2 files changed, 68 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/
>> x86/amd/pmf/pmf.h
>> index a79808fda1d8..c343eaa84755 100644
>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>> @@ -616,6 +616,20 @@ enum ta_slider {
>>       TA_MAX,
>>   };
>>   +enum platform_type {
>> +    PTYPE_UNKNOWN = 0,
>> +    LID_CLOSE,
>> +    CLAMSHELL,
>> +    FLAT,
>> +    TENT,
>> +    STAND,
>> +    TABLET,
>> +    BOOK,
>> +    PRESENTATION,
>> +    PULL_FWD,
>> +    PTYPE_INVALID = 0Xf,
>> +};
>> +
>>   /* Command ids for TA communication */
>>   enum ta_pmf_command {
>>       TA_PMF_COMMAND_POLICY_BUILDER_INITIALIZE,
>> @@ -667,7 +681,9 @@ struct ta_pmf_condition_info {
>>       u32 device_state;
>>       u32 socket_power;
>>       u32 skin_temperature;
>> -    u32 rsvd3[5];
>> +    u32 rsvd3[2];
>> +    u32 platform_type;
>> +    u32 rsvd3_1[2];
>>       u32 ambient_light;
>>       u32 length;
>>       u32 avg_c0residency;
>> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/
>> x86/amd/pmf/spc.c
>> index 06226eb0eab3..d5f764e624b4 100644
>> --- a/drivers/platform/x86/amd/pmf/spc.c
>> +++ b/drivers/platform/x86/amd/pmf/spc.c
>> @@ -16,6 +16,46 @@
>>   #include "pmf.h"
>>     #ifdef CONFIG_AMD_PMF_DEBUG
>> +static const char *platform_type_as_str(u16 platform_type)
>> +{
>> +    switch (platform_type) {
> 
> I notice you're missing "LID_CLOSE" case here.

This was actually intentional. There is already one such print present
in amd_pmf_dump_ta_inputs() which gets populated via the
acpi_lid_open() call

dev_dbg(dev->dev, "LID State: %s\n", in->ev_info.lid_state ? "close" :
"open");

So thought to exclude it here. Makes sense?

Thanks,
Shyam


> 
>> +    case CLAMSHELL:
>> +        return "CLAMSHELL";
>> +    case FLAT:
>> +        return "FLAT";
>> +    case TENT:
>> +        return "TENT";
>> +    case STAND:
>> +        return "STAND";
>> +    case TABLET:
>> +        return "TABLET";
>> +    case BOOK:
>> +        return "BOOK";
>> +    case PRESENTATION:
>> +        return "PRESENTATION";
>> +    case PULL_FWD:
>> +        return "PULL_FWD";
>> +    default:
>> +        return "UNKNOWN";
>> +    }
>> +}
>> +
>> +static const char *laptop_placement_as_str(u16 device_state)
>> +{
>> +    switch (device_state) {
>> +    case ON_TABLE:
>> +        return "ON_TABLE";
>> +    case ON_LAP_MOTION:
>> +        return "ON_LAP_MOTION";
>> +    case IN_BAG:
>> +        return "IN_BAG";
>> +    case OUT_OF_BAG:
>> +        return "OUT_OF_BAG";
>> +    default:
>> +        return "UNKNOWN";
>> +    }
>> +}
>> +
>>   static const char *ta_slider_as_str(unsigned int state)
>>   {
>>       switch (state) {
>> @@ -47,6 +87,9 @@ void amd_pmf_dump_ta_inputs(struct amd_pmf_dev
>> *dev, struct ta_pmf_enact_table *
>>       dev_dbg(dev->dev, "LID State: %s\n", in->ev_info.lid_state ?
>> "close" : "open");
>>       dev_dbg(dev->dev, "User Presence: %s\n", in-
>> >ev_info.user_present ? "Present" : "Away");
>>       dev_dbg(dev->dev, "Ambient Light: %d\n", in-
>> >ev_info.ambient_light);
>> +    dev_dbg(dev->dev, "Platform type: %s\n",
>> platform_type_as_str(in->ev_info.platform_type));
>> +    dev_dbg(dev->dev, "Laptop placement: %s\n",
>> +        laptop_placement_as_str(in->ev_info.device_state));
>>       dev_dbg(dev->dev, "==== TA inputs END ====\n");
>>   }
>>   #else
>> @@ -190,6 +233,14 @@ static void amd_pmf_get_sensor_info(struct
>> amd_pmf_dev *dev, struct ta_pmf_enact
>>       } else {
>>           dev_dbg(dev->dev, "HPD is not enabled/detected\n");
>>       }
>> +
>> +    /* Get SRA (Secondary Accelerometer) data */
>> +    if (!amd_get_sfh_info(&sfh_info, MT_SRA)) {
>> +        in->ev_info.platform_type = sfh_info.platform_type;
>> +        in->ev_info.device_state = sfh_info.laptop_placement;
>> +    } else {
>> +        dev_dbg(dev->dev, "SRA is not enabled/detected\n");
>> +    }
>>   }
>>     void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct
>> ta_pmf_enact_table *in)
>