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

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

On Wed, 2025-07-02 at 11:14 +0300, Jani Nikula wrote:
> 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.

Thank you for giving the go-ahead. Patch sent.

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