Hi Jacopo On 01/07/2025 14:17, Jacopo Mondi wrote:
Hi Dan On Tue, Jul 01, 2025 at 02:01:25PM +0100, Dan Scally wrote: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++; }YesWe 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!You know, looking at what [video|media]_pipeline_[alloc]_start() does, it could even be renamed to pipeline_validate() and your new functions named 'start' :) Anyway, just for the sake of discussion, one could even create a [video|media]_pipeline_[alloc]_run() function that bundles together the existing _start() functions and the newly proposed _try_run(). The driver would be an even more compact 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; rzv2h_ivc_format_configure(ivc); ivc->buffers.sequence = 0; ivc->vvalid_ifp = 0; ret = video_device_pipeline_alloc_run(&ivc->vdev.dev); if (ret) { dev_err(ivc->dev, "failed to start media pipeline\n"); goto err_pm_runtime_put; } media_jobs_run_jobs(ivc->sched);
Wouldn't this still need to be conditional? We'd need video_device_pipeline_alloc_run() to be able to return something that indicated:
1. Failure 2. Success, but the pipeline isn't ready to start 3. Success and the pipeline is ready to start right?
return 0; } Do you see a use case for first starting the pipeline then trying to run it in a separate step ?
Not off the top of my head...they're both operations that ought to happen in .start_streaming()...but do you propose replacing the existing video_device_pipeline_start() with a bundled function, or just adding a new function and retaining the old one?
Dan
+} + +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_getIt was just to guarantee that it's in a known state, but I can probably drop itI 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 ?RightSo 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 loopAck+ + 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 formatsOh 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 explicitI'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() somewhereThat'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 DanThanks 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
