From: Tomasz Pakuła tomasz.pakula.oficjalny@gmail.com
[ Upstream commit f345a4798dab800159b09d088e7bdae0f16076c3 ]
The already fixed bug in SDL only affected conditional effects. This should fix FFB in Forza Horizion 4/5 on Moza Devices as Forza Horizon flips the constant force direction instead of using negative magnitude values.
Changing the direction in the effect directly in pidff_upload_effect() would affect it's value in further operations like comparing to the old effect and/or just reading the effect values in the user application.
This, in turn, would lead to constant PID_SET_EFFECT spam as the effect direction would constantly not match the value that's set by the application.
This way, it's still transparent to any software/API.
Only affects conditional effects now so it's better for it to explicitly state that in the name. If any HW ever needs fixed direction for other effects, we'll add more quirks.
Signed-off-by: Tomasz Pakuła tomasz.pakula.oficjalny@gmail.com Reviewed-by: Oleg Makarenko oleg@makarenk.ooo Signed-off-by: Jiri Kosina jkosina@suse.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
- What it fixes - The earlier quirk forced a fixed direction (0x4000) for all effect types on specific wheelbases to work around mis-set PID directions in some user stacks (dinput → wine → SDL). That broad forcing breaks games that legitimately use the direction field for non-conditional effects (e.g., Forza Horizon 4/5 uses direction flips for constant force instead of negative magnitudes). This patch narrows the quirk to conditional effects only (spring, damper, inertia, friction), matching where the SDL-side bug actually applied.
- How it changes behavior - Adds a helper to detect conditional effects and a wrapper to set the direction only when appropriate: - New logic: pidff_is_effect_conditional() and pidff_set_effect_direction() in drivers/hid/usbhid/hid-pidff.c (replaces unconditional forcing in Set Effect). - Set Effect now calls the helper instead of unconditionally forcing direction for all effects. - Renames the quirk to reflect scope: HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION → HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION, still using the same bit (BIT(3)). - Updates device ID table entries to use the new quirk name for Moza devices in drivers/hid/hid-universal-pidff.c.
- Why it’s appropriate for stable - User-visible bug/regression: Fixes incorrect or missing FFB in Forza Horizon 4/5 on Moza devices when the kernel overrode constant-force directions. The message explicitly states this resolves that case. - Small and contained: Limited to HID PID force-feedback code and quirk tables; no architectural changes. - Minimal risk: Only affects devices already marked with the quirk bit. Behavior is narrowed (less intrusive) by applying the fixed direction only to conditional effects; other effect types now honor the application-provided direction as intended. - No side effects on unrelated subsystems: Touches only HID FFB code paths. - Clear lineage: It logically corrects the earlier “FIX_WHEEL_DIRECTION” quirk (drivers/hid/usbhid/hid-pidff.c:397) that forced direction for all effect types.
- Specific code references - Current unconditional forcing (to be replaced by helper call): drivers/hid/usbhid/hid-pidff.c:397 - Quirk bit definition that’s renamed but remains BIT(3): drivers/hid/usbhid/hid-pidff.h:20 - Fixed direction constant (still used, but now applied conditionally): drivers/hid/usbhid/hid-pidff.c:151 - Device entries using the quirk (updated to new name): drivers/hid/hid-universal-pidff.c:6
- Additional considerations - The commit avoids mutating effect->direction in pidff_upload_effect(), preventing mismatches with the application’s state and avoiding needless PID_SET_EFFECT churn. It keeps behavior transparent to user space. - If any future hardware requires fixed direction for non-conditional effects, the commit message notes that more targeted quirks will be added, which further limits regression risk.
Given it’s a targeted regression fix for a real-world breakage, small in scope, and reduces the quirk’s blast radius, it’s a strong candidate for stable backport (especially on branches that already carry the earlier FIX_WHEEL_DIRECTION quirk).
drivers/hid/hid-universal-pidff.c | 20 ++++++++++---------- drivers/hid/usbhid/hid-pidff.c | 28 +++++++++++++++++++++++----- drivers/hid/usbhid/hid-pidff.h | 2 +- 3 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/drivers/hid/hid-universal-pidff.c b/drivers/hid/hid-universal-pidff.c index 554a6559aeb73..70fce0f88e825 100644 --- a/drivers/hid/hid-universal-pidff.c +++ b/drivers/hid/hid-universal-pidff.c @@ -144,25 +144,25 @@ static int universal_pidff_input_configured(struct hid_device *hdev,
static const struct hid_device_id universal_pidff_devices[] = { { HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R3), - .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION }, + .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION }, { HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R3_2), - .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION }, + .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION }, { HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R5), - .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION }, + .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION }, { HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R5_2), - .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION }, + .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION }, { HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R9), - .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION }, + .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION }, { HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R9_2), - .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION }, + .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION }, { HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R12), - .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION }, + .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION }, { HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R12_2), - .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION }, + .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION }, { HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R16_R21), - .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION }, + .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION }, { HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R16_R21_2), - .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION }, + .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION }, { HID_USB_DEVICE(USB_VENDOR_ID_CAMMUS, USB_DEVICE_ID_CAMMUS_C5) }, { HID_USB_DEVICE(USB_VENDOR_ID_CAMMUS, USB_DEVICE_ID_CAMMUS_C12) }, { HID_USB_DEVICE(USB_VENDOR_ID_VRS, USB_DEVICE_ID_VRS_DFP), diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c index 614a20b620231..c6b4f61e535d5 100644 --- a/drivers/hid/usbhid/hid-pidff.c +++ b/drivers/hid/usbhid/hid-pidff.c @@ -205,6 +205,14 @@ struct pidff_device { u8 effect_count; };
+static int pidff_is_effect_conditional(struct ff_effect *effect) +{ + return effect->type == FF_SPRING || + effect->type == FF_DAMPER || + effect->type == FF_INERTIA || + effect->type == FF_FRICTION; +} + /* * Clamp value for a given field */ @@ -294,6 +302,20 @@ static void pidff_set_duration(struct pidff_usage *usage, u16 duration) pidff_set_time(usage, duration); }
+static void pidff_set_effect_direction(struct pidff_device *pidff, + struct ff_effect *effect) +{ + u16 direction = effect->direction; + + /* Use fixed direction if needed */ + if (pidff->quirks & HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION && + pidff_is_effect_conditional(effect)) + direction = PIDFF_FIXED_WHEEL_DIRECTION; + + pidff->effect_direction->value[0] = + pidff_rescale(direction, U16_MAX, pidff->effect_direction); +} + /* * Send envelope report to the device */ @@ -395,11 +417,7 @@ static void pidff_set_effect_report(struct pidff_device *pidff, pidff->set_effect[PID_GAIN].field->logical_maximum; pidff->set_effect[PID_DIRECTION_ENABLE].value[0] = 1;
- /* Use fixed direction if needed */ - pidff->effect_direction->value[0] = pidff_rescale( - pidff->quirks & HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION ? - PIDFF_FIXED_WHEEL_DIRECTION : effect->direction, - U16_MAX, pidff->effect_direction); + pidff_set_effect_direction(pidff, effect);
/* Omit setting delay field if it's missing */ if (!(pidff->quirks & HID_PIDFF_QUIRK_MISSING_DELAY)) diff --git a/drivers/hid/usbhid/hid-pidff.h b/drivers/hid/usbhid/hid-pidff.h index a53a8b436baa6..f321f675e1318 100644 --- a/drivers/hid/usbhid/hid-pidff.h +++ b/drivers/hid/usbhid/hid-pidff.h @@ -16,7 +16,7 @@ #define HID_PIDFF_QUIRK_PERMISSIVE_CONTROL BIT(2)
/* Use fixed 0x4000 direction during SET_EFFECT report upload */ -#define HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION BIT(3) +#define HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION BIT(3)
/* Force all periodic effects to be uploaded as SINE */ #define HID_PIDFF_QUIRK_PERIODIC_SINE_ONLY BIT(4)