diff mbox

[05/15] drm/sun4i: Add TCON TOP driver

Message ID 20180519183127.2718-6-jernej.skrabec@siol.net (mailing list archive)
State New, archived
Headers show

Commit Message

Jernej Škrabec May 19, 2018, 6:31 p.m. UTC 
As already described in DT binding, TCON TOP is responsible for
configuring display pipeline. In this initial driver focus is on HDMI
pipeline, so TVE and LCD configuration is not implemented.

Implemented features:
- HDMI source selection
- clock driver (TCON and DSI gating)
- connecting mixers and TCONS

Something similar also existed in previous SoCs, except that it was part
of first TCON.

Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
 drivers/gpu/drm/sun4i/Makefile             |   3 +-
 drivers/gpu/drm/sun4i/sun8i_tcon_top.c     | 256 +++++++++++++++++++++
 drivers/gpu/drm/sun4i/sun8i_tcon_top.h     |  20 ++
 include/dt-bindings/clock/sun8i-tcon-top.h |  11 +
 4 files changed, 289 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/sun4i/sun8i_tcon_top.c
 create mode 100644 drivers/gpu/drm/sun4i/sun8i_tcon_top.h
 create mode 100644 include/dt-bindings/clock/sun8i-tcon-top.h

Comments

Maxime Ripard May 21, 2018, 8:05 a.m. UTC | #1 
On Sat, May 19, 2018 at 08:31:17PM +0200, Jernej Skrabec wrote:
> As already described in DT binding, TCON TOP is responsible for
> configuring display pipeline. In this initial driver focus is on HDMI
> pipeline, so TVE and LCD configuration is not implemented.
> 
> Implemented features:
> - HDMI source selection
> - clock driver (TCON and DSI gating)
> - connecting mixers and TCONS
> 
> Something similar also existed in previous SoCs, except that it was part
> of first TCON.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  drivers/gpu/drm/sun4i/Makefile             |   3 +-
>  drivers/gpu/drm/sun4i/sun8i_tcon_top.c     | 256 +++++++++++++++++++++
>  drivers/gpu/drm/sun4i/sun8i_tcon_top.h     |  20 ++
>  include/dt-bindings/clock/sun8i-tcon-top.h |  11 +
>  4 files changed, 289 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/sun4i/sun8i_tcon_top.c
>  create mode 100644 drivers/gpu/drm/sun4i/sun8i_tcon_top.h
>  create mode 100644 include/dt-bindings/clock/sun8i-tcon-top.h
> 
> diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile
> index 2589f4acd5ae..09fbfd6304ba 100644
> --- a/drivers/gpu/drm/sun4i/Makefile
> +++ b/drivers/gpu/drm/sun4i/Makefile
> @@ -16,7 +16,8 @@ sun8i-drm-hdmi-y		+= sun8i_hdmi_phy_clk.o
>  
>  sun8i-mixer-y			+= sun8i_mixer.o sun8i_ui_layer.o \
>  				   sun8i_vi_layer.o sun8i_ui_scaler.o \
> -				   sun8i_vi_scaler.o sun8i_csc.o
> +				   sun8i_vi_scaler.o sun8i_csc.o \
> +				   sun8i_tcon_top.o
>  
>  sun4i-tcon-y			+= sun4i_crtc.o
>  sun4i-tcon-y			+= sun4i_dotclock.o
> diff --git a/drivers/gpu/drm/sun4i/sun8i_tcon_top.c b/drivers/gpu/drm/sun4i/sun8i_tcon_top.c
> new file mode 100644
> index 000000000000..075a356a6dfa
> --- /dev/null
> +++ b/drivers/gpu/drm/sun4i/sun8i_tcon_top.c
> @@ -0,0 +1,256 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/* Copyright (c) 2018 Jernej Skrabec <jernej.skrabec@siol.net> */
> +
> +#include <drm/drmP.h>
> +
> +#include <dt-bindings/clock/sun8i-tcon-top.h>
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/spinlock.h>
> +
> +#include "sun8i_tcon_top.h"
> +
> +#define TCON_TOP_PORT_SEL_REG		0x1C
> +#define TCON_TOP_PORT_DE0_MSK			GENMASK(1, 0)
> +#define TCON_TOP_PORT_DE1_MSK			GENMASK(5, 4)
> +#define TCON_TOP_PORT_TCON_LCD0			0
> +#define TCON_TOP_PORT_TCON_LCD1			1
> +#define TCON_TOP_PORT_TCON_TV0			2
> +#define TCON_TOP_PORT_TCON_TV1			3
> +
> +#define TCON_TOP_GATE_SRC_REG		0x20
> +#define TCON_TOP_HDMI_SRC_MSK			GENMASK(29, 28)
> +#define TCON_TOP_HDMI_SRC_NONE			0
> +#define TCON_TOP_HDMI_SRC_TCON_TV0		1
> +#define TCON_TOP_HDMI_SRC_TCON_TV1		2
> +#define TCON_TOP_TCON_TV1_GATE			24
> +#define TCON_TOP_TCON_TV0_GATE			20
> +#define TCON_TOP_TCON_DSI_GATE			16
> +
> +#define CLK_NUM					3
> +
> +struct sun8i_tcon_top {
> +	struct clk		*bus;
> +	void __iomem		*regs;
> +	struct reset_control	*rst;
> +
> +	/*
> +	 * spinlock is used for locking access to registers from different
> +	 * places - tcon driver and clk subsystem.
> +	 */
> +	spinlock_t		reg_lock;
> +};
> +
> +struct sun8i_tcon_top_gate {
> +	const char	*name;
> +	u8		bit;
> +	int		index;
> +};
> +
> +static const struct sun8i_tcon_top_gate gates[] = {
> +	{"bus-tcon-top-dsi", TCON_TOP_TCON_DSI_GATE, CLK_BUS_TCON_TOP_DSI},
> +	{"bus-tcon-top-tv0", TCON_TOP_TCON_TV0_GATE, CLK_BUS_TCON_TOP_TV0},
> +	{"bus-tcon-top-tv1", TCON_TOP_TCON_TV1_GATE, CLK_BUS_TCON_TOP_TV1},
> +};
> +
> +void sun8i_tcon_top_set_hdmi_src(struct sun8i_tcon_top *tcon_top, int tcon)
> +{
> +	unsigned long flags;
> +	u32 val;
> +
> +	if (tcon > 1) {
> +		DRM_ERROR("TCON index is too high!\n");
> +		return;
> +	}
> +
> +	spin_lock_irqsave(&tcon_top->reg_lock, flags);
> +
> +	val = readl(tcon_top->regs + TCON_TOP_GATE_SRC_REG);
> +	val &= ~TCON_TOP_HDMI_SRC_MSK;
> +	val |= FIELD_PREP(TCON_TOP_HDMI_SRC_MSK,
> +			  TCON_TOP_HDMI_SRC_TCON_TV0 + tcon);
> +	writel(val, tcon_top->regs + TCON_TOP_GATE_SRC_REG);
> +
> +	spin_unlock_irqrestore(&tcon_top->reg_lock, flags);
> +}
> +
> +void sun8i_tcon_top_de_config(struct sun8i_tcon_top *tcon_top,
> +			      int mixer, enum tcon_type tcon_type, int tcon)
> +{
> +	unsigned long flags;
> +	u32 val, reg;
> +
> +	if (mixer > 1) {
> +		DRM_ERROR("Mixer index is too high!\n");
> +		return;
> +	}
> +
> +	if (tcon > 1) {
> +		DRM_ERROR("TCON index is too high!\n");
> +		return;
> +	}
> +
> +	switch (tcon_type) {
> +	case tcon_type_lcd:
> +		val = TCON_TOP_PORT_TCON_LCD0 + tcon;
> +		break;
> +	case tcon_type_tv:
> +		val = TCON_TOP_PORT_TCON_TV0 + tcon;
> +		break;
> +	default:
> +		DRM_ERROR("Invalid TCON type!\n");
> +		return;
> +	}
> +
> +	spin_lock_irqsave(&tcon_top->reg_lock, flags);
> +
> +	reg = readl(tcon_top->regs + TCON_TOP_PORT_SEL_REG);
> +	if (mixer == 0) {
> +		reg &= ~TCON_TOP_PORT_DE0_MSK;
> +		reg |= FIELD_PREP(TCON_TOP_PORT_DE0_MSK, val);
> +	} else {
> +		reg &= ~TCON_TOP_PORT_DE1_MSK;
> +		reg |= FIELD_PREP(TCON_TOP_PORT_DE1_MSK, val);
> +	}
> +	writel(reg, tcon_top->regs + TCON_TOP_PORT_SEL_REG);
> +
> +	spin_unlock_irqrestore(&tcon_top->reg_lock, flags);
> +}
> +
> +static int sun8i_tcon_top_probe(struct platform_device *pdev)
> +{
> +	struct clk_hw_onecell_data *clk_data;
> +	struct sun8i_tcon_top *tcon_top;
> +	struct device *dev = &pdev->dev;
> +	struct resource *res;
> +	int ret, i;
> +
> +	tcon_top = devm_kzalloc(dev, sizeof(*tcon_top), GFP_KERNEL);
> +	if (!tcon_top)
> +		return -ENOMEM;
> +
> +	clk_data = devm_kzalloc(&pdev->dev, sizeof(*clk_data) +
> +				sizeof(*clk_data->hws) * CLK_NUM,
> +				GFP_KERNEL);
> +	if (!clk_data)
> +		return -ENOMEM;
> +
> +	spin_lock_init(&tcon_top->reg_lock);
> +
> +	tcon_top->rst = devm_reset_control_get(dev, "rst");
> +	if (IS_ERR(tcon_top->rst)) {
> +		dev_err(dev, "Couldn't get our reset line\n");
> +		return PTR_ERR(tcon_top->rst);
> +	}
> +
> +	tcon_top->bus = devm_clk_get(dev, "bus");
> +	if (IS_ERR(tcon_top->bus)) {
> +		dev_err(dev, "Couldn't get the bus clock\n");
> +		return PTR_ERR(tcon_top->bus);
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	tcon_top->regs = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(tcon_top->regs))
> +		return PTR_ERR(tcon_top->regs);
> +
> +	ret = reset_control_deassert(tcon_top->rst);
> +	if (ret) {
> +		dev_err(dev, "Could not deassert ctrl reset control\n");
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(tcon_top->bus);
> +	if (ret) {
> +		dev_err(dev, "Could not enable bus clock\n");
> +		goto err_assert_reset;
> +	}
> +
> +	/*
> +	 * Default register values might have some reserved bits set, which
> +	 * prevents TCON TOP from working properly. Set them to 0 here.
> +	 */
> +	writel(0, tcon_top->regs + TCON_TOP_PORT_SEL_REG);
> +	writel(0, tcon_top->regs + TCON_TOP_GATE_SRC_REG);
> +
> +	for (i = 0; i < CLK_NUM; i++) {
> +		const char *parent_name = "bus-tcon-top";

I guess retrieving the parent's clock name at runtime would be more
flexible.

> +		struct clk_init_data init;
> +		struct clk_gate *gate;
> +
> +		gate = devm_kzalloc(dev, sizeof(*gate), GFP_KERNEL);
> +		if (!gate) {
> +			ret = -ENOMEM;
> +			goto err_disable_clock;
> +		}
> +
> +		init.name = gates[i].name;
> +		init.ops = &clk_gate_ops;
> +		init.flags = CLK_IS_BASIC;
> +		init.parent_names = &parent_name;
> +		init.num_parents = 1;
> +
> +		gate->reg = tcon_top->regs + TCON_TOP_GATE_SRC_REG;
> +		gate->bit_idx = gates[i].bit;
> +		gate->lock = &tcon_top->reg_lock;
> +		gate->hw.init = &init;
> +
> +		ret = devm_clk_hw_register(dev, &gate->hw);
> +		if (ret)
> +			goto err_disable_clock;

Isn't it what clk_hw_register_gate is doing?

> +		clk_data->hws[gates[i].index] = &gate->hw;
> +	}
> +
> +	clk_data->num = CLK_NUM;
> +
> +	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
> +	if (ret)
> +		goto err_disable_clock;
> +
> +	platform_set_drvdata(pdev, tcon_top);
> +
> +	return 0;
> +
> +err_disable_clock:
> +	clk_disable_unprepare(tcon_top->bus);
> +err_assert_reset:
> +	reset_control_assert(tcon_top->rst);
> +
> +	return ret;
> +}
> +
> +static int sun8i_tcon_top_remove(struct platform_device *pdev)
> +{
> +	struct sun8i_tcon_top *tcon_top = platform_get_drvdata(pdev);
> +
> +	clk_disable_unprepare(tcon_top->bus);
> +	reset_control_assert(tcon_top->rst);
> +
> +	return 0;
> +}
> +
> +const struct of_device_id sun8i_tcon_top_of_table[] = {
> +	{ .compatible = "allwinner,sun8i-r40-tcon-top" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, sun8i_tcon_top_of_table);
> +
> +static struct platform_driver sun8i_tcon_top_platform_driver = {
> +	.probe		= sun8i_tcon_top_probe,
> +	.remove		= sun8i_tcon_top_remove,
> +	.driver		= {
> +		.name		= "sun8i-tcon-top",
> +		.of_match_table	= sun8i_tcon_top_of_table,
> +	},
> +};
> +module_platform_driver(sun8i_tcon_top_platform_driver);
> +
> +MODULE_AUTHOR("Jernej Skrabec <jernej.skrabec@siol.net>");
> +MODULE_DESCRIPTION("Allwinner R40 TCON TOP driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/gpu/drm/sun4i/sun8i_tcon_top.h b/drivers/gpu/drm/sun4i/sun8i_tcon_top.h
> new file mode 100644
> index 000000000000..19126e07d2a6
> --- /dev/null
> +++ b/drivers/gpu/drm/sun4i/sun8i_tcon_top.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/* Copyright (c) 2018 Jernej Skrabec <jernej.skrabec@siol.net> */
> +
> +#ifndef _SUN8I_TCON_TOP_H_
> +#define _SUN8I_TCON_TOP_H_
> +
> +#include <linux/device.h>
> +
> +struct sun8i_tcon_top;
> +
> +enum tcon_type {
> +	tcon_type_lcd,
> +	tcon_type_tv,

The usual practice is to have the enum values upper-case.

Thanks!
Maxime
Jernej Škrabec May 21, 2018, 3:15 p.m. UTC | #2 
Hi,

Dne ponedeljek, 21. maj 2018 ob 10:05:17 CEST je Maxime Ripard napisal(a):
> On Sat, May 19, 2018 at 08:31:17PM +0200, Jernej Skrabec wrote:
> > As already described in DT binding, TCON TOP is responsible for
> > configuring display pipeline. In this initial driver focus is on HDMI
> > pipeline, so TVE and LCD configuration is not implemented.
> > 
> > Implemented features:
> > - HDMI source selection
> > - clock driver (TCON and DSI gating)
> > - connecting mixers and TCONS
> > 
> > Something similar also existed in previous SoCs, except that it was part
> > of first TCON.
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > ---
> > 
> >  drivers/gpu/drm/sun4i/Makefile             |   3 +-
> >  drivers/gpu/drm/sun4i/sun8i_tcon_top.c     | 256 +++++++++++++++++++++
> >  drivers/gpu/drm/sun4i/sun8i_tcon_top.h     |  20 ++
> >  include/dt-bindings/clock/sun8i-tcon-top.h |  11 +
> >  4 files changed, 289 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/gpu/drm/sun4i/sun8i_tcon_top.c
> >  create mode 100644 drivers/gpu/drm/sun4i/sun8i_tcon_top.h
> >  create mode 100644 include/dt-bindings/clock/sun8i-tcon-top.h
> > 
> > diff --git a/drivers/gpu/drm/sun4i/Makefile
> > b/drivers/gpu/drm/sun4i/Makefile index 2589f4acd5ae..09fbfd6304ba 100644
> > --- a/drivers/gpu/drm/sun4i/Makefile
> > +++ b/drivers/gpu/drm/sun4i/Makefile
> > @@ -16,7 +16,8 @@ sun8i-drm-hdmi-y		+= sun8i_hdmi_phy_clk.o
> > 
> >  sun8i-mixer-y			+= sun8i_mixer.o sun8i_ui_layer.o \
> >  
> >  				   sun8i_vi_layer.o sun8i_ui_scaler.o \
> > 
> > -				   sun8i_vi_scaler.o sun8i_csc.o
> > +				   sun8i_vi_scaler.o sun8i_csc.o \
> > +				   sun8i_tcon_top.o
> > 
> >  sun4i-tcon-y			+= sun4i_crtc.o
> >  sun4i-tcon-y			+= sun4i_dotclock.o
> > 
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_tcon_top.c
> > b/drivers/gpu/drm/sun4i/sun8i_tcon_top.c new file mode 100644
> > index 000000000000..075a356a6dfa
> > --- /dev/null
> > +++ b/drivers/gpu/drm/sun4i/sun8i_tcon_top.c
> > @@ -0,0 +1,256 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/* Copyright (c) 2018 Jernej Skrabec <jernej.skrabec@siol.net> */
> > +
> > +#include <drm/drmP.h>
> > +
> > +#include <dt-bindings/clock/sun8i-tcon-top.h>
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/clk.h>
> > +#include <linux/clk-provider.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/reset.h>
> > +#include <linux/spinlock.h>
> > +
> > +#include "sun8i_tcon_top.h"
> > +
> > +#define TCON_TOP_PORT_SEL_REG		0x1C
> > +#define TCON_TOP_PORT_DE0_MSK			GENMASK(1, 0)
> > +#define TCON_TOP_PORT_DE1_MSK			GENMASK(5, 4)
> > +#define TCON_TOP_PORT_TCON_LCD0			0
> > +#define TCON_TOP_PORT_TCON_LCD1			1
> > +#define TCON_TOP_PORT_TCON_TV0			2
> > +#define TCON_TOP_PORT_TCON_TV1			3
> > +
> > +#define TCON_TOP_GATE_SRC_REG		0x20
> > +#define TCON_TOP_HDMI_SRC_MSK			GENMASK(29, 28)
> > +#define TCON_TOP_HDMI_SRC_NONE			0
> > +#define TCON_TOP_HDMI_SRC_TCON_TV0		1
> > +#define TCON_TOP_HDMI_SRC_TCON_TV1		2
> > +#define TCON_TOP_TCON_TV1_GATE			24
> > +#define TCON_TOP_TCON_TV0_GATE			20
> > +#define TCON_TOP_TCON_DSI_GATE			16
> > +
> > +#define CLK_NUM					3
> > +
> > +struct sun8i_tcon_top {
> > +	struct clk		*bus;
> > +	void __iomem		*regs;
> > +	struct reset_control	*rst;
> > +
> > +	/*
> > +	 * spinlock is used for locking access to registers from different
> > +	 * places - tcon driver and clk subsystem.
> > +	 */
> > +	spinlock_t		reg_lock;
> > +};
> > +
> > +struct sun8i_tcon_top_gate {
> > +	const char	*name;
> > +	u8		bit;
> > +	int		index;
> > +};
> > +
> > +static const struct sun8i_tcon_top_gate gates[] = {
> > +	{"bus-tcon-top-dsi", TCON_TOP_TCON_DSI_GATE, CLK_BUS_TCON_TOP_DSI},
> > +	{"bus-tcon-top-tv0", TCON_TOP_TCON_TV0_GATE, CLK_BUS_TCON_TOP_TV0},
> > +	{"bus-tcon-top-tv1", TCON_TOP_TCON_TV1_GATE, CLK_BUS_TCON_TOP_TV1},
> > +};
> > +
> > +void sun8i_tcon_top_set_hdmi_src(struct sun8i_tcon_top *tcon_top, int
> > tcon) +{
> > +	unsigned long flags;
> > +	u32 val;
> > +
> > +	if (tcon > 1) {
> > +		DRM_ERROR("TCON index is too high!\n");
> > +		return;
> > +	}
> > +
> > +	spin_lock_irqsave(&tcon_top->reg_lock, flags);
> > +
> > +	val = readl(tcon_top->regs + TCON_TOP_GATE_SRC_REG);
> > +	val &= ~TCON_TOP_HDMI_SRC_MSK;
> > +	val |= FIELD_PREP(TCON_TOP_HDMI_SRC_MSK,
> > +			  TCON_TOP_HDMI_SRC_TCON_TV0 + tcon);
> > +	writel(val, tcon_top->regs + TCON_TOP_GATE_SRC_REG);
> > +
> > +	spin_unlock_irqrestore(&tcon_top->reg_lock, flags);
> > +}
> > +
> > +void sun8i_tcon_top_de_config(struct sun8i_tcon_top *tcon_top,
> > +			      int mixer, enum tcon_type tcon_type, int tcon)
> > +{
> > +	unsigned long flags;
> > +	u32 val, reg;
> > +
> > +	if (mixer > 1) {
> > +		DRM_ERROR("Mixer index is too high!\n");
> > +		return;
> > +	}
> > +
> > +	if (tcon > 1) {
> > +		DRM_ERROR("TCON index is too high!\n");
> > +		return;
> > +	}
> > +
> > +	switch (tcon_type) {
> > +	case tcon_type_lcd:
> > +		val = TCON_TOP_PORT_TCON_LCD0 + tcon;
> > +		break;
> > +	case tcon_type_tv:
> > +		val = TCON_TOP_PORT_TCON_TV0 + tcon;
> > +		break;
> > +	default:
> > +		DRM_ERROR("Invalid TCON type!\n");
> > +		return;
> > +	}
> > +
> > +	spin_lock_irqsave(&tcon_top->reg_lock, flags);
> > +
> > +	reg = readl(tcon_top->regs + TCON_TOP_PORT_SEL_REG);
> > +	if (mixer == 0) {
> > +		reg &= ~TCON_TOP_PORT_DE0_MSK;
> > +		reg |= FIELD_PREP(TCON_TOP_PORT_DE0_MSK, val);
> > +	} else {
> > +		reg &= ~TCON_TOP_PORT_DE1_MSK;
> > +		reg |= FIELD_PREP(TCON_TOP_PORT_DE1_MSK, val);
> > +	}
> > +	writel(reg, tcon_top->regs + TCON_TOP_PORT_SEL_REG);
> > +
> > +	spin_unlock_irqrestore(&tcon_top->reg_lock, flags);
> > +}
> > +
> > +static int sun8i_tcon_top_probe(struct platform_device *pdev)
> > +{
> > +	struct clk_hw_onecell_data *clk_data;
> > +	struct sun8i_tcon_top *tcon_top;
> > +	struct device *dev = &pdev->dev;
> > +	struct resource *res;
> > +	int ret, i;
> > +
> > +	tcon_top = devm_kzalloc(dev, sizeof(*tcon_top), GFP_KERNEL);
> > +	if (!tcon_top)
> > +		return -ENOMEM;
> > +
> > +	clk_data = devm_kzalloc(&pdev->dev, sizeof(*clk_data) +
> > +				sizeof(*clk_data->hws) * CLK_NUM,
> > +				GFP_KERNEL);
> > +	if (!clk_data)
> > +		return -ENOMEM;
> > +
> > +	spin_lock_init(&tcon_top->reg_lock);
> > +
> > +	tcon_top->rst = devm_reset_control_get(dev, "rst");
> > +	if (IS_ERR(tcon_top->rst)) {
> > +		dev_err(dev, "Couldn't get our reset line\n");
> > +		return PTR_ERR(tcon_top->rst);
> > +	}
> > +
> > +	tcon_top->bus = devm_clk_get(dev, "bus");
> > +	if (IS_ERR(tcon_top->bus)) {
> > +		dev_err(dev, "Couldn't get the bus clock\n");
> > +		return PTR_ERR(tcon_top->bus);
> > +	}
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	tcon_top->regs = devm_ioremap_resource(dev, res);
> > +	if (IS_ERR(tcon_top->regs))
> > +		return PTR_ERR(tcon_top->regs);
> > +
> > +	ret = reset_control_deassert(tcon_top->rst);
> > +	if (ret) {
> > +		dev_err(dev, "Could not deassert ctrl reset control\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = clk_prepare_enable(tcon_top->bus);
> > +	if (ret) {
> > +		dev_err(dev, "Could not enable bus clock\n");
> > +		goto err_assert_reset;
> > +	}
> > +
> > +	/*
> > +	 * Default register values might have some reserved bits set, which
> > +	 * prevents TCON TOP from working properly. Set them to 0 here.
> > +	 */
> > +	writel(0, tcon_top->regs + TCON_TOP_PORT_SEL_REG);
> > +	writel(0, tcon_top->regs + TCON_TOP_GATE_SRC_REG);
> > +
> > +	for (i = 0; i < CLK_NUM; i++) {
> > +		const char *parent_name = "bus-tcon-top";
> 
> I guess retrieving the parent's clock name at runtime would be more
> flexible.
> 

It is, but will it ever be anything else?

> > +		struct clk_init_data init;
> > +		struct clk_gate *gate;
> > +
> > +		gate = devm_kzalloc(dev, sizeof(*gate), GFP_KERNEL);
> > +		if (!gate) {
> > +			ret = -ENOMEM;
> > +			goto err_disable_clock;
> > +		}
> > +
> > +		init.name = gates[i].name;
> > +		init.ops = &clk_gate_ops;
> > +		init.flags = CLK_IS_BASIC;
> > +		init.parent_names = &parent_name;
> > +		init.num_parents = 1;
> > +
> > +		gate->reg = tcon_top->regs + TCON_TOP_GATE_SRC_REG;
> > +		gate->bit_idx = gates[i].bit;
> > +		gate->lock = &tcon_top->reg_lock;
> > +		gate->hw.init = &init;
> > +
> > +		ret = devm_clk_hw_register(dev, &gate->hw);
> > +		if (ret)
> > +			goto err_disable_clock;
> 
> Isn't it what clk_hw_register_gate is doing?
> 

Almost, but not exactly. My goal was to use devm_* functions, so there is no 
need to do any special cleanup.

> > +		clk_data->hws[gates[i].index] = &gate->hw;
> > +	}
> > +
> > +	clk_data->num = CLK_NUM;
> > +
> > +	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, 
clk_data);
> > +	if (ret)
> > +		goto err_disable_clock;
> > +
> > +	platform_set_drvdata(pdev, tcon_top);
> > +
> > +	return 0;
> > +
> > +err_disable_clock:
> > +	clk_disable_unprepare(tcon_top->bus);
> > +err_assert_reset:
> > +	reset_control_assert(tcon_top->rst);
> > +
> > +	return ret;
> > +}
> > +
> > +static int sun8i_tcon_top_remove(struct platform_device *pdev)
> > +{
> > +	struct sun8i_tcon_top *tcon_top = platform_get_drvdata(pdev);
> > +
> > +	clk_disable_unprepare(tcon_top->bus);
> > +	reset_control_assert(tcon_top->rst);
> > +
> > +	return 0;
> > +}
> > +
> > +const struct of_device_id sun8i_tcon_top_of_table[] = {
> > +	{ .compatible = "allwinner,sun8i-r40-tcon-top" },
> > +	{ /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, sun8i_tcon_top_of_table);
> > +
> > +static struct platform_driver sun8i_tcon_top_platform_driver = {
> > +	.probe		= sun8i_tcon_top_probe,
> > +	.remove		= sun8i_tcon_top_remove,
> > +	.driver		= {
> > +		.name		= "sun8i-tcon-top",
> > +		.of_match_table	= sun8i_tcon_top_of_table,
> > +	},
> > +};
> > +module_platform_driver(sun8i_tcon_top_platform_driver);
> > +
> > +MODULE_AUTHOR("Jernej Skrabec <jernej.skrabec@siol.net>");
> > +MODULE_DESCRIPTION("Allwinner R40 TCON TOP driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_tcon_top.h
> > b/drivers/gpu/drm/sun4i/sun8i_tcon_top.h new file mode 100644
> > index 000000000000..19126e07d2a6
> > --- /dev/null
> > +++ b/drivers/gpu/drm/sun4i/sun8i_tcon_top.h
> > @@ -0,0 +1,20 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/* Copyright (c) 2018 Jernej Skrabec <jernej.skrabec@siol.net> */
> > +
> > +#ifndef _SUN8I_TCON_TOP_H_
> > +#define _SUN8I_TCON_TOP_H_
> > +
> > +#include <linux/device.h>
> > +
> > +struct sun8i_tcon_top;
> > +
> > +enum tcon_type {
> > +	tcon_type_lcd,
> > +	tcon_type_tv,
> 
> The usual practice is to have the enum values upper-case.

Ok.

Best regards,
Jernej
kernel test robot May 22, 2018, 2:25 a.m. UTC | #3 
Hi Jernej,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on v4.17-rc6 next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jernej-Skrabec/Add-support-for-R40-HDMI-pipeline/20180521-131839
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: arm-multi_v7_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/sun4i/sun8i_tcon_top.o: In function `init_module':
>> sun8i_tcon_top.c:(.init.text+0x0): multiple definition of `init_module'
   drivers/gpu/drm/sun4i/sun8i_mixer.o:sun8i_mixer.c:(.init.text+0x0): first defined here
   drivers/gpu/drm/sun4i/sun8i_tcon_top.o: In function `cleanup_module':
>> sun8i_tcon_top.c:(.exit.text+0x0): multiple definition of `cleanup_module'
   drivers/gpu/drm/sun4i/sun8i_mixer.o:sun8i_mixer.c:(.exit.text+0x0): first defined here

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Rob Herring (Arm) May 23, 2018, 6:23 p.m. UTC | #4 
On Sat, May 19, 2018 at 08:31:17PM +0200, Jernej Skrabec wrote:
> As already described in DT binding, TCON TOP is responsible for
> configuring display pipeline. In this initial driver focus is on HDMI
> pipeline, so TVE and LCD configuration is not implemented.
> 
> Implemented features:
> - HDMI source selection
> - clock driver (TCON and DSI gating)
> - connecting mixers and TCONS
> 
> Something similar also existed in previous SoCs, except that it was part
> of first TCON.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  drivers/gpu/drm/sun4i/Makefile             |   3 +-
>  drivers/gpu/drm/sun4i/sun8i_tcon_top.c     | 256 +++++++++++++++++++++
>  drivers/gpu/drm/sun4i/sun8i_tcon_top.h     |  20 ++
>  include/dt-bindings/clock/sun8i-tcon-top.h |  11 +

This belongs with the binding doc patch.

>  4 files changed, 289 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/sun4i/sun8i_tcon_top.c
>  create mode 100644 drivers/gpu/drm/sun4i/sun8i_tcon_top.h
>  create mode 100644 include/dt-bindings/clock/sun8i-tcon-top.h
Maxime Ripard May 24, 2018, 8:43 a.m. UTC | #5 
Hi,

On Mon, May 21, 2018 at 05:15:15PM +0200, Jernej Škrabec wrote:
> > > +	/*
> > > +	 * Default register values might have some reserved bits set, which
> > > +	 * prevents TCON TOP from working properly. Set them to 0 here.
> > > +	 */
> > > +	writel(0, tcon_top->regs + TCON_TOP_PORT_SEL_REG);
> > > +	writel(0, tcon_top->regs + TCON_TOP_GATE_SRC_REG);
> > > +
> > > +	for (i = 0; i < CLK_NUM; i++) {
> > > +		const char *parent_name = "bus-tcon-top";
> > 
> > I guess retrieving the parent's clock name at runtime would be more
> > flexible.
> 
> It is, but will it ever be anything else?

Probably not, but when the complexity is exactly the same (using
__clk_get_name), we'd better use the more appropriate solution. If we
ever need to change that clock name, or to use the driver with an SoC
that wouldn't have the same clock name for whatever reason, it will
just work.

> > > +		struct clk_init_data init;
> > > +		struct clk_gate *gate;
> > > +
> > > +		gate = devm_kzalloc(dev, sizeof(*gate), GFP_KERNEL);
> > > +		if (!gate) {
> > > +			ret = -ENOMEM;
> > > +			goto err_disable_clock;
> > > +		}
> > > +
> > > +		init.name = gates[i].name;
> > > +		init.ops = &clk_gate_ops;
> > > +		init.flags = CLK_IS_BASIC;
> > > +		init.parent_names = &parent_name;
> > > +		init.num_parents = 1;
> > > +
> > > +		gate->reg = tcon_top->regs + TCON_TOP_GATE_SRC_REG;
> > > +		gate->bit_idx = gates[i].bit;
> > > +		gate->lock = &tcon_top->reg_lock;
> > > +		gate->hw.init = &init;
> > > +
> > > +		ret = devm_clk_hw_register(dev, &gate->hw);
> > > +		if (ret)
> > > +			goto err_disable_clock;
> > 
> > Isn't it what clk_hw_register_gate is doing?
> 
> Almost, but not exactly. My goal was to use devm_* functions, so there is no 
> need to do any special cleanup.

Is it the only difference? If so, you can just create a
devm_clk_hw_register gate.

Maxime
Jernej Škrabec May 24, 2018, 8:33 p.m. UTC | #6 
Hi,

Dne četrtek, 24. maj 2018 ob 10:43:51 CEST je Maxime Ripard napisal(a):
> Hi,
> 
> On Mon, May 21, 2018 at 05:15:15PM +0200, Jernej Škrabec wrote:
> > > > +	/*
> > > > +	 * Default register values might have some reserved bits set, 
which
> > > > +	 * prevents TCON TOP from working properly. Set them to 0 here.
> > > > +	 */
> > > > +	writel(0, tcon_top->regs + TCON_TOP_PORT_SEL_REG);
> > > > +	writel(0, tcon_top->regs + TCON_TOP_GATE_SRC_REG);
> > > > +
> > > > +	for (i = 0; i < CLK_NUM; i++) {
> > > > +		const char *parent_name = "bus-tcon-top";
> > > 
> > > I guess retrieving the parent's clock name at runtime would be more
> > > flexible.
> > 
> > It is, but will it ever be anything else?
> 
> Probably not, but when the complexity is exactly the same (using
> __clk_get_name), we'd better use the more appropriate solution. If we
> ever need to change that clock name, or to use the driver with an SoC
> that wouldn't have the same clock name for whatever reason, it will
> just work.
> 
> > > > +		struct clk_init_data init;
> > > > +		struct clk_gate *gate;
> > > > +
> > > > +		gate = devm_kzalloc(dev, sizeof(*gate), GFP_KERNEL);
> > > > +		if (!gate) {
> > > > +			ret = -ENOMEM;
> > > > +			goto err_disable_clock;
> > > > +		}
> > > > +
> > > > +		init.name = gates[i].name;
> > > > +		init.ops = &clk_gate_ops;
> > > > +		init.flags = CLK_IS_BASIC;
> > > > +		init.parent_names = &parent_name;
> > > > +		init.num_parents = 1;
> > > > +
> > > > +		gate->reg = tcon_top->regs + TCON_TOP_GATE_SRC_REG;
> > > > +		gate->bit_idx = gates[i].bit;
> > > > +		gate->lock = &tcon_top->reg_lock;
> > > > +		gate->hw.init = &init;
> > > > +
> > > > +		ret = devm_clk_hw_register(dev, &gate->hw);
> > > > +		if (ret)
> > > > +			goto err_disable_clock;
> > > 
> > > Isn't it what clk_hw_register_gate is doing?
> > 
> > Almost, but not exactly. My goal was to use devm_* functions, so there is
> > no need to do any special cleanup.
> 
> Is it the only difference? If so, you can just create a
> devm_clk_hw_register gate.

I checked around and it seems that in clk core there are only non devm_* 
helpers like clk_hw_register_gate() for some reason. I guess I'll just use 
that and manually unregister all the clocks in cleanup function.

Best regards,
Jernej
diff mbox

Patch

diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile
index 2589f4acd5ae..09fbfd6304ba 100644
--- a/drivers/gpu/drm/sun4i/Makefile
+++ b/drivers/gpu/drm/sun4i/Makefile
@@ -16,7 +16,8 @@  sun8i-drm-hdmi-y		+= sun8i_hdmi_phy_clk.o
 
 sun8i-mixer-y			+= sun8i_mixer.o sun8i_ui_layer.o \
 				   sun8i_vi_layer.o sun8i_ui_scaler.o \
-				   sun8i_vi_scaler.o sun8i_csc.o
+				   sun8i_vi_scaler.o sun8i_csc.o \
+				   sun8i_tcon_top.o
 
 sun4i-tcon-y			+= sun4i_crtc.o
 sun4i-tcon-y			+= sun4i_dotclock.o
diff --git a/drivers/gpu/drm/sun4i/sun8i_tcon_top.c b/drivers/gpu/drm/sun4i/sun8i_tcon_top.c
new file mode 100644
index 000000000000..075a356a6dfa
--- /dev/null
+++ b/drivers/gpu/drm/sun4i/sun8i_tcon_top.c
@@ -0,0 +1,256 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright (c) 2018 Jernej Skrabec <jernej.skrabec@siol.net> */
+
+#include <drm/drmP.h>
+
+#include <dt-bindings/clock/sun8i-tcon-top.h>
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/spinlock.h>
+
+#include "sun8i_tcon_top.h"
+
+#define TCON_TOP_PORT_SEL_REG		0x1C
+#define TCON_TOP_PORT_DE0_MSK			GENMASK(1, 0)
+#define TCON_TOP_PORT_DE1_MSK			GENMASK(5, 4)
+#define TCON_TOP_PORT_TCON_LCD0			0
+#define TCON_TOP_PORT_TCON_LCD1			1
+#define TCON_TOP_PORT_TCON_TV0			2
+#define TCON_TOP_PORT_TCON_TV1			3
+
+#define TCON_TOP_GATE_SRC_REG		0x20
+#define TCON_TOP_HDMI_SRC_MSK			GENMASK(29, 28)
+#define TCON_TOP_HDMI_SRC_NONE			0
+#define TCON_TOP_HDMI_SRC_TCON_TV0		1
+#define TCON_TOP_HDMI_SRC_TCON_TV1		2
+#define TCON_TOP_TCON_TV1_GATE			24
+#define TCON_TOP_TCON_TV0_GATE			20
+#define TCON_TOP_TCON_DSI_GATE			16
+
+#define CLK_NUM					3
+
+struct sun8i_tcon_top {
+	struct clk		*bus;
+	void __iomem		*regs;
+	struct reset_control	*rst;
+
+	/*
+	 * spinlock is used for locking access to registers from different
+	 * places - tcon driver and clk subsystem.
+	 */
+	spinlock_t		reg_lock;
+};
+
+struct sun8i_tcon_top_gate {
+	const char	*name;
+	u8		bit;
+	int		index;
+};
+
+static const struct sun8i_tcon_top_gate gates[] = {
+	{"bus-tcon-top-dsi", TCON_TOP_TCON_DSI_GATE, CLK_BUS_TCON_TOP_DSI},
+	{"bus-tcon-top-tv0", TCON_TOP_TCON_TV0_GATE, CLK_BUS_TCON_TOP_TV0},
+	{"bus-tcon-top-tv1", TCON_TOP_TCON_TV1_GATE, CLK_BUS_TCON_TOP_TV1},
+};
+
+void sun8i_tcon_top_set_hdmi_src(struct sun8i_tcon_top *tcon_top, int tcon)
+{
+	unsigned long flags;
+	u32 val;
+
+	if (tcon > 1) {
+		DRM_ERROR("TCON index is too high!\n");
+		return;
+	}
+
+	spin_lock_irqsave(&tcon_top->reg_lock, flags);
+
+	val = readl(tcon_top->regs + TCON_TOP_GATE_SRC_REG);
+	val &= ~TCON_TOP_HDMI_SRC_MSK;
+	val |= FIELD_PREP(TCON_TOP_HDMI_SRC_MSK,
+			  TCON_TOP_HDMI_SRC_TCON_TV0 + tcon);
+	writel(val, tcon_top->regs + TCON_TOP_GATE_SRC_REG);
+
+	spin_unlock_irqrestore(&tcon_top->reg_lock, flags);
+}
+
+void sun8i_tcon_top_de_config(struct sun8i_tcon_top *tcon_top,
+			      int mixer, enum tcon_type tcon_type, int tcon)
+{
+	unsigned long flags;
+	u32 val, reg;
+
+	if (mixer > 1) {
+		DRM_ERROR("Mixer index is too high!\n");
+		return;
+	}
+
+	if (tcon > 1) {
+		DRM_ERROR("TCON index is too high!\n");
+		return;
+	}
+
+	switch (tcon_type) {
+	case tcon_type_lcd:
+		val = TCON_TOP_PORT_TCON_LCD0 + tcon;
+		break;
+	case tcon_type_tv:
+		val = TCON_TOP_PORT_TCON_TV0 + tcon;
+		break;
+	default:
+		DRM_ERROR("Invalid TCON type!\n");
+		return;
+	}
+
+	spin_lock_irqsave(&tcon_top->reg_lock, flags);
+
+	reg = readl(tcon_top->regs + TCON_TOP_PORT_SEL_REG);
+	if (mixer == 0) {
+		reg &= ~TCON_TOP_PORT_DE0_MSK;
+		reg |= FIELD_PREP(TCON_TOP_PORT_DE0_MSK, val);
+	} else {
+		reg &= ~TCON_TOP_PORT_DE1_MSK;
+		reg |= FIELD_PREP(TCON_TOP_PORT_DE1_MSK, val);
+	}
+	writel(reg, tcon_top->regs + TCON_TOP_PORT_SEL_REG);
+
+	spin_unlock_irqrestore(&tcon_top->reg_lock, flags);
+}
+
+static int sun8i_tcon_top_probe(struct platform_device *pdev)
+{
+	struct clk_hw_onecell_data *clk_data;
+	struct sun8i_tcon_top *tcon_top;
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	int ret, i;
+
+	tcon_top = devm_kzalloc(dev, sizeof(*tcon_top), GFP_KERNEL);
+	if (!tcon_top)
+		return -ENOMEM;
+
+	clk_data = devm_kzalloc(&pdev->dev, sizeof(*clk_data) +
+				sizeof(*clk_data->hws) * CLK_NUM,
+				GFP_KERNEL);
+	if (!clk_data)
+		return -ENOMEM;
+
+	spin_lock_init(&tcon_top->reg_lock);
+
+	tcon_top->rst = devm_reset_control_get(dev, "rst");
+	if (IS_ERR(tcon_top->rst)) {
+		dev_err(dev, "Couldn't get our reset line\n");
+		return PTR_ERR(tcon_top->rst);
+	}
+
+	tcon_top->bus = devm_clk_get(dev, "bus");
+	if (IS_ERR(tcon_top->bus)) {
+		dev_err(dev, "Couldn't get the bus clock\n");
+		return PTR_ERR(tcon_top->bus);
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	tcon_top->regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(tcon_top->regs))
+		return PTR_ERR(tcon_top->regs);
+
+	ret = reset_control_deassert(tcon_top->rst);
+	if (ret) {
+		dev_err(dev, "Could not deassert ctrl reset control\n");
+		return ret;
+	}
+
+	ret = clk_prepare_enable(tcon_top->bus);
+	if (ret) {
+		dev_err(dev, "Could not enable bus clock\n");
+		goto err_assert_reset;
+	}
+
+	/*
+	 * Default register values might have some reserved bits set, which
+	 * prevents TCON TOP from working properly. Set them to 0 here.
+	 */
+	writel(0, tcon_top->regs + TCON_TOP_PORT_SEL_REG);
+	writel(0, tcon_top->regs + TCON_TOP_GATE_SRC_REG);
+
+	for (i = 0; i < CLK_NUM; i++) {
+		const char *parent_name = "bus-tcon-top";
+		struct clk_init_data init;
+		struct clk_gate *gate;
+
+		gate = devm_kzalloc(dev, sizeof(*gate), GFP_KERNEL);
+		if (!gate) {
+			ret = -ENOMEM;
+			goto err_disable_clock;
+		}
+
+		init.name = gates[i].name;
+		init.ops = &clk_gate_ops;
+		init.flags = CLK_IS_BASIC;
+		init.parent_names = &parent_name;
+		init.num_parents = 1;
+
+		gate->reg = tcon_top->regs + TCON_TOP_GATE_SRC_REG;
+		gate->bit_idx = gates[i].bit;
+		gate->lock = &tcon_top->reg_lock;
+		gate->hw.init = &init;
+
+		ret = devm_clk_hw_register(dev, &gate->hw);
+		if (ret)
+			goto err_disable_clock;
+
+		clk_data->hws[gates[i].index] = &gate->hw;
+	}
+
+	clk_data->num = CLK_NUM;
+
+	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
+	if (ret)
+		goto err_disable_clock;
+
+	platform_set_drvdata(pdev, tcon_top);
+
+	return 0;
+
+err_disable_clock:
+	clk_disable_unprepare(tcon_top->bus);
+err_assert_reset:
+	reset_control_assert(tcon_top->rst);
+
+	return ret;
+}
+
+static int sun8i_tcon_top_remove(struct platform_device *pdev)
+{
+	struct sun8i_tcon_top *tcon_top = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(tcon_top->bus);
+	reset_control_assert(tcon_top->rst);
+
+	return 0;
+}
+
+const struct of_device_id sun8i_tcon_top_of_table[] = {
+	{ .compatible = "allwinner,sun8i-r40-tcon-top" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, sun8i_tcon_top_of_table);
+
+static struct platform_driver sun8i_tcon_top_platform_driver = {
+	.probe		= sun8i_tcon_top_probe,
+	.remove		= sun8i_tcon_top_remove,
+	.driver		= {
+		.name		= "sun8i-tcon-top",
+		.of_match_table	= sun8i_tcon_top_of_table,
+	},
+};
+module_platform_driver(sun8i_tcon_top_platform_driver);
+
+MODULE_AUTHOR("Jernej Skrabec <jernej.skrabec@siol.net>");
+MODULE_DESCRIPTION("Allwinner R40 TCON TOP driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/sun4i/sun8i_tcon_top.h b/drivers/gpu/drm/sun4i/sun8i_tcon_top.h
new file mode 100644
index 000000000000..19126e07d2a6
--- /dev/null
+++ b/drivers/gpu/drm/sun4i/sun8i_tcon_top.h
@@ -0,0 +1,20 @@ 
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (c) 2018 Jernej Skrabec <jernej.skrabec@siol.net> */
+
+#ifndef _SUN8I_TCON_TOP_H_
+#define _SUN8I_TCON_TOP_H_
+
+#include <linux/device.h>
+
+struct sun8i_tcon_top;
+
+enum tcon_type {
+	tcon_type_lcd,
+	tcon_type_tv,
+};
+
+void sun8i_tcon_top_set_hdmi_src(struct sun8i_tcon_top *tcon_top, int tcon);
+void sun8i_tcon_top_de_config(struct sun8i_tcon_top *tcon_top,
+			      int mixer, enum tcon_type tcon_type, int tcon);
+
+#endif /* _SUN8I_TCON_TOP_H_ */
diff --git a/include/dt-bindings/clock/sun8i-tcon-top.h b/include/dt-bindings/clock/sun8i-tcon-top.h
new file mode 100644
index 000000000000..c05e92770402
--- /dev/null
+++ b/include/dt-bindings/clock/sun8i-tcon-top.h
@@ -0,0 +1,11 @@ 
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/* Copyright (C) 2018 Jernej Skrabec <jernej.skrabec@siol.net> */
+
+#ifndef _DT_BINDINGS_CLOCK_SUN8I_TCON_TOP_H_
+#define _DT_BINDINGS_CLOCK_SUN8I_TCON_TOP_H_
+
+#define CLK_BUS_TCON_TOP_DSI	0
+#define CLK_BUS_TCON_TOP_TV0	1
+#define CLK_BUS_TCON_TOP_TV1	2
+
+#endif /* _DT_BINDINGS_CLOCK_SUN8I_TCON_TOP_H_ */