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

["Almahallawy, Khaled" <khaled.almahallawy@intel.com>]

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!

Thank You for your review

Khaled

> -Jonathan Cavitt
> 
> > +
> >  	return lttpr_count;
> >  }
> >  
> > -- 
> > 2.43.0
> > 
> >