Hi Stefan On Wed, 3 Jul 2024 at 16:32, Stefan Wahren <wahrenst@xxxxxxx> 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@xxxxxxx/ > >>> Signed-off-by: Stefan Wahren <wahrenst@xxxxxxx> > >>> --- > >>> 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@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 > > >