2018-05-09 02:16:22

by ming_qian

[permalink] [raw]
Subject: [PATCH] media: uvcvideo: Support realtek's UVC 1.5 device

From: ming_qian <[email protected]>

The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1.
Change it to 48 for UVC 1.5 device,
and the UVC 1.5 device can be recognized.

More changes to the driver are needed for full UVC 1.5 compatibility.
However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have
been reported to work well.

Signed-off-by: ming_qian <[email protected]>
---
drivers/media/usb/uvc/uvc_video.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index aa0082f..32dfb32 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -171,6 +171,8 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
int ret;

size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
+ if (stream->dev->uvc_version >= 0x0150)
+ size = 48;
if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
query == UVC_GET_DEF)
return -EIO;
@@ -259,6 +261,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
int ret;

size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
+ if (stream->dev->uvc_version >= 0x0150)
+ size = 48;
data = kzalloc(size, GFP_KERNEL);
if (data == NULL)
return -ENOMEM;
--
2.7.4



2018-05-10 15:07:35

by Kai-Heng Feng

[permalink] [raw]
Subject: Re: [PATCH] media: uvcvideo: Support realtek's UVC 1.5 device

at 10:13, [email protected] wrote:

> From: ming_qian <[email protected]>
>
> The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1.
> Change it to 48 for UVC 1.5 device,
> and the UVC 1.5 device can be recognized.
>
> More changes to the driver are needed for full UVC 1.5 compatibility.
> However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have
> been reported to work well.

I think this should also Cc: stable.

Tested-by: Kai-Heng Feng <[email protected]>

>
> Signed-off-by: ming_qian <[email protected]>
> ---
> drivers/media/usb/uvc/uvc_video.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c
> b/drivers/media/usb/uvc/uvc_video.c
> index aa0082f..32dfb32 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -171,6 +171,8 @@ static int uvc_get_video_ctrl(struct uvc_streaming
> *stream,
> int ret;
>
> size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> + if (stream->dev->uvc_version >= 0x0150)
> + size = 48;
> if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
> query == UVC_GET_DEF)
> return -EIO;
> @@ -259,6 +261,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming
> *stream,
> int ret;
>
> size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> + if (stream->dev->uvc_version >= 0x0150)
> + size = 48;
> data = kzalloc(size, GFP_KERNEL);
> if (data == NULL)
> return -ENOMEM;
> --
> 2.7.4



2018-05-10 15:39:34

by Hans de Goede

[permalink] [raw]
Subject: Re: media: uvcvideo: Support realtek's UVC 1.5 device

Hi,

On 09-05-18 04:13, [email protected] wrote:
> From: ming_qian <[email protected]>
>
> The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1.
> Change it to 48 for UVC 1.5 device,
> and the UVC 1.5 device can be recognized.
>
> More changes to the driver are needed for full UVC 1.5 compatibility.
> However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have
> been reported to work well.
>
> Signed-off-by: ming_qian <[email protected]>
> Tested-by: Kai-Heng Feng <[email protected]>

Looks good to me:

Reviewed-by: Hans de Goede <[email protected]>

Regards,

Hans



> ---
> drivers/media/usb/uvc/uvc_video.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index aa0082f..32dfb32 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -171,6 +171,8 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
> int ret;
>
> size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> + if (stream->dev->uvc_version >= 0x0150)
> + size = 48;
> if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
> query == UVC_GET_DEF)
> return -EIO;
> @@ -259,6 +261,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
> int ret;
>
> size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> + if (stream->dev->uvc_version >= 0x0150)
> + size = 48;
> data = kzalloc(size, GFP_KERNEL);
> if (data == NULL)
> return -ENOMEM;
>

2018-05-14 17:41:44

by Josef Šimánek

[permalink] [raw]
Subject: RE: media: uvcvideo: Support realtek's UVC 1.5 device

> The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1.
> Change it to 48 for UVC 1.5 device,
> and the UVC 1.5 device can be recognized.
>
> More changes to the driver are needed for full UVC 1.5 compatibility.
> However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have
> been reported to work well.
>
> Signed-off-by: ming_qian <[email protected]>
> Tested-by: Kai-Heng Feng <[email protected]>
Hello! I have sucessfully tested this patch on Kernel 4.16.1 (Fedora 28) with Dell XPS 9370
using following device (output from lsusb):

Bus 001 Device 002: ID 0bda:58f4 Realtek Semiconductor Corp.

You can also find related dmesg output at https://bugs.launchpad.net/dell-sputnik/+bug/1763748/comments/35

Tested-by: Josef Šimánek <[email protected]>
> Reviewed-by: Hans de Goede <[email protected]>
> ---
> drivers/media/usb/uvc/uvc_video.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index aa0082f..32dfb32 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -171,6 +171,8 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
> int ret;
>
> size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> + if (stream->dev->uvc_version >= 0x0150)
> + size = 48;
> if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
> query == UVC_GET_DEF)
> return -EIO;
> @@ -259,6 +261,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
> int ret;
>
> size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> + if (stream->dev->uvc_version >= 0x0150)
> + size = 48;
> data = kzalloc(size, GFP_KERNEL);
> if (data == NULL)
> return -ENOMEM;

2018-05-14 17:44:05

by Josef Šimánek

[permalink] [raw]
Subject: Re: media: uvcvideo: Support realtek's UVC 1.5 device

I'm really sorry, but this message was originally meant to be sent
(and was already also sent) to linux-media. Please ignore here.

2018-05-14 19:28 GMT+02:00 Josef Šimánek <[email protected]>:
>
> > The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1.
> > Change it to 48 for UVC 1.5 device,
> > and the UVC 1.5 device can be recognized.
> >
> > More changes to the driver are needed for full UVC 1.5 compatibility.
> > However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have
> > been reported to work well.
> >
> > Signed-off-by: ming_qian <[email protected]>
> > Tested-by: Kai-Heng Feng <[email protected]>
> Hello! I have sucessfully tested this patch on Kernel 4.16.1 (Fedora 28) with Dell XPS 9370
> using following device (output from lsusb):
>
> Bus 001 Device 002: ID 0bda:58f4 Realtek Semiconductor Corp.
>
> You can also find related dmesg output at https://bugs.launchpad.net/dell-sputnik/+bug/1763748/comments/35
>
> Tested-by: Josef Šimánek <[email protected]>
> > Reviewed-by: Hans de Goede <[email protected]>
> > ---
> > drivers/media/usb/uvc/uvc_video.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> > index aa0082f..32dfb32 100644
> > --- a/drivers/media/usb/uvc/uvc_video.c
> > +++ b/drivers/media/usb/uvc/uvc_video.c
> > @@ -171,6 +171,8 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
> > int ret;
> >
> > size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> > + if (stream->dev->uvc_version >= 0x0150)
> > + size = 48;
> > if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
> > query == UVC_GET_DEF)
> > return -EIO;
> > @@ -259,6 +261,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
> > int ret;
> >
> > size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> > + if (stream->dev->uvc_version >= 0x0150)
> > + size = 48;
> > data = kzalloc(size, GFP_KERNEL);
> > if (data == NULL)
> > return -ENOMEM;

2018-05-18 14:05:42

by Ana Guerrero Lopez

[permalink] [raw]
Subject: Re: media: uvcvideo: Support realtek's UVC 1.5 device

On Wed, May 09, 2018 at 10:13:08AM +0800, [email protected] wrote:
> From: ming_qian <[email protected]>
>
> The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1.
> Change it to 48 for UVC 1.5 device,
> and the UVC 1.5 device can be recognized.
>
> More changes to the driver are needed for full UVC 1.5 compatibility.
> However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have
> been reported to work well.
>
> Signed-off-by: ming_qian <[email protected]>
> Tested-by: Kai-Heng Feng <[email protected]>
> Reviewed-by: Hans de Goede <[email protected]>
> Tested-by: Josef Šimánek <[email protected]>

It works perfectly here on 4.16.5 with a Dell XPS 9370 in Debian.

Tested-by: Ana Guerrero Lopez <[email protected]>

Cheers,
Ana

2018-05-22 20:32:57

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH] media: uvcvideo: Support realtek's UVC 1.5 device

Hello,

Thank you for the patch.

On Wednesday, 9 May 2018 05:13:08 EEST [email protected] wrote:
> From: ming_qian <[email protected]>
>
> The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1.
> Change it to 48 for UVC 1.5 device,
> and the UVC 1.5 device can be recognized.
>
> More changes to the driver are needed for full UVC 1.5 compatibility.
> However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have
> been reported to work well.

This patch is however not specific to Realtek devices, so I think we should
make the subject line more generic. It's fine mentioning in the commit message
itself that the Realtek RTS5847/RTS5852 cameras have been successfully tested.

> Signed-off-by: ming_qian <[email protected]>
> ---
> drivers/media/usb/uvc/uvc_video.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c
> b/drivers/media/usb/uvc/uvc_video.c index aa0082f..32dfb32 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -171,6 +171,8 @@ static int uvc_get_video_ctrl(struct uvc_streaming
> *stream, int ret;
>
> size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> + if (stream->dev->uvc_version >= 0x0150)
> + size = 48;
> if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
> query == UVC_GET_DEF)
> return -EIO;
> @@ -259,6 +261,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming
> *stream, int ret;
>
> size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> + if (stream->dev->uvc_version >= 0x0150)
> + size = 48;
> data = kzalloc(size, GFP_KERNEL);
> if (data == NULL)
> return -ENOMEM;

Instead of duplicating the computation in both functions, I think we should
move the code to a helper function.

Furthermore there are equality checks further down both functions that compare
the size to 34, they should be updated to also support UVC 1.5.

I propose the following updated patch. If you're fine with it there's no need
to resubmit, I'll queue it for v4.19.

I have dropped the Reviewed-by and Tested-by tags as the patch has changed.

commit a9c002732695eab2096580a0d1a1687bc2f95928
Author: ming_qian <[email protected]>
Date: Wed May 9 10:13:08 2018 +0800

media: uvcvideo: Support UVC 1.5 video probe & commit controls

The length of UVC 1.5 video control is 48, and it is 34 for UVC 1.1.
Change it to 48 for UVC 1.5 device, and the UVC 1.5 device can be
recognized.

More changes to the driver are needed for full UVC 1.5 compatibility.
However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have been
reported to work well.

Cc: [email protected]
Signed-off-by: ming_qian <[email protected]>
[Factor out code to helper function, update size checks]
Signed-off-by: Laurent Pinchart <[email protected]>

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index eb9e04a59427..285b0e813b9d 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -207,14 +207,27 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream,
}
}

+static size_t uvc_video_ctrl_size(struct uvc_streaming *stream)
+{
+ /*
+ * Return the size of the video probe and commit controls, which depends
+ * on the protocol version.
+ */
+ if (stream->dev->uvc_version < 0x0110)
+ return 26;
+ else if (stream->dev->uvc_version < 0x0150)
+ return 34;
+ else
+ return 48;
+}
+
static int uvc_get_video_ctrl(struct uvc_streaming *stream,
struct uvc_streaming_control *ctrl, int probe, u8 query)
{
+ u16 size = uvc_video_ctrl_size(stream);
u8 *data;
- u16 size;
int ret;

- size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
query == UVC_GET_DEF)
return -EIO;
@@ -271,7 +284,7 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
ctrl->dwMaxVideoFrameSize = get_unaligned_le32(&data[18]);
ctrl->dwMaxPayloadTransferSize = get_unaligned_le32(&data[22]);

- if (size == 34) {
+ if (size >= 34) {
ctrl->dwClockFrequency = get_unaligned_le32(&data[26]);
ctrl->bmFramingInfo = data[30];
ctrl->bPreferedVersion = data[31];
@@ -300,11 +313,10 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
static int uvc_set_video_ctrl(struct uvc_streaming *stream,
struct uvc_streaming_control *ctrl, int probe)
{
+ u16 size = uvc_video_ctrl_size(stream);
u8 *data;
- u16 size;
int ret;

- size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
data = kzalloc(size, GFP_KERNEL);
if (data == NULL)
return -ENOMEM;
@@ -321,7 +333,7 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
put_unaligned_le32(ctrl->dwMaxVideoFrameSize, &data[18]);
put_unaligned_le32(ctrl->dwMaxPayloadTransferSize, &data[22]);

- if (size == 34) {
+ if (size >= 34) {
put_unaligned_le32(ctrl->dwClockFrequency, &data[26]);
data[30] = ctrl->bmFramingInfo;
data[31] = ctrl->bPreferedVersion;

--
Regards,

Laurent Pinchart




2018-05-23 05:43:41

by Kai-Heng Feng

[permalink] [raw]
Subject: Re: [PATCH] media: uvcvideo: Support realtek's UVC 1.5 device


> On May 23, 2018, at 4:32 AM, Laurent Pinchart
> <[email protected]> wrote:
>
> Hello,
>
> Thank you for the patch.
>
> On Wednesday, 9 May 2018 05:13:08 EEST [email protected] wrote:
>> From: ming_qian <[email protected]>
>>
>> The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1.
>> Change it to 48 for UVC 1.5 device,
>> and the UVC 1.5 device can be recognized.
>>
>> More changes to the driver are needed for full UVC 1.5 compatibility.
>> However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have
>> been reported to work well.
>
> This patch is however not specific to Realtek devices, so I think we should
> make the subject line more generic. It's fine mentioning in the commit
> message
> itself that the Realtek RTS5847/RTS5852 cameras have been successfully
> tested.
>
>> Signed-off-by: ming_qian <[email protected]>
>> ---
>> drivers/media/usb/uvc/uvc_video.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/media/usb/uvc/uvc_video.c
>> b/drivers/media/usb/uvc/uvc_video.c index aa0082f..32dfb32 100644
>> --- a/drivers/media/usb/uvc/uvc_video.c
>> +++ b/drivers/media/usb/uvc/uvc_video.c
>> @@ -171,6 +171,8 @@ static int uvc_get_video_ctrl(struct uvc_streaming
>> *stream, int ret;
>>
>> size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
>> + if (stream->dev->uvc_version >= 0x0150)
>> + size = 48;
>> if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
>> query == UVC_GET_DEF)
>> return -EIO;
>> @@ -259,6 +261,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming
>> *stream, int ret;
>>
>> size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
>> + if (stream->dev->uvc_version >= 0x0150)
>> + size = 48;
>> data = kzalloc(size, GFP_KERNEL);
>> if (data == NULL)
>> return -ENOMEM;
>
> Instead of duplicating the computation in both functions, I think we should
> move the code to a helper function.
>
> Furthermore there are equality checks further down both functions that
> compare
> the size to 34, they should be updated to also support UVC 1.5.
>
> I propose the following updated patch. If you're fine with it there's no
> need
> to resubmit, I'll queue it for v4.19.
>
> I have dropped the Reviewed-by and Tested-by tags as the patch has changed.
>
> commit a9c002732695eab2096580a0d1a1687bc2f95928
> Author: ming_qian <[email protected]>
> Date: Wed May 9 10:13:08 2018 +0800
>
> media: uvcvideo: Support UVC 1.5 video probe & commit controls
>
> The length of UVC 1.5 video control is 48, and it is 34 for UVC 1.1.
> Change it to 48 for UVC 1.5 device, and the UVC 1.5 device can be
> recognized.
>
> More changes to the driver are needed for full UVC 1.5 compatibility.
> However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have been
> reported to work well.
>
> Cc: [email protected]
> Signed-off-by: ming_qian <[email protected]>
> [Factor out code to helper function, update size checks]
> Signed-off-by: Laurent Pinchart <[email protected]>

I tested this new patch and it works well.

Tested-by: Kai-Heng Feng <[email protected]>

>
> diff --git a/drivers/media/usb/uvc/uvc_video.c
> b/drivers/media/usb/uvc/uvc_video.c
> index eb9e04a59427..285b0e813b9d 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -207,14 +207,27 @@ static void uvc_fixup_video_ctrl(struct
> uvc_streaming *stream,
> }
> }
>
> +static size_t uvc_video_ctrl_size(struct uvc_streaming *stream)
> +{
> + /*
> + * Return the size of the video probe and commit controls, which depends
> + * on the protocol version.
> + */
> + if (stream->dev->uvc_version < 0x0110)
> + return 26;
> + else if (stream->dev->uvc_version < 0x0150)
> + return 34;
> + else
> + return 48;
> +}
> +
> static int uvc_get_video_ctrl(struct uvc_streaming *stream,
> struct uvc_streaming_control *ctrl, int probe, u8 query)
> {
> + u16 size = uvc_video_ctrl_size(stream);
> u8 *data;
> - u16 size;
> int ret;
>
> - size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
> query == UVC_GET_DEF)
> return -EIO;
> @@ -271,7 +284,7 @@ static int uvc_get_video_ctrl(struct uvc_streaming
> *stream,
> ctrl->dwMaxVideoFrameSize = get_unaligned_le32(&data[18]);
> ctrl->dwMaxPayloadTransferSize = get_unaligned_le32(&data[22]);
>
> - if (size == 34) {
> + if (size >= 34) {
> ctrl->dwClockFrequency = get_unaligned_le32(&data[26]);
> ctrl->bmFramingInfo = data[30];
> ctrl->bPreferedVersion = data[31];
> @@ -300,11 +313,10 @@ static int uvc_get_video_ctrl(struct uvc_streaming
> *stream,
> static int uvc_set_video_ctrl(struct uvc_streaming *stream,
> struct uvc_streaming_control *ctrl, int probe)
> {
> + u16 size = uvc_video_ctrl_size(stream);
> u8 *data;
> - u16 size;
> int ret;
>
> - size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> data = kzalloc(size, GFP_KERNEL);
> if (data == NULL)
> return -ENOMEM;
> @@ -321,7 +333,7 @@ static int uvc_set_video_ctrl(struct uvc_streaming
> *stream,
> put_unaligned_le32(ctrl->dwMaxVideoFrameSize, &data[18]);
> put_unaligned_le32(ctrl->dwMaxPayloadTransferSize, &data[22]);
>
> - if (size == 34) {
> + if (size >= 34) {
> put_unaligned_le32(ctrl->dwClockFrequency, &data[26]);
> data[30] = ctrl->bmFramingInfo;
> data[31] = ctrl->bPreferedVersion;
>
> --
> Regards,
>
> Laurent Pinchart

2018-05-23 09:43:54

by Ana Guerrero Lopez

[permalink] [raw]
Subject: Re: [PATCH] media: uvcvideo: Support realtek's UVC 1.5 device

> > commit a9c002732695eab2096580a0d1a1687bc2f95928
> > Author: ming_qian <[email protected]>
> > Date: Wed May 9 10:13:08 2018 +0800
> >
> > media: uvcvideo: Support UVC 1.5 video probe & commit controls
> >
> > The length of UVC 1.5 video control is 48, and it is 34 for UVC 1.1.
> > Change it to 48 for UVC 1.5 device, and the UVC 1.5 device can be
> > recognized.
> >
> > More changes to the driver are needed for full UVC 1.5 compatibility.
> > However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have been
> > reported to work well.
> >
> > Cc: [email protected]
> > Signed-off-by: ming_qian <[email protected]>
> > [Factor out code to helper function, update size checks]
> > Signed-off-by: Laurent Pinchart <[email protected]>
>
> I tested this new patch and it works well.
>
> Tested-by: Kai-Heng Feng <[email protected]>

I tested it as well and it works well.

Tested-by: Ana Guerrero Lopez <[email protected]>


> >
> > diff --git a/drivers/media/usb/uvc/uvc_video.c
> > b/drivers/media/usb/uvc/uvc_video.c
> > index eb9e04a59427..285b0e813b9d 100644
> > --- a/drivers/media/usb/uvc/uvc_video.c
> > +++ b/drivers/media/usb/uvc/uvc_video.c
> > @@ -207,14 +207,27 @@ static void uvc_fixup_video_ctrl(struct
> > uvc_streaming *stream,
> > }
> > }
> >
> > +static size_t uvc_video_ctrl_size(struct uvc_streaming *stream)
> > +{
> > + /*
> > + * Return the size of the video probe and commit controls, which depends
> > + * on the protocol version.
> > + */
> > + if (stream->dev->uvc_version < 0x0110)
> > + return 26;
> > + else if (stream->dev->uvc_version < 0x0150)
> > + return 34;
> > + else
> > + return 48;
> > +}
> > +
> > static int uvc_get_video_ctrl(struct uvc_streaming *stream,
> > struct uvc_streaming_control *ctrl, int probe, u8 query)
> > {
> > + u16 size = uvc_video_ctrl_size(stream);
> > u8 *data;
> > - u16 size;
> > int ret;
> >
> > - size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> > if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
> > query == UVC_GET_DEF)
> > return -EIO;
> > @@ -271,7 +284,7 @@ static int uvc_get_video_ctrl(struct uvc_streaming
> > *stream,
> > ctrl->dwMaxVideoFrameSize = get_unaligned_le32(&data[18]);
> > ctrl->dwMaxPayloadTransferSize = get_unaligned_le32(&data[22]);
> >
> > - if (size == 34) {
> > + if (size >= 34) {
> > ctrl->dwClockFrequency = get_unaligned_le32(&data[26]);
> > ctrl->bmFramingInfo = data[30];
> > ctrl->bPreferedVersion = data[31];
> > @@ -300,11 +313,10 @@ static int uvc_get_video_ctrl(struct uvc_streaming
> > *stream,
> > static int uvc_set_video_ctrl(struct uvc_streaming *stream,
> > struct uvc_streaming_control *ctrl, int probe)
> > {
> > + u16 size = uvc_video_ctrl_size(stream);
> > u8 *data;
> > - u16 size;
> > int ret;
> >
> > - size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> > data = kzalloc(size, GFP_KERNEL);
> > if (data == NULL)
> > return -ENOMEM;
> > @@ -321,7 +333,7 @@ static int uvc_set_video_ctrl(struct uvc_streaming
> > *stream,
> > put_unaligned_le32(ctrl->dwMaxVideoFrameSize, &data[18]);
> > put_unaligned_le32(ctrl->dwMaxPayloadTransferSize, &data[22]);
> >
> > - if (size == 34) {
> > + if (size >= 34) {
> > put_unaligned_le32(ctrl->dwClockFrequency, &data[26]);
> > data[30] = ctrl->bmFramingInfo;
> > data[31] = ctrl->bPreferedVersion;
> >
> > --
> > Regards,
> >
> > Laurent Pinchart

2018-05-25 02:50:41

by Darren Hart

[permalink] [raw]
Subject: Re: media: uvcvideo: Support realtek's UVC 1.5 device

On Fri, May 18, 2018 at 04:04:35PM +0200, Ana Guerrero Lopez wrote:
> On Wed, May 09, 2018 at 10:13:08AM +0800, [email protected] wrote:
> > From: ming_qian <[email protected]>
> >
> > The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1.
> > Change it to 48 for UVC 1.5 device,
> > and the UVC 1.5 device can be recognized.
> >
> > More changes to the driver are needed for full UVC 1.5 compatibility.
> > However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have
> > been reported to work well.
> >
> > Signed-off-by: ming_qian <[email protected]>
> > Tested-by: Kai-Heng Feng <[email protected]>
> > Reviewed-by: Hans de Goede <[email protected]>
> > Tested-by: Josef Šimánek <[email protected]>
>
> It works perfectly here on 4.16.5 with a Dell XPS 9370 in Debian.
>
> Tested-by: Ana Guerrero Lopez <[email protected]>

I worked with Eilís Ní Fhlannagáin on social media who has also confirmed this to solve
her camera issue with her Dell XPS 13 (I didn't get the specific model).

Took a looking at linux-next today, I didn't see it. Anything else needed to get
this queued up? We'd love to get this in the 4.18 merge window, and stable if
possible.

--
Darren Hart
VMware Open Source Technology Center