[PATCH v4 00/10] Add support for multi-planar formats and 10 bit formats

[PATCH v4 00/10] Add support for multi-planar formats and 10 bit formats

[Satish Kumar Nagireddy]

 The patches are for xilinx v4l. The patcheset enable support to handle multiplanar
 formats and 10 bit formats. The implemenation has handling of single plane formats
 too for backward compatibility of some existing applications.

Changes in v4 (Thanks to Sakari Ailus, Hyun Kwon and Ian Arkver):
 - rst documentation is moved to 24 bit yuv formats group
 - Single plane implementation is removed as multi-plane supports both
 - num_buffers and bpl_factor parameters are removed to have clean
   implementation
 - macropixel concept is used to calculate number of bytes in a row
   for 10 bit formats
 - Video format descriptor table updated with 10 bit format information

Changes in v3:
 - Fixed table alignment issue in rst file. Ensured the output is proper uisng
   'make pdfdocs'

Changes in v2:
 - Added rst documentation for MEDIA_BUS_FMT_VYYUYY8_1X24

Jeffrey Mouroux (2):
  Documentation: uapi: media: v4l: New pixel format
  uapi: media: New fourcc codes needed by Xilinx Video IP

Laurent Pinchart (1):
  xilinx: v4l: dma: Use the dmaengine_terminate_all() wrapper

Radhey Shyam Pandey (1):
  v4l: xilinx: dma: Remove colorspace check in xvip_dma_verify_format

Rohit Athavale (2):
  xilinx: v4l: dma: Update driver to allow for probe defer
  media: Add new dt-bindings/vf_codes for supported formats

Satish Kumar Nagireddy (4):
  media-bus: uapi: Add YCrCb 420 media bus format
  v4l: xilinx: dma: Update video format descriptor
  v4l: xilinx: dma: Add multi-planar support
  v4l: xilinx: dma: Add support for 10 bit formats

 Documentation/media/uapi/v4l/pixfmt-xv15.rst    | 135 ++++++++++++++++++
 Documentation/media/uapi/v4l/pixfmt-xv20.rst    | 136 ++++++++++++++++++
 Documentation/media/uapi/v4l/subdev-formats.rst |  38 ++++-
 Documentation/media/uapi/v4l/yuv-formats.rst    |   2 +
 drivers/media/platform/xilinx/xilinx-dma.c      | 177 +++++++++++++++---------
 drivers/media/platform/xilinx/xilinx-dma.h      |   4 +-
 drivers/media/platform/xilinx/xilinx-vip.c      |  45 ++++--
 drivers/media/platform/xilinx/xilinx-vip.h      |  13 +-
 drivers/media/platform/xilinx/xilinx-vipp.c     |  16 +--
 include/dt-bindings/media/xilinx-vip.h          |   2 +
 include/uapi/linux/media-bus-format.h           |   3 +-
 include/uapi/linux/videodev2.h                  |   4 +
 12 files changed, 488 insertions(+), 87 deletions(-)
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-xv15.rst
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-xv20.rst

-- 
2.1.1

[PATCH v4 01/10] v4l: xilinx: dma: Remove colorspace check in xvip_dma_verify_format

[Satish Kumar Nagireddy]

From: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>

In current implementation driver only checks the colorspace
between the last subdev in the pipeline and the connected video node,
the pipeline could be configured with wrong colorspace information
until the very end. It thus makes little sense to check the
colorspace only at the video node. So check can be dropped until
we find a better solution to carry colorspace information
through pipelines and to userspace.

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com>
---
 drivers/media/platform/xilinx/xilinx-dma.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
index 522cdfd..cb20ada 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -75,8 +75,7 @@ static int xvip_dma_verify_format(struct xvip_dma *dma)
 
 	if (dma->fmtinfo->code != fmt.format.code ||
 	    dma->format.height != fmt.format.height ||
-	    dma->format.width != fmt.format.width ||
-	    dma->format.colorspace != fmt.format.colorspace)
+	    dma->format.width != fmt.format.width)
 		return -EINVAL;
 
 	return 0;
-- 
2.1.1

[PATCH v4 02/10] xilinx: v4l: dma: Use the dmaengine_terminate_all() wrapper

[Satish Kumar Nagireddy]

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Calling dmaengine_device_control() to terminate transfers is an internal
API that will disappear, use the stable API wrapper instead.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com>
---
 drivers/media/platform/xilinx/xilinx-dma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
index cb20ada..a5bf345 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -434,6 +434,7 @@ static int xvip_dma_start_streaming(struct vb2_queue *vq, unsigned int count)
 	return 0;
 
 error_stop:
+	dmaengine_terminate_all(dma->dma);
 	media_pipeline_stop(&dma->video.entity);
 
 error:
-- 
2.1.1

Re: [PATCH v4 02/10] xilinx: v4l: dma: Use the dmaengine_terminate_all() wrapper

[Hyun Kwon]

Hi Satish,

Thanks for the patch.

On Mon, 2018-04-30 at 18:35:05 -0700, Satish Kumar Nagireddy wrote:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Calling dmaengine_device_control() to terminate transfers is an internal
> API that will disappear, use the stable API wrapper instead.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com>
> ---
>  drivers/media/platform/xilinx/xilinx-dma.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
> index cb20ada..a5bf345 100644
> --- a/drivers/media/platform/xilinx/xilinx-dma.c
> +++ b/drivers/media/platform/xilinx/xilinx-dma.c
> @@ -434,6 +434,7 @@ static int xvip_dma_start_streaming(struct vb2_queue *vq, unsigned int count)
>  	return 0;
>  
>  error_stop:
> +	dmaengine_terminate_all(dma->dma);

The patch and change are incorrectly mapped. The change adds dma termination
on error, which doesn't match with patch description.

And this API is deprecated. Please use dmaengine_terminate_sync() instead.
Probably it makes sense to change another call of dmaengine_terminate_all()
in this file. You can also do it in a separate patch. Up to you.

Thanks,
-hyun

>  	media_pipeline_stop(&dma->video.entity);
>  
>  error:
> -- 
> 2.1.1
> 

[PATCH v4 03/10] xilinx: v4l: dma: Update driver to allow for probe defer

[Satish Kumar Nagireddy]

From: Rohit Athavale <rathaval@xilinx.com>

Update xvip_dma_init() to use dma_request_chan(), enabling probe
deferral. Also update the cleanup routine to prevent dereferencing
an ERR_PTR().

Signed-off-by: Rohit Athavale <rathaval@xilinx.com>
Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com>
---
 drivers/media/platform/xilinx/xilinx-dma.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
index a5bf345..16aeb46 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -730,10 +730,13 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma,
 
 	/* ... and the DMA channel. */
 	snprintf(name, sizeof(name), "port%u", port);
-	dma->dma = dma_request_slave_channel(dma->xdev->dev, name);
-	if (dma->dma == NULL) {
-		dev_err(dma->xdev->dev, "no VDMA channel found\n");
-		ret = -ENODEV;
+	dma->dma = dma_request_chan(dma->xdev->dev, name);
+	if (IS_ERR(dma->dma)) {
+		ret = PTR_ERR(dma->dma);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dma->xdev->dev,
+				"No Video DMA channel found");
+
 		goto error;
 	}
 
@@ -757,7 +760,7 @@ void xvip_dma_cleanup(struct xvip_dma *dma)
 	if (video_is_registered(&dma->video))
 		video_unregister_device(&dma->video);
 
-	if (dma->dma)
+	if (!IS_ERR(dma->dma))
 		dma_release_channel(dma->dma);
 
 	media_entity_cleanup(&dma->video.entity);
-- 
2.1.1

[PATCH v4 04/10] Documentation: uapi: media: v4l: New pixel format

[Satish Kumar Nagireddy]

From: Jeffrey Mouroux <jmouroux@xilinx.com>

These descriptions are for YUV 420 and YUV 422 10 bit
formats.

Signed-off-by: Jeffrey Mouroux <jmouroux@xilinx.com>
Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com>
---
 Documentation/media/uapi/v4l/pixfmt-xv15.rst | 135 ++++++++++++++++++++++++++
 Documentation/media/uapi/v4l/pixfmt-xv20.rst | 136 +++++++++++++++++++++++++++
 Documentation/media/uapi/v4l/yuv-formats.rst |   2 +
 3 files changed, 273 insertions(+)
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-xv15.rst
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-xv20.rst

diff --git a/Documentation/media/uapi/v4l/pixfmt-xv15.rst b/Documentation/media/uapi/v4l/pixfmt-xv15.rst
new file mode 100644
index 0000000..313d056
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-xv15.rst
@@ -0,0 +1,135 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _V4L2-PIX-FMT-XV15:
+.. _V4L2-PIX-FMT-XV15M:
+
+*******************************************************
+V4L2_PIX_FMT_XV15 ('XV15'), V4L2_PIX_FMT_XV15 ('XV15M')
+*******************************************************
+
+Semi-planar YUV 420 10-bit
+
+
+Description
+===========
+
+This is the 10-bit version of YUV 420 semi-planar format.
+XV15M differs from XV15 insofar as the chroma plane is not contiguous with the
+luma plane in memory.
+
+Each pixel of YUV 420 contains a single luma component of 10-bits in length.
+Three luma components are stored per word with the remaining two bits serving
+as padding.
+
+The chroma plane is subsampled and is only 1/2 the size of the luma plane.  A
+single chroma component serves two pixels on a given row and is re-used on the
+adjacent row of luma data.
+
+**Data Layout of Luma Plane**
+Each cell is one 32-bit word.
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+
+    * - word + 0:
+      - X'\ :sub:`[31:30]`
+      - Y'\ :sub:`02 [29:20]`
+      - Y'\ :sub:`01 [19:10]`
+      - Y'\ :sub:`00 [09:00]`
+      -
+    * - word + 1:
+      - X'\ :sub:`[31:30]`
+      - Y'\ :sub:`05 [29:20]`
+      - Y'\ :sub:`04 [19:10]`
+      - Y'\ :sub:`03 [09:00]`
+      -
+    * - word + 2:
+      - X'\ :sub:`[31:30]`
+      - Y'\ :sub:`08 [29:20]`
+      - Y'\ :sub:`07 [19:10]`
+      - Y'\ :sub:`06 [09:00]`
+      -
+
+
+**Data Layout of Chroma Plane**
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+
+    * - word + 0:
+      - X'\ :sub:`[31:30]`
+      - U'\ :sub:`02 [29:20]`
+      - V'\ :sub:`01 [19:10]`
+      - U'\ :sub:`00 [09:00]`
+      -
+    * - word + 1:
+      - X'\ :sub:`[31:30]`
+      - V'\ :sub:`05 [29:20]`
+      - U'\ :sub:`04 [19:10]`
+      - V'\ :sub:`03 [09:00]`
+      -
+    * - word + 2:
+      - X'\ :sub:`[31:30]`
+      - U'\ :sub:`08 [29:20]`
+      - V'\ :sub:`07 [19:10]`
+      - U'\ :sub:`06 [09:00]`
+      -
+
+**Color Sample Location**
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+
+    * -
+      - 0
+      -
+      - 1
+      - 2
+      -
+      - 3
+    * - 0
+      - Y
+      -
+      - Y
+      - Y
+      -
+      - Y
+    * -
+      -
+      - C
+      -
+      -
+      - C
+      -
+    * - 1
+      - Y
+      -
+      - Y
+      - Y
+      -
+      - Y
+    * -
+    * - 2
+      - Y
+      -
+      - Y
+      - Y
+      -
+      - Y
+    * -
+      -
+      - C
+      -
+      -
+      - C
+      -
+    * - 3
+      - Y
+      -
+      - Y
+      - Y
+      -
+      - Y
diff --git a/Documentation/media/uapi/v4l/pixfmt-xv20.rst b/Documentation/media/uapi/v4l/pixfmt-xv20.rst
new file mode 100644
index 0000000..fe9dac2
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-xv20.rst
@@ -0,0 +1,136 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _V4L2-PIX-FMT-XV20:
+.. _V4L2-PIX-FMT-XV20M:
+
+*******************************************************
+V4L2_PIX_FMT_XV20 ('XV20'), V4L2_PIX_FMT_XV20 ('XV20M')
+*******************************************************
+
+Semi-planar YUV422 10-bit
+
+
+Description
+===========
+
+This is the 10-bit version of YUV 422 semi-planar format.
+XV20M differs from XV20 insofar as the chroma plane is not contiquous with the
+luma plane in memory.
+
+
+Each pixel of YUV 422 contains a single luma component of 10-bits in length.
+Three luma components are stored per word with the remaining two bits serving
+as padding.
+
+The chroma plane is subsampled in and is the size of the luma plane.  A single
+chroma component (U or V) serves two pixels on a given row.
+
+**Data Layout of Luma Plane**
+Each cell is one 32-bit word.
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+
+    * - word + 0:
+      - X'\ :sub:`[31:30]`
+      - Y'\ :sub:`02 [29:20]`
+      - Y'\ :sub:`01 [19:10]`
+      - Y'\ :sub:`00 [09:00]`
+      -
+    * - word + 1:
+      - X'\ :sub:`[31:30]`
+      - Y'\ :sub:`05 [29:20]`
+      - Y'\ :sub:`04 [19:10]`
+      - Y'\ :sub:`03 [09:00]`
+      -
+    * - word + 2:
+      - X'\ :sub:`[31:30]`
+      - Y'\ :sub:`08 [29:20]`
+      - Y'\ :sub:`07 [19:10]`
+      - Y'\ :sub:`06 [09:00]`
+      -
+
+
+**Data Layout of Chroma Plane**
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+
+    * - word + 0:
+      - X'\ :sub:`[31:30]`
+      - U'\ :sub:`02 [29:20]`
+      - V'\ :sub:`01 [19:10]`
+      - U'\ :sub:`00 [09:00]`
+      -
+    * - word + 1:
+      - X'\ :sub:`[31:30]`
+      - V'\ :sub:`05 [29:20]`
+      - U'\ :sub:`04 [19:10]`
+      - V'\ :sub:`03 [09:00]`
+      -
+    * - word + 2:
+      - X'\ :sub:`[31:30]`
+      - U'\ :sub:`08 [29:20]`
+      - V'\ :sub:`07 [19:10]`
+      - U'\ :sub:`06 [09:00]`
+      -
+
+
+**Color Sample Location**
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+
+    * -
+      - 0
+      -
+      - 1
+      - 2
+      -
+      - 3
+    * - 0
+      - Y
+      -
+      - Y
+      - Y
+      -
+      - Y
+    * -
+      -
+      - C
+      -
+      -
+      - C
+      -
+    * - 1
+      - Y
+      -
+      - Y
+      - Y
+      -
+      - Y
+    * -
+    * - 2
+      - Y
+      -
+      - Y
+      - Y
+      -
+      - Y
+    * -
+      -
+      - C
+      -
+      -
+      - C
+      -
+    * - 3
+      - Y
+      -
+      - Y
+      - Y
+      -
+      - Y
diff --git a/Documentation/media/uapi/v4l/yuv-formats.rst b/Documentation/media/uapi/v4l/yuv-formats.rst
index 3334ea4..0e6b44f 100644
--- a/Documentation/media/uapi/v4l/yuv-formats.rst
+++ b/Documentation/media/uapi/v4l/yuv-formats.rst
@@ -49,7 +49,9 @@ to brightness information.
     pixfmt-nv12
     pixfmt-nv12m
     pixfmt-nv12mt
+    pixfmt-xv15
     pixfmt-nv16
     pixfmt-nv16m
+    pixfmt-xv20
     pixfmt-nv24
     pixfmt-m420
-- 
2.1.1

[PATCH v4 05/10] uapi: media: New fourcc codes needed by Xilinx Video IP

[Satish Kumar Nagireddy]

From: Jeffrey Mouroux <jmouroux@xilinx.com>

The Xilinx Video Framebuffer DMA IP supports video memory formats
that are not represented in the current V4L2 fourcc library. This
patch adds those missing fourcc codes. This includes both new
8-bit and 10-bit pixel formats.

Signed-off-by: Jeffrey Mouroux <jmouroux@xilinx.com>
Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com>
---
 include/uapi/linux/videodev2.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 600877b..97b6633 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -510,6 +510,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_RGB32   v4l2_fourcc('R', 'G', 'B', '4') /* 32  RGB-8-8-8-8   */
 #define V4L2_PIX_FMT_ARGB32  v4l2_fourcc('B', 'A', '2', '4') /* 32  ARGB-8-8-8-8  */
 #define V4L2_PIX_FMT_XRGB32  v4l2_fourcc('B', 'X', '2', '4') /* 32  XRGB-8-8-8-8  */
+#define V4L2_PIX_FMT_BGRX32  v4l2_fourcc('X', 'B', 'G', 'R') /* 32  XBGR-8-8-8-8 */
 
 /* Grey formats */
 #define V4L2_PIX_FMT_GREY    v4l2_fourcc('G', 'R', 'E', 'Y') /*  8  Greyscale     */
@@ -537,6 +538,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_VYUY    v4l2_fourcc('V', 'Y', 'U', 'Y') /* 16  YUV 4:2:2     */
 #define V4L2_PIX_FMT_Y41P    v4l2_fourcc('Y', '4', '1', 'P') /* 12  YUV 4:1:1     */
 #define V4L2_PIX_FMT_YUV444  v4l2_fourcc('Y', '4', '4', '4') /* 16  xxxxyyyy uuuuvvvv */
+#define V4L2_PIX_FMT_VUY24   v4l2_fourcc('V', 'U', '2', '4') /* 24  VUY 8:8:8 */
 #define V4L2_PIX_FMT_YUV555  v4l2_fourcc('Y', 'U', 'V', 'O') /* 16  YUV-5-5-5     */
 #define V4L2_PIX_FMT_YUV565  v4l2_fourcc('Y', 'U', 'V', 'P') /* 16  YUV-5-6-5     */
 #define V4L2_PIX_FMT_YUV32   v4l2_fourcc('Y', 'U', 'V', '4') /* 32  YUV-8-8-8-8   */
@@ -551,6 +553,8 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_NV61    v4l2_fourcc('N', 'V', '6', '1') /* 16  Y/CrCb 4:2:2  */
 #define V4L2_PIX_FMT_NV24    v4l2_fourcc('N', 'V', '2', '4') /* 24  Y/CbCr 4:4:4  */
 #define V4L2_PIX_FMT_NV42    v4l2_fourcc('N', 'V', '4', '2') /* 24  Y/CrCb 4:4:4  */
+#define V4L2_PIX_FMT_XV20    v4l2_fourcc('X', 'V', '2', '0') /* 32  XY/UV 4:2:2 10-bit */
+#define V4L2_PIX_FMT_XV15    v4l2_fourcc('X', 'V', '1', '5') /* 32  XY/UV 4:2:0 10-bit */
 
 /* two non contiguous planes - one Y, one Cr + Cb interleaved  */
 #define V4L2_PIX_FMT_NV12M   v4l2_fourcc('N', 'M', '1', '2') /* 12  Y/CbCr 4:2:0  */
-- 
2.1.1

Re: [PATCH v4 05/10] uapi: media: New fourcc codes needed by Xilinx Video IP

[Hyun Kwon]

Hi Satish,

Thanks for the patch.

On Mon, 2018-04-30 at 18:35:08 -0700, Satish Kumar Nagireddy wrote:
> From: Jeffrey Mouroux <jmouroux@xilinx.com>
> 
> The Xilinx Video Framebuffer DMA IP supports video memory formats
> that are not represented in the current V4L2 fourcc library. This
> patch adds those missing fourcc codes. This includes both new
> 8-bit and 10-bit pixel formats.
> 
> Signed-off-by: Jeffrey Mouroux <jmouroux@xilinx.com>
> Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com>
> ---
>  include/uapi/linux/videodev2.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 600877b..97b6633 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -510,6 +510,7 @@ struct v4l2_pix_format {
>  #define V4L2_PIX_FMT_RGB32   v4l2_fourcc('R', 'G', 'B', '4') /* 32  RGB-8-8-8-8   */
>  #define V4L2_PIX_FMT_ARGB32  v4l2_fourcc('B', 'A', '2', '4') /* 32  ARGB-8-8-8-8  */
>  #define V4L2_PIX_FMT_XRGB32  v4l2_fourcc('B', 'X', '2', '4') /* 32  XRGB-8-8-8-8  */
> +#define V4L2_PIX_FMT_BGRX32  v4l2_fourcc('X', 'B', 'G', 'R') /* 32  XBGR-8-8-8-8 */
>  
>  /* Grey formats */
>  #define V4L2_PIX_FMT_GREY    v4l2_fourcc('G', 'R', 'E', 'Y') /*  8  Greyscale     */
> @@ -537,6 +538,7 @@ struct v4l2_pix_format {
>  #define V4L2_PIX_FMT_VYUY    v4l2_fourcc('V', 'Y', 'U', 'Y') /* 16  YUV 4:2:2     */
>  #define V4L2_PIX_FMT_Y41P    v4l2_fourcc('Y', '4', '1', 'P') /* 12  YUV 4:1:1     */
>  #define V4L2_PIX_FMT_YUV444  v4l2_fourcc('Y', '4', '4', '4') /* 16  xxxxyyyy uuuuvvvv */
> +#define V4L2_PIX_FMT_VUY24   v4l2_fourcc('V', 'U', '2', '4') /* 24  VUY 8:8:8 */

This format needs documentation, and probably entries v4l_fill_fmtdesc()?

Then would it make sense to squash this into 4/10 which documents these?
Or at least, this should come before documentation.

Thanks,
-hyun

>  #define V4L2_PIX_FMT_YUV555  v4l2_fourcc('Y', 'U', 'V', 'O') /* 16  YUV-5-5-5     */
>  #define V4L2_PIX_FMT_YUV565  v4l2_fourcc('Y', 'U', 'V', 'P') /* 16  YUV-5-6-5     */
>  #define V4L2_PIX_FMT_YUV32   v4l2_fourcc('Y', 'U', 'V', '4') /* 32  YUV-8-8-8-8   */
> @@ -551,6 +553,8 @@ struct v4l2_pix_format {
>  #define V4L2_PIX_FMT_NV61    v4l2_fourcc('N', 'V', '6', '1') /* 16  Y/CrCb 4:2:2  */
>  #define V4L2_PIX_FMT_NV24    v4l2_fourcc('N', 'V', '2', '4') /* 24  Y/CbCr 4:4:4  */
>  #define V4L2_PIX_FMT_NV42    v4l2_fourcc('N', 'V', '4', '2') /* 24  Y/CrCb 4:4:4  */
> +#define V4L2_PIX_FMT_XV20    v4l2_fourcc('X', 'V', '2', '0') /* 32  XY/UV 4:2:2 10-bit */
> +#define V4L2_PIX_FMT_XV15    v4l2_fourcc('X', 'V', '1', '5') /* 32  XY/UV 4:2:0 10-bit */
>  
>  /* two non contiguous planes - one Y, one Cr + Cb interleaved  */
>  #define V4L2_PIX_FMT_NV12M   v4l2_fourcc('N', 'M', '1', '2') /* 12  Y/CbCr 4:2:0  */
> -- 
> 2.1.1
> 

[PATCH v4 06/10] media-bus: uapi: Add YCrCb 420 media bus format

[Satish Kumar Nagireddy]

This commit adds YUV 420 media bus format. VYYUYY8_1X24
is an approximate way to descrive the pixels sent over
the bus.

This patch also contain rst documentation for media bus format.

Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com>
---
 Documentation/media/uapi/v4l/subdev-formats.rst | 38 ++++++++++++++++++++++++-
 include/uapi/linux/media-bus-format.h           |  3 +-
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/Documentation/media/uapi/v4l/subdev-formats.rst b/Documentation/media/uapi/v4l/subdev-formats.rst
index 9fcabe7..904c52b 100644
--- a/Documentation/media/uapi/v4l/subdev-formats.rst
+++ b/Documentation/media/uapi/v4l/subdev-formats.rst
@@ -6640,6 +6640,43 @@ the following codes.
       - u\ :sub:`2`
       - u\ :sub:`1`
       - u\ :sub:`0`
+    * .. _MEDIA-BUS-FMT-VYYUYY8-1X24:
+
+      - MEDIA_BUS_FMT_VYYUYY8_1X24
+      - 0x202c
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      - v\ :sub:`3`
+      - v\ :sub:`2`
+      - v\ :sub:`1`
+      - v\ :sub:`0`
+      - y\ :sub:`7`
+      - y\ :sub:`6`
+      - y\ :sub:`5`
+      - y\ :sub:`4`
+      - y\ :sub:`3`
+      - y\ :sub:`2`
+      - y\ :sub:`1`
+      - y\ :sub:`0`
+      - u\ :sub:`3`
+      - u\ :sub:`2`
+      - u\ :sub:`1`
+      - u\ :sub:`0`
+      - y\ :sub:`7`
+      - y\ :sub:`6`
+      - y\ :sub:`5`
+      - y\ :sub:`4`
+      - y\ :sub:`3`
+      - y\ :sub:`2`
+      - y\ :sub:`1`
+      - y\ :sub:`0`
     * .. _MEDIA-BUS-FMT-YUV10-1X30:
 
       - MEDIA_BUS_FMT_YUV10_1X30
@@ -7287,7 +7324,6 @@ The following table list existing packed 48bit wide YUV formats.
       - y\ :sub:`1`
       - y\ :sub:`0`
 
-
 .. raw:: latex
 
 	\endgroup
diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h
index 9e35117..ade7e9d 100644
--- a/include/uapi/linux/media-bus-format.h
+++ b/include/uapi/linux/media-bus-format.h
@@ -62,7 +62,7 @@
 #define MEDIA_BUS_FMT_RGB121212_1X36		0x1019
 #define MEDIA_BUS_FMT_RGB161616_1X48		0x101a
 
-/* YUV (including grey) - next is	0x202c */
+/* YUV (including grey) - next is	0x202d */
 #define MEDIA_BUS_FMT_Y8_1X8			0x2001
 #define MEDIA_BUS_FMT_UV8_1X8			0x2015
 #define MEDIA_BUS_FMT_UYVY8_1_5X8		0x2002
@@ -106,6 +106,7 @@
 #define MEDIA_BUS_FMT_YUV12_1X36		0x2029
 #define MEDIA_BUS_FMT_YUV16_1X48		0x202a
 #define MEDIA_BUS_FMT_UYYVYY16_0_5X48		0x202b
+#define MEDIA_BUS_FMT_VYYUYY8_1X24		0x202c
 
 /* Bayer - next is	0x3021 */
 #define MEDIA_BUS_FMT_SBGGR8_1X8		0x3001
-- 
2.1.1

[PATCH v4 07/10] media: Add new dt-bindings/vf_codes for supported formats

[Satish Kumar Nagireddy]

From: Rohit Athavale <rathaval@xilinx.com>

This commit adds new entries to the exisiting vf_codes that are used
to describe the media bus formats in the DT bindings. The newly added
8-bit and 10-bit color depth related formats will need these updates.

Signed-off-by: Rohit Athavale <rathaval@xilinx.com>
Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com>
---
 include/dt-bindings/media/xilinx-vip.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/dt-bindings/media/xilinx-vip.h b/include/dt-bindings/media/xilinx-vip.h
index 6298fec..fcd34d7 100644
--- a/include/dt-bindings/media/xilinx-vip.h
+++ b/include/dt-bindings/media/xilinx-vip.h
@@ -35,5 +35,7 @@
 #define XVIP_VF_CUSTOM2			13
 #define XVIP_VF_CUSTOM3			14
 #define XVIP_VF_CUSTOM4			15
+#define XVIP_VF_VUY_422			16
+#define XVIP_VF_XBGR			17
 
 #endif /* __DT_BINDINGS_MEDIA_XILINX_VIP_H__ */
-- 
2.1.1

Re: [PATCH v4 07/10] media: Add new dt-bindings/vf_codes for supported formats

[Hyun Kwon]

On Mon, 2018-04-30 at 18:35:10 -0700, Satish Kumar Nagireddy wrote:
> From: Rohit Athavale <rathaval@xilinx.com>
> 
> This commit adds new entries to the exisiting vf_codes that are used
> to describe the media bus formats in the DT bindings. The newly added
> 8-bit and 10-bit color depth related formats will need these updates.
> 
> Signed-off-by: Rohit Athavale <rathaval@xilinx.com>
> Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com>
> ---
>  include/dt-bindings/media/xilinx-vip.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/dt-bindings/media/xilinx-vip.h b/include/dt-bindings/media/xilinx-vip.h
> index 6298fec..fcd34d7 100644
> --- a/include/dt-bindings/media/xilinx-vip.h
> +++ b/include/dt-bindings/media/xilinx-vip.h
> @@ -35,5 +35,7 @@
>  #define XVIP_VF_CUSTOM2			13
>  #define XVIP_VF_CUSTOM3			14
>  #define XVIP_VF_CUSTOM4			15
> +#define XVIP_VF_VUY_422			16
> +#define XVIP_VF_XBGR			17

The existing ones are from UG934, while new ones are not. We need to organize
this a little differently, and this should come with more details.

Thanks,
-hyun

>  
>  #endif /* __DT_BINDINGS_MEDIA_XILINX_VIP_H__ */
> -- 
> 2.1.1
> 

[PATCH v4 08/10] v4l: xilinx: dma: Update video format descriptor

[Satish Kumar Nagireddy]

This patch updates video format descriptor to help information
viz., number of planes per color format and chroma sub sampling
factors.

Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com>
---
 drivers/media/platform/xilinx/xilinx-dma.c | 12 ++++++------
 drivers/media/platform/xilinx/xilinx-vip.c | 28 +++++++++++++++++++---------
 drivers/media/platform/xilinx/xilinx-vip.h |  8 +++++++-
 3 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
index 16aeb46..658586e 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -366,7 +366,7 @@ static void xvip_dma_buffer_queue(struct vb2_buffer *vb)
 	}
 
 	dma->xt.frame_size = 1;
-	dma->sgl[0].size = dma->format.width * dma->fmtinfo->bpp;
+	dma->sgl[0].size = dma->format.width * dma->fmtinfo->bpp[0];
 	dma->sgl[0].icg = dma->format.bytesperline - dma->sgl[0].size;
 	dma->xt.numf = dma->format.height;
 
@@ -569,12 +569,12 @@ __xvip_dma_try_format(struct xvip_dma *dma, struct v4l2_pix_format *pix,
 	 * the minimum and maximum values, clamp the requested width and convert
 	 * it back to pixels.
 	 */
-	align = lcm(dma->align, info->bpp);
+	align = lcm(dma->align, info->bpp[0]);
 	min_width = roundup(XVIP_DMA_MIN_WIDTH, align);
 	max_width = rounddown(XVIP_DMA_MAX_WIDTH, align);
-	width = rounddown(pix->width * info->bpp, align);
+	width = rounddown(pix->width * info->bpp[0], align);
 
-	pix->width = clamp(width, min_width, max_width) / info->bpp;
+	pix->width = clamp(width, min_width, max_width) / info->bpp[0];
 	pix->height = clamp(pix->height, XVIP_DMA_MIN_HEIGHT,
 			    XVIP_DMA_MAX_HEIGHT);
 
@@ -582,7 +582,7 @@ __xvip_dma_try_format(struct xvip_dma *dma, struct v4l2_pix_format *pix,
 	 * line value is zero, the module doesn't support user configurable line
 	 * sizes. Override the requested value with the minimum in that case.
 	 */
-	min_bpl = pix->width * info->bpp;
+	min_bpl = pix->width * info->bpp[0];
 	max_bpl = rounddown(XVIP_DMA_MAX_WIDTH, dma->align);
 	bpl = rounddown(pix->bytesperline, dma->align);
 
@@ -676,7 +676,7 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma,
 	dma->format.field = V4L2_FIELD_NONE;
 	dma->format.width = XVIP_DMA_DEF_WIDTH;
 	dma->format.height = XVIP_DMA_DEF_HEIGHT;
-	dma->format.bytesperline = dma->format.width * dma->fmtinfo->bpp;
+	dma->format.bytesperline = dma->format.width * dma->fmtinfo->bpp[0];
 	dma->format.sizeimage = dma->format.bytesperline * dma->format.height;
 
 	/* Initialize the media entity... */
diff --git a/drivers/media/platform/xilinx/xilinx-vip.c b/drivers/media/platform/xilinx/xilinx-vip.c
index 3112591..81cc0d2 100644
--- a/drivers/media/platform/xilinx/xilinx-vip.c
+++ b/drivers/media/platform/xilinx/xilinx-vip.c
@@ -27,22 +27,32 @@
  */
 
 static const struct xvip_video_format xvip_video_formats[] = {
+	{ XVIP_VF_YUV_420, 8, NULL, MEDIA_BUS_FMT_VYYUYY8_1X24,
+	  {1, 2, 0}, V4L2_PIX_FMT_NV12, 2, 2, 2, "4:2:0, semi-planar, YUV" },
+	{ XVIP_VF_YUV_420, 10, NULL, MEDIA_BUS_FMT_VYYUYY8_1X24,
+	  {1, 2, 0}, V4L2_PIX_FMT_XV15, 2, 2, 2, "4:2:0, 10-bit 2-plane cont" },
 	{ XVIP_VF_YUV_422, 8, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
-	  2, V4L2_PIX_FMT_YUYV, "4:2:2, packed, YUYV" },
-	{ XVIP_VF_YUV_444, 8, NULL, MEDIA_BUS_FMT_VUY8_1X24,
-	  3, V4L2_PIX_FMT_YUV444, "4:4:4, packed, YUYV" },
+	  {2, 0, 0}, V4L2_PIX_FMT_YUYV, 1, 2, 1, "4:2:2, packed, YUYV" },
+	{ XVIP_VF_VUY_422, 8, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
+	  {2, 0, 0}, V4L2_PIX_FMT_UYVY, 1, 2, 1, "4:2:2, packed, UYVY" },
+	{ XVIP_VF_YUV_422, 8, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
+	  {1, 2, 0}, V4L2_PIX_FMT_NV16, 2, 2, 1, "4:2:2, semi-planar, YUV" },
+	{ XVIP_VF_YUV_422, 10, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
+	  {1, 2, 0}, V4L2_PIX_FMT_XV20, 2, 2, 1, "4:2:2, 10-bit 2-plane cont" },
+	{ XVIP_VF_RBG, 8, NULL, MEDIA_BUS_FMT_RBG888_1X24,
+	  {3, 0, 0}, V4L2_PIX_FMT_BGR24, 1, 1, 1, "24-bit RGB" },
 	{ XVIP_VF_RBG, 8, NULL, MEDIA_BUS_FMT_RBG888_1X24,
-	  3, 0, NULL },
+	  {3, 0, 0}, V4L2_PIX_FMT_RGB24, 1, 1, 1, "24-bit RGB" },
 	{ XVIP_VF_MONO_SENSOR, 8, "mono", MEDIA_BUS_FMT_Y8_1X8,
-	  1, V4L2_PIX_FMT_GREY, "Greyscale 8-bit" },
+	  {1, 0, 0}, V4L2_PIX_FMT_GREY, 1, 1, 1, "Greyscale 8-bit" },
 	{ XVIP_VF_MONO_SENSOR, 8, "rggb", MEDIA_BUS_FMT_SRGGB8_1X8,
-	  1, V4L2_PIX_FMT_SGRBG8, "Bayer 8-bit RGGB" },
+	  {1, 0, 0}, V4L2_PIX_FMT_SGRBG8, 1, 1, 1, "Bayer 8-bit RGGB" },
 	{ XVIP_VF_MONO_SENSOR, 8, "grbg", MEDIA_BUS_FMT_SGRBG8_1X8,
-	  1, V4L2_PIX_FMT_SGRBG8, "Bayer 8-bit GRBG" },
+	  {1, 0, 0}, V4L2_PIX_FMT_SGRBG8, 1, 1, 1, "Bayer 8-bit GRBG" },
 	{ XVIP_VF_MONO_SENSOR, 8, "gbrg", MEDIA_BUS_FMT_SGBRG8_1X8,
-	  1, V4L2_PIX_FMT_SGBRG8, "Bayer 8-bit GBRG" },
+	  {1, 0, 0}, V4L2_PIX_FMT_SGBRG8, 1, 1, 1, "Bayer 8-bit GBRG" },
 	{ XVIP_VF_MONO_SENSOR, 8, "bggr", MEDIA_BUS_FMT_SBGGR8_1X8,
-	  1, V4L2_PIX_FMT_SBGGR8, "Bayer 8-bit BGGR" },
+	  {1, 0, 0}, V4L2_PIX_FMT_SBGGR8, 1, 1, 1, "Bayer 8-bit BGGR" },
 };
 
 /**
diff --git a/drivers/media/platform/xilinx/xilinx-vip.h b/drivers/media/platform/xilinx/xilinx-vip.h
index 42fee20..5e7a978 100644
--- a/drivers/media/platform/xilinx/xilinx-vip.h
+++ b/drivers/media/platform/xilinx/xilinx-vip.h
@@ -111,6 +111,9 @@ struct xvip_device {
  * @code: media bus format code
  * @bpp: bytes per pixel (when stored in memory)
  * @fourcc: V4L2 pixel format FCC identifier
+ * @num_planes: number of planes w.r.t. color format
+ * @hsub: Horizontal sampling factor of Chroma
+ * @vsub: Vertical sampling factor of Chroma
  * @description: format description, suitable for userspace
  */
 struct xvip_video_format {
@@ -118,8 +121,11 @@ struct xvip_video_format {
 	unsigned int width;
 	const char *pattern;
 	unsigned int code;
-	unsigned int bpp;
+	unsigned int bpp[3];
 	u32 fourcc;
+	u8 num_planes;
+	u8 hsub;
+	u8 vsub;
 	const char *description;
 };
 
-- 
2.1.1

Re: [PATCH v4 08/10] v4l: xilinx: dma: Update video format descriptor

[Hyun Kwon]

Hi Satish,

Thanks for the patch.

On Mon, 2018-04-30 at 18:35:11 -0700, Satish Kumar Nagireddy wrote:
> This patch updates video format descriptor to help information
> viz., number of planes per color format and chroma sub sampling
> factors.
> 
> Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com>
> ---
>  drivers/media/platform/xilinx/xilinx-dma.c | 12 ++++++------
>  drivers/media/platform/xilinx/xilinx-vip.c | 28 +++++++++++++++++++---------
>  drivers/media/platform/xilinx/xilinx-vip.h |  8 +++++++-
>  3 files changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
> index 16aeb46..658586e 100644
> --- a/drivers/media/platform/xilinx/xilinx-dma.c
> +++ b/drivers/media/platform/xilinx/xilinx-dma.c
> @@ -366,7 +366,7 @@ static void xvip_dma_buffer_queue(struct vb2_buffer *vb)
>  	}
>  
>  	dma->xt.frame_size = 1;
> -	dma->sgl[0].size = dma->format.width * dma->fmtinfo->bpp;
> +	dma->sgl[0].size = dma->format.width * dma->fmtinfo->bpp[0];
>  	dma->sgl[0].icg = dma->format.bytesperline - dma->sgl[0].size;
>  	dma->xt.numf = dma->format.height;
>  
> @@ -569,12 +569,12 @@ __xvip_dma_try_format(struct xvip_dma *dma, struct v4l2_pix_format *pix,
>  	 * the minimum and maximum values, clamp the requested width and convert
>  	 * it back to pixels.
>  	 */
> -	align = lcm(dma->align, info->bpp);
> +	align = lcm(dma->align, info->bpp[0]);
>  	min_width = roundup(XVIP_DMA_MIN_WIDTH, align);
>  	max_width = rounddown(XVIP_DMA_MAX_WIDTH, align);
> -	width = rounddown(pix->width * info->bpp, align);
> +	width = rounddown(pix->width * info->bpp[0], align);
>  
> -	pix->width = clamp(width, min_width, max_width) / info->bpp;
> +	pix->width = clamp(width, min_width, max_width) / info->bpp[0];
>  	pix->height = clamp(pix->height, XVIP_DMA_MIN_HEIGHT,
>  			    XVIP_DMA_MAX_HEIGHT);
>  
> @@ -582,7 +582,7 @@ __xvip_dma_try_format(struct xvip_dma *dma, struct v4l2_pix_format *pix,
>  	 * line value is zero, the module doesn't support user configurable line
>  	 * sizes. Override the requested value with the minimum in that case.
>  	 */
> -	min_bpl = pix->width * info->bpp;
> +	min_bpl = pix->width * info->bpp[0];
>  	max_bpl = rounddown(XVIP_DMA_MAX_WIDTH, dma->align);
>  	bpl = rounddown(pix->bytesperline, dma->align);
>  
> @@ -676,7 +676,7 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma,
>  	dma->format.field = V4L2_FIELD_NONE;
>  	dma->format.width = XVIP_DMA_DEF_WIDTH;
>  	dma->format.height = XVIP_DMA_DEF_HEIGHT;
> -	dma->format.bytesperline = dma->format.width * dma->fmtinfo->bpp;
> +	dma->format.bytesperline = dma->format.width * dma->fmtinfo->bpp[0];
>  	dma->format.sizeimage = dma->format.bytesperline * dma->format.height;
>  
>  	/* Initialize the media entity... */
> diff --git a/drivers/media/platform/xilinx/xilinx-vip.c b/drivers/media/platform/xilinx/xilinx-vip.c
> index 3112591..81cc0d2 100644
> --- a/drivers/media/platform/xilinx/xilinx-vip.c
> +++ b/drivers/media/platform/xilinx/xilinx-vip.c
> @@ -27,22 +27,32 @@
>   */
>  
>  static const struct xvip_video_format xvip_video_formats[] = {
> +	{ XVIP_VF_YUV_420, 8, NULL, MEDIA_BUS_FMT_VYYUYY8_1X24,
> +	  {1, 2, 0}, V4L2_PIX_FMT_NV12, 2, 2, 2, "4:2:0, semi-planar, YUV" },
> +	{ XVIP_VF_YUV_420, 10, NULL, MEDIA_BUS_FMT_VYYUYY8_1X24,
> +	  {1, 2, 0}, V4L2_PIX_FMT_XV15, 2, 2, 2, "4:2:0, 10-bit 2-plane cont" },
>  	{ XVIP_VF_YUV_422, 8, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
> -	  2, V4L2_PIX_FMT_YUYV, "4:2:2, packed, YUYV" },
> -	{ XVIP_VF_YUV_444, 8, NULL, MEDIA_BUS_FMT_VUY8_1X24,
> -	  3, V4L2_PIX_FMT_YUV444, "4:4:4, packed, YUYV" },
> +	  {2, 0, 0}, V4L2_PIX_FMT_YUYV, 1, 2, 1, "4:2:2, packed, YUYV" },
> +	{ XVIP_VF_VUY_422, 8, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
> +	  {2, 0, 0}, V4L2_PIX_FMT_UYVY, 1, 2, 1, "4:2:2, packed, UYVY" },
> +	{ XVIP_VF_YUV_422, 8, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
> +	  {1, 2, 0}, V4L2_PIX_FMT_NV16, 2, 2, 1, "4:2:2, semi-planar, YUV" },
> +	{ XVIP_VF_YUV_422, 10, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
> +	  {1, 2, 0}, V4L2_PIX_FMT_XV20, 2, 2, 1, "4:2:2, 10-bit 2-plane cont" },
> +	{ XVIP_VF_RBG, 8, NULL, MEDIA_BUS_FMT_RBG888_1X24,
> +	  {3, 0, 0}, V4L2_PIX_FMT_BGR24, 1, 1, 1, "24-bit RGB" },
>  	{ XVIP_VF_RBG, 8, NULL, MEDIA_BUS_FMT_RBG888_1X24,
> -	  3, 0, NULL },
> +	  {3, 0, 0}, V4L2_PIX_FMT_RGB24, 1, 1, 1, "24-bit RGB" },
>  	{ XVIP_VF_MONO_SENSOR, 8, "mono", MEDIA_BUS_FMT_Y8_1X8,
> -	  1, V4L2_PIX_FMT_GREY, "Greyscale 8-bit" },
> +	  {1, 0, 0}, V4L2_PIX_FMT_GREY, 1, 1, 1, "Greyscale 8-bit" },
>  	{ XVIP_VF_MONO_SENSOR, 8, "rggb", MEDIA_BUS_FMT_SRGGB8_1X8,
> -	  1, V4L2_PIX_FMT_SGRBG8, "Bayer 8-bit RGGB" },
> +	  {1, 0, 0}, V4L2_PIX_FMT_SGRBG8, 1, 1, 1, "Bayer 8-bit RGGB" },
>  	{ XVIP_VF_MONO_SENSOR, 8, "grbg", MEDIA_BUS_FMT_SGRBG8_1X8,
> -	  1, V4L2_PIX_FMT_SGRBG8, "Bayer 8-bit GRBG" },
> +	  {1, 0, 0}, V4L2_PIX_FMT_SGRBG8, 1, 1, 1, "Bayer 8-bit GRBG" },
>  	{ XVIP_VF_MONO_SENSOR, 8, "gbrg", MEDIA_BUS_FMT_SGBRG8_1X8,
> -	  1, V4L2_PIX_FMT_SGBRG8, "Bayer 8-bit GBRG" },
> +	  {1, 0, 0}, V4L2_PIX_FMT_SGBRG8, 1, 1, 1, "Bayer 8-bit GBRG" },
>  	{ XVIP_VF_MONO_SENSOR, 8, "bggr", MEDIA_BUS_FMT_SBGGR8_1X8,
> -	  1, V4L2_PIX_FMT_SBGGR8, "Bayer 8-bit BGGR" },
> +	  {1, 0, 0}, V4L2_PIX_FMT_SBGGR8, 1, 1, 1, "Bayer 8-bit BGGR" },
>  };
>  
>  /**
> diff --git a/drivers/media/platform/xilinx/xilinx-vip.h b/drivers/media/platform/xilinx/xilinx-vip.h
> index 42fee20..5e7a978 100644
> --- a/drivers/media/platform/xilinx/xilinx-vip.h
> +++ b/drivers/media/platform/xilinx/xilinx-vip.h
> @@ -111,6 +111,9 @@ struct xvip_device {
>   * @code: media bus format code
>   * @bpp: bytes per pixel (when stored in memory)

Better to mention 'bpp' is per plane.

>   * @fourcc: V4L2 pixel format FCC identifier
> + * @num_planes: number of planes w.r.t. color format
> + * @hsub: Horizontal sampling factor of Chroma
> + * @vsub: Vertical sampling factor of Chroma
>   * @description: format description, suitable for userspace
>   */
>  struct xvip_video_format {
> @@ -118,8 +121,11 @@ struct xvip_video_format {
>  	unsigned int width;
>  	const char *pattern;
>  	unsigned int code;
> -	unsigned int bpp;
> +	unsigned int bpp[3];

Hm, only first one is used here, even when this becomes an array.

Thanks,
-hyun

>  	u32 fourcc;
> +	u8 num_planes;
> +	u8 hsub;
> +	u8 vsub;
>  	const char *description;
>  };
>  
> -- 
> 2.1.1
> 

[PATCH v4 09/10] v4l: xilinx: dma: Add multi-planar support

[Satish Kumar Nagireddy]

The current v4l driver supports single plane formats. This patch
adds support to handle multi-planar formats. Driver can handle
both single and multi-planar formats.

Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com>
---
 drivers/media/platform/xilinx/xilinx-dma.c  | 159 ++++++++++++++++++----------
 drivers/media/platform/xilinx/xilinx-dma.h  |   4 +-
 drivers/media/platform/xilinx/xilinx-vipp.c |  16 +--
 3 files changed, 111 insertions(+), 68 deletions(-)

diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
index 658586e..a714057 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -74,8 +74,8 @@ static int xvip_dma_verify_format(struct xvip_dma *dma)
 		return ret == -ENOIOCTLCMD ? -EINVAL : ret;
 
 	if (dma->fmtinfo->code != fmt.format.code ||
-	    dma->format.height != fmt.format.height ||
-	    dma->format.width != fmt.format.width)
+	    dma->format.fmt.pix_mp.width != fmt.format.width ||
+	    dma->format.fmt.pix_mp.height != fmt.format.height)
 		return -EINVAL;
 
 	return 0;
@@ -310,7 +310,8 @@ static void xvip_dma_complete(void *param)
 	buf->buf.field = V4L2_FIELD_NONE;
 	buf->buf.sequence = dma->sequence++;
 	buf->buf.vb2_buf.timestamp = ktime_get_ns();
-	vb2_set_plane_payload(&buf->buf.vb2_buf, 0, dma->format.sizeimage);
+	vb2_set_plane_payload(&buf->buf.vb2_buf, 0,
+			      dma->format.fmt.pix_mp.plane_fmt[0].sizeimage);
 	vb2_buffer_done(&buf->buf.vb2_buf, VB2_BUF_STATE_DONE);
 }
 
@@ -320,13 +321,15 @@ xvip_dma_queue_setup(struct vb2_queue *vq,
 		     unsigned int sizes[], struct device *alloc_devs[])
 {
 	struct xvip_dma *dma = vb2_get_drv_priv(vq);
+	s32 sizeimage;
 
 	/* Make sure the image size is large enough. */
+	sizeimage = dma->format.fmt.pix_mp.plane_fmt[0].sizeimage;
 	if (*nplanes)
-		return sizes[0] < dma->format.sizeimage ? -EINVAL : 0;
+		return sizes[0] < sizeimage ? -EINVAL : 0;
 
 	*nplanes = 1;
-	sizes[0] = dma->format.sizeimage;
+	sizes[0] = sizeimage;
 
 	return 0;
 }
@@ -350,14 +353,15 @@ static void xvip_dma_buffer_queue(struct vb2_buffer *vb)
 	struct dma_async_tx_descriptor *desc;
 	dma_addr_t addr = vb2_dma_contig_plane_dma_addr(vb, 0);
 	u32 flags;
+	struct v4l2_pix_format_mplane *pix_mp = &dma->format.fmt.pix_mp;
 
-	if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
+	if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
 		flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
 		dma->xt.dir = DMA_DEV_TO_MEM;
 		dma->xt.src_sgl = false;
 		dma->xt.dst_sgl = true;
 		dma->xt.dst_start = addr;
-	} else {
+	} else if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
 		flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
 		dma->xt.dir = DMA_MEM_TO_DEV;
 		dma->xt.src_sgl = true;
@@ -365,10 +369,11 @@ static void xvip_dma_buffer_queue(struct vb2_buffer *vb)
 		dma->xt.src_start = addr;
 	}
 
-	dma->xt.frame_size = 1;
-	dma->sgl[0].size = dma->format.width * dma->fmtinfo->bpp[0];
-	dma->sgl[0].icg = dma->format.bytesperline - dma->sgl[0].size;
-	dma->xt.numf = dma->format.height;
+	dma->xt.frame_size = dma->fmtinfo->num_planes;
+	dma->sgl[0].size = pix_mp->width * dma->fmtinfo->bpp[0];
+	dma->sgl[0].icg = pix_mp->plane_fmt[0].bytesperline - dma->sgl[0].size;
+	dma->xt.numf = pix_mp->height;
+	dma->sgl[0].dst_icg = 0;
 
 	desc = dmaengine_prep_interleaved_dma(dma->dma, &dma->xt, flags);
 	if (!desc) {
@@ -497,10 +502,15 @@ xvip_dma_querycap(struct file *file, void *fh, struct v4l2_capability *cap)
 	cap->capabilities = V4L2_CAP_DEVICE_CAPS | V4L2_CAP_STREAMING
 			  | dma->xdev->v4l2_caps;
 
-	if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
-		cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
-	else
-		cap->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
+	cap->device_caps = V4L2_CAP_STREAMING;
+	switch (dma->queue.type) {
+	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
+		cap->device_caps |= V4L2_CAP_VIDEO_CAPTURE_MPLANE;
+		break;
+	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
+		cap->device_caps |= V4L2_CAP_VIDEO_OUTPUT_MPLANE;
+		break;
+	}
 
 	strlcpy(cap->driver, "xilinx-vipp", sizeof(cap->driver));
 	strlcpy(cap->card, dma->video.name, sizeof(cap->card));
@@ -524,7 +534,7 @@ xvip_dma_enum_format(struct file *file, void *fh, struct v4l2_fmtdesc *f)
 	if (f->index > 0)
 		return -EINVAL;
 
-	f->pixelformat = dma->format.pixelformat;
+	f->pixelformat = dma->format.fmt.pix_mp.pixelformat;
 	strlcpy(f->description, dma->fmtinfo->description,
 		sizeof(f->description));
 
@@ -537,13 +547,14 @@ xvip_dma_get_format(struct file *file, void *fh, struct v4l2_format *format)
 	struct v4l2_fh *vfh = file->private_data;
 	struct xvip_dma *dma = to_xvip_dma(vfh->vdev);
 
-	format->fmt.pix = dma->format;
+	format->fmt.pix_mp = dma->format.fmt.pix_mp;
 
 	return 0;
 }
 
 static void
-__xvip_dma_try_format(struct xvip_dma *dma, struct v4l2_pix_format *pix,
+__xvip_dma_try_format(struct xvip_dma *dma,
+		      struct v4l2_format *format,
 		      const struct xvip_video_format **fmtinfo)
 {
 	const struct xvip_video_format *info;
@@ -551,19 +562,21 @@ __xvip_dma_try_format(struct xvip_dma *dma, struct v4l2_pix_format *pix,
 	unsigned int max_width;
 	unsigned int min_bpl;
 	unsigned int max_bpl;
-	unsigned int width;
+	unsigned int width, height;
 	unsigned int align;
 	unsigned int bpl;
+	unsigned int i;
+	struct v4l2_pix_format_mplane *pix_mp = &format->fmt.pix_mp;
+	struct v4l2_plane_pix_format *plane_fmt = pix_mp->plane_fmt;
 
 	/* Retrieve format information and select the default format if the
 	 * requested format isn't supported.
 	 */
-	info = xvip_get_format_by_fourcc(pix->pixelformat);
+	info = xvip_get_format_by_fourcc(pix_mp->pixelformat);
 	if (IS_ERR(info))
 		info = xvip_get_format_by_fourcc(XVIP_DMA_DEF_FORMAT);
 
-	pix->pixelformat = info->fourcc;
-	pix->field = V4L2_FIELD_NONE;
+	pix_mp->field = V4L2_FIELD_NONE;
 
 	/* The transfer alignment requirements are expressed in bytes. Compute
 	 * the minimum and maximum values, clamp the requested width and convert
@@ -572,22 +585,38 @@ __xvip_dma_try_format(struct xvip_dma *dma, struct v4l2_pix_format *pix,
 	align = lcm(dma->align, info->bpp[0]);
 	min_width = roundup(XVIP_DMA_MIN_WIDTH, align);
 	max_width = rounddown(XVIP_DMA_MAX_WIDTH, align);
-	width = rounddown(pix->width * info->bpp[0], align);
-
-	pix->width = clamp(width, min_width, max_width) / info->bpp[0];
-	pix->height = clamp(pix->height, XVIP_DMA_MIN_HEIGHT,
-			    XVIP_DMA_MAX_HEIGHT);
+	pix_mp->width = clamp(pix_mp->width, min_width, max_width);
+	pix_mp->height = clamp(pix_mp->height, XVIP_DMA_MIN_HEIGHT,
+			       XVIP_DMA_MAX_HEIGHT);
 
-	/* Clamp the requested bytes per line value. If the maximum bytes per
-	 * line value is zero, the module doesn't support user configurable line
-	 * sizes. Override the requested value with the minimum in that case.
+	/*
+	 * Clamp the requested bytes per line value. If the maximum
+	 * bytes per line value is zero, the module doesn't support
+	 * user configurable line sizes. Override the requested value
+	 * with the minimum in that case.
 	 */
-	min_bpl = pix->width * info->bpp[0];
-	max_bpl = rounddown(XVIP_DMA_MAX_WIDTH, dma->align);
-	bpl = rounddown(pix->bytesperline, dma->align);
-
-	pix->bytesperline = clamp(bpl, min_bpl, max_bpl);
-	pix->sizeimage = pix->bytesperline * pix->height;
+	max_bpl = rounddown(XVIP_DMA_MAX_WIDTH, align);
+	min_bpl = pix_mp->width * info->bpp[0];
+	min_bpl = roundup(min_bpl, align);
+	bpl = roundup(plane_fmt[0].bytesperline, align);
+	plane_fmt[0].bytesperline = clamp(bpl, min_bpl, max_bpl);
+
+	if (info->num_planes == 1) {
+		/* Single plane formats */
+		plane_fmt[0].sizeimage = plane_fmt[0].bytesperline *
+					 pix_mp->height;
+	} else {
+		/* Supports Multi plane formats in a contiguous buffer,
+		 * so we need only one buffer
+		 */
+		plane_fmt[0].sizeimage = 0;
+		for (i = 0; i < info->num_planes; i++) {
+			width = plane_fmt[0].bytesperline /
+				(i ? info->hsub : 1);
+			height = pix_mp->height / (i ? info->vsub : 1);
+			plane_fmt[0].sizeimage += width * info->bpp[i] * height;
+		}
+	}
 
 	if (fmtinfo)
 		*fmtinfo = info;
@@ -599,7 +628,7 @@ xvip_dma_try_format(struct file *file, void *fh, struct v4l2_format *format)
 	struct v4l2_fh *vfh = file->private_data;
 	struct xvip_dma *dma = to_xvip_dma(vfh->vdev);
 
-	__xvip_dma_try_format(dma, &format->fmt.pix, NULL);
+	__xvip_dma_try_format(dma, format, NULL);
 	return 0;
 }
 
@@ -610,12 +639,13 @@ xvip_dma_set_format(struct file *file, void *fh, struct v4l2_format *format)
 	struct xvip_dma *dma = to_xvip_dma(vfh->vdev);
 	const struct xvip_video_format *info;
 
-	__xvip_dma_try_format(dma, &format->fmt.pix, &info);
+	__xvip_dma_try_format(dma, format, &info);
 
 	if (vb2_is_busy(&dma->queue))
 		return -EBUSY;
 
-	dma->format = format->fmt.pix;
+	dma->format.fmt.pix_mp = format->fmt.pix_mp;
+
 	dma->fmtinfo = info;
 
 	return 0;
@@ -623,13 +653,14 @@ xvip_dma_set_format(struct file *file, void *fh, struct v4l2_format *format)
 
 static const struct v4l2_ioctl_ops xvip_dma_ioctl_ops = {
 	.vidioc_querycap		= xvip_dma_querycap,
-	.vidioc_enum_fmt_vid_cap	= xvip_dma_enum_format,
-	.vidioc_g_fmt_vid_cap		= xvip_dma_get_format,
-	.vidioc_g_fmt_vid_out		= xvip_dma_get_format,
-	.vidioc_s_fmt_vid_cap		= xvip_dma_set_format,
-	.vidioc_s_fmt_vid_out		= xvip_dma_set_format,
-	.vidioc_try_fmt_vid_cap		= xvip_dma_try_format,
-	.vidioc_try_fmt_vid_out		= xvip_dma_try_format,
+	.vidioc_enum_fmt_vid_cap_mplane	= xvip_dma_enum_format,
+	.vidioc_enum_fmt_vid_out_mplane	= xvip_dma_enum_format,
+	.vidioc_g_fmt_vid_cap_mplane	= xvip_dma_get_format,
+	.vidioc_g_fmt_vid_out_mplane	= xvip_dma_get_format,
+	.vidioc_s_fmt_vid_cap_mplane	= xvip_dma_set_format,
+	.vidioc_s_fmt_vid_out_mplane	= xvip_dma_set_format,
+	.vidioc_try_fmt_vid_cap_mplane	= xvip_dma_try_format,
+	.vidioc_try_fmt_vid_out_mplane	= xvip_dma_try_format,
 	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
 	.vidioc_querybuf		= vb2_ioctl_querybuf,
 	.vidioc_qbuf			= vb2_ioctl_qbuf,
@@ -662,6 +693,7 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma,
 {
 	char name[16];
 	int ret;
+	struct v4l2_pix_format_mplane *pix_mp;
 
 	dma->xdev = xdev;
 	dma->port = port;
@@ -671,17 +703,23 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma,
 	spin_lock_init(&dma->queued_lock);
 
 	dma->fmtinfo = xvip_get_format_by_fourcc(XVIP_DMA_DEF_FORMAT);
-	dma->format.pixelformat = dma->fmtinfo->fourcc;
-	dma->format.colorspace = V4L2_COLORSPACE_SRGB;
-	dma->format.field = V4L2_FIELD_NONE;
-	dma->format.width = XVIP_DMA_DEF_WIDTH;
-	dma->format.height = XVIP_DMA_DEF_HEIGHT;
-	dma->format.bytesperline = dma->format.width * dma->fmtinfo->bpp[0];
-	dma->format.sizeimage = dma->format.bytesperline * dma->format.height;
+	dma->format.type = type;
+
+	pix_mp = &dma->format.fmt.pix_mp;
+	pix_mp->pixelformat = dma->fmtinfo->fourcc;
+	pix_mp->colorspace = V4L2_COLORSPACE_SRGB;
+	pix_mp->field = V4L2_FIELD_NONE;
+	pix_mp->width = XVIP_DMA_DEF_WIDTH;
+	pix_mp->plane_fmt[0].bytesperline = pix_mp->width *
+					    dma->fmtinfo->bpp[0];
+	pix_mp->plane_fmt[0].sizeimage = pix_mp->plane_fmt[0].bytesperline *
+					 pix_mp->height;
 
 	/* Initialize the media entity... */
-	dma->pad.flags = type == V4L2_BUF_TYPE_VIDEO_CAPTURE
-		       ? MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
+	if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+		dma->pad.flags = MEDIA_PAD_FL_SINK;
+	else if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+		dma->pad.flags = MEDIA_PAD_FL_SOURCE;
 
 	ret = media_entity_pads_init(&dma->video.entity, 1, &dma->pad);
 	if (ret < 0)
@@ -693,11 +731,16 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma,
 	dma->video.queue = &dma->queue;
 	snprintf(dma->video.name, sizeof(dma->video.name), "%s %s %u",
 		 xdev->dev->of_node->name,
-		 type == V4L2_BUF_TYPE_VIDEO_CAPTURE ? "output" : "input",
+		 type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE
+					? "output" : "input",
 		 port);
+
 	dma->video.vfl_type = VFL_TYPE_GRABBER;
-	dma->video.vfl_dir = type == V4L2_BUF_TYPE_VIDEO_CAPTURE
-			   ? VFL_DIR_RX : VFL_DIR_TX;
+	if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+		dma->video.vfl_dir = VFL_DIR_RX;
+	else if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+		dma->video.vfl_dir = VFL_DIR_TX;
+
 	dma->video.release = video_device_release_empty;
 	dma->video.ioctl_ops = &xvip_dma_ioctl_ops;
 	dma->video.lock = &dma->lock;
diff --git a/drivers/media/platform/xilinx/xilinx-dma.h b/drivers/media/platform/xilinx/xilinx-dma.h
index e95d136..96933ed 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.h
+++ b/drivers/media/platform/xilinx/xilinx-dma.h
@@ -62,7 +62,7 @@ static inline struct xvip_pipeline *to_xvip_pipeline(struct media_entity *e)
  * @pipe: pipeline belonging to the DMA channel
  * @port: composite device DT node port number for the DMA channel
  * @lock: protects the @format, @fmtinfo and @queue fields
- * @format: active V4L2 pixel format
+ * @format: V4L2 format
  * @fmtinfo: format information corresponding to the active @format
  * @queue: vb2 buffers queue
  * @sequence: V4L2 buffers sequence number
@@ -83,7 +83,7 @@ struct xvip_dma {
 	unsigned int port;
 
 	struct mutex lock;
-	struct v4l2_pix_format format;
+	struct v4l2_format format;
 	const struct xvip_video_format *fmtinfo;
 
 	struct vb2_queue queue;
diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c b/drivers/media/platform/xilinx/xilinx-vipp.c
index 6bb28cd..509b50f 100644
--- a/drivers/media/platform/xilinx/xilinx-vipp.c
+++ b/drivers/media/platform/xilinx/xilinx-vipp.c
@@ -433,12 +433,15 @@ static int xvip_graph_dma_init_one(struct xvip_composite_device *xdev,
 	if (ret < 0)
 		return ret;
 
-	if (strcmp(direction, "input") == 0)
-		type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-	else if (strcmp(direction, "output") == 0)
-		type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
-	else
+	if (strcmp(direction, "input") == 0) {
+		type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
+		xdev->v4l2_caps |= V4L2_CAP_VIDEO_CAPTURE_MPLANE;
+	} else if (strcmp(direction, "output") == 0) {
+		type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
+		xdev->v4l2_caps |= V4L2_CAP_VIDEO_OUTPUT_MPLANE;
+	} else {
 		return -EINVAL;
+	}
 
 	of_property_read_u32(node, "reg", &index);
 
@@ -454,9 +457,6 @@ static int xvip_graph_dma_init_one(struct xvip_composite_device *xdev,
 
 	list_add_tail(&dma->list, &xdev->dmas);
 
-	xdev->v4l2_caps |= type == V4L2_BUF_TYPE_VIDEO_CAPTURE
-			 ? V4L2_CAP_VIDEO_CAPTURE : V4L2_CAP_VIDEO_OUTPUT;
-
 	return 0;
 }
 
-- 
2.1.1

Re: [PATCH v4 09/10] v4l: xilinx: dma: Add multi-planar support

[Ian Arkver]

Hi Satish,

On 01/05/18 02:35, Satish Kumar Nagireddy wrote:
> The current v4l driver supports single plane formats. This patch
> adds support to handle multi-planar formats. Driver can handle
> both single and multi-planar formats.
> 
> Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com>
> ---
>   drivers/media/platform/xilinx/xilinx-dma.c  | 159 ++++++++++++++++++----------
>   drivers/media/platform/xilinx/xilinx-dma.h  |   4 +-
>   drivers/media/platform/xilinx/xilinx-vipp.c |  16 +--
>   3 files changed, 111 insertions(+), 68 deletions(-)
> 
> diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
> index 658586e..a714057 100644
> --- a/drivers/media/platform/xilinx/xilinx-dma.c
> +++ b/drivers/media/platform/xilinx/xilinx-dma.c
> @@ -74,8 +74,8 @@ static int xvip_dma_verify_format(struct xvip_dma *dma)
>   		return ret == -ENOIOCTLCMD ? -EINVAL : ret;
>   
>   	if (dma->fmtinfo->code != fmt.format.code ||
> -	    dma->format.height != fmt.format.height ||
> -	    dma->format.width != fmt.format.width)
> +	    dma->format.fmt.pix_mp.width != fmt.format.width ||
> +	    dma->format.fmt.pix_mp.height != fmt.format.height)
>   		return -EINVAL;
>   
>   	return 0;
> @@ -310,7 +310,8 @@ static void xvip_dma_complete(void *param)
>   	buf->buf.field = V4L2_FIELD_NONE;
>   	buf->buf.sequence = dma->sequence++;
>   	buf->buf.vb2_buf.timestamp = ktime_get_ns();
> -	vb2_set_plane_payload(&buf->buf.vb2_buf, 0, dma->format.sizeimage);
> +	vb2_set_plane_payload(&buf->buf.vb2_buf, 0,
> +			      dma->format.fmt.pix_mp.plane_fmt[0].sizeimage);
>   	vb2_buffer_done(&buf->buf.vb2_buf, VB2_BUF_STATE_DONE);
>   }
>   
> @@ -320,13 +321,15 @@ xvip_dma_queue_setup(struct vb2_queue *vq,
>   		     unsigned int sizes[], struct device *alloc_devs[])
>   {
>   	struct xvip_dma *dma = vb2_get_drv_priv(vq);
> +	s32 sizeimage;
>   
>   	/* Make sure the image size is large enough. */
> +	sizeimage = dma->format.fmt.pix_mp.plane_fmt[0].sizeimage;
>   	if (*nplanes)
> -		return sizes[0] < dma->format.sizeimage ? -EINVAL : 0;
> +		return sizes[0] < sizeimage ? -EINVAL : 0;
>   
>   	*nplanes = 1;
> -	sizes[0] = dma->format.sizeimage;
> +	sizes[0] = sizeimage;
>   
>   	return 0;
>   }
> @@ -350,14 +353,15 @@ static void xvip_dma_buffer_queue(struct vb2_buffer *vb)
>   	struct dma_async_tx_descriptor *desc;
>   	dma_addr_t addr = vb2_dma_contig_plane_dma_addr(vb, 0);
>   	u32 flags;
> +	struct v4l2_pix_format_mplane *pix_mp = &dma->format.fmt.pix_mp;
>   
> -	if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> +	if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
>   		flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
>   		dma->xt.dir = DMA_DEV_TO_MEM;
>   		dma->xt.src_sgl = false;
>   		dma->xt.dst_sgl = true;
>   		dma->xt.dst_start = addr;
> -	} else {
> +	} else if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
>   		flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
>   		dma->xt.dir = DMA_MEM_TO_DEV;
>   		dma->xt.src_sgl = true;
> @@ -365,10 +369,11 @@ static void xvip_dma_buffer_queue(struct vb2_buffer *vb)
>   		dma->xt.src_start = addr;
>   	}
>   
> -	dma->xt.frame_size = 1;
> -	dma->sgl[0].size = dma->format.width * dma->fmtinfo->bpp[0];
> -	dma->sgl[0].icg = dma->format.bytesperline - dma->sgl[0].size;
> -	dma->xt.numf = dma->format.height;
> +	dma->xt.frame_size = dma->fmtinfo->num_planes;
> +	dma->sgl[0].size = pix_mp->width * dma->fmtinfo->bpp[0];
> +	dma->sgl[0].icg = pix_mp->plane_fmt[0].bytesperline - dma->sgl[0].size;
> +	dma->xt.numf = pix_mp->height;
> +	dma->sgl[0].dst_icg = 0;
>   
>   	desc = dmaengine_prep_interleaved_dma(dma->dma, &dma->xt, flags);
>   	if (!desc) {
> @@ -497,10 +502,15 @@ xvip_dma_querycap(struct file *file, void *fh, struct v4l2_capability *cap)
>   	cap->capabilities = V4L2_CAP_DEVICE_CAPS | V4L2_CAP_STREAMING
>   			  | dma->xdev->v4l2_caps;
>   
> -	if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> -		cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> -	else
> -		cap->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
> +	cap->device_caps = V4L2_CAP_STREAMING;
> +	switch (dma->queue.type) {
> +	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> +		cap->device_caps |= V4L2_CAP_VIDEO_CAPTURE_MPLANE;
> +		break;
> +	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> +		cap->device_caps |= V4L2_CAP_VIDEO_OUTPUT_MPLANE;
> +		break;
> +	}
>   
>   	strlcpy(cap->driver, "xilinx-vipp", sizeof(cap->driver));
>   	strlcpy(cap->card, dma->video.name, sizeof(cap->card));
> @@ -524,7 +534,7 @@ xvip_dma_enum_format(struct file *file, void *fh, struct v4l2_fmtdesc *f)
>   	if (f->index > 0)
>   		return -EINVAL;
>   
> -	f->pixelformat = dma->format.pixelformat;
> +	f->pixelformat = dma->format.fmt.pix_mp.pixelformat;
>   	strlcpy(f->description, dma->fmtinfo->description,
>   		sizeof(f->description));
>   
> @@ -537,13 +547,14 @@ xvip_dma_get_format(struct file *file, void *fh, struct v4l2_format *format)
>   	struct v4l2_fh *vfh = file->private_data;
>   	struct xvip_dma *dma = to_xvip_dma(vfh->vdev);
>   
> -	format->fmt.pix = dma->format;
> +	format->fmt.pix_mp = dma->format.fmt.pix_mp;
>   
>   	return 0;
>   }
>   
>   static void
> -__xvip_dma_try_format(struct xvip_dma *dma, struct v4l2_pix_format *pix,
> +__xvip_dma_try_format(struct xvip_dma *dma,
> +		      struct v4l2_format *format,
>   		      const struct xvip_video_format **fmtinfo)
>   {
>   	const struct xvip_video_format *info;
> @@ -551,19 +562,21 @@ __xvip_dma_try_format(struct xvip_dma *dma, struct v4l2_pix_format *pix,
>   	unsigned int max_width;
>   	unsigned int min_bpl;
>   	unsigned int max_bpl;
> -	unsigned int width;
> +	unsigned int width, height;
>   	unsigned int align;
>   	unsigned int bpl;
> +	unsigned int i;
> +	struct v4l2_pix_format_mplane *pix_mp = &format->fmt.pix_mp;
> +	struct v4l2_plane_pix_format *plane_fmt = pix_mp->plane_fmt;
>   
>   	/* Retrieve format information and select the default format if the
>   	 * requested format isn't supported.
>   	 */
> -	info = xvip_get_format_by_fourcc(pix->pixelformat);
> +	info = xvip_get_format_by_fourcc(pix_mp->pixelformat);
>   	if (IS_ERR(info))
>   		info = xvip_get_format_by_fourcc(XVIP_DMA_DEF_FORMAT);
>   
> -	pix->pixelformat = info->fourcc;
> -	pix->field = V4L2_FIELD_NONE;
> +	pix_mp->field = V4L2_FIELD_NONE;
>   
>   	/* The transfer alignment requirements are expressed in bytes. Compute
>   	 * the minimum and maximum values, clamp the requested width and convert
> @@ -572,22 +585,38 @@ __xvip_dma_try_format(struct xvip_dma *dma, struct v4l2_pix_format *pix,
>   	align = lcm(dma->align, info->bpp[0]);
>   	min_width = roundup(XVIP_DMA_MIN_WIDTH, align);
>   	max_width = rounddown(XVIP_DMA_MAX_WIDTH, align);
> -	width = rounddown(pix->width * info->bpp[0], align);
> -
> -	pix->width = clamp(width, min_width, max_width) / info->bpp[0];
> -	pix->height = clamp(pix->height, XVIP_DMA_MIN_HEIGHT,
> -			    XVIP_DMA_MAX_HEIGHT);
> +	pix_mp->width = clamp(pix_mp->width, min_width, max_width);
> +	pix_mp->height = clamp(pix_mp->height, XVIP_DMA_MIN_HEIGHT,
> +			       XVIP_DMA_MAX_HEIGHT);
>   
> -	/* Clamp the requested bytes per line value. If the maximum bytes per
> -	 * line value is zero, the module doesn't support user configurable line
> -	 * sizes. Override the requested value with the minimum in that case.
> +	/*
> +	 * Clamp the requested bytes per line value. If the maximum
> +	 * bytes per line value is zero, the module doesn't support
> +	 * user configurable line sizes. Override the requested value
> +	 * with the minimum in that case.
>   	 */
> -	min_bpl = pix->width * info->bpp[0];
> -	max_bpl = rounddown(XVIP_DMA_MAX_WIDTH, dma->align);
> -	bpl = rounddown(pix->bytesperline, dma->align);
> -
> -	pix->bytesperline = clamp(bpl, min_bpl, max_bpl);
> -	pix->sizeimage = pix->bytesperline * pix->height;
> +	max_bpl = rounddown(XVIP_DMA_MAX_WIDTH, align);
> +	min_bpl = pix_mp->width * info->bpp[0];
> +	min_bpl = roundup(min_bpl, align);
> +	bpl = roundup(plane_fmt[0].bytesperline, align);
> +	plane_fmt[0].bytesperline = clamp(bpl, min_bpl, max_bpl);
> +
> +	if (info->num_planes == 1) {
> +		/* Single plane formats */
> +		plane_fmt[0].sizeimage = plane_fmt[0].bytesperline *
> +					 pix_mp->height;
> +	} else {
> +		/* Supports Multi plane formats in a contiguous buffer,
> +		 * so we need only one buffer
> +		 */
> +		plane_fmt[0].sizeimage = 0;
> +		for (i = 0; i < info->num_planes; i++) {
> +			width = plane_fmt[0].bytesperline /
> +				(i ? info->hsub : 1);
> +			height = pix_mp->height / (i ? info->vsub : 1);
> +			plane_fmt[0].sizeimage += width * info->bpp[i] * height;
> +		}
> +	}
>   
>   	if (fmtinfo)
>   		*fmtinfo = info;
> @@ -599,7 +628,7 @@ xvip_dma_try_format(struct file *file, void *fh, struct v4l2_format *format)
>   	struct v4l2_fh *vfh = file->private_data;
>   	struct xvip_dma *dma = to_xvip_dma(vfh->vdev);
>   
> -	__xvip_dma_try_format(dma, &format->fmt.pix, NULL);
> +	__xvip_dma_try_format(dma, format, NULL);
>   	return 0;
>   }
>   
> @@ -610,12 +639,13 @@ xvip_dma_set_format(struct file *file, void *fh, struct v4l2_format *format)
>   	struct xvip_dma *dma = to_xvip_dma(vfh->vdev);
>   	const struct xvip_video_format *info;
>   
> -	__xvip_dma_try_format(dma, &format->fmt.pix, &info);
> +	__xvip_dma_try_format(dma, format, &info);
>   
>   	if (vb2_is_busy(&dma->queue))
>   		return -EBUSY;
>   
> -	dma->format = format->fmt.pix;
> +	dma->format.fmt.pix_mp = format->fmt.pix_mp;
> +
>   	dma->fmtinfo = info;
>   
>   	return 0;
> @@ -623,13 +653,14 @@ xvip_dma_set_format(struct file *file, void *fh, struct v4l2_format *format)
>   
>   static const struct v4l2_ioctl_ops xvip_dma_ioctl_ops = {
>   	.vidioc_querycap		= xvip_dma_querycap,
> -	.vidioc_enum_fmt_vid_cap	= xvip_dma_enum_format,
> -	.vidioc_g_fmt_vid_cap		= xvip_dma_get_format,
> -	.vidioc_g_fmt_vid_out		= xvip_dma_get_format,
> -	.vidioc_s_fmt_vid_cap		= xvip_dma_set_format,
> -	.vidioc_s_fmt_vid_out		= xvip_dma_set_format,
> -	.vidioc_try_fmt_vid_cap		= xvip_dma_try_format,
> -	.vidioc_try_fmt_vid_out		= xvip_dma_try_format,
> +	.vidioc_enum_fmt_vid_cap_mplane	= xvip_dma_enum_format,
> +	.vidioc_enum_fmt_vid_out_mplane	= xvip_dma_enum_format,
> +	.vidioc_g_fmt_vid_cap_mplane	= xvip_dma_get_format,
> +	.vidioc_g_fmt_vid_out_mplane	= xvip_dma_get_format,
> +	.vidioc_s_fmt_vid_cap_mplane	= xvip_dma_set_format,
> +	.vidioc_s_fmt_vid_out_mplane	= xvip_dma_set_format,
> +	.vidioc_try_fmt_vid_cap_mplane	= xvip_dma_try_format,
> +	.vidioc_try_fmt_vid_out_mplane	= xvip_dma_try_format,
>   	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
>   	.vidioc_querybuf		= vb2_ioctl_querybuf,
>   	.vidioc_qbuf			= vb2_ioctl_qbuf,
> @@ -662,6 +693,7 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma,
>   {
>   	char name[16];
>   	int ret;
> +	struct v4l2_pix_format_mplane *pix_mp;
>   
>   	dma->xdev = xdev;
>   	dma->port = port;
> @@ -671,17 +703,23 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma,
>   	spin_lock_init(&dma->queued_lock);
>   
>   	dma->fmtinfo = xvip_get_format_by_fourcc(XVIP_DMA_DEF_FORMAT);
> -	dma->format.pixelformat = dma->fmtinfo->fourcc;
> -	dma->format.colorspace = V4L2_COLORSPACE_SRGB;
> -	dma->format.field = V4L2_FIELD_NONE;
> -	dma->format.width = XVIP_DMA_DEF_WIDTH;
> -	dma->format.height = XVIP_DMA_DEF_HEIGHT;
> -	dma->format.bytesperline = dma->format.width * dma->fmtinfo->bpp[0];
> -	dma->format.sizeimage = dma->format.bytesperline * dma->format.height;
> +	dma->format.type = type;
> +
> +	pix_mp = &dma->format.fmt.pix_mp;
> +	pix_mp->pixelformat = dma->fmtinfo->fourcc;
> +	pix_mp->colorspace = V4L2_COLORSPACE_SRGB;
> +	pix_mp->field = V4L2_FIELD_NONE;
> +	pix_mp->width = XVIP_DMA_DEF_WIDTH;

Did you mean to remove setting the default height here?

Regards,
Ian.

> +	pix_mp->plane_fmt[0].bytesperline = pix_mp->width *
> +					    dma->fmtinfo->bpp[0];
> +	pix_mp->plane_fmt[0].sizeimage = pix_mp->plane_fmt[0].bytesperline *
> +					 pix_mp->height;
>   
>   	/* Initialize the media entity... */
> -	dma->pad.flags = type == V4L2_BUF_TYPE_VIDEO_CAPTURE
> -		       ? MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
> +	if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> +		dma->pad.flags = MEDIA_PAD_FL_SINK;
> +	else if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> +		dma->pad.flags = MEDIA_PAD_FL_SOURCE;
>   
>   	ret = media_entity_pads_init(&dma->video.entity, 1, &dma->pad);
>   	if (ret < 0)
> @@ -693,11 +731,16 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma,
>   	dma->video.queue = &dma->queue;
>   	snprintf(dma->video.name, sizeof(dma->video.name), "%s %s %u",
>   		 xdev->dev->of_node->name,
> -		 type == V4L2_BUF_TYPE_VIDEO_CAPTURE ? "output" : "input",
> +		 type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE
> +					? "output" : "input",
>   		 port);
> +
>   	dma->video.vfl_type = VFL_TYPE_GRABBER;
> -	dma->video.vfl_dir = type == V4L2_BUF_TYPE_VIDEO_CAPTURE
> -			   ? VFL_DIR_RX : VFL_DIR_TX;
> +	if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> +		dma->video.vfl_dir = VFL_DIR_RX;
> +	else if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> +		dma->video.vfl_dir = VFL_DIR_TX;
> +
>   	dma->video.release = video_device_release_empty;
>   	dma->video.ioctl_ops = &xvip_dma_ioctl_ops;
>   	dma->video.lock = &dma->lock;
> diff --git a/drivers/media/platform/xilinx/xilinx-dma.h b/drivers/media/platform/xilinx/xilinx-dma.h
> index e95d136..96933ed 100644
> --- a/drivers/media/platform/xilinx/xilinx-dma.h
> +++ b/drivers/media/platform/xilinx/xilinx-dma.h
> @@ -62,7 +62,7 @@ static inline struct xvip_pipeline *to_xvip_pipeline(struct media_entity *e)
>    * @pipe: pipeline belonging to the DMA channel
>    * @port: composite device DT node port number for the DMA channel
>    * @lock: protects the @format, @fmtinfo and @queue fields
> - * @format: active V4L2 pixel format
> + * @format: V4L2 format
>    * @fmtinfo: format information corresponding to the active @format
>    * @queue: vb2 buffers queue
>    * @sequence: V4L2 buffers sequence number
> @@ -83,7 +83,7 @@ struct xvip_dma {
>   	unsigned int port;
>   
>   	struct mutex lock;
> -	struct v4l2_pix_format format;
> +	struct v4l2_format format;
>   	const struct xvip_video_format *fmtinfo;
>   
>   	struct vb2_queue queue;
> diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c b/drivers/media/platform/xilinx/xilinx-vipp.c
> index 6bb28cd..509b50f 100644
> --- a/drivers/media/platform/xilinx/xilinx-vipp.c
> +++ b/drivers/media/platform/xilinx/xilinx-vipp.c
> @@ -433,12 +433,15 @@ static int xvip_graph_dma_init_one(struct xvip_composite_device *xdev,
>   	if (ret < 0)
>   		return ret;
>   
> -	if (strcmp(direction, "input") == 0)
> -		type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> -	else if (strcmp(direction, "output") == 0)
> -		type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
> -	else
> +	if (strcmp(direction, "input") == 0) {
> +		type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> +		xdev->v4l2_caps |= V4L2_CAP_VIDEO_CAPTURE_MPLANE;
> +	} else if (strcmp(direction, "output") == 0) {
> +		type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> +		xdev->v4l2_caps |= V4L2_CAP_VIDEO_OUTPUT_MPLANE;
> +	} else {
>   		return -EINVAL;
> +	}
>   
>   	of_property_read_u32(node, "reg", &index);
>   
> @@ -454,9 +457,6 @@ static int xvip_graph_dma_init_one(struct xvip_composite_device *xdev,
>   
>   	list_add_tail(&dma->list, &xdev->dmas);
>   
> -	xdev->v4l2_caps |= type == V4L2_BUF_TYPE_VIDEO_CAPTURE
> -			 ? V4L2_CAP_VIDEO_CAPTURE : V4L2_CAP_VIDEO_OUTPUT;
> -
>   	return 0;
>   }
>   
> 

RE: [PATCH v4 09/10] v4l: xilinx: dma: Add multi-planar support

[Satish Kumar Nagireddy]

Hi Ian,

Thanks for the review.

> -----Original Message-----
> From: Ian Arkver [mailto:ian.arkver.dev@gmail.com]
> Sent: Monday, April 30, 2018 11:44 PM
> To: Satish Kumar Nagireddy <SATISHNA@xilinx.com>; linux-
> media@vger.kernel.org; laurent.pinchart@ideasonboard.com;
> michal.simek@xilinx.com; Hyun Kwon <hyunk@xilinx.com>
> Subject: Re: [PATCH v4 09/10] v4l: xilinx: dma: Add multi-planar support
> 
> Hi Satish,
> 
> On 01/05/18 02:35, Satish Kumar Nagireddy wrote:
> > The current v4l driver supports single plane formats. This patch adds
> > support to handle multi-planar formats. Driver can handle both single
> > and multi-planar formats.
> >
> > Signed-off-by: Satish Kumar Nagireddy
> > <satish.nagireddy.nagireddy@xilinx.com>
> > ---
> >   drivers/media/platform/xilinx/xilinx-dma.c  | 159 ++++++++++++++++++---
> -------
> >   drivers/media/platform/xilinx/xilinx-dma.h  |   4 +-
> >   drivers/media/platform/xilinx/xilinx-vipp.c |  16 +--
> >   3 files changed, 111 insertions(+), 68 deletions(-)
> >
> > diff --git a/drivers/media/platform/xilinx/xilinx-dma.c
> > b/drivers/media/platform/xilinx/xilinx-dma.c
> > index 658586e..a714057 100644
> > --- a/drivers/media/platform/xilinx/xilinx-dma.c
> > +++ b/drivers/media/platform/xilinx/xilinx-dma.c
> > @@ -74,8 +74,8 @@ static int xvip_dma_verify_format(struct xvip_dma
> *dma)
> >   		return ret == -ENOIOCTLCMD ? -EINVAL : ret;
> >
> >   	if (dma->fmtinfo->code != fmt.format.code ||
> > -	    dma->format.height != fmt.format.height ||
> > -	    dma->format.width != fmt.format.width)
> > +	    dma->format.fmt.pix_mp.width != fmt.format.width ||
> > +	    dma->format.fmt.pix_mp.height != fmt.format.height)
> >   		return -EINVAL;
> >
> >   	return 0;
> > @@ -310,7 +310,8 @@ static void xvip_dma_complete(void *param)
> >   	buf->buf.field = V4L2_FIELD_NONE;
> >   	buf->buf.sequence = dma->sequence++;
> >   	buf->buf.vb2_buf.timestamp = ktime_get_ns();
> > -	vb2_set_plane_payload(&buf->buf.vb2_buf, 0, dma-
> >format.sizeimage);
> > +	vb2_set_plane_payload(&buf->buf.vb2_buf, 0,
> > +			      dma-
> >format.fmt.pix_mp.plane_fmt[0].sizeimage);
> >   	vb2_buffer_done(&buf->buf.vb2_buf, VB2_BUF_STATE_DONE);
> >   }
> >
> > @@ -320,13 +321,15 @@ xvip_dma_queue_setup(struct vb2_queue *vq,
> >   		     unsigned int sizes[], struct device *alloc_devs[])
> >   {
> >   	struct xvip_dma *dma = vb2_get_drv_priv(vq);
> > +	s32 sizeimage;
> >
> >   	/* Make sure the image size is large enough. */
> > +	sizeimage = dma->format.fmt.pix_mp.plane_fmt[0].sizeimage;
> >   	if (*nplanes)
> > -		return sizes[0] < dma->format.sizeimage ? -EINVAL : 0;
> > +		return sizes[0] < sizeimage ? -EINVAL : 0;
> >
> >   	*nplanes = 1;
> > -	sizes[0] = dma->format.sizeimage;
> > +	sizes[0] = sizeimage;
> >
> >   	return 0;
> >   }
> > @@ -350,14 +353,15 @@ static void xvip_dma_buffer_queue(struct
> vb2_buffer *vb)
> >   	struct dma_async_tx_descriptor *desc;
> >   	dma_addr_t addr = vb2_dma_contig_plane_dma_addr(vb, 0);
> >   	u32 flags;
> > +	struct v4l2_pix_format_mplane *pix_mp = &dma-
> >format.fmt.pix_mp;
> >
> > -	if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> > +	if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> {
> >   		flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
> >   		dma->xt.dir = DMA_DEV_TO_MEM;
> >   		dma->xt.src_sgl = false;
> >   		dma->xt.dst_sgl = true;
> >   		dma->xt.dst_start = addr;
> > -	} else {
> > +	} else if (dma->queue.type ==
> V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
> >   		flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
> >   		dma->xt.dir = DMA_MEM_TO_DEV;
> >   		dma->xt.src_sgl = true;
> > @@ -365,10 +369,11 @@ static void xvip_dma_buffer_queue(struct
> vb2_buffer *vb)
> >   		dma->xt.src_start = addr;
> >   	}
> >
> > -	dma->xt.frame_size = 1;
> > -	dma->sgl[0].size = dma->format.width * dma->fmtinfo->bpp[0];
> > -	dma->sgl[0].icg = dma->format.bytesperline - dma->sgl[0].size;
> > -	dma->xt.numf = dma->format.height;
> > +	dma->xt.frame_size = dma->fmtinfo->num_planes;
> > +	dma->sgl[0].size = pix_mp->width * dma->fmtinfo->bpp[0];
> > +	dma->sgl[0].icg = pix_mp->plane_fmt[0].bytesperline - dma-
> >sgl[0].size;
> > +	dma->xt.numf = pix_mp->height;
> > +	dma->sgl[0].dst_icg = 0;
> >
> >   	desc = dmaengine_prep_interleaved_dma(dma->dma, &dma->xt,
> flags);
> >   	if (!desc) {
> > @@ -497,10 +502,15 @@ xvip_dma_querycap(struct file *file, void *fh,
> struct v4l2_capability *cap)
> >   	cap->capabilities = V4L2_CAP_DEVICE_CAPS | V4L2_CAP_STREAMING
> >   			  | dma->xdev->v4l2_caps;
> >
> > -	if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> > -		cap->device_caps = V4L2_CAP_VIDEO_CAPTURE |
> V4L2_CAP_STREAMING;
> > -	else
> > -		cap->device_caps = V4L2_CAP_VIDEO_OUTPUT |
> V4L2_CAP_STREAMING;
> > +	cap->device_caps = V4L2_CAP_STREAMING;
> > +	switch (dma->queue.type) {
> > +	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> > +		cap->device_caps |= V4L2_CAP_VIDEO_CAPTURE_MPLANE;
> > +		break;
> > +	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> > +		cap->device_caps |= V4L2_CAP_VIDEO_OUTPUT_MPLANE;
> > +		break;
> > +	}
> >
> >   	strlcpy(cap->driver, "xilinx-vipp", sizeof(cap->driver));
> >   	strlcpy(cap->card, dma->video.name, sizeof(cap->card)); @@ -524,7
> > +534,7 @@ xvip_dma_enum_format(struct file *file, void *fh, struct
> v4l2_fmtdesc *f)
> >   	if (f->index > 0)
> >   		return -EINVAL;
> >
> > -	f->pixelformat = dma->format.pixelformat;
> > +	f->pixelformat = dma->format.fmt.pix_mp.pixelformat;
> >   	strlcpy(f->description, dma->fmtinfo->description,
> >   		sizeof(f->description));
> >
> > @@ -537,13 +547,14 @@ xvip_dma_get_format(struct file *file, void *fh,
> struct v4l2_format *format)
> >   	struct v4l2_fh *vfh = file->private_data;
> >   	struct xvip_dma *dma = to_xvip_dma(vfh->vdev);
> >
> > -	format->fmt.pix = dma->format;
> > +	format->fmt.pix_mp = dma->format.fmt.pix_mp;
> >
> >   	return 0;
> >   }
> >
> >   static void
> > -__xvip_dma_try_format(struct xvip_dma *dma, struct v4l2_pix_format
> > *pix,
> > +__xvip_dma_try_format(struct xvip_dma *dma,
> > +		      struct v4l2_format *format,
> >   		      const struct xvip_video_format **fmtinfo)
> >   {
> >   	const struct xvip_video_format *info; @@ -551,19 +562,21 @@
> > __xvip_dma_try_format(struct xvip_dma *dma, struct v4l2_pix_format
> *pix,
> >   	unsigned int max_width;
> >   	unsigned int min_bpl;
> >   	unsigned int max_bpl;
> > -	unsigned int width;
> > +	unsigned int width, height;
> >   	unsigned int align;
> >   	unsigned int bpl;
> > +	unsigned int i;
> > +	struct v4l2_pix_format_mplane *pix_mp = &format->fmt.pix_mp;
> > +	struct v4l2_plane_pix_format *plane_fmt = pix_mp->plane_fmt;
> >
> >   	/* Retrieve format information and select the default format if the
> >   	 * requested format isn't supported.
> >   	 */
> > -	info = xvip_get_format_by_fourcc(pix->pixelformat);
> > +	info = xvip_get_format_by_fourcc(pix_mp->pixelformat);
> >   	if (IS_ERR(info))
> >   		info =
> xvip_get_format_by_fourcc(XVIP_DMA_DEF_FORMAT);
> >
> > -	pix->pixelformat = info->fourcc;
> > -	pix->field = V4L2_FIELD_NONE;
> > +	pix_mp->field = V4L2_FIELD_NONE;
> >
> >   	/* The transfer alignment requirements are expressed in bytes.
> Compute
> >   	 * the minimum and maximum values, clamp the requested width
> and
> > convert @@ -572,22 +585,38 @@ __xvip_dma_try_format(struct xvip_dma
> *dma, struct v4l2_pix_format *pix,
> >   	align = lcm(dma->align, info->bpp[0]);
> >   	min_width = roundup(XVIP_DMA_MIN_WIDTH, align);
> >   	max_width = rounddown(XVIP_DMA_MAX_WIDTH, align);
> > -	width = rounddown(pix->width * info->bpp[0], align);
> > -
> > -	pix->width = clamp(width, min_width, max_width) / info->bpp[0];
> > -	pix->height = clamp(pix->height, XVIP_DMA_MIN_HEIGHT,
> > -			    XVIP_DMA_MAX_HEIGHT);
> > +	pix_mp->width = clamp(pix_mp->width, min_width, max_width);
> > +	pix_mp->height = clamp(pix_mp->height, XVIP_DMA_MIN_HEIGHT,
> > +			       XVIP_DMA_MAX_HEIGHT);
> >
> > -	/* Clamp the requested bytes per line value. If the maximum bytes
> per
> > -	 * line value is zero, the module doesn't support user configurable
> line
> > -	 * sizes. Override the requested value with the minimum in that case.
> > +	/*
> > +	 * Clamp the requested bytes per line value. If the maximum
> > +	 * bytes per line value is zero, the module doesn't support
> > +	 * user configurable line sizes. Override the requested value
> > +	 * with the minimum in that case.
> >   	 */
> > -	min_bpl = pix->width * info->bpp[0];
> > -	max_bpl = rounddown(XVIP_DMA_MAX_WIDTH, dma->align);
> > -	bpl = rounddown(pix->bytesperline, dma->align);
> > -
> > -	pix->bytesperline = clamp(bpl, min_bpl, max_bpl);
> > -	pix->sizeimage = pix->bytesperline * pix->height;
> > +	max_bpl = rounddown(XVIP_DMA_MAX_WIDTH, align);
> > +	min_bpl = pix_mp->width * info->bpp[0];
> > +	min_bpl = roundup(min_bpl, align);
> > +	bpl = roundup(plane_fmt[0].bytesperline, align);
> > +	plane_fmt[0].bytesperline = clamp(bpl, min_bpl, max_bpl);
> > +
> > +	if (info->num_planes == 1) {
> > +		/* Single plane formats */
> > +		plane_fmt[0].sizeimage = plane_fmt[0].bytesperline *
> > +					 pix_mp->height;
> > +	} else {
> > +		/* Supports Multi plane formats in a contiguous buffer,
> > +		 * so we need only one buffer
> > +		 */
> > +		plane_fmt[0].sizeimage = 0;
> > +		for (i = 0; i < info->num_planes; i++) {
> > +			width = plane_fmt[0].bytesperline /
> > +				(i ? info->hsub : 1);
> > +			height = pix_mp->height / (i ? info->vsub : 1);
> > +			plane_fmt[0].sizeimage += width * info->bpp[i] *
> height;
> > +		}
> > +	}
> >
> >   	if (fmtinfo)
> >   		*fmtinfo = info;
> > @@ -599,7 +628,7 @@ xvip_dma_try_format(struct file *file, void *fh,
> struct v4l2_format *format)
> >   	struct v4l2_fh *vfh = file->private_data;
> >   	struct xvip_dma *dma = to_xvip_dma(vfh->vdev);
> >
> > -	__xvip_dma_try_format(dma, &format->fmt.pix, NULL);
> > +	__xvip_dma_try_format(dma, format, NULL);
> >   	return 0;
> >   }
> >
> > @@ -610,12 +639,13 @@ xvip_dma_set_format(struct file *file, void *fh,
> struct v4l2_format *format)
> >   	struct xvip_dma *dma = to_xvip_dma(vfh->vdev);
> >   	const struct xvip_video_format *info;
> >
> > -	__xvip_dma_try_format(dma, &format->fmt.pix, &info);
> > +	__xvip_dma_try_format(dma, format, &info);
> >
> >   	if (vb2_is_busy(&dma->queue))
> >   		return -EBUSY;
> >
> > -	dma->format = format->fmt.pix;
> > +	dma->format.fmt.pix_mp = format->fmt.pix_mp;
> > +
> >   	dma->fmtinfo = info;
> >
> >   	return 0;
> > @@ -623,13 +653,14 @@ xvip_dma_set_format(struct file *file, void *fh,
> > struct v4l2_format *format)
> >
> >   static const struct v4l2_ioctl_ops xvip_dma_ioctl_ops = {
> >   	.vidioc_querycap		= xvip_dma_querycap,
> > -	.vidioc_enum_fmt_vid_cap	= xvip_dma_enum_format,
> > -	.vidioc_g_fmt_vid_cap		= xvip_dma_get_format,
> > -	.vidioc_g_fmt_vid_out		= xvip_dma_get_format,
> > -	.vidioc_s_fmt_vid_cap		= xvip_dma_set_format,
> > -	.vidioc_s_fmt_vid_out		= xvip_dma_set_format,
> > -	.vidioc_try_fmt_vid_cap		= xvip_dma_try_format,
> > -	.vidioc_try_fmt_vid_out		= xvip_dma_try_format,
> > +	.vidioc_enum_fmt_vid_cap_mplane	= xvip_dma_enum_format,
> > +	.vidioc_enum_fmt_vid_out_mplane	= xvip_dma_enum_format,
> > +	.vidioc_g_fmt_vid_cap_mplane	= xvip_dma_get_format,
> > +	.vidioc_g_fmt_vid_out_mplane	= xvip_dma_get_format,
> > +	.vidioc_s_fmt_vid_cap_mplane	= xvip_dma_set_format,
> > +	.vidioc_s_fmt_vid_out_mplane	= xvip_dma_set_format,
> > +	.vidioc_try_fmt_vid_cap_mplane	= xvip_dma_try_format,
> > +	.vidioc_try_fmt_vid_out_mplane	= xvip_dma_try_format,
> >   	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
> >   	.vidioc_querybuf		= vb2_ioctl_querybuf,
> >   	.vidioc_qbuf			= vb2_ioctl_qbuf,
> > @@ -662,6 +693,7 @@ int xvip_dma_init(struct xvip_composite_device
> *xdev, struct xvip_dma *dma,
> >   {
> >   	char name[16];
> >   	int ret;
> > +	struct v4l2_pix_format_mplane *pix_mp;
> >
> >   	dma->xdev = xdev;
> >   	dma->port = port;
> > @@ -671,17 +703,23 @@ int xvip_dma_init(struct xvip_composite_device
> *xdev, struct xvip_dma *dma,
> >   	spin_lock_init(&dma->queued_lock);
> >
> >   	dma->fmtinfo =
> xvip_get_format_by_fourcc(XVIP_DMA_DEF_FORMAT);
> > -	dma->format.pixelformat = dma->fmtinfo->fourcc;
> > -	dma->format.colorspace = V4L2_COLORSPACE_SRGB;
> > -	dma->format.field = V4L2_FIELD_NONE;
> > -	dma->format.width = XVIP_DMA_DEF_WIDTH;
> > -	dma->format.height = XVIP_DMA_DEF_HEIGHT;
> > -	dma->format.bytesperline = dma->format.width * dma->fmtinfo-
> >bpp[0];
> > -	dma->format.sizeimage = dma->format.bytesperline * dma-
> >format.height;
> > +	dma->format.type = type;
> > +
> > +	pix_mp = &dma->format.fmt.pix_mp;
> > +	pix_mp->pixelformat = dma->fmtinfo->fourcc;
> > +	pix_mp->colorspace = V4L2_COLORSPACE_SRGB;
> > +	pix_mp->field = V4L2_FIELD_NONE;
> > +	pix_mp->width = XVIP_DMA_DEF_WIDTH;
> 
> Did you mean to remove setting the default height here?

     [Satish]: Thanks you for catching this, I have missed setting default height. I will add in v5 patch-set.

> 
> Regards,
> Ian.
> 
> > +	pix_mp->plane_fmt[0].bytesperline = pix_mp->width *
> > +					    dma->fmtinfo->bpp[0];
> > +	pix_mp->plane_fmt[0].sizeimage = pix_mp-
> >plane_fmt[0].bytesperline *
> > +					 pix_mp->height;
> >
> >   	/* Initialize the media entity... */
> > -	dma->pad.flags = type == V4L2_BUF_TYPE_VIDEO_CAPTURE
> > -		       ? MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
> > +	if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> > +		dma->pad.flags = MEDIA_PAD_FL_SINK;
> > +	else if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > +		dma->pad.flags = MEDIA_PAD_FL_SOURCE;
> >
> >   	ret = media_entity_pads_init(&dma->video.entity, 1, &dma->pad);
> >   	if (ret < 0)
> > @@ -693,11 +731,16 @@ int xvip_dma_init(struct xvip_composite_device
> *xdev, struct xvip_dma *dma,
> >   	dma->video.queue = &dma->queue;
> >   	snprintf(dma->video.name, sizeof(dma->video.name), "%s %s %u",
> >   		 xdev->dev->of_node->name,
> > -		 type == V4L2_BUF_TYPE_VIDEO_CAPTURE ? "output" :
> "input",
> > +		 type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE
> > +					? "output" : "input",
> >   		 port);
> > +
> >   	dma->video.vfl_type = VFL_TYPE_GRABBER;
> > -	dma->video.vfl_dir = type == V4L2_BUF_TYPE_VIDEO_CAPTURE
> > -			   ? VFL_DIR_RX : VFL_DIR_TX;
> > +	if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> > +		dma->video.vfl_dir = VFL_DIR_RX;
> > +	else if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > +		dma->video.vfl_dir = VFL_DIR_TX;
> > +
> >   	dma->video.release = video_device_release_empty;
> >   	dma->video.ioctl_ops = &xvip_dma_ioctl_ops;
> >   	dma->video.lock = &dma->lock;
> > diff --git a/drivers/media/platform/xilinx/xilinx-dma.h
> > b/drivers/media/platform/xilinx/xilinx-dma.h
> > index e95d136..96933ed 100644
> > --- a/drivers/media/platform/xilinx/xilinx-dma.h
> > +++ b/drivers/media/platform/xilinx/xilinx-dma.h
> > @@ -62,7 +62,7 @@ static inline struct xvip_pipeline
> *to_xvip_pipeline(struct media_entity *e)
> >    * @pipe: pipeline belonging to the DMA channel
> >    * @port: composite device DT node port number for the DMA channel
> >    * @lock: protects the @format, @fmtinfo and @queue fields
> > - * @format: active V4L2 pixel format
> > + * @format: V4L2 format
> >    * @fmtinfo: format information corresponding to the active @format
> >    * @queue: vb2 buffers queue
> >    * @sequence: V4L2 buffers sequence number @@ -83,7 +83,7 @@ struct
> > xvip_dma {
> >   	unsigned int port;
> >
> >   	struct mutex lock;
> > -	struct v4l2_pix_format format;
> > +	struct v4l2_format format;
> >   	const struct xvip_video_format *fmtinfo;
> >
> >   	struct vb2_queue queue;
> > diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c
> > b/drivers/media/platform/xilinx/xilinx-vipp.c
> > index 6bb28cd..509b50f 100644
> > --- a/drivers/media/platform/xilinx/xilinx-vipp.c
> > +++ b/drivers/media/platform/xilinx/xilinx-vipp.c
> > @@ -433,12 +433,15 @@ static int xvip_graph_dma_init_one(struct
> xvip_composite_device *xdev,
> >   	if (ret < 0)
> >   		return ret;
> >
> > -	if (strcmp(direction, "input") == 0)
> > -		type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> > -	else if (strcmp(direction, "output") == 0)
> > -		type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
> > -	else
> > +	if (strcmp(direction, "input") == 0) {
> > +		type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> > +		xdev->v4l2_caps |= V4L2_CAP_VIDEO_CAPTURE_MPLANE;
> > +	} else if (strcmp(direction, "output") == 0) {
> > +		type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> > +		xdev->v4l2_caps |= V4L2_CAP_VIDEO_OUTPUT_MPLANE;
> > +	} else {
> >   		return -EINVAL;
> > +	}
> >
> >   	of_property_read_u32(node, "reg", &index);
> >
> > @@ -454,9 +457,6 @@ static int xvip_graph_dma_init_one(struct
> > xvip_composite_device *xdev,
> >
> >   	list_add_tail(&dma->list, &xdev->dmas);
> >
> > -	xdev->v4l2_caps |= type == V4L2_BUF_TYPE_VIDEO_CAPTURE
> > -			 ? V4L2_CAP_VIDEO_CAPTURE :
> V4L2_CAP_VIDEO_OUTPUT;
> > -
> >   	return 0;
> >   }
> >
> >

Re: [PATCH v4 09/10] v4l: xilinx: dma: Add multi-planar support

[Hyun Kwon]

On Mon, 2018-04-30 at 18:35:12 -0700, Satish Kumar Nagireddy wrote:
> The current v4l driver supports single plane formats. This patch
> adds support to handle multi-planar formats. Driver can handle
> both single and multi-planar formats.
> 
> Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com>
> ---
>  drivers/media/platform/xilinx/xilinx-dma.c  | 159 ++++++++++++++++++----------
>  drivers/media/platform/xilinx/xilinx-dma.h  |   4 +-
>  drivers/media/platform/xilinx/xilinx-vipp.c |  16 +--
>  3 files changed, 111 insertions(+), 68 deletions(-)
> 
> diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
> index 658586e..a714057 100644
> --- a/drivers/media/platform/xilinx/xilinx-dma.c
> +++ b/drivers/media/platform/xilinx/xilinx-dma.c
> @@ -74,8 +74,8 @@ static int xvip_dma_verify_format(struct xvip_dma *dma)
>  		return ret == -ENOIOCTLCMD ? -EINVAL : ret;
>  
>  	if (dma->fmtinfo->code != fmt.format.code ||
> -	    dma->format.height != fmt.format.height ||
> -	    dma->format.width != fmt.format.width)
> +	    dma->format.fmt.pix_mp.width != fmt.format.width ||
> +	    dma->format.fmt.pix_mp.height != fmt.format.height)
>  		return -EINVAL;
>  
>  	return 0;
> @@ -310,7 +310,8 @@ static void xvip_dma_complete(void *param)
>  	buf->buf.field = V4L2_FIELD_NONE;
>  	buf->buf.sequence = dma->sequence++;
>  	buf->buf.vb2_buf.timestamp = ktime_get_ns();
> -	vb2_set_plane_payload(&buf->buf.vb2_buf, 0, dma->format.sizeimage);
> +	vb2_set_plane_payload(&buf->buf.vb2_buf, 0,
> +			      dma->format.fmt.pix_mp.plane_fmt[0].sizeimage);
>  	vb2_buffer_done(&buf->buf.vb2_buf, VB2_BUF_STATE_DONE);
>  }
>  
> @@ -320,13 +321,15 @@ xvip_dma_queue_setup(struct vb2_queue *vq,
>  		     unsigned int sizes[], struct device *alloc_devs[])
>  {
>  	struct xvip_dma *dma = vb2_get_drv_priv(vq);
> +	s32 sizeimage;

u32?

>  
>  	/* Make sure the image size is large enough. */
> +	sizeimage = dma->format.fmt.pix_mp.plane_fmt[0].sizeimage;

I'm a little confused again. :-) This doesn't seem handling nplanes > 1,
while there are such formats in the format table. is my understanding
correct?

>  	if (*nplanes)
> -		return sizes[0] < dma->format.sizeimage ? -EINVAL : 0;
> +		return sizes[0] < sizeimage ? -EINVAL : 0;
>  
>  	*nplanes = 1;
> -	sizes[0] = dma->format.sizeimage;
> +	sizes[0] = sizeimage;
>  
>  	return 0;
>  }
> @@ -350,14 +353,15 @@ static void xvip_dma_buffer_queue(struct vb2_buffer *vb)
>  	struct dma_async_tx_descriptor *desc;
>  	dma_addr_t addr = vb2_dma_contig_plane_dma_addr(vb, 0);
>  	u32 flags;
> +	struct v4l2_pix_format_mplane *pix_mp = &dma->format.fmt.pix_mp;
>  
> -	if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> +	if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
>  		flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
>  		dma->xt.dir = DMA_DEV_TO_MEM;
>  		dma->xt.src_sgl = false;
>  		dma->xt.dst_sgl = true;
>  		dma->xt.dst_start = addr;
> -	} else {
> +	} else if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {

I'd do 'else' even if any other value is not possible at this point.

>  		flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
>  		dma->xt.dir = DMA_MEM_TO_DEV;
>  		dma->xt.src_sgl = true;
> @@ -365,10 +369,11 @@ static void xvip_dma_buffer_queue(struct vb2_buffer *vb)
>  		dma->xt.src_start = addr;
>  	}
>  
> -	dma->xt.frame_size = 1;
> -	dma->sgl[0].size = dma->format.width * dma->fmtinfo->bpp[0];
> -	dma->sgl[0].icg = dma->format.bytesperline - dma->sgl[0].size;
> -	dma->xt.numf = dma->format.height;
> +	dma->xt.frame_size = dma->fmtinfo->num_planes;
> +	dma->sgl[0].size = pix_mp->width * dma->fmtinfo->bpp[0];
> +	dma->sgl[0].icg = pix_mp->plane_fmt[0].bytesperline - dma->sgl[0].size;
> +	dma->xt.numf = pix_mp->height;
> +	dma->sgl[0].dst_icg = 0;
>  
>  	desc = dmaengine_prep_interleaved_dma(dma->dma, &dma->xt, flags);
>  	if (!desc) {
> @@ -497,10 +502,15 @@ xvip_dma_querycap(struct file *file, void *fh, struct v4l2_capability *cap)
>  	cap->capabilities = V4L2_CAP_DEVICE_CAPS | V4L2_CAP_STREAMING
>  			  | dma->xdev->v4l2_caps;
>  
> -	if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> -		cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> -	else
> -		cap->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
> +	cap->device_caps = V4L2_CAP_STREAMING;
> +	switch (dma->queue.type) {
> +	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> +		cap->device_caps |= V4L2_CAP_VIDEO_CAPTURE_MPLANE;
> +		break;
> +	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> +		cap->device_caps |= V4L2_CAP_VIDEO_OUTPUT_MPLANE;
> +		break;
> +	}

I'd keep if - else rather than switch - case.

>  
>  	strlcpy(cap->driver, "xilinx-vipp", sizeof(cap->driver));
>  	strlcpy(cap->card, dma->video.name, sizeof(cap->card));
> @@ -524,7 +534,7 @@ xvip_dma_enum_format(struct file *file, void *fh, struct v4l2_fmtdesc *f)
>  	if (f->index > 0)
>  		return -EINVAL;
>  
> -	f->pixelformat = dma->format.pixelformat;
> +	f->pixelformat = dma->format.fmt.pix_mp.pixelformat;
>  	strlcpy(f->description, dma->fmtinfo->description,
>  		sizeof(f->description));
>  
> @@ -537,13 +547,14 @@ xvip_dma_get_format(struct file *file, void *fh, struct v4l2_format *format)
>  	struct v4l2_fh *vfh = file->private_data;
>  	struct xvip_dma *dma = to_xvip_dma(vfh->vdev);
>  
> -	format->fmt.pix = dma->format;
> +	format->fmt.pix_mp = dma->format.fmt.pix_mp;
>  
>  	return 0;
>  }
>  
>  static void
> -__xvip_dma_try_format(struct xvip_dma *dma, struct v4l2_pix_format *pix,
> +__xvip_dma_try_format(struct xvip_dma *dma,
> +		      struct v4l2_format *format,
>  		      const struct xvip_video_format **fmtinfo)
>  {
>  	const struct xvip_video_format *info;
> @@ -551,19 +562,21 @@ __xvip_dma_try_format(struct xvip_dma *dma, struct v4l2_pix_format *pix,
>  	unsigned int max_width;
>  	unsigned int min_bpl;
>  	unsigned int max_bpl;
> -	unsigned int width;
> +	unsigned int width, height;
>  	unsigned int align;
>  	unsigned int bpl;
> +	unsigned int i;
> +	struct v4l2_pix_format_mplane *pix_mp = &format->fmt.pix_mp;
> +	struct v4l2_plane_pix_format *plane_fmt = pix_mp->plane_fmt;
>  
>  	/* Retrieve format information and select the default format if the
>  	 * requested format isn't supported.
>  	 */
> -	info = xvip_get_format_by_fourcc(pix->pixelformat);
> +	info = xvip_get_format_by_fourcc(pix_mp->pixelformat);
>  	if (IS_ERR(info))
>  		info = xvip_get_format_by_fourcc(XVIP_DMA_DEF_FORMAT);
>  
> -	pix->pixelformat = info->fourcc;
> -	pix->field = V4L2_FIELD_NONE;
> +	pix_mp->field = V4L2_FIELD_NONE;
>  
>  	/* The transfer alignment requirements are expressed in bytes. Compute
>  	 * the minimum and maximum values, clamp the requested width and convert
> @@ -572,22 +585,38 @@ __xvip_dma_try_format(struct xvip_dma *dma, struct v4l2_pix_format *pix,
>  	align = lcm(dma->align, info->bpp[0]);
>  	min_width = roundup(XVIP_DMA_MIN_WIDTH, align);
>  	max_width = rounddown(XVIP_DMA_MAX_WIDTH, align);
> -	width = rounddown(pix->width * info->bpp[0], align);
> -
> -	pix->width = clamp(width, min_width, max_width) / info->bpp[0];
> -	pix->height = clamp(pix->height, XVIP_DMA_MIN_HEIGHT,
> -			    XVIP_DMA_MAX_HEIGHT);
> +	pix_mp->width = clamp(pix_mp->width, min_width, max_width);
> +	pix_mp->height = clamp(pix_mp->height, XVIP_DMA_MIN_HEIGHT,
> +			       XVIP_DMA_MAX_HEIGHT);
>  
> -	/* Clamp the requested bytes per line value. If the maximum bytes per
> -	 * line value is zero, the module doesn't support user configurable line
> -	 * sizes. Override the requested value with the minimum in that case.
> +	/*
> +	 * Clamp the requested bytes per line value. If the maximum
> +	 * bytes per line value is zero, the module doesn't support
> +	 * user configurable line sizes. Override the requested value
> +	 * with the minimum in that case.
>  	 */
> -	min_bpl = pix->width * info->bpp[0];
> -	max_bpl = rounddown(XVIP_DMA_MAX_WIDTH, dma->align);
> -	bpl = rounddown(pix->bytesperline, dma->align);
> -
> -	pix->bytesperline = clamp(bpl, min_bpl, max_bpl);
> -	pix->sizeimage = pix->bytesperline * pix->height;
> +	max_bpl = rounddown(XVIP_DMA_MAX_WIDTH, align);
> +	min_bpl = pix_mp->width * info->bpp[0];
> +	min_bpl = roundup(min_bpl, align);
> +	bpl = roundup(plane_fmt[0].bytesperline, align);
> +	plane_fmt[0].bytesperline = clamp(bpl, min_bpl, max_bpl);
> +
> +	if (info->num_planes == 1) {
> +		/* Single plane formats */
> +		plane_fmt[0].sizeimage = plane_fmt[0].bytesperline *
> +					 pix_mp->height;
> +	} else {
> +		/* Supports Multi plane formats in a contiguous buffer,
> +		 * so we need only one buffer
> +		 */

Please use multiline comment style.

> +		plane_fmt[0].sizeimage = 0;

You can declare 'i' here.

> +		for (i = 0; i < info->num_planes; i++) {
> +			width = plane_fmt[0].bytesperline /
> +				(i ? info->hsub : 1);
> +			height = pix_mp->height / (i ? info->vsub : 1);
> +			plane_fmt[0].sizeimage += width * info->bpp[i] * height;

Just to clarify, only contiguous formats are supported, which is aligned with
the comment above on nplane > 1, correct?

> +		}
> +	}
>  
>  	if (fmtinfo)
>  		*fmtinfo = info;
> @@ -599,7 +628,7 @@ xvip_dma_try_format(struct file *file, void *fh, struct v4l2_format *format)
>  	struct v4l2_fh *vfh = file->private_data;
>  	struct xvip_dma *dma = to_xvip_dma(vfh->vdev);
>  
> -	__xvip_dma_try_format(dma, &format->fmt.pix, NULL);
> +	__xvip_dma_try_format(dma, format, NULL);
>  	return 0;
>  }
>  
> @@ -610,12 +639,13 @@ xvip_dma_set_format(struct file *file, void *fh, struct v4l2_format *format)
>  	struct xvip_dma *dma = to_xvip_dma(vfh->vdev);
>  	const struct xvip_video_format *info;
>  
> -	__xvip_dma_try_format(dma, &format->fmt.pix, &info);
> +	__xvip_dma_try_format(dma, format, &info);
>  
>  	if (vb2_is_busy(&dma->queue))
>  		return -EBUSY;
>  
> -	dma->format = format->fmt.pix;
> +	dma->format.fmt.pix_mp = format->fmt.pix_mp;
> +
>  	dma->fmtinfo = info;
>  
>  	return 0;
> @@ -623,13 +653,14 @@ xvip_dma_set_format(struct file *file, void *fh, struct v4l2_format *format)
>  
>  static const struct v4l2_ioctl_ops xvip_dma_ioctl_ops = {
>  	.vidioc_querycap		= xvip_dma_querycap,
> -	.vidioc_enum_fmt_vid_cap	= xvip_dma_enum_format,
> -	.vidioc_g_fmt_vid_cap		= xvip_dma_get_format,
> -	.vidioc_g_fmt_vid_out		= xvip_dma_get_format,
> -	.vidioc_s_fmt_vid_cap		= xvip_dma_set_format,
> -	.vidioc_s_fmt_vid_out		= xvip_dma_set_format,
> -	.vidioc_try_fmt_vid_cap		= xvip_dma_try_format,
> -	.vidioc_try_fmt_vid_out		= xvip_dma_try_format,
> +	.vidioc_enum_fmt_vid_cap_mplane	= xvip_dma_enum_format,
> +	.vidioc_enum_fmt_vid_out_mplane	= xvip_dma_enum_format,
> +	.vidioc_g_fmt_vid_cap_mplane	= xvip_dma_get_format,
> +	.vidioc_g_fmt_vid_out_mplane	= xvip_dma_get_format,
> +	.vidioc_s_fmt_vid_cap_mplane	= xvip_dma_set_format,
> +	.vidioc_s_fmt_vid_out_mplane	= xvip_dma_set_format,
> +	.vidioc_try_fmt_vid_cap_mplane	= xvip_dma_try_format,
> +	.vidioc_try_fmt_vid_out_mplane	= xvip_dma_try_format,
>  	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
>  	.vidioc_querybuf		= vb2_ioctl_querybuf,
>  	.vidioc_qbuf			= vb2_ioctl_qbuf,
> @@ -662,6 +693,7 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma,
>  {
>  	char name[16];
>  	int ret;
> +	struct v4l2_pix_format_mplane *pix_mp;
>  
>  	dma->xdev = xdev;
>  	dma->port = port;
> @@ -671,17 +703,23 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma,
>  	spin_lock_init(&dma->queued_lock);
>  
>  	dma->fmtinfo = xvip_get_format_by_fourcc(XVIP_DMA_DEF_FORMAT);
> -	dma->format.pixelformat = dma->fmtinfo->fourcc;
> -	dma->format.colorspace = V4L2_COLORSPACE_SRGB;
> -	dma->format.field = V4L2_FIELD_NONE;
> -	dma->format.width = XVIP_DMA_DEF_WIDTH;
> -	dma->format.height = XVIP_DMA_DEF_HEIGHT;
> -	dma->format.bytesperline = dma->format.width * dma->fmtinfo->bpp[0];
> -	dma->format.sizeimage = dma->format.bytesperline * dma->format.height;
> +	dma->format.type = type;
> +
> +	pix_mp = &dma->format.fmt.pix_mp;
> +	pix_mp->pixelformat = dma->fmtinfo->fourcc;
> +	pix_mp->colorspace = V4L2_COLORSPACE_SRGB;
> +	pix_mp->field = V4L2_FIELD_NONE;
> +	pix_mp->width = XVIP_DMA_DEF_WIDTH;
> +	pix_mp->plane_fmt[0].bytesperline = pix_mp->width *
> +					    dma->fmtinfo->bpp[0];
> +	pix_mp->plane_fmt[0].sizeimage = pix_mp->plane_fmt[0].bytesperline *
> +					 pix_mp->height;
>  
>  	/* Initialize the media entity... */
> -	dma->pad.flags = type == V4L2_BUF_TYPE_VIDEO_CAPTURE
> -		       ? MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
> +	if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> +		dma->pad.flags = MEDIA_PAD_FL_SINK;
> +	else if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> +		dma->pad.flags = MEDIA_PAD_FL_SOURCE;

I'd use 'else' or stick to the previous line just changing to MPLANE.

>  
>  	ret = media_entity_pads_init(&dma->video.entity, 1, &dma->pad);
>  	if (ret < 0)
> @@ -693,11 +731,16 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma,
>  	dma->video.queue = &dma->queue;
>  	snprintf(dma->video.name, sizeof(dma->video.name), "%s %s %u",
>  		 xdev->dev->of_node->name,
> -		 type == V4L2_BUF_TYPE_VIDEO_CAPTURE ? "output" : "input",
> +		 type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE
> +					? "output" : "input",
>  		 port);
> +
>  	dma->video.vfl_type = VFL_TYPE_GRABBER;
> -	dma->video.vfl_dir = type == V4L2_BUF_TYPE_VIDEO_CAPTURE
> -			   ? VFL_DIR_RX : VFL_DIR_TX;
> +	if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> +		dma->video.vfl_dir = VFL_DIR_RX;
> +	else if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> +		dma->video.vfl_dir = VFL_DIR_TX;

Ditto.

Thanks,
-hyun

> +
>  	dma->video.release = video_device_release_empty;
>  	dma->video.ioctl_ops = &xvip_dma_ioctl_ops;
>  	dma->video.lock = &dma->lock;
> diff --git a/drivers/media/platform/xilinx/xilinx-dma.h b/drivers/media/platform/xilinx/xilinx-dma.h
> index e95d136..96933ed 100644
> --- a/drivers/media/platform/xilinx/xilinx-dma.h
> +++ b/drivers/media/platform/xilinx/xilinx-dma.h
> @@ -62,7 +62,7 @@ static inline struct xvip_pipeline *to_xvip_pipeline(struct media_entity *e)
>   * @pipe: pipeline belonging to the DMA channel
>   * @port: composite device DT node port number for the DMA channel
>   * @lock: protects the @format, @fmtinfo and @queue fields
> - * @format: active V4L2 pixel format
> + * @format: V4L2 format
>   * @fmtinfo: format information corresponding to the active @format
>   * @queue: vb2 buffers queue
>   * @sequence: V4L2 buffers sequence number
> @@ -83,7 +83,7 @@ struct xvip_dma {
>  	unsigned int port;
>  
>  	struct mutex lock;
> -	struct v4l2_pix_format format;
> +	struct v4l2_format format;
>  	const struct xvip_video_format *fmtinfo;
>  
>  	struct vb2_queue queue;
> diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c b/drivers/media/platform/xilinx/xilinx-vipp.c
> index 6bb28cd..509b50f 100644
> --- a/drivers/media/platform/xilinx/xilinx-vipp.c
> +++ b/drivers/media/platform/xilinx/xilinx-vipp.c
> @@ -433,12 +433,15 @@ static int xvip_graph_dma_init_one(struct xvip_composite_device *xdev,
>  	if (ret < 0)
>  		return ret;
>  
> -	if (strcmp(direction, "input") == 0)
> -		type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> -	else if (strcmp(direction, "output") == 0)
> -		type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
> -	else
> +	if (strcmp(direction, "input") == 0) {
> +		type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> +		xdev->v4l2_caps |= V4L2_CAP_VIDEO_CAPTURE_MPLANE;
> +	} else if (strcmp(direction, "output") == 0) {
> +		type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> +		xdev->v4l2_caps |= V4L2_CAP_VIDEO_OUTPUT_MPLANE;
> +	} else {
>  		return -EINVAL;
> +	}
>  
>  	of_property_read_u32(node, "reg", &index);
>  
> @@ -454,9 +457,6 @@ static int xvip_graph_dma_init_one(struct xvip_composite_device *xdev,
>  
>  	list_add_tail(&dma->list, &xdev->dmas);
>  
> -	xdev->v4l2_caps |= type == V4L2_BUF_TYPE_VIDEO_CAPTURE
> -			 ? V4L2_CAP_VIDEO_CAPTURE : V4L2_CAP_VIDEO_OUTPUT;
> -
>  	return 0;
>  }
>  
> -- 
> 2.1.1
> 

RE: [PATCH v4 09/10] v4l: xilinx: dma: Add multi-planar support

[Satish Kumar Nagireddy]

Hi Hyun,

Thanks for the review.

> -----Original Message-----
> From: Hyun Kwon [mailto:hyun.kwon@xilinx.com]
> Sent: Tuesday, May 01, 2018 2:25 PM
> To: Satish Kumar Nagireddy <SATISHNA@xilinx.com>
> Cc: linux-media@vger.kernel.org; laurent.pinchart@ideasonboard.com;
> michal.simek@xilinx.com; Hyun Kwon <hyunk@xilinx.com>; Satish Kumar
> Nagireddy <SATISHNA@xilinx.com>
> Subject: Re: [PATCH v4 09/10] v4l: xilinx: dma: Add multi-planar support
> 
> On Mon, 2018-04-30 at 18:35:12 -0700, Satish Kumar Nagireddy wrote:
> > The current v4l driver supports single plane formats. This patch adds
> > support to handle multi-planar formats. Driver can handle both single
> > and multi-planar formats.
> >
> > Signed-off-by: Satish Kumar Nagireddy
> > <satish.nagireddy.nagireddy@xilinx.com>
> > ---
> >  drivers/media/platform/xilinx/xilinx-dma.c  | 159 ++++++++++++++++++----
> ------
> >  drivers/media/platform/xilinx/xilinx-dma.h  |   4 +-
> >  drivers/media/platform/xilinx/xilinx-vipp.c |  16 +--
> >  3 files changed, 111 insertions(+), 68 deletions(-)
> >
> > diff --git a/drivers/media/platform/xilinx/xilinx-dma.c
> > b/drivers/media/platform/xilinx/xilinx-dma.c
> > index 658586e..a714057 100644
> > --- a/drivers/media/platform/xilinx/xilinx-dma.c
> > +++ b/drivers/media/platform/xilinx/xilinx-dma.c
> > @@ -74,8 +74,8 @@ static int xvip_dma_verify_format(struct xvip_dma
> *dma)
> >  		return ret == -ENOIOCTLCMD ? -EINVAL : ret;
> >
> >  	if (dma->fmtinfo->code != fmt.format.code ||
> > -	    dma->format.height != fmt.format.height ||
> > -	    dma->format.width != fmt.format.width)
> > +	    dma->format.fmt.pix_mp.width != fmt.format.width ||
> > +	    dma->format.fmt.pix_mp.height != fmt.format.height)
> >  		return -EINVAL;
> >
> >  	return 0;
> > @@ -310,7 +310,8 @@ static void xvip_dma_complete(void *param)
> >  	buf->buf.field = V4L2_FIELD_NONE;
> >  	buf->buf.sequence = dma->sequence++;
> >  	buf->buf.vb2_buf.timestamp = ktime_get_ns();
> > -	vb2_set_plane_payload(&buf->buf.vb2_buf, 0, dma-
> >format.sizeimage);
> > +	vb2_set_plane_payload(&buf->buf.vb2_buf, 0,
> > +			      dma-
> >format.fmt.pix_mp.plane_fmt[0].sizeimage);
> >  	vb2_buffer_done(&buf->buf.vb2_buf, VB2_BUF_STATE_DONE);  }
> >
> > @@ -320,13 +321,15 @@ xvip_dma_queue_setup(struct vb2_queue *vq,
> >  		     unsigned int sizes[], struct device *alloc_devs[])  {
> >  	struct xvip_dma *dma = vb2_get_drv_priv(vq);
> > +	s32 sizeimage;
> 
> u32?
> 
> >
> >  	/* Make sure the image size is large enough. */
> > +	sizeimage = dma->format.fmt.pix_mp.plane_fmt[0].sizeimage;
> 
> I'm a little confused again. :-) This doesn't seem handling nplanes > 1, while
> there are such formats in the format table. is my understanding correct?

    [satish]: Current implementation of driver supports only contiguous multi-planar formats like V4L2_PIX_FMT_NV12
                    And not supporting non-contiguous formats like V4L2_PIX_FMT_NV12M.
> 
> >  	if (*nplanes)
> > -		return sizes[0] < dma->format.sizeimage ? -EINVAL : 0;
> > +		return sizes[0] < sizeimage ? -EINVAL : 0;
> >
> >  	*nplanes = 1;
> > -	sizes[0] = dma->format.sizeimage;
> > +	sizes[0] = sizeimage;
> >
> >  	return 0;
> >  }
> > @@ -350,14 +353,15 @@ static void xvip_dma_buffer_queue(struct
> vb2_buffer *vb)
> >  	struct dma_async_tx_descriptor *desc;
> >  	dma_addr_t addr = vb2_dma_contig_plane_dma_addr(vb, 0);
> >  	u32 flags;
> > +	struct v4l2_pix_format_mplane *pix_mp = &dma-
> >format.fmt.pix_mp;
> >
> > -	if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> > +	if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> {
> >  		flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
> >  		dma->xt.dir = DMA_DEV_TO_MEM;
> >  		dma->xt.src_sgl = false;
> >  		dma->xt.dst_sgl = true;
> >  		dma->xt.dst_start = addr;
> > -	} else {
> > +	} else if (dma->queue.type ==
> V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
> 
> I'd do 'else' even if any other value is not possible at this point.
> 
> >  		flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
> >  		dma->xt.dir = DMA_MEM_TO_DEV;
> >  		dma->xt.src_sgl = true;
> > @@ -365,10 +369,11 @@ static void xvip_dma_buffer_queue(struct
> vb2_buffer *vb)
> >  		dma->xt.src_start = addr;
> >  	}
> >
> > -	dma->xt.frame_size = 1;
> > -	dma->sgl[0].size = dma->format.width * dma->fmtinfo->bpp[0];
> > -	dma->sgl[0].icg = dma->format.bytesperline - dma->sgl[0].size;
> > -	dma->xt.numf = dma->format.height;
> > +	dma->xt.frame_size = dma->fmtinfo->num_planes;
> > +	dma->sgl[0].size = pix_mp->width * dma->fmtinfo->bpp[0];
> > +	dma->sgl[0].icg = pix_mp->plane_fmt[0].bytesperline - dma-
> >sgl[0].size;
> > +	dma->xt.numf = pix_mp->height;
> > +	dma->sgl[0].dst_icg = 0;
> >
> >  	desc = dmaengine_prep_interleaved_dma(dma->dma, &dma->xt,
> flags);
> >  	if (!desc) {
> > @@ -497,10 +502,15 @@ xvip_dma_querycap(struct file *file, void *fh,
> struct v4l2_capability *cap)
> >  	cap->capabilities = V4L2_CAP_DEVICE_CAPS | V4L2_CAP_STREAMING
> >  			  | dma->xdev->v4l2_caps;
> >
> > -	if (dma->queue.type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> > -		cap->device_caps = V4L2_CAP_VIDEO_CAPTURE |
> V4L2_CAP_STREAMING;
> > -	else
> > -		cap->device_caps = V4L2_CAP_VIDEO_OUTPUT |
> V4L2_CAP_STREAMING;
> > +	cap->device_caps = V4L2_CAP_STREAMING;
> > +	switch (dma->queue.type) {
> > +	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> > +		cap->device_caps |= V4L2_CAP_VIDEO_CAPTURE_MPLANE;
> > +		break;
> > +	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> > +		cap->device_caps |= V4L2_CAP_VIDEO_OUTPUT_MPLANE;
> > +		break;
> > +	}
> 
> I'd keep if - else rather than switch - case.
> 
> >
> >  	strlcpy(cap->driver, "xilinx-vipp", sizeof(cap->driver));
> >  	strlcpy(cap->card, dma->video.name, sizeof(cap->card)); @@ -524,7
> > +534,7 @@ xvip_dma_enum_format(struct file *file, void *fh, struct
> v4l2_fmtdesc *f)
> >  	if (f->index > 0)
> >  		return -EINVAL;
> >
> > -	f->pixelformat = dma->format.pixelformat;
> > +	f->pixelformat = dma->format.fmt.pix_mp.pixelformat;
> >  	strlcpy(f->description, dma->fmtinfo->description,
> >  		sizeof(f->description));
> >
> > @@ -537,13 +547,14 @@ xvip_dma_get_format(struct file *file, void *fh,
> struct v4l2_format *format)
> >  	struct v4l2_fh *vfh = file->private_data;
> >  	struct xvip_dma *dma = to_xvip_dma(vfh->vdev);
> >
> > -	format->fmt.pix = dma->format;
> > +	format->fmt.pix_mp = dma->format.fmt.pix_mp;
> >
> >  	return 0;
> >  }
> >
> >  static void
> > -__xvip_dma_try_format(struct xvip_dma *dma, struct v4l2_pix_format
> > *pix,
> > +__xvip_dma_try_format(struct xvip_dma *dma,
> > +		      struct v4l2_format *format,
> >  		      const struct xvip_video_format **fmtinfo)  {
> >  	const struct xvip_video_format *info; @@ -551,19 +562,21 @@
> > __xvip_dma_try_format(struct xvip_dma *dma, struct v4l2_pix_format
> *pix,
> >  	unsigned int max_width;
> >  	unsigned int min_bpl;
> >  	unsigned int max_bpl;
> > -	unsigned int width;
> > +	unsigned int width, height;
> >  	unsigned int align;
> >  	unsigned int bpl;
> > +	unsigned int i;
> > +	struct v4l2_pix_format_mplane *pix_mp = &format->fmt.pix_mp;
> > +	struct v4l2_plane_pix_format *plane_fmt = pix_mp->plane_fmt;
> >
> >  	/* Retrieve format information and select the default format if the
> >  	 * requested format isn't supported.
> >  	 */
> > -	info = xvip_get_format_by_fourcc(pix->pixelformat);
> > +	info = xvip_get_format_by_fourcc(pix_mp->pixelformat);
> >  	if (IS_ERR(info))
> >  		info =
> xvip_get_format_by_fourcc(XVIP_DMA_DEF_FORMAT);
> >
> > -	pix->pixelformat = info->fourcc;
> > -	pix->field = V4L2_FIELD_NONE;
> > +	pix_mp->field = V4L2_FIELD_NONE;
> >
> >  	/* The transfer alignment requirements are expressed in bytes.
> Compute
> >  	 * the minimum and maximum values, clamp the requested width
> and
> > convert @@ -572,22 +585,38 @@ __xvip_dma_try_format(struct xvip_dma
> *dma, struct v4l2_pix_format *pix,
> >  	align = lcm(dma->align, info->bpp[0]);
> >  	min_width = roundup(XVIP_DMA_MIN_WIDTH, align);
> >  	max_width = rounddown(XVIP_DMA_MAX_WIDTH, align);
> > -	width = rounddown(pix->width * info->bpp[0], align);
> > -
> > -	pix->width = clamp(width, min_width, max_width) / info->bpp[0];
> > -	pix->height = clamp(pix->height, XVIP_DMA_MIN_HEIGHT,
> > -			    XVIP_DMA_MAX_HEIGHT);
> > +	pix_mp->width = clamp(pix_mp->width, min_width, max_width);
> > +	pix_mp->height = clamp(pix_mp->height, XVIP_DMA_MIN_HEIGHT,
> > +			       XVIP_DMA_MAX_HEIGHT);
> >
> > -	/* Clamp the requested bytes per line value. If the maximum bytes
> per
> > -	 * line value is zero, the module doesn't support user configurable
> line
> > -	 * sizes. Override the requested value with the minimum in that case.
> > +	/*
> > +	 * Clamp the requested bytes per line value. If the maximum
> > +	 * bytes per line value is zero, the module doesn't support
> > +	 * user configurable line sizes. Override the requested value
> > +	 * with the minimum in that case.
> >  	 */
> > -	min_bpl = pix->width * info->bpp[0];
> > -	max_bpl = rounddown(XVIP_DMA_MAX_WIDTH, dma->align);
> > -	bpl = rounddown(pix->bytesperline, dma->align);
> > -
> > -	pix->bytesperline = clamp(bpl, min_bpl, max_bpl);
> > -	pix->sizeimage = pix->bytesperline * pix->height;
> > +	max_bpl = rounddown(XVIP_DMA_MAX_WIDTH, align);
> > +	min_bpl = pix_mp->width * info->bpp[0];
> > +	min_bpl = roundup(min_bpl, align);
> > +	bpl = roundup(plane_fmt[0].bytesperline, align);
> > +	plane_fmt[0].bytesperline = clamp(bpl, min_bpl, max_bpl);
> > +
> > +	if (info->num_planes == 1) {
> > +		/* Single plane formats */
> > +		plane_fmt[0].sizeimage = plane_fmt[0].bytesperline *
> > +					 pix_mp->height;
> > +	} else {
> > +		/* Supports Multi plane formats in a contiguous buffer,
> > +		 * so we need only one buffer
> > +		 */
> 
> Please use multiline comment style.
> 
> > +		plane_fmt[0].sizeimage = 0;
> 
> You can declare 'i' here.
> 
> > +		for (i = 0; i < info->num_planes; i++) {
> > +			width = plane_fmt[0].bytesperline /
> > +				(i ? info->hsub : 1);
> > +			height = pix_mp->height / (i ? info->vsub : 1);
> > +			plane_fmt[0].sizeimage += width * info->bpp[i] *
> height;
> 
> Just to clarify, only contiguous formats are supported, which is aligned with
> the comment above on nplane > 1, correct?

     [Satish]: Yes

Regards,
Satish

> 
> > +		}
> > +	}
> >
> >  	if (fmtinfo)
> >  		*fmtinfo = info;
> > @@ -599,7 +628,7 @@ xvip_dma_try_format(struct file *file, void *fh,
> struct v4l2_format *format)
> >  	struct v4l2_fh *vfh = file->private_data;
> >  	struct xvip_dma *dma = to_xvip_dma(vfh->vdev);
> >
> > -	__xvip_dma_try_format(dma, &format->fmt.pix, NULL);
> > +	__xvip_dma_try_format(dma, format, NULL);
> >  	return 0;
> >  }
> >
> > @@ -610,12 +639,13 @@ xvip_dma_set_format(struct file *file, void *fh,
> struct v4l2_format *format)
> >  	struct xvip_dma *dma = to_xvip_dma(vfh->vdev);
> >  	const struct xvip_video_format *info;
> >
> > -	__xvip_dma_try_format(dma, &format->fmt.pix, &info);
> > +	__xvip_dma_try_format(dma, format, &info);
> >
> >  	if (vb2_is_busy(&dma->queue))
> >  		return -EBUSY;
> >
> > -	dma->format = format->fmt.pix;
> > +	dma->format.fmt.pix_mp = format->fmt.pix_mp;
> > +
> >  	dma->fmtinfo = info;
> >
> >  	return 0;
> > @@ -623,13 +653,14 @@ xvip_dma_set_format(struct file *file, void *fh,
> > struct v4l2_format *format)
> >
> >  static const struct v4l2_ioctl_ops xvip_dma_ioctl_ops = {
> >  	.vidioc_querycap		= xvip_dma_querycap,
> > -	.vidioc_enum_fmt_vid_cap	= xvip_dma_enum_format,
> > -	.vidioc_g_fmt_vid_cap		= xvip_dma_get_format,
> > -	.vidioc_g_fmt_vid_out		= xvip_dma_get_format,
> > -	.vidioc_s_fmt_vid_cap		= xvip_dma_set_format,
> > -	.vidioc_s_fmt_vid_out		= xvip_dma_set_format,
> > -	.vidioc_try_fmt_vid_cap		= xvip_dma_try_format,
> > -	.vidioc_try_fmt_vid_out		= xvip_dma_try_format,
> > +	.vidioc_enum_fmt_vid_cap_mplane	= xvip_dma_enum_format,
> > +	.vidioc_enum_fmt_vid_out_mplane	= xvip_dma_enum_format,
> > +	.vidioc_g_fmt_vid_cap_mplane	= xvip_dma_get_format,
> > +	.vidioc_g_fmt_vid_out_mplane	= xvip_dma_get_format,
> > +	.vidioc_s_fmt_vid_cap_mplane	= xvip_dma_set_format,
> > +	.vidioc_s_fmt_vid_out_mplane	= xvip_dma_set_format,
> > +	.vidioc_try_fmt_vid_cap_mplane	= xvip_dma_try_format,
> > +	.vidioc_try_fmt_vid_out_mplane	= xvip_dma_try_format,
> >  	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
> >  	.vidioc_querybuf		= vb2_ioctl_querybuf,
> >  	.vidioc_qbuf			= vb2_ioctl_qbuf,
> > @@ -662,6 +693,7 @@ int xvip_dma_init(struct xvip_composite_device
> > *xdev, struct xvip_dma *dma,  {
> >  	char name[16];
> >  	int ret;
> > +	struct v4l2_pix_format_mplane *pix_mp;
> >
> >  	dma->xdev = xdev;
> >  	dma->port = port;
> > @@ -671,17 +703,23 @@ int xvip_dma_init(struct xvip_composite_device
> *xdev, struct xvip_dma *dma,
> >  	spin_lock_init(&dma->queued_lock);
> >
> >  	dma->fmtinfo =
> xvip_get_format_by_fourcc(XVIP_DMA_DEF_FORMAT);
> > -	dma->format.pixelformat = dma->fmtinfo->fourcc;
> > -	dma->format.colorspace = V4L2_COLORSPACE_SRGB;
> > -	dma->format.field = V4L2_FIELD_NONE;
> > -	dma->format.width = XVIP_DMA_DEF_WIDTH;
> > -	dma->format.height = XVIP_DMA_DEF_HEIGHT;
> > -	dma->format.bytesperline = dma->format.width * dma->fmtinfo-
> >bpp[0];
> > -	dma->format.sizeimage = dma->format.bytesperline * dma-
> >format.height;
> > +	dma->format.type = type;
> > +
> > +	pix_mp = &dma->format.fmt.pix_mp;
> > +	pix_mp->pixelformat = dma->fmtinfo->fourcc;
> > +	pix_mp->colorspace = V4L2_COLORSPACE_SRGB;
> > +	pix_mp->field = V4L2_FIELD_NONE;
> > +	pix_mp->width = XVIP_DMA_DEF_WIDTH;
> > +	pix_mp->plane_fmt[0].bytesperline = pix_mp->width *
> > +					    dma->fmtinfo->bpp[0];
> > +	pix_mp->plane_fmt[0].sizeimage = pix_mp-
> >plane_fmt[0].bytesperline *
> > +					 pix_mp->height;
> >
> >  	/* Initialize the media entity... */
> > -	dma->pad.flags = type == V4L2_BUF_TYPE_VIDEO_CAPTURE
> > -		       ? MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
> > +	if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> > +		dma->pad.flags = MEDIA_PAD_FL_SINK;
> > +	else if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > +		dma->pad.flags = MEDIA_PAD_FL_SOURCE;
> 
> I'd use 'else' or stick to the previous line just changing to MPLANE.
> 
> >
> >  	ret = media_entity_pads_init(&dma->video.entity, 1, &dma->pad);
> >  	if (ret < 0)
> > @@ -693,11 +731,16 @@ int xvip_dma_init(struct xvip_composite_device
> *xdev, struct xvip_dma *dma,
> >  	dma->video.queue = &dma->queue;
> >  	snprintf(dma->video.name, sizeof(dma->video.name), "%s %s %u",
> >  		 xdev->dev->of_node->name,
> > -		 type == V4L2_BUF_TYPE_VIDEO_CAPTURE ? "output" :
> "input",
> > +		 type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE
> > +					? "output" : "input",
> >  		 port);
> > +
> >  	dma->video.vfl_type = VFL_TYPE_GRABBER;
> > -	dma->video.vfl_dir = type == V4L2_BUF_TYPE_VIDEO_CAPTURE
> > -			   ? VFL_DIR_RX : VFL_DIR_TX;
> > +	if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> > +		dma->video.vfl_dir = VFL_DIR_RX;
> > +	else if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > +		dma->video.vfl_dir = VFL_DIR_TX;
> 
> Ditto.
> 
> Thanks,
> -hyun
> 
> > +
> >  	dma->video.release = video_device_release_empty;
> >  	dma->video.ioctl_ops = &xvip_dma_ioctl_ops;
> >  	dma->video.lock = &dma->lock;
> > diff --git a/drivers/media/platform/xilinx/xilinx-dma.h
> > b/drivers/media/platform/xilinx/xilinx-dma.h
> > index e95d136..96933ed 100644
> > --- a/drivers/media/platform/xilinx/xilinx-dma.h
> > +++ b/drivers/media/platform/xilinx/xilinx-dma.h
> > @@ -62,7 +62,7 @@ static inline struct xvip_pipeline
> *to_xvip_pipeline(struct media_entity *e)
> >   * @pipe: pipeline belonging to the DMA channel
> >   * @port: composite device DT node port number for the DMA channel
> >   * @lock: protects the @format, @fmtinfo and @queue fields
> > - * @format: active V4L2 pixel format
> > + * @format: V4L2 format
> >   * @fmtinfo: format information corresponding to the active @format
> >   * @queue: vb2 buffers queue
> >   * @sequence: V4L2 buffers sequence number @@ -83,7 +83,7 @@ struct
> > xvip_dma {
> >  	unsigned int port;
> >
> >  	struct mutex lock;
> > -	struct v4l2_pix_format format;
> > +	struct v4l2_format format;
> >  	const struct xvip_video_format *fmtinfo;
> >
> >  	struct vb2_queue queue;
> > diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c
> > b/drivers/media/platform/xilinx/xilinx-vipp.c
> > index 6bb28cd..509b50f 100644
> > --- a/drivers/media/platform/xilinx/xilinx-vipp.c
> > +++ b/drivers/media/platform/xilinx/xilinx-vipp.c
> > @@ -433,12 +433,15 @@ static int xvip_graph_dma_init_one(struct
> xvip_composite_device *xdev,
> >  	if (ret < 0)
> >  		return ret;
> >
> > -	if (strcmp(direction, "input") == 0)
> > -		type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> > -	else if (strcmp(direction, "output") == 0)
> > -		type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
> > -	else
> > +	if (strcmp(direction, "input") == 0) {
> > +		type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> > +		xdev->v4l2_caps |= V4L2_CAP_VIDEO_CAPTURE_MPLANE;
> > +	} else if (strcmp(direction, "output") == 0) {
> > +		type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> > +		xdev->v4l2_caps |= V4L2_CAP_VIDEO_OUTPUT_MPLANE;
> > +	} else {
> >  		return -EINVAL;
> > +	}
> >
> >  	of_property_read_u32(node, "reg", &index);
> >
> > @@ -454,9 +457,6 @@ static int xvip_graph_dma_init_one(struct
> > xvip_composite_device *xdev,
> >
> >  	list_add_tail(&dma->list, &xdev->dmas);
> >
> > -	xdev->v4l2_caps |= type == V4L2_BUF_TYPE_VIDEO_CAPTURE
> > -			 ? V4L2_CAP_VIDEO_CAPTURE :
> V4L2_CAP_VIDEO_OUTPUT;
> > -
> >  	return 0;
> >  }
> >
> > --
> > 2.1.1
> >

[PATCH v4 10/10] v4l: xilinx: dma: Add support for 10 bit formats

[Satish Kumar Nagireddy]

This patch adds xvip_format_plane_width_bytes function to
calculate number of bytes for a macropixel formats and also
adds new 10 bit pixel formats to video descriptor table.

Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com>
---
 drivers/media/platform/xilinx/xilinx-dma.c |  5 ++--
 drivers/media/platform/xilinx/xilinx-vip.c | 43 +++++++++++++++++++++---------
 drivers/media/platform/xilinx/xilinx-vip.h |  5 ++++
 3 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
index a714057..b33e4b9 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -370,7 +370,8 @@ static void xvip_dma_buffer_queue(struct vb2_buffer *vb)
 	}
 
 	dma->xt.frame_size = dma->fmtinfo->num_planes;
-	dma->sgl[0].size = pix_mp->width * dma->fmtinfo->bpp[0];
+	dma->sgl[0].size = xvip_fmt_plane_width_bytes(dma->fmtinfo,
+						      pix_mp->width);
 	dma->sgl[0].icg = pix_mp->plane_fmt[0].bytesperline - dma->sgl[0].size;
 	dma->xt.numf = pix_mp->height;
 	dma->sgl[0].dst_icg = 0;
@@ -596,7 +597,7 @@ __xvip_dma_try_format(struct xvip_dma *dma,
 	 * with the minimum in that case.
 	 */
 	max_bpl = rounddown(XVIP_DMA_MAX_WIDTH, align);
-	min_bpl = pix_mp->width * info->bpp[0];
+	min_bpl = xvip_fmt_plane_width_bytes(info, pix_mp->width);
 	min_bpl = roundup(min_bpl, align);
 	bpl = roundup(plane_fmt[0].bytesperline, align);
 	plane_fmt[0].bytesperline = clamp(bpl, min_bpl, max_bpl);
diff --git a/drivers/media/platform/xilinx/xilinx-vip.c b/drivers/media/platform/xilinx/xilinx-vip.c
index 81cc0d2..1825f5d 100644
--- a/drivers/media/platform/xilinx/xilinx-vip.c
+++ b/drivers/media/platform/xilinx/xilinx-vip.c
@@ -28,31 +28,31 @@
 
 static const struct xvip_video_format xvip_video_formats[] = {
 	{ XVIP_VF_YUV_420, 8, NULL, MEDIA_BUS_FMT_VYYUYY8_1X24,
-	  {1, 2, 0}, V4L2_PIX_FMT_NV12, 2, 2, 2, "4:2:0, semi-planar, YUV" },
+	  {1, 2, 0}, V4L2_PIX_FMT_NV12, 2, 2, 2, 1, 1, "4:2:0, semi-planar, YUV" },
 	{ XVIP_VF_YUV_420, 10, NULL, MEDIA_BUS_FMT_VYYUYY8_1X24,
-	  {1, 2, 0}, V4L2_PIX_FMT_XV15, 2, 2, 2, "4:2:0, 10-bit 2-plane cont" },
+	  {1, 2, 0}, V4L2_PIX_FMT_XV15, 2, 2, 2, 4, 3, "4:2:0, 10-bit 2-plane cont" },
 	{ XVIP_VF_YUV_422, 8, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
-	  {2, 0, 0}, V4L2_PIX_FMT_YUYV, 1, 2, 1, "4:2:2, packed, YUYV" },
+	  {2, 0, 0}, V4L2_PIX_FMT_YUYV, 1, 2, 1, 1, 1, "4:2:2, packed, YUYV" },
 	{ XVIP_VF_VUY_422, 8, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
-	  {2, 0, 0}, V4L2_PIX_FMT_UYVY, 1, 2, 1, "4:2:2, packed, UYVY" },
+	  {2, 0, 0}, V4L2_PIX_FMT_UYVY, 1, 2, 1, 1, 1, "4:2:2, packed, UYVY" },
 	{ XVIP_VF_YUV_422, 8, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
-	  {1, 2, 0}, V4L2_PIX_FMT_NV16, 2, 2, 1, "4:2:2, semi-planar, YUV" },
+	  {1, 2, 0}, V4L2_PIX_FMT_NV16, 2, 2, 1, 1, 1, "4:2:2, semi-planar, YUV" },
 	{ XVIP_VF_YUV_422, 10, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
-	  {1, 2, 0}, V4L2_PIX_FMT_XV20, 2, 2, 1, "4:2:2, 10-bit 2-plane cont" },
+	  {1, 2, 0}, V4L2_PIX_FMT_XV20, 2, 2, 1, 4, 3, "4:2:2, 10-bit 2-plane cont" },
 	{ XVIP_VF_RBG, 8, NULL, MEDIA_BUS_FMT_RBG888_1X24,
-	  {3, 0, 0}, V4L2_PIX_FMT_BGR24, 1, 1, 1, "24-bit RGB" },
+	  {3, 0, 0}, V4L2_PIX_FMT_BGR24, 1, 1, 1, 1, 1, "24-bit RGB" },
 	{ XVIP_VF_RBG, 8, NULL, MEDIA_BUS_FMT_RBG888_1X24,
-	  {3, 0, 0}, V4L2_PIX_FMT_RGB24, 1, 1, 1, "24-bit RGB" },
+	  {3, 0, 0}, V4L2_PIX_FMT_RGB24, 1, 1, 1, 1, 1, "24-bit RGB" },
 	{ XVIP_VF_MONO_SENSOR, 8, "mono", MEDIA_BUS_FMT_Y8_1X8,
-	  {1, 0, 0}, V4L2_PIX_FMT_GREY, 1, 1, 1, "Greyscale 8-bit" },
+	  {1, 0, 0}, V4L2_PIX_FMT_GREY, 1, 1, 1, 1, 1, "Greyscale 8-bit" },
 	{ XVIP_VF_MONO_SENSOR, 8, "rggb", MEDIA_BUS_FMT_SRGGB8_1X8,
-	  {1, 0, 0}, V4L2_PIX_FMT_SGRBG8, 1, 1, 1, "Bayer 8-bit RGGB" },
+	  {1, 0, 0}, V4L2_PIX_FMT_SGRBG8, 1, 1, 1, 1, 1, "Bayer 8-bit RGGB" },
 	{ XVIP_VF_MONO_SENSOR, 8, "grbg", MEDIA_BUS_FMT_SGRBG8_1X8,
-	  {1, 0, 0}, V4L2_PIX_FMT_SGRBG8, 1, 1, 1, "Bayer 8-bit GRBG" },
+	  {1, 0, 0}, V4L2_PIX_FMT_SGRBG8, 1, 1, 1, 1, 1, "Bayer 8-bit GRBG" },
 	{ XVIP_VF_MONO_SENSOR, 8, "gbrg", MEDIA_BUS_FMT_SGBRG8_1X8,
-	  {1, 0, 0}, V4L2_PIX_FMT_SGBRG8, 1, 1, 1, "Bayer 8-bit GBRG" },
+	  {1, 0, 0}, V4L2_PIX_FMT_SGBRG8, 1, 1, 1, 1, 1, "Bayer 8-bit GBRG" },
 	{ XVIP_VF_MONO_SENSOR, 8, "bggr", MEDIA_BUS_FMT_SBGGR8_1X8,
-	  {1, 0, 0}, V4L2_PIX_FMT_SBGGR8, 1, 1, 1, "Bayer 8-bit BGGR" },
+	  {1, 0, 0}, V4L2_PIX_FMT_SBGGR8, 1, 1, 1, 1, 1, "Bayer 8-bit BGGR" },
 };
 
 /**
@@ -102,6 +102,23 @@ const struct xvip_video_format *xvip_get_format_by_fourcc(u32 fourcc)
 EXPORT_SYMBOL_GPL(xvip_get_format_by_fourcc);
 
 /**
+ * xvip_fmt_plane_width_bytes - bytes of the given width of the plane
+ * @info: VIP format description
+ * @width: width
+ *
+ * Return: Returns the number of bytes for given @width
+ */
+int xvip_fmt_plane_width_bytes(const struct xvip_video_format *info, u32 width)
+{
+	if (!info)
+		return 0;
+
+	return DIV_ROUND_UP(width * info->bytes_per_macropixel * info->bpp[0],
+			    info->pixels_per_macropixel);
+}
+EXPORT_SYMBOL_GPL(xvip_fmt_plane_width_bytes);
+
+/**
  * xvip_of_get_format - Parse a device tree node and return format information
  * @node: the device tree node
  *
diff --git a/drivers/media/platform/xilinx/xilinx-vip.h b/drivers/media/platform/xilinx/xilinx-vip.h
index 5e7a978..7c614d3 100644
--- a/drivers/media/platform/xilinx/xilinx-vip.h
+++ b/drivers/media/platform/xilinx/xilinx-vip.h
@@ -114,6 +114,8 @@ struct xvip_device {
  * @num_planes: number of planes w.r.t. color format
  * @hsub: Horizontal sampling factor of Chroma
  * @vsub: Vertical sampling factor of Chroma
+ * @bytes_per_macropixel: Number of bytes per macro-pixel
+ * @pixels_per_macropixel: Number of pixels per macro-pixel
  * @description: format description, suitable for userspace
  */
 struct xvip_video_format {
@@ -126,12 +128,15 @@ struct xvip_video_format {
 	u8 num_planes;
 	u8 hsub;
 	u8 vsub;
+	u32 bytes_per_macropixel;
+	u32 pixels_per_macropixel;
 	const char *description;
 };
 
 const struct xvip_video_format *xvip_get_format_by_code(unsigned int code);
 const struct xvip_video_format *xvip_get_format_by_fourcc(u32 fourcc);
 const struct xvip_video_format *xvip_of_get_format(struct device_node *node);
+int xvip_fmt_plane_width_bytes(const struct xvip_video_format *info, u32 width);
 void xvip_set_format_size(struct v4l2_mbus_framefmt *format,
 			  const struct v4l2_subdev_format *fmt);
 int xvip_enum_mbus_code(struct v4l2_subdev *subdev,
-- 
2.1.1

Re: [PATCH v4 10/10] v4l: xilinx: dma: Add support for 10 bit formats

[Hyun Kwon]

Hi Satish,

Thanks for that patch.

On Mon, 2018-04-30 at 18:35:13 -0700, Satish Kumar Nagireddy wrote:
> This patch adds xvip_format_plane_width_bytes function to
> calculate number of bytes for a macropixel formats and also
> adds new 10 bit pixel formats to video descriptor table.
> 
> Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com>
> ---
>  drivers/media/platform/xilinx/xilinx-dma.c |  5 ++--
>  drivers/media/platform/xilinx/xilinx-vip.c | 43 +++++++++++++++++++++---------
>  drivers/media/platform/xilinx/xilinx-vip.h |  5 ++++
>  3 files changed, 38 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
> index a714057..b33e4b9 100644
> --- a/drivers/media/platform/xilinx/xilinx-dma.c
> +++ b/drivers/media/platform/xilinx/xilinx-dma.c
> @@ -370,7 +370,8 @@ static void xvip_dma_buffer_queue(struct vb2_buffer *vb)
>  	}
>  
>  	dma->xt.frame_size = dma->fmtinfo->num_planes;
> -	dma->sgl[0].size = pix_mp->width * dma->fmtinfo->bpp[0];
> +	dma->sgl[0].size = xvip_fmt_plane_width_bytes(dma->fmtinfo,
> +						      pix_mp->width);
>  	dma->sgl[0].icg = pix_mp->plane_fmt[0].bytesperline - dma->sgl[0].size;
>  	dma->xt.numf = pix_mp->height;
>  	dma->sgl[0].dst_icg = 0;
> @@ -596,7 +597,7 @@ __xvip_dma_try_format(struct xvip_dma *dma,
>  	 * with the minimum in that case.
>  	 */
>  	max_bpl = rounddown(XVIP_DMA_MAX_WIDTH, align);
> -	min_bpl = pix_mp->width * info->bpp[0];
> +	min_bpl = xvip_fmt_plane_width_bytes(info, pix_mp->width);
>  	min_bpl = roundup(min_bpl, align);
>  	bpl = roundup(plane_fmt[0].bytesperline, align);
>  	plane_fmt[0].bytesperline = clamp(bpl, min_bpl, max_bpl);
> diff --git a/drivers/media/platform/xilinx/xilinx-vip.c b/drivers/media/platform/xilinx/xilinx-vip.c
> index 81cc0d2..1825f5d 100644
> --- a/drivers/media/platform/xilinx/xilinx-vip.c
> +++ b/drivers/media/platform/xilinx/xilinx-vip.c
> @@ -28,31 +28,31 @@
>  
>  static const struct xvip_video_format xvip_video_formats[] = {
>  	{ XVIP_VF_YUV_420, 8, NULL, MEDIA_BUS_FMT_VYYUYY8_1X24,
> -	  {1, 2, 0}, V4L2_PIX_FMT_NV12, 2, 2, 2, "4:2:0, semi-planar, YUV" },
> +	  {1, 2, 0}, V4L2_PIX_FMT_NV12, 2, 2, 2, 1, 1, "4:2:0, semi-planar, YUV" },
>  	{ XVIP_VF_YUV_420, 10, NULL, MEDIA_BUS_FMT_VYYUYY8_1X24,
> -	  {1, 2, 0}, V4L2_PIX_FMT_XV15, 2, 2, 2, "4:2:0, 10-bit 2-plane cont" },
> +	  {1, 2, 0}, V4L2_PIX_FMT_XV15, 2, 2, 2, 4, 3, "4:2:0, 10-bit 2-plane cont" },
>  	{ XVIP_VF_YUV_422, 8, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
> -	  {2, 0, 0}, V4L2_PIX_FMT_YUYV, 1, 2, 1, "4:2:2, packed, YUYV" },
> +	  {2, 0, 0}, V4L2_PIX_FMT_YUYV, 1, 2, 1, 1, 1, "4:2:2, packed, YUYV" },
>  	{ XVIP_VF_VUY_422, 8, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
> -	  {2, 0, 0}, V4L2_PIX_FMT_UYVY, 1, 2, 1, "4:2:2, packed, UYVY" },
> +	  {2, 0, 0}, V4L2_PIX_FMT_UYVY, 1, 2, 1, 1, 1, "4:2:2, packed, UYVY" },
>  	{ XVIP_VF_YUV_422, 8, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
> -	  {1, 2, 0}, V4L2_PIX_FMT_NV16, 2, 2, 1, "4:2:2, semi-planar, YUV" },
> +	  {1, 2, 0}, V4L2_PIX_FMT_NV16, 2, 2, 1, 1, 1, "4:2:2, semi-planar, YUV" },
>  	{ XVIP_VF_YUV_422, 10, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
> -	  {1, 2, 0}, V4L2_PIX_FMT_XV20, 2, 2, 1, "4:2:2, 10-bit 2-plane cont" },
> +	  {1, 2, 0}, V4L2_PIX_FMT_XV20, 2, 2, 1, 4, 3, "4:2:2, 10-bit 2-plane cont" },
>  	{ XVIP_VF_RBG, 8, NULL, MEDIA_BUS_FMT_RBG888_1X24,
> -	  {3, 0, 0}, V4L2_PIX_FMT_BGR24, 1, 1, 1, "24-bit RGB" },
> +	  {3, 0, 0}, V4L2_PIX_FMT_BGR24, 1, 1, 1, 1, 1, "24-bit RGB" },
>  	{ XVIP_VF_RBG, 8, NULL, MEDIA_BUS_FMT_RBG888_1X24,
> -	  {3, 0, 0}, V4L2_PIX_FMT_RGB24, 1, 1, 1, "24-bit RGB" },
> +	  {3, 0, 0}, V4L2_PIX_FMT_RGB24, 1, 1, 1, 1, 1, "24-bit RGB" },
>  	{ XVIP_VF_MONO_SENSOR, 8, "mono", MEDIA_BUS_FMT_Y8_1X8,
> -	  {1, 0, 0}, V4L2_PIX_FMT_GREY, 1, 1, 1, "Greyscale 8-bit" },
> +	  {1, 0, 0}, V4L2_PIX_FMT_GREY, 1, 1, 1, 1, 1, "Greyscale 8-bit" },
>  	{ XVIP_VF_MONO_SENSOR, 8, "rggb", MEDIA_BUS_FMT_SRGGB8_1X8,
> -	  {1, 0, 0}, V4L2_PIX_FMT_SGRBG8, 1, 1, 1, "Bayer 8-bit RGGB" },
> +	  {1, 0, 0}, V4L2_PIX_FMT_SGRBG8, 1, 1, 1, 1, 1, "Bayer 8-bit RGGB" },
>  	{ XVIP_VF_MONO_SENSOR, 8, "grbg", MEDIA_BUS_FMT_SGRBG8_1X8,
> -	  {1, 0, 0}, V4L2_PIX_FMT_SGRBG8, 1, 1, 1, "Bayer 8-bit GRBG" },
> +	  {1, 0, 0}, V4L2_PIX_FMT_SGRBG8, 1, 1, 1, 1, 1, "Bayer 8-bit GRBG" },
>  	{ XVIP_VF_MONO_SENSOR, 8, "gbrg", MEDIA_BUS_FMT_SGBRG8_1X8,
> -	  {1, 0, 0}, V4L2_PIX_FMT_SGBRG8, 1, 1, 1, "Bayer 8-bit GBRG" },
> +	  {1, 0, 0}, V4L2_PIX_FMT_SGBRG8, 1, 1, 1, 1, 1, "Bayer 8-bit GBRG" },
>  	{ XVIP_VF_MONO_SENSOR, 8, "bggr", MEDIA_BUS_FMT_SBGGR8_1X8,
> -	  {1, 0, 0}, V4L2_PIX_FMT_SBGGR8, 1, 1, 1, "Bayer 8-bit BGGR" },
> +	  {1, 0, 0}, V4L2_PIX_FMT_SBGGR8, 1, 1, 1, 1, 1, "Bayer 8-bit BGGR" },
>  };
>  
>  /**
> @@ -102,6 +102,23 @@ const struct xvip_video_format *xvip_get_format_by_fourcc(u32 fourcc)
>  EXPORT_SYMBOL_GPL(xvip_get_format_by_fourcc);
>  
>  /**
> + * xvip_fmt_plane_width_bytes - bytes of the given width of the plane
> + * @info: VIP format description
> + * @width: width
> + *
> + * Return: Returns the number of bytes for given @width
> + */
> +int xvip_fmt_plane_width_bytes(const struct xvip_video_format *info, u32 width)
> +{
> +	if (!info)
> +		return 0;
> +
> +	return DIV_ROUND_UP(width * info->bytes_per_macropixel * info->bpp[0],

I don't see any bpp other than bpp[0] is used, so it may not have to be an array.
Then I am not sure how it models some formats with different bpp in other planes.
I'll take another close look.

Thanks,
-hyun

> +			    info->pixels_per_macropixel);
> +}
> +EXPORT_SYMBOL_GPL(xvip_fmt_plane_width_bytes);
> +
> +/**
>   * xvip_of_get_format - Parse a device tree node and return format information
>   * @node: the device tree node
>   *
> diff --git a/drivers/media/platform/xilinx/xilinx-vip.h b/drivers/media/platform/xilinx/xilinx-vip.h
> index 5e7a978..7c614d3 100644
> --- a/drivers/media/platform/xilinx/xilinx-vip.h
> +++ b/drivers/media/platform/xilinx/xilinx-vip.h
> @@ -114,6 +114,8 @@ struct xvip_device {
>   * @num_planes: number of planes w.r.t. color format
>   * @hsub: Horizontal sampling factor of Chroma
>   * @vsub: Vertical sampling factor of Chroma
> + * @bytes_per_macropixel: Number of bytes per macro-pixel
> + * @pixels_per_macropixel: Number of pixels per macro-pixel
>   * @description: format description, suitable for userspace
>   */
>  struct xvip_video_format {
> @@ -126,12 +128,15 @@ struct xvip_video_format {
>  	u8 num_planes;
>  	u8 hsub;
>  	u8 vsub;
> +	u32 bytes_per_macropixel;
> +	u32 pixels_per_macropixel;
>  	const char *description;
>  };
>  
>  const struct xvip_video_format *xvip_get_format_by_code(unsigned int code);
>  const struct xvip_video_format *xvip_get_format_by_fourcc(u32 fourcc);
>  const struct xvip_video_format *xvip_of_get_format(struct device_node *node);
> +int xvip_fmt_plane_width_bytes(const struct xvip_video_format *info, u32 width);
>  void xvip_set_format_size(struct v4l2_mbus_framefmt *format,
>  			  const struct v4l2_subdev_format *fmt);
>  int xvip_enum_mbus_code(struct v4l2_subdev *subdev,
> -- 
> 2.1.1
>