Re: [PATCH] drm/i915/pps: don't orphan the VDD wakeref when the HW force bit is reset

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

On Mon, 31 Aug 2026, Arun R Murthy <arun.r.murthy@intel.com> wrote:
> pps_vdd_init() takes an AUX power-domain reference into
> intel_dp->pps.vdd_wakeref when it finds EDP_FORCE_VDD already enabled by
> the BIOS, and defers dropping it to a later scheduled VDD off.
>
> intel_pps_vdd_on_unlocked() decides whether it already owns that reference
> by reading the live EDP_FORCE_VDD bit via edp_have_panel_vdd(). That bit
> can be reset under the driver (DC states, DMC, BIOS) while we still hold
> vdd_wakeref. When that happens the function believes it holds nothing,
> overwrites vdd_wakeref with a freshly acquired reference and orphans the
> original one. The leak surfaces at suspend-late as:
>
>   i915 raw-wakerefs=1 wakelocks=1 on cleanup
>
> with the leaked wakeref's backtrace pointing at pps_vdd_init() ->
> intel_pps_init() from driver probe.
>
> Make vdd_wakeref the authoritative record of the reference: only acquire
> a new one when we don't already hold it, and re-assert the HW bit either
> way. This keeps the reference balanced regardless of the volatile HW
> state and drops the now-incorrect WARN_ON().
>
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/8477
> Assisted-by: Claude:claude-3-opus
> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> Tested-by: Adharsh K <adharshkandath@gmail.com>
> ---
>  drivers/gpu/drm/i915/display/intel_pps.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
> index d4c98b150fa2..43e89aaf419f 100644
> --- a/drivers/gpu/drm/i915/display/intel_pps.c
> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> @@ -758,9 +758,10 @@ bool intel_pps_vdd_on_unlocked(struct intel_dp *intel_dp)
>  	if (edp_have_panel_vdd(intel_dp))
>  		return need_to_disable;
>  
> -	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));
> +	if (!intel_dp->pps.vdd_wakeref)
> +		intel_dp->pps.vdd_wakeref =
> +			intel_display_power_get(display,
> +						intel_aux_power_domain(dig_port));

This now silently accepts a whole class of mismatched VDD
enable/disable, losing one of the main points of the whole vdd_wakeref.

BR,
Jani.

>  
>  	pp_stat_reg = _pp_stat_reg(intel_dp);
>  	pp_ctrl_reg = _pp_ctrl_reg(intel_dp);

-- 
Jani Nikula, Intel