Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Medium] drm/i915/alpm: bounds check rejects max valid LFPS half cycle 
duration
--

commit df710743c62bb2cde8d5cec635597f5355e70e89
Author: Animesh Manna <[email protected]>
Subject: drm/i915/alpm: Program zero-based LFPS half cycle duration

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.

> diff --git a/drivers/gpu/drm/i915/display/intel_psr_regs.h 
> b/drivers/gpu/drm/i915/display/intel_psr_regs.h
> index 8f60b671755aa..387edf8df6e98 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr_regs.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr_regs.h
> @@ -339,11 +339,17 @@

[ ... ]

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

[Severity: Medium]
Since this macro now subtracts 1 for zero-based encoding, does the bounds
check in _lnl_compute_aux_less_alpm_params() also need to be updated?

drivers/gpu/drm/i915/display/intel_alpm.c:_lnl_compute_aux_less_alpm_params() {
        ...
        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)
                return false;
        ...
}

It looks like this check restricts the maximum allowed symbols to 31 (the
mask value). With the new zero-based encoding, shouldn't a value of 32 be
permitted? If lfps_half_cycle is 32, this bounds check will return false,
which could artificially restrict the limit and reject valid hardware
display configurations.

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=17

Reply via email to