[PATCH 14/15] platform/x86/amd/pmf: Add PMF-AMDSFH interface for HPD
Mario Limonciello
mario.limonciello at amd.com
Fri Sep 22 21:04:32 UTC 2023
On 9/22/2023 14:04, Mario Limonciello wrote:
> On 9/22/2023 12:50, Shyam Sundar S K wrote:
>> From: Basavaraj Natikar <Basavaraj.Natikar at amd.com>
>>
>> AMDSFH has information about the User presence information via the Human
>> Presence Detection (HPD) sensor 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 at amd.com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k at amd.com>
>> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar at amd.com>
>> ---
>> drivers/hid/amd-sfh-hid/amd_sfh_common.h | 5 ++++
>> drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 2 +-
>> drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 11 ++++++++
>> .../amd-sfh-hid/sfh1_1/amd_sfh_interface.c | 28 +++++++++++++++++++
>> .../amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 1 +
>> drivers/platform/x86/amd/pmf/spc.c | 21 ++++++++++++++
>> include/linux/amd-pmf-io.h | 22 ++++++++++++++-
>> 7 files changed, 88 insertions(+), 2 deletions(-)
Somthing else I noticed about this change.
I believe you should be adding to Kconfig:
depends on AMD_SFH_HID
>>
>> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
>> b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
>> index 2643bb14fee2..cd57037bf217 100644
>> --- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
>> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
>> @@ -37,6 +37,10 @@ struct amd_mp2_sensor_info {
>> dma_addr_t dma_address;
>> };
>> +struct sfh_dev_status {
>> + bool is_hpd_present;
>> +};
>> +
>> struct amd_mp2_dev {
>> struct pci_dev *pdev;
>> struct amdtp_cl_data *cl_data;
>> @@ -47,6 +51,7 @@ struct amd_mp2_dev {
>> struct amd_input_data in_data;
>> /* mp2 active control status */
>> u32 mp2_acs;
>> + struct sfh_dev_status dev_en;
>> };
>> struct amd_mp2_ops {
>> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
>> b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
>> index 06bdcf072d10..d7467c41ad3b 100644
>> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
>> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
>> @@ -132,7 +132,7 @@ static void get_common_inputs(struct
>> common_input_property *common, int report_i
>> common->event_type = HID_USAGE_SENSOR_EVENT_DATA_UPDATED_ENUM;
>> }
>> -static int float_to_int(u32 flt32_val)
>> +int float_to_int(u32 flt32_val)
>> {
>> int fraction, shift, mantissa, sign, exp, zeropre;
>> 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 e9c6413af24a..9c623456ee12 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
>> @@ -73,6 +73,12 @@ static int amd_sfh_hid_client_deinit(struct
>> amd_mp2_dev *privdata)
>> int i, status;
>> for (i = 0; i < cl_data->num_hid_devices; i++) {
>> + switch (cl_data->sensor_idx[i]) {
>> + case HPD_IDX:
>> + privdata->dev_en.is_hpd_present = false;
>> + break;
>> + }
>> +
>> if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
>> privdata->mp2_ops->stop(privdata, cl_data->sensor_idx[i]);
>> status = amd_sfh_wait_for_response
>> @@ -178,6 +184,11 @@ static int amd_sfh1_1_hid_client_init(struct
>> amd_mp2_dev *privdata)
>> rc = amdtp_hid_probe(i, cl_data);
>> if (rc)
>> goto cleanup;
>> + switch (cl_data->sensor_idx[i]) {
>> + case HPD_IDX:
>> + privdata->dev_en.is_hpd_present = true;
>
> Looks like this is missing a tab.
>
>> + break;
>> + }
>> }
>> dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
>> cl_data->sensor_idx[i],
>> get_sensor_name(cl_data->sensor_idx[i]),
>> 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 4f81ef2d4f56..63a5bbca5a09 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
>> @@ -7,11 +7,14 @@
>> *
>> * Author: Basavaraj Natikar <Basavaraj.Natikar at amd.com>
>> */
>> +#include <linux/amd-pmf-io.h>
>> #include <linux/io-64-nonatomic-lo-hi.h>
>> #include <linux/iopoll.h>
>> #include "amd_sfh_interface.h"
>> +static struct amd_mp2_dev *emp2;
>> +
>> static int amd_sfh_wait_response(struct amd_mp2_dev *mp2, u8 sid,
>> u32 cmd_id)
>> {
>> struct sfh_cmd_response cmd_resp;
>> @@ -76,4 +79,29 @@ static struct amd_mp2_ops amd_sfh_ops = {
>> void sfh_interface_init(struct amd_mp2_dev *mp2)
>> {
>> mp2->mp2_ops = &amd_sfh_ops;
>> + emp2 = mp2;
>> +}
>> +
>> +static int amd_sfh_hpd_info(u8 *user_present)
>> +{
>> + if (emp2 && emp2->dev_en.is_hpd_present) {
>> + struct hpd_status hpdstatus;
>> +
>> + hpdstatus.val = readl(emp2->mmio + AMD_C2P_MSG(4));
>> + *user_present = hpdstatus.shpd.presence;
>> + return 0;
>> + }
>> + return -ENODEV;
>
> Extra whitespace here.
>
>> +}
>> +
IMO you should add some kernel doc here or in the header as good
practice for exported symbols.
>> +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);
>> + }
>> + }
>> + return -1;
>
> This looks like it should probably be -EINVAL.
>
>> }
>> +EXPORT_SYMBOL_GPL(amd_get_sfh_info);
>> 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 9d31d5b510eb..8a36386e6bce 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
>> @@ -149,6 +149,7 @@ struct hpd_status {
>> };
>> };
>> +int float_to_int(u32 flt32_val);
>> void sfh_interface_init(struct amd_mp2_dev *mp2);
>> void amd_sfh1_1_set_desc_ops(struct amd_mp2_ops *mp2_ops);
>> #endif
>> diff --git a/drivers/platform/x86/amd/pmf/spc.c
>> b/drivers/platform/x86/amd/pmf/spc.c
>> index 5f3ab1ce09d2..97293ae25cf5 100644
>> --- a/drivers/platform/x86/amd/pmf/spc.c
>> +++ b/drivers/platform/x86/amd/pmf/spc.c
>> @@ -48,6 +48,7 @@ void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev,
>> struct ta_pmf_enact_table *
>> dev_dbg(dev->dev, "Primary Display State : %s\n",
>> in->ev_info.display_state ?
>> "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, "==== TA inputs END ====\n");
>> }
>> #else
>> @@ -156,6 +157,25 @@ static void amd_pmf_get_gpu_info(struct
>> amd_pmf_dev *dev, struct ta_pmf_enact_ta
>> in->ev_info.display_state = dev->gfx_data.con_status[0];
>> }
>> +static void amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct
>> ta_pmf_enact_table *in)
>> +{
>> + struct amd_sfh_info sfh_info;
>> +
>> + /* get HPD data */
>> + amd_get_sfh_info(&sfh_info, MT_HPD);
>
> amd_get_sfh_info() is int.
>
> Shouldn't you look for the return code here?
>
>> + switch (sfh_info.user_present) {
>> + case SFH_NOT_DETECTED:
>> + in->ev_info.user_present = 0xff; /* assume no sensors
>> connected */
>> + break;
>> + case SFH_USER_PRESENT:
>> + in->ev_info.user_present = 1;
>> + break;
>> + case SFH_USER_AWAY:
>> + in->ev_info.user_present = 0;
>> + break;
>> + }
>> +}
>> +
>> void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct
>> ta_pmf_enact_table *in)
>> {
>> /* TA side lid open is 1 and close is 0, hence the ! here */
>> @@ -165,4 +185,5 @@ void amd_pmf_populate_ta_inputs(struct amd_pmf_dev
>> *dev, struct ta_pmf_enact_tab
>> amd_pmf_get_battery_info(dev, in);
>> amd_pmf_get_slider_info(dev, in);
>> amd_pmf_get_gpu_info(dev, in);
>> + amd_pmf_get_sensor_info(dev, in);
>> }
>> diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
>> index ecae387ddaa6..4f82973f6ad2 100644
>> --- a/include/linux/amd-pmf-io.h
>> +++ b/include/linux/amd-pmf-io.h
>> @@ -5,7 +5,8 @@
>> * Copyright (c) 2023, Advanced Micro Devices, Inc.
>> * All Rights Reserved.
>> *
>> - * Author: Shyam Sundar S K <Shyam-sundar.S-k at amd.com>
>> + * Authors: Shyam Sundar S K <Shyam-sundar.S-k at amd.com>
>> + * Basavaraj Natikar <Basavaraj.Natikar at amd.com>
>> */
>> #ifndef AMD_PMF_IO_H
>> @@ -26,4 +27,23 @@ struct amd_gpu_pmf_data {
>> int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
>> int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf);
>> +
>> +/* amd-sfh */
>> +enum sfh_message_type {
>> + MT_HPD,
>> +};
>> +
>> +enum hpd_info {
>> + SFH_NOT_DETECTED,
>> + SFH_USER_PRESENT,
>> + SFH_USER_AWAY
>> +};
>> +
>> +struct amd_sfh_info {
>> + u8 user_present;
>> + /* add future caps below */
>> +};
>> +
>> +int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum
>> sfh_message_type op);
>> +
>> #endif
>
More information about the amd-gfx
mailing list