Re: [PATCH v2 2/3] media: platform: Add Renesas Input Video Control block driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jacopo

On 01/07/2025 13:58, Jacopo Mondi wrote:
Hi Dan

On Tue, Jul 01, 2025 at 12:27:41PM +0100, Dan Scally wrote:
Hi Jacopo

[snip]

+static bool rzv2h_ivc_pipeline_ready(struct media_pipeline *pipe)
+{
+	struct media_pipeline_entity_iter iter;
+	unsigned int n_video_devices = 0;
+	struct media_entity *entity;
+	int ret;
+
+	ret = media_pipeline_entity_iter_init(pipe, &iter);
+	if (ret)
+		return ret;
+
+	media_pipeline_for_each_entity(pipe, &iter, entity) {
+		if (entity->obj_type == MEDIA_ENTITY_TYPE_VIDEO_DEVICE)
+			n_video_devices++;
+	}
This counts the ISP video devices as well, right ?
That's right

+
+	media_pipeline_entity_iter_cleanup(&iter);
+
+	return n_video_devices == pipe->start_count;
So this checks that all other video devices have started when this one
is started as well. What if we start the IVC first and the ISP later?
Doesn't matter which order; nothing happens until they're all started and
then the .pipeline_started() callbacks for the entities run.
Ah sure, thanks, I got it wrong.

I see that all drivers in the series (IVC and mali) that use the
media_pipeline_started() helper have to implement a function similar
in spirit to rzv2h_ivc_pipeline_ready(). Can't the framework do that ?
So that drivers can call media_pipeline_started() [*] unconditionally
and use its return value to find out if the pipeline has actually
started or not ?
The steer I got from Laurent and Sakari was that code from mc shouldn't be
checking for MEDIA_ENTITY_TYPE_VIDEO_DEVICE, but perhaps we could have a
V4L2 helper that does that instead?
Do you mean this check ?

	media_pipeline_for_each_entity(pipe, &iter, entity) {
		if (entity->obj_type == MEDIA_ENTITY_TYPE_VIDEO_DEVICE)
			n_video_devices++;
	}


Yes


We have wrappers like

__must_check int video_device_pipeline_start(struct video_device *vdev,
					     struct media_pipeline *pipe)

already, so this might become something like

int video_device_try_run_pipeline(vdev)
{
         pipe = video_device_pipeline(&ivc->vdev.dev);

       	ret = media_pipeline_entity_iter_init(pipe, &iter);
	if (ret)
		return ret;

	media_pipeline_for_each_entity(pipe, &iter, entity) {
		if (entity->obj_type == MEDIA_ENTITY_TYPE_VIDEO_DEVICE)
			n_video_devices++;
	}

	media_pipeline_entity_iter_cleanup(&iter);

	return (n_video_devices == pipe->start_count) ?
		media_pipeline_started(pipe) : -ENODEV;
}

The drivers then should become something like:

static int rzv2h_ivc_start_streaming(struct vb2_queue *q, unsigned int count)
{
	struct rzv2h_ivc *ivc = vb2_get_drv_priv(q);
	struct media_pipeline *pipe;
	int ret;

	ret = pm_runtime_resume_and_get(ivc->dev);
	if (ret)
		goto err_return_buffers;

	ret = video_device_pipeline_alloc_start(&ivc->vdev.dev);
	if (ret) {
		dev_err(ivc->dev, "failed to start media pipeline\n");
		goto err_pm_runtime_put;
	}

	rzv2h_ivc_format_configure(ivc);

	ivc->buffers.sequence = 0;
	ivc->vvalid_ifp = 0;

         if (!video_device_try_run_pipeline(ivc->dev))
		media_jobs_run_jobs(ivc->sched);

	return 0;

err_pm_runtime_put:
	pm_runtime_put(ivc->dev);
err_return_buffers:
	rzv2h_ivc_return_buffers(ivc, VB2_BUF_STATE_QUEUED);

	return ret;
}

Removing a bit of boilerplate in all drivers using
media_pipeline_started()/stopped() ?
Yes something like video_device_pipeline_start() is what I was thinking, what you've done there looks spot on to me!
+}
+
+static int rzv2h_ivc_start_streaming(struct vb2_queue *q, unsigned int count)
+{
+	struct rzv2h_ivc *ivc = vb2_get_drv_priv(q);
+	struct media_pipeline *pipe;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(ivc->dev);
+	if (ret)
+		goto err_return_buffers;
+
+	ret = video_device_pipeline_alloc_start(&ivc->vdev.dev);
+	if (ret) {
+		dev_err(ivc->dev, "failed to start media pipeline\n");
+		goto err_pm_runtime_put;
+	}
+
+	rzv2h_ivc_format_configure(ivc);
+
+	ivc->buffers.sequence = 0;
+
+	spin_lock(&ivc->spinlock);
+	ivc->vvalid_ifp = 0;
+	spin_unlock(&ivc->spinlock);
scoped_guard() maybe, and I wonder if you need this if you initialize
the variable before resume_and_get
It was just to guarantee that it's in a known state, but I can probably drop it
I don't contest resetting it to 0, I'm just pointing out you can do
that earlier and avoid locking ?
Ah! Yes certainly true.
+
+	pipe = video_device_pipeline(&ivc->vdev.dev);
+	if (rzv2h_ivc_pipeline_ready(pipe)) {
+		ret = media_pipeline_started(pipe);
+		if (ret)
+			goto err_stop_pipeline;
+
+		media_jobs_run_jobs(ivc->sched);
+	}
+
+	return 0;
+
+err_stop_pipeline:
+	video_device_pipeline_stop(&ivc->vdev.dev);
+err_pm_runtime_put:
+	pm_runtime_put(ivc->dev);
+err_return_buffers:
+	rzv2h_ivc_return_buffers(ivc, VB2_BUF_STATE_QUEUED);
+
+	return ret;
+}
+
+static void rzv2h_ivc_stop_streaming(struct vb2_queue *q)
+{
+	struct rzv2h_ivc *ivc = vb2_get_drv_priv(q);
+	struct media_pipeline *pipe;
+
+	pipe = video_device_pipeline(&ivc->vdev.dev);
+	if (rzv2h_ivc_pipeline_ready(pipe)) {
+		media_pipeline_stopped(pipe);
+		media_jobs_cancel_jobs(ivc->sched);
+	}
I suspect I already asked about this, but this returns true only if
all video devices have started right ?
Right
    So what if ISP is stopped first
then IVC ?
It doesn't matter which gets stopped first, it's just to make sure we run
media_pipeline_stopped() and media_jobs_cancel_jobs() whenever the _first_
video device is stopped
+
+	rzv2h_ivc_return_buffers(ivc, VB2_BUF_STATE_ERROR);
+	video_device_pipeline_stop(&ivc->vdev.dev);
+	pm_runtime_mark_last_busy(ivc->dev);
+	pm_runtime_put_autosuspend(ivc->dev);
+}
+
+static const struct vb2_ops rzv2h_ivc_vb2_ops = {
+	.queue_setup		= &rzv2h_ivc_queue_setup,
+	.buf_queue		= &rzv2h_ivc_buf_queue,
+	.wait_prepare		= vb2_ops_wait_prepare,
+	.wait_finish		= vb2_ops_wait_finish,
+	.start_streaming	= &rzv2h_ivc_start_streaming,
+	.stop_streaming		= &rzv2h_ivc_stop_streaming,
+};
+
+static const struct rzv2h_ivc_format *
+rzv2h_ivc_format_from_pixelformat(u32 fourcc)
+{
+	unsigned int i;
nit: Could live inside the for loop
Ack
+
+	for (i = 0; i < ARRAY_SIZE(rzv2h_ivc_formats); i++)
+		if (fourcc == rzv2h_ivc_formats[i].fourcc)
+			return &rzv2h_ivc_formats[i];
+
+	return &rzv2h_ivc_formats[0];
+}
+
+static int rzv2h_ivc_enum_fmt_vid_out(struct file *file, void *fh,
+				      struct v4l2_fmtdesc *f)
+{
+	if (f->index >= ARRAY_SIZE(rzv2h_ivc_formats))
+		return -EINVAL;
+
+	f->pixelformat = rzv2h_ivc_formats[f->index].fourcc;
+	return 0;
+}
+
+static int rzv2h_ivc_g_fmt_vid_out(struct file *file, void *fh,
+				   struct v4l2_format *f)
+{
+	struct rzv2h_ivc *ivc = video_drvdata(file);
+
+	f->fmt.pix = ivc->format.pix;
+
+	return 0;
+}
+
+static void rzv2h_ivc_try_fmt(struct v4l2_pix_format *pix,
+			      const struct rzv2h_ivc_format *fmt)
+{
+	pix->pixelformat = fmt->fourcc;
+
+	pix->width = clamp(pix->width, RZV2H_IVC_MIN_WIDTH,
+			   RZV2H_IVC_MAX_WIDTH);
+	pix->height = clamp(pix->height, RZV2H_IVC_MIN_HEIGHT,
+			    RZV2H_IVC_MAX_HEIGHT);
+
+	pix->field = V4L2_FIELD_NONE;
+	pix->colorspace = V4L2_COLORSPACE_RAW;
+	pix->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+	pix->quantization = V4L2_QUANTIZATION_DEFAULT;
Same as per the subdevice use explicit values, or at least the
DEFAULT() macros

+
+	v4l2_fill_pixfmt(pix, pix->pixelformat, pix->width, pix->height);
+}
+
+static void rzv2h_ivc_set_format(struct rzv2h_ivc *ivc,
+				 struct v4l2_pix_format *pix)
+{
+	const struct rzv2h_ivc_format *fmt;
+
+	fmt = rzv2h_ivc_format_from_pixelformat(pix->pixelformat);
+
+	rzv2h_ivc_try_fmt(pix, fmt);
+	ivc->format.pix = *pix;
+	ivc->format.fmt = fmt;
+}
+
+static int rzv2h_ivc_s_fmt_vid_out(struct file *file, void *fh,
+				   struct v4l2_format *f)
+{
+	struct rzv2h_ivc *ivc = video_drvdata(file);
+	struct v4l2_pix_format *pix = &f->fmt.pix;
+
+	if (vb2_is_busy(&ivc->vdev.vb2q))
+		return -EBUSY;
+
+	rzv2h_ivc_set_format(ivc, pix);
+
+	return 0;
+}
+
+static int rzv2h_ivc_try_fmt_vid_out(struct file *file, void *fh,
+				     struct v4l2_format *f)
+{
+	const struct rzv2h_ivc_format *fmt;
+
+	fmt = rzv2h_ivc_format_from_pixelformat(f->fmt.pix.pixelformat);
+
+	rzv2h_ivc_try_fmt(&f->fmt.pix, fmt);
nit: maybe remove the previous empty line and add one before return ?

+	return 0;
+}
+
+static int rzv2h_ivc_querycap(struct file *file, void *fh,
+			      struct v4l2_capability *cap)
+{
+	strscpy(cap->driver, "rzv2h-ivc", sizeof(cap->driver));
+	strscpy(cap->card, "Renesas Input Video Control", sizeof(cap->card));
+
+	return 0;
+}
+
+static const struct v4l2_ioctl_ops rzv2h_ivc_v4l2_ioctl_ops = {
+	.vidioc_reqbufs = vb2_ioctl_reqbufs,
+	.vidioc_querybuf = vb2_ioctl_querybuf,
+	.vidioc_create_bufs = vb2_ioctl_create_bufs,
+	.vidioc_qbuf = vb2_ioctl_qbuf,
+	.vidioc_expbuf = vb2_ioctl_expbuf,
+	.vidioc_dqbuf = vb2_ioctl_dqbuf,
+	.vidioc_prepare_buf = vb2_ioctl_prepare_buf,
+	.vidioc_streamon = vb2_ioctl_streamon,
+	.vidioc_streamoff = vb2_ioctl_streamoff,
+	.vidioc_enum_fmt_vid_out = rzv2h_ivc_enum_fmt_vid_out,
+	.vidioc_g_fmt_vid_out = rzv2h_ivc_g_fmt_vid_out,
+	.vidioc_s_fmt_vid_out = rzv2h_ivc_s_fmt_vid_out,
+	.vidioc_try_fmt_vid_out = rzv2h_ivc_try_fmt_vid_out,
+	.vidioc_querycap = rzv2h_ivc_querycap,
+	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
+	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
+};
+
+static const struct v4l2_file_operations rzv2h_ivc_v4l2_fops = {
+	.owner = THIS_MODULE,
+	.unlocked_ioctl = video_ioctl2,
+	.open = v4l2_fh_open,
+	.release = vb2_fop_release,
+	.poll = vb2_fop_poll,
+	.mmap = vb2_fop_mmap,
+};
+
+static bool rzv2h_ivc_job_ready(void *data)
+{
+	struct rzv2h_ivc *ivc = data;
+
+	guard(spinlock)(&ivc->buffers.lock);
+
+	if (list_empty(&ivc->buffers.pending))
+		return false;
+
+	return true;
+}
+
+static void rzv2h_ivc_job_queue(void *data)
+{
+	struct rzv2h_ivc *ivc = data;
+	struct rzv2h_ivc_buf *buf;
+
+	/*
+	 * We need to move an entry from the pending queue to the input queue
+	 * here. We know that there is one, or .check_dep() would not have
+	 * allowed us to get this far. The entry needs to be removed or the same
+	 * check would allow a new job to be queued for the exact same buffer.
+	 */
+	guard(spinlock)(&ivc->buffers.lock);
+	buf = list_first_entry(&ivc->buffers.pending,
+			       struct rzv2h_ivc_buf, queue);
+	list_move_tail(&buf->queue, &ivc->buffers.queue);
+}
+
+static void rzv2h_ivc_job_abort(void *data)
+{
+	struct rzv2h_ivc *ivc = data;
+	struct rzv2h_ivc_buf *buf;
+
+	guard(spinlock)(&ivc->buffers.lock);
+	buf = list_first_entry(&ivc->buffers.queue,
+			       struct rzv2h_ivc_buf, queue);
+
+	if (buf)
+		list_move(&buf->queue, &ivc->buffers.pending);
+}
+
+static int rzv2h_ivc_job_add_steps(struct media_job *job, void *data)
+{
+	struct rzv2h_ivc *ivc = data;
+	int ret;
+
+	ret = media_jobs_add_job_step(job, rzv2h_ivc_set_next_buffer, ivc,
+				      MEDIA_JOBS_FL_STEP_ANYWHERE, 0);
+	if (ret)
+		return ret;
+
+	/*
+	 * This stage will be the second to last one to run - the ISP driver may
+	 * have some post-frame processing to do.
+	 */
+	return media_jobs_add_job_step(job, rzv2h_ivc_transfer_buffer, ivc,
+				       MEDIA_JOBS_FL_STEP_FROM_BACK, 1);
+}
+
+static struct media_job_contributor_ops rzv2h_ivc_media_job_ops = {
+	.add_steps	= rzv2h_ivc_job_add_steps,
+	.ready		= rzv2h_ivc_job_ready,
+	.queue		= rzv2h_ivc_job_queue,
+	.abort		= rzv2h_ivc_job_abort
+};
+
+int rzv2h_initialise_video_dev_and_queue(struct rzv2h_ivc *ivc,
+					 struct v4l2_device *v4l2_dev)
+{
+	struct v4l2_pix_format pix = { };
+	struct video_device *vdev;
+	struct vb2_queue *vb2q;
+	int ret;
+
+	spin_lock_init(&ivc->buffers.lock);
+	INIT_LIST_HEAD(&ivc->buffers.queue);
+	INIT_LIST_HEAD(&ivc->buffers.pending);
+	init_waitqueue_head(&ivc->buffers.wq);
+
+	/* Initialise vb2 queue */
+	vb2q = &ivc->vdev.vb2q;
+	vb2q->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
it's my understandin that MPLANE API is usually preferred also for devices
that only support single planar formats
Oh ok - thanks, I'll make the switch
+	vb2q->io_modes = VB2_MMAP | VB2_DMABUF;
+	vb2q->drv_priv = ivc;
+	vb2q->mem_ops = &vb2_dma_contig_memops;
+	vb2q->ops = &rzv2h_ivc_vb2_ops;
+	vb2q->buf_struct_size = sizeof(struct rzv2h_ivc_buf);
+	vb2q->min_queued_buffers = 0;
You can spare this, or keep it if you want it explicit
I'll probably keep it
+	vb2q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	vb2q->lock = &ivc->lock;
+	vb2q->dev = ivc->dev;
+
+	ret = vb2_queue_init(vb2q);
+	if (ret)
+		return dev_err_probe(ivc->dev, ret, "vb2 queue init failed\n");
+
+	/* Initialise Video Device */
+	vdev = &ivc->vdev.dev;
+	strscpy(vdev->name, "rzv2h-ivc", sizeof(vdev->name));
+	vdev->release = video_device_release_empty;
+	vdev->fops = &rzv2h_ivc_v4l2_fops;
+	vdev->ioctl_ops = &rzv2h_ivc_v4l2_ioctl_ops;
+	vdev->lock = &ivc->lock;
+	vdev->v4l2_dev = v4l2_dev;
+	vdev->queue = vb2q;
+	vdev->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
+	vdev->vfl_dir = VFL_DIR_TX;
+	video_set_drvdata(vdev, ivc);
+
+	pix.pixelformat = V4L2_PIX_FMT_SRGGB16;
+	pix.width = RZV2H_IVC_DEFAULT_WIDTH;
+	pix.height = RZV2H_IVC_DEFAULT_HEIGHT;
+	rzv2h_ivc_set_format(ivc, &pix);
+
+	ivc->vdev.pad.flags = MEDIA_PAD_FL_SOURCE;
+	ivc->vdev.dev.entity.ops = &rzv2h_ivc_media_ops;
+	ret = media_entity_pads_init(&ivc->vdev.dev.entity, 1, &ivc->vdev.pad);
+	if (ret)
+		goto err_release_vb2q;
+
+	ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);
+	if (ret) {
+		dev_err(ivc->dev, "failed to register IVC video device\n");
+		goto err_cleanup_vdev_entity;
+	}
What is the path that registers the subdevice devnode to userspace ?
IOW I was expecting to see v4l2_device_register_subdev_nodes()
somewhere
That's in the ISP driver - the IVC's subdevice connects through the V4L2
asynchronous API to the ISP's notifier, and the notifier's .complete()
callback runs v4l2_device_register_subdev_nodes()

Ack, sure, thanks for clarifying it!

+
+	ret = media_create_pad_link(&vdev->entity, 0, &ivc->subdev.sd.entity,
+				    RZV2H_IVC_SUBDEV_SINK_PAD,
+				    MEDIA_LNK_FL_ENABLED |
+				    MEDIA_LNK_FL_IMMUTABLE);
+	if (ret) {
+		dev_err(ivc->dev, "failed to create media link\n");
+		goto err_unregister_vdev;
+	}
+
+	ivc->sched = media_jobs_get_scheduler(vdev->entity.graph_obj.mdev);
+	if (IS_ERR(ivc->sched)) {
+		ret = PTR_ERR(ivc->sched);
+		goto err_remove_link;
+	}
+
+	ret = media_jobs_register_job_contributor(ivc->sched,
+						  &rzv2h_ivc_media_job_ops, ivc,
+						  MEDIA_JOB_TYPE_PIPELINE_PULSE);
+	if (ret)
+		goto err_put_media_job_scheduler;
+
+	return 0;
+
+err_put_media_job_scheduler:
+	media_jobs_put_scheduler(ivc->sched);
+err_remove_link:
+	media_entity_remove_links(&vdev->entity);
+err_unregister_vdev:
+	video_unregister_device(vdev);
+err_cleanup_vdev_entity:
+	media_entity_cleanup(&vdev->entity);
+err_release_vb2q:
+	vb2_queue_release(vb2q);
+
+	return ret;
+}
+
+void rzv2h_deinit_video_dev_and_queue(struct rzv2h_ivc *ivc)
+{
+	struct video_device *vdev = &ivc->vdev.dev;
+	struct vb2_queue *vb2q = &ivc->vdev.vb2q;
+
+	if (!ivc->sched)
+		return;
+
+	media_jobs_put_scheduler(ivc->sched);
+	vb2_video_unregister_device(vdev);
+	media_entity_cleanup(&vdev->entity);
+	vb2_queue_release(vb2q);
Shouldn't you get here also in case of !ivc->sched ?
This driver (at least in this version) should always have a sched pointer,
so this was just a convenient way to check if initialisation finished before
trying to deinit anything...it'll probably change though.
I see, a comment to explain that might be enough!
Sure - I'll add one.


Thanks

Dan

Thanks
    j


+}
diff --git a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h
new file mode 100644
index 0000000000000000000000000000000000000000..d2e310ce868125772d97259619b9369ccbcefe3d
--- /dev/null
+++ b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h
@@ -0,0 +1,133 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Renesas RZ/V2H Input Video Control Block driver
+ *
+ * Copyright (C) 2025 Ideas on Board Oy
+ */
+
+#include <linux/clk.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/reset.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+#include <linux/videodev2.h>
+#include <linux/wait.h>
+
+#include <media/media-entity.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-subdev.h>
+#include <media/videobuf2-core.h>
+#include <media/videobuf2-v4l2.h>
+
+#define RZV2H_IVC_REG_AXIRX_PLNUM			0x0000
+#define RZV2H_IVC_ONE_EXPOSURE				0x00
+#define RZV2H_IVC_TWO_EXPOSURE				0x01
+#define RZV2H_IVC_REG_AXIRX_PXFMT			0x0004
+#define RZV2H_IVC_INPUT_FMT_MIPI			(0 << 16)
+#define RZV2H_IVC_INPUT_FMT_CRU_PACKED			(1 << 16)
+#define RZV2H_IVC_PXFMT_DTYPE				GENMASK(7, 0)
+#define RZV2H_IVC_REG_AXIRX_SADDL_P0			0x0010
+#define RZV2H_IVC_REG_AXIRX_SADDH_P0			0x0014
+#define RZV2H_IVC_REG_AXIRX_SADDL_P1			0x0018
+#define RZV2H_IVC_REG_AXIRX_SADDH_P1			0x001c
+#define RZV2H_IVC_REG_AXIRX_HSIZE			0x0020
+#define RZV2H_IVC_REG_AXIRX_VSIZE			0x0024
+#define RZV2H_IVC_REG_AXIRX_BLANK			0x0028
+#define RZV2H_IVC_VBLANK(x)				((x) << 16)
+#define RZV2H_IVC_REG_AXIRX_STRD			0x0030
+#define RZV2H_IVC_REG_AXIRX_ISSU			0x0040
+#define RZV2H_IVC_REG_AXIRX_ERACT			0x0048
+#define RZV2H_IVC_REG_FM_CONTEXT			0x0100
+#define RZV2H_IVC_SOFTWARE_CFG				0x00
+#define RZV2H_IVC_SINGLE_CONTEXT_SW_HW_CFG		BIT(0)
+#define RZV2H_IVC_MULTI_CONTEXT_SW_HW_CFG		BIT(1)
+#define RZV2H_IVC_REG_FM_MCON				0x0104
+#define RZV2H_IVC_REG_FM_FRCON				0x0108
+#define RZV2H_IVC_REG_FM_STOP				0x010c
+#define RZV2H_IVC_REG_FM_INT_EN				0x0120
+#define RZV2H_IVC_VVAL_IFPE				BIT(0)
+#define RZV2H_IVC_REG_FM_INT_STA			0x0124
+#define RZV2H_IVC_REG_AXIRX_FIFOCAP0			0x0208
+#define RZV2H_IVC_REG_CORE_CAPCON			0x020c
+#define RZV2H_IVC_REG_CORE_FIFOCAP0			0x0228
+#define RZV2H_IVC_REG_CORE_FIFOCAP1			0x022c
+
+#define RZV2H_IVC_MIN_WIDTH				640
+#define RZV2H_IVC_MAX_WIDTH				4096
+#define RZV2H_IVC_MIN_HEIGHT				480
+#define RZV2H_IVC_MAX_HEIGHT				4096
+#define RZV2H_IVC_DEFAULT_WIDTH				1920
+#define RZV2H_IVC_DEFAULT_HEIGHT			1080
+
+#define RZV2H_IVC_NUM_CLOCKS				3
+#define RZV2H_IVC_NUM_RESETS				3
+
+struct device;
+
+enum rzv2h_ivc_subdev_pads {
+	RZV2H_IVC_SUBDEV_SINK_PAD,
+	RZV2H_IVC_SUBDEV_SOURCE_PAD,
+	RZV2H_IVC_NUM_SUBDEV_PADS
+};
+
+struct rzv2h_ivc_format {
+	u32 fourcc;
+	/*
+	 * The CRU packed pixel formats are bayer-order agnostic, so each could
+	 * support any one of the 4 possible media bus formats.
+	 */
+	u32 mbus_codes[4];
+	u8 dtype;
+};
+
+struct rzv2h_ivc {
+	struct device *dev;
+	void __iomem *base;
+	struct clk_bulk_data clks[RZV2H_IVC_NUM_CLOCKS];
+	struct reset_control_bulk_data resets[RZV2H_IVC_NUM_RESETS];
+	int irqnum;
+	u8 vvalid_ifp;
+
+	struct {
+		struct video_device dev;
+		struct vb2_queue vb2q;
+		struct media_pad pad;
+	} vdev;
+
+	struct {
+		struct v4l2_subdev sd;
+		struct media_pad pads[RZV2H_IVC_NUM_SUBDEV_PADS];
+	} subdev;
+
+	struct {
+		/* Spinlock to guard buffer queue */
+		spinlock_t lock;
+		wait_queue_head_t wq;
+		struct list_head queue;
+		struct list_head pending;
+		struct rzv2h_ivc_buf *curr;
+		unsigned int sequence;
+	} buffers;
+
+	struct media_job_scheduler *sched;
+
+	struct {
+		struct v4l2_pix_format pix;
+		const struct rzv2h_ivc_format *fmt;
+	} format;
+
+	/* Mutex to provide to vb2 */
+	struct mutex lock;
+	/* Lock to protect the interrupt counter */
+	spinlock_t spinlock;
+};
+
+int rzv2h_initialise_video_dev_and_queue(struct rzv2h_ivc *ivc,
+					 struct v4l2_device *v4l2_dev);
+void rzv2h_deinit_video_dev_and_queue(struct rzv2h_ivc *ivc);
+int rzv2h_ivc_initialise_subdevice(struct rzv2h_ivc *ivc);
+void rzv2h_ivc_deinit_subdevice(struct rzv2h_ivc *ivc);
+void rzv2h_ivc_write(struct rzv2h_ivc *ivc, u32 addr, u32 val);
+void rzv2h_ivc_update_bits(struct rzv2h_ivc *ivc, unsigned int addr,
+			   u32 mask, u32 val);

As agreed, I didn't review the job scheduler part but only the IVC
specific bits. A few nits here and there and next version should be
good to go!

Thanks
     j

--
2.34.1






[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux