| Message ID | 20210416153909.v4.23.Ibd31b8f7c73255d68c5c9f5b611b4bfaa036f727@changeid |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm: Fix EDID reading on ti-sn65dsi86; solve some chicken-and-egg problems | expand |
On Fri 16 Apr 17:39 CDT 2021, Douglas Anderson wrote: > I don't believe that it ever makes sense to read the EDID when a panel > is not powered and the powering on of the panel is the job of > prepare(). Let's make sure that this happens before we try to read the > EDID. We use the pm_runtime functions directly rather than directly > calling the normal prepare() function because the pm_runtime functions > are definitely refcounted whereas it's less clear if the prepare() one > is. > > NOTE: I'm not 100% sure how EDID reading was working for folks in the > past, but I can only assume that it was failing on the initial attempt > and then working only later. This patch, presumably, will fix that. If > some panel out there really can read the EDID without powering up and > it's a big advantage to preserve the old behavior we can add a > per-panel flag. It appears that providing the DDC bus to the panel in > the past was somewhat uncommon in any case. > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > > (no changes since v1) > > drivers/gpu/drm/panel/panel-simple.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > index c91e8aa108f7..40382c1be692 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -510,12 +510,18 @@ static int panel_simple_get_modes(struct drm_panel *panel, > > /* probe EDID if a DDC bus is available */ > if (p->ddc) { > - struct edid *edid = drm_get_edid(connector, p->ddc); > + struct edid *edid; > > + pm_runtime_get_sync(panel->dev); > + > + edid = drm_get_edid(connector, p->ddc); > if (edid) { > num += drm_add_edid_modes(connector, edid); > kfree(edid); > } > + > + pm_runtime_mark_last_busy(panel->dev); > + pm_runtime_put_autosuspend(panel->dev); > } > > /* add hard-coded panel modes */ > -- > 2.31.1.368.gbe11c130af-goog >
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index c91e8aa108f7..40382c1be692 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -510,12 +510,18 @@ static int panel_simple_get_modes(struct drm_panel *panel, /* probe EDID if a DDC bus is available */ if (p->ddc) { - struct edid *edid = drm_get_edid(connector, p->ddc); + struct edid *edid; + pm_runtime_get_sync(panel->dev); + + edid = drm_get_edid(connector, p->ddc); if (edid) { num += drm_add_edid_modes(connector, edid); kfree(edid); } + + pm_runtime_mark_last_busy(panel->dev); + pm_runtime_put_autosuspend(panel->dev); } /* add hard-coded panel modes */
I don't believe that it ever makes sense to read the EDID when a panel is not powered and the powering on of the panel is the job of prepare(). Let's make sure that this happens before we try to read the EDID. We use the pm_runtime functions directly rather than directly calling the normal prepare() function because the pm_runtime functions are definitely refcounted whereas it's less clear if the prepare() one is. NOTE: I'm not 100% sure how EDID reading was working for folks in the past, but I can only assume that it was failing on the initial attempt and then working only later. This patch, presumably, will fix that. If some panel out there really can read the EDID without powering up and it's a big advantage to preserve the old behavior we can add a per-panel flag. It appears that providing the DDC bus to the panel in the past was somewhat uncommon in any case. Signed-off-by: Douglas Anderson <dianders@chromium.org> --- (no changes since v1) drivers/gpu/drm/panel/panel-simple.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)