[PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[Michael Grzeschik]

This series improves the uvc video gadget by parsing the configfs
entries. With the configfs data, the userspace now is able to use simple
v4l2 api calls like enum and try_format to check for valid configurations
initially set by configfs.

Michael Grzeschik (4):
  media: v4l: move helper functions for fractions from uvc to
    v4l2-common
  media: uvcvideo: move uvc_format_desc to common header
  usb: gadget: uvc: add v4l2 enumeration api calls
  usb: gadget: uvc: add v4l2 try_format api call

 drivers/media/usb/uvc/uvc_ctrl.c       |   1 +
 drivers/media/usb/uvc/uvc_driver.c     | 290 +-------------------
 drivers/media/usb/uvc/uvc_v4l2.c       |  14 +-
 drivers/media/usb/uvc/uvcvideo.h       | 147 ----------
 drivers/media/v4l2-core/v4l2-common.c  |  86 ++++++
 drivers/usb/gadget/function/f_uvc.c    |  30 +++
 drivers/usb/gadget/function/uvc.h      |   2 +
 drivers/usb/gadget/function/uvc_v4l2.c | 286 ++++++++++++++++++++
 include/media/v4l2-common.h            |   4 +
 include/media/v4l2-uvc.h               | 359 +++++++++++++++++++++++++
 10 files changed, 776 insertions(+), 443 deletions(-)
 create mode 100644 include/media/v4l2-uvc.h

-- 
2.30.2

[PATCH v2 1/4] media: v4l: move helper functions for fractions from uvc to v4l2-common

[Michael Grzeschik]

The functions uvc_simplify_fraction and uvc_fraction_to_interval are
generic helpers which are also useful for other v4l2 drivers. This patch
moves them to v4l2-common.

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Tested-by: Daniel Scally <dan.scally@ideasonboard.com>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

---
v13 -> v1:
    - added Reviewed-by and Tested-by from Dan
    - moved to this smaller patch series
v1 -> v2:
    -

 drivers/media/usb/uvc/uvc_driver.c    | 84 --------------------------
 drivers/media/usb/uvc/uvc_v4l2.c      | 14 ++---
 drivers/media/usb/uvc/uvcvideo.h      |  3 -
 drivers/media/v4l2-core/v4l2-common.c | 86 +++++++++++++++++++++++++++
 include/media/v4l2-common.h           |  4 ++
 5 files changed, 97 insertions(+), 94 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 9c05776f11d1f0..0f14dee4b6d794 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -329,90 +329,6 @@ static enum v4l2_ycbcr_encoding uvc_ycbcr_enc(const u8 matrix_coefficients)
 	return V4L2_YCBCR_ENC_DEFAULT;  /* Reserved */
 }
 
-/*
- * Simplify a fraction using a simple continued fraction decomposition. The
- * idea here is to convert fractions such as 333333/10000000 to 1/30 using
- * 32 bit arithmetic only. The algorithm is not perfect and relies upon two
- * arbitrary parameters to remove non-significative terms from the simple
- * continued fraction decomposition. Using 8 and 333 for n_terms and threshold
- * respectively seems to give nice results.
- */
-void uvc_simplify_fraction(u32 *numerator, u32 *denominator,
-		unsigned int n_terms, unsigned int threshold)
-{
-	u32 *an;
-	u32 x, y, r;
-	unsigned int i, n;
-
-	an = kmalloc_array(n_terms, sizeof(*an), GFP_KERNEL);
-	if (an == NULL)
-		return;
-
-	/*
-	 * Convert the fraction to a simple continued fraction. See
-	 * https://en.wikipedia.org/wiki/Continued_fraction
-	 * Stop if the current term is bigger than or equal to the given
-	 * threshold.
-	 */
-	x = *numerator;
-	y = *denominator;
-
-	for (n = 0; n < n_terms && y != 0; ++n) {
-		an[n] = x / y;
-		if (an[n] >= threshold) {
-			if (n < 2)
-				n++;
-			break;
-		}
-
-		r = x - an[n] * y;
-		x = y;
-		y = r;
-	}
-
-	/* Expand the simple continued fraction back to an integer fraction. */
-	x = 0;
-	y = 1;
-
-	for (i = n; i > 0; --i) {
-		r = y;
-		y = an[i-1] * y + x;
-		x = r;
-	}
-
-	*numerator = y;
-	*denominator = x;
-	kfree(an);
-}
-
-/*
- * Convert a fraction to a frame interval in 100ns multiples. The idea here is
- * to compute numerator / denominator * 10000000 using 32 bit fixed point
- * arithmetic only.
- */
-u32 uvc_fraction_to_interval(u32 numerator, u32 denominator)
-{
-	u32 multiplier;
-
-	/* Saturate the result if the operation would overflow. */
-	if (denominator == 0 ||
-	    numerator/denominator >= ((u32)-1)/10000000)
-		return (u32)-1;
-
-	/*
-	 * Divide both the denominator and the multiplier by two until
-	 * numerator * multiplier doesn't overflow. If anyone knows a better
-	 * algorithm please let me know.
-	 */
-	multiplier = 10000000;
-	while (numerator > ((u32)-1)/multiplier) {
-		multiplier /= 2;
-		denominator /= 2;
-	}
-
-	return denominator ? numerator * multiplier / denominator : 0;
-}
-
 /* ------------------------------------------------------------------------
  * Terminal and unit management
  */
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 4cc3fa6b8c9812..f4d4c33b6dfbd7 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -386,7 +386,7 @@ static int uvc_v4l2_get_streamparm(struct uvc_streaming *stream,
 	mutex_unlock(&stream->mutex);
 
 	denominator = 10000000;
-	uvc_simplify_fraction(&numerator, &denominator, 8, 333);
+	v4l2_simplify_fraction(&numerator, &denominator, 8, 333);
 
 	memset(parm, 0, sizeof(*parm));
 	parm->type = stream->type;
@@ -427,7 +427,7 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming *stream,
 	else
 		timeperframe = parm->parm.output.timeperframe;
 
-	interval = uvc_fraction_to_interval(timeperframe.numerator,
+	interval = v4l2_fraction_to_interval(timeperframe.numerator,
 		timeperframe.denominator);
 	uvc_dbg(stream->dev, FORMAT, "Setting frame interval to %u/%u (%u)\n",
 		timeperframe.numerator, timeperframe.denominator, interval);
@@ -481,7 +481,7 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming *stream,
 	/* Return the actual frame period. */
 	timeperframe.numerator = probe.dwFrameInterval;
 	timeperframe.denominator = 10000000;
-	uvc_simplify_fraction(&timeperframe.numerator,
+	v4l2_simplify_fraction(&timeperframe.numerator,
 		&timeperframe.denominator, 8, 333);
 
 	if (parm->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
@@ -1275,7 +1275,7 @@ static int uvc_ioctl_enum_frameintervals(struct file *file, void *fh,
 		fival->discrete.numerator =
 			frame->dwFrameInterval[index];
 		fival->discrete.denominator = 10000000;
-		uvc_simplify_fraction(&fival->discrete.numerator,
+		v4l2_simplify_fraction(&fival->discrete.numerator,
 			&fival->discrete.denominator, 8, 333);
 	} else {
 		fival->type = V4L2_FRMIVAL_TYPE_STEPWISE;
@@ -1285,11 +1285,11 @@ static int uvc_ioctl_enum_frameintervals(struct file *file, void *fh,
 		fival->stepwise.max.denominator = 10000000;
 		fival->stepwise.step.numerator = frame->dwFrameInterval[2];
 		fival->stepwise.step.denominator = 10000000;
-		uvc_simplify_fraction(&fival->stepwise.min.numerator,
+		v4l2_simplify_fraction(&fival->stepwise.min.numerator,
 			&fival->stepwise.min.denominator, 8, 333);
-		uvc_simplify_fraction(&fival->stepwise.max.numerator,
+		v4l2_simplify_fraction(&fival->stepwise.max.numerator,
 			&fival->stepwise.max.denominator, 8, 333);
-		uvc_simplify_fraction(&fival->stepwise.step.numerator,
+		v4l2_simplify_fraction(&fival->stepwise.step.numerator,
 			&fival->stepwise.step.denominator, 8, 333);
 	}
 
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 24c911aeebce56..ff710bdd38b3fd 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -911,9 +911,6 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
 		      struct uvc_xu_control_query *xqry);
 
 /* Utility functions */
-void uvc_simplify_fraction(u32 *numerator, u32 *denominator,
-			   unsigned int n_terms, unsigned int threshold);
-u32 uvc_fraction_to_interval(u32 numerator, u32 denominator);
 struct usb_host_endpoint *uvc_find_endpoint(struct usb_host_interface *alts,
 					    u8 epaddr);
 u16 uvc_endpoint_max_bpi(struct usb_device *dev, struct usb_host_endpoint *ep);
diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index e0fbe6ba4b6c49..40f56e044640d7 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -484,3 +484,89 @@ s64 v4l2_get_link_freq(struct v4l2_ctrl_handler *handler, unsigned int mul,
 	return freq > 0 ? freq : -EINVAL;
 }
 EXPORT_SYMBOL_GPL(v4l2_get_link_freq);
+
+/*
+ * Simplify a fraction using a simple continued fraction decomposition. The
+ * idea here is to convert fractions such as 333333/10000000 to 1/30 using
+ * 32 bit arithmetic only. The algorithm is not perfect and relies upon two
+ * arbitrary parameters to remove non-significative terms from the simple
+ * continued fraction decomposition. Using 8 and 333 for n_terms and threshold
+ * respectively seems to give nice results.
+ */
+void v4l2_simplify_fraction(u32 *numerator, u32 *denominator,
+		unsigned int n_terms, unsigned int threshold)
+{
+	u32 *an;
+	u32 x, y, r;
+	unsigned int i, n;
+
+	an = kmalloc_array(n_terms, sizeof(*an), GFP_KERNEL);
+	if (an == NULL)
+		return;
+
+	/*
+	 * Convert the fraction to a simple continued fraction. See
+	 * https://en.wikipedia.org/wiki/Continued_fraction
+	 * Stop if the current term is bigger than or equal to the given
+	 * threshold.
+	 */
+	x = *numerator;
+	y = *denominator;
+
+	for (n = 0; n < n_terms && y != 0; ++n) {
+		an[n] = x / y;
+		if (an[n] >= threshold) {
+			if (n < 2)
+				n++;
+			break;
+		}
+
+		r = x - an[n] * y;
+		x = y;
+		y = r;
+	}
+
+	/* Expand the simple continued fraction back to an integer fraction. */
+	x = 0;
+	y = 1;
+
+	for (i = n; i > 0; --i) {
+		r = y;
+		y = an[i-1] * y + x;
+		x = r;
+	}
+
+	*numerator = y;
+	*denominator = x;
+	kfree(an);
+}
+EXPORT_SYMBOL_GPL(v4l2_simplify_fraction);
+
+/*
+ * Convert a fraction to a frame interval in 100ns multiples. The idea here is
+ * to compute numerator / denominator * 10000000 using 32 bit fixed point
+ * arithmetic only.
+ */
+u32 v4l2_fraction_to_interval(u32 numerator, u32 denominator)
+{
+	u32 multiplier;
+
+	/* Saturate the result if the operation would overflow. */
+	if (denominator == 0 ||
+	    numerator/denominator >= ((u32)-1)/10000000)
+		return (u32)-1;
+
+	/*
+	 * Divide both the denominator and the multiplier by two until
+	 * numerator * multiplier doesn't overflow. If anyone knows a better
+	 * algorithm please let me know.
+	 */
+	multiplier = 10000000;
+	while (numerator > ((u32)-1)/multiplier) {
+		multiplier /= 2;
+		denominator /= 2;
+	}
+
+	return denominator ? numerator * multiplier / denominator : 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_fraction_to_interval);
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index b708d63995f458..725ff91b26e063 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -540,6 +540,10 @@ int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, u32 pixelformat,
 s64 v4l2_get_link_freq(struct v4l2_ctrl_handler *handler, unsigned int mul,
 		       unsigned int div);
 
+void v4l2_simplify_fraction(u32 *numerator, u32 *denominator,
+		unsigned int n_terms, unsigned int threshold);
+u32 v4l2_fraction_to_interval(u32 numerator, u32 denominator);
+
 static inline u64 v4l2_buffer_get_timestamp(const struct v4l2_buffer *buf)
 {
 	/*
-- 
2.30.2

[PATCH v2 2/4] media: uvcvideo: move uvc_format_desc to common header

[Michael Grzeschik]

The uvc_format_desc, GUID defines and the uvc_format_by_guid helper is
also useful for the uvc gadget stack. This patch moves them to a common
header.

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Tested-by: Daniel Scally <dan.scally@ideasonboard.com>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

---
v13 -> v1:
    - added Reviewed-by and Tested-by from Dan
    - moved to this smaller patch series
v1 -> v2:
    -

 drivers/media/usb/uvc/uvc_ctrl.c   |   1 +
 drivers/media/usb/uvc/uvc_driver.c | 206 +----------------
 drivers/media/usb/uvc/uvcvideo.h   | 144 ------------
 include/media/v4l2-uvc.h           | 359 +++++++++++++++++++++++++++++
 4 files changed, 361 insertions(+), 349 deletions(-)
 create mode 100644 include/media/v4l2-uvc.h

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 8c208db9600b46..b8a00a51679f6d 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -18,6 +18,7 @@
 #include <linux/workqueue.h>
 #include <linux/atomic.h>
 #include <media/v4l2-ctrls.h>
+#include <media/v4l2-uvc.h>
 
 #include "uvcvideo.h"
 
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 0f14dee4b6d794..2891bc9d319280 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -20,6 +20,7 @@
 
 #include <media/v4l2-common.h>
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-uvc.h>
 
 #include "uvcvideo.h"
 
@@ -34,198 +35,6 @@ static unsigned int uvc_quirks_param = -1;
 unsigned int uvc_dbg_param;
 unsigned int uvc_timeout_param = UVC_CTRL_STREAMING_TIMEOUT;
 
-/* ------------------------------------------------------------------------
- * Video formats
- */
-
-static struct uvc_format_desc uvc_fmts[] = {
-	{
-		.name		= "YUV 4:2:2 (YUYV)",
-		.guid		= UVC_GUID_FORMAT_YUY2,
-		.fcc		= V4L2_PIX_FMT_YUYV,
-	},
-	{
-		.name		= "YUV 4:2:2 (YUYV)",
-		.guid		= UVC_GUID_FORMAT_YUY2_ISIGHT,
-		.fcc		= V4L2_PIX_FMT_YUYV,
-	},
-	{
-		.name		= "YUV 4:2:0 (NV12)",
-		.guid		= UVC_GUID_FORMAT_NV12,
-		.fcc		= V4L2_PIX_FMT_NV12,
-	},
-	{
-		.name		= "MJPEG",
-		.guid		= UVC_GUID_FORMAT_MJPEG,
-		.fcc		= V4L2_PIX_FMT_MJPEG,
-	},
-	{
-		.name		= "YVU 4:2:0 (YV12)",
-		.guid		= UVC_GUID_FORMAT_YV12,
-		.fcc		= V4L2_PIX_FMT_YVU420,
-	},
-	{
-		.name		= "YUV 4:2:0 (I420)",
-		.guid		= UVC_GUID_FORMAT_I420,
-		.fcc		= V4L2_PIX_FMT_YUV420,
-	},
-	{
-		.name		= "YUV 4:2:0 (M420)",
-		.guid		= UVC_GUID_FORMAT_M420,
-		.fcc		= V4L2_PIX_FMT_M420,
-	},
-	{
-		.name		= "YUV 4:2:2 (UYVY)",
-		.guid		= UVC_GUID_FORMAT_UYVY,
-		.fcc		= V4L2_PIX_FMT_UYVY,
-	},
-	{
-		.name		= "Greyscale 8-bit (Y800)",
-		.guid		= UVC_GUID_FORMAT_Y800,
-		.fcc		= V4L2_PIX_FMT_GREY,
-	},
-	{
-		.name		= "Greyscale 8-bit (Y8  )",
-		.guid		= UVC_GUID_FORMAT_Y8,
-		.fcc		= V4L2_PIX_FMT_GREY,
-	},
-	{
-		.name		= "Greyscale 8-bit (D3DFMT_L8)",
-		.guid		= UVC_GUID_FORMAT_D3DFMT_L8,
-		.fcc		= V4L2_PIX_FMT_GREY,
-	},
-	{
-		.name		= "IR 8-bit (L8_IR)",
-		.guid		= UVC_GUID_FORMAT_KSMEDIA_L8_IR,
-		.fcc		= V4L2_PIX_FMT_GREY,
-	},
-	{
-		.name		= "Greyscale 10-bit (Y10 )",
-		.guid		= UVC_GUID_FORMAT_Y10,
-		.fcc		= V4L2_PIX_FMT_Y10,
-	},
-	{
-		.name		= "Greyscale 12-bit (Y12 )",
-		.guid		= UVC_GUID_FORMAT_Y12,
-		.fcc		= V4L2_PIX_FMT_Y12,
-	},
-	{
-		.name		= "Greyscale 16-bit (Y16 )",
-		.guid		= UVC_GUID_FORMAT_Y16,
-		.fcc		= V4L2_PIX_FMT_Y16,
-	},
-	{
-		.name		= "BGGR Bayer (BY8 )",
-		.guid		= UVC_GUID_FORMAT_BY8,
-		.fcc		= V4L2_PIX_FMT_SBGGR8,
-	},
-	{
-		.name		= "BGGR Bayer (BA81)",
-		.guid		= UVC_GUID_FORMAT_BA81,
-		.fcc		= V4L2_PIX_FMT_SBGGR8,
-	},
-	{
-		.name		= "GBRG Bayer (GBRG)",
-		.guid		= UVC_GUID_FORMAT_GBRG,
-		.fcc		= V4L2_PIX_FMT_SGBRG8,
-	},
-	{
-		.name		= "GRBG Bayer (GRBG)",
-		.guid		= UVC_GUID_FORMAT_GRBG,
-		.fcc		= V4L2_PIX_FMT_SGRBG8,
-	},
-	{
-		.name		= "RGGB Bayer (RGGB)",
-		.guid		= UVC_GUID_FORMAT_RGGB,
-		.fcc		= V4L2_PIX_FMT_SRGGB8,
-	},
-	{
-		.name		= "RGB565",
-		.guid		= UVC_GUID_FORMAT_RGBP,
-		.fcc		= V4L2_PIX_FMT_RGB565,
-	},
-	{
-		.name		= "BGR 8:8:8 (BGR3)",
-		.guid		= UVC_GUID_FORMAT_BGR3,
-		.fcc		= V4L2_PIX_FMT_BGR24,
-	},
-	{
-		.name		= "H.264",
-		.guid		= UVC_GUID_FORMAT_H264,
-		.fcc		= V4L2_PIX_FMT_H264,
-	},
-	{
-		.name		= "H.265",
-		.guid		= UVC_GUID_FORMAT_H265,
-		.fcc		= V4L2_PIX_FMT_HEVC,
-	},
-	{
-		.name		= "Greyscale 8 L/R (Y8I)",
-		.guid		= UVC_GUID_FORMAT_Y8I,
-		.fcc		= V4L2_PIX_FMT_Y8I,
-	},
-	{
-		.name		= "Greyscale 12 L/R (Y12I)",
-		.guid		= UVC_GUID_FORMAT_Y12I,
-		.fcc		= V4L2_PIX_FMT_Y12I,
-	},
-	{
-		.name		= "Depth data 16-bit (Z16)",
-		.guid		= UVC_GUID_FORMAT_Z16,
-		.fcc		= V4L2_PIX_FMT_Z16,
-	},
-	{
-		.name		= "Bayer 10-bit (SRGGB10P)",
-		.guid		= UVC_GUID_FORMAT_RW10,
-		.fcc		= V4L2_PIX_FMT_SRGGB10P,
-	},
-	{
-		.name		= "Bayer 16-bit (SBGGR16)",
-		.guid		= UVC_GUID_FORMAT_BG16,
-		.fcc		= V4L2_PIX_FMT_SBGGR16,
-	},
-	{
-		.name		= "Bayer 16-bit (SGBRG16)",
-		.guid		= UVC_GUID_FORMAT_GB16,
-		.fcc		= V4L2_PIX_FMT_SGBRG16,
-	},
-	{
-		.name		= "Bayer 16-bit (SRGGB16)",
-		.guid		= UVC_GUID_FORMAT_RG16,
-		.fcc		= V4L2_PIX_FMT_SRGGB16,
-	},
-	{
-		.name		= "Bayer 16-bit (SGRBG16)",
-		.guid		= UVC_GUID_FORMAT_GR16,
-		.fcc		= V4L2_PIX_FMT_SGRBG16,
-	},
-	{
-		.name		= "Depth data 16-bit (Z16)",
-		.guid		= UVC_GUID_FORMAT_INVZ,
-		.fcc		= V4L2_PIX_FMT_Z16,
-	},
-	{
-		.name		= "Greyscale 10-bit (Y10 )",
-		.guid		= UVC_GUID_FORMAT_INVI,
-		.fcc		= V4L2_PIX_FMT_Y10,
-	},
-	{
-		.name		= "IR:Depth 26-bit (INZI)",
-		.guid		= UVC_GUID_FORMAT_INZI,
-		.fcc		= V4L2_PIX_FMT_INZI,
-	},
-	{
-		.name		= "4-bit Depth Confidence (Packed)",
-		.guid		= UVC_GUID_FORMAT_CNF4,
-		.fcc		= V4L2_PIX_FMT_CNF4,
-	},
-	{
-		.name		= "HEVC",
-		.guid		= UVC_GUID_FORMAT_HEVC,
-		.fcc		= V4L2_PIX_FMT_HEVC,
-	},
-};
-
 /* ------------------------------------------------------------------------
  * Utility functions
  */
@@ -245,19 +54,6 @@ struct usb_host_endpoint *uvc_find_endpoint(struct usb_host_interface *alts,
 	return NULL;
 }
 
-static struct uvc_format_desc *uvc_format_by_guid(const u8 guid[16])
-{
-	unsigned int len = ARRAY_SIZE(uvc_fmts);
-	unsigned int i;
-
-	for (i = 0; i < len; ++i) {
-		if (memcmp(guid, uvc_fmts[i].guid, 16) == 0)
-			return &uvc_fmts[i];
-	}
-
-	return NULL;
-}
-
 static enum v4l2_colorspace uvc_colorspace(const u8 primaries)
 {
 	static const enum v4l2_colorspace colorprimaries[] = {
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index ff710bdd38b3fd..df93db259312e7 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -41,144 +41,6 @@
 #define UVC_EXT_GPIO_UNIT		0x7ffe
 #define UVC_EXT_GPIO_UNIT_ID		0x100
 
-/* ------------------------------------------------------------------------
- * GUIDs
- */
-#define UVC_GUID_UVC_CAMERA \
-	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
-	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}
-#define UVC_GUID_UVC_OUTPUT \
-	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
-	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02}
-#define UVC_GUID_UVC_MEDIA_TRANSPORT_INPUT \
-	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
-	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03}
-#define UVC_GUID_UVC_PROCESSING \
-	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
-	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x01}
-#define UVC_GUID_UVC_SELECTOR \
-	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
-	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x02}
-#define UVC_GUID_EXT_GPIO_CONTROLLER \
-	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
-	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03}
-
-#define UVC_GUID_FORMAT_MJPEG \
-	{ 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_YUY2 \
-	{ 'Y',  'U',  'Y',  '2', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_YUY2_ISIGHT \
-	{ 'Y',  'U',  'Y',  '2', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0x00, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_NV12 \
-	{ 'N',  'V',  '1',  '2', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_YV12 \
-	{ 'Y',  'V',  '1',  '2', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_I420 \
-	{ 'I',  '4',  '2',  '0', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_UYVY \
-	{ 'U',  'Y',  'V',  'Y', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_Y800 \
-	{ 'Y',  '8',  '0',  '0', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_Y8 \
-	{ 'Y',  '8',  ' ',  ' ', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_Y10 \
-	{ 'Y',  '1',  '0',  ' ', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_Y12 \
-	{ 'Y',  '1',  '2',  ' ', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_Y16 \
-	{ 'Y',  '1',  '6',  ' ', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_BY8 \
-	{ 'B',  'Y',  '8',  ' ', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_BA81 \
-	{ 'B',  'A',  '8',  '1', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_GBRG \
-	{ 'G',  'B',  'R',  'G', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_GRBG \
-	{ 'G',  'R',  'B',  'G', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_RGGB \
-	{ 'R',  'G',  'G',  'B', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_BG16 \
-	{ 'B',  'G',  '1',  '6', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_GB16 \
-	{ 'G',  'B',  '1',  '6', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_RG16 \
-	{ 'R',  'G',  '1',  '6', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_GR16 \
-	{ 'G',  'R',  '1',  '6', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_RGBP \
-	{ 'R',  'G',  'B',  'P', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_BGR3 \
-	{ 0x7d, 0xeb, 0x36, 0xe4, 0x4f, 0x52, 0xce, 0x11, \
-	 0x9f, 0x53, 0x00, 0x20, 0xaf, 0x0b, 0xa7, 0x70}
-#define UVC_GUID_FORMAT_M420 \
-	{ 'M',  '4',  '2',  '0', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-
-#define UVC_GUID_FORMAT_H264 \
-	{ 'H',  '2',  '6',  '4', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_H265 \
-	{ 'H',  '2',  '6',  '5', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_Y8I \
-	{ 'Y',  '8',  'I',  ' ', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_Y12I \
-	{ 'Y',  '1',  '2',  'I', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_Z16 \
-	{ 'Z',  '1',  '6',  ' ', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_RW10 \
-	{ 'R',  'W',  '1',  '0', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_INVZ \
-	{ 'I',  'N',  'V',  'Z', 0x90, 0x2d, 0x58, 0x4a, \
-	 0x92, 0x0b, 0x77, 0x3f, 0x1f, 0x2c, 0x55, 0x6b}
-#define UVC_GUID_FORMAT_INZI \
-	{ 'I',  'N',  'Z',  'I', 0x66, 0x1a, 0x42, 0xa2, \
-	 0x90, 0x65, 0xd0, 0x18, 0x14, 0xa8, 0xef, 0x8a}
-#define UVC_GUID_FORMAT_INVI \
-	{ 'I',  'N',  'V',  'I', 0xdb, 0x57, 0x49, 0x5e, \
-	 0x8e, 0x3f, 0xf4, 0x79, 0x53, 0x2b, 0x94, 0x6f}
-#define UVC_GUID_FORMAT_CNF4 \
-	{ 'C',  ' ',  ' ',  ' ', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-
-#define UVC_GUID_FORMAT_D3DFMT_L8 \
-	{0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-#define UVC_GUID_FORMAT_KSMEDIA_L8_IR \
-	{0x32, 0x00, 0x00, 0x00, 0x02, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-
-#define UVC_GUID_FORMAT_HEVC \
-	{ 'H',  'E',  'V',  'C', 0x00, 0x00, 0x10, 0x00, \
-	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-
-
 /* ------------------------------------------------------------------------
  * Driver specific constants.
  */
@@ -283,12 +145,6 @@ struct uvc_control {
 	struct uvc_fh *handle;	/* File handle that last changed the control. */
 };
 
-struct uvc_format_desc {
-	char *name;
-	u8 guid[16];
-	u32 fcc;
-};
-
 /*
  * The term 'entity' refers to both UVC units and UVC terminals.
  *
diff --git a/include/media/v4l2-uvc.h b/include/media/v4l2-uvc.h
new file mode 100644
index 00000000000000..f83e31661333bb
--- /dev/null
+++ b/include/media/v4l2-uvc.h
@@ -0,0 +1,359 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ *  v4l2 uvc internal API header
+ *
+ *  Some commonly needed functions for uvc drivers
+ */
+
+#ifndef __LINUX_V4L2_UVC_H
+#define __LINUX_V4L2_UVC_H
+
+/* ------------------------------------------------------------------------
+ * GUIDs
+ */
+#define UVC_GUID_UVC_CAMERA \
+	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
+	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}
+#define UVC_GUID_UVC_OUTPUT \
+	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
+	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02}
+#define UVC_GUID_UVC_MEDIA_TRANSPORT_INPUT \
+	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
+	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03}
+#define UVC_GUID_UVC_PROCESSING \
+	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
+	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x01}
+#define UVC_GUID_UVC_SELECTOR \
+	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
+	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x02}
+#define UVC_GUID_EXT_GPIO_CONTROLLER \
+	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
+	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03}
+
+#define UVC_GUID_FORMAT_MJPEG \
+	{ 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_YUY2 \
+	{ 'Y',  'U',  'Y',  '2', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_YUY2_ISIGHT \
+	{ 'Y',  'U',  'Y',  '2', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0x00, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_NV12 \
+	{ 'N',  'V',  '1',  '2', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_YV12 \
+	{ 'Y',  'V',  '1',  '2', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_I420 \
+	{ 'I',  '4',  '2',  '0', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_UYVY \
+	{ 'U',  'Y',  'V',  'Y', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_Y800 \
+	{ 'Y',  '8',  '0',  '0', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_Y8 \
+	{ 'Y',  '8',  ' ',  ' ', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_Y10 \
+	{ 'Y',  '1',  '0',  ' ', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_Y12 \
+	{ 'Y',  '1',  '2',  ' ', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_Y16 \
+	{ 'Y',  '1',  '6',  ' ', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_BY8 \
+	{ 'B',  'Y',  '8',  ' ', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_BA81 \
+	{ 'B',  'A',  '8',  '1', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_GBRG \
+	{ 'G',  'B',  'R',  'G', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_GRBG \
+	{ 'G',  'R',  'B',  'G', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_RGGB \
+	{ 'R',  'G',  'G',  'B', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_BG16 \
+	{ 'B',  'G',  '1',  '6', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_GB16 \
+	{ 'G',  'B',  '1',  '6', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_RG16 \
+	{ 'R',  'G',  '1',  '6', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_GR16 \
+	{ 'G',  'R',  '1',  '6', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_RGBP \
+	{ 'R',  'G',  'B',  'P', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_BGR3 \
+	{ 0x7d, 0xeb, 0x36, 0xe4, 0x4f, 0x52, 0xce, 0x11, \
+	 0x9f, 0x53, 0x00, 0x20, 0xaf, 0x0b, 0xa7, 0x70}
+#define UVC_GUID_FORMAT_M420 \
+	{ 'M',  '4',  '2',  '0', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+
+#define UVC_GUID_FORMAT_H264 \
+	{ 'H',  '2',  '6',  '4', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_H265 \
+	{ 'H',  '2',  '6',  '5', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_Y8I \
+	{ 'Y',  '8',  'I',  ' ', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_Y12I \
+	{ 'Y',  '1',  '2',  'I', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_Z16 \
+	{ 'Z',  '1',  '6',  ' ', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_RW10 \
+	{ 'R',  'W',  '1',  '0', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_INVZ \
+	{ 'I',  'N',  'V',  'Z', 0x90, 0x2d, 0x58, 0x4a, \
+	 0x92, 0x0b, 0x77, 0x3f, 0x1f, 0x2c, 0x55, 0x6b}
+#define UVC_GUID_FORMAT_INZI \
+	{ 'I',  'N',  'Z',  'I', 0x66, 0x1a, 0x42, 0xa2, \
+	 0x90, 0x65, 0xd0, 0x18, 0x14, 0xa8, 0xef, 0x8a}
+#define UVC_GUID_FORMAT_INVI \
+	{ 'I',  'N',  'V',  'I', 0xdb, 0x57, 0x49, 0x5e, \
+	 0x8e, 0x3f, 0xf4, 0x79, 0x53, 0x2b, 0x94, 0x6f}
+#define UVC_GUID_FORMAT_CNF4 \
+	{ 'C',  ' ',  ' ',  ' ', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+
+#define UVC_GUID_FORMAT_D3DFMT_L8 \
+	{0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_KSMEDIA_L8_IR \
+	{0x32, 0x00, 0x00, 0x00, 0x02, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+
+#define UVC_GUID_FORMAT_HEVC \
+	{ 'H',  'E',  'V',  'C', 0x00, 0x00, 0x10, 0x00, \
+	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+
+/* ------------------------------------------------------------------------
+ * Video formats
+ */
+
+struct uvc_format_desc {
+	char *name;
+	u8 guid[16];
+	u32 fcc;
+};
+
+static struct uvc_format_desc uvc_fmts[] = {
+	{
+		.name		= "YUV 4:2:2 (YUYV)",
+		.guid		= UVC_GUID_FORMAT_YUY2,
+		.fcc		= V4L2_PIX_FMT_YUYV,
+	},
+	{
+		.name		= "YUV 4:2:2 (YUYV)",
+		.guid		= UVC_GUID_FORMAT_YUY2_ISIGHT,
+		.fcc		= V4L2_PIX_FMT_YUYV,
+	},
+	{
+		.name		= "YUV 4:2:0 (NV12)",
+		.guid		= UVC_GUID_FORMAT_NV12,
+		.fcc		= V4L2_PIX_FMT_NV12,
+	},
+	{
+		.name		= "MJPEG",
+		.guid		= UVC_GUID_FORMAT_MJPEG,
+		.fcc		= V4L2_PIX_FMT_MJPEG,
+	},
+	{
+		.name		= "YVU 4:2:0 (YV12)",
+		.guid		= UVC_GUID_FORMAT_YV12,
+		.fcc		= V4L2_PIX_FMT_YVU420,
+	},
+	{
+		.name		= "YUV 4:2:0 (I420)",
+		.guid		= UVC_GUID_FORMAT_I420,
+		.fcc		= V4L2_PIX_FMT_YUV420,
+	},
+	{
+		.name		= "YUV 4:2:0 (M420)",
+		.guid		= UVC_GUID_FORMAT_M420,
+		.fcc		= V4L2_PIX_FMT_M420,
+	},
+	{
+		.name		= "YUV 4:2:2 (UYVY)",
+		.guid		= UVC_GUID_FORMAT_UYVY,
+		.fcc		= V4L2_PIX_FMT_UYVY,
+	},
+	{
+		.name		= "Greyscale 8-bit (Y800)",
+		.guid		= UVC_GUID_FORMAT_Y800,
+		.fcc		= V4L2_PIX_FMT_GREY,
+	},
+	{
+		.name		= "Greyscale 8-bit (Y8  )",
+		.guid		= UVC_GUID_FORMAT_Y8,
+		.fcc		= V4L2_PIX_FMT_GREY,
+	},
+	{
+		.name		= "Greyscale 8-bit (D3DFMT_L8)",
+		.guid		= UVC_GUID_FORMAT_D3DFMT_L8,
+		.fcc		= V4L2_PIX_FMT_GREY,
+	},
+	{
+		.name		= "IR 8-bit (L8_IR)",
+		.guid		= UVC_GUID_FORMAT_KSMEDIA_L8_IR,
+		.fcc		= V4L2_PIX_FMT_GREY,
+	},
+	{
+		.name		= "Greyscale 10-bit (Y10 )",
+		.guid		= UVC_GUID_FORMAT_Y10,
+		.fcc		= V4L2_PIX_FMT_Y10,
+	},
+	{
+		.name		= "Greyscale 12-bit (Y12 )",
+		.guid		= UVC_GUID_FORMAT_Y12,
+		.fcc		= V4L2_PIX_FMT_Y12,
+	},
+	{
+		.name		= "Greyscale 16-bit (Y16 )",
+		.guid		= UVC_GUID_FORMAT_Y16,
+		.fcc		= V4L2_PIX_FMT_Y16,
+	},
+	{
+		.name		= "BGGR Bayer (BY8 )",
+		.guid		= UVC_GUID_FORMAT_BY8,
+		.fcc		= V4L2_PIX_FMT_SBGGR8,
+	},
+	{
+		.name		= "BGGR Bayer (BA81)",
+		.guid		= UVC_GUID_FORMAT_BA81,
+		.fcc		= V4L2_PIX_FMT_SBGGR8,
+	},
+	{
+		.name		= "GBRG Bayer (GBRG)",
+		.guid		= UVC_GUID_FORMAT_GBRG,
+		.fcc		= V4L2_PIX_FMT_SGBRG8,
+	},
+	{
+		.name		= "GRBG Bayer (GRBG)",
+		.guid		= UVC_GUID_FORMAT_GRBG,
+		.fcc		= V4L2_PIX_FMT_SGRBG8,
+	},
+	{
+		.name		= "RGGB Bayer (RGGB)",
+		.guid		= UVC_GUID_FORMAT_RGGB,
+		.fcc		= V4L2_PIX_FMT_SRGGB8,
+	},
+	{
+		.name		= "RGB565",
+		.guid		= UVC_GUID_FORMAT_RGBP,
+		.fcc		= V4L2_PIX_FMT_RGB565,
+	},
+	{
+		.name		= "BGR 8:8:8 (BGR3)",
+		.guid		= UVC_GUID_FORMAT_BGR3,
+		.fcc		= V4L2_PIX_FMT_BGR24,
+	},
+	{
+		.name		= "H.264",
+		.guid		= UVC_GUID_FORMAT_H264,
+		.fcc		= V4L2_PIX_FMT_H264,
+	},
+	{
+		.name		= "H.265",
+		.guid		= UVC_GUID_FORMAT_H265,
+		.fcc		= V4L2_PIX_FMT_HEVC,
+	},
+	{
+		.name		= "Greyscale 8 L/R (Y8I)",
+		.guid		= UVC_GUID_FORMAT_Y8I,
+		.fcc		= V4L2_PIX_FMT_Y8I,
+	},
+	{
+		.name		= "Greyscale 12 L/R (Y12I)",
+		.guid		= UVC_GUID_FORMAT_Y12I,
+		.fcc		= V4L2_PIX_FMT_Y12I,
+	},
+	{
+		.name		= "Depth data 16-bit (Z16)",
+		.guid		= UVC_GUID_FORMAT_Z16,
+		.fcc		= V4L2_PIX_FMT_Z16,
+	},
+	{
+		.name		= "Bayer 10-bit (SRGGB10P)",
+		.guid		= UVC_GUID_FORMAT_RW10,
+		.fcc		= V4L2_PIX_FMT_SRGGB10P,
+	},
+	{
+		.name		= "Bayer 16-bit (SBGGR16)",
+		.guid		= UVC_GUID_FORMAT_BG16,
+		.fcc		= V4L2_PIX_FMT_SBGGR16,
+	},
+	{
+		.name		= "Bayer 16-bit (SGBRG16)",
+		.guid		= UVC_GUID_FORMAT_GB16,
+		.fcc		= V4L2_PIX_FMT_SGBRG16,
+	},
+	{
+		.name		= "Bayer 16-bit (SRGGB16)",
+		.guid		= UVC_GUID_FORMAT_RG16,
+		.fcc		= V4L2_PIX_FMT_SRGGB16,
+	},
+	{
+		.name		= "Bayer 16-bit (SGRBG16)",
+		.guid		= UVC_GUID_FORMAT_GR16,
+		.fcc		= V4L2_PIX_FMT_SGRBG16,
+	},
+	{
+		.name		= "Depth data 16-bit (Z16)",
+		.guid		= UVC_GUID_FORMAT_INVZ,
+		.fcc		= V4L2_PIX_FMT_Z16,
+	},
+	{
+		.name		= "Greyscale 10-bit (Y10 )",
+		.guid		= UVC_GUID_FORMAT_INVI,
+		.fcc		= V4L2_PIX_FMT_Y10,
+	},
+	{
+		.name		= "IR:Depth 26-bit (INZI)",
+		.guid		= UVC_GUID_FORMAT_INZI,
+		.fcc		= V4L2_PIX_FMT_INZI,
+	},
+	{
+		.name		= "4-bit Depth Confidence (Packed)",
+		.guid		= UVC_GUID_FORMAT_CNF4,
+		.fcc		= V4L2_PIX_FMT_CNF4,
+	},
+	{
+		.name		= "HEVC",
+		.guid		= UVC_GUID_FORMAT_HEVC,
+		.fcc		= V4L2_PIX_FMT_HEVC,
+	},
+};
+
+static inline struct uvc_format_desc *uvc_format_by_guid(const u8 guid[16])
+{
+	unsigned int len = ARRAY_SIZE(uvc_fmts);
+	unsigned int i;
+
+	for (i = 0; i < len; ++i) {
+		if (memcmp(guid, uvc_fmts[i].guid, 16) == 0)
+			return &uvc_fmts[i];
+	}
+
+	return NULL;
+}
+
+#endif /* __LINUX_V4L2_UVC_H */
-- 
2.30.2

[PATCH v2 3/4] usb: gadget: uvc: add v4l2 enumeration api calls

[Michael Grzeschik]

This patch adds support to the v4l2 VIDIOCs for enum_format,
enum_framesizes and enum_frameintervals. This way, the userspace
application can use these VIDIOCS to query the via configfs exported
frame capabilities. With thes callbacks the userspace doesn't have to
bring its own configfs parser.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

---
v13 -> v1:
    - moved to this smaller patch series
    - improved the error handling in uvc_alloc like suggested by Dan Scally
v1  -> v2:
    - declare all local helper functions as static

 drivers/usb/gadget/function/f_uvc.c    |  30 +++++
 drivers/usb/gadget/function/uvc.h      |   2 +
 drivers/usb/gadget/function/uvc_v4l2.c | 176 +++++++++++++++++++++++++
 3 files changed, 208 insertions(+)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 09961f4ca981da..e6948cf8def30b 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -888,6 +888,7 @@ static void uvc_free(struct usb_function *f)
 	struct uvc_device *uvc = to_uvc(f);
 	struct f_uvc_opts *opts = container_of(f->fi, struct f_uvc_opts,
 					       func_inst);
+	config_item_put(&uvc->header->item);
 	--opts->refcnt;
 	kfree(uvc);
 }
@@ -945,6 +946,7 @@ static struct usb_function *uvc_alloc(struct usb_function_instance *fi)
 	struct uvc_device *uvc;
 	struct f_uvc_opts *opts;
 	struct uvc_descriptor_header **strm_cls;
+	struct config_item *streaming, *header, *h;
 
 	uvc = kzalloc(sizeof(*uvc), GFP_KERNEL);
 	if (uvc == NULL)
@@ -977,6 +979,29 @@ static struct usb_function *uvc_alloc(struct usb_function_instance *fi)
 	uvc->desc.fs_streaming = opts->fs_streaming;
 	uvc->desc.hs_streaming = opts->hs_streaming;
 	uvc->desc.ss_streaming = opts->ss_streaming;
+
+	streaming = config_group_find_item(&opts->func_inst.group, "streaming");
+	if (!streaming)
+		goto err_config;
+
+	header = config_group_find_item(to_config_group(streaming), "header");
+	config_item_put(streaming);
+	if (!header)
+		goto err_config;
+
+	h = config_group_find_item(to_config_group(header), "h");
+	config_item_put(header);
+	if (!h)
+		goto err_config;
+
+	uvc->header = to_uvcg_streaming_header(h);
+	config_item_put(h);
+	if (!uvc->header->linked) {
+		mutex_unlock(&opts->lock);
+		kfree(uvc);
+		return ERR_PTR(-EBUSY);
+	}
+
 	++opts->refcnt;
 	mutex_unlock(&opts->lock);
 
@@ -992,6 +1017,11 @@ static struct usb_function *uvc_alloc(struct usb_function_instance *fi)
 	uvc->func.bind_deactivated = true;
 
 	return &uvc->func;
+
+err_config:
+	mutex_unlock(&opts->lock);
+	kfree(uvc);
+	return ERR_PTR(-ENOENT);
 }
 
 DECLARE_USB_FUNCTION_INIT(uvc, uvc_alloc_inst, uvc_alloc);
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
index 1a31e6c6a5ffb8..40226b1f7e148a 100644
--- a/drivers/usb/gadget/function/uvc.h
+++ b/drivers/usb/gadget/function/uvc.h
@@ -134,6 +134,8 @@ struct uvc_device {
 	bool func_connected;
 	wait_queue_head_t func_connected_queue;
 
+	struct uvcg_streaming_header *header;
+
 	/* Descriptors */
 	struct {
 		const struct uvc_descriptor_header * const *fs_control;
diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c
index d6dbf9b763b2e1..417655e4a83dd7 100644
--- a/drivers/usb/gadget/function/uvc_v4l2.c
+++ b/drivers/usb/gadget/function/uvc_v4l2.c
@@ -18,12 +18,92 @@
 #include <media/v4l2-dev.h>
 #include <media/v4l2-event.h>
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-uvc.h>
 
 #include "f_uvc.h"
 #include "uvc.h"
 #include "uvc_queue.h"
 #include "uvc_video.h"
 #include "uvc_v4l2.h"
+#include "uvc_configfs.h"
+
+static struct uvc_format_desc *to_uvc_format(struct uvcg_format *uformat)
+{
+	char guid[16] = UVC_GUID_FORMAT_MJPEG;
+	struct uvc_format_desc *format;
+	struct uvcg_uncompressed *unc;
+
+	if (uformat->type == UVCG_UNCOMPRESSED) {
+		unc = to_uvcg_uncompressed(&uformat->group.cg_item);
+		if (!unc)
+			return ERR_PTR(-EINVAL);
+
+		memcpy(guid, unc->desc.guidFormat, sizeof(guid));
+	}
+
+	format = uvc_format_by_guid(guid);
+	if (!format)
+		return ERR_PTR(-EINVAL);
+
+	return format;
+}
+
+static struct uvcg_format *find_format_by_index(struct uvc_device *uvc, int index)
+{
+	struct uvcg_format_ptr *format;
+	struct uvcg_format *uformat = NULL;
+	int i = 1;
+
+	list_for_each_entry(format, &uvc->header->formats, entry) {
+		if (index == i) {
+			uformat = format->fmt;
+			break;
+		}
+		i++;
+	}
+
+	return uformat;
+}
+
+static struct uvcg_frame *find_frame_by_index(struct uvc_device *uvc,
+				       struct uvcg_format *uformat,
+				       int index)
+{
+	struct uvcg_format_ptr *format;
+	struct uvcg_frame_ptr *frame;
+	struct uvcg_frame *uframe = NULL;
+
+	list_for_each_entry(format, &uvc->header->formats, entry) {
+		if (format->fmt->type != uformat->type)
+			continue;
+		list_for_each_entry(frame, &format->fmt->frames, entry) {
+			if (index == frame->frm->frame.b_frame_index) {
+				uframe = frame->frm;
+				break;
+			}
+		}
+	}
+
+	return uframe;
+}
+
+static struct uvcg_format *find_format_by_pix(struct uvc_device *uvc,
+					      u32 pixelformat)
+{
+	struct uvcg_format_ptr *format;
+	struct uvcg_format *uformat = NULL;
+
+	list_for_each_entry(format, &uvc->header->formats, entry) {
+		struct uvc_format_desc *fmtdesc = to_uvc_format(format->fmt);
+
+		if (fmtdesc->fcc == pixelformat) {
+			uformat = format->fmt;
+			break;
+		}
+	}
+
+	return uformat;
+}
 
 /* --------------------------------------------------------------------------
  * Requests handling
@@ -134,6 +214,99 @@ uvc_v4l2_set_format(struct file *file, void *fh, struct v4l2_format *fmt)
 	return 0;
 }
 
+static int
+uvc_v4l2_enum_frameintervals(struct file *file, void *fh,
+		struct v4l2_frmivalenum *fival)
+{
+	struct video_device *vdev = video_devdata(file);
+	struct uvc_device *uvc = video_get_drvdata(vdev);
+	struct uvcg_format *uformat = NULL;
+	struct uvcg_frame *uframe = NULL;
+	struct uvcg_frame_ptr *frame;
+
+	uformat = find_format_by_pix(uvc, fival->pixel_format);
+	if (!uformat)
+		return -EINVAL;
+
+	list_for_each_entry(frame, &uformat->frames, entry) {
+		if (frame->frm->frame.w_width == fival->width &&
+		    frame->frm->frame.w_height == fival->height) {
+			uframe = frame->frm;
+			break;
+		}
+	}
+	if (!uframe)
+		return -EINVAL;
+
+	if (fival->index >= uframe->frame.b_frame_interval_type)
+		return -EINVAL;
+
+	fival->discrete.numerator =
+		uframe->dw_frame_interval[fival->index];
+
+	/* TODO: handle V4L2_FRMIVAL_TYPE_STEPWISE */
+	fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
+	fival->discrete.denominator = 10000000;
+	v4l2_simplify_fraction(&fival->discrete.numerator,
+		&fival->discrete.denominator, 8, 333);
+
+	return 0;
+}
+
+static int
+uvc_v4l2_enum_framesizes(struct file *file, void *fh,
+		struct v4l2_frmsizeenum *fsize)
+{
+	struct video_device *vdev = video_devdata(file);
+	struct uvc_device *uvc = video_get_drvdata(vdev);
+	struct uvcg_format *uformat = NULL;
+	struct uvcg_frame *uframe = NULL;
+
+	uformat = find_format_by_pix(uvc, fsize->pixel_format);
+	if (!uformat)
+		return -EINVAL;
+
+	if (fsize->index >= uformat->num_frames)
+		return -EINVAL;
+
+	uframe = find_frame_by_index(uvc, uformat, fsize->index + 1);
+	if (!uframe)
+		return -EINVAL;
+
+	fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
+	fsize->discrete.width = uframe->frame.w_width;
+	fsize->discrete.height = uframe->frame.w_height;
+
+	return 0;
+}
+
+static int
+uvc_v4l2_enum_format(struct file *file, void *fh, struct v4l2_fmtdesc *f)
+{
+	struct video_device *vdev = video_devdata(file);
+	struct uvc_device *uvc = video_get_drvdata(vdev);
+	struct uvc_format_desc *fmtdesc;
+	struct uvcg_format *uformat;
+
+	if (f->index >= uvc->header->num_fmt)
+		return -EINVAL;
+
+	uformat = find_format_by_index(uvc, f->index + 1);
+	if (!uformat)
+		return -EINVAL;
+
+	if (uformat->type != UVCG_UNCOMPRESSED)
+		f->flags |= V4L2_FMT_FLAG_COMPRESSED;
+
+	fmtdesc = to_uvc_format(uformat);
+	f->pixelformat = fmtdesc->fcc;
+
+	strscpy(f->description, fmtdesc->name, sizeof(f->description));
+	f->description[strlen(fmtdesc->name) - 1] = 0;
+
+	return 0;
+}
+
 static int
 uvc_v4l2_reqbufs(struct file *file, void *fh, struct v4l2_requestbuffers *b)
 {
@@ -300,6 +473,9 @@ const struct v4l2_ioctl_ops uvc_v4l2_ioctl_ops = {
 	.vidioc_querycap = uvc_v4l2_querycap,
 	.vidioc_g_fmt_vid_out = uvc_v4l2_get_format,
 	.vidioc_s_fmt_vid_out = uvc_v4l2_set_format,
+	.vidioc_enum_frameintervals = uvc_v4l2_enum_frameintervals,
+	.vidioc_enum_framesizes = uvc_v4l2_enum_framesizes,
+	.vidioc_enum_fmt_vid_out = uvc_v4l2_enum_format,
 	.vidioc_reqbufs = uvc_v4l2_reqbufs,
 	.vidioc_querybuf = uvc_v4l2_querybuf,
 	.vidioc_qbuf = uvc_v4l2_qbuf,
-- 
2.30.2

[PATCH v2 4/4] usb: gadget: uvc: add v4l2 try_format api call

[Michael Grzeschik]

This patch adds the uvc_v4l2_try_format api call to validate
the setting of v4l2_format. It will fallback to the nearest
allowed framesize.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

---
v13 -> v1:
    - moved to this smaller patch series
v1  -> v2:
    - declaring all helper functions as static

 drivers/usb/gadget/function/uvc_v4l2.c | 110 +++++++++++++++++++++++++
 1 file changed, 110 insertions(+)

diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c
index 417655e4a83dd7..c4ed48d6b8a407 100644
--- a/drivers/usb/gadget/function/uvc_v4l2.c
+++ b/drivers/usb/gadget/function/uvc_v4l2.c
@@ -48,6 +48,31 @@ static struct uvc_format_desc *to_uvc_format(struct uvcg_format *uformat)
 	return format;
 }
 
+static int uvc_v4l2_get_bytesperline(struct uvcg_format *uformat,
+			      struct uvcg_frame *uframe)
+{
+	struct uvcg_uncompressed *u;
+
+	if (uformat->type == UVCG_UNCOMPRESSED) {
+		u = to_uvcg_uncompressed(&uformat->group.cg_item);
+		if (!u)
+			return 0;
+
+		return u->desc.bBitsPerPixel * uframe->frame.w_width / 8;
+	}
+
+	return 0;
+}
+
+static int uvc_get_frame_size(struct uvcg_format *uformat,
+		       struct uvcg_frame *uframe)
+{
+	unsigned int bpl = uvc_v4l2_get_bytesperline(uformat, uframe);
+
+	return bpl ? bpl * uframe->frame.w_height :
+		uframe->frame.dw_max_video_frame_buffer_size;
+}
+
 static struct uvcg_format *find_format_by_index(struct uvc_device *uvc, int index)
 {
 	struct uvcg_format_ptr *format;
@@ -105,6 +130,50 @@ static struct uvcg_format *find_format_by_pix(struct uvc_device *uvc,
 	return uformat;
 }
 
+static struct uvcg_frame *find_closest_frame_by_size(struct uvc_device *uvc,
+					   struct uvcg_format *uformat,
+					   u16 rw, u16 rh)
+{
+	struct uvc_video *video = &uvc->video;
+	struct uvcg_format_ptr *format;
+	struct uvcg_frame_ptr *frame;
+	struct uvcg_frame *uframe = NULL;
+	unsigned int d, maxd;
+
+	/* Find the closest image size. The distance between image sizes is
+	 * the size in pixels of the non-overlapping regions between the
+	 * requested size and the frame-specified size.
+	 */
+	maxd = (unsigned int)-1;
+
+	list_for_each_entry(format, &uvc->header->formats, entry) {
+		if (format->fmt->type != uformat->type)
+			continue;
+
+		list_for_each_entry(frame, &format->fmt->frames, entry) {
+			u16 w, h;
+
+			w = frame->frm->frame.w_width;
+			h = frame->frm->frame.w_height;
+
+			d = min(w, rw) * min(h, rh);
+			d = w*h + rw*rh - 2*d;
+			if (d < maxd) {
+				maxd = d;
+				uframe = frame->frm;
+			}
+
+			if (maxd == 0)
+				break;
+		}
+	}
+
+	if (!uframe)
+		uvcg_dbg(&video->uvc->func, "Unsupported size %ux%u\n", rw, rh);
+
+	return uframe;
+}
+
 /* --------------------------------------------------------------------------
  * Requests handling
  */
@@ -214,6 +283,46 @@ uvc_v4l2_set_format(struct file *file, void *fh, struct v4l2_format *fmt)
 	return 0;
 }
 
+static int
+uvc_v4l2_try_format(struct file *file, void *fh, struct v4l2_format *fmt)
+{
+	struct video_device *vdev = video_devdata(file);
+	struct uvc_device *uvc = video_get_drvdata(vdev);
+	struct uvc_video *video = &uvc->video;
+	struct uvcg_format *uformat;
+	struct uvcg_frame *uframe;
+	u8 *fcc;
+
+	if (fmt->type != video->queue.queue.type)
+		return -EINVAL;
+
+	fcc = (u8 *)&fmt->fmt.pix.pixelformat;
+	uvcg_dbg(&uvc->func, "Trying format 0x%08x (%c%c%c%c): %ux%u\n",
+		fmt->fmt.pix.pixelformat,
+		fcc[0], fcc[1], fcc[2], fcc[3],
+		fmt->fmt.pix.width, fmt->fmt.pix.height);
+
+	uformat = find_format_by_pix(uvc, fmt->fmt.pix.pixelformat);
+	if (!uformat)
+		return -EINVAL;
+
+	uframe = find_closest_frame_by_size(uvc, uformat,
+				fmt->fmt.pix.width, fmt->fmt.pix.height);
+	if (!uframe)
+		return -EINVAL;
+
+	fmt->fmt.pix.width = uframe->frame.w_width;
+	fmt->fmt.pix.height = uframe->frame.w_height;
+	fmt->fmt.pix.field = V4L2_FIELD_NONE;
+	fmt->fmt.pix.bytesperline = uvc_v4l2_get_bytesperline(uformat, uframe);
+	fmt->fmt.pix.sizeimage = uvc_get_frame_size(uformat, uframe);
+	fmt->fmt.pix.pixelformat = to_uvc_format(uformat)->fcc;
+	fmt->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
+	fmt->fmt.pix.priv = 0;
+
+	return 0;
+}
+
 static int
 uvc_v4l2_enum_frameintervals(struct file *file, void *fh,
 		struct v4l2_frmivalenum *fival)
@@ -471,6 +580,7 @@ uvc_v4l2_ioctl_default(struct file *file, void *fh, bool valid_prio,
 
 const struct v4l2_ioctl_ops uvc_v4l2_ioctl_ops = {
 	.vidioc_querycap = uvc_v4l2_querycap,
+	.vidioc_try_fmt_vid_out = uvc_v4l2_try_format,
 	.vidioc_g_fmt_vid_out = uvc_v4l2_get_format,
 	.vidioc_s_fmt_vid_out = uvc_v4l2_set_format,
 	.vidioc_enum_frameintervals = uvc_v4l2_enum_frameintervals,
-- 
2.30.2

Re: [PATCH v2 2/4] media: uvcvideo: move uvc_format_desc to common header

[Laurent Pinchart]

Hi Michael,

I've just realized that this patch got merged. There's an issue, please
see below.

On Sat, Sep 10, 2022 at 12:13:33AM +0200, Michael Grzeschik wrote:
> The uvc_format_desc, GUID defines and the uvc_format_by_guid helper is
> also useful for the uvc gadget stack. This patch moves them to a common
> header.
> 
> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
> Tested-by: Daniel Scally <dan.scally@ideasonboard.com>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> 
> ---
> v13 -> v1:
>     - added Reviewed-by and Tested-by from Dan
>     - moved to this smaller patch series
> v1 -> v2:
>     -
> 
>  drivers/media/usb/uvc/uvc_ctrl.c   |   1 +
>  drivers/media/usb/uvc/uvc_driver.c | 206 +----------------
>  drivers/media/usb/uvc/uvcvideo.h   | 144 ------------
>  include/media/v4l2-uvc.h           | 359 +++++++++++++++++++++++++++++
>  4 files changed, 361 insertions(+), 349 deletions(-)
>  create mode 100644 include/media/v4l2-uvc.h
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 8c208db9600b46..b8a00a51679f6d 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -18,6 +18,7 @@
>  #include <linux/workqueue.h>
>  #include <linux/atomic.h>
>  #include <media/v4l2-ctrls.h>
> +#include <media/v4l2-uvc.h>
>  
>  #include "uvcvideo.h"
>  
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 0f14dee4b6d794..2891bc9d319280 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -20,6 +20,7 @@
>  
>  #include <media/v4l2-common.h>
>  #include <media/v4l2-ioctl.h>
> +#include <media/v4l2-uvc.h>
>  
>  #include "uvcvideo.h"
>  
> @@ -34,198 +35,6 @@ static unsigned int uvc_quirks_param = -1;
>  unsigned int uvc_dbg_param;
>  unsigned int uvc_timeout_param = UVC_CTRL_STREAMING_TIMEOUT;
>  
> -/* ------------------------------------------------------------------------
> - * Video formats
> - */
> -
> -static struct uvc_format_desc uvc_fmts[] = {
> -	{
> -		.name		= "YUV 4:2:2 (YUYV)",
> -		.guid		= UVC_GUID_FORMAT_YUY2,
> -		.fcc		= V4L2_PIX_FMT_YUYV,
> -	},
> -	{
> -		.name		= "YUV 4:2:2 (YUYV)",
> -		.guid		= UVC_GUID_FORMAT_YUY2_ISIGHT,
> -		.fcc		= V4L2_PIX_FMT_YUYV,
> -	},
> -	{
> -		.name		= "YUV 4:2:0 (NV12)",
> -		.guid		= UVC_GUID_FORMAT_NV12,
> -		.fcc		= V4L2_PIX_FMT_NV12,
> -	},
> -	{
> -		.name		= "MJPEG",
> -		.guid		= UVC_GUID_FORMAT_MJPEG,
> -		.fcc		= V4L2_PIX_FMT_MJPEG,
> -	},
> -	{
> -		.name		= "YVU 4:2:0 (YV12)",
> -		.guid		= UVC_GUID_FORMAT_YV12,
> -		.fcc		= V4L2_PIX_FMT_YVU420,
> -	},
> -	{
> -		.name		= "YUV 4:2:0 (I420)",
> -		.guid		= UVC_GUID_FORMAT_I420,
> -		.fcc		= V4L2_PIX_FMT_YUV420,
> -	},
> -	{
> -		.name		= "YUV 4:2:0 (M420)",
> -		.guid		= UVC_GUID_FORMAT_M420,
> -		.fcc		= V4L2_PIX_FMT_M420,
> -	},
> -	{
> -		.name		= "YUV 4:2:2 (UYVY)",
> -		.guid		= UVC_GUID_FORMAT_UYVY,
> -		.fcc		= V4L2_PIX_FMT_UYVY,
> -	},
> -	{
> -		.name		= "Greyscale 8-bit (Y800)",
> -		.guid		= UVC_GUID_FORMAT_Y800,
> -		.fcc		= V4L2_PIX_FMT_GREY,
> -	},
> -	{
> -		.name		= "Greyscale 8-bit (Y8  )",
> -		.guid		= UVC_GUID_FORMAT_Y8,
> -		.fcc		= V4L2_PIX_FMT_GREY,
> -	},
> -	{
> -		.name		= "Greyscale 8-bit (D3DFMT_L8)",
> -		.guid		= UVC_GUID_FORMAT_D3DFMT_L8,
> -		.fcc		= V4L2_PIX_FMT_GREY,
> -	},
> -	{
> -		.name		= "IR 8-bit (L8_IR)",
> -		.guid		= UVC_GUID_FORMAT_KSMEDIA_L8_IR,
> -		.fcc		= V4L2_PIX_FMT_GREY,
> -	},
> -	{
> -		.name		= "Greyscale 10-bit (Y10 )",
> -		.guid		= UVC_GUID_FORMAT_Y10,
> -		.fcc		= V4L2_PIX_FMT_Y10,
> -	},
> -	{
> -		.name		= "Greyscale 12-bit (Y12 )",
> -		.guid		= UVC_GUID_FORMAT_Y12,
> -		.fcc		= V4L2_PIX_FMT_Y12,
> -	},
> -	{
> -		.name		= "Greyscale 16-bit (Y16 )",
> -		.guid		= UVC_GUID_FORMAT_Y16,
> -		.fcc		= V4L2_PIX_FMT_Y16,
> -	},
> -	{
> -		.name		= "BGGR Bayer (BY8 )",
> -		.guid		= UVC_GUID_FORMAT_BY8,
> -		.fcc		= V4L2_PIX_FMT_SBGGR8,
> -	},
> -	{
> -		.name		= "BGGR Bayer (BA81)",
> -		.guid		= UVC_GUID_FORMAT_BA81,
> -		.fcc		= V4L2_PIX_FMT_SBGGR8,
> -	},
> -	{
> -		.name		= "GBRG Bayer (GBRG)",
> -		.guid		= UVC_GUID_FORMAT_GBRG,
> -		.fcc		= V4L2_PIX_FMT_SGBRG8,
> -	},
> -	{
> -		.name		= "GRBG Bayer (GRBG)",
> -		.guid		= UVC_GUID_FORMAT_GRBG,
> -		.fcc		= V4L2_PIX_FMT_SGRBG8,
> -	},
> -	{
> -		.name		= "RGGB Bayer (RGGB)",
> -		.guid		= UVC_GUID_FORMAT_RGGB,
> -		.fcc		= V4L2_PIX_FMT_SRGGB8,
> -	},
> -	{
> -		.name		= "RGB565",
> -		.guid		= UVC_GUID_FORMAT_RGBP,
> -		.fcc		= V4L2_PIX_FMT_RGB565,
> -	},
> -	{
> -		.name		= "BGR 8:8:8 (BGR3)",
> -		.guid		= UVC_GUID_FORMAT_BGR3,
> -		.fcc		= V4L2_PIX_FMT_BGR24,
> -	},
> -	{
> -		.name		= "H.264",
> -		.guid		= UVC_GUID_FORMAT_H264,
> -		.fcc		= V4L2_PIX_FMT_H264,
> -	},
> -	{
> -		.name		= "H.265",
> -		.guid		= UVC_GUID_FORMAT_H265,
> -		.fcc		= V4L2_PIX_FMT_HEVC,
> -	},
> -	{
> -		.name		= "Greyscale 8 L/R (Y8I)",
> -		.guid		= UVC_GUID_FORMAT_Y8I,
> -		.fcc		= V4L2_PIX_FMT_Y8I,
> -	},
> -	{
> -		.name		= "Greyscale 12 L/R (Y12I)",
> -		.guid		= UVC_GUID_FORMAT_Y12I,
> -		.fcc		= V4L2_PIX_FMT_Y12I,
> -	},
> -	{
> -		.name		= "Depth data 16-bit (Z16)",
> -		.guid		= UVC_GUID_FORMAT_Z16,
> -		.fcc		= V4L2_PIX_FMT_Z16,
> -	},
> -	{
> -		.name		= "Bayer 10-bit (SRGGB10P)",
> -		.guid		= UVC_GUID_FORMAT_RW10,
> -		.fcc		= V4L2_PIX_FMT_SRGGB10P,
> -	},
> -	{
> -		.name		= "Bayer 16-bit (SBGGR16)",
> -		.guid		= UVC_GUID_FORMAT_BG16,
> -		.fcc		= V4L2_PIX_FMT_SBGGR16,
> -	},
> -	{
> -		.name		= "Bayer 16-bit (SGBRG16)",
> -		.guid		= UVC_GUID_FORMAT_GB16,
> -		.fcc		= V4L2_PIX_FMT_SGBRG16,
> -	},
> -	{
> -		.name		= "Bayer 16-bit (SRGGB16)",
> -		.guid		= UVC_GUID_FORMAT_RG16,
> -		.fcc		= V4L2_PIX_FMT_SRGGB16,
> -	},
> -	{
> -		.name		= "Bayer 16-bit (SGRBG16)",
> -		.guid		= UVC_GUID_FORMAT_GR16,
> -		.fcc		= V4L2_PIX_FMT_SGRBG16,
> -	},
> -	{
> -		.name		= "Depth data 16-bit (Z16)",
> -		.guid		= UVC_GUID_FORMAT_INVZ,
> -		.fcc		= V4L2_PIX_FMT_Z16,
> -	},
> -	{
> -		.name		= "Greyscale 10-bit (Y10 )",
> -		.guid		= UVC_GUID_FORMAT_INVI,
> -		.fcc		= V4L2_PIX_FMT_Y10,
> -	},
> -	{
> -		.name		= "IR:Depth 26-bit (INZI)",
> -		.guid		= UVC_GUID_FORMAT_INZI,
> -		.fcc		= V4L2_PIX_FMT_INZI,
> -	},
> -	{
> -		.name		= "4-bit Depth Confidence (Packed)",
> -		.guid		= UVC_GUID_FORMAT_CNF4,
> -		.fcc		= V4L2_PIX_FMT_CNF4,
> -	},
> -	{
> -		.name		= "HEVC",
> -		.guid		= UVC_GUID_FORMAT_HEVC,
> -		.fcc		= V4L2_PIX_FMT_HEVC,
> -	},
> -};
> -
>  /* ------------------------------------------------------------------------
>   * Utility functions
>   */
> @@ -245,19 +54,6 @@ struct usb_host_endpoint *uvc_find_endpoint(struct usb_host_interface *alts,
>  	return NULL;
>  }
>  
> -static struct uvc_format_desc *uvc_format_by_guid(const u8 guid[16])
> -{
> -	unsigned int len = ARRAY_SIZE(uvc_fmts);
> -	unsigned int i;
> -
> -	for (i = 0; i < len; ++i) {
> -		if (memcmp(guid, uvc_fmts[i].guid, 16) == 0)
> -			return &uvc_fmts[i];
> -	}
> -
> -	return NULL;
> -}
> -
>  static enum v4l2_colorspace uvc_colorspace(const u8 primaries)
>  {
>  	static const enum v4l2_colorspace colorprimaries[] = {
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index ff710bdd38b3fd..df93db259312e7 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -41,144 +41,6 @@
>  #define UVC_EXT_GPIO_UNIT		0x7ffe
>  #define UVC_EXT_GPIO_UNIT_ID		0x100
>  
> -/* ------------------------------------------------------------------------
> - * GUIDs
> - */
> -#define UVC_GUID_UVC_CAMERA \
> -	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> -	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}
> -#define UVC_GUID_UVC_OUTPUT \
> -	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> -	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02}
> -#define UVC_GUID_UVC_MEDIA_TRANSPORT_INPUT \
> -	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> -	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03}
> -#define UVC_GUID_UVC_PROCESSING \
> -	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> -	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x01}
> -#define UVC_GUID_UVC_SELECTOR \
> -	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> -	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x02}
> -#define UVC_GUID_EXT_GPIO_CONTROLLER \
> -	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> -	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03}
> -
> -#define UVC_GUID_FORMAT_MJPEG \
> -	{ 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_YUY2 \
> -	{ 'Y',  'U',  'Y',  '2', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_YUY2_ISIGHT \
> -	{ 'Y',  'U',  'Y',  '2', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0x00, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_NV12 \
> -	{ 'N',  'V',  '1',  '2', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_YV12 \
> -	{ 'Y',  'V',  '1',  '2', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_I420 \
> -	{ 'I',  '4',  '2',  '0', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_UYVY \
> -	{ 'U',  'Y',  'V',  'Y', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_Y800 \
> -	{ 'Y',  '8',  '0',  '0', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_Y8 \
> -	{ 'Y',  '8',  ' ',  ' ', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_Y10 \
> -	{ 'Y',  '1',  '0',  ' ', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_Y12 \
> -	{ 'Y',  '1',  '2',  ' ', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_Y16 \
> -	{ 'Y',  '1',  '6',  ' ', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_BY8 \
> -	{ 'B',  'Y',  '8',  ' ', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_BA81 \
> -	{ 'B',  'A',  '8',  '1', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_GBRG \
> -	{ 'G',  'B',  'R',  'G', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_GRBG \
> -	{ 'G',  'R',  'B',  'G', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_RGGB \
> -	{ 'R',  'G',  'G',  'B', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_BG16 \
> -	{ 'B',  'G',  '1',  '6', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_GB16 \
> -	{ 'G',  'B',  '1',  '6', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_RG16 \
> -	{ 'R',  'G',  '1',  '6', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_GR16 \
> -	{ 'G',  'R',  '1',  '6', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_RGBP \
> -	{ 'R',  'G',  'B',  'P', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_BGR3 \
> -	{ 0x7d, 0xeb, 0x36, 0xe4, 0x4f, 0x52, 0xce, 0x11, \
> -	 0x9f, 0x53, 0x00, 0x20, 0xaf, 0x0b, 0xa7, 0x70}
> -#define UVC_GUID_FORMAT_M420 \
> -	{ 'M',  '4',  '2',  '0', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -
> -#define UVC_GUID_FORMAT_H264 \
> -	{ 'H',  '2',  '6',  '4', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_H265 \
> -	{ 'H',  '2',  '6',  '5', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_Y8I \
> -	{ 'Y',  '8',  'I',  ' ', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_Y12I \
> -	{ 'Y',  '1',  '2',  'I', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_Z16 \
> -	{ 'Z',  '1',  '6',  ' ', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_RW10 \
> -	{ 'R',  'W',  '1',  '0', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_INVZ \
> -	{ 'I',  'N',  'V',  'Z', 0x90, 0x2d, 0x58, 0x4a, \
> -	 0x92, 0x0b, 0x77, 0x3f, 0x1f, 0x2c, 0x55, 0x6b}
> -#define UVC_GUID_FORMAT_INZI \
> -	{ 'I',  'N',  'Z',  'I', 0x66, 0x1a, 0x42, 0xa2, \
> -	 0x90, 0x65, 0xd0, 0x18, 0x14, 0xa8, 0xef, 0x8a}
> -#define UVC_GUID_FORMAT_INVI \
> -	{ 'I',  'N',  'V',  'I', 0xdb, 0x57, 0x49, 0x5e, \
> -	 0x8e, 0x3f, 0xf4, 0x79, 0x53, 0x2b, 0x94, 0x6f}
> -#define UVC_GUID_FORMAT_CNF4 \
> -	{ 'C',  ' ',  ' ',  ' ', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -
> -#define UVC_GUID_FORMAT_D3DFMT_L8 \
> -	{0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -#define UVC_GUID_FORMAT_KSMEDIA_L8_IR \
> -	{0x32, 0x00, 0x00, 0x00, 0x02, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -
> -#define UVC_GUID_FORMAT_HEVC \
> -	{ 'H',  'E',  'V',  'C', 0x00, 0x00, 0x10, 0x00, \
> -	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> -
> -
>  /* ------------------------------------------------------------------------
>   * Driver specific constants.
>   */
> @@ -283,12 +145,6 @@ struct uvc_control {
>  	struct uvc_fh *handle;	/* File handle that last changed the control. */
>  };
>  
> -struct uvc_format_desc {
> -	char *name;
> -	u8 guid[16];
> -	u32 fcc;
> -};
> -
>  /*
>   * The term 'entity' refers to both UVC units and UVC terminals.
>   *
> diff --git a/include/media/v4l2-uvc.h b/include/media/v4l2-uvc.h
> new file mode 100644
> index 00000000000000..f83e31661333bb
> --- /dev/null
> +++ b/include/media/v4l2-uvc.h
> @@ -0,0 +1,359 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + *  v4l2 uvc internal API header
> + *
> + *  Some commonly needed functions for uvc drivers
> + */
> +
> +#ifndef __LINUX_V4L2_UVC_H
> +#define __LINUX_V4L2_UVC_H
> +
> +/* ------------------------------------------------------------------------
> + * GUIDs
> + */
> +#define UVC_GUID_UVC_CAMERA \
> +	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}
> +#define UVC_GUID_UVC_OUTPUT \
> +	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02}
> +#define UVC_GUID_UVC_MEDIA_TRANSPORT_INPUT \
> +	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03}
> +#define UVC_GUID_UVC_PROCESSING \
> +	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x01}
> +#define UVC_GUID_UVC_SELECTOR \
> +	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x02}
> +#define UVC_GUID_EXT_GPIO_CONTROLLER \
> +	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> +	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03}
> +
> +#define UVC_GUID_FORMAT_MJPEG \
> +	{ 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_YUY2 \
> +	{ 'Y',  'U',  'Y',  '2', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_YUY2_ISIGHT \
> +	{ 'Y',  'U',  'Y',  '2', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0x00, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_NV12 \
> +	{ 'N',  'V',  '1',  '2', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_YV12 \
> +	{ 'Y',  'V',  '1',  '2', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_I420 \
> +	{ 'I',  '4',  '2',  '0', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_UYVY \
> +	{ 'U',  'Y',  'V',  'Y', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_Y800 \
> +	{ 'Y',  '8',  '0',  '0', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_Y8 \
> +	{ 'Y',  '8',  ' ',  ' ', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_Y10 \
> +	{ 'Y',  '1',  '0',  ' ', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_Y12 \
> +	{ 'Y',  '1',  '2',  ' ', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_Y16 \
> +	{ 'Y',  '1',  '6',  ' ', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_BY8 \
> +	{ 'B',  'Y',  '8',  ' ', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_BA81 \
> +	{ 'B',  'A',  '8',  '1', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_GBRG \
> +	{ 'G',  'B',  'R',  'G', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_GRBG \
> +	{ 'G',  'R',  'B',  'G', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_RGGB \
> +	{ 'R',  'G',  'G',  'B', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_BG16 \
> +	{ 'B',  'G',  '1',  '6', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_GB16 \
> +	{ 'G',  'B',  '1',  '6', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_RG16 \
> +	{ 'R',  'G',  '1',  '6', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_GR16 \
> +	{ 'G',  'R',  '1',  '6', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_RGBP \
> +	{ 'R',  'G',  'B',  'P', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_BGR3 \
> +	{ 0x7d, 0xeb, 0x36, 0xe4, 0x4f, 0x52, 0xce, 0x11, \
> +	 0x9f, 0x53, 0x00, 0x20, 0xaf, 0x0b, 0xa7, 0x70}
> +#define UVC_GUID_FORMAT_M420 \
> +	{ 'M',  '4',  '2',  '0', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +
> +#define UVC_GUID_FORMAT_H264 \
> +	{ 'H',  '2',  '6',  '4', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_H265 \
> +	{ 'H',  '2',  '6',  '5', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_Y8I \
> +	{ 'Y',  '8',  'I',  ' ', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_Y12I \
> +	{ 'Y',  '1',  '2',  'I', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_Z16 \
> +	{ 'Z',  '1',  '6',  ' ', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_RW10 \
> +	{ 'R',  'W',  '1',  '0', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_INVZ \
> +	{ 'I',  'N',  'V',  'Z', 0x90, 0x2d, 0x58, 0x4a, \
> +	 0x92, 0x0b, 0x77, 0x3f, 0x1f, 0x2c, 0x55, 0x6b}
> +#define UVC_GUID_FORMAT_INZI \
> +	{ 'I',  'N',  'Z',  'I', 0x66, 0x1a, 0x42, 0xa2, \
> +	 0x90, 0x65, 0xd0, 0x18, 0x14, 0xa8, 0xef, 0x8a}
> +#define UVC_GUID_FORMAT_INVI \
> +	{ 'I',  'N',  'V',  'I', 0xdb, 0x57, 0x49, 0x5e, \
> +	 0x8e, 0x3f, 0xf4, 0x79, 0x53, 0x2b, 0x94, 0x6f}
> +#define UVC_GUID_FORMAT_CNF4 \
> +	{ 'C',  ' ',  ' ',  ' ', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +
> +#define UVC_GUID_FORMAT_D3DFMT_L8 \
> +	{0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_KSMEDIA_L8_IR \
> +	{0x32, 0x00, 0x00, 0x00, 0x02, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +
> +#define UVC_GUID_FORMAT_HEVC \
> +	{ 'H',  'E',  'V',  'C', 0x00, 0x00, 0x10, 0x00, \
> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +
> +/* ------------------------------------------------------------------------
> + * Video formats
> + */
> +
> +struct uvc_format_desc {
> +	char *name;
> +	u8 guid[16];
> +	u32 fcc;
> +};
> +
> +static struct uvc_format_desc uvc_fmts[] = {
> +	{
> +		.name		= "YUV 4:2:2 (YUYV)",
> +		.guid		= UVC_GUID_FORMAT_YUY2,
> +		.fcc		= V4L2_PIX_FMT_YUYV,
> +	},
> +	{
> +		.name		= "YUV 4:2:2 (YUYV)",
> +		.guid		= UVC_GUID_FORMAT_YUY2_ISIGHT,
> +		.fcc		= V4L2_PIX_FMT_YUYV,
> +	},
> +	{
> +		.name		= "YUV 4:2:0 (NV12)",
> +		.guid		= UVC_GUID_FORMAT_NV12,
> +		.fcc		= V4L2_PIX_FMT_NV12,
> +	},
> +	{
> +		.name		= "MJPEG",
> +		.guid		= UVC_GUID_FORMAT_MJPEG,
> +		.fcc		= V4L2_PIX_FMT_MJPEG,
> +	},
> +	{
> +		.name		= "YVU 4:2:0 (YV12)",
> +		.guid		= UVC_GUID_FORMAT_YV12,
> +		.fcc		= V4L2_PIX_FMT_YVU420,
> +	},
> +	{
> +		.name		= "YUV 4:2:0 (I420)",
> +		.guid		= UVC_GUID_FORMAT_I420,
> +		.fcc		= V4L2_PIX_FMT_YUV420,
> +	},
> +	{
> +		.name		= "YUV 4:2:0 (M420)",
> +		.guid		= UVC_GUID_FORMAT_M420,
> +		.fcc		= V4L2_PIX_FMT_M420,
> +	},
> +	{
> +		.name		= "YUV 4:2:2 (UYVY)",
> +		.guid		= UVC_GUID_FORMAT_UYVY,
> +		.fcc		= V4L2_PIX_FMT_UYVY,
> +	},
> +	{
> +		.name		= "Greyscale 8-bit (Y800)",
> +		.guid		= UVC_GUID_FORMAT_Y800,
> +		.fcc		= V4L2_PIX_FMT_GREY,
> +	},
> +	{
> +		.name		= "Greyscale 8-bit (Y8  )",
> +		.guid		= UVC_GUID_FORMAT_Y8,
> +		.fcc		= V4L2_PIX_FMT_GREY,
> +	},
> +	{
> +		.name		= "Greyscale 8-bit (D3DFMT_L8)",
> +		.guid		= UVC_GUID_FORMAT_D3DFMT_L8,
> +		.fcc		= V4L2_PIX_FMT_GREY,
> +	},
> +	{
> +		.name		= "IR 8-bit (L8_IR)",
> +		.guid		= UVC_GUID_FORMAT_KSMEDIA_L8_IR,
> +		.fcc		= V4L2_PIX_FMT_GREY,
> +	},
> +	{
> +		.name		= "Greyscale 10-bit (Y10 )",
> +		.guid		= UVC_GUID_FORMAT_Y10,
> +		.fcc		= V4L2_PIX_FMT_Y10,
> +	},
> +	{
> +		.name		= "Greyscale 12-bit (Y12 )",
> +		.guid		= UVC_GUID_FORMAT_Y12,
> +		.fcc		= V4L2_PIX_FMT_Y12,
> +	},
> +	{
> +		.name		= "Greyscale 16-bit (Y16 )",
> +		.guid		= UVC_GUID_FORMAT_Y16,
> +		.fcc		= V4L2_PIX_FMT_Y16,
> +	},
> +	{
> +		.name		= "BGGR Bayer (BY8 )",
> +		.guid		= UVC_GUID_FORMAT_BY8,
> +		.fcc		= V4L2_PIX_FMT_SBGGR8,
> +	},
> +	{
> +		.name		= "BGGR Bayer (BA81)",
> +		.guid		= UVC_GUID_FORMAT_BA81,
> +		.fcc		= V4L2_PIX_FMT_SBGGR8,
> +	},
> +	{
> +		.name		= "GBRG Bayer (GBRG)",
> +		.guid		= UVC_GUID_FORMAT_GBRG,
> +		.fcc		= V4L2_PIX_FMT_SGBRG8,
> +	},
> +	{
> +		.name		= "GRBG Bayer (GRBG)",
> +		.guid		= UVC_GUID_FORMAT_GRBG,
> +		.fcc		= V4L2_PIX_FMT_SGRBG8,
> +	},
> +	{
> +		.name		= "RGGB Bayer (RGGB)",
> +		.guid		= UVC_GUID_FORMAT_RGGB,
> +		.fcc		= V4L2_PIX_FMT_SRGGB8,
> +	},
> +	{
> +		.name		= "RGB565",
> +		.guid		= UVC_GUID_FORMAT_RGBP,
> +		.fcc		= V4L2_PIX_FMT_RGB565,
> +	},
> +	{
> +		.name		= "BGR 8:8:8 (BGR3)",
> +		.guid		= UVC_GUID_FORMAT_BGR3,
> +		.fcc		= V4L2_PIX_FMT_BGR24,
> +	},
> +	{
> +		.name		= "H.264",
> +		.guid		= UVC_GUID_FORMAT_H264,
> +		.fcc		= V4L2_PIX_FMT_H264,
> +	},
> +	{
> +		.name		= "H.265",
> +		.guid		= UVC_GUID_FORMAT_H265,
> +		.fcc		= V4L2_PIX_FMT_HEVC,
> +	},
> +	{
> +		.name		= "Greyscale 8 L/R (Y8I)",
> +		.guid		= UVC_GUID_FORMAT_Y8I,
> +		.fcc		= V4L2_PIX_FMT_Y8I,
> +	},
> +	{
> +		.name		= "Greyscale 12 L/R (Y12I)",
> +		.guid		= UVC_GUID_FORMAT_Y12I,
> +		.fcc		= V4L2_PIX_FMT_Y12I,
> +	},
> +	{
> +		.name		= "Depth data 16-bit (Z16)",
> +		.guid		= UVC_GUID_FORMAT_Z16,
> +		.fcc		= V4L2_PIX_FMT_Z16,
> +	},
> +	{
> +		.name		= "Bayer 10-bit (SRGGB10P)",
> +		.guid		= UVC_GUID_FORMAT_RW10,
> +		.fcc		= V4L2_PIX_FMT_SRGGB10P,
> +	},
> +	{
> +		.name		= "Bayer 16-bit (SBGGR16)",
> +		.guid		= UVC_GUID_FORMAT_BG16,
> +		.fcc		= V4L2_PIX_FMT_SBGGR16,
> +	},
> +	{
> +		.name		= "Bayer 16-bit (SGBRG16)",
> +		.guid		= UVC_GUID_FORMAT_GB16,
> +		.fcc		= V4L2_PIX_FMT_SGBRG16,
> +	},
> +	{
> +		.name		= "Bayer 16-bit (SRGGB16)",
> +		.guid		= UVC_GUID_FORMAT_RG16,
> +		.fcc		= V4L2_PIX_FMT_SRGGB16,
> +	},
> +	{
> +		.name		= "Bayer 16-bit (SGRBG16)",
> +		.guid		= UVC_GUID_FORMAT_GR16,
> +		.fcc		= V4L2_PIX_FMT_SGRBG16,
> +	},
> +	{
> +		.name		= "Depth data 16-bit (Z16)",
> +		.guid		= UVC_GUID_FORMAT_INVZ,
> +		.fcc		= V4L2_PIX_FMT_Z16,
> +	},
> +	{
> +		.name		= "Greyscale 10-bit (Y10 )",
> +		.guid		= UVC_GUID_FORMAT_INVI,
> +		.fcc		= V4L2_PIX_FMT_Y10,
> +	},
> +	{
> +		.name		= "IR:Depth 26-bit (INZI)",
> +		.guid		= UVC_GUID_FORMAT_INZI,
> +		.fcc		= V4L2_PIX_FMT_INZI,
> +	},
> +	{
> +		.name		= "4-bit Depth Confidence (Packed)",
> +		.guid		= UVC_GUID_FORMAT_CNF4,
> +		.fcc		= V4L2_PIX_FMT_CNF4,
> +	},
> +	{
> +		.name		= "HEVC",
> +		.guid		= UVC_GUID_FORMAT_HEVC,
> +		.fcc		= V4L2_PIX_FMT_HEVC,
> +	},
> +};

This large table is now duplicated in all compilation units in which
this header is included. That's not good. Could you please send a patch
to fix that ?

> +
> +static inline struct uvc_format_desc *uvc_format_by_guid(const u8 guid[16])
> +{
> +	unsigned int len = ARRAY_SIZE(uvc_fmts);
> +	unsigned int i;
> +
> +	for (i = 0; i < len; ++i) {
> +		if (memcmp(guid, uvc_fmts[i].guid, 16) == 0)
> +			return &uvc_fmts[i];
> +	}
> +
> +	return NULL;
> +}

While at it, please also de-duplicate this function.

> +
> +#endif /* __LINUX_V4L2_UVC_H */

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[Laurent Pinchart]

Hi Michael,

On Sat, Sep 10, 2022 at 12:13:31AM +0200, Michael Grzeschik wrote:
> This series improves the uvc video gadget by parsing the configfs
> entries. With the configfs data, the userspace now is able to use simple
> v4l2 api calls like enum and try_format to check for valid configurations
> initially set by configfs.

I've realized that this whole series got merged, despite my multiple
attempts to explain why I think it's not a good idea. The UVC gadget
requires userspace support, and there's no point in trying to move all
these things to the kernel side. It only bloats the kernel, makes the
code more complex, more difficult to maintain, and will make UVC 1.5
support more difficult.

I'm fairly unhappy with this, it will lower my trust towards your
patches.

> Michael Grzeschik (4):
>   media: v4l: move helper functions for fractions from uvc to
>     v4l2-common
>   media: uvcvideo: move uvc_format_desc to common header
>   usb: gadget: uvc: add v4l2 enumeration api calls
>   usb: gadget: uvc: add v4l2 try_format api call
> 
>  drivers/media/usb/uvc/uvc_ctrl.c       |   1 +
>  drivers/media/usb/uvc/uvc_driver.c     | 290 +-------------------
>  drivers/media/usb/uvc/uvc_v4l2.c       |  14 +-
>  drivers/media/usb/uvc/uvcvideo.h       | 147 ----------
>  drivers/media/v4l2-core/v4l2-common.c  |  86 ++++++
>  drivers/usb/gadget/function/f_uvc.c    |  30 +++
>  drivers/usb/gadget/function/uvc.h      |   2 +
>  drivers/usb/gadget/function/uvc_v4l2.c | 286 ++++++++++++++++++++
>  include/media/v4l2-common.h            |   4 +
>  include/media/v4l2-uvc.h               | 359 +++++++++++++++++++++++++
>  10 files changed, 776 insertions(+), 443 deletions(-)
>  create mode 100644 include/media/v4l2-uvc.h

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[Michael Grzeschik]

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

Hi Laurent,

On Sat, Dec 03, 2022 at 11:26:14PM +0200, Laurent Pinchart wrote:
>On Sat, Sep 10, 2022 at 12:13:31AM +0200, Michael Grzeschik wrote:
>> This series improves the uvc video gadget by parsing the configfs
>> entries. With the configfs data, the userspace now is able to use simple
>> v4l2 api calls like enum and try_format to check for valid configurations
>> initially set by configfs.
>
>I've realized that this whole series got merged, despite my multiple
>attempts to explain why I think it's not a good idea. The UVC gadget
>requires userspace support, and there's no point in trying to move all
>these things to the kernel side. It only bloats the kernel, makes the
>code more complex, more difficult to maintain, and will make UVC 1.5
>support more difficult.

Those patches that got merged are already a compromise. And I think a
better one. Since the last rounds I realized that many steps that I
thought would be needed in the kernel, can indeed be made in userspace.
So beside thise code that only adds the typical vidioc functions to
parse what is configured in configfs. There is nothing that changed the
working function of the gadget. That said, it is up to you to use the
vidiocs or your application sticks with own parsing of the configfs.

So, with that said, I am unsure what you are exactly unhappy about.
Beside the points you mentioned in the previous mail.

>I'm fairly unhappy with this, it will lower my trust towards your
>patches.

If you don't trust my patches, than review them or at least nack them
with a comment so we have an object of disscussion.

>> Michael Grzeschik (4):
>>   media: v4l: move helper functions for fractions from uvc to
>>     v4l2-common
>>   media: uvcvideo: move uvc_format_desc to common header
>>   usb: gadget: uvc: add v4l2 enumeration api calls
>>   usb: gadget: uvc: add v4l2 try_format api call
>>
>>  drivers/media/usb/uvc/uvc_ctrl.c       |   1 +
>>  drivers/media/usb/uvc/uvc_driver.c     | 290 +-------------------
>>  drivers/media/usb/uvc/uvc_v4l2.c       |  14 +-
>>  drivers/media/usb/uvc/uvcvideo.h       | 147 ----------
>>  drivers/media/v4l2-core/v4l2-common.c  |  86 ++++++
>>  drivers/usb/gadget/function/f_uvc.c    |  30 +++
>>  drivers/usb/gadget/function/uvc.h      |   2 +
>>  drivers/usb/gadget/function/uvc_v4l2.c | 286 ++++++++++++++++++++
>>  include/media/v4l2-common.h            |   4 +
>>  include/media/v4l2-uvc.h               | 359 +++++++++++++++++++++++++
>>  10 files changed, 776 insertions(+), 443 deletions(-)
>>  create mode 100644 include/media/v4l2-uvc.h
>
>-- 
>Regards,
>
>Laurent Pinchart
>

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[Greg KH]

On Sat, Dec 03, 2022 at 11:26:14PM +0200, Laurent Pinchart wrote:
> Hi Michael,
> 
> On Sat, Sep 10, 2022 at 12:13:31AM +0200, Michael Grzeschik wrote:
> > This series improves the uvc video gadget by parsing the configfs
> > entries. With the configfs data, the userspace now is able to use simple
> > v4l2 api calls like enum and try_format to check for valid configurations
> > initially set by configfs.
> 
> I've realized that this whole series got merged, despite my multiple
> attempts to explain why I think it's not a good idea. The UVC gadget
> requires userspace support, and there's no point in trying to move all
> these things to the kernel side. It only bloats the kernel, makes the
> code more complex, more difficult to maintain, and will make UVC 1.5
> support more difficult.

I can easily revert them, but I did not see any objections to them
originally and so I merged them as is the normal method :)

thanks,

greg k-h

Re: [PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[Laurent Pinchart]

On Sun, Dec 04, 2022 at 09:29:16AM +0100, Greg KH wrote:
> On Sat, Dec 03, 2022 at 11:26:14PM +0200, Laurent Pinchart wrote:
> > Hi Michael,
> > 
> > On Sat, Sep 10, 2022 at 12:13:31AM +0200, Michael Grzeschik wrote:
> > > This series improves the uvc video gadget by parsing the configfs
> > > entries. With the configfs data, the userspace now is able to use simple
> > > v4l2 api calls like enum and try_format to check for valid configurations
> > > initially set by configfs.
> > 
> > I've realized that this whole series got merged, despite my multiple
> > attempts to explain why I think it's not a good idea. The UVC gadget
> > requires userspace support, and there's no point in trying to move all
> > these things to the kernel side. It only bloats the kernel, makes the
> > code more complex, more difficult to maintain, and will make UVC 1.5
> > support more difficult.
> 
> I can easily revert them, but I did not see any objections to them
> originally and so I merged them as is the normal method :)

I don't think a revert is needed. The issue I pointed out regarding the
duplication of static const data can be solved on top. The API additions
from this series are, in my opinion, not a good idea for the reasons I
explained, but they don't hurt so much that we need to go nuclear on
this.

Michael, will you be addressing the static const data issue ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[Michael Grzeschik]

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

On Mon, Dec 05, 2022 at 11:17:15PM +0200, Laurent Pinchart wrote:
>On Sun, Dec 04, 2022 at 09:29:16AM +0100, Greg KH wrote:
>> On Sat, Dec 03, 2022 at 11:26:14PM +0200, Laurent Pinchart wrote:
>> > Hi Michael,
>> >
>> > On Sat, Sep 10, 2022 at 12:13:31AM +0200, Michael Grzeschik wrote:
>> > > This series improves the uvc video gadget by parsing the configfs
>> > > entries. With the configfs data, the userspace now is able to use simple
>> > > v4l2 api calls like enum and try_format to check for valid configurations
>> > > initially set by configfs.
>> >
>> > I've realized that this whole series got merged, despite my multiple
>> > attempts to explain why I think it's not a good idea. The UVC gadget
>> > requires userspace support, and there's no point in trying to move all
>> > these things to the kernel side. It only bloats the kernel, makes the
>> > code more complex, more difficult to maintain, and will make UVC 1.5
>> > support more difficult.
>>
>> I can easily revert them, but I did not see any objections to them
>> originally and so I merged them as is the normal method :)
>
>I don't think a revert is needed. The issue I pointed out regarding the
>duplication of static const data can be solved on top. The API additions
>from this series are, in my opinion, not a good idea for the reasons I
>explained, but they don't hurt so much that we need to go nuclear on
>this.
>
>Michael, will you be addressing the static const data issue ?

Yes. I will also move the uvc_fmts[] array and uvc_format_by_guid to its
own compile unit.

I will go with drivers/media/usb/uvc.c

While at it the headerfile will better also be moved from
include/media/v4l2-uvc.h to linux/usb/uvc.h.

Thanks,
Michael


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[Ricardo Ribalda]

Hi Michael

On Tue, 6 Dec 2022 at 18:09, Michael Grzeschik <mgr@pengutronix.de> wrote:
>
> On Mon, Dec 05, 2022 at 11:17:15PM +0200, Laurent Pinchart wrote:
> >On Sun, Dec 04, 2022 at 09:29:16AM +0100, Greg KH wrote:
> >> On Sat, Dec 03, 2022 at 11:26:14PM +0200, Laurent Pinchart wrote:
> >> > Hi Michael,
> >> >
> >> > On Sat, Sep 10, 2022 at 12:13:31AM +0200, Michael Grzeschik wrote:
> >> > > This series improves the uvc video gadget by parsing the configfs
> >> > > entries. With the configfs data, the userspace now is able to use simple
> >> > > v4l2 api calls like enum and try_format to check for valid configurations
> >> > > initially set by configfs.
> >> >
> >> > I've realized that this whole series got merged, despite my multiple
> >> > attempts to explain why I think it's not a good idea. The UVC gadget
> >> > requires userspace support, and there's no point in trying to move all
> >> > these things to the kernel side. It only bloats the kernel, makes the
> >> > code more complex, more difficult to maintain, and will make UVC 1.5
> >> > support more difficult.
> >>
> >> I can easily revert them, but I did not see any objections to them
> >> originally and so I merged them as is the normal method :)
> >
> >I don't think a revert is needed. The issue I pointed out regarding the
> >duplication of static const data can be solved on top. The API additions
> >from this series are, in my opinion, not a good idea for the reasons I
> >explained, but they don't hurt so much that we need to go nuclear on
> >this.
> >
> >Michael, will you be addressing the static const data issue ?
>
> Yes. I will also move the uvc_fmts[] array and uvc_format_by_guid to its
> own compile unit.
>
> I will go with drivers/media/usb/uvc.c
>
> While at it the headerfile will better also be moved from
> include/media/v4l2-uvc.h to linux/usb/uvc.h.

And since it is free to ask :P....

Could you also try to remove the string definition and use the one
from v4l2-ioctl.c. ?

Or maybe it is not feasible?

Thanks!

>
> Thanks,
> Michael
>
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



-- 
Ricardo Ribalda

Re: [PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[Laurent Pinchart]

Hi Michael,

On Tue, Dec 06, 2022 at 06:07:21PM +0100, Michael Grzeschik wrote:
> On Mon, Dec 05, 2022 at 11:17:15PM +0200, Laurent Pinchart wrote:
> >On Sun, Dec 04, 2022 at 09:29:16AM +0100, Greg KH wrote:
> >> On Sat, Dec 03, 2022 at 11:26:14PM +0200, Laurent Pinchart wrote:
> >> > On Sat, Sep 10, 2022 at 12:13:31AM +0200, Michael Grzeschik wrote:
> >> > > This series improves the uvc video gadget by parsing the configfs
> >> > > entries. With the configfs data, the userspace now is able to use simple
> >> > > v4l2 api calls like enum and try_format to check for valid configurations
> >> > > initially set by configfs.
> >> >
> >> > I've realized that this whole series got merged, despite my multiple
> >> > attempts to explain why I think it's not a good idea. The UVC gadget
> >> > requires userspace support, and there's no point in trying to move all
> >> > these things to the kernel side. It only bloats the kernel, makes the
> >> > code more complex, more difficult to maintain, and will make UVC 1.5
> >> > support more difficult.
> >>
> >> I can easily revert them, but I did not see any objections to them
> >> originally and so I merged them as is the normal method :)
> >
> > I don't think a revert is needed. The issue I pointed out regarding the
> > duplication of static const data can be solved on top. The API additions
> > from this series are, in my opinion, not a good idea for the reasons I
> > explained, but they don't hurt so much that we need to go nuclear on
> > this.
> >
> > Michael, will you be addressing the static const data issue ?
> 
> Yes. I will also move the uvc_fmts[] array and uvc_format_by_guid to its
> own compile unit.

Thank you.

> I will go with drivers/media/usb/uvc.c
> 
> While at it the headerfile will better also be moved from
> include/media/v4l2-uvc.h to linux/usb/uvc.h.

Works for me, especially for the GUIDs. For the structure and function
prototype, I don't mind using include/media/, up to you.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[Krzysztof Opasiak]

Hi Greg,

On 4.12.2022 09:29, Greg KH wrote:
> On Sat, Dec 03, 2022 at 11:26:14PM +0200, Laurent Pinchart wrote:
>> Hi Michael,
>>
>> On Sat, Sep 10, 2022 at 12:13:31AM +0200, Michael Grzeschik wrote:
>>> This series improves the uvc video gadget by parsing the configfs
>>> entries. With the configfs data, the userspace now is able to use simple
>>> v4l2 api calls like enum and try_format to check for valid configurations
>>> initially set by configfs.
>>
>> I've realized that this whole series got merged, despite my multiple
>> attempts to explain why I think it's not a good idea. The UVC gadget
>> requires userspace support, and there's no point in trying to move all
>> these things to the kernel side. It only bloats the kernel, makes the
>> code more complex, more difficult to maintain, and will make UVC 1.5
>> support more difficult.
> 
> I can easily revert them, but I did not see any objections to them
> originally and so I merged them as is the normal method :)
> 

I know that it's already 2025 and I'm very late to the game but this 
series breaks our userspace scripts as it implicitly adds a requirement 
to name a streaming header directory as "h" which previously was a 
user-selected name. This requirement is coming from here:

+
+       streaming = config_group_find_item(&opts->func_inst.group, 
"streaming");
+       if (!streaming)
+               goto err_config;
+
+       header = config_group_find_item(to_config_group(streaming), 
"header");
+       config_item_put(streaming);
+       if (!header)
+               goto err_config;
+
+       h = config_group_find_item(to_config_group(header), "h");
+       config_item_put(header);
+       if (!h)
+               goto err_config;

If you name this directory as "header" gadget just fails to link to a 
configuration. Although this isn't a big deal on its own as we could 
simply rename the dir in our code but this is just the tip of the iceberg.

To my understanding, this patch broke an important feature of UVC 
ConfigFS interface which is allowing to present different list of 
available formats for different USB speeds as for example, it does not 
make sense to expose 1080p30 in high speed as it simply just does not 
fit into the ISO bandwidth of HS. For example what we've been using 
previously:

mkdir uvc.nf/streaming/uncompressed/hsu
mkdir uvc.nf/streaming/uncompressed/hsu/360p

mkdir uvc.nf/streaming/uncompressed/ssu
mkdir uvc.nf/streaming/uncompressed/ssu/360p
mkdir uvc.nf/streaming/uncompressed/ssu/720p
mkdir uvc.nf/streaming/uncompressed/ssu/1080p

symlink uvc.nf/streaming/uncompressed/hsu \
         uvc.nf/streaming/header/hsh/hsu

symlink uvc.nf/streaming/uncompressed/ssu \
         uvc.nf/streaming/header/ssh/hsu

symlink uvc.nf/streaming/header/hsh \
         uvc.nf/streaming/class/hs/h
symlink uvc.nf/streaming/header/ssh \
         uvc.nf/streaming/class/ss/h

no longer works as the patch requires presence of "h" directory inside 
"streaming/header" and even if we rename one of the headers directory to 
h the patch would be actually wrong as it would expose via v4l2 calls 
only formats available in the "h" directory and not in any other. (and 
at least on our adroid kernel it makes the kernel crash but haven't 
tested if that would be the case for mainline as well)

Given the limitations of the v4l2 interface I kind of don't see a way 
how we could fix this series to present proper formats to the userspace 
using v4l2 calls as the list of formats can change when the speed 
changes and userspace would have no way of knowing that.

Given that I'd like to suggest that it seems to actually make sense to 
revert this unless there are some ideas how to fix it.

-- 
Krzysztof Opasiak | R&D Team
neat.

Re: [PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[Greg KH]

On Mon, May 12, 2025 at 12:19:07PM +0200, Krzysztof Opasiak wrote:
> Hi Greg,
> 
> On 4.12.2022 09:29, Greg KH wrote:
> > On Sat, Dec 03, 2022 at 11:26:14PM +0200, Laurent Pinchart wrote:
> > > Hi Michael,
> > > 
> > > On Sat, Sep 10, 2022 at 12:13:31AM +0200, Michael Grzeschik wrote:
> > > > This series improves the uvc video gadget by parsing the configfs
> > > > entries. With the configfs data, the userspace now is able to use simple
> > > > v4l2 api calls like enum and try_format to check for valid configurations
> > > > initially set by configfs.
> > > 
> > > I've realized that this whole series got merged, despite my multiple
> > > attempts to explain why I think it's not a good idea. The UVC gadget
> > > requires userspace support, and there's no point in trying to move all
> > > these things to the kernel side. It only bloats the kernel, makes the
> > > code more complex, more difficult to maintain, and will make UVC 1.5
> > > support more difficult.
> > 
> > I can easily revert them, but I did not see any objections to them
> > originally and so I merged them as is the normal method :)
> > 
> 
> I know that it's already 2025 and I'm very late to the game but this series
> breaks our userspace scripts as it implicitly adds a requirement to name a
> streaming header directory as "h" which previously was a user-selected name.
> This requirement is coming from here:
> 
> +
> +       streaming = config_group_find_item(&opts->func_inst.group,
> "streaming");
> +       if (!streaming)
> +               goto err_config;
> +
> +       header = config_group_find_item(to_config_group(streaming),
> "header");
> +       config_item_put(streaming);
> +       if (!header)
> +               goto err_config;
> +
> +       h = config_group_find_item(to_config_group(header), "h");
> +       config_item_put(header);
> +       if (!h)
> +               goto err_config;
> 
> If you name this directory as "header" gadget just fails to link to a
> configuration. Although this isn't a big deal on its own as we could simply
> rename the dir in our code but this is just the tip of the iceberg.
> 
> To my understanding, this patch broke an important feature of UVC ConfigFS
> interface which is allowing to present different list of available formats
> for different USB speeds as for example, it does not make sense to expose
> 1080p30 in high speed as it simply just does not fit into the ISO bandwidth
> of HS. For example what we've been using previously:
> 
> mkdir uvc.nf/streaming/uncompressed/hsu
> mkdir uvc.nf/streaming/uncompressed/hsu/360p
> 
> mkdir uvc.nf/streaming/uncompressed/ssu
> mkdir uvc.nf/streaming/uncompressed/ssu/360p
> mkdir uvc.nf/streaming/uncompressed/ssu/720p
> mkdir uvc.nf/streaming/uncompressed/ssu/1080p
> 
> symlink uvc.nf/streaming/uncompressed/hsu \
>         uvc.nf/streaming/header/hsh/hsu
> 
> symlink uvc.nf/streaming/uncompressed/ssu \
>         uvc.nf/streaming/header/ssh/hsu
> 
> symlink uvc.nf/streaming/header/hsh \
>         uvc.nf/streaming/class/hs/h
> symlink uvc.nf/streaming/header/ssh \
>         uvc.nf/streaming/class/ss/h
> 
> no longer works as the patch requires presence of "h" directory inside
> "streaming/header" and even if we rename one of the headers directory to h
> the patch would be actually wrong as it would expose via v4l2 calls only
> formats available in the "h" directory and not in any other. (and at least
> on our adroid kernel it makes the kernel crash but haven't tested if that
> would be the case for mainline as well)
> 
> Given the limitations of the v4l2 interface I kind of don't see a way how we
> could fix this series to present proper formats to the userspace using v4l2
> calls as the list of formats can change when the speed changes and userspace
> would have no way of knowing that.
> 
> Given that I'd like to suggest that it seems to actually make sense to
> revert this unless there are some ideas how to fix it.

Sorry about this, can you submit a patch series that reverts the
offending commits?  As it was years ago, I don't exactly know what you
are referring to anymore.

thanks,

greg k-h

Re: [PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[Krzysztof Opasiak]

On 12.05.2025 12:38, Greg KH wrote:
> On Mon, May 12, 2025 at 12:19:07PM +0200, Krzysztof Opasiak wrote:
>> Hi Greg,
>>
>> On 4.12.2022 09:29, Greg KH wrote:
>>> On Sat, Dec 03, 2022 at 11:26:14PM +0200, Laurent Pinchart wrote:
>>>> Hi Michael,
>>>>
>>>> On Sat, Sep 10, 2022 at 12:13:31AM +0200, Michael Grzeschik wrote:
>>>>> This series improves the uvc video gadget by parsing the configfs
>>>>> entries. With the configfs data, the userspace now is able to use simple
>>>>> v4l2 api calls like enum and try_format to check for valid configurations
>>>>> initially set by configfs.
>>>>
>>>> I've realized that this whole series got merged, despite my multiple
>>>> attempts to explain why I think it's not a good idea. The UVC gadget
>>>> requires userspace support, and there's no point in trying to move all
>>>> these things to the kernel side. It only bloats the kernel, makes the
>>>> code more complex, more difficult to maintain, and will make UVC 1.5
>>>> support more difficult.
>>>
>>> I can easily revert them, but I did not see any objections to them
>>> originally and so I merged them as is the normal method :)
>>>
>>
>> I know that it's already 2025 and I'm very late to the game but this series
>> breaks our userspace scripts as it implicitly adds a requirement to name a
>> streaming header directory as "h" which previously was a user-selected name.
>> This requirement is coming from here:
>>
>> +
>> +       streaming = config_group_find_item(&opts->func_inst.group,
>> "streaming");
>> +       if (!streaming)
>> +               goto err_config;
>> +
>> +       header = config_group_find_item(to_config_group(streaming),
>> "header");
>> +       config_item_put(streaming);
>> +       if (!header)
>> +               goto err_config;
>> +
>> +       h = config_group_find_item(to_config_group(header), "h");
>> +       config_item_put(header);
>> +       if (!h)
>> +               goto err_config;
>>
>> If you name this directory as "header" gadget just fails to link to a
>> configuration. Although this isn't a big deal on its own as we could simply
>> rename the dir in our code but this is just the tip of the iceberg.
>>
>> To my understanding, this patch broke an important feature of UVC ConfigFS
>> interface which is allowing to present different list of available formats
>> for different USB speeds as for example, it does not make sense to expose
>> 1080p30 in high speed as it simply just does not fit into the ISO bandwidth
>> of HS. For example what we've been using previously:
>>
>> mkdir uvc.nf/streaming/uncompressed/hsu
>> mkdir uvc.nf/streaming/uncompressed/hsu/360p
>>
>> mkdir uvc.nf/streaming/uncompressed/ssu
>> mkdir uvc.nf/streaming/uncompressed/ssu/360p
>> mkdir uvc.nf/streaming/uncompressed/ssu/720p
>> mkdir uvc.nf/streaming/uncompressed/ssu/1080p
>>
>> symlink uvc.nf/streaming/uncompressed/hsu \
>>          uvc.nf/streaming/header/hsh/hsu
>>
>> symlink uvc.nf/streaming/uncompressed/ssu \
>>          uvc.nf/streaming/header/ssh/hsu
>>
>> symlink uvc.nf/streaming/header/hsh \
>>          uvc.nf/streaming/class/hs/h
>> symlink uvc.nf/streaming/header/ssh \
>>          uvc.nf/streaming/class/ss/h
>>
>> no longer works as the patch requires presence of "h" directory inside
>> "streaming/header" and even if we rename one of the headers directory to h
>> the patch would be actually wrong as it would expose via v4l2 calls only
>> formats available in the "h" directory and not in any other. (and at least
>> on our adroid kernel it makes the kernel crash but haven't tested if that
>> would be the case for mainline as well)
>>
>> Given the limitations of the v4l2 interface I kind of don't see a way how we
>> could fix this series to present proper formats to the userspace using v4l2
>> calls as the list of formats can change when the speed changes and userspace
>> would have no way of knowing that.
>>
>> Given that I'd like to suggest that it seems to actually make sense to
>> revert this unless there are some ideas how to fix it.
> 
> Sorry about this, can you submit a patch series that reverts the
> offending commits?  As it was years ago, I don't exactly know what you
> are referring to anymore.
> 

Sure! Will do.

-- 
Krzysztof Opasiak | R&D Team
neat.

Re: [PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[Krzysztof Opasiak]

On 12.05.2025 12:43, Krzysztof Opasiak wrote:
> On 12.05.2025 12:38, Greg KH wrote:
>> On Mon, May 12, 2025 at 12:19:07PM +0200, Krzysztof Opasiak wrote:
>>> Hi Greg,
>>>
>>> On 4.12.2022 09:29, Greg KH wrote:
>>>> On Sat, Dec 03, 2022 at 11:26:14PM +0200, Laurent Pinchart wrote:
>>>>> Hi Michael,
>>>>>
>>>>> On Sat, Sep 10, 2022 at 12:13:31AM +0200, Michael Grzeschik wrote:
[...]
>>>
>>> Given that I'd like to suggest that it seems to actually make sense to
>>> revert this unless there are some ideas how to fix it.
>>
>> Sorry about this, can you submit a patch series that reverts the
>> offending commits?  As it was years ago, I don't exactly know what you
>> are referring to anymore.
>>
> 
> Sure! Will do.
> 

Would you prefer to have a set of actual reverts related to this:

da692963df4e Revert "usb: gadget: uvc: add v4l2 enumeration api calls"
bca75df69aaf Revert "usb: gadget: uvc: add v4l2 try_format api call"
e56c767a6d3c Revert "usb: gadget: uvc: also use try_format in set_format"
20f275b86960 Revert "usb: gadget: uvc: fix try format returns on 
uncompressed formats"
059d98f60c21 Revert "usb: gadget: uvc: Fix ERR_PTR dereference in 
uvc_v4l2.c"
e6fd9b67414c Revert "usb: gadget: webcam: Make g_webcam loadable again"

but have a negative consequence that the series isn't really bisectable 
from functional perspective. For example commit e6fd9b67414c breaks 
g_uvc until we apply da692963df4e so the series would have to go in as a 
whole.

Or you would prefer a single commit that technically isn't a revert but 
it just "undoes" the negative consequences of "usb: gadget: uvc: add 
v4l2 enumeration api calls" (kind of a squash of all commits above)?

Best regards,
-- 
Krzysztof Opasiak | R&D Team
neat.

Re: [PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[Greg KH]

On Mon, May 12, 2025 at 11:03:41PM +0200, Krzysztof Opasiak wrote:
> On 12.05.2025 12:43, Krzysztof Opasiak wrote:
> > On 12.05.2025 12:38, Greg KH wrote:
> > > On Mon, May 12, 2025 at 12:19:07PM +0200, Krzysztof Opasiak wrote:
> > > > Hi Greg,
> > > > 
> > > > On 4.12.2022 09:29, Greg KH wrote:
> > > > > On Sat, Dec 03, 2022 at 11:26:14PM +0200, Laurent Pinchart wrote:
> > > > > > Hi Michael,
> > > > > > 
> > > > > > On Sat, Sep 10, 2022 at 12:13:31AM +0200, Michael Grzeschik wrote:
> [...]
> > > > 
> > > > Given that I'd like to suggest that it seems to actually make sense to
> > > > revert this unless there are some ideas how to fix it.
> > > 
> > > Sorry about this, can you submit a patch series that reverts the
> > > offending commits?  As it was years ago, I don't exactly know what you
> > > are referring to anymore.
> > > 
> > 
> > Sure! Will do.
> > 
> 
> Would you prefer to have a set of actual reverts related to this:
> 
> da692963df4e Revert "usb: gadget: uvc: add v4l2 enumeration api calls"
> bca75df69aaf Revert "usb: gadget: uvc: add v4l2 try_format api call"
> e56c767a6d3c Revert "usb: gadget: uvc: also use try_format in set_format"
> 20f275b86960 Revert "usb: gadget: uvc: fix try format returns on
> uncompressed formats"
> 059d98f60c21 Revert "usb: gadget: uvc: Fix ERR_PTR dereference in
> uvc_v4l2.c"
> e6fd9b67414c Revert "usb: gadget: webcam: Make g_webcam loadable again"
> 
> but have a negative consequence that the series isn't really bisectable from
> functional perspective. For example commit e6fd9b67414c breaks g_uvc until
> we apply da692963df4e so the series would have to go in as a whole.
> 
> Or you would prefer a single commit that technically isn't a revert but it
> just "undoes" the negative consequences of "usb: gadget: uvc: add v4l2
> enumeration api calls" (kind of a squash of all commits above)?

Ideally we can bisect at all places in the tree, so it's odd that
reverting patches would cause problems as when adding them all should
have been ok for every commit, right?

But if there are merge issues, or other problems, then yes, maybe just
one big one is needed, your choice.

thanks,

greg k-h

Re: [PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[Nicolas Dufresne]

Hi Greg, Krzysztof,

Le mardi 13 mai 2025 à 07:04 +0200, Greg KH a écrit :
> On Mon, May 12, 2025 at 11:03:41PM +0200, Krzysztof Opasiak wrote:
> > On 12.05.2025 12:43, Krzysztof Opasiak wrote:
> > > On 12.05.2025 12:38, Greg KH wrote:
> > > > On Mon, May 12, 2025 at 12:19:07PM +0200, Krzysztof Opasiak wrote:
> > > > > Hi Greg,
> > > > > 
> > > > > On 4.12.2022 09:29, Greg KH wrote:
> > > > > > On Sat, Dec 03, 2022 at 11:26:14PM +0200, Laurent Pinchart wrote:
> > > > > > > Hi Michael,
> > > > > > > 
> > > > > > > On Sat, Sep 10, 2022 at 12:13:31AM +0200, Michael Grzeschik wrote:
> > [...]
> > > > > 
> > > > > Given that I'd like to suggest that it seems to actually make sense to
> > > > > revert this unless there are some ideas how to fix it.
> > > > 
> > > > Sorry about this, can you submit a patch series that reverts the
> > > > offending commits?  As it was years ago, I don't exactly know what you
> > > > are referring to anymore.
> > > > 
> > > 
> > > Sure! Will do.
> > > 
> > 
> > Would you prefer to have a set of actual reverts related to this:
> > 
> > da692963df4e Revert "usb: gadget: uvc: add v4l2 enumeration api calls"
> > bca75df69aaf Revert "usb: gadget: uvc: add v4l2 try_format api call"
> > e56c767a6d3c Revert "usb: gadget: uvc: also use try_format in set_format"
> > 20f275b86960 Revert "usb: gadget: uvc: fix try format returns on
> > uncompressed formats"
> > 059d98f60c21 Revert "usb: gadget: uvc: Fix ERR_PTR dereference in
> > uvc_v4l2.c"
> > e6fd9b67414c Revert "usb: gadget: webcam: Make g_webcam loadable again"
> > 
> > but have a negative consequence that the series isn't really bisectable from
> > functional perspective. For example commit e6fd9b67414c breaks g_uvc until
> > we apply da692963df4e so the series would have to go in as a whole.
> > 
> > Or you would prefer a single commit that technically isn't a revert but it
> > just "undoes" the negative consequences of "usb: gadget: uvc: add v4l2
> > enumeration api calls" (kind of a squash of all commits above)?
> 
> Ideally we can bisect at all places in the tree, so it's odd that
> reverting patches would cause problems as when adding them all should
> have been ok for every commit, right?
> 
> But if there are merge issues, or other problems, then yes, maybe just
> one big one is needed, your choice.

Won't a plain revert break userspace like GStreamer that have depended on
that patch for years ? In such a delicate case, wouldn't it be less
damageable to introduce workaround, like alias ? This is one personal
script against numerous users of a generic framework implementation.

I believe due to the delay, you are facing an unusual ABI breakage, which
requires a more delicate handling.

regards,
Nicolas

> 
> thanks,
> 
> greg k-h

Re: [PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[Krzysztof Opasiak]

On 13.05.2025 12:04, Nicolas Dufresne wrote:
> Hi Greg, Krzysztof,
> 
> Le mardi 13 mai 2025 à 07:04 +0200, Greg KH a écrit :
>> On Mon, May 12, 2025 at 11:03:41PM +0200, Krzysztof Opasiak wrote:
>>> On 12.05.2025 12:43, Krzysztof Opasiak wrote:
>>>> On 12.05.2025 12:38, Greg KH wrote:
>>>>> On Mon, May 12, 2025 at 12:19:07PM +0200, Krzysztof Opasiak wrote:
>>>>>> Hi Greg,
>>>>>>
>>>>>> On 4.12.2022 09:29, Greg KH wrote:
>>>>>>> On Sat, Dec 03, 2022 at 11:26:14PM +0200, Laurent Pinchart wrote:
>>>>>>>> Hi Michael,
>>>>>>>>
>>>>>>>> On Sat, Sep 10, 2022 at 12:13:31AM +0200, Michael Grzeschik wrote:
>>> [...]
>>>>>>
>>>>>> Given that I'd like to suggest that it seems to actually make sense to
>>>>>> revert this unless there are some ideas how to fix it.
>>>>>
>>>>> Sorry about this, can you submit a patch series that reverts the
>>>>> offending commits?  As it was years ago, I don't exactly know what you
>>>>> are referring to anymore.
>>>>>
>>>>
>>>> Sure! Will do.
>>>>
>>>
>>> Would you prefer to have a set of actual reverts related to this:
>>>
>>> da692963df4e Revert "usb: gadget: uvc: add v4l2 enumeration api calls"
>>> bca75df69aaf Revert "usb: gadget: uvc: add v4l2 try_format api call"
>>> e56c767a6d3c Revert "usb: gadget: uvc: also use try_format in set_format"
>>> 20f275b86960 Revert "usb: gadget: uvc: fix try format returns on
>>> uncompressed formats"
>>> 059d98f60c21 Revert "usb: gadget: uvc: Fix ERR_PTR dereference in
>>> uvc_v4l2.c"
>>> e6fd9b67414c Revert "usb: gadget: webcam: Make g_webcam loadable again"
>>>
>>> but have a negative consequence that the series isn't really bisectable from
>>> functional perspective. For example commit e6fd9b67414c breaks g_uvc until
>>> we apply da692963df4e so the series would have to go in as a whole.
>>>
>>> Or you would prefer a single commit that technically isn't a revert but it
>>> just "undoes" the negative consequences of "usb: gadget: uvc: add v4l2
>>> enumeration api calls" (kind of a squash of all commits above)?
>>
>> Ideally we can bisect at all places in the tree, so it's odd that
>> reverting patches would cause problems as when adding them all should
>> have been ok for every commit, right?
>>
>> But if there are merge issues, or other problems, then yes, maybe just
>> one big one is needed, your choice.
> 
> Won't a plain revert break userspace like GStreamer that have depended on
> that patch for years ? In such a delicate case, wouldn't it be less
> damageable to introduce workaround, like alias ? This is one personal
> script against numerous users of a generic framework implementation.

Shouldn't GStreamer be robust enough to handle cases of old-kernel which 
haven't had this feature at all?

The main reason why I reported this is not really the name limitation 
but the fact that this patch is just incorrect for cases where you would 
like to expose different formats at different speeds. This feature was 
there for years and we do have products that depend on it and this 
change along with the further commits that depend on it broke that 
support for us.

You are absolutely right that those commits added a feature that now 
someone else may depend thus it would be perfect to fix it in a way that 
doesn't break anyone's userspace but the problem is that due to the way 
v4l2 API is designed I really don't see a way how we could make it 
"speed-aware" without breaking all the users. That's why we are kind of 
stuck between:

1. Leave those commits in and then you cannot different streaming 
headers for different speeds but users of those API will keep working.

2. Revert and bring back the feature of UVC ConfigFS interface that was 
there since its inception but break the users of "New API".

> 
> I believe due to the delay, you are facing an unusual ABI breakage, which
> requires a more delicate handling.

Agree. The situation isn't simple as whatever we do would break some 
userspace... I'm not an expert on v4l2, so if anyone with a better 
knowledge of v4l2 internals has a suggestion how we could make this work 
with the existing API I'd be more than happy to try to follow that path.

Best regards,
-- 
Krzysztof Opasiak | R&D Team
neat.

Re: [PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[Nicolas Dufresne]

Hi Krzysztof,

Le mardi 13 mai 2025 à 12:31 +0200, Krzysztof Opasiak a écrit :
> On 13.05.2025 12:04, Nicolas Dufresne wrote:
> > Hi Greg, Krzysztof,
> > 
> > Le mardi 13 mai 2025 à 07:04 +0200, Greg KH a écrit :
> > > On Mon, May 12, 2025 at 11:03:41PM +0200, Krzysztof Opasiak wrote:
> > > > On 12.05.2025 12:43, Krzysztof Opasiak wrote:
> > > > > On 12.05.2025 12:38, Greg KH wrote:
> > > > > > On Mon, May 12, 2025 at 12:19:07PM +0200, Krzysztof Opasiak wrote:
> > > > > > > Hi Greg,
> > > > > > > 
> > > > > > > On 4.12.2022 09:29, Greg KH wrote:
> > > > > > > > On Sat, Dec 03, 2022 at 11:26:14PM +0200, Laurent Pinchart wrote:
> > > > > > > > > Hi Michael,
> > > > > > > > > 
> > > > > > > > > On Sat, Sep 10, 2022 at 12:13:31AM +0200, Michael Grzeschik wrote:
> > > > [...]
> > > > > > > 
> > > > > > > Given that I'd like to suggest that it seems to actually make sense to
> > > > > > > revert this unless there are some ideas how to fix it.
> > > > > > 
> > > > > > Sorry about this, can you submit a patch series that reverts the
> > > > > > offending commits?  As it was years ago, I don't exactly know what you
> > > > > > are referring to anymore.
> > > > > > 
> > > > > 
> > > > > Sure! Will do.
> > > > > 
> > > > 
> > > > Would you prefer to have a set of actual reverts related to this:
> > > > 
> > > > da692963df4e Revert "usb: gadget: uvc: add v4l2 enumeration api calls"
> > > > bca75df69aaf Revert "usb: gadget: uvc: add v4l2 try_format api call"
> > > > e56c767a6d3c Revert "usb: gadget: uvc: also use try_format in set_format"
> > > > 20f275b86960 Revert "usb: gadget: uvc: fix try format returns on
> > > > uncompressed formats"
> > > > 059d98f60c21 Revert "usb: gadget: uvc: Fix ERR_PTR dereference in
> > > > uvc_v4l2.c"
> > > > e6fd9b67414c Revert "usb: gadget: webcam: Make g_webcam loadable again"
> > > > 
> > > > but have a negative consequence that the series isn't really bisectable from
> > > > functional perspective. For example commit e6fd9b67414c breaks g_uvc until
> > > > we apply da692963df4e so the series would have to go in as a whole.
> > > > 
> > > > Or you would prefer a single commit that technically isn't a revert but it
> > > > just "undoes" the negative consequences of "usb: gadget: uvc: add v4l2
> > > > enumeration api calls" (kind of a squash of all commits above)?
> > > 
> > > Ideally we can bisect at all places in the tree, so it's odd that
> > > reverting patches would cause problems as when adding them all should
> > > have been ok for every commit, right?
> > > 
> > > But if there are merge issues, or other problems, then yes, maybe just
> > > one big one is needed, your choice.
> > 
> > Won't a plain revert break userspace like GStreamer that have depended on
> > that patch for years ? In such a delicate case, wouldn't it be less
> > damageable to introduce workaround, like alias ? This is one personal
> > script against numerous users of a generic framework implementation.
> 
> Shouldn't GStreamer be robust enough to handle cases of old-kernel which 
> haven't had this feature at all?

This only valid if the userspace code was written before this change made
in 2022. The uvcsink code in GStreamer is based on kernel at the time it
was written, and will effectively maintain backward compatibility from
there.

That being said, would be nice for someone with a working setup
to first verify it if breaks, how it breaks, and share.

> 
> The main reason why I reported this is not really the name limitation 
> but the fact that this patch is just incorrect for cases where you would 
> like to expose different formats at different speeds. This feature was 
> there for years and we do have products that depend on it and this 
> change along with the further commits that depend on it broke that 
> support for us.
> 
> You are absolutely right that those commits added a feature that now 
> someone else may depend thus it would be perfect to fix it in a way that 
> doesn't break anyone's userspace but the problem is that due to the way 
> v4l2 API is designed I really don't see a way how we could make it 
> "speed-aware" without breaking all the users. That's why we are kind of 
> stuck between:
> 
> 1. Leave those commits in and then you cannot different streaming 
> headers for different speeds but users of those API will keep working.
> 
> 2. Revert and bring back the feature of UVC ConfigFS interface that was 
> there since its inception but break the users of "New API".
> 
> > 
> > I believe due to the delay, you are facing an unusual ABI breakage, which
> > requires a more delicate handling.
> 
> Agree. The situation isn't simple as whatever we do would break some 
> userspace... I'm not an expert on v4l2, so if anyone with a better 
> knowledge of v4l2 internals has a suggestion how we could make this work 
> with the existing API I'd be more than happy to try to follow that path.

It is a situation for sure, and I'm not denying either way, am not deeply
into the details either. I was just reviewing that work in GStreamer with
little view on the kernel activity.

regards,
Nicolas

> 
> Best regards,

Re: [PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[Michael Grzeschik]

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

Hi Greg, Krzysztof and Nicolas,

On Tue, May 13, 2025 at 12:31:49PM +0200, Krzysztof Opasiak wrote:
>On 13.05.2025 12:04, Nicolas Dufresne wrote:
>>Hi Greg, Krzysztof,
>>
>>Le mardi 13 mai 2025 à 07:04 +0200, Greg KH a écrit :
>>>On Mon, May 12, 2025 at 11:03:41PM +0200, Krzysztof Opasiak wrote:
>>>>On 12.05.2025 12:43, Krzysztof Opasiak wrote:
>>>>>On 12.05.2025 12:38, Greg KH wrote:
>>>>>>On Mon, May 12, 2025 at 12:19:07PM +0200, Krzysztof Opasiak wrote:
>>>>>>>Hi Greg,
>>>>>>>
>>>>>>>On 4.12.2022 09:29, Greg KH wrote:
>>>>>>>>On Sat, Dec 03, 2022 at 11:26:14PM +0200, Laurent Pinchart wrote:
>>>>>>>>>Hi Michael,
>>>>>>>>>
>>>>>>>>>On Sat, Sep 10, 2022 at 12:13:31AM +0200, Michael Grzeschik wrote:
>>>>[...]
>>>>>>>
>>>>>>>Given that I'd like to suggest that it seems to actually make sense to
>>>>>>>revert this unless there are some ideas how to fix it.
>>>>>>
>>>>>>Sorry about this, can you submit a patch series that reverts the
>>>>>>offending commits?  As it was years ago, I don't exactly know what you
>>>>>>are referring to anymore.
>>>>>>
>>>>>
>>>>>Sure! Will do.
>>>>>
>>>>
>>>>Would you prefer to have a set of actual reverts related to this:
>>>>
>>>>da692963df4e Revert "usb: gadget: uvc: add v4l2 enumeration api calls"
>>>>bca75df69aaf Revert "usb: gadget: uvc: add v4l2 try_format api call"
>>>>e56c767a6d3c Revert "usb: gadget: uvc: also use try_format in set_format"
>>>>20f275b86960 Revert "usb: gadget: uvc: fix try format returns on
>>>>uncompressed formats"
>>>>059d98f60c21 Revert "usb: gadget: uvc: Fix ERR_PTR dereference in
>>>>uvc_v4l2.c"
>>>>e6fd9b67414c Revert "usb: gadget: webcam: Make g_webcam loadable again"
>>>>
>>>>but have a negative consequence that the series isn't really bisectable from
>>>>functional perspective. For example commit e6fd9b67414c breaks g_uvc until
>>>>we apply da692963df4e so the series would have to go in as a whole.
>>>>
>>>>Or you would prefer a single commit that technically isn't a revert but it
>>>>just "undoes" the negative consequences of "usb: gadget: uvc: add v4l2
>>>>enumeration api calls" (kind of a squash of all commits above)?
>>>
>>>Ideally we can bisect at all places in the tree, so it's odd that
>>>reverting patches would cause problems as when adding them all should
>>>have been ok for every commit, right?
>>>
>>>But if there are merge issues, or other problems, then yes, maybe just
>>>one big one is needed, your choice.
>>
>>Won't a plain revert break userspace like GStreamer that have depended on
>>that patch for years ? In such a delicate case, wouldn't it be less
>>damageable to introduce workaround, like alias ? This is one personal
>>script against numerous users of a generic framework implementation.
>
>Shouldn't GStreamer be robust enough to handle cases of old-kernel 
>which haven't had this feature at all?
>
>The main reason why I reported this is not really the name limitation 
>but the fact that this patch is just incorrect for cases where you 
>would like to expose different formats at different speeds. This 
>feature was there for years and we do have products that depend on it 
>and this change along with the further commits that depend on it broke 
>that support for us.
>
>You are absolutely right that those commits added a feature that now 
>someone else may depend thus it would be perfect to fix it in a way 
>that doesn't break anyone's userspace but the problem is that due to 
>the way v4l2 API is designed I really don't see a way how we could 
>make it "speed-aware" without breaking all the users. That's why we 
>are kind of stuck between:
>
>1. Leave those commits in and then you cannot different streaming 
>headers for different speeds but users of those API will keep working.
>
>2. Revert and bring back the feature of UVC ConfigFS interface that 
>was there since its inception but break the users of "New API".
>
>>
>>I believe due to the delay, you are facing an unusual ABI breakage, which
>>requires a more delicate handling.
>
>Agree. The situation isn't simple as whatever we do would break some 
>userspace... I'm not an expert on v4l2, so if anyone with a better 
>knowledge of v4l2 internals has a suggestion how we could make this 
>work with the existing API I'd be more than happy to try to follow 
>that path.

As a shortcomming compromise I would suggest to support both worlds by
conditionally set uvc->header if the directory h was found. If the
uvc->header was not set then we could print some info and disable the
main part of the possible uvc callbacks that depend on this uvc->header
objects.

The only downside with that would be that using directory h in the
streaming header will implicitly create the limitations of not
indipendent formats per speed that Krzysztof mentioned.

The alternative would be to put more effort into this and parse all
directories in the streaming header subnode. However since the idea of
using the v4l2-api is already talked dead by a long discussion history,
I would rather focus on transition the remaining functionality of the
gstreamer uvcgadget plugin to finally become independent of the v4l2api.

How about that transition path?

Regards,
Michael

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[Krzysztof Opasiak]

On 13.05.2025 14:55, Michael Grzeschik wrote:
> Hi Greg, Krzysztof and Nicolas,
> 
> On Tue, May 13, 2025 at 12:31:49PM +0200, Krzysztof Opasiak wrote:
>> On 13.05.2025 12:04, Nicolas Dufresne wrote:
>>> Hi Greg, Krzysztof,
>>>
>>> Le mardi 13 mai 2025 à 07:04 +0200, Greg KH a écrit :
>>>> On Mon, May 12, 2025 at 11:03:41PM +0200, Krzysztof Opasiak wrote:
>>>>> On 12.05.2025 12:43, Krzysztof Opasiak wrote:
>>>>>> On 12.05.2025 12:38, Greg KH wrote:
>>>>>>> On Mon, May 12, 2025 at 12:19:07PM +0200, Krzysztof Opasiak wrote:
>>>>>>>> Hi Greg,
>>>>>>>>
>>>>>>>> On 4.12.2022 09:29, Greg KH wrote:
>>>>>>>>> On Sat, Dec 03, 2022 at 11:26:14PM +0200, Laurent Pinchart wrote:
>>>>>>>>>> Hi Michael,
>>>>>>>>>>
>>>>>>>>>> On Sat, Sep 10, 2022 at 12:13:31AM +0200, Michael Grzeschik 
>>>>>>>>>> wrote:
>>>>> [...]
>>>>>>>>
>>>>>>>> Given that I'd like to suggest that it seems to actually make 
>>>>>>>> sense to
>>>>>>>> revert this unless there are some ideas how to fix it.
>>>>>>>
>>>>>>> Sorry about this, can you submit a patch series that reverts the
>>>>>>> offending commits?  As it was years ago, I don't exactly know 
>>>>>>> what you
>>>>>>> are referring to anymore.
>>>>>>>
>>>>>>
>>>>>> Sure! Will do.
>>>>>>
>>>>>
>>>>> Would you prefer to have a set of actual reverts related to this:
>>>>>
>>>>> da692963df4e Revert "usb: gadget: uvc: add v4l2 enumeration api calls"
>>>>> bca75df69aaf Revert "usb: gadget: uvc: add v4l2 try_format api call"
>>>>> e56c767a6d3c Revert "usb: gadget: uvc: also use try_format in 
>>>>> set_format"
>>>>> 20f275b86960 Revert "usb: gadget: uvc: fix try format returns on
>>>>> uncompressed formats"
>>>>> 059d98f60c21 Revert "usb: gadget: uvc: Fix ERR_PTR dereference in
>>>>> uvc_v4l2.c"
>>>>> e6fd9b67414c Revert "usb: gadget: webcam: Make g_webcam loadable 
>>>>> again"
>>>>>
>>>>> but have a negative consequence that the series isn't really 
>>>>> bisectable from
>>>>> functional perspective. For example commit e6fd9b67414c breaks 
>>>>> g_uvc until
>>>>> we apply da692963df4e so the series would have to go in as a whole.
>>>>>
>>>>> Or you would prefer a single commit that technically isn't a revert 
>>>>> but it
>>>>> just "undoes" the negative consequences of "usb: gadget: uvc: add v4l2
>>>>> enumeration api calls" (kind of a squash of all commits above)?
>>>>
>>>> Ideally we can bisect at all places in the tree, so it's odd that
>>>> reverting patches would cause problems as when adding them all should
>>>> have been ok for every commit, right?
>>>>
>>>> But if there are merge issues, or other problems, then yes, maybe just
>>>> one big one is needed, your choice.
>>>
>>> Won't a plain revert break userspace like GStreamer that have 
>>> depended on
>>> that patch for years ? In such a delicate case, wouldn't it be less
>>> damageable to introduce workaround, like alias ? This is one personal
>>> script against numerous users of a generic framework implementation.
>>
>> Shouldn't GStreamer be robust enough to handle cases of old-kernel 
>> which haven't had this feature at all?
>>
>> The main reason why I reported this is not really the name limitation 
>> but the fact that this patch is just incorrect for cases where you 
>> would like to expose different formats at different speeds. This 
>> feature was there for years and we do have products that depend on it 
>> and this change along with the further commits that depend on it broke 
>> that support for us.
>>
>> You are absolutely right that those commits added a feature that now 
>> someone else may depend thus it would be perfect to fix it in a way 
>> that doesn't break anyone's userspace but the problem is that due to 
>> the way v4l2 API is designed I really don't see a way how we could 
>> make it "speed-aware" without breaking all the users. That's why we 
>> are kind of stuck between:
>>
>> 1. Leave those commits in and then you cannot different streaming 
>> headers for different speeds but users of those API will keep working.
>>
>> 2. Revert and bring back the feature of UVC ConfigFS interface that 
>> was there since its inception but break the users of "New API".
>>
>>>
>>> I believe due to the delay, you are facing an unusual ABI breakage, 
>>> which
>>> requires a more delicate handling.
>>
>> Agree. The situation isn't simple as whatever we do would break some 
>> userspace... I'm not an expert on v4l2, so if anyone with a better 
>> knowledge of v4l2 internals has a suggestion how we could make this 
>> work with the existing API I'd be more than happy to try to follow 
>> that path.
> 
> As a shortcomming compromise I would suggest to support both worlds by
> conditionally set uvc->header if the directory h was found. If the
> uvc->header was not set then we could print some info and disable the
> main part of the possible uvc callbacks that depend on this uvc->header
> objects.
> 
> The only downside with that would be that using directory h in the
> streaming header will implicitly create the limitations of not
> indipendent formats per speed that Krzysztof mentioned.
> 
> The alternative would be to put more effort into this and parse all
> directories in the streaming header subnode. However since the idea of
> using the v4l2-api is already talked dead by a long discussion history,
> I would rather focus on transition the remaining functionality of the
> gstreamer uvcgadget plugin to finally become independent of the v4l2api.
> 
> How about that transition path?
> 

If I understood you correctly, you are proposing that if the header 
directories are named in a different way than h we would not set the 
uvc->header pointer and make all the v4l2 callback that are using it 
just return -ENOTSUP in this case? It would also mean that the 
uvc_v4l2_set_format() would need to somehow be broad back to the 
previous version which does not use -uvc_v4l2_try_format().

Although seems kind of hacky to change kernel behavior based on the 
directory name I think if we add this to the doc this could be an 
acceptable compromise that would make both scenarios work. 
Alternatively, instead of basing that on the directory name maybe a 
proper, named ConfigFS attribute somewhere inside UVC directory 
"gstreamer_support" or something else would be more explicit for the user?


-- 
Krzysztof Opasiak | R&D Team
neat.

Re: [PATCH v2 0/4] usb: gadget: uvc: parse configfs entries and implement v4l2 enum api calls

[Michael Grzeschik]

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

On Tue, May 13, 2025 at 03:07:07PM +0200, Krzysztof Opasiak wrote:
>On 13.05.2025 14:55, Michael Grzeschik wrote:
>>Hi Greg, Krzysztof and Nicolas,
>>
>>On Tue, May 13, 2025 at 12:31:49PM +0200, Krzysztof Opasiak wrote:
>>>On 13.05.2025 12:04, Nicolas Dufresne wrote:
>>>>Hi Greg, Krzysztof,
>>>>
>>>>Le mardi 13 mai 2025 à 07:04 +0200, Greg KH a écrit :
>>>>>On Mon, May 12, 2025 at 11:03:41PM +0200, Krzysztof Opasiak wrote:
>>>>>>On 12.05.2025 12:43, Krzysztof Opasiak wrote:
>>>>>>>On 12.05.2025 12:38, Greg KH wrote:
>>>>>>>>On Mon, May 12, 2025 at 12:19:07PM +0200, Krzysztof Opasiak wrote:
>>>>>>>>>Hi Greg,
>>>>>>>>>
>>>>>>>>>On 4.12.2022 09:29, Greg KH wrote:
>>>>>>>>>>On Sat, Dec 03, 2022 at 11:26:14PM +0200, Laurent Pinchart wrote:
>>>>>>>>>>>Hi Michael,
>>>>>>>>>>>
>>>>>>>>>>>On Sat, Sep 10, 2022 at 12:13:31AM +0200, Michael 
>>>>>>>>>>>Grzeschik wrote:
>>>>>>[...]
>>>>>>>>>
>>>>>>>>>Given that I'd like to suggest that it seems to 
>>>>>>>>>actually make sense to
>>>>>>>>>revert this unless there are some ideas how to fix it.
>>>>>>>>
>>>>>>>>Sorry about this, can you submit a patch series that reverts the
>>>>>>>>offending commits?  As it was years ago, I don't exactly 
>>>>>>>>know what you
>>>>>>>>are referring to anymore.
>>>>>>>>
>>>>>>>
>>>>>>>Sure! Will do.
>>>>>>>
>>>>>>
>>>>>>Would you prefer to have a set of actual reverts related to this:
>>>>>>
>>>>>>da692963df4e Revert "usb: gadget: uvc: add v4l2 enumeration api calls"
>>>>>>bca75df69aaf Revert "usb: gadget: uvc: add v4l2 try_format api call"
>>>>>>e56c767a6d3c Revert "usb: gadget: uvc: also use try_format 
>>>>>>in set_format"
>>>>>>20f275b86960 Revert "usb: gadget: uvc: fix try format returns on
>>>>>>uncompressed formats"
>>>>>>059d98f60c21 Revert "usb: gadget: uvc: Fix ERR_PTR dereference in
>>>>>>uvc_v4l2.c"
>>>>>>e6fd9b67414c Revert "usb: gadget: webcam: Make g_webcam 
>>>>>>loadable again"
>>>>>>
>>>>>>but have a negative consequence that the series isn't really 
>>>>>>bisectable from
>>>>>>functional perspective. For example commit e6fd9b67414c 
>>>>>>breaks g_uvc until
>>>>>>we apply da692963df4e so the series would have to go in as a whole.
>>>>>>
>>>>>>Or you would prefer a single commit that technically isn't a 
>>>>>>revert but it
>>>>>>just "undoes" the negative consequences of "usb: gadget: uvc: add v4l2
>>>>>>enumeration api calls" (kind of a squash of all commits above)?
>>>>>
>>>>>Ideally we can bisect at all places in the tree, so it's odd that
>>>>>reverting patches would cause problems as when adding them all should
>>>>>have been ok for every commit, right?
>>>>>
>>>>>But if there are merge issues, or other problems, then yes, maybe just
>>>>>one big one is needed, your choice.
>>>>
>>>>Won't a plain revert break userspace like GStreamer that have 
>>>>depended on
>>>>that patch for years ? In such a delicate case, wouldn't it be less
>>>>damageable to introduce workaround, like alias ? This is one personal
>>>>script against numerous users of a generic framework implementation.
>>>
>>>Shouldn't GStreamer be robust enough to handle cases of old-kernel 
>>>which haven't had this feature at all?
>>>
>>>The main reason why I reported this is not really the name 
>>>limitation but the fact that this patch is just incorrect for 
>>>cases where you would like to expose different formats at 
>>>different speeds. This feature was there for years and we do have 
>>>products that depend on it and this change along with the further 
>>>commits that depend on it broke that support for us.
>>>
>>>You are absolutely right that those commits added a feature that 
>>>now someone else may depend thus it would be perfect to fix it in 
>>>a way that doesn't break anyone's userspace but the problem is 
>>>that due to the way v4l2 API is designed I really don't see a way 
>>>how we could make it "speed-aware" without breaking all the users. 
>>>That's why we are kind of stuck between:
>>>
>>>1. Leave those commits in and then you cannot different streaming 
>>>headers for different speeds but users of those API will keep 
>>>working.
>>>
>>>2. Revert and bring back the feature of UVC ConfigFS interface 
>>>that was there since its inception but break the users of "New 
>>>API".
>>>
>>>>
>>>>I believe due to the delay, you are facing an unusual ABI 
>>>>breakage, which
>>>>requires a more delicate handling.
>>>
>>>Agree. The situation isn't simple as whatever we do would break 
>>>some userspace... I'm not an expert on v4l2, so if anyone with a 
>>>better knowledge of v4l2 internals has a suggestion how we could 
>>>make this work with the existing API I'd be more than happy to try 
>>>to follow that path.
>>
>>As a shortcomming compromise I would suggest to support both worlds by
>>conditionally set uvc->header if the directory h was found. If the
>>uvc->header was not set then we could print some info and disable the
>>main part of the possible uvc callbacks that depend on this uvc->header
>>objects.
>>
>>The only downside with that would be that using directory h in the
>>streaming header will implicitly create the limitations of not
>>indipendent formats per speed that Krzysztof mentioned.
>>
>>The alternative would be to put more effort into this and parse all
>>directories in the streaming header subnode. However since the idea of
>>using the v4l2-api is already talked dead by a long discussion history,
>>I would rather focus on transition the remaining functionality of the
>>gstreamer uvcgadget plugin to finally become independent of the v4l2api.
>>
>>How about that transition path?
>>
>
>If I understood you correctly, you are proposing that if the header 
>directories are named in a different way than h we would not set the 
>uvc->header pointer and make all the v4l2 callback that are using it 
>just return -ENOTSUP in this case? It would also mean that the 
>uvc_v4l2_set_format() would need to somehow be broad back to the 
>previous version which does not use -uvc_v4l2_try_format().

Yes.

>Although seems kind of hacky to change kernel behavior based on the 
>directory name I think if we add this to the doc this could be an 
>acceptable compromise that would make both scenarios work. 
>Alternatively, instead of basing that on the directory name maybe a 
>proper, named ConfigFS attribute somewhere inside UVC directory 
>"gstreamer_support" or something else would be more explicit for the 
>user?

This sounds even better. That way we would not depend on the hacky
directory behaviour. And documenting this attribute would be even less
shamefull.

That sounds like it all should work out.

Michael

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]