-----Original Message----- From: Lyude Paul lyude@redhat.com Sent: Saturday, December 7, 2019 3:57 AM To: Lin, Wayne Wayne.Lin@amd.com; dri-devel@lists.freedesktop.org; amd-gfx@lists.freedesktop.org Cc: Kazlauskas, Nicholas Nicholas.Kazlauskas@amd.com; Wentland, Harry Harry.Wentland@amd.com; Zuo, Jerry Jerry.Zuo@amd.com; stable@vger.kernel.org Subject: Re: [PATCH v2] drm/dp_mst: Remove VCPI while disabling topology mgr
On Fri, 2019-12-06 at 14:24 -0500, Lyude Paul wrote:
Reviewed-by: Lyude Paul lyude@redhat.com
I'll go ahead and push this to drm-misc-next-fixes right now, thanks!
Whoops-meant to say drm-misc-next here, anyway, pushed!
Thanks for your time!
On Thu, 2019-12-05 at 17:00 +0800, Wayne Lin wrote:
[Why]
This patch is trying to address the issue observed when hotplug DP daisy chain monitors.
e.g. src-mstb-mstb-sst -> src (unplug) mstb-mstb-sst -> src-mstb-mstb-sst (plug in again)
Once unplug a DP MST capable device, driver will call drm_dp_mst_topology_mgr_set_mst() to disable MST. In this function, it cleans data of topology manager while disabling mst_state. However, it doesn't clean up the proposed_vcpis of topology manager. If proposed_vcpi is not reset, once plug in MST daisy chain monitors later, code will fail at checking port validation while trying to allocate payloads.
When MST capable device is plugged in again and try to allocate payloads by calling drm_dp_update_payload_part1(), this function will iterate over all proposed virtual channels to see if any proposed VCPI's num_slots is greater than 0. If any proposed VCPI's num_slots is greater than 0 and the port which the specific virtual channel directed to is not in the topology, code then fails at the port validation. Since there are stale VCPI allocations from the previous topology enablement in proposed_vcpi[], code will fail at port validation and reurn EINVAL.
[How]
Clean up the data of stale proposed_vcpi[] and reset mgr->proposed_vcpis to NULL while disabling mst in
drm_dp_mst_topology_mgr_set_mst().
Changes since v1: *Add on more details in commit message to describe the issue which the patch is trying to fix
Signed-off-by: Wayne Lin Wayne.Lin@amd.com
drivers/gpu/drm/drm_dp_mst_topology.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index ae5809a1f19a..bf4f745a4edb 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -3386,6 +3386,7 @@ static int drm_dp_get_vc_payload_bw(u8 dp_link_bw, u8 dp_link_count) int drm_dp_mst_topology_mgr_set_mst(struct
drm_dp_mst_topology_mgr
*mgr, bool mst_state) { int ret = 0;
int i = 0; struct drm_dp_mst_branch *mstb = NULL;
mutex_lock(&mgr->lock);
@@ -3446,10 +3447,21 @@ int
drm_dp_mst_topology_mgr_set_mst(struct
drm_dp_mst_topology_mgr *mgr, bool ms /* this can fail if the device is gone */ drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL, 0); ret = 0;
mutex_lock(&mgr->payload_lock);drm_dp_payload)); mgr->payload_mask = 0; set_bit(0, &mgr->payload_mask);
for (i = 0; i < mgr->max_payloads; i++) {struct drm_dp_vcpi *vcpi = mgr->proposed_vcpis[i];if (vcpi) {vcpi->vcpi = 0;vcpi->num_slots = 0;}mgr->proposed_vcpis[i] = NULL;}mutex_unlock(&mgr->payload_lock);out_unlock:
-- Cheers, Lyude Paul
-- Regards, Wayne Lin