[PATCH v2] drm/fb-helper: Only consider active CRTCs for vblank sync

[PATCH v2] drm/fb-helper: Only consider active CRTCs for vblank sync

[Thomas Zimmermann]

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/1c9e0e24-9c4a-4259-8700-cf9e5fd60ca3@suse.de/

Co-authored-by: H. Nikolaus Schaller <hns@goldelico.com>
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: d8c4bddcd8bcb ("drm/fb-helper: Synchronize dirty worker with vblank")
Tested-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
Closes: https://bugs.debian.org/1138033
Cc: <stable@vger.kernel.org> # 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 */
+
+		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))
-- 
2.54.0

Re: [PATCH v2] drm/fb-helper: Only consider active CRTCs for vblank sync

[Jani Nikula]

On Mon, 22 Jun 2026, Thomas Zimmermann <tzimmermann@suse.de> 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/1c9e0e24-9c4a-4259-8700-cf9e5fd60ca3@suse.de/
>
> Co-authored-by: H. Nikolaus Schaller <hns@goldelico.com>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: d8c4bddcd8bcb ("drm/fb-helper: Synchronize dirty worker with vblank")
> Tested-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
> Closes: https://bugs.debian.org/1138033
> Cc: <stable@vger.kernel.org> # 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

Re: [PATCH v2] drm/fb-helper: Only consider active CRTCs for vblank sync

[Thomas Zimmermann]

Hi

Am 22.06.26 um 15:34 schrieb Jani Nikula:
> On Mon, 22 Jun 2026, Thomas Zimmermann <tzimmermann@suse.de> 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/1c9e0e24-9c4a-4259-8700-cf9e5fd60ca3@suse.de/
>>
>> Co-authored-by: H. Nikolaus Schaller <hns@goldelico.com>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Fixes: d8c4bddcd8bcb ("drm/fb-helper: Synchronize dirty worker with vblank")
>> Tested-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
>> Closes: https://bugs.debian.org/1138033
>> Cc: <stable@vger.kernel.org> # 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.

Ok, makes sense.

Best regards
Thomas

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

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