On Mon, Mar 25, 2024 at 2:57 PM Douglas Anderson <dianders@xxxxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxxxx>
Reviewed-by: Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx>
> ---
>
> 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
>
