[PATCH v3 0/4] Converter R-Car DU to the DRM bridge connector helper

[PATCH v3 0/4] Converter R-Car DU to the DRM bridge connector helper

[Laurent Pinchart]

Hello,

This patch series converts the R-Car DU driver to use the DRM bridge
connector helper drm_bridge_connector_init().

The bulk of the v1 series was converting the adv7511, simple-bridge and
dw-hdmi drivers to make connector creation optional (through the
DRM_BRIDGE_ATTACH_NO_CONNECTOR flag), and have already been merged. v2
included the remaining patches and has bitrotten. v3 rebased the code
and should be ready for merge.

Patch 1/4 adds support to the dw-hdmi driver to attach to a downstream
bridge if one is specified in DT. As the DT port number corresponding to
the video output differs between platforms that integrate the dw-hdmi
(some of them even don't have a video output port, which should probably
be fixed, but that's out of scope for this series), the port number has
to be specified by the platform glue layer.

Patch 2/4 then addresses the rcar-lvds driver. Instead of implementing
direct support for DRM_BRIDGE_ATTACH_NO_CONNECTOR, it simply removes
code that shouldn't have been in the driver in the first place by
switching to the panel bridge helper.

Patch 3/4 specifies the port number in the R-Car dw-hdmi glue layer, as
required by 1/4.

Patch 4/4 finally makes use of the drm_bridge_connector_init() helper.

The series has been tested on the Renesas R-Car Salvator-XS and Draak
boards with the VGA, HDMI and LVDS outputs.

Laurent Pinchart (4):
  drm: bridge: dw-hdmi: Attach to next bridge if available
  drm: rcar-du: lvds: Convert to DRM panel bridge helper
  drm: rcar-du: dw-hdmi: Set output port number
  drm: rcar-du: Use drm_bridge_connector_init() helper

 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  54 +++++++++-
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c |  26 ++++-
 drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c    |   1 +
 drivers/gpu/drm/rcar-du/rcar_lvds.c       | 120 +++-------------------
 include/drm/bridge/dw_hdmi.h              |   2 +
 5 files changed, 89 insertions(+), 114 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH v3 1/4] drm: bridge: dw-hdmi: Attach to next bridge if available

[Laurent Pinchart]

On all platforms except i.MX and Rockchip, the dw-hdmi DT bindings
require a video output port connected to an HDMI sink (most likely an
HDMI connector, in rare cases another bridges converting HDMI to another
protocol). For those platforms, retrieve the next bridge and attach it
from the dw-hdmi bridge attach handler.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
---
Changes since v1:

- Make missing endpoint a fatal error
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 54 ++++++++++++++++++++++-
 include/drm/bridge/dw_hdmi.h              |  2 +
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index e7c7c9b9c646..f9065ca4cd88 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -143,6 +143,7 @@ struct dw_hdmi_phy_data {
 struct dw_hdmi {
 	struct drm_connector connector;
 	struct drm_bridge bridge;
+	struct drm_bridge *next_bridge;
 
 	unsigned int version;
 
@@ -2775,7 +2776,8 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge,
 	struct dw_hdmi *hdmi = bridge->driver_private;
 
 	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
-		return 0;
+		return drm_bridge_attach(bridge->encoder, hdmi->next_bridge,
+					 bridge, flags);
 
 	return dw_hdmi_connector_create(hdmi);
 }
@@ -3160,6 +3162,52 @@ static void dw_hdmi_init_hw(struct dw_hdmi *hdmi)
 /* -----------------------------------------------------------------------------
  * Probe/remove API, used from platforms based on the DRM bridge API.
  */
+
+static int dw_hdmi_parse_dt(struct dw_hdmi *hdmi)
+{
+	struct device_node *endpoint;
+	struct device_node *remote;
+
+	if (!hdmi->plat_data->output_port)
+		return 0;
+
+	endpoint = of_graph_get_endpoint_by_regs(hdmi->dev->of_node,
+						 hdmi->plat_data->output_port,
+						 -1);
+	if (!endpoint) {
+		/*
+		 * On platforms whose bindings don't make the output port
+		 * mandatory (such as Rockchip) the plat_data->output_port
+		 * field isn't set, so it's safe to make this a fatal error.
+		 */
+		dev_err(hdmi->dev, "Missing endpoint in port@%u\n",
+			hdmi->plat_data->output_port);
+		return -ENODEV;
+	}
+
+	remote = of_graph_get_remote_port_parent(endpoint);
+	of_node_put(endpoint);
+	if (!remote) {
+		dev_err(hdmi->dev, "Endpoint in port@%u unconnected\n",
+			hdmi->plat_data->output_port);
+		return -ENODEV;
+	}
+
+	if (!of_device_is_available(remote)) {
+		dev_err(hdmi->dev, "port@%u remote device is disabled\n",
+			hdmi->plat_data->output_port);
+		of_node_put(remote);
+		return -ENODEV;
+	}
+
+	hdmi->next_bridge = of_drm_find_bridge(remote);
+	of_node_put(remote);
+	if (!hdmi->next_bridge)
+		return -EPROBE_DEFER;
+
+	return 0;
+}
+
 struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
 			      const struct dw_hdmi_plat_data *plat_data)
 {
@@ -3196,6 +3244,10 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
 	mutex_init(&hdmi->cec_notifier_mutex);
 	spin_lock_init(&hdmi->audio_lock);
 
+	ret = dw_hdmi_parse_dt(hdmi);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
 	ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
 	if (ddc_node) {
 		hdmi->ddc = of_get_i2c_adapter_by_node(ddc_node);
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index 6a5716655619..2a1f85f9a8a3 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -126,6 +126,8 @@ struct dw_hdmi_phy_ops {
 struct dw_hdmi_plat_data {
 	struct regmap *regm;
 
+	unsigned int output_port;
+
 	unsigned long input_bus_encoding;
 	bool use_drm_infoframe;
 	bool ycbcr_420_allowed;
-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 1/4] drm: bridge: dw-hdmi: Attach to next bridge if available

[Kieran Bingham]

Hi Laurent,

On 20/05/2021 07:50, Laurent Pinchart wrote:
> On all platforms except i.MX and Rockchip, the dw-hdmi DT bindings
> require a video output port connected to an HDMI sink (most likely an
> HDMI connector, in rare cases another bridges converting HDMI to another
> protocol). For those platforms, retrieve the next bridge and attach it
> from the dw-hdmi bridge attach handler.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

Nothing jumping out at me here.

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> ---
> Changes since v1:
> 
> - Make missing endpoint a fatal error
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 54 ++++++++++++++++++++++-
>  include/drm/bridge/dw_hdmi.h              |  2 +
>  2 files changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index e7c7c9b9c646..f9065ca4cd88 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -143,6 +143,7 @@ struct dw_hdmi_phy_data {
>  struct dw_hdmi {
>  	struct drm_connector connector;
>  	struct drm_bridge bridge;
> +	struct drm_bridge *next_bridge;
>  
>  	unsigned int version;
>  
> @@ -2775,7 +2776,8 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge,
>  	struct dw_hdmi *hdmi = bridge->driver_private;
>  
>  	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
> -		return 0;
> +		return drm_bridge_attach(bridge->encoder, hdmi->next_bridge,
> +					 bridge, flags);
>  
>  	return dw_hdmi_connector_create(hdmi);
>  }
> @@ -3160,6 +3162,52 @@ static void dw_hdmi_init_hw(struct dw_hdmi *hdmi)
>  /* -----------------------------------------------------------------------------
>   * Probe/remove API, used from platforms based on the DRM bridge API.
>   */
> +
> +static int dw_hdmi_parse_dt(struct dw_hdmi *hdmi)
> +{
> +	struct device_node *endpoint;
> +	struct device_node *remote;
> +
> +	if (!hdmi->plat_data->output_port)
> +		return 0;
> +
> +	endpoint = of_graph_get_endpoint_by_regs(hdmi->dev->of_node,
> +						 hdmi->plat_data->output_port,
> +						 -1);
> +	if (!endpoint) {
> +		/*
> +		 * On platforms whose bindings don't make the output port
> +		 * mandatory (such as Rockchip) the plat_data->output_port
> +		 * field isn't set, so it's safe to make this a fatal error.
> +		 */
> +		dev_err(hdmi->dev, "Missing endpoint in port@%u\n",
> +			hdmi->plat_data->output_port);
> +		return -ENODEV;
> +	}
> +
> +	remote = of_graph_get_remote_port_parent(endpoint);
> +	of_node_put(endpoint);
> +	if (!remote) {
> +		dev_err(hdmi->dev, "Endpoint in port@%u unconnected\n",
> +			hdmi->plat_data->output_port);
> +		return -ENODEV;
> +	}
> +
> +	if (!of_device_is_available(remote)) {
> +		dev_err(hdmi->dev, "port@%u remote device is disabled\n",
> +			hdmi->plat_data->output_port);
> +		of_node_put(remote);
> +		return -ENODEV;
> +	}
> +
> +	hdmi->next_bridge = of_drm_find_bridge(remote);
> +	of_node_put(remote);
> +	if (!hdmi->next_bridge)
> +		return -EPROBE_DEFER;
> +
> +	return 0;
> +}
> +
>  struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
>  			      const struct dw_hdmi_plat_data *plat_data)
>  {
> @@ -3196,6 +3244,10 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
>  	mutex_init(&hdmi->cec_notifier_mutex);
>  	spin_lock_init(&hdmi->audio_lock);
>  
> +	ret = dw_hdmi_parse_dt(hdmi);
> +	if (ret < 0)
> +		return ERR_PTR(ret);
> +
>  	ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
>  	if (ddc_node) {
>  		hdmi->ddc = of_get_i2c_adapter_by_node(ddc_node);
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index 6a5716655619..2a1f85f9a8a3 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -126,6 +126,8 @@ struct dw_hdmi_phy_ops {
>  struct dw_hdmi_plat_data {
>  	struct regmap *regm;
>  
> +	unsigned int output_port;
> +
>  	unsigned long input_bus_encoding;
>  	bool use_drm_infoframe;
>  	bool ycbcr_420_allowed;
> 

[PATCH v3 2/4] drm: rcar-du: lvds: Convert to DRM panel bridge helper

[Laurent Pinchart]

Replace the manual panel handling with usage of the DRM panel bridge
helper. This simplifies the driver, and brings support for
DRM_BRIDGE_ATTACH_NO_CONNECTOR as an added bonus.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 120 +++-------------------------
 1 file changed, 12 insertions(+), 108 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 70dbbe44bb23..1b360e06658c 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -63,7 +63,6 @@ struct rcar_lvds {
 	struct drm_bridge bridge;
 
 	struct drm_bridge *next_bridge;
-	struct drm_connector connector;
 	struct drm_panel *panel;
 
 	void __iomem *mmio;
@@ -80,73 +79,11 @@ struct rcar_lvds {
 #define bridge_to_rcar_lvds(b) \
 	container_of(b, struct rcar_lvds, bridge)
 
-#define connector_to_rcar_lvds(c) \
-	container_of(c, struct rcar_lvds, connector)
-
 static void rcar_lvds_write(struct rcar_lvds *lvds, u32 reg, u32 data)
 {
 	iowrite32(data, lvds->mmio + reg);
 }
 
-/* -----------------------------------------------------------------------------
- * Connector & Panel
- */
-
-static int rcar_lvds_connector_get_modes(struct drm_connector *connector)
-{
-	struct rcar_lvds *lvds = connector_to_rcar_lvds(connector);
-
-	return drm_panel_get_modes(lvds->panel, connector);
-}
-
-static int rcar_lvds_connector_atomic_check(struct drm_connector *connector,
-					    struct drm_atomic_state *state)
-{
-	struct rcar_lvds *lvds = connector_to_rcar_lvds(connector);
-	const struct drm_display_mode *panel_mode;
-	struct drm_connector_state *conn_state;
-	struct drm_crtc_state *crtc_state;
-
-	conn_state = drm_atomic_get_new_connector_state(state, connector);
-	if (!conn_state->crtc)
-		return 0;
-
-	if (list_empty(&connector->modes)) {
-		dev_dbg(lvds->dev, "connector: empty modes list\n");
-		return -EINVAL;
-	}
-
-	panel_mode = list_first_entry(&connector->modes,
-				      struct drm_display_mode, head);
-
-	/* We're not allowed to modify the resolution. */
-	crtc_state = drm_atomic_get_crtc_state(state, conn_state->crtc);
-	if (IS_ERR(crtc_state))
-		return PTR_ERR(crtc_state);
-
-	if (crtc_state->mode.hdisplay != panel_mode->hdisplay ||
-	    crtc_state->mode.vdisplay != panel_mode->vdisplay)
-		return -EINVAL;
-
-	/* The flat panel mode is fixed, just copy it to the adjusted mode. */
-	drm_mode_copy(&crtc_state->adjusted_mode, panel_mode);
-
-	return 0;
-}
-
-static const struct drm_connector_helper_funcs rcar_lvds_conn_helper_funcs = {
-	.get_modes = rcar_lvds_connector_get_modes,
-	.atomic_check = rcar_lvds_connector_atomic_check,
-};
-
-static const struct drm_connector_funcs rcar_lvds_conn_funcs = {
-	.reset = drm_atomic_helper_connector_reset,
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = drm_connector_cleanup,
-	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
-	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-
 /* -----------------------------------------------------------------------------
  * PLL Setup
  */
@@ -583,11 +520,6 @@ static void __rcar_lvds_atomic_enable(struct drm_bridge *bridge,
 	/* Turn the output on. */
 	lvdcr0 |= LVDCR0_LVRES;
 	rcar_lvds_write(lvds, LVDCR0, lvdcr0);
-
-	if (lvds->panel) {
-		drm_panel_prepare(lvds->panel);
-		drm_panel_enable(lvds->panel);
-	}
 }
 
 static void rcar_lvds_atomic_enable(struct drm_bridge *bridge,
@@ -609,11 +541,6 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
 {
 	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
 
-	if (lvds->panel) {
-		drm_panel_disable(lvds->panel);
-		drm_panel_unprepare(lvds->panel);
-	}
-
 	rcar_lvds_write(lvds, LVDCR0, 0);
 	rcar_lvds_write(lvds, LVDCR1, 0);
 	rcar_lvds_write(lvds, LVDPLLCR, 0);
@@ -648,45 +575,13 @@ static int rcar_lvds_attach(struct drm_bridge *bridge,
 			    enum drm_bridge_attach_flags flags)
 {
 	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
-	struct drm_connector *connector = &lvds->connector;
-	struct drm_encoder *encoder = bridge->encoder;
-	int ret;
 
-	/* If we have a next bridge just attach it. */
-	if (lvds->next_bridge)
-		return drm_bridge_attach(bridge->encoder, lvds->next_bridge,
-					 bridge, flags);
-
-	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
-		DRM_ERROR("Fix bridge driver to make connector optional!");
-		return -EINVAL;
-	}
-
-	/* Otherwise if we have a panel, create a connector. */
-	if (!lvds->panel)
-		return 0;
-
-	ret = drm_connector_init(bridge->dev, connector, &rcar_lvds_conn_funcs,
-				 DRM_MODE_CONNECTOR_LVDS);
-	if (ret < 0)
-		return ret;
-
-	drm_connector_helper_add(connector, &rcar_lvds_conn_helper_funcs);
-
-	ret = drm_connector_attach_encoder(connector, encoder);
-	if (ret < 0)
-		return ret;
-
-	return 0;
-}
-
-static void rcar_lvds_detach(struct drm_bridge *bridge)
-{
+	return drm_bridge_attach(bridge->encoder, lvds->next_bridge, bridge,
+				 flags);
 }
 
 static const struct drm_bridge_funcs rcar_lvds_bridge_ops = {
 	.attach = rcar_lvds_attach,
-	.detach = rcar_lvds_detach,
 	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
 	.atomic_reset = drm_atomic_helper_bridge_reset,
@@ -759,7 +654,7 @@ static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds)
 		 * that we are expected to generate even pixels from the primary
 		 * encoder, and odd pixels from the companion encoder.
 		 */
-		if (lvds->next_bridge && lvds->next_bridge->timings &&
+		if (lvds->next_bridge->timings &&
 		    lvds->next_bridge->timings->dual_link)
 			lvds->link_type = RCAR_LVDS_DUAL_LINK_EVEN_ODD_PIXELS;
 		else
@@ -811,6 +706,15 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
 	if (ret)
 		goto done;
 
+	if (lvds->panel) {
+		lvds->next_bridge = devm_drm_panel_bridge_add(lvds->dev,
+							      lvds->panel);
+		if (IS_ERR_OR_NULL(lvds->next_bridge)) {
+			ret = -EINVAL;
+			goto done;
+		}
+	}
+
 	if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
 		ret = rcar_lvds_parse_dt_companion(lvds);
 
-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 2/4] drm: rcar-du: lvds: Convert to DRM panel bridge helper

[Kieran Bingham]

Hi Laurent,

On 20/05/2021 07:50, Laurent Pinchart wrote:
> Replace the manual panel handling with usage of the DRM panel bridge
> helper. This simplifies the driver, and brings support for
> DRM_BRIDGE_ATTACH_NO_CONNECTOR as an added bonus.
> 

That's a lot of code removal. Excellent.

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 120 +++-------------------------
>  1 file changed, 12 insertions(+), 108 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 70dbbe44bb23..1b360e06658c 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -63,7 +63,6 @@ struct rcar_lvds {
>  	struct drm_bridge bridge;
>  
>  	struct drm_bridge *next_bridge;
> -	struct drm_connector connector;
>  	struct drm_panel *panel;
>  
>  	void __iomem *mmio;
> @@ -80,73 +79,11 @@ struct rcar_lvds {
>  #define bridge_to_rcar_lvds(b) \
>  	container_of(b, struct rcar_lvds, bridge)
>  
> -#define connector_to_rcar_lvds(c) \
> -	container_of(c, struct rcar_lvds, connector)
> -
>  static void rcar_lvds_write(struct rcar_lvds *lvds, u32 reg, u32 data)
>  {
>  	iowrite32(data, lvds->mmio + reg);
>  }
>  
> -/* -----------------------------------------------------------------------------
> - * Connector & Panel
> - */
> -
> -static int rcar_lvds_connector_get_modes(struct drm_connector *connector)
> -{
> -	struct rcar_lvds *lvds = connector_to_rcar_lvds(connector);
> -
> -	return drm_panel_get_modes(lvds->panel, connector);
> -}
> -
> -static int rcar_lvds_connector_atomic_check(struct drm_connector *connector,
> -					    struct drm_atomic_state *state)
> -{
> -	struct rcar_lvds *lvds = connector_to_rcar_lvds(connector);
> -	const struct drm_display_mode *panel_mode;
> -	struct drm_connector_state *conn_state;
> -	struct drm_crtc_state *crtc_state;
> -
> -	conn_state = drm_atomic_get_new_connector_state(state, connector);
> -	if (!conn_state->crtc)
> -		return 0;
> -
> -	if (list_empty(&connector->modes)) {
> -		dev_dbg(lvds->dev, "connector: empty modes list\n");
> -		return -EINVAL;
> -	}
> -
> -	panel_mode = list_first_entry(&connector->modes,
> -				      struct drm_display_mode, head);
> -
> -	/* We're not allowed to modify the resolution. */
> -	crtc_state = drm_atomic_get_crtc_state(state, conn_state->crtc);
> -	if (IS_ERR(crtc_state))
> -		return PTR_ERR(crtc_state);
> -
> -	if (crtc_state->mode.hdisplay != panel_mode->hdisplay ||
> -	    crtc_state->mode.vdisplay != panel_mode->vdisplay)
> -		return -EINVAL;
> -
> -	/* The flat panel mode is fixed, just copy it to the adjusted mode. */
> -	drm_mode_copy(&crtc_state->adjusted_mode, panel_mode);
> -
> -	return 0;
> -}
> -
> -static const struct drm_connector_helper_funcs rcar_lvds_conn_helper_funcs = {
> -	.get_modes = rcar_lvds_connector_get_modes,
> -	.atomic_check = rcar_lvds_connector_atomic_check,
> -};
> -
> -static const struct drm_connector_funcs rcar_lvds_conn_funcs = {
> -	.reset = drm_atomic_helper_connector_reset,
> -	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.destroy = drm_connector_cleanup,
> -	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> -	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> -};
> -
>  /* -----------------------------------------------------------------------------
>   * PLL Setup
>   */
> @@ -583,11 +520,6 @@ static void __rcar_lvds_atomic_enable(struct drm_bridge *bridge,
>  	/* Turn the output on. */
>  	lvdcr0 |= LVDCR0_LVRES;
>  	rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> -
> -	if (lvds->panel) {
> -		drm_panel_prepare(lvds->panel);
> -		drm_panel_enable(lvds->panel);
> -	}
>  }
>  
>  static void rcar_lvds_atomic_enable(struct drm_bridge *bridge,
> @@ -609,11 +541,6 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
>  {
>  	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
>  
> -	if (lvds->panel) {
> -		drm_panel_disable(lvds->panel);
> -		drm_panel_unprepare(lvds->panel);
> -	}
> -
>  	rcar_lvds_write(lvds, LVDCR0, 0);
>  	rcar_lvds_write(lvds, LVDCR1, 0);
>  	rcar_lvds_write(lvds, LVDPLLCR, 0);
> @@ -648,45 +575,13 @@ static int rcar_lvds_attach(struct drm_bridge *bridge,
>  			    enum drm_bridge_attach_flags flags)
>  {
>  	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> -	struct drm_connector *connector = &lvds->connector;
> -	struct drm_encoder *encoder = bridge->encoder;
> -	int ret;
>  
> -	/* If we have a next bridge just attach it. */
> -	if (lvds->next_bridge)
> -		return drm_bridge_attach(bridge->encoder, lvds->next_bridge,
> -					 bridge, flags);
> -
> -	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
> -		DRM_ERROR("Fix bridge driver to make connector optional!");
> -		return -EINVAL;
> -	}
> -
> -	/* Otherwise if we have a panel, create a connector. */
> -	if (!lvds->panel)
> -		return 0;
> -
> -	ret = drm_connector_init(bridge->dev, connector, &rcar_lvds_conn_funcs,
> -				 DRM_MODE_CONNECTOR_LVDS);
> -	if (ret < 0)
> -		return ret;
> -
> -	drm_connector_helper_add(connector, &rcar_lvds_conn_helper_funcs);
> -
> -	ret = drm_connector_attach_encoder(connector, encoder);
> -	if (ret < 0)
> -		return ret;
> -
> -	return 0;
> -}
> -
> -static void rcar_lvds_detach(struct drm_bridge *bridge)
> -{
> +	return drm_bridge_attach(bridge->encoder, lvds->next_bridge, bridge,
> +				 flags);
>  }
>  
>  static const struct drm_bridge_funcs rcar_lvds_bridge_ops = {
>  	.attach = rcar_lvds_attach,
> -	.detach = rcar_lvds_detach,
>  	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
>  	.atomic_reset = drm_atomic_helper_bridge_reset,
> @@ -759,7 +654,7 @@ static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds)
>  		 * that we are expected to generate even pixels from the primary
>  		 * encoder, and odd pixels from the companion encoder.
>  		 */
> -		if (lvds->next_bridge && lvds->next_bridge->timings &&
> +		if (lvds->next_bridge->timings &&
>  		    lvds->next_bridge->timings->dual_link)
>  			lvds->link_type = RCAR_LVDS_DUAL_LINK_EVEN_ODD_PIXELS;
>  		else
> @@ -811,6 +706,15 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
>  	if (ret)
>  		goto done;
>  
> +	if (lvds->panel) {
> +		lvds->next_bridge = devm_drm_panel_bridge_add(lvds->dev,
> +							      lvds->panel);
> +		if (IS_ERR_OR_NULL(lvds->next_bridge)) {
> +			ret = -EINVAL;
> +			goto done;
> +		}
> +	}
> +
>  	if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
>  		ret = rcar_lvds_parse_dt_companion(lvds);
>  
> 

Re: [PATCH v3 2/4] drm: rcar-du: lvds: Convert to DRM panel bridge helper

[Geert Uytterhoeven]

Hi Laurent,

On Thu, May 20, 2021 at 8:51 AM Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> Replace the manual panel handling with usage of the DRM panel bridge
> helper. This simplifies the driver, and brings support for
> DRM_BRIDGE_ATTACH_NO_CONNECTOR as an added bonus.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Thanks for your patch, which is now commit e9e056949c927e5b ("drm:
rcar-du: lvds: Convert to DRM panel bridge helper") in drm-next.

This causes the following scary messages in the kernel log on Ebisu-4D:

[drm:drm_bridge_attach] *ERROR* failed to attach bridge
/soc/lvds-encoder@feb90100 to encoder None-66: -22
renesas_sdhi_internal_dmac ee100000.mmc: mmc1 base at
0x00000000ee100000, max clock rate 200 MHz
rcar-du feb00000.display: failed to initialize encoder
/soc/lvds-encoder@feb90100 on output 3 (-22), skipping
renesas_sdhi_internal_dmac ee120000.mmc: mmc2 base at
0x00000000ee120000, max clock rate 200 MHz
------------[ cut here ]------------
renesas_sdhi_internal_dmac ee160000.mmc: mmc0 base at
0x00000000ee160000, max clock rate 200 MHz
possible_clones mismatch: [ENCODER:62:None-62] mask=0x1
possible_clones=0x3 vs. [ENCODER:66:None-66] mask=0x4
possible_clones=0x3
WARNING: CPU: 1 PID: 68 at drivers/gpu/drm/drm_mode_config.c:583
drm_mode_config_validate+0x3cc/0x4c8
CPU: 1 PID: 68 Comm: kworker/u4:2 Not tainted
5.14.0-rc3-arm64-renesas-00324-ge9e056949c92 #1277
Hardware name: Renesas Ebisu-4D board based on r8a77990 (DT)
Workqueue: events_unbound deferred_probe_work_func
pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
pc : drm_mode_config_validate+0x3cc/0x4c8
lr : drm_mode_config_validate+0x3cc/0x4c8
sp : ffff80001239ba80
x29: ffff80001239ba90 x28: ffff00000801c005 x27: 0000000000000001
x26: ffff000009de8868 x25: 000000000000001f x24: ffff00000a490f80
mmc0: new HS400 MMC card at address 0001
x23: ffff000009de8868 x22: ffff800011029ea0 x21: ffff800011029e20
x20: ffff000009de8018 x19: ffff00000a490b80 x18: 0000000000000020
x17: 7364766c2f636f73 x16: 0000000000004a12 x15: ffff00000892df38
x14: 0000000000000000 x13: 0000000000000003 x12: ffff8000113e3a80
x11: 0000000000000001 x10: 0000000000000078
mmcblk0: mmc0:0001 BGSD3R 29.1 GiB
 x9 : ffff800011172994
x8 : 0000000000000000 x7 : 0000000000000001 x6 : 0000000000000001
x5 : 0000000000000000 x4 : 0000000000000003 x3 : 0000000000000000
x2 : ffff8000113e38d8 x1 : 0000000000000000 x0 : 0000000000000000
Call trace:
 drm_mode_config_validate+0x3cc/0x4c8
 drm_dev_register+0x174/0x208
 rcar_du_probe+0xc4/0x110
 platform_probe+0x64/0xd0
 really_probe+0x134/0x2e8
 __driver_probe_device+0x74/0xd8
 driver_probe_device+0x3c/0xe0
 __device_attach_driver+0xa8/0xf0
 bus_for_each_drv+0x74/0xc8
 __device_attach+0xec/0x148
 device_initial_probe+0x10/0x18
 bus_probe_device+0x98/0xa0
mmcblk0boot0: mmc0:0001 BGSD3R 16.0 MiB
 deferred_probe_work_func+0x88/0xc0
 process_one_work+0x284/0x6d0
 worker_thread+0x48/0x418
 kthread+0x14c/0x158
 ret_from_fork+0x10/0x18
irq event stamp: 27688
hardirqs last  enabled at (27687): [<ffff800010104424>] vprintk_emit+0x2f4/0x2f8
hardirqs last disabled at (27688): [<ffff800010ca4eb4>] el1_dbg+0x24/0x88
mmcblk0boot1: mmc0:0001 BGSD3R 16.0 MiB
softirqs last  enabled at (27616): [<ffff8000100104ac>] _stext+0x4ac/0x620
softirqs last disabled at (27607): [<ffff80001008ffdc>] irq_exit+0x1b4/0x1c0
---[ end trace 6e42cb0428a6481b ]---
------------[ cut here ]------------
mmcblk0rpmb: mmc0:0001 BGSD3R 4.00 MiB, chardev (243:0)
possible_clones mismatch: [ENCODER:64:None-64] mask=0x2
possible_clones=0x3 vs. [ENCODER:66:None-66] mask=0x4
possible_clones=0x3
WARNING: CPU: 1 PID: 68 at drivers/gpu/drm/drm_mode_config.c:583
drm_mode_config_validate+0x3cc/0x4c8
CPU: 1 PID: 68 Comm: kworker/u4:2 Tainted: G        W
5.14.0-rc3-arm64-renesas-00324-ge9e056949c92 #1277
Hardware name: Renesas Ebisu-4D board based on r8a77990 (DT)
Workqueue: events_unbound deferred_probe_work_func
pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
pc : drm_mode_config_validate+0x3cc/0x4c8
lr : drm_mode_config_validate+0x3cc/0x4c8
sp : ffff80001239ba80
x29: ffff80001239ba90 x28: ffff00000801c005 x27: 0000000000000001
x26: ffff000009de8868 x25: 000000000000001f x24: ffff00000a490f80
x23: ffff000009de8868 x22: ffff800011029ea0 x21: ffff800011029e20
x20: ffff000009de8018 x19: ffff00000a490d80 x18: 0000000000000010
x17: 746978655f717269 x16: 205d3e6364666638 x15: 076c0763075f0765
x14: 000000000000014d x13: ffff00000892df38 x12: 00000000ffffffea
x11: ffff800011453b70 x10: ffff80001143bb30 x9 : ffff80001143bb88
x8 : 0000000000000000 x7 : 0000000000000001 x6 : 0000000000000001
x5 : 0000000000000000 x4 : 0000000000000003 x3 : 0000000000000000
x2 : ffff8000113e38d8 x1 : 0000000000000000 x0 : 0000000000000000
Call trace:
 drm_mode_config_validate+0x3cc/0x4c8
 drm_dev_register+0x174/0x208
 rcar_du_probe+0xc4/0x110
 platform_probe+0x64/0xd0
 really_probe+0x134/0x2e8
 __driver_probe_device+0x74/0xd8
 driver_probe_device+0x3c/0xe0
 __device_attach_driver+0xa8/0xf0
 bus_for_each_drv+0x74/0xc8
 __device_attach+0xec/0x148
 device_initial_probe+0x10/0x18
 bus_probe_device+0x98/0xa0
 deferred_probe_work_func+0x88/0xc0
 process_one_work+0x284/0x6d0
 worker_thread+0x48/0x418
 kthread+0x14c/0x158
 ret_from_fork+0x10/0x18
irq event stamp: 27858
hardirqs last  enabled at (27857): [<ffff800010103e84>]
console_unlock+0x4d4/0x638
hardirqs last disabled at (27858): [<ffff800010ca4eb4>] el1_dbg+0x24/0x88
softirqs last  enabled at (27822): [<ffff8000100104ac>] _stext+0x4ac/0x620
softirqs last disabled at (27817): [<ffff80001008ffdc>] irq_exit+0x1b4/0x1c0
---[ end trace 6e42cb0428a6481c ]---
------------[ cut here ]------------
possible_clones mismatch: [ENCODER:66:None-66] mask=0x4
possible_clones=0x3 vs. [ENCODER:62:None-62] mask=0x1
possible_clones=0x3
WARNING: CPU: 1 PID: 68 at drivers/gpu/drm/drm_mode_config.c:583
drm_mode_config_validate+0x3cc/0x4c8
CPU: 1 PID: 68 Comm: kworker/u4:2 Tainted: G        W
5.14.0-rc3-arm64-renesas-00324-ge9e056949c92 #1277
Hardware name: Renesas Ebisu-4D board based on r8a77990 (DT)
Workqueue: events_unbound deferred_probe_work_func
pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
pc : drm_mode_config_validate+0x3cc/0x4c8
lr : drm_mode_config_validate+0x3cc/0x4c8
sp : ffff80001239ba80
x29: ffff80001239ba90 x28: ffff00000801c005 x27: 0000000000000001
x26: ffff000009de8868 x25: 000000000000001f x24: ffff00000a490b80
x23: ffff000009de8868 x22: ffff800011029ea0 x21: ffff800011029e20
x20: ffff000009de8018 x19: ffff00000a490f80 x18: 0000000000000010
x17: 746978655f717269 x16: 205d3e6364666638 x15: 076c0763075f0765
x14: 0000000000000179 x13: ffff00000892df38 x12: 00000000ffffffea
x11: ffff800011453b70 x10: ffff80001143bb30 x9 : ffff80001143bb88
x8 : 0000000000000000 x7 : 0000000000000001 x6 : 0000000000000001
x5 : 0000000000000000 x4 : 0000000000000003 x3 : 0000000000000000
x2 : ffff8000113e38d8 x1 : 0000000000000000 x0 : 0000000000000000
Call trace:
 drm_mode_config_validate+0x3cc/0x4c8
 drm_dev_register+0x174/0x208
 rcar_du_probe+0xc4/0x110
 platform_probe+0x64/0xd0
 really_probe+0x134/0x2e8
 __driver_probe_device+0x74/0xd8
 driver_probe_device+0x3c/0xe0
 __device_attach_driver+0xa8/0xf0
 bus_for_each_drv+0x74/0xc8
 __device_attach+0xec/0x148
 device_initial_probe+0x10/0x18
 bus_probe_device+0x98/0xa0
 deferred_probe_work_func+0x88/0xc0
 process_one_work+0x284/0x6d0
 worker_thread+0x48/0x418
 kthread+0x14c/0x158
 ret_from_fork+0x10/0x18
irq event stamp: 27888
hardirqs last  enabled at (27887): [<ffff800010103e84>]
console_unlock+0x4d4/0x638
hardirqs last disabled at (27888): [<ffff800010ca4eb4>] el1_dbg+0x24/0x88
softirqs last  enabled at (27866): [<ffff8000100104ac>] _stext+0x4ac/0x620
softirqs last disabled at (27861): [<ffff80001008ffdc>] irq_exit+0x1b4/0x1c0
---[ end trace 6e42cb0428a6481d ]---
------------[ cut here ]------------
possible_clones mismatch: [ENCODER:66:None-66] mask=0x4
possible_clones=0x3 vs. [ENCODER:64:None-64] mask=0x2
possible_clones=0x3
WARNING: CPU: 1 PID: 68 at drivers/gpu/drm/drm_mode_config.c:583
drm_mode_config_validate+0x3cc/0x4c8
CPU: 1 PID: 68 Comm: kworker/u4:2 Tainted: G        W
5.14.0-rc3-arm64-renesas-00324-ge9e056949c92 #1277
Hardware name: Renesas Ebisu-4D board based on r8a77990 (DT)
Workqueue: events_unbound deferred_probe_work_func
pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
pc : drm_mode_config_validate+0x3cc/0x4c8
lr : drm_mode_config_validate+0x3cc/0x4c8
sp : ffff80001239ba80
x29: ffff80001239ba90 x28: ffff00000801c005 x27: 0000000000000001
x26: ffff000009de8868 x25: 000000000000001f x24: ffff00000a490d80
x23: ffff000009de8868 x22: ffff800011029ea0 x21: ffff800011029e20
x20: ffff000009de8018 x19: ffff00000a490f80 x18: 0000000000000010
x17: 746978655f717269 x16: 205d3e6364666638 x15: 076c0763075f0765
x14: 00000000000001a5 x13: ffff00000892df38 x12: 00000000ffffffea
x11: ffff800011453b70 x10: ffff80001143bb30 x9 : ffff80001143bb88
x8 : 0000000000000000 x7 : 0000000000000001 x6 : 0000000000000001
x5 : 0000000000000000 x4 : 0000000000000003 x3 : 0000000000000000
x2 : ffff8000113e38d8 x1 : 0000000000000000 x0 : 0000000000000000
Call trace:
 drm_mode_config_validate+0x3cc/0x4c8
 drm_dev_register+0x174/0x208
 rcar_du_probe+0xc4/0x110
 platform_probe+0x64/0xd0
 really_probe+0x134/0x2e8
 __driver_probe_device+0x74/0xd8
 driver_probe_device+0x3c/0xe0
 __device_attach_driver+0xa8/0xf0
 bus_for_each_drv+0x74/0xc8
 __device_attach+0xec/0x148
 device_initial_probe+0x10/0x18
 bus_probe_device+0x98/0xa0
 deferred_probe_work_func+0x88/0xc0
 process_one_work+0x284/0x6d0
 worker_thread+0x48/0x418
 kthread+0x14c/0x158
 ret_from_fork+0x10/0x18
irq event stamp: 27924
hardirqs last  enabled at (27923): [<ffff800010103e84>]
console_unlock+0x4d4/0x638
hardirqs last disabled at (27924): [<ffff800010ca4eb4>] el1_dbg+0x24/0x88
softirqs last  enabled at (27902): [<ffff8000100104ac>] _stext+0x4ac/0x620
softirqs last disabled at (27891): [<ffff80001008ffdc>] irq_exit+0x1b4/0x1c0
---[ end trace 6e42cb0428a6481e ]---
------------[ cut here ]------------
Bogus possible_clones: [ENCODER:66:None-66] possible_clones=0x3 (full
encoder mask=0x1f)
WARNING: CPU: 1 PID: 68 at drivers/gpu/drm/drm_mode_config.c:594
drm_mode_config_validate+0x150/0x4c8
CPU: 1 PID: 68 Comm: kworker/u4:2 Tainted: G        W
5.14.0-rc3-arm64-renesas-00324-ge9e056949c92 #1277
Hardware name: Renesas Ebisu-4D board based on r8a77990 (DT)
Workqueue: events_unbound deferred_probe_work_func
pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
pc : drm_mode_config_validate+0x150/0x4c8
lr : drm_mode_config_validate+0x150/0x4c8
sp : ffff80001239ba80
x29: ffff80001239ba90 x28: ffff00000801c005 x27: 0000000000000001
x26: ffff000009de8868 x25: 000000000000001f x24: ffff000009de8860
x23: ffff000009de8868 x22: ffff800011029ea0 x21: ffff800011029e20
x20: ffff000009de8018 x19: ffff00000a490f80 x18: 0000000000000010
x17: 203378303d73656e x16: 6f6c635f656c6269 x15: 0720072007200720
x14: 00000000000001d1 x13: ffff00000892df38 x12: 00000000ffffffea
x11: ffff800011453b70 x10: ffff80001143bb30 x9 : ffff80001143bb88
x8 : 0000000000000000 x7 : 0000000000000001 x6 : 0000000000000001
x5 : 0000000000000000 x4 : 0000000000000003 x3 : 0000000000000000
x2 : ffff8000113e38d8 x1 : 0000000000000000 x0 : 0000000000000000
Call trace:
 drm_mode_config_validate+0x150/0x4c8
 drm_dev_register+0x174/0x208
 rcar_du_probe+0xc4/0x110
 platform_probe+0x64/0xd0
 really_probe+0x134/0x2e8
 __driver_probe_device+0x74/0xd8
 driver_probe_device+0x3c/0xe0
 __device_attach_driver+0xa8/0xf0
 bus_for_each_drv+0x74/0xc8
 __device_attach+0xec/0x148
 device_initial_probe+0x10/0x18
 bus_probe_device+0x98/0xa0
 deferred_probe_work_func+0x88/0xc0
 process_one_work+0x284/0x6d0
 worker_thread+0x48/0x418
 kthread+0x14c/0x158
 ret_from_fork+0x10/0x18
irq event stamp: 27966
hardirqs last  enabled at (27965): [<ffff800010103e84>]
console_unlock+0x4d4/0x638
hardirqs last disabled at (27966): [<ffff800010ca4eb4>] el1_dbg+0x24/0x88
softirqs last  enabled at (27940): [<ffff8000100104ac>] _stext+0x4ac/0x620
softirqs last disabled at (27927): [<ffff80001008ffdc>] irq_exit+0x1b4/0x1c0
---[ end trace 6e42cb0428a6481f ]---
[drm] Initialized rcar-du 1.0.0 20130110 for feb00000.display on minor 0
drm] Device feb00000.display probed
Console: switching to colour frame buffer device 128x48
rcar-du feb00000.display: [drm] fb0: rcar-du frame buffer device

Reverting this commit fixes the issue.
Note that I have no actual display device attached, so I do not know
if display out really works.
Salvator-X(S) seems to be fine, with the same disclaimer.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 2/4] drm: rcar-du: lvds: Convert to DRM panel bridge helper

[Geert Uytterhoeven]

Hi Laurent,

On Tue, Aug 10, 2021 at 5:47 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Thu, May 20, 2021 at 8:51 AM Laurent Pinchart
> <laurent.pinchart+renesas@ideasonboard.com> wrote:
> > Replace the manual panel handling with usage of the DRM panel bridge
> > helper. This simplifies the driver, and brings support for
> > DRM_BRIDGE_ATTACH_NO_CONNECTOR as an added bonus.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>
> Thanks for your patch, which is now commit e9e056949c927e5b ("drm:
> rcar-du: lvds: Convert to DRM panel bridge helper") in drm-next.
>
> This causes the following scary messages in the kernel log on Ebisu-4D:
>
> [drm:drm_bridge_attach] *ERROR* failed to attach bridge

> Reverting this commit fixes the issue.

Oops, that's not true (should not write tentative messages while booting
a test kernel, and press send without checking first ;-)

Reverting the commit doesn't seem to make a difference.
But the parent 5bcc48395b9f35da ("drm: bridge: dw-hdmi: Attach to
next bridge if available") of the bad commit is fine.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 2/4] drm: rcar-du: lvds: Convert to DRM panel bridge helper

[Laurent Pinchart]

Hi Geert,

On Tue, Aug 10, 2021 at 05:51:57PM +0200, Geert Uytterhoeven wrote:
> On Tue, Aug 10, 2021 at 5:47 PM Geert Uytterhoeven wrote:
> > On Thu, May 20, 2021 at 8:51 AM Laurent Pinchart wrote:
> > > Replace the manual panel handling with usage of the DRM panel bridge
> > > helper. This simplifies the driver, and brings support for
> > > DRM_BRIDGE_ATTACH_NO_CONNECTOR as an added bonus.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> >
> > Thanks for your patch, which is now commit e9e056949c927e5b ("drm:
> > rcar-du: lvds: Convert to DRM panel bridge helper") in drm-next.
> >
> > This causes the following scary messages in the kernel log on Ebisu-4D:
> >
> > [drm:drm_bridge_attach] *ERROR* failed to attach bridge
> 
> > Reverting this commit fixes the issue.
> 
> Oops, that's not true (should not write tentative messages while booting
> a test kernel, and press send without checking first ;-)
> 
> Reverting the commit doesn't seem to make a difference.
> But the parent 5bcc48395b9f35da ("drm: bridge: dw-hdmi: Attach to
> next bridge if available") of the bad commit is fine.

Thanks for the report. I've reproduced this and will send a fix shortly.

-- 
Regards,

Laurent Pinchart

[PATCH v3 3/4] drm: rcar-du: dw-hdmi: Set output port number

[Laurent Pinchart]

Report the DT output port number in dw_hdmi_plat_data to connect to the
next bridge in the dw-hdmi driver.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
index 7b8ec8310699..18ed14911b98 100644
--- a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
@@ -75,6 +75,7 @@ static int rcar_hdmi_phy_configure(struct dw_hdmi *hdmi, void *data,
 }
 
 static const struct dw_hdmi_plat_data rcar_dw_hdmi_plat_data = {
+	.output_port = 1,
 	.mode_valid = rcar_hdmi_mode_valid,
 	.configure_phy	= rcar_hdmi_phy_configure,
 };
-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 3/4] drm: rcar-du: dw-hdmi: Set output port number

[Kieran Bingham]

Hi Laurent,

On 20/05/2021 07:50, Laurent Pinchart wrote:
> Report the DT output port number in dw_hdmi_plat_data to connect to the
> next bridge in the dw-hdmi driver.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
> index 7b8ec8310699..18ed14911b98 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
> @@ -75,6 +75,7 @@ static int rcar_hdmi_phy_configure(struct dw_hdmi *hdmi, void *data,
>  }
>  
>  static const struct dw_hdmi_plat_data rcar_dw_hdmi_plat_data = {
> +	.output_port = 1,

I presume this matches the bindings at "port@1:HDMI output port"

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

>  	.mode_valid = rcar_hdmi_mode_valid,
>  	.configure_phy	= rcar_hdmi_phy_configure,
>  };
> 

[PATCH v3 4/4] drm: rcar-du: Use drm_bridge_connector_init() helper

[Laurent Pinchart]

Use the drm_bridge_connector_init() helper to create a drm_connector for
each output, instead of relying on the bridge drivers doing so. Attach
the bridges with the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag to instruct
them not to create a connector.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
Changes since v2:

- Declare ret variable
---
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 26 ++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
index ca3761772211..0daa8bba50f5 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
@@ -11,6 +11,7 @@
 #include <linux/slab.h>
 
 #include <drm/drm_bridge.h>
+#include <drm/drm_bridge_connector.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_managed.h>
 #include <drm/drm_modeset_helper_vtables.h>
@@ -53,7 +54,9 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
 			 struct device_node *enc_node)
 {
 	struct rcar_du_encoder *renc;
+	struct drm_connector *connector;
 	struct drm_bridge *bridge;
+	int ret;
 
 	/*
 	 * Locate the DRM bridge from the DT node. For the DPAD outputs, if the
@@ -103,9 +106,22 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
 
 	renc->output = output;
 
-	/*
-	 * Attach the bridge to the encoder. The bridge will create the
-	 * connector.
-	 */
-	return drm_bridge_attach(&renc->base, bridge, NULL, 0);
+	/* Attach the bridge to the encoder. */
+	ret = drm_bridge_attach(&renc->base, bridge, NULL,
+				DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+	if (ret) {
+		dev_err(rcdu->dev, "failed to attach bridge for output %u\n",
+			output);
+		return ret;
+	}
+
+	/* Create the connector for the chain of bridges. */
+	connector = drm_bridge_connector_init(&rcdu->ddev, &renc->base);
+	if (IS_ERR(connector)) {
+		dev_err(rcdu->dev,
+			"failed to created connector for output %u\n", output);
+		return PTR_ERR(connector);
+	}
+
+	return drm_connector_attach_encoder(connector, &renc->base);
 }
-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 4/4] drm: rcar-du: Use drm_bridge_connector_init() helper

[Kieran Bingham]

Hi Laurent,


On 20/05/2021 07:50, Laurent Pinchart wrote:
> Use the drm_bridge_connector_init() helper to create a drm_connector for
> each output, instead of relying on the bridge drivers doing so. Attach
> the bridges with the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag to instruct
> them not to create a connector.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Aha, this is the one that fixed my issues!

Looks good, and tests well ;-)

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> ---
> Changes since v2:
> 
> - Declare ret variable
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 26 ++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> index ca3761772211..0daa8bba50f5 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> @@ -11,6 +11,7 @@
>  #include <linux/slab.h>
>  
>  #include <drm/drm_bridge.h>
> +#include <drm/drm_bridge_connector.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_managed.h>
>  #include <drm/drm_modeset_helper_vtables.h>
> @@ -53,7 +54,9 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
>  			 struct device_node *enc_node)
>  {
>  	struct rcar_du_encoder *renc;
> +	struct drm_connector *connector;
>  	struct drm_bridge *bridge;
> +	int ret;
>  
>  	/*
>  	 * Locate the DRM bridge from the DT node. For the DPAD outputs, if the
> @@ -103,9 +106,22 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
>  
>  	renc->output = output;
>  
> -	/*
> -	 * Attach the bridge to the encoder. The bridge will create the
> -	 * connector.
> -	 */
> -	return drm_bridge_attach(&renc->base, bridge, NULL, 0);
> +	/* Attach the bridge to the encoder. */
> +	ret = drm_bridge_attach(&renc->base, bridge, NULL,
> +				DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> +	if (ret) {
> +		dev_err(rcdu->dev, "failed to attach bridge for output %u\n",
> +			output);
> +		return ret;
> +	}
> +
> +	/* Create the connector for the chain of bridges. */
> +	connector = drm_bridge_connector_init(&rcdu->ddev, &renc->base);
> +	if (IS_ERR(connector)) {
> +		dev_err(rcdu->dev,
> +			"failed to created connector for output %u\n", output);
> +		return PTR_ERR(connector);
> +	}
> +
> +	return drm_connector_attach_encoder(connector, &renc->base);
>  }
> 

Re: [PATCH v3 0/4] Converter R-Car DU to the DRM bridge connector helper

[Kieran Bingham]

Hi Laurent,

On 20/05/2021 07:50, Laurent Pinchart wrote:
> Hello,
> 
> This patch series converts the R-Car DU driver to use the DRM bridge
> connector helper drm_bridge_connector_init().
> 
> The bulk of the v1 series was converting the adv7511, simple-bridge and
> dw-hdmi drivers to make connector creation optional (through the
> DRM_BRIDGE_ATTACH_NO_CONNECTOR flag), and have already been merged. v2
> included the remaining patches and has bitrotten. v3 rebased the code
> and should be ready for merge.
> 
> Patch 1/4 adds support to the dw-hdmi driver to attach to a downstream
> bridge if one is specified in DT. As the DT port number corresponding to
> the video output differs between platforms that integrate the dw-hdmi
> (some of them even don't have a video output port, which should probably
> be fixed, but that's out of scope for this series), the port number has
> to be specified by the platform glue layer.
> 
> Patch 2/4 then addresses the rcar-lvds driver. Instead of implementing
> direct support for DRM_BRIDGE_ATTACH_NO_CONNECTOR, it simply removes
> code that shouldn't have been in the driver in the first place by
> switching to the panel bridge helper.
> 
> Patch 3/4 specifies the port number in the R-Car dw-hdmi glue layer, as
> required by 1/4.
> 
> Patch 4/4 finally makes use of the drm_bridge_connector_init() helper.
> 
> The series has been tested on the Renesas R-Car Salvator-XS and Draak
> boards with the VGA, HDMI and LVDS outputs.

I'll start here,

This series fixes the connector issue I had on V3U, and removes the need
for me to carry a workaround. So that's excellent ;-)

For the series,

Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com> (on V3U)

> 
> Laurent Pinchart (4):
>   drm: bridge: dw-hdmi: Attach to next bridge if available
>   drm: rcar-du: lvds: Convert to DRM panel bridge helper
>   drm: rcar-du: dw-hdmi: Set output port number
>   drm: rcar-du: Use drm_bridge_connector_init() helper
> 
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  54 +++++++++-
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.c |  26 ++++-
>  drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c    |   1 +
>  drivers/gpu/drm/rcar-du/rcar_lvds.c       | 120 +++-------------------
>  include/drm/bridge/dw_hdmi.h              |   2 +
>  5 files changed, 89 insertions(+), 114 deletions(-)
>