From: Mario Limonciello <mario.limonciello@amd.com>
To: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>,
hdegoede@redhat.com, markgross@kernel.org,
basavaraj.natikar@amd.com, jikos@kernel.org,
benjamin.tissoires@redhat.com, alexander.deucher@amd.com,
christian.koenig@amd.com, Xinhui.Pan@amd.com, airlied@gmail.com,
daniel@ffwll.ch
Cc: dri-devel@lists.freedesktop.org, linux-input@vger.kernel.org,
amd-gfx@lists.freedesktop.org, Patil.Reddy@amd.com,
platform-driver-x86@vger.kernel.org
Subject: Re: [PATCH 14/15] platform/x86/amd/pmf: Add PMF-AMDSFH interface for HPD
Date: Fri, 22 Sep 2023 16:04:32 -0500 [thread overview]
Message-ID: <fcdd044c-07e8-4c16-bcac-14a193867816@amd.com> (raw)
In-Reply-To: <f370fd18-345a-4b7d-b074-550539e7933c@amd.com>
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@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@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 | 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@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@amd.com>
>> + * Authors: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> + * Basavaraj Natikar <Basavaraj.Natikar@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
>
next prev parent reply other threads:[~2023-09-22 21:04 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-22 17:50 [PATCH 00/15] Introduce PMF Smart PC Solution Builder Feature Shyam Sundar S K
2023-09-22 17:50 ` [PATCH 01/15] platform/x86/amd/pmf: Add PMF TEE interface Shyam Sundar S K
2023-09-26 16:42 ` Ilpo Järvinen
2023-09-22 17:50 ` [PATCH 02/15] platform/x86/amd/pmf: Add support PMF-TA interaction Shyam Sundar S K
2023-09-26 16:48 ` Ilpo Järvinen
2023-09-22 17:50 ` [PATCH 03/15] platform/x86/amd/pmf: Change signature of amd_pmf_set_dram_addr Shyam Sundar S K
2023-09-26 16:52 ` Ilpo Järvinen
2023-09-22 17:50 ` [PATCH 04/15] platform/x86/amd/pmf: Add support for PMF Policy Binary Shyam Sundar S K
2023-09-22 18:51 ` Mario Limonciello
2023-09-30 3:55 ` Shyam Sundar S K
2023-09-26 17:05 ` Ilpo Järvinen
2023-09-27 12:19 ` Ilpo Järvinen
2023-09-22 17:50 ` [PATCH 05/15] platform/x86/amd/pmf: change debugfs init sequence Shyam Sundar S K
2023-09-26 16:53 ` Ilpo Järvinen
2023-09-22 17:50 ` [PATCH 06/15] platform/x86/amd/pmf: Add support to get inputs from other subsystems Shyam Sundar S K
2023-09-26 17:08 ` Ilpo Järvinen
2023-09-30 8:40 ` Shyam Sundar S K
2023-10-02 9:10 ` Ilpo Järvinen
2023-09-22 17:50 ` [PATCH 07/15] platform/x86/amd/pmf: Add support update p3t limit Shyam Sundar S K
2023-09-27 12:19 ` Ilpo Järvinen
2023-09-22 17:50 ` [PATCH 08/15] platform/x86/amd/pmf: Add support to update system state Shyam Sundar S K
2023-09-25 21:42 ` kernel test robot
2023-09-27 12:22 ` Ilpo Järvinen
2023-09-22 17:50 ` [PATCH 09/15] platform/x86/amd/pmf: Add facility to dump TA inputs Shyam Sundar S K
2023-09-27 12:25 ` Ilpo Järvinen
2023-09-22 17:50 ` [PATCH 10/15] platform/x86/amd/pmf: Add capability to sideload of policy binary Shyam Sundar S K
2023-09-25 2:14 ` kernel test robot
2023-09-27 12:33 ` Ilpo Järvinen
2023-09-22 17:50 ` [PATCH 11/15] platform/x86/amd/pmf: dump policy binary data Shyam Sundar S K
2023-09-22 19:01 ` Mario Limonciello
2023-09-30 4:41 ` Shyam Sundar S K
2023-09-22 17:50 ` [PATCH 12/15] platform/x86/amd/pmf: Add PMF-AMDGPU get interface Shyam Sundar S K
2023-09-27 12:54 ` Ilpo Järvinen
2023-09-22 17:50 ` [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set interface Shyam Sundar S K
2023-09-25 16:27 ` Deucher, Alexander
2023-09-25 16:30 ` Mario Limonciello
2023-09-26 11:17 ` Shyam Sundar S K
2023-09-26 11:15 ` Shyam Sundar S K
2023-09-26 10:35 ` Hans de Goede
2023-09-26 11:24 ` Shyam Sundar S K
2023-09-26 12:56 ` Hans de Goede
2023-09-26 13:17 ` Christian König
2023-09-26 13:48 ` Shyam Sundar S K
2023-09-27 13:04 ` Hans de Goede
2023-09-27 13:47 ` Shyam Sundar S K
2023-09-27 13:36 ` Ilpo Järvinen
2023-09-22 17:50 ` [PATCH 14/15] platform/x86/amd/pmf: Add PMF-AMDSFH interface for HPD Shyam Sundar S K
2023-09-22 19:04 ` Mario Limonciello
2023-09-22 21:04 ` Mario Limonciello [this message]
2023-09-27 13:32 ` Ilpo Järvinen
2023-09-22 17:50 ` [PATCH 15/15] platform/x86/amd/pmf: Add PMF-AMDSFH interface for ALS Shyam Sundar S K
2023-09-22 19:06 ` Mario Limonciello
2023-09-27 13:33 ` Ilpo Järvinen
2023-09-27 13:48 ` Shyam Sundar S K
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fcdd044c-07e8-4c16-bcac-14a193867816@amd.com \
--to=mario.limonciello@amd.com \
--cc=Patil.Reddy@amd.com \
--cc=Shyam-sundar.S-k@amd.com \
--cc=Xinhui.Pan@amd.com \
--cc=airlied@gmail.com \
--cc=alexander.deucher@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=basavaraj.natikar@amd.com \
--cc=benjamin.tissoires@redhat.com \
--cc=christian.koenig@amd.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=hdegoede@redhat.com \
--cc=jikos@kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=markgross@kernel.org \
--cc=platform-driver-x86@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox