Re: [PATCH] HID: hid-steam: Fix Lizard Mode disabling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>
> ---
>  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




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux