| Message ID | 20240325145626.3.I552e8af0ddb1691cc0fe5d27ea3d8020e36f7006@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm-panel: Don't make failures quite so fatal | expand |
On Mon, Mar 25, 2024 at 2:57 PM Douglas Anderson <dianders@chromium.org> wrote: > > If we're using the AUX channel for eDP backlight and it fails to probe > for some reason, let's _not_ fail the panel probe. > > At least one case where we could fail to init the backlight is because > of a dead or physically missing panel. As talked about in detail in > the earlier patch in this series, ("drm/panel-edp: If we fail to > powerup/get EDID, use conservative timings"), this can cause the > entire system's display pipeline to fail to come up and that's > non-ideal. > > If we fail to init the backlight for some transitory reason, we should > dig in and see if there's a way to fix this (perhaps retries?). Even > in that case, though, having a panel whose backlight is stuck at 100% > (the default, at least in the panel Samsung ATNA33XC20 I tested) is > better than having no panel at all. > > Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Hsin-Yi Wang <hsinyi@chromium.org> > --- > If needed, I could split this into two patches: one for each of the > two panels that use drm_panel_dp_aux_backlight(). Since they both go > through drm-misc, though, it doesn't feel worth it. > > drivers/gpu/drm/panel/panel-edp.c | 8 +++++++- > drivers/gpu/drm/panel/panel-samsung-atna33xc20.c | 9 +++++++-- > 2 files changed, 14 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c > index 607cdd6feda9..0bf66d9dd5b8 100644 > --- a/drivers/gpu/drm/panel/panel-edp.c > +++ b/drivers/gpu/drm/panel/panel-edp.c > @@ -944,8 +944,14 @@ static int panel_edp_probe(struct device *dev, const struct panel_desc *desc, > err = drm_panel_dp_aux_backlight(&panel->base, panel->aux); > pm_runtime_mark_last_busy(dev); > pm_runtime_put_autosuspend(dev); > + > + /* > + * Warn if we get an error, but don't consider it fatal. Having > + * a panel where we can't control the backlight is better than > + * no panel. > + */ > if (err) > - goto err_finished_pm_runtime; > + dev_warn(dev, "failed to register dp aux backlight: %d\n", err); > } > > drm_panel_add(&panel->base); > diff --git a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c > index 9c336c71562b..6828a4f24d14 100644 > --- a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c > +++ b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c > @@ -328,9 +328,14 @@ static int atana33xc20_probe(struct dp_aux_ep_device *aux_ep) > ret = drm_panel_dp_aux_backlight(&panel->base, aux_ep->aux); > pm_runtime_mark_last_busy(dev); > pm_runtime_put_autosuspend(dev); > + > + /* > + * Warn if we get an error, but don't consider it fatal. Having > + * a panel where we can't control the backlight is better than > + * no panel. > + */ > if (ret) > - return dev_err_probe(dev, ret, > - "failed to register dp aux backlight\n"); > + dev_warn(dev, "failed to register dp aux backlight: %d\n", ret); > > drm_panel_add(&panel->base); > > -- > 2.44.0.396.g6e790dbe36-goog >
Hi, On Mon, Mar 25, 2024 at 5:07 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote: > > On Mon, Mar 25, 2024 at 2:57 PM Douglas Anderson <dianders@chromium.org> wrote: > > > > If we're using the AUX channel for eDP backlight and it fails to probe > > for some reason, let's _not_ fail the panel probe. > > > > At least one case where we could fail to init the backlight is because > > of a dead or physically missing panel. As talked about in detail in > > the earlier patch in this series, ("drm/panel-edp: If we fail to > > powerup/get EDID, use conservative timings"), this can cause the > > entire system's display pipeline to fail to come up and that's > > non-ideal. > > > > If we fail to init the backlight for some transitory reason, we should > > dig in and see if there's a way to fix this (perhaps retries?). Even > > in that case, though, having a panel whose backlight is stuck at 100% > > (the default, at least in the panel Samsung ATNA33XC20 I tested) is > > better than having no panel at all. > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > Reviewed-by: Hsin-Yi Wang <hsinyi@chromium.org> Pushed to drm-misc-next: b48ccb18e642 drm-panel: If drm_panel_dp_aux_backlight() fails, don't fail panel probe
diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c index 607cdd6feda9..0bf66d9dd5b8 100644 --- a/drivers/gpu/drm/panel/panel-edp.c +++ b/drivers/gpu/drm/panel/panel-edp.c @@ -944,8 +944,14 @@ static int panel_edp_probe(struct device *dev, const struct panel_desc *desc, err = drm_panel_dp_aux_backlight(&panel->base, panel->aux); pm_runtime_mark_last_busy(dev); pm_runtime_put_autosuspend(dev); + + /* + * Warn if we get an error, but don't consider it fatal. Having + * a panel where we can't control the backlight is better than + * no panel. + */ if (err) - goto err_finished_pm_runtime; + dev_warn(dev, "failed to register dp aux backlight: %d\n", err); } drm_panel_add(&panel->base); diff --git a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c index 9c336c71562b..6828a4f24d14 100644 --- a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c +++ b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c @@ -328,9 +328,14 @@ static int atana33xc20_probe(struct dp_aux_ep_device *aux_ep) ret = drm_panel_dp_aux_backlight(&panel->base, aux_ep->aux); pm_runtime_mark_last_busy(dev); pm_runtime_put_autosuspend(dev); + + /* + * Warn if we get an error, but don't consider it fatal. Having + * a panel where we can't control the backlight is better than + * no panel. + */ if (ret) - return dev_err_probe(dev, ret, - "failed to register dp aux backlight\n"); + dev_warn(dev, "failed to register dp aux backlight: %d\n", ret); drm_panel_add(&panel->base);
If we're using the AUX channel for eDP backlight and it fails to probe for some reason, let's _not_ fail the panel probe. At least one case where we could fail to init the backlight is because of a dead or physically missing panel. As talked about in detail in the earlier patch in this series, ("drm/panel-edp: If we fail to powerup/get EDID, use conservative timings"), this can cause the entire system's display pipeline to fail to come up and that's non-ideal. If we fail to init the backlight for some transitory reason, we should dig in and see if there's a way to fix this (perhaps retries?). Even in that case, though, having a panel whose backlight is stuck at 100% (the default, at least in the panel Samsung ATNA33XC20 I tested) is better than having no panel at all. Signed-off-by: Douglas Anderson <dianders@chromium.org> --- If needed, I could split this into two patches: one for each of the two panels that use drm_panel_dp_aux_backlight(). Since they both go through drm-misc, though, it doesn't feel worth it. drivers/gpu/drm/panel/panel-edp.c | 8 +++++++- drivers/gpu/drm/panel/panel-samsung-atna33xc20.c | 9 +++++++-- 2 files changed, 14 insertions(+), 3 deletions(-)