* [PATCH 08/15] platform/x86/amd/pmf: Add support to update system state
From: Shyam Sundar S K @ 2023-09-22 17:50 UTC (permalink / raw)
To: hdegoede, markgross, basavaraj.natikar, jikos, benjamin.tissoires,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel, Shyam Sundar S K
In-Reply-To: <20230922175056.244940-1-Shyam-sundar.S-k@amd.com>
PMF driver based on the output actions from the TA can request to update
the system states like entering s0i3, lock screen etc. by generating
an uevent. Based on the udev rules set in the userspace the event id
matching the uevent shall get updated accordingly using the systemctl.
Sample udev rules under Documentation/admin-guide/pmf.rst.
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
Documentation/admin-guide/pmf.rst | 24 ++++++++++++++++
drivers/platform/x86/amd/pmf/pmf.h | 9 ++++++
drivers/platform/x86/amd/pmf/tee-if.c | 40 ++++++++++++++++++++++++++-
3 files changed, 72 insertions(+), 1 deletion(-)
create mode 100644 Documentation/admin-guide/pmf.rst
diff --git a/Documentation/admin-guide/pmf.rst b/Documentation/admin-guide/pmf.rst
new file mode 100644
index 000000000000..b60f381410c3
--- /dev/null
+++ b/Documentation/admin-guide/pmf.rst
@@ -0,0 +1,24 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Set udev rules for PMF Smart PC Builder
+---------------------------------------
+
+AMD PMF(Platform Management Framework) Smart PC Solution builder has to set the system states
+like S0i3, Screen lock, hibernate etc, based on the output actions provided by the PMF
+TA (Trusted Application).
+
+In order for this to work the PMF driver generates a uevent for userspace to react to. Below are
+sample udev rules that can facilitate this experience when a machine has PMF Smart PC solution builder
+enabled.
+
+Please add the following line(s) to
+``/etc/udev/rules.d/99-local.rules``::
+ DRIVERS=="amd-pmf", ACTION=="change", ENV{EVENT_ID}=="1", RUN+="/usr/bin/systemctl suspend"
+ DRIVERS=="amd-pmf", ACTION=="change", ENV{EVENT_ID}=="2", RUN+="/usr/bin/systemctl hibernate"
+ DRIVERS=="amd-pmf", ACTION=="change", ENV{EVENT_ID}=="3", RUN+="/bin/loginctl lock-sessions"
+
+EVENT_ID values:
+1= Put the system to S0i3/S2Idle
+2= Put the system to hibernate
+3= Lock the screen
+
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 897f61b75e2f..c5334f1177a4 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -70,6 +70,7 @@
#define PMF_POLICY_STT_MIN 6
#define PMF_POLICY_STT_SKINTEMP_APU 7
#define PMF_POLICY_STT_SKINTEMP_HS2 8
+#define PMF_POLICY_SYSTEM_STATE 9
#define PMF_POLICY_P3T 38
/* TA macros */
@@ -436,6 +437,13 @@ struct apmf_dyn_slider_output {
} __packed;
/* Smart PC - TA internals */
+enum system_state {
+ SYSTEM_STATE__S0i3 = 1,
+ SYSTEM_STATE__S4,
+ SYSTEM_STATE__SCREEN_LOCK,
+ SYSTEM_STATE__MAX
+};
+
enum ta_slider {
TA_BEST_BATTERY, /* Best Battery */
TA_BETTER_BATTERY, /* Better Battery */
@@ -467,6 +475,7 @@ enum ta_pmf_error_type {
};
struct pmf_action_table {
+ enum system_state system_state;
unsigned long spl; /* in mW */
unsigned long sppt; /* in mW */
unsigned long sppt_apuonly; /* in mW */
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 883dd143375a..1629856c20b4 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -24,6 +24,20 @@ MODULE_PARM_DESC(pb_actions_ms, "Policy binary actions sampling frequency (defau
static const uuid_t amd_pmf_ta_uuid = UUID_INIT(0x6fd93b77, 0x3fb8, 0x524d,
0xb1, 0x2d, 0xc5, 0x29, 0xb1, 0x3d, 0x85, 0x43);
+static const char *amd_pmf_uevent_as_str(unsigned int state)
+{
+ switch (state) {
+ case SYSTEM_STATE__S0i3:
+ return "S0i3";
+ case SYSTEM_STATE__S4:
+ return "S4";
+ case SYSTEM_STATE__SCREEN_LOCK:
+ return "SCREEN_LOCK";
+ default:
+ return "Unknown Smart PC event";
+ }
+}
+
static void amd_pmf_prepare_args(struct amd_pmf_dev *dev, int cmd,
struct tee_ioctl_invoke_arg *arg,
struct tee_param *param)
@@ -42,9 +56,23 @@ static void amd_pmf_prepare_args(struct amd_pmf_dev *dev, int cmd,
param[0].u.memref.shm_offs = 0;
}
+static int amd_pmf_update_uevents(struct amd_pmf_dev *dev, u16 event)
+{
+ char *envp[2] = {};
+
+ envp[0] = kasprintf(GFP_KERNEL, "EVENT_ID=%d", event);
+ if (!envp[0])
+ return -EINVAL;
+
+ kobject_uevent_env(&dev->dev->kobj, KOBJ_CHANGE, envp);
+
+ kfree(envp[0]);
+ return 0;
+}
+
static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
{
- u32 val;
+ u32 val, event = 0;
int idx;
for (idx = 0; idx < out->actions_count; idx++) {
@@ -113,6 +141,16 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
dev->prev_data->p3t_limit = val;
}
break;
+
+ case PMF_POLICY_SYSTEM_STATE:
+ event = val + 1;
+ if (dev->prev_data->system_state != event) {
+ amd_pmf_update_uevents(dev, event);
+ dev_dbg(dev->dev, "update SYSTEM_STATE : %s\n",
+ amd_pmf_uevent_as_str(event));
+ dev->prev_data->system_state = 0;
+ }
+ break;
}
}
}
--
2.25.1
^ permalink raw reply related
* [PATCH 09/15] platform/x86/amd/pmf: Add facility to dump TA inputs
From: Shyam Sundar S K @ 2023-09-22 17:50 UTC (permalink / raw)
To: hdegoede, markgross, basavaraj.natikar, jikos, benjamin.tissoires,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel, Shyam Sundar S K
In-Reply-To: <20230922175056.244940-1-Shyam-sundar.S-k@amd.com>
PMF driver sends constant inputs to TA which its gets via the other
subsystems in the kernel. To debug certain TA issues knowing what inputs
being sent to TA becomes critical. Add debug facility to the driver which
can isolate Smart PC and TA related issues.
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/pmf.h | 3 +++
drivers/platform/x86/amd/pmf/spc.c | 37 +++++++++++++++++++++++++++
drivers/platform/x86/amd/pmf/sps.c | 2 +-
drivers/platform/x86/amd/pmf/tee-if.c | 1 +
4 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index c5334f1177a4..61a0f3225b62 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -592,6 +592,7 @@ int apmf_get_static_slider_granular(struct amd_pmf_dev *pdev,
bool is_pprof_balanced(struct amd_pmf_dev *pmf);
int amd_pmf_power_slider_update_event(struct amd_pmf_dev *dev);
+const char *source_as_str(unsigned int state);
int apmf_update_fan_idx(struct amd_pmf_dev *pdev, bool manual, u32 idx);
int amd_pmf_set_sps_power_limits(struct amd_pmf_dev *pmf);
@@ -622,4 +623,6 @@ int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev);
/* Smart PC - TA interfaces */
void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in);
+void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in);
+
#endif /* PMF_H */
diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
index 08159cd5f853..5c6745f56ed1 100644
--- a/drivers/platform/x86/amd/pmf/spc.c
+++ b/drivers/platform/x86/amd/pmf/spc.c
@@ -13,6 +13,43 @@
#include <linux/power_supply.h>
#include "pmf.h"
+#ifdef CONFIG_AMD_PMF_DEBUG
+static const char *ta_slider_as_str(unsigned int state)
+{
+ switch (state) {
+ case TA_BEST_PERFORMANCE:
+ return "PERFORMANCE";
+ case TA_BETTER_PERFORMANCE:
+ return "BALANCED";
+ case TA_BEST_BATTERY:
+ return "POWER_SAVER";
+ default:
+ return "Unknown TA Slider State";
+ }
+}
+
+void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
+{
+ dev_dbg(dev->dev, "==== TA inputs START ====\n");
+ dev_dbg(dev->dev, "Slider State : %s\n", ta_slider_as_str(in->ev_info.power_slider));
+ dev_dbg(dev->dev, "Power Source : %s\n", source_as_str(in->ev_info.power_source));
+ dev_dbg(dev->dev, "Battery Percentage : %d\n", in->ev_info.bat_percentage);
+ dev_dbg(dev->dev, "Designed Battery Capacity : %d\n", in->ev_info.bat_design);
+ dev_dbg(dev->dev, "Fully Charged Capacity : %d\n", in->ev_info.full_charge_capacity);
+ dev_dbg(dev->dev, "Drain Rate : %d\n", in->ev_info.drain_rate);
+ dev_dbg(dev->dev, "Socket Power : %d\n", in->ev_info.socket_power);
+ dev_dbg(dev->dev, "Skin Temperature : %d\n", in->ev_info.skin_temperature);
+ dev_dbg(dev->dev, "Avg C0 Residency : %d\n", in->ev_info.avg_c0residency);
+ dev_dbg(dev->dev, "Max C0 Residency : %d\n", in->ev_info.max_c0residency);
+ dev_dbg(dev->dev, "GFX Busy : %d\n", in->ev_info.gfx_busy);
+ dev_dbg(dev->dev, "Connected Display Count : %d\n", in->ev_info.monitor_count);
+ dev_dbg(dev->dev, "LID State : %s\n", in->ev_info.lid_state ? "Close" : "Open");
+ dev_dbg(dev->dev, "==== TA inputs END ====\n");
+}
+#else
+void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in) {}
+#endif
+
static void amd_pmf_get_smu_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
{
u16 max, avg = 0;
diff --git a/drivers/platform/x86/amd/pmf/sps.c b/drivers/platform/x86/amd/pmf/sps.c
index a70e67749be3..13e36b52dfe8 100644
--- a/drivers/platform/x86/amd/pmf/sps.c
+++ b/drivers/platform/x86/amd/pmf/sps.c
@@ -27,7 +27,7 @@ static const char *slider_as_str(unsigned int state)
}
}
-static const char *source_as_str(unsigned int state)
+const char *source_as_str(unsigned int state)
{
switch (state) {
case POWER_SOURCE_AC:
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 1629856c20b4..4844782d93c7 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -186,6 +186,7 @@ static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
}
if (ta_sm->pmf_result == TA_PMF_TYPE__SUCCESS && out->actions_count) {
+ amd_pmf_dump_ta_inputs(dev, in);
dev_dbg(dev->dev, "action count:%d result:%x\n", out->actions_count,
ta_sm->pmf_result);
amd_pmf_apply_policies(dev, out);
--
2.25.1
^ permalink raw reply related
* [PATCH 10/15] platform/x86/amd/pmf: Add capability to sideload of policy binary
From: Shyam Sundar S K @ 2023-09-22 17:50 UTC (permalink / raw)
To: hdegoede, markgross, basavaraj.natikar, jikos, benjamin.tissoires,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel, Shyam Sundar S K
In-Reply-To: <20230922175056.244940-1-Shyam-sundar.S-k@amd.com>
A policy binary is OS agnostic, and the same policies are expected to work
across the OSes. At times it becomes difficult to debug when the policies
inside the policy binaries starts to misbehave. Add a way to sideload such
policies independently to debug them via a debugfs entry.
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/pmf.h | 1 +
drivers/platform/x86/amd/pmf/tee-if.c | 60 +++++++++++++++++++++++++++
2 files changed, 61 insertions(+)
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 61a0f3225b62..780c442239e3 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -215,6 +215,7 @@ struct amd_pmf_dev {
bool cnqf_supported;
struct notifier_block pwr_src_notifier;
/* Smart PC solution builder */
+ struct dentry *esbin;
unsigned char *policy_buf;
u32 policy_sz;
struct tee_context *tee_ctx;
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 4844782d93c7..fa37cfab2dc7 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -8,6 +8,7 @@
* Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
*/
+#include <linux/debugfs.h>
#include <linux/tee_drv.h>
#include <linux/uuid.h>
#include "pmf.h"
@@ -21,6 +22,13 @@ module_param(pb_actions_ms, int, 0644);
MODULE_PARM_DESC(pb_actions_ms, "Policy binary actions sampling frequency (default = 1000ms)");
#endif
+#ifdef CONFIG_AMD_PMF_DEBUG
+/* Sideload policy binaries to debug policy failures */
+static bool pb_side_load;
+module_param(pb_side_load, bool, 0444);
+MODULE_PARM_DESC(pb_side_load, "Sideload policy binaries debug policy failures");
+#endif
+
static const uuid_t amd_pmf_ta_uuid = UUID_INIT(0x6fd93b77, 0x3fb8, 0x524d,
0xb1, 0x2d, 0xc5, 0x29, 0xb1, 0x3d, 0x85, 0x43);
@@ -267,6 +275,49 @@ static int amd_pmf_start_policy_engine(struct amd_pmf_dev *dev)
return 0;
}
+#ifdef CONFIG_AMD_PMF_DEBUG
+static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
+ size_t length, loff_t *pos)
+{
+ struct amd_pmf_dev *dev = filp->private_data;
+ int ret;
+
+ /* policy binary size cannot exceed POLICY_BUF_MAX_SZ */
+ if (length > POLICY_BUF_MAX_SZ || length == 0)
+ return -EINVAL;
+
+ dev->policy_sz = length;
+ if (copy_from_user(dev->policy_buf, buf, dev->policy_sz))
+ return -EFAULT;
+
+ ret = amd_pmf_start_policy_engine(dev);
+ if (ret)
+ return -EINVAL;
+
+ return length;
+}
+
+static const struct file_operations pb_fops = {
+ .write = amd_pmf_get_pb_data,
+ .open = simple_open,
+};
+
+int amd_pmf_open_pb(struct amd_pmf_dev *dev, struct dentry *debugfs_root)
+{
+ struct dentry *file = NULL;
+
+ dev->esbin = debugfs_create_dir("pb", debugfs_root);
+ if (IS_ERR(dev->esbin))
+ return -EINVAL;
+
+ file = debugfs_create_file("update_policy", 0644, dev->esbin, dev, &pb_fops);
+ if (!file)
+ return -EINVAL;
+
+ return 0;
+}
+#endif
+
static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
{
dev->policy_buf = kzalloc(dev->policy_sz, GFP_KERNEL);
@@ -279,6 +330,11 @@ static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
memcpy(dev->policy_buf, dev->policy_base, dev->policy_sz);
+#ifdef CONFIG_AMD_PMF_DEBUG
+ if (pb_side_load)
+ amd_pmf_open_pb(dev, dev->dbgfs_dir);
+#endif
+
return amd_pmf_start_policy_engine(dev);
}
@@ -381,6 +437,10 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev)
{
+#ifdef CONFIG_AMD_PMF_DEBUG
+ if (pb_side_load)
+ debugfs_remove_recursive(dev->esbin);
+#endif
kfree(dev->prev_data);
kfree(dev->policy_buf);
cancel_delayed_work_sync(&dev->pb_work);
--
2.25.1
^ permalink raw reply related
* [PATCH 11/15] platform/x86/amd/pmf: dump policy binary data
From: Shyam Sundar S K @ 2023-09-22 17:50 UTC (permalink / raw)
To: hdegoede, markgross, basavaraj.natikar, jikos, benjamin.tissoires,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel, Shyam Sundar S K
In-Reply-To: <20230922175056.244940-1-Shyam-sundar.S-k@amd.com>
Sometimes policy binary retrieved from the BIOS maybe incorrect that can
end up in failing to enable the Smart PC solution feature.
Use print_hex_dump_debug() to dump the policy binary in hex, so that we
debug the issues related to the binary even before sending that to TA.
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/tee-if.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index fa37cfab2dc7..3daa122f35d5 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -290,6 +290,9 @@ static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
if (copy_from_user(dev->policy_buf, buf, dev->policy_sz))
return -EFAULT;
+ print_hex_dump_debug("(pb): ", DUMP_PREFIX_OFFSET, 16, 1, dev->policy_buf,
+ dev->policy_sz, false);
+
ret = amd_pmf_start_policy_engine(dev);
if (ret)
return -EINVAL;
@@ -329,6 +332,10 @@ static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
return -ENOMEM;
memcpy(dev->policy_buf, dev->policy_base, dev->policy_sz);
+#ifdef CONFIG_AMD_PMF_DEBUG
+ print_hex_dump_debug("(pb): ", DUMP_PREFIX_OFFSET, 16, 1, dev->policy_buf,
+ dev->policy_sz, false);
+#endif
#ifdef CONFIG_AMD_PMF_DEBUG
if (pb_side_load)
--
2.25.1
^ permalink raw reply related
* [PATCH 12/15] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
From: Shyam Sundar S K @ 2023-09-22 17:50 UTC (permalink / raw)
To: hdegoede, markgross, basavaraj.natikar, jikos, benjamin.tissoires,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel, Shyam Sundar S K
In-Reply-To: <20230922175056.244940-1-Shyam-sundar.S-k@amd.com>
In order to provide GPU inputs to TA for the Smart PC solution to work, we
need to have interface between the PMF driver and the AMDGPU driver.
Add the initial code path for get interface from AMDGPU.
Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/gpu/drm/amd/amdgpu/Makefile | 2 +
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 70 +++++++++++++++++++++++++
drivers/platform/x86/amd/pmf/Kconfig | 1 +
drivers/platform/x86/amd/pmf/core.c | 1 +
drivers/platform/x86/amd/pmf/pmf.h | 4 ++
drivers/platform/x86/amd/pmf/spc.c | 13 +++++
drivers/platform/x86/amd/pmf/tee-if.c | 22 ++++++++
include/linux/amd-pmf-io.h | 28 ++++++++++
9 files changed, 142 insertions(+)
create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
create mode 100644 include/linux/amd-pmf-io.h
diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index 384b798a9bad..7fafccefbd7a 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -86,6 +86,8 @@ amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
+amdgpu-$(CONFIG_AMD_PMF) += amdgpu_pmf.o
+
# add asic specific block
amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o \
dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index dc2d53081e80..475f3e248f35 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -50,6 +50,7 @@
#include <linux/hashtable.h>
#include <linux/dma-fence.h>
#include <linux/pci.h>
+#include <linux/amd-pmf-io.h>
#include <drm/ttm/ttm_bo.h>
#include <drm/ttm/ttm_placement.h>
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
new file mode 100644
index 000000000000..232d11833ddc
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
@@ -0,0 +1,70 @@
+/*
+ * Copyright 2023 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+
+ * * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
+ */
+
+#include <linux/backlight.h>
+#include "amdgpu.h"
+
+int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf)
+{
+ struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
+ struct drm_mode_config *mode_config = &drm_dev->mode_config;
+ struct amdgpu_device *adev = drm_to_adev(drm_dev);
+ struct drm_connector_list_iter iter;
+ struct drm_connector *connector;
+ struct backlight_device *bd;
+ int i = 0;
+
+ /* reset the count to zero */
+ pmf->display_count = 0;
+ if (!(adev->flags & AMD_IS_APU)) {
+ DRM_ERROR("PMF-AMDGPU interface not supported\n");
+ return -ENODEV;
+ }
+
+ bd = backlight_device_get_by_type(BACKLIGHT_RAW);
+ if (!bd)
+ return -ENODEV;
+
+ pmf->brightness = backlight_get_brightness(bd);
+
+ mutex_lock(&mode_config->mutex);
+ drm_connector_list_iter_begin(drm_dev, &iter);
+
+ drm_for_each_connector_iter(connector, &iter) {
+ if (i > MAX_SUPPORTED)
+ break;
+
+ if (connector->status == connector_status_connected) {
+ pmf->con_status[i] = connector->status;
+ pmf->connector_type[i] = connector->connector_type;
+ pmf->display_count++;
+ }
+ i++;
+ }
+ drm_connector_list_iter_end(&iter);
+ mutex_unlock(&mode_config->mutex);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
index 437b78c6d1c5..0cd08f9ab51b 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 AMDTEE
+ depends on DRM_AMDGPU
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/core.c b/drivers/platform/x86/amd/pmf/core.c
index dbfe7c1d6fc4..c468d208b1dc 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -396,6 +396,7 @@ static int amd_pmf_probe(struct platform_device *pdev)
}
dev->cpu_id = rdev->device;
+ dev->root = rdev;
err = amd_smn_read(0, AMD_PMF_BASE_ADDR_LO, &val);
if (err) {
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 780c442239e3..9032df4ba48a 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -13,6 +13,8 @@
#include <linux/acpi.h>
#include <linux/platform_profile.h>
+#include <linux/amd-pmf-io.h>
+
#define POLICY_BUF_MAX_SZ 0x4b000
#define POLICY_SIGN_COOKIE 0x31535024
@@ -224,9 +226,11 @@ struct amd_pmf_dev {
void *shbuf;
struct delayed_work pb_work;
struct pmf_action_table *prev_data;
+ struct amd_gpu_pmf_data gfx_data;
u64 policy_addr;
void *policy_base;
bool smart_pc_enabled;
+ struct pci_dev *root;
};
struct apmf_sps_prop_granular {
diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
index 5c6745f56ed1..5f3ab1ce09d2 100644
--- a/drivers/platform/x86/amd/pmf/spc.c
+++ b/drivers/platform/x86/amd/pmf/spc.c
@@ -43,6 +43,10 @@ void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *
dev_dbg(dev->dev, "Max C0 Residency : %d\n", in->ev_info.max_c0residency);
dev_dbg(dev->dev, "GFX Busy : %d\n", in->ev_info.gfx_busy);
dev_dbg(dev->dev, "Connected Display Count : %d\n", in->ev_info.monitor_count);
+ dev_dbg(dev->dev, "Primary Display Type : %s\n",
+ drm_get_connector_type_name(in->ev_info.display_type));
+ 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, "==== TA inputs END ====\n");
}
@@ -144,6 +148,14 @@ static int amd_pmf_get_slider_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_
return 0;
}
+static void amd_pmf_get_gpu_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
+{
+ amd_pmf_get_gfx_data(&dev->gfx_data);
+ in->ev_info.monitor_count = dev->gfx_data.display_count;
+ in->ev_info.display_type = dev->gfx_data.connector_type[0];
+ in->ev_info.display_state = dev->gfx_data.con_status[0];
+}
+
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 */
@@ -152,4 +164,5 @@ void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_tab
amd_pmf_get_smu_info(dev, in);
amd_pmf_get_battery_info(dev, in);
amd_pmf_get_slider_info(dev, in);
+ amd_pmf_get_gpu_info(dev, in);
}
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 3daa122f35d5..1608996654e8 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -9,6 +9,7 @@
*/
#include <linux/debugfs.h>
+#include <linux/pci.h>
#include <linux/tee_drv.h>
#include <linux/uuid.h>
#include "pmf.h"
@@ -345,6 +346,20 @@ static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
return amd_pmf_start_policy_engine(dev);
}
+static int amd_pmf_get_gpu_handle(struct pci_dev *pdev, void *data)
+{
+ struct amd_pmf_dev *dev = data;
+
+ if (pdev->vendor == PCI_VENDOR_ID_ATI && pdev->devfn == 0) {
+ dev->gfx_data.gpu_dev = pci_get_device(pdev->vendor, pdev->device, NULL);
+ if (dev->gfx_data.gpu_dev) {
+ pci_dev_put(pdev);
+ return 1; /* stop walking */
+ }
+ }
+ return 0; /* continue walking */
+}
+
static int amd_pmf_amdtee_ta_match(struct tee_ioctl_version_data *ver, const void *data)
{
return ver->impl_id == TEE_IMPL_ID_AMDTEE;
@@ -435,6 +450,12 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
INIT_DELAYED_WORK(&dev->pb_work, amd_pmf_invoke_cmd);
amd_pmf_set_dram_addr(dev);
amd_pmf_get_bios_buffer(dev);
+
+ /* get amdgpu handle */
+ pci_walk_bus(dev->root->bus, amd_pmf_get_gpu_handle, dev);
+ if (!dev->gfx_data.gpu_dev)
+ dev_err(dev->dev, "GPU handle not found!\n");
+
dev->prev_data = kzalloc(sizeof(*dev->prev_data), GFP_KERNEL);
if (!dev->prev_data)
return -ENOMEM;
@@ -451,5 +472,6 @@ void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev)
kfree(dev->prev_data);
kfree(dev->policy_buf);
cancel_delayed_work_sync(&dev->pb_work);
+ pci_dev_put(dev->gfx_data.gpu_dev);
amd_pmf_tee_deinit(dev);
}
diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
new file mode 100644
index 000000000000..a2d4af231362
--- /dev/null
+++ b/include/linux/amd-pmf-io.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * AMD Platform Management Framework Interface
+ *
+ * Copyright (c) 2023, Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
+ */
+
+#ifndef AMD_PMF_IO_H
+#define AMD_PMF_IO_H
+
+#include <drm/drm_connector.h>
+
+#define MAX_SUPPORTED 4
+
+/* amdgpu */
+struct amd_gpu_pmf_data {
+ struct pci_dev *gpu_dev;
+ enum drm_connector_status con_status[MAX_SUPPORTED];
+ int display_count;
+ int connector_type[MAX_SUPPORTED];
+ int brightness;
+};
+
+int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
+#endif
--
2.25.1
^ permalink raw reply related
* [PATCH 14/15] platform/x86/amd/pmf: Add PMF-AMDSFH interface for HPD
From: Shyam Sundar S K @ 2023-09-22 17:50 UTC (permalink / raw)
To: hdegoede, markgross, basavaraj.natikar, jikos, benjamin.tissoires,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel, Basavaraj Natikar, Shyam Sundar S K
In-Reply-To: <20230922175056.244940-1-Shyam-sundar.S-k@amd.com>
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(-)
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;
+ 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;
+}
+
+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;
}
+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);
+ 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
--
2.25.1
^ permalink raw reply related
* [PATCH 15/15] platform/x86/amd/pmf: Add PMF-AMDSFH interface for ALS
From: Shyam Sundar S K @ 2023-09-22 17:50 UTC (permalink / raw)
To: hdegoede, markgross, basavaraj.natikar, jikos, benjamin.tissoires,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel, Basavaraj Natikar, Shyam Sundar S K
In-Reply-To: <20230922175056.244940-1-Shyam-sundar.S-k@amd.com>
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);
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 */
};
--
2.25.1
^ permalink raw reply related
* [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set interface
From: Shyam Sundar S K @ 2023-09-22 17:50 UTC (permalink / raw)
To: hdegoede, markgross, basavaraj.natikar, jikos, benjamin.tissoires,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel, Shyam Sundar S K
In-Reply-To: <20230922175056.244940-1-Shyam-sundar.S-k@amd.com>
For the Smart PC Solution to fully work, it has to enact to the actions
coming from TA. Add the initial code path for set interface to AMDGPU.
Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 21 +++++++++++++++++++++
drivers/platform/x86/amd/pmf/pmf.h | 2 ++
drivers/platform/x86/amd/pmf/tee-if.c | 19 +++++++++++++++++--
include/linux/amd-pmf-io.h | 1 +
4 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
index 232d11833ddc..5c567bff0548 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
@@ -68,3 +68,24 @@ int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf)
return 0;
}
EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
+
+int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf)
+{
+ struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
+ struct amdgpu_device *adev = drm_to_adev(drm_dev);
+ struct backlight_device *bd;
+
+ if (!(adev->flags & AMD_IS_APU)) {
+ DRM_ERROR("PMF-AMDGPU interface not supported\n");
+ return -ENODEV;
+ }
+
+ bd = backlight_device_get_by_type(BACKLIGHT_RAW);
+ if (!bd)
+ return -ENODEV;
+
+ backlight_device_set_brightness(bd, pmf->brightness);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(amd_pmf_set_gfx_data);
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 9032df4ba48a..ce89cc0daa5a 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -73,6 +73,7 @@
#define PMF_POLICY_STT_SKINTEMP_APU 7
#define PMF_POLICY_STT_SKINTEMP_HS2 8
#define PMF_POLICY_SYSTEM_STATE 9
+#define PMF_POLICY_DISPLAY_BRIGHTNESS 12
#define PMF_POLICY_P3T 38
/* TA macros */
@@ -480,6 +481,7 @@ enum ta_pmf_error_type {
};
struct pmf_action_table {
+ unsigned long display_brightness;
enum system_state system_state;
unsigned long spl; /* in mW */
unsigned long sppt; /* in mW */
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 1608996654e8..eefffff83a4c 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -79,10 +79,10 @@ static int amd_pmf_update_uevents(struct amd_pmf_dev *dev, u16 event)
return 0;
}
-static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
+static int amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
{
u32 val, event = 0;
- int idx;
+ int idx, ret;
for (idx = 0; idx < out->actions_count; idx++) {
val = out->actions_list[idx].value;
@@ -160,8 +160,23 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
dev->prev_data->system_state = 0;
}
break;
+
+ case PMF_POLICY_DISPLAY_BRIGHTNESS:
+ ret = amd_pmf_get_gfx_data(&dev->gfx_data);
+ if (ret)
+ return ret;
+
+ dev->prev_data->display_brightness = dev->gfx_data.brightness;
+ if (dev->prev_data->display_brightness != val) {
+ dev->gfx_data.brightness = val;
+ amd_pmf_set_gfx_data(&dev->gfx_data);
+ dev_dbg(dev->dev, "update DISPLAY_BRIGHTNESS : %d\n", val);
+ }
+ break;
}
}
+
+ return 0;
}
static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
index a2d4af231362..ecae387ddaa6 100644
--- a/include/linux/amd-pmf-io.h
+++ b/include/linux/amd-pmf-io.h
@@ -25,4 +25,5 @@ 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);
#endif
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 04/15] platform/x86/amd/pmf: Add support for PMF Policy Binary
From: Mario Limonciello @ 2023-09-22 18:51 UTC (permalink / raw)
To: Shyam Sundar S K, hdegoede, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <20230922175056.244940-5-Shyam-sundar.S-k@amd.com>
On 9/22/2023 12:50, Shyam Sundar S K wrote:
> PMF Policy binary is a encrypted and signed binary that will be part
> of the BIOS. PMF driver via the ACPI interface checks the existence
> of Smart PC bit. If the advertised bit is found, PMF driver walks
> the acpi namespace to find out the policy binary size and the address
> which has to be passed to the TA during the TA init sequence.
>
> The policy binary is comprised of inputs (or the events) and outputs
> (or the actions). With the PMF ecosystem, OEMs generate the policy
> binary (or could be multiple binaries) that contains a supported set
> of inputs and outputs which could be specifically carved out for each
> usage segment (or for each user also) that could influence the system
> behavior either by enriching the user experience or/and boost/throttle
> power limits.
>
> Once the TA init command succeeds, the PMF driver sends the changing
> events in the current environment to the TA for a constant sampling
> frequency time (the event here could be a lid close or open) and
> if the policy binary has corresponding action built within it, the
> TA sends the action for it in the subsequent enact command.
>
> If the inputs sent to the TA has no output defined in the policy
> binary generated by OEMs, there will be no action to be performed
> by the PMF driver.
>
> Example policies:
>
> 1) if slider is performance ; set the SPL to 40W
> Here PMF driver registers with the platform profile interface and
> when the slider position is changed, PMF driver lets the TA know
> about this. TA sends back an action to update the Sustained
> Power Limit (SPL). PMF driver updates this limit via the PMFW mailbox.
>
> 2) if user_away ; then lock the system
> Here PMF driver hooks to the AMD SFH driver to know the user presence
> and send the inputs to TA and if the condition is met, the TA sends
> the action of locking the system. PMF driver generates a uevent and
> based on the udev rule in the userland the system gets locked with
> systemctl.
>
> The intent here is to provide the OEM's to make a policy to lock the
> system when the user is away ; but the userland can make a choice to
> ignore it.
>
> and so on.
>
> The OEMs will have an utility to create numerous such policies and
> the policies shall be reviewed by AMD before signing and encrypting
> them. Policies are shared between operating systems to have seemless user
> experience.
>
> Since all this action has to happen via the "amdtee" driver, currently
> there is no caller for it in the kernel which can load the amdtee driver.
> Without amdtee driver loading onto the system the "tee" calls shall fail
> from the PMF driver. Hence an explicit "request_module" has been added
> to address this.
>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> drivers/platform/x86/amd/pmf/Kconfig | 1 +
> drivers/platform/x86/amd/pmf/acpi.c | 37 +++++++
> drivers/platform/x86/amd/pmf/core.c | 12 +++
> drivers/platform/x86/amd/pmf/pmf.h | 132 ++++++++++++++++++++++++
> drivers/platform/x86/amd/pmf/tee-if.c | 141 +++++++++++++++++++++++++-
> 5 files changed, 321 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
> index 3064bc8ea167..437b78c6d1c5 100644
> --- a/drivers/platform/x86/amd/pmf/Kconfig
> +++ b/drivers/platform/x86/amd/pmf/Kconfig
> @@ -9,6 +9,7 @@ config AMD_PMF
> depends on POWER_SUPPLY
> depends on AMD_NB
> select ACPI_PLATFORM_PROFILE
> + depends on AMDTEE
> 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/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
> index 3fc5e4547d9f..d0512af2cd42 100644
> --- a/drivers/platform/x86/amd/pmf/acpi.c
> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> @@ -286,6 +286,43 @@ int apmf_install_handler(struct amd_pmf_dev *pmf_dev)
> return 0;
> }
>
> +static acpi_status apmf_walk_resources(struct acpi_resource *res, void *data)
> +{
> + struct amd_pmf_dev *dev = data;
> +
> + switch (res->type) {
> + case ACPI_RESOURCE_TYPE_ADDRESS64:
> + dev->policy_addr = res->data.address64.address.minimum;
> + dev->policy_sz = res->data.address64.address.address_length;
> + break;
> + case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
> + dev->policy_addr = res->data.fixed_memory32.address;
> + dev->policy_sz = res->data.fixed_memory32.address_length;
> + break;
> + }
> +
> + if (!dev->policy_addr || dev->policy_sz > POLICY_BUF_MAX_SZ || dev->policy_sz == 0) {
> + pr_err("Incorrect Policy params, possibly a SBIOS bug\n");
> + return AE_ERROR;
> + }
> +
> + return AE_OK;
> +}
> +
> +int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev)
> +{
> + acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev);
> + acpi_status status;
> +
> + status = acpi_walk_resources(ahandle, METHOD_NAME__CRS, apmf_walk_resources, pmf_dev);
> + if (ACPI_FAILURE(status)) {
> + dev_err(pmf_dev->dev, "acpi_walk_resources failed\n");
> + return status;
> + }
> +
> + return 0;
> +}
> +
> void apmf_acpi_deinit(struct amd_pmf_dev *pmf_dev)
> {
> acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev);
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index 5fb03ed614ff..6f36c43e081e 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -376,6 +376,18 @@ static int amd_pmf_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> dev->dev = &pdev->dev;
> + err = apmf_check_smart_pc(dev);
> + if (!err) {
> + /* in order for Smart PC solution to work it has a hard dependency
> + * on the amdtee driver to be loaded first even before the PMF driver
> + * loads. PMF ASL has a _CRS method that advertises the existence
> + * of Smart PC bit. If this information is present, use this to
> + * explicitly probe the amdtee driver, so that "tee" plumbing is done
> + * before the PMF Smart PC init happens.
> + */
> + if (request_module("amdtee"))
> + pr_err("Failed to load amdtee. PMF Smart PC not enabled!\n");
In this case; would it make sense to fail the driver load entirely?
I tend to think yes.
My argument for failing driver load is that if the platform uses Smart
PC policies, then falling back will basically leave you with at best a
broken static slider and at worst a driver that you can't fix by loading
amdtee manually later on.
> + }
>
> rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
> if (!rdev || !pci_match_id(pmf_pci_ids, rdev)) {
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index ea15ce547d24..81acf2a37366 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -13,6 +13,8 @@
>
> #include <linux/acpi.h>
> #include <linux/platform_profile.h>
> +#define POLICY_BUF_MAX_SZ 0x4b000
> +#define POLICY_SIGN_COOKIE 0x31535024
>
> /* APMF Functions */
> #define APMF_FUNC_VERIFY_INTERFACE 0
> @@ -59,8 +61,20 @@
> #define ARG_NONE 0
> #define AVG_SAMPLE_SIZE 3
>
> +/* Policy Actions */
> +#define PMF_POLICY_SPL 2
> +#define PMF_POLICY_SPPT 3
> +#define PMF_POLICY_FPPT 4
> +#define PMF_POLICY_SPPT_APU_ONLY 5
> +#define PMF_POLICY_STT_MIN 6
> +#define PMF_POLICY_STT_SKINTEMP_APU 7
> +#define PMF_POLICY_STT_SKINTEMP_HS2 8
> +
> /* TA macros */
> #define PMF_TA_IF_VERSION__MAJOR 1
> +#define TA_PMF_ACTION_MAX 32
> +#define TA_PMF_UNDO_MAX 8
> +#define MAX_OPERATION_PARAMS 4
>
> /* AMD PMF BIOS interfaces */
> struct apmf_verify_interface {
> @@ -183,11 +197,16 @@ struct amd_pmf_dev {
> bool cnqf_supported;
> struct notifier_block pwr_src_notifier;
> /* Smart PC solution builder */
> + unsigned char *policy_buf;
> + u32 policy_sz;
> struct tee_context *tee_ctx;
> struct tee_shm *fw_shm_pool;
> u32 session_id;
> void *shbuf;
> struct delayed_work pb_work;
> + struct pmf_action_table *prev_data;
> + u64 policy_addr;
> + void *policy_base;
> bool smart_pc_enabled;
> };
>
> @@ -399,17 +418,129 @@ struct apmf_dyn_slider_output {
> struct apmf_cnqf_power_set ps[APMF_CNQF_MAX];
> } __packed;
>
> +/* Smart PC - TA internals */
> +enum ta_slider {
> + TA_BEST_BATTERY, /* Best Battery */
> + TA_BETTER_BATTERY, /* Better Battery */
> + TA_BETTER_PERFORMANCE, /* Better Performance */
> + TA_BEST_PERFORMANCE, /* Best Performance */
> + TA_MAX,
> +};
> +
> /* cmd ids for TA communication */
> enum ta_pmf_command {
> TA_PMF_COMMAND_POLICY_BUILDER__INITIALIZE,
> TA_PMF_COMMAND_POLICY_BUILDER__ENACT_POLICIES
> };
>
> +enum ta_pmf_error_type {
> + TA_PMF_TYPE__SUCCESS,
> + TA_PMF_ERROR_TYPE__GENERIC,
> + TA_PMF_ERROR_TYPE__CRYPTO,
> + TA_PMF_ERROR_TYPE__CRYPTO_VALIDATE,
> + TA_PMF_ERROR_TYPE__CRYPTO_VERIFY_OEM,
> + TA_PMF_ERROR_TYPE__POLICY_BUILDER,
> + TA_PMF_ERROR_TYPE__PB_CONVERT,
> + TA_PMF_ERROR_TYPE__PB_SETUP,
> + TA_PMF_ERROR_TYPE__PB_ENACT,
> + TA_PMF_ERROR_TYPE__ASD_GET_DEVICE_INFO,
> + TA_PMF_ERROR_TYPE__ASD_GET_DEVICE_PCIE_INFO,
> + TA_PMF_ERROR_TYPE__SYS_DRV_FW_VALIDATION,
> + TA_PMF_ERROR_TYPE__MAX,
> +};
> +
> +struct pmf_action_table {
> + unsigned long spl; /* in mW */
> + unsigned long sppt; /* in mW */
> + unsigned long sppt_apuonly; /* in mW */
> + unsigned long fppt; /* in mW */
> + unsigned long stt_minlimit; /* in mW */
> + unsigned long stt_skintemp_apu; /* in C */
> + unsigned long stt_skintemp_hs2; /* in C */
> +};
> +
> +/* Input conditions */
> +struct ta_pmf_condition_info {
> + u32 power_source;
> + u32 bat_percentage;
> + u32 power_slider;
> + u32 lid_state;
> + bool user_present;
> + u32 rsvd1[2];
> + u32 monitor_count;
> + u32 rsvd2[2];
> + u32 bat_design;
> + u32 full_charge_capacity;
> + int drain_rate;
> + bool user_engaged;
> + u32 device_state;
> + u32 socket_power;
> + u32 skin_temperature;
> + u32 rsvd3[5];
> + u32 ambient_light;
> + u32 length;
> + u32 avg_c0residency;
> + u32 max_c0residency;
> + u32 s0i3_entry;
> + u32 gfx_busy;
> + u32 rsvd4[7];
> + bool camera_state;
> + u32 workload_type;
> + u32 display_type;
> + u32 display_state;
> + u32 rsvd5[150];
> +};
> +
> +struct ta_pmf_load_policy_table {
> + u32 table_size;
> + u8 table[POLICY_BUF_MAX_SZ];
> +};
> +
> +/* TA initialization params */
> +struct ta_pmf_init_table {
> + u32 frequency; /* SMU sampling frequency */
> + bool validate;
> + bool sku_check;
> + bool metadata_macrocheck;
> + struct ta_pmf_load_policy_table policies_table;
> +};
> +
> +/* Everything the TA needs to Enact Policies */
> +struct ta_pmf_enact_table {
> + struct ta_pmf_condition_info ev_info;
> + u32 name;
> +};
> +
> +struct ta_pmf_action {
> + u32 action_index;
> + u32 value;
> +};
> +
> +/* output actions from TA */
> +struct ta_pmf_enact_result {
> + u32 actions_count;
> + struct ta_pmf_action actions_list[TA_PMF_ACTION_MAX];
> + u32 undo_count;
> + struct ta_pmf_action undo_list[TA_PMF_UNDO_MAX];
> +};
> +
> +union ta_pmf_input {
> + struct ta_pmf_enact_table enact_table;
> + struct ta_pmf_init_table init_table;
> +};
> +
> +union ta_pmf_output {
> + struct ta_pmf_enact_result policy_apply_table;
> + u32 rsvd[906];
> +};
> +
> struct ta_pmf_shared_memory {
> int command_id;
> int resp_id;
> u32 pmf_result;
> u32 if_version;
> + union ta_pmf_output pmf_output;
> + union ta_pmf_input pmf_input;
> };
>
> /* Core Layer */
> @@ -460,4 +591,5 @@ extern const struct attribute_group cnqf_feature_attribute_group;
> /* Smart PC builder Layer*/
> int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev);
> void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev);
> +int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev);
> #endif /* PMF_H */
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 1fce04beacb3..a8b05e746efd 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -42,9 +42,77 @@ static void amd_pmf_prepare_args(struct amd_pmf_dev *dev, int cmd,
> param[0].u.memref.shm_offs = 0;
> }
>
> +static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
> +{
> + u32 val;
> + int idx;
> +
> + for (idx = 0; idx < out->actions_count; idx++) {
> + val = out->actions_list[idx].value;
> + switch (out->actions_list[idx].action_index) {
> + case PMF_POLICY_SPL:
> + if (dev->prev_data->spl != val) {
> + amd_pmf_send_cmd(dev, SET_SPL, false, val, NULL);
> + dev_dbg(dev->dev, "update SPL : %d\n", val);
> + dev->prev_data->spl = val;
> + }
> + break;
> +
> + case PMF_POLICY_SPPT:
> + if (dev->prev_data->sppt != val) {
> + amd_pmf_send_cmd(dev, SET_SPPT, false, val, NULL);
> + dev_dbg(dev->dev, "update SPPT : %d\n", val);
> + dev->prev_data->sppt = val;
> + }
> + break;
> +
> + case PMF_POLICY_FPPT:
> + if (dev->prev_data->fppt != val) {
> + amd_pmf_send_cmd(dev, SET_FPPT, false, val, NULL);
> + dev_dbg(dev->dev, "update FPPT : %d\n", val);
> + dev->prev_data->fppt = val;
> + }
> + break;
> +
> + case PMF_POLICY_SPPT_APU_ONLY:
> + if (dev->prev_data->sppt_apuonly != val) {
> + amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false, val, NULL);
> + dev_dbg(dev->dev, "update SPPT_APU_ONLY : %d\n", val);
> + dev->prev_data->sppt_apuonly = val;
> + }
> + break;
> +
> + case PMF_POLICY_STT_MIN:
> + if (dev->prev_data->stt_minlimit != val) {
> + amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false, val, NULL);
> + dev_dbg(dev->dev, "update STT_MIN : %d\n", val);
> + dev->prev_data->stt_minlimit = val;
> + }
> + break;
> +
> + case PMF_POLICY_STT_SKINTEMP_APU:
> + if (dev->prev_data->stt_skintemp_apu != val) {
> + amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false, val, NULL);
> + dev_dbg(dev->dev, "update STT_SKINTEMP_APU : %d\n", val);
> + dev->prev_data->stt_skintemp_apu = val;
> + }
> + break;
> +
> + case PMF_POLICY_STT_SKINTEMP_HS2:
> + if (dev->prev_data->stt_skintemp_hs2 != val) {
> + amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false, val, NULL);
> + dev_dbg(dev->dev, "update STT_SKINTEMP_HS2 : %d\n", val);
> + dev->prev_data->stt_skintemp_hs2 = val;
> + }
> + break;
> + }
> + }
> +}
> +
> static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
> {
> struct ta_pmf_shared_memory *ta_sm = NULL;
> + struct ta_pmf_enact_result *out = NULL;
> struct tee_param param[MAX_TEE_PARAM];
> struct tee_ioctl_invoke_arg arg;
> int ret = 0;
> @@ -52,7 +120,10 @@ static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
> if (!dev->tee_ctx)
> return -ENODEV;
>
> + memset(dev->shbuf, 0, dev->policy_sz);
> ta_sm = (struct ta_pmf_shared_memory *)dev->shbuf;
> + out = &ta_sm->pmf_output.policy_apply_table;
> +
> memset(ta_sm, 0, sizeof(struct ta_pmf_shared_memory));
> ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER__ENACT_POLICIES;
> ta_sm->if_version = PMF_TA_IF_VERSION__MAJOR;
> @@ -65,6 +136,12 @@ static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
> return -EINVAL;
> }
>
> + if (ta_sm->pmf_result == TA_PMF_TYPE__SUCCESS && out->actions_count) {
> + dev_dbg(dev->dev, "action count:%d result:%x\n", out->actions_count,
> + ta_sm->pmf_result);
> + amd_pmf_apply_policies(dev, out);
> + }
> +
> return 0;
> }
>
> @@ -72,6 +149,7 @@ static int amd_pmf_invoke_cmd_init(struct amd_pmf_dev *dev)
> {
> struct ta_pmf_shared_memory *ta_sm = NULL;
> struct tee_param param[MAX_TEE_PARAM];
> + struct ta_pmf_init_table *in = NULL;
> struct tee_ioctl_invoke_arg arg;
> int ret = 0;
>
> @@ -80,10 +158,20 @@ static int amd_pmf_invoke_cmd_init(struct amd_pmf_dev *dev)
> return -ENODEV;
> }
>
> + dev_dbg(dev->dev, "Policy Binary size: %d bytes\n", dev->policy_sz);
> + memset(dev->shbuf, 0, dev->policy_sz);
> ta_sm = (struct ta_pmf_shared_memory *)dev->shbuf;
> + in = &ta_sm->pmf_input.init_table;
> +
> ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER__INITIALIZE;
> ta_sm->if_version = PMF_TA_IF_VERSION__MAJOR;
> + in->metadata_macrocheck = false;
> + in->sku_check = false;
> + in->validate = true;
> + in->frequency = pb_actions_ms;
> + in->policies_table.table_size = dev->policy_sz;
>
> + memcpy(in->policies_table.table, dev->policy_buf, dev->policy_sz);
> amd_pmf_prepare_args(dev, TA_PMF_COMMAND_POLICY_BUILDER__INITIALIZE, &arg, param);
>
> ret = tee_client_invoke_func(dev->tee_ctx, &arg, param);
> @@ -103,6 +191,47 @@ static void amd_pmf_invoke_cmd(struct work_struct *work)
> schedule_delayed_work(&dev->pb_work, msecs_to_jiffies(pb_actions_ms));
> }
>
> +static int amd_pmf_start_policy_engine(struct amd_pmf_dev *dev)
> +{
> + u32 cookie, length;
> + int res;
> +
> + cookie = readl(dev->policy_buf + 0x10);
> + length = readl(dev->policy_buf + 0x14);
> +
> + if (cookie != POLICY_SIGN_COOKIE || !length)
> + return -EINVAL;
> +
> + /* update the actual length */
> + dev->policy_sz = length + 512;
> + res = amd_pmf_invoke_cmd_init(dev);
> + if (res == TA_PMF_TYPE__SUCCESS) {
> + /* now its safe to announce that smart pc is enabled */
> + dev->smart_pc_enabled = 1;
> + schedule_delayed_work(&dev->pb_work, msecs_to_jiffies(pb_actions_ms * 3));
> + } else {
> + dev_err(dev->dev, "%s ta invoke_cmd_init failed err: %x\n", __func__, res);
> + return res;
> + }
> +
> + return 0;
> +}
> +
> +static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
> +{
> + dev->policy_buf = kzalloc(dev->policy_sz, GFP_KERNEL);
> + if (!dev->policy_buf)
> + return -ENOMEM;
> +
> + dev->policy_base = ioremap(dev->policy_addr, dev->policy_sz);
> + if (!dev->policy_base)
> + return -ENOMEM;
> +
> + memcpy(dev->policy_buf, dev->policy_base, dev->policy_sz);
> +
> + return amd_pmf_start_policy_engine(dev);
> +}
> +
> static int amd_pmf_amdtee_ta_match(struct tee_ioctl_version_data *ver, const void *data)
> {
> return ver->impl_id == TEE_IMPL_ID_AMDTEE;
> @@ -149,7 +278,7 @@ static int amd_pmf_tee_init(struct amd_pmf_dev *dev)
> goto out_ctx;
> }
>
> - size = sizeof(struct ta_pmf_shared_memory);
> + size = sizeof(struct ta_pmf_shared_memory) + dev->policy_sz;
> dev->fw_shm_pool = tee_shm_alloc_kernel_buf(dev->tee_ctx, size);
> if (IS_ERR(dev->fw_shm_pool)) {
> dev_err(dev->dev, "%s: tee_shm_alloc_kernel_buf failed\n", __func__);
> @@ -191,11 +320,19 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
> return ret;
>
> INIT_DELAYED_WORK(&dev->pb_work, amd_pmf_invoke_cmd);
> - return 0;
> + amd_pmf_set_dram_addr(dev);
> + amd_pmf_get_bios_buffer(dev);
> + dev->prev_data = kzalloc(sizeof(*dev->prev_data), GFP_KERNEL);
> + if (!dev->prev_data)
> + return -ENOMEM;
> +
> + return dev->smart_pc_enabled;
> }
>
> void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev)
> {
> + kfree(dev->prev_data);
> + kfree(dev->policy_buf);
> cancel_delayed_work_sync(&dev->pb_work);
> amd_pmf_tee_deinit(dev);
> }
^ permalink raw reply
* Re: [PATCH 11/15] platform/x86/amd/pmf: dump policy binary data
From: Mario Limonciello @ 2023-09-22 19:01 UTC (permalink / raw)
To: Shyam Sundar S K, hdegoede, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <20230922175056.244940-12-Shyam-sundar.S-k@amd.com>
On 9/22/2023 12:50, Shyam Sundar S K wrote:
> Sometimes policy binary retrieved from the BIOS maybe incorrect that can
> end up in failing to enable the Smart PC solution feature.
>
> Use print_hex_dump_debug() to dump the policy binary in hex, so that we
> debug the issues related to the binary even before sending that to TA.
>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> drivers/platform/x86/amd/pmf/tee-if.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index fa37cfab2dc7..3daa122f35d5 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -290,6 +290,9 @@ static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
> if (copy_from_user(dev->policy_buf, buf, dev->policy_sz))
> return -EFAULT;
>
> + print_hex_dump_debug("(pb): ", DUMP_PREFIX_OFFSET, 16, 1, dev->policy_buf,
> + dev->policy_sz, false);
> +
Should this one also be guarded by CONFIG_AMD_PMF_DEBUG or no?
> ret = amd_pmf_start_policy_engine(dev);
> if (ret)
> return -EINVAL;
> @@ -329,6 +332,10 @@ static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
> return -ENOMEM;
>
> memcpy(dev->policy_buf, dev->policy_base, dev->policy_sz);
> +#ifdef CONFIG_AMD_PMF_DEBUG
> + print_hex_dump_debug("(pb): ", DUMP_PREFIX_OFFSET, 16, 1, dev->policy_buf,
> + dev->policy_sz, false);
> +#endif
>
> #ifdef CONFIG_AMD_PMF_DEBUG
> if (pb_side_load)
It looks like you can combine the two #ifdef from the previous patches.
^ permalink raw reply
* Re: [PATCH 14/15] platform/x86/amd/pmf: Add PMF-AMDSFH interface for HPD
From: Mario Limonciello @ 2023-09-22 19:04 UTC (permalink / raw)
To: Shyam Sundar S K, hdegoede, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <20230922175056.244940-15-Shyam-sundar.S-k@amd.com>
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(-)
>
> 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.
> +}
> +
> +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
^ permalink raw reply
* Re: [PATCH 15/15] platform/x86/amd/pmf: Add PMF-AMDSFH interface for ALS
From: Mario Limonciello @ 2023-09-22 19:06 UTC (permalink / raw)
To: Shyam Sundar S K, hdegoede, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <20230922175056.244940-16-Shyam-sundar.S-k@amd.com>
On 9/22/2023 12:50, 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;
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 */
> };
^ permalink raw reply
* Re: [RFC PATCH] of: device: Support 2nd sources of probeable but undiscoverable devices
From: Rob Herring @ 2023-09-22 19:08 UTC (permalink / raw)
To: Doug Anderson
Cc: Krzysztof Kozlowski, Conor Dooley, devicetree, Benjamin Tissoires,
Chen-Yu Tsai, linux-input, Jiri Kosina, Hsin-Yi Wang, linux-gpio,
linus.walleij, Dmitry Torokhov, Johan Hovold, andriy.shevchenko,
broonie, frowand.list, gregkh, hdegoede, james.clark, james,
keescook, linux-kernel, rafael, tglx
In-Reply-To: <CAD=FV=WXxGhX0Fw2nSS7PxYb1O-LUewAhoUVPn=2EpbSD2OeHQ@mail.gmail.com>
On Fri, Sep 22, 2023 at 12:40 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Fri, Sep 22, 2023 at 7:14 AM Rob Herring <robh+dt@kernel.org> wrote:
> >
> > > Let's attempt to do something better. Specifically, we'll allow
> > > tagging nodes in the device tree as mutually exclusive from one
> > > another. This says that only one of the components in this group is
> > > present on any given board. To make it concrete, in my proposal this
> > > looks like:
> > >
> > > / {
> > > tp_ex_group: trackpad-exclusion-group {
> > > };
> >
> > Interesting way to just get a unique identifier. But it could be any
> > phandle not used by another group. So just point all the devices in a
> > group to one of the devices in the group.
>
> Fair enough.
>
>
> > > &i2c_bus {
> > > tp1: trackpad@10 {
> > > ...
> > > mutual-exclusion-group = <&tp_ex_group>;
> > > };
> > > tp2: trackpad@20 {
> > > ...
> > > mutual-exclusion-group = <&tp_ex_group>;
> > > };
> > > tp3: trackpad@30 {
> > > ...
> > > mutual-exclusion-group = <&tp_ex_group>;
> > > };
> > > };
> > >
> > > In Linux, we can make things work by simply only probing one of the
> > > devices in the group at a time. We can make a mutex per group and
> > > enforce locking that mutex around probe. If the first device that gets
> > > the mutex fails to probe then it won't try again. If it succeeds then
> > > it will acquire the shared resources and future devices (which we know
> > > can't be present) will fail to get the shared resources. Future
> > > patches could quiet down errors about failing to acquire shared
> > > resources or failing to probe if a device is in a
> > > mutual-exclusion-group.
> >
> > This seems like overkill to me. Do we really need groups and a mutex
> > for each group? Worst case is what? 2-3 groups of 2-3 devices?
> > Instead, what about extending "status" with another value
> > ("fail-needs-probe"? (fail-xxx is a documented value)). Currently, the
> > kernel would just ignore nodes with that status. Then we can process
> > those nodes separately 1-by-1.
>
> My worry here is that this has the potential to impact boot speed in a
> non-trivial way. While trackpads and touchscreens _are_ probable,
> their probe routines are often quite slow. This is even mentioned in
> Dmitry's initial patches adding async probe to the kernel. See commit
> 765230b5f084 ("driver-core: add asynchronous probing support for
> drivers") where he specifically brings up input devices as examples.
Perhaps then this should be solved in userspace where it can learn
which device is actually present and save that information for
subsequent boots.
> It wouldn't be absurd to have a system that has multiple sources for
> both the trackpad and the touchscreen. If we have to probe each of
> these one at a time then it could be slow. It would be quicker to be
> able to probe the trackpads (one at a time) at the same time we're
> probing the touchscreens (one at a time). Using the "fail-needs-probe"
> doesn't provide information needed to know which devices conflict with
> each other.
I would guess most of the time that's pretty evident. They are going
to be on the same bus/link. If unrelated devices are on the same bus,
then that's going to get serialized anyways (if bus accesses are what
make things slow).
We could add information on the class of device. touchscreen and
touchpad aliases or something.
> IMO this is still better than nothing, but it worries me
> to pick the less-expressive solution for the dts which means that the
> information simply isn't there and the OS can't be made better later.
>
> Thinking about this more, I guess even my proposed solution isn't
> ideal for probe speed. Let's imagine that we had:
>
> &i2c_bus {
> tp1: trackpad@10 {
> compatible = "hid-over-i2c";
> reg = <0x10>;
> post-power-on-delay-ms = <200>;
> ...
> mutual-exclusion-group = <&tp1>;
> };
> tp2: trackpad@20 {
> compatible = "hid-over-i2c";
> reg = <0x20>;
> post-power-on-delay-ms = <200>;
> ...
> mutual-exclusion-group = <&tp1>;
> };
> };
>
> With my solution, we'd power the first device up, wait 200 ms, then
> check to see if anything acks an i2c xfer at address 0x10. If it
> didn't, we'd power down. Then we'd power up the second device
> (presumably the same power rail), wait 200 ms, and check to see if
> anything acks an i2c xfer at 0x20. It would have been better to just
> power up once, wait 200 ms, then check for a device at either 0x10 or
> 0x20.
>
> I guess with more complex touchscreens this could be more important. I
> don't know if we need to try to solve it at this point, but I guess I
> could imagine a case where we truly need to take into account all
> possible devices (maybe taking the maximum of delays?) to ensure we
> don't violate power sequencing requirements for any of them while
> probing.
>
> That would lead me to suggest this:
>
> &i2c_bus {
> trackpad-prober {
> compatible = "mt8173-elm-hana-trackpad-prober";
>
> tp1: trackpad@10 {
> compatible = "hid-over-i2c";
> reg = <0x10>;
> ...
> post-power-on-delay-ms = <200>;
> };
> tp2: trackpad@20 {
> compatible = "hid-over-i2c";
> reg = <0x20>;
> ...
> post-power-on-delay-ms = <200>;
> };
> };
> };
>
> ...but I suspect that would be insta-NAKed because it's creating a
> completely virtual device ("mt8173-elm-hana-trackpad-prober") in the
> device tree. I don't know if there's something that's functionally
> similar that would be OK?
Why do you need the intermediate node other than a convenient way to
instantiate a driver? You just need a flag in each node which needs
this special handling. Again, "status" could work well here since it
keeps the normal probe from happening. But I'm not saying you can't
have some board specific code. Sometimes you just need code to deal
with this stuff. Don't try to parameterize everything to DT
properties.
Note that the above only works with "generic" compatibles with
"generic" power sequencing properties (I won't repeat my dislike
again). If only the driver knows how to handle the device, then you
still just have to have the driver probe. If you *only* wanted to
solve the above case, I'd just make "hid-over-i2c" take a 2nd (and
3rd) I2C address in reg and have those as fallbacks.
You could always make the driver probe smarter where if your supply
was already powered on, then don't delay. Then something else could
ensure that the supply is enabled. I'm not sure if regulators have the
same issue as clocks where the clock might be on from the bootloader,
then a failed probe which gets then puts the clock turns it off.
Rob
^ permalink raw reply
* Re: [RFC PATCH] of: device: Support 2nd sources of probeable but undiscoverable devices
From: Mark Brown @ 2023-09-22 19:28 UTC (permalink / raw)
To: Rob Herring
Cc: Doug Anderson, Krzysztof Kozlowski, Conor Dooley, devicetree,
Benjamin Tissoires, Chen-Yu Tsai, linux-input, Jiri Kosina,
Hsin-Yi Wang, linux-gpio, linus.walleij, Dmitry Torokhov,
Johan Hovold, andriy.shevchenko, frowand.list, gregkh, hdegoede,
james.clark, james, keescook, linux-kernel, rafael, tglx
In-Reply-To: <CAL_JsqKJyRJmwJzB1yew71Ld7BeMMat+rzhX9XtDtiFE8Dbvcw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 431 bytes --]
On Fri, Sep 22, 2023 at 02:08:08PM -0500, Rob Herring wrote:
> You could always make the driver probe smarter where if your supply
> was already powered on, then don't delay. Then something else could
> ensure that the supply is enabled. I'm not sure if regulators have the
> same issue as clocks where the clock might be on from the bootloader,
> then a failed probe which gets then puts the clock turns it off.
That'll happen.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 14/15] platform/x86/amd/pmf: Add PMF-AMDSFH interface for HPD
From: Mario Limonciello @ 2023-09-22 21:04 UTC (permalink / raw)
To: Shyam Sundar S K, hdegoede, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, platform-driver-x86, linux-input, amd-gfx, dri-devel
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
>
^ permalink raw reply
* Re: [RFC PATCH] of: device: Support 2nd sources of probeable but undiscoverable devices
From: Doug Anderson @ 2023-09-23 0:11 UTC (permalink / raw)
To: Rob Herring
Cc: Krzysztof Kozlowski, Conor Dooley, devicetree, Benjamin Tissoires,
Chen-Yu Tsai, linux-input, Jiri Kosina, Hsin-Yi Wang, linux-gpio,
linus.walleij, Dmitry Torokhov, Johan Hovold, andriy.shevchenko,
broonie, frowand.list, gregkh, hdegoede, james.clark, james,
keescook, linux-kernel, rafael, tglx
In-Reply-To: <CAL_JsqKJyRJmwJzB1yew71Ld7BeMMat+rzhX9XtDtiFE8Dbvcw@mail.gmail.com>
Hi,
On Fri, Sep 22, 2023 at 12:08 PM Rob Herring <robh+dt@kernel.org> wrote:
>
> > > This seems like overkill to me. Do we really need groups and a mutex
> > > for each group? Worst case is what? 2-3 groups of 2-3 devices?
> > > Instead, what about extending "status" with another value
> > > ("fail-needs-probe"? (fail-xxx is a documented value)). Currently, the
> > > kernel would just ignore nodes with that status. Then we can process
> > > those nodes separately 1-by-1.
> >
> > My worry here is that this has the potential to impact boot speed in a
> > non-trivial way. While trackpads and touchscreens _are_ probable,
> > their probe routines are often quite slow. This is even mentioned in
> > Dmitry's initial patches adding async probe to the kernel. See commit
> > 765230b5f084 ("driver-core: add asynchronous probing support for
> > drivers") where he specifically brings up input devices as examples.
>
> Perhaps then this should be solved in userspace where it can learn
> which device is actually present and save that information for
> subsequent boots.
Yeah, the thought occurred to me as well. I think there are a few
problems, though:
a) Userspace can't itself probe these devices effectively. While
userspace could turn on GPIOs manually and query the i2c bus manually,
it can't (I believe) turn on regulators nor can it turn on clocks, if
they are needed. About the best userspace could do would be to blindly
try binding an existing kernel driver, and in that case why did we
need userspace involved anyway?
b) While deferring to userspace can work for solutions like ChromeOS
or Android where it's easy to ensure the userspace bits are there,
it's less appealing as a general solution. I think in Johan's case
he's taking a laptop that initially ran Windows and then is trying to
run a generic Linux distro on it. For anyone in a similar situation,
they'd either need to pick a Linux distro that has the magic userspace
bits that are needed or they need to know that, on their laptop, they
need to manually install some software. While requiring special
userspace might make sense if you've got a special peripheral, like an
LTE modem, it makes less sense to need special userspace just to get
the right devices bound...
> > It wouldn't be absurd to have a system that has multiple sources for
> > both the trackpad and the touchscreen. If we have to probe each of
> > these one at a time then it could be slow. It would be quicker to be
> > able to probe the trackpads (one at a time) at the same time we're
> > probing the touchscreens (one at a time). Using the "fail-needs-probe"
> > doesn't provide information needed to know which devices conflict with
> > each other.
>
> I would guess most of the time that's pretty evident. They are going
> to be on the same bus/link. If unrelated devices are on the same bus,
> then that's going to get serialized anyways (if bus accesses are what
> make things slow).
>
> We could add information on the class of device. touchscreen and
> touchpad aliases or something.
Ah, I see. So something like "fail-needs-probe-<class>". The
touchscreens could have "fail-needs-probe-touchscreen" and the
trackpads could have "fail-needs-probe-trackpad" ? That could work. In
theory that could fall back to the same solution of grabbing a mutex
based on the group ID...
Also: if having the mutex in the "struct device" is seen as a bad
idea, it would also be easy to remove. __driver_probe_device() could
just make a call like "of_device_probe_start()" at the beginning that
locks the mutex and then "of_device_probe_end()" that unlocks it. Both
of those calls could easily lookup the mutex in a list, which would
get rid of the need to store it in the "struct device".
> > That would lead me to suggest this:
> >
> > &i2c_bus {
> > trackpad-prober {
> > compatible = "mt8173-elm-hana-trackpad-prober";
> >
> > tp1: trackpad@10 {
> > compatible = "hid-over-i2c";
> > reg = <0x10>;
> > ...
> > post-power-on-delay-ms = <200>;
> > };
> > tp2: trackpad@20 {
> > compatible = "hid-over-i2c";
> > reg = <0x20>;
> > ...
> > post-power-on-delay-ms = <200>;
> > };
> > };
> > };
> >
> > ...but I suspect that would be insta-NAKed because it's creating a
> > completely virtual device ("mt8173-elm-hana-trackpad-prober") in the
> > device tree. I don't know if there's something that's functionally
> > similar that would be OK?
>
> Why do you need the intermediate node other than a convenient way to
> instantiate a driver? You just need a flag in each node which needs
> this special handling. Again, "status" could work well here since it
> keeps the normal probe from happening. But I'm not saying you can't
> have some board specific code. Sometimes you just need code to deal
> with this stuff. Don't try to parameterize everything to DT
> properties.
I think I'd have an easier time understanding if I knew where you
envisioned the board-specific code living. Do you have an example of
board specific code running at boot time in the kernel on DT systems?
> Note that the above only works with "generic" compatibles with
> "generic" power sequencing properties (I won't repeat my dislike
> again).
I don't think so? I was imagining that we'd have some board specific
code that ran that knew all the possible combinations of devices,
could probe them, and then could instantiate the correct driver.
Imagine that instead of the hated "hid-over-i2c" compatible we were
using two other devices. Imagine that a given board could have a
"elan,ekth6915" and a "goodix,gt7375p". Both of these devices have
specific timing requirements on how to sequence their supplies and
reset GPIOs. For Elan we power on the supplies, wait at least 1 ms,
deassert reset, wait at least 300 ms, and then can talk i2c. For
Goodix we power on the supply, wait at least 10 ms, deassert reset,
wait at least 180 ms, and then can talk i2c. If we had a
board-specific probing driver then it would power on the supplies,
wait at least 10 ms (the max of the two), deassert reset, wait at
least 300 ms (the max of the two), and then see which device talked.
Then it would instantiate whichever of the two drivers. This could be
done for any two devices that EEs have determined have "compatible"
probing sequences.
Ideally in the above situation we'd be able to avoid turning the
device off and on again between the board-specific probe code and the
normal driver. That optimization might need special code per-driver
but it feels doable by passing some sort of hint to the child driver
when it's instantiated.
> If only the driver knows how to handle the device, then you
> still just have to have the driver probe. If you *only* wanted to
> solve the above case, I'd just make "hid-over-i2c" take a 2nd (and
> 3rd) I2C address in reg and have those as fallbacks.
Yeah, it did occur to me that having "hid-over-i2c" take more than one
register (and I guess more than one "hid-descr-addr") would work in my
earlier example and this might actually be a good solution for Johan.
I'm hoping for a better generic solution, though.
> You could always make the driver probe smarter where if your supply
> was already powered on, then don't delay. Then something else could
> ensure that the supply is enabled. I'm not sure if regulators have the
> same issue as clocks where the clock might be on from the bootloader,
> then a failed probe which gets then puts the clock turns it off.
I'm not sure it's that simple. Even if the supply didn't turn off by
itself in some cases, we wouldn't know how long the supply was on.
-Doug
^ permalink raw reply
* Re: [PATCH] input: Annotate struct evdev_client with __counted_by
From: Gustavo A. R. Silva @ 2023-09-23 16:29 UTC (permalink / raw)
To: Kees Cook, Dmitry Torokhov
Cc: linux-input, Nathan Chancellor, Nick Desaulniers, Tom Rix,
linux-kernel, llvm, linux-hardening
In-Reply-To: <20230922175027.work.563-kees@kernel.org>
On 9/22/23 11:50, Kees Cook wrote:
> Prepare for the coming implementation by GCC and Clang of the __counted_by
> attribute. Flexible array members annotated with __counted_by can have
> their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
> (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
> functions).
>
> As found with Coccinelle[1], add __counted_by for struct evdev_client.
>
> [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Thanks
--
Gustavo
> ---
> drivers/input/evdev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index 95f90699d2b1..51e0c4954600 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -50,7 +50,7 @@ struct evdev_client {
> bool revoked;
> unsigned long *evmasks[EV_CNT];
> unsigned int bufsize;
> - struct input_event buffer[];
> + struct input_event buffer[] __counted_by(bufsize);
> };
>
> static size_t evdev_get_mask_cnt(unsigned int type)
^ permalink raw reply
* Re: [PATCH] Input: Annotate struct input_leds with __counted_by
From: Gustavo A. R. Silva @ 2023-09-23 16:29 UTC (permalink / raw)
To: Kees Cook, Dmitry Torokhov
Cc: linux-input, Nathan Chancellor, Nick Desaulniers, Tom Rix,
linux-kernel, llvm, linux-hardening
In-Reply-To: <20230922175031.work.467-kees@kernel.org>
On 9/22/23 11:50, Kees Cook wrote:
> Prepare for the coming implementation by GCC and Clang of the __counted_by
> attribute. Flexible array members annotated with __counted_by can have
> their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
> (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
> functions).
>
> As found with Coccinelle[1], add __counted_by for struct input_leds.
>
> [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Thanks
--
Gustavo
> ---
> drivers/input/input-leds.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
> index 0b11990ade46..0e935914bc3a 100644
> --- a/drivers/input/input-leds.c
> +++ b/drivers/input/input-leds.c
> @@ -44,7 +44,7 @@ struct input_led {
> struct input_leds {
> struct input_handle handle;
> unsigned int num_leds;
> - struct input_led leds[];
> + struct input_led leds[] __counted_by(num_leds);
> };
>
> static enum led_brightness input_leds_brightness_get(struct led_classdev *cdev)
^ permalink raw reply
* Re: [PATCH] input: mt: Annotate struct input_mt with __counted_by
From: Gustavo A. R. Silva @ 2023-09-23 16:29 UTC (permalink / raw)
To: Kees Cook, Dmitry Torokhov
Cc: linux-input, Nathan Chancellor, Nick Desaulniers, Tom Rix,
linux-kernel, llvm, linux-hardening
In-Reply-To: <20230922175036.work.762-kees@kernel.org>
On 9/22/23 11:50, Kees Cook wrote:
> Prepare for the coming implementation by GCC and Clang of the __counted_by
> attribute. Flexible array members annotated with __counted_by can have
> their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
> (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
> functions).
>
> As found with Coccinelle[1], add __counted_by for struct input_mt.
>
> [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Thanks
--
Gustavo
> ---
> include/linux/input/mt.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/input/mt.h b/include/linux/input/mt.h
> index 3b8580bd33c1..2cf89a538b18 100644
> --- a/include/linux/input/mt.h
> +++ b/include/linux/input/mt.h
> @@ -47,7 +47,7 @@ struct input_mt {
> unsigned int flags;
> unsigned int frame;
> int *red;
> - struct input_mt_slot slots[];
> + struct input_mt_slot slots[] __counted_by(num_slots);
> };
>
> static inline void input_mt_set_value(struct input_mt_slot *slot,
^ permalink raw reply
* Re: [PATCH 00/49] iio: Convert to platform remove callback returning void
From: Jonathan Cameron @ 2023-09-23 17:35 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jiri Kosina, Srinivas Pandruvada, Lars-Peter Clausen, linux-input,
linux-iio, kernel, Linus Walleij, linux-arm-kernel, Eugen Hristev,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Jinjie Ruan,
Rob Herring, Heiko Stuebner, Yang Yingliang, Chen-Yu Tsai,
Aidan MacDonald, Andy Shevchenko, Ray Jui, Scott Branden,
Broadcom internal kernel review list, Hartley Sweeten,
Alexander Sverdlin, Krzysztof Kozlowski, Alim Akhtar,
linux-samsung-soc, Shawn Guo, Sascha Hauer, Fabio Estevam,
NXP Linux Team, Andreas Klinger, Cai Huoqing, Haibo Chen,
Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
George Stark, Andy Shevchenko, Nuno Sá, linux-amlogic,
Saravanan Sekar, Jiakai Luo, Dongliang Mu, Avi Fishman,
Tomer Maimon, Tali Perry, Patrick Venture, Nancy Yuen,
Benjamin Fair, openbmc, Andy Gross, Bjorn Andersson,
Konrad Dybcio, linux-arm-msm, Marek Vasut, Maxime Coquelin,
Alexandre Torgue, Olivier Moysan, Fabrice Gasnier, Zhang Shurong,
Yangtao Li, linux-stm32, Sean Nyekjaer, Tom Rix, Jernej Skrabec,
Samuel Holland, Rafael J. Wysocki, Damien Le Moal, Mark Brown,
Ido Schimmel, Daniel Lezcano, linux-sunxi, Dmitry Torokhov,
Andreas Kemnade, Peter Rosin, Vladimir Zapolskiy, Kevin Tsai,
Benson Leung, Guenter Roeck, chrome-platform
In-Reply-To: <20230919174931.1417681-1-u.kleine-koenig@pengutronix.de>
On Tue, 19 Sep 2023 19:48:42 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> this series converts all platform drivers below drivers/iio to use
> .remove_new(). The motivation is to get rid of an integer return code
> that is (mostly) ignored by the platform driver core and error prone on
> the driver side. As all platform drivers return zero unconditionally in their
> remove callback up to now, the conversions are "trivial".
>
> See commit 5c5a7680e67b ("platform: Provide a remove callback that
> returns no value") for an extended explanation and the eventual goal.
>
> There are no interdependencies between the patches. As there are still
> quite a few drivers to convert, I'm happy about every patch that makes
> it in. So even if there is a merge conflict with one patch until you
> apply or I picked a wrong subject prefix, please apply the remainder of
> this series anyhow.
>
Series applied to the togreg branch of iio.git and pushed out as testing
to see if 0-day finds anything we are missing.
I've picked up all tags give as of early today.
Thanks,
Jonathan
> Best regards
> Uwe
>
> Uwe Kleine-König (49):
> iio: accel: hid-sensor-accel-3d: Convert to platform remove callback
> returning void
> iio: adc: ab8500-gpadc: Convert to platform remove callback returning
> void
> iio: adc: at91-sama5d2: Convert to platform remove callback returning
> void
> iio: adc: at91: Convert to platform remove callback returning void
> iio: adc: axp20x: Convert to platform remove callback returning void
> iio: adc: bcm_iproc: Convert to platform remove callback returning
> void
> iio: adc: dln2: Convert to platform remove callback returning void
> iio: adc: ep93xx: Convert to platform remove callback returning void
> iio: adc: exynos: Convert to platform remove callback returning void
> iio: adc: fsl-imx25-gcq: Convert to platform remove callback returning
> void
> iio: adc: hx711: Convert to platform remove callback returning void
> iio: adc: imx8qxp: Convert to platform remove callback returning void
> iio: adc: imx93: Convert to platform remove callback returning void
> iio: adc: meson_saradc: Convert to platform remove callback returning
> void
> iio: adc: mp2629: Convert to platform remove callback returning void
> iio: adc: mxs-lradc: Convert to platform remove callback returning
> void
> iio: adc: npcm: Convert to platform remove callback returning void
> iio: adc: qcom-pm8xxx-xoadc: Convert to platform remove callback
> returning void
> iio: adc: rcar-gyroadc: Convert to platform remove callback returning
> void
> iio: adc: stm32-adc-core: Convert to platform remove callback
> returning void
> iio: adc: stm32-adc: Convert to platform remove callback returning
> void
> iio: adc: stm32-dfsdm-adc: Convert to platform remove callback
> returning void
> iio: adc: stm32-dfsdm-core: Convert to platform remove callback
> returning void
> iio: adc: sun4i-gpadc-iio: Convert to platform remove callback
> returning void
> iio: adc: ti_am335x_adc: Convert to platform remove callback returning
> void
> iio: adc: twl4030-madc: Convert to platform remove callback returning
> void
> iio: adc: twl6030-gpadc: Convert to platform remove callback returning
> void
> iio: adc: vf610_adc: Convert to platform remove callback returning
> void
> iio: dac: dpot-dac: Convert to platform remove callback returning void
> iio: dac: lpc18xx_dac: Convert to platform remove callback returning
> void
> iio: dac: stm32-dac-core: Convert to platform remove callback
> returning void
> iio: dac: stm32-dac: Convert to platform remove callback returning
> void
> iio: dac: vf610: Convert to platform remove callback returning void
> iio: gyro: hid-sensor-gyro-3d: Convert to platform remove callback
> returning void
> iio: humidity: hid-sensor-humidity: Convert to platform remove
> callback returning void
> iio: light: cm3605: Convert to platform remove callback returning void
> iio: light: hid-sensor-als: Convert to platform remove callback
> returning void
> iio: light: hid-sensor-prox: Convert to platform remove callback
> returning void
> iio: light: lm3533-als: Convert to platform remove callback returning
> void
> iio: magnetometer: hid-sensor-magn-3d: Convert to platform remove
> callback returning void
> iio: orientation: hid-sensor-incl-3d: Convert to platform remove
> callback returning void
> iio: orientation: hid-sensor-rotation: Convert to platform remove
> callback returning void
> iio: position: hid-sensor-custom-intel-hinge: Convert to platform
> remove callback returning void
> iio: pressure: hid-sensor: Convert to platform remove callback
> returning void
> iio: proximity: cros_ec_mkbp: Convert to platform remove callback
> returning void
> iio: proximity: srf04: Convert to platform remove callback returning
> void
> iio: temperature: hid-sensor: Convert to platform remove callback
> returning void
> iio: trigger: iio-trig-interrupt: Convert to platform remove callback
> returning void
> iio: trigger: stm32-timer: Convert to platform remove callback
> returning void
>
> drivers/iio/accel/hid-sensor-accel-3d.c | 6 ++----
> drivers/iio/adc/ab8500-gpadc.c | 6 ++----
> drivers/iio/adc/at91-sama5d2_adc.c | 6 ++----
> drivers/iio/adc/at91_adc.c | 6 ++----
> drivers/iio/adc/axp20x_adc.c | 6 ++----
> drivers/iio/adc/bcm_iproc_adc.c | 6 ++----
> drivers/iio/adc/dln2-adc.c | 5 ++---
> drivers/iio/adc/ep93xx_adc.c | 6 ++----
> drivers/iio/adc/exynos_adc.c | 6 ++----
> drivers/iio/adc/fsl-imx25-gcq.c | 6 ++----
> drivers/iio/adc/hx711.c | 6 ++----
> drivers/iio/adc/imx8qxp-adc.c | 6 ++----
> drivers/iio/adc/imx93_adc.c | 6 ++----
> drivers/iio/adc/meson_saradc.c | 6 ++----
> drivers/iio/adc/mp2629_adc.c | 6 ++----
> drivers/iio/adc/mxs-lradc-adc.c | 6 ++----
> drivers/iio/adc/npcm_adc.c | 6 ++----
> drivers/iio/adc/qcom-pm8xxx-xoadc.c | 6 ++----
> drivers/iio/adc/rcar-gyroadc.c | 6 ++----
> drivers/iio/adc/stm32-adc-core.c | 6 ++----
> drivers/iio/adc/stm32-adc.c | 6 ++----
> drivers/iio/adc/stm32-dfsdm-adc.c | 6 ++----
> drivers/iio/adc/stm32-dfsdm-core.c | 6 ++----
> drivers/iio/adc/sun4i-gpadc-iio.c | 8 +++-----
> drivers/iio/adc/ti_am335x_adc.c | 6 ++----
> drivers/iio/adc/twl4030-madc.c | 6 ++----
> drivers/iio/adc/twl6030-gpadc.c | 6 ++----
> drivers/iio/adc/vf610_adc.c | 6 ++----
> drivers/iio/dac/dpot-dac.c | 6 ++----
> drivers/iio/dac/lpc18xx_dac.c | 6 ++----
> drivers/iio/dac/stm32-dac-core.c | 6 ++----
> drivers/iio/dac/stm32-dac.c | 6 ++----
> drivers/iio/dac/vf610_dac.c | 6 ++----
> drivers/iio/gyro/hid-sensor-gyro-3d.c | 6 ++----
> drivers/iio/humidity/hid-sensor-humidity.c | 6 ++----
> drivers/iio/light/cm3605.c | 6 ++----
> drivers/iio/light/hid-sensor-als.c | 6 ++----
> drivers/iio/light/hid-sensor-prox.c | 6 ++----
> drivers/iio/light/lm3533-als.c | 6 ++----
> drivers/iio/magnetometer/hid-sensor-magn-3d.c | 6 ++----
> drivers/iio/orientation/hid-sensor-incl-3d.c | 6 ++----
> drivers/iio/orientation/hid-sensor-rotation.c | 6 ++----
> drivers/iio/position/hid-sensor-custom-intel-hinge.c | 6 ++----
> drivers/iio/pressure/hid-sensor-press.c | 6 ++----
> drivers/iio/proximity/cros_ec_mkbp_proximity.c | 6 ++----
> drivers/iio/proximity/srf04.c | 6 ++----
> drivers/iio/temperature/hid-sensor-temperature.c | 6 ++----
> drivers/iio/trigger/iio-trig-interrupt.c | 6 ++----
> drivers/iio/trigger/stm32-timer-trigger.c | 6 ++----
> 49 files changed, 99 insertions(+), 196 deletions(-)
>
>
> base-commit: 29e400e3ea486bf942b214769fc9778098114113
^ permalink raw reply
* Re: [RESEND PATCH v6 1/3] input: pm8xxx-vib: refactor to easily support new SPMI vibrator
From: Dmitry Baryshkov @ 2023-09-23 19:05 UTC (permalink / raw)
To: Fenglin Wu
Cc: linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt, robh+dt,
agross, andersson, Konrad Dybcio, Dmitry Torokhov, linux-input,
quic_collinsd, quic_subbaram, quic_kamalw, jestar
In-Reply-To: <20230922083801.3056724-2-quic_fenglinw@quicinc.com>
On Fri, 22 Sept 2023 at 11:38, Fenglin Wu <quic_fenglinw@quicinc.com> wrote:
>
> Currently, all vibrator control register addresses are hard coded,
> including the base address and the offset, it's not flexible to support
> new SPMI vibrator module which is usually included in different PMICs
> with different base address. Refactor this by defining register offset
> with HW type combination, and register base address which is defined
> in 'reg' property is added for SPMI vibrators.
>
> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
> ---
> drivers/input/misc/pm8xxx-vibrator.c | 122 ++++++++++++++++-----------
> 1 file changed, 73 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
> index 04cb87efd799..d6b468324c77 100644
> --- a/drivers/input/misc/pm8xxx-vibrator.c
> +++ b/drivers/input/misc/pm8xxx-vibrator.c
> @@ -12,36 +12,44 @@
> #include <linux/regmap.h>
> #include <linux/slab.h>
>
> +#define SSBL_VIB_DRV_REG 0x4A
SSBI_VIB....
> +#define SSBI_VIB_DRV_EN_MANUAL_MASK GENMASK(7, 2)
> +#define SSBI_VIB_DRV_LEVEL_MASK GENMASK(7, 3)
> +#define SSBI_VIB_DRV_SHIFT 3
> +
> +#define SPMI_VIB_DRV_REG 0x41
> +#define SPMI_VIB_DRV_LEVEL_MASK GENMASK(4, 0)
> +#define SPMI_VIB_DRV_SHIFT 0
> +
> +#define SPMI_VIB_EN_REG 0x46
> +#define SPMI_VIB_EN_BIT BIT(7)
> +
> #define VIB_MAX_LEVEL_mV (3100)
> #define VIB_MIN_LEVEL_mV (1200)
> #define VIB_MAX_LEVELS (VIB_MAX_LEVEL_mV - VIB_MIN_LEVEL_mV)
>
> #define MAX_FF_SPEED 0xff
>
> -struct pm8xxx_regs {
> - unsigned int enable_addr;
> - unsigned int enable_mask;
> +enum vib_hw_type {
> + SSBI_VIB,
> + SPMI_VIB,
> +};
>
> - unsigned int drv_addr;
> - unsigned int drv_mask;
> - unsigned int drv_shift;
> - unsigned int drv_en_manual_mask;
> +struct pm8xxx_vib_data {
> + enum vib_hw_type hw_type;
> + unsigned int enable_addr;
> + unsigned int drv_addr;
> };
>
> -static const struct pm8xxx_regs pm8058_regs = {
> - .drv_addr = 0x4A,
> - .drv_mask = 0xf8,
> - .drv_shift = 3,
> - .drv_en_manual_mask = 0xfc,
> +static const struct pm8xxx_vib_data ssbi_vib_data = {
> + .hw_type = SSBI_VIB,
> + .drv_addr = SSBL_VIB_DRV_REG,
> };
>
> -static struct pm8xxx_regs pm8916_regs = {
> - .enable_addr = 0xc046,
> - .enable_mask = BIT(7),
> - .drv_addr = 0xc041,
> - .drv_mask = 0x1F,
> - .drv_shift = 0,
> - .drv_en_manual_mask = 0,
> +static const struct pm8xxx_vib_data spmi_vib_data = {
> + .hw_type = SPMI_VIB,
> + .enable_addr = SPMI_VIB_EN_REG,
> + .drv_addr = SPMI_VIB_DRV_REG,
> };
>
> /**
> @@ -49,7 +57,8 @@ static struct pm8xxx_regs pm8916_regs = {
> * @vib_input_dev: input device supporting force feedback
> * @work: work structure to set the vibration parameters
> * @regmap: regmap for register read/write
> - * @regs: registers' info
> + * @data: vibrator HW info
> + * @reg_base: the register base of the module
> * @speed: speed of vibration set from userland
> * @active: state of vibrator
> * @level: level of vibration to set in the chip
> @@ -59,7 +68,8 @@ struct pm8xxx_vib {
> struct input_dev *vib_input_dev;
> struct work_struct work;
> struct regmap *regmap;
> - const struct pm8xxx_regs *regs;
> + const struct pm8xxx_vib_data *data;
> + unsigned int reg_base;
> int speed;
> int level;
> bool active;
> @@ -75,24 +85,31 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
> {
> int rc;
> unsigned int val = vib->reg_vib_drv;
> - const struct pm8xxx_regs *regs = vib->regs;
> + u32 mask = SPMI_VIB_DRV_LEVEL_MASK;
> + u32 shift = SPMI_VIB_DRV_SHIFT;
> +
> + if (vib->data->hw_type == SSBI_VIB) {
> + mask = SSBI_VIB_DRV_LEVEL_MASK;
> + shift = SSBI_VIB_DRV_SHIFT;
> + }
>
> if (on)
> - val |= (vib->level << regs->drv_shift) & regs->drv_mask;
> + val |= (vib->level << shift) & mask;
> else
> - val &= ~regs->drv_mask;
> + val &= ~mask;
>
> - rc = regmap_write(vib->regmap, regs->drv_addr, val);
> + rc = regmap_update_bits(vib->regmap, vib->reg_base + vib->data->drv_addr, mask, val);
> if (rc < 0)
> return rc;
>
> vib->reg_vib_drv = val;
>
> - if (regs->enable_mask)
> - rc = regmap_update_bits(vib->regmap, regs->enable_addr,
> - regs->enable_mask, on ? ~0 : 0);
> + if (vib->data->hw_type == SSBI_VIB)
> + return 0;
>
> - return rc;
> + mask = SPMI_VIB_EN_BIT;
> + val = on ? SPMI_VIB_EN_BIT : 0;
> + return regmap_update_bits(vib->regmap, vib->reg_base + vib->data->enable_addr, mask, val);
> }
>
> /**
> @@ -102,13 +119,6 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
> static void pm8xxx_work_handler(struct work_struct *work)
> {
> struct pm8xxx_vib *vib = container_of(work, struct pm8xxx_vib, work);
> - const struct pm8xxx_regs *regs = vib->regs;
> - int rc;
> - unsigned int val;
> -
> - rc = regmap_read(vib->regmap, regs->drv_addr, &val);
> - if (rc < 0)
> - return;
>
> /*
> * pmic vibrator supports voltage ranges from 1.2 to 3.1V, so
> @@ -168,9 +178,9 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
> {
> struct pm8xxx_vib *vib;
> struct input_dev *input_dev;
> + const struct pm8xxx_vib_data *data;
> int error;
> - unsigned int val;
> - const struct pm8xxx_regs *regs;
> + unsigned int val, reg_base;
>
> vib = devm_kzalloc(&pdev->dev, sizeof(*vib), GFP_KERNEL);
> if (!vib)
> @@ -187,19 +197,33 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
> INIT_WORK(&vib->work, pm8xxx_work_handler);
> vib->vib_input_dev = input_dev;
>
> - regs = of_device_get_match_data(&pdev->dev);
> + data = of_device_get_match_data(&pdev->dev);
> + if (!data)
> + return -EINVAL;
>
> - /* operate in manual mode */
> - error = regmap_read(vib->regmap, regs->drv_addr, &val);
> - if (error < 0)
> - return error;
> + if (data->hw_type != SSBI_VIB) {
You can drop this condition, if ssbi_vib_data.drv_addr is 0.
> + error = fwnode_property_read_u32(pdev->dev.fwnode, "reg", ®_base);
> + if (error < 0) {
> + dev_err(&pdev->dev, "Failed to read reg address, rc=%d\n", error);
> + return error;
> + }
> +
> + vib->reg_base += reg_base;
> + }
>
> - val &= regs->drv_en_manual_mask;
> - error = regmap_write(vib->regmap, regs->drv_addr, val);
> + error = regmap_read(vib->regmap, vib->reg_base + data->drv_addr, &val);
> if (error < 0)
> return error;
>
> - vib->regs = regs;
> + /* operate in manual mode */
> + if (data->hw_type == SSBI_VIB) {
> + val &= SSBI_VIB_DRV_EN_MANUAL_MASK;
> + error = regmap_write(vib->regmap, vib->reg_base + data->drv_addr, val);
> + if (error < 0)
> + return error;
> + }
> +
> + vib->data = data;
> vib->reg_vib_drv = val;
>
> input_dev->name = "pm8xxx_vib_ffmemless";
> @@ -239,9 +263,9 @@ static int pm8xxx_vib_suspend(struct device *dev)
> static DEFINE_SIMPLE_DEV_PM_OPS(pm8xxx_vib_pm_ops, pm8xxx_vib_suspend, NULL);
>
> static const struct of_device_id pm8xxx_vib_id_table[] = {
> - { .compatible = "qcom,pm8058-vib", .data = &pm8058_regs },
> - { .compatible = "qcom,pm8921-vib", .data = &pm8058_regs },
> - { .compatible = "qcom,pm8916-vib", .data = &pm8916_regs },
> + { .compatible = "qcom,pm8058-vib", .data = &ssbi_vib_data },
> + { .compatible = "qcom,pm8921-vib", .data = &ssbi_vib_data },
> + { .compatible = "qcom,pm8916-vib", .data = &spmi_vib_data },
> { }
> };
> MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);
> --
> 2.25.1
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [RESEND PATCH v6 3/3] input: pm8xxx-vibrator: add new SPMI vibrator support
From: Dmitry Baryshkov @ 2023-09-23 19:07 UTC (permalink / raw)
To: Fenglin Wu
Cc: linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt, robh+dt,
agross, andersson, Konrad Dybcio, Dmitry Torokhov, linux-input,
quic_collinsd, quic_subbaram, quic_kamalw, jestar, Luca Weiss
In-Reply-To: <20230922083801.3056724-4-quic_fenglinw@quicinc.com>
On Fri, 22 Sept 2023 at 11:38, Fenglin Wu <quic_fenglinw@quicinc.com> wrote:
>
> Add new SPMI vibrator module which is very similar to the SPMI vibrator
> module inside PM8916 but just has a finer drive voltage step (1mV vs
> 100mV) hence its drive level control is expanded to across 2 registers.
> The vibrator module can be found in Qualcomm PMIC PMI632, then following
> PM7250B, PM7325B, PM7550BA PMICs.
>
> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
> Tested-by: Luca Weiss <luca.weiss@fairphone.com> # sdm632-fairphone-fp3 (pmi632)
> ---
> drivers/input/misc/pm8xxx-vibrator.c | 55 +++++++++++++++++++++++++---
> 1 file changed, 50 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
> index d6b468324c77..990e8a9ac018 100644
> --- a/drivers/input/misc/pm8xxx-vibrator.c
> +++ b/drivers/input/misc/pm8xxx-vibrator.c
> @@ -21,6 +21,13 @@
> #define SPMI_VIB_DRV_LEVEL_MASK GENMASK(4, 0)
> #define SPMI_VIB_DRV_SHIFT 0
>
> +#define SPMI_VIB_GEN2_DRV_REG 0x40
> +#define SPMI_VIB_GEN2_DRV_MASK GENMASK(7, 0)
> +#define SPMI_VIB_GEN2_DRV_SHIFT 0
> +#define SPMI_VIB_GEN2_DRV2_REG 0x41
> +#define SPMI_VIB_GEN2_DRV2_MASK GENMASK(3, 0)
> +#define SPMI_VIB_GEN2_DRV2_SHIFT 8
> +
> #define SPMI_VIB_EN_REG 0x46
> #define SPMI_VIB_EN_BIT BIT(7)
>
> @@ -33,12 +40,14 @@
> enum vib_hw_type {
> SSBI_VIB,
> SPMI_VIB,
> + SPMI_VIB_GEN2
> };
>
> struct pm8xxx_vib_data {
> enum vib_hw_type hw_type;
> unsigned int enable_addr;
> unsigned int drv_addr;
> + unsigned int drv2_addr;
> };
>
> static const struct pm8xxx_vib_data ssbi_vib_data = {
> @@ -52,6 +61,13 @@ static const struct pm8xxx_vib_data spmi_vib_data = {
> .drv_addr = SPMI_VIB_DRV_REG,
> };
>
> +static const struct pm8xxx_vib_data spmi_vib_gen2_data = {
> + .hw_type = SPMI_VIB_GEN2,
> + .enable_addr = SPMI_VIB_EN_REG,
> + .drv_addr = SPMI_VIB_GEN2_DRV_REG,
> + .drv2_addr = SPMI_VIB_GEN2_DRV2_REG,
> +};
> +
> /**
> * struct pm8xxx_vib - structure to hold vibrator data
> * @vib_input_dev: input device supporting force feedback
> @@ -85,12 +101,24 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
> {
> int rc;
> unsigned int val = vib->reg_vib_drv;
> - u32 mask = SPMI_VIB_DRV_LEVEL_MASK;
> - u32 shift = SPMI_VIB_DRV_SHIFT;
> + u32 mask, shift;
>
> - if (vib->data->hw_type == SSBI_VIB) {
> +
> + switch (vib->data->hw_type) {
> + case SSBI_VIB:
> mask = SSBI_VIB_DRV_LEVEL_MASK;
> shift = SSBI_VIB_DRV_SHIFT;
> + break;
> + case SPMI_VIB:
> + mask = SPMI_VIB_DRV_LEVEL_MASK;
> + shift = SPMI_VIB_DRV_SHIFT;
> + break;
> + case SPMI_VIB_GEN2:
> + mask = SPMI_VIB_GEN2_DRV_MASK;
> + shift = SPMI_VIB_GEN2_DRV_SHIFT;
> + break;
> + default:
> + return -EINVAL;
Could you please move the switch to the previous patch? Then it would
be more obvious that you are just adding the SPMI_VIB_GEN2 here.
Other than that LGTM.
> }
>
> if (on)
> @@ -104,6 +132,19 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
>
> vib->reg_vib_drv = val;
>
> + if (vib->data->hw_type == SPMI_VIB_GEN2) {
> + mask = SPMI_VIB_GEN2_DRV2_MASK;
> + shift = SPMI_VIB_GEN2_DRV2_SHIFT;
> + if (on)
> + val = (vib->level >> shift) & mask;
> + else
> + val = 0;
> + rc = regmap_update_bits(vib->regmap,
> + vib->reg_base + vib->data->drv2_addr, mask, val);
> + if (rc < 0)
> + return rc;
> + }
> +
> if (vib->data->hw_type == SSBI_VIB)
> return 0;
>
> @@ -128,10 +169,13 @@ static void pm8xxx_work_handler(struct work_struct *work)
> vib->active = true;
> vib->level = ((VIB_MAX_LEVELS * vib->speed) / MAX_FF_SPEED) +
> VIB_MIN_LEVEL_mV;
> - vib->level /= 100;
> + if (vib->data->hw_type != SPMI_VIB_GEN2)
> + vib->level /= 100;
> } else {
> vib->active = false;
> - vib->level = VIB_MIN_LEVEL_mV / 100;
> + vib->level = VIB_MIN_LEVEL_mV;
> + if (vib->data->hw_type != SPMI_VIB_GEN2)
> + vib->level /= 100;
> }
>
> pm8xxx_vib_set(vib, vib->active);
> @@ -266,6 +310,7 @@ static const struct of_device_id pm8xxx_vib_id_table[] = {
> { .compatible = "qcom,pm8058-vib", .data = &ssbi_vib_data },
> { .compatible = "qcom,pm8921-vib", .data = &ssbi_vib_data },
> { .compatible = "qcom,pm8916-vib", .data = &spmi_vib_data },
> + { .compatible = "qcom,pmi632-vib", .data = &spmi_vib_gen2_data },
> { }
> };
> MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);
> --
> 2.25.1
>
--
With best wishes
Dmitry
^ permalink raw reply
* [PATCH] HID: lenovo: Detect quirk-free fw on cptkbd and stop applying workaround
From: Mikhail Khvainitski @ 2023-09-23 22:58 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Mikhail Khvainitski
In-Reply-To: <20230923231522.94060-1-me@khvoinitsky.org>
Built-in firmware of cptkbd handles scrolling by itself (when middle
button is pressed) but with issues: it does not support horizontal and
hi-res scrolling and upon middle button release it sends middle button
click even if there was a scrolling event. Commit 3cb5ff0220e3 ("HID:
lenovo: Hide middle-button press until release") workarounds last
issue but it's impossible to workaround scrolling-related issues
without firmware modification.
Likely, Dennis Schneider has reverse engineered the firmware and
provided an instruction on how to patch it [1]. However,
aforementioned workaround prevents userspace (libinput) from knowing
exact moment when middle button has been pressed down and performing
"On-Button scrolling". This commit detects correctly-behaving patched
firmware if cursor movement events has been received during middle
button being pressed and stops applying workaround for this device.
Link: https://hohlerde.org/rauch/en/elektronik/projekte/tpkbd-fix/ [1]
Signed-off-by: Mikhail Khvainitski <me@khvoinitsky.org>
---
drivers/hid/hid-lenovo.c | 68 ++++++++++++++++++++++++++--------------
1 file changed, 45 insertions(+), 23 deletions(-)
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 44763c0da444..9c1181313e44 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -51,7 +51,12 @@ struct lenovo_drvdata {
int select_right;
int sensitivity;
int press_speed;
- u8 middlebutton_state; /* 0:Up, 1:Down (undecided), 2:Scrolling */
+ /* 0: Up
+ * 1: Down (undecided)
+ * 2: Scrolling
+ * 3: Patched firmware, disable workaround
+ */
+ u8 middlebutton_state;
bool fn_lock;
};
@@ -668,31 +673,48 @@ static int lenovo_event_cptkbd(struct hid_device *hdev,
{
struct lenovo_drvdata *cptkbd_data = hid_get_drvdata(hdev);
- /* "wheel" scroll events */
- if (usage->type == EV_REL && (usage->code == REL_WHEEL ||
- usage->code == REL_HWHEEL)) {
- /* Scroll events disable middle-click event */
- cptkbd_data->middlebutton_state = 2;
- return 0;
- }
+ if (cptkbd_data->middlebutton_state != 3) {
+ /* REL_X and REL_Y events during middle button pressed
+ * are only possible on patched, bug-free firmware
+ * so set middlebutton_state to 3
+ * to never apply workaround anymore
+ */
+ if (cptkbd_data->middlebutton_state == 1 &&
+ usage->type == EV_REL &&
+ (usage->code == REL_X || usage->code == REL_Y)) {
+ cptkbd_data->middlebutton_state = 3;
+ /* send middle button press which was hold before */
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 1);
+ input_sync(field->hidinput->input);
+ }
- /* Middle click events */
- if (usage->type == EV_KEY && usage->code == BTN_MIDDLE) {
- if (value == 1) {
- cptkbd_data->middlebutton_state = 1;
- } else if (value == 0) {
- if (cptkbd_data->middlebutton_state == 1) {
- /* No scrolling inbetween, send middle-click */
- input_event(field->hidinput->input,
- EV_KEY, BTN_MIDDLE, 1);
- input_sync(field->hidinput->input);
- input_event(field->hidinput->input,
- EV_KEY, BTN_MIDDLE, 0);
- input_sync(field->hidinput->input);
+ /* "wheel" scroll events */
+ if (usage->type == EV_REL && (usage->code == REL_WHEEL ||
+ usage->code == REL_HWHEEL)) {
+ /* Scroll events disable middle-click event */
+ cptkbd_data->middlebutton_state = 2;
+ return 0;
+ }
+
+ /* Middle click events */
+ if (usage->type == EV_KEY && usage->code == BTN_MIDDLE) {
+ if (value == 1) {
+ cptkbd_data->middlebutton_state = 1;
+ } else if (value == 0) {
+ if (cptkbd_data->middlebutton_state == 1) {
+ /* No scrolling inbetween, send middle-click */
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 1);
+ input_sync(field->hidinput->input);
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 0);
+ input_sync(field->hidinput->input);
+ }
+ cptkbd_data->middlebutton_state = 0;
}
- cptkbd_data->middlebutton_state = 0;
+ return 1;
}
- return 1;
}
if (usage->type == EV_KEY && usage->code == KEY_FN_ESC && value == 1) {
--
2.42.0
^ permalink raw reply related
* Re: [PATCH] HID: lenovo: Add middleclick_workaround sysfs knob for cptkbd
From: Mikhail Khvainitski @ 2023-09-23 22:58 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Mikhail Khvainitski
In-Reply-To: <20230918145042.37368-1-me@khvoinitsky.org>
So, while I was waiting for the review of the patch, I've come up with
much better idea: we can just autodetect if stock and buggy or
patched and bug-free firmware is used and have no need to introduce
additional sysfs knob. So the patch which implements this follows in
the next message.
While the patch is ready to be merged as is (of course, if there are
no comments from reviewers), I thought about the following: may be we
should add a message to dmesg upon detecting stock and buggy firmware
that user should consider patching the firmware? If so, what is the
best place to put an instruction to? Post a link to dmesg? Can the
link point directly to Dennis' blog or its better to put it somewhere
else (with credits, of course)?
Thanks.
^ permalink raw reply
* Re: [PATCH 00/52] input: Convert to platform remove callback returning void
From: Dmitry Torokhov @ 2023-09-24 2:48 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Michael Hennerich, linux-input, kernel, Benson Leung,
Guenter Roeck, Greg Kroah-Hartman, Jonathan Cameron,
joewu (吳仲振), chrome-platform,
Andy Shevchenko, Mattijs Korpershoek, Jeff LaBundy, Rob Herring,
Siarhei Volkau, Pavel Machek, Steven Rostedt (Google),
Paolo Abeni, Kalle Valo, Yangtao Li, ye xingchen, Maxime Coquelin,
Alexandre Torgue, linux-stm32, linux-arm-kernel,
Support Opensource, Andrey Moiseev, Lee Jones, Linus Walleij,
Andy Gross, Bjorn Andersson, Konrad Dybcio, linux-arm-msm,
Hans de Goede, Miloslav Trmac, patches, Christophe JAILLET,
Liang He, Chen Jun, Ruan Jinjie, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, linux-sunxi, Michal Simek, Robert Jarzmik,
Dmitry Baryshkov, Arnd Bergmann, Rafael J. Wysocki,
Krzysztof Kozlowski, Daniel Lezcano, Jonathan Corbet
In-Reply-To: <20230920125829.1478827-1-u.kleine-koenig@pengutronix.de>
On Wed, Sep 20, 2023 at 02:57:37PM +0200, Uwe Kleine-König wrote:
> Hello,
>
> this series converts all platform drivers below drivers/input to use
> remove_new. The motivation is to get rid of an integer return code
> that is (mostly) ignored by the platform driver core and error prone on
> the driver side.
>
> See commit 5c5a7680e67b ("platform: Provide a remove callback that
> returns no value") for an extended explanation and the eventual goal.
>
> There are no interdependencies between the patches. As there are still
> quite a few drivers to convert, I'm happy about every patch that makes
> it in. So even if there is a merge conflict with one patch until you
> apply or a subject prefix is suboptimal, please apply the remainder of
> this series anyhow.
Applied the lot (fixing the i8042-sparcio patch subject), thank you!
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox