[PATCH 0/2] Add new capabilities to PMF Smart PC

[PATCH 0/2] Add new capabilities to PMF Smart PC

[Shyam Sundar S K]

This series introduces the following enhancements:

- Support for device operating states in the AMD SFH driver through the
SRA sensor.
- Functionality in the PMF driver to export SRA data from the SFH driver
to the PMF, allowing it to be used as input for the PMF-TA in evaluating
policy conditions that involve SRA sensor data.

Basavaraj Natikar (1):
  HID: amd_sfh: Add support to export device operating states

Shyam Sundar S K (1):
  platform/x86/amd/pmf: Get SRA sensor data from AMD SFH driver

 drivers/hid/amd-sfh-hid/amd_sfh_common.h      |  1 +
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 22 ++++++++
 .../amd-sfh-hid/sfh1_1/amd_sfh_interface.c    | 35 +++++++++++++
 .../amd-sfh-hid/sfh1_1/amd_sfh_interface.h    | 20 ++++++++
 drivers/platform/x86/amd/pmf/pmf.h            | 18 ++++++-
 drivers/platform/x86/amd/pmf/spc.c            | 51 +++++++++++++++++++
 include/linux/amd-pmf-io.h                    | 15 ++++++
 7 files changed, 161 insertions(+), 1 deletion(-)

-- 
2.34.1

[PATCH 1/2] HID: amd_sfh: Add support to export device operating states

[Shyam Sundar S K]

From: Basavaraj Natikar <basavaraj.natikar@amd.com>

Add support to export device operating states, such as laptop placement,
platform types and propagate this data to AMD PMF driver for use in
actions.

To retrieve the device operating states data, SRA sensor support need to
be enabled in AMD SFH driver. So add support to enable the SRA sensor.

Co-developed-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
Signed-off-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
Signed-off-by: Basavaraj Natikar <basavaraj.natikar@amd.com>
---
 drivers/hid/amd-sfh-hid/amd_sfh_common.h      |  1 +
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 22 ++++++++++++
 .../amd-sfh-hid/sfh1_1/amd_sfh_interface.c    | 35 +++++++++++++++++++
 .../amd-sfh-hid/sfh1_1/amd_sfh_interface.h    | 20 +++++++++++
 include/linux/amd-pmf-io.h                    | 15 ++++++++
 5 files changed, 93 insertions(+)

diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
index e5620d7db569..799b8686a88a 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
@@ -43,6 +43,7 @@ struct amd_mp2_sensor_info {
 struct sfh_dev_status {
 	bool is_hpd_present;
 	bool is_als_present;
+	bool is_sra_present;
 };
 
 struct amd_mp2_dev {
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
index db36d87d5634..03c028f1aab4 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
@@ -30,6 +30,7 @@ static int amd_sfh_get_sensor_num(struct amd_mp2_dev *mp2, u8 *sensor_id)
 		case ACCEL_IDX:
 		case GYRO_IDX:
 		case MAG_IDX:
+		case SRA_IDX:
 		case ALS_IDX:
 		case HPD_IDX:
 			if (BIT(i) & slist->sl.sensors)
@@ -58,6 +59,8 @@ static const char *get_sensor_name(int idx)
 		return "gyroscope";
 	case MAG_IDX:
 		return "magnetometer";
+	case SRA_IDX:
+		return "SRA";
 	case ALS_IDX:
 		return "ALS";
 	case HPD_IDX:
@@ -130,6 +133,23 @@ static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata)
 
 	for (i = 0; i < cl_data->num_hid_devices; i++) {
 		cl_data->sensor_sts[i] = SENSOR_DISABLED;
+
+		if (cl_data->num_hid_devices == 1 && cl_data->sensor_idx[0] == SRA_IDX)
+			break;
+
+		if (cl_data->sensor_idx[i] == SRA_IDX) {
+			info.sensor_idx = cl_data->sensor_idx[i];
+			writel(0, privdata->mmio + amd_get_p2c_val(privdata, 0));
+			mp2_ops->start(privdata, info);
+			status = amd_sfh_wait_for_response
+				(privdata, cl_data->sensor_idx[i], ENABLE_SENSOR);
+
+			cl_data->sensor_sts[i] = (status == 0) ? SENSOR_ENABLED : SENSOR_DISABLED;
+			if (cl_data->sensor_sts[i] == SENSOR_ENABLED)
+				privdata->dev_en.is_sra_present = true;
+			continue;
+		}
+
 		cl_data->sensor_requested_cnt[i] = 0;
 		cl_data->cur_hid_dev = i;
 		cl_idx = cl_data->sensor_idx[i];
@@ -181,6 +201,8 @@ static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata)
 	}
 
 	for (i = 0; i < cl_data->num_hid_devices; i++) {
+		if (cl_data->sensor_idx[i] == SRA_IDX)
+			continue;
 		cl_data->cur_hid_dev = i;
 		if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
 			cl_data->is_any_sensor_enabled = true;
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
index 4676f060da26..b4c0d96ab152 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
@@ -87,6 +87,38 @@ void sfh_interface_init(struct amd_mp2_dev *mp2)
 	emp2 = mp2;
 }
 
+static int amd_sfh_mode_info(u32 *platform_type, u32 *laptop_placement)
+{
+	struct sfh_op_mode mode;
+
+	if (!platform_type || !laptop_placement)
+		return -EINVAL;
+
+	if (!emp2 || !emp2->dev_en.is_sra_present)
+		return -ENODEV;
+
+	mode.val = readl(emp2->mmio + amd_get_c2p_val(emp2, 3));
+
+	*platform_type = mode.op_mode.devicemode;
+
+	if (mode.op_mode.ontablestate == 1)
+		*laptop_placement = ON_TABLE;
+	else if (mode.op_mode.ontablestate == 2)
+		*laptop_placement = ON_LAP_MOTION;
+	else if (mode.op_mode.inbagstate == 1)
+		*laptop_placement = IN_BAG;
+	else if (mode.op_mode.outbagstate == 1)
+		*laptop_placement = OUT_OF_BAG;
+	else if (mode.op_mode.ontablestate == 0 || mode.op_mode.inbagstate == 0 ||
+		 mode.op_mode.outbagstate == 0)
+		*laptop_placement = LP_UNKNOWN;
+	else if (mode.op_mode.ontablestate == 3 || mode.op_mode.inbagstate == 3 ||
+		 mode.op_mode.outbagstate == 3)
+		*laptop_placement = LP_UNDEFINED;
+
+	return 0;
+}
+
 static int amd_sfh_hpd_info(u8 *user_present)
 {
 	struct hpd_status hpdstatus;
@@ -131,6 +163,9 @@ int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op)
 			return amd_sfh_hpd_info(&sfh_info->user_present);
 		case MT_ALS:
 			return amd_sfh_als_info(&sfh_info->ambient_light);
+		case MT_SRA:
+			return amd_sfh_mode_info(&sfh_info->platform_type,
+						 &sfh_info->laptop_placement);
 		}
 	}
 	return -EINVAL;
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
index 2c211d28764d..f7eb2539bccc 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
@@ -22,6 +22,7 @@ enum sensor_index {
 	ACCEL_IDX,
 	GYRO_IDX,
 	MAG_IDX,
+	SRA_IDX,
 	ALS_IDX = 4,
 	HPD_IDX = 5,
 	MAX_IDX = 15,
@@ -164,6 +165,25 @@ struct hpd_status {
 	};
 };
 
+struct sfh_op_mode {
+	union {
+		u32 val;
+		struct {
+			u32 mode		: 3;
+			u32 lidstatus		: 1;
+			u32 angle		: 10;
+			u32 inbagstatedbg	: 2;
+			u32 ontablestate	: 2;
+			u32 inbagstate		: 2;
+			u32 outbagstate		: 2;
+			u32 inbagmlcstate	: 1;
+			u32 powerstate		: 2;
+			u32 data		: 3;
+			u32 devicemode		: 4;
+		} op_mode;
+	};
+};
+
 void sfh_interface_init(struct amd_mp2_dev *mp2);
 void sfh_deinit_emp2(void);
 void amd_sfh1_1_set_desc_ops(struct amd_mp2_ops *mp2_ops);
diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
index b4f818205216..01f2b12c56a6 100644
--- a/include/linux/amd-pmf-io.h
+++ b/include/linux/amd-pmf-io.h
@@ -18,10 +18,12 @@
  * enum sfh_message_type - Query the SFH message type
  * @MT_HPD: Message ID to know the Human presence info from MP2 FW
  * @MT_ALS: Message ID to know the Ambient light info from MP2 FW
+ * @MT_SRA: Message ID to know the SRA data from MP2 FW
  */
 enum sfh_message_type {
 	MT_HPD,
 	MT_ALS,
+	MT_SRA,
 };
 
 /**
@@ -40,10 +42,23 @@ enum sfh_hpd_info {
  * struct amd_sfh_info - get HPD sensor info from MP2 FW
  * @ambient_light: Populates the ambient light information
  * @user_present: Populates the user presence information
+ * @platform_type: Operating modes (clmashell, flat, tent, etc.)
+ * @laptop_placement: Device states (ontable, onlap, outbag)
  */
 struct amd_sfh_info {
 	u32 ambient_light;
 	u8 user_present;
+	u32 platform_type;
+	u32 laptop_placement;
+};
+
+enum laptop_placement {
+	LP_UNKNOWN = 0,
+	ON_TABLE,
+	ON_LAP_MOTION,
+	IN_BAG,
+	OUT_OF_BAG,
+	LP_UNDEFINED,
 };
 
 int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op);
-- 
2.34.1

Re: [PATCH 1/2] HID: amd_sfh: Add support to export device operating states

[Mario Limonciello]

On 12/12/2024 09:19, Shyam Sundar S K wrote:
> From: Basavaraj Natikar <basavaraj.natikar@amd.com>
> 
> Add support to export device operating states, such as laptop placement,
> platform types and propagate this data to AMD PMF driver for use in
> actions.
> 
> To retrieve the device operating states data, SRA sensor support need to
> be enabled in AMD SFH driver. So add support to enable the SRA sensor.
> 
> Co-developed-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
> Signed-off-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
> Signed-off-by: Basavaraj Natikar <basavaraj.natikar@amd.com>

When you send someone else's patch but don't change it you are still 
supposed to add your "own" S-o-b.

I have two small nits below.

> ---
>   drivers/hid/amd-sfh-hid/amd_sfh_common.h      |  1 +
>   drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 22 ++++++++++++
>   .../amd-sfh-hid/sfh1_1/amd_sfh_interface.c    | 35 +++++++++++++++++++
>   .../amd-sfh-hid/sfh1_1/amd_sfh_interface.h    | 20 +++++++++++
>   include/linux/amd-pmf-io.h                    | 15 ++++++++
>   5 files changed, 93 insertions(+)
> 
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> index e5620d7db569..799b8686a88a 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> @@ -43,6 +43,7 @@ struct amd_mp2_sensor_info {
>   struct sfh_dev_status {
>   	bool is_hpd_present;
>   	bool is_als_present;
> +	bool is_sra_present;
>   };
>   
>   struct amd_mp2_dev {
> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
> index db36d87d5634..03c028f1aab4 100644
> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
> @@ -30,6 +30,7 @@ static int amd_sfh_get_sensor_num(struct amd_mp2_dev *mp2, u8 *sensor_id)
>   		case ACCEL_IDX:
>   		case GYRO_IDX:
>   		case MAG_IDX:
> +		case SRA_IDX:
>   		case ALS_IDX:
>   		case HPD_IDX:
>   			if (BIT(i) & slist->sl.sensors)
> @@ -58,6 +59,8 @@ static const char *get_sensor_name(int idx)
>   		return "gyroscope";
>   	case MAG_IDX:
>   		return "magnetometer";
> +	case SRA_IDX:
> +		return "SRA";
>   	case ALS_IDX:
>   		return "ALS";
>   	case HPD_IDX:
> @@ -130,6 +133,23 @@ static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata)
>   
>   	for (i = 0; i < cl_data->num_hid_devices; i++) {
>   		cl_data->sensor_sts[i] = SENSOR_DISABLED;
> +
> +		if (cl_data->num_hid_devices == 1 && cl_data->sensor_idx[0] == SRA_IDX)
> +			break;
> +
> +		if (cl_data->sensor_idx[i] == SRA_IDX) {
> +			info.sensor_idx = cl_data->sensor_idx[i];
> +			writel(0, privdata->mmio + amd_get_p2c_val(privdata, 0));
> +			mp2_ops->start(privdata, info);
> +			status = amd_sfh_wait_for_response
> +				(privdata, cl_data->sensor_idx[i], ENABLE_SENSOR);
> +
> +			cl_data->sensor_sts[i] = (status == 0) ? SENSOR_ENABLED : SENSOR_DISABLED;
> +			if (cl_data->sensor_sts[i] == SENSOR_ENABLED)
> +				privdata->dev_en.is_sra_present = true;
> +			continue;
> +		}
> +
>   		cl_data->sensor_requested_cnt[i] = 0;
>   		cl_data->cur_hid_dev = i;
>   		cl_idx = cl_data->sensor_idx[i];
> @@ -181,6 +201,8 @@ static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata)
>   	}
>   
>   	for (i = 0; i < cl_data->num_hid_devices; i++) {
> +		if (cl_data->sensor_idx[i] == SRA_IDX)
> +			continue;
>   		cl_data->cur_hid_dev = i;
>   		if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
>   			cl_data->is_any_sensor_enabled = true;
> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
> index 4676f060da26..b4c0d96ab152 100644
> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
> @@ -87,6 +87,38 @@ void sfh_interface_init(struct amd_mp2_dev *mp2)
>   	emp2 = mp2;
>   }
>   
> +static int amd_sfh_mode_info(u32 *platform_type, u32 *laptop_placement)
> +{
> +	struct sfh_op_mode mode;
> +
> +	if (!platform_type || !laptop_placement)
> +		return -EINVAL;
> +
> +	if (!emp2 || !emp2->dev_en.is_sra_present)
> +		return -ENODEV;
> +
> +	mode.val = readl(emp2->mmio + amd_get_c2p_val(emp2, 3));
> +
> +	*platform_type = mode.op_mode.devicemode;
> +
> +	if (mode.op_mode.ontablestate == 1)
> +		*laptop_placement = ON_TABLE;
> +	else if (mode.op_mode.ontablestate == 2)
> +		*laptop_placement = ON_LAP_MOTION;
> +	else if (mode.op_mode.inbagstate == 1)
> +		*laptop_placement = IN_BAG;
> +	else if (mode.op_mode.outbagstate == 1)
> +		*laptop_placement = OUT_OF_BAG;
> +	else if (mode.op_mode.ontablestate == 0 || mode.op_mode.inbagstate == 0 ||
> +		 mode.op_mode.outbagstate == 0)
> +		*laptop_placement = LP_UNKNOWN;
> +	else if (mode.op_mode.ontablestate == 3 || mode.op_mode.inbagstate == 3 ||
> +		 mode.op_mode.outbagstate == 3)
> +		*laptop_placement = LP_UNDEFINED;

What do you think of doing a pr_warn_once() when you end up with an 
undefined placement?  This could help point out that the driver needs to 
be changed for a newly created mode that the hardware detected.

> +
> +	return 0;
> +}
> +
>   static int amd_sfh_hpd_info(u8 *user_present)
>   {
>   	struct hpd_status hpdstatus;
> @@ -131,6 +163,9 @@ int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op)
>   			return amd_sfh_hpd_info(&sfh_info->user_present);
>   		case MT_ALS:
>   			return amd_sfh_als_info(&sfh_info->ambient_light);
> +		case MT_SRA:
> +			return amd_sfh_mode_info(&sfh_info->platform_type,
> +						 &sfh_info->laptop_placement);
>   		}
>   	}
>   	return -EINVAL;
> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
> index 2c211d28764d..f7eb2539bccc 100644
> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
> @@ -22,6 +22,7 @@ enum sensor_index {
>   	ACCEL_IDX,
>   	GYRO_IDX,
>   	MAG_IDX,
> +	SRA_IDX,
>   	ALS_IDX = 4,
>   	HPD_IDX = 5,

IIRC in C enums start at 0 right?  So ALS_IDX and HPD_IDX don't need 
explicit assingments anymore.

>   	MAX_IDX = 15,
> @@ -164,6 +165,25 @@ struct hpd_status {
>   	};
>   };
>   
> +struct sfh_op_mode {
> +	union {
> +		u32 val;
> +		struct {
> +			u32 mode		: 3;
> +			u32 lidstatus		: 1;
> +			u32 angle		: 10;
> +			u32 inbagstatedbg	: 2;
> +			u32 ontablestate	: 2;
> +			u32 inbagstate		: 2;
> +			u32 outbagstate		: 2;
> +			u32 inbagmlcstate	: 1;
> +			u32 powerstate		: 2;
> +			u32 data		: 3;
> +			u32 devicemode		: 4;
> +		} op_mode;
> +	};
> +};
> +
>   void sfh_interface_init(struct amd_mp2_dev *mp2);
>   void sfh_deinit_emp2(void);
>   void amd_sfh1_1_set_desc_ops(struct amd_mp2_ops *mp2_ops);
> diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
> index b4f818205216..01f2b12c56a6 100644
> --- a/include/linux/amd-pmf-io.h
> +++ b/include/linux/amd-pmf-io.h
> @@ -18,10 +18,12 @@
>    * enum sfh_message_type - Query the SFH message type
>    * @MT_HPD: Message ID to know the Human presence info from MP2 FW
>    * @MT_ALS: Message ID to know the Ambient light info from MP2 FW
> + * @MT_SRA: Message ID to know the SRA data from MP2 FW
>    */
>   enum sfh_message_type {
>   	MT_HPD,
>   	MT_ALS,
> +	MT_SRA,
>   };
>   
>   /**
> @@ -40,10 +42,23 @@ enum sfh_hpd_info {
>    * struct amd_sfh_info - get HPD sensor info from MP2 FW
>    * @ambient_light: Populates the ambient light information
>    * @user_present: Populates the user presence information
> + * @platform_type: Operating modes (clmashell, flat, tent, etc.)
> + * @laptop_placement: Device states (ontable, onlap, outbag)
>    */
>   struct amd_sfh_info {
>   	u32 ambient_light;
>   	u8 user_present;
> +	u32 platform_type;
> +	u32 laptop_placement;
> +};
> +
> +enum laptop_placement {
> +	LP_UNKNOWN = 0,
> +	ON_TABLE,
> +	ON_LAP_MOTION,
> +	IN_BAG,
> +	OUT_OF_BAG,
> +	LP_UNDEFINED,
>   };
>   
>   int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op);

Re: [PATCH 1/2] HID: amd_sfh: Add support to export device operating states

[Shyam Sundar S K]

On 12/12/2024 21:16, Mario Limonciello wrote:
> On 12/12/2024 09:19, Shyam Sundar S K wrote:
>> From: Basavaraj Natikar <basavaraj.natikar@amd.com>
>>
>> Add support to export device operating states, such as laptop
>> placement,
>> platform types and propagate this data to AMD PMF driver for use in
>> actions.
>>
>> To retrieve the device operating states data, SRA sensor support
>> need to
>> be enabled in AMD SFH driver. So add support to enable the SRA sensor.
>>
>> Co-developed-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
>> Signed-off-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
>> Signed-off-by: Basavaraj Natikar <basavaraj.natikar@amd.com>
> 
> When you send someone else's patch but don't change it you are still
> supposed to add your "own" S-o-b.

ah! Thanks. I missed to add it.

> 
> I have two small nits below.
> 

Sure, but I have a question to Hans and Ilpo

while we address the remarks what should be approach for merging this
series? Should it go via pdx86 tree or hid because patch 2/2 is
dependent of 1/2.

Thanks,
Shyam
>> ---
>>   drivers/hid/amd-sfh-hid/amd_sfh_common.h      |  1 +
>>   drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 22 ++++++++++++
>>   .../amd-sfh-hid/sfh1_1/amd_sfh_interface.c    | 35 +++++++++++++++
>> ++++
>>   .../amd-sfh-hid/sfh1_1/amd_sfh_interface.h    | 20 +++++++++++
>>   include/linux/amd-pmf-io.h                    | 15 ++++++++
>>   5 files changed, 93 insertions(+)
>>
>> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/
>> amd-sfh-hid/amd_sfh_common.h
>> index e5620d7db569..799b8686a88a 100644
>> --- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
>> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
>> @@ -43,6 +43,7 @@ struct amd_mp2_sensor_info {
>>   struct sfh_dev_status {
>>       bool is_hpd_present;
>>       bool is_als_present;
>> +    bool is_sra_present;
>>   };
>>     struct amd_mp2_dev {
>> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c b/
>> drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
>> index db36d87d5634..03c028f1aab4 100644
>> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
>> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
>> @@ -30,6 +30,7 @@ static int amd_sfh_get_sensor_num(struct
>> amd_mp2_dev *mp2, u8 *sensor_id)
>>           case ACCEL_IDX:
>>           case GYRO_IDX:
>>           case MAG_IDX:
>> +        case SRA_IDX:
>>           case ALS_IDX:
>>           case HPD_IDX:
>>               if (BIT(i) & slist->sl.sensors)
>> @@ -58,6 +59,8 @@ static const char *get_sensor_name(int idx)
>>           return "gyroscope";
>>       case MAG_IDX:
>>           return "magnetometer";
>> +    case SRA_IDX:
>> +        return "SRA";
>>       case ALS_IDX:
>>           return "ALS";
>>       case HPD_IDX:
>> @@ -130,6 +133,23 @@ static int amd_sfh1_1_hid_client_init(struct
>> amd_mp2_dev *privdata)
>>         for (i = 0; i < cl_data->num_hid_devices; i++) {
>>           cl_data->sensor_sts[i] = SENSOR_DISABLED;
>> +
>> +        if (cl_data->num_hid_devices == 1 && cl_data->sensor_idx[0]
>> == SRA_IDX)
>> +            break;
>> +
>> +        if (cl_data->sensor_idx[i] == SRA_IDX) {
>> +            info.sensor_idx = cl_data->sensor_idx[i];
>> +            writel(0, privdata->mmio + amd_get_p2c_val(privdata, 0));
>> +            mp2_ops->start(privdata, info);
>> +            status = amd_sfh_wait_for_response
>> +                (privdata, cl_data->sensor_idx[i], ENABLE_SENSOR);
>> +
>> +            cl_data->sensor_sts[i] = (status == 0) ?
>> SENSOR_ENABLED : SENSOR_DISABLED;
>> +            if (cl_data->sensor_sts[i] == SENSOR_ENABLED)
>> +                privdata->dev_en.is_sra_present = true;
>> +            continue;
>> +        }
>> +
>>           cl_data->sensor_requested_cnt[i] = 0;
>>           cl_data->cur_hid_dev = i;
>>           cl_idx = cl_data->sensor_idx[i];
>> @@ -181,6 +201,8 @@ static int amd_sfh1_1_hid_client_init(struct
>> amd_mp2_dev *privdata)
>>       }
>>         for (i = 0; i < cl_data->num_hid_devices; i++) {
>> +        if (cl_data->sensor_idx[i] == SRA_IDX)
>> +            continue;
>>           cl_data->cur_hid_dev = i;
>>           if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
>>               cl_data->is_any_sensor_enabled = true;
>> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/
>> drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
>> index 4676f060da26..b4c0d96ab152 100644
>> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
>> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
>> @@ -87,6 +87,38 @@ void sfh_interface_init(struct amd_mp2_dev *mp2)
>>       emp2 = mp2;
>>   }
>>   +static int amd_sfh_mode_info(u32 *platform_type, u32
>> *laptop_placement)
>> +{
>> +    struct sfh_op_mode mode;
>> +
>> +    if (!platform_type || !laptop_placement)
>> +        return -EINVAL;
>> +
>> +    if (!emp2 || !emp2->dev_en.is_sra_present)
>> +        return -ENODEV;
>> +
>> +    mode.val = readl(emp2->mmio + amd_get_c2p_val(emp2, 3));
>> +
>> +    *platform_type = mode.op_mode.devicemode;
>> +
>> +    if (mode.op_mode.ontablestate == 1)
>> +        *laptop_placement = ON_TABLE;
>> +    else if (mode.op_mode.ontablestate == 2)
>> +        *laptop_placement = ON_LAP_MOTION;
>> +    else if (mode.op_mode.inbagstate == 1)
>> +        *laptop_placement = IN_BAG;
>> +    else if (mode.op_mode.outbagstate == 1)
>> +        *laptop_placement = OUT_OF_BAG;
>> +    else if (mode.op_mode.ontablestate == 0 ||
>> mode.op_mode.inbagstate == 0 ||
>> +         mode.op_mode.outbagstate == 0)
>> +        *laptop_placement = LP_UNKNOWN;
>> +    else if (mode.op_mode.ontablestate == 3 ||
>> mode.op_mode.inbagstate == 3 ||
>> +         mode.op_mode.outbagstate == 3)
>> +        *laptop_placement = LP_UNDEFINED;
> 
> What do you think of doing a pr_warn_once() when you end up with an
> undefined placement?  This could help point out that the driver needs
> to be changed for a newly created mode that the hardware detected.
> 
>> +
>> +    return 0;
>> +}
>> +
>>   static int amd_sfh_hpd_info(u8 *user_present)
>>   {
>>       struct hpd_status hpdstatus;
>> @@ -131,6 +163,9 @@ int amd_get_sfh_info(struct amd_sfh_info
>> *sfh_info, enum sfh_message_type op)
>>               return amd_sfh_hpd_info(&sfh_info->user_present);
>>           case MT_ALS:
>>               return amd_sfh_als_info(&sfh_info->ambient_light);
>> +        case MT_SRA:
>> +            return amd_sfh_mode_info(&sfh_info->platform_type,
>> +                         &sfh_info->laptop_placement);
>>           }
>>       }
>>       return -EINVAL;
>> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/
>> drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
>> index 2c211d28764d..f7eb2539bccc 100644
>> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
>> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
>> @@ -22,6 +22,7 @@ enum sensor_index {
>>       ACCEL_IDX,
>>       GYRO_IDX,
>>       MAG_IDX,
>> +    SRA_IDX,
>>       ALS_IDX = 4,
>>       HPD_IDX = 5,
> 
> IIRC in C enums start at 0 right?  So ALS_IDX and HPD_IDX don't need
> explicit assingments anymore.
> 
>>       MAX_IDX = 15,
>> @@ -164,6 +165,25 @@ struct hpd_status {
>>       };
>>   };
>>   +struct sfh_op_mode {
>> +    union {
>> +        u32 val;
>> +        struct {
>> +            u32 mode        : 3;
>> +            u32 lidstatus        : 1;
>> +            u32 angle        : 10;
>> +            u32 inbagstatedbg    : 2;
>> +            u32 ontablestate    : 2;
>> +            u32 inbagstate        : 2;
>> +            u32 outbagstate        : 2;
>> +            u32 inbagmlcstate    : 1;
>> +            u32 powerstate        : 2;
>> +            u32 data        : 3;
>> +            u32 devicemode        : 4;
>> +        } op_mode;
>> +    };
>> +};
>> +
>>   void sfh_interface_init(struct amd_mp2_dev *mp2);
>>   void sfh_deinit_emp2(void);
>>   void amd_sfh1_1_set_desc_ops(struct amd_mp2_ops *mp2_ops);
>> diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
>> index b4f818205216..01f2b12c56a6 100644
>> --- a/include/linux/amd-pmf-io.h
>> +++ b/include/linux/amd-pmf-io.h
>> @@ -18,10 +18,12 @@
>>    * enum sfh_message_type - Query the SFH message type
>>    * @MT_HPD: Message ID to know the Human presence info from MP2 FW
>>    * @MT_ALS: Message ID to know the Ambient light info from MP2 FW
>> + * @MT_SRA: Message ID to know the SRA data from MP2 FW
>>    */
>>   enum sfh_message_type {
>>       MT_HPD,
>>       MT_ALS,
>> +    MT_SRA,
>>   };
>>     /**
>> @@ -40,10 +42,23 @@ enum sfh_hpd_info {
>>    * struct amd_sfh_info - get HPD sensor info from MP2 FW
>>    * @ambient_light: Populates the ambient light information
>>    * @user_present: Populates the user presence information
>> + * @platform_type: Operating modes (clmashell, flat, tent, etc.)
>> + * @laptop_placement: Device states (ontable, onlap, outbag)
>>    */
>>   struct amd_sfh_info {
>>       u32 ambient_light;
>>       u8 user_present;
>> +    u32 platform_type;
>> +    u32 laptop_placement;
>> +};
>> +
>> +enum laptop_placement {
>> +    LP_UNKNOWN = 0,
>> +    ON_TABLE,
>> +    ON_LAP_MOTION,
>> +    IN_BAG,
>> +    OUT_OF_BAG,
>> +    LP_UNDEFINED,
>>   };
>>     int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum
>> sfh_message_type op);
> 

Re: [PATCH 1/2] HID: amd_sfh: Add support to export device operating states

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 10787 bytes --]

On Thu, 12 Dec 2024, Shyam Sundar S K wrote:
> On 12/12/2024 21:16, Mario Limonciello wrote:
> > On 12/12/2024 09:19, Shyam Sundar S K wrote:
> >> From: Basavaraj Natikar <basavaraj.natikar@amd.com>
> >>
> >> Add support to export device operating states, such as laptop
> >> placement,
> >> platform types and propagate this data to AMD PMF driver for use in
> >> actions.
> >>
> >> To retrieve the device operating states data, SRA sensor support
> >> need to
> >> be enabled in AMD SFH driver. So add support to enable the SRA sensor.
> >>
> >> Co-developed-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
> >> Signed-off-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
> >> Signed-off-by: Basavaraj Natikar <basavaraj.natikar@amd.com>
> > 
> > When you send someone else's patch but don't change it you are still
> > supposed to add your "own" S-o-b.
> 
> ah! Thanks. I missed to add it.
> 
> > 
> > I have two small nits below.
> > 
> 
> Sure, but I have a question to Hans and Ilpo
> 
> while we address the remarks what should be approach for merging this
> series? Should it go via pdx86 tree or hid because patch 2/2 is
> dependent of 1/2.

Hi,

Given pdx86 pmf driver gets much more changes overall, it would seem 
better to merge the series through pdx86 tree. But I also want to mention 
that generally it's also possible to make requests on merge path as the 
submitter of the series, in particular, it is good to take into account
if you know there are patches that might conflict with the changes 
(within this kernel cycle) to make the merge window less problematic for 
maintainers.

[In some cases it's possible to create an immutable branch which can be 
merged by two (or more) subsystems but I don't think it provides added 
value here given how low traffic amd-sfh-hid is.]

-- 
 i.

> >> ---
> >>   drivers/hid/amd-sfh-hid/amd_sfh_common.h      |  1 +
> >>   drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 22 ++++++++++++
> >>   .../amd-sfh-hid/sfh1_1/amd_sfh_interface.c    | 35 +++++++++++++++
> >> ++++
> >>   .../amd-sfh-hid/sfh1_1/amd_sfh_interface.h    | 20 +++++++++++
> >>   include/linux/amd-pmf-io.h                    | 15 ++++++++
> >>   5 files changed, 93 insertions(+)
> >>
> >> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/
> >> amd-sfh-hid/amd_sfh_common.h
> >> index e5620d7db569..799b8686a88a 100644
> >> --- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> >> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> >> @@ -43,6 +43,7 @@ struct amd_mp2_sensor_info {
> >>   struct sfh_dev_status {
> >>       bool is_hpd_present;
> >>       bool is_als_present;
> >> +    bool is_sra_present;
> >>   };
> >>     struct amd_mp2_dev {
> >> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c b/
> >> drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
> >> index db36d87d5634..03c028f1aab4 100644
> >> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
> >> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
> >> @@ -30,6 +30,7 @@ static int amd_sfh_get_sensor_num(struct
> >> amd_mp2_dev *mp2, u8 *sensor_id)
> >>           case ACCEL_IDX:
> >>           case GYRO_IDX:
> >>           case MAG_IDX:
> >> +        case SRA_IDX:
> >>           case ALS_IDX:
> >>           case HPD_IDX:
> >>               if (BIT(i) & slist->sl.sensors)
> >> @@ -58,6 +59,8 @@ static const char *get_sensor_name(int idx)
> >>           return "gyroscope";
> >>       case MAG_IDX:
> >>           return "magnetometer";
> >> +    case SRA_IDX:
> >> +        return "SRA";
> >>       case ALS_IDX:
> >>           return "ALS";
> >>       case HPD_IDX:
> >> @@ -130,6 +133,23 @@ static int amd_sfh1_1_hid_client_init(struct
> >> amd_mp2_dev *privdata)
> >>         for (i = 0; i < cl_data->num_hid_devices; i++) {
> >>           cl_data->sensor_sts[i] = SENSOR_DISABLED;
> >> +
> >> +        if (cl_data->num_hid_devices == 1 && cl_data->sensor_idx[0]
> >> == SRA_IDX)
> >> +            break;
> >> +
> >> +        if (cl_data->sensor_idx[i] == SRA_IDX) {
> >> +            info.sensor_idx = cl_data->sensor_idx[i];
> >> +            writel(0, privdata->mmio + amd_get_p2c_val(privdata, 0));
> >> +            mp2_ops->start(privdata, info);
> >> +            status = amd_sfh_wait_for_response
> >> +                (privdata, cl_data->sensor_idx[i], ENABLE_SENSOR);
> >> +
> >> +            cl_data->sensor_sts[i] = (status == 0) ?
> >> SENSOR_ENABLED : SENSOR_DISABLED;
> >> +            if (cl_data->sensor_sts[i] == SENSOR_ENABLED)
> >> +                privdata->dev_en.is_sra_present = true;
> >> +            continue;
> >> +        }
> >> +
> >>           cl_data->sensor_requested_cnt[i] = 0;
> >>           cl_data->cur_hid_dev = i;
> >>           cl_idx = cl_data->sensor_idx[i];
> >> @@ -181,6 +201,8 @@ static int amd_sfh1_1_hid_client_init(struct
> >> amd_mp2_dev *privdata)
> >>       }
> >>         for (i = 0; i < cl_data->num_hid_devices; i++) {
> >> +        if (cl_data->sensor_idx[i] == SRA_IDX)
> >> +            continue;
> >>           cl_data->cur_hid_dev = i;
> >>           if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
> >>               cl_data->is_any_sensor_enabled = true;
> >> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/
> >> drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
> >> index 4676f060da26..b4c0d96ab152 100644
> >> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
> >> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
> >> @@ -87,6 +87,38 @@ void sfh_interface_init(struct amd_mp2_dev *mp2)
> >>       emp2 = mp2;
> >>   }
> >>   +static int amd_sfh_mode_info(u32 *platform_type, u32
> >> *laptop_placement)
> >> +{
> >> +    struct sfh_op_mode mode;
> >> +
> >> +    if (!platform_type || !laptop_placement)
> >> +        return -EINVAL;
> >> +
> >> +    if (!emp2 || !emp2->dev_en.is_sra_present)
> >> +        return -ENODEV;
> >> +
> >> +    mode.val = readl(emp2->mmio + amd_get_c2p_val(emp2, 3));
> >> +
> >> +    *platform_type = mode.op_mode.devicemode;
> >> +
> >> +    if (mode.op_mode.ontablestate == 1)
> >> +        *laptop_placement = ON_TABLE;
> >> +    else if (mode.op_mode.ontablestate == 2)
> >> +        *laptop_placement = ON_LAP_MOTION;
> >> +    else if (mode.op_mode.inbagstate == 1)
> >> +        *laptop_placement = IN_BAG;
> >> +    else if (mode.op_mode.outbagstate == 1)
> >> +        *laptop_placement = OUT_OF_BAG;
> >> +    else if (mode.op_mode.ontablestate == 0 ||
> >> mode.op_mode.inbagstate == 0 ||
> >> +         mode.op_mode.outbagstate == 0)
> >> +        *laptop_placement = LP_UNKNOWN;
> >> +    else if (mode.op_mode.ontablestate == 3 ||
> >> mode.op_mode.inbagstate == 3 ||
> >> +         mode.op_mode.outbagstate == 3)
> >> +        *laptop_placement = LP_UNDEFINED;
> > 
> > What do you think of doing a pr_warn_once() when you end up with an
> > undefined placement?  This could help point out that the driver needs
> > to be changed for a newly created mode that the hardware detected.
> > 
> >> +
> >> +    return 0;
> >> +}
> >> +
> >>   static int amd_sfh_hpd_info(u8 *user_present)
> >>   {
> >>       struct hpd_status hpdstatus;
> >> @@ -131,6 +163,9 @@ int amd_get_sfh_info(struct amd_sfh_info
> >> *sfh_info, enum sfh_message_type op)
> >>               return amd_sfh_hpd_info(&sfh_info->user_present);
> >>           case MT_ALS:
> >>               return amd_sfh_als_info(&sfh_info->ambient_light);
> >> +        case MT_SRA:
> >> +            return amd_sfh_mode_info(&sfh_info->platform_type,
> >> +                         &sfh_info->laptop_placement);
> >>           }
> >>       }
> >>       return -EINVAL;
> >> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/
> >> drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
> >> index 2c211d28764d..f7eb2539bccc 100644
> >> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
> >> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
> >> @@ -22,6 +22,7 @@ enum sensor_index {
> >>       ACCEL_IDX,
> >>       GYRO_IDX,
> >>       MAG_IDX,
> >> +    SRA_IDX,
> >>       ALS_IDX = 4,
> >>       HPD_IDX = 5,
> > 
> > IIRC in C enums start at 0 right?  So ALS_IDX and HPD_IDX don't need
> > explicit assingments anymore.
> > 
> >>       MAX_IDX = 15,
> >> @@ -164,6 +165,25 @@ struct hpd_status {
> >>       };
> >>   };
> >>   +struct sfh_op_mode {
> >> +    union {
> >> +        u32 val;
> >> +        struct {
> >> +            u32 mode        : 3;
> >> +            u32 lidstatus        : 1;
> >> +            u32 angle        : 10;
> >> +            u32 inbagstatedbg    : 2;
> >> +            u32 ontablestate    : 2;
> >> +            u32 inbagstate        : 2;
> >> +            u32 outbagstate        : 2;
> >> +            u32 inbagmlcstate    : 1;
> >> +            u32 powerstate        : 2;
> >> +            u32 data        : 3;
> >> +            u32 devicemode        : 4;
> >> +        } op_mode;
> >> +    };
> >> +};
> >> +
> >>   void sfh_interface_init(struct amd_mp2_dev *mp2);
> >>   void sfh_deinit_emp2(void);
> >>   void amd_sfh1_1_set_desc_ops(struct amd_mp2_ops *mp2_ops);
> >> diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
> >> index b4f818205216..01f2b12c56a6 100644
> >> --- a/include/linux/amd-pmf-io.h
> >> +++ b/include/linux/amd-pmf-io.h
> >> @@ -18,10 +18,12 @@
> >>    * enum sfh_message_type - Query the SFH message type
> >>    * @MT_HPD: Message ID to know the Human presence info from MP2 FW
> >>    * @MT_ALS: Message ID to know the Ambient light info from MP2 FW
> >> + * @MT_SRA: Message ID to know the SRA data from MP2 FW
> >>    */
> >>   enum sfh_message_type {
> >>       MT_HPD,
> >>       MT_ALS,
> >> +    MT_SRA,
> >>   };
> >>     /**
> >> @@ -40,10 +42,23 @@ enum sfh_hpd_info {
> >>    * struct amd_sfh_info - get HPD sensor info from MP2 FW
> >>    * @ambient_light: Populates the ambient light information
> >>    * @user_present: Populates the user presence information
> >> + * @platform_type: Operating modes (clmashell, flat, tent, etc.)
> >> + * @laptop_placement: Device states (ontable, onlap, outbag)
> >>    */
> >>   struct amd_sfh_info {
> >>       u32 ambient_light;
> >>       u8 user_present;
> >> +    u32 platform_type;
> >> +    u32 laptop_placement;
> >> +};
> >> +
> >> +enum laptop_placement {
> >> +    LP_UNKNOWN = 0,
> >> +    ON_TABLE,
> >> +    ON_LAP_MOTION,
> >> +    IN_BAG,
> >> +    OUT_OF_BAG,
> >> +    LP_UNDEFINED,
> >>   };
> >>     int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum
> >> sfh_message_type op);
> > 
> 

Re: [PATCH 1/2] HID: amd_sfh: Add support to export device operating states

[Shyam Sundar S K]

Hi Ilpo,

On 12/12/2024 22:17, Ilpo Järvinen wrote:
> On Thu, 12 Dec 2024, Shyam Sundar S K wrote:
>> On 12/12/2024 21:16, Mario Limonciello wrote:
>>> On 12/12/2024 09:19, Shyam Sundar S K wrote:
>>>> From: Basavaraj Natikar <basavaraj.natikar@amd.com>
>>>>
>>>> Add support to export device operating states, such as laptop
>>>> placement,
>>>> platform types and propagate this data to AMD PMF driver for use in
>>>> actions.
>>>>
>>>> To retrieve the device operating states data, SRA sensor support
>>>> need to
>>>> be enabled in AMD SFH driver. So add support to enable the SRA sensor.
>>>>
>>>> Co-developed-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
>>>> Signed-off-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
>>>> Signed-off-by: Basavaraj Natikar <basavaraj.natikar@amd.com>
>>>
>>> When you send someone else's patch but don't change it you are still
>>> supposed to add your "own" S-o-b.
>>
>> ah! Thanks. I missed to add it.
>>
>>>
>>> I have two small nits below.
>>>
>>
>> Sure, but I have a question to Hans and Ilpo
>>
>> while we address the remarks what should be approach for merging this
>> series? Should it go via pdx86 tree or hid because patch 2/2 is
>> dependent of 1/2.
> 
> Hi,
> 
> Given pdx86 pmf driver gets much more changes overall, it would seem 
> better to merge the series through pdx86 tree. But I also want to mention 
> that generally it's also possible to make requests on merge path as the 
> submitter of the series, in particular, it is good to take into account
> if you know there are patches that might conflict with the changes 
> (within this kernel cycle) to make the merge window less problematic for 
> maintainers.
> 
> [In some cases it's possible to create an immutable branch which can be 
> merged by two (or more) subsystems but I don't think it provides added 
> value here given how low traffic amd-sfh-hid is.]
> 

Thank you. I have sent out v2 and added additional notes to the
cover-letter.

Thanks,
Shyam

Re: [PATCH 1/2] HID: amd_sfh: Add support to export device operating states

[Mario Limonciello]

On 12/12/2024 09:19, Shyam Sundar S K wrote:
>   
>   /**
> @@ -40,10 +42,23 @@ enum sfh_hpd_info {
>    * struct amd_sfh_info - get HPD sensor info from MP2 FW
>    * @ambient_light: Populates the ambient light information
>    * @user_present: Populates the user presence information
> + * @platform_type: Operating modes (clmashell, flat, tent, etc.)

Sorry, one more issue.  You have a spelling error here "clmashell".

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

[Shyam Sundar S K]

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) {
+	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)
-- 
2.34.1

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

[Mario Limonciello]

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.

> +	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)

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

[Shyam Sundar S K]

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)
> 

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

[Mario Limonciello]

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)
>>
>