From 5e320c68c031e251b27679e33d238b839e8116a0 Mon Sep 17 00:00:00 2001 From: Kai Krakow Date: Sat, 2 May 2020 03:13:09 +0200 Subject: [PATCH] [WIP] hid-xpadneo: only send rumble output reports when needed The user-space driver implementation xow only sends rumble reports to the device if the rumble motors are running or the desired magnitude is non-zero. To implement this here, we first allocate a private device structure for maintaining the rumble state. This will be passed in via the `input_ff_create_memless()` hook. Additionally, in the hook we need to protect concurrent access to the structure with a spinlock, tho I'm not sure if the hook may really be called in parallel. Maybe it's better to defer this into a dedicated worker queue as `hid-microsoft.c` does it. This may reduce pressure on the device firmware and stabilize the bluetooth connection. Maybe-related: https://github.com/atar-axis/xpadneo/issues/171 Maybe-related: https://github.com/atar-axis/xpadneo/issues/122 --- hid-xpadneo/src/hid-xpadneo.c | 105 ++++++++++++++++++++++------------ 1 file changed, 68 insertions(+), 37 deletions(-) diff --git a/hid-xpadneo/src/hid-xpadneo.c b/hid-xpadneo/src/hid-xpadneo.c index df855631..9c57cef6 100644 --- a/hid-xpadneo/src/hid-xpadneo.c +++ b/hid-xpadneo/src/hid-xpadneo.c @@ -191,6 +191,14 @@ struct xpadneo_devdata { s32 last_abs_rz; }; +struct xpadneo_ffdata { + /* mutual exclusion */ + spinlock_t lock; + + /* is the device rumbling? */ + u8 rumbling; +}; + void create_ff_pck (struct ff_report *pck, u8 id, u8 en_act, u8 mag_lt, u8 mag_rt, u8 mag_l, u8 mag_r, @@ -233,6 +241,7 @@ static int xpadneo_ff_play(struct input_dev *dev, void *data, */ struct ff_report ff_pck; + unsigned long flags; u32 weak, strong, direction, max, max_damped, max_unscaled; u8 mag_main_right, mag_main_left, mag_trigger_right, mag_trigger_left; u8 ff_active; @@ -253,6 +262,7 @@ static int xpadneo_ff_play(struct input_dev *dev, void *data, struct hid_device *hdev = input_get_drvdata(dev); + struct xpadneo_ffdata *ffdata = data; if (param_disable_ff == PARAM_DISABLE_FF_ALL) return 0; @@ -285,42 +295,57 @@ static int xpadneo_ff_play(struct input_dev *dev, void *data, if (param_disable_ff & PARAM_DISABLE_FF_MAIN) ff_active &= ~(FF_ENABLE_LEFT | FF_ENABLE_RIGHT); - /* get the proportions from a precalculated cosine table - * calculation goes like: - * cosine(a) * 1000 = {100, 96, 85, 69, 50, 31, 15, 4, 0, 4, 15, 31, 50, 69, 85, 96, 100} - * fractions_percent(a) = round(50 + (cosine * 50)) - */ - fraction_TL = 0; - fraction_TR = 0; - if (direction >= DIRECTION_LEFT && direction <= DIRECTION_RIGHT) { - index_left = (direction - DIRECTION_LEFT) >> 11; - index_right = proportions_idx_max - index_left; - fraction_TL = fractions_percent[index_left]; - fraction_TR = fractions_percent[index_right]; - } + /* only send rumble report while we are rumbling (so we can set gains + * to 0) or the controller should actually rumble (max has the maximum + * value of the main trigger motors and will also control the trigger + * gain, so it's sufficient to check that single value) */ + spin_lock_irqsave(&ffdata->lock, flags); + if (ffdata->rumbling || (ff_active && (max > 0))) { + + /* remember if the device is rumbling */ + ffdata->rumbling = ff_active && (max > 0); - /* the user can change the damping at runtime, hence check the range */ - trigger_rumble_damping_nonzero - = param_trigger_rumble_damping == 0 ? 1 : param_trigger_rumble_damping; - max_damped = max_unscaled / trigger_rumble_damping_nonzero; - mag_trigger_left = (u8)((max_damped * fraction_TL) / 0xFFFF); - mag_trigger_right = (u8)((max_damped * fraction_TR) / 0xFFFF); - - create_ff_pck( - &ff_pck, 0x03, - ff_active, - mag_trigger_left, mag_trigger_right, - mag_main_left, mag_main_right, - 0); - - hid_dbg_lvl(DBG_LVL_FEW, hdev, - "active: %#04x, max: %#04x, prop_left: %#04x, prop_right: %#04x, left trigger: %#04x, right: %#04x\n", - ff_active, - max, fraction_TL, fraction_TR, - ff_pck.ff.magnitude_left_trigger, - ff_pck.ff.magnitude_right_trigger); - - hid_hw_output_report(hdev, (u8 *)&ff_pck, sizeof(ff_pck)); + /* get the proportions from a precalculated cosine table + * calculation goes like: + * cosine(a) * 1000 = {100, 96, 85, 69, 50, 31, 15, 4, 0, 4, 15, 31, 50, 69, 85, 96, 100} + * fractions_percent(a) = round(50 + (cosine * 50)) + */ + fraction_TL = 0; + fraction_TR = 0; + if (direction >= DIRECTION_LEFT && direction <= DIRECTION_RIGHT) { + index_left = (direction - DIRECTION_LEFT) >> 11; + index_right = proportions_idx_max - index_left; + fraction_TL = fractions_percent[index_left]; + fraction_TR = fractions_percent[index_right]; + } + + /* the user can change the damping at runtime, hence check the range */ + trigger_rumble_damping_nonzero + = param_trigger_rumble_damping == 0 ? 1 : param_trigger_rumble_damping; + max_damped = max_unscaled / trigger_rumble_damping_nonzero; + mag_trigger_left = (u8)((max_damped * fraction_TL) / 0xFFFF); + mag_trigger_right = (u8)((max_damped * fraction_TR) / 0xFFFF); + + create_ff_pck( + &ff_pck, 0x03, + ff_active, + mag_trigger_left, mag_trigger_right, + mag_main_left, mag_main_right, + 0); + + hid_dbg_lvl(DBG_LVL_FEW, hdev, + "active: %#04x, max: %#04x, prop_left: %#04x, prop_right: %#04x, left trigger: %#04x, right: %#04x\n", + ff_active, + max, fraction_TL, fraction_TR, + ff_pck.ff.magnitude_left_trigger, + ff_pck.ff.magnitude_right_trigger); + + hid_hw_output_report(hdev, (u8 *)&ff_pck, sizeof(ff_pck)); + } + else { + hid_dbg_lvl(DBG_LVL_FEW, hdev, "skipping effect, rumble motors are idle"); + } + spin_unlock_irqrestore(&ffdata->lock, flags); return 0; } @@ -340,6 +365,10 @@ static int xpadneo_initDevice(struct hid_device *hdev) struct ff_report ff_pck; + struct xpadneo_ffdata *ffdata = kzalloc(sizeof(struct xpadneo_ffdata), GFP_KERNEL); + if (!ffdata) + return -ENOMEM; + /* TODO: outsource that */ ff_pck.ff = ff_clear; @@ -370,9 +399,11 @@ static int xpadneo_initDevice(struct hid_device *hdev) /* Init Input System for Force Feedback (FF) */ input_set_capability(idev, EV_FF, FF_RUMBLE); - error = input_ff_create_memless(idev, NULL, xpadneo_ff_play); - if (error) + error = input_ff_create_memless(idev, (void *)ffdata, xpadneo_ff_play); + if (error) { + kfree(ffdata); return error; + } /*