drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
diff mbox

Message ID 1520361468-32087-1-git-send-email-matthew.s.atwood@intel.com
State New
Headers show

Commit Message

Matt Atwood March 6, 2018, 6:37 p.m. UTC
From: Matt Atwood <matthew.s.atwood@intel.com>

DP_TRAINING_AUX_RD_INTERVAL with DP 1.3 spec changed bit scheme from 8
bits to 7 bits in DPCD 0x000e. The 8th bit describes a new feature, for
panels that use this new feature, this would cause a wait interval for
clock recovery of at least 512 ms, much higher then spec maximum of 16 ms.
This behavior is described in table 2-158 of DP 1.4 spec address 0000Eh.
To avoid breaking panels that are not spec compliant we now warn on
invalid values.

V2: commit title/message, masking all 7 bits, warn on out of spec values.

Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++----
 include/drm/drm_dp_helper.h     |  1 +
 2 files changed, 15 insertions(+), 4 deletions(-)

Comments

Benson Leung March 6, 2018, 7:21 p.m. UTC | #1
Hi Matt,

On Tue, Mar 06, 2018 at 10:37:48AM -0800, matthew.s.atwood@intel.com wrote:
> From: Matt Atwood <matthew.s.atwood@intel.com>
> 
> DP_TRAINING_AUX_RD_INTERVAL with DP 1.3 spec changed bit scheme from 8
> bits to 7 bits in DPCD 0x000e. The 8th bit describes a new feature, for
> panels that use this new feature, this would cause a wait interval for
> clock recovery of at least 512 ms, much higher then spec maximum of 16 ms.
> This behavior is described in table 2-158 of DP 1.4 spec address 0000Eh.
> To avoid breaking panels that are not spec compliant we now warn on
> invalid values.
> 
> V2: commit title/message, masking all 7 bits, warn on out of spec values.
> 
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>

Tested-by: Benson Leung <bleung@chromium.org>

Tested this patch on a DP 1.3 panel which sets the
EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT bit in DPCD 0000Eh. It has
a value of 0x80 in that field, indicating the extended caps, and 400us for
the Main-Link Channel Equalization phase.

Confirmed that link training passes normally where prior to this it would fail
after the driver waits too long.

Thanks for the fix!
Vivi, Rodrigo March 6, 2018, 11:24 p.m. UTC | #2
On Tue, Mar 06, 2018 at 10:37:48AM -0800, matthew.s.atwood@intel.com wrote:
> From: Matt Atwood <matthew.s.atwood@intel.com>
> 
> DP_TRAINING_AUX_RD_INTERVAL with DP 1.3 spec changed bit scheme from 8
> bits to 7 bits in DPCD 0x000e. The 8th bit describes a new feature, for
> panels that use this new feature, this would cause a wait interval for
> clock recovery of at least 512 ms, much higher then spec maximum of 16 ms.
> This behavior is described in table 2-158 of DP 1.4 spec address 0000Eh.
> To avoid breaking panels that are not spec compliant we now warn on
> invalid values.
> 
> V2: commit title/message, masking all 7 bits, warn on out of spec values.

this approach is even better imho.

> 
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++----
>  include/drm/drm_dp_helper.h     |  1 +
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index adf79be..a718ccc 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -119,18 +119,28 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 link_status[DP_LINK_STATUS_SI
>  EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis);
>  
>  void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
> -	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
> +	int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK;
> +
> +	if (rd_interval > 4)
> +		DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)", rd_interval);
> +
> +	if (rd_interval == 0)
>  		udelay(100);
>  	else
> -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> +		mdelay(rd_interval * 4);
>  }
>  EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
>  
>  void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
> -	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
> +	int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK;
> +
> +	if (rd_interval > 4)
> +		DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)", rd_interval);
> +
> +	if (rd_interval == 0)
>  		udelay(400);
>  	else
> -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> +		mdelay(rd_interval * 4);
>  }
>  EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);
>  
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index da58a42..f80acf1 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -118,6 +118,7 @@
>  # define DP_DPCD_DISPLAY_CONTROL_CAPABLE     (1 << 3) /* edp v1.2 or higher */
>  
>  #define DP_TRAINING_AUX_RD_INTERVAL         0x00e   /* XXX 1.2? */
> +# define DP_TRAINING_AUX_RD_MASK            0x7F     /* 1.3 */
>  
>  #define DP_ADAPTER_CAP			    0x00f   /* 1.2 */
>  # define DP_FORCE_LOAD_SENSE_CAP	    (1 << 0)
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Dhinakaran Pandiyan March 7, 2018, 12:24 a.m. UTC | #3
On Tue, 2018-03-06 at 15:24 -0800, Rodrigo Vivi wrote:
> On Tue, Mar 06, 2018 at 10:37:48AM -0800, matthew.s.atwood@intel.com wrote:

> > From: Matt Atwood <matthew.s.atwood@intel.com>

> > 

> > DP_TRAINING_AUX_RD_INTERVAL with DP 1.3 spec changed bit scheme from 8

> > bits to 7 bits in DPCD 0x000e. The 8th bit describes a new feature, for

> > panels that use this new feature, this would cause a wait interval for

> > clock recovery of at least 512 ms, much higher then spec maximum of 16 ms.

> > This behavior is described in table 2-158 of DP 1.4 spec address 0000Eh.

> > To avoid breaking panels 


See comment below:

> that are not spec compliant we now warn on

> > invalid values.

> > 

> > V2: commit title/message, masking all 7 bits, warn on out of spec values.

> 

> this approach is even better imho.

> 

> > 

> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>

> 

> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 

> > ---

> >  drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++----

> >  include/drm/drm_dp_helper.h     |  1 +

> >  2 files changed, 15 insertions(+), 4 deletions(-)

> > 

> > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c

> > index adf79be..a718ccc 100644

> > --- a/drivers/gpu/drm/drm_dp_helper.c

> > +++ b/drivers/gpu/drm/drm_dp_helper.c

> > @@ -119,18 +119,28 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 link_status[DP_LINK_STATUS_SI

> >  EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis);

> >  

> >  void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {

> > -	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)

> > +	int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK;

> > +

> > +	if (rd_interval > 4)

> > +		DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)", rd_interval);


Some default for panels without a valid value?
		rd_interval = 4;
		"AUX read interval out of range, using max %d ms"

	      
> > +

> > +	if (rd_interval == 0)

> >  		udelay(100);

> >  	else

> > -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);

> > +		mdelay(rd_interval * 4);

> >  }

> >  EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);

> >  

> >  void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {

> > -	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)

> > +	int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK;

> > +

> > +	if (rd_interval > 4)

> > +		DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)", rd_interval);

> > +

> > +	if (rd_interval == 0)

> >  		udelay(400);

> >  	else

> > -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);

> > +		mdelay(rd_interval * 4);

> >  }

> >  EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);

> >  

> > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h

> > index da58a42..f80acf1 100644

> > --- a/include/drm/drm_dp_helper.h

> > +++ b/include/drm/drm_dp_helper.h

> > @@ -118,6 +118,7 @@

> >  # define DP_DPCD_DISPLAY_CONTROL_CAPABLE     (1 << 3) /* edp v1.2 or higher */

> >  

> >  #define DP_TRAINING_AUX_RD_INTERVAL         0x00e   /* XXX 1.2? */

> > +# define DP_TRAINING_AUX_RD_MASK            0x7F     /* 1.3 */

> >  

> >  #define DP_ADAPTER_CAP			    0x00f   /* 1.2 */

> >  # define DP_FORCE_LOAD_SENSE_CAP	    (1 << 0)

> > -- 

> > 2.7.4

> > 

> > _______________________________________________

> > Intel-gfx mailing list

> > Intel-gfx@lists.freedesktop.org

> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

> _______________________________________________

> Intel-gfx mailing list

> Intel-gfx@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Dhinakaran Pandiyan March 7, 2018, 12:41 a.m. UTC | #4
On Tue, 2018-03-06 at 16:48 -0800, Dhinakaran Pandiyan wrote:
> 

> 

> On Tue, 2018-03-06 at 15:24 -0800, Rodrigo Vivi wrote:

> > On Tue, Mar 06, 2018 at 10:37:48AM -0800, matthew.s.atwood@intel.com wrote:

> > > From: Matt Atwood <matthew.s.atwood@intel.com>

> > > 

> > > DP_TRAINING_AUX_RD_INTERVAL with DP 1.3 spec changed bit scheme from 8

> > > bits to 7 bits in DPCD 0x000e. The 8th bit describes a new feature, for

> > > panels that use this new feature, this would cause a wait interval for

> > > clock recovery of at least 512 ms, much higher then spec maximum of 16 ms.

> > > This behavior is described in table 2-158 of DP 1.4 spec address 0000Eh.

> > > To avoid breaking panels 

> 

> See comment below:

> 

> > that are not spec compliant we now warn on

> > > invalid values.

> > > 

> > > V2: commit title/message, masking all 7 bits, warn on out of spec values.

> > 

> > this approach is even better imho.

> > 

> > > 

> > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>

> > 

> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> > 

> > > ---

> > >  drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++----

> > >  include/drm/drm_dp_helper.h     |  1 +

> > >  2 files changed, 15 insertions(+), 4 deletions(-)

> > > 

> > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c

> > > index adf79be..a718ccc 100644

> > > --- a/drivers/gpu/drm/drm_dp_helper.c

> > > +++ b/drivers/gpu/drm/drm_dp_helper.c

> > > @@ -119,18 +119,28 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 link_status[DP_LINK_STATUS_SI

> > >  EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis);

> > >  

> > >  void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {

> > > -	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)

> > > +	int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK;

> > > +

> > > +	if (rd_interval > 4)

> > > +		DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)", rd_interval);

> 

> Some default for panels without a valid value?

> 		rd_interval = 4;

> 		"AUX read interval out of range, using max %d ms"

> 

> 	      

> > > +

> > > +	if (rd_interval == 0)

> > >  		udelay(100);

> > >  	else

> > > -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);

> > > +		mdelay(rd_interval * 4);


Btw, DP 1.4 spec also says this is 100 us for *all* values in the
register. Updating that for DP 1.4 panels should speed up link
training. 


> > >  }

> > >  EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);

> > >  

> > >  void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {

> > > -	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)

> > > +	int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK;

> > > +

> > > +	if (rd_interval > 4)

> > > +		DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)", rd_interval);

> > > +

> > > +	if (rd_interval == 0)

> > >  		udelay(400);

> > >  	else

> > > -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);

> > > +		mdelay(rd_interval * 4);

> > >  }

> > >  EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);

> > >  

> > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h

> > > index da58a42..f80acf1 100644

> > > --- a/include/drm/drm_dp_helper.h

> > > +++ b/include/drm/drm_dp_helper.h

> > > @@ -118,6 +118,7 @@

> > >  # define DP_DPCD_DISPLAY_CONTROL_CAPABLE     (1 << 3) /* edp v1.2 or higher */

> > >  

> > >  #define DP_TRAINING_AUX_RD_INTERVAL         0x00e   /* XXX 1.2? */

> > > +# define DP_TRAINING_AUX_RD_MASK            0x7F     /* 1.3 */

> > >  

> > >  #define DP_ADAPTER_CAP			    0x00f   /* 1.2 */

> > >  # define DP_FORCE_LOAD_SENSE_CAP	    (1 << 0)

> > > -- 

> > > 2.7.4

> > > 

> > > _______________________________________________

> > > Intel-gfx mailing list

> > > Intel-gfx@lists.freedesktop.org

> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

> > _______________________________________________

> > Intel-gfx mailing list

> > Intel-gfx@lists.freedesktop.org

> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Manasi Navare March 7, 2018, 1:36 a.m. UTC | #5
On Wed, Mar 07, 2018 at 12:24:46AM +0000, Pandiyan, Dhinakaran wrote:
> 
> 
> 
> On Tue, 2018-03-06 at 15:24 -0800, Rodrigo Vivi wrote:
> > On Tue, Mar 06, 2018 at 10:37:48AM -0800, matthew.s.atwood@intel.com wrote:
> > > From: Matt Atwood <matthew.s.atwood@intel.com>
> > > 
> > > DP_TRAINING_AUX_RD_INTERVAL with DP 1.3 spec changed bit scheme from 8
> > > bits to 7 bits in DPCD 0x000e. The 8th bit describes a new feature, for
> > > panels that use this new feature, this would cause a wait interval for
> > > clock recovery of at least 512 ms, much higher then spec maximum of 16 ms.
> > > This behavior is described in table 2-158 of DP 1.4 spec address 0000Eh.
> > > To avoid breaking panels 
> 
> See comment below:
> 
> > that are not spec compliant we now warn on
> > > invalid values.
> > > 
> > > V2: commit title/message, masking all 7 bits, warn on out of spec values.
> > 
> > this approach is even better imho.
> > 
> > > 
> > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > 
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > > ---
> > >  drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++----
> > >  include/drm/drm_dp_helper.h     |  1 +
> > >  2 files changed, 15 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> > > index adf79be..a718ccc 100644
> > > --- a/drivers/gpu/drm/drm_dp_helper.c
> > > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > > @@ -119,18 +119,28 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 link_status[DP_LINK_STATUS_SI
> > >  EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis);
> > >  
> > >  void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
> > > -	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
> > > +	int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK;
> > > +
> > > +	if (rd_interval > 4)
> > > +		DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)", rd_interval);
> 
> Some default for panels without a valid value?
> 		rd_interval = 4;
> 		"AUX read interval out of range, using max %d ms"
>

The problem with setting the upper bound to 4 is that there are panels
that do not follow the spec and expect a longer than 16 ms delay. So
if we set the upper bound to 4 in those cases the panels might not work.

So we decided to go with this approach where we tell the users that panel is requesting
out of range AUX value but then set it to the value * 4 in the else part.

Manasi
 
> 	      
> > > +
> > > +	if (rd_interval == 0)
> > >  		udelay(100);
> > >  	else
> > > -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> > > +		mdelay(rd_interval * 4);
> > >  }
> > >  EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
> > >  
> > >  void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
> > > -	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
> > > +	int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK;
> > > +
> > > +	if (rd_interval > 4)
> > > +		DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)", rd_interval);
> > > +
> > > +	if (rd_interval == 0)
> > >  		udelay(400);
> > >  	else
> > > -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> > > +		mdelay(rd_interval * 4);
> > >  }
> > >  EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);
> > >  
> > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> > > index da58a42..f80acf1 100644
> > > --- a/include/drm/drm_dp_helper.h
> > > +++ b/include/drm/drm_dp_helper.h
> > > @@ -118,6 +118,7 @@
> > >  # define DP_DPCD_DISPLAY_CONTROL_CAPABLE     (1 << 3) /* edp v1.2 or higher */
> > >  
> > >  #define DP_TRAINING_AUX_RD_INTERVAL         0x00e   /* XXX 1.2? */
> > > +# define DP_TRAINING_AUX_RD_MASK            0x7F     /* 1.3 */
> > >  
> > >  #define DP_ADAPTER_CAP			    0x00f   /* 1.2 */
> > >  # define DP_FORCE_LOAD_SENSE_CAP	    (1 << 0)
> > > -- 
> > > 2.7.4
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Dhinakaran Pandiyan March 7, 2018, 2:13 a.m. UTC | #6
On Tue, 2018-03-06 at 17:36 -0800, Manasi Navare wrote:
> On Wed, Mar 07, 2018 at 12:24:46AM +0000, Pandiyan, Dhinakaran wrote:

> > 

> > 

> > 

> > On Tue, 2018-03-06 at 15:24 -0800, Rodrigo Vivi wrote:

> > > On Tue, Mar 06, 2018 at 10:37:48AM -0800, matthew.s.atwood@intel.com wrote:

> > > > From: Matt Atwood <matthew.s.atwood@intel.com>

> > > > 

> > > > DP_TRAINING_AUX_RD_INTERVAL with DP 1.3 spec changed bit scheme from 8

> > > > bits to 7 bits in DPCD 0x000e. The 8th bit describes a new feature, for

> > > > panels that use this new feature, this would cause a wait interval for

> > > > clock recovery of at least 512 ms, much higher then spec maximum of 16 ms.

> > > > This behavior is described in table 2-158 of DP 1.4 spec address 0000Eh.

> > > > To avoid breaking panels 

> > 

> > See comment below:

> > 

> > > that are not spec compliant we now warn on

> > > > invalid values.

> > > > 

> > > > V2: commit title/message, masking all 7 bits, warn on out of spec values.

> > > 

> > > this approach is even better imho.

> > > 

> > > > 

> > > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>

> > > 

> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> > > 

> > > > ---

> > > >  drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++----

> > > >  include/drm/drm_dp_helper.h     |  1 +

> > > >  2 files changed, 15 insertions(+), 4 deletions(-)

> > > > 

> > > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c

> > > > index adf79be..a718ccc 100644

> > > > --- a/drivers/gpu/drm/drm_dp_helper.c

> > > > +++ b/drivers/gpu/drm/drm_dp_helper.c

> > > > @@ -119,18 +119,28 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 link_status[DP_LINK_STATUS_SI

> > > >  EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis);

> > > >  

> > > >  void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {

> > > > -	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)

> > > > +	int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK;

> > > > +

> > > > +	if (rd_interval > 4)

> > > > +		DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)", rd_interval);

> > 

> > Some default for panels without a valid value?

> > 		rd_interval = 4;

> > 		"AUX read interval out of range, using max %d ms"

> >

> 

> The problem with setting the upper bound to 4 is that there are panels

> that do not follow the spec and expect a longer than 16 ms delay. So

> if we set the upper bound to 4 in those cases the panels might not work.

> 

> So we decided to go with this approach where we tell the users that panel is requesting

> out of range AUX value but then set it to the value * 4 in the else part.

> 


Thanks for the clarification. My concern is if the DPCD is advertizing
an out of spec value, it might as well be advertizing a delay that the
panel doesn't need. And I thought panel quirks were supposed to be used
for working around things like this. To be clear, this is not a big
enough concern to block this fix.

Like I said in the other email, this patch refers to DP 1.4, shouldn't
the clock recovery delay be updated too (in a separate patch)?


> Manasi

>  

> > 	      

> > > > +

> > > > +	if (rd_interval == 0)

> > > >  		udelay(100);

> > > >  	else

> > > > -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);

> > > > +		mdelay(rd_interval * 4);

> > > >  }

> > > >  EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);

> > > >  

> > > >  void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {

> > > > -	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)

> > > > +	int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK;

> > > > +

> > > > +	if (rd_interval > 4)

> > > > +		DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)", rd_interval);

> > > > +

> > > > +	if (rd_interval == 0)

> > > >  		udelay(400);

> > > >  	else

> > > > -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);

> > > > +		mdelay(rd_interval * 4);

> > > >  }

> > > >  EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);

> > > >  

> > > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h

> > > > index da58a42..f80acf1 100644

> > > > --- a/include/drm/drm_dp_helper.h

> > > > +++ b/include/drm/drm_dp_helper.h

> > > > @@ -118,6 +118,7 @@

> > > >  # define DP_DPCD_DISPLAY_CONTROL_CAPABLE     (1 << 3) /* edp v1.2 or higher */

> > > >  

> > > >  #define DP_TRAINING_AUX_RD_INTERVAL         0x00e   /* XXX 1.2? */

> > > > +# define DP_TRAINING_AUX_RD_MASK            0x7F     /* 1.3 */

> > > >  

> > > >  #define DP_ADAPTER_CAP			    0x00f   /* 1.2 */

> > > >  # define DP_FORCE_LOAD_SENSE_CAP	    (1 << 0)

> > > > -- 

> > > > 2.7.4

> > > > 

> > > > _______________________________________________

> > > > Intel-gfx mailing list

> > > > Intel-gfx@lists.freedesktop.org

> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

> > > _______________________________________________

> > > Intel-gfx mailing list

> > > Intel-gfx@lists.freedesktop.org

> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

> > _______________________________________________

> > Intel-gfx mailing list

> > Intel-gfx@lists.freedesktop.org

> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Vivi, Rodrigo March 7, 2018, 10:06 p.m. UTC | #7
On Wed, Mar 07, 2018 at 02:13:21AM +0000, Pandiyan, Dhinakaran wrote:
> 
> 
> 
> On Tue, 2018-03-06 at 17:36 -0800, Manasi Navare wrote:
> > On Wed, Mar 07, 2018 at 12:24:46AM +0000, Pandiyan, Dhinakaran wrote:
> > > 
> > > 
> > > 
> > > On Tue, 2018-03-06 at 15:24 -0800, Rodrigo Vivi wrote:
> > > > On Tue, Mar 06, 2018 at 10:37:48AM -0800, matthew.s.atwood@intel.com wrote:
> > > > > From: Matt Atwood <matthew.s.atwood@intel.com>
> > > > > 
> > > > > DP_TRAINING_AUX_RD_INTERVAL with DP 1.3 spec changed bit scheme from 8
> > > > > bits to 7 bits in DPCD 0x000e. The 8th bit describes a new feature, for
> > > > > panels that use this new feature, this would cause a wait interval for
> > > > > clock recovery of at least 512 ms, much higher then spec maximum of 16 ms.
> > > > > This behavior is described in table 2-158 of DP 1.4 spec address 0000Eh.
> > > > > To avoid breaking panels 
> > > 
> > > See comment below:
> > > 
> > > > that are not spec compliant we now warn on
> > > > > invalid values.
> > > > > 
> > > > > V2: commit title/message, masking all 7 bits, warn on out of spec values.
> > > > 
> > > > this approach is even better imho.
> > > > 
> > > > > 
> > > > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > > > 
> > > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > 
> > > > > ---
> > > > >  drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++----
> > > > >  include/drm/drm_dp_helper.h     |  1 +
> > > > >  2 files changed, 15 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> > > > > index adf79be..a718ccc 100644
> > > > > --- a/drivers/gpu/drm/drm_dp_helper.c
> > > > > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > > > > @@ -119,18 +119,28 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 link_status[DP_LINK_STATUS_SI
> > > > >  EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis);
> > > > >  
> > > > >  void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
> > > > > -	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
> > > > > +	int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK;
> > > > > +
> > > > > +	if (rd_interval > 4)
> > > > > +		DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)", rd_interval);
> > > 
> > > Some default for panels without a valid value?
> > > 		rd_interval = 4;
> > > 		"AUX read interval out of range, using max %d ms"
> > >
> > 
> > The problem with setting the upper bound to 4 is that there are panels
> > that do not follow the spec and expect a longer than 16 ms delay. So
> > if we set the upper bound to 4 in those cases the panels might not work.
> > 
> > So we decided to go with this approach where we tell the users that panel is requesting
> > out of range AUX value but then set it to the value * 4 in the else part.
> > 
> 
> Thanks for the clarification. My concern is if the DPCD is advertizing
> an out of spec value, it might as well be advertizing a delay that the
> panel doesn't need. And I thought panel quirks were supposed to be used
> for working around things like this. To be clear, this is not a big
> enough concern to block this fix.
> 
> Like I said in the other email, this patch refers to DP 1.4, shouldn't
> the clock recovery delay be updated too (in a separate patch)?

We clearly need more work here.

I can see here on DP-v1.2a_d11:

00h = 100us for the Main Link Clock Recovery phase 400us for the Main Link Channel
Equalization phase and for FAUX training.
01h = 4ms all.
02h = 8ms all.
03h = 12ms all.
04h = 16ms all.

So probably the initial mask on this patch should be marked with /* XXX 1.2? */
because it clearly got introduced in some 1.2 minor release.

But even for DP 1.2 it doesn't seem we are doing it right on the 0 case.
It seems that we are using 100us for both channel eq and clock recovery, right?
or am I missing something?

Then DP 1.3 keeps same config.

But DP 1.4 change all values.

clock recovery is always 100us and channel eq is depending on this bit * 4 and 400us when bit is zeroed.

But limited to 4.

So we probably need 3 patches here:
1. - This one to protect against bad panels masking it and mentioning DP 1.2,
     nor 1.3 or 1.4. Also limiting rd_interval to 4 as DK suggested. Panels cannot
     expect all drivers are using this value * 4 blindly since it is not on spec.
2. - Fix channel eq for 0 case since 1.2. It should be 400us.
3. - For DP version >= 1.4 always use 100us for clock req or follow this register for
     channel eq.

Thoughts?

> 
> 
> > Manasi
> >  
> > > 	      
> > > > > +
> > > > > +	if (rd_interval == 0)
> > > > >  		udelay(100);
> > > > >  	else
> > > > > -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> > > > > +		mdelay(rd_interval * 4);
> > > > >  }
> > > > >  EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
> > > > >  
> > > > >  void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
> > > > > -	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
> > > > > +	int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK;
> > > > > +
> > > > > +	if (rd_interval > 4)
> > > > > +		DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)", rd_interval);
> > > > > +
> > > > > +	if (rd_interval == 0)
> > > > >  		udelay(400);
> > > > >  	else
> > > > > -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> > > > > +		mdelay(rd_interval * 4);
> > > > >  }
> > > > >  EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);
> > > > >  
> > > > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> > > > > index da58a42..f80acf1 100644
> > > > > --- a/include/drm/drm_dp_helper.h
> > > > > +++ b/include/drm/drm_dp_helper.h
> > > > > @@ -118,6 +118,7 @@
> > > > >  # define DP_DPCD_DISPLAY_CONTROL_CAPABLE     (1 << 3) /* edp v1.2 or higher */
> > > > >  
> > > > >  #define DP_TRAINING_AUX_RD_INTERVAL         0x00e   /* XXX 1.2? */
> > > > > +# define DP_TRAINING_AUX_RD_MASK            0x7F     /* 1.3 */
> > > > >  
> > > > >  #define DP_ADAPTER_CAP			    0x00f   /* 1.2 */
> > > > >  # define DP_FORCE_LOAD_SENSE_CAP	    (1 << 0)
> > > > > -- 
> > > > > 2.7.4
> > > > > 
> > > > > _______________________________________________
> > > > > Intel-gfx mailing list
> > > > > Intel-gfx@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Manasi Navare March 7, 2018, 10:20 p.m. UTC | #8
On Wed, Mar 07, 2018 at 02:06:08PM -0800, Rodrigo Vivi wrote:
> On Wed, Mar 07, 2018 at 02:13:21AM +0000, Pandiyan, Dhinakaran wrote:
> > 
> > 
> > 
> > On Tue, 2018-03-06 at 17:36 -0800, Manasi Navare wrote:
> > > On Wed, Mar 07, 2018 at 12:24:46AM +0000, Pandiyan, Dhinakaran wrote:
> > > > 
> > > > 
> > > > 
> > > > On Tue, 2018-03-06 at 15:24 -0800, Rodrigo Vivi wrote:
> > > > > On Tue, Mar 06, 2018 at 10:37:48AM -0800, matthew.s.atwood@intel.com wrote:
> > > > > > From: Matt Atwood <matthew.s.atwood@intel.com>
> > > > > > 
> > > > > > DP_TRAINING_AUX_RD_INTERVAL with DP 1.3 spec changed bit scheme from 8
> > > > > > bits to 7 bits in DPCD 0x000e. The 8th bit describes a new feature, for
> > > > > > panels that use this new feature, this would cause a wait interval for
> > > > > > clock recovery of at least 512 ms, much higher then spec maximum of 16 ms.
> > > > > > This behavior is described in table 2-158 of DP 1.4 spec address 0000Eh.
> > > > > > To avoid breaking panels 
> > > > 
> > > > See comment below:
> > > > 
> > > > > that are not spec compliant we now warn on
> > > > > > invalid values.
> > > > > > 
> > > > > > V2: commit title/message, masking all 7 bits, warn on out of spec values.
> > > > > 
> > > > > this approach is even better imho.
> > > > > 
> > > > > > 
> > > > > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > > > > 
> > > > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > 
> > > > > > ---
> > > > > >  drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++----
> > > > > >  include/drm/drm_dp_helper.h     |  1 +
> > > > > >  2 files changed, 15 insertions(+), 4 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> > > > > > index adf79be..a718ccc 100644
> > > > > > --- a/drivers/gpu/drm/drm_dp_helper.c
> > > > > > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > > > > > @@ -119,18 +119,28 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 link_status[DP_LINK_STATUS_SI
> > > > > >  EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis);
> > > > > >  
> > > > > >  void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
> > > > > > -	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
> > > > > > +	int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK;
> > > > > > +
> > > > > > +	if (rd_interval > 4)
> > > > > > +		DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)", rd_interval);
> > > > 
> > > > Some default for panels without a valid value?
> > > > 		rd_interval = 4;
> > > > 		"AUX read interval out of range, using max %d ms"
> > > >
> > > 
> > > The problem with setting the upper bound to 4 is that there are panels
> > > that do not follow the spec and expect a longer than 16 ms delay. So
> > > if we set the upper bound to 4 in those cases the panels might not work.
> > > 
> > > So we decided to go with this approach where we tell the users that panel is requesting
> > > out of range AUX value but then set it to the value * 4 in the else part.
> > > 
> > 
> > Thanks for the clarification. My concern is if the DPCD is advertizing
> > an out of spec value, it might as well be advertizing a delay that the
> > panel doesn't need. And I thought panel quirks were supposed to be used
> > for working around things like this. To be clear, this is not a big
> > enough concern to block this fix.
> > 
> > Like I said in the other email, this patch refers to DP 1.4, shouldn't
> > the clock recovery delay be updated too (in a separate patch)?
> 
> We clearly need more work here.
> 
> I can see here on DP-v1.2a_d11:
> 
> 00h = 100us for the Main Link Clock Recovery phase 400us for the Main Link Channel
> Equalization phase and for FAUX training.
> 01h = 4ms all.
> 02h = 8ms all.
> 03h = 12ms all.
> 04h = 16ms all.
> 
> So probably the initial mask on this patch should be marked with /* XXX 1.2? */
> because it clearly got introduced in some 1.2 minor release.
> 
> But even for DP 1.2 it doesn't seem we are doing it right on the 0 case.
> It seems that we are using 100us for both channel eq and clock recovery, right?
> or am I missing something?
> 
> Then DP 1.3 keeps same config.
> 
> But DP 1.4 change all values.
> 
> clock recovery is always 100us and channel eq is depending on this bit * 4 and 400us when bit is zeroed.
> 
> But limited to 4.
> 
> So we probably need 3 patches here:
> 1. - This one to protect against bad panels masking it and mentioning DP 1.2,
>      nor 1.3 or 1.4. Also limiting rd_interval to 4 as DK suggested. Panels cannot
>      expect all drivers are using this value * 4 blindly since it is not on spec.

So if some panels still expect a greater delay, those will fail link training. But
yes if we want them to be DP compliant, just follow the spec, limit it to the max value of 4 with
a warning.

> 2. - Fix channel eq for 0 case since 1.2. It should be 400us.

Channel eq is 400 us for DP 1.2, 1.3 and 1.4 and then *4 for all other values.
We are doing that correctly here. So no change there.


> 3. - For DP version >= 1.4 always use 100us for clock req or follow this register for
>      channel eq.
>

yes this needs to be fixed for DP REV >= 1.4

Manasi
 
> Thoughts?
> 
> > 
> > 
> > > Manasi
> > >  
> > > > 	      
> > > > > > +
> > > > > > +	if (rd_interval == 0)
> > > > > >  		udelay(100);
> > > > > >  	else
> > > > > > -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> > > > > > +		mdelay(rd_interval * 4);
> > > > > >  }
> > > > > >  EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
> > > > > >  
> > > > > >  void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
> > > > > > -	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
> > > > > > +	int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK;
> > > > > > +
> > > > > > +	if (rd_interval > 4)
> > > > > > +		DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)", rd_interval);
> > > > > > +
> > > > > > +	if (rd_interval == 0)
> > > > > >  		udelay(400);
> > > > > >  	else
> > > > > > -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> > > > > > +		mdelay(rd_interval * 4);
> > > > > >  }
> > > > > >  EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);
> > > > > >  
> > > > > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> > > > > > index da58a42..f80acf1 100644
> > > > > > --- a/include/drm/drm_dp_helper.h
> > > > > > +++ b/include/drm/drm_dp_helper.h
> > > > > > @@ -118,6 +118,7 @@
> > > > > >  # define DP_DPCD_DISPLAY_CONTROL_CAPABLE     (1 << 3) /* edp v1.2 or higher */
> > > > > >  
> > > > > >  #define DP_TRAINING_AUX_RD_INTERVAL         0x00e   /* XXX 1.2? */
> > > > > > +# define DP_TRAINING_AUX_RD_MASK            0x7F     /* 1.3 */
> > > > > >  
> > > > > >  #define DP_ADAPTER_CAP			    0x00f   /* 1.2 */
> > > > > >  # define DP_FORCE_LOAD_SENSE_CAP	    (1 << 0)
> > > > > > -- 
> > > > > > 2.7.4
> > > > > > 
> > > > > > _______________________________________________
> > > > > > Intel-gfx mailing list
> > > > > > Intel-gfx@lists.freedesktop.org
> > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > > _______________________________________________
> > > > > Intel-gfx mailing list
> > > > > Intel-gfx@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patch
diff mbox

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index adf79be..a718ccc 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -119,18 +119,28 @@  u8 drm_dp_get_adjust_request_pre_emphasis(const u8 link_status[DP_LINK_STATUS_SI
 EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis);
 
 void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
-	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
+	int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK;
+
+	if (rd_interval > 4)
+		DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)", rd_interval);
+
+	if (rd_interval == 0)
 		udelay(100);
 	else
-		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
+		mdelay(rd_interval * 4);
 }
 EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
 
 void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
-	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
+	int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK;
+
+	if (rd_interval > 4)
+		DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)", rd_interval);
+
+	if (rd_interval == 0)
 		udelay(400);
 	else
-		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
+		mdelay(rd_interval * 4);
 }
 EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);
 
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index da58a42..f80acf1 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -118,6 +118,7 @@ 
 # define DP_DPCD_DISPLAY_CONTROL_CAPABLE     (1 << 3) /* edp v1.2 or higher */
 
 #define DP_TRAINING_AUX_RD_INTERVAL         0x00e   /* XXX 1.2? */
+# define DP_TRAINING_AUX_RD_MASK            0x7F     /* 1.3 */
 
 #define DP_ADAPTER_CAP			    0x00f   /* 1.2 */
 # define DP_FORCE_LOAD_SENSE_CAP	    (1 << 0)