RE: [PATCH i-g-t v5 0/4] kms_dp_linktrain_fallback: Add UHBR-to-HBR fallback subtest

On Mon, 17 Aug 2026, "S, Sowmiya" <[email protected]> wrote:
> Hi @Nikula, Jani,
>
>> -----Original Message-----
>> From: Nikula, Jani <[email protected]>
>> Sent: 17 August 2026 14:32
>> To: S, Sowmiya <[email protected]>; [email protected]
>> Cc: B S, Karthik <[email protected]>; Joshi, Kunal1
>> <[email protected]>; S, Sowmiya <[email protected]>
>> Subject: Re: [PATCH i-g-t v5 0/4] kms_dp_linktrain_fallback: Add UHBR-to-HBR
>> fallback subtest
>> 
>> On Mon, 17 Aug 2026, Sowmiya S <[email protected]> wrote:
>> > DP 2.x links use 128b/132b channel encoding for UHBR rates
>> > (UHBR10/13.5/20) and 8b/10b for legacy HBR3 and below. The transition
>> > between the two encodings goes through a port-slice reset on the
>> > source side. The existing dp-fallback subtest only covers
>> > 8b/10b-to-8b/10b fallback; this series adds a dedicated subtest for the UHBR-
>> >HBR direction.
>> >
>> > Adds two library helpers to lib/i915/i915_dp:
>> >   - i915_dp_is_uhbr_rate(): mirrors the kernel's drm_dp_is_uhbr_rate()
>> >   - i915_dp_get_next_lower_rate(): parses the force_link_rate debugfs list
>> >     with proper strtok_r/strtol/errno handling
>> 
>> It occurs to me anything named "i915" is misleading here. This is about Intel
>> display shared between the i915 and xe drivers, not specific to
>> i915 alone.
>> 
>> BR,
>> Jani.
>> 
> Agreed on the naming being misleading - but every other function in i915_dp.c/.h already uses the i915_dp_ prefix (i915_dp_get_current_link_rate(), i915_dp_force_link_retrain(), i915_dp_reset_link_params(), etc.), and all of those are used by the same xe-capable tests too. If I rename just these two new functions to intel_dp_* now, the file ends up with two inconsistent naming conventions side by side, which is arguably more confusing than the current single (if misleadingly-named) one.
>
> If you can confirm, I can add these two following the existing i915_dp_ convention for now, and send a separate follow-up patch renaming the whole file's API (and called from other functions) to intel_dp*? 

The long term goal should be to migrate towards driver agnostic naming
for Intel display stuff.

BR,
Jani.


>> >
>> > const char * signature for i915_dp_set_link_params() so string
>> > literals can be passed directly without a writable-array workaround.
>> >
>> > Fixes a pre-existing bug where the file-static
>> > traversed_mst_output_count was never reset between subtests, causing
>> > all MST outputs to be silently skipped as "already visited" in a full-binary run.
>> >
>> > Adds the uhbr-to-hbr-fallback subtest. The subtest pins the link at
>> > the highest sustainable UHBR rate (stepping down if the cable cannot
>> > sustain the sink's max), forces repeated LT_FAILURE_REDUCED_CAPS
>> > failures, and asserts the rate drops below UHBR10. MST topologies are
>> > supported since all siblings share the physical link. The existing
>> > run_lt_fallback_test() is extended with force_uhbr parameter to avoid ~45
>> lines of duplication.
>> >
>> > v2: Resets MST traversal state per test run
>> > v3:
>> >   - Drop "exercising the port slice reset path" from SUBTEST description
>> >   - Revert unrelated cosmetic hunk in setup_mst_outputs()
>> >   - Single UHBR capability gate using i915_dp_get_max_link_rate()
>> >   - remove duplicate gate using i915_dp_get_max_supported_rate()
>> >   - Fix link rate log units (10 kbit/s, not kbps)
>> >   - Add igt_reset_connectors() to pin-loop early-return path
>> >   - Replace UHBR10_LINK_RATE define with i915_dp_is_uhbr_rate() helper
>> >   - Replace open-coded strtok() parsing with i915_dp_get_next_lower_rate()
>> >   - Hard-assert "fallback not reached" instead of returning false
>> >   - Remove dead traversed_mst_output_count reset in run_dsc path
>> >   - Merge run_uhbr_to_hbr_fallback_test() into run_lt_fallback_test()
>> >   - Use "auto" string literal directly (const char * param fix)
>> >   - Move reset before UHBR capability gate read in run_lt_fallback_test()
>> >   - Document force_uhbr parameter and "clear the pin" reset behaviour
>> > v4:
>> >   - name the UHBR10 threshold and make i915_dp_is_uhbr_rate() a
>> >     static inline in the header to avoid a cross-TU call
>> >   - parse intel_dp_allowed_link_configs instead of the source-only
>> >     i915_dp_force_link_rate list in i915_dp_get_next_lower_rate()
>> >   - avoid the implicit long to int narrowing on the return value
>> > v5: commit display reset before fallback
>> >
>> > Sowmiya S (4):
>> >   lib/i915/i915_dp: Add UHBR helpers and const-correct set_link_params
>> >   tests/intel/kms_dp_linktrain_fallback: Reset traversed MST output
>> >     count
>> >   tests/intel/kms_dp_linktrain_fallback: Add UHBR-to-HBR fallback
>> >     subtest
>> >   tests/intel/kms_dp_linktrain_fallback: commit display reset before
>> >     fallback
>> >
>> >  lib/i915/i915_dp.c                      |  58 ++++++-
>> >  lib/i915/i915_dp.h                      |  24 ++-
>> >  tests/intel/kms_dp_linktrain_fallback.c | 202
>> > +++++++++++++++++++++---
>> >  3 files changed, 260 insertions(+), 24 deletions(-)
>> 
>> --
>> Jani Nikula, Intel

-- 
Jani Nikula, Intel