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

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

[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: 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.
 	 */
+	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);
 
-- 
2.52.0

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

[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.

(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

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@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

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

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

[Maarten Lankhorst]

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

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

[Icenowy Zheng]

在 2026-05-22五的 11:23 +0200，Maarten Lankhorst写道：
> 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.

Well the problem here seems to be a CRTC is active, but it's not CRTC 0
(for embedded hardwares CRTCs could be somehow fixed and tied to
physical ports); and the fbdev helpers hardcode 0 as the CRTC ID.

Thanks,
Icenowy

> 
> 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

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

[Thomas Zimmermann]

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: 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.
>   	 */
> +	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.

>   	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.

Best regards
Thomas



>   }
>   EXPORT_SYMBOL(drm_client_modeset_wait_for_vblank);
>   

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

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

[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: 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.
> >   	 */
> > +	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.

> 
> >   	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".

-- 
Ville Syrjälä
Intel

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

[Thomas Zimmermann]

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: 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.
>>>    	 */
>>> +	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.  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)

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

[Ville Syrjälä]

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: 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.
> >>>    	 */
> >>> +	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

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

[Thomas Zimmermann]

Hi

Am 22.05.26 um 15:28 schrieb Ville Syrjälä:
[...]
>>>> 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.

I mean that the connected outputs could change at a later point or we 
could have multiple CRTCs in use. Today, someone in #intel-gfx reported 
a problem with panning if multiple CRTCs are in use.

Therefore picking a CRTC freely could be a problem. Let's say we 
configure modes from one CRTC, but later wait/pan/flush with another 
CRTC. I would not trust this to work correctly.

Hence, my suggestion is to select a primary CRTC during the fbdev 
client's probe and use it for all later operations until the next probe 
happens.  All other CRTCs would mirror the primary one.

Best regards
Thomas


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

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

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

[Icenowy Zheng]

在 2026-05-22五的 15:43 +0200，Thomas Zimmermann写道：
> Hi
> 
> Am 22.05.26 um 15:28 schrieb Ville Syrjälä:
> [...]
> > > > > 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.

Yes, for many embedded display solutions the CRTC-connector map is
totally fixed.

> > > > 
> > > > 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.
> 
> I mean that the connected outputs could change at a later point or we
> could have multiple CRTCs in use. Today, someone in #intel-gfx
> reported 
> a problem with panning if multiple CRTCs are in use.
> 
> Therefore picking a CRTC freely could be a problem. Let's say we 
> configure modes from one CRTC, but later wait/pan/flush with another 
> CRTC. I would not trust this to work correctly.
> 
> Hence, my suggestion is to select a primary CRTC during the fbdev 
> client's probe and use it for all later operations until the next
> probe 
> happens.  All other CRTCs would mirror the primary one.

What will happen if the "primary CRTC" is then disabled because of no
connected connectors can be driven with it?

Thanks,
Icenowy

> 
> Best regards
> Thomas
> 
> 
> > 
> > > 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)
> > > 

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

[Thomas Zimmermann]

Hi

Am 22.05.26 um 16:01 schrieb Icenowy Zheng:
> 在 2026-05-22五的 15:43 +0200，Thomas Zimmermann写道：
>> Hi
>>
>> Am 22.05.26 um 15:28 schrieb Ville Syrjälä:
>> [...]
>>>>>> 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.
> Yes, for many embedded display solutions the CRTC-connector map is
> totally fixed.
>
>>>>> 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.
>> I mean that the connected outputs could change at a later point or we
>> could have multiple CRTCs in use. Today, someone in #intel-gfx
>> reported
>> a problem with panning if multiple CRTCs are in use.
>>
>> Therefore picking a CRTC freely could be a problem. Let's say we
>> configure modes from one CRTC, but later wait/pan/flush with another
>> CRTC. I would not trust this to work correctly.
>>
>> Hence, my suggestion is to select a primary CRTC during the fbdev
>> client's probe and use it for all later operations until the next
>> probe
>> happens.  All other CRTCs would mirror the primary one.
> What will happen if the "primary CRTC" is then disabled because of no
> connected connectors can be driven with it?

This happens during a client hotplug event. We'd re-detect all 
connectors, pick the CRTC/output with the lowest spec as new primary and 
mirror it to all other connected outputs.

Best regards
Thomas

>
> Thanks,
> Icenowy
>
>> Best regards
>> Thomas
>>
>>
>>>> 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)
>>>>

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

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

[Ville Syrjälä]

On Fri, May 22, 2026 at 03:43:26PM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 22.05.26 um 15:28 schrieb Ville Syrjälä:
> [...]
> >>>> 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.
> 
> I mean that the connected outputs could change at a later point or we 
> could have multiple CRTCs in use. Today, someone in #intel-gfx reported 
> a problem with panning if multiple CRTCs are in use.
> 
> Therefore picking a CRTC freely could be a problem. Let's say we 
> configure modes from one CRTC, but later wait/pan/flush with another 
> CRTC. I would not trust this to work correctly.
> 
> Hence, my suggestion is to select a primary CRTC during the fbdev 
> client's probe and use it for all later operations until the next probe 
> happens.  All other CRTCs would mirror the primary one.

Actual mirroring may not be possible due to different modes supported
on each output. The whole multi-output fbdev thing in the drm fb helper
is kind of a hack that's rather hard to make work 100% sensibly.

For the panning possibly the only sensible thing is to use the max of
hdisplay/vdisplay of all the crtcs as the xres/yres so it's clear
how much things can actually be panned. Oh and tiled displays (assuming
we would actually want the fbdev stuff to tile correctly) make the
situation even more complicated. I think the current support for tiled
displays in the fb helper is semi-busted.

> Best regards
> Thomas
> 
> 
> >
> >> 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)
> >>
> 
> -- 
> --
> 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