Re: [PATCH] drm/dp: Keep base enhanced framing cap from extended DPCD caps

[Dmitry Malovanyy]

On Wed, 28 Oct 2026, Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:
> Additionally, a DPRX with DPCD r1.2+ must indicate support for enhanced
> framing. The display is also in violation of the spec.
>
> That said, the spec kind of says to use enhanced framing for DPRX with
> DPCD r1.2+. It's subtle, but it doesn't say check the capability
> anymore, just the DPCD revision. That's my reading of DP v1.1a, v1.2a,
> v1.3, v1.4a and v2.1a anyway.

Thanks for the quick review!

Yes, I checked DP 1.2 and it is there in 2.2.1.1, "An upstream device
interoperating with a downstream device with DPCD Revision of 1.2 or
higher must enable Enhanced Framing Mode". So doing it in
drm_dp_enhanced_frame_cap() makes more sense, agree.

> I think drm_dp_enhanced_frame_cap() should probably be:
>
> static inline bool
> drm_dp_enhanced_frame_cap(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> {
>         return dpcd[DP_DPCD_REV] >= 0x12 || dpcd[DP_DPCD_REV] >= 0x11 &&
>                 (dpcd[DP_MAX_LANE_COUNT] & DP_ENHANCED_FRAME_CAP);
> }
>
> And the remaining direct references to DP_ENHANCED_FRAME_CAP should be
> migrated to use drm_dp_enhanced_frame_cap().

Will do it in v2. I will add parens around the && part, otherwise gcc
complains with -Wparentheses.

For the direct users I found cdns-mhdp8546, it6505 and gma500, I will
convert them into separate patches.

There is also nouveau, dispnv50/disp.c checks the bit for the watermark
calculation, but the actual enable is in nvkm/engine/disp/dp.c with its
own definition. If I change only disp.c they will not agree with each
other. Should I convert both, or leave nouveau out for now?

drm_dp_get_phy_test_pattern() also uses DP_ENHANCED_FRAME_CAP but there
it is the DP_TEST_LANE_COUNT register, so I would leave it as is.

> One of the main reasons I don't like this is that we have the logging
> for the base and extended caps, and this sneaks in a change without it
> being present in the logs.

Makes sense, I didn't think about the logs.

Thanks,
Dmytro

On Wed, 28 Oct 2026 at 18:24, Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:
>
> On Wed, 28 Oct 2026, Dmytro Malovanyi <dmitrymalovanyy@xxxxxxxxx> wrote:
> > Some sinks set DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT and advertise
> > DP_ENHANCED_FRAME_CAP in the base receiver caps, but clear it in the
> > extended receiver caps at 0x2200. drm_dp_read_extended_dpcd_caps()
> > replaces the base caps with the extended ones, so the link is trained
> > without enhanced framing.
> >
> > The Xiaomi Monitor A24i (P24FBA-RAGL) is one such sink:
> >
> >   base:     12 0a c4 01 01 00 01 80 02 00 06 00 00 00 81
> >   extended: 14 0a 44 00 01 00 01 80 02 02 06 00 00 00 81
> >
> > Without enhanced framing all lanes train (0x202-0x204: 77 77 81), but
> > the sink never syncs to the stream (SINK_STATUS 0x205: 00) and shows
> > "No signal" until it is power cycled. The same monitor works on macOS.
> >
> > DP 1.4 section 2.9.3.1, as quoted in the commit that added extended
> > caps support, says the extended caps match the base caps except for
> > DPCD_REV, MAX_LINK_RATE and DOWN_STREAM_PORT_PRESENT. Keep the base
> > value of DP_ENHANCED_FRAME_CAP.
>
> Thanks for the patch and explanation, refreshing clarity.
>
> The display is obviously being silly.
>
> Additionally, a DPRX with DPCD r1.2+ must indicate support for enhanced
> framing. The display is also in violation of the spec.
>
> That said, the spec kind of says to use enhanced framing for DPRX with
> DPCD r1.2+. It's subtle, but it doesn't say check the capability
> anymore, just the DPCD revision. That's my reading of DP v1.1a, v1.2a,
> v1.3, v1.4a and v2.1a anyway.
>
> So I guess that might be the more appropriate fix, really. Even if it's
> slightly more work.
>
> I think drm_dp_enhanced_frame_cap() should probably be:
>
> static inline bool
> drm_dp_enhanced_frame_cap(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> {
>         return dpcd[DP_DPCD_REV] >= 0x12 || dpcd[DP_DPCD_REV] >= 0x11 &&
>                 (dpcd[DP_MAX_LANE_COUNT] & DP_ENHANCED_FRAME_CAP);
> }
>
> And the remaining direct references to DP_ENHANCED_FRAME_CAP should be
> migrated to use drm_dp_enhanced_frame_cap().
>
> > With this change the link uses enhanced framing (0x101: 84) and the
> > sink syncs (0x205: 03).
> >
> > Tested on i915 (Arrow Lake-U) through a USB4 dock.
> >
> > Fixes: a1d926527b79 ("drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/17719
> > Signed-off-by: Dmytro Malovanyi <dmitrymalovanyy@xxxxxxxxx>
> > ---
> >  drivers/gpu/drm/display/drm_dp_helper.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> > index e623ccb4c..205b6cc89 100644
> > --- a/drivers/gpu/drm/display/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> > @@ -1219,6 +1219,9 @@ static int drm_dp_read_extended_dpcd_caps(struct drm_dp_aux *aux,
> >
> >       drm_dbg_kms(aux->drm_dev, "%s: Base DPCD: %*ph\n", aux->name, DP_RECEIVER_CAP_SIZE, dpcd);
> >
> > +     /* Keep the base enhanced framing cap if the extended caps drop it */
> > +     dpcd_ext[DP_MAX_LANE_COUNT] |= dpcd[DP_MAX_LANE_COUNT] & DP_ENHANCED_FRAME_CAP;
>
> One of the main reasons I don't like this is that we have the logging
> for the base and extended caps, and this sneaks in a change without it
> being present in the logs.
>
> And having the decision in drm_dp_enhanced_frame_cap() is explicit and
> same for everyone.
>
> BR,
> Jani.
>
> > +
> >       memcpy(dpcd, dpcd_ext, sizeof(dpcd_ext));
> >
> >       return 0;
> >
> > base-commit: feaa32dc98090b2f45f7b6f179711a6640169c1a
>
> --
> Jani Nikula, Intel