On Fri, May 22, 2026 at 03:24:05PM +0200, Thomas Zimmermann wrote: > Hi > > Am 22.05.26 um 15:13 schrieb Ville Syrjälä: > > On Fri, May 22, 2026 at 01:55:59PM +0200, Thomas Zimmermann wrote: > >> Hi > >> > >> Am 19.05.26 um 11:24 schrieb Icenowy Zheng: > >>> Currently the implementaion of drm_client_modeset_wait_for_vblank() > >>> assumes drm_vblank_get() will fail when the CRTC isn't active. However > >>> it seems that this is not true, and running fbcon on a device with the > >>> first CRTC inactive will lead to kernel warning in some cases (which > >>> could be reproduced with the loongson driver). > >>> > >>> Change the implementation to add a check for the active state (atomic) / > >>> enabled state (non-atomic) before calling drm_vblank_get(). As the > >>> assumption of drm_vblank_get() failing for inactive CRTC isn't met, the > >>> error status of drm_vblank_get() can now be exported too. > >>> > >>> Cc: [email protected] > >>> Fixes: d8c4bddcd8bc ("drm/fb-helper: Synchronize dirty worker with > >>> vblank") > >>> Signed-off-by: Icenowy Zheng <[email protected]> > >>> --- > >>> drivers/gpu/drm/drm_client_modeset.c | 13 +++++++++++-- > >>> 1 file changed, 11 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/drm_client_modeset.c > >>> b/drivers/gpu/drm/drm_client_modeset.c > >>> index bb49b8361271a..1b03bf351256e 100644 > >>> --- a/drivers/gpu/drm/drm_client_modeset.c > >>> +++ b/drivers/gpu/drm/drm_client_modeset.c > >>> @@ -1310,7 +1310,7 @@ int drm_client_modeset_wait_for_vblank(struct > >>> drm_client_dev *client, unsigned i > >>> { > >>> struct drm_device *dev = client->dev; > >>> struct drm_crtc *crtc; > >>> - int ret; > >>> + int ret = 0; > >>> > >>> /* > >>> * Rate-limit update frequency to vblank. If there's a DRM > >>> master > >>> @@ -1326,15 +1326,24 @@ int drm_client_modeset_wait_for_vblank(struct > >>> drm_client_dev *client, unsigned i > >>> * Only wait for a vblank event if the CRTC is enabled, > >>> otherwise > >>> * just don't do anything, not even report an error. > >>> */ > >>> + if (drm_drv_uses_atomic_modeset(dev)) { > >>> + if (!crtc->state || !crtc->state->active) > >>> + goto out; > >>> + } else { > >>> + if (!crtc->enabled) > >>> + goto out; > >>> + } > >>> + > >> This part is good. > > Locking is missing. > > Ok > > > > >>> ret = drm_crtc_vblank_get(crtc); > >>> if (!ret) { > >>> drm_crtc_wait_one_vblank(crtc); > >>> drm_crtc_vblank_put(crtc); > >>> } > >>> > >>> +out: > >>> drm_master_internal_release(dev); > >>> > >>> - return 0; > >>> + return ret; > >> But this isn't. There can be CRTCs without any vblank at all. We still > >> want to fail silently for them. So we still have to return 0 here. > >> > >> Having set this, fixing this helper is only partially what you want. > >> Since your device has vblanking, the emulation should check on the > >> correct CRTC. IOW you need to pass the right CRTC index at > >> > >> https://elixir.bootlin.com/linux/v7.1-rc1/source/drivers/gpu/drm/drm_fb_helper.c#L237 > >> https://elixir.bootlin.com/linux/v7.1-rc1/source/drivers/gpu/drm/drm_fb_helper.c#L920 > >> > >> I'm not quite sure how to support this. The CRTC is under > >> fb_helper->client.modesets.crtc. You'd have to figure out which is the > >> relevant one and use that. But that's also not so great, as fbdev ioctls > >> only support CRTC 0. Doing internal re-mappings only complicates matters. > >> > >> But why does your HW use CRTC 1 in the first place. > > Could be eg. the enabled outputs can't be driven with CRTC 0. > > > > I guess what you want to do is pick the first crtc from modesets[] > > which is enabled. Or perhaps even "pick the Nth enabled crtc from > > modesets[] based on the ioctl argument". > > The enable-status of each CRTC could change later on, which might lead > to problems.
Sound like a locking issue if someone is changing the configuration at the same time we're trying to do the vblank wait here. > Picking the one CRTC/output with the lowest spec and > mirroring it to the others might work. This CRTC would then be the one > to wait for. > > Best regards > Thomas > > > > > -- > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com > GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg) > -- Ville Syrjälä Intel
