From: Sean Paul <seanpaul@chromium.org> To: Douglas Anderson <dianders@chromium.org> Cc: Andrzej Hajda <a.hajda@samsung.com>, Neil Armstrong <narmstrong@baylibre.com>, Laurent Pinchart <Laurent.pinchart@ideasonboard.com>, Jonas Karlman <jonas@kwiboo.se>, Jernej Skrabec <jernej.skrabec@siol.net>, Sam Ravnborg <sam@ravnborg.org>, Wolfram Sang <wsa@kernel.org>, robdclark@chromium.org, dri-devel <dri-devel@lists.freedesktop.org>, David Airlie <airlied@linux.ie>, linux-arm-msm <linux-arm-msm@vger.kernel.org>, Steev Klimaszewski <steev@kali.org>, Stephen Boyd <swboyd@chromium.org>, Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>, Thierry Reding <thierry.reding@gmail.com>, linux-i2c@vger.kernel.org, Bjorn Andersson <bjorn.andersson@linaro.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v5 17/20] drm/panel: panel-simple: Power the panel when reading the EDID Date: Wed, 28 Apr 2021 12:44:45 -0400 [thread overview] Message-ID: <CAOw6vbLqDgqhXH8pUZqfhKCZk68SfzLf76Z4au3g83=Uw_8z2Q@mail.gmail.com> (raw) In-Reply-To: <20210423095743.v5.17.Ibd31b8f7c73255d68c5c9f5b611b4bfaa036f727@changeid> On Fri, Apr 23, 2021 at 1:00 PM Douglas Anderson <dianders@chromium.org> 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@chromium.org> > 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 | 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: Sean Paul <seanpaul@chromium.org> To: Douglas Anderson <dianders@chromium.org> Cc: robdclark@chromium.org, Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>, Jernej Skrabec <jernej.skrabec@siol.net>, Jonas Karlman <jonas@kwiboo.se>, David Airlie <airlied@linux.ie>, linux-arm-msm <linux-arm-msm@vger.kernel.org>, Neil Armstrong <narmstrong@baylibre.com>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, dri-devel <dri-devel@lists.freedesktop.org>, Stephen Boyd <swboyd@chromium.org>, Wolfram Sang <wsa@kernel.org>, Andrzej Hajda <a.hajda@samsung.com>, Thierry Reding <thierry.reding@gmail.com>, Laurent Pinchart <Laurent.pinchart@ideasonboard.com>, Bjorn Andersson <bjorn.andersson@linaro.org>, Sam Ravnborg <sam@ravnborg.org>, Steev Klimaszewski <steev@kali.org>, linux-i2c@vger.kernel.org Subject: Re: [PATCH v5 17/20] drm/panel: panel-simple: Power the panel when reading the EDID Date: Wed, 28 Apr 2021 12:44:45 -0400 [thread overview] Message-ID: <CAOw6vbLqDgqhXH8pUZqfhKCZk68SfzLf76Z4au3g83=Uw_8z2Q@mail.gmail.com> (raw) In-Reply-To: <20210423095743.v5.17.Ibd31b8f7c73255d68c5c9f5b611b4bfaa036f727@changeid> On Fri, Apr 23, 2021 at 1:00 PM Douglas Anderson <dianders@chromium.org> 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@chromium.org> > 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 | 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2021-04-28 16:45 UTC|newest] Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-04-23 16:58 [PATCH v5 00/20] drm: Fix EDID reading on ti-sn65dsi86; solve some chicken-and-egg problems Douglas Anderson 2021-04-23 16:58 ` Douglas Anderson 2021-04-23 16:58 ` [PATCH v5 01/20] drm/panel: panel-simple: Add missing pm_runtime_disable() calls Douglas Anderson 2021-04-23 16:58 ` Douglas Anderson 2021-04-28 16:32 ` Sean Paul 2021-04-28 16:32 ` Sean Paul 2021-04-30 0:58 ` Linus Walleij 2021-04-30 0:58 ` Linus Walleij 2021-04-30 1:24 ` Doug Anderson 2021-04-30 1:24 ` Doug Anderson 2021-04-30 1:27 ` Linus Walleij 2021-04-30 1:27 ` Linus Walleij 2021-04-30 21:04 ` Doug Anderson 2021-04-30 21:04 ` Doug Anderson 2021-05-01 12:07 ` Linus Walleij 2021-05-01 12:07 ` Linus Walleij 2021-05-03 20:41 ` Doug Anderson 2021-05-03 20:41 ` Doug Anderson 2021-05-05 12:51 ` Linus Walleij 2021-05-05 12:51 ` Linus Walleij 2021-04-23 16:58 ` [PATCH v5 02/20] drm/bridge: ti-sn65dsi86: Rename the main driver data structure Douglas Anderson 2021-04-23 16:58 ` Douglas Anderson 2021-04-23 16:58 ` [PATCH v5 03/20] drm/bridge: ti-sn65dsi86: More renames in prep for sub-devices Douglas Anderson 2021-04-23 16:58 ` Douglas Anderson 2021-04-23 16:58 ` [PATCH v5 04/20] drm/bridge: ti-sn65dsi86: Use devm to do our runtime_disable Douglas Anderson 2021-04-23 16:58 ` Douglas Anderson 2021-04-23 16:58 ` [PATCH v5 05/20] drm/bridge: ti-sn65dsi86: Clean debugfs code Douglas Anderson 2021-04-23 16:58 ` Douglas Anderson 2021-04-23 16:58 ` [PATCH v5 06/20] drm/bridge: ti-sn65dsi86: Add local var for "dev" to simplify probe Douglas Anderson 2021-04-23 16:58 ` Douglas Anderson 2021-04-23 16:58 ` [PATCH v5 07/20] drm/bridge: ti-sn65dsi86: Cleanup managing of drvdata Douglas Anderson 2021-04-23 16:58 ` Douglas Anderson 2021-04-23 16:58 ` [PATCH v5 08/20] drm/bridge: ti-sn65dsi86: Move all the chip-related init to the start Douglas Anderson 2021-04-23 16:58 ` Douglas Anderson 2021-04-23 16:58 ` [PATCH v5 09/20] drm/bridge: ti-sn65dsi86: Break GPIO and MIPI-to-eDP bridge into sub-drivers Douglas Anderson 2021-04-23 16:58 ` Douglas Anderson 2021-05-01 11:59 ` Linus Walleij 2021-05-01 11:59 ` Linus Walleij 2021-05-03 16:55 ` Doug Anderson 2021-05-03 16:55 ` Doug Anderson 2021-04-23 16:58 ` [PATCH v5 10/20] drm/panel: panel-simple: Get rid of hacky HPD chicken-and-egg code Douglas Anderson 2021-04-23 16:58 ` Douglas Anderson 2021-04-28 16:35 ` Sean Paul 2021-04-28 16:35 ` Sean Paul 2021-04-23 16:58 ` [PATCH v5 11/20] drm/bridge: ti-sn65dsi86: Use pm_runtime autosuspend Douglas Anderson 2021-04-23 16:58 ` Douglas Anderson 2021-04-23 16:58 ` [PATCH v5 12/20] drm/bridge: ti-sn65dsi86: Code motion of refclk management functions Douglas Anderson 2021-04-23 16:58 ` Douglas Anderson 2021-04-23 16:58 ` [PATCH v5 13/20] drm/bridge: ti-sn65dsi86: If refclk, DP AUX can happen w/out pre-enable Douglas Anderson 2021-04-23 16:58 ` Douglas Anderson 2021-04-23 16:59 ` [PATCH v5 14/20] drm/bridge: ti-sn65dsi86: Promote the AUX channel to its own sub-dev Douglas Anderson 2021-04-23 16:59 ` Douglas Anderson 2021-05-03 20:27 ` Doug Anderson 2021-05-03 20:27 ` Doug Anderson 2021-04-23 16:59 ` [PATCH v5 15/20] i2c: i2c-core-of: Fix corner case of finding adapter by node Douglas Anderson 2021-04-23 16:59 ` Douglas Anderson 2021-04-23 16:59 ` [PATCH v5 16/20] drm/panel: panel-simple: Remove extra call: drm_connector_update_edid_property() Douglas Anderson 2021-04-23 16:59 ` Douglas Anderson 2021-04-28 16:38 ` Sean Paul 2021-04-28 16:38 ` Sean Paul 2021-04-23 16:59 ` [PATCH v5 17/20] drm/panel: panel-simple: Power the panel when reading the EDID Douglas Anderson 2021-04-23 16:59 ` Douglas Anderson 2021-04-28 16:44 ` Sean Paul [this message] 2021-04-28 16:44 ` Sean Paul 2021-04-23 16:59 ` [PATCH v5 18/20] drm/panel: panel-simple: Cache the EDID as long as we retain power Douglas Anderson 2021-04-23 16:59 ` Douglas Anderson 2021-04-28 16:50 ` Sean Paul 2021-04-28 16:50 ` Sean Paul 2021-04-23 16:59 ` [PATCH v5 19/20] drm/bridge: ti-sn65dsi86: Don't read EDID blob over DDC Douglas Anderson 2021-04-23 16:59 ` Douglas Anderson 2021-04-23 16:59 ` [PATCH v5 20/20] arm64: dts: qcom: Link the panel to the bridge's DDC bus Douglas Anderson 2021-04-23 16:59 ` Douglas Anderson
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAOw6vbLqDgqhXH8pUZqfhKCZk68SfzLf76Z4au3g83=Uw_8z2Q@mail.gmail.com' \ --to=seanpaul@chromium.org \ --cc=Laurent.pinchart@ideasonboard.com \ --cc=a.hajda@samsung.com \ --cc=airlied@linux.ie \ --cc=bjorn.andersson@linaro.org \ --cc=dianders@chromium.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=jernej.skrabec@siol.net \ --cc=jonas@kwiboo.se \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-i2c@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=narmstrong@baylibre.com \ --cc=robdclark@chromium.org \ --cc=sam@ravnborg.org \ --cc=stanislav.lisovskiy@intel.com \ --cc=steev@kali.org \ --cc=swboyd@chromium.org \ --cc=thierry.reding@gmail.com \ --cc=wsa@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.