Re: [RESEND] Input: support pre-stored effects

[James Ogletree]

> On Jun 12, 2023, at 8:25 PM, Jeff LaBundy <jeff@xxxxxxxxxxx> wrote:
> 
> Hi James,
> 
> On Mon, Jun 12, 2023 at 07:43:57PM +0000, James Ogletree wrote:
>> At present, the best way to define effects that
>> pre-exist in device memory is by utilizing
>> the custom_data field, which it was not intended
>> for, and requires arbitrary interpretation by
>> the driver to make meaningful.
>> 
>> Provide option for defining pre-stored effects in
>> device memory.
>> 
>> Signed-off-by: James Ogletree <james.ogletree@xxxxxxxxxx>
>> ---
>> include/uapi/linux/input.h | 32 ++++++++++++++++++++++----------
>> 1 file changed, 22 insertions(+), 10 deletions(-)
>> 
>> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
>> index 2557eb7b0561..689e5fa10647 100644
>> --- a/include/uapi/linux/input.h
>> +++ b/include/uapi/linux/input.h
>> @@ -428,17 +428,27 @@ struct ff_rumble_effect {
>> __u16 weak_magnitude;
>> };
>> 
>> +/**
>> + * struct ff_prestored_effect - defines parameters of a pre-stored force-feedback effect
>> + * @index: index of effect
>> + * @bank: memory bank of effect
>> + */
>> +struct ff_prestored_effect {
>> + __u16 index;
>> + __u16 bank;
>> +};
> 
> This seems like a good start; I do wonder if it's useful to review recent
> customer vibrator HAL implementations and decide whether you want to pack
> any additional members here such as magnitude, etc. as is done for periodic
> effects?
> 
> Back in L25 days, some customers would assign click or tap effects to one
> or more entries in the wavetable and then use a separate digital volume
> control (at that time exposed through sysfs) to create a few discrete
> amplitude levels. Perhaps it would be handy to bundle this information as
> part of the same call?
> 
> It's just a suggestion; I'll defer to your much more recent expertise.
> 

My thinking is that ff_prestored_effect ought to be for effects being used
“off-the-shelf”, and in such cases it would seem appropriate to defer to
firmware for the effect design. I think this fits nicely as-is with the other
structures as it serves a clear and distinct use-case. Otherwise one might
just add these two members to ff_periodic_effect (or every kind of effect).

I think the current predominant method for setting "magnitude" for these
pre-stored effects is by using the FF_GAIN event code as a separate write
call, so I think adding a magnitude member would go unused, if I understand
you correctly.

Thanks,
James