* [PATCH] drm/udl: Ensure channel is selected before using the device.
@ 2016-08-14 18:19 Jamie Lentin
2016-08-15 6:43 ` Daniel Vetter
0 siblings, 1 reply; 12+ messages in thread
From: Jamie Lentin @ 2016-08-14 18:19 UTC (permalink / raw)
To: David Airlie; +Cc: dri-devel, linux-kernel, Jamie Lentin
Lift configuration command from udlfb. If this command is not sent,
then the device never outputs a signal, it's status LED is continually
flashing and occasional "udlfb: wait for urb interrupted" messages are
produced.
Tested with a Rextron VCUD-60 attached to a Thinkpad X201s on Linux 4.7.0
Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
---
This ended up in udl_connector_init() since the name suggests it has
something to do with configuring which output to use, although a quick
search through other displaylink drivers didn't shed any light on what
the bytes in set_def_chn actually mean.
I'm not sure which displaylink chipset the VCUD-60 has, but it's USB ID
is 17e9:0136 if that helps. The vendor descriptor is all 00.
Cheers,
---
drivers/gpu/drm/udl/udl_connector.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/gpu/drm/udl/udl_connector.c b/drivers/gpu/drm/udl/udl_connector.c
index 4709b54..2ee8b38 100644
--- a/drivers/gpu/drm/udl/udl_connector.c
+++ b/drivers/gpu/drm/udl/udl_connector.c
@@ -16,6 +16,8 @@
#include <drm/drm_crtc_helper.h>
#include "udl_drv.h"
+#define NR_USB_REQUEST_CHANNEL 0x12
+
/* dummy connector to just get EDID,
all UDL appear to have a DVI-D */
@@ -54,6 +56,26 @@ error:
return NULL;
}
+/*
+ * This is necessary before we can communicate with the display controller.
+ */
+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 int udl_get_modes(struct drm_connector *connector)
{
struct udl_device *udl = connector->dev->dev_private;
@@ -139,6 +161,7 @@ static const struct drm_connector_funcs udl_connector_funcs = {
int udl_connector_init(struct drm_device *dev, struct drm_encoder *encoder)
{
struct drm_connector *connector;
+ int ret;
connector = kzalloc(sizeof(struct drm_connector), GFP_KERNEL);
if (!connector)
@@ -147,6 +170,10 @@ int udl_connector_init(struct drm_device *dev, struct drm_encoder *encoder)
drm_connector_init(dev, connector, &udl_connector_funcs, DRM_MODE_CONNECTOR_DVII);
drm_connector_helper_add(connector, &udl_connector_helper_funcs);
+ ret = udl_select_std_channel(connector->dev->dev_private);
+ if (ret)
+ DRM_ERROR("Selecting channel failed err %x\n", ret);
+
drm_connector_register(connector);
drm_mode_connector_attach_encoder(connector, encoder);
--
2.8.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/udl: Ensure channel is selected before using the device.
2016-08-14 18:19 [PATCH] drm/udl: Ensure channel is selected before using the device Jamie Lentin
@ 2016-08-15 6:43 ` Daniel Vetter
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2016-08-15 6:43 UTC (permalink / raw)
To: Jamie Lentin; +Cc: David Airlie, linux-kernel, dri-devel
On Sun, Aug 14, 2016 at 07:19:35PM +0100, Jamie Lentin wrote:
> Lift configuration command from udlfb. If this command is not sent,
> then the device never outputs a signal, it's status LED is continually
> flashing and occasional "udlfb: wait for urb interrupted" messages are
> produced.
>
> Tested with a Rextron VCUD-60 attached to a Thinkpad X201s on Linux 4.7.0
>
> Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
> ---
> This ended up in udl_connector_init() since the name suggests it has
> something to do with configuring which output to use, although a quick
> search through other displaylink drivers didn't shed any light on what
> the bytes in set_def_chn actually mean.
>
> I'm not sure which displaylink chipset the VCUD-60 has, but it's USB ID
> is 17e9:0136 if that helps. The vendor descriptor is all 00.
>
> Cheers,
Connector init feels like the wrong place. I'd either put it into
udl_driver_load (if it's really just one-time init work). Or into
udl_encoder_commit if we need to set this every time we do a modeset, e.g.
also after resume.
-Daniel
> ---
> drivers/gpu/drm/udl/udl_connector.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/udl/udl_connector.c b/drivers/gpu/drm/udl/udl_connector.c
> index 4709b54..2ee8b38 100644
> --- a/drivers/gpu/drm/udl/udl_connector.c
> +++ b/drivers/gpu/drm/udl/udl_connector.c
> @@ -16,6 +16,8 @@
> #include <drm/drm_crtc_helper.h>
> #include "udl_drv.h"
>
> +#define NR_USB_REQUEST_CHANNEL 0x12
> +
> /* dummy connector to just get EDID,
> all UDL appear to have a DVI-D */
>
> @@ -54,6 +56,26 @@ error:
> return NULL;
> }
>
> +/*
> + * This is necessary before we can communicate with the display controller.
> + */
> +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 int udl_get_modes(struct drm_connector *connector)
> {
> struct udl_device *udl = connector->dev->dev_private;
> @@ -139,6 +161,7 @@ static const struct drm_connector_funcs udl_connector_funcs = {
> int udl_connector_init(struct drm_device *dev, struct drm_encoder *encoder)
> {
> struct drm_connector *connector;
> + int ret;
>
> connector = kzalloc(sizeof(struct drm_connector), GFP_KERNEL);
> if (!connector)
> @@ -147,6 +170,10 @@ int udl_connector_init(struct drm_device *dev, struct drm_encoder *encoder)
> drm_connector_init(dev, connector, &udl_connector_funcs, DRM_MODE_CONNECTOR_DVII);
> drm_connector_helper_add(connector, &udl_connector_helper_funcs);
>
> + ret = udl_select_std_channel(connector->dev->dev_private);
> + if (ret)
> + DRM_ERROR("Selecting channel failed err %x\n", ret);
> +
> drm_connector_register(connector);
> drm_mode_connector_attach_encoder(connector, encoder);
>
> --
> 2.8.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/udl: Ensure channel is selected before using the device.
@ 2016-08-15 6:43 ` Daniel Vetter
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2016-08-15 6:43 UTC (permalink / raw)
To: Jamie Lentin; +Cc: linux-kernel, dri-devel
On Sun, Aug 14, 2016 at 07:19:35PM +0100, Jamie Lentin wrote:
> Lift configuration command from udlfb. If this command is not sent,
> then the device never outputs a signal, it's status LED is continually
> flashing and occasional "udlfb: wait for urb interrupted" messages are
> produced.
>
> Tested with a Rextron VCUD-60 attached to a Thinkpad X201s on Linux 4.7.0
>
> Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
> ---
> This ended up in udl_connector_init() since the name suggests it has
> something to do with configuring which output to use, although a quick
> search through other displaylink drivers didn't shed any light on what
> the bytes in set_def_chn actually mean.
>
> I'm not sure which displaylink chipset the VCUD-60 has, but it's USB ID
> is 17e9:0136 if that helps. The vendor descriptor is all 00.
>
> Cheers,
Connector init feels like the wrong place. I'd either put it into
udl_driver_load (if it's really just one-time init work). Or into
udl_encoder_commit if we need to set this every time we do a modeset, e.g.
also after resume.
-Daniel
> ---
> drivers/gpu/drm/udl/udl_connector.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/udl/udl_connector.c b/drivers/gpu/drm/udl/udl_connector.c
> index 4709b54..2ee8b38 100644
> --- a/drivers/gpu/drm/udl/udl_connector.c
> +++ b/drivers/gpu/drm/udl/udl_connector.c
> @@ -16,6 +16,8 @@
> #include <drm/drm_crtc_helper.h>
> #include "udl_drv.h"
>
> +#define NR_USB_REQUEST_CHANNEL 0x12
> +
> /* dummy connector to just get EDID,
> all UDL appear to have a DVI-D */
>
> @@ -54,6 +56,26 @@ error:
> return NULL;
> }
>
> +/*
> + * This is necessary before we can communicate with the display controller.
> + */
> +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 int udl_get_modes(struct drm_connector *connector)
> {
> struct udl_device *udl = connector->dev->dev_private;
> @@ -139,6 +161,7 @@ static const struct drm_connector_funcs udl_connector_funcs = {
> int udl_connector_init(struct drm_device *dev, struct drm_encoder *encoder)
> {
> struct drm_connector *connector;
> + int ret;
>
> connector = kzalloc(sizeof(struct drm_connector), GFP_KERNEL);
> if (!connector)
> @@ -147,6 +170,10 @@ int udl_connector_init(struct drm_device *dev, struct drm_encoder *encoder)
> drm_connector_init(dev, connector, &udl_connector_funcs, DRM_MODE_CONNECTOR_DVII);
> drm_connector_helper_add(connector, &udl_connector_helper_funcs);
>
> + ret = udl_select_std_channel(connector->dev->dev_private);
> + if (ret)
> + DRM_ERROR("Selecting channel failed err %x\n", ret);
> +
> drm_connector_register(connector);
> drm_mode_connector_attach_encoder(connector, encoder);
>
> --
> 2.8.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] drm/udl: Ensure channel is selected before using the device.
2016-08-15 6:43 ` Daniel Vetter
(?)
@ 2016-08-22 22:17 ` Jamie Lentin
2016-08-23 5:57 ` Daniel Vetter
-1 siblings, 1 reply; 12+ messages in thread
From: Jamie Lentin @ 2016-08-22 22:17 UTC (permalink / raw)
To: Daniel Vetter, David Airlie; +Cc: dri-devel, linux-kernel, 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 <jm@lentin.co.uk>
---
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/udl: Ensure channel is selected before using the device.
2016-08-22 22:17 ` [PATCH v2] " Jamie Lentin
@ 2016-08-23 5:57 ` Daniel Vetter
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2016-08-23 5:57 UTC (permalink / raw)
To: Jamie Lentin; +Cc: Daniel Vetter, David Airlie, dri-devel, linux-kernel
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 <jm@lentin.co.uk>
> ---
> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/udl: Ensure channel is selected before using the device.
@ 2016-08-23 5:57 ` Daniel Vetter
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2016-08-23 5:57 UTC (permalink / raw)
To: Jamie Lentin; +Cc: dri-devel, linux-kernel
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 <jm@lentin.co.uk>
> ---
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/udl: Ensure channel is selected before using the device.
2016-08-23 5:57 ` Daniel Vetter
@ 2016-11-08 6:32 ` poma
-1 siblings, 0 replies; 12+ messages in thread
From: poma @ 2016-11-08 6:32 UTC (permalink / raw)
To: Jamie Lentin, David Airlie, dri-devel, linux-kernel
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 <jm@lentin.co.uk>
>> ---
>> 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:
[<ffffffffb646b1b3>] dump_stack+0x86/0xc3
[<ffffffffb60af55b>] __warn+0xcb/0xf0
[<ffffffffb60af5df>] warn_slowpath_fmt+0x5f/0x80
[<ffffffffb649e86b>] ? debug_dma_mapping_error+0x7b/0x90
[<ffffffffb6694a8f>] usb_hcd_map_urb_for_dma+0x49f/0x770
[<ffffffffb6110e25>] ? trace_hardirqs_on_caller+0xf5/0x1b0
[<ffffffffb669539d>] usb_hcd_submit_urb+0x34d/0xb50
[<ffffffffb6110d06>] ? mark_held_locks+0x76/0xa0
[<ffffffffb6116601>] ? __raw_spin_lock_init+0x21/0x60
[<ffffffffb6110e25>] ? trace_hardirqs_on_caller+0xf5/0x1b0
[<ffffffffb6696d74>] usb_submit_urb+0x2f4/0x560
[<ffffffffb610e171>] ? lockdep_init_map+0x61/0x210
[<ffffffffb66976f4>] usb_start_wait_urb+0x74/0x180
[<ffffffffb66978dc>] usb_control_msg+0xdc/0x120
[<ffffffffc08923e1>] udl_driver_load+0x141/0x590 [udl]
[<ffffffffb6110e01>] ? trace_hardirqs_on_caller+0xd1/0x1b0
[<ffffffffc059fbe9>] drm_dev_register+0xa9/0xd0 [drm]
[<ffffffffc08910c2>] udl_usb_probe+0x42/0x90 [udl]
[<ffffffffb669c18f>] usb_probe_interface+0x15f/0x2d0
[<ffffffffb65eb2f3>] driver_probe_device+0x223/0x430
[<ffffffffb65eb5e3>] __driver_attach+0xe3/0xf0
[<ffffffffb65eb500>] ? driver_probe_device+0x430/0x430
[<ffffffffb65e8bf3>] bus_for_each_dev+0x73/0xc0
[<ffffffffb65eaa1e>] driver_attach+0x1e/0x20
[<ffffffffb65ea443>] bus_add_driver+0x173/0x270
[<ffffffffb65ec240>] driver_register+0x60/0xe0
[<ffffffffb669a9fa>] usb_register_driver+0xaa/0x160
[<ffffffffc089a000>] ? 0xffffffffc089a000
[<ffffffffc089a01e>] udl_driver_init+0x1e/0x1000 [udl]
[<ffffffffb6002190>] do_one_initcall+0x50/0x180
[<ffffffffb6131045>] ? rcu_read_lock_sched_held+0x45/0x80
[<ffffffffb6275787>] ? kmem_cache_alloc_trace+0x277/0x2d0
[<ffffffffb61fb2ce>] ? do_init_module+0x27/0x1f1
[<ffffffffb61fb306>] do_init_module+0x5f/0x1f1
[<ffffffffb615e3d1>] load_module+0x2401/0x2b40
[<ffffffffb615ac00>] ? __symbol_put+0x70/0x70
[<ffffffffb60ecd10>] ? sched_clock_cpu+0x90/0xc0
[<ffffffffb615ecab>] SYSC_init_module+0x19b/0x1c0
[<ffffffffb615edee>] SyS_init_module+0xe/0x10
[<ffffffffb6003eec>] do_syscall_64+0x6c/0x1f0
[<ffffffffb690bd49>] 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/udl: Ensure channel is selected before using the device.
@ 2016-11-08 6:32 ` poma
0 siblings, 0 replies; 12+ messages in thread
From: poma @ 2016-11-08 6:32 UTC (permalink / raw)
To: Jamie Lentin, David Airlie, dri-devel, linux-kernel
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 <jm@lentin.co.uk>
>> ---
>> 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:
[<ffffffffb646b1b3>] dump_stack+0x86/0xc3
[<ffffffffb60af55b>] __warn+0xcb/0xf0
[<ffffffffb60af5df>] warn_slowpath_fmt+0x5f/0x80
[<ffffffffb649e86b>] ? debug_dma_mapping_error+0x7b/0x90
[<ffffffffb6694a8f>] usb_hcd_map_urb_for_dma+0x49f/0x770
[<ffffffffb6110e25>] ? trace_hardirqs_on_caller+0xf5/0x1b0
[<ffffffffb669539d>] usb_hcd_submit_urb+0x34d/0xb50
[<ffffffffb6110d06>] ? mark_held_locks+0x76/0xa0
[<ffffffffb6116601>] ? __raw_spin_lock_init+0x21/0x60
[<ffffffffb6110e25>] ? trace_hardirqs_on_caller+0xf5/0x1b0
[<ffffffffb6696d74>] usb_submit_urb+0x2f4/0x560
[<ffffffffb610e171>] ? lockdep_init_map+0x61/0x210
[<ffffffffb66976f4>] usb_start_wait_urb+0x74/0x180
[<ffffffffb66978dc>] usb_control_msg+0xdc/0x120
[<ffffffffc08923e1>] udl_driver_load+0x141/0x590 [udl]
[<ffffffffb6110e01>] ? trace_hardirqs_on_caller+0xd1/0x1b0
[<ffffffffc059fbe9>] drm_dev_register+0xa9/0xd0 [drm]
[<ffffffffc08910c2>] udl_usb_probe+0x42/0x90 [udl]
[<ffffffffb669c18f>] usb_probe_interface+0x15f/0x2d0
[<ffffffffb65eb2f3>] driver_probe_device+0x223/0x430
[<ffffffffb65eb5e3>] __driver_attach+0xe3/0xf0
[<ffffffffb65eb500>] ? driver_probe_device+0x430/0x430
[<ffffffffb65e8bf3>] bus_for_each_dev+0x73/0xc0
[<ffffffffb65eaa1e>] driver_attach+0x1e/0x20
[<ffffffffb65ea443>] bus_add_driver+0x173/0x270
[<ffffffffb65ec240>] driver_register+0x60/0xe0
[<ffffffffb669a9fa>] usb_register_driver+0xaa/0x160
[<ffffffffc089a000>] ? 0xffffffffc089a000
[<ffffffffc089a01e>] udl_driver_init+0x1e/0x1000 [udl]
[<ffffffffb6002190>] do_one_initcall+0x50/0x180
[<ffffffffb6131045>] ? rcu_read_lock_sched_held+0x45/0x80
[<ffffffffb6275787>] ? kmem_cache_alloc_trace+0x277/0x2d0
[<ffffffffb61fb2ce>] ? do_init_module+0x27/0x1f1
[<ffffffffb61fb306>] do_init_module+0x5f/0x1f1
[<ffffffffb615e3d1>] load_module+0x2401/0x2b40
[<ffffffffb615ac00>] ? __symbol_put+0x70/0x70
[<ffffffffb60ecd10>] ? sched_clock_cpu+0x90/0xc0
[<ffffffffb615ecab>] SYSC_init_module+0x19b/0x1c0
[<ffffffffb615edee>] SyS_init_module+0xe/0x10
[<ffffffffb6003eec>] do_syscall_64+0x6c/0x1f0
[<ffffffffb690bd49>] 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/udl: Ensure channel is selected before using the device.
2016-11-08 6:32 ` poma
@ 2016-11-08 6:40 ` Dave Airlie
-1 siblings, 0 replies; 12+ messages in thread
From: Dave Airlie @ 2016-11-08 6:40 UTC (permalink / raw)
To: poma; +Cc: Jamie Lentin, David Airlie, dri-devel, LKML
> [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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/udl: Ensure channel is selected before using the device.
@ 2016-11-08 6:40 ` Dave Airlie
0 siblings, 0 replies; 12+ messages in thread
From: Dave Airlie @ 2016-11-08 6:40 UTC (permalink / raw)
To: poma; +Cc: Jamie Lentin, LKML, dri-devel
> [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.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/udl: Ensure channel is selected before using the device.
2016-11-08 6:40 ` Dave Airlie
@ 2016-11-08 6:48 ` Dave Airlie
-1 siblings, 0 replies; 12+ messages in thread
From: Dave Airlie @ 2016-11-08 6:48 UTC (permalink / raw)
To: poma; +Cc: Jamie Lentin, David Airlie, dri-devel, LKML
On 8 November 2016 at 16:40, Dave Airlie <airlied@gmail.com> 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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/udl: Ensure channel is selected before using the device.
@ 2016-11-08 6:48 ` Dave Airlie
0 siblings, 0 replies; 12+ messages in thread
From: Dave Airlie @ 2016-11-08 6:48 UTC (permalink / raw)
To: poma; +Cc: Jamie Lentin, LKML, dri-devel
On 8 November 2016 at 16:40, Dave Airlie <airlied@gmail.com> 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.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-11-08 6:48 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-14 18:19 [PATCH] drm/udl: Ensure channel is selected before using the device Jamie Lentin
2016-08-15 6:43 ` Daniel Vetter
2016-08-15 6:43 ` Daniel Vetter
2016-08-22 22:17 ` [PATCH v2] " Jamie Lentin
2016-08-23 5:57 ` Daniel Vetter
2016-08-23 5:57 ` Daniel Vetter
2016-11-08 6:32 ` poma
2016-11-08 6:32 ` poma
2016-11-08 6:40 ` Dave Airlie
2016-11-08 6:40 ` Dave Airlie
2016-11-08 6:48 ` Dave Airlie
2016-11-08 6:48 ` Dave Airlie
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.