Re: [PATCH] drm/i915/pps: Don't block DC states while the VDD override is held

[Imre Deak <imre.deak@intel.com>]

On Mon, Sep 28, 2026 at 11:44:13AM +0300, Kandpal, Suraj wrote:
> > arun.r.murthy@intel.comm; Kandpal, Suraj <suraj.kandpal@intel.com>
> > Subject: Re: [PATCH] drm/i915/pps: Don't block DC states while the VDD
> > override is held
> > 
> > On Mon, 28 Sep 2026, Suraj Kandpal <suraj.kandpal@intel.com> wrote:
> > > intel_pps_vdd_on_unlocked() takes the AUX power domain and holds it
> > > until
> > > edp_panel_vdd_schedule_off() drops it, panel_power_cycle_delay * 5 ms
> > > after the last AUX transfer. That is seconds on a typical panel.
> > > The AUX domains now block DC states on Xe3p_LPD, so that reference
> > > keeps DC_off up for all of it. DC3co arms with a 1 ms put delay and
> > > never gets a window, and its residency counter does not increment.
> > > Nothing in the panel power sequence needs DC states off for as long as
> > > the override is held. Take AUX IO instead, which powers up the same
> > > well without blocking DC states, the same split
> > > intel_ddi_main_link_aux_domain() makes for PSR. AUX transfers still
> > > take the DC state blocking domain around each transfer.

What causes the continuous AUX transfers which leads to "DC3co never
getting a window"?

> > > Bspec: 49277
> > > Fixes: 2827c44f148c ("drm/i915/xe3plpd: Map AUX power domains to
> > > DC_off")
> > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_pps.c | 21 +++++++++++++++++----
> > >  1 file changed, 17 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_pps.c
> > > b/drivers/gpu/drm/i915/display/intel_pps.c
> > > index d4c98b150fa2..bdbaa10bce23 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_pps.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> > > @@ -11,6 +11,7 @@
> > >  #include "g4x_dp.h"
> > >  #include "intel_de.h"
> > >  #include "intel_display_jiffies.h"
> > > +#include "intel_display_power.h"
> > >  #include "intel_display_power_well.h"
> > >  #include "intel_display_regs.h"
> > >  #include "intel_display_types.h"
> > > @@ -734,6 +735,18 @@ static  u32 ilk_get_pp_control(struct intel_dp
> > *intel_dp)
> > >  	return control;
> > >  }
> > >
> > > +static enum intel_display_power_domain
> > > +intel_pps_vdd_power_domain(struct intel_dp *intel_dp) {
> > > +	struct intel_display *display = to_intel_display(intel_dp);
> > > +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> > > +
> > > +	if (DISPLAY_VER(display) >= 35 && !intel_encoder_is_tc(&dig_port-
> > >base))
> > > +		return intel_display_power_aux_io_domain(display,
> > > +dig_port->aux_ch);
> > 
> > Seems like this is something that the power domain framework should take
> > care of instead of hacking together locally.
> 
> Actually it does not at least not for Xe3p which now has DC_OFF when taking AUX domain,
> Because of that when PPS take AUX power domain it also takes DC_OFF(exiting DC states) which is not required
> And since it keeps the reference for longer than what a normal aux transaction would DC3CO does not increment its
> Counter.
> 
> > 
> > Cc: Imre, which you should pretty much always do when doing anything
> > related to power domains.
> > 
> 
> Sure will keep in mind going forward.
> 
> Regards,
> Suraj Kandpal
> 
> > 
> > BR,
> > Jani.
> > 
> > > +
> > > +	return intel_aux_power_domain(dig_port); }
> > > +
> > >  /*
> > >   * Must be paired with intel_pps_vdd_off_unlocked().
> > >   * Must hold pps_mutex around the whole on/off sequence.
> > > @@ -760,7 +773,7 @@ bool intel_pps_vdd_on_unlocked(struct intel_dp
> > > *intel_dp)
> > >
> > >  	drm_WARN_ON(display->drm, intel_dp->pps.vdd_wakeref);
> > >  	intel_dp->pps.vdd_wakeref = intel_display_power_get(display,
> > > -
> > intel_aux_power_domain(dig_port));
> > > +
> > intel_pps_vdd_power_domain(intel_dp));
> > >
> > >  	pp_stat_reg = _pp_stat_reg(intel_dp);
> > >  	pp_ctrl_reg = _pp_ctrl_reg(intel_dp); @@ -862,7 +875,7 @@ static
> > > void intel_pps_vdd_off_sync_unlocked(struct intel_dp *intel_dp)
> > >  	}
> > >
> > >  	intel_display_power_put(display,
> > > -				intel_aux_power_domain(dig_port),
> > > +				intel_pps_vdd_power_domain(intel_dp),
> > >  				fetch_and_zero(&intel_dp-
> > >pps.vdd_wakeref));
> > >  }
> > >
> > > @@ -1064,7 +1077,7 @@ void intel_pps_off_unlocked(struct intel_dp
> > > *intel_dp)
> > >
> > >  	/* We got a reference when we enabled the VDD. */
> > >  	intel_display_power_put(display,
> > > -				intel_aux_power_domain(dig_port),
> > > +				intel_pps_vdd_power_domain(intel_dp),
> > >  				fetch_and_zero(&intel_dp-
> > >pps.vdd_wakeref));
> > >  }
> > >
> > > @@ -1336,7 +1349,7 @@ static void pps_vdd_init(struct intel_dp
> > *intel_dp)
> > >  		    pps_name(intel_dp));
> > >  	drm_WARN_ON(display->drm, intel_dp->pps.vdd_wakeref);
> > >  	intel_dp->pps.vdd_wakeref = intel_display_power_get(display,
> > > -
> > intel_aux_power_domain(dig_port));
> > > +
> > intel_pps_vdd_power_domain(intel_dp));
> > >  }
> > >
> > >  bool intel_pps_have_panel_power_or_vdd(struct intel_dp *intel_dp)
> > 
> > --
> > Jani Nikula, Intel