Re: [Intel-gfx] [PATCH] drm/i915/display: Use external dependency loop for port sync

[Manasi Navare <manasi.d.navare@intel.com>]

On Mon, Jan 06, 2020 at 06:28:23AM -0800, José Roberto de Souza wrote:
> This loop was added directly to intel_atomic_check() to be used by
> all other features that have external pipe dependencies, so using it
> and removing intel_atomic_check_synced_crtcs().
> 
> After this changes is_trans_port_sync_master() it not used anywhere,
> so removing it.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 69 +++++---------------
>  1 file changed, 17 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index da5266e76738..f48439d7290b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -553,13 +553,6 @@ is_trans_port_sync_mode(const struct intel_crtc_state *crtc_state)
>  		crtc_state->sync_mode_slaves_mask);
>  }
>  
> -static bool
> -is_trans_port_sync_master(const struct intel_crtc_state *crtc_state)
> -{
> -	return (crtc_state->master_transcoder == INVALID_TRANSCODER &&
> -		crtc_state->sync_mode_slaves_mask);
> -}
> -
>  static bool
>  is_trans_port_sync_slave(const struct intel_crtc_state *crtc_state)
>  {
> @@ -14370,31 +14363,6 @@ intel_modeset_synced_crtcs(struct intel_atomic_state *state,
>  	}
>  }
>  
> -static void
> -intel_atomic_check_synced_crtcs(struct intel_atomic_state *state)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -	struct intel_crtc_state *new_crtc_state;
> -	struct intel_crtc *crtc;
> -	int i;
> -
> -	if (INTEL_GEN(dev_priv) < 11)
> -		return;
> -
> -	for_each_new_intel_crtc_in_state(state, crtc,
> -					 new_crtc_state, i) {
> -		if (is_trans_port_sync_master(new_crtc_state) &&
> -		    needs_modeset(new_crtc_state)) {
> -			intel_modeset_synced_crtcs(state,
> -						   new_crtc_state->sync_mode_slaves_mask);
> -		} else if (is_trans_port_sync_slave(new_crtc_state) &&
> -			   needs_modeset(new_crtc_state)) {
> -			intel_modeset_synced_crtcs(state,
> -						   BIT(new_crtc_state->master_transcoder));
> -		}
> -	}
> -}
> -
>  static int
>  intel_modeset_all_tiles(struct intel_atomic_state *state, int tile_grp_id)
>  {
> @@ -14538,33 +14506,30 @@ static int intel_atomic_check(struct drm_device *dev,
>  	 *
>  	 * Right now it only forces a fullmodeset when the MST master
>  	 * transcoder did not changed but the pipe of the master transcoder
> -	 * needs a fullmodeset so all slaves also needs to do a fullmodeset.
> +	 * needs a fullmodeset so all slaves also needs to do a fullmodeset or
> +	 * in case of port synced crtcs, if one of the synced crtcs
> +	 * needs a full modeset, all other synced crtcs should be
> +	 * forced a full modeset.
>  	 */
>  	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> -		enum transcoder master = new_crtc_state->mst_master_transcoder;
> -
> -		if (!new_crtc_state->hw.enable ||
> -		    needs_modeset(new_crtc_state) ||
> -		    !intel_dp_mst_is_slave_trans(new_crtc_state))
> +		if (!new_crtc_state->hw.enable || needs_modeset(new_crtc_state))
>  			continue;
>  
> -		if (intel_cpu_transcoder_needs_modeset(state, master)) {
> -			new_crtc_state->uapi.mode_changed = true;
> -			new_crtc_state->update_pipe = false;
> +		if (intel_dp_mst_is_slave_trans(new_crtc_state)) {
> +			enum transcoder master = new_crtc_state->mst_master_transcoder;
> +
> +			if (intel_cpu_transcoder_needs_modeset(state, master)) {
> +				new_crtc_state->uapi.mode_changed = true;
> +				new_crtc_state->update_pipe = false;
> +			}
> +		} else if (is_trans_port_sync_mode(new_crtc_state)) {
> +			u8 trans = new_crtc_state->sync_mode_slaves_mask |
> +				   BIT(new_crtc_state->master_transcoder);
> +
> +			intel_modeset_synced_crtcs(state, trans);
>  		}
>  	}
>  
> -	/**
> -	 * In case of port synced crtcs, if one of the synced crtcs
> -	 * needs a full modeset, all other synced crtcs should be
> -	 * forced a full modeset. This checks if fastset is allowed
> -	 * by other dependencies like the synced crtcs.
> -	 * Here we set the mode_changed to true directly to force full
> -	 * modeset hence we do not explicitly call the function
> -	 * drm_atomic_helper_check_modeset().
> -	 */
> -	intel_atomic_check_synced_crtcs(state);
> -
>  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>  					    new_crtc_state, i) {
>  		if (needs_modeset(new_crtc_state)) {
> -- 
> 2.24.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx