| Message ID | 20240329011254.24160-3-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Bigjoiner modeset sequence redesign and MST support | expand |
> -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville > Syrjala > Sent: Friday, March 29, 2024 6:43 AM > To: intel-gfx@lists.freedesktop.org > Subject: [PATCH 02/22] drm/i915: Fix intel_modeset_pipe_config_late() for > bigjoiner > > From: Ville Syrjälä <ville.syrjala@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@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@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
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); }