| Message ID | 20210423095743.v5.18.If050957eaa85cf45b10bcf61e6f7fa61c9750ebf@changeid |
|---|---|
| State | Accepted |
| Commit | 63358e24ee79e8f43dadb755cbe8e955230c03a1 |
| Headers | show |
| Series | drm: Fix EDID reading on ti-sn65dsi86; solve some chicken-and-egg problems | expand |
On Fri, Apr 23, 2021 at 1:00 PM Douglas Anderson <dianders@chromium.org> wrote: > > It doesn't make sense to go out to the bus and read the EDID over and > over again. Let's cache it and throw away the cache when we turn power > off from the panel. Autosuspend means that even if there are several > calls to read the EDID before we officially turn the power on then we > should get good use out of this cache. > I think i915 caches the edid once on init and never refreshes it (assuming no hotplugs). That said, I think it makes sense for a more conservative approach in panel-simple. > Signed-off-by: Douglas Anderson <dianders@chromium.org> > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > > (no changes since v1) > > drivers/gpu/drm/panel/panel-simple.c | 17 ++++++++++------- > 1 file changed, 10 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > index a12dfe8b8d90..9be050ab372f 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -189,6 +189,8 @@ struct panel_simple { > struct gpio_desc *enable_gpio; > struct gpio_desc *hpd_gpio; > > + struct edid *edid; > + > struct drm_display_mode override_mode; > > enum drm_panel_orientation orientation; > @@ -345,6 +347,9 @@ static int panel_simple_suspend(struct device *dev) > regulator_disable(p->supply); > p->unprepared_time = ktime_get(); > > + kfree(p->edid); > + p->edid = NULL; > + > return 0; > } > > @@ -510,15 +515,13 @@ static int panel_simple_get_modes(struct drm_panel *panel, > > /* probe EDID if a DDC bus is available */ > if (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); > - } > + if (!p->edid) > + p->edid = drm_get_edid(connector, p->ddc); > + > + if (p->edid) > + num += drm_add_edid_modes(connector, p->edid); I suppose this would keep banging on the ddc if drm_get_edid() continuously returns NULL, but maybe that's desireable (it'll succeed sometime in the future)? At any rate, this is an improvement on current behavior so it has my vote. Reviewed-by: Sean Paul <seanpaul@chromium.org> > > pm_runtime_mark_last_busy(panel->dev); > pm_runtime_put_autosuspend(panel->dev); > -- > 2.31.1.498.g6c1eba8ee3d-goog > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index a12dfe8b8d90..9be050ab372f 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -189,6 +189,8 @@ struct panel_simple { struct gpio_desc *enable_gpio; struct gpio_desc *hpd_gpio; + struct edid *edid; + struct drm_display_mode override_mode; enum drm_panel_orientation orientation; @@ -345,6 +347,9 @@ static int panel_simple_suspend(struct device *dev) regulator_disable(p->supply); p->unprepared_time = ktime_get(); + kfree(p->edid); + p->edid = NULL; + return 0; } @@ -510,15 +515,13 @@ static int panel_simple_get_modes(struct drm_panel *panel, /* probe EDID if a DDC bus is available */ if (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); - } + if (!p->edid) + p->edid = drm_get_edid(connector, p->ddc); + + if (p->edid) + num += drm_add_edid_modes(connector, p->edid); pm_runtime_mark_last_busy(panel->dev); pm_runtime_put_autosuspend(panel->dev);