vc4: HDMI Sink doesn't support RGB, something's wrong.

[Stefan Wahren]

Am 17.10.24 um 18:37 schrieb Dave Stevenson:
> On Thu, 17 Oct 2024 at 16:59, Maxime Ripard <mripard at kernel.org> wrote:
>> On Thu, Oct 17, 2024 at 05:26:46PM GMT, Stefan Wahren wrote:
>>> Am 17.10.24 um 16:27 schrieb Maxime Ripard:
>>>> On Wed, Oct 16, 2024 at 07:16:43PM GMT, Dave Stevenson wrote:
>>>>> Hi Stefan
>>>>>
>>>>> On Tue, 15 Oct 2024 at 22:13, Stefan Wahren <wahrenst at gmx.net> wrote:
>>>>>> Hi Dave,
>>> ...
>>>>>> i prepared a branch for you, which contains the latest suspend2idle patches:
>>>>>>
>>>>>> https://github.com/lategoodbye/linux-dev/commits/v6.12-pm/
>>>>>>
>>>>>> Steps:
>>>>>> 1. Flash latest Raspberry Pi OS (32 bit) on SD card
>>>>>> 2. Build Kernel from repo above with arm/multi_v7_defconfig
>>>>>> 3. Replace Kernel, modules + DTB on SD card with build ones
>>>>>> 4. add the following to confix.txt
>>>>>> device_tree=bcm2837-rpi-3-b-plus.dtb
>>>>>> enable_uart=1
>>>>>> 5. change/add the following to cmdline.txt
>>>>>> console=ttyS1,115200
>>>>>> no_console_suspend=1
>>>>>> 6. connect the following devices to Raspberry Pi 3 B+ :
>>>>>> USB mouse
>>>>>> USB keyboard
>>>>>> HDMI monitor
>>>>>> Debug UART adapter (USB side to PC)
>>>>>> 7. Power on board and boot into X11
>>>>>> 8. Change to root
>>>>>> 9. Enable wakeup for ttyS1
>>>>> So I remember for next time
>>>>> echo enabled > /sys/class/tty/ttyS1/power/wakeup
>>>>>
>>>>>> 10. Trigger suspend to idle via X11 (echo freeze > /sys/power/state)
>>>>>> 11. Wakeup Raspberry Pi via Debug UART
>>>>> I don't get the error you are seeing, but I also don't get the display resuming.
>>>>> pm has obviously killed the power to the HDMI block, and it has the
>>>>> reset values in as can be seen via /sys/kernel/debug/dri/0/hdmi_regs.
>>>>> Nothing in the driver restores these registers, and I'm not sure if it
>>>>> is meant to do so.
>>>>> Run kmstest or similar from this state and the change of mode
>>>>> reprogrammes the blocks so we get the display back again.
>>>>>
>>>>> I've also enabled CONFIG_DRM_LOAD_EDID_FIRMWARE so that I can use your
>>>>> EDID, and get the same results.
>>>>>
>>>>> Knee-capping the HDMI block on suspend seems an unlikely mechanism to
>>>>> work reliably. On the more recent Pis there is a need to be quite
>>>>> careful in disabling the pipeline to avoid getting data stuck in
>>>>> FIFOs.
>>>>> I feel I must be missing something here.
>>>> I think we're probably missing calls to drm_mode_config_helper_suspend and
>>>> drm_mode_config_helper_resume.
>>> Okay, i tried this and it works better (HDMI resumes fast), but it also
>>> triggers a lot of WARN
>> vc4_plane_reset deviates from the helper there:
>> https://elixir.bootlin.com/linux/v6.11.3/source/drivers/gpu/drm/drm_atomic_state_helper.c#L326
>>
>> We should adjust it.
> Yes, it looks like that WARN is inappropriate, and we should be
> freeing the old state.
>
>>> and the "doesn't support RGB ..." warnings are still there.
>>>
>>> I pushed my changes to the branch and attached the dmesg output.
>> I can't help you there, it doesn't make sense to me. The EDID should be correct.
> Nor can I. I've just taken the latest branch and HDMI does resume
> correctly after suspend now.
No problem. At the end I just wanted to know if the warning was related
to the problem that HDMI doesn't resume. Now it's clear these are not
related and I can investigate further.
> We have seen monitors that do weird things on HPD when they stop
> getting video and go into standby mode, so I wonder if that is the
> case with your monitor.
>
> I do wonder if the HDMI part of the display is the correct place to
> handle drm_mode_config_helper_[suspend|resume]. All other users seem
> to do it at the base DRM driver level, which would be vc4_drv.c.
> I've done that and pushed it to
> https://github.com/6by9/linux/tree/lategoodbye-suspend. That also
> works for me without your changes to the HDMI side. That branch also
> includes the above fix to vc4_plane_reset too.
Yes, that's a better place. Nice, thank you.
>
>    Dave