| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 29 Mar 2021 19:53:42 -0700
From: Douglas Anderson <dianders@...omium.org>
To: Andrzej Hajda <a.hajda@...sung.com>,
Neil Armstrong <narmstrong@...libre.com>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Jonas Karlman <jonas@...boo.se>,
Jernej Skrabec <jernej.skrabec@...l.net>,
Sam Ravnborg <sam@...nborg.org>
Cc: Bjorn Andersson <bjorn.andersson@...aro.org>,
Stephen Boyd <swboyd@...omium.org>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
robdclark@...omium.org,
Stanislav Lisovskiy <stanislav.lisovskiy@...el.com>,
linux-arm-msm@...r.kernel.org, Steev Klimaszewski <steev@...i.org>,
Linus W <linus.walleij@...aro.org>,
Douglas Anderson <dianders@...omium.org>,
Daniel Vetter <daniel@...ll.ch>,
David Airlie <airlied@...ux.ie>,
Robert Foss <robert.foss@...aro.org>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: [PATCH v2 11/14] drm/bridge: ti-sn65dsi86: Power things properly for reading the EDID
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.
One thing that's a bit odd here is taking advantage of the EDID that
the core might have cached for us. See the patch ("drm/edid: Use the
cached EDID in drm_get_edid() if eDP"). We manage to get at the cache
by:
- Instantly failing aux transfers if we're not powered.
- If the first read of the EDID fails we try again after powering.
Fixes: 58074b08c04a ("drm/bridge: ti-sn65dsi86: Read EDID blob over DDC")
Signed-off-by: Douglas Anderson <dianders@...omium.org>
---
Depending on what people think of the other patches in this series,
some of this could change.
- If everyone loves the "runtime PM" in the panel driver then we
could, in theory, put the pre-enable chaining straight in the "aux
transfer" function.
- If everyone hates the EDID cache moving to the core then we can
avoid some of the awkward flow of things and keep the EDID cache in
the sn65dsi86 driver.
(no changes since v1)
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 39 +++++++++++++++++++++++++--
1 file changed, 37 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index c0398daaa4a6..673c9f1c2d8e 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -128,6 +128,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
@@ -155,6 +156,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;
@@ -268,11 +270,33 @@ 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;
+ bool was_enabled;
int num = 0;
- pm_runtime_get_sync(pdata->dev);
+ /*
+ * Try to get the EDID first without anything special. There are
+ * three things that could happen with this call.
+ * a) It might just return from its cache.
+ * b) It might try to initiate an AUX transfer which might work.
+ * c) It might try to initiate an AUX transfer which might fail because
+ * we're not powered up.
+ *
+ * If we get a failure we'll assume case c) and try again. NOTE: we
+ * don't want to power up every time because that's slow and we don't
+ * have visibility into whether the data has already been cached.
+ */
edid = drm_get_edid(connector, &pdata->aux.ddc);
- pm_runtime_put(pdata->dev);
+ if (!edid) {
+ was_enabled = pdata->pre_enabled;
+
+ if (!was_enabled)
+ drm_bridge_chain_pre_enable(&pdata->bridge);
+
+ edid = drm_get_edid(connector, &pdata->aux.ddc);
+
+ if (!was_enabled)
+ drm_bridge_chain_post_disable(&pdata->bridge);
+ }
if (edid) {
if (drm_edid_is_valid(edid))
@@ -852,12 +876,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);
@@ -891,6 +919,13 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
int ret;
u8 addr_len[SN_AUX_LENGTH_REG + 1 - SN_AUX_ADDR_19_16_REG];
+ /*
+ * Things just won't work if the panel isn't powered. Return failure
+ * right away.
+ */
+ if (!pdata->pre_enabled)
+ return -EIO;
+
if (len > SN_AUX_MAX_PAYLOAD_BYTES)
return -EINVAL;
--
2.31.0.291.g576ba9dcdaf-goog
Powered by blists - more mailing lists