* [PATCH RESEND] drm/udl: fail fast on stalled URB pool
@ 2026-10-28 6:13 oushixiong1025
2026-10-28 7:32 ` Thomas Zimmermann
0 siblings, 1 reply; 2+ messages in thread
From: oushixiong1025 @ 2026-10-28 6:13 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: Dave Airlie, Sean Paul, Maarten Lankhorst, Maxime Ripard,
David Airlie, Simona Vetter, dri-devel, linux-kernel,
Shixiong Ou
From: Shixiong Ou <oushixiong@kylinos.cn>
A stalled device never returns its URBs, so every allocation waited
out the full timeout and stalled the display stack on each operation.
Flag the stall and fail fast until an URB comes back; give damage a
short timeout and keep the long one for modeset.
Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
---
drivers/gpu/drm/udl/udl_drv.h | 14 +++++++++++++-
drivers/gpu/drm/udl/udl_main.c | 30 +++++++++++++++++++++++++-----
drivers/gpu/drm/udl/udl_modeset.c | 6 +++---
drivers/gpu/drm/udl/udl_transfer.c | 2 +-
4 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 145bb95ccc48..43cff97e12f7 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -11,6 +11,7 @@
#ifndef UDL_DRV_H
#define UDL_DRV_H
+#include <linux/jiffies.h>
#include <linux/mm_types.h>
#include <linux/usb.h>
@@ -46,6 +47,7 @@ struct urb_list {
int available;
int count;
size_t size;
+ bool wedged;
};
struct udl_device {
@@ -72,7 +74,17 @@ static inline struct usb_device *udl_to_usb_device(struct udl_device *udl)
int udl_modeset_init(struct udl_device *udl);
struct drm_connector *udl_connector_init(struct drm_device *dev);
-struct urb *udl_get_urb(struct udl_device *udl);
+/*
+ * How long to wait for a free URB before giving up. The damage path runs
+ * on every atomic update, so keep its timeout short: a stalled device never
+ * returns its URBs and the wait would otherwise stall the whole display
+ * stack on every operation. Modeset is low-frequency and worth waiting
+ * longer for. Once the pool is flagged wedged, neither path blocks at all.
+ */
+#define UDL_GET_URB_TIMEOUT_DAMAGE msecs_to_jiffies(100)
+#define UDL_GET_URB_TIMEOUT_MODESET msecs_to_jiffies(2000)
+
+struct urb *udl_get_urb(struct udl_device *udl, long timeout);
int udl_submit_urb(struct udl_device *udl, struct urb *urb, size_t len);
void udl_sync_pending_urbs(struct udl_device *udl);
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index 17950fe3a0ec..b92cd5300030 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -169,6 +169,7 @@ void udl_urb_completion(struct urb *urb)
spin_lock_irqsave(&udl->urbs.lock, flags);
list_add_tail(&unode->entry, &udl->urbs.list);
udl->urbs.available++;
+ udl->urbs.wedged = false;
spin_unlock_irqrestore(&udl->urbs.lock, flags);
wake_up(&udl->urbs.sleep);
@@ -213,6 +214,7 @@ static int udl_alloc_urb_list(struct udl_device *udl, int count, size_t size)
init_waitqueue_head(&udl->urbs.sleep);
udl->urbs.count = 0;
udl->urbs.available = 0;
+ udl->urbs.wedged = false;
retry:
udl->urbs.size = size;
@@ -265,16 +267,34 @@ static struct urb *udl_get_urb_locked(struct udl_device *udl, long timeout)
assert_spin_locked(&udl->urbs.lock);
+ /* A device that stopped completing transfers will never wake us up:
+ * fail fast instead of stalling callers for the full timeout on every
+ * call. Allocations for teardown (MAX_SCHEDULE_TIMEOUT) still wait,
+ * since the USB core returns all URBs on disconnect.
+ */
+ if (udl->urbs.wedged && timeout != MAX_SCHEDULE_TIMEOUT) {
+ if (list_empty(&udl->urbs.list))
+ return NULL;
+ goto take;
+ }
+
/* Wait for an in-flight buffer to complete and get re-queued */
if (!wait_event_lock_irq_timeout(udl->urbs.sleep,
!udl->urbs.count ||
!list_empty(&udl->urbs.list),
udl->urbs.lock, timeout)) {
- DRM_INFO("wait for urb interrupted: available: %d\n",
- udl->urbs.available);
+ if (timeout == MAX_SCHEDULE_TIMEOUT) {
+ DRM_INFO("wait for urb interrupted: available: %d\n",
+ udl->urbs.available);
+ } else {
+ udl->urbs.wedged = true;
+ drm_warn_ratelimited(&udl->drm,
+ "no URB returned within %u ms (available %d); device stalled, dropping frames\n",
+ jiffies_to_msecs(timeout), udl->urbs.available);
+ }
return NULL;
}
-
+take:
if (!udl->urbs.count)
return NULL;
@@ -285,12 +305,12 @@ static struct urb *udl_get_urb_locked(struct udl_device *udl, long timeout)
return unode->urb;
}
-struct urb *udl_get_urb(struct udl_device *udl)
+struct urb *udl_get_urb(struct udl_device *udl, long timeout)
{
struct urb *urb;
spin_lock_irq(&udl->urbs.lock);
- urb = udl_get_urb_locked(udl, HZ * 2);
+ urb = udl_get_urb_locked(udl, timeout);
spin_unlock_irq(&udl->urbs.lock);
return urb;
}
diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c
index 289711035b67..c907846048f8 100644
--- a/drivers/gpu/drm/udl/udl_modeset.c
+++ b/drivers/gpu/drm/udl/udl_modeset.c
@@ -218,7 +218,7 @@ static int udl_handle_damage(struct drm_framebuffer *fb,
return ret;
log_bpp = ret;
- urb = udl_get_urb(udl);
+ urb = udl_get_urb(udl, UDL_GET_URB_TIMEOUT_DAMAGE);
if (!urb)
return -ENOMEM;
cmd = urb->transfer_buffer;
@@ -342,7 +342,7 @@ static void udl_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atom
if (!drm_dev_enter(dev, &idx))
return;
- urb = udl_get_urb(udl);
+ urb = udl_get_urb(udl, UDL_GET_URB_TIMEOUT_MODESET);
if (!urb) {
drm_err_ratelimited(dev, "get urb failed when enabling crtc\n");
goto out;
@@ -377,7 +377,7 @@ static void udl_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_ato
if (!drm_dev_enter(dev, &idx))
return;
- urb = udl_get_urb(udl);
+ urb = udl_get_urb(udl, UDL_GET_URB_TIMEOUT_MODESET);
if (!urb)
goto out;
diff --git a/drivers/gpu/drm/udl/udl_transfer.c b/drivers/gpu/drm/udl/udl_transfer.c
index 7d670b3a5293..bfff17bba58d 100644
--- a/drivers/gpu/drm/udl/udl_transfer.c
+++ b/drivers/gpu/drm/udl/udl_transfer.c
@@ -202,7 +202,7 @@ int udl_render_hline(struct udl_device *udl, int log_bpp, struct urb **urb_ptr,
int ret = udl_submit_urb(udl, urb, len);
if (ret)
return ret;
- urb = udl_get_urb(udl);
+ urb = udl_get_urb(udl, UDL_GET_URB_TIMEOUT_DAMAGE);
if (!urb)
return -EAGAIN;
*urb_ptr = urb;
--
2.25.1
No virus found
Checked by Hillstone Network AntiVirus
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [PATCH RESEND] drm/udl: fail fast on stalled URB pool
2026-10-28 6:13 [PATCH RESEND] drm/udl: fail fast on stalled URB pool oushixiong1025
@ 2026-10-28 7:32 ` Thomas Zimmermann
0 siblings, 0 replies; 2+ messages in thread
From: Thomas Zimmermann @ 2026-10-28 7:32 UTC (permalink / raw)
To: oushixiong1025
Cc: Dave Airlie, Sean Paul, Maarten Lankhorst, Maxime Ripard,
David Airlie, Simona Vetter, dri-devel, linux-kernel,
Shixiong Ou
Hi
Am 28.10.26 um 07:13 schrieb oushixiong1025@163.com:
> From: Shixiong Ou <oushixiong@kylinos.cn>
>
> A stalled device never returns its URBs, so every allocation waited
> out the full timeout and stalled the display stack on each operation.
> Flag the stall and fail fast until an URB comes back; give damage a
> short timeout and keep the long one for modeset.
I'm not comfortable with merging this change. It's nigh impossible to
test. It could also break a working set up if there's some temporary
contention on the USB.
Best regards
Thomas
>
> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
> ---
> drivers/gpu/drm/udl/udl_drv.h | 14 +++++++++++++-
> drivers/gpu/drm/udl/udl_main.c | 30 +++++++++++++++++++++++++-----
> drivers/gpu/drm/udl/udl_modeset.c | 6 +++---
> drivers/gpu/drm/udl/udl_transfer.c | 2 +-
> 4 files changed, 42 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
> index 145bb95ccc48..43cff97e12f7 100644
> --- a/drivers/gpu/drm/udl/udl_drv.h
> +++ b/drivers/gpu/drm/udl/udl_drv.h
> @@ -11,6 +11,7 @@
> #ifndef UDL_DRV_H
> #define UDL_DRV_H
>
> +#include <linux/jiffies.h>
> #include <linux/mm_types.h>
> #include <linux/usb.h>
>
> @@ -46,6 +47,7 @@ struct urb_list {
> int available;
> int count;
> size_t size;
> + bool wedged;
> };
>
> struct udl_device {
> @@ -72,7 +74,17 @@ static inline struct usb_device *udl_to_usb_device(struct udl_device *udl)
> int udl_modeset_init(struct udl_device *udl);
> struct drm_connector *udl_connector_init(struct drm_device *dev);
>
> -struct urb *udl_get_urb(struct udl_device *udl);
> +/*
> + * How long to wait for a free URB before giving up. The damage path runs
> + * on every atomic update, so keep its timeout short: a stalled device never
> + * returns its URBs and the wait would otherwise stall the whole display
> + * stack on every operation. Modeset is low-frequency and worth waiting
> + * longer for. Once the pool is flagged wedged, neither path blocks at all.
> + */
> +#define UDL_GET_URB_TIMEOUT_DAMAGE msecs_to_jiffies(100)
> +#define UDL_GET_URB_TIMEOUT_MODESET msecs_to_jiffies(2000)
> +
> +struct urb *udl_get_urb(struct udl_device *udl, long timeout);
>
> int udl_submit_urb(struct udl_device *udl, struct urb *urb, size_t len);
> void udl_sync_pending_urbs(struct udl_device *udl);
> diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
> index 17950fe3a0ec..b92cd5300030 100644
> --- a/drivers/gpu/drm/udl/udl_main.c
> +++ b/drivers/gpu/drm/udl/udl_main.c
> @@ -169,6 +169,7 @@ void udl_urb_completion(struct urb *urb)
> spin_lock_irqsave(&udl->urbs.lock, flags);
> list_add_tail(&unode->entry, &udl->urbs.list);
> udl->urbs.available++;
> + udl->urbs.wedged = false;
> spin_unlock_irqrestore(&udl->urbs.lock, flags);
>
> wake_up(&udl->urbs.sleep);
> @@ -213,6 +214,7 @@ static int udl_alloc_urb_list(struct udl_device *udl, int count, size_t size)
> init_waitqueue_head(&udl->urbs.sleep);
> udl->urbs.count = 0;
> udl->urbs.available = 0;
> + udl->urbs.wedged = false;
>
> retry:
> udl->urbs.size = size;
> @@ -265,16 +267,34 @@ static struct urb *udl_get_urb_locked(struct udl_device *udl, long timeout)
>
> assert_spin_locked(&udl->urbs.lock);
>
> + /* A device that stopped completing transfers will never wake us up:
> + * fail fast instead of stalling callers for the full timeout on every
> + * call. Allocations for teardown (MAX_SCHEDULE_TIMEOUT) still wait,
> + * since the USB core returns all URBs on disconnect.
> + */
> + if (udl->urbs.wedged && timeout != MAX_SCHEDULE_TIMEOUT) {
> + if (list_empty(&udl->urbs.list))
> + return NULL;
> + goto take;
> + }
> +
> /* Wait for an in-flight buffer to complete and get re-queued */
> if (!wait_event_lock_irq_timeout(udl->urbs.sleep,
> !udl->urbs.count ||
> !list_empty(&udl->urbs.list),
> udl->urbs.lock, timeout)) {
> - DRM_INFO("wait for urb interrupted: available: %d\n",
> - udl->urbs.available);
> + if (timeout == MAX_SCHEDULE_TIMEOUT) {
> + DRM_INFO("wait for urb interrupted: available: %d\n",
> + udl->urbs.available);
> + } else {
> + udl->urbs.wedged = true;
> + drm_warn_ratelimited(&udl->drm,
> + "no URB returned within %u ms (available %d); device stalled, dropping frames\n",
> + jiffies_to_msecs(timeout), udl->urbs.available);
> + }
> return NULL;
> }
> -
> +take:
> if (!udl->urbs.count)
> return NULL;
>
> @@ -285,12 +305,12 @@ static struct urb *udl_get_urb_locked(struct udl_device *udl, long timeout)
> return unode->urb;
> }
>
> -struct urb *udl_get_urb(struct udl_device *udl)
> +struct urb *udl_get_urb(struct udl_device *udl, long timeout)
> {
> struct urb *urb;
>
> spin_lock_irq(&udl->urbs.lock);
> - urb = udl_get_urb_locked(udl, HZ * 2);
> + urb = udl_get_urb_locked(udl, timeout);
> spin_unlock_irq(&udl->urbs.lock);
> return urb;
> }
> diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c
> index 289711035b67..c907846048f8 100644
> --- a/drivers/gpu/drm/udl/udl_modeset.c
> +++ b/drivers/gpu/drm/udl/udl_modeset.c
> @@ -218,7 +218,7 @@ static int udl_handle_damage(struct drm_framebuffer *fb,
> return ret;
> log_bpp = ret;
>
> - urb = udl_get_urb(udl);
> + urb = udl_get_urb(udl, UDL_GET_URB_TIMEOUT_DAMAGE);
> if (!urb)
> return -ENOMEM;
> cmd = urb->transfer_buffer;
> @@ -342,7 +342,7 @@ static void udl_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atom
> if (!drm_dev_enter(dev, &idx))
> return;
>
> - urb = udl_get_urb(udl);
> + urb = udl_get_urb(udl, UDL_GET_URB_TIMEOUT_MODESET);
> if (!urb) {
> drm_err_ratelimited(dev, "get urb failed when enabling crtc\n");
> goto out;
> @@ -377,7 +377,7 @@ static void udl_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_ato
> if (!drm_dev_enter(dev, &idx))
> return;
>
> - urb = udl_get_urb(udl);
> + urb = udl_get_urb(udl, UDL_GET_URB_TIMEOUT_MODESET);
> if (!urb)
> goto out;
>
> diff --git a/drivers/gpu/drm/udl/udl_transfer.c b/drivers/gpu/drm/udl/udl_transfer.c
> index 7d670b3a5293..bfff17bba58d 100644
> --- a/drivers/gpu/drm/udl/udl_transfer.c
> +++ b/drivers/gpu/drm/udl/udl_transfer.c
> @@ -202,7 +202,7 @@ int udl_render_hline(struct udl_device *udl, int log_bpp, struct urb **urb_ptr,
> int ret = udl_submit_urb(udl, urb, len);
> if (ret)
> return ret;
> - urb = udl_get_urb(udl);
> + urb = udl_get_urb(udl, UDL_GET_URB_TIMEOUT_DAMAGE);
> if (!urb)
> return -EAGAIN;
> *urb_ptr = urb;
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri, (HRB 36809, AG Nürnberg)
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-28 7:32 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-28 6:13 [PATCH RESEND] drm/udl: fail fast on stalled URB pool oushixiong1025
2026-10-28 7:32 ` Thomas Zimmermann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®