[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e82274a2-9e60-4344-9d54-78232f42b05d@163.com>
Date: Wed, 5 Nov 2025 17:07:31 +0800
From: oushixiong <oushixiong1025@....com>
To: Thomas Zimmermann <tzimmermann@...e.de>, 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
If the wait times for all operations increase, it would likely cause
significant blocking in the display process.
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 ?
Best regards
Shixiong
在 2025/11/5 16:57, Thomas Zimmermann 写道:
> 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;
>
Powered by blists - more mailing lists