All of lore.kernel.org
 help / color / mirror / Atom feed
From: Douglas Anderson <dianders@chromium.org>
To: dri-devel@lists.freedesktop.org
Cc: Pin-yen Lin <treapking@chromium.org>,
	Prahlad Kilambi <prahladk@google.com>,
	Hsin-Yi Wang <hsinyi@chromium.org>,
	Douglas Anderson <dianders@chromium.org>,
	Daniel Vetter <daniel@ffwll.ch>, David Airlie <airlied@gmail.com>,
	Jessica Zhang <quic_jesszhan@quicinc.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Sam Ravnborg <sam@ravnborg.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 2/3] drm/panel-edp: If we fail to powerup/get EDID, use conservative timings
Date: Mon, 25 Mar 2024 14:56:26 -0700	[thread overview]
Message-ID: <20240325145626.2.Ia7a55a9657b0b6aa4644fd497a0bc595a771258c@changeid> (raw)
In-Reply-To: <20240325215631.3804796-1-dianders@chromium.org>

If at boot we fail to power up the eDP panel (most often happens if
the eDP panel never asserts HPD to us) or if we are unable to read the
EDID at bootup to figure out the panel's ID then let's use the
conservative eDP panel powerup/powerdown timings but _not_ fail the
probe.

It might seem strange to _not_ fail the probe in this case since we
were unable to powerup the panel and confirm it's there. However,
there is a reason to do this. Specifically, if we fail to probe the
panel then it really throws the whole display pipeline for loop. Most
DRM subsystems are written so that they wait until all components
(including the panel) have probed before they set everything up. When
the panel doesn't come up then this never happens. As a side effect of
not setting everything up then other display adapters don't get
initialized. As a practical example, I can see that if I open up a
sc7180-trogdor based Chromebook that's using the generic "edp-panel"
and unplug the eDP panel that it causes the _external_ DP monitor not
to function. This is obviously bad because it means that a device with
a dead eDP panel becomes e-waste when it could instead still be given
useful life with an external display.

NOTES:
- When we fail to probe like this, boot is a bit slow because we try
  several times to power the panel up. This doesn't feel horrible
  because it'll eventually work and the retries are known to help
  bring some panels up.
- In the case where we hit the condition of failing to power up, the
  display will likely _never_ have a chance to work again until
  reboot. Once the panel-edp pm_runtime resume function fails it
  doesn't ever seem to retry. This is probably for the best given that
  we don't have any real timing/modes. eDP isn't expected to be
  "hotplugged" so this makes some sense.
- It turns out that this makes panel-edp behave more similarly for
  users of the generic "edp-panel" compatible string and the old fixed
  panel compatible string. With the old fixed panel compatible string
  we don't talk to the panel during probe so we'd actually behave much
  the same way that we'll now behave for the generic "edp-panel".

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 drivers/gpu/drm/panel/panel-edp.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
index 8a19fea90ddf..607cdd6feda9 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -808,7 +808,10 @@ static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel)
 	/* Power the panel on so we can read the EDID */
 	ret = pm_runtime_get_sync(dev);
 	if (ret < 0) {
-		dev_err(dev, "Couldn't power on panel to read EDID: %d\n", ret);
+		dev_err(dev,
+			"Couldn't power on panel to ID it; using conservative timings: %d\n",
+			ret);
+		panel_edp_set_conservative_timings(panel, desc);
 		goto exit;
 	}
 
@@ -816,8 +819,8 @@ static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel)
 	if (base_block) {
 		panel_id = drm_edid_get_panel_id(base_block);
 	} else {
-		dev_err(dev, "Couldn't identify panel via EDID\n");
-		ret = -EIO;
+		dev_err(dev, "Couldn't read EDID for ID; using conservative timings\n");
+		panel_edp_set_conservative_timings(panel, desc);
 		goto exit;
 	}
 	drm_edid_decode_panel_id(panel_id, vend, &product_id);
@@ -844,12 +847,11 @@ static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel)
 		desc->delay = *panel->detected_panel->delay;
 	}
 
-	ret = 0;
 exit:
 	pm_runtime_mark_last_busy(dev);
 	pm_runtime_put_autosuspend(dev);
 
-	return ret;
+	return 0;
 }
 
 static int panel_edp_probe(struct device *dev, const struct panel_desc *desc,
-- 
2.44.0.396.g6e790dbe36-goog


  parent reply	other threads:[~2024-03-25 21:57 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-25 21:56 [PATCH 0/3] drm-panel: Don't make failures quite so fatal Douglas Anderson
2024-03-25 21:56 ` [PATCH 1/3] drm/panel-edp: Abstract out function to set conservative timings Douglas Anderson
2024-03-25 23:51   ` Hsin-Yi Wang
2024-04-08  6:54     ` Doug Anderson
2024-03-25 21:56 ` Douglas Anderson [this message]
2024-03-26  0:05   ` [PATCH 2/3] drm/panel-edp: If we fail to powerup/get EDID, use " Hsin-Yi Wang
2024-04-08  6:55     ` Doug Anderson
2024-03-25 21:56 ` [PATCH 3/3] drm-panel: If drm_panel_dp_aux_backlight() fails, don't fail panel probe Douglas Anderson
2024-03-26  0:07   ` Hsin-Yi Wang
2024-04-08  6:55     ` Doug 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:
  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=20240325145626.2.Ia7a55a9657b0b6aa4644fd497a0bc595a771258c@changeid \
    --to=dianders@chromium.org \
    --cc=airlied@gmail.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hsinyi@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=neil.armstrong@linaro.org \
    --cc=prahladk@google.com \
    --cc=quic_jesszhan@quicinc.com \
    --cc=sam@ravnborg.org \
    --cc=treapking@chromium.org \
    --cc=tzimmermann@suse.de \
    /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.