> -----Original Message----- > From: Manna, Animesh <[email protected]> > Sent: Friday, May 22, 2026 10:31 AM > To: Dibin Moolakadan Subrahmanian > <[email protected]>; [email protected]; > [email protected] > Cc: Shankar, Uma <[email protected]>; [email protected]; > Nikula, Jani <[email protected]> > Subject: RE: [PATCH v6 01/16] drm/i915/cmtg: Add intel_cmtg_is_allowed() for > CMTG > > > > > -----Original Message----- > > From: Dibin Moolakadan Subrahmanian > > <[email protected]> > > Sent: Thursday, May 21, 2026 6:06 PM > > To: Manna, Animesh <[email protected]>; intel- > > [email protected]; [email protected] > > Cc: Shankar, Uma <[email protected]>; > > [email protected]; Nikula, Jani <[email protected]> > > Subject: Re: [PATCH v6 01/16] drm/i915/cmtg: Add > > intel_cmtg_is_allowed() for CMTG > > > > > > On 21-05-2026 14:31, Manna, Animesh wrote: > > > > > >> -----Original Message----- > > >> From: Dibin Moolakadan Subrahmanian > > >> <[email protected]> > > >> Sent: Thursday, May 21, 2026 1:49 PM > > >> To: Manna, Animesh <[email protected]>; intel- > > >> [email protected]; [email protected] > > >> Cc: Shankar, Uma <[email protected]>; > > >> [email protected]; Nikula, Jani <[email protected]> > > >> Subject: Re: [PATCH v6 01/16] drm/i915/cmtg: Add > > >> intel_cmtg_is_allowed() for CMTG > > >> > > >> > > >> On 13-05-2026 22:08, Animesh Manna wrote: > > >>> CMTG will be enabled only with DC3co, so add a separate function > > >>> intel_cmtg_is_allowed() to check the prerequisites for enabling CMTG. > > >>> DC3co will be enabled in a separate patch. > > >>> > > >>> v2: > > >>> - Remove separate flag for DC3co from crtc_state. [Uma, Dibin] > > >>> > > >>> v3: > > >>> - Do not access power domain members directly. [Jani] > > >>> > > >>> Signed-off-by: Animesh Manna <[email protected]> > > >>> --- > > >>> drivers/gpu/drm/i915/display/intel_cmtg.c | 15 ++++++++++++++- > > >>> drivers/gpu/drm/i915/display/intel_cmtg.h | 4 ++++ > > >>> 2 files changed, 18 insertions(+), 1 deletion(-) > > >>> > > >>> diff --git a/drivers/gpu/drm/i915/display/intel_cmtg.c > > >>> b/drivers/gpu/drm/i915/display/intel_cmtg.c > > >>> index e1fdc6fe9762..dc039bea452c 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> > > >>> @@ -16,6 +15,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 > > +185,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) && > > >>> + DISPLAY_VER(display) == 35 && > > >>> +intel_crtc_has_type(crtc_state, > > >> INTEL_OUTPUT_EDP) && > > >>> + intel_display_power_get_current_dc_state(display) == > > >>> +DC_STATE_EN_DC3CO) > > >> I would enable CMTG before setting the DC3CO state, since CMTG > > >> running is a must condition for DC3CO. > > >> Doing it the current way creates a cyclic dependency. > > > Copy -pasting from bspec: > > > CMTG must be enabled with dynamic DC states. > > > > > > Dynamic DC state refers to Dc3co/Dc6v. > > > So added a check for dc-state which will be used inside cmtg > > > functions. Do > > not want to allow touching CMTG register if target_dc_state is not DC3co. > > > > > > Again copy-pasting from bspec: > > > CMTG state needs to be saved and restored during power state transitions. > > > > > > Which means CMTG will lose its value with DC5/DC6. So want to be > > > little > > cautious while handling CMTG. > > > > I can see DC6 restore is already handled in patch 15. > > > > target_dc_state is a software state doesn't guarantee any actual DC > > state register write. > > > > I think its better to leave the target dc state decision to DC3CO > > implementation rather than using it as flag variable for CMTG enable check. > > This is just a protective measure, in my opinion not harmful.
I think we can leave CMTG enabling to the caller. Caller should check if CMTG is needed and call the enable helper. No need for CMTG to check for target DC state. Also during restore from DC6, before enabling DC3CO CMTG timing registers which lose value should be restored. This will simplify the checks. Regards, Uma Shankar > Regards, > Animesh > > > > > > > > > We should finalize what will be the target_dc_state based use case > > > like > > PSR2/LOBF/PR-ALPM and single EDP configuration. > > > This is the only flag and based on that CMTG and DC3co will be > > > enabled. So, > > no cyclic dependency. Good to know if I am missing anything. > > > > > > Regards, > > > Animesh > > > > > >>> + 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..ed540581738f 100644 > > >>> --- a/drivers/gpu/drm/i915/display/intel_cmtg.h > > >>> +++ b/drivers/gpu/drm/i915/display/intel_cmtg.h > > >>> @@ -6,8 +6,12 @@ > > >>> #ifndef __INTEL_CMTG_H__ > > >>> #define __INTEL_CMTG_H__ > > >>> > > >>> +#include <linux/types.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__ */
