Skip to contentSkip to conversation

drm/msm: Use device graph to parse connected panels

PATCH Superseded Archit Taneja 2 participants 6 messages Jun 22, 2015 – Jun 23, 2015

Subsystems (4)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

cover drm/msm: Use device graph to parse connected panels

drm/msm currently relies on phandles/child nodes to get data about connected panels to LVDS and DSI. This method has known limitations.

Use device graphs in DT to represent the connections between the encoder outputs and the panels. Use of_graph helpers in the driver to get the panel device node.

The usage of device graphs should also simplify management in dual dsi mode. I haven't tried this out yet, though.

Archit Taneja (3):
drm/msm: dsi host: add missing of_node_put()
drm/msm: dsi host: Use device graph parsing to parse connected panel drm/msm: mdp4 lvds: get panel node via of graph parsing

 Documentation/devicetree/bindings/drm/msm/dsi.txt | 15 ++++++
 drivers/gpu/drm/msm/dsi/dsi_host.c                | 62 ++++++++++++++++++-----
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c           | 32 ++++++++----
 drivers/gpu/drm/msm/msm_drv.h                     |  1 +
 4 files changed, 87 insertions(+), 23 deletions(-)
Signature · 7 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
#1 lore raw

PATCH 1/3 drm/msm: dsi host: add missing of_node_put()

Decrement device node refcount if of_get_child_by_name is successfully called.

---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/gpu/drm/msm/dsi/dsi_host.c +2 -0
index de04009..1751659 100644
@@ -1582,6 +1582,8 @@ int msm_dsi_host_register(struct mipi_dsi_host *host, bool check_defer)
 			node = of_get_child_by_name(msm_host->pdev->dev.of_node,
 							"panel");
 			if (node) {
+				of_node_put(node);
+
 				if (!of_drm_find_panel(node))
 					return -EPROBE_DEFER;
 			}
Signature · 7 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
#2 lore raw

Re: [PATCH 1/3] drm/msm: dsi host: add missing of_node_put()

Quoted 13 lines · Archit Taneja

Decrement device node refcount if of_get_child_by_name is successfully called.

---
  drivers/gpu/drm/msm/dsi/dsi_host.c | 2 ++
  1 file changed, 2 insertions(+)
drivers/gpu/drm/msm/dsi/dsi_host.c +0 -0
index de04009..1751659 100644

drivers/gpu/drm/msm/dsi/dsi_host.c int msm_dsi_host_register(struct mipi_dsi_host *host, bool check_defer) · line 1586

Quoted 5 lines
drivers/gpu/drm/msm/dsi/dsi_host.c +2 -0
@@ -1582,6 +1582,8 @@ int msm_dsi_host_register(struct mipi_dsi_host *host, bool check_defer)
  			node = of_get_child_by_name(msm_host->pdev->dev.of_node,
  							"panel");
  			if (node) {
+				of_node_put(node);
+

Atleast from the current state of code, It looks like the driver is going to refer to the node of_node_put?, So I think this is not the right place to have of_node_put.

Quoted 4 lines
drivers/gpu/drm/msm/dsi/dsi_host.c +0 -0
  				if (!of_drm_find_panel(node))
  					return -EPROBE_DEFER;
  			}
#3 lore raw

Re: [PATCH 1/3] drm/msm: dsi host: add missing of_node_put()

Quoted 11 lines · Srinivas Kandagatla
Quoted 8 lines · Archit Taneja

Decrement device node refcount if of_get_child_by_name is successfully called.

---
  drivers/gpu/drm/msm/dsi/dsi_host.c | 2 ++
  1 file changed, 2 insertions(+)
Quoted 16 lines

drivers/gpu/drm/msm/dsi/dsi_host.c int msm_dsi_host_register(struct mipi_dsi_host · line 1587

Quoted 12 lines
drivers/gpu/drm/msm/dsi/dsi_host.c +2 -0
index de04009..1751659 100644
@@ -1582,6 +1582,8 @@ int msm_dsi_host_register(struct mipi_dsi_host
 *host, bool check_defer)
              node = of_get_child_by_name(msm_host->pdev->dev.of_node,
                              "panel");
              if (node) {
+                of_node_put(node);
+

Atleast from the current state of code, It looks like the driver is going to refer to the node of_node_put?, So I think this is not the right place to have of_node_put.

That's right. I made the same mistake in the later patches too. I'll fix these.

Thanks,
Archit

Signature · 6 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
#4 lore raw

PATCH 2/3 drm/msm: dsi host: Use device graph parsing to parse connected panel

In reply to Archit Taneja

The dsi host looks for the connected panel node by parsing for a child named 'panel'. This hierarchy isn't very flexible. The connected panel is forced to be a child to the dsi host, and hence, a mipi dsi device. This isn't suitable for dsi devices that don't use mipi dsi as their control bus.

Follow the of_graph approach of creating ports and endpoints to represent the connections between the dsi host and the panel connected to it. In our case, the dsi host will only have one output port, linked to the panel's input port.

Update DT binding documentation with device graph usage info.

---
 Documentation/devicetree/bindings/drm/msm/dsi.txt | 15 ++++++
 drivers/gpu/drm/msm/dsi/dsi_host.c                | 64 +++++++++++++++++------
 2 files changed, 63 insertions(+), 16 deletions(-)
Documentation/devicetree/bindings/drm/msm/dsi.txt +15 -0
index 6ccd860..c88ec3c 100644
@@ -25,6 +25,9 @@ Required properties:
 - vddio-supply: phandle to vdd-io regulator device node
 - vdda-supply: phandle to vdda regulator device node
 - qcom,dsi-phy: phandle to DSI PHY device node
+- port: DSI controller output port. This contains one endpoint subnode, with its
+  remote-endpoint set to the phandle of the connected panel's endpoint.
+  See Documentation/devicetree/bindings/graph.txt for device graph info.
 
 Optional properties:
 - panel@0: Node of panel connected to this DSI controller.
@@ -101,6 +104,18 @@ Example:
 
 			power-supply = <...>;
 			backlight = <...>;
+
+			port {
+				panel_in: endpoint {
+					remote-endpoint = <&dsi0_out>;
+				};
+			};
+		};
+
+		port {
+			dsi0_out: endpoint {
+				remote-endpoint = <&panel_in>;
+			};
 		};
 	};
 
drivers/gpu/drm/msm/dsi/dsi_host.c +48 -16

103 lines not on this page · raw message

Signature · 7 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
#5 lore raw

PATCH 3/3 drm/msm: mdp4 lvds: get panel node via of graph parsing

In reply to Archit Taneja

We currently get the output connected to LVDS by looking for a phandle called 'qcom,lvds-panel' under the mdp DT node.

Use the more standard of_graph approach to create an lvds output port, and retrieve the panel node from the port's endpoint data.

---
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 32 +++++++++++++++++++++++---------
 drivers/gpu/drm/msm/msm_drv.h           |  1 +
 2 files changed, 24 insertions(+), 9 deletions(-)
drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c +23 -9
index 531e4ac..cf72eda 100644
@@ -241,22 +241,36 @@ int mdp4_enable(struct mdp4_kms *mdp4_kms)
 }
 
 #ifdef CONFIG_OF
-static struct drm_panel *detect_panel(struct drm_device *dev, const char *name)
+static struct drm_panel *detect_panel(struct drm_device *dev)
 {
-	struct device_node *n;
+	struct device_node *endpoint, *panel_node;
+	struct device_node *np = dev->dev->of_node;
 	struct drm_panel *panel = NULL;
 
-	n = of_parse_phandle(dev->dev->of_node, name, 0);
-	if (n) {
-		panel = of_drm_find_panel(n);
-		if (!panel)
-			panel = ERR_PTR(-EPROBE_DEFER);
+	endpoint = of_graph_get_next_endpoint(np, NULL);
+	if (IS_ERR(endpoint)) {
+		dev_err(dev->dev, "no valid endpoint\n");
+		return ERR_CAST(endpoint);
 	}
 
+	of_node_put(endpoint);
+
+	panel_node = of_graph_get_remote_port_parent(endpoint);
+	if (IS_ERR(panel_node)) {
+		dev_err(dev->dev, "no valid panel node\n");
+		return ERR_CAST(panel_node);
+	}
+
+	of_node_put(panel_node);
+
+	panel = of_drm_find_panel(panel_node);
+	if (IS_ERR(panel))
+		panel = ERR_PTR(-EPROBE_DEFER);
+
 	return panel;
 }
 #else
-static struct drm_panel *detect_panel(struct drm_device *dev, const char *name)
+static struct drm_panel *detect_panel(struct drm_device *dev)
 {
 	// ??? maybe use a module param to specify which panel is attached?
 }
@@ -294,7 +308,7 @@ static int modeset_init(struct mdp4_kms *mdp4_kms)
 	 * Setup the LCDC/LVDS path: RGB2 -> DMA_P -> LCDC -> LVDS:
 	 */
 
-	panel = detect_panel(dev, "qcom,lvds-panel");
+	panel = detect_panel(dev);
 	if (IS_ERR(panel)) {
 		ret = PTR_ERR(panel);
 		dev_err(dev->dev, "failed to detect LVDS panel: %d\n", ret);
drivers/gpu/drm/msm/msm_drv.h +1 -0
index e7c5ea1..0cbeb1d 100644
@@ -30,6 +30,7 @@
 #include <linux/list.h>
 #include <linux/iommu.h>
 #include <linux/types.h>
+#include <linux/of_graph.h>
 #include <asm/sizes.h>
 
 #ifndef CONFIG_OF
Signature · 7 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel