2022-09-06 07:45:01

by Takashi Iwai

[permalink] [raw]
Subject: [PATCH v2 02/11] drm/udl: Add reset_resume

From: Thomas Zimmermann <[email protected]>

Implement the reset_resume callback of struct usb_driver. Set the
standard channel when called.

Signed-off-by: Thomas Zimmermann <[email protected]>
Signed-off-by: Takashi Iwai <[email protected]>
---
drivers/gpu/drm/udl/udl_drv.c | 11 +++++++++++
drivers/gpu/drm/udl/udl_drv.h | 1 +
drivers/gpu/drm/udl/udl_main.c | 2 +-
3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index 5703277c6f52..0ba88e5472a9 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -32,6 +32,16 @@ static int udl_usb_resume(struct usb_interface *interface)
return drm_mode_config_helper_resume(dev);
}

+static int udl_usb_reset_resume(struct usb_interface *interface)
+{
+ struct drm_device *dev = usb_get_intfdata(interface);
+ struct udl_device *udl = to_udl(dev);
+
+ udl_select_std_channel(udl);
+
+ return drm_mode_config_helper_resume(dev);
+}
+
/*
* FIXME: Dma-buf sharing requires DMA support by the importing device.
* This function is a workaround to make USB devices work as well.
@@ -140,6 +150,7 @@ static struct usb_driver udl_driver = {
.disconnect = udl_usb_disconnect,
.suspend = udl_usb_suspend,
.resume = udl_usb_resume,
+ .reset_resume = udl_usb_reset_resume,
.id_table = id_table,
};
module_usb_driver(udl_driver);
diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 28aaf75d71cf..37c14b0ff1fc 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -95,6 +95,7 @@ int udl_render_hline(struct drm_device *dev, int log_bpp, struct urb **urb_ptr,
u32 byte_offset, u32 device_byte_offset, u32 byte_width);

int udl_drop_usb(struct drm_device *dev);
+int udl_select_std_channel(struct udl_device *udl);

#define CMD_WRITE_RAW8 "\xAF\x60" /**< 8 bit raw write command. */
#define CMD_WRITE_RL8 "\xAF\x61" /**< 8 bit run length command. */
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index fdafbf8f3c3c..7d1e6bbc165c 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -92,7 +92,7 @@ static int udl_parse_vendor_descriptor(struct udl_device *udl)
/*
* Need to ensure a channel is selected before submitting URBs
*/
-static int udl_select_std_channel(struct udl_device *udl)
+int udl_select_std_channel(struct udl_device *udl)
{
static const u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
0x1C, 0x88, 0x5E, 0x15,
--
2.35.3


2022-09-07 07:26:57

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH v2 02/11] drm/udl: Add reset_resume

On Tue, Sep 06, 2022 at 09:39:42AM +0200, Takashi Iwai wrote:
> From: Thomas Zimmermann <[email protected]>
>
> Implement the reset_resume callback of struct usb_driver. Set the
> standard channel when called.
>
> Signed-off-by: Thomas Zimmermann <[email protected]>
> Signed-off-by: Takashi Iwai <[email protected]>
> ---
> drivers/gpu/drm/udl/udl_drv.c | 11 +++++++++++
> drivers/gpu/drm/udl/udl_drv.h | 1 +
> drivers/gpu/drm/udl/udl_main.c | 2 +-
> 3 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
> index 5703277c6f52..0ba88e5472a9 100644
> --- a/drivers/gpu/drm/udl/udl_drv.c
> +++ b/drivers/gpu/drm/udl/udl_drv.c
> @@ -32,6 +32,16 @@ static int udl_usb_resume(struct usb_interface *interface)
> return drm_mode_config_helper_resume(dev);
> }
>
> +static int udl_usb_reset_resume(struct usb_interface *interface)
> +{
> + struct drm_device *dev = usb_get_intfdata(interface);
> + struct udl_device *udl = to_udl(dev);
> +
> + udl_select_std_channel(udl);
> +
> + return drm_mode_config_helper_resume(dev);

Bit a bikeshed, but in the driver load case it would be neat to put the
call to udl_select_std_channel right above the call to
drm_mode_config_reset. But that is _really_ a bikeshed :-)

Also thanks for sending me on a bit a wild goose chase trying to figure
out what this reset_resume hook actually does and why.

Reviewed-by: Daniel Vetter <[email protected]>

> +}
> +
> /*
> * FIXME: Dma-buf sharing requires DMA support by the importing device.
> * This function is a workaround to make USB devices work as well.
> @@ -140,6 +150,7 @@ static struct usb_driver udl_driver = {
> .disconnect = udl_usb_disconnect,
> .suspend = udl_usb_suspend,
> .resume = udl_usb_resume,
> + .reset_resume = udl_usb_reset_resume,
> .id_table = id_table,
> };
> module_usb_driver(udl_driver);
> diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
> index 28aaf75d71cf..37c14b0ff1fc 100644
> --- a/drivers/gpu/drm/udl/udl_drv.h
> +++ b/drivers/gpu/drm/udl/udl_drv.h
> @@ -95,6 +95,7 @@ int udl_render_hline(struct drm_device *dev, int log_bpp, struct urb **urb_ptr,
> u32 byte_offset, u32 device_byte_offset, u32 byte_width);
>
> int udl_drop_usb(struct drm_device *dev);
> +int udl_select_std_channel(struct udl_device *udl);
>
> #define CMD_WRITE_RAW8 "\xAF\x60" /**< 8 bit raw write command. */
> #define CMD_WRITE_RL8 "\xAF\x61" /**< 8 bit run length command. */
> diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
> index fdafbf8f3c3c..7d1e6bbc165c 100644
> --- a/drivers/gpu/drm/udl/udl_main.c
> +++ b/drivers/gpu/drm/udl/udl_main.c
> @@ -92,7 +92,7 @@ static int udl_parse_vendor_descriptor(struct udl_device *udl)
> /*
> * Need to ensure a channel is selected before submitting URBs
> */
> -static int udl_select_std_channel(struct udl_device *udl)
> +int udl_select_std_channel(struct udl_device *udl)
> {
> static const u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
> 0x1C, 0x88, 0x5E, 0x15,
> --
> 2.35.3
>

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch