> -----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. 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__ */
