archive mirror
 help / color / mirror / Atom feed
From: Douglas Anderson <>
To: Andrzej Hajda <>,
	Neil Armstrong <>,
	Laurent Pinchart <>,
	Jonas Karlman <>,
	Jernej Skrabec <>,
	Sam Ravnborg <>, Wolfram Sang <>
Cc: Lyude Paul <>,
	Steev Klimaszewski <>,
	Stephen Boyd <>,
	Bjorn Andersson <>,,
	Stanislav Lisovskiy <>,
	Linus W <>,,
	Maarten Lankhorst <>,
	Thierry Reding <>,,,
	Douglas Anderson <>,
	Daniel Vetter <>, David Airlie <>,
	Robert Foss <>,
Subject: [PATCH v6 4/5] drm/bridge: ti-sn65dsi86: Don't read EDID blob over DDC
Date: Mon,  3 May 2021 14:58:43 -0700	[thread overview]
Message-ID: <20210503145750.v6.4.I9330684c25f65bb318eff57f0616500f83eac3cc@changeid> (raw)
In-Reply-To: <>

This is really just a revert of commit 58074b08c04a ("drm/bridge:
ti-sn65dsi86: Read EDID blob over DDC"), resolving conflicts.

The old code failed to read the EDID properly in a very important
case: before the bridge's pre_enable() was called. The way things need
to work:
1. Read the EDID.
2. Based on the EDID, decide on video settings and pixel clock.
3. Enable the bridge w/ the desired settings.

The way things were working:
1. Try to read the EDID but fail; fall back to hardcoded values.
2. Based on hardcoded values, decide on video settings and pixel clock.
3. Enable the bridge w/ the desired settings.
4. Try again to read the EDID, it works now!
5. Realize that the hardcoded settings weren't quite right.
6. Disable / reenable the bridge w/ the right settings.

The reasons for the failures were twofold:
a) Since we never ran the bridge chip's pre-enable then we never set
   the bit to ignore HPD. This meant the bridge chip didn't even _try_
   to go out on the bus and communicate with the panel.
b) Even if we fixed things to ignore HPD, the EDID still wouldn't read
   if the panel wasn't on.

Instead of reverting the code, we could fix it to set the HPD bit and
also power on the panel. However, it also works nicely to just let the
panel code read the EDID. Now that we've split the driver up we can
expose the DDC AUX channel bus to the panel node. The panel can take
charge of reading the EDID.

NOTE: in order for things to work, anyone that needs to read the EDID
will need to add something that looks like this to their panel in the
  ddc-i2c-bus = <&sn65dsi86_bridge>;

Presumably it's OK to land this without waiting for users to add the
dts property since the EDID reading was a bit broken anyway, was
"recently" added, and we know we must have the fallback mode to use
(since the EDID reading was a bit broken).

Suggested-by: Andrzej Hajda <>
Signed-off-by: Douglas Anderson <>
Reviewed-by: Bjorn Andersson <>

(no changes since v1)

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 594aac57bdbc..9f0f785f380d 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -125,7 +125,6 @@
  * @connector:    Our connector.
  * @host_node:    Remote DSI node.
  * @dsi:          Our MIPI DSI source.
- * @edid:         Detected EDID of eDP panel.
  * @refclk:       Our reference clock.
  * @panel:        Our panel.
  * @enable_gpio:  The GPIO we toggle to enable the bridge.
@@ -156,7 +155,6 @@ struct ti_sn65dsi86 {
 	struct drm_dp_aux		aux;
 	struct drm_bridge		bridge;
 	struct drm_connector		connector;
-	struct edid			*edid;
 	struct device_node		*host_node;
 	struct mipi_dsi_device		*dsi;
 	struct clk			*refclk;
@@ -405,24 +403,6 @@ connector_to_ti_sn65dsi86(struct drm_connector *connector)
 static int ti_sn_bridge_connector_get_modes(struct drm_connector *connector)
 	struct ti_sn65dsi86 *pdata = connector_to_ti_sn65dsi86(connector);
-	struct edid *edid = pdata->edid;
-	int num, ret;
-	if (!edid) {
-		pm_runtime_get_sync(pdata->dev);
-		edid = pdata->edid = drm_get_edid(connector, &pdata->aux.ddc);
-		pm_runtime_put_autosuspend(pdata->dev);
-	}
-	if (edid && drm_edid_is_valid(edid)) {
-		ret = drm_connector_update_edid_property(connector, edid);
-		if (!ret) {
-			num = drm_add_edid_modes(connector, edid);
-			if (num)
-				return num;
-		}
-	}
 	return drm_panel_get_modes(pdata->panel, connector);
@@ -1353,8 +1333,6 @@ static void ti_sn_bridge_remove(struct auxiliary_device *adev)
-	kfree(pdata->edid);

  parent reply	other threads:[~2021-05-03 21:59 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-03 21:58 [PATCH v6 0/5] drm: Fix EDID reading on ti-sn65dsi86; solve some chicken-and-egg problems Douglas Anderson
2021-05-03 21:58 ` [PATCH v6 1/5] i2c: i2c-core-of: Fix corner case of finding adapter by node Douglas Anderson
2021-05-17 20:16   ` Doug Anderson
2021-05-03 21:58 ` [PATCH v6 2/5] drm/dp: Allow an early call to register DDC i2c bus Douglas Anderson
2021-05-07 21:18   ` Lyude Paul
2021-05-07 22:00     ` Bjorn Andersson
2021-05-07 22:09       ` Lyude Paul
2021-05-14 11:16         ` Jani Nikula
2021-05-17 20:17           ` Doug Anderson
2021-05-03 21:58 ` [PATCH v6 3/5] drm/bridge: ti-sn65dsi86: Promote the AUX channel to its own sub-dev Douglas Anderson
2021-05-03 21:58 ` Douglas Anderson [this message]
2021-05-03 21:58 ` [PATCH v6 5/5] arm64: dts: qcom: Link the panel to the bridge's DDC bus Douglas Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210503145750.v6.4.I9330684c25f65bb318eff57f0616500f83eac3cc@changeid \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).