All of lore.kernel.org
 help / color / mirror / Atom feed
From: Douglas Anderson <dianders@chromium.org>
To: Andrzej Hajda <a.hajda@samsung.com>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	Jonas Karlman <jonas@kwiboo.se>,
	Jernej Skrabec <jernej.skrabec@siol.net>,
	Sam Ravnborg <sam@ravnborg.org>
Cc: Linus W <linus.walleij@linaro.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	robdclark@chromium.org, Stephen Boyd <swboyd@chromium.org>,
	Steev Klimaszewski <steev@kali.org>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	linux-arm-msm@vger.kernel.org,
	Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>,
	Douglas Anderson <dianders@chromium.org>,
	Daniel Vetter <daniel@ffwll.ch>, David Airlie <airlied@linux.ie>,
	Robert Foss <robert.foss@linaro.org>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: [PATCH v3 08/12] drm/bridge: ti-sn65dsi86: Power things properly for reading the EDID
Date: Fri,  2 Apr 2021 15:28:42 -0700	[thread overview]
Message-ID: <20210402152701.v3.8.Ied721dc895156046ac523baa55a71da241cd09c7@changeid> (raw)
In-Reply-To: <20210402222846.2461042-1-dianders@chromium.org>

eDP panels won't provide their EDID unless they're powered on. Let's
chain a power-on before we read the EDID. This roughly matches what
was done in 'parade-ps8640.c'.

NOTE: The old code attempted to call pm_runtime_get_sync() before
reading the EDID. While that was enough to power the bridge chip on,
it wasn't enough to talk to the panel for two reasons:
1. 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.
2. Even if we fixed things to ignore HPD, the EDID still wouldn't read
   if the panel wasn't on.

ALSO NOTE: Without the future patch ("drm/panel: panel-simple: Use
runtime pm to avoid excessive unprepare / prepare") there will be boot
speed implications here. Specifically we'll power the panel on to read
the EDID, then fully off. Then we'll likely have to wait the minimum
time between power off and power on.

Fixes: 58074b08c04a ("drm/bridge: ti-sn65dsi86: Read EDID blob over DDC")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v3:
- Rebased now that we're not moving EDID caching to the core.
- Separating out patch to block AUX channel when not powered.
- Added note about boot speed implications.

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 6390bc58f29a..543590801a8e 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -129,6 +129,7 @@
  * @dp_lanes:     Count of dp_lanes we're using.
  * @ln_assign:    Value to program to the LN_ASSIGN register.
  * @ln_polrs:     Value for the 4-bit LN_POLRS field of SN_ENH_FRAME_REG.
+ * @pre_enabled:  If true then pre_enable() has run.
  *
  * @gchip:        If we expose our GPIOs, this is used.
  * @gchip_output: A cache of whether we've set GPIOs to output.  This
@@ -157,6 +158,7 @@ struct ti_sn_bridge {
 	int				dp_lanes;
 	u8				ln_assign;
 	u8				ln_polrs;
+	bool				pre_enabled;
 
 #if defined(CONFIG_OF_GPIO)
 	struct gpio_chip		gchip;
@@ -270,12 +272,17 @@ static int ti_sn_bridge_connector_get_modes(struct drm_connector *connector)
 {
 	struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
 	struct edid *edid = pdata->edid;
+	bool was_enabled;
 	int num;
 
 	if (!edid) {
-		pm_runtime_get_sync(pdata->dev);
+		was_enabled = pdata->pre_enabled;
+
+		if (!was_enabled)
+			drm_bridge_chain_pre_enable(&pdata->bridge);
 		edid = pdata->edid = drm_get_edid(connector, &pdata->aux.ddc);
-		pm_runtime_put(pdata->dev);
+		if (!was_enabled)
+			drm_bridge_chain_post_disable(&pdata->bridge);
 	}
 
 	if (edid && drm_edid_is_valid(edid)) {
@@ -846,12 +853,16 @@ static void ti_sn_bridge_pre_enable(struct drm_bridge *bridge)
 			   HPD_DISABLE);
 
 	drm_panel_prepare(pdata->panel);
+
+	pdata->pre_enabled = true;
 }
 
 static void ti_sn_bridge_post_disable(struct drm_bridge *bridge)
 {
 	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
 
+	pdata->pre_enabled = false;
+
 	drm_panel_unprepare(pdata->panel);
 
 	clk_disable_unprepare(pdata->refclk);
-- 
2.31.0.208.g409f899ff0-goog


WARNING: multiple messages have this Message-ID (diff)
From: Douglas Anderson <dianders@chromium.org>
To: Andrzej Hajda <a.hajda@samsung.com>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	Jonas Karlman <jonas@kwiboo.se>,
	Jernej Skrabec <jernej.skrabec@siol.net>,
	Sam Ravnborg <sam@ravnborg.org>
Cc: robdclark@chromium.org, dri-devel@lists.freedesktop.org,
	David Airlie <airlied@linux.ie>,
	linux-arm-msm@vger.kernel.org,
	Douglas Anderson <dianders@chromium.org>,
	Steev Klimaszewski <steev@kali.org>,
	Stephen Boyd <swboyd@chromium.org>,
	Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>,
	Robert Foss <robert.foss@linaro.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	linux-kernel@vger.kernel.org
Subject: [PATCH v3 08/12] drm/bridge: ti-sn65dsi86: Power things properly for reading the EDID
Date: Fri,  2 Apr 2021 15:28:42 -0700	[thread overview]
Message-ID: <20210402152701.v3.8.Ied721dc895156046ac523baa55a71da241cd09c7@changeid> (raw)
In-Reply-To: <20210402222846.2461042-1-dianders@chromium.org>

eDP panels won't provide their EDID unless they're powered on. Let's
chain a power-on before we read the EDID. This roughly matches what
was done in 'parade-ps8640.c'.

NOTE: The old code attempted to call pm_runtime_get_sync() before
reading the EDID. While that was enough to power the bridge chip on,
it wasn't enough to talk to the panel for two reasons:
1. 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.
2. Even if we fixed things to ignore HPD, the EDID still wouldn't read
   if the panel wasn't on.

ALSO NOTE: Without the future patch ("drm/panel: panel-simple: Use
runtime pm to avoid excessive unprepare / prepare") there will be boot
speed implications here. Specifically we'll power the panel on to read
the EDID, then fully off. Then we'll likely have to wait the minimum
time between power off and power on.

Fixes: 58074b08c04a ("drm/bridge: ti-sn65dsi86: Read EDID blob over DDC")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v3:
- Rebased now that we're not moving EDID caching to the core.
- Separating out patch to block AUX channel when not powered.
- Added note about boot speed implications.

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 6390bc58f29a..543590801a8e 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -129,6 +129,7 @@
  * @dp_lanes:     Count of dp_lanes we're using.
  * @ln_assign:    Value to program to the LN_ASSIGN register.
  * @ln_polrs:     Value for the 4-bit LN_POLRS field of SN_ENH_FRAME_REG.
+ * @pre_enabled:  If true then pre_enable() has run.
  *
  * @gchip:        If we expose our GPIOs, this is used.
  * @gchip_output: A cache of whether we've set GPIOs to output.  This
@@ -157,6 +158,7 @@ struct ti_sn_bridge {
 	int				dp_lanes;
 	u8				ln_assign;
 	u8				ln_polrs;
+	bool				pre_enabled;
 
 #if defined(CONFIG_OF_GPIO)
 	struct gpio_chip		gchip;
@@ -270,12 +272,17 @@ static int ti_sn_bridge_connector_get_modes(struct drm_connector *connector)
 {
 	struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
 	struct edid *edid = pdata->edid;
+	bool was_enabled;
 	int num;
 
 	if (!edid) {
-		pm_runtime_get_sync(pdata->dev);
+		was_enabled = pdata->pre_enabled;
+
+		if (!was_enabled)
+			drm_bridge_chain_pre_enable(&pdata->bridge);
 		edid = pdata->edid = drm_get_edid(connector, &pdata->aux.ddc);
-		pm_runtime_put(pdata->dev);
+		if (!was_enabled)
+			drm_bridge_chain_post_disable(&pdata->bridge);
 	}
 
 	if (edid && drm_edid_is_valid(edid)) {
@@ -846,12 +853,16 @@ static void ti_sn_bridge_pre_enable(struct drm_bridge *bridge)
 			   HPD_DISABLE);
 
 	drm_panel_prepare(pdata->panel);
+
+	pdata->pre_enabled = true;
 }
 
 static void ti_sn_bridge_post_disable(struct drm_bridge *bridge)
 {
 	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
 
+	pdata->pre_enabled = false;
+
 	drm_panel_unprepare(pdata->panel);
 
 	clk_disable_unprepare(pdata->refclk);
-- 
2.31.0.208.g409f899ff0-goog

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

  parent reply	other threads:[~2021-04-02 22:29 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-02 22:28 [PATCH v3 00/12] drm: Fix EDID reading on ti-sn65dsi86 Douglas Anderson
2021-04-02 22:28 ` Douglas Anderson
2021-04-02 22:28 ` [PATCH v3 01/12] drm/bridge: Fix the stop condition of drm_bridge_chain_pre_enable() Douglas Anderson
2021-04-02 22:28   ` Douglas Anderson
2021-04-05  0:49   ` Laurent Pinchart
2021-04-05  0:49     ` Laurent Pinchart
2021-04-15  1:19     ` Doug Anderson
2021-04-15  1:19       ` Doug Anderson
2021-04-15  1:56       ` Laurent Pinchart
2021-04-15  1:56         ` Laurent Pinchart
2021-04-15 14:48         ` Doug Anderson
2021-04-15 14:48           ` Doug Anderson
2021-04-02 22:28 ` [PATCH v3 02/12] drm/bridge: ti-sn65dsi86: Simplify refclk handling Douglas Anderson
2021-04-02 22:28   ` Douglas Anderson
2021-04-02 22:28 ` [PATCH v3 03/12] drm/bridge: ti-sn65dsi86: Remove incorrectly tagged kerneldoc comment Douglas Anderson
2021-04-02 22:28   ` Douglas Anderson
2021-04-05  0:50   ` Laurent Pinchart
2021-04-05  0:50     ` Laurent Pinchart
2021-04-02 22:28 ` [PATCH v3 04/12] drm/bridge: ti-sn65dsi86: Reorder remove() Douglas Anderson
2021-04-02 22:28   ` Douglas Anderson
2021-04-05  0:52   ` Laurent Pinchart
2021-04-05  0:52     ` Laurent Pinchart
2021-04-02 22:28 ` [PATCH v3 05/12] drm/bridge: ti-sn65dsi86: Move drm_panel_unprepare() to post_disable() Douglas Anderson
2021-04-02 22:28   ` Douglas Anderson
2021-04-05  0:58   ` Laurent Pinchart
2021-04-05  0:58     ` Laurent Pinchart
2021-04-02 22:28 ` [PATCH v3 06/12] drm/bridge: ti-sn65dsi86: Get rid of the useless detect() function Douglas Anderson
2021-04-02 22:28   ` Douglas Anderson
2021-04-05  0:58   ` Laurent Pinchart
2021-04-05  0:58     ` Laurent Pinchart
2021-04-02 22:28 ` [PATCH v3 07/12] drm/bridge: ti-sn65dsi86: Remove extra call: drm_connector_update_edid_property() Douglas Anderson
2021-04-02 22:28   ` Douglas Anderson
2021-04-05  1:01   ` Laurent Pinchart
2021-04-05  1:01     ` Laurent Pinchart
2021-04-02 22:28 ` Douglas Anderson [this message]
2021-04-02 22:28   ` [PATCH v3 08/12] drm/bridge: ti-sn65dsi86: Power things properly for reading the EDID Douglas Anderson
2021-04-02 22:28 ` [PATCH v3 09/12] drm/bridge: ti-sn65dsi86: Fail aux transfers right away if not powered Douglas Anderson
2021-04-02 22:28   ` Douglas Anderson
2021-04-02 22:28 ` [PATCH v3 10/12] drm/bridge: ti-sn65dsi86: Read the EDID only if refclk was provided Douglas Anderson
2021-04-02 22:28   ` Douglas Anderson
2021-04-05  1:04   ` Laurent Pinchart
2021-04-05  1:04     ` Laurent Pinchart
2021-04-02 22:28 ` [PATCH v3 11/12] drm/bridge: ti-sn65dsi86: Print an error if we fallback to panel modes Douglas Anderson
2021-04-02 22:28   ` Douglas Anderson
2021-04-05  1:04   ` Laurent Pinchart
2021-04-05  1:04     ` Laurent Pinchart
2021-04-02 22:28 ` [PATCH v3 12/12] drm/panel: panel-simple: Use runtime pm to avoid excessive unprepare / prepare Douglas Anderson
2021-04-02 22:28   ` Douglas Anderson
2021-04-15  0:58   ` Laurent Pinchart
2021-04-15  0:58     ` Laurent Pinchart
2021-04-15  1:22     ` Doug Anderson
2021-04-15  1:22       ` Doug Anderson
2021-04-15  1:30       ` Laurent Pinchart
2021-04-15  1:30         ` Laurent Pinchart

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=20210402152701.v3.8.Ied721dc895156046ac523baa55a71da241cd09c7@changeid \
    --to=dianders@chromium.org \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=a.hajda@samsung.com \
    --cc=airlied@linux.ie \
    --cc=bjorn.andersson@linaro.org \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jernej.skrabec@siol.net \
    --cc=jonas@kwiboo.se \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=narmstrong@baylibre.com \
    --cc=robdclark@chromium.org \
    --cc=robert.foss@linaro.org \
    --cc=sam@ravnborg.org \
    --cc=stanislav.lisovskiy@intel.com \
    --cc=steev@kali.org \
    --cc=swboyd@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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 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.