Re: [PATCH v6 12/18] drm/i915/alpm: Program LTTPR count for DP 2.1 ALPM

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] drm/i915/alpm: DP 2.1 ALPM LTTPR logic is unreachable dead code
- [Medium] drm/i915/alpm: Unchecked negative return from drm_dp_lttpr_count() causes register corruption
- [Low] drm/i915/alpm: Dead code in lttpr_count error check
- [Low] drm/i915/alpm: Comment contradicts LTTPR revision code check
--

commit a317ca0f7af6803f7b3932311f82447dfda1b648
Author: Animesh Manna <animesh.manna@intel.com>
drm/i915/alpm: Program LTTPR count for DP 2.1 ALPM

This commit programs the total number of LTTPRs for DP 2.1 ALPM before
link training by issuing an AUX write transaction to DP_TOTAL_LTTPR_CNT.

> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
> index da653a407e97e..06c1a59aa2c51 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -726,7 +726,7 @@ static void lnl_alpm_configure(struct intel_dp *intel_dp,
>  {
>  	struct intel_display *display = to_intel_display(intel_dp);
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> -	u32 alpm_ctl;
> +	u32 alpm_ctl, alpm_ctl2, lttpr_count;
>  
[ ... ]
> @@ -762,6 +762,11 @@ static void lnl_alpm_configure(struct intel_dp *intel_dp,
>  
>  	alpm_ctl |= ALPM_CTL_ALPM_ENTRY_CHECK(crtc_state->alpm_state.check_entry_lines);
>  
> +	lttpr_count = drm_dp_lttpr_count(intel_dp->lttpr_common_caps);
> +
> +	alpm_ctl2 = ALPM_CTL2_NUMBER_OF_LTTPR(lttpr_count);

[Severity: Medium]
Does this assignment to a u32 variable cause error codes to go unhandled?

The function drm_dp_lttpr_count() can return negative error codes like
-ERANGE or -EINVAL on failure. If this happens, assigning it to the unsigned
lttpr_count variable strips the negative sign and results in a large value
being passed into ALPM_CTL2_NUMBER_OF_LTTPR(). This could corrupt the
register programming by injecting invalid bits via REG_FIELD_PREP.

> +
> +	intel_de_write(display, ALPM_CTL2(display, cpu_transcoder), alpm_ctl2);
>  	intel_de_write(display, ALPM_CTL(display, cpu_transcoder), alpm_ctl);
>  	mutex_unlock(&intel_dp->alpm.lock);
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index cb92cff906146..37c8306a1fa0d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -2265,6 +2265,25 @@ intel_dp_128b132b_link_train(struct intel_dp *intel_dp,
>  	return passed;
>  }
>  
> +static void intel_dp_update_lttpr_count(struct intel_dp *intel_dp,
> +					int lttpr_count)
> +{
> +	u8 val;
> +
> +	/*
> +	 * Program only for DP2.1 and return if LTTPR revison is less than 0x20
> +	 * where 0:3 represents minor rev and 4:7 represent major rev
> +	 */

[Severity: Low]
This isn't a bug, but the comment specifies "less than 0x20", while the
code below implements a less than or equal to 0x20 (<= 0x20) check.

> +	if (!intel_dp_is_edp(intel_dp) ||
> +	    lttpr_count < 0 ||

[Severity: Low]
Is this lttpr_count < 0 check reachable?

In intel_dp_start_link_train(), the lttpr_count is clamped to 0 on error
before being passed to this function:

	if (lttpr_count < 0)
		/* Still continue with enabling the port and link training. */
		lttpr_count = 0;

	intel_dp_prepare_link_train(intel_dp, crtc_state);
	intel_dp_update_lttpr_count(intel_dp, lttpr_count);

> +	    intel_dp->lttpr_common_caps[0] <= 0x20)

[Severity: High]
Can this LTTPR programming ever be reached for either DP or eDP configurations?

For standard DP connections, the function returns immediately because of the
!intel_dp_is_edp(intel_dp) check above.

For eDP connections, intel_dp_init_lttpr_and_dprx_caps() explicitly skips
LTTPR capabilities initialization, leaving intel_dp->lttpr_common_caps[0]
as 0. That causes the function to also return immediately due to this
<= 0x20 check, rendering the feature unreachable.

> +		return;
> +
> +	val = intel_dp->lttpr_common_caps[DP_PHY_REPEATER_CNT -
> +					  DP_LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV];
> +	drm_dp_dpcd_writeb(&intel_dp->aux, DP_TOTAL_LTTPR_CNT, val);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915180449.277933-1-animesh.manna@intel.com?part=12