Re: [PATCH v1 05/19] uvcvideo: Add support for UVC1.5 PC control.

2013-09-03 Thread Laurent Pinchart
Hi Pawel,

Thank you for the patch.

On Friday 30 August 2013 11:17:04 Pawel Osciak wrote:
 Add support for UVC 1.5 Probe  Commit control.
 
 Signed-off-by: Pawel Osciak posc...@chromium.org
 ---
  drivers/media/usb/uvc/uvc_video.c | 52 +---
  include/uapi/linux/usb/video.h|  7 ++
  2 files changed, 55 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/media/usb/uvc/uvc_video.c
 b/drivers/media/usb/uvc/uvc_video.c index 1198989..b4ebccd 100644
 --- a/drivers/media/usb/uvc/uvc_video.c
 +++ b/drivers/media/usb/uvc/uvc_video.c
 @@ -168,14 +168,25 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming
 *stream, }
  }
 
 +int uvc_get_probe_ctrl_size(struct uvc_streaming *stream)
 +{
 + 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)
  {
   __u8 *data;
   __u16 size;
   int ret;
 + int i;

Could you please make i unsigned, as it's used as an unsigned loop counter ?

 - size = stream-dev-uvc_version = 0x0110 ? 34 : 26;
 + size = uvc_get_probe_ctrl_size(stream);
   if ((stream-dev-quirks  UVC_QUIRK_PROBE_DEF) 
   query == UVC_GET_DEF)
   return -EIO;
 @@ -230,7 +241,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];
 @@ -244,6 +255,26 @@ static int uvc_get_video_ctrl(struct uvc_streaming
 *stream, ctrl-bMaxVersion = 0;
   }
 
 + if (size = 48) {
 + ctrl-bUsage = data[34];
 + ctrl-bBitDepthLuma = data[35];
 + ctrl-bmSetting = data[36];
 + ctrl-bMaxNumberOfRefFramesPlus1 = data[37];
 + ctrl-bmRateControlModes = get_unaligned_le16(data[38]);
 + for (i = 0; i  ARRAY_SIZE(ctrl-bmLayoutPerStream); ++i) {
 + ctrl-bmLayoutPerStream[i] =
 + get_unaligned_le16(data[40 + i * 2]);
 + }
 + } else {
 + ctrl-bUsage = 0;
 + ctrl-bBitDepthLuma = 0;
 + ctrl-bmSetting = 0;
 + ctrl-bMaxNumberOfRefFramesPlus1 = 0;
 + ctrl-bmRateControlModes = 0;
 + for (i = 0; i  ARRAY_SIZE(ctrl-bmLayoutPerStream); ++i)
 + ctrl-bmLayoutPerStream[i] = 0;
 + }

I wonder whether we shouldn't just memset the whole structure to zero to start 
with. What do you think ?

 +
   /* Some broken devices return null or wrong dwMaxVideoFrameSize and
* dwMaxPayloadTransferSize fields. Try to get the value from the
* format and frame descriptors.
 @@ -262,8 +293,9 @@ static int uvc_set_video_ctrl(struct uvc_streaming
 *stream, __u8 *data;
   __u16 size;
   int ret;
 + int i;

Unsigned here as well please.

 
 - size = stream-dev-uvc_version = 0x0110 ? 34 : 26;
 + size = uvc_get_probe_ctrl_size(stream);
   data = kzalloc(size, GFP_KERNEL);
   if (data == NULL)
   return -ENOMEM;
 @@ -280,7 +312,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;
 @@ -288,6 +320,18 @@ static int uvc_set_video_ctrl(struct uvc_streaming
 *stream, data[33] = ctrl-bMaxVersion;
   }
 
 + if (size = 48) {
 + data[34] = ctrl-bUsage;
 + data[35] = ctrl-bBitDepthLuma;
 + data[36] = ctrl-bmSetting;
 + data[37] = ctrl-bMaxNumberOfRefFramesPlus1;
 + *(__le16 *)data[38] = cpu_to_le16(ctrl-bmRateControlModes);
 + for (i = 0; i  ARRAY_SIZE(ctrl-bmLayoutPerStream); ++i) {
 + *(__le16 *)data[40 + i * 2] =
 + cpu_to_le16(ctrl-bmLayoutPerStream[i]);
 + }
 + }
 +
   ret = __uvc_query_ctrl(stream-dev, UVC_SET_CUR, 0, stream-intfnum,
   probe ? UVC_VS_PROBE_CONTROL : UVC_VS_COMMIT_CONTROL, data,
   size, uvc_timeout_param);
 diff --git a/include/uapi/linux/usb/video.h b/include/uapi/linux/usb/video.h
 index 3b3b95e..331c071 100644
 --- a/include/uapi/linux/usb/video.h
 +++ b/include/uapi/linux/usb/video.h
 @@ -432,6 +432,7 @@ struct uvc_color_matching_descriptor {
  #define 

[PATCH v1 05/19] uvcvideo: Add support for UVC1.5 PC control.

2013-08-29 Thread Pawel Osciak
Add support for UVC 1.5 Probe  Commit control.

Signed-off-by: Pawel Osciak posc...@chromium.org
---
 drivers/media/usb/uvc/uvc_video.c | 52 ---
 include/uapi/linux/usb/video.h|  7 ++
 2 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c 
b/drivers/media/usb/uvc/uvc_video.c
index 1198989..b4ebccd 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -168,14 +168,25 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming 
*stream,
}
 }
 
+int uvc_get_probe_ctrl_size(struct uvc_streaming *stream)
+{
+   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)
 {
__u8 *data;
__u16 size;
int ret;
+   int i;
 
-   size = stream-dev-uvc_version = 0x0110 ? 34 : 26;
+   size = uvc_get_probe_ctrl_size(stream);
if ((stream-dev-quirks  UVC_QUIRK_PROBE_DEF) 
query == UVC_GET_DEF)
return -EIO;
@@ -230,7 +241,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];
@@ -244,6 +255,26 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
ctrl-bMaxVersion = 0;
}
 
+   if (size = 48) {
+   ctrl-bUsage = data[34];
+   ctrl-bBitDepthLuma = data[35];
+   ctrl-bmSetting = data[36];
+   ctrl-bMaxNumberOfRefFramesPlus1 = data[37];
+   ctrl-bmRateControlModes = get_unaligned_le16(data[38]);
+   for (i = 0; i  ARRAY_SIZE(ctrl-bmLayoutPerStream); ++i) {
+   ctrl-bmLayoutPerStream[i] =
+   get_unaligned_le16(data[40 + i * 2]);
+   }
+   } else {
+   ctrl-bUsage = 0;
+   ctrl-bBitDepthLuma = 0;
+   ctrl-bmSetting = 0;
+   ctrl-bMaxNumberOfRefFramesPlus1 = 0;
+   ctrl-bmRateControlModes = 0;
+   for (i = 0; i  ARRAY_SIZE(ctrl-bmLayoutPerStream); ++i)
+   ctrl-bmLayoutPerStream[i] = 0;
+   }
+
/* Some broken devices return null or wrong dwMaxVideoFrameSize and
 * dwMaxPayloadTransferSize fields. Try to get the value from the
 * format and frame descriptors.
@@ -262,8 +293,9 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
__u8 *data;
__u16 size;
int ret;
+   int i;
 
-   size = stream-dev-uvc_version = 0x0110 ? 34 : 26;
+   size = uvc_get_probe_ctrl_size(stream);
data = kzalloc(size, GFP_KERNEL);
if (data == NULL)
return -ENOMEM;
@@ -280,7 +312,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;
@@ -288,6 +320,18 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
data[33] = ctrl-bMaxVersion;
}
 
+   if (size = 48) {
+   data[34] = ctrl-bUsage;
+   data[35] = ctrl-bBitDepthLuma;
+   data[36] = ctrl-bmSetting;
+   data[37] = ctrl-bMaxNumberOfRefFramesPlus1;
+   *(__le16 *)data[38] = cpu_to_le16(ctrl-bmRateControlModes);
+   for (i = 0; i  ARRAY_SIZE(ctrl-bmLayoutPerStream); ++i) {
+   *(__le16 *)data[40 + i * 2] =
+   cpu_to_le16(ctrl-bmLayoutPerStream[i]);
+   }
+   }
+
ret = __uvc_query_ctrl(stream-dev, UVC_SET_CUR, 0, stream-intfnum,
probe ? UVC_VS_PROBE_CONTROL : UVC_VS_COMMIT_CONTROL, data,
size, uvc_timeout_param);
diff --git a/include/uapi/linux/usb/video.h b/include/uapi/linux/usb/video.h
index 3b3b95e..331c071 100644
--- a/include/uapi/linux/usb/video.h
+++ b/include/uapi/linux/usb/video.h
@@ -432,6 +432,7 @@ struct uvc_color_matching_descriptor {
 #define UVC_DT_COLOR_MATCHING_SIZE 6
 
 /* 4.3.1.1. Video Probe and Commit Controls */
+#define UVC_NUM_SIMULCAST_STREAMS  4
 struct uvc_streaming_control {
__u16 bmHint;