linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] media: vimc: Add a V4L2 output device
@ 2019-07-02 15:47 André Almeida
  2019-07-02 15:47 ` [PATCH 1/7] media: vimc: Create video module André Almeida
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: André Almeida @ 2019-07-02 15:47 UTC (permalink / raw)
  To: linux-media
  Cc: mchehab, hverkuil, helen.koike, kernel, linux-kernel, André Almeida

Hello,

This patch adds a V4L2 output device on vimc, that comply with V4L2 API
for video output. If there is an output device and a capture device at the
same pipeline, one can get a video loopback pipeline feeding frames at
the output and then seeing them at the capture. It's possible to insert
vimc submodules at the pipeline to modify the image (e.g. a scaler).

If one starts a streaming at the capture, with the output off, the
capture will display a noisy frame. If one starts a streaming at the
output with the capture off, the output will just consume the buffers,
without sending them to the pipeline. If both output and capture are
streaming, the loopback will happen.

The patches 1 and 2 provide some ground to create the output
device. The patch 3 creates the device and modify how the vimc-streamer
was dealing with the s_stream callback on other vimc modules, to make
simpler implementing this callback at vimc-output. Patch 4 change the
behavior of the pipeline in order to be closer to a real life hardware.
Patches 5-7 updates the default pipeline and the documentation to
include the new output device.

This is the result of v4l2-compliance after this patch series:
$ v4l2-compliance -m0 -s50
Grand Total for vimc device /dev/media0: 476, Succeeded: 476, Failed: 0,
Warnings: 0

A git tree up to date with media-master and with this changes can be found
at: https://gitlab.collabora.com/tonyk/linux/tree/vimc/output

In order to test it, one can follow these instructions:

1 - Configure the pipeline (requires v4l-utils):

$ media-ctl -d platform:vimc -V '"Sensor A":0[fmt:SBGGR8_1X8/640x480]'
$ media-ctl -d platform:vimc -V '"Debayer A":0[fmt:SBGGR8_1X8/640x480]'
$ media-ctl -d platform:vimc -V '"Sensor B":0[fmt:SBGGR8_1X8/640x480]'
$ media-ctl -d platform:vimc -V '"Debayer B":0[fmt:SBGGR8_1X8/640x480]'
$ v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440
$ v4l2-ctl -z platform:vimc -d "Raw Capture 0" -v pixelformat=BA81
$ v4l2-ctl -z platform:vimc -d "Raw Capture 1" -v pixelformat=BA81
$ v4l2-ctl -z platform:vimc -e "RGB/YUV Input" -v width=640,height=480

2 - Use a userspace application:
2.a gst-launch (requires gstreamer and gst-plugins-good):

Feed frames into the output and grab from the capture (rescaled for
convenience):

$ gst-launch-1.0 videotestsrc pattern=ball ! \
	video/x-raw,width=640,height=480,format=RGB \
	! v4l2sink device=/dev/video2 v4l2src device=/dev/video3 ! \
	video/x-raw,width=1920,height=1440,format=RGB ! videoscale ! \
	video/x-raw,width=640,height=480 ! videoconvert ! ximagesink

2.b qv4l2 (requires v4l-utils):

Open the output device:

$ qv4l2 -d2

Open the capture device:

$ qv4l2 -d3

Start the streaming at both, at any order. You can change the frame
content at "Test Pattern Generator" -> "Test Pattern" on the output.

Thanks,
	André

André Almeida (7):
  media: vimc: Create video module
  media: vimc: video: Add write file operation
  media: vimc: Create a V4L2 output device
  media: vimc: Send null buffer through the pipeline
  media: vimc: core: Add output device on the pipeline
  media: vimc.dot: Update default topology diagram
  media: vimc.rst: Add output device

 Documentation/media/v4l-drivers/vimc.dot    |   4 +-
 Documentation/media/v4l-drivers/vimc.rst    |  12 +-
 drivers/media/platform/vimc/Makefile        |   4 +-
 drivers/media/platform/vimc/vimc-capture.c  | 356 +++----------------
 drivers/media/platform/vimc/vimc-common.h   |   5 +-
 drivers/media/platform/vimc/vimc-core.c     |   7 +-
 drivers/media/platform/vimc/vimc-debayer.c  |  14 +-
 drivers/media/platform/vimc/vimc-output.c   | 362 ++++++++++++++++++++
 drivers/media/platform/vimc/vimc-scaler.c   |  13 +-
 drivers/media/platform/vimc/vimc-sensor.c   |  10 +-
 drivers/media/platform/vimc/vimc-streamer.c |  24 +-
 drivers/media/platform/vimc/vimc-video.c    | 273 +++++++++++++++
 drivers/media/platform/vimc/vimc-video.h    | 130 +++++++
 13 files changed, 849 insertions(+), 365 deletions(-)
 create mode 100644 drivers/media/platform/vimc/vimc-output.c
 create mode 100644 drivers/media/platform/vimc/vimc-video.c
 create mode 100644 drivers/media/platform/vimc/vimc-video.h

-- 
2.22.0


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

* [PATCH 1/7] media: vimc: Create video module
  2019-07-02 15:47 [PATCH 0/7] media: vimc: Add a V4L2 output device André Almeida
@ 2019-07-02 15:47 ` André Almeida
  2019-07-02 15:47 ` [PATCH 2/7] media: vimc: video: Add write file operation André Almeida
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: André Almeida @ 2019-07-02 15:47 UTC (permalink / raw)
  To: linux-media
  Cc: mchehab, hverkuil, helen.koike, kernel, linux-kernel, André Almeida

V4L2 video capture and video output devices shares a lot of common code.
To enhance code reuse with future video output, split vimc-capture.c in
three files: vimc-capture.c and vimc-video.{c,h}. Keep strict capture
related functions on vimc-capture.c. This change is meant to the future
addition of a video output device in vimc, as it will make easier for code
reuse and simplicity.

Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
 drivers/media/platform/vimc/Makefile       |   2 +-
 drivers/media/platform/vimc/vimc-capture.c | 341 ++-------------------
 drivers/media/platform/vimc/vimc-video.c   | 264 ++++++++++++++++
 drivers/media/platform/vimc/vimc-video.h   | 127 ++++++++
 4 files changed, 417 insertions(+), 317 deletions(-)
 create mode 100644 drivers/media/platform/vimc/vimc-video.c
 create mode 100644 drivers/media/platform/vimc/vimc-video.h

diff --git a/drivers/media/platform/vimc/Makefile b/drivers/media/platform/vimc/Makefile
index 96d06f030c31..fb90aa0f33a5 100644
--- a/drivers/media/platform/vimc/Makefile
+++ b/drivers/media/platform/vimc/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 vimc-y := vimc-core.o vimc-common.o vimc-streamer.o
 
-obj-$(CONFIG_VIDEO_VIMC) += vimc.o vimc-capture.o vimc-debayer.o \
+obj-$(CONFIG_VIDEO_VIMC) += vimc.o vimc-video.o vimc-capture.o vimc-debayer.o \
                 vimc-scaler.o vimc-sensor.o
diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c
index 664855708fdf..e80fa1ee3dc1 100644
--- a/drivers/media/platform/vimc/vimc-capture.c
+++ b/drivers/media/platform/vimc/vimc-capture.c
@@ -5,231 +5,18 @@
  * Copyright (C) 2015-2017 Helen Koike <helen.fornazier@gmail.com>
  */
 
-#include <linux/component.h>
-#include <linux/module.h>
-#include <linux/mod_devicetable.h>
-#include <linux/platform_device.h>
-#include <media/v4l2-ioctl.h>
-#include <media/videobuf2-core.h>
-#include <media/videobuf2-vmalloc.h>
-
-#include "vimc-common.h"
-#include "vimc-streamer.h"
+#include "vimc-video.h"
 
 #define VIMC_CAP_DRV_NAME "vimc-capture"
 
-static const u32 vimc_cap_supported_pixfmt[] = {
-	V4L2_PIX_FMT_BGR24,
-	V4L2_PIX_FMT_RGB24,
-	V4L2_PIX_FMT_ARGB32,
-	V4L2_PIX_FMT_SBGGR8,
-	V4L2_PIX_FMT_SGBRG8,
-	V4L2_PIX_FMT_SGRBG8,
-	V4L2_PIX_FMT_SRGGB8,
-	V4L2_PIX_FMT_SBGGR10,
-	V4L2_PIX_FMT_SGBRG10,
-	V4L2_PIX_FMT_SGRBG10,
-	V4L2_PIX_FMT_SRGGB10,
-	V4L2_PIX_FMT_SBGGR10ALAW8,
-	V4L2_PIX_FMT_SGBRG10ALAW8,
-	V4L2_PIX_FMT_SGRBG10ALAW8,
-	V4L2_PIX_FMT_SRGGB10ALAW8,
-	V4L2_PIX_FMT_SBGGR10DPCM8,
-	V4L2_PIX_FMT_SGBRG10DPCM8,
-	V4L2_PIX_FMT_SGRBG10DPCM8,
-	V4L2_PIX_FMT_SRGGB10DPCM8,
-	V4L2_PIX_FMT_SBGGR12,
-	V4L2_PIX_FMT_SGBRG12,
-	V4L2_PIX_FMT_SGRBG12,
-	V4L2_PIX_FMT_SRGGB12,
-};
-
-struct vimc_cap_device {
-	struct vimc_ent_device ved;
-	struct video_device vdev;
-	struct device *dev;
-	struct v4l2_pix_format format;
-	struct vb2_queue queue;
-	struct list_head buf_list;
-	/*
-	 * NOTE: in a real driver, a spin lock must be used to access the
-	 * queue because the frames are generated from a hardware interruption
-	 * and the isr is not allowed to sleep.
-	 * Even if it is not necessary a spinlock in the vimc driver, we
-	 * use it here as a code reference
-	 */
-	spinlock_t qlock;
-	struct mutex lock;
-	u32 sequence;
-	struct vimc_stream stream;
-};
-
-static const struct v4l2_pix_format fmt_default = {
-	.width = 640,
-	.height = 480,
-	.pixelformat = V4L2_PIX_FMT_RGB24,
-	.field = V4L2_FIELD_NONE,
-	.colorspace = V4L2_COLORSPACE_DEFAULT,
-};
-
-struct vimc_cap_buffer {
-	/*
-	 * struct vb2_v4l2_buffer must be the first element
-	 * the videobuf2 framework will allocate this struct based on
-	 * buf_struct_size and use the first sizeof(struct vb2_buffer) bytes of
-	 * memory as a vb2_buffer
-	 */
-	struct vb2_v4l2_buffer vb2;
-	struct list_head list;
-};
-
-static int vimc_cap_querycap(struct file *file, void *priv,
-			     struct v4l2_capability *cap)
-{
-	strscpy(cap->driver, VIMC_PDEV_NAME, sizeof(cap->driver));
-	strscpy(cap->card, KBUILD_MODNAME, sizeof(cap->card));
-	snprintf(cap->bus_info, sizeof(cap->bus_info),
-		 "platform:%s", VIMC_PDEV_NAME);
-
-	return 0;
-}
-
-static void vimc_cap_get_format(struct vimc_ent_device *ved,
-				struct v4l2_pix_format *fmt)
-{
-	struct vimc_cap_device *vcap = container_of(ved, struct vimc_cap_device,
-						    ved);
-
-	*fmt = vcap->format;
-}
-
-static int vimc_cap_g_fmt_vid_cap(struct file *file, void *priv,
-				  struct v4l2_format *f)
-{
-	struct vimc_cap_device *vcap = video_drvdata(file);
-
-	f->fmt.pix = vcap->format;
-
-	return 0;
-}
-
-static int vimc_cap_try_fmt_vid_cap(struct file *file, void *priv,
-				    struct v4l2_format *f)
-{
-	struct v4l2_pix_format *format = &f->fmt.pix;
-
-	format->width = clamp_t(u32, format->width, VIMC_FRAME_MIN_WIDTH,
-				VIMC_FRAME_MAX_WIDTH) & ~1;
-	format->height = clamp_t(u32, format->height, VIMC_FRAME_MIN_HEIGHT,
-				 VIMC_FRAME_MAX_HEIGHT) & ~1;
-
-	vimc_colorimetry_clamp(format);
-
-	if (format->field == V4L2_FIELD_ANY)
-		format->field = fmt_default.field;
-
-	/* TODO: Add support for custom bytesperline values */
-
-	/* Don't accept a pixelformat that is not on the table */
-	if (!v4l2_format_info(format->pixelformat))
-		format->pixelformat = fmt_default.pixelformat;
-
-	return v4l2_fill_pixfmt(format, format->pixelformat,
-				format->width, format->height);
-}
-
-static int vimc_cap_s_fmt_vid_cap(struct file *file, void *priv,
-				  struct v4l2_format *f)
-{
-	struct vimc_cap_device *vcap = video_drvdata(file);
-	int ret;
-
-	/* Do not change the format while stream is on */
-	if (vb2_is_busy(&vcap->queue))
-		return -EBUSY;
-
-	ret = vimc_cap_try_fmt_vid_cap(file, priv, f);
-	if (ret)
-		return ret;
-
-	dev_dbg(vcap->dev, "%s: format update: "
-		"old:%dx%d (0x%x, %d, %d, %d, %d) "
-		"new:%dx%d (0x%x, %d, %d, %d, %d)\n", vcap->vdev.name,
-		/* old */
-		vcap->format.width, vcap->format.height,
-		vcap->format.pixelformat, vcap->format.colorspace,
-		vcap->format.quantization, vcap->format.xfer_func,
-		vcap->format.ycbcr_enc,
-		/* new */
-		f->fmt.pix.width, f->fmt.pix.height,
-		f->fmt.pix.pixelformat,	f->fmt.pix.colorspace,
-		f->fmt.pix.quantization, f->fmt.pix.xfer_func,
-		f->fmt.pix.ycbcr_enc);
-
-	vcap->format = f->fmt.pix;
-
-	return 0;
-}
-
-static int vimc_cap_enum_fmt_vid_cap(struct file *file, void *priv,
-				     struct v4l2_fmtdesc *f)
-{
-	if (f->index >= ARRAY_SIZE(vimc_cap_supported_pixfmt))
-		return -EINVAL;
-
-	f->pixelformat = vimc_cap_supported_pixfmt[f->index];
-
-	return 0;
-}
-
-static bool vimc_cap_is_pixfmt_supported(u32 pixelformat)
-{
-	unsigned int i;
-
-	for (i = 0; i < ARRAY_SIZE(vimc_cap_supported_pixfmt); i++)
-		if (vimc_cap_supported_pixfmt[i] == pixelformat)
-			return true;
-	return false;
-}
-
-static int vimc_cap_enum_framesizes(struct file *file, void *fh,
-				    struct v4l2_frmsizeenum *fsize)
-{
-	if (fsize->index)
-		return -EINVAL;
-
-	if (!vimc_cap_is_pixfmt_supported(fsize->pixel_format))
-		return -EINVAL;
-
-	fsize->type = V4L2_FRMSIZE_TYPE_CONTINUOUS;
-	fsize->stepwise.min_width = VIMC_FRAME_MIN_WIDTH;
-	fsize->stepwise.max_width = VIMC_FRAME_MAX_WIDTH;
-	fsize->stepwise.min_height = VIMC_FRAME_MIN_HEIGHT;
-	fsize->stepwise.max_height = VIMC_FRAME_MAX_HEIGHT;
-	fsize->stepwise.step_width = 1;
-	fsize->stepwise.step_height = 1;
-
-	return 0;
-}
-
-static const struct v4l2_file_operations vimc_cap_fops = {
-	.owner		= THIS_MODULE,
-	.open		= v4l2_fh_open,
-	.release	= vb2_fop_release,
-	.read           = vb2_fop_read,
-	.poll		= vb2_fop_poll,
-	.unlocked_ioctl = video_ioctl2,
-	.mmap           = vb2_fop_mmap,
-};
-
 static const struct v4l2_ioctl_ops vimc_cap_ioctl_ops = {
-	.vidioc_querycap = vimc_cap_querycap,
+	.vidioc_querycap = vimc_vid_querycap,
 
-	.vidioc_g_fmt_vid_cap = vimc_cap_g_fmt_vid_cap,
-	.vidioc_s_fmt_vid_cap = vimc_cap_s_fmt_vid_cap,
-	.vidioc_try_fmt_vid_cap = vimc_cap_try_fmt_vid_cap,
-	.vidioc_enum_fmt_vid_cap = vimc_cap_enum_fmt_vid_cap,
-	.vidioc_enum_framesizes = vimc_cap_enum_framesizes,
+	.vidioc_g_fmt_vid_cap = vimc_vid_g_fmt,
+	.vidioc_s_fmt_vid_cap = vimc_vid_s_fmt,
+	.vidioc_try_fmt_vid_cap = vimc_vid_try_fmt,
+	.vidioc_enum_fmt_vid_cap = vimc_vid_enum_fmt,
+	.vidioc_enum_framesizes = vimc_vid_enum_framesizes,
 
 	.vidioc_reqbufs = vb2_ioctl_reqbufs,
 	.vidioc_create_bufs = vb2_ioctl_create_bufs,
@@ -242,24 +29,9 @@ static const struct v4l2_ioctl_ops vimc_cap_ioctl_ops = {
 	.vidioc_streamoff = vb2_ioctl_streamoff,
 };
 
-static void vimc_cap_return_all_buffers(struct vimc_cap_device *vcap,
-					enum vb2_buffer_state state)
-{
-	struct vimc_cap_buffer *vbuf, *node;
-
-	spin_lock(&vcap->qlock);
-
-	list_for_each_entry_safe(vbuf, node, &vcap->buf_list, list) {
-		list_del(&vbuf->list);
-		vb2_buffer_done(&vbuf->vb2.vb2_buf, state);
-	}
-
-	spin_unlock(&vcap->qlock);
-}
-
 static int vimc_cap_start_streaming(struct vb2_queue *vq, unsigned int count)
 {
-	struct vimc_cap_device *vcap = vb2_get_drv_priv(vq);
+	struct vimc_vid_device *vcap = vb2_get_drv_priv(vq);
 	struct media_entity *entity = &vcap->vdev.entity;
 	int ret;
 
@@ -268,7 +40,7 @@ static int vimc_cap_start_streaming(struct vb2_queue *vq, unsigned int count)
 	/* Start the media pipeline */
 	ret = media_pipeline_start(entity, &vcap->stream.pipe);
 	if (ret) {
-		vimc_cap_return_all_buffers(vcap, VB2_BUF_STATE_QUEUED);
+		vimc_vid_return_all_buffers(vcap, VB2_BUF_STATE_QUEUED);
 		return ret;
 	}
 
@@ -276,7 +48,7 @@ static int vimc_cap_start_streaming(struct vb2_queue *vq, unsigned int count)
 	ret = vimc_streamer_s_stream(&vcap->stream, &vcap->ved, 1);
 	if (ret) {
 		media_pipeline_stop(entity);
-		vimc_cap_return_all_buffers(vcap, VB2_BUF_STATE_QUEUED);
+		vimc_vid_return_all_buffers(vcap, VB2_BUF_STATE_QUEUED);
 		return ret;
 	}
 
@@ -287,65 +59,23 @@ static int vimc_cap_start_streaming(struct vb2_queue *vq, unsigned int count)
  * Stop the stream engine. Any remaining buffers in the stream queue are
  * dequeued and passed on to the vb2 framework marked as STATE_ERROR.
  */
-static void vimc_cap_stop_streaming(struct vb2_queue *vq)
+void vimc_cap_stop_streaming(struct vb2_queue *vq)
 {
-	struct vimc_cap_device *vcap = vb2_get_drv_priv(vq);
+	struct vimc_vid_device *vcap = vb2_get_drv_priv(vq);
 
 	vimc_streamer_s_stream(&vcap->stream, &vcap->ved, 0);
 
-	/* Stop the media pipeline */
 	media_pipeline_stop(&vcap->vdev.entity);
 
-	/* Release all active buffers */
-	vimc_cap_return_all_buffers(vcap, VB2_BUF_STATE_ERROR);
-}
-
-static void vimc_cap_buf_queue(struct vb2_buffer *vb2_buf)
-{
-	struct vimc_cap_device *vcap = vb2_get_drv_priv(vb2_buf->vb2_queue);
-	struct vimc_cap_buffer *buf = container_of(vb2_buf,
-						   struct vimc_cap_buffer,
-						   vb2.vb2_buf);
-
-	spin_lock(&vcap->qlock);
-	list_add_tail(&buf->list, &vcap->buf_list);
-	spin_unlock(&vcap->qlock);
-}
-
-static int vimc_cap_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
-				unsigned int *nplanes, unsigned int sizes[],
-				struct device *alloc_devs[])
-{
-	struct vimc_cap_device *vcap = vb2_get_drv_priv(vq);
-
-	if (*nplanes)
-		return sizes[0] < vcap->format.sizeimage ? -EINVAL : 0;
-	/* We don't support multiplanes for now */
-	*nplanes = 1;
-	sizes[0] = vcap->format.sizeimage;
-
-	return 0;
-}
-
-static int vimc_cap_buffer_prepare(struct vb2_buffer *vb)
-{
-	struct vimc_cap_device *vcap = vb2_get_drv_priv(vb->vb2_queue);
-	unsigned long size = vcap->format.sizeimage;
-
-	if (vb2_plane_size(vb, 0) < size) {
-		dev_err(vcap->dev, "%s: buffer too small (%lu < %lu)\n",
-			vcap->vdev.name, vb2_plane_size(vb, 0), size);
-		return -EINVAL;
-	}
-	return 0;
+	vimc_vid_return_all_buffers(vcap, VB2_BUF_STATE_ERROR);
 }
 
 static const struct vb2_ops vimc_cap_qops = {
 	.start_streaming	= vimc_cap_start_streaming,
 	.stop_streaming		= vimc_cap_stop_streaming,
-	.buf_queue		= vimc_cap_buf_queue,
-	.queue_setup		= vimc_cap_queue_setup,
-	.buf_prepare		= vimc_cap_buffer_prepare,
+	.buf_queue		= vimc_vid_buf_queue,
+	.queue_setup		= vimc_vid_queue_setup,
+	.buf_prepare		= vimc_vid_buffer_prepare,
 	/*
 	 * Since q->lock is set we can use the standard
 	 * vb2_ops_wait_prepare/finish helper functions.
@@ -358,33 +88,12 @@ static const struct media_entity_operations vimc_cap_mops = {
 	.link_validate		= vimc_link_validate,
 };
 
-static void vimc_cap_release(struct video_device *vdev)
-{
-	struct vimc_cap_device *vcap =
-		container_of(vdev, struct vimc_cap_device, vdev);
-
-	vimc_pads_cleanup(vcap->ved.pads);
-	kfree(vcap);
-}
-
-static void vimc_cap_comp_unbind(struct device *comp, struct device *master,
-				 void *master_data)
-{
-	struct vimc_ent_device *ved = dev_get_drvdata(comp);
-	struct vimc_cap_device *vcap = container_of(ved, struct vimc_cap_device,
-						    ved);
-
-	vb2_queue_release(&vcap->queue);
-	media_entity_cleanup(ved->ent);
-	video_unregister_device(&vcap->vdev);
-}
-
 static void *vimc_cap_process_frame(struct vimc_ent_device *ved,
 				    const void *frame)
 {
-	struct vimc_cap_device *vcap = container_of(ved, struct vimc_cap_device,
+	struct vimc_vid_device *vcap = container_of(ved, struct vimc_vid_device,
 						    ved);
-	struct vimc_cap_buffer *vimc_buf;
+	struct vimc_vid_buffer *vimc_buf;
 	void *vbuf;
 
 	spin_lock(&vcap->qlock);
@@ -423,12 +132,12 @@ static int vimc_cap_comp_bind(struct device *comp, struct device *master,
 {
 	struct v4l2_device *v4l2_dev = master_data;
 	struct vimc_platform_data *pdata = comp->platform_data;
-	struct vimc_cap_device *vcap;
+	struct vimc_vid_device *vcap;
 	struct video_device *vdev;
 	struct vb2_queue *q;
 	int ret;
 
-	/* Allocate the vimc_cap_device struct */
+	/* Allocate the vimc_vid_device struct */
 	vcap = kzalloc(sizeof(*vcap), GFP_KERNEL);
 	if (!vcap)
 		return -ENOMEM;
@@ -457,7 +166,7 @@ static int vimc_cap_comp_bind(struct device *comp, struct device *master,
 	q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
 	q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
 	q->drv_priv = vcap;
-	q->buf_struct_size = sizeof(struct vimc_cap_buffer);
+	q->buf_struct_size = sizeof(struct vimc_vid_buffer);
 	q->ops = &vimc_cap_qops;
 	q->mem_ops = &vb2_vmalloc_memops;
 	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
@@ -483,7 +192,7 @@ static int vimc_cap_comp_bind(struct device *comp, struct device *master,
 	/* Fill the vimc_ent_device struct */
 	vcap->ved.ent = &vcap->vdev.entity;
 	vcap->ved.process_frame = vimc_cap_process_frame;
-	vcap->ved.vdev_get_format = vimc_cap_get_format;
+	vcap->ved.vdev_get_format = vimc_vid_get_format;
 	dev_set_drvdata(comp, &vcap->ved);
 	vcap->dev = comp;
 
@@ -491,8 +200,8 @@ static int vimc_cap_comp_bind(struct device *comp, struct device *master,
 	vdev = &vcap->vdev;
 	vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
 	vdev->entity.ops = &vimc_cap_mops;
-	vdev->release = vimc_cap_release;
-	vdev->fops = &vimc_cap_fops;
+	vdev->release = vimc_vid_release;
+	vdev->fops = &vimc_vid_fops;
 	vdev->ioctl_ops = &vimc_cap_ioctl_ops;
 	vdev->lock = &vcap->lock;
 	vdev->queue = q;
@@ -525,7 +234,7 @@ static int vimc_cap_comp_bind(struct device *comp, struct device *master,
 
 static const struct component_ops vimc_cap_comp_ops = {
 	.bind = vimc_cap_comp_bind,
-	.unbind = vimc_cap_comp_unbind,
+	.unbind = vimc_vid_comp_unbind,
 };
 
 static int vimc_cap_probe(struct platform_device *pdev)
diff --git a/drivers/media/platform/vimc/vimc-video.c b/drivers/media/platform/vimc/vimc-video.c
new file mode 100644
index 000000000000..f7ccb2e9d6c5
--- /dev/null
+++ b/drivers/media/platform/vimc/vimc-video.c
@@ -0,0 +1,264 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * vimc-video.c Virtual Media Controller Driver
+ *
+ *	Derivated work from original vimc-capture by:
+ *		Helen Koike <helen.koike@collabora.com>
+ *
+ * Copyright 2019 Collabora, Ltd.
+ */
+
+#include "vimc-video.h"
+
+/*
+ * TODO: video device should only enum formats that all subdevices on
+ * topology accepts
+ */
+static const u32 vimc_vid_supported_pixfmt[] = {
+	V4L2_PIX_FMT_BGR24,
+	V4L2_PIX_FMT_RGB24,
+	V4L2_PIX_FMT_ARGB32,
+	V4L2_PIX_FMT_SBGGR8,
+	V4L2_PIX_FMT_SGBRG8,
+	V4L2_PIX_FMT_SGRBG8,
+	V4L2_PIX_FMT_SRGGB8,
+	V4L2_PIX_FMT_SBGGR10,
+	V4L2_PIX_FMT_SGBRG10,
+	V4L2_PIX_FMT_SGRBG10,
+	V4L2_PIX_FMT_SRGGB10,
+	V4L2_PIX_FMT_SBGGR10ALAW8,
+	V4L2_PIX_FMT_SGBRG10ALAW8,
+	V4L2_PIX_FMT_SGRBG10ALAW8,
+	V4L2_PIX_FMT_SRGGB10ALAW8,
+	V4L2_PIX_FMT_SBGGR10DPCM8,
+	V4L2_PIX_FMT_SGBRG10DPCM8,
+	V4L2_PIX_FMT_SGRBG10DPCM8,
+	V4L2_PIX_FMT_SRGGB10DPCM8,
+	V4L2_PIX_FMT_SBGGR12,
+	V4L2_PIX_FMT_SGBRG12,
+	V4L2_PIX_FMT_SGRBG12,
+	V4L2_PIX_FMT_SRGGB12,
+};
+
+int vimc_vid_querycap(struct file *file, void *priv,
+		      struct v4l2_capability *cap)
+{
+	strscpy(cap->driver, VIMC_PDEV_NAME, sizeof(cap->driver));
+	strscpy(cap->card, KBUILD_MODNAME, sizeof(cap->card));
+	snprintf(cap->bus_info, sizeof(cap->bus_info),
+		 "platform:%s", VIMC_PDEV_NAME);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(vimc_vid_querycap);
+
+void vimc_vid_get_format(struct vimc_ent_device *ved,
+			 struct v4l2_pix_format *fmt)
+{
+	struct vimc_vid_device *vid = container_of(ved, struct vimc_vid_device,
+						    ved);
+
+	*fmt = vid->format;
+}
+EXPORT_SYMBOL_GPL(vimc_vid_get_format);
+
+int vimc_vid_g_fmt(struct file *file, void *priv, struct v4l2_format *f)
+{
+	struct vimc_vid_device *vid = video_drvdata(file);
+
+	f->fmt.pix = vid->format;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(vimc_vid_g_fmt);
+
+int vimc_vid_try_fmt(struct file *file, void *priv, struct v4l2_format *f)
+{
+	struct v4l2_pix_format *format = &f->fmt.pix;
+
+	format->width = clamp_t(u32, format->width, VIMC_FRAME_MIN_WIDTH,
+				VIMC_FRAME_MAX_WIDTH) & ~1;
+	format->height = clamp_t(u32, format->height, VIMC_FRAME_MIN_HEIGHT,
+				 VIMC_FRAME_MAX_HEIGHT) & ~1;
+
+	vimc_colorimetry_clamp(format);
+
+	if (format->field == V4L2_FIELD_ANY)
+		format->field = fmt_default.field;
+
+	/* TODO: Add support for custom bytesperline values */
+
+	/* Don't accept a pixelformat that is not on the table */
+	if (!v4l2_format_info(format->pixelformat))
+		format->pixelformat = fmt_default.pixelformat;
+
+	return v4l2_fill_pixfmt(format, format->pixelformat,
+				format->width, format->height);
+}
+EXPORT_SYMBOL_GPL(vimc_vid_try_fmt);
+
+int vimc_vid_s_fmt(struct file *file, void *priv, struct v4l2_format *f)
+{
+	struct vimc_vid_device *vid = video_drvdata(file);
+	int ret;
+
+	/* Do not change the format while stream is on */
+	if (vb2_is_busy(&vid->queue))
+		return -EBUSY;
+
+	ret = vimc_vid_try_fmt(file, priv, f);
+	if (ret)
+		return ret;
+
+	dev_dbg(vid->dev, "%s: format update: "
+		"old:%dx%d (0x%x, %d, %d, %d, %d) "
+		"new:%dx%d (0x%x, %d, %d, %d, %d)\n", vid->vdev.name,
+		/* old */
+		vid->format.width, vid->format.height,
+		vid->format.pixelformat, vid->format.colorspace,
+		vid->format.quantization, vid->format.xfer_func,
+		vid->format.ycbcr_enc,
+		/* new */
+		f->fmt.pix.width, f->fmt.pix.height,
+		f->fmt.pix.pixelformat,	f->fmt.pix.colorspace,
+		f->fmt.pix.quantization, f->fmt.pix.xfer_func,
+		f->fmt.pix.ycbcr_enc);
+
+	vid->format = f->fmt.pix;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(vimc_vid_s_fmt);
+
+int vimc_vid_enum_fmt(struct file *file, void *priv, struct v4l2_fmtdesc *f)
+{
+	if (f->index >= ARRAY_SIZE(vimc_vid_supported_pixfmt))
+		return -EINVAL;
+
+	f->pixelformat = vimc_vid_supported_pixfmt[f->index];
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(vimc_vid_enum_fmt);
+
+bool vimc_vid_is_pixfmt_supported(u32 pixelformat)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(vimc_vid_supported_pixfmt); i++)
+		if (vimc_vid_supported_pixfmt[i] == pixelformat)
+			return true;
+	return false;
+}
+EXPORT_SYMBOL_GPL(vimc_vid_is_pixfmt_supported);
+
+int vimc_vid_enum_framesizes(struct file *file, void *fh,
+			     struct v4l2_frmsizeenum *fsize)
+{
+	if (fsize->index)
+		return -EINVAL;
+
+	if (!vimc_vid_is_pixfmt_supported(fsize->pixel_format))
+		return -EINVAL;
+
+	fsize->type = V4L2_FRMSIZE_TYPE_CONTINUOUS;
+	fsize->stepwise.min_width = VIMC_FRAME_MIN_WIDTH;
+	fsize->stepwise.max_width = VIMC_FRAME_MAX_WIDTH;
+	fsize->stepwise.min_height = VIMC_FRAME_MIN_HEIGHT;
+	fsize->stepwise.max_height = VIMC_FRAME_MAX_HEIGHT;
+	fsize->stepwise.step_width = 1;
+	fsize->stepwise.step_height = 1;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(vimc_vid_enum_framesizes);
+
+void vimc_vid_return_all_buffers(struct vimc_vid_device *vid,
+				 enum vb2_buffer_state state)
+{
+	struct vimc_vid_buffer *vbuf, *node;
+
+	spin_lock(&vid->qlock);
+
+	list_for_each_entry_safe(vbuf, node, &vid->buf_list, list) {
+		list_del(&vbuf->list);
+		vb2_buffer_done(&vbuf->vb2.vb2_buf, state);
+	}
+
+	spin_unlock(&vid->qlock);
+}
+EXPORT_SYMBOL_GPL(vimc_vid_return_all_buffers);
+
+/*
+ * vb2 operations
+ */
+
+void vimc_vid_buf_queue(struct vb2_buffer *vb2_buf)
+{
+	struct vimc_vid_device *vid = vb2_get_drv_priv(vb2_buf->vb2_queue);
+	struct vimc_vid_buffer *buf = container_of(vb2_buf,
+						   struct vimc_vid_buffer,
+						   vb2.vb2_buf);
+
+	spin_lock(&vid->qlock);
+	list_add_tail(&buf->list, &vid->buf_list);
+	spin_unlock(&vid->qlock);
+}
+EXPORT_SYMBOL_GPL(vimc_vid_buf_queue);
+
+int vimc_vid_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
+			 unsigned int *nplanes, unsigned int sizes[],
+			 struct device *alloc_devs[])
+{
+	struct vimc_vid_device *vid = vb2_get_drv_priv(vq);
+
+	if (*nplanes)
+		return sizes[0] < vid->format.sizeimage ? -EINVAL : 0;
+	/* We don't support multiplanes for now */
+	*nplanes = 1;
+	sizes[0] = vid->format.sizeimage;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(vimc_vid_queue_setup);
+
+int vimc_vid_buffer_prepare(struct vb2_buffer *vb)
+{
+	struct vimc_vid_device *vid = vb2_get_drv_priv(vb->vb2_queue);
+	unsigned long size = vid->format.sizeimage;
+
+	if (vb2_plane_size(vb, 0) < size) {
+		dev_err(vid->dev, "%s: buffer too small (%lu < %lu)\n",
+			vid->vdev.name, vb2_plane_size(vb, 0), size);
+		return -EINVAL;
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(vimc_vid_buffer_prepare);
+
+void vimc_vid_release(struct video_device *vdev)
+{
+	struct vimc_vid_device *vid =
+		container_of(vdev, struct vimc_vid_device, vdev);
+
+	vimc_pads_cleanup(vid->ved.pads);
+	kfree(vid);
+}
+EXPORT_SYMBOL_GPL(vimc_vid_release);
+
+void vimc_vid_comp_unbind(struct device *comp, struct device *master,
+			  void *master_data)
+{
+	struct vimc_ent_device *ved = dev_get_drvdata(comp);
+	struct vimc_vid_device *vid = container_of(ved, struct vimc_vid_device,
+						    ved);
+
+	vb2_queue_release(&vid->queue);
+	media_entity_cleanup(ved->ent);
+	video_unregister_device(&vid->vdev);
+}
+EXPORT_SYMBOL_GPL(vimc_vid_comp_unbind);
+
+MODULE_DESCRIPTION("Virtual Media Controller Driver (VIMC) Video");
+MODULE_AUTHOR("André Almeida <andrealmeid@collabora.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/media/platform/vimc/vimc-video.h b/drivers/media/platform/vimc/vimc-video.h
new file mode 100644
index 000000000000..d329345cc77f
--- /dev/null
+++ b/drivers/media/platform/vimc/vimc-video.h
@@ -0,0 +1,127 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * vimc-video.h Virtual Media Controller Driver
+ *
+ *	Derivated work from original vimc-capture by:
+ *		Helen Koike <helen.koike@collabora.com>
+ *
+ * Copyright 2019 Collabora, Ltd.
+ */
+
+#ifndef _VIMC_VIDEO_H_
+#define _VIMC_VIDEO_H_
+
+#include <linux/component.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <media/v4l2-ioctl.h>
+#include <media/videobuf2-core.h>
+#include <media/videobuf2-vmalloc.h>
+
+#include "vimc-common.h"
+#include "vimc-streamer.h"
+
+static const struct v4l2_pix_format fmt_default = {
+	.width = 640,
+	.height = 480,
+	.pixelformat = V4L2_PIX_FMT_RGB24,
+	.field = V4L2_FIELD_NONE,
+	.colorspace = V4L2_COLORSPACE_DEFAULT,
+};
+
+static const struct v4l2_file_operations vimc_vid_fops = {
+	.owner		= THIS_MODULE,
+	.open		= v4l2_fh_open,
+	.release	= vb2_fop_release,
+	.read           = vb2_fop_read,
+	.poll		= vb2_fop_poll,
+	.unlocked_ioctl = video_ioctl2,
+	.mmap           = vb2_fop_mmap,
+};
+
+struct vimc_vid_device {
+	struct vimc_ent_device ved;
+	struct video_device vdev;
+	struct device *dev;
+	struct v4l2_pix_format format;
+	struct vb2_queue queue;
+	struct list_head buf_list;
+	/*
+	 * NOTE: in a real driver, a spin lock must be used to access the
+	 * queue because the frames are generated from a hardware interruption
+	 * and the isr is not allowed to sleep.
+	 * Even if it is not necessary a spinlock in the vimc driver, we
+	 * use it here as a code reference
+	 */
+	spinlock_t qlock;
+	struct mutex lock;
+	u32 sequence;
+	struct vimc_stream stream;
+	u8 *frame;
+	struct task_struct *kthread_fd;
+};
+
+struct vimc_vid_buffer {
+	/*
+	 * struct vb2_v4l2_buffer must be the first element
+	 * the videobuf2 framework will allocate this struct based on
+	 * buf_struct_size and use the first sizeof(struct vb2_buffer) bytes of
+	 * memory as a vb2_buffer
+	 */
+	struct vb2_v4l2_buffer vb2;
+	struct list_head list;
+};
+
+int vimc_vid_querycap(struct file *file, void *priv,
+		      struct v4l2_capability *cap);
+
+void vimc_vid_get_format(struct vimc_ent_device *ved,
+			 struct v4l2_pix_format *fmt);
+
+int vimc_vid_g_fmt(struct file *file, void *priv,
+		   struct v4l2_format *f);
+
+int vimc_vid_try_fmt(struct file *file, void *priv,
+		     struct v4l2_format *f);
+
+int vimc_vid_s_fmt(struct file *file, void *priv,
+		   struct v4l2_format *f);
+
+int vimc_vid_enum_fmt_vid(struct file *file, void *priv,
+			  struct v4l2_fmtdesc *f);
+
+bool vimc_vid_is_pixfmt_supported(u32 pixelformat);
+
+void vimc_vid_release(struct video_device *vdev);
+
+void vimc_vid_comp_unbind(struct device *comp, struct device *master,
+			  void *master_data);
+
+int vimc_vid_enum_fmt(struct file *file, void *priv,
+				 struct v4l2_fmtdesc *f);
+
+int vimc_vid_enum_framesizes(struct file *file, void *fh,
+			     struct v4l2_frmsizeenum *fsize);
+
+void vimc_vid_return_all_buffers(struct vimc_vid_device *vid,
+				 enum vb2_buffer_state state);
+
+void vimc_vid_buf_queue(struct vb2_buffer *vb2_buf);
+
+int vimc_vid_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
+			 unsigned int *nplanes, unsigned int sizes[],
+			 struct device *alloc_devs[]);
+
+int vimc_vid_buffer_prepare(struct vb2_buffer *vb);
+
+int vimc_vid_probe(struct platform_device *pdev);
+
+int vimc_vid_remove(struct platform_device *pdev);
+
+void vimc_vid_release(struct video_device *vdev);
+
+void vimc_vid_comp_unbind(struct device *comp, struct device *master,
+				 void *master_data);
+
+#endif /* _VIMC_COMMON_H_ */
-- 
2.22.0


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

* [PATCH 2/7] media: vimc: video: Add write file operation
  2019-07-02 15:47 [PATCH 0/7] media: vimc: Add a V4L2 output device André Almeida
  2019-07-02 15:47 ` [PATCH 1/7] media: vimc: Create video module André Almeida
@ 2019-07-02 15:47 ` André Almeida
  2019-07-02 15:47 ` [PATCH 3/7] media: vimc: Create a V4L2 output device André Almeida
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: André Almeida @ 2019-07-02 15:47 UTC (permalink / raw)
  To: linux-media
  Cc: mchehab, hverkuil, helen.koike, kernel, linux-kernel, André Almeida

Add write on the list of vb2 file operations. This is required to
create a V4L2 output device.

Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
 drivers/media/platform/vimc/vimc-video.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/vimc/vimc-video.h b/drivers/media/platform/vimc/vimc-video.h
index d329345cc77f..22cb0e2dbdbb 100644
--- a/drivers/media/platform/vimc/vimc-video.h
+++ b/drivers/media/platform/vimc/vimc-video.h
@@ -35,6 +35,7 @@ static const struct v4l2_file_operations vimc_vid_fops = {
 	.open		= v4l2_fh_open,
 	.release	= vb2_fop_release,
 	.read           = vb2_fop_read,
+	.write          = vb2_fop_write,
 	.poll		= vb2_fop_poll,
 	.unlocked_ioctl = video_ioctl2,
 	.mmap           = vb2_fop_mmap,
-- 
2.22.0


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

* [PATCH 3/7] media: vimc: Create a V4L2 output device
  2019-07-02 15:47 [PATCH 0/7] media: vimc: Add a V4L2 output device André Almeida
  2019-07-02 15:47 ` [PATCH 1/7] media: vimc: Create video module André Almeida
  2019-07-02 15:47 ` [PATCH 2/7] media: vimc: video: Add write file operation André Almeida
@ 2019-07-02 15:47 ` André Almeida
  2019-07-09 22:20   ` Helen Koike
  2019-07-02 15:47 ` [PATCH 4/7] media: vimc: Send null buffer through the pipeline André Almeida
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: André Almeida @ 2019-07-02 15:47 UTC (permalink / raw)
  To: linux-media
  Cc: mchehab, hverkuil, helen.koike, kernel, linux-kernel, André Almeida

Using the functions on vimc-video, create a V4L2 output device.
When a streaming is initialized on the output device, it'll start
returning those buffers on to the userspace, while always keeping one on
the list. When the capture device starts streaming on the same pipeline,
it will copy the first frame on the list to the process_frame pipeline.

The vimc-streamer has a callback to warn all subdevices when a streaming
will start or stop. This is useful so the subdevices can allocate a
frame with suitable size to send through the pipeline, and then free
then when it not used anymore. Since the vimc-output will also be part
of the pipeline (but it is not a V4L2 subdevice), change the callback to
be part of vimc_ent_device, rather than a linked with the subdevice API.
In this way, it's possible to have a generic solution to call start/stop
streaming callbacks to all devices in the pipeline.

Add a vb2 buffer validate function. This is required to
create a V4L2 output device.

Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
 drivers/media/platform/vimc/Makefile        |   4 +-
 drivers/media/platform/vimc/vimc-common.h   |   5 +-
 drivers/media/platform/vimc/vimc-debayer.c  |  11 +-
 drivers/media/platform/vimc/vimc-output.c   | 362 ++++++++++++++++++++
 drivers/media/platform/vimc/vimc-scaler.c   |  10 +-
 drivers/media/platform/vimc/vimc-sensor.c   |  10 +-
 drivers/media/platform/vimc/vimc-streamer.c |  22 +-
 drivers/media/platform/vimc/vimc-video.c    |   9 +
 drivers/media/platform/vimc/vimc-video.h    |   2 +
 9 files changed, 395 insertions(+), 40 deletions(-)
 create mode 100644 drivers/media/platform/vimc/vimc-output.c

diff --git a/drivers/media/platform/vimc/Makefile b/drivers/media/platform/vimc/Makefile
index fb90aa0f33a5..611331b9fceb 100644
--- a/drivers/media/platform/vimc/Makefile
+++ b/drivers/media/platform/vimc/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 vimc-y := vimc-core.o vimc-common.o vimc-streamer.o
 
-obj-$(CONFIG_VIDEO_VIMC) += vimc.o vimc-video.o vimc-capture.o vimc-debayer.o \
-                vimc-scaler.o vimc-sensor.o
+obj-$(CONFIG_VIDEO_VIMC) += vimc.o vimc-video.o vimc-capture.o vimc-output.o \
+                vimc-debayer.o vimc-scaler.o vimc-sensor.o
diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h
index 7b4d988b208b..003024bcdaed 100644
--- a/drivers/media/platform/vimc/vimc-common.h
+++ b/drivers/media/platform/vimc/vimc-common.h
@@ -78,6 +78,8 @@ struct vimc_platform_data {
  * @vdev_get_format:	callback that returns the current format a pad, used
  *			only when is_media_entity_v4l2_video_device(ent) returns
  *			true
+ * @s_stream:		callback used when vimc-capture will start/stop a
+ *			streaming, to subdevice alloc/free it frames
  *
  * Each node of the topology must create a vimc_ent_device struct. Depending on
  * the node it will be of an instance of v4l2_subdev or video_device struct
@@ -94,7 +96,8 @@ struct vimc_ent_device {
 	void * (*process_frame)(struct vimc_ent_device *ved,
 				const void *frame);
 	void (*vdev_get_format)(struct vimc_ent_device *ved,
-			      struct v4l2_pix_format *fmt);
+				struct v4l2_pix_format *fmt);
+	int (*s_stream)(struct vimc_ent_device *ved, int enable);
 };
 
 /**
diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
index 00598fbf3cba..9e0c7c2c3c72 100644
--- a/drivers/media/platform/vimc/vimc-debayer.c
+++ b/drivers/media/platform/vimc/vimc-debayer.c
@@ -320,9 +320,10 @@ static void vimc_deb_set_rgb_pix_rgb24(struct vimc_deb_device *vdeb,
 		vdeb->src_frame[index + i] = rgb[i];
 }
 
-static int vimc_deb_s_stream(struct v4l2_subdev *sd, int enable)
+static int vimc_deb_s_stream(struct vimc_ent_device *ved, int enable)
 {
-	struct vimc_deb_device *vdeb = v4l2_get_subdevdata(sd);
+	struct vimc_deb_device *vdeb = container_of(ved, struct vimc_deb_device,
+						    ved);
 
 	if (enable) {
 		u32 src_pixelformat = vdeb->ved.stream->producer_pixfmt;
@@ -373,13 +374,8 @@ static int vimc_deb_s_stream(struct v4l2_subdev *sd, int enable)
 	return 0;
 }
 
-static const struct v4l2_subdev_video_ops vimc_deb_video_ops = {
-	.s_stream = vimc_deb_s_stream,
-};
-
 static const struct v4l2_subdev_ops vimc_deb_ops = {
 	.pad = &vimc_deb_pad_ops,
-	.video = &vimc_deb_video_ops,
 };
 
 static unsigned int vimc_deb_get_val(const u8 *bytes,
@@ -549,6 +545,7 @@ static int vimc_deb_comp_bind(struct device *comp, struct device *master,
 	}
 
 	vdeb->ved.process_frame = vimc_deb_process_frame;
+	vdeb->ved.s_stream = vimc_deb_s_stream;
 	dev_set_drvdata(comp, &vdeb->ved);
 	vdeb->dev = comp;
 
diff --git a/drivers/media/platform/vimc/vimc-output.c b/drivers/media/platform/vimc/vimc-output.c
new file mode 100644
index 000000000000..3fb9441adc43
--- /dev/null
+++ b/drivers/media/platform/vimc/vimc-output.c
@@ -0,0 +1,362 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * vimc-output.c Virtual Media Controller Driver
+ *
+ * Copyright 2019 Collabora, Ltd.
+ */
+
+#include "vimc-video.h"
+#include <linux/kthread.h>
+#include <linux/freezer.h>
+
+#define VIMC_OUT_DRV_NAME "vimc-output"
+
+static const struct v4l2_ioctl_ops vimc_out_ioctl_ops = {
+	.vidioc_querycap = vimc_vid_querycap,
+
+	.vidioc_g_fmt_vid_out = vimc_vid_g_fmt,
+	.vidioc_s_fmt_vid_out = vimc_vid_s_fmt,
+	.vidioc_try_fmt_vid_out = vimc_vid_try_fmt,
+	.vidioc_enum_fmt_vid_out = vimc_vid_enum_fmt,
+	.vidioc_enum_framesizes = vimc_vid_enum_framesizes,
+
+	.vidioc_reqbufs = vb2_ioctl_reqbufs,
+	.vidioc_create_bufs = vb2_ioctl_create_bufs,
+	.vidioc_prepare_buf = vb2_ioctl_prepare_buf,
+	.vidioc_querybuf = vb2_ioctl_querybuf,
+	.vidioc_qbuf = vb2_ioctl_qbuf,
+	.vidioc_dqbuf = vb2_ioctl_dqbuf,
+	.vidioc_expbuf = vb2_ioctl_expbuf,
+	.vidioc_streamon = vb2_ioctl_streamon,
+	.vidioc_streamoff = vb2_ioctl_streamoff,
+};
+
+/**
+ * vimc_out_consume_frame - grab a frame from the queue and release it
+ *
+ * @arg: *vimc_vid_device passed as argument to kthread
+ *
+ * This function is called when the stream is on. If there's a frame available
+ * on the queue, it will get it and "consume" it. This means, marks the frame
+ * as done and make sure there're at least one frame to be on the list.
+ * It should stop when the streaming stops too
+ *
+ * Return: always 0
+ */
+static int vimc_out_consume_frame(void *arg)
+{
+	struct vimc_vid_device *vout = arg;
+	struct vimc_vid_buffer *vimc_buf;
+
+	set_freezable();
+
+	for (;;) {
+		try_to_freeze();
+		if (kthread_should_stop())
+			break;
+
+		/* TODO: support custom framerate */
+		set_current_state(TASK_UNINTERRUPTIBLE);
+		schedule_timeout(HZ / 60);
+
+		spin_lock(&vout->qlock);
+
+		/*
+		 * if there's only on frame on the buffer, keep it there
+		 * this is done in a way that the output always can send a
+		 * frame to the pipeline, even if the capture framerate is
+		 * higher than the output framarate
+		 */
+		if (list_empty(&vout->buf_list) ||
+		    list_is_singular(&vout->buf_list)) {
+			spin_unlock(&vout->qlock);
+			continue;
+		}
+
+		vimc_buf = list_first_entry(&vout->buf_list,
+				    typeof(*vimc_buf), list);
+		list_del(&vimc_buf->list);
+		spin_unlock(&vout->qlock);
+
+		vimc_buf->vb2.vb2_buf.timestamp = ktime_get_ns();
+		vimc_buf->vb2.sequence = vout->sequence++;
+		vimc_buf->vb2.field = vout->format.field;
+		vb2_buffer_done(&vimc_buf->vb2.vb2_buf, VB2_BUF_STATE_DONE);
+	}
+
+	return 0;
+}
+
+/**
+ * vimc_out_start_streaming - callback used when userspace starts streaming
+ *
+ * @vq:	   queue of vb2 video buffers
+ * @count: number of already queued buffers
+ *
+ * Allocate the output frame and starts the consume thread
+ *
+ * Return: 0 on success or error code
+ */
+static int vimc_out_start_streaming(struct vb2_queue *vq, unsigned int count)
+{
+	struct vimc_vid_device *vout = vb2_get_drv_priv(vq);
+
+	vout->sequence = 0;
+
+	vout->kthread_fd = kthread_run(vimc_out_consume_frame, vout,
+					      "vimc-output consume thread");
+
+	if (IS_ERR(vout->kthread_fd))
+		return PTR_ERR(vout->kthread_fd);
+
+	return 0;
+}
+
+/**
+ * vimc_out_stop_streaming - callback used when userspace stops streaming
+ *
+ * @vq:	   queue of vb2 video buffers
+ *
+ * Wait until vimc_stream stops consuming a frame (using a mutex) and free it.
+ * Stops the thread and return all unused buffers to userspace.
+ */
+static void vimc_out_stop_streaming(struct vb2_queue *vq)
+{
+	struct vimc_vid_device *vout = vb2_get_drv_priv(vq);
+
+	vout->sequence = 0;
+
+	kthread_stop(vout->kthread_fd);
+
+	vimc_vid_return_all_buffers(vout, VB2_BUF_STATE_ERROR);
+}
+
+static const struct vb2_ops vimc_out_qops = {
+	.start_streaming	= vimc_out_start_streaming,
+	.stop_streaming		= vimc_out_stop_streaming,
+	.buf_queue		= vimc_vid_buf_queue,
+	.queue_setup		= vimc_vid_queue_setup,
+	.buf_prepare		= vimc_vid_buffer_prepare,
+	.buf_out_validate	= vimc_vid_buffer_validate,
+	/*
+	 * Since q->lock is set we can use the standard
+	 * vb2_ops_wait_prepare/finish helper functions.
+	 */
+	.wait_prepare		= vb2_ops_wait_prepare,
+	.wait_finish		= vb2_ops_wait_finish,
+};
+
+static const struct media_entity_operations vimc_out_mops = {
+	.link_validate		= vimc_link_validate,
+};
+
+/**
+ * vimc_out_process_frame - process a frame on media pipeline
+ *
+ * @ved:   the vimc entity structure to process the frame
+ * @frame: pointer to the pipeline frame. Unused by output
+ *
+ * If there's a frame to be sent to the pipeline, copy the newest frame and
+ * send it through the pipeline.
+ * If there's no frame on vimc_output buffer list (this probably means that
+ * there's no streaming going on at the output) or the pixelformat of the stream
+ * mismatch the output format, send NULL.
+ */
+static void *vimc_out_process_frame(struct vimc_ent_device *ved,
+						 const void *frame)
+{
+	struct vimc_vid_device *vout = container_of(ved, struct vimc_vid_device,
+						    ved);
+	struct vimc_vid_buffer *vimc_buf;
+	void *vbuf;
+
+	if (ved->stream->producer_pixfmt != vout->format.pixelformat)
+		return NULL;
+
+	spin_lock(&vout->qlock);
+	vimc_buf = list_first_entry_or_null(&vout->buf_list,
+					    typeof(*vimc_buf), list);
+	spin_unlock(&vout->qlock);
+
+	if (!vimc_buf)
+		return NULL;
+
+	vbuf = vb2_plane_vaddr(&vimc_buf->vb2.vb2_buf, 0);
+	memcpy(vout->frame, vbuf, vout->format.sizeimage);
+
+	return vout->frame;
+}
+
+/**
+ * vimc_out_s_stream - callback before/after the start/stop of pipeline stream
+ *
+ * @ved:	the vimc_ent_device compenent of vimc_vid_device
+ * @enable:	0 means that the streaming has stop, non-zero means the
+ *		will begin
+ *
+ */
+static int vimc_out_s_stream(struct vimc_ent_device *ved, int enable)
+{
+	struct vimc_vid_device *vout =
+				container_of(ved, struct vimc_vid_device, ved);
+
+	if (enable) {
+		vout->frame = vmalloc(vout->format.sizeimage);
+		if (!vout->frame)
+			return -ENOMEM;
+
+	} else {
+		vfree(vout->frame);
+		vout->frame = NULL;
+	}
+
+	return 0;
+}
+
+static int vimc_out_comp_bind(struct device *comp, struct device *master,
+		void *master_data)
+{
+	struct v4l2_device *v4l2_dev = master_data;
+	struct vimc_platform_data *pdata = comp->platform_data;
+	struct vimc_vid_device *vout;
+	struct video_device *vdev;
+	struct vb2_queue *q;
+	int ret;
+
+	/* Allocate the vimc_vid_device struct */
+	vout = kzalloc(sizeof(*vout), GFP_KERNEL);
+	if (!vout)
+		return -ENOMEM;
+
+	vout->stream.pipe_size = 0;
+	/* Allocate the pads */
+	vout->ved.pads =
+		vimc_pads_init(1, (const unsigned long[1]) {MEDIA_PAD_FL_SOURCE});
+	if (IS_ERR(vout->ved.pads)) {
+		ret = PTR_ERR(vout->ved.pads);
+		goto err_free_vout;
+	}
+
+	/* Initialize the media entity */
+	vout->vdev.entity.name = pdata->entity_name;
+	vout->vdev.entity.function = MEDIA_ENT_F_IO_V4L;
+	ret = media_entity_pads_init(&vout->vdev.entity,
+				     1, vout->ved.pads);
+	if (ret)
+		goto err_clean_pads;
+
+	/* Initialize the lock */
+	mutex_init(&vout->lock);
+
+	/* Initialize the vb2 queue */
+	q = &vout->queue;
+	q->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
+	q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
+	q->drv_priv = vout;
+	q->buf_struct_size = sizeof(struct vimc_vid_buffer);
+	q->ops = &vimc_out_qops;
+	q->mem_ops = &vb2_vmalloc_memops;
+	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	q->min_buffers_needed = 2;
+	q->lock = &vout->lock;
+
+	ret = vb2_queue_init(q);
+	if (ret) {
+		dev_err(comp, "%s: vb2 queue init failed (err=%d)\n",
+				pdata->entity_name, ret);
+		goto err_clean_m_ent;
+	}
+
+	/* Initialize buffer list and its lock */
+	INIT_LIST_HEAD(&vout->buf_list);
+	spin_lock_init(&vout->qlock);
+
+	/* Set default frame format */
+	vout->format = fmt_default;
+	v4l2_fill_pixfmt(&vout->format, vout->format.pixelformat,
+			 vout->format.width, vout->format.height);
+
+	/* Fill the vimc_ent_device struct */
+	vout->ved.ent = &vout->vdev.entity;
+	vout->ved.process_frame = vimc_out_process_frame;
+	vout->ved.s_stream = vimc_out_s_stream;
+	vout->ved.vdev_get_format = vimc_vid_get_format;
+	dev_set_drvdata(comp, &vout->ved);
+	vout->dev = comp;
+
+	/* Initialize the video_device struct */
+	vdev = &vout->vdev;
+	vdev->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
+	vdev->entity.ops = &vimc_out_mops;
+	vdev->release = vimc_vid_release;
+	vdev->fops = &vimc_vid_fops;
+	vdev->ioctl_ops = &vimc_out_ioctl_ops;
+	vdev->lock = &vout->lock;
+	vdev->queue = q;
+	vdev->v4l2_dev = v4l2_dev;
+	vdev->vfl_dir = VFL_DIR_TX;
+	strscpy(vdev->name, pdata->entity_name, sizeof(vdev->name));
+	video_set_drvdata(vdev, &vout->ved);
+
+	/* Register the video_device with the v4l2 and the media framework */
+	ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
+	if (ret) {
+		dev_err(comp, "%s: video register failed (err=%d)\n",
+				vout->vdev.name, ret);
+		goto err_release_queue;
+	}
+
+	return 0;
+
+err_release_queue:
+	vb2_queue_release(q);
+err_clean_m_ent:
+	media_entity_cleanup(&vout->vdev.entity);
+err_clean_pads:
+	vimc_pads_cleanup(vout->ved.pads);
+err_free_vout:
+	kfree(vout);
+
+	return ret;
+}
+
+static const struct component_ops vimc_out_comp_ops = {
+	.bind = vimc_out_comp_bind,
+	.unbind = vimc_vid_comp_unbind,
+};
+
+static int vimc_out_probe(struct platform_device *pdev)
+{
+	return component_add(&pdev->dev, &vimc_out_comp_ops);
+}
+
+static int vimc_out_remove(struct platform_device *pdev)
+{
+	component_del(&pdev->dev, &vimc_out_comp_ops);
+
+	return 0;
+}
+
+static const struct platform_device_id vimc_out_driver_ids[] = {
+	{
+		.name           = VIMC_OUT_DRV_NAME,
+	},
+	{ }
+};
+
+static struct platform_driver vimc_out_pdrv = {
+	.probe		= vimc_out_probe,
+	.remove		= vimc_out_remove,
+	.id_table	= vimc_out_driver_ids,
+	.driver		= {
+		.name	= VIMC_OUT_DRV_NAME,
+	},
+};
+
+module_platform_driver(vimc_out_pdrv);
+
+MODULE_DEVICE_TABLE(platform, vimc_out_driver_ids);
+
+MODULE_DESCRIPTION("Virtual Media Controller Driver (VIMC) Output");
+MODULE_AUTHOR("André Almeida <andrealmeid@collabora.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c
index c7123a45c55b..2c3e486a29c0 100644
--- a/drivers/media/platform/vimc/vimc-scaler.c
+++ b/drivers/media/platform/vimc/vimc-scaler.c
@@ -194,9 +194,10 @@ static const struct v4l2_subdev_pad_ops vimc_sca_pad_ops = {
 	.set_fmt		= vimc_sca_set_fmt,
 };
 
-static int vimc_sca_s_stream(struct v4l2_subdev *sd, int enable)
+static int vimc_sca_s_stream(struct vimc_ent_device *ved, int enable)
 {
-	struct vimc_sca_device *vsca = v4l2_get_subdevdata(sd);
+	struct vimc_sca_device *vsca = container_of(ved, struct vimc_sca_device,
+						    ved);
 
 	if (enable) {
 		u32 pixelformat = vsca->ved.stream->producer_pixfmt;
@@ -239,13 +240,9 @@ static int vimc_sca_s_stream(struct v4l2_subdev *sd, int enable)
 	return 0;
 }
 
-static const struct v4l2_subdev_video_ops vimc_sca_video_ops = {
-	.s_stream = vimc_sca_s_stream,
-};
 
 static const struct v4l2_subdev_ops vimc_sca_ops = {
 	.pad = &vimc_sca_pad_ops,
-	.video = &vimc_sca_video_ops,
 };
 
 static void vimc_sca_fill_pix(u8 *const ptr,
@@ -381,6 +378,7 @@ static int vimc_sca_comp_bind(struct device *comp, struct device *master,
 	}
 
 	vsca->ved.process_frame = vimc_sca_process_frame;
+	vsca->ved.s_stream = vimc_sca_s_stream,
 	dev_set_drvdata(comp, &vsca->ved);
 	vsca->dev = comp;
 
diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
index 51359472eef2..a1afb840493d 100644
--- a/drivers/media/platform/vimc/vimc-sensor.c
+++ b/drivers/media/platform/vimc/vimc-sensor.c
@@ -177,10 +177,10 @@ static void *vimc_sen_process_frame(struct vimc_ent_device *ved,
 	return vsen->frame;
 }
 
-static int vimc_sen_s_stream(struct v4l2_subdev *sd, int enable)
+static int vimc_sen_s_stream(struct vimc_ent_device *ved, int enable)
 {
 	struct vimc_sen_device *vsen =
-				container_of(sd, struct vimc_sen_device, sd);
+				container_of(ved, struct vimc_sen_device, ved);
 
 	if (enable) {
 		u32 pixelformat = vsen->ved.stream->producer_pixfmt;
@@ -218,14 +218,9 @@ static const struct v4l2_subdev_core_ops vimc_sen_core_ops = {
 	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
 };
 
-static const struct v4l2_subdev_video_ops vimc_sen_video_ops = {
-	.s_stream = vimc_sen_s_stream,
-};
-
 static const struct v4l2_subdev_ops vimc_sen_ops = {
 	.core = &vimc_sen_core_ops,
 	.pad = &vimc_sen_pad_ops,
-	.video = &vimc_sen_video_ops,
 };
 
 static int vimc_sen_s_ctrl(struct v4l2_ctrl *ctrl)
@@ -351,6 +346,7 @@ static int vimc_sen_comp_bind(struct device *comp, struct device *master,
 		goto err_free_hdl;
 
 	vsen->ved.process_frame = vimc_sen_process_frame;
+	vsen->ved.s_stream = vimc_sen_s_stream;
 	dev_set_drvdata(comp, &vsen->ved);
 	vsen->dev = comp;
 
diff --git a/drivers/media/platform/vimc/vimc-streamer.c b/drivers/media/platform/vimc/vimc-streamer.c
index 3b3f36357a0e..fd330415ac38 100644
--- a/drivers/media/platform/vimc/vimc-streamer.c
+++ b/drivers/media/platform/vimc/vimc-streamer.c
@@ -47,7 +47,6 @@ static struct media_entity *vimc_get_source_entity(struct media_entity *ent)
 static void vimc_streamer_pipeline_terminate(struct vimc_stream *stream)
 {
 	struct vimc_ent_device *ved;
-	struct v4l2_subdev *sd;
 
 	while (stream->pipe_size) {
 		stream->pipe_size--;
@@ -55,11 +54,8 @@ static void vimc_streamer_pipeline_terminate(struct vimc_stream *stream)
 		ved->stream = NULL;
 		stream->ved_pipeline[stream->pipe_size] = NULL;
 
-		if (!is_media_entity_v4l2_subdev(ved->ent))
-			continue;
-
-		sd = media_entity_to_v4l2_subdev(ved->ent);
-		v4l2_subdev_call(sd, video, s_stream, 0);
+		if (ved->s_stream)
+			ved->s_stream(ved, 0);
 	}
 }
 
@@ -79,7 +75,6 @@ static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
 	struct media_entity *entity;
 	struct video_device *vdev;
 	struct v4l2_subdev *sd;
-	int ret = 0;
 
 	stream->pipe_size = 0;
 	while (stream->pipe_size < VIMC_STREAMER_PIPELINE_MAX_SIZE) {
@@ -90,16 +85,8 @@ static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
 		stream->ved_pipeline[stream->pipe_size++] = ved;
 		ved->stream = stream;
 
-		if (is_media_entity_v4l2_subdev(ved->ent)) {
-			sd = media_entity_to_v4l2_subdev(ved->ent);
-			ret = v4l2_subdev_call(sd, video, s_stream, 1);
-			if (ret && ret != -ENOIOCTLCMD) {
-				pr_err("subdev_call error %s\n",
-				       ved->ent->name);
-				vimc_streamer_pipeline_terminate(stream);
-				return ret;
-			}
-		}
+		if (ved->s_stream)
+			ved->s_stream(ved, 1);
 
 		entity = vimc_get_source_entity(ved->ent);
 		/* Check if the end of the pipeline was reached*/
@@ -149,6 +136,7 @@ static int vimc_streamer_thread(void *data)
 			if (!frame || IS_ERR(frame))
 				break;
 		}
+
 		//wait for 60hz
 		set_current_state(TASK_UNINTERRUPTIBLE);
 		schedule_timeout(HZ / 60);
diff --git a/drivers/media/platform/vimc/vimc-video.c b/drivers/media/platform/vimc/vimc-video.c
index f7ccb2e9d6c5..0187e699543a 100644
--- a/drivers/media/platform/vimc/vimc-video.c
+++ b/drivers/media/platform/vimc/vimc-video.c
@@ -236,6 +236,15 @@ int vimc_vid_buffer_prepare(struct vb2_buffer *vb)
 }
 EXPORT_SYMBOL_GPL(vimc_vid_buffer_prepare);
 
+int vimc_vid_buffer_validate(struct vb2_buffer *vb)
+{
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+
+	vbuf->field = V4L2_FIELD_NONE;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(vimc_vid_buffer_validate);
+
 void vimc_vid_release(struct video_device *vdev)
 {
 	struct vimc_vid_device *vid =
diff --git a/drivers/media/platform/vimc/vimc-video.h b/drivers/media/platform/vimc/vimc-video.h
index 22cb0e2dbdbb..67f13244bf92 100644
--- a/drivers/media/platform/vimc/vimc-video.h
+++ b/drivers/media/platform/vimc/vimc-video.h
@@ -116,6 +116,8 @@ int vimc_vid_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
 
 int vimc_vid_buffer_prepare(struct vb2_buffer *vb);
 
+int vimc_vid_buffer_validate(struct vb2_buffer *vb);
+
 int vimc_vid_probe(struct platform_device *pdev);
 
 int vimc_vid_remove(struct platform_device *pdev);
-- 
2.22.0


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

* [PATCH 4/7] media: vimc: Send null buffer through the pipeline
  2019-07-02 15:47 [PATCH 0/7] media: vimc: Add a V4L2 output device André Almeida
                   ` (2 preceding siblings ...)
  2019-07-02 15:47 ` [PATCH 3/7] media: vimc: Create a V4L2 output device André Almeida
@ 2019-07-02 15:47 ` André Almeida
  2019-07-02 15:47 ` [PATCH 5/7] media: vimc: core: Add output device on " André Almeida
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: André Almeida @ 2019-07-02 15:47 UTC (permalink / raw)
  To: linux-media
  Cc: mchehab, hverkuil, helen.koike, kernel, linux-kernel, André Almeida

Send a NULL buffer through the video pipeline. If the Capture device
gets a NULL buffer, it uses it default fallback frame. Make the capture
device behave more like real devices when there's no frame to show.

Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
 drivers/media/platform/vimc/vimc-capture.c  | 15 +++++++++++++++
 drivers/media/platform/vimc/vimc-debayer.c  |  3 +++
 drivers/media/platform/vimc/vimc-scaler.c   |  3 +++
 drivers/media/platform/vimc/vimc-streamer.c |  2 +-
 4 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c
index e80fa1ee3dc1..4c65bf73530f 100644
--- a/drivers/media/platform/vimc/vimc-capture.c
+++ b/drivers/media/platform/vimc/vimc-capture.c
@@ -37,6 +37,15 @@ static int vimc_cap_start_streaming(struct vb2_queue *vq, unsigned int count)
 
 	vcap->sequence = 0;
 
+	/* this is a fallback frame, it will be used if the pipeline
+	 * is sending NULL frames
+	 */
+	vcap->frame = vmalloc(vcap->format.sizeimage);
+	if (!vcap->frame) {
+		vimc_vid_return_all_buffers(vcap, VB2_BUF_STATE_QUEUED);
+		return -ENOMEM;
+	}
+
 	/* Start the media pipeline */
 	ret = media_pipeline_start(entity, &vcap->stream.pipe);
 	if (ret) {
@@ -65,6 +74,9 @@ void vimc_cap_stop_streaming(struct vb2_queue *vq)
 
 	vimc_streamer_s_stream(&vcap->stream, &vcap->ved, 0);
 
+	vfree(vcap->frame);
+	vcap->frame = NULL;
+
 	media_pipeline_stop(&vcap->vdev.entity);
 
 	vimc_vid_return_all_buffers(vcap, VB2_BUF_STATE_ERROR);
@@ -96,6 +108,9 @@ static void *vimc_cap_process_frame(struct vimc_ent_device *ved,
 	struct vimc_vid_buffer *vimc_buf;
 	void *vbuf;
 
+	if (!frame)
+		frame = &vcap->frame;
+
 	spin_lock(&vcap->qlock);
 
 	/* Get the first entry of the list */
diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
index 9e0c7c2c3c72..d5f370f94573 100644
--- a/drivers/media/platform/vimc/vimc-debayer.c
+++ b/drivers/media/platform/vimc/vimc-debayer.c
@@ -483,6 +483,9 @@ static void *vimc_deb_process_frame(struct vimc_ent_device *ved,
 	unsigned int rgb[3];
 	unsigned int i, j;
 
+	if (!sink_frame)
+		return NULL;
+
 	/* If the stream in this node is not active, just return */
 	if (!vdeb->src_frame)
 		return ERR_PTR(-EINVAL);
diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c
index 2c3e486a29c0..9edad3b14526 100644
--- a/drivers/media/platform/vimc/vimc-scaler.c
+++ b/drivers/media/platform/vimc/vimc-scaler.c
@@ -320,6 +320,9 @@ static void *vimc_sca_process_frame(struct vimc_ent_device *ved,
 	struct vimc_sca_device *vsca = container_of(ved, struct vimc_sca_device,
 						    ved);
 
+	if (!sink_frame)
+		return NULL;
+
 	/* If the stream in this node is not active, just return */
 	if (!ved->stream)
 		return ERR_PTR(-EINVAL);
diff --git a/drivers/media/platform/vimc/vimc-streamer.c b/drivers/media/platform/vimc/vimc-streamer.c
index fd330415ac38..f06c9308a41b 100644
--- a/drivers/media/platform/vimc/vimc-streamer.c
+++ b/drivers/media/platform/vimc/vimc-streamer.c
@@ -133,7 +133,7 @@ static int vimc_streamer_thread(void *data)
 		for (i = stream->pipe_size - 1; i >= 0; i--) {
 			frame = stream->ved_pipeline[i]->process_frame(
 					stream->ved_pipeline[i], frame);
-			if (!frame || IS_ERR(frame))
+			if (IS_ERR(frame))
 				break;
 		}
 
-- 
2.22.0


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

* [PATCH 5/7] media: vimc: core: Add output device on the pipeline
  2019-07-02 15:47 [PATCH 0/7] media: vimc: Add a V4L2 output device André Almeida
                   ` (3 preceding siblings ...)
  2019-07-02 15:47 ` [PATCH 4/7] media: vimc: Send null buffer through the pipeline André Almeida
@ 2019-07-02 15:47 ` André Almeida
  2019-07-09 22:20   ` Helen Koike
  2019-07-02 15:47 ` [PATCH 6/7] media: vimc.dot: Update default topology diagram André Almeida
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: André Almeida @ 2019-07-02 15:47 UTC (permalink / raw)
  To: linux-media
  Cc: mchehab, hverkuil, helen.koike, kernel, linux-kernel, André Almeida

Add the output video device on the hardcoded pipeline. Change the link
to it be enabled by default.

Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
 drivers/media/platform/vimc/vimc-core.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-core.c b/drivers/media/platform/vimc/vimc-core.c
index 571c55aa0e16..ecdea1d631c5 100644
--- a/drivers/media/platform/vimc/vimc-core.c
+++ b/drivers/media/platform/vimc/vimc-core.c
@@ -95,8 +95,7 @@ static const struct vimc_ent_config ent_config[] = {
 	},
 	{
 		.name = "RGB/YUV Input",
-		/* TODO: change this to vimc-input when it is implemented */
-		.drv = "vimc-sensor",
+		.drv = "vimc-output",
 	},
 	{
 		.name = "Scaler",
@@ -118,11 +117,11 @@ static const struct vimc_ent_link ent_links[] = {
 	/* Link: Sensor B (Pad 0)->(Pad 0) Raw Capture 1 */
 	VIMC_ENT_LINK(1, 0, 5, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
 	/* Link: Debayer A (Pad 1)->(Pad 0) Scaler */
-	VIMC_ENT_LINK(2, 1, 7, 0, MEDIA_LNK_FL_ENABLED),
+	VIMC_ENT_LINK(2, 1, 7, 0, 0),
 	/* Link: Debayer B (Pad 1)->(Pad 0) Scaler */
 	VIMC_ENT_LINK(3, 1, 7, 0, 0),
 	/* Link: RGB/YUV Input (Pad 0)->(Pad 0) Scaler */
-	VIMC_ENT_LINK(6, 0, 7, 0, 0),
+	VIMC_ENT_LINK(6, 0, 7, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
 	/* Link: Scaler (Pad 1)->(Pad 0) RGB/YUV Capture */
 	VIMC_ENT_LINK(7, 1, 8, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
 };
-- 
2.22.0


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

* [PATCH 6/7] media: vimc.dot: Update default topology diagram
  2019-07-02 15:47 [PATCH 0/7] media: vimc: Add a V4L2 output device André Almeida
                   ` (4 preceding siblings ...)
  2019-07-02 15:47 ` [PATCH 5/7] media: vimc: core: Add output device on " André Almeida
@ 2019-07-02 15:47 ` André Almeida
  2019-07-02 15:47 ` [PATCH 7/7] media: vimc.rst: Add output device André Almeida
  2019-07-09 22:19 ` [PATCH 0/7] media: vimc: Add a V4L2 " Helen Koike
  7 siblings, 0 replies; 15+ messages in thread
From: André Almeida @ 2019-07-02 15:47 UTC (permalink / raw)
  To: linux-media
  Cc: mchehab, hverkuil, helen.koike, kernel, linux-kernel, André Almeida

Update the default topology diagram to reflect the current state of the
driver.

Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
 Documentation/media/v4l-drivers/vimc.dot | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/media/v4l-drivers/vimc.dot b/Documentation/media/v4l-drivers/vimc.dot
index 57863a13fa39..b40c151ff790 100644
--- a/Documentation/media/v4l-drivers/vimc.dot
+++ b/Documentation/media/v4l-drivers/vimc.dot
@@ -9,13 +9,13 @@ digraph board {
 	n00000003:port0 -> n00000008:port0 [style=bold]
 	n00000003:port0 -> n0000000f [style=bold]
 	n00000005 [label="{{<port0> 0} | Debayer A\n/dev/v4l-subdev2 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
-	n00000005:port1 -> n00000017:port0
+	n00000005:port1 -> n00000017:port0 [style=dashed]
 	n00000008 [label="{{<port0> 0} | Debayer B\n/dev/v4l-subdev3 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
 	n00000008:port1 -> n00000017:port0 [style=dashed]
 	n0000000b [label="Raw Capture 0\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
 	n0000000f [label="Raw Capture 1\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
 	n00000013 [label="RGB/YUV Input\n/dev/video2", shape=box, style=filled, fillcolor=yellow]
-	n00000013 -> n00000017:port0 [style=dashed]
+	n00000013 -> n00000017:port0 [style=bold]
 	n00000017 [label="{{<port0> 0} | Scaler\n/dev/v4l-subdev4 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
 	n00000017:port1 -> n0000001a [style=bold]
 	n0000001a [label="RGB/YUV Capture\n/dev/video3", shape=box, style=filled, fillcolor=yellow]
-- 
2.22.0


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

* [PATCH 7/7] media: vimc.rst: Add output device
  2019-07-02 15:47 [PATCH 0/7] media: vimc: Add a V4L2 output device André Almeida
                   ` (5 preceding siblings ...)
  2019-07-02 15:47 ` [PATCH 6/7] media: vimc.dot: Update default topology diagram André Almeida
@ 2019-07-02 15:47 ` André Almeida
  2019-07-09 22:19 ` [PATCH 0/7] media: vimc: Add a V4L2 " Helen Koike
  7 siblings, 0 replies; 15+ messages in thread
From: André Almeida @ 2019-07-02 15:47 UTC (permalink / raw)
  To: linux-media
  Cc: mchehab, hverkuil, helen.koike, kernel, linux-kernel, André Almeida

Add information about the output device. Remove wrong information about
what the capture exposes.

Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
 Documentation/media/v4l-drivers/vimc.rst | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/Documentation/media/v4l-drivers/vimc.rst b/Documentation/media/v4l-drivers/vimc.rst
index 4628b12d417f..ccc04101ea51 100644
--- a/Documentation/media/v4l-drivers/vimc.rst
+++ b/Documentation/media/v4l-drivers/vimc.rst
@@ -4,7 +4,8 @@ The Virtual Media Controller Driver (vimc)
 ==========================================
 
 The vimc driver emulates complex video hardware using the V4L2 API and the Media
-API. It has a capture device and three subdevices: sensor, debayer and scaler.
+API. It has a capture device, an output device and three subdevices: sensor,
+debayer and scaler.
 
 Topology
 --------
@@ -40,6 +41,7 @@ of commands fits for the default topology:
         v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440
         v4l2-ctl -z platform:vimc -d "Raw Capture 0" -v pixelformat=BA81
         v4l2-ctl -z platform:vimc -d "Raw Capture 1" -v pixelformat=BA81
+        v4l2-ctl -z platform:vimc -e "RGB/YUV Input" -v width=640,height=480
 
 Subdevices
 ----------
@@ -74,7 +76,13 @@ vimc-capture:
 	Exposes:
 
 	* 1 Pad sink
-	* 1 Pad source
+
+vimc-output:
+        Exposes node /dev/videoX to allow userspace to send frames to the
+        stream.
+        Exposes:
+
+        * 1 Source sink
 
 Module options
 ---------------
-- 
2.22.0


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

* Re: [PATCH 0/7] media: vimc: Add a V4L2 output device
  2019-07-02 15:47 [PATCH 0/7] media: vimc: Add a V4L2 output device André Almeida
                   ` (6 preceding siblings ...)
  2019-07-02 15:47 ` [PATCH 7/7] media: vimc.rst: Add output device André Almeida
@ 2019-07-09 22:19 ` Helen Koike
  2019-07-10  7:33   ` Hans Verkuil
  7 siblings, 1 reply; 15+ messages in thread
From: Helen Koike @ 2019-07-09 22:19 UTC (permalink / raw)
  To: André Almeida, linux-media; +Cc: mchehab, hverkuil, kernel, linux-kernel

Hi André,

Thanks for the patches.

On 7/2/19 12:47 PM, André Almeida wrote:
> Hello,
> 
> This patch adds a V4L2 output device on vimc, that comply with V4L2 API
> for video output. If there is an output device and a capture device at the
> same pipeline, one can get a video loopback pipeline feeding frames at
> the output and then seeing them at the capture. It's possible to insert
> vimc submodules at the pipeline to modify the image (e.g. a scaler).
> 
> If one starts a streaming at the capture, with the output off, the
> capture will display a noisy frame. If one starts a streaming at the
> output with the capture off, the output will just consume the buffers,
> without sending them to the pipeline. If both output and capture are
> streaming, the loopback will happen.

I understand why it is done like this in vivid, but I was wondering, if we
have a pipeline like:
output -> capture
Shouldn't streaming from the capture just stalls if there is no frame
available in the output (i.e. streaming in the output is off) ? But then I'm
not sure what the framerate in the capture would mean.

Hans, what do you think?

Thanks,
Helen

> 
> The patches 1 and 2 provide some ground to create the output
> device. The patch 3 creates the device and modify how the vimc-streamer
> was dealing with the s_stream callback on other vimc modules, to make
> simpler implementing this callback at vimc-output. Patch 4 change the
> behavior of the pipeline in order to be closer to a real life hardware.
> Patches 5-7 updates the default pipeline and the documentation to
> include the new output device.
> 
> This is the result of v4l2-compliance after this patch series:
> $ v4l2-compliance -m0 -s50
> Grand Total for vimc device /dev/media0: 476, Succeeded: 476, Failed: 0,
> Warnings: 0
> 
> A git tree up to date with media-master and with this changes can be found
> at: https://gitlab.collabora.com/tonyk/linux/tree/vimc/output
> 
> In order to test it, one can follow these instructions:
> 
> 1 - Configure the pipeline (requires v4l-utils):
> 
> $ media-ctl -d platform:vimc -V '"Sensor A":0[fmt:SBGGR8_1X8/640x480]'
> $ media-ctl -d platform:vimc -V '"Debayer A":0[fmt:SBGGR8_1X8/640x480]'
> $ media-ctl -d platform:vimc -V '"Sensor B":0[fmt:SBGGR8_1X8/640x480]'
> $ media-ctl -d platform:vimc -V '"Debayer B":0[fmt:SBGGR8_1X8/640x480]'
> $ v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440
> $ v4l2-ctl -z platform:vimc -d "Raw Capture 0" -v pixelformat=BA81
> $ v4l2-ctl -z platform:vimc -d "Raw Capture 1" -v pixelformat=BA81
> $ v4l2-ctl -z platform:vimc -e "RGB/YUV Input" -v width=640,height=480
> 
> 2 - Use a userspace application:
> 2.a gst-launch (requires gstreamer and gst-plugins-good):
> 
> Feed frames into the output and grab from the capture (rescaled for
> convenience):
> 
> $ gst-launch-1.0 videotestsrc pattern=ball ! \
> 	video/x-raw,width=640,height=480,format=RGB \
> 	! v4l2sink device=/dev/video2 v4l2src device=/dev/video3 ! \
> 	video/x-raw,width=1920,height=1440,format=RGB ! videoscale ! \
> 	video/x-raw,width=640,height=480 ! videoconvert ! ximagesink
> 
> 2.b qv4l2 (requires v4l-utils):
> 
> Open the output device:
> 
> $ qv4l2 -d2
> 
> Open the capture device:
> 
> $ qv4l2 -d3
> 
> Start the streaming at both, at any order. You can change the frame
> content at "Test Pattern Generator" -> "Test Pattern" on the output.
> 
> Thanks,
> 	André
> 
> André Almeida (7):
>   media: vimc: Create video module
>   media: vimc: video: Add write file operation
>   media: vimc: Create a V4L2 output device
>   media: vimc: Send null buffer through the pipeline
>   media: vimc: core: Add output device on the pipeline
>   media: vimc.dot: Update default topology diagram
>   media: vimc.rst: Add output device
> 
>  Documentation/media/v4l-drivers/vimc.dot    |   4 +-
>  Documentation/media/v4l-drivers/vimc.rst    |  12 +-
>  drivers/media/platform/vimc/Makefile        |   4 +-
>  drivers/media/platform/vimc/vimc-capture.c  | 356 +++----------------
>  drivers/media/platform/vimc/vimc-common.h   |   5 +-
>  drivers/media/platform/vimc/vimc-core.c     |   7 +-
>  drivers/media/platform/vimc/vimc-debayer.c  |  14 +-
>  drivers/media/platform/vimc/vimc-output.c   | 362 ++++++++++++++++++++
>  drivers/media/platform/vimc/vimc-scaler.c   |  13 +-
>  drivers/media/platform/vimc/vimc-sensor.c   |  10 +-
>  drivers/media/platform/vimc/vimc-streamer.c |  24 +-
>  drivers/media/platform/vimc/vimc-video.c    | 273 +++++++++++++++
>  drivers/media/platform/vimc/vimc-video.h    | 130 +++++++
>  13 files changed, 849 insertions(+), 365 deletions(-)
>  create mode 100644 drivers/media/platform/vimc/vimc-output.c
>  create mode 100644 drivers/media/platform/vimc/vimc-video.c
>  create mode 100644 drivers/media/platform/vimc/vimc-video.h
> 

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

* Re: [PATCH 3/7] media: vimc: Create a V4L2 output device
  2019-07-02 15:47 ` [PATCH 3/7] media: vimc: Create a V4L2 output device André Almeida
@ 2019-07-09 22:20   ` Helen Koike
  0 siblings, 0 replies; 15+ messages in thread
From: Helen Koike @ 2019-07-09 22:20 UTC (permalink / raw)
  To: André Almeida, linux-media; +Cc: mchehab, hverkuil, kernel, linux-kernel

Hi André,

Thanks for the patch, just some minor comments below.

On 7/2/19 12:47 PM, André Almeida wrote:
> Using the functions on vimc-video, create a V4L2 output device.
> When a streaming is initialized on the output device, it'll start
> returning those buffers on to the userspace, while always keeping one on
> the list. When the capture device starts streaming on the same pipeline,
> it will copy the first frame on the list to the process_frame pipeline.
> 
> The vimc-streamer has a callback to warn all subdevices when a streaming
> will start or stop. This is useful so the subdevices can allocate a
> frame with suitable size to send through the pipeline, and then free
> then when it not used anymore. Since the vimc-output will also be part
> of the pipeline (but it is not a V4L2 subdevice), change the callback to
> be part of vimc_ent_device, rather than a linked with the subdevice API.
> In this way, it's possible to have a generic solution to call start/stop
> streaming callbacks to all devices in the pipeline.
> 
> Add a vb2 buffer validate function. This is required to
> create a V4L2 output device.
> 
> Signed-off-by: André Almeida <andrealmeid@collabora.com>
> ---
>  drivers/media/platform/vimc/Makefile        |   4 +-
>  drivers/media/platform/vimc/vimc-common.h   |   5 +-
>  drivers/media/platform/vimc/vimc-debayer.c  |  11 +-
>  drivers/media/platform/vimc/vimc-output.c   | 362 ++++++++++++++++++++
>  drivers/media/platform/vimc/vimc-scaler.c   |  10 +-
>  drivers/media/platform/vimc/vimc-sensor.c   |  10 +-
>  drivers/media/platform/vimc/vimc-streamer.c |  22 +-
>  drivers/media/platform/vimc/vimc-video.c    |   9 +
>  drivers/media/platform/vimc/vimc-video.h    |   2 +
>  9 files changed, 395 insertions(+), 40 deletions(-)
>  create mode 100644 drivers/media/platform/vimc/vimc-output.c
> 
> diff --git a/drivers/media/platform/vimc/Makefile b/drivers/media/platform/vimc/Makefile
> index fb90aa0f33a5..611331b9fceb 100644
> --- a/drivers/media/platform/vimc/Makefile
> +++ b/drivers/media/platform/vimc/Makefile
> @@ -1,5 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
>  vimc-y := vimc-core.o vimc-common.o vimc-streamer.o
>  
> -obj-$(CONFIG_VIDEO_VIMC) += vimc.o vimc-video.o vimc-capture.o vimc-debayer.o \
> -                vimc-scaler.o vimc-sensor.o
> +obj-$(CONFIG_VIDEO_VIMC) += vimc.o vimc-video.o vimc-capture.o vimc-output.o \
> +                vimc-debayer.o vimc-scaler.o vimc-sensor.o
> diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h
> index 7b4d988b208b..003024bcdaed 100644
> --- a/drivers/media/platform/vimc/vimc-common.h
> +++ b/drivers/media/platform/vimc/vimc-common.h
> @@ -78,6 +78,8 @@ struct vimc_platform_data {
>   * @vdev_get_format:	callback that returns the current format a pad, used
>   *			only when is_media_entity_v4l2_video_device(ent) returns
>   *			true
> + * @s_stream:		callback used when vimc-capture will start/stop a
> + *			streaming, to subdevice alloc/free it frames

I would re-word this a bit:
callback used when vimc-capture start/stop streaming, so subdevices can alloc/free frames.

>   *
>   * Each node of the topology must create a vimc_ent_device struct. Depending on
>   * the node it will be of an instance of v4l2_subdev or video_device struct
> @@ -94,7 +96,8 @@ struct vimc_ent_device {
>  	void * (*process_frame)(struct vimc_ent_device *ved,
>  				const void *frame);
>  	void (*vdev_get_format)(struct vimc_ent_device *ved,
> -			      struct v4l2_pix_format *fmt);
> +				struct v4l2_pix_format *fmt);
> +	int (*s_stream)(struct vimc_ent_device *ved, int enable);
>  };
>  
>  /**
> diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
> index 00598fbf3cba..9e0c7c2c3c72 100644
> --- a/drivers/media/platform/vimc/vimc-debayer.c
> +++ b/drivers/media/platform/vimc/vimc-debayer.c
> @@ -320,9 +320,10 @@ static void vimc_deb_set_rgb_pix_rgb24(struct vimc_deb_device *vdeb,
>  		vdeb->src_frame[index + i] = rgb[i];
>  }
>  
> -static int vimc_deb_s_stream(struct v4l2_subdev *sd, int enable)
> +static int vimc_deb_s_stream(struct vimc_ent_device *ved, int enable)
>  {
> -	struct vimc_deb_device *vdeb = v4l2_get_subdevdata(sd);
> +	struct vimc_deb_device *vdeb = container_of(ved, struct vimc_deb_device,
> +						    ved);
>  
>  	if (enable) {
>  		u32 src_pixelformat = vdeb->ved.stream->producer_pixfmt;
> @@ -373,13 +374,8 @@ static int vimc_deb_s_stream(struct v4l2_subdev *sd, int enable)
>  	return 0;
>  }
>  
> -static const struct v4l2_subdev_video_ops vimc_deb_video_ops = {
> -	.s_stream = vimc_deb_s_stream,
> -};
> -
>  static const struct v4l2_subdev_ops vimc_deb_ops = {
>  	.pad = &vimc_deb_pad_ops,
> -	.video = &vimc_deb_video_ops,
>  };
>  
>  static unsigned int vimc_deb_get_val(const u8 *bytes,
> @@ -549,6 +545,7 @@ static int vimc_deb_comp_bind(struct device *comp, struct device *master,
>  	}
>  
>  	vdeb->ved.process_frame = vimc_deb_process_frame;
> +	vdeb->ved.s_stream = vimc_deb_s_stream;
>  	dev_set_drvdata(comp, &vdeb->ved);
>  	vdeb->dev = comp;
>  
> diff --git a/drivers/media/platform/vimc/vimc-output.c b/drivers/media/platform/vimc/vimc-output.c
> new file mode 100644
> index 000000000000..3fb9441adc43
> --- /dev/null
> +++ b/drivers/media/platform/vimc/vimc-output.c
> @@ -0,0 +1,362 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * vimc-output.c Virtual Media Controller Driver
> + *
> + * Copyright 2019 Collabora, Ltd.
> + */
> +
> +#include "vimc-video.h"
> +#include <linux/kthread.h>
> +#include <linux/freezer.h>
> +
> +#define VIMC_OUT_DRV_NAME "vimc-output"
> +
> +static const struct v4l2_ioctl_ops vimc_out_ioctl_ops = {
> +	.vidioc_querycap = vimc_vid_querycap,
> +
> +	.vidioc_g_fmt_vid_out = vimc_vid_g_fmt,
> +	.vidioc_s_fmt_vid_out = vimc_vid_s_fmt,
> +	.vidioc_try_fmt_vid_out = vimc_vid_try_fmt,
> +	.vidioc_enum_fmt_vid_out = vimc_vid_enum_fmt,
> +	.vidioc_enum_framesizes = vimc_vid_enum_framesizes,
> +
> +	.vidioc_reqbufs = vb2_ioctl_reqbufs,
> +	.vidioc_create_bufs = vb2_ioctl_create_bufs,
> +	.vidioc_prepare_buf = vb2_ioctl_prepare_buf,
> +	.vidioc_querybuf = vb2_ioctl_querybuf,
> +	.vidioc_qbuf = vb2_ioctl_qbuf,
> +	.vidioc_dqbuf = vb2_ioctl_dqbuf,
> +	.vidioc_expbuf = vb2_ioctl_expbuf,
> +	.vidioc_streamon = vb2_ioctl_streamon,
> +	.vidioc_streamoff = vb2_ioctl_streamoff,
> +};
> +
> +/**
> + * vimc_out_consume_frame - grab a frame from the queue and release it
> + *
> + * @arg: *vimc_vid_device passed as argument to kthread
> + *
> + * This function is called when the stream is on. If there's a frame available

s/This function is called/This thread is started/

> + * on the queue, it will get it and "consume" it. This means, marks the frame
> + * as done and make sure there're at least one frame to be on the list.
> + * It should stop when the streaming stops too
> + *
> + * Return: always 0
> + */
> +static int vimc_out_consume_frame(void *arg)
> +{
> +	struct vimc_vid_device *vout = arg;
> +	struct vimc_vid_buffer *vimc_buf;
> +
> +	set_freezable();
> +
> +	for (;;) {
> +		try_to_freeze();
> +		if (kthread_should_stop())
> +			break;
> +
> +		/* TODO: support custom framerate */
> +		set_current_state(TASK_UNINTERRUPTIBLE);
> +		schedule_timeout(HZ / 60);
> +
> +		spin_lock(&vout->qlock);
> +
> +		/*
> +		 * if there's only on frame on the buffer, keep it there
> +		 * this is done in a way that the output always can send a
> +		 * frame to the pipeline, even if the capture framerate is
> +		 * higher than the output framarate

s/framarate/framerate/

> +		 */
> +		if (list_empty(&vout->buf_list) ||
> +		    list_is_singular(&vout->buf_list)) {
> +			spin_unlock(&vout->qlock);
> +			continue;
> +		}
> +
> +		vimc_buf = list_first_entry(&vout->buf_list,
> +				    typeof(*vimc_buf), list);
> +		list_del(&vimc_buf->list);
> +		spin_unlock(&vout->qlock);
> +
> +		vimc_buf->vb2.vb2_buf.timestamp = ktime_get_ns();
> +		vimc_buf->vb2.sequence = vout->sequence++;
> +		vimc_buf->vb2.field = vout->format.field;
> +		vb2_buffer_done(&vimc_buf->vb2.vb2_buf, VB2_BUF_STATE_DONE);
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * vimc_out_start_streaming - callback used when userspace starts streaming
> + *
> + * @vq:	   queue of vb2 video buffers
> + * @count: number of already queued buffers
> + *
> + * Allocate the output frame and starts the consume thread
> + *
> + * Return: 0 on success or error code
> + */
> +static int vimc_out_start_streaming(struct vb2_queue *vq, unsigned int count)
> +{
> +	struct vimc_vid_device *vout = vb2_get_drv_priv(vq);
> +
> +	vout->sequence = 0;
> +
> +	vout->kthread_fd = kthread_run(vimc_out_consume_frame, vout,
> +					      "vimc-output consume thread");

Alignment

> +
> +	if (IS_ERR(vout->kthread_fd))
> +		return PTR_ERR(vout->kthread_fd);
> +
> +	return 0;
> +}
> +
> +/**
> + * vimc_out_stop_streaming - callback used when userspace stops streaming
> + *
> + * @vq:	   queue of vb2 video buffers
> + *
> + * Wait until vimc_stream stops consuming a frame (using a mutex) and free it.
> + * Stops the thread and return all unused buffers to userspace.
> + */
> +static void vimc_out_stop_streaming(struct vb2_queue *vq)
> +{
> +	struct vimc_vid_device *vout = vb2_get_drv_priv(vq);
> +
> +	vout->sequence = 0;
> +
> +	kthread_stop(vout->kthread_fd);
> +
> +	vimc_vid_return_all_buffers(vout, VB2_BUF_STATE_ERROR);
> +}
> +
> +static const struct vb2_ops vimc_out_qops = {
> +	.start_streaming	= vimc_out_start_streaming,
> +	.stop_streaming		= vimc_out_stop_streaming,
> +	.buf_queue		= vimc_vid_buf_queue,
> +	.queue_setup		= vimc_vid_queue_setup,
> +	.buf_prepare		= vimc_vid_buffer_prepare,
> +	.buf_out_validate	= vimc_vid_buffer_validate,
> +	/*
> +	 * Since q->lock is set we can use the standard
> +	 * vb2_ops_wait_prepare/finish helper functions.
> +	 */
> +	.wait_prepare		= vb2_ops_wait_prepare,
> +	.wait_finish		= vb2_ops_wait_finish,
> +};
> +
> +static const struct media_entity_operations vimc_out_mops = {
> +	.link_validate		= vimc_link_validate,
> +};
> +
> +/**
> + * vimc_out_process_frame - process a frame on media pipeline
> + *
> + * @ved:   the vimc entity structure to process the frame
> + * @frame: pointer to the pipeline frame. Unused by output
> + *
> + * If there's a frame to be sent to the pipeline, copy the newest frame and
> + * send it through the pipeline.
> + * If there's no frame on vimc_output buffer list (this probably means that
> + * there's no streaming going on at the output) or the pixelformat of the stream
> + * mismatch the output format, send NULL.
> + */
> +static void *vimc_out_process_frame(struct vimc_ent_device *ved,
> +						 const void *frame)

Alignment

> +{
> +	struct vimc_vid_device *vout = container_of(ved, struct vimc_vid_device,
> +						    ved);
> +	struct vimc_vid_buffer *vimc_buf;
> +	void *vbuf;
> +
> +	if (ved->stream->producer_pixfmt != vout->format.pixelformat)
> +		return NULL;

fyi, after reverting "media: vimc: propagate pixel format in the stream"
this check should be done in vimc_link_validate

> +
> +	spin_lock(&vout->qlock);
> +	vimc_buf = list_first_entry_or_null(&vout->buf_list,
> +					    typeof(*vimc_buf), list);
> +	spin_unlock(&vout->qlock);
> +
> +	if (!vimc_buf)
> +		return NULL;
> +
> +	vbuf = vb2_plane_vaddr(&vimc_buf->vb2.vb2_buf, 0);
> +	memcpy(vout->frame, vbuf, vout->format.sizeimage);
> +
> +	return vout->frame;
> +}
> +
> +/**
> + * vimc_out_s_stream - callback before/after the start/stop of pipeline stream
> + *
> + * @ved:	the vimc_ent_device compenent of vimc_vid_device
> + * @enable:	0 means that the streaming has stop, non-zero means the
> + *		will begin

s/has/has to/
I think some word is missing. Maybe just:
0 stop the stream, non-zero start the stream

> + *
> + */
> +static int vimc_out_s_stream(struct vimc_ent_device *ved, int enable)
> +{
> +	struct vimc_vid_device *vout =
> +				container_of(ved, struct vimc_vid_device, ved);
> +
> +	if (enable) {
> +		vout->frame = vmalloc(vout->format.sizeimage);
> +		if (!vout->frame)
> +			return -ENOMEM;
> +
> +	} else {
> +		vfree(vout->frame);
> +		vout->frame = NULL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int vimc_out_comp_bind(struct device *comp, struct device *master,
> +		void *master_data)
> +{
> +	struct v4l2_device *v4l2_dev = master_data;
> +	struct vimc_platform_data *pdata = comp->platform_data;
> +	struct vimc_vid_device *vout;
> +	struct video_device *vdev;
> +	struct vb2_queue *q;
> +	int ret;
> +
> +	/* Allocate the vimc_vid_device struct */
> +	vout = kzalloc(sizeof(*vout), GFP_KERNEL);
> +	if (!vout)
> +		return -ENOMEM;
> +
> +	vout->stream.pipe_size = 0;
> +	/* Allocate the pads */
> +	vout->ved.pads =
> +		vimc_pads_init(1, (const unsigned long[1]) {MEDIA_PAD_FL_SOURCE});
> +	if (IS_ERR(vout->ved.pads)) {
> +		ret = PTR_ERR(vout->ved.pads);
> +		goto err_free_vout;
> +	}
> +
> +	/* Initialize the media entity */
> +	vout->vdev.entity.name = pdata->entity_name;
> +	vout->vdev.entity.function = MEDIA_ENT_F_IO_V4L;
> +	ret = media_entity_pads_init(&vout->vdev.entity,
> +				     1, vout->ved.pads);
> +	if (ret)
> +		goto err_clean_pads;
> +
> +	/* Initialize the lock */
> +	mutex_init(&vout->lock);
> +
> +	/* Initialize the vb2 queue */
> +	q = &vout->queue;
> +	q->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
> +	q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> +	q->drv_priv = vout;
> +	q->buf_struct_size = sizeof(struct vimc_vid_buffer);
> +	q->ops = &vimc_out_qops;
> +	q->mem_ops = &vb2_vmalloc_memops;
> +	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> +	q->min_buffers_needed = 2;
> +	q->lock = &vout->lock;
> +
> +	ret = vb2_queue_init(q);
> +	if (ret) {
> +		dev_err(comp, "%s: vb2 queue init failed (err=%d)\n",
> +				pdata->entity_name, ret);

alignment

> +		goto err_clean_m_ent;
> +	}
> +
> +	/* Initialize buffer list and its lock */
> +	INIT_LIST_HEAD(&vout->buf_list);
> +	spin_lock_init(&vout->qlock);
> +
> +	/* Set default frame format */
> +	vout->format = fmt_default;
> +	v4l2_fill_pixfmt(&vout->format, vout->format.pixelformat,
> +			 vout->format.width, vout->format.height);
> +
> +	/* Fill the vimc_ent_device struct */
> +	vout->ved.ent = &vout->vdev.entity;
> +	vout->ved.process_frame = vimc_out_process_frame;
> +	vout->ved.s_stream = vimc_out_s_stream;
> +	vout->ved.vdev_get_format = vimc_vid_get_format;
> +	dev_set_drvdata(comp, &vout->ved);
> +	vout->dev = comp;
> +
> +	/* Initialize the video_device struct */
> +	vdev = &vout->vdev;
> +	vdev->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
> +	vdev->entity.ops = &vimc_out_mops;
> +	vdev->release = vimc_vid_release;
> +	vdev->fops = &vimc_vid_fops;
> +	vdev->ioctl_ops = &vimc_out_ioctl_ops;
> +	vdev->lock = &vout->lock;
> +	vdev->queue = q;
> +	vdev->v4l2_dev = v4l2_dev;
> +	vdev->vfl_dir = VFL_DIR_TX;
> +	strscpy(vdev->name, pdata->entity_name, sizeof(vdev->name));
> +	video_set_drvdata(vdev, &vout->ved);
> +
> +	/* Register the video_device with the v4l2 and the media framework */
> +	ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
> +	if (ret) {
> +		dev_err(comp, "%s: video register failed (err=%d)\n",
> +				vout->vdev.name, ret);
> +		goto err_release_queue;
> +	}
> +
> +	return 0;
> +
> +err_release_queue:
> +	vb2_queue_release(q);
> +err_clean_m_ent:
> +	media_entity_cleanup(&vout->vdev.entity);
> +err_clean_pads:
> +	vimc_pads_cleanup(vout->ved.pads);
> +err_free_vout:
> +	kfree(vout);
> +
> +	return ret;
> +}
> +
> +static const struct component_ops vimc_out_comp_ops = {
> +	.bind = vimc_out_comp_bind,
> +	.unbind = vimc_vid_comp_unbind,
> +};
> +
> +static int vimc_out_probe(struct platform_device *pdev)
> +{
> +	return component_add(&pdev->dev, &vimc_out_comp_ops);
> +}
> +
> +static int vimc_out_remove(struct platform_device *pdev)
> +{
> +	component_del(&pdev->dev, &vimc_out_comp_ops);
> +
> +	return 0;
> +}
> +
> +static const struct platform_device_id vimc_out_driver_ids[] = {
> +	{
> +		.name           = VIMC_OUT_DRV_NAME,
> +	},
> +	{ }
> +};
> +
> +static struct platform_driver vimc_out_pdrv = {
> +	.probe		= vimc_out_probe,
> +	.remove		= vimc_out_remove,
> +	.id_table	= vimc_out_driver_ids,
> +	.driver		= {
> +		.name	= VIMC_OUT_DRV_NAME,
> +	},
> +};
> +
> +module_platform_driver(vimc_out_pdrv);
> +
> +MODULE_DEVICE_TABLE(platform, vimc_out_driver_ids);
> +
> +MODULE_DESCRIPTION("Virtual Media Controller Driver (VIMC) Output");
> +MODULE_AUTHOR("André Almeida <andrealmeid@collabora.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c
> index c7123a45c55b..2c3e486a29c0 100644
> --- a/drivers/media/platform/vimc/vimc-scaler.c
> +++ b/drivers/media/platform/vimc/vimc-scaler.c
> @@ -194,9 +194,10 @@ static const struct v4l2_subdev_pad_ops vimc_sca_pad_ops = {
>  	.set_fmt		= vimc_sca_set_fmt,
>  };
>  
> -static int vimc_sca_s_stream(struct v4l2_subdev *sd, int enable)
> +static int vimc_sca_s_stream(struct vimc_ent_device *ved, int enable)
>  {
> -	struct vimc_sca_device *vsca = v4l2_get_subdevdata(sd);
> +	struct vimc_sca_device *vsca = container_of(ved, struct vimc_sca_device,
> +						    ved);
>  
>  	if (enable) {
>  		u32 pixelformat = vsca->ved.stream->producer_pixfmt;
> @@ -239,13 +240,9 @@ static int vimc_sca_s_stream(struct v4l2_subdev *sd, int enable)
>  	return 0;
>  }
>  
> -static const struct v4l2_subdev_video_ops vimc_sca_video_ops = {
> -	.s_stream = vimc_sca_s_stream,
> -};
>  
>  static const struct v4l2_subdev_ops vimc_sca_ops = {
>  	.pad = &vimc_sca_pad_ops,
> -	.video = &vimc_sca_video_ops,
>  };
>  
>  static void vimc_sca_fill_pix(u8 *const ptr,
> @@ -381,6 +378,7 @@ static int vimc_sca_comp_bind(struct device *comp, struct device *master,
>  	}
>  
>  	vsca->ved.process_frame = vimc_sca_process_frame;
> +	vsca->ved.s_stream = vimc_sca_s_stream,
>  	dev_set_drvdata(comp, &vsca->ved);
>  	vsca->dev = comp;
>  
> diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
> index 51359472eef2..a1afb840493d 100644
> --- a/drivers/media/platform/vimc/vimc-sensor.c
> +++ b/drivers/media/platform/vimc/vimc-sensor.c
> @@ -177,10 +177,10 @@ static void *vimc_sen_process_frame(struct vimc_ent_device *ved,
>  	return vsen->frame;
>  }
>  
> -static int vimc_sen_s_stream(struct v4l2_subdev *sd, int enable)
> +static int vimc_sen_s_stream(struct vimc_ent_device *ved, int enable)
>  {
>  	struct vimc_sen_device *vsen =
> -				container_of(sd, struct vimc_sen_device, sd);
> +				container_of(ved, struct vimc_sen_device, ved);
>  
>  	if (enable) {
>  		u32 pixelformat = vsen->ved.stream->producer_pixfmt;
> @@ -218,14 +218,9 @@ static const struct v4l2_subdev_core_ops vimc_sen_core_ops = {
>  	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
>  };
>  
> -static const struct v4l2_subdev_video_ops vimc_sen_video_ops = {
> -	.s_stream = vimc_sen_s_stream,
> -};
> -
>  static const struct v4l2_subdev_ops vimc_sen_ops = {
>  	.core = &vimc_sen_core_ops,
>  	.pad = &vimc_sen_pad_ops,
> -	.video = &vimc_sen_video_ops,
>  };
>  
>  static int vimc_sen_s_ctrl(struct v4l2_ctrl *ctrl)
> @@ -351,6 +346,7 @@ static int vimc_sen_comp_bind(struct device *comp, struct device *master,
>  		goto err_free_hdl;
>  
>  	vsen->ved.process_frame = vimc_sen_process_frame;
> +	vsen->ved.s_stream = vimc_sen_s_stream;
>  	dev_set_drvdata(comp, &vsen->ved);
>  	vsen->dev = comp;
>  
> diff --git a/drivers/media/platform/vimc/vimc-streamer.c b/drivers/media/platform/vimc/vimc-streamer.c
> index 3b3f36357a0e..fd330415ac38 100644
> --- a/drivers/media/platform/vimc/vimc-streamer.c
> +++ b/drivers/media/platform/vimc/vimc-streamer.c
> @@ -47,7 +47,6 @@ static struct media_entity *vimc_get_source_entity(struct media_entity *ent)
>  static void vimc_streamer_pipeline_terminate(struct vimc_stream *stream)
>  {
>  	struct vimc_ent_device *ved;
> -	struct v4l2_subdev *sd;
>  
>  	while (stream->pipe_size) {
>  		stream->pipe_size--;
> @@ -55,11 +54,8 @@ static void vimc_streamer_pipeline_terminate(struct vimc_stream *stream)
>  		ved->stream = NULL;
>  		stream->ved_pipeline[stream->pipe_size] = NULL;
>  
> -		if (!is_media_entity_v4l2_subdev(ved->ent))
> -			continue;
> -
> -		sd = media_entity_to_v4l2_subdev(ved->ent);
> -		v4l2_subdev_call(sd, video, s_stream, 0);
> +		if (ved->s_stream)
> +			ved->s_stream(ved, 0);
>  	}
>  }
>  
> @@ -79,7 +75,6 @@ static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
>  	struct media_entity *entity;
>  	struct video_device *vdev;
>  	struct v4l2_subdev *sd;
> -	int ret = 0;
>  
>  	stream->pipe_size = 0;
>  	while (stream->pipe_size < VIMC_STREAMER_PIPELINE_MAX_SIZE) {
> @@ -90,16 +85,8 @@ static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
>  		stream->ved_pipeline[stream->pipe_size++] = ved;
>  		ved->stream = stream;
>  
> -		if (is_media_entity_v4l2_subdev(ved->ent)) {
> -			sd = media_entity_to_v4l2_subdev(ved->ent);
> -			ret = v4l2_subdev_call(sd, video, s_stream, 1);
> -			if (ret && ret != -ENOIOCTLCMD) {
> -				pr_err("subdev_call error %s\n",
> -				       ved->ent->name);
> -				vimc_streamer_pipeline_terminate(stream);
> -				return ret;
> -			}
> -		}
> +		if (ved->s_stream)
> +			ved->s_stream(ved, 1);
>  
>  		entity = vimc_get_source_entity(ved->ent);
>  		/* Check if the end of the pipeline was reached*/
> @@ -149,6 +136,7 @@ static int vimc_streamer_thread(void *data)
>  			if (!frame || IS_ERR(frame))
>  				break;
>  		}
> +
>  		//wait for 60hz
>  		set_current_state(TASK_UNINTERRUPTIBLE);
>  		schedule_timeout(HZ / 60);
> diff --git a/drivers/media/platform/vimc/vimc-video.c b/drivers/media/platform/vimc/vimc-video.c
> index f7ccb2e9d6c5..0187e699543a 100644
> --- a/drivers/media/platform/vimc/vimc-video.c
> +++ b/drivers/media/platform/vimc/vimc-video.c
> @@ -236,6 +236,15 @@ int vimc_vid_buffer_prepare(struct vb2_buffer *vb)
>  }
>  EXPORT_SYMBOL_GPL(vimc_vid_buffer_prepare);
>  
> +int vimc_vid_buffer_validate(struct vb2_buffer *vb)
> +{
> +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +
> +	vbuf->field = V4L2_FIELD_NONE;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(vimc_vid_buffer_validate);

Is this function only used by the output? Maybe just keep it private in the output.

> +
>  void vimc_vid_release(struct video_device *vdev)
>  {
>  	struct vimc_vid_device *vid =
> diff --git a/drivers/media/platform/vimc/vimc-video.h b/drivers/media/platform/vimc/vimc-video.h
> index 22cb0e2dbdbb..67f13244bf92 100644
> --- a/drivers/media/platform/vimc/vimc-video.h
> +++ b/drivers/media/platform/vimc/vimc-video.h
> @@ -116,6 +116,8 @@ int vimc_vid_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
>  
>  int vimc_vid_buffer_prepare(struct vb2_buffer *vb);
>  
> +int vimc_vid_buffer_validate(struct vb2_buffer *vb);
> +
>  int vimc_vid_probe(struct platform_device *pdev);
>  
>  int vimc_vid_remove(struct platform_device *pdev);
> 

Thanks,
Helen

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

* Re: [PATCH 5/7] media: vimc: core: Add output device on the pipeline
  2019-07-02 15:47 ` [PATCH 5/7] media: vimc: core: Add output device on " André Almeida
@ 2019-07-09 22:20   ` Helen Koike
  0 siblings, 0 replies; 15+ messages in thread
From: Helen Koike @ 2019-07-09 22:20 UTC (permalink / raw)
  To: André Almeida, linux-media; +Cc: mchehab, hverkuil, kernel, linux-kernel



On 7/2/19 12:47 PM, André Almeida wrote:
> Add the output video device on the hardcoded pipeline. Change the link
> to it be enabled by default.
> 
> Signed-off-by: André Almeida <andrealmeid@collabora.com>
> ---
>  drivers/media/platform/vimc/vimc-core.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/vimc/vimc-core.c b/drivers/media/platform/vimc/vimc-core.c
> index 571c55aa0e16..ecdea1d631c5 100644
> --- a/drivers/media/platform/vimc/vimc-core.c
> +++ b/drivers/media/platform/vimc/vimc-core.c
> @@ -95,8 +95,7 @@ static const struct vimc_ent_config ent_config[] = {
>  	},
>  	{
>  		.name = "RGB/YUV Input",

Could you also change the name here to Output? I think it makes more sense.

Helen

> -		/* TODO: change this to vimc-input when it is implemented */
> -		.drv = "vimc-sensor",
> +		.drv = "vimc-output",
>  	},
>  	{
>  		.name = "Scaler",
> @@ -118,11 +117,11 @@ static const struct vimc_ent_link ent_links[] = {
>  	/* Link: Sensor B (Pad 0)->(Pad 0) Raw Capture 1 */
>  	VIMC_ENT_LINK(1, 0, 5, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
>  	/* Link: Debayer A (Pad 1)->(Pad 0) Scaler */
> -	VIMC_ENT_LINK(2, 1, 7, 0, MEDIA_LNK_FL_ENABLED),
> +	VIMC_ENT_LINK(2, 1, 7, 0, 0),
>  	/* Link: Debayer B (Pad 1)->(Pad 0) Scaler */
>  	VIMC_ENT_LINK(3, 1, 7, 0, 0),
>  	/* Link: RGB/YUV Input (Pad 0)->(Pad 0) Scaler */
> -	VIMC_ENT_LINK(6, 0, 7, 0, 0),
> +	VIMC_ENT_LINK(6, 0, 7, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
>  	/* Link: Scaler (Pad 1)->(Pad 0) RGB/YUV Capture */
>  	VIMC_ENT_LINK(7, 1, 8, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE),
>  };
> 

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

* Re: [PATCH 0/7] media: vimc: Add a V4L2 output device
  2019-07-09 22:19 ` [PATCH 0/7] media: vimc: Add a V4L2 " Helen Koike
@ 2019-07-10  7:33   ` Hans Verkuil
  2019-07-12 15:38     ` André Almeida
  0 siblings, 1 reply; 15+ messages in thread
From: Hans Verkuil @ 2019-07-10  7:33 UTC (permalink / raw)
  To: Helen Koike, André Almeida, linux-media
  Cc: mchehab, kernel, linux-kernel

On 7/10/19 12:19 AM, Helen Koike wrote:
> Hi André,
> 
> Thanks for the patches.
> 
> On 7/2/19 12:47 PM, André Almeida wrote:
>> Hello,
>>
>> This patch adds a V4L2 output device on vimc, that comply with V4L2 API
>> for video output. If there is an output device and a capture device at the
>> same pipeline, one can get a video loopback pipeline feeding frames at
>> the output and then seeing them at the capture. It's possible to insert
>> vimc submodules at the pipeline to modify the image (e.g. a scaler).
>>
>> If one starts a streaming at the capture, with the output off, the
>> capture will display a noisy frame. If one starts a streaming at the
>> output with the capture off, the output will just consume the buffers,
>> without sending them to the pipeline. If both output and capture are
>> streaming, the loopback will happen.
> 
> I understand why it is done like this in vivid, but I was wondering, if we
> have a pipeline like:
> output -> capture
> Shouldn't streaming from the capture just stalls if there is no frame
> available in the output (i.e. streaming in the output is off) ? But then I'm
> not sure what the framerate in the capture would mean.
> 
> Hans, what do you think?

If you set up the pipeline like this:

Video Output -> Scaler -> Video Capture

Then this is a mem2mem device (except with two separate video devices) and
framerate doesn't apply anymore. And video capture will just stall if there
is no video output frame provided.

It's how e.g. omap3isp works.

Regards,

	Hans

> 
> Thanks,
> Helen
> 
>>
>> The patches 1 and 2 provide some ground to create the output
>> device. The patch 3 creates the device and modify how the vimc-streamer
>> was dealing with the s_stream callback on other vimc modules, to make
>> simpler implementing this callback at vimc-output. Patch 4 change the
>> behavior of the pipeline in order to be closer to a real life hardware.
>> Patches 5-7 updates the default pipeline and the documentation to
>> include the new output device.
>>
>> This is the result of v4l2-compliance after this patch series:
>> $ v4l2-compliance -m0 -s50
>> Grand Total for vimc device /dev/media0: 476, Succeeded: 476, Failed: 0,
>> Warnings: 0
>>
>> A git tree up to date with media-master and with this changes can be found
>> at: https://gitlab.collabora.com/tonyk/linux/tree/vimc/output
>>
>> In order to test it, one can follow these instructions:
>>
>> 1 - Configure the pipeline (requires v4l-utils):
>>
>> $ media-ctl -d platform:vimc -V '"Sensor A":0[fmt:SBGGR8_1X8/640x480]'
>> $ media-ctl -d platform:vimc -V '"Debayer A":0[fmt:SBGGR8_1X8/640x480]'
>> $ media-ctl -d platform:vimc -V '"Sensor B":0[fmt:SBGGR8_1X8/640x480]'
>> $ media-ctl -d platform:vimc -V '"Debayer B":0[fmt:SBGGR8_1X8/640x480]'
>> $ v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440
>> $ v4l2-ctl -z platform:vimc -d "Raw Capture 0" -v pixelformat=BA81
>> $ v4l2-ctl -z platform:vimc -d "Raw Capture 1" -v pixelformat=BA81
>> $ v4l2-ctl -z platform:vimc -e "RGB/YUV Input" -v width=640,height=480
>>
>> 2 - Use a userspace application:
>> 2.a gst-launch (requires gstreamer and gst-plugins-good):
>>
>> Feed frames into the output and grab from the capture (rescaled for
>> convenience):
>>
>> $ gst-launch-1.0 videotestsrc pattern=ball ! \
>> 	video/x-raw,width=640,height=480,format=RGB \
>> 	! v4l2sink device=/dev/video2 v4l2src device=/dev/video3 ! \
>> 	video/x-raw,width=1920,height=1440,format=RGB ! videoscale ! \
>> 	video/x-raw,width=640,height=480 ! videoconvert ! ximagesink
>>
>> 2.b qv4l2 (requires v4l-utils):
>>
>> Open the output device:
>>
>> $ qv4l2 -d2
>>
>> Open the capture device:
>>
>> $ qv4l2 -d3
>>
>> Start the streaming at both, at any order. You can change the frame
>> content at "Test Pattern Generator" -> "Test Pattern" on the output.
>>
>> Thanks,
>> 	André
>>
>> André Almeida (7):
>>   media: vimc: Create video module
>>   media: vimc: video: Add write file operation
>>   media: vimc: Create a V4L2 output device
>>   media: vimc: Send null buffer through the pipeline
>>   media: vimc: core: Add output device on the pipeline
>>   media: vimc.dot: Update default topology diagram
>>   media: vimc.rst: Add output device
>>
>>  Documentation/media/v4l-drivers/vimc.dot    |   4 +-
>>  Documentation/media/v4l-drivers/vimc.rst    |  12 +-
>>  drivers/media/platform/vimc/Makefile        |   4 +-
>>  drivers/media/platform/vimc/vimc-capture.c  | 356 +++----------------
>>  drivers/media/platform/vimc/vimc-common.h   |   5 +-
>>  drivers/media/platform/vimc/vimc-core.c     |   7 +-
>>  drivers/media/platform/vimc/vimc-debayer.c  |  14 +-
>>  drivers/media/platform/vimc/vimc-output.c   | 362 ++++++++++++++++++++
>>  drivers/media/platform/vimc/vimc-scaler.c   |  13 +-
>>  drivers/media/platform/vimc/vimc-sensor.c   |  10 +-
>>  drivers/media/platform/vimc/vimc-streamer.c |  24 +-
>>  drivers/media/platform/vimc/vimc-video.c    | 273 +++++++++++++++
>>  drivers/media/platform/vimc/vimc-video.h    | 130 +++++++
>>  13 files changed, 849 insertions(+), 365 deletions(-)
>>  create mode 100644 drivers/media/platform/vimc/vimc-output.c
>>  create mode 100644 drivers/media/platform/vimc/vimc-video.c
>>  create mode 100644 drivers/media/platform/vimc/vimc-video.h
>>


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

* Re: [PATCH 0/7] media: vimc: Add a V4L2 output device
  2019-07-10  7:33   ` Hans Verkuil
@ 2019-07-12 15:38     ` André Almeida
  2019-07-13 10:03       ` Hans Verkuil
  0 siblings, 1 reply; 15+ messages in thread
From: André Almeida @ 2019-07-12 15:38 UTC (permalink / raw)
  To: Hans Verkuil, Helen Koike, linux-media; +Cc: mchehab, kernel, linux-kernel

Hello,

On 7/10/19 4:33 AM, Hans Verkuil wrote:
> On 7/10/19 12:19 AM, Helen Koike wrote:
>> Hi André,
>>
>> Thanks for the patches.
>>
>> On 7/2/19 12:47 PM, André Almeida wrote:
>>> Hello,
>>>
>>> This patch adds a V4L2 output device on vimc, that comply with V4L2 API
>>> for video output. If there is an output device and a capture device at the
>>> same pipeline, one can get a video loopback pipeline feeding frames at
>>> the output and then seeing them at the capture. It's possible to insert
>>> vimc submodules at the pipeline to modify the image (e.g. a scaler).
>>>
>>> If one starts a streaming at the capture, with the output off, the
>>> capture will display a noisy frame. If one starts a streaming at the
>>> output with the capture off, the output will just consume the buffers,
>>> without sending them to the pipeline. If both output and capture are
>>> streaming, the loopback will happen.
>> I understand why it is done like this in vivid, but I was wondering, if we
>> have a pipeline like:
>> output -> capture
>> Shouldn't streaming from the capture just stalls if there is no frame
>> available in the output (i.e. streaming in the output is off) ? But then I'm
>> not sure what the framerate in the capture would mean.
>>
>> Hans, what do you think?
> If you set up the pipeline like this:
>
> Video Output -> Scaler -> Video Capture

If the capture will stall if there's no frame from the video output, how
can I add support for this kind of pipeline at test-media? It would be
required to send frames to the output device while running
`v4l2-compliance` at the capture device to make testing possible.

Thanks,
    André

> Then this is a mem2mem device (except with two separate video devices) and
> framerate doesn't apply anymore. And video capture will just stall if there
> is no video output frame provided.
>
> It's how e.g. omap3isp works.
>
> Regards,
>
> 	Hans
>
>> Thanks,
>> Helen
>>
>>> The patches 1 and 2 provide some ground to create the output
>>> device. The patch 3 creates the device and modify how the vimc-streamer
>>> was dealing with the s_stream callback on other vimc modules, to make
>>> simpler implementing this callback at vimc-output. Patch 4 change the
>>> behavior of the pipeline in order to be closer to a real life hardware.
>>> Patches 5-7 updates the default pipeline and the documentation to
>>> include the new output device.
>>>
>>> This is the result of v4l2-compliance after this patch series:
>>> $ v4l2-compliance -m0 -s50
>>> Grand Total for vimc device /dev/media0: 476, Succeeded: 476, Failed: 0,
>>> Warnings: 0
>>>
>>> A git tree up to date with media-master and with this changes can be found
>>> at: https://gitlab.collabora.com/tonyk/linux/tree/vimc/output
>>>
>>> In order to test it, one can follow these instructions:
>>>
>>> 1 - Configure the pipeline (requires v4l-utils):
>>>
>>> $ media-ctl -d platform:vimc -V '"Sensor A":0[fmt:SBGGR8_1X8/640x480]'
>>> $ media-ctl -d platform:vimc -V '"Debayer A":0[fmt:SBGGR8_1X8/640x480]'
>>> $ media-ctl -d platform:vimc -V '"Sensor B":0[fmt:SBGGR8_1X8/640x480]'
>>> $ media-ctl -d platform:vimc -V '"Debayer B":0[fmt:SBGGR8_1X8/640x480]'
>>> $ v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440
>>> $ v4l2-ctl -z platform:vimc -d "Raw Capture 0" -v pixelformat=BA81
>>> $ v4l2-ctl -z platform:vimc -d "Raw Capture 1" -v pixelformat=BA81
>>> $ v4l2-ctl -z platform:vimc -e "RGB/YUV Input" -v width=640,height=480
>>>
>>> 2 - Use a userspace application:
>>> 2.a gst-launch (requires gstreamer and gst-plugins-good):
>>>
>>> Feed frames into the output and grab from the capture (rescaled for
>>> convenience):
>>>
>>> $ gst-launch-1.0 videotestsrc pattern=ball ! \
>>> 	video/x-raw,width=640,height=480,format=RGB \
>>> 	! v4l2sink device=/dev/video2 v4l2src device=/dev/video3 ! \
>>> 	video/x-raw,width=1920,height=1440,format=RGB ! videoscale ! \
>>> 	video/x-raw,width=640,height=480 ! videoconvert ! ximagesink
>>>
>>> 2.b qv4l2 (requires v4l-utils):
>>>
>>> Open the output device:
>>>
>>> $ qv4l2 -d2
>>>
>>> Open the capture device:
>>>
>>> $ qv4l2 -d3
>>>
>>> Start the streaming at both, at any order. You can change the frame
>>> content at "Test Pattern Generator" -> "Test Pattern" on the output.
>>>
>>> Thanks,
>>> 	André
>>>
>>> André Almeida (7):
>>>   media: vimc: Create video module
>>>   media: vimc: video: Add write file operation
>>>   media: vimc: Create a V4L2 output device
>>>   media: vimc: Send null buffer through the pipeline
>>>   media: vimc: core: Add output device on the pipeline
>>>   media: vimc.dot: Update default topology diagram
>>>   media: vimc.rst: Add output device
>>>
>>>  Documentation/media/v4l-drivers/vimc.dot    |   4 +-
>>>  Documentation/media/v4l-drivers/vimc.rst    |  12 +-
>>>  drivers/media/platform/vimc/Makefile        |   4 +-
>>>  drivers/media/platform/vimc/vimc-capture.c  | 356 +++----------------
>>>  drivers/media/platform/vimc/vimc-common.h   |   5 +-
>>>  drivers/media/platform/vimc/vimc-core.c     |   7 +-
>>>  drivers/media/platform/vimc/vimc-debayer.c  |  14 +-
>>>  drivers/media/platform/vimc/vimc-output.c   | 362 ++++++++++++++++++++
>>>  drivers/media/platform/vimc/vimc-scaler.c   |  13 +-
>>>  drivers/media/platform/vimc/vimc-sensor.c   |  10 +-
>>>  drivers/media/platform/vimc/vimc-streamer.c |  24 +-
>>>  drivers/media/platform/vimc/vimc-video.c    | 273 +++++++++++++++
>>>  drivers/media/platform/vimc/vimc-video.h    | 130 +++++++
>>>  13 files changed, 849 insertions(+), 365 deletions(-)
>>>  create mode 100644 drivers/media/platform/vimc/vimc-output.c
>>>  create mode 100644 drivers/media/platform/vimc/vimc-video.c
>>>  create mode 100644 drivers/media/platform/vimc/vimc-video.h
>>>

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

* Re: [PATCH 0/7] media: vimc: Add a V4L2 output device
  2019-07-12 15:38     ` André Almeida
@ 2019-07-13 10:03       ` Hans Verkuil
  2019-07-31  3:00         ` André Almeida
  0 siblings, 1 reply; 15+ messages in thread
From: Hans Verkuil @ 2019-07-13 10:03 UTC (permalink / raw)
  To: André Almeida, Helen Koike, linux-media
  Cc: mchehab, kernel, linux-kernel

On 7/12/19 5:38 PM, André Almeida wrote:
> Hello,
> 
> On 7/10/19 4:33 AM, Hans Verkuil wrote:
>> On 7/10/19 12:19 AM, Helen Koike wrote:
>>> Hi André,
>>>
>>> Thanks for the patches.
>>>
>>> On 7/2/19 12:47 PM, André Almeida wrote:
>>>> Hello,
>>>>
>>>> This patch adds a V4L2 output device on vimc, that comply with V4L2 API
>>>> for video output. If there is an output device and a capture device at the
>>>> same pipeline, one can get a video loopback pipeline feeding frames at
>>>> the output and then seeing them at the capture. It's possible to insert
>>>> vimc submodules at the pipeline to modify the image (e.g. a scaler).
>>>>
>>>> If one starts a streaming at the capture, with the output off, the
>>>> capture will display a noisy frame. If one starts a streaming at the
>>>> output with the capture off, the output will just consume the buffers,
>>>> without sending them to the pipeline. If both output and capture are
>>>> streaming, the loopback will happen.
>>> I understand why it is done like this in vivid, but I was wondering, if we
>>> have a pipeline like:
>>> output -> capture
>>> Shouldn't streaming from the capture just stalls if there is no frame
>>> available in the output (i.e. streaming in the output is off) ? But then I'm
>>> not sure what the framerate in the capture would mean.
>>>
>>> Hans, what do you think?
>> If you set up the pipeline like this:
>>
>> Video Output -> Scaler -> Video Capture
> 
> If the capture will stall if there's no frame from the video output, how
> can I add support for this kind of pipeline at test-media? It would be
> required to send frames to the output device while running
> `v4l2-compliance` at the capture device to make testing possible.

The compliance test doesn't support such devices at the moment.

I think a new option (or options) are needed to tell the compliance test
that the capture and output video devices together constitute an m2m device.

Regards,

	Hans

> 
> Thanks,
>     André
> 
>> Then this is a mem2mem device (except with two separate video devices) and
>> framerate doesn't apply anymore. And video capture will just stall if there
>> is no video output frame provided.
>>
>> It's how e.g. omap3isp works.
>>
>> Regards,
>>
>> 	Hans
>>
>>> Thanks,
>>> Helen
>>>
>>>> The patches 1 and 2 provide some ground to create the output
>>>> device. The patch 3 creates the device and modify how the vimc-streamer
>>>> was dealing with the s_stream callback on other vimc modules, to make
>>>> simpler implementing this callback at vimc-output. Patch 4 change the
>>>> behavior of the pipeline in order to be closer to a real life hardware.
>>>> Patches 5-7 updates the default pipeline and the documentation to
>>>> include the new output device.
>>>>
>>>> This is the result of v4l2-compliance after this patch series:
>>>> $ v4l2-compliance -m0 -s50
>>>> Grand Total for vimc device /dev/media0: 476, Succeeded: 476, Failed: 0,
>>>> Warnings: 0
>>>>
>>>> A git tree up to date with media-master and with this changes can be found
>>>> at: https://gitlab.collabora.com/tonyk/linux/tree/vimc/output
>>>>
>>>> In order to test it, one can follow these instructions:
>>>>
>>>> 1 - Configure the pipeline (requires v4l-utils):
>>>>
>>>> $ media-ctl -d platform:vimc -V '"Sensor A":0[fmt:SBGGR8_1X8/640x480]'
>>>> $ media-ctl -d platform:vimc -V '"Debayer A":0[fmt:SBGGR8_1X8/640x480]'
>>>> $ media-ctl -d platform:vimc -V '"Sensor B":0[fmt:SBGGR8_1X8/640x480]'
>>>> $ media-ctl -d platform:vimc -V '"Debayer B":0[fmt:SBGGR8_1X8/640x480]'
>>>> $ v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440
>>>> $ v4l2-ctl -z platform:vimc -d "Raw Capture 0" -v pixelformat=BA81
>>>> $ v4l2-ctl -z platform:vimc -d "Raw Capture 1" -v pixelformat=BA81
>>>> $ v4l2-ctl -z platform:vimc -e "RGB/YUV Input" -v width=640,height=480
>>>>
>>>> 2 - Use a userspace application:
>>>> 2.a gst-launch (requires gstreamer and gst-plugins-good):
>>>>
>>>> Feed frames into the output and grab from the capture (rescaled for
>>>> convenience):
>>>>
>>>> $ gst-launch-1.0 videotestsrc pattern=ball ! \
>>>> 	video/x-raw,width=640,height=480,format=RGB \
>>>> 	! v4l2sink device=/dev/video2 v4l2src device=/dev/video3 ! \
>>>> 	video/x-raw,width=1920,height=1440,format=RGB ! videoscale ! \
>>>> 	video/x-raw,width=640,height=480 ! videoconvert ! ximagesink
>>>>
>>>> 2.b qv4l2 (requires v4l-utils):
>>>>
>>>> Open the output device:
>>>>
>>>> $ qv4l2 -d2
>>>>
>>>> Open the capture device:
>>>>
>>>> $ qv4l2 -d3
>>>>
>>>> Start the streaming at both, at any order. You can change the frame
>>>> content at "Test Pattern Generator" -> "Test Pattern" on the output.
>>>>
>>>> Thanks,
>>>> 	André
>>>>
>>>> André Almeida (7):
>>>>   media: vimc: Create video module
>>>>   media: vimc: video: Add write file operation
>>>>   media: vimc: Create a V4L2 output device
>>>>   media: vimc: Send null buffer through the pipeline
>>>>   media: vimc: core: Add output device on the pipeline
>>>>   media: vimc.dot: Update default topology diagram
>>>>   media: vimc.rst: Add output device
>>>>
>>>>  Documentation/media/v4l-drivers/vimc.dot    |   4 +-
>>>>  Documentation/media/v4l-drivers/vimc.rst    |  12 +-
>>>>  drivers/media/platform/vimc/Makefile        |   4 +-
>>>>  drivers/media/platform/vimc/vimc-capture.c  | 356 +++----------------
>>>>  drivers/media/platform/vimc/vimc-common.h   |   5 +-
>>>>  drivers/media/platform/vimc/vimc-core.c     |   7 +-
>>>>  drivers/media/platform/vimc/vimc-debayer.c  |  14 +-
>>>>  drivers/media/platform/vimc/vimc-output.c   | 362 ++++++++++++++++++++
>>>>  drivers/media/platform/vimc/vimc-scaler.c   |  13 +-
>>>>  drivers/media/platform/vimc/vimc-sensor.c   |  10 +-
>>>>  drivers/media/platform/vimc/vimc-streamer.c |  24 +-
>>>>  drivers/media/platform/vimc/vimc-video.c    | 273 +++++++++++++++
>>>>  drivers/media/platform/vimc/vimc-video.h    | 130 +++++++
>>>>  13 files changed, 849 insertions(+), 365 deletions(-)
>>>>  create mode 100644 drivers/media/platform/vimc/vimc-output.c
>>>>  create mode 100644 drivers/media/platform/vimc/vimc-video.c
>>>>  create mode 100644 drivers/media/platform/vimc/vimc-video.h
>>>>


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

* Re: [PATCH 0/7] media: vimc: Add a V4L2 output device
  2019-07-13 10:03       ` Hans Verkuil
@ 2019-07-31  3:00         ` André Almeida
  0 siblings, 0 replies; 15+ messages in thread
From: André Almeida @ 2019-07-31  3:00 UTC (permalink / raw)
  To: Hans Verkuil, Helen Koike, linux-media; +Cc: mchehab, kernel, linux-kernel

On 7/13/19 7:03 AM, Hans Verkuil wrote:
> On 7/12/19 5:38 PM, André Almeida wrote:
>> Hello,
>>
>> On 7/10/19 4:33 AM, Hans Verkuil wrote:
>>> On 7/10/19 12:19 AM, Helen Koike wrote:
>>>> Hi André,
>>>>
>>>> Thanks for the patches.
>>>>
>>>> On 7/2/19 12:47 PM, André Almeida wrote:
>>>>> Hello,
>>>>>
>>>>> This patch adds a V4L2 output device on vimc, that comply with V4L2 API
>>>>> for video output. If there is an output device and a capture device at the
>>>>> same pipeline, one can get a video loopback pipeline feeding frames at
>>>>> the output and then seeing them at the capture. It's possible to insert
>>>>> vimc submodules at the pipeline to modify the image (e.g. a scaler).
>>>>>
>>>>> If one starts a streaming at the capture, with the output off, the
>>>>> capture will display a noisy frame. If one starts a streaming at the
>>>>> output with the capture off, the output will just consume the buffers,
>>>>> without sending them to the pipeline. If both output and capture are
>>>>> streaming, the loopback will happen.
>>>> I understand why it is done like this in vivid, but I was wondering, if we
>>>> have a pipeline like:
>>>> output -> capture
>>>> Shouldn't streaming from the capture just stalls if there is no frame
>>>> available in the output (i.e. streaming in the output is off) ? But then I'm
>>>> not sure what the framerate in the capture would mean.
>>>>
>>>> Hans, what do you think?
>>> If you set up the pipeline like this:
>>>
>>> Video Output -> Scaler -> Video Capture
>>
>> If the capture will stall if there's no frame from the video output, how
>> can I add support for this kind of pipeline at test-media? It would be
>> required to send frames to the output device while running
>> `v4l2-compliance` at the capture device to make testing possible.
> 
> The compliance test doesn't support such devices at the moment.

The implementation of the expected behavior can be found here:
https://gitlab.collabora.com/tonyk/linux/tree/vimc/output-v2

> 
> I think a new option (or options) are needed to tell the compliance test
> that the capture and output video devices together constitute an m2m device.

I've reading the v4l-utils code base and I had a look at both m2m tests
and capture/output tests, but I'm not sure how to implement this new
option. How do you think it should be implemented? Should it resemble
how v4l2-compliance tests vim2m? Something like this:

	v4l2-compliance -m platform:vim2m -z platform:vivid-002 -e
vivid-002-vid-cap -s10 -P -a

Thanks,
	André

> 
> Regards,
> 
> 	Hans
> 
>>
>> Thanks,
>>     André
>>
>>> Then this is a mem2mem device (except with two separate video devices) and
>>> framerate doesn't apply anymore. And video capture will just stall if there
>>> is no video output frame provided.
>>>
>>> It's how e.g. omap3isp works.
>>>
>>> Regards,
>>>
>>> 	Hans
>>>
>>>> Thanks,
>>>> Helen
>>>>
>>>>> The patches 1 and 2 provide some ground to create the output
>>>>> device. The patch 3 creates the device and modify how the vimc-streamer
>>>>> was dealing with the s_stream callback on other vimc modules, to make
>>>>> simpler implementing this callback at vimc-output. Patch 4 change the
>>>>> behavior of the pipeline in order to be closer to a real life hardware.
>>>>> Patches 5-7 updates the default pipeline and the documentation to
>>>>> include the new output device.
>>>>>
>>>>> This is the result of v4l2-compliance after this patch series:
>>>>> $ v4l2-compliance -m0 -s50
>>>>> Grand Total for vimc device /dev/media0: 476, Succeeded: 476, Failed: 0,
>>>>> Warnings: 0
>>>>>
>>>>> A git tree up to date with media-master and with this changes can be found
>>>>> at: https://gitlab.collabora.com/tonyk/linux/tree/vimc/output
>>>>>
>>>>> In order to test it, one can follow these instructions:
>>>>>
>>>>> 1 - Configure the pipeline (requires v4l-utils):
>>>>>
>>>>> $ media-ctl -d platform:vimc -V '"Sensor A":0[fmt:SBGGR8_1X8/640x480]'
>>>>> $ media-ctl -d platform:vimc -V '"Debayer A":0[fmt:SBGGR8_1X8/640x480]'
>>>>> $ media-ctl -d platform:vimc -V '"Sensor B":0[fmt:SBGGR8_1X8/640x480]'
>>>>> $ media-ctl -d platform:vimc -V '"Debayer B":0[fmt:SBGGR8_1X8/640x480]'
>>>>> $ v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440
>>>>> $ v4l2-ctl -z platform:vimc -d "Raw Capture 0" -v pixelformat=BA81
>>>>> $ v4l2-ctl -z platform:vimc -d "Raw Capture 1" -v pixelformat=BA81
>>>>> $ v4l2-ctl -z platform:vimc -e "RGB/YUV Input" -v width=640,height=480
>>>>>
>>>>> 2 - Use a userspace application:
>>>>> 2.a gst-launch (requires gstreamer and gst-plugins-good):
>>>>>
>>>>> Feed frames into the output and grab from the capture (rescaled for
>>>>> convenience):
>>>>>
>>>>> $ gst-launch-1.0 videotestsrc pattern=ball ! \
>>>>> 	video/x-raw,width=640,height=480,format=RGB \
>>>>> 	! v4l2sink device=/dev/video2 v4l2src device=/dev/video3 ! \
>>>>> 	video/x-raw,width=1920,height=1440,format=RGB ! videoscale ! \
>>>>> 	video/x-raw,width=640,height=480 ! videoconvert ! ximagesink
>>>>>
>>>>> 2.b qv4l2 (requires v4l-utils):
>>>>>
>>>>> Open the output device:
>>>>>
>>>>> $ qv4l2 -d2
>>>>>
>>>>> Open the capture device:
>>>>>
>>>>> $ qv4l2 -d3
>>>>>
>>>>> Start the streaming at both, at any order. You can change the frame
>>>>> content at "Test Pattern Generator" -> "Test Pattern" on the output.
>>>>>
>>>>> Thanks,
>>>>> 	André
>>>>>
>>>>> André Almeida (7):
>>>>>   media: vimc: Create video module
>>>>>   media: vimc: video: Add write file operation
>>>>>   media: vimc: Create a V4L2 output device
>>>>>   media: vimc: Send null buffer through the pipeline
>>>>>   media: vimc: core: Add output device on the pipeline
>>>>>   media: vimc.dot: Update default topology diagram
>>>>>   media: vimc.rst: Add output device
>>>>>
>>>>>  Documentation/media/v4l-drivers/vimc.dot    |   4 +-
>>>>>  Documentation/media/v4l-drivers/vimc.rst    |  12 +-
>>>>>  drivers/media/platform/vimc/Makefile        |   4 +-
>>>>>  drivers/media/platform/vimc/vimc-capture.c  | 356 +++----------------
>>>>>  drivers/media/platform/vimc/vimc-common.h   |   5 +-
>>>>>  drivers/media/platform/vimc/vimc-core.c     |   7 +-
>>>>>  drivers/media/platform/vimc/vimc-debayer.c  |  14 +-
>>>>>  drivers/media/platform/vimc/vimc-output.c   | 362 ++++++++++++++++++++
>>>>>  drivers/media/platform/vimc/vimc-scaler.c   |  13 +-
>>>>>  drivers/media/platform/vimc/vimc-sensor.c   |  10 +-
>>>>>  drivers/media/platform/vimc/vimc-streamer.c |  24 +-
>>>>>  drivers/media/platform/vimc/vimc-video.c    | 273 +++++++++++++++
>>>>>  drivers/media/platform/vimc/vimc-video.h    | 130 +++++++
>>>>>  13 files changed, 849 insertions(+), 365 deletions(-)
>>>>>  create mode 100644 drivers/media/platform/vimc/vimc-output.c
>>>>>  create mode 100644 drivers/media/platform/vimc/vimc-video.c
>>>>>  create mode 100644 drivers/media/platform/vimc/vimc-video.h
>>>>>
> 


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

end of thread, other threads:[~2019-07-31  3:01 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-02 15:47 [PATCH 0/7] media: vimc: Add a V4L2 output device André Almeida
2019-07-02 15:47 ` [PATCH 1/7] media: vimc: Create video module André Almeida
2019-07-02 15:47 ` [PATCH 2/7] media: vimc: video: Add write file operation André Almeida
2019-07-02 15:47 ` [PATCH 3/7] media: vimc: Create a V4L2 output device André Almeida
2019-07-09 22:20   ` Helen Koike
2019-07-02 15:47 ` [PATCH 4/7] media: vimc: Send null buffer through the pipeline André Almeida
2019-07-02 15:47 ` [PATCH 5/7] media: vimc: core: Add output device on " André Almeida
2019-07-09 22:20   ` Helen Koike
2019-07-02 15:47 ` [PATCH 6/7] media: vimc.dot: Update default topology diagram André Almeida
2019-07-02 15:47 ` [PATCH 7/7] media: vimc.rst: Add output device André Almeida
2019-07-09 22:19 ` [PATCH 0/7] media: vimc: Add a V4L2 " Helen Koike
2019-07-10  7:33   ` Hans Verkuil
2019-07-12 15:38     ` André Almeida
2019-07-13 10:03       ` Hans Verkuil
2019-07-31  3:00         ` André Almeida

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).