Re: Tree dumb questions from an occasional

[Benjamin Tissoires <benjamin.tissoires@redhat.com>]

On Wed, Jun 14, 2023 at 12:18 PM Marco Morandini <marco.morandini@polimi.it> wrote:
>
> > Actually, the one place where it would make sense to have such dynamic
> > quirks is in the hid-core (hid.ko) module itself. It would make sense
> > to have a BUS:VID:PID:QUIRKS parameter.
> > But having such a parameter is not without constraints, because it's
> > not really "dynamic", and we can only set a limited number of quirks.
>
> Ok
>
> >
> > In your particular case, we might as well use an HID-BPF program that
> > tweaks the report descriptor which would force the kernel to "use" the
> > multi-input quirk.
>
> If this means that it could be a nice example (something you would put in
> samples/hid) for HID-BPF, this would be great
> (and I would be curious to understand how to do it).
> But don't waste time for me: the patch is already in your
> for-next and for-6.4/upstream-fixes branches, and for sure I can wait
> and deal with what I have right now.

The program is actually simple: knowing that the kernel splits by
application collection, we can replace the second collection from being
exported as a mouse into a pointer:

See the branch hp_elite_presenter of https://gitlab.freedesktop.org/bentiss/udev-hid-bpf/-/tree/hp_elite_presenter

The program just replaces the byte 79 from 0x02 (mouse) to 0x01
(presenter):

---
SEC("fmod_ret/hid_bpf_rdesc_fixup")
int BPF_PROG(hid_fix_rdesc, struct hid_bpf_ctx *hctx)
{
	__u8 *data = hid_bpf_get_data(hctx, 0 /* offset */, 4096 /* size */);

	if (!data)
		return 0; /* EPERM check */

	/* replace application mouse by application pointer on the second collection */
	if (data[79] == 0x02)
		data[79] = 0x01;

	return 0;
}
---

>
> > Would you mind attaching the output of hid-recorder while you do some
> > HID events and where you show the bug?
>
> Attached; I've tried to use all the mouses/buttons. Do you need the same
> kind of recording taken with the patched kernel?

Nope, no need. hid-recorder is what comes from the device, so the kernel
processing depends on my current tree, which explains how I can test the
various quirks :)

>
> You'll notice that the HID descriptor advertises two mouses and
> two consumer controls, each with different Report IDs.
> This is because this device can be used in two different
> configurations: one is a traditional mouse, that you use on your desk.
> If you turn the device, then you access the second one,
> where you are dealing with a virtual pointer:
> two gyroscopes sense the change of attitude of the device,
> and you can control the cursor by waving around the pointer.
> The buttons that you use in the two different configurations
> are different as well.
>
> > Also, FWIW, the number of MULTI_INPUT quirk required in the kernel is
> > probably a sign that we are not using the best default implementation,
> > and I've already been pinged to change that. I couldn't find the time
> > to get back to this, but your device might also help me in having a
> > broader range of use cases so that we can ditch that quirk once and
> > for all.
>
> I was clumsy looking around trying to understand why it's better
> not to have it as a default, but for sure there are good reason
> for the actual behavior. Perhaps this has to do with the fact that
> you don't want to have duplicated OUTPUTs (if there are any)?
> e.g. https://lkml.iu.edu/hypermail/linux/kernel/0701.1/1132.html ?
>

IIRC it was mostly because some devices were having separate
declarations for their input/output and features, and they were not
especially grouped together. The multi_input quirk splits features/input
/output in different devices, making it harder to configure (in case of
the multitouch ones, where to need to set an operating mode), and the
default was just having one input node for the whole HID device.

Cheers,
Benjamin