Re: [PATCH v2] drm/udl: Ensure channel is selected before using the device.

2016-11-07 Thread Dave Airlie 
On 8 November 2016 at 16:40, Dave Airlie  wrote:
>> [drm:udl_driver_load [udl]] *ERROR* Selecting channel failed
>> udl 1-2:1.0: fb1: udldrmfb frame buffer device
>> [drm] Initialized udl on minor 1
>> usbcore: registered new interface driver udl
>>
>>
>> Is this expected WARN?
>
> I've just posted a patch in theory to fix it, please test if oyu have time.

Actually I posted a v2 of it, because the first one didn't work either.

Dave.

Re: [PATCH v2] drm/udl: Ensure channel is selected before using the device.

2016-11-07 Thread Dave Airlie 
On 8 November 2016 at 16:40, Dave Airlie  wrote:
>> [drm:udl_driver_load [udl]] *ERROR* Selecting channel failed
>> udl 1-2:1.0: fb1: udldrmfb frame buffer device
>> [drm] Initialized udl on minor 1
>> usbcore: registered new interface driver udl
>>
>>
>> Is this expected WARN?
>
> I've just posted a patch in theory to fix it, please test if oyu have time.

Actually I posted a v2 of it, because the first one didn't work either.

Dave.

Re: [PATCH v2] drm/udl: Ensure channel is selected before using the device.

2016-11-07 Thread Dave Airlie 
> [drm:udl_driver_load [udl]] *ERROR* Selecting channel failed
> udl 1-2:1.0: fb1: udldrmfb frame buffer device
> [drm] Initialized udl on minor 1
> usbcore: registered new interface driver udl
>
>
> Is this expected WARN?

I've just posted a patch in theory to fix it, please test if oyu have time.

Dave.

Re: [PATCH v2] drm/udl: Ensure channel is selected before using the device.

2016-11-07 Thread Dave Airlie 
> [drm:udl_driver_load [udl]] *ERROR* Selecting channel failed
> udl 1-2:1.0: fb1: udldrmfb frame buffer device
> [drm] Initialized udl on minor 1
> usbcore: registered new interface driver udl
>
>
> Is this expected WARN?

I've just posted a patch in theory to fix it, please test if oyu have time.

Dave.

Re: [PATCH v2] drm/udl: Ensure channel is selected before using the device.

2016-11-07 Thread poma 
On 23.08.2016 07:57, Daniel Vetter wrote:
> On Mon, Aug 22, 2016 at 11:17:34PM +0100, Jamie Lentin wrote:
>> Lift configuration command from udlfb. This appears to be essential for
>> at least a Rextron VCUD-60, without which no URB communication occurs.
>>
>> Signed-off-by: Jamie Lentin 
>> ---
>> udl_encoder_commit() is too late to do this set up in it seems. This
>> setup doesn't need to be performed again after a suspend, although this
>> is somewhat academic until I send the patch adding suspend and resume
>> functions.
>>
>> Tested with a Rextron VCUD-60 attached to a Thinkpad X201s on Linux 4.7.0
> 
> Applied to drm-misc, thanks.
> -Daniel
> 
>>
>> Cheers,
>> ---
>>  drivers/gpu/drm/udl/udl_main.c | 25 +
>>  1 file changed, 25 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
>> index 33dbfb2..29f0207 100644
>> --- a/drivers/gpu/drm/udl/udl_main.c
>> +++ b/drivers/gpu/drm/udl/udl_main.c
>> @@ -16,6 +16,8 @@
>>  /* -BULK_SIZE as per usb-skeleton. Can we get full page and avoid overhead? 
>> */
>>  #define BULK_SIZE 512
>>  
>> +#define NR_USB_REQUEST_CHANNEL 0x12
>> +
>>  #define MAX_TRANSFER (PAGE_SIZE*16 - BULK_SIZE)
>>  #define WRITES_IN_FLIGHT (4)
>>  #define MAX_VENDOR_DESCRIPTOR_SIZE 256
>> @@ -90,6 +92,26 @@ success:
>>  return true;
>>  }
>>  
>> +/*
>> + * Need to ensure a channel is selected before submitting URBs
>> + */
>> +static int udl_select_std_channel(struct udl_device *udl)
>> +{
>> +int ret;
>> +u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
>> +0x1C, 0x88, 0x5E, 0x15,
>> +0x60, 0xFE, 0xC6, 0x97,
>> +0x16, 0x3D, 0x47, 0xF2};
>> +
>> +ret = usb_control_msg(udl->udev,
>> +  usb_sndctrlpipe(udl->udev, 0),
>> +  NR_USB_REQUEST_CHANNEL,
>> +  (USB_DIR_OUT | USB_TYPE_VENDOR), 0, 0,
>> +  set_def_chn, sizeof(set_def_chn),
>> +  USB_CTRL_SET_TIMEOUT);
>> +return ret < 0 ? ret : 0;
>> +}
>> +
>>  static void udl_release_urb_work(struct work_struct *work)
>>  {
>>  struct urb_node *unode = container_of(work, struct urb_node,
>> @@ -301,6 +323,9 @@ int udl_driver_load(struct drm_device *dev, unsigned 
>> long flags)
>>  goto err;
>>  }
>>  
>> +if (udl_select_std_channel(udl))
>> +DRM_ERROR("Selecting channel failed\n");
>> +
>>  if (!udl_alloc_urb_list(dev, WRITES_IN_FLIGHT, MAX_TRANSFER)) {
>>  DRM_ERROR("udl_alloc_urb_list failed\n");
>>  goto err;
>> -- 
>> 2.8.1
>>
> 

It doesn't breaks the device, however under the hood it is boiling,

[ cut here ]
WARNING: CPU: 0 PID: 381 at drivers/usb/core/hcd.c:1584 
usb_hcd_map_urb_for_dma+0x49f/0x770
transfer buffer not dma capable
Modules linked in: ... udl(+) ... drm_kms_helper ... drm ...
CPU: 0 PID: 381 Comm: systemd-udevd Not tainted 
4.9.0-0.rc4.git0.1.fc26.x86_64+debug #1
...
Call Trace:
 [] dump_stack+0x86/0xc3
 [] __warn+0xcb/0xf0
 [] warn_slowpath_fmt+0x5f/0x80
 [] ? debug_dma_mapping_error+0x7b/0x90
 [] usb_hcd_map_urb_for_dma+0x49f/0x770
 [] ? trace_hardirqs_on_caller+0xf5/0x1b0
 [] usb_hcd_submit_urb+0x34d/0xb50
 [] ? mark_held_locks+0x76/0xa0
 [] ? __raw_spin_lock_init+0x21/0x60
 [] ? trace_hardirqs_on_caller+0xf5/0x1b0
 [] usb_submit_urb+0x2f4/0x560
 [] ? lockdep_init_map+0x61/0x210
 [] usb_start_wait_urb+0x74/0x180
 [] usb_control_msg+0xdc/0x120
 [] udl_driver_load+0x141/0x590 [udl]
 [] ? trace_hardirqs_on_caller+0xd1/0x1b0
 [] drm_dev_register+0xa9/0xd0 [drm]
 [] udl_usb_probe+0x42/0x90 [udl]
 [] usb_probe_interface+0x15f/0x2d0
 [] driver_probe_device+0x223/0x430
 [] __driver_attach+0xe3/0xf0
 [] ? driver_probe_device+0x430/0x430
 [] bus_for_each_dev+0x73/0xc0
 [] driver_attach+0x1e/0x20
 [] bus_add_driver+0x173/0x270
 [] driver_register+0x60/0xe0
 [] usb_register_driver+0xaa/0x160
 [] ? 0xc089a000
 [] udl_driver_init+0x1e/0x1000 [udl]
 [] do_one_initcall+0x50/0x180
 [] ? rcu_read_lock_sched_held+0x45/0x80
 [] ? kmem_cache_alloc_trace+0x277/0x2d0
 [] ? do_init_module+0x27/0x1f1
 [] do_init_module+0x5f/0x1f1
 [] load_module+0x2401/0x2b40
 [] ? __symbol_put+0x70/0x70
 [] ? sched_clock_cpu+0x90/0xc0
 [] SYSC_init_module+0x19b/0x1c0
 [] SyS_init_module+0xe/0x10
 [] do_syscall_64+0x6c/0x1f0
 [] entry_SYSCALL64_slow_path+0x25/0x25
---[ end trace 1fa5e22a0dcf62da ]---
[drm:udl_driver_load [udl]] *ERROR* Selecting channel failed
udl 1-2:1.0: fb1: udldrmfb frame buffer device
[drm] Initialized udl on minor 1
usbcore: registered new interface driver udl


Is this expected WARN?


Ref.
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/udl?id=d1c151dc
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/core/hcd.c?id=refs/tags/v4.9-rc4#n1584

Re: [PATCH v2] drm/udl: Ensure channel is selected before using the device.

2016-11-07 Thread poma 
On 23.08.2016 07:57, Daniel Vetter wrote:
> On Mon, Aug 22, 2016 at 11:17:34PM +0100, Jamie Lentin wrote:
>> Lift configuration command from udlfb. This appears to be essential for
>> at least a Rextron VCUD-60, without which no URB communication occurs.
>>
>> Signed-off-by: Jamie Lentin 
>> ---
>> udl_encoder_commit() is too late to do this set up in it seems. This
>> setup doesn't need to be performed again after a suspend, although this
>> is somewhat academic until I send the patch adding suspend and resume
>> functions.
>>
>> Tested with a Rextron VCUD-60 attached to a Thinkpad X201s on Linux 4.7.0
> 
> Applied to drm-misc, thanks.
> -Daniel
> 
>>
>> Cheers,
>> ---
>>  drivers/gpu/drm/udl/udl_main.c | 25 +
>>  1 file changed, 25 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
>> index 33dbfb2..29f0207 100644
>> --- a/drivers/gpu/drm/udl/udl_main.c
>> +++ b/drivers/gpu/drm/udl/udl_main.c
>> @@ -16,6 +16,8 @@
>>  /* -BULK_SIZE as per usb-skeleton. Can we get full page and avoid overhead? 
>> */
>>  #define BULK_SIZE 512
>>  
>> +#define NR_USB_REQUEST_CHANNEL 0x12
>> +
>>  #define MAX_TRANSFER (PAGE_SIZE*16 - BULK_SIZE)
>>  #define WRITES_IN_FLIGHT (4)
>>  #define MAX_VENDOR_DESCRIPTOR_SIZE 256
>> @@ -90,6 +92,26 @@ success:
>>  return true;
>>  }
>>  
>> +/*
>> + * Need to ensure a channel is selected before submitting URBs
>> + */
>> +static int udl_select_std_channel(struct udl_device *udl)
>> +{
>> +int ret;
>> +u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
>> +0x1C, 0x88, 0x5E, 0x15,
>> +0x60, 0xFE, 0xC6, 0x97,
>> +0x16, 0x3D, 0x47, 0xF2};
>> +
>> +ret = usb_control_msg(udl->udev,
>> +  usb_sndctrlpipe(udl->udev, 0),
>> +  NR_USB_REQUEST_CHANNEL,
>> +  (USB_DIR_OUT | USB_TYPE_VENDOR), 0, 0,
>> +  set_def_chn, sizeof(set_def_chn),
>> +  USB_CTRL_SET_TIMEOUT);
>> +return ret < 0 ? ret : 0;
>> +}
>> +
>>  static void udl_release_urb_work(struct work_struct *work)
>>  {
>>  struct urb_node *unode = container_of(work, struct urb_node,
>> @@ -301,6 +323,9 @@ int udl_driver_load(struct drm_device *dev, unsigned 
>> long flags)
>>  goto err;
>>  }
>>  
>> +if (udl_select_std_channel(udl))
>> +DRM_ERROR("Selecting channel failed\n");
>> +
>>  if (!udl_alloc_urb_list(dev, WRITES_IN_FLIGHT, MAX_TRANSFER)) {
>>  DRM_ERROR("udl_alloc_urb_list failed\n");
>>  goto err;
>> -- 
>> 2.8.1
>>
> 

It doesn't breaks the device, however under the hood it is boiling,

[ cut here ]
WARNING: CPU: 0 PID: 381 at drivers/usb/core/hcd.c:1584 
usb_hcd_map_urb_for_dma+0x49f/0x770
transfer buffer not dma capable
Modules linked in: ... udl(+) ... drm_kms_helper ... drm ...
CPU: 0 PID: 381 Comm: systemd-udevd Not tainted 
4.9.0-0.rc4.git0.1.fc26.x86_64+debug #1
...
Call Trace:
 [] dump_stack+0x86/0xc3
 [] __warn+0xcb/0xf0
 [] warn_slowpath_fmt+0x5f/0x80
 [] ? debug_dma_mapping_error+0x7b/0x90
 [] usb_hcd_map_urb_for_dma+0x49f/0x770
 [] ? trace_hardirqs_on_caller+0xf5/0x1b0
 [] usb_hcd_submit_urb+0x34d/0xb50
 [] ? mark_held_locks+0x76/0xa0
 [] ? __raw_spin_lock_init+0x21/0x60
 [] ? trace_hardirqs_on_caller+0xf5/0x1b0
 [] usb_submit_urb+0x2f4/0x560
 [] ? lockdep_init_map+0x61/0x210
 [] usb_start_wait_urb+0x74/0x180
 [] usb_control_msg+0xdc/0x120
 [] udl_driver_load+0x141/0x590 [udl]
 [] ? trace_hardirqs_on_caller+0xd1/0x1b0
 [] drm_dev_register+0xa9/0xd0 [drm]
 [] udl_usb_probe+0x42/0x90 [udl]
 [] usb_probe_interface+0x15f/0x2d0
 [] driver_probe_device+0x223/0x430
 [] __driver_attach+0xe3/0xf0
 [] ? driver_probe_device+0x430/0x430
 [] bus_for_each_dev+0x73/0xc0
 [] driver_attach+0x1e/0x20
 [] bus_add_driver+0x173/0x270
 [] driver_register+0x60/0xe0
 [] usb_register_driver+0xaa/0x160
 [] ? 0xc089a000
 [] udl_driver_init+0x1e/0x1000 [udl]
 [] do_one_initcall+0x50/0x180
 [] ? rcu_read_lock_sched_held+0x45/0x80
 [] ? kmem_cache_alloc_trace+0x277/0x2d0
 [] ? do_init_module+0x27/0x1f1
 [] do_init_module+0x5f/0x1f1
 [] load_module+0x2401/0x2b40
 [] ? __symbol_put+0x70/0x70
 [] ? sched_clock_cpu+0x90/0xc0
 [] SYSC_init_module+0x19b/0x1c0
 [] SyS_init_module+0xe/0x10
 [] do_syscall_64+0x6c/0x1f0
 [] entry_SYSCALL64_slow_path+0x25/0x25
---[ end trace 1fa5e22a0dcf62da ]---
[drm:udl_driver_load [udl]] *ERROR* Selecting channel failed
udl 1-2:1.0: fb1: udldrmfb frame buffer device
[drm] Initialized udl on minor 1
usbcore: registered new interface driver udl


Is this expected WARN?


Ref.
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/udl?id=d1c151dc
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/core/hcd.c?id=refs/tags/v4.9-rc4#n1584

Re: [PATCH v2] drm/udl: Ensure channel is selected before using the device.

2016-08-22 Thread Daniel Vetter 
On Mon, Aug 22, 2016 at 11:17:34PM +0100, Jamie Lentin wrote:
> Lift configuration command from udlfb. This appears to be essential for
> at least a Rextron VCUD-60, without which no URB communication occurs.
> 
> Signed-off-by: Jamie Lentin 
> ---
> udl_encoder_commit() is too late to do this set up in it seems. This
> setup doesn't need to be performed again after a suspend, although this
> is somewhat academic until I send the patch adding suspend and resume
> functions.
> 
> Tested with a Rextron VCUD-60 attached to a Thinkpad X201s on Linux 4.7.0

Applied to drm-misc, thanks.
-Daniel

> 
> Cheers,
> ---
>  drivers/gpu/drm/udl/udl_main.c | 25 +
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
> index 33dbfb2..29f0207 100644
> --- a/drivers/gpu/drm/udl/udl_main.c
> +++ b/drivers/gpu/drm/udl/udl_main.c
> @@ -16,6 +16,8 @@
>  /* -BULK_SIZE as per usb-skeleton. Can we get full page and avoid overhead? 
> */
>  #define BULK_SIZE 512
>  
> +#define NR_USB_REQUEST_CHANNEL 0x12
> +
>  #define MAX_TRANSFER (PAGE_SIZE*16 - BULK_SIZE)
>  #define WRITES_IN_FLIGHT (4)
>  #define MAX_VENDOR_DESCRIPTOR_SIZE 256
> @@ -90,6 +92,26 @@ success:
>   return true;
>  }
>  
> +/*
> + * Need to ensure a channel is selected before submitting URBs
> + */
> +static int udl_select_std_channel(struct udl_device *udl)
> +{
> + int ret;
> + u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
> + 0x1C, 0x88, 0x5E, 0x15,
> + 0x60, 0xFE, 0xC6, 0x97,
> + 0x16, 0x3D, 0x47, 0xF2};
> +
> + ret = usb_control_msg(udl->udev,
> +   usb_sndctrlpipe(udl->udev, 0),
> +   NR_USB_REQUEST_CHANNEL,
> +   (USB_DIR_OUT | USB_TYPE_VENDOR), 0, 0,
> +   set_def_chn, sizeof(set_def_chn),
> +   USB_CTRL_SET_TIMEOUT);
> + return ret < 0 ? ret : 0;
> +}
> +
>  static void udl_release_urb_work(struct work_struct *work)
>  {
>   struct urb_node *unode = container_of(work, struct urb_node,
> @@ -301,6 +323,9 @@ int udl_driver_load(struct drm_device *dev, unsigned long 
> flags)
>   goto err;
>   }
>  
> + if (udl_select_std_channel(udl))
> + DRM_ERROR("Selecting channel failed\n");
> +
>   if (!udl_alloc_urb_list(dev, WRITES_IN_FLIGHT, MAX_TRANSFER)) {
>   DRM_ERROR("udl_alloc_urb_list failed\n");
>   goto err;
> -- 
> 2.8.1
> 

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

Re: [PATCH v2] drm/udl: Ensure channel is selected before using the device.

2016-08-22 Thread Daniel Vetter 
On Mon, Aug 22, 2016 at 11:17:34PM +0100, Jamie Lentin wrote:
> Lift configuration command from udlfb. This appears to be essential for
> at least a Rextron VCUD-60, without which no URB communication occurs.
> 
> Signed-off-by: Jamie Lentin 
> ---
> udl_encoder_commit() is too late to do this set up in it seems. This
> setup doesn't need to be performed again after a suspend, although this
> is somewhat academic until I send the patch adding suspend and resume
> functions.
> 
> Tested with a Rextron VCUD-60 attached to a Thinkpad X201s on Linux 4.7.0

Applied to drm-misc, thanks.
-Daniel

> 
> Cheers,
> ---
>  drivers/gpu/drm/udl/udl_main.c | 25 +
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
> index 33dbfb2..29f0207 100644
> --- a/drivers/gpu/drm/udl/udl_main.c
> +++ b/drivers/gpu/drm/udl/udl_main.c
> @@ -16,6 +16,8 @@
>  /* -BULK_SIZE as per usb-skeleton. Can we get full page and avoid overhead? 
> */
>  #define BULK_SIZE 512
>  
> +#define NR_USB_REQUEST_CHANNEL 0x12
> +
>  #define MAX_TRANSFER (PAGE_SIZE*16 - BULK_SIZE)
>  #define WRITES_IN_FLIGHT (4)
>  #define MAX_VENDOR_DESCRIPTOR_SIZE 256
> @@ -90,6 +92,26 @@ success:
>   return true;
>  }
>  
> +/*
> + * Need to ensure a channel is selected before submitting URBs
> + */
> +static int udl_select_std_channel(struct udl_device *udl)
> +{
> + int ret;
> + u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
> + 0x1C, 0x88, 0x5E, 0x15,
> + 0x60, 0xFE, 0xC6, 0x97,
> + 0x16, 0x3D, 0x47, 0xF2};
> +
> + ret = usb_control_msg(udl->udev,
> +   usb_sndctrlpipe(udl->udev, 0),
> +   NR_USB_REQUEST_CHANNEL,
> +   (USB_DIR_OUT | USB_TYPE_VENDOR), 0, 0,
> +   set_def_chn, sizeof(set_def_chn),
> +   USB_CTRL_SET_TIMEOUT);
> + return ret < 0 ? ret : 0;
> +}
> +
>  static void udl_release_urb_work(struct work_struct *work)
>  {
>   struct urb_node *unode = container_of(work, struct urb_node,
> @@ -301,6 +323,9 @@ int udl_driver_load(struct drm_device *dev, unsigned long 
> flags)
>   goto err;
>   }
>  
> + if (udl_select_std_channel(udl))
> + DRM_ERROR("Selecting channel failed\n");
> +
>   if (!udl_alloc_urb_list(dev, WRITES_IN_FLIGHT, MAX_TRANSFER)) {
>   DRM_ERROR("udl_alloc_urb_list failed\n");
>   goto err;
> -- 
> 2.8.1
> 

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

[PATCH v2] drm/udl: Ensure channel is selected before using the device.

2016-08-22 Thread Jamie Lentin 
Lift configuration command from udlfb. This appears to be essential for
at least a Rextron VCUD-60, without which no URB communication occurs.

Signed-off-by: Jamie Lentin 
---
udl_encoder_commit() is too late to do this set up in it seems. This
setup doesn't need to be performed again after a suspend, although this
is somewhat academic until I send the patch adding suspend and resume
functions.

Tested with a Rextron VCUD-60 attached to a Thinkpad X201s on Linux 4.7.0

Cheers,
---
 drivers/gpu/drm/udl/udl_main.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index 33dbfb2..29f0207 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -16,6 +16,8 @@
 /* -BULK_SIZE as per usb-skeleton. Can we get full page and avoid overhead? */
 #define BULK_SIZE 512
 
+#define NR_USB_REQUEST_CHANNEL 0x12
+
 #define MAX_TRANSFER (PAGE_SIZE*16 - BULK_SIZE)
 #define WRITES_IN_FLIGHT (4)
 #define MAX_VENDOR_DESCRIPTOR_SIZE 256
@@ -90,6 +92,26 @@ success:
return true;
 }
 
+/*
+ * Need to ensure a channel is selected before submitting URBs
+ */
+static int udl_select_std_channel(struct udl_device *udl)
+{
+   int ret;
+   u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
+   0x1C, 0x88, 0x5E, 0x15,
+   0x60, 0xFE, 0xC6, 0x97,
+   0x16, 0x3D, 0x47, 0xF2};
+
+   ret = usb_control_msg(udl->udev,
+ usb_sndctrlpipe(udl->udev, 0),
+ NR_USB_REQUEST_CHANNEL,
+ (USB_DIR_OUT | USB_TYPE_VENDOR), 0, 0,
+ set_def_chn, sizeof(set_def_chn),
+ USB_CTRL_SET_TIMEOUT);
+   return ret < 0 ? ret : 0;
+}
+
 static void udl_release_urb_work(struct work_struct *work)
 {
struct urb_node *unode = container_of(work, struct urb_node,
@@ -301,6 +323,9 @@ int udl_driver_load(struct drm_device *dev, unsigned long 
flags)
goto err;
}
 
+   if (udl_select_std_channel(udl))
+   DRM_ERROR("Selecting channel failed\n");
+
if (!udl_alloc_urb_list(dev, WRITES_IN_FLIGHT, MAX_TRANSFER)) {
DRM_ERROR("udl_alloc_urb_list failed\n");
goto err;
-- 
2.8.1

[PATCH v2] drm/udl: Ensure channel is selected before using the device.

2016-08-22 Thread Jamie Lentin 
Lift configuration command from udlfb. This appears to be essential for
at least a Rextron VCUD-60, without which no URB communication occurs.

Signed-off-by: Jamie Lentin 
---
udl_encoder_commit() is too late to do this set up in it seems. This
setup doesn't need to be performed again after a suspend, although this
is somewhat academic until I send the patch adding suspend and resume
functions.

Tested with a Rextron VCUD-60 attached to a Thinkpad X201s on Linux 4.7.0

Cheers,
---
 drivers/gpu/drm/udl/udl_main.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index 33dbfb2..29f0207 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -16,6 +16,8 @@
 /* -BULK_SIZE as per usb-skeleton. Can we get full page and avoid overhead? */
 #define BULK_SIZE 512
 
+#define NR_USB_REQUEST_CHANNEL 0x12
+
 #define MAX_TRANSFER (PAGE_SIZE*16 - BULK_SIZE)
 #define WRITES_IN_FLIGHT (4)
 #define MAX_VENDOR_DESCRIPTOR_SIZE 256
@@ -90,6 +92,26 @@ success:
return true;
 }
 
+/*
+ * Need to ensure a channel is selected before submitting URBs
+ */
+static int udl_select_std_channel(struct udl_device *udl)
+{
+   int ret;
+   u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
+   0x1C, 0x88, 0x5E, 0x15,
+   0x60, 0xFE, 0xC6, 0x97,
+   0x16, 0x3D, 0x47, 0xF2};
+
+   ret = usb_control_msg(udl->udev,
+ usb_sndctrlpipe(udl->udev, 0),
+ NR_USB_REQUEST_CHANNEL,
+ (USB_DIR_OUT | USB_TYPE_VENDOR), 0, 0,
+ set_def_chn, sizeof(set_def_chn),
+ USB_CTRL_SET_TIMEOUT);
+   return ret < 0 ? ret : 0;
+}
+
 static void udl_release_urb_work(struct work_struct *work)
 {
struct urb_node *unode = container_of(work, struct urb_node,
@@ -301,6 +323,9 @@ int udl_driver_load(struct drm_device *dev, unsigned long 
flags)
goto err;
}
 
+   if (udl_select_std_channel(udl))
+   DRM_ERROR("Selecting channel failed\n");
+
if (!udl_alloc_urb_list(dev, WRITES_IN_FLIGHT, MAX_TRANSFER)) {
DRM_ERROR("udl_alloc_urb_list failed\n");
goto err;
-- 
2.8.1