diff mbox

[14/15] drm/i915/mst: use reference counted connectors.

Message ID 1460697046-23781-15-git-send-email-airlied@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dave Airlie April 15, 2016, 5:10 a.m. UTC
From: Dave Airlie <airlied@redhat.com>

Don't just free the connector when we get the destroy callback.

Drop a reference to it, and set it's mst_port to NULL so
no more mst work is done on it.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/i915/intel_dp_mst.c | 46 ++++++++++++++++++-------------------
 drivers/gpu/drm/i915/intel_drv.h    |  2 +-
 2 files changed, 24 insertions(+), 24 deletions(-)

Comments

Daniel Vetter April 22, 2016, 9:03 a.m. UTC | #1
On Fri, Apr 15, 2016 at 03:10:45PM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> Don't just free the connector when we get the destroy callback.
> 
> Drop a reference to it, and set it's mst_port to NULL so
> no more mst work is done on it.
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>

Looks correct, but two comments for better api for shared helpers inline below.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_dp_mst.c | 46 ++++++++++++++++++-------------------
>  drivers/gpu/drm/i915/intel_drv.h    |  2 +-
>  2 files changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index a2bd698..2e730b6 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -113,7 +113,7 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder)
>  
>  	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
>  
> -	drm_dp_mst_reset_vcpi_slots(&intel_dp->mst_mgr, intel_mst->port);
> +	drm_dp_mst_reset_vcpi_slots(&intel_dp->mst_mgr, intel_mst->connector->port);
>  
>  	ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
>  	if (ret) {
> @@ -138,10 +138,12 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder)
>  	/* and this can also fail */
>  	drm_dp_update_payload_part2(&intel_dp->mst_mgr);
>  
> -	drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, intel_mst->port);
> +	drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, intel_mst->connector->port);
>  
>  	intel_dp->active_mst_links--;
> -	intel_mst->port = NULL;
> +
> +	drm_connector_unreference(&intel_mst->connector->base);
> +	intel_mst->connector = NULL;
>  	if (intel_dp->active_mst_links == 0) {
>  		intel_dig_port->base.post_disable(&intel_dig_port->base);
>  		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> @@ -181,7 +183,9 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder)
>  	found->encoder = encoder;
>  
>  	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
> -	intel_mst->port = found->port;
> +
> +	intel_mst->connector = found;
> +	drm_connector_reference(&intel_mst->connector->base);
>  
>  	if (intel_dp->active_mst_links == 0) {
>  		intel_prepare_ddi_buffer(&intel_dig_port->base);
> @@ -199,7 +203,7 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder)
>  	}
>  
>  	ret = drm_dp_mst_allocate_vcpi(&intel_dp->mst_mgr,
> -				       intel_mst->port,
> +				       intel_mst->connector->port,
>  				       intel_crtc->config->pbn, &slots);
>  	if (ret == false) {
>  		DRM_ERROR("failed to allocate vcpi\n");
> @@ -248,7 +252,7 @@ static bool intel_dp_mst_enc_get_hw_state(struct intel_encoder *encoder,
>  {
>  	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
>  	*pipe = intel_mst->pipe;
> -	if (intel_mst->port)
> +	if (intel_mst->connector)
>  		return true;
>  	return false;
>  }
> @@ -312,6 +316,11 @@ static int intel_dp_mst_get_ddc_modes(struct drm_connector *connector)
>  	struct edid *edid;
>  	int ret;
>  
> +	if (!intel_connector->port || !intel_dp) {
> +		ret = intel_connector_update_modes(connector, NULL);
> +		return ret;
> +	}
> +
>  	edid = drm_dp_mst_get_edid(connector, &intel_dp->mst_mgr, intel_connector->port);
>  	if (!edid)
>  		return 0;
> @@ -328,6 +337,8 @@ 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_connector->port || !intel_dp)
> +		return connector_status_disconnected;
>  	return drm_dp_mst_detect_port(connector, &intel_dp->mst_mgr, intel_connector->port);

Should we push this change and the preceeding one into dp helpers, i.e.
make them cope with a null port? Otoh more work to fish out the
->mst_mgr, so not sure.

Hm ... isn't the mst_mgr a redundant argument, and we could figure that
out purely from the port? Would be a bit of refactoring, but I think the
shared code for this crucial bit of semantics (there's no way the mst port
is ever connected if it's unplugged) would be good.

>  }
>  
> @@ -393,6 +404,8 @@ 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)
> +		return NULL;
>  	return &intel_dp->mst_encoders[crtc->pipe]->base.base;
>  }
>  
> @@ -400,6 +413,8 @@ static struct drm_encoder *intel_mst_best_encoder(struct drm_connector *connecto
>  {
>  	struct intel_connector *intel_connector = to_intel_connector(connector);
>  	struct intel_dp *intel_dp = intel_connector->mst_port;
> +	if (!intel_dp)
> +		return NULL;
>  	return &intel_dp->mst_encoders[0]->base.base;
>  }
>  
> @@ -506,29 +521,14 @@ static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
>  	struct intel_connector *intel_connector = to_intel_connector(connector);
>  	struct drm_device *dev = connector->dev;
>  
> -	/* need to nuke the connector */
> -	drm_modeset_lock_all(dev);
> -	if (connector->state->crtc) {
> -		struct drm_mode_set set;
> -		int ret;
> -
> -		memset(&set, 0, sizeof(set));
> -		set.crtc = connector->state->crtc,
> -
> -		ret = drm_atomic_helper_set_config(&set);
> -
> -		WARN(ret, "Disabling mst crtc failed with %i\n", ret);
> -	}
> -	drm_modeset_unlock_all(dev);
> -
>  	intel_connector->unregister(intel_connector);
>  
>  	drm_modeset_lock_all(dev);
>  	intel_connector_remove_from_fbdev(intel_connector);
> -	drm_connector_cleanup(connector);
> +	intel_connector->mst_port = NULL;
>  	drm_modeset_unlock_all(dev);

Hm, ugly that we still need to grab modeset locks here. I'd like to avoid
that, since it could be a major stall. Maybe we need to have a separate
lock for the fbdev connector list, and then push the locking for that
(including refcounting) down into fbdev helpers?

>  
> -	kfree(intel_connector);
> +	drm_connector_unreference(&intel_connector->base);
>  	DRM_DEBUG_KMS("\n");
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4c027d6..0d2360e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -831,7 +831,7 @@ struct intel_dp_mst_encoder {
>  	struct intel_encoder base;
>  	enum pipe pipe;
>  	struct intel_digital_port *primary;
> -	void *port; /* store this opaque as its illegal to dereference it */
> +	struct intel_connector *connector;
>  };
>  
>  static inline enum dpio_channel
> -- 
> 2.5.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Dave Airlie April 27, 2016, 1:54 a.m. UTC | #2
On 22 April 2016 at 19:03, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Apr 15, 2016 at 03:10:45PM +1000, Dave Airlie wrote:
>> From: Dave Airlie <airlied@redhat.com>
>>
>> Don't just free the connector when we get the destroy callback.
>>
>> Drop a reference to it, and set it's mst_port to NULL so
>> no more mst work is done on it.
>>
>> Signed-off-by: Dave Airlie <airlied@redhat.com>
>
> Looks correct, but two comments for better api for shared helpers inline below.
> -Daniel
>
>> ---
>>  drivers/gpu/drm/i915/intel_dp_mst.c | 46 ++++++++++++++++++-------------------
>>  drivers/gpu/drm/i915/intel_drv.h    |  2 +-
>>  2 files changed, 24 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
>> index a2bd698..2e730b6 100644
>> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
>> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
>> @@ -113,7 +113,7 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder)
>>
>>       DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
>>
>> -     drm_dp_mst_reset_vcpi_slots(&intel_dp->mst_mgr, intel_mst->port);
>> +     drm_dp_mst_reset_vcpi_slots(&intel_dp->mst_mgr, intel_mst->connector->port);
>>
>>       ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
>>       if (ret) {
>> @@ -138,10 +138,12 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder)
>>       /* and this can also fail */
>>       drm_dp_update_payload_part2(&intel_dp->mst_mgr);
>>
>> -     drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, intel_mst->port);
>> +     drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, intel_mst->connector->port);
>>
>>       intel_dp->active_mst_links--;
>> -     intel_mst->port = NULL;
>> +
>> +     drm_connector_unreference(&intel_mst->connector->base);
>> +     intel_mst->connector = NULL;
>>       if (intel_dp->active_mst_links == 0) {
>>               intel_dig_port->base.post_disable(&intel_dig_port->base);
>>               intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>> @@ -181,7 +183,9 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder)
>>       found->encoder = encoder;
>>
>>       DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
>> -     intel_mst->port = found->port;
>> +
>> +     intel_mst->connector = found;
>> +     drm_connector_reference(&intel_mst->connector->base);
>>
>>       if (intel_dp->active_mst_links == 0) {
>>               intel_prepare_ddi_buffer(&intel_dig_port->base);
>> @@ -199,7 +203,7 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder)
>>       }
>>
>>       ret = drm_dp_mst_allocate_vcpi(&intel_dp->mst_mgr,
>> -                                    intel_mst->port,
>> +                                    intel_mst->connector->port,
>>                                      intel_crtc->config->pbn, &slots);
>>       if (ret == false) {
>>               DRM_ERROR("failed to allocate vcpi\n");
>> @@ -248,7 +252,7 @@ static bool intel_dp_mst_enc_get_hw_state(struct intel_encoder *encoder,
>>  {
>>       struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
>>       *pipe = intel_mst->pipe;
>> -     if (intel_mst->port)
>> +     if (intel_mst->connector)
>>               return true;
>>       return false;
>>  }
>> @@ -312,6 +316,11 @@ static int intel_dp_mst_get_ddc_modes(struct drm_connector *connector)
>>       struct edid *edid;
>>       int ret;
>>
>> +     if (!intel_connector->port || !intel_dp) {
>> +             ret = intel_connector_update_modes(connector, NULL);
>> +             return ret;
>> +     }
>> +
>>       edid = drm_dp_mst_get_edid(connector, &intel_dp->mst_mgr, intel_connector->port);
>>       if (!edid)
>>               return 0;
>> @@ -328,6 +337,8 @@ 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_connector->port || !intel_dp)
>> +             return connector_status_disconnected;
>>       return drm_dp_mst_detect_port(connector, &intel_dp->mst_mgr, intel_connector->port);
>
> Should we push this change and the preceeding one into dp helpers, i.e.
> make them cope with a null port? Otoh more work to fish out the
> ->mst_mgr, so not sure.

Actually they cope with a NULL port fine, so we can drop a bit of
those two hunks,

>
> Hm ... isn't the mst_mgr a redundant argument, and we could figure that
> out purely from the port? Would be a bit of refactoring, but I think the
> shared code for this crucial bit of semantics (there's no way the mst port
> is ever connected if it's unplugged) would be good.

No the port isn't a reference counted object, it's just a point we
revalidate in the
mst code. The MST manager does't disappear. So you can't get from port
to anything
else until you check it's valid. You can't reference count port on the
connector or
else you just end up with circular references between the two.


>> -     drm_modeset_unlock_all(dev);
>> -
>>       intel_connector->unregister(intel_connector);
>>
>>       drm_modeset_lock_all(dev);
>>       intel_connector_remove_from_fbdev(intel_connector);
>> -     drm_connector_cleanup(connector);
>> +     intel_connector->mst_port = NULL;
>>       drm_modeset_unlock_all(dev);
>
> Hm, ugly that we still need to grab modeset locks here. I'd like to avoid
> that, since it could be a major stall. Maybe we need to have a separate
> lock for the fbdev connector list, and then push the locking for that
> (including refcounting) down into fbdev helpers?

If you plug out an MST device a stall isn't going to be a major worry, you
are going to get a modeset most likely straight away.

So I think we should only address this problem when we have this problem.

Dave.
Daniel Vetter April 27, 2016, 6:29 a.m. UTC | #3
On Wed, Apr 27, 2016 at 11:54:35AM +1000, Dave Airlie wrote:
> On 22 April 2016 at 19:03, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Fri, Apr 15, 2016 at 03:10:45PM +1000, Dave Airlie wrote:
> >> From: Dave Airlie <airlied@redhat.com>
> >>
> >> Don't just free the connector when we get the destroy callback.
> >>
> >> Drop a reference to it, and set it's mst_port to NULL so
> >> no more mst work is done on it.
> >>
> >> Signed-off-by: Dave Airlie <airlied@redhat.com>
> >
> > Looks correct, but two comments for better api for shared helpers inline below.
> > -Daniel
> >
> >> ---
> >>  drivers/gpu/drm/i915/intel_dp_mst.c | 46 ++++++++++++++++++-------------------
> >>  drivers/gpu/drm/i915/intel_drv.h    |  2 +-
> >>  2 files changed, 24 insertions(+), 24 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> >> index a2bd698..2e730b6 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> >> @@ -113,7 +113,7 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder)
> >>
> >>       DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
> >>
> >> -     drm_dp_mst_reset_vcpi_slots(&intel_dp->mst_mgr, intel_mst->port);
> >> +     drm_dp_mst_reset_vcpi_slots(&intel_dp->mst_mgr, intel_mst->connector->port);
> >>
> >>       ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
> >>       if (ret) {
> >> @@ -138,10 +138,12 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder)
> >>       /* and this can also fail */
> >>       drm_dp_update_payload_part2(&intel_dp->mst_mgr);
> >>
> >> -     drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, intel_mst->port);
> >> +     drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, intel_mst->connector->port);
> >>
> >>       intel_dp->active_mst_links--;
> >> -     intel_mst->port = NULL;
> >> +
> >> +     drm_connector_unreference(&intel_mst->connector->base);
> >> +     intel_mst->connector = NULL;
> >>       if (intel_dp->active_mst_links == 0) {
> >>               intel_dig_port->base.post_disable(&intel_dig_port->base);
> >>               intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> >> @@ -181,7 +183,9 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder)
> >>       found->encoder = encoder;
> >>
> >>       DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
> >> -     intel_mst->port = found->port;
> >> +
> >> +     intel_mst->connector = found;
> >> +     drm_connector_reference(&intel_mst->connector->base);
> >>
> >>       if (intel_dp->active_mst_links == 0) {
> >>               intel_prepare_ddi_buffer(&intel_dig_port->base);
> >> @@ -199,7 +203,7 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder)
> >>       }
> >>
> >>       ret = drm_dp_mst_allocate_vcpi(&intel_dp->mst_mgr,
> >> -                                    intel_mst->port,
> >> +                                    intel_mst->connector->port,
> >>                                      intel_crtc->config->pbn, &slots);
> >>       if (ret == false) {
> >>               DRM_ERROR("failed to allocate vcpi\n");
> >> @@ -248,7 +252,7 @@ static bool intel_dp_mst_enc_get_hw_state(struct intel_encoder *encoder,
> >>  {
> >>       struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
> >>       *pipe = intel_mst->pipe;
> >> -     if (intel_mst->port)
> >> +     if (intel_mst->connector)
> >>               return true;
> >>       return false;
> >>  }
> >> @@ -312,6 +316,11 @@ static int intel_dp_mst_get_ddc_modes(struct drm_connector *connector)
> >>       struct edid *edid;
> >>       int ret;
> >>
> >> +     if (!intel_connector->port || !intel_dp) {
> >> +             ret = intel_connector_update_modes(connector, NULL);
> >> +             return ret;
> >> +     }
> >> +
> >>       edid = drm_dp_mst_get_edid(connector, &intel_dp->mst_mgr, intel_connector->port);
> >>       if (!edid)
> >>               return 0;
> >> @@ -328,6 +337,8 @@ 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_connector->port || !intel_dp)
> >> +             return connector_status_disconnected;
> >>       return drm_dp_mst_detect_port(connector, &intel_dp->mst_mgr, intel_connector->port);
> >
> > Should we push this change and the preceeding one into dp helpers, i.e.
> > make them cope with a null port? Otoh more work to fish out the
> > ->mst_mgr, so not sure.
> 
> Actually they cope with a NULL port fine, so we can drop a bit of
> those two hunks,
> 
> >
> > Hm ... isn't the mst_mgr a redundant argument, and we could figure that
> > out purely from the port? Would be a bit of refactoring, but I think the
> > shared code for this crucial bit of semantics (there's no way the mst port
> > is ever connected if it's unplugged) would be good.
> 
> No the port isn't a reference counted object, it's just a point we
> revalidate in the
> mst code. The MST manager does't disappear. So you can't get from port
> to anything
> else until you check it's valid. You can't reference count port on the
> connector or
> else you just end up with circular references between the two.

Yeah I think this is beyond my understanding of the mst helpers. Really
should fix that sometimes ;-)

> >> -     drm_modeset_unlock_all(dev);
> >> -
> >>       intel_connector->unregister(intel_connector);
> >>
> >>       drm_modeset_lock_all(dev);
> >>       intel_connector_remove_from_fbdev(intel_connector);
> >> -     drm_connector_cleanup(connector);
> >> +     intel_connector->mst_port = NULL;
> >>       drm_modeset_unlock_all(dev);
> >
> > Hm, ugly that we still need to grab modeset locks here. I'd like to avoid
> > that, since it could be a major stall. Maybe we need to have a separate
> > lock for the fbdev connector list, and then push the locking for that
> > (including refcounting) down into fbdev helpers?
> 
> If you plug out an MST device a stall isn't going to be a major worry, you
> are going to get a modeset most likely straight away.
> 
> So I think we should only address this problem when we have this problem.

Well it's more general unhappiness with how fbdev locking piggy-packs on
top of drm_modeset_lock_all(). At least ime reusing BKLs because they're
there already sooner or later leads to headaches - having separate locks
for separate things makes review easier. Avoid the stall is just a benefit
on top. Anyway, was just an idea.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index a2bd698..2e730b6 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -113,7 +113,7 @@  static void intel_mst_disable_dp(struct intel_encoder *encoder)
 
 	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
 
-	drm_dp_mst_reset_vcpi_slots(&intel_dp->mst_mgr, intel_mst->port);
+	drm_dp_mst_reset_vcpi_slots(&intel_dp->mst_mgr, intel_mst->connector->port);
 
 	ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
 	if (ret) {
@@ -138,10 +138,12 @@  static void intel_mst_post_disable_dp(struct intel_encoder *encoder)
 	/* and this can also fail */
 	drm_dp_update_payload_part2(&intel_dp->mst_mgr);
 
-	drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, intel_mst->port);
+	drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, intel_mst->connector->port);
 
 	intel_dp->active_mst_links--;
-	intel_mst->port = NULL;
+
+	drm_connector_unreference(&intel_mst->connector->base);
+	intel_mst->connector = NULL;
 	if (intel_dp->active_mst_links == 0) {
 		intel_dig_port->base.post_disable(&intel_dig_port->base);
 		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
@@ -181,7 +183,9 @@  static void intel_mst_pre_enable_dp(struct intel_encoder *encoder)
 	found->encoder = encoder;
 
 	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
-	intel_mst->port = found->port;
+
+	intel_mst->connector = found;
+	drm_connector_reference(&intel_mst->connector->base);
 
 	if (intel_dp->active_mst_links == 0) {
 		intel_prepare_ddi_buffer(&intel_dig_port->base);
@@ -199,7 +203,7 @@  static void intel_mst_pre_enable_dp(struct intel_encoder *encoder)
 	}
 
 	ret = drm_dp_mst_allocate_vcpi(&intel_dp->mst_mgr,
-				       intel_mst->port,
+				       intel_mst->connector->port,
 				       intel_crtc->config->pbn, &slots);
 	if (ret == false) {
 		DRM_ERROR("failed to allocate vcpi\n");
@@ -248,7 +252,7 @@  static bool intel_dp_mst_enc_get_hw_state(struct intel_encoder *encoder,
 {
 	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
 	*pipe = intel_mst->pipe;
-	if (intel_mst->port)
+	if (intel_mst->connector)
 		return true;
 	return false;
 }
@@ -312,6 +316,11 @@  static int intel_dp_mst_get_ddc_modes(struct drm_connector *connector)
 	struct edid *edid;
 	int ret;
 
+	if (!intel_connector->port || !intel_dp) {
+		ret = intel_connector_update_modes(connector, NULL);
+		return ret;
+	}
+
 	edid = drm_dp_mst_get_edid(connector, &intel_dp->mst_mgr, intel_connector->port);
 	if (!edid)
 		return 0;
@@ -328,6 +337,8 @@  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_connector->port || !intel_dp)
+		return connector_status_disconnected;
 	return drm_dp_mst_detect_port(connector, &intel_dp->mst_mgr, intel_connector->port);
 }
 
@@ -393,6 +404,8 @@  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)
+		return NULL;
 	return &intel_dp->mst_encoders[crtc->pipe]->base.base;
 }
 
@@ -400,6 +413,8 @@  static struct drm_encoder *intel_mst_best_encoder(struct drm_connector *connecto
 {
 	struct intel_connector *intel_connector = to_intel_connector(connector);
 	struct intel_dp *intel_dp = intel_connector->mst_port;
+	if (!intel_dp)
+		return NULL;
 	return &intel_dp->mst_encoders[0]->base.base;
 }
 
@@ -506,29 +521,14 @@  static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
 	struct intel_connector *intel_connector = to_intel_connector(connector);
 	struct drm_device *dev = connector->dev;
 
-	/* need to nuke the connector */
-	drm_modeset_lock_all(dev);
-	if (connector->state->crtc) {
-		struct drm_mode_set set;
-		int ret;
-
-		memset(&set, 0, sizeof(set));
-		set.crtc = connector->state->crtc,
-
-		ret = drm_atomic_helper_set_config(&set);
-
-		WARN(ret, "Disabling mst crtc failed with %i\n", ret);
-	}
-	drm_modeset_unlock_all(dev);
-
 	intel_connector->unregister(intel_connector);
 
 	drm_modeset_lock_all(dev);
 	intel_connector_remove_from_fbdev(intel_connector);
-	drm_connector_cleanup(connector);
+	intel_connector->mst_port = NULL;
 	drm_modeset_unlock_all(dev);
 
-	kfree(intel_connector);
+	drm_connector_unreference(&intel_connector->base);
 	DRM_DEBUG_KMS("\n");
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4c027d6..0d2360e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -831,7 +831,7 @@  struct intel_dp_mst_encoder {
 	struct intel_encoder base;
 	enum pipe pipe;
 	struct intel_digital_port *primary;
-	void *port; /* store this opaque as its illegal to dereference it */
+	struct intel_connector *connector;
 };
 
 static inline enum dpio_channel