[PATCH 0/4] drm: bridge: adv7511: Enable usage with DRM bridge connector helper

[PATCH 0/4] drm: bridge: adv7511: Enable usage with DRM bridge connector helper

[Laurent Pinchart]

Hello,

This patch series enables usage of the adv7511 driver with the DRM
bridge connector helper (drm_bridge_connector.c).

Patch 1/4 and 2/4 start by splitting EDID read and connector creation to
separate functions to ease review of 3/4 and 4/4. Patch 3/4 performs the
bulk of the work by implementing the DRM bridge connector-related
operations, and patch 4/4 then makes connector creation optional.

I've had trouble wrapping my head around the HPD handling code in the
av7511 driver (this is why I've CC'ed Rob and Sean who last touched
this). The split of the code between the .detect() operation and the HPD
IRQ seems a bit weird to me, and I haven't dared touching it as it also
appears fragile.

In particular, I'm not sure why we need to retrieve modes in the
.detect() operation. git blame didn't help, as the code has been there
since the beginning. I'd like to remove that completely, but to avoid
breakages, patch 3/4 only does so when the adv7511 is used without
creating a DRM connector.

Usage of both adv7511->status and adv7511->connector.status in the two
operations also seems awkward, and I would like to drop usage of the
latter in the new code path, but I also haven't dared refactoring that
yet. Feedback would be welcome.

Laurent Pinchart (4):
  drm: bridge: adv7511: Split EDID read to a separate function
  drm: bridge: adv7511: Split connector creation to a separate function
  drm: bridge: adv7511: Implement bridge connector operations
  drm: bridge: adv7511: Make connector creation optional

 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 137 +++++++++++++------
 1 file changed, 98 insertions(+), 39 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH 0/4] drm: bridge: adv7511: Enable usage with DRM bridge connector helper

[Laurent Pinchart]

Hello,

This patch series enables usage of the adv7511 driver with the DRM
bridge connector helper (drm_bridge_connector.c).

Patch 1/4 and 2/4 start by splitting EDID read and connector creation to
separate functions to ease review of 3/4 and 4/4. Patch 3/4 performs the
bulk of the work by implementing the DRM bridge connector-related
operations, and patch 4/4 then makes connector creation optional.

I've had trouble wrapping my head around the HPD handling code in the
av7511 driver (this is why I've CC'ed Rob and Sean who last touched
this). The split of the code between the .detect() operation and the HPD
IRQ seems a bit weird to me, and I haven't dared touching it as it also
appears fragile.

In particular, I'm not sure why we need to retrieve modes in the
.detect() operation. git blame didn't help, as the code has been there
since the beginning. I'd like to remove that completely, but to avoid
breakages, patch 3/4 only does so when the adv7511 is used without
creating a DRM connector.

Usage of both adv7511->status and adv7511->connector.status in the two
operations also seems awkward, and I would like to drop usage of the
latter in the new code path, but I also haven't dared refactoring that
yet. Feedback would be welcome.

Laurent Pinchart (4):
  drm: bridge: adv7511: Split EDID read to a separate function
  drm: bridge: adv7511: Split connector creation to a separate function
  drm: bridge: adv7511: Implement bridge connector operations
  drm: bridge: adv7511: Make connector creation optional

 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 137 +++++++++++++------
 1 file changed, 98 insertions(+), 39 deletions(-)

-- 
Regards,

Laurent Pinchart

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

[PATCH 1/4] drm: bridge: adv7511: Split EDID read to a separate function

[Laurent Pinchart]

To prepare for the implementation of the DRM bridge connector
operations, move EDID read out of adv7511_get_modes() to a separate
function.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 23 ++++++++++++++------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 87b58c1acff4..58d02e92b6b9 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -589,11 +589,10 @@ static int adv7511_get_edid_block(void *data, u8 *buf, unsigned int block,
  * ADV75xx helpers
  */
 
-static int adv7511_get_modes(struct adv7511 *adv7511,
-			     struct drm_connector *connector)
+static struct edid *adv7511_get_edid(struct adv7511 *adv7511,
+				     struct drm_connector *connector)
 {
 	struct edid *edid;
-	unsigned int count;
 
 	/* Reading the EDID only works if the device is powered */
 	if (!adv7511->powered) {
@@ -612,15 +611,25 @@ static int adv7511_get_modes(struct adv7511 *adv7511,
 	if (!adv7511->powered)
 		__adv7511_power_off(adv7511);
 
-
-	drm_connector_update_edid_property(connector, edid);
-	count = drm_add_edid_modes(connector, edid);
-
 	adv7511_set_config_csc(adv7511, connector, adv7511->rgb,
 			       drm_detect_hdmi_monitor(edid));
 
 	cec_s_phys_addr_from_edid(adv7511->cec_adap, edid);
 
+	return edid;
+}
+
+static int adv7511_get_modes(struct adv7511 *adv7511,
+			     struct drm_connector *connector)
+{
+	struct edid *edid;
+	unsigned int count;
+
+	edid = adv7511_get_edid(adv7511, connector);
+
+	drm_connector_update_edid_property(connector, edid);
+	count = drm_add_edid_modes(connector, edid);
+
 	kfree(edid);
 
 	return count;
-- 
Regards,

Laurent Pinchart

[PATCH 1/4] drm: bridge: adv7511: Split EDID read to a separate function

[Laurent Pinchart]

To prepare for the implementation of the DRM bridge connector
operations, move EDID read out of adv7511_get_modes() to a separate
function.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 23 ++++++++++++++------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 87b58c1acff4..58d02e92b6b9 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -589,11 +589,10 @@ static int adv7511_get_edid_block(void *data, u8 *buf, unsigned int block,
  * ADV75xx helpers
  */
 
-static int adv7511_get_modes(struct adv7511 *adv7511,
-			     struct drm_connector *connector)
+static struct edid *adv7511_get_edid(struct adv7511 *adv7511,
+				     struct drm_connector *connector)
 {
 	struct edid *edid;
-	unsigned int count;
 
 	/* Reading the EDID only works if the device is powered */
 	if (!adv7511->powered) {
@@ -612,15 +611,25 @@ static int adv7511_get_modes(struct adv7511 *adv7511,
 	if (!adv7511->powered)
 		__adv7511_power_off(adv7511);
 
-
-	drm_connector_update_edid_property(connector, edid);
-	count = drm_add_edid_modes(connector, edid);
-
 	adv7511_set_config_csc(adv7511, connector, adv7511->rgb,
 			       drm_detect_hdmi_monitor(edid));
 
 	cec_s_phys_addr_from_edid(adv7511->cec_adap, edid);
 
+	return edid;
+}
+
+static int adv7511_get_modes(struct adv7511 *adv7511,
+			     struct drm_connector *connector)
+{
+	struct edid *edid;
+	unsigned int count;
+
+	edid = adv7511_get_edid(adv7511, connector);
+
+	drm_connector_update_edid_property(connector, edid);
+	count = drm_add_edid_modes(connector, edid);
+
 	kfree(edid);
 
 	return count;
-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 1/4] drm: bridge: adv7511: Split EDID read to a separate function

[Sam Ravnborg]

Hi Laurent.

On Thu, Apr 09, 2020 at 03:46:07AM +0300, Laurent Pinchart wrote:
> To prepare for the implementation of the DRM bridge connector
> operations, move EDID read out of adv7511_get_modes() to a separate
> function.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

The code do not check if drm_do_get_edid() return NULL.
But all functions called with the edid seems to handle a NULL edid well.
So thats OK.

Acked-by: Sam Ravnborg <sam@ravnborg.org>

> ---
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 23 ++++++++++++++------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 87b58c1acff4..58d02e92b6b9 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -589,11 +589,10 @@ static int adv7511_get_edid_block(void *data, u8 *buf, unsigned int block,
>   * ADV75xx helpers
>   */
>  
> -static int adv7511_get_modes(struct adv7511 *adv7511,
> -			     struct drm_connector *connector)
> +static struct edid *adv7511_get_edid(struct adv7511 *adv7511,
> +				     struct drm_connector *connector)
>  {
>  	struct edid *edid;
> -	unsigned int count;
>  
>  	/* Reading the EDID only works if the device is powered */
>  	if (!adv7511->powered) {
> @@ -612,15 +611,25 @@ static int adv7511_get_modes(struct adv7511 *adv7511,
>  	if (!adv7511->powered)
>  		__adv7511_power_off(adv7511);
>  
> -
> -	drm_connector_update_edid_property(connector, edid);
> -	count = drm_add_edid_modes(connector, edid);
> -
>  	adv7511_set_config_csc(adv7511, connector, adv7511->rgb,
>  			       drm_detect_hdmi_monitor(edid));
>  
>  	cec_s_phys_addr_from_edid(adv7511->cec_adap, edid);
>  
> +	return edid;
> +}
> +
> +static int adv7511_get_modes(struct adv7511 *adv7511,
> +			     struct drm_connector *connector)
> +{
> +	struct edid *edid;
> +	unsigned int count;
> +
> +	edid = adv7511_get_edid(adv7511, connector);
> +
> +	drm_connector_update_edid_property(connector, edid);
> +	count = drm_add_edid_modes(connector, edid);
> +
>  	kfree(edid);
>  
>  	return count;
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm: bridge: adv7511: Split EDID read to a separate function

[Sam Ravnborg]

Hi Laurent.

On Thu, Apr 09, 2020 at 03:46:07AM +0300, Laurent Pinchart wrote:
> To prepare for the implementation of the DRM bridge connector
> operations, move EDID read out of adv7511_get_modes() to a separate
> function.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

The code do not check if drm_do_get_edid() return NULL.
But all functions called with the edid seems to handle a NULL edid well.
So thats OK.

Acked-by: Sam Ravnborg <sam@ravnborg.org>

> ---
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 23 ++++++++++++++------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 87b58c1acff4..58d02e92b6b9 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -589,11 +589,10 @@ static int adv7511_get_edid_block(void *data, u8 *buf, unsigned int block,
>   * ADV75xx helpers
>   */
>  
> -static int adv7511_get_modes(struct adv7511 *adv7511,
> -			     struct drm_connector *connector)
> +static struct edid *adv7511_get_edid(struct adv7511 *adv7511,
> +				     struct drm_connector *connector)
>  {
>  	struct edid *edid;
> -	unsigned int count;
>  
>  	/* Reading the EDID only works if the device is powered */
>  	if (!adv7511->powered) {
> @@ -612,15 +611,25 @@ static int adv7511_get_modes(struct adv7511 *adv7511,
>  	if (!adv7511->powered)
>  		__adv7511_power_off(adv7511);
>  
> -
> -	drm_connector_update_edid_property(connector, edid);
> -	count = drm_add_edid_modes(connector, edid);
> -
>  	adv7511_set_config_csc(adv7511, connector, adv7511->rgb,
>  			       drm_detect_hdmi_monitor(edid));
>  
>  	cec_s_phys_addr_from_edid(adv7511->cec_adap, edid);
>  
> +	return edid;
> +}
> +
> +static int adv7511_get_modes(struct adv7511 *adv7511,
> +			     struct drm_connector *connector)
> +{
> +	struct edid *edid;
> +	unsigned int count;
> +
> +	edid = adv7511_get_edid(adv7511, connector);
> +
> +	drm_connector_update_edid_property(connector, edid);
> +	count = drm_add_edid_modes(connector, edid);
> +
>  	kfree(edid);
>  
>  	return count;
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/4] drm: bridge: adv7511: Split connector creation to a separate function

[Laurent Pinchart]

To prepare for making the connector creation optional, move the related
code out of adv7511_bridge_attach() to a separate function.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 62 +++++++++++++-------
 1 file changed, 40 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 58d02e92b6b9..e3b62ad95389 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -783,7 +783,10 @@ static void adv7511_mode_set(struct adv7511 *adv7511,
 	adv7511->f_tmds = mode->clock;
 }
 
-/* Connector funcs */
+/* -----------------------------------------------------------------------------
+ * DRM Connector Operations
+ */
+
 static struct adv7511 *connector_to_adv7511(struct drm_connector *connector)
 {
 	return container_of(connector, struct adv7511, connector);
@@ -827,7 +830,40 @@ static const struct drm_connector_funcs adv7511_connector_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
-/* Bridge funcs */
+static int adv7511_connector_init(struct adv7511 *adv)
+{
+	struct drm_bridge *bridge = &adv->bridge;
+	int ret;
+
+	if (!bridge->encoder) {
+		DRM_ERROR("Parent encoder object not found");
+		return -ENODEV;
+	}
+
+	if (adv->i2c_main->irq)
+		adv->connector.polled = DRM_CONNECTOR_POLL_HPD;
+	else
+		adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT |
+				DRM_CONNECTOR_POLL_DISCONNECT;
+
+	ret = drm_connector_init(bridge->dev, &adv->connector,
+				 &adv7511_connector_funcs,
+				 DRM_MODE_CONNECTOR_HDMIA);
+	if (ret) {
+		DRM_ERROR("Failed to initialize connector with drm\n");
+		return ret;
+	}
+	drm_connector_helper_add(&adv->connector,
+				 &adv7511_connector_helper_funcs);
+	drm_connector_attach_encoder(&adv->connector, bridge->encoder);
+
+	return 0;
+}
+
+/* -----------------------------------------------------------------------------
+ * DRM Bridge Operations
+ */
+
 static struct adv7511 *bridge_to_adv7511(struct drm_bridge *bridge)
 {
 	return container_of(bridge, struct adv7511, bridge);
@@ -867,27 +903,9 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge,
 		return -EINVAL;
 	}
 
-	if (!bridge->encoder) {
-		DRM_ERROR("Parent encoder object not found");
-		return -ENODEV;
-	}
-
-	if (adv->i2c_main->irq)
-		adv->connector.polled = DRM_CONNECTOR_POLL_HPD;
-	else
-		adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT |
-				DRM_CONNECTOR_POLL_DISCONNECT;
-
-	ret = drm_connector_init(bridge->dev, &adv->connector,
-				 &adv7511_connector_funcs,
-				 DRM_MODE_CONNECTOR_HDMIA);
-	if (ret) {
-		DRM_ERROR("Failed to initialize connector with drm\n");
+	ret = adv7511_connector_init(adv);
+	if (ret < 0)
 		return ret;
-	}
-	drm_connector_helper_add(&adv->connector,
-				 &adv7511_connector_helper_funcs);
-	drm_connector_attach_encoder(&adv->connector, bridge->encoder);
 
 	if (adv->type == ADV7533 || adv->type == ADV7535)
 		ret = adv7533_attach_dsi(adv);
-- 
Regards,

Laurent Pinchart

[PATCH 2/4] drm: bridge: adv7511: Split connector creation to a separate function

[Laurent Pinchart]

To prepare for making the connector creation optional, move the related
code out of adv7511_bridge_attach() to a separate function.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 62 +++++++++++++-------
 1 file changed, 40 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 58d02e92b6b9..e3b62ad95389 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -783,7 +783,10 @@ static void adv7511_mode_set(struct adv7511 *adv7511,
 	adv7511->f_tmds = mode->clock;
 }
 
-/* Connector funcs */
+/* -----------------------------------------------------------------------------
+ * DRM Connector Operations
+ */
+
 static struct adv7511 *connector_to_adv7511(struct drm_connector *connector)
 {
 	return container_of(connector, struct adv7511, connector);
@@ -827,7 +830,40 @@ static const struct drm_connector_funcs adv7511_connector_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
-/* Bridge funcs */
+static int adv7511_connector_init(struct adv7511 *adv)
+{
+	struct drm_bridge *bridge = &adv->bridge;
+	int ret;
+
+	if (!bridge->encoder) {
+		DRM_ERROR("Parent encoder object not found");
+		return -ENODEV;
+	}
+
+	if (adv->i2c_main->irq)
+		adv->connector.polled = DRM_CONNECTOR_POLL_HPD;
+	else
+		adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT |
+				DRM_CONNECTOR_POLL_DISCONNECT;
+
+	ret = drm_connector_init(bridge->dev, &adv->connector,
+				 &adv7511_connector_funcs,
+				 DRM_MODE_CONNECTOR_HDMIA);
+	if (ret) {
+		DRM_ERROR("Failed to initialize connector with drm\n");
+		return ret;
+	}
+	drm_connector_helper_add(&adv->connector,
+				 &adv7511_connector_helper_funcs);
+	drm_connector_attach_encoder(&adv->connector, bridge->encoder);
+
+	return 0;
+}
+
+/* -----------------------------------------------------------------------------
+ * DRM Bridge Operations
+ */
+
 static struct adv7511 *bridge_to_adv7511(struct drm_bridge *bridge)
 {
 	return container_of(bridge, struct adv7511, bridge);
@@ -867,27 +903,9 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge,
 		return -EINVAL;
 	}
 
-	if (!bridge->encoder) {
-		DRM_ERROR("Parent encoder object not found");
-		return -ENODEV;
-	}
-
-	if (adv->i2c_main->irq)
-		adv->connector.polled = DRM_CONNECTOR_POLL_HPD;
-	else
-		adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT |
-				DRM_CONNECTOR_POLL_DISCONNECT;
-
-	ret = drm_connector_init(bridge->dev, &adv->connector,
-				 &adv7511_connector_funcs,
-				 DRM_MODE_CONNECTOR_HDMIA);
-	if (ret) {
-		DRM_ERROR("Failed to initialize connector with drm\n");
+	ret = adv7511_connector_init(adv);
+	if (ret < 0)
 		return ret;
-	}
-	drm_connector_helper_add(&adv->connector,
-				 &adv7511_connector_helper_funcs);
-	drm_connector_attach_encoder(&adv->connector, bridge->encoder);
 
 	if (adv->type == ADV7533 || adv->type == ADV7535)
 		ret = adv7533_attach_dsi(adv);
-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 2/4] drm: bridge: adv7511: Split connector creation to a separate function

[Sam Ravnborg]

Hi Laurent.

On Thu, Apr 09, 2020 at 03:46:08AM +0300, Laurent Pinchart wrote:
> To prepare for making the connector creation optional, move the related
> code out of adv7511_bridge_attach() to a separate function.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

On nit below, but otherwise:
Acked-by: Sam Ravnborg <sam@ravnborg.org>

	Sam

> ---
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 62 +++++++++++++-------
>  1 file changed, 40 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 58d02e92b6b9..e3b62ad95389 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -783,7 +783,10 @@ static void adv7511_mode_set(struct adv7511 *adv7511,
>  	adv7511->f_tmds = mode->clock;
>  }
>  
> -/* Connector funcs */
> +/* -----------------------------------------------------------------------------
> + * DRM Connector Operations
> + */
> +
>  static struct adv7511 *connector_to_adv7511(struct drm_connector *connector)
>  {
>  	return container_of(connector, struct adv7511, connector);
> @@ -827,7 +830,40 @@ static const struct drm_connector_funcs adv7511_connector_funcs = {
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  };
>  
> -/* Bridge funcs */
> +static int adv7511_connector_init(struct adv7511 *adv)
> +{
> +	struct drm_bridge *bridge = &adv->bridge;
> +	int ret;
> +
> +	if (!bridge->encoder) {
> +		DRM_ERROR("Parent encoder object not found");
> +		return -ENODEV;
> +	}
> +
> +	if (adv->i2c_main->irq)
> +		adv->connector.polled = DRM_CONNECTOR_POLL_HPD;
> +	else
> +		adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT |
> +				DRM_CONNECTOR_POLL_DISCONNECT;
> +
> +	ret = drm_connector_init(bridge->dev, &adv->connector,
> +				 &adv7511_connector_funcs,
> +				 DRM_MODE_CONNECTOR_HDMIA);
> +	if (ret) {

Here we test for ret != 0

> +		DRM_ERROR("Failed to initialize connector with drm\n");
> +		return ret;
> +	}
> +	drm_connector_helper_add(&adv->connector,
> +				 &adv7511_connector_helper_funcs);
> +	drm_connector_attach_encoder(&adv->connector, bridge->encoder);
> +
> +	return 0;
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * DRM Bridge Operations
> + */
> +
>  static struct adv7511 *bridge_to_adv7511(struct drm_bridge *bridge)
>  {
>  	return container_of(bridge, struct adv7511, bridge);
> @@ -867,27 +903,9 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge,
>  		return -EINVAL;
>  	}
>  
> -	if (!bridge->encoder) {
> -		DRM_ERROR("Parent encoder object not found");
> -		return -ENODEV;
> -	}
> -
> -	if (adv->i2c_main->irq)
> -		adv->connector.polled = DRM_CONNECTOR_POLL_HPD;
> -	else
> -		adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT |
> -				DRM_CONNECTOR_POLL_DISCONNECT;
> -
> -	ret = drm_connector_init(bridge->dev, &adv->connector,
> -				 &adv7511_connector_funcs,
> -				 DRM_MODE_CONNECTOR_HDMIA);
> -	if (ret) {
> -		DRM_ERROR("Failed to initialize connector with drm\n");
> +	ret = adv7511_connector_init(adv);
> +	if (ret < 0)
Here we test for ret < 0

The code works - but it is inconsistent.
drm_connector_init() is documented to:
"Zero on success, error code on failure."

>  		return ret;
> -	}
> -	drm_connector_helper_add(&adv->connector,
> -				 &adv7511_connector_helper_funcs);
> -	drm_connector_attach_encoder(&adv->connector, bridge->encoder);
>  
>  	if (adv->type == ADV7533 || adv->type == ADV7535)
>  		ret = adv7533_attach_dsi(adv);
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/4] drm: bridge: adv7511: Split connector creation to a separate function

[Sam Ravnborg]

Hi Laurent.

On Thu, Apr 09, 2020 at 03:46:08AM +0300, Laurent Pinchart wrote:
> To prepare for making the connector creation optional, move the related
> code out of adv7511_bridge_attach() to a separate function.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

On nit below, but otherwise:
Acked-by: Sam Ravnborg <sam@ravnborg.org>

	Sam

> ---
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 62 +++++++++++++-------
>  1 file changed, 40 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 58d02e92b6b9..e3b62ad95389 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -783,7 +783,10 @@ static void adv7511_mode_set(struct adv7511 *adv7511,
>  	adv7511->f_tmds = mode->clock;
>  }
>  
> -/* Connector funcs */
> +/* -----------------------------------------------------------------------------
> + * DRM Connector Operations
> + */
> +
>  static struct adv7511 *connector_to_adv7511(struct drm_connector *connector)
>  {
>  	return container_of(connector, struct adv7511, connector);
> @@ -827,7 +830,40 @@ static const struct drm_connector_funcs adv7511_connector_funcs = {
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  };
>  
> -/* Bridge funcs */
> +static int adv7511_connector_init(struct adv7511 *adv)
> +{
> +	struct drm_bridge *bridge = &adv->bridge;
> +	int ret;
> +
> +	if (!bridge->encoder) {
> +		DRM_ERROR("Parent encoder object not found");
> +		return -ENODEV;
> +	}
> +
> +	if (adv->i2c_main->irq)
> +		adv->connector.polled = DRM_CONNECTOR_POLL_HPD;
> +	else
> +		adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT |
> +				DRM_CONNECTOR_POLL_DISCONNECT;
> +
> +	ret = drm_connector_init(bridge->dev, &adv->connector,
> +				 &adv7511_connector_funcs,
> +				 DRM_MODE_CONNECTOR_HDMIA);
> +	if (ret) {

Here we test for ret != 0

> +		DRM_ERROR("Failed to initialize connector with drm\n");
> +		return ret;
> +	}
> +	drm_connector_helper_add(&adv->connector,
> +				 &adv7511_connector_helper_funcs);
> +	drm_connector_attach_encoder(&adv->connector, bridge->encoder);
> +
> +	return 0;
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * DRM Bridge Operations
> + */
> +
>  static struct adv7511 *bridge_to_adv7511(struct drm_bridge *bridge)
>  {
>  	return container_of(bridge, struct adv7511, bridge);
> @@ -867,27 +903,9 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge,
>  		return -EINVAL;
>  	}
>  
> -	if (!bridge->encoder) {
> -		DRM_ERROR("Parent encoder object not found");
> -		return -ENODEV;
> -	}
> -
> -	if (adv->i2c_main->irq)
> -		adv->connector.polled = DRM_CONNECTOR_POLL_HPD;
> -	else
> -		adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT |
> -				DRM_CONNECTOR_POLL_DISCONNECT;
> -
> -	ret = drm_connector_init(bridge->dev, &adv->connector,
> -				 &adv7511_connector_funcs,
> -				 DRM_MODE_CONNECTOR_HDMIA);
> -	if (ret) {
> -		DRM_ERROR("Failed to initialize connector with drm\n");
> +	ret = adv7511_connector_init(adv);
> +	if (ret < 0)
Here we test for ret < 0

The code works - but it is inconsistent.
drm_connector_init() is documented to:
"Zero on success, error code on failure."

>  		return ret;
> -	}
> -	drm_connector_helper_add(&adv->connector,
> -				 &adv7511_connector_helper_funcs);
> -	drm_connector_attach_encoder(&adv->connector, bridge->encoder);
>  
>  	if (adv->type == ADV7533 || adv->type == ADV7535)
>  		ret = adv7533_attach_dsi(adv);
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/4] drm: bridge: adv7511: Implement bridge connector operations

[Laurent Pinchart]

Implement the bridge connector-related .get_edid(), .detect() and
.hpd_notify() operations, and report the related bridge capabilities.

Output status detection is implemented using the same backend as for the
DRM connector, but requires making mode retrieval at detection time
optional as no pointer to the connector is available to the bridge
.detect() operation. The reason for the need to retrieve modes at
detection time is unclear to me, and this may benefit from further
refactoring of hot plug handling code.

Hot plug detection is notified through the bridge HPD notification
framework when the bridge is used without creating a connector, and
falls back to the existing implementation otherwise. CEC handling of
disconnection is handled in the new .hpd_notify() operation in the new
code path.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 43 ++++++++++++++++++--
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index e3b62ad95389..723560b36ee0 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -443,9 +443,14 @@ static void adv7511_hpd_work(struct work_struct *work)
 
 	if (adv7511->connector.status != status) {
 		adv7511->connector.status = status;
-		if (status == connector_status_disconnected)
-			cec_phys_addr_invalidate(adv7511->cec_adap);
-		drm_kms_helper_hotplug_event(adv7511->connector.dev);
+
+		if (adv7511->connector.dev) {
+			if (status == connector_status_disconnected)
+				cec_phys_addr_invalidate(adv7511->cec_adap);
+			drm_kms_helper_hotplug_event(adv7511->connector.dev);
+		} else {
+			drm_bridge_hpd_notify(&adv7511->bridge, status);
+		}
 	}
 }
 
@@ -661,7 +666,8 @@ adv7511_detect(struct adv7511 *adv7511, struct drm_connector *connector)
 	if (status == connector_status_connected && hpd && adv7511->powered) {
 		regcache_mark_dirty(adv7511->regmap);
 		adv7511_power_on(adv7511);
-		adv7511_get_modes(adv7511, connector);
+		if (connector)
+			adv7511_get_modes(adv7511, connector);
 		if (adv7511->status == connector_status_connected)
 			status = connector_status_disconnected;
 	} else {
@@ -917,11 +923,38 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge,
 	return ret;
 }
 
+static enum drm_connector_status adv7511_bridge_detect(struct drm_bridge *bridge)
+{
+	struct adv7511 *adv = bridge_to_adv7511(bridge);
+
+	return adv7511_detect(adv, NULL);
+}
+
+static struct edid *adv7511_bridge_get_edid(struct drm_bridge *bridge,
+					    struct drm_connector *connector)
+{
+	struct adv7511 *adv = bridge_to_adv7511(bridge);
+
+	return adv7511_get_edid(adv, connector);
+}
+
+static void adv7511_bridge_hpd_notify(struct drm_bridge *bridge,
+				      enum drm_connector_status status)
+{
+	struct adv7511 *adv = bridge_to_adv7511(bridge);
+
+	if (status == connector_status_disconnected)
+		cec_phys_addr_invalidate(adv->cec_adap);
+}
+
 static const struct drm_bridge_funcs adv7511_bridge_funcs = {
 	.enable = adv7511_bridge_enable,
 	.disable = adv7511_bridge_disable,
 	.mode_set = adv7511_bridge_mode_set,
 	.attach = adv7511_bridge_attach,
+	.detect = adv7511_bridge_detect,
+	.get_edid = adv7511_bridge_get_edid,
+	.hpd_notify = adv7511_bridge_hpd_notify,
 };
 
 /* -----------------------------------------------------------------------------
@@ -1250,6 +1283,8 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
 		goto err_unregister_cec;
 
 	adv7511->bridge.funcs = &adv7511_bridge_funcs;
+	adv7511->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID
+			    | DRM_BRIDGE_OP_HPD;
 	adv7511->bridge.of_node = dev->of_node;
 
 	drm_bridge_add(&adv7511->bridge);
-- 
Regards,

Laurent Pinchart

[PATCH 3/4] drm: bridge: adv7511: Implement bridge connector operations

[Laurent Pinchart]

Implement the bridge connector-related .get_edid(), .detect() and
.hpd_notify() operations, and report the related bridge capabilities.

Output status detection is implemented using the same backend as for the
DRM connector, but requires making mode retrieval at detection time
optional as no pointer to the connector is available to the bridge
.detect() operation. The reason for the need to retrieve modes at
detection time is unclear to me, and this may benefit from further
refactoring of hot plug handling code.

Hot plug detection is notified through the bridge HPD notification
framework when the bridge is used without creating a connector, and
falls back to the existing implementation otherwise. CEC handling of
disconnection is handled in the new .hpd_notify() operation in the new
code path.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 43 ++++++++++++++++++--
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index e3b62ad95389..723560b36ee0 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -443,9 +443,14 @@ static void adv7511_hpd_work(struct work_struct *work)
 
 	if (adv7511->connector.status != status) {
 		adv7511->connector.status = status;
-		if (status == connector_status_disconnected)
-			cec_phys_addr_invalidate(adv7511->cec_adap);
-		drm_kms_helper_hotplug_event(adv7511->connector.dev);
+
+		if (adv7511->connector.dev) {
+			if (status == connector_status_disconnected)
+				cec_phys_addr_invalidate(adv7511->cec_adap);
+			drm_kms_helper_hotplug_event(adv7511->connector.dev);
+		} else {
+			drm_bridge_hpd_notify(&adv7511->bridge, status);
+		}
 	}
 }
 
@@ -661,7 +666,8 @@ adv7511_detect(struct adv7511 *adv7511, struct drm_connector *connector)
 	if (status == connector_status_connected && hpd && adv7511->powered) {
 		regcache_mark_dirty(adv7511->regmap);
 		adv7511_power_on(adv7511);
-		adv7511_get_modes(adv7511, connector);
+		if (connector)
+			adv7511_get_modes(adv7511, connector);
 		if (adv7511->status == connector_status_connected)
 			status = connector_status_disconnected;
 	} else {
@@ -917,11 +923,38 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge,
 	return ret;
 }
 
+static enum drm_connector_status adv7511_bridge_detect(struct drm_bridge *bridge)
+{
+	struct adv7511 *adv = bridge_to_adv7511(bridge);
+
+	return adv7511_detect(adv, NULL);
+}
+
+static struct edid *adv7511_bridge_get_edid(struct drm_bridge *bridge,
+					    struct drm_connector *connector)
+{
+	struct adv7511 *adv = bridge_to_adv7511(bridge);
+
+	return adv7511_get_edid(adv, connector);
+}
+
+static void adv7511_bridge_hpd_notify(struct drm_bridge *bridge,
+				      enum drm_connector_status status)
+{
+	struct adv7511 *adv = bridge_to_adv7511(bridge);
+
+	if (status == connector_status_disconnected)
+		cec_phys_addr_invalidate(adv->cec_adap);
+}
+
 static const struct drm_bridge_funcs adv7511_bridge_funcs = {
 	.enable = adv7511_bridge_enable,
 	.disable = adv7511_bridge_disable,
 	.mode_set = adv7511_bridge_mode_set,
 	.attach = adv7511_bridge_attach,
+	.detect = adv7511_bridge_detect,
+	.get_edid = adv7511_bridge_get_edid,
+	.hpd_notify = adv7511_bridge_hpd_notify,
 };
 
 /* -----------------------------------------------------------------------------
@@ -1250,6 +1283,8 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
 		goto err_unregister_cec;
 
 	adv7511->bridge.funcs = &adv7511_bridge_funcs;
+	adv7511->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID
+			    | DRM_BRIDGE_OP_HPD;
 	adv7511->bridge.of_node = dev->of_node;
 
 	drm_bridge_add(&adv7511->bridge);
-- 
Regards,

Laurent Pinchart

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

[PATCH 4/4] drm: bridge: adv7511: Make connector creation optional

[Laurent Pinchart]

Now that the driver supports all the connector-related bridge
operations, make the connector creation optional. This enables usage of
the adv7511 with the DRM bridge connector helper.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 723560b36ee0..60efd19fa4df 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -902,17 +902,14 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge,
 				 enum drm_bridge_attach_flags flags)
 {
 	struct adv7511 *adv = bridge_to_adv7511(bridge);
-	int ret;
+	int ret = 0;
 
-	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
-		DRM_ERROR("Fix bridge driver to make connector optional!");
-		return -EINVAL;
+	if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
+		ret = adv7511_connector_init(adv);
+		if (ret < 0)
+			return ret;
 	}
 
-	ret = adv7511_connector_init(adv);
-	if (ret < 0)
-		return ret;
-
 	if (adv->type == ADV7533 || adv->type == ADV7535)
 		ret = adv7533_attach_dsi(adv);
 
-- 
Regards,

Laurent Pinchart

[PATCH 4/4] drm: bridge: adv7511: Make connector creation optional

[Laurent Pinchart]

Now that the driver supports all the connector-related bridge
operations, make the connector creation optional. This enables usage of
the adv7511 with the DRM bridge connector helper.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 723560b36ee0..60efd19fa4df 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -902,17 +902,14 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge,
 				 enum drm_bridge_attach_flags flags)
 {
 	struct adv7511 *adv = bridge_to_adv7511(bridge);
-	int ret;
+	int ret = 0;
 
-	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
-		DRM_ERROR("Fix bridge driver to make connector optional!");
-		return -EINVAL;
+	if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
+		ret = adv7511_connector_init(adv);
+		if (ret < 0)
+			return ret;
 	}
 
-	ret = adv7511_connector_init(adv);
-	if (ret < 0)
-		return ret;
-
 	if (adv->type == ADV7533 || adv->type == ADV7535)
 		ret = adv7533_attach_dsi(adv);
 
-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 4/4] drm: bridge: adv7511: Make connector creation optional

[Sam Ravnborg]

On Thu, Apr 09, 2020 at 03:46:10AM +0300, Laurent Pinchart wrote:
> Now that the driver supports all the connector-related bridge
> operations, make the connector creation optional. This enables usage of
> the adv7511 with the DRM bridge connector helper.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>

	Sam

> ---
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 723560b36ee0..60efd19fa4df 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -902,17 +902,14 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge,
>  				 enum drm_bridge_attach_flags flags)
>  {
>  	struct adv7511 *adv = bridge_to_adv7511(bridge);
> -	int ret;
> +	int ret = 0;
>  
> -	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
> -		DRM_ERROR("Fix bridge driver to make connector optional!");
> -		return -EINVAL;
> +	if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
> +		ret = adv7511_connector_init(adv);
> +		if (ret < 0)
> +			return ret;
>  	}
>  
> -	ret = adv7511_connector_init(adv);
> -	if (ret < 0)
> -		return ret;
> -
>  	if (adv->type == ADV7533 || adv->type == ADV7535)
>  		ret = adv7533_attach_dsi(adv);
>  
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] drm: bridge: adv7511: Make connector creation optional

[Sam Ravnborg]

On Thu, Apr 09, 2020 at 03:46:10AM +0300, Laurent Pinchart wrote:
> Now that the driver supports all the connector-related bridge
> operations, make the connector creation optional. This enables usage of
> the adv7511 with the DRM bridge connector helper.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>

	Sam

> ---
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 723560b36ee0..60efd19fa4df 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -902,17 +902,14 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge,
>  				 enum drm_bridge_attach_flags flags)
>  {
>  	struct adv7511 *adv = bridge_to_adv7511(bridge);
> -	int ret;
> +	int ret = 0;
>  
> -	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
> -		DRM_ERROR("Fix bridge driver to make connector optional!");
> -		return -EINVAL;
> +	if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
> +		ret = adv7511_connector_init(adv);
> +		if (ret < 0)
> +			return ret;
>  	}
>  
> -	ret = adv7511_connector_init(adv);
> -	if (ret < 0)
> -		return ret;
> -
>  	if (adv->type == ADV7533 || adv->type == ADV7535)
>  		ret = adv7533_attach_dsi(adv);
>  
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel