> -----Original Message-----
> From: Manna, Animesh <[email protected]>
> Sent: Thursday, June 4, 2026 1:24 AM
> To: [email protected]; [email protected]
> Cc: Shankar, Uma <[email protected]>; Dibin Moolakadan Subrahmanian
> <[email protected]>; [email protected];
> Nikula, Jani <[email protected]>; Manna, Animesh
> <[email protected]>
> Subject: [PATCH v8 07/20] drm/i915/cmtg: Set timings for CMTG by using
> transcoder timing helpers
>
> Expose intel_set_transcoder_timings() & intel_set_transcoder_timings_lrr()
> so that they can program timings on any transcoder, and use them from a new
> intel_cmtg_set_timings() helper instead of duplicating the timing register
> write
> sequence for CMTG.
>
> intel_cmtg_set_timings() maps the CPU transcoder to the corresponding CMTG
> transcoder (TRANSCODER_A->TRANSCODER_CMTG0, TRANSCODER_B->
> TRANSCODER_CMTG1) and calls the shared helper, gated by
> intel_cmtg_is_allowed(). It is invoked from hsw_configure_cpu_transcoder() for
> the full modeset path and from intel_pipe_fastset() for the LRR update path.
>
> v2:
> - Use sw state instead of reading directly from hardware. [Jani]
> - Move set_timing later after encoder enable. [Dibin]
>
> v3:
> - Replace id with trans. [Jani]
> - Program cmtg set_timing() along with primary transcoder timing.
>
> v4:
> - Use _MMIO_TRANS() for cmtg registers instead of direct multiplication.
> [Jani]
>
> v5:
> - Modify register definition approach and match existing transcoder
> definition.
> [Ville]
>
> v6:
> - Reuse transcoder timing helpers. [Ville]
>
> Bspec: 68989
> Signed-off-by: Animesh Manna <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_cmtg.c | 25 ++++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_cmtg.h | 1 +
> drivers/gpu/drm/i915/display/intel_display.c | 13 +++++-----
> drivers/gpu/drm/i915/display/intel_display.h | 4 ++++
> 4 files changed, 37 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cmtg.c
> b/drivers/gpu/drm/i915/display/intel_cmtg.c
> index fbc8a4f2b9cb..082c04bec9ee 100644
> --- a/drivers/gpu/drm/i915/display/intel_cmtg.c
> +++ b/drivers/gpu/drm/i915/display/intel_cmtg.c
> @@ -219,3 +219,28 @@ void intel_cmtg_set_clk_select(const struct
> intel_crtc_state *crtc_state)
> if (clk_sel_set)
> intel_de_rmw(display, CMTG_CLK_SEL, clk_sel_clr, clk_sel_set);
> }
> +
> +static inline enum transcoder to_cmtg_transcoder(enum transcoder
> +cpu_transcoder) {
> + switch (cpu_transcoder) {
> + case TRANSCODER_A:
> + return TRANSCODER_CMTG0;
> + case TRANSCODER_B:
> + return TRANSCODER_CMTG1;
> + default:
> + return INVALID_TRANSCODER;
> + }
> +}
> +
> +void intel_cmtg_set_timings(const struct intel_crtc_state *crtc_state,
> +bool lrr) {
> + enum transcoder cmtg_transcoder =
> +to_cmtg_transcoder(crtc_state->cpu_transcoder);
This can get INVALID_TRANSCODER, we should add protection for it.
Check below should help, but better to add an explicit check.
> +
> + if (!intel_cmtg_is_allowed(crtc_state))
> + return;
> +
> + if (lrr)
> + intel_set_transcoder_timings_lrr(crtc_state, cmtg_transcoder);
> + else
> + intel_set_transcoder_timings(crtc_state, cmtg_transcoder); }
> diff --git a/drivers/gpu/drm/i915/display/intel_cmtg.h
> b/drivers/gpu/drm/i915/display/intel_cmtg.h
> index 87092ce6d67b..53a44f505dd2 100644
> --- a/drivers/gpu/drm/i915/display/intel_cmtg.h
> +++ b/drivers/gpu/drm/i915/display/intel_cmtg.h
> @@ -11,6 +11,7 @@
> struct intel_display;
> struct intel_crtc_state;
>
> +void intel_cmtg_set_timings(const struct intel_crtc_state *crtc_state,
> +bool lrr);
> void intel_cmtg_set_clk_select(const struct intel_crtc_state *crtc_state);
> void
> intel_cmtg_sanitize(struct intel_display *display); bool
> intel_cmtg_is_allowed(const struct intel_crtc_state *crtc_state); diff --git
> a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 17621f66501f..a6a1da4bd98d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -60,6 +60,7 @@
> #include "intel_bw.h"
> #include "intel_cdclk.h"
> #include "intel_clock_gating.h"
> +#include "intel_cmtg.h"
> #include "intel_color.h"
> #include "intel_crt.h"
> #include "intel_crtc.h"
> @@ -132,8 +133,6 @@
> #include "vlv_dsi_pll.h"
> #include "vlv_dsi_regs.h"
>
> -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, @@ -1637,6 +1636,7 @@ static
> void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta
> }
>
> intel_set_transcoder_timings(crtc_state, crtc_state->cpu_transcoder);
> + intel_cmtg_set_timings(crtc_state, false);
>
> if (cpu_transcoder != TRANSCODER_EDP)
> intel_de_write(display, TRANS_MULT(display, cpu_transcoder),
> @@ -2665,8 +2665,8 @@ 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,
> - enum transcoder transcoder)
> +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);
> @@ -2777,8 +2777,8 @@ static void intel_set_transcoder_timings(const struct
> intel_crtc_state *crtc_sta
> }
> }
>
> -static void intel_set_transcoder_timings_lrr(const struct intel_crtc_state
> *crtc_state,
> - enum transcoder transcoder)
> +void intel_set_transcoder_timings_lrr(const struct intel_crtc_state
> *crtc_state,
> + enum transcoder transcoder)
> {
> struct intel_display *display = to_intel_display(crtc_state);
> const struct drm_display_mode *adjusted_mode = &crtc_state-
> >hw.adjusted_mode; @@ -6673,6 +6673,7 @@ static void
> intel_pipe_fastset(const struct intel_crtc_state *old_crtc_state,
>
> if (new_crtc_state->update_lrr) {
> intel_set_transcoder_timings_lrr(new_crtc_state, new_crtc_state-
> >cpu_transcoder);
> + intel_cmtg_set_timings(new_crtc_state, true);
Maybe instead of true and false here, an enum with explicit names can be
used as argument.
With above fixed, this is
Reviewed-by: Uma Shankar <[email protected]>
> intel_vrr_set_fixed_rr_timings(new_crtc_state);
> intel_vrr_transcoder_enable(new_crtc_state);
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h
> b/drivers/gpu/drm/i915/display/intel_display.h
> index 1963dbc80221..ef7e0506f77f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -424,6 +424,10 @@ void intel_set_m_n(struct intel_display *display,
> const struct intel_link_m_n *m_n,
> intel_reg_t data_m_reg, intel_reg_t data_n_reg,
> intel_reg_t link_m_reg, intel_reg_t link_n_reg);
> +void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_state,
> + enum transcoder transcoder);
> +void intel_set_transcoder_timings_lrr(const struct intel_crtc_state
> *crtc_state,
> + enum transcoder transcoder);
> void intel_get_m_n(struct intel_display *display,
> struct intel_link_m_n *m_n,
> intel_reg_t data_m_reg, intel_reg_t data_n_reg,
> --
> 2.29.0
