linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/dp_mst: Fix return code on sideband message failure
@ 2021-06-29 20:08 Kuogee Hsieh
  2021-06-29 20:41 ` Stephen Boyd
  0 siblings, 1 reply; 3+ messages in thread
From: Kuogee Hsieh @ 2021-06-29 20:08 UTC (permalink / raw)
  To: robdclark, sean, swboyd
  Cc: abhinavk, aravindh, khsieh, rsubbia, rnayak, freedreno, airlied,
	daniel, maarten.lankhorst, mripard, tzimmermann, dri-devel,
	linux-kernel

From: Rajkumar Subbiah <rsubbia@codeaurora.org>

The commit 2f015ec6eab69301fdcf54d397810d72362d7223 added some debug
code for sideband message tracing. But it seems to have unintentionally
changed the behavior on sideband message failure. It catches and returns
failure only if DRM_UT_DP is enabled. Otherwise it ignores the error code
and returns success. So on an MST unplug, the caller is unaware that the
clear payload message failed and ends up waiting for 4 seconds for the
response.

This change fixes the issue by returning the proper error code.

Change-Id: I2887b7ca21355fe84a7968f7619d5e8199cbb0c6
Signed-off-by: Rajkumar Subbiah <rsubbia@codeaurora.org>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 1590144..8d97430 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2887,11 +2887,13 @@ static int process_single_tx_qlock(struct drm_dp_mst_topology_mgr *mgr,
 	idx += tosend + 1;
 
 	ret = drm_dp_send_sideband_msg(mgr, up, chunk, idx);
-	if (unlikely(ret) && drm_debug_enabled(DRM_UT_DP)) {
-		struct drm_printer p = drm_debug_printer(DBG_PREFIX);
+	if (unlikely(ret)) {
+		if (drm_debug_enabled(DRM_UT_DP)) {
+			struct drm_printer p = drm_debug_printer(DBG_PREFIX);
 
-		drm_printf(&p, "sideband msg failed to send\n");
-		drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
+			drm_printf(&p, "sideband msg failed to send\n");
+			drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
+		}
 		return ret;
 	}
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/dp_mst: Fix return code on sideband message failure
  2021-06-29 20:08 [PATCH] drm/dp_mst: Fix return code on sideband message failure Kuogee Hsieh
@ 2021-06-29 20:41 ` Stephen Boyd
  2021-06-29 21:38   ` Lyude Paul
  0 siblings, 1 reply; 3+ messages in thread
From: Stephen Boyd @ 2021-06-29 20:41 UTC (permalink / raw)
  To: Kuogee Hsieh, robdclark, sean, Lyude Paul
  Cc: abhinavk, aravindh, rsubbia, rnayak, freedreno, airlied, daniel,
	maarten.lankhorst, mripard, tzimmermann, dri-devel, linux-kernel

+Lyude, author of fixed commit. Please add relevant folks in the future.

Quoting Kuogee Hsieh (2021-06-29 13:08:56)
> From: Rajkumar Subbiah <rsubbia@codeaurora.org>
>
> The commit 2f015ec6eab69301fdcf54d397810d72362d7223 added some debug

Please write

Commit 2f015ec6eab6 ("drm/dp_mst: Add sideband down request tracing +
selftests") added some debug

> code for sideband message tracing. But it seems to have unintentionally
> changed the behavior on sideband message failure. It catches and returns
> failure only if DRM_UT_DP is enabled. Otherwise it ignores the error code
> and returns success. So on an MST unplug, the caller is unaware that the
> clear payload message failed and ends up waiting for 4 seconds for the
> response.
>
> This change fixes the issue by returning the proper error code.

$ git grep "This patch" -- Documentation/process

>
> Change-Id: I2887b7ca21355fe84a7968f7619d5e8199cbb0c6

Please replace with

Fixes: 2f015ec6eab6 ("drm/dp_mst: Add sideband down request tracing +
selftests")

> Signed-off-by: Rajkumar Subbiah <rsubbia@codeaurora.org>

Should be a Signed-off-by from Kuogee Hsieh here.

> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 1590144..8d97430 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -2887,11 +2887,13 @@ static int process_single_tx_qlock(struct drm_dp_mst_topology_mgr *mgr,
>         idx += tosend + 1;
>
>         ret = drm_dp_send_sideband_msg(mgr, up, chunk, idx);
> -       if (unlikely(ret) && drm_debug_enabled(DRM_UT_DP)) {
> -               struct drm_printer p = drm_debug_printer(DBG_PREFIX);
> +       if (unlikely(ret)) {
> +               if (drm_debug_enabled(DRM_UT_DP)) {
> +                       struct drm_printer p = drm_debug_printer(DBG_PREFIX);
>
> -               drm_printf(&p, "sideband msg failed to send\n");
> -               drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
> +                       drm_printf(&p, "sideband msg failed to send\n");
> +                       drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
> +               }
>                 return ret;
>         }
>

With the above fixed up

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/dp_mst: Fix return code on sideband message failure
  2021-06-29 20:41 ` Stephen Boyd
@ 2021-06-29 21:38   ` Lyude Paul
  0 siblings, 0 replies; 3+ messages in thread
From: Lyude Paul @ 2021-06-29 21:38 UTC (permalink / raw)
  To: Stephen Boyd, Kuogee Hsieh, robdclark, sean
  Cc: abhinavk, aravindh, rsubbia, rnayak, freedreno, airlied, daniel,
	maarten.lankhorst, mripard, tzimmermann, dri-devel, linux-kernel

JFYI this is on my radar and I'll take a look asap

On Tue, 2021-06-29 at 13:41 -0700, Stephen Boyd wrote:
> +Lyude, author of fixed commit. Please add relevant folks in the future.
> 
> Quoting Kuogee Hsieh (2021-06-29 13:08:56)
> > From: Rajkumar Subbiah <rsubbia@codeaurora.org>
> > 
> > The commit 2f015ec6eab69301fdcf54d397810d72362d7223 added some debug
> 
> Please write
> 
> Commit 2f015ec6eab6 ("drm/dp_mst: Add sideband down request tracing +
> selftests") added some debug
> 
> > code for sideband message tracing. But it seems to have unintentionally
> > changed the behavior on sideband message failure. It catches and returns
> > failure only if DRM_UT_DP is enabled. Otherwise it ignores the error code
> > and returns success. So on an MST unplug, the caller is unaware that the
> > clear payload message failed and ends up waiting for 4 seconds for the
> > response.
> > 
> > This change fixes the issue by returning the proper error code.
> 
> $ git grep "This patch" -- Documentation/process
> 
> > 
> > Change-Id: I2887b7ca21355fe84a7968f7619d5e8199cbb0c6
> 
> Please replace with
> 
> Fixes: 2f015ec6eab6 ("drm/dp_mst: Add sideband down request tracing +
> selftests")
> 
> > Signed-off-by: Rajkumar Subbiah <rsubbia@codeaurora.org>
> 
> Should be a Signed-off-by from Kuogee Hsieh here.
> 
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 1590144..8d97430 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -2887,11 +2887,13 @@ static int process_single_tx_qlock(struct
> > drm_dp_mst_topology_mgr *mgr,
> >         idx += tosend + 1;
> > 
> >         ret = drm_dp_send_sideband_msg(mgr, up, chunk, idx);
> > -       if (unlikely(ret) && drm_debug_enabled(DRM_UT_DP)) {
> > -               struct drm_printer p = drm_debug_printer(DBG_PREFIX);
> > +       if (unlikely(ret)) {
> > +               if (drm_debug_enabled(DRM_UT_DP)) {
> > +                       struct drm_printer p =
> > drm_debug_printer(DBG_PREFIX);
> > 
> > -               drm_printf(&p, "sideband msg failed to send\n");
> > -               drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
> > +                       drm_printf(&p, "sideband msg failed to send\n");
> > +                       drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
> > +               }
> >                 return ret;
> >         }
> > 
> 
> With the above fixed up
> 
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> 

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-06-29 21:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-29 20:08 [PATCH] drm/dp_mst: Fix return code on sideband message failure Kuogee Hsieh
2021-06-29 20:41 ` Stephen Boyd
2021-06-29 21:38   ` Lyude Paul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).