Re: [Intel-gfx] [PATCH 2/2] drm/i915/dp_mst: don't pull unregistered connectors into state

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Dec 15, 2022 at 03:51:50PM +0000, Simon Ser wrote:
> In intel_dp_mst_atomic_master_trans_check(), we pull connectors
> sharing the same DP-MST stream into the atomic state. However,
> if the connector is unregistered, this later fails with:
> 
>     [  559.425658] i915 0000:00:02.0: [drm:drm_atomic_helper_check_modeset] [CONNECTOR:378:DP-7] is not registered
> 
> Skip these unregistered connectors to allow user-space to turn them
> off.
> 
> Fixes part of this wlroots issue:
> https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/3407
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Lyude Paul <lyude@redhat.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index f773e117ebc4..70859a927a9d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -280,7 +280,8 @@ intel_dp_mst_atomic_master_trans_check(struct intel_connector *connector,
>  		struct intel_crtc *crtc;
>  
>  		if (connector_iter->mst_port != connector->mst_port ||
> -		    connector_iter == connector)
> +		    connector_iter == connector ||
> +		    connector_iter->base.registration_state == DRM_CONNECTOR_UNREGISTERED)
>  			continue;

We can't really do that. It would risk leaving slave transcoders
enabled while the master is undergoing a full modeset.

I think a couple of ways we could go about this:
- kill the registration check entirely/partially
  I think Imre already has some plans for the partial killing
  due to some type-c vs. pm firmware issues that also need force
  a full modeset
- relocate this stuff to happen after drm_atomic_helper_check_modeset()
  like we already do for eg. bigjoiner. IIRC this was discussed as an
  option when we added intel_dp_mst_atomic_master_trans_check() but
  I don't recall anymore why we specifically chose to do this from
  connector.atomic_check().

>  
>  		conn_iter_state = intel_atomic_get_digital_connector_state(state,
> -- 
> 2.39.0
> 

-- 
Ville Syrjälä
Intel