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

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Hey,

Den 2026-05-19 kl. 13:29, skrev Icenowy Zheng:
> 在 2026-05-19二的 12:41 +0300，Jani Nikula写道：
>> On Tue, 19 May 2026, Icenowy Zheng <zhengxingda@iscas.ac.cn> 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@vger.kernel.org
>>> Fixes: d8c4bddcd8bc ("drm/fb-helper: Synchronize dirty worker with
>>> vblank")
>>> Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
>>> ---
>>>  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
I would rather have expected drm_fb_helper_ioctl to fail like you mention.
Probably needs a fbcon_is_active() there to prevent it.

The damage helper should not be triggered if no CRTC is active, so that means
the check here is slightly too late.

Can you fix it at a different level, like damage helper or its callers instead?

I believe when the client gets suspended, all the pending damage is flushed before
suspend.

Kind regards,
~Maarten Lankhorst