Re: [PATCH v4 01/24] input: Add ff-memless-next module

[Michal Malý]

On Wednesday 14 of May 2014 11:05:58 Dmitry Torokhov wrote:
> On Wed, May 14, 2014 at 10:35:25AM +0200, Michal Malý wrote:
> > Hi Dmitry,
> > 
> > thank you for reviewing this.
> > 
> > On Tuesday 13 of May 2014 23:38:06 Dmitry Torokhov wrote:
> > > On Sat, Apr 26, 2014 at 05:02:00PM +0200, Michal Malý wrote:
> > > > +
> > > > +/** DEFINITION OF TERMS
> > > > + *
> > > > + * Combined effect - An effect whose force is a superposition of
> > > > forces
> > > > + *                   generated by all effects that can be added
> > > > together.
> > > > + *                   Only one combined effect can be playing at a
> > > > time.
> > > > + *                   Effects that can be added together to create a
> > > > combined + *                   effect are FF_CONSTANT, FF_PERIODIC and
> > > > FF_RAMP. + * Uncombinable effect - An effect that cannot be combined
> > > > with
> > > > another effect. + *                       All conditional effects -
> > > > FF_DAMPER, FF_FRICTION, + *                       FF_INERTIA and
> > > > FF_SPRING are uncombinable. + *                       Number of
> > > > uncombinable effects playing simultaneously + *
> > > > depends on the capabilities of the hardware. + * Rumble effect - An
> > > > effect generated by device's rumble motors instead of + *
> > > > force feedback actuators.
> > > > + *
> > > > + *
> > > > + * HANDLING OF UNCOMBINABLE EFFECTS
> > > > + *
> > > > + * Uncombinable effects cannot be combined together into just one
> > > > effect,
> > > > at + * least not in a clear and obvious manner. Therefore these
> > > > effects
> > > > have to + * be handled individually by ff-memless-next. Handling of
> > > > these
> > > > effects is + * left entirely to the hardware-specific driver,
> > > > ff-memless-next merely + * passes these effects to the
> > > > hardware-specific
> > > > driver at appropriate time. + * ff-memless-next provides the UPLOAD
> > > > command to notify the hardware-specific + * driver that the userspace
> > > > is
> > > > about to request playback of an uncombinable + * effect. The
> > > > hardware-specific driver shall take all steps needed to make + * the
> > > > device ready to play the effect when it receives the UPLOAD command. +
> > > > *
> > > > The actual playback shall commence when START_UNCOMB command is
> > > > received.
> > > > + * Opposite to the UPLOAD command is the ERASE command which tells +
> > > > *
> > > > the hardware-specific driver that the playback has finished and that +
> > > > *
> > > > the effect will not be restarted. STOP_UNCOMB command tells
> > > > + * the hardware-specific driver that the playback shall stop but the
> > > > device + * shall still be ready to resume the playback immediately.
> > > > + *
> > > > + * In case it is not possible to make the device ready to play an
> > > > uncombinable + * effect (all hardware effect slots are occupied), the
> > > > hardware-specific + * driver may return an error when it receives an
> > > > UPLOAD command. If the
> > > 
> > > This part concerns me. It seems to me that devices supporting
> > > "uncombinable" effects are in fact not memoryless devices and we should
> > > not be introducing this term here. If the goal is to work around limited
> > > number of effect slots in the devices by combining certain effects then
> > > it needs to be done at ff-core level as it will be potentially useful
> > > for all devices.
> > 
> > Force generated by a conditional effect (referred to as "uncombinable"
> > within ff-memless-next to make the distinction clear) depends on a
> > position of the device. For instance the more a device is deflected from
> > a neutral position the greater force FF_SPRING generates. A truly
> > memoryless device would have to report its position to the driver, have
> > it calculate the appropriate force and send it back to the device. IMHO
> > such a loop would require a very high USB polling rate to play
> > conditional effects with acceptable quality.
> > 
> > We know for a fact that at least many (all?) Logitech devices that support
> > conditional effects use this "semi-memoryless" approach where FF_CONSTANT
> > and FF_PERIODIC are handled in the memoryless fashion and conditional
> > effects are uploaded to the device (in a somewhat simplified form). The
> > amount of effects that can be uploaded to a device is limited which is
> > why ff-memless-next uses two steps (UPLOAD/ERASE and START/STOP) to
> > handle these effects.
> > 
> > Conditional effects - even if they are of the same type - cannot be
> > effectively combined into one because superposition doesn't seem to work
> > here so they have to be processed one by one.
> > 
> > If we ever come across a really memoryless device it should not be
> > particularly difficult to add another callback to ff-memless-next which
> > would emulate conditional effects with constant force.
> 
> Thank you for the explanation. This further solidifies for me the idea
> that handling of such effects that are in fact uploaded to and managed
> by the device should not be handled by the memoryless core but rather by
> the driver itself. I.e. such drivers should implement their own play(),
> upload(), erase(), etc, and decide whether to use a hardware slot for
> the effect or handle effect in memoryless fashion (if possible). We can
> open ff-memless to allow such drivers to use parts of memoryless
> handling.
> 
> Thanks.

To bring this to a conclusion we could go from, would this be an acceptable 
solution?

- Have the HW-specific driver talk directly to ff-core and reimplement upload(), 
play(), etc.
- Rewrite "ff-memless-next" so that it is not a self-contained module but a 
library of functions.
- Have the driver either:
  - Upload an effect to a device directly if the device can fully manage the 
effect by itself.
  - Use provided timing functions to know when an effect should start, stop, 
restart etc...
  - Use provided timing AND processing functions to combine effects that can be 
combined into one, calculate periodic waveforms etc?

I have no problem with throwing my current approach away but before I start 
working on a new one I'd like to know which way to go...

Thanks,
Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/