diff mbox series

[3/6] drm/i915: Leave intel_conn->mst_port set, use mst_port_gone instead

Message ID 20180918230637.20700-4-lyude@redhat.com (mailing list archive)
State New, archived
Headers show
Series Fix legacy DPMS changes with MST | expand

Commit Message

Lyude Paul Sept. 18, 2018, 11:06 p.m. UTC 
Currently we set intel_connector->mst_port to NULL to signify that the
MST port has been removed from the system so that we can prevent further
action on the port such as connector probes, mode probing, etc.
However, we're going to need access to intel_connector->mst_port in
order to fixup ->best_encoder() so that it can always return the correct
encoder for an MST port to prevent legacy DPMS prop changes from
failing. This should be safe, so instead keep intel_connector->mst_port
always set and instead add intel_connector->mst_port_gone in order to
signify whether or not the connector has disappeared from the system.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/i915/intel_dp_mst.c | 14 +++++++-------
 drivers/gpu/drm/i915/intel_drv.h    |  1 +
 2 files changed, 8 insertions(+), 7 deletions(-)

Comments

Sasha Levin Sept. 19, 2018, 6:58 p.m. UTC | #1 
Hi,

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v4.18.8, v4.14.70, v4.9.127, v4.4.156, v3.18.122, 

v4.18.8: Build OK!
v4.14.70: Build OK!
v4.9.127: Failed to apply! Possible dependencies:
    06bfe5b0d892 ("drm/i915: Avoid null dereference if mst_port is unset.")
    22a2c8e0457f ("drm/i915: Validate mode against max. link data rate for DP MST")
    9301397a63b3 ("drm/i915: Implement Link Rate fallback on Link training failure")
    af2405af07d1 ("drm/fb-helper: Push down modeset lock into FB helpers")

v4.4.156: Failed to apply! Possible dependencies:
    0552f7651bc2 ("drm/i915/mst: use reference counted connectors. (v3)")
    06bfe5b0d892 ("drm/i915: Avoid null dereference if mst_port is unset.")
    22a2c8e0457f ("drm/i915: Validate mode against max. link data rate for DP MST")
    9301397a63b3 ("drm/i915: Implement Link Rate fallback on Link training failure")
    af2405af07d1 ("drm/fb-helper: Push down modeset lock into FB helpers")

v3.18.122: Failed to apply! Possible dependencies:
    0552f7651bc2 ("drm/i915/mst: use reference counted connectors. (v3)")
    06bfe5b0d892 ("drm/i915: Avoid null dereference if mst_port is unset.")
    22a2c8e0457f ("drm/i915: Validate mode against max. link data rate for DP MST")
    459485ad3513 ("drm/i915: Fixup dp mst encoder selection")
    9301397a63b3 ("drm/i915: Implement Link Rate fallback on Link training failure")
    af2405af07d1 ("drm/fb-helper: Push down modeset lock into FB helpers")
    c6a0aed4d493 ("drm/mst: cached EDID for logical ports (v2)")


Please let us know how to resolve this.

--
Thanks,
Sasha
Lyude Paul Sept. 19, 2018, 10:01 p.m. UTC | #2 
On Wed, 2018-09-19 at 18:58 +0000, Sasha Levin wrote:
> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: all
> 
> The bot has tested the following trees: v4.18.8, v4.14.70, v4.9.127, v4.4.156,
> v3.18.122, 
> 
> v4.18.8: Build OK!
> v4.14.70: Build OK!
> v4.9.127: Failed to apply! Possible dependencies:
>     06bfe5b0d892 ("drm/i915: Avoid null dereference if mst_port is unset.")
>     22a2c8e0457f ("drm/i915: Validate mode against max. link data rate for DP
> MST")
>     9301397a63b3 ("drm/i915: Implement Link Rate fallback on Link training
> failure")
>     af2405af07d1 ("drm/fb-helper: Push down modeset lock into FB helpers")
> 
> v4.4.156: Failed to apply! Possible dependencies:
>     0552f7651bc2 ("drm/i915/mst: use reference counted connectors. (v3)")
>     06bfe5b0d892 ("drm/i915: Avoid null dereference if mst_port is unset.")
>     22a2c8e0457f ("drm/i915: Validate mode against max. link data rate for DP
> MST")
>     9301397a63b3 ("drm/i915: Implement Link Rate fallback on Link training
> failure")
>     af2405af07d1 ("drm/fb-helper: Push down modeset lock into FB helpers")
> 
> v3.18.122: Failed to apply! Possible dependencies:
>     0552f7651bc2 ("drm/i915/mst: use reference counted connectors. (v3)")
>     06bfe5b0d892 ("drm/i915: Avoid null dereference if mst_port is unset.")
>     22a2c8e0457f ("drm/i915: Validate mode against max. link data rate for DP
> MST")
>     459485ad3513 ("drm/i915: Fixup dp mst encoder selection")
>     9301397a63b3 ("drm/i915: Implement Link Rate fallback on Link training
> failure")
>     af2405af07d1 ("drm/fb-helper: Push down modeset lock into FB helpers")
>     c6a0aed4d493 ("drm/mst: cached EDID for logical ports (v2)")
> 
> 
> Please let us know how to resolve this.

...is this the email address I'm supposed to "let you know how to resolve this"
with? if that is the case, it's 100% OK to simply ignore all of the versions
that don't apply with this patch.
> 
> --
> Thanks,
> Sasha
Daniel Vetter Sept. 21, 2018, 9:27 a.m. UTC | #3 
On Tue, Sep 18, 2018 at 07:06:19PM -0400, Lyude Paul wrote:
> Currently we set intel_connector->mst_port to NULL to signify that the
> MST port has been removed from the system so that we can prevent further
> action on the port such as connector probes, mode probing, etc.
> However, we're going to need access to intel_connector->mst_port in
> order to fixup ->best_encoder() so that it can always return the correct
> encoder for an MST port to prevent legacy DPMS prop changes from
> failing. This should be safe, so instead keep intel_connector->mst_port
> always set and instead add intel_connector->mst_port_gone in order to
> signify whether or not the connector has disappeared from the system.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Cc: stable@vger.kernel.org

Hm, how exactly do legacy DPMS setprop blow up here? Or at least I can't
come up with a scenario that's specific to legacy props, atomic should be
equally affected. By the time we land in this code, the core code already
remapping it to be purely an atomic update.

Another thought: Should we handle at least some of this in the probe
helpers? I.e. once you unplugged a drm_connector, it always shows
disconnected and mode list is gone, instead of duplicating this over all
drivers. We could just check connector->registered.

Thanks, Daniel
> ---
>  drivers/gpu/drm/i915/intel_dp_mst.c | 14 +++++++-------
>  drivers/gpu/drm/i915/intel_drv.h    |  1 +
>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 4ecd65375603..fcb9b87b9339 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -311,9 +311,8 @@ static int intel_dp_mst_get_ddc_modes(struct drm_connector *connector)
>  	struct edid *edid;
>  	int ret;
>  
> -	if (!intel_dp) {
> +	if (intel_connector->mst_port_gone)
>  		return intel_connector_update_modes(connector, NULL);
> -	}
>  
>  	edid = drm_dp_mst_get_edid(connector, &intel_dp->mst_mgr, intel_connector->port);
>  	ret = intel_connector_update_modes(connector, edid);
> @@ -328,9 +327,10 @@ intel_dp_mst_detect(struct drm_connector *connector, bool force)
>  	struct intel_connector *intel_connector = to_intel_connector(connector);
>  	struct intel_dp *intel_dp = intel_connector->mst_port;
>  
> -	if (!intel_dp)
> +	if (intel_connector->mst_port_gone)
>  		return connector_status_disconnected;
> -	return drm_dp_mst_detect_port(connector, &intel_dp->mst_mgr, intel_connector->port);
> +	return drm_dp_mst_detect_port(connector, &intel_dp->mst_mgr,
> +				      intel_connector->port);
>  }
>  
>  static void
> @@ -370,7 +370,7 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
>  	int bpp = 24; /* MST uses fixed bpp */
>  	int max_rate, mode_rate, max_lanes, max_link_clock;
>  
> -	if (!intel_dp)
> +	if (intel_connector->mst_port_gone)
>  		return MODE_ERROR;
>  
>  	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> @@ -402,7 +402,7 @@ static struct drm_encoder *intel_mst_atomic_best_encoder(struct drm_connector *c
>  	struct intel_dp *intel_dp = intel_connector->mst_port;
>  	struct intel_crtc *crtc = to_intel_crtc(state->crtc);
>  
> -	if (!intel_dp)
> +	if (intel_connector->mst_port_gone)
>  		return NULL;
>  	return &intel_dp->mst_encoders[crtc->pipe]->base.base;
>  }
> @@ -514,7 +514,7 @@ static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
>  						   connector);
>  	/* prevent race with the check in ->detect */
>  	drm_modeset_lock(&connector->dev->mode_config.connection_mutex, NULL);
> -	intel_connector->mst_port = NULL;
> +	intel_connector->mst_port_gone = true;
>  	drm_modeset_unlock(&connector->dev->mode_config.connection_mutex);
>  
>  	drm_connector_put(connector);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 8fc61e96754f..87ce772ae7f8 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -409,6 +409,7 @@ struct intel_connector {
>  	void *port; /* store this opaque as its illegal to dereference it */
>  
>  	struct intel_dp *mst_port;
> +	bool mst_port_gone;
>  
>  	/* Work struct to schedule a uevent on link train failure */
>  	struct work_struct modeset_retry_work;
> -- 
> 2.17.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Lyude Paul Sept. 21, 2018, 8:17 p.m. UTC | #4 
On Fri, 2018-09-21 at 11:27 +0200, Daniel Vetter wrote:
> On Tue, Sep 18, 2018 at 07:06:19PM -0400, Lyude Paul wrote:
> > Currently we set intel_connector->mst_port to NULL to signify that the
> > MST port has been removed from the system so that we can prevent further
> > action on the port such as connector probes, mode probing, etc.
> > However, we're going to need access to intel_connector->mst_port in
> > order to fixup ->best_encoder() so that it can always return the correct
> > encoder for an MST port to prevent legacy DPMS prop changes from
> > failing. This should be safe, so instead keep intel_connector->mst_port
> > always set and instead add intel_connector->mst_port_gone in order to
> > signify whether or not the connector has disappeared from the system.
> > 
> > Signed-off-by: Lyude Paul <lyude@redhat.com>
> > Cc: stable@vger.kernel.org
> 
> Hm, how exactly do legacy DPMS setprop blow up here? Or at least I can't
> come up with a scenario that's specific to legacy props, atomic should be
> equally affected. By the time we land in this code, the core code already
> remapping it to be purely an atomic update.
So, what I've been seeing with X that's been blowing this up:
 * Hotplug event gets received, X does reprobe
 * Notices MST connectors are now disconnected, sets DPMS from on->off
 * During atomic_check, drm_atomic_helper_check_modeset() is called
 * update_connector_routing() gets called
 * funcs->best_encoder() returns NULL for the encoder
 * update_connector_routing() fails atomic commit with "No suitable encoder
   found", line 320 of drm_atomic_helper
> 
> Another thought: Should we handle at least some of this in the probe
> helpers? I.e. once you unplugged a drm_connector, it always shows
> disconnected and mode list is gone, instead of duplicating this over all
> drivers. We could just check connector->registered.
oooh, good idea! I will certainly go do that
> 
> Thanks, Daniel
> > ---
> >  drivers/gpu/drm/i915/intel_dp_mst.c | 14 +++++++-------
> >  drivers/gpu/drm/i915/intel_drv.h    |  1 +
> >  2 files changed, 8 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
> > b/drivers/gpu/drm/i915/intel_dp_mst.c
> > index 4ecd65375603..fcb9b87b9339 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > @@ -311,9 +311,8 @@ static int intel_dp_mst_get_ddc_modes(struct
> > drm_connector *connector)
> >  	struct edid *edid;
> >  	int ret;
> >  
> > -	if (!intel_dp) {
> > +	if (intel_connector->mst_port_gone)
> >  		return intel_connector_update_modes(connector, NULL);
> > -	}
> >  
> >  	edid = drm_dp_mst_get_edid(connector, &intel_dp->mst_mgr,
> > intel_connector->port);
> >  	ret = intel_connector_update_modes(connector, edid);
> > @@ -328,9 +327,10 @@ intel_dp_mst_detect(struct drm_connector *connector,
> > bool force)
> >  	struct intel_connector *intel_connector =
> > to_intel_connector(connector);
> >  	struct intel_dp *intel_dp = intel_connector->mst_port;
> >  
> > -	if (!intel_dp)
> > +	if (intel_connector->mst_port_gone)
> >  		return connector_status_disconnected;
> > -	return drm_dp_mst_detect_port(connector, &intel_dp->mst_mgr,
> > intel_connector->port);
> > +	return drm_dp_mst_detect_port(connector, &intel_dp->mst_mgr,
> > +				      intel_connector->port);
> >  }
> >  
> >  static void
> > @@ -370,7 +370,7 @@ intel_dp_mst_mode_valid(struct drm_connector
> > *connector,
> >  	int bpp = 24; /* MST uses fixed bpp */
> >  	int max_rate, mode_rate, max_lanes, max_link_clock;
> >  
> > -	if (!intel_dp)
> > +	if (intel_connector->mst_port_gone)
> >  		return MODE_ERROR;
> >  
> >  	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> > @@ -402,7 +402,7 @@ static struct drm_encoder
> > *intel_mst_atomic_best_encoder(struct drm_connector *c
> >  	struct intel_dp *intel_dp = intel_connector->mst_port;
> >  	struct intel_crtc *crtc = to_intel_crtc(state->crtc);
> >  
> > -	if (!intel_dp)
> > +	if (intel_connector->mst_port_gone)
> >  		return NULL;
> >  	return &intel_dp->mst_encoders[crtc->pipe]->base.base;
> >  }
> > @@ -514,7 +514,7 @@ static void intel_dp_destroy_mst_connector(struct
> > drm_dp_mst_topology_mgr *mgr,
> >  						   connector);
> >  	/* prevent race with the check in ->detect */
> >  	drm_modeset_lock(&connector->dev->mode_config.connection_mutex, NULL);
> > -	intel_connector->mst_port = NULL;
> > +	intel_connector->mst_port_gone = true;
> >  	drm_modeset_unlock(&connector->dev->mode_config.connection_mutex);
> >  
> >  	drm_connector_put(connector);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 8fc61e96754f..87ce772ae7f8 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -409,6 +409,7 @@ struct intel_connector {
> >  	void *port; /* store this opaque as its illegal to dereference it */
> >  
> >  	struct intel_dp *mst_port;
> > +	bool mst_port_gone;
> >  
> >  	/* Work struct to schedule a uevent on link train failure */
> >  	struct work_struct modeset_retry_work;
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
>
Daniel Vetter Sept. 22, 2018, 8:51 a.m. UTC | #5 
On Fri, Sep 21, 2018 at 04:17:16PM -0400, Lyude Paul wrote:
> On Fri, 2018-09-21 at 11:27 +0200, Daniel Vetter wrote:
> > On Tue, Sep 18, 2018 at 07:06:19PM -0400, Lyude Paul wrote:
> > > Currently we set intel_connector->mst_port to NULL to signify that the
> > > MST port has been removed from the system so that we can prevent further
> > > action on the port such as connector probes, mode probing, etc.
> > > However, we're going to need access to intel_connector->mst_port in
> > > order to fixup ->best_encoder() so that it can always return the correct
> > > encoder for an MST port to prevent legacy DPMS prop changes from
> > > failing. This should be safe, so instead keep intel_connector->mst_port
> > > always set and instead add intel_connector->mst_port_gone in order to
> > > signify whether or not the connector has disappeared from the system.
> > > 
> > > Signed-off-by: Lyude Paul <lyude@redhat.com>
> > > Cc: stable@vger.kernel.org
> > 
> > Hm, how exactly do legacy DPMS setprop blow up here? Or at least I can't
> > come up with a scenario that's specific to legacy props, atomic should be
> > equally affected. By the time we land in this code, the core code already
> > remapping it to be purely an atomic update.
> So, what I've been seeing with X that's been blowing this up:
>  * Hotplug event gets received, X does reprobe
>  * Notices MST connectors are now disconnected, sets DPMS from on->off
>  * During atomic_check, drm_atomic_helper_check_modeset() is called
>  * update_connector_routing() gets called
>  * funcs->best_encoder() returns NULL for the encoder
>  * update_connector_routing() fails atomic commit with "No suitable encoder
>    found", line 320 of drm_atomic_helper

Uh ... And X then doesn't try to fully shut down the CRTC? That would be a
SETCRTC with num_connectors=0, mode=0.

The failure mode has nothing to do with legacy dpms, it's just anything
that touches the modeset state will fail (except when you fully turn
things off, not just dpms off). If we fix this, then a DPMS on will
probably also "work", with possibly some hilarious dmesg noise and driver
bugs. So we need to be careful that this only allows disabling, not
re-enabling afterwards.

Hm ... thinking a bit more, I think a full disable also will not quite
work. Definitely need to test that.

> > Another thought: Should we handle at least some of this in the probe
> > helpers? I.e. once you unplugged a drm_connector, it always shows
> > disconnected and mode list is gone, instead of duplicating this over all
> > drivers. We could just check connector->registered.
> oooh, good idea! I will certainly go do that

We'll probably also need some checks in atomic helpers to prevent
re-enabling of a ghost connector.
-Daniel

> > 
> > Thanks, Daniel
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp_mst.c | 14 +++++++-------
> > >  drivers/gpu/drm/i915/intel_drv.h    |  1 +
> > >  2 files changed, 8 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
> > > b/drivers/gpu/drm/i915/intel_dp_mst.c
> > > index 4ecd65375603..fcb9b87b9339 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > > @@ -311,9 +311,8 @@ static int intel_dp_mst_get_ddc_modes(struct
> > > drm_connector *connector)
> > >  	struct edid *edid;
> > >  	int ret;
> > >  
> > > -	if (!intel_dp) {
> > > +	if (intel_connector->mst_port_gone)
> > >  		return intel_connector_update_modes(connector, NULL);
> > > -	}
> > >  
> > >  	edid = drm_dp_mst_get_edid(connector, &intel_dp->mst_mgr,
> > > intel_connector->port);
> > >  	ret = intel_connector_update_modes(connector, edid);
> > > @@ -328,9 +327,10 @@ intel_dp_mst_detect(struct drm_connector *connector,
> > > bool force)
> > >  	struct intel_connector *intel_connector =
> > > to_intel_connector(connector);
> > >  	struct intel_dp *intel_dp = intel_connector->mst_port;
> > >  
> > > -	if (!intel_dp)
> > > +	if (intel_connector->mst_port_gone)
> > >  		return connector_status_disconnected;
> > > -	return drm_dp_mst_detect_port(connector, &intel_dp->mst_mgr,
> > > intel_connector->port);
> > > +	return drm_dp_mst_detect_port(connector, &intel_dp->mst_mgr,
> > > +				      intel_connector->port);
> > >  }
> > >  
> > >  static void
> > > @@ -370,7 +370,7 @@ intel_dp_mst_mode_valid(struct drm_connector
> > > *connector,
> > >  	int bpp = 24; /* MST uses fixed bpp */
> > >  	int max_rate, mode_rate, max_lanes, max_link_clock;
> > >  
> > > -	if (!intel_dp)
> > > +	if (intel_connector->mst_port_gone)
> > >  		return MODE_ERROR;
> > >  
> > >  	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> > > @@ -402,7 +402,7 @@ static struct drm_encoder
> > > *intel_mst_atomic_best_encoder(struct drm_connector *c
> > >  	struct intel_dp *intel_dp = intel_connector->mst_port;
> > >  	struct intel_crtc *crtc = to_intel_crtc(state->crtc);
> > >  
> > > -	if (!intel_dp)
> > > +	if (intel_connector->mst_port_gone)
> > >  		return NULL;
> > >  	return &intel_dp->mst_encoders[crtc->pipe]->base.base;
> > >  }
> > > @@ -514,7 +514,7 @@ static void intel_dp_destroy_mst_connector(struct
> > > drm_dp_mst_topology_mgr *mgr,
> > >  						   connector);
> > >  	/* prevent race with the check in ->detect */
> > >  	drm_modeset_lock(&connector->dev->mode_config.connection_mutex, NULL);
> > > -	intel_connector->mst_port = NULL;
> > > +	intel_connector->mst_port_gone = true;
> > >  	drm_modeset_unlock(&connector->dev->mode_config.connection_mutex);
> > >  
> > >  	drm_connector_put(connector);
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index 8fc61e96754f..87ce772ae7f8 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -409,6 +409,7 @@ struct intel_connector {
> > >  	void *port; /* store this opaque as its illegal to dereference it */
> > >  
> > >  	struct intel_dp *mst_port;
> > > +	bool mst_port_gone;
> > >  
> > >  	/* Work struct to schedule a uevent on link train failure */
> > >  	struct work_struct modeset_retry_work;
> > > -- 
> > > 2.17.1
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > 
> -- 
> Cheers,
> 	Lyude Paul
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 4ecd65375603..fcb9b87b9339 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -311,9 +311,8 @@  static int intel_dp_mst_get_ddc_modes(struct drm_connector *connector)
 	struct edid *edid;
 	int ret;
 
-	if (!intel_dp) {
+	if (intel_connector->mst_port_gone)
 		return intel_connector_update_modes(connector, NULL);
-	}
 
 	edid = drm_dp_mst_get_edid(connector, &intel_dp->mst_mgr, intel_connector->port);
 	ret = intel_connector_update_modes(connector, edid);
@@ -328,9 +327,10 @@  intel_dp_mst_detect(struct drm_connector *connector, bool force)
 	struct intel_connector *intel_connector = to_intel_connector(connector);
 	struct intel_dp *intel_dp = intel_connector->mst_port;
 
-	if (!intel_dp)
+	if (intel_connector->mst_port_gone)
 		return connector_status_disconnected;
-	return drm_dp_mst_detect_port(connector, &intel_dp->mst_mgr, intel_connector->port);
+	return drm_dp_mst_detect_port(connector, &intel_dp->mst_mgr,
+				      intel_connector->port);
 }
 
 static void
@@ -370,7 +370,7 @@  intel_dp_mst_mode_valid(struct drm_connector *connector,
 	int bpp = 24; /* MST uses fixed bpp */
 	int max_rate, mode_rate, max_lanes, max_link_clock;
 
-	if (!intel_dp)
+	if (intel_connector->mst_port_gone)
 		return MODE_ERROR;
 
 	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
@@ -402,7 +402,7 @@  static struct drm_encoder *intel_mst_atomic_best_encoder(struct drm_connector *c
 	struct intel_dp *intel_dp = intel_connector->mst_port;
 	struct intel_crtc *crtc = to_intel_crtc(state->crtc);
 
-	if (!intel_dp)
+	if (intel_connector->mst_port_gone)
 		return NULL;
 	return &intel_dp->mst_encoders[crtc->pipe]->base.base;
 }
@@ -514,7 +514,7 @@  static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
 						   connector);
 	/* prevent race with the check in ->detect */
 	drm_modeset_lock(&connector->dev->mode_config.connection_mutex, NULL);
-	intel_connector->mst_port = NULL;
+	intel_connector->mst_port_gone = true;
 	drm_modeset_unlock(&connector->dev->mode_config.connection_mutex);
 
 	drm_connector_put(connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8fc61e96754f..87ce772ae7f8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -409,6 +409,7 @@  struct intel_connector {
 	void *port; /* store this opaque as its illegal to dereference it */
 
 	struct intel_dp *mst_port;
+	bool mst_port_gone;
 
 	/* Work struct to schedule a uevent on link train failure */
 	struct work_struct modeset_retry_work;