* [PATCH v4 11/17] platform/x86/amd/pmf: Add capability to sideload of policy binary
From: Shyam Sundar S K @ 2023-10-18 7:02 UTC (permalink / raw)
To: hdegoede, markgross, ilpo.jarvinen, 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: <20231018070241.2041529-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 | 67 +++++++++++++++++++++++++++
2 files changed, 68 insertions(+)
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 593930519039..8712299ad52b 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -219,6 +219,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 0eba258f4040..6c4ce22ba518 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"
@@ -16,9 +17,14 @@
/* Policy binary actions sampling frequency (in ms) */
static int pb_actions_ms = MSEC_PER_SEC;
+/* Sideload policy binaries to debug policy failures */
+static bool pb_side_load;
+
#ifdef CONFIG_AMD_PMF_DEBUG
module_param(pb_actions_ms, int, 0644);
MODULE_PARM_DESC(pb_actions_ms, "Policy binary actions sampling frequency (default = 1000ms)");
+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,
@@ -269,6 +275,61 @@ 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,
+};
+
+static 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;
+}
+
+static void amd_pmf_remove_pb(struct amd_pmf_dev *dev)
+{
+ debugfs_remove_recursive(dev->esbin);
+}
+#else
+static int amd_pmf_open_pb(struct amd_pmf_dev *dev, struct dentry *debugfs_root)
+{
+ return 0;
+}
+
+static void amd_pmf_remove_pb(struct amd_pmf_dev *dev) {}
+#endif
+
static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
{
dev->policy_buf = kzalloc(dev->policy_sz, GFP_KERNEL);
@@ -281,6 +342,9 @@ static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
memcpy(dev->policy_buf, dev->policy_base, dev->policy_sz);
+ if (pb_side_load)
+ amd_pmf_open_pb(dev, dev->dbgfs_dir);
+
return amd_pmf_start_policy_engine(dev);
}
@@ -382,6 +446,9 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev)
{
+ if (pb_side_load)
+ amd_pmf_remove_pb(dev);
+
kfree(dev->prev_data);
kfree(dev->policy_buf);
cancel_delayed_work_sync(&dev->pb_work);
--
2.25.1
^ permalink raw reply related
* [PATCH v4 12/17] platform/x86/amd/pmf: dump policy binary data
From: Shyam Sundar S K @ 2023-10-18 7:02 UTC (permalink / raw)
To: hdegoede, markgross, ilpo.jarvinen, 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: <20231018070241.2041529-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.
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/tee-if.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 6c4ce22ba518..2f5fb8623c20 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -276,6 +276,12 @@ static int amd_pmf_start_policy_engine(struct amd_pmf_dev *dev)
}
#ifdef CONFIG_AMD_PMF_DEBUG
+static void amd_pmf_hex_dump_pb(struct amd_pmf_dev *dev)
+{
+ print_hex_dump_debug("(pb): ", DUMP_PREFIX_OFFSET, 16, 1, dev->policy_buf,
+ dev->policy_sz, false);
+}
+
static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
size_t length, loff_t *pos)
{
@@ -290,6 +296,7 @@ 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;
+ amd_pmf_hex_dump_pb(dev);
ret = amd_pmf_start_policy_engine(dev);
if (ret)
return -EINVAL;
@@ -328,6 +335,7 @@ static int amd_pmf_open_pb(struct amd_pmf_dev *dev, struct dentry *debugfs_root)
}
static void amd_pmf_remove_pb(struct amd_pmf_dev *dev) {}
+static void amd_pmf_hex_dump_pb(struct amd_pmf_dev *dev) {}
#endif
static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
@@ -342,6 +350,7 @@ static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
memcpy(dev->policy_buf, dev->policy_base, dev->policy_sz);
+ amd_pmf_hex_dump_pb(dev);
if (pb_side_load)
amd_pmf_open_pb(dev, dev->dbgfs_dir);
--
2.25.1
^ permalink raw reply related
* [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
From: Shyam Sundar S K @ 2023-10-18 7:02 UTC (permalink / raw)
To: hdegoede, markgross, ilpo.jarvinen, 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: <20231018070241.2041529-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 | 138 ++++++++++++++++++++++++
drivers/platform/x86/amd/pmf/Kconfig | 1 +
drivers/platform/x86/amd/pmf/core.c | 1 +
drivers/platform/x86/amd/pmf/pmf.h | 3 +
drivers/platform/x86/amd/pmf/spc.c | 13 +++
drivers/platform/x86/amd/pmf/tee-if.c | 26 +++++
include/linux/amd-pmf-io.h | 35 ++++++
9 files changed, 220 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 a79d53bdbe13..26ffa1b4fe57 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..ac981848df50
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
@@ -0,0 +1,138 @@
+/*
+ * 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;
+ 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;
+ }
+
+ mutex_lock(&mode_config->mutex);
+ drm_connector_list_iter_begin(drm_dev, &iter);
+ drm_for_each_connector_iter(connector, &iter) {
+ if (connector->status == connector_status_connected)
+ pmf->display_count++;
+ if (connector->status != pmf->con_status[i])
+ pmf->con_status[i] = connector->status;
+ if (connector->connector_type != pmf->connector_type[i])
+ pmf->connector_type[i] = connector->connector_type;
+
+ i++;
+ if (i >= MAX_SUPPORTED)
+ break;
+ }
+ drm_connector_list_iter_end(&iter);
+ mutex_unlock(&mode_config->mutex);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
+
+static int amd_pmf_gpu_get_cur_state(struct thermal_cooling_device *cooling_dev,
+ unsigned long *state)
+{
+ struct backlight_device *bd;
+
+ if (!acpi_video_backlight_use_native())
+ return -ENODEV;
+
+ bd = backlight_device_get_by_type(BACKLIGHT_RAW);
+ if (!bd)
+ return -ENODEV;
+
+ *state = backlight_get_brightness(bd);
+
+ return 0;
+}
+
+static int amd_pmf_gpu_get_max_state(struct thermal_cooling_device *cooling_dev,
+ unsigned long *state)
+{
+ struct backlight_device *bd;
+
+ if (!acpi_video_backlight_use_native())
+ return -ENODEV;
+
+ bd = backlight_device_get_by_type(BACKLIGHT_RAW);
+ if (!bd)
+ return -ENODEV;
+
+ if (backlight_is_blank(bd))
+ *state = 0;
+ else
+ *state = bd->props.max_brightness;
+
+ return 0;
+}
+
+static const struct thermal_cooling_device_ops bd_cooling_ops = {
+ .get_max_state = amd_pmf_gpu_get_max_state,
+ .get_cur_state = amd_pmf_gpu_get_cur_state,
+};
+
+int amd_pmf_gpu_init(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);
+
+ if (!(adev->flags & AMD_IS_APU)) {
+ DRM_ERROR("PMF-AMDGPU interface not supported\n");
+ return -ENODEV;
+ }
+
+ if (!acpi_video_backlight_use_native())
+ return -ENODEV;
+
+ pmf->raw_bd = backlight_device_get_by_type(BACKLIGHT_RAW);
+ if (!pmf->raw_bd)
+ return -ENODEV;
+
+ pmf->cooling_dev = thermal_cooling_device_register("pmf_gpu_bd",
+ pmf, &bd_cooling_ops);
+ if (IS_ERR(pmf->cooling_dev))
+ return -ENODEV;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(amd_pmf_gpu_init);
+
+void amd_pmf_gpu_deinit(struct amd_gpu_pmf_data *pmf)
+{
+ thermal_cooling_device_unregister(pmf->cooling_dev);
+}
+EXPORT_SYMBOL_GPL(amd_pmf_gpu_deinit);
diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
index f246252bddd8..7f430de7af44 100644
--- a/drivers/platform/x86/amd/pmf/Kconfig
+++ b/drivers/platform/x86/amd/pmf/Kconfig
@@ -10,6 +10,7 @@ config AMD_PMF
depends on AMD_NB
select ACPI_PLATFORM_PROFILE
depends on TEE && AMDTEE
+ depends on 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 4b8156033fa6..c59ba527ff49 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -411,6 +411,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 8712299ad52b..615cd3a31986 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -13,6 +13,7 @@
#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
@@ -228,9 +229,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 512e0c66efdc..cf4962ef97c2 100644
--- a/drivers/platform/x86/amd/pmf/spc.c
+++ b/drivers/platform/x86/amd/pmf/spc.c
@@ -44,6 +44,10 @@ void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *
dev_dbg(dev->dev, "Max C0 Residency : %u\n", in->ev_info.max_c0residency);
dev_dbg(dev->dev, "GFX Busy : %u\n", in->ev_info.gfx_busy);
dev_dbg(dev->dev, "Connected Display Count : %u\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");
}
@@ -146,6 +150,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 */
@@ -154,4 +166,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 2f5fb8623c20..956e66b78605 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"
@@ -357,6 +358,19 @@ 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) {
+ /* Found the amdgpu handle from the pci root after walking through the pci bus */
+ dev->gfx_data.gpu_dev = 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;
@@ -446,6 +460,15 @@ 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");
+
+ if (!amd_pmf_gpu_init(&dev->gfx_data))
+ dev->gfx_data.gpu_dev_en = true;
+
dev->prev_data = kzalloc(sizeof(*dev->prev_data), GFP_KERNEL);
if (!dev->prev_data)
return -ENOMEM;
@@ -461,5 +484,8 @@ 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);
+ if (dev->gfx_data.gpu_dev_en)
+ amd_pmf_gpu_deinit(&dev->gfx_data);
+ 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..5f79e66a41b3
--- /dev/null
+++ b/include/linux/amd-pmf-io.h
@@ -0,0 +1,35 @@
+/* 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 <acpi/video.h>
+#include <drm/drm_connector.h>
+#include <linux/backlight.h>
+#include <linux/thermal.h>
+
+#define MAX_SUPPORTED 4
+
+/* amdgpu */
+struct amd_gpu_pmf_data {
+ struct pci_dev *gpu_dev;
+ struct backlight_device *raw_bd;
+ struct thermal_cooling_device *cooling_dev;
+ enum drm_connector_status con_status[MAX_SUPPORTED];
+ int display_count;
+ int connector_type[MAX_SUPPORTED];
+ bool gpu_dev_en;
+};
+
+int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
+int amd_pmf_gpu_init(struct amd_gpu_pmf_data *pmf);
+void amd_pmf_gpu_deinit(struct amd_gpu_pmf_data *pmf);
+#endif
--
2.25.1
^ permalink raw reply related
* [PATCH v4 14/17] platform/x86/amd/pmf: Add PMF-AMDGPU set interface
From: Shyam Sundar S K @ 2023-10-18 7:02 UTC (permalink / raw)
To: hdegoede, markgross, ilpo.jarvinen, 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: <20231018070241.2041529-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.
Change amd_pmf_apply_policies() return type, so that it can return
errors when the call to retrieve information from amdgpu fails.
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 | 18 ++++++++++++++++++
drivers/platform/x86/amd/pmf/pmf.h | 2 ++
drivers/platform/x86/amd/pmf/tee-if.c | 21 +++++++++++++++++++--
3 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
index ac981848df50..57acfc32219a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
@@ -63,6 +63,23 @@ int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf)
}
EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
+static int amd_pmf_gpu_set_cur_state(struct thermal_cooling_device *cooling_dev,
+ unsigned long state)
+{
+ struct backlight_device *bd;
+
+ if (!acpi_video_backlight_use_native())
+ return -ENODEV;
+
+ bd = backlight_device_get_by_type(BACKLIGHT_RAW);
+ if (!bd)
+ return -ENODEV;
+
+ backlight_device_set_brightness(bd, state);
+
+ return 0;
+}
+
static int amd_pmf_gpu_get_cur_state(struct thermal_cooling_device *cooling_dev,
unsigned long *state)
{
@@ -103,6 +120,7 @@ static int amd_pmf_gpu_get_max_state(struct thermal_cooling_device *cooling_dev,
static const struct thermal_cooling_device_ops bd_cooling_ops = {
.get_max_state = amd_pmf_gpu_get_max_state,
.get_cur_state = amd_pmf_gpu_get_cur_state,
+ .set_cur_state = amd_pmf_gpu_set_cur_state,
};
int amd_pmf_gpu_init(struct amd_gpu_pmf_data *pmf)
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 615cd3a31986..641af94ba378 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -75,6 +75,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 */
@@ -488,6 +489,7 @@ enum ta_pmf_error_type {
};
struct pmf_action_table {
+ unsigned long display_brightness;
enum system_state system_state;
u32 spl; /* in mW */
u32 sppt; /* in mW */
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 956e66b78605..b39f30a4f92d 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -77,8 +77,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)
{
+ struct thermal_cooling_device *cdev = dev->gfx_data.cooling_dev;
+ unsigned long state;
u32 val;
int idx;
@@ -154,8 +156,21 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
dev_dbg(dev->dev, "update SYSTEM_STATE : %s\n",
amd_pmf_uevent_as_str(val));
break;
+
+ case PMF_POLICY_DISPLAY_BRIGHTNESS:
+ if (!dev->gfx_data.gpu_dev_en)
+ return -ENODEV;
+
+ cdev->ops->get_cur_state(cdev, &state);
+ if (state != val) {
+ cdev->ops->set_cur_state(cdev, val);
+ dev_dbg(dev->dev, "update DISPLAY_BRIGHTNESS : %u\n", val);
+ }
+ break;
}
}
+
+ return 0;
}
static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
@@ -192,7 +207,9 @@ static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
amd_pmf_dump_ta_inputs(dev, in);
dev_dbg(dev->dev, "action count:%u result:%x\n", out->actions_count,
ta_sm->pmf_result);
- amd_pmf_apply_policies(dev, out);
+ ret = amd_pmf_apply_policies(dev, out);
+ if (ret)
+ return ret;
}
return 0;
--
2.25.1
^ permalink raw reply related
* [PATCH v4 15/17] HID: amd_sfh: rename float_to_int() to amd_sfh_float_to_int()
From: Shyam Sundar S K @ 2023-10-18 7:02 UTC (permalink / raw)
To: hdegoede, markgross, ilpo.jarvinen, 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: <20231018070241.2041529-1-Shyam-sundar.S-k@amd.com>
From: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Current amd_sfh driver has float_to_int() to convert units from
float to int. This is fine until this function gets called outside of
the current scope of file.
Add a prefix "amd_sfh" to float_to_int() so that function represents
the driver name. This function will be called by multiple callers in the
next patch.
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/sfh1_1/amd_sfh_desc.c | 22 +++++++++----------
.../amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 1 +
2 files changed, 12 insertions(+), 11 deletions(-)
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..dbc8c6943ca1 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 amd_sfh_float_to_int(u32 flt32_val)
{
int fraction, shift, mantissa, sign, exp, zeropre;
@@ -200,9 +200,9 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
OFFSET_SENSOR_DATA_DEFAULT;
memcpy_fromio(&accel_data, sensoraddr, sizeof(struct sfh_accel_data));
get_common_inputs(&acc_input.common_property, report_id);
- acc_input.in_accel_x_value = float_to_int(accel_data.acceldata.x) / 100;
- acc_input.in_accel_y_value = float_to_int(accel_data.acceldata.y) / 100;
- acc_input.in_accel_z_value = float_to_int(accel_data.acceldata.z) / 100;
+ acc_input.in_accel_x_value = amd_sfh_float_to_int(accel_data.acceldata.x) / 100;
+ acc_input.in_accel_y_value = amd_sfh_float_to_int(accel_data.acceldata.y) / 100;
+ acc_input.in_accel_z_value = amd_sfh_float_to_int(accel_data.acceldata.z) / 100;
memcpy(input_report, &acc_input, sizeof(acc_input));
report_size = sizeof(acc_input);
break;
@@ -211,9 +211,9 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
OFFSET_SENSOR_DATA_DEFAULT;
memcpy_fromio(&gyro_data, sensoraddr, sizeof(struct sfh_gyro_data));
get_common_inputs(&gyro_input.common_property, report_id);
- gyro_input.in_angel_x_value = float_to_int(gyro_data.gyrodata.x) / 1000;
- gyro_input.in_angel_y_value = float_to_int(gyro_data.gyrodata.y) / 1000;
- gyro_input.in_angel_z_value = float_to_int(gyro_data.gyrodata.z) / 1000;
+ gyro_input.in_angel_x_value = amd_sfh_float_to_int(gyro_data.gyrodata.x) / 1000;
+ gyro_input.in_angel_y_value = amd_sfh_float_to_int(gyro_data.gyrodata.y) / 1000;
+ gyro_input.in_angel_z_value = amd_sfh_float_to_int(gyro_data.gyrodata.z) / 1000;
memcpy(input_report, &gyro_input, sizeof(gyro_input));
report_size = sizeof(gyro_input);
break;
@@ -222,9 +222,9 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
OFFSET_SENSOR_DATA_DEFAULT;
memcpy_fromio(&mag_data, sensoraddr, sizeof(struct sfh_mag_data));
get_common_inputs(&magno_input.common_property, report_id);
- magno_input.in_magno_x = float_to_int(mag_data.magdata.x) / 100;
- magno_input.in_magno_y = float_to_int(mag_data.magdata.y) / 100;
- magno_input.in_magno_z = float_to_int(mag_data.magdata.z) / 100;
+ magno_input.in_magno_x = amd_sfh_float_to_int(mag_data.magdata.x) / 100;
+ magno_input.in_magno_y = amd_sfh_float_to_int(mag_data.magdata.y) / 100;
+ magno_input.in_magno_z = amd_sfh_float_to_int(mag_data.magdata.z) / 100;
magno_input.in_magno_accuracy = mag_data.accuracy / 100;
memcpy(input_report, &magno_input, sizeof(magno_input));
report_size = sizeof(magno_input);
@@ -234,7 +234,7 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
OFFSET_SENSOR_DATA_DEFAULT;
memcpy_fromio(&als_data, sensoraddr, sizeof(struct sfh_als_data));
get_common_inputs(&als_input.common_property, report_id);
- als_input.illuminance_value = float_to_int(als_data.lux);
+ als_input.illuminance_value = amd_sfh_float_to_int(als_data.lux);
report_size = sizeof(als_input);
memcpy(input_report, &als_input, sizeof(als_input));
break;
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..78e22850417a 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
@@ -151,4 +151,5 @@ struct hpd_status {
void sfh_interface_init(struct amd_mp2_dev *mp2);
void amd_sfh1_1_set_desc_ops(struct amd_mp2_ops *mp2_ops);
+int amd_sfh_float_to_int(u32 flt32_val);
#endif
--
2.25.1
^ permalink raw reply related
* [PATCH v4 16/17] platform/x86/amd/pmf: Add PMF-AMDSFH interface for HPD
From: Shyam Sundar S K @ 2023-10-18 7:02 UTC (permalink / raw)
To: hdegoede, markgross, ilpo.jarvinen, 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: <20231018070241.2041529-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_init.c | 11 ++++++++
.../amd-sfh-hid/sfh1_1/amd_sfh_interface.c | 28 +++++++++++++++++++
drivers/platform/x86/amd/pmf/Kconfig | 1 +
drivers/platform/x86/amd/pmf/spc.c | 21 ++++++++++++++
include/linux/amd-pmf-io.h | 20 ++++++++++++-
6 files changed, 85 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
index 2643bb14fee2..cd57037bf217 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
@@ -37,6 +37,10 @@ struct amd_mp2_sensor_info {
dma_addr_t dma_address;
};
+struct sfh_dev_status {
+ bool is_hpd_present;
+};
+
struct amd_mp2_dev {
struct pci_dev *pdev;
struct amdtp_cl_data *cl_data;
@@ -47,6 +51,7 @@ struct amd_mp2_dev {
struct amd_input_data in_data;
/* mp2 active control status */
u32 mp2_acs;
+ struct sfh_dev_status dev_en;
};
struct amd_mp2_ops {
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
index e9c6413af24a..3dc652d41d7d 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
@@ -73,6 +73,12 @@ static int amd_sfh_hid_client_deinit(struct amd_mp2_dev *privdata)
int i, status;
for (i = 0; i < cl_data->num_hid_devices; i++) {
+ switch (cl_data->sensor_idx[i]) {
+ case HPD_IDX:
+ privdata->dev_en.is_hpd_present = false;
+ break;
+ }
+
if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
privdata->mp2_ops->stop(privdata, cl_data->sensor_idx[i]);
status = amd_sfh_wait_for_response
@@ -178,6 +184,11 @@ static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata)
rc = amdtp_hid_probe(i, cl_data);
if (rc)
goto cleanup;
+ switch (cl_data->sensor_idx[i]) {
+ case HPD_IDX:
+ privdata->dev_en.is_hpd_present = true;
+ break;
+ }
}
dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
cl_data->sensor_idx[i], get_sensor_name(cl_data->sensor_idx[i]),
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
index 4f81ef2d4f56..7637da0dec6f 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 -EINVAL;
}
+EXPORT_SYMBOL_GPL(amd_get_sfh_info);
diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
index 7f430de7af44..d368d35a49ac 100644
--- a/drivers/platform/x86/amd/pmf/Kconfig
+++ b/drivers/platform/x86/amd/pmf/Kconfig
@@ -11,6 +11,7 @@ config AMD_PMF
select ACPI_PLATFORM_PROFILE
depends on TEE && AMDTEE
depends on DRM_AMDGPU
+ depends on AMD_SFH_HID
help
This driver provides support for the AMD Platform Management Framework.
The goal is to enhance end user experience by making AMD PCs smarter,
diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
index cf4962ef97c2..8bdfb395f316 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 *
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
@@ -158,6 +159,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 */
@@ -167,4 +187,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 5f79e66a41b3..76e42552b62c 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
@@ -32,4 +33,21 @@ struct amd_gpu_pmf_data {
int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
int amd_pmf_gpu_init(struct amd_gpu_pmf_data *pmf);
void amd_pmf_gpu_deinit(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;
+};
+
+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 v4 17/17] platform/x86/amd/pmf: Add PMF-AMDSFH interface for ALS
From: Shyam Sundar S K @ 2023-10-18 7:02 UTC (permalink / raw)
To: hdegoede, markgross, ilpo.jarvinen, 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: <20231018070241.2041529-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.
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
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 | 9 ++++++++-
include/linux/amd-pmf-io.h | 2 ++
5 files changed, 37 insertions(+), 1 deletion(-)
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 3dc652d41d7d..f2890d329459 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 7637da0dec6f..48a7a450e029 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 = amd_sfh_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 -EINVAL;
diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
index 8bdfb395f316..934afe346f7e 100644
--- a/drivers/platform/x86/amd/pmf/spc.c
+++ b/drivers/platform/x86/amd/pmf/spc.c
@@ -50,6 +50,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
@@ -162,8 +163,14 @@ static void amd_pmf_get_gpu_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_ta
static void amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
{
struct amd_sfh_info sfh_info;
+ int ret;
- /* get HPD data */
+ /* Get ALS data */
+ ret = amd_get_sfh_info(&sfh_info, MT_ALS);
+ if (!ret)
+ 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) {
case SFH_NOT_DETECTED:
diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
index 76e42552b62c..912d341d5fbe 100644
--- a/include/linux/amd-pmf-io.h
+++ b/include/linux/amd-pmf-io.h
@@ -37,6 +37,7 @@ void amd_pmf_gpu_deinit(struct amd_gpu_pmf_data *pmf);
/* amd-sfh */
enum sfh_message_type {
MT_HPD,
+ MT_ALS,
};
enum hpd_info {
@@ -46,6 +47,7 @@ enum hpd_info {
};
struct amd_sfh_info {
+ u32 ambient_light;
u8 user_present;
};
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v3 2/4] HID: touchscreen: Add initial support for Himax HID-over-SPI
From: Benjamin Tissoires @ 2023-10-18 7:15 UTC (permalink / raw)
To: Tylor Yang
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos,
benjamin.tissoires, linux-input, devicetree, linux-kernel,
poyuan_chang, jingyliang, hbarnor, wuxy23, luolm1, poyu_hung
In-Reply-To: <20231017091900.801989-3-tylor_yang@himax.corp-partner.google.com>
Hi Tylor,
[in addition to any other reviews]
On Oct 17 2023, Tylor Yang wrote:
> The hx83102j is a TDDI IC (Touch with Display Driver). The
> IC using SPI to transferring HID packet to host CPU. The IC also
> report HID report descriptor for driver to register HID device.
> The driver is designed as a framework for future expansion and
> hx83102j is the first case. Each hx_spi_hid_hx8xxxxx modules are
> mutual exclusive, it should be initiate one at a time.
>
> This driver takes a position similar to i2c-hid, it initialize
> and control the touch IC below and register HID to upper hid-core.
> When touch ic report an interrupt, it receive the data from IC
> and report as HID input to hid-core. Let hid-core dispatch input
> to registered hid-protocol and report to related input sub-system.
>
> This driver also provide advanced functions by hidraw interface:
Generally speaking, when your commit message has an "also" in the
middle, it means that the next feature(s) need to be split in their own
patches.
> - runtime firmware update
> - debug functions, such as reg r/w
> - self test for touch panel
So this means that this patch should at least be split in 4.
>
> Due to patch size is too big, separate into 3 part. This is part 1.
This is the wrong reason to split a patch series. Well, it's true, it's
too big, but you have to take into account the reviewers/maintainers
point of view:
- we don't know the internals of your device
- we don't (necessarily) have access to the docs
- we don't have a lot of time to spend on a review
- we can not focus on a 9000 lines of code patch and remember every
single aspect when reviewing, to be able to point bugs
Given that you compared this driver to i2c-hid, please have a look at
the history of it:
- my first initial submission[0] (v1) was a single patch of 1000 loc,
but it contained only the core functionality to bind a driver. I
stripped everything else that could make it useful (ACPI or DT
bindings) but it was a an attempt at being a one-to-one mapping of the
I2C part of the publicly available specification
- shortly after I sent a separate 14 patches series to do more cleanups
on the initial patch, as things were moving forward
- then Mika submitted the ACPI handling[1]
- and DT bindings came later [2] (8 months after the initial submission)
My point is, when the code is that big, it's perfectly fine to have it
split and maybe not have all of the functionalities available in the
first submission.
Bonus point for not having everything and in smaller patches: it's less
of a pain to change or drop stuff :)
>
> Signed-off-by: Tylor Yang <tylor_yang@himax.corp-partner.google.com>
> ---
> MAINTAINERS | 1 +
> drivers/hid/hx-hid/hx_acpi.c | 81 ++
> drivers/hid/hx-hid/hx_core.c | 1605 +++++++++++++++++++++++++++++
> drivers/hid/hx-hid/hx_core.h | 489 +++++++++
> drivers/hid/hx-hid/hx_hid.c | 753 ++++++++++++++
> drivers/hid/hx-hid/hx_hid.h | 96 ++
> drivers/hid/hx-hid/hx_ic_83102j.c | 340 ++++++
> drivers/hid/hx-hid/hx_ic_83102j.h | 42 +
hx-hid is a terrible name. Why not at least "himax-hid"? Or maybe
"himax-spi-hid"?
Also, I can't remember if this was already asked, but is that driver
vaguely related to the HID over SPI specification from Microsoft?
We have seen one submission in the past regarding that even if it didn't
went through, but if your driver implements this protocol following
Microsoft's specification, I'd rather not have a custom vendor code when
we can have a "standardized" one.
[...]
Cheers,
Benjamin
[0] https://lore.kernel.org/linux-input/1347630103-4105-1-git-send-email-benjamin.tissoires@gmail.com/
[1] https://lore.kernel.org/linux-input/1357650332-30031-1-git-send-email-mika.westerberg@linux.intel.com/
[2] https://lore.kernel.org/linux-input/1371109835-30796-1-git-send-email-benjamin.tissoires@redhat.com/
^ permalink raw reply
* Re: [PATCH v4 15/17] HID: amd_sfh: rename float_to_int() to amd_sfh_float_to_int()
From: Ilpo Järvinen @ 2023-10-18 8:15 UTC (permalink / raw)
To: Shyam Sundar S K
Cc: Hans de Goede, markgross, Basavaraj Natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel, Patil.Reddy, mario.limonciello,
platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <20231018070241.2041529-16-Shyam-sundar.S-k@amd.com>
[-- Attachment #1: Type: text/plain, Size: 5104 bytes --]
On Wed, 18 Oct 2023, Shyam Sundar S K wrote:
> From: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
>
> Current amd_sfh driver has float_to_int() to convert units from
> float to int. This is fine until this function gets called outside of
> the current scope of file.
>
> Add a prefix "amd_sfh" to float_to_int() so that function represents
> the driver name. This function will be called by multiple callers in the
> next patch.
>
> 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/sfh1_1/amd_sfh_desc.c | 22 +++++++++----------
> .../amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 1 +
> 2 files changed, 12 insertions(+), 11 deletions(-)
>
> 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..dbc8c6943ca1 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 amd_sfh_float_to_int(u32 flt32_val)
> {
> int fraction, shift, mantissa, sign, exp, zeropre;
>
> @@ -200,9 +200,9 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
> OFFSET_SENSOR_DATA_DEFAULT;
> memcpy_fromio(&accel_data, sensoraddr, sizeof(struct sfh_accel_data));
> get_common_inputs(&acc_input.common_property, report_id);
> - acc_input.in_accel_x_value = float_to_int(accel_data.acceldata.x) / 100;
> - acc_input.in_accel_y_value = float_to_int(accel_data.acceldata.y) / 100;
> - acc_input.in_accel_z_value = float_to_int(accel_data.acceldata.z) / 100;
> + acc_input.in_accel_x_value = amd_sfh_float_to_int(accel_data.acceldata.x) / 100;
> + acc_input.in_accel_y_value = amd_sfh_float_to_int(accel_data.acceldata.y) / 100;
> + acc_input.in_accel_z_value = amd_sfh_float_to_int(accel_data.acceldata.z) / 100;
> memcpy(input_report, &acc_input, sizeof(acc_input));
> report_size = sizeof(acc_input);
> break;
> @@ -211,9 +211,9 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
> OFFSET_SENSOR_DATA_DEFAULT;
> memcpy_fromio(&gyro_data, sensoraddr, sizeof(struct sfh_gyro_data));
> get_common_inputs(&gyro_input.common_property, report_id);
> - gyro_input.in_angel_x_value = float_to_int(gyro_data.gyrodata.x) / 1000;
> - gyro_input.in_angel_y_value = float_to_int(gyro_data.gyrodata.y) / 1000;
> - gyro_input.in_angel_z_value = float_to_int(gyro_data.gyrodata.z) / 1000;
> + gyro_input.in_angel_x_value = amd_sfh_float_to_int(gyro_data.gyrodata.x) / 1000;
> + gyro_input.in_angel_y_value = amd_sfh_float_to_int(gyro_data.gyrodata.y) / 1000;
> + gyro_input.in_angel_z_value = amd_sfh_float_to_int(gyro_data.gyrodata.z) / 1000;
> memcpy(input_report, &gyro_input, sizeof(gyro_input));
> report_size = sizeof(gyro_input);
> break;
> @@ -222,9 +222,9 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
> OFFSET_SENSOR_DATA_DEFAULT;
> memcpy_fromio(&mag_data, sensoraddr, sizeof(struct sfh_mag_data));
> get_common_inputs(&magno_input.common_property, report_id);
> - magno_input.in_magno_x = float_to_int(mag_data.magdata.x) / 100;
> - magno_input.in_magno_y = float_to_int(mag_data.magdata.y) / 100;
> - magno_input.in_magno_z = float_to_int(mag_data.magdata.z) / 100;
> + magno_input.in_magno_x = amd_sfh_float_to_int(mag_data.magdata.x) / 100;
> + magno_input.in_magno_y = amd_sfh_float_to_int(mag_data.magdata.y) / 100;
> + magno_input.in_magno_z = amd_sfh_float_to_int(mag_data.magdata.z) / 100;
> magno_input.in_magno_accuracy = mag_data.accuracy / 100;
> memcpy(input_report, &magno_input, sizeof(magno_input));
> report_size = sizeof(magno_input);
> @@ -234,7 +234,7 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
> OFFSET_SENSOR_DATA_DEFAULT;
> memcpy_fromio(&als_data, sensoraddr, sizeof(struct sfh_als_data));
> get_common_inputs(&als_input.common_property, report_id);
> - als_input.illuminance_value = float_to_int(als_data.lux);
> + als_input.illuminance_value = amd_sfh_float_to_int(als_data.lux);
> report_size = sizeof(als_input);
> memcpy(input_report, &als_input, sizeof(als_input));
> break;
> 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..78e22850417a 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
> @@ -151,4 +151,5 @@ struct hpd_status {
>
> void sfh_interface_init(struct amd_mp2_dev *mp2);
> void amd_sfh1_1_set_desc_ops(struct amd_mp2_ops *mp2_ops);
> +int amd_sfh_float_to_int(u32 flt32_val);
> #endif
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply
* Re: [PATCH v4 16/17] platform/x86/amd/pmf: Add PMF-AMDSFH interface for HPD
From: Ilpo Järvinen @ 2023-10-18 8:29 UTC (permalink / raw)
To: Shyam Sundar S K
Cc: Hans de Goede, markgross, Basavaraj Natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel, Patil.Reddy, mario.limonciello,
platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <20231018070241.2041529-17-Shyam-sundar.S-k@amd.com>
On Wed, 18 Oct 2023, Shyam Sundar S K wrote:
> From: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
>
> AMDSFH has information about the User presence information via the Human
> Presence Detection (HPD) sensor which is part of the AMD sensor fusion hub.
> Add PMF and AMDSFH interface to get this information.
>
> Co-developed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> ---
> drivers/hid/amd-sfh-hid/amd_sfh_common.h | 5 ++++
> drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 11 ++++++++
> .../amd-sfh-hid/sfh1_1/amd_sfh_interface.c | 28 +++++++++++++++++++
> drivers/platform/x86/amd/pmf/Kconfig | 1 +
> drivers/platform/x86/amd/pmf/spc.c | 21 ++++++++++++++
> include/linux/amd-pmf-io.h | 20 ++++++++++++-
> 6 files changed, 85 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> index 2643bb14fee2..cd57037bf217 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> @@ -37,6 +37,10 @@ struct amd_mp2_sensor_info {
> dma_addr_t dma_address;
> };
>
> +struct sfh_dev_status {
> + bool is_hpd_present;
> +};
> +
> struct amd_mp2_dev {
> struct pci_dev *pdev;
> struct amdtp_cl_data *cl_data;
> @@ -47,6 +51,7 @@ struct amd_mp2_dev {
> struct amd_input_data in_data;
> /* mp2 active control status */
> u32 mp2_acs;
> + struct sfh_dev_status dev_en;
> };
>
> struct amd_mp2_ops {
> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
> index e9c6413af24a..3dc652d41d7d 100644
> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
> @@ -73,6 +73,12 @@ static int amd_sfh_hid_client_deinit(struct amd_mp2_dev *privdata)
> int i, status;
>
> for (i = 0; i < cl_data->num_hid_devices; i++) {
> + switch (cl_data->sensor_idx[i]) {
> + case HPD_IDX:
> + privdata->dev_en.is_hpd_present = false;
> + break;
> + }
> +
> if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
> privdata->mp2_ops->stop(privdata, cl_data->sensor_idx[i]);
> status = amd_sfh_wait_for_response
> @@ -178,6 +184,11 @@ static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata)
> rc = amdtp_hid_probe(i, cl_data);
> if (rc)
> goto cleanup;
> + switch (cl_data->sensor_idx[i]) {
> + case HPD_IDX:
> + privdata->dev_en.is_hpd_present = true;
> + break;
> + }
> }
> dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
> cl_data->sensor_idx[i], get_sensor_name(cl_data->sensor_idx[i]),
> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
> index 4f81ef2d4f56..7637da0dec6f 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;
Reverse the logic and early return with -ENODEV. Make the same change to
the next patch too.
--
i.
> +}
> +
> +int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op)
> +{
> + if (sfh_info) {
> + switch (op) {
> + case MT_HPD:
> + return amd_sfh_hpd_info(&sfh_info->user_present);
> + }
> + }
> + return -EINVAL;
> }
> +EXPORT_SYMBOL_GPL(amd_get_sfh_info);
> diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
> index 7f430de7af44..d368d35a49ac 100644
> --- a/drivers/platform/x86/amd/pmf/Kconfig
> +++ b/drivers/platform/x86/amd/pmf/Kconfig
> @@ -11,6 +11,7 @@ config AMD_PMF
> select ACPI_PLATFORM_PROFILE
> depends on TEE && AMDTEE
> depends on DRM_AMDGPU
> + depends on AMD_SFH_HID
> help
> This driver provides support for the AMD Platform Management Framework.
> The goal is to enhance end user experience by making AMD PCs smarter,
> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> index cf4962ef97c2..8bdfb395f316 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 *
> 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
> @@ -158,6 +159,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 */
> @@ -167,4 +187,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 5f79e66a41b3..76e42552b62c 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
> @@ -32,4 +33,21 @@ struct amd_gpu_pmf_data {
> int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
> int amd_pmf_gpu_init(struct amd_gpu_pmf_data *pmf);
> void amd_pmf_gpu_deinit(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;
> +};
> +
> +int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op);
> #endif
>
^ permalink raw reply
* Re: [PATCH v4 10/17] platform/x86/amd/pmf: Add facility to dump TA inputs
From: Ilpo Järvinen @ 2023-10-18 9:00 UTC (permalink / raw)
To: Shyam Sundar S K
Cc: Hans de Goede, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel, Patil.Reddy, mario.limonciello,
platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <20231018070241.2041529-11-Shyam-sundar.S-k@amd.com>
On Wed, 18 Oct 2023, Shyam Sundar S K wrote:
> 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.
>
> Also, make source_as_str() as non-static function as this helper is
> required outside of sps.c file.
>
> 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/tee-if.c | 1 +
> 3 files changed, 41 insertions(+)
>
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 216a9f795436..593930519039 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -602,6 +602,7 @@ bool is_pprof_balanced(struct amd_pmf_dev *pmf);
> int amd_pmf_power_slider_update_event(struct amd_pmf_dev *dev);
> const char *amd_pmf_source_as_str(unsigned int state);
>
> +const char *amd_pmf_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);
> @@ -632,4 +633,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 bd5061fdfdbe..512e0c66efdc 100644
> --- a/drivers/platform/x86/amd/pmf/spc.c
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -14,6 +14,43 @@
> #include <linux/units.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", amd_pmf_source_as_str(in->ev_info.power_source));
> + dev_dbg(dev->dev, "Battery Percentage : %u\n", in->ev_info.bat_percentage);
> + dev_dbg(dev->dev, "Designed Battery Capacity : %u\n", in->ev_info.bat_design);
> + dev_dbg(dev->dev, "Fully Charged Capacity : %u\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 : %u\n", in->ev_info.socket_power);
> + dev_dbg(dev->dev, "Skin Temperature : %u\n", in->ev_info.skin_temperature);
> + dev_dbg(dev->dev, "Avg C0 Residency : %u\n", in->ev_info.avg_c0residency);
> + dev_dbg(dev->dev, "Max C0 Residency : %u\n", in->ev_info.max_c0residency);
> + dev_dbg(dev->dev, "GFX Busy : %u\n", in->ev_info.gfx_busy);
> + dev_dbg(dev->dev, "Connected Display Count : %u\n", in->ev_info.monitor_count);
> + dev_dbg(dev->dev, "LID State : %s\n", in->ev_info.lid_state ? "Close" : "Open");
"open" / "closed" is generic enough that I think it would warrant adding
include/linux/string_choices.h helper for it (it should be lowercase
there but that difference probably is not an issue for these debug
prints).
I'd also remove that extra space before :.
--
i.
> + 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/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index d48f980fb1db..0eba258f4040 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -182,6 +182,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:%u result:%x\n", out->actions_count,
> ta_sm->pmf_result);
> amd_pmf_apply_policies(dev, out);
>
^ permalink raw reply
* Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
From: Ilpo Järvinen @ 2023-10-18 9:20 UTC (permalink / raw)
To: Shyam Sundar S K
Cc: Hans de Goede, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel, Patil.Reddy, mario.limonciello,
platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <20231018070241.2041529-14-Shyam-sundar.S-k@amd.com>
On Wed, 18 Oct 2023, Shyam Sundar S K wrote:
> 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 | 138 ++++++++++++++++++++++++
> drivers/platform/x86/amd/pmf/Kconfig | 1 +
> drivers/platform/x86/amd/pmf/core.c | 1 +
> drivers/platform/x86/amd/pmf/pmf.h | 3 +
> drivers/platform/x86/amd/pmf/spc.c | 13 +++
> drivers/platform/x86/amd/pmf/tee-if.c | 26 +++++
> include/linux/amd-pmf-io.h | 35 ++++++
> 9 files changed, 220 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 a79d53bdbe13..26ffa1b4fe57 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..ac981848df50
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> @@ -0,0 +1,138 @@
> +/*
> + * 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.
This is MIT, right? Add the required SPDX-License-Identifier line for it
at the top of the file, thank you.
> + *
> + * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> + *
> + */
Remove the extra empty line at the end of the comment.
> +
> +#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;
> + 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;
> + }
> +
> + mutex_lock(&mode_config->mutex);
> + drm_connector_list_iter_begin(drm_dev, &iter);
> + drm_for_each_connector_iter(connector, &iter) {
> + if (connector->status == connector_status_connected)
> + pmf->display_count++;
> + if (connector->status != pmf->con_status[i])
> + pmf->con_status[i] = connector->status;
> + if (connector->connector_type != pmf->connector_type[i])
> + pmf->connector_type[i] = connector->connector_type;
> +
> + i++;
> + if (i >= MAX_SUPPORTED)
> + break;
> + }
> + drm_connector_list_iter_end(&iter);
> + mutex_unlock(&mode_config->mutex);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
> +
> +static int amd_pmf_gpu_get_cur_state(struct thermal_cooling_device *cooling_dev,
> + unsigned long *state)
> +{
> + struct backlight_device *bd;
> +
> + if (!acpi_video_backlight_use_native())
> + return -ENODEV;
> +
> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
> + if (!bd)
> + return -ENODEV;
> +
> + *state = backlight_get_brightness(bd);
> +
> + return 0;
> +}
> +
> +static int amd_pmf_gpu_get_max_state(struct thermal_cooling_device *cooling_dev,
> + unsigned long *state)
> +{
> + struct backlight_device *bd;
> +
> + if (!acpi_video_backlight_use_native())
> + return -ENODEV;
> +
> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
> + if (!bd)
> + return -ENODEV;
> +
> + if (backlight_is_blank(bd))
> + *state = 0;
> + else
> + *state = bd->props.max_brightness;
> +
> + return 0;
> +}
> +
> +static const struct thermal_cooling_device_ops bd_cooling_ops = {
> + .get_max_state = amd_pmf_gpu_get_max_state,
> + .get_cur_state = amd_pmf_gpu_get_cur_state,
> +};
> +
> +int amd_pmf_gpu_init(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);
> +
> + if (!(adev->flags & AMD_IS_APU)) {
> + DRM_ERROR("PMF-AMDGPU interface not supported\n");
> + return -ENODEV;
> + }
> +
> + if (!acpi_video_backlight_use_native())
> + return -ENODEV;
> +
> + pmf->raw_bd = backlight_device_get_by_type(BACKLIGHT_RAW);
> + if (!pmf->raw_bd)
> + return -ENODEV;
> +
> + pmf->cooling_dev = thermal_cooling_device_register("pmf_gpu_bd",
> + pmf, &bd_cooling_ops);
> + if (IS_ERR(pmf->cooling_dev))
> + return -ENODEV;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(amd_pmf_gpu_init);
> +
> +void amd_pmf_gpu_deinit(struct amd_gpu_pmf_data *pmf)
> +{
> + thermal_cooling_device_unregister(pmf->cooling_dev);
> +}
> +EXPORT_SYMBOL_GPL(amd_pmf_gpu_deinit);
> diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
> index f246252bddd8..7f430de7af44 100644
> --- a/drivers/platform/x86/amd/pmf/Kconfig
> +++ b/drivers/platform/x86/amd/pmf/Kconfig
> @@ -10,6 +10,7 @@ config AMD_PMF
> depends on AMD_NB
> select ACPI_PLATFORM_PROFILE
> depends on TEE && AMDTEE
> + depends on 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 4b8156033fa6..c59ba527ff49 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -411,6 +411,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 8712299ad52b..615cd3a31986 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -13,6 +13,7 @@
>
> #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
> @@ -228,9 +229,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 512e0c66efdc..cf4962ef97c2 100644
> --- a/drivers/platform/x86/amd/pmf/spc.c
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -44,6 +44,10 @@ void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *
> dev_dbg(dev->dev, "Max C0 Residency : %u\n", in->ev_info.max_c0residency);
> dev_dbg(dev->dev, "GFX Busy : %u\n", in->ev_info.gfx_busy);
> dev_dbg(dev->dev, "Connected Display Count : %u\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");
> }
> @@ -146,6 +150,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 */
> @@ -154,4 +166,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 2f5fb8623c20..956e66b78605 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"
> @@ -357,6 +358,19 @@ 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) {
> + /* Found the amdgpu handle from the pci root after walking through the pci bus */
If you insist on having this comment, make it a function comment instead
(with appropriate modifications into the content of it) but I personally
don't find it that useful so it could be just dropped as well, IMO.
> + dev->gfx_data.gpu_dev = 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;
> @@ -446,6 +460,15 @@ 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 */
Useless comment.
> + 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");
> +
> + if (!amd_pmf_gpu_init(&dev->gfx_data))
> + dev->gfx_data.gpu_dev_en = true;
> +
> dev->prev_data = kzalloc(sizeof(*dev->prev_data), GFP_KERNEL);
> if (!dev->prev_data)
> return -ENOMEM;
> @@ -461,5 +484,8 @@ 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);
> + if (dev->gfx_data.gpu_dev_en)
> + amd_pmf_gpu_deinit(&dev->gfx_data);
> + 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..5f79e66a41b3
> --- /dev/null
> +++ b/include/linux/amd-pmf-io.h
> @@ -0,0 +1,35 @@
> +/* 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 <acpi/video.h>
> +#include <drm/drm_connector.h>
> +#include <linux/backlight.h>
> +#include <linux/thermal.h>
> +
> +#define MAX_SUPPORTED 4
> +
> +/* amdgpu */
Document the structure properly with kerneldoc instead of an unhelpful
comment like above :-). Please also check if you add any other structs
into kernel-wide headers that you didn't document yet. Or fields into
existing structs.
> +struct amd_gpu_pmf_data {
> + struct pci_dev *gpu_dev;
> + struct backlight_device *raw_bd;
> + struct thermal_cooling_device *cooling_dev;
> + enum drm_connector_status con_status[MAX_SUPPORTED];
> + int display_count;
> + int connector_type[MAX_SUPPORTED];
> + bool gpu_dev_en;
> +};
> +
> +int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
> +int amd_pmf_gpu_init(struct amd_gpu_pmf_data *pmf);
> +void amd_pmf_gpu_deinit(struct amd_gpu_pmf_data *pmf);
> +#endif
>
--
i.
^ permalink raw reply
* Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
From: Shyam Sundar S K @ 2023-10-18 9:28 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Hans de Goede, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel, Patil.Reddy, mario.limonciello,
platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <9e6c4a42-fbce-c5ea-15ce-5eb22fc3767a@linux.intel.com>
On 10/18/2023 2:50 PM, Ilpo Järvinen wrote:
> On Wed, 18 Oct 2023, Shyam Sundar S K wrote:
>
>> 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 | 138 ++++++++++++++++++++++++
>> drivers/platform/x86/amd/pmf/Kconfig | 1 +
>> drivers/platform/x86/amd/pmf/core.c | 1 +
>> drivers/platform/x86/amd/pmf/pmf.h | 3 +
>> drivers/platform/x86/amd/pmf/spc.c | 13 +++
>> drivers/platform/x86/amd/pmf/tee-if.c | 26 +++++
>> include/linux/amd-pmf-io.h | 35 ++++++
>> 9 files changed, 220 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 a79d53bdbe13..26ffa1b4fe57 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..ac981848df50
>> --- /dev/null
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>> @@ -0,0 +1,138 @@
>> +/*
>> + * 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.
>
> This is MIT, right? Add the required SPDX-License-Identifier line for it
> at the top of the file, thank you.
>
all the files in drivers/gpu/drm/amd/amdgpu/* carry the same license
text. So, have retained it to maintain uniformity.
>> + *
>> + * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> + *
>> + */
>
> Remove the extra empty line at the end of the comment.
>
I just took the standard template for all the gpu files. Is that OK to
retain the blank line?
If not, I can remove it in the next version.
Rest all remarks I will address.
Thanks,
Shyam
>> +
>> +#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;
>> + 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;
>> + }
>> +
>> + mutex_lock(&mode_config->mutex);
>> + drm_connector_list_iter_begin(drm_dev, &iter);
>> + drm_for_each_connector_iter(connector, &iter) {
>> + if (connector->status == connector_status_connected)
>> + pmf->display_count++;
>> + if (connector->status != pmf->con_status[i])
>> + pmf->con_status[i] = connector->status;
>> + if (connector->connector_type != pmf->connector_type[i])
>> + pmf->connector_type[i] = connector->connector_type;
>> +
>> + i++;
>> + if (i >= MAX_SUPPORTED)
>> + break;
>> + }
>> + drm_connector_list_iter_end(&iter);
>> + mutex_unlock(&mode_config->mutex);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
>> +
>> +static int amd_pmf_gpu_get_cur_state(struct thermal_cooling_device *cooling_dev,
>> + unsigned long *state)
>> +{
>> + struct backlight_device *bd;
>> +
>> + if (!acpi_video_backlight_use_native())
>> + return -ENODEV;
>> +
>> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>> + if (!bd)
>> + return -ENODEV;
>> +
>> + *state = backlight_get_brightness(bd);
>> +
>> + return 0;
>> +}
>> +
>> +static int amd_pmf_gpu_get_max_state(struct thermal_cooling_device *cooling_dev,
>> + unsigned long *state)
>> +{
>> + struct backlight_device *bd;
>> +
>> + if (!acpi_video_backlight_use_native())
>> + return -ENODEV;
>> +
>> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>> + if (!bd)
>> + return -ENODEV;
>> +
>> + if (backlight_is_blank(bd))
>> + *state = 0;
>> + else
>> + *state = bd->props.max_brightness;
>> +
>> + return 0;
>> +}
>> +
>> +static const struct thermal_cooling_device_ops bd_cooling_ops = {
>> + .get_max_state = amd_pmf_gpu_get_max_state,
>> + .get_cur_state = amd_pmf_gpu_get_cur_state,
>> +};
>> +
>> +int amd_pmf_gpu_init(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);
>> +
>> + if (!(adev->flags & AMD_IS_APU)) {
>> + DRM_ERROR("PMF-AMDGPU interface not supported\n");
>> + return -ENODEV;
>> + }
>> +
>> + if (!acpi_video_backlight_use_native())
>> + return -ENODEV;
>> +
>> + pmf->raw_bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>> + if (!pmf->raw_bd)
>> + return -ENODEV;
>> +
>> + pmf->cooling_dev = thermal_cooling_device_register("pmf_gpu_bd",
>> + pmf, &bd_cooling_ops);
>> + if (IS_ERR(pmf->cooling_dev))
>> + return -ENODEV;
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(amd_pmf_gpu_init);
>> +
>> +void amd_pmf_gpu_deinit(struct amd_gpu_pmf_data *pmf)
>> +{
>> + thermal_cooling_device_unregister(pmf->cooling_dev);
>> +}
>> +EXPORT_SYMBOL_GPL(amd_pmf_gpu_deinit);
>> diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
>> index f246252bddd8..7f430de7af44 100644
>> --- a/drivers/platform/x86/amd/pmf/Kconfig
>> +++ b/drivers/platform/x86/amd/pmf/Kconfig
>> @@ -10,6 +10,7 @@ config AMD_PMF
>> depends on AMD_NB
>> select ACPI_PLATFORM_PROFILE
>> depends on TEE && AMDTEE
>> + depends on 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 4b8156033fa6..c59ba527ff49 100644
>> --- a/drivers/platform/x86/amd/pmf/core.c
>> +++ b/drivers/platform/x86/amd/pmf/core.c
>> @@ -411,6 +411,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 8712299ad52b..615cd3a31986 100644
>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>> @@ -13,6 +13,7 @@
>>
>> #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
>> @@ -228,9 +229,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 512e0c66efdc..cf4962ef97c2 100644
>> --- a/drivers/platform/x86/amd/pmf/spc.c
>> +++ b/drivers/platform/x86/amd/pmf/spc.c
>> @@ -44,6 +44,10 @@ void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *
>> dev_dbg(dev->dev, "Max C0 Residency : %u\n", in->ev_info.max_c0residency);
>> dev_dbg(dev->dev, "GFX Busy : %u\n", in->ev_info.gfx_busy);
>> dev_dbg(dev->dev, "Connected Display Count : %u\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");
>> }
>> @@ -146,6 +150,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 */
>> @@ -154,4 +166,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 2f5fb8623c20..956e66b78605 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"
>> @@ -357,6 +358,19 @@ 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) {
>> + /* Found the amdgpu handle from the pci root after walking through the pci bus */
>
> If you insist on having this comment, make it a function comment instead
> (with appropriate modifications into the content of it) but I personally
> don't find it that useful so it could be just dropped as well, IMO.
>
>> + dev->gfx_data.gpu_dev = 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;
>> @@ -446,6 +460,15 @@ 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 */
>
> Useless comment.
>
>> + 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");
>> +
>> + if (!amd_pmf_gpu_init(&dev->gfx_data))
>> + dev->gfx_data.gpu_dev_en = true;
>> +
>> dev->prev_data = kzalloc(sizeof(*dev->prev_data), GFP_KERNEL);
>> if (!dev->prev_data)
>> return -ENOMEM;
>> @@ -461,5 +484,8 @@ 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);
>> + if (dev->gfx_data.gpu_dev_en)
>> + amd_pmf_gpu_deinit(&dev->gfx_data);
>> + 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..5f79e66a41b3
>> --- /dev/null
>> +++ b/include/linux/amd-pmf-io.h
>> @@ -0,0 +1,35 @@
>> +/* 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 <acpi/video.h>
>> +#include <drm/drm_connector.h>
>> +#include <linux/backlight.h>
>> +#include <linux/thermal.h>
>> +
>> +#define MAX_SUPPORTED 4
>> +
>> +/* amdgpu */
>
> Document the structure properly with kerneldoc instead of an unhelpful
> comment like above :-). Please also check if you add any other structs
> into kernel-wide headers that you didn't document yet. Or fields into
> existing structs.
>
>> +struct amd_gpu_pmf_data {
>> + struct pci_dev *gpu_dev;
>> + struct backlight_device *raw_bd;
>> + struct thermal_cooling_device *cooling_dev;
>> + enum drm_connector_status con_status[MAX_SUPPORTED];
>> + int display_count;
>> + int connector_type[MAX_SUPPORTED];
>> + bool gpu_dev_en;
>> +};
>> +
>> +int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
>> +int amd_pmf_gpu_init(struct amd_gpu_pmf_data *pmf);
>> +void amd_pmf_gpu_deinit(struct amd_gpu_pmf_data *pmf);
>> +#endif
>>
>
^ permalink raw reply
* Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
From: Ilpo Järvinen @ 2023-10-18 9:37 UTC (permalink / raw)
To: Shyam Sundar S K
Cc: Hans de Goede, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel, Patil.Reddy, mario.limonciello,
platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <84af64f2-42bd-4e09-a1c9-bde2a935c8f2@amd.com>
[-- Attachment #1: Type: text/plain, Size: 4635 bytes --]
On Wed, 18 Oct 2023, Shyam Sundar S K wrote:
> On 10/18/2023 2:50 PM, Ilpo Järvinen wrote:
> > On Wed, 18 Oct 2023, Shyam Sundar S K wrote:
> >
> >> 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 | 138 ++++++++++++++++++++++++
> >> drivers/platform/x86/amd/pmf/Kconfig | 1 +
> >> drivers/platform/x86/amd/pmf/core.c | 1 +
> >> drivers/platform/x86/amd/pmf/pmf.h | 3 +
> >> drivers/platform/x86/amd/pmf/spc.c | 13 +++
> >> drivers/platform/x86/amd/pmf/tee-if.c | 26 +++++
> >> include/linux/amd-pmf-io.h | 35 ++++++
> >> 9 files changed, 220 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 a79d53bdbe13..26ffa1b4fe57 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..ac981848df50
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> >> @@ -0,0 +1,138 @@
> >> +/*
> >> + * 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.
> >
> > This is MIT, right? Add the required SPDX-License-Identifier line for it
> > at the top of the file, thank you.
> >
>
> all the files in drivers/gpu/drm/amd/amdgpu/* carry the same license
> text. So, have retained it to maintain uniformity.
What does adding SPDX identifier line at the top of the file take away
from that? I'd be fine if you want to add the identifier line to all of
them too to keep them identical.
> >> + *
> >> + * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> >> + *
> >> + */
> >
> > Remove the extra empty line at the end of the comment.
> >
>
> I just took the standard template for all the gpu files. Is that OK to
> retain the blank line?
>
> If not, I can remove it in the next version.
I don't want to fight over a blank line when you insist on keeping it :-).
--
i.
^ permalink raw reply
* Re: [PATCH v4 12/17] platform/x86/amd/pmf: dump policy binary data
From: Ilpo Järvinen @ 2023-10-18 10:09 UTC (permalink / raw)
To: Shyam Sundar S K
Cc: Hans de Goede, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel, Patil.Reddy, mario.limonciello,
platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <20231018070241.2041529-13-Shyam-sundar.S-k@amd.com>
[-- Attachment #1: Type: text/plain, Size: 2148 bytes --]
On Wed, 18 Oct 2023, 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.
>
> 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/tee-if.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 6c4ce22ba518..2f5fb8623c20 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -276,6 +276,12 @@ static int amd_pmf_start_policy_engine(struct amd_pmf_dev *dev)
> }
>
> #ifdef CONFIG_AMD_PMF_DEBUG
> +static void amd_pmf_hex_dump_pb(struct amd_pmf_dev *dev)
> +{
> + print_hex_dump_debug("(pb): ", DUMP_PREFIX_OFFSET, 16, 1, dev->policy_buf,
> + dev->policy_sz, false);
> +}
> +
> static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
> size_t length, loff_t *pos)
> {
> @@ -290,6 +296,7 @@ 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;
>
> + amd_pmf_hex_dump_pb(dev);
> ret = amd_pmf_start_policy_engine(dev);
> if (ret)
> return -EINVAL;
> @@ -328,6 +335,7 @@ static int amd_pmf_open_pb(struct amd_pmf_dev *dev, struct dentry *debugfs_root)
> }
>
> static void amd_pmf_remove_pb(struct amd_pmf_dev *dev) {}
> +static void amd_pmf_hex_dump_pb(struct amd_pmf_dev *dev) {}
> #endif
>
> static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
> @@ -342,6 +350,7 @@ static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
>
> memcpy(dev->policy_buf, dev->policy_base, dev->policy_sz);
>
> + amd_pmf_hex_dump_pb(dev);
> if (pb_side_load)
> amd_pmf_open_pb(dev, dev->dbgfs_dir);
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply
* Re: [PATCH v3 2/4] HID: touchscreen: Add initial support for Himax HID-over-SPI
From: kernel test robot @ 2023-10-18 10:14 UTC (permalink / raw)
To: Tylor Yang, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
conor+dt, jikos, benjamin.tissoires, linux-input, devicetree,
linux-kernel
Cc: oe-kbuild-all, poyuan_chang, jingyliang, hbarnor, wuxy23, luolm1,
poyu_hung, Tylor Yang
In-Reply-To: <20231017091900.801989-3-tylor_yang@himax.corp-partner.google.com>
Hi Tylor,
kernel test robot noticed the following build warnings:
[auto build test WARNING on hid/for-next]
[also build test WARNING on dtor-input/next dtor-input/for-linus robh/for-next linus/master v6.6-rc6 next-20231018]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Tylor-Yang/dt-bindings-input-Introduce-Himax-HID-over-SPI-device/20231017-172156
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20231017091900.801989-3-tylor_yang%40himax.corp-partner.google.com
patch subject: [PATCH v3 2/4] HID: touchscreen: Add initial support for Himax HID-over-SPI
reproduce: (https://download.01.org/0day-ci/archive/20231018/202310181806.Ne95k5nt-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310181806.Ne95k5nt-lkp@intel.com/
versioncheck warnings: (new ones prefixed by >>)
INFO PATH=/opt/cross/clang/bin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
/usr/bin/timeout -k 100 3h /usr/bin/make KCFLAGS= -Wrestrict -Warray-bounds -Wformat-overflow -Wformat-truncation -Wstringop-overflow -Wundef -funsigned-char -Wenum-conversion -Werror=return-type W=1 --keep-going HOSTCC=gcc-12 CC=gcc-12 -j32 KBUILD_MODPOST_WARN=1 ARCH=x86_64 versioncheck
find ./* \( -name SCCS -o -name BitKeeper -o -name .svn -o -name CVS -o -name .pc -o -name .hg -o -name .git \) -prune -o \
-name '*.[hcS]' -type f -print | sort \
| xargs perl -w ./scripts/checkversion.pl
./drivers/accessibility/speakup/genmap.c: 13 linux/version.h not needed.
./drivers/accessibility/speakup/makemapdata.c: 13 linux/version.h not needed.
>> ./drivers/hid/hx-hid/hx_core.h: 25 linux/version.h not needed.
./drivers/staging/media/atomisp/include/linux/atomisp.h: 23 linux/version.h not needed.
./samples/bpf/spintest.bpf.c: 8 linux/version.h not needed.
./samples/trace_events/trace_custom_sched.c: 11 linux/version.h not needed.
./sound/soc/codecs/cs42l42.c: 14 linux/version.h not needed.
./tools/lib/bpf/bpf_helpers.h: 402: need linux/version.h
./tools/testing/selftests/bpf/progs/dev_cgroup.c: 9 linux/version.h not needed.
./tools/testing/selftests/bpf/progs/netcnt_prog.c: 3 linux/version.h not needed.
./tools/testing/selftests/bpf/progs/test_map_lock.c: 4 linux/version.h not needed.
./tools/testing/selftests/bpf/progs/test_send_signal_kern.c: 4 linux/version.h not needed.
./tools/testing/selftests/bpf/progs/test_spin_lock.c: 4 linux/version.h not needed.
./tools/testing/selftests/bpf/progs/test_tcp_estats.c: 37 linux/version.h not needed.
./tools/testing/selftests/wireguard/qemu/init.c: 27 linux/version.h not needed.
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH 0/3] Input: atkbd - add skip_commands module parameter
From: Hans de Goede @ 2023-10-18 10:19 UTC (permalink / raw)
To: Shang Ye; +Cc: Dmitry Torokhov, linux-input
In-Reply-To: <886D6167733841AE+20231017135318.11142-1-yesh25@mail2.sysu.edu.cn>
Hi Shang,
On 10/17/23 15:53, Shang Ye wrote:
> Hi Hans,
>
> I very much support the inclusion of this patch, because there has been
> a similar keyboard issue on at least 3 (presumably 9) types of Lenovo
> laptops, which may also be avoided by simply skipping the GETID command.
> My patch and a list of the affected laptop types may be found at:
> https://github.com/yescallop/atkbd-nogetid
>
> In my last patch submission, I have included the issue details:
> https://lore.kernel.org/linux-input/20230530131340.39961-1-yesh25@mail2.sysu.edu.cn/
>
> There were also two other patch submissions aimed at enabling
> `i8042.dumbkbd` on some HP laptops in order to avoid sending the GETID
> command, which isn't very desirable because it breaks the Caps Lock LED:
> https://lore.kernel.org/linux-input/2iAJTwqZV6lQs26cTb38RNYqxvsink6SRmrZ5h0cBUSuf9NT0tZTsf9fEAbbto2maavHJEOP8GA1evlKa6xjKOsaskDhtJWxjcnrgPigzVo=@gurevit.ch/
> https://lore.kernel.org/linux-input/20210609073333.8425-1-egori@altlinux.org/
>
> And another patch submisson aimed at fixing the issue generically,
> which, sadly, did not work on my laptop because the GETID command would
> trigger more errornous behaviours on it:
> https://lore.kernel.org/linux-input/20210201160336.16008-1-anton@cpp.in/
Interesting, that might be the issue which is hitting the HP models
which I wrote this series for too.
> I hope that these materials will help people better understand the
> nature of the issue and the urgency to address it.
>
> Below are some comments on the patch:
>
>> +MODULE_PARM_DESC(skip_commands, "Bitfield where each bits skips a specific keyboard cmd (0 - 0x3f)");
>
> "bits" -> "bit"?
Indeed, if we go with this patch-series this should be fixed.
> I think we may also need to document the new module parameter at
> Documentation/admin-guide/kernel-parameters.txt and clarify which bit
> skips which keyboard command.
>
> Lastly, would you think it is appropriate to include in this patch
> series the quirks for Lenovo laptops on which my patch was tested to
> work? If so, the quirk table entries would be:
>
> System vendor: "LENOVO"
> Product names: "82G2", "82NC", "82TK"
> Driver data : ATKBD_SKIP_GETID
Looking at your github and seeing how many models are affected,
I'm thinking that we should maybe just skip the entire keyboard
atkbd_probe() when atkbd->translated is set.
The probe is really only necessary in the untranslated case
to check if there is a mouse there or if there is one of the
(quite old) special ps/2 keyboards there which have some
special handling (search for "id == 0x" to find the special
cases) these special cases are all only hit/valid when
(atkbd->translated == 0) is true, so when atkbd->translated
is true we can just skip the probe and use an assumed id of 0xab00
(already used when i8042.dumbkbd is set) and then immediately bail
from atkbd_probe(). I think this generic solution is a better
approach then any of the previous approaches since it is nice
and KISS and does not rely on any DMI quirks.
Regards,
Hans
> On 2023/10/06 04:15, Hans de Goede wrote:
>> Hi all,
>>
>> While debugging a keyboard issue on some HP laptops adding i8042.dumbkbd
>> helped to avoid the issue. So one of the commands send by atkbd.c seemed
>> to be the culprit.
>>
>> This series a skip_commands option to help debug cases like this by adding
>> a bit-field which allows disabling a subset of the ps2_command()
>> calls the atkbd driver makes.
>>
>> It also replaces the existing atkbd_skip_deactivate flag
>> with the new parameter and adds a DMI quirk for the HP laptops
>> to avoid the keyboard issue there.
>>
>> Regards,
>>
>> Hans
>>
>>
>> Hans de Goede (3):
>> Input: atkbd - add skip_commands module parameter
>> Input: atkbd - drop atkbd_skip_deactivate flag
>> Input: atkbd - set skip_commands = ATKBD_SKIP_GETID for HP laptop
>> 15s-fq* laptops
>>
>> drivers/input/keyboard/atkbd.c | 88 ++++++++++++++++++++++++++--------
>> 1 file changed, 69 insertions(+), 19 deletions(-)
>>
>> --
>> 2.41.0
>>
>
^ permalink raw reply
* Re: [PATCH v4 11/17] platform/x86/amd/pmf: Add capability to sideload of policy binary
From: Ilpo Järvinen @ 2023-10-18 10:21 UTC (permalink / raw)
To: Shyam Sundar S K
Cc: Hans de Goede, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel, Patil.Reddy, mario.limonciello,
platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <20231018070241.2041529-12-Shyam-sundar.S-k@amd.com>
On Wed, 18 Oct 2023, Shyam Sundar S K wrote:
> 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 | 67 +++++++++++++++++++++++++++
> 2 files changed, 68 insertions(+)
>
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 593930519039..8712299ad52b 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -219,6 +219,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 0eba258f4040..6c4ce22ba518 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"
> @@ -16,9 +17,14 @@
>
> /* Policy binary actions sampling frequency (in ms) */
> static int pb_actions_ms = MSEC_PER_SEC;
> +/* Sideload policy binaries to debug policy failures */
> +static bool pb_side_load;
> +
> #ifdef CONFIG_AMD_PMF_DEBUG
> module_param(pb_actions_ms, int, 0644);
> MODULE_PARM_DESC(pb_actions_ms, "Policy binary actions sampling frequency (default = 1000ms)");
> +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,
> @@ -269,6 +275,61 @@ 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,
> +};
> +
> +static 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)
debugfs_create_file() returns ERR_PTR() on errors. I don't actually know
if NULL even needs to be checked or if it'd return errno in that case
because the usually custom is to just ignore debugfs_create_file()
return value.
Why is this function returning int anyway? It's not checked by the caller
so why bother when all it does is deal with debugfs for which the normal
approach is to ignore the errors.
--
i.
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static void amd_pmf_remove_pb(struct amd_pmf_dev *dev)
> +{
> + debugfs_remove_recursive(dev->esbin);
> +}
> +#else
> +static int amd_pmf_open_pb(struct amd_pmf_dev *dev, struct dentry *debugfs_root)
> +{
> + return 0;
> +}
> +
> +static void amd_pmf_remove_pb(struct amd_pmf_dev *dev) {}
> +#endif
> +
> static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
> {
> dev->policy_buf = kzalloc(dev->policy_sz, GFP_KERNEL);
> @@ -281,6 +342,9 @@ static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
>
> memcpy(dev->policy_buf, dev->policy_base, dev->policy_sz);
>
> + if (pb_side_load)
> + amd_pmf_open_pb(dev, dev->dbgfs_dir);
> +
> return amd_pmf_start_policy_engine(dev);
> }
>
> @@ -382,6 +446,9 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
>
> void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev)
> {
> + if (pb_side_load)
> + amd_pmf_remove_pb(dev);
> +
> kfree(dev->prev_data);
> kfree(dev->policy_buf);
> cancel_delayed_work_sync(&dev->pb_work);
>
^ permalink raw reply
* Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
From: Christian König @ 2023-10-18 13:40 UTC (permalink / raw)
To: Shyam Sundar S K, Ilpo Järvinen
Cc: Hans de Goede, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, Xinhui.Pan, airlied,
daniel, Patil.Reddy, mario.limonciello, platform-driver-x86,
linux-input, amd-gfx, dri-devel
In-Reply-To: <84af64f2-42bd-4e09-a1c9-bde2a935c8f2@amd.com>
Am 18.10.23 um 11:28 schrieb Shyam Sundar S K:
>
> On 10/18/2023 2:50 PM, Ilpo Järvinen wrote:
>> On Wed, 18 Oct 2023, Shyam Sundar S K wrote:
>>
>>> 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 | 138 ++++++++++++++++++++++++
>>> drivers/platform/x86/amd/pmf/Kconfig | 1 +
>>> drivers/platform/x86/amd/pmf/core.c | 1 +
>>> drivers/platform/x86/amd/pmf/pmf.h | 3 +
>>> drivers/platform/x86/amd/pmf/spc.c | 13 +++
>>> drivers/platform/x86/amd/pmf/tee-if.c | 26 +++++
>>> include/linux/amd-pmf-io.h | 35 ++++++
>>> 9 files changed, 220 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 a79d53bdbe13..26ffa1b4fe57 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..ac981848df50
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>> @@ -0,0 +1,138 @@
>>> +/*
>>> + * 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.
>> This is MIT, right? Add the required SPDX-License-Identifier line for it
>> at the top of the file, thank you.
>>
> all the files in drivers/gpu/drm/amd/amdgpu/* carry the same license
> text. So, have retained it to maintain uniformity.
Please add the SPDX License Identifier for any file you add.
Apart from that the whole approach of attaching this directly to amdgpu
looks extremely questionable to me.
Regards,
Christian.
>
>>> + *
>>> + * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>> + *
>>> + */
>> Remove the extra empty line at the end of the comment.
>>
> I just took the standard template for all the gpu files. Is that OK to
> retain the blank line?
>
> If not, I can remove it in the next version.
>
> Rest all remarks I will address.
>
> Thanks,
> Shyam
>
>>> +
>>> +#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;
>>> + 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;
>>> + }
>>> +
>>> + mutex_lock(&mode_config->mutex);
>>> + drm_connector_list_iter_begin(drm_dev, &iter);
>>> + drm_for_each_connector_iter(connector, &iter) {
>>> + if (connector->status == connector_status_connected)
>>> + pmf->display_count++;
>>> + if (connector->status != pmf->con_status[i])
>>> + pmf->con_status[i] = connector->status;
>>> + if (connector->connector_type != pmf->connector_type[i])
>>> + pmf->connector_type[i] = connector->connector_type;
>>> +
>>> + i++;
>>> + if (i >= MAX_SUPPORTED)
>>> + break;
>>> + }
>>> + drm_connector_list_iter_end(&iter);
>>> + mutex_unlock(&mode_config->mutex);
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
>>> +
>>> +static int amd_pmf_gpu_get_cur_state(struct thermal_cooling_device *cooling_dev,
>>> + unsigned long *state)
>>> +{
>>> + struct backlight_device *bd;
>>> +
>>> + if (!acpi_video_backlight_use_native())
>>> + return -ENODEV;
>>> +
>>> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>> + if (!bd)
>>> + return -ENODEV;
>>> +
>>> + *state = backlight_get_brightness(bd);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int amd_pmf_gpu_get_max_state(struct thermal_cooling_device *cooling_dev,
>>> + unsigned long *state)
>>> +{
>>> + struct backlight_device *bd;
>>> +
>>> + if (!acpi_video_backlight_use_native())
>>> + return -ENODEV;
>>> +
>>> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>> + if (!bd)
>>> + return -ENODEV;
>>> +
>>> + if (backlight_is_blank(bd))
>>> + *state = 0;
>>> + else
>>> + *state = bd->props.max_brightness;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct thermal_cooling_device_ops bd_cooling_ops = {
>>> + .get_max_state = amd_pmf_gpu_get_max_state,
>>> + .get_cur_state = amd_pmf_gpu_get_cur_state,
>>> +};
>>> +
>>> +int amd_pmf_gpu_init(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);
>>> +
>>> + if (!(adev->flags & AMD_IS_APU)) {
>>> + DRM_ERROR("PMF-AMDGPU interface not supported\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + if (!acpi_video_backlight_use_native())
>>> + return -ENODEV;
>>> +
>>> + pmf->raw_bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>> + if (!pmf->raw_bd)
>>> + return -ENODEV;
>>> +
>>> + pmf->cooling_dev = thermal_cooling_device_register("pmf_gpu_bd",
>>> + pmf, &bd_cooling_ops);
>>> + if (IS_ERR(pmf->cooling_dev))
>>> + return -ENODEV;
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(amd_pmf_gpu_init);
>>> +
>>> +void amd_pmf_gpu_deinit(struct amd_gpu_pmf_data *pmf)
>>> +{
>>> + thermal_cooling_device_unregister(pmf->cooling_dev);
>>> +}
>>> +EXPORT_SYMBOL_GPL(amd_pmf_gpu_deinit);
>>> diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
>>> index f246252bddd8..7f430de7af44 100644
>>> --- a/drivers/platform/x86/amd/pmf/Kconfig
>>> +++ b/drivers/platform/x86/amd/pmf/Kconfig
>>> @@ -10,6 +10,7 @@ config AMD_PMF
>>> depends on AMD_NB
>>> select ACPI_PLATFORM_PROFILE
>>> depends on TEE && AMDTEE
>>> + depends on 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 4b8156033fa6..c59ba527ff49 100644
>>> --- a/drivers/platform/x86/amd/pmf/core.c
>>> +++ b/drivers/platform/x86/amd/pmf/core.c
>>> @@ -411,6 +411,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 8712299ad52b..615cd3a31986 100644
>>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>>> @@ -13,6 +13,7 @@
>>>
>>> #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
>>> @@ -228,9 +229,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 512e0c66efdc..cf4962ef97c2 100644
>>> --- a/drivers/platform/x86/amd/pmf/spc.c
>>> +++ b/drivers/platform/x86/amd/pmf/spc.c
>>> @@ -44,6 +44,10 @@ void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *
>>> dev_dbg(dev->dev, "Max C0 Residency : %u\n", in->ev_info.max_c0residency);
>>> dev_dbg(dev->dev, "GFX Busy : %u\n", in->ev_info.gfx_busy);
>>> dev_dbg(dev->dev, "Connected Display Count : %u\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");
>>> }
>>> @@ -146,6 +150,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 */
>>> @@ -154,4 +166,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 2f5fb8623c20..956e66b78605 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"
>>> @@ -357,6 +358,19 @@ 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) {
>>> + /* Found the amdgpu handle from the pci root after walking through the pci bus */
>> If you insist on having this comment, make it a function comment instead
>> (with appropriate modifications into the content of it) but I personally
>> don't find it that useful so it could be just dropped as well, IMO.
>>
>>> + dev->gfx_data.gpu_dev = 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;
>>> @@ -446,6 +460,15 @@ 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 */
>> Useless comment.
>>
>>> + 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");
>>> +
>>> + if (!amd_pmf_gpu_init(&dev->gfx_data))
>>> + dev->gfx_data.gpu_dev_en = true;
>>> +
>>> dev->prev_data = kzalloc(sizeof(*dev->prev_data), GFP_KERNEL);
>>> if (!dev->prev_data)
>>> return -ENOMEM;
>>> @@ -461,5 +484,8 @@ 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);
>>> + if (dev->gfx_data.gpu_dev_en)
>>> + amd_pmf_gpu_deinit(&dev->gfx_data);
>>> + 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..5f79e66a41b3
>>> --- /dev/null
>>> +++ b/include/linux/amd-pmf-io.h
>>> @@ -0,0 +1,35 @@
>>> +/* 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 <acpi/video.h>
>>> +#include <drm/drm_connector.h>
>>> +#include <linux/backlight.h>
>>> +#include <linux/thermal.h>
>>> +
>>> +#define MAX_SUPPORTED 4
>>> +
>>> +/* amdgpu */
>> Document the structure properly with kerneldoc instead of an unhelpful
>> comment like above :-). Please also check if you add any other structs
>> into kernel-wide headers that you didn't document yet. Or fields into
>> existing structs.
>>
>>> +struct amd_gpu_pmf_data {
>>> + struct pci_dev *gpu_dev;
>>> + struct backlight_device *raw_bd;
>>> + struct thermal_cooling_device *cooling_dev;
>>> + enum drm_connector_status con_status[MAX_SUPPORTED];
>>> + int display_count;
>>> + int connector_type[MAX_SUPPORTED];
>>> + bool gpu_dev_en;
>>> +};
>>> +
>>> +int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
>>> +int amd_pmf_gpu_init(struct amd_gpu_pmf_data *pmf);
>>> +void amd_pmf_gpu_deinit(struct amd_gpu_pmf_data *pmf);
>>> +#endif
>>>
^ permalink raw reply
* Re: [PATCH v3 1/3] pwm: make it possible to apply pwm changes in atomic context
From: Hans de Goede @ 2023-10-18 13:57 UTC (permalink / raw)
To: Sean Young, linux-media, linux-pwm, Ivaylo Dimitrov,
Thierry Reding, Uwe Kleine-König, Jonathan Corbet,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
David Airlie, Daniel Vetter, Javier Martinez Canillas,
Jean Delvare, Guenter Roeck, Support Opensource, Dmitry Torokhov,
Pavel Machek, Lee Jones, Mauro Carvalho Chehab,
Ilpo Järvinen, Mark Gross, Liam Girdwood, Mark Brown,
Daniel Thompson, Jingoo Han, Helge Deller
Cc: linux-doc, linux-kernel, intel-gfx, dri-devel, linux-hwmon,
linux-input, linux-leds, platform-driver-x86, linux-arm-kernel,
linux-fbdev
In-Reply-To: <a7fcd19938d5422abc59c968ff7b3d5c275577ed.1697534024.git.sean@mess.org>
Hi Sean,
On 10/17/23 11:17, Sean Young wrote:
> Some drivers require sleeping, for example if the pwm device is connected
> over i2c. The pwm-ir-tx requires precise timing, and sleeping causes havoc
> with the generated IR signal when sleeping occurs.
>
> This patch makes it possible to use pwm when the driver does not sleep,
> by introducing the pwm_can_sleep() function.
>
> Signed-off-by: Sean Young <sean@mess.org>
I have no objection to this patch by itself, but it seems a bit
of unnecessary churn to change all current callers of pwm_apply_state()
to a new API.
Why not just keep pwm_apply_state() as is and introduce a new
pwm_apply_state_atomic() for callers which want to apply state
in a case where sleeping is not allowed ?
Regards,
Hans
> ---
> Documentation/driver-api/pwm.rst | 16 +++-
> .../gpu/drm/i915/display/intel_backlight.c | 6 +-
> drivers/gpu/drm/solomon/ssd130x.c | 2 +-
> drivers/hwmon/pwm-fan.c | 8 +-
> drivers/input/misc/da7280.c | 4 +-
> drivers/input/misc/pwm-beeper.c | 4 +-
> drivers/input/misc/pwm-vibra.c | 8 +-
> drivers/leds/leds-pwm.c | 2 +-
> drivers/leds/rgb/leds-pwm-multicolor.c | 4 +-
> drivers/media/rc/pwm-ir-tx.c | 4 +-
> drivers/platform/x86/lenovo-yogabook.c | 2 +-
> drivers/pwm/core.c | 75 ++++++++++++++-----
> drivers/pwm/pwm-renesas-tpu.c | 1 -
> drivers/pwm/pwm-twl-led.c | 2 +-
> drivers/pwm/pwm-vt8500.c | 2 +-
> drivers/pwm/sysfs.c | 10 +--
> drivers/regulator/pwm-regulator.c | 4 +-
> drivers/video/backlight/lm3630a_bl.c | 2 +-
> drivers/video/backlight/lp855x_bl.c | 2 +-
> drivers/video/backlight/pwm_bl.c | 6 +-
> drivers/video/fbdev/ssd1307fb.c | 2 +-
> include/linux/pwm.h | 57 ++++++++++----
> 22 files changed, 147 insertions(+), 76 deletions(-)
>
> diff --git a/Documentation/driver-api/pwm.rst b/Documentation/driver-api/pwm.rst
> index 3fdc95f7a1d15..a2fb5f8f6e1f8 100644
> --- a/Documentation/driver-api/pwm.rst
> +++ b/Documentation/driver-api/pwm.rst
> @@ -41,7 +41,15 @@ the getter, devm_pwm_get() and devm_fwnode_pwm_get(), also exist.
>
> After being requested, a PWM has to be configured using::
>
> - int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state);
> + int pwm_apply_cansleep(struct pwm_device *pwm, struct pwm_state *state);
> +
> +If the PWM support atomic mode, which can be determined with::
> +
> + bool pwm_is_atomic(struct pwm_device *pwm);
> +
> +Then the PWM can be configured with::
> +
> + int pwm_apply(struct pwm_device *pwm, struct pwm_state *state);
>
> This API controls both the PWM period/duty_cycle config and the
> enable/disable state.
> @@ -57,13 +65,13 @@ If supported by the driver, the signal can be optimized, for example to improve
> EMI by phase shifting the individual channels of a chip.
>
> The pwm_config(), pwm_enable() and pwm_disable() functions are just wrappers
> -around pwm_apply_state() and should not be used if the user wants to change
> +around pwm_apply_cansleep() and should not be used if the user wants to change
> several parameter at once. For example, if you see pwm_config() and
> pwm_{enable,disable}() calls in the same function, this probably means you
> -should switch to pwm_apply_state().
> +should switch to pwm_apply_cansleep().
>
> The PWM user API also allows one to query the PWM state that was passed to the
> -last invocation of pwm_apply_state() using pwm_get_state(). Note this is
> +last invocation of pwm_apply_cansleep() using pwm_get_state(). Note this is
> different to what the driver has actually implemented if the request cannot be
> satisfied exactly with the hardware in use. There is currently no way for
> consumers to get the actually implemented settings.
> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
> index 2e8f17c045222..cf516190cde8f 100644
> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
> @@ -274,7 +274,7 @@ static void ext_pwm_set_backlight(const struct drm_connector_state *conn_state,
> struct intel_panel *panel = &to_intel_connector(conn_state->connector)->panel;
>
> pwm_set_relative_duty_cycle(&panel->backlight.pwm_state, level, 100);
> - pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state);
> + pwm_apply_cansleep(panel->backlight.pwm, &panel->backlight.pwm_state);
> }
>
> static void
> @@ -427,7 +427,7 @@ static void ext_pwm_disable_backlight(const struct drm_connector_state *old_conn
> intel_backlight_set_pwm_level(old_conn_state, level);
>
> panel->backlight.pwm_state.enabled = false;
> - pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state);
> + pwm_apply_cansleep(panel->backlight.pwm, &panel->backlight.pwm_state);
> }
>
> void intel_backlight_disable(const struct drm_connector_state *old_conn_state)
> @@ -749,7 +749,7 @@ static void ext_pwm_enable_backlight(const struct intel_crtc_state *crtc_state,
>
> pwm_set_relative_duty_cycle(&panel->backlight.pwm_state, level, 100);
> panel->backlight.pwm_state.enabled = true;
> - pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state);
> + pwm_apply_cansleep(panel->backlight.pwm, &panel->backlight.pwm_state);
> }
>
> static void __intel_backlight_enable(const struct intel_crtc_state *crtc_state,
> diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
> index 5a80b228d18ca..5045966d43039 100644
> --- a/drivers/gpu/drm/solomon/ssd130x.c
> +++ b/drivers/gpu/drm/solomon/ssd130x.c
> @@ -267,7 +267,7 @@ static int ssd130x_pwm_enable(struct ssd130x_device *ssd130x)
>
> pwm_init_state(ssd130x->pwm, &pwmstate);
> pwm_set_relative_duty_cycle(&pwmstate, 50, 100);
> - pwm_apply_state(ssd130x->pwm, &pwmstate);
> + pwm_apply_cansleep(ssd130x->pwm, &pwmstate);
>
> /* Enable the PWM */
> pwm_enable(ssd130x->pwm);
> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index 6e4516c2ab894..f68deb1f236b7 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c
> @@ -151,7 +151,7 @@ static int pwm_fan_power_on(struct pwm_fan_ctx *ctx)
> }
>
> state->enabled = true;
> - ret = pwm_apply_state(ctx->pwm, state);
> + ret = pwm_apply_cansleep(ctx->pwm, state);
> if (ret) {
> dev_err(ctx->dev, "failed to enable PWM\n");
> goto disable_regulator;
> @@ -181,7 +181,7 @@ static int pwm_fan_power_off(struct pwm_fan_ctx *ctx)
>
> state->enabled = false;
> state->duty_cycle = 0;
> - ret = pwm_apply_state(ctx->pwm, state);
> + ret = pwm_apply_cansleep(ctx->pwm, state);
> if (ret) {
> dev_err(ctx->dev, "failed to disable PWM\n");
> return ret;
> @@ -207,7 +207,7 @@ static int __set_pwm(struct pwm_fan_ctx *ctx, unsigned long pwm)
>
> period = state->period;
> state->duty_cycle = DIV_ROUND_UP(pwm * (period - 1), MAX_PWM);
> - ret = pwm_apply_state(ctx->pwm, state);
> + ret = pwm_apply_cansleep(ctx->pwm, state);
> if (ret)
> return ret;
> ret = pwm_fan_power_on(ctx);
> @@ -278,7 +278,7 @@ static int pwm_fan_update_enable(struct pwm_fan_ctx *ctx, long val)
> state,
> &enable_regulator);
>
> - pwm_apply_state(ctx->pwm, state);
> + pwm_apply_cansleep(ctx->pwm, state);
> pwm_fan_switch_power(ctx, enable_regulator);
> pwm_fan_update_state(ctx, 0);
> }
> diff --git a/drivers/input/misc/da7280.c b/drivers/input/misc/da7280.c
> index ce82548916bbc..f10be2cdba803 100644
> --- a/drivers/input/misc/da7280.c
> +++ b/drivers/input/misc/da7280.c
> @@ -352,7 +352,7 @@ static int da7280_haptic_set_pwm(struct da7280_haptic *haptics, bool enabled)
> state.duty_cycle = period_mag_multi;
> }
>
> - error = pwm_apply_state(haptics->pwm_dev, &state);
> + error = pwm_apply_cansleep(haptics->pwm_dev, &state);
> if (error)
> dev_err(haptics->dev, "Failed to apply pwm state: %d\n", error);
>
> @@ -1175,7 +1175,7 @@ static int da7280_probe(struct i2c_client *client)
> /* Sync up PWM state and ensure it is off. */
> pwm_init_state(haptics->pwm_dev, &state);
> state.enabled = false;
> - error = pwm_apply_state(haptics->pwm_dev, &state);
> + error = pwm_apply_cansleep(haptics->pwm_dev, &state);
> if (error) {
> dev_err(dev, "Failed to apply PWM state: %d\n", error);
> return error;
> diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
> index 1e731d8397c6f..1d6c4fb5f0caf 100644
> --- a/drivers/input/misc/pwm-beeper.c
> +++ b/drivers/input/misc/pwm-beeper.c
> @@ -39,7 +39,7 @@ static int pwm_beeper_on(struct pwm_beeper *beeper, unsigned long period)
> state.period = period;
> pwm_set_relative_duty_cycle(&state, 50, 100);
>
> - error = pwm_apply_state(beeper->pwm, &state);
> + error = pwm_apply_cansleep(beeper->pwm, &state);
> if (error)
> return error;
>
> @@ -138,7 +138,7 @@ static int pwm_beeper_probe(struct platform_device *pdev)
> /* Sync up PWM state and ensure it is off. */
> pwm_init_state(beeper->pwm, &state);
> state.enabled = false;
> - error = pwm_apply_state(beeper->pwm, &state);
> + error = pwm_apply_cansleep(beeper->pwm, &state);
> if (error) {
> dev_err(dev, "failed to apply initial PWM state: %d\n",
> error);
> diff --git a/drivers/input/misc/pwm-vibra.c b/drivers/input/misc/pwm-vibra.c
> index acac79c488aa1..6552ce712d8dc 100644
> --- a/drivers/input/misc/pwm-vibra.c
> +++ b/drivers/input/misc/pwm-vibra.c
> @@ -56,7 +56,7 @@ static int pwm_vibrator_start(struct pwm_vibrator *vibrator)
> pwm_set_relative_duty_cycle(&state, vibrator->level, 0xffff);
> state.enabled = true;
>
> - err = pwm_apply_state(vibrator->pwm, &state);
> + err = pwm_apply_cansleep(vibrator->pwm, &state);
> if (err) {
> dev_err(pdev, "failed to apply pwm state: %d\n", err);
> return err;
> @@ -67,7 +67,7 @@ static int pwm_vibrator_start(struct pwm_vibrator *vibrator)
> state.duty_cycle = vibrator->direction_duty_cycle;
> state.enabled = true;
>
> - err = pwm_apply_state(vibrator->pwm_dir, &state);
> + err = pwm_apply_cansleep(vibrator->pwm_dir, &state);
> if (err) {
> dev_err(pdev, "failed to apply dir-pwm state: %d\n", err);
> pwm_disable(vibrator->pwm);
> @@ -160,7 +160,7 @@ static int pwm_vibrator_probe(struct platform_device *pdev)
> /* Sync up PWM state and ensure it is off. */
> pwm_init_state(vibrator->pwm, &state);
> state.enabled = false;
> - err = pwm_apply_state(vibrator->pwm, &state);
> + err = pwm_apply_cansleep(vibrator->pwm, &state);
> if (err) {
> dev_err(&pdev->dev, "failed to apply initial PWM state: %d\n",
> err);
> @@ -174,7 +174,7 @@ static int pwm_vibrator_probe(struct platform_device *pdev)
> /* Sync up PWM state and ensure it is off. */
> pwm_init_state(vibrator->pwm_dir, &state);
> state.enabled = false;
> - err = pwm_apply_state(vibrator->pwm_dir, &state);
> + err = pwm_apply_cansleep(vibrator->pwm_dir, &state);
> if (err) {
> dev_err(&pdev->dev, "failed to apply initial PWM state: %d\n",
> err);
> diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
> index 419b710984ab6..e1fe1fd8f189a 100644
> --- a/drivers/leds/leds-pwm.c
> +++ b/drivers/leds/leds-pwm.c
> @@ -54,7 +54,7 @@ static int led_pwm_set(struct led_classdev *led_cdev,
>
> led_dat->pwmstate.duty_cycle = duty;
> led_dat->pwmstate.enabled = duty > 0;
> - return pwm_apply_state(led_dat->pwm, &led_dat->pwmstate);
> + return pwm_apply_cansleep(led_dat->pwm, &led_dat->pwmstate);
> }
>
> __attribute__((nonnull))
> diff --git a/drivers/leds/rgb/leds-pwm-multicolor.c b/drivers/leds/rgb/leds-pwm-multicolor.c
> index 46cd062b8b24c..8114adcdad9bb 100644
> --- a/drivers/leds/rgb/leds-pwm-multicolor.c
> +++ b/drivers/leds/rgb/leds-pwm-multicolor.c
> @@ -51,8 +51,8 @@ static int led_pwm_mc_set(struct led_classdev *cdev,
>
> priv->leds[i].state.duty_cycle = duty;
> priv->leds[i].state.enabled = duty > 0;
> - ret = pwm_apply_state(priv->leds[i].pwm,
> - &priv->leds[i].state);
> + ret = pwm_apply_cansleep(priv->leds[i].pwm,
> + &priv->leds[i].state);
> if (ret)
> break;
> }
> diff --git a/drivers/media/rc/pwm-ir-tx.c b/drivers/media/rc/pwm-ir-tx.c
> index c5f37c03af9c9..ccb86890adcea 100644
> --- a/drivers/media/rc/pwm-ir-tx.c
> +++ b/drivers/media/rc/pwm-ir-tx.c
> @@ -68,7 +68,7 @@ static int pwm_ir_tx(struct rc_dev *dev, unsigned int *txbuf,
>
> for (i = 0; i < count; i++) {
> state.enabled = !(i % 2);
> - pwm_apply_state(pwm, &state);
> + pwm_apply_cansleep(pwm, &state);
>
> edge = ktime_add_us(edge, txbuf[i]);
> delta = ktime_us_delta(edge, ktime_get());
> @@ -77,7 +77,7 @@ static int pwm_ir_tx(struct rc_dev *dev, unsigned int *txbuf,
> }
>
> state.enabled = false;
> - pwm_apply_state(pwm, &state);
> + pwm_apply_cansleep(pwm, &state);
>
> return count;
> }
> diff --git a/drivers/platform/x86/lenovo-yogabook.c b/drivers/platform/x86/lenovo-yogabook.c
> index b8d0239192cbf..cbc285f77c2bd 100644
> --- a/drivers/platform/x86/lenovo-yogabook.c
> +++ b/drivers/platform/x86/lenovo-yogabook.c
> @@ -435,7 +435,7 @@ static int yogabook_pdev_set_kbd_backlight(struct yogabook_data *data, u8 level)
> .enabled = level,
> };
>
> - pwm_apply_state(data->kbd_bl_pwm, &state);
> + pwm_apply_cansleep(data->kbd_bl_pwm, &state);
> gpiod_set_value(data->kbd_bl_led_enable, level ? 1 : 0);
> return 0;
> }
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index dc66e3405bf50..99896a59a25aa 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -382,8 +382,8 @@ struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip,
> }
> EXPORT_SYMBOL_GPL(pwm_request_from_chip);
>
> -static void pwm_apply_state_debug(struct pwm_device *pwm,
> - const struct pwm_state *state)
> +static void pwm_apply_cansleep_debug(struct pwm_device *pwm,
> + const struct pwm_state *state)
> {
> struct pwm_state *last = &pwm->last;
> struct pwm_chip *chip = pwm->chip;
> @@ -489,24 +489,15 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
> }
>
> /**
> - * pwm_apply_state() - atomically apply a new state to a PWM device
> + * pwm_apply_unchecked() - atomically apply a new state to a PWM device
> * @pwm: PWM device
> * @state: new state to apply
> */
> -int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
> +static int pwm_apply_unchecked(struct pwm_device *pwm, const struct pwm_state *state)
> {
> struct pwm_chip *chip;
> int err;
>
> - /*
> - * Some lowlevel driver's implementations of .apply() make use of
> - * mutexes, also with some drivers only returning when the new
> - * configuration is active calling pwm_apply_state() from atomic context
> - * is a bad idea. So make it explicit that calling this function might
> - * sleep.
> - */
> - might_sleep();
> -
> if (!pwm || !state || !state->period ||
> state->duty_cycle > state->period)
> return -EINVAL;
> @@ -527,15 +518,63 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
>
> pwm->state = *state;
>
> + return 0;
> +}
> +
> +/**
> + * pwm_apply_cansleep() - atomically apply a new state to a PWM device
> + * Cannot be used in atomic context.
> + * @pwm: PWM device
> + * @state: new state to apply
> + */
> +int pwm_apply_cansleep(struct pwm_device *pwm, const struct pwm_state *state)
> +{
> + int err;
> +
> + /*
> + * Some lowlevel driver's implementations of .apply() make use of
> + * mutexes, also with some drivers only returning when the new
> + * configuration is active calling pwm_apply_cansleep() from atomic context
> + * is a bad idea. So make it explicit that calling this function might
> + * sleep.
> + */
> + might_sleep();
> +
> + if (IS_ENABLED(CONFIG_PWM_DEBUG) && pwm->chip->atomic) {
> + /*
> + * Catch any sleeping drivers when atomic is set.
> + */
> + non_block_start();
> + err = pwm_apply_unchecked(pwm, state);
> + non_block_end();
> + } else {
> + err = pwm_apply_unchecked(pwm, state);
> + }
> +
> /*
> * only do this after pwm->state was applied as some
> * implementations of .get_state depend on this
> */
> - pwm_apply_state_debug(pwm, state);
> + pwm_apply_cansleep_debug(pwm, state);
>
> - return 0;
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(pwm_apply_cansleep);
> +
> +/**
> + * pwm_apply() - atomically apply a new state to a PWM device
> + * Can be used from atomic context.
> + * @pwm: PWM device
> + * @state: new state to apply
> + */
> +int pwm_apply(struct pwm_device *pwm, const struct pwm_state *state)
> +{
> + WARN_ONCE(!pwm->chip->atomic,
> + "sleeping pwm driver used in atomic context");
> +
> + return pwm_apply_unchecked(pwm, state);
> }
> -EXPORT_SYMBOL_GPL(pwm_apply_state);
> +EXPORT_SYMBOL_GPL(pwm_apply);
>
> /**
> * pwm_capture() - capture and report a PWM signal
> @@ -593,7 +632,7 @@ int pwm_adjust_config(struct pwm_device *pwm)
> state.period = pargs.period;
> state.polarity = pargs.polarity;
>
> - return pwm_apply_state(pwm, &state);
> + return pwm_apply_cansleep(pwm, &state);
> }
>
> /*
> @@ -616,7 +655,7 @@ int pwm_adjust_config(struct pwm_device *pwm)
> state.duty_cycle = state.period - state.duty_cycle;
> }
>
> - return pwm_apply_state(pwm, &state);
> + return pwm_apply_cansleep(pwm, &state);
> }
> EXPORT_SYMBOL_GPL(pwm_adjust_config);
>
> diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c
> index d7311614c846d..96797a33d8c62 100644
> --- a/drivers/pwm/pwm-renesas-tpu.c
> +++ b/drivers/pwm/pwm-renesas-tpu.c
> @@ -11,7 +11,6 @@
> #include <linux/init.h>
> #include <linux/ioport.h>
> #include <linux/module.h>
> -#include <linux/mutex.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> diff --git a/drivers/pwm/pwm-twl-led.c b/drivers/pwm/pwm-twl-led.c
> index 8fb84b4418538..a1fc2fa0d03e0 100644
> --- a/drivers/pwm/pwm-twl-led.c
> +++ b/drivers/pwm/pwm-twl-led.c
> @@ -172,7 +172,7 @@ static int twl4030_pwmled_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> * We cannot skip calling ->config even if state->period ==
> * pwm->state.period && state->duty_cycle == pwm->state.duty_cycle
> * because we might have exited early in the last call to
> - * pwm_apply_state because of !state->enabled and so the two values in
> + * pwm_apply_cansleep because of !state->enabled and so the two values in
> * pwm->state might not be configured in hardware.
> */
> ret = twl4030_pwmled_config(pwm->chip, pwm,
> diff --git a/drivers/pwm/pwm-vt8500.c b/drivers/pwm/pwm-vt8500.c
> index 6d46db51daacc..3a815dfbf31ce 100644
> --- a/drivers/pwm/pwm-vt8500.c
> +++ b/drivers/pwm/pwm-vt8500.c
> @@ -206,7 +206,7 @@ static int vt8500_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> * We cannot skip calling ->config even if state->period ==
> * pwm->state.period && state->duty_cycle == pwm->state.duty_cycle
> * because we might have exited early in the last call to
> - * pwm_apply_state because of !state->enabled and so the two values in
> + * pwm_apply_cansleep because of !state->enabled and so the two values in
> * pwm->state might not be configured in hardware.
> */
> err = vt8500_pwm_config(pwm->chip, pwm, state->duty_cycle, state->period);
> diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
> index 8d1254761e4dd..eca9cad3be765 100644
> --- a/drivers/pwm/sysfs.c
> +++ b/drivers/pwm/sysfs.c
> @@ -62,7 +62,7 @@ static ssize_t period_store(struct device *child,
> mutex_lock(&export->lock);
> pwm_get_state(pwm, &state);
> state.period = val;
> - ret = pwm_apply_state(pwm, &state);
> + ret = pwm_apply_cansleep(pwm, &state);
> mutex_unlock(&export->lock);
>
> return ret ? : size;
> @@ -97,7 +97,7 @@ static ssize_t duty_cycle_store(struct device *child,
> mutex_lock(&export->lock);
> pwm_get_state(pwm, &state);
> state.duty_cycle = val;
> - ret = pwm_apply_state(pwm, &state);
> + ret = pwm_apply_cansleep(pwm, &state);
> mutex_unlock(&export->lock);
>
> return ret ? : size;
> @@ -144,7 +144,7 @@ static ssize_t enable_store(struct device *child,
> goto unlock;
> }
>
> - ret = pwm_apply_state(pwm, &state);
> + ret = pwm_apply_cansleep(pwm, &state);
>
> unlock:
> mutex_unlock(&export->lock);
> @@ -194,7 +194,7 @@ static ssize_t polarity_store(struct device *child,
> mutex_lock(&export->lock);
> pwm_get_state(pwm, &state);
> state.polarity = polarity;
> - ret = pwm_apply_state(pwm, &state);
> + ret = pwm_apply_cansleep(pwm, &state);
> mutex_unlock(&export->lock);
>
> return ret ? : size;
> @@ -401,7 +401,7 @@ static int pwm_class_apply_state(struct pwm_export *export,
> struct pwm_device *pwm,
> struct pwm_state *state)
> {
> - int ret = pwm_apply_state(pwm, state);
> + int ret = pwm_apply_cansleep(pwm, state);
>
> /* release lock taken in pwm_class_get_state */
> mutex_unlock(&export->lock);
> diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
> index 2aff6db748e2c..c19d37a479d43 100644
> --- a/drivers/regulator/pwm-regulator.c
> +++ b/drivers/regulator/pwm-regulator.c
> @@ -90,7 +90,7 @@ static int pwm_regulator_set_voltage_sel(struct regulator_dev *rdev,
> pwm_set_relative_duty_cycle(&pstate,
> drvdata->duty_cycle_table[selector].dutycycle, 100);
>
> - ret = pwm_apply_state(drvdata->pwm, &pstate);
> + ret = pwm_apply_cansleep(drvdata->pwm, &pstate);
> if (ret) {
> dev_err(&rdev->dev, "Failed to configure PWM: %d\n", ret);
> return ret;
> @@ -216,7 +216,7 @@ static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
>
> pwm_set_relative_duty_cycle(&pstate, dutycycle, duty_unit);
>
> - ret = pwm_apply_state(drvdata->pwm, &pstate);
> + ret = pwm_apply_cansleep(drvdata->pwm, &pstate);
> if (ret) {
> dev_err(&rdev->dev, "Failed to configure PWM: %d\n", ret);
> return ret;
> diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
> index 8fcb62be597b8..5cb702989ef61 100644
> --- a/drivers/video/backlight/lm3630a_bl.c
> +++ b/drivers/video/backlight/lm3630a_bl.c
> @@ -180,7 +180,7 @@ static int lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
>
> pchip->pwmd_state.enabled = pchip->pwmd_state.duty_cycle ? true : false;
>
> - return pwm_apply_state(pchip->pwmd, &pchip->pwmd_state);
> + return pwm_apply_cansleep(pchip->pwmd, &pchip->pwmd_state);
> }
>
> /* update and get brightness */
> diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
> index da1f124db69c0..b7edbaaa169a4 100644
> --- a/drivers/video/backlight/lp855x_bl.c
> +++ b/drivers/video/backlight/lp855x_bl.c
> @@ -234,7 +234,7 @@ static int lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
> state.duty_cycle = div_u64(br * state.period, max_br);
> state.enabled = state.duty_cycle;
>
> - return pwm_apply_state(lp->pwm, &state);
> + return pwm_apply_cansleep(lp->pwm, &state);
> }
>
> static int lp855x_bl_update_status(struct backlight_device *bl)
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index a51fbab963680..f2568aaae4769 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -103,7 +103,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
> pwm_get_state(pb->pwm, &state);
> state.duty_cycle = compute_duty_cycle(pb, brightness, &state);
> state.enabled = true;
> - pwm_apply_state(pb->pwm, &state);
> + pwm_apply_cansleep(pb->pwm, &state);
>
> pwm_backlight_power_on(pb);
> } else {
> @@ -120,7 +120,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
> * inactive output.
> */
> state.enabled = !pb->power_supply && !pb->enable_gpio;
> - pwm_apply_state(pb->pwm, &state);
> + pwm_apply_cansleep(pb->pwm, &state);
> }
>
> if (pb->notify_after)
> @@ -528,7 +528,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> if (!state.period && (data->pwm_period_ns > 0))
> state.period = data->pwm_period_ns;
>
> - ret = pwm_apply_state(pb->pwm, &state);
> + ret = pwm_apply_cansleep(pb->pwm, &state);
> if (ret) {
> dev_err(&pdev->dev, "failed to apply initial PWM state: %d\n",
> ret);
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index 5ae48e36fccb4..e5cca01af55f3 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -347,7 +347,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
>
> pwm_init_state(par->pwm, &pwmstate);
> pwm_set_relative_duty_cycle(&pwmstate, 50, 100);
> - pwm_apply_state(par->pwm, &pwmstate);
> + pwm_apply_cansleep(par->pwm, &pwmstate);
>
> /* Enable the PWM */
> pwm_enable(par->pwm);
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> index d2f9f690a9c14..373b5a4fe27dc 100644
> --- a/include/linux/pwm.h
> +++ b/include/linux/pwm.h
> @@ -95,8 +95,8 @@ struct pwm_device {
> * @state: state to fill with the current PWM state
> *
> * The returned PWM state represents the state that was applied by a previous call to
> - * pwm_apply_state(). Drivers may have to slightly tweak that state before programming it to
> - * hardware. If pwm_apply_state() was never called, this returns either the current hardware
> + * pwm_apply_cansleep(). Drivers may have to slightly tweak that state before programming it to
> + * hardware. If pwm_apply_cansleep() was never called, this returns either the current hardware
> * state (if supported) or the default settings.
> */
> static inline void pwm_get_state(const struct pwm_device *pwm,
> @@ -160,20 +160,20 @@ static inline void pwm_get_args(const struct pwm_device *pwm,
> }
>
> /**
> - * pwm_init_state() - prepare a new state to be applied with pwm_apply_state()
> + * pwm_init_state() - prepare a new state to be applied with pwm_apply_cansleep()
> * @pwm: PWM device
> * @state: state to fill with the prepared PWM state
> *
> * This functions prepares a state that can later be tweaked and applied
> - * to the PWM device with pwm_apply_state(). This is a convenient function
> + * to the PWM device with pwm_apply_cansleep(). This is a convenient function
> * that first retrieves the current PWM state and the replaces the period
> * and polarity fields with the reference values defined in pwm->args.
> * Once the function returns, you can adjust the ->enabled and ->duty_cycle
> - * fields according to your needs before calling pwm_apply_state().
> + * fields according to your needs before calling pwm_apply_cansleep().
> *
> * ->duty_cycle is initially set to zero to avoid cases where the current
> * ->duty_cycle value exceed the pwm_args->period one, which would trigger
> - * an error if the user calls pwm_apply_state() without adjusting ->duty_cycle
> + * an error if the user calls pwm_apply_cansleep() without adjusting ->duty_cycle
> * first.
> */
> static inline void pwm_init_state(const struct pwm_device *pwm,
> @@ -229,7 +229,7 @@ pwm_get_relative_duty_cycle(const struct pwm_state *state, unsigned int scale)
> *
> * pwm_init_state(pwm, &state);
> * pwm_set_relative_duty_cycle(&state, 50, 100);
> - * pwm_apply_state(pwm, &state);
> + * pwm_apply_cansleep(pwm, &state);
> *
> * This functions returns -EINVAL if @duty_cycle and/or @scale are
> * inconsistent (@scale == 0 or @duty_cycle > @scale).
> @@ -289,6 +289,7 @@ struct pwm_ops {
> * @npwm: number of PWMs controlled by this chip
> * @of_xlate: request a PWM device given a device tree PWM specifier
> * @of_pwm_n_cells: number of cells expected in the device tree PWM specifier
> + * @atomic: can the driver execute pwm_apply_cansleep in atomic context
> * @list: list node for internal use
> * @pwms: array of PWM devices allocated by the framework
> */
> @@ -301,6 +302,7 @@ struct pwm_chip {
> struct pwm_device * (*of_xlate)(struct pwm_chip *chip,
> const struct of_phandle_args *args);
> unsigned int of_pwm_n_cells;
> + bool atomic;
>
> /* only used internally by the PWM framework */
> struct list_head list;
> @@ -309,7 +311,8 @@ struct pwm_chip {
>
> #if IS_ENABLED(CONFIG_PWM)
> /* PWM user APIs */
> -int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state);
> +int pwm_apply_cansleep(struct pwm_device *pwm, const struct pwm_state *state);
> +int pwm_apply(struct pwm_device *pwm, const struct pwm_state *state);
> int pwm_adjust_config(struct pwm_device *pwm);
>
> /**
> @@ -337,7 +340,7 @@ static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
>
> state.duty_cycle = duty_ns;
> state.period = period_ns;
> - return pwm_apply_state(pwm, &state);
> + return pwm_apply_cansleep(pwm, &state);
> }
>
> /**
> @@ -358,7 +361,7 @@ static inline int pwm_enable(struct pwm_device *pwm)
> return 0;
>
> state.enabled = true;
> - return pwm_apply_state(pwm, &state);
> + return pwm_apply_cansleep(pwm, &state);
> }
>
> /**
> @@ -377,7 +380,18 @@ static inline void pwm_disable(struct pwm_device *pwm)
> return;
>
> state.enabled = false;
> - pwm_apply_state(pwm, &state);
> + pwm_apply_cansleep(pwm, &state);
> +}
> +
> +/**
> + * pwm_is_atomic() - is pwm_apply() supported?
> + * @pwm: PWM device
> + *
> + * Returns: true pwm_apply() can be called from atomic context.
> + */
> +static inline bool pwm_is_atomic(struct pwm_device *pwm)
> +{
> + return pwm->chip->atomic;
> }
>
> /* PWM provider APIs */
> @@ -408,16 +422,27 @@ struct pwm_device *devm_fwnode_pwm_get(struct device *dev,
> struct fwnode_handle *fwnode,
> const char *con_id);
> #else
> -static inline int pwm_apply_state(struct pwm_device *pwm,
> - const struct pwm_state *state)
> +static inline bool pwm_is_atomic(struct pwm_device *pwm)
> +{
> + return false;
> +}
> +
> +static inline int pwm_apply_cansleep(struct pwm_device *pwm,
> + const struct pwm_state *state)
> {
> might_sleep();
> - return -ENOTSUPP;
> + return -EOPNOTSUPP;
> +}
> +
> +static inline int pwm_apply(struct pwm_device *pwm,
> + const struct pwm_state *state)
> +{
> + return -EOPNOTSUPP;
> }
>
> static inline int pwm_adjust_config(struct pwm_device *pwm)
> {
> - return -ENOTSUPP;
> + return -EOPNOTSUPP;
> }
>
> static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
> @@ -536,7 +561,7 @@ static inline void pwm_apply_args(struct pwm_device *pwm)
> state.period = pwm->args.period;
> state.usage_power = false;
>
> - pwm_apply_state(pwm, &state);
> + pwm_apply_cansleep(pwm, &state);
> }
>
> struct pwm_lookup {
^ permalink raw reply
* [PATCH AUTOSEL 6.5 01/31] Input: xpad - add HyperX Clutch Gladiate Support
From: Sasha Levin @ 2023-10-18 14:11 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Max Nguyen, Chris Toledanes, Carl Ng, Rahul Rameshbabu,
Dmitry Torokhov, Sasha Levin, vi, swyterzone, doublej472,
pgriffais, slouken, matthias.benkmann, linux-input
From: Max Nguyen <maxwell.nguyen@hp.com>
[ Upstream commit e28a0974d749e5105d77233c0a84d35c37da047e ]
Add HyperX controller support to xpad_device and xpad_table.
Suggested-by: Chris Toledanes <chris.toledanes@hp.com>
Reviewed-by: Carl Ng <carl.ng@hp.com>
Signed-off-by: Max Nguyen <maxwell.nguyen@hp.com>
Reviewed-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
Link: https://lore.kernel.org/r/20230906231514.4291-1-hphyperxdev@gmail.com
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/input/joystick/xpad.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index cdb193317c3b6..1e377d040c435 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -130,6 +130,7 @@ static const struct xpad_device {
{ 0x0079, 0x18d4, "GPD Win 2 X-Box Controller", 0, XTYPE_XBOX360 },
{ 0x03eb, 0xff01, "Wooting One (Legacy)", 0, XTYPE_XBOX360 },
{ 0x03eb, 0xff02, "Wooting Two (Legacy)", 0, XTYPE_XBOX360 },
+ { 0x03f0, 0x0495, "HyperX Clutch Gladiate", 0, XTYPE_XBOXONE },
{ 0x044f, 0x0f00, "Thrustmaster Wheel", 0, XTYPE_XBOX },
{ 0x044f, 0x0f03, "Thrustmaster Wheel", 0, XTYPE_XBOX },
{ 0x044f, 0x0f07, "Thrustmaster, Inc. Controller", 0, XTYPE_XBOX },
@@ -457,6 +458,7 @@ static const struct usb_device_id xpad_table[] = {
{ USB_INTERFACE_INFO('X', 'B', 0) }, /* Xbox USB-IF not-approved class */
XPAD_XBOX360_VENDOR(0x0079), /* GPD Win 2 controller */
XPAD_XBOX360_VENDOR(0x03eb), /* Wooting Keyboards (Legacy) */
+ XPAD_XBOXONE_VENDOR(0x03f0), /* HP HyperX Xbox One controllers */
XPAD_XBOX360_VENDOR(0x044f), /* Thrustmaster Xbox 360 controllers */
XPAD_XBOX360_VENDOR(0x045e), /* Microsoft Xbox 360 controllers */
XPAD_XBOXONE_VENDOR(0x045e), /* Microsoft Xbox One controllers */
--
2.40.1
^ permalink raw reply related
* [PATCH AUTOSEL 6.5 10/31] Input: i8042 - add Fujitsu Lifebook E5411 to i8042 quirk table
From: Sasha Levin @ 2023-10-18 14:11 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Szilard Fabian, Dmitry Torokhov, Sasha Levin, wse, hdegoede,
jdenose, tiwai, linux-input
In-Reply-To: <20231018141151.1334501-1-sashal@kernel.org>
From: Szilard Fabian <szfabian@bluemarch.art>
[ Upstream commit 80f39e1c27ba9e5a1ea7e68e21c569c9d8e46062 ]
In the initial boot stage the integrated keyboard of Fujitsu Lifebook E5411
refuses to work and it's not possible to type for example a dm-crypt
passphrase without the help of an external keyboard.
i8042.nomux kernel parameter resolves this issue but using that a PS/2
mouse is detected. This input device is unused even when the i2c-hid-acpi
kernel module is blacklisted making the integrated ELAN touchpad
(04F3:308A) not working at all.
Since the integrated touchpad is managed by the i2c_designware input
driver in the Linux kernel and you can't find a PS/2 mouse port on the
computer I think it's safe to not use the PS/2 mouse port at all.
Signed-off-by: Szilard Fabian <szfabian@bluemarch.art>
Link: https://lore.kernel.org/r/20231004011749.101789-1-szfabian@bluemarch.art
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/input/serio/i8042-acpipnpio.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/input/serio/i8042-acpipnpio.h b/drivers/input/serio/i8042-acpipnpio.h
index 1724d6cb8649d..9c39553d30fa2 100644
--- a/drivers/input/serio/i8042-acpipnpio.h
+++ b/drivers/input/serio/i8042-acpipnpio.h
@@ -618,6 +618,14 @@ static const struct dmi_system_id i8042_dmi_quirk_table[] __initconst = {
},
.driver_data = (void *)(SERIO_QUIRK_NOMUX)
},
+ {
+ /* Fujitsu Lifebook E5411 */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU CLIENT COMPUTING LIMITED"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "LIFEBOOK E5411"),
+ },
+ .driver_data = (void *)(SERIO_QUIRK_NOAUX)
+ },
{
/* Gigabyte M912 */
.matches = {
--
2.40.1
^ permalink raw reply related
* [PATCH AUTOSEL 6.5 11/31] Input: goodix - ensure int GPIO is in input for gpio_count == 1 && gpio_int_idx == 0 case
From: Sasha Levin @ 2023-10-18 14:11 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Hans de Goede, Michael Smith, Dmitry Torokhov, Sasha Levin,
hadess, linux-input
In-Reply-To: <20231018141151.1334501-1-sashal@kernel.org>
From: Hans de Goede <hdegoede@redhat.com>
[ Upstream commit 423622a90abb243944d1517b9f57db53729e45c4 ]
Add a special case for gpio_count == 1 && gpio_int_idx == 0 to
goodix_add_acpi_gpio_mappings().
It seems that on newer x86/ACPI devices the reset and irq GPIOs are no
longer listed as GPIO resources instead there is only 1 GpioInt resource
and _PS0 does the whole reset sequence for us.
This means that we must call acpi_device_fix_up_power() on these devices
to ensure that the chip is reset before we try to use it.
This part was already fixed in commit 3de93e6ed2df ("Input: goodix - call
acpi_device_fix_up_power() in some cases") by adding a call to
acpi_device_fix_up_power() to the generic "Unexpected ACPI resources"
catch all.
But it turns out that this case on some hw needs some more special
handling. Specifically the firmware may bootup with the IRQ pin in
output mode. The reset sequence from ACPI _PS0 (executed by
acpi_device_fix_up_power()) should put the pin in input mode,
but the GPIO subsystem has cached the direction at bootup, causing
request_irq() to fail due to gpiochip_lock_as_irq() failure:
[ 9.119864] Goodix-TS i2c-GDIX1002:00: Unexpected ACPI resources: gpio_count 1, gpio_int_idx 0
[ 9.317443] Goodix-TS i2c-GDIX1002:00: ID 911, version: 1060
[ 9.321902] input: Goodix Capacitive TouchScreen as /devices/pci0000:00/0000:00:17.0/i2c_designware.4/i2c-5/i2c-GDIX1002:00/input/input8
[ 9.327840] gpio gpiochip0: (INT3453:00): gpiochip_lock_as_irq: tried to flag a GPIO set as output for IRQ
[ 9.327856] gpio gpiochip0: (INT3453:00): unable to lock HW IRQ 26 for IRQ
[ 9.327861] genirq: Failed to request resources for GDIX1002:00 (irq 131) on irqchip intel-gpio
[ 9.327912] Goodix-TS i2c-GDIX1002:00: request IRQ failed: -5
Fix this by adding a special case for gpio_count == 1 && gpio_int_idx == 0
which adds an ACPI GPIO lookup table for the int GPIO even though we cannot
use it for reset purposes (as there is no reset GPIO).
Adding the lookup will make the gpiod_int = gpiod_get(..., GPIOD_IN) call
succeed, which will explicitly set the direction to input fixing the issue.
Note this re-uses the acpi_goodix_int_first_gpios[] lookup table, since
there is only 1 GPIO in the ACPI resources the reset entry in that
lookup table will amount to a no-op.
Reported-and-tested-by: Michael Smith <1973.mjsmith@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20231003215144.69527-1-hdegoede@redhat.com
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/input/touchscreen/goodix.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index f5aa240739f97..0f727dbc7232f 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -900,6 +900,25 @@ static int goodix_add_acpi_gpio_mappings(struct goodix_ts_data *ts)
dev_info(dev, "No ACPI GpioInt resource, assuming that the GPIO order is reset, int\n");
ts->irq_pin_access_method = IRQ_PIN_ACCESS_ACPI_GPIO;
gpio_mapping = acpi_goodix_int_last_gpios;
+ } else if (ts->gpio_count == 1 && ts->gpio_int_idx == 0) {
+ /*
+ * On newer devices there is only 1 GpioInt resource and _PS0
+ * does the whole reset sequence for us.
+ */
+ acpi_device_fix_up_power(ACPI_COMPANION(dev));
+
+ /*
+ * Before the _PS0 call the int GPIO may have been in output
+ * mode and the call should have put the int GPIO in input mode,
+ * but the GPIO subsys cached state may still think it is
+ * in output mode, causing gpiochip_lock_as_irq() failure.
+ *
+ * Add a mapping for the int GPIO to make the
+ * gpiod_int = gpiod_get(..., GPIOD_IN) call succeed,
+ * which will explicitly set the direction to input.
+ */
+ ts->irq_pin_access_method = IRQ_PIN_ACCESS_NONE;
+ gpio_mapping = acpi_goodix_int_first_gpios;
} else {
dev_warn(dev, "Unexpected ACPI resources: gpio_count %d, gpio_int_idx %d\n",
ts->gpio_count, ts->gpio_int_idx);
--
2.40.1
^ permalink raw reply related
* [PATCH AUTOSEL 6.5 29/31] Input: synaptics-rmi4 - handle reset delay when using SMBus trsnsport
From: Sasha Levin @ 2023-10-18 14:11 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Dmitry Torokhov, Jeffery Miller, Sasha Levin, rrangel,
u.kleine-koenig, Jonathan.Cameron, linux-input
In-Reply-To: <20231018141151.1334501-1-sashal@kernel.org>
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
[ Upstream commit 5030b2fe6aab37fe42d14f31842ea38be7c55c57 ]
Touch controllers need some time after receiving reset command for the
firmware to finish re-initializing and be ready to respond to commands
from the host. The driver already had handling for the post-reset delay
for I2C and SPI transports, this change adds the handling to
SMBus-connected devices.
SMBus devices are peculiar because they implement legacy PS/2
compatibility mode, so reset is actually issued by psmouse driver on the
associated serio port, after which the control is passed to the RMI4
driver with SMBus companion device.
Note that originally the delay was added to psmouse driver in
92e24e0e57f7 ("Input: psmouse - add delay when deactivating for SMBus
mode"), but that resulted in an unwanted delay in "fast" reconnect
handler for the serio port, so it was decided to revert the patch and
have the delay being handled in the RMI4 driver, similar to the other
transports.
Tested-by: Jeffery Miller <jefferymiller@google.com>
Link: https://lore.kernel.org/r/ZR1yUFJ8a9Zt606N@penguin
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/input/mouse/synaptics.c | 1 +
drivers/input/rmi4/rmi_smbus.c | 50 ++++++++++++++++++---------------
2 files changed, 29 insertions(+), 22 deletions(-)
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index ada299ec5bba5..6ccc4a099b510 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -1752,6 +1752,7 @@ static int synaptics_create_intertouch(struct psmouse *psmouse,
psmouse_matches_pnp_id(psmouse, topbuttonpad_pnp_ids) &&
!SYN_CAP_EXT_BUTTONS_STICK(info->ext_cap_10);
const struct rmi_device_platform_data pdata = {
+ .reset_delay_ms = 30,
.sensor_pdata = {
.sensor_type = rmi_sensor_touchpad,
.axis_align.flip_y = true,
diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
index 7059a2762aebc..b0b099b5528a8 100644
--- a/drivers/input/rmi4/rmi_smbus.c
+++ b/drivers/input/rmi4/rmi_smbus.c
@@ -235,12 +235,29 @@ static void rmi_smb_clear_state(struct rmi_smb_xport *rmi_smb)
static int rmi_smb_enable_smbus_mode(struct rmi_smb_xport *rmi_smb)
{
- int retval;
+ struct i2c_client *client = rmi_smb->client;
+ int smbus_version;
+
+ /*
+ * psmouse driver resets the controller, we only need to wait
+ * to give the firmware chance to fully reinitialize.
+ */
+ if (rmi_smb->xport.pdata.reset_delay_ms)
+ msleep(rmi_smb->xport.pdata.reset_delay_ms);
/* we need to get the smbus version to activate the touchpad */
- retval = rmi_smb_get_version(rmi_smb);
- if (retval < 0)
- return retval;
+ smbus_version = rmi_smb_get_version(rmi_smb);
+ if (smbus_version < 0)
+ return smbus_version;
+
+ rmi_dbg(RMI_DEBUG_XPORT, &client->dev, "Smbus version is %d",
+ smbus_version);
+
+ if (smbus_version != 2 && smbus_version != 3) {
+ dev_err(&client->dev, "Unrecognized SMB version %d\n",
+ smbus_version);
+ return -ENODEV;
+ }
return 0;
}
@@ -253,11 +270,10 @@ static int rmi_smb_reset(struct rmi_transport_dev *xport, u16 reset_addr)
rmi_smb_clear_state(rmi_smb);
/*
- * we do not call the actual reset command, it has to be handled in
- * PS/2 or there will be races between PS/2 and SMBus.
- * PS/2 should ensure that a psmouse_reset is called before
- * intializing the device and after it has been removed to be in a known
- * state.
+ * We do not call the actual reset command, it has to be handled in
+ * PS/2 or there will be races between PS/2 and SMBus. PS/2 should
+ * ensure that a psmouse_reset is called before initializing the
+ * device and after it has been removed to be in a known state.
*/
return rmi_smb_enable_smbus_mode(rmi_smb);
}
@@ -272,7 +288,6 @@ static int rmi_smb_probe(struct i2c_client *client)
{
struct rmi_device_platform_data *pdata = dev_get_platdata(&client->dev);
struct rmi_smb_xport *rmi_smb;
- int smbus_version;
int error;
if (!pdata) {
@@ -311,18 +326,9 @@ static int rmi_smb_probe(struct i2c_client *client)
rmi_smb->xport.proto_name = "smb";
rmi_smb->xport.ops = &rmi_smb_ops;
- smbus_version = rmi_smb_get_version(rmi_smb);
- if (smbus_version < 0)
- return smbus_version;
-
- rmi_dbg(RMI_DEBUG_XPORT, &client->dev, "Smbus version is %d",
- smbus_version);
-
- if (smbus_version != 2 && smbus_version != 3) {
- dev_err(&client->dev, "Unrecognized SMB version %d\n",
- smbus_version);
- return -ENODEV;
- }
+ error = rmi_smb_enable_smbus_mode(rmi_smb);
+ if (error)
+ return error;
i2c_set_clientdata(client, rmi_smb);
--
2.40.1
^ permalink raw reply related
* [PATCH AUTOSEL 6.5 30/31] Input: xpad - add PXN V900 support
From: Sasha Levin @ 2023-10-18 14:11 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Matthias Berndt, Dmitry Torokhov, Sasha Levin, vi, swyterzone,
aicommander, carl.ng, matthias.benkmann, slouken, linux-input
In-Reply-To: <20231018141151.1334501-1-sashal@kernel.org>
From: Matthias Berndt <matthias_berndt@gmx.de>
[ Upstream commit a65cd7ef5a864bdbbe037267c327786b7759d4c6 ]
Add VID and PID to the xpad_device table to allow driver to use the PXN
V900 steering wheel, which is XTYPE_XBOX360 compatible in xinput mode.
Signed-off-by: Matthias Berndt <matthias_berndt@gmx.de>
Link: https://lore.kernel.org/r/4932699.31r3eYUQgx@fedora
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/input/joystick/xpad.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 1e377d040c435..fab2e7240ef22 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -272,6 +272,7 @@ static const struct xpad_device {
{ 0x1038, 0x1430, "SteelSeries Stratus Duo", 0, XTYPE_XBOX360 },
{ 0x1038, 0x1431, "SteelSeries Stratus Duo", 0, XTYPE_XBOX360 },
{ 0x11c9, 0x55f0, "Nacon GC-100XF", 0, XTYPE_XBOX360 },
+ { 0x11ff, 0x0511, "PXN V900", 0, XTYPE_XBOX360 },
{ 0x1209, 0x2882, "Ardwiino Controller", 0, XTYPE_XBOX360 },
{ 0x12ab, 0x0004, "Honey Bee Xbox360 dancepad", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360 },
{ 0x12ab, 0x0301, "PDP AFTERGLOW AX.1", 0, XTYPE_XBOX360 },
@@ -477,6 +478,7 @@ static const struct usb_device_id xpad_table[] = {
XPAD_XBOX360_VENDOR(0x1038), /* SteelSeries controllers */
XPAD_XBOXONE_VENDOR(0x10f5), /* Turtle Beach Controllers */
XPAD_XBOX360_VENDOR(0x11c9), /* Nacon GC100XF */
+ XPAD_XBOX360_VENDOR(0x11ff), /* PXN V900 */
XPAD_XBOX360_VENDOR(0x1209), /* Ardwiino Controllers */
XPAD_XBOX360_VENDOR(0x12ab), /* Xbox 360 dance pads */
XPAD_XBOX360_VENDOR(0x1430), /* RedOctane Xbox 360 controllers */
--
2.40.1
^ permalink raw reply related
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