On Thu, Feb 03, 2022 at 08:38:16PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä <[email protected]> > > There's some weird junk in intel_atomic_check_bigjoiner() > that's trying to look at the old crtc state's bigjoiner > usage for some reason. That code is totally unnecessary, > and maybe even actively harmful. Not entirely sure which > since it's such a mess that I can't actually wrap my brain > around what it ends up doing. > > Either way, thanks to intel_bigjoiner_add_affected_crtcs() > all of the old bigjoiner crtcs are guaranteed to be in the > state already if any one of them is in the state. Also if > any one of those crtcs got flagged for a modeset, then all > of them will have been flagged, and the bigjoiner links > will have been detached via kill_bigjoiner_slave(). > > So there is no need to look examing any old bigjoiner > usage in intel_atomic_check_bigjoiner(). All we have to care > about is whether bigjoiner is needed for the new state, > and whether we can get the slave crtc we need. > > Signed-off-by: Ville Syrjälä <[email protected]>
Completely agree with this cleanup, makes it so much easier to add new future code Reviewed-by: Manasi Navare <[email protected]> Manasi > --- > drivers/gpu/drm/i915/display/intel_display.c | 33 +++++++------------- > 1 file changed, 11 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index 2006eec6e166..b5701ca57889 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -7584,38 +7584,28 @@ static bool intel_cpu_transcoders_need_modeset(struct > intel_atomic_state *state, > } > > static int intel_atomic_check_bigjoiner(struct intel_atomic_state *state, > - struct intel_crtc *crtc, > - struct intel_crtc_state *old_crtc_state, > - struct intel_crtc_state *new_crtc_state) > + struct intel_crtc *master_crtc) > { > struct drm_i915_private *i915 = to_i915(state->base.dev); > - struct intel_crtc_state *slave_crtc_state, *master_crtc_state; > - struct intel_crtc *slave_crtc, *master_crtc; > + struct intel_crtc_state *master_crtc_state = > + intel_atomic_get_new_crtc_state(state, master_crtc); > + struct intel_crtc_state *slave_crtc_state; > + struct intel_crtc *slave_crtc; > > - /* slave being enabled, is master is still claiming this crtc? */ > - if (old_crtc_state->bigjoiner_slave) { > - slave_crtc = crtc; > - master_crtc = old_crtc_state->bigjoiner_linked_crtc; > - master_crtc_state = intel_atomic_get_new_crtc_state(state, > master_crtc); > - if (!master_crtc_state || > !intel_crtc_needs_modeset(master_crtc_state)) > - goto claimed; > - } > - > - if (!new_crtc_state->bigjoiner) > + if (!master_crtc_state->bigjoiner) > return 0; > > - slave_crtc = intel_dsc_get_bigjoiner_secondary(crtc); > + slave_crtc = intel_dsc_get_bigjoiner_secondary(master_crtc); > if (!slave_crtc) { > drm_dbg_kms(&i915->drm, > "[CRTC:%d:%s] Big joiner configuration requires " > "CRTC + 1 to be used, doesn't exist\n", > - crtc->base.base.id, crtc->base.name); > + master_crtc->base.base.id, master_crtc->base.name); > return -EINVAL; > } > > - new_crtc_state->bigjoiner_linked_crtc = slave_crtc; > + master_crtc_state->bigjoiner_linked_crtc = slave_crtc; > slave_crtc_state = intel_atomic_get_crtc_state(&state->base, > slave_crtc); > - master_crtc = crtc; > if (IS_ERR(slave_crtc_state)) > return PTR_ERR(slave_crtc_state); > > @@ -7627,7 +7617,7 @@ static int intel_atomic_check_bigjoiner(struct > intel_atomic_state *state, > "[CRTC:%d:%s] Used as slave for big joiner\n", > slave_crtc->base.base.id, slave_crtc->base.name); > > - return copy_bigjoiner_crtc_state(slave_crtc_state, new_crtc_state); > + return copy_bigjoiner_crtc_state(slave_crtc_state, master_crtc_state); > > claimed: > drm_dbg_kms(&i915->drm, > @@ -7899,8 +7889,7 @@ static int intel_atomic_check(struct drm_device *dev, > if (ret) > goto fail; > > - ret = intel_atomic_check_bigjoiner(state, crtc, old_crtc_state, > - new_crtc_state); > + ret = intel_atomic_check_bigjoiner(state, crtc); > if (ret) > goto fail; > } > -- > 2.34.1 >
