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

[Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahmanian@intel.com>]

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.

>>   enum intel_pipe_crc_source {
>> --
>> 2.29.0