Re: [PATCH] drm/i915/display: Read DP_ADAPTER_CAP to pass LinkLayer DPCD&EDID tests

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 02 Jul 2025, "Almahallawy, Khaled" <khaled.almahallawy@intel.com> wrote:
> On Tue, 2025-07-01 at 21:50 +0000, Cavitt, Jonathan wrote:
>> -----Original Message-----
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
>> Of Khaled Almahallawy
>> Sent: Tuesday, July 1, 2025 12:27 PM
>> To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org
>> Cc: Almahallawy, Khaled <khaled.almahallawy@intel.com>; Shankar, Uma
>> <uma.shankar@intel.com>; Nikula, Jani <jani.nikula@intel.com>
>> Subject: [PATCH] drm/i915/display: Read DP_ADAPTER_CAP to pass
>> LinkLayer DPCD&EDID tests
>> > 
>> > 4.2.2.1 LinkLayer test states:
>> > "Verify that Source DUT does the following within 5 seconds after
>> > HPD
>> > being asserted:
>> > Reads the DPCD Receiver Capability field (DPCD: 00000h:0000Fh)"
>> > ...
>> > 
>> > Fail1:
>> > Source DUT failed to read the DPCD Receiver Capability field (DPCD:
>> > 00000h:0000Fh) through AUX_CH before link training."
>> > 
>> > Fix this by reading DP_ADAPTER_CAP(0x000F & 0x220F)
>> > 
>> > Cc: Uma Shankar <uma.shankar@intel.com>
>> > Cc: Jani Nikula <jani.nikula@intel.com>
>> > Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_dp_link_training.c | 7 +++++++
>> >  1 file changed, 7 insertions(+)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> > b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> > index a479b63112ea..762dc073b824 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> > @@ -252,6 +252,7 @@ int intel_dp_init_lttpr_and_dprx_caps(struct
>> > intel_dp *intel_dp)
>> >  {
>> >  	struct intel_display *display =
>> > to_intel_display(intel_dp);
>> >  	int lttpr_count = 0;
>> > +	u8 adapter_cap = 0;
>> >  
>> >  	/*
>> >  	 * Detecting LTTPRs must be avoided on platforms with an
>> > AUX timeout
>> > @@ -277,6 +278,12 @@ int intel_dp_init_lttpr_and_dprx_caps(struct
>> > intel_dp *intel_dp)
>> >  		return -EIO;
>> >  	}
>> >  
>> > +	/* Read DP_ADAPTET_CAP to pass LinkLayer CTS */
>> > +	drm_dp_dpcd_readb(&intel_dp->aux, DP_ADAPTER_CAP,
>> > +				   &adapter_cap);
>> 
>> I'm working under the assumption that we're reading these registers
>> not to actually use
>> the values they hold but because reading them has some other effect
>> during runtime.
>> 
>> Does reading that location clear the register field?  Or does it
>> signal to the system to
>> perform some other process?
>
> We read these two bytes for the sole purpose of passing the test. If we
> don't read these two addresses, the test equipment will fail the test.
> Unfortunately, this issue affects nearly all DPCD/EDID LL tests. It
> seems that reading these two bytes has no side effect at all, as it
> appears that no one is using DP_ADAPTER_CAP at all.
>
>> 
>> > +	drm_dp_dpcd_readb(&intel_dp->aux, 0x220f,
>> > +				   &adapter_cap);
>> 
>> 0x220f should probably have a #define macro associated with it. 
>> Could you please create one?
>> Maybe it needs to be added to drm_dp.h.  Or is there a better
>> location for it?
>
> sure, I can do that.
>
> Another approach I considered is to increase DP_RECEIVER_CAP_SIZE from
> 0xf to 0x10, in line with DP2.1 section 5.1.4 "If the link is unstable
> or lost, the Source device then reads the DPCD Receiver Capabilities
> registers at DPCD 00000h through 0000Fh to determine the appropriate
> information needed to train the link.". 
>
> This adjustment might also ensure the test succeeds with other drivers!

Increasing DP_RECEIVER_CAP_SIZE to 0x10 sounds much better to me than
adding single dummy reads.

BR,
Jani.


>
> Thank You for your review
>
> Khaled
>
>> -Jonathan Cavitt
>> 
>> > +
>> >  	return lttpr_count;
>> >  }
>> >  
>> > -- 
>> > 2.43.0
>> > 
>> > 
>

-- 
Jani Nikula, Intel