All of lore.kernel.org
 help / color / mirror / Atom feed
From: Douglas Anderson <dianders@chromium.org>
To: dri-devel@lists.freedesktop.org
Cc: "Lyude Paul" <lyude@redhat.com>,
	"Dmitry Baryshkov" <dmitry.baryshkov@linaro.org>,
	"Daniel Vetter" <daniel.vetter@ffwll.ch>,
	"Maxime Ripard" <maxime@cerno.tech>,
	linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org,
	"Stephen Boyd" <swboyd@chromium.org>,
	"Robert Foss" <robert.foss@linaro.org>,
	"Laurent Pinchart" <Laurent.pinchart@ideasonboard.com>,
	"Hsin-Yi Wang" <hsinyi@chromium.org>,
	"Douglas Anderson" <dianders@chromium.org>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"David Airlie" <airlied@linux.ie>,
	"Imre Deak" <imre.deak@intel.com>,
	"Jani Nikula" <jani.nikula@intel.com>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH] drm: Document that power requirements for DP AUX transfers
Date: Tue,  3 May 2022 16:21:08 -0700	[thread overview]
Message-ID: <20220503162033.1.Ia8651894026707e4fa61267da944ff739610d180@changeid> (raw)

When doing DP AUX transfers there are two actors that need to be
powered in order for the DP AUX transfer to work: the DP source and
the DP sync. Commit bacbab58f09d ("drm: Mention the power state
requirement on side-channel operations") added some documentation
saying that the DP source is required to power itself up (if needed)
to do AUX transfers. However, that commit doesn't talk anything about
the DP sink.

For full fledged DP the sink isn't really a problem. It's expected
that if an external DP monitor isn't plugged in that attempting to do
AUX transfers won't work. It's also expected that if a DP monitor is
plugged in (and thus asserting HPD) that it AUX transfers will work.

When we're looking at eDP, however, things are less obvious. Let's add
some documentation about expectations. Here's what we'll say:

1. We don't expect the DP AUX transfer function to power on an eDP
panel. If an eDP panel is physically connected but powered off then it
makes sense for the transfer to fail.

2. We'll document that the official way to power on a panel is via the
bridge chain, specifically by making sure that the panel's prepare
function has been called (which is called by
panel_bridge_pre_enable()). It's already specified in the kernel doc
of drm_panel_prepare() that this is the way to power the panel on and
also that after this call "it is possible to communicate with any
integrated circuitry via a command bus."

3. We'll also document that for code running in the panel driver
itself that it is legal for the panel driver to power itself up
however it wants (it doesn't need to officially call
drm_panel_pre_enable()) and then it can do AUX bus transfers. This is
currently the way that edp-panel works when it's running atop the DP
AUX bus.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 include/drm/display/drm_dp_helper.h | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index dca40a045dd6..e5165b708a40 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -370,9 +370,17 @@ struct drm_dp_aux {
 	 * helpers assume this is the case.
 	 *
 	 * Also note that this callback can be called no matter the
-	 * state @dev is in. Drivers that need that device to be powered
-	 * to perform this operation will first need to make sure it's
-	 * been properly enabled.
+	 * state @dev is in and also no matter what state the panel is
+	 * in. It's expected:
+	 * - If the @dev providing the AUX bus is currently unpowered then
+	 *   it will power itself up for the transfer.
+	 * - If we're on eDP and the panel is not in a state where it can
+	 *   respond (it's not powered or it's in a low power state) then this
+	 *   function will return an error (but not crash). Note that if a
+	 *   panel driver is initiating a DP AUX transfer it may power itself
+	 *   up however it wants. All other code should ensure that the
+	 *   pre_enable() bridge chain (which eventually calls the panel
+	 *   prepare function) has powered the panel.
 	 */
 	ssize_t (*transfer)(struct drm_dp_aux *aux,
 			    struct drm_dp_aux_msg *msg);
-- 
2.36.0.464.gb9c8b46e94-goog


WARNING: multiple messages have this Message-ID (diff)
From: Douglas Anderson <dianders@chromium.org>
To: dri-devel@lists.freedesktop.org
Cc: Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@linux.ie>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Hsin-Yi Wang <hsinyi@chromium.org>,
	linux-kernel@vger.kernel.org,
	Douglas Anderson <dianders@chromium.org>,
	Robert Foss <robert.foss@linaro.org>,
	Stephen Boyd <swboyd@chromium.org>,
	Jani Nikula <jani.nikula@intel.com>,
	Maxime Ripard <maxime@cerno.tech>,
	linux-arm-msm@vger.kernel.org,
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	freedreno@lists.freedesktop.org,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Subject: [PATCH] drm: Document that power requirements for DP AUX transfers
Date: Tue,  3 May 2022 16:21:08 -0700	[thread overview]
Message-ID: <20220503162033.1.Ia8651894026707e4fa61267da944ff739610d180@changeid> (raw)

When doing DP AUX transfers there are two actors that need to be
powered in order for the DP AUX transfer to work: the DP source and
the DP sync. Commit bacbab58f09d ("drm: Mention the power state
requirement on side-channel operations") added some documentation
saying that the DP source is required to power itself up (if needed)
to do AUX transfers. However, that commit doesn't talk anything about
the DP sink.

For full fledged DP the sink isn't really a problem. It's expected
that if an external DP monitor isn't plugged in that attempting to do
AUX transfers won't work. It's also expected that if a DP monitor is
plugged in (and thus asserting HPD) that it AUX transfers will work.

When we're looking at eDP, however, things are less obvious. Let's add
some documentation about expectations. Here's what we'll say:

1. We don't expect the DP AUX transfer function to power on an eDP
panel. If an eDP panel is physically connected but powered off then it
makes sense for the transfer to fail.

2. We'll document that the official way to power on a panel is via the
bridge chain, specifically by making sure that the panel's prepare
function has been called (which is called by
panel_bridge_pre_enable()). It's already specified in the kernel doc
of drm_panel_prepare() that this is the way to power the panel on and
also that after this call "it is possible to communicate with any
integrated circuitry via a command bus."

3. We'll also document that for code running in the panel driver
itself that it is legal for the panel driver to power itself up
however it wants (it doesn't need to officially call
drm_panel_pre_enable()) and then it can do AUX bus transfers. This is
currently the way that edp-panel works when it's running atop the DP
AUX bus.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 include/drm/display/drm_dp_helper.h | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index dca40a045dd6..e5165b708a40 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -370,9 +370,17 @@ struct drm_dp_aux {
 	 * helpers assume this is the case.
 	 *
 	 * Also note that this callback can be called no matter the
-	 * state @dev is in. Drivers that need that device to be powered
-	 * to perform this operation will first need to make sure it's
-	 * been properly enabled.
+	 * state @dev is in and also no matter what state the panel is
+	 * in. It's expected:
+	 * - If the @dev providing the AUX bus is currently unpowered then
+	 *   it will power itself up for the transfer.
+	 * - If we're on eDP and the panel is not in a state where it can
+	 *   respond (it's not powered or it's in a low power state) then this
+	 *   function will return an error (but not crash). Note that if a
+	 *   panel driver is initiating a DP AUX transfer it may power itself
+	 *   up however it wants. All other code should ensure that the
+	 *   pre_enable() bridge chain (which eventually calls the panel
+	 *   prepare function) has powered the panel.
 	 */
 	ssize_t (*transfer)(struct drm_dp_aux *aux,
 			    struct drm_dp_aux_msg *msg);
-- 
2.36.0.464.gb9c8b46e94-goog


             reply	other threads:[~2022-05-03 23:21 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-03 23:21 Douglas Anderson [this message]
2022-05-03 23:21 ` [PATCH] drm: Document that power requirements for DP AUX transfers Douglas Anderson
2022-05-04  6:12 ` Dmitry Baryshkov
2022-05-04  6:12   ` Dmitry Baryshkov
2022-05-04 12:21 ` Ville Syrjälä
2022-05-04 12:21   ` Ville Syrjälä
2022-05-04 16:04   ` Doug Anderson
2022-05-04 16:04     ` Doug Anderson
2022-05-04 18:10     ` Lyude Paul
2022-05-04 18:10       ` Lyude Paul
2022-05-05 14:46       ` Ville Syrjälä
2022-05-05 14:46         ` Ville Syrjälä
2022-05-05 15:00         ` Doug Anderson
2022-05-05 15:00           ` Doug Anderson
2022-05-05 15:08           ` Ville Syrjälä
2022-05-05 15:08             ` Ville Syrjälä
2022-05-05 15:53             ` Doug Anderson
2022-05-05 15:53               ` Doug Anderson
2022-05-05 19:19               ` Ville Syrjälä
2022-05-05 19:19                 ` Ville Syrjälä
2022-05-05 20:12                 ` Doug Anderson
2022-05-05 20:12                   ` Doug Anderson
2022-05-05 20:09               ` Dmitry Baryshkov
2022-05-05 20:09                 ` Dmitry Baryshkov
2022-05-05 20:21                 ` Doug Anderson
2022-05-05 20:21                   ` Doug Anderson
2022-05-05 20:56                   ` Dmitry Baryshkov
2022-05-05 20:56                     ` Dmitry Baryshkov
2022-05-05 21:24                     ` Doug Anderson
2022-05-05 21:24                       ` Doug Anderson
2022-05-05 22:15                       ` Dmitry Baryshkov
2022-05-05 22:15                         ` Dmitry Baryshkov
2022-05-05 22:28                         ` Doug Anderson
2022-05-05 22:28                           ` Doug Anderson
2022-05-05 14:47       ` Doug Anderson
2022-05-05 14:47         ` Doug Anderson
2022-05-05 19:47 ` Lyude Paul
2022-05-05 19:47   ` Lyude Paul

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=20220503162033.1.Ia8651894026707e4fa61267da944ff739610d180@changeid \
    --to=dianders@chromium.org \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=airlied@linux.ie \
    --cc=daniel.vetter@ffwll.ch \
    --cc=daniel@ffwll.ch \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=hsinyi@chromium.org \
    --cc=imre.deak@intel.com \
    --cc=jani.nikula@intel.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lyude@redhat.com \
    --cc=maxime@cerno.tech \
    --cc=robert.foss@linaro.org \
    --cc=swboyd@chromium.org \
    --cc=tzimmermann@suse.de \
    --cc=ville.syrjala@linux.intel.com \
    /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: link
Be 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.