Re: [PATCH v5 16/17] platform/x86/amd/pmf: Add PMF-AMDSFH interface for HPD

[Mario Limonciello <mario.limonciello@amd.com>]

On 11/17/2023 02:07, 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_init.c | 11 +++++
>   .../amd-sfh-hid/sfh1_1/amd_sfh_interface.c    | 28 +++++++++++
>   drivers/platform/x86/amd/pmf/Kconfig          |  1 +
>   drivers/platform/x86/amd/pmf/spc.c            | 22 +++++++++
>   include/linux/amd-pmf-io.h                    | 46 +++++++++++++++++++
>   6 files changed, 113 insertions(+)
>   create mode 100644 include/linux/amd-pmf-io.h
> 
> 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_init.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
> index e9c6413af24a..3dc652d41d7d 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;
> +				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..c0f31efe4927 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;

How about when SFH is destroyed?  I think you want to explicitly set 
emp2 to NULL;

> +}
> +
> +static int amd_sfh_hpd_info(u8 *user_present)
> +{
> +	struct hpd_status hpdstatus;
> +
> +	if (!emp2 || !emp2->dev_en.is_hpd_present)
> +		return -ENODEV;
> +
> +	hpdstatus.val = readl(emp2->mmio + AMD_C2P_MSG(4));
> +	*user_present = hpdstatus.shpd.presence;
> +	return 0;
> +}
> +
> +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 -EINVAL;
>   }
> +EXPORT_SYMBOL_GPL(amd_get_sfh_info);
> diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
> index f246252bddd8..f4fa8bd8bda8 100644
> --- a/drivers/platform/x86/amd/pmf/Kconfig
> +++ b/drivers/platform/x86/amd/pmf/Kconfig
> @@ -10,6 +10,7 @@ config AMD_PMF
>   	depends on AMD_NB
>   	select ACPI_PLATFORM_PROFILE
>   	depends on TEE && AMDTEE
> +	depends on AMD_SFH_HID
>   	help
>   	  This driver provides support for the AMD Platform Management Framework.
>   	  The goal is to enhance end user experience by making AMD PCs smarter,
> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> index 47ec563088b8..7f63973ab40e 100644
> --- a/drivers/platform/x86/amd/pmf/spc.c
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -10,6 +10,7 @@
>    */
>   
>   #include <acpi/button.h>
> +#include <linux/amd-pmf-io.h>
>   #include <linux/power_supply.h>
>   #include <linux/units.h>
>   #include "pmf.h"
> @@ -49,6 +50,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
> @@ -185,6 +187,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);
> +	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 */
> @@ -195,4 +216,5 @@ void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_tab
>   	amd_pmf_get_slider_info(dev, in);
>   	if (dev->drm_dev)
>   		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
> new file mode 100644
> index 000000000000..5b6d29d36922
> --- /dev/null
> +++ b/include/linux/amd-pmf-io.h
> @@ -0,0 +1,46 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * AMD Platform Management Framework Interface
> + *
> + * Copyright (c) 2023, Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Authors: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> + *          Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> + */
> +
> +#ifndef AMD_PMF_IO_H
> +#define AMD_PMF_IO_H
> +
> +#include <linux/types.h>
> +
> +/**
> + * enum sfh_message_type - Query the SFH message type
> + * @MT_HPD: Message ID to know the Human presence info from MP2 FW
> + */
> +enum sfh_message_type {
> +	MT_HPD,
> +};
> +
> +/**
> + * enum sfh_hpd_info - Query the Human presence information
> + * @SFH_NOT_DETECTED: Check the HPD connection information from MP2 FW
> + * @SFH_USER_PRESENT: Check if the user is present from HPD sensor
> + * @SFH_USER_AWAY: Check if the user is away from HPD sensor
> + */
> +enum sfh_hpd_info {
> +	SFH_NOT_DETECTED,
> +	SFH_USER_PRESENT,
> +	SFH_USER_AWAY,
> +};
> +
> +/**
> + * struct amd_sfh_info - get HPD sensor info from MP2 FW
> + * @user_present: Populates the user presence information
> + */
> +struct amd_sfh_info {
> +	u8 user_present;
> +};
> +
> +int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op);
> +#endif