From: Marcus Glocker Subject: Re: ims(4) touchscreen support for the SGB4E To: tech@openbsd.org Date: Tue, 26 May 2026 21:04:59 +0200 Here's a status update for this diff: - The DTS part (the power-on path for the Samsung touchscreen) was committed this morning, so it's out of the picture for this diff now. - I received a few test reports, all reporting no regressions. What the diff does: Today ihidev(4) doesn't handle empty packets for polling devices at all -- that's the /* * TODO: all fingers are up, should we pass to hid * layer? */ comment, which this diff completes with an edge-triggered check. ihidev(4) remembers the report-id of the last non-empty report. When we have delivered a report for some report-id and then read an empty packet while polling, the device has gone idle (all contacts up). We then hand that report-id's subdevice a zeroed report, so the upper layer can register the release. The edge condition makes this fire once, on the transition, not on every idle poll. Interrupt-driven devices are not affected -- their empty-packet path still just returns as before. The zeroed report then reaches hidms. For an interrupt-driven absolute device the real release report carries the exact coordinates; for our polled device the synthesized report carries 0,0. Currently hidms just sets the new absolute coordinates, so for the polled device the 0,0 snaps the pointer to the upper-left corner. To prevent that, we add a check in hidms: if a button was pressed before and is now released, we skip setting the absolute coordinates on that report and leave the pointer where it was. For an interrupt-driven device this is invisible -- the previous position is the same point the release report would have set -- and for the polled device it prevents the corner snap. Importantly, this hidms check only does anything for devices that report absolute coordinates (HIDMS_ABSX/ABSY). Relative pointers -- ordinary mice and touchpads, that is nearly all ums(4) users -- skip it entirely, which is why the test reports (all relative devices) show no change. The only absolute device exercising this path is the touchscreen. On regression risk: - For ihidev(4) I think it's close to zero: we've only completed a code path that wasn't implemented for polling devices, and interrupt-driven devices are untouched. - For hidms I think it's low as well: we now hold the last position on release instead of re-setting it to the position from the release report, which is the same point, so there's no visible change for existing absolute (interrupt-driven) devices and it's a no-op for relative ones. hidms_input() is used by ims(4) and ums(4); uwacom(4) uses hidms for setup but parses input itself, so it is not affected. If a regression does show up, the change is small and trivial to back out. Based on that, ok? Index: sys/dev/hid/hidms.c =================================================================== RCS file: /cvs/src/sys/dev/hid/hidms.c,v diff -u -p -u -p -r1.11 hidms.c --- sys/dev/hid/hidms.c 10 May 2024 10:49:10 -0000 1.11 +++ sys/dev/hid/hidms.c 23 May 2026 14:30:24 -0000 @@ -533,7 +533,7 @@ hidms_input(struct hidms *ms, uint8_t *d { int dx, dy, dz, dw; u_int32_t buttons = 0; - int i, s; + int i, s, released; DPRINTFN(5,("hidms_input: len=%d\n", len)); @@ -595,17 +595,26 @@ hidms_input(struct hidms *ms, uint8_t *d buttons != ms->sc_buttons) { DPRINTFN(10, ("hidms_input: x:%d y:%d z:%d w:%d buttons:0x%x\n", dx, dy, dz, dw, buttons)); + released = (ms->sc_buttons != 0 && buttons == 0); ms->sc_buttons = buttons; if (ms->sc_wsmousedev != NULL) { s = spltty(); + /* + * A polled touchscreen reports finger-up as a zeroed + * packet; emitting its (0,0) as an absolute position + * would snap the pointer to the corner. On a button + * release edge, hold the last position instead. + */ if (ms->sc_flags & HIDMS_ABSX) { - wsmouse_set(ms->sc_wsmousedev, - WSMOUSE_ABS_X, dx, 0); + if (!released) + wsmouse_set(ms->sc_wsmousedev, + WSMOUSE_ABS_X, dx, 0); dx = 0; } if (ms->sc_flags & HIDMS_ABSY) { - wsmouse_set(ms->sc_wsmousedev, - WSMOUSE_ABS_Y, dy, 0); + if (!released) + wsmouse_set(ms->sc_wsmousedev, + WSMOUSE_ABS_Y, dy, 0); dy = 0; } WSMOUSE_INPUT(ms->sc_wsmousedev, Index: sys/dev/i2c/ihidev.c =================================================================== RCS file: /cvs/src/sys/dev/i2c/ihidev.c,v diff -u -p -u -p -r1.43 ihidev.c --- sys/dev/i2c/ihidev.c 23 May 2026 11:10:57 -0000 1.43 +++ sys/dev/i2c/ihidev.c 23 May 2026 14:30:24 -0000 @@ -142,6 +142,7 @@ ihidev_attach(struct device *parent, str sc->sc_tag = ia->ia_tag; sc->sc_addr = ia->ia_addr; sc->sc_hid_desc_addr = ia->ia_size; + sc->sc_lastrepid = -1; if (ihidev_hid_command(sc, I2C_HID_CMD_DESCR, NULL) || ihidev_hid_desc_parse(sc)) { @@ -159,10 +160,13 @@ ihidev_attach(struct device *parent, str /* find largest report size and allocate memory for input buffer */ sc->sc_isize = letoh16(sc->hid_desc.wMaxInputLength); + sc->sc_repsizes = mallocarray(sc->sc_nrepid, sizeof(int), + M_DEVBUF, M_WAITOK | M_ZERO); for (repid = 0; repid < sc->sc_nrepid; repid++) { repsz = hid_report_size(sc->sc_report, sc->sc_reportlen, hid_input, repid); repsizes[repid] = repsz; + sc->sc_repsizes[repid] = repsz; if (repsz > sc->sc_isize) sc->sc_isize = repsz; if (repsz != 0) @@ -255,6 +259,9 @@ ihidev_detach(struct device *self, int f if (sc->sc_report != NULL) free(sc->sc_report, M_DEVBUF, sc->sc_reportlen); + if (sc->sc_repsizes != NULL) + free(sc->sc_repsizes, M_DEVBUF, sc->sc_nrepid * sizeof(int)); + return (0); } @@ -718,10 +725,21 @@ ihidev_intr(void *arg) psize = sc->sc_ibuf[0] | sc->sc_ibuf[1] << 8; if (psize <= 2 || psize > sc->sc_isize) { if (sc->sc_poll) { - /* - * TODO: all fingers are up, should we pass to hid - * layer? - */ + /* empty packet: hand the last subdev a zeroed report + * once so it releases its contacts (polled finger-up) */ + int lrep = sc->sc_lastrepid; + int rsz; + + if (lrep >= 0 && lrep < sc->sc_nrepid && + (scd = sc->sc_subdevs[lrep]) != NULL && + (scd->sc_state & IHIDEV_OPEN) && !sc->sc_dying) { + rsz = sc->sc_repsizes[lrep]; + if (rsz > 0 && rsz <= sc->sc_isize) { + memset(sc->sc_ibuf, 0, rsz); + scd->sc_intr(scd, sc->sc_ibuf, rsz); + } + sc->sc_lastrepid = -1; + } sc->sc_fastpoll = 0; goto more_polling; } else @@ -770,8 +788,10 @@ ihidev_intr(void *arg) return (1); } - if (!sc->sc_dying) + if (!sc->sc_dying) { + sc->sc_lastrepid = rep; scd->sc_intr(scd, p, psize); + } if (sc->sc_poll && (fast != sc->sc_fastpoll)) { DPRINTF(("%s: %s->%s polling\n", sc->sc_dev.dv_xname, Index: sys/dev/i2c/ihidev.h =================================================================== RCS file: /cvs/src/sys/dev/i2c/ihidev.h,v diff -u -p -u -p -r1.11 ihidev.h --- sys/dev/i2c/ihidev.h 7 Jan 2025 19:26:14 -0000 1.11 +++ sys/dev/i2c/ihidev.h 23 May 2026 14:30:24 -0000 @@ -85,6 +85,8 @@ struct ihidev_softc { u_int sc_isize; u_char *sc_ibuf; + int sc_lastrepid; /* report id of last non-empty input */ + int *sc_repsizes; /* per-report input size, for poll path */ int sc_refcnt;