> -----Original Message-----
> From: Nikula, Jani <[email protected]>
> Sent: Thursday, February 5, 2026 2:17 PM
> To: Manna, Animesh <[email protected]>; intel-
> [email protected]; [email protected]
> Cc: Dibin Moolakadan Subrahmanian
> <[email protected]>; Manna, Animesh
> <[email protected]>
> Subject: Re: [PATCH v2 03/10] drm/i915/cmtg: set timings for cmtg
>
> On Tue, 03 Feb 2026, Animesh Manna <[email protected]> wrote:
> > Timing registers are separate for CMTG, read transcoder register and
> > program cmtg transcoder with those values.
> >
> > v2:
> > - Use sw state instead of reading directly from hardware. [Jani]
> > - Move set_timing later after encoder enable. [Dibin]
> >
> > Signed-off-by: Animesh Manna <[email protected]>
> > Signed-off-by: Dibin Moolakadan Subrahmanian
> > <[email protected]>
> > ---
> > drivers/gpu/drm/i915/display/intel_cmtg.c | 24 +++++++++
> > drivers/gpu/drm/i915/display/intel_cmtg.h | 1 +
> > .../gpu/drm/i915/display/intel_cmtg_regs.h | 7 +++
> > drivers/gpu/drm/i915/display/intel_display.c | 51 ++++++++++++-------
> > .../drm/i915/display/intel_display_types.h | 2 +
> > 5 files changed, 67 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cmtg.c
> > b/drivers/gpu/drm/i915/display/intel_cmtg.c
> > index f5364f5a848f..4220eeece07f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cmtg.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cmtg.c
> > @@ -207,3 +207,27 @@ 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 void intel_cmtg_set_timings(const struct intel_crtc_state
> > +*crtc_state) {
> > + struct intel_display *display = to_intel_display(crtc_state);
> > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > +
> > + intel_de_write(display, TRANS_HTOTAL_CMTG(cpu_transcoder), crtc-
> >cmtg.htotal);
> > + intel_de_write(display, TRANS_HBLANK_CMTG(cpu_transcoder), crtc-
> >cmtg.hblank);
> > + intel_de_write(display, TRANS_HSYNC_CMTG(cpu_transcoder), crtc-
> >cmtg.hsync);
> > + intel_de_write(display, TRANS_VTOTAL_CMTG(cpu_transcoder), crtc-
> >cmtg.vtotal);
> > + intel_de_write(display, TRANS_VBLANK_CMTG(cpu_transcoder), crtc-
> >cmtg.vblank);
> > + intel_de_write(display, TRANS_VSYNC_CMTG(cpu_transcoder),
> > +crtc->cmtg.vsync); }
> > +
> > +void intel_cmtg_enable(const struct intel_crtc_state *crtc_state) {
> > + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > +
> > + if (cpu_transcoder != TRANSCODER_A && cpu_transcoder !=
> TRANSCODER_B)
> > + return;
> > +
> > + intel_cmtg_set_timings(crtc_state);
> > +}
> > diff --git a/drivers/gpu/drm/i915/display/intel_cmtg.h
> > b/drivers/gpu/drm/i915/display/intel_cmtg.h
> > index bef2426b2787..b2bb60d160fa 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cmtg.h
> > +++ b/drivers/gpu/drm/i915/display/intel_cmtg.h
> > @@ -9,6 +9,7 @@
> > struct intel_display;
> > struct intel_crtc_state;
> >
> > +void intel_cmtg_enable(const struct intel_crtc_state *crtc_state);
> > void intel_cmtg_set_clk_select(const struct intel_crtc_state
> > *crtc_state); void intel_cmtg_sanitize(struct intel_display
> > *display);
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cmtg_regs.h
> > b/drivers/gpu/drm/i915/display/intel_cmtg_regs.h
> > index 8a767b659a23..eb24827d22f5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cmtg_regs.h
> > +++ b/drivers/gpu/drm/i915/display/intel_cmtg_regs.h
> > @@ -20,4 +20,11 @@
> > #define TRANS_CMTG_CTL_B _MMIO(0x6fb88)
> > #define CMTG_ENABLE REG_BIT(31)
> >
> > +#define TRANS_HTOTAL_CMTG(id) _MMIO(0x6F000 + (id) *
> 0x100)
>
> What is id? If it's a transcoder, please use trans for the param name.
Ok.
>
> > +#define TRANS_HBLANK_CMTG(id) _MMIO(0x6F004 + (id) *
> 0x100)
> > +#define TRANS_HSYNC_CMTG(id) _MMIO(0x6F008 + (id) *
> 0x100)
> > +#define TRANS_VTOTAL_CMTG(id) _MMIO(0x6F00C + (id) *
> 0x100)
> > +#define TRANS_VBLANK_CMTG(id) _MMIO(0x6F010 + (id) *
> 0x100)
> > +#define TRANS_VSYNC_CMTG(id) _MMIO(0x6F014 + (id) *
> 0x100)
> > +
> > #endif /* __INTEL_CMTG_REGS_H__ */
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 564d11925af3..976af9eb3c3a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -62,6 +62,7 @@
> > #include "intel_casf.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"
> > @@ -1722,6 +1723,9 @@ static void hsw_crtc_enable(struct
> intel_atomic_state *state,
> > intel_crtc_wait_for_next_vblank(wa_crtc);
> > }
> > }
> > +
> > + if (crtc->cmtg.enable)
> > + intel_cmtg_enable(new_crtc_state);
> > }
> >
> > static void ilk_crtc_disable(struct intel_atomic_state *state, @@
> > -2654,6 +2658,8 @@ static void intel_set_transcoder_timings(const struct
> intel_crtc_state *crtc_sta
> > 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;
> > + u32 trans_htotal_val, trans_hblank_val, trans_hsync_val;
> > + u32 trans_vtotal_val, trans_vblank_val, trans_vsync_val;
> >
> > drm_WARN_ON(display->drm, transcoder_is_dsi(cpu_transcoder));
> >
> > @@ -2702,15 +2708,15 @@ static void intel_set_transcoder_timings(const
> struct intel_crtc_state *crtc_sta
> > TRANS_VSYNCSHIFT(display, cpu_transcoder),
> > vsyncshift);
> >
> > - intel_de_write(display, TRANS_HTOTAL(display, cpu_transcoder),
> > - HACTIVE(adjusted_mode->crtc_hdisplay - 1) |
> > - HTOTAL(adjusted_mode->crtc_htotal - 1));
> > - intel_de_write(display, TRANS_HBLANK(display, cpu_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),
> > - HSYNC_START(adjusted_mode->crtc_hsync_start - 1) |
> > - HSYNC_END(adjusted_mode->crtc_hsync_end - 1));
> > + trans_htotal_val = HACTIVE(adjusted_mode->crtc_hdisplay - 1) |
> > + HTOTAL(adjusted_mode->crtc_htotal - 1);
> > + trans_hblank_val = HBLANK_START(adjusted_mode-
> >crtc_hblank_start - 1) |
> > + HBLANK_END(adjusted_mode->crtc_hblank_end -
> 1);
> > + trans_hsync_val = HSYNC_START(adjusted_mode->crtc_hsync_start -
> 1) |
> > + HSYNC_END(adjusted_mode->crtc_hsync_end - 1);
> > + intel_de_write(display, TRANS_HTOTAL(display, cpu_transcoder),
> trans_htotal_val);
> > + intel_de_write(display, TRANS_HBLANK(display, cpu_transcoder),
> trans_hblank_val);
> > + intel_de_write(display, TRANS_HSYNC(display, cpu_transcoder),
> > +trans_hsync_val);
> >
> > /*
> > * For platforms that always use VRR Timing Generator, the
> > VTOTAL.Vtotal @@ -2721,15 +2727,15 @@ 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),
> > - VACTIVE(crtc_vdisplay - 1) |
> > - VTOTAL(crtc_vtotal - 1));
> > - intel_de_write(display, TRANS_VBLANK(display, cpu_transcoder),
> > - VBLANK_START(crtc_vblank_start - 1) |
> > - VBLANK_END(crtc_vblank_end - 1));
> > - intel_de_write(display, TRANS_VSYNC(display, cpu_transcoder),
> > - VSYNC_START(adjusted_mode->crtc_vsync_start - 1) |
> > - VSYNC_END(adjusted_mode->crtc_vsync_end - 1));
> > + trans_vtotal_val = VACTIVE(crtc_vdisplay - 1) |
> > + VTOTAL(crtc_vtotal - 1);
> > + trans_vblank_val = VBLANK_START(crtc_vblank_start - 1) |
> > + VBLANK_END(crtc_vblank_end - 1);
> > + trans_vsync_val = VSYNC_START(adjusted_mode->crtc_vsync_start -
> 1) |
> > + VSYNC_END(adjusted_mode->crtc_vsync_end - 1);
> > + intel_de_write(display, TRANS_VTOTAL(display, cpu_transcoder),
> trans_vtotal_val);
> > + intel_de_write(display, TRANS_VBLANK(display, cpu_transcoder),
> trans_vblank_val);
> > + intel_de_write(display, TRANS_VSYNC(display, cpu_transcoder),
> > +trans_vsync_val);
> >
> > /* Workaround: when the EDP input selection is B, the VTOTAL_B
> must be
> > * programmed with the VTOTAL_EDP value. Same for VTOTAL_C.
> This is
> > @@ -2753,6 +2759,15 @@ static void intel_set_transcoder_timings(const
> struct intel_crtc_state *crtc_sta
> > intel_de_write(display,
> DP_MIN_HBLANK_CTL(cpu_transcoder),
> > crtc_state->min_hblank);
> > }
> > +
> > + if (crtc->cmtg.enable) {
> > + crtc->cmtg.htotal = trans_htotal_val;
> > + crtc->cmtg.hblank = trans_hblank_val;
> > + crtc->cmtg.hsync = trans_hsync_val;
> > + crtc->cmtg.vtotal = trans_vtotal_val;
> > + crtc->cmtg.vblank = trans_vblank_val;
> > + crtc->cmtg.vsync = trans_vsync_val;
> > + }
> > }
> >
> > static void intel_set_transcoder_timings_lrr(const struct
> > intel_crtc_state *crtc_state) diff --git
> > a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 1081615a14fb..defb54dd0bbe 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1572,6 +1572,8 @@ struct intel_crtc {
> >
> > struct {
> > bool enable;
> > + u32 htotal, hblank, hsync;
> > + u32 vtotal, vblank, vsync;
>
> Why are these stored in the crtc?
Cannot store in crtc_state store as it is const. Some adjustment is done before
writing to these registers.
So exact same register value need to keep for cmtg as it need to enable as
secondary mode after modeset.
Storing in Intel_crtc I felt is a way, but open for any better approach.
Regards,
Animesh
>
> > } cmtg;
> > };
>
> --
> Jani Nikula, Intel
