[PATCH 02/22] drm/i915: Fix intel_modeset_pipe_config_late() for bigjoiner

[Kulkarni, Vandita]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Friday, March 29, 2024 6:43 AM
> To: intel-gfx at lists.freedesktop.org
> Subject: [PATCH 02/22] drm/i915: Fix intel_modeset_pipe_config_late() for
> bigjoiner
> 
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Currently intel_modeset_pipe_config_late() is called after the bigjoiner state
> copy, and it will actually not do anything for bigjoiner slaves. This can lead to
> a mismatched state between the master and slave.
> 
> The two things that we do in the encoder .compute_config_late() hook are
> mst master transcoder and port sync master transcoder elections. So if either
> of either MST or port sync is combined with bigjoiner then we can see the
> mismatch.
> 
> Currently this problem is more or less theoretical; MST+bigjoiner has not
> been implemented yet, and port sync+bigjoiner would require a tiled display
> with >5k tiles (or a very high dotclock per tile). Although we do have
> kms_tiled_display in igt which can fake a tiled display, and we can now force
> bigjoiner via debugfs, so it is possible to trigger this if you try hard enough.
> 
> Reorder the code such that intel_modeset_pipe_config_late() will be called
> before the bigjoiner state copy happens so that both pipes will end up with
> the same state.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

As we are already calling out on not being able to support port sync + bigjoiner  not being able  to support as of now in the patch 1, this change looks good to me.

Reviewed-by: Vandita Kulkarni <vandita.kulkarni at intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 46 ++++++++++++++------
>  1 file changed, 32 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 614e60420a29..08705042b4f8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4753,8 +4753,6 @@ intel_modeset_pipe_config_late(struct
> intel_atomic_state *state,
>  	struct drm_connector *connector;
>  	int i;
> 
> -	intel_bigjoiner_adjust_pipe_src(crtc_state);
> -
>  	for_each_new_connector_in_state(&state->base, connector,
>  					conn_state, i) {
>  		struct intel_encoder *encoder =
> @@ -6248,27 +6246,37 @@ static int intel_atomic_check_config(struct
> intel_atomic_state *state,
>  			continue;
>  		}
> 
> -		if (intel_crtc_is_bigjoiner_slave(new_crtc_state)) {
> -			drm_WARN_ON(&i915->drm, new_crtc_state-
> >uapi.enable);
> +		if (drm_WARN_ON(&i915->drm,
> +intel_crtc_is_bigjoiner_slave(new_crtc_state)))
>  			continue;
> -		}
> 
>  		ret = intel_crtc_prepare_cleared_state(state, crtc);
>  		if (ret)
> -			break;
> +			goto fail;
> 
>  		if (!new_crtc_state->hw.enable)
>  			continue;
> 
>  		ret = intel_modeset_pipe_config(state, crtc, limits);
>  		if (ret)
> -			break;
> +			goto fail;
> +	}
> 
> -		ret = intel_atomic_check_bigjoiner(state, crtc);
> +	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> +		if (!intel_crtc_needs_modeset(new_crtc_state))
> +			continue;
> +
> +		if (drm_WARN_ON(&i915->drm,
> intel_crtc_is_bigjoiner_slave(new_crtc_state)))
> +			continue;
> +
> +		if (!new_crtc_state->hw.enable)
> +			continue;
> +
> +		ret = intel_modeset_pipe_config_late(state, crtc);
>  		if (ret)
> -			break;
> +			goto fail;
>  	}
> 
> +fail:
>  	if (ret)
>  		*failed_pipe = crtc->pipe;
> 
> @@ -6364,16 +6372,26 @@ int intel_atomic_check(struct drm_device *dev,
>  	if (ret)
>  		goto fail;
> 
> +	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> +		if (!intel_crtc_needs_modeset(new_crtc_state))
> +			continue;
> +
> +		if (intel_crtc_is_bigjoiner_slave(new_crtc_state)) {
> +			drm_WARN_ON(&dev_priv->drm, new_crtc_state-
> >uapi.enable);
> +			continue;
> +		}
> +
> +		ret = intel_atomic_check_bigjoiner(state, crtc);
> +		if (ret)
> +			goto fail;
> +	}
> +
>  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>  					    new_crtc_state, i) {
>  		if (!intel_crtc_needs_modeset(new_crtc_state))
>  			continue;
> 
> -		if (new_crtc_state->hw.enable) {
> -			ret = intel_modeset_pipe_config_late(state, crtc);
> -			if (ret)
> -				goto fail;
> -		}
> +		intel_bigjoiner_adjust_pipe_src(new_crtc_state);
> 
>  		intel_crtc_check_fastset(old_crtc_state, new_crtc_state);
>  	}
> --
> 2.43.2