| Message ID | 20241225155507.30288-1-eugenyshcheglov@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: hid-steam: Fix Lizard Mode disabling | expand |
This is, in theory, an opinionated change about how desktop, gamepad, and lizard modes should interact. Whether or not that opinion is broadly applicable is somewhat questionable. In practice, it doesn't actually do all it promises to do. On 12/25/24 7:55 AM, Eugeny Shcheglov wrote: > Disable Lizard Mode by setting the lizard_mode option. > Set lizard_mode to 0 to disable switching between Desktop and Gamepad > using the Options button, and use Gamepad input. Switching between gamepad and desktop modes is already blocked when lizard_mode is disabled. See line 1053 in steam_mode_switch_cb. The work is scheduled sure, but it doesn't do anything when it triggers. > > Signed-off-by: Eugeny Shcheglov <eugenyshcheglov@gmail.com> > --- > drivers/hid/hid-steam.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c > index 6439913372a8..c64f716c9c14 100644 > --- a/drivers/hid/hid-steam.c > +++ b/drivers/hid/hid-steam.c > @@ -10,7 +10,7 @@ > * This controller has a builtin emulation of mouse and keyboard: the right pad > * can be used as a mouse, the shoulder buttons are mouse buttons, A and B > * buttons are ENTER and ESCAPE, and so on. This is implemented as additional > - * HID interfaces. > + * HID interfaces. Joystick input is blocked when Lizard Mode is active. This is conflating desktop and lizard modes, which have not been treated the same before. I don't think this is desirable. Lizard mode is a hardware mode, wherein mouse and gamepad input is sent, but desktop mode is a software mode where lizard mode is enabled and gamepad input is ignored. Making them the same is not desirable. That said, I do not see you actually implementing this later in the patch. > * > * This is known as the "lizard mode", because apparently lizards like to use > * the computer from the coach, without a proper mouse and keyboard. > @@ -555,9 +555,6 @@ static int steam_play_effect(struct input_dev *dev, void *data, > > static void steam_set_lizard_mode(struct steam_device *steam, bool enable) > { > - if (steam->gamepad_mode) > - enable = false; > - > if (enable) { > mutex_lock(&steam->report_mutex); > /* enable esc, enter, cursors */ > @@ -566,6 +563,7 @@ static void steam_set_lizard_mode(struct steam_device *steam, bool enable) > steam_send_report_byte(steam, ID_LOAD_DEFAULT_SETTINGS); > mutex_unlock(&steam->report_mutex); > } else { > + steam->gamepad_mode = true; > mutex_lock(&steam->report_mutex); > /* disable esc, enter, cursor */ > steam_send_report_byte(steam, ID_CLEAR_DIGITAL_MAPPINGS); The purpose of this appears to be just to make the lizard mode setting override the gamepad mode setting instead of vice versa, which I suppose is fine, though I'd need to double check how this interacts with the hidraw being opened. However, you aren't canceling the work in the process, which you should probably do if you want to make this change as such. > @@ -1590,12 +1588,14 @@ static void steam_do_deck_input_event(struct steam_device *steam, > b13 = data[13]; > b14 = data[14]; > > - if (!(b9 & BIT(6)) && steam->did_mode_switch) { > - steam->did_mode_switch = false; > - cancel_delayed_work_sync(&steam->mode_switch); > - } else if (!steam->client_opened && (b9 & BIT(6)) && !steam->did_mode_switch) { > - steam->did_mode_switch = true; > - schedule_delayed_work(&steam->mode_switch, 45 * HZ / 100); > + if (lizard_mode) { > + if (!(b9 & BIT(6)) && steam->did_mode_switch) { > + steam->did_mode_switch = false; > + cancel_delayed_work_sync(&steam->mode_switch); > + } else if (!steam->client_opened && (b9 & BIT(6)) && !steam->did_mode_switch) { > + steam->did_mode_switch = true; > + schedule_delayed_work(&steam->mode_switch, 45 * HZ / 100); > + } This is in theory superfluous per my comment on steam_mode_switch_cb. It also introduces a theoretical bug whereby it won't cancel the work if lizard mode is disabled while Options is held, as per the previous comment. > } > > if (!steam->gamepad_mode) All in all, this patch doesn't actually "fix" anything. It makes an opinionated change about lizard mode toggling, which is probably ok but not actually a "fix" per se, and does some peripheral stuff handling the scheduled work around it that is a minor but not really functional change nor fix. Further, the changes it makes are also incomplete and introduce a minor bug. Ironically, prior to this change it wouldn't even really be considered a bug, but the minor changes introduce semantics about how the work scheduling should be handled that aren't actually respected in the patch itself. Vicki
On 12/26/24 5:54 AM, Eugeny Shcheglov wrote: > Hi, Vicki, > > First, I should explain the motivation behind this change. I'm a developer of a gaming platform based on SteamDeck. My app runs in kiosk mode, and the system boots directly into the app, so I need to be able to read Gamepad values by default. > > Let's ensure that we are on the same page and that I understand the driver behavior correctly. > > There is one hardware mode: "Gamepad" mode (probably we can call it something like a "Full" mode), where all inputs from buttons, pads, and switches are processed. "Lizard Mode" is a possibility to filter input, right? So, if Lizard Mode is enabled, it gives us a > possibility to change modes between Gamepad (full input) and Desktop (mouse/keyboard only) by pressing the "Options" button OR automatically when the Steam client is opened. This is not at all accurate. Lizard mode is handled by firmware in the controller board itself and when enabled emulates a keyboard and mouse in addition to the gamepad controls. If lizard mode is disabled, then that emulation is disabled. Game/desktop modes are handled exclusively in the driver. They do different filtering of inputs based on if the evdev node is open or not. I forget the exact specifics and would need to review the code. If you want fine control over how all of this works, you can open the hidraw manually and parse it. The format is well-documented in the driver, and opening the hidraw disables all of this stuff (though you will need to send the heartbeat manually to keep lizard mode off--it automatically turns back on if a watchdog turns off). > >> Switching between gamepad and desktop modes is already blocked when lizard_mode is disabled. See line 1053 in steam_mode_switch_cb. > > No, it doesn't. Mode switching happens on line 1052, right before the "if (!lizard_mode) return" condition in the steam_mode_switch_cb. Oh, yes, come to think of it that should probably be on the other side of that early return. > > Logically, the "lizard_mode=0" option should disable the possibility to filter the input. However, when I tried setting "lizard_mode=0" on my SteamDeck, nothing changed except that the mouse was gone, along with the haptic pulse during mode switching. I'm still able to switch the joystick inputs on and off. Yup, that's because lizard mode is literally just the keyboard/mouse emulation. Since without that desktop mode is useless, it disables the mode changing. Or it should. It looks like that might not be handled entirely correctly, as you just said. > >> All in all, this patch doesn't actually "fix" anything. > > Well, in that case, what should "lizard_mode=0" do? As I mentioned before, on my SteamDeck, it only disables the haptic pulse and "desktop mode" functionality like mouse input. Vicki
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c index 6439913372a8..c64f716c9c14 100644 --- a/drivers/hid/hid-steam.c +++ b/drivers/hid/hid-steam.c @@ -10,7 +10,7 @@ * This controller has a builtin emulation of mouse and keyboard: the right pad * can be used as a mouse, the shoulder buttons are mouse buttons, A and B * buttons are ENTER and ESCAPE, and so on. This is implemented as additional - * HID interfaces. + * HID interfaces. Joystick input is blocked when Lizard Mode is active. * * This is known as the "lizard mode", because apparently lizards like to use * the computer from the coach, without a proper mouse and keyboard. @@ -555,9 +555,6 @@ static int steam_play_effect(struct input_dev *dev, void *data, static void steam_set_lizard_mode(struct steam_device *steam, bool enable) { - if (steam->gamepad_mode) - enable = false; - if (enable) { mutex_lock(&steam->report_mutex); /* enable esc, enter, cursors */ @@ -566,6 +563,7 @@ static void steam_set_lizard_mode(struct steam_device *steam, bool enable) steam_send_report_byte(steam, ID_LOAD_DEFAULT_SETTINGS); mutex_unlock(&steam->report_mutex); } else { + steam->gamepad_mode = true; mutex_lock(&steam->report_mutex); /* disable esc, enter, cursor */ steam_send_report_byte(steam, ID_CLEAR_DIGITAL_MAPPINGS); @@ -1590,12 +1588,14 @@ static void steam_do_deck_input_event(struct steam_device *steam, b13 = data[13]; b14 = data[14]; - if (!(b9 & BIT(6)) && steam->did_mode_switch) { - steam->did_mode_switch = false; - cancel_delayed_work_sync(&steam->mode_switch); - } else if (!steam->client_opened && (b9 & BIT(6)) && !steam->did_mode_switch) { - steam->did_mode_switch = true; - schedule_delayed_work(&steam->mode_switch, 45 * HZ / 100); + if (lizard_mode) { + if (!(b9 & BIT(6)) && steam->did_mode_switch) { + steam->did_mode_switch = false; + cancel_delayed_work_sync(&steam->mode_switch); + } else if (!steam->client_opened && (b9 & BIT(6)) && !steam->did_mode_switch) { + steam->did_mode_switch = true; + schedule_delayed_work(&steam->mode_switch, 45 * HZ / 100); + } } if (!steam->gamepad_mode)
Disable Lizard Mode by setting the lizard_mode option. Set lizard_mode to 0 to disable switching between Desktop and Gamepad using the Options button, and use Gamepad input. Signed-off-by: Eugeny Shcheglov <eugenyshcheglov@gmail.com> --- drivers/hid/hid-steam.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)