Re: [PATCH] drm/udl: Increase get urb timeout for modeset

[Thomas Zimmermann]

Hi

Am 05.11.25 um 10:07 schrieb oushixiong:
If the wait times for all operations increase, it would likely cause 
significant blocking in the display process.

But we're usually not waiting until timeout, but only until the next urb 
becomes available again. And we have several in the queue pre-allocated. 
Increasing the timeout should therefore not be a problem.  The current 
drawing timeout of 1 second (== HZ) would already be a major issue 
otherwise.

Should we make a distinction between the two, or base it on what you 
said about increasing  the regular GET_URB_TIMEOUT for all operations ?

Please try to double the timeout.

(If this really doesn't work, we should try to drop the urb caching 
entirely and allocate urbs as we need them.)

Best regards
Thomas


Best regards
Shixiong

在 2025/11/5 16:57, Thomas Zimmermann 写道:
Hi

Am 05.11.25 um 09:30 schrieb oushixiong1...@163.com:
From: Shixiong Ou <oushixi...@kylinos.cn>

[WHY]
There is a situation where udl_handle_damage() was running successfully
but the screen was black. it was because 
udl_crtc_helper_atomic_enable() failed,
and there were no error messages.

[HOW]
The priority for mode settings needs to be higher than damage 
handle, requiring
a higher success rate than ordinary operations.
Increase get urb timeout for modeset.

Signed-off-by: Shixiong Ou <oushixi...@kylinos.cn>
---
  drivers/gpu/drm/udl/udl_drv.h      |  5 ++++-
  drivers/gpu/drm/udl/udl_main.c     |  5 ++---
  drivers/gpu/drm/udl/udl_modeset.c  | 11 +++++++----
  drivers/gpu/drm/udl/udl_transfer.c |  2 +-
  4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_drv.h 
b/drivers/gpu/drm/udl/udl_drv.h
index 145bb95ccc48..38b3bdf1ae4a 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -31,6 +31,9 @@ struct drm_mode_create_dumb;
  #define DRIVER_MINOR        0
  #define DRIVER_PATCHLEVEL    1
  +#define GET_URB_TIMEOUT    HZ
+#define MODESET_GET_URB_TIMEOUT    (HZ*2)
+

Just increase the regular GET_URB_TIMEOUT for all operations.

Best regards
Thomas

  struct udl_device;
    struct urb_node {
@@ -72,7 +75,7 @@ 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);
+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 bc58991a6f14..891996f0f74b 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -285,13 +285,12 @@ static struct urb *udl_get_urb_locked(struct 
udl_device *udl, long timeout)
      return unode->urb;
  }
  -#define GET_URB_TIMEOUT    HZ
-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, GET_URB_TIMEOUT);
+    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 231e829bd709..6adca5e3e471 100644
--- a/drivers/gpu/drm/udl/udl_modeset.c
+++ b/drivers/gpu/drm/udl/udl_modeset.c
@@ -21,6 +21,7 @@
  #include <drm/drm_gem_framebuffer_helper.h>
  #include <drm/drm_gem_shmem_helper.h>
  #include <drm/drm_modeset_helper_vtables.h>
+#include <drm/drm_print.h>
  #include <drm/drm_probe_helper.h>
  #include <drm/drm_vblank.h>
  @@ -217,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, GET_URB_TIMEOUT);
      if (!urb)
          return -ENOMEM;
      cmd = urb->transfer_buffer;
@@ -341,9 +342,11 @@ 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);
-    if (!urb)
+    urb = udl_get_urb(udl, MODESET_GET_URB_TIMEOUT);
+    if (!urb) {
+        DRM_ERROR("Udl get urb failed when enabling crtc");
          goto out;
+    }
        buf = (char *)urb->transfer_buffer;
      buf = udl_vidreg_lock(buf);
@@ -374,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, MODESET_GET_URB_TIMEOUT);
      if (!urb)
          goto out;
  diff --git a/drivers/gpu/drm/udl/udl_transfer.c 
b/drivers/gpu/drm/udl/udl_transfer.c
index 7d670b3a5293..858b47522d78 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, GET_URB_TIMEOUT);
              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: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)