dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v8 0/7] Support DRM bridges on NVIDIA Tegra
@ 2020-06-17 22:26 Dmitry Osipenko
  2020-06-17 22:26 ` [PATCH v8 1/7] of_graph: add of_graph_presents() Dmitry Osipenko
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Dmitry Osipenko @ 2020-06-17 22:26 UTC (permalink / raw)
  To: Thierry Reding, Sam Ravnborg, Laurent Pinchart, Rob Herring,
	Frank Rowand
  Cc: linux-tegra, devicetree, linux-kernel, dri-devel

Hello,

This series adds initial support for the DRM bridges to NVIDIA Tegra DRM
driver. This is required by newer device-trees where we model the LVDS
encoder bridge properly.

Changelog:

v8: - The new of_graph_get_local_port() helper is replaced with the
      of_graph_presents(), which simply checks the graph presence in a
      given DT node. Thank to Laurent Pinchart for the suggestion!

    - The of_graph_get_local_port() is still there, but now it isn't a public
      function anymore. In the review to v7 Laurent Pinchart suggested that
      the function's doc-comments and name could be improved and I implemented
      these suggestions in v8.

    - A day ago I discovered that devm_drm_panel_bridge_add() requires
      panel to have connector type to be properly set, otherwise function
      rejects panels with the incomplete description. So, I checked what
      LVDS panels are used on Tegra and fixed the missing connector types
      in this new patch:

        drm/panel-simple: Add missing connector type for some panels

v7: - Removed the obscure unused structs (which GCC doesn't detect, but CLANG
      does) in the patch "Wrap directly-connected panel into DRM bridge",
      which was reported by kernel test robot for v6.

v6: - Added r-b and acks from Rob Herring and Sam Ravnborg.

    - Rebased on a recent linux-next, patches now apply without fuzz.

v5: - Added new patches that make drm_of_find_panel_or_bridge() more usable
      if graph isn't defined in a device-tree:

        of_graph: add of_graph_get_local_port()
        drm/of: Make drm_of_find_panel_or_bridge() to check graph's presence

    - Updated "Support DRM bridges" patch to use drm_of_find_panel_or_bridge()
      directly and added WARN_ON(output->panel || output->bridge) sanity-check.

    - Added new "Wrap directly-connected panel into DRM bridge" patch, as
      was suggested by Laurent Pinchart.

v4: - Following review comments that were made by Laurent Pinchart to the v3,
      we now create and use the "bridge connector".

v3: - Following recommendation from Sam Ravnborg, the new bridge attachment
      model is now being used, i.e. we ask bridge to *not* create a connector
      using the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag.

    - The bridge is now created only for the RGB (LVDS) output, and only
      when necessary. For now we don't need bridges for HDMI or DSI outputs.

    - I noticed that we're leaking OF node in the panel's error code path,
      this is fixed now by the new patch "Don't leak OF node on error".

v2: - Added the new "rgb: Don't register connector if bridge is used"
      patch, which hides the unused connector provided by the Tegra DRM
      driver when bridge is used, since bridge provides its own connector
      to us.

Dmitry Osipenko (7):
  of_graph: add of_graph_presents()
  drm/of: Make drm_of_find_panel_or_bridge() to check graph's presence
  drm/tegra: output: Don't leak OF node on error
  drm/tegra: output: Support DRM bridges
  drm/tegra: output: rgb: Support LVDS encoder bridge
  drm/tegra: output: rgb: Wrap directly-connected panel into DRM bridge
  drm/panel-simple: Add missing connector type for some panels

 drivers/gpu/drm/drm_of.c             |   9 +++
 drivers/gpu/drm/panel/panel-simple.c |   7 ++
 drivers/gpu/drm/tegra/drm.h          |   2 +
 drivers/gpu/drm/tegra/output.c       |  21 ++++--
 drivers/gpu/drm/tegra/rgb.c          | 102 ++++++++++++++-------------
 drivers/of/property.c                |  52 +++++++++++---
 include/linux/of_graph.h             |   6 ++
 7 files changed, 137 insertions(+), 62 deletions(-)

-- 
2.26.0

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v8 1/7] of_graph: add of_graph_presents()
  2020-06-17 22:26 [PATCH v8 0/7] Support DRM bridges on NVIDIA Tegra Dmitry Osipenko
@ 2020-06-17 22:26 ` Dmitry Osipenko
  2020-06-29 23:12   ` Rob Herring
  2020-06-17 22:26 ` [PATCH v8 2/7] drm/of: Make drm_of_find_panel_or_bridge() to check graph's presence Dmitry Osipenko
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Dmitry Osipenko @ 2020-06-17 22:26 UTC (permalink / raw)
  To: Thierry Reding, Sam Ravnborg, Laurent Pinchart, Rob Herring,
	Frank Rowand
  Cc: linux-tegra, devicetree, linux-kernel, dri-devel

In some case, like a DRM display code for example, it's useful to silently
check whether port node exists at all in a device-tree before proceeding
with parsing of the graph.

This patch adds of_graph_presents() that returns true if given device-tree
node contains OF graph port.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/of/property.c    | 52 +++++++++++++++++++++++++++++++++-------
 include/linux/of_graph.h |  6 +++++
 2 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 1f2086f4e7ce..b84ed6a7cf50 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -29,6 +29,48 @@
 
 #include "of_private.h"
 
+/**
+ * of_graph_get_first_local_port() - get first local port node
+ * @node: pointer to a local endpoint device_node
+ *
+ * Return: First local port node associated with local endpoint node linked
+ *	   to @node. Use of_node_put() on it when done.
+ */
+static struct device_node *
+of_graph_get_first_local_port(const struct device_node *node)
+{
+	struct device_node *ports, *port;
+
+	ports = of_get_child_by_name(node, "ports");
+	if (ports)
+		node = ports;
+
+	port = of_get_child_by_name(node, "port");
+	of_node_put(ports);
+
+	return port;
+}
+
+/**
+ * of_graph_presents() - check graph's presence
+ * @node: pointer to a device_node checked for the graph's presence
+ *
+ * Return: True if @node has a port or ports sub-node, false otherwise.
+ */
+bool of_graph_presents(const struct device_node *node)
+{
+	struct device_node *local;
+
+	local = of_graph_get_first_local_port(node);
+	if (!local)
+		return false;
+
+	of_node_put(local);
+
+	return true;
+}
+EXPORT_SYMBOL(of_graph_presents);
+
 /**
  * of_property_count_elems_of_size - Count the number of elements in a property
  *
@@ -608,15 +650,7 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
 	 * parent port node.
 	 */
 	if (!prev) {
-		struct device_node *node;
-
-		node = of_get_child_by_name(parent, "ports");
-		if (node)
-			parent = node;
-
-		port = of_get_child_by_name(parent, "port");
-		of_node_put(node);
-
+		port = of_graph_get_first_local_port(parent);
 		if (!port) {
 			pr_err("graph: no port node found in %pOF\n", parent);
 			return NULL;
diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
index 01038a6aade0..cc3028a0659d 100644
--- a/include/linux/of_graph.h
+++ b/include/linux/of_graph.h
@@ -38,6 +38,7 @@ struct of_endpoint {
 	     child = of_graph_get_next_endpoint(parent, child))
 
 #ifdef CONFIG_OF
+bool of_graph_presents(const struct device_node *node);
 int of_graph_parse_endpoint(const struct device_node *node,
 				struct of_endpoint *endpoint);
 int of_graph_get_endpoint_count(const struct device_node *np);
@@ -56,6 +57,11 @@ struct device_node *of_graph_get_remote_node(const struct device_node *node,
 					     u32 port, u32 endpoint);
 #else
 
+static inline bool of_graph_presents(const struct device_node *node)
+{
+	return false;
+}
+
 static inline int of_graph_parse_endpoint(const struct device_node *node,
 					struct of_endpoint *endpoint)
 {
-- 
2.26.0

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v8 2/7] drm/of: Make drm_of_find_panel_or_bridge() to check graph's presence
  2020-06-17 22:26 [PATCH v8 0/7] Support DRM bridges on NVIDIA Tegra Dmitry Osipenko
  2020-06-17 22:26 ` [PATCH v8 1/7] of_graph: add of_graph_presents() Dmitry Osipenko
@ 2020-06-17 22:26 ` Dmitry Osipenko
  2020-06-17 22:26 ` [PATCH v8 3/7] drm/tegra: output: Don't leak OF node on error Dmitry Osipenko
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Dmitry Osipenko @ 2020-06-17 22:26 UTC (permalink / raw)
  To: Thierry Reding, Sam Ravnborg, Laurent Pinchart, Rob Herring,
	Frank Rowand
  Cc: linux-tegra, devicetree, linux-kernel, dri-devel

When graph isn't defined in a device-tree, the of_graph_get_remote_node()
prints a noisy error message, telling that port node is not found. This is
undesirable behaviour in our case because absence of a panel/bridge graph
is a valid case. Let's check the graph's presence in a device-tree before
proceeding with parsing of the graph.

Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/gpu/drm/drm_of.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index b50b44e76279..cbe65efdae39 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -246,6 +246,15 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
 	if (panel)
 		*panel = NULL;
 
+	/*
+	 * of_graph_get_remote_node() produces a noisy error message if port
+	 * node isn't found and the absence of the port is a legit case here,
+	 * so at first we silently check whether graph presents in the
+	 * device-tree node.
+	 */
+	if (!of_graph_presents(np))
+		return -ENODEV;
+
 	remote = of_graph_get_remote_node(np, port, endpoint);
 	if (!remote)
 		return -ENODEV;
-- 
2.26.0

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v8 3/7] drm/tegra: output: Don't leak OF node on error
  2020-06-17 22:26 [PATCH v8 0/7] Support DRM bridges on NVIDIA Tegra Dmitry Osipenko
  2020-06-17 22:26 ` [PATCH v8 1/7] of_graph: add of_graph_presents() Dmitry Osipenko
  2020-06-17 22:26 ` [PATCH v8 2/7] drm/of: Make drm_of_find_panel_or_bridge() to check graph's presence Dmitry Osipenko
@ 2020-06-17 22:26 ` Dmitry Osipenko
  2020-06-17 22:27 ` [PATCH v8 4/7] drm/tegra: output: Support DRM bridges Dmitry Osipenko
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Dmitry Osipenko @ 2020-06-17 22:26 UTC (permalink / raw)
  To: Thierry Reding, Sam Ravnborg, Laurent Pinchart, Rob Herring,
	Frank Rowand
  Cc: linux-tegra, devicetree, linux-kernel, dri-devel

The OF node should be put before returning error in tegra_output_probe(),
otherwise node's refcount will be leaked.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/gpu/drm/tegra/output.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
index e36e5e7c2f69..a6a711d54e88 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -102,10 +102,10 @@ int tegra_output_probe(struct tegra_output *output)
 	panel = of_parse_phandle(output->of_node, "nvidia,panel", 0);
 	if (panel) {
 		output->panel = of_drm_find_panel(panel);
+		of_node_put(panel);
+
 		if (IS_ERR(output->panel))
 			return PTR_ERR(output->panel);
-
-		of_node_put(panel);
 	}
 
 	output->edid = of_get_property(output->of_node, "nvidia,edid", &size);
@@ -113,13 +113,12 @@ int tegra_output_probe(struct tegra_output *output)
 	ddc = of_parse_phandle(output->of_node, "nvidia,ddc-i2c-bus", 0);
 	if (ddc) {
 		output->ddc = of_find_i2c_adapter_by_node(ddc);
+		of_node_put(ddc);
+
 		if (!output->ddc) {
 			err = -EPROBE_DEFER;
-			of_node_put(ddc);
 			return err;
 		}
-
-		of_node_put(ddc);
 	}
 
 	output->hpd_gpio = devm_gpiod_get_from_of_node(output->dev,
-- 
2.26.0

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v8 4/7] drm/tegra: output: Support DRM bridges
  2020-06-17 22:26 [PATCH v8 0/7] Support DRM bridges on NVIDIA Tegra Dmitry Osipenko
                   ` (2 preceding siblings ...)
  2020-06-17 22:26 ` [PATCH v8 3/7] drm/tegra: output: Don't leak OF node on error Dmitry Osipenko
@ 2020-06-17 22:27 ` Dmitry Osipenko
  2020-06-17 22:27 ` [PATCH v8 5/7] drm/tegra: output: rgb: Support LVDS encoder bridge Dmitry Osipenko
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Dmitry Osipenko @ 2020-06-17 22:27 UTC (permalink / raw)
  To: Thierry Reding, Sam Ravnborg, Laurent Pinchart, Rob Herring,
	Frank Rowand
  Cc: linux-tegra, devicetree, linux-kernel, dri-devel

Newer Tegra device-trees will specify a video output graph which involves
a bridge. This patch adds initial support for the DRM bridges to the Tegra
DRM output.

Acked-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/gpu/drm/tegra/drm.h    |  2 ++
 drivers/gpu/drm/tegra/output.c | 12 ++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index b25443255be6..f38de08e0c95 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -12,6 +12,7 @@
 #include <linux/gpio/consumer.h>
 
 #include <drm/drm_atomic.h>
+#include <drm/drm_bridge.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_encoder.h>
 #include <drm/drm_fb_helper.h>
@@ -116,6 +117,7 @@ struct tegra_output {
 	struct device_node *of_node;
 	struct device *dev;
 
+	struct drm_bridge *bridge;
 	struct drm_panel *panel;
 	struct i2c_adapter *ddc;
 	const struct edid *edid;
diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
index a6a711d54e88..ccd1421f1b24 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -5,6 +5,7 @@
  */
 
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_of.h>
 #include <drm/drm_panel.h>
 #include <drm/drm_simple_kms_helper.h>
 
@@ -99,8 +100,19 @@ int tegra_output_probe(struct tegra_output *output)
 	if (!output->of_node)
 		output->of_node = output->dev->of_node;
 
+	err = drm_of_find_panel_or_bridge(output->of_node, -1, -1,
+					  &output->panel, &output->bridge);
+	if (err && err != -ENODEV)
+		return err;
+
 	panel = of_parse_phandle(output->of_node, "nvidia,panel", 0);
 	if (panel) {
+		/*
+		 * Don't mix nvidia,panel phandle with the graph in a
+		 * device-tree.
+		 */
+		WARN_ON(output->panel || output->bridge);
+
 		output->panel = of_drm_find_panel(panel);
 		of_node_put(panel);
 
-- 
2.26.0

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v8 5/7] drm/tegra: output: rgb: Support LVDS encoder bridge
  2020-06-17 22:26 [PATCH v8 0/7] Support DRM bridges on NVIDIA Tegra Dmitry Osipenko
                   ` (3 preceding siblings ...)
  2020-06-17 22:27 ` [PATCH v8 4/7] drm/tegra: output: Support DRM bridges Dmitry Osipenko
@ 2020-06-17 22:27 ` Dmitry Osipenko
  2020-06-17 22:27 ` [PATCH v8 6/7] drm/tegra: output: rgb: Wrap directly-connected panel into DRM bridge Dmitry Osipenko
  2020-06-17 22:27 ` [PATCH v8 7/7] drm/panel-simple: Add missing connector type for some panels Dmitry Osipenko
  6 siblings, 0 replies; 17+ messages in thread
From: Dmitry Osipenko @ 2020-06-17 22:27 UTC (permalink / raw)
  To: Thierry Reding, Sam Ravnborg, Laurent Pinchart, Rob Herring,
	Frank Rowand
  Cc: linux-tegra, devicetree, linux-kernel, dri-devel

Newer Tegra device-trees will specify a video output graph, which involves
LVDS encoder bridge. This patch adds support for the LVDS encoder bridge
to the RGB output, allowing us to model the display hardware properly.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/gpu/drm/tegra/rgb.c | 58 +++++++++++++++++++++++++++++++------
 1 file changed, 49 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/tegra/rgb.c b/drivers/gpu/drm/tegra/rgb.c
index 0562a7eb793f..9a7024ec96bc 100644
--- a/drivers/gpu/drm/tegra/rgb.c
+++ b/drivers/gpu/drm/tegra/rgb.c
@@ -7,6 +7,7 @@
 #include <linux/clk.h>
 
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge_connector.h>
 #include <drm/drm_panel.h>
 #include <drm/drm_simple_kms_helper.h>
 
@@ -267,24 +268,63 @@ int tegra_dc_rgb_remove(struct tegra_dc *dc)
 int tegra_dc_rgb_init(struct drm_device *drm, struct tegra_dc *dc)
 {
 	struct tegra_output *output = dc->rgb;
+	struct drm_connector *connector;
 	int err;
 
 	if (!dc->rgb)
 		return -ENODEV;
 
-	drm_connector_init(drm, &output->connector, &tegra_rgb_connector_funcs,
-			   DRM_MODE_CONNECTOR_LVDS);
-	drm_connector_helper_add(&output->connector,
-				 &tegra_rgb_connector_helper_funcs);
-	output->connector.dpms = DRM_MODE_DPMS_OFF;
-
 	drm_simple_encoder_init(drm, &output->encoder, DRM_MODE_ENCODER_LVDS);
 	drm_encoder_helper_add(&output->encoder,
 			       &tegra_rgb_encoder_helper_funcs);
 
-	drm_connector_attach_encoder(&output->connector,
-					  &output->encoder);
-	drm_connector_register(&output->connector);
+	/*
+	 * Tegra devices that have LVDS panel utilize LVDS encoder bridge
+	 * for converting up to 28 LCD LVTTL lanes into 5/4 LVDS lanes that
+	 * go to display panel's receiver.
+	 *
+	 * Encoder usually have a power-down control which needs to be enabled
+	 * in order to transmit data to the panel.  Historically devices that
+	 * use an older device-tree version didn't model the bridge, assuming
+	 * that encoder is turned ON by default, while today's DRM allows us
+	 * to model LVDS encoder properly.
+	 *
+	 * Newer device-trees utilize LVDS encoder bridge, which provides
+	 * us with a connector and handles the display panel.
+	 *
+	 * For older device-trees we fall back to our own connector and use
+	 * nvidia,panel phandle.
+	 */
+	if (output->bridge) {
+		err = drm_bridge_attach(&output->encoder, output->bridge,
+					NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+		if (err) {
+			dev_err(output->dev, "failed to attach bridge: %d\n",
+				err);
+			return err;
+		}
+
+		connector = drm_bridge_connector_init(drm, &output->encoder);
+		if (IS_ERR(connector)) {
+			dev_err(output->dev,
+				"failed to initialize bridge connector: %pe\n",
+				connector);
+			return PTR_ERR(connector);
+		}
+
+		drm_connector_attach_encoder(connector, &output->encoder);
+	} else {
+		drm_connector_init(drm, &output->connector,
+				   &tegra_rgb_connector_funcs,
+				   DRM_MODE_CONNECTOR_LVDS);
+		drm_connector_helper_add(&output->connector,
+					 &tegra_rgb_connector_helper_funcs);
+		output->connector.dpms = DRM_MODE_DPMS_OFF;
+
+		drm_connector_attach_encoder(&output->connector,
+					     &output->encoder);
+		drm_connector_register(&output->connector);
+	}
 
 	err = tegra_output_init(drm, output);
 	if (err < 0) {
-- 
2.26.0

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v8 6/7] drm/tegra: output: rgb: Wrap directly-connected panel into DRM bridge
  2020-06-17 22:26 [PATCH v8 0/7] Support DRM bridges on NVIDIA Tegra Dmitry Osipenko
                   ` (4 preceding siblings ...)
  2020-06-17 22:27 ` [PATCH v8 5/7] drm/tegra: output: rgb: Support LVDS encoder bridge Dmitry Osipenko
@ 2020-06-17 22:27 ` Dmitry Osipenko
  2020-06-17 22:27 ` [PATCH v8 7/7] drm/panel-simple: Add missing connector type for some panels Dmitry Osipenko
  6 siblings, 0 replies; 17+ messages in thread
From: Dmitry Osipenko @ 2020-06-17 22:27 UTC (permalink / raw)
  To: Thierry Reding, Sam Ravnborg, Laurent Pinchart, Rob Herring,
	Frank Rowand
  Cc: linux-tegra, devicetree, linux-kernel, dri-devel

Currently Tegra DRM driver manually manages display panel, but this
management could be moved out into DRM core if we'll wrap panel into
DRM bridge. This patch wraps RGB panel into a DRM bridge and removes
manual handling of the panel from the RGB output code.

Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/gpu/drm/tegra/rgb.c | 70 ++++++++++---------------------------
 1 file changed, 18 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/tegra/rgb.c b/drivers/gpu/drm/tegra/rgb.c
index 9a7024ec96bc..4142a56ca764 100644
--- a/drivers/gpu/drm/tegra/rgb.c
+++ b/drivers/gpu/drm/tegra/rgb.c
@@ -8,7 +8,6 @@
 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge_connector.h>
-#include <drm/drm_panel.h>
 #include <drm/drm_simple_kms_helper.h>
 
 #include "drm.h"
@@ -86,45 +85,13 @@ static void tegra_dc_write_regs(struct tegra_dc *dc,
 		tegra_dc_writel(dc, table[i].value, table[i].offset);
 }
 
-static const struct drm_connector_funcs tegra_rgb_connector_funcs = {
-	.reset = drm_atomic_helper_connector_reset,
-	.detect = tegra_output_connector_detect,
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = tegra_output_connector_destroy,
-	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
-	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-
-static enum drm_mode_status
-tegra_rgb_connector_mode_valid(struct drm_connector *connector,
-			       struct drm_display_mode *mode)
-{
-	/*
-	 * FIXME: For now, always assume that the mode is okay. There are
-	 * unresolved issues with clk_round_rate(), which doesn't always
-	 * reliably report whether a frequency can be set or not.
-	 */
-	return MODE_OK;
-}
-
-static const struct drm_connector_helper_funcs tegra_rgb_connector_helper_funcs = {
-	.get_modes = tegra_output_connector_get_modes,
-	.mode_valid = tegra_rgb_connector_mode_valid,
-};
-
 static void tegra_rgb_encoder_disable(struct drm_encoder *encoder)
 {
 	struct tegra_output *output = encoder_to_output(encoder);
 	struct tegra_rgb *rgb = to_rgb(output);
 
-	if (output->panel)
-		drm_panel_disable(output->panel);
-
 	tegra_dc_write_regs(rgb->dc, rgb_disable, ARRAY_SIZE(rgb_disable));
 	tegra_dc_commit(rgb->dc);
-
-	if (output->panel)
-		drm_panel_unprepare(output->panel);
 }
 
 static void tegra_rgb_encoder_enable(struct drm_encoder *encoder)
@@ -133,9 +100,6 @@ static void tegra_rgb_encoder_enable(struct drm_encoder *encoder)
 	struct tegra_rgb *rgb = to_rgb(output);
 	u32 value;
 
-	if (output->panel)
-		drm_panel_prepare(output->panel);
-
 	tegra_dc_write_regs(rgb->dc, rgb_enable, ARRAY_SIZE(rgb_enable));
 
 	value = DE_SELECT_ACTIVE | DE_CONTROL_NORMAL;
@@ -157,9 +121,6 @@ static void tegra_rgb_encoder_enable(struct drm_encoder *encoder)
 	tegra_dc_writel(rgb->dc, value, DC_DISP_SHIFT_CLOCK_OPTIONS);
 
 	tegra_dc_commit(rgb->dc);
-
-	if (output->panel)
-		drm_panel_enable(output->panel);
 }
 
 static int
@@ -278,6 +239,23 @@ int tegra_dc_rgb_init(struct drm_device *drm, struct tegra_dc *dc)
 	drm_encoder_helper_add(&output->encoder,
 			       &tegra_rgb_encoder_helper_funcs);
 
+	/*
+	 * Wrap directly-connected panel into DRM bridge in order to let
+	 * DRM core to handle panel for us.
+	 */
+	if (output->panel) {
+		output->bridge = devm_drm_panel_bridge_add(output->dev,
+							   output->panel);
+		if (IS_ERR(output->bridge)) {
+			dev_err(output->dev,
+				"failed to wrap panel into bridge: %pe\n",
+				output->bridge);
+			return PTR_ERR(output->bridge);
+		}
+
+		output->panel = NULL;
+	}
+
 	/*
 	 * Tegra devices that have LVDS panel utilize LVDS encoder bridge
 	 * for converting up to 28 LCD LVTTL lanes into 5/4 LVDS lanes that
@@ -292,8 +270,7 @@ int tegra_dc_rgb_init(struct drm_device *drm, struct tegra_dc *dc)
 	 * Newer device-trees utilize LVDS encoder bridge, which provides
 	 * us with a connector and handles the display panel.
 	 *
-	 * For older device-trees we fall back to our own connector and use
-	 * nvidia,panel phandle.
+	 * For older device-trees we wrapped panel into the panel-bridge.
 	 */
 	if (output->bridge) {
 		err = drm_bridge_attach(&output->encoder, output->bridge,
@@ -313,17 +290,6 @@ int tegra_dc_rgb_init(struct drm_device *drm, struct tegra_dc *dc)
 		}
 
 		drm_connector_attach_encoder(connector, &output->encoder);
-	} else {
-		drm_connector_init(drm, &output->connector,
-				   &tegra_rgb_connector_funcs,
-				   DRM_MODE_CONNECTOR_LVDS);
-		drm_connector_helper_add(&output->connector,
-					 &tegra_rgb_connector_helper_funcs);
-		output->connector.dpms = DRM_MODE_DPMS_OFF;
-
-		drm_connector_attach_encoder(&output->connector,
-					     &output->encoder);
-		drm_connector_register(&output->connector);
 	}
 
 	err = tegra_output_init(drm, output);
-- 
2.26.0

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v8 7/7] drm/panel-simple: Add missing connector type for some panels
  2020-06-17 22:26 [PATCH v8 0/7] Support DRM bridges on NVIDIA Tegra Dmitry Osipenko
                   ` (5 preceding siblings ...)
  2020-06-17 22:27 ` [PATCH v8 6/7] drm/tegra: output: rgb: Wrap directly-connected panel into DRM bridge Dmitry Osipenko
@ 2020-06-17 22:27 ` Dmitry Osipenko
  2020-06-20 11:21   ` Sam Ravnborg
  6 siblings, 1 reply; 17+ messages in thread
From: Dmitry Osipenko @ 2020-06-17 22:27 UTC (permalink / raw)
  To: Thierry Reding, Sam Ravnborg, Laurent Pinchart, Rob Herring,
	Frank Rowand
  Cc: linux-tegra, devicetree, linux-kernel, dri-devel

The DRM panel bridge core requires connector type to be set up properly,
otherwise it rejects the panel. The missing connector type problem popped
up while I was trying to wrap CLAA070WP03XG panel into a DRM bridge in
order to test whether panel's rotation property work properly using
panel-simple driver on NVIDIA Tegra30 Nexus 7 tablet device, which uses
CLAA070WP03XG display panel.

The NVIDIA Tegra DRM driver recently gained DRM bridges support for the
RGB output and now driver wraps directly-connected panels into DRM bridge.
Hence all panels should have connector type set properly now, otherwise
the panel's wrapping fails.

This patch adds missing connector types for the LVDS panels that are found
on NVIDIA Tegra devices:

  1. AUO B101AW03
  2. Chunghwa CLAA070WP03XG
  3. Chunghwa CLAA101WA01A
  4. Chunghwa CLAA101WB01
  5. EDT ET057090DHU
  6. Innolux N156BGE L21
  7. Samsung LTN101NT05

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/gpu/drm/panel/panel-simple.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 6764ac630e22..9eb2dbb7bfa6 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -687,6 +687,7 @@ static const struct panel_desc auo_b101aw03 = {
 		.width = 223,
 		.height = 125,
 	},
+	.connector_type = DRM_MODE_CONNECTOR_LVDS,
 };
 
 static const struct display_timing auo_b101ean01_timing = {
@@ -1340,6 +1341,7 @@ static const struct panel_desc chunghwa_claa070wp03xg = {
 		.width = 94,
 		.height = 150,
 	},
+	.connector_type = DRM_MODE_CONNECTOR_LVDS,
 };
 
 static const struct drm_display_mode chunghwa_claa101wa01a_mode = {
@@ -1362,6 +1364,7 @@ static const struct panel_desc chunghwa_claa101wa01a = {
 		.width = 220,
 		.height = 120,
 	},
+	.connector_type = DRM_MODE_CONNECTOR_LVDS,
 };
 
 static const struct drm_display_mode chunghwa_claa101wb01_mode = {
@@ -1384,6 +1387,7 @@ static const struct panel_desc chunghwa_claa101wb01 = {
 		.width = 223,
 		.height = 125,
 	},
+	.connector_type = DRM_MODE_CONNECTOR_LVDS,
 };
 
 static const struct drm_display_mode dataimage_scf0700c48ggu18_mode = {
@@ -1573,6 +1577,7 @@ static const struct panel_desc edt_et057090dhu = {
 	},
 	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
 	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
+	.connector_type = DRM_MODE_CONNECTOR_LVDS,
 };
 
 static const struct drm_display_mode edt_etm0700g0dh6_mode = {
@@ -2055,6 +2060,7 @@ static const struct panel_desc innolux_n156bge_l21 = {
 		.width = 344,
 		.height = 193,
 	},
+	.connector_type = DRM_MODE_CONNECTOR_LVDS,
 };
 
 static const struct drm_display_mode innolux_p120zdg_bf1_mode = {
@@ -3001,6 +3007,7 @@ static const struct panel_desc samsung_ltn101nt05 = {
 		.width = 223,
 		.height = 125,
 	},
+	.connector_type = DRM_MODE_CONNECTOR_LVDS,
 };
 
 static const struct drm_display_mode samsung_ltn140at29_301_mode = {
-- 
2.26.0

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 7/7] drm/panel-simple: Add missing connector type for some panels
  2020-06-17 22:27 ` [PATCH v8 7/7] drm/panel-simple: Add missing connector type for some panels Dmitry Osipenko
@ 2020-06-20 11:21   ` Sam Ravnborg
  2020-06-20 11:49     ` Laurent Pinchart
  0 siblings, 1 reply; 17+ messages in thread
From: Sam Ravnborg @ 2020-06-20 11:21 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: devicetree, dri-devel, linux-kernel, Rob Herring, Thierry Reding,
	Laurent Pinchart, linux-tegra, Frank Rowand

Hi Dmitry

On Thu, Jun 18, 2020 at 01:27:03AM +0300, Dmitry Osipenko wrote:
> The DRM panel bridge core requires connector type to be set up properly,
> otherwise it rejects the panel. The missing connector type problem popped
> up while I was trying to wrap CLAA070WP03XG panel into a DRM bridge in
> order to test whether panel's rotation property work properly using
> panel-simple driver on NVIDIA Tegra30 Nexus 7 tablet device, which uses
> CLAA070WP03XG display panel.
> 
> The NVIDIA Tegra DRM driver recently gained DRM bridges support for the
> RGB output and now driver wraps directly-connected panels into DRM bridge.
> Hence all panels should have connector type set properly now, otherwise
> the panel's wrapping fails.
> 
> This patch adds missing connector types for the LVDS panels that are found
> on NVIDIA Tegra devices:
> 
>   1. AUO B101AW03
>   2. Chunghwa CLAA070WP03XG
>   3. Chunghwa CLAA101WA01A
>   4. Chunghwa CLAA101WB01
>   5. EDT ET057090DHU
>   6. Innolux N156BGE L21
>   7. Samsung LTN101NT05
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Very good to have this fixed.
I went ahead and pushed this commit to drm-misc-next as it is really
independent from the rest of the series.

	Sam

> ---
>  drivers/gpu/drm/panel/panel-simple.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index 6764ac630e22..9eb2dbb7bfa6 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -687,6 +687,7 @@ static const struct panel_desc auo_b101aw03 = {
>  		.width = 223,
>  		.height = 125,
>  	},
> +	.connector_type = DRM_MODE_CONNECTOR_LVDS,
>  };
>  
>  static const struct display_timing auo_b101ean01_timing = {
> @@ -1340,6 +1341,7 @@ static const struct panel_desc chunghwa_claa070wp03xg = {
>  		.width = 94,
>  		.height = 150,
>  	},
> +	.connector_type = DRM_MODE_CONNECTOR_LVDS,
>  };
>  
>  static const struct drm_display_mode chunghwa_claa101wa01a_mode = {
> @@ -1362,6 +1364,7 @@ static const struct panel_desc chunghwa_claa101wa01a = {
>  		.width = 220,
>  		.height = 120,
>  	},
> +	.connector_type = DRM_MODE_CONNECTOR_LVDS,
>  };
>  
>  static const struct drm_display_mode chunghwa_claa101wb01_mode = {
> @@ -1384,6 +1387,7 @@ static const struct panel_desc chunghwa_claa101wb01 = {
>  		.width = 223,
>  		.height = 125,
>  	},
> +	.connector_type = DRM_MODE_CONNECTOR_LVDS,
>  };
>  
>  static const struct drm_display_mode dataimage_scf0700c48ggu18_mode = {
> @@ -1573,6 +1577,7 @@ static const struct panel_desc edt_et057090dhu = {
>  	},
>  	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
>  	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
> +	.connector_type = DRM_MODE_CONNECTOR_LVDS,
>  };
>  
>  static const struct drm_display_mode edt_etm0700g0dh6_mode = {
> @@ -2055,6 +2060,7 @@ static const struct panel_desc innolux_n156bge_l21 = {
>  		.width = 344,
>  		.height = 193,
>  	},
> +	.connector_type = DRM_MODE_CONNECTOR_LVDS,
>  };
>  
>  static const struct drm_display_mode innolux_p120zdg_bf1_mode = {
> @@ -3001,6 +3007,7 @@ static const struct panel_desc samsung_ltn101nt05 = {
>  		.width = 223,
>  		.height = 125,
>  	},
> +	.connector_type = DRM_MODE_CONNECTOR_LVDS,
>  };
>  
>  static const struct drm_display_mode samsung_ltn140at29_301_mode = {
> -- 
> 2.26.0
> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 7/7] drm/panel-simple: Add missing connector type for some panels
  2020-06-20 11:21   ` Sam Ravnborg
@ 2020-06-20 11:49     ` Laurent Pinchart
  2020-06-20 13:19       ` Dmitry Osipenko
  0 siblings, 1 reply; 17+ messages in thread
From: Laurent Pinchart @ 2020-06-20 11:49 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: devicetree, linux-kernel, Rob Herring, Thierry Reding, dri-devel,
	linux-tegra, Dmitry Osipenko, Frank Rowand

Hi Sam and Dmitry,

On Sat, Jun 20, 2020 at 01:21:32PM +0200, Sam Ravnborg wrote:
> On Thu, Jun 18, 2020 at 01:27:03AM +0300, Dmitry Osipenko wrote:
> > The DRM panel bridge core requires connector type to be set up properly,
> > otherwise it rejects the panel. The missing connector type problem popped
> > up while I was trying to wrap CLAA070WP03XG panel into a DRM bridge in
> > order to test whether panel's rotation property work properly using
> > panel-simple driver on NVIDIA Tegra30 Nexus 7 tablet device, which uses
> > CLAA070WP03XG display panel.
> > 
> > The NVIDIA Tegra DRM driver recently gained DRM bridges support for the
> > RGB output and now driver wraps directly-connected panels into DRM bridge.
> > Hence all panels should have connector type set properly now, otherwise
> > the panel's wrapping fails.
> > 
> > This patch adds missing connector types for the LVDS panels that are found
> > on NVIDIA Tegra devices:
> > 
> >   1. AUO B101AW03
> >   2. Chunghwa CLAA070WP03XG
> >   3. Chunghwa CLAA101WA01A
> >   4. Chunghwa CLAA101WB01
> >   5. EDT ET057090DHU
> >   6. Innolux N156BGE L21
> >   7. Samsung LTN101NT05
> > 
> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> 
> Very good to have this fixed.
> I went ahead and pushed this commit to drm-misc-next as it is really
> independent from the rest of the series.
> 
> > ---
> >  drivers/gpu/drm/panel/panel-simple.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > index 6764ac630e22..9eb2dbb7bfa6 100644
> > --- a/drivers/gpu/drm/panel/panel-simple.c
> > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > @@ -687,6 +687,7 @@ static const struct panel_desc auo_b101aw03 = {
> >  		.width = 223,
> >  		.height = 125,
> >  	},
> > +	.connector_type = DRM_MODE_CONNECTOR_LVDS,

Note that, for LVDS panels, the bus_format field is mandatory. This
panel, for instance, according to
http://www.vslcd.com/Specification/B101AW03%20V.0.pdf, uses
MEDIA_BUS_FMT_RGB666_1X7X3_SPWG (see
https://linuxtv.org/downloads/v4l-dvb-apis/userspace-api/v4l/subdev-formats.html#v4l2-mbus-pixelcode).
The panels below need to be investigated similarly.

> >  };
> >  
> >  static const struct display_timing auo_b101ean01_timing = {
> > @@ -1340,6 +1341,7 @@ static const struct panel_desc chunghwa_claa070wp03xg = {
> >  		.width = 94,
> >  		.height = 150,
> >  	},
> > +	.connector_type = DRM_MODE_CONNECTOR_LVDS,
> >  };
> >  
> >  static const struct drm_display_mode chunghwa_claa101wa01a_mode = {
> > @@ -1362,6 +1364,7 @@ static const struct panel_desc chunghwa_claa101wa01a = {
> >  		.width = 220,
> >  		.height = 120,
> >  	},
> > +	.connector_type = DRM_MODE_CONNECTOR_LVDS,
> >  };
> >  
> >  static const struct drm_display_mode chunghwa_claa101wb01_mode = {
> > @@ -1384,6 +1387,7 @@ static const struct panel_desc chunghwa_claa101wb01 = {
> >  		.width = 223,
> >  		.height = 125,
> >  	},
> > +	.connector_type = DRM_MODE_CONNECTOR_LVDS,
> >  };
> >  
> >  static const struct drm_display_mode dataimage_scf0700c48ggu18_mode = {
> > @@ -1573,6 +1577,7 @@ static const struct panel_desc edt_et057090dhu = {
> >  	},
> >  	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> >  	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
> > +	.connector_type = DRM_MODE_CONNECTOR_LVDS,

This contradicts .bus_format and .bus_flags that hint that the panel is
a DPI panel, not an LVDS panel. According to
https://www.lcdtek.co.uk/dwpdf/ET057090DHU-RoHS.pdf, this isn't an LVDS
panel.

I'm worried enough research hasn't gone into this patch, and I'd prefer
reverting it until we check each panel individually.

> >  };
> >  
> >  static const struct drm_display_mode edt_etm0700g0dh6_mode = {
> > @@ -2055,6 +2060,7 @@ static const struct panel_desc innolux_n156bge_l21 = {
> >  		.width = 344,
> >  		.height = 193,
> >  	},
> > +	.connector_type = DRM_MODE_CONNECTOR_LVDS,
> >  };
> >  
> >  static const struct drm_display_mode innolux_p120zdg_bf1_mode = {
> > @@ -3001,6 +3007,7 @@ static const struct panel_desc samsung_ltn101nt05 = {
> >  		.width = 223,
> >  		.height = 125,
> >  	},
> > +	.connector_type = DRM_MODE_CONNECTOR_LVDS,
> >  };
> >  
> >  static const struct drm_display_mode samsung_ltn140at29_301_mode = {

-- 
Regards,

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 7/7] drm/panel-simple: Add missing connector type for some panels
  2020-06-20 11:49     ` Laurent Pinchart
@ 2020-06-20 13:19       ` Dmitry Osipenko
  2020-06-20 14:31         ` Sam Ravnborg
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Osipenko @ 2020-06-20 13:19 UTC (permalink / raw)
  To: Laurent Pinchart, Sam Ravnborg
  Cc: devicetree, linux-kernel, Rob Herring, Thierry Reding, dri-devel,
	linux-tegra, Frank Rowand

20.06.2020 14:49, Laurent Pinchart пишет:
> Hi Sam and Dmitry,
> 
> On Sat, Jun 20, 2020 at 01:21:32PM +0200, Sam Ravnborg wrote:
>> On Thu, Jun 18, 2020 at 01:27:03AM +0300, Dmitry Osipenko wrote:
>>> The DRM panel bridge core requires connector type to be set up properly,
>>> otherwise it rejects the panel. The missing connector type problem popped
>>> up while I was trying to wrap CLAA070WP03XG panel into a DRM bridge in
>>> order to test whether panel's rotation property work properly using
>>> panel-simple driver on NVIDIA Tegra30 Nexus 7 tablet device, which uses
>>> CLAA070WP03XG display panel.
>>>
>>> The NVIDIA Tegra DRM driver recently gained DRM bridges support for the
>>> RGB output and now driver wraps directly-connected panels into DRM bridge.
>>> Hence all panels should have connector type set properly now, otherwise
>>> the panel's wrapping fails.
>>>
>>> This patch adds missing connector types for the LVDS panels that are found
>>> on NVIDIA Tegra devices:
>>>
>>>   1. AUO B101AW03
>>>   2. Chunghwa CLAA070WP03XG
>>>   3. Chunghwa CLAA101WA01A
>>>   4. Chunghwa CLAA101WB01
>>>   5. EDT ET057090DHU
>>>   6. Innolux N156BGE L21
>>>   7. Samsung LTN101NT05
>>>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>
>> Very good to have this fixed.
>> I went ahead and pushed this commit to drm-misc-next as it is really
>> independent from the rest of the series.
>>
>>> ---
>>>  drivers/gpu/drm/panel/panel-simple.c | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
>>> index 6764ac630e22..9eb2dbb7bfa6 100644
>>> --- a/drivers/gpu/drm/panel/panel-simple.c
>>> +++ b/drivers/gpu/drm/panel/panel-simple.c
>>> @@ -687,6 +687,7 @@ static const struct panel_desc auo_b101aw03 = {
>>>  		.width = 223,
>>>  		.height = 125,
>>>  	},
>>> +	.connector_type = DRM_MODE_CONNECTOR_LVDS,
> 
> Note that, for LVDS panels, the bus_format field is mandatory. This
> panel, for instance, according to
> http://www.vslcd.com/Specification/B101AW03%20V.0.pdf, uses
> MEDIA_BUS_FMT_RGB666_1X7X3_SPWG (see
> https://linuxtv.org/downloads/v4l-dvb-apis/userspace-api/v4l/subdev-formats.html#v4l2-mbus-pixelcode).
> The panels below need to be investigated similarly.

Okay! I'll add the missing field in v9.

>>>  };
>>>  
>>>  static const struct display_timing auo_b101ean01_timing = {
>>> @@ -1340,6 +1341,7 @@ static const struct panel_desc chunghwa_claa070wp03xg = {
>>>  		.width = 94,
>>>  		.height = 150,
>>>  	},
>>> +	.connector_type = DRM_MODE_CONNECTOR_LVDS,
>>>  };
>>>  
>>>  static const struct drm_display_mode chunghwa_claa101wa01a_mode = {
>>> @@ -1362,6 +1364,7 @@ static const struct panel_desc chunghwa_claa101wa01a = {
>>>  		.width = 220,
>>>  		.height = 120,
>>>  	},
>>> +	.connector_type = DRM_MODE_CONNECTOR_LVDS,
>>>  };
>>>  
>>>  static const struct drm_display_mode chunghwa_claa101wb01_mode = {
>>> @@ -1384,6 +1387,7 @@ static const struct panel_desc chunghwa_claa101wb01 = {
>>>  		.width = 223,
>>>  		.height = 125,
>>>  	},
>>> +	.connector_type = DRM_MODE_CONNECTOR_LVDS,
>>>  };
>>>  
>>>  static const struct drm_display_mode dataimage_scf0700c48ggu18_mode = {
>>> @@ -1573,6 +1577,7 @@ static const struct panel_desc edt_et057090dhu = {
>>>  	},
>>>  	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
>>>  	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
>>> +	.connector_type = DRM_MODE_CONNECTOR_LVDS,
> 
> This contradicts .bus_format and .bus_flags that hint that the panel is
> a DPI panel, not an LVDS panel. According to
> https://www.lcdtek.co.uk/dwpdf/ET057090DHU-RoHS.pdf, this isn't an LVDS
> panel.
> 
> I'm worried enough research hasn't gone into this patch, and I'd prefer
> reverting it until we check each panel individually.

Hello Sam and Laurent,

Oops! Good catch! Indeed, I blindly set the LVDS type to all these
panels. Please revert this patch, I'll double check each panel and
prepare an updated version of this patch. Thank you very much for the
review!
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 7/7] drm/panel-simple: Add missing connector type for some panels
  2020-06-20 13:19       ` Dmitry Osipenko
@ 2020-06-20 14:31         ` Sam Ravnborg
  2020-06-20 15:05           ` Dmitry Osipenko
  0 siblings, 1 reply; 17+ messages in thread
From: Sam Ravnborg @ 2020-06-20 14:31 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: devicetree, dri-devel, linux-kernel, Rob Herring, Thierry Reding,
	Laurent Pinchart, linux-tegra, Frank Rowand

Hi Dmitry

> 
> Oops! Good catch!
Yep, thanks Laurent. Should have taken a better look before applying.

> Indeed, I blindly set the LVDS type to all these
> panels. Please revert this patch, I'll double check each panel and
> prepare an updated version of this patch. Thank you very much for the
> review!

If you can prepare a fix within a few days then lets wait for that.
I will do a better review next time.

	Sam

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 7/7] drm/panel-simple: Add missing connector type for some panels
  2020-06-20 14:31         ` Sam Ravnborg
@ 2020-06-20 15:05           ` Dmitry Osipenko
  2020-06-20 15:30             ` Sam Ravnborg
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Osipenko @ 2020-06-20 15:05 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: devicetree, dri-devel, linux-kernel, Rob Herring, Thierry Reding,
	Laurent Pinchart, linux-tegra, Frank Rowand

20.06.2020 17:31, Sam Ravnborg пишет:
> Hi Dmitry
> 
>>
>> Oops! Good catch!
> Yep, thanks Laurent. Should have taken a better look before applying.
> 
>> Indeed, I blindly set the LVDS type to all these
>> panels. Please revert this patch, I'll double check each panel and
>> prepare an updated version of this patch. Thank you very much for the
>> review!
> 
> If you can prepare a fix within a few days then lets wait for that.
> I will do a better review next time.

Hello Sam,

I should be able to make it later today or tomorrow. Could you please
clarify what do you mean by the fix, do you what it to be as an
additional patch on top of the applied one or a new version of the patch?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 7/7] drm/panel-simple: Add missing connector type for some panels
  2020-06-20 15:05           ` Dmitry Osipenko
@ 2020-06-20 15:30             ` Sam Ravnborg
  2020-06-20 16:18               ` Dmitry Osipenko
  0 siblings, 1 reply; 17+ messages in thread
From: Sam Ravnborg @ 2020-06-20 15:30 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: devicetree, dri-devel, linux-kernel, Rob Herring, Thierry Reding,
	Laurent Pinchart, linux-tegra, Frank Rowand

Hi Dmitry
On Sat, Jun 20, 2020 at 06:05:37PM +0300, Dmitry Osipenko wrote:
> 20.06.2020 17:31, Sam Ravnborg пишет:
> > Hi Dmitry
> > 
> >>
> >> Oops! Good catch!
> > Yep, thanks Laurent. Should have taken a better look before applying.
> > 
> >> Indeed, I blindly set the LVDS type to all these
> >> panels. Please revert this patch, I'll double check each panel and
> >> prepare an updated version of this patch. Thank you very much for the
> >> review!
> > 
> > If you can prepare a fix within a few days then lets wait for that.
> > I will do a better review next time.
> 
> Hello Sam,
> 
> I should be able to make it later today or tomorrow. Could you please
> clarify what do you mean by the fix, do you what it to be as an
> additional patch on top of the applied one or a new version of the patch?
An additional patch on top of the one applied.
It shall carry a proper fixes: tag like this:

Fixes: 94f07917ebe8 ("drm/panel-simple: Add missing connector type for some panels")
Cc: Dmitry Osipenko <digetx@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: dri-devel@lists.freedesktop.org

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 7/7] drm/panel-simple: Add missing connector type for some panels
  2020-06-20 15:30             ` Sam Ravnborg
@ 2020-06-20 16:18               ` Dmitry Osipenko
  0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Osipenko @ 2020-06-20 16:18 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: devicetree, dri-devel, linux-kernel, Rob Herring, Thierry Reding,
	Laurent Pinchart, linux-tegra, Frank Rowand

20.06.2020 18:30, Sam Ravnborg пишет:
> Hi Dmitry
> On Sat, Jun 20, 2020 at 06:05:37PM +0300, Dmitry Osipenko wrote:
>> 20.06.2020 17:31, Sam Ravnborg пишет:
>>> Hi Dmitry
>>>
>>>>
>>>> Oops! Good catch!
>>> Yep, thanks Laurent. Should have taken a better look before applying.
>>>
>>>> Indeed, I blindly set the LVDS type to all these
>>>> panels. Please revert this patch, I'll double check each panel and
>>>> prepare an updated version of this patch. Thank you very much for the
>>>> review!
>>>
>>> If you can prepare a fix within a few days then lets wait for that.
>>> I will do a better review next time.
>>
>> Hello Sam,
>>
>> I should be able to make it later today or tomorrow. Could you please
>> clarify what do you mean by the fix, do you what it to be as an
>> additional patch on top of the applied one or a new version of the patch?
> An additional patch on top of the one applied.
> It shall carry a proper fixes: tag like this:
> 
> Fixes: 94f07917ebe8 ("drm/panel-simple: Add missing connector type for some panels")
> Cc: Dmitry Osipenko <digetx@gmail.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: dri-devel@lists.freedesktop.org

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 1/7] of_graph: add of_graph_presents()
  2020-06-17 22:26 ` [PATCH v8 1/7] of_graph: add of_graph_presents() Dmitry Osipenko
@ 2020-06-29 23:12   ` Rob Herring
  2020-06-30 17:02     ` Dmitry Osipenko
  0 siblings, 1 reply; 17+ messages in thread
From: Rob Herring @ 2020-06-29 23:12 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: devicetree, Sam Ravnborg, linux-kernel, dri-devel,
	Thierry Reding, Laurent Pinchart, linux-tegra, Frank Rowand

On Thu, Jun 18, 2020 at 01:26:57AM +0300, Dmitry Osipenko wrote:
> In some case, like a DRM display code for example, it's useful to silently
> check whether port node exists at all in a device-tree before proceeding
> with parsing of the graph.
> 
> This patch adds of_graph_presents() that returns true if given device-tree
> node contains OF graph port.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/of/property.c    | 52 +++++++++++++++++++++++++++++++++-------
>  include/linux/of_graph.h |  6 +++++
>  2 files changed, 49 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 1f2086f4e7ce..b84ed6a7cf50 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -29,6 +29,48 @@
>  
>  #include "of_private.h"
>  
> +/**
> + * of_graph_get_first_local_port() - get first local port node
> + * @node: pointer to a local endpoint device_node
> + *
> + * Return: First local port node associated with local endpoint node linked
> + *	   to @node. Use of_node_put() on it when done.
> + */
> +static struct device_node *
> +of_graph_get_first_local_port(const struct device_node *node)
> +{
> +	struct device_node *ports, *port;
> +
> +	ports = of_get_child_by_name(node, "ports");
> +	if (ports)
> +		node = ports;
> +
> +	port = of_get_child_by_name(node, "port");
> +	of_node_put(ports);
> +
> +	return port;
> +}
> +
> +/**
> + * of_graph_presents() - check graph's presence
> + * @node: pointer to a device_node checked for the graph's presence
> + *
> + * Return: True if @node has a port or ports sub-node, false otherwise.
> + */
> +bool of_graph_presents(const struct device_node *node)

of_graph_is_present

Otherwise,

Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 1/7] of_graph: add of_graph_presents()
  2020-06-29 23:12   ` Rob Herring
@ 2020-06-30 17:02     ` Dmitry Osipenko
  0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Osipenko @ 2020-06-30 17:02 UTC (permalink / raw)
  To: Rob Herring, Thierry Reding, Sam Ravnborg, Laurent Pinchart
  Cc: linux-tegra, devicetree, Frank Rowand, linux-kernel, dri-devel

30.06.2020 02:12, Rob Herring пишет:
> On Thu, Jun 18, 2020 at 01:26:57AM +0300, Dmitry Osipenko wrote:
>> In some case, like a DRM display code for example, it's useful to silently
>> check whether port node exists at all in a device-tree before proceeding
>> with parsing of the graph.
>>
>> This patch adds of_graph_presents() that returns true if given device-tree
>> node contains OF graph port.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/of/property.c    | 52 +++++++++++++++++++++++++++++++++-------
>>  include/linux/of_graph.h |  6 +++++
>>  2 files changed, 49 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/of/property.c b/drivers/of/property.c
>> index 1f2086f4e7ce..b84ed6a7cf50 100644
>> --- a/drivers/of/property.c
>> +++ b/drivers/of/property.c
>> @@ -29,6 +29,48 @@
>>  
>>  #include "of_private.h"
>>  
>> +/**
>> + * of_graph_get_first_local_port() - get first local port node
>> + * @node: pointer to a local endpoint device_node
>> + *
>> + * Return: First local port node associated with local endpoint node linked
>> + *	   to @node. Use of_node_put() on it when done.
>> + */
>> +static struct device_node *
>> +of_graph_get_first_local_port(const struct device_node *node)
>> +{
>> +	struct device_node *ports, *port;
>> +
>> +	ports = of_get_child_by_name(node, "ports");
>> +	if (ports)
>> +		node = ports;
>> +
>> +	port = of_get_child_by_name(node, "port");
>> +	of_node_put(ports);
>> +
>> +	return port;
>> +}
>> +
>> +/**
>> + * of_graph_presents() - check graph's presence
>> + * @node: pointer to a device_node checked for the graph's presence
>> + *
>> + * Return: True if @node has a port or ports sub-node, false otherwise.
>> + */
>> +bool of_graph_presents(const struct device_node *node)
> 
> of_graph_is_present
> 
> Otherwise,
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> 

Thanks!

I'll address the comment and then factor out this and the
drm_of_find_panel_or_bridge() changes into a separate series in v9,
since the Tegra DRM patches could be applied separately by Thierry.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2020-07-01  7:12 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-17 22:26 [PATCH v8 0/7] Support DRM bridges on NVIDIA Tegra Dmitry Osipenko
2020-06-17 22:26 ` [PATCH v8 1/7] of_graph: add of_graph_presents() Dmitry Osipenko
2020-06-29 23:12   ` Rob Herring
2020-06-30 17:02     ` Dmitry Osipenko
2020-06-17 22:26 ` [PATCH v8 2/7] drm/of: Make drm_of_find_panel_or_bridge() to check graph's presence Dmitry Osipenko
2020-06-17 22:26 ` [PATCH v8 3/7] drm/tegra: output: Don't leak OF node on error Dmitry Osipenko
2020-06-17 22:27 ` [PATCH v8 4/7] drm/tegra: output: Support DRM bridges Dmitry Osipenko
2020-06-17 22:27 ` [PATCH v8 5/7] drm/tegra: output: rgb: Support LVDS encoder bridge Dmitry Osipenko
2020-06-17 22:27 ` [PATCH v8 6/7] drm/tegra: output: rgb: Wrap directly-connected panel into DRM bridge Dmitry Osipenko
2020-06-17 22:27 ` [PATCH v8 7/7] drm/panel-simple: Add missing connector type for some panels Dmitry Osipenko
2020-06-20 11:21   ` Sam Ravnborg
2020-06-20 11:49     ` Laurent Pinchart
2020-06-20 13:19       ` Dmitry Osipenko
2020-06-20 14:31         ` Sam Ravnborg
2020-06-20 15:05           ` Dmitry Osipenko
2020-06-20 15:30             ` Sam Ravnborg
2020-06-20 16:18               ` Dmitry Osipenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).