> -----Original Message-----
> From: Shankar, Uma <[email protected]>
> Sent: Thursday, June 11, 2026 10:10 PM
> To: Manna, Animesh <[email protected]>; intel-
> [email protected]; [email protected]
> Cc: Dibin Moolakadan Subrahmanian
> <[email protected]>; [email protected];
> Nikula, Jani <[email protected]>
> Subject: RE: [PATCH v8 07/20] drm/i915/cmtg: Set timings for CMTG by using
> transcoder timing helpers
>
>
>
> > -----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.
Added an explicit check in next version.
>
> > +
> > + 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.
enum set_timing_type {
MODESET = 0,
LRR
};
Added the above enum to intel_cmtg.h in next version. Hope that works.
Regards,
Animesh
>
> 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
