drm/kms: DPI signals disabled when dpms sleeps display.

Gadgetoid commented 2 years ago

Describe the bug

I have discovered the switch from Pi's homegrown setup to DRM/KMS has some side-effects/regressions when dpms kicks in.

DRM/KMS turns off the DPI signals to the display (in this case HyperPixel 4 Square) when sleeping. Doing this 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. Previously this was a non-issue, since the backlight would be turned off but the display would continue to be updated.

Is it 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.

Steps to reproduce the behaviour

Grab a Pi 4 + HyperPixel 4 Square
Install Raspberry Pi OS
Add dtoverlay=vc4-kms-dpi-hyperpixel4sq to /boot/config.txt
Either leave it to sleep, or xset dpms force standby
Leave for a few minutes
Wake to observe issues with display

Device (s)

Raspberry Pi 4 Mod. B

System

cat /etc/rpi-issue:

Raspberry Pi reference 2022-04-04
Generated using pi-gen, https://github.com/RPi-Distro/pi-gen, 226b479f8d32919c9fe36dd5b4c20c02682f8180, stage4

vcgencmd version:

Aug  9 2022 13:44:40 
Copyright (c) 2012 Broadcom
version 273b410636cf8854ca35af91fd738a3d5f8b39b6 (clean) (release) (start)

uname -a:

Linux inkypi 5.15.56-v7l+ #1575 SMP Fri Jul 22 20:29:46 BST 2022 armv7l GNU/Linux

Logs

No response

Additional context

No response

6by9 commented 2 years ago

SPI/MIPI signal is not connected

That's a bit of a pain, but I see that's in the screen and not your adapter board. I won't ask how much time was spent developing the initialisation script for that board when it doesn't do anything.

Reproduced but using xrandr --output DPI-1 --off and xrandr --output DPI-1 --auto --right-of HDMI-1. It does produce a rather ugly flicker on the display.

I don't know of a way of forcing the clock to stay enabled as the whole pipeline gets torn down from HVS, via pixel valve, to DPI. DPI is going to be the one producing the clock, predominantly from having the pixel clock enabled (clk_set_rate and clk_prepare_enable calls in https://github.com/raspberrypi/linux/blob/rpi-5.15.y/drivers/gpu/drm/vc4/vc4_dpi.c#L236) and DPI_C having DPI_ENABLE enabled. Looking at vc4_dpi_encoder_disable it only stops the clock, leaving the DPI block enabled - quirky.

I don't know whether the DPI block is going to be happy if the clock is running when the upstream PV is disabled, but it looks like it may. It does then becomes a bit messy should the display support multiple resolutions and need to change the pixel clock rate in the process - you need to disable the clock before altering the rate and enabling again. All that is possible, and can be done by adding an extra flag to device tree for the DPI block (there's little point in trying to make it a generic flag for the panel as this isn't the fault of the panel per se).

Gadgetoid commented 1 year ago

I've just been reminded of this issue by a deluge of display flickering reports :grimacing:

Could you possibly walk me through what's required to add a flag to the DPI block? I'm quite thoroughly out of my depth here.

Another approach could potentially be to tweak dpms such that it doesn't affect the display, but controls only the backlight. But- I guess that pretty much amounts to what you're proposing. In our case at least the display is a single, fixed resolution but I appreciate that this isn't a particularly common problem or generic fix.

Thanks!

6by9 commented 1 year ago

As a test try removing the clk_disable_unprepare(dpi->pixel_clock); at https://github.com/raspberrypi/linux/blob/rpi-6.1.y/drivers/gpu/drm/vc4/vc4_dpi.c#L132

You also need a conditional around

    ret = clk_set_rate(dpi->pixel_clock, mode->clock * 1000);
    if (ret)
        DRM_ERROR("Failed to set clock rate: %d\n", ret);

    ret = clk_prepare_enable(dpi->pixel_clock);
    if (ret)
        DRM_ERROR("Failed to set clock rate: %d\n", ret);

at https://github.com/raspberrypi/linux/blob/rpi-6.1.y/drivers/gpu/drm/vc4/vc4_dpi.c#L240-L246 so that it is only called once, not every time through. (You can't change an enabled clock, and prepare and enable are reference counted). If mode->clock changes, then you'll need to do a clk_disable_unprepare, clk_set_rate, and then clk_prepare_enable.

(Presumably the display also gets upset from power up until the DPI clock is present. A more complete and simple fix may be to enable dpi->pixel_clock from _bind, and then _enable is always the set of 3 clock operations should the clock rate change).

raspberrypi / linux