[PATCH 07/11] drm/vc4: hdmi: Disable connector status polling during suspend

[Dave Stevenson]

Hi Stefan

On Wed, 3 Jul 2024 at 16:32, Stefan Wahren <wahrenst at gmx.net> wrote:
>
> Am 03.07.24 um 12:28 schrieb Stefan Wahren:
> > Hi Maxime,
> >
> > Am 02.07.24 um 15:48 schrieb Maxime Ripard:
> >> Hi,
> >>
> >> On Sun, Jun 30, 2024 at 05:36:48PM GMT, Stefan Wahren wrote:
> >>> Suspend of VC4 HDMI will likely triggers a warning from
> >>> vc4_hdmi_connector_detect_ctx() during poll of connector status.
> >>> The power management will prevent the resume and keep the relevant
> >>> power domain disabled.
> >>>
> >>> Since there is no reason to poll the connector status during
> >>> suspend, the polling should be disabled during this.
> >>>
> >>> It not possible to use drm_mode_config_helper_suspend() here,
> >>> because the callbacks might be called during bind phase and not all
> >>> components are fully initialized.
> >>>
> >>> Link:
> >>> https://lore.kernel.org/dri-devel/7003512d-7303-4f41-b0d6-a8af5bf8e497@gmx.net/
> >>> Signed-off-by: Stefan Wahren <wahrenst at gmx.net>
> >>> ---
> >>>   drivers/gpu/drm/vc4/vc4_hdmi.c | 11 +++++++++++
> >>>   1 file changed, 11 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c
> >>> b/drivers/gpu/drm/vc4/vc4_hdmi.c
> >>> index b3a42b709718..e80495cea6ac 100644
> >>> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> >>> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> >>> @@ -3106,6 +3106,13 @@ static int vc5_hdmi_init_resources(struct
> >>> drm_device *drm,
> >>>   static int vc4_hdmi_runtime_suspend(struct device *dev)
> >>>   {
> >>>       struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
> >>> +    struct drm_device *drm = vc4_hdmi->connector.dev;
> >>> +
> >>> +    /*
> >>> +     * Don't disable polling if it was never initialized
> >>> +     */
> >>> +    if (drm && drm->mode_config.poll_enabled)
> >>> +        drm_kms_helper_poll_disable(drm);
> >> Does it make sense to add it to runtime_suspend?
> > i saw that other drm drivers used drm_mode_config_helper_suspend() in
> > the RUNTIME_PM_OPS. But i agree, it should be better moved to
> > SYSTEM_SLEEP_PM_OPS.
> >> What if the board boots without a display connected, and only after a
> >> while one is connected? Wouldn't that prevent the driver from detecting
> >> it?
> > tbh I noticed that HDMI re-detection didn't worked in my setup
> > 6.10-rcX before this series. I need to investigate ...
> Okay, this patch breaks definitely HDMI re-detection. So please ignore
> this patch. Sorry, about this mess.
>
> There is another minor issue which already exists before that cause the
> following log entry on HDMI reconnect:
>
> [   74.078745] vc4-drm soc:gpu: [drm] User-defined mode not supported:
> "1920x1200": 60 154000 1920 1968 2000 2080 1200 1203 1209 1235 0x68 0x9
>
> But in this case HDMI works.

That's saying the mode specified on the kernel command line via
"video=" is invalid. All other modes enumerated from the EDID should
still be present.

I don't immediately see anything wrong with the mode - it's just DMT
mode 0x44 aka 1920x1200 at 60Hz with reduced blanking. 154MHz clock is
less than the 162MHz limit for Pi0-3 so should be supported.
Setting the module parameter drm.debug to 0x4 should give you the
extra debug of the reason the mode was rejected via the log message at
[1].

At a guess the firmware has inserted the video= line based on the mode
it configured. Whilst that was useful, we've moved away from that now
by setting "disable_fw_kms_setup=1" in config.txt.

  Dave

[1] https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/drm_modes.c#L1815-L1816

> Regards
> >>
> >> Maxime
> >
>