[U-Boot] [PATCH 0/5] rockchip: video: rk3399: enable HDMI output

[U-Boot] [PATCH 0/5] rockchip: video: rk3399: enable HDMI output

[Philipp Tomsich]

This series provides HDMI enablement for the RK3399 and will support
video console on the RK3399 either with VOP-lit or VOP-big:
 - pinctrl and clk support for the hdmi node
 - a refactoring of rk_vop.c and rk_hdmi.c to allow for the minor
   differences between the RK3288 and RK3399 VOP and HDMI blocks

To see the full series in the context of a working configuration (i.e.
including DTS, defconfig, etc. and on top of the MIPI enablement
change series from Rockchip), refer to
       https://git.theobroma-systems.com/puma-u-boot.git/log/?h=puma-hdmi-rfc

This has been tested (on top of Eric Gao's change series for MIPI-DSI)
using 'bmp display' and 'setenv stdout vidconsole' on the RK3399-Q7
with various HDMI monitors, both for VOP-lit and VOP-big (setting the
other one to 'disabled').


Philipp Tomsich (5):
  rockchip: clk: rk3399: allow requests for HDMI clocks
  rockchip: pinctrl: rk3399: add support for the HDMI I2C pins
  rockchip: video: rk3399: enable HDMI output (from the rk_vop) for the
    RK3399
  rockchip: video: rk3399: add HDMI TX support on the RK3399
  rockchip: dts: rk3399: enable HDMI output in the DTS

 arch/arm/dts/rk3399.dtsi                        | 110 +++++++++++++++
 arch/arm/include/asm/arch-rockchip/grf_rk3399.h |   7 +
 arch/arm/include/asm/arch-rockchip/vop_rk3288.h |  11 ++
 drivers/clk/rockchip/clk_rk3399.c               |   7 +
 drivers/pinctrl/rockchip/pinctrl_rk3399.c       |  26 ++++
 drivers/video/rockchip/rk_hdmi.c                | 165 +++++++++++++++++-----
 drivers/video/rockchip/rk_vop.c                 | 180 ++++++++++++++++++++----
 7 files changed, 441 insertions(+), 65 deletions(-)

-- 
1.9.1

[U-Boot] [PATCH 1/5] rockchip: clk: rk3399: allow requests for HDMI clocks

[Philipp Tomsich]

This allows requests (via the DTS) for PCLK_HDMI_CTRL/PCLK_VIO_GRF,
which are clock gates in the HDMI output path for the RK3399.

As these are enabled by default (i.e. after reset), we don't implement
any logic to actively open/close these clock gates and simply assume
that their reset-default has not been changed.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 drivers/clk/rockchip/clk_rk3399.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/clk/rockchip/clk_rk3399.c b/drivers/clk/rockchip/clk_rk3399.c
index 72395e2..d820ea6 100644
--- a/drivers/clk/rockchip/clk_rk3399.c
+++ b/drivers/clk/rockchip/clk_rk3399.c
@@ -879,6 +879,9 @@ static ulong rk3399_clk_get_rate(struct clk *clk)
 	case SCLK_UART0:
 	case SCLK_UART2:
 		return 24000000;
+		break;
+	case PCLK_HDMI_CTRL:
+		break;
 	case DCLK_VOP0:
 	case DCLK_VOP1:
 		break;
@@ -916,6 +919,10 @@ static ulong rk3399_clk_set_rate(struct clk *clk, ulong rate)
 	case SCLK_SPI0...SCLK_SPI5:
 		ret = rk3399_spi_set_clk(priv->cru, clk->id, rate);
 		break;
+	case PCLK_HDMI_CTRL:
+	case PCLK_VIO_GRF:
+		/* the PCLK gates for video are enabled by default */
+		break;
 	case DCLK_VOP0:
 	case DCLK_VOP1:
 		ret = rk3399_vop_set_clk(priv->cru, clk->id, rate);
-- 
1.9.1

[U-Boot] [PATCH 2/5] rockchip: pinctrl: rk3399: add support for the HDMI I2C pins

[Philipp Tomsich]

To add HDMI support for the RK3399, this commit provides the needed
pinctrl functionality to configure the HDMI I2C pins (used for reading
the screen's EDID).

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

---

 arch/arm/include/asm/arch-rockchip/grf_rk3399.h |  2 ++
 drivers/pinctrl/rockchip/pinctrl_rk3399.c       | 26 +++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3399.h b/arch/arm/include/asm/arch-rockchip/grf_rk3399.h
index cbcff2e..22d8d97 100644
--- a/arch/arm/include/asm/arch-rockchip/grf_rk3399.h
+++ b/arch/arm/include/asm/arch-rockchip/grf_rk3399.h
@@ -439,9 +439,11 @@ enum {
 	GRF_GPIO4C0_SEL_SHIFT   = 0,
 	GRF_GPIO4C0_SEL_MASK    = 3 << GRF_GPIO4C0_SEL_SHIFT,
 	GRF_UART2DGBB_SIN       = 2,
+	GRF_HDMII2C_SCL         = 3,
 	GRF_GPIO4C1_SEL_SHIFT   = 2,
 	GRF_GPIO4C1_SEL_MASK    = 3 << GRF_GPIO4C1_SEL_SHIFT,
 	GRF_UART2DGBB_SOUT      = 2,
+	GRF_HDMII2C_SDA         = 3,
 	GRF_GPIO4C2_SEL_SHIFT   = 4,
 	GRF_GPIO4C2_SEL_MASK    = 3 << GRF_GPIO4C2_SEL_SHIFT,
 	GRF_PWM_0               = 1,
diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3399.c b/drivers/pinctrl/rockchip/pinctrl_rk3399.c
index 6eb657f..0226731 100644
--- a/drivers/pinctrl/rockchip/pinctrl_rk3399.c
+++ b/drivers/pinctrl/rockchip/pinctrl_rk3399.c
@@ -247,6 +247,23 @@ static void pinctrl_rk3399_gmac_config(struct rk3399_grf_regs *grf, int mmc_id)
 }
 #endif
 
+#if !defined(CONFIG_SPL_BUILD)
+static void pinctrl_rk3399_hdmi_config(struct rk3399_grf_regs *grf, int hdmi_id)
+{
+	switch (hdmi_id) {
+	case PERIPH_ID_HDMI:
+		rk_clrsetreg(&grf->gpio4c_iomux,
+			     GRF_GPIO4C0_SEL_MASK | GRF_GPIO4C1_SEL_MASK,
+			     (GRF_HDMII2C_SCL << GRF_GPIO4C0_SEL_SHIFT) |
+			     (GRF_HDMII2C_SDA << GRF_GPIO4C1_SEL_SHIFT));
+		break;
+	default:
+		debug("%s: hdmi_id = %d unsupported\n", __func__, hdmi_id);
+		break;
+	}
+}
+#endif
+
 static int rk3399_pinctrl_request(struct udevice *dev, int func, int flags)
 {
 	struct rk3399_pinctrl_priv *priv = dev_get_priv(dev);
@@ -296,6 +313,11 @@ static int rk3399_pinctrl_request(struct udevice *dev, int func, int flags)
 		pinctrl_rk3399_gmac_config(priv->grf, func);
 		break;
 #endif
+#if !defined(CONFIG_SPL_BUILD)
+	case PERIPH_ID_HDMI:
+		pinctrl_rk3399_hdmi_config(priv->grf, func);
+		break;
+#endif
 	default:
 		return -EINVAL;
 	}
@@ -342,6 +364,10 @@ static int rk3399_pinctrl_get_periph_id(struct udevice *dev,
 	case 12:
 		return PERIPH_ID_GMAC;
 #endif
+#if !defined(CONFIG_SPL_BUILD)
+	case 23:
+		return PERIPH_ID_HDMI;
+#endif
 	}
 #endif
 	return -ENOENT;
-- 
1.9.1

[U-Boot] [PATCH 3/5] rockchip: video: rk3399: enable HDMI output (from the rk_vop) for the RK3399

[Philipp Tomsich]

This commit enables RK3399 support for HDMI through the following
changes:
- adds a driverdata structure to mirror some subtle version
  differences between the RK3399 VOPs and those in the RK3288
  (e.g. the pin-polarity configuration)
- configures the VOP to output 32bpp for HDMI
- handles whether a VOP can output 10BIT data or not (i.e. RGBaaa vs. RGB888)
  using the driverdata structure

And as we touch this file anyway, we also increase the size of the
framebuffer to a slightly overzealous 4K2K at 32bpp.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 arch/arm/include/asm/arch-rockchip/vop_rk3288.h |  11 ++
 drivers/video/rockchip/rk_vop.c                 | 180 ++++++++++++++++++++----
 2 files changed, 161 insertions(+), 30 deletions(-)

diff --git a/arch/arm/include/asm/arch-rockchip/vop_rk3288.h b/arch/arm/include/asm/arch-rockchip/vop_rk3288.h
index 0ce3d67..bca6860 100644
--- a/arch/arm/include/asm/arch-rockchip/vop_rk3288.h
+++ b/arch/arm/include/asm/arch-rockchip/vop_rk3288.h
@@ -196,9 +196,20 @@ enum vop_modes {
 #define V_DSP_DEN_POL(x)               (((x) & 1) << 6)
 #define V_DSP_VSYNC_POL(x)             (((x) & 1) << 5)
 #define V_DSP_HSYNC_POL(x)             (((x) & 1) << 4)
+#define V_DSP_PIN_POL(x)               (((x) & 0xf) << 4)
 #define V_DSP_OUT_MODE(x)              ((x) & 0xf)
 
 /* VOP_DSP_CTRL1 */
+#define V_RK3399_DSP_MIPI_POL(x)       ((x) << 28)
+#define V_RK3399_DSP_EDP_POL(x)        ((x) << 24)
+#define V_RK3399_DSP_HDMI_POL(x)       ((x) << 20)
+#define V_RK3399_DSP_LVDS_POL(x)       ((x) << 16)
+
+#define M_RK3399_DSP_MIPI_POL          (V_RK3399_DSP_MIPI_POL(0xf))
+#define M_RK3399_DSP_EDP_POL           (V_RK3399_DSP_EDP_POL(0xf))
+#define M_RK3399_DSP_HDMI_POL          (V_RK3399_DSP_HDMI_POL(0xf))
+#define M_RK3399_DSP_LVDS_POL          (V_RK3399_DSP_LVDS_POL(0xf))
+
 #define M_DSP_LAYER3_SEL               (3 << 14)
 #define M_DSP_LAYER2_SEL               (3 << 12)
 #define M_DSP_LAYER1_SEL               (3 << 10)
diff --git a/drivers/video/rockchip/rk_vop.c b/drivers/video/rockchip/rk_vop.c
index bc02f80..01b4b6a 100644
--- a/drivers/video/rockchip/rk_vop.c
+++ b/drivers/video/rockchip/rk_vop.c
@@ -5,6 +5,8 @@
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
+#define DEBUG
+
 #include <common.h>
 #include <clk.h>
 #include <display.h>
@@ -28,11 +30,78 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+enum vop_pol {
+	HSYNC_POSITIVE = 0,
+	VSYNC_POSITIVE = 1,
+	DEN_NEGATIVE   = 2,
+	DCLK_INVERT    = 3
+};
+
 struct rk_vop_priv {
 	struct rk3288_vop *regs;
 	struct rk3288_grf *grf;
 };
 
+enum vop_features {
+	VOP_FEATURE_OUTPUT_10BIT = (1 << 0),
+};
+
+struct rkvop_driverdata {
+	/* configuration */
+	u32 features;
+	/* block-specific setters/getters */
+	void (*set_pin_polarity)(struct udevice *, enum vop_modes, u32);
+};
+
+static void rk3288_set_pin_polarity(struct udevice *dev,
+				    enum vop_modes mode, u32 polarity)
+{
+	struct rk_vop_priv *priv = dev_get_priv(dev);
+	struct rk3288_vop *regs = priv->regs;
+
+	/* The RK3328 VOP (v3.1) has its polarity configuration in ctrl0 */
+	clrsetbits_le32(&regs->dsp_ctrl0,
+			M_DSP_DCLK_POL | M_DSP_DEN_POL |
+			M_DSP_VSYNC_POL | M_DSP_HSYNC_POL,
+			V_DSP_PIN_POL(polarity));
+}
+
+static void rk3399_set_pin_polarity(struct udevice *dev,
+				    enum vop_modes mode, u32 polarity)
+{
+	struct rk_vop_priv *priv = dev_get_priv(dev);
+	struct rk3288_vop *regs = priv->regs;
+
+	/*
+	 * The RK3399 VOPs (v3.5 and v3.6) require a per-mode setting of
+	 * the polarity configuration (in ctrl1).
+	 */
+	switch (mode) {
+	case VOP_MODE_HDMI:
+		clrsetbits_le32(&regs->dsp_ctrl1,
+				M_RK3399_DSP_HDMI_POL,
+				V_RK3399_DSP_HDMI_POL(polarity));
+		break;
+
+	case VOP_MODE_EDP:
+		clrsetbits_le32(&regs->dsp_ctrl1,
+				M_RK3399_DSP_EDP_POL,
+				V_RK3399_DSP_EDP_POL(polarity));
+		break;
+
+	case VOP_MODE_MIPI:
+		clrsetbits_le32(&regs->dsp_ctrl1,
+				M_RK3399_DSP_MIPI_POL,
+				V_RK3399_DSP_MIPI_POL(polarity));
+		break;
+
+	case VOP_MODE_LVDS:
+		/* The RK3399 has neither parallel RGB nor LVDS output. */
+	default:
+		debug("%s: unsupported output mode %x\n", __func__, mode);
+	}
+}
+
 void rkvop_enable(struct rk3288_vop *regs, ulong fbbase,
 		  int fb_bits_per_pixel, const struct display_timing *edid)
 {
@@ -89,50 +158,77 @@ void rkvop_enable(struct rk3288_vop *regs, ulong fbbase,
 	writel(0x01, &regs->reg_cfg_done); /* enable reg config */
 }
 
-void rkvop_mode_set(struct rk3288_vop *regs,
-		    const struct display_timing *edid, enum vop_modes mode)
+static void rkvop_set_pin_polarity(struct udevice *dev,
+				   enum vop_modes mode, u32 polarity)
 {
-	u32 hactive = edid->hactive.typ;
-	u32 vactive = edid->vactive.typ;
-	u32 hsync_len = edid->hsync_len.typ;
-	u32 hback_porch = edid->hback_porch.typ;
-	u32 vsync_len = edid->vsync_len.typ;
-	u32 vback_porch = edid->vback_porch.typ;
-	u32 hfront_porch = edid->hfront_porch.typ;
-	u32 vfront_porch = edid->vfront_porch.typ;
-	uint flags;
-	int mode_flags;
+	struct rkvop_driverdata *ops =
+		(struct rkvop_driverdata *)dev_get_driver_data(dev);
+
+	if (ops->set_pin_polarity)
+		ops->set_pin_polarity(dev, mode, polarity);
+}
+
+static void rkvop_enable_output(struct udevice *dev, enum vop_modes mode)
+{
+	struct rk_vop_priv *priv = dev_get_priv(dev);
+	struct rk3288_vop *regs = priv->regs;
 
 	switch (mode) {
 	case VOP_MODE_HDMI:
 		clrsetbits_le32(&regs->sys_ctrl, M_ALL_OUT_EN,
 				V_HDMI_OUT_EN(1));
 		break;
+
 	case VOP_MODE_EDP:
-	default:
 		clrsetbits_le32(&regs->sys_ctrl, M_ALL_OUT_EN,
 				V_EDP_OUT_EN(1));
 		break;
+
 	case VOP_MODE_LVDS:
 		clrsetbits_le32(&regs->sys_ctrl, M_ALL_OUT_EN,
 				V_RGB_OUT_EN(1));
 		break;
+
+	default:
+		debug("%s: unsupported output mode %x\n", __func__, mode);
 	}
+}
 
-	if (mode == VOP_MODE_HDMI || mode == VOP_MODE_EDP)
-		/* RGBaaa */
-		mode_flags = 15;
-	else
-		/* RGB888 */
-		mode_flags = 0;
+void rkvop_mode_set(struct udevice *dev,
+		    const struct display_timing *edid, enum vop_modes mode)
+{
+	struct rk_vop_priv *priv = dev_get_priv(dev);
+	struct rk3288_vop *regs = priv->regs;
+	struct rkvop_driverdata *data =
+		(struct rkvop_driverdata *)dev_get_driver_data(dev);
+
+	u32 hactive = edid->hactive.typ;
+	u32 vactive = edid->vactive.typ;
+	u32 hsync_len = edid->hsync_len.typ;
+	u32 hback_porch = edid->hback_porch.typ;
+	u32 vsync_len = edid->vsync_len.typ;
+	u32 vback_porch = edid->vback_porch.typ;
+	u32 hfront_porch = edid->hfront_porch.typ;
+	u32 vfront_porch = edid->vfront_porch.typ;
+	int mode_flags;
+	u32 pin_polarity;
 
-	flags = V_DSP_OUT_MODE(mode_flags) |
-		V_DSP_HSYNC_POL(!!(edid->flags & DISPLAY_FLAGS_HSYNC_HIGH)) |
-		V_DSP_VSYNC_POL(!!(edid->flags & DISPLAY_FLAGS_VSYNC_HIGH));
+	pin_polarity = BIT(DCLK_INVERT);
+	if (edid->flags & DISPLAY_FLAGS_HSYNC_HIGH)
+		pin_polarity |= BIT(HSYNC_POSITIVE);
+	if (edid->flags & DISPLAY_FLAGS_VSYNC_HIGH)
+		pin_polarity |= BIT(VSYNC_POSITIVE);
 
-	clrsetbits_le32(&regs->dsp_ctrl0,
-			M_DSP_OUT_MODE | M_DSP_VSYNC_POL | M_DSP_HSYNC_POL,
-			flags);
+	rkvop_set_pin_polarity(dev, mode, pin_polarity);
+	rkvop_enable_output(dev, mode);
+
+	mode_flags = 0;  /* RGB888 */
+	if ((data->features & VOP_FEATURE_OUTPUT_10BIT) &&
+	    (mode == VOP_MODE_HDMI || mode == VOP_MODE_EDP))
+		mode_flags = 15;  /* RGBaaa */
+
+	clrsetbits_le32(&regs->dsp_ctrl0, M_DSP_OUT_MODE,
+			V_DSP_OUT_MODE(mode_flags));
 
 	writel(V_HSYNC(hsync_len) |
 	       V_HORPRD(hsync_len + hback_porch + hactive + hfront_porch),
@@ -249,8 +345,7 @@ int rk_display_init(struct udevice *dev, ulong fbbase,
 		return ret;
 	}
 
-	rkvop_mode_set(regs, &timing, vop_id);
-
+	rkvop_mode_set(dev, &timing, vop_id);
 	rkvop_enable(regs, fbbase, 1 << l2bpp, &timing);
 
 	ret = display_enable(disp, 1 << l2bpp, &timing);
@@ -341,19 +436,44 @@ static int rk_vop_bind(struct udevice *dev)
 {
 	struct video_uc_platdata *plat = dev_get_uclass_platdata(dev);
 
-	plat->size = 1920 * 1080 * 2;
+	/*
+	 * Let's allocate an excessively large framebuffer (i.e. the
+	 * maximum any of the supported VOP variants can output), so
+	 * we are prepared for the day when someone first tries to
+	 * drive a 4K2K HDMI signal from the bootloader.
+	 */
+	plat->size = 4096 * 2160 * 4;
 
 	return 0;
 }
 
-static const struct video_ops rk_vop_ops = {
+struct rkvop_driverdata rk3288_driverdata = {
+	.features = VOP_FEATURE_OUTPUT_10BIT,
+	.set_pin_polarity = rk3288_set_pin_polarity,
+};
+
+struct rkvop_driverdata rk3399_lit_driverdata = {
+	.set_pin_polarity = rk3399_set_pin_polarity,
+};
+
+struct rkvop_driverdata rk3399_big_driverdata = {
+	.features = VOP_FEATURE_OUTPUT_10BIT,
+	.set_pin_polarity = rk3399_set_pin_polarity,
 };
 
 static const struct udevice_id rk_vop_ids[] = {
-	{ .compatible = "rockchip,rk3288-vop" },
+	{ .compatible = "rockchip,rk3288-vop",
+	  .data = (ulong)&rk3288_driverdata },
+	{ .compatible = "rockchip,rk3399-vop-big",
+	  .data = (ulong)&rk3399_big_driverdata },
+	{ .compatible = "rockchip,rk3399-vop-lit",
+	  .data = (ulong)&rk3399_lit_driverdata },
 	{ }
 };
 
+static const struct video_ops rk_vop_ops = {
+};
+
 U_BOOT_DRIVER(rk_vop) = {
 	.name	= "rk_vop",
 	.id	= UCLASS_VIDEO,
-- 
1.9.1

[U-Boot] [PATCH 3/5] rockchip: video: rk3399: enable HDMI output (from the rk_vop) for the RK3399

[Jernej Škrabec]

Hi Philipp,

Dne petek, 28. april 2017 ob 17:53:10 CEST je Philipp Tomsich napisal(a):
> This commit enables RK3399 support for HDMI through the following
> changes:
> - adds a driverdata structure to mirror some subtle version
>   differences between the RK3399 VOPs and those in the RK3288
>   (e.g. the pin-polarity configuration)
> - configures the VOP to output 32bpp for HDMI
> - handles whether a VOP can output 10BIT data or not (i.e. RGBaaa vs.
> RGB888) using the driverdata structure
> 
> And as we touch this file anyway, we also increase the size of the
> framebuffer to a slightly overzealous 4K2K at 32bpp.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
> 
>  arch/arm/include/asm/arch-rockchip/vop_rk3288.h |  11 ++
>  drivers/video/rockchip/rk_vop.c                 | 180
> ++++++++++++++++++++---- 2 files changed, 161 insertions(+), 30
> deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-rockchip/vop_rk3288.h
> b/arch/arm/include/asm/arch-rockchip/vop_rk3288.h index 0ce3d67..bca6860
> 100644
> --- a/arch/arm/include/asm/arch-rockchip/vop_rk3288.h
> +++ b/arch/arm/include/asm/arch-rockchip/vop_rk3288.h
> @@ -196,9 +196,20 @@ enum vop_modes {
>  #define V_DSP_DEN_POL(x)               (((x) & 1) << 6)
>  #define V_DSP_VSYNC_POL(x)             (((x) & 1) << 5)
>  #define V_DSP_HSYNC_POL(x)             (((x) & 1) << 4)
> +#define V_DSP_PIN_POL(x)               (((x) & 0xf) << 4)
>  #define V_DSP_OUT_MODE(x)              ((x) & 0xf)
> 
>  /* VOP_DSP_CTRL1 */
> +#define V_RK3399_DSP_MIPI_POL(x)       ((x) << 28)
> +#define V_RK3399_DSP_EDP_POL(x)        ((x) << 24)
> +#define V_RK3399_DSP_HDMI_POL(x)       ((x) << 20)
> +#define V_RK3399_DSP_LVDS_POL(x)       ((x) << 16)
> +
> +#define M_RK3399_DSP_MIPI_POL          (V_RK3399_DSP_MIPI_POL(0xf))
> +#define M_RK3399_DSP_EDP_POL           (V_RK3399_DSP_EDP_POL(0xf))
> +#define M_RK3399_DSP_HDMI_POL          (V_RK3399_DSP_HDMI_POL(0xf))
> +#define M_RK3399_DSP_LVDS_POL          (V_RK3399_DSP_LVDS_POL(0xf))
> +
>  #define M_DSP_LAYER3_SEL               (3 << 14)
>  #define M_DSP_LAYER2_SEL               (3 << 12)
>  #define M_DSP_LAYER1_SEL               (3 << 10)
> diff --git a/drivers/video/rockchip/rk_vop.c
> b/drivers/video/rockchip/rk_vop.c index bc02f80..01b4b6a 100644
> --- a/drivers/video/rockchip/rk_vop.c
> +++ b/drivers/video/rockchip/rk_vop.c
> @@ -5,6 +5,8 @@
>   * SPDX-License-Identifier:	GPL-2.0+
>   */
> 
> +#define DEBUG
> +

You probably forgot to remove that?

Regards,
Jernej

>  #include <common.h>
>  #include <clk.h>
>  #include <display.h>
> @@ -28,11 +30,78 @@
> 
>  DECLARE_GLOBAL_DATA_PTR;
> 
> +enum vop_pol {
> +	HSYNC_POSITIVE = 0,
> +	VSYNC_POSITIVE = 1,
> +	DEN_NEGATIVE   = 2,
> +	DCLK_INVERT    = 3
> +};
> +
>  struct rk_vop_priv {
>  	struct rk3288_vop *regs;
>  	struct rk3288_grf *grf;
>  };
> 
> +enum vop_features {
> +	VOP_FEATURE_OUTPUT_10BIT = (1 << 0),
> +};
> +
> +struct rkvop_driverdata {
> +	/* configuration */
> +	u32 features;
> +	/* block-specific setters/getters */
> +	void (*set_pin_polarity)(struct udevice *, enum vop_modes, u32);
> +};
> +
> +static void rk3288_set_pin_polarity(struct udevice *dev,
> +				    enum vop_modes mode, u32 polarity)
> +{
> +	struct rk_vop_priv *priv = dev_get_priv(dev);
> +	struct rk3288_vop *regs = priv->regs;
> +
> +	/* The RK3328 VOP (v3.1) has its polarity configuration in ctrl0 */
> +	clrsetbits_le32(&regs->dsp_ctrl0,
> +			M_DSP_DCLK_POL | M_DSP_DEN_POL |
> +			M_DSP_VSYNC_POL | M_DSP_HSYNC_POL,
> +			V_DSP_PIN_POL(polarity));
> +}
> +
> +static void rk3399_set_pin_polarity(struct udevice *dev,
> +				    enum vop_modes mode, u32 polarity)
> +{
> +	struct rk_vop_priv *priv = dev_get_priv(dev);
> +	struct rk3288_vop *regs = priv->regs;
> +
> +	/*
> +	 * The RK3399 VOPs (v3.5 and v3.6) require a per-mode setting of
> +	 * the polarity configuration (in ctrl1).
> +	 */
> +	switch (mode) {
> +	case VOP_MODE_HDMI:
> +		clrsetbits_le32(&regs->dsp_ctrl1,
> +				M_RK3399_DSP_HDMI_POL,
> +				V_RK3399_DSP_HDMI_POL(polarity));
> +		break;
> +
> +	case VOP_MODE_EDP:
> +		clrsetbits_le32(&regs->dsp_ctrl1,
> +				M_RK3399_DSP_EDP_POL,
> +				V_RK3399_DSP_EDP_POL(polarity));
> +		break;
> +
> +	case VOP_MODE_MIPI:
> +		clrsetbits_le32(&regs->dsp_ctrl1,
> +				M_RK3399_DSP_MIPI_POL,
> +				V_RK3399_DSP_MIPI_POL(polarity));
> +		break;
> +
> +	case VOP_MODE_LVDS:
> +		/* The RK3399 has neither parallel RGB nor LVDS output. */
> +	default:
> +		debug("%s: unsupported output mode %x\n", __func__, mode);
> +	}
> +}
> +
>  void rkvop_enable(struct rk3288_vop *regs, ulong fbbase,
>  		  int fb_bits_per_pixel, const struct display_timing *edid)
>  {
> @@ -89,50 +158,77 @@ void rkvop_enable(struct rk3288_vop *regs, ulong
> fbbase, writel(0x01, &regs->reg_cfg_done); /* enable reg config */
>  }
> 
> -void rkvop_mode_set(struct rk3288_vop *regs,
> -		    const struct display_timing *edid, enum vop_modes mode)
> +static void rkvop_set_pin_polarity(struct udevice *dev,
> +				   enum vop_modes mode, u32 polarity)
>  {
> -	u32 hactive = edid->hactive.typ;
> -	u32 vactive = edid->vactive.typ;
> -	u32 hsync_len = edid->hsync_len.typ;
> -	u32 hback_porch = edid->hback_porch.typ;
> -	u32 vsync_len = edid->vsync_len.typ;
> -	u32 vback_porch = edid->vback_porch.typ;
> -	u32 hfront_porch = edid->hfront_porch.typ;
> -	u32 vfront_porch = edid->vfront_porch.typ;
> -	uint flags;
> -	int mode_flags;
> +	struct rkvop_driverdata *ops =
> +		(struct rkvop_driverdata *)dev_get_driver_data(dev);
> +
> +	if (ops->set_pin_polarity)
> +		ops->set_pin_polarity(dev, mode, polarity);
> +}
> +
> +static void rkvop_enable_output(struct udevice *dev, enum vop_modes mode)
> +{
> +	struct rk_vop_priv *priv = dev_get_priv(dev);
> +	struct rk3288_vop *regs = priv->regs;
> 
>  	switch (mode) {
>  	case VOP_MODE_HDMI:
>  		clrsetbits_le32(&regs->sys_ctrl, M_ALL_OUT_EN,
>  				V_HDMI_OUT_EN(1));
>  		break;
> +
>  	case VOP_MODE_EDP:
> -	default:
>  		clrsetbits_le32(&regs->sys_ctrl, M_ALL_OUT_EN,
>  				V_EDP_OUT_EN(1));
>  		break;
> +
>  	case VOP_MODE_LVDS:
>  		clrsetbits_le32(&regs->sys_ctrl, M_ALL_OUT_EN,
>  				V_RGB_OUT_EN(1));
>  		break;
> +
> +	default:
> +		debug("%s: unsupported output mode %x\n", __func__, mode);
>  	}
> +}
> 
> -	if (mode == VOP_MODE_HDMI || mode == VOP_MODE_EDP)
> -		/* RGBaaa */
> -		mode_flags = 15;
> -	else
> -		/* RGB888 */
> -		mode_flags = 0;
> +void rkvop_mode_set(struct udevice *dev,
> +		    const struct display_timing *edid, enum vop_modes mode)
> +{
> +	struct rk_vop_priv *priv = dev_get_priv(dev);
> +	struct rk3288_vop *regs = priv->regs;
> +	struct rkvop_driverdata *data =
> +		(struct rkvop_driverdata *)dev_get_driver_data(dev);
> +
> +	u32 hactive = edid->hactive.typ;
> +	u32 vactive = edid->vactive.typ;
> +	u32 hsync_len = edid->hsync_len.typ;
> +	u32 hback_porch = edid->hback_porch.typ;
> +	u32 vsync_len = edid->vsync_len.typ;
> +	u32 vback_porch = edid->vback_porch.typ;
> +	u32 hfront_porch = edid->hfront_porch.typ;
> +	u32 vfront_porch = edid->vfront_porch.typ;
> +	int mode_flags;
> +	u32 pin_polarity;
> 
> -	flags = V_DSP_OUT_MODE(mode_flags) |
> -		V_DSP_HSYNC_POL(!!(edid->flags & DISPLAY_FLAGS_HSYNC_HIGH)) |
> -		V_DSP_VSYNC_POL(!!(edid->flags & DISPLAY_FLAGS_VSYNC_HIGH));
> +	pin_polarity = BIT(DCLK_INVERT);
> +	if (edid->flags & DISPLAY_FLAGS_HSYNC_HIGH)
> +		pin_polarity |= BIT(HSYNC_POSITIVE);
> +	if (edid->flags & DISPLAY_FLAGS_VSYNC_HIGH)
> +		pin_polarity |= BIT(VSYNC_POSITIVE);
> 
> -	clrsetbits_le32(&regs->dsp_ctrl0,
> -			M_DSP_OUT_MODE | M_DSP_VSYNC_POL | M_DSP_HSYNC_POL,
> -			flags);
> +	rkvop_set_pin_polarity(dev, mode, pin_polarity);
> +	rkvop_enable_output(dev, mode);
> +
> +	mode_flags = 0;  /* RGB888 */
> +	if ((data->features & VOP_FEATURE_OUTPUT_10BIT) &&
> +	    (mode == VOP_MODE_HDMI || mode == VOP_MODE_EDP))
> +		mode_flags = 15;  /* RGBaaa */
> +
> +	clrsetbits_le32(&regs->dsp_ctrl0, M_DSP_OUT_MODE,
> +			V_DSP_OUT_MODE(mode_flags));
> 
>  	writel(V_HSYNC(hsync_len) |
>  	       V_HORPRD(hsync_len + hback_porch + hactive + hfront_porch),
> @@ -249,8 +345,7 @@ int rk_display_init(struct udevice *dev, ulong fbbase,
>  		return ret;
>  	}
> 
> -	rkvop_mode_set(regs, &timing, vop_id);
> -
> +	rkvop_mode_set(dev, &timing, vop_id);
>  	rkvop_enable(regs, fbbase, 1 << l2bpp, &timing);
> 
>  	ret = display_enable(disp, 1 << l2bpp, &timing);
> @@ -341,19 +436,44 @@ static int rk_vop_bind(struct udevice *dev)
>  {
>  	struct video_uc_platdata *plat = dev_get_uclass_platdata(dev);
> 
> -	plat->size = 1920 * 1080 * 2;
> +	/*
> +	 * Let's allocate an excessively large framebuffer (i.e. the
> +	 * maximum any of the supported VOP variants can output), so
> +	 * we are prepared for the day when someone first tries to
> +	 * drive a 4K2K HDMI signal from the bootloader.
> +	 */
> +	plat->size = 4096 * 2160 * 4;
> 
>  	return 0;
>  }
> 
> -static const struct video_ops rk_vop_ops = {
> +struct rkvop_driverdata rk3288_driverdata = {
> +	.features = VOP_FEATURE_OUTPUT_10BIT,
> +	.set_pin_polarity = rk3288_set_pin_polarity,
> +};
> +
> +struct rkvop_driverdata rk3399_lit_driverdata = {
> +	.set_pin_polarity = rk3399_set_pin_polarity,
> +};
> +
> +struct rkvop_driverdata rk3399_big_driverdata = {
> +	.features = VOP_FEATURE_OUTPUT_10BIT,
> +	.set_pin_polarity = rk3399_set_pin_polarity,
>  };
> 
>  static const struct udevice_id rk_vop_ids[] = {
> -	{ .compatible = "rockchip,rk3288-vop" },
> +	{ .compatible = "rockchip,rk3288-vop",
> +	  .data = (ulong)&rk3288_driverdata },
> +	{ .compatible = "rockchip,rk3399-vop-big",
> +	  .data = (ulong)&rk3399_big_driverdata },
> +	{ .compatible = "rockchip,rk3399-vop-lit",
> +	  .data = (ulong)&rk3399_lit_driverdata },
>  	{ }
>  };
> 
> +static const struct video_ops rk_vop_ops = {
> +};
> +
>  U_BOOT_DRIVER(rk_vop) = {
>  	.name	= "rk_vop",
>  	.id	= UCLASS_VIDEO,
> --
> 1.9.1

[U-Boot] [PATCH 3/5] rockchip: video: rk3399: enable HDMI output (from the rk_vop) for the RK3399

[Simon Glass]

Hi Philipp,

On 28 April 2017 at 09:53, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> This commit enables RK3399 support for HDMI through the following
> changes:
> - adds a driverdata structure to mirror some subtle version
>   differences between the RK3399 VOPs and those in the RK3288
>   (e.g. the pin-polarity configuration)
> - configures the VOP to output 32bpp for HDMI
> - handles whether a VOP can output 10BIT data or not (i.e. RGBaaa vs. RGB888)
>   using the driverdata structure
>
> And as we touch this file anyway, we also increase the size of the
> framebuffer to a slightly overzealous 4K2K at 32bpp.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  arch/arm/include/asm/arch-rockchip/vop_rk3288.h |  11 ++
>  drivers/video/rockchip/rk_vop.c                 | 180 ++++++++++++++++++++----
>  2 files changed, 161 insertions(+), 30 deletions(-)

I'd like to somehow keep the SoC-specific code out of this driver.
You've done a nice job separating it out, but I wonder if we can go a
bit further.

I'm thinking of perhaps having two vop drivers, one for rk3288 and one
for rk3399. They can share most of the operations (e.g. bind()) which
can stay in the existing rk_vop.c file. For probe() you can rename the
existing probe() function to something like rockchip_vop_probe(), and
pass it the device and a rkvop_driverdata *.

Does that make sense? Then when we add more chips we'll have a small
extra file with the SoC-specific functionality.

Also please put the plat->size change in its own patch.

Regards,
Simon

[U-Boot] [PATCH 3/5] rockchip: video: rk3399: enable HDMI output (from the rk_vop) for the RK3399

[Dr. Philipp Tomsich]

Hi Simon,

> On 30 Apr 2017, at 05:49, Simon Glass <sjg@chromium.org> wrote:
> 
> Hi Philipp,
> 
> On 28 April 2017 at 09:53, Philipp Tomsich
> <philipp.tomsich at theobroma-systems.com <mailto:philipp.tomsich@theobroma-systems.com>> wrote:
>> This commit enables RK3399 support for HDMI through the following
>> changes:
>> - adds a driverdata structure to mirror some subtle version
>>  differences between the RK3399 VOPs and those in the RK3288
>>  (e.g. the pin-polarity configuration)
>> - configures the VOP to output 32bpp for HDMI
>> - handles whether a VOP can output 10BIT data or not (i.e. RGBaaa vs. RGB888)
>>  using the driverdata structure
>> 
>> And as we touch this file anyway, we also increase the size of the
>> framebuffer to a slightly overzealous 4K2K at 32bpp.
>> 
>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>> ---
>> 
>> arch/arm/include/asm/arch-rockchip/vop_rk3288.h |  11 ++
>> drivers/video/rockchip/rk_vop.c                 | 180 ++++++++++++++++++++----
>> 2 files changed, 161 insertions(+), 30 deletions(-)
> 
> I'd like to somehow keep the SoC-specific code out of this driver.
> You've done a nice job separating it out, but I wonder if we can go a
> bit further.
> 
> I'm thinking of perhaps having two vop drivers, one for rk3288 and one
> for rk3399. They can share most of the operations (e.g. bind()) which
> can stay in the existing rk_vop.c file. For probe() you can rename the
> existing probe() function to something like rockchip_vop_probe(), and
> pass it the device and a rkvop_driverdata *.
> 
> Does that make sense? Then when we add more chips we'll have a small
> extra file with the SoC-specific functionality.

I had considered (for any future work on this) to go into the opposite direction
and to port drivers/gpu/drm/rockchip/rockchip_vop_reg.[ch] from Linux (and
possibly even drivers/gpu/drm/rockchip/rockchip_drm_vop.c … those treat
the various variants (3288,3328,  3366, 3368, 3399-lit, 3399-big) as a single
hardware block that has a different version and sometimes has the capability
of supporting optional features (e.g. 10bit RGB output)

Instead of splitting things up it would thus put them into a single driver and
then use driverdata to model all the differences.  And to make things easier
for the long-term maintenance (and avoid mistakes in the first place), I would
rather try and reuse (large parts of) rockchip_vop_reg.[ch] verbatim in U-Boot.

Keeping things aligned with the Linux driver would be my preferred long-term
solution, if I can get a consensus for it…

Cheers,
Philipp. 

[U-Boot] [PATCH 3/5] rockchip: video: rk3399: enable HDMI output (from the rk_vop) for the RK3399

[Simon Glass]

Hi Philipp,

On 30 April 2017 at 06:31, Dr. Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> Hi Simon,
>
> On 30 Apr 2017, at 05:49, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Philipp,
>
> On 28 April 2017 at 09:53, Philipp Tomsich
> <philipp.tomsich@theobroma-systems.com> wrote:
>
> This commit enables RK3399 support for HDMI through the following
> changes:
> - adds a driverdata structure to mirror some subtle version
>  differences between the RK3399 VOPs and those in the RK3288
>  (e.g. the pin-polarity configuration)
> - configures the VOP to output 32bpp for HDMI
> - handles whether a VOP can output 10BIT data or not (i.e. RGBaaa vs.
> RGB888)
>  using the driverdata structure
>
> And as we touch this file anyway, we also increase the size of the
> framebuffer to a slightly overzealous 4K2K at 32bpp.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
> arch/arm/include/asm/arch-rockchip/vop_rk3288.h |  11 ++
> drivers/video/rockchip/rk_vop.c                 | 180
> ++++++++++++++++++++----
> 2 files changed, 161 insertions(+), 30 deletions(-)
>
>
> I'd like to somehow keep the SoC-specific code out of this driver.
> You've done a nice job separating it out, but I wonder if we can go a
> bit further.
>
> I'm thinking of perhaps having two vop drivers, one for rk3288 and one
> for rk3399. They can share most of the operations (e.g. bind()) which
> can stay in the existing rk_vop.c file. For probe() you can rename the
> existing probe() function to something like rockchip_vop_probe(), and
> pass it the device and a rkvop_driverdata *.
>
> Does that make sense? Then when we add more chips we'll have a small
> extra file with the SoC-specific functionality.
>
>
> I had considered (for any future work on this) to go into the opposite
> direction
> and to port drivers/gpu/drm/rockchip/rockchip_vop_reg.[ch] from Linux (and
> possibly even drivers/gpu/drm/rockchip/rockchip_drm_vop.c … those treat
> the various variants (3288,3328,  3366, 3368, 3399-lit, 3399-big) as a
> single
> hardware block that has a different version and sometimes has the capability
> of supporting optional features (e.g. 10bit RGB output)
>
> Instead of splitting things up it would thus put them into a single driver
> and
> then use driverdata to model all the differences.  And to make things easier
> for the long-term maintenance (and avoid mistakes in the first place), I
> would
> rather try and reuse (large parts of) rockchip_vop_reg.[ch] verbatim in
> U-Boot.
>
> Keeping things aligned with the Linux driver would be my preferred long-term
> solution, if I can get a consensus for it…

I do understand this desire, but I worry that we will end up with
monolithic drivers where adding video support will cost a lot in code
space. Linux has essentially unlimited code space so the trade-offs
are different.

If you do end up porting these things more directly from Linux then
I'd like to see how things can be broken up so that we can scale the
code size functionality a bit.

You are effectively creating a new API layer within the driver to
handle hardware differences. I really like that approach, but I feel
that putting all the implementations in this one file is unwieldy.

Having said that, from the example you gave I doubt there is a very
large difference in code size?

Regards,
Simon

[U-Boot] [PATCH 3/5] rockchip: video: rk3399: enable HDMI output (from the rk_vop) for the RK3399

[Dr. Philipp Tomsich]

> On 03 May 2017, at 05:00, Simon Glass <sjg@chromium.org> wrote:
> 
> Hi Philipp,
> 
> On 30 April 2017 at 06:31, Dr. Philipp Tomsich
> <philipp.tomsich at theobroma-systems.com <mailto:philipp.tomsich@theobroma-systems.com>> wrote:
>> Hi Simon,
>> 
>> On 30 Apr 2017, at 05:49, Simon Glass <sjg@chromium.org> wrote:
>> 
>> Hi Philipp,
>> 
>> On 28 April 2017 at 09:53, Philipp Tomsich
>> <philipp.tomsich@theobroma-systems.com> wrote:
>> 
>> This commit enables RK3399 support for HDMI through the following
>> changes:
>> - adds a driverdata structure to mirror some subtle version
>> differences between the RK3399 VOPs and those in the RK3288
>> (e.g. the pin-polarity configuration)
>> - configures the VOP to output 32bpp for HDMI
>> - handles whether a VOP can output 10BIT data or not (i.e. RGBaaa vs.
>> RGB888)
>> using the driverdata structure
>> 
>> And as we touch this file anyway, we also increase the size of the
>> framebuffer to a slightly overzealous 4K2K at 32bpp.
>> 
>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>> ---
>> 
>> arch/arm/include/asm/arch-rockchip/vop_rk3288.h |  11 ++
>> drivers/video/rockchip/rk_vop.c                 | 180
>> ++++++++++++++++++++----
>> 2 files changed, 161 insertions(+), 30 deletions(-)
>> 
>> 
>> I'd like to somehow keep the SoC-specific code out of this driver.
>> You've done a nice job separating it out, but I wonder if we can go a
>> bit further.
>> 
>> I'm thinking of perhaps having two vop drivers, one for rk3288 and one
>> for rk3399. They can share most of the operations (e.g. bind()) which
>> can stay in the existing rk_vop.c file. For probe() you can rename the
>> existing probe() function to something like rockchip_vop_probe(), and
>> pass it the device and a rkvop_driverdata *.
>> 
>> Does that make sense? Then when we add more chips we'll have a small
>> extra file with the SoC-specific functionality.
>> 
>> 
>> I had considered (for any future work on this) to go into the opposite
>> direction
>> and to port drivers/gpu/drm/rockchip/rockchip_vop_reg.[ch] from Linux (and
>> possibly even drivers/gpu/drm/rockchip/rockchip_drm_vop.c … those treat
>> the various variants (3288,3328,  3366, 3368, 3399-lit, 3399-big) as a
>> single
>> hardware block that has a different version and sometimes has the capability
>> of supporting optional features (e.g. 10bit RGB output)
>> 
>> Instead of splitting things up it would thus put them into a single driver
>> and
>> then use driverdata to model all the differences.  And to make things easier
>> for the long-term maintenance (and avoid mistakes in the first place), I
>> would
>> rather try and reuse (large parts of) rockchip_vop_reg.[ch] verbatim in
>> U-Boot.
>> 
>> Keeping things aligned with the Linux driver would be my preferred long-term
>> solution, if I can get a consensus for it…
> 
> I do understand this desire, but I worry that we will end up with
> monolithic drivers where adding video support will cost a lot in code
> space. Linux has essentially unlimited code space so the trade-offs
> are different.
> 
> If you do end up porting these things more directly from Linux then
> I'd like to see how things can be broken up so that we can scale the
> code size functionality a bit.
> 
> You are effectively creating a new API layer within the driver to
> handle hardware differences. I really like that approach, but I feel
> that putting all the implementations in this one file is unwieldy.
> 
> Having said that, from the example you gave I doubt there is a very
> large difference in code size?

In fact I started to look into the changes in more detail yesterday and already
started to cut this up into individual drivers along your suggestion: on second
look reusing the Linux code does not conflict the approach you suggested, as
I can supply the register layout differences from the SoC-specific “mini-drivers”.

Regards,
Philipp.

[U-Boot] [PATCH 4/5] rockchip: video: rk3399: add HDMI TX support on the RK3399

[Philipp Tomsich]

This commit enables the RK3399 HDMI TX, which is very similar to the
one found on the RK3288. The differences between the two SoCs (mainly
the input VOP selection) is abstracted away through the driverdata.

Note that the I2C communication for reading the EDID works well with
the default settings, but does not with the alternate settings used on
the RK3288... so this configuration aspect also is part of the
driverdata.

Having some sort of DTS-based configuration for the regulator
dependencies would be nice for the future, but for now we simply use
lists of regulator names (also via driverdata) that we probe.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 arch/arm/include/asm/arch-rockchip/grf_rk3399.h |   5 +
 drivers/video/rockchip/rk_hdmi.c                | 165 +++++++++++++++++++-----
 2 files changed, 135 insertions(+), 35 deletions(-)

diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3399.h b/arch/arm/include/asm/arch-rockchip/grf_rk3399.h
index 22d8d97..ddd558e 100644
--- a/arch/arm/include/asm/arch-rockchip/grf_rk3399.h
+++ b/arch/arm/include/asm/arch-rockchip/grf_rk3399.h
@@ -462,6 +462,11 @@ enum {
 	GRF_UART_DBG_SEL_MASK	= 3 << GRF_UART_DBG_SEL_SHIFT,
 	GRF_UART_DBG_SEL_C	= 2,
 
+	/* GRF_SOC_CON20 */
+	GRF_RK3399_HDMI_VOP_SEL_MASK = 1 << 6,
+	GRF_RK3399_HDMI_VOP_SEL_B = 0 << 6,
+	GRF_RK3399_HDMI_VOP_SEL_L = 1 << 6,
+
 	/*  PMUGRF_GPIO0A_IOMUX */
 	PMUGRF_GPIO0A6_SEL_SHIFT        = 12,
 	PMUGRF_GPIO0A6_SEL_MASK = 3 << PMUGRF_GPIO0A6_SEL_SHIFT,
diff --git a/drivers/video/rockchip/rk_hdmi.c b/drivers/video/rockchip/rk_hdmi.c
index db07588..ea92551 100644
--- a/drivers/video/rockchip/rk_hdmi.c
+++ b/drivers/video/rockchip/rk_hdmi.c
@@ -1,10 +1,13 @@
 /*
+ * Copyright (c) 2017 Theobroma Systems Design und Consulting GmbH
  * Copyright (c) 2015 Google, Inc
  * Copyright 2014 Rockchip Inc.
  *
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
+#define DEBUG
+
 #include <common.h>
 #include <clk.h>
 #include <display.h>
@@ -16,12 +19,25 @@
 #include <asm/gpio.h>
 #include <asm/io.h>
 #include <asm/arch/clock.h>
+#include <asm/arch/hardware.h>
 #include <asm/arch/grf_rk3288.h>
+#include <asm/arch/grf_rk3399.h>
 #include <power/regulator.h>
 
 struct rk_hdmi_priv {
 	struct dw_hdmi hdmi;
-	struct rk3288_grf *grf;
+	void *grf;
+};
+
+struct rkhdmi_driverdata {
+	/* configuration */
+	u8 i2c_clk_high;
+	u8 i2c_clk_low;
+	const char * const *regulator_names;
+	u32 regulator_names_cnt;
+	/* setters/getters */
+	int (*set_input_vop)(struct udevice *dev);
+	int (*clk_config)(struct udevice *dev);
 };
 
 static const struct hdmi_phy_config rockchip_phy_config[] = {
@@ -65,6 +81,60 @@ static const struct hdmi_mpll_config rockchip_mpll_cfg[] = {
 	}
 };
 
+static int rk3288_set_input_vop(struct udevice *dev)
+{
+	struct rk_hdmi_priv *priv = dev_get_priv(dev);
+	struct display_plat *uc_plat = dev_get_uclass_platdata(dev);
+	int vop_id = uc_plat->source_id;
+	struct rk3288_grf *grf = priv->grf;
+
+	/* hdmi source select hdmi controller */
+	rk_setreg(&grf->soc_con6, 1 << 15);
+
+	/* hdmi data from vop id */
+	rk_clrsetreg(&grf->soc_con6, 1 << 4, (vop_id == 1) ? (1 << 4) : 0);
+
+	return 0;
+}
+
+static int rk3399_set_input_vop(struct udevice *dev)
+{
+	struct rk_hdmi_priv *priv = dev_get_priv(dev);
+	struct display_plat *uc_plat = dev_get_uclass_platdata(dev);
+	int vop_id = uc_plat->source_id;
+	struct rk3399_grf_regs *grf = priv->grf;
+
+	/* hdmi data from vop id */
+	rk_clrsetreg(&grf->soc_con20, GRF_RK3399_HDMI_VOP_SEL_MASK,
+		     (vop_id == 1) ? GRF_RK3399_HDMI_VOP_SEL_L : 0);
+
+	return 0;
+}
+
+static int rk3288_clk_config(struct udevice *dev)
+{
+	struct display_plat *uc_plat = dev_get_uclass_platdata(dev);
+	struct clk clk;
+	int ret;
+
+	/*
+	 * Configure the maximum clock to permit whatever resolution the
+	 * monitor wants
+	 */
+	ret = clk_get_by_index(uc_plat->src_dev, 0, &clk);
+	if (ret >= 0) {
+		ret = clk_set_rate(&clk, 384000000);
+		clk_free(&clk);
+	}
+	if (ret < 0) {
+		debug("%s: Failed to set clock in source device '%s': ret=%d\n",
+		      __func__, uc_plat->src_dev->name, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int rk_hdmi_read_edid(struct udevice *dev, u8 *buf, int buf_size)
 {
 	struct rk_hdmi_priv *priv = dev_get_priv(dev);
@@ -83,20 +153,16 @@ static int rk_hdmi_enable(struct udevice *dev, int panel_bpp,
 static int rk_hdmi_ofdata_to_platdata(struct udevice *dev)
 {
 	struct rk_hdmi_priv *priv = dev_get_priv(dev);
+	struct rkhdmi_driverdata *data =
+		(struct rkhdmi_driverdata *)dev_get_driver_data(dev);
 	struct dw_hdmi *hdmi = &priv->hdmi;
 
 	hdmi->ioaddr = (ulong)dev_get_addr(dev);
 	hdmi->mpll_cfg = rockchip_mpll_cfg;
 	hdmi->phy_cfg = rockchip_phy_config;
-	hdmi->i2c_clk_high = 0x7a;
-	hdmi->i2c_clk_low = 0x8d;
+	hdmi->i2c_clk_high = data->i2c_clk_high;
+	hdmi->i2c_clk_low = data->i2c_clk_low;
 
-	/*
-	 * TODO(sjg at chromium.org): The above values don't work - these ones
-	 * work better, but generate lots of errors in the data.
-	 */
-	hdmi->i2c_clk_high = 0x0d;
-	hdmi->i2c_clk_low = 0x0d;
 	hdmi->reg_io_width = 4;
 	hdmi->phy_set = dw_hdmi_phy_cfg;
 
@@ -107,13 +173,15 @@ static int rk_hdmi_ofdata_to_platdata(struct udevice *dev)
 
 static int rk_hdmi_probe(struct udevice *dev)
 {
-	struct display_plat *uc_plat = dev_get_uclass_platdata(dev);
+	struct rkhdmi_driverdata *data =
+		(struct rkhdmi_driverdata *)dev_get_driver_data(dev);
 	struct rk_hdmi_priv *priv = dev_get_priv(dev);
 	struct dw_hdmi *hdmi = &priv->hdmi;
 	struct udevice *reg;
 	struct clk clk;
+	const char *regulator_name;
 	int ret;
-	int vop_id = uc_plat->source_id;
+	int i;
 
 	ret = clk_get_by_index(dev, 0, &clk);
 	if (ret >= 0) {
@@ -125,33 +193,27 @@ static int rk_hdmi_probe(struct udevice *dev)
 		return ret;
 	}
 
-	/*
-	 * Configure the maximum clock to permit whatever resolution the
-	 * monitor wants
-	 */
-	ret = clk_get_by_index(uc_plat->src_dev, 0, &clk);
-	if (ret >= 0) {
-		ret = clk_set_rate(&clk, 384000000);
-		clk_free(&clk);
-	}
-	if (ret < 0) {
-		debug("%s: Failed to set clock in source device '%s': ret=%d\n",
-		      __func__, uc_plat->src_dev->name, ret);
-		return ret;
+	if (data->clk_config) {
+		ret = data->clk_config(dev);
+		if (ret < 0)
+			return ret;
 	}
 
-	ret = regulator_get_by_platname("vcc50_hdmi", &reg);
-	if (!ret)
-		ret = regulator_set_enable(reg, true);
-	if (ret)
-		debug("%s: Cannot set regulator vcc50_hdmi\n", __func__);
+	for (i = 0; i < data->regulator_names_cnt; ++i) {
+		regulator_name = data->regulator_names[i];
+		debug("%s: probing regulator '%s'\n", __func__, regulator_name);
 
-	/* hdmi source select hdmi controller */
-	rk_setreg(&priv->grf->soc_con6, 1 << 15);
+		ret = regulator_autoset_by_name(regulator_name, &reg);
+		if (!ret)
+			ret = regulator_set_enable(reg, true);
+	}
 
-	/* hdmi data from vop id */
-	rk_clrsetreg(&priv->grf->soc_con6, 1 << 4,
-		     (vop_id == 1) ? (1 << 4) : 0);
+	if (!data->set_input_vop) {
+		debug("%s: data->set_input_vop not set\n", __func__);
+		return -1;
+	}
+
+	data->set_input_vop(dev);
 
 	ret = dw_hdmi_phy_wait_for_hpd(hdmi);
 	if (ret < 0) {
@@ -170,8 +232,41 @@ static const struct dm_display_ops rk_hdmi_ops = {
 	.enable = rk_hdmi_enable,
 };
 
+static const char * const rk3288_regulator_names[] = {
+	"vcc50_hdmi",
+};
+
+static const struct rkhdmi_driverdata rk3288_driverdata = {
+	/*
+	 * TODO(sjg@chromium.org): The above values don't work - these ones
+	 * work better, but generate lots of errors in the data.
+	 */
+	.i2c_clk_high = 0x0d,
+	.i2c_clk_low = 0x0d,
+	.regulator_names = rk3288_regulator_names,
+	.regulator_names_cnt = ARRAY_SIZE(rk3288_regulator_names),
+	.set_input_vop = rk3288_set_input_vop,
+	.clk_config = rk3288_clk_config,
+};
+
+static const char * const rk3399_regulator_names[] = {
+	"vcc1v8_hdmi",
+	"vcc0v9_hdmi"
+};
+
+static const struct rkhdmi_driverdata rk3399_driverdata = {
+	.i2c_clk_high = 0x7a,
+	.i2c_clk_low = 0x8d,
+	.regulator_names = rk3399_regulator_names,
+	.regulator_names_cnt = ARRAY_SIZE(rk3399_regulator_names),
+	.set_input_vop = rk3399_set_input_vop,
+};
+
 static const struct udevice_id rk_hdmi_ids[] = {
-	{ .compatible = "rockchip,rk3288-dw-hdmi" },
+	{ .compatible = "rockchip,rk3288-dw-hdmi",
+	  .data = (ulong)&rk3288_driverdata },
+	{ .compatible = "rockchip,rk3399-dw-hdmi",
+	  .data = (ulong)&rk3399_driverdata },
 	{ }
 };
 
-- 
1.9.1

[U-Boot] [PATCH 4/5] rockchip: video: rk3399: add HDMI TX support on the RK3399

[Jernej Škrabec]

Hi Philipp,

Dne petek, 28. april 2017 ob 17:53:11 CEST je Philipp Tomsich napisal(a):
> This commit enables the RK3399 HDMI TX, which is very similar to the
> one found on the RK3288. The differences between the two SoCs (mainly
> the input VOP selection) is abstracted away through the driverdata.
> 
> Note that the I2C communication for reading the EDID works well with
> the default settings, but does not with the alternate settings used on
> the RK3288... so this configuration aspect also is part of the
> driverdata.
> 
> Having some sort of DTS-based configuration for the regulator
> dependencies would be nice for the future, but for now we simply use
> lists of regulator names (also via driverdata) that we probe.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
> 
>  arch/arm/include/asm/arch-rockchip/grf_rk3399.h |   5 +
>  drivers/video/rockchip/rk_hdmi.c                | 165
> +++++++++++++++++++----- 2 files changed, 135 insertions(+), 35
> deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3399.h
> b/arch/arm/include/asm/arch-rockchip/grf_rk3399.h index 22d8d97..ddd558e
> 100644
> --- a/arch/arm/include/asm/arch-rockchip/grf_rk3399.h
> +++ b/arch/arm/include/asm/arch-rockchip/grf_rk3399.h
> @@ -462,6 +462,11 @@ enum {
>  	GRF_UART_DBG_SEL_MASK	= 3 << GRF_UART_DBG_SEL_SHIFT,
>  	GRF_UART_DBG_SEL_C	= 2,
> 
> +	/* GRF_SOC_CON20 */
> +	GRF_RK3399_HDMI_VOP_SEL_MASK = 1 << 6,
> +	GRF_RK3399_HDMI_VOP_SEL_B = 0 << 6,
> +	GRF_RK3399_HDMI_VOP_SEL_L = 1 << 6,
> +
>  	/*  PMUGRF_GPIO0A_IOMUX */
>  	PMUGRF_GPIO0A6_SEL_SHIFT        = 12,
>  	PMUGRF_GPIO0A6_SEL_MASK = 3 << PMUGRF_GPIO0A6_SEL_SHIFT,
> diff --git a/drivers/video/rockchip/rk_hdmi.c
> b/drivers/video/rockchip/rk_hdmi.c index db07588..ea92551 100644
> --- a/drivers/video/rockchip/rk_hdmi.c
> +++ b/drivers/video/rockchip/rk_hdmi.c
> @@ -1,10 +1,13 @@
>  /*
> + * Copyright (c) 2017 Theobroma Systems Design und Consulting GmbH
>   * Copyright (c) 2015 Google, Inc
>   * Copyright 2014 Rockchip Inc.
>   *
>   * SPDX-License-Identifier:	GPL-2.0+
>   */
> 
> +#define DEBUG
> +

That shouldn't be in final version.

>  #include <common.h>
>  #include <clk.h>
>  #include <display.h>
> @@ -16,12 +19,25 @@
>  #include <asm/gpio.h>
>  #include <asm/io.h>
>  #include <asm/arch/clock.h>
> +#include <asm/arch/hardware.h>
>  #include <asm/arch/grf_rk3288.h>
> +#include <asm/arch/grf_rk3399.h>
>  #include <power/regulator.h>
> 
>  struct rk_hdmi_priv {
>  	struct dw_hdmi hdmi;
> -	struct rk3288_grf *grf;
> +	void *grf;
> +};
> +
> +struct rkhdmi_driverdata {
> +	/* configuration */
> +	u8 i2c_clk_high;
> +	u8 i2c_clk_low;
> +	const char * const *regulator_names;
> +	u32 regulator_names_cnt;
> +	/* setters/getters */
> +	int (*set_input_vop)(struct udevice *dev);
> +	int (*clk_config)(struct udevice *dev);
>  };
> 
>  static const struct hdmi_phy_config rockchip_phy_config[] = {
> @@ -65,6 +81,60 @@ static const struct hdmi_mpll_config rockchip_mpll_cfg[]
> = { }
>  };
> 
> +static int rk3288_set_input_vop(struct udevice *dev)
> +{
> +	struct rk_hdmi_priv *priv = dev_get_priv(dev);
> +	struct display_plat *uc_plat = dev_get_uclass_platdata(dev);
> +	int vop_id = uc_plat->source_id;
> +	struct rk3288_grf *grf = priv->grf;
> +
> +	/* hdmi source select hdmi controller */
> +	rk_setreg(&grf->soc_con6, 1 << 15);
> +
> +	/* hdmi data from vop id */
> +	rk_clrsetreg(&grf->soc_con6, 1 << 4, (vop_id == 1) ? (1 << 4) : 0);
> +
> +	return 0;
> +}
> +
> +static int rk3399_set_input_vop(struct udevice *dev)
> +{
> +	struct rk_hdmi_priv *priv = dev_get_priv(dev);
> +	struct display_plat *uc_plat = dev_get_uclass_platdata(dev);
> +	int vop_id = uc_plat->source_id;
> +	struct rk3399_grf_regs *grf = priv->grf;
> +
> +	/* hdmi data from vop id */
> +	rk_clrsetreg(&grf->soc_con20, GRF_RK3399_HDMI_VOP_SEL_MASK,
> +		     (vop_id == 1) ? GRF_RK3399_HDMI_VOP_SEL_L : 0);
> +
> +	return 0;
> +}
> +
> +static int rk3288_clk_config(struct udevice *dev)
> +{
> +	struct display_plat *uc_plat = dev_get_uclass_platdata(dev);
> +	struct clk clk;
> +	int ret;
> +
> +	/*
> +	 * Configure the maximum clock to permit whatever resolution the
> +	 * monitor wants
> +	 */
> +	ret = clk_get_by_index(uc_plat->src_dev, 0, &clk);
> +	if (ret >= 0) {
> +		ret = clk_set_rate(&clk, 384000000);
> +		clk_free(&clk);
> +	}
> +	if (ret < 0) {
> +		debug("%s: Failed to set clock in source device '%s': ret=%d\n",
> +		      __func__, uc_plat->src_dev->name, ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static int rk_hdmi_read_edid(struct udevice *dev, u8 *buf, int buf_size)
>  {
>  	struct rk_hdmi_priv *priv = dev_get_priv(dev);
> @@ -83,20 +153,16 @@ static int rk_hdmi_enable(struct udevice *dev, int
> panel_bpp, static int rk_hdmi_ofdata_to_platdata(struct udevice *dev)
>  {
>  	struct rk_hdmi_priv *priv = dev_get_priv(dev);
> +	struct rkhdmi_driverdata *data =
> +		(struct rkhdmi_driverdata *)dev_get_driver_data(dev);
>  	struct dw_hdmi *hdmi = &priv->hdmi;
> 
>  	hdmi->ioaddr = (ulong)dev_get_addr(dev);
>  	hdmi->mpll_cfg = rockchip_mpll_cfg;
>  	hdmi->phy_cfg = rockchip_phy_config;
> -	hdmi->i2c_clk_high = 0x7a;
> -	hdmi->i2c_clk_low = 0x8d;
> +	hdmi->i2c_clk_high = data->i2c_clk_high;
> +	hdmi->i2c_clk_low = data->i2c_clk_low;
> 
> -	/*
> -	 * TODO(sjg at chromium.org): The above values don't work - these ones
> -	 * work better, but generate lots of errors in the data.
> -	 */
> -	hdmi->i2c_clk_high = 0x0d;
> -	hdmi->i2c_clk_low = 0x0d;
>  	hdmi->reg_io_width = 4;
>  	hdmi->phy_set = dw_hdmi_phy_cfg;
> 
> @@ -107,13 +173,15 @@ static int rk_hdmi_ofdata_to_platdata(struct udevice
> *dev)
> 
>  static int rk_hdmi_probe(struct udevice *dev)
>  {
> -	struct display_plat *uc_plat = dev_get_uclass_platdata(dev);
> +	struct rkhdmi_driverdata *data =
> +		(struct rkhdmi_driverdata *)dev_get_driver_data(dev);
>  	struct rk_hdmi_priv *priv = dev_get_priv(dev);
>  	struct dw_hdmi *hdmi = &priv->hdmi;
>  	struct udevice *reg;
>  	struct clk clk;
> +	const char *regulator_name;
>  	int ret;
> -	int vop_id = uc_plat->source_id;
> +	int i;
> 
>  	ret = clk_get_by_index(dev, 0, &clk);
>  	if (ret >= 0) {
> @@ -125,33 +193,27 @@ static int rk_hdmi_probe(struct udevice *dev)
>  		return ret;
>  	}
> 
> -	/*
> -	 * Configure the maximum clock to permit whatever resolution the
> -	 * monitor wants
> -	 */
> -	ret = clk_get_by_index(uc_plat->src_dev, 0, &clk);
> -	if (ret >= 0) {
> -		ret = clk_set_rate(&clk, 384000000);
> -		clk_free(&clk);
> -	}
> -	if (ret < 0) {
> -		debug("%s: Failed to set clock in source device '%s': ret=%d\n",
> -		      __func__, uc_plat->src_dev->name, ret);
> -		return ret;
> +	if (data->clk_config) {
> +		ret = data->clk_config(dev);
> +		if (ret < 0)
> +			return ret;
>  	}
> 
> -	ret = regulator_get_by_platname("vcc50_hdmi", &reg);
> -	if (!ret)
> -		ret = regulator_set_enable(reg, true);
> -	if (ret)
> -		debug("%s: Cannot set regulator vcc50_hdmi\n", __func__);
> +	for (i = 0; i < data->regulator_names_cnt; ++i) {
> +		regulator_name = data->regulator_names[i];
> +		debug("%s: probing regulator '%s'\n", __func__, regulator_name);
> 
> -	/* hdmi source select hdmi controller */
> -	rk_setreg(&priv->grf->soc_con6, 1 << 15);
> +		ret = regulator_autoset_by_name(regulator_name, &reg);
> +		if (!ret)
> +			ret = regulator_set_enable(reg, true);
> +	}
> 
> -	/* hdmi data from vop id */
> -	rk_clrsetreg(&priv->grf->soc_con6, 1 << 4,
> -		     (vop_id == 1) ? (1 << 4) : 0);
> +	if (!data->set_input_vop) {
> +		debug("%s: data->set_input_vop not set\n", __func__);
> +		return -1;
> +	}
> +
> +	data->set_input_vop(dev);
> 
>  	ret = dw_hdmi_phy_wait_for_hpd(hdmi);
>  	if (ret < 0) {
> @@ -170,8 +232,41 @@ static const struct dm_display_ops rk_hdmi_ops = {
>  	.enable = rk_hdmi_enable,
>  };
> 
> +static const char * const rk3288_regulator_names[] = {
> +	"vcc50_hdmi",
> +};
> +
> +static const struct rkhdmi_driverdata rk3288_driverdata = {
> +	/*
> +	 * TODO(sjg at chromium.org): The above values don't work - these ones
> +	 * work better, but generate lots of errors in the data.
> +	 */

Maybe you should rephrase the comment. In this form it doesn't make sense 
anymore.

Regards,
Jernej

> +	.i2c_clk_high = 0x0d,
> +	.i2c_clk_low = 0x0d,
> +	.regulator_names = rk3288_regulator_names,
> +	.regulator_names_cnt = ARRAY_SIZE(rk3288_regulator_names),
> +	.set_input_vop = rk3288_set_input_vop,
> +	.clk_config = rk3288_clk_config,
> +};
> +
> +static const char * const rk3399_regulator_names[] = {
> +	"vcc1v8_hdmi",
> +	"vcc0v9_hdmi"
> +};
> +
> +static const struct rkhdmi_driverdata rk3399_driverdata = {
> +	.i2c_clk_high = 0x7a,
> +	.i2c_clk_low = 0x8d,
> +	.regulator_names = rk3399_regulator_names,
> +	.regulator_names_cnt = ARRAY_SIZE(rk3399_regulator_names),
> +	.set_input_vop = rk3399_set_input_vop,
> +};
> +
>  static const struct udevice_id rk_hdmi_ids[] = {
> -	{ .compatible = "rockchip,rk3288-dw-hdmi" },
> +	{ .compatible = "rockchip,rk3288-dw-hdmi",
> +	  .data = (ulong)&rk3288_driverdata },
> +	{ .compatible = "rockchip,rk3399-dw-hdmi",
> +	  .data = (ulong)&rk3399_driverdata },
>  	{ }
>  };
> 
> --
> 1.9.1

[U-Boot] [PATCH 4/5] rockchip: video: rk3399: add HDMI TX support on the RK3399

[Simon Glass]

Hi Philipp,

On 28 April 2017 at 09:53, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> This commit enables the RK3399 HDMI TX, which is very similar to the
> one found on the RK3288. The differences between the two SoCs (mainly
> the input VOP selection) is abstracted away through the driverdata.
>
> Note that the I2C communication for reading the EDID works well with
> the default settings, but does not with the alternate settings used on
> the RK3288... so this configuration aspect also is part of the
> driverdata.
>
> Having some sort of DTS-based configuration for the regulator
> dependencies would be nice for the future, but for now we simply use
> lists of regulator names (also via driverdata) that we probe.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  arch/arm/include/asm/arch-rockchip/grf_rk3399.h |   5 +
>  drivers/video/rockchip/rk_hdmi.c                | 165 +++++++++++++++++++-----
>  2 files changed, 135 insertions(+), 35 deletions(-)

Similar comment to the previous patch.

- Simon

[U-Boot] [PATCH 5/5] rockchip: dts: rk3399: enable HDMI output in the DTS

[Philipp Tomsich]

This commit enables HDMI output in the DTS by adding the necessary
nodes to vopl/vopb and by adding the HDMI node.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 arch/arm/dts/rk3399.dtsi | 110 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)

diff --git a/arch/arm/dts/rk3399.dtsi b/arch/arm/dts/rk3399.dtsi
index d94d780..4d40198 100644
--- a/arch/arm/dts/rk3399.dtsi
+++ b/arch/arm/dts/rk3399.dtsi
@@ -684,6 +684,116 @@
 		status = "disabled";
 	};
 
+	vopl: vop at ff8f0000 {
+		u-boot,dm-pre-reloc;
+		compatible = "rockchip,rk3399-vop-lit";
+		reg = <0x0 0xff8f0000 0x0 0x3efc>;
+		interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH 0>;
+		clocks = <&cru ACLK_VOP1>, <&cru DCLK_VOP1>, <&cru HCLK_VOP1>;
+		clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
+		resets = <&cru SRST_A_VOP1>, <&cru SRST_H_VOP1>, <&cru SRST_D_VOP1>;
+		reset-names = "axi", "ahb", "dclk";
+		status = "okay";
+		vopl_out: port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			vopl_out_mipi: endpoint at 0 {
+				reg = <3>;
+				remote-endpoint = <&mipi_in_vopl>;
+			};
+
+			vopl_out_hdmi: endpoint at 1 {
+				reg = <1>;
+				remote-endpoint = <&hdmi_in_vopl>;
+			};
+		};
+	};
+
+	vopb: vop at ff900000 {
+		u-boot,dm-pre-reloc;
+		compatible = "rockchip,rk3399-vop-big";
+		reg = <0x0 0xff900000 0x0 0x3efc>;
+		interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH 0>;
+		clocks = <&cru ACLK_VOP0>, <&cru DCLK_VOP0>, <&cru HCLK_VOP0>;
+		#clock-cells = <0>;
+		clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
+		resets = <&cru SRST_A_VOP0>, <&cru SRST_H_VOP0>, <&cru SRST_D_VOP0>;
+		reset-names = "axi", "ahb", "dclk";
+		status = "okay";
+		vopb_out: port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			vopb_out_mipi: endpoint at 0 {
+				reg = <3>;
+				remote-endpoint = <&mipi_in_vopb>;
+			};
+
+			vopb_out_hdmi: endpoint at 1 {
+				reg = <1>;
+				remote-endpoint = <&hdmi_in_vopb>;
+			};
+		};
+	};
+
+	hdmi: hdmi at ff940000 {
+		compatible = "rockchip,rk3399-dw-hdmi";
+		reg = <0x0 0xff940000 0x0 0x20000>;
+		reg-io-width = <4>;
+		rockchip,grf = <&grf>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&hdmi_i2c_xfer>;
+		power-domains = <&power RK3399_PD_HDCP>;
+		interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH 0>;
+		clocks = <&cru PCLK_HDMI_CTRL>, <&cru SCLK_HDMI_SFR>, <&cru PLL_VPLL>, <&cru PCLK_VIO_GRF>;
+		clock-names = "iahb", "isfr", "vpll", "grf";
+		status = "disabled";
+
+		ports {
+			hdmi_in: port {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				hdmi_in_vopb: endpoint at 0 {
+					reg = <0>;
+					remote-endpoint = <&vopb_out_hdmi>;
+				};
+				hdmi_in_vopl: endpoint at 1 {
+					reg = <1>;
+					remote-endpoint = <&vopl_out_hdmi>;
+				};
+			};
+		};
+	};
+
+	mipi_dsi: mipi at ff960000 {
+		compatible = "rockchip,rk3399_mipi_dsi";
+		reg = <0x0 0xff960000 0x0 0x8000>;
+		interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH 0>;
+		clocks = <&cru SCLK_MIPIDPHY_REF>, <&cru PCLK_MIPI_DSI0>,
+		         <&cru SCLK_DPHY_TX0_CFG>;
+		clock-names = "ref", "pclk", "phy_cfg";
+		rockchip,grf = <&grf>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <1>;
+			mipi_in: port {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				mipi_in_vopb: endpoint at 0 {
+					reg = <0>;
+					remote-endpoint = <&vopb_out_mipi>;
+				};
+				mipi_in_vopl: endpoint at 1 {
+					reg = <1>;
+					remote-endpoint = <&vopl_out_mipi>;
+				};
+			};
+		};
+	};
+
 	pinctrl: pinctrl {
 		u-boot,dm-pre-reloc;
 		compatible = "rockchip,rk3399-pinctrl";
-- 
1.9.1