Re: [PATCH v2 2/3] media: sunxi: Add H6 deinterlace driver

[Samuel Holland]

On 11/7/22 15:02, 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@xxxxxxxxx>
> ---
>  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      | 1149 +++++++++++++++++
>  .../platform/sunxi/sun50i-di/sun50i-di.h      |  175 +++
>  7 files changed, 1346 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
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index cf0f18502372..df35acab10fc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5781,12 +5781,14 @@ M:	"Maciej W. Rozycki" <macro@xxxxxxxxxxx>
>  S:	Maintained
>  F:	drivers/net/fddi/defza.*
>  
> -DEINTERLACE DRIVERS FOR ALLWINNER H3
> +DEINTERLACE DRIVERS FOR ALLWINNER SOCS
>  M:	Jernej Skrabec <jernej.skrabec@xxxxxxxxx>
>  L:	linux-media@xxxxxxxxxxxxxxx
>  S:	Maintained
>  T:	git git://linuxtv.org/media_tree.git
> +F:	Documentation/devicetree/bindings/media/allwinner,sun50i-h6-deinterlace.yaml
>  F:	Documentation/devicetree/bindings/media/allwinner,sun8i-h3-deinterlace.yaml

I don't think YAML bindings need to be listed here, since
get_maintainer.pl looks at the maintainers listed inside.

> +F:	drivers/media/platform/sunxi/sun50i-di/
>  F:	drivers/media/platform/sunxi/sun8i-di/
>  
>  DELL LAPTOP DRIVER
> diff --git a/drivers/media/platform/sunxi/Kconfig b/drivers/media/platform/sunxi/Kconfig
> index 2dd15083a1d9..413a79d23625 100644
> --- a/drivers/media/platform/sunxi/Kconfig
> +++ b/drivers/media/platform/sunxi/Kconfig
> @@ -7,4 +7,5 @@ source "drivers/media/platform/sunxi/sun6i-csi/Kconfig"
>  source "drivers/media/platform/sunxi/sun6i-mipi-csi2/Kconfig"
>  source "drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/Kconfig"
>  source "drivers/media/platform/sunxi/sun8i-di/Kconfig"
> +source "drivers/media/platform/sunxi/sun50i-di/Kconfig"
>  source "drivers/media/platform/sunxi/sun8i-rotate/Kconfig"
> diff --git a/drivers/media/platform/sunxi/Makefile b/drivers/media/platform/sunxi/Makefile
> index 9aa01cb01883..f92927f49f93 100644
> --- a/drivers/media/platform/sunxi/Makefile
> +++ b/drivers/media/platform/sunxi/Makefile
> @@ -5,4 +5,5 @@ obj-y		+= sun6i-csi/
>  obj-y		+= sun6i-mipi-csi2/
>  obj-y		+= sun8i-a83t-mipi-csi2/
>  obj-y		+= sun8i-di/
> +obj-y		+= sun50i-di/
>  obj-y		+= sun8i-rotate/

Would be nice to sort the above three files consistently.

> diff --git a/drivers/media/platform/sunxi/sun50i-di/Kconfig b/drivers/media/platform/sunxi/sun50i-di/Kconfig
> new file mode 100644
> index 000000000000..cc92f5086862
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun50i-di/Kconfig
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config VIDEO_SUN50I_DEINTERLACE
> +	tristate "Allwinner Deinterlace v2 driver"
> +	depends on V4L_MEM2MEM_DRIVERS
> +	depends on VIDEO_DEV
> +	depends on ARCH_SUNXI || COMPILE_TEST
> +	depends on COMMON_CLK && OF
> +	depends on PM
> +	select VIDEOBUF2_DMA_CONTIG
> +	select V4L2_MEM2MEM_DEV
> +	help
> +	  Support for the Allwinner deinterlace v2 unit found on
> +	  some SoCs, like H6.
> +	  To compile this driver as a module choose m here. The
> +	  module will be called sun50i-di.
> diff --git a/drivers/media/platform/sunxi/sun50i-di/Makefile b/drivers/media/platform/sunxi/sun50i-di/Makefile
> new file mode 100644
> index 000000000000..225b3b808069
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun50i-di/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_VIDEO_SUN50I_DEINTERLACE) += sun50i-di.o
> diff --git a/drivers/media/platform/sunxi/sun50i-di/sun50i-di.c b/drivers/media/platform/sunxi/sun50i-di/sun50i-di.c
> new file mode 100644
> index 000000000000..b2e53052c12b
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun50i-di/sun50i-di.c
[...]
> +static int deinterlace_runtime_resume(struct device *device)
> +{
> +	struct deinterlace_dev *dev = dev_get_drvdata(device);
> +	int ret;
> +
> +	ret = clk_set_rate_exclusive(dev->mod_clk, 300000000);
> +	if (ret) {
> +		dev_err(dev->dev, "Failed to set exclusive mod clock rate\n");
> +
> +		return ret;
> +	}
> +
> +	ret = reset_control_deassert(dev->rstc);
> +	if (ret) {
> +		dev_err(dev->dev, "Failed to apply reset\n");
> +
> +		return ret;

This should be `goto err_put_exclusive_rate;`. Otherwise,

Acked-by: Samuel Holland <samuel@xxxxxxxxxxxx>

Regards,
Samuel

> +	}
> +
> +	ret = clk_prepare_enable(dev->bus_clk);
> +	if (ret) {
> +		dev_err(dev->dev, "Failed to enable bus clock\n");
> +
> +		goto err_assert_reset;
> +	}
> +
> +	ret = clk_prepare_enable(dev->mod_clk);
> +	if (ret) {
> +		dev_err(dev->dev, "Failed to enable mod clock\n");
> +
> +		goto err_disable_bus_clk;
> +	}
> +
> +	ret = clk_prepare_enable(dev->ram_clk);
> +	if (ret) {
> +		dev_err(dev->dev, "Failed to enable ram clock\n");
> +
> +		goto err_disable_mod_clk;
> +	}
> +
> +	deinterlace_init(dev);
> +
> +	return 0;
> +
> +err_disable_mod_clk:
> +	clk_disable_unprepare(dev->mod_clk);
> +err_disable_bus_clk:
> +	clk_disable_unprepare(dev->bus_clk);
> +err_assert_reset:
> +	reset_control_assert(dev->rstc);
> +err_put_exclusive_rate:
> +	clk_rate_exclusive_put(dev->mod_clk);
> +
> +	return ret;
> +}