All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values
@ 2018-03-02 22:25 matthew.s.atwood
  2018-03-02 23:22 ` Rodrigo Vivi
                   ` (12 more replies)
  0 siblings, 13 replies; 36+ messages in thread
From: matthew.s.atwood @ 2018-03-02 22:25 UTC (permalink / raw)
  To: dri-devel; +Cc: Matt Atwood

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

For panels that do not follow Display Port specifications mask off invalid
values for DP_TRAINING_AUX_RD_INTERVAL. Specification lists acceptable
values 0-4 all other values are reserved and bit 7 of DPCD 0x0000e
describes another feature. Currently the code uses all of DPCD 0x0000e and
can cause max wait for 1024 ms instead of 16 ms as specified table 2-158.
This address is read for both clock recovery and channel equalization.

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

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index adf79be..a7e9b75 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -122,7 +122,7 @@ void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
 	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
 		udelay(100);
 	else
-		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
+		mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4);
 }
 EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
 
@@ -130,7 +130,7 @@ void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
 	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
 		udelay(400);
 	else
-		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
+		mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 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..77ba003 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            0x7     /* 1.4 */
 
 #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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values
  2018-03-02 22:25 [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values matthew.s.atwood
@ 2018-03-02 23:22 ` Rodrigo Vivi
  2018-03-04 10:03   ` Jani Nikula
  2018-03-02 23:24 ` Manasi Navare
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 36+ messages in thread
From: Rodrigo Vivi @ 2018-03-02 23:22 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: dri-devel

On Fri, Mar 02, 2018 at 02:25:58PM -0800, matthew.s.atwood@intel.com wrote:
> From: Matt Atwood <matthew.s.atwood@intel.com>
> 
> For panels that do not follow Display Port specifications mask off invalid
> values for DP_TRAINING_AUX_RD_INTERVAL. Specification lists acceptable
> values 0-4 all other values are reserved and bit 7 of DPCD 0x0000e
> describes another feature. Currently the code uses all of DPCD 0x0000e and
> can cause max wait for 1024 ms instead of 16 ms as specified table 2-158.
> This address is read for both clock recovery and channel equalization.
> 
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 4 ++--
>  include/drm/drm_dp_helper.h     | 1 +
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index adf79be..a7e9b75 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -122,7 +122,7 @@ void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
>  	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
>  		udelay(100);
>  	else
> -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> +		mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4);
>  }
>  EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
>  
> @@ -130,7 +130,7 @@ void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
>  	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
>  		udelay(400);
>  	else
> -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> +		mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 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..77ba003 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            0x7     /* 1.4 */

I confirmed it is already part of 1.2 and 1.3
So probably a good idea to already to s/XXX 1.2?/1.2/
but this is optional and up to you.

With s/1.4/1.2 feel free to add:

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

(well... in a hope that no other bad panel uses 5,6 or 7,
that are also reserved values. But I believe these cases shouldn't
be as bad as this one you faced here anyways)

>  
>  #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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values
  2018-03-02 22:25 [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values matthew.s.atwood
  2018-03-02 23:22 ` Rodrigo Vivi
@ 2018-03-02 23:24 ` Manasi Navare
  2018-03-03  7:34 ` Benson Leung
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 36+ messages in thread
From: Manasi Navare @ 2018-03-02 23:24 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: dri-devel

Thanks for the patch. Masking the training AUX RD interval value to get
only the allowed values sounds of 0-4 is absolutely needed. Just one nit below:

On Fri, Mar 02, 2018 at 02:25:58PM -0800, matthew.s.atwood@intel.com wrote:
> From: Matt Atwood <matthew.s.atwood@intel.com>
> 
> For panels that do not follow Display Port specifications mask off invalid
> values for DP_TRAINING_AUX_RD_INTERVAL. Specification lists acceptable

It would be good to call out DP specification and a particular version number if possible
So DP 1.2 specification.

With that change,
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

> values 0-4 all other values are reserved and bit 7 of DPCD 0x0000e
> describes another feature. Currently the code uses all of DPCD 0x0000e and
> can cause max wait for 1024 ms instead of 16 ms as specified table 2-158.
> This address is read for both clock recovery and channel equalization.
>

> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 4 ++--
>  include/drm/drm_dp_helper.h     | 1 +
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index adf79be..a7e9b75 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -122,7 +122,7 @@ void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
>  	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
>  		udelay(100);
>  	else
> -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> +		mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4);
>  }
>  EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
>  
> @@ -130,7 +130,7 @@ void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
>  	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
>  		udelay(400);
>  	else
> -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> +		mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 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..77ba003 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            0x7     /* 1.4 */
>  
>  #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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values
  2018-03-02 22:25 [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values matthew.s.atwood
  2018-03-02 23:22 ` Rodrigo Vivi
  2018-03-02 23:24 ` Manasi Navare
@ 2018-03-03  7:34 ` Benson Leung
  2018-03-04 10:02 ` Jani Nikula
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 36+ messages in thread
From: Benson Leung @ 2018-03-03  7:34 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: manasi.d.navare, dri-devel, rodrigo.vivi


[-- Attachment #1.1: Type: text/plain, Size: 1006 bytes --]

Hi Matt,

On Fri, Mar 02, 2018 at 02:25:58PM -0800, matthew.s.atwood@intel.com wrote:
> From: Matt Atwood <matthew.s.atwood@intel.com>
> 
> For panels that do not follow Display Port specifications mask off invalid
> values for DP_TRAINING_AUX_RD_INTERVAL. Specification lists acceptable
> values 0-4 all other values are reserved and bit 7 of DPCD 0x0000e
> describes another feature. Currently the code uses all of DPCD 0x0000e and
> can cause max wait for 1024 ms instead of 16 ms as specified table 2-158.
> This address is read for both clock recovery and channel equalization.
> 
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>

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

I tested this patch on that particularly problematic panel, with the out of
spec value of 9. With the masking, the panel link trains successfully.

Thanks!

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values
  2018-03-02 22:25 [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values matthew.s.atwood
                   ` (2 preceding siblings ...)
  2018-03-03  7:34 ` Benson Leung
@ 2018-03-04 10:02 ` Jani Nikula
  2018-03-06 18:37 ` [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4 matthew.s.atwood
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 36+ messages in thread
From: Jani Nikula @ 2018-03-04 10:02 UTC (permalink / raw)
  To: dri-devel; +Cc: Manasi Navare, Matt Atwood, Vivi, Rodrigo

On Fri, 02 Mar 2018, matthew.s.atwood@intel.com wrote:
> From: Matt Atwood <matthew.s.atwood@intel.com>
>
> For panels that do not follow Display Port specifications mask off invalid
> values for DP_TRAINING_AUX_RD_INTERVAL. Specification lists acceptable
> values 0-4 all other values are reserved and bit 7 of DPCD 0x0000e
> describes another feature. Currently the code uses all of DPCD 0x0000e and
> can cause max wait for 1024 ms instead of 16 ms as specified table 2-158.
> This address is read for both clock recovery and channel equalization.
>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>

Cc: stable@vger.kernel.org

> ---
>  drivers/gpu/drm/drm_dp_helper.c | 4 ++--
>  include/drm/drm_dp_helper.h     | 1 +
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index adf79be..a7e9b75 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -122,7 +122,7 @@ void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
>  	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
>  		udelay(100);
>  	else
> -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> +		mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4);
>  }
>  EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
>  
> @@ -130,7 +130,7 @@ void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
>  	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
>  		udelay(400);
>  	else
> -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> +		mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 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..77ba003 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            0x7     /* 1.4 */
>  
>  #define DP_ADAPTER_CAP			    0x00f   /* 1.2 */
>  # define DP_FORCE_LOAD_SENSE_CAP	    (1 << 0)

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values
  2018-03-02 23:22 ` Rodrigo Vivi
@ 2018-03-04 10:03   ` Jani Nikula
  0 siblings, 0 replies; 36+ messages in thread
From: Jani Nikula @ 2018-03-04 10:03 UTC (permalink / raw)
  To: Rodrigo Vivi, matthew.s.atwood; +Cc: dri-devel

On Fri, 02 Mar 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Fri, Mar 02, 2018 at 02:25:58PM -0800, matthew.s.atwood@intel.com wrote:
>> From: Matt Atwood <matthew.s.atwood@intel.com>
>> 
>> For panels that do not follow Display Port specifications mask off invalid
>> values for DP_TRAINING_AUX_RD_INTERVAL. Specification lists acceptable
>> values 0-4 all other values are reserved and bit 7 of DPCD 0x0000e
>> describes another feature. Currently the code uses all of DPCD 0x0000e and
>> can cause max wait for 1024 ms instead of 16 ms as specified table 2-158.
>> This address is read for both clock recovery and channel equalization.
>> 
>> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
>> ---
>>  drivers/gpu/drm/drm_dp_helper.c | 4 ++--
>>  include/drm/drm_dp_helper.h     | 1 +
>>  2 files changed, 3 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
>> index adf79be..a7e9b75 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -122,7 +122,7 @@ void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
>>  	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
>>  		udelay(100);
>>  	else
>> -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
>> +		mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4);
>>  }
>>  EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
>>  
>> @@ -130,7 +130,7 @@ void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
>>  	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
>>  		udelay(400);
>>  	else
>> -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
>> +		mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 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..77ba003 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            0x7     /* 1.4 */
>
> I confirmed it is already part of 1.2 and 1.3
> So probably a good idea to already to s/XXX 1.2?/1.2/
> but this is optional and up to you.
>
> With s/1.4/1.2 feel free to add:
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Please send v2 Cc: intel-gfx so we get i915 CI on this. It doesn't
automatically happen on dri-devel yet.

BR,
Jani.

>
> (well... in a hope that no other bad panel uses 5,6 or 7,
> that are also reserved values. But I believe these cases shouldn't
> be as bad as this one you faced here anyways)
>
>>  
>>  #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
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-02 22:25 [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values matthew.s.atwood
                   ` (3 preceding siblings ...)
  2018-03-04 10:02 ` Jani Nikula
@ 2018-03-06 18:37 ` matthew.s.atwood
  2018-03-06 19:21   ` Benson Leung
  2018-03-06 23:24   ` Rodrigo Vivi
  2018-03-06 20:08 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (7 subsequent siblings)
  12 siblings, 2 replies; 36+ messages in thread
From: matthew.s.atwood @ 2018-03-06 18:37 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

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(-)

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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-06 18:37 ` [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4 matthew.s.atwood
@ 2018-03-06 19:21   ` Benson Leung
  2018-03-06 23:24   ` Rodrigo Vivi
  1 sibling, 0 replies; 36+ messages in thread
From: Benson Leung @ 2018-03-06 19:21 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: intel-gfx, bleung, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1324 bytes --]

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!

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* ✓ Fi.CI.BAT: success for drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-02 22:25 [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values matthew.s.atwood
                   ` (4 preceding siblings ...)
  2018-03-06 18:37 ` [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4 matthew.s.atwood
@ 2018-03-06 20:08 ` Patchwork
  2018-03-07  0:43 ` ✓ Fi.CI.IGT: " Patchwork
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 36+ messages in thread
From: Patchwork @ 2018-03-06 20:08 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: intel-gfx

== Series Details ==

Series: drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
URL   : https://patchwork.freedesktop.org/series/39473/
State : success

== Summary ==

Series 39473v1 drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
https://patchwork.freedesktop.org/api/1.0/series/39473/revisions/1/mbox/

---- Known issues:

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                fail       -> PASS       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:424s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:426s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:369s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:497s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:277s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:489s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:489s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:480s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:466s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:406s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:570s
fi-cfl-u         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:503s
fi-cnl-y3        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:590s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:412s
fi-gdg-551       total:288  pass:180  dwarn:0   dfail:0   fail:0   skip:108 time:291s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:518s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:396s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:408s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:455s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:417s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:466s
fi-kbl-7560u     total:108  pass:104  dwarn:0   dfail:0   fail:0   skip:3  
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:466s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:507s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:588s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:440s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:519s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:532s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:498s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:481s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:421s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:426s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:522s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:388s

72d7aac2959d8b0dcdbe6d5e7f072a07fbe2d377 drm-tip: 2018y-03m-06d-19h-09m-24s UTC integration manifest
92bb0aca39bc drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8248/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-06 18:37 ` [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4 matthew.s.atwood
  2018-03-06 19:21   ` Benson Leung
@ 2018-03-06 23:24   ` Rodrigo Vivi
  2018-03-07  0:24     ` Pandiyan, Dhinakaran
  1 sibling, 1 reply; 36+ messages in thread
From: Rodrigo Vivi @ 2018-03-06 23:24 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: intel-gfx, dri-devel

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-06 23:24   ` Rodrigo Vivi
@ 2018-03-07  0:24     ` Pandiyan, Dhinakaran
  2018-03-07  0:41       ` [Intel-gfx] " Pandiyan, Dhinakaran
  2018-03-07  1:36       ` Manasi Navare
  0 siblings, 2 replies; 36+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-03-07  0:24 UTC (permalink / raw)
  To: Vivi, Rodrigo; +Cc: intel-gfx, dri-devel




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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-07  0:24     ` Pandiyan, Dhinakaran
@ 2018-03-07  0:41       ` Pandiyan, Dhinakaran
  2018-03-07  1:36       ` Manasi Navare
  1 sibling, 0 replies; 36+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-03-07  0:41 UTC (permalink / raw)
  To: Vivi, Rodrigo; +Cc: intel-gfx, Atwood, Matthew S, dri-devel




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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* ✓ Fi.CI.IGT: success for drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-02 22:25 [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values matthew.s.atwood
                   ` (5 preceding siblings ...)
  2018-03-06 20:08 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-03-07  0:43 ` Patchwork
  2018-03-07 23:44 ` [PATCH] " matthew.s.atwood
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 36+ messages in thread
From: Patchwork @ 2018-03-07  0:43 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: intel-gfx

== Series Details ==

Series: drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
URL   : https://patchwork.freedesktop.org/series/39473/
State : success

== Summary ==

---- Possible new issues:

Test kms_busy:
        Subgroup extended-modeset-hang-newfb-render-b:
                dmesg-warn -> PASS       (shard-hsw)
Test kms_chv_cursor_fail:
        Subgroup pipe-c-64x64-right-edge:
                incomplete -> PASS       (shard-hsw)

---- Known issues:

Test gem_softpin:
        Subgroup noreloc-s3:
                incomplete -> PASS       (shard-hsw) fdo#103540
Test kms_chv_cursor_fail:
        Subgroup pipe-b-256x256-bottom-edge:
                dmesg-warn -> PASS       (shard-snb) fdo#105185 +2
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-primscrn-spr-indfb-draw-blt:
                skip       -> PASS       (shard-snb) fdo#101623

fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#105185 https://bugs.freedesktop.org/show_bug.cgi?id=105185
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623

shard-apl        total:3381 pass:1781 dwarn:1   dfail:0   fail:7   skip:1591 time:11865s
shard-hsw        total:3467 pass:1772 dwarn:1   dfail:0   fail:2   skip:1691 time:11961s
shard-snb        total:3467 pass:1362 dwarn:2   dfail:0   fail:2   skip:2101 time:6934s
Blacklisted hosts:
shard-kbl        total:3467 pass:1951 dwarn:1   dfail:0   fail:8   skip:1507 time:9487s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8248/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-07  0:24     ` Pandiyan, Dhinakaran
  2018-03-07  0:41       ` [Intel-gfx] " Pandiyan, Dhinakaran
@ 2018-03-07  1:36       ` Manasi Navare
  2018-03-07  2:13         ` Pandiyan, Dhinakaran
  1 sibling, 1 reply; 36+ messages in thread
From: Manasi Navare @ 2018-03-07  1:36 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran; +Cc: intel-gfx, dri-devel, Vivi, Rodrigo

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-07  1:36       ` Manasi Navare
@ 2018-03-07  2:13         ` Pandiyan, Dhinakaran
  2018-03-07 22:06           ` Rodrigo Vivi
  0 siblings, 1 reply; 36+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-03-07  2:13 UTC (permalink / raw)
  To: Navare, Manasi D; +Cc: intel-gfx, dri-devel, Vivi, Rodrigo




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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-07  2:13         ` Pandiyan, Dhinakaran
@ 2018-03-07 22:06           ` Rodrigo Vivi
  2018-03-07 22:20             ` [Intel-gfx] " Manasi Navare
  0 siblings, 1 reply; 36+ messages in thread
From: Rodrigo Vivi @ 2018-03-07 22:06 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran; +Cc: intel-gfx, dri-devel

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-07 22:06           ` Rodrigo Vivi
@ 2018-03-07 22:20             ` Manasi Navare
  0 siblings, 0 replies; 36+ messages in thread
From: Manasi Navare @ 2018-03-07 22:20 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, Pandiyan, Dhinakaran, dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-02 22:25 [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values matthew.s.atwood
                   ` (6 preceding siblings ...)
  2018-03-07  0:43 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-03-07 23:44 ` matthew.s.atwood
  2018-03-07 23:58   ` Ilia Mirkin
  2018-03-08  0:18   ` Manasi Navare
  2018-03-08  0:13 ` matthew.s.atwood
                   ` (4 subsequent siblings)
  12 siblings, 2 replies; 36+ messages in thread
From: matthew.s.atwood @ 2018-03-07 23:44 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Matt Atwood

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))
 		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)
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-07 23:44 ` [PATCH] " matthew.s.atwood
@ 2018-03-07 23:58   ` Ilia Mirkin
  2018-03-08  0:18   ` Manasi Navare
  1 sibling, 0 replies; 36+ messages in thread
From: Ilia Mirkin @ 2018-03-07 23:58 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: Intel Graphics Development, dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-02 22:25 [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values matthew.s.atwood
                   ` (7 preceding siblings ...)
  2018-03-07 23:44 ` [PATCH] " matthew.s.atwood
@ 2018-03-08  0:13 ` matthew.s.atwood
  2018-03-08  0:36   ` Benson Leung
  2018-03-08  0:28 ` matthew.s.atwood
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 36+ messages in thread
From: matthew.s.atwood @ 2018-03-08  0:13 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Matt Atwood

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

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..6985ff3 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..1269ef8 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 DP_REV_10                          0x10
+# define DP_REV_11                          0x11
+# define DP_REV_12                          0x12
+# define DP_REV_13                          0x13
+# define DP_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    /* 1.3 */
 
 #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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-07 23:44 ` [PATCH] " matthew.s.atwood
  2018-03-07 23:58   ` Ilia Mirkin
@ 2018-03-08  0:18   ` Manasi Navare
  1 sibling, 0 replies; 36+ messages in thread
From: Manasi Navare @ 2018-03-08  0:18 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: intel-gfx, dri-devel

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-02 22:25 [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values matthew.s.atwood
                   ` (8 preceding siblings ...)
  2018-03-08  0:13 ` matthew.s.atwood
@ 2018-03-08  0:28 ` matthew.s.atwood
  2018-03-08  0:49   ` Benson Leung
  2018-03-08  7:22   ` [Intel-gfx] " Jani Nikula
  2018-03-14 17:40 ` matthew.s.atwood
                   ` (2 subsequent siblings)
  12 siblings, 2 replies; 36+ messages in thread
From: matthew.s.atwood @ 2018-03-08  0:28 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Matt Atwood

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

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..cdb04c9 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..1269ef8 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 DP_REV_10                          0x10
+# define DP_REV_11                          0x11
+# define DP_REV_12                          0x12
+# define DP_REV_13                          0x13
+# define DP_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    /* 1.3 */
 
 #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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-08  0:13 ` matthew.s.atwood
@ 2018-03-08  0:36   ` Benson Leung
  0 siblings, 0 replies; 36+ messages in thread
From: Benson Leung @ 2018-03-08  0:36 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: intel-gfx, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 2658 bytes --]

Hey Matt,

Your patch doesn't build. Missing semicolon, dude.

On Wed, Mar 07, 2018 at 04:13:58PM -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.
> V4: style changes
> 
> 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..6985ff3 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)

Need a semicolon here.

/mnt/host/source/src/third_party/kernel/v4.14/drivers/gpu/drm/drm_dp_helper.c: In function 'drm_dp_link_train_clock_recovery_delay':
/mnt/host/source/src/third_party/kernel/v4.14/drivers/gpu/drm/drm_dp_helper.c:131:1: error: expected ';' before '}' token
 }
 ^
make[4]: *** [/mnt/host/source/src/third_party/kernel/v4.14/scripts/Makefile.build:320: drivers/gpu/drm/drm_dp_helper.o] Error 1


Thanks,
Benson

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-08  0:28 ` matthew.s.atwood
@ 2018-03-08  0:49   ` Benson Leung
  2018-03-08  7:22   ` [Intel-gfx] " Jani Nikula
  1 sibling, 0 replies; 36+ messages in thread
From: Benson Leung @ 2018-03-08  0:49 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: intel-gfx, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1286 bytes --]

Hi Matt,

On Wed, Mar 07, 2018 at 04:28:51PM -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.
> V4: style changes
> V5: typo
> 
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>

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

V5 passes link training on that same panel from before with 8th bit set in
DPCD 0x000e.
 
Thanks,
Benson

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-08  0:28 ` matthew.s.atwood
  2018-03-08  0:49   ` Benson Leung
@ 2018-03-08  7:22   ` Jani Nikula
  2018-03-09 23:49     ` Atwood, Matthew S
  1 sibling, 1 reply; 36+ messages in thread
From: Jani Nikula @ 2018-03-08  7:22 UTC (permalink / raw)
  To: matthew.s.atwood, intel-gfx, dri-devel

On Wed, 07 Mar 2018, 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
>
> 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..cdb04c9 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);

\n missing.

> +
> +	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);

\n missing.

> +
> +	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..1269ef8 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 DP_REV_10                          0x10
> +# define DP_REV_11                          0x11
> +# define DP_REV_12                          0x12
> +# define DP_REV_13                          0x13
> +# define DP_REV_14                          0x14

I am not sure what good these buy us, but if people think they're the
way to go, then so be it. Just bear in mind that per spec, "The DPCD
revision number does not necessarily match the DisplayPort version
number." so "DP_REV" doesn't actually mean *DP* revision.


BR,
Jani.

>  
>  #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    /* 1.3 */
>  
>  #define DP_ADAPTER_CAP			    0x00f   /* 1.2 */
>  # define DP_FORCE_LOAD_SENSE_CAP	    (1 << 0)

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-08  7:22   ` [Intel-gfx] " Jani Nikula
@ 2018-03-09 23:49     ` Atwood, Matthew S
  2018-03-12 19:39       ` Rodrigo Vivi
  0 siblings, 1 reply; 36+ messages in thread
From: Atwood, Matthew S @ 2018-03-09 23:49 UTC (permalink / raw)
  To: dri-devel, intel-gfx, jani.nikula

On Thu, 2018-03-08 at 09:22 +0200, Jani Nikula wrote:
> On Wed, 07 Mar 2018, 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
> > 
> > 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..cdb04c9 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);
> \n missing.
will do
> 
> > 
> > +
> > +	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);
> \n missing.
will do
> 
> > 
> > +
> > +	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..1269ef8 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 DP_REV_10                          0x10
> > +# define DP_REV_11                          0x11
> > +# define DP_REV_12                          0x12
> > +# define DP_REV_13                          0x13
> > +# define DP_REV_14                          0x14
> I am not sure what good these buy us, but if people think they're the
> way to go, then so be it. Just bear in mind that per spec, "The DPCD
> revision number does not necessarily match the DisplayPort version
> number." so "DP_REV" doesn't actually mean *DP* revision.
> 
> 
> BR,
> Jani.
you're right likely a better name is DPCD_REV_XX. I think we sill want
to base the main-link clock recovery on time on this value. Next
revision will include this naming convention. 
> 
> > 
> >  
> >  #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    /* 1.3 */
Rodrigo has shown me a DP 1.2 spec that had this change and conflicts
with my copy so I'll be changing to XXX 1.2

Matt
> >  
> >  #define DP_ADAPTER_CAP			    0x00f   /* 1.2
> > */
> >  # define DP_FORCE_LOAD_SENSE_CAP	    (1 << 0)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-09 23:49     ` Atwood, Matthew S
@ 2018-03-12 19:39       ` Rodrigo Vivi
  0 siblings, 0 replies; 36+ messages in thread
From: Rodrigo Vivi @ 2018-03-12 19:39 UTC (permalink / raw)
  To: Atwood, Matthew S; +Cc: intel-gfx, dri-devel

On Fri, Mar 09, 2018 at 11:49:44PM +0000, Atwood, Matthew S wrote:
> On Thu, 2018-03-08 at 09:22 +0200, Jani Nikula wrote:
> > On Wed, 07 Mar 2018, 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
> > > 
> > > 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..cdb04c9 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);
> > \n missing.
> will do
> > 
> > > 
> > > +
> > > +	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);
> > \n missing.
> will do
> > 
> > > 
> > > +
> > > +	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..1269ef8 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 DP_REV_10                          0x10
> > > +# define DP_REV_11                          0x11
> > > +# define DP_REV_12                          0x12
> > > +# define DP_REV_13                          0x13
> > > +# define DP_REV_14                          0x14
> > I am not sure what good these buy us, but if people think they're the
> > way to go, then so be it. Just bear in mind that per spec, "The DPCD
> > revision number does not necessarily match the DisplayPort version
> > number." so "DP_REV" doesn't actually mean *DP* revision.
> > 
> > 
> > BR,
> > Jani.
> you're right likely a better name is DPCD_REV_XX. I think we sill want
> to base the main-link clock recovery on time on this value. Next
> revision will include this naming convention. 

yep, I believe we need this anyways even without a necessarily match.
And DPCD_REV_ seems better indeed.

> > 
> > > 
> > >  
> > >  #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    /* 1.3 */
> Rodrigo has shown me a DP 1.2 spec that had this change and conflicts
> with my copy so I'll be changing to XXX 1.2

I thought you had convinced me otherwise that day. The other bit
on this range didn't exist on this so the mask wouldn't be needed,
but the max value there is anyways == 4 so it can apply anyways.

but up to you how you want to proceed here.

> 
> Matt
> > >  
> > >  #define DP_ADAPTER_CAP			    0x00f   /* 1.2
> > > */
> > >  # define DP_FORCE_LOAD_SENSE_CAP	    (1 << 0)
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-02 22:25 [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values matthew.s.atwood
                   ` (9 preceding siblings ...)
  2018-03-08  0:28 ` matthew.s.atwood
@ 2018-03-14 17:40 ` matthew.s.atwood
  2018-03-14 20:22   ` Rodrigo Vivi
  2018-03-16 11:47   ` kbuild test robot
  2018-03-14 20:20 ` matthew.s.atwood
  2018-03-15 21:08 ` matthew.s.atwood
  12 siblings, 2 replies; 36+ messages in thread
From: matthew.s.atwood @ 2018-03-14 17:40 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Matt Atwood

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))
 		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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-02 22:25 [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values matthew.s.atwood
                   ` (10 preceding siblings ...)
  2018-03-14 17:40 ` matthew.s.atwood
@ 2018-03-14 20:20 ` matthew.s.atwood
  2018-03-14 20:59   ` Rodrigo Vivi
  2018-03-15 21:08 ` matthew.s.atwood
  12 siblings, 1 reply; 36+ messages in thread
From: matthew.s.atwood @ 2018-03-14 20:20 UTC (permalink / raw)
  To: intel-gfx, dri-devel

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
V7: typo

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..793c0ff 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] >= DPCD_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)
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-14 17:40 ` matthew.s.atwood
@ 2018-03-14 20:22   ` Rodrigo Vivi
  2018-03-16 11:47   ` kbuild test robot
  1 sibling, 0 replies; 36+ messages in thread
From: Rodrigo Vivi @ 2018-03-14 20:22 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: intel-gfx, dri-devel

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-14 20:20 ` matthew.s.atwood
@ 2018-03-14 20:59   ` Rodrigo Vivi
  0 siblings, 0 replies; 36+ messages in thread
From: Rodrigo Vivi @ 2018-03-14 20:59 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: intel-gfx, dri-devel

On Wed, Mar 14, 2018 at 01:20:06PM -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
> V7: typo

https://patchwork.freedesktop.org/series/39473/
Checkpatch noticed few lines like this over 80 char.

> 
> 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..793c0ff 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;

	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);


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

> +
> +	if (rd_interval == 0 || (dpcd[DP_DPCD_REV] >= DPCD_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;

ditto

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

ditto

> +
> +	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

DP_DPCD_REV_ to match the reg name

>  
>  #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 */

maybe add "?" to be in sync with the reg offset?

>  
>  #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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-02 22:25 [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values matthew.s.atwood
                   ` (11 preceding siblings ...)
  2018-03-14 20:20 ` matthew.s.atwood
@ 2018-03-15 21:08 ` matthew.s.atwood
  2018-03-16  0:39   ` Rodrigo Vivi
                     ` (2 more replies)
  12 siblings, 3 replies; 36+ messages in thread
From: matthew.s.atwood @ 2018-03-15 21:08 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

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
V7: typo
V8: Style

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

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index adf79be..6bee2df 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -119,18 +119,32 @@ 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] >= DPCD_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..8c59ce4 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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-15 21:08 ` matthew.s.atwood
@ 2018-03-16  0:39   ` Rodrigo Vivi
  2018-03-16 23:10   ` kbuild test robot
  2018-03-17  3:34   ` Benson Leung
  2 siblings, 0 replies; 36+ messages in thread
From: Rodrigo Vivi @ 2018-03-16  0:39 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: intel-gfx, dri-devel

On Thu, Mar 15, 2018 at 02:08:51PM -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
> V7: typo
> V8: Style

thanks :)

> 
> 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 | 22 ++++++++++++++++++----
>  include/drm/drm_dp_helper.h     |  6 ++++++
>  2 files changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index adf79be..6bee2df 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -119,18 +119,32 @@ 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] >= DPCD_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..8c59ce4 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
> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-14 17:40 ` matthew.s.atwood
  2018-03-14 20:22   ` Rodrigo Vivi
@ 2018-03-16 11:47   ` kbuild test robot
  1 sibling, 0 replies; 36+ messages in thread
From: kbuild test robot @ 2018-03-16 11:47 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: intel-gfx, kbuild-all, dri-devel

[-- Attachment #1: Type: text/plain, Size: 6042 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32845 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-15 21:08 ` matthew.s.atwood
  2018-03-16  0:39   ` Rodrigo Vivi
@ 2018-03-16 23:10   ` kbuild test robot
  2018-03-17  3:34   ` Benson Leung
  2 siblings, 0 replies; 36+ messages in thread
From: kbuild test robot @ 2018-03-16 23:10 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: intel-gfx, kbuild-all, dri-devel

[-- Attachment #1: Type: text/plain, Size: 1736 bytes --]

Hi Matt,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[also build test ERROR 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-222756
config: x86_64-federa-25 (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=x86_64 

All errors (new ones prefixed by >>):

   In file included from drivers/gpu/drm/amd/amdgpu/../display/include/dpcd_defs.h:29:0,
                    from drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:42:
>> include/drm/drm_dp_helper.h:67:45: error: expected identifier before numeric constant
    # define DPCD_REV_10                        0x10
                                                ^
   drivers/gpu/drm/amd/amdgpu/../display/include/dpcd_defs.h:32:2: note: in expansion of macro 'DPCD_REV_10'
     DPCD_REV_10 = 0x10,
     ^~~~~~~~~~~

vim +67 include/drm/drm_dp_helper.h

    63	
    64	/* AUX CH addresses */
    65	/* DPCD */
    66	#define DP_DPCD_REV                         0x000
  > 67	# define DPCD_REV_10                        0x10
    68	# define DPCD_REV_11                        0x11
    69	# define DPCD_REV_12                        0x12
    70	# define DPCD_REV_13                        0x13
    71	# define DPCD_REV_14                        0x14
    72	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 48270 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
  2018-03-15 21:08 ` matthew.s.atwood
  2018-03-16  0:39   ` Rodrigo Vivi
  2018-03-16 23:10   ` kbuild test robot
@ 2018-03-17  3:34   ` Benson Leung
  2 siblings, 0 replies; 36+ messages in thread
From: Benson Leung @ 2018-03-17  3:34 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: intel-gfx, bleung, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1369 bytes --]

On Thu, Mar 15, 2018 at 02:08:51PM -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
> V7: typo
> V8: Style
> 
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>

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

This version still passes link training on the panel with 8th bit set in
DPCD 0x000e.

Thanks,
Benson

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2018-03-17  3:34 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-02 22:25 [PATCH] drm/dp: only accept valid DP_TRAINING_AUX_RD_INTERVAL values matthew.s.atwood
2018-03-02 23:22 ` Rodrigo Vivi
2018-03-04 10:03   ` Jani Nikula
2018-03-02 23:24 ` Manasi Navare
2018-03-03  7:34 ` Benson Leung
2018-03-04 10:02 ` Jani Nikula
2018-03-06 18:37 ` [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4 matthew.s.atwood
2018-03-06 19:21   ` Benson Leung
2018-03-06 23:24   ` Rodrigo Vivi
2018-03-07  0:24     ` Pandiyan, Dhinakaran
2018-03-07  0:41       ` [Intel-gfx] " Pandiyan, Dhinakaran
2018-03-07  1:36       ` Manasi Navare
2018-03-07  2:13         ` Pandiyan, Dhinakaran
2018-03-07 22:06           ` Rodrigo Vivi
2018-03-07 22:20             ` [Intel-gfx] " Manasi Navare
2018-03-06 20:08 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-03-07  0:43 ` ✓ Fi.CI.IGT: " Patchwork
2018-03-07 23:44 ` [PATCH] " matthew.s.atwood
2018-03-07 23:58   ` Ilia Mirkin
2018-03-08  0:18   ` Manasi Navare
2018-03-08  0:13 ` matthew.s.atwood
2018-03-08  0:36   ` Benson Leung
2018-03-08  0:28 ` matthew.s.atwood
2018-03-08  0:49   ` Benson Leung
2018-03-08  7:22   ` [Intel-gfx] " Jani Nikula
2018-03-09 23:49     ` Atwood, Matthew S
2018-03-12 19:39       ` Rodrigo Vivi
2018-03-14 17:40 ` matthew.s.atwood
2018-03-14 20:22   ` Rodrigo Vivi
2018-03-16 11:47   ` kbuild test robot
2018-03-14 20:20 ` matthew.s.atwood
2018-03-14 20:59   ` Rodrigo Vivi
2018-03-15 21:08 ` matthew.s.atwood
2018-03-16  0:39   ` Rodrigo Vivi
2018-03-16 23:10   ` kbuild test robot
2018-03-17  3:34   ` Benson Leung

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.