[PATCH v3 00/20] dw-hdmi: Cleanups, fixes and updates for v4.11

[PATCH v3 00/20] dw-hdmi: Cleanups, fixes and updates for v4.11

[Laurent Pinchart]

Hello,

This patch series contains the part of my pending dw-hdmi patches that have
been successfully tested on all three supported platforms (i.MX6, RK3288 and
R-Car H3) and have been reviewed without any problem being reported.

All patches here have been previously posted as part of the "[PATCH v2 00/29]
R-Car Gen3 HDMI output support" and "[PATCH 0/3] dw-hdmi: miscellaneous
cleanups and fix" series. This series is a repost with all review tags
collected.

Kieran Bingham (2):
  drm: bridge: dw-hdmi: Remove unused function parameter
  drm: bridge: dw-hdmi: Remove PHY configuration resolution parameter

Laurent Pinchart (18):
  drm: bridge: dw-hdmi: Merge __hdmi_phy_i2c_write and
    hdmi_phy_i2c_write
  drm: bridge: dw-hdmi: Remove unneeded arguments to bind/unbind
    functions
  drm: bridge: dw-hdmi: Embed drm_bridge in struct dw_hdmi
  drm: bridge: dw-hdmi: Remove encoder field from struct dw_hdmi
  drm: bridge: dw-hdmi: Don't forward HPD events to DRM core before
    attach
  drm: bridge: dw-hdmi: Move IRQ and IO resource allocation to common
    code
  drm: bridge: dw-hdmi: Reorder functions to prepare for next commit
  drm: bridge: dw-hdmi: Create connector in the bridge attach operation
  drm: bridge: dw-hdmi: Implement DRM bridge registration
  drm: bridge: dw-hdmi: Rename CONF0 SPARECTRL bit to SVSRET
  drm: bridge: dw-hdmi: Reject invalid product IDs
  drm: bridge: dw-hdmi: Detect AHB audio DMA using correct register
  drm: bridge: dw-hdmi: Handle overflow workaround based on device
    version
  drm: bridge: dw-hdmi: Detect PHY type at runtime
  drm: bridge: dw-hdmi: Define and use macros for PHY register addresses
  drm: bridge: dw-hdmi: Fix the name of the PHY reset macros
  drm: bridge: dw-hdmi: Assert SVSRET before resetting the PHY
  dt-bindings: display: dw-hdmi: Clean up DT bindings documentation

 .../devicetree/bindings/display/bridge/dw_hdmi.txt |  85 ++--
 .../devicetree/bindings/display/imx/hdmi.txt       |  51 +--
 .../bindings/display/rockchip/dw_hdmi-rockchip.txt |  43 +-
 drivers/gpu/drm/bridge/dw-hdmi.c                   | 438 +++++++++++++--------
 drivers/gpu/drm/bridge/dw-hdmi.h                   |  85 +++-
 drivers/gpu/drm/imx/dw_hdmi-imx.c                  |  14 +-
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        |  14 +-
 include/drm/bridge/dw_hdmi.h                       |  19 +-
 8 files changed, 472 insertions(+), 277 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH v3 00/20] dw-hdmi: Cleanups, fixes and updates for v4.11

[Laurent Pinchart]

Hello,

This patch series contains the part of my pending dw-hdmi patches that have
been successfully tested on all three supported platforms (i.MX6, RK3288 and
R-Car H3) and have been reviewed without any problem being reported.

All patches here have been previously posted as part of the "[PATCH v2 00/29]
R-Car Gen3 HDMI output support" and "[PATCH 0/3] dw-hdmi: miscellaneous
cleanups and fix" series. This series is a repost with all review tags
collected.

Kieran Bingham (2):
  drm: bridge: dw-hdmi: Remove unused function parameter
  drm: bridge: dw-hdmi: Remove PHY configuration resolution parameter

Laurent Pinchart (18):
  drm: bridge: dw-hdmi: Merge __hdmi_phy_i2c_write and
    hdmi_phy_i2c_write
  drm: bridge: dw-hdmi: Remove unneeded arguments to bind/unbind
    functions
  drm: bridge: dw-hdmi: Embed drm_bridge in struct dw_hdmi
  drm: bridge: dw-hdmi: Remove encoder field from struct dw_hdmi
  drm: bridge: dw-hdmi: Don't forward HPD events to DRM core before
    attach
  drm: bridge: dw-hdmi: Move IRQ and IO resource allocation to common
    code
  drm: bridge: dw-hdmi: Reorder functions to prepare for next commit
  drm: bridge: dw-hdmi: Create connector in the bridge attach operation
  drm: bridge: dw-hdmi: Implement DRM bridge registration
  drm: bridge: dw-hdmi: Rename CONF0 SPARECTRL bit to SVSRET
  drm: bridge: dw-hdmi: Reject invalid product IDs
  drm: bridge: dw-hdmi: Detect AHB audio DMA using correct register
  drm: bridge: dw-hdmi: Handle overflow workaround based on device
    version
  drm: bridge: dw-hdmi: Detect PHY type at runtime
  drm: bridge: dw-hdmi: Define and use macros for PHY register addresses
  drm: bridge: dw-hdmi: Fix the name of the PHY reset macros
  drm: bridge: dw-hdmi: Assert SVSRET before resetting the PHY
  dt-bindings: display: dw-hdmi: Clean up DT bindings documentation

 .../devicetree/bindings/display/bridge/dw_hdmi.txt |  85 ++--
 .../devicetree/bindings/display/imx/hdmi.txt       |  51 +--
 .../bindings/display/rockchip/dw_hdmi-rockchip.txt |  43 +-
 drivers/gpu/drm/bridge/dw-hdmi.c                   | 438 +++++++++++++--------
 drivers/gpu/drm/bridge/dw-hdmi.h                   |  85 +++-
 drivers/gpu/drm/imx/dw_hdmi-imx.c                  |  14 +-
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        |  14 +-
 include/drm/bridge/dw_hdmi.h                       |  19 +-
 8 files changed, 472 insertions(+), 277 deletions(-)

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 01/20] drm: bridge: dw-hdmi: Merge __hdmi_phy_i2c_write and hdmi_phy_i2c_write

[Laurent Pinchart]

The latter is just an int wrapper around the former void function that
unconditionally returns 0. As the return value is never checked, merge
the two functions into one.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index f5009ae39b89..a6685502571e 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -868,7 +868,7 @@ static bool hdmi_phy_wait_i2c_done(struct dw_hdmi *hdmi, int msec)
 	return true;
 }
 
-static void __hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
+static void hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
 				 unsigned char addr)
 {
 	hdmi_writeb(hdmi, 0xFF, HDMI_IH_I2CMPHY_STAT0);
@@ -882,13 +882,6 @@ static void __hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
 	hdmi_phy_wait_i2c_done(hdmi, 1000);
 }
 
-static int hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
-			      unsigned char addr)
-{
-	__hdmi_phy_i2c_write(hdmi, data, addr);
-	return 0;
-}
-
 static void dw_hdmi_phy_enable_powerdown(struct dw_hdmi *hdmi, bool enable)
 {
 	hdmi_mask_writeb(hdmi, !enable, HDMI_PHY_CONF0,
-- 
Regards,

Laurent Pinchart

[PATCH v3 01/20] drm: bridge: dw-hdmi: Merge __hdmi_phy_i2c_write and hdmi_phy_i2c_write

[Laurent Pinchart]

The latter is just an int wrapper around the former void function that
unconditionally returns 0. As the return value is never checked, merge
the two functions into one.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index f5009ae39b89..a6685502571e 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -868,7 +868,7 @@ static bool hdmi_phy_wait_i2c_done(struct dw_hdmi *hdmi, int msec)
 	return true;
 }
 
-static void __hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
+static void hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
 				 unsigned char addr)
 {
 	hdmi_writeb(hdmi, 0xFF, HDMI_IH_I2CMPHY_STAT0);
@@ -882,13 +882,6 @@ static void __hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
 	hdmi_phy_wait_i2c_done(hdmi, 1000);
 }
 
-static int hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
-			      unsigned char addr)
-{
-	__hdmi_phy_i2c_write(hdmi, data, addr);
-	return 0;
-}
-
 static void dw_hdmi_phy_enable_powerdown(struct dw_hdmi *hdmi, bool enable)
 {
 	hdmi_mask_writeb(hdmi, !enable, HDMI_PHY_CONF0,
-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 02/20] drm: bridge: dw-hdmi: Remove unneeded arguments to bind/unbind functions

[Laurent Pinchart]

The master argument isn't used. The data argument, a void pointer, is
used by the bind function only where it's cast to a drm_device pointer,
which can easily be obtained from the encoder argument instead. Remove
them.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c            | 8 +++-----
 drivers/gpu/drm/imx/dw_hdmi-imx.c           | 4 ++--
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 4 ++--
 include/drm/bridge/dw_hdmi.h                | 5 ++---
 4 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index a6685502571e..f86894622070 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1854,12 +1854,10 @@ static int dw_hdmi_register(struct drm_device *drm, struct dw_hdmi *hdmi)
 	return 0;
 }
 
-int dw_hdmi_bind(struct device *dev, struct device *master,
-		 void *data, struct drm_encoder *encoder,
+int dw_hdmi_bind(struct device *dev, struct drm_encoder *encoder,
 		 struct resource *iores, int irq,
 		 const struct dw_hdmi_plat_data *plat_data)
 {
-	struct drm_device *drm = data;
 	struct device_node *np = dev->of_node;
 	struct platform_device_info pdevinfo;
 	struct device_node *ddc_node;
@@ -1992,7 +1990,7 @@ int dw_hdmi_bind(struct device *dev, struct device *master,
 	if (ret)
 		goto err_iahb;
 
-	ret = dw_hdmi_register(drm, hdmi);
+	ret = dw_hdmi_register(encoder->dev, hdmi);
 	if (ret)
 		goto err_iahb;
 
@@ -2059,7 +2057,7 @@ int dw_hdmi_bind(struct device *dev, struct device *master,
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_bind);
 
-void dw_hdmi_unbind(struct device *dev, struct device *master, void *data)
+void dw_hdmi_unbind(struct device *dev)
 {
 	struct dw_hdmi *hdmi = dev_get_drvdata(dev);
 
diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c b/drivers/gpu/drm/imx/dw_hdmi-imx.c
index 359cd2765552..f79665801543 100644
--- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
+++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
@@ -249,7 +249,7 @@ static int dw_hdmi_imx_bind(struct device *dev, struct device *master,
 	drm_encoder_init(drm, encoder, &dw_hdmi_imx_encoder_funcs,
 			 DRM_MODE_ENCODER_TMDS, NULL);
 
-	ret = dw_hdmi_bind(dev, master, data, encoder, iores, irq, plat_data);
+	ret = dw_hdmi_bind(dev, encoder, iores, irq, plat_data);
 
 	/*
 	 * If dw_hdmi_bind() fails we'll never call dw_hdmi_unbind(),
@@ -264,7 +264,7 @@ static int dw_hdmi_imx_bind(struct device *dev, struct device *master,
 static void dw_hdmi_imx_unbind(struct device *dev, struct device *master,
 			       void *data)
 {
-	return dw_hdmi_unbind(dev, master, data);
+	return dw_hdmi_unbind(dev);
 }
 
 static const struct component_ops dw_hdmi_imx_ops = {
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 0665fb915579..e8fb5c56b224 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -301,7 +301,7 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
 	drm_encoder_init(drm, encoder, &dw_hdmi_rockchip_encoder_funcs,
 			 DRM_MODE_ENCODER_TMDS, NULL);
 
-	ret = dw_hdmi_bind(dev, master, data, encoder, iores, irq, plat_data);
+	ret = dw_hdmi_bind(dev, encoder, iores, irq, plat_data);
 
 	/*
 	 * If dw_hdmi_bind() fails we'll never call dw_hdmi_unbind(),
@@ -316,7 +316,7 @@ 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)
 {
-	return dw_hdmi_unbind(dev, master, data);
+	return dw_hdmi_unbind(dev);
 }
 
 static const struct component_ops dw_hdmi_rockchip_ops = {
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index bae79f3c4d28..11edda631a9d 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -56,9 +56,8 @@ struct dw_hdmi_plat_data {
 					   struct drm_display_mode *mode);
 };
 
-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,
+void dw_hdmi_unbind(struct device *dev);
+int dw_hdmi_bind(struct device *dev, struct drm_encoder *encoder,
 		 struct resource *iores, int irq,
 		 const struct dw_hdmi_plat_data *plat_data);
 
-- 
Regards,

Laurent Pinchart

[PATCH v3 03/20] drm: bridge: dw-hdmi: Remove unused function parameter

[Laurent Pinchart]

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

The 'prep' parameter passed to hdmi_phy_configure() is useless. It is
hardcoded as 0, and if set, simply prevents the configure function from
executing.

Remove it.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index f86894622070..5f8044a7d602 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -931,7 +931,7 @@ static void dw_hdmi_phy_sel_interface_control(struct dw_hdmi *hdmi, u8 enable)
 			 HDMI_PHY_CONF0_SELDIPIF_MASK);
 }
 
-static int hdmi_phy_configure(struct dw_hdmi *hdmi, unsigned char prep,
+static int hdmi_phy_configure(struct dw_hdmi *hdmi,
 			      unsigned char res, int cscon)
 {
 	unsigned res_idx;
@@ -941,9 +941,6 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, unsigned char prep,
 	const struct dw_hdmi_curr_ctrl *curr_ctrl = pdata->cur_ctr;
 	const struct dw_hdmi_phy_config *phy_config = pdata->phy_config;
 
-	if (prep)
-		return -EINVAL;
-
 	switch (res) {
 	case 0:	/* color resolution 0 is 8 bit colour depth */
 	case 8:
@@ -1072,7 +1069,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi)
 		dw_hdmi_phy_enable_powerdown(hdmi, true);
 
 		/* Enable CSC */
-		ret = hdmi_phy_configure(hdmi, 0, 8, cscon);
+		ret = hdmi_phy_configure(hdmi, 8, cscon);
 		if (ret)
 			return ret;
 	}
-- 
Regards,

Laurent Pinchart

[PATCH v3 03/20] drm: bridge: dw-hdmi: Remove unused function parameter

[Laurent Pinchart]

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

The 'prep' parameter passed to hdmi_phy_configure() is useless. It is
hardcoded as 0, and if set, simply prevents the configure function from
executing.

Remove it.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index f86894622070..5f8044a7d602 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -931,7 +931,7 @@ static void dw_hdmi_phy_sel_interface_control(struct dw_hdmi *hdmi, u8 enable)
 			 HDMI_PHY_CONF0_SELDIPIF_MASK);
 }
 
-static int hdmi_phy_configure(struct dw_hdmi *hdmi, unsigned char prep,
+static int hdmi_phy_configure(struct dw_hdmi *hdmi,
 			      unsigned char res, int cscon)
 {
 	unsigned res_idx;
@@ -941,9 +941,6 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, unsigned char prep,
 	const struct dw_hdmi_curr_ctrl *curr_ctrl = pdata->cur_ctr;
 	const struct dw_hdmi_phy_config *phy_config = pdata->phy_config;
 
-	if (prep)
-		return -EINVAL;
-
 	switch (res) {
 	case 0:	/* color resolution 0 is 8 bit colour depth */
 	case 8:
@@ -1072,7 +1069,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi)
 		dw_hdmi_phy_enable_powerdown(hdmi, true);
 
 		/* Enable CSC */
-		ret = hdmi_phy_configure(hdmi, 0, 8, cscon);
+		ret = hdmi_phy_configure(hdmi, 8, cscon);
 		if (ret)
 			return ret;
 	}
-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 04/20] drm: bridge: dw-hdmi: Embed drm_bridge in struct dw_hdmi

[Laurent Pinchart]

The drm_bridge instance is always needed, there's no point in allocating
it separately.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 5f8044a7d602..2c85b6c07a80 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -116,7 +116,7 @@ struct dw_hdmi_i2c {
 struct dw_hdmi {
 	struct drm_connector connector;
 	struct drm_encoder *encoder;
-	struct drm_bridge *bridge;
+	struct drm_bridge bridge;
 
 	struct platform_device *audio;
 	enum dw_hdmi_devtype dev_type;
@@ -1806,7 +1806,7 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
 	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
 		dev_dbg(hdmi->dev, "EVENT=%s\n",
 			phy_int_pol & HDMI_PHY_HPD ? "plugin" : "plugout");
-		drm_helper_hpd_irq_event(hdmi->bridge->dev);
+		drm_helper_hpd_irq_event(hdmi->bridge.dev);
 	}
 
 	hdmi_writeb(hdmi, intr_stat, HDMI_IH_PHY_STAT0);
@@ -1819,16 +1819,9 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
 static int dw_hdmi_register(struct drm_device *drm, struct dw_hdmi *hdmi)
 {
 	struct drm_encoder *encoder = hdmi->encoder;
-	struct drm_bridge *bridge;
+	struct drm_bridge *bridge = &hdmi->bridge;
 	int ret;
 
-	bridge = devm_kzalloc(drm->dev, sizeof(*bridge), GFP_KERNEL);
-	if (!bridge) {
-		DRM_ERROR("Failed to allocate drm bridge\n");
-		return -ENOMEM;
-	}
-
-	hdmi->bridge = bridge;
 	bridge->driver_private = hdmi;
 	bridge->funcs = &dw_hdmi_bridge_funcs;
 	ret = drm_bridge_attach(encoder, bridge, NULL);
-- 
Regards,

Laurent Pinchart

[PATCH v3 05/20] drm: bridge: dw-hdmi: Remove encoder field from struct dw_hdmi

[Laurent Pinchart]

The field isn't needed, remove it.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 2c85b6c07a80..ef10bb866b2f 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -115,7 +115,6 @@ struct dw_hdmi_i2c {
 
 struct dw_hdmi {
 	struct drm_connector connector;
-	struct drm_encoder *encoder;
 	struct drm_bridge bridge;
 
 	struct platform_device *audio;
@@ -1816,9 +1815,8 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int dw_hdmi_register(struct drm_device *drm, struct dw_hdmi *hdmi)
+static int dw_hdmi_register(struct drm_encoder *encoder, struct dw_hdmi *hdmi)
 {
-	struct drm_encoder *encoder = hdmi->encoder;
 	struct drm_bridge *bridge = &hdmi->bridge;
 	int ret;
 
@@ -1835,7 +1833,7 @@ static int dw_hdmi_register(struct drm_device *drm, struct dw_hdmi *hdmi)
 	drm_connector_helper_add(&hdmi->connector,
 				 &dw_hdmi_connector_helper_funcs);
 
-	drm_connector_init(drm, &hdmi->connector,
+	drm_connector_init(encoder->dev, &hdmi->connector,
 			   &dw_hdmi_connector_funcs,
 			   DRM_MODE_CONNECTOR_HDMIA);
 
@@ -1867,7 +1865,6 @@ int dw_hdmi_bind(struct device *dev, struct drm_encoder *encoder,
 	hdmi->dev = dev;
 	hdmi->dev_type = plat_data->dev_type;
 	hdmi->sample_rate = 48000;
-	hdmi->encoder = encoder;
 	hdmi->disabled = true;
 	hdmi->rxsense = true;
 	hdmi->phy_mask = (u8)~(HDMI_PHY_HPD | HDMI_PHY_RX_SENSE);
@@ -1980,7 +1977,7 @@ int dw_hdmi_bind(struct device *dev, struct drm_encoder *encoder,
 	if (ret)
 		goto err_iahb;
 
-	ret = dw_hdmi_register(encoder->dev, hdmi);
+	ret = dw_hdmi_register(encoder, hdmi);
 	if (ret)
 		goto err_iahb;
 
-- 
Regards,

Laurent Pinchart

[PATCH v3 05/20] drm: bridge: dw-hdmi: Remove encoder field from struct dw_hdmi

[Laurent Pinchart]

The field isn't needed, remove it.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 2c85b6c07a80..ef10bb866b2f 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -115,7 +115,6 @@ struct dw_hdmi_i2c {
 
 struct dw_hdmi {
 	struct drm_connector connector;
-	struct drm_encoder *encoder;
 	struct drm_bridge bridge;
 
 	struct platform_device *audio;
@@ -1816,9 +1815,8 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int dw_hdmi_register(struct drm_device *drm, struct dw_hdmi *hdmi)
+static int dw_hdmi_register(struct drm_encoder *encoder, struct dw_hdmi *hdmi)
 {
-	struct drm_encoder *encoder = hdmi->encoder;
 	struct drm_bridge *bridge = &hdmi->bridge;
 	int ret;
 
@@ -1835,7 +1833,7 @@ static int dw_hdmi_register(struct drm_device *drm, struct dw_hdmi *hdmi)
 	drm_connector_helper_add(&hdmi->connector,
 				 &dw_hdmi_connector_helper_funcs);
 
-	drm_connector_init(drm, &hdmi->connector,
+	drm_connector_init(encoder->dev, &hdmi->connector,
 			   &dw_hdmi_connector_funcs,
 			   DRM_MODE_CONNECTOR_HDMIA);
 
@@ -1867,7 +1865,6 @@ int dw_hdmi_bind(struct device *dev, struct drm_encoder *encoder,
 	hdmi->dev = dev;
 	hdmi->dev_type = plat_data->dev_type;
 	hdmi->sample_rate = 48000;
-	hdmi->encoder = encoder;
 	hdmi->disabled = true;
 	hdmi->rxsense = true;
 	hdmi->phy_mask = (u8)~(HDMI_PHY_HPD | HDMI_PHY_RX_SENSE);
@@ -1980,7 +1977,7 @@ int dw_hdmi_bind(struct device *dev, struct drm_encoder *encoder,
 	if (ret)
 		goto err_iahb;
 
-	ret = dw_hdmi_register(encoder->dev, hdmi);
+	ret = dw_hdmi_register(encoder, hdmi);
 	if (ret)
 		goto err_iahb;
 
-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 06/20] drm: bridge: dw-hdmi: Don't forward HPD events to DRM core before attach

[Laurent Pinchart]

Hotplug events should only be forwarded to the DRM core by the interrupt
handler when the bridge has been attached, otherwise the DRM device
pointer will be NULL, resulting in a crash.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index ef10bb866b2f..488dc1a9204f 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1805,7 +1805,8 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
 	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
 		dev_dbg(hdmi->dev, "EVENT=%s\n",
 			phy_int_pol & HDMI_PHY_HPD ? "plugin" : "plugout");
-		drm_helper_hpd_irq_event(hdmi->bridge.dev);
+		if (hdmi->bridge.dev)
+			drm_helper_hpd_irq_event(hdmi->bridge.dev);
 	}
 
 	hdmi_writeb(hdmi, intr_stat, HDMI_IH_PHY_STAT0);
-- 
Regards,

Laurent Pinchart

[PATCH v3 07/20] drm: bridge: dw-hdmi: Move IRQ and IO resource allocation to common code

[Laurent Pinchart]

There's no need to duplicate identical code in multiple drivers (two at
the moment, one more to come soon). Move it to the dw-hdmi core where it
can be shared. If resource allocation ever becomes device-specific later
we'll always have the option of splitting it out again.

While it at pass the platform device to the bind function to avoid
having to cast struct device to struct platform_device.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c            | 13 ++++++++++---
 drivers/gpu/drm/imx/dw_hdmi-imx.c           | 12 +-----------
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 12 +-----------
 include/drm/bridge/dw_hdmi.h                |  3 +--
 4 files changed, 13 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 488dc1a9204f..563cbec47da6 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1843,14 +1843,16 @@ static int dw_hdmi_register(struct drm_encoder *encoder, struct dw_hdmi *hdmi)
 	return 0;
 }
 
-int dw_hdmi_bind(struct device *dev, struct drm_encoder *encoder,
-		 struct resource *iores, int irq,
+int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
 		 const struct dw_hdmi_plat_data *plat_data)
 {
+	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
 	struct platform_device_info pdevinfo;
 	struct device_node *ddc_node;
 	struct dw_hdmi *hdmi;
+	struct resource *iores;
+	int irq;
 	int ret;
 	u32 val = 1;
 	u8 config0;
@@ -1903,6 +1905,7 @@ int dw_hdmi_bind(struct device *dev, struct drm_encoder *encoder,
 		dev_dbg(hdmi->dev, "no ddc property found\n");
 	}
 
+	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	hdmi->regs = devm_ioremap_resource(dev, iores);
 	if (IS_ERR(hdmi->regs)) {
 		ret = PTR_ERR(hdmi->regs);
@@ -1945,6 +1948,10 @@ int dw_hdmi_bind(struct device *dev, struct drm_encoder *encoder,
 
 	initialize_hdmi_ih_mutes(hdmi);
 
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		goto err_iahb;
+
 	ret = devm_request_threaded_irq(dev, irq, dw_hdmi_hardirq,
 					dw_hdmi_irq, IRQF_SHARED,
 					dev_name(dev), hdmi);
@@ -2025,7 +2032,7 @@ int dw_hdmi_bind(struct device *dev, struct drm_encoder *encoder,
 	if (hdmi->i2c)
 		dw_hdmi_i2c_init(hdmi);
 
-	dev_set_drvdata(dev, hdmi);
+	platform_set_drvdata(pdev, hdmi);
 
 	return 0;
 
diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c b/drivers/gpu/drm/imx/dw_hdmi-imx.c
index f79665801543..f645275e6e63 100644
--- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
+++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
@@ -207,8 +207,6 @@ static int dw_hdmi_imx_bind(struct device *dev, struct device *master,
 	struct drm_device *drm = data;
 	struct drm_encoder *encoder;
 	struct imx_hdmi *hdmi;
-	struct resource *iores;
-	int irq;
 	int ret;
 
 	if (!pdev->dev.of_node)
@@ -223,14 +221,6 @@ static int dw_hdmi_imx_bind(struct device *dev, struct device *master,
 	hdmi->dev = &pdev->dev;
 	encoder = &hdmi->encoder;
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
-
-	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!iores)
-		return -ENXIO;
-
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
 	/*
 	 * If we failed to find the CRTC(s) which this encoder is
@@ -249,7 +239,7 @@ static int dw_hdmi_imx_bind(struct device *dev, struct device *master,
 	drm_encoder_init(drm, encoder, &dw_hdmi_imx_encoder_funcs,
 			 DRM_MODE_ENCODER_TMDS, NULL);
 
-	ret = dw_hdmi_bind(dev, encoder, iores, irq, plat_data);
+	ret = dw_hdmi_bind(pdev, encoder, plat_data);
 
 	/*
 	 * If dw_hdmi_bind() fails we'll never call dw_hdmi_unbind(),
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index e8fb5c56b224..a6d4a0236e8f 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -257,8 +257,6 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
 	struct drm_device *drm = data;
 	struct drm_encoder *encoder;
 	struct rockchip_hdmi *hdmi;
-	struct resource *iores;
-	int irq;
 	int ret;
 
 	if (!pdev->dev.of_node)
@@ -273,14 +271,6 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
 	hdmi->dev = &pdev->dev;
 	encoder = &hdmi->encoder;
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
-
-	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!iores)
-		return -ENXIO;
-
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
 	/*
 	 * If we failed to find the CRTC(s) which this encoder is
@@ -301,7 +291,7 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
 	drm_encoder_init(drm, encoder, &dw_hdmi_rockchip_encoder_funcs,
 			 DRM_MODE_ENCODER_TMDS, NULL);
 
-	ret = dw_hdmi_bind(dev, encoder, iores, irq, plat_data);
+	ret = dw_hdmi_bind(pdev, encoder, plat_data);
 
 	/*
 	 * If dw_hdmi_bind() fails we'll never call dw_hdmi_unbind(),
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index 11edda631a9d..94ff6edd070b 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -57,8 +57,7 @@ struct dw_hdmi_plat_data {
 };
 
 void dw_hdmi_unbind(struct device *dev);
-int dw_hdmi_bind(struct device *dev, struct drm_encoder *encoder,
-		 struct resource *iores, int irq,
+int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
 		 const struct dw_hdmi_plat_data *plat_data);
 
 void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);
-- 
Regards,

Laurent Pinchart

[PATCH v3 08/20] drm: bridge: dw-hdmi: Reorder functions to prepare for next commit

[Laurent Pinchart]

The next commit will reference structures and functions in a way that
currently requires forward declarations. Reorder the functions to avoid
that. No functional change to the code is performed.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 72 ++++++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 563cbec47da6..92ce9e532603 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1575,42 +1575,6 @@ static void dw_hdmi_update_phy_mask(struct dw_hdmi *hdmi)
 		hdmi_writeb(hdmi, hdmi->phy_mask, HDMI_PHY_MASK0);
 }
 
-static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge,
-				    struct drm_display_mode *orig_mode,
-				    struct drm_display_mode *mode)
-{
-	struct dw_hdmi *hdmi = bridge->driver_private;
-
-	mutex_lock(&hdmi->mutex);
-
-	/* Store the display mode for plugin/DKMS poweron events */
-	memcpy(&hdmi->previous_mode, mode, sizeof(hdmi->previous_mode));
-
-	mutex_unlock(&hdmi->mutex);
-}
-
-static void dw_hdmi_bridge_disable(struct drm_bridge *bridge)
-{
-	struct dw_hdmi *hdmi = bridge->driver_private;
-
-	mutex_lock(&hdmi->mutex);
-	hdmi->disabled = true;
-	dw_hdmi_update_power(hdmi);
-	dw_hdmi_update_phy_mask(hdmi);
-	mutex_unlock(&hdmi->mutex);
-}
-
-static void dw_hdmi_bridge_enable(struct drm_bridge *bridge)
-{
-	struct dw_hdmi *hdmi = bridge->driver_private;
-
-	mutex_lock(&hdmi->mutex);
-	hdmi->disabled = false;
-	dw_hdmi_update_power(hdmi);
-	dw_hdmi_update_phy_mask(hdmi);
-	mutex_unlock(&hdmi->mutex);
-}
-
 static enum drm_connector_status
 dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
 {
@@ -1703,6 +1667,42 @@ static const struct drm_connector_helper_funcs dw_hdmi_connector_helper_funcs =
 	.best_encoder = drm_atomic_helper_best_encoder,
 };
 
+static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge,
+				    struct drm_display_mode *orig_mode,
+				    struct drm_display_mode *mode)
+{
+	struct dw_hdmi *hdmi = bridge->driver_private;
+
+	mutex_lock(&hdmi->mutex);
+
+	/* Store the display mode for plugin/DKMS poweron events */
+	memcpy(&hdmi->previous_mode, mode, sizeof(hdmi->previous_mode));
+
+	mutex_unlock(&hdmi->mutex);
+}
+
+static void dw_hdmi_bridge_disable(struct drm_bridge *bridge)
+{
+	struct dw_hdmi *hdmi = bridge->driver_private;
+
+	mutex_lock(&hdmi->mutex);
+	hdmi->disabled = true;
+	dw_hdmi_update_power(hdmi);
+	dw_hdmi_update_phy_mask(hdmi);
+	mutex_unlock(&hdmi->mutex);
+}
+
+static void dw_hdmi_bridge_enable(struct drm_bridge *bridge)
+{
+	struct dw_hdmi *hdmi = bridge->driver_private;
+
+	mutex_lock(&hdmi->mutex);
+	hdmi->disabled = false;
+	dw_hdmi_update_power(hdmi);
+	dw_hdmi_update_phy_mask(hdmi);
+	mutex_unlock(&hdmi->mutex);
+}
+
 static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = {
 	.enable = dw_hdmi_bridge_enable,
 	.disable = dw_hdmi_bridge_disable,
-- 
Regards,

Laurent Pinchart

[PATCH v3 08/20] drm: bridge: dw-hdmi: Reorder functions to prepare for next commit

[Laurent Pinchart]

The next commit will reference structures and functions in a way that
currently requires forward declarations. Reorder the functions to avoid
that. No functional change to the code is performed.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 72 ++++++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 563cbec47da6..92ce9e532603 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1575,42 +1575,6 @@ static void dw_hdmi_update_phy_mask(struct dw_hdmi *hdmi)
 		hdmi_writeb(hdmi, hdmi->phy_mask, HDMI_PHY_MASK0);
 }
 
-static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge,
-				    struct drm_display_mode *orig_mode,
-				    struct drm_display_mode *mode)
-{
-	struct dw_hdmi *hdmi = bridge->driver_private;
-
-	mutex_lock(&hdmi->mutex);
-
-	/* Store the display mode for plugin/DKMS poweron events */
-	memcpy(&hdmi->previous_mode, mode, sizeof(hdmi->previous_mode));
-
-	mutex_unlock(&hdmi->mutex);
-}
-
-static void dw_hdmi_bridge_disable(struct drm_bridge *bridge)
-{
-	struct dw_hdmi *hdmi = bridge->driver_private;
-
-	mutex_lock(&hdmi->mutex);
-	hdmi->disabled = true;
-	dw_hdmi_update_power(hdmi);
-	dw_hdmi_update_phy_mask(hdmi);
-	mutex_unlock(&hdmi->mutex);
-}
-
-static void dw_hdmi_bridge_enable(struct drm_bridge *bridge)
-{
-	struct dw_hdmi *hdmi = bridge->driver_private;
-
-	mutex_lock(&hdmi->mutex);
-	hdmi->disabled = false;
-	dw_hdmi_update_power(hdmi);
-	dw_hdmi_update_phy_mask(hdmi);
-	mutex_unlock(&hdmi->mutex);
-}
-
 static enum drm_connector_status
 dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
 {
@@ -1703,6 +1667,42 @@ static const struct drm_connector_helper_funcs dw_hdmi_connector_helper_funcs =
 	.best_encoder = drm_atomic_helper_best_encoder,
 };
 
+static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge,
+				    struct drm_display_mode *orig_mode,
+				    struct drm_display_mode *mode)
+{
+	struct dw_hdmi *hdmi = bridge->driver_private;
+
+	mutex_lock(&hdmi->mutex);
+
+	/* Store the display mode for plugin/DKMS poweron events */
+	memcpy(&hdmi->previous_mode, mode, sizeof(hdmi->previous_mode));
+
+	mutex_unlock(&hdmi->mutex);
+}
+
+static void dw_hdmi_bridge_disable(struct drm_bridge *bridge)
+{
+	struct dw_hdmi *hdmi = bridge->driver_private;
+
+	mutex_lock(&hdmi->mutex);
+	hdmi->disabled = true;
+	dw_hdmi_update_power(hdmi);
+	dw_hdmi_update_phy_mask(hdmi);
+	mutex_unlock(&hdmi->mutex);
+}
+
+static void dw_hdmi_bridge_enable(struct drm_bridge *bridge)
+{
+	struct dw_hdmi *hdmi = bridge->driver_private;
+
+	mutex_lock(&hdmi->mutex);
+	hdmi->disabled = false;
+	dw_hdmi_update_power(hdmi);
+	dw_hdmi_update_phy_mask(hdmi);
+	mutex_unlock(&hdmi->mutex);
+}
+
 static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = {
 	.enable = dw_hdmi_bridge_enable,
 	.disable = dw_hdmi_bridge_disable,
-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 09/20] drm: bridge: dw-hdmi: Create connector in the bridge attach operation

[Laurent Pinchart]

The DRM device is not guaranteed by the bridge API to be available
before the attach callback. The driver performs properly at the moment
as it doesn't use the drm_bridge_add() registration method. As this will
be changed later, move connector creation to attach time to ensure
compatibility with the API.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 92ce9e532603..88cd40adb977 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1667,6 +1667,25 @@ static const struct drm_connector_helper_funcs dw_hdmi_connector_helper_funcs =
 	.best_encoder = drm_atomic_helper_best_encoder,
 };
 
+static int dw_hdmi_bridge_attach(struct drm_bridge *bridge)
+{
+	struct dw_hdmi *hdmi = bridge->driver_private;
+	struct drm_encoder *encoder = bridge->encoder;
+	struct drm_connector *connector = &hdmi->connector;
+
+	connector->interlace_allowed = 1;
+	connector->polled = DRM_CONNECTOR_POLL_HPD;
+
+	drm_connector_helper_add(connector, &dw_hdmi_connector_helper_funcs);
+
+	drm_connector_init(bridge->dev, connector, &dw_hdmi_connector_funcs,
+			   DRM_MODE_CONNECTOR_HDMIA);
+
+	drm_mode_connector_attach_encoder(connector, encoder);
+
+	return 0;
+}
+
 static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge,
 				    struct drm_display_mode *orig_mode,
 				    struct drm_display_mode *mode)
@@ -1704,6 +1723,7 @@ static void dw_hdmi_bridge_enable(struct drm_bridge *bridge)
 }
 
 static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = {
+	.attach = dw_hdmi_bridge_attach,
 	.enable = dw_hdmi_bridge_enable,
 	.disable = dw_hdmi_bridge_disable,
 	.mode_set = dw_hdmi_bridge_mode_set,
@@ -1829,17 +1849,6 @@ static int dw_hdmi_register(struct drm_encoder *encoder, struct dw_hdmi *hdmi)
 		return -EINVAL;
 	}
 
-	hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD;
-
-	drm_connector_helper_add(&hdmi->connector,
-				 &dw_hdmi_connector_helper_funcs);
-
-	drm_connector_init(encoder->dev, &hdmi->connector,
-			   &dw_hdmi_connector_funcs,
-			   DRM_MODE_CONNECTOR_HDMIA);
-
-	drm_mode_connector_attach_encoder(&hdmi->connector, encoder);
-
 	return 0;
 }
 
@@ -1862,8 +1871,6 @@ int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
 	if (!hdmi)
 		return -ENOMEM;
 
-	hdmi->connector.interlace_allowed = 1;
-
 	hdmi->plat_data = plat_data;
 	hdmi->dev = dev;
 	hdmi->dev_type = plat_data->dev_type;
-- 
Regards,

Laurent Pinchart

[PATCH v3 10/20] drm: bridge: dw-hdmi: Implement DRM bridge registration

[Laurent Pinchart]

As an option for drivers not based on the component framework, register
the bridge with the DRM core with the DRM bridge API. Existing drivers
based on dw_hdmi_bind() and dw_hdmi_unbind() are not affected as those
functions are preserved with their current behaviour.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 112 ++++++++++++++++++++++++++++-----------
 include/drm/bridge/dw_hdmi.h     |   3 ++
 2 files changed, 83 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 88cd40adb977..107fea49c4c6 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1836,24 +1836,9 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int dw_hdmi_register(struct drm_encoder *encoder, struct dw_hdmi *hdmi)
-{
-	struct drm_bridge *bridge = &hdmi->bridge;
-	int ret;
-
-	bridge->driver_private = hdmi;
-	bridge->funcs = &dw_hdmi_bridge_funcs;
-	ret = drm_bridge_attach(encoder, bridge, NULL);
-	if (ret) {
-		DRM_ERROR("Failed to initialize bridge with drm\n");
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
-int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
-		 const struct dw_hdmi_plat_data *plat_data)
+static struct dw_hdmi *
+__dw_hdmi_probe(struct platform_device *pdev,
+		const struct dw_hdmi_plat_data *plat_data)
 {
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
@@ -1869,7 +1854,7 @@ int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
 
 	hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL);
 	if (!hdmi)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	hdmi->plat_data = plat_data;
 	hdmi->dev = dev;
@@ -1896,7 +1881,7 @@ int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
 		break;
 	default:
 		dev_err(dev, "reg-io-width must be 1 or 4\n");
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
@@ -1905,7 +1890,7 @@ int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
 		of_node_put(ddc_node);
 		if (!hdmi->ddc) {
 			dev_dbg(hdmi->dev, "failed to read ddc node\n");
-			return -EPROBE_DEFER;
+			return ERR_PTR(-EPROBE_DEFER);
 		}
 
 	} else {
@@ -1956,8 +1941,10 @@ int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
 	initialize_hdmi_ih_mutes(hdmi);
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
+	if (irq < 0) {
+		ret = irq;
 		goto err_iahb;
+	}
 
 	ret = devm_request_threaded_irq(dev, irq, dw_hdmi_hardirq,
 					dw_hdmi_irq, IRQF_SHARED,
@@ -1988,11 +1975,11 @@ int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
 	hdmi_writeb(hdmi, HDMI_IH_PHY_STAT0_HPD | HDMI_IH_PHY_STAT0_RX_SENSE,
 		    HDMI_IH_PHY_STAT0);
 
-	ret = dw_hdmi_fb_registered(hdmi);
-	if (ret)
-		goto err_iahb;
+	hdmi->bridge.driver_private = hdmi;
+	hdmi->bridge.funcs = &dw_hdmi_bridge_funcs;
+	hdmi->bridge.of_node = pdev->dev.of_node;
 
-	ret = dw_hdmi_register(encoder, hdmi);
+	ret = dw_hdmi_fb_registered(hdmi);
 	if (ret)
 		goto err_iahb;
 
@@ -2041,7 +2028,7 @@ int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
 
 	platform_set_drvdata(pdev, hdmi);
 
-	return 0;
+	return hdmi;
 
 err_iahb:
 	if (hdmi->i2c) {
@@ -2055,14 +2042,11 @@ int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
 err_res:
 	i2c_put_adapter(hdmi->ddc);
 
-	return ret;
+	return ERR_PTR(ret);
 }
-EXPORT_SYMBOL_GPL(dw_hdmi_bind);
 
-void dw_hdmi_unbind(struct device *dev)
+static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
 {
-	struct dw_hdmi *hdmi = dev_get_drvdata(dev);
-
 	if (hdmi->audio && !IS_ERR(hdmi->audio))
 		platform_device_unregister(hdmi->audio);
 
@@ -2077,6 +2061,70 @@ void dw_hdmi_unbind(struct device *dev)
 	else
 		i2c_put_adapter(hdmi->ddc);
 }
+
+/* -----------------------------------------------------------------------------
+ * Probe/remove API, used from platforms based on the DRM bridge API.
+ */
+int dw_hdmi_probe(struct platform_device *pdev,
+		  const struct dw_hdmi_plat_data *plat_data)
+{
+	struct dw_hdmi *hdmi;
+	int ret;
+
+	hdmi = __dw_hdmi_probe(pdev, plat_data);
+	if (IS_ERR(hdmi))
+		return PTR_ERR(hdmi);
+
+	ret = drm_bridge_add(&hdmi->bridge);
+	if (ret < 0) {
+		__dw_hdmi_remove(hdmi);
+		return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_probe);
+
+void dw_hdmi_remove(struct platform_device *pdev)
+{
+	struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
+
+	drm_bridge_remove(&hdmi->bridge);
+
+	__dw_hdmi_remove(hdmi);
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_remove);
+
+/* -----------------------------------------------------------------------------
+ * Bind/unbind API, used from platforms based on the component framework.
+ */
+int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
+		 const struct dw_hdmi_plat_data *plat_data)
+{
+	struct dw_hdmi *hdmi;
+	int ret;
+
+	hdmi = __dw_hdmi_probe(pdev, plat_data);
+	if (IS_ERR(hdmi))
+		return PTR_ERR(hdmi);
+
+	ret = drm_bridge_attach(encoder, &hdmi->bridge, NULL);
+	if (ret) {
+		dw_hdmi_remove(pdev);
+		DRM_ERROR("Failed to initialize bridge with drm\n");
+		return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_bind);
+
+void dw_hdmi_unbind(struct device *dev)
+{
+	struct dw_hdmi *hdmi = dev_get_drvdata(dev);
+
+	__dw_hdmi_remove(hdmi);
+}
 EXPORT_SYMBOL_GPL(dw_hdmi_unbind);
 
 MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index 94ff6edd070b..3bb22a849830 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -56,6 +56,9 @@ struct dw_hdmi_plat_data {
 					   struct drm_display_mode *mode);
 };
 
+int dw_hdmi_probe(struct platform_device *pdev,
+		  const struct dw_hdmi_plat_data *plat_data);
+void dw_hdmi_remove(struct platform_device *pdev);
 void dw_hdmi_unbind(struct device *dev);
 int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
 		 const struct dw_hdmi_plat_data *plat_data);
-- 
Regards,

Laurent Pinchart

[PATCH v3 11/20] drm: bridge: dw-hdmi: Remove PHY configuration resolution parameter

[Laurent Pinchart]

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

The current code hard codes the call of hdmi_phy_configure() to be 8bpp
and provides extraneous error checking to verify that this hardcoded
value is correct. Simplify the implementation by removing the argument.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 27 +++++----------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 107fea49c4c6..b4fb0bd78910 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -930,31 +930,14 @@ static void dw_hdmi_phy_sel_interface_control(struct dw_hdmi *hdmi, u8 enable)
 			 HDMI_PHY_CONF0_SELDIPIF_MASK);
 }
 
-static int hdmi_phy_configure(struct dw_hdmi *hdmi,
-			      unsigned char res, int cscon)
+static int hdmi_phy_configure(struct dw_hdmi *hdmi, int cscon)
 {
-	unsigned res_idx;
 	u8 val, msec;
 	const struct dw_hdmi_plat_data *pdata = hdmi->plat_data;
 	const struct dw_hdmi_mpll_config *mpll_config = pdata->mpll_cfg;
 	const struct dw_hdmi_curr_ctrl *curr_ctrl = pdata->cur_ctr;
 	const struct dw_hdmi_phy_config *phy_config = pdata->phy_config;
 
-	switch (res) {
-	case 0:	/* color resolution 0 is 8 bit colour depth */
-	case 8:
-		res_idx = DW_HDMI_RES_8;
-		break;
-	case 10:
-		res_idx = DW_HDMI_RES_10;
-		break;
-	case 12:
-		res_idx = DW_HDMI_RES_12;
-		break;
-	default:
-		return -EINVAL;
-	}
-
 	/* PLL/MPLL Cfg - always match on final entry */
 	for (; mpll_config->mpixelclock != ~0UL; mpll_config++)
 		if (hdmi->hdmi_data.video_mode.mpixelclock <=
@@ -1004,11 +987,11 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi,
 		    HDMI_PHY_I2CM_SLAVE_ADDR);
 	hdmi_phy_test_clear(hdmi, 0);
 
-	hdmi_phy_i2c_write(hdmi, mpll_config->res[res_idx].cpce, 0x06);
-	hdmi_phy_i2c_write(hdmi, mpll_config->res[res_idx].gmp, 0x15);
+	hdmi_phy_i2c_write(hdmi, mpll_config->res[0].cpce, 0x06);
+	hdmi_phy_i2c_write(hdmi, mpll_config->res[0].gmp, 0x15);
 
 	/* CURRCTRL */
-	hdmi_phy_i2c_write(hdmi, curr_ctrl->curr[res_idx], 0x10);
+	hdmi_phy_i2c_write(hdmi, curr_ctrl->curr[0], 0x10);
 
 	hdmi_phy_i2c_write(hdmi, 0x0000, 0x13);  /* PLLPHBYCTRL */
 	hdmi_phy_i2c_write(hdmi, 0x0006, 0x17);
@@ -1068,7 +1051,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi)
 		dw_hdmi_phy_enable_powerdown(hdmi, true);
 
 		/* Enable CSC */
-		ret = hdmi_phy_configure(hdmi, 8, cscon);
+		ret = hdmi_phy_configure(hdmi, cscon);
 		if (ret)
 			return ret;
 	}
-- 
Regards,

Laurent Pinchart

[PATCH v3 12/20] drm: bridge: dw-hdmi: Rename CONF0 SPARECTRL bit to SVSRET

[Laurent Pinchart]

The bit is documented in a Rockchip BSP as

 #define m_SVSRET_SIG		(1 << 5) /* depend on PHY_MHL_COMB0=1 */

This is confirmed by a Renesas platform, which uses a 2.0 DWC HDMI TX as
the RK3288. Rename the bit accordingly.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 8 ++++----
 drivers/gpu/drm/bridge/dw-hdmi.h | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index b4fb0bd78910..06c252f560ad 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -895,11 +895,11 @@ static void dw_hdmi_phy_enable_tmds(struct dw_hdmi *hdmi, u8 enable)
 			 HDMI_PHY_CONF0_ENTMDS_MASK);
 }
 
-static void dw_hdmi_phy_enable_spare(struct dw_hdmi *hdmi, u8 enable)
+static void dw_hdmi_phy_enable_svsret(struct dw_hdmi *hdmi, u8 enable)
 {
 	hdmi_mask_writeb(hdmi, enable, HDMI_PHY_CONF0,
-			 HDMI_PHY_CONF0_SPARECTRL_OFFSET,
-			 HDMI_PHY_CONF0_SPARECTRL_MASK);
+			 HDMI_PHY_CONF0_SVSRET_OFFSET,
+			 HDMI_PHY_CONF0_SVSRET_MASK);
 }
 
 static void dw_hdmi_phy_gen2_pddq(struct dw_hdmi *hdmi, u8 enable)
@@ -1014,7 +1014,7 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, int cscon)
 	dw_hdmi_phy_gen2_pddq(hdmi, 0);
 
 	if (hdmi->dev_type == RK3288_HDMI)
-		dw_hdmi_phy_enable_spare(hdmi, 1);
+		dw_hdmi_phy_enable_svsret(hdmi, 1);
 
 	/*Wait for PHY PLL lock */
 	msec = 5;
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.h b/drivers/gpu/drm/bridge/dw-hdmi.h
index 55135bbd0c16..08235aef2fa3 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.h
+++ b/drivers/gpu/drm/bridge/dw-hdmi.h
@@ -847,8 +847,8 @@ enum {
 	HDMI_PHY_CONF0_PDZ_OFFSET = 7,
 	HDMI_PHY_CONF0_ENTMDS_MASK = 0x40,
 	HDMI_PHY_CONF0_ENTMDS_OFFSET = 6,
-	HDMI_PHY_CONF0_SPARECTRL_MASK = 0x20,
-	HDMI_PHY_CONF0_SPARECTRL_OFFSET = 5,
+	HDMI_PHY_CONF0_SVSRET_MASK = 0x20,
+	HDMI_PHY_CONF0_SVSRET_OFFSET = 5,
 	HDMI_PHY_CONF0_GEN2_PDDQ_MASK = 0x10,
 	HDMI_PHY_CONF0_GEN2_PDDQ_OFFSET = 4,
 	HDMI_PHY_CONF0_GEN2_TXPWRON_MASK = 0x8,
-- 
Regards,

Laurent Pinchart

[PATCH v3 13/20] drm: bridge: dw-hdmi: Reject invalid product IDs

[Laurent Pinchart]

The DWC HDMI TX can be recognized by the two product identification
registers. If the registers don't read as expect the IP will be very
different than what the driver has been designed for, or will be
misconfigured in a way that makes it non-operational (invalid memory
address, incorrect clocks, ...). We should reject this situation with an
error.

While this isn't critical for proper operation with supported IPs at the
moment, the driver will soon gain automatic device-specific handling
based on runtime device identification. This change makes it easier to
implement that without having to default to a random guess in case the
device can't be identified.

While at it print a readable version number in the device identification
message instead of raw register values.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 25 +++++++++++++++++++------
 drivers/gpu/drm/bridge/dw-hdmi.h |  8 ++++++++
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 06c252f560ad..1809247745b8 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1832,6 +1832,9 @@ __dw_hdmi_probe(struct platform_device *pdev,
 	int irq;
 	int ret;
 	u32 val = 1;
+	u16 version;
+	u8 prod_id0;
+	u8 prod_id1;
 	u8 config0;
 	u8 config1;
 
@@ -1914,12 +1917,22 @@ __dw_hdmi_probe(struct platform_device *pdev,
 	}
 
 	/* Product and revision IDs */
-	dev_info(dev,
-		 "Detected HDMI controller 0x%x:0x%x:0x%x:0x%x\n",
-		 hdmi_readb(hdmi, HDMI_DESIGN_ID),
-		 hdmi_readb(hdmi, HDMI_REVISION_ID),
-		 hdmi_readb(hdmi, HDMI_PRODUCT_ID0),
-		 hdmi_readb(hdmi, HDMI_PRODUCT_ID1));
+	version = (hdmi_readb(hdmi, HDMI_DESIGN_ID) << 8)
+		| (hdmi_readb(hdmi, HDMI_REVISION_ID) << 0);
+	prod_id0 = hdmi_readb(hdmi, HDMI_PRODUCT_ID0);
+	prod_id1 = hdmi_readb(hdmi, HDMI_PRODUCT_ID1);
+
+	if (prod_id0 != HDMI_PRODUCT_ID0_HDMI_TX ||
+	    (prod_id1 & ~HDMI_PRODUCT_ID1_HDCP) != HDMI_PRODUCT_ID1_HDMI_TX) {
+		dev_err(dev, "Unsupported HDMI controller (%04x:%02x:%02x)\n",
+			version, prod_id0, prod_id1);
+		ret = -ENODEV;
+		goto err_iahb;
+	}
+
+	dev_info(dev, "Detected HDMI TX controller v%x.%03x %s HDCP\n",
+		 version >> 12, version & 0xfff,
+		 prod_id1 & HDMI_PRODUCT_ID1_HDCP ? "with" : "without");
 
 	initialize_hdmi_ih_mutes(hdmi);
 
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.h b/drivers/gpu/drm/bridge/dw-hdmi.h
index 08235aef2fa3..91d7fabbd6e5 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.h
+++ b/drivers/gpu/drm/bridge/dw-hdmi.h
@@ -545,6 +545,14 @@
 #define HDMI_I2CM_FS_SCL_LCNT_0_ADDR            0x7E12
 
 enum {
+/* PRODUCT_ID0 field values */
+	HDMI_PRODUCT_ID0_HDMI_TX = 0xa0,
+
+/* PRODUCT_ID1 field values */
+	HDMI_PRODUCT_ID1_HDCP = 0xc0,
+	HDMI_PRODUCT_ID1_HDMI_RX = 0x02,
+	HDMI_PRODUCT_ID1_HDMI_TX = 0x01,
+
 /* CONFIG0_ID field values */
 	HDMI_CONFIG0_I2S = 0x10,
 
-- 
Regards,

Laurent Pinchart

[PATCH v3 14/20] drm: bridge: dw-hdmi: Detect AHB audio DMA using correct register

[Laurent Pinchart]

Bit 0 in CONFIG1_ID tells whether the IP core uses an AHB slave
interface for control. The correct way to identify AHB audio DMA support
is through bit 1 in CONFIG3_ID.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 6 +++---
 drivers/gpu/drm/bridge/dw-hdmi.h | 4 ++++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 1809247745b8..730a7558d4d4 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1836,7 +1836,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
 	u8 prod_id0;
 	u8 prod_id1;
 	u8 config0;
-	u8 config1;
+	u8 config3;
 
 	hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL);
 	if (!hdmi)
@@ -1988,9 +1988,9 @@ __dw_hdmi_probe(struct platform_device *pdev,
 	pdevinfo.id = PLATFORM_DEVID_AUTO;
 
 	config0 = hdmi_readb(hdmi, HDMI_CONFIG0_ID);
-	config1 = hdmi_readb(hdmi, HDMI_CONFIG1_ID);
+	config3 = hdmi_readb(hdmi, HDMI_CONFIG3_ID);
 
-	if (config1 & HDMI_CONFIG1_AHB) {
+	if (config3 & HDMI_CONFIG3_AHBAUDDMA) {
 		struct dw_hdmi_audio_data audio;
 
 		audio.phys = iores->start;
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.h b/drivers/gpu/drm/bridge/dw-hdmi.h
index 91d7fabbd6e5..a4fd64a203c9 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.h
+++ b/drivers/gpu/drm/bridge/dw-hdmi.h
@@ -559,6 +559,10 @@ enum {
 /* CONFIG1_ID field values */
 	HDMI_CONFIG1_AHB = 0x01,
 
+/* CONFIG3_ID field values */
+	HDMI_CONFIG3_AHBAUDDMA = 0x02,
+	HDMI_CONFIG3_GPAUD = 0x01,
+
 /* IH_FC_INT2 field values */
 	HDMI_IH_FC_INT2_OVERFLOW_MASK = 0x03,
 	HDMI_IH_FC_INT2_LOW_PRIORITY_OVERFLOW = 0x02,
-- 
Regards,

Laurent Pinchart

[PATCH v3 15/20] drm: bridge: dw-hdmi: Handle overflow workaround based on device version

[Laurent Pinchart]

Use the device version queried at runtime instead of the device type
provided through platform data to handle the overflow workaround. This
will make support of other SoCs integrating the same HDMI TX controller
version easier.

Among the supported platforms only i.MX6DL and i.MX6Q have been
identified as needing the workaround. Disabling it on Rockchip RK3288
(which integrates a v2.00a controller) didn't produce any error or
artifact.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 46 ++++++++++++++++++++++++++++------------
 1 file changed, 33 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 730a7558d4d4..f4faa14213e5 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -117,8 +117,10 @@ struct dw_hdmi {
 	struct drm_connector connector;
 	struct drm_bridge bridge;
 
-	struct platform_device *audio;
 	enum dw_hdmi_devtype dev_type;
+	unsigned int version;
+
+	struct platform_device *audio;
 	struct device *dev;
 	struct clk *isfr_clk;
 	struct clk *iahb_clk;
@@ -1323,19 +1325,38 @@ static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi)
 /* Workaround to clear the overflow condition */
 static void dw_hdmi_clear_overflow(struct dw_hdmi *hdmi)
 {
-	int count;
+	unsigned int count;
+	unsigned int i;
 	u8 val;
 
-	/* TMDS software reset */
-	hdmi_writeb(hdmi, (u8)~HDMI_MC_SWRSTZ_TMDSSWRST_REQ, HDMI_MC_SWRSTZ);
+	/*
+	 * Under some circumstances the Frame Composer arithmetic unit can miss
+	 * an FC register write due to being busy processing the previous one.
+	 * The issue can be worked around by issuing a TMDS software reset and
+	 * then write one of the FC registers several times.
+	 *
+	 * The number of iterations matters and depends on the HDMI TX revision
+	 * (and possibly on the platform). So far only i.MX6Q (v1.30a) and
+	 * i.MX6DL (v1.31a) have been identified as needing the workaround, with
+	 * 4 and 1 iterations respectively.
+	 */
 
-	val = hdmi_readb(hdmi, HDMI_FC_INVIDCONF);
-	if (hdmi->dev_type == IMX6DL_HDMI) {
-		hdmi_writeb(hdmi, val, HDMI_FC_INVIDCONF);
+	switch (hdmi->version) {
+	case 0x130a:
+		count = 4;
+		break;
+	case 0x131a:
+		count = 1;
+		break;
+	default:
 		return;
 	}
 
-	for (count = 0; count < 4; count++)
+	/* TMDS software reset */
+	hdmi_writeb(hdmi, (u8)~HDMI_MC_SWRSTZ_TMDSSWRST_REQ, HDMI_MC_SWRSTZ);
+
+	val = hdmi_readb(hdmi, HDMI_FC_INVIDCONF);
+	for (i = 0; i < count; i++)
 		hdmi_writeb(hdmi, val, HDMI_FC_INVIDCONF);
 }
 
@@ -1832,7 +1853,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
 	int irq;
 	int ret;
 	u32 val = 1;
-	u16 version;
 	u8 prod_id0;
 	u8 prod_id1;
 	u8 config0;
@@ -1917,21 +1937,21 @@ __dw_hdmi_probe(struct platform_device *pdev,
 	}
 
 	/* Product and revision IDs */
-	version = (hdmi_readb(hdmi, HDMI_DESIGN_ID) << 8)
-		| (hdmi_readb(hdmi, HDMI_REVISION_ID) << 0);
+	hdmi->version = (hdmi_readb(hdmi, HDMI_DESIGN_ID) << 8)
+		      | (hdmi_readb(hdmi, HDMI_REVISION_ID) << 0);
 	prod_id0 = hdmi_readb(hdmi, HDMI_PRODUCT_ID0);
 	prod_id1 = hdmi_readb(hdmi, HDMI_PRODUCT_ID1);
 
 	if (prod_id0 != HDMI_PRODUCT_ID0_HDMI_TX ||
 	    (prod_id1 & ~HDMI_PRODUCT_ID1_HDCP) != HDMI_PRODUCT_ID1_HDMI_TX) {
 		dev_err(dev, "Unsupported HDMI controller (%04x:%02x:%02x)\n",
-			version, prod_id0, prod_id1);
+			hdmi->version, prod_id0, prod_id1);
 		ret = -ENODEV;
 		goto err_iahb;
 	}
 
 	dev_info(dev, "Detected HDMI TX controller v%x.%03x %s HDCP\n",
-		 version >> 12, version & 0xfff,
+		 hdmi->version >> 12, hdmi->version & 0xfff,
 		 prod_id1 & HDMI_PRODUCT_ID1_HDCP ? "with" : "without");
 
 	initialize_hdmi_ih_mutes(hdmi);
-- 
Regards,

Laurent Pinchart

[PATCH v3 16/20] drm: bridge: dw-hdmi: Detect PHY type at runtime

[Laurent Pinchart]

Detect the PHY type and use it to handle the PHY type-specific SVSRET
signal.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 68 ++++++++++++++++++++++++++++++++++++++--
 include/drm/bridge/dw_hdmi.h     | 10 ++++++
 2 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index f4faa14213e5..ef4f2f96ed2c 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -113,6 +113,12 @@ struct dw_hdmi_i2c {
 	bool			is_regaddr;
 };
 
+struct dw_hdmi_phy_data {
+	enum dw_hdmi_phy_type type;
+	const char *name;
+	bool has_svsret;
+};
+
 struct dw_hdmi {
 	struct drm_connector connector;
 	struct drm_bridge bridge;
@@ -134,7 +140,9 @@ struct dw_hdmi {
 	u8 edid[HDMI_EDID_LEN];
 	bool cable_plugin;
 
+	const struct dw_hdmi_phy_data *phy;
 	bool phy_enabled;
+
 	struct drm_display_mode previous_mode;
 
 	struct i2c_adapter *ddc;
@@ -1015,7 +1023,8 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, int cscon)
 	dw_hdmi_phy_gen2_txpwron(hdmi, 1);
 	dw_hdmi_phy_gen2_pddq(hdmi, 0);
 
-	if (hdmi->dev_type == RK3288_HDMI)
+	/* The DWC MHL and HDMI 2.0 PHYs need the SVSRET signal to be set. */
+	if (hdmi->phy->has_svsret)
 		dw_hdmi_phy_enable_svsret(hdmi, 1);
 
 	/*Wait for PHY PLL lock */
@@ -1840,6 +1849,54 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static const struct dw_hdmi_phy_data dw_hdmi_phys[] = {
+	{
+		.type = DW_HDMI_PHY_DWC_HDMI_TX_PHY,
+		.name = "DWC HDMI TX PHY",
+	}, {
+		.type = DW_HDMI_PHY_DWC_MHL_PHY_HEAC,
+		.name = "DWC MHL PHY + HEAC PHY",
+		.has_svsret = true,
+	}, {
+		.type = DW_HDMI_PHY_DWC_MHL_PHY,
+		.name = "DWC MHL PHY",
+		.has_svsret = true,
+	}, {
+		.type = DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY_HEAC,
+		.name = "DWC HDMI 3D TX PHY + HEAC PHY",
+	}, {
+		.type = DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY,
+		.name = "DWC HDMI 3D TX PHY",
+	}, {
+		.type = DW_HDMI_PHY_DWC_HDMI20_TX_PHY,
+		.name = "DWC HDMI 2.0 TX PHY",
+		.has_svsret = true,
+	}
+};
+
+static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi)
+{
+	unsigned int i;
+	u8 phy_type;
+
+	phy_type = hdmi_readb(hdmi, HDMI_CONFIG2_ID);
+
+	for (i = 0; i < ARRAY_SIZE(dw_hdmi_phys); ++i) {
+		if (dw_hdmi_phys[i].type == phy_type) {
+			hdmi->phy = &dw_hdmi_phys[i];
+			return 0;
+		}
+	}
+
+	if (phy_type == DW_HDMI_PHY_VENDOR_PHY)
+		dev_err(hdmi->dev, "Unsupported vendor HDMI PHY\n");
+	else
+		dev_err(hdmi->dev, "Unsupported HDMI PHY type (%02x)\n",
+			phy_type);
+
+	return -ENODEV;
+}
+
 static struct dw_hdmi *
 __dw_hdmi_probe(struct platform_device *pdev,
 		const struct dw_hdmi_plat_data *plat_data)
@@ -1950,9 +2007,14 @@ __dw_hdmi_probe(struct platform_device *pdev,
 		goto err_iahb;
 	}
 
-	dev_info(dev, "Detected HDMI TX controller v%x.%03x %s HDCP\n",
+	ret = dw_hdmi_detect_phy(hdmi);
+	if (ret < 0)
+		goto err_iahb;
+
+	dev_info(dev, "Detected HDMI TX controller v%x.%03x %s HDCP (%s)\n",
 		 hdmi->version >> 12, hdmi->version & 0xfff,
-		 prod_id1 & HDMI_PRODUCT_ID1_HDCP ? "with" : "without");
+		 prod_id1 & HDMI_PRODUCT_ID1_HDCP ? "with" : "without",
+		 hdmi->phy->name);
 
 	initialize_hdmi_ih_mutes(hdmi);
 
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index 3bb22a849830..b080a171a23f 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -27,6 +27,16 @@ enum dw_hdmi_devtype {
 	RK3288_HDMI,
 };
 
+enum dw_hdmi_phy_type {
+	DW_HDMI_PHY_DWC_HDMI_TX_PHY = 0x00,
+	DW_HDMI_PHY_DWC_MHL_PHY_HEAC = 0xb2,
+	DW_HDMI_PHY_DWC_MHL_PHY = 0xc2,
+	DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY_HEAC = 0xe2,
+	DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY = 0xf2,
+	DW_HDMI_PHY_DWC_HDMI20_TX_PHY = 0xf3,
+	DW_HDMI_PHY_VENDOR_PHY = 0xfe,
+};
+
 struct dw_hdmi_mpll_config {
 	unsigned long mpixelclock;
 	struct {
-- 
Regards,

Laurent Pinchart

[PATCH v3 17/20] drm: bridge: dw-hdmi: Define and use macros for PHY register addresses

[Laurent Pinchart]

Replace the hardcoded register address numerical values with macros to
clarify the code.

This change has been tested by comparing the assembly code before and
after the change.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 35 ++++++++++++---------
 drivers/gpu/drm/bridge/dw-hdmi.h | 66 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index ef4f2f96ed2c..6e605fd910ef 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -997,21 +997,26 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, int cscon)
 		    HDMI_PHY_I2CM_SLAVE_ADDR);
 	hdmi_phy_test_clear(hdmi, 0);
 
-	hdmi_phy_i2c_write(hdmi, mpll_config->res[0].cpce, 0x06);
-	hdmi_phy_i2c_write(hdmi, mpll_config->res[0].gmp, 0x15);
-
-	/* CURRCTRL */
-	hdmi_phy_i2c_write(hdmi, curr_ctrl->curr[0], 0x10);
-
-	hdmi_phy_i2c_write(hdmi, 0x0000, 0x13);  /* PLLPHBYCTRL */
-	hdmi_phy_i2c_write(hdmi, 0x0006, 0x17);
-
-	hdmi_phy_i2c_write(hdmi, phy_config->term, 0x19);  /* TXTERM */
-	hdmi_phy_i2c_write(hdmi, phy_config->sym_ctr, 0x09); /* CKSYMTXCTRL */
-	hdmi_phy_i2c_write(hdmi, phy_config->vlev_ctr, 0x0E); /* VLEVCTRL */
-
-	/* REMOVE CLK TERM */
-	hdmi_phy_i2c_write(hdmi, 0x8000, 0x05);  /* CKCALCTRL */
+	hdmi_phy_i2c_write(hdmi, mpll_config->res[0].cpce,
+			   HDMI_3D_TX_PHY_CPCE_CTRL);
+	hdmi_phy_i2c_write(hdmi, mpll_config->res[0].gmp,
+			   HDMI_3D_TX_PHY_GMPCTRL);
+	hdmi_phy_i2c_write(hdmi, curr_ctrl->curr[0],
+			   HDMI_3D_TX_PHY_CURRCTRL);
+
+	hdmi_phy_i2c_write(hdmi, 0, HDMI_3D_TX_PHY_PLLPHBYCTRL);
+	hdmi_phy_i2c_write(hdmi, HDMI_3D_TX_PHY_MSM_CTRL_CKO_SEL_FB_CLK,
+			   HDMI_3D_TX_PHY_MSM_CTRL);
+
+	hdmi_phy_i2c_write(hdmi, phy_config->term, HDMI_3D_TX_PHY_TXTERM);
+	hdmi_phy_i2c_write(hdmi, phy_config->sym_ctr,
+			   HDMI_3D_TX_PHY_CKSYMTXCTRL);
+	hdmi_phy_i2c_write(hdmi, phy_config->vlev_ctr,
+			   HDMI_3D_TX_PHY_VLEVCTRL);
+
+	/* Override and disable clock termination. */
+	hdmi_phy_i2c_write(hdmi, HDMI_3D_TX_PHY_CKCALCTRL_OVERRIDE,
+			   HDMI_3D_TX_PHY_CKCALCTRL);
 
 	dw_hdmi_phy_enable_powerdown(hdmi, false);
 
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.h b/drivers/gpu/drm/bridge/dw-hdmi.h
index a4fd64a203c9..f3c149c88d71 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.h
+++ b/drivers/gpu/drm/bridge/dw-hdmi.h
@@ -1085,4 +1085,70 @@ enum {
 	HDMI_I2CM_CTLINT_ARB_MASK = 0x4,
 };
 
+/*
+ * HDMI 3D TX PHY registers
+ */
+#define HDMI_3D_TX_PHY_PWRCTRL			0x00
+#define HDMI_3D_TX_PHY_SERDIVCTRL		0x01
+#define HDMI_3D_TX_PHY_SERCKCTRL		0x02
+#define HDMI_3D_TX_PHY_SERCKKILLCTRL		0x03
+#define HDMI_3D_TX_PHY_TXRESCTRL		0x04
+#define HDMI_3D_TX_PHY_CKCALCTRL		0x05
+#define HDMI_3D_TX_PHY_CPCE_CTRL		0x06
+#define HDMI_3D_TX_PHY_TXCLKMEASCTRL		0x07
+#define HDMI_3D_TX_PHY_TXMEASCTRL		0x08
+#define HDMI_3D_TX_PHY_CKSYMTXCTRL		0x09
+#define HDMI_3D_TX_PHY_CMPSEQCTRL		0x0a
+#define HDMI_3D_TX_PHY_CMPPWRCTRL		0x0b
+#define HDMI_3D_TX_PHY_CMPMODECTRL		0x0c
+#define HDMI_3D_TX_PHY_MEASCTRL			0x0d
+#define HDMI_3D_TX_PHY_VLEVCTRL			0x0e
+#define HDMI_3D_TX_PHY_D2ACTRL			0x0f
+#define HDMI_3D_TX_PHY_CURRCTRL			0x10
+#define HDMI_3D_TX_PHY_DRVANACTRL		0x11
+#define HDMI_3D_TX_PHY_PLLMEASCTRL		0x12
+#define HDMI_3D_TX_PHY_PLLPHBYCTRL		0x13
+#define HDMI_3D_TX_PHY_GRP_CTRL			0x14
+#define HDMI_3D_TX_PHY_GMPCTRL			0x15
+#define HDMI_3D_TX_PHY_MPLLMEASCTRL		0x16
+#define HDMI_3D_TX_PHY_MSM_CTRL			0x17
+#define HDMI_3D_TX_PHY_SCRPB_STATUS		0x18
+#define HDMI_3D_TX_PHY_TXTERM			0x19
+#define HDMI_3D_TX_PHY_PTRPT_ENBL		0x1a
+#define HDMI_3D_TX_PHY_PATTERNGEN		0x1b
+#define HDMI_3D_TX_PHY_SDCAP_MODE		0x1c
+#define HDMI_3D_TX_PHY_SCOPEMODE		0x1d
+#define HDMI_3D_TX_PHY_DIGTXMODE		0x1e
+#define HDMI_3D_TX_PHY_STR_STATUS		0x1f
+#define HDMI_3D_TX_PHY_SCOPECNT0		0x20
+#define HDMI_3D_TX_PHY_SCOPECNT1		0x21
+#define HDMI_3D_TX_PHY_SCOPECNT2		0x22
+#define HDMI_3D_TX_PHY_SCOPECNTCLK		0x23
+#define HDMI_3D_TX_PHY_SCOPESAMPLE		0x24
+#define HDMI_3D_TX_PHY_SCOPECNTMSB01		0x25
+#define HDMI_3D_TX_PHY_SCOPECNTMSB2CK		0x26
+
+/* HDMI_3D_TX_PHY_CKCALCTRL values */
+#define HDMI_3D_TX_PHY_CKCALCTRL_OVERRIDE		BIT(15)
+
+/* HDMI_3D_TX_PHY_MSM_CTRL values */
+#define HDMI_3D_TX_PHY_MSM_CTRL_MPLL_PH_SEL_CK		BIT(13)
+#define HDMI_3D_TX_PHY_MSM_CTRL_CKO_SEL_CLK_REF_MPLL	(0 << 1)
+#define HDMI_3D_TX_PHY_MSM_CTRL_CKO_SEL_OFF		(1 << 1)
+#define HDMI_3D_TX_PHY_MSM_CTRL_CKO_SEL_PCLK		(2 << 1)
+#define HDMI_3D_TX_PHY_MSM_CTRL_CKO_SEL_FB_CLK		(3 << 1)
+#define HDMI_3D_TX_PHY_MSM_CTRL_SCOPE_CK_SEL		BIT(0)
+
+/* HDMI_3D_TX_PHY_PTRPT_ENBL values */
+#define HDMI_3D_TX_PHY_PTRPT_ENBL_OVERRIDE		BIT(15)
+#define HDMI_3D_TX_PHY_PTRPT_ENBL_PG_SKIP_BIT2		BIT(8)
+#define HDMI_3D_TX_PHY_PTRPT_ENBL_PG_SKIP_BIT1		BIT(7)
+#define HDMI_3D_TX_PHY_PTRPT_ENBL_PG_SKIP_BIT0		BIT(6)
+#define HDMI_3D_TX_PHY_PTRPT_ENBL_CK_REF_ENB		BIT(5)
+#define HDMI_3D_TX_PHY_PTRPT_ENBL_RCAL_ENB		BIT(4)
+#define HDMI_3D_TX_PHY_PTRPT_ENBL_TX_CLK_ALIGN_ENB	BIT(3)
+#define HDMI_3D_TX_PHY_PTRPT_ENBL_TX_READY		BIT(2)
+#define HDMI_3D_TX_PHY_PTRPT_ENBL_CKO_WORD_ENB		BIT(1)
+#define HDMI_3D_TX_PHY_PTRPT_ENBL_REFCLK_ENB		BIT(0)
+
 #endif /* __DW_HDMI_H__ */
-- 
Regards,

Laurent Pinchart

[PATCH v3 18/20] drm: bridge: dw-hdmi: Fix the name of the PHY reset macros

[Laurent Pinchart]

The PHY reset signal is controlled by bit PHYRSTZ in the MC_PHYRSTZ
register. The signal is active low on Gen1 PHYs and active high on Gen2
PHYs. The driver toggles the signal high then low, which is correct for
all currently supported platforms, but the register values macros are
incorrectly named. Replace them with a single macro named after the bit,
and add a comment to the source code to explain the behaviour.

The driver's behaviour isn't changed by this rename, the code will still
need to be fixed to support Gen1 PHYs.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 6 +++---
 drivers/gpu/drm/bridge/dw-hdmi.h | 3 +--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 6e605fd910ef..93e8816f1f78 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -986,9 +986,9 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, int cscon)
 	/* gen2 pddq */
 	dw_hdmi_phy_gen2_pddq(hdmi, 1);
 
-	/* PHY reset */
-	hdmi_writeb(hdmi, HDMI_MC_PHYRSTZ_DEASSERT, HDMI_MC_PHYRSTZ);
-	hdmi_writeb(hdmi, HDMI_MC_PHYRSTZ_ASSERT, HDMI_MC_PHYRSTZ);
+	/* PHY reset. The reset signal is active high on Gen2 PHYs. */
+	hdmi_writeb(hdmi, HDMI_MC_PHYRSTZ_PHYRSTZ, HDMI_MC_PHYRSTZ);
+	hdmi_writeb(hdmi, 0, HDMI_MC_PHYRSTZ);
 
 	hdmi_writeb(hdmi, HDMI_MC_HEACPHY_RST_ASSERT, HDMI_MC_HEACPHY_RST);
 
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.h b/drivers/gpu/drm/bridge/dw-hdmi.h
index f3c149c88d71..325b0b8ae639 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.h
+++ b/drivers/gpu/drm/bridge/dw-hdmi.h
@@ -989,8 +989,7 @@ enum {
 	HDMI_MC_FLOWCTRL_FEED_THROUGH_OFF_CSC_BYPASS = 0x0,
 
 /* MC_PHYRSTZ field values */
-	HDMI_MC_PHYRSTZ_ASSERT = 0x0,
-	HDMI_MC_PHYRSTZ_DEASSERT = 0x1,
+	HDMI_MC_PHYRSTZ_PHYRSTZ = 0x01,
 
 /* MC_HEACPHY_RST field values */
 	HDMI_MC_HEACPHY_RST_ASSERT = 0x1,
-- 
Regards,

Laurent Pinchart

[PATCH v3 19/20] drm: bridge: dw-hdmi: Assert SVSRET before resetting the PHY

[Laurent Pinchart]

According to the PHY IP core vendor, the SVSRET signal must be asserted
before resetting the PHY. Tests on RK3288 and R-Car Gen3 showed no
regression, the change should thus be safe.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 93e8816f1f78..4fda0717e789 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -986,6 +986,10 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, int cscon)
 	/* gen2 pddq */
 	dw_hdmi_phy_gen2_pddq(hdmi, 1);
 
+	/* Leave low power consumption mode by asserting SVSRET. */
+	if (hdmi->phy->has_svsret)
+		dw_hdmi_phy_enable_svsret(hdmi, 1);
+
 	/* PHY reset. The reset signal is active high on Gen2 PHYs. */
 	hdmi_writeb(hdmi, HDMI_MC_PHYRSTZ_PHYRSTZ, HDMI_MC_PHYRSTZ);
 	hdmi_writeb(hdmi, 0, HDMI_MC_PHYRSTZ);
@@ -1028,11 +1032,7 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, int cscon)
 	dw_hdmi_phy_gen2_txpwron(hdmi, 1);
 	dw_hdmi_phy_gen2_pddq(hdmi, 0);
 
-	/* The DWC MHL and HDMI 2.0 PHYs need the SVSRET signal to be set. */
-	if (hdmi->phy->has_svsret)
-		dw_hdmi_phy_enable_svsret(hdmi, 1);
-
-	/*Wait for PHY PLL lock */
+	/* Wait for PHY PLL lock */
 	msec = 5;
 	do {
 		val = hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_TX_PHY_LOCK;
-- 
Regards,

Laurent Pinchart

[PATCH v3 20/20] dt-bindings: display: dw-hdmi: Clean up DT bindings documentation

[Laurent Pinchart]

Make it clear that the core bridge/dw_hdmi.txt document isn't a device
tree binding by itself but is meant to be referenced by platform device
tree bindings, and update the Rockchip and Freescale DWC HDMI TX
bindings to reference it.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/display/bridge/dw_hdmi.txt | 85 +++++++++-------------
 .../devicetree/bindings/display/imx/hdmi.txt       | 51 +++++++------
 .../bindings/display/rockchip/dw_hdmi-rockchip.txt | 43 +++++++----
 3 files changed, 91 insertions(+), 88 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt b/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
index 5e9a84d6e5f1..33bf981fbe33 100644
--- a/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
+++ b/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
@@ -1,52 +1,33 @@
-DesignWare HDMI bridge bindings
-
-Required properties:
-- compatible: platform specific such as:
-   * "snps,dw-hdmi-tx"
-   * "fsl,imx6q-hdmi"
-   * "fsl,imx6dl-hdmi"
-   * "rockchip,rk3288-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,
-  as described in Documentation/devicetree/bindings/clock/clock-bindings.txt,
-  the clocks are soc specific, the clock-names should be "iahb", "isfr"
--port@[X]: SoC specific port nodes with endpoint definitions as defined
-   in Documentation/devicetree/bindings/media/video-interfaces.txt,
-   please refer to the SoC specific binding document:
-    * Documentation/devicetree/bindings/display/imx/hdmi.txt
-    * Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
-
-Optional properties
-- reg-io-width: the width of the reg:1,4, default set to 1 if not present
-- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing,
-  if the property is omitted, a functionally reduced I2C bus
-  controller on DW HDMI is probed
-- clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec"
-
-Example:
-	hdmi: hdmi@0120000 {
-		compatible = "fsl,imx6q-hdmi";
-		reg = <0x00120000 0x9000>;
-		interrupts = <0 115 0x04>;
-		gpr = <&gpr>;
-		clocks = <&clks 123>, <&clks 124>;
-		clock-names = "iahb", "isfr";
-		ddc-i2c-bus = <&i2c2>;
-
-		port@0 {
-			reg = <0>;
-
-			hdmi_mux_0: endpoint {
-				remote-endpoint = <&ipu1_di0_hdmi>;
-			};
-		};
-
-		port@1 {
-			reg = <1>;
-
-			hdmi_mux_1: endpoint {
-				remote-endpoint = <&ipu1_di1_hdmi>;
-			};
-		};
-	};
+Synopsys DesignWare HDMI TX Encoder
+===================================
+
+This document defines device tree properties for the Synopsys DesignWare HDMI
+TX Encoder (DWC HDMI TX). It doesn't constitue a device tree binding
+specification by itself but is meant to be referenced by platform-specific
+device tree bindings.
+
+When referenced from platform device tree bindings the properties defined in
+this document are defined as follows. The platform device tree bindings are
+responsible for defining whether each property is required or optional.
+
+- reg: Memory mapped base address and length of the DWC HDMI TX registers.
+
+- reg-io-width: Width of the registers specified by the reg property. The
+  value is expressed in bytes and must be equal to 1 or 4 if specified. The
+  register width defaults to 1 if the property is not present.
+
+- interrupts: Reference to the DWC HDMI TX interrupt.
+
+- clocks: References to all the clocks specified in the clock-names property
+  as specified in Documentation/devicetree/bindings/clock/clock-bindings.txt.
+
+- clock-names: The DWC HDMI TX uses the following clocks.
+
+  - "iahb" is the bus clock for either AHB and APB (mandatory).
+  - "isfr" is the internal register configuration clock (mandatory).
+  - "cec" is the HDMI CEC controller main clock (optional).
+
+- ports: The connectivity of the DWC HDMI TX with the rest of the system is
+  expressed in using ports as specified in the device graph bindings defined
+  in Documentation/devicetree/bindings/graph.txt. The numbering of the ports
+  is platform-specific.
diff --git a/Documentation/devicetree/bindings/display/imx/hdmi.txt b/Documentation/devicetree/bindings/display/imx/hdmi.txt
index 1b756cf9afb0..66a8f86e5d12 100644
--- a/Documentation/devicetree/bindings/display/imx/hdmi.txt
+++ b/Documentation/devicetree/bindings/display/imx/hdmi.txt
@@ -1,29 +1,36 @@
-Device-Tree bindings for HDMI Transmitter
+Freescale i.MX6 DWC HDMI TX Encoder
+===================================
 
-HDMI Transmitter
-================
+The HDMI transmitter is a Synopsys DesignWare HDMI 1.4 TX controller IP
+with a companion PHY IP.
+
+These DT bindings follow the Synopsys DWC HDMI TX bindings defined in
+Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt with the
+following device-specific properties.
 
-The HDMI Transmitter is a Synopsys DesignWare HDMI 1.4 TX controller IP
-with accompanying PHY IP.
 
 Required properties:
- - #address-cells : should be <1>
- - #size-cells : should be <0>
- - compatible : should be "fsl,imx6q-hdmi" or "fsl,imx6dl-hdmi".
- - gpr : should be <&gpr>.
-   The phandle points to the iomuxc-gpr region containing the HDMI
-   multiplexer control register.
- - clocks, clock-names : phandles to the HDMI iahb and isrf clocks, as described
-   in Documentation/devicetree/bindings/clock/clock-bindings.txt and
-   Documentation/devicetree/bindings/clock/imx6q-clock.txt.
- - port@[0-4]: Up to four port nodes with endpoint definitions as defined in
-   Documentation/devicetree/bindings/media/video-interfaces.txt,
-   corresponding to the four inputs to the HDMI multiplexer.
-
-Optional properties:
- - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
-
-example:
+
+- compatible : Shall be one of "fsl,imx6q-hdmi" or "fsl,imx6dl-hdmi".
+- reg: See dw_hdmi.txt.
+- interrupts: HDMI interrupt number
+- clocks: See dw_hdmi.txt.
+- clock-names: Shall contain "iahb" and "isfr" as defined in dw_hdmi.txt.
+- ports: See dw_hdmi.txt. The DWC HDMI shall have between one and four ports,
+  numbered 0 to 3, corresponding to the four inputs of the HDMI multiplexer.
+  Each port shall have a single endpoint.
+- gpr : Shall contain a phandle to the iomuxc-gpr region containing the HDMI
+  multiplexer control register.
+
+Optional properties
+
+- ddc-i2c-bus: The HDMI DDC bus can be connected to either a system I2C master
+  or the functionally-reduced I2C master contained in the DWC HDMI. When
+  connected to a system I2C master this property contains a phandle to that
+  I2C master controller.
+
+
+Example:
 
 	gpr: iomuxc-gpr@020e0000 {
 		/* ... */
diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
index 668091f27674..046076c6b277 100644
--- a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt
@@ -1,24 +1,39 @@
-Rockchip specific extensions to the Synopsys Designware HDMI
-================================
+Rockchip DWC HDMI TX Encoder
+============================
+
+The HDMI transmitter is a Synopsys DesignWare HDMI 1.4 TX controller IP
+with a companion PHY IP.
+
+These DT bindings follow the Synopsys DWC HDMI TX bindings defined in
+Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt with the
+following device-specific properties.
+
 
 Required properties:
-- compatible: "rockchip,rk3288-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"
-- rockchip,grf: this soc should set GRF regs to mux vopl/vopb.
+
+- compatible: Shall contain "rockchip,rk3288-dw-hdmi".
+- reg: See dw_hdmi.txt.
+- reg-io-width: See dw_hdmi.txt. Shall be 4.
 - interrupts: HDMI interrupt number
-- ports: contain a port node with endpoint definitions as defined in
-  Documentation/devicetree/bindings/media/video-interfaces.txt. For
-  vopb,set the reg = <0> and set the reg = <1> for vopl.
-- reg-io-width: the width of the reg:1,4, the value should be 4 on
-  rk3288 platform
+- clocks: See dw_hdmi.txt.
+- clock-names: Shall contain "iahb" and "isfr" as defined in dw_hdmi.txt.
+- ports: See dw_hdmi.txt. The DWC HDMI shall have a single port numbered 0
+  corresponding to the video input of the controller. The port shall have two
+  endpoints, numbered 0 and 1, connected respectively to the vopb and vopl.
+- rockchip,grf: Shall reference the GRF to mux vopl/vopb.
 
 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"
+
+- ddc-i2c-bus: The HDMI DDC bus can be connected to either a system I2C master
+  or the functionally-reduced I2C master contained in the DWC HDMI. When
+  connected to a system I2C master this property contains a phandle to that
+  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.
+
 
 Example:
+
 hdmi: hdmi@ff980000 {
 	compatible = "rockchip,rk3288-dw-hdmi";
 	reg = <0xff980000 0x20000>;
-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 00/20] dw-hdmi: Cleanups, fixes and updates for v4.11

[Archit Taneja]

On 01/17/2017 01:58 PM, Laurent Pinchart wrote:
> Hello,
>
> This patch series contains the part of my pending dw-hdmi patches that have
> been successfully tested on all three supported platforms (i.MX6, RK3288 and
> R-Car H3) and have been reviewed without any problem being reported.
>
> All patches here have been previously posted as part of the "[PATCH v2 00/29]
> R-Car Gen3 HDMI output support" and "[PATCH 0/3] dw-hdmi: miscellaneous
> cleanups and fix" series. This series is a repost with all review tags
> collected.

queued to drm-misc-next

Archit

>
> Kieran Bingham (2):
>   drm: bridge: dw-hdmi: Remove unused function parameter
>   drm: bridge: dw-hdmi: Remove PHY configuration resolution parameter
>
> Laurent Pinchart (18):
>   drm: bridge: dw-hdmi: Merge __hdmi_phy_i2c_write and
>     hdmi_phy_i2c_write
>   drm: bridge: dw-hdmi: Remove unneeded arguments to bind/unbind
>     functions
>   drm: bridge: dw-hdmi: Embed drm_bridge in struct dw_hdmi
>   drm: bridge: dw-hdmi: Remove encoder field from struct dw_hdmi
>   drm: bridge: dw-hdmi: Don't forward HPD events to DRM core before
>     attach
>   drm: bridge: dw-hdmi: Move IRQ and IO resource allocation to common
>     code
>   drm: bridge: dw-hdmi: Reorder functions to prepare for next commit
>   drm: bridge: dw-hdmi: Create connector in the bridge attach operation
>   drm: bridge: dw-hdmi: Implement DRM bridge registration
>   drm: bridge: dw-hdmi: Rename CONF0 SPARECTRL bit to SVSRET
>   drm: bridge: dw-hdmi: Reject invalid product IDs
>   drm: bridge: dw-hdmi: Detect AHB audio DMA using correct register
>   drm: bridge: dw-hdmi: Handle overflow workaround based on device
>     version
>   drm: bridge: dw-hdmi: Detect PHY type at runtime
>   drm: bridge: dw-hdmi: Define and use macros for PHY register addresses
>   drm: bridge: dw-hdmi: Fix the name of the PHY reset macros
>   drm: bridge: dw-hdmi: Assert SVSRET before resetting the PHY
>   dt-bindings: display: dw-hdmi: Clean up DT bindings documentation
>
>  .../devicetree/bindings/display/bridge/dw_hdmi.txt |  85 ++--
>  .../devicetree/bindings/display/imx/hdmi.txt       |  51 +--
>  .../bindings/display/rockchip/dw_hdmi-rockchip.txt |  43 +-
>  drivers/gpu/drm/bridge/dw-hdmi.c                   | 438 +++++++++++++--------
>  drivers/gpu/drm/bridge/dw-hdmi.h                   |  85 +++-
>  drivers/gpu/drm/imx/dw_hdmi-imx.c                  |  14 +-
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        |  14 +-
>  include/drm/bridge/dw_hdmi.h                       |  19 +-
>  8 files changed, 472 insertions(+), 277 deletions(-)
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 00/20] dw-hdmi: Cleanups, fixes and updates for v4.11

[Archit Taneja]

On 01/17/2017 01:58 PM, Laurent Pinchart wrote:
> Hello,
>
> This patch series contains the part of my pending dw-hdmi patches that have
> been successfully tested on all three supported platforms (i.MX6, RK3288 and
> R-Car H3) and have been reviewed without any problem being reported.
>
> All patches here have been previously posted as part of the "[PATCH v2 00/29]
> R-Car Gen3 HDMI output support" and "[PATCH 0/3] dw-hdmi: miscellaneous
> cleanups and fix" series. This series is a repost with all review tags
> collected.

queued to drm-misc-next

Archit

>
> Kieran Bingham (2):
>   drm: bridge: dw-hdmi: Remove unused function parameter
>   drm: bridge: dw-hdmi: Remove PHY configuration resolution parameter
>
> Laurent Pinchart (18):
>   drm: bridge: dw-hdmi: Merge __hdmi_phy_i2c_write and
>     hdmi_phy_i2c_write
>   drm: bridge: dw-hdmi: Remove unneeded arguments to bind/unbind
>     functions
>   drm: bridge: dw-hdmi: Embed drm_bridge in struct dw_hdmi
>   drm: bridge: dw-hdmi: Remove encoder field from struct dw_hdmi
>   drm: bridge: dw-hdmi: Don't forward HPD events to DRM core before
>     attach
>   drm: bridge: dw-hdmi: Move IRQ and IO resource allocation to common
>     code
>   drm: bridge: dw-hdmi: Reorder functions to prepare for next commit
>   drm: bridge: dw-hdmi: Create connector in the bridge attach operation
>   drm: bridge: dw-hdmi: Implement DRM bridge registration
>   drm: bridge: dw-hdmi: Rename CONF0 SPARECTRL bit to SVSRET
>   drm: bridge: dw-hdmi: Reject invalid product IDs
>   drm: bridge: dw-hdmi: Detect AHB audio DMA using correct register
>   drm: bridge: dw-hdmi: Handle overflow workaround based on device
>     version
>   drm: bridge: dw-hdmi: Detect PHY type at runtime
>   drm: bridge: dw-hdmi: Define and use macros for PHY register addresses
>   drm: bridge: dw-hdmi: Fix the name of the PHY reset macros
>   drm: bridge: dw-hdmi: Assert SVSRET before resetting the PHY
>   dt-bindings: display: dw-hdmi: Clean up DT bindings documentation
>
>  .../devicetree/bindings/display/bridge/dw_hdmi.txt |  85 ++--
>  .../devicetree/bindings/display/imx/hdmi.txt       |  51 +--
>  .../bindings/display/rockchip/dw_hdmi-rockchip.txt |  43 +-
>  drivers/gpu/drm/bridge/dw-hdmi.c                   | 438 +++++++++++++--------
>  drivers/gpu/drm/bridge/dw-hdmi.h                   |  85 +++-
>  drivers/gpu/drm/imx/dw_hdmi-imx.c                  |  14 +-
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        |  14 +-
>  include/drm/bridge/dw_hdmi.h                       |  19 +-
>  8 files changed, 472 insertions(+), 277 deletions(-)
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v3,15/20] drm: bridge: dw-hdmi: Handle overflow workaround based on device version

[Eugeniu Rosca]

Hello Laurent, Jose,

As you might know, Renesas uses the same Synopsis Designware HDMI
transmitter IP in the Rcar Gen3 SoCs. There are v4.9+ patches present
in [1] to enable support for this.

Why I reply in this specific thread is because I am having a trial
attempt to rebase the RCAR-specific v4.9+ dw-hdmi patches on top of
a more recent kernel, which already contains mainline commit [2],
being proposed as part of this patch series.

Based on [3], it looks like that the `dw_hdmi_clear_overflow()`
workaround, formerly implemented for IMX6, is still used on Rcar3.

Current patch (integrated in upstream as commit [2]) assumes that
the workaround is only needed for the older v1.30a and v1.31a IP
implementations. Given much newer HDMI controller contained in RCAR3
(kernel reports v2.01a) and given it's you who witnessed the
original issue, I wonder what is the easiest way to check if the
problem fixed by the workaround still occurs on the Renesas SoC?
I have plenty of Salvator-X and ULCB boards, so HW is not a problem.

Your feedback is much appreciated.

Best regards,
Eugeniu.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/
[2] mainline commit be41fc55f1aa ("drm: bridge: dw-hdmi: Handle overflow workaround based on device version")
[3] https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?h=8fdb115e6e1f