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 <[email protected]> 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/[email protected]?part=23
