* Re: [RFC PATCH] drm/i915/dp: Ignore inconsistent TMDS limits on an Anker DP branch
[not found] <20260923221018.42133-1-conor10@gmail.com>
@ 2026-09-23 22:21 ` sashiko-bot
2026-09-24 8:03 ` Jani Nikula
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-09-23 22:21 UTC (permalink / raw)
To: Conor Svensson; +Cc: intel-xe, dri-devel, intel-gfx
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] drm/i915/dp: incomplete downstream ports signature match
--
commit 025075775ae9b93d6ea2089caff0dfbc102027cd
Author: Conor Svensson <conor10@gmail.com>
drm/i915/dp: Ignore inconsistent TMDS limits on an Anker DP branch
This commit adds an experimental workaround to clear derived TMDS limits
for a specific Anker DP branch device that reports inconsistent capabilities
when hotplugged, which can cause the loss of native modes.
Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/17180
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 0cd5e6b5034cf..beba989860056 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
[ ... ]
> +static bool
> +intel_dp_has_anker_tmds_mismatch(struct intel_dp *intel_dp,
> + const struct drm_edid *drm_edid)
> +{
> + static const struct drm_dp_dpcd_ident branch = {
> + .oui = { 0x90, 0xcc, 0x24 },
> + .device_id = { 'S', 'Y', 'N', 'A', 'b', 0x10 },
> + .hw_rev = 0x10,
> + .sw_major_rev = 0x06,
> + .sw_minor_rev = 0x05,
> + };
> + static const u8 downstream_ports[] = {
> + 0x0a, 0x42, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + };
[ ... ]
> + return !memcmp(&intel_dp->desc.ident, &branch, sizeof(branch)) &&
> + !memcmp(intel_dp->downstream_ports, downstream_ports,
> + sizeof(downstream_ports)) &&
[Severity: Low]
Does this comparison in intel_dp_has_anker_tmds_mismatch() strictly validate
the entire downstream port state as intended?
Because the local downstream_ports array is defined with only 8 elements,
sizeof(downstream_ports) evaluates to 8. However, intel_dp->downstream_ports
is a 16-byte array (DP_MAX_DOWNSTREAM_PORTS).
This partial match could theoretically trigger the quirk on a different device
that shares the same firmware and first 8 bytes of port capabilities but
differs in the remaining 8 bytes, which seems contrary to the deliberately
narrow intent described in the commit message.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260923221018.42133-1-conor10@gmail.com?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [RFC PATCH] drm/i915/dp: Ignore inconsistent TMDS limits on an Anker DP branch
[not found] <20260923221018.42133-1-conor10@gmail.com>
2026-09-23 22:21 ` [RFC PATCH] drm/i915/dp: Ignore inconsistent TMDS limits on an Anker DP branch sashiko-bot
@ 2026-09-24 8:03 ` Jani Nikula
[not found] ` <20260924211852.436787-1-conor10@gmail.com>
1 sibling, 1 reply; 3+ messages in thread
From: Jani Nikula @ 2026-09-24 8:03 UTC (permalink / raw)
To: Conor Svensson, intel-gfx
Cc: Conor Svensson, intel-xe, dri-devel, rodrigo.vivi,
joonas.lahtinen, tursulin, airlied, simona, linux-kernel
On Wed, 23 Sep 2026, Conor Svensson <conor10@gmail.com> wrote:
> Reconnecting an Anker 565 A8388 USB-C hub with a DisplayPort monitor
> attached can leave the branch reporting a DVI downstream descriptor
> with 25-165 MHz TMDS limits. On a ThinkPad L14 Gen 4 Intel with a Dell
> S3422DWG, this removes the native 3440x1440 mode from the connector's
> mode list even though the EDID still contains it. An explicit native
> modeline works with the same hardware state.
>
> Ignore the derived TMDS limits only for the captured branch identity,
> firmware and downstream descriptor when EDID 1.4 identifies a digital
> DisplayPort input. Retain the raw descriptor and other mode checks.
> Do not restrict the match to a particular monitor or laptop model.
>
> With otherwise matching upstream Linux 7.2.5 control/patched builds,
> the control loses native modes after USB-C reconnect, while the patched
> kernel selects 3440x1440 at 59.973 Hz automatically. Repeated reconnects,
> both USB-C ports, suspend/resume, undocking while asleep and docked boot
> pass. HDMI also works and does not activate the workaround. Periodic
> picture cycling observed on the control stops with the patch.
>
> The branch identity is not proven unique to this retail adapter. This
> is an experimental workaround for review, not an explanation of why the
> branch reports inconsistent capabilities. Other adapters and monitors
> have not been tested.
>
> Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/17180
> Assisted-by: LLM
> Signed-off-by: Conor Svensson <conor10@gmail.com>
> ---
> RFC: is this best handled as a branch quirk here, or in the common DP
> helpers? The captured identity and descriptor match is deliberately narrow;
> there is no monitor/laptop model restriction. The informational log marker
> is retained to show exactly when this experimental workaround executes.
> The commit introducing the underlying problem has not been identified, so
> there is no speculative Fixes tag.
>
> Hardware A/B testing used upstream Linux 7.2.5 with otherwise identical
> configurations. This RFC applies the same 48 added lines to drm-tip at the
> base commit below. This drm-tip revision has not been boot-tested. The affected intel_dp.o
> compiles successfully here with no compiler warnings; strict checkpatch
> passes (human sign-off pending), and 426 predicate boundary cases pass
> with ASan/UBSan. These predicate tests are not DRM integration tests.
>
> Results: three USB-C reconnects, alternate USB-C port, docked s2idle
> resume plus reconnect, undock while asleep/wake/reconnect, and docked
> DisplayPort reboot all passed. HDMI also passed without activating the
> workaround. The control lost native modes and showed periodic picture
> cycling; that cycling stopped on the patched kernel. No custom modeline
> was used in either test kernel. Other hardware and higher refresh rates
> remain untested.
>
> AI assistance: Codex (GPT-6) helped investigate the reported hotplug
> failure, wrote the match/limit-clearing change, prepared the predicate
> tests, collected diagnostics and drafted this message. The human reporter
> performed the physical reconnect, suspend and reboot tests and confirmed
> the visible results. The assistance arose from an extended troubleshooting
> session, rather than a single code-generation prompt.
>
> Evidence and detailed test results:
> https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/17180#note_3677750
>
> This is my first kernel patch submission. I'd appreciate feedback on
> whether this belongs here or in the common DP helpers, and whether the
> matching criteria are appropriate.
Thanks for the patch. Please let's first root cause the issue (on the
gitlab issue) before jumping into quirking specific devices.
Regardless, a few comments below.
>
> Thanks,
> Conor
>
> drivers/gpu/drm/i915/display/intel_dp.c | 48 +++++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index ffddf4b33..6d6767ca7 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -6153,6 +6153,39 @@ intel_dp_get_edid(struct intel_dp *intel_dp)
> return drm_edid_read_ddc(&connector->base, &intel_dp->aux.ddc);
> }
>
> +static bool
> +intel_dp_has_anker_tmds_mismatch(struct intel_dp *intel_dp,
> + const struct drm_edid *drm_edid)
> +{
> + static const struct drm_dp_dpcd_ident branch = {
> + .oui = { 0x90, 0xcc, 0x24 },
> + .device_id = { 'S', 'Y', 'N', 'A', 'b', 0x10 },
> + .hw_rev = 0x10,
> + .sw_major_rev = 0x06,
> + .sw_minor_rev = 0x05,
> + };
> + static const u8 downstream_ports[] = {
> + 0x0a, 0x42, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + };
intel_dp->downstream_ports is read from DPCD address 0x80, and it
dynamically reflects the downstream port information, i.e. what's
currently connected to the branch device. Having a fixed comparison like
this will only work for a specific type of downstream device connected
to a specific downstream port.
> + const struct edid *edid = drm_edid_raw(drm_edid);
No new drm_edid_raw() calls are to be added.
> +
> + if (intel_dp_is_edp(intel_dp) || intel_dp->is_mst ||
> + !drm_dp_is_branch(intel_dp->dpcd) || !edid)
> + return false;
> +
> + /* HDMI and DVI inputs must retain their downstream limits. */
> + if (edid->version != 1 || edid->revision < 4 ||
> + !(edid->input & DRM_EDID_INPUT_DIGITAL) ||
> + (edid->input & DRM_EDID_DIGITAL_TYPE_MASK) != DRM_EDID_DIGITAL_TYPE_DP)
> + return false;
> +
> + return !memcmp(&intel_dp->desc.ident, &branch, sizeof(branch)) &&
> + !memcmp(intel_dp->downstream_ports, downstream_ports,
> + sizeof(downstream_ports)) &&
> + intel_dp->dfp.min_tmds_clock == 25000 &&
> + intel_dp->dfp.max_tmds_clock == 165000;
> +}
We have intel_quirks.c with various structured mechanisms for
identifying quirks. If we end up needing a quirk for this, the existing
mechanisms need to be used instead of adding a bunch of code inline like
this.
Again, please let's first root cause and debug the issue instead of
trying to fix the patch.
BR,
Jani.
> +
> static void
> intel_dp_update_dfp(struct intel_dp *intel_dp,
> const struct drm_edid *drm_edid)
> @@ -6181,6 +6214,21 @@ intel_dp_update_dfp(struct intel_dp *intel_dp,
> drm_dp_get_pcon_max_frl_bw(intel_dp->dpcd,
> intel_dp->downstream_ports);
>
> + /*
> + * Experimental workaround for the branch observed in an Anker A8388.
> + * USB-C hotplug can expose a DVI descriptor for the DP output. The
> + * monitor's native timing works when requested explicitly, despite
> + * the reported 165 MHz limit. Keep the captured identity and failure
> + * signature checks narrow until the underlying cause is understood.
> + */
> + if (intel_dp_has_anker_tmds_mismatch(intel_dp, drm_edid)) {
> + intel_dp->dfp.min_tmds_clock = 0;
> + intel_dp->dfp.max_tmds_clock = 0;
> + drm_info(display->drm,
> + "[CONNECTOR:%d:%s] experimental Anker DP TMDS limit workaround\n",
> + connector->base.base.id, connector->base.name);
> + }
> +
> drm_dbg_kms(display->drm,
> "[CONNECTOR:%d:%s] DFP max bpc %d, max dotclock %d, TMDS clock %d-%d, PCON Max FRL BW %dGbps\n",
> connector->base.base.id, connector->base.name,
>
> base-commit: 703cd271db3a35088460ab3fec0a96bd48e7fe04
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 3+ messages in thread