Re: [PATCH v3 01/12] drm/i915/cmtg: add is_enable_allowed() for cmtg

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

On Tue, Apr 07, 2026 at 02:29:46PM +0530, Dibin Moolakadan Subrahmanian wrote:
> On 07-04-2026 00:18, Shankar, Uma wrote:
> >
> >> -----Original Message-----
> >> From: Manna, Animesh <animesh.manna@intel.com>
> >> Sent: Friday, March 13, 2026 9:03 PM
> >> To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org
> >> Cc: Nikula, Jani <jani.nikula@intel.com>; Shankar, Uma
> >> <uma.shankar@intel.com>; Dibin Moolakadan Subrahmanian
> >> <dibin.moolakadan.subrahmanian@intel.com>; Manna, Animesh
> >> <animesh.manna@intel.com>
> >> Subject: [PATCH v3 01/12] drm/i915/cmtg: add is_enable_allowed() for cmtg
> > Nit: Name here in subject doesn't match the actual function. Better to use exact same name.
> >
> >> Introduce a flag for DC3co. CMTG will be enabled only with DC3co so add a
> >> separate function is_allowed() for cmtg. DC3co flag will be enabled in a separate
> >> patch.
> >>
> >> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/display/intel_cmtg.c          | 14 ++++++++++++++
> >>   drivers/gpu/drm/i915/display/intel_cmtg.h          |  2 ++
> >>   .../gpu/drm/i915/display/intel_display_device.h    |  1 +
> >>   drivers/gpu/drm/i915/display/intel_display_types.h |  4 ++++
> >>   4 files changed, 21 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_cmtg.c
> >> b/drivers/gpu/drm/i915/display/intel_cmtg.c
> >> index e1fdc6fe9762..024d753eca55 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_cmtg.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_cmtg.c
> >> @@ -16,6 +16,7 @@
> >>   #include "intel_display_device.h"
> >>   #include "intel_display_power.h"
> >>   #include "intel_display_regs.h"
> >> +#include "intel_display_types.h"
> >>
> >>   /**
> >>    * DOC: Common Primary Timing Generator (CMTG) @@ -185,3 +186,16 @@
> >> void intel_cmtg_sanitize(struct intel_display *display)
> >>
> >>   	intel_cmtg_disable(display, &cmtg_config);  }
> >> +
> >> +bool intel_cmtg_is_allowed(const struct intel_crtc_state *crtc_state) {
> >> +	struct intel_display *display = to_intel_display(crtc_state);
> >> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >> +
> >> +	if ((cpu_transcoder == TRANSCODER_A || cpu_transcoder ==
> >> TRANSCODER_B) &&
> >> +	    HAS_DC3CO(display) && intel_crtc_has_type(crtc_state,
> >> INTEL_OUTPUT_EDP) &&
> >> +	    crtc_state->dc3co.enable)
> > Don't think we need both HAS_DC3CO and dc3co.enable here. Later should never be set if HAS_DC3CO
> > not true.
> >
> >> +		return true;
> >> +
> >> +	return false;
> >> +}
> >> diff --git a/drivers/gpu/drm/i915/display/intel_cmtg.h
> >> b/drivers/gpu/drm/i915/display/intel_cmtg.h
> >> index ba62199adaa2..7692cc98cf87 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_cmtg.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_cmtg.h
> >> @@ -7,7 +7,9 @@
> >>   #define __INTEL_CMTG_H__
> >>
> >>   struct intel_display;
> >> +struct intel_crtc_state;
> >>
> >>   void intel_cmtg_sanitize(struct intel_display *display);
> >> +bool intel_cmtg_is_allowed(const struct intel_crtc_state *crtc_state);
> >>
> >>   #endif /* __INTEL_CMTG_H__ */
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h
> >> b/drivers/gpu/drm/i915/display/intel_display_device.h
> >> index e84c190dcc4f..35e06fcf794d 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> >> @@ -189,6 +189,7 @@ struct intel_display_platforms {
> >>   #define HAS_LRR(__display)		(DISPLAY_VER(__display) >= 12)
> >>   #define HAS_LSPCON(__display)		(IS_DISPLAY_VER(__display, 9,
> >> 10))
> >>   #define HAS_LT_PHY(__display)		((__display)->platform.novalake)
> >> +#define HAS_DC3CO(__display)		((__display)->platform.novalake)
> >>   #define HAS_MBUS_JOINING(__display)	((__display)->platform.alderlake_p
> >> || DISPLAY_VER(__display) >= 14)
> >>   #define HAS_MSO(__display)		(DISPLAY_VER(__display) >= 12)
> >>   #define HAS_OVERLAY(__display)		(DISPLAY_INFO(__display)-
> >>> has_overlay)
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> >> b/drivers/gpu/drm/i915/display/intel_display_types.h
> >> index e189f8c39ccb..8a92ea4f1438 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> >> @@ -1434,6 +1434,10 @@ struct intel_crtc_state {
> >>
> >>   	/* to track changes in plane color blocks */
> >>   	bool plane_color_changed;
> >> +
> >> +	struct {
> >> +		bool enable;
> >> +	} dc3co;
> >>   };
> 
> DC3CO shouldn’t be part of struct intel_crtc_state.
> It’s a global display power feature, not per-CRTC state.
> struct intel_atomic_state would be a more appropriate place to track this.

intel_atomic_state isn't actually a state, and so should not be used
to track anything. To better reflect its role drm_atomic_state will
soon be renamed to drm_atomic_commit, and we need to follow up with
the corresponding intel_atomic_* rename.

-- 
Ville Syrjälä
Intel