[V4] drm: Add LTTPR defines for DP 1.4a

Submitted by Rodrigo Siqueira on Sept. 4, 2019, 7:02 p.m.

Details

Message ID 20190904190116.klyt2rjg3svswmot@outlook.office365.com
State New
Headers show
Series "drm: Add LTTPR defines for DP 1.4a" ( rev: 3 ) in DRI devel

Not browsing as part of any series.

Commit Message

Rodrigo Siqueira Sept. 4, 2019, 7:02 p.m.
DP 1.4a specification defines Link Training Tunable PHY Repeater (LTTPR)
which is required to add support for systems with Thunderbolt or other
repeater devices.

Changes since V3:
- Replace spaces by tabs
Changes since V2:
- Drop the kernel-doc comment
- Reorder LTTPR according to register offset
Changes since V1:
- Adjusts registers names to be aligned with spec and the rest of the
  file
- Update spec comment from 1.4 to 1.4a

Cc: Abdoulaye Berthe <Abdoulaye.Berthe@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Leo Li <sunpeng.li@amd.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Signed-off-by: Abdoulaye Berthe <Abdoulaye.Berthe@amd.com>
Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
---
 include/drm/drm_dp_helper.h | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

Patch hide | download patch | download mbox

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 8364502f92cf..5abed96a1cb1 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -966,6 +966,31 @@ 
 #define DP_HDCP_2_2_REG_STREAM_TYPE_OFFSET	0x69494
 #define DP_HDCP_2_2_REG_DBG_OFFSET		0x69518
 
+/* Link Training (LT)-tunable Physical Repeaters - DP 1.4a */
+#define DP_LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV	0xf0000
+#define DP_MAX_LINK_RATE_PHY_REPEATER				0xf0001
+#define DP_PHY_REPEATER_CNT					0xf0002
+#define DP_PHY_REPEATER_MODE					0xf0003
+#define DP_MAX_LANE_COUNT_PHY_REPEATER				0xf0004
+#define DP_PHY_REPEATER_EXTENDED_WAIT_TIMEOUT			0xf0005
+#define DP_TRAINING_PATTERN_SET_PHY_REPEATER1			0xf0010
+#define DP_TRAINING_LANE0_SET_PHY_REPEATER1			0xf0011
+#define DP_TRAINING_LANE1_SET_PHY_REPEATER1			0xf0012
+#define DP_TRAINING_LANE2_SET_PHY_REPEATER1			0xf0013
+#define DP_TRAINING_LANE3_SET_PHY_REPEATER1			0xf0014
+#define DP_TRAINING_AUX_RD_INTERVAL_PHY_REPEATER1		0xf0020
+#define DP_TRANSMITTER_CAPABILITY_PHY_REPEATER1			0xf0021
+#define DP_LANE0_1_STATUS_PHY_REPEATER1				0xf0030
+#define DP_LANE2_3_STATUS_PHY_REPEATER1				0xf0031
+#define DP_LANE_ALIGN_STATUS_UPDATED_PHY_REPEATER1		0xf0032
+#define DP_ADJUST_REQUEST_LANE0_1_PHY_REPEATER1			0xf0033
+#define DP_ADJUST_REQUEST_LANE2_3_PHY_REPEATER1			0xf0034
+#define DP_SYMBOL_ERROR_COUNT_LANE0_PHY_REPEATER1		0xf0035
+#define DP_SYMBOL_ERROR_COUNT_LANE1_PHY_REPEATER1		0xf0037
+#define DP_SYMBOL_ERROR_COUNT_LANE2_PHY_REPEATER1		0xf0039
+#define DP_SYMBOL_ERROR_COUNT_LANE3_PHY_REPEATER1		0xf003b
+#define DP_FEC_STATUS_PHY_REPEATER1				0xf0290
+
 /* DP HDCP message start offsets in DPCD address space */
 #define DP_HDCP_2_2_AKE_INIT_OFFSET		DP_HDCP_2_2_REG_RTX_OFFSET
 #define DP_HDCP_2_2_AKE_SEND_CERT_OFFSET	DP_HDCP_2_2_REG_CERT_RX_OFFSET

Comments

On Wed, Sep 04, 2019 at 07:02:18PM +0000, Siqueira, Rodrigo wrote:
> DP 1.4a specification defines Link Training Tunable PHY Repeater (LTTPR)

A bunch of this stuff was already in DP 1.3 so the statement here
(and in the comment) is a bit misleading.

"LTTPR" is not a name that appears anywhere in the spec AFAICS, so
calling it that is a bit confusing.

> which is required to add support for systems with Thunderbolt or other
> repeater devices.

"required" seems a bit strong. IIRC by default these things should be in
transparent mode so the DPTX can remain blissfully unaware of their
presence.

> 
> Changes since V3:
> - Replace spaces by tabs
> Changes since V2:
> - Drop the kernel-doc comment
> - Reorder LTTPR according to register offset
> Changes since V1:
> - Adjusts registers names to be aligned with spec and the rest of the
>   file
> - Update spec comment from 1.4 to 1.4a
> 
> Cc: Abdoulaye Berthe <Abdoulaye.Berthe@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Leo Li <sunpeng.li@amd.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
> Signed-off-by: Abdoulaye Berthe <Abdoulaye.Berthe@amd.com>
> Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
> ---
>  include/drm/drm_dp_helper.h | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 8364502f92cf..5abed96a1cb1 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -966,6 +966,31 @@
>  #define DP_HDCP_2_2_REG_STREAM_TYPE_OFFSET	0x69494
>  #define DP_HDCP_2_2_REG_DBG_OFFSET		0x69518
>  
> +/* Link Training (LT)-tunable Physical Repeaters - DP 1.4a */

s/Physical/PHY/ to match the spec.

> +#define DP_LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV	0xf0000
> +#define DP_MAX_LINK_RATE_PHY_REPEATER				0xf0001
> +#define DP_PHY_REPEATER_CNT					0xf0002
> +#define DP_PHY_REPEATER_MODE					0xf0003
> +#define DP_MAX_LANE_COUNT_PHY_REPEATER				0xf0004
> +#define DP_PHY_REPEATER_EXTENDED_WAIT_TIMEOUT			0xf0005

The last two are DP 1.4a it seems.

0xf0004 was called Repeater_FEC_CAPABILITY in 1.4. But the spec doesn't
say anything about the DPCD revision so I have no idea how you're
supposed to decide which definition to use.

DP 1.4a seems to have added FEC_CAPABILITY_PHY_REPEATER1 at 0xf0294.
To replace the 1.4 Repeater_FEC_CAPABILITY I suppose.

> +#define DP_TRAINING_PATTERN_SET_PHY_REPEATER1			0xf0010
> +#define DP_TRAINING_LANE0_SET_PHY_REPEATER1			0xf0011
> +#define DP_TRAINING_LANE1_SET_PHY_REPEATER1			0xf0012
> +#define DP_TRAINING_LANE2_SET_PHY_REPEATER1			0xf0013
> +#define DP_TRAINING_LANE3_SET_PHY_REPEATER1			0xf0014
> +#define DP_TRAINING_AUX_RD_INTERVAL_PHY_REPEATER1		0xf0020
> +#define DP_TRANSMITTER_CAPABILITY_PHY_REPEATER1			0xf0021

Above two are DP 1.4a.

> +#define DP_LANE0_1_STATUS_PHY_REPEATER1				0xf0030
> +#define DP_LANE2_3_STATUS_PHY_REPEATER1				0xf0031
> +#define DP_LANE_ALIGN_STATUS_UPDATED_PHY_REPEATER1		0xf0032
> +#define DP_ADJUST_REQUEST_LANE0_1_PHY_REPEATER1			0xf0033
> +#define DP_ADJUST_REQUEST_LANE2_3_PHY_REPEATER1			0xf0034
> +#define DP_SYMBOL_ERROR_COUNT_LANE0_PHY_REPEATER1		0xf0035
> +#define DP_SYMBOL_ERROR_COUNT_LANE1_PHY_REPEATER1		0xf0037
> +#define DP_SYMBOL_ERROR_COUNT_LANE2_PHY_REPEATER1		0xf0039
> +#define DP_SYMBOL_ERROR_COUNT_LANE3_PHY_REPEATER1		0xf003b
> +#define DP_FEC_STATUS_PHY_REPEATER1				0xf0290

This seems to have appared in DP 1.4.

You skipped quite a few registers here. I guess those were deemed not
important?

> +
>  /* DP HDCP message start offsets in DPCD address space */
>  #define DP_HDCP_2_2_AKE_INIT_OFFSET		DP_HDCP_2_2_REG_RTX_OFFSET
>  #define DP_HDCP_2_2_AKE_SEND_CERT_OFFSET	DP_HDCP_2_2_REG_CERT_RX_OFFSET
> -- 
> 2.23.0
On 2019-09-05 1:29 p.m., Ville Syrjälä wrote:
> On Wed, Sep 04, 2019 at 07:02:18PM +0000, Siqueira, Rodrigo wrote:

>> DP 1.4a specification defines Link Training Tunable PHY Repeater (LTTPR)

> 

> A bunch of this stuff was already in DP 1.3 so the statement here

> (and in the comment) is a bit misleading.

> 

> "LTTPR" is not a name that appears anywhere in the spec AFAICS, so

> calling it that is a bit confusing.

> 

>> which is required to add support for systems with Thunderbolt or other

>> repeater devices.

> 

> "required" seems a bit strong. IIRC by default these things should be in

> transparent mode so the DPTX can remain blissfully unaware of their

> presence.

> 


That's the idea but in reality things usually don't work out like this.
I remember a couple years back debugging Thunderbolt and having it
modify DPCD register reads on me and messing up link training with
certain receivers.

Either way, we've found that in order to receive a reliable
implementation we need to make use of the LTTPR functionality.

>>

>> Changes since V3:

>> - Replace spaces by tabs

>> Changes since V2:

>> - Drop the kernel-doc comment

>> - Reorder LTTPR according to register offset

>> Changes since V1:

>> - Adjusts registers names to be aligned with spec and the rest of the

>>   file

>> - Update spec comment from 1.4 to 1.4a

>>

>> Cc: Abdoulaye Berthe <Abdoulaye.Berthe@amd.com>

>> Cc: Harry Wentland <harry.wentland@amd.com>

>> Cc: Leo Li <sunpeng.li@amd.com>

>> Cc: Jani Nikula <jani.nikula@linux.intel.com>

>> Cc: Manasi Navare <manasi.d.navare@intel.com>

>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

>> Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>

>> Signed-off-by: Abdoulaye Berthe <Abdoulaye.Berthe@amd.com>

>> Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>

>> ---

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

>>  1 file changed, 25 insertions(+)

>>

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

>> index 8364502f92cf..5abed96a1cb1 100644

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

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

>> @@ -966,6 +966,31 @@

>>  #define DP_HDCP_2_2_REG_STREAM_TYPE_OFFSET	0x69494

>>  #define DP_HDCP_2_2_REG_DBG_OFFSET		0x69518

>>  

>> +/* Link Training (LT)-tunable Physical Repeaters - DP 1.4a */

> 

> s/Physical/PHY/ to match the spec.

> 

>> +#define DP_LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV	0xf0000

>> +#define DP_MAX_LINK_RATE_PHY_REPEATER				0xf0001

>> +#define DP_PHY_REPEATER_CNT					0xf0002

>> +#define DP_PHY_REPEATER_MODE					0xf0003

>> +#define DP_MAX_LANE_COUNT_PHY_REPEATER				0xf0004

>> +#define DP_PHY_REPEATER_EXTENDED_WAIT_TIMEOUT			0xf0005

> 

> The last two are DP 1.4a it seems.

> 

> 0xf0004 was called Repeater_FEC_CAPABILITY in 1.4. But the spec doesn't

> say anything about the DPCD revision so I have no idea how you're

> supposed to decide which definition to use.

> 

> DP 1.4a seems to have added FEC_CAPABILITY_PHY_REPEATER1 at 0xf0294.

> To replace the 1.4 Repeater_FEC_CAPABILITY I suppose.

> 


This part confused me when I saw it in 1.4 and 1.4a. It's probably
safest to go with the 1.4a definition.

>> +#define DP_TRAINING_PATTERN_SET_PHY_REPEATER1			0xf0010

>> +#define DP_TRAINING_LANE0_SET_PHY_REPEATER1			0xf0011

>> +#define DP_TRAINING_LANE1_SET_PHY_REPEATER1			0xf0012

>> +#define DP_TRAINING_LANE2_SET_PHY_REPEATER1			0xf0013

>> +#define DP_TRAINING_LANE3_SET_PHY_REPEATER1			0xf0014

>> +#define DP_TRAINING_AUX_RD_INTERVAL_PHY_REPEATER1		0xf0020

>> +#define DP_TRANSMITTER_CAPABILITY_PHY_REPEATER1			0xf0021

> 

> Above two are DP 1.4a.

> 

>> +#define DP_LANE0_1_STATUS_PHY_REPEATER1				0xf0030

>> +#define DP_LANE2_3_STATUS_PHY_REPEATER1				0xf0031

>> +#define DP_LANE_ALIGN_STATUS_UPDATED_PHY_REPEATER1		0xf0032

>> +#define DP_ADJUST_REQUEST_LANE0_1_PHY_REPEATER1			0xf0033

>> +#define DP_ADJUST_REQUEST_LANE2_3_PHY_REPEATER1			0xf0034

>> +#define DP_SYMBOL_ERROR_COUNT_LANE0_PHY_REPEATER1		0xf0035

>> +#define DP_SYMBOL_ERROR_COUNT_LANE1_PHY_REPEATER1		0xf0037

>> +#define DP_SYMBOL_ERROR_COUNT_LANE2_PHY_REPEATER1		0xf0039

>> +#define DP_SYMBOL_ERROR_COUNT_LANE3_PHY_REPEATER1		0xf003b

>> +#define DP_FEC_STATUS_PHY_REPEATER1				0xf0290

> 

> This seems to have appared in DP 1.4.

> 

> You skipped quite a few registers here. I guess those were deemed not

> important?

> 


They won't be used by us at the moment.

Harry

>> +

>>  /* DP HDCP message start offsets in DPCD address space */

>>  #define DP_HDCP_2_2_AKE_INIT_OFFSET		DP_HDCP_2_2_REG_RTX_OFFSET

>>  #define DP_HDCP_2_2_AKE_SEND_CERT_OFFSET	DP_HDCP_2_2_REG_CERT_RX_OFFSET

>> -- 

>> 2.23.0

> 

> 

>

On Thu, Sep 05, 2019 at 07:51:59PM +0000, Siqueira, Rodrigo wrote:
> Hi Ville,
> 
> First of all, thank you very much for the review.
> 
> I added some comments below.
> 
> On 09/05, Wentland, Harry wrote:
> > On 2019-09-05 1:29 p.m., Ville Syrjälä wrote:
> > > On Wed, Sep 04, 2019 at 07:02:18PM +0000, Siqueira, Rodrigo wrote:
> > >> DP 1.4a specification defines Link Training Tunable PHY Repeater (LTTPR)
> > > 
> > > A bunch of this stuff was already in DP 1.3 so the statement here
> > > (and in the comment) is a bit misleading.
> > > 
> > > "LTTPR" is not a name that appears anywhere in the spec AFAICS, so
> > > calling it that is a bit confusing.
> > > 
> 
> We are using "VESA DisplayPort (DP) Standard 1.4a" as a reference. We
> double-checked the specification, and we found many occurrences of
> LTTPR.

Ah, yes I can see it in 1.4a. Earlier versions didn't use that acronym.
I withdraw my objection to this then.