Re: [PATCH v7 4/6] media: starfive: camss: Add video driver

Hi Jack,

On Tue, Aug 01, 2023 at 02:23:07PM +0800, Jack Zhu wrote:

On 2023/7/27 16:49, Hans Verkuil wrote:

quoted

On 19/06/2023 13:28, Jack Zhu wrote:

quoted

Add video driver for StarFive Camera Subsystem.

Signed-off-by: Jack Zhu <jack.zhu@starfivetech.com>
---
 .../media/platform/starfive/camss/Makefile    |   4 +-
 .../media/platform/starfive/camss/stf_video.c | 724 ++++++++++++++++++
 .../media/platform/starfive/camss/stf_video.h |  92 +++
 3 files changed, 819 insertions(+), 1 deletion(-)
 create mode 100644 drivers/media/platform/starfive/camss/stf_video.c
 create mode 100644 drivers/media/platform/starfive/camss/stf_video.h

[snip]

quoted

quoted

diff --git a/drivers/media/platform/starfive/camss/stf_video.c b/drivers/media/platform/starfive/camss/stf_video.c
new file mode 100644
index 000000000000..2e6472fe51c6
--- /dev/null
+++ b/drivers/media/platform/starfive/camss/stf_video.c

@@ -0,0 +1,724 @@

[snip]

quoted

quoted

+int stf_video_register(struct stfcamss_video *video,
+		       struct v4l2_device *v4l2_dev, const char *name)
+{
+	struct video_device *vdev;
+	struct vb2_queue *q;
+	struct media_pad *pad = &video->pad;
+	int ret;
+
+	vdev = &video->vdev;
+
+	mutex_init(&video->q_lock);
+
+	q = &video->vb2_q;
+	q->drv_priv = video;
+	q->mem_ops = &vb2_dma_contig_memops;
+	q->ops = &stf_video_vb2_q_ops;
+	q->type = video->type;
+	q->io_modes = VB2_DMABUF | VB2_MMAP | VB2_READ;

VB2_READ support does not generally make sense for uncompressed video since
read() always requires a memcpy, and that makes it very inefficient.

It doesn't hurt though, so it is up to you whether or not you want this.

Yes, we would like to retain this feature to meet some possible special needs.

The issue with enabling READ support in drivers is that it encourages
applications to do the wrong thing. If you want to keep it, I'd like to
know what use cases you envision would strongly require it.

quoted

quoted

+	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	q->buf_struct_size = sizeof(struct stfcamss_buffer);
+	q->dev = video->stfcamss->dev;
+	q->lock = &video->q_lock;
+	q->min_buffers_needed = STFCAMSS_MIN_BUFFERS;
+	ret = vb2_queue_init(q);
+	if (ret < 0) {
+		dev_err(video->stfcamss->dev,
+			"Failed to init vb2 queue: %d\n", ret);
+		goto err_vb2_init;
+	}
+
+	pad->flags = MEDIA_PAD_FL_SINK;
+	ret = media_entity_pads_init(&vdev->entity, 1, pad);
+	if (ret < 0) {
+		dev_err(video->stfcamss->dev,
+			"Failed to init video entity: %d\n", ret);
+		goto err_vb2_init;
+	}
+
+	mutex_init(&video->lock);
+
+	if (video->id == STF_V_LINE_WR) {
+		video->formats = formats_pix_wr;
+		video->nformats = ARRAY_SIZE(formats_pix_wr);
+		video->bpl_alignment = 8;
+	} else {
+		video->formats = formats_pix_isp;
+		video->nformats = ARRAY_SIZE(formats_pix_isp);
+		video->bpl_alignment = 1;
+	}
+
+	ret = stf_video_init_format(video);
+	if (ret < 0) {
+		dev_err(video->stfcamss->dev,
+			"Failed to init format: %d\n", ret);
+		goto err_vid_init_format;
+	}
+
+	vdev->fops = &stf_vid_fops;
+	vdev->ioctl_ops = &stf_vid_ioctl_ops;
+	vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE;
+	vdev->vfl_dir = VFL_DIR_RX;
+	vdev->device_caps |= V4L2_CAP_STREAMING | V4L2_CAP_READWRITE;
+	vdev->release = stf_video_release;
+	vdev->v4l2_dev = v4l2_dev;
+	vdev->queue = &video->vb2_q;
+	vdev->lock = &video->lock;
+	strscpy(vdev->name, name, sizeof(vdev->name));
+
+	ret = video_register_device(vdev, VFL_TYPE_VIDEO, video->id);
+	if (ret < 0) {
+		dev_err(video->stfcamss->dev,
+			"Failed to register video device: %d\n", ret);
+		goto err_vid_reg;
+	}
+
+	video_set_drvdata(vdev, video);
+	return 0;
+
+err_vid_reg:
+err_vid_init_format:
+	media_entity_cleanup(&vdev->entity);
+	mutex_destroy(&video->lock);
+err_vb2_init:
+	mutex_destroy(&video->q_lock);
+	return ret;
+}

[snip]

-- 
Regards,

Laurent Pinchart