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

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

Commit Message

Matt Atwood March 7, 2018, 11:44 p.m. UTC
From: Matt Atwood <matthew.s.atwood@intel.com>

DP_TRAINING_AUX_RD_INTERVAL with DP 1.3 spec changed bit scheeme from 8
bits to 7 in DPCD 0x000e. The 8th bit is used to identify extended
receiver capabilities. For panels that use this new feature wait interval
would be increased by 512 ms, when spec is max 16 ms. This behavior is
described in table 2-158 of DP 1.4 spec address 0000eh.

With the introduction of DP 1.4 spec main link clock recovery was
standardized to 100 us regardless of TRAINING_AUX_RD_INTERVAL value.

To avoid breaking panels that are not spec compiant we now warn on
invalid values.

V2: commit title/message, masking all 7 bits, warn on out of spec values.
V3: commit message, make link train clock recovery follow DP 1.4 spec.

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

Comments

Ilia Mirkin March 7, 2018, 11:58 p.m. UTC | #1
On Wed, Mar 7, 2018 at 6:44 PM,  <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 scheeme from 8
> bits to 7 in DPCD 0x000e. The 8th bit is used to identify extended
> receiver capabilities. For panels that use this new feature wait interval
> would be increased by 512 ms, when spec is max 16 ms. This behavior is
> described in table 2-158 of DP 1.4 spec address 0000eh.
>
> With the introduction of DP 1.4 spec main link clock recovery was
> standardized to 100 us regardless of TRAINING_AUX_RD_INTERVAL value.
>
> To avoid breaking panels that are not spec compiant we now warn on
> invalid values.
>
> V2: commit title/message, masking all 7 bits, warn on out of spec values.
> V3: commit message, make link train clock recovery follow DP 1.4 spec.
>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++----
>  include/drm/drm_dp_helper.h     |  4 ++++
>  2 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index adf79be..671b823 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 || (dpcd[DP_DPCD_REV] & DP_REV_14))

Was this meant to be dpcd[DP_DPCD_REV] >= DP_REV_14? It doesn't appear
to be a bitmask...

Also I think you're supposed to say "if (" rather than "if(".

  -ilia
Navare, Manasi March 8, 2018, 12:18 a.m. UTC | #2
On Wed, Mar 07, 2018 at 03:44:09PM -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 scheeme from 8
> bits to 7 in DPCD 0x000e. The 8th bit is used to identify extended
> receiver capabilities. For panels that use this new feature wait interval
> would be increased by 512 ms, when spec is max 16 ms. This behavior is
> described in table 2-158 of DP 1.4 spec address 0000eh.
> 
> With the introduction of DP 1.4 spec main link clock recovery was
> standardized to 100 us regardless of TRAINING_AUX_RD_INTERVAL value.
> 
> To avoid breaking panels that are not spec compiant we now warn on
> invalid values.
> 
> V2: commit title/message, masking all 7 bits, warn on out of spec values.
> V3: commit message, make link train clock recovery follow DP 1.4 spec.
> 
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++----
>  include/drm/drm_dp_helper.h     |  4 ++++
>  2 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index adf79be..671b823 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);

I thought there were comments about setting this to a max of 4 if its greater
than 4.

> +
> +	if(rd_interval == 0 || (dpcd[DP_DPCD_REV] & DP_REV_14))
         ^ space needed between if and open bracket

>  		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..5bac397 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -64,6 +64,9 @@
>  /* AUX CH addresses */
>  /* DPCD */
>  #define DP_DPCD_REV                         0x000
> +# define DP_REV_12                          0x012
> +# define DP_REV_13                          0x013
> +# define DP_REV_14                          0x014
>

IMHO, if we are creating these #defines for revisions then we should
do it for all values so 0x10, 0x11.

Manasi
  
>  #define DP_MAX_LINK_RATE                    0x001
>  
> @@ -118,6 +121,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

Patch
diff mbox

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index adf79be..671b823 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 || (dpcd[DP_DPCD_REV] & DP_REV_14))
 		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..5bac397 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -64,6 +64,9 @@ 
 /* AUX CH addresses */
 /* DPCD */
 #define DP_DPCD_REV                         0x000
+# define DP_REV_12                          0x012
+# define DP_REV_13                          0x013
+# define DP_REV_14                          0x014
 
 #define DP_MAX_LINK_RATE                    0x001
 
@@ -118,6 +121,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)