On Fri, Apr 23, 2021 at 1:00 PM Douglas Anderson <dianders@xxxxxxxxxxxx> 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. > Maybe some combination of drivers caching the EDID for panels while they're already powered and overly broad pm runtime references? At any rate, this makes sense to me, Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx> > Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx> > Reviewed-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx> > --- > > (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 4de33c929a59..a12dfe8b8d90 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.498.g6c1eba8ee3d-goog > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel