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

[Daniel Martin]

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

>                 port->mstb = NULL;
> +               drm_dp_put_mst_branch_device(mstb);
>                 break;
>         }
>  }
> --
> 2.1.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel