Page MenuHomeFreeBSD

Add absolute positioning support to ums
Needs ReviewPublic

Authored by vi_endrift.com on Feb 16 2017, 12:05 AM.

Details

Reviewers
grehan
hselasky
Summary

This adds support for using absolute positioning in ums in conjunction with evdev. Due to the design of the legacy sysmouse interface, it cannot use absolute positioning, so when any axis is absolute, sysmouse does not receive events.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

vi_endrift.com retitled this revision from to Add absolute positioning support to ums.
vi_endrift.com updated this object.
vi_endrift.com edited the test plan for this revision. (Show Details)
vi_endrift.com added reviewers: grehan, hselasky.
vi_endrift.com set the repository for this revision to rS FreeBSD src repository - subversion.

This looks very promising. Give me a day or two to review.

sys/dev/usb/input/ums.c
539

add "default:" and "break;" and "/* FALLTHROUGH */" to the sc_flags checks.

vi_endrift.com edited edge metadata.

Updated per comments

Do you have sample of description of absolute device obtained on Linux with evemu-record or similar tool?

sys/dev/usb/input/ums.c
784

Are there any reasons to allocate fifo in absolute mode?

798

What is the type of your absolute device? For touchscreens INPUT_PROP_DIRECT should be set, not INPUT_PROP_POINTER

802

I think, no need to set EV_REL when no relative axes are used

807

You should parse HID descriptor for logical reporting range and physical resolution and advertise it here

1016

You can use

evdev_push_abs(sc->sc_evdev, ABS_X, dx);

here to be consistent with relative case

1026

-dz ?

I don't have a sample of the device, I was testing using bhyve's "tablet" device, which I am told works as-is on other OSes.

sys/dev/usb/input/ums.c
784

This is not my code, so I have no idea. Changes to this should be done in a different commit.

802

There may still be some relative axes present. I can check.

807

Yep, I was basing this change off of another driver that had a TODO there. I can actually parse it out though.

1016

I did not think that exists? But I just checked and it does.

1026

Noted, thanks.

I'll make the other changes in the morning and submit a revised patch after testing.

sys/dev/usb/input/ums.c
802

Actually, the code as-is always pushes a relative event for HWHEEL, assuming HWHEEL is present. It's probably possible to avoid setting EV_REL if not needed, but it might just add unneeded complexity.

sys/dev/usb/input/ums.c
784

usb_fifo_attach() here creates /dev/umsX character device which is silent in absolute mode according to proposed changes

802

the code as-is always pushes a relative event for HWHEEL, assuming HWHEEL is present

No. HWHEEL events are filtered out at evdev layer if HWHEEL has not been advertised at device registration. evdev_push is called unconditionaly here just to skip extra useless checking

but it might just add unneeded complexity.

I recommend to compare evdev description with Linux-derived one and set exactly the same properties. In several cases e.g. xf86-input-evdev software uses libmagic-alike routines to detect device types and quirks and setting different device description can bring incompatibilities.

Yep, I was basing this change off of another driver that had a TODO there. I can actually parse it out though.

Following is logical reporting range and physical resolution parsing code from one of my drivers

Helper function (I think it should be placed somewhere in uhid library to reuse):

/*
 * calculate HID item resolution. unit/mm for distances, unit/rad for angles
 */
static int32_t
hid_item_resolution(struct hid_item *hi)
{
        /*
         * hid unit scaling table according to HID Usage Table Review
         * Request 39 Tbl 17 http://www.usb.org/developers/hidpage/HUTRR39b.pdf
         */
        static const int64_t scale[][2] = {
            { 2, 2 },           /* 0x00 */
            { 2, 20 },          /* 0x01 */
            { 2, 200 },         /* 0x02 */
            { 2, 2000 },        /* 0x03 */
            { 2, 20000 },       /* 0x04 */
            { 2, 200000 },      /* 0x05 */
            { 2, 2000000 },     /* 0x06 */
            { 2, 20000000 },    /* 0x07 */
            { 100000000, 1 },   /* 0x08 */
            { 10000000, 1 },    /* 0x09 */
            { 1000000, 1 },     /* 0x0A */
            { 100000, 1 },      /* 0x0B */
            { 10000, 1 },       /* 0x0C */
            { 1000, 1 },        /* 0x0D */
            { 100, 1 },         /* 0x0E */
            { 10, 1 },          /* 0x0F */
        };
        int64_t logical_size, physical_size, resolution;
        int64_t multiplier = 1, divisor = 1;

#define HIUU_CENTIMETER 0x11
#define HIUU_RADIANS    0x12
#define HIUU_INCHES     0x13
#define HIUU_DEGREES    0x14
        switch (hi->unit) {
        case HIUU_CENTIMETER:
                divisor = 10;
                break;
        case HIUU_INCHES:
                multiplier = 10;
                divisor = 254;
                break;
        case HIUU_RADIANS:
                break;
        case HIUU_DEGREES:
                multiplier = 573;
                divisor = 10;
                break;
        default:
                return (0);
        }

        if ((hi->logical_maximum <= hi->logical_minimum) ||
            (hi->physical_maximum <= hi->physical_minimum) ||
            (hi->unit_exponent < 0) || (hi->unit_exponent >= nitems(scale)))
                return (0);

        logical_size = (int64_t)hi->logical_maximum -
            (int64_t)hi->logical_minimum;
        physical_size = (int64_t)hi->physical_maximum -
            (int64_t)hi->physical_minimum;
        /* Round to nearest */
        resolution = ((multiplier * scale[hi->unit_exponent][0] / 2) +
            logical_size * multiplier * scale[hi->unit_exponent][0]) /
            (physical_size * divisor * scale[hi->unit_exponent][1]);

        if (resolution > INT32_MAX)
                return (0);

        return (resolution);
}

actual parsing looks like:

struct hid_item hi;
hid_start_parse();
while (hid_get_item(hd, &hi)) {
....
evdev_support_abs(sc->evdev, event_code, 0, hi.logical_minimum, hi.logical_maximum, 0, 0, hid_item_resolution(&hi));
....
}
hid_end_parse()

What is the status of this work? This feature remains desirable.

FYI from 13.0R:


Index: sys/dev/usb/input/ums.c
===================================================================
--- sys/dev/usb/input/ums.c
+++ sys/dev/usb/input/ums.c

Patching file sys/dev/usb/input/ums.c using Plan A...
Hunk #1 succeeded at 122 (offset 4 lines).
Hunk #2 succeeded at 232 (offset 4 lines).
Hunk #3 failed at 282.
Hunk #4 succeeded at 318 (offset 4 lines).
Hunk #5 succeeded at 332 with fuzz 1 (offset 4 lines).
Hunk #6 failed at 356.
Hunk #7 succeeded at 371 (offset -1 lines).
Hunk #8 succeeded at 409 with fuzz 1 (offset 2 lines).
Hunk #9 failed at 515.
Hunk #10 succeeded at 540 (offset -5 lines).
Hunk #11 failed at 567.
Hunk #12 succeeded at 633 (offset -2 lines).
Hunk #13 succeeded at 780 (offset -6 lines).
Hunk #14 succeeded at 795 (offset -2 lines).
Hunk #15 succeeded at 1005 (offset -7 lines).
4 out of 15 hunks failed while patching sys/dev/usb/input/ums.c
done

Could you rebase this patch on top of 14-current / main?

I will have a look at it again.

Thanks for the reminder.

--HPS

This revision is obsolete and should be abandoned. Absolute positioning is supported by hms(4) on FreeBSD 13+