[PATCH v1 0/6] Add RK3399 HDMI Support

[PATCH v1 0/6] Add RK3399 HDMI Support

[Yakir Yang]

Mark,

RK3399 and RK3288 shared the same HDMI IP controller, only some
light difference with GRF configure, and an external VPLL clock
need to configure.

Thanks,
- Yakir


Douglas Anderson (2):
  drm/rockchip: dw_hdmi: Set cur_ctr to 0 always
  drm/rockchip: dw_hdmi: Use auto-generated tables

Yakir Yang (4):
  drm/rockchip: dw_hdmi: adjust cklvl & txlvl for RF/EMI
  drm/rockchip: dw_hdmi: add RK3399 HDMI support
  drm/rockchip: dw_hdmi: introduce the VPLL clock setting
  drm/rockchip: dw_hdmi: introduce the pclk for grf

 .../devicetree/bindings/display/bridge/dw_hdmi.txt |   1 +
 .../bindings/display/rockchip/dw_hdmi-rockchip.txt |   7 +-
 drivers/gpu/drm/bridge/dw-hdmi.c                   |   2 +-
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        | 238 +++++++++++++--------
 include/drm/bridge/dw_hdmi.h                       |   6 +
 5 files changed, 167 insertions(+), 87 deletions(-)

-- 
1.9.1

[PATCH v1 1/6] drm/rockchip: dw_hdmi: Set cur_ctr to 0 always

[Yakir Yang]

From: Douglas Anderson <dianders@chromium.org>

Jitter was improved by lowering the MPLL bandwidth to account for high
frequency noise in the rk3288 PLL.  In each case MPLL bandwidth was
lowered only enough to get us a comfortable margin.  We believe that
lowering the bandwidth like this is safe given sufficient testing.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 801110f..a621118 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -115,20 +115,8 @@ static const struct dw_hdmi_mpll_config rockchip_mpll_cfg[] = {
 static const struct dw_hdmi_curr_ctrl rockchip_cur_ctr[] = {
 	/*      pixelclk    bpp8    bpp10   bpp12 */
 	{
-		40000000,  { 0x0018, 0x0018, 0x0018 },
-	}, {
-		65000000,  { 0x0028, 0x0028, 0x0028 },
-	}, {
-		66000000,  { 0x0038, 0x0038, 0x0038 },
-	}, {
-		74250000,  { 0x0028, 0x0038, 0x0038 },
-	}, {
-		83500000,  { 0x0028, 0x0038, 0x0038 },
-	}, {
-		146250000, { 0x0038, 0x0038, 0x0038 },
-	}, {
-		148500000, { 0x0000, 0x0038, 0x0038 },
-	}, {
+		600000000, { 0x0000, 0x0000, 0x0000 },
+	},  {
 		~0UL,      { 0x0000, 0x0000, 0x0000},
 	}
 };
-- 
1.9.1

[PATCH v1 2/6] drm/rockchip: dw_hdmi: adjust cklvl & txlvl for RF/EMI

[Yakir Yang]

Dut to the high HDMI signal voltage driver, Mickey have meet
a serious RF/EMI problem, so we decided to reduce HDMI signal
voltage to a proper value.

The default params for phy is cklvl = 20 & txlvl = 13 (RF/EMI failed)
  ck: lvl = 13, term=100, vlo = 2.71, vhi=3.14, vswing = 0.43
  tx: lvl = 20, term=100, vlo = 2.81, vhi=3.16, vswing = 0.35

1. We decided to reduce voltage value to lower, but VSwing still
keep high, RF/EMI have been improved but still failed.
   ck: lvl =  6, term=100, vlo = 2.61, vhi=3.11, vswing = 0.50
   tx: lvl =  6, term=100, vlo = 2.61, vhi=3.11, vswing = 0.50

2. We try to keep voltage value and vswing both lower, then RF/EMI
test all passed  ;)
   ck: lvl = 11, term= 66, vlo = 2.68, vhi=3.09, vswing = 0.40
   tx: lvl = 11, term= 66, vlo = 2.68, vhi=3.09, vswing = 0.40
When we back to run HDMI different test and single-end test, we see
different test passed, but signle-end test failed. The oscilloscope
show that simgle-end clock's VL value is 1.78v (which remind LowLimit
should not lower then 2.6v).

3. That's to say there are some different between PHY document and
measure value. And according to experiment 2 results, we need to
higher clock voltage and lower data voltage, then we can keep RF/EMI
satisfied and single-end & differen test passed.
  ck: lvl =  9, term=100, vlo = 2.65, vhi=3.12, vswing = 0.47
  tx: lvl = 16, term=100, vlo = 2.75, vhi=3.15, vswing = 0.39

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index a621118..cdb63ac 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -124,7 +124,7 @@ static const struct dw_hdmi_curr_ctrl rockchip_cur_ctr[] = {
 static const struct dw_hdmi_phy_config rockchip_phy_config[] = {
 	/*pixelclk   symbol   term   vlev*/
 	{ 74250000,  0x8009, 0x0004, 0x0272},
-	{ 148500000, 0x802b, 0x0004, 0x028d},
+	{ 165000000, 0x802b, 0x0004, 0x0209},
 	{ 297000000, 0x8039, 0x0005, 0x028d},
 	{ ~0UL,	     0x0000, 0x0000, 0x0000}
 };
-- 
1.9.1

[PATCH v1 3/6] drm/rockchip: dw_hdmi: Use auto-generated tables

[Yakir Yang]

From: Douglas Anderson <dianders@chromium.org>

The previous tables for mpll_cfg and curr_ctrl were created using the
20-pages of example settings provided by the PHY vendor.  Those
example settings weren't particularly dense, so there were places
where we were guessing what the settings would be for 10-bit and
12-bit (not that we use those anyway).  It was also always a lot of
extra work every time we wanted to add a new clock rate since we had
to cross-reference several tables.

In <http://crosreview.com/285855> I've gone through the work to figure
out how to generate this table automatically.  Let's now use the
automatically generated table and then we'll never need to look at it
again.

We only support 8-bit mode right now and only support a small number
of clock rates and and I've verified that the only 8-bit rate that was
affected was 148.5.  That mode appears to have been wrong in the old
table.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Yakir Yang <ykk@rock-chips.com>
Reviewed-by: St?phane Marchesin <marcheu@chromium.org>
---
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 130 +++++++++++++++-------------
 1 file changed, 69 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index cdb63ac..8cb9ed2 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -34,80 +34,88 @@ struct rockchip_hdmi {
 
 static const struct dw_hdmi_mpll_config rockchip_mpll_cfg[] = {
 	{
-		27000000, {
-			{ 0x00b3, 0x0000},
-			{ 0x2153, 0x0000},
-			{ 0x40f3, 0x0000}
+		30666000, {
+			{ 0x00b3, 0x0000 },
+			{ 0x2153, 0x0000 },
+			{ 0x40f3, 0x0000 },
 		},
-	}, {
-		36000000, {
-			{ 0x00b3, 0x0000},
-			{ 0x2153, 0x0000},
-			{ 0x40f3, 0x0000}
+	},  {
+		36800000, {
+			{ 0x00b3, 0x0000 },
+			{ 0x2153, 0x0000 },
+			{ 0x40a2, 0x0001 },
 		},
-	}, {
-		40000000, {
-			{ 0x00b3, 0x0000},
-			{ 0x2153, 0x0000},
-			{ 0x40f3, 0x0000}
+	},  {
+		46000000, {
+			{ 0x00b3, 0x0000 },
+			{ 0x2142, 0x0001 },
+			{ 0x40a2, 0x0001 },
 		},
-	}, {
-		54000000, {
-			{ 0x0072, 0x0001},
-			{ 0x2142, 0x0001},
-			{ 0x40a2, 0x0001},
+	},  {
+		61333000, {
+			{ 0x0072, 0x0001 },
+			{ 0x2142, 0x0001 },
+			{ 0x40a2, 0x0001 },
 		},
-	}, {
-		65000000, {
-			{ 0x0072, 0x0001},
-			{ 0x2142, 0x0001},
-			{ 0x40a2, 0x0001},
+	},  {
+		73600000, {
+			{ 0x0072, 0x0001 },
+			{ 0x2142, 0x0001 },
+			{ 0x4061, 0x0002 },
 		},
-	}, {
-		66000000, {
-			{ 0x013e, 0x0003},
-			{ 0x217e, 0x0002},
-			{ 0x4061, 0x0002}
+	},  {
+		92000000, {
+			{ 0x0072, 0x0001 },
+			{ 0x2145, 0x0002 },
+			{ 0x4061, 0x0002 },
+		},
+	},  {
+		122666000, {
+			{ 0x0051, 0x0002 },
+			{ 0x2145, 0x0002 },
+			{ 0x4061, 0x0002 },
 		},
-	}, {
-		74250000, {
-			{ 0x0072, 0x0001},
-			{ 0x2145, 0x0002},
-			{ 0x4061, 0x0002}
+	},  {
+		147200000, {
+			{ 0x0051, 0x0002 },
+			{ 0x2145, 0x0002 },
+			{ 0x4064, 0x0003 },
 		},
-	}, {
-		83500000, {
-			{ 0x0072, 0x0001},
+	},  {
+		184000000, {
+			{ 0x0051, 0x0002 },
+			{ 0x214c, 0x0003 },
+			{ 0x4064, 0x0003 },
 		},
-	}, {
-		108000000, {
-			{ 0x0051, 0x0002},
-			{ 0x2145, 0x0002},
-			{ 0x4061, 0x0002}
+	},  {
+		226666000, {
+			{ 0x0040, 0x0003 },
+			{ 0x214c, 0x0003 },
+			{ 0x4064, 0x0003 },
 		},
-	}, {
-		106500000, {
-			{ 0x0051, 0x0002},
-			{ 0x2145, 0x0002},
-			{ 0x4061, 0x0002}
+	},  {
+		272000000, {
+			{ 0x0040, 0x0003 },
+			{ 0x214c, 0x0003 },
+			{ 0x5a64, 0x0003 },
 		},
-	}, {
-		146250000, {
-			{ 0x0051, 0x0002},
-			{ 0x2145, 0x0002},
-			{ 0x4061, 0x0002}
+	},  {
+		340000000, {
+			{ 0x0040, 0x0003 },
+			{ 0x3b4c, 0x0003 },
+			{ 0x5a64, 0x0003 },
 		},
-	}, {
-		148500000, {
-			{ 0x0051, 0x0003},
-			{ 0x214c, 0x0003},
-			{ 0x4064, 0x0003}
+	},  {
+		600000000, {
+			{ 0x1a40, 0x0003 },
+			{ 0x3b4c, 0x0003 },
+			{ 0x5a64, 0x0003 },
 		},
-	}, {
+	},  {
 		~0UL, {
-			{ 0x00a0, 0x000a },
-			{ 0x2001, 0x000f },
-			{ 0x4002, 0x000f },
+			{ 0x0000, 0x0000 },
+			{ 0x0000, 0x0000 },
+			{ 0x0000, 0x0000 },
 		},
 	}
 };
-- 
1.9.1

[PATCH v1 4/6] drm/rockchip: dw_hdmi: add RK3399 HDMI support

[Yakir Yang]

RK3399 and RK3288 shared the same HDMI IP controller, only some light
difference with GRF configure.

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
 .../devicetree/bindings/display/bridge/dw_hdmi.txt |  1 +
 .../bindings/display/rockchip/dw_hdmi-rockchip.txt |  3 +-
 drivers/gpu/drm/bridge/dw-hdmi.c                   |  2 +-
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        | 45 ++++++++++++++++++----
 include/drm/bridge/dw_hdmi.h                       |  6 +++
 5 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt b/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
index dc1452f..f50d4d5 100644
--- a/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
+++ b/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
@@ -6,6 +6,7 @@ Required properties:
    * "fsl,imx6q-hdmi"
    * "fsl,imx6dl-hdmi"
    * "rockchip,rk3288-dw-hdmi"
+   * "rockchip,rk3399-dw-hdmi"
 - reg: Physical base address and length of the controller's registers.
 - interrupts: The HDMI interrupt number
 - clocks, clock-names : must have the phandles to the HDMI iahb and isfr clocks,
diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
index 668091f..4e573d2 100644
--- a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
@@ -2,7 +2,8 @@ Rockchip specific extensions to the Synopsys Designware HDMI
 ================================
 
 Required properties:
-- compatible: "rockchip,rk3288-dw-hdmi";
+- compatible: "rockchip,rk3288-dw-hdmi",
+	      "rockchip,rk3399-dw-hdmi";
 - reg: Physical base address and length of the controller's registers.
 - clocks: phandle to hdmi iahb and isfr clocks.
 - clock-names: should be "iahb" "isfr"
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 70b1f7d..29f855a 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -833,7 +833,7 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, unsigned char prep,
 	dw_hdmi_phy_gen2_txpwron(hdmi, 1);
 	dw_hdmi_phy_gen2_pddq(hdmi, 0);
 
-	if (hdmi->dev_type == RK3288_HDMI)
+	if (is_rockchip(hdmi->dev_type))
 		dw_hdmi_phy_enable_spare(hdmi, 1);
 
 	/*Wait for PHY PLL lock */
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 8cb9ed2..329099b 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -21,13 +21,18 @@
 #include "rockchip_drm_drv.h"
 #include "rockchip_drm_vop.h"
 
-#define GRF_SOC_CON6                    0x025c
-#define HDMI_SEL_VOP_LIT                (1 << 4)
+#define RK3288_GRF_SOC_CON6		0x025C
+#define RK3288_HDMI_LCDC_SEL		BIT(4)
+#define RK3399_GRF_SOC_CON20		0x6250
+#define RK3399_HDMI_LCDC_SEL		BIT(6)
+
+#define HIWORD_UPDATE(val, mask)	(val | (mask) << 16)
 
 struct rockchip_hdmi {
 	struct device *dev;
 	struct regmap *regmap;
 	struct drm_encoder encoder;
+	enum dw_hdmi_devtype dev_type;
 };
 
 #define to_rockchip_hdmi(x)	container_of(x, struct rockchip_hdmi, x)
@@ -194,16 +199,30 @@ static void dw_hdmi_rockchip_encoder_mode_set(struct drm_encoder *encoder,
 static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder)
 {
 	struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder);
+	u32 lcdsel_grf_reg, lcdsel_mask;
 	u32 val;
 	int mux;
 
+	switch (hdmi->dev_type) {
+	case RK3288_HDMI:
+		lcdsel_grf_reg = RK3288_GRF_SOC_CON6;
+		lcdsel_mask = RK3288_HDMI_LCDC_SEL;
+		break;
+	case RK3399_HDMI:
+		lcdsel_grf_reg = RK3399_GRF_SOC_CON20;
+		lcdsel_mask = RK3399_HDMI_LCDC_SEL;
+		break;
+	default:
+		return;
+	};
+
 	mux = drm_of_encoder_active_endpoint_id(hdmi->dev->of_node, encoder);
 	if (mux)
-		val = HDMI_SEL_VOP_LIT | (HDMI_SEL_VOP_LIT << 16);
+		val = HIWORD_UPDATE(lcdsel_mask, lcdsel_mask);
 	else
-		val = HDMI_SEL_VOP_LIT << 16;
+		val = HIWORD_UPDATE(0, lcdsel_mask);
 
-	regmap_write(hdmi->regmap, GRF_SOC_CON6, val);
+	regmap_write(hdmi->regmap, lcdsel_grf_reg, val);
 	dev_dbg(hdmi->dev, "vop %s output to hdmi\n",
 		(mux) ? "LIT" : "BIG");
 }
@@ -229,7 +248,7 @@ static const struct drm_encoder_helper_funcs dw_hdmi_rockchip_encoder_helper_fun
 	.atomic_check = dw_hdmi_rockchip_encoder_atomic_check,
 };
 
-static const struct dw_hdmi_plat_data rockchip_hdmi_drv_data = {
+static const struct dw_hdmi_plat_data rk3288_hdmi_drv_data = {
 	.mode_valid = dw_hdmi_rockchip_mode_valid,
 	.mpll_cfg   = rockchip_mpll_cfg,
 	.cur_ctr    = rockchip_cur_ctr,
@@ -237,9 +256,20 @@ static const struct dw_hdmi_plat_data rockchip_hdmi_drv_data = {
 	.dev_type   = RK3288_HDMI,
 };
 
+static const struct dw_hdmi_plat_data rk3399_hdmi_drv_data = {
+	.mode_valid = dw_hdmi_rockchip_mode_valid,
+	.mpll_cfg   = rockchip_mpll_cfg,
+	.cur_ctr    = rockchip_cur_ctr,
+	.phy_config = rockchip_phy_config,
+	.dev_type   = RK3399_HDMI,
+};
+
 static const struct of_device_id dw_hdmi_rockchip_dt_ids[] = {
 	{ .compatible = "rockchip,rk3288-dw-hdmi",
-	  .data = &rockchip_hdmi_drv_data
+	  .data = &rk3288_hdmi_drv_data
+	},
+	{ .compatible = "rockchip,rk3399-dw-hdmi",
+	  .data = &rk3399_hdmi_drv_data
 	},
 	{},
 };
@@ -268,6 +298,7 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
 	match = of_match_node(dw_hdmi_rockchip_dt_ids, pdev->dev.of_node);
 	plat_data = match->data;
 	hdmi->dev = &pdev->dev;
+	hdmi->dev_type = plat_data->dev_type;
 	encoder = &hdmi->encoder;
 
 	irq = platform_get_irq(pdev, 0);
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index bae79f3..26c06d5 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -25,6 +25,7 @@ enum dw_hdmi_devtype {
 	IMX6Q_HDMI,
 	IMX6DL_HDMI,
 	RK3288_HDMI,
+	RK3399_HDMI,
 };
 
 struct dw_hdmi_mpll_config {
@@ -56,6 +57,11 @@ struct dw_hdmi_plat_data {
 					   struct drm_display_mode *mode);
 };
 
+static inline bool is_rockchip(enum dw_hdmi_devtype dev_type)
+{
+	return dev_type == RK3288_HDMI || dev_type == RK3399_HDMI;
+}
+
 void dw_hdmi_unbind(struct device *dev, struct device *master, void *data);
 int dw_hdmi_bind(struct device *dev, struct device *master,
 		 void *data, struct drm_encoder *encoder,
-- 
1.9.1

[PATCH v1 4/6] drm/rockchip: dw_hdmi: add RK3399 HDMI support

[Philipp Zabel]

Am Montag, den 11.07.2016, 19:05 +0800 schrieb Yakir Yang:
> RK3399 and RK3288 shared the same HDMI IP controller, only some light
> difference with GRF configure.
> 
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

[PATCH v1 4/6] drm/rockchip: dw_hdmi: add RK3399 HDMI support

[Yakir Yang]

Philipp,

On 07/11/2016 07:51 PM, Philipp Zabel wrote:
> Am Montag, den 11.07.2016, 19:05 +0800 schrieb Yakir Yang:
>> RK3399 and RK3288 shared the same HDMI IP controller, only some light
>> difference with GRF configure.
>>
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

Thanks for your fast respond :-D
- Yakir

> regards
> Philipp
>
>
>
>

[PATCH v1 4/6] drm/rockchip: dw_hdmi: add RK3399 HDMI support

[Rob Herring]

On Mon, Jul 11, 2016 at 07:05:46PM +0800, Yakir Yang wrote:
> RK3399 and RK3288 shared the same HDMI IP controller, only some light
> difference with GRF configure.
> 
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> ---
>  .../devicetree/bindings/display/bridge/dw_hdmi.txt |  1 +
>  .../bindings/display/rockchip/dw_hdmi-rockchip.txt |  3 +-

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/gpu/drm/bridge/dw-hdmi.c                   |  2 +-
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        | 45 ++++++++++++++++++----
>  include/drm/bridge/dw_hdmi.h                       |  6 +++
>  5 files changed, 48 insertions(+), 9 deletions(-)

[PATCH v1 5/6] drm/rockchip: dw_hdmi: introduce the VPLL clock setting

[Yakir Yang]

For RK3399 HDMI, there is an external clock need for HDMI PHY,
and it should keep the same clock rate with VOP DCLK.

VPLL have supported the clock for HDMI PHY, but there is no
clock divider bewteen VPLL and HDMI PHY. So we need to set the
VPLL rate manually in HDMI driver.

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
 .../bindings/display/rockchip/dw_hdmi-rockchip.txt |  3 ++-
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        | 25 +++++++++++++++++++++-
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
index 4e573d2..4e23ca4 100644
--- a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
@@ -17,7 +17,8 @@ Required properties:
 
 Optional properties
 - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
-- clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec"
+- clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec",
+		       phandle to the VPLL clock, name should be "vpll".
 
 Example:
 hdmi: hdmi at ff980000 {
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 329099b..701bb73 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -7,10 +7,12 @@
  * (at your option) any later version.
  */
 
+#include <linux/clk.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
-#include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
+
 #include <drm/drm_of.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
@@ -33,6 +35,7 @@ struct rockchip_hdmi {
 	struct regmap *regmap;
 	struct drm_encoder encoder;
 	enum dw_hdmi_devtype dev_type;
+	struct clk *vpll_clk;
 };
 
 #define to_rockchip_hdmi(x)	container_of(x, struct rockchip_hdmi, x)
@@ -145,6 +148,7 @@ static const struct dw_hdmi_phy_config rockchip_phy_config[] = {
 static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi)
 {
 	struct device_node *np = hdmi->dev->of_node;
+	int ret;
 
 	hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
 	if (IS_ERR(hdmi->regmap)) {
@@ -152,6 +156,22 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi)
 		return PTR_ERR(hdmi->regmap);
 	}
 
+	hdmi->vpll_clk = devm_clk_get(hdmi->dev, "vpll");
+	if (PTR_ERR(hdmi->vpll_clk) == -ENOENT) {
+		hdmi->vpll_clk = NULL;
+	} else if (PTR_ERR(hdmi->vpll_clk) == -EPROBE_DEFER) {
+		return -EPROBE_DEFER;
+	} else if (IS_ERR(hdmi->vpll_clk)) {
+		dev_err(hdmi->dev, "failed to get grf clock\n");
+		return PTR_ERR(hdmi->vpll_clk);
+	}
+
+	ret = clk_prepare_enable(hdmi->vpll_clk);
+	if (ret) {
+		dev_err(hdmi->dev, "Failed to enable HDMI vpll: %d\n", ret);
+		return ret;
+	}
+
 	return 0;
 }
 
@@ -194,6 +214,9 @@ static void dw_hdmi_rockchip_encoder_mode_set(struct drm_encoder *encoder,
 					      struct drm_display_mode *mode,
 					      struct drm_display_mode *adj_mode)
 {
+	struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder);
+
+	clk_set_rate(hdmi->vpll_clk, adj_mode->clock * 1000);
 }
 
 static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder)
-- 
1.9.1

[PATCH v1 5/6] drm/rockchip: dw_hdmi: introduce the VPLL clock setting

[Heiko Stübner]

Hi Yakir,

Am Montag, 11. Juli 2016, 19:05:49 schrieb Yakir Yang:
> For RK3399 HDMI, there is an external clock need for HDMI PHY,
> and it should keep the same clock rate with VOP DCLK.
> 
> VPLL have supported the clock for HDMI PHY, but there is no
> clock divider bewteen VPLL and HDMI PHY. So we need to set the
> VPLL rate manually in HDMI driver.

I don't think reserving the vpll for the hdmi at all times is the right way to 
go. 

While I do agree that reserving the VPLL (and NPLL on the rk3288) for graphics 
use looks like the right way, I think the core Rockchip drm driver should be 
responsible for managing it and also for deciding which output encoder gets to 
use it.

While true that on the Chromebook device-types the edp is static and hdmi 
needs the broad range of dynamic frequencies, this is not necessarily the case 
for all future device types and/or socs.

In the other thread we discussed adding that as rockchip,dclk-pll = <&...>; to 
the base display-subsystem node, Doug didn't manage to find time to respond yet 
though - and is on vacation right now.

I still believe that would be the best solution :-) .


That property could list 1 or even 2 plls, depending on the soc or board 
layout - maybe someone frees up the cpll in some special layout or something.
If none are listed, then the drm driver would need to cope with its available 
clocks.

Implementation-wise you could even still keep your shortcut until we encounter 
a device with different , let the drm use the pll for hdmi only until someone 
comes up with a better concept, but still the binding should be correct and 
versatile from the start.


Heiko

> ---
>  .../bindings/display/rockchip/dw_hdmi-rockchip.txt |  3 ++-
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        | 25
> +++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git
> a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
> b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
> index 4e573d2..4e23ca4 100644
> ---
> a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
> +++
> b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
> @@ -17,7 +17,8 @@ Required properties:
> 
>  Optional properties
>  - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
> -- clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec"
> +- clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec",
> +		       phandle to the VPLL clock, name should be "vpll".
> 
>  Example:
>  hdmi: hdmi at ff980000 {
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index 329099b..701bb73 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -7,10 +7,12 @@
>   * (at your option) any later version.
>   */
> 
> +#include <linux/clk.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> -#include <linux/mfd/syscon.h>
>  #include <linux/regmap.h>
> +
>  #include <drm/drm_of.h>
>  #include <drm/drmP.h>
>  #include <drm/drm_crtc_helper.h>
> @@ -33,6 +35,7 @@ struct rockchip_hdmi {
>  	struct regmap *regmap;
>  	struct drm_encoder encoder;
>  	enum dw_hdmi_devtype dev_type;
> +	struct clk *vpll_clk;
>  };
> 
>  #define to_rockchip_hdmi(x)	container_of(x, struct rockchip_hdmi, x)
> @@ -145,6 +148,7 @@ static const struct dw_hdmi_phy_config
> rockchip_phy_config[] = { static int rockchip_hdmi_parse_dt(struct
> rockchip_hdmi *hdmi)
>  {
>  	struct device_node *np = hdmi->dev->of_node;
> +	int ret;
> 
>  	hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
>  	if (IS_ERR(hdmi->regmap)) {
> @@ -152,6 +156,22 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi
> *hdmi) return PTR_ERR(hdmi->regmap);
>  	}
> 
> +	hdmi->vpll_clk = devm_clk_get(hdmi->dev, "vpll");
> +	if (PTR_ERR(hdmi->vpll_clk) == -ENOENT) {
> +		hdmi->vpll_clk = NULL;
> +	} else if (PTR_ERR(hdmi->vpll_clk) == -EPROBE_DEFER) {
> +		return -EPROBE_DEFER;
> +	} else if (IS_ERR(hdmi->vpll_clk)) {
> +		dev_err(hdmi->dev, "failed to get grf clock\n");
> +		return PTR_ERR(hdmi->vpll_clk);
> +	}
> +
> +	ret = clk_prepare_enable(hdmi->vpll_clk);
> +	if (ret) {
> +		dev_err(hdmi->dev, "Failed to enable HDMI vpll: %d\n", ret);
> +		return ret;
> +	}
> +
>  	return 0;
>  }
> 
> @@ -194,6 +214,9 @@ static void dw_hdmi_rockchip_encoder_mode_set(struct
> drm_encoder *encoder, struct drm_display_mode *mode,
>  					      struct drm_display_mode *adj_mode)
>  {
> +	struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder);
> +
> +	clk_set_rate(hdmi->vpll_clk, adj_mode->clock * 1000);
>  }
> 
>  static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder)

[PATCH v1 5/6] drm/rockchip: dw_hdmi: introduce the VPLL clock setting

[Yakir Yang]

Heiko,

On 07/12/2016 10:27 PM, Heiko St?bner wrote:
> Hi Yakir,
>
> Am Montag, 11. Juli 2016, 19:05:49 schrieb Yakir Yang:
>> For RK3399 HDMI, there is an external clock need for HDMI PHY,
>> and it should keep the same clock rate with VOP DCLK.
>>
>> VPLL have supported the clock for HDMI PHY, but there is no
>> clock divider bewteen VPLL and HDMI PHY. So we need to set the
>> VPLL rate manually in HDMI driver.
> I don't think reserving the vpll for the hdmi at all times is the right way to
> go.
>
> While I do agree that reserving the VPLL (and NPLL on the rk3288) for graphics
> use looks like the right way, I think the core Rockchip drm driver should be
> responsible for managing it and also for deciding which output encoder gets to
> use it.
> While true that on the Chromebook device-types the edp is static and hdmi
> needs the broad range of dynamic frequencies, this is not necessarily the case
> for all future device types and/or socs.
>
> In the other thread we discussed adding that as rockchip,dclk-pll = <&...>; to
> the base display-subsystem node, Doug didn't manage to find time to respond yet
> though - and is on vacation right now.

Great idea. Let a separate drm-pll driver to maintain all connector's 
clock requirement.

> I still believe that would be the best solution :-) .
>
>
> That property could list 1 or even 2 plls, depending on the soc or board
> layout - maybe someone frees up the cpll in some special layout or something.
> If none are listed, then the drm driver would need to cope with its available
> clocks.
>
> Implementation-wise you could even still keep your shortcut until we encounter
> a device with different , let the drm use the pll for hdmi only until someone
> comes up with a better concept, but still the binding should be correct and
> versatile from the start.

Someone should start to prepare the drm core pll driver, it's great idea.

BR,
- Yakir

>
> Heiko
>
>> ---
>>   .../bindings/display/rockchip/dw_hdmi-rockchip.txt |  3 ++-
>>   drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        | 25
>> +++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
>> b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
>> index 4e573d2..4e23ca4 100644
>> ---
>> a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
>> +++
>> b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
>> @@ -17,7 +17,8 @@ Required properties:
>>
>>   Optional properties
>>   - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
>> -- clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec"
>> +- clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec",
>> +		       phandle to the VPLL clock, name should be "vpll".
>>
>>   Example:
>>   hdmi: hdmi at ff980000 {
>> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>> b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index 329099b..701bb73 100644
>> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>> @@ -7,10 +7,12 @@
>>    * (at your option) any later version.
>>    */
>>
>> +#include <linux/clk.h>
>> +#include <linux/mfd/syscon.h>
>>   #include <linux/module.h>
>>   #include <linux/platform_device.h>
>> -#include <linux/mfd/syscon.h>
>>   #include <linux/regmap.h>
>> +
>>   #include <drm/drm_of.h>
>>   #include <drm/drmP.h>
>>   #include <drm/drm_crtc_helper.h>
>> @@ -33,6 +35,7 @@ struct rockchip_hdmi {
>>   	struct regmap *regmap;
>>   	struct drm_encoder encoder;
>>   	enum dw_hdmi_devtype dev_type;
>> +	struct clk *vpll_clk;
>>   };
>>
>>   #define to_rockchip_hdmi(x)	container_of(x, struct rockchip_hdmi, x)
>> @@ -145,6 +148,7 @@ static const struct dw_hdmi_phy_config
>> rockchip_phy_config[] = { static int rockchip_hdmi_parse_dt(struct
>> rockchip_hdmi *hdmi)
>>   {
>>   	struct device_node *np = hdmi->dev->of_node;
>> +	int ret;
>>
>>   	hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
>>   	if (IS_ERR(hdmi->regmap)) {
>> @@ -152,6 +156,22 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi
>> *hdmi) return PTR_ERR(hdmi->regmap);
>>   	}
>>
>> +	hdmi->vpll_clk = devm_clk_get(hdmi->dev, "vpll");
>> +	if (PTR_ERR(hdmi->vpll_clk) == -ENOENT) {
>> +		hdmi->vpll_clk = NULL;
>> +	} else if (PTR_ERR(hdmi->vpll_clk) == -EPROBE_DEFER) {
>> +		return -EPROBE_DEFER;
>> +	} else if (IS_ERR(hdmi->vpll_clk)) {
>> +		dev_err(hdmi->dev, "failed to get grf clock\n");
>> +		return PTR_ERR(hdmi->vpll_clk);
>> +	}
>> +
>> +	ret = clk_prepare_enable(hdmi->vpll_clk);
>> +	if (ret) {
>> +		dev_err(hdmi->dev, "Failed to enable HDMI vpll: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>>   	return 0;
>>   }
>>
>> @@ -194,6 +214,9 @@ static void dw_hdmi_rockchip_encoder_mode_set(struct
>> drm_encoder *encoder, struct drm_display_mode *mode,
>>   					      struct drm_display_mode *adj_mode)
>>   {
>> +	struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder);
>> +
>> +	clk_set_rate(hdmi->vpll_clk, adj_mode->clock * 1000);
>>   }
>>
>>   static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder)
>
>
>

[PATCH v1 6/6] drm/rockchip: dw_hdmi: introduce the pclk for grf

[Yakir Yang]

For RK3399's GRF module, if we want to operate the graphic related grf
registers, we need to enable the pclk_vio_grf which supply power for VIO
GRF IOs, so it's better to introduce an optional grf clock in driver.

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
 .../bindings/display/rockchip/dw_hdmi-rockchip.txt   |  3 ++-
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c          | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
index 4e23ca4..e22d70f 100644
--- a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
@@ -18,7 +18,8 @@ Required properties:
 Optional properties
 - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
 - clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec",
-		       phandle to the VPLL clock, name should be "vpll".
+		       phandle to the VPLL clock, name should be "vpll",
+		       phandle to the GRF clock, name should be "grf".
 
 Example:
 hdmi: hdmi at ff980000 {
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 701bb73..69e6efb 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -36,6 +36,7 @@ struct rockchip_hdmi {
 	struct drm_encoder encoder;
 	enum dw_hdmi_devtype dev_type;
 	struct clk *vpll_clk;
+	struct clk *grf_clk;
 };
 
 #define to_rockchip_hdmi(x)	container_of(x, struct rockchip_hdmi, x)
@@ -166,6 +167,16 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi)
 		return PTR_ERR(hdmi->vpll_clk);
 	}
 
+	hdmi->grf_clk = devm_clk_get(hdmi->dev, "grf");
+	if (PTR_ERR(hdmi->grf_clk) == -ENOENT) {
+		hdmi->grf_clk = NULL;
+	} else if (PTR_ERR(hdmi->grf_clk) == -EPROBE_DEFER) {
+		return -EPROBE_DEFER;
+	} else if (IS_ERR(hdmi->grf_clk)) {
+		dev_err(hdmi->dev, "failed to get grf clock\n");
+		return PTR_ERR(hdmi->grf_clk);
+	}
+
 	ret = clk_prepare_enable(hdmi->vpll_clk);
 	if (ret) {
 		dev_err(hdmi->dev, "Failed to enable HDMI vpll: %d\n", ret);
@@ -225,6 +236,7 @@ static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder)
 	u32 lcdsel_grf_reg, lcdsel_mask;
 	u32 val;
 	int mux;
+	int ret;
 
 	switch (hdmi->dev_type) {
 	case RK3288_HDMI:
@@ -245,9 +257,17 @@ static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder)
 	else
 		val = HIWORD_UPDATE(0, lcdsel_mask);
 
+	ret = clk_prepare_enable(hdmi->grf_clk);
+	if (ret < 0) {
+		dev_err(hdmi->dev, "failed to enable grfclk %d\n", ret);
+		return;
+	}
+
 	regmap_write(hdmi->regmap, lcdsel_grf_reg, val);
 	dev_dbg(hdmi->dev, "vop %s output to hdmi\n",
 		(mux) ? "LIT" : "BIG");
+
+	clk_disable_unprepare(hdmi->grf_clk);
 }
 
 static int
-- 
1.9.1