[PATCH 1/1] input: ff-memless: Change FF_ENVELOPE_INTERVAL to a module parameter.

[PATCH 1/1] input: ff-memless: Change FF_ENVELOPE_INTERVAL to a module parameter.

[Kalle Jokiniemi]

Sometimes you need to have tighter control over the ff-memless
effects. E.g. when creating short button / VKB press effects,
the effect duration is typically <= 40ms, and you want to have
a short acceleration period in beginning and fade out the end
to avoid "electric tooth brush" effect. With 50ms envelope
interval this control is not possible.

To allow this control without patching over ff-memless, change the
FF_ENVELOPE_INTERVAL macro to a module parameter that can be modified
via kernel command line or during runtime from
/sys/module/ff_memless/parameters/ff_envelope_interval sysfs file.

Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@jolla.com>
---
 drivers/input/ff-memless.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c
index fcc6c33..058e4cb 100644
--- a/drivers/input/ff-memless.c
+++ b/drivers/input/ff-memless.c
@@ -41,7 +41,8 @@ MODULE_DESCRIPTION("Force feedback support for memoryless devices");
 #define FF_MEMLESS_EFFECTS	16
 
 /* Envelope update interval in ms */
-#define FF_ENVELOPE_INTERVAL	50
+static int ff_envelope_interval = 50;
+module_param(ff_envelope_interval, int, S_IWUSR | S_IRUGO);
 
 #define FF_EFFECT_STARTED	0
 #define FF_EFFECT_PLAYING	1
@@ -96,7 +97,7 @@ static unsigned long calculate_next_time(struct ml_effect_state *state)
 			msecs_to_jiffies(envelope->attack_length);
 		if (time_before(state->adj_at, attack_stop))
 			return state->adj_at +
-					msecs_to_jiffies(FF_ENVELOPE_INTERVAL);
+					msecs_to_jiffies(ff_envelope_interval);
 	}
 
 	if (state->effect->replay.length) {
@@ -110,7 +111,7 @@ static unsigned long calculate_next_time(struct ml_effect_state *state)
 
 			/* already fading, advance to next checkpoint */
 			next_fade = state->adj_at +
-					msecs_to_jiffies(FF_ENVELOPE_INTERVAL);
+					msecs_to_jiffies(ff_envelope_interval);
 			if (time_before(next_fade, state->stop_at))
 				return next_fade;
 		}
-- 
1.9.1

Re: [PATCH 1/1] input: ff-memless: Change FF_ENVELOPE_INTERVAL to a module parameter.

[Dmitry Torokhov]

Hi Kalle,

On Fri, Aug 28, 2015 at 02:56:33PM +0300, Kalle Jokiniemi wrote:
> Sometimes you need to have tighter control over the ff-memless
> effects. E.g. when creating short button / VKB press effects,
> the effect duration is typically <= 40ms, and you want to have
> a short acceleration period in beginning and fade out the end
> to avoid "electric tooth brush" effect. With 50ms envelope
> interval this control is not possible.
> 
> To allow this control without patching over ff-memless, change the
> FF_ENVELOPE_INTERVAL macro to a module parameter that can be modified
> via kernel command line or during runtime from
> /sys/module/ff_memless/parameters/ff_envelope_interval sysfs file.

How would users know that they need to change this parameter? Could we
maybe adjust sampling time dynamically, based on the attack length of
the envelope?

Thanks.

-- 
Dmitry

Re: [PATCH 1/1] input: ff-memless: Change FF_ENVELOPE_INTERVAL to a module parameter.

[Kalle Jokiniemi]

Hi,

On 29.08.2015 03:29, Dmitry Torokhov wrote:
> Hi Kalle,
>
> On Fri, Aug 28, 2015 at 02:56:33PM +0300, Kalle Jokiniemi wrote:
>> Sometimes you need to have tighter control over the ff-memless
>> effects. E.g. when creating short button / VKB press effects,
>> the effect duration is typically <= 40ms, and you want to have
>> a short acceleration period in beginning and fade out the end
>> to avoid "electric tooth brush" effect. With 50ms envelope
>> interval this control is not possible.
>>
>> To allow this control without patching over ff-memless, change the
>> FF_ENVELOPE_INTERVAL macro to a module parameter that can be modified
>> via kernel command line or during runtime from
>> /sys/module/ff_memless/parameters/ff_envelope_interval sysfs file.
>
> How would users know that they need to change this parameter? Could we
> maybe adjust sampling time dynamically, based on the attack length of
> the envelope?

Well, I'm looking after some parts of the Sailfish OS haptics / vibrator 
parts, and consider myself a user. For haptic effects (e.g. the VKB 
example above), I know I need as short as I can get to be able to tune 
those effects to work best with the Hardware I'm working with. Then for 
long alarm vibrations, the interval doesn't matter that much. But 
typically I would just set it to the smallest I can get with HZ setting 
the kernel has.

I've been thinking the "user" for this setting more as a system 
administrator / OS maintainer / device maker setting. But your idea 
could maybe work. Top of my head the downside would be added complexity, 
and slight "unpredictability" of the function if the timing changes 
dynamically. Would we then again need a user space setting for the 
threshold where to start using longer timers?

BTW, how would you feel about hr timers instead of jiffy based timing in 
ff-memless? That is a bit of a bottle neck for me (I see some jitter on 
the first event and typical 100Hz kernel only gives 10ms control interval).

- Kalle

>
> Thanks.
>

Re: [PATCH 1/1] input: ff-memless: Change FF_ENVELOPE_INTERVAL to a module parameter.

[Dmitry Torokhov]

On Mon, Aug 31, 2015 at 11:47:06AM +0300, Kalle Jokiniemi wrote:
> Hi,
> 
> On 29.08.2015 03:29, Dmitry Torokhov wrote:
> >Hi Kalle,
> >
> >On Fri, Aug 28, 2015 at 02:56:33PM +0300, Kalle Jokiniemi wrote:
> >>Sometimes you need to have tighter control over the ff-memless
> >>effects. E.g. when creating short button / VKB press effects,
> >>the effect duration is typically <= 40ms, and you want to have
> >>a short acceleration period in beginning and fade out the end
> >>to avoid "electric tooth brush" effect. With 50ms envelope
> >>interval this control is not possible.
> >>
> >>To allow this control without patching over ff-memless, change the
> >>FF_ENVELOPE_INTERVAL macro to a module parameter that can be modified
> >>via kernel command line or during runtime from
> >>/sys/module/ff_memless/parameters/ff_envelope_interval sysfs file.
> >
> >How would users know that they need to change this parameter? Could we
> >maybe adjust sampling time dynamically, based on the attack length of
> >the envelope?
> 
> Well, I'm looking after some parts of the Sailfish OS haptics /
> vibrator parts, and consider myself a user. For haptic effects (e.g.
> the VKB example above), I know I need as short as I can get to be
> able to tune those effects to work best with the Hardware I'm
> working with. Then for long alarm vibrations, the interval doesn't
> matter that much. But typically I would just set it to the smallest
> I can get with HZ setting the kernel has.
> 
> I've been thinking the "user" for this setting more as a system
> administrator / OS maintainer / device maker setting. But your idea
> could maybe work. Top of my head the downside would be added
> complexity, and slight "unpredictability" of the function if the
> timing changes dynamically. Would we then again need a user space
> setting for the threshold where to start using longer timers?

I'd say keep the interval between [5,50] msecs and scale it that you
have at least N points for the attack duration. I.e if you decide that
you want at least 8 points you do:

	interval = attack_duration / 8;
	interval = clamp_val(interval, 5, 50);

> 
> BTW, how would you feel about hr timers instead of jiffy based
> timing in ff-memless? That is a bit of a bottle neck for me (I see
> some jitter on the first event and typical 100Hz kernel only gives
> 10ms control interval).

I do not have objections to doing this.

Thanks.

-- 
Dmitry