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

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

Hi Ilpo,

On 9/27/2023 7:03 PM, Ilpo Järvinen wrote:
> On Fri, 22 Sep 2023, Shyam Sundar S K wrote:
> 
>> From: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
>>
>> 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@amd.com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@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 |  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;
>>  			}
>>  		}
>>  		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);
> 
> %d vs u32

Thank you very much for your feedback. I will respin a new version
soon addressing your review remarks.

Thanks,
Shyam

> 
>>  	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);
>> +	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 */
>>  };
>>
>