[PATCH v3 0/3] Add RK3399 HDMI Support

[PATCH v3 0/3] Add RK3399 HDMI Support

[Mark Yao]

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

base on Yakir's v1 thread:
   http://lkml.iu.edu/hypermail/linux/kernel/1607.1/01468.html

rely on Jose's hdmi phy patch:
   https://patchwork.kernel.org/patch/9702229

Some Yakir's hdmi patches cause hdmi no display on my test,
so I remove them on this thread.

Tested on rk3399 evb board with kernel 4.12.0-rc1.

Changes in v3:
  remove hdmi_phy_configure_dwc_hdmi_3d_tx callbak.

Changes in v2:
  rebase to newest uptream, reuse hdmi_phy_configure_dwc_hdmi_3d_tx

Mark Yao (3):
  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

 .../bindings/display/rockchip/dw_hdmi-rockchip.txt |   6 +-
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        | 110 ++++++++++++++++++---
 2 files changed, 102 insertions(+), 14 deletions(-)

-- 
1.9.1

[PATCH v3 1/3] drm/rockchip: dw_hdmi: add RK3399 HDMI support

[Mark Yao]

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>
Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
Changes in v3:
  remove hdmi_phy_configure_dwc_hdmi_3d_tx callbak.

Changes in v2:
  reuse hdmi_phy_configure_dwc_hdmi_3d_tx for phy configure
  fixup Documentation

 .../bindings/display/rockchip/dw_hdmi-rockchip.txt |  3 +-
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        | 67 ++++++++++++++++++----
 2 files changed, 58 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
index 046076c..495bcf5 100644
--- a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
@@ -11,7 +11,8 @@ following device-specific properties.
 
 Required properties:
 
-- compatible: Shall contain "rockchip,rk3288-dw-hdmi".
+- compatible: "rockchip,rk3288-dw-hdmi",
+              "rockchip,rk3399-dw-hdmi";
 - reg: See dw_hdmi.txt.
 - reg-io-width: See dw_hdmi.txt. Shall be 4.
 - interrupts: HDMI interrupt number
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 63dab6f..90aaaf4 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -20,13 +20,30 @@
 #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_chip_data - splite the grf setting of kind of chips
+ * @lcdsel_grf_reg: grf register offset of lcdc select
+ * @lcdsel_big: reg value of selecting vop big for HDMI
+ * @lcdsel_lit: reg value of selecting vop little for HDMI
+ */
+struct rockchip_hdmi_chip_data {
+	u32	lcdsel_grf_reg;
+	u32	lcdsel_big;
+	u32	lcdsel_lit;
+};
 
 struct rockchip_hdmi {
 	struct device *dev;
 	struct regmap *regmap;
 	struct drm_encoder encoder;
+	const struct rockchip_hdmi_chip_data *chip_data;
 };
 
 #define to_rockchip_hdmi(x)	container_of(x, struct rockchip_hdmi, x)
@@ -198,17 +215,20 @@ static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder)
 {
 	struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder);
 	u32 val;
-	int mux;
+	int ret;
 
-	mux = drm_of_encoder_active_endpoint_id(hdmi->dev->of_node, encoder);
-	if (mux)
-		val = HDMI_SEL_VOP_LIT | (HDMI_SEL_VOP_LIT << 16);
+	ret = drm_of_encoder_active_endpoint_id(hdmi->dev->of_node, encoder);
+	if (ret)
+		val = hdmi->chip_data->lcdsel_lit;
 	else
-		val = HDMI_SEL_VOP_LIT << 16;
+		val = hdmi->chip_data->lcdsel_big;
+
+	ret = regmap_write(hdmi->regmap, hdmi->chip_data->lcdsel_grf_reg, val);
+	if (ret != 0)
+		dev_err(hdmi->dev, "Could not write to GRF: %d\n", ret);
 
-	regmap_write(hdmi->regmap, GRF_SOC_CON6, val);
 	dev_dbg(hdmi->dev, "vop %s output to hdmi\n",
-		(mux) ? "LIT" : "BIG");
+		ret ? "LIT" : "BIG");
 }
 
 static int
@@ -232,16 +252,40 @@ static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder)
 	.atomic_check = dw_hdmi_rockchip_encoder_atomic_check,
 };
 
-static const struct dw_hdmi_plat_data rockchip_hdmi_drv_data = {
+static struct rockchip_hdmi_chip_data rk3288_chip_data = {
+	.lcdsel_grf_reg = RK3288_GRF_SOC_CON6,
+	.lcdsel_big = HIWORD_UPDATE(0, RK3288_HDMI_LCDC_SEL),
+	.lcdsel_lit = HIWORD_UPDATE(RK3288_HDMI_LCDC_SEL, RK3288_HDMI_LCDC_SEL),
+};
+
+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,
+	.phy_config = rockchip_phy_config,
+	.phy_data = &rk3288_chip_data,
+};
+
+static struct rockchip_hdmi_chip_data rk3399_chip_data = {
+	.lcdsel_grf_reg = RK3399_GRF_SOC_CON20,
+	.lcdsel_big = HIWORD_UPDATE(0, RK3399_HDMI_LCDC_SEL),
+	.lcdsel_lit = HIWORD_UPDATE(RK3399_HDMI_LCDC_SEL, RK3399_HDMI_LCDC_SEL),
+};
+
+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,
+	.phy_data = &rk3399_chip_data,
 };
 
 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 +312,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->chip_data = plat_data->phy_data;
 	encoder = &hdmi->encoder;
 
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
-- 
1.9.1

Re: [PATCH v3 1/3] drm/rockchip: dw_hdmi: add RK3399 HDMI support

[Rob Herring]

On Fri, Jun 09, 2017 at 03:10:36PM +0800, Mark Yao 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>
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
> Changes in v3:
>   remove hdmi_phy_configure_dwc_hdmi_3d_tx callbak.
> 
> Changes in v2:
>   reuse hdmi_phy_configure_dwc_hdmi_3d_tx for phy configure
>   fixup Documentation
> 
>  .../bindings/display/rockchip/dw_hdmi-rockchip.txt |  3 +-
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        | 67 ++++++++++++++++++----
>  2 files changed, 58 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
> index 046076c..495bcf5 100644
> --- a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
> +++ b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
> @@ -11,7 +11,8 @@ following device-specific properties.
>  
>  Required properties:
>  
> -- compatible: Shall contain "rockchip,rk3288-dw-hdmi".
> +- compatible: "rockchip,rk3288-dw-hdmi",
> +              "rockchip,rk3399-dw-hdmi";

Is this supposed to be "one of" or both? Please format it as 1 valid 
combination per line. Drop the ',' and the ';' because I see that as dts 
syntax.

>  - reg: See dw_hdmi.txt.
>  - reg-io-width: See dw_hdmi.txt. Shall be 4.
>  - interrupts: HDMI interrupt number

[PATCH v3.1 1/3] drm/rockchip: dw_hdmi: add RK3399 HDMI support

[Mark Yao]

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>
Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
Changes in v3.1:
  Correct documentation compatible's format(Rob Herring).
Changes in v3:
  remove hdmi_phy_configure_dwc_hdmi_3d_tx callbak.

Changes in v2:
  reuse hdmi_phy_configure_dwc_hdmi_3d_tx for phy configure
  fixup Documentation

 .../bindings/display/rockchip/dw_hdmi-rockchip.txt |  4 +-
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        | 67 ++++++++++++++++++----
 2 files changed, 59 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
index 046076c..7039a15 100644
--- a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
@@ -11,7 +11,9 @@ following device-specific properties.
 
 Required properties:
 
-- compatible: Shall contain "rockchip,rk3288-dw-hdmi".
+- compatible: should be one of the following:
+		"rockchip,rk3288-dw-hdmi"
+		"rockchip,rk3399-dw-hdmi"
 - reg: See dw_hdmi.txt.
 - reg-io-width: See dw_hdmi.txt. Shall be 4.
 - interrupts: HDMI interrupt number
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 63dab6f..90aaaf4 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -20,13 +20,30 @@
 #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_chip_data - splite the grf setting of kind of chips
+ * @lcdsel_grf_reg: grf register offset of lcdc select
+ * @lcdsel_big: reg value of selecting vop big for HDMI
+ * @lcdsel_lit: reg value of selecting vop little for HDMI
+ */
+struct rockchip_hdmi_chip_data {
+	u32	lcdsel_grf_reg;
+	u32	lcdsel_big;
+	u32	lcdsel_lit;
+};
 
 struct rockchip_hdmi {
 	struct device *dev;
 	struct regmap *regmap;
 	struct drm_encoder encoder;
+	const struct rockchip_hdmi_chip_data *chip_data;
 };
 
 #define to_rockchip_hdmi(x)	container_of(x, struct rockchip_hdmi, x)
@@ -198,17 +215,20 @@ static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder)
 {
 	struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder);
 	u32 val;
-	int mux;
+	int ret;
 
-	mux = drm_of_encoder_active_endpoint_id(hdmi->dev->of_node, encoder);
-	if (mux)
-		val = HDMI_SEL_VOP_LIT | (HDMI_SEL_VOP_LIT << 16);
+	ret = drm_of_encoder_active_endpoint_id(hdmi->dev->of_node, encoder);
+	if (ret)
+		val = hdmi->chip_data->lcdsel_lit;
 	else
-		val = HDMI_SEL_VOP_LIT << 16;
+		val = hdmi->chip_data->lcdsel_big;
+
+	ret = regmap_write(hdmi->regmap, hdmi->chip_data->lcdsel_grf_reg, val);
+	if (ret != 0)
+		dev_err(hdmi->dev, "Could not write to GRF: %d\n", ret);
 
-	regmap_write(hdmi->regmap, GRF_SOC_CON6, val);
 	dev_dbg(hdmi->dev, "vop %s output to hdmi\n",
-		(mux) ? "LIT" : "BIG");
+		ret ? "LIT" : "BIG");
 }
 
 static int
@@ -232,16 +252,40 @@ static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder)
 	.atomic_check = dw_hdmi_rockchip_encoder_atomic_check,
 };
 
-static const struct dw_hdmi_plat_data rockchip_hdmi_drv_data = {
+static struct rockchip_hdmi_chip_data rk3288_chip_data = {
+	.lcdsel_grf_reg = RK3288_GRF_SOC_CON6,
+	.lcdsel_big = HIWORD_UPDATE(0, RK3288_HDMI_LCDC_SEL),
+	.lcdsel_lit = HIWORD_UPDATE(RK3288_HDMI_LCDC_SEL, RK3288_HDMI_LCDC_SEL),
+};
+
+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,
+	.phy_config = rockchip_phy_config,
+	.phy_data = &rk3288_chip_data,
+};
+
+static struct rockchip_hdmi_chip_data rk3399_chip_data = {
+	.lcdsel_grf_reg = RK3399_GRF_SOC_CON20,
+	.lcdsel_big = HIWORD_UPDATE(0, RK3399_HDMI_LCDC_SEL),
+	.lcdsel_lit = HIWORD_UPDATE(RK3399_HDMI_LCDC_SEL, RK3399_HDMI_LCDC_SEL),
+};
+
+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,
+	.phy_data = &rk3399_chip_data,
 };
 
 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 +312,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->chip_data = plat_data->phy_data;
 	encoder = &hdmi->encoder;
 
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
-- 
1.9.1

Re: [PATCH v3.1 1/3] drm/rockchip: dw_hdmi: add RK3399 HDMI support

[Heiko Stuebner]

Hi Mark,

Am Donnerstag, 22. Juni 2017, 15:17:24 CEST schrieb Mark Yao:
> 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>
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
> Changes in v3.1:
>   Correct documentation compatible's format(Rob Herring).
> Changes in v3:
>   remove hdmi_phy_configure_dwc_hdmi_3d_tx callbak.
> 
> Changes in v2:
>   reuse hdmi_phy_configure_dwc_hdmi_3d_tx for phy configure
>   fixup Documentation
> 
>  .../bindings/display/rockchip/dw_hdmi-rockchip.txt |  4 +-
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        | 67 ++++++++++++++++++----
>  2 files changed, 59 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
> index 046076c..7039a15 100644
> --- a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
> +++ b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
> @@ -11,7 +11,9 @@ following device-specific properties.
>  
>  Required properties:
>  
> -- compatible: Shall contain "rockchip,rk3288-dw-hdmi".
> +- compatible: should be one of the following:
> +		"rockchip,rk3288-dw-hdmi"
> +		"rockchip,rk3399-dw-hdmi"
>  - reg: See dw_hdmi.txt.
>  - reg-io-width: See dw_hdmi.txt. Shall be 4.
>  - interrupts: HDMI interrupt number
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index 63dab6f..90aaaf4 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -20,13 +20,30 @@
>  #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_chip_data - splite the grf setting of kind of chips
> + * @lcdsel_grf_reg: grf register offset of lcdc select
> + * @lcdsel_big: reg value of selecting vop big for HDMI
> + * @lcdsel_lit: reg value of selecting vop little for HDMI
> + */
> +struct rockchip_hdmi_chip_data {
> +	u32	lcdsel_grf_reg;

How do you plan on handling the rk3368 (with only one VOP and thus
no selection happening)? I'd just make the above an int, so we could
set it to -1 for that case. (value 0 is after all a valid reg).


Heiko


> +	u32	lcdsel_big;
> +	u32	lcdsel_lit;
> +};
>  
>  struct rockchip_hdmi {
>  	struct device *dev;
>  	struct regmap *regmap;
>  	struct drm_encoder encoder;
> +	const struct rockchip_hdmi_chip_data *chip_data;
>  };
>  
>  #define to_rockchip_hdmi(x)	container_of(x, struct rockchip_hdmi, x)
> @@ -198,17 +215,20 @@ static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder)
>  {
>  	struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder);
>  	u32 val;
> -	int mux;
> +	int ret;
>  
> -	mux = drm_of_encoder_active_endpoint_id(hdmi->dev->of_node, encoder);
> -	if (mux)
> -		val = HDMI_SEL_VOP_LIT | (HDMI_SEL_VOP_LIT << 16);
> +	ret = drm_of_encoder_active_endpoint_id(hdmi->dev->of_node, encoder);
> +	if (ret)
> +		val = hdmi->chip_data->lcdsel_lit;
>  	else
> -		val = HDMI_SEL_VOP_LIT << 16;
> +		val = hdmi->chip_data->lcdsel_big;
> +
> +	ret = regmap_write(hdmi->regmap, hdmi->chip_data->lcdsel_grf_reg, val);
> +	if (ret != 0)
> +		dev_err(hdmi->dev, "Could not write to GRF: %d\n", ret);
>  
> -	regmap_write(hdmi->regmap, GRF_SOC_CON6, val);
>  	dev_dbg(hdmi->dev, "vop %s output to hdmi\n",
> -		(mux) ? "LIT" : "BIG");
> +		ret ? "LIT" : "BIG");
>  }
>  
>  static int
> @@ -232,16 +252,40 @@ static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder)
>  	.atomic_check = dw_hdmi_rockchip_encoder_atomic_check,
>  };
>  
> -static const struct dw_hdmi_plat_data rockchip_hdmi_drv_data = {
> +static struct rockchip_hdmi_chip_data rk3288_chip_data = {
> +	.lcdsel_grf_reg = RK3288_GRF_SOC_CON6,
> +	.lcdsel_big = HIWORD_UPDATE(0, RK3288_HDMI_LCDC_SEL),
> +	.lcdsel_lit = HIWORD_UPDATE(RK3288_HDMI_LCDC_SEL, RK3288_HDMI_LCDC_SEL),
> +};
> +
> +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,
> +	.phy_config = rockchip_phy_config,
> +	.phy_data = &rk3288_chip_data,
> +};
> +
> +static struct rockchip_hdmi_chip_data rk3399_chip_data = {
> +	.lcdsel_grf_reg = RK3399_GRF_SOC_CON20,
> +	.lcdsel_big = HIWORD_UPDATE(0, RK3399_HDMI_LCDC_SEL),
> +	.lcdsel_lit = HIWORD_UPDATE(RK3399_HDMI_LCDC_SEL, RK3399_HDMI_LCDC_SEL),
> +};
> +
> +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,
> +	.phy_data = &rk3399_chip_data,
>  };
>  
>  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 +312,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->chip_data = plat_data->phy_data;
>  	encoder = &hdmi->encoder;
>  
>  	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
> 

Re: [PATCH v3.1 1/3] drm/rockchip: dw_hdmi: add RK3399 HDMI support

[Mark yao]

On 2017年06月22日 15:31, Heiko Stuebner wrote:
>> +
>> >+/**
>> >+ * struct rockchip_hdmi_chip_data - splite the grf setting of kind of chips
>> >+ * @lcdsel_grf_reg: grf register offset of lcdc select
>> >+ * @lcdsel_big: reg value of selecting vop big for HDMI
>> >+ * @lcdsel_lit: reg value of selecting vop little for HDMI
>> >+ */
>> >+struct rockchip_hdmi_chip_data {
>> >+	u32	lcdsel_grf_reg;
> How do you plan on handling the rk3368 (with only one VOP and thus
> no selection happening)? I'd just make the above an int, so we could
> set it to -1 for that case. (value 0 is after all a valid reg).

It's a problem handling on rk3368, using -1 to judge means that we need
initial the lcdsel_grf_reg to -1 on rk3368 platform, we need always add a platform
data to handle it, seems not good enough.

Since the hdmi chip data only use for vop selection, maybe we can judge with
checking hdmi->chip_data == NULL for the case.

Mark.

>
> Heiko
>
>

-- 
Ｍark Yao

Re: [PATCH v3.1 1/3] drm/rockchip: dw_hdmi: add RK3399 HDMI support

[Heiko Stuebner]

Am Donnerstag, 22. Juni 2017, 16:02:44 CEST schrieb Mark yao:
> On 2017年06月22日 15:31, Heiko Stuebner wrote:
> >> +
> >> >+/**
> >> >+ * struct rockchip_hdmi_chip_data - splite the grf setting of kind of chips
> >> >+ * @lcdsel_grf_reg: grf register offset of lcdc select
> >> >+ * @lcdsel_big: reg value of selecting vop big for HDMI
> >> >+ * @lcdsel_lit: reg value of selecting vop little for HDMI
> >> >+ */
> >> >+struct rockchip_hdmi_chip_data {
> >> >+	u32	lcdsel_grf_reg;
> > How do you plan on handling the rk3368 (with only one VOP and thus
> > no selection happening)? I'd just make the above an int, so we could
> > set it to -1 for that case. (value 0 is after all a valid reg).
> 
> It's a problem handling on rk3368, using -1 to judge means that we need
> initial the lcdsel_grf_reg to -1 on rk3368 platform, we need always add a platform
> data to handle it, seems not good enough.
> 
> Since the hdmi chip data only use for vop selection, maybe we can judge with
> checking hdmi->chip_data == NULL for the case.

yeah, that could work as well ... as long as the grf-vop-select is the only chip-data

Re: [PATCH v3.1 1/3] drm/rockchip: dw_hdmi: add RK3399 HDMI support

[Rob Herring]

On Thu, Jun 22, 2017 at 2:17 AM, Mark Yao <mark.yao@rock-chips.com> 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>
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
> Changes in v3.1:
>   Correct documentation compatible's format(Rob Herring).
> Changes in v3:
>   remove hdmi_phy_configure_dwc_hdmi_3d_tx callbak.
>
> Changes in v2:
>   reuse hdmi_phy_configure_dwc_hdmi_3d_tx for phy configure
>   fixup Documentation
>
>  .../bindings/display/rockchip/dw_hdmi-rockchip.txt |  4 +-

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

>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        | 67 ++++++++++++++++++----
>  2 files changed, 59 insertions(+), 12 deletions(-)

[PATCH v3 2/3] drm/rockchip: dw_hdmi: introduce the VPLL clock setting

[Mark Yao]

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>
Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
Changes in v3: none
Changes in v2: describe vpll on Documentation.

 .../bindings/display/rockchip/dw_hdmi-rockchip.txt |  2 +-
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        | 25 +++++++++++++++++++++-
 2 files changed, 25 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 495bcf5..779e8ac 100644
--- a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
@@ -31,7 +31,7 @@ Optional properties
   I2C master controller.
 - clock-names: See dw_hdmi.txt. The "cec" clock is optional.
 - clock-names: May contain "cec" as defined in dw_hdmi.txt.
-
+- clock-names: May contain "vpll", external clock for some hdmi phy.
 
 Example:
 
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 90aaaf4..aaa0f3e 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>
@@ -44,6 +46,7 @@ struct rockchip_hdmi {
 	struct regmap *regmap;
 	struct drm_encoder encoder;
 	const struct rockchip_hdmi_chip_data *chip_data;
+	struct clk *vpll_clk;
 };
 
 #define to_rockchip_hdmi(x)	container_of(x, struct rockchip_hdmi, x)
@@ -160,6 +163,7 @@ struct rockchip_hdmi {
 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)) {
@@ -167,6 +171,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;
 }
 
@@ -209,6 +229,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

Re: [PATCH v3 2/3] drm/rockchip: dw_hdmi: introduce the VPLL clock setting

[Rob Herring]

On Fri, Jun 09, 2017 at 03:10:41PM +0800, Mark Yao wrote:
> 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>
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
> Changes in v3: none
> Changes in v2: describe vpll on Documentation.
> 
>  .../bindings/display/rockchip/dw_hdmi-rockchip.txt |  2 +-

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

>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        | 25 +++++++++++++++++++++-
>  2 files changed, 25 insertions(+), 2 deletions(-)

[PATCH v3 3/3] drm/rockchip: dw_hdmi: introduce the pclk for grf

[Mark Yao]

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>
Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
Changes in v3: none
Changes in v2: describe grf on Documentation.

 .../bindings/display/rockchip/dw_hdmi-rockchip.txt     |  1 +
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c            | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
index 779e8ac..6bce639 100644
--- a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
@@ -31,6 +31,7 @@ Optional properties
   I2C master controller.
 - clock-names: See dw_hdmi.txt. The "cec" clock is optional.
 - clock-names: May contain "cec" as defined in dw_hdmi.txt.
+- clock-names: May contain "grf", power for grf io.
 - clock-names: May contain "vpll", external clock for some hdmi phy.
 
 Example:
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index aaa0f3e..2b9cb47 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -47,6 +47,7 @@ struct rockchip_hdmi {
 	struct drm_encoder encoder;
 	const struct rockchip_hdmi_chip_data *chip_data;
 	struct clk *vpll_clk;
+	struct clk *grf_clk;
 };
 
 #define to_rockchip_hdmi(x)	container_of(x, struct rockchip_hdmi, x)
@@ -181,6 +182,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);
@@ -246,10 +257,17 @@ static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder)
 	else
 		val = hdmi->chip_data->lcdsel_big;
 
+	ret = clk_prepare_enable(hdmi->grf_clk);
+	if (ret < 0) {
+		dev_err(hdmi->dev, "failed to enable grfclk %d\n", ret);
+		return;
+	}
+
 	ret = regmap_write(hdmi->regmap, hdmi->chip_data->lcdsel_grf_reg, val);
 	if (ret != 0)
 		dev_err(hdmi->dev, "Could not write to GRF: %d\n", ret);
 
+	clk_disable_unprepare(hdmi->grf_clk);
 	dev_dbg(hdmi->dev, "vop %s output to hdmi\n",
 		ret ? "LIT" : "BIG");
 }
-- 
1.9.1

Re: [PATCH v3 3/3] drm/rockchip: dw_hdmi: introduce the pclk for grf

[Rob Herring]

On Fri, Jun 09, 2017 at 03:10:46PM +0800, Mark Yao wrote:
> 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>
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
> Changes in v3: none
> Changes in v2: describe grf on Documentation.
> 
>  .../bindings/display/rockchip/dw_hdmi-rockchip.txt     |  1 +
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c            | 18 ++++++++++++++++++
>  2 files changed, 19 insertions(+)

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

Re: [PATCH v3 0/3] Add RK3399 HDMI Support

[Mark yao]

Thanks for Heiko's review and Rob's Ack.

Seems no more confuse, pushed to drm-misc-next.

Thanks.

On 2017年06月09日 15:10, Mark Yao wrote:
> RK3399 and RK3288 shared the same HDMI IP controller, only some
> light difference with GRF configure, and an external VPLL clock
> need to configure.
>
> base on Yakir's v1 thread:
>     http://lkml.iu.edu/hypermail/linux/kernel/1607.1/01468.html
>
> rely on Jose's hdmi phy patch:
>     https://patchwork.kernel.org/patch/9702229
>
> Some Yakir's hdmi patches cause hdmi no display on my test,
> so I remove them on this thread.
>
> Tested on rk3399 evb board with kernel 4.12.0-rc1.
>
> Changes in v3:
>    remove hdmi_phy_configure_dwc_hdmi_3d_tx callbak.
>
> Changes in v2:
>    rebase to newest uptream, reuse hdmi_phy_configure_dwc_hdmi_3d_tx
>
> Mark Yao (3):
>    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
>
>   .../bindings/display/rockchip/dw_hdmi-rockchip.txt |   6 +-
>   drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        | 110 ++++++++++++++++++---
>   2 files changed, 102 insertions(+), 14 deletions(-)
>


-- 
Ｍark Yao