- Rework the effect replacing logic

- Preallocate memory for ff_effect structs

diff --git a/klgd_ff_plugin.c b/klgd_ff_plugin.c
index b6b7694fef549394da0d2e22d13b40547a706f2b..e8d3061f90023de81fb34ed1408c8f6245b99096 100644 (file)
--- a/klgd_ff_plugin.c
+++ b/klgd_ff_plugin.c
@@ -2,7 +2,7 @@
 #include <linux/slab.h>
 
 static bool ffpl_has_gain(const struct ff_effect *eff);
-static bool ffpl_replace_effect(const struct ff_effect *ac_eff, const struct ff_effect *la_eff);
+static bool ffpl_needs_replacing(const struct ff_effect *ac_eff, const struct ff_effect *la_eff);
 
 static int ffpl_erase_effect(struct klgd_plugin_private *priv, struct klgd_command_stream *s, struct ffpl_effect *eff)
 {
@@ -11,19 +11,38 @@ static int ffpl_erase_effect(struct klgd_plugin_private *priv, struct klgd_comma
                union ffpl_control_data data;
                int ret;
 
-               data.effect = eff->active;
+               data.effects.cur = &eff->active;
+               data.effects.old = NULL;
                ret = priv->control(dev, s, FFPL_UPL_TO_EMP, data);
                if (ret)
                        return ret;
        }
 
-       kfree(eff->active);
-       eff->active = NULL;
        eff->state = FFPL_EMPTY;
        eff->uploaded_to_device = false;
        return 0;
 }
 
+static int ffpl_replace_effect(struct klgd_plugin_private *priv, struct klgd_command_stream *s, struct ffpl_effect *eff,
+                              const enum ffpl_control_command cmd)
+{
+       struct input_dev *dev = priv->dev;
+       union ffpl_control_data data;
+       int ret;
+
+       data.effects.cur = &eff->latest;
+       data.effects.old = &eff->active;
+       ret = priv->control(dev, s, cmd, data);
+       if (!ret) {
+               eff->active = eff->latest;
+               eff->state = (cmd == FFPL_OWR_TO_UPL) ? FFPL_UPLOADED : FFPL_STARTED;
+               eff->replace = false;
+               eff->change = FFPL_DONT_TOUCH;
+               return 0;
+       }
+       return ret;
+}
+
 static int ffpl_start_effect(struct klgd_plugin_private *priv, struct klgd_command_stream *s, struct ffpl_effect *eff)
 {
        struct input_dev *dev = priv->dev;
@@ -31,7 +50,8 @@ static int ffpl_start_effect(struct klgd_plugin_private *priv, struct klgd_comma
        int ret;
        enum ffpl_control_command cmd;
 
-       data.effect = eff->active;
+       data.effects.cur = &eff->active;
+       data.effects.old = NULL;
        if (priv->upload_when_started) {
                if (eff->uploaded_to_device)
                        cmd = FFPL_UPL_TO_SRT;
@@ -64,7 +84,8 @@ static int ffpl_stop_effect(struct klgd_plugin_private *priv, struct klgd_comman
        int ret;
        enum ffpl_control_command cmd;
 
-       data.effect = eff->active;
+       data.effects.cur = &eff->active;
+       data.effects.old = NULL;
        if (priv->erase_when_stopped || (priv->has_srt_to_emp && eff->change == FFPL_TO_ERASE))
                cmd = FFPL_SRT_TO_EMP;
        else
@@ -88,34 +109,32 @@ static int ffpl_update_effect(struct klgd_plugin_private *priv, struct klgd_comm
        if (!eff->uploaded_to_device)
                return ffpl_start_effect(priv, s, eff);
 
-       data.effect = eff->latest;
+       data.effects.cur = &eff->latest;
+       data.effects.old = NULL;
        ret = priv->control(dev, s, FFPL_SRT_TO_UDT, data);
        if (ret)
                return ret;
        eff->active = eff->latest;
-       eff->latest = NULL;
        return 0;
 }
 
 static int ffpl_upload_effect(struct klgd_plugin_private *priv, struct klgd_command_stream *s, struct ffpl_effect *eff)
 {
-       struct input_dev *dev = priv->dev;
-       union ffpl_control_data data;
-       int ret;
-
-       if (priv->upload_when_started)
-               goto set;
+       if (!priv->upload_when_started) {
+               struct input_dev *dev = priv->dev;
+               union ffpl_control_data data;
+               int ret;
 
-       data.effect = eff->latest;
-       ret = priv->control(dev, s, FFPL_EMP_TO_UPL, data);
-       if (ret)
-               return ret;
-       eff->uploaded_to_device = true;
+               data.effects.cur = &eff->latest;
+               data.effects.old = NULL;
+               ret = priv->control(dev, s, FFPL_EMP_TO_UPL, data);
+               if (ret)
+                       return ret;
+               eff->uploaded_to_device = true;
+       }
 
-set:
        eff->state = FFPL_UPLOADED;
        eff->active = eff->latest;
-       eff->latest = NULL;
        return 0;
 }
 
@@ -124,13 +143,7 @@ static void ffpl_destroy_rq(struct ff_device *ff)
 {      
        struct klgd_plugin *self = ff->private;
        struct klgd_plugin_private *priv = self->private;
-       size_t idx;
-
-       for (idx = 0; idx < priv->effect_count; idx++) {
-               struct ffpl_effect *eff = &priv->effects[idx];
 
-               kfree(eff->latest);
-       }
        kfree(priv->effects);
        kfree(priv);
 }
@@ -178,13 +191,11 @@ static int ffpl_upload_rq(struct input_dev *dev, struct ff_effect *effect, struc
        klgd_lock_plugins(self->plugins_lock);
        spin_lock_irq(&dev->event_lock);
 
-
        /* Copy the new effect to the "latest" slot */
-       kfree(eff->latest);
-       eff->latest = kmemdup(effect, sizeof(struct ff_effect), GFP_KERNEL);
+       eff->latest = *effect;
 
        if (eff->state != FFPL_EMPTY) {
-               if (ffpl_replace_effect(eff->active, eff->latest))
+               if (ffpl_needs_replacing(&eff->active, &eff->latest))
                        eff->replace = true;
                else {
                        eff->replace = false;
@@ -212,7 +223,7 @@ static void ffpl_set_gain_rq(struct input_dev *dev, u16 gain)
        for (idx = 0; idx < priv->effect_count; idx++) {
                struct ffpl_effect *eff = &priv->effects[idx];
 
-               if (ffpl_has_gain(eff->active))
+               if (ffpl_has_gain(&eff->active))
                        eff->change = FFPL_TO_UPDATE;
        }
        /* priv->gain = gain;*/
@@ -229,7 +240,6 @@ static struct klgd_command_stream * ffpl_get_commands(struct klgd_plugin *self,
 {
        struct klgd_plugin_private *priv = self->private;
        struct klgd_command_stream *s;
-       int ret = 0;
        size_t idx;
 
        s = klgd_alloc_stream();
@@ -238,31 +248,88 @@ static struct klgd_command_stream * ffpl_get_commands(struct klgd_plugin *self,
   
        for (idx = 0; idx < priv->effect_count; idx++) {
                struct ffpl_effect *eff = &priv->effects[idx];
-
-               /* Effect has not been touched since the last update, skip it */
-               if (eff->change == FFPL_DONT_TOUCH)
-                       continue;
+               int ret = 0;
 
                /* Latest effect is of different type than currently active effect,
                 * remove it from the device and upload the latest one */
                if (eff->replace) {
-                       switch (eff->state) {
-                       case FFPL_STARTED:
-                               ret = ffpl_stop_effect(priv, s, eff);
-                               if (ret)
+                       switch (eff->change) {
+                       case FFPL_TO_ERASE:
+                               switch (eff->state) {
+                               case FFPL_STARTED:
+                                       ret = ffpl_stop_effect(priv, s, eff);
+                                       if (ret)
+                                               break;
+                               case FFPL_UPLOADED:
+                                       ret = ffpl_erase_effect(priv, s, eff);
+                                       /* TODO: Handle error */
+                               default:
+                                       /* Nothing to do - the effect that is replacing the old effect is about to be erased anyway 
+                                        * State of the effect to be replaced should also never be EMPTY */
                                        break;
-                       case FFPL_UPLOADED:
-                               ret = ffpl_erase_effect(priv, s, eff);
-                               if (ret)
+                               }
+                               break;
+                       case FFPL_TO_UPLOAD:
+                       case FFPL_TO_STOP: /* There is no difference between stopping or uploading an effect when we are replacing it */
+                               switch (eff->state) {
+                               case FFPL_STARTED:
+                                       /* Overwrite the currently active effect and set it to UPLOADED state */
+                                       if (priv->has_owr_to_upl) {
+                                               ret = ffpl_replace_effect(priv, s, eff, FFPL_OWR_TO_UPL);
+                                               if (ret)
+                                                       break;
+                                               continue;
+                                       }
+                                       ret = ffpl_stop_effect(priv, s, eff);
+                                       if (ret)
+                                               break;
+                               case FFPL_UPLOADED:
+                                       ret = ffpl_erase_effect(priv, s, eff);
+                                       if (ret)
+                                               break;
+                               case FFPL_EMPTY: /* State cannot actually be FFPL_EMPTY becuase only uploaded or started effects have to be replaced like this */
+                                       ret = ffpl_upload_effect(priv, s, eff);
                                        break;
+                               }
+                               break;
+                       case FFPL_TO_START:
+                       case FFPL_TO_UPDATE: /* There is no difference between staring or updating an effect when we are replacing it */
+                               switch (eff->state) {
+                               case FFPL_STARTED:
+                                       if (priv->has_owr_to_srt) {
+                                               ret = ffpl_replace_effect(priv, s, eff, FFPL_OWR_TO_SRT);
+                                               if (ret)
+                                                       break;
+                                               continue;
+                                       }
+                                       ret = ffpl_stop_effect(priv, s, eff);
+                                       if (ret)
+                                               break;
+                               case FFPL_UPLOADED:
+                                       ret = ffpl_erase_effect(priv, s, eff);
+                                       if (ret)
+                                               break;
+                               case FFPL_EMPTY: /* State cannot actually be FFPL_EMPTY - same as above applies */
+                                       ret = ffpl_upload_effect(priv, s, eff);
+                                       if (ret)
+                                               break;
+                                       ret = ffpl_start_effect(priv, s, eff);
+                                       break;
+                               }
+                       case FFPL_DONT_TOUCH:
+                               printk(KERN_WARNING "Got FFPL_DONT_TOUCH change for effect that should be replaced - this should not happen!\n");
+                               break;
                        default:
+                               printk(KERN_WARNING "Unhandled state change while replacing effect\n");
                                break;
                        }
-                       eff->replace = false;
-
-                       /* Even the new effect is about to be erased */
-                       if (eff->change == FFPL_TO_ERASE)
+                       if (ret)
+                               printk(KERN_WARNING "Error %d while replacing effect %lu\n", ret, idx);
+                       else {
+                               eff->replace = false;
+                               eff->state = FFPL_DONT_TOUCH;
                                continue;
+                       }
                }
 
                switch (eff->change) {
@@ -281,15 +348,9 @@ static struct klgd_command_stream * ffpl_get_commands(struct klgd_plugin *self,
                case FFPL_TO_START:
                        switch (eff->state) {
                        case FFPL_EMPTY:
-                               if (priv->has_emp_to_srt) {
-                                       ret = ffpl_start_effect(priv, s, eff);
-                                       break;
-                               }
                                ret = ffpl_upload_effect(priv, s, eff);
                                if (ret)
                                        break;
-                               ret = ffpl_start_effect(priv, s, eff);
-                               break;
                        case FFPL_UPLOADED:
                                ret = ffpl_start_effect(priv, s, eff);
                                break;
@@ -326,13 +387,18 @@ static struct klgd_command_stream * ffpl_get_commands(struct klgd_plugin *self,
                        break;
                case FFPL_TO_UPDATE:
                        ret = ffpl_update_effect(priv, s, eff);
+                       break;
+               case FFPL_DONT_TOUCH:
+                       continue;
                default:
                        pr_debug("Unhandled state\n");
                }
 
                /* TODO: Handle errors */
-
-               eff->change = FFPL_DONT_TOUCH;
+               if (ret)
+                       printk(KERN_WARNING "Error %d while processing effect %lu\n", ret, idx);
+               else
+                       eff->change = FFPL_DONT_TOUCH;
        }
 
        return s;
@@ -415,6 +481,12 @@ int ffpl_init_plugin(struct klgd_plugin **plugin, struct input_dev *dev, const s
                ret = -ENOMEM;
                goto err_out2;
        }
+       for (idx = 0; idx < effect_count; idx++) {
+               priv->effects[idx].replace = false;
+               priv->effects[idx].uploaded_to_device = false;
+               priv->effects[idx].state = FFPL_EMPTY;
+               priv->effects[idx].change = FFPL_DONT_TOUCH;
+       }
 
        self->deinit = ffpl_deinit;
        self->get_commands = ffpl_get_commands;
@@ -446,6 +518,14 @@ int ffpl_init_plugin(struct klgd_plugin **plugin, struct input_dev *dev, const s
                priv->erase_when_stopped = true;
                printk("KLGDFF: Using ERASE WHEN STOPPED\n");
        }
+       if (FFPL_REPLACE_UPLOADED & flags) {
+               priv->has_owr_to_upl = true;
+               printk("KLGDFF: Using REPLACE UPLOADED\n");
+       }
+       if (FFPL_REPLACE_STARTED & flags) {
+               priv->has_owr_to_srt = true;
+               printk("KLGDFF: Using REPLACE STARTED\n");
+       }
 
        set_bit(FF_GAIN, dev->ffbit);
        for (idx = 0; idx <= (FF_EFFECT_MAX - FF_EFFECT_MIN); idx++) {
@@ -463,15 +543,20 @@ err_out1:
        return ret;
 }
 
-static bool ffpl_replace_effect(const struct ff_effect *ac_eff, const struct ff_effect *la_eff)
+static bool ffpl_needs_replacing(const struct ff_effect *ac_eff, const struct ff_effect *la_eff)
 {
-       if (ac_eff->type != la_eff->type)
+       if (ac_eff->type != la_eff->type) {
+               printk(KERN_NOTICE "KLGDFF - Effects are of different type - replacing (%d x %d)\n", ac_eff->type, la_eff->type);
                return true;
+       }
 
        if (ac_eff->type == FF_PERIODIC) {
-               if (ac_eff->u.periodic.waveform != la_eff->u.periodic.waveform)
+               if (ac_eff->u.periodic.waveform != la_eff->u.periodic.waveform) {
+                       printk(KERN_NOTICE "KLGDFF - Effects have different waveforms - replacing\n");
                        return true;
+               }
        }
 
+       printk(KERN_NOTICE "KLGDFF - Effect does not have to be replaced, updating\n");
        return false;
 }

diff --git a/klgd_ff_plugin.h b/klgd_ff_plugin.h
index 9a58d52fd9d02fe2717dc5dc4b339eb5ce789574..f07f9bc67daf8ffc3a6bce1150b94d2b55b51521 100644 (file)
--- a/klgd_ff_plugin.h
+++ b/klgd_ff_plugin.h
@@ -8,6 +8,8 @@
 #define FFPL_HAS_SRT_TO_EMP BIT(1) /* Device supports direct "stop and erase" */
 #define FFPL_UPLOAD_WHEN_STARTED BIT(2) /* Upload effects only when they are started - this implies HAS_EMP_TO_SRT */
 #define FFPL_ERASE_WHEN_STOPPED BIT(3) /* Erases effect from device when it is stopped - this implies HAS_SRT_TO_EMP */
+#define FFPL_REPLACE_UPLOADED BIT(4) /* Device can accept a new effect to UPLOADED state without the need to explicitly stop and erase the previously uploaded effect beforehand */
+#define FFPL_REPLACE_STARTED BIT(5) /* Device can accept a new effect to STARTED state without the need to explicitly stop and erase the previously uploaded effect beforehand */
 
 enum ffpl_control_command {
        /* Force feedback state transitions */
@@ -18,11 +20,18 @@ enum ffpl_control_command {
        FFPL_SRT_TO_UDT, /* Update started effect */
        /* Optional force feedback state transitions */
        FFPL_EMP_TO_SRT, /* Upload and start effect */
-       FFPL_SRT_TO_EMP  /* Stop and erase started effect */
+       FFPL_SRT_TO_EMP, /* Stop and erase started effect */
+       FFPL_OWR_TO_UPL, /* Overwrite an effect with a new one and set its state to UPLOADED */
+       FFPL_OWR_TO_SRT  /* Overwrite an effect with a new one and set its state to STARTED */
+};
+
+struct ffpl_effects {
+       const struct ff_effect *cur;  /* Pointer to the effect that is being uploaded/started/stopped/erased */
+       const struct ff_effect *old;  /* Pointer to the currently active effect. Valid only with OWR_* commands, otherwise NULL */
 };
 
 union ffpl_control_data {
-       const struct ff_effect *effect;
+       struct ffpl_effects effects;
        u16 ac_magnitude;
        u16 gain;
 };

diff --git a/klgd_ff_plugin_p.h b/klgd_ff_plugin_p.h
index 0dce9a920fbe6adbb131fd6eb5ba4b1ca130b840..213b964586ab8f81058dbd8fcd3530a27bdac527 100644 (file)
--- a/klgd_ff_plugin_p.h
+++ b/klgd_ff_plugin_p.h
@@ -18,8 +18,8 @@ enum ffpl_state {
 };
 
 struct ffpl_effect {
-       struct ff_effect *active;       /* Last effect submitted to device */
-       struct ff_effect *latest;       /* Last effect submitted to us by userspace */
+       struct ff_effect active;        /* Last effect submitted to device */
+       struct ff_effect latest;        /* Last effect submitted to us by userspace */
        enum ffpl_st_change change;     /* State to which the effect shall be put */
        enum ffpl_state state;          /* State of the active effect */
        bool replace;                   /* Active effect has to be replaced => active effect shall be erased and latest uploaded */
@@ -31,10 +31,13 @@ struct klgd_plugin_private {
        unsigned long supported_effects;
        size_t effect_count;
        struct input_dev *dev;
+       int (*control)(struct input_dev *dev, struct klgd_command_stream *s, const enum ffpl_control_command cmd, const union ffpl_control_data data);
        /* Optional device capabilities */
        bool has_emp_to_srt;
        bool has_srt_to_emp;
        bool upload_when_started;
        bool erase_when_stopped;
-       int (*control)(struct input_dev *dev, struct klgd_command_stream *s, const enum ffpl_control_command cmd, const union ffpl_control_data data);
+       bool has_owr_to_upl;
+       bool has_owr_to_srt;
+       u32 padding:26;
 };

diff --git a/klgdff.c b/klgdff.c
index 1987ba2ecb3f6cd3a189ef061dee225acfab1612..90719ecba15485ccd2ea09e0bebff8b823bf6107 100644 (file)
--- a/klgdff.c
+++ b/klgdff.c
@@ -27,6 +27,34 @@ static int klgdff_erase(struct klgd_command_stream *s, const struct ff_effect *e
        return klgd_append_cmd(s, c);
 }
 
+static int klgdff_owr_start(struct klgd_command_stream *s, const struct ff_effect *effect, const struct ff_effect *old_effect)
+{
+       char *text = kasprintf(GFP_KERNEL, "Overwriting effect to STARTED state, type %d, id %d, old type %d", effect->type, effect->id, old_effect->type);
+       size_t len = strlen(text);
+       struct klgd_command *c = klgd_alloc_cmd(len + 1);
+
+       if (!c)
+               return -ENOMEM;
+
+       memcpy(c->bytes, text, len);
+       kfree(text);
+       return klgd_append_cmd(s, c);
+}
+
+static int klgdff_owr_upload(struct klgd_command_stream *s, const struct ff_effect *effect, const struct ff_effect *old_effect)
+{
+       char *text = kasprintf(GFP_KERNEL, "Overwriting effect to UPLOADED state, type %d, id %d, old type %d", effect->type, effect->id, old_effect->type);
+       size_t len = strlen(text);
+       struct klgd_command *c = klgd_alloc_cmd(len + 1);
+
+       if (!c)
+               return -ENOMEM;
+
+       memcpy(c->bytes, text, len);
+       kfree(text);
+       return klgd_append_cmd(s, c);
+}
+
 static int klgdff_er_stop(struct klgd_command_stream *s, const struct ff_effect *effect)
 {
        char *text = kasprintf(GFP_KERNEL, "Stopping and erasing effect, type %d, id %d", effect->type, effect->id);
@@ -119,37 +147,56 @@ int klgdff_callback(void *data, const struct klgd_command_stream *s)
        for (idx = 0; idx < s->count; idx++)
                printk(KERN_NOTICE "KLGDTM - EFF %s\n", s->commands[idx]->bytes);
 
-       usleep_range(7500, 8500);
+       /* Simulate default USB polling rate of 125 Hz */
+       /*usleep_range(7500, 8500);*/
+       /* Long delay to test more complicated steps */
+       usleep_range(25000, 35000);
 
        return 0;
 }
 
 int klgdff_control(struct input_dev *dev, struct klgd_command_stream *s, const enum ffpl_control_command cmd, const union ffpl_control_data data)
 {
+       if (!s)
+               return -EINVAL;
+
+       if (!data.effects.cur) {
+               printk(KERN_WARNING "KLGDTM - NULL effect, this _cannot_ happen!\n");
+               return -EINVAL;
+       }
+
        switch (cmd) {
        case FFPL_EMP_TO_UPL:
-               return klgdff_upload(s, data.effect);
+               return klgdff_upload(s, data.effects.cur);
                break;
        case FFPL_UPL_TO_SRT:
-               return klgdff_start(s, data.effect);
+               return klgdff_start(s, data.effects.cur);
                break;
        case FFPL_SRT_TO_UPL:
-               return klgdff_stop(s, data.effect);
+               return klgdff_stop(s, data.effects.cur);
                break;
        case FFPL_UPL_TO_EMP:
-               return klgdff_erase(s, data.effect);
+               return klgdff_erase(s, data.effects.cur);
                break;
        case FFPL_SRT_TO_UDT:
-               return klgdff_update(s, data.effect);
+               return klgdff_update(s, data.effects.cur);
                break;
-       /* "Uploadless" commands */
+       /* "Uploadless/eraseless" commands */
        case FFPL_EMP_TO_SRT:
-               return klgdff_up_start(s, data.effect);
+               return klgdff_up_start(s, data.effects.cur);
                break;
        case FFPL_SRT_TO_EMP:
-               return klgdff_er_stop(s, data.effect);
+               return klgdff_er_stop(s, data.effects.cur);
+               break;
+       /* "Direct" replacing commands */
+       case FFPL_OWR_TO_SRT:
+               return klgdff_owr_start(s, data.effects.cur, data.effects.old);
+               break;
+        case FFPL_OWR_TO_UPL:
+               return klgdff_owr_upload(s, data.effects.cur, data.effects.old);
                break;
        default:
+               printk(KERN_NOTICE "KLGDTM - Unhandled command\n");
                break;
        }
 
@@ -201,7 +248,7 @@ static int __init klgdff_init(void)
        input_set_abs_params(dev, ABS_Y, -0x7fff, 0x7fff, 0, 0);
 
        ret = ffpl_init_plugin(&ff_plugin, dev, EFFECT_COUNT, ffbits,
-                              FFPL_ERASE_WHEN_STOPPED,
+                              FFPL_UPLOAD_WHEN_STARTED | FFPL_REPLACE_STARTED,
                               klgdff_control);
        if (ret) {
                printk(KERN_ERR "KLGDFF: Cannot init plugin\n");