On Mon, 22 Jun 2026, Thomas Zimmermann <[email protected]> wrote:
> Only synchronize fbdev output to the vblank of an active CRTC. Go over
> the list of CRTCs and pick the first that matches. Fixes warnings as
> the one shown below
>
> [ 77.201354] WARNING: drivers/gpu/drm/drm_vblank.c:1320 at
> drm_crtc_wait_one_vblank+0x194/0x1cc [drm], CPU#1: kworker/1:7/1867
> [ 77.201354] omapdrm omapdrm.0: [drm] vblank wait timed out on crtc 0
>
> This currently happens if the fbdev output is not on CRTC 0.
>
> Atomic and non-atomic drivers require distinct code paths. As for other
> fbdev operations, implement both and select the correct one at runtime.
>
> Not finding an active CRTC is not a bug. Do not wait in this case, but
> flush the display update as before.
>
> v2:
> - move look-up code into separate helper
> - support drivers with legacy modesetting
> v1:
> - see
> https://lore.kernel.org/dri-devel/[email protected]/
>
> Co-authored-by: H. Nikolaus Schaller <[email protected]>
> Signed-off-by: Thomas Zimmermann <[email protected]>
> Fixes: d8c4bddcd8bcb ("drm/fb-helper: Synchronize dirty worker with vblank")
> Tested-by: Icenowy Zheng <[email protected]>
> Closes: https://bugs.debian.org/1138033
> Cc: <[email protected]> # v6.19+
> ---
> drivers/gpu/drm/drm_fb_helper.c | 71 ++++++++++++++++++++++++++++++++-
> 1 file changed, 70 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 7b11a582f8ec..cbf0a9a7b8e5 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -225,16 +225,85 @@ static void drm_fb_helper_resume_worker(struct
> work_struct *work)
> console_unlock();
> }
>
> +static int find_crtc_index_atomic(struct drm_fb_helper *helper)
> +{
> + struct drm_device *dev = helper->dev;
> + struct drm_plane *plane;
> +
> + drm_for_each_plane(plane, dev) {
> + const struct drm_plane_state *plane_state;
> + const struct drm_crtc *crtc;
> +
> + if (plane->type != DRM_PLANE_TYPE_PRIMARY)
> + continue;
> +
> + plane_state = plane->state;
> + if (plane_state->fb != helper->fb || !plane_state->crtc)
> + continue; /* plane doesn't display fbdev emulation */
> +
> + crtc = plane_state->crtc;
> + if (!crtc->state->active)
> + continue;
> + if (drm_WARN_ON_ONCE(dev, crtc->index > INT_MAX))
> + continue; /* driver bug */
I take it this is here because crtc->index is unsigned, and this
function returns int so you can have negative error codes. Ditto the
other function below.
I feel like this is something that should be checked once somewhere, if
that. I think adding arbitrary checks like this invites more arbitrary
checks everywhere. crtc->index is supposed to be invariant over the
lifetime of the CRTC.
BR,
Jani.
> +
> + return crtc->index;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int find_crtc_index_legacy(struct drm_fb_helper *helper)
> +{
> + struct drm_device *dev = helper->dev;
> + struct drm_crtc *crtc;
> +
> + drm_for_each_crtc(crtc, dev) {
> + struct drm_plane *plane = crtc->primary;
> +
> + if (!crtc->enabled)
> + continue;
> + if (!plane || plane->fb != helper->fb)
> + continue; /* CRTC doesn't display fbdev emulation */
> + if (drm_WARN_ON_ONCE(dev, crtc->index > INT_MAX))
> + continue; /* driver bug */
> +
> + return crtc->index;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int drm_fb_helper_find_crtc_index(struct drm_fb_helper *helper)
> +{
> + struct drm_device *dev = helper->dev;
> + int crtc_index;
> +
> + mutex_lock(&dev->mode_config.mutex);
> +
> + if (drm_drv_uses_atomic_modeset(dev))
> + crtc_index = find_crtc_index_atomic(helper);
> + else
> + crtc_index = find_crtc_index_legacy(helper);
> +
> + mutex_unlock(&dev->mode_config.mutex);
> +
> + return crtc_index;
> +}
> +
> static void drm_fb_helper_fb_dirty(struct drm_fb_helper *helper)
> {
> struct drm_device *dev = helper->dev;
> struct drm_clip_rect *clip = &helper->damage_clip;
> struct drm_clip_rect clip_copy;
> + int crtc_index;
> unsigned long flags;
> int ret;
>
> mutex_lock(&helper->lock);
> - drm_client_modeset_wait_for_vblank(&helper->client, 0);
> + crtc_index = drm_fb_helper_find_crtc_index(helper);
> + if (crtc_index >= 0)
> + drm_client_modeset_wait_for_vblank(&helper->client, crtc_index);
> mutex_unlock(&helper->lock);
>
> if (drm_WARN_ON_ONCE(dev, !helper->funcs->fb_dirty))
--
Jani Nikula, Intel
