Re: [PATCH v6 1/8] drm/msm/dp: Add eDP support via aux_bus

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 30/03/2022 19:02, Sankeerth Billakanti wrote:
This patch adds support for generic eDP sink through aux_bus. The eDP/DP
controller driver should support aux transactions originating from the
panel-edp driver and hence should be initialized and ready.

The panel bridge supporting the panel should be ready before the bridge
connector is initialized. The generic panel probe needs the controller
resources to be enabled to support the aux transactions originating from
the panel probe.

Signed-off-by: Sankeerth Billakanti <quic_sbillaka@xxxxxxxxxxx>
---

Changes in v6:
   - Remove initialization
   - Fix aux_bus node leak
   - Split the patches

  drivers/gpu/drm/msm/dp/dp_display.c | 54 +++++++++++++++++++++++++++++++++++--
  drivers/gpu/drm/msm/dp/dp_drm.c     | 10 ++++---
  drivers/gpu/drm/msm/dp/dp_parser.c  | 21 +--------------
  drivers/gpu/drm/msm/dp/dp_parser.h  |  1 +
  4 files changed, 60 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 382b3aa..e082d02 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -10,6 +10,7 @@
  #include <linux/component.h>
  #include <linux/of_irq.h>
  #include <linux/delay.h>
+#include <drm/dp/drm_dp_aux_bus.h>
#include "msm_drv.h"
  #include "msm_kms.h"
@@ -265,8 +266,6 @@ static int dp_display_bind(struct device *dev, struct device *master,
  		goto end;
  	}
- dp->dp_display.next_bridge = dp->parser->next_bridge;
-
  	dp->aux->drm_dev = drm;
  	rc = dp_aux_register(dp->aux);
  	if (rc) {
@@ -1524,6 +1523,53 @@ void msm_dp_debugfs_init(struct msm_dp *dp_display, struct drm_minor *minor)
  	}
  }
+static int dp_display_get_next_bridge(struct msm_dp *dp)
+{
+	int rc;
+	struct dp_display_private *dp_priv;
+	struct device_node *aux_bus;
+	struct device *dev;
+
+	dp_priv = container_of(dp, struct dp_display_private, dp_display);
+	dev = &dp_priv->pdev->dev;
+	aux_bus = of_get_child_by_name(dev->of_node, "aux-bus");
+
+	if (aux_bus) {
+		dp_display_host_init(dp_priv);
+		dp_catalog_ctrl_hpd_config(dp_priv->catalog);
+		enable_irq(dp_priv->irq);
+		dp_display_host_phy_init(dp_priv);
+
+		devm_of_dp_aux_populate_ep_devices(dp_priv->aux);
+
+		disable_irq(dp_priv->irq);
+		of_node_put(aux_bus);
+	}
+
+	/*
+	 * External bridges are mandatory for eDP interfaces: one has to
+	 * provide at least an eDP panel (which gets wrapped into panel-bridge).
+	 *
+	 * For DisplayPort interfaces external bridges are optional, so
+	 * silently ignore an error if one is not present (-ENODEV).
+	 */
+	rc = dp_parser_find_next_bridge(dp_priv->parser);
+	if (rc == -ENODEV) {
+		if (dp->connector_type == DRM_MODE_CONNECTOR_eDP) {

The more I think about these conditions, the closer I dislike them (yes, I added this one in one of the patches). I'd suggest to change dp->connector_type to boolean 'is_edp' field use it in all conditions instead.

+			DRM_ERROR("eDP: next bridge is not present\n");
+			return rc;
+		}
+	} else if (rc) {
+		if (rc != -EPROBE_DEFER)
+			DRM_ERROR("DP: error parsing next bridge: %d\n", rc);
+		return rc;
+	}
+
+	dp->next_bridge = dp_priv->parser->next_bridge;
+
+	return 0;
+}
+
  int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
  			struct drm_encoder *encoder)
  {
@@ -1547,6 +1593,10 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
dp_display->encoder = encoder; + ret = dp_display_get_next_bridge(dp_display);
+	if (ret)
+		return ret;
+
  	dp_display->bridge = dp_bridge_init(dp_display, dev, encoder);
  	if (IS_ERR(dp_display->bridge)) {
  		ret = PTR_ERR(dp_display->bridge);
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 7ce1aca..5254bd6 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -114,10 +114,12 @@ struct drm_bridge *dp_bridge_init(struct msm_dp *dp_display, struct drm_device *
  	bridge->funcs = &dp_bridge_ops;
  	bridge->type = dp_display->connector_type;
- bridge->ops =
-		DRM_BRIDGE_OP_DETECT |
-		DRM_BRIDGE_OP_HPD |
-		DRM_BRIDGE_OP_MODES;
+	if (bridge->type == DRM_MODE_CONNECTOR_DisplayPort) {

And in this case we can also check dp_display->connector_type (or the suggested dp_display->is_edp) for the uniformity of the code.

+		bridge->ops =
+			DRM_BRIDGE_OP_DETECT |
+			DRM_BRIDGE_OP_HPD |
+			DRM_BRIDGE_OP_MODES;

I think OP_MODES should be used for eDP, shouldn't it?

+	}
rc = drm_bridge_attach(encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
  	if (rc) {
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
index 1056b8d..6317dce 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -265,7 +265,7 @@ static int dp_parser_clock(struct dp_parser *parser)
  	return 0;
  }
-static int dp_parser_find_next_bridge(struct dp_parser *parser)
+int dp_parser_find_next_bridge(struct dp_parser *parser)
  {
  	struct device *dev = &parser->pdev->dev;
  	struct drm_bridge *bridge;
@@ -300,25 +300,6 @@ static int dp_parser_parse(struct dp_parser *parser, int connector_type)
  	if (rc)
  		return rc;
- /*
-	 * External bridges are mandatory for eDP interfaces: one has to
-	 * provide at least an eDP panel (which gets wrapped into panel-bridge).
-	 *
-	 * For DisplayPort interfaces external bridges are optional, so
-	 * silently ignore an error if one is not present (-ENODEV).
-	 */
-	rc = dp_parser_find_next_bridge(parser);
-	if (rc == -ENODEV) {
-		if (connector_type == DRM_MODE_CONNECTOR_eDP) {
-			DRM_ERROR("eDP: next bridge is not present\n");
-			return rc;
-		}
-	} else if (rc) {
-		if (rc != -EPROBE_DEFER)
-			DRM_ERROR("DP: error parsing next bridge: %d\n", rc);
-		return rc;
-	}
-
  	/* Map the corresponding regulator information according to
  	 * version. Currently, since we only have one supported platform,
  	 * mapping the regulator directly.
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h
index d371bae..091ff41 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.h
+++ b/drivers/gpu/drm/msm/dp/dp_parser.h
@@ -140,5 +140,6 @@ struct dp_parser {
   * can be parsed using this module.
   */
  struct dp_parser *dp_parser_get(struct platform_device *pdev);
+int dp_parser_find_next_bridge(struct dp_parser *parser);
#endif


--
With best wishes
Dmitry



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux