Re: [PATCH v2] drm/i915/dp: Disable DC5/6 before AUX programming

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/i915/dp: incorrect power sequencing for DC_OFF and AUX domains
--

commit f07687ba1d41d832937531ae2d822e56d80de373
Author: Suraj Kandpal <suraj.kandpal@intel.com>

drm/i915/dp: Disable DC5/6 before AUX programming

This commit introduces explicit disabling of DC5/6 states during the
AUX programming sequence on Xe3p and later platforms with LT PHY. The
change aims to honor the hardware Bspec which requires the PHY to be
powered down before AUX programming.

> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> index 3cb07b0a901cf..879b203f92585 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
[ ... ]
> @@ -286,6 +287,9 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>  
>  	aux_wakeref = intel_display_power_get(display, aux_domain);
>  
> +	if (HAS_LT_PHY(display))
> +		dc_off_wakeref = intel_display_power_get(display, POWER_DOMAIN_DC_OFF);
> +

[Severity: High]
The commit message states we need to make sure DC5/6 is disabled before
the AUX programming sequence. However, does this sequence actually power
on the AUX domain before disabling the DC states?

By calling intel_display_power_get() for aux_domain before grabbing the
POWER_DOMAIN_DC_OFF wakeref, we seem to be violating the hardware
requirement to disable DC5/6 first. Could this regression cause AUX
programming failures or display bring-up issues on platforms with LT PHY?

[ ... ]
> @@ -459,6 +463,9 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>  	if (pps_wakeref)
>  		intel_pps_unlock(intel_dp, pps_wakeref);
>  
> +	if (dc_off_wakeref)
> +		intel_display_power_put(display, POWER_DOMAIN_DC_OFF, dc_off_wakeref);
> +
>  	intel_display_power_put_async(display, aux_domain, aux_wakeref);
>  out_unlock:
>  	intel_digital_port_unlock(encoder);

[Severity: High]
Does this regression cause rapid hardware toggling during back-to-back
AUX transactions?

The aux_domain wakeref is released asynchronously right after this via
intel_display_power_put_async(), which usually prevents rapid toggling
between transfers. But because dc_off_wakeref is released synchronously
first, it appears we allow DC states to re-enable while the AUX domain
is still powered. 

Should the POWER_DOMAIN_DC_OFF wakeref also be released asynchronously,
or at least released after the AUX domain is fully powered down?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918063829.1321703-1-suraj.kandpal@intel.com?part=1