[PATCH v2 1/7] drm/rockchip: dw_hdmi: update dw_hdmi_rockchip_dt_ids

[PATCH v2 1/7] drm/rockchip: dw_hdmi: update dw_hdmi_rockchip_dt_ids

[Algea Cao]

Add rk3328-dw-hdmi to support rk3328.

Signed-off-by: Algea Cao <algea.cao@rock-chips.com>
---
 .../devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt       | 1 +
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c                         | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
index adc94fc..8b9649c 100644
--- a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
@@ -14,6 +14,7 @@ Required properties:
 - compatible: should be one of the following:
 		"rockchip,rk3288-dw-hdmi"
 		"rockchip,rk3399-dw-hdmi"
+		"rockchip,rk3328-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 ccd5d59..719452d 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -313,6 +313,9 @@ static struct rockchip_hdmi_chip_data rk3399_chip_data = {
 	.lcdsel_lit = HIWORD_UPDATE(RK3399_HDMI_LCDC_SEL, RK3399_HDMI_LCDC_SEL),
 };
 
+static const struct dw_hdmi_plat_data rk3328_hdmi_drv_data = {
+	.mode_valid = dw_hdmi_rockchip_mode_valid,
+};
 static const struct dw_hdmi_plat_data rk3399_hdmi_drv_data = {
 	.mode_valid = dw_hdmi_rockchip_mode_valid,
 	.mpll_cfg   = rockchip_mpll_cfg,
@@ -328,6 +331,9 @@ static const struct of_device_id dw_hdmi_rockchip_dt_ids[] = {
 	{ .compatible = "rockchip,rk3399-dw-hdmi",
 	  .data = &rk3399_hdmi_drv_data
 	},
+	{ .compatible = "rockchip,rk3328-dw-hdmi",
+	  .data = &rk3328_hdmi_drv_data
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, dw_hdmi_rockchip_dt_ids);
-- 
2.7.4

[PATCH v2 2/7] drm/rockchip: dw_hdmi: add device type

[Algea Cao]

To determine type of SOC, we add a parameter dev_type.

Signed-off-by: Algea Cao <algea.cao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c |  5 +++++
 include/drm/bridge/dw_hdmi.h                | 10 ++++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 719452d..09c77f9 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -46,6 +46,7 @@ struct rockchip_hdmi {
 	struct regmap *regmap;
 	struct drm_encoder encoder;
 	const struct rockchip_hdmi_chip_data *chip_data;
+	enum dw_hdmi_devtype dev_type;
 	struct clk *vpll_clk;
 	struct clk *grf_clk;
 };
@@ -305,6 +306,7 @@ static const struct dw_hdmi_plat_data rk3288_hdmi_drv_data = {
 	.cur_ctr    = rockchip_cur_ctr,
 	.phy_config = rockchip_phy_config,
 	.phy_data = &rk3288_chip_data,
+	.dev_type   = RK3288_HDMI,
 };
 
 static struct rockchip_hdmi_chip_data rk3399_chip_data = {
@@ -315,6 +317,7 @@ static struct rockchip_hdmi_chip_data rk3399_chip_data = {
 
 static const struct dw_hdmi_plat_data rk3328_hdmi_drv_data = {
 	.mode_valid = dw_hdmi_rockchip_mode_valid,
+	.dev_type   = RK3328_HDMI,
 };
 static const struct dw_hdmi_plat_data rk3399_hdmi_drv_data = {
 	.mode_valid = dw_hdmi_rockchip_mode_valid,
@@ -322,6 +325,7 @@ static const struct dw_hdmi_plat_data rk3399_hdmi_drv_data = {
 	.cur_ctr    = rockchip_cur_ctr,
 	.phy_config = rockchip_phy_config,
 	.phy_data = &rk3399_chip_data,
+	.dev_type   = RK3399_HDMI,
 };
 
 static const struct of_device_id dw_hdmi_rockchip_dt_ids[] = {
@@ -360,6 +364,7 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
 	plat_data = match->data;
 	hdmi->dev = &pdev->dev;
 	hdmi->chip_data = plat_data->phy_data;
+	hdmi->dev_type = plat_data->dev_type;
 	encoder = &hdmi->encoder;
 
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index 182f832..fdb1f0a 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -92,6 +92,15 @@ enum dw_hdmi_phy_type {
 	DW_HDMI_PHY_VENDOR_PHY = 0xfe,
 };
 
+enum dw_hdmi_devtype {
+	RK3228_HDMI,
+	RK3288_HDMI,
+	RK3328_HDMI,
+	RK3366_HDMI,
+	RK3368_HDMI,
+	RK3399_HDMI,
+};
+
 struct dw_hdmi_mpll_config {
 	unsigned long mpixelclock;
 	struct {
@@ -124,6 +133,7 @@ struct dw_hdmi_phy_ops {
 
 struct dw_hdmi_plat_data {
 	struct regmap *regm;
+	enum dw_hdmi_devtype dev_type;
 	enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
 					   const struct drm_display_mode *mode);
 	unsigned long input_bus_format;
-- 
2.7.4

Re: [PATCH v2 2/7] drm/rockchip: dw_hdmi: add device type

[Neil Armstrong]

On 30/09/2017 03:43, Algea Cao wrote:
> To determine type of SOC, we add a parameter dev_type.
> 
> Signed-off-by: Algea Cao <algea.cao@rock-chips.com>
> ---
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c |  5 +++++
>  include/drm/bridge/dw_hdmi.h                | 10 ++++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index 719452d..09c77f9 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -46,6 +46,7 @@ struct rockchip_hdmi {
>  	struct regmap *regmap;
>  	struct drm_encoder encoder;
>  	const struct rockchip_hdmi_chip_data *chip_data;
> +	enum dw_hdmi_devtype dev_type;
>  	struct clk *vpll_clk;
>  	struct clk *grf_clk;
>  };
> @@ -305,6 +306,7 @@ static const struct dw_hdmi_plat_data rk3288_hdmi_drv_data = {
>  	.cur_ctr    = rockchip_cur_ctr,
>  	.phy_config = rockchip_phy_config,
>  	.phy_data = &rk3288_chip_data,
> +	.dev_type   = RK3288_HDMI,
>  };
>  
>  static struct rockchip_hdmi_chip_data rk3399_chip_data = {
> @@ -315,6 +317,7 @@ static struct rockchip_hdmi_chip_data rk3399_chip_data = {
>  
>  static const struct dw_hdmi_plat_data rk3328_hdmi_drv_data = {
>  	.mode_valid = dw_hdmi_rockchip_mode_valid,
> +	.dev_type   = RK3328_HDMI,
>  };
>  static const struct dw_hdmi_plat_data rk3399_hdmi_drv_data = {
>  	.mode_valid = dw_hdmi_rockchip_mode_valid,
> @@ -322,6 +325,7 @@ static const struct dw_hdmi_plat_data rk3399_hdmi_drv_data = {
>  	.cur_ctr    = rockchip_cur_ctr,
>  	.phy_config = rockchip_phy_config,
>  	.phy_data = &rk3399_chip_data,
> +	.dev_type   = RK3399_HDMI,
>  };
>  
>  static const struct of_device_id dw_hdmi_rockchip_dt_ids[] = {
> @@ -360,6 +364,7 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
>  	plat_data = match->data;
>  	hdmi->dev = &pdev->dev;
>  	hdmi->chip_data = plat_data->phy_data;
> +	hdmi->dev_type = plat_data->dev_type;
>  	encoder = &hdmi->encoder;
>  
>  	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index 182f832..fdb1f0a 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -92,6 +92,15 @@ enum dw_hdmi_phy_type {
>  	DW_HDMI_PHY_VENDOR_PHY = 0xfe,
>  };
>  
> +enum dw_hdmi_devtype {
> +	RK3228_HDMI,
> +	RK3288_HDMI,
> +	RK3328_HDMI,
> +	RK3366_HDMI,
> +	RK3368_HDMI,
> +	RK3399_HDMI,
> +};
> +
>  struct dw_hdmi_mpll_config {
>  	unsigned long mpixelclock;
>  	struct {
> @@ -124,6 +133,7 @@ struct dw_hdmi_phy_ops {
>  
>  struct dw_hdmi_plat_data {
>  	struct regmap *regm;
> +	enum dw_hdmi_devtype dev_type;

Please don't do that, dw-hdmi driver should not have knowledge of the device type,
if the vendor PHY reporting is broken, add a way to force phy_type to DW_HDMI_PHY_VENDOR_PHY.

Neil

>  	enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
>  					   const struct drm_display_mode *mode);
>  	unsigned long input_bus_format;
> 

[PATCH v2 3/7] drm: bridge: dw-hdmi: change hdmi phy hpd read function to export

[Algea Cao]

Change dw_hdmi_phy_read_hpd from static to export.
inno hdmi phy ops will call this interface to get
hpd status.

Signed-off-by: Algea Cao <algea.cao@rock-chips.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 5 +++--
 include/drm/bridge/dw_hdmi.h              | 3 ++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index bf14214..34eeaf6 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1250,12 +1250,13 @@ static void dw_hdmi_phy_disable(struct dw_hdmi *hdmi, void *data)
 	dw_hdmi_phy_power_off(hdmi);
 }
 
-static enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
-						      void *data)
+enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
+					       void *data)
 {
 	return hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_HPD ?
 		connector_status_connected : connector_status_disconnected;
 }
+EXPORT_SYMBOL_GPL(dw_hdmi_phy_read_hpd);
 
 static void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
 				   bool force, bool disabled, bool rxsense)
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index fdb1f0a..01e4979 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -161,7 +161,8 @@ int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
 		 const struct dw_hdmi_plat_data *plat_data);
 
 void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense);
-
+enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
+					       void *data);
 void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);
 void dw_hdmi_audio_enable(struct dw_hdmi *hdmi);
 void dw_hdmi_audio_disable(struct dw_hdmi *hdmi);
-- 
2.7.4

[PATCH v2 4/7] drm/rockchip: dw_hdmi: add inno hdmi phy ops

[Algea Cao]

Because some RK chips use inno hdmi phy, such as RK3328, we add
inno hdmi phy ops.

Signed-off-by: Algea Cao <algea.cao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 110 +++++++++++++++++++++++++++-
 1 file changed, 107 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 09c77f9..7658b2f 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -12,7 +12,7 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
-
+#include <linux/phy/phy.h>
 #include <drm/drm_of.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
@@ -26,6 +26,18 @@
 #define RK3288_HDMI_LCDC_SEL		BIT(4)
 #define RK3399_GRF_SOC_CON20		0x6250
 #define RK3399_HDMI_LCDC_SEL		BIT(6)
+#define RK3328_GRF_SOC_CON2		0x0408
+#define RK3328_DDC_MASK_EN		((3 << 10) | (3 << (10 + 16)))
+#define RK3328_GRF_SOC_CON3		0x040c
+#define RK3328_IO_CTRL_BY_HDMI		(0xf0000000 | BIT(13) | BIT(12))
+#define RK3328_GRF_SOC_CON4		0x0410
+#define RK3328_IO_3V_DOMAIN		(7 << (9 + 16))
+#define RK3328_IO_5V_DOMAIN		((7 << 9) | (3 << (9 + 16)))
+#define RK3328_HPD_3V			(BIT(8 + 16) | BIT(13 + 16))
+#define RK3228_GRF_SOC_CON2		0x0408
+#define RK3228_DDC_MASK_EN		((3 << 13) | (3 << (13 + 16)))
+#define RK3228_GRF_SOC_CON6		0x0418
+#define RK3228_IO_3V_DOMAIN		((7 << 4) | (7 << (4 + 16)))
 
 #define HIWORD_UPDATE(val, mask)	(val | (mask) << 16)
 
@@ -49,10 +61,82 @@ struct rockchip_hdmi {
 	enum dw_hdmi_devtype dev_type;
 	struct clk *vpll_clk;
 	struct clk *grf_clk;
+	struct phy *phy;
 };
 
 #define to_rockchip_hdmi(x)	container_of(x, struct rockchip_hdmi, x)
 
+static int
+inno_dw_hdmi_phy_init(struct dw_hdmi *dw_hdmi, void *data,
+		      struct drm_display_mode *mode)
+{
+	struct rockchip_hdmi *hdmi = (struct rockchip_hdmi *)data;
+
+	return phy_power_on(hdmi->phy);
+}
+
+static void inno_dw_hdmi_phy_disable(struct dw_hdmi *dw_hdmi, void *data)
+{
+	struct rockchip_hdmi *hdmi = (struct rockchip_hdmi *)data;
+
+	phy_power_off(hdmi->phy);
+}
+
+static enum drm_connector_status
+inno_dw_hdmi_phy_read_hpd(struct dw_hdmi *dw_hdmi, void *data)
+{
+	struct rockchip_hdmi *hdmi = (struct rockchip_hdmi *)data;
+	enum drm_connector_status status;
+
+	status = dw_hdmi_phy_read_hpd(dw_hdmi, data);
+
+	if (hdmi->dev_type == RK3228_HDMI)
+		return status;
+
+	if (status == connector_status_connected)
+		regmap_write(hdmi->regmap,
+			     RK3328_GRF_SOC_CON4,
+			     RK3328_IO_5V_DOMAIN);
+	else
+		regmap_write(hdmi->regmap,
+			     RK3328_GRF_SOC_CON4,
+			     RK3328_IO_3V_DOMAIN);
+	return status;
+}
+
+static int inno_dw_hdmi_init(struct rockchip_hdmi *hdmi)
+{
+	int ret;
+
+	ret = clk_prepare_enable(hdmi->grf_clk);
+	if (ret < 0) {
+		dev_err(hdmi->dev, "failed to enable grfclk %d\n", ret);
+		return -EPROBE_DEFER;
+	}
+	if (hdmi->dev_type == RK3328_HDMI) {
+		/* Map HPD pin to 3V io */
+		regmap_write(hdmi->regmap,
+			     RK3328_GRF_SOC_CON4,
+			     RK3328_IO_3V_DOMAIN |
+			     RK3328_HPD_3V);
+		/* Map ddc pin to 5V io */
+		regmap_write(hdmi->regmap,
+			     RK3328_GRF_SOC_CON3,
+			     RK3328_IO_CTRL_BY_HDMI);
+		regmap_write(hdmi->regmap,
+			     RK3328_GRF_SOC_CON2,
+			     RK3328_DDC_MASK_EN |
+			     BIT(18));
+	} else {
+		regmap_write(hdmi->regmap, RK3228_GRF_SOC_CON2,
+			     RK3228_DDC_MASK_EN);
+		regmap_write(hdmi->regmap, RK3228_GRF_SOC_CON6,
+			     RK3228_IO_3V_DOMAIN);
+	}
+	clk_disable_unprepare(hdmi->grf_clk);
+	return 0;
+}
+
 static const struct dw_hdmi_mpll_config rockchip_mpll_cfg[] = {
 	{
 		27000000, {
@@ -294,6 +378,12 @@ 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_phy_ops inno_dw_hdmi_phy_ops = {
+	.init		= inno_dw_hdmi_phy_init,
+	.disable	= inno_dw_hdmi_phy_disable,
+	.read_hpd	= inno_dw_hdmi_phy_read_hpd,
+};
+
 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),
@@ -317,6 +407,8 @@ static struct rockchip_hdmi_chip_data rk3399_chip_data = {
 
 static const struct dw_hdmi_plat_data rk3328_hdmi_drv_data = {
 	.mode_valid = dw_hdmi_rockchip_mode_valid,
+	.phy_ops    = &inno_dw_hdmi_phy_ops,
+	.phy_name   = "inno_dw_hdmi_phy2",
 	.dev_type   = RK3328_HDMI,
 };
 static const struct dw_hdmi_plat_data rk3399_hdmi_drv_data = {
@@ -346,7 +438,7 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
 				 void *data)
 {
 	struct platform_device *pdev = to_platform_device(dev);
-	const struct dw_hdmi_plat_data *plat_data;
+	struct dw_hdmi_plat_data *plat_data;
 	const struct of_device_id *match;
 	struct drm_device *drm = data;
 	struct drm_encoder *encoder;
@@ -361,7 +453,7 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
 		return -ENOMEM;
 
 	match = of_match_node(dw_hdmi_rockchip_dt_ids, pdev->dev.of_node);
-	plat_data = match->data;
+	plat_data = (struct dw_hdmi_plat_data *)match->data;
 	hdmi->dev = &pdev->dev;
 	hdmi->chip_data = plat_data->phy_data;
 	hdmi->dev_type = plat_data->dev_type;
@@ -383,6 +475,18 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
 		return ret;
 	}
 
+	if (hdmi->dev_type == RK3328_HDMI || hdmi->dev_type == RK3228_HDMI) {
+		hdmi->phy = devm_phy_get(dev, "hdmi_phy");
+		if (IS_ERR(hdmi->phy)) {
+			ret = PTR_ERR(hdmi->phy);
+			dev_err(dev, "failed to get phy: %d\n", ret);
+			return ret;
+		}
+		plat_data->phy_data = hdmi;
+		ret = inno_dw_hdmi_init(hdmi);
+		if (ret < 0)
+			return ret;
+	}
 	drm_encoder_helper_add(encoder, &dw_hdmi_rockchip_encoder_helper_funcs);
 	drm_encoder_init(drm, encoder, &dw_hdmi_rockchip_encoder_funcs,
 			 DRM_MODE_ENCODER_TMDS, NULL);
-- 
2.7.4

Re: [PATCH v2 4/7] drm/rockchip: dw_hdmi: add inno hdmi phy ops

[Heiko Stuebner]

Hi Algea,

Am Samstag, 30. September 2017, 09:44:46 CET schrieb Algea Cao:
> Because some RK chips use inno hdmi phy, such as RK3328, we add
> inno hdmi phy ops.
> 
> Signed-off-by: Algea Cao <algea.cao@rock-chips.com>
> ---
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 110 +++++++++++++++++++++++++++-
>  1 file changed, 107 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index 09c77f9..7658b2f 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -12,7 +12,7 @@
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
> -
> +#include <linux/phy/phy.h>
>  #include <drm/drm_of.h>
>  #include <drm/drmP.h>
>  #include <drm/drm_crtc_helper.h>
> @@ -26,6 +26,18 @@
>  #define RK3288_HDMI_LCDC_SEL		BIT(4)
>  #define RK3399_GRF_SOC_CON20		0x6250
>  #define RK3399_HDMI_LCDC_SEL		BIT(6)
> +#define RK3328_GRF_SOC_CON2		0x0408
> +#define RK3328_DDC_MASK_EN		((3 << 10) | (3 << (10 + 16)))
> +#define RK3328_GRF_SOC_CON3		0x040c
> +#define RK3328_IO_CTRL_BY_HDMI		(0xf0000000 | BIT(13) | BIT(12))
> +#define RK3328_GRF_SOC_CON4		0x0410
> +#define RK3328_IO_3V_DOMAIN		(7 << (9 + 16))
> +#define RK3328_IO_5V_DOMAIN		((7 << 9) | (3 << (9 + 16)))

This is slightly confusing. (9+16) is obviously the write-enable-mask, so
shouldn't the write-enable-mask not at least cover the changed bits?


> +static enum drm_connector_status
> +inno_dw_hdmi_phy_read_hpd(struct dw_hdmi *dw_hdmi, void *data)
> +{
> +	struct rockchip_hdmi *hdmi = (struct rockchip_hdmi *)data;
> +	enum drm_connector_status status;
> +
> +	status = dw_hdmi_phy_read_hpd(dw_hdmi, data);
> +
> +	if (hdmi->dev_type == RK3228_HDMI)
> +		return status;

There is no need to encode the functionality for both variants
(and possibly later ones) into one function.

As Neil said in his reply to patch2, we don't really want to carry
that dev_type property, so why don't you just define this as
rk3328_inno_phy_read_hpd and then use it like 

static const struct dw_hdmi_phy_ops rk3228_dw_hdmi_phy_ops = {
	.init		= inno_dw_hdmi_phy_init,
	.disable	= inno_dw_hdmi_phy_disable,
	.read_hpd	= dw_hdmi_phy_read_hpd,
};

static const struct dw_hdmi_phy_ops rk3328_dw_hdmi_phy_ops = {
	.init		= inno_dw_hdmi_phy_init,
	.disable	= inno_dw_hdmi_phy_disable,
	.read_hpd	= rk3328_inno_phy_read_hpd,
};

> +	if (status == connector_status_connected)
> +		regmap_write(hdmi->regmap,
> +			     RK3328_GRF_SOC_CON4,
> +			     RK3328_IO_5V_DOMAIN);
> +	else
> +		regmap_write(hdmi->regmap,
> +			     RK3328_GRF_SOC_CON4,
> +			     RK3328_IO_3V_DOMAIN);

That should definitely get a comment explaining what this is doing and
why :-) . Especially as it seems related to the plug/unplug events.

According to the TRM, the hdmiphy IP block has its own interrupt. Can
you tell me if it is firing on hotplug events? Because I'm wondering if
these GRF settings wouldn't be better live in the hdmiphy driver
[if it gets an irq on plug/unplug], which in turn would make the phy
handling in dw_hdmi a lot easier.

> +	return status;
> +}
> +

[...]

> @@ -361,7 +453,7 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
>  		return -ENOMEM;
>  
>  	match = of_match_node(dw_hdmi_rockchip_dt_ids, pdev->dev.of_node);
> -	plat_data = match->data;
> +	plat_data = (struct dw_hdmi_plat_data *)match->data;
>  	hdmi->dev = &pdev->dev;
>  	hdmi->chip_data = plat_data->phy_data;
>  	hdmi->dev_type = plat_data->dev_type;
> @@ -383,6 +475,18 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
>  		return ret;
>  	}
>  
> +	if (hdmi->dev_type == RK3328_HDMI || hdmi->dev_type == RK3228_HDMI) {
> +		hdmi->phy = devm_phy_get(dev, "hdmi_phy");
> +		if (IS_ERR(hdmi->phy)) {
> +			ret = PTR_ERR(hdmi->phy);
> +			dev_err(dev, "failed to get phy: %d\n", ret);
> +			return ret;
> +		}
> +		plat_data->phy_data = hdmi;
> +		ret = inno_dw_hdmi_init(hdmi);
> +		if (ret < 0)
> +			return ret;
> +	}

You could also just declare it optional in the binding, try to get the phy
via devm_phy_optional_get and if it's NULL just assume that it's the
regular internal phy as with previous socs. That way you can drop the
dev-type-check.

Aka, if a phy is defined we are pretty sure we want to use that one :-) .


Heiko

[PATCH v2 5/7] drm/rockchip: dw_hdmi: add hclk_vio

[Algea Cao]

Add clk hclk_vio and enable it when hdmi bind.

Signed-off-by: Algea Cao <algea.cao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 7658b2f..e1a9941 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -61,6 +61,7 @@ struct rockchip_hdmi {
 	enum dw_hdmi_devtype dev_type;
 	struct clk *vpll_clk;
 	struct clk *grf_clk;
+	struct clk *hclk_vio;
 	struct phy *phy;
 };
 
@@ -277,12 +278,27 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi)
 		return PTR_ERR(hdmi->grf_clk);
 	}
 
+	hdmi->hclk_vio = devm_clk_get(hdmi->dev, "hclk_vio");
+	if (PTR_ERR(hdmi->hclk_vio) == -ENOENT) {
+		hdmi->hclk_vio = NULL;
+	} else if (PTR_ERR(hdmi->hclk_vio) == -EPROBE_DEFER) {
+		return -EPROBE_DEFER;
+	} else if (IS_ERR(hdmi->hclk_vio)) {
+		dev_dbg(hdmi->dev, "failed to get hclk_vio clock\n");
+		return PTR_ERR(hdmi->hclk_vio);
+	}
 	ret = clk_prepare_enable(hdmi->vpll_clk);
 	if (ret) {
 		dev_err(hdmi->dev, "Failed to enable HDMI vpll: %d\n", ret);
 		return ret;
 	}
 
+	ret = clk_prepare_enable(hdmi->hclk_vio);
+	if (ret) {
+		dev_dbg(hdmi->dev, "Failed to eanble HDMI hclk_vio: %d\n",
+			ret);
+		return ret;
+	}
 	return 0;
 }
 
@@ -506,6 +522,11 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
 static void dw_hdmi_rockchip_unbind(struct device *dev, struct device *master,
 				    void *data)
 {
+	struct rockchip_hdmi *hdmi = container_of(&dev, struct rockchip_hdmi,
+						  dev);
+
+	clk_disable_unprepare(hdmi->hclk_vio);
+
 	return dw_hdmi_unbind(dev);
 }
 
-- 
2.7.4

Re: [PATCH v2 5/7] drm/rockchip: dw_hdmi: add hclk_vio

[Heiko Stuebner]

Hi Algea,

Am Samstag, 30. September 2017, 09:45:12 CET schrieb Algea Cao:
> Add clk hclk_vio and enable it when hdmi bind.

Could you explain what the hclk_vio reference is needed for please?

Because from from what I tracked down in the TRM and code, this hclk_vio
is defined wrong in the clock-driver.

According to the TRM, that hclk_vio (gate22[1]) is actually hclk_vio_niu and
hence the clock for the interconnect <-> hdmi  connection.
As this clock is a property of the interconnect, which we don't model so far,
all niu clocks are simply defined as critical in the clock driver itself, as can
be seen in most clock drivers.

So I'd suggest fixing the clock-driver accordingly and dropping this patch.


Heiko


> Signed-off-by: Algea Cao <algea.cao@rock-chips.com>
> ---
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index 7658b2f..e1a9941 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -61,6 +61,7 @@ struct rockchip_hdmi {
>  	enum dw_hdmi_devtype dev_type;
>  	struct clk *vpll_clk;
>  	struct clk *grf_clk;
> +	struct clk *hclk_vio;
>  	struct phy *phy;
>  };
>  
> @@ -277,12 +278,27 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi)
>  		return PTR_ERR(hdmi->grf_clk);
>  	}
>  
> +	hdmi->hclk_vio = devm_clk_get(hdmi->dev, "hclk_vio");
> +	if (PTR_ERR(hdmi->hclk_vio) == -ENOENT) {
> +		hdmi->hclk_vio = NULL;
> +	} else if (PTR_ERR(hdmi->hclk_vio) == -EPROBE_DEFER) {
> +		return -EPROBE_DEFER;
> +	} else if (IS_ERR(hdmi->hclk_vio)) {
> +		dev_dbg(hdmi->dev, "failed to get hclk_vio clock\n");
> +		return PTR_ERR(hdmi->hclk_vio);
> +	}
>  	ret = clk_prepare_enable(hdmi->vpll_clk);
>  	if (ret) {
>  		dev_err(hdmi->dev, "Failed to enable HDMI vpll: %d\n", ret);
>  		return ret;
>  	}
>  
> +	ret = clk_prepare_enable(hdmi->hclk_vio);
> +	if (ret) {
> +		dev_dbg(hdmi->dev, "Failed to eanble HDMI hclk_vio: %d\n",
> +			ret);
> +		return ret;
> +	}
>  	return 0;
>  }
>  
> @@ -506,6 +522,11 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
>  static void dw_hdmi_rockchip_unbind(struct device *dev, struct device *master,
>  				    void *data)
>  {
> +	struct rockchip_hdmi *hdmi = container_of(&dev, struct rockchip_hdmi,
> +						  dev);
> +
> +	clk_disable_unprepare(hdmi->hclk_vio);
> +
>  	return dw_hdmi_unbind(dev);
>  }
>  
> 

[PATCH v2 6/7] drm/rockchip: dw_hdmi: update dw-hdmi encoder enable

[Algea Cao]

Writing grf register according to device type.

Signed-off-by: Algea Cao <algea.cao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index e1a9941..b9087af 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -349,9 +349,26 @@ 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 val;
+	struct drm_crtc *crtc = encoder->crtc;
+	u32 val, lcdsel_grf_reg, lcdsel_mask;
 	int ret;
 
+	if (WARN_ON(!crtc || !crtc->state))
+		return;
+
+	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;
+	}
+
 	ret = drm_of_encoder_active_endpoint_id(hdmi->dev->of_node, encoder);
 	if (ret)
 		val = hdmi->chip_data->lcdsel_lit;
-- 
2.7.4

[PATCH v2 7/7] drm: bridge: dw-hdmi: get phy ops by device type

[Algea Cao]

Add device type to distinguish different chips.Different chips
use different phy ops, get them by device type.

Signed-off-by: Algea Cao <algea.cao@rock-chips.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 34eeaf6..b414aef 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -144,6 +144,7 @@ struct dw_hdmi {
 	const struct dw_hdmi_plat_data *plat_data;
 
 	int vic;
+	enum dw_hdmi_devtype dev_type;
 
 	u8 edid[HDMI_EDID_LEN];
 	bool cable_plugin;
@@ -2202,7 +2203,9 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi)
 
 	phy_type = hdmi_readb(hdmi, HDMI_CONFIG2_ID);
 
-	if (phy_type == DW_HDMI_PHY_VENDOR_PHY) {
+	if (phy_type == DW_HDMI_PHY_VENDOR_PHY ||
+	    hdmi->dev_type == RK3328_HDMI ||
+	    hdmi->dev_type == RK3228_HDMI) {
 		/* Vendor PHYs require support from the glue layer. */
 		if (!hdmi->plat_data->phy_ops || !hdmi->plat_data->phy_name) {
 			dev_err(hdmi->dev,
@@ -2298,6 +2301,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
 	if (!hdmi)
 		return ERR_PTR(-ENOMEM);
 
+	hdmi->dev_type = plat_data->dev_type;
 	hdmi->plat_data = plat_data;
 	hdmi->dev = dev;
 	hdmi->sample_rate = 48000;
-- 
2.7.4