2022-08-16 15:40:23

by Takashi Iwai

[permalink] [raw]
Subject: [PATCH 04/12] Revert "drm/udl: Kill pending URBs at suspend and disconnect"

This reverts the recent fix commit
e25d5954264d ("drm/udl: Kill pending URBs at suspend and disconnect")
as it turned out to lead to potential hangup at a disconnection, and
it doesn't help much for suspend/resume problem, either.

Signed-off-by: Takashi Iwai <[email protected]>
---
drivers/gpu/drm/udl/udl_drv.h | 2 --
drivers/gpu/drm/udl/udl_main.c | 25 +++----------------------
drivers/gpu/drm/udl/udl_modeset.c | 2 --
3 files changed, 3 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 37c14b0ff1fc..5923d2e02bc8 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -39,7 +39,6 @@ struct urb_node {

struct urb_list {
struct list_head list;
- struct list_head in_flight;
spinlock_t lock;
wait_queue_head_t sleep;
int available;
@@ -85,7 +84,6 @@ static inline struct urb *udl_get_urb(struct drm_device *dev)

int udl_submit_urb(struct drm_device *dev, struct urb *urb, size_t len);
int udl_sync_pending_urbs(struct drm_device *dev);
-void udl_kill_pending_urbs(struct drm_device *dev);
void udl_urb_completion(struct urb *urb);

int udl_init(struct udl_device *udl);
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index 7d1e6bbc165c..a9f6b710b254 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -135,7 +135,7 @@ void udl_urb_completion(struct urb *urb)
urb->transfer_buffer_length = udl->urbs.size; /* reset to actual */

spin_lock_irqsave(&udl->urbs.lock, flags);
- list_move(&unode->entry, &udl->urbs.list);
+ list_add_tail(&unode->entry, &udl->urbs.list);
udl->urbs.available++;
spin_unlock_irqrestore(&udl->urbs.lock, flags);

@@ -180,7 +180,6 @@ static int udl_alloc_urb_list(struct drm_device *dev, int count, size_t size)
retry:
udl->urbs.size = size;
INIT_LIST_HEAD(&udl->urbs.list);
- INIT_LIST_HEAD(&udl->urbs.in_flight);

init_waitqueue_head(&udl->urbs.sleep);
udl->urbs.count = 0;
@@ -247,7 +246,7 @@ struct urb *udl_get_urb_timeout(struct drm_device *dev, long timeout)
}

unode = list_first_entry(&udl->urbs.list, struct urb_node, entry);
- list_move(&unode->entry, &udl->urbs.in_flight);
+ list_del_init(&unode->entry);
udl->urbs.available--;

unlock:
@@ -281,7 +280,7 @@ int udl_sync_pending_urbs(struct drm_device *dev)
spin_lock_irq(&udl->urbs.lock);
/* 2 seconds as a sane timeout */
if (!wait_event_lock_irq_timeout(udl->urbs.sleep,
- list_empty(&udl->urbs.in_flight),
+ udl->urbs.available == udl->urbs.count,
udl->urbs.lock,
msecs_to_jiffies(2000)))
ret = -ETIMEDOUT;
@@ -289,23 +288,6 @@ int udl_sync_pending_urbs(struct drm_device *dev)
return ret;
}

-/* kill pending URBs */
-void udl_kill_pending_urbs(struct drm_device *dev)
-{
- struct udl_device *udl = to_udl(dev);
- struct urb_node *unode;
-
- spin_lock_irq(&udl->urbs.lock);
- while (!list_empty(&udl->urbs.in_flight)) {
- unode = list_first_entry(&udl->urbs.in_flight,
- struct urb_node, entry);
- spin_unlock_irq(&udl->urbs.lock);
- usb_kill_urb(unode->urb);
- spin_lock_irq(&udl->urbs.lock);
- }
- spin_unlock_irq(&udl->urbs.lock);
-}
-
int udl_init(struct udl_device *udl)
{
struct drm_device *dev = &udl->drm;
@@ -354,7 +336,6 @@ int udl_drop_usb(struct drm_device *dev)
{
struct udl_device *udl = to_udl(dev);

- udl_kill_pending_urbs(dev);
udl_free_urb_list(dev);
put_device(udl->dmadev);
udl->dmadev = NULL;
diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c
index 187aba2d7825..c34d564773a3 100644
--- a/drivers/gpu/drm/udl/udl_modeset.c
+++ b/drivers/gpu/drm/udl/udl_modeset.c
@@ -398,8 +398,6 @@ udl_simple_display_pipe_disable(struct drm_simple_display_pipe *pipe)
struct urb *urb;
char *buf;

- udl_kill_pending_urbs(dev);
-
urb = udl_get_urb(dev);
if (!urb)
return;
--
2.35.3


2022-09-05 09:21:20

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: [PATCH 04/12] Revert "drm/udl: Kill pending URBs at suspend and disconnect"

Hi

Am 16.08.22 um 17:36 schrieb Takashi Iwai:
> This reverts the recent fix commit
> e25d5954264d ("drm/udl: Kill pending URBs at suspend and disconnect")
> as it turned out to lead to potential hangup at a disconnection, and
> it doesn't help much for suspend/resume problem, either.
>
> Signed-off-by: Takashi Iwai <[email protected]>

Acked-by: Thomas Zimmermann <[email protected]>

> ---
> drivers/gpu/drm/udl/udl_drv.h | 2 --
> drivers/gpu/drm/udl/udl_main.c | 25 +++----------------------
> drivers/gpu/drm/udl/udl_modeset.c | 2 --
> 3 files changed, 3 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
> index 37c14b0ff1fc..5923d2e02bc8 100644
> --- a/drivers/gpu/drm/udl/udl_drv.h
> +++ b/drivers/gpu/drm/udl/udl_drv.h
> @@ -39,7 +39,6 @@ struct urb_node {
>
> struct urb_list {
> struct list_head list;
> - struct list_head in_flight;
> spinlock_t lock;
> wait_queue_head_t sleep;
> int available;
> @@ -85,7 +84,6 @@ static inline struct urb *udl_get_urb(struct drm_device *dev)
>
> int udl_submit_urb(struct drm_device *dev, struct urb *urb, size_t len);
> int udl_sync_pending_urbs(struct drm_device *dev);
> -void udl_kill_pending_urbs(struct drm_device *dev);
> void udl_urb_completion(struct urb *urb);
>
> int udl_init(struct udl_device *udl);
> diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
> index 7d1e6bbc165c..a9f6b710b254 100644
> --- a/drivers/gpu/drm/udl/udl_main.c
> +++ b/drivers/gpu/drm/udl/udl_main.c
> @@ -135,7 +135,7 @@ void udl_urb_completion(struct urb *urb)
> urb->transfer_buffer_length = udl->urbs.size; /* reset to actual */
>
> spin_lock_irqsave(&udl->urbs.lock, flags);
> - list_move(&unode->entry, &udl->urbs.list);
> + list_add_tail(&unode->entry, &udl->urbs.list);
> udl->urbs.available++;
> spin_unlock_irqrestore(&udl->urbs.lock, flags);
>
> @@ -180,7 +180,6 @@ static int udl_alloc_urb_list(struct drm_device *dev, int count, size_t size)
> retry:
> udl->urbs.size = size;
> INIT_LIST_HEAD(&udl->urbs.list);
> - INIT_LIST_HEAD(&udl->urbs.in_flight);
>
> init_waitqueue_head(&udl->urbs.sleep);
> udl->urbs.count = 0;
> @@ -247,7 +246,7 @@ struct urb *udl_get_urb_timeout(struct drm_device *dev, long timeout)
> }
>
> unode = list_first_entry(&udl->urbs.list, struct urb_node, entry);
> - list_move(&unode->entry, &udl->urbs.in_flight);
> + list_del_init(&unode->entry);
> udl->urbs.available--;
>
> unlock:
> @@ -281,7 +280,7 @@ int udl_sync_pending_urbs(struct drm_device *dev)
> spin_lock_irq(&udl->urbs.lock);
> /* 2 seconds as a sane timeout */
> if (!wait_event_lock_irq_timeout(udl->urbs.sleep,
> - list_empty(&udl->urbs.in_flight),
> + udl->urbs.available == udl->urbs.count,
> udl->urbs.lock,
> msecs_to_jiffies(2000)))
> ret = -ETIMEDOUT;
> @@ -289,23 +288,6 @@ int udl_sync_pending_urbs(struct drm_device *dev)
> return ret;
> }
>
> -/* kill pending URBs */
> -void udl_kill_pending_urbs(struct drm_device *dev)
> -{
> - struct udl_device *udl = to_udl(dev);
> - struct urb_node *unode;
> -
> - spin_lock_irq(&udl->urbs.lock);
> - while (!list_empty(&udl->urbs.in_flight)) {
> - unode = list_first_entry(&udl->urbs.in_flight,
> - struct urb_node, entry);
> - spin_unlock_irq(&udl->urbs.lock);
> - usb_kill_urb(unode->urb);
> - spin_lock_irq(&udl->urbs.lock);
> - }
> - spin_unlock_irq(&udl->urbs.lock);
> -}
> -
> int udl_init(struct udl_device *udl)
> {
> struct drm_device *dev = &udl->drm;
> @@ -354,7 +336,6 @@ int udl_drop_usb(struct drm_device *dev)
> {
> struct udl_device *udl = to_udl(dev);
>
> - udl_kill_pending_urbs(dev);
> udl_free_urb_list(dev);
> put_device(udl->dmadev);
> udl->dmadev = NULL;
> diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c
> index 187aba2d7825..c34d564773a3 100644
> --- a/drivers/gpu/drm/udl/udl_modeset.c
> +++ b/drivers/gpu/drm/udl/udl_modeset.c
> @@ -398,8 +398,6 @@ udl_simple_display_pipe_disable(struct drm_simple_display_pipe *pipe)
> struct urb *urb;
> char *buf;
>
> - udl_kill_pending_urbs(dev);
> -
> urb = udl_get_urb(dev);
> if (!urb)
> return;

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev


Attachments:
OpenPGP_signature (855.00 B)
OpenPGP digital signature