| Message ID | 1624997336-2245-1-git-send-email-khsieh@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/dp_mst: Fix return code on sideband message failure | expand |
+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>
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> >
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; }