[PATCH] Input: Add direction to ff-memless

[PATCH] Input: Add direction to ff-memless

[Jari Vanhala]

This adds simple direction calculation to effect
combine. It's useful to decide motor direction for
rumble (vibrator).

Signed-off-by: Jari Vanhala <ext-jari.vanhala@nokia.com>
---
 drivers/input/ff-memless.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c
index 2d1415e..9af009a 100644
--- a/drivers/input/ff-memless.c
+++ b/drivers/input/ff-memless.c
@@ -221,6 +221,14 @@ static int get_compatible_type(struct ff_device *ff, int effect_type)
 	return 0;
 }
 
+static unsigned int ml_calculate_direction(
+	unsigned int direction, unsigned int force,
+	unsigned int new_direction, unsigned int new_force)
+{
+	return ((u32)direction * force + new_direction * new_force) /
+		(force + new_force);
+}
+
 /*
  * Combine two effects and apply gain.
  */
@@ -255,6 +263,19 @@ static void ml_combine_effects(struct ff_effect *effect,
 	case FF_RUMBLE:
 		strong = new->u.rumble.strong_magnitude * gain / 0xffff;
 		weak = new->u.rumble.weak_magnitude * gain / 0xffff;
+
+		if (effect->u.rumble.strong_magnitude + strong)
+			effect->direction = ml_calculate_direction(
+				effect->direction,
+				effect->u.rumble.strong_magnitude,
+				new->direction, strong);
+		else if (effect->u.rumble.weak_magnitude + weak)
+			effect->direction = ml_calculate_direction(
+				effect->direction,
+				effect->u.rumble.weak_magnitude,
+				new->direction, weak);
+		else
+			effect->direction = 0;
 		effect->u.rumble.strong_magnitude =
 			min(strong + effect->u.rumble.strong_magnitude,
 			    0xffffU);
@@ -269,6 +290,13 @@ static void ml_combine_effects(struct ff_effect *effect,
 		/* here we also scale it 0x7fff => 0xffff */
 		i = i * gain / 0x7fff;
 
+		if (effect->u.rumble.strong_magnitude + i)
+			effect->direction = ml_calculate_direction(
+				effect->direction,
+				effect->u.rumble.strong_magnitude,
+				new->direction, i);
+		else
+			effect->direction = 0;
 		effect->u.rumble.strong_magnitude =
 			min(i + effect->u.rumble.strong_magnitude, 0xffffU);
 		effect->u.rumble.weak_magnitude =
-- 
1.6.3.3

[PATCH] Input: Force Feedback core memory leak

[Jari Vanhala]

Effects is allocated, but not freed anywhere.

Signed-off-by: Jari Vanhala <ext-jari.vanhala@nokia.com>
---
 drivers/input/ff-core.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/input/ff-core.c b/drivers/input/ff-core.c
index 3d7816c..d310a97 100644
--- a/drivers/input/ff-core.c
+++ b/drivers/input/ff-core.c
@@ -367,6 +367,7 @@ void input_ff_destroy(struct input_dev *dev)
 		if (dev->ff->destroy)
 			dev->ff->destroy(dev->ff);
 		kfree(dev->ff->private);
+		kfree(dev->ff->effects);
 		kfree(dev->ff);
 		dev->ff = NULL;
 	}
-- 
1.6.3.3

Re: [PATCH] Input: Force Feedback core memory leak

[Dmitry Torokhov]

On Fri, Dec 04, 2009 at 03:05:14PM +0200, Jari Vanhala wrote:
> Effects is allocated, but not freed anywhere.

Applied, thank you Jari.

-- 
Dmitry

Re: [PATCH] Input: Add direction to ff-memless

[Anssi Hannula]

Jari Vanhala wrote:
> This adds simple direction calculation to effect
> combine. It's useful to decide motor direction for
> rumble (vibrator).

Interesting. None of the existing rumble drivers use the direction
field. I assume your device has two directions for the motor?

I'm somewhat suspicious if the motor direction matters at all, but I
guess it could.

> Signed-off-by: Jari Vanhala <ext-jari.vanhala@nokia.com>
> ---
>  drivers/input/ff-memless.c |   28 ++++++++++++++++++++++++++++
>  1 files changed, 28 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c
> index 2d1415e..9af009a 100644
> --- a/drivers/input/ff-memless.c
> +++ b/drivers/input/ff-memless.c
> @@ -221,6 +221,14 @@ static int get_compatible_type(struct ff_device *ff, int effect_type)
>  	return 0;
>  }
>  
> +static unsigned int ml_calculate_direction(
> +	unsigned int direction, unsigned int force,
> +	unsigned int new_direction, unsigned int new_force)
> +{
> +	return ((u32)direction * force + new_direction * new_force) /
> +		(force + new_force);
> +}

What if direction is 0xf000 and new_direction is 0x1000? The correct
behaviour would be to use direction 0x0000 or so, but this code would
return 0x8000 (assuming equal forces).

>  /*
>   * Combine two effects and apply gain.
>   */
> @@ -255,6 +263,19 @@ static void ml_combine_effects(struct ff_effect *effect,
>  	case FF_RUMBLE:
>  		strong = new->u.rumble.strong_magnitude * gain / 0xffff;
>  		weak = new->u.rumble.weak_magnitude * gain / 0xffff;
> +
> +		if (effect->u.rumble.strong_magnitude + strong)
> +			effect->direction = ml_calculate_direction(
> +				effect->direction,
> +				effect->u.rumble.strong_magnitude,
> +				new->direction, strong);
> +		else if (effect->u.rumble.weak_magnitude + weak)
> +			effect->direction = ml_calculate_direction(
> +				effect->direction,
> +				effect->u.rumble.weak_magnitude,
> +				new->direction, weak);
> +		else
> +			effect->direction = 0;
>  		effect->u.rumble.strong_magnitude =
>  			min(strong + effect->u.rumble.strong_magnitude,
>  			    0xffffU);

Looks ok.

We could also additionally carry the effect of directions into magnitude
when summing the force vectors, so that two equal-magnitude rumbles with
opposite directions would cancel themselves out. I'd be slightly against
that, though, as rumble doesn't really have clear directions anyway.
(just mentioning this in case someone thinks we should do that)

-- 
Anssi Hannula

Re: [PATCH] Input: Add direction to ff-memless

[Jari Vanhala]

On Mon, 2009-12-07 at 22:16 +0100, ext Anssi Hannula wrote:
> Jari Vanhala wrote:
> > This adds simple direction calculation to effect
> > combine. It's useful to decide motor direction for
> > rumble (vibrator).
> 
> Interesting. None of the existing rumble drivers use the direction
> field. I assume your device has two directions for the motor?

Motors can be driven forward and reverse.

> I'm somewhat suspicious if the motor direction matters at all, but I
> guess it could.

They feel a little different and chancing direction makes it briefly
stop. In one effect it's not that clear, but having effects with
different direction (in sequence) it's notable.

> > +static unsigned int ml_calculate_direction(
> > +	unsigned int direction, unsigned int force,
> > +	unsigned int new_direction, unsigned int new_force)
> > +{
> > +	return ((u32)direction * force + new_direction * new_force) /
> > +		(force + new_force);
> > +}
> 
> What if direction is 0xf000 and new_direction is 0x1000? The correct
> behaviour would be to use direction 0x0000 or so, but this code would
> return 0x8000 (assuming equal forces).

I know it's not perfect, but it's simple and gives good enough result.
And I just need to know which side of 0x8000 it is.

> We could also additionally carry the effect of directions into magnitude
> when summing the force vectors, so that two equal-magnitude rumbles with
> opposite directions would cancel themselves out. I'd be slightly against
> that, though, as rumble doesn't really have clear directions anyway.
> (just mentioning this in case someone thinks we should do that)

I also thought that, but magnitude is felt much more that direction. If
that direction used would be more than forward/reverse, it could make
sense.

	++Jam

Re: [PATCH] Input: Add direction to ff-memless

[Anssi Hannula]

Jari Vanhala wrote:
> On Mon, 2009-12-07 at 22:16 +0100, ext Anssi Hannula wrote:
>> Jari Vanhala wrote:
>>> +static unsigned int ml_calculate_direction(
>>> +	unsigned int direction, unsigned int force,
>>> +	unsigned int new_direction, unsigned int new_force)
>>> +{
>>> +	return ((u32)direction * force + new_direction * new_force) /
>>> +		(force + new_force);
>>> +}
>> What if direction is 0xf000 and new_direction is 0x1000? The correct
>> behaviour would be to use direction 0x0000 or so, but this code would
>> return 0x8000 (assuming equal forces).
> 
> I know it's not perfect, but it's simple and gives good enough result.
> And I just need to know which side of 0x8000 it is.

Ah, I assumed the motor directions (forward, reverse) were mapped to
up/down (180/0), not left/right.

Your way seems somewhat counter-intuitive, but it indeed makes
ml_calculate_direction() simpler :) And as there are no other rumble
drivers using the direction field, I guess the wrong results do not
matter, then.

I think a comment should be added in ml_calculate_direction(), anyway,
to make it clear that it only works correctly with motors with
left/right directions only.

-- 
Anssi Hannula

[PATCH v2] Input: Add direction to ff-memless

[Jari Vanhala]

This adds simple direction calculation to effect
combine. It's useful to decide motor direction for
rumble (vibrator).

Signed-off-by: Jari Vanhala <ext-jari.vanhala@nokia.com>
---
 drivers/input/ff-memless.c |   36 ++++++++++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c
index b483b29..5914a7d 100644
--- a/drivers/input/ff-memless.c
+++ b/drivers/input/ff-memless.c
@@ -221,6 +221,22 @@ static int get_compatible_type(struct ff_device *ff, int effect_type)
 }
 
 /*
+ * Only left/right direction should be used (under/over 0x8000) for
+ * forward/reverse motor direction (to keep calculation fast & simple).
+ */
+static u16 ml_calculate_direction(u16 direction, u16 force,
+				  u16 new_direction, u16 new_force)
+{
+	if (!force)
+		return new_direction;
+	if (!new_force)
+		return direction;
+	return (((u32)(direction >> 1) * force +
+		 (new_direction >> 1) * new_force) /
+		(force + new_force)) << 1;
+}
+
+/*
  * Combine two effects and apply gain.
  */
 static void ml_combine_effects(struct ff_effect *effect,
@@ -254,6 +270,19 @@ static void ml_combine_effects(struct ff_effect *effect,
 	case FF_RUMBLE:
 		strong = new->u.rumble.strong_magnitude * gain / 0xffff;
 		weak = new->u.rumble.weak_magnitude * gain / 0xffff;
+
+		if (effect->u.rumble.strong_magnitude + strong)
+			effect->direction = ml_calculate_direction(
+				effect->direction,
+				effect->u.rumble.strong_magnitude,
+				new->direction, strong);
+		else if (effect->u.rumble.weak_magnitude + weak)
+			effect->direction = ml_calculate_direction(
+				effect->direction,
+				effect->u.rumble.weak_magnitude,
+				new->direction, weak);
+		else
+			effect->direction = 0;
 		effect->u.rumble.strong_magnitude =
 			min(strong + effect->u.rumble.strong_magnitude,
 			    0xffffU);
@@ -268,6 +297,13 @@ static void ml_combine_effects(struct ff_effect *effect,
 		/* here we also scale it 0x7fff => 0xffff */
 		i = i * gain / 0x7fff;
 
+		if (effect->u.rumble.strong_magnitude + i)
+			effect->direction = ml_calculate_direction(
+				effect->direction,
+				effect->u.rumble.strong_magnitude,
+				new->direction, i);
+		else
+			effect->direction = 0;
 		effect->u.rumble.strong_magnitude =
 			min(i + effect->u.rumble.strong_magnitude, 0xffffU);
 		effect->u.rumble.weak_magnitude =
-- 
1.6.3.3

Re: [PATCH v2] Input: Add direction to ff-memless

[Anssi Hannula]

On 23.12.2009 14:58, Jari Vanhala wrote:
> This adds simple direction calculation to effect
> combine. It's useful to decide motor direction for
> rumble (vibrator).
> 
> Signed-off-by: Jari Vanhala <ext-jari.vanhala@nokia.com>
> ---
>  drivers/input/ff-memless.c |   36 ++++++++++++++++++++++++++++++++++++
>  1 files changed, 36 insertions(+), 0 deletions(-)

Acked-by: Anssi Hannula <anssi.hannula@iki.fi>

-- 
Anssi Hannula