Hi Jernej,

On 11/1/22 07:32, Jernej Skrabec wrote:

This driver covers H6 deinterlace core, which is marked in vendor driver
as v2.3. Contrary to older cores, covered by sun8i-di, it doesn't
support scaling, but it supports iommu and has additional motion
compensated deinterlacing algorithm.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 MAINTAINERS                                   |    4 +-
 drivers/media/platform/sunxi/Kconfig          |    1 +
 drivers/media/platform/sunxi/Makefile         |    1 +
 .../media/platform/sunxi/sun50i-di/Kconfig    |   15 +
 .../media/platform/sunxi/sun50i-di/Makefile   |    2 +
 .../platform/sunxi/sun50i-di/sun50i-di.c      | 1142 +++++++++++++++++
 .../platform/sunxi/sun50i-di/sun50i-di.h      |  175 +++
 7 files changed, 1339 insertions(+), 1 deletion(-)
 create mode 100644 drivers/media/platform/sunxi/sun50i-di/Kconfig
 create mode 100644 drivers/media/platform/sunxi/sun50i-di/Makefile
 create mode 100644 drivers/media/platform/sunxi/sun50i-di/sun50i-di.c
 create mode 100644 drivers/media/platform/sunxi/sun50i-di/sun50i-di.h

[...]

+static int deinterlace_start_streaming(struct vb2_queue *vq, unsigned int count)
+{
+	struct deinterlace_ctx *ctx = vb2_get_drv_priv(vq);
+	struct device *dev = ctx->dev->dev;
+	int ret;
+
+	if (V4L2_TYPE_IS_OUTPUT(vq->type)) {
+		ret = pm_runtime_get_sync(dev);

This should use pm_runtime_resume_and_get().

+		if (ret < 0) {
+			dev_err(dev, "Failed to enable module\n");
+
+			goto err_runtime_get;
+		}
+
+		ctx->first_field =
+			ctx->src_fmt.field == V4L2_FIELD_INTERLACED_BT;
+		ctx->field = ctx->first_field;
+
+		ctx->prev[0] = NULL;
+		ctx->prev[1] = NULL;
+		ctx->aborting = 0;
+
+		ctx->flag1_buf = dma_alloc_coherent(dev, FLAG_SIZE,
+						    &ctx->flag1_buf_dma,
+						    GFP_KERNEL);
+		if (!ctx->flag1_buf) {
+			ret = -ENOMEM;
+
+			goto err_no_mem1;
+		}
+
+		ctx->flag2_buf = dma_alloc_coherent(dev, FLAG_SIZE,
+						    &ctx->flag2_buf_dma,
+						    GFP_KERNEL);
+		if (!ctx->flag2_buf) {
+			ret = -ENOMEM;
+
+			goto err_no_mem2;
+		}
+	}
+
+	return 0;
+
+err_no_mem2:
+	dma_free_coherent(dev, FLAG_SIZE, ctx->flag1_buf,
+			  ctx->flag1_buf_dma);
+err_no_mem1:
+	pm_runtime_put(dev);
+err_runtime_get:
+	deinterlace_queue_cleanup(vq, VB2_BUF_STATE_QUEUED);
+
+	return ret;
+}
+
+static void deinterlace_stop_streaming(struct vb2_queue *vq)
+{
+	struct deinterlace_ctx *ctx = vb2_get_drv_priv(vq);
+
+	if (V4L2_TYPE_IS_OUTPUT(vq->type)) {
+		struct device *dev = ctx->dev->dev;
+
+		dma_free_coherent(dev, FLAG_SIZE, ctx->flag1_buf,
+				  ctx->flag1_buf_dma);
+		dma_free_coherent(dev, FLAG_SIZE, ctx->flag2_buf,
+				  ctx->flag2_buf_dma);
+
+		pm_runtime_put(dev);
+	}
+
+	deinterlace_queue_cleanup(vq, VB2_BUF_STATE_ERROR);
+}
+
+static const struct vb2_ops deinterlace_qops = {
+	.queue_setup		= deinterlace_queue_setup,
+	.buf_prepare		= deinterlace_buf_prepare,
+	.buf_queue		= deinterlace_buf_queue,
+	.start_streaming	= deinterlace_start_streaming,
+	.stop_streaming		= deinterlace_stop_streaming,
+	.wait_prepare		= vb2_ops_wait_prepare,
+	.wait_finish		= vb2_ops_wait_finish,
+};
+
+static int deinterlace_queue_init(void *priv, struct vb2_queue *src_vq,
+				  struct vb2_queue *dst_vq)
+{
+	struct deinterlace_ctx *ctx = priv;
+	int ret;
+
+	src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
+	src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
+	src_vq->drv_priv = ctx;
+	src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
+	src_vq->min_buffers_needed = 1;
+	src_vq->ops = &deinterlace_qops;
+	src_vq->mem_ops = &vb2_dma_contig_memops;
+	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
+	src_vq->lock = &ctx->dev->dev_mutex;
+	src_vq->dev = ctx->dev->dev;
+
+	ret = vb2_queue_init(src_vq);
+	if (ret)
+		return ret;
+
+	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
+	dst_vq->drv_priv = ctx;
+	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
+	dst_vq->min_buffers_needed = 2;
+	dst_vq->ops = &deinterlace_qops;
+	dst_vq->mem_ops = &vb2_dma_contig_memops;
+	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
+	dst_vq->lock = &ctx->dev->dev_mutex;
+	dst_vq->dev = ctx->dev->dev;
+
+	ret = vb2_queue_init(dst_vq);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int deinterlace_open(struct file *file)
+{
+	struct deinterlace_dev *dev = video_drvdata(file);
+	struct deinterlace_ctx *ctx = NULL;
+	int ret;
+
+	if (mutex_lock_interruptible(&dev->dev_mutex))
+		return -ERESTARTSYS;
+
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);

This memory allocation does not need to be inside the lock.

+	if (!ctx) {
+		mutex_unlock(&dev->dev_mutex);
+		return -ENOMEM;
+	}
+
+	/* set default output and capture format */
+	deinterlace_set_out_format(ctx, &ctx->src_fmt);
+
+	v4l2_fh_init(&ctx->fh, video_devdata(file));
+	file->private_data = &ctx->fh;
+	ctx->dev = dev;
+
+	ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->m2m_dev, ctx,
+					    &deinterlace_queue_init);
+	if (IS_ERR(ctx->fh.m2m_ctx)) {
+		ret = PTR_ERR(ctx->fh.m2m_ctx);
+		goto err_free;
+	}
+
+	v4l2_fh_add(&ctx->fh);
+
+	mutex_unlock(&dev->dev_mutex);
+
+	return 0;
+
+err_free:
+	kfree(ctx);
+	mutex_unlock(&dev->dev_mutex);
+
+	return ret;
+}
+
+static int deinterlace_release(struct file *file)
+{
+	struct deinterlace_dev *dev = video_drvdata(file);
+	struct deinterlace_ctx *ctx = container_of(file->private_data,
+						   struct deinterlace_ctx, fh);
+
+	mutex_lock(&dev->dev_mutex);
+
+	v4l2_fh_del(&ctx->fh);
+	v4l2_fh_exit(&ctx->fh);
+	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
+
+	kfree(ctx);

This does not need to be inside the lock either.

+
+	mutex_unlock(&dev->dev_mutex);
+
+	return 0;
+}
+
+static const struct v4l2_file_operations deinterlace_fops = {
+	.owner		= THIS_MODULE,
+	.open		= deinterlace_open,
+	.release	= deinterlace_release,
+	.poll		= v4l2_m2m_fop_poll,
+	.unlocked_ioctl	= video_ioctl2,
+	.mmap		= v4l2_m2m_fop_mmap,
+};
+
+static const struct video_device deinterlace_video_device = {
+	.name		= DEINTERLACE_NAME,
+	.vfl_dir	= VFL_DIR_M2M,
+	.fops		= &deinterlace_fops,
+	.ioctl_ops	= &deinterlace_ioctl_ops,
+	.minor		= -1,
+	.release	= video_device_release_empty,
+	.device_caps	= V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING,
+};
+
+static const struct v4l2_m2m_ops deinterlace_m2m_ops = {
+	.device_run	= deinterlace_device_run,
+	.job_ready	= deinterlace_job_ready,
+	.job_abort	= deinterlace_job_abort,
+};
+
+static int deinterlace_probe(struct platform_device *pdev)
+{
+	struct deinterlace_dev *dev;
+	struct video_device *vfd;
+	int irq, ret;
+
+	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
+	if (!dev)
+		return -ENOMEM;
+
+	dev->vfd = deinterlace_video_device;
+	dev->dev = &pdev->dev;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq <= 0)
+		return irq;
+
+	ret = devm_request_irq(dev->dev, irq, deinterlace_irq,
+			       0, dev_name(dev->dev), dev);

You should expect to receive an interrupt as soon as you call this
function, but dev->base is not initialized yet.

+	if (ret) {
+		dev_err(dev->dev, "Failed to request IRQ\n");
+
+		return ret;
+	}
+
+	dev->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(dev->base))
+		return PTR_ERR(dev->base);
+
+	dev->bus_clk = devm_clk_get(dev->dev, "bus");
+	if (IS_ERR(dev->bus_clk))
+		return dev_err_probe(dev->dev, PTR_ERR(dev->bus_clk),
+				     "Failed to get bus clock\n");
+
+	dev->mod_clk = devm_clk_get(dev->dev, "mod");
+	if (IS_ERR(dev->mod_clk))
+		return dev_err_probe(dev->dev, PTR_ERR(dev->mod_clk),
+				     "Failed to get mod clock\n");
+
+	dev->ram_clk = devm_clk_get(dev->dev, "ram");
+	if (IS_ERR(dev->ram_clk))
+		return dev_err_probe(dev->dev, PTR_ERR(dev->ram_clk),
+				     "Failed to get ram clock\n");
+
+	dev->rstc = devm_reset_control_get(dev->dev, NULL);
+	if (IS_ERR(dev->rstc))
+		return dev_err_probe(dev->dev, PTR_ERR(dev->rstc),
+				     "Failed to get reset control\n");
+
+	dma_set_mask_and_coherent(dev->dev, DMA_BIT_MASK(32));
+	dma_set_max_seg_size(dev->dev, UINT_MAX);
+
+	mutex_init(&dev->dev_mutex);
+
+	INIT_DELAYED_WORK(&dev->watchdog_work, deinterlace_watchdog);
+
+	ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
+	if (ret) {
+		dev_err(dev->dev, "Failed to register V4L2 device\n");
+
+		return ret;
+	}
+
+	vfd = &dev->vfd;
+	vfd->lock = &dev->dev_mutex;
+	vfd->v4l2_dev = &dev->v4l2_dev;
+
+	snprintf(vfd->name, sizeof(vfd->name), "%s",
+		 deinterlace_video_device.name);
+	video_set_drvdata(vfd, dev);
+
+	ret = video_register_device(vfd, VFL_TYPE_VIDEO, 0);
+	if (ret) {
+		v4l2_err(&dev->v4l2_dev, "Failed to register video device\n");
+
+		goto err_v4l2;
+	}
+
+	v4l2_info(&dev->v4l2_dev,
+		  "Device registered as /dev/video%d\n", vfd->num);
+
+	dev->m2m_dev = v4l2_m2m_init(&deinterlace_m2m_ops);
+	if (IS_ERR(dev->m2m_dev)) {
+		v4l2_err(&dev->v4l2_dev,
+			 "Failed to initialize V4L2 M2M device\n");
+		ret = PTR_ERR(dev->m2m_dev);
+
+		goto err_video;
+	}
+
+	platform_set_drvdata(pdev, dev);
+
+	pm_runtime_enable(dev->dev);
+
+	return 0;
+
+err_video:
+	video_unregister_device(&dev->vfd);
+err_v4l2:
+	v4l2_device_unregister(&dev->v4l2_dev);
+
+	return ret;
+}
+
+static int deinterlace_remove(struct platform_device *pdev)
+{
+	struct deinterlace_dev *dev = platform_get_drvdata(pdev);
+
+	v4l2_m2m_release(dev->m2m_dev);
+	video_unregister_device(&dev->vfd);
+	v4l2_device_unregister(&dev->v4l2_dev);
+
+	pm_runtime_force_suspend(&pdev->dev);
+
+	return 0;
+}
+
+static int deinterlace_runtime_resume(struct device *device)
+{
+	struct deinterlace_dev *dev = dev_get_drvdata(device);
+	int ret;
+
+	ret = clk_prepare_enable(dev->bus_clk);
+	if (ret) {
+		dev_err(dev->dev, "Failed to enable bus clock\n");
+
+		return ret;
+	}
+
+	ret = clk_prepare_enable(dev->mod_clk);
+	if (ret) {
+		dev_err(dev->dev, "Failed to enable mod clock\n");
+
+		goto err_bus_clk;
+	}
+
+	ret = clk_prepare_enable(dev->ram_clk);
+	if (ret) {
+		dev_err(dev->dev, "Failed to enable ram clock\n");
+
+		goto err_mod_clk;
+	}
+
+	ret = reset_control_deassert(dev->rstc);
+	if (ret) {
+		dev_err(dev->dev, "Failed to apply reset\n");
+
+		goto err_ram_clk;
+	}

The manuals generally recommend deasserting the reset before enabling
the clock.

Also, the BSP sets the module clock rate to 300 MHz (PLL_PERIPH0 / 2).
Do you think we should do the same? I am guessing the default is 600 MHz
(PLL_PERIPH0 / 1), which is fine as long as the module works at that
frequency, although it may not be optimal for power usage.

The header guard doesn't match the file.

Regards,
Samuel


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel