From: Kirill A. Korinsky Subject: Re: qcgpio(4) X1E support based on ACPI tables To: Marcus Glocker Cc: tech@openbsd.org Date: Sun, 22 Dec 2024 13:35:02 +0100 On Sun, 22 Dec 2024 10:51:35 +0100, Marcus Glocker wrote: > > patrick@ found a NetBSD commit which figured out how to map the GPIO > pins based on the ACPI tables for X1E devices: > > https://github.com/NetBSD/src/commit/575554bd388d93cfff9174abadf2aab93fbfdf8e > > Currently we're only using hard coded mapping in qcgpio(4), which > might not do the correct mapping, or miss some mappings. > > Following a diff which tries to adapt the NetBSD implementation in > qcpgio(4) for X1E devices. > > Before I throw this diff in my attic, maybe some X1E users want to > regression test it, or see if it even fixes some GPIO mappings. > > On my Samsung Galaxy Book4 Edge it causes no regression (keyboard and > touch-pad still work). Unfortunately the touch-screen still doesn't > work. I guess there is another issue with that. > > On MagicBook Art 14 Snapdragon it fixes an issue with coursor moving with this patch: https://marc.info/?l=openbsd-tech&m=173483399503786&w=2 and lead to need only one quirk IHIDEV_QUIRK_SKIP_RESET. Keaboard: no regression on embeded one, fixes use USB-C connected keyboard. Before that patch I can connect keyboard only to USB-A port. It doesn't affect touchscreen: works as before I like it and I think we need it. FWIW ok @kirill > Index: sys/dev/acpi/qcgpio.c > =================================================================== > RCS file: /cvs/src/sys/dev/acpi/qcgpio.c,v > diff -u -p -u -p -r1.11 qcgpio.c > --- sys/dev/acpi/qcgpio.c 15 Jul 2024 15:33:54 -0000 1.11 > +++ sys/dev/acpi/qcgpio.c 15 Dec 2024 19:03:25 -0000 > @@ -25,6 +25,14 @@ > #include > #include > > +#define QCGPIO_DEBUG > +#ifdef QCGPIO_DEBUG > +int qcgpio_debug = 1; > +#define DPRINTF(l, x...) do { if ((l) <= qcgpio_debug) printf(x); } while (0) > +#else > +#define DPRINTF(l, x...) > +#endif > + > /* Registers. */ > #define TLMM_GPIO_IN_OUT(pin) (0x0004 + 0x1000 * (pin)) > #define TLMM_GPIO_IN_OUT_GPIO_IN (1 << 0) > @@ -62,6 +70,11 @@ struct qcgpio_intrhand { > void *ih_arg; > }; > > +struct qcgpio_pdcmap { > + int pm_pin; > + uint32_t pm_irq; > +}; > + > struct qcgpio_softc { > struct device sc_dev; > struct acpi_softc *sc_acpi; > @@ -73,10 +86,15 @@ struct qcgpio_softc { > void *sc_ih; > > uint32_t sc_npins; > - int (*sc_pin_map)(int, bus_size_t *); > + int (*sc_pin_map)(struct qcgpio_softc *, int, > + bus_size_t *); > struct qcgpio_intrhand *sc_pin_ih; > > struct acpi_gpio sc_gpio; > + > + struct qcgpio_pdcmap *sc_pdcmap; > + uint32_t sc_npdcmap; > + uint32_t sc_ipdcmap; > }; > > int qcgpio_acpi_match(struct device *, void *, void *); > @@ -97,9 +115,29 @@ const char *qcgpio_hids[] = { > NULL > }; > > -int qcgpio_sc7180_pin_map(int, bus_size_t *); > -int qcgpio_sc8280xp_pin_map(int, bus_size_t *); > -int qcgpio_x1e80100_pin_map(int, bus_size_t *); > +/* 98b9b2a4-1663-4a5f-82f2-c6c99a394726 */ > +static uint8_t qcgpio_gpio_dsm_uuid[] = { > + 0xa4, 0xb2, 0xb9, 0x98, 0x63, 0x16, 0x5f, 0x4a, > + 0x82, 0xf2, 0xc6, 0xc9, 0x9a, 0x39, 0x47, 0x26 > +}; > +#define QCGPIO_GPIO_DSM_REV 0 > +#define QCGPIO_GPIO_DSM_FUNC_NUM_PINS 2 > + > +/* 921b0fd4-567c-43a0-bb14-2648f7b2a18c */ > +static uint8_t qcgpio_pdc_dsm_uuid[] = { > + 0xd4, 0x0f, 0x1b, 0x92, 0x7c, 0x56, 0xa0, 0x43, > + 0xbb, 0x14, 0x26, 0x48, 0xf7, 0xb2, 0xa1, 0x8c > +}; > +#define QCGPIO_PDC_DSM_REV 0 > +#define QCGPIO_PDC_DSM_FUNC_CIPR 2 > + > +int qcgpio_get_nirq(int, union acpi_resource *, void *); > +int qcgpio_get_irqs(int, union acpi_resource *, void *); > +void qcgpio_fill_pdcmap(struct qcgpio_softc *); > +int qcgpio_get_pin_count(struct acpi_softc *, struct aml_node *); > +int qcgpio_sc7180_pin_map(struct qcgpio_softc *, int, bus_size_t *); > +int qcgpio_sc8280xp_pin_map(struct qcgpio_softc *, int, bus_size_t *); > +int qcgpio_x1e80100_pin_map(struct qcgpio_softc *, int, bus_size_t *); > > int qcgpio_read_pin(void *, int); > void qcgpio_write_pin(void *, int, int); > @@ -109,6 +147,137 @@ void qcgpio_intr_disable(void *, int); > int qcgpio_intr(void *); > > int > +qcgpio_get_nirq(int crsidx, union acpi_resource *crs, void *arg) > +{ > + struct qcgpio_softc *sc = arg; > + int typ; > + > + typ = AML_CRSTYPE(crs); > + > + switch (typ) { > + case LR_EXTIRQ: > + sc->sc_npdcmap++; > + break; > + } > + > + return 0; > +} > + > +int > +qcgpio_get_irqs(int crsidx, union acpi_resource *crs, void *arg) > +{ > + struct qcgpio_softc *sc = arg; > + int typ; > + > + typ = AML_CRSTYPE(crs); > + > + switch (typ) { > + case LR_EXTIRQ: > + sc->sc_pdcmap[sc->sc_ipdcmap].pm_irq = crs->lr_extirq.irq[0]; > + sc->sc_pdcmap[sc->sc_ipdcmap].pm_pin = -1; > + DPRINTF(1, "%s: irq index %d: irq %d\n", > + __func__, sc->sc_ipdcmap, > + sc->sc_pdcmap[sc->sc_ipdcmap].pm_irq); > + sc->sc_ipdcmap++; > + break; > + } > + > + return 0; > +} > + > +void > +qcgpio_fill_pdcmap(struct qcgpio_softc *sc) > +{ > + struct aml_value cmd[4], res, *ref; > + int i, j, pin; > + uint32_t irq; > + > + bzero(&cmd, sizeof(cmd)); > + cmd[0].type = AML_OBJTYPE_BUFFER; > + cmd[0].v_buffer = (uint8_t *)&qcgpio_pdc_dsm_uuid; > + cmd[0].length = sizeof(qcgpio_pdc_dsm_uuid); > + /* rev */ > + cmd[1].type = AML_OBJTYPE_INTEGER; > + cmd[1].v_integer = QCGPIO_PDC_DSM_REV; > + cmd[1].length = 1; > + /* func */ > + cmd[2].type = AML_OBJTYPE_INTEGER; > + cmd[2].v_integer = QCGPIO_PDC_DSM_FUNC_CIPR; > + cmd[2].length = 1; > + /* not used */ > + cmd[3].type = AML_OBJTYPE_PACKAGE; > + cmd[3].v_integer = 0; > + cmd[3].length = 0; > + > + if (aml_evalname(sc->sc_acpi, sc->sc_node, "_DSM", 4, cmd, &res)) { > + printf("%s: PDC _DSM failed\n", __func__); > + return; > + } > + > + for (i = 0; i < res.length; i++) { > + ref = res.v_package[i]; > + > + if (ref->type != AML_OBJTYPE_PACKAGE || > + ref->length < 3 || > + ref->v_package[0]->type != AML_OBJTYPE_INTEGER || > + ref->v_package[1]->type != AML_OBJTYPE_INTEGER || > + ref->v_package[2]->type != AML_OBJTYPE_INTEGER) { > + continue; > + } > + > + irq = ref->v_package[2]->v_integer; > + pin = ref->v_package[1]->v_integer; > + DPRINTF(1, "%s: pdc index %d: probing irq %d, pin %d\n", > + __func__, i, irq, pin); > + > + for (j = 0; j < sc->sc_npdcmap; j++) { > + if (sc->sc_pdcmap[j].pm_irq == irq) { > + sc->sc_pdcmap[j].pm_pin = pin; > + break; > + } > + } > + } > +#ifdef QCGPIO_DEBUG > + for (i = 0; i < sc->sc_npdcmap; i++) { > + printf("%s: irq index %d: irq=%d, pin=%d\n", > + __func__, i, sc->sc_pdcmap[i].pm_irq, > + sc->sc_pdcmap[i].pm_pin); > + } > +#endif > +} > + > +int > +qcgpio_get_pin_count(struct acpi_softc *sc, struct aml_node *node) > +{ > + struct aml_value cmd[4]; > + int64_t npins; > + > + bzero(&cmd, sizeof(cmd)); > + cmd[0].type = AML_OBJTYPE_BUFFER; > + cmd[0].v_buffer = (uint8_t *)&qcgpio_gpio_dsm_uuid; > + cmd[0].length = sizeof(qcgpio_gpio_dsm_uuid); > + /* rev */ > + cmd[1].type = AML_OBJTYPE_INTEGER; > + cmd[1].v_integer = QCGPIO_GPIO_DSM_REV; > + cmd[1].length = 1; > + /* func */ > + cmd[2].type = AML_OBJTYPE_INTEGER; > + cmd[2].v_integer = QCGPIO_GPIO_DSM_FUNC_NUM_PINS; > + cmd[2].length = 1; > + /* not used */ > + cmd[3].type = AML_OBJTYPE_PACKAGE; > + cmd[3].v_integer = 0; > + cmd[3].length = 0; > + > + if (aml_evalinteger(sc, node, "_DSM", 4, cmd, &npins)) { > + printf("%s: GPIO _DSM failed\n", __func__); > + return 0; > + } > + > + return (uint32_t)npins; > +} > + > +int > qcgpio_acpi_match(struct device *parent, void *match, void *aux) > { > struct acpi_attach_args *aaa = aux; > @@ -124,6 +293,7 @@ qcgpio_acpi_attach(struct device *parent > { > struct acpi_attach_args *aaa = aux; > struct qcgpio_softc *sc = (struct qcgpio_softc *)self; > + struct aml_value res; > > sc->sc_acpi = (struct acpi_softc *)parent; > sc->sc_node = aaa->aaa_node; > @@ -145,7 +315,25 @@ qcgpio_acpi_attach(struct device *parent > sc->sc_npins = 228; > sc->sc_pin_map = qcgpio_sc8280xp_pin_map; > } else if (strcmp(aaa->aaa_dev, "QCOM0C0C") == 0) { > - sc->sc_npins = 239; > + if (aml_evalname(sc->sc_acpi, sc->sc_node, "_CRS", 0, NULL, > + &res)) { > + printf("no _CRS method\n"); > + return; > + } > + if (res.type != AML_OBJTYPE_BUFFER || res.length < 5) { > + printf("invalid _CRS object\n"); > + aml_freevalue(&res); > + return; > + } > + aml_parse_resource(&res, qcgpio_get_nirq, sc); > + DPRINTF(1, "\n%s: npdcmap=%d\n", __func__, sc->sc_npdcmap); > + sc->sc_pdcmap = mallocarray(sc->sc_npdcmap, > + sizeof(*sc->sc_pdcmap), M_DEVBUF, M_WAITOK | M_ZERO); > + aml_parse_resource(&res, qcgpio_get_irqs, sc); > + aml_freevalue(&res); > + qcgpio_fill_pdcmap(sc); > + sc->sc_npins = qcgpio_get_pin_count(sc->sc_acpi, sc->sc_node); > + DPRINTF(1, "%s: npins=%d\n", __func__, sc->sc_npins); > sc->sc_pin_map = qcgpio_x1e80100_pin_map; > } > KASSERT(sc->sc_npins != 0); > @@ -180,11 +368,12 @@ unmap: > if (sc->sc_ih) > acpi_intr_disestablish(sc->sc_ih); > free(sc->sc_pin_ih, M_DEVBUF, sc->sc_npins * sizeof(*sc->sc_pin_ih)); > + free(sc->sc_pdcmap, M_DEVBUF, sc->sc_npdcmap * sizeof(*sc->sc_pdcmap)); > bus_space_unmap(sc->sc_iot, sc->sc_ioh, aaa->aaa_size[0]); > } > > int > -qcgpio_sc7180_pin_map(int pin, bus_size_t *off) > +qcgpio_sc7180_pin_map(struct qcgpio_softc *sc, int pin, bus_size_t *off) > { > switch (pin) { > case 30: > @@ -211,7 +400,7 @@ qcgpio_sc7180_pin_map(int pin, bus_size_ > } > > int > -qcgpio_sc8280xp_pin_map(int pin, bus_size_t *off) > +qcgpio_sc8280xp_pin_map(struct qcgpio_softc *sc, int pin, bus_size_t *off) > { > switch (pin) { > case 107: > @@ -229,21 +418,19 @@ qcgpio_sc8280xp_pin_map(int pin, bus_siz > } > > int > -qcgpio_x1e80100_pin_map(int pin, bus_size_t *off) > +qcgpio_x1e80100_pin_map(struct qcgpio_softc *sc, int pin, bus_size_t *off) > { > - switch (pin) { > - case 3: > - case 51: > - return pin; > - case 0x180: > - return 67; > - case 0x380: > - return 33; > - case 0x3c0: > - return 3; > - default: > - return -1; > + int real_pin = -1; > + > + if (pin < sc->sc_npins) { > + real_pin = pin; > + } else if (pin / 64 < sc->sc_npdcmap) { > + real_pin = sc->sc_pdcmap[pin / 64].pm_pin; > } > + > + DPRINTF(2, "%s: map pin %d to real_pin %d\n", __func__, pin, real_pin); > + > + return real_pin; > } > > int > @@ -253,7 +440,7 @@ qcgpio_read_pin(void *cookie, int pin) > bus_size_t off = 0; > uint32_t reg; > > - pin = sc->sc_pin_map(pin, &off); > + pin = sc->sc_pin_map(sc, pin, &off); > if (pin < 0 || pin >= sc->sc_npins) > return 0; > > @@ -267,7 +454,7 @@ qcgpio_write_pin(void *cookie, int pin, > struct qcgpio_softc *sc = cookie; > bus_size_t off = 0; > > - pin = sc->sc_pin_map(pin, &off); > + pin = sc->sc_pin_map(sc, pin, &off); > if (pin < 0 || pin >= sc->sc_npins) > return; > > @@ -288,7 +475,7 @@ qcgpio_intr_establish(void *cookie, int > bus_size_t off = 0; > uint32_t reg; > > - pin = sc->sc_pin_map(pin, &off); > + pin = sc->sc_pin_map(sc, pin, &off); > if (pin < 0 || pin >= sc->sc_npins) > return; > > @@ -335,7 +522,7 @@ qcgpio_intr_enable(void *cookie, int pin > struct qcgpio_softc *sc = cookie; > bus_size_t off = 0; > > - pin = sc->sc_pin_map(pin, &off); > + pin = sc->sc_pin_map(sc, pin, &off); > if (pin < 0 || pin >= sc->sc_npins) > return; > > @@ -349,7 +536,7 @@ qcgpio_intr_disable(void *cookie, int pi > struct qcgpio_softc *sc = cookie; > bus_size_t off = 0; > > - pin = sc->sc_pin_map(pin, &off); > + pin = sc->sc_pin_map(sc, pin, &off); > if (pin < 0 || pin >= sc->sc_npins) > return; > > @@ -369,7 +556,7 @@ qcgpio_intr(void *arg) > if (sc->sc_pin_ih[pin].ih_func == NULL) > continue; > > - sc->sc_pin_map(pin, &off); > + sc->sc_pin_map(sc, pin, &off); > > stat = HREAD4(sc, off + TLMM_GPIO_INTR_STATUS(pin)); > if (stat & TLMM_GPIO_INTR_STATUS_INTR_STATUS) { > -- wbr, Kirill