| Message ID | 20240702080626.2902171-1-Shyam-sundar.S-k@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/2] platform/x86/amd/pmf: Use existing input event codes to update system states | expand |
On 7/2/2024 3:06, Shyam Sundar S K wrote: > At present, the PMF driver employs custom system state codes to update > system states. It is recommended to replace these with existing input > event codes (KEY_SLEEP, KEY_SUSPEND, and KEY_SCREENLOCK) to align system > updates with the PMF-TA output actions. > > Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com> > Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com> > Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/platform/x86/amd/pmf/pmf.h | 2 + > drivers/platform/x86/amd/pmf/tee-if.c | 62 +++++++++++++++++++++------ > 2 files changed, 52 insertions(+), 12 deletions(-) > > diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h > index eeedd0c0395a..753d5662c080 100644 > --- a/drivers/platform/x86/amd/pmf/pmf.h > +++ b/drivers/platform/x86/amd/pmf/pmf.h > @@ -12,6 +12,7 @@ > #define PMF_H > > #include <linux/acpi.h> > +#include <linux/input.h> > #include <linux/platform_profile.h> > > #define POLICY_BUF_MAX_SZ 0x4b000 > @@ -300,6 +301,7 @@ struct amd_pmf_dev { > void __iomem *policy_base; > bool smart_pc_enabled; > u16 pmf_if_version; > + struct input_dev *pmf_idev; > }; > > struct apmf_sps_prop_granular_v2 { > diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c > index b438de4d6bfc..b0449f912048 100644 > --- a/drivers/platform/x86/amd/pmf/tee-if.c > +++ b/drivers/platform/x86/amd/pmf/tee-if.c > @@ -62,18 +62,12 @@ static void amd_pmf_prepare_args(struct amd_pmf_dev *dev, int cmd, > param[0].u.memref.shm_offs = 0; > } > > -static int amd_pmf_update_uevents(struct amd_pmf_dev *dev, u16 event) > +static void amd_pmf_update_uevents(struct amd_pmf_dev *dev, u16 event) > { > - char *envp[2] = {}; > - > - envp[0] = kasprintf(GFP_KERNEL, "EVENT_ID=%d", event); > - if (!envp[0]) > - return -EINVAL; > - > - kobject_uevent_env(&dev->dev->kobj, KOBJ_CHANGE, envp); > - > - kfree(envp[0]); > - return 0; > + input_report_key(dev->pmf_idev, event, 1); /* key press */ > + input_sync(dev->pmf_idev); > + input_report_key(dev->pmf_idev, event, 0); /* key release */ > + input_sync(dev->pmf_idev); > } > > static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out) > @@ -149,7 +143,20 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_ > break; > > case PMF_POLICY_SYSTEM_STATE: > - amd_pmf_update_uevents(dev, val); > + switch (val) { > + case 0: > + amd_pmf_update_uevents(dev, KEY_SLEEP); > + break; > + case 1: > + amd_pmf_update_uevents(dev, KEY_SUSPEND); > + break; > + case 2: > + amd_pmf_update_uevents(dev, KEY_SCREENLOCK); > + break; > + default: > + dev_err(dev->dev, "Invalid PMF policy system state: %d\n", val); > + } > + > dev_dbg(dev->dev, "update SYSTEM_STATE: %s\n", > amd_pmf_uevent_as_str(val)); > break; > @@ -368,6 +375,30 @@ static int amd_pmf_ta_open_session(struct tee_context *ctx, u32 *id) > return rc; > } > > +static int amd_pmf_register_input_device(struct amd_pmf_dev *dev) > +{ > + int err; > + > + dev->pmf_idev = devm_input_allocate_device(dev->dev); > + if (!dev->pmf_idev) > + return -ENOMEM; > + > + dev->pmf_idev->name = "PMF-TA output events"; > + dev->pmf_idev->phys = "amd-pmf/input0"; > + > + input_set_capability(dev->pmf_idev, EV_KEY, KEY_SLEEP); > + input_set_capability(dev->pmf_idev, EV_KEY, KEY_SCREENLOCK); > + input_set_capability(dev->pmf_idev, EV_KEY, KEY_SUSPEND); > + > + err = input_register_device(dev->pmf_idev); > + if (err) { > + dev_err(dev->dev, "Failed to register input device: %d\n", err); > + return err; > + } > + > + return 0; > +} > + > static int amd_pmf_tee_init(struct amd_pmf_dev *dev) > { > u32 size; > @@ -475,6 +506,10 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev) > if (pb_side_load) > amd_pmf_open_pb(dev, dev->dbgfs_dir); > > + ret = amd_pmf_register_input_device(dev); > + if (ret) > + goto error; > + > return 0; > > error: > @@ -488,6 +523,9 @@ void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev) > if (pb_side_load && dev->esbin) > amd_pmf_remove_pb(dev); > > + if (dev->pmf_idev) > + input_unregister_device(dev->pmf_idev); > + > cancel_delayed_work_sync(&dev->pb_work); > kfree(dev->prev_data); > dev->prev_data = NULL;
On Tue, 2 Jul 2024, Shyam Sundar S K wrote: > At present, the PMF driver employs custom system state codes to update > system states. It is recommended to replace these with existing input This change entirely removes the way userspace worked before this change? We cannot take userspace functionality away like this.
On 7/6/2024 18:39, Ilpo Järvinen wrote: > On Tue, 2 Jul 2024, Shyam Sundar S K wrote: > >> At present, the PMF driver employs custom system state codes to update >> system states. It is recommended to replace these with existing input > > This change entirely removes the way userspace worked before this change? No. It's still the same way it worked like before; just that we are removing an additional technical debt of maintaining a separate udev rules file for kicking in the user-space action. GNOME folks told that they cannot have a custom actions defined in udev rules, instead asked to use KEY_SCREENLOCK, KEY_SLEEP, and KEY_SUSPEND so it entirely matches the behavior. > We cannot take userspace functionality away like this. > Can you please take a look at PATCH 2/2, that may help to clarify on why PATCH 1/2 is required. Thanks, Shyam
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h index eeedd0c0395a..753d5662c080 100644 --- a/drivers/platform/x86/amd/pmf/pmf.h +++ b/drivers/platform/x86/amd/pmf/pmf.h @@ -12,6 +12,7 @@ #define PMF_H #include <linux/acpi.h> +#include <linux/input.h> #include <linux/platform_profile.h> #define POLICY_BUF_MAX_SZ 0x4b000 @@ -300,6 +301,7 @@ struct amd_pmf_dev { void __iomem *policy_base; bool smart_pc_enabled; u16 pmf_if_version; + struct input_dev *pmf_idev; }; struct apmf_sps_prop_granular_v2 { diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c index b438de4d6bfc..b0449f912048 100644 --- a/drivers/platform/x86/amd/pmf/tee-if.c +++ b/drivers/platform/x86/amd/pmf/tee-if.c @@ -62,18 +62,12 @@ static void amd_pmf_prepare_args(struct amd_pmf_dev *dev, int cmd, param[0].u.memref.shm_offs = 0; } -static int amd_pmf_update_uevents(struct amd_pmf_dev *dev, u16 event) +static void amd_pmf_update_uevents(struct amd_pmf_dev *dev, u16 event) { - char *envp[2] = {}; - - envp[0] = kasprintf(GFP_KERNEL, "EVENT_ID=%d", event); - if (!envp[0]) - return -EINVAL; - - kobject_uevent_env(&dev->dev->kobj, KOBJ_CHANGE, envp); - - kfree(envp[0]); - return 0; + input_report_key(dev->pmf_idev, event, 1); /* key press */ + input_sync(dev->pmf_idev); + input_report_key(dev->pmf_idev, event, 0); /* key release */ + input_sync(dev->pmf_idev); } static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out) @@ -149,7 +143,20 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_ break; case PMF_POLICY_SYSTEM_STATE: - amd_pmf_update_uevents(dev, val); + switch (val) { + case 0: + amd_pmf_update_uevents(dev, KEY_SLEEP); + break; + case 1: + amd_pmf_update_uevents(dev, KEY_SUSPEND); + break; + case 2: + amd_pmf_update_uevents(dev, KEY_SCREENLOCK); + break; + default: + dev_err(dev->dev, "Invalid PMF policy system state: %d\n", val); + } + dev_dbg(dev->dev, "update SYSTEM_STATE: %s\n", amd_pmf_uevent_as_str(val)); break; @@ -368,6 +375,30 @@ static int amd_pmf_ta_open_session(struct tee_context *ctx, u32 *id) return rc; } +static int amd_pmf_register_input_device(struct amd_pmf_dev *dev) +{ + int err; + + dev->pmf_idev = devm_input_allocate_device(dev->dev); + if (!dev->pmf_idev) + return -ENOMEM; + + dev->pmf_idev->name = "PMF-TA output events"; + dev->pmf_idev->phys = "amd-pmf/input0"; + + input_set_capability(dev->pmf_idev, EV_KEY, KEY_SLEEP); + input_set_capability(dev->pmf_idev, EV_KEY, KEY_SCREENLOCK); + input_set_capability(dev->pmf_idev, EV_KEY, KEY_SUSPEND); + + err = input_register_device(dev->pmf_idev); + if (err) { + dev_err(dev->dev, "Failed to register input device: %d\n", err); + return err; + } + + return 0; +} + static int amd_pmf_tee_init(struct amd_pmf_dev *dev) { u32 size; @@ -475,6 +506,10 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev) if (pb_side_load) amd_pmf_open_pb(dev, dev->dbgfs_dir); + ret = amd_pmf_register_input_device(dev); + if (ret) + goto error; + return 0; error: @@ -488,6 +523,9 @@ void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev) if (pb_side_load && dev->esbin) amd_pmf_remove_pb(dev); + if (dev->pmf_idev) + input_unregister_device(dev->pmf_idev); + cancel_delayed_work_sync(&dev->pb_work); kfree(dev->prev_data); dev->prev_data = NULL;