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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 28 Sep 2026, "Kandpal, Suraj" <suraj.kandpal@intel.com> 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.
>> >
>> > 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.

I mean the power domain should be modified to take care of this, not
that it should already take care of this.

BR,
Jani.


>
>> 
>> 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

-- 
Jani Nikula, Intel