Re: [DIGImend-devel] PT-1001 digitizer
Brought to you by:
spb_nick
Menu
▾
▴
Re: [DIGImend-devel] PT-1001 digitizer
|
From: Nikolai K. <sp...@gm...> - 2015-02-22 12:22:03
|
On 02/19/2015 06:19 PM, Yann Vernier wrote: > On Sat, 31 Jan 2015 12:35:28 +0200 > Nikolai Kondrashov <sp...@gm...> wrote: >> I'm not sure how repeated report ID items in a single collection even work. >> Also duplicate report IDs sound fishy. All-in-all, this is some really weird >> report descriptor and would be difficult to reason about. I would start from >> scratch, taking one of the fixed report descriptors from other tablets and >> adjusting it to the bit fields this one reports. > > I think it's some sort of indirect description, applying resolution multipliers > to the preexisting report 1, so as not to confuse the boot protocol interpretation. I have my doubts about this. First, boot protocol doesn't involve parsing the report descriptor. Then, the device is unlikely to actually need a resolution multiplier. What for? > However, I have not gotten the device to send the multiplier reports, and Linux > seems to happily ignore them. That's probably just some imagination/engineering gone wild and never implemented. >>> /* I expect patching in these values (from interface 2) >>> would get Xorg to use tablet mode: */ >>> 0x09, 0x42, /* Usage (Tip Switch), */ >>> 0x09, 0x44, /* Usage (Barrel Switch), */ >>> 0x09, 0x45, /* Usage (Eraser), */ /* Really on the barrel too */ >> >> I suggest to use "Tablet Pick" here. I added it to the kernel and use it for >> the second barrel button on all of the tablets. It will map to the third >> button. > > Certainly. Since this is a patch driver for Linux anyway, convention there takes > priority. I don't know if other tablets also abuse eraser like this. Yes, quite a lot of them do that. >>> /* The original usages should have mapped to mouse buttons, but Linux >>> happily reports them as buttons 10-12. */ >>> /* 0x05, 0x0f, / * Usage Page (Button), */ >>> /* 0x19, 0x01, / * Usage Minimum (01h), */ >>> /* 0x29, 0x03, / * Usage Maximum (03h), */ >>> /* End of Stylus button usage */ >> >> I think having six usages and only three reports wouldn't work. > > Those were simply commented out versions of what the device originally sent. Ah, sorry, didn't notice that. >>> 0x15, 0x00, /* Logical Minimum (0), */ >>> 0x25, 0x01, /* Logical Maximum (1), */ >>> 0x95, 0x03, /* Report Count (3), */ >>> 0x75, 0x01, /* Report Size (1), */ >>> 0x81, 0x02, /* Input (Variable), */ >>> 0x95, 0x05, /* Report Count (5), */ >>> 0x81, 0x01, /* Input (Constant), */ >>> 0x05, 0x01, /* Usage Page (Desktop), */ >>> 0x09, 0x30, /* Usage (X), */ >>> 0x09, 0x31, /* Usage (Y), */ >>> 0x15, 0x00, /* Logical Minimum (0), */ >>> 0x26, 0x00, 0x10, /* Logical Maximum (4096), */ >>> 0x35, 0x00, /* Physical Minimum (0), */ >>> 0x46, 0x00, 0x10, /* Physical Maximum (4096), */ >> >> Is the actual physical drawing area square? > > No. I believe it's meant to be 16:10 proportions. > A quick measurement by the printed border shows: > X: inner 103.65mm, outer 105mm > Y: inner 65.6mm, outer 66.45mm > This gives an aspect ratio somewhere from 1.56 to 1.60. Measuring the firmware limits > would be a bit harder (for instance, it doesn't measure the tip but the coil position). > I guess 105.6*66mm would be a reasonable approximation. This would be awfully low resolution and most likely scaled by firmware for compatibility. The oldest tablet I have (UC-Logic WP8060U) has higher resolution, I think. There's very little chance a non-square tablet would report equal coordinate range for both axes in hardware. There might be a way to get it to report real ranges over USB. I would try poking at those features in the report descriptor. There should have been a driver for this tablet doing that. Perhaps we can find it and take a look at the USB traffic. We can postpone it for later, though. > I have tried to put this data into the attached descriptor. BTW, the original descriptor for interface 2 seems to have the correct range and physical extents. You can take the extents from there at least, which seem to be 4x2.5 inches. > It would also be a little tricky to measure the unit of pressure, so I took the > push/pop example from one of the uc-logic descriptors. Sure. It would also be mostly pointless. I don't think any manufacturer really calibrates them. There is no kernel support for communicating the resolution and people just use pressure curves to adjust the behavior as necessary anyway. > The attached version uses pick, and thus reports (using input-events) stylus > for the side button closer to the tip and stylus2 for the one further away. > > It was tested to work smoothly in inkscape and krita. Since it overrides the > descriptor it doesn't care about the readout bugs, so it works on its own (but > still requires the unbinding of hid-generic from the relevant interface). Great! Yes, you need rebinding for out-of-tree drivers anyway. That's just the way kernel works. It won't be necessary if it gets merged into the kernel. Speaking of which, I think you need to make the report descriptor as simple as possible to make it easier to get into the kernel. Strip it down to the absolute minimum required to work. Remove unused report IDs and collections. > http://www.nybytt.se/zeniq-pen-tablet/ is a clear example of one of these tablets. > Mine happen to have the Leogics brand instead. Ah, another rebranding. I have some comments to the code below. > /* > * HID driver for Polostar devices not fully compliant with HID standard > * > * Copyright (c) 2010 Nikolai Kondrashov There's no need to put my copyright on here. > * Copyright (c) 2015 Yann Vernier > */ > > /* > * This program is free software; you can redistribute it and/or modify it > * under the terms of the GNU General Public License as published by the Free > * Software Foundation; either version 2 of the License, or (at your option) > * any later version. > */ > > #include <linux/device.h> > #include <linux/hid.h> > #include <linux/module.h> > #include <linux/usb.h> > > #include "hid-ids.h" > /* Known as Zippy Technology Corp. in usbutils */ > #define USB_VENDOR_ID_POLOSTAR 0x099a > #define USB_DEVICE_ID_POLOSTAR_TABLET_PT1001 0x2620 > > /* Size of the original descriptor of PT-1001 tablets */ > #define PT1001_RDESC_ORIG_SIZE 317 > > /* Fixed PT1001 report descriptor */ > static __u8 pt1001_rdesc_fixed[] = { > 0x05, 0x01, /* Usage Page (Desktop), */ > 0x09, 0x02, /* Usage (Mouse), */ > 0xA1, 0x01, /* Collection (Application), */ > 0x05, 0x09, /* Usage Page (Button), */ > /* Report ID 1 is used for mouse mode and scroll */ > 0x85, 0x01, /* Report ID (1), */ > 0x19, 0x01, /* Usage Minimum (01h), */ > 0x29, 0x05, /* Usage Maximum (05h), */ > 0x95, 0x05, /* Report Count (5), */ > 0x75, 0x01, /* Report Size (1), */ > 0x15, 0x00, /* Logical Minimum (0), */ > 0x25, 0x01, /* Logical Maximum (1), */ > 0x81, 0x02, /* Input (Variable), */ > 0x95, 0x03, /* Report Count (3), */ > 0x81, 0x01, /* Input (Constant), */ > 0x05, 0x01, /* Usage Page (Desktop), */ > 0x09, 0x01, /* Usage (Pointer), */ > 0xA1, 0x00, /* Collection (Physical), */ > 0x09, 0x30, /* Usage (X), */ > 0x09, 0x31, /* Usage (Y), */ > 0x95, 0x02, /* Report Count (2), */ > 0x75, 0x10, /* Report Size (16), */ > 0x16, 0x01, 0x80, /* Logical Minimum (-32767), */ > 0x26, 0xFF, 0x7F, /* Logical Maximum (32767), */ > 0x81, 0x06, /* Input (Variable, Relative), */ > 0xC0, /* End Collection, */ > /* The multipliers do not seem to occur as reports. */ > /* They seem to relate to vertical and horisontal scroll. */ > 0xA1, 0x02, /* Collection (Logical), */ > 0x85, 0x02, /* Report ID (2), */ > 0x09, 0x48, /* Usage (Resolution Multiplier), */ > 0x15, 0x00, /* Logical Minimum (0), */ > 0x25, 0x01, /* Logical Maximum (1), */ > 0x95, 0x01, /* Report Count (1), */ > 0x75, 0x02, /* Report Size (2), */ > 0xB1, 0x02, /* Feature (Variable), */ > 0x85, 0x01, /* Report ID (1), */ > 0x09, 0x38, /* Usage (Wheel), */ > 0x15, 0x81, /* Logical Minimum (-127), */ > 0x25, 0x7F, /* Logical Maximum (127), */ > 0x75, 0x08, /* Report Size (8), */ > 0x95, 0x01, /* Report Count (1), */ > 0x81, 0x06, /* Input (Variable, Relative), */ > 0xC0, /* End Collection, */ > 0xA1, 0x02, /* Collection (Logical), */ > 0x85, 0x02, /* Report ID (2), */ > 0x09, 0x48, /* Usage (Resolution Multiplier), */ > 0x15, 0x00, /* Logical Minimum (0), */ > 0x25, 0x01, /* Logical Maximum (1), */ > 0x75, 0x02, /* Report Size (2), */ > 0xB1, 0x02, /* Feature (Variable), */ > 0x75, 0x04, /* Report Size (4), */ > 0xB1, 0x01, /* Feature (Constant), */ > 0x85, 0x01, /* Report ID (1), */ > 0x05, 0x0C, /* Usage Page (Consumer), */ > 0x0A, 0x38, 0x02, /* Usage (AC Pan), */ > 0x15, 0x81, /* Logical Minimum (-127), */ > 0x25, 0x7F, /* Logical Maximum (127), */ > 0x75, 0x08, /* Report Size (8), */ > 0x95, 0x01, /* Report Count (1), */ > 0x81, 0x06, /* Input (Variable, Relative), */ > 0xC0, /* End Collection, */ > 0xC0, /* End Collection, */ You can replace the multiplier fields with constant fields, if they're unused. Also get rid of nested collections and repeated report IDs, if you can. > /* Report 3 looks like the raw protocol. Unknown how to enable. */ > 0x06, 0x00, 0xFF, /* Usage Page (FF00h), */ > 0x09, 0x01, /* Usage (01h), */ > 0xA1, 0x01, /* Collection (Application), */ > 0x85, 0x03, /* Report ID (3), */ > 0x19, 0x01, /* Usage Minimum (01h), */ > 0x29, 0x01, /* Usage Maximum (01h), */ > 0x95, 0x07, /* Report Count (7), */ > 0x81, 0x02, /* Input (Variable), */ > 0x19, 0x01, /* Usage Minimum (01h), */ > 0x29, 0x01, /* Usage Maximum (01h), */ > 0xB1, 0x02, /* Feature (Variable), */ > 0x09, 0x02, /* Usage (02h), */ > 0x15, 0x00, /* Logical Minimum (0), */ > 0x26, 0xFF, 0x00, /* Logical Maximum (255), */ > 0x75, 0x08, /* Report Size (8), */ > 0x95, 0x01, /* Report Count (1), */ > 0x91, 0x02, /* Output (Variable), */ > 0xC0, /* End Collection, */ It's better to remove it, if it's unused. You can always add it back if you figure out a way to make it work. > /* Report ID 5 is used for some periphery buttons */ > 0x05, 0x0C, /* Usage Page (Consumer), */ > 0x09, 0x01, /* Usage (Consumer Control), */ > 0xA1, 0x01, /* Collection (Application), */ > 0x85, 0x05, /* Report ID (5), */ > 0x95, 0x01, /* Report Count (1), */ > 0x75, 0x08, /* Report Size (8), */ > 0x81, 0x01, /* Input (Constant), */ > 0x15, 0x00, /* Logical Minimum (0), */ > 0x25, 0x01, /* Logical Maximum (1), */ > 0x75, 0x01, /* Report Size (1), */ > 0x95, 0x12, /* Report Count (18), */ > 0x0A, 0x83, 0x01, /* Usage (AL Consumer Control Config), */ > 0x0A, 0x8A, 0x01, /* Usage (AL Email Reader), */ > 0x0A, 0x92, 0x01, /* Usage (AL Calculator), */ > 0x0A, 0x94, 0x01, /* Usage (AL Local Machine Brwsr), */ > 0x0A, 0x21, 0x02, /* Usage (AC Search), */ > 0x0A, 0x23, 0x02, /* Usage (AC Home), */ > 0x0A, 0x24, 0x02, /* Usage (AC Back), */ > 0x0A, 0x25, 0x02, /* Usage (AC Forward), */ > 0x0A, 0x26, 0x02, /* Usage (AC Stop), */ > 0x0A, 0x27, 0x02, /* Usage (AC Refresh), */ > 0x0A, 0x2A, 0x02, /* Usage (AC Bookmarks), */ > 0x09, 0xB5, /* Usage (Scan Next Track), */ > 0x09, 0xB6, /* Usage (Scan Previous Track), */ > 0x09, 0xB7, /* Usage (Stop), */ > 0x09, 0xCD, /* Usage (Play Pause), */ > 0x09, 0xE2, /* Usage (Mute), */ > 0x09, 0xE9, /* Usage (Volume Inc), */ > 0x09, 0xEA, /* Usage (Volume Dec), */ > 0x81, 0x62, /* Input (Variable, No Preferred, Null State), */ > 0x95, 0x06, /* Report Count (6), */ > 0x75, 0x01, /* Report Size (1), */ > 0x81, 0x03, /* Input (Constant, Variable), */ > 0xC0, /* End Collection, */ > > /* Report 9 is the primary digitizer report. */ > 0x05, 0x0D, /* Usage Page (Digitizer), */ > 0x09, 0x01, /* Usage (Digitizer), */ > 0xA1, 0x01, /* Collection (Application), */ > 0x85, 0x09, /* Report ID (9), */ > 0x09, 0x20, /* Usage (Stylus), */ > 0xA1, 0x00, /* Collection (Physical), */ > > 0x09, 0x42, /* Usage (Tip Switch), */ > 0x09, 0x44, /* Usage (Barrel Switch), */ > 0x09, 0x46, /* Usage (Tablet Pick), */ /* Really on the barrel too */ > 0x15, 0x00, /* Logical Minimum (0), */ > 0x25, 0x01, /* Logical Maximum (1), */ > 0x95, 0x03, /* Report Count (3), */ > 0x75, 0x01, /* Report Size (1), */ > 0x81, 0x02, /* Input (Variable), */ > > 0x95, 0x05, /* Report Count (5), */ > 0x81, 0x01, /* Input (Constant), */ > > 0x05, 0x01, /* Usage Page (Desktop), */ If you put this usage page inside the push/pop pair, you won't need to restore the digitizer page explicitly later. > 0x15, 0x00, /* Logical Minimum (0), */ > 0x26, 0x00, 0x10, /* Logical Maximum (4096), */ > 0x75, 0x10, /* Report Size (16), */ > 0x95, 0x01, /* Report Count (1), */ > > 0xa4, /* Push */ > > 0x55, 0xfe, /* unit exponent -2: cm -> 0.1mm */ > 0x65, 0x11, /* SI linear centimeters */ > > 0x09, 0x30, /* Usage (X), */ > 0x35, 0x00, /* Physical Minimum (0), */ > 0x46, 0x20, 0x04, /* Physical Maximum (1056), */ > 0x81, 0x02, /* Input (Variable), */ > > 0x09, 0x31, /* Usage (Y), */ > 0x35, 0x00, /* Physical Minimum (0), */ There is no need to repeat physical minimum, but it might be easier to read this way. > 0x46, 0x94, 0x02, /* Physical Maximum (660), */ > 0x81, 0x02, /* Input (Variable), */ > > 0xb4, /* Pop (restore no unit) */ > > 0x05, 0x0D, /* Usage Page (Digitizer), */ > 0x09, 0x30, /* Usage (Tip Pressure), */ > 0x26, 0xFF, 0x03, /* Logical Maximum (1023), */ > 0x95, 0x01, /* Report Count (1), */ There's no need to repeat report count either. > 0x81, 0x02, /* Input (Variable), */ > 0xC0, /* End Collection, */ > 0xC0, /* End Collection */ > }; > > static __u8 *polostar_report_fixup(struct hid_device *hdev, __u8 *rdesc, > unsigned int *rsize) > { > struct usb_interface *iface = to_usb_interface(hdev->dev.parent); > __u8 iface_num = iface->cur_altsetting->desc.bInterfaceNumber; > > switch (hdev->product) { > case USB_DEVICE_ID_POLOSTAR_TABLET_PT1001: > if (iface_num==1 && *rsize == PT1001_RDESC_ORIG_SIZE) { There should be spaces around "==" according to the Linux Kernel Coding Style. > rdesc = pt1001_rdesc_fixed; > *rsize = sizeof(pt1001_rdesc_fixed); > } > break; > } > > return rdesc; > } > > static int polostar_probe(struct hid_device *hdev, const struct hid_device_id *id) > { > int rc; > > hdev->quirks |= id->driver_data; I would disable interface #2 here, if we can't make it work, with something like this: struct usb_interface *intf = to_usb_interface(hdev->dev.parent); if (intf->cur_altsetting->desc.bInterfaceNumber == 2) return -ENODEV; > rc = hid_parse(hdev); > if (rc) { > hid_err(hdev, "parse failed\n"); > return rc; > } > > rc = hid_hw_start(hdev, HID_CONNECT_DEFAULT); > if (rc) { > hid_err(hdev, "hw start failed\n"); > return rc; > } > > return 0; > } > > static const struct hid_device_id polostar_devices[] = { > { HID_USB_DEVICE(USB_VENDOR_ID_POLOSTAR, > USB_DEVICE_ID_POLOSTAR_TABLET_PT1001), > .driver_data = HID_QUIRK_MULTI_INPUT }, > { } > }; > MODULE_DEVICE_TABLE(hid, polostar_devices); > > static struct hid_driver polostar_driver = { > .name = "polostar", > .id_table = polostar_devices, > .probe = polostar_probe, > .report_fixup = polostar_report_fixup, > }; > module_driver(polostar_driver, hid_register_driver, hid_unregister_driver); > > MODULE_LICENSE("GPL"); > MODULE_VERSION("5"); BTW, can you make pull requests to digimend-kernel-drivers instead of sending files? Thank you. Nick |
