Re: [PATCH 15/15] platform/x86/amd/pmf: Add PMF-AMDSFH interface for ALS

[Mario Limonciello <mario.limonciello@xxxxxxx>]

On 9/22/2023 12:50, Shyam Sundar S K wrote:
> From: Basavaraj Natikar <Basavaraj.Natikar@xxxxxxx>
>
> AMDSFH has information about the Ambient light via the Ambient
> Light Sensor (ALS) which is part of the AMD sensor fusion hub.
> Add PMF and AMDSFH interface to get this information.
>
> Co-developed-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@xxxxxxx>
> ---
>   drivers/hid/amd-sfh-hid/amd_sfh_common.h      |  1 +
>   drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c |  6 ++++++
>   .../amd-sfh-hid/sfh1_1/amd_sfh_interface.c    | 20 +++++++++++++++++++
>   drivers/platform/x86/amd/pmf/spc.c            |  5 +++++
>   include/linux/amd-pmf-io.h                    |  2 ++
>   5 files changed, 34 insertions(+)
>
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> index cd57037bf217..a1950bc6e6ce 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> @@ -39,6 +39,7 @@ struct amd_mp2_sensor_info {
>   
>   struct sfh_dev_status {
>   	bool is_hpd_present;
> +	bool is_als_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 9c623456ee12..d8dad39d68b5 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
> @@ -77,6 +77,9 @@ static int amd_sfh_hid_client_deinit(struct amd_mp2_dev *privdata)
>   		case HPD_IDX:
>   			privdata->dev_en.is_hpd_present = false;
>   			break;
> +		case ALS_IDX:
> +			privdata->dev_en.is_als_present = false;
> +			break;
>   		}
>   
>   		if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
> @@ -188,6 +191,9 @@ static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata)
>   			case HPD_IDX:
>   			privdata->dev_en.is_hpd_present = true;
>   				break;
> +			case ALS_IDX:
> +			privdata->dev_en.is_als_present = true;
> +				break;

Same missing tab here as previous patch

>   			}
>   		}
>   		dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
> 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 63a5bbca5a09..2f8200fc3062 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
> @@ -94,12 +94,32 @@ static int amd_sfh_hpd_info(u8 *user_present)
>   	return  -ENODEV;
>   }
>   
> +static int amd_sfh_als_info(u32 *ambient_light)
> +{
> +	if (emp2 && emp2->dev_en.is_als_present) {
> +		struct sfh_als_data als_data;
> +		void __iomem *sensoraddr;
> +
> +		sensoraddr = emp2->vsbase +
> +			(ALS_IDX * SENSOR_DATA_MEM_SIZE_DEFAULT) +
> +			OFFSET_SENSOR_DATA_DEFAULT;
> +		memcpy_fromio(&als_data, sensoraddr, sizeof(struct sfh_als_data));
> +		*ambient_light = float_to_int(als_data.lux);
> +
> +		return 0;
> +	}
> +
> +	return -ENODEV;
> +}
> +
>   int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op)
>   {
>   	if (sfh_info) {
>   		switch (op) {
>   		case MT_HPD:
>   			return amd_sfh_hpd_info(&sfh_info->user_present);
> +		case MT_ALS:
> +			return amd_sfh_als_info(&sfh_info->ambient_light);
>   		}
>   	}
>   	return -1;
> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> index 97293ae25cf5..8e19b351e76f 100644
> --- a/drivers/platform/x86/amd/pmf/spc.c
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -49,6 +49,7 @@ void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *
>   			"Connected" : "disconnected/unknown");
>   	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, "==== TA inputs END ====\n");
>   }
>   #else
> @@ -161,6 +162,10 @@ static void amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct ta_pmf_enact
>   {
>   	struct amd_sfh_info sfh_info;
>   
> +	/* get ALS data */
> +	amd_get_sfh_info(&sfh_info, MT_ALS);

Like previous patch, I think you should look at return code here.

> +	in->ev_info.ambient_light = sfh_info.ambient_light;
> +
>   	/* get HPD data */
>   	amd_get_sfh_info(&sfh_info, MT_HPD);
>   	switch (sfh_info.user_present) {
> diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
> index 4f82973f6ad2..dac0af573a16 100644
> --- a/include/linux/amd-pmf-io.h
> +++ b/include/linux/amd-pmf-io.h
> @@ -31,6 +31,7 @@ int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf);
>   /* amd-sfh */
>   enum sfh_message_type {
>   	MT_HPD,
> +	MT_ALS,
>   };
>   
>   enum hpd_info {
> @@ -40,6 +41,7 @@ enum hpd_info {
>   };
>   
>   struct amd_sfh_info {
> +	u32 ambient_light;
>   	u8 user_present;
>   	/* add future caps below */
>   };