[PATCH RFC] drm/amd/display: Disable CRTC on MST connector unregister

[PATCH RFC] drm/amd/display: Disable CRTC on MST connector unregister

[David Medina via B4 Relay]

From: David Medina <b2amedina@gmail.com>

amdgpu_dm_mst_connector_early_unregister() releases the connector's
sink but leaves the CRTC it was driving active and its MST encoder
assigned. Because the connector is unregistered in the same step,
userspace has no chance to disable the CRTC before it is gone. When the
same display is re-plugged a new MST connector is created, and the
re-commit fails in drm_atomic_helper_check_modeset() because the stale
connector still owns the encoder (handle_conflicting_encoders), leaving
the display dark until a full modeset (e.g. a session restart) tears
everything down.

Disable the CRTC from the driver when the connector is torn down, so the
MST encoder is released and the DC stream dropped. Reuse the
drm_atomic_helper_disable_all() pattern: deactivate the CRTC, clear the
mode, and disconnect the connector.

Reproduced 100% of the time by unplugging and re-plugging the Thunderbolt
dock driving the LG ULTRAWIDE: on re-plug the display stays dark and
dmesg shows drm_atomic_helper_check_modeset() failing with an encoder
conflict (handle_conflicting_encoders). Verified over four unplug/replug
cycles with this patch applied.

I am not certain that forcing a modeset from early_unregister() is the
right approach, and the disable here is intentionally minimal: it
deactivates the CRTC and disconnects the connector but does not
add/detach the CRTC's affected planes the way
drm_atomic_helper_disable_all() does. The minimal version resolves the
failure in testing; I would welcome guidance on whether the full
affected-planes teardown (or a different location) is required.

Fixes: a1b27e99229a ("drm/amd/display: Implement MST Aux device registration")
Signed-off-by: David Medina <b2amedina@gmail.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 53 ++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 045a7f88b754..24b146068739 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -29,6 +29,7 @@
 #include <drm/display/drm_dp_mst_helper.h>
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_atomic_uapi.h>
 #include <drm/drm_fixed.h>
 #include <drm/drm_edid.h>
 #include "dm_services.h"
@@ -227,6 +228,7 @@ amdgpu_dm_mst_connector_early_unregister(struct drm_connector *connector)
 	struct amdgpu_dm_connector *root = aconnector->mst_root;
 	struct dc_link *dc_link = aconnector->dc_link;
 	struct dc_sink *dc_sink = aconnector->dc_sink;
+	struct drm_crtc *crtc = connector->state ? connector->state->crtc : NULL;
 
 	drm_dp_mst_connector_early_unregister(connector, port);
 
@@ -250,6 +252,57 @@ amdgpu_dm_mst_connector_early_unregister(struct drm_connector *connector)
 
 	aconnector->mst_status = MST_STATUS_DEFAULT;
 	drm_modeset_unlock(&root->mst_mgr.base.lock);
+
+	/*
+	 * The connector is being removed from the MST topology. If a CRTC is
+	 * still driving it, force a modeset that disables the CRTC so that the
+	 * MST encoder is released and the DC stream dropped. Otherwise, on a
+	 * subsequent re-plug the stale CRTC/encoder assignment triggers an
+	 * encoder conflict in drm_atomic_helper_check_modeset() and the display
+	 * stays dark.
+	 */
+	if (crtc) {
+		struct drm_atomic_commit *state;
+		struct drm_connector_state *conn_state;
+		struct drm_crtc_state *crtc_state;
+		struct drm_modeset_acquire_ctx ctx;
+		int ret;
+
+		drm_modeset_acquire_init(&ctx, 0);
+retry:
+		state = drm_atomic_commit_alloc(connector->dev);
+		if (!state)
+			goto out;
+		state->acquire_ctx = &ctx;
+
+		crtc_state = drm_atomic_get_crtc_state(state, crtc);
+		ret = PTR_ERR_OR_ZERO(crtc_state);
+		if (!ret)
+			crtc_state->active = false;
+		if (!ret)
+			ret = drm_atomic_set_mode_prop_for_crtc(crtc_state, NULL);
+		if (!ret) {
+			conn_state = drm_atomic_get_connector_state(state, connector);
+			ret = PTR_ERR_OR_ZERO(conn_state);
+		}
+		if (!ret)
+			ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
+		if (!ret)
+			ret = drm_atomic_commit(state);
+
+		drm_atomic_commit_put(state);
+		if (ret == -EDEADLK) {
+			drm_modeset_backoff(&ctx);
+			goto retry;
+		}
+		if (ret)
+			drm_err(connector->dev,
+				"DM_MST: failed to disable CRTC for removed connector %s (%d)\n",
+				connector->name, ret);
+out:
+		drm_modeset_drop_locks(&ctx);
+		drm_modeset_acquire_fini(&ctx);
+	}
 }
 
 static const struct drm_connector_funcs dm_dp_mst_connector_funcs = {

---
base-commit: 551c722f40809618230001baccf219193e22fc5a
change-id: 20260930-amd-mst-teardown-cab8a3731596

Best regards,
--  
David Medina <b2amedina@gmail.com>

Re: [PATCH RFC] drm/amd/display: Disable CRTC on MST connector unregister

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/amd: TOCTOU race and UAF from lockless crtc read
- [Medium] drm/amd: incomplete CRTC disable leaks planes and FBs
--

commit fad621ba901c426d67d64397ce2dc5fd953430db
Author: David Medina <b2amedina@gmail.com>

drm/amd/display: Disable CRTC on MST connector unregister

This commit forces a modeset that disables the CRTC when an MST connector is
unregistered. This prevents an encoder conflict during subsequent re-plugs.

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> index 045a7f88b7546..24b1460687393 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> @@ -227,6 +228,7 @@ amdgpu_dm_mst_connector_early_unregister(struct drm_connector *connector)
>  	struct amdgpu_dm_connector *root = aconnector->mst_root;
>  	struct dc_link *dc_link = aconnector->dc_link;
>  	struct dc_sink *dc_sink = aconnector->dc_sink;
> +	struct drm_crtc *crtc = connector->state ? connector->state->crtc : NULL;

[Severity: High]
Does amdgpu_dm_mst_connector_early_unregister() in
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c read
connector->state->crtc locklessly before locks are acquired?

If a concurrent modeset occurs when hot-unplugging an MST display, the
captured crtc pointer may be stale. When drm_atomic_get_crtc_state() later
locks and disables this CRTC, could it now belong to another display or have
been freed, leading to a use-after-free or unintended display teardown?

[ ... ]
> +	if (crtc) {
> +		struct drm_atomic_commit *state;
> +		struct drm_connector_state *conn_state;
> +		struct drm_crtc_state *crtc_state;
> +		struct drm_modeset_acquire_ctx ctx;
> +		int ret;
> +
> +		drm_modeset_acquire_init(&ctx, 0);
> +retry:
> +		state = drm_atomic_commit_alloc(connector->dev);
> +		if (!state)
> +			goto out;
> +		state->acquire_ctx = &ctx;
> +
> +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
> +		ret = PTR_ERR_OR_ZERO(crtc_state);
> +		if (!ret)
> +			crtc_state->active = false;
> +		if (!ret)
> +			ret = drm_atomic_set_mode_prop_for_crtc(crtc_state, NULL);
> +		if (!ret) {
> +			conn_state = drm_atomic_get_connector_state(state, connector);
> +			ret = PTR_ERR_OR_ZERO(conn_state);
> +		}
> +		if (!ret)
> +			ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
> +		if (!ret)
> +			ret = drm_atomic_commit(state);

[Severity: Medium]
Is amdgpu_dm_mst_connector_early_unregister() missing the step to detach the
affected planes from the disabled CRTC before committing the atomic state?

The DRM core atomic helpers require that planes attached to a disabled CRTC
have their framebuffers detached.

If drm_atomic_helper_check_modeset() automatically adds the planes to the
state, won't drm_atomic_helper_check_planes() reject the commit because the
planes still have framebuffers?

If these checks are bypassed, could this leak planes and framebuffers
indefinitely when an active MST connector is unplugged?

> +
> +		drm_atomic_commit_put(state);
> +		if (ret == -EDEADLK) {
> +			drm_modeset_backoff(&ctx);
> +			goto retry;

[Severity: High]
If a deadlock occurs and we drop locks to backoff, should the crtc pointer
be re-read safely under lock after jumping back to the retry label?

By reusing the pointer read locklessly earlier in the function, the retry
loop might operate on a stale pointer if the state changed during the
backoff.

> +		}
> +		if (ret)
> +			drm_err(connector->dev,
> +				"DM_MST: failed to disable CRTC for removed connector %s (%d)\n",
> +				connector->name, ret);
> +out:
> +		drm_modeset_drop_locks(&ctx);
> +		drm_modeset_acquire_fini(&ctx);
> +	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930-amd-mst-teardown-v1-1-16a39ebdd4de@gmail.com?part=1

Re: [PATCH RFC] drm/amd/display: Disable CRTC on MST connector unregister

[Harry Wentland]

On 2026-09-30 22:13, David Medina via B4 Relay wrote:
> From: David Medina <b2amedina@gmail.com>
> 
> amdgpu_dm_mst_connector_early_unregister() releases the connector's
> sink but leaves the CRTC it was driving active and its MST encoder
> assigned. Because the connector is unregistered in the same step,
> userspace has no chance to disable the CRTC before it is gone. When the

Why does userspace have no chance to disable the CRTC? It's the connector
that goes away, not the CRTC.

Userspace controls enablement. The kernel driver shouldn't disable something
proactively based on a hotplug.

Harry

> same display is re-plugged a new MST connector is created, and the
> re-commit fails in drm_atomic_helper_check_modeset() because the stale
> connector still owns the encoder (handle_conflicting_encoders), leaving
> the display dark until a full modeset (e.g. a session restart) tears
> everything down.
> 
> Disable the CRTC from the driver when the connector is torn down, so the
> MST encoder is released and the DC stream dropped. Reuse the
> drm_atomic_helper_disable_all() pattern: deactivate the CRTC, clear the
> mode, and disconnect the connector.
> 
> Reproduced 100% of the time by unplugging and re-plugging the Thunderbolt
> dock driving the LG ULTRAWIDE: on re-plug the display stays dark and
> dmesg shows drm_atomic_helper_check_modeset() failing with an encoder
> conflict (handle_conflicting_encoders). Verified over four unplug/replug
> cycles with this patch applied.
> 
> I am not certain that forcing a modeset from early_unregister() is the
> right approach, and the disable here is intentionally minimal: it
> deactivates the CRTC and disconnects the connector but does not
> add/detach the CRTC's affected planes the way
> drm_atomic_helper_disable_all() does. The minimal version resolves the
> failure in testing; I would welcome guidance on whether the full
> affected-planes teardown (or a different location) is required.
> 
> Fixes: a1b27e99229a ("drm/amd/display: Implement MST Aux device registration")
> Signed-off-by: David Medina <b2amedina@gmail.com>
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 53 ++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> index 045a7f88b754..24b146068739 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> @@ -29,6 +29,7 @@
>  #include <drm/display/drm_dp_mst_helper.h>
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
> +#include <drm/drm_atomic_uapi.h>
>  #include <drm/drm_fixed.h>
>  #include <drm/drm_edid.h>
>  #include "dm_services.h"
> @@ -227,6 +228,7 @@ amdgpu_dm_mst_connector_early_unregister(struct drm_connector *connector)
>  	struct amdgpu_dm_connector *root = aconnector->mst_root;
>  	struct dc_link *dc_link = aconnector->dc_link;
>  	struct dc_sink *dc_sink = aconnector->dc_sink;
> +	struct drm_crtc *crtc = connector->state ? connector->state->crtc : NULL;
>  
>  	drm_dp_mst_connector_early_unregister(connector, port);
>  
> @@ -250,6 +252,57 @@ amdgpu_dm_mst_connector_early_unregister(struct drm_connector *connector)
>  
>  	aconnector->mst_status = MST_STATUS_DEFAULT;
>  	drm_modeset_unlock(&root->mst_mgr.base.lock);
> +
> +	/*
> +	 * The connector is being removed from the MST topology. If a CRTC is
> +	 * still driving it, force a modeset that disables the CRTC so that the
> +	 * MST encoder is released and the DC stream dropped. Otherwise, on a
> +	 * subsequent re-plug the stale CRTC/encoder assignment triggers an
> +	 * encoder conflict in drm_atomic_helper_check_modeset() and the display
> +	 * stays dark.
> +	 */
> +	if (crtc) {
> +		struct drm_atomic_commit *state;
> +		struct drm_connector_state *conn_state;
> +		struct drm_crtc_state *crtc_state;
> +		struct drm_modeset_acquire_ctx ctx;
> +		int ret;
> +
> +		drm_modeset_acquire_init(&ctx, 0);
> +retry:
> +		state = drm_atomic_commit_alloc(connector->dev);
> +		if (!state)
> +			goto out;
> +		state->acquire_ctx = &ctx;
> +
> +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
> +		ret = PTR_ERR_OR_ZERO(crtc_state);
> +		if (!ret)
> +			crtc_state->active = false;
> +		if (!ret)
> +			ret = drm_atomic_set_mode_prop_for_crtc(crtc_state, NULL);
> +		if (!ret) {
> +			conn_state = drm_atomic_get_connector_state(state, connector);
> +			ret = PTR_ERR_OR_ZERO(conn_state);
> +		}
> +		if (!ret)
> +			ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
> +		if (!ret)
> +			ret = drm_atomic_commit(state);
> +
> +		drm_atomic_commit_put(state);
> +		if (ret == -EDEADLK) {
> +			drm_modeset_backoff(&ctx);
> +			goto retry;
> +		}
> +		if (ret)
> +			drm_err(connector->dev,
> +				"DM_MST: failed to disable CRTC for removed connector %s (%d)\n",
> +				connector->name, ret);
> +out:
> +		drm_modeset_drop_locks(&ctx);
> +		drm_modeset_acquire_fini(&ctx);
> +	}
>  }
>  
>  static const struct drm_connector_funcs dm_dp_mst_connector_funcs = {
> 
> ---
> base-commit: 551c722f40809618230001baccf219193e22fc5a
> change-id: 20260930-amd-mst-teardown-cab8a3731596
> 
> Best regards,
> --  
> David Medina <b2amedina@gmail.com>

Re: [PATCH RFC] drm/amd/display: Disable CRTC on MST connector unregister

[David Medina]

Hi Harry,

Thanks for the feedback — that's a fair point. Userspace does control
enablement, and the driver force-disabling a CRTC from
early_unregister() is the wrong layer.

The symptom I'm chasing: with a Thunderbolt 3 dock driving an LG
Ultrawide, unplugging the dock unregisters the MST connector, but the
CRTC that was driving it keeps its MST encoder assignment. On re-plug a
new connector is created and the re-commit fails in
drm_atomic_helper_check_modeset() with an encoder conflict
(handle_conflicting_encoders), so the display stays dark until a full
modeset (e.g. a session restart) tears everything down.

So my real question is where this is supposed to be handled: is there
something the driver should do during connector unregister to release
the stale CRTC/encoder assignment, or is this expected to be resolved
by userspace on the hotplug uevent? If the latter, is the compositor at
fault for leaving the CRTC active?

I only proposed the early_unregister() approach because it was the
minimal thing that fixed the failure in testing — not because I'm
confident it's correct. Happy to dig in whichever direction you point
me.

Thanks,
David

On Thu, Oct 1, 2026 at 9:54 AM Harry Wentland <harry.wentland@amd.com> wrote:
>
>
>
> On 2026-09-30 22:13, David Medina via B4 Relay wrote:
> > From: David Medina <b2amedina@gmail.com>
> >
> > amdgpu_dm_mst_connector_early_unregister() releases the connector's
> > sink but leaves the CRTC it was driving active and its MST encoder
> > assigned. Because the connector is unregistered in the same step,
> > userspace has no chance to disable the CRTC before it is gone. When the
>
> Why does userspace have no chance to disable the CRTC? It's the connector
> that goes away, not the CRTC.
>
> Userspace controls enablement. The kernel driver shouldn't disable something
> proactively based on a hotplug.
>
> Harry
>
> > same display is re-plugged a new MST connector is created, and the
> > re-commit fails in drm_atomic_helper_check_modeset() because the stale
> > connector still owns the encoder (handle_conflicting_encoders), leaving
> > the display dark until a full modeset (e.g. a session restart) tears
> > everything down.
> >
> > Disable the CRTC from the driver when the connector is torn down, so the
> > MST encoder is released and the DC stream dropped. Reuse the
> > drm_atomic_helper_disable_all() pattern: deactivate the CRTC, clear the
> > mode, and disconnect the connector.
> >
> > Reproduced 100% of the time by unplugging and re-plugging the Thunderbolt
> > dock driving the LG ULTRAWIDE: on re-plug the display stays dark and
> > dmesg shows drm_atomic_helper_check_modeset() failing with an encoder
> > conflict (handle_conflicting_encoders). Verified over four unplug/replug
> > cycles with this patch applied.
> >
> > I am not certain that forcing a modeset from early_unregister() is the
> > right approach, and the disable here is intentionally minimal: it
> > deactivates the CRTC and disconnects the connector but does not
> > add/detach the CRTC's affected planes the way
> > drm_atomic_helper_disable_all() does. The minimal version resolves the
> > failure in testing; I would welcome guidance on whether the full
> > affected-planes teardown (or a different location) is required.
> >
> > Fixes: a1b27e99229a ("drm/amd/display: Implement MST Aux device registration")
> > Signed-off-by: David Medina <b2amedina@gmail.com>
> > ---
> >  .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 53 ++++++++++++++++++++++
> >  1 file changed, 53 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> > index 045a7f88b754..24b146068739 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> > @@ -29,6 +29,7 @@
> >  #include <drm/display/drm_dp_mst_helper.h>
> >  #include <drm/drm_atomic.h>
> >  #include <drm/drm_atomic_helper.h>
> > +#include <drm/drm_atomic_uapi.h>
> >  #include <drm/drm_fixed.h>
> >  #include <drm/drm_edid.h>
> >  #include "dm_services.h"
> > @@ -227,6 +228,7 @@ amdgpu_dm_mst_connector_early_unregister(struct drm_connector *connector)
> >       struct amdgpu_dm_connector *root = aconnector->mst_root;
> >       struct dc_link *dc_link = aconnector->dc_link;
> >       struct dc_sink *dc_sink = aconnector->dc_sink;
> > +     struct drm_crtc *crtc = connector->state ? connector->state->crtc : NULL;
> >
> >       drm_dp_mst_connector_early_unregister(connector, port);
> >
> > @@ -250,6 +252,57 @@ amdgpu_dm_mst_connector_early_unregister(struct drm_connector *connector)
> >
> >       aconnector->mst_status = MST_STATUS_DEFAULT;
> >       drm_modeset_unlock(&root->mst_mgr.base.lock);
> > +
> > +     /*
> > +      * The connector is being removed from the MST topology. If a CRTC is
> > +      * still driving it, force a modeset that disables the CRTC so that the
> > +      * MST encoder is released and the DC stream dropped. Otherwise, on a
> > +      * subsequent re-plug the stale CRTC/encoder assignment triggers an
> > +      * encoder conflict in drm_atomic_helper_check_modeset() and the display
> > +      * stays dark.
> > +      */
> > +     if (crtc) {
> > +             struct drm_atomic_commit *state;
> > +             struct drm_connector_state *conn_state;
> > +             struct drm_crtc_state *crtc_state;
> > +             struct drm_modeset_acquire_ctx ctx;
> > +             int ret;
> > +
> > +             drm_modeset_acquire_init(&ctx, 0);
> > +retry:
> > +             state = drm_atomic_commit_alloc(connector->dev);
> > +             if (!state)
> > +                     goto out;
> > +             state->acquire_ctx = &ctx;
> > +
> > +             crtc_state = drm_atomic_get_crtc_state(state, crtc);
> > +             ret = PTR_ERR_OR_ZERO(crtc_state);
> > +             if (!ret)
> > +                     crtc_state->active = false;
> > +             if (!ret)
> > +                     ret = drm_atomic_set_mode_prop_for_crtc(crtc_state, NULL);
> > +             if (!ret) {
> > +                     conn_state = drm_atomic_get_connector_state(state, connector);
> > +                     ret = PTR_ERR_OR_ZERO(conn_state);
> > +             }
> > +             if (!ret)
> > +                     ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
> > +             if (!ret)
> > +                     ret = drm_atomic_commit(state);
> > +
> > +             drm_atomic_commit_put(state);
> > +             if (ret == -EDEADLK) {
> > +                     drm_modeset_backoff(&ctx);
> > +                     goto retry;
> > +             }
> > +             if (ret)
> > +                     drm_err(connector->dev,
> > +                             "DM_MST: failed to disable CRTC for removed connector %s (%d)\n",
> > +                             connector->name, ret);
> > +out:
> > +             drm_modeset_drop_locks(&ctx);
> > +             drm_modeset_acquire_fini(&ctx);
> > +     }
> >  }
> >
> >  static const struct drm_connector_funcs dm_dp_mst_connector_funcs = {
> >
> > ---
> > base-commit: 551c722f40809618230001baccf219193e22fc5a
> > change-id: 20260930-amd-mst-teardown-cab8a3731596
> >
> > Best regards,
> > --
> > David Medina <b2amedina@gmail.com>
>


-- 
David Medina
14901 SW 87th Ave
Palmetto Bay, FL 33176
786-280-0880