Re: [PATCH v7 01/20] drm/i915/alpm: Program zero-based LFPS half cycle duration

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

On Mon, 26 Oct 2026, Animesh Manna <animesh.manna@intel.com> wrote:
> The LFPS Half Cycle Duration field, as well as the First and Last LFPS
> Half Cycle Duration fields, in PORT_ALPM_LFPS_CTL are zero-based: per
> Bspec the value programmed must be N_Symbol - 1. The number of symbols
> was programmed directly, making each half cycle one symbol clock too
> long.
>
> Subtract one in the PORT_ALPM_LFPS_CTL_LFPS_HALF_CYCLE_DURATION,
> PORT_ALPM_LFPS_CTL_FIRST_LFPS_HALF_CYCLE_DURATION and
> PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION field macros so the
> programmed value matches the zero-based field encoding.
>
> Since the fields now encode 1 to 32 symbols, update the bounds check
> in _lnl_compute_aux_less_alpm_params() to accept a half cycle of 32
> symbols and reject 0, which cannot be encoded.
>
> v2: Adjust half cycle bounds check for zero-based encoding. [sashiko]
>
> Bspec: 71632
> Cc: Jouni Högander <jouni.hogander@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_alpm.c     |  3 ++-
>  drivers/gpu/drm/i915/display/intel_psr_regs.h | 12 +++++++++---
>  2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
> index f54b29e193ee..8542bd9c9666 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -134,7 +134,8 @@ _lnl_compute_aux_less_alpm_params(struct intel_dp *intel_dp,
>  
>  	if (aux_less_wake_lines > ALPM_CTL_AUX_LESS_WAKE_TIME_MASK ||
>  	    silence_period > PORT_ALPM_CTL_SILENCE_PERIOD_MASK ||
> -	    lfps_half_cycle > PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION_MASK)
> +	    lfps_half_cycle < 1 ||
> +	    lfps_half_cycle > PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION_MASK + 1)
>  		return false;
>  
>  	if (display->params.psr_safest_params)
> diff --git a/drivers/gpu/drm/i915/display/intel_psr_regs.h b/drivers/gpu/drm/i915/display/intel_psr_regs.h
> index a57bc111290d..78a05387eab5 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr_regs.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr_regs.h
> @@ -335,11 +335,17 @@
>  #define  PORT_ALPM_LFPS_CTL_LFPS_CYCLE_COUNT_MASK		REG_GENMASK(27, 24)
>  #define  PORT_ALPM_LFPS_CTL_LFPS_CYCLE_COUNT_MIN		7
>  #define  PORT_ALPM_LFPS_CTL_LFPS_CYCLE_COUNT(val)		REG_FIELD_PREP(PORT_ALPM_LFPS_CTL_LFPS_CYCLE_COUNT_MASK, (val) - PORT_ALPM_LFPS_CTL_LFPS_CYCLE_COUNT_MIN)
> +/*
> + * The LFPS half cycle duration fields (including the first and last half
> + * cycle) are zero-based, i.e. the value programmed is N_Symbol - 1.
> + *
> + * Bspec: 71632
> + */

Please let's not normalize adding bspec references to code comments.

>  #define  PORT_ALPM_LFPS_CTL_LFPS_HALF_CYCLE_DURATION_MASK	REG_GENMASK(20, 16)
> -#define  PORT_ALPM_LFPS_CTL_LFPS_HALF_CYCLE_DURATION(val)	REG_FIELD_PREP(PORT_ALPM_LFPS_CTL_LFPS_HALF_CYCLE_DURATION_MASK, val)
> +#define  PORT_ALPM_LFPS_CTL_LFPS_HALF_CYCLE_DURATION(val)	REG_FIELD_PREP(PORT_ALPM_LFPS_CTL_LFPS_HALF_CYCLE_DURATION_MASK, (val) - 1)
>  #define  PORT_ALPM_LFPS_CTL_FIRST_LFPS_HALF_CYCLE_DURATION_MASK	REG_GENMASK(12, 8)
> -#define  PORT_ALPM_LFPS_CTL_FIRST_LFPS_HALF_CYCLE_DURATION(val)	REG_FIELD_PREP(PORT_ALPM_LFPS_CTL_FIRST_LFPS_HALF_CYCLE_DURATION_MASK, val)
> +#define  PORT_ALPM_LFPS_CTL_FIRST_LFPS_HALF_CYCLE_DURATION(val)	REG_FIELD_PREP(PORT_ALPM_LFPS_CTL_FIRST_LFPS_HALF_CYCLE_DURATION_MASK, (val) - 1)
>  #define  PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION_MASK	REG_GENMASK(4, 0)
> -#define  PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION(val)	REG_FIELD_PREP(PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION_MASK, val)
> +#define  PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION(val)	REG_FIELD_PREP(PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION_MASK, (val) - 1)
>  
>  #endif /* __INTEL_PSR_REGS_H__ */

-- 
Jani Nikula, Intel