Re: [PATCH v1 07/10] media: platform: mediatek: add isp_7x video ops

["CK Hu (胡俊光)" <ck.hu@mediatek.com>]

Hi, Shu-hsiang:

On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote:
> Introduces the V4L2 video interface and feature management for the
> MediaTek ISP7x CAMSYS. These interfaces include various functionalities,
> such as video operation initialization and registration. They also
> manage MediaTek-specific formats and handle buffers for MediaTek camera
> video devices. This integrates CAMSYS functionalities to be compatible
> with the V4L2 framework.
> 
> Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@mediatek.com>
> ---

[snip]

> +int mtk_cam_video_register(struct mtk_cam_video_device *video,
> +			   struct v4l2_device *v4l2_dev)
> +{
> +	struct mtk_cam_device *cam =
> +		container_of(v4l2_dev, struct mtk_cam_device, v4l2_dev);
> +	struct media_pad *pad = &video->pad;
> +	struct video_device *vdev = &video->vdev;
> +	struct vb2_queue *q = &video->vb2_q;
> +	unsigned int output = V4L2_TYPE_IS_OUTPUT(video->desc.buf_type);
> +	int ret;
> +
> +	if (video->desc.link_flags & MEDIA_LNK_FL_ENABLED)
> +		video->enabled = true;
> +	else
> +		video->enabled = false;
> +
> +	mutex_init(&video->q_lock);
> +
> +	/* initialize vb2_queue */
> +	q->type = video->desc.buf_type;
> +	q->io_modes = VB2_MMAP | VB2_DMABUF;
> +
> +	if (q->type == V4L2_BUF_TYPE_META_OUTPUT)
> +		q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> +	else
> +		q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_BOOTIME;

Why V4L2_BUF_TYPE_META_OUTPUT use different timestamp flag?
Different output data is generate at the same time,
so use the same timestamp so we are able to link these output data to the same frame.

And I could not find anywhere to define V4L2_BUF_FLAG_TIMESTAMP_BOOTIME.
So drop V4L2_BUF_FLAG_TIMESTAMP_BOOTIME and use V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC for all queue.

Regards,
CK

> +
> +	if (video->desc.smem_alloc) {
> +		q->bidirectional = 1;
> +		/* reserved memory */
> +		q->dev = cam->smem_dev;
> +	} else if (is_yuv_node(video->desc.id)) {
> +		q->dev = cam->raw.yuvs[0];
> +	} else {
> +		q->dev = cam->raw.devs[0];
> +	}
> +
> +	q->supports_requests = true;
> +	q->lock = &video->q_lock;
> +	q->ops = &mtk_cam_vb2_ops;
> +	q->mem_ops = &vb2_dma_contig_memops;
> +	q->drv_priv = cam;
> +	q->buf_struct_size = sizeof(struct mtk_cam_buffer);
> +
> +	if (output)
> +		q->timestamp_flags |= V4L2_BUF_FLAG_TSTAMP_SRC_EOF;
> +	else
> +		q->timestamp_flags |= V4L2_BUF_FLAG_TSTAMP_SRC_SOE;
> +
> +	/* No minimum buffers limitation */
> +	q->min_queued_buffers = 0;
> +
> +	ret = vb2_queue_init(q);
> +	if (ret < 0) {
> +		dev_info(v4l2_dev->dev, "Failed to init vb2 queue: %d\n", ret);
> +		goto error_vb2_init;
> +	}
> +
> +	pad->flags = output ? MEDIA_PAD_FL_SOURCE : MEDIA_PAD_FL_SINK;
> +	ret = media_entity_pads_init(&vdev->entity, 1, pad);
> +	if (ret < 0) {
> +		dev_info(v4l2_dev->dev, "Failed to init video entity: %d\n", ret);
> +		goto error_media_init;
> +	}
> +
> +	ret = mtk_video_init_format(video);
> +	if (ret < 0) {
> +		dev_info(v4l2_dev->dev, "Failed to init format: %d\n", ret);
> +		goto error_video_register;
> +	}
> +
> +	vdev->entity.function = MEDIA_ENT_F_IO_V4L;
> +	vdev->entity.ops = NULL;
> +	vdev->fops = &mtk_cam_v4l2_fops;
> +	vdev->device_caps = video->desc.cap | V4L2_CAP_STREAMING;
> +	vdev->v4l2_dev = v4l2_dev;
> +
> +	vdev->vfl_dir = output ? VFL_DIR_TX : VFL_DIR_RX;
> +	vdev->queue = &video->vb2_q;
> +	vdev->ioctl_ops = video->desc.ioctl_ops;
> +	vdev->release = video_device_release_empty;
> +	/* share q_lock */
> +	vdev->lock = &video->q_lock;
> +	strscpy(vdev->name, video->desc.name, sizeof(vdev->name));
> +
> +	ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);
> +	if (ret < 0) {
> +		dev_info(v4l2_dev->dev, "Failed to register video device: %d\n",
> +			 ret);
> +		goto error_video_register;
> +	}
> +	video_set_drvdata(vdev, cam);
> +
> +	dev_dbg(v4l2_dev->dev, "registered vdev:%d:%s\n",
> +		video->desc.id, vdev->name);
> +
> +	return 0;
> +
> +error_video_register:
> +	media_entity_cleanup(&vdev->entity);
> +error_media_init:
> +	vb2_queue_release(&video->vb2_q);
> +error_vb2_init:
> +	mutex_destroy(&video->q_lock);
> +
> +	return ret;
> +}
> +