Message ID | 20221101123201.3021129-1-jernej.skrabec@gmail.com |
---|---|
Headers | show |
Series | media: sunxi: Add H6 deinterlace driver | expand |
On 04/11/2022 18:36, Jernej Škrabec wrote: > Hi Hans, > > Dne petek, 04. november 2022 ob 12:33:50 CET je Hans Verkuil napisal(a): >> Hi Jernej, >> >> Can you add a patch updating the MAINTAINERS file? Just post a 4/3 patch :-) > > It's already part of patch 2. I didn't see the reason to split it to separate > patch. I totally missed that. Apologies for the noise! Regards, Hans > > Best regards, > Jernej > >> >> Regards, >> >> Hans >> >> On 01/11/2022 13:31, Jernej Skrabec wrote: >>> This series implements driver for H6 deinterlace core, which is >>> newer version of core, covered by sun8i-di (v2.3). Contrary to >>> older one, it doesn't support scaling, but it supports iommu, >>> has additional motion compensated deinterlacing algorithm and >>> supports different pixel formats. >>> >>> v4l2-compliance 1.23.0-4961, 64 bits, 64-bit time_t >>> v4l2-compliance SHA: f86484524f32 2022-10-21 10:08:58 >>> >>> Compliance test for sun50i-di device /dev/video0: >>> >>> Driver Info: >>> Driver name : sun50i-di >>> Card type : sun50i-di >>> Bus info : platform:sun50i-di >>> Driver version : 6.1.0 >>> Capabilities : 0x84208000 >>> >>> Video Memory-to-Memory >>> Streaming >>> Extended Pix Format >>> Device Capabilities >>> >>> Device Caps : 0x04208000 >>> >>> Video Memory-to-Memory >>> Streaming >>> Extended Pix Format >>> >>> Required ioctls: >>> test VIDIOC_QUERYCAP: OK >>> test invalid ioctls: OK >>> >>> Allow for multiple opens: >>> test second /dev/video0 open: OK >>> test VIDIOC_QUERYCAP: OK >>> test VIDIOC_G/S_PRIORITY: OK >>> test for unlimited opens: OK >>> >>> Debug ioctls: >>> test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) >>> test VIDIOC_LOG_STATUS: OK (Not Supported) >>> >>> Input ioctls: >>> test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) >>> test VIDIOC_G/S_FREQUENCY: OK (Not Supported) >>> test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) >>> test VIDIOC_ENUMAUDIO: OK (Not Supported) >>> test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) >>> test VIDIOC_G/S_AUDIO: OK (Not Supported) >>> Inputs: 0 Audio Inputs: 0 Tuners: 0 >>> >>> Output ioctls: >>> test VIDIOC_G/S_MODULATOR: OK (Not Supported) >>> test VIDIOC_G/S_FREQUENCY: OK (Not Supported) >>> test VIDIOC_ENUMAUDOUT: OK (Not Supported) >>> test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) >>> test VIDIOC_G/S_AUDOUT: OK (Not Supported) >>> Outputs: 0 Audio Outputs: 0 Modulators: 0 >>> >>> Input/Output configuration ioctls: >>> test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) >>> test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) >>> test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) >>> test VIDIOC_G/S_EDID: OK (Not Supported) >>> >>> Control ioctls: >>> test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported) >>> test VIDIOC_QUERYCTRL: OK (Not Supported) >>> test VIDIOC_G/S_CTRL: OK (Not Supported) >>> test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported) >>> test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported) >>> test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) >>> Standard Controls: 0 Private Controls: 0 >>> >>> Format ioctls: >>> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK >>> test VIDIOC_G/S_PARM: OK (Not Supported) >>> test VIDIOC_G_FBUF: OK (Not Supported) >>> test VIDIOC_G_FMT: OK >>> test VIDIOC_TRY_FMT: OK >>> test VIDIOC_S_FMT: OK >>> test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) >>> test Cropping: OK (Not Supported) >>> test Composing: OK (Not Supported) >>> test Scaling: OK (Not Supported) >>> >>> Codec ioctls: >>> test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) >>> test VIDIOC_G_ENC_INDEX: OK (Not Supported) >>> test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) >>> >>> Buffer ioctls: >>> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK >>> test VIDIOC_EXPBUF: OK >>> test Requests: OK (Not Supported) >>> >>> Total for sun50i-di device /dev/video0: 45, Succeeded: 45, Failed: 0, >>> Warnings: 0 >>> >>> Best regards, >>> Jernej >>> >>> Jernej Skrabec (3): >>> media: dt-bindings: media: Add Allwinner H6 Deinterlace binding >>> media: sunxi: Add H6 deinterlace driver >>> arm64: dts: allwinner: h6: Add deinterlace node >>> >>> .../allwinner,sun50i-h6-deinterlace.yaml | 74 ++ >>> MAINTAINERS | 4 +- >>> arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 12 + >>> 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 +++ >>> 9 files changed, 1425 insertions(+), 1 deletion(-) >>> create mode 100644 >>> Documentation/devicetree/bindings/media/allwinner,sun50i-h6-deinterlace. >>> yaml 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 >>> >>> -- >>> 2.38.1 > > > >
On 11/1/22 07:31, Jernej Skrabec wrote: > Allwinner H6 Deinterlace core is used for deinterlacing interlaced video > content. > > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com> > --- > .../allwinner,sun50i-h6-deinterlace.yaml | 74 +++++++++++++++++++ > 1 file changed, 74 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun50i-h6-deinterlace.yaml Reviewed-by: Samuel Holland <samuel@sholland.org>
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. > + > + deinterlace_init(dev); > + > + return 0; > + > +err_ram_clk: > + clk_disable_unprepare(dev->ram_clk); > +err_mod_clk: > + clk_disable_unprepare(dev->mod_clk); > +err_bus_clk: > + clk_disable_unprepare(dev->bus_clk); > + > + return ret; > +} > + > +static int deinterlace_runtime_suspend(struct device *device) > +{ > + struct deinterlace_dev *dev = dev_get_drvdata(device); > + > + reset_control_assert(dev->rstc); > + > + clk_disable_unprepare(dev->ram_clk); > + clk_disable_unprepare(dev->mod_clk); > + clk_disable_unprepare(dev->bus_clk); > + > + return 0; > +} > + > +static const struct of_device_id deinterlace_dt_match[] = { > + { .compatible = "allwinner,sun50i-h6-deinterlace" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, deinterlace_dt_match); > + > +static const struct dev_pm_ops deinterlace_pm_ops = { > + .runtime_resume = deinterlace_runtime_resume, > + .runtime_suspend = deinterlace_runtime_suspend, > +}; > + > +static struct platform_driver deinterlace_driver = { > + .probe = deinterlace_probe, > + .remove = deinterlace_remove, > + .driver = { > + .name = DEINTERLACE_NAME, > + .of_match_table = deinterlace_dt_match, > + .pm = &deinterlace_pm_ops, > + }, > +}; > +module_platform_driver(deinterlace_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Jernej Skrabec <jernej.skrabec@gmail.com>"); > +MODULE_DESCRIPTION("Allwinner sun50i deinterlace driver"); > diff --git a/drivers/media/platform/sunxi/sun50i-di/sun50i-di.h b/drivers/media/platform/sunxi/sun50i-di/sun50i-di.h > new file mode 100644 > index 000000000000..c6eae30040e9 > --- /dev/null > +++ b/drivers/media/platform/sunxi/sun50i-di/sun50i-di.h > @@ -0,0 +1,175 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Allwinner Deinterlace driver > + * > + * Copyright (C) 2020 Jernej Skrabec <jernej.skrabec@gmail.com> > + */ > + > +#ifndef _SUN8I_DEINTERLACE_H_ > +#define _SUN8I_DEINTERLACE_H_ The header guard doesn't match the file. Regards, Samuel