Nowadays many laptops have camera access keys, yet there is no usage codes
mapped to them, even though it's introduced in HUTRR72. Start point of
this patch series was adding it and making IdeaPads send it to userspace.
But later I discovered that camera_power attribute of ideapad-laptop
driver on my IdeaPad 520-15IKB doesn't work, so I can't toggle it with
that. I managed to find a way to check whether an IdeaPad supports
camera_power attribute (which sends VPCCMD_W_CAMERA to EC), don't expose
it to sysfs so userspace will know that it can't toggle camera access via
camera_power, in my case, after receiving KEY_CAMERA_ACCESS_TOGGLE.
Along the way I discovered that old IdeaPads, like S10-3, may not be able
to toggle their touchpad as a regression of a commit aimed for newer
IdeaPads, so I reverted it.
Also I noticed that I can get/set the state of my keyboard light,
so one of the patches also adds supports for this kind of keyboard lights,
which I call "partially supported keyboard lights". I expect that commit
to add keyboard light support for 520-15IKB, 330-17ICH, 5 (15) and more.
Currently only tested on 520-15IKB.
---
Changes in v2:
- Added Dmitry Torokhov's Acked-By to patch 2
- Applied Barnabás Pőcze's recommendations to patch 5:
- strncmp -> strstarts
- static global "CAM" string -> inlined "CAM" string
- move new variables to the scope they're used, and order them
- Added patch 7, which removes "touchpad" attr for SYNA2B33
Eray Orçunus (7):
Revert "platform/x86: ideapad-laptop: check for touchpad support in
_CFG"
HID: add mapping for camera access keys
platform/x86: ideapad-laptop: Report KEY_CAMERA_ACCESS_TOGGLE instead
of KEY_CAMERA
platform/x86: ideapad-laptop: Add new _CFG bit numbers for future use
platform/x86: ideapad-laptop: Expose camera_power only if supported
platform/x86: ideapad-laptop: Keyboard backlight support for more
IdeaPads
platform/x86: ideapad-laptop: Don't expose touchpad attr on IdeaPads
with SYNA2B33
drivers/hid/hid-debug.c | 3 +
drivers/hid/hid-input.c | 3 +
drivers/platform/x86/ideapad-laptop.c | 170 ++++++++++++++++++++++---
include/uapi/linux/input-event-codes.h | 3 +
4 files changed, 162 insertions(+), 17 deletions(-)
base-commit: d9db04c1dec6189413701c52b9498a7a56c96445
--
2.34.1
IdeaPads dropped support for VPCCMD_W_CAMERA somewhere between 2014-2016,
none of the IdeaPads produced after that I tested supports it. Fortunately
I found a way to check it; if the DSDT has camera device(s) defined, it
shouldn't have working VPCCMD_W_CAMERA, thus camera_power shouldn't be
exposed to sysfs. To accomplish this, walk the ACPI namespace in
ideapad_check_features and check the devices starting with "CAM".
Tested on 520-15IKB and Legion Y520, which successfully didn't expose
the camera_power attribute.
Link: https://www.spinics.net/lists/platform-driver-x86/msg26147.html
Signed-off-by: Eray Orçunus <[email protected]>
---
drivers/platform/x86/ideapad-laptop.c | 52 ++++++++++++++++++++++++++-
1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index f3d4f2beda07..e8c088e7a53d 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -149,6 +149,7 @@ struct ideapad_private {
bool fn_lock : 1;
bool hw_rfkill_switch : 1;
bool kbd_bl : 1;
+ bool cam_ctrl_via_ec : 1;
bool touchpad_ctrl_via_ec : 1;
bool usb_charging : 1;
} features;
@@ -163,6 +164,24 @@ static bool no_bt_rfkill;
module_param(no_bt_rfkill, bool, 0444);
MODULE_PARM_DESC(no_bt_rfkill, "No rfkill for bluetooth.");
+static acpi_status acpi_find_device_callback(acpi_handle handle, u32 level,
+ void *context, void **return_value)
+{
+ struct acpi_buffer ret_buf;
+ char buffer[8];
+
+ ret_buf.length = sizeof(buffer);
+ ret_buf.pointer = buffer;
+
+ if (ACPI_SUCCESS(acpi_get_name(handle, ACPI_SINGLE_NAME, &ret_buf)))
+ if (strstarts(ret_buf.pointer, context)) {
+ *return_value = handle;
+ return AE_CTRL_TERMINATE;
+ }
+
+ return AE_OK;
+}
+
/*
* ACPI Helpers
*/
@@ -675,7 +694,7 @@ static umode_t ideapad_is_visible(struct kobject *kobj,
bool supported = true;
if (attr == &dev_attr_camera_power.attr)
- supported = test_bit(CFG_CAP_CAM_BIT, &priv->cfg);
+ supported = priv->features.cam_ctrl_via_ec;
else if (attr == &dev_attr_conservation_mode.attr)
supported = priv->features.conservation_mode;
else if (attr == &dev_attr_fan_mode.attr)
@@ -1527,6 +1546,37 @@ static void ideapad_check_features(struct ideapad_private *priv)
priv->features.hw_rfkill_switch = dmi_check_system(hw_rfkill_list);
+ /*
+ * Some IdeaPads have camera switch via EC (mostly older ones),
+ * some don't. Fortunately we know that if DSDT contains device
+ * object for the camera, camera isn't switchable via EC.
+ * So, let's walk the namespace and try to find CAM* object.
+ * If we can't find it, set cam_ctrl_via_ec to true.
+ */
+
+ priv->features.cam_ctrl_via_ec = false;
+
+ if (test_bit(CFG_CAP_CAM_BIT, &priv->cfg)) {
+ acpi_handle temp_handle = NULL;
+ acpi_handle pci_handle;
+ acpi_status status;
+
+ status = acpi_get_handle(handle, "^^^", &pci_handle);
+ if (ACPI_SUCCESS(status)) {
+ status = acpi_walk_namespace(ACPI_TYPE_DEVICE, pci_handle,
+ ACPI_UINT32_MAX,
+ acpi_find_device_callback,
+ NULL, "CAM",
+ &temp_handle);
+
+ if (ACPI_SUCCESS(status) && temp_handle == NULL)
+ priv->features.cam_ctrl_via_ec = true;
+
+ } else
+ dev_warn(&priv->platform_device->dev,
+ "Could not find PCI* node in the namespace\n");
+ }
+
/* Most ideapads with ELAN0634 touchpad don't use EC touchpad switch */
priv->features.touchpad_ctrl_via_ec = !acpi_dev_present("ELAN0634", NULL, -1);
--
2.34.1
Later IdeaPads report various things in last 8 bits of _CFG, at least
5 of them represent supported on-screen-displays. Add those bit numbers
to the enum, and use CFG_OSD_ as prefix of their names. Also expose
the values of these bits to debugfs, since they can be useful.
Signed-off-by: Eray Orçunus <[email protected]>
---
drivers/platform/x86/ideapad-laptop.c | 33 +++++++++++++++++++++++----
1 file changed, 29 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 0ef40b88b240..f3d4f2beda07 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -46,10 +46,22 @@ static const char *const ideapad_wmi_fnesc_events[] = {
#endif
enum {
- CFG_CAP_BT_BIT = 16,
- CFG_CAP_3G_BIT = 17,
- CFG_CAP_WIFI_BIT = 18,
- CFG_CAP_CAM_BIT = 19,
+ CFG_CAP_BT_BIT = 16,
+ CFG_CAP_3G_BIT = 17,
+ CFG_CAP_WIFI_BIT = 18,
+ CFG_CAP_CAM_BIT = 19,
+
+ /*
+ * These are OnScreenDisplay support bits that can be useful to determine
+ * whether a hotkey exists/should show OSD. But they aren't particularly
+ * meaningful since they were introduced later, i.e. 2010 IdeaPads
+ * don't have these, but they still have had OSD for hotkeys.
+ */
+ CFG_OSD_NUMLK_BIT = 27,
+ CFG_OSD_CAPSLK_BIT = 28,
+ CFG_OSD_MICMUTE_BIT = 29,
+ CFG_OSD_TOUCHPAD_BIT = 30,
+ CFG_OSD_CAM_BIT = 31,
};
enum {
@@ -368,6 +380,19 @@ static int debugfs_cfg_show(struct seq_file *s, void *data)
seq_puts(s, " camera");
seq_puts(s, "\n");
+ seq_puts(s, "OSD support:");
+ if (test_bit(CFG_OSD_NUMLK_BIT, &priv->cfg))
+ seq_puts(s, " num-lock");
+ if (test_bit(CFG_OSD_CAPSLK_BIT, &priv->cfg))
+ seq_puts(s, " caps-lock");
+ if (test_bit(CFG_OSD_MICMUTE_BIT, &priv->cfg))
+ seq_puts(s, " mic-mute");
+ if (test_bit(CFG_OSD_TOUCHPAD_BIT, &priv->cfg))
+ seq_puts(s, " touchpad");
+ if (test_bit(CFG_OSD_CAM_BIT, &priv->cfg))
+ seq_puts(s, " camera");
+ seq_puts(s, "\n");
+
seq_puts(s, "Graphics: ");
switch (priv->cfg & 0x700) {
case 0x100:
--
2.34.1
HUTRR72 added 3 new usage codes for keys that are supposed to enable,
disable and toggle camera access. These are useful, considering many
laptops today have key(s) for toggling access to camera.
This patch adds new key definitions for KEY_CAMERA_ACCESS_ENABLE,
KEY_CAMERA_ACCESS_DISABLE and KEY_CAMERA_ACCESS_TOGGLE. Additionally
hid-debug is adjusted to recognize this new usage codes as well.
Signed-off-by: Eray Orçunus <[email protected]>
Acked-by: Dmitry Torokhov <[email protected]>
---
drivers/hid/hid-debug.c | 3 +++
drivers/hid/hid-input.c | 3 +++
include/uapi/linux/input-event-codes.h | 3 +++
3 files changed, 9 insertions(+)
diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index f48d3534e020..991f880fdbd4 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c
@@ -936,6 +936,9 @@ static const char *keys[KEY_MAX + 1] = {
[KEY_ASSISTANT] = "Assistant",
[KEY_KBD_LAYOUT_NEXT] = "KbdLayoutNext",
[KEY_EMOJI_PICKER] = "EmojiPicker",
+ [KEY_CAMERA_ACCESS_ENABLE] = "CameraAccessEnable",
+ [KEY_CAMERA_ACCESS_DISABLE] = "CameraAccessDisable",
+ [KEY_CAMERA_ACCESS_TOGGLE] = "CameraAccessToggle",
[KEY_DICTATE] = "Dictate",
[KEY_BRIGHTNESS_MIN] = "BrightnessMin",
[KEY_BRIGHTNESS_MAX] = "BrightnessMax",
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index f197aed6444a..f8e6513e77b8 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -995,6 +995,9 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
case 0x0cd: map_key_clear(KEY_PLAYPAUSE); break;
case 0x0cf: map_key_clear(KEY_VOICECOMMAND); break;
+ case 0x0d5: map_key_clear(KEY_CAMERA_ACCESS_ENABLE); break;
+ case 0x0d6: map_key_clear(KEY_CAMERA_ACCESS_DISABLE); break;
+ case 0x0d7: map_key_clear(KEY_CAMERA_ACCESS_TOGGLE); break;
case 0x0d8: map_key_clear(KEY_DICTATE); break;
case 0x0d9: map_key_clear(KEY_EMOJI_PICKER); break;
diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 7989d9483ea7..ef392d0f943f 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -614,6 +614,9 @@
#define KEY_KBD_LAYOUT_NEXT 0x248 /* AC Next Keyboard Layout Select */
#define KEY_EMOJI_PICKER 0x249 /* Show/hide emoji picker (HUTRR101) */
#define KEY_DICTATE 0x24a /* Start or Stop Voice Dictation Session (HUTRR99) */
+#define KEY_CAMERA_ACCESS_ENABLE 0x24b /* Enables programmatic access to camera devices. (HUTRR72) */
+#define KEY_CAMERA_ACCESS_DISABLE 0x24c /* Disables programmatic access to camera devices. (HUTRR72) */
+#define KEY_CAMERA_ACCESS_TOGGLE 0x24d /* Toggles the current state of the camera access control. (HUTRR72) */
#define KEY_BRIGHTNESS_MIN 0x250 /* Set Brightness to Minimum */
#define KEY_BRIGHTNESS_MAX 0x251 /* Set Brightness to Maximum */
--
2.34.1
On Sat, 29 Oct 2022, Eray Orçunus wrote:
> HUTRR72 added 3 new usage codes for keys that are supposed to enable,
> disable and toggle camera access. These are useful, considering many
> laptops today have key(s) for toggling access to camera.
>
> This patch adds new key definitions for KEY_CAMERA_ACCESS_ENABLE,
> KEY_CAMERA_ACCESS_DISABLE and KEY_CAMERA_ACCESS_TOGGLE. Additionally
> hid-debug is adjusted to recognize this new usage codes as well.
>
> Signed-off-by: Eray Orçunus <[email protected]>
> Acked-by: Dmitry Torokhov <[email protected]>
Acked-by: Jiri Kosina <[email protected]>
--
Jiri Kosina
SUSE Labs
On 10/29/22 20:03, Eray Orçunus wrote:
> Nowadays many laptops have camera access keys, yet there is no usage codes
> mapped to them, even though it's introduced in HUTRR72. Start point of
> this patch series was adding it and making IdeaPads send it to userspace.
> But later I discovered that camera_power attribute of ideapad-laptop
> driver on my IdeaPad 520-15IKB doesn't work, so I can't toggle it with
> that. I managed to find a way to check whether an IdeaPad supports
> camera_power attribute (which sends VPCCMD_W_CAMERA to EC), don't expose
> it to sysfs so userspace will know that it can't toggle camera access via
> camera_power, in my case, after receiving KEY_CAMERA_ACCESS_TOGGLE.
>
> Along the way I discovered that old IdeaPads, like S10-3, may not be able
> to toggle their touchpad as a regression of a commit aimed for newer
> IdeaPads, so I reverted it.
>
> Also I noticed that I can get/set the state of my keyboard light,
> so one of the patches also adds supports for this kind of keyboard lights,
> which I call "partially supported keyboard lights". I expect that commit
> to add keyboard light support for 520-15IKB, 330-17ICH, 5 (15) and more.
> Currently only tested on 520-15IKB.
Thanks. Also test on my ideapad s410 and it looks good.
Acked-by: Ike Panhc <[email protected]>
On 11/08/22 06:56, Ike Panhc wrote:
> On 10/29/22 20:03, Eray Orçunus wrote:
> > Nowadays many laptops have camera access keys, yet there is no usage codes
> > mapped to them, even though it's introduced in HUTRR72. Start point of
> > this patch series was adding it and making IdeaPads send it to userspace.
> > But later I discovered that camera_power attribute of ideapad-laptop
> > driver on my IdeaPad 520-15IKB doesn't work, so I can't toggle it with
> > that. I managed to find a way to check whether an IdeaPad supports
> > camera_power attribute (which sends VPCCMD_W_CAMERA to EC), don't expose
> > it to sysfs so userspace will know that it can't toggle camera access via
> > camera_power, in my case, after receiving KEY_CAMERA_ACCESS_TOGGLE.
> >
> > Along the way I discovered that old IdeaPads, like S10-3, may not be able
> > to toggle their touchpad as a regression of a commit aimed for newer
> > IdeaPads, so I reverted it.
> >
> > Also I noticed that I can get/set the state of my keyboard light,
> > so one of the patches also adds supports for this kind of keyboard lights,
> > which I call "partially supported keyboard lights". I expect that commit
> > to add keyboard light support for 520-15IKB, 330-17ICH, 5 (15) and more.
> > Currently only tested on 520-15IKB.
>
> Thanks. Also test on my ideapad s410 and it looks good.
>
> Acked-by: Ike Panhc <[email protected]>
Thank you :)
I need some advice since I'm new in here, sadly another patch has been
merged to ideapad-laptop along the way and currently it's not possible to
merge patch #7, does that mean I should send v3 of my patch series?
And whom should I wait for merge, x86 platform drivers maintainers?
I think that is the only subsystem whose maintainers haven't replied yet.
-eray
Hi Eray,
Sorry for the long silence, I have not done any pdx86 patch review
the last 2 weeks due to personal circumstances.
On 11/9/22 13:58, Eray Orçunus wrote:
> On 11/08/22 06:56, Ike Panhc wrote:
>> On 10/29/22 20:03, Eray Orçunus wrote:
>>> Nowadays many laptops have camera access keys, yet there is no usage codes
>>> mapped to them, even though it's introduced in HUTRR72. Start point of
>>> this patch series was adding it and making IdeaPads send it to userspace.
>>> But later I discovered that camera_power attribute of ideapad-laptop
>>> driver on my IdeaPad 520-15IKB doesn't work, so I can't toggle it with
>>> that. I managed to find a way to check whether an IdeaPad supports
>>> camera_power attribute (which sends VPCCMD_W_CAMERA to EC), don't expose
>>> it to sysfs so userspace will know that it can't toggle camera access via
>>> camera_power, in my case, after receiving KEY_CAMERA_ACCESS_TOGGLE.
>>>
>>> Along the way I discovered that old IdeaPads, like S10-3, may not be able
>>> to toggle their touchpad as a regression of a commit aimed for newer
>>> IdeaPads, so I reverted it.
>>>
>>> Also I noticed that I can get/set the state of my keyboard light,
>>> so one of the patches also adds supports for this kind of keyboard lights,
>>> which I call "partially supported keyboard lights". I expect that commit
>>> to add keyboard light support for 520-15IKB, 330-17ICH, 5 (15) and more.
>>> Currently only tested on 520-15IKB.
>>
>> Thanks. Also test on my ideapad s410 and it looks good.
>>
>> Acked-by: Ike Panhc <[email protected]>
>
>
> Thank you :)
>
> I need some advice since I'm new in here, sadly another patch has been
> merged to ideapad-laptop along the way and currently it's not possible to
> merge patch #7, does that mean I should send v3 of my patch series?
No that is not necessary, I can rework it to apply on top of the other
patch.
But TBH I think we really need to work on a different solution for
the problem with the touchpad issues with ideapad-laptop we cannot
just keep adding touchpad and/or DMI ids because the driver is
breaking touchpad functionality left and right.
I will send out an email after this one to all authors of recent
patches which all do "priv->features.touchpad_ctrl_via_ec = 0"
in some way.
With a request to gather some more info of why exactly this is
necessary and to see if we cannot come up with a more generic fix.
> And whom should I wait for merge, x86 platform drivers maintainers?
I'm the x86 platform drivers maintainer.
I believe it makes sense to merge this series through the
x86 platform drivers git tree.
I need to coordinate the merging of patch 2/7 with wDmitry
(the input subsystem maintainer) I'll send him an email
about this. After that I can likely merge patches 2-6.
For the touchpad patches I would first like to get
a better handle on how to fix things more generic.
Specifically patch 1/7 will cause priv->features.touchpad_ctrl_via_ec
to get set to 1 on more models and since that is causing issues
I don't think that is a good idea (even though the patch does
make sense) and for 7/7 I hope to come up with something
more generic.
If you can run the tests from the touchpad mail soon that
would really help!
Note I do plan to send 7/7 out as a fix for 6.1 if we
run out of time wrt coming up with a recent fix. Getting
at least some fix out the door is also why I already
merged the other patch using the DMI ids.
> I think that is the only subsystem whose maintainers haven't replied yet.
Correct, but I have replied now :)
Regards,
Hans
Hi!
On 11/9/22 19:38, Hans de Goede wrote:
> Hi Eray,
>
> Sorry for the long silence, I have not done any pdx86 patch review
> the last 2 weeks due to personal circumstances.
Oh, I wasn't even aware I had to wait for pdx86 review, and Ike Panhc
just sent his Acked-By anyway, no problem at all.
> On 11/9/22 13:58, Eray Orçunus wrote:
> > On 11/08/22 06:56, Ike Panhc wrote:
> >>
> >> Thanks. Also test on my ideapad s410 and it looks good.
> >>
> >> Acked-by: Ike Panhc <[email protected]>
> >
> >
> > Thank you :)
> >
> > I need some advice since I'm new in here, sadly another patch has been
> > merged to ideapad-laptop along the way and currently it's not possible to
> > merge patch #7, does that mean I should send v3 of my patch series?
>
> No that is not necessary, I can rework it to apply on top of the other
> patch.
Oh, that's great, thank you.
> For the touchpad patches I would first like to get
> a better handle on how to fix things more generic.
>
> Specifically patch 1/7 will cause priv->features.touchpad_ctrl_via_ec
> to get set to 1 on more models and since that is causing issues
> I don't think that is a good idea (even though the patch does
> make sense) and for 7/7 I hope to come up with something
> more generic.
>
> If you can run the tests from the touchpad mail soon that
> would really help!
That sounds great! I will try to help as much as I can. And yeah,
I couldn't guess patch 1 can cause a regression on some IdeaPads.
> > I think that is the only subsystem whose maintainers haven't replied yet.
>
> Correct, but I have replied now :)
Hehe, this reply was very informative, thank you :)
Best,
Eray
Hi Dmitry,
On 10/29/22 14:03, Eray Orçunus wrote:
> HUTRR72 added 3 new usage codes for keys that are supposed to enable,
> disable and toggle camera access. These are useful, considering many
> laptops today have key(s) for toggling access to camera.
>
> This patch adds new key definitions for KEY_CAMERA_ACCESS_ENABLE,
> KEY_CAMERA_ACCESS_DISABLE and KEY_CAMERA_ACCESS_TOGGLE. Additionally
> hid-debug is adjusted to recognize this new usage codes as well.
>
> Signed-off-by: Eray Orçunus <[email protected]>
> Acked-by: Dmitry Torokhov <[email protected]>
I have rejected the drivers/platform/x86 patch which depends
on this, because it changes current behavior, potentially
breaking userspace.
Since this means I won't be taking any patches depending on
this I believe it is best if this is merged through the input tree.
Note this also has a:
Acked-by: Jiri Kosina <[email protected]>
tag given in this email thread.
Regards,
Hans
> ---
> drivers/hid/hid-debug.c | 3 +++
> drivers/hid/hid-input.c | 3 +++
> include/uapi/linux/input-event-codes.h | 3 +++
> 3 files changed, 9 insertions(+)
>
> diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
> index f48d3534e020..991f880fdbd4 100644
> --- a/drivers/hid/hid-debug.c
> +++ b/drivers/hid/hid-debug.c
> @@ -936,6 +936,9 @@ static const char *keys[KEY_MAX + 1] = {
> [KEY_ASSISTANT] = "Assistant",
> [KEY_KBD_LAYOUT_NEXT] = "KbdLayoutNext",
> [KEY_EMOJI_PICKER] = "EmojiPicker",
> + [KEY_CAMERA_ACCESS_ENABLE] = "CameraAccessEnable",
> + [KEY_CAMERA_ACCESS_DISABLE] = "CameraAccessDisable",
> + [KEY_CAMERA_ACCESS_TOGGLE] = "CameraAccessToggle",
> [KEY_DICTATE] = "Dictate",
> [KEY_BRIGHTNESS_MIN] = "BrightnessMin",
> [KEY_BRIGHTNESS_MAX] = "BrightnessMax",
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index f197aed6444a..f8e6513e77b8 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -995,6 +995,9 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> case 0x0cd: map_key_clear(KEY_PLAYPAUSE); break;
> case 0x0cf: map_key_clear(KEY_VOICECOMMAND); break;
>
> + case 0x0d5: map_key_clear(KEY_CAMERA_ACCESS_ENABLE); break;
> + case 0x0d6: map_key_clear(KEY_CAMERA_ACCESS_DISABLE); break;
> + case 0x0d7: map_key_clear(KEY_CAMERA_ACCESS_TOGGLE); break;
> case 0x0d8: map_key_clear(KEY_DICTATE); break;
> case 0x0d9: map_key_clear(KEY_EMOJI_PICKER); break;
>
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 7989d9483ea7..ef392d0f943f 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -614,6 +614,9 @@
> #define KEY_KBD_LAYOUT_NEXT 0x248 /* AC Next Keyboard Layout Select */
> #define KEY_EMOJI_PICKER 0x249 /* Show/hide emoji picker (HUTRR101) */
> #define KEY_DICTATE 0x24a /* Start or Stop Voice Dictation Session (HUTRR99) */
> +#define KEY_CAMERA_ACCESS_ENABLE 0x24b /* Enables programmatic access to camera devices. (HUTRR72) */
> +#define KEY_CAMERA_ACCESS_DISABLE 0x24c /* Disables programmatic access to camera devices. (HUTRR72) */
> +#define KEY_CAMERA_ACCESS_TOGGLE 0x24d /* Toggles the current state of the camera access control. (HUTRR72) */
>
> #define KEY_BRIGHTNESS_MIN 0x250 /* Set Brightness to Minimum */
> #define KEY_BRIGHTNESS_MAX 0x251 /* Set Brightness to Maximum */
Hi,
On 10/29/22 14:03, Eray Orçunus wrote:
> Later IdeaPads report various things in last 8 bits of _CFG, at least
> 5 of them represent supported on-screen-displays. Add those bit numbers
> to the enum, and use CFG_OSD_ as prefix of their names. Also expose
> the values of these bits to debugfs, since they can be useful.
>
> Signed-off-by: Eray Orçunus <[email protected]>
Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
> ---
> drivers/platform/x86/ideapad-laptop.c | 33 +++++++++++++++++++++++----
> 1 file changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
> index 0ef40b88b240..f3d4f2beda07 100644
> --- a/drivers/platform/x86/ideapad-laptop.c
> +++ b/drivers/platform/x86/ideapad-laptop.c
> @@ -46,10 +46,22 @@ static const char *const ideapad_wmi_fnesc_events[] = {
> #endif
>
> enum {
> - CFG_CAP_BT_BIT = 16,
> - CFG_CAP_3G_BIT = 17,
> - CFG_CAP_WIFI_BIT = 18,
> - CFG_CAP_CAM_BIT = 19,
> + CFG_CAP_BT_BIT = 16,
> + CFG_CAP_3G_BIT = 17,
> + CFG_CAP_WIFI_BIT = 18,
> + CFG_CAP_CAM_BIT = 19,
> +
> + /*
> + * These are OnScreenDisplay support bits that can be useful to determine
> + * whether a hotkey exists/should show OSD. But they aren't particularly
> + * meaningful since they were introduced later, i.e. 2010 IdeaPads
> + * don't have these, but they still have had OSD for hotkeys.
> + */
> + CFG_OSD_NUMLK_BIT = 27,
> + CFG_OSD_CAPSLK_BIT = 28,
> + CFG_OSD_MICMUTE_BIT = 29,
> + CFG_OSD_TOUCHPAD_BIT = 30,
> + CFG_OSD_CAM_BIT = 31,
> };
>
> enum {
> @@ -368,6 +380,19 @@ static int debugfs_cfg_show(struct seq_file *s, void *data)
> seq_puts(s, " camera");
> seq_puts(s, "\n");
>
> + seq_puts(s, "OSD support:");
> + if (test_bit(CFG_OSD_NUMLK_BIT, &priv->cfg))
> + seq_puts(s, " num-lock");
> + if (test_bit(CFG_OSD_CAPSLK_BIT, &priv->cfg))
> + seq_puts(s, " caps-lock");
> + if (test_bit(CFG_OSD_MICMUTE_BIT, &priv->cfg))
> + seq_puts(s, " mic-mute");
> + if (test_bit(CFG_OSD_TOUCHPAD_BIT, &priv->cfg))
> + seq_puts(s, " touchpad");
> + if (test_bit(CFG_OSD_CAM_BIT, &priv->cfg))
> + seq_puts(s, " camera");
> + seq_puts(s, "\n");
> +
> seq_puts(s, "Graphics: ");
> switch (priv->cfg & 0x700) {
> case 0x100:
Hi Eray,
On 10/29/22 14:03, Eray Orçunus wrote:
> IdeaPads dropped support for VPCCMD_W_CAMERA somewhere between 2014-2016,
> none of the IdeaPads produced after that I tested supports it. Fortunately
> I found a way to check it; if the DSDT has camera device(s) defined, it
> shouldn't have working VPCCMD_W_CAMERA, thus camera_power shouldn't be
> exposed to sysfs. To accomplish this, walk the ACPI namespace in
> ideapad_check_features and check the devices starting with "CAM".
> Tested on 520-15IKB and Legion Y520, which successfully didn't expose
> the camera_power attribute.
>
> Link: https://www.spinics.net/lists/platform-driver-x86/msg26147.html
> Signed-off-by: Eray Orçunus <[email protected]>
> ---
> drivers/platform/x86/ideapad-laptop.c | 52 ++++++++++++++++++++++++++-
> 1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
> index f3d4f2beda07..e8c088e7a53d 100644
> --- a/drivers/platform/x86/ideapad-laptop.c
> +++ b/drivers/platform/x86/ideapad-laptop.c
> @@ -149,6 +149,7 @@ struct ideapad_private {
> bool fn_lock : 1;
> bool hw_rfkill_switch : 1;
> bool kbd_bl : 1;
> + bool cam_ctrl_via_ec : 1;
> bool touchpad_ctrl_via_ec : 1;
> bool usb_charging : 1;
> } features;
> @@ -163,6 +164,24 @@ static bool no_bt_rfkill;
> module_param(no_bt_rfkill, bool, 0444);
> MODULE_PARM_DESC(no_bt_rfkill, "No rfkill for bluetooth.");
>
> +static acpi_status acpi_find_device_callback(acpi_handle handle, u32 level,
> + void *context, void **return_value)
> +{
> + struct acpi_buffer ret_buf;
> + char buffer[8];
> +
> + ret_buf.length = sizeof(buffer);
> + ret_buf.pointer = buffer;
> +
> + if (ACPI_SUCCESS(acpi_get_name(handle, ACPI_SINGLE_NAME, &ret_buf)))
> + if (strstarts(ret_buf.pointer, context)) {
> + *return_value = handle;
> + return AE_CTRL_TERMINATE;
> + }
> +
> + return AE_OK;
> +}
> +
> /*
> * ACPI Helpers
> */
> @@ -675,7 +694,7 @@ static umode_t ideapad_is_visible(struct kobject *kobj,
> bool supported = true;
>
> if (attr == &dev_attr_camera_power.attr)
> - supported = test_bit(CFG_CAP_CAM_BIT, &priv->cfg);
> + supported = priv->features.cam_ctrl_via_ec;
> else if (attr == &dev_attr_conservation_mode.attr)
> supported = priv->features.conservation_mode;
> else if (attr == &dev_attr_fan_mode.attr)
> @@ -1527,6 +1546,37 @@ static void ideapad_check_features(struct ideapad_private *priv)
>
> priv->features.hw_rfkill_switch = dmi_check_system(hw_rfkill_list);
>
> + /*
> + * Some IdeaPads have camera switch via EC (mostly older ones),
> + * some don't. Fortunately we know that if DSDT contains device
> + * object for the camera, camera isn't switchable via EC.
> + * So, let's walk the namespace and try to find CAM* object.
> + * If we can't find it, set cam_ctrl_via_ec to true.
> + */
> +
> + priv->features.cam_ctrl_via_ec = false;
There is no need to explicitly set this to false since the entire
struct is allocated with kzalloc() and a bunch of other features
flags are also not explicitly set to false. Please drop this line.
> +
> + if (test_bit(CFG_CAP_CAM_BIT, &priv->cfg)) {
> + acpi_handle temp_handle = NULL;
> + acpi_handle pci_handle;
> + acpi_status status;
> +
> + status = acpi_get_handle(handle, "^^^", &pci_handle);
> + if (ACPI_SUCCESS(status)) {
> + status = acpi_walk_namespace(ACPI_TYPE_DEVICE, pci_handle,
> + ACPI_UINT32_MAX,
> + acpi_find_device_callback,
> + NULL, "CAM",
> + &temp_handle);
Why not just use acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, ... ?
The PCI root is usually pretty much the only object under the root anyways
and this way you can avoid the acpi_get_handle() call + its error handling,
so using ACPI_ROOT_OBJECT would lead to a nice cleanup.
> +
> + if (ACPI_SUCCESS(status) && temp_handle == NULL)
> + priv->features.cam_ctrl_via_ec = true;
> +
> + } else
> + dev_warn(&priv->platform_device->dev,
> + "Could not find PCI* node in the namespace\n");
> + }
> +
> /* Most ideapads with ELAN0634 touchpad don't use EC touchpad switch */
> priv->features.touchpad_ctrl_via_ec = !acpi_dev_present("ELAN0634", NULL, -1);
>
Regards,
Hans
Hi Eray,
On 11/15/22 21:43, Hans de Goede wrote:
> Hi Eray,
>
> On 10/29/22 14:03, Eray Orçunus wrote:
>> IdeaPads dropped support for VPCCMD_W_CAMERA somewhere between 2014-2016,
>> none of the IdeaPads produced after that I tested supports it. Fortunately
>> I found a way to check it; if the DSDT has camera device(s) defined, it
>> shouldn't have working VPCCMD_W_CAMERA, thus camera_power shouldn't be
>> exposed to sysfs. To accomplish this, walk the ACPI namespace in
>> ideapad_check_features and check the devices starting with "CAM".
>> Tested on 520-15IKB and Legion Y520, which successfully didn't expose
>> the camera_power attribute.
>>
>> Link: https://www.spinics.net/lists/platform-driver-x86/msg26147.html
>> Signed-off-by: Eray Orçunus <[email protected]>
>> ---
>> drivers/platform/x86/ideapad-laptop.c | 52 ++++++++++++++++++++++++++-
>> 1 file changed, 51 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
>> index f3d4f2beda07..e8c088e7a53d 100644
>> --- a/drivers/platform/x86/ideapad-laptop.c
>> +++ b/drivers/platform/x86/ideapad-laptop.c
>> @@ -149,6 +149,7 @@ struct ideapad_private {
>> bool fn_lock : 1;
>> bool hw_rfkill_switch : 1;
>> bool kbd_bl : 1;
>> + bool cam_ctrl_via_ec : 1;
>> bool touchpad_ctrl_via_ec : 1;
>> bool usb_charging : 1;
>> } features;
>> @@ -163,6 +164,24 @@ static bool no_bt_rfkill;
>> module_param(no_bt_rfkill, bool, 0444);
>> MODULE_PARM_DESC(no_bt_rfkill, "No rfkill for bluetooth.");
>>
>> +static acpi_status acpi_find_device_callback(acpi_handle handle, u32 level,
>> + void *context, void **return_value)
>> +{
>> + struct acpi_buffer ret_buf;
>> + char buffer[8];
>> +
>> + ret_buf.length = sizeof(buffer);
>> + ret_buf.pointer = buffer;
>> +
>> + if (ACPI_SUCCESS(acpi_get_name(handle, ACPI_SINGLE_NAME, &ret_buf)))
>> + if (strstarts(ret_buf.pointer, context)) {
>> + *return_value = handle;
>> + return AE_CTRL_TERMINATE;
>> + }
>> +
>> + return AE_OK;
>> +}
>> +
>> /*
>> * ACPI Helpers
>> */
>> @@ -675,7 +694,7 @@ static umode_t ideapad_is_visible(struct kobject *kobj,
>> bool supported = true;
>>
>> if (attr == &dev_attr_camera_power.attr)
>> - supported = test_bit(CFG_CAP_CAM_BIT, &priv->cfg);
>> + supported = priv->features.cam_ctrl_via_ec;
>> else if (attr == &dev_attr_conservation_mode.attr)
>> supported = priv->features.conservation_mode;
>> else if (attr == &dev_attr_fan_mode.attr)
>> @@ -1527,6 +1546,37 @@ static void ideapad_check_features(struct ideapad_private *priv)
>>
>> priv->features.hw_rfkill_switch = dmi_check_system(hw_rfkill_list);
>>
>> + /*
>> + * Some IdeaPads have camera switch via EC (mostly older ones),
>> + * some don't. Fortunately we know that if DSDT contains device
>> + * object for the camera, camera isn't switchable via EC.
>> + * So, let's walk the namespace and try to find CAM* object.
>> + * If we can't find it, set cam_ctrl_via_ec to true.
>> + */
>> +
>> + priv->features.cam_ctrl_via_ec = false;
>
> There is no need to explicitly set this to false since the entire
> struct is allocated with kzalloc() and a bunch of other features
> flags are also not explicitly set to false. Please drop this line.
>
>> +
>> + if (test_bit(CFG_CAP_CAM_BIT, &priv->cfg)) {
>> + acpi_handle temp_handle = NULL;
>> + acpi_handle pci_handle;
>> + acpi_status status;
>> +
>> + status = acpi_get_handle(handle, "^^^", &pci_handle);
>> + if (ACPI_SUCCESS(status)) {
>> + status = acpi_walk_namespace(ACPI_TYPE_DEVICE, pci_handle,
>> + ACPI_UINT32_MAX,
>> + acpi_find_device_callback,
>> + NULL, "CAM",
>> + &temp_handle);
>
> Why not just use acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, ... ?
>
> The PCI root is usually pretty much the only object under the root anyways
> and this way you can avoid the acpi_get_handle() call + its error handling,
> so using ACPI_ROOT_OBJECT would lead to a nice cleanup.
Note when you send out a new version of this patch + patch 6/7,
please base it on top of my current review-hans branch since
a bunch of other ideapad-laptop changes have landed there.
Regards,
Hans
On Tue, Nov 15, 2022 at 09:33:49PM +0100, Hans de Goede wrote:
> Hi Dmitry,
>
> On 10/29/22 14:03, Eray Or?unus wrote:
> > HUTRR72 added 3 new usage codes for keys that are supposed to enable,
> > disable and toggle camera access. These are useful, considering many
> > laptops today have key(s) for toggling access to camera.
> >
> > This patch adds new key definitions for KEY_CAMERA_ACCESS_ENABLE,
> > KEY_CAMERA_ACCESS_DISABLE and KEY_CAMERA_ACCESS_TOGGLE. Additionally
> > hid-debug is adjusted to recognize this new usage codes as well.
> >
> > Signed-off-by: Eray Or?unus <[email protected]>
> > Acked-by: Dmitry Torokhov <[email protected]>
>
> I have rejected the drivers/platform/x86 patch which depends
> on this, because it changes current behavior, potentially
> breaking userspace.
>
> Since this means I won't be taking any patches depending on
> this I believe it is best if this is merged through the input tree.
>
> Note this also has a:
>
> Acked-by: Jiri Kosina <[email protected]>
>
> tag given in this email thread.
OK, I picked it up.
Thanks.
--
Dmitry
Hi Eray,
On Sat, Oct 29, 2022 at 5:04 AM Eray Orçunus <[email protected]> wrote:
>
> HUTRR72 added 3 new usage codes for keys that are supposed to enable,
> disable and toggle camera access. These are useful, considering many
> laptops today have key(s) for toggling access to camera.
>
> This patch adds new key definitions for KEY_CAMERA_ACCESS_ENABLE,
> KEY_CAMERA_ACCESS_DISABLE and KEY_CAMERA_ACCESS_TOGGLE. Additionally
> hid-debug is adjusted to recognize this new usage codes as well.
>
> Signed-off-by: Eray Orçunus <[email protected]>
> Acked-by: Dmitry Torokhov <[email protected]>
> ---
> drivers/hid/hid-debug.c | 3 +++
> drivers/hid/hid-input.c | 3 +++
> include/uapi/linux/input-event-codes.h | 3 +++
> 3 files changed, 9 insertions(+)
>
> diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
> index f48d3534e020..991f880fdbd4 100644
> --- a/drivers/hid/hid-debug.c
> +++ b/drivers/hid/hid-debug.c
> @@ -936,6 +936,9 @@ static const char *keys[KEY_MAX + 1] = {
> [KEY_ASSISTANT] = "Assistant",
> [KEY_KBD_LAYOUT_NEXT] = "KbdLayoutNext",
> [KEY_EMOJI_PICKER] = "EmojiPicker",
> + [KEY_CAMERA_ACCESS_ENABLE] = "CameraAccessEnable",
> + [KEY_CAMERA_ACCESS_DISABLE] = "CameraAccessDisable",
> + [KEY_CAMERA_ACCESS_TOGGLE] = "CameraAccessToggle",
> [KEY_DICTATE] = "Dictate",
> [KEY_BRIGHTNESS_MIN] = "BrightnessMin",
> [KEY_BRIGHTNESS_MAX] = "BrightnessMax",
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index f197aed6444a..f8e6513e77b8 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -995,6 +995,9 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> case 0x0cd: map_key_clear(KEY_PLAYPAUSE); break;
> case 0x0cf: map_key_clear(KEY_VOICECOMMAND); break;
>
> + case 0x0d5: map_key_clear(KEY_CAMERA_ACCESS_ENABLE); break;
> + case 0x0d6: map_key_clear(KEY_CAMERA_ACCESS_DISABLE); break;
> + case 0x0d7: map_key_clear(KEY_CAMERA_ACCESS_TOGGLE); break;
I was looking at the HID Usages table spec and it came to my attention
that HUTRR72 actually established 0x76, 0x77 and 0x78 as the usages for
then new camera access controls:
https://www.usb.org/sites/default/files/hutrr72_-_usages_to_control_camera_access_0.pdf
Where did 0xd5, 0xd6 and 0xd7 came from?
It looks like we need to update the hid-input mappings as the are
clashing with game recording controls from HUTRR64:
https://www.usb.org/sites/default/files/hutrr64b_-_game_recording_controllers_0.pdf
Thanks.
--
Dmitry