Re: [PATCH v7 14/16] intel-ipu3: Add v4l2 driver based on media framework

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Yong,

On Mon, Oct 29, 2018 at 03:23:08PM -0700, Yong Zhi wrote:
> Implement video driver that utilizes v4l2, vb2 queue support
> and media controller APIs. The driver exposes single
> subdevice and six nodes.
> 
> Signed-off-by: Yong Zhi <yong.zhi@xxxxxxxxx>
> ---
>  drivers/media/pci/intel/ipu3/ipu3-v4l2.c | 1091 ++++++++++++++++++++++++++++++
>  1 file changed, 1091 insertions(+)
>  create mode 100644 drivers/media/pci/intel/ipu3/ipu3-v4l2.c
> 
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-v4l2.c b/drivers/media/pci/intel/ipu3/ipu3-v4l2.c
> new file mode 100644
> index 0000000..31a3514
> --- /dev/null
> +++ b/drivers/media/pci/intel/ipu3/ipu3-v4l2.c
> @@ -0,0 +1,1091 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2018 Intel Corporation
> +
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +
> +#include <media/v4l2-ioctl.h>
> +
> +#include "ipu3.h"
> +#include "ipu3-dmamap.h"
> +
> +/******************** v4l2_subdev_ops ********************/
> +
> +static int ipu3_subdev_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> +	struct imgu_device *imgu = container_of(sd, struct imgu_device, subdev);
> +	struct v4l2_rect try_crop = {
> +		.top = 0,
> +		.left = 0,
> +		.height = imgu->nodes[IMGU_NODE_IN].vdev_fmt.fmt.pix_mp.height,
> +		.width = imgu->nodes[IMGU_NODE_IN].vdev_fmt.fmt.pix_mp.width,
> +	};
> +	unsigned int i;
> +
> +	/* Initialize try_fmt */
> +	for (i = 0; i < IMGU_NODE_NUM; i++)
> +		*v4l2_subdev_get_try_format(sd, fh->pad, i) =
> +			imgu->nodes[i].pad_fmt;

The try formats should reflect the defaults, not the current device state.

> +
> +	*v4l2_subdev_get_try_crop(sd, fh->pad, IMGU_NODE_IN) = try_crop;

Same for the crop. How about the compose rectangle?

> +
> +	return 0;
> +}

...

> +int ipu3_v4l2_register(struct imgu_device *imgu)
> +{
> +	struct v4l2_mbus_framefmt def_bus_fmt = { 0 };
> +	struct v4l2_pix_format_mplane def_pix_fmt = { 0 };
> +

Extra newline.

> +	int i, r;
> +
> +	/* Initialize miscellaneous variables */
> +	imgu->streaming = false;
> +
> +	/* Init media device */
> +	media_device_pci_init(&imgu->media_dev, imgu->pci_dev, IMGU_NAME);
> +
> +	/* Set up v4l2 device */
> +	imgu->v4l2_dev.mdev = &imgu->media_dev;
> +	imgu->v4l2_dev.ctrl_handler = imgu->ctrl_handler;
> +	r = v4l2_device_register(&imgu->pci_dev->dev, &imgu->v4l2_dev);
> +	if (r) {
> +		dev_err(&imgu->pci_dev->dev,
> +			"failed to register V4L2 device (%d)\n", r);
> +		goto fail_v4l2_dev;
> +	}
> +
> +	/* Initialize subdev media entity */
> +	imgu->subdev_pads = kzalloc(sizeof(*imgu->subdev_pads) *
> +					IMGU_NODE_NUM, GFP_KERNEL);

As the number of pads is static, could you instead put the array directly
to the struct, instead of using a pointer? Remember to remove to
corresponding kfree, too.

> +	if (!imgu->subdev_pads) {
> +		r = -ENOMEM;
> +		goto fail_subdev_pads;
> +	}
> +	r = media_entity_pads_init(&imgu->subdev.entity, IMGU_NODE_NUM,
> +				   imgu->subdev_pads);
> +	if (r) {
> +		dev_err(&imgu->pci_dev->dev,
> +			"failed initialize subdev media entity (%d)\n", r);
> +		goto fail_media_entity;
> +	}
> +	imgu->subdev.entity.ops = &ipu3_media_ops;
> +	for (i = 0; i < IMGU_NODE_NUM; i++) {
> +		imgu->subdev_pads[i].flags = imgu->nodes[i].output ?
> +			MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
> +	}
> +
> +	/* Initialize subdev */
> +	v4l2_subdev_init(&imgu->subdev, &ipu3_subdev_ops);
> +	imgu->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_STATISTICS;
> +	imgu->subdev.internal_ops = &ipu3_subdev_internal_ops;
> +	imgu->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	strlcpy(imgu->subdev.name, IMGU_NAME, sizeof(imgu->subdev.name));

strscpy(), please. Same elsewhere in this and also on the 13th.

> +	v4l2_set_subdevdata(&imgu->subdev, imgu);
> +	imgu->subdev.ctrl_handler = imgu->ctrl_handler;
> +	r = v4l2_device_register_subdev(&imgu->v4l2_dev, &imgu->subdev);
> +	if (r) {
> +		dev_err(&imgu->pci_dev->dev,
> +			"failed initialize subdev (%d)\n", r);
> +		goto fail_subdev;
> +	}
> +	r = v4l2_device_register_subdev_nodes(&imgu->v4l2_dev);
> +	if (r) {
> +		dev_err(&imgu->pci_dev->dev,
> +			"failed to register subdevs (%d)\n", r);
> +		goto fail_subdevs;
> +	}
> +
> +	/* Initialize formats to default values */
> +	def_bus_fmt.width = 1920;
> +	def_bus_fmt.height = 1080;
> +	def_bus_fmt.code = MEDIA_BUS_FMT_UYVY8_2X8;
> +	def_bus_fmt.field = V4L2_FIELD_NONE;
> +	def_bus_fmt.colorspace = V4L2_COLORSPACE_RAW;
> +	def_bus_fmt.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> +	def_bus_fmt.quantization = V4L2_QUANTIZATION_DEFAULT;
> +	def_bus_fmt.xfer_func = V4L2_XFER_FUNC_DEFAULT;
> +
> +	def_pix_fmt.width = def_bus_fmt.width;
> +	def_pix_fmt.height = def_bus_fmt.height;
> +	def_pix_fmt.field = def_bus_fmt.field;
> +	def_pix_fmt.num_planes = 1;
> +	def_pix_fmt.plane_fmt[0].bytesperline = def_pix_fmt.width * 2;
> +	def_pix_fmt.plane_fmt[0].sizeimage =
> +		def_pix_fmt.height * def_pix_fmt.plane_fmt[0].bytesperline;
> +	def_pix_fmt.flags = 0;
> +	def_pix_fmt.colorspace = def_bus_fmt.colorspace;
> +	def_pix_fmt.ycbcr_enc = def_bus_fmt.ycbcr_enc;
> +	def_pix_fmt.quantization = def_bus_fmt.quantization;
> +	def_pix_fmt.xfer_func = def_bus_fmt.xfer_func;
> +
> +	/* Create video nodes and links */
> +	for (i = 0; i < IMGU_NODE_NUM; i++) {
> +		struct imgu_video_device *node = &imgu->nodes[i];
> +		struct video_device *vdev = &node->vdev;
> +		struct vb2_queue *vbq = &node->vbq;
> +		u32 flags;
> +
> +		/* Initialize miscellaneous variables */
> +		mutex_init(&node->lock);
> +		INIT_LIST_HEAD(&node->buffers);
> +
> +		/* Initialize formats to default values */
> +		node->pad_fmt = def_bus_fmt;
> +		ipu3_node_to_v4l2(i, vdev, &node->vdev_fmt);
> +		if (node->vdev_fmt.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE ||
> +		    node->vdev_fmt.type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
> +			def_pix_fmt.pixelformat = node->output ?
> +						V4L2_PIX_FMT_IPU3_SGRBG10 :
> +						V4L2_PIX_FMT_NV12;
> +			node->vdev_fmt.fmt.pix_mp = def_pix_fmt;
> +		}
> +		/* Initialize media entities */
> +		r = media_entity_pads_init(&vdev->entity, 1, &node->vdev_pad);
> +		if (r) {
> +			dev_err(&imgu->pci_dev->dev,
> +				"failed initialize media entity (%d)\n", r);
> +			goto fail_vdev_media_entity;
> +		}
> +		node->vdev_pad.flags = node->output ?
> +			MEDIA_PAD_FL_SOURCE : MEDIA_PAD_FL_SINK;
> +		vdev->entity.ops = NULL;
> +
> +		/* Initialize vbq */
> +		vbq->type = node->vdev_fmt.type;
> +		vbq->io_modes = VB2_USERPTR | VB2_MMAP | VB2_DMABUF;
> +		vbq->ops = &ipu3_vb2_ops;
> +		vbq->mem_ops = &vb2_dma_sg_memops;
> +		if (imgu->buf_struct_size <= 0)
> +			imgu->buf_struct_size = sizeof(struct ipu3_vb2_buffer);
> +		vbq->buf_struct_size = imgu->buf_struct_size;
> +		vbq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> +		vbq->min_buffers_needed = 0;	/* Can streamon w/o buffers */
> +		vbq->drv_priv = imgu;
> +		vbq->lock = &node->lock;
> +		r = vb2_queue_init(vbq);
> +		if (r) {
> +			dev_err(&imgu->pci_dev->dev,
> +				"failed to initialize video queue (%d)\n", r);
> +			goto fail_vdev;
> +		}
> +
> +		/* Initialize vdev */
> +		snprintf(vdev->name, sizeof(vdev->name), "%s %s",
> +			 IMGU_NAME, node->name);
> +		vdev->release = video_device_release_empty;
> +		vdev->fops = &ipu3_v4l2_fops;
> +		vdev->lock = &node->lock;
> +		vdev->v4l2_dev = &imgu->v4l2_dev;
> +		vdev->queue = &node->vbq;
> +		vdev->vfl_dir = node->output ? VFL_DIR_TX : VFL_DIR_RX;
> +		video_set_drvdata(vdev, imgu);
> +		r = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
> +		if (r) {
> +			dev_err(&imgu->pci_dev->dev,
> +				"failed to register video device (%d)\n", r);
> +			goto fail_vdev;
> +		}
> +
> +		/* Create link between video node and the subdev pad */
> +		flags = 0;
> +		if (node->enabled)
> +			flags |= MEDIA_LNK_FL_ENABLED;
> +		if (node->immutable)
> +			flags |= MEDIA_LNK_FL_IMMUTABLE;
> +		if (node->output) {
> +			r = media_create_pad_link(&vdev->entity, 0,
> +						  &imgu->subdev.entity,
> +						 i, flags);
> +		} else {
> +			r = media_create_pad_link(&imgu->subdev.entity,
> +						  i, &vdev->entity, 0, flags);
> +		}
> +		if (r)
> +			goto fail_link;
> +	}
> +
> +	r = media_device_register(&imgu->media_dev);
> +	if (r) {
> +		dev_err(&imgu->pci_dev->dev,
> +			"failed to register media device (%d)\n", r);
> +		i--;
> +		goto fail_link;
> +	}
> +
> +	return 0;
> +
> +	for (; i >= 0; i--) {
> +fail_link:
> +		video_unregister_device(&imgu->nodes[i].vdev);
> +fail_vdev:
> +		media_entity_cleanup(&imgu->nodes[i].vdev.entity);
> +fail_vdev_media_entity:
> +		mutex_destroy(&imgu->nodes[i].lock);
> +	}
> +fail_subdevs:
> +	v4l2_device_unregister_subdev(&imgu->subdev);
> +fail_subdev:
> +	media_entity_cleanup(&imgu->subdev.entity);
> +fail_media_entity:
> +	kfree(imgu->subdev_pads);
> +fail_subdev_pads:
> +	v4l2_device_unregister(&imgu->v4l2_dev);
> +fail_v4l2_dev:
> +	media_device_cleanup(&imgu->media_dev);
> +
> +	return r;
> +}
> +EXPORT_SYMBOL_GPL(ipu3_v4l2_register);
> +
> +int ipu3_v4l2_unregister(struct imgu_device *imgu)
> +{
> +	unsigned int i;
> +
> +	media_device_unregister(&imgu->media_dev);
> +	media_device_cleanup(&imgu->media_dev);

media_device_cleanup() should take place after unregistering the
sub-devices.

> +
> +	for (i = 0; i < IMGU_NODE_NUM; i++) {
> +		video_unregister_device(&imgu->nodes[i].vdev);
> +		media_entity_cleanup(&imgu->nodes[i].vdev.entity);
> +		mutex_destroy(&imgu->nodes[i].lock);
> +	}
> +
> +	v4l2_device_unregister_subdev(&imgu->subdev);
> +	media_entity_cleanup(&imgu->subdev.entity);
> +	kfree(imgu->subdev_pads);
> +	v4l2_device_unregister(&imgu->v4l2_dev);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(ipu3_v4l2_unregister);
> +
> +void ipu3_v4l2_buffer_done(struct vb2_buffer *vb,
> +			   enum vb2_buffer_state state)
> +{
> +	struct ipu3_vb2_buffer *b =
> +		container_of(vb, struct ipu3_vb2_buffer, vbb.vb2_buf);
> +
> +	list_del(&b->list);
> +	vb2_buffer_done(&b->vbb.vb2_buf, state);
> +}
> +EXPORT_SYMBOL_GPL(ipu3_v4l2_buffer_done);

-- 
Kind regards,

Sakari Ailus
sakari.ailus@xxxxxxxxxxxxxxx



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux