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

[Mario Limonciello <mario.limonciello@amd.com>]

On 12/16/2024 11:57, Shyam Sundar S K wrote:
> 
> 
> 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?

Oh I see.

OK, that's fine and makes more 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)
>>
>