[Intel-gfx] [PATCH] drm/dp-mst: Remove branches before dropping the reference

[David Airlie]

> On 8 December 2014 at 22:55, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> > When we unplug a dp mst branch we unreference the entire tree from
> > the root towards the leaves. Which is ok, since that's the way the
> > pointers and so also the refcounts go.
> >
> > But when we drop the reference we must make sure that we remove the
> > branches/ports from the lists/pointers before dropping the reference.
> > Otherwise the get_validated functions will still return it instead
> > of returning NULL (which indicates a potentially on-going unplug).
> >
> > The mst branch destroy gets this right for ports: First it deletes
> > the port from the ports list, then it unrefs. But the ports destroy
> > function gets it wrong: First it unrefs, then it drops the ref. Which
> > means a zombie mst branch can still be validate with get_validated_mstb_ref
> > when it shouldn't.
> >
> > Fix this.
> >
> > This should address a backtrace Dave dug out somewhere on unplug:
> >
> >  [<ffffffffa00cc262>] drm_dp_mst_get_validated_mstb_ref_locked+0x92/0xa0
> >  [drm_kms_helper]
> >  [<ffffffffa00cc211>] drm_dp_mst_get_validated_mstb_ref_locked+0x41/0xa0
> >  [drm_kms_helper]
> >  [<ffffffffa00cc2aa>] drm_dp_get_validated_mstb_ref+0x3a/0x60
> >  [drm_kms_helper]
> >  [<ffffffffa00cc2fb>] drm_dp_payload_send_msg.isra.14+0x2b/0x100
> >  [drm_kms_helper]
> >  [<ffffffffa00cc547>] drm_dp_update_payload_part1+0x177/0x360
> >  [drm_kms_helper]
> >  [<ffffffffa015c52e>] intel_mst_disable_dp+0x3e/0x80 [i915]
> >  [<ffffffffa013d60b>] haswell_crtc_disable+0x1cb/0x340 [i915]
> >  [<ffffffffa0136739>] intel_crtc_control+0x49/0x100 [i915]
> >  [<ffffffffa0136857>] intel_crtc_update_dpms+0x67/0x80 [i915]
> >  [<ffffffffa013fa59>] intel_connector_dpms+0x59/0x70 [i915]
> >  [<ffffffffa015c752>] intel_dp_destroy_mst_connector+0x32/0xc0 [i915]
> >  [<ffffffffa00cb44b>] drm_dp_destroy_port+0x6b/0xa0 [drm_kms_helper]
> >  [<ffffffffa00cb588>] drm_dp_destroy_mst_branch_device+0x108/0x130
> >  [drm_kms_helper]
> >  [<ffffffffa00cb3cd>] drm_dp_port_teardown_pdt+0x3d/0x50 [drm_kms_helper]
> >  [<ffffffffa00cdb79>] drm_dp_mst_handle_up_req+0x499/0x540 [drm_kms_helper]
> >  [<ffffffff810d9ead>] ? trace_hardirqs_on_caller+0x15d/0x200
> >  [<ffffffffa00cdc73>]
> >  drm_dp_mst_hpd_irq+0x53/0xa00 [drm_kms_helper] [<ffffffffa00c7dfb>]
> >  ? drm_dp_dpcd_read+0x1b/0x20 [drm_kms_helper] [<ffffffffa0153ed8>]
> >  ? intel_dp_dpcd_read_wake+0x38/0x70 [i915] [<ffffffffa015a225>]
> >  intel_dp_check_mst_status+0xb5/0x250 [i915] [<ffffffffa015ac71>]
> >  intel_dp_hpd_pulse+0x181/0x210 [i915] [<ffffffffa01104f6>]
> >  i915_digport_work_func+0x96/0x120 [i915]
> >
> > Cc: Dave Airlie <airlied at redhat.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 5682d7e9f1ec..71a56d65a0d2 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -839,6 +839,8 @@ static void drm_dp_put_mst_branch_device(struct
> > drm_dp_mst_branch *mstb)
> >
> >  static void drm_dp_port_teardown_pdt(struct drm_dp_mst_port *port, int
> >  old_pdt)
> >  {
> > +       struct drm_dp_mst_branch *mstb;
> > +
> >         switch (old_pdt) {
> >         case DP_PEER_DEVICE_DP_LEGACY_CONV:
> >         case DP_PEER_DEVICE_SST_SINK:
> > @@ -846,8 +848,9 @@ static void drm_dp_port_teardown_pdt(struct
> > drm_dp_mst_port *port, int old_pdt)
> >                 drm_dp_mst_unregister_i2c_bus(&port->aux);
> >                 break;
> >         case DP_PEER_DEVICE_MST_BRANCHING:
> > -               drm_dp_put_mst_branch_device(port->mstb);
> > +               mstb = port_mstb;
> 
> drivers/gpu/drm/drm_dp_mst_topology.c:851:10: error: ‘port_mstb’ undeclared
> 
> This needs to be port->mstb. Spotted while testing this for
>     https://bugs.freedesktop.org/show_bug.cgi?id=87099

The version I checked in fixed this.

Dave.