| Message ID | 20230111012336.2915827-2-vi@endrift.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] HID: hid-steam: Add Steam Deck support | expand |
On Tue, 2023-01-10 at 17:23 -0800, Vicki Pfau wrote: > The Steam Deck includes a new report that allows for emulating XInput-style > rumble motors with the Deck's actuators. This adds support for passing these > values directly to the Deck. > > Signed-off-by: Vicki Pfau <vi@endrift.com> > --- > drivers/hid/Kconfig | 8 ++++++ > drivers/hid/hid-steam.c | 55 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 63 insertions(+) > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > index e2a5d30c8895..e9de0a2d3cd3 100644 > --- a/drivers/hid/Kconfig > +++ b/drivers/hid/Kconfig > @@ -1025,6 +1025,14 @@ config HID_STEAM > without running the Steam Client. It supports both the wired and > the wireless adaptor. > > +config STEAM_FF > + bool "Steam Deck force feedback support" > + depends on HID_STEAM > + select INPUT_FF_MEMLESS > + help > + Say Y here if you want to enable force feedback support for the Steam > + Deck. > + > config HID_STEELSERIES > tristate "Steelseries SRW-S1 steering wheel support" > help > diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c > index efd192d6c51a..788b5baaf145 100644 > --- a/drivers/hid/hid-steam.c > +++ b/drivers/hid/hid-steam.c > @@ -91,6 +91,7 @@ static LIST_HEAD(steam_devices); > #define STEAM_CMD_FORCEFEEDBAK 0x8f > #define STEAM_CMD_REQUEST_COMM_STATUS 0xb4 > #define STEAM_CMD_GET_SERIAL 0xae > +#define STEAM_CMD_HAPTIC_RUMBLE 0xeb > > /* Some useful register ids */ > #define STEAM_REG_LPAD_MODE 0x07 > @@ -134,6 +135,9 @@ struct steam_device { > u8 battery_charge; > u16 voltage; > struct delayed_work heartbeat; > + struct work_struct rumble_work; > + u16 rumble_left; > + u16 rumble_right; > }; > > static int steam_recv_report(struct steam_device *steam, > @@ -290,6 +294,45 @@ static inline int steam_request_conn_status(struct steam_device *steam) > return steam_send_report_byte(steam, STEAM_CMD_REQUEST_COMM_STATUS); > } > > +static inline int steam_haptic_rumble(struct steam_device *steam, > + u16 intensity, u16 left_speed, u16 right_speed, > + u8 left_gain, u8 right_gain) > +{ > + u8 report[11] = {STEAM_CMD_HAPTIC_RUMBLE, 9}; > + > + report[3] = intensity & 0xFF; > + report[4] = intensity >> 8; > + report[5] = left_speed & 0xFF; > + report[6] = left_speed >> 8; > + report[7] = right_speed & 0xFF; > + report[8] = right_speed >> 8; > + report[9] = left_gain; > + report[10] = right_gain; > + > + return steam_send_report(steam, report, sizeof(report)); > +} > + > +static void steam_haptic_rumble_cb(struct work_struct *work) > +{ > + struct steam_device *steam = container_of(work, struct steam_device, > + rumble_work); > + steam_haptic_rumble(steam, 0, steam->rumble_left, > + steam->rumble_right, 2, 0); > +} > + > +#ifdef CONFIG_STEAM_FF > +static int steam_play_effect(struct input_dev *dev, void *data, > + struct ff_effect *effect) > +{ > + struct steam_device *steam = input_get_drvdata(dev); > + > + steam->rumble_left = effect->u.rumble.strong_magnitude; > + steam->rumble_right = effect->u.rumble.weak_magnitude; > + > + return schedule_work(&steam->rumble_work); > +} > +#endif > + > static void steam_set_lizard_mode(struct steam_device *steam, bool enable) > { > if (enable) { > @@ -541,6 +584,15 @@ static int steam_input_register(struct steam_device *steam) > input_abs_set_res(input, ABS_HAT0X, STEAM_PAD_RESOLUTION); > input_abs_set_res(input, ABS_HAT0Y, STEAM_PAD_RESOLUTION); > > +#ifdef CONFIG_STEAM_FF > + if (steam->quirks & STEAM_QUIRK_DECK) { > + input_set_capability(input, EV_FF, FF_RUMBLE); > + ret = input_ff_create_memless(input, NULL, steam_play_effect); > + if (ret) > + goto init_ff_fail; > + } > +#endif > + > ret = input_register_device(input); > if (ret) > goto input_register_fail; > @@ -549,6 +601,7 @@ static int steam_input_register(struct steam_device *steam) > return 0; > > input_register_fail: > +init_ff_fail: JFYI, this actually causes a compilation warning with CONFIG_STEAM_FF disabled: drivers/hid/hid-steam.c: In function ‘steam_input_register’: drivers/hid/hid-steam.c:604:1: warning: label ‘init_ff_fail’ defined but not used [-Wunused-label] 604 | init_ff_fail: | ^~~~~~~~~~~~ TBH I think we should be fine just reusing the input_register_fail: jump label for this instead of adding another label. FWIW as well if you want: you could just drop the Kconfig option for this entirely, which bentiss may or may not want. It would at least leave a little less chance for compilation warnings like this, since the more Kconfig options you have for a module the higher the chance you'll leave a warning by mistake in some random kernel config. If you end up deciding to leave the Kconfig in I'd at least update the commit message to mention explicitly you added it so people notice it even if they don't look at the diff (e.g. maintainers just merging reviewed patches). I have no hard opinion either way as long as we fix the compilation warning :). With the issues mentioned here addressed, this patch is: Reviewed-by: Lyude Paul <lyude@redhat.com> > input_free_device(input); > return ret; > } > @@ -842,6 +895,7 @@ static int steam_probe(struct hid_device *hdev, > INIT_WORK(&steam->work_connect, steam_work_connect_cb); > INIT_LIST_HEAD(&steam->list); > INIT_DEFERRABLE_WORK(&steam->heartbeat, steam_lizard_mode_heartbeat); > + INIT_WORK(&steam->rumble_work, steam_haptic_rumble_cb); > > steam->client_hdev = steam_create_client_hid(hdev); > if (IS_ERR(steam->client_hdev)) { > @@ -898,6 +952,7 @@ static int steam_probe(struct hid_device *hdev, > client_hdev_fail: > cancel_work_sync(&steam->work_connect); > cancel_delayed_work_sync(&steam->heartbeat); > + cancel_work_sync(&steam->rumble_work); > steam_alloc_fail: > hid_err(hdev, "%s: failed with error %d\n", > __func__, ret);
On Wed, Jan 18, 2023 at 12:16 AM Lyude Paul <lyude@redhat.com> wrote: > > On Tue, 2023-01-10 at 17:23 -0800, Vicki Pfau wrote: > > The Steam Deck includes a new report that allows for emulating XInput-style > > rumble motors with the Deck's actuators. This adds support for passing these > > values directly to the Deck. > > > > Signed-off-by: Vicki Pfau <vi@endrift.com> > > --- > > drivers/hid/Kconfig | 8 ++++++ > > drivers/hid/hid-steam.c | 55 +++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 63 insertions(+) > > > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > > index e2a5d30c8895..e9de0a2d3cd3 100644 > > --- a/drivers/hid/Kconfig > > +++ b/drivers/hid/Kconfig > > @@ -1025,6 +1025,14 @@ config HID_STEAM > > without running the Steam Client. It supports both the wired and > > the wireless adaptor. > > > > +config STEAM_FF > > + bool "Steam Deck force feedback support" > > + depends on HID_STEAM > > + select INPUT_FF_MEMLESS > > + help > > + Say Y here if you want to enable force feedback support for the Steam > > + Deck. > > + > > config HID_STEELSERIES > > tristate "Steelseries SRW-S1 steering wheel support" > > help > > diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c > > index efd192d6c51a..788b5baaf145 100644 > > --- a/drivers/hid/hid-steam.c > > +++ b/drivers/hid/hid-steam.c > > @@ -91,6 +91,7 @@ static LIST_HEAD(steam_devices); > > #define STEAM_CMD_FORCEFEEDBAK 0x8f > > #define STEAM_CMD_REQUEST_COMM_STATUS 0xb4 > > #define STEAM_CMD_GET_SERIAL 0xae > > +#define STEAM_CMD_HAPTIC_RUMBLE 0xeb > > > > /* Some useful register ids */ > > #define STEAM_REG_LPAD_MODE 0x07 > > @@ -134,6 +135,9 @@ struct steam_device { > > u8 battery_charge; > > u16 voltage; > > struct delayed_work heartbeat; > > + struct work_struct rumble_work; > > + u16 rumble_left; > > + u16 rumble_right; > > }; > > > > static int steam_recv_report(struct steam_device *steam, > > @@ -290,6 +294,45 @@ static inline int steam_request_conn_status(struct steam_device *steam) > > return steam_send_report_byte(steam, STEAM_CMD_REQUEST_COMM_STATUS); > > } > > > > +static inline int steam_haptic_rumble(struct steam_device *steam, > > + u16 intensity, u16 left_speed, u16 right_speed, > > + u8 left_gain, u8 right_gain) > > +{ > > + u8 report[11] = {STEAM_CMD_HAPTIC_RUMBLE, 9}; > > + > > + report[3] = intensity & 0xFF; > > + report[4] = intensity >> 8; > > + report[5] = left_speed & 0xFF; > > + report[6] = left_speed >> 8; > > + report[7] = right_speed & 0xFF; > > + report[8] = right_speed >> 8; > > + report[9] = left_gain; > > + report[10] = right_gain; > > + > > + return steam_send_report(steam, report, sizeof(report)); > > +} > > + > > +static void steam_haptic_rumble_cb(struct work_struct *work) > > +{ > > + struct steam_device *steam = container_of(work, struct steam_device, > > + rumble_work); > > + steam_haptic_rumble(steam, 0, steam->rumble_left, > > + steam->rumble_right, 2, 0); > > +} > > + > > +#ifdef CONFIG_STEAM_FF > > +static int steam_play_effect(struct input_dev *dev, void *data, > > + struct ff_effect *effect) > > +{ > > + struct steam_device *steam = input_get_drvdata(dev); > > + > > + steam->rumble_left = effect->u.rumble.strong_magnitude; > > + steam->rumble_right = effect->u.rumble.weak_magnitude; > > + > > + return schedule_work(&steam->rumble_work); > > +} > > +#endif > > + > > static void steam_set_lizard_mode(struct steam_device *steam, bool enable) > > { > > if (enable) { > > @@ -541,6 +584,15 @@ static int steam_input_register(struct steam_device *steam) > > input_abs_set_res(input, ABS_HAT0X, STEAM_PAD_RESOLUTION); > > input_abs_set_res(input, ABS_HAT0Y, STEAM_PAD_RESOLUTION); > > > > +#ifdef CONFIG_STEAM_FF > > + if (steam->quirks & STEAM_QUIRK_DECK) { > > + input_set_capability(input, EV_FF, FF_RUMBLE); > > + ret = input_ff_create_memless(input, NULL, steam_play_effect); > > + if (ret) > > + goto init_ff_fail; > > + } > > +#endif > > + > > ret = input_register_device(input); > > if (ret) > > goto input_register_fail; > > @@ -549,6 +601,7 @@ static int steam_input_register(struct steam_device *steam) > > return 0; > > > > input_register_fail: > > +init_ff_fail: > > JFYI, this actually causes a compilation warning with CONFIG_STEAM_FF > disabled: > > drivers/hid/hid-steam.c: In function ‘steam_input_register’: > drivers/hid/hid-steam.c:604:1: warning: label ‘init_ff_fail’ defined but not > used [-Wunused-label] > 604 | init_ff_fail: > | ^~~~~~~~~~~~ > > TBH I think we should be fine just reusing the input_register_fail: jump label > for this instead of adding another label. > > FWIW as well if you want: you could just drop the Kconfig option for this > entirely, which bentiss may or may not want. It would at least leave a little > less chance for compilation warnings like this, since the more Kconfig options > you have for a module the higher the chance you'll leave a warning by mistake > in some random kernel config. I agree with Lyude here. However the whole HID tree is crippled with those "if input_ff" and that would mean a bigger policy change. So I would suggest keeping the Kconfig option for now, and if you want, Vicki (or anybody else) maybe it's time to get rid of those input_ff Kconfigs and make them slightly more evident for users. Whether the first driver to use it selects input_ff or they all depend on input_ff without the ability to disable it is entirely the responsibility of the submitter :) Cheers, Benjamin > > If you end up deciding to leave the Kconfig in I'd at least update the commit > message to mention explicitly you added it so people notice it even if they > don't look at the diff (e.g. maintainers just merging reviewed patches). > > I have no hard opinion either way as long as we fix the compilation warning > :). With the issues mentioned here addressed, this patch is: > > Reviewed-by: Lyude Paul <lyude@redhat.com> > > > input_free_device(input); > > return ret; > > } > > @@ -842,6 +895,7 @@ static int steam_probe(struct hid_device *hdev, > > INIT_WORK(&steam->work_connect, steam_work_connect_cb); > > INIT_LIST_HEAD(&steam->list); > > INIT_DEFERRABLE_WORK(&steam->heartbeat, steam_lizard_mode_heartbeat); > > + INIT_WORK(&steam->rumble_work, steam_haptic_rumble_cb); > > > > steam->client_hdev = steam_create_client_hid(hdev); > > if (IS_ERR(steam->client_hdev)) { > > @@ -898,6 +952,7 @@ static int steam_probe(struct hid_device *hdev, > > client_hdev_fail: > > cancel_work_sync(&steam->work_connect); > > cancel_delayed_work_sync(&steam->heartbeat); > > + cancel_work_sync(&steam->rumble_work); > > steam_alloc_fail: > > hid_err(hdev, "%s: failed with error %d\n", > > __func__, ret); > > -- > Cheers, > Lyude Paul (she/her) > Software Engineer at Red Hat >
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index e2a5d30c8895..e9de0a2d3cd3 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -1025,6 +1025,14 @@ config HID_STEAM without running the Steam Client. It supports both the wired and the wireless adaptor. +config STEAM_FF + bool "Steam Deck force feedback support" + depends on HID_STEAM + select INPUT_FF_MEMLESS + help + Say Y here if you want to enable force feedback support for the Steam + Deck. + config HID_STEELSERIES tristate "Steelseries SRW-S1 steering wheel support" help diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c index efd192d6c51a..788b5baaf145 100644 --- a/drivers/hid/hid-steam.c +++ b/drivers/hid/hid-steam.c @@ -91,6 +91,7 @@ static LIST_HEAD(steam_devices); #define STEAM_CMD_FORCEFEEDBAK 0x8f #define STEAM_CMD_REQUEST_COMM_STATUS 0xb4 #define STEAM_CMD_GET_SERIAL 0xae +#define STEAM_CMD_HAPTIC_RUMBLE 0xeb /* Some useful register ids */ #define STEAM_REG_LPAD_MODE 0x07 @@ -134,6 +135,9 @@ struct steam_device { u8 battery_charge; u16 voltage; struct delayed_work heartbeat; + struct work_struct rumble_work; + u16 rumble_left; + u16 rumble_right; }; static int steam_recv_report(struct steam_device *steam, @@ -290,6 +294,45 @@ static inline int steam_request_conn_status(struct steam_device *steam) return steam_send_report_byte(steam, STEAM_CMD_REQUEST_COMM_STATUS); } +static inline int steam_haptic_rumble(struct steam_device *steam, + u16 intensity, u16 left_speed, u16 right_speed, + u8 left_gain, u8 right_gain) +{ + u8 report[11] = {STEAM_CMD_HAPTIC_RUMBLE, 9}; + + report[3] = intensity & 0xFF; + report[4] = intensity >> 8; + report[5] = left_speed & 0xFF; + report[6] = left_speed >> 8; + report[7] = right_speed & 0xFF; + report[8] = right_speed >> 8; + report[9] = left_gain; + report[10] = right_gain; + + return steam_send_report(steam, report, sizeof(report)); +} + +static void steam_haptic_rumble_cb(struct work_struct *work) +{ + struct steam_device *steam = container_of(work, struct steam_device, + rumble_work); + steam_haptic_rumble(steam, 0, steam->rumble_left, + steam->rumble_right, 2, 0); +} + +#ifdef CONFIG_STEAM_FF +static int steam_play_effect(struct input_dev *dev, void *data, + struct ff_effect *effect) +{ + struct steam_device *steam = input_get_drvdata(dev); + + steam->rumble_left = effect->u.rumble.strong_magnitude; + steam->rumble_right = effect->u.rumble.weak_magnitude; + + return schedule_work(&steam->rumble_work); +} +#endif + static void steam_set_lizard_mode(struct steam_device *steam, bool enable) { if (enable) { @@ -541,6 +584,15 @@ static int steam_input_register(struct steam_device *steam) input_abs_set_res(input, ABS_HAT0X, STEAM_PAD_RESOLUTION); input_abs_set_res(input, ABS_HAT0Y, STEAM_PAD_RESOLUTION); +#ifdef CONFIG_STEAM_FF + if (steam->quirks & STEAM_QUIRK_DECK) { + input_set_capability(input, EV_FF, FF_RUMBLE); + ret = input_ff_create_memless(input, NULL, steam_play_effect); + if (ret) + goto init_ff_fail; + } +#endif + ret = input_register_device(input); if (ret) goto input_register_fail; @@ -549,6 +601,7 @@ static int steam_input_register(struct steam_device *steam) return 0; input_register_fail: +init_ff_fail: input_free_device(input); return ret; } @@ -842,6 +895,7 @@ static int steam_probe(struct hid_device *hdev, INIT_WORK(&steam->work_connect, steam_work_connect_cb); INIT_LIST_HEAD(&steam->list); INIT_DEFERRABLE_WORK(&steam->heartbeat, steam_lizard_mode_heartbeat); + INIT_WORK(&steam->rumble_work, steam_haptic_rumble_cb); steam->client_hdev = steam_create_client_hid(hdev); if (IS_ERR(steam->client_hdev)) { @@ -898,6 +952,7 @@ static int steam_probe(struct hid_device *hdev, client_hdev_fail: cancel_work_sync(&steam->work_connect); cancel_delayed_work_sync(&steam->heartbeat); + cancel_work_sync(&steam->rumble_work); steam_alloc_fail: hid_err(hdev, "%s: failed with error %d\n", __func__, ret);
The Steam Deck includes a new report that allows for emulating XInput-style rumble motors with the Deck's actuators. This adds support for passing these values directly to the Deck. Signed-off-by: Vicki Pfau <vi@endrift.com> --- drivers/hid/Kconfig | 8 ++++++ drivers/hid/hid-steam.c | 55 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+)