On Thu, Feb 03, 2022 at 08:38:16PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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ä <ville.syrjala@xxxxxxxxxxxxxxx>
Completely agree with this cleanup, makes it so much easier to add new future code
Reviewed-by: Manasi Navare <manasi.d.navare@xxxxxxxxx>
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
>
