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 <[email protected]>
> ---
> 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
[email protected]