Re: [PATCH v8 04/20] drm/i915/display: Pass target transcoder to intel_set_transcoder_timings()

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Jun 04, 2026 at 01:24:00AM +0530, Animesh Manna wrote:
> Let intel_set_transcoder_timings() take the target transcoder as an
> explicit argument instead of always using crtc_state->cpu_transcoder.
> This makes the helper reusable for callers that need to program timings
> for a transcoder other than the CRTC's CPU transcoder.
> 
> Update all existing callers to pass crtc_state->cpu_transcoder so there
> is no functional change.
> 
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 35 ++++++++++----------
>  1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 8e269b71f18e..9031264a34fc 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -132,7 +132,8 @@
>  #include "vlv_dsi_pll.h"
>  #include "vlv_dsi_regs.h"
>  
> -static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_state);
> +static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_state,
> +					 enum transcoder transcoder);
>  static void intel_set_pipe_src_size(const struct intel_crtc_state *crtc_state);
>  static void hsw_set_transconf(const struct intel_crtc_state *crtc_state);
>  static void bdw_set_pipe_misc(struct intel_dsb *dsb,
> @@ -1504,7 +1505,7 @@ static void ilk_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta
>  					       &crtc_state->dp_m2_n2);
>  	}
>  
> -	intel_set_transcoder_timings(crtc_state);
> +	intel_set_transcoder_timings(crtc_state, crtc_state->cpu_transcoder);
>  
>  	ilk_set_pipeconf(crtc_state);
>  }
> @@ -1635,7 +1636,7 @@ static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta
>  					       &crtc_state->dp_m2_n2);
>  	}
>  
> -	intel_set_transcoder_timings(crtc_state);
> +	intel_set_transcoder_timings(crtc_state, crtc_state->cpu_transcoder);
>  
>  	if (cpu_transcoder != TRANSCODER_EDP)
>  		intel_de_write(display, TRANS_MULT(display, cpu_transcoder),
> @@ -2048,7 +2049,7 @@ static void i9xx_configure_cpu_transcoder(const struct intel_crtc_state *crtc_st
>  					       &crtc_state->dp_m2_n2);
>  	}
>  
> -	intel_set_transcoder_timings(crtc_state);
> +	intel_set_transcoder_timings(crtc_state, crtc_state->cpu_transcoder);
>  
>  	i9xx_set_pipeconf(crtc_state);
>  }
> @@ -2664,17 +2665,17 @@ transcoder_has_vrr(const struct intel_crtc_state *crtc_state)
>  	return HAS_VRR(display) && !transcoder_is_dsi(cpu_transcoder);
>  }
>  
> -static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_state)
> +static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_state,
> +					 enum transcoder transcoder)
>  {
>  	struct intel_display *display = to_intel_display(crtc_state);
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	enum pipe pipe = crtc->pipe;
> -	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
>  	u32 crtc_vdisplay, crtc_vtotal, crtc_vblank_start, crtc_vblank_end;
>  	int vsyncshift = 0;
>  
> -	drm_WARN_ON(display->drm, transcoder_is_dsi(cpu_transcoder));
> +	drm_WARN_ON(display->drm, transcoder_is_dsi(transcoder));

If you want to rename the variable, then do so in a separate patch.
Mixing the rename with other changes is destroying the signal to
noise ratio of the patch.

>  
>  	/* We need to be careful not to changed the adjusted mode, for otherwise
>  	 * the hw state checker will get angry at the mismatch. */
> @@ -2703,7 +2704,7 @@ static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_sta
>  	 */
>  	if (DISPLAY_VER(display) >= 13) {
>  		intel_de_write(display,
> -			       TRANS_SET_CONTEXT_LATENCY(display, cpu_transcoder),
> +			       TRANS_SET_CONTEXT_LATENCY(display, transcoder),
>  			       crtc_state->set_context_latency);
>  
>  		/*
> @@ -2718,16 +2719,16 @@ static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_sta
>  
>  	if (DISPLAY_VER(display) >= 4 && DISPLAY_VER(display) < 35)
>  		intel_de_write(display,
> -			       TRANS_VSYNCSHIFT(display, cpu_transcoder),
> +			       TRANS_VSYNCSHIFT(display, transcoder),
>  			       vsyncshift);
>  
> -	intel_de_write(display, TRANS_HTOTAL(display, cpu_transcoder),
> +	intel_de_write(display, TRANS_HTOTAL(display, transcoder),
>  		       HACTIVE(adjusted_mode->crtc_hdisplay - 1) |
>  		       HTOTAL(adjusted_mode->crtc_htotal - 1));
> -	intel_de_write(display, TRANS_HBLANK(display, cpu_transcoder),
> +	intel_de_write(display, TRANS_HBLANK(display, transcoder),
>  		       HBLANK_START(adjusted_mode->crtc_hblank_start - 1) |
>  		       HBLANK_END(adjusted_mode->crtc_hblank_end - 1));
> -	intel_de_write(display, TRANS_HSYNC(display, cpu_transcoder),
> +	intel_de_write(display, TRANS_HSYNC(display, transcoder),
>  		       HSYNC_START(adjusted_mode->crtc_hsync_start - 1) |
>  		       HSYNC_END(adjusted_mode->crtc_hsync_end - 1));
>  
> @@ -2740,13 +2741,13 @@ static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_sta
>  	if (intel_vrr_always_use_vrr_tg(display))
>  		crtc_vtotal = 1;
>  
> -	intel_de_write(display, TRANS_VTOTAL(display, cpu_transcoder),
> +	intel_de_write(display, TRANS_VTOTAL(display, transcoder),
>  		       VACTIVE(crtc_vdisplay - 1) |
>  		       VTOTAL(crtc_vtotal - 1));
> -	intel_de_write(display, TRANS_VBLANK(display, cpu_transcoder),
> +	intel_de_write(display, TRANS_VBLANK(display, transcoder),
>  		       VBLANK_START(crtc_vblank_start - 1) |
>  		       VBLANK_END(crtc_vblank_end - 1));
> -	intel_de_write(display, TRANS_VSYNC(display, cpu_transcoder),
> +	intel_de_write(display, TRANS_VSYNC(display, transcoder),
>  		       VSYNC_START(adjusted_mode->crtc_vsync_start - 1) |
>  		       VSYNC_END(adjusted_mode->crtc_vsync_end - 1));
>  
> @@ -2754,7 +2755,7 @@ static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_sta
>  	 * programmed with the VTOTAL_EDP value. Same for VTOTAL_C. This is
>  	 * documented on the DDI_FUNC_CTL register description, EDP Input Select
>  	 * bits. */
> -	if (display->platform.haswell && cpu_transcoder == TRANSCODER_EDP &&
> +	if (display->platform.haswell && transcoder == TRANSCODER_EDP &&
>  	    (pipe == PIPE_B || pipe == PIPE_C))
>  		intel_de_write(display, TRANS_VTOTAL(display, pipe),
>  			       VACTIVE(crtc_vdisplay - 1) |
> @@ -2769,7 +2770,7 @@ static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_sta
>  		 * followed by BE which DPRX devices are unable to handle.
>  		 * https://groups.vesa.org/wg/DP/document/20494
>  		 */
> -		intel_de_write(display, DP_MIN_HBLANK_CTL(cpu_transcoder),
> +		intel_de_write(display, DP_MIN_HBLANK_CTL(transcoder),
>  			       crtc_state->min_hblank);
>  	}
>  }
> -- 
> 2.29.0

-- 
Ville Syrjälä
Intel