Re: [PATCH] drm/client: check whether CRTC is active before waiting for vblank

[Icenowy Zheng]

在 2026-05-19二的 12:41 +0300，Jani Nikula写道：
> On Tue, 19 May 2026, Icenowy Zheng <zhengxingda@xxxxxxxxxxx> 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: stable@xxxxxxxxxxxxxxx
> > Fixes: d8c4bddcd8bc ("drm/fb-helper: Synchronize dirty worker with
> > vblank")
> > Signed-off-by: Icenowy Zheng <zhengxingda@xxxxxxxxxxx>
> > ---
> >  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.

If the change is right, it perfectly matches what the comment above is
saying -- it's the current behavior that does not match the comment.

Thanks,
Icenowy

> 
> (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);