> -----Original Message-----
> From: Shankar, Uma <[email protected]>
> Sent: Thursday, June 11, 2026 11:27 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 14/20] drm/i915/cmtg: Modify existing hook to disable
> CMTG
>
>
>
> > -----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 14/20] drm/i915/cmtg: Modify existing hook to
> > disable CMTG
> >
> > From: Dibin Moolakadan Subrahmanian
> > <[email protected]>
> >
> > Earlier cmtg_disable() used to disable all instances of CMTG which
> > cannot handle individual request for specific CMTG instance.
> > Introduce cmtg_disable_all() which will disable all cmtg instances and
> > cmtg_disable() only disable specific instance.
> >
> > v2:
> > - Use intel_de_rmw to simplify. [Uma]
> >
> > Signed-off-by: Dibin Moolakadan Subrahmanian
> > <[email protected]>
> > Signed-off-by: Animesh Manna <[email protected]>
> > ---
> > drivers/gpu/drm/i915/display/intel_cmtg.c | 62 ++++++++++++++-----
> > drivers/gpu/drm/i915/display/intel_cmtg.h | 1 +
> > .../gpu/drm/i915/display/intel_cmtg_regs.h | 1 +
> > 3 files changed, 49 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cmtg.c
> > b/drivers/gpu/drm/i915/display/intel_cmtg.c
> > index 077653e2f599..20b74c2856c4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cmtg.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cmtg.c
> > @@ -83,6 +83,18 @@ static void intel_cmtg_dump_config(struct
> > intel_display *display,
> > str_yes_no(cmtg_config->trans_b_secondary));
> > }
> >
> > +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;
> > + }
> > +}
>
> This seems to be already defined in patch 7, drop this duplicate.
Sure, taken care in next version.
>
> > static bool intel_cmtg_transcoder_is_secondary(struct intel_display
> *display,
> > enum transcoder trans)
> > {
> > @@ -126,8 +138,8 @@ static bool
> > intel_cmtg_disable_requires_modeset(struct
> > intel_display *display,
> > return cmtg_config->trans_a_secondary || cmtg_config-
> > >trans_b_secondary; }
> >
> > -static void intel_cmtg_disable(struct intel_display *display,
> > - struct intel_cmtg_config *cmtg_config)
> > +static void intel_cmtg_disable_all(struct intel_display *display,
> > + struct intel_cmtg_config *cmtg_config)
> > {
> > u32 clk_sel_clr = 0;
> > u32 clk_sel_set = 0;
> > @@ -158,6 +170,38 @@ static void intel_cmtg_disable(struct
> > intel_display *display,
> > intel_de_rmw(display, CMTG_CLK_SEL, clk_sel_clr,
> clk_sel_set); }
> >
> > +void intel_cmtg_disable(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;
> > + enum transcoder cmtg_transcoder = to_cmtg_transcoder(crtc_state-
> > >cpu_transcoder);
> > + u32 clk_sel_clr = 0;
> > +
> > + if (!crtc->cmtg.enabled)
> > + return;
> > +
> > + crtc->cmtg.enabled = false;
> > + intel_de_rmw(display, TRANS_VRR_CTL(display, cmtg_transcoder),
> > + VRR_CTL_VRR_ENABLE | VRR_CTL_FLIP_LINE_EN, 0);
> > +
> > + intel_de_rmw(display, TRANS_DDI_FUNC_CTL2(display,
> > cpu_transcoder),
> > + CMTG_SECONDARY_MODE, 0);
> > +
> > + intel_de_rmw(display, TRANS_CMTG_CTL(cpu_transcoder),
> > CMTG_ENABLE, 0);
>
> Should it not be cmtg_transcoder ?
CMTG exclusive register which are not part of normal transcoder will use
cpu_transcoder.
>
> > +
> > + if (intel_de_wait_for_clear_ms(display,
> > TRANS_CMTG_CTL(cpu_transcoder), CMTG_STATE, 50)) {
> > + drm_WARN(display->drm, 1, "CMTG: %s disable timeout\n",
> > + transcoder_name(cpu_transcoder));
>
>
> Can you add a comment explaining which transcoder is referred here
> cpu_transcoder or cmtg_transcoder and why.
>
Added the below code comment to next version, hope that works.
/*
* Use cpu_transcoder for:
* 1. Exclusive CMTG registers that do not use the standard transcoder
offset
* (e.g., TRANS_CMTG_CTL, CMTG_CLK_SEL).
* 2. Registers shared between the eDP and CMTG transcoders.
* (e.g., TRANS_DDI_FUNC_CTL2).
*/
Regards,
Animesh
> > + return;
> > + }
> > +
> > + clk_sel_clr = cpu_transcoder == TRANSCODER_A ?
> > CMTG_CLK_SEL_A_MASK : CMTG_CLK_SEL_B_MASK;
> > + intel_de_rmw(display, CMTG_CLK_SEL, clk_sel_clr, 0);
> > +
> > + drm_dbg_kms(display->drm, "CMTG: %s disabled\n",
> > +transcoder_name(cpu_transcoder)); }
> > +
> > /*
> > * Read out CMTG configuration and, on platforms that allow disabling it
> without
> > * a modeset, do it.
> > @@ -185,7 +229,7 @@ void intel_cmtg_sanitize(struct intel_display
> *display)
> > if (intel_cmtg_disable_requires_modeset(display, &cmtg_config))
> > return;
> >
> > - intel_cmtg_disable(display, &cmtg_config);
> > + intel_cmtg_disable_all(display, &cmtg_config);
> > }
> >
> > bool intel_cmtg_is_allowed(const struct intel_crtc_state *crtc_state)
> > @@ -222,18
> > +266,6 @@ void intel_cmtg_set_clk_select(const struct intel_crtc_state
> > *crtc_state)
> > 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;
> > - }
> > -}
>
> Oh this is moved up, but this is unnecessary change part of same series,
> handle it gracefully instead of moving across the patches.
>
> > -
> > 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);
> > diff --git a/drivers/gpu/drm/i915/display/intel_cmtg.h
> > b/drivers/gpu/drm/i915/display/intel_cmtg.h
> > index 12abbafa7d08..79785afccc51 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_disable(const struct intel_crtc_state *crtc_state);
> > void intel_cmtg_enable_ddi(const struct intel_crtc_state
> > *crtc_state); void intel_cmtg_enable_sync(const struct
> > intel_crtc_state *crtc_state); void intel_cmtg_set_m_n(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 a93236bf7b75..240a02cd4a3a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cmtg_regs.h
> > +++ b/drivers/gpu/drm/i915/display/intel_cmtg_regs.h
> > @@ -22,5 +22,6 @@
> > _TRANS_CMTG_CTL_A,
> > _TRANS_CMTG_CTL_B)
> > #define CMTG_ENABLE REG_BIT(31)
> > #define CMTG_SYNC_TO_PORT REG_BIT(29)
> > +#define CMTG_STATE REG_BIT(23)
> >
> > #endif /* __INTEL_CMTG_REGS_H__ */
> > --
> > 2.29.0
