Re: [Intel-gfx] [PATCH 08/12] drm/i915: Fix MSO vs. bigjoiner timings confusion

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

On Tue, Feb 15, 2022 at 08:32:04PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> When calculating pipe_mode and when doing readout we need
> to order our steps correctly.
> 
> 1. We start with adjusted_mode crtc timings being populated
>    with the transcoder timings (either via readout or
>    compute_config(). These will be per-segment for MSO.
> 2. For all other uses we want the full crtc timings so
>    we ask intel_splitter_adjust_timings() to expand
>    the per-segment numbers to their full glory
> 3. If bigjoiner is used we the divide the full numbers
>    down to per-pipe numbers using intel_bigjoiner_adjust_timings()
> 
> During readout we also have to reconstruct the adjusted_mode
> normal timings (ie. not the crtc_ stuff). These are supposed
> to reflect the full timings of the display. So we grab these
> between steps 2 and 3.
> 
> The "user" mode readout (mainly done for fastboot purposes)
> should be whatever mode the user would have used had they
> asked us to do a modeset. We want the full timings for this
> as the per-segment timings are not suppoesed to be user visible.
> Also the user mode normal timings hdisplay/vdisplay need to
> match PIPESRC (that is where we get our PIPESRC size
> we doing a modeset with a user supplied mode).
> 
> And we end up with
> - adjusted_mode normal timigns == full timings
> - adjusted_mode crtc timings == transcoder timings
>   (per-segment timings for MSO, full timings otherwise)
> - pipe_mode normal/crtc timings == pipe timings
>   (full timings divided by the number of bigjoiner pipes, if any)
> - user mode normal timings == full timings with
>   hdisplay/vdisplay replaced with PIPESRC size
> - user mode crtc timings == full timings
> 
> Yes, that is a lot of timings. One day we'll try to remove
> some of the ones we don't actually need to keep around...
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

Manasi

> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 50 +++++++++++++-------
>  1 file changed, 32 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 6ff58164929c..131be3bb8026 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2769,25 +2769,33 @@ static void intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state
>  	struct drm_display_mode *pipe_mode = &crtc_state->hw.pipe_mode;
>  	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
>  
> +	/*
> +	 * Start with the adjusted_mode crtc timings, which
> +	 * have been filled with the transcoder timings.
> +	 */
>  	drm_mode_copy(pipe_mode, adjusted_mode);
>  
> -	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);
> -
> -	if (crtc_state->splitter.enable) {
> -		intel_splitter_adjust_timings(crtc_state, pipe_mode);
> -
> -		intel_mode_from_crtc_timings(pipe_mode, pipe_mode);
> -		intel_mode_from_crtc_timings(adjusted_mode, pipe_mode);
> -	} else {
> -		intel_mode_from_crtc_timings(pipe_mode, pipe_mode);
> -		intel_mode_from_crtc_timings(adjusted_mode, adjusted_mode);
> -	}
> -
> -	intel_crtc_compute_pixel_rate(crtc_state);
> -
> -	drm_mode_copy(mode, adjusted_mode);
> +	/* Expand MSO per-segment transcoder timings to full */
> +	intel_splitter_adjust_timings(crtc_state, pipe_mode);
> +
> +	/*
> +	 * We want the full numbers in adjusted_mode normal timings,
> +	 * adjusted_mode crtc timings are left with the raw transcoder
> +	 * timings.
> +	 */
> +	intel_mode_from_crtc_timings(adjusted_mode, pipe_mode);
> +
> +	/* Populate the "user" mode with full numbers */
> +	drm_mode_copy(mode, pipe_mode);
> +	intel_mode_from_crtc_timings(mode, mode);
>  	mode->hdisplay = crtc_state->pipe_src_w << crtc_state->bigjoiner;
>  	mode->vdisplay = crtc_state->pipe_src_h;
> +
> +	/* Derive per-pipe timings in case bigjoiner is used */
> +	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);
> +	intel_mode_from_crtc_timings(pipe_mode, pipe_mode);
> +
> +	intel_crtc_compute_pixel_rate(crtc_state);
>  }
>  
>  static void intel_encoder_get_config(struct intel_encoder *encoder,
> @@ -2836,15 +2844,21 @@ static int intel_crtc_compute_pipe_mode(struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
>  	struct drm_display_mode *pipe_mode = &crtc_state->hw.pipe_mode;
>  	int clock_limit = i915->max_dotclk_freq;
>  
> -	drm_mode_copy(pipe_mode, &crtc_state->hw.adjusted_mode);
> -
> -	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);
> +	/*
> +	 * Start with the adjusted_mode crtc timings, which
> +	 * have been filled with the transcoder timings.
> +	 */
> +	drm_mode_copy(pipe_mode, adjusted_mode);
>  
> +	/* Expand MSO per-segment transcoder timings to full */
>  	intel_splitter_adjust_timings(crtc_state, pipe_mode);
>  
> +	/* Derive per-pipe timings in case bigjoiner is used */
> +	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);
>  	intel_mode_from_crtc_timings(pipe_mode, pipe_mode);
>  
>  	if (DISPLAY_VER(i915) < 4) {
> -- 
> 2.34.1
>