On Fri, 2019-12-06 at 14:24 -0500, Lyude Paul wrote:
> Reviewed-by: Lyude Paul <lyude@xxxxxxxxxx>
>
> 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!
>
> 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@xxxxxxx>
> > ---
> > 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
