[RFC v4 0/8] v4l2: add support for colorspace conversion API (CSC) for video capture and subdevices

[RFC v4 0/8] v4l2: add support for colorspace conversion API (CSC) for video capture and subdevices

[Dafna Hirschfeld]

Quoted from patch "v4l2: add support for colorspace conversion API (CSC)
 for video capture and subdevices":
========================================================================

For video capture it is the driver that reports the colorspace,
Y'CbCr/HSV encoding, quantization range and transfer function
used by the video, and there is no way to request something
different, even though many HDTV receivers have some sort of
colorspace conversion capabilities.

For output video this feature already exists since the application
specifies this information for the video format it will send out, and
the transmitter will enable any available CSC if a format conversion has
to be performed in order to match the capabilities of the sink.

For video capture we propose adding new v4l2_pix_format flag:
V4L2_PIX_FMT_FLAG_SET_CSC. The flag is set by the application,
the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields
as the requested colorspace information and will attempt to
do the conversion it supports.

Drivers set the flags
V4L2_FMT_FLAG_CSC_YCBCR_ENC,
V4L2_FMT_FLAG_CSC_HSV_ENC,
V4L2_FMT_FLAG_CSC_QUANTIZATION,
in the flags field of the struct v4l2_fmtdesc during enumeration to
indicate that they support colorspace conversion for the respective field.
Currently the conversion of the fields 'colorspace' and 'xfer_func' is not
supported since there are no drivers that support it.

The same API is added for the subdevices. With the flag
V4L2_MBUS_FRAMEFMT_SET_CSC set by the application in VIDIOC_SUBDEV_S_FMT
ioctl and the flags V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC,
V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION set by the driver in the
VIDIOC_SUBDEV_ENUM_MBUS_CODE ioctl.

For subdevices, new 'flags' fields were added to the structs
v4l2_subdev_mbus_code_enum, v4l2_mbus_framefmt which are borrowed from the
'reserved' field

Drivers do not have to actually look at the flagsr. If the flags are not
set, then the colorspace, ycbcr_enc and quantization fields are set to
the default values by the core, i.e. just pass on the received format
without conversion.
========================================================================

The patchset includes a patch in vivid that implements the API for video capture devices
and 2 patches in rkisp1 driver that implement the API for subdevices.

Patches Summary:
----------------
patches 1,2 are bug fixes in rkisp1 driver, they are not part of the patchset
but the patchset needs those fixes for the rkisp1 implementation of the API.

patch 3 - moves the description of the v4l2_pix_format flags in the Documentation to the right
place where it should be. This is just a fix

patch 4 - Adds the API - Adds the new macros and fields and their documentation

patches 5,6 - uses the API in rkisp1 to allow userspace to change the quantization for YUV
formats. The support is added to the entities 'isp', 'resizer' 'capture' userspace
should make sure that the quantization matches along the pipe, otherwise the validation fails
when trying to stream.

patch 7 - uses the API in the capture device of vivid. Using the API, userspace is allowed to:
1. Set the ycbcr_enc function for YUV formats.
2. Set the hsv_enc function for HSV formats
3. Set the quantization for YUV and RGB formats.

patch 8 - adds the field 'hsv_enc' as a union together with 'ycbcr_enc' in struct v4l2_mbus_framefmt
and adds the flag V4L2_SUBDEV_MBUS_CODE_CSC_HSV_ENC to support the API for HSV media bus formats.

Changes from v3:
----------------
- patches 1,2,3,7,8 are new (rkisp1 bug fixes, move v4l2-pixfmt flags docs, vivid patch, support for HSV media bus)

- patch 4 (The API):
1) reformulate the Documentation according to comments from Verkuil and Tomasz Figa
2) remove the code in drivers/media/v4l2-core/v4l2-subdev.c and drivers/media/v4l2-core/v4l2-ioctl.c
so that except of the new macros and fields no new code is added to the core

- patches 5,6 - refactor the rkisp1 implementation so that the userspace should set the quantization separately
to each entity. In patch 6 the validation callbacks make sure the quantizations matche.

Dafna Hirschfeld (7):
  media: staging: rkisp1: rsz: supported formats are the isp's src
    formats, not sink formats
  media: staging: rkisp1: rsz: set default format if the given format is
    not RKISP1_DIR_SRC
  media: Documentation: v4l: move table of v4l2_pix_format(_mplane)
    flags to pixfmt-v4l2.rst
  media: staging: rkisp1: allow quantization conversion from userspace
    for isp source pad
  media: staging: rkisp1: validate quantization matching in
    link_validate callbacks
  media: vivid: Add support to the CSC API
  media: v4l2: add support for the CSC API for hsv_enc on mediabus

Philipp Zabel (1):
  v4l2: add support for colorspace conversion API (CSC) for video
    capture and subdevices

 .../media/v4l/pixfmt-reserved.rst             | 17 -----
 .../media/v4l/pixfmt-v4l2-mplane.rst          | 16 ++---
 .../userspace-api/media/v4l/pixfmt-v4l2.rst   | 63 ++++++++++++++++-
 .../media/v4l/subdev-formats.rst              | 70 ++++++++++++++++++-
 .../media/v4l/vidioc-enum-fmt.rst             | 22 +++++-
 .../v4l/vidioc-subdev-enum-mbus-code.rst      | 28 ++++++++
 .../media/videodev2.h.rst.exceptions          |  5 +-
 .../media/test-drivers/vivid/vivid-vid-cap.c  | 49 +++++++++++--
 .../test-drivers/vivid/vivid-vid-common.c     | 20 ++++++
 drivers/staging/media/rkisp1/rkisp1-capture.c | 26 ++++---
 drivers/staging/media/rkisp1/rkisp1-common.h  |  4 ++
 drivers/staging/media/rkisp1/rkisp1-isp.c     | 26 ++++---
 drivers/staging/media/rkisp1/rkisp1-resizer.c | 49 +++++++++++--
 include/uapi/linux/v4l2-mediabus.h            | 13 +++-
 include/uapi/linux/v4l2-subdev.h              |  6 +-
 include/uapi/linux/videodev2.h                |  4 ++
 16 files changed, 353 insertions(+), 65 deletions(-)

-- 
2.17.1

[RFC v4 1/8] media: staging: rkisp1: rsz: supported formats are the isp's src formats, not sink formats

[Dafna Hirschfeld]

The rkisp1_resizer's enum callback 'rkisp1_rsz_enum_mbus_code'
calls the enum callback of the 'rkisp1_isp' on it's video sink pad.
This is a bug, the resizer should support the same formats
supported by the 'rkisp1_isp' on the source pad (not the sink pad).

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 drivers/staging/media/rkisp1/rkisp1-resizer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
index d049374413dc..d64c064bdb1d 100644
--- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
+++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
@@ -437,8 +437,8 @@ static int rkisp1_rsz_enum_mbus_code(struct v4l2_subdev *sd,
 	u32 pad = code->pad;
 	int ret;
 
-	/* supported mbus codes are the same in isp sink pad */
-	code->pad = RKISP1_ISP_PAD_SINK_VIDEO;
+	/* supported mbus codes are the same in isp video src pad */
+	code->pad = RKISP1_ISP_PAD_SOURCE_VIDEO;
 	ret = v4l2_subdev_call(&rsz->rkisp1->isp.sd, pad, enum_mbus_code,
 			       &dummy_cfg, code);
 
-- 
2.17.1

[RFC v4 2/8] media: staging: rkisp1: rsz: set default format if the given format is not RKISP1_DIR_SRC

[Dafna Hirschfeld]

When setting the sink format of the 'rkisp1_resizer'
the format should be supported by 'rkisp1_isp' on
the video source pad. This patch checks this condition
and set the format to default if the condition is false.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 drivers/staging/media/rkisp1/rkisp1-common.h  | 4 ++++
 drivers/staging/media/rkisp1/rkisp1-isp.c     | 4 ----
 drivers/staging/media/rkisp1/rkisp1-resizer.c | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/media/rkisp1/rkisp1-common.h b/drivers/staging/media/rkisp1/rkisp1-common.h
index 0c4fe503adc9..39d8e46d8d8a 100644
--- a/drivers/staging/media/rkisp1/rkisp1-common.h
+++ b/drivers/staging/media/rkisp1/rkisp1-common.h
@@ -22,6 +22,10 @@
 #include "rkisp1-regs.h"
 #include "uapi/rkisp1-config.h"
 
+#define RKISP1_DIR_SRC BIT(0)
+#define RKISP1_DIR_SINK BIT(1)
+#define RKISP1_DIR_SINK_SRC (RKISP1_DIR_SINK | RKISP1_DIR_SRC)
+
 #define RKISP1_ISP_MAX_WIDTH		4032
 #define RKISP1_ISP_MAX_HEIGHT		3024
 #define RKISP1_ISP_MIN_WIDTH		32
diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c
index dc2b59a0160a..e66e87d6ea8b 100644
--- a/drivers/staging/media/rkisp1/rkisp1-isp.c
+++ b/drivers/staging/media/rkisp1/rkisp1-isp.c
@@ -23,10 +23,6 @@
 
 #define RKISP1_ISP_DEV_NAME	RKISP1_DRIVER_NAME "_isp"
 
-#define RKISP1_DIR_SRC BIT(0)
-#define RKISP1_DIR_SINK BIT(1)
-#define RKISP1_DIR_SINK_SRC (RKISP1_DIR_SINK | RKISP1_DIR_SRC)
-
 /*
  * NOTE: MIPI controller and input MUX are also configured in this file.
  * This is because ISP Subdev describes not only ISP submodule (input size,
diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
index d64c064bdb1d..fa28f4bd65c0 100644
--- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
+++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
@@ -542,7 +542,7 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
 					    which);
 	sink_fmt->code = format->code;
 	mbus_info = rkisp1_isp_mbus_info_get(sink_fmt->code);
-	if (!mbus_info) {
+	if (!mbus_info || !(mbus_info->direction & RKISP1_DIR_SRC)) {
 		sink_fmt->code = RKISP1_DEF_FMT;
 		mbus_info = rkisp1_isp_mbus_info_get(sink_fmt->code);
 	}
-- 
2.17.1

[RFC v4 3/8] media: Documentation: v4l: move table of v4l2_pix_format(_mplane) flags to pixfmt-v4l2.rst

[Dafna Hirschfeld]

The table of the flags of the structs
v4l2_pix_format(_mplane) is currently in pixfmt-reserved.rst
which is wrong, it should be in pixfmt-v4l2.rst

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 .../userspace-api/media/v4l/pixfmt-reserved.rst | 17 -----------------
 .../userspace-api/media/v4l/pixfmt-v4l2.rst     | 17 +++++++++++++++++
 .../media/videodev2.h.rst.exceptions            |  2 +-
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
index 59b9e7238f90..74ab6b5ce294 100644
--- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
+++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
@@ -263,20 +263,3 @@ please make a proposal on the linux-media mailing list.
 	of tiles, resulting in 32-aligned resolutions for the luminance plane
 	and 16-aligned resolutions for the chrominance plane (with 2x2
 	subsampling).
-
-.. tabularcolumns:: |p{6.6cm}|p{2.2cm}|p{8.7cm}|
-
-.. _format-flags:
-
-.. flat-table:: Format Flags
-    :header-rows:  0
-    :stub-columns: 0
-    :widths:       3 1 4
-
-    * - ``V4L2_PIX_FMT_FLAG_PREMUL_ALPHA``
-      - 0x00000001
-      - The color values are premultiplied by the alpha channel value. For
-	example, if a light blue pixel with 50% transparency was described
-	by RGBA values (128, 192, 255, 128), the same pixel described with
-	premultiplied colors would be described by RGBA values (64, 96,
-	128, 128)
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
index 759420a872d6..ffa539592822 100644
--- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
+++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
@@ -169,3 +169,20 @@ Single-planar format structure
         This information supplements the ``colorspace`` and must be set by
 	the driver for capture streams and by the application for output
 	streams, see :ref:`colorspaces`.
+
+.. tabularcolumns:: |p{6.6cm}|p{2.2cm}|p{8.7cm}|
+
+.. _format-flags:
+
+.. flat-table:: Format Flags
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       3 1 4
+
+    * - ``V4L2_PIX_FMT_FLAG_PREMUL_ALPHA``
+      - 0x00000001
+      - The color values are premultiplied by the alpha channel value. For
+        example, if a light blue pixel with 50% transparency was described
+	by RGBA values (128, 192, 255, 128), the same pixel described with
+	premultiplied colors would be described by RGBA values (64, 96,
+	128, 128)
diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
index a625fb90e3a9..564a3bf5bc6d 100644
--- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
+++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
@@ -180,7 +180,7 @@ replace define V4L2_CAP_IO_MC device-capabilities
 
 # V4L2 pix flags
 replace define V4L2_PIX_FMT_PRIV_MAGIC :c:type:`v4l2_pix_format`
-replace define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA reserved-formats
+replace define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA format-flags
 
 # V4L2 format flags
 replace define V4L2_FMT_FLAG_COMPRESSED fmtdesc-flags
-- 
2.17.1

[RFC v4 4/8] v4l2: add support for colorspace conversion API (CSC) for video capture and subdevices

[Dafna Hirschfeld]

From: Philipp Zabel <p.zabel@pengutronix.de>

For video capture it is the driver that reports the colorspace,
Y'CbCr/HSV encoding, quantization range and transfer function
used by the video, and there is no way to request something
different, even though many HDTV receivers have some sort of
colorspace conversion capabilities.

For output video this feature already exists since the application
specifies this information for the video format it will send out, and
the transmitter will enable any available CSC if a format conversion has
to be performed in order to match the capabilities of the sink.

For video capture we propose adding new v4l2_pix_format flag:
V4L2_PIX_FMT_FLAG_SET_CSC. The flag is set by the application,
the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields
as the requested colorspace information and will attempt to
do the conversion it supports.

Drivers set the flags
V4L2_FMT_FLAG_CSC_YCBCR_ENC,
V4L2_FMT_FLAG_CSC_HSV_ENC,
V4L2_FMT_FLAG_CSC_QUANTIZATION,
in the flags field of the struct v4l2_fmtdesc during enumeration to
indicate that they support colorspace conversion for the respective field.
Currently the conversion of the fields 'colorspace' and 'xfer_func' is not
supported since there are no drivers that support it.

The same API is added for the subdevices. With the flag
V4L2_MBUS_FRAMEFMT_SET_CSC set by the application in VIDIOC_SUBDEV_S_FMT
ioctl and the flags V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC,
V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION set by the driver in the
VIDIOC_SUBDEV_ENUM_MBUS_CODE ioctl.

For subdevices, new 'flags' fields were added to the structs
v4l2_subdev_mbus_code_enum, v4l2_mbus_framefmt which are borrowed from the
'reserved' field

Drivers do not have to actually look at the flagsr. If the flags are not
set, then the colorspace, ycbcr_enc and quantization fields are set to
the default values by the core, i.e. just pass on the received format
without conversion.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 .../media/v4l/pixfmt-v4l2-mplane.rst          | 16 ++----
 .../userspace-api/media/v4l/pixfmt-v4l2.rst   | 46 ++++++++++++++--
 .../media/v4l/subdev-formats.rst              | 52 +++++++++++++++++--
 .../media/v4l/vidioc-enum-fmt.rst             | 22 +++++++-
 .../v4l/vidioc-subdev-enum-mbus-code.rst      | 28 ++++++++++
 .../media/videodev2.h.rst.exceptions          |  3 ++
 include/uapi/linux/v4l2-mediabus.h            |  5 +-
 include/uapi/linux/v4l2-subdev.h              |  5 +-
 include/uapi/linux/videodev2.h                |  4 ++
 9 files changed, 160 insertions(+), 21 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst
index 444b4082684c..66f3365d7b72 100644
--- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst
+++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst
@@ -105,29 +105,21 @@ describing all planes of that format.
     * - __u8
       - ``ycbcr_enc``
       - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
-        This information supplements the ``colorspace`` and must be set by
-	the driver for capture streams and by the application for output
-	streams, see :ref:`colorspaces`.
+	See struct :c:type:`v4l2_pix_format`.
     * - __u8
       - ``hsv_enc``
       - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`.
-        This information supplements the ``colorspace`` and must be set by
-	the driver for capture streams and by the application for output
-	streams, see :ref:`colorspaces`.
+	See struct :c:type:`v4l2_pix_format`.
     * - }
       -
     * - __u8
       - ``quantization``
       - Quantization range, from enum :c:type:`v4l2_quantization`.
-        This information supplements the ``colorspace`` and must be set by
-	the driver for capture streams and by the application for output
-	streams, see :ref:`colorspaces`.
+	See struct :c:type:`v4l2_pix_format`.
     * - __u8
       - ``xfer_func``
       - Transfer function, from enum :c:type:`v4l2_xfer_func`.
-        This information supplements the ``colorspace`` and must be set by
-	the driver for capture streams and by the application for output
-	streams, see :ref:`colorspaces`.
+	See struct :c:type:`v4l2_pix_format`.
     * - __u8
       - ``reserved[7]``
       - Reserved for future extensions. Should be zeroed by drivers and
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
index ffa539592822..f23404efd90f 100644
--- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
+++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
@@ -148,13 +148,29 @@ Single-planar format structure
       - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
         This information supplements the ``colorspace`` and must be set by
 	the driver for capture streams and by the application for output
-	streams, see :ref:`colorspaces`.
+	streams, see :ref:`colorspaces`. If the application sets the
+	flag ``V4L2_PIX_FMT_FLAG_SET_CSC`` then the application can set
+	this field for a capture stream to request a specific Y'CbCr encoding
+	for the captured image data. If the driver cannot handle requested
+	conversion, it will return another supported encoding.
+	This field is ignored for HSV pixelformats. The driver indicates that
+	ycbcr_enc conversion is supported by setting the flag
+	V4L2_FMT_FLAG_CSC_YCBCR_ENC in the corresponding struct
+	:c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags`
     * - __u32
       - ``hsv_enc``
       - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`.
         This information supplements the ``colorspace`` and must be set by
 	the driver for capture streams and by the application for output
-	streams, see :ref:`colorspaces`.
+	streams, see :ref:`colorspaces`. If the application sets the flag
+	``V4L2_PIX_FMT_FLAG_SET_CSC`` then the application can set this
+	field for a capture stream to request a specific HSV encoding for the
+	captured image data. If the driver cannot handle requested
+	conversion, it will return another supported encoding.
+	This field is ignored for non-HSV pixelformats. The driver indicates
+	that hsv_enc conversion is supported by setting the flag
+	V4L2_FMT_FLAG_CSC_HSV_ENC in the corresponding struct
+	:c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags`
     * - }
       -
     * - __u32
@@ -162,7 +178,14 @@ Single-planar format structure
       - Quantization range, from enum :c:type:`v4l2_quantization`.
         This information supplements the ``colorspace`` and must be set by
 	the driver for capture streams and by the application for output
-	streams, see :ref:`colorspaces`.
+	streams, see :ref:`colorspaces`. If the application sets the flag
+	``V4L2_PIX_FMT_FLAG_SET_CSC`` then the application can set
+	this field for a capture stream to request a specific quantization
+	range for the captured image data. If the driver cannot handle requested
+	conversion, it will return another supported encoding.
+	The driver indicates that quantization conversion is supported by setting
+	the flag V4L2_FMT_FLAG_CSC_QUANTIZATION in the corresponding struct
+	:c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags`
     * - __u32
       - ``xfer_func``
       - Transfer function, from enum :c:type:`v4l2_xfer_func`.
@@ -186,3 +209,20 @@ Single-planar format structure
 	by RGBA values (128, 192, 255, 128), the same pixel described with
 	premultiplied colors would be described by RGBA values (64, 96,
 	128, 128)
+    * .. _`v4l2-pix-fmt-flag-set-csc`:
+
+      - ``V4L2_PIX_FMT_FLAG_SET_CSC``
+      - 0x00000002
+      - Set by the application. It is only used for capture and is
+        ignored for output streams. If set, then request the device to do
+	colorspace conversion from the received colorspace to the requested
+	colorspace values. If colorimetry field (``ycncr_enc``, ``hsv_enc``
+	or ``quantization``) is set to 0, then that colorimetry setting will
+	remain unchanged from what was received. So to change the quantization
+	only the ``quantization`` field shall be set to non-zero values
+	(``V4L2_QUANTIZATION_FULL_RANGE`` or ``V4L2_QUANTIZATION_LIM_RANGE``)
+	and all other colorimetry fields shall be set to 0. The API does not
+	support the conversion of the fields ``colorspace`` and ``xfer_func``
+
+	To check which conversions are supported by the hardware for the current
+	pixel format, see :ref:`fmtdesc-flags`.
diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
index 9a4d61b0d76f..75eb7f8bb4c5 100644
--- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
+++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
@@ -49,13 +49,32 @@ Media Bus Formats
       - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
         This information supplements the ``colorspace`` and must be set by
 	the driver for capture streams and by the application for output
-	streams, see :ref:`colorspaces`.
+	streams, see :ref:`colorspaces`. If the application sets the
+	flag ``V4L2_MBUS_FRAMEFMT_SET_CSC`` then the application can set
+	this field for a capture stream to request a specific Y'CbCr encoding
+	for the media bus data. If the driver cannot handle requested
+	conversion, it will return another supported encoding.
+	This field is ignored for HSV media bus formats. The driver indicates
+	that ycbcr_enc conversion is supported by setting the flag
+	V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC in the corresponding struct
+	:c:type:`v4l2_subdev_mbus_code_enum` during enumeration.
+	See :ref:`v4l2-subdev-mbus-code-flags`
+
     * - __u16
       - ``quantization``
       - Quantization range, from enum :c:type:`v4l2_quantization`.
         This information supplements the ``colorspace`` and must be set by
 	the driver for capture streams and by the application for output
-	streams, see :ref:`colorspaces`.
+	streams, see :ref:`colorspaces`. If the application sets the
+	flag ``V4L2_MBUS_FRAMEFMT_SET_CSC`` then the application can set
+	this field for a capture stream to request a specific quantization
+	encoding for the media bus data. If the driver cannot handle requested
+	conversion, it will return another supported encoding.
+	The driver indicates that quantization conversion is supported by
+	setting the flag V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION in the
+	corresponding struct :c:type:`v4l2_subdev_mbus_code_enum`
+	during enumeration. See :ref:`v4l2-subdev-mbus-code-flags`
+
     * - __u16
       - ``xfer_func``
       - Transfer function, from enum :c:type:`v4l2_xfer_func`.
@@ -63,10 +82,37 @@ Media Bus Formats
 	the driver for capture streams and by the application for output
 	streams, see :ref:`colorspaces`.
     * - __u16
-      - ``reserved``\ [11]
+      - ``flags``
+      - flags See:  :ref:v4l2-mbus-framefmt-flags
+    * - __u16
+      - ``reserved``\ [10]
       - Reserved for future extensions. Applications and drivers must set
 	the array to zero.
 
+.. _v4l2-mbus-framefmt-flags:
+
+.. flat-table:: v4l2_mbus_framefmt Flags
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       3 1 4
+
+    * .. _`mbus-framefmt-set-csc`:
+
+      - ``V4L2_MBUS_FRAMEFMT_SET_CSC``
+      - 0x0001
+      - Set by the application. It is only used for capture and is
+	ignored for output streams. If set, then request the subdevice to do
+	colorspace conversion from the received colorspace to the requested
+	colorspace values. If colorimetry field (``ycbcr_enc`` or
+	``quantization``) is set to 0, then that colorimetry setting will remain
+	unchanged from what was received. So to change the quantization, only the
+	``quantization`` field shall be set to non-zero values
+	(``V4L2_QUANTIZATION_FULL_RANGE`` or ``V4L2_QUANTIZATION_LIM_RANGE``)
+	and all other colorimetry fields shall be set to 0. The API does not
+	support the conversion of the fields ``colorspace`` and ``xfer_func``.
+
+	To check which conversions are supported by the hardware for the current
+	media bus frame format, see :ref:`v4l2-mbus-framefmt-flags`.
 
 
 .. _v4l2-mbus-pixelcode:
diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
index a53dd3d7f7e2..11323755d41b 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
@@ -178,7 +178,27 @@ the ``mbus_code`` field is handled differently:
 	parameters are detected. This flag can only be used in combination
 	with the ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to
 	compressed formats only. It is also only applies to stateful codecs.
-
+    * - ``V4L2_FMT_FLAG_CSC_YCBCR_ENC``
+      - 0x0010
+      - The driver allows the application to try to change the default
+	Y'CbCr encoding. This flag is relevant only for capture devices.
+	The application can ask to configure the ycbcr_enc of the capture device
+	when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
+	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
+    * - ``V4L2_FMT_FLAG_CSC_HSV_ENC``
+      - 0x0010
+      - The driver allows the application to try to change the default
+	HSV encoding. This flag is relevant only for capture devices.
+	The application can ask to configure the hsv_enc of the capture device
+	when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
+	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
+    * - ``V4L2_FMT_FLAG_CSC_QUANTIZATION``
+      - 0x0020
+      - The driver allows the application to try to change the default
+	quantization. This flag is relevant only for capture devices.
+	The application can ask to configure the quantization of the capture
+	device when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
+	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
 
 Return Value
 ============
diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
index 35b8607203a4..3d3430bdd71f 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
@@ -78,12 +78,40 @@ information about the try formats.
       - ``which``
       - Media bus format codes to be enumerated, from enum
 	:ref:`v4l2_subdev_format_whence <v4l2-subdev-format-whence>`.
+    * - __u32
+      - ``flags``
+      - See :ref:`v4l2-subdev-mbus-code-flags`
     * - __u32
       - ``reserved``\ [8]
       - Reserved for future extensions. Applications and drivers must set
 	the array to zero.
 
 
+
+.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{7.7cm}|
+
+.. _v4l2-subdev-mbus-code-flags:
+
+.. flat-table:: Subdev Media Bus Code Enumerate Flags
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC
+      - 0x00000001
+      - The driver allows the application to try to change the default Y'CbCr
+	encoding. The application can ask to configure the ycbcr_enc of the
+	subdevice when calling the :ref:`VIDIOC_SUBDEV_S_FMT <VIDIOC_SUBDEV_G_FMT>`
+	ioctl with :ref:`V4L2_MBUS_FRAMEFMT_SET_CSC <mbus-framefmt-set-csc>` set.
+	See :ref:`v4l2-mbus-format` on how to do this.
+    * - V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION
+      - 0x00000002
+      - The driver allows the application to try to change the default
+	quantization. The application can ask to configure the quantization of
+	the subdevice when calling the :ref:`VIDIOC_SUBDEV_S_FMT <VIDIOC_SUBDEV_G_FMT>`
+	ioctl with :ref:`V4L2_MBUS_FRAMEFMT_SET_CSC <mbus-framefmt-set-csc>` set.
+	See :ref:`v4l2-mbus-format` on how to do this.
+
 Return Value
 ============
 
diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
index 564a3bf5bc6d..f7be008cd479 100644
--- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
+++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
@@ -187,6 +187,9 @@ replace define V4L2_FMT_FLAG_COMPRESSED fmtdesc-flags
 replace define V4L2_FMT_FLAG_EMULATED fmtdesc-flags
 replace define V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM fmtdesc-flags
 replace define V4L2_FMT_FLAG_DYN_RESOLUTION fmtdesc-flags
+replace define V4L2_FMT_FLAG_CSC_YCBCR_ENC fmtdesc-flags
+replace define V4L2_FMT_FLAG_CSC_HSV_ENC fmtdesc-flags
+replace define V4L2_FMT_FLAG_CSC_QUANTIZATION fmtdesc-flags
 
 # V4L2 timecode types
 replace define V4L2_TC_TYPE_24FPS timecode-type
diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h
index 123a231001a8..0f916278137a 100644
--- a/include/uapi/linux/v4l2-mediabus.h
+++ b/include/uapi/linux/v4l2-mediabus.h
@@ -16,6 +16,8 @@
 #include <linux/types.h>
 #include <linux/videodev2.h>
 
+#define V4L2_MBUS_FRAMEFMT_SET_CSC	0x0001
+
 /**
  * struct v4l2_mbus_framefmt - frame format on the media bus
  * @width:	image width
@@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt {
 	__u16			ycbcr_enc;
 	__u16			quantization;
 	__u16			xfer_func;
-	__u16			reserved[11];
+	__u16			flags;
+	__u16			reserved[10];
 };
 
 #ifndef __KERNEL__
diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
index 5d2a1dab7911..972e64d8b54e 100644
--- a/include/uapi/linux/v4l2-subdev.h
+++ b/include/uapi/linux/v4l2-subdev.h
@@ -65,6 +65,8 @@ struct v4l2_subdev_crop {
 	__u32 reserved[8];
 };
 
+#define V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC	0x00000001
+#define V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION	0x00000002
 /**
  * struct v4l2_subdev_mbus_code_enum - Media bus format enumeration
  * @pad: pad number, as reported by the media API
@@ -77,7 +79,8 @@ struct v4l2_subdev_mbus_code_enum {
 	__u32 index;
 	__u32 code;
 	__u32 which;
-	__u32 reserved[8];
+	__u32 flags;
+	__u32 reserved[7];
 };
 
 /**
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index c3a1cf1c507f..15824316e0ca 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -774,6 +774,7 @@ struct v4l2_pix_format {
 
 /* Flags */
 #define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA	0x00000001
+#define V4L2_PIX_FMT_FLAG_SET_CSC	0x00000002
 
 /*
  *	F O R M A T   E N U M E R A T I O N
@@ -792,6 +793,9 @@ struct v4l2_fmtdesc {
 #define V4L2_FMT_FLAG_EMULATED			0x0002
 #define V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM	0x0004
 #define V4L2_FMT_FLAG_DYN_RESOLUTION		0x0008
+#define V4L2_FMT_FLAG_CSC_YCBCR_ENC		0x0010
+#define V4L2_FMT_FLAG_CSC_HSV_ENC		0x0010
+#define V4L2_FMT_FLAG_CSC_QUANTIZATION		0x0020
 
 	/* Frame Size and frame rate enumeration */
 /*
-- 
2.17.1

[RFC v4 5/8] media: staging: rkisp1: allow quantization conversion from userspace for isp source pad

[Dafna Hirschfeld]

The isp entity has a hardware support to force full range quantization
for YUV formats. Use the CSC API to allow userspace to set the
quantization for YUV formats on the isp, resizer and capture entities.

- The flag V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION is set on
YUV formats during enumeration of the resizer and isp formats.
- The flag V4L2_FMT_FLAG_CSC_QUANTIZATION is set on the YUV formats
during enumeration of the capture formats.
- The full quantization is set on YUV formats if userspace ask it
on the entities using the flag V4L2_MBUS_FRAMEFMT_SET_CSC
for subdevices and the flag V4L2_PIX_FMT_FLAG_SET_CSC on for
the capture entity.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 drivers/staging/media/rkisp1/rkisp1-capture.c | 23 ++++++++++++-------
 drivers/staging/media/rkisp1/rkisp1-isp.c     | 22 ++++++++++++++----
 drivers/staging/media/rkisp1/rkisp1-resizer.c | 22 ++++++++++++++++++
 3 files changed, 55 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
index f69235f82c45..66856d5eb576 100644
--- a/drivers/staging/media/rkisp1/rkisp1-capture.c
+++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
@@ -1085,8 +1085,6 @@ static void rkisp1_try_fmt(const struct rkisp1_capture *cap,
 			   const struct v4l2_format_info **fmt_info)
 {
 	const struct rkisp1_capture_config *config = cap->config;
-	struct rkisp1_capture *other_cap =
-			&cap->rkisp1->capture_devs[cap->id ^ 1];
 	const struct rkisp1_capture_fmt_cfg *fmt;
 	const struct v4l2_format_info *info;
 	const unsigned int max_widths[] = { RKISP1_RSZ_MP_SRC_MAX_WIDTH,
@@ -1108,16 +1106,21 @@ static void rkisp1_try_fmt(const struct rkisp1_capture *cap,
 	pixm->field = V4L2_FIELD_NONE;
 	pixm->colorspace = V4L2_COLORSPACE_DEFAULT;
 	pixm->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+	pixm->xfer_func = V4L2_XFER_FUNC_DEFAULT;
 
 	info = rkisp1_fill_pixfmt(pixm, cap->id);
 
-	/* can not change quantization when stream-on */
-	if (other_cap->is_streaming)
-		pixm->quantization = other_cap->pix.fmt.quantization;
-	/* output full range by default, take effect in params */
-	else if (!pixm->quantization ||
-		 pixm->quantization > V4L2_QUANTIZATION_LIM_RANGE)
+	/*
+	 * isp has a feature to select between limited and full range for YUV
+	 * formats.
+	 * So we should also support it in the capture using the CSC API
+	 */
+	if (pixm->flags & V4L2_PIX_FMT_FLAG_SET_CSC &&
+	    pixm->quantization == V4L2_QUANTIZATION_FULL_RANGE &&
+	    v4l2_is_format_yuv(info))
 		pixm->quantization = V4L2_QUANTIZATION_FULL_RANGE;
+	else
+		pixm->quantization = V4L2_QUANTIZATION_DEFAULT;
 
 	if (fmt_cfg)
 		*fmt_cfg = fmt;
@@ -1152,12 +1155,16 @@ static int rkisp1_enum_fmt_vid_cap_mplane(struct file *file, void *priv,
 {
 	struct rkisp1_capture *cap = video_drvdata(file);
 	const struct rkisp1_capture_fmt_cfg *fmt = NULL;
+	const struct v4l2_format_info *info;
 
 	if (f->index >= cap->config->fmt_size)
 		return -EINVAL;
 
 	fmt = &cap->config->fmts[f->index];
 	f->pixelformat = fmt->fourcc;
+	info = v4l2_format_info(f->pixelformat);
+	if (v4l2_is_format_yuv(info))
+		f->flags = V4L2_FMT_FLAG_CSC_QUANTIZATION;
 
 	return 0;
 }
diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c
index e66e87d6ea8b..28e0a3c594aa 100644
--- a/drivers/staging/media/rkisp1/rkisp1-isp.c
+++ b/drivers/staging/media/rkisp1/rkisp1-isp.c
@@ -591,6 +591,10 @@ static int rkisp1_isp_enum_mbus_code(struct v4l2_subdev *sd,
 
 		if (code->index == pos - 1) {
 			code->code = fmt->mbus_code;
+			if (fmt->pixel_enc == V4L2_PIXEL_ENC_YUV &&
+			    dir == RKISP1_DIR_SRC)
+				code->flags =
+					V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION;
 			return 0;
 		}
 	}
@@ -622,7 +626,6 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
 					     RKISP1_ISP_PAD_SOURCE_VIDEO);
 	*src_fmt = *sink_fmt;
 	src_fmt->code = RKISP1_DEF_SRC_PAD_FMT;
-	src_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
 
 	src_crop = v4l2_subdev_get_try_crop(sd, cfg,
 					    RKISP1_ISP_PAD_SOURCE_VIDEO);
@@ -665,10 +668,21 @@ static void rkisp1_isp_set_src_fmt(struct rkisp1_isp *isp,
 		isp->src_fmt = mbus_info;
 	src_fmt->width  = src_crop->width;
 	src_fmt->height = src_crop->height;
-	src_fmt->quantization = format->quantization;
-	/* full range by default */
-	if (!src_fmt->quantization)
+
+	src_fmt->colorspace = V4L2_COLORSPACE_DEFAULT;
+	src_fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+	src_fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
+
+	/*
+	 * The CSC API is used to allow userspace to force full
+	 * quantization on YUV formats.
+	 */
+	if (format->flags & V4L2_MBUS_FRAMEFMT_SET_CSC &&
+	    format->quantization == V4L2_QUANTIZATION_FULL_RANGE &&
+	    mbus_info->pixel_enc == V4L2_PIXEL_ENC_YUV)
 		src_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
+	else
+		src_fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
 
 	*format = *src_fmt;
 }
diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
index fa28f4bd65c0..237cce9183f7 100644
--- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
+++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
@@ -549,8 +549,30 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
 	if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
 		rsz->pixel_enc = mbus_info->pixel_enc;
 
+	sink_fmt->colorspace = V4L2_COLORSPACE_DEFAULT;
+	sink_fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+	sink_fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
+
+	/*
+	 * isp has a feature to select between limited and full range for
+	 * YUV formats.
+	 * so we should support it in the resizer using the CSC API
+	 */
+
+	if (format->flags & V4L2_MBUS_FRAMEFMT_SET_CSC &&
+	    format->quantization == V4L2_QUANTIZATION_FULL_RANGE &&
+	    mbus_info->pixel_enc == V4L2_PIXEL_ENC_YUV)
+		sink_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
+	else
+		sink_fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
+
 	/* Propagete to source pad */
 	src_fmt->code = sink_fmt->code;
+	src_fmt->field = sink_fmt->field;
+	src_fmt->colorspace = sink_fmt->colorspace;
+	src_fmt->ycbcr_enc = sink_fmt->ycbcr_enc;
+	src_fmt->xfer_func = sink_fmt->xfer_func;
+	src_fmt->quantization = sink_fmt->quantization;
 
 	sink_fmt->width = clamp_t(u32, format->width,
 				  rsz->config->min_rsz_width,
-- 
2.17.1

[RFC v4 6/8] media: staging: rkisp1: validate quantization matching in link_validate callbacks

[Dafna Hirschfeld]

The quantization of the rkisp1 entities can be set by userspace
using the CSC API. Therefore we validate that the
quantization field matches on the links in the link_validate
callbacks.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 drivers/staging/media/rkisp1/rkisp1-capture.c |  3 ++-
 drivers/staging/media/rkisp1/rkisp1-resizer.c | 21 ++++++++++++++++++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
index 66856d5eb576..3eb2ea1a9eb1 100644
--- a/drivers/staging/media/rkisp1/rkisp1-capture.c
+++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
@@ -1263,7 +1263,8 @@ static int rkisp1_capture_link_validate(struct media_link *link)
 		return ret;
 
 	if (sd_fmt.format.height != cap->pix.fmt.height ||
-	    sd_fmt.format.width != cap->pix.fmt.width)
+	    sd_fmt.format.width != cap->pix.fmt.width ||
+	    sd_fmt.format.quantization != cap->pix.fmt.quantization)
 		return -EPIPE;
 
 	return 0;
diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
index 237cce9183f7..027396b00124 100644
--- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
+++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
@@ -671,6 +671,25 @@ static int rkisp1_rsz_set_selection(struct v4l2_subdev *sd,
 	return 0;
 }
 
+int rkisp1_rsz_link_validate(struct v4l2_subdev *sd, struct media_link *link,
+			     struct v4l2_subdev_format *source_fmt,
+			     struct v4l2_subdev_format *sink_fmt)
+{
+	int ret;
+
+	ret = v4l2_subdev_link_validate_default(sd, link, source_fmt, sink_fmt);
+	if (ret)
+		return ret;
+	if (source_fmt->format.quantization != sink_fmt->format.quantization) {
+		struct device *dev = link->graph_obj.mdev->dev;
+
+		dev_warn(dev, "isp->resizer link validation failed, ");
+		dev_warn(dev, "quantizations don't match\n");
+		return -EPIPE;
+	}
+	return 0;
+}
+
 static const struct media_entity_operations rkisp1_rsz_media_ops = {
 	.link_validate = v4l2_subdev_link_validate,
 };
@@ -682,7 +701,7 @@ static const struct v4l2_subdev_pad_ops rkisp1_rsz_pad_ops = {
 	.init_cfg = rkisp1_rsz_init_config,
 	.get_fmt = rkisp1_rsz_get_fmt,
 	.set_fmt = rkisp1_rsz_set_fmt,
-	.link_validate = v4l2_subdev_link_validate_default,
+	.link_validate = rkisp1_rsz_link_validate,
 };
 
 /* ----------------------------------------------------------------------------
-- 
2.17.1

[RFC v4 7/8] media: vivid: Add support to the CSC API

[Dafna Hirschfeld]

The CSC API (Colorspace conversion) allows userspace to try
to configure the ycbcr/hsv_enc function and the quantization
for capture devices. This patch adds support to the CSC API
in vivid.
Using the CSC API, userspace is allowed to do the following:

1. Set the ycbcr_enc function for YUV formats.
2. Set the hsv_enc function for HSV formats
3. Set the quantization for YUV and RGB formats.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 .../media/test-drivers/vivid/vivid-vid-cap.c  | 49 +++++++++++++++++--
 .../test-drivers/vivid/vivid-vid-common.c     | 20 ++++++++
 2 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
index e94beef008c8..8a43d7ebe53f 100644
--- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c
+++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
@@ -549,6 +549,29 @@ int vivid_g_fmt_vid_cap(struct file *file, void *priv,
 	return 0;
 }
 
+static bool vivid_is_hsv_enc_valid(__u8 hsv_enc)
+{
+	if (hsv_enc == V4L2_HSV_ENC_180 || hsv_enc == V4L2_HSV_ENC_256)
+		return true;
+	return false;
+}
+
+static bool vivid_is_ycbcr_enc_valid(__u8 ycbcr_enc)
+{
+	/* V4L2_YCBCR_ENC_SMPTE240M is the last ycbcr_enc enum */
+	if (ycbcr_enc && ycbcr_enc <= V4L2_YCBCR_ENC_SMPTE240M)
+		return true;
+	return false;
+}
+
+static bool vivid_is_quant_valid(__u8 quantization)
+{
+	if (quantization == V4L2_QUANTIZATION_FULL_RANGE ||
+	    quantization == V4L2_QUANTIZATION_LIM_RANGE)
+		return true;
+	return false;
+}
+
 int vivid_try_fmt_vid_cap(struct file *file, void *priv,
 			struct v4l2_format *f)
 {
@@ -560,6 +583,7 @@ int vivid_try_fmt_vid_cap(struct file *file, void *priv,
 	unsigned factor = 1;
 	unsigned w, h;
 	unsigned p;
+	bool user_set_csc = !!(mp->flags & V4L2_PIX_FMT_FLAG_SET_CSC);
 
 	fmt = vivid_get_format(dev, mp->pixelformat);
 	if (!fmt) {
@@ -634,12 +658,23 @@ int vivid_try_fmt_vid_cap(struct file *file, void *priv,
 			(fmt->bit_depth[0] / fmt->vdownsampling[0]);
 
 	mp->colorspace = vivid_colorspace_cap(dev);
-	if (fmt->color_enc == TGP_COLOR_ENC_HSV)
-		mp->hsv_enc = vivid_hsv_enc_cap(dev);
-	else
+	if (fmt->color_enc == TGP_COLOR_ENC_HSV) {
+		if (!user_set_csc || !vivid_is_hsv_enc_valid(mp->hsv_enc))
+			mp->hsv_enc = vivid_hsv_enc_cap(dev);
+	} else if (fmt->color_enc == TGP_COLOR_ENC_YCBCR) {
+		if (!user_set_csc || !vivid_is_ycbcr_enc_valid(mp->ycbcr_enc))
+			mp->ycbcr_enc = vivid_ycbcr_enc_cap(dev);
+	} else {
 		mp->ycbcr_enc = vivid_ycbcr_enc_cap(dev);
+	}
 	mp->xfer_func = vivid_xfer_func_cap(dev);
-	mp->quantization = vivid_quantization_cap(dev);
+	if (fmt->color_enc == TGP_COLOR_ENC_YCBCR ||
+	    fmt->color_enc == TGP_COLOR_ENC_RGB) {
+		if (!user_set_csc || !vivid_is_quant_valid(mp->quantization))
+			mp->quantization = vivid_quantization_cap(dev);
+	} else {
+		mp->quantization = vivid_quantization_cap(dev);
+	}
 	memset(mp->reserved, 0, sizeof(mp->reserved));
 	return 0;
 }
@@ -769,6 +804,12 @@ int vivid_s_fmt_vid_cap(struct file *file, void *priv,
 	if (vivid_is_sdtv_cap(dev))
 		dev->tv_field_cap = mp->field;
 	tpg_update_mv_step(&dev->tpg);
+	dev->tpg.quantization = mp->quantization;
+	if (dev->fmt_cap->color_enc == TGP_COLOR_ENC_YCBCR)
+		dev->tpg.ycbcr_enc = mp->ycbcr_enc;
+	else
+		dev->tpg.hsv_enc = mp->hsv_enc;
+
 	return 0;
 }
 
diff --git a/drivers/media/test-drivers/vivid/vivid-vid-common.c b/drivers/media/test-drivers/vivid/vivid-vid-common.c
index 76b0be670ebb..19aacb180e67 100644
--- a/drivers/media/test-drivers/vivid/vivid-vid-common.c
+++ b/drivers/media/test-drivers/vivid/vivid-vid-common.c
@@ -920,6 +920,26 @@ int vivid_enum_fmt_vid(struct file *file, void  *priv,
 	fmt = &vivid_formats[f->index];
 
 	f->pixelformat = fmt->fourcc;
+
+	if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
+	    f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+		return 0;
+	/*
+	 * For capture devices, we support the CSC API.
+	 * We allow userspace to:
+	 * 1. set the ycbcr_enc on yuv format
+	 * 2. set the hsv_enc on hsv format
+	 * 3. set the quantization on yuv and rgb formats
+	 */
+	if (fmt->color_enc == TGP_COLOR_ENC_YCBCR) {
+		f->flags |= V4L2_FMT_FLAG_CSC_YCBCR_ENC;
+		f->flags |= V4L2_FMT_FLAG_CSC_QUANTIZATION;
+	} else if (fmt->color_enc == TGP_COLOR_ENC_HSV) {
+		f->flags |= V4L2_FMT_FLAG_CSC_HSV_ENC;
+	} else if (fmt->color_enc == TGP_COLOR_ENC_RGB) {
+		f->flags |= V4L2_FMT_FLAG_CSC_QUANTIZATION;
+	}
+
 	return 0;
 }
 
-- 
2.17.1

[RFC v4 8/8] media: v4l2: add support for the CSC API for hsv_enc on mediabus

[Dafna Hirschfeld]

Add the flag V4L2_SUBDEV_MBUS_CODE_CSC_HSV_ENC that drivers
can set when enumerating the supported mediabus formats
on subdevices to indicate that the userspace can ask to
set the 'hsv_enc'. The patch also replaces the 'ycbcr_enc'
field in 'struct v4l2_mbus_framefmt' with a union that
includes 'hsv_enc'

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 .../media/v4l/subdev-formats.rst              | 22 +++++++++++++++++--
 include/uapi/linux/v4l2-mediabus.h            |  8 ++++++-
 include/uapi/linux/v4l2-subdev.h              |  1 +
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
index 75eb7f8bb4c5..d06d1ac95452 100644
--- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
+++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
@@ -44,6 +44,8 @@ Media Bus Formats
       - Image colorspace, from enum
 	:c:type:`v4l2_colorspace`. See
 	:ref:`colorspaces` for details.
+    * - union {
+      - (anonymous)
     * - __u16
       - ``ycbcr_enc``
       - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
@@ -59,7 +61,23 @@ Media Bus Formats
 	V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC in the corresponding struct
 	:c:type:`v4l2_subdev_mbus_code_enum` during enumeration.
 	See :ref:`v4l2-subdev-mbus-code-flags`
-
+    * - __u16
+      - ``hsv_enc``
+      - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`.
+        This information supplements the ``colorspace`` and must be set by
+	the driver for capture streams and by the application for output
+	streams, see :ref:`colorspaces`. If the application sets the
+	flag ``V4L2_MBUS_FRAMEFMT_SET_CSC`` then the application can set
+	this field for a capture stream to request a specific HSV encoding
+	for the media bus data. If the driver cannot handle requested
+	conversion, it will return another supported encoding.
+	This field is ignored for Y'CbCr media bus formats. The driver indicates
+	that hsv_enc conversion is supported by setting the flag
+	V4L2_SUBDEV_MBUS_CODE_CSC_HSV_ENC in the corresponding struct
+	:c:type:`v4l2_subdev_mbus_code_enum` during enumeration.
+	See :ref:`v4l2-subdev-mbus-code-flags`
+    * - }
+      -
     * - __u16
       - ``quantization``
       - Quantization range, from enum :c:type:`v4l2_quantization`.
@@ -103,7 +121,7 @@ Media Bus Formats
       - Set by the application. It is only used for capture and is
 	ignored for output streams. If set, then request the subdevice to do
 	colorspace conversion from the received colorspace to the requested
-	colorspace values. If colorimetry field (``ycbcr_enc`` or
+	colorspace values. If colorimetry field (``ycbcr_enc``, ``hsv_enc`` or
 	``quantization``) is set to 0, then that colorimetry setting will remain
 	unchanged from what was received. So to change the quantization, only the
 	``quantization`` field shall be set to non-zero values
diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h
index 0f916278137a..9c4a10d21a49 100644
--- a/include/uapi/linux/v4l2-mediabus.h
+++ b/include/uapi/linux/v4l2-mediabus.h
@@ -26,6 +26,7 @@
  * @field:	used interlacing type (from enum v4l2_field)
  * @colorspace:	colorspace of the data (from enum v4l2_colorspace)
  * @ycbcr_enc:	YCbCr encoding of the data (from enum v4l2_ycbcr_encoding)
+ * @hsv_enc:	HSV encoding of the data (from enum v4l2_hsv_encoding)
  * @quantization: quantization of the data (from enum v4l2_quantization)
  * @xfer_func:  transfer function of the data (from enum v4l2_xfer_func)
  */
@@ -35,7 +36,12 @@ struct v4l2_mbus_framefmt {
 	__u32			code;
 	__u32			field;
 	__u32			colorspace;
-	__u16			ycbcr_enc;
+	union {
+		/* enum v4l2_ycbcr_encoding */
+		__u16			ycbcr_enc;
+		/* enum v4l2_hsv_encoding */
+		__u16			hsv_enc;
+	};
 	__u16			quantization;
 	__u16			xfer_func;
 	__u16			flags;
diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
index 972e64d8b54e..685f920de5c7 100644
--- a/include/uapi/linux/v4l2-subdev.h
+++ b/include/uapi/linux/v4l2-subdev.h
@@ -66,6 +66,7 @@ struct v4l2_subdev_crop {
 };
 
 #define V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC	0x00000001
+#define V4L2_SUBDEV_MBUS_CODE_CSC_HSV_ENC	0x00000001
 #define V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION	0x00000002
 /**
  * struct v4l2_subdev_mbus_code_enum - Media bus format enumeration
-- 
2.17.1

Re: [RFC v4 5/8] media: staging: rkisp1: allow quantization conversion from userspace for isp source pad

[Dafna Hirschfeld]

On 05.06.20 19:26, Dafna Hirschfeld wrote:
> The isp entity has a hardware support to force full range quantization
> for YUV formats. Use the CSC API to allow userspace to set the
> quantization for YUV formats on the isp, resizer and capture entities.
> 
> - The flag V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION is set on
> YUV formats during enumeration of the resizer and isp formats.
> - The flag V4L2_FMT_FLAG_CSC_QUANTIZATION is set on the YUV formats
> during enumeration of the capture formats.
> - The full quantization is set on YUV formats if userspace ask it
> on the entities using the flag V4L2_MBUS_FRAMEFMT_SET_CSC
> for subdevices and the flag V4L2_PIX_FMT_FLAG_SET_CSC on for
> the capture entity.
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
>   drivers/staging/media/rkisp1/rkisp1-capture.c | 23 ++++++++++++-------
>   drivers/staging/media/rkisp1/rkisp1-isp.c     | 22 ++++++++++++++----
>   drivers/staging/media/rkisp1/rkisp1-resizer.c | 22 ++++++++++++++++++
>   3 files changed, 55 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
> index f69235f82c45..66856d5eb576 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-capture.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
> @@ -1085,8 +1085,6 @@ static void rkisp1_try_fmt(const struct rkisp1_capture *cap,
>   			   const struct v4l2_format_info **fmt_info)
>   {
>   	const struct rkisp1_capture_config *config = cap->config;
> -	struct rkisp1_capture *other_cap =
> -			&cap->rkisp1->capture_devs[cap->id ^ 1];
>   	const struct rkisp1_capture_fmt_cfg *fmt;
>   	const struct v4l2_format_info *info;
>   	const unsigned int max_widths[] = { RKISP1_RSZ_MP_SRC_MAX_WIDTH,
> @@ -1108,16 +1106,21 @@ static void rkisp1_try_fmt(const struct rkisp1_capture *cap,
>   	pixm->field = V4L2_FIELD_NONE;
>   	pixm->colorspace = V4L2_COLORSPACE_DEFAULT;
>   	pixm->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> +	pixm->xfer_func = V4L2_XFER_FUNC_DEFAULT;
>   
>   	info = rkisp1_fill_pixfmt(pixm, cap->id);
>   
> -	/* can not change quantization when stream-on */
> -	if (other_cap->is_streaming)
> -		pixm->quantization = other_cap->pix.fmt.quantization;
> -	/* output full range by default, take effect in params */
> -	else if (!pixm->quantization ||
> -		 pixm->quantization > V4L2_QUANTIZATION_LIM_RANGE)
> +	/*
> +	 * isp has a feature to select between limited and full range for YUV
> +	 * formats.
> +	 * So we should also support it in the capture using the CSC API
> +	 */
> +	if (pixm->flags & V4L2_PIX_FMT_FLAG_SET_CSC &&
> +	    pixm->quantization == V4L2_QUANTIZATION_FULL_RANGE &&
> +	    v4l2_is_format_yuv(info))
>   		pixm->quantization = V4L2_QUANTIZATION_FULL_RANGE;


I notice now that this assignment makes no sense since we already check two lines
above that the pixm->quantization is full-range, I'll fix it in v5.
The logic is anyway correct.

Thanks,
Dafna

> +	else
> +		pixm->quantization = V4L2_QUANTIZATION_DEFAULT;
>   
>   	if (fmt_cfg)
>   		*fmt_cfg = fmt;
> @@ -1152,12 +1155,16 @@ static int rkisp1_enum_fmt_vid_cap_mplane(struct file *file, void *priv,
>   {
>   	struct rkisp1_capture *cap = video_drvdata(file);
>   	const struct rkisp1_capture_fmt_cfg *fmt = NULL;
> +	const struct v4l2_format_info *info;
>   
>   	if (f->index >= cap->config->fmt_size)
>   		return -EINVAL;
>   
>   	fmt = &cap->config->fmts[f->index];
>   	f->pixelformat = fmt->fourcc;
> +	info = v4l2_format_info(f->pixelformat);
> +	if (v4l2_is_format_yuv(info))
> +		f->flags = V4L2_FMT_FLAG_CSC_QUANTIZATION;
>   
>   	return 0;
>   }
> diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c
> index e66e87d6ea8b..28e0a3c594aa 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-isp.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-isp.c
> @@ -591,6 +591,10 @@ static int rkisp1_isp_enum_mbus_code(struct v4l2_subdev *sd,
>   
>   		if (code->index == pos - 1) {
>   			code->code = fmt->mbus_code;
> +			if (fmt->pixel_enc == V4L2_PIXEL_ENC_YUV &&
> +			    dir == RKISP1_DIR_SRC)
> +				code->flags =
> +					V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION;
>   			return 0;
>   		}
>   	}
> @@ -622,7 +626,6 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
>   					     RKISP1_ISP_PAD_SOURCE_VIDEO);
>   	*src_fmt = *sink_fmt;
>   	src_fmt->code = RKISP1_DEF_SRC_PAD_FMT;
> -	src_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
>   
>   	src_crop = v4l2_subdev_get_try_crop(sd, cfg,
>   					    RKISP1_ISP_PAD_SOURCE_VIDEO);
> @@ -665,10 +668,21 @@ static void rkisp1_isp_set_src_fmt(struct rkisp1_isp *isp,
>   		isp->src_fmt = mbus_info;
>   	src_fmt->width  = src_crop->width;
>   	src_fmt->height = src_crop->height;
> -	src_fmt->quantization = format->quantization;
> -	/* full range by default */
> -	if (!src_fmt->quantization)
> +
> +	src_fmt->colorspace = V4L2_COLORSPACE_DEFAULT;
> +	src_fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> +	src_fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> +
> +	/*
> +	 * The CSC API is used to allow userspace to force full
> +	 * quantization on YUV formats.
> +	 */
> +	if (format->flags & V4L2_MBUS_FRAMEFMT_SET_CSC &&
> +	    format->quantization == V4L2_QUANTIZATION_FULL_RANGE &&
> +	    mbus_info->pixel_enc == V4L2_PIXEL_ENC_YUV)
>   		src_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE> +	else
> +		src_fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
>   
>   	*format = *src_fmt;
>   }
> diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
> index fa28f4bd65c0..237cce9183f7 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
> @@ -549,8 +549,30 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
>   	if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
>   		rsz->pixel_enc = mbus_info->pixel_enc;
>   
> +	sink_fmt->colorspace = V4L2_COLORSPACE_DEFAULT;
> +	sink_fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> +	sink_fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> +
> +	/*
> +	 * isp has a feature to select between limited and full range for
> +	 * YUV formats.
> +	 * so we should support it in the resizer using the CSC API
> +	 */
> +
> +	if (format->flags & V4L2_MBUS_FRAMEFMT_SET_CSC &&
> +	    format->quantization == V4L2_QUANTIZATION_FULL_RANGE &&
> +	    mbus_info->pixel_enc == V4L2_PIXEL_ENC_YUV)
> +		sink_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
> +	else
> +		sink_fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
> +
>   	/* Propagete to source pad */
>   	src_fmt->code = sink_fmt->code;
> +	src_fmt->field = sink_fmt->field;
> +	src_fmt->colorspace = sink_fmt->colorspace;
> +	src_fmt->ycbcr_enc = sink_fmt->ycbcr_enc;
> +	src_fmt->xfer_func = sink_fmt->xfer_func;
> +	src_fmt->quantization = sink_fmt->quantization;
>   
>   	sink_fmt->width = clamp_t(u32, format->width,
>   				  rsz->config->min_rsz_width,
> 

Re: [RFC v4 4/8] v4l2: add support for colorspace conversion API (CSC) for video capture and subdevices

[Hans Verkuil]

On 05/06/2020 19:26, Dafna Hirschfeld wrote:
> From: Philipp Zabel <p.zabel@pengutronix.de>
> 
> For video capture it is the driver that reports the colorspace,
> Y'CbCr/HSV encoding, quantization range and transfer function
> used by the video, and there is no way to request something
> different, even though many HDTV receivers have some sort of
> colorspace conversion capabilities.
> 
> For output video this feature already exists since the application
> specifies this information for the video format it will send out, and
> the transmitter will enable any available CSC if a format conversion has
> to be performed in order to match the capabilities of the sink.
> 
> For video capture we propose adding new v4l2_pix_format flag:
> V4L2_PIX_FMT_FLAG_SET_CSC. The flag is set by the application,
> the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields
> as the requested colorspace information and will attempt to
> do the conversion it supports.
> 
> Drivers set the flags
> V4L2_FMT_FLAG_CSC_YCBCR_ENC,
> V4L2_FMT_FLAG_CSC_HSV_ENC,
> V4L2_FMT_FLAG_CSC_QUANTIZATION,
> in the flags field of the struct v4l2_fmtdesc during enumeration to
> indicate that they support colorspace conversion for the respective field.
> Currently the conversion of the fields 'colorspace' and 'xfer_func' is not
> supported since there are no drivers that support it.
> 
> The same API is added for the subdevices. With the flag
> V4L2_MBUS_FRAMEFMT_SET_CSC set by the application in VIDIOC_SUBDEV_S_FMT
> ioctl and the flags V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC,
> V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION set by the driver in the
> VIDIOC_SUBDEV_ENUM_MBUS_CODE ioctl.
> 
> For subdevices, new 'flags' fields were added to the structs
> v4l2_subdev_mbus_code_enum, v4l2_mbus_framefmt which are borrowed from the
> 'reserved' field
> 
> Drivers do not have to actually look at the flagsr. If the flags are not

flagsr -> flags

> set, then the colorspace, ycbcr_enc and quantization fields are set to
> the default values by the core, i.e. just pass on the received format
> without conversion.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
>  .../media/v4l/pixfmt-v4l2-mplane.rst          | 16 ++----
>  .../userspace-api/media/v4l/pixfmt-v4l2.rst   | 46 ++++++++++++++--
>  .../media/v4l/subdev-formats.rst              | 52 +++++++++++++++++--
>  .../media/v4l/vidioc-enum-fmt.rst             | 22 +++++++-
>  .../v4l/vidioc-subdev-enum-mbus-code.rst      | 28 ++++++++++
>  .../media/videodev2.h.rst.exceptions          |  3 ++
>  include/uapi/linux/v4l2-mediabus.h            |  5 +-
>  include/uapi/linux/v4l2-subdev.h              |  5 +-
>  include/uapi/linux/videodev2.h                |  4 ++
>  9 files changed, 160 insertions(+), 21 deletions(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst
> index 444b4082684c..66f3365d7b72 100644
> --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst
> +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst
> @@ -105,29 +105,21 @@ describing all planes of that format.
>      * - __u8
>        - ``ycbcr_enc``
>        - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
> -        This information supplements the ``colorspace`` and must be set by
> -	the driver for capture streams and by the application for output
> -	streams, see :ref:`colorspaces`.
> +	See struct :c:type:`v4l2_pix_format`.
>      * - __u8
>        - ``hsv_enc``
>        - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`.
> -        This information supplements the ``colorspace`` and must be set by
> -	the driver for capture streams and by the application for output
> -	streams, see :ref:`colorspaces`.
> +	See struct :c:type:`v4l2_pix_format`.
>      * - }
>        -
>      * - __u8
>        - ``quantization``
>        - Quantization range, from enum :c:type:`v4l2_quantization`.
> -        This information supplements the ``colorspace`` and must be set by
> -	the driver for capture streams and by the application for output
> -	streams, see :ref:`colorspaces`.
> +	See struct :c:type:`v4l2_pix_format`.
>      * - __u8
>        - ``xfer_func``
>        - Transfer function, from enum :c:type:`v4l2_xfer_func`.
> -        This information supplements the ``colorspace`` and must be set by
> -	the driver for capture streams and by the application for output
> -	streams, see :ref:`colorspaces`.
> +	See struct :c:type:`v4l2_pix_format`.
>      * - __u8
>        - ``reserved[7]``
>        - Reserved for future extensions. Should be zeroed by drivers and
> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
> index ffa539592822..f23404efd90f 100644
> --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
> +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
> @@ -148,13 +148,29 @@ Single-planar format structure
>        - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
>          This information supplements the ``colorspace`` and must be set by
>  	the driver for capture streams and by the application for output
> -	streams, see :ref:`colorspaces`.
> +	streams, see :ref:`colorspaces`. If the application sets the
> +	flag ``V4L2_PIX_FMT_FLAG_SET_CSC`` then the application can set
> +	this field for a capture stream to request a specific Y'CbCr encoding
> +	for the captured image data. If the driver cannot handle requested
> +	conversion, it will return another supported encoding.
> +	This field is ignored for HSV pixelformats. The driver indicates that
> +	ycbcr_enc conversion is supported by setting the flag
> +	V4L2_FMT_FLAG_CSC_YCBCR_ENC in the corresponding struct
> +	:c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags`

Missing period at the end.

>      * - __u32
>        - ``hsv_enc``
>        - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`.
>          This information supplements the ``colorspace`` and must be set by
>  	the driver for capture streams and by the application for output
> -	streams, see :ref:`colorspaces`.
> +	streams, see :ref:`colorspaces`. If the application sets the flag
> +	``V4L2_PIX_FMT_FLAG_SET_CSC`` then the application can set this
> +	field for a capture stream to request a specific HSV encoding for the
> +	captured image data. If the driver cannot handle requested
> +	conversion, it will return another supported encoding.
> +	This field is ignored for non-HSV pixelformats. The driver indicates
> +	that hsv_enc conversion is supported by setting the flag
> +	V4L2_FMT_FLAG_CSC_HSV_ENC in the corresponding struct
> +	:c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags`

Missing period at the end.

>      * - }
>        -
>      * - __u32
> @@ -162,7 +178,14 @@ Single-planar format structure
>        - Quantization range, from enum :c:type:`v4l2_quantization`.
>          This information supplements the ``colorspace`` and must be set by
>  	the driver for capture streams and by the application for output
> -	streams, see :ref:`colorspaces`.
> +	streams, see :ref:`colorspaces`. If the application sets the flag
> +	``V4L2_PIX_FMT_FLAG_SET_CSC`` then the application can set
> +	this field for a capture stream to request a specific quantization
> +	range for the captured image data. If the driver cannot handle requested
> +	conversion, it will return another supported encoding.
> +	The driver indicates that quantization conversion is supported by setting
> +	the flag V4L2_FMT_FLAG_CSC_QUANTIZATION in the corresponding struct
> +	:c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags`

Missing period at the end.

>      * - __u32
>        - ``xfer_func``
>        - Transfer function, from enum :c:type:`v4l2_xfer_func`.
> @@ -186,3 +209,20 @@ Single-planar format structure
>  	by RGBA values (128, 192, 255, 128), the same pixel described with
>  	premultiplied colors would be described by RGBA values (64, 96,
>  	128, 128)
> +    * .. _`v4l2-pix-fmt-flag-set-csc`:
> +
> +      - ``V4L2_PIX_FMT_FLAG_SET_CSC``
> +      - 0x00000002
> +      - Set by the application. It is only used for capture and is
> +        ignored for output streams. If set, then request the device to do
> +	colorspace conversion from the received colorspace to the requested
> +	colorspace values. If colorimetry field (``ycncr_enc``, ``hsv_enc``

ycncr_enc -> ycbcr_enc

> +	or ``quantization``) is set to 0, then that colorimetry setting will
> +	remain unchanged from what was received. So to change the quantization
> +	only the ``quantization`` field shall be set to non-zero values
> +	(``V4L2_QUANTIZATION_FULL_RANGE`` or ``V4L2_QUANTIZATION_LIM_RANGE``)
> +	and all other colorimetry fields shall be set to 0. The API does not

does not -> does not (yet)

> +	support the conversion of the fields ``colorspace`` and ``xfer_func``

Missing period at the end.

> +
> +	To check which conversions are supported by the hardware for the current
> +	pixel format, see :ref:`fmtdesc-flags`.
> diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> index 9a4d61b0d76f..75eb7f8bb4c5 100644
> --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> @@ -49,13 +49,32 @@ Media Bus Formats
>        - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
>          This information supplements the ``colorspace`` and must be set by
>  	the driver for capture streams and by the application for output
> -	streams, see :ref:`colorspaces`.
> +	streams, see :ref:`colorspaces`. If the application sets the
> +	flag ``V4L2_MBUS_FRAMEFMT_SET_CSC`` then the application can set
> +	this field for a capture stream to request a specific Y'CbCr encoding
> +	for the media bus data. If the driver cannot handle requested
> +	conversion, it will return another supported encoding.
> +	This field is ignored for HSV media bus formats. The driver indicates
> +	that ycbcr_enc conversion is supported by setting the flag
> +	V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC in the corresponding struct
> +	:c:type:`v4l2_subdev_mbus_code_enum` during enumeration.
> +	See :ref:`v4l2-subdev-mbus-code-flags`

Missing period at the end.

> +
>      * - __u16
>        - ``quantization``
>        - Quantization range, from enum :c:type:`v4l2_quantization`.
>          This information supplements the ``colorspace`` and must be set by
>  	the driver for capture streams and by the application for output
> -	streams, see :ref:`colorspaces`.
> +	streams, see :ref:`colorspaces`. If the application sets the
> +	flag ``V4L2_MBUS_FRAMEFMT_SET_CSC`` then the application can set
> +	this field for a capture stream to request a specific quantization
> +	encoding for the media bus data. If the driver cannot handle requested
> +	conversion, it will return another supported encoding.
> +	The driver indicates that quantization conversion is supported by
> +	setting the flag V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION in the
> +	corresponding struct :c:type:`v4l2_subdev_mbus_code_enum`
> +	during enumeration. See :ref:`v4l2-subdev-mbus-code-flags`

Missing period at the end.

> +
>      * - __u16
>        - ``xfer_func``
>        - Transfer function, from enum :c:type:`v4l2_xfer_func`.
> @@ -63,10 +82,37 @@ Media Bus Formats
>  	the driver for capture streams and by the application for output
>  	streams, see :ref:`colorspaces`.
>      * - __u16
> -      - ``reserved``\ [11]
> +      - ``flags``
> +      - flags See:  :ref:v4l2-mbus-framefmt-flags
> +    * - __u16
> +      - ``reserved``\ [10]
>        - Reserved for future extensions. Applications and drivers must set
>  	the array to zero.
>  
> +.. _v4l2-mbus-framefmt-flags:
> +
> +.. flat-table:: v4l2_mbus_framefmt Flags
> +    :header-rows:  0
> +    :stub-columns: 0
> +    :widths:       3 1 4
> +
> +    * .. _`mbus-framefmt-set-csc`:
> +
> +      - ``V4L2_MBUS_FRAMEFMT_SET_CSC``
> +      - 0x0001
> +      - Set by the application. It is only used for capture and is
> +	ignored for output streams. If set, then request the subdevice to do
> +	colorspace conversion from the received colorspace to the requested
> +	colorspace values. If colorimetry field (``ycbcr_enc`` or
> +	``quantization``) is set to 0, then that colorimetry setting will remain
> +	unchanged from what was received. So to change the quantization, only the
> +	``quantization`` field shall be set to non-zero values
> +	(``V4L2_QUANTIZATION_FULL_RANGE`` or ``V4L2_QUANTIZATION_LIM_RANGE``)
> +	and all other colorimetry fields shall be set to 0. The API does not
> +	support the conversion of the fields ``colorspace`` and ``xfer_func``.
> +
> +	To check which conversions are supported by the hardware for the current
> +	media bus frame format, see :ref:`v4l2-mbus-framefmt-flags`.
>  
>  
>  .. _v4l2-mbus-pixelcode:
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> index a53dd3d7f7e2..11323755d41b 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> @@ -178,7 +178,27 @@ the ``mbus_code`` field is handled differently:
>  	parameters are detected. This flag can only be used in combination
>  	with the ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to
>  	compressed formats only. It is also only applies to stateful codecs.
> -
> +    * - ``V4L2_FMT_FLAG_CSC_YCBCR_ENC``
> +      - 0x0010
> +      - The driver allows the application to try to change the default
> +	Y'CbCr encoding. This flag is relevant only for capture devices.
> +	The application can ask to configure the ycbcr_enc of the capture device
> +	when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
> +	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
> +    * - ``V4L2_FMT_FLAG_CSC_HSV_ENC``
> +      - 0x0010
> +      - The driver allows the application to try to change the default
> +	HSV encoding. This flag is relevant only for capture devices.
> +	The application can ask to configure the hsv_enc of the capture device
> +	when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
> +	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
> +    * - ``V4L2_FMT_FLAG_CSC_QUANTIZATION``
> +      - 0x0020
> +      - The driver allows the application to try to change the default
> +	quantization. This flag is relevant only for capture devices.
> +	The application can ask to configure the quantization of the capture
> +	device when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
> +	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
>  
>  Return Value
>  ============
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
> index 35b8607203a4..3d3430bdd71f 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
> @@ -78,12 +78,40 @@ information about the try formats.
>        - ``which``
>        - Media bus format codes to be enumerated, from enum
>  	:ref:`v4l2_subdev_format_whence <v4l2-subdev-format-whence>`.
> +    * - __u32
> +      - ``flags``
> +      - See :ref:`v4l2-subdev-mbus-code-flags`
>      * - __u32
>        - ``reserved``\ [8]

'[8]' should now be changed to '[7]'.

>        - Reserved for future extensions. Applications and drivers must set
>  	the array to zero.
>  
>  
> +
> +.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{7.7cm}|
> +
> +.. _v4l2-subdev-mbus-code-flags:
> +
> +.. flat-table:: Subdev Media Bus Code Enumerate Flags
> +    :header-rows:  0
> +    :stub-columns: 0
> +    :widths:       1 1 2
> +
> +    * - V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC
> +      - 0x00000001
> +      - The driver allows the application to try to change the default Y'CbCr
> +	encoding. The application can ask to configure the ycbcr_enc of the
> +	subdevice when calling the :ref:`VIDIOC_SUBDEV_S_FMT <VIDIOC_SUBDEV_G_FMT>`
> +	ioctl with :ref:`V4L2_MBUS_FRAMEFMT_SET_CSC <mbus-framefmt-set-csc>` set.
> +	See :ref:`v4l2-mbus-format` on how to do this.
> +    * - V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION
> +      - 0x00000002
> +      - The driver allows the application to try to change the default
> +	quantization. The application can ask to configure the quantization of
> +	the subdevice when calling the :ref:`VIDIOC_SUBDEV_S_FMT <VIDIOC_SUBDEV_G_FMT>`
> +	ioctl with :ref:`V4L2_MBUS_FRAMEFMT_SET_CSC <mbus-framefmt-set-csc>` set.
> +	See :ref:`v4l2-mbus-format` on how to do this.
> +
>  Return Value
>  ============
>  
> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> index 564a3bf5bc6d..f7be008cd479 100644
> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> @@ -187,6 +187,9 @@ replace define V4L2_FMT_FLAG_COMPRESSED fmtdesc-flags
>  replace define V4L2_FMT_FLAG_EMULATED fmtdesc-flags
>  replace define V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM fmtdesc-flags
>  replace define V4L2_FMT_FLAG_DYN_RESOLUTION fmtdesc-flags
> +replace define V4L2_FMT_FLAG_CSC_YCBCR_ENC fmtdesc-flags
> +replace define V4L2_FMT_FLAG_CSC_HSV_ENC fmtdesc-flags
> +replace define V4L2_FMT_FLAG_CSC_QUANTIZATION fmtdesc-flags
>  
>  # V4L2 timecode types
>  replace define V4L2_TC_TYPE_24FPS timecode-type
> diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h
> index 123a231001a8..0f916278137a 100644
> --- a/include/uapi/linux/v4l2-mediabus.h
> +++ b/include/uapi/linux/v4l2-mediabus.h
> @@ -16,6 +16,8 @@
>  #include <linux/types.h>
>  #include <linux/videodev2.h>
>  
> +#define V4L2_MBUS_FRAMEFMT_SET_CSC	0x0001
> +
>  /**
>   * struct v4l2_mbus_framefmt - frame format on the media bus
>   * @width:	image width
> @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt {
>  	__u16			ycbcr_enc;
>  	__u16			quantization;
>  	__u16			xfer_func;
> -	__u16			reserved[11];
> +	__u16			flags;
> +	__u16			reserved[10];
>  };
>  
>  #ifndef __KERNEL__
> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
> index 5d2a1dab7911..972e64d8b54e 100644
> --- a/include/uapi/linux/v4l2-subdev.h
> +++ b/include/uapi/linux/v4l2-subdev.h
> @@ -65,6 +65,8 @@ struct v4l2_subdev_crop {
>  	__u32 reserved[8];
>  };
>  
> +#define V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC	0x00000001
> +#define V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION	0x00000002
>  /**
>   * struct v4l2_subdev_mbus_code_enum - Media bus format enumeration
>   * @pad: pad number, as reported by the media API
> @@ -77,7 +79,8 @@ struct v4l2_subdev_mbus_code_enum {
>  	__u32 index;
>  	__u32 code;
>  	__u32 which;
> -	__u32 reserved[8];
> +	__u32 flags;
> +	__u32 reserved[7];
>  };
>  
>  /**
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index c3a1cf1c507f..15824316e0ca 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -774,6 +774,7 @@ struct v4l2_pix_format {
>  
>  /* Flags */
>  #define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA	0x00000001
> +#define V4L2_PIX_FMT_FLAG_SET_CSC	0x00000002
>  
>  /*
>   *	F O R M A T   E N U M E R A T I O N
> @@ -792,6 +793,9 @@ struct v4l2_fmtdesc {
>  #define V4L2_FMT_FLAG_EMULATED			0x0002
>  #define V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM	0x0004
>  #define V4L2_FMT_FLAG_DYN_RESOLUTION		0x0008
> +#define V4L2_FMT_FLAG_CSC_YCBCR_ENC		0x0010
> +#define V4L2_FMT_FLAG_CSC_HSV_ENC		0x0010
> +#define V4L2_FMT_FLAG_CSC_QUANTIZATION		0x0020
>  
>  	/* Frame Size and frame rate enumeration */
>  /*
> 

Regards,

	Hans

Re: [RFC v4 7/8] media: vivid: Add support to the CSC API

[Hans Verkuil]

On 05/06/2020 19:26, Dafna Hirschfeld wrote:
> The CSC API (Colorspace conversion) allows userspace to try
> to configure the ycbcr/hsv_enc function and the quantization
> for capture devices. This patch adds support to the CSC API
> in vivid.
> Using the CSC API, userspace is allowed to do the following:
> 
> 1. Set the ycbcr_enc function for YUV formats.
> 2. Set the hsv_enc function for HSV formats
> 3. Set the quantization for YUV and RGB formats.

I just realized something. We excluded colorspace and xfer_func from the CSC API
for now since there are no drivers that support those conversions. But actually,
that's not true since vivid *does* support this unless it is in loopback mode
(then no CSC conversion is supported).

So if dev->loop_video is true, then disable CSC conversion in vivid.

Otherwise CSC can be fully enabled for all four colorimetry fields.

So I think we should add flags to signal this for colorspace and xfer_func as well
and implement this in vivid.

Sorry about this, I should have realized this (much) earlier.

Regards,

	Hans

> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
>  .../media/test-drivers/vivid/vivid-vid-cap.c  | 49 +++++++++++++++++--
>  .../test-drivers/vivid/vivid-vid-common.c     | 20 ++++++++
>  2 files changed, 65 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
> index e94beef008c8..8a43d7ebe53f 100644
> --- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c
> +++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
> @@ -549,6 +549,29 @@ int vivid_g_fmt_vid_cap(struct file *file, void *priv,
>  	return 0;
>  }
>  
> +static bool vivid_is_hsv_enc_valid(__u8 hsv_enc)
> +{
> +	if (hsv_enc == V4L2_HSV_ENC_180 || hsv_enc == V4L2_HSV_ENC_256)
> +		return true;
> +	return false;
> +}
> +
> +static bool vivid_is_ycbcr_enc_valid(__u8 ycbcr_enc)
> +{
> +	/* V4L2_YCBCR_ENC_SMPTE240M is the last ycbcr_enc enum */
> +	if (ycbcr_enc && ycbcr_enc <= V4L2_YCBCR_ENC_SMPTE240M)
> +		return true;
> +	return false;
> +}
> +
> +static bool vivid_is_quant_valid(__u8 quantization)
> +{
> +	if (quantization == V4L2_QUANTIZATION_FULL_RANGE ||
> +	    quantization == V4L2_QUANTIZATION_LIM_RANGE)
> +		return true;
> +	return false;
> +}
> +
>  int vivid_try_fmt_vid_cap(struct file *file, void *priv,
>  			struct v4l2_format *f)
>  {
> @@ -560,6 +583,7 @@ int vivid_try_fmt_vid_cap(struct file *file, void *priv,
>  	unsigned factor = 1;
>  	unsigned w, h;
>  	unsigned p;
> +	bool user_set_csc = !!(mp->flags & V4L2_PIX_FMT_FLAG_SET_CSC);
>  
>  	fmt = vivid_get_format(dev, mp->pixelformat);
>  	if (!fmt) {
> @@ -634,12 +658,23 @@ int vivid_try_fmt_vid_cap(struct file *file, void *priv,
>  			(fmt->bit_depth[0] / fmt->vdownsampling[0]);
>  
>  	mp->colorspace = vivid_colorspace_cap(dev);
> -	if (fmt->color_enc == TGP_COLOR_ENC_HSV)
> -		mp->hsv_enc = vivid_hsv_enc_cap(dev);
> -	else
> +	if (fmt->color_enc == TGP_COLOR_ENC_HSV) {
> +		if (!user_set_csc || !vivid_is_hsv_enc_valid(mp->hsv_enc))
> +			mp->hsv_enc = vivid_hsv_enc_cap(dev);
> +	} else if (fmt->color_enc == TGP_COLOR_ENC_YCBCR) {
> +		if (!user_set_csc || !vivid_is_ycbcr_enc_valid(mp->ycbcr_enc))
> +			mp->ycbcr_enc = vivid_ycbcr_enc_cap(dev);
> +	} else {
>  		mp->ycbcr_enc = vivid_ycbcr_enc_cap(dev);
> +	}
>  	mp->xfer_func = vivid_xfer_func_cap(dev);
> -	mp->quantization = vivid_quantization_cap(dev);
> +	if (fmt->color_enc == TGP_COLOR_ENC_YCBCR ||
> +	    fmt->color_enc == TGP_COLOR_ENC_RGB) {
> +		if (!user_set_csc || !vivid_is_quant_valid(mp->quantization))
> +			mp->quantization = vivid_quantization_cap(dev);
> +	} else {
> +		mp->quantization = vivid_quantization_cap(dev);
> +	}
>  	memset(mp->reserved, 0, sizeof(mp->reserved));
>  	return 0;
>  }
> @@ -769,6 +804,12 @@ int vivid_s_fmt_vid_cap(struct file *file, void *priv,
>  	if (vivid_is_sdtv_cap(dev))
>  		dev->tv_field_cap = mp->field;
>  	tpg_update_mv_step(&dev->tpg);
> +	dev->tpg.quantization = mp->quantization;
> +	if (dev->fmt_cap->color_enc == TGP_COLOR_ENC_YCBCR)
> +		dev->tpg.ycbcr_enc = mp->ycbcr_enc;
> +	else
> +		dev->tpg.hsv_enc = mp->hsv_enc;
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/media/test-drivers/vivid/vivid-vid-common.c b/drivers/media/test-drivers/vivid/vivid-vid-common.c
> index 76b0be670ebb..19aacb180e67 100644
> --- a/drivers/media/test-drivers/vivid/vivid-vid-common.c
> +++ b/drivers/media/test-drivers/vivid/vivid-vid-common.c
> @@ -920,6 +920,26 @@ int vivid_enum_fmt_vid(struct file *file, void  *priv,
>  	fmt = &vivid_formats[f->index];
>  
>  	f->pixelformat = fmt->fourcc;
> +
> +	if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> +	    f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> +		return 0;
> +	/*
> +	 * For capture devices, we support the CSC API.
> +	 * We allow userspace to:
> +	 * 1. set the ycbcr_enc on yuv format
> +	 * 2. set the hsv_enc on hsv format
> +	 * 3. set the quantization on yuv and rgb formats
> +	 */
> +	if (fmt->color_enc == TGP_COLOR_ENC_YCBCR) {
> +		f->flags |= V4L2_FMT_FLAG_CSC_YCBCR_ENC;
> +		f->flags |= V4L2_FMT_FLAG_CSC_QUANTIZATION;
> +	} else if (fmt->color_enc == TGP_COLOR_ENC_HSV) {
> +		f->flags |= V4L2_FMT_FLAG_CSC_HSV_ENC;
> +	} else if (fmt->color_enc == TGP_COLOR_ENC_RGB) {
> +		f->flags |= V4L2_FMT_FLAG_CSC_QUANTIZATION;
> +	}
> +
>  	return 0;
>  }
>  
> 

Re: [RFC v4 2/8] media: staging: rkisp1: rsz: set default format if the given format is not RKISP1_DIR_SRC

[Hans Verkuil]

On 05/06/2020 19:26, Dafna Hirschfeld wrote:
> When setting the sink format of the 'rkisp1_resizer'
> the format should be supported by 'rkisp1_isp' on
> the video source pad. This patch checks this condition
> and set the format to default if the condition is false.
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
>  drivers/staging/media/rkisp1/rkisp1-common.h  | 4 ++++
>  drivers/staging/media/rkisp1/rkisp1-isp.c     | 4 ----
>  drivers/staging/media/rkisp1/rkisp1-resizer.c | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/media/rkisp1/rkisp1-common.h b/drivers/staging/media/rkisp1/rkisp1-common.h
> index 0c4fe503adc9..39d8e46d8d8a 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-common.h
> +++ b/drivers/staging/media/rkisp1/rkisp1-common.h
> @@ -22,6 +22,10 @@
>  #include "rkisp1-regs.h"
>  #include "uapi/rkisp1-config.h"
>  
> +#define RKISP1_DIR_SRC BIT(0)
> +#define RKISP1_DIR_SINK BIT(1)
> +#define RKISP1_DIR_SINK_SRC (RKISP1_DIR_SINK | RKISP1_DIR_SRC)
> +
>  #define RKISP1_ISP_MAX_WIDTH		4032
>  #define RKISP1_ISP_MAX_HEIGHT		3024
>  #define RKISP1_ISP_MIN_WIDTH		32
> diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c
> index dc2b59a0160a..e66e87d6ea8b 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-isp.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-isp.c
> @@ -23,10 +23,6 @@
>  
>  #define RKISP1_ISP_DEV_NAME	RKISP1_DRIVER_NAME "_isp"
>  
> -#define RKISP1_DIR_SRC BIT(0)
> -#define RKISP1_DIR_SINK BIT(1)
> -#define RKISP1_DIR_SINK_SRC (RKISP1_DIR_SINK | RKISP1_DIR_SRC)

Helen had a comment about this in the original patch (https://patchwork.linuxtv.org/patch/64292/).

Can you process that comment for the v5 of this series?

Regards,

	Hans

> -
>  /*
>   * NOTE: MIPI controller and input MUX are also configured in this file.
>   * This is because ISP Subdev describes not only ISP submodule (input size,
> diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
> index d64c064bdb1d..fa28f4bd65c0 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
> @@ -542,7 +542,7 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
>  					    which);
>  	sink_fmt->code = format->code;
>  	mbus_info = rkisp1_isp_mbus_info_get(sink_fmt->code);
> -	if (!mbus_info) {
> +	if (!mbus_info || !(mbus_info->direction & RKISP1_DIR_SRC)) {
>  		sink_fmt->code = RKISP1_DEF_FMT;
>  		mbus_info = rkisp1_isp_mbus_info_get(sink_fmt->code);
>  	}
> 

Re: [RFC v4 1/8] media: staging: rkisp1: rsz: supported formats are the isp's src formats, not sink formats

[Hans Verkuil]

On 05/06/2020 19:26, Dafna Hirschfeld wrote:
> The rkisp1_resizer's enum callback 'rkisp1_rsz_enum_mbus_code'
> calls the enum callback of the 'rkisp1_isp' on it's video sink pad.
> This is a bug, the resizer should support the same formats
> supported by the 'rkisp1_isp' on the source pad (not the sink pad).
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>

Please include Helen's Ack when you post a v5.

(https://patchwork.linuxtv.org/patch/64291/)

For that matter, shouldn't this be a 'Fixes:' patch with a Cc to stable?

Regards,

	Hans

> ---
>  drivers/staging/media/rkisp1/rkisp1-resizer.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
> index d049374413dc..d64c064bdb1d 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
> @@ -437,8 +437,8 @@ static int rkisp1_rsz_enum_mbus_code(struct v4l2_subdev *sd,
>  	u32 pad = code->pad;
>  	int ret;
>  
> -	/* supported mbus codes are the same in isp sink pad */
> -	code->pad = RKISP1_ISP_PAD_SINK_VIDEO;
> +	/* supported mbus codes are the same in isp video src pad */
> +	code->pad = RKISP1_ISP_PAD_SOURCE_VIDEO;
>  	ret = v4l2_subdev_call(&rsz->rkisp1->isp.sd, pad, enum_mbus_code,
>  			       &dummy_cfg, code);
>  
> 

Re: [RFC v4 7/8] media: vivid: Add support to the CSC API

[Dafna Hirschfeld]

On 08.06.20 12:10, Hans Verkuil wrote:
> On 05/06/2020 19:26, Dafna Hirschfeld wrote:
>> The CSC API (Colorspace conversion) allows userspace to try
>> to configure the ycbcr/hsv_enc function and the quantization
>> for capture devices. This patch adds support to the CSC API
>> in vivid.
>> Using the CSC API, userspace is allowed to do the following:
>>
>> 1. Set the ycbcr_enc function for YUV formats.
>> 2. Set the hsv_enc function for HSV formats
>> 3. Set the quantization for YUV and RGB formats.
> 
> I just realized something. We excluded colorspace and xfer_func from the CSC API
> for now since there are no drivers that support those conversions. But actually,
> that's not true since vivid *does* support this unless it is in loopback mode
> (then no CSC conversion is supported).

But shouldn't there be at least one real hardware device that support it in order
to justify it in the API?

Thanks,
Dafna

> 
> So if dev->loop_video is true, then disable CSC conversion in vivid.
> 
> Otherwise CSC can be fully enabled for all four colorimetry fields.
> 
> So I think we should add flags to signal this for colorspace and xfer_func as well
> and implement this in vivid.
> 
> Sorry about this, I should have realized this (much) earlier.
> 
> Regards,
> 
> 	Hans
> 
>>
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>> ---
>>   .../media/test-drivers/vivid/vivid-vid-cap.c  | 49 +++++++++++++++++--
>>   .../test-drivers/vivid/vivid-vid-common.c     | 20 ++++++++
>>   2 files changed, 65 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
>> index e94beef008c8..8a43d7ebe53f 100644
>> --- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c
>> +++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
>> @@ -549,6 +549,29 @@ int vivid_g_fmt_vid_cap(struct file *file, void *priv,
>>   	return 0;
>>   }
>>   
>> +static bool vivid_is_hsv_enc_valid(__u8 hsv_enc)
>> +{
>> +	if (hsv_enc == V4L2_HSV_ENC_180 || hsv_enc == V4L2_HSV_ENC_256)
>> +		return true;
>> +	return false;
>> +}
>> +
>> +static bool vivid_is_ycbcr_enc_valid(__u8 ycbcr_enc)
>> +{
>> +	/* V4L2_YCBCR_ENC_SMPTE240M is the last ycbcr_enc enum */
>> +	if (ycbcr_enc && ycbcr_enc <= V4L2_YCBCR_ENC_SMPTE240M)
>> +		return true;
>> +	return false;
>> +}
>> +
>> +static bool vivid_is_quant_valid(__u8 quantization)
>> +{
>> +	if (quantization == V4L2_QUANTIZATION_FULL_RANGE ||
>> +	    quantization == V4L2_QUANTIZATION_LIM_RANGE)
>> +		return true;
>> +	return false;
>> +}
>> +
>>   int vivid_try_fmt_vid_cap(struct file *file, void *priv,
>>   			struct v4l2_format *f)
>>   {
>> @@ -560,6 +583,7 @@ int vivid_try_fmt_vid_cap(struct file *file, void *priv,
>>   	unsigned factor = 1;
>>   	unsigned w, h;
>>   	unsigned p;
>> +	bool user_set_csc = !!(mp->flags & V4L2_PIX_FMT_FLAG_SET_CSC);
>>   
>>   	fmt = vivid_get_format(dev, mp->pixelformat);
>>   	if (!fmt) {
>> @@ -634,12 +658,23 @@ int vivid_try_fmt_vid_cap(struct file *file, void *priv,
>>   			(fmt->bit_depth[0] / fmt->vdownsampling[0]);
>>   
>>   	mp->colorspace = vivid_colorspace_cap(dev);
>> -	if (fmt->color_enc == TGP_COLOR_ENC_HSV)
>> -		mp->hsv_enc = vivid_hsv_enc_cap(dev);
>> -	else
>> +	if (fmt->color_enc == TGP_COLOR_ENC_HSV) {
>> +		if (!user_set_csc || !vivid_is_hsv_enc_valid(mp->hsv_enc))
>> +			mp->hsv_enc = vivid_hsv_enc_cap(dev);
>> +	} else if (fmt->color_enc == TGP_COLOR_ENC_YCBCR) {
>> +		if (!user_set_csc || !vivid_is_ycbcr_enc_valid(mp->ycbcr_enc))
>> +			mp->ycbcr_enc = vivid_ycbcr_enc_cap(dev);
>> +	} else {
>>   		mp->ycbcr_enc = vivid_ycbcr_enc_cap(dev);
>> +	}
>>   	mp->xfer_func = vivid_xfer_func_cap(dev);
>> -	mp->quantization = vivid_quantization_cap(dev);
>> +	if (fmt->color_enc == TGP_COLOR_ENC_YCBCR ||
>> +	    fmt->color_enc == TGP_COLOR_ENC_RGB) {
>> +		if (!user_set_csc || !vivid_is_quant_valid(mp->quantization))
>> +			mp->quantization = vivid_quantization_cap(dev);
>> +	} else {
>> +		mp->quantization = vivid_quantization_cap(dev);
>> +	}
>>   	memset(mp->reserved, 0, sizeof(mp->reserved));
>>   	return 0;
>>   }
>> @@ -769,6 +804,12 @@ int vivid_s_fmt_vid_cap(struct file *file, void *priv,
>>   	if (vivid_is_sdtv_cap(dev))
>>   		dev->tv_field_cap = mp->field;
>>   	tpg_update_mv_step(&dev->tpg);
>> +	dev->tpg.quantization = mp->quantization;
>> +	if (dev->fmt_cap->color_enc == TGP_COLOR_ENC_YCBCR)
>> +		dev->tpg.ycbcr_enc = mp->ycbcr_enc;
>> +	else
>> +		dev->tpg.hsv_enc = mp->hsv_enc;
>> +
>>   	return 0;
>>   }
>>   
>> diff --git a/drivers/media/test-drivers/vivid/vivid-vid-common.c b/drivers/media/test-drivers/vivid/vivid-vid-common.c
>> index 76b0be670ebb..19aacb180e67 100644
>> --- a/drivers/media/test-drivers/vivid/vivid-vid-common.c
>> +++ b/drivers/media/test-drivers/vivid/vivid-vid-common.c
>> @@ -920,6 +920,26 @@ int vivid_enum_fmt_vid(struct file *file, void  *priv,
>>   	fmt = &vivid_formats[f->index];
>>   
>>   	f->pixelformat = fmt->fourcc;
>> +
>> +	if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
>> +	    f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
>> +		return 0;
>> +	/*
>> +	 * For capture devices, we support the CSC API.
>> +	 * We allow userspace to:
>> +	 * 1. set the ycbcr_enc on yuv format
>> +	 * 2. set the hsv_enc on hsv format
>> +	 * 3. set the quantization on yuv and rgb formats
>> +	 */
>> +	if (fmt->color_enc == TGP_COLOR_ENC_YCBCR) {
>> +		f->flags |= V4L2_FMT_FLAG_CSC_YCBCR_ENC;
>> +		f->flags |= V4L2_FMT_FLAG_CSC_QUANTIZATION;
>> +	} else if (fmt->color_enc == TGP_COLOR_ENC_HSV) {
>> +		f->flags |= V4L2_FMT_FLAG_CSC_HSV_ENC;
>> +	} else if (fmt->color_enc == TGP_COLOR_ENC_RGB) {
>> +		f->flags |= V4L2_FMT_FLAG_CSC_QUANTIZATION;
>> +	}
>> +
>>   	return 0;
>>   }
>>   
>>
> 

Re: [RFC v4 7/8] media: vivid: Add support to the CSC API

[Hans Verkuil]

On 08/06/2020 18:34, Dafna Hirschfeld wrote:
> 
> 
> On 08.06.20 12:10, Hans Verkuil wrote:
>> On 05/06/2020 19:26, Dafna Hirschfeld wrote:
>>> The CSC API (Colorspace conversion) allows userspace to try
>>> to configure the ycbcr/hsv_enc function and the quantization
>>> for capture devices. This patch adds support to the CSC API
>>> in vivid.
>>> Using the CSC API, userspace is allowed to do the following:
>>>
>>> 1. Set the ycbcr_enc function for YUV formats.
>>> 2. Set the hsv_enc function for HSV formats
>>> 3. Set the quantization for YUV and RGB formats.
>>
>> I just realized something. We excluded colorspace and xfer_func from the CSC API
>> for now since there are no drivers that support those conversions. But actually,
>> that's not true since vivid *does* support this unless it is in loopback mode
>> (then no CSC conversion is supported).
> 
> But shouldn't there be at least one real hardware device that support it in order
> to justify it in the API?

This is similar to e.g. the stateless decoder APIs: they are based on standards and
not specific HW implementation. It's the same with colorimetry: this is a standards
based API, so HW implementations will follow the standards.

It is actually IMHO more important to have one driver that supports all CSC features
to enable applications to test this.

In fact, it would be interesting to add a vivid control through which you can control
the CSC feature set (i.e. just quantization range + ycbcr_enc, or all four colorimetry
fields).

Regards,

	Hans

> 
> Thanks,
> Dafna
> 
>>
>> So if dev->loop_video is true, then disable CSC conversion in vivid.
>>
>> Otherwise CSC can be fully enabled for all four colorimetry fields.
>>
>> So I think we should add flags to signal this for colorspace and xfer_func as well
>> and implement this in vivid.
>>
>> Sorry about this, I should have realized this (much) earlier.
>>
>> Regards,
>>
>> 	Hans
>>
>>>
>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>>> ---
>>>   .../media/test-drivers/vivid/vivid-vid-cap.c  | 49 +++++++++++++++++--
>>>   .../test-drivers/vivid/vivid-vid-common.c     | 20 ++++++++
>>>   2 files changed, 65 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
>>> index e94beef008c8..8a43d7ebe53f 100644
>>> --- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c
>>> +++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
>>> @@ -549,6 +549,29 @@ int vivid_g_fmt_vid_cap(struct file *file, void *priv,
>>>   	return 0;
>>>   }
>>>   
>>> +static bool vivid_is_hsv_enc_valid(__u8 hsv_enc)
>>> +{
>>> +	if (hsv_enc == V4L2_HSV_ENC_180 || hsv_enc == V4L2_HSV_ENC_256)
>>> +		return true;
>>> +	return false;
>>> +}
>>> +
>>> +static bool vivid_is_ycbcr_enc_valid(__u8 ycbcr_enc)
>>> +{
>>> +	/* V4L2_YCBCR_ENC_SMPTE240M is the last ycbcr_enc enum */
>>> +	if (ycbcr_enc && ycbcr_enc <= V4L2_YCBCR_ENC_SMPTE240M)
>>> +		return true;
>>> +	return false;
>>> +}
>>> +
>>> +static bool vivid_is_quant_valid(__u8 quantization)
>>> +{
>>> +	if (quantization == V4L2_QUANTIZATION_FULL_RANGE ||
>>> +	    quantization == V4L2_QUANTIZATION_LIM_RANGE)
>>> +		return true;
>>> +	return false;
>>> +}
>>> +
>>>   int vivid_try_fmt_vid_cap(struct file *file, void *priv,
>>>   			struct v4l2_format *f)
>>>   {
>>> @@ -560,6 +583,7 @@ int vivid_try_fmt_vid_cap(struct file *file, void *priv,
>>>   	unsigned factor = 1;
>>>   	unsigned w, h;
>>>   	unsigned p;
>>> +	bool user_set_csc = !!(mp->flags & V4L2_PIX_FMT_FLAG_SET_CSC);
>>>   
>>>   	fmt = vivid_get_format(dev, mp->pixelformat);
>>>   	if (!fmt) {
>>> @@ -634,12 +658,23 @@ int vivid_try_fmt_vid_cap(struct file *file, void *priv,
>>>   			(fmt->bit_depth[0] / fmt->vdownsampling[0]);
>>>   
>>>   	mp->colorspace = vivid_colorspace_cap(dev);
>>> -	if (fmt->color_enc == TGP_COLOR_ENC_HSV)
>>> -		mp->hsv_enc = vivid_hsv_enc_cap(dev);
>>> -	else
>>> +	if (fmt->color_enc == TGP_COLOR_ENC_HSV) {
>>> +		if (!user_set_csc || !vivid_is_hsv_enc_valid(mp->hsv_enc))
>>> +			mp->hsv_enc = vivid_hsv_enc_cap(dev);
>>> +	} else if (fmt->color_enc == TGP_COLOR_ENC_YCBCR) {
>>> +		if (!user_set_csc || !vivid_is_ycbcr_enc_valid(mp->ycbcr_enc))
>>> +			mp->ycbcr_enc = vivid_ycbcr_enc_cap(dev);
>>> +	} else {
>>>   		mp->ycbcr_enc = vivid_ycbcr_enc_cap(dev);
>>> +	}
>>>   	mp->xfer_func = vivid_xfer_func_cap(dev);
>>> -	mp->quantization = vivid_quantization_cap(dev);
>>> +	if (fmt->color_enc == TGP_COLOR_ENC_YCBCR ||
>>> +	    fmt->color_enc == TGP_COLOR_ENC_RGB) {
>>> +		if (!user_set_csc || !vivid_is_quant_valid(mp->quantization))
>>> +			mp->quantization = vivid_quantization_cap(dev);
>>> +	} else {
>>> +		mp->quantization = vivid_quantization_cap(dev);
>>> +	}
>>>   	memset(mp->reserved, 0, sizeof(mp->reserved));
>>>   	return 0;
>>>   }
>>> @@ -769,6 +804,12 @@ int vivid_s_fmt_vid_cap(struct file *file, void *priv,
>>>   	if (vivid_is_sdtv_cap(dev))
>>>   		dev->tv_field_cap = mp->field;
>>>   	tpg_update_mv_step(&dev->tpg);
>>> +	dev->tpg.quantization = mp->quantization;
>>> +	if (dev->fmt_cap->color_enc == TGP_COLOR_ENC_YCBCR)
>>> +		dev->tpg.ycbcr_enc = mp->ycbcr_enc;
>>> +	else
>>> +		dev->tpg.hsv_enc = mp->hsv_enc;
>>> +
>>>   	return 0;
>>>   }
>>>   
>>> diff --git a/drivers/media/test-drivers/vivid/vivid-vid-common.c b/drivers/media/test-drivers/vivid/vivid-vid-common.c
>>> index 76b0be670ebb..19aacb180e67 100644
>>> --- a/drivers/media/test-drivers/vivid/vivid-vid-common.c
>>> +++ b/drivers/media/test-drivers/vivid/vivid-vid-common.c
>>> @@ -920,6 +920,26 @@ int vivid_enum_fmt_vid(struct file *file, void  *priv,
>>>   	fmt = &vivid_formats[f->index];
>>>   
>>>   	f->pixelformat = fmt->fourcc;
>>> +
>>> +	if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
>>> +	    f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
>>> +		return 0;
>>> +	/*
>>> +	 * For capture devices, we support the CSC API.
>>> +	 * We allow userspace to:
>>> +	 * 1. set the ycbcr_enc on yuv format
>>> +	 * 2. set the hsv_enc on hsv format
>>> +	 * 3. set the quantization on yuv and rgb formats
>>> +	 */
>>> +	if (fmt->color_enc == TGP_COLOR_ENC_YCBCR) {
>>> +		f->flags |= V4L2_FMT_FLAG_CSC_YCBCR_ENC;
>>> +		f->flags |= V4L2_FMT_FLAG_CSC_QUANTIZATION;
>>> +	} else if (fmt->color_enc == TGP_COLOR_ENC_HSV) {
>>> +		f->flags |= V4L2_FMT_FLAG_CSC_HSV_ENC;
>>> +	} else if (fmt->color_enc == TGP_COLOR_ENC_RGB) {
>>> +		f->flags |= V4L2_FMT_FLAG_CSC_QUANTIZATION;
>>> +	}
>>> +
>>>   	return 0;
>>>   }
>>>   
>>>
>>

Re: [RFC v4 3/8] media: Documentation: v4l: move table of v4l2_pix_format(_mplane) flags to pixfmt-v4l2.rst

[Helen Koike]

On 6/5/20 2:26 PM, Dafna Hirschfeld wrote:
> The table of the flags of the structs
> v4l2_pix_format(_mplane) is currently in pixfmt-reserved.rst
> which is wrong, it should be in pixfmt-v4l2.rst
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
>  .../userspace-api/media/v4l/pixfmt-reserved.rst | 17 -----------------
>  .../userspace-api/media/v4l/pixfmt-v4l2.rst     | 17 +++++++++++++++++
>  .../media/videodev2.h.rst.exceptions            |  2 +-
>  3 files changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
> index 59b9e7238f90..74ab6b5ce294 100644
> --- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
> +++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
> @@ -263,20 +263,3 @@ please make a proposal on the linux-media mailing list.
>  	of tiles, resulting in 32-aligned resolutions for the luminance plane
>  	and 16-aligned resolutions for the chrominance plane (with 2x2
>  	subsampling).
> -
> -.. tabularcolumns:: |p{6.6cm}|p{2.2cm}|p{8.7cm}|
> -
> -.. _format-flags:
> -
> -.. flat-table:: Format Flags
> -    :header-rows:  0
> -    :stub-columns: 0
> -    :widths:       3 1 4
> -
> -    * - ``V4L2_PIX_FMT_FLAG_PREMUL_ALPHA``
> -      - 0x00000001
> -      - The color values are premultiplied by the alpha channel value. For
> -	example, if a light blue pixel with 50% transparency was described
> -	by RGBA values (128, 192, 255, 128), the same pixel described with
> -	premultiplied colors would be described by RGBA values (64, 96,
> -	128, 128)> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
> index 759420a872d6..ffa539592822 100644
> --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
> +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
> @@ -169,3 +169,20 @@ Single-planar format structure
>          This information supplements the ``colorspace`` and must be set by
>  	the driver for capture streams and by the application for output
>  	streams, see :ref:`colorspaces`.
> +
> +.. tabularcolumns:: |p{6.6cm}|p{2.2cm}|p{8.7cm}|
> +
> +.. _format-flags:
> +
> +.. flat-table:: Format Flags
> +    :header-rows:  0
> +    :stub-columns: 0
> +    :widths:       3 1 4
> +
> +    * - ``V4L2_PIX_FMT_FLAG_PREMUL_ALPHA``
> +      - 0x00000001
> +      - The color values are premultiplied by the alpha channel value. For
> +        example, if a light blue pixel with 50% transparency was described
> +	by RGBA values (128, 192, 255, 128), the same pixel described with
> +	premultiplied colors would be described by RGBA values (64, 96,
> +	128, 128)


I see this is also pointed by Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst, but I don't
oppose moving the flags to this page.

Regards,
Helen


> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> index a625fb90e3a9..564a3bf5bc6d 100644
> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> @@ -180,7 +180,7 @@ replace define V4L2_CAP_IO_MC device-capabilities
>  
>  # V4L2 pix flags
>  replace define V4L2_PIX_FMT_PRIV_MAGIC :c:type:`v4l2_pix_format`
> -replace define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA reserved-formats
> +replace define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA format-flags
>  
>  # V4L2 format flags
>  replace define V4L2_FMT_FLAG_COMPRESSED fmtdesc-flags
> 

Re: [RFC v4 4/8] v4l2: add support for colorspace conversion API (CSC) for video capture and subdevices

[Helen Koike]

On 6/5/20 2:26 PM, Dafna Hirschfeld wrote:
> From: Philipp Zabel <p.zabel@pengutronix.de>
> 
> For video capture it is the driver that reports the colorspace,
> Y'CbCr/HSV encoding, quantization range and transfer function
> used by the video, and there is no way to request something
> different, even though many HDTV receivers have some sort of
> colorspace conversion capabilities.
> 
> For output video this feature already exists since the application
> specifies this information for the video format it will send out, and
> the transmitter will enable any available CSC if a format conversion has
> to be performed in order to match the capabilities of the sink.
> 
> For video capture we propose adding new v4l2_pix_format flag:
> V4L2_PIX_FMT_FLAG_SET_CSC. The flag is set by the application,
> the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields
> as the requested colorspace information and will attempt to
> do the conversion it supports.
> 
> Drivers set the flags
> V4L2_FMT_FLAG_CSC_YCBCR_ENC,
> V4L2_FMT_FLAG_CSC_HSV_ENC,
> V4L2_FMT_FLAG_CSC_QUANTIZATION,
> in the flags field of the struct v4l2_fmtdesc during enumeration to
> indicate that they support colorspace conversion for the respective field.
> Currently the conversion of the fields 'colorspace' and 'xfer_func' is not
> supported since there are no drivers that support it.
> 
> The same API is added for the subdevices. With the flag
> V4L2_MBUS_FRAMEFMT_SET_CSC set by the application in VIDIOC_SUBDEV_S_FMT
> ioctl and the flags V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC,
> V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION set by the driver in the
> VIDIOC_SUBDEV_ENUM_MBUS_CODE ioctl.
> 
> For subdevices, new 'flags' fields were added to the structs
> v4l2_subdev_mbus_code_enum, v4l2_mbus_framefmt which are borrowed from the
> 'reserved' field
> 
> Drivers do not have to actually look at the flagsr. If the flags are not
> set, then the colorspace, ycbcr_enc and quantization fields are set to
> the default values by the core, i.e. just pass on the received format
> without conversion.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
>  .../media/v4l/pixfmt-v4l2-mplane.rst          | 16 ++----
>  .../userspace-api/media/v4l/pixfmt-v4l2.rst   | 46 ++++++++++++++--
>  .../media/v4l/subdev-formats.rst              | 52 +++++++++++++++++--
>  .../media/v4l/vidioc-enum-fmt.rst             | 22 +++++++-
>  .../v4l/vidioc-subdev-enum-mbus-code.rst      | 28 ++++++++++
>  .../media/videodev2.h.rst.exceptions          |  3 ++
>  include/uapi/linux/v4l2-mediabus.h            |  5 +-
>  include/uapi/linux/v4l2-subdev.h              |  5 +-
>  include/uapi/linux/videodev2.h                |  4 ++
>  9 files changed, 160 insertions(+), 21 deletions(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst
> index 444b4082684c..66f3365d7b72 100644
> --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst
> +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst
> @@ -105,29 +105,21 @@ describing all planes of that format.
>      * - __u8
>        - ``ycbcr_enc``
>        - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
> -        This information supplements the ``colorspace`` and must be set by
> -	the driver for capture streams and by the application for output
> -	streams, see :ref:`colorspaces`.
> +	See struct :c:type:`v4l2_pix_format`.
>      * - __u8
>        - ``hsv_enc``
>        - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`.
> -        This information supplements the ``colorspace`` and must be set by
> -	the driver for capture streams and by the application for output
> -	streams, see :ref:`colorspaces`.
> +	See struct :c:type:`v4l2_pix_format`.
>      * - }
>        -
>      * - __u8
>        - ``quantization``
>        - Quantization range, from enum :c:type:`v4l2_quantization`.
> -        This information supplements the ``colorspace`` and must be set by
> -	the driver for capture streams and by the application for output
> -	streams, see :ref:`colorspaces`.
> +	See struct :c:type:`v4l2_pix_format`.
>      * - __u8
>        - ``xfer_func``
>        - Transfer function, from enum :c:type:`v4l2_xfer_func`.
> -        This information supplements the ``colorspace`` and must be set by
> -	the driver for capture streams and by the application for output
> -	streams, see :ref:`colorspaces`.
> +	See struct :c:type:`v4l2_pix_format`.
>      * - __u8
>        - ``reserved[7]``
>        - Reserved for future extensions. Should be zeroed by drivers and
> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
> index ffa539592822..f23404efd90f 100644
> --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
> +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
> @@ -148,13 +148,29 @@ Single-planar format structure
>        - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
>          This information supplements the ``colorspace`` and must be set by
>  	the driver for capture streams and by the application for output
> -	streams, see :ref:`colorspaces`.
> +	streams, see :ref:`colorspaces`. If the application sets the
> +	flag ``V4L2_PIX_FMT_FLAG_SET_CSC`` then the application can set
> +	this field for a capture stream to request a specific Y'CbCr encoding
> +	for the captured image data. If the driver cannot handle requested
> +	conversion, it will return another supported encoding.
> +	This field is ignored for HSV pixelformats. The driver indicates that
> +	ycbcr_enc conversion is supported by setting the flag
> +	V4L2_FMT_FLAG_CSC_YCBCR_ENC in the corresponding struct
> +	:c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags`
>      * - __u32
>        - ``hsv_enc``
>        - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`.
>          This information supplements the ``colorspace`` and must be set by
>  	the driver for capture streams and by the application for output
> -	streams, see :ref:`colorspaces`.
> +	streams, see :ref:`colorspaces`. If the application sets the flag
> +	``V4L2_PIX_FMT_FLAG_SET_CSC`` then the application can set this
> +	field for a capture stream to request a specific HSV encoding for the
> +	captured image data. If the driver cannot handle requested
> +	conversion, it will return another supported encoding.
> +	This field is ignored for non-HSV pixelformats. The driver indicates
> +	that hsv_enc conversion is supported by setting the flag
> +	V4L2_FMT_FLAG_CSC_HSV_ENC in the corresponding struct
> +	:c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags`
>      * - }
>        -
>      * - __u32
> @@ -162,7 +178,14 @@ Single-planar format structure
>        - Quantization range, from enum :c:type:`v4l2_quantization`.
>          This information supplements the ``colorspace`` and must be set by
>  	the driver for capture streams and by the application for output
> -	streams, see :ref:`colorspaces`.
> +	streams, see :ref:`colorspaces`. If the application sets the flag
> +	``V4L2_PIX_FMT_FLAG_SET_CSC`` then the application can set
> +	this field for a capture stream to request a specific quantization
> +	range for the captured image data. If the driver cannot handle requested
> +	conversion, it will return another supported encoding.
> +	The driver indicates that quantization conversion is supported by setting
> +	the flag V4L2_FMT_FLAG_CSC_QUANTIZATION in the corresponding struct
> +	:c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags`
>      * - __u32
>        - ``xfer_func``
>        - Transfer function, from enum :c:type:`v4l2_xfer_func`.
> @@ -186,3 +209,20 @@ Single-planar format structure
>  	by RGBA values (128, 192, 255, 128), the same pixel described with
>  	premultiplied colors would be described by RGBA values (64, 96,
>  	128, 128)
> +    * .. _`v4l2-pix-fmt-flag-set-csc`:
> +
> +      - ``V4L2_PIX_FMT_FLAG_SET_CSC``
> +      - 0x00000002
> +      - Set by the application. It is only used for capture and is
> +        ignored for output streams. If set, then request the device to do
> +	colorspace conversion from the received colorspace to the requested
> +	colorspace values. If colorimetry field (``ycncr_enc``, ``hsv_enc``
> +	or ``quantization``) is set to 0, then that colorimetry setting will
> +	remain unchanged from what was received. So to change the quantization
> +	only the ``quantization`` field shall be set to non-zero values
> +	(``V4L2_QUANTIZATION_FULL_RANGE`` or ``V4L2_QUANTIZATION_LIM_RANGE``)
> +	and all other colorimetry fields shall be set to 0. The API does not
> +	support the conversion of the fields ``colorspace`` and ``xfer_func``
> +
> +	To check which conversions are supported by the hardware for the current
> +	pixel format, see :ref:`fmtdesc-flags`.
> diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> index 9a4d61b0d76f..75eb7f8bb4c5 100644
> --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> @@ -49,13 +49,32 @@ Media Bus Formats
>        - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
>          This information supplements the ``colorspace`` and must be set by
>  	the driver for capture streams and by the application for output
> -	streams, see :ref:`colorspaces`.
> +	streams, see :ref:`colorspaces`. If the application sets the
> +	flag ``V4L2_MBUS_FRAMEFMT_SET_CSC`` then the application can set
> +	this field for a capture stream to request a specific Y'CbCr encoding
> +	for the media bus data. If the driver cannot handle requested
> +	conversion, it will return another supported encoding.
> +	This field is ignored for HSV media bus formats. The driver indicates
> +	that ycbcr_enc conversion is supported by setting the flag
> +	V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC in the corresponding struct
> +	:c:type:`v4l2_subdev_mbus_code_enum` during enumeration.
> +	See :ref:`v4l2-subdev-mbus-code-flags`
> +
>      * - __u16
>        - ``quantization``
>        - Quantization range, from enum :c:type:`v4l2_quantization`.
>          This information supplements the ``colorspace`` and must be set by
>  	the driver for capture streams and by the application for output
> -	streams, see :ref:`colorspaces`.
> +	streams, see :ref:`colorspaces`. If the application sets the
> +	flag ``V4L2_MBUS_FRAMEFMT_SET_CSC`` then the application can set
> +	this field for a capture stream to request a specific quantization
> +	encoding for the media bus data. If the driver cannot handle requested
> +	conversion, it will return another supported encoding.
> +	The driver indicates that quantization conversion is supported by
> +	setting the flag V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION in the
> +	corresponding struct :c:type:`v4l2_subdev_mbus_code_enum`
> +	during enumeration. See :ref:`v4l2-subdev-mbus-code-flags`
> +
>      * - __u16
>        - ``xfer_func``
>        - Transfer function, from enum :c:type:`v4l2_xfer_func`.
> @@ -63,10 +82,37 @@ Media Bus Formats
>  	the driver for capture streams and by the application for output
>  	streams, see :ref:`colorspaces`.
>      * - __u16
> -      - ``reserved``\ [11]
> +      - ``flags``
> +      - flags See:  :ref:v4l2-mbus-framefmt-flags
> +    * - __u16
> +      - ``reserved``\ [10]
>        - Reserved for future extensions. Applications and drivers must set
>  	the array to zero.
>  
> +.. _v4l2-mbus-framefmt-flags:
> +
> +.. flat-table:: v4l2_mbus_framefmt Flags
> +    :header-rows:  0
> +    :stub-columns: 0
> +    :widths:       3 1 4
> +
> +    * .. _`mbus-framefmt-set-csc`:
> +
> +      - ``V4L2_MBUS_FRAMEFMT_SET_CSC``
> +      - 0x0001
> +      - Set by the application. It is only used for capture and is
> +	ignored for output streams. If set, then request the subdevice to do
> +	colorspace conversion from the received colorspace to the requested
> +	colorspace values. If colorimetry field (``ycbcr_enc`` or
> +	``quantization``) is set to 0, then that colorimetry setting will remain
> +	unchanged from what was received. So to change the quantization, only the
> +	``quantization`` field shall be set to non-zero values
> +	(``V4L2_QUANTIZATION_FULL_RANGE`` or ``V4L2_QUANTIZATION_LIM_RANGE``)
> +	and all other colorimetry fields shall be set to 0. The API does not
> +	support the conversion of the fields ``colorspace`` and ``xfer_func``.
> +
> +	To check which conversions are supported by the hardware for the current
> +	media bus frame format, see :ref:`v4l2-mbus-framefmt-flags`.
>  
>  
>  .. _v4l2-mbus-pixelcode:
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> index a53dd3d7f7e2..11323755d41b 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> @@ -178,7 +178,27 @@ the ``mbus_code`` field is handled differently:
>  	parameters are detected. This flag can only be used in combination
>  	with the ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to
>  	compressed formats only. It is also only applies to stateful codecs.
> -
> +    * - ``V4L2_FMT_FLAG_CSC_YCBCR_ENC``
> +      - 0x0010
> +      - The driver allows the application to try to change the default
> +	Y'CbCr encoding. This flag is relevant only for capture devices.
> +	The application can ask to configure the ycbcr_enc of the capture device
> +	when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
> +	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
> +    * - ``V4L2_FMT_FLAG_CSC_HSV_ENC``
> +      - 0x0010
> +      - The driver allows the application to try to change the default
> +	HSV encoding. This flag is relevant only for capture devices.
> +	The application can ask to configure the hsv_enc of the capture device
> +	when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
> +	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
> +    * - ``V4L2_FMT_FLAG_CSC_QUANTIZATION``
> +      - 0x0020
> +      - The driver allows the application to try to change the default
> +	quantization. This flag is relevant only for capture devices.
> +	The application can ask to configure the quantization of the capture
> +	device when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
> +	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
>  
>  Return Value
>  ============
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
> index 35b8607203a4..3d3430bdd71f 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
> @@ -78,12 +78,40 @@ information about the try formats.
>        - ``which``
>        - Media bus format codes to be enumerated, from enum
>  	:ref:`v4l2_subdev_format_whence <v4l2-subdev-format-whence>`.
> +    * - __u32
> +      - ``flags``
> +      - See :ref:`v4l2-subdev-mbus-code-flags`
>      * - __u32
>        - ``reserved``\ [8]
>        - Reserved for future extensions. Applications and drivers must set
>  	the array to zero.
>  
>  
> +
> +.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{7.7cm}|
> +
> +.. _v4l2-subdev-mbus-code-flags:
> +
> +.. flat-table:: Subdev Media Bus Code Enumerate Flags
> +    :header-rows:  0
> +    :stub-columns: 0
> +    :widths:       1 1 2
> +
> +    * - V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC
> +      - 0x00000001
> +      - The driver allows the application to try to change the default Y'CbCr
> +	encoding. The application can ask to configure the ycbcr_enc of the
> +	subdevice when calling the :ref:`VIDIOC_SUBDEV_S_FMT <VIDIOC_SUBDEV_G_FMT>`
> +	ioctl with :ref:`V4L2_MBUS_FRAMEFMT_SET_CSC <mbus-framefmt-set-csc>` set.
> +	See :ref:`v4l2-mbus-format` on how to do this.
> +    * - V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION
> +      - 0x00000002
> +      - The driver allows the application to try to change the default
> +	quantization. The application can ask to configure the quantization of
> +	the subdevice when calling the :ref:`VIDIOC_SUBDEV_S_FMT <VIDIOC_SUBDEV_G_FMT>`
> +	ioctl with :ref:`V4L2_MBUS_FRAMEFMT_SET_CSC <mbus-framefmt-set-csc>` set.
> +	See :ref:`v4l2-mbus-format` on how to do this.
> +
>  Return Value
>  ============
>  
> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> index 564a3bf5bc6d..f7be008cd479 100644
> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> @@ -187,6 +187,9 @@ replace define V4L2_FMT_FLAG_COMPRESSED fmtdesc-flags
>  replace define V4L2_FMT_FLAG_EMULATED fmtdesc-flags
>  replace define V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM fmtdesc-flags
>  replace define V4L2_FMT_FLAG_DYN_RESOLUTION fmtdesc-flags
> +replace define V4L2_FMT_FLAG_CSC_YCBCR_ENC fmtdesc-flags
> +replace define V4L2_FMT_FLAG_CSC_HSV_ENC fmtdesc-flags
> +replace define V4L2_FMT_FLAG_CSC_QUANTIZATION fmtdesc-flags
>  
>  # V4L2 timecode types
>  replace define V4L2_TC_TYPE_24FPS timecode-type
> diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h
> index 123a231001a8..0f916278137a 100644
> --- a/include/uapi/linux/v4l2-mediabus.h
> +++ b/include/uapi/linux/v4l2-mediabus.h
> @@ -16,6 +16,8 @@
>  #include <linux/types.h>
>  #include <linux/videodev2.h>
>  
> +#define V4L2_MBUS_FRAMEFMT_SET_CSC	0x0001
> +
>  /**
>   * struct v4l2_mbus_framefmt - frame format on the media bus
>   * @width:	image width
> @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt {
>  	__u16			ycbcr_enc;
>  	__u16			quantization;
>  	__u16			xfer_func;
> -	__u16			reserved[11];
> +	__u16			flags;
> +	__u16			reserved[10];
>  };
>  
>  #ifndef __KERNEL__
> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
> index 5d2a1dab7911..972e64d8b54e 100644
> --- a/include/uapi/linux/v4l2-subdev.h
> +++ b/include/uapi/linux/v4l2-subdev.h
> @@ -65,6 +65,8 @@ struct v4l2_subdev_crop {
>  	__u32 reserved[8];
>  };
>  
> +#define V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC	0x00000001
> +#define V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION	0x00000002
>  /**
>   * struct v4l2_subdev_mbus_code_enum - Media bus format enumeration
>   * @pad: pad number, as reported by the media API
> @@ -77,7 +79,8 @@ struct v4l2_subdev_mbus_code_enum {
>  	__u32 index;
>  	__u32 code;
>  	__u32 which;
> -	__u32 reserved[8];
> +	__u32 flags;
> +	__u32 reserved[7];
>  };
>  
>  /**
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index c3a1cf1c507f..15824316e0ca 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -774,6 +774,7 @@ struct v4l2_pix_format {
>  
>  /* Flags */
>  #define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA	0x00000001
> +#define V4L2_PIX_FMT_FLAG_SET_CSC	0x00000002
>  
>  /*
>   *	F O R M A T   E N U M E R A T I O N
> @@ -792,6 +793,9 @@ struct v4l2_fmtdesc {
>  #define V4L2_FMT_FLAG_EMULATED			0x0002
>  #define V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM	0x0004
>  #define V4L2_FMT_FLAG_DYN_RESOLUTION		0x0008
> +#define V4L2_FMT_FLAG_CSC_YCBCR_ENC		0x0010
> +#define V4L2_FMT_FLAG_CSC_HSV_ENC		0x0010

Shouldn't those have different values? Or is this intentional?

Regards,
Helen

> +#define V4L2_FMT_FLAG_CSC_QUANTIZATION		0x0020
>  
>  	/* Frame Size and frame rate enumeration */
>  /*
> 

Re: [RFC v4 5/8] media: staging: rkisp1: allow quantization conversion from userspace for isp source pad

[Helen Koike]

Hi Dafna,

Thanks for the patch,

On 6/5/20 2:26 PM, Dafna Hirschfeld wrote:
> The isp entity has a hardware support to force full range quantization
> for YUV formats. Use the CSC API to allow userspace to set the
> quantization for YUV formats on the isp, resizer and capture entities.
> 
> - The flag V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION is set on
> YUV formats during enumeration of the resizer and isp formats.
> - The flag V4L2_FMT_FLAG_CSC_QUANTIZATION is set on the YUV formats
> during enumeration of the capture formats.
> - The full quantization is set on YUV formats if userspace ask it
> on the entities using the flag V4L2_MBUS_FRAMEFMT_SET_CSC
> for subdevices and the flag V4L2_PIX_FMT_FLAG_SET_CSC on for
> the capture entity.
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
>  drivers/staging/media/rkisp1/rkisp1-capture.c | 23 ++++++++++++-------
>  drivers/staging/media/rkisp1/rkisp1-isp.c     | 22 ++++++++++++++----
>  drivers/staging/media/rkisp1/rkisp1-resizer.c | 22 ++++++++++++++++++
>  3 files changed, 55 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
> index f69235f82c45..66856d5eb576 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-capture.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
> @@ -1085,8 +1085,6 @@ static void rkisp1_try_fmt(const struct rkisp1_capture *cap,
>  			   const struct v4l2_format_info **fmt_info)
>  {
>  	const struct rkisp1_capture_config *config = cap->config;
> -	struct rkisp1_capture *other_cap =
> -			&cap->rkisp1->capture_devs[cap->id ^ 1];
>  	const struct rkisp1_capture_fmt_cfg *fmt;
>  	const struct v4l2_format_info *info;
>  	const unsigned int max_widths[] = { RKISP1_RSZ_MP_SRC_MAX_WIDTH,
> @@ -1108,16 +1106,21 @@ static void rkisp1_try_fmt(const struct rkisp1_capture *cap,
>  	pixm->field = V4L2_FIELD_NONE;
>  	pixm->colorspace = V4L2_COLORSPACE_DEFAULT;

Shouldn't the driver set colorspace instead of leaving as default?
In this case it's V4L2_COLORSPACE_SRGB for BT.601 specification.

>  	pixm->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> +	pixm->xfer_func = V4L2_XFER_FUNC_DEFAULT;
>  
>  	info = rkisp1_fill_pixfmt(pixm, cap->id);
>  
> -	/* can not change quantization when stream-on */
> -	if (other_cap->is_streaming)
> -		pixm->quantization = other_cap->pix.fmt.quantization;

We need to investigate why we had this in the original driver.
Can we have different quantization values for each capture streaming at the same time?

> -	/* output full range by default, take effect in params */
> -	else if (!pixm->quantization ||
> -		 pixm->quantization > V4L2_QUANTIZATION_LIM_RANGE)
> +	/*
> +	 * isp has a feature to select between limited and full range for YUV
> +	 * formats.
> +	 * So we should also support it in the capture using the CSC API
> +	 */
> +	if (pixm->flags & V4L2_PIX_FMT_FLAG_SET_CSC &&
> +	    pixm->quantization == V4L2_QUANTIZATION_FULL_RANGE &&
> +	    v4l2_is_format_yuv(info))
>  		pixm->quantization = V4L2_QUANTIZATION_FULL_RANGE;
> +	else
> +		pixm->quantization = V4L2_QUANTIZATION_DEFAULT;
>  
>  	if (fmt_cfg)
>  		*fmt_cfg = fmt;
> @@ -1152,12 +1155,16 @@ static int rkisp1_enum_fmt_vid_cap_mplane(struct file *file, void *priv,
>  {
>  	struct rkisp1_capture *cap = video_drvdata(file);
>  	const struct rkisp1_capture_fmt_cfg *fmt = NULL;
> +	const struct v4l2_format_info *info;
>  
>  	if (f->index >= cap->config->fmt_size)
>  		return -EINVAL;
>  
>  	fmt = &cap->config->fmts[f->index];
>  	f->pixelformat = fmt->fourcc;
> +	info = v4l2_format_info(f->pixelformat);
> +	if (v4l2_is_format_yuv(info))
> +		f->flags = V4L2_FMT_FLAG_CSC_QUANTIZATION;
>  
>  	return 0;
>  }
> diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c
> index e66e87d6ea8b..28e0a3c594aa 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-isp.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-isp.c
> @@ -591,6 +591,10 @@ static int rkisp1_isp_enum_mbus_code(struct v4l2_subdev *sd,
>  
>  		if (code->index == pos - 1) {
>  			code->code = fmt->mbus_code;
> +			if (fmt->pixel_enc == V4L2_PIXEL_ENC_YUV &&
> +			    dir == RKISP1_DIR_SRC)
> +				code->flags =
> +					V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION;
>  			return 0;
>  		}
>  	}
> @@ -622,7 +626,6 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
>  					     RKISP1_ISP_PAD_SOURCE_VIDEO);
>  	*src_fmt = *sink_fmt;
>  	src_fmt->code = RKISP1_DEF_SRC_PAD_FMT;
> -	src_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
>  
>  	src_crop = v4l2_subdev_get_try_crop(sd, cfg,
>  					    RKISP1_ISP_PAD_SOURCE_VIDEO);
> @@ -665,10 +668,21 @@ static void rkisp1_isp_set_src_fmt(struct rkisp1_isp *isp,
>  		isp->src_fmt = mbus_info;
>  	src_fmt->width  = src_crop->width;
>  	src_fmt->height = src_crop->height;
> -	src_fmt->quantization = format->quantization;
> -	/* full range by default */
> -	if (!src_fmt->quantization)
> +
> +	src_fmt->colorspace = V4L2_COLORSPACE_DEFAULT;

Shouldn't this be V4L2_COLORSPACE_SRGB ? Same comment for other places using V4L2_COLORSPACE_DEFAULT

Regards,
Helen

> +	src_fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> +	src_fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> +
> +	/*
> +	 * The CSC API is used to allow userspace to force full
> +	 * quantization on YUV formats.
> +	 */
> +	if (format->flags & V4L2_MBUS_FRAMEFMT_SET_CSC &&
> +	    format->quantization == V4L2_QUANTIZATION_FULL_RANGE &&
> +	    mbus_info->pixel_enc == V4L2_PIXEL_ENC_YUV)
>  		src_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
> +	else
> +		src_fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
>  
>  	*format = *src_fmt;
>  }
> diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
> index fa28f4bd65c0..237cce9183f7 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
> @@ -549,8 +549,30 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
>  	if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
>  		rsz->pixel_enc = mbus_info->pixel_enc;
>  
> +	sink_fmt->colorspace = V4L2_COLORSPACE_DEFAULT;
> +	sink_fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> +	sink_fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> +
> +	/*
> +	 * isp has a feature to select between limited and full range for
> +	 * YUV formats.
> +	 * so we should support it in the resizer using the CSC API
> +	 */
> +
> +	if (format->flags & V4L2_MBUS_FRAMEFMT_SET_CSC &&
> +	    format->quantization == V4L2_QUANTIZATION_FULL_RANGE &&
> +	    mbus_info->pixel_enc == V4L2_PIXEL_ENC_YUV)
> +		sink_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
> +	else
> +		sink_fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
> +
>  	/* Propagete to source pad */
>  	src_fmt->code = sink_fmt->code;
> +	src_fmt->field = sink_fmt->field;
> +	src_fmt->colorspace = sink_fmt->colorspace;
> +	src_fmt->ycbcr_enc = sink_fmt->ycbcr_enc;
> +	src_fmt->xfer_func = sink_fmt->xfer_func;
> +	src_fmt->quantization = sink_fmt->quantization;
>  
>  	sink_fmt->width = clamp_t(u32, format->width,
>  				  rsz->config->min_rsz_width,
> 

Re: [RFC v4 5/8] media: staging: rkisp1: allow quantization conversion from userspace for isp source pad

[Dafna Hirschfeld]

On 26.06.20 01:52, Helen Koike wrote:
> Hi Dafna,
> 
> Thanks for the patch,
> 
> On 6/5/20 2:26 PM, Dafna Hirschfeld wrote:
>> The isp entity has a hardware support to force full range quantization
>> for YUV formats. Use the CSC API to allow userspace to set the
>> quantization for YUV formats on the isp, resizer and capture entities.
>>
>> - The flag V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION is set on
>> YUV formats during enumeration of the resizer and isp formats.
>> - The flag V4L2_FMT_FLAG_CSC_QUANTIZATION is set on the YUV formats
>> during enumeration of the capture formats.
>> - The full quantization is set on YUV formats if userspace ask it
>> on the entities using the flag V4L2_MBUS_FRAMEFMT_SET_CSC
>> for subdevices and the flag V4L2_PIX_FMT_FLAG_SET_CSC on for
>> the capture entity.
>>
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>> ---
>>   drivers/staging/media/rkisp1/rkisp1-capture.c | 23 ++++++++++++-------
>>   drivers/staging/media/rkisp1/rkisp1-isp.c     | 22 ++++++++++++++----
>>   drivers/staging/media/rkisp1/rkisp1-resizer.c | 22 ++++++++++++++++++
>>   3 files changed, 55 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
>> index f69235f82c45..66856d5eb576 100644
>> --- a/drivers/staging/media/rkisp1/rkisp1-capture.c
>> +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
>> @@ -1085,8 +1085,6 @@ static void rkisp1_try_fmt(const struct rkisp1_capture *cap,
>>   			   const struct v4l2_format_info **fmt_info)
>>   {
>>   	const struct rkisp1_capture_config *config = cap->config;
>> -	struct rkisp1_capture *other_cap =
>> -			&cap->rkisp1->capture_devs[cap->id ^ 1];
>>   	const struct rkisp1_capture_fmt_cfg *fmt;
>>   	const struct v4l2_format_info *info;
>>   	const unsigned int max_widths[] = { RKISP1_RSZ_MP_SRC_MAX_WIDTH,
>> @@ -1108,16 +1106,21 @@ static void rkisp1_try_fmt(const struct rkisp1_capture *cap,
>>   	pixm->field = V4L2_FIELD_NONE;
>>   	pixm->colorspace = V4L2_COLORSPACE_DEFAULT;
> 
> Shouldn't the driver set colorspace instead of leaving as default?
> In this case it's V4L2_COLORSPACE_SRGB for BT.601 specification.
> 
>>   	pixm->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
>> +	pixm->xfer_func = V4L2_XFER_FUNC_DEFAULT;
>>   
>>   	info = rkisp1_fill_pixfmt(pixm, cap->id);
>>   
>> -	/* can not change quantization when stream-on */
>> -	if (other_cap->is_streaming)
>> -		pixm->quantization = other_cap->pix.fmt.quantization;
> 
> We need to investigate why we had this in the original driver.
> Can we have different quantization values for each capture streaming at the same time?

With this patchset, both captures need to have the same quantization
as the isp entity in order to stream, so checking the values of the other
capture is not needed anymore.

Thanks,
Dafna

> 
>> -	/* output full range by default, take effect in params */
>> -	else if (!pixm->quantization ||
>> -		 pixm->quantization > V4L2_QUANTIZATION_LIM_RANGE)
>> +	/*
>> +	 * isp has a feature to select between limited and full range for YUV
>> +	 * formats.
>> +	 * So we should also support it in the capture using the CSC API
>> +	 */
>> +	if (pixm->flags & V4L2_PIX_FMT_FLAG_SET_CSC &&
>> +	    pixm->quantization == V4L2_QUANTIZATION_FULL_RANGE &&
>> +	    v4l2_is_format_yuv(info))
>>   		pixm->quantization = V4L2_QUANTIZATION_FULL_RANGE;
>> +	else
>> +		pixm->quantization = V4L2_QUANTIZATION_DEFAULT;
>>   
>>   	if (fmt_cfg)
>>   		*fmt_cfg = fmt;
>> @@ -1152,12 +1155,16 @@ static int rkisp1_enum_fmt_vid_cap_mplane(struct file *file, void *priv,
>>   {
>>   	struct rkisp1_capture *cap = video_drvdata(file);
>>   	const struct rkisp1_capture_fmt_cfg *fmt = NULL;
>> +	const struct v4l2_format_info *info;
>>   
>>   	if (f->index >= cap->config->fmt_size)
>>   		return -EINVAL;
>>   
>>   	fmt = &cap->config->fmts[f->index];
>>   	f->pixelformat = fmt->fourcc;
>> +	info = v4l2_format_info(f->pixelformat);
>> +	if (v4l2_is_format_yuv(info))
>> +		f->flags = V4L2_FMT_FLAG_CSC_QUANTIZATION;
>>   
>>   	return 0;
>>   }
>> diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c
>> index e66e87d6ea8b..28e0a3c594aa 100644
>> --- a/drivers/staging/media/rkisp1/rkisp1-isp.c
>> +++ b/drivers/staging/media/rkisp1/rkisp1-isp.c
>> @@ -591,6 +591,10 @@ static int rkisp1_isp_enum_mbus_code(struct v4l2_subdev *sd,
>>   
>>   		if (code->index == pos - 1) {
>>   			code->code = fmt->mbus_code;
>> +			if (fmt->pixel_enc == V4L2_PIXEL_ENC_YUV &&
>> +			    dir == RKISP1_DIR_SRC)
>> +				code->flags =
>> +					V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION;
>>   			return 0;
>>   		}
>>   	}
>> @@ -622,7 +626,6 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
>>   					     RKISP1_ISP_PAD_SOURCE_VIDEO);
>>   	*src_fmt = *sink_fmt;
>>   	src_fmt->code = RKISP1_DEF_SRC_PAD_FMT;
>> -	src_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
>>   
>>   	src_crop = v4l2_subdev_get_try_crop(sd, cfg,
>>   					    RKISP1_ISP_PAD_SOURCE_VIDEO);
>> @@ -665,10 +668,21 @@ static void rkisp1_isp_set_src_fmt(struct rkisp1_isp *isp,
>>   		isp->src_fmt = mbus_info;
>>   	src_fmt->width  = src_crop->width;
>>   	src_fmt->height = src_crop->height;
>> -	src_fmt->quantization = format->quantization;
>> -	/* full range by default */
>> -	if (!src_fmt->quantization)
>> +
>> +	src_fmt->colorspace = V4L2_COLORSPACE_DEFAULT;
> 
> Shouldn't this be V4L2_COLORSPACE_SRGB ? Same comment for other places using V4L2_COLORSPACE_DEFAULT
> 
> Regards,
> Helen
> 
>> +	src_fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
>> +	src_fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
>> +
>> +	/*
>> +	 * The CSC API is used to allow userspace to force full
>> +	 * quantization on YUV formats.
>> +	 */
>> +	if (format->flags & V4L2_MBUS_FRAMEFMT_SET_CSC &&
>> +	    format->quantization == V4L2_QUANTIZATION_FULL_RANGE &&
>> +	    mbus_info->pixel_enc == V4L2_PIXEL_ENC_YUV)
>>   		src_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
>> +	else
>> +		src_fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
>>   
>>   	*format = *src_fmt;
>>   }
>> diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
>> index fa28f4bd65c0..237cce9183f7 100644
>> --- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
>> +++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
>> @@ -549,8 +549,30 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
>>   	if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
>>   		rsz->pixel_enc = mbus_info->pixel_enc;
>>   
>> +	sink_fmt->colorspace = V4L2_COLORSPACE_DEFAULT;
>> +	sink_fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
>> +	sink_fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
>> +
>> +	/*
>> +	 * isp has a feature to select between limited and full range for
>> +	 * YUV formats.
>> +	 * so we should support it in the resizer using the CSC API
>> +	 */
>> +
>> +	if (format->flags & V4L2_MBUS_FRAMEFMT_SET_CSC &&
>> +	    format->quantization == V4L2_QUANTIZATION_FULL_RANGE &&
>> +	    mbus_info->pixel_enc == V4L2_PIXEL_ENC_YUV)
>> +		sink_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
>> +	else
>> +		sink_fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
>> +
>>   	/* Propagete to source pad */
>>   	src_fmt->code = sink_fmt->code;
>> +	src_fmt->field = sink_fmt->field;
>> +	src_fmt->colorspace = sink_fmt->colorspace;
>> +	src_fmt->ycbcr_enc = sink_fmt->ycbcr_enc;
>> +	src_fmt->xfer_func = sink_fmt->xfer_func;
>> +	src_fmt->quantization = sink_fmt->quantization;
>>   
>>   	sink_fmt->width = clamp_t(u32, format->width,
>>   				  rsz->config->min_rsz_width,
>>

Re: [RFC v4 4/8] v4l2: add support for colorspace conversion API (CSC) for video capture and subdevices

[Hans Verkuil]

On 26/06/2020 01:29, Helen Koike wrote:
> 
> 
> On 6/5/20 2:26 PM, Dafna Hirschfeld wrote:

<snip>

>>  /*
>>   *	F O R M A T   E N U M E R A T I O N
>> @@ -792,6 +793,9 @@ struct v4l2_fmtdesc {
>>  #define V4L2_FMT_FLAG_EMULATED			0x0002
>>  #define V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM	0x0004
>>  #define V4L2_FMT_FLAG_DYN_RESOLUTION		0x0008
>> +#define V4L2_FMT_FLAG_CSC_YCBCR_ENC		0x0010
>> +#define V4L2_FMT_FLAG_CSC_HSV_ENC		0x0010
> 
> Shouldn't those have different values? Or is this intentional?

It's intentional, but it would probably be better to write this as:

#define V4L2_FMT_FLAG_CSC_YCBCR_ENC		0x0010
#define V4L2_FMT_FLAG_CSC_HSV_ENC		V4L2_FMT_FLAG_CSC_YCBCR_ENC

That makes it explicit that HSV_ENC is an alias for YCBCR_ENC.

Regards,

	Hans

> 
> Regards,
> Helen
> 
>> +#define V4L2_FMT_FLAG_CSC_QUANTIZATION		0x0020
>>  
>>  	/* Frame Size and frame rate enumeration */
>>  /*
>>

Re: [RFC v4 4/8] v4l2: add support for colorspace conversion API (CSC) for video capture and subdevices

[Philipp Zabel]

Hi Dafna,

On Mon, 2020-06-08 at 12:00 +0200, Hans Verkuil wrote:
[...]
> > diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h
> > index 123a231001a8..0f916278137a 100644
> > --- a/include/uapi/linux/v4l2-mediabus.h
> > +++ b/include/uapi/linux/v4l2-mediabus.h
> > @@ -16,6 +16,8 @@
> >  #include <linux/types.h>
> >  #include <linux/videodev2.h>
> >  
> > +#define V4L2_MBUS_FRAMEFMT_SET_CSC	0x0001
> > +
> >  /**
> >   * struct v4l2_mbus_framefmt - frame format on the media bus
> >   * @width:	image width
> > @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt {
> >  	__u16			ycbcr_enc;
> >  	__u16			quantization;
> >  	__u16			xfer_func;
> > -	__u16			reserved[11];
> > +	__u16			flags;
> > +	__u16			reserved[10];
> >  };

The the flags field should also be added to the kerneldoc comment.

> >  
> >  #ifndef __KERNEL__
> > diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
> > index 5d2a1dab7911..972e64d8b54e 100644
> > --- a/include/uapi/linux/v4l2-subdev.h
> > +++ b/include/uapi/linux/v4l2-subdev.h
> > @@ -65,6 +65,8 @@ struct v4l2_subdev_crop {
> >  	__u32 reserved[8];
> >  };
> >  
> > +#define V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC	0x00000001
> > +#define V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION	0x00000002
> >  /**
> >   * struct v4l2_subdev_mbus_code_enum - Media bus format enumeration
> >   * @pad: pad number, as reported by the media API
> > @@ -77,7 +79,8 @@ struct v4l2_subdev_mbus_code_enum {
> >  	__u32 index;
> >  	__u32 code;
> >  	__u32 which;
> > -	__u32 reserved[8];
> > +	__u32 flags;
> > +	__u32 reserved[7];
> >  };

Same as above.

regards
Philipp

Re: [RFC v4 8/8] media: v4l2: add support for the CSC API for hsv_enc on mediabus

[Philipp Zabel]

Hi Dafna,

On Fri, 2020-06-05 at 19:26 +0200, Dafna Hirschfeld wrote:
[...]
> --- a/include/uapi/linux/v4l2-subdev.h
> +++ b/include/uapi/linux/v4l2-subdev.h
> @@ -66,6 +66,7 @@ struct v4l2_subdev_crop {
>  };
>  
>  #define V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC	0x00000001
> +#define V4L2_SUBDEV_MBUS_CODE_CSC_HSV_ENC	0x00000001

Hans' comment on patch 4 applies here as well, it would probably be
better to write:

#define V4L2_SUBDEV_MBUS_CODE_CSC_HSV_ENC	V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC
regards
Philipp

Re: [RFC v4 4/8] v4l2: add support for colorspace conversion API (CSC) for video capture and subdevices

[Dafna Hirschfeld]

On 26.06.20 14:22, Philipp Zabel wrote:
> Hi Dafna,
> 
> On Mon, 2020-06-08 at 12:00 +0200, Hans Verkuil wrote:
> [...]
>>> diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h
>>> index 123a231001a8..0f916278137a 100644
>>> --- a/include/uapi/linux/v4l2-mediabus.h
>>> +++ b/include/uapi/linux/v4l2-mediabus.h
>>> @@ -16,6 +16,8 @@
>>>   #include <linux/types.h>
>>>   #include <linux/videodev2.h>
>>>   
>>> +#define V4L2_MBUS_FRAMEFMT_SET_CSC	0x0001
>>> +
>>>   /**
>>>    * struct v4l2_mbus_framefmt - frame format on the media bus
>>>    * @width:	image width
>>> @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt {
>>>   	__u16			ycbcr_enc;
>>>   	__u16			quantization;
>>>   	__u16			xfer_func;
>>> -	__u16			reserved[11];
>>> +	__u16			flags;
>>> +	__u16			reserved[10];
>>>   };
> 
> The the flags field should also be added to the kerneldoc comment.

Hi, Which kerneldoc comment do you mean?
I added to the doc of the v4l2-mbus-framefmt:

     * - __u16
       - ``flags``
       - flags See:  :ref:v4l2-mbus-framefmt-flags
  

> 
>>>   
>>>   #ifndef __KERNEL__
>>> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
>>> index 5d2a1dab7911..972e64d8b54e 100644
>>> --- a/include/uapi/linux/v4l2-subdev.h
>>> +++ b/include/uapi/linux/v4l2-subdev.h
>>> @@ -65,6 +65,8 @@ struct v4l2_subdev_crop {
>>>   	__u32 reserved[8];
>>>   };
>>>   
>>> +#define V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC	0x00000001
>>> +#define V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION	0x00000002
>>>   /**
>>>    * struct v4l2_subdev_mbus_code_enum - Media bus format enumeration
>>>    * @pad: pad number, as reported by the media API
>>> @@ -77,7 +79,8 @@ struct v4l2_subdev_mbus_code_enum {
>>>   	__u32 index;
>>>   	__u32 code;
>>>   	__u32 which;
>>> -	__u32 reserved[8];
>>> +	__u32 flags;
>>> +	__u32 reserved[7];
>>>   };
> 
> Same as above.

Same questions as above.

Thanks,
Dafna

> 
> regards
> Philipp
> 

Re: [RFC v4 4/8] v4l2: add support for colorspace conversion API (CSC) for video capture and subdevices

[Dafna Hirschfeld]

Hi Hans and everyone,

On 26.06.20 01:29, Helen Koike wrote:
> 
> 
> On 6/5/20 2:26 PM, Dafna Hirschfeld wrote:
>> From: Philipp Zabel <p.zabel@pengutronix.de>
>>
>> For video capture it is the driver that reports the colorspace,
>> Y'CbCr/HSV encoding, quantization range and transfer function
>> used by the video, and there is no way to request something
>> different, even though many HDTV receivers have some sort of
>> colorspace conversion capabilities.
>>
>> For output video this feature already exists since the application
>> specifies this information for the video format it will send out, and
>> the transmitter will enable any available CSC if a format conversion has
>> to be performed in order to match the capabilities of the sink.
>>
>> For video capture we propose adding new v4l2_pix_format flag:
>> V4L2_PIX_FMT_FLAG_SET_CSC. The flag is set by the application,
>> the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields
>> as the requested colorspace information and will attempt to
>> do the conversion it supports.
>>
>> Drivers set the flags
>> V4L2_FMT_FLAG_CSC_YCBCR_ENC,
>> V4L2_FMT_FLAG_CSC_HSV_ENC,
>> V4L2_FMT_FLAG_CSC_QUANTIZATION,
>> in the flags field of the struct v4l2_fmtdesc during enumeration to
>> indicate that they support colorspace conversion for the respective field.
>> Currently the conversion of the fields 'colorspace' and 'xfer_func' is not
>> supported since there are no drivers that support it.
>>
>> The same API is added for the subdevices. With the flag
>> V4L2_MBUS_FRAMEFMT_SET_CSC set by the application in VIDIOC_SUBDEV_S_FMT
>> ioctl and the flags V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC,
>> V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION set by the driver in the
>> VIDIOC_SUBDEV_ENUM_MBUS_CODE ioctl.
>>
>> For subdevices, new 'flags' fields were added to the structs
>> v4l2_subdev_mbus_code_enum, v4l2_mbus_framefmt which are borrowed from the
>> 'reserved' field
>>
>> Drivers do not have to actually look at the flagsr. If the flags are not
>> set, then the colorspace, ycbcr_enc and quantization fields are set to
>> the default values by the core, i.e. just pass on the received format
>> without conversion.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>> ---
>>   .../media/v4l/pixfmt-v4l2-mplane.rst          | 16 ++----
>>   .../userspace-api/media/v4l/pixfmt-v4l2.rst   | 46 ++++++++++++++--
>>   .../media/v4l/subdev-formats.rst              | 52 +++++++++++++++++--
>>   .../media/v4l/vidioc-enum-fmt.rst             | 22 +++++++-
>>   .../v4l/vidioc-subdev-enum-mbus-code.rst      | 28 ++++++++++
>>   .../media/videodev2.h.rst.exceptions          |  3 ++
>>   include/uapi/linux/v4l2-mediabus.h            |  5 +-
>>   include/uapi/linux/v4l2-subdev.h              |  5 +-
>>   include/uapi/linux/videodev2.h                |  4 ++
>>   9 files changed, 160 insertions(+), 21 deletions(-)
>>
>> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst
>> index 444b4082684c..66f3365d7b72 100644
>> --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst
>> +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst
>> @@ -105,29 +105,21 @@ describing all planes of that format.
>>       * - __u8
>>         - ``ycbcr_enc``
>>         - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
>> -        This information supplements the ``colorspace`` and must be set by
>> -	the driver for capture streams and by the application for output
>> -	streams, see :ref:`colorspaces`.
>> +	See struct :c:type:`v4l2_pix_format`.
>>       * - __u8
>>         - ``hsv_enc``
>>         - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`.
>> -        This information supplements the ``colorspace`` and must be set by
>> -	the driver for capture streams and by the application for output
>> -	streams, see :ref:`colorspaces`.
>> +	See struct :c:type:`v4l2_pix_format`.
>>       * - }
>>         -
>>       * - __u8
>>         - ``quantization``
>>         - Quantization range, from enum :c:type:`v4l2_quantization`.
>> -        This information supplements the ``colorspace`` and must be set by
>> -	the driver for capture streams and by the application for output
>> -	streams, see :ref:`colorspaces`.
>> +	See struct :c:type:`v4l2_pix_format`.
>>       * - __u8
>>         - ``xfer_func``
>>         - Transfer function, from enum :c:type:`v4l2_xfer_func`.
>> -        This information supplements the ``colorspace`` and must be set by
>> -	the driver for capture streams and by the application for output
>> -	streams, see :ref:`colorspaces`.
>> +	See struct :c:type:`v4l2_pix_format`.
>>       * - __u8
>>         - ``reserved[7]``
>>         - Reserved for future extensions. Should be zeroed by drivers and
>> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
>> index ffa539592822..f23404efd90f 100644
>> --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
>> +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
>> @@ -148,13 +148,29 @@ Single-planar format structure
>>         - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
>>           This information supplements the ``colorspace`` and must be set by
>>   	the driver for capture streams and by the application for output
>> -	streams, see :ref:`colorspaces`.
>> +	streams, see :ref:`colorspaces`. If the application sets the
>> +	flag ``V4L2_PIX_FMT_FLAG_SET_CSC`` then the application can set
>> +	this field for a capture stream to request a specific Y'CbCr encoding
>> +	for the captured image data. If the driver cannot handle requested
>> +	conversion, it will return another supported encoding.
>> +	This field is ignored for HSV pixelformats. The driver indicates that
>> +	ycbcr_enc conversion is supported by setting the flag
>> +	V4L2_FMT_FLAG_CSC_YCBCR_ENC in the corresponding struct
>> +	:c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags`
>>       * - __u32
>>         - ``hsv_enc``
>>         - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`.
>>           This information supplements the ``colorspace`` and must be set by
>>   	the driver for capture streams and by the application for output
>> -	streams, see :ref:`colorspaces`.
>> +	streams, see :ref:`colorspaces`. If the application sets the flag
>> +	``V4L2_PIX_FMT_FLAG_SET_CSC`` then the application can set this
>> +	field for a capture stream to request a specific HSV encoding for the
>> +	captured image data. If the driver cannot handle requested
>> +	conversion, it will return another supported encoding.
>> +	This field is ignored for non-HSV pixelformats. The driver indicates
>> +	that hsv_enc conversion is supported by setting the flag
>> +	V4L2_FMT_FLAG_CSC_HSV_ENC in the corresponding struct
>> +	:c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags`
>>       * - }
>>         -
>>       * - __u32
>> @@ -162,7 +178,14 @@ Single-planar format structure
>>         - Quantization range, from enum :c:type:`v4l2_quantization`.
>>           This information supplements the ``colorspace`` and must be set by
>>   	the driver for capture streams and by the application for output
>> -	streams, see :ref:`colorspaces`.
>> +	streams, see :ref:`colorspaces`. If the application sets the flag
>> +	``V4L2_PIX_FMT_FLAG_SET_CSC`` then the application can set
>> +	this field for a capture stream to request a specific quantization
>> +	range for the captured image data. If the driver cannot handle requested
>> +	conversion, it will return another supported encoding.
>> +	The driver indicates that quantization conversion is supported by setting
>> +	the flag V4L2_FMT_FLAG_CSC_QUANTIZATION in the corresponding struct
>> +	:c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags`
>>       * - __u32
>>         - ``xfer_func``
>>         - Transfer function, from enum :c:type:`v4l2_xfer_func`.
>> @@ -186,3 +209,20 @@ Single-planar format structure
>>   	by RGBA values (128, 192, 255, 128), the same pixel described with
>>   	premultiplied colors would be described by RGBA values (64, 96,
>>   	128, 128)
>> +    * .. _`v4l2-pix-fmt-flag-set-csc`:
>> +
>> +      - ``V4L2_PIX_FMT_FLAG_SET_CSC``
>> +      - 0x00000002
>> +      - Set by the application. It is only used for capture and is
>> +        ignored for output streams. If set, then request the device to do
>> +	colorspace conversion from the received colorspace to the requested
>> +	colorspace values. If colorimetry field (``ycncr_enc``, ``hsv_enc``
>> +	or ``quantization``) is set to 0, then that colorimetry setting will
>> +	remain unchanged from what was received. So to change the quantization
>> +	only the ``quantization`` field shall be set to non-zero values
>> +	(``V4L2_QUANTIZATION_FULL_RANGE`` or ``V4L2_QUANTIZATION_LIM_RANGE``)
>> +	and all other colorimetry fields shall be set to 0. The API does not
>> +	support the conversion of the fields ``colorspace`` and ``xfer_func``
>> +
>> +	To check which conversions are supported by the hardware for the current
>> +	pixel format, see :ref:`fmtdesc-flags`.
>> diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
>> index 9a4d61b0d76f..75eb7f8bb4c5 100644
>> --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
>> +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
>> @@ -49,13 +49,32 @@ Media Bus Formats
>>         - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
>>           This information supplements the ``colorspace`` and must be set by
>>   	the driver for capture streams and by the application for output
>> -	streams, see :ref:`colorspaces`.
>> +	streams, see :ref:`colorspaces`. If the application sets the
>> +	flag ``V4L2_MBUS_FRAMEFMT_SET_CSC`` then the application can set
>> +	this field for a capture stream to request a specific Y'CbCr encoding

As Tomasz already mentioned, the terms 'capture/output stream' is not relevant to
sub-devices, right?
I wonder if there is any subdevice that looks at the colorspace fields
from userspace? maybe the doc can change to:

This information supplements the ``colorspace`` and must be set by the driver,
see :ref:`colorspaces`. If the application sets the flag ``V4L2_MBUS_FRAMEFMT_SET_CSC``
then the application can set this field to request a specific Y'CbCr encoding.
...

Thanks,
Dafna



>> +	for the media bus data. If the driver cannot handle requested
>> +	conversion, it will return another supported encoding.
>> +	This field is ignored for HSV media bus formats. The driver indicates
>> +	that ycbcr_enc conversion is supported by setting the flag
>> +	V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC in the corresponding struct
>> +	:c:type:`v4l2_subdev_mbus_code_enum` during enumeration.
>> +	See :ref:`v4l2-subdev-mbus-code-flags`
>> +
>>       * - __u16
>>         - ``quantization``
>>         - Quantization range, from enum :c:type:`v4l2_quantization`.
>>           This information supplements the ``colorspace`` and must be set by
>>   	the driver for capture streams and by the application for output
>> -	streams, see :ref:`colorspaces`.
>> +	streams, see :ref:`colorspaces`. If the application sets the
>> +	flag ``V4L2_MBUS_FRAMEFMT_SET_CSC`` then the application can set
>> +	this field for a capture stream to request a specific quantization
>> +	encoding for the media bus data. If the driver cannot handle requested
>> +	conversion, it will return another supported encoding.
>> +	The driver indicates that quantization conversion is supported by
>> +	setting the flag V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION in the
>> +	corresponding struct :c:type:`v4l2_subdev_mbus_code_enum`
>> +	during enumeration. See :ref:`v4l2-subdev-mbus-code-flags`
>> +
>>       * - __u16
>>         - ``xfer_func``
>>         - Transfer function, from enum :c:type:`v4l2_xfer_func`.
>> @@ -63,10 +82,37 @@ Media Bus Formats
>>   	the driver for capture streams and by the application for output
>>   	streams, see :ref:`colorspaces`.
>>       * - __u16
>> -      - ``reserved``\ [11]
>> +      - ``flags``
>> +      - flags See:  :ref:v4l2-mbus-framefmt-flags
>> +    * - __u16
>> +      - ``reserved``\ [10]
>>         - Reserved for future extensions. Applications and drivers must set
>>   	the array to zero.
>>   
>> +.. _v4l2-mbus-framefmt-flags:
>> +
>> +.. flat-table:: v4l2_mbus_framefmt Flags
>> +    :header-rows:  0
>> +    :stub-columns: 0
>> +    :widths:       3 1 4
>> +
>> +    * .. _`mbus-framefmt-set-csc`:
>> +
>> +      - ``V4L2_MBUS_FRAMEFMT_SET_CSC``
>> +      - 0x0001
>> +      - Set by the application. It is only used for capture and is
>> +	ignored for output streams. If set, then request the subdevice to do
>> +	colorspace conversion from the received colorspace to the requested
>> +	colorspace values. If colorimetry field (``ycbcr_enc`` or
>> +	``quantization``) is set to 0, then that colorimetry setting will remain
>> +	unchanged from what was received. So to change the quantization, only the
>> +	``quantization`` field shall be set to non-zero values
>> +	(``V4L2_QUANTIZATION_FULL_RANGE`` or ``V4L2_QUANTIZATION_LIM_RANGE``)
>> +	and all other colorimetry fields shall be set to 0. The API does not
>> +	support the conversion of the fields ``colorspace`` and ``xfer_func``.
>> +
>> +	To check which conversions are supported by the hardware for the current
>> +	media bus frame format, see :ref:`v4l2-mbus-framefmt-flags`.
>>   
>>   
>>   .. _v4l2-mbus-pixelcode:
>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
>> index a53dd3d7f7e2..11323755d41b 100644
>> --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
>> +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
>> @@ -178,7 +178,27 @@ the ``mbus_code`` field is handled differently:
>>   	parameters are detected. This flag can only be used in combination
>>   	with the ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to
>>   	compressed formats only. It is also only applies to stateful codecs.
>> -
>> +    * - ``V4L2_FMT_FLAG_CSC_YCBCR_ENC``
>> +      - 0x0010
>> +      - The driver allows the application to try to change the default
>> +	Y'CbCr encoding. This flag is relevant only for capture devices.
>> +	The application can ask to configure the ycbcr_enc of the capture device
>> +	when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
>> +	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
>> +    * - ``V4L2_FMT_FLAG_CSC_HSV_ENC``
>> +      - 0x0010
>> +      - The driver allows the application to try to change the default
>> +	HSV encoding. This flag is relevant only for capture devices.
>> +	The application can ask to configure the hsv_enc of the capture device
>> +	when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
>> +	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
>> +    * - ``V4L2_FMT_FLAG_CSC_QUANTIZATION``
>> +      - 0x0020
>> +      - The driver allows the application to try to change the default
>> +	quantization. This flag is relevant only for capture devices.
>> +	The application can ask to configure the quantization of the capture
>> +	device when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
>> +	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
>>   
>>   Return Value
>>   ============
>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
>> index 35b8607203a4..3d3430bdd71f 100644
>> --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
>> +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
>> @@ -78,12 +78,40 @@ information about the try formats.
>>         - ``which``
>>         - Media bus format codes to be enumerated, from enum
>>   	:ref:`v4l2_subdev_format_whence <v4l2-subdev-format-whence>`.
>> +    * - __u32
>> +      - ``flags``
>> +      - See :ref:`v4l2-subdev-mbus-code-flags`
>>       * - __u32
>>         - ``reserved``\ [8]
>>         - Reserved for future extensions. Applications and drivers must set
>>   	the array to zero.
>>   
>>   
>> +
>> +.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{7.7cm}|
>> +
>> +.. _v4l2-subdev-mbus-code-flags:
>> +
>> +.. flat-table:: Subdev Media Bus Code Enumerate Flags
>> +    :header-rows:  0
>> +    :stub-columns: 0
>> +    :widths:       1 1 2
>> +
>> +    * - V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC
>> +      - 0x00000001
>> +      - The driver allows the application to try to change the default Y'CbCr
>> +	encoding. The application can ask to configure the ycbcr_enc of the
>> +	subdevice when calling the :ref:`VIDIOC_SUBDEV_S_FMT <VIDIOC_SUBDEV_G_FMT>`
>> +	ioctl with :ref:`V4L2_MBUS_FRAMEFMT_SET_CSC <mbus-framefmt-set-csc>` set.
>> +	See :ref:`v4l2-mbus-format` on how to do this.
>> +    * - V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION
>> +      - 0x00000002
>> +      - The driver allows the application to try to change the default
>> +	quantization. The application can ask to configure the quantization of
>> +	the subdevice when calling the :ref:`VIDIOC_SUBDEV_S_FMT <VIDIOC_SUBDEV_G_FMT>`
>> +	ioctl with :ref:`V4L2_MBUS_FRAMEFMT_SET_CSC <mbus-framefmt-set-csc>` set.
>> +	See :ref:`v4l2-mbus-format` on how to do this.
>> +
>>   Return Value
>>   ============
>>   
>> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>> index 564a3bf5bc6d..f7be008cd479 100644
>> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>> @@ -187,6 +187,9 @@ replace define V4L2_FMT_FLAG_COMPRESSED fmtdesc-flags
>>   replace define V4L2_FMT_FLAG_EMULATED fmtdesc-flags
>>   replace define V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM fmtdesc-flags
>>   replace define V4L2_FMT_FLAG_DYN_RESOLUTION fmtdesc-flags
>> +replace define V4L2_FMT_FLAG_CSC_YCBCR_ENC fmtdesc-flags
>> +replace define V4L2_FMT_FLAG_CSC_HSV_ENC fmtdesc-flags
>> +replace define V4L2_FMT_FLAG_CSC_QUANTIZATION fmtdesc-flags
>>   
>>   # V4L2 timecode types
>>   replace define V4L2_TC_TYPE_24FPS timecode-type
>> diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h
>> index 123a231001a8..0f916278137a 100644
>> --- a/include/uapi/linux/v4l2-mediabus.h
>> +++ b/include/uapi/linux/v4l2-mediabus.h
>> @@ -16,6 +16,8 @@
>>   #include <linux/types.h>
>>   #include <linux/videodev2.h>
>>   
>> +#define V4L2_MBUS_FRAMEFMT_SET_CSC	0x0001
>> +
>>   /**
>>    * struct v4l2_mbus_framefmt - frame format on the media bus
>>    * @width:	image width
>> @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt {
>>   	__u16			ycbcr_enc;
>>   	__u16			quantization;
>>   	__u16			xfer_func;
>> -	__u16			reserved[11];
>> +	__u16			flags;
>> +	__u16			reserved[10];
>>   };
>>   
>>   #ifndef __KERNEL__
>> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
>> index 5d2a1dab7911..972e64d8b54e 100644
>> --- a/include/uapi/linux/v4l2-subdev.h
>> +++ b/include/uapi/linux/v4l2-subdev.h
>> @@ -65,6 +65,8 @@ struct v4l2_subdev_crop {
>>   	__u32 reserved[8];
>>   };
>>   
>> +#define V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC	0x00000001
>> +#define V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION	0x00000002
>>   /**
>>    * struct v4l2_subdev_mbus_code_enum - Media bus format enumeration
>>    * @pad: pad number, as reported by the media API
>> @@ -77,7 +79,8 @@ struct v4l2_subdev_mbus_code_enum {
>>   	__u32 index;
>>   	__u32 code;
>>   	__u32 which;
>> -	__u32 reserved[8];
>> +	__u32 flags;
>> +	__u32 reserved[7];
>>   };
>>   
>>   /**
>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> index c3a1cf1c507f..15824316e0ca 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -774,6 +774,7 @@ struct v4l2_pix_format {
>>   
>>   /* Flags */
>>   #define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA	0x00000001
>> +#define V4L2_PIX_FMT_FLAG_SET_CSC	0x00000002
>>   
>>   /*
>>    *	F O R M A T   E N U M E R A T I O N
>> @@ -792,6 +793,9 @@ struct v4l2_fmtdesc {
>>   #define V4L2_FMT_FLAG_EMULATED			0x0002
>>   #define V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM	0x0004
>>   #define V4L2_FMT_FLAG_DYN_RESOLUTION		0x0008
>> +#define V4L2_FMT_FLAG_CSC_YCBCR_ENC		0x0010
>> +#define V4L2_FMT_FLAG_CSC_HSV_ENC		0x0010
> 
> Shouldn't those have different values? Or is this intentional?
> 
> Regards,
> Helen
> 
>> +#define V4L2_FMT_FLAG_CSC_QUANTIZATION		0x0020
>>   
>>   	/* Frame Size and frame rate enumeration */
>>   /*
>>

Re: [RFC v4 4/8] v4l2: add support for colorspace conversion API (CSC) for video capture and subdevices

[Philipp Zabel]

On Tue, 2020-06-30 at 17:23 +0200, Dafna Hirschfeld wrote:
> 
> On 26.06.20 14:22, Philipp Zabel wrote:
> > Hi Dafna,
> > 
> > On Mon, 2020-06-08 at 12:00 +0200, Hans Verkuil wrote:
> > [...]
> > > > diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h
> > > > index 123a231001a8..0f916278137a 100644
> > > > --- a/include/uapi/linux/v4l2-mediabus.h
> > > > +++ b/include/uapi/linux/v4l2-mediabus.h
> > > > @@ -16,6 +16,8 @@
> > > >   #include <linux/types.h>
> > > >   #include <linux/videodev2.h>
> > > >   
> > > > +#define V4L2_MBUS_FRAMEFMT_SET_CSC	0x0001
> > > > +
> > > >   /**
> > > >    * struct v4l2_mbus_framefmt - frame format on the media bus
> > > >    * @width:	image width

             ^^^^^
             this one

> > > > @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt {
> > > >   	__u16			ycbcr_enc;
> > > >   	__u16			quantization;
> > > >   	__u16			xfer_func;
> > > > -	__u16			reserved[11];
> > > > +	__u16			flags;
> > > > +	__u16			reserved[10];
> > > >   };
> > 
> > The the flags field should also be added to the kerneldoc comment.
> 
> Hi, Which kerneldoc comment do you mean?
> I added to the doc of the v4l2-mbus-framefmt:

See above, I meant the comment right above the structure definition.

regards
Philipp