> -----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.

>  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 ?

> +
> +     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.

> +             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

Reply via email to