FrameworkComputer / EmbeddedController

Embedded Controller firmware for the Framework Laptop
BSD 3-Clause "New" or "Revised" License
958 stars 64 forks source link

Add option to emit HID display toggle from F9 display key #49

Open Jules-Bertholet opened 3 weeks ago

Jules-Bertholet commented 3 weeks ago

By default, this key emits Win+P, which is not portable to non-QWERTY keyboard layouts and non-Windows OSes. This adds an option (off by default, set with 0x3E16 flag) to emit the HID display toggle key instead.

This flag should be exposed in the BIOS settings.

See discussion at https://community.frame.work/t/f9-display-switch-is-broken-on-non-qwerty-layouts/59773

JohnAZoidberg commented 2 weeks ago

Have you tried this, which operating systems does this work on?

Jules-Bertholet commented 2 weeks ago

I have not tested this patch by flashing the firmware to a Framework, no. However, I have tested the HID device/report descriptor on Linux by emulating it with uhid, and it works as expected (makes the display switch menu appear). I have not tested on Windows or any other OS.

leo60228 commented 2 weeks ago

I ported this to azalea except for the host event/configurability aspect, and it appears to work in KDE. I don't have a Windows install to test there. https://github.com/leo60228/EmbeddedController/commit/88b3f2928714bfd79e625cbe499fecff8a547f46

Infinidoge commented 2 weeks ago

Thank you :pray:

I've been frustrated with this ever since I got my framework years ago, and I've been meaning to figure out a patch for it myself, but just never got around to it.

Would it be possible to try this on my framework without modification (i.e., is it possible to set that setting flag), or would I need to modify the patch to do it by default?

JohnAZoidberg commented 2 weeks ago

I've been frustrated with this ever since I got my framework years ago, and I've been meaning to figure out a patch for it myself, but just never got around to it.

But the current combination does work on GNOME. WIN+P also brings up the display selection menu. @Jules-Bertholet is this the same one that this HID usage brings up?

image

Would it be possible to try this on my framework without modification (i.e., is it possible to set that setting flag), or would I need to modify the patch to do it by default?

Unfortunately no, because it's a changed HID report descriptor.

Infinidoge commented 2 weeks ago

I use a tiling window manager, and I have Super-p bound to start a key chord for power management. (So Super-p h hibernates, for example.)

As you can probably guess, having the display button emulate Super-p at the firmware level instead of giving me a keysym I can work with is very frustrating when it would be perfect to open arandr.

By "without modification" I meant without modifying the bios or patch, not sure how I missed that. Recompiling and reflashing the EC I can definitely do.

JohnAZoidberg commented 2 weeks ago

By "without modification" I meant without modifying the bios or patch, not sure how I missed that. Recompiling and reflashing the EC I can definitely do.

This patch implements changes for 11-13th gen intel systems.

Infinidoge commented 2 weeks ago

I'm not really sure what that has to do with what I said. (I do have an 11th gen Intel so should be fine for my purposes.) I want to use the patch, but it seems to say that it needs a bios option added to toggle it which isn't part of the EC, so unless I modify the patch to hard code it on, I don't know how I would turn it on after flashing the EC. (Unless fw-ectool could toggle it I suppose.)

Jules-Bertholet commented 2 weeks ago

But the current combination does work on GNOME. WIN+P also brings up the display selection menu.

Yes, GNOME copies Windows’s shortcut (I cannot speak to other DEs or OSes). However, I use Colemak, not QWERTY, so what the firmware thinks is P, my OS interprets as Y. Win+Y, of course, does not bring up the menu, in any OS. The HID display toggle, in contrast, works no matter what keyboard layout is set in the OS.

Jules-Bertholet commented 2 weeks ago

so unless I modify the patch to hard code it

Replace if (display_toggle_key_hid) { with if (1) { in keyboard_customization.c