Add MPEG-2 decoding to Rockchip VPU

Add two new API helpers, v4l2_fill_pixfmt and v4l2_fill_pixfmt_mp, to be used by drivers to calculate plane sizes and bytes per lines.

Note that driver-specific padding and alignment are not taken into account, and must be done by drivers using this API.

Signed-off-by Ezequiel Garcia <ezequiel-ZGY8ohtN/[email protected]>

---
 drivers/media/v4l2-core/v4l2-common.c | 186 ++++++++++++++++++++++++++
 include/media/v4l2-common.h           |  32 +++++
 2 files changed, 218 insertions(+)

drivers/media/v4l2-core/v4l2-common.c +12 -0

index 663730f088cd..11a16bb3efda 100644
@@ -44,6 +44,7 @@
  * Added Gerd Knorrs v4l1 enhancements (Justin Schoeman)
  */
 
+#include <linux/ctype.h>
 #include <linux/module.h>
 #include <linux/types.h>
 #include <linux/kernel.h>

@@ -445,3 +446,188 @@ int v4l2_s_parm_cap(struct video_device *vdev,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(v4l2_s_parm_cap);
+
+static char printable_char(int c)
+{
+	return isascii(c) && isprint(c) ? c : '?';
+}
+
+const char *v4l2_get_fourcc_name(uint32_t format)
+{
+	static char buf[8];
+
+	snprintf(buf, 8,

drivers/media/v4l2-core/v4l2-common.c +164 -0

+
+const struct v4l2_format_info *v4l2_format_info(u32 format)
+{
+	static const struct v4l2_format_info formats[] = {
+		/* RGB formats */
+		{ .format = V4L2_PIX_FMT_BGR24,   .mem_planes = 1, .comp_planes = 1, .bpp = { 3, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_RGB24,   .mem_planes = 1, .comp_planes = 1, .bpp = { 3, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_HSV24,   .mem_planes = 1, .comp_planes = 1, .bpp = { 3, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_BGR32,   .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_XBGR32,  .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_RGB32,   .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_XRGB32,  .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_HSV32,   .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_ARGB32,  .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_ABGR32,  .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_GREY,    .mem_planes = 1, .comp_planes = 1, .bpp = { 1, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
+
+		/* YUV packed formats */
+		{ .format = V4L2_PIX_FMT_YUYV,    .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_YVYU,    .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_UYVY,    .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_VYUY,    .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 },
+
+		/* YUV planar formats */
+		{ .format = V4L2_PIX_FMT_NV12,    .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 },
+		{ .format = V4L2_PIX_FMT_NV21,    .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 },
+		{ .format = V4L2_PIX_FMT_NV16,    .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_NV61,    .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_NV24,    .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 1, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_NV42,    .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 1, .vdiv = 1 },
+
+		{ .format = V4L2_PIX_FMT_YUV410,  .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 4, .vdiv = 4 },
+		{ .format = V4L2_PIX_FMT_YVU410,  .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 4, .vdiv = 4 },
+		{ .format = V4L2_PIX_FMT_YUV411P, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 4, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_YUV420,  .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 },
+		{ .format = V4L2_PIX_FMT_YVU420,  .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 },
+		{ .format = V4L2_PIX_FMT_YUV422P, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 1 },
+
+		/* YUV planar formats, non contiguous variant */
+		{ .format = V4L2_PIX_FMT_YUV420M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 },
+		{ .format = V4L2_PIX_FMT_YVU420M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 },
+		{ .format = V4L2_PIX_FMT_YUV422M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_YVU422M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_YUV444M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 1, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_YVU444M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 1, .vdiv = 1 },
+
+		{ .format = V4L2_PIX_FMT_NV12M,   .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 },
+		{ .format = V4L2_PIX_FMT_NV21M,   .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 },
+		{ .format = V4L2_PIX_FMT_NV16M,   .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_NV61M,   .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 1 },
+	};
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(formats); ++i)
+		if (formats[i].format == format)
+			return &formats[i];
+	return NULL;
+}
+EXPORT_SYMBOL(v4l2_format_info);
+
+static inline unsigned int v4l2_format_block_width(const struct v4l2_format_info *info, int plane)
+{
+	if (!info->block_w[plane])
+		return 1;
+	return info->block_w[plane];
+}
+
+static inline unsigned int v4l2_format_block_height(const struct v4l2_format_info *info, int plane)
+{
+	if (!info->block_h[plane])
+		return 1;
+	return info->block_h[plane];
+}
+
+int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt,
+			 int pixelformat, int width, int height)
+{
+	const struct v4l2_format_info *info;
+	struct v4l2_plane_pix_format *plane;
+	int i;
+
+	info = v4l2_format_info(pixelformat);
+	if (!info)
+		return -EINVAL;
+
+	pixfmt->width = width;
+	pixfmt->height = height;
+	pixfmt->pixelformat = pixelformat;
+	pixfmt->num_planes = info->mem_planes;
+
+	if (info->mem_planes == 1) {
+		plane = &pixfmt->plane_fmt[0];
+		plane->bytesperline = ALIGN(width, v4l2_format_block_width(info, 0)) * info->bpp[0];
+		plane->sizeimage = 0;
+
+		for (i = 0; i < info->comp_planes; i++) {
+			unsigned int hdiv = (i == 0) ? 1 : info->hdiv;
+			unsigned int vdiv = (i == 0) ? 1 : info->vdiv;
+			unsigned int aligned_width;
+			unsigned int aligned_height;
+
+			aligned_width = ALIGN(width, v4l2_format_block_width(info, i));
+			aligned_height = ALIGN(height, v4l2_format_block_height(info, i));
+
+			plane->sizeimage += info->bpp[i] *
+				DIV_ROUND_UP(aligned_width, hdiv) *
+				DIV_ROUND_UP(aligned_height, vdiv);
+		}
+	} else {
+		for (i = 0; i < info->comp_planes; i++) {
+			unsigned int hdiv = (i == 0) ? 1 : info->hdiv;
+			unsigned int vdiv = (i == 0) ? 1 : info->vdiv;
+			unsigned int aligned_width;
+			unsigned int aligned_height;
+
+			aligned_width = ALIGN(width, v4l2_format_block_width(info, i));
+			aligned_height = ALIGN(height, v4l2_format_block_height(info, i));
+
+			plane = &pixfmt->plane_fmt[i];
+			plane->bytesperline =
+				info->bpp[i] * DIV_ROUND_UP(aligned_width, hdiv);
+			plane->sizeimage =
+				plane->bytesperline * DIV_ROUND_UP(aligned_height, vdiv);
+		}
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt_mp);
+
+int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, int pixelformat, int width, int height)
+{
+	const struct v4l2_format_info *info;
+	int i;
+
+	info = v4l2_format_info(pixelformat);
+	if (!info)
+		return -EINVAL;
+
+	/* Single planar API cannot be used for multi plane formats. */
+	if (info->mem_planes > 1)
+		return -EINVAL;
+
+	pixfmt->width = width;
+	pixfmt->height = height;
+	pixfmt->pixelformat = pixelformat;
+	pixfmt->bytesperline = ALIGN(width, v4l2_format_block_width(info, 0)) * info->bpp[0];
+	pixfmt->sizeimage = 0;
+
+	for (i = 0; i < info->comp_planes; i++) {
+		unsigned int hdiv = (i == 0) ? 1 : info->hdiv;
+		unsigned int vdiv = (i == 0) ? 1 : info->vdiv;
+		unsigned int aligned_width;
+		unsigned int aligned_height;
+
+		aligned_width = ALIGN(width, v4l2_format_block_width(info, i));
+		aligned_height = ALIGN(height, v4l2_format_block_height(info, i));
+
+		pixfmt->sizeimage += info->bpp[i] *
+			DIV_ROUND_UP(aligned_width, hdiv) *
+			DIV_ROUND_UP(aligned_height, vdiv);
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt);

include/media/v4l2-common.h +32 -0

index 2b93cb281fa5..937b74a946cd 100644
@@ -392,4 +392,36 @@ int v4l2_s_parm_cap(struct video_device *vdev,
 	((u64)(a).numerator * (b).denominator OP	\
 	(u64)(b).numerator * (a).denominator)
 
+/* ------------------------------------------------------------------------- */
+
+/* Pixel format and FourCC helpers */
+
+/**
+ * struct v4l2_format_info - information about a V4L2 format
+ * @format: 4CC format identifier (V4L2_PIX_FMT_*)
+ * @mem_planes: Number of memory planes, which includes the alpha plane (1 to 4).
+ * @comp_planes: Number of component planes, which includes the alpha plane (1 to 4).
+ * @bpp: Array of per-plane bytes per pixel
+ * @hdiv: Horizontal chroma subsampling factor
+ * @vdiv: Vertical chroma subsampling factor
+ */
+struct v4l2_format_info {
+	u32 format;
+	u8 mem_planes;
+	u8 comp_planes;
+	u8 bpp[4];
+	u8 hdiv;
+	u8 vdiv;
+	u8 block_w[4];
+	u8 block_h[4];
+};
+
+const struct v4l2_format_info *v4l2_format_info(u32 format);
+const char *v4l2_get_fourcc_name(u32 format);
+
+int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, int pixelformat,
+		     int width, int height);
+int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, int pixelformat,
+			int width, int height);
+
 #endif /* V4L2_COMMON_H_ */

Quoted 39 lines · Ezequiel Garcia

Add two new API helpers, v4l2_fill_pixfmt and v4l2_fill_pixfmt_mp, to be used by drivers to calculate plane sizes and bytes per lines.

Note that driver-specific padding and alignment are not taken into account, and must be done by drivers using this API.

Signed-off-by Ezequiel Garcia <ezequiel-ZGY8ohtN/[email protected]>

---
 drivers/media/v4l2-core/v4l2-common.c | 186 ++++++++++++++++++++++++++
 include/media/v4l2-common.h           |  32 +++++
 2 files changed, 218 insertions(+)

drivers/media/v4l2-core/v4l2-common.c +12 -0

index 663730f088cd..11a16bb3efda 100644
@@ -44,6 +44,7 @@
  * Added Gerd Knorrs v4l1 enhancements (Justin Schoeman)
  */
 
+#include <linux/ctype.h>
 #include <linux/module.h>
 #include <linux/types.h>
 #include <linux/kernel.h>

@@ -445,3 +446,188 @@ int v4l2_s_parm_cap(struct video_device *vdev,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(v4l2_s_parm_cap);
+
+static char printable_char(int c)
+{
+	return isascii(c) && isprint(c) ? c : '?';
+}
+
+const char *v4l2_get_fourcc_name(uint32_t format)
+{
+	static char buf[8];
+
+	snprintf(buf, 8,

drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 469

Quoted 10 lines

drivers/media/v4l2-core/v4l2-common.c +10 -0

+		 "%c%c%c%c%s",
+		 printable_char(format & 0xff),
+		 printable_char((format >> 8) & 0xff),
+		 printable_char((format >> 16) & 0xff),
+		 printable_char((format >> 24) & 0x7f),
+		 (format & BIT(31)) ? "-BE" : "");
+
+	return buf;
+}
+EXPORT_SYMBOL(v4l2_get_fourcc_name);

This function isn't re-entrant, but it should be. Multiple threads may be calling it at the same time.

It is probably best to pass the buffer pointer as an argument.

I would also prefer to split this patch into two: the first adding v4l2_format_info, the second adding v4l2_get_fourcc_name. This in case that the v4l2_get_fourcc_name() function needs more work.

include/media/v4l2-common.h +11 -0

+
+/* Pixel format and FourCC helpers */
+
+/**
+ * struct v4l2_format_info - information about a V4L2 format
+ * @format: 4CC format identifier (V4L2_PIX_FMT_*)
+ * @mem_planes: Number of memory planes, which includes the alpha plane (1 to 4).
+ * @comp_planes: Number of component planes, which includes the alpha plane (1 to 4).
+ * @bpp: Array of per-plane bytes per pixel
+ * @hdiv: Horizontal chroma subsampling factor
+ * @vdiv: Vertical chroma subsampling factor

In order to make the code more generic, introduce a pair of start/stop codec operations, and use them to allocate and release the JPEG bounce buffer.

Signed-off-by Ezequiel Garcia <ezequiel-ZGY8ohtN/[email protected]>

---
 .../media/rockchip/vpu/rk3288_vpu_hw.c        |  2 ++
 .../rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c     |  4 +--
 .../media/rockchip/vpu/rk3399_vpu_hw.c        |  2 ++
 .../rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c     |  4 +--
 .../staging/media/rockchip/vpu/rockchip_vpu.h | 12 ++++----
 .../media/rockchip/vpu/rockchip_vpu_drv.c     | 10 +++++--
 .../media/rockchip/vpu/rockchip_vpu_enc.c     | 23 +++++----------
 .../media/rockchip/vpu/rockchip_vpu_hw.h      | 28 +++++++++++++++++++
 .../media/rockchip/vpu/rockchip_vpu_jpeg.c    | 25 +++++++++++++++++
 9 files changed, 81 insertions(+), 29 deletions(-)

drivers/staging/media/rockchip/vpu/rockchip_vpu.h +1 -4

index 1ec2be483e27..76ee24abc141 100644
@@ -124,10 +124,7 @@ struct rockchip_vpu_dev {
  * @jpeg_quality:      User-specified JPEG compression quality.
  *
  * @codec_ops:         Set of operations related to codec mode.
- *
- * @bounce_dma_addr:   Bounce buffer bus address.
- * @bounce_buf:                Bounce buffer pointer.
- * @bounce_size:       Bounce buffer size.
+ * @jpeg_enc_ctx:      JPEG-encoding context.
  */
 struct rockchip_vpu_ctx {
        struct rockchip_vpu_dev *dev;

drivers/staging/media/rockchip/vpu/rockchip_vpu.h +0 -0

 };
 
 /**

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +0 -0

index 5647b0bdac20..1a6dd36c71ab 100644

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +3 -1

+                */
+               if (ctx->jpeg_enc_ctx.bounce_buffer.cpu) {
                        memcpy(vb2_plane_vaddr(&dst->vb2_buf, 0) +
                               ctx->vpu_dst_fmt->header_size,
-                              ctx->bounce_buf, bytesused);
+                              ctx->jpeg_enc_ctx.bounce_buffer.cpu, bytesused);
                }
                dst->vb2_buf.planes[0].bytesused =
                        ctx->vpu_dst_fmt->header_size + bytesused;

/**
* struct rockchip_vpu_codec_ops - codec mode specific operations *

+ * @start:     If needed, can be used for initialization.
+ *             Optional and called from process context.
+ * @stop:      If needed, can be used to undo the .start phase.
+ *             Optional and called from process context.

Hi Ezequiel,

Quoted 5 lines · Ezequiel Garcia

In order to make the code more generic, introduce a pair of start/stop codec operations, and use them to allocate and release the JPEG bounce buffer.

Signed-off-by Ezequiel Garcia <ezequiel-ZGY8ohtN/[email protected]>

Quoted 12 lines

---
 .../media/rockchip/vpu/rk3288_vpu_hw.c        |  2 ++
 .../rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c     |  4 +--
 .../media/rockchip/vpu/rk3399_vpu_hw.c        |  2 ++
 .../rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c     |  4 +--
 .../staging/media/rockchip/vpu/rockchip_vpu.h | 12 ++++----
 .../media/rockchip/vpu/rockchip_vpu_drv.c     | 10 +++++--
 .../media/rockchip/vpu/rockchip_vpu_enc.c     | 23 +++++----------
 .../media/rockchip/vpu/rockchip_vpu_hw.h      | 28 +++++++++++++++++++
 .../media/rockchip/vpu/rockchip_vpu_jpeg.c    | 25 +++++++++++++++++
 9 files changed, 81 insertions(+), 29 deletions(-)

Thanks for the series! Really sorry for late reply...

[snip]

Quoted 20 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu.h +1 -4

index 1ec2be483e27..76ee24abc141 100644
@@ -124,10 +124,7 @@ struct rockchip_vpu_dev {
  * @jpeg_quality:      User-specified JPEG compression quality.
  *
  * @codec_ops:         Set of operations related to codec mode.
- *
- * @bounce_dma_addr:   Bounce buffer bus address.
- * @bounce_buf:                Bounce buffer pointer.
- * @bounce_size:       Bounce buffer size.
+ * @jpeg_enc_ctx:      JPEG-encoding context.
  */
 struct rockchip_vpu_ctx {
        struct rockchip_vpu_dev *dev;

@@ -146,9 +143,10 @@ struct rockchip_vpu_ctx {
 
        const struct rockchip_vpu_codec_ops *codec_ops;
 

drivers/staging/media/rockchip/vpu/rockchip_vpu.h struct rockchip_vpu_ctx { · line 149

Quoted 7 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu.h +4 -3

-       dma_addr_t bounce_dma_addr;
-       void *bounce_buf;
-       size_t bounce_size;
+       /* Specific for particular codec modes. */
+       union {
+               struct rockchip_vpu_jpeg_enc_hw_ctx jpeg_enc_ctx;
+       };

nit: Would it perhaps make it a bit cleaner to call the union "ctx"? Then we wouldn't need to repeat "_ctx" in every field.

Quoted 12 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu.h +0 -0

 };
 
 /**

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +0 -1

index 5647b0bdac20..1a6dd36c71ab 100644
@@ -64,10 +64,16 @@ static void rockchip_vpu_job_finish(struct rockchip_vpu_dev *vpu,
        avail_size = vb2_plane_size(&dst->vb2_buf, 0) -
                     ctx->vpu_dst_fmt->header_size;
        if (bytesused <= avail_size) {
-               if (ctx->bounce_buf) {

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c static void rockchip_vpu_job_finish(struct rockchip_vpu_dev *vpu, · line 71

Quoted 5 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +5 -0

+               /*
+                * This works while JPEG is the only encoder this driver
+                * supports. We will have to abstract this step, or get
+                * rid of the bounce buffer before we can support
+                * encoding other codecs.

Hmm, why wouldn't it work for other encoders, which shouldn't require a bounce buffer?

Quoted 9 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +3 -1

+                */
+               if (ctx->jpeg_enc_ctx.bounce_buffer.cpu) {
                        memcpy(vb2_plane_vaddr(&dst->vb2_buf, 0) +
                               ctx->vpu_dst_fmt->header_size,
-                              ctx->bounce_buf, bytesused);
+                              ctx->jpeg_enc_ctx.bounce_buffer.cpu, bytesused);
                }
                dst->vb2_buf.planes[0].bytesused =
                        ctx->vpu_dst_fmt->header_size + bytesused;

[snip]

Quoted 7 lines

/**
* struct rockchip_vpu_codec_ops - codec mode specific operations *

+ * @start:     If needed, can be used for initialization.
+ *             Optional and called from process context.
+ * @stop:      If needed, can be used to undo the .start phase.
+ *             Optional and called from process context.

nit: Perhaps .init and .exit could reflect better what these ops do?

Quoted 45 lines · Tomasz Figa

Hi Ezequiel,

Quoted 5 lines · Ezequiel Garcia

In order to make the code more generic, introduce a pair of start/stop codec operations, and use them to allocate and release the JPEG bounce buffer.

Signed-off-by Ezequiel Garcia <ezequiel-ZGY8ohtN/[email protected]>

Quoted 12 lines

---
 .../media/rockchip/vpu/rk3288_vpu_hw.c        |  2 ++
 .../rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c     |  4 +--
 .../media/rockchip/vpu/rk3399_vpu_hw.c        |  2 ++
 .../rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c     |  4 +--
 .../staging/media/rockchip/vpu/rockchip_vpu.h | 12 ++++----
 .../media/rockchip/vpu/rockchip_vpu_drv.c     | 10 +++++--
 .../media/rockchip/vpu/rockchip_vpu_enc.c     | 23 +++++----------
 .../media/rockchip/vpu/rockchip_vpu_hw.h      | 28 +++++++++++++++++++
 .../media/rockchip/vpu/rockchip_vpu_jpeg.c    | 25 +++++++++++++++++
 9 files changed, 81 insertions(+), 29 deletions(-)

Thanks for the series! Really sorry for late reply...

[snip]

Quoted 20 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu.h +1 -4

index 1ec2be483e27..76ee24abc141 100644
@@ -124,10 +124,7 @@ struct rockchip_vpu_dev {
  * @jpeg_quality:      User-specified JPEG compression quality.
  *
  * @codec_ops:         Set of operations related to codec mode.
- *
- * @bounce_dma_addr:   Bounce buffer bus address.
- * @bounce_buf:                Bounce buffer pointer.
- * @bounce_size:       Bounce buffer size.
+ * @jpeg_enc_ctx:      JPEG-encoding context.
  */
 struct rockchip_vpu_ctx {
        struct rockchip_vpu_dev *dev;

@@ -146,9 +143,10 @@ struct rockchip_vpu_ctx {
 
        const struct rockchip_vpu_codec_ops *codec_ops;
 

Quoted 11 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu.h struct rockchip_vpu_ctx { · line 149

Quoted 7 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu.h +4 -3

-       dma_addr_t bounce_dma_addr;
-       void *bounce_buf;
-       size_t bounce_size;
+       /* Specific for particular codec modes. */
+       union {
+               struct rockchip_vpu_jpeg_enc_hw_ctx jpeg_enc_ctx;
+       };

nit: Would it perhaps make it a bit cleaner to call the union "ctx"? Then we wouldn't need to repeat "_ctx" in every field.

Well, the struct is already rockchip_vpu_ctx, so access would look like:

   ctx->ctx.jpeg_enc, ctx->ctx.mpeg2_dec

Perhaps we can just drop the _ctx, and so:

   ctx->jpeg_enc, ctx->mpeg2_dec

Quoted 12 lines

Quoted 12 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu.h +0 -0

 };
 
 /**

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +0 -1

index 5647b0bdac20..1a6dd36c71ab 100644
@@ -64,10 +64,16 @@ static void rockchip_vpu_job_finish(struct rockchip_vpu_dev *vpu,
        avail_size = vb2_plane_size(&dst->vb2_buf, 0) -
                     ctx->vpu_dst_fmt->header_size;
        if (bytesused <= avail_size) {
-               if (ctx->bounce_buf) {

Quoted 9 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c static void rockchip_vpu_job_finish(struct rockchip_vpu_dev *vpu, · line 71

Quoted 5 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +5 -0

+               /*
+                * This works while JPEG is the only encoder this driver
+                * supports. We will have to abstract this step, or get
+                * rid of the bounce buffer before we can support
+                * encoding other codecs.

Hmm, why wouldn't it work for other encoders, which shouldn't require a bounce buffer?

Well, I had a TODO item to remove the bounce buffer for JPEG.

IIRC, the ChromeOS VPU driver doesn't have any bounce buffer, so I was under the impression we wouldn't need one.

However, we have yet to discuss the encoder implementation. We might decide to produce the headers for some video coded format, in which case we might need a bounce buffer.

In any case, this is just a comment to reflect that this piece of code is just temporary.

In preparation to support decoders, using a single memory-to-memory device, we need to roll our own media controller entities registration.

Signed-off-by Ezequiel Garcia <ezequiel-ZGY8ohtN/[email protected]>

---
 .../staging/media/rockchip/vpu/rockchip_vpu.h |  35 ++++
 .../media/rockchip/vpu/rockchip_vpu_drv.c     | 181 ++++++++++++++++--
 2 files changed, 204 insertions(+), 12 deletions(-)

drivers/staging/media/rockchip/vpu/rockchip_vpu.h +0 -0

index 76ee24abc141..084f58cadda1 100644

drivers/staging/media/rockchip/vpu/rockchip_vpu.h +26 -0

+ * @source_pad:                &struct media_pad with the source pad.
+ *                     Used only when the M2M device is registered via
+ *                     v4l2_m2m_unregister_media_controller().
+ * @sink:              &struct media_entity pointer with the sink entity
+ *                     Used only when the M2M device is registered via
+ *                     v4l2_m2m_unregister_media_controller().
+ * @sink_pad:          &struct media_pad with the sink pad.
+ *                     Used only when the M2M device is registered via
+ *                     v4l2_m2m_unregister_media_controller().
+ * @proc:              &struct media_entity pointer with the M2M device itself.
+ * @proc_pads:         &struct media_pad with the @proc pads.
+ *                     Used only when the M2M device is registered via
+ *                     v4l2_m2m_unregister_media_controller().
+ * @intf_devnode:      &struct media_intf devnode pointer with the interface
+ *                     with controls the M2M device.
+ */
+struct rockchip_vpu_mc {
+       struct media_entity     *source;
+       struct media_pad        source_pad;
+       struct media_entity     sink;
+       struct media_pad        sink_pad;
+       struct media_entity     proc;
+       struct media_pad        proc_pads[2];
+       struct media_intf_devnode *intf_devnode;
+};
+
 /**
  * struct rockchip_vpu_dev - driver data
  * @v4l2_dev:          V4L2 device to register video devices for.

drivers/staging/media/rockchip/vpu/rockchip_vpu.h +2 -0

+ *                     for encoder and decoder.
  * @dev:               Pointer to device for convenient logging using
  *                     dev_ macros.
  * @clocks:            Array of clock handles.

@@ -95,6 +129,7 @@ struct rockchip_vpu_dev {
        struct media_device mdev;
        struct video_device *vfd_enc;
        struct platform_device *pdev;
+       struct rockchip_vpu_mc mc[2];
        struct device *dev;
        struct clk_bulk_data clocks[ROCKCHIP_VPU_MAX_CLOCKS];
        void __iomem *base;

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +10 -10

index 1a6dd36c71ab..af2481ca2228 100644
@@ -332,7 +332,7 @@ static int rockchip_vpu_video_device_register(struct rockchip_vpu_dev *vpu)
 {
        const struct of_device_id *match;
        struct video_device *vfd;
-       int function, ret;
+       int ret;
 
        match = of_match_node(of_rockchip_vpu_match, vpu->dev->of_node);
        vfd = video_device_alloc();

@@ -359,21 +359,169 @@ static int rockchip_vpu_video_device_register(struct rockchip_vpu_dev *vpu)
        }
        v4l2_info(&vpu->v4l2_dev, "registered as /dev/video%d\n", vfd->num);
 
-       function = MEDIA_ENT_F_PROC_VIDEO_ENCODER;
-       ret = v4l2_m2m_register_media_controller(vpu->m2m_dev, vfd, function);
-       if (ret) {
-               v4l2_err(&vpu->v4l2_dev, "Failed to init mem2mem media controller\n");
-               goto err_unreg_video;
-       }
        return 0;
-
-err_unreg_video:
-       video_unregister_device(vfd);
 err_free_dev:
        video_device_release(vfd);
        return ret;
 }
 
+static int rockchip_vpu_register_entity(struct media_device *mdev,
+       struct media_entity *entity, const char *entity_name,
+       struct media_pad *pads, int num_pads, int function,
+       struct video_device *vdev)
+{
+       unsigned int len;
+       char *name;
+       int ret;
+

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +63 -0

+
+       return 0;
+}
+
+static int rockchip_register_mc(struct media_device *mdev,
+                               struct rockchip_vpu_mc *mc,
+                               struct video_device *vdev,
+                               int function)
+{
+       struct media_link *link;
+       int ret;
+
+       /* Create the three encoder entities with their pads */
+       mc->source = &vdev->entity;
+       mc->source_pad.flags = MEDIA_PAD_FL_SOURCE;
+       ret = rockchip_vpu_register_entity(mdev, mc->source,
+                       "source", &mc->source_pad, 1, MEDIA_ENT_F_IO_V4L, vdev);
+       if (ret)
+               return ret;
+
+       mc->proc_pads[0].flags = MEDIA_PAD_FL_SINK;
+       mc->proc_pads[1].flags = MEDIA_PAD_FL_SOURCE;
+       ret = rockchip_vpu_register_entity(mdev, &mc->proc,
+                       "proc", mc->proc_pads, 2, function, vdev);
+       if (ret)
+               goto err_rel_entity0;
+
+       mc->sink_pad.flags = MEDIA_PAD_FL_SINK;
+       ret = rockchip_vpu_register_entity(mdev, &mc->sink,
+                       "sink", &mc->sink_pad, 1, MEDIA_ENT_F_IO_V4L, vdev);
+       if (ret)
+               goto err_rel_entity1;
+
+       /* Connect the three entities */
+       ret = media_create_pad_link(mc->source, 0, &mc->proc, 1,
+                       MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
+       if (ret)
+               goto err_rel_entity2;
+
+       ret = media_create_pad_link(&mc->proc, 0, &mc->sink, 0,
+                       MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
+       if (ret)
+               goto err_rm_links0;
+
+       /* Create video interface */
+       mc->intf_devnode = media_devnode_create(mdev,
+                       MEDIA_INTF_T_V4L_VIDEO, 0,
+                       VIDEO_MAJOR, vdev->minor);
+       if (!mc->intf_devnode) {
+               ret = -ENOMEM;
+               goto err_rm_links1;
+       }
+
+       /* Connect the two DMA engines to the interface */
+       link = media_create_intf_link(mc->source,
+                       &mc->intf_devnode->intf,
+                       MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
+       if (!link) {
+               ret = -ENOMEM;
+               goto err_rm_devnode;
+       }
+
+       link = media_create_intf_link(&mc->sink,

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +29 -0

+err_rm_devnode:
+       media_devnode_remove(mc->intf_devnode);
+err_rm_links1:
+       media_entity_remove_links(&mc->sink);
+err_rm_links0:
+       media_entity_remove_links(&mc->proc);
+       media_entity_remove_links(mc->source);
+err_rel_entity2:
+       media_device_unregister_entity(&mc->proc);
+       kfree(mc->proc.name);
+err_rel_entity1:
+       media_device_unregister_entity(&mc->sink);
+       kfree(mc->sink.name);
+err_rel_entity0:
+       media_device_unregister_entity(mc->source);
+       kfree(mc->source->name);
+       return ret;
+}
+
+static void rockchip_unregister_mc(struct rockchip_vpu_mc *mc)
+{
+       media_remove_intf_links(&mc->intf_devnode->intf);
+       media_devnode_remove(mc->intf_devnode);
+       media_entity_remove_links(mc->source);
+       media_entity_remove_links(&mc->sink);
+       media_entity_remove_links(&mc->proc);
+       media_device_unregister_entity(mc->source);
+       media_device_unregister_entity(&mc->sink);
+       media_device_unregister_entity(&mc->proc);

Quoted 14 lines · Ezequiel Garcia

In preparation to support decoders, using a single memory-to-memory device, we need to roll our own media controller entities registration.

Signed-off-by Ezequiel Garcia <ezequiel-ZGY8ohtN/[email protected]>

---
 .../staging/media/rockchip/vpu/rockchip_vpu.h |  35 ++++
 .../media/rockchip/vpu/rockchip_vpu_drv.c     | 181 ++++++++++++++++--
 2 files changed, 204 insertions(+), 12 deletions(-)

drivers/staging/media/rockchip/vpu/rockchip_vpu.h +0 -0

index 76ee24abc141..084f58cadda1 100644

drivers/staging/media/rockchip/vpu/rockchip_vpu.h enum rockchip_vpu_codec_mode { · line 79

Quoted 9 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu.h +6 -0

@@ -71,6 +71,38 @@ enum rockchip_vpu_codec_mode {
        RK_VPU_MODE_JPEG_ENC,
 };
 
+/*
+ * struct rockchip_vpu_mc - media controller data
+ *
+ * @source:            &struct media_entity pointer with the source entity
+ *                     Used only when the M2M device is registered via
+ *                     v4l2_m2m_unregister_media_controller().

_register? (and few other places below)

But I'm really confused, because this patch exactly removes the uses of v4l2_m2m_(un)register_media_controller() in this driver.

Quoted 28 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu.h +26 -0

+ * @source_pad:                &struct media_pad with the source pad.
+ *                     Used only when the M2M device is registered via
+ *                     v4l2_m2m_unregister_media_controller().
+ * @sink:              &struct media_entity pointer with the sink entity
+ *                     Used only when the M2M device is registered via
+ *                     v4l2_m2m_unregister_media_controller().
+ * @sink_pad:          &struct media_pad with the sink pad.
+ *                     Used only when the M2M device is registered via
+ *                     v4l2_m2m_unregister_media_controller().
+ * @proc:              &struct media_entity pointer with the M2M device itself.
+ * @proc_pads:         &struct media_pad with the @proc pads.
+ *                     Used only when the M2M device is registered via
+ *                     v4l2_m2m_unregister_media_controller().
+ * @intf_devnode:      &struct media_intf devnode pointer with the interface
+ *                     with controls the M2M device.
+ */
+struct rockchip_vpu_mc {
+       struct media_entity     *source;
+       struct media_pad        source_pad;
+       struct media_entity     sink;
+       struct media_pad        sink_pad;
+       struct media_entity     proc;
+       struct media_pad        proc_pads[2];
+       struct media_intf_devnode *intf_devnode;
+};
+
 /**
  * struct rockchip_vpu_dev - driver data

drivers/staging/media/rockchip/vpu/rockchip_vpu.h enum rockchip_vpu_codec_mode { · line 113

Quoted 6 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu.h +1 -0

  * @v4l2_dev:          V4L2 device to register video devices for.

@@ -78,6 +110,8 @@ enum rockchip_vpu_codec_mode {
  * @mdev:              media device associated to this device.
  * @vfd_enc:           Video device for encoder.
  * @pdev:              Pointer to VPU platform device.
+ * @mc:                        Array of media controller topology structs

Is it just me or there is something wrong with indentation here?

Quoted 57 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu.h +2 -0

+ *                     for encoder and decoder.
  * @dev:               Pointer to device for convenient logging using
  *                     dev_ macros.
  * @clocks:            Array of clock handles.

@@ -95,6 +129,7 @@ struct rockchip_vpu_dev {
        struct media_device mdev;
        struct video_device *vfd_enc;
        struct platform_device *pdev;
+       struct rockchip_vpu_mc mc[2];
        struct device *dev;
        struct clk_bulk_data clocks[ROCKCHIP_VPU_MAX_CLOCKS];
        void __iomem *base;

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +14 -10

index 1a6dd36c71ab..af2481ca2228 100644
@@ -332,7 +332,7 @@ static int rockchip_vpu_video_device_register(struct rockchip_vpu_dev *vpu)
 {
        const struct of_device_id *match;
        struct video_device *vfd;
-       int function, ret;
+       int ret;
 
        match = of_match_node(of_rockchip_vpu_match, vpu->dev->of_node);
        vfd = video_device_alloc();

@@ -359,21 +359,169 @@ static int rockchip_vpu_video_device_register(struct rockchip_vpu_dev *vpu)
        }
        v4l2_info(&vpu->v4l2_dev, "registered as /dev/video%d\n", vfd->num);
 
-       function = MEDIA_ENT_F_PROC_VIDEO_ENCODER;
-       ret = v4l2_m2m_register_media_controller(vpu->m2m_dev, vfd, function);
-       if (ret) {
-               v4l2_err(&vpu->v4l2_dev, "Failed to init mem2mem media controller\n");
-               goto err_unreg_video;
-       }
        return 0;
-
-err_unreg_video:
-       video_unregister_device(vfd);
 err_free_dev:
        video_device_release(vfd);
        return ret;
 }
 
+static int rockchip_vpu_register_entity(struct media_device *mdev,
+       struct media_entity *entity, const char *entity_name,
+       struct media_pad *pads, int num_pads, int function,
+       struct video_device *vdev)
+{
+       unsigned int len;
+       char *name;
+       int ret;
+
+       entity->obj_type = MEDIA_ENTITY_TYPE_BASE;
+       if (function == MEDIA_ENT_F_IO_V4L) {
+               entity->info.dev.major = VIDEO_MAJOR;
+               entity->info.dev.minor = vdev->minor;

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c static int rockchip_vpu_video_device_register(struct rockchip_vpu_dev *vpu) · line 386

Quoted 6 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +6 -0

+       }
+       len = strlen(vdev->name) + 2 + strlen(entity_name);
+       name = kmalloc(len, GFP_KERNEL);
+       if (!name)
+               return -ENOMEM;
+       snprintf(name, len, "%s-%s", vdev->name, entity_name);

How about using kasprintf()?

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c static int rockchip_vpu_video_device_register(struct rockchip_vpu_dev *vpu) · line 395

Quoted 9 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +9 -0

+       entity->name = name;
+       entity->function = function;
+
+       ret = media_entity_pads_init(entity, num_pads, pads);
+       if (ret)
+               return ret;
+       ret = media_device_register_entity(mdev, entity);
+       if (ret)
+               return ret;

How about the memory allocated for entity->name?

Quoted 68 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +68 -0

+
+       return 0;
+}
+
+static int rockchip_register_mc(struct media_device *mdev,
+                               struct rockchip_vpu_mc *mc,
+                               struct video_device *vdev,
+                               int function)
+{
+       struct media_link *link;
+       int ret;
+
+       /* Create the three encoder entities with their pads */
+       mc->source = &vdev->entity;
+       mc->source_pad.flags = MEDIA_PAD_FL_SOURCE;
+       ret = rockchip_vpu_register_entity(mdev, mc->source,
+                       "source", &mc->source_pad, 1, MEDIA_ENT_F_IO_V4L, vdev);
+       if (ret)
+               return ret;
+
+       mc->proc_pads[0].flags = MEDIA_PAD_FL_SINK;
+       mc->proc_pads[1].flags = MEDIA_PAD_FL_SOURCE;
+       ret = rockchip_vpu_register_entity(mdev, &mc->proc,
+                       "proc", mc->proc_pads, 2, function, vdev);
+       if (ret)
+               goto err_rel_entity0;
+
+       mc->sink_pad.flags = MEDIA_PAD_FL_SINK;
+       ret = rockchip_vpu_register_entity(mdev, &mc->sink,
+                       "sink", &mc->sink_pad, 1, MEDIA_ENT_F_IO_V4L, vdev);
+       if (ret)
+               goto err_rel_entity1;
+
+       /* Connect the three entities */
+       ret = media_create_pad_link(mc->source, 0, &mc->proc, 1,
+                       MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
+       if (ret)
+               goto err_rel_entity2;
+
+       ret = media_create_pad_link(&mc->proc, 0, &mc->sink, 0,
+                       MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
+       if (ret)
+               goto err_rm_links0;
+
+       /* Create video interface */
+       mc->intf_devnode = media_devnode_create(mdev,
+                       MEDIA_INTF_T_V4L_VIDEO, 0,
+                       VIDEO_MAJOR, vdev->minor);
+       if (!mc->intf_devnode) {
+               ret = -ENOMEM;
+               goto err_rm_links1;
+       }
+
+       /* Connect the two DMA engines to the interface */
+       link = media_create_intf_link(mc->source,
+                       &mc->intf_devnode->intf,
+                       MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
+       if (!link) {
+               ret = -ENOMEM;
+               goto err_rm_devnode;
+       }
+
+       link = media_create_intf_link(&mc->sink,
+                       &mc->intf_devnode->intf,
+                       MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
+       if (!link) {
+               ret = -ENOMEM;
+               goto err_rm_intf_link;

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c static int rockchip_vpu_video_device_register(struct rockchip_vpu_dev *vpu) · line 468

Quoted 5 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +5 -0

+       }
+       return 0;
+
+err_rm_intf_link:
+       media_remove_intf_links(&mc->intf_devnode->intf);

Do we need to explicitly remove the links here? The entity removal functions remove the links implicitly.

Quoted 29 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +29 -0

+err_rm_devnode:
+       media_devnode_remove(mc->intf_devnode);
+err_rm_links1:
+       media_entity_remove_links(&mc->sink);
+err_rm_links0:
+       media_entity_remove_links(&mc->proc);
+       media_entity_remove_links(mc->source);
+err_rel_entity2:
+       media_device_unregister_entity(&mc->proc);
+       kfree(mc->proc.name);
+err_rel_entity1:
+       media_device_unregister_entity(&mc->sink);
+       kfree(mc->sink.name);
+err_rel_entity0:
+       media_device_unregister_entity(mc->source);
+       kfree(mc->source->name);
+       return ret;
+}
+
+static void rockchip_unregister_mc(struct rockchip_vpu_mc *mc)
+{
+       media_remove_intf_links(&mc->intf_devnode->intf);
+       media_devnode_remove(mc->intf_devnode);
+       media_entity_remove_links(mc->source);
+       media_entity_remove_links(&mc->sink);
+       media_entity_remove_links(&mc->proc);
+       media_device_unregister_entity(mc->source);
+       media_device_unregister_entity(&mc->sink);
+       media_device_unregister_entity(&mc->proc);

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c static int rockchip_vpu_video_device_register(struct rockchip_vpu_dev *vpu) · line 507

Quoted 10 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +10 -0

+       kfree(mc->source->name);
+       kfree(mc->sink.name);
+       kfree(mc->proc.name);
+}
+
+static int rockchip_register_media_controller(struct rockchip_vpu_dev *vpu)
+{
+       int ret;
+
+       /* We have one memory-to-memory device, to hold a single queue

Coding style:

/*
* We have one [...]

Quoted 1 line · Tomasz Figa

On Tue, Mar 5, 2019 at 4:27 AM Ezequiel Garcia <ezequiel-ZGY8ohtN/[email protected]> wrote:

Quoted 28 lines

Quoted 28 lines

+ * @source_pad: &struct media_pad with the source pad.

+ *                     Used only when the M2M device is registered via
+ *                     v4l2_m2m_unregister_media_controller().
+ * @sink:              &struct media_entity pointer with the sink entity
+ *                     Used only when the M2M device is registered via
+ *                     v4l2_m2m_unregister_media_controller().
+ * @sink_pad:          &struct media_pad with the sink pad.
+ *                     Used only when the M2M device is registered via
+ *                     v4l2_m2m_unregister_media_controller().
+ * @proc:              &struct media_entity pointer with the M2M device itself.
+ * @proc_pads:         &struct media_pad with the @proc pads.
+ *                     Used only when the M2M device is registered via
+ *                     v4l2_m2m_unregister_media_controller().
+ * @intf_devnode:      &struct media_intf devnode pointer with the interface
+ *                     with controls the M2M device.

+ */

drivers/staging/media/rockchip/vpu/rockchip_vpu.h +10 -0

+struct rockchip_vpu_mc {
+       struct media_entity     *source;
+       struct media_pad        source_pad;
+       struct media_entity     sink;
+       struct media_pad        sink_pad;
+       struct media_entity     proc;
+       struct media_pad        proc_pads[2];
+       struct media_intf_devnode *intf_devnode;
+};
+
 /**
  * struct rockchip_vpu_dev - driver data

Quoted 9 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu.h enum rockchip_vpu_codec_mode { · line 113

Quoted 6 lines

* @v4l2_dev: V4L2 device to register video devices for.

drivers/staging/media/rockchip/vpu/rockchip_vpu.h +1 -0

@@ -78,6 +110,8 @@ enum rockchip_vpu_codec_mode {
  * @mdev:              media device associated to this device.
  * @vfd_enc:           Video device for encoder.
  * @pdev:              Pointer to VPU platform device.
+ * @mc:                        Array of media controller topology structs

Is it just me or there is something wrong with indentation here?

It seems to be fine here -- it's all tabs.

Quoted 68 lines

Quoted 68 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +68 -0

+
+       return 0;
+}
+
+static int rockchip_register_mc(struct media_device *mdev,
+                               struct rockchip_vpu_mc *mc,
+                               struct video_device *vdev,
+                               int function)
+{
+       struct media_link *link;
+       int ret;
+
+       /* Create the three encoder entities with their pads */
+       mc->source = &vdev->entity;
+       mc->source_pad.flags = MEDIA_PAD_FL_SOURCE;
+       ret = rockchip_vpu_register_entity(mdev, mc->source,
+                       "source", &mc->source_pad, 1, MEDIA_ENT_F_IO_V4L, vdev);
+       if (ret)
+               return ret;
+
+       mc->proc_pads[0].flags = MEDIA_PAD_FL_SINK;
+       mc->proc_pads[1].flags = MEDIA_PAD_FL_SOURCE;
+       ret = rockchip_vpu_register_entity(mdev, &mc->proc,
+                       "proc", mc->proc_pads, 2, function, vdev);
+       if (ret)
+               goto err_rel_entity0;
+
+       mc->sink_pad.flags = MEDIA_PAD_FL_SINK;
+       ret = rockchip_vpu_register_entity(mdev, &mc->sink,
+                       "sink", &mc->sink_pad, 1, MEDIA_ENT_F_IO_V4L, vdev);
+       if (ret)
+               goto err_rel_entity1;
+
+       /* Connect the three entities */
+       ret = media_create_pad_link(mc->source, 0, &mc->proc, 1,
+                       MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
+       if (ret)
+               goto err_rel_entity2;
+
+       ret = media_create_pad_link(&mc->proc, 0, &mc->sink, 0,
+                       MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
+       if (ret)
+               goto err_rm_links0;
+
+       /* Create video interface */
+       mc->intf_devnode = media_devnode_create(mdev,
+                       MEDIA_INTF_T_V4L_VIDEO, 0,
+                       VIDEO_MAJOR, vdev->minor);
+       if (!mc->intf_devnode) {
+               ret = -ENOMEM;
+               goto err_rm_links1;
+       }
+
+       /* Connect the two DMA engines to the interface */
+       link = media_create_intf_link(mc->source,
+                       &mc->intf_devnode->intf,
+                       MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
+       if (!link) {
+               ret = -ENOMEM;
+               goto err_rm_devnode;
+       }
+
+       link = media_create_intf_link(&mc->sink,
+                       &mc->intf_devnode->intf,
+                       MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
+       if (!link) {
+               ret = -ENOMEM;
+               goto err_rm_intf_link;

Quoted 9 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c static int rockchip_vpu_video_device_register(struct rockchip_vpu_dev *vpu) · line 468

Quoted 5 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +5 -0

+       }
+       return 0;
+
+err_rm_intf_link:
+       media_remove_intf_links(&mc->intf_devnode->intf);

Do we need to explicitly remove the links here? The entity removal functions remove the links implicitly.

You mean the media_devnode_remove, right? In that case, seems you are right.

Introduce support for the Request API. Although the JPEG encoder does not mandate using the Request API, it's perfectly possible to use it, if the application wants to.

In addition, add helpers that will be used by video decoders.

Signed-off-by Ezequiel Garcia <ezequiel-ZGY8ohtN/[email protected]>

---
 .../rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c     |  6 ++++
 .../media/rockchip/vpu/rockchip_vpu_common.h  |  3 ++
 .../media/rockchip/vpu/rockchip_vpu_drv.c     | 29 ++++++++++++++++++-
 .../media/rockchip/vpu/rockchip_vpu_enc.c     | 18 ++++++++++++
 4 files changed, 55 insertions(+), 1 deletion(-)

drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c +6 -0

index fe92c40d0a84..e11ee152b8ce 100644
@@ -113,11 +113,15 @@ void rk3399_vpu_jpeg_enc_run(struct rockchip_vpu_ctx *ctx)
        struct rockchip_vpu_dev *vpu = ctx->dev;
        struct vb2_v4l2_buffer *src_buf, *dst_buf;
        struct rockchip_vpu_jpeg_ctx jpeg_ctx;
+       struct media_request *src_req;
        u32 reg;
 
        src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
        dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
 
+       src_req = src_buf->vb2_buf.req_obj.req;
+       v4l2_ctrl_request_setup(src_req, &ctx->ctrl_handler);
+
        memset(&jpeg_ctx, 0, sizeof(jpeg_ctx));
        jpeg_ctx.buffer = vb2_plane_vaddr(&dst_buf->vb2_buf, 0);
        jpeg_ctx.width = ctx->dst_fmt.width;

@@ -153,6 +157,8 @@ void rk3399_vpu_jpeg_enc_run(struct rockchip_vpu_ctx *ctx)
                | VEPU_REG_ENCODE_FORMAT_JPEG
                | VEPU_REG_ENCODE_ENABLE;
 
+       v4l2_ctrl_request_complete(src_req, &ctx->ctrl_handler);
+
        /* Kick the watchdog and start encoding */
        schedule_delayed_work(&vpu->watchdog_work, msecs_to_jiffies(2000));
        vepu_write(vpu, reg, VEPU_REG_ENCODE_START);

drivers/staging/media/rockchip/vpu/rockchip_vpu_common.h +3 -0

index ca77668d9579..ac018136a7bc 100644
@@ -26,4 +26,7 @@ void rockchip_vpu_enc_reset_src_fmt(struct rockchip_vpu_dev *vpu,
 void rockchip_vpu_enc_reset_dst_fmt(struct rockchip_vpu_dev *vpu,
                                    struct rockchip_vpu_ctx *ctx);
 
+void *rockchip_vpu_get_ctrl(struct rockchip_vpu_ctx *ctx, u32 id);
+dma_addr_t rockchip_vpu_get_ref(struct vb2_queue *q, u64 ts);
+
 #endif /* ROCKCHIP_VPU_COMMON_H_ */

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +8 -0

index af2481ca2228..937e9cfb4568 100644
@@ -36,6 +36,24 @@ module_param_named(debug, rockchip_vpu_debug, int, 0644);
 MODULE_PARM_DESC(debug,
                 "Debug level - higher value produces more verbose messages");
 
+void *rockchip_vpu_get_ctrl(struct rockchip_vpu_ctx *ctx, u32 id)
+{
+       struct v4l2_ctrl *ctrl;
+
+       ctrl = v4l2_ctrl_find(&ctx->ctrl_handler, id);
+       return ctrl ? ctrl->p_cur.p : NULL;
+}
+

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +6 -0

 static void rockchip_vpu_job_finish(struct rockchip_vpu_dev *vpu,
                                    struct rockchip_vpu_ctx *ctx,
                                    unsigned int bytesused,

@@ -164,6 +182,7 @@ enc_queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
        src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
        src_vq->lock = &ctx->dev->vpu_mutex;
        src_vq->dev = ctx->dev->v4l2_dev.dev;
+       src_vq->supports_requests = true;
 
        ret = vb2_queue_init(src_vq);
        if (ret)

@@ -328,6 +347,11 @@ static const struct of_device_id of_rockchip_vpu_match[] = {
 };
 MODULE_DEVICE_TABLE(of, of_rockchip_vpu_match);
 
+static const struct media_device_ops rockchip_m2m_media_ops = {
+       .req_validate = vb2_request_validate,
+       .req_queue = v4l2_m2m_request_queue,
+};
+
 static int rockchip_vpu_video_device_register(struct rockchip_vpu_dev *vpu)
 {
        const struct of_device_id *match;

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +1 -0

        media_device_init(&vpu->mdev);
+       vpu->mdev.ops = &rockchip_m2m_media_ops;
        vpu->v4l2_dev.mdev = &vpu->mdev;
 
        ret = rockchip_vpu_video_device_register(vpu);

drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c +3 -0

index 2b28403314bc..1b5a675ef24f 100644
@@ -555,14 +555,32 @@ static void rockchip_vpu_stop_streaming(struct vb2_queue *q)
                        vbuf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
                if (!vbuf)
                        break;
+               v4l2_ctrl_request_complete(vbuf->vb2_buf.req_obj.req, &ctx->ctrl_handler);
                v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
        }
 }
 
+static void rockchip_vpu_buf_request_complete(struct vb2_buffer *vb)
+{

drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c static void rockchip_vpu_stop_streaming(struct vb2_queue *q) · line 574

Quoted 5 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c +5 -0

+static int rockchip_vpu_buf_out_validate(struct vb2_buffer *vb)
+{
+       struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+
+       vbuf->field = V4L2_FIELD_NONE;

Hmm, "validate" in the name of this callback would suggest that we should just check the contents, not change them. Hans, what was the intention when adding this callback? Are we missing the const specifier in the argument?

On 3/28/19 8:20 AM, Tomasz Figa wrote:

<snip>

Quoted 10 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c static void rockchip_vpu_stop_streaming(struct vb2_queue *q) · line 574

Quoted 5 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c +5 -0

+static int rockchip_vpu_buf_out_validate(struct vb2_buffer *vb)
+{
+       struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+
+       vbuf->field = V4L2_FIELD_NONE;

Hmm, "validate" in the name of this callback would suggest that we should just check the contents, not change them. Hans, what was the intention when adding this callback? Are we missing the const specifier in the argument?

See the original commit log:

https://www.mail-archive.com/[email protected]/msg143163.html

It is allowed to either just validate and return an error if wrong, or change it to something known to be valid. In particular, userspace can set this to FIELD_ANY, and in that case the driver must replace it with something valid.

Most drivers just support FIELD_NONE, and just set it.

That said, I think we should tighten the spec for this as this is not well documented.

I propose that if vbuf->field == FIELD_ANY, then replace it with something sane. Otherwise validate it and return an error if the field value is not supported.

And FIELD_ALTERNATE is never allowed here, that's always wrong.

Quoted 79 lines · Ezequiel Garcia

Introduce support for the Request API. Although the JPEG encoder does not mandate using the Request API, it's perfectly possible to use it, if the application wants to.

In addition, add helpers that will be used by video decoders.

Signed-off-by Ezequiel Garcia <ezequiel-ZGY8ohtN/[email protected]>

---
 .../rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c     |  6 ++++
 .../media/rockchip/vpu/rockchip_vpu_common.h  |  3 ++
 .../media/rockchip/vpu/rockchip_vpu_drv.c     | 29 ++++++++++++++++++-
 .../media/rockchip/vpu/rockchip_vpu_enc.c     | 18 ++++++++++++
 4 files changed, 55 insertions(+), 1 deletion(-)

drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c +6 -0

index fe92c40d0a84..e11ee152b8ce 100644
@@ -113,11 +113,15 @@ void rk3399_vpu_jpeg_enc_run(struct rockchip_vpu_ctx *ctx)
        struct rockchip_vpu_dev *vpu = ctx->dev;
        struct vb2_v4l2_buffer *src_buf, *dst_buf;
        struct rockchip_vpu_jpeg_ctx jpeg_ctx;
+       struct media_request *src_req;
        u32 reg;
 
        src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
        dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
 
+       src_req = src_buf->vb2_buf.req_obj.req;
+       v4l2_ctrl_request_setup(src_req, &ctx->ctrl_handler);
+
        memset(&jpeg_ctx, 0, sizeof(jpeg_ctx));
        jpeg_ctx.buffer = vb2_plane_vaddr(&dst_buf->vb2_buf, 0);
        jpeg_ctx.width = ctx->dst_fmt.width;

@@ -153,6 +157,8 @@ void rk3399_vpu_jpeg_enc_run(struct rockchip_vpu_ctx *ctx)
                | VEPU_REG_ENCODE_FORMAT_JPEG
                | VEPU_REG_ENCODE_ENABLE;
 
+       v4l2_ctrl_request_complete(src_req, &ctx->ctrl_handler);
+
        /* Kick the watchdog and start encoding */
        schedule_delayed_work(&vpu->watchdog_work, msecs_to_jiffies(2000));
        vepu_write(vpu, reg, VEPU_REG_ENCODE_START);

drivers/staging/media/rockchip/vpu/rockchip_vpu_common.h +3 -0

index ca77668d9579..ac018136a7bc 100644
@@ -26,4 +26,7 @@ void rockchip_vpu_enc_reset_src_fmt(struct rockchip_vpu_dev *vpu,
 void rockchip_vpu_enc_reset_dst_fmt(struct rockchip_vpu_dev *vpu,
                                    struct rockchip_vpu_ctx *ctx);
 
+void *rockchip_vpu_get_ctrl(struct rockchip_vpu_ctx *ctx, u32 id);
+dma_addr_t rockchip_vpu_get_ref(struct vb2_queue *q, u64 ts);
+
 #endif /* ROCKCHIP_VPU_COMMON_H_ */

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +16 -0

index af2481ca2228..937e9cfb4568 100644
@@ -36,6 +36,24 @@ module_param_named(debug, rockchip_vpu_debug, int, 0644);
 MODULE_PARM_DESC(debug,
                 "Debug level - higher value produces more verbose messages");
 
+void *rockchip_vpu_get_ctrl(struct rockchip_vpu_ctx *ctx, u32 id)
+{
+       struct v4l2_ctrl *ctrl;
+
+       ctrl = v4l2_ctrl_find(&ctx->ctrl_handler, id);
+       return ctrl ? ctrl->p_cur.p : NULL;
+}
+
+dma_addr_t rockchip_vpu_get_ref(struct vb2_queue *q, u64 ts)
+{
+       int index;
+
+       index = vb2_find_timestamp(q, ts, 0);
+       if (index >= 0)
+               return vb2_dma_contig_plane_dma_addr(q->bufs[index], 0);
+       return 0;

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c module_param_named(debug, rockchip_vpu_debug, int, 0644); · line 56

Quoted 2 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +2 -0

+}
+

Hmm, I assume these 2 are the "helpers that will be used by video decoders"? Probably overly nitpicking, but these are relatively small, so why couldn't they be just added in the patch that actually adds the code using them? Otherwise, if for some reason we separate the series, we end up with dead code.

Quoted 26 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +6 -0

 static void rockchip_vpu_job_finish(struct rockchip_vpu_dev *vpu,
                                    struct rockchip_vpu_ctx *ctx,
                                    unsigned int bytesused,

@@ -164,6 +182,7 @@ enc_queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
        src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
        src_vq->lock = &ctx->dev->vpu_mutex;
        src_vq->dev = ctx->dev->v4l2_dev.dev;
+       src_vq->supports_requests = true;
 
        ret = vb2_queue_init(src_vq);
        if (ret)

@@ -328,6 +347,11 @@ static const struct of_device_id of_rockchip_vpu_match[] = {
 };
 MODULE_DEVICE_TABLE(of, of_rockchip_vpu_match);
 
+static const struct media_device_ops rockchip_m2m_media_ops = {
+       .req_validate = vb2_request_validate,
+       .req_queue = v4l2_m2m_request_queue,
+};
+
 static int rockchip_vpu_video_device_register(struct rockchip_vpu_dev *vpu)
 {
        const struct of_device_id *match;

@@ -610,8 +634,11 @@ static int rockchip_vpu_probe(struct platform_device *pdev)
        }
 

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c static int rockchip_vpu_probe(struct platform_device *pdev) · line 639

Quoted 5 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +3 -1

        vpu->mdev.dev = vpu->dev;
-       strlcpy(vpu->mdev.model, DRIVER_NAME, sizeof(vpu->mdev.model));
+       strscpy(vpu->mdev.model, DRIVER_NAME, sizeof(vpu->mdev.model));
+       strscpy(vpu->mdev.bus_info, "platform:" DRIVER_NAME,
+               sizeof(vpu->mdev.bus_info));

Unrelated changes?

Quoted 27 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +1 -0

        media_device_init(&vpu->mdev);
+       vpu->mdev.ops = &rockchip_m2m_media_ops;
        vpu->v4l2_dev.mdev = &vpu->mdev;
 
        ret = rockchip_vpu_video_device_register(vpu);

drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c +10 -0

index 2b28403314bc..1b5a675ef24f 100644
@@ -555,14 +555,32 @@ static void rockchip_vpu_stop_streaming(struct vb2_queue *q)
                        vbuf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
                if (!vbuf)
                        break;
+               v4l2_ctrl_request_complete(vbuf->vb2_buf.req_obj.req, &ctx->ctrl_handler);
                v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
        }
 }
 
+static void rockchip_vpu_buf_request_complete(struct vb2_buffer *vb)
+{
+       struct rockchip_vpu_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
+
+       v4l2_ctrl_request_complete(vb->req_obj.req, &ctx->ctrl_handler);
+}
+
+static int rockchip_vpu_buf_out_validate(struct vb2_buffer *vb)
+{

drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c static void rockchip_vpu_stop_streaming(struct vb2_queue *q) · line 574

Quoted 3 lines

drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c +3 -0

+       struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+
+       vbuf->field = V4L2_FIELD_NONE;

Hmm, "validate" in the name of this callback would suggest that we should just check the contents, not change them. Hans, what was the intention when adding this callback? Are we missing the const specifier in the argument?

drivers/staging/media/rockchip/vpu/rockchip_vpu_dec.c new file +52 -0

+static const struct rockchip_vpu_fmt *
+rockchip_vpu_find_format(struct rockchip_vpu_ctx *ctx, u32 fourcc)
+{
+       struct rockchip_vpu_dev *dev = ctx->dev;
+       const struct rockchip_vpu_fmt *formats;
+       unsigned int num_fmts, i;
+
+       formats = dev->variant->dec_fmts;
+       num_fmts = dev->variant->num_dec_fmts;
+       for (i = 0; i < num_fmts; i++)
+               if (formats[i].fourcc == fourcc)
+                       return &formats[i];
+       return NULL;
+}
+
+static const struct rockchip_vpu_fmt *
+rockchip_vpu_get_default_fmt(struct rockchip_vpu_ctx *ctx, bool bitstream)
+{
+       struct rockchip_vpu_dev *dev = ctx->dev;
+       const struct rockchip_vpu_fmt *formats;
+       unsigned int num_fmts, i;
+
+       formats = dev->variant->dec_fmts;
+       num_fmts = dev->variant->num_dec_fmts;
+       for (i = 0; i < num_fmts; i++)
+               if (bitstream == (formats[i].codec_mode != RK_VPU_MODE_NONE))
+                       return &formats[i];
+       return NULL;
+}
+
+static int vidioc_querycap(struct file *file, void *priv,
+                          struct v4l2_capability *cap)
+{
+       struct rockchip_vpu_dev *vpu = video_drvdata(file);
+
+       strlcpy(cap->driver, vpu->dev->driver->name, sizeof(cap->driver));
+       strlcpy(cap->card, vpu->vfd_dec->name, sizeof(cap->card));
+       snprintf(cap->bus_info, sizeof(cap->bus_info), "platform: %s",
+                vpu->dev->driver->name);
+       return 0;
+}
+
+static int vidioc_enum_framesizes(struct file *file, void *priv,
+                                 struct v4l2_frmsizeenum *fsize)
+{
+       struct rockchip_vpu_ctx *ctx = fh_to_ctx(priv);
+       const struct rockchip_vpu_fmt *fmt;
+
+       if (fsize->index != 0) {
+               vpu_debug(0, "invalid frame size index (expected 0, got %d)\n",
+                               fsize->index);
+               return -EINVAL;

drivers/staging/media/rockchip/vpu/rockchip_vpu_dec.c new file +45 -0

+       if (fmt->codec_mode == RK_VPU_MODE_NONE)
+               return -EINVAL;
+
+       fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
+       fsize->stepwise = fmt->frmsize;
+
+       return 0;
+}
+
+static int vidioc_enum_fmt_vid_cap_mplane(struct file *file, void *priv,
+                                         struct v4l2_fmtdesc *f)
+{
+       struct rockchip_vpu_dev *dev = video_drvdata(file);
+       const struct rockchip_vpu_fmt *fmt;
+       const struct rockchip_vpu_fmt *formats;
+       int num_fmts, i, j = 0;
+
+       formats = dev->variant->dec_fmts;
+       num_fmts = dev->variant->num_dec_fmts;
+       for (i = 0; i < num_fmts; i++) {
+               /* Skip compressed formats */
+               if (formats[i].codec_mode != RK_VPU_MODE_NONE)
+                       continue;
+               if (j == f->index) {
+                       fmt = &formats[i];
+                       f->pixelformat = fmt->fourcc;
+                       return 0;
+               }
+               ++j;
+       }
+       return -EINVAL;
+}
+
+static int vidioc_enum_fmt_vid_out_mplane(struct file *file, void *priv,
+                                         struct v4l2_fmtdesc *f)
+{
+       struct rockchip_vpu_dev *dev = video_drvdata(file);
+       const struct rockchip_vpu_fmt *formats;
+       const struct rockchip_vpu_fmt *fmt;
+       int num_fmts, i, j = 0;
+
+       formats = dev->variant->dec_fmts;
+       num_fmts = dev->variant->num_dec_fmts;
+       for (i = 0; i < num_fmts; i++) {
+               if (formats[i].codec_mode == RK_VPU_MODE_NONE)

drivers/staging/media/rockchip/vpu/rockchip_vpu_dec.c new file +110 -0

+
+static int vidioc_g_fmt_out_mplane(struct file *file, void *priv,
+                                  struct v4l2_format *f)
+{
+       struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
+       struct rockchip_vpu_ctx *ctx = fh_to_ctx(priv);
+
+       vpu_debug(4, "f->type = %d\n", f->type);
+
+       *pix_mp = ctx->src_fmt;
+
+       return 0;
+}
+
+static int vidioc_g_fmt_cap_mplane(struct file *file, void *priv,
+                                  struct v4l2_format *f)
+{
+       struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
+       struct rockchip_vpu_ctx *ctx = fh_to_ctx(priv);
+
+       vpu_debug(4, "f->type = %d\n", f->type);
+
+       *pix_mp = ctx->dst_fmt;
+
+       return 0;
+}
+
+static int
+vidioc_try_fmt_cap_mplane(struct file *file, void *priv, struct v4l2_format *f)
+{
+       struct rockchip_vpu_ctx *ctx = fh_to_ctx(priv);
+       struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
+       const struct rockchip_vpu_fmt *fmt;
+       unsigned int width, height;
+
+       vpu_debug(4, "%c%c%c%c\n",
+                 (pix_mp->pixelformat & 0x7f),
+                 (pix_mp->pixelformat >> 8) & 0x7f,
+                 (pix_mp->pixelformat >> 16) & 0x7f,
+                 (pix_mp->pixelformat >> 24) & 0x7f);
+
+       fmt = rockchip_vpu_find_format(ctx, pix_mp->pixelformat);
+       if (!fmt) {
+               fmt = rockchip_vpu_get_default_fmt(ctx, false);
+               f->fmt.pix.pixelformat = fmt->fourcc;
+       }
+
+       pix_mp->field = V4L2_FIELD_NONE;
+       width = clamp(pix_mp->width,
+                     ctx->vpu_src_fmt->frmsize.min_width,
+                     ctx->vpu_src_fmt->frmsize.max_width);
+       height = clamp(pix_mp->height,
+                      ctx->vpu_src_fmt->frmsize.min_height,
+                      ctx->vpu_src_fmt->frmsize.max_height);
+       /* Round up to macroblocks. */
+       width = round_up(width, ctx->vpu_src_fmt->frmsize.step_width);
+       height = round_up(height, ctx->vpu_src_fmt->frmsize.step_height);
+
+       /* Fill remaining fields */
+       v4l2_fill_pixfmt_mp(pix_mp, fmt->fourcc, width, height);
+       return 0;
+}
+
+static int
+vidioc_try_fmt_out_mplane(struct file *file, void *priv, struct v4l2_format *f)
+{
+       struct rockchip_vpu_ctx *ctx = fh_to_ctx(priv);
+       struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
+       const struct rockchip_vpu_fmt *fmt;
+
+       fmt = rockchip_vpu_find_format(ctx, pix_mp->pixelformat);
+       if (!fmt) {
+               fmt = rockchip_vpu_get_default_fmt(ctx, true);
+               f->fmt.pix.pixelformat = fmt->fourcc;
+       }
+
+       pix_mp->num_planes = 1;
+       pix_mp->field = V4L2_FIELD_NONE;
+       pix_mp->width = clamp(pix_mp->width,
+                               fmt->frmsize.min_width,
+                               fmt->frmsize.max_width);
+       pix_mp->height = clamp(pix_mp->height,
+                               fmt->frmsize.min_height,
+                               fmt->frmsize.max_height);
+       /* Round up to macroblocks. */
+       pix_mp->width = round_up(pix_mp->width, fmt->frmsize.step_width);
+       pix_mp->height = round_up(pix_mp->height, fmt->frmsize.step_height);
+
+       /*
+        * For compressed formats the application can specify
+        * sizeimage. If the application passes a zero sizeimage,
+        * let's default to the maximum frame size.
+        */
+       if (!pix_mp->plane_fmt[0].sizeimage)
+               pix_mp->plane_fmt[0].sizeimage = fmt->header_size +
+                       pix_mp->width * pix_mp->height * fmt->max_depth;
+       return 0;
+}
+
+void rockchip_vpu_dec_reset_dst_fmt(struct rockchip_vpu_dev *vpu,
+                               struct rockchip_vpu_ctx *ctx)
+{
+       struct v4l2_pix_format_mplane *fmt = &ctx->dst_fmt;
+       unsigned int width, height;
+
+       ctx->vpu_dst_fmt = rockchip_vpu_get_default_fmt(ctx, false);
+
+       memset(fmt, 0, sizeof(*fmt));
+
+       width = clamp(fmt->width, ctx->vpu_src_fmt->frmsize.min_width,

drivers/staging/media/rockchip/vpu/rockchip_vpu_dec.c new file +15 -0

+}
+
+void rockchip_vpu_dec_reset_src_fmt(struct rockchip_vpu_dev *vpu,
+                               struct rockchip_vpu_ctx *ctx)
+{
+       struct v4l2_pix_format_mplane *fmt = &ctx->src_fmt;
+
+       ctx->vpu_src_fmt = rockchip_vpu_get_default_fmt(ctx, true);
+
+       memset(fmt, 0, sizeof(*fmt));
+
+       fmt->num_planes = 1;
+       fmt->width = clamp(fmt->width, ctx->vpu_src_fmt->frmsize.min_width,
+                     ctx->vpu_src_fmt->frmsize.max_width);
+       fmt->height = clamp(fmt->height, ctx->vpu_src_fmt->frmsize.min_height,