Re: [PATCH v2 23/56] drm/i915/ddi_buf: Add Vswing table for HDMI FRL

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/i915: HDMI TMDS mode restricted from using new Vswing table
--

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

drm/i915/ddi_buf: Add Vswing table for HDMI FRL

This commit defines a Vswing table for HDMI FRL to set signal levels
for LT PHY on Xe3PLPD and onwards.

> diff --git a/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c b/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> index cc5912178d9cd..597bf1c781045 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
[ ... ]
> @@ -1164,6 +1165,14 @@ static const union intel_ddi_buf_trans_entry _xe3plpd_lt_trans_edp[] = {
>  	{ .lt = { 1, 3, 0, 26, 0 } },
>  };
>  
> +static const union intel_ddi_buf_trans_entry _xe3plpd_lt_trans_hdmi_frl[] = {
> +	{ .lt = { 0, 0, 0, 48, 0  } },
> +	{ .lt = { 0, 0, 4, 38, 6  } },
> +	{ .lt = { 0, 0, 4, 36, 8  } },
> +	{ .lt = { 0, 0, 4, 34, 10 } },
> +	{ .lt = { 0, 0, 4, 32, 12 } },
> +};
> +
>  static const struct intel_ddi_buf_trans xe3plpd_lt_trans_dp14 = {
>  	.entries = _xe3plpd_lt_trans_dp14,
>  	.num_entries = ARRAY_SIZE(_xe3plpd_lt_trans_dp14),
>  };

[Severity: High]
Because xe3plpd_lt_trans_dp14 lacks an explicit .hdmi_default_entry,
intel_ddi_hdmi_level() will silently default to index 0.

Index 0 of the dp14 table is { .lt = { 1, 0, 0, 21, 0 } }, which provides
a very low voltage swing of 21 instead of the 48 that is intended for
HDMI TMDS.

[ ... ]
> @@ -1780,6 +1794,9 @@ xe3plpd_get_lt_buf_trans(struct intel_encoder *encoder,
>  		return intel_get_buf_trans(&xe3plpd_lt_trans_uhbr, n_entries);
>  	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
>  		return intel_get_buf_trans(&xe3plpd_lt_trans_edp, n_entries);
> +	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI) &&
> +		 intel_hdmi_is_frl(crtc_state->port_clock))
> +		return intel_get_buf_trans(&xe3plpd_lt_trans_hdmi_frl, n_entries);
>  	else
>  		return intel_get_buf_trans(&xe3plpd_lt_trans_dp14, n_entries);
>  }

[Severity: High]
Does this condition incorrectly restrict HDMI TMDS outputs from utilizing
the new _xe3plpd_lt_trans_hdmi_frl table?

The newly introduced _xe3plpd_lt_trans_hdmi_frl table contains the entry
{ .lt = { 0, 0, 0, 48, 0 } } at index 0. This index is never accessed
during FRL training because intel_hdmi_frl_level() strictly returns 1-4.
The index 0 entry (voltage swing 48) appears to be explicitly designed
for HDMI TMDS.

However, by gating the use of this table with intel_hdmi_is_frl() here
in xe3plpd_get_lt_buf_trans(), HDMI TMDS configurations bypass it and
fall through to the xe3plpd_lt_trans_dp14 fallback table.

Because of the missing .hdmi_default_entry discussed above, this fallback
causes HDMI TMDS outputs to resolve to an invalid voltage swing of 21.

Could this result in poor signal integrity or link training failures for
TMDS HDMI sinks?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012035806.3361439-1-ankit.k.nautiyal@intel.com?part=23