| 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
| ||
|
Message-ID: <CABXOdTcfb9F8mKSNnQKWRCAgANyJMNn1yzk4n_E_MpUgH23KCw@mail.gmail.com>
Date: Fri, 15 Mar 2024 15:19:07 -0700
From: Guenter Roeck <groeck@...gle.com>
To: Douglas Anderson <dianders@...omium.org>
Cc: Rob Clark <robdclark@...il.com>, Abhinav Kumar <quic_abhinavk@...cinc.com>,
Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, Bjorn Andersson <quic_bjorande@...cinc.com>,
Daniel Vetter <daniel@...ll.ch>, David Airlie <airlied@...il.com>, Guenter Roeck <groeck@...omium.org>,
Kuogee Hsieh <quic_khsieh@...cinc.com>, Marijn Suijten <marijn.suijten@...ainline.org>,
Sean Paul <sean@...rly.run>, Stephen Boyd <swboyd@...omium.org>,
Vara Reddy <quic_varar@...cinc.com>, dri-devel@...ts.freedesktop.org,
freedreno@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/4] drm/msm/dp: Avoid a long timeout for AUX transfer
if nothing connected
On Fri, Mar 15, 2024 at 2:37 PM Douglas Anderson <dianders@...omiumorg> wrote:
>
> As documented in the description of the transfer() function of
> "struct drm_dp_aux", the transfer() function can be called at any time
> regardless of the state of the DP port. Specifically if the kernel has
> the DP AUX character device enabled and userspace accesses
> "/dev/drm_dp_auxN" directly then the AUX transfer function will be
> called regardless of whether a DP device is connected.
>
> For eDP panels we have a special rule where we wait (with a 5 second
> timeout) for HPD to go high. This rule was important before all panels
> drivers were converted to call wait_hpd_asserted() and actually can be
> removed in a future commit.
>
> For external DP devices we never checked for HPD. That means that
> trying to access the DP AUX character device (AKA `hexdump -C
> /dev/drm_dp_auxN`) would very, very slowly timeout. Specifically on my
> system:
> $ time hexdump -C /dev/drm_dp_aux0
> hexdump: /dev/drm_dp_aux0: Connection timed out
> real 0m8.200s
> We want access to the drm_dp_auxN character device to fail faster than
> 8 seconds when no DP cable is plugged in.
>
> Let's add a test to make transfers fail right away if a device isn't
> plugged in. Rather than testing the HPD line directly, we have the
> dp_display module tell us when AUX transfers should be enabled so we
> can handle cases where HPD is signaled out of band like with Type C.
>
> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
> Signed-off-by: Douglas Anderson <dianders@...omium.org>
Reviewed-by: Guenter Roeck <groeck@...omium.org>
> ---
>
> Changes in v2:
> - Don't look at the HPD line directly; have dp_display call us.
>
> drivers/gpu/drm/msm/dp/dp_aux.c | 20 ++++++++++++++++++++
> drivers/gpu/drm/msm/dp/dp_aux.h | 1 +
> drivers/gpu/drm/msm/dp/dp_display.c | 4 ++++
> 3 files changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c
> index 03f4951c49f4..e67a80d56948 100644
> --- a/drivers/gpu/drm/msm/dp/dp_aux.c
> +++ b/drivers/gpu/drm/msm/dp/dp_aux.c
> @@ -35,6 +35,7 @@ struct dp_aux_private {
> bool no_send_stop;
> bool initted;
> bool is_edp;
> + bool enable_xfers;
> u32 offset;
> u32 segment;
>
> @@ -301,6 +302,17 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux,
> goto exit;
> }
>
> + /*
> + * If we're using DP and an external display isn't connected then the
> + * transfer won't succeed. Return right away. If we don't do this we
> + * can end up with long timeouts if someone tries to access the DP AUX
> + * character device when no DP device is connected.
> + */
> + if (!aux->is_edp && !aux->enable_xfers) {
> + ret = -ENXIO;
> + goto exit;
> + }
> +
> /*
> * For eDP it's important to give a reasonably long wait here for HPD
> * to be asserted. This is because the panel driver may have _just_
> @@ -433,6 +445,14 @@ irqreturn_t dp_aux_isr(struct drm_dp_aux *dp_aux)
> return IRQ_HANDLED;
> }
>
> +void dp_aux_enable_xfers(struct drm_dp_aux *dp_aux, bool enabled)
> +{
> + struct dp_aux_private *aux;
> +
> + aux = container_of(dp_aux, struct dp_aux_private, dp_aux);
> + aux->enable_xfers = enabled;
> +}
> +
> void dp_aux_reconfig(struct drm_dp_aux *dp_aux)
> {
> struct dp_aux_private *aux;
> diff --git a/drivers/gpu/drm/msm/dp/dp_aux.h b/drivers/gpu/drm/msm/dp/dp_aux.h
> index 511305da4f66..f3052cb43306 100644
> --- a/drivers/gpu/drm/msm/dp/dp_aux.h
> +++ b/drivers/gpu/drm/msm/dp/dp_aux.h
> @@ -12,6 +12,7 @@
> int dp_aux_register(struct drm_dp_aux *dp_aux);
> void dp_aux_unregister(struct drm_dp_aux *dp_aux);
> irqreturn_t dp_aux_isr(struct drm_dp_aux *dp_aux);
> +void dp_aux_enable_xfers(struct drm_dp_aux *dp_aux, bool enabled);
> void dp_aux_init(struct drm_dp_aux *dp_aux);
> void dp_aux_deinit(struct drm_dp_aux *dp_aux);
> void dp_aux_reconfig(struct drm_dp_aux *dp_aux);
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 4c72124ffb5d..b0f3e2ef5a6b 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -565,6 +565,8 @@ static int dp_hpd_plug_handle(struct dp_display_private *dp, u32 data)
> int ret;
> struct platform_device *pdev = dp->dp_display.pdev;
>
> + dp_aux_enable_xfers(dp->aux, true);
> +
> mutex_lock(&dp->event_mutex);
>
> state = dp->hpd_state;
> @@ -629,6 +631,8 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data)
> u32 state;
> struct platform_device *pdev = dp->dp_display.pdev;
>
> + dp_aux_enable_xfers(dp->aux, false);
> +
> mutex_lock(&dp->event_mutex);
>
> state = dp->hpd_state;
> --
> 2.44.0.291.gc1ea87d7ee-goog
>
Powered by blists - more mailing lists