On Tue, 19 May 2026, Icenowy Zheng <[email protected]> wrote:
> 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.
>        */

I'll dodge the question whether the change below is right or not, but
for sure the comment above needs to be amended to match the change.

(Please wait for other review comments before sending another version
with the comment changed.)

BR,
Jani.

> +     if (drm_drv_uses_atomic_modeset(dev)) {
> +             if (!crtc->state || !crtc->state->active)
> +                     goto out;
> +     } else {
> +             if (!crtc->enabled)
> +                     goto out;
> +     }
> +
>       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;
>  }
>  EXPORT_SYMBOL(drm_client_modeset_wait_for_vblank);

-- 
Jani Nikula, Intel

Reply via email to