> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Ville
> Syrjala
> Sent: Friday, March 29, 2024 6:43 AM
> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject: [PATCH 02/22] drm/i915: Fix intel_modeset_pipe_config_late() for
> bigjoiner
>
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx>
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@xxxxxxxxx>
> ---
> 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
