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

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

Commit Message

Matt Atwood March 14, 2018, 5:40 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.
V4: style changes
V5: typo
V6: print statement revisions, DP_REV to DPCD_REV, comment correction

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

Comments

Vivi, Rodrigo March 14, 2018, 8:22 p.m. UTC | #1
On Wed, Mar 14, 2018 at 10:40:08AM -0700, 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.
> V4: style changes
> V5: typo
> V6: print statement revisions, DP_REV to DPCD_REV, comment correction
> 
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++----
>  include/drm/drm_dp_helper.h     |  6 ++++++
>  2 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index adf79be..392e92e 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)\n", rd_interval);
> +
> +	if (rd_interval == 0 || (dpcd[DP_DPCD_REV] >= DP_REV_14))

s/DP_REV_14/DPCD_REV_14 right?

>  		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)\n", 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..9afea9f 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -64,6 +64,11 @@
>  /* AUX CH addresses */
>  /* DPCD */
>  #define DP_DPCD_REV                         0x000
> +# define DPCD_REV_10                        0x10
> +# define DPCD_REV_11                        0x11
> +# define DPCD_REV_12                        0x12
> +# define DPCD_REV_13                        0x13
> +# define DPCD_REV_14                        0x14
>  
>  #define DP_MAX_LINK_RATE                    0x001
>  
> @@ -118,6 +123,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    /* XXX 1.2 */
>  
>  #define DP_ADAPTER_CAP			    0x00f   /* 1.2 */
>  # define DP_FORCE_LOAD_SENSE_CAP	    (1 << 0)
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
kernel test robot March 16, 2018, 11:47 a.m. UTC | #2
Hi Matt,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v4.16-rc4]
[also build test WARNING on next-20180316]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/matthew-s-atwood-intel-com/drm-dp-Correctly-mask-DP_TRAINING_AUX_RD_INTERVAL-values-for-DP-1-4/20180316-185136
config: i386-randconfig-x003-201810 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:10:0,
                    from drivers/gpu/drm/drm_dp_helper.c:23:
   drivers/gpu/drm/drm_dp_helper.c: In function 'drm_dp_link_train_clock_recovery_delay':
   drivers/gpu/drm/drm_dp_helper.c:127:48: error: 'DP_REV_14' undeclared (first use in this function); did you mean 'DPCD_REV_14'?
     if (rd_interval == 0 || (dpcd[DP_DPCD_REV] >= DP_REV_14))
                                                   ^
   include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> drivers/gpu/drm/drm_dp_helper.c:127:2: note: in expansion of macro 'if'
     if (rd_interval == 0 || (dpcd[DP_DPCD_REV] >= DP_REV_14))
     ^~
   drivers/gpu/drm/drm_dp_helper.c:127:48: note: each undeclared identifier is reported only once for each function it appears in
     if (rd_interval == 0 || (dpcd[DP_DPCD_REV] >= DP_REV_14))
                                                   ^
   include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> drivers/gpu/drm/drm_dp_helper.c:127:2: note: in expansion of macro 'if'
     if (rd_interval == 0 || (dpcd[DP_DPCD_REV] >= DP_REV_14))
     ^~

vim +/if +127 drivers/gpu/drm/drm_dp_helper.c

  > 23	#include <linux/kernel.h>
    24	#include <linux/module.h>
    25	#include <linux/delay.h>
    26	#include <linux/init.h>
    27	#include <linux/errno.h>
    28	#include <linux/sched.h>
    29	#include <linux/i2c.h>
    30	#include <linux/seq_file.h>
    31	#include <drm/drm_dp_helper.h>
    32	#include <drm/drmP.h>
    33	
    34	#include "drm_crtc_helper_internal.h"
    35	
    36	/**
    37	 * DOC: dp helpers
    38	 *
    39	 * These functions contain some common logic and helpers at various abstraction
    40	 * levels to deal with Display Port sink devices and related things like DP aux
    41	 * channel transfers, EDID reading over DP aux channels, decoding certain DPCD
    42	 * blocks, ...
    43	 */
    44	
    45	/* Helpers for DP link training */
    46	static u8 dp_link_status(const u8 link_status[DP_LINK_STATUS_SIZE], int r)
    47	{
    48		return link_status[r - DP_LANE0_1_STATUS];
    49	}
    50	
    51	static u8 dp_get_lane_status(const u8 link_status[DP_LINK_STATUS_SIZE],
    52				     int lane)
    53	{
    54		int i = DP_LANE0_1_STATUS + (lane >> 1);
    55		int s = (lane & 1) * 4;
    56		u8 l = dp_link_status(link_status, i);
    57		return (l >> s) & 0xf;
    58	}
    59	
    60	bool drm_dp_channel_eq_ok(const u8 link_status[DP_LINK_STATUS_SIZE],
    61				  int lane_count)
    62	{
    63		u8 lane_align;
    64		u8 lane_status;
    65		int lane;
    66	
    67		lane_align = dp_link_status(link_status,
    68					    DP_LANE_ALIGN_STATUS_UPDATED);
    69		if ((lane_align & DP_INTERLANE_ALIGN_DONE) == 0)
    70			return false;
    71		for (lane = 0; lane < lane_count; lane++) {
    72			lane_status = dp_get_lane_status(link_status, lane);
    73			if ((lane_status & DP_CHANNEL_EQ_BITS) != DP_CHANNEL_EQ_BITS)
    74				return false;
    75		}
    76		return true;
    77	}
    78	EXPORT_SYMBOL(drm_dp_channel_eq_ok);
    79	
    80	bool drm_dp_clock_recovery_ok(const u8 link_status[DP_LINK_STATUS_SIZE],
    81				      int lane_count)
    82	{
    83		int lane;
    84		u8 lane_status;
    85	
    86		for (lane = 0; lane < lane_count; lane++) {
    87			lane_status = dp_get_lane_status(link_status, lane);
    88			if ((lane_status & DP_LANE_CR_DONE) == 0)
    89				return false;
    90		}
    91		return true;
    92	}
    93	EXPORT_SYMBOL(drm_dp_clock_recovery_ok);
    94	
    95	u8 drm_dp_get_adjust_request_voltage(const u8 link_status[DP_LINK_STATUS_SIZE],
    96					     int lane)
    97	{
    98		int i = DP_ADJUST_REQUEST_LANE0_1 + (lane >> 1);
    99		int s = ((lane & 1) ?
   100			 DP_ADJUST_VOLTAGE_SWING_LANE1_SHIFT :
   101			 DP_ADJUST_VOLTAGE_SWING_LANE0_SHIFT);
   102		u8 l = dp_link_status(link_status, i);
   103	
   104		return ((l >> s) & 0x3) << DP_TRAIN_VOLTAGE_SWING_SHIFT;
   105	}
   106	EXPORT_SYMBOL(drm_dp_get_adjust_request_voltage);
   107	
   108	u8 drm_dp_get_adjust_request_pre_emphasis(const u8 link_status[DP_LINK_STATUS_SIZE],
   109						  int lane)
   110	{
   111		int i = DP_ADJUST_REQUEST_LANE0_1 + (lane >> 1);
   112		int s = ((lane & 1) ?
   113			 DP_ADJUST_PRE_EMPHASIS_LANE1_SHIFT :
   114			 DP_ADJUST_PRE_EMPHASIS_LANE0_SHIFT);
   115		u8 l = dp_link_status(link_status, i);
   116	
   117		return ((l >> s) & 0x3) << DP_TRAIN_PRE_EMPHASIS_SHIFT;
   118	}
   119	EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis);
   120	
   121	void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
   122		int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK;
   123	
   124		if (rd_interval > 4)
   125			DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)\n", rd_interval);
   126	
 > 127		if (rd_interval == 0 || (dpcd[DP_DPCD_REV] >= DP_REV_14))
   128			udelay(100);
   129		else
   130			mdelay(rd_interval * 4);
   131	}
   132	EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
   133	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Patch
diff mbox

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index adf79be..392e92e 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)\n", 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)\n", 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..9afea9f 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -64,6 +64,11 @@ 
 /* AUX CH addresses */
 /* DPCD */
 #define DP_DPCD_REV                         0x000
+# define DPCD_REV_10                        0x10
+# define DPCD_REV_11                        0x11
+# define DPCD_REV_12                        0x12
+# define DPCD_REV_13                        0x13
+# define DPCD_REV_14                        0x14
 
 #define DP_MAX_LINK_RATE                    0x001
 
@@ -118,6 +123,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    /* XXX 1.2 */
 
 #define DP_ADAPTER_CAP			    0x00f   /* 1.2 */
 # define DP_FORCE_LOAD_SENSE_CAP	    (1 << 0)