On 2023/8/2 2:47, Laurent Pinchart wrote: > Hi Jack, > > On Tue, Aug 01, 2023 at 02:23:07PM +0800, Jack Zhu wrote: >> On 2023/7/27 16:49, Hans Verkuil wrote: >> > On 19/06/2023 13:28, Jack Zhu wrote: >> >> Add video driver for StarFive Camera Subsystem. >> >> >> >> Signed-off-by: Jack Zhu <jack.zhu@xxxxxxxxxxxxxxxx> >> >> --- >> >> .../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] > >> >> 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] > >> >> +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. > OK, I'll drop it. Thank you! >> >> + 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] >