linux-kernel - Re: [PATCH] drm: Document that power requirements for DP AUX transfers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 4 May 2022 09:12:51 +0300
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     Douglas Anderson <dianders@...omium.org>,
        dri-devel@...ts.freedesktop.org
Cc:     Lyude Paul <lyude@...hat.com>,
        Daniel Vetter <daniel.vetter@...ll.ch>,
        Maxime Ripard <maxime@...no.tech>,
        linux-arm-msm@...r.kernel.org, freedreno@...ts.freedesktop.org,
        Stephen Boyd <swboyd@...omium.org>,
        Robert Foss <robert.foss@...aro.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Hsin-Yi Wang <hsinyi@...omium.org>,
        Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...ux.ie>,
        Imre Deak <imre.deak@...el.com>,
        Jani Nikula <jani.nikula@...el.com>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Ville Syrjälä <ville.syrjala@...ux.intel.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm: Document that power requirements for DP AUX
 transfers

On 04/05/2022 02:21, Douglas Anderson wrote:
> 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.

Nit: sink

> 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.

then

> 
> 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@...omium.org>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.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);


-- 
With best wishes
Dmitry