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]
>
