Index | Thread | Search

From:
Marcus Glocker <marcus@nazgul.ch>
Subject:
ims(4) touchscreen support for the SGB4E
To:
tech@openbsd.org
Cc:
"Kirill A. Korinsky" <kirill@korins.ky>, joshua stein <jcs@jcs.org>
Date:
Sat, 23 May 2026 17:01:50 +0200

Download raw body.

Thread
  • Marcus Glocker:

    ims(4) touchscreen support for the SGB4E

This supersedes the earlier imt(4) touchscreen diff, which regressed
kirill's Honor panel.

The difference is that we enable the Samsung Galaxy Book4 Edge
touchscreen support with ims(4), not imt(4), which has less impact,
and introduces less risk for regression:

- The ihidev(4) change resolves the in-tree polled finger-up TODO
  and is child-agnostic.  This touchscreen remains polled, until we
  can find out which is the correct GPIO for enabling interrupts on
  this device.

- The hidms gate is edge-not-level so it spares pens and never touches
  relative mice.

Together with the DTS diff (not changed from last time) to power-on
the touchpanel device, this makes the touchscreen work on the
Samsung Galaxy Book4 Edge.

I would appreciate some regression testing from people who are using
ims(4) devices.


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;
 
Index: sysutils/firmware/arm64-qcom-dtb/Makefile
===================================================================
RCS file: /cvs/ports/sysutils/firmware/arm64-qcom-dtb/Makefile,v
diff -u -p -u -p -r1.29 Makefile
--- sysutils/firmware/arm64-qcom-dtb/Makefile	19 May 2026 04:47:35 -0000	1.29
+++ sysutils/firmware/arm64-qcom-dtb/Makefile	21 May 2026 19:32:13 -0000
@@ -1,6 +1,6 @@
 FW_DRIVER=	arm64-qcom-dtb
 FW_VER=		2.7
-REVISION=	1
+REVISION=	2
 
 DISTNAME=	devicetree-rebasing-6.17-dts
 
Index: sysutils/firmware/arm64-qcom-dtb/patches/patch-src_arm64_qcom_x1e80100-samsung-galaxy-book4-edge_dts
===================================================================
RCS file: /cvs/ports/sysutils/firmware/arm64-qcom-dtb/patches/patch-src_arm64_qcom_x1e80100-samsung-galaxy-book4-edge_dts,v
diff -u -p -u -p -r1.3 patch-src_arm64_qcom_x1e80100-samsung-galaxy-book4-edge_dts
--- sysutils/firmware/arm64-qcom-dtb/patches/patch-src_arm64_qcom_x1e80100-samsung-galaxy-book4-edge_dts	19 May 2026 04:47:35 -0000	1.3
+++ sysutils/firmware/arm64-qcom-dtb/patches/patch-src_arm64_qcom_x1e80100-samsung-galaxy-book4-edge_dts	21 May 2026 19:32:13 -0000
@@ -1,7 +1,7 @@
 Index: src/arm64/qcom/x1e80100-samsung-galaxy-book4-edge.dts
 --- src/arm64/qcom/x1e80100-samsung-galaxy-book4-edge.dts.orig
 +++ src/arm64/qcom/x1e80100-samsung-galaxy-book4-edge.dts
-@@ -0,0 +1,977 @@
+@@ -0,0 +1,988 @@
 +// SPDX-License-Identifier: BSD-3-Clause
 +/*
 + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
@@ -10,6 +10,7 @@ Index: src/arm64/qcom/x1e80100-samsung-g
 +/dts-v1/;
 +
 +#include <dt-bindings/gpio/gpio.h>
++#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
 +#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
 +
 +#include "x1e80100.dtsi"
@@ -181,6 +182,24 @@ Index: src/arm64/qcom/x1e80100-samsung-g
 +		regulator-always-on;
 +		regulator-boot-on;
 +	};
++
++	/* 3V3 rail powering the touchscreen, gated by pm8550ve_8 gpio6 */
++	vreg_misc_3p3: regulator-misc-3p3 {
++		compatible = "regulator-fixed";
++
++		regulator-name = "VREG_MISC_3P3";
++		regulator-min-microvolt = <3300000>;
++		regulator-max-microvolt = <3300000>;
++
++		gpio = <&pm8550ve_8_gpios 6 GPIO_ACTIVE_HIGH>;
++		enable-active-high;
++
++		pinctrl-0 = <&misc_3p3_reg_en>;
++		pinctrl-names = "default";
++
++		regulator-boot-on;
++		regulator-always-on;
++	};
 +};
 +
 +&apps_rsc {
@@ -572,26 +591,33 @@ Index: src/arm64/qcom/x1e80100-samsung-g
 +	};
 +};
 +
++/* GXTP7936 touchscreen, powered by vreg_misc_3p3.  No usable IRQ pin
++ * known, so ihidev(4) polls. */
 +&i2c8 {
 +	clock-frequency = <400000>;
 +
-+	status = "disabled";
++	status = "okay";
 +
 +	touchscreen@5d {
 +		compatible = "hid-over-i2c";
 +		reg = <0x5d>;
 +
 +		hid-descr-addr = <0x1>;
-+		/*
-+		 * Pin 51 is creating an interrupt storm.  Hence, choosing
-+		 * some other pin which just does nothing.  But obviously,
-+		 * the touchscreen won't work for now.
-+		 */
-+		 //interrupts-extended = <&tlmm 51 IRQ_TYPE_LEVEL_LOW>;
-+		interrupts-extended = <&tlmm 33 IRQ_TYPE_LEVEL_LOW>;
 +
-+		pinctrl-0 = <&ts0_default>;
-+		pinctrl-names = "default";
++		vdd-supply = <&vreg_misc_3p3>;
++	};
++};
++
++&pm8550ve_8_gpios {
++	misc_3p3_reg_en: misc-3p3-reg-en-state {
++		pins = "gpio6";
++		function = "normal";
++		bias-disable;
++		input-disable;
++		output-enable;
++		drive-push-pull;
++		power-source = <1>;	/* 1.8 V */
++		qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>;
 +	};
 +};
 +
@@ -863,21 +889,6 @@ Index: src/arm64/qcom/x1e80100-samsung-g
 +		pins = "gpio3";
 +		function = "gpio";
 +		bias-disable;
-+	};
-+
-+	ts0_default: ts0-default-state {
-+		int-n-pins {
-+			pins = "gpio51";
-+			function = "gpio";
-+			bias-disable;
-+		};
-+
-+		reset-n-pins {
-+			pins = "gpio48";
-+			function = "gpio";
-+			output-high;
-+			drive-strength = <16>;
-+		};
 +	};
 +
 +	wcd_default: wcd-reset-n-active-state {