[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <38110178-0f27-44ba-9925-5bbe74a1bf9b@suse.de>
Date: Wed, 5 Nov 2025 09:57:14 +0100
From: Thomas Zimmermann <tzimmermann@...e.de>
To: oushixiong1025@....com, Dave Airlie <airlied@...hat.com>
Cc: Sean Paul <sean@...rly.run>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org, Shixiong Ou <oushixiong@...inos.cn>
Subject: Re: [PATCH] drm/udl: Increase get urb timeout for modeset
Hi
Am 05.11.25 um 09:30 schrieb oushixiong1025@....com:
> From: Shixiong Ou <oushixiong@...inos.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 <oushixiong@...inos.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)
Powered by blists - more mailing lists