RE: [PATCH v2] drm/dp_mst: Remove VCPI while disabling topology mgr

["Lin, Wayne" <Wayne.Lin@amd.com>]

> -----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);
> > >  		memset(mgr->payloads, 0, mgr->max_payloads * sizeof(struct
> > > 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;
> > > +		}
> > >  		mgr->vcpi_mask = 0;
> > > +		mutex_unlock(&mgr->payload_lock);
> > >  	}
> > >
> > >  out_unlock:
> --
> Cheers,
> 	Lyude Paul

--
Regards,
Wayne Lin