All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] usb: gadget: uvc: add framebased stream support
@ 2022-02-16  8:16 3090101217
  2022-02-16 18:22   ` kernel test robot
  2022-02-17 10:44 ` [PATCH v2] " 3090101217
  0 siblings, 2 replies; 9+ messages in thread
From: 3090101217 @ 2022-02-16  8:16 UTC (permalink / raw)
  To: laurent.pinchart, balbi, gregkh, corbet, mchehab+huawei, rdunlap,
	bilbao, pawell
  Cc: linux-kernel, linux-usb, Jing Leng

From: Jing Leng <jleng@ambarella.com>

Currently the uvc gadget can't support H264/HEVC transport, After
adding framebased stream support, the driver can support them.

Framebased stream is a little different from uncompressed stream.
So we can support framebased stream on the basis of uncompressed stream.

Here are the differences:

1. For the format, framebased format has a extra member (
__u8 bVariableSize) than uncompressed format.

2. For the frame, the layout of last three members of framebased frame
is different from uncompressed frame.
a. Last three members of uncompressed frame are:
  u32	dw_max_video_frame_buffer_size;
  u32	dw_default_frame_interval;
  u8	b_frame_interval_type;
b. Last three members of framebased frame are:
  u32	dw_default_frame_interval;
  u8	b_frame_interval_type;
  u32	dw_bytes_perline;

Here is an example of configuring H264:

cd /sys/kernel/config/usb_gadget/g1
ndir=functions/uvc.usb0/streaming/uncompressed/$NAME
mkdir -p $ndir
echo -n "H264" > $ndir/guidFormat  # H264 or HEVC
echo 0 > $ndir/bBitsPerPixel
echo 1 > $ndir/bVariableSize
wdir=functions/uvc.usb0/streaming/uncompressed/$NAME/${HEIGHT}p
mkdir -p $wdir
echo 0 > $wdir/dwBytesPerLine
echo $WIDTH  > $wdir/wWidth
echo $HEIGHT > $wdir/wHeight
echo 29491200 > $wdir/dwMinBitRate
echo 29491200 > $wdir/dwMaxBitRate
cat <<EOF > $wdir/dwFrameInterval
$INTERVAL
EOF

Signed-off-by: Jing Leng <jleng@ambarella.com>
---
 .../ABI/testing/configfs-usb-gadget-uvc       | 13 +++-
 drivers/usb/gadget/function/uvc_configfs.c    | 72 +++++++++++++++++--
 drivers/usb/gadget/function/uvc_v4l2.c        |  2 +
 include/uapi/linux/usb/video.h                |  3 +
 4 files changed, 81 insertions(+), 9 deletions(-)

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
index 889ed45be4ca..2bf515dad516 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
@@ -243,7 +243,7 @@ Description:	Uncompressed format descriptors
 What:		/config/usb-gadget/gadget/functions/uvc.name/streaming/uncompressed/name
 Date:		Dec 2014
 KernelVersion:	4.0
-Description:	Specific uncompressed format descriptors
+Description:	Specific uncompressed/framebased format descriptors
 
 		==================	=======================================
 		bFormatIndex		unique id for this format descriptor;
@@ -264,12 +264,15 @@ Description:	Specific uncompressed format descriptors
 					frame
 		guidFormat		globally unique id used to identify
 					stream-encoding format
+		bVariableSize		whether the data within the frame is of
+					variable length from frame to frame (
+					only for framebased format)
 		==================	=======================================
 
 What:		/config/usb-gadget/gadget/functions/uvc.name/streaming/uncompressed/name/name
 Date:		Dec 2014
 KernelVersion:	4.0
-Description:	Specific uncompressed frame descriptors
+Description:	Specific uncompressed/framebased frame descriptors
 
 		=========================  =====================================
 		bFrameIndex		   unique id for this framedescriptor;
@@ -283,7 +286,11 @@ Description:	Specific uncompressed frame descriptors
 					   like to use as default
 		dwMaxVideoFrameBufferSize  the maximum number of bytes the
 					   compressor will produce for a video
-					   frame or still image
+					   frame or still image (only for
+					   uncompressed frame)
+		dwBytesPerLine		   the per-line bytes of the framebased
+					   frame, e.g. H264 or HEVC (only for
+					   framebased frame)
 		dwMaxBitRate		   the maximum bit rate at the shortest
 					   frame interval in bps
 		dwMinBitRate		   the minimum bit rate at the longest
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 77d64031aa9c..d7c1c96fd8a3 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -11,6 +11,7 @@
  */
 
 #include <linux/sort.h>
+#include <linux/videodev2.h>
 
 #include "u_uvc.h"
 #include "uvc_configfs.h"
@@ -782,6 +783,8 @@ struct uvcg_format {
 	__u8			bmaControls[UVCG_STREAMING_CONTROL_SIZE];
 };
 
+static u8 uvcg_uncompressed_subtype(struct uvcg_format *fmt);
+
 static struct uvcg_format *to_uvcg_format(struct config_item *item)
 {
 	return container_of(to_config_group(item), struct uvcg_format, group);
@@ -1072,7 +1075,23 @@ struct uvcg_frame {
 		u16	w_height;
 		u32	dw_min_bit_rate;
 		u32	dw_max_bit_rate;
-		u32	dw_max_video_frame_buffer_size;
+
+		/*
+		 * The layout of last three members of framebased frame
+		 * is different from uncompressed frame.
+		 *   Last three members of uncompressed frame are:
+		 *     u32	dw_max_video_frame_buffer_size;
+		 *     u32	dw_default_frame_interval;
+		 *     u8	b_frame_interval_type;
+		 *   Last three members of framebased frame are:
+		 *     u32	dw_default_frame_interval;
+		 *     u8	b_frame_interval_type;
+		 *     u32	dw_bytes_perline;
+		 */
+		union {
+			u32	dw_max_video_frame_buffer_size;
+			u32	dw_bytes_perline;
+		};
 		u32	dw_default_frame_interval;
 		u8	b_frame_interval_type;
 	} __attribute__((packed)) frame;
@@ -1185,6 +1204,7 @@ UVCG_FRAME_ATTR(dw_min_bit_rate, dwMinBitRate, 32);
 UVCG_FRAME_ATTR(dw_max_bit_rate, dwMaxBitRate, 32);
 UVCG_FRAME_ATTR(dw_max_video_frame_buffer_size, dwMaxVideoFrameBufferSize, 32);
 UVCG_FRAME_ATTR(dw_default_frame_interval, dwDefaultFrameInterval, 32);
+UVCG_FRAME_ATTR(dw_bytes_perline, dwBytesPerLine, 32);
 
 #undef UVCG_FRAME_ATTR
 
@@ -1329,6 +1349,7 @@ static struct configfs_attribute *uvcg_frame_attrs[] = {
 	&uvcg_frame_attr_dw_max_video_frame_buffer_size,
 	&uvcg_frame_attr_dw_default_frame_interval,
 	&uvcg_frame_attr_dw_frame_interval,
+	&uvcg_frame_attr_dw_bytes_perline,
 	NULL,
 };
 
@@ -1365,7 +1386,12 @@ static struct config_item *uvcg_frame_make(struct config_group *group,
 	mutex_lock(&opts->lock);
 	fmt = to_uvcg_format(&group->cg_item);
 	if (fmt->type == UVCG_UNCOMPRESSED) {
-		h->frame.b_descriptor_subtype = UVC_VS_FRAME_UNCOMPRESSED;
+		if (uvcg_uncompressed_subtype(fmt) == UVC_VS_FORMAT_UNCOMPRESSED) {
+			h->frame.b_descriptor_subtype = UVC_VS_FRAME_UNCOMPRESSED;
+		} else {
+			h->frame.b_descriptor_subtype = UVC_VS_FRAME_FRAME_BASED;
+			h->frame.dw_bytes_perline = 0;
+		}
 		h->fmt_type = UVCG_UNCOMPRESSED;
 	} else if (fmt->type == UVCG_MJPEG) {
 		h->frame.b_descriptor_subtype = UVC_VS_FRAME_MJPEG;
@@ -1425,6 +1451,14 @@ struct uvcg_uncompressed {
 	struct uvc_format_uncompressed	desc;
 };
 
+static u8 uvcg_uncompressed_subtype(struct uvcg_format *fmt)
+{
+	struct uvcg_uncompressed *ch = container_of(fmt,
+					struct uvcg_uncompressed, fmt);
+
+	return ch->desc.bDescriptorSubType;
+}
+
 static struct uvcg_uncompressed *to_uvcg_uncompressed(struct config_item *item)
 {
 	return container_of(
@@ -1466,6 +1500,7 @@ static ssize_t uvcg_uncompressed_guid_format_store(struct config_item *item,
 	struct f_uvc_opts *opts;
 	struct config_item *opts_item;
 	struct mutex *su_mutex = &ch->fmt.group.cg_subsys->su_mutex;
+	u32 fcc = 0;
 	int ret;
 
 	mutex_lock(su_mutex); /* for navigating configfs hierarchy */
@@ -1481,7 +1516,17 @@ static ssize_t uvcg_uncompressed_guid_format_store(struct config_item *item,
 
 	memcpy(ch->desc.guidFormat, page,
 	       min(sizeof(ch->desc.guidFormat), len));
-	ret = sizeof(ch->desc.guidFormat);
+	ret = len;
+
+	fcc = v4l2_fourcc(ch->desc.guidFormat[0], ch->desc.guidFormat[1],
+		ch->desc.guidFormat[2], ch->desc.guidFormat[3]);
+	if (fcc == V4L2_PIX_FMT_H264 || fcc == V4L2_PIX_FMT_HEVC) {
+		ch->desc.bLength		= UVC_DT_FORMAT_FRAMEBASED_SIZE;
+		ch->desc.bDescriptorSubType	= UVC_VS_FORMAT_FRAME_BASED;
+	} else {
+		ch->desc.bLength		= UVC_DT_FORMAT_UNCOMPRESSED_SIZE;
+		ch->desc.bDescriptorSubType	= UVC_VS_FORMAT_UNCOMPRESSED;
+	}
 
 end:
 	mutex_unlock(&opts->lock);
@@ -1581,6 +1626,7 @@ UVCG_UNCOMPRESSED_ATTR(b_default_frame_index, bDefaultFrameIndex, 8);
 UVCG_UNCOMPRESSED_ATTR_RO(b_aspect_ratio_x, bAspectRatioX, 8);
 UVCG_UNCOMPRESSED_ATTR_RO(b_aspect_ratio_y, bAspectRatioY, 8);
 UVCG_UNCOMPRESSED_ATTR_RO(bm_interface_flags, bmInterfaceFlags, 8);
+UVCG_UNCOMPRESSED_ATTR(b_variable_size, bVariableSize, 8);
 
 #undef UVCG_UNCOMPRESSED_ATTR
 #undef UVCG_UNCOMPRESSED_ATTR_RO
@@ -1611,6 +1657,7 @@ static struct configfs_attribute *uvcg_uncompressed_attrs[] = {
 	&uvcg_uncompressed_attr_b_aspect_ratio_y,
 	&uvcg_uncompressed_attr_bm_interface_flags,
 	&uvcg_uncompressed_attr_bma_controls,
+	&uvcg_uncompressed_attr_b_variable_size,
 	NULL,
 };
 
@@ -1644,6 +1691,7 @@ static struct config_group *uvcg_uncompressed_make(struct config_group *group,
 	h->desc.bAspectRatioY		= 0;
 	h->desc.bmInterfaceFlags	= 0;
 	h->desc.bCopyProtect		= 0;
+	h->desc.bVariableSize		= 0;
 
 	h->fmt.type = UVCG_UNCOMPRESSED;
 	config_group_init_type_name(&h->fmt.group, name,
@@ -2038,7 +2086,7 @@ static int __uvcg_cnt_strm(void *priv1, void *priv2, void *priv3, int n,
 				container_of(fmt, struct uvcg_uncompressed,
 					     fmt);
 
-			*size += sizeof(u->desc);
+			*size += u->desc.bLength;
 		} else if (fmt->type == UVCG_MJPEG) {
 			struct uvcg_mjpeg *m =
 				container_of(fmt, struct uvcg_mjpeg, fmt);
@@ -2108,8 +2156,8 @@ static int __uvcg_fill_strm(void *priv1, void *priv2, void *priv3, int n,
 
 			u->desc.bFormatIndex = n + 1;
 			u->desc.bNumFrameDescriptors = fmt->num_frames;
-			memcpy(*dest, &u->desc, sizeof(u->desc));
-			*dest += sizeof(u->desc);
+			memcpy(*dest, &u->desc, u->desc.bLength);
+			*dest += u->desc.bLength;
 		} else if (fmt->type == UVCG_MJPEG) {
 			struct uvcg_mjpeg *m =
 				container_of(fmt, struct uvcg_mjpeg, fmt);
@@ -2129,6 +2177,18 @@ static int __uvcg_fill_strm(void *priv1, void *priv2, void *priv3, int n,
 
 		sz = sizeof(frm->frame);
 		memcpy(*dest, &frm->frame, sz);
+		/*
+		 * Reorder the frame struct layout due to the difference
+		 * between uncompressed frame and framebased frame.
+		 */
+		if (frm->frame.b_descriptor_subtype == UVC_VS_FRAME_FRAME_BASED) {
+			u8 *data = (u8 *)*dest;
+
+			memcpy(data + 17, &frm->frame.dw_default_frame_interval, 4);
+			memcpy(data + 21, &frm->frame.b_frame_interval_type, 1);
+			memcpy(data + 22, &frm->frame.dw_bytes_perline, 4);
+		}
+
 		*dest += sz;
 		sz = frm->frame.b_frame_interval_type *
 			sizeof(*frm->dw_frame_interval);
diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c
index a2c78690c5c2..3d6217328c50 100644
--- a/drivers/usb/gadget/function/uvc_v4l2.c
+++ b/drivers/usb/gadget/function/uvc_v4l2.c
@@ -58,6 +58,8 @@ struct uvc_format {
 static struct uvc_format uvc_formats[] = {
 	{ 16, V4L2_PIX_FMT_YUYV  },
 	{ 0,  V4L2_PIX_FMT_MJPEG },
+	{ 0,  V4L2_PIX_FMT_H264 },
+	{ 0,  V4L2_PIX_FMT_HEVC },
 };
 
 static int
diff --git a/include/uapi/linux/usb/video.h b/include/uapi/linux/usb/video.h
index bfdae12cdacf..383980bc9618 100644
--- a/include/uapi/linux/usb/video.h
+++ b/include/uapi/linux/usb/video.h
@@ -468,9 +468,12 @@ struct uvc_format_uncompressed {
 	__u8  bAspectRatioY;
 	__u8  bmInterfaceFlags;
 	__u8  bCopyProtect;
+	/* bVariableSize is only for framebased format. */
+	__u8  bVariableSize;
 } __attribute__((__packed__));
 
 #define UVC_DT_FORMAT_UNCOMPRESSED_SIZE			27
+#define UVC_DT_FORMAT_FRAMEBASED_SIZE			28
 
 /* Uncompressed Payload - 3.1.2. Uncompressed Video Frame Descriptor */
 struct uvc_frame_uncompressed {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] usb: gadget: uvc: add framebased stream support
  2022-02-16  8:16 [PATCH] usb: gadget: uvc: add framebased stream support 3090101217
@ 2022-02-16 18:22   ` kernel test robot
  2022-02-17 10:44 ` [PATCH v2] " 3090101217
  1 sibling, 0 replies; 9+ messages in thread
From: kernel test robot @ 2022-02-16 18:22 UTC (permalink / raw)
  To: 3090101217, laurent.pinchart, balbi, gregkh, corbet,
	mchehab+huawei, rdunlap, bilbao, pawell
  Cc: llvm, kbuild-all, linux-kernel, linux-usb, Jing Leng

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on linus/master v5.17-rc4 next-20220216]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/3090101217-zju-edu-cn/usb-gadget-uvc-add-framebased-stream-support/20220216-162037
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: arm-randconfig-r023-20220216 (https://download.01.org/0day-ci/archive/20220217/202202170211.van9U4Ha-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 0e628a783b935c70c80815db6c061ec84f884af5)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/475fba6b60329d9270b699550907a5e077cd84b6
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review 3090101217-zju-edu-cn/usb-gadget-uvc-add-framebased-stream-support/20220216-162037
        git checkout 475fba6b60329d9270b699550907a5e077cd84b6
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash block// drivers/usb/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/usb/gadget/function/uvc_configfs.c:1091:3: warning: field  within 'struct uvcg_frame::(unnamed at drivers/usb/gadget/function/uvc_configfs.c:1068:2)' is less aligned than 'union uvcg_frame::(anonymous at drivers/usb/gadget/function/uvc_configfs.c:1091:3)' and is usually due to 'struct uvcg_frame::(unnamed at drivers/usb/gadget/function/uvc_configfs.c:1068:2)' being packed, which can lead to unaligned accesses [-Wunaligned-access]
                   union {
                   ^
   1 warning generated.


vim +1091 drivers/usb/gadget/function/uvc_configfs.c

  1060	
  1061	/* -----------------------------------------------------------------------------
  1062	 * streaming/<mode>/<format>/<NAME>
  1063	 */
  1064	
  1065	struct uvcg_frame {
  1066		struct config_item	item;
  1067		enum uvcg_format_type	fmt_type;
  1068		struct {
  1069			u8	b_length;
  1070			u8	b_descriptor_type;
  1071			u8	b_descriptor_subtype;
  1072			u8	b_frame_index;
  1073			u8	bm_capabilities;
  1074			u16	w_width;
  1075			u16	w_height;
  1076			u32	dw_min_bit_rate;
  1077			u32	dw_max_bit_rate;
  1078	
  1079			/*
  1080			 * The layout of last three members of framebased frame
  1081			 * is different from uncompressed frame.
  1082			 *   Last three members of uncompressed frame are:
  1083			 *     u32	dw_max_video_frame_buffer_size;
  1084			 *     u32	dw_default_frame_interval;
  1085			 *     u8	b_frame_interval_type;
  1086			 *   Last three members of framebased frame are:
  1087			 *     u32	dw_default_frame_interval;
  1088			 *     u8	b_frame_interval_type;
  1089			 *     u32	dw_bytes_perline;
  1090			 */
> 1091			union {
  1092				u32	dw_max_video_frame_buffer_size;
  1093				u32	dw_bytes_perline;
  1094			};
  1095			u32	dw_default_frame_interval;
  1096			u8	b_frame_interval_type;
  1097		} __attribute__((packed)) frame;
  1098		u32 *dw_frame_interval;
  1099	};
  1100	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] usb: gadget: uvc: add framebased stream support
@ 2022-02-16 18:22   ` kernel test robot
  0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2022-02-16 18:22 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3907 bytes --]

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on linus/master v5.17-rc4 next-20220216]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/3090101217-zju-edu-cn/usb-gadget-uvc-add-framebased-stream-support/20220216-162037
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: arm-randconfig-r023-20220216 (https://download.01.org/0day-ci/archive/20220217/202202170211.van9U4Ha-lkp(a)intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 0e628a783b935c70c80815db6c061ec84f884af5)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/475fba6b60329d9270b699550907a5e077cd84b6
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review 3090101217-zju-edu-cn/usb-gadget-uvc-add-framebased-stream-support/20220216-162037
        git checkout 475fba6b60329d9270b699550907a5e077cd84b6
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash block// drivers/usb/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/usb/gadget/function/uvc_configfs.c:1091:3: warning: field  within 'struct uvcg_frame::(unnamed at drivers/usb/gadget/function/uvc_configfs.c:1068:2)' is less aligned than 'union uvcg_frame::(anonymous at drivers/usb/gadget/function/uvc_configfs.c:1091:3)' and is usually due to 'struct uvcg_frame::(unnamed at drivers/usb/gadget/function/uvc_configfs.c:1068:2)' being packed, which can lead to unaligned accesses [-Wunaligned-access]
                   union {
                   ^
   1 warning generated.


vim +1091 drivers/usb/gadget/function/uvc_configfs.c

  1060	
  1061	/* -----------------------------------------------------------------------------
  1062	 * streaming/<mode>/<format>/<NAME>
  1063	 */
  1064	
  1065	struct uvcg_frame {
  1066		struct config_item	item;
  1067		enum uvcg_format_type	fmt_type;
  1068		struct {
  1069			u8	b_length;
  1070			u8	b_descriptor_type;
  1071			u8	b_descriptor_subtype;
  1072			u8	b_frame_index;
  1073			u8	bm_capabilities;
  1074			u16	w_width;
  1075			u16	w_height;
  1076			u32	dw_min_bit_rate;
  1077			u32	dw_max_bit_rate;
  1078	
  1079			/*
  1080			 * The layout of last three members of framebased frame
  1081			 * is different from uncompressed frame.
  1082			 *   Last three members of uncompressed frame are:
  1083			 *     u32	dw_max_video_frame_buffer_size;
  1084			 *     u32	dw_default_frame_interval;
  1085			 *     u8	b_frame_interval_type;
  1086			 *   Last three members of framebased frame are:
  1087			 *     u32	dw_default_frame_interval;
  1088			 *     u8	b_frame_interval_type;
  1089			 *     u32	dw_bytes_perline;
  1090			 */
> 1091			union {
  1092				u32	dw_max_video_frame_buffer_size;
  1093				u32	dw_bytes_perline;
  1094			};
  1095			u32	dw_default_frame_interval;
  1096			u8	b_frame_interval_type;
  1097		} __attribute__((packed)) frame;
  1098		u32 *dw_frame_interval;
  1099	};
  1100	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] usb: gadget: uvc: add framebased stream support
  2022-02-16  8:16 [PATCH] usb: gadget: uvc: add framebased stream support 3090101217
  2022-02-16 18:22   ` kernel test robot
@ 2022-02-17 10:44 ` 3090101217
  2022-02-17 15:27   ` Greg KH
  2022-05-12  9:43   ` [PATCH v3] " 3090101217
  1 sibling, 2 replies; 9+ messages in thread
From: 3090101217 @ 2022-02-17 10:44 UTC (permalink / raw)
  To: gregkh, balbi, bilbao, corbet, laurent.pinchart, mchehab+huawei,
	pawell, rdunlap
  Cc: linux-kernel, linux-usb, Jing Leng

From: Jing Leng <jleng@ambarella.com>

Currently the uvc gadget can't support H264/HEVC transport, After
adding framebased stream support, the driver can support them.

Framebased stream is a little different from uncompressed stream.
So we can support framebased stream on the basis of uncompressed stream.

Here are the differences:

1. For the format, framebased format has an extra member (
__u8 bVariableSize) than uncompressed format.

2. For the frame, the layout of last three members of framebased frame
is different from uncompressed frame.
a. Last three members of uncompressed frame are:
  u32	dw_max_video_frame_buffer_size;
  u32	dw_default_frame_interval;
  u8	b_frame_interval_type;
b. Last three members of framebased frame are:
  u32	dw_default_frame_interval;
  u8	b_frame_interval_type;
  u32	dw_bytes_perline;

Here is an example of configuring H264:

cd /sys/kernel/config/usb_gadget/g1
ndir=functions/uvc.usb0/streaming/uncompressed/$NAME
mkdir -p $ndir
echo -n "H264" > $ndir/guidFormat  # H264 or HEVC
echo 0 > $ndir/bBitsPerPixel
echo 1 > $ndir/bVariableSize
wdir=functions/uvc.usb0/streaming/uncompressed/$NAME/${HEIGHT}p
mkdir -p $wdir
echo 0 > $wdir/dwBytesPerLine
echo $WIDTH  > $wdir/wWidth
echo $HEIGHT > $wdir/wHeight
echo 29491200 > $wdir/dwMinBitRate
echo 29491200 > $wdir/dwMaxBitRate
cat <<EOF > $wdir/dwFrameInterval
$INTERVAL
EOF

Signed-off-by: Jing Leng <jleng@ambarella.com>
---
ChangeLog v1->v2:
- Use another way to handle frames, previous implementation within
- using union has a warning. (Reported-by: kernel test robot <lkp@intel.com>)
---
 .../ABI/testing/configfs-usb-gadget-uvc       | 13 +++-
 drivers/usb/gadget/function/uvc_configfs.c    | 67 +++++++++++++++++--
 drivers/usb/gadget/function/uvc_v4l2.c        |  2 +
 include/uapi/linux/usb/video.h                |  3 +
 4 files changed, 75 insertions(+), 10 deletions(-)

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
index 889ed45be4ca..2bf515dad516 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
@@ -243,7 +243,7 @@ Description:	Uncompressed format descriptors
 What:		/config/usb-gadget/gadget/functions/uvc.name/streaming/uncompressed/name
 Date:		Dec 2014
 KernelVersion:	4.0
-Description:	Specific uncompressed format descriptors
+Description:	Specific uncompressed/framebased format descriptors
 
 		==================	=======================================
 		bFormatIndex		unique id for this format descriptor;
@@ -264,12 +264,15 @@ Description:	Specific uncompressed format descriptors
 					frame
 		guidFormat		globally unique id used to identify
 					stream-encoding format
+		bVariableSize		whether the data within the frame is of
+					variable length from frame to frame (
+					only for framebased format)
 		==================	=======================================
 
 What:		/config/usb-gadget/gadget/functions/uvc.name/streaming/uncompressed/name/name
 Date:		Dec 2014
 KernelVersion:	4.0
-Description:	Specific uncompressed frame descriptors
+Description:	Specific uncompressed/framebased frame descriptors
 
 		=========================  =====================================
 		bFrameIndex		   unique id for this framedescriptor;
@@ -283,7 +286,11 @@ Description:	Specific uncompressed frame descriptors
 					   like to use as default
 		dwMaxVideoFrameBufferSize  the maximum number of bytes the
 					   compressor will produce for a video
-					   frame or still image
+					   frame or still image (only for
+					   uncompressed frame)
+		dwBytesPerLine		   the per-line bytes of the framebased
+					   frame, e.g. H264 or HEVC (only for
+					   framebased frame)
 		dwMaxBitRate		   the maximum bit rate at the shortest
 					   frame interval in bps
 		dwMinBitRate		   the minimum bit rate at the longest
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 77d64031aa9c..695f9b68290f 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -11,6 +11,7 @@
  */
 
 #include <linux/sort.h>
+#include <linux/videodev2.h>
 
 #include "u_uvc.h"
 #include "uvc_configfs.h"
@@ -782,6 +783,8 @@ struct uvcg_format {
 	__u8			bmaControls[UVCG_STREAMING_CONTROL_SIZE];
 };
 
+static u8 uvcg_uncompressed_subtype(struct uvcg_format *fmt);
+
 static struct uvcg_format *to_uvcg_format(struct config_item *item)
 {
 	return container_of(to_config_group(item), struct uvcg_format, group);
@@ -1072,9 +1075,23 @@ struct uvcg_frame {
 		u16	w_height;
 		u32	dw_min_bit_rate;
 		u32	dw_max_bit_rate;
+
+		/*
+		 * The layout of last three members of framebased frame
+		 * is different from uncompressed frame.
+		 *   Last three members of uncompressed frame are:
+		 *     u32	dw_max_video_frame_buffer_size;
+		 *     u32	dw_default_frame_interval;
+		 *     u8	b_frame_interval_type;
+		 *   Last three members of framebased frame are:
+		 *     u32	dw_default_frame_interval;
+		 *     u8	b_frame_interval_type;
+		 *     u32	dw_bytes_perline;
+		 */
 		u32	dw_max_video_frame_buffer_size;
 		u32	dw_default_frame_interval;
 		u8	b_frame_interval_type;
+		u32	dw_bytes_perline;
 	} __attribute__((packed)) frame;
 	u32 *dw_frame_interval;
 };
@@ -1185,6 +1202,7 @@ UVCG_FRAME_ATTR(dw_min_bit_rate, dwMinBitRate, 32);
 UVCG_FRAME_ATTR(dw_max_bit_rate, dwMaxBitRate, 32);
 UVCG_FRAME_ATTR(dw_max_video_frame_buffer_size, dwMaxVideoFrameBufferSize, 32);
 UVCG_FRAME_ATTR(dw_default_frame_interval, dwDefaultFrameInterval, 32);
+UVCG_FRAME_ATTR(dw_bytes_perline, dwBytesPerLine, 32);
 
 #undef UVCG_FRAME_ATTR
 
@@ -1329,6 +1347,7 @@ static struct configfs_attribute *uvcg_frame_attrs[] = {
 	&uvcg_frame_attr_dw_max_video_frame_buffer_size,
 	&uvcg_frame_attr_dw_default_frame_interval,
 	&uvcg_frame_attr_dw_frame_interval,
+	&uvcg_frame_attr_dw_bytes_perline,
 	NULL,
 };
 
@@ -1358,6 +1377,7 @@ static struct config_item *uvcg_frame_make(struct config_group *group,
 	h->frame.dw_max_bit_rate		= 55296000;
 	h->frame.dw_max_video_frame_buffer_size	= 460800;
 	h->frame.dw_default_frame_interval	= 666666;
+	h->frame.dw_bytes_perline		= 0;
 
 	opts_item = group->cg_item.ci_parent->ci_parent->ci_parent;
 	opts = to_f_uvc_opts(opts_item);
@@ -1365,7 +1385,10 @@ static struct config_item *uvcg_frame_make(struct config_group *group,
 	mutex_lock(&opts->lock);
 	fmt = to_uvcg_format(&group->cg_item);
 	if (fmt->type == UVCG_UNCOMPRESSED) {
-		h->frame.b_descriptor_subtype = UVC_VS_FRAME_UNCOMPRESSED;
+		if (uvcg_uncompressed_subtype(fmt) == UVC_VS_FORMAT_UNCOMPRESSED)
+			h->frame.b_descriptor_subtype = UVC_VS_FRAME_UNCOMPRESSED;
+		else
+			h->frame.b_descriptor_subtype = UVC_VS_FRAME_FRAME_BASED;
 		h->fmt_type = UVCG_UNCOMPRESSED;
 	} else if (fmt->type == UVCG_MJPEG) {
 		h->frame.b_descriptor_subtype = UVC_VS_FRAME_MJPEG;
@@ -1425,6 +1448,14 @@ struct uvcg_uncompressed {
 	struct uvc_format_uncompressed	desc;
 };
 
+static u8 uvcg_uncompressed_subtype(struct uvcg_format *fmt)
+{
+	struct uvcg_uncompressed *ch = container_of(fmt,
+					struct uvcg_uncompressed, fmt);
+
+	return ch->desc.bDescriptorSubType;
+}
+
 static struct uvcg_uncompressed *to_uvcg_uncompressed(struct config_item *item)
 {
 	return container_of(
@@ -1466,6 +1497,7 @@ static ssize_t uvcg_uncompressed_guid_format_store(struct config_item *item,
 	struct f_uvc_opts *opts;
 	struct config_item *opts_item;
 	struct mutex *su_mutex = &ch->fmt.group.cg_subsys->su_mutex;
+	u32 fcc;
 	int ret;
 
 	mutex_lock(su_mutex); /* for navigating configfs hierarchy */
@@ -1481,7 +1513,17 @@ static ssize_t uvcg_uncompressed_guid_format_store(struct config_item *item,
 
 	memcpy(ch->desc.guidFormat, page,
 	       min(sizeof(ch->desc.guidFormat), len));
-	ret = sizeof(ch->desc.guidFormat);
+	ret = len;
+
+	fcc = v4l2_fourcc(ch->desc.guidFormat[0], ch->desc.guidFormat[1],
+		ch->desc.guidFormat[2], ch->desc.guidFormat[3]);
+	if (fcc == V4L2_PIX_FMT_H264 || fcc == V4L2_PIX_FMT_HEVC) {
+		ch->desc.bLength		= UVC_DT_FORMAT_FRAMEBASED_SIZE;
+		ch->desc.bDescriptorSubType	= UVC_VS_FORMAT_FRAME_BASED;
+	} else {
+		ch->desc.bLength		= UVC_DT_FORMAT_UNCOMPRESSED_SIZE;
+		ch->desc.bDescriptorSubType	= UVC_VS_FORMAT_UNCOMPRESSED;
+	}
 
 end:
 	mutex_unlock(&opts->lock);
@@ -1581,6 +1623,7 @@ UVCG_UNCOMPRESSED_ATTR(b_default_frame_index, bDefaultFrameIndex, 8);
 UVCG_UNCOMPRESSED_ATTR_RO(b_aspect_ratio_x, bAspectRatioX, 8);
 UVCG_UNCOMPRESSED_ATTR_RO(b_aspect_ratio_y, bAspectRatioY, 8);
 UVCG_UNCOMPRESSED_ATTR_RO(bm_interface_flags, bmInterfaceFlags, 8);
+UVCG_UNCOMPRESSED_ATTR(b_variable_size, bVariableSize, 8);
 
 #undef UVCG_UNCOMPRESSED_ATTR
 #undef UVCG_UNCOMPRESSED_ATTR_RO
@@ -1611,6 +1654,7 @@ static struct configfs_attribute *uvcg_uncompressed_attrs[] = {
 	&uvcg_uncompressed_attr_b_aspect_ratio_y,
 	&uvcg_uncompressed_attr_bm_interface_flags,
 	&uvcg_uncompressed_attr_bma_controls,
+	&uvcg_uncompressed_attr_b_variable_size,
 	NULL,
 };
 
@@ -1644,6 +1688,7 @@ static struct config_group *uvcg_uncompressed_make(struct config_group *group,
 	h->desc.bAspectRatioY		= 0;
 	h->desc.bmInterfaceFlags	= 0;
 	h->desc.bCopyProtect		= 0;
+	h->desc.bVariableSize		= 0;
 
 	h->fmt.type = UVCG_UNCOMPRESSED;
 	config_group_init_type_name(&h->fmt.group, name,
@@ -2038,7 +2083,7 @@ static int __uvcg_cnt_strm(void *priv1, void *priv2, void *priv3, int n,
 				container_of(fmt, struct uvcg_uncompressed,
 					     fmt);
 
-			*size += sizeof(u->desc);
+			*size += u->desc.bLength;
 		} else if (fmt->type == UVCG_MJPEG) {
 			struct uvcg_mjpeg *m =
 				container_of(fmt, struct uvcg_mjpeg, fmt);
@@ -2053,7 +2098,7 @@ static int __uvcg_cnt_strm(void *priv1, void *priv2, void *priv3, int n,
 		struct uvcg_frame *frm = priv1;
 		int sz = sizeof(frm->dw_frame_interval);
 
-		*size += sizeof(frm->frame);
+		*size += sizeof(frm->frame) - 4;
 		*size += frm->frame.b_frame_interval_type * sz;
 	}
 	break;
@@ -2108,8 +2153,8 @@ static int __uvcg_fill_strm(void *priv1, void *priv2, void *priv3, int n,
 
 			u->desc.bFormatIndex = n + 1;
 			u->desc.bNumFrameDescriptors = fmt->num_frames;
-			memcpy(*dest, &u->desc, sizeof(u->desc));
-			*dest += sizeof(u->desc);
+			memcpy(*dest, &u->desc, u->desc.bLength);
+			*dest += u->desc.bLength;
 		} else if (fmt->type == UVCG_MJPEG) {
 			struct uvcg_mjpeg *m =
 				container_of(fmt, struct uvcg_mjpeg, fmt);
@@ -2127,8 +2172,16 @@ static int __uvcg_fill_strm(void *priv1, void *priv2, void *priv3, int n,
 		struct uvcg_frame *frm = priv1;
 		struct uvc_descriptor_header *h = *dest;
 
-		sz = sizeof(frm->frame);
+		sz = sizeof(frm->frame) - 4;
 		memcpy(*dest, &frm->frame, sz);
+		/*
+		 * Framebased frame doesn't have dw_max_video_frame_buffer_size,
+		 * and has dw_bytes_perline, so we should handle the last three
+		 * members of frame descriptor.
+		 */
+		if (frm->frame.b_descriptor_subtype == UVC_VS_FRAME_FRAME_BASED)
+			memcpy((u8 *)*dest + 17, (u8 *)&frm->frame + 21, 9);
+
 		*dest += sz;
 		sz = frm->frame.b_frame_interval_type *
 			sizeof(*frm->dw_frame_interval);
diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c
index a2c78690c5c2..3d6217328c50 100644
--- a/drivers/usb/gadget/function/uvc_v4l2.c
+++ b/drivers/usb/gadget/function/uvc_v4l2.c
@@ -58,6 +58,8 @@ struct uvc_format {
 static struct uvc_format uvc_formats[] = {
 	{ 16, V4L2_PIX_FMT_YUYV  },
 	{ 0,  V4L2_PIX_FMT_MJPEG },
+	{ 0,  V4L2_PIX_FMT_H264 },
+	{ 0,  V4L2_PIX_FMT_HEVC },
 };
 
 static int
diff --git a/include/uapi/linux/usb/video.h b/include/uapi/linux/usb/video.h
index bfdae12cdacf..383980bc9618 100644
--- a/include/uapi/linux/usb/video.h
+++ b/include/uapi/linux/usb/video.h
@@ -468,9 +468,12 @@ struct uvc_format_uncompressed {
 	__u8  bAspectRatioY;
 	__u8  bmInterfaceFlags;
 	__u8  bCopyProtect;
+	/* bVariableSize is only for framebased format. */
+	__u8  bVariableSize;
 } __attribute__((__packed__));
 
 #define UVC_DT_FORMAT_UNCOMPRESSED_SIZE			27
+#define UVC_DT_FORMAT_FRAMEBASED_SIZE			28
 
 /* Uncompressed Payload - 3.1.2. Uncompressed Video Frame Descriptor */
 struct uvc_frame_uncompressed {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] usb: gadget: uvc: add framebased stream support
  2022-02-17 10:44 ` [PATCH v2] " 3090101217
@ 2022-02-17 15:27   ` Greg KH
  2022-02-23  2:06     ` Jing Leng
  2022-05-12  9:43   ` [PATCH v3] " 3090101217
  1 sibling, 1 reply; 9+ messages in thread
From: Greg KH @ 2022-02-17 15:27 UTC (permalink / raw)
  To: 3090101217
  Cc: balbi, bilbao, corbet, laurent.pinchart, mchehab+huawei, pawell,
	rdunlap, linux-kernel, linux-usb, Jing Leng

On Thu, Feb 17, 2022 at 06:44:50PM +0800, 3090101217@zju.edu.cn wrote:
> From: Jing Leng <jleng@ambarella.com>
> 
> Currently the uvc gadget can't support H264/HEVC transport, After
> adding framebased stream support, the driver can support them.
> 
> Framebased stream is a little different from uncompressed stream.
> So we can support framebased stream on the basis of uncompressed stream.
> 
> Here are the differences:
> 
> 1. For the format, framebased format has an extra member (
> __u8 bVariableSize) than uncompressed format.
> 
> 2. For the frame, the layout of last three members of framebased frame
> is different from uncompressed frame.
> a. Last three members of uncompressed frame are:
>   u32	dw_max_video_frame_buffer_size;
>   u32	dw_default_frame_interval;
>   u8	b_frame_interval_type;
> b. Last three members of framebased frame are:
>   u32	dw_default_frame_interval;
>   u8	b_frame_interval_type;
>   u32	dw_bytes_perline;
> 
> Here is an example of configuring H264:
> 
> cd /sys/kernel/config/usb_gadget/g1
> ndir=functions/uvc.usb0/streaming/uncompressed/$NAME
> mkdir -p $ndir
> echo -n "H264" > $ndir/guidFormat  # H264 or HEVC
> echo 0 > $ndir/bBitsPerPixel
> echo 1 > $ndir/bVariableSize
> wdir=functions/uvc.usb0/streaming/uncompressed/$NAME/${HEIGHT}p
> mkdir -p $wdir
> echo 0 > $wdir/dwBytesPerLine
> echo $WIDTH  > $wdir/wWidth
> echo $HEIGHT > $wdir/wHeight
> echo 29491200 > $wdir/dwMinBitRate
> echo 29491200 > $wdir/dwMaxBitRate
> cat <<EOF > $wdir/dwFrameInterval
> $INTERVAL
> EOF
> 
> Signed-off-by: Jing Leng <jleng@ambarella.com>
> ---
> ChangeLog v1->v2:
> - Use another way to handle frames, previous implementation within
> - using union has a warning. (Reported-by: kernel test robot <lkp@intel.com>)
> ---
>  .../ABI/testing/configfs-usb-gadget-uvc       | 13 +++-
>  drivers/usb/gadget/function/uvc_configfs.c    | 67 +++++++++++++++++--
>  drivers/usb/gadget/function/uvc_v4l2.c        |  2 +
>  include/uapi/linux/usb/video.h                |  3 +
>  4 files changed, 75 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
> index 889ed45be4ca..2bf515dad516 100644
> --- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
> +++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
> @@ -243,7 +243,7 @@ Description:	Uncompressed format descriptors
>  What:		/config/usb-gadget/gadget/functions/uvc.name/streaming/uncompressed/name
>  Date:		Dec 2014
>  KernelVersion:	4.0
> -Description:	Specific uncompressed format descriptors
> +Description:	Specific uncompressed/framebased format descriptors
>  
>  		==================	=======================================
>  		bFormatIndex		unique id for this format descriptor;
> @@ -264,12 +264,15 @@ Description:	Specific uncompressed format descriptors
>  					frame
>  		guidFormat		globally unique id used to identify
>  					stream-encoding format
> +		bVariableSize		whether the data within the frame is of
> +					variable length from frame to frame (
> +					only for framebased format)
>  		==================	=======================================
>  
>  What:		/config/usb-gadget/gadget/functions/uvc.name/streaming/uncompressed/name/name
>  Date:		Dec 2014
>  KernelVersion:	4.0
> -Description:	Specific uncompressed frame descriptors
> +Description:	Specific uncompressed/framebased frame descriptors
>  
>  		=========================  =====================================
>  		bFrameIndex		   unique id for this framedescriptor;
> @@ -283,7 +286,11 @@ Description:	Specific uncompressed frame descriptors
>  					   like to use as default
>  		dwMaxVideoFrameBufferSize  the maximum number of bytes the
>  					   compressor will produce for a video
> -					   frame or still image
> +					   frame or still image (only for
> +					   uncompressed frame)
> +		dwBytesPerLine		   the per-line bytes of the framebased
> +					   frame, e.g. H264 or HEVC (only for
> +					   framebased frame)
>  		dwMaxBitRate		   the maximum bit rate at the shortest
>  					   frame interval in bps
>  		dwMinBitRate		   the minimum bit rate at the longest
> diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
> index 77d64031aa9c..695f9b68290f 100644
> --- a/drivers/usb/gadget/function/uvc_configfs.c
> +++ b/drivers/usb/gadget/function/uvc_configfs.c
> @@ -11,6 +11,7 @@
>   */
>  
>  #include <linux/sort.h>
> +#include <linux/videodev2.h>
>  
>  #include "u_uvc.h"
>  #include "uvc_configfs.h"
> @@ -782,6 +783,8 @@ struct uvcg_format {
>  	__u8			bmaControls[UVCG_STREAMING_CONTROL_SIZE];
>  };
>  
> +static u8 uvcg_uncompressed_subtype(struct uvcg_format *fmt);
> +
>  static struct uvcg_format *to_uvcg_format(struct config_item *item)
>  {
>  	return container_of(to_config_group(item), struct uvcg_format, group);
> @@ -1072,9 +1075,23 @@ struct uvcg_frame {
>  		u16	w_height;
>  		u32	dw_min_bit_rate;
>  		u32	dw_max_bit_rate;
> +
> +		/*
> +		 * The layout of last three members of framebased frame
> +		 * is different from uncompressed frame.
> +		 *   Last three members of uncompressed frame are:
> +		 *     u32	dw_max_video_frame_buffer_size;
> +		 *     u32	dw_default_frame_interval;
> +		 *     u8	b_frame_interval_type;
> +		 *   Last three members of framebased frame are:
> +		 *     u32	dw_default_frame_interval;
> +		 *     u8	b_frame_interval_type;
> +		 *     u32	dw_bytes_perline;
> +		 */
>  		u32	dw_max_video_frame_buffer_size;
>  		u32	dw_default_frame_interval;
>  		u8	b_frame_interval_type;
> +		u32	dw_bytes_perline;

Why not use a union here as this is coming from the hardware, right?


>  	} __attribute__((packed)) frame;
>  	u32 *dw_frame_interval;
>  };
> @@ -1185,6 +1202,7 @@ UVCG_FRAME_ATTR(dw_min_bit_rate, dwMinBitRate, 32);
>  UVCG_FRAME_ATTR(dw_max_bit_rate, dwMaxBitRate, 32);
>  UVCG_FRAME_ATTR(dw_max_video_frame_buffer_size, dwMaxVideoFrameBufferSize, 32);
>  UVCG_FRAME_ATTR(dw_default_frame_interval, dwDefaultFrameInterval, 32);
> +UVCG_FRAME_ATTR(dw_bytes_perline, dwBytesPerLine, 32);
>  
>  #undef UVCG_FRAME_ATTR
>  
> @@ -1329,6 +1347,7 @@ static struct configfs_attribute *uvcg_frame_attrs[] = {
>  	&uvcg_frame_attr_dw_max_video_frame_buffer_size,
>  	&uvcg_frame_attr_dw_default_frame_interval,
>  	&uvcg_frame_attr_dw_frame_interval,
> +	&uvcg_frame_attr_dw_bytes_perline,
>  	NULL,
>  };
>  
> @@ -1358,6 +1377,7 @@ static struct config_item *uvcg_frame_make(struct config_group *group,
>  	h->frame.dw_max_bit_rate		= 55296000;
>  	h->frame.dw_max_video_frame_buffer_size	= 460800;
>  	h->frame.dw_default_frame_interval	= 666666;
> +	h->frame.dw_bytes_perline		= 0;
>  
>  	opts_item = group->cg_item.ci_parent->ci_parent->ci_parent;
>  	opts = to_f_uvc_opts(opts_item);
> @@ -1365,7 +1385,10 @@ static struct config_item *uvcg_frame_make(struct config_group *group,
>  	mutex_lock(&opts->lock);
>  	fmt = to_uvcg_format(&group->cg_item);
>  	if (fmt->type == UVCG_UNCOMPRESSED) {
> -		h->frame.b_descriptor_subtype = UVC_VS_FRAME_UNCOMPRESSED;
> +		if (uvcg_uncompressed_subtype(fmt) == UVC_VS_FORMAT_UNCOMPRESSED)
> +			h->frame.b_descriptor_subtype = UVC_VS_FRAME_UNCOMPRESSED;
> +		else
> +			h->frame.b_descriptor_subtype = UVC_VS_FRAME_FRAME_BASED;
>  		h->fmt_type = UVCG_UNCOMPRESSED;
>  	} else if (fmt->type == UVCG_MJPEG) {
>  		h->frame.b_descriptor_subtype = UVC_VS_FRAME_MJPEG;
> @@ -1425,6 +1448,14 @@ struct uvcg_uncompressed {
>  	struct uvc_format_uncompressed	desc;
>  };
>  
> +static u8 uvcg_uncompressed_subtype(struct uvcg_format *fmt)
> +{
> +	struct uvcg_uncompressed *ch = container_of(fmt,
> +					struct uvcg_uncompressed, fmt);
> +
> +	return ch->desc.bDescriptorSubType;
> +}
> +
>  static struct uvcg_uncompressed *to_uvcg_uncompressed(struct config_item *item)
>  {
>  	return container_of(
> @@ -1466,6 +1497,7 @@ static ssize_t uvcg_uncompressed_guid_format_store(struct config_item *item,
>  	struct f_uvc_opts *opts;
>  	struct config_item *opts_item;
>  	struct mutex *su_mutex = &ch->fmt.group.cg_subsys->su_mutex;
> +	u32 fcc;
>  	int ret;
>  
>  	mutex_lock(su_mutex); /* for navigating configfs hierarchy */
> @@ -1481,7 +1513,17 @@ static ssize_t uvcg_uncompressed_guid_format_store(struct config_item *item,
>  
>  	memcpy(ch->desc.guidFormat, page,
>  	       min(sizeof(ch->desc.guidFormat), len));
> -	ret = sizeof(ch->desc.guidFormat);
> +	ret = len;
> +
> +	fcc = v4l2_fourcc(ch->desc.guidFormat[0], ch->desc.guidFormat[1],
> +		ch->desc.guidFormat[2], ch->desc.guidFormat[3]);
> +	if (fcc == V4L2_PIX_FMT_H264 || fcc == V4L2_PIX_FMT_HEVC) {
> +		ch->desc.bLength		= UVC_DT_FORMAT_FRAMEBASED_SIZE;
> +		ch->desc.bDescriptorSubType	= UVC_VS_FORMAT_FRAME_BASED;
> +	} else {
> +		ch->desc.bLength		= UVC_DT_FORMAT_UNCOMPRESSED_SIZE;
> +		ch->desc.bDescriptorSubType	= UVC_VS_FORMAT_UNCOMPRESSED;
> +	}
>  
>  end:
>  	mutex_unlock(&opts->lock);
> @@ -1581,6 +1623,7 @@ UVCG_UNCOMPRESSED_ATTR(b_default_frame_index, bDefaultFrameIndex, 8);
>  UVCG_UNCOMPRESSED_ATTR_RO(b_aspect_ratio_x, bAspectRatioX, 8);
>  UVCG_UNCOMPRESSED_ATTR_RO(b_aspect_ratio_y, bAspectRatioY, 8);
>  UVCG_UNCOMPRESSED_ATTR_RO(bm_interface_flags, bmInterfaceFlags, 8);
> +UVCG_UNCOMPRESSED_ATTR(b_variable_size, bVariableSize, 8);

Why is this writable, but the other variables are not?

>  
>  #undef UVCG_UNCOMPRESSED_ATTR
>  #undef UVCG_UNCOMPRESSED_ATTR_RO
> @@ -1611,6 +1654,7 @@ static struct configfs_attribute *uvcg_uncompressed_attrs[] = {
>  	&uvcg_uncompressed_attr_b_aspect_ratio_y,
>  	&uvcg_uncompressed_attr_bm_interface_flags,
>  	&uvcg_uncompressed_attr_bma_controls,
> +	&uvcg_uncompressed_attr_b_variable_size,
>  	NULL,
>  };
>  
> @@ -1644,6 +1688,7 @@ static struct config_group *uvcg_uncompressed_make(struct config_group *group,
>  	h->desc.bAspectRatioY		= 0;
>  	h->desc.bmInterfaceFlags	= 0;
>  	h->desc.bCopyProtect		= 0;
> +	h->desc.bVariableSize		= 0;
>  
>  	h->fmt.type = UVCG_UNCOMPRESSED;
>  	config_group_init_type_name(&h->fmt.group, name,
> @@ -2038,7 +2083,7 @@ static int __uvcg_cnt_strm(void *priv1, void *priv2, void *priv3, int n,
>  				container_of(fmt, struct uvcg_uncompressed,
>  					     fmt);
>  
> -			*size += sizeof(u->desc);
> +			*size += u->desc.bLength;
>  		} else if (fmt->type == UVCG_MJPEG) {
>  			struct uvcg_mjpeg *m =
>  				container_of(fmt, struct uvcg_mjpeg, fmt);
> @@ -2053,7 +2098,7 @@ static int __uvcg_cnt_strm(void *priv1, void *priv2, void *priv3, int n,
>  		struct uvcg_frame *frm = priv1;
>  		int sz = sizeof(frm->dw_frame_interval);
>  
> -		*size += sizeof(frm->frame);
> +		*size += sizeof(frm->frame) - 4;

Where did "4" come from?

>  		*size += frm->frame.b_frame_interval_type * sz;
>  	}
>  	break;
> @@ -2108,8 +2153,8 @@ static int __uvcg_fill_strm(void *priv1, void *priv2, void *priv3, int n,
>  
>  			u->desc.bFormatIndex = n + 1;
>  			u->desc.bNumFrameDescriptors = fmt->num_frames;
> -			memcpy(*dest, &u->desc, sizeof(u->desc));
> -			*dest += sizeof(u->desc);
> +			memcpy(*dest, &u->desc, u->desc.bLength);
> +			*dest += u->desc.bLength;
>  		} else if (fmt->type == UVCG_MJPEG) {
>  			struct uvcg_mjpeg *m =
>  				container_of(fmt, struct uvcg_mjpeg, fmt);
> @@ -2127,8 +2172,16 @@ static int __uvcg_fill_strm(void *priv1, void *priv2, void *priv3, int n,
>  		struct uvcg_frame *frm = priv1;
>  		struct uvc_descriptor_header *h = *dest;
>  
> -		sz = sizeof(frm->frame);
> +		sz = sizeof(frm->frame) - 4;
>  		memcpy(*dest, &frm->frame, sz);
> +		/*
> +		 * Framebased frame doesn't have dw_max_video_frame_buffer_size,
> +		 * and has dw_bytes_perline, so we should handle the last three
> +		 * members of frame descriptor.
> +		 */
> +		if (frm->frame.b_descriptor_subtype == UVC_VS_FRAME_FRAME_BASED)
> +			memcpy((u8 *)*dest + 17, (u8 *)&frm->frame + 21, 9);
> +
>  		*dest += sz;
>  		sz = frm->frame.b_frame_interval_type *
>  			sizeof(*frm->dw_frame_interval);
> diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c
> index a2c78690c5c2..3d6217328c50 100644
> --- a/drivers/usb/gadget/function/uvc_v4l2.c
> +++ b/drivers/usb/gadget/function/uvc_v4l2.c
> @@ -58,6 +58,8 @@ struct uvc_format {
>  static struct uvc_format uvc_formats[] = {
>  	{ 16, V4L2_PIX_FMT_YUYV  },
>  	{ 0,  V4L2_PIX_FMT_MJPEG },
> +	{ 0,  V4L2_PIX_FMT_H264 },
> +	{ 0,  V4L2_PIX_FMT_HEVC },
>  };
>  
>  static int
> diff --git a/include/uapi/linux/usb/video.h b/include/uapi/linux/usb/video.h
> index bfdae12cdacf..383980bc9618 100644
> --- a/include/uapi/linux/usb/video.h
> +++ b/include/uapi/linux/usb/video.h
> @@ -468,9 +468,12 @@ struct uvc_format_uncompressed {
>  	__u8  bAspectRatioY;
>  	__u8  bmInterfaceFlags;
>  	__u8  bCopyProtect;
> +	/* bVariableSize is only for framebased format. */
> +	__u8  bVariableSize;

This just changed a user visable structure size.  What broke when doing
this?  What tool uses this?

>  } __attribute__((__packed__));
>  
>  #define UVC_DT_FORMAT_UNCOMPRESSED_SIZE			27
> +#define UVC_DT_FORMAT_FRAMEBASED_SIZE			28
>  
>  /* Uncompressed Payload - 3.1.2. Uncompressed Video Frame Descriptor */
>  struct uvc_frame_uncompressed {
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Re: [PATCH] usb: gadget: uvc: add framebased stream support
  2022-02-16 18:22   ` kernel test robot
@ 2022-02-18 11:08     ` Jing Leng
  -1 siblings, 0 replies; 9+ messages in thread
From: Jing Leng @ 2022-02-18 11:08 UTC (permalink / raw)
  To: kernel test robot
  Cc: laurent.pinchart, balbi, gregkh, corbet, mchehab+huawei, rdunlap,
	bilbao, pawell, llvm, kbuild-all, linux-kernel, linux-usb,
	Jing Leng

Hi Greg KH,

> Why not use a union here as this is coming from the hardware, right?
>

I used union in PATCH v1, I compiled it to arm64 binary with GCC 11.2.1, 
the binary works properly.
But "kernel test robot <lkp@intel.com>" reported a warnings:
 >> drivers/usb/gadget/function/uvc_configfs.c:1091:3: warning: 
 field  within 'struct uvcg_frame::(unnamed at drivers/usb/gadget/function/uvc_configfs.c:1068:2)' 
 is less aligned than 'union uvcg_frame::(anonymous at drivers/usb/gadget/function/uvc_configfs.c:1091:3)' 
 and is usually due to 'struct uvcg_frame::(unnamed at drivers/usb/gadget/function/uvc_configfs.c:1068:2)' 
 being packed, which can lead to unaligned accesses [-Wunaligned-access]
                   union {
                   ^
   1 warning generated.
So I use another way to handle the frame structure.

> Why is this writable, but the other variables are not?
> 

1. bFormatIndex is automatic auto calculated by the driver.
   So it is read-only.
2. I don't know why "b_aspect_ratio_x / b_aspect_ratio_y / bm_interface_flags"
   are read-only, Perhaps these parameters can be obtained directly from actual stream.
   so driver does not need to care about these parameters.
3. If bVariableSize is 1, then dwBytesPerLine must be set to zero (0).
   If bVariableSize is 0, then dwBytesPerLine can be setted to other value.
   So it is writable.

> > -		*size += sizeof(frm->frame);
> > +		*size += sizeof(frm->frame) - 4;
> 
> Where did "4" come from?
>

Uncompressed frame doesn't have "u32 dw_bytes_perline".
Framebased frame doesn't have "u32 dw_max_video_frame_buffer_size".
If we use union, there's no need to do this.
Maybe we can add "#define UVCG_SUB_FRAME_PAYLOAD_LENGTH 26", and use
"UVCG_SUB_FRAME_PAYLOAD_LENGTH" to replace "sizeof(frm->frame) - 4".

> > +	/* bVariableSize is only for framebased format. */
> > +	__u8  bVariableSize;
> 
> This just changed a user visable structure size.  What broke when doing
> this?  What tool uses this?
> 

As long as users use "UVC_DT_FORMAT_UNCOMPRESSED_SIZE" instead of
"sizeof(struct uvc_format_uncompressed)" to get the length, there is
no problem. So I have the following modifications:
    -			*size += sizeof(u->desc);
    +			*size += u->desc.bLength;

Currently this change does not break the kernel, and uvc stream APP
based on UVC gadget doesn't need to use "struct uvc_format_uncompressed".

There may be some tools that use it, They can use "UVC_DT_FORMAT_UNCOMPRESSED_SIZE"
to cover the modification.

We don't need "copy all uncompressed code, rename
uncompressed as framebased, make a little change" to access framebased
stream support.

Thanks,
Jing Leng

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] usb: gadget: uvc: add framebased stream support
@ 2022-02-18 11:08     ` Jing Leng
  0 siblings, 0 replies; 9+ messages in thread
From: Jing Leng @ 2022-02-18 11:08 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2711 bytes --]

Hi Greg KH,

> Why not use a union here as this is coming from the hardware, right?
>

I used union in PATCH v1, I compiled it to arm64 binary with GCC 11.2.1, 
the binary works properly.
But "kernel test robot <lkp@intel.com>" reported a warnings:
 >> drivers/usb/gadget/function/uvc_configfs.c:1091:3: warning: 
 field  within 'struct uvcg_frame::(unnamed at drivers/usb/gadget/function/uvc_configfs.c:1068:2)' 
 is less aligned than 'union uvcg_frame::(anonymous at drivers/usb/gadget/function/uvc_configfs.c:1091:3)' 
 and is usually due to 'struct uvcg_frame::(unnamed at drivers/usb/gadget/function/uvc_configfs.c:1068:2)' 
 being packed, which can lead to unaligned accesses [-Wunaligned-access]
                   union {
                   ^
   1 warning generated.
So I use another way to handle the frame structure.

> Why is this writable, but the other variables are not?
> 

1. bFormatIndex is automatic auto calculated by the driver.
   So it is read-only.
2. I don't know why "b_aspect_ratio_x / b_aspect_ratio_y / bm_interface_flags"
   are read-only, Perhaps these parameters can be obtained directly from actual stream.
   so driver does not need to care about these parameters.
3. If bVariableSize is 1, then dwBytesPerLine must be set to zero (0).
   If bVariableSize is 0, then dwBytesPerLine can be setted to other value.
   So it is writable.

> > -		*size += sizeof(frm->frame);
> > +		*size += sizeof(frm->frame) - 4;
> 
> Where did "4" come from?
>

Uncompressed frame doesn't have "u32 dw_bytes_perline".
Framebased frame doesn't have "u32 dw_max_video_frame_buffer_size".
If we use union, there's no need to do this.
Maybe we can add "#define UVCG_SUB_FRAME_PAYLOAD_LENGTH 26", and use
"UVCG_SUB_FRAME_PAYLOAD_LENGTH" to replace "sizeof(frm->frame) - 4".

> > +	/* bVariableSize is only for framebased format. */
> > +	__u8  bVariableSize;
> 
> This just changed a user visable structure size.  What broke when doing
> this?  What tool uses this?
> 

As long as users use "UVC_DT_FORMAT_UNCOMPRESSED_SIZE" instead of
"sizeof(struct uvc_format_uncompressed)" to get the length, there is
no problem. So I have the following modifications:
    -			*size += sizeof(u->desc);
    +			*size += u->desc.bLength;

Currently this change does not break the kernel, and uvc stream APP
based on UVC gadget doesn't need to use "struct uvc_format_uncompressed".

There may be some tools that use it, They can use "UVC_DT_FORMAT_UNCOMPRESSED_SIZE"
to cover the modification.

We don't need "copy all uncompressed code, rename
uncompressed as framebased, make a little change" to access framebased
stream support.

Thanks,
Jing Leng

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Re: [PATCH v2] usb: gadget: uvc: add framebased stream support
  2022-02-17 15:27   ` Greg KH
@ 2022-02-23  2:06     ` Jing Leng
  0 siblings, 0 replies; 9+ messages in thread
From: Jing Leng @ 2022-02-23  2:06 UTC (permalink / raw)
  To: Greg KH
  Cc: balbi, bilbao, corbet, laurent.pinchart, mchehab+huawei, pawell,
	rdunlap, linux-kernel, linux-usb, Jing Leng

Hi Greg KH,

> Why not use a union here as this is coming from the hardware, right?
>

I used a union in PATCH v1
(https://patchwork.kernel.org/project/linux-usb/patch/20220216081651.9089-1-3090101217@zju.edu.cn/),
I compiled it to arm64 binary with GCC 11.2.1, the binary works properly.
But "kernel test robot <lkp@intel.com>" reported a warnings:
 >> drivers/usb/gadget/function/uvc_configfs.c:1091:3: warning: 
 field  within 'struct uvcg_frame::(unnamed at drivers/usb/gadget/function/uvc_configfs.c:1068:2)' 
 is less aligned than 'union uvcg_frame::(anonymous at drivers/usb/gadget/function/uvc_configfs.c:1091:3)' 
 and is usually due to 'struct uvcg_frame::(unnamed at drivers/usb/gadget/function/uvc_configfs.c:1068:2)' 
 being packed, which can lead to unaligned accesses [-Wunaligned-access]
                   union {
                   ^
   1 warning generated.
So I use another way to handle the frame structure.

> Why is this writable, but the other variables are not?
> 

1. bFormatIndex is automatic auto calculated by the driver.
   So it is read-only.
2. I don't know why "b_aspect_ratio_x / b_aspect_ratio_y / bm_interface_flags"
   are read-only. Perhaps these parameters can be obtained directly from 
   the actual stream, so driver doesn't need to take care of these parameters.
3. If bVariableSize is 1, then dwBytesPerLine must be setted to zero (0).
   If bVariableSize is 0, then dwBytesPerLine can be setted to other value.
   So it is writable.

> > -		*size += sizeof(frm->frame);
> > +		*size += sizeof(frm->frame) - 4;
> 
> Where did "4" come from?
>

Uncompressed frame doesn't have "u32 dw_bytes_perline".
Framebased frame doesn't have "u32 dw_max_video_frame_buffer_size".
If we use a union like PATCH v1, there's no need to do this.
Maybe we can add "#define UVCG_SUB_FRAME_PAYLOAD_LENGTH 26", and use
"UVCG_SUB_FRAME_PAYLOAD_LENGTH" to replace "sizeof(frm->frame) - 4"
for the new PATCH.

> > +	/* bVariableSize is only for framebased format. */
> > +	__u8  bVariableSize;
> 
> This just changed a user visable structure size.  What broke when doing
> this?  What tool uses this?
> 

As long as users use "UVC_DT_FORMAT_UNCOMPRESSED_SIZE" instead of
"sizeof(struct uvc_format_uncompressed)" to get the length, there is
no problem. So I have the following modifications:
    -			*size += sizeof(u->desc);
    +			*size += u->desc.bLength;

Currently this change does not break the kernel, and uvc stream APP
based on UVC gadget doesn't need to use "struct uvc_format_uncompressed".

There may be some tools which use it, they can use 
"UVC_DT_FORMAT_UNCOMPRESSED_SIZE" to cover the modification.
In addition, we don't need "copy all uncompressed code, rename
uncompressed as framebased, and make a little change" to access framebased
stream support.

Thanks,
Jing Leng

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3] usb: gadget: uvc: add framebased stream support
  2022-02-17 10:44 ` [PATCH v2] " 3090101217
  2022-02-17 15:27   ` Greg KH
@ 2022-05-12  9:43   ` 3090101217
  1 sibling, 0 replies; 9+ messages in thread
From: 3090101217 @ 2022-05-12  9:43 UTC (permalink / raw)
  To: gregkh, balbi, bilbao, corbet, laurent.pinchart, mchehab+huawei,
	pawell, rdunlap
  Cc: linux-kernel, linux-usb, Jing Leng

From: Jing Leng <jleng@ambarella.com>

Currently the uvc gadget can't support H264/HEVC transport, After
adding framebased stream support, the driver can support them.

Framebased stream is a little different from uncompressed stream.
So we can support framebased stream on the basis of uncompressed stream.

Here are the differences:

1. For the format, framebased format has an extra member (
__u8 bVariableSize) than uncompressed format.

2. For the frame, the layout of last three members of framebased frame
is different from uncompressed frame.
a. Last three members of uncompressed frame are:
  u32	dw_max_video_frame_buffer_size;
  u32	dw_default_frame_interval;
  u8	b_frame_interval_type;
b. Last three members of framebased frame are:
  u32	dw_default_frame_interval;
  u8	b_frame_interval_type;
  u32	dw_bytes_perline;

Here is an example of configuring H264:

cd /sys/kernel/config/usb_gadget/g1
ndir=functions/uvc.usb0/streaming/uncompressed/$NAME
mkdir -p $ndir
echo -n "H264" > $ndir/guidFormat  # H264 or HEVC
echo 12 > $ndir/bBitsPerPixel
echo 1 > $ndir/bVariableSize
wdir=functions/uvc.usb0/streaming/uncompressed/$NAME/${HEIGHT}p
mkdir -p $wdir
echo 0 > $wdir/dwBytesPerLine
echo $WIDTH  > $wdir/wWidth
echo $HEIGHT > $wdir/wHeight
echo 29491200 > $wdir/dwMinBitRate
echo 29491200 > $wdir/dwMaxBitRate
cat <<EOF > $wdir/dwFrameInterval
$INTERVAL
EOF

Signed-off-by: Jing Leng <jleng@ambarella.com>
---
ChangeLog v2->v3:
- Revert modification in v2, use '#define' to handle frames.
- Update the example, the value of bBitsPerPixel is 12 for YUV420 encoded.
ChangeLog v1->v2:
- Use another way to handle frames, previous implementation within
- using union has a warning. (Reported-by: kernel test robot <lkp@intel.com>)
---
 .../ABI/testing/configfs-usb-gadget-uvc       | 13 ++-
 drivers/usb/gadget/function/uvc_configfs.c    | 79 +++++++++++++++++--
 drivers/usb/gadget/function/uvc_v4l2.c        |  2 +
 include/uapi/linux/usb/video.h                |  3 +
 4 files changed, 89 insertions(+), 8 deletions(-)

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
index 889ed45be4ca..2bf515dad516 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
@@ -243,7 +243,7 @@ Description:	Uncompressed format descriptors
 What:		/config/usb-gadget/gadget/functions/uvc.name/streaming/uncompressed/name
 Date:		Dec 2014
 KernelVersion:	4.0
-Description:	Specific uncompressed format descriptors
+Description:	Specific uncompressed/framebased format descriptors
 
 		==================	=======================================
 		bFormatIndex		unique id for this format descriptor;
@@ -264,12 +264,15 @@ Description:	Specific uncompressed format descriptors
 					frame
 		guidFormat		globally unique id used to identify
 					stream-encoding format
+		bVariableSize		whether the data within the frame is of
+					variable length from frame to frame (
+					only for framebased format)
 		==================	=======================================
 
 What:		/config/usb-gadget/gadget/functions/uvc.name/streaming/uncompressed/name/name
 Date:		Dec 2014
 KernelVersion:	4.0
-Description:	Specific uncompressed frame descriptors
+Description:	Specific uncompressed/framebased frame descriptors
 
 		=========================  =====================================
 		bFrameIndex		   unique id for this framedescriptor;
@@ -283,7 +286,11 @@ Description:	Specific uncompressed frame descriptors
 					   like to use as default
 		dwMaxVideoFrameBufferSize  the maximum number of bytes the
 					   compressor will produce for a video
-					   frame or still image
+					   frame or still image (only for
+					   uncompressed frame)
+		dwBytesPerLine		   the per-line bytes of the framebased
+					   frame, e.g. H264 or HEVC (only for
+					   framebased frame)
 		dwMaxBitRate		   the maximum bit rate at the shortest
 					   frame interval in bps
 		dwMinBitRate		   the minimum bit rate at the longest
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 77d64031aa9c..5dda825408ca 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -11,6 +11,7 @@
  */
 
 #include <linux/sort.h>
+#include <linux/videodev2.h>
 
 #include "u_uvc.h"
 #include "uvc_configfs.h"
@@ -782,6 +783,8 @@ struct uvcg_format {
 	__u8			bmaControls[UVCG_STREAMING_CONTROL_SIZE];
 };
 
+static u8 uvcg_uncompressed_subtype(struct uvcg_format *fmt);
+
 static struct uvcg_format *to_uvcg_format(struct config_item *item)
 {
 	return container_of(to_config_group(item), struct uvcg_format, group);
@@ -1072,9 +1075,23 @@ struct uvcg_frame {
 		u16	w_height;
 		u32	dw_min_bit_rate;
 		u32	dw_max_bit_rate;
+
+		/*
+		 * The layout of last three members of framebased frame
+		 * is different from uncompressed frame.
+		 *   Last three members of uncompressed frame are:
+		 *     u32	dw_max_video_frame_buffer_size;
+		 *     u32	dw_default_frame_interval;
+		 *     u8	b_frame_interval_type;
+		 *   Last three members of framebased frame are:
+		 *     u32	dw_default_frame_interval;
+		 *     u8	b_frame_interval_type;
+		 *     u32	dw_bytes_perline;
+		 */
 		u32	dw_max_video_frame_buffer_size;
 		u32	dw_default_frame_interval;
 		u8	b_frame_interval_type;
+#define dw_bytes_perline	dw_max_video_frame_buffer_size
 	} __attribute__((packed)) frame;
 	u32 *dw_frame_interval;
 };
@@ -1186,6 +1203,18 @@ UVCG_FRAME_ATTR(dw_max_bit_rate, dwMaxBitRate, 32);
 UVCG_FRAME_ATTR(dw_max_video_frame_buffer_size, dwMaxVideoFrameBufferSize, 32);
 UVCG_FRAME_ATTR(dw_default_frame_interval, dwDefaultFrameInterval, 32);
 
+/*
+ * Set the alias of "dwMaxVideoFrameBufferSize" in uncompressed frame
+ * to "dwBytesPerLine" in framebased frame.
+ */
+static struct configfs_attribute uvcg_frame_attr_dw_bytes_perline = {
+	.ca_name  = "dwBytesPerLine",
+	.ca_mode  = 0666,
+	.ca_owner = THIS_MODULE,
+	.show     = uvcg_frame_dw_max_video_frame_buffer_size_show,
+	.store    = uvcg_frame_dw_max_video_frame_buffer_size_store,
+};
+
 #undef UVCG_FRAME_ATTR
 
 static ssize_t uvcg_frame_dw_frame_interval_show(struct config_item *item,
@@ -1329,6 +1358,7 @@ static struct configfs_attribute *uvcg_frame_attrs[] = {
 	&uvcg_frame_attr_dw_max_video_frame_buffer_size,
 	&uvcg_frame_attr_dw_default_frame_interval,
 	&uvcg_frame_attr_dw_frame_interval,
+	&uvcg_frame_attr_dw_bytes_perline,
 	NULL,
 };
 
@@ -1365,7 +1395,12 @@ static struct config_item *uvcg_frame_make(struct config_group *group,
 	mutex_lock(&opts->lock);
 	fmt = to_uvcg_format(&group->cg_item);
 	if (fmt->type == UVCG_UNCOMPRESSED) {
-		h->frame.b_descriptor_subtype = UVC_VS_FRAME_UNCOMPRESSED;
+		if (uvcg_uncompressed_subtype(fmt) == UVC_VS_FORMAT_UNCOMPRESSED) {
+			h->frame.b_descriptor_subtype = UVC_VS_FRAME_UNCOMPRESSED;
+		} else {
+			h->frame.b_descriptor_subtype = UVC_VS_FRAME_FRAME_BASED;
+			h->frame.dw_bytes_perline = 0;
+		}
 		h->fmt_type = UVCG_UNCOMPRESSED;
 	} else if (fmt->type == UVCG_MJPEG) {
 		h->frame.b_descriptor_subtype = UVC_VS_FRAME_MJPEG;
@@ -1425,6 +1460,14 @@ struct uvcg_uncompressed {
 	struct uvc_format_uncompressed	desc;
 };
 
+static u8 uvcg_uncompressed_subtype(struct uvcg_format *fmt)
+{
+	struct uvcg_uncompressed *ch = container_of(fmt,
+					struct uvcg_uncompressed, fmt);
+
+	return ch->desc.bDescriptorSubType;
+}
+
 static struct uvcg_uncompressed *to_uvcg_uncompressed(struct config_item *item)
 {
 	return container_of(
@@ -1466,6 +1509,7 @@ static ssize_t uvcg_uncompressed_guid_format_store(struct config_item *item,
 	struct f_uvc_opts *opts;
 	struct config_item *opts_item;
 	struct mutex *su_mutex = &ch->fmt.group.cg_subsys->su_mutex;
+	u32 fcc;
 	int ret;
 
 	mutex_lock(su_mutex); /* for navigating configfs hierarchy */
@@ -1481,7 +1525,17 @@ static ssize_t uvcg_uncompressed_guid_format_store(struct config_item *item,
 
 	memcpy(ch->desc.guidFormat, page,
 	       min(sizeof(ch->desc.guidFormat), len));
-	ret = sizeof(ch->desc.guidFormat);
+	ret = len;
+
+	fcc = v4l2_fourcc(ch->desc.guidFormat[0], ch->desc.guidFormat[1],
+		ch->desc.guidFormat[2], ch->desc.guidFormat[3]);
+	if (fcc == V4L2_PIX_FMT_H264 || fcc == V4L2_PIX_FMT_HEVC) {
+		ch->desc.bLength		= UVC_DT_FORMAT_FRAMEBASED_SIZE;
+		ch->desc.bDescriptorSubType	= UVC_VS_FORMAT_FRAME_BASED;
+	} else {
+		ch->desc.bLength		= UVC_DT_FORMAT_UNCOMPRESSED_SIZE;
+		ch->desc.bDescriptorSubType	= UVC_VS_FORMAT_UNCOMPRESSED;
+	}
 
 end:
 	mutex_unlock(&opts->lock);
@@ -1581,6 +1635,7 @@ UVCG_UNCOMPRESSED_ATTR(b_default_frame_index, bDefaultFrameIndex, 8);
 UVCG_UNCOMPRESSED_ATTR_RO(b_aspect_ratio_x, bAspectRatioX, 8);
 UVCG_UNCOMPRESSED_ATTR_RO(b_aspect_ratio_y, bAspectRatioY, 8);
 UVCG_UNCOMPRESSED_ATTR_RO(bm_interface_flags, bmInterfaceFlags, 8);
+UVCG_UNCOMPRESSED_ATTR(b_variable_size, bVariableSize, 8);
 
 #undef UVCG_UNCOMPRESSED_ATTR
 #undef UVCG_UNCOMPRESSED_ATTR_RO
@@ -1611,6 +1666,7 @@ static struct configfs_attribute *uvcg_uncompressed_attrs[] = {
 	&uvcg_uncompressed_attr_b_aspect_ratio_y,
 	&uvcg_uncompressed_attr_bm_interface_flags,
 	&uvcg_uncompressed_attr_bma_controls,
+	&uvcg_uncompressed_attr_b_variable_size,
 	NULL,
 };
 
@@ -1644,6 +1700,7 @@ static struct config_group *uvcg_uncompressed_make(struct config_group *group,
 	h->desc.bAspectRatioY		= 0;
 	h->desc.bmInterfaceFlags	= 0;
 	h->desc.bCopyProtect		= 0;
+	h->desc.bVariableSize		= 0;
 
 	h->fmt.type = UVCG_UNCOMPRESSED;
 	config_group_init_type_name(&h->fmt.group, name,
@@ -2038,7 +2095,7 @@ static int __uvcg_cnt_strm(void *priv1, void *priv2, void *priv3, int n,
 				container_of(fmt, struct uvcg_uncompressed,
 					     fmt);
 
-			*size += sizeof(u->desc);
+			*size += u->desc.bLength;
 		} else if (fmt->type == UVCG_MJPEG) {
 			struct uvcg_mjpeg *m =
 				container_of(fmt, struct uvcg_mjpeg, fmt);
@@ -2108,8 +2165,8 @@ static int __uvcg_fill_strm(void *priv1, void *priv2, void *priv3, int n,
 
 			u->desc.bFormatIndex = n + 1;
 			u->desc.bNumFrameDescriptors = fmt->num_frames;
-			memcpy(*dest, &u->desc, sizeof(u->desc));
-			*dest += sizeof(u->desc);
+			memcpy(*dest, &u->desc, u->desc.bLength);
+			*dest += u->desc.bLength;
 		} else if (fmt->type == UVCG_MJPEG) {
 			struct uvcg_mjpeg *m =
 				container_of(fmt, struct uvcg_mjpeg, fmt);
@@ -2129,6 +2186,18 @@ static int __uvcg_fill_strm(void *priv1, void *priv2, void *priv3, int n,
 
 		sz = sizeof(frm->frame);
 		memcpy(*dest, &frm->frame, sz);
+		/*
+		 * Reorder the frame struct layout due to the difference
+		 * between uncompressed frame and framebased frame.
+		 */
+		if (frm->frame.b_descriptor_subtype == UVC_VS_FRAME_FRAME_BASED) {
+			u8 *data = (u8 *)*dest;
+
+			memcpy(data + 17, &frm->frame.dw_default_frame_interval, 4);
+			memcpy(data + 21, &frm->frame.b_frame_interval_type, 1);
+			memcpy(data + 22, &frm->frame.dw_bytes_perline, 4);
+		}
+
 		*dest += sz;
 		sz = frm->frame.b_frame_interval_type *
 			sizeof(*frm->dw_frame_interval);
diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c
index a2c78690c5c2..3d6217328c50 100644
--- a/drivers/usb/gadget/function/uvc_v4l2.c
+++ b/drivers/usb/gadget/function/uvc_v4l2.c
@@ -58,6 +58,8 @@ struct uvc_format {
 static struct uvc_format uvc_formats[] = {
 	{ 16, V4L2_PIX_FMT_YUYV  },
 	{ 0,  V4L2_PIX_FMT_MJPEG },
+	{ 0,  V4L2_PIX_FMT_H264 },
+	{ 0,  V4L2_PIX_FMT_HEVC },
 };
 
 static int
diff --git a/include/uapi/linux/usb/video.h b/include/uapi/linux/usb/video.h
index bfdae12cdacf..383980bc9618 100644
--- a/include/uapi/linux/usb/video.h
+++ b/include/uapi/linux/usb/video.h
@@ -468,9 +468,12 @@ struct uvc_format_uncompressed {
 	__u8  bAspectRatioY;
 	__u8  bmInterfaceFlags;
 	__u8  bCopyProtect;
+	/* bVariableSize is only for framebased format. */
+	__u8  bVariableSize;
 } __attribute__((__packed__));
 
 #define UVC_DT_FORMAT_UNCOMPRESSED_SIZE			27
+#define UVC_DT_FORMAT_FRAMEBASED_SIZE			28
 
 /* Uncompressed Payload - 3.1.2. Uncompressed Video Frame Descriptor */
 struct uvc_frame_uncompressed {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-05-12  9:44 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-16  8:16 [PATCH] usb: gadget: uvc: add framebased stream support 3090101217
2022-02-16 18:22 ` kernel test robot
2022-02-16 18:22   ` kernel test robot
2022-02-18 11:08   ` Jing Leng
2022-02-18 11:08     ` Jing Leng
2022-02-17 10:44 ` [PATCH v2] " 3090101217
2022-02-17 15:27   ` Greg KH
2022-02-23  2:06     ` Jing Leng
2022-05-12  9:43   ` [PATCH v3] " 3090101217

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.