On Mon, Oct 30, 2023 at 05:58:39PM +0200, Imre Deak wrote:
> Factor out a helper to clear the pipe update flags, used by a follow-up
> patch to modeset an MST topology.
> 
> v2:
> - Move the intel_crtc_needs_modeset() check to the callers. (Ville)
> 
> Reviewed-by: Stanislav Lisovskiy <stanislav.lisovs...@intel.com> (v1)
> Signed-off-by: Imre Deak <imre.d...@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 52 ++++++++++----------
>  1 file changed, 27 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 1133c2893bb38..d27afda0cfaa6 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5551,6 +5551,14 @@ int intel_modeset_pipes_in_mask_early(struct 
> intel_atomic_state *state,
>       return 0;
>  }
>  
> +static void
> +clear_pipe_update_flags_on_modeset_crtc(struct intel_crtc_state *crtc_state)

The 'on_modeset_crtc()' part doesn't make sense anymore.

Hmm. Another idea would be to move the mode_changed=true assignment
here and call this something like intel_crtc_flag_fmodeset(), which
might be a bit more descriptive.

> +{
> +     crtc_state->update_pipe = false;
> +     crtc_state->update_m_n = false;
> +     crtc_state->update_lrr = false;
> +}
> +
>  /**
>   * intel_modeset_all_pipes_late - force a full modeset on all pipes
>   * @state: intel atomic state
> @@ -5584,9 +5592,8 @@ int intel_modeset_all_pipes_late(struct 
> intel_atomic_state *state,
>               if (ret)
>                       return ret;
>  
> -             crtc_state->update_pipe = false;
> -             crtc_state->update_m_n = false;
> -             crtc_state->update_lrr = false;
> +             clear_pipe_update_flags_on_modeset_crtc(crtc_state);
> +
>               crtc_state->update_planes |= crtc_state->active_planes;
>               crtc_state->async_flip_planes = 0;
>               crtc_state->do_async_flip = false;
> @@ -5699,13 +5706,14 @@ static void intel_crtc_check_fastset(const struct 
> intel_crtc_state *old_crtc_sta
>       else
>               new_crtc_state->uapi.mode_changed = false;
>  
> -     if (intel_crtc_needs_modeset(new_crtc_state) ||
> -         intel_compare_link_m_n(&old_crtc_state->dp_m_n,
> +     if (intel_crtc_needs_modeset(new_crtc_state))
> +             clear_pipe_update_flags_on_modeset_crtc(new_crtc_state);
> +
> +     if (intel_compare_link_m_n(&old_crtc_state->dp_m_n,
>                                  &new_crtc_state->dp_m_n))
>               new_crtc_state->update_m_n = false;
>  
> -     if (intel_crtc_needs_modeset(new_crtc_state) ||
> -         (old_crtc_state->hw.adjusted_mode.crtc_vtotal == 
> new_crtc_state->hw.adjusted_mode.crtc_vtotal &&
> +     if ((old_crtc_state->hw.adjusted_mode.crtc_vtotal == 
> new_crtc_state->hw.adjusted_mode.crtc_vtotal &&
>            old_crtc_state->hw.adjusted_mode.crtc_vblank_end == 
> new_crtc_state->hw.adjusted_mode.crtc_vblank_end))
>               new_crtc_state->update_lrr = false;
>  
> @@ -6484,12 +6492,9 @@ int intel_atomic_check(struct drm_device *dev,
>               if (intel_dp_mst_is_slave_trans(new_crtc_state)) {
>                       enum transcoder master = 
> new_crtc_state->mst_master_transcoder;
>  
> -                     if (intel_cpu_transcoders_need_modeset(state, 
> BIT(master))) {
> -                             new_crtc_state->uapi.mode_changed = true;
> -                             new_crtc_state->update_pipe = false;
> -                             new_crtc_state->update_m_n = false;
> -                             new_crtc_state->update_lrr = false;
> -                     }
> +                     if (intel_cpu_transcoders_need_modeset(state, 
> BIT(master)))
> +                             intel_modeset_pipes_in_mask_early(state, "MST 
> master transcoder",
> +                                                               
> BIT(crtc->pipe));

These changes don't really belong in this refactoring patch.

>               }
>  
>               if (is_trans_port_sync_mode(new_crtc_state)) {
> @@ -6498,22 +6503,19 @@ int intel_atomic_check(struct drm_device *dev,
>                       if (new_crtc_state->master_transcoder != 
> INVALID_TRANSCODER)
>                               trans |= BIT(new_crtc_state->master_transcoder);
>  
> -                     if (intel_cpu_transcoders_need_modeset(state, trans)) {
> -                             new_crtc_state->uapi.mode_changed = true;
> -                             new_crtc_state->update_pipe = false;
> -                             new_crtc_state->update_m_n = false;
> -                             new_crtc_state->update_lrr = false;
> -                     }
> +                     if (intel_cpu_transcoders_need_modeset(state, trans))
> +                             intel_modeset_pipes_in_mask_early(state, "port 
> sync",
> +                                                               
> BIT(crtc->pipe));
>               }
>  
>               if (new_crtc_state->bigjoiner_pipes) {
> -                     if (intel_pipes_need_modeset(state, 
> new_crtc_state->bigjoiner_pipes)) {
> -                             new_crtc_state->uapi.mode_changed = true;
> -                             new_crtc_state->update_pipe = false;
> -                             new_crtc_state->update_m_n = false;
> -                             new_crtc_state->update_lrr = false;
> -                     }
> +                     if (intel_pipes_need_modeset(state, 
> new_crtc_state->bigjoiner_pipes))
> +                             intel_modeset_pipes_in_mask_early(state, 
> "bigjoiner pipes",
> +                                                               
> BIT(crtc->pipe));
>               }
> +
> +             if (intel_crtc_needs_modeset(new_crtc_state))
> +                     clear_pipe_update_flags_on_modeset_crtc(new_crtc_state);
>       }
>  
>       for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> -- 
> 2.39.2

-- 
Ville Syrjälä
Intel

Reply via email to