All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/5] drm/bridge: Update tc358767 and nxp-ptn3460 to support chained bridges
@ 2020-07-27 17:03 Sam Ravnborg
  2020-07-27 17:03 ` [PATCH v5 1/5] drm/bridge: tc358767: add detect bridge operation Sam Ravnborg
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Sam Ravnborg @ 2020-07-27 17:03 UTC (permalink / raw)
  To: dri-devel, Laurent Pinchart
  Cc: Andrzej Hajda, Jernej Skrabec, Neil Armstrong, Sam Ravnborg,
	Jonas Karlman

This patch-set aims to make connector creation optional
and prepare the bridge drivers for use in a chained setup.

The objective is that all bridge drivers shall support a chained setup
connector creation is moved to the display drivers.
This is just one step on this path.

The general approach for the bridge drivers:
- Introduce bridge operations
- Introduce use of panel bridge and make connector creation optional

v5:
  - Applied reviewed patches, so we went from 15 to 5
  - Fixed bug in connector creation in both drivers

v4:
  - Dropped patch for ti-sn65dsi86. Await full conversion
  - Dropped patch for ti-tpd12s015. It was wrong (Laurent)
  - Drop boe,hv070wsa-100 patch, it was applied
  - Combined a few patches to fix connector created twice (Laurent)
  - Fix memory leak in get_edid (Laurent)
  - Added patch to validate panel descriptions in panel-simple
  - Set bridge.type in relevant drivers
 
v3:
  - Rebase on top of drm-misc-next
  - Address kbuild test robot feedback
 
v2:
  - Updated bus_flags for boe,hv070wsa-100
  - Collected r-b, but did not apply patches yet
  - On the panel side the panel-simple driver gained a default
    connector type for all the dumb panels that do not
    include so in their description.
    With this change panels always provide a connector type,
    and we have the potential to drop most uses of
    devm_drm_panel_bridge_add_typed().
  - Added conversion of a few more bridge drivers

Patches can build but no run-time testing.
So both test and review feedback appreciated!

	Sam


Sam Ravnborg (5):
      drm/bridge: tc358767: add detect bridge operation
      drm/bridge: tc358767: add get_edid bridge operation
      drm/bridge: tc358767: add drm_panel_bridge support
      drm/bridge: nxp-ptn3460: add get_edid bridge operation
      drm/bridge: nxp-ptn3460: add drm_panel_bridge support

 drivers/gpu/drm/bridge/nxp-ptn3460.c | 103 +++++++++++++---------------
 drivers/gpu/drm/bridge/tc358767.c    | 126 +++++++++++++++++++----------------
 2 files changed, 114 insertions(+), 115 deletions(-)


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

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

* [PATCH v5 1/5] drm/bridge: tc358767: add detect bridge operation
  2020-07-27 17:03 [PATCH v5 0/5] drm/bridge: Update tc358767 and nxp-ptn3460 to support chained bridges Sam Ravnborg
@ 2020-07-27 17:03 ` Sam Ravnborg
  2020-07-27 17:03 ` [PATCH v5 2/5] drm/bridge: tc358767: add get_edid " Sam Ravnborg
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Sam Ravnborg @ 2020-07-27 17:03 UTC (permalink / raw)
  To: dri-devel, Laurent Pinchart
  Cc: Jernej Skrabec, Neil Armstrong, Jonas Karlman, Andrzej Hajda,
	Laurent Pinchart, Sam Ravnborg

Prepare the bridge driver for chained operation by adding
support for the detect operation.

v3:
  - Fix code to make it readable (Laurent)

v2:
  - Do not announce detect operation if there is no hpd pin (Laurent)

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
---
 drivers/gpu/drm/bridge/tc358767.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index c2777b226c75..bde89e213882 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1369,21 +1369,13 @@ static const struct drm_connector_helper_funcs tc_connector_helper_funcs = {
 	.get_modes = tc_connector_get_modes,
 };
 
-static enum drm_connector_status tc_connector_detect(struct drm_connector *connector,
-						     bool force)
+static enum drm_connector_status tc_bridge_detect(struct drm_bridge *bridge)
 {
-	struct tc_data *tc = connector_to_tc(connector);
+	struct tc_data *tc = bridge_to_tc(bridge);
 	bool conn;
 	u32 val;
 	int ret;
 
-	if (tc->hpd_pin < 0) {
-		if (tc->panel)
-			return connector_status_connected;
-		else
-			return connector_status_unknown;
-	}
-
 	ret = regmap_read(tc->regmap, GPIOI, &val);
 	if (ret)
 		return connector_status_unknown;
@@ -1396,6 +1388,20 @@ static enum drm_connector_status tc_connector_detect(struct drm_connector *conne
 		return connector_status_disconnected;
 }
 
+static enum drm_connector_status
+tc_connector_detect(struct drm_connector *connector, bool force)
+{
+	struct tc_data *tc = connector_to_tc(connector);
+
+	if (tc->hpd_pin >= 0)
+		return tc_bridge_detect(&tc->bridge);
+
+	if (tc->panel)
+		return connector_status_connected;
+	else
+		return connector_status_unknown;
+}
+
 static const struct drm_connector_funcs tc_connector_funcs = {
 	.detect = tc_connector_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
@@ -1458,6 +1464,7 @@ static const struct drm_bridge_funcs tc_bridge_funcs = {
 	.disable = tc_bridge_disable,
 	.post_disable = tc_bridge_post_disable,
 	.mode_fixup = tc_bridge_mode_fixup,
+	.detect = tc_bridge_detect,
 };
 
 static bool tc_readable_reg(struct device *dev, unsigned int reg)
@@ -1680,6 +1687,9 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		return ret;
 
 	tc->bridge.funcs = &tc_bridge_funcs;
+	if (tc->hpd_pin >= 0)
+		tc->bridge.ops |= DRM_BRIDGE_OP_DETECT;
+
 	tc->bridge.of_node = dev->of_node;
 	drm_bridge_add(&tc->bridge);
 
-- 
2.25.1

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

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

* [PATCH v5 2/5] drm/bridge: tc358767: add get_edid bridge operation
  2020-07-27 17:03 [PATCH v5 0/5] drm/bridge: Update tc358767 and nxp-ptn3460 to support chained bridges Sam Ravnborg
  2020-07-27 17:03 ` [PATCH v5 1/5] drm/bridge: tc358767: add detect bridge operation Sam Ravnborg
@ 2020-07-27 17:03 ` Sam Ravnborg
  2020-07-27 17:03 ` [PATCH v5 3/5] drm/bridge: tc358767: add drm_panel_bridge support Sam Ravnborg
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Sam Ravnborg @ 2020-07-27 17:03 UTC (permalink / raw)
  To: dri-devel, Laurent Pinchart
  Cc: Jernej Skrabec, Neil Armstrong, Jonas Karlman, Andrzej Hajda,
	Laurent Pinchart, Sam Ravnborg

Prepare for chained bridge with the addition of
get_edid support.

v2:
  - Fixed handling of edid storage (Laurent)

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
---
 drivers/gpu/drm/bridge/tc358767.c | 34 ++++++++++++++++---------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index bde89e213882..b26c669f3e91 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -250,8 +250,6 @@ struct tc_data {
 	/* link settings */
 	struct tc_edp_link	link;
 
-	/* display edid */
-	struct edid		*edid;
 	/* current mode */
 	struct drm_display_mode	mode;
 
@@ -1335,11 +1333,19 @@ static void tc_bridge_mode_set(struct drm_bridge *bridge,
 	tc->mode = *mode;
 }
 
+static struct edid *tc_get_edid(struct drm_bridge *bridge,
+				struct drm_connector *connector)
+{
+	struct tc_data *tc = bridge_to_tc(bridge);
+
+	return drm_get_edid(connector, &tc->aux.ddc);
+}
+
 static int tc_connector_get_modes(struct drm_connector *connector)
 {
 	struct tc_data *tc = connector_to_tc(connector);
+	int num_modes;
 	struct edid *edid;
-	int count;
 	int ret;
 
 	ret = tc_get_display_props(tc);
@@ -1348,21 +1354,15 @@ static int tc_connector_get_modes(struct drm_connector *connector)
 		return 0;
 	}
 
-	count = drm_panel_get_modes(tc->panel, connector);
-	if (count > 0)
-		return count;
-
-	edid = drm_get_edid(connector, &tc->aux.ddc);
-
-	kfree(tc->edid);
-	tc->edid = edid;
-	if (!edid)
-		return 0;
+	num_modes = drm_panel_get_modes(tc->panel, connector);
+	if (num_modes > 0)
+		return num_modes;
 
-	drm_connector_update_edid_property(connector, edid);
-	count = drm_add_edid_modes(connector, edid);
+	edid = tc_get_edid(&tc->bridge, connector);
+	num_modes = drm_add_edid_modes(connector, edid);
+	kfree(edid);
 
-	return count;
+	return num_modes;
 }
 
 static const struct drm_connector_helper_funcs tc_connector_helper_funcs = {
@@ -1465,6 +1465,7 @@ static const struct drm_bridge_funcs tc_bridge_funcs = {
 	.post_disable = tc_bridge_post_disable,
 	.mode_fixup = tc_bridge_mode_fixup,
 	.detect = tc_bridge_detect,
+	.get_edid = tc_get_edid,
 };
 
 static bool tc_readable_reg(struct device *dev, unsigned int reg)
@@ -1689,6 +1690,7 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	tc->bridge.funcs = &tc_bridge_funcs;
 	if (tc->hpd_pin >= 0)
 		tc->bridge.ops |= DRM_BRIDGE_OP_DETECT;
+	tc->bridge.ops |= DRM_BRIDGE_OP_EDID;
 
 	tc->bridge.of_node = dev->of_node;
 	drm_bridge_add(&tc->bridge);
-- 
2.25.1

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

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

* [PATCH v5 3/5] drm/bridge: tc358767: add drm_panel_bridge support
  2020-07-27 17:03 [PATCH v5 0/5] drm/bridge: Update tc358767 and nxp-ptn3460 to support chained bridges Sam Ravnborg
  2020-07-27 17:03 ` [PATCH v5 1/5] drm/bridge: tc358767: add detect bridge operation Sam Ravnborg
  2020-07-27 17:03 ` [PATCH v5 2/5] drm/bridge: tc358767: add get_edid " Sam Ravnborg
@ 2020-07-27 17:03 ` Sam Ravnborg
  2020-07-27 17:12   ` Laurent Pinchart
  2020-07-27 17:03 ` [PATCH v5 4/5] drm/bridge: nxp-ptn3460: add get_edid bridge operation Sam Ravnborg
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Sam Ravnborg @ 2020-07-27 17:03 UTC (permalink / raw)
  To: dri-devel, Laurent Pinchart
  Cc: Andrzej Hajda, Jernej Skrabec, Neil Armstrong, Sam Ravnborg,
	Jonas Karlman

With the bridge operations implemented the last step to prepare
this driver for a chained setup is the use of the bridge panel driver.

The bridge panel driver is only used when a port@2 is present in
the DT. So when the display driver requests a connector
support both situations:
- connector created by bridge panel driver
- connector created by this driver

And on top, support that the display driver creates the connector,
which is the preferred setup.

Note: the bridge panel will use the connector type from the panel.

v3:
  - Fix wrong logic in connector creation (Laurent)

v2:
  - Merge connector and drm_panel_bridge patches
    and fix so we do not create two connectors (Laurent)

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
---
 drivers/gpu/drm/bridge/tc358767.c | 70 +++++++++++++++----------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index b26c669f3e91..cbad2607ab43 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -244,8 +244,8 @@ struct tc_data {
 	struct drm_dp_aux	aux;
 
 	struct drm_bridge	bridge;
+	struct drm_bridge	*panel_bridge;
 	struct drm_connector	connector;
-	struct drm_panel	*panel;
 
 	/* link settings */
 	struct tc_edp_link	link;
@@ -1234,13 +1234,6 @@ static int tc_stream_disable(struct tc_data *tc)
 	return 0;
 }
 
-static void tc_bridge_pre_enable(struct drm_bridge *bridge)
-{
-	struct tc_data *tc = bridge_to_tc(bridge);
-
-	drm_panel_prepare(tc->panel);
-}
-
 static void tc_bridge_enable(struct drm_bridge *bridge)
 {
 	struct tc_data *tc = bridge_to_tc(bridge);
@@ -1264,8 +1257,6 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
 		tc_main_link_disable(tc);
 		return;
 	}
-
-	drm_panel_enable(tc->panel);
 }
 
 static void tc_bridge_disable(struct drm_bridge *bridge)
@@ -1273,8 +1264,6 @@ static void tc_bridge_disable(struct drm_bridge *bridge)
 	struct tc_data *tc = bridge_to_tc(bridge);
 	int ret;
 
-	drm_panel_disable(tc->panel);
-
 	ret = tc_stream_disable(tc);
 	if (ret < 0)
 		dev_err(tc->dev, "main link stream stop error: %d\n", ret);
@@ -1284,13 +1273,6 @@ static void tc_bridge_disable(struct drm_bridge *bridge)
 		dev_err(tc->dev, "main link disable error: %d\n", ret);
 }
 
-static void tc_bridge_post_disable(struct drm_bridge *bridge)
-{
-	struct tc_data *tc = bridge_to_tc(bridge);
-
-	drm_panel_unprepare(tc->panel);
-}
-
 static bool tc_bridge_mode_fixup(struct drm_bridge *bridge,
 				 const struct drm_display_mode *mode,
 				 struct drm_display_mode *adj)
@@ -1354,9 +1336,11 @@ static int tc_connector_get_modes(struct drm_connector *connector)
 		return 0;
 	}
 
-	num_modes = drm_panel_get_modes(tc->panel, connector);
-	if (num_modes > 0)
-		return num_modes;
+	if (tc->panel_bridge) {
+		num_modes = drm_bridge_get_modes(tc->panel_bridge, connector);
+		if (num_modes > 0)
+			return num_modes;
+	}
 
 	edid = tc_get_edid(&tc->bridge, connector);
 	num_modes = drm_add_edid_modes(connector, edid);
@@ -1396,7 +1380,7 @@ tc_connector_detect(struct drm_connector *connector, bool force)
 	if (tc->hpd_pin >= 0)
 		return tc_bridge_detect(&tc->bridge);
 
-	if (tc->panel)
+	if (tc->panel_bridge)
 		return connector_status_connected;
 	else
 		return connector_status_unknown;
@@ -1419,16 +1403,23 @@ static int tc_bridge_attach(struct drm_bridge *bridge,
 	struct drm_device *drm = bridge->dev;
 	int ret;
 
-	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
-		DRM_ERROR("Fix bridge driver to make connector optional!");
-		return -EINVAL;
+	if (tc->panel_bridge) {
+		enum drm_bridge_attach_flags panel_flags;
+
+		/* If a connector is required then this driver shall create it */
+		panel_flags = flags | DRM_BRIDGE_ATTACH_NO_CONNECTOR;
+		ret = drm_bridge_attach(tc->bridge.encoder, tc->panel_bridge,
+					&tc->bridge, panel_flags);
+		if (ret)
+			return ret;
 	}
 
+	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
+		return 0;
+
 	/* Create DP/eDP connector */
 	drm_connector_helper_add(&tc->connector, &tc_connector_helper_funcs);
-	ret = drm_connector_init(drm, &tc->connector, &tc_connector_funcs,
-				 tc->panel ? DRM_MODE_CONNECTOR_eDP :
-				 DRM_MODE_CONNECTOR_DisplayPort);
+	ret = drm_connector_init(drm, &tc->connector, &tc_connector_funcs, tc->bridge.type);
 	if (ret)
 		return ret;
 
@@ -1441,9 +1432,6 @@ static int tc_bridge_attach(struct drm_bridge *bridge,
 					       DRM_CONNECTOR_POLL_DISCONNECT;
 	}
 
-	if (tc->panel)
-		drm_panel_attach(tc->panel, &tc->connector);
-
 	drm_display_info_set_bus_formats(&tc->connector.display_info,
 					 &bus_format, 1);
 	tc->connector.display_info.bus_flags =
@@ -1459,10 +1447,8 @@ static const struct drm_bridge_funcs tc_bridge_funcs = {
 	.attach = tc_bridge_attach,
 	.mode_valid = tc_mode_valid,
 	.mode_set = tc_bridge_mode_set,
-	.pre_enable = tc_bridge_pre_enable,
 	.enable = tc_bridge_enable,
 	.disable = tc_bridge_disable,
-	.post_disable = tc_bridge_post_disable,
 	.mode_fixup = tc_bridge_mode_fixup,
 	.detect = tc_bridge_detect,
 	.get_edid = tc_get_edid,
@@ -1555,6 +1541,7 @@ static irqreturn_t tc_irq_handler(int irq, void *arg)
 static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id)
 {
 	struct device *dev = &client->dev;
+	struct drm_panel *panel;
 	struct tc_data *tc;
 	int ret;
 
@@ -1565,10 +1552,23 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	tc->dev = dev;
 
 	/* port@2 is the output port */
-	ret = drm_of_find_panel_or_bridge(dev->of_node, 2, 0, &tc->panel, NULL);
+	ret = drm_of_find_panel_or_bridge(dev->of_node, 2, 0, &panel, NULL);
 	if (ret && ret != -ENODEV)
 		return ret;
 
+	if (panel) {
+		struct drm_bridge *panel_bridge;
+
+		panel_bridge = devm_drm_panel_bridge_add(dev, panel);
+		if (IS_ERR(panel_bridge))
+			return PTR_ERR(panel_bridge);
+
+		tc->panel_bridge = panel_bridge;
+		tc->bridge.type = DRM_MODE_CONNECTOR_eDP;
+	} else {
+		tc->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
+	}
+
 	/* Shut down GPIO is optional */
 	tc->sd_gpio = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_HIGH);
 	if (IS_ERR(tc->sd_gpio))
-- 
2.25.1

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

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

* [PATCH v5 4/5] drm/bridge: nxp-ptn3460: add get_edid bridge operation
  2020-07-27 17:03 [PATCH v5 0/5] drm/bridge: Update tc358767 and nxp-ptn3460 to support chained bridges Sam Ravnborg
                   ` (2 preceding siblings ...)
  2020-07-27 17:03 ` [PATCH v5 3/5] drm/bridge: tc358767: add drm_panel_bridge support Sam Ravnborg
@ 2020-07-27 17:03 ` Sam Ravnborg
  2020-07-27 17:03 ` [PATCH v5 5/5] drm/bridge: nxp-ptn3460: add drm_panel_bridge support Sam Ravnborg
  2020-07-27 17:26 ` [PATCH v5 0/5] drm/bridge: Update tc358767 and nxp-ptn3460 to support chained bridges Sam Ravnborg
  5 siblings, 0 replies; 9+ messages in thread
From: Sam Ravnborg @ 2020-07-27 17:03 UTC (permalink / raw)
  To: dri-devel, Laurent Pinchart
  Cc: Jernej Skrabec, Neil Armstrong, Jonas Karlman, Andrzej Hajda,
	Laurent Pinchart, Sam Ravnborg

Add the get_edid() bridge operation to prepare for
use as a chained bridge.
Add helper function that is also used by the connector.

v2:
  - Fix memory leak (Laurent)
  - Do not save a copy of edid, read it when needed

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
---
 drivers/gpu/drm/bridge/nxp-ptn3460.c | 43 ++++++++++++++++------------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/bridge/nxp-ptn3460.c b/drivers/gpu/drm/bridge/nxp-ptn3460.c
index 438e566ce0a4..2805c8938f98 100644
--- a/drivers/gpu/drm/bridge/nxp-ptn3460.c
+++ b/drivers/gpu/drm/bridge/nxp-ptn3460.c
@@ -29,7 +29,6 @@ struct ptn3460_bridge {
 	struct drm_connector connector;
 	struct i2c_client *client;
 	struct drm_bridge bridge;
-	struct edid *edid;
 	struct drm_panel *panel;
 	struct gpio_desc *gpio_pd_n;
 	struct gpio_desc *gpio_rst_n;
@@ -184,17 +183,13 @@ static void ptn3460_post_disable(struct drm_bridge *bridge)
 	}
 }
 
-static int ptn3460_get_modes(struct drm_connector *connector)
+static struct edid *ptn3460_get_edid(struct drm_bridge *bridge,
+				     struct drm_connector *connector)
 {
-	struct ptn3460_bridge *ptn_bridge;
-	u8 *edid;
-	int ret, num_modes = 0;
+	struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge);
 	bool power_off;
-
-	ptn_bridge = connector_to_ptn3460(connector);
-
-	if (ptn_bridge->edid)
-		return drm_add_edid_modes(connector, ptn_bridge->edid);
+	u8 *edid;
+	int ret;
 
 	power_off = !ptn_bridge->enabled;
 	ptn3460_pre_enable(&ptn_bridge->bridge);
@@ -202,30 +197,40 @@ static int ptn3460_get_modes(struct drm_connector *connector)
 	edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
 	if (!edid) {
 		DRM_ERROR("Failed to allocate EDID\n");
-		return 0;
+		goto out;
 	}
 
 	ret = ptn3460_read_bytes(ptn_bridge, PTN3460_EDID_ADDR, edid,
-			EDID_LENGTH);
+				 EDID_LENGTH);
 	if (ret) {
 		kfree(edid);
+		edid = NULL;
 		goto out;
 	}
 
-	ptn_bridge->edid = (struct edid *)edid;
-	drm_connector_update_edid_property(connector, ptn_bridge->edid);
-
-	num_modes = drm_add_edid_modes(connector, ptn_bridge->edid);
-
 out:
 	if (power_off)
 		ptn3460_disable(&ptn_bridge->bridge);
 
+	return (struct edid *)edid;
+}
+
+static int ptn3460_connector_get_modes(struct drm_connector *connector)
+{
+	struct ptn3460_bridge *ptn_bridge = connector_to_ptn3460(connector);
+	struct edid *edid;
+	int num_modes;
+
+	edid = ptn3460_get_edid(&ptn_bridge->bridge, connector);
+	drm_connector_update_edid_property(connector, edid);
+	num_modes = drm_add_edid_modes(connector, edid);
+	kfree(edid);
+
 	return num_modes;
 }
 
 static const struct drm_connector_helper_funcs ptn3460_connector_helper_funcs = {
-	.get_modes = ptn3460_get_modes,
+	.get_modes = ptn3460_connector_get_modes,
 };
 
 static const struct drm_connector_funcs ptn3460_connector_funcs = {
@@ -279,6 +284,7 @@ static const struct drm_bridge_funcs ptn3460_bridge_funcs = {
 	.disable = ptn3460_disable,
 	.post_disable = ptn3460_post_disable,
 	.attach = ptn3460_bridge_attach,
+	.get_edid = ptn3460_get_edid,
 };
 
 static int ptn3460_probe(struct i2c_client *client,
@@ -327,6 +333,7 @@ static int ptn3460_probe(struct i2c_client *client,
 	}
 
 	ptn_bridge->bridge.funcs = &ptn3460_bridge_funcs;
+	ptn_bridge->bridge.ops = DRM_BRIDGE_OP_EDID;
 	ptn_bridge->bridge.of_node = dev->of_node;
 	drm_bridge_add(&ptn_bridge->bridge);
 
-- 
2.25.1

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

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

* [PATCH v5 5/5] drm/bridge: nxp-ptn3460: add drm_panel_bridge support
  2020-07-27 17:03 [PATCH v5 0/5] drm/bridge: Update tc358767 and nxp-ptn3460 to support chained bridges Sam Ravnborg
                   ` (3 preceding siblings ...)
  2020-07-27 17:03 ` [PATCH v5 4/5] drm/bridge: nxp-ptn3460: add get_edid bridge operation Sam Ravnborg
@ 2020-07-27 17:03 ` Sam Ravnborg
  2020-07-27 17:13   ` Laurent Pinchart
  2020-07-27 17:26 ` [PATCH v5 0/5] drm/bridge: Update tc358767 and nxp-ptn3460 to support chained bridges Sam Ravnborg
  5 siblings, 1 reply; 9+ messages in thread
From: Sam Ravnborg @ 2020-07-27 17:03 UTC (permalink / raw)
  To: dri-devel, Laurent Pinchart
  Cc: Jernej Skrabec, Neil Armstrong, Jonas Karlman, Andrzej Hajda,
	Laurent Pinchart, Sam Ravnborg

Prepare the bridge driver for use in a chained setup.

- Replacing direct use of drm_panel with drm_panel_bridge support.
- Make the connector creation optional

Note: the bridge panel will use the connector type from the panel.

v3:
  - Fix wrong logic in connector creation (Laurent)

v2:
  - Use panel_bridge for local variable name to align with other drivers
  - Fix that connector was created twice (Laurent)
  - Set bridge.type to DRM_MODE_CONNECTOR_LVDS.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
---
 drivers/gpu/drm/bridge/nxp-ptn3460.c | 60 ++++++++++------------------
 1 file changed, 20 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/bridge/nxp-ptn3460.c b/drivers/gpu/drm/bridge/nxp-ptn3460.c
index 2805c8938f98..391c1f66f60f 100644
--- a/drivers/gpu/drm/bridge/nxp-ptn3460.c
+++ b/drivers/gpu/drm/bridge/nxp-ptn3460.c
@@ -29,7 +29,7 @@ struct ptn3460_bridge {
 	struct drm_connector connector;
 	struct i2c_client *client;
 	struct drm_bridge bridge;
-	struct drm_panel *panel;
+	struct drm_bridge *panel_bridge;
 	struct gpio_desc *gpio_pd_n;
 	struct gpio_desc *gpio_rst_n;
 	u32 edid_emulation;
@@ -126,11 +126,6 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)
 	usleep_range(10, 20);
 	gpiod_set_value(ptn_bridge->gpio_rst_n, 1);
 
-	if (drm_panel_prepare(ptn_bridge->panel)) {
-		DRM_ERROR("failed to prepare panel\n");
-		return;
-	}
-
 	/*
 	 * There's a bug in the PTN chip where it falsely asserts hotplug before
 	 * it is fully functional. We're forced to wait for the maximum start up
@@ -145,16 +140,6 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)
 	ptn_bridge->enabled = true;
 }
 
-static void ptn3460_enable(struct drm_bridge *bridge)
-{
-	struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge);
-
-	if (drm_panel_enable(ptn_bridge->panel)) {
-		DRM_ERROR("failed to enable panel\n");
-		return;
-	}
-}
-
 static void ptn3460_disable(struct drm_bridge *bridge)
 {
 	struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge);
@@ -164,24 +149,10 @@ static void ptn3460_disable(struct drm_bridge *bridge)
 
 	ptn_bridge->enabled = false;
 
-	if (drm_panel_disable(ptn_bridge->panel)) {
-		DRM_ERROR("failed to disable panel\n");
-		return;
-	}
-
 	gpiod_set_value(ptn_bridge->gpio_rst_n, 1);
 	gpiod_set_value(ptn_bridge->gpio_pd_n, 0);
 }
 
-static void ptn3460_post_disable(struct drm_bridge *bridge)
-{
-	struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge);
-
-	if (drm_panel_unprepare(ptn_bridge->panel)) {
-		DRM_ERROR("failed to unprepare panel\n");
-		return;
-	}
-}
 
 static struct edid *ptn3460_get_edid(struct drm_bridge *bridge,
 				     struct drm_connector *connector)
@@ -245,12 +216,18 @@ static int ptn3460_bridge_attach(struct drm_bridge *bridge,
 				 enum drm_bridge_attach_flags flags)
 {
 	struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge);
+	enum drm_bridge_attach_flags panel_flags;
 	int ret;
 
-	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
-		DRM_ERROR("Fix bridge driver to make connector optional!");
-		return -EINVAL;
-	}
+	/* Let this driver create connector if requested */
+	panel_flags = flags | DRM_BRIDGE_ATTACH_NO_CONNECTOR;
+	ret = drm_bridge_attach(bridge->encoder, ptn_bridge->panel_bridge,
+				bridge, panel_flags);
+	if (ret < 0)
+		return ret;
+
+	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
+		return 0;
 
 	if (!bridge->encoder) {
 		DRM_ERROR("Parent encoder object not found");
@@ -270,9 +247,6 @@ static int ptn3460_bridge_attach(struct drm_bridge *bridge,
 	drm_connector_attach_encoder(&ptn_bridge->connector,
 							bridge->encoder);
 
-	if (ptn_bridge->panel)
-		drm_panel_attach(ptn_bridge->panel, &ptn_bridge->connector);
-
 	drm_helper_hpd_irq_event(ptn_bridge->connector.dev);
 
 	return ret;
@@ -280,9 +254,7 @@ static int ptn3460_bridge_attach(struct drm_bridge *bridge,
 
 static const struct drm_bridge_funcs ptn3460_bridge_funcs = {
 	.pre_enable = ptn3460_pre_enable,
-	.enable = ptn3460_enable,
 	.disable = ptn3460_disable,
-	.post_disable = ptn3460_post_disable,
 	.attach = ptn3460_bridge_attach,
 	.get_edid = ptn3460_get_edid,
 };
@@ -292,6 +264,8 @@ static int ptn3460_probe(struct i2c_client *client,
 {
 	struct device *dev = &client->dev;
 	struct ptn3460_bridge *ptn_bridge;
+	struct drm_bridge *panel_bridge;
+	struct drm_panel *panel;
 	int ret;
 
 	ptn_bridge = devm_kzalloc(dev, sizeof(*ptn_bridge), GFP_KERNEL);
@@ -299,10 +273,15 @@ static int ptn3460_probe(struct i2c_client *client,
 		return -ENOMEM;
 	}
 
-	ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0, &ptn_bridge->panel, NULL);
+	ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0, &panel, NULL);
 	if (ret)
 		return ret;
 
+	panel_bridge = devm_drm_panel_bridge_add(dev, panel);
+	if (IS_ERR(panel_bridge))
+		return PTR_ERR(panel_bridge);
+
+	ptn_bridge->panel_bridge = panel_bridge;
 	ptn_bridge->client = client;
 
 	ptn_bridge->gpio_pd_n = devm_gpiod_get(&client->dev, "powerdown",
@@ -334,6 +313,7 @@ static int ptn3460_probe(struct i2c_client *client,
 
 	ptn_bridge->bridge.funcs = &ptn3460_bridge_funcs;
 	ptn_bridge->bridge.ops = DRM_BRIDGE_OP_EDID;
+	ptn_bridge->bridge.type = DRM_MODE_CONNECTOR_LVDS;
 	ptn_bridge->bridge.of_node = dev->of_node;
 	drm_bridge_add(&ptn_bridge->bridge);
 
-- 
2.25.1

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

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

* Re: [PATCH v5 3/5] drm/bridge: tc358767: add drm_panel_bridge support
  2020-07-27 17:03 ` [PATCH v5 3/5] drm/bridge: tc358767: add drm_panel_bridge support Sam Ravnborg
@ 2020-07-27 17:12   ` Laurent Pinchart
  0 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2020-07-27 17:12 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Andrzej Hajda, Jernej Skrabec, Neil Armstrong, dri-devel, Jonas Karlman

Hi Sam,

Thank you for the patch.

On Mon, Jul 27, 2020 at 07:03:18PM +0200, Sam Ravnborg wrote:
> With the bridge operations implemented the last step to prepare
> this driver for a chained setup is the use of the bridge panel driver.
> 
> The bridge panel driver is only used when a port@2 is present in
> the DT. So when the display driver requests a connector
> support both situations:
> - connector created by bridge panel driver
> - connector created by this driver
> 
> And on top, support that the display driver creates the connector,
> which is the preferred setup.
> 
> Note: the bridge panel will use the connector type from the panel.
> 
> v3:
>   - Fix wrong logic in connector creation (Laurent)
> 
> v2:
>   - Merge connector and drm_panel_bridge patches
>     and fix so we do not create two connectors (Laurent)
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  drivers/gpu/drm/bridge/tc358767.c | 70 +++++++++++++++----------------
>  1 file changed, 35 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index b26c669f3e91..cbad2607ab43 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -244,8 +244,8 @@ struct tc_data {
>  	struct drm_dp_aux	aux;
>  
>  	struct drm_bridge	bridge;
> +	struct drm_bridge	*panel_bridge;
>  	struct drm_connector	connector;
> -	struct drm_panel	*panel;
>  
>  	/* link settings */
>  	struct tc_edp_link	link;
> @@ -1234,13 +1234,6 @@ static int tc_stream_disable(struct tc_data *tc)
>  	return 0;
>  }
>  
> -static void tc_bridge_pre_enable(struct drm_bridge *bridge)
> -{
> -	struct tc_data *tc = bridge_to_tc(bridge);
> -
> -	drm_panel_prepare(tc->panel);
> -}
> -
>  static void tc_bridge_enable(struct drm_bridge *bridge)
>  {
>  	struct tc_data *tc = bridge_to_tc(bridge);
> @@ -1264,8 +1257,6 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
>  		tc_main_link_disable(tc);
>  		return;
>  	}
> -
> -	drm_panel_enable(tc->panel);
>  }
>  
>  static void tc_bridge_disable(struct drm_bridge *bridge)
> @@ -1273,8 +1264,6 @@ static void tc_bridge_disable(struct drm_bridge *bridge)
>  	struct tc_data *tc = bridge_to_tc(bridge);
>  	int ret;
>  
> -	drm_panel_disable(tc->panel);
> -
>  	ret = tc_stream_disable(tc);
>  	if (ret < 0)
>  		dev_err(tc->dev, "main link stream stop error: %d\n", ret);
> @@ -1284,13 +1273,6 @@ static void tc_bridge_disable(struct drm_bridge *bridge)
>  		dev_err(tc->dev, "main link disable error: %d\n", ret);
>  }
>  
> -static void tc_bridge_post_disable(struct drm_bridge *bridge)
> -{
> -	struct tc_data *tc = bridge_to_tc(bridge);
> -
> -	drm_panel_unprepare(tc->panel);
> -}
> -
>  static bool tc_bridge_mode_fixup(struct drm_bridge *bridge,
>  				 const struct drm_display_mode *mode,
>  				 struct drm_display_mode *adj)
> @@ -1354,9 +1336,11 @@ static int tc_connector_get_modes(struct drm_connector *connector)
>  		return 0;
>  	}
>  
> -	num_modes = drm_panel_get_modes(tc->panel, connector);
> -	if (num_modes > 0)
> -		return num_modes;
> +	if (tc->panel_bridge) {
> +		num_modes = drm_bridge_get_modes(tc->panel_bridge, connector);
> +		if (num_modes > 0)
> +			return num_modes;
> +	}
>  
>  	edid = tc_get_edid(&tc->bridge, connector);
>  	num_modes = drm_add_edid_modes(connector, edid);
> @@ -1396,7 +1380,7 @@ tc_connector_detect(struct drm_connector *connector, bool force)
>  	if (tc->hpd_pin >= 0)
>  		return tc_bridge_detect(&tc->bridge);
>  
> -	if (tc->panel)
> +	if (tc->panel_bridge)
>  		return connector_status_connected;
>  	else
>  		return connector_status_unknown;
> @@ -1419,16 +1403,23 @@ static int tc_bridge_attach(struct drm_bridge *bridge,
>  	struct drm_device *drm = bridge->dev;
>  	int ret;
>  
> -	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
> -		DRM_ERROR("Fix bridge driver to make connector optional!");
> -		return -EINVAL;
> +	if (tc->panel_bridge) {
> +		enum drm_bridge_attach_flags panel_flags;
> +
> +		/* If a connector is required then this driver shall create it */
> +		panel_flags = flags | DRM_BRIDGE_ATTACH_NO_CONNECTOR;
> +		ret = drm_bridge_attach(tc->bridge.encoder, tc->panel_bridge,
> +					&tc->bridge, panel_flags);

You could write this

					flags | DRM_BRIDGE_ATTACH_NO_CONNECTOR);

and drop the panel_flags variable.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +		if (ret)
> +			return ret;
>  	}
>  
> +	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
> +		return 0;
> +
>  	/* Create DP/eDP connector */
>  	drm_connector_helper_add(&tc->connector, &tc_connector_helper_funcs);
> -	ret = drm_connector_init(drm, &tc->connector, &tc_connector_funcs,
> -				 tc->panel ? DRM_MODE_CONNECTOR_eDP :
> -				 DRM_MODE_CONNECTOR_DisplayPort);
> +	ret = drm_connector_init(drm, &tc->connector, &tc_connector_funcs, tc->bridge.type);
>  	if (ret)
>  		return ret;
>  
> @@ -1441,9 +1432,6 @@ static int tc_bridge_attach(struct drm_bridge *bridge,
>  					       DRM_CONNECTOR_POLL_DISCONNECT;
>  	}
>  
> -	if (tc->panel)
> -		drm_panel_attach(tc->panel, &tc->connector);
> -
>  	drm_display_info_set_bus_formats(&tc->connector.display_info,
>  					 &bus_format, 1);
>  	tc->connector.display_info.bus_flags =
> @@ -1459,10 +1447,8 @@ static const struct drm_bridge_funcs tc_bridge_funcs = {
>  	.attach = tc_bridge_attach,
>  	.mode_valid = tc_mode_valid,
>  	.mode_set = tc_bridge_mode_set,
> -	.pre_enable = tc_bridge_pre_enable,
>  	.enable = tc_bridge_enable,
>  	.disable = tc_bridge_disable,
> -	.post_disable = tc_bridge_post_disable,
>  	.mode_fixup = tc_bridge_mode_fixup,
>  	.detect = tc_bridge_detect,
>  	.get_edid = tc_get_edid,
> @@ -1555,6 +1541,7 @@ static irqreturn_t tc_irq_handler(int irq, void *arg)
>  static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  {
>  	struct device *dev = &client->dev;
> +	struct drm_panel *panel;
>  	struct tc_data *tc;
>  	int ret;
>  
> @@ -1565,10 +1552,23 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  	tc->dev = dev;
>  
>  	/* port@2 is the output port */
> -	ret = drm_of_find_panel_or_bridge(dev->of_node, 2, 0, &tc->panel, NULL);
> +	ret = drm_of_find_panel_or_bridge(dev->of_node, 2, 0, &panel, NULL);
>  	if (ret && ret != -ENODEV)
>  		return ret;
>  
> +	if (panel) {
> +		struct drm_bridge *panel_bridge;
> +
> +		panel_bridge = devm_drm_panel_bridge_add(dev, panel);
> +		if (IS_ERR(panel_bridge))
> +			return PTR_ERR(panel_bridge);
> +
> +		tc->panel_bridge = panel_bridge;
> +		tc->bridge.type = DRM_MODE_CONNECTOR_eDP;
> +	} else {
> +		tc->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
> +	}
> +
>  	/* Shut down GPIO is optional */
>  	tc->sd_gpio = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_HIGH);
>  	if (IS_ERR(tc->sd_gpio))

-- 
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] 9+ messages in thread

* Re: [PATCH v5 5/5] drm/bridge: nxp-ptn3460: add drm_panel_bridge support
  2020-07-27 17:03 ` [PATCH v5 5/5] drm/bridge: nxp-ptn3460: add drm_panel_bridge support Sam Ravnborg
@ 2020-07-27 17:13   ` Laurent Pinchart
  0 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2020-07-27 17:13 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Andrzej Hajda, Jernej Skrabec, Neil Armstrong, dri-devel, Jonas Karlman

Hi Sam,

Thank you for the patch.

On Mon, Jul 27, 2020 at 07:03:20PM +0200, Sam Ravnborg wrote:
> Prepare the bridge driver for use in a chained setup.
> 
> - Replacing direct use of drm_panel with drm_panel_bridge support.
> - Make the connector creation optional
> 
> Note: the bridge panel will use the connector type from the panel.
> 
> v3:
>   - Fix wrong logic in connector creation (Laurent)
> 
> v2:
>   - Use panel_bridge for local variable name to align with other drivers
>   - Fix that connector was created twice (Laurent)
>   - Set bridge.type to DRM_MODE_CONNECTOR_LVDS.
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  drivers/gpu/drm/bridge/nxp-ptn3460.c | 60 ++++++++++------------------
>  1 file changed, 20 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/nxp-ptn3460.c b/drivers/gpu/drm/bridge/nxp-ptn3460.c
> index 2805c8938f98..391c1f66f60f 100644
> --- a/drivers/gpu/drm/bridge/nxp-ptn3460.c
> +++ b/drivers/gpu/drm/bridge/nxp-ptn3460.c
> @@ -29,7 +29,7 @@ struct ptn3460_bridge {
>  	struct drm_connector connector;
>  	struct i2c_client *client;
>  	struct drm_bridge bridge;
> -	struct drm_panel *panel;
> +	struct drm_bridge *panel_bridge;
>  	struct gpio_desc *gpio_pd_n;
>  	struct gpio_desc *gpio_rst_n;
>  	u32 edid_emulation;
> @@ -126,11 +126,6 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)
>  	usleep_range(10, 20);
>  	gpiod_set_value(ptn_bridge->gpio_rst_n, 1);
>  
> -	if (drm_panel_prepare(ptn_bridge->panel)) {
> -		DRM_ERROR("failed to prepare panel\n");
> -		return;
> -	}
> -
>  	/*
>  	 * There's a bug in the PTN chip where it falsely asserts hotplug before
>  	 * it is fully functional. We're forced to wait for the maximum start up
> @@ -145,16 +140,6 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)
>  	ptn_bridge->enabled = true;
>  }
>  
> -static void ptn3460_enable(struct drm_bridge *bridge)
> -{
> -	struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge);
> -
> -	if (drm_panel_enable(ptn_bridge->panel)) {
> -		DRM_ERROR("failed to enable panel\n");
> -		return;
> -	}
> -}
> -
>  static void ptn3460_disable(struct drm_bridge *bridge)
>  {
>  	struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge);
> @@ -164,24 +149,10 @@ static void ptn3460_disable(struct drm_bridge *bridge)
>  
>  	ptn_bridge->enabled = false;
>  
> -	if (drm_panel_disable(ptn_bridge->panel)) {
> -		DRM_ERROR("failed to disable panel\n");
> -		return;
> -	}
> -
>  	gpiod_set_value(ptn_bridge->gpio_rst_n, 1);
>  	gpiod_set_value(ptn_bridge->gpio_pd_n, 0);
>  }
>  
> -static void ptn3460_post_disable(struct drm_bridge *bridge)
> -{
> -	struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge);
> -
> -	if (drm_panel_unprepare(ptn_bridge->panel)) {
> -		DRM_ERROR("failed to unprepare panel\n");
> -		return;
> -	}
> -}
>  
>  static struct edid *ptn3460_get_edid(struct drm_bridge *bridge,
>  				     struct drm_connector *connector)
> @@ -245,12 +216,18 @@ static int ptn3460_bridge_attach(struct drm_bridge *bridge,
>  				 enum drm_bridge_attach_flags flags)
>  {
>  	struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge);
> +	enum drm_bridge_attach_flags panel_flags;
>  	int ret;
>  
> -	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
> -		DRM_ERROR("Fix bridge driver to make connector optional!");
> -		return -EINVAL;
> -	}
> +	/* Let this driver create connector if requested */
> +	panel_flags = flags | DRM_BRIDGE_ATTACH_NO_CONNECTOR;
> +	ret = drm_bridge_attach(bridge->encoder, ptn_bridge->panel_bridge,
> +				bridge, panel_flags);

Same here, you could write

				flags | DRM_BRIDGE_ATTACH_NO_CONNECTOR);

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +	if (ret < 0)
> +		return ret;
> +
> +	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
> +		return 0;
>  
>  	if (!bridge->encoder) {
>  		DRM_ERROR("Parent encoder object not found");
> @@ -270,9 +247,6 @@ static int ptn3460_bridge_attach(struct drm_bridge *bridge,
>  	drm_connector_attach_encoder(&ptn_bridge->connector,
>  							bridge->encoder);
>  
> -	if (ptn_bridge->panel)
> -		drm_panel_attach(ptn_bridge->panel, &ptn_bridge->connector);
> -
>  	drm_helper_hpd_irq_event(ptn_bridge->connector.dev);
>  
>  	return ret;
> @@ -280,9 +254,7 @@ static int ptn3460_bridge_attach(struct drm_bridge *bridge,
>  
>  static const struct drm_bridge_funcs ptn3460_bridge_funcs = {
>  	.pre_enable = ptn3460_pre_enable,
> -	.enable = ptn3460_enable,
>  	.disable = ptn3460_disable,
> -	.post_disable = ptn3460_post_disable,
>  	.attach = ptn3460_bridge_attach,
>  	.get_edid = ptn3460_get_edid,
>  };
> @@ -292,6 +264,8 @@ static int ptn3460_probe(struct i2c_client *client,
>  {
>  	struct device *dev = &client->dev;
>  	struct ptn3460_bridge *ptn_bridge;
> +	struct drm_bridge *panel_bridge;
> +	struct drm_panel *panel;
>  	int ret;
>  
>  	ptn_bridge = devm_kzalloc(dev, sizeof(*ptn_bridge), GFP_KERNEL);
> @@ -299,10 +273,15 @@ static int ptn3460_probe(struct i2c_client *client,
>  		return -ENOMEM;
>  	}
>  
> -	ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0, &ptn_bridge->panel, NULL);
> +	ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0, &panel, NULL);
>  	if (ret)
>  		return ret;
>  
> +	panel_bridge = devm_drm_panel_bridge_add(dev, panel);
> +	if (IS_ERR(panel_bridge))
> +		return PTR_ERR(panel_bridge);
> +
> +	ptn_bridge->panel_bridge = panel_bridge;
>  	ptn_bridge->client = client;
>  
>  	ptn_bridge->gpio_pd_n = devm_gpiod_get(&client->dev, "powerdown",
> @@ -334,6 +313,7 @@ static int ptn3460_probe(struct i2c_client *client,
>  
>  	ptn_bridge->bridge.funcs = &ptn3460_bridge_funcs;
>  	ptn_bridge->bridge.ops = DRM_BRIDGE_OP_EDID;
> +	ptn_bridge->bridge.type = DRM_MODE_CONNECTOR_LVDS;
>  	ptn_bridge->bridge.of_node = dev->of_node;
>  	drm_bridge_add(&ptn_bridge->bridge);
>  

-- 
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] 9+ messages in thread

* Re: [PATCH v5 0/5] drm/bridge: Update tc358767 and nxp-ptn3460 to support chained bridges
  2020-07-27 17:03 [PATCH v5 0/5] drm/bridge: Update tc358767 and nxp-ptn3460 to support chained bridges Sam Ravnborg
                   ` (4 preceding siblings ...)
  2020-07-27 17:03 ` [PATCH v5 5/5] drm/bridge: nxp-ptn3460: add drm_panel_bridge support Sam Ravnborg
@ 2020-07-27 17:26 ` Sam Ravnborg
  5 siblings, 0 replies; 9+ messages in thread
From: Sam Ravnborg @ 2020-07-27 17:26 UTC (permalink / raw)
  To: dri-devel, Laurent Pinchart
  Cc: Andrzej Hajda, Jernej Skrabec, Neil Armstrong, Jonas Karlman

Hi all.

On Mon, Jul 27, 2020 at 07:03:15PM +0200, Sam Ravnborg wrote:
> This patch-set aims to make connector creation optional
> and prepare the bridge drivers for use in a chained setup.
> 
> The objective is that all bridge drivers shall support a chained setup
> connector creation is moved to the display drivers.
> This is just one step on this path.
> 
> The general approach for the bridge drivers:
> - Introduce bridge operations
> - Introduce use of panel bridge and make connector creation optional
> 
> v5:
>   - Applied reviewed patches, so we went from 15 to 5
>   - Fixed bug in connector creation in both drivers
> 
> v4:
>   - Dropped patch for ti-sn65dsi86. Await full conversion
>   - Dropped patch for ti-tpd12s015. It was wrong (Laurent)
>   - Drop boe,hv070wsa-100 patch, it was applied
>   - Combined a few patches to fix connector created twice (Laurent)
>   - Fix memory leak in get_edid (Laurent)
>   - Added patch to validate panel descriptions in panel-simple
>   - Set bridge.type in relevant drivers
>  
> v3:
>   - Rebase on top of drm-misc-next
>   - Address kbuild test robot feedback
>  
> v2:
>   - Updated bus_flags for boe,hv070wsa-100
>   - Collected r-b, but did not apply patches yet
>   - On the panel side the panel-simple driver gained a default
>     connector type for all the dumb panels that do not
>     include so in their description.
>     With this change panels always provide a connector type,
>     and we have the potential to drop most uses of
>     devm_drm_panel_bridge_add_typed().
>   - Added conversion of a few more bridge drivers
> 
> Patches can build but no run-time testing.
> So both test and review feedback appreciated!
> 
> 	Sam
> 
> 
> Sam Ravnborg (5):
>       drm/bridge: tc358767: add detect bridge operation
>       drm/bridge: tc358767: add get_edid bridge operation
>       drm/bridge: tc358767: add drm_panel_bridge support
>       drm/bridge: nxp-ptn3460: add get_edid bridge operation
>       drm/bridge: nxp-ptn3460: add drm_panel_bridge support

Fixed up per Laurent's suggestion and applied to drm-misc-next.

	Sam


> 
>  drivers/gpu/drm/bridge/nxp-ptn3460.c | 103 +++++++++++++---------------
>  drivers/gpu/drm/bridge/tc358767.c    | 126 +++++++++++++++++++----------------
>  2 files changed, 114 insertions(+), 115 deletions(-)
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

end of thread, other threads:[~2020-07-27 17:26 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-27 17:03 [PATCH v5 0/5] drm/bridge: Update tc358767 and nxp-ptn3460 to support chained bridges Sam Ravnborg
2020-07-27 17:03 ` [PATCH v5 1/5] drm/bridge: tc358767: add detect bridge operation Sam Ravnborg
2020-07-27 17:03 ` [PATCH v5 2/5] drm/bridge: tc358767: add get_edid " Sam Ravnborg
2020-07-27 17:03 ` [PATCH v5 3/5] drm/bridge: tc358767: add drm_panel_bridge support Sam Ravnborg
2020-07-27 17:12   ` Laurent Pinchart
2020-07-27 17:03 ` [PATCH v5 4/5] drm/bridge: nxp-ptn3460: add get_edid bridge operation Sam Ravnborg
2020-07-27 17:03 ` [PATCH v5 5/5] drm/bridge: nxp-ptn3460: add drm_panel_bridge support Sam Ravnborg
2020-07-27 17:13   ` Laurent Pinchart
2020-07-27 17:26 ` [PATCH v5 0/5] drm/bridge: Update tc358767 and nxp-ptn3460 to support chained bridges Sam Ravnborg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.