DRM/KMS Pimoroni HyperPixel4 driver

[6by9]

From https://forums.raspberrypi.com/viewtopic.php?t=324640

This happens to be against rpi-5.15.y as that was the tree I had at the time. It should easily backport to 5.10 should it be needed.

Comments on the register settings could be extended based on https://github.com/pimoroni/hyperpixel4/blob/pi4-i2c-fix/dist/hyperpixel4-init#L57

There is still an outstanding issue on how to handle the shared use of GPIO 27 for both SPI CLK and touch screen interrupt, and it needs a dtbinding too if it should be upstreamed.

@Gadgetoid for info.

[Gadgetoid]

@6by9 you don't know the extent to which you've just made my day. I haven't had the time and headspace to take the helpful input from the forumites and put it into practise. Thank you!

It may be possible to run the touchscreen in polled mode to skirt the GPIO conflict, albeit that would be suboptimal. The GPIO state machine approach seems workable, but I haven't had an opportunity to play with it yet. I'm currently tackling an issue with the touch IC having different default register settings across hardware revisions, which may warrant some form of upstream change to the Goodix GT911 driver.

Do you have some idea how a custom SPI init sequence can be supplied to ILI9806E? I've seen various examples of blobs in device tree, plus the notion of "firmware" but I'm not sure what the canonical approach for this might be. I would- I suspect- have ended up writing a driver specific to hyperpixel4 that (erroneously) bakes this init in.

[6by9]

Polling the touchscreen is the fallback, but with it being a bitbashed I2C that really is suboptimal. I'll see where Phil gets to in looking at gpio-fsm.
The Goodix driver doesn't currently support a polled mode, so that would need some further work to implement, but a very similar thing was done for edt5x06, and it's not overly complex.

Do you have some idea how a custom SPI init sequence can be supplied to ILI9806E? I've seen various examples of blobs in device tree, plus the notion of "firmware" but I'm not sure what the canonical approach for this might be. I would- I suspect- have ended up writing a driver specific to hyperpixel4 that (erroneously) bakes this init in.

Whilst I've scanned the datasheet, I don't know the detail of the modes that can reasonably be expected to be supported by ILI9806 and how that affects the init sequences.
Blobs shouldn't go in DT. There is discussion in mainline (dri-devel) at present for configuring the SPI type displays, and I think the conclusion was to use the firmware load mechanism to pick up the relevant blob from /lib/firmware.

Having had a closer look at the HyperPixel4, the compatible string probably ought to be txw,txw397017s2 rather than ilitek,ili9806e to identify the panel rather than the driver chip (txw would need to be added as a vendor prefix). Panel variants that use the same driver chip would have their own unique compatible string, and that can pick up a specific init sequence (see panel-ilitek-ili9881c for an example of that).

The fact that ILI9806E also appears to support DSI input as well as DPI just confuses matters further, and adds extra potential options for the driver. I'm definitely ignoring that one for now!

[pelwell]

Hmm - GitHub isn't letting me update your PR, but there is a rebased version with my (non-gpio-fsm) hacks in https://github.com/pelwell/linux/commits/hyperpixel4.

[6by9]

I've added you as a collaborator which will probably give you push access.
I'll cherry-pick your change though and update the PR.

If you could drop the display back into the office when you're next in that would be useful.

[pelwell]

If you could drop the display back into the office when you're next in that would be useful.

Will do.

[6by9]

Updated after having cleaned up the driver, merged in Phil's changes, and added the HyperPixel4 Square driver and overlay.

So it just leaves the question over how we handle the NONEXCLUSIVE flag and spi-gpio mods. Do we add a DT parameter to enable that? I don't believe there's a way to do it via the existing GPIO flags in there.

[6by9]

HyperPixel2 Round is going to be trickier as it uses the ST7701S driver chip. There's already a driver for that, but only when connected over DSI (it supports SPI, DSI, and DPI), so the driver needs to be reworked to support multiple interfaces. It's quite possible to extend it (panel-simple already does DSI and non-DSI panels), but it requires more time than I have available now.

[6by9]

Sorry, last push needed as I hadn't attached the spi_device_id struct, so the kernel complained when loading the driver (even though it worked).

[pelwell]

So it just leaves the question over how we handle the NONEXCLUSIVE flag and spi-gpio mods. Do we add a DT parameter to enable that? I don't believe there's a way to do it via the existing GPIO flags in there.

The way of doing it would be to add another GPIO flag - I haven't traced it through, but it shouldn't be a very invasive change since it is all within the GPIO framework.

spi-gpio driver change to return its clock pin to an input would be triggered by a DT parameter. We should probably do the same for the MOSI pin as well to make it less arbitrary.

[6by9]

OK, I'll look at the required changes when I have some more spare time.

HyperPixel 4 Square uses edt5406 (same as the Pi 7" DSI panel), but that driver doesn't do an devm_gpiod_get_optional for the GPIO, just

		error = devm_request_threaded_irq(&client->dev, client->irq,
						  NULL, edt_ft5x06_ts_isr,
						  irq_flags, client->name,
						  tsdata);

That said, I don't see a reason why it doesn't do the gpiod call first, and as we already have the polling fallback in there it's relatively easy to do.

[pelwell]

By the time the probe function is run, the i2c framework has already found the interrupts declaration in the DT node and claimed an interrupt, presented in client->irq. The goodix driver has the parallel irq-gpios declaration because the device overloads the interrupt pin - driving it low at various times for various purposes. I hope that isn't something we have to deal with.

[pelwell]

The good news is that I think adding the NONEXCLUSIVE support to DT is straightforward:

1. Add an entry to the of_gpio_flags enum.
2. Add an equivalent flag to include/dt-bindings/gpio/gpio.h.
3. Check for the new bit in gpiod_get_from_of_node, setting it in dflags.

I'm hoping that's it.

[6by9]

Hmm, are interrupts handled by pinctrl and gpios by gpio?

I've done the plumbing, but for Hyperpixel4 Square I get an error thrown by pinctrl due to pin 27 already being used. I'd copied Phil's dt for the touch, and that included pinctrl definitions for the interrupt line on the touch.

Indeed I appear to be able to revert all the changes (except GPIO direction in spi-gpio) as long as only one defines the pinctrl node.

[pelwell]

Interrupts are things generated by gpios - pinctrl has no part in it, except to possibly prevent multiple uses of the same pin.

You should be able to drop edt_ft5x06_pins completely.

[6by9]

One last update to add a rotate override in the same way as vc4-kms-dpi-[panel|generic] support.

I've just noticed that it doesn't work quite as well as one might have hoped, but that's a framework thing.
For rotate=180, fbdev sets rotate 180 on the plane within KMS (it can't do transpose). All good.
If your then start X, it notices the rot180 required as well, and configures xrandr to do it (via GL), but does NOT clear the rotation within KMS. Your desktop is therefore back to where you started. Doh!
X should ideally use KMS plane rotations where possible, or at least clear them.

Tested with all 3 panels. Video works on all 3. Touch on 4 and 4Square.

All happy now?

[Gadgetoid]

I don't know where to raise this, so in the hope I get some general prodding in the right direction- I have discovered the switch from Pi's homegrown setup to DRM/KMS has some side-effects when dpms kicks in.

In short, turning off the DPI signals to the display (in this case HyperPixel 4 Square) for any duration will "upset" the display proportional to that duration. The longer it's left on without a signal, the more extreme flickering and smearing issues will appear when it comes back up.

I made some effort to fix this in the driver before I realized that our SPI/MIPI signal is not connected- so it's impossible to sleep or turn off the display (to correspond with power management) and try to combat the flicker.

Does anyone know if it's possible to prevent dpms from disabling the DPI signals, preferring instead to keep them running and only affect the backlight? I've looked through the docs for DRM connector and various associated structs and flags, but I haven't managed to turn anything up.

Note: This can be "fixed" by disabling dpms "xset -dpms" and just controlling the backlight via /sys/class/backlight/backlight/bl_power but this is rather sub-optimal.

Thank you!

[6by9]

I don't know where to raise this, so in the hope I get some general prodding in the right direction- I have discovered the switch from Pi's homegrown setup to DRM/KMS has some side-effects when dpms kicks in.

Could you raise this a a new issue on this repo please? It'll get lost against a closed merge request.