Re: [PATCH v4 03/13] drm/i915/cmtg: Set timings for CMTG

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

On Fri, Apr 17, 2026 at 06:03:52AM +0000, Manna, Animesh wrote:
> 
> 
> > -----Original Message-----
> > From: Nikula, Jani <jani.nikula@intel.com>
> > Sent: Tuesday, April 14, 2026 7:03 PM
> > To: Manna, Animesh <animesh.manna@intel.com>; intel-
> > gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org
> > Cc: Shankar, Uma <uma.shankar@intel.com>; Dibin Moolakadan
> > Subrahmanian <dibin.moolakadan.subrahmanian@intel.com>; Manna,
> > Animesh <animesh.manna@intel.com>
> > Subject: Re: [PATCH v4 03/13] drm/i915/cmtg: Set timings for CMTG
> > 
> > On Sun, 12 Apr 2026, Animesh Manna <animesh.manna@intel.com> 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]
> > >
> > > 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]
> > >
> > > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_cmtg.c     | 61 ++++++++++++++++++-
> > >  drivers/gpu/drm/i915/display/intel_cmtg.h     |  3 +
> > >  .../gpu/drm/i915/display/intel_cmtg_regs.h    | 31 ++++++++++
> > >  drivers/gpu/drm/i915/display/intel_display.c  |  4 ++
> > >  4 files changed, 98 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cmtg.c
> > > b/drivers/gpu/drm/i915/display/intel_cmtg.c
> > > index 403f9e10a8dc..a3db1368bd83 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cmtg.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_cmtg.c
> > > @@ -4,7 +4,6 @@
> > >   */
> > >
> > >  #include <linux/string_choices.h>
> > > -#include <linux/types.h>
> > >
> > >  #include <drm/drm_device.h>
> > >  #include <drm/drm_print.h>
> > > @@ -222,3 +221,63 @@ 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);  }
> > > +
> > > +void intel_cmtg_set_timings(const struct intel_crtc_state
> > > +*crtc_state, bool lrr) {
> > > +	struct intel_display *display = to_intel_display(crtc_state);
> > > +	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;
> > > +
> > > +	if (!intel_cmtg_is_allowed(crtc_state))
> > > +		return;
> > > +
> > > +	crtc_vdisplay = adjusted_mode->crtc_vdisplay;
> > > +
> > > +	/*
> > > +	 * For platforms that always use VRR Timing Generator, the
> > VTOTAL.Vtotal
> > > +	 * bits are not required. Since the support for these bits is going to
> > > +	 * be deprecated in upcoming platforms, avoid writing these bits for
> > the
> > > +	 * platforms that do not use legacy Timing Generator.
> > > +	 */
> > > +	crtc_vtotal = 1;
> > > +
> > > +	/*
> > > +	 * VBLANK_START not used by hw, just clear it
> > > +	 * to make it stand out in register dumps.
> > > +	 */
> > > +	crtc_vblank_start = 1;
> > > +
> > > +	crtc_vblank_end = adjusted_mode->crtc_vblank_end;
> > > +
> > > +	if (lrr) {
> > > +		intel_de_write(display,
> > TRANS_VTOTAL_CMTG(cpu_transcoder),
> > > +			       VACTIVE(crtc_vdisplay - 1) |
> > > +			       VTOTAL(crtc_vtotal - 1));
> > > +		intel_de_write(display,
> > TRANS_VBLANK_CMTG(cpu_transcoder),
> > > +			       VBLANK_START(crtc_vblank_start - 1) |
> > > +			       VBLANK_END(crtc_vblank_end - 1));
> > > +		return;
> > > +	}
> > > +
> > > +	intel_de_write(display, TRANS_HTOTAL_CMTG(cpu_transcoder),
> > > +		       HACTIVE(adjusted_mode->crtc_hdisplay - 1) |
> > > +		       HTOTAL(adjusted_mode->crtc_htotal - 1));
> > > +	intel_de_write(display, TRANS_HBLANK_CMTG(cpu_transcoder),
> > > +		       HBLANK_START(adjusted_mode->crtc_hblank_start - 1) |
> > > +		       HBLANK_END(adjusted_mode->crtc_hblank_end - 1));
> > > +	intel_de_write(display, TRANS_HSYNC_CMTG(cpu_transcoder),
> > > +		       HSYNC_START(adjusted_mode->crtc_hsync_start - 1) |
> > > +		       HSYNC_END(adjusted_mode->crtc_hsync_end - 1));
> > > +	intel_de_write(display, TRANS_VTOTAL_CMTG(cpu_transcoder),
> > > +		       VACTIVE(crtc_vdisplay - 1) |
> > > +		       VTOTAL(crtc_vtotal - 1));
> > > +	intel_de_write(display, TRANS_VBLANK_CMTG(cpu_transcoder),
> > > +		       VBLANK_START(crtc_vblank_start - 1) |
> > > +		       VBLANK_END(crtc_vblank_end - 1));
> > > +	intel_de_write(display, TRANS_VSYNC_CMTG(cpu_transcoder),
> > > +		       VSYNC_START(adjusted_mode->crtc_vsync_start - 1) |
> > > +		       VSYNC_END(adjusted_mode->crtc_vsync_end - 1));
> > > +	intel_de_write(display,
> > TRANS_SET_CTX_LATENCY_CMTG(cpu_transcoder),
> > > +		       crtc_state->set_context_latency); }
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cmtg.h
> > > b/drivers/gpu/drm/i915/display/intel_cmtg.h
> > > index 660ec513626e..53a44f505dd2 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cmtg.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_cmtg.h
> > > @@ -6,9 +6,12 @@
> > >  #ifndef __INTEL_CMTG_H__
> > >  #define __INTEL_CMTG_H__
> > >
> > > +#include <linux/types.h>
> > > +
> > >  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_cmtg_regs.h
> > > b/drivers/gpu/drm/i915/display/intel_cmtg_regs.h
> > > index 4a80b88d88fd..f7fc812d8ef0 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cmtg_regs.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_cmtg_regs.h
> > > @@ -20,4 +20,35 @@
> > >  #define TRANS_CMTG_CTL_B		_MMIO(0x6fb88)
> > >  #define  CMTG_ENABLE			REG_BIT(31)
> > >
> > > +#define _TRANS_HTOTAL_CMTG_A		0x6F000
> > > +#define _TRANS_HTOTAL_CMTG_B		0x6F100
> > > +#define TRANS_HTOTAL_CMTG(trans)	_MMIO_TRANS((trans), \
> > > +						    _TRANS_HTOTAL_CMTG_A,
> > _TRANS_HTOTAL_CMTG_B)
> > > +#define _TRANS_HBLANK_CMTG_A		0x6F004
> > > +#define _TRANS_HBLANK_CMTG_B		0x6F104
> > > +#define TRANS_HBLANK_CMTG(trans)	_MMIO_TRANS((trans), \
> > > +						    _TRANS_HBLANK_CMTG_A,
> > _TRANS_HBLANK_CMTG_B)
> > > +#define _TRANS_HSYNC_CMTG_A		0x6F008
> > > +#define _TRANS_HSYNC_CMTG_B		0x6F108
> > > +#define TRANS_HSYNC_CMTG(trans)
> > 	_MMIO_TRANS((trans), \
> > > +						    _TRANS_HSYNC_CMTG_A,
> > _TRANS_HSYNC_CMTG_B)
> > > +#define _TRANS_VTOTAL_CMTG_A		0x6F00C
> > > +#define _TRANS_VTOTAL_CMTG_B		0x6F10C
> > > +#define TRANS_VTOTAL_CMTG(trans)	_MMIO_TRANS((trans), \
> > > +						    _TRANS_VTOTAL_CMTG_A,
> > _TRANS_VTOTAL_CMTG_B)
> > > +#define _TRANS_VBLANK_CMTG_A		0x6F010
> > > +#define _TRANS_VBLANK_CMTG_B		0x6F110
> > > +#define TRANS_VBLANK_CMTG(trans)	_MMIO_TRANS((trans), \
> > > +						    _TRANS_VBLANK_CMTG_A,
> > _TRANS_VBLANK_CMTG_B)
> > > +#define _TRANS_VSYNC_CMTG_A		0x6F014
> > > +#define _TRANS_VSYNC_CMTG_B		0x6F114
> > > +#define TRANS_VSYNC_CMTG(trans)
> > 	_MMIO_TRANS((trans), \
> > > +						    _TRANS_VSYNC_CMTG_A,
> > _TRANS_VSYNC_CMTG_B)
> > 
> > I though there was already feedback that these match the regular transcoder
> > registers.
> 
> _TRANS_HTOTAL_A         0x60000
> _TRANS_HTOTAL_B         0x61000
> 
> _TRANS_HTOTAL_CMTG_A            0x6F000
> _TRANS_HTOTAL_CMTG_B            0x6F100
> 
> I am not clear how to match?

#define TRANSCODER_CMTG0_OFFSET 0x6F000
#define TRANSCODER_CMTG1_OFFSET 0x6F100

-- 
Ville Syrjälä
Intel