Here we go.
I wrote this patch about two years ago...sorry. I have yet to submit it to upstream. I'm actually in the process of re-testing against a recent kernel so that I can do just that. In the meantime, if you want your steering experience on ETS2 to not suck, you can try building your own kernel with the patch included below. (Jump to the very end for the goodies.)
Just a bit of background on the problem...
The Linux force feedback architecture naively assumes two things:
- That a program will politely send force feedback commands in a gentle, trickling stream.
- That the force feedback device will consume USB commands - and thereby rip through the workqueue - at the speed of light.
ETS2 breaks both of these assumptions by spamming the force feedback system at absurdly high rates, while the kernel pops at whatever rate it's able to, quickly overwhelming the driver and queuing up a MASSIVE batch of commands that the driver will eventually churn through...by which time you'll have decided that ETS2 + Linux FF sucks, you turn off the game and momentarily think about reinstalling Windows (immediately after which you spontaneously vomit).
I got my first hint at this after going through the above experience, and then exiting the game and watching my wheel bump and thump and shudder and rumble for about 3 minutes. I was pondering... Something seemed non-random about the things it was doing, and then it suddenly STOPPED. I realized it had been playing all the effects I had just experienced, just really, really, really, really late.
In lieu of waiting for the ETS2 developers to fix their code, I decided to take a stab at the Logitech FF subsystem in the driver itself. I'll spare you the gory details of my testing. Ultimately, I built something that works well enough. And by well enough, I mean that I absolutely LOVE the FF in ETS2 now, it works great.
But I'm a programmer, so I have to say "there is definitely room for improvement," but here's what it does:
The basic premise is that we have only certain things we do to a FF device. We can create and delete effects. We can play and stop them. We can update them. That's about it.
If we update something a bunch of times, it doesn't make sense to keep the old updates queued and waiting to be sent. We assume the device just cares about the last update for a given effect, so we just save that and flag that there's an update pending for that specific effect number. Future updates overwrite our queued update, so that only the latest update is sent by the time the workqueue gets around to this request.
If we play and then stop something, but the driver hasn't gotten around to playing it at all, then it's as though it had never been played to begin with. Don't send the play event. Likewise, if we stop and then play an effect again, it's like it never stopped (so to speak). We might queue up a single play event to restart it, but ultimately it's a single event.
If we queue a bunch of play events up for a single effect, we only care about the last one: if the driver/device had kept up, they'd all be played. Since it didn't, they're gone (expired) and the program wants us to do something else (even if it's the same thing).
If we update and play and stop and do all sorts of things, and then go and DELETE the effect, then all those updates and plays and stops are pointless. Don't bother with them.
What this all boils down to is that we:
- Keep the queue squashed to the almost minimum possible size (I said there were improvements to make...)
- Send significantly fewer events (faster queue processing, since we're not waiting on synchronous USB calls to complete all the time)
- Send only the most recent events
- Maintain a sensible, current state, so that we're not playing old effects that have since been updated.
Effectively, nothing is lost by doing the above. We're not skipping anything that matters. We are only skipping requests that don't matter, because they were replaced with newer requests. As long as the assumption holds that the wheel itself is simple-minded, all this works actually quite well.
Ideally, the Logitech FF subsystem in the kernel would be better off maintaining a FULL STATE for the device (i.e. the effects), such that all it cares about sending during its workqueue job is the delta of its old state to its current state. I might eventually get around to doing that. This would finally squash the last minor bug: the create/destroy queue exploder bug.
This last bug is where a program (I'm looking at you, ETS2) creates and destroys effects super-fast. Ignore the pointlessness of this, and just deal with the fact that IT HAPPENS. In the current patch, creates and deletes only really affect updates and play/stops. This means the queue can load up with creates and deletes. The good news is that these are very quickly processed, such that instead of waiting five minutes (no joke) for it to catch up, you might have to wait all of about 5 seconds, by which point the game will have finished loading into the level and you'll be processing FF events in realtime.
Anyway, enough exposition. Here's the patch. I'm not going into the gritty details of applying it - that's left as an exercise to the reader. After all, if you've kept up this far, you probably know how to patch and build a kernel. NOTE: it's actually not terrible to just build the module itself and overwrite your existing kernel module, as long as all the version numbers match up. This is how I did early testing. Just be aware that the module will be lost if you upgrade your kernel...at least until I get this stupid thing submitted.
--- /a/hid-logitech-hidpp.c 2020-05-17 02:17:49.005405172 -0400
+++ /b/hid-logitech-hidpp.c 2020-05-17 02:17:17.398901709 -0400
@@ -26,6 +26,7 @@
#include <linux/workqueue.h>
#include <linux/atomic.h>
#include <linux/fixp-arith.h>
+#include <linux/mutex.h>
#include <asm/unaligned.h>
#include "usbhid/usbhid.h"
#include "hid-ids.h"
@@ -34,12 +35,12 @@
MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
MODULE_AUTHOR("Nestor Lopez Casado <nlopezcasad@logitech.com>");
-static bool disable_raw_mode;
+static bool disable_raw_mode = false;
module_param(disable_raw_mode, bool, 0644);
MODULE_PARM_DESC(disable_raw_mode,
"Disable Raw mode reporting for touchpads and keep firmware gestures.");
-static bool disable_tap_to_click;
+static bool disable_tap_to_click = false;
module_param(disable_tap_to_click, bool, 0644);
MODULE_PARM_DESC(disable_tap_to_click,
"Disable Tap-To-Click mode reporting for touchpads (only on the K400 currently).");
@@ -157,7 +158,7 @@
struct hidpp_device *hidpp = hid_get_drvdata(hdev);
int fields_count, ret;
- hidpp = hid_get_drvdata(hdev);
+// hidpp = hid_get_drvdata(hdev);
switch (hidpp_report->report_id) {
case REPORT_ID_HIDPP_SHORT:
@@ -815,6 +816,14 @@
#define HIDPP_FF_EFFECTID_NONE -1
#define HIDPP_FF_EFFECTID_AUTOCENTER -2
+#define HIDPP_FF_EFFECT_STATUS_PLAY_PENDING 0x01
+#define HIDPP_FF_EFFECT_STATUS_UPDATE_PENDING 0x02
+#define HIDPP_FF_EFFECT_STATUS_STOP_PENDING 0x03
+#define HIDPP_FF_EFFECT_STATUS_STOPPED 0x04
+#define HIDPP_FF_EFFECT_STATUS_ASSIGN_PENDING 0x05
+#define HIDPP_FF_EFFECT_STATUS_PLAYING 0x06
+#define HIDPP_FF_EFFECT_STATUS_DESTROYING 0x07
+
#define HIDPP_FF_MAX_PARAMS 20
#define HIDPP_FF_RESERVED_SLOTS 1
@@ -827,10 +836,19 @@
u8 slot_autocenter;
u8 num_effects;
int *effect_ids;
+ struct mutex post_mutex;
+ struct mutex update_mutex;
+ unsigned long *effect_status;
+ struct hidpp_ff_effect_update_data *effect_updates;
struct workqueue_struct *wq;
atomic_t workqueue_size;
};
+struct hidpp_ff_effect_update_data {
+ u8 params[HIDPP_FF_MAX_PARAMS];
+ u8 size;
+};
+
struct hidpp_ff_work_data {
struct work_struct work;
struct hidpp_ff_private_data *data;
@@ -840,7 +858,7 @@
u8 size;
};
-static const signed short hiddpp_ff_effects[] = {
+static const signed short hidpp_ff_effects[] = {
FF_CONSTANT,
FF_PERIODIC,
FF_SINE,
@@ -855,7 +873,7 @@
-1
};
-static const signed short hiddpp_ff_effects_v2[] = {
+static const signed short hidpp_ff_effects_v2[] = {
FF_RAMP,
FF_FRICTION,
FF_INERTIA,
@@ -876,7 +894,17 @@
"inertia"
};
+static const char *HIDPP_FF_PERIODIC_NAMES[] = {
+ "invalid",
+ "sine",
+ "square",
+ "triangle",
+ "sawtooth-up",
+ "sawtooth-down"
+};
+
+/*
static u8 hidpp_ff_find_effect(struct hidpp_ff_private_data *data, int effect_id)
{
int i;
@@ -887,6 +915,7 @@
return 0;
}
+*/
static void hidpp_ff_work_handler(struct work_struct *w)
{
@@ -896,61 +925,189 @@
u8 slot;
int ret;
+ /* If the command is DOWNLOAD_EFFECT, we are going to use the data
+ * from the internal effect_update array, since it represents the
+ * most recently received effect update for this effect.
+ * DOES NOT APPLY TO RESERVED EFFECTS!!
+ *
+ * If it is an effect STATE UPDATE, we set the appropriate parameter
+ * value and clear the playback update flags.
+ *
+ * If the effect is scheduled to be DESTROYed, then we can skip
+ * playing, stopping, or updating the effect altogether.
+ */
+ mutex_lock(&data->update_mutex);
+ switch (wd->command) {
+ case HIDPP_FF_DOWNLOAD_EFFECT:
+ if (wd->effect_id >= 0) {
+ wd->size = data->effect_updates[wd->effect_id].size;
+ memcpy(wd->params, data->effect_updates[wd->effect_id].params, wd->size);
+
+ clear_bit(HIDPP_FF_EFFECT_STATUS_UPDATE_PENDING, &data->effect_status[wd->effect_id]);
+ }
+ break;
+ case HIDPP_FF_SET_EFFECT_STATE:
+ /* Technically only ONE of these flags should be active... */
+ if (test_bit(HIDPP_FF_EFFECT_STATUS_PLAY_PENDING, &data->effect_status[wd->effect_id]) &&
+ test_bit(HIDPP_FF_EFFECT_STATUS_STOP_PENDING, &data->effect_status[wd->effect_id])) {
+ hid_warn(data->hidpp->hid_dev, "Both play and stop request flags found for effect id %d - stopping effect! (This might not be what you want.)\n", wd->effect_id);
+
+ clear_bit(HIDPP_FF_EFFECT_STATUS_PLAY_PENDING, &data->effect_status[wd->effect_id]);
+ }
+
+ if (test_bit(HIDPP_FF_EFFECT_STATUS_PLAY_PENDING, &data->effect_status[wd->effect_id])) {
+ wd->params[1] = HIDPP_FF_EFFECT_STATE_PLAY;
+ set_bit(HIDPP_FF_EFFECT_STATUS_PLAYING, &data->effect_status[wd->effect_id]);
+ clear_bit(HIDPP_FF_EFFECT_STATUS_STOPPED, &data->effect_status[wd->effect_id]);
+ } else if (test_bit(HIDPP_FF_EFFECT_STATUS_STOP_PENDING, &data->effect_status[wd->effect_id])) {
+ wd->params[1] = HIDPP_FF_EFFECT_STATE_STOP;
+ set_bit(HIDPP_FF_EFFECT_STATUS_STOPPED, &data->effect_status[wd->effect_id]);
+ clear_bit(HIDPP_FF_EFFECT_STATUS_PLAYING, &data->effect_status[wd->effect_id]);
+ }
+
+ clear_bit(HIDPP_FF_EFFECT_STATUS_PLAY_PENDING, &data->effect_status[wd->effect_id]);
+ clear_bit(HIDPP_FF_EFFECT_STATUS_STOP_PENDING, &data->effect_status[wd->effect_id]);
+
+ break;
+ }
+
+ /* Check to see if we should even do what we've been told to do... */
+ if (wd->effect_id >= 0 && test_bit(HIDPP_FF_EFFECT_STATUS_DESTROYING, &data->effect_status[wd->effect_id]) &&
+ (wd->command == HIDPP_FF_DOWNLOAD_EFFECT || wd->command == HIDPP_FF_SET_EFFECT_STATE)) {
+ /* We're about to destroy this effect. Skip whatever this is. */
+ mutex_unlock(&data->update_mutex);
+ goto out;
+ }
+
+ mutex_unlock(&data->update_mutex);
+
+
/* add slot number if needed */
+ /* ff_core provides a zero-based effect ID to us and to the caller.
+ * It takes care of the "effect assignment," since we are in another
+ * thread and the original ioctl call already returned to caller.
+ * We have one effect reserved and it's in its own slot: autocenter.
+ * Everything else simply needs to map between the caller's ID and the
+ * ID that the device supplies us.
+ */
switch (wd->effect_id) {
case HIDPP_FF_EFFECTID_AUTOCENTER:
+ //dbg_hid("work_handler slot_autocenter selected: %d\n", data->slot_autocenter);
wd->params[0] = data->slot_autocenter;
break;
case HIDPP_FF_EFFECTID_NONE:
/* leave slot as zero */
+ //dbg_hid("work_handler no slot selected\n");
break;
default:
/* find current slot for effect */
- wd->params[0] = hidpp_ff_find_effect(data, wd->effect_id);
+ wd->params[0] = data->effect_ids[wd->effect_id];
+ //dbg_hid("work_handler get slot for effect_id=%d -> %d\n", wd->effect_id, wd->params[0]);
break;
}
+
/* send command and wait for reply */
ret = hidpp_send_fap_command_sync(data->hidpp, data->feature_index,
wd->command, wd->params, wd->size, &response);
if (ret) {
hid_err(data->hidpp->hid_dev, "Failed to send command to device!\n");
+ switch (wd->command) {
+ case HIDPP_FF_DOWNLOAD_EFFECT:
+ hid_err(data->hidpp->hid_dev, "--- failed command: DOWNLOAD EFFECT - id=%d\n", wd->effect_id);
+ break;
+ case HIDPP_FF_DESTROY_EFFECT:
+ hid_err(data->hidpp->hid_dev, "--- failed command: DESTROY EFFECT - id=%d\n", wd->effect_id);
+ break;
+ case HIDPP_FF_SET_GLOBAL_GAINS:
+ hid_err(data->hidpp->hid_dev, "--- failed command: SET GLOBAL GAIN - id=%d\n", wd->effect_id);
+ break;
+ case HIDPP_FF_SET_APERTURE:
+ hid_err(data->hidpp->hid_dev, "--- failed command: SET APERTURE - id=%d\n", wd->effect_id);
+ break;
+ case HIDPP_FF_SET_EFFECT_STATE:
+ hid_err(data->hidpp->hid_dev, "--- failed command: SET EFFECT STATE - id=%d\n", wd->effect_id);
+ break;
+ default:
+ hid_err(data->hidpp->hid_dev, "--- failed command: OTHER... - id=%d\n", wd->effect_id);
+ }
goto out;
}
/* parse return data */
+ mutex_lock(&data->update_mutex);
switch (wd->command) {
case HIDPP_FF_DOWNLOAD_EFFECT:
+ dbg_hid("work_handler: download effect id=%d\n", wd->effect_id);
+ //hid_warn(data->hidpp->hid_dev, "work handler: download effect id %d\n", wd->effect_id);
slot = response.fap.params[0];
- if (slot > 0 && slot <= data->num_effects) {
- if (wd->effect_id >= 0)
- /* regular effect uploaded */
- data->effect_ids[slot-1] = wd->effect_id;
- else if (wd->effect_id >= HIDPP_FF_EFFECTID_AUTOCENTER)
- /* autocenter spring uploaded */
+ //dbg_hid("work_handler response slot: %d\n", slot);
+ /* The device returns a non-zero effect ID. Store it in the map
+ * unless we know we've just assigned the AUTOCENTER effect.
+ */
+ if (slot > 0) {
+ switch (wd->effect_id) {
+ case HIDPP_FF_EFFECTID_AUTOCENTER:
+ /* autocenter spring uploaded */
+ //dbg_hid("assigning autocenter slot # %d\n", slot);
data->slot_autocenter = slot;
+ break;
+ default:
+ //dbg_hid("assigning/updating effect-id %d slot # %d (array index %d)\n", wd->effect_id, slot, wd->effect_id);
+ /* regular effect uploaded */
+ data->effect_ids[wd->effect_id] = slot;
+ break;
+ }
}
+
+ clear_bit(HIDPP_FF_EFFECT_STATUS_ASSIGN_PENDING, &data->effect_status[wd->effect_id]);
+
break;
case HIDPP_FF_DESTROY_EFFECT:
- if (wd->effect_id >= 0)
- /* regular effect destroyed */
- data->effect_ids[wd->params[0]-1] = -1;
- else if (wd->effect_id >= HIDPP_FF_EFFECTID_AUTOCENTER)
+ dbg_hid("work_handler: destroy effect id=%d\n", wd->effect_id);
+ //hid_warn(data->hidpp->hid_dev, "work handler: destroy effect id %d\n", wd->effect_id);
+ switch (wd->effect_id) {
+ case HIDPP_FF_EFFECTID_AUTOCENTER:
/* autocenter spring destoyed */
data->slot_autocenter = 0;
+ break;
+ default:
+ /* regular effect destroyed - Erase the mapping. */
+ data->effect_ids[wd->effect_id] = 0;
+
+ /* Do not destroy status bits, since there may be outstanding commands later in the queue. */
+ /* Instead, just set/clear the appropriate bits. */
+ clear_bit(HIDPP_FF_EFFECT_STATUS_DESTROYING, &data->effect_status[wd->effect_id]);
+ clear_bit(HIDPP_FF_EFFECT_STATUS_PLAYING, &data->effect_status[wd->effect_id]);
+ set_bit(HIDPP_FF_EFFECT_STATUS_STOPPED, &data->effect_status[wd->effect_id]);
+ break;
+ }
break;
case HIDPP_FF_SET_GLOBAL_GAINS:
+ dbg_hid("work_handler: set global gains\n");
data->gain = (wd->params[0] << 8) + wd->params[1];
break;
case HIDPP_FF_SET_APERTURE:
+ dbg_hid("work_handler: set aperture\n");
data->range = (wd->params[0] << 8) + wd->params[1];
break;
+ case HIDPP_FF_SET_EFFECT_STATE:
+ /* always clear the play_pending bit, even if it isn't set.
+ * We only care that an effect is either been manually stopped,
+ * or that it has executed its play command. */
+ dbg_hid("work_handler: set effect state id=%d\n", wd->effect_id);
+ //hid_warn(data->hidpp->hid_dev, "work handler: play/stop effect id %d\n", wd->effect_id);
+ //clear_bit(HIDPP_FF_EFFECT_STATUS_PLAY_PENDING, &data->effect_status[wd->effect_id]);
+ break;
default:
+ dbg_hid("work_handler: other\n");
/* no action needed */
break;
}
+ mutex_unlock(&data->update_mutex);
+
out:
atomic_dec(&data->workqueue_size);
kfree(wd);
@@ -973,13 +1130,13 @@
memcpy(wd->params, params, size);
atomic_inc(&data->workqueue_size);
- queue_work(data->wq, &wd->work);
-
/* warn about excessive queue size */
s = atomic_read(&data->workqueue_size);
if (s >= 20 && s % 20 == 0)
hid_warn(data->hidpp->hid_dev, "Force feedback command queue contains %d commands, causing substantial delays!", s);
+ queue_work(data->wq, &wd->work);
+
return 0;
}
@@ -990,6 +1147,11 @@
u8 size;
int force;
+ if (effect->id < 0 || effect->id >= data->num_effects)
+ return -EINVAL;
+
+ //hid_warn(data->hidpp->hid_dev, "update effect id %d\n", effect->id);
+
/* set common parameters */
params[2] = effect->replay.length >> 8;
params[3] = effect->replay.length & 255;
@@ -1009,14 +1171,21 @@
params[12] = effect->u.constant.envelope.fade_length >> 8;
params[13] = effect->u.constant.envelope.fade_length & 255;
size = 14;
- dbg_hid("Uploading constant force level=%d in dir %d = %d\n",
+ dbg_hid("Uploading id [%d] CONSTANT force\n", effect->id);
+ dbg_hid("** [%d] CONSTANT level=%d in dir %d = %d\n",
+ effect->id,
effect->u.constant.level,
effect->direction, force);
- dbg_hid(" envelope attack=(%d, %d ms) fade=(%d, %d ms)\n",
+ dbg_hid("** [%d] CONSTANT envelope attack=(%d, %d ms) fade=(%d, %d ms)\n",
+ effect->id,
effect->u.constant.envelope.attack_level,
effect->u.constant.envelope.attack_length,
effect->u.constant.envelope.fade_level,
effect->u.constant.envelope.fade_length);
+ dbg_hid("** [%d] CONSTANT replay duration=%d ms, delay=%d ms\n",
+ effect->id,
+ effect->replay.length,
+ effect->replay.delay);
break;
case FF_PERIODIC:
{
@@ -1056,16 +1225,23 @@
params[18] = effect->u.periodic.envelope.fade_length >> 8;
params[19] = effect->u.periodic.envelope.fade_length & 255;
size = 20;
- dbg_hid("Uploading periodic force mag=%d/dir=%d, offset=%d, period=%d ms, phase=%d\n",
+ dbg_hid("Uploading id [%d] PERIODIC <%s> force\n", effect->id, HIDPP_FF_PERIODIC_NAMES[params[1]]);
+ dbg_hid("** [%d] PERIODIC mag=%d/dir=%d, offset=%d, period=%d ms, phase=%d\n",
+ effect->id,
effect->u.periodic.magnitude, effect->direction,
effect->u.periodic.offset,
effect->u.periodic.period,
effect->u.periodic.phase);
- dbg_hid(" envelope attack=(%d, %d ms) fade=(%d, %d ms)\n",
+ dbg_hid("** [%d] PERIODIC envelope attack=(%d, %d ms) fade=(%d, %d ms)\n",
+ effect->id,
effect->u.periodic.envelope.attack_level,
effect->u.periodic.envelope.attack_length,
effect->u.periodic.envelope.fade_level,
effect->u.periodic.envelope.fade_length);
+ dbg_hid("** [%d] PERIODIC replay duration=%d ms, delay=%d ms\n",
+ effect->id,
+ effect->replay.length,
+ effect->replay.delay);
break;
}
case FF_RAMP:
@@ -1083,15 +1259,22 @@
params[14] = effect->u.ramp.envelope.fade_length >> 8;
params[15] = effect->u.ramp.envelope.fade_length & 255;
size = 16;
- dbg_hid("Uploading ramp force level=%d -> %d in dir %d = %d\n",
+ dbg_hid("Uploading id [%d] RAMP force\n", effect->id);
+ dbg_hid("** [%d] RAMP level=%d -> %d in dir %d = %d\n",
+ effect->id,
effect->u.ramp.start_level,
effect->u.ramp.end_level,
effect->direction, force);
- dbg_hid(" envelope attack=(%d, %d ms) fade=(%d, %d ms)\n",
+ dbg_hid("** [%d] RAMP envelope attack=(%d, %d ms) fade=(%d, %d ms)\n",
+ effect->id,
effect->u.ramp.envelope.attack_level,
effect->u.ramp.envelope.attack_length,
effect->u.ramp.envelope.fade_level,
effect->u.ramp.envelope.fade_length);
+ dbg_hid("** [%d] RAMP replay duration=%d ms, delay=%d ms\n",
+ effect->id,
+ effect->replay.length,
+ effect->replay.delay);
break;
case FF_FRICTION:
case FF_INERTIA:
@@ -1111,21 +1294,59 @@
params[16] = effect->u.condition[0].right_saturation >> 9;
params[17] = (effect->u.condition[0].right_saturation >> 1) & 255;
size = 18;
- dbg_hid("Uploading %s force left coeff=%d, left sat=%d, right coeff=%d, right sat=%d\n",
- HIDPP_FF_CONDITION_NAMES[effect->type - FF_SPRING],
+ dbg_hid("Uploading id [%d] CONDITION <%s> force\n",
+ effect->id,
+ HIDPP_FF_CONDITION_NAMES[effect->type - FF_SPRING]);
+ dbg_hid("** [%d] CONDITION left coeff=%d, left sat=%d, right coeff=%d, right sat=%d\n",
+ effect->id,
effect->u.condition[0].left_coeff,
effect->u.condition[0].left_saturation,
effect->u.condition[0].right_coeff,
effect->u.condition[0].right_saturation);
- dbg_hid(" deadband=%d, center=%d\n",
+ dbg_hid("** [%d] CONDITION deadband=%d, center=%d\n",
+ effect->id,
effect->u.condition[0].deadband,
effect->u.condition[0].center);
+ dbg_hid("** [%d] CONDITION replay duration=%d ms, delay=%d ms\n",
+ effect->id,
+ effect->replay.length,
+ effect->replay.delay);
break;
default:
hid_err(data->hidpp->hid_dev, "Unexpected force type %i!\n", effect->type);
return -EINVAL;
}
+ /* copy data into effect_updates array. */
+ mutex_lock(&data->update_mutex);
+
+ data->effect_updates[effect->id].size = size;
+ /* memcpy here is not as efficient as it could be, but it's also vastly safer since
+ * the above code might return -EINVAL at multiple places. */
+ memcpy(data->effect_updates[effect->id].params, params, size);
+
+ /* If there is an update already pending, we're done and can simply return. */
+ /* Note that an ASSIGN can only be true if UPDATE is true. */
+ if (test_bit(HIDPP_FF_EFFECT_STATUS_UPDATE_PENDING, &data->effect_status[effect->id])) {
+ dbg_hid("** [%d] overwriting outstanding request\n", effect->id);
+ //hid_warn(data->hidpp->hid_dev, "Rapid update of FF effect: overwriting outstanding request for effect id %d\n", effect->id);
+ mutex_unlock(&data->update_mutex);
+ return 0;
+ }
+
+ /* Set the update bit and push the update onto the workqueue. */
+ set_bit(HIDPP_FF_EFFECT_STATUS_UPDATE_PENDING, &data->effect_status[effect->id]);
+
+ /* This will be a new slot assignment */
+ if (data->effect_ids[effect->id] == 0)
+ set_bit(HIDPP_FF_EFFECT_STATUS_ASSIGN_PENDING, &data->effect_status[effect->id]);
+
+ mutex_unlock(&data->update_mutex);
+
+ /* Don't copy ram again. */
+ size = 0;
+ dbg_hid("** [%d] posting new request\n", effect->id);
+
return hidpp_ff_queue_work(data, effect->id, HIDPP_FF_DOWNLOAD_EFFECT, params, size);
}
@@ -1133,12 +1354,58 @@
{
struct hidpp_ff_private_data *data = dev->ff->private;
u8 params[2];
+ bool play_pending, stop_pending, playing, stopped;
+ bool queue_update;
+
+ if (effect_id < 0 || effect_id >= data->num_effects)
+ return -EINVAL;
+
+ dbg_hid("ST%s playback of effect [%d].\n", value?"ART":"OP", effect_id);
+ //hid_warn(data->hidpp->hid_dev, "ST%s playback of effect [%d].\n", value?"ART":"OP", effect_id);
+
+ /* don't cue work that is already done. */
+ /* If we queued a play request and it hasn't been processed yet, don't queue another.
+ * Since we can't tell when playing stops (if it stops itself), we will always honor
+ * play requests as long as they don't stack up.
+ * Stop requests are always considered instanteous and final.
+ */
+ mutex_lock(&data->update_mutex);
+ play_pending = test_bit(HIDPP_FF_EFFECT_STATUS_PLAY_PENDING, &data->effect_status[effect_id]);
+ playing = test_bit(HIDPP_FF_EFFECT_STATUS_PLAYING, &data->effect_status[effect_id]);
+ stop_pending = test_bit(HIDPP_FF_EFFECT_STATUS_STOP_PENDING, &data->effect_status[effect_id]);
+ stopped = test_bit(HIDPP_FF_EFFECT_STATUS_STOPPED, &data->effect_status[effect_id]);
+
+ if ((value && (play_pending || playing)) || (!value && (stop_pending || stopped))) {
+ dbg_hid("Playback of effect [%d] already ST%sED.\n", effect_id, value?"ART":"OPP");
+ mutex_unlock(&data->update_mutex);
+ return 0;
+ }
+
+ /* If we already have a playback update pending, don't queue up another update.
+ * Instead, just update the necessary flag.
+ */
+ if (play_pending || stop_pending) {
+ queue_update = false;
+ } else {
+ queue_update = true;
+ }
- params[1] = value ? HIDPP_FF_EFFECT_STATE_PLAY : HIDPP_FF_EFFECT_STATE_STOP;
+// params[1] = value ? HIDPP_FF_EFFECT_STATE_PLAY : HIDPP_FF_EFFECT_STATE_STOP;
- dbg_hid("St%sing playback of effect %d.\n", value?"art":"opp", effect_id);
+ /* set the appropriate flags */
+ if (value) {
+ set_bit(HIDPP_FF_EFFECT_STATUS_PLAY_PENDING, &data->effect_status[effect_id]);
+ clear_bit(HIDPP_FF_EFFECT_STATUS_STOP_PENDING, &data->effect_status[effect_id]);
+ } else {
+ clear_bit(HIDPP_FF_EFFECT_STATUS_PLAY_PENDING, &data->effect_status[effect_id]);
+ set_bit(HIDPP_FF_EFFECT_STATUS_STOP_PENDING, &data->effect_status[effect_id]);
+ }
+ mutex_unlock(&data->update_mutex);
- return hidpp_ff_queue_work(data, effect_id, HIDPP_FF_SET_EFFECT_STATE, params, ARRAY_SIZE(params));
+ if (queue_update)
+ return hidpp_ff_queue_work(data, effect_id, HIDPP_FF_SET_EFFECT_STATE, params, ARRAY_SIZE(params));
+
+ return 0;
}
static int hidpp_ff_erase_effect(struct input_dev *dev, int effect_id)
@@ -1146,7 +1413,29 @@
struct hidpp_ff_private_data *data = dev->ff->private;
u8 slot = 0;
- dbg_hid("Erasing effect %d.\n", effect_id);
+ if (effect_id == HIDPP_FF_EFFECTID_AUTOCENTER) {
+ /* We are requesting the autocenter be destroyed... for now, just warn. */
+ hid_err(data->hidpp->hid_dev, "WARNING: FEATURE NOT IMPLEMENTED - destroy AUTOCENTER effect\n");
+ return -EINVAL;
+ }
+
+ if (effect_id < 0 || effect_id >= data->num_effects)
+ return -EINVAL;
+
+ mutex_lock(&data->update_mutex);
+ /* Check that we either uploaded an effect that hasn't finished uploading yet,
+ * or we have not defined this effect. */
+ if ((!data->effect_ids[effect_id] && !test_bit(HIDPP_FF_EFFECT_STATUS_ASSIGN_PENDING, &data->effect_status[effect_id])) ||
+ test_bit(HIDPP_FF_EFFECT_STATUS_DESTROYING, &data->effect_status[effect_id])) {
+ mutex_unlock(&data->update_mutex);
+ return 0;
+ }
+
+ set_bit(HIDPP_FF_EFFECT_STATUS_DESTROYING, &data->effect_status[effect_id]);
+ mutex_unlock(&data->update_mutex);
+
+ /* It's either defined or about to be. Issue its destruction. */
+ dbg_hid("ERASE effect [%d].\n", effect_id);
return hidpp_ff_queue_work(data, effect_id, HIDPP_FF_DESTROY_EFFECT, &slot, 1);
}
@@ -1156,7 +1445,7 @@
struct hidpp_ff_private_data *data = dev->ff->private;
u8 params[18];
- dbg_hid("Setting autocenter to %d.\n", magnitude);
+ dbg_hid("SET AUTOCENTER to magnitude %d.\n", magnitude);
/* start a standard spring effect */
params[1] = HIDPP_FF_EFFECT_SPRING | HIDPP_FF_EFFECT_AUTOSTART;
@@ -1178,7 +1467,7 @@
struct hidpp_ff_private_data *data = dev->ff->private;
u8 params[4];
- dbg_hid("Setting gain to %d.\n", gain);
+ dbg_hid("SET GAIN to %d.\n", gain);
params[0] = gain >> 8;
params[1] = gain & 255;
@@ -1224,6 +1513,11 @@
struct hidpp_ff_private_data *data = ff->private;
kfree(data->effect_ids);
+ kfree(data->effect_updates);
+ kfree(data->effect_status);
+
+ mutex_destroy(&data->update_mutex);
+ mutex_destroy(&data->post_mutex);
}
static int hidpp_ff_init(struct hidpp_device *hidpp, u8 feature_index)
@@ -1248,11 +1542,11 @@
version = bcdDevice & 255;
/* Set supported force feedback capabilities */
- for (j = 0; hiddpp_ff_effects[j] >= 0; j++)
- set_bit(hiddpp_ff_effects[j], dev->ffbit);
+ for (j = 0; hidpp_ff_effects[j] >= 0; j++)
+ set_bit(hidpp_ff_effects[j], dev->ffbit);
if (version > 1)
- for (j = 0; hiddpp_ff_effects_v2[j] >= 0; j++)
- set_bit(hiddpp_ff_effects_v2[j], dev->ffbit);
+ for (j = 0; hidpp_ff_effects_v2[j] >= 0; j++)
+ set_bit(hidpp_ff_effects_v2[j], dev->ffbit);
/* Read number of slots available in device */
error = hidpp_send_fap_command_sync(hidpp, feature_index,
@@ -1287,8 +1581,25 @@
data->version = version;
data->slot_autocenter = 0;
data->num_effects = num_slots;
- for (j = 0; j < num_slots; j++)
- data->effect_ids[j] = -1;
+
+ data->effect_updates = kcalloc(num_slots, sizeof(*(data->effect_updates)), GFP_KERNEL);
+ if (!data->effect_updates) {
+ kfree(data->effect_ids);
+ kfree(data);
+ return -ENOMEM;
+ }
+
+
+ data->effect_status = kcalloc(num_slots, sizeof(unsigned long), GFP_KERNEL);
+ if (!data->effect_status) {
+ kfree(data->effect_updates);
+ kfree(data->effect_ids);
+ kfree(data);
+ return -ENOMEM;
+ }
+
+ mutex_init(&data->post_mutex);
+ mutex_init(&data->update_mutex);
ff = dev->ff;
ff->private = data;
