[PATCH 2/2] drm/amd/display: Drop reusing drm connector for MST

[Lyude Paul]

Almost there. First thing I should mention: removing the connector reusage
with this patch ended up exposing an issue that I think was getting papered
over before because of it. It ended up being rather trivial to fix though, so
I'll send the patch with more information on the issue/a fix for it in just a
moment. We should definitely make sure it gets included with this patch series
when it gets pushed.

In the mean time though, one nitpick:

On Tue, 2018-10-30 at 18:09 -0400, Jerry (Fangzhi) Zuo wrote:
> [why]
> It is not safe to keep existing connector while entire topology
> has been removed. Could lead potential impact to uapi.
> Entirely unregister all the connectors on the topology,
> and use a new set of connectors when the topology is plugged back
> on.
> 
> [How]
> Remove the drm connector entirely each time when the
> corresponding MST topology is gone.
> When hotunplug a connector (e.g., DP2)
> 1. Remove connector from userspace.
> 2. Drop it's reference.
> When hotplug back on:
> 1. Detect new topology, and create new connectors.
> 2. Notify userspace with sysfs hotplug event.
> 3. Reprobe new connectors, and reassign CRTC from old (e.g., DP2)
> to new (e.g., DP3) connector.
> 
> Signed-off-by: Jerry (Fangzhi) Zuo <Jerry.Zuo at amd.com>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h  |  2 --
>  .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 40 ++++---------------
> ---
>  2 files changed, 7 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index 101e122945a4..23f2d05cf07e 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -208,8 +208,6 @@ struct amdgpu_dm_connector {
>  	struct mutex hpd_lock;
>  
>  	bool fake_enable;
> -
> -	bool mst_connected;
>  };
>  
>  #define to_amdgpu_dm_connector(x) container_of(x, struct
> amdgpu_dm_connector, base)
> 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 744d97cc0d39..621afe39de13 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
> @@ -320,25 +320,6 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr
> *mgr,
>  	struct amdgpu_device *adev = dev->dev_private;
>  	struct amdgpu_dm_connector *aconnector;
>  	struct drm_connector *connector;
> -	struct drm_connector_list_iter conn_iter;
> -
> -	drm_connector_list_iter_begin(dev, &conn_iter);
> -	drm_for_each_connector_iter(connector, &conn_iter) {
> -		aconnector = to_amdgpu_dm_connector(connector);
> -		if (aconnector->mst_port == master
> -				&& !aconnector->port) {
> -			DRM_INFO("DM_MST: reusing connector: %p [id: %d]
> [master: %p]\n",
> -						aconnector, connector-
> >base.id, aconnector->mst_port);
> -
> -			aconnector->port = port;
> -			drm_connector_set_path_property(connector, pathprop);
> -
> -			drm_connector_list_iter_end(&conn_iter);
> -			aconnector->mst_connected = true;
> -			return &aconnector->base;
> -		}
> -	}
> -	drm_connector_list_iter_end(&conn_iter);
>  
>  	aconnector = kzalloc(sizeof(*aconnector), GFP_KERNEL);
>  	if (!aconnector)
> @@ -387,8 +368,6 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr
> *mgr,
>  	 */
>  	amdgpu_dm_connector_funcs_reset(connector);
>  
> -	aconnector->mst_connected = true;
> -
>  	DRM_INFO("DM_MST: added connector: %p [id: %d] [master: %p]\n",
>  			aconnector, connector->base.id, aconnector->mst_port);
>  
> @@ -400,6 +379,9 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr
> *mgr,
>  static void dm_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr
> *mgr,
>  					struct drm_connector *connector)
>  {
> +	struct amdgpu_dm_connector *master = container_of(mgr, struct
> amdgpu_dm_connector, mst_mgr);
> +	struct drm_device *dev = master->base.dev;
> +	struct amdgpu_device *adev = dev->dev_private;
>  	struct amdgpu_dm_connector *aconnector =
> to_amdgpu_dm_connector(connector);
>  
>  	DRM_INFO("DM_MST: Disabling connector: %p [id: %d] [master: %p]\n",
> @@ -413,7 +395,10 @@ static void dm_dp_destroy_mst_connector(struct
> drm_dp_mst_topology_mgr *mgr,
>  		aconnector->dc_sink = NULL;
>  	}
>  
> -	aconnector->mst_connected = false;
> +	drm_connector_unregister(connector);
> +	if (adev->mode_info.rfbdev)
> +		drm_fb_helper_remove_one_connector(&adev->mode_info.rfbdev-
> >helper, connector);
> +	drm_connector_unreference(connector);
This should be drm_connector_put(), drm_connector_unreference() is deprecated
(see the kernel doc comments for it).

With that change, and the patch I'm about to respond to this email with
included in the final series:

Reviewed-by: Lyude Paul <lyude at redhat.com>
>  }
>  
>  static void dm_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr)
> @@ -424,18 +409,10 @@ static void dm_dp_mst_hotplug(struct
> drm_dp_mst_topology_mgr *mgr)
>  	drm_kms_helper_hotplug_event(dev);
>  }
>  
> -static void dm_dp_mst_link_status_reset(struct drm_connector *connector)
> -{
> -	mutex_lock(&connector->dev->mode_config.mutex);
> -	drm_connector_set_link_status_property(connector,
> DRM_MODE_LINK_STATUS_BAD);
> -	mutex_unlock(&connector->dev->mode_config.mutex);
> -}
> -
>  static void dm_dp_mst_register_connector(struct drm_connector *connector)
>  {
>  	struct drm_device *dev = connector->dev;
>  	struct amdgpu_device *adev = dev->dev_private;
> -	struct amdgpu_dm_connector *aconnector =
> to_amdgpu_dm_connector(connector);
>  
>  	if (adev->mode_info.rfbdev)
>  		drm_fb_helper_add_one_connector(&adev->mode_info.rfbdev-
> >helper, connector);
> @@ -443,9 +420,6 @@ static void dm_dp_mst_register_connector(struct
> drm_connector *connector)
>  		DRM_ERROR("adev->mode_info.rfbdev is NULL\n");
>  
>  	drm_connector_register(connector);
> -
> -	if (aconnector->mst_connected)
> -		dm_dp_mst_link_status_reset(connector);
>  }
>  
>  static const struct drm_dp_mst_topology_cbs dm_mst_cbs = {
-- 
Cheers,
	Lyude Paul