[RFC 01/13] drm/dp: Read fast training capability from link

[RFC 01/13] drm/dp: Read fast training capability from link

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

While probing the DisplayPort link, query the fast training capability.
If supported, drivers can use the fast link training sequence instead of
the more involved full link training sequence.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 5 ++++-
 include/drm/drm_dp_helper.h     | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 80a02a412607..bf480a08d7f9 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -295,7 +295,7 @@ EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
  */
 int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 {
-	u8 values[3];
+	u8 values[4];
 	int err;
 
 	memset(link, 0, sizeof(*link));
@@ -311,6 +311,9 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	if (values[2] & DP_ENHANCED_FRAME_CAP)
 		link->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
 
+	if (values[3] & DP_NO_AUX_HANDSHAKE_LINK_TRAINING)
+		link->capabilities |= DP_LINK_CAP_FAST_TRAINING;
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_dp_link_probe);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 94898f6ea02a..61c0518704a3 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -742,6 +742,7 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
  * DisplayPort link
  */
 #define DP_LINK_CAP_ENHANCED_FRAMING (1 << 0)
+#define DP_LINK_CAP_FAST_TRAINING (1 << 1)
 
 struct drm_dp_link {
 	unsigned char revision;
-- 
2.4.5

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

[RFC 02/13] drm/dp: Set channel coding on link configuration

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Make use of ANSI 8B/10B channel coding if the DisplayPort sink supports
it.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 16 ++++++++++++++--
 include/drm/drm_dp_helper.h     |  2 ++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index bf480a08d7f9..c48addaf4301 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -295,7 +295,7 @@ EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
  */
 int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 {
-	u8 values[4];
+	u8 values[7];
 	int err;
 
 	memset(link, 0, sizeof(*link));
@@ -314,6 +314,9 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	if (values[3] & DP_NO_AUX_HANDSHAKE_LINK_TRAINING)
 		link->capabilities |= DP_LINK_CAP_FAST_TRAINING;
 
+	if (values[6] & DP_SET_ANSI_8B10B)
+		link->capabilities |= DP_LINK_CAP_ANSI_8B10B;
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_dp_link_probe);
@@ -396,7 +399,7 @@ EXPORT_SYMBOL(drm_dp_link_power_down);
  */
 int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
 {
-	u8 values[2];
+	u8 values[2], value = 0;
 	int err;
 
 	values[0] = drm_dp_link_rate_to_bw_code(link->rate);
@@ -409,6 +412,15 @@ int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	if (err < 0)
 		return err;
 
+	if (link->capabilities & DP_LINK_CAP_ANSI_8B10B)
+		value = DP_SET_ANSI_8B10B;
+	else
+		value = 0;
+
+	err = drm_dp_dpcd_writeb(aux, DP_MAIN_LINK_CHANNEL_CODING_SET, value);
+	if (err < 0)
+		return err;
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_dp_link_configure);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 61c0518704a3..83be142395ff 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -88,6 +88,7 @@
 # define DP_DETAILED_CAP_INFO_AVAILABLE	    (1 << 4) /* DPI */
 
 #define DP_MAIN_LINK_CHANNEL_CODING         0x006
+# define DP_CODING_ANSI_8B10B		    (1 << 0)
 
 #define DP_DOWN_STREAM_PORT_COUNT	    0x007
 # define DP_PORT_COUNT_MASK		    0x0f
@@ -743,6 +744,7 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
  */
 #define DP_LINK_CAP_ENHANCED_FRAMING (1 << 0)
 #define DP_LINK_CAP_FAST_TRAINING (1 << 1)
+#define DP_LINK_CAP_ANSI_8B10B (1 << 2)
 
 struct drm_dp_link {
 	unsigned char revision;
-- 
2.4.5

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

[RFC 03/13] drm/dp: Read TPS3 capability from sink

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

The TPS3 capability can be exposed by DP 1.2 and later sinks if they
support the alternative training pattern for channel equalization.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 4 ++++
 include/drm/drm_dp_helper.h     | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index c48addaf4301..45701c650a5d 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -311,6 +311,10 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	if (values[2] & DP_ENHANCED_FRAME_CAP)
 		link->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
 
+	if (link->revision >= 0x12)
+		if (values[2] & DP_TPS3_SUPPORTED)
+			link->capabilities |= DP_LINK_CAP_TPS3;
+
 	if (values[3] & DP_NO_AUX_HANDSHAKE_LINK_TRAINING)
 		link->capabilities |= DP_LINK_CAP_FAST_TRAINING;
 
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 83be142395ff..9e70ea8b907d 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -745,6 +745,7 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
 #define DP_LINK_CAP_ENHANCED_FRAMING (1 << 0)
 #define DP_LINK_CAP_FAST_TRAINING (1 << 1)
 #define DP_LINK_CAP_ANSI_8B10B (1 << 2)
+#define DP_LINK_CAP_TPS3 (1 << 3)
 
 struct drm_dp_link {
 	unsigned char revision;
-- 
2.4.5

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

[RFC 04/13] drm/dp: Enhanced framing capability is DP 1.1+

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

The enhanced framing capability was added in DisplayPort 1.1, so any
code dealing with it needs to be protected accordingly.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 45701c650a5d..dcfd6898aebe 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -308,8 +308,9 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	link->rate = drm_dp_bw_code_to_link_rate(values[1]);
 	link->num_lanes = values[2] & DP_MAX_LANE_COUNT_MASK;
 
-	if (values[2] & DP_ENHANCED_FRAME_CAP)
-		link->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
+	if (link->revision >= 0x11)
+		if (values[2] & DP_ENHANCED_FRAME_CAP)
+			link->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
 
 	if (link->revision >= 0x12)
 		if (values[2] & DP_TPS3_SUPPORTED)
-- 
2.4.5

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

Re: [RFC 04/13] drm/dp: Enhanced framing capability is DP 1.1+

[Jani Nikula]

On Wed, 12 Aug 2015, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> The enhanced framing capability was added in DisplayPort 1.1, so any
> code dealing with it needs to be protected accordingly.

I guess we should talk about DPCD 1.1, since "Note: The DPCD revision
number does not necessarily match the DisplayPort version number."

BR,
Jani.


>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 45701c650a5d..dcfd6898aebe 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -308,8 +308,9 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
>  	link->rate = drm_dp_bw_code_to_link_rate(values[1]);
>  	link->num_lanes = values[2] & DP_MAX_LANE_COUNT_MASK;
>  
> -	if (values[2] & DP_ENHANCED_FRAME_CAP)
> -		link->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
> +	if (link->revision >= 0x11)
> +		if (values[2] & DP_ENHANCED_FRAME_CAP)
> +			link->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
>  
>  	if (link->revision >= 0x12)
>  		if (values[2] & DP_TPS3_SUPPORTED)
> -- 
> 2.4.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [RFC 04/13] drm/dp: Enhanced framing capability is DP 1.1+

[Jani Nikula]

On Wed, 12 Aug 2015, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> The enhanced framing capability was added in DisplayPort 1.1, so any
> code dealing with it needs to be protected accordingly.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 45701c650a5d..dcfd6898aebe 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -308,8 +308,9 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
>  	link->rate = drm_dp_bw_code_to_link_rate(values[1]);
>  	link->num_lanes = values[2] & DP_MAX_LANE_COUNT_MASK;
>  
> -	if (values[2] & DP_ENHANCED_FRAME_CAP)
> -		link->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
> +	if (link->revision >= 0x11)
> +		if (values[2] & DP_ENHANCED_FRAME_CAP)
> +			link->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;

Oh, and perhaps the conditions should be encoded into helpers (of
helpers!) in drm_dp_helper.h. There's already drm_dp_enhanced_frame_cap
that checks DPCD REV.

BR,
Jani.

>  
>  	if (link->revision >= 0x12)
>  		if (values[2] & DP_TPS3_SUPPORTED)
> -- 
> 2.4.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[RFC 05/13] drm/dp: Fast link training is DP 1.1+

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

The capability to train the link without the AUX CH handshake was
introduced in DisplayPort 1.1, so it must be guarded accordingly.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index dcfd6898aebe..1fe181525604 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -316,8 +316,9 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 		if (values[2] & DP_TPS3_SUPPORTED)
 			link->capabilities |= DP_LINK_CAP_TPS3;
 
-	if (values[3] & DP_NO_AUX_HANDSHAKE_LINK_TRAINING)
-		link->capabilities |= DP_LINK_CAP_FAST_TRAINING;
+	if (link->revision >= 0x11)
+		if (values[3] & DP_NO_AUX_HANDSHAKE_LINK_TRAINING)
+			link->capabilities |= DP_LINK_CAP_FAST_TRAINING;
 
 	if (values[6] & DP_SET_ANSI_8B10B)
 		link->capabilities |= DP_LINK_CAP_ANSI_8B10B;
-- 
2.4.5

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

[RFC 06/13] drm/dp: Read eDP version from DPCD

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

If the sink support eDP, read the eDP revision from it's DPCD.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 30 +++++++++++++++++++++++++++++-
 include/drm/drm_dp_helper.h     |  1 +
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 1fe181525604..c711b690508b 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -295,7 +295,7 @@ EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
  */
 int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 {
-	u8 values[7];
+	u8 values[16], value;
 	int err;
 
 	memset(link, 0, sizeof(*link));
@@ -323,6 +323,34 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	if (values[6] & DP_SET_ANSI_8B10B)
 		link->capabilities |= DP_LINK_CAP_ANSI_8B10B;
 
+	if (values[13] & DP_ALTERNATE_SCRAMBLER_RESET_CAP) {
+		err = drm_dp_dpcd_readb(aux, DP_EDP_DPCD_REV, &value);
+		if (err < 0)
+			return err;
+
+		switch (value) {
+		case DP_EDP_11:
+			link->edp = 0x11;
+			break;
+
+		case DP_EDP_12:
+			link->edp = 0x12;
+			break;
+
+		case DP_EDP_13:
+			link->edp = 0x13;
+			break;
+
+		case DP_EDP_14:
+			link->edp = 0x14;
+			break;
+
+		default:
+			DRM_ERROR("unsupported eDP version: %02x\n", value);
+			break;
+		}
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_dp_link_probe);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 9e70ea8b907d..f9e7f484a225 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -749,6 +749,7 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
 
 struct drm_dp_link {
 	unsigned char revision;
+	unsigned char edp;
 	unsigned int rate;
 	unsigned int num_lanes;
 	unsigned long capabilities;
-- 
2.4.5

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

Re: [RFC 06/13] drm/dp: Read eDP version from DPCD

[Jani Nikula]

On Wed, 12 Aug 2015, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> If the sink support eDP, read the eDP revision from it's DPCD.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 30 +++++++++++++++++++++++++++++-
>  include/drm/drm_dp_helper.h     |  1 +
>  2 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 1fe181525604..c711b690508b 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -295,7 +295,7 @@ EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
>   */
>  int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
>  {
> -	u8 values[7];
> +	u8 values[16], value;
>  	int err;
>  
>  	memset(link, 0, sizeof(*link));
> @@ -323,6 +323,34 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
>  	if (values[6] & DP_SET_ANSI_8B10B)
>  		link->capabilities |= DP_LINK_CAP_ANSI_8B10B;
>  
> +	if (values[13] & DP_ALTERNATE_SCRAMBLER_RESET_CAP) {

That's not indicative of eDP, that's indicative of edp && alternate
scrambler support. DPCD_DISPLAY_CONTROL_CAPABLE bit is a better match:

"To allow Source devices to easily identify panels that use DPCD
Addresses 00700h through 007FFh, the DPCD_DISPLAY_CONTROL_CAPABLE bit in
the eDP_CONFIGURATION_CAP register (DPCD Address 0000Dh, bit 3) has been
assigned (eDP v1.2 (and higher)) to indicate this capability, as
described in Table 3-3."

Also, I'd really appreciate using the macros for DPCD offsets. I do not
remember these offsets by heart, and we've defined the macros according
to the spec so it would be faster to look things up there.

> +		err = drm_dp_dpcd_readb(aux, DP_EDP_DPCD_REV, &value);
> +		if (err < 0)
> +			return err;
> +
> +		switch (value) {
> +		case DP_EDP_11:
> +			link->edp = 0x11;

I'm really not sure if this is a good idea, since DP_EDP_11 ==
0. Essentially you're promoting to use the magic values for the version
checking in code. Sure, we've done that with the DPCD version, but now
link->revision is directly from DPCD, and this would not be.

BR,
Jani.


> +			break;
> +
> +		case DP_EDP_12:
> +			link->edp = 0x12;
> +			break;
> +
> +		case DP_EDP_13:
> +			link->edp = 0x13;
> +			break;
> +
> +		case DP_EDP_14:
> +			link->edp = 0x14;
> +			break;
> +
> +		default:
> +			DRM_ERROR("unsupported eDP version: %02x\n", value);
> +			break;
> +		}
> +	}
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_dp_link_probe);
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 9e70ea8b907d..f9e7f484a225 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -749,6 +749,7 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
>  
>  struct drm_dp_link {
>  	unsigned char revision;
> +	unsigned char edp;
>  	unsigned int rate;
>  	unsigned int num_lanes;
>  	unsigned long capabilities;
> -- 
> 2.4.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[RFC 07/13] drm/dp: Read AUX read interval from DPCD

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Store the AUX read interval from DPCD, so that it can be used to wait
for the durations given in the specification during link training.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 3 +++
 include/drm/drm_dp_helper.h     | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index c711b690508b..9d6ef0576abc 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -351,6 +351,9 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 		}
 	}
 
+	/* DP_TRAINING_AUX_RD_INTERVAL is in units of 4 milliseconds */
+	link->aux_rd_interval = values[14] * 4000;
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_dp_link_probe);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index f9e7f484a225..1fda4643af1f 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -753,6 +753,7 @@ struct drm_dp_link {
 	unsigned int rate;
 	unsigned int num_lanes;
 	unsigned long capabilities;
+	unsigned long aux_rd_interval;
 };
 
 int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link);
-- 
2.4.5

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

[RFC 08/13] drm/dp: Add helper to get post-cursor adjustments

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

If the transmitter supports pre-emphasis post cursor2 the sink will
request adjustments in a similar way to how it requests adjustments to
the voltage swing and pre-emphasis settings.

Add a helper to extract these adjustments on a per-lane basis from the
DPCD link status.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 10 ++++++++++
 include/drm/drm_dp_helper.h     | 12 ++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 9d6ef0576abc..d503cc3f1772 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -115,6 +115,16 @@ 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);
 
+u8 drm_dp_get_adjust_request_post_cursor(const u8 link_status[DP_LINK_STATUS_SIZE],
+					 unsigned int lane)
+{
+	unsigned int offset = DP_ADJUST_REQUEST_POST_CURSOR2;
+	u8 value = dp_link_status(link_status, offset);
+
+	return (value >> (lane << 1)) & 0x3;
+}
+EXPORT_SYMBOL(drm_dp_get_adjust_request_post_cursor);
+
 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);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 1fda4643af1f..2165b9ef8504 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -400,6 +400,16 @@
 # define DP_ADJUST_PRE_EMPHASIS_LANE1_MASK   0xc0
 # define DP_ADJUST_PRE_EMPHASIS_LANE1_SHIFT  6
 
+#define DP_ADJUST_REQUEST_POST_CURSOR2	    0x20c
+# define DP_ADJUST_POST_CURSOR2_LANE0_MASK  0x03
+# define DP_ADJUST_POST_CURSOR2_LANE0_SHIFT 0
+# define DP_ADJUST_POST_CURSOR2_LANE1_MASK  0x0c
+# define DP_ADJUST_POST_CURSOR2_LANE1_SHIFT 2
+# define DP_ADJUST_POST_CURSOR2_LANE2_MASK  0x30
+# define DP_ADJUST_POST_CURSOR2_LANE2_SHIFT 4
+# define DP_ADJUST_POST_CURSOR2_LANE3_MASK  0xc0
+# define DP_ADJUST_POST_CURSOR2_LANE3_SHIFT 6
+
 #define DP_TEST_REQUEST			    0x218
 # define DP_TEST_LINK_TRAINING		    (1 << 0)
 # define DP_TEST_LINK_VIDEO_PATTERN	    (1 << 1)
@@ -578,6 +588,8 @@ u8 drm_dp_get_adjust_request_voltage(const u8 link_status[DP_LINK_STATUS_SIZE],
 				     int lane);
 u8 drm_dp_get_adjust_request_pre_emphasis(const u8 link_status[DP_LINK_STATUS_SIZE],
 					  int lane);
+u8 drm_dp_get_adjust_request_post_cursor(const u8 link_status[DP_LINK_STATUS_SIZE],
+					 unsigned int lane);
 
 #define DP_RECEIVER_CAP_SIZE		0xf
 #define EDP_PSR_RECEIVER_CAP_SIZE	2
-- 
2.4.5

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

[RFC 09/13] drm/dp: Enable alternate scrambler when supported

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

If the sink is eDP and supports the alternate scrambler reset, enable
it.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 9 +++++++++
 include/drm/drm_dp_helper.h     | 1 +
 2 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index d503cc3f1772..543349e14481 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -334,6 +334,8 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 		link->capabilities |= DP_LINK_CAP_ANSI_8B10B;
 
 	if (values[13] & DP_ALTERNATE_SCRAMBLER_RESET_CAP) {
+		link->capabilities |= DP_LINK_CAP_ALTERNATE_SCRAMBLER_RESET;
+
 		err = drm_dp_dpcd_readb(aux, DP_EDP_DPCD_REV, &value);
 		if (err < 0)
 			return err;
@@ -468,6 +470,13 @@ int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	if (err < 0)
 		return err;
 
+	if (link->capabilities & DP_LINK_CAP_ALTERNATE_SCRAMBLER_RESET) {
+		err = drm_dp_dpcd_writeb(aux, DP_EDP_CONFIGURATION_SET,
+					 DP_ALTERNATE_SCRAMBLER_RESET_ENABLE);
+		if (err < 0)
+			return err;
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_dp_link_configure);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 2165b9ef8504..600347f8cdd4 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -758,6 +758,7 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
 #define DP_LINK_CAP_FAST_TRAINING (1 << 1)
 #define DP_LINK_CAP_ANSI_8B10B (1 << 2)
 #define DP_LINK_CAP_TPS3 (1 << 3)
+#define DP_LINK_CAP_ALTERNATE_SCRAMBLER_RESET (1 << 4)
 
 struct drm_dp_link {
 	unsigned char revision;
-- 
2.4.5

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

[RFC 10/13] drm/dp: Add link training helper

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Add a helper that will perform link training as described in the
DisplayPort specification.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 476 +++++++++++++++++++++++++++++++++++++++-
 include/drm/drm_dp_helper.h     |  49 +++++
 2 files changed, 524 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 543349e14481..8968201ea93c 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -292,6 +292,19 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
 }
 EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
 
+static void drm_dp_link_reset(struct drm_dp_link *link)
+{
+	if (!link)
+		return;
+
+	link->revision = 0;
+	link->edp = 0;
+	link->rate = 0;
+	link->num_lanes = 0;
+	link->capabilities = 0;
+	link->aux_rd_interval = 0;
+}
+
 /**
  * drm_dp_link_probe() - probe a DisplayPort link for capabilities
  * @aux: DisplayPort AUX channel
@@ -308,7 +321,7 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	u8 values[16], value;
 	int err;
 
-	memset(link, 0, sizeof(*link));
+	drm_dp_link_reset(link);
 
 	err = drm_dp_dpcd_read(aux, DP_DPCD_REV, values, sizeof(values));
 	if (err < 0)
@@ -451,6 +464,14 @@ int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	u8 values[2], value = 0;
 	int err;
 
+	if (link->ops && link->ops->configure) {
+		err = link->ops->configure(link);
+		if (err < 0) {
+			DRM_ERROR("failed to configure DP link: %d\n", err);
+			return err;
+		}
+	}
+
 	values[0] = drm_dp_link_rate_to_bw_code(link->rate);
 	values[1] = link->num_lanes;
 
@@ -727,3 +748,456 @@ void drm_dp_aux_unregister(struct drm_dp_aux *aux)
 	i2c_del_adapter(&aux->ddc);
 }
 EXPORT_SYMBOL(drm_dp_aux_unregister);
+
+/**
+ * DOC: Link training
+ *
+ * These functions contain common logic and helpers to implement DisplayPort
+ * link training.
+ */
+
+/**
+ * drm_dp_link_train_init() - initialize DisplayPort link training state
+ * @train: DisplayPort link training state
+ */
+void drm_dp_link_train_init(struct drm_dp_link_train *train)
+{
+	struct drm_dp_link_train_set *request = &train->request;
+	struct drm_dp_link_train_set *adjust = &train->adjust;
+	unsigned int i;
+
+	for (i = 0; i < 4; i++) {
+		request->voltage_swing[i] = 0;
+		adjust->voltage_swing[i] = 0;
+
+		request->pre_emphasis[i] = 0;
+		adjust->pre_emphasis[i] = 0;
+
+		request->post_cursor[i] = 0;
+		adjust->post_cursor[i] = 0;
+	}
+
+	train->pattern = DP_TRAINING_PATTERN_DISABLE;
+	train->clock_recovered = false;
+	train->channel_equalized = false;
+}
+
+static bool drm_dp_link_train_valid(const struct drm_dp_link_train *train)
+{
+	return train->clock_recovered && train->channel_equalized;
+}
+
+static int drm_dp_link_apply_training(struct drm_dp_link *link)
+{
+	struct drm_dp_link_train_set *request = &link->train.request;
+	unsigned int lanes = link->num_lanes, *vs, *pe, *pc, i;
+	struct drm_dp_aux *aux = link->aux;
+	u8 values[4], pattern = 0;
+	int err;
+
+	err = link->ops->apply_training(link);
+	if (err < 0) {
+		DRM_ERROR("failed to apply link training: %d\n", err);
+		return err;
+	}
+
+	vs = request->voltage_swing;
+	pe = request->pre_emphasis;
+	pc = request->post_cursor;
+
+	/* write currently selected voltage-swing and pre-emphasis levels */
+	for (i = 0; i < lanes; i++)
+		values[i] = DP_TRAIN_VOLTAGE_SWING_LEVEL(vs[i]) |
+			    DP_TRAIN_PRE_EMPHASIS_LEVEL(pe[i]);
+
+	err = drm_dp_dpcd_write(aux, DP_TRAINING_LANE0_SET, values, lanes);
+	if (err < 0) {
+		DRM_ERROR("failed to set training parameters: %d\n", err);
+		return err;
+	}
+
+	/* write currently selected post-cursor level (if supported) */
+	if (link->revision >= 0x12 && link->rate == 540000) {
+		values[0] = values[1] = 0;
+
+		for (i = 0; i < lanes; i++)
+			values[i / 2] |= DP_LANE_POST_CURSOR(i, pc[i]);
+
+		err = drm_dp_dpcd_write(aux, DP_TRAINING_LANE0_1_SET2, values,
+					DIV_ROUND_UP(lanes, 2));
+		if (err < 0) {
+			DRM_ERROR("failed to set post-cursor: %d\n", err);
+			return err;
+		}
+	}
+
+	/* write link pattern */
+	if (link->train.pattern != DP_TRAINING_PATTERN_DISABLE)
+		pattern |= DP_LINK_SCRAMBLING_DISABLE;
+
+	pattern |= link->train.pattern;
+
+	err = drm_dp_dpcd_writeb(aux, DP_TRAINING_PATTERN_SET, pattern);
+	if (err < 0) {
+		DRM_ERROR("failed to set training pattern: %d\n", err);
+		return err;
+	}
+
+	return 0;
+}
+
+static void drm_dp_link_train_wait(struct drm_dp_link *link)
+{
+	unsigned long min = 0;
+
+	if (link->aux_rd_interval == 0) {
+		switch (link->train.pattern) {
+		case DP_TRAINING_PATTERN_1:
+			min = 100;
+			break;
+
+		case DP_TRAINING_PATTERN_2:
+		case DP_TRAINING_PATTERN_3:
+			min = 400;
+			break;
+
+		default:
+			break;
+		}
+	} else {
+		min = link->aux_rd_interval;
+	}
+
+	if (min > 0)
+		usleep_range(min, 2 * min);
+}
+
+static void drm_dp_link_get_adjustments(struct drm_dp_link *link,
+					u8 status[DP_LINK_STATUS_SIZE])
+{
+	struct drm_dp_link_train_set *adjust = &link->train.adjust;
+	unsigned int i;
+
+	for (i = 0; i < link->num_lanes; i++) {
+		adjust->voltage_swing[i] =
+			drm_dp_get_adjust_request_voltage(status, i) >>
+				DP_TRAIN_VOLTAGE_SWING_SHIFT;
+
+		adjust->pre_emphasis[i] =
+			drm_dp_get_adjust_request_pre_emphasis(status, i) >>
+				DP_TRAIN_PRE_EMPHASIS_SHIFT;
+
+		adjust->post_cursor[i] =
+			drm_dp_get_adjust_request_post_cursor(status, i);
+	}
+}
+
+static void drm_dp_link_train_adjust(struct drm_dp_link_train *train)
+{
+	struct drm_dp_link_train_set *request = &train->request;
+	struct drm_dp_link_train_set *adjust = &train->adjust;
+	unsigned int i;
+
+	for (i = 0; i < 4; i++)
+		if (request->voltage_swing[i] != adjust->voltage_swing[i])
+			request->voltage_swing[i] = adjust->voltage_swing[i];
+
+	for (i = 0; i < 4; i++)
+		if (request->pre_emphasis[i] != adjust->pre_emphasis[i])
+			request->pre_emphasis[i] = adjust->pre_emphasis[i];
+
+	for (i = 0; i < 4; i++)
+		if (request->post_cursor[i] != adjust->post_cursor[i])
+			request->post_cursor[i] = adjust->post_cursor[i];
+}
+
+static int drm_dp_link_recover_clock(struct drm_dp_link *link)
+{
+	u8 status[DP_LINK_STATUS_SIZE];
+	int err;
+
+	err = drm_dp_link_apply_training(link);
+	if (err < 0)
+		return err;
+
+	drm_dp_link_train_wait(link);
+
+	err = drm_dp_dpcd_read_link_status(link->aux, status);
+	if (err < 0) {
+		DRM_ERROR("failed to read link status: %d\n", err);
+		return err;
+	}
+
+	if (!drm_dp_clock_recovery_ok(status, link->num_lanes))
+		drm_dp_link_get_adjustments(link, status);
+	else
+		link->train.clock_recovered = true;
+
+	return 0;
+}
+
+static int drm_dp_link_clock_recovery(struct drm_dp_link *link)
+{
+	unsigned int repeat;
+	int err;
+
+	/* start clock recovery using training pattern 1 */
+	link->train.pattern = DP_TRAINING_PATTERN_1;
+
+	for (repeat = 1; repeat < 5; repeat++) {
+		err = drm_dp_link_recover_clock(link);
+		if (err < 0) {
+			DRM_ERROR("failed to recover clock: %d\n", err);
+			return err;
+		}
+
+		drm_dp_link_train_adjust(&link->train);
+
+		if (link->train.clock_recovered)
+			break;
+	}
+
+	return 0;
+}
+
+static int drm_dp_link_equalize_channel(struct drm_dp_link *link)
+{
+	struct drm_dp_aux *aux = link->aux;
+	u8 status[DP_LINK_STATUS_SIZE];
+	int err;
+
+	err = drm_dp_link_apply_training(link);
+	if (err < 0)
+		return err;
+
+	drm_dp_link_train_wait(link);
+
+	err = drm_dp_dpcd_read_link_status(aux, status);
+	if (err < 0) {
+		DRM_ERROR("failed to read link status: %d\n", err);
+		return err;
+	}
+
+	if (!drm_dp_clock_recovery_ok(status, link->num_lanes)) {
+		DRM_ERROR("clock recovery lost while equalizing channel\n");
+		link->train.clock_recovered = false;
+		return 0;
+	}
+
+	if (!drm_dp_channel_eq_ok(status, link->num_lanes))
+		drm_dp_link_get_adjustments(link, status);
+	else
+		link->train.channel_equalized = true;
+
+	return 0;
+}
+
+static int drm_dp_link_channel_equalization(struct drm_dp_link *link)
+{
+	unsigned int repeat;
+	int err;
+
+	/* start channel equalization using pattern 2 or 3 */
+	if (link->capabilities & DP_LINK_CAP_TPS3)
+		link->train.pattern = DP_TRAINING_PATTERN_3;
+	else
+		link->train.pattern = DP_TRAINING_PATTERN_2;
+
+	for (repeat = 1; repeat < 5; repeat++) {
+		err = drm_dp_link_equalize_channel(link);
+		if (err < 0) {
+			DRM_ERROR("failed to equalize channel: %d\n", err);
+			return err;
+		}
+
+		drm_dp_link_train_adjust(&link->train);
+
+		if (link->train.channel_equalized)
+			break;
+	}
+
+	return 0;
+}
+
+static int drm_dp_link_downgrade(struct drm_dp_link *link)
+{
+	switch (link->rate) {
+	case 162000:
+		return -EINVAL;
+
+	case 270000:
+		link->rate = 162000;
+		break;
+
+	case 540000:
+		link->rate = 270000;
+		return 0;
+	}
+
+	return 0;
+}
+
+static void drm_dp_link_train_disable(struct drm_dp_link *link)
+{
+	int err;
+
+	link->train.pattern = DP_TRAINING_PATTERN_DISABLE;
+
+	err = drm_dp_link_apply_training(link);
+	if (err < 0)
+		DRM_ERROR("failed to disable link training: %d\n", err);
+}
+
+static int drm_dp_link_train_full(struct drm_dp_link *link)
+{
+	int err;
+
+retry:
+	DRM_DEBUG_KMS("full-training link: %u lane%s at %u MHz\n",
+		      link->num_lanes, (link->num_lanes > 1) ? "s" : "",
+		      link->rate / 100);
+
+	err = drm_dp_link_configure(link->aux, link);
+	if (err < 0) {
+		DRM_ERROR("failed to configure DP link: %d\n", err);
+		return err;
+	}
+
+	err = drm_dp_link_clock_recovery(link);
+	if (err < 0) {
+		DRM_ERROR("clock recovery failed: %d\n", err);
+		goto out;
+	}
+
+	if (!link->train.clock_recovered) {
+		DRM_ERROR("clock recovery failed, downgrading link\n");
+
+		err = drm_dp_link_downgrade(link);
+		if (err < 0)
+			goto out;
+
+		goto retry;
+	}
+
+	DRM_DEBUG_KMS("clock recovery succeeded\n");
+
+	err = drm_dp_link_channel_equalization(link);
+	if (err < 0) {
+		DRM_ERROR("channel equalization failed: %d\n", err);
+		goto out;
+	}
+
+	if (!link->train.channel_equalized) {
+		DRM_ERROR("channel equalization failed, downgrading link\n");
+
+		err = drm_dp_link_downgrade(link);
+		if (err < 0)
+			goto out;
+
+		goto retry;
+	}
+
+	DRM_DEBUG_KMS("channel equalization succeeded\n");
+
+out:
+	drm_dp_link_train_disable(link);
+	return err;
+}
+
+static int drm_dp_link_train_fast(struct drm_dp_link *link)
+{
+	u8 status[DP_LINK_STATUS_SIZE];
+	int err;
+
+	DRM_DEBUG_KMS("fast-training link: %u lane%s at %u MHz\n",
+		      link->num_lanes, (link->num_lanes > 1) ? "s" : "",
+		      link->rate / 100);
+
+	err = drm_dp_link_configure(link->aux, link);
+	if (err < 0) {
+		DRM_ERROR("failed to configure DP link: %d\n", err);
+		return err;
+	}
+
+	/* transmit training pattern 1 for 500 microseconds */
+	link->train.pattern = DP_TRAINING_PATTERN_1;
+
+	err = drm_dp_link_apply_training(link);
+	if (err < 0)
+		goto out;
+
+	usleep_range(500, 1000);
+
+	/* transmit training pattern 2 or 3 for 500 microseconds */
+	if (link->capabilities & DP_LINK_CAP_TPS3)
+		link->train.pattern = DP_TRAINING_PATTERN_3;
+	else
+		link->train.pattern = DP_TRAINING_PATTERN_2;
+
+	err = drm_dp_link_apply_training(link);
+	if (err < 0)
+		goto out;
+
+	usleep_range(500, 1000);
+
+	err = drm_dp_dpcd_read_link_status(link->aux, status);
+	if (err < 0) {
+		DRM_ERROR("failed to read link status: %d\n", err);
+		goto out;
+	}
+
+	if (!drm_dp_clock_recovery_ok(status, link->num_lanes)) {
+		DRM_ERROR("clock recovery failed\n");
+		err = -EIO;
+	}
+
+	if (!drm_dp_channel_eq_ok(status, link->num_lanes)) {
+		DRM_ERROR("channel equalization failed\n");
+		err = -EIO;
+	}
+
+out:
+	drm_dp_link_train_disable(link);
+	return err;
+}
+
+/**
+ * drm_dp_link_train() - perform DisplayPort link training
+ * @link: a DP link object
+ *
+ * Uses the context stored in the DP link object to perform link training. It
+ * is expected that drivers will call drm_dp_link_probe() to obtain the link
+ * capabilities before performing link training.
+ *
+ * If the sink supports fast link training (no AUX CH handshake) and valid
+ * training settings are available, this function will try to perform fast
+ * link training and fall back to full link training on failure.
+ *
+ * Returns: 0 on success or a negative error code on failure.
+ */
+int drm_dp_link_train(struct drm_dp_link *link)
+{
+	int err;
+
+	if (link->capabilities & DP_LINK_CAP_FAST_TRAINING) {
+		if (drm_dp_link_train_valid(&link->train)) {
+			err = drm_dp_link_train_fast(link);
+			if (err < 0)
+				DRM_ERROR("fast link training failed: %d\n",
+					  err);
+			else
+				return 0;
+		} else {
+			DRM_DEBUG_KMS("training parameters not available\n");
+		}
+	} else {
+		DRM_DEBUG_KMS("fast link training not supported\n");
+	}
+
+	err = drm_dp_link_train_full(link);
+	if (err < 0)
+		DRM_ERROR("full link training failed: %d\n", err);
+
+	return err;
+}
+EXPORT_SYMBOL(drm_dp_link_train);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 600347f8cdd4..d041bb00d6a0 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -265,12 +265,14 @@
 # define DP_TRAIN_VOLTAGE_SWING_LEVEL_1 (1 << 0)
 # define DP_TRAIN_VOLTAGE_SWING_LEVEL_2 (2 << 0)
 # define DP_TRAIN_VOLTAGE_SWING_LEVEL_3 (3 << 0)
+# define DP_TRAIN_VOLTAGE_SWING_LEVEL(x) ((x) << 0)
 
 # define DP_TRAIN_PRE_EMPHASIS_MASK	    (3 << 3)
 # define DP_TRAIN_PRE_EMPH_LEVEL_0		(0 << 3)
 # define DP_TRAIN_PRE_EMPH_LEVEL_1		(1 << 3)
 # define DP_TRAIN_PRE_EMPH_LEVEL_2		(2 << 3)
 # define DP_TRAIN_PRE_EMPH_LEVEL_3		(3 << 3)
+# define DP_TRAIN_PRE_EMPHASIS_LEVEL(x)		((x) << 3)
 
 # define DP_TRAIN_PRE_EMPHASIS_SHIFT	    3
 # define DP_TRAIN_MAX_PRE_EMPHASIS_REACHED  (1 << 5)
@@ -308,6 +310,7 @@
 # define DP_LANE02_MAX_POST_CURSOR2_REACHED (1 << 2)
 # define DP_LANE13_POST_CURSOR2_SET_MASK    (3 << 4)
 # define DP_LANE13_MAX_POST_CURSOR2_REACHED (1 << 6)
+# define DP_LANE_POST_CURSOR(i, x)	    (((x) & 0x3) << (((i) & 1) << 2))
 
 #define DP_MSTM_CTRL			    0x111   /* 1.2 */
 # define DP_MST_EN			    (1 << 0)
@@ -751,6 +754,45 @@ static inline ssize_t drm_dp_dpcd_writeb(struct drm_dp_aux *aux,
 int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
 				 u8 status[DP_LINK_STATUS_SIZE]);
 
+/**
+ * struct drm_dp_link_train_set - link training settings
+ * @voltage_swing: per-lane voltage swing
+ * @pre_emphasis: per-lane pre-emphasis
+ * @post_cursor: per-lane post-cursor
+ */
+struct drm_dp_link_train_set {
+	unsigned int voltage_swing[4];
+	unsigned int pre_emphasis[4];
+	unsigned int post_cursor[4];
+};
+
+/**
+ * struct drm_dp_link_train - link training state information
+ * @request: currently requested settings
+ * @adjust: adjustments requested by sink
+ * @pattern: currently requested training pattern
+ * @clock_recovered: flag to track if clock recovery has completed
+ * @channel_equalized: flag to track if channel equalization has completed
+ */
+struct drm_dp_link_train {
+	struct drm_dp_link_train_set request;
+	struct drm_dp_link_train_set adjust;
+
+	unsigned int pattern;
+
+	bool clock_recovered;
+	bool channel_equalized;
+};
+
+void drm_dp_link_train_init(struct drm_dp_link_train *train);
+
+struct drm_dp_link;
+
+struct drm_dp_link_ops {
+	int (*apply_training)(struct drm_dp_link *link);
+	int (*configure)(struct drm_dp_link *link);
+};
+
 /*
  * DisplayPort link
  */
@@ -767,6 +809,11 @@ struct drm_dp_link {
 	unsigned int num_lanes;
 	unsigned long capabilities;
 	unsigned long aux_rd_interval;
+
+	const struct drm_dp_link_ops *ops;
+	struct drm_dp_aux *aux;
+
+	struct drm_dp_link_train train;
 };
 
 int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link);
@@ -774,6 +821,8 @@ int drm_dp_link_power_up(struct drm_dp_aux *aux, struct drm_dp_link *link);
 int drm_dp_link_power_down(struct drm_dp_aux *aux, struct drm_dp_link *link);
 int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link);
 
+int drm_dp_link_train(struct drm_dp_link *link);
+
 int drm_dp_aux_register(struct drm_dp_aux *aux);
 void drm_dp_aux_unregister(struct drm_dp_aux *aux);
 
-- 
2.4.5

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

[RFC 11/13] drm/dp: Add helper to dump DPCD

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

The new drm_dp_dpcd_dump() helper dumps the contents of a DPCD to a
seq_file and can be used to make the DPCD available via debugfs for
example.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 146 ++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_dp_helper.h     |   2 +
 2 files changed, 148 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 8968201ea93c..ea74884c9cb3 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -27,6 +27,7 @@
 #include <linux/errno.h>
 #include <linux/sched.h>
 #include <linux/i2c.h>
+#include <linux/seq_file.h>
 #include <drm/drm_dp_helper.h>
 #include <drm/drmP.h>
 
@@ -292,6 +293,151 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
 }
 EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
 
+/**
+ * drm_dp_dpcd_dump() - dump DPCD content
+ * @aux: DisplayPort AUX channel
+ * @s: destination for DPCD dump
+ *
+ * Reads registers from the DPCD via a DisplayPort AUX channel and dumps them
+ * to a seq_file.
+ */
+void drm_dp_dpcd_dump(struct drm_dp_aux *aux, struct seq_file *s)
+{
+#define DUMP_REG(aux, offset) ({					\
+		u8 value;						\
+		int err;						\
+		err = drm_dp_dpcd_readb(aux, offset, &value);		\
+		if (err < 0) {						\
+			dev_err((aux)->dev, "failed to read %s: %d\n",	\
+				#offset, err);				\
+			return;						\
+		}							\
+		seq_printf(s, "%-35s 0x%04x 0x%02x\n", #offset, offset,	\
+			   value);					\
+	})
+
+	DUMP_REG(aux, DP_DPCD_REV);
+	DUMP_REG(aux, DP_MAX_LINK_RATE);
+	DUMP_REG(aux, DP_MAX_LANE_COUNT);
+	DUMP_REG(aux, DP_MAX_DOWNSPREAD);
+	DUMP_REG(aux, DP_NORP);
+	DUMP_REG(aux, DP_DOWNSTREAMPORT_PRESENT);
+	DUMP_REG(aux, DP_MAIN_LINK_CHANNEL_CODING);
+	DUMP_REG(aux, DP_DOWN_STREAM_PORT_COUNT);
+	DUMP_REG(aux, DP_RECEIVE_PORT_0_CAP_0);
+	DUMP_REG(aux, DP_RECEIVE_PORT_0_BUFFER_SIZE);
+	DUMP_REG(aux, DP_RECEIVE_PORT_1_CAP_0);
+	DUMP_REG(aux, DP_RECEIVE_PORT_1_BUFFER_SIZE);
+	DUMP_REG(aux, DP_I2C_SPEED_CAP);
+	DUMP_REG(aux, DP_EDP_CONFIGURATION_CAP);
+	DUMP_REG(aux, DP_TRAINING_AUX_RD_INTERVAL);
+	DUMP_REG(aux, DP_ADAPTER_CAP);
+	DUMP_REG(aux, DP_SUPPORTED_LINK_RATES);
+	DUMP_REG(aux, DP_FAUX_CAP);
+	DUMP_REG(aux, DP_MSTM_CAP);
+	DUMP_REG(aux, DP_NUMBER_OF_AUDIO_ENDPOINTS);
+	DUMP_REG(aux, DP_AV_GRANULARITY);
+	DUMP_REG(aux, DP_AUD_DEC_LAT0);
+	DUMP_REG(aux, DP_AUD_DEC_LAT1);
+	DUMP_REG(aux, DP_AUD_PP_LAT0);
+	DUMP_REG(aux, DP_AUD_PP_LAT1);
+	DUMP_REG(aux, DP_VID_INTER_LAT);
+	DUMP_REG(aux, DP_VID_PROG_LAT);
+	DUMP_REG(aux, DP_REP_LAT);
+	DUMP_REG(aux, DP_AUD_DEL_INS0);
+	DUMP_REG(aux, DP_AUD_DEL_INS1);
+	DUMP_REG(aux, DP_AUD_DEL_INS2);
+	DUMP_REG(aux, DP_RECEIVER_ALPM_CAP);
+	DUMP_REG(aux, DP_AUD_DEL_INS0);
+	DUMP_REG(aux, DP_GUID);
+	DUMP_REG(aux, DP_PSR_SUPPORT);
+	DUMP_REG(aux, DP_PSR_CAPS);
+	DUMP_REG(aux, DP_DOWNSTREAM_PORT_0);
+	DUMP_REG(aux, DP_LINK_BW_SET);
+	DUMP_REG(aux, DP_LANE_COUNT_SET);
+	DUMP_REG(aux, DP_TRAINING_PATTERN_SET);
+	DUMP_REG(aux, DP_TRAINING_LANE0_SET);
+	DUMP_REG(aux, DP_TRAINING_LANE1_SET);
+	DUMP_REG(aux, DP_TRAINING_LANE2_SET);
+	DUMP_REG(aux, DP_TRAINING_LANE3_SET);
+	DUMP_REG(aux, DP_DOWNSPREAD_CTRL);
+	DUMP_REG(aux, DP_MAIN_LINK_CHANNEL_CODING_SET);
+	DUMP_REG(aux, DP_I2C_SPEED_CONTROL_STATUS);
+	DUMP_REG(aux, DP_EDP_CONFIGURATION_SET);
+	DUMP_REG(aux, DP_LINK_QUAL_LANE0_SET);
+	DUMP_REG(aux, DP_LINK_QUAL_LANE1_SET);
+	DUMP_REG(aux, DP_LINK_QUAL_LANE2_SET);
+	DUMP_REG(aux, DP_LINK_QUAL_LANE3_SET);
+	DUMP_REG(aux, DP_TRAINING_LANE0_1_SET2);
+	DUMP_REG(aux, DP_TRAINING_LANE2_3_SET2);
+	DUMP_REG(aux, DP_MSTM_CTRL);
+	DUMP_REG(aux, DP_AUDIO_DELAY0);
+	DUMP_REG(aux, DP_AUDIO_DELAY1);
+	DUMP_REG(aux, DP_AUDIO_DELAY2);
+	DUMP_REG(aux, DP_LINK_RATE_SET);
+	DUMP_REG(aux, DP_RECEIVER_ALPM_CONFIG);
+	DUMP_REG(aux, DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF);
+	DUMP_REG(aux, DP_UPSTREAM_DEVICE_DP_PWR_NEED);
+	DUMP_REG(aux, DP_AUX_FRAME_SYNC_VALUE);
+	DUMP_REG(aux, DP_PSR_EN_CFG);
+	DUMP_REG(aux, DP_ADAPTER_CTRL);
+	DUMP_REG(aux, DP_BRANCH_DEVICE_CTRL);
+	DUMP_REG(aux, DP_PAYLOAD_ALLOCATE_SET);
+	DUMP_REG(aux, DP_PAYLOAD_ALLOCATE_START_TIME_SLOT);
+	DUMP_REG(aux, DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT);
+	DUMP_REG(aux, DP_SINK_COUNT);
+	DUMP_REG(aux, DP_DEVICE_SERVICE_IRQ_VECTOR);
+	DUMP_REG(aux, DP_LANE0_1_STATUS);
+	DUMP_REG(aux, DP_LANE2_3_STATUS);
+	DUMP_REG(aux, DP_LANE_ALIGN_STATUS_UPDATED);
+	DUMP_REG(aux, DP_SINK_STATUS);
+	DUMP_REG(aux, DP_ADJUST_REQUEST_LANE0_1);
+	DUMP_REG(aux, DP_ADJUST_REQUEST_LANE2_3);
+	DUMP_REG(aux, DP_TEST_REQUEST);
+	DUMP_REG(aux, DP_TEST_LINK_RATE);
+	DUMP_REG(aux, DP_TEST_LANE_COUNT);
+	DUMP_REG(aux, DP_TEST_CRC_R_CR);
+	DUMP_REG(aux, DP_TEST_CRC_G_Y);
+	DUMP_REG(aux, DP_TEST_CRC_B_CB);
+	DUMP_REG(aux, DP_TEST_SINK_MISC);
+	DUMP_REG(aux, DP_TEST_RESPONSE);
+	DUMP_REG(aux, DP_TEST_EDID_CHECKSUM);
+	DUMP_REG(aux, DP_TEST_SINK);
+	DUMP_REG(aux, DP_PAYLOAD_TABLE_UPDATE_STATUS);
+	DUMP_REG(aux, DP_VC_PAYLOAD_ID_SLOT_1);
+	DUMP_REG(aux, DP_SOURCE_OUI);
+	DUMP_REG(aux, DP_SINK_OUI);
+	DUMP_REG(aux, DP_BRANCH_OUI);
+	DUMP_REG(aux, DP_SET_POWER);
+	DUMP_REG(aux, DP_EDP_DPCD_REV);
+	DUMP_REG(aux, DP_EDP_GENERAL_CAP_1);
+	DUMP_REG(aux, DP_EDP_BACKLIGHT_ADJUSTMENT_CAP);
+	DUMP_REG(aux, DP_EDP_GENERAL_CAP_2);
+	DUMP_REG(aux, DP_EDP_GENERAL_CAP_3);
+	DUMP_REG(aux, DP_EDP_DISPLAY_CONTROL_REGISTER);
+	DUMP_REG(aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER);
+	DUMP_REG(aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB);
+	DUMP_REG(aux, DP_EDP_BACKLIGHT_BRIGHTNESS_LSB);
+	DUMP_REG(aux, DP_EDP_PWMGEN_BIT_COUNT);
+	DUMP_REG(aux, DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN);
+	DUMP_REG(aux, DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX);
+	DUMP_REG(aux, DP_EDP_BACKLIGHT_CONTROL_STATUS);
+	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_SET);
+	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MSB);
+	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MID);
+	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MIN_LSB);
+	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MAX_MSB);
+	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MAX_MID);
+	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MAX_LSB);
+	DUMP_REG(aux, DP_EDP_DBC_MINIMUM_BRIGHTNESS_SET);
+	DUMP_REG(aux, DP_EDP_DBC_MAXIMUM_BRIGHTNESS_SET);
+	DUMP_REG(aux, DP_EDP_REGIONAL_BACKLIGHT_BASE);
+	DUMP_REG(aux, DP_EDP_REGIONAL_BACKLIGHT_0);
+
+#undef DUMP_REG
+}
+EXPORT_SYMBOL(drm_dp_dpcd_dump);
+
 static void drm_dp_link_reset(struct drm_dp_link *link)
 {
 	if (!link)
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index d041bb00d6a0..089d274f857d 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -754,6 +754,8 @@ static inline ssize_t drm_dp_dpcd_writeb(struct drm_dp_aux *aux,
 int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
 				 u8 status[DP_LINK_STATUS_SIZE]);
 
+void drm_dp_dpcd_dump(struct drm_dp_aux *aux, struct seq_file *s);
+
 /**
  * struct drm_dp_link_train_set - link training settings
  * @voltage_swing: per-lane voltage swing
-- 
2.4.5

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

Re: [RFC 11/13] drm/dp: Add helper to dump DPCD

[Jani Nikula]

On Wed, 12 Aug 2015, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> The new drm_dp_dpcd_dump() helper dumps the contents of a DPCD to a
> seq_file and can be used to make the DPCD available via debugfs for
> example.

See i915/i915_debugfs.c for one DPCD dump implementation.

Around the time that was added, there was also some discussion (and
patches [1]) to expose a read/write debugfs interface to DPCD, letting
userspace access arbitrary DPCD registers.

Just this week there was some discussion about revisiting that. It was
about accessing some proprietary panel features, but there's also the
ease of debugging without having to keep updating the kernel to dump
more.

I think it would be great to agree on a common debugfs interface to
access DPCD arbitrarily. Last time I checked, the blocker to that was
access to the aux channel from generic code; it's always driver
specific. SMOP. ;)

I could put some effort into this (maybe Rafael too?), as long as we
could agree on the interface. As I wrote in the referenced thread, I
wasn't thrilled about what was proposed.

BR,
Jani.


[1] http://mid.gmane.org/1428493301-20293-1-git-send-email-durgadoss.r@intel.com



>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 146 ++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_dp_helper.h     |   2 +
>  2 files changed, 148 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 8968201ea93c..ea74884c9cb3 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -27,6 +27,7 @@
>  #include <linux/errno.h>
>  #include <linux/sched.h>
>  #include <linux/i2c.h>
> +#include <linux/seq_file.h>
>  #include <drm/drm_dp_helper.h>
>  #include <drm/drmP.h>
>  
> @@ -292,6 +293,151 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
>  }
>  EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
>  
> +/**
> + * drm_dp_dpcd_dump() - dump DPCD content
> + * @aux: DisplayPort AUX channel
> + * @s: destination for DPCD dump
> + *
> + * Reads registers from the DPCD via a DisplayPort AUX channel and dumps them
> + * to a seq_file.
> + */
> +void drm_dp_dpcd_dump(struct drm_dp_aux *aux, struct seq_file *s)
> +{
> +#define DUMP_REG(aux, offset) ({					\
> +		u8 value;						\
> +		int err;						\
> +		err = drm_dp_dpcd_readb(aux, offset, &value);		\
> +		if (err < 0) {						\
> +			dev_err((aux)->dev, "failed to read %s: %d\n",	\
> +				#offset, err);				\
> +			return;						\
> +		}							\
> +		seq_printf(s, "%-35s 0x%04x 0x%02x\n", #offset, offset,	\
> +			   value);					\
> +	})
> +
> +	DUMP_REG(aux, DP_DPCD_REV);
> +	DUMP_REG(aux, DP_MAX_LINK_RATE);
> +	DUMP_REG(aux, DP_MAX_LANE_COUNT);
> +	DUMP_REG(aux, DP_MAX_DOWNSPREAD);
> +	DUMP_REG(aux, DP_NORP);
> +	DUMP_REG(aux, DP_DOWNSTREAMPORT_PRESENT);
> +	DUMP_REG(aux, DP_MAIN_LINK_CHANNEL_CODING);
> +	DUMP_REG(aux, DP_DOWN_STREAM_PORT_COUNT);
> +	DUMP_REG(aux, DP_RECEIVE_PORT_0_CAP_0);
> +	DUMP_REG(aux, DP_RECEIVE_PORT_0_BUFFER_SIZE);
> +	DUMP_REG(aux, DP_RECEIVE_PORT_1_CAP_0);
> +	DUMP_REG(aux, DP_RECEIVE_PORT_1_BUFFER_SIZE);
> +	DUMP_REG(aux, DP_I2C_SPEED_CAP);
> +	DUMP_REG(aux, DP_EDP_CONFIGURATION_CAP);
> +	DUMP_REG(aux, DP_TRAINING_AUX_RD_INTERVAL);
> +	DUMP_REG(aux, DP_ADAPTER_CAP);
> +	DUMP_REG(aux, DP_SUPPORTED_LINK_RATES);
> +	DUMP_REG(aux, DP_FAUX_CAP);
> +	DUMP_REG(aux, DP_MSTM_CAP);
> +	DUMP_REG(aux, DP_NUMBER_OF_AUDIO_ENDPOINTS);
> +	DUMP_REG(aux, DP_AV_GRANULARITY);
> +	DUMP_REG(aux, DP_AUD_DEC_LAT0);
> +	DUMP_REG(aux, DP_AUD_DEC_LAT1);
> +	DUMP_REG(aux, DP_AUD_PP_LAT0);
> +	DUMP_REG(aux, DP_AUD_PP_LAT1);
> +	DUMP_REG(aux, DP_VID_INTER_LAT);
> +	DUMP_REG(aux, DP_VID_PROG_LAT);
> +	DUMP_REG(aux, DP_REP_LAT);
> +	DUMP_REG(aux, DP_AUD_DEL_INS0);
> +	DUMP_REG(aux, DP_AUD_DEL_INS1);
> +	DUMP_REG(aux, DP_AUD_DEL_INS2);
> +	DUMP_REG(aux, DP_RECEIVER_ALPM_CAP);
> +	DUMP_REG(aux, DP_AUD_DEL_INS0);
> +	DUMP_REG(aux, DP_GUID);
> +	DUMP_REG(aux, DP_PSR_SUPPORT);
> +	DUMP_REG(aux, DP_PSR_CAPS);
> +	DUMP_REG(aux, DP_DOWNSTREAM_PORT_0);
> +	DUMP_REG(aux, DP_LINK_BW_SET);
> +	DUMP_REG(aux, DP_LANE_COUNT_SET);
> +	DUMP_REG(aux, DP_TRAINING_PATTERN_SET);
> +	DUMP_REG(aux, DP_TRAINING_LANE0_SET);
> +	DUMP_REG(aux, DP_TRAINING_LANE1_SET);
> +	DUMP_REG(aux, DP_TRAINING_LANE2_SET);
> +	DUMP_REG(aux, DP_TRAINING_LANE3_SET);
> +	DUMP_REG(aux, DP_DOWNSPREAD_CTRL);
> +	DUMP_REG(aux, DP_MAIN_LINK_CHANNEL_CODING_SET);
> +	DUMP_REG(aux, DP_I2C_SPEED_CONTROL_STATUS);
> +	DUMP_REG(aux, DP_EDP_CONFIGURATION_SET);
> +	DUMP_REG(aux, DP_LINK_QUAL_LANE0_SET);
> +	DUMP_REG(aux, DP_LINK_QUAL_LANE1_SET);
> +	DUMP_REG(aux, DP_LINK_QUAL_LANE2_SET);
> +	DUMP_REG(aux, DP_LINK_QUAL_LANE3_SET);
> +	DUMP_REG(aux, DP_TRAINING_LANE0_1_SET2);
> +	DUMP_REG(aux, DP_TRAINING_LANE2_3_SET2);
> +	DUMP_REG(aux, DP_MSTM_CTRL);
> +	DUMP_REG(aux, DP_AUDIO_DELAY0);
> +	DUMP_REG(aux, DP_AUDIO_DELAY1);
> +	DUMP_REG(aux, DP_AUDIO_DELAY2);
> +	DUMP_REG(aux, DP_LINK_RATE_SET);
> +	DUMP_REG(aux, DP_RECEIVER_ALPM_CONFIG);
> +	DUMP_REG(aux, DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF);
> +	DUMP_REG(aux, DP_UPSTREAM_DEVICE_DP_PWR_NEED);
> +	DUMP_REG(aux, DP_AUX_FRAME_SYNC_VALUE);
> +	DUMP_REG(aux, DP_PSR_EN_CFG);
> +	DUMP_REG(aux, DP_ADAPTER_CTRL);
> +	DUMP_REG(aux, DP_BRANCH_DEVICE_CTRL);
> +	DUMP_REG(aux, DP_PAYLOAD_ALLOCATE_SET);
> +	DUMP_REG(aux, DP_PAYLOAD_ALLOCATE_START_TIME_SLOT);
> +	DUMP_REG(aux, DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT);
> +	DUMP_REG(aux, DP_SINK_COUNT);
> +	DUMP_REG(aux, DP_DEVICE_SERVICE_IRQ_VECTOR);
> +	DUMP_REG(aux, DP_LANE0_1_STATUS);
> +	DUMP_REG(aux, DP_LANE2_3_STATUS);
> +	DUMP_REG(aux, DP_LANE_ALIGN_STATUS_UPDATED);
> +	DUMP_REG(aux, DP_SINK_STATUS);
> +	DUMP_REG(aux, DP_ADJUST_REQUEST_LANE0_1);
> +	DUMP_REG(aux, DP_ADJUST_REQUEST_LANE2_3);
> +	DUMP_REG(aux, DP_TEST_REQUEST);
> +	DUMP_REG(aux, DP_TEST_LINK_RATE);
> +	DUMP_REG(aux, DP_TEST_LANE_COUNT);
> +	DUMP_REG(aux, DP_TEST_CRC_R_CR);
> +	DUMP_REG(aux, DP_TEST_CRC_G_Y);
> +	DUMP_REG(aux, DP_TEST_CRC_B_CB);
> +	DUMP_REG(aux, DP_TEST_SINK_MISC);
> +	DUMP_REG(aux, DP_TEST_RESPONSE);
> +	DUMP_REG(aux, DP_TEST_EDID_CHECKSUM);
> +	DUMP_REG(aux, DP_TEST_SINK);
> +	DUMP_REG(aux, DP_PAYLOAD_TABLE_UPDATE_STATUS);
> +	DUMP_REG(aux, DP_VC_PAYLOAD_ID_SLOT_1);
> +	DUMP_REG(aux, DP_SOURCE_OUI);
> +	DUMP_REG(aux, DP_SINK_OUI);
> +	DUMP_REG(aux, DP_BRANCH_OUI);
> +	DUMP_REG(aux, DP_SET_POWER);
> +	DUMP_REG(aux, DP_EDP_DPCD_REV);
> +	DUMP_REG(aux, DP_EDP_GENERAL_CAP_1);
> +	DUMP_REG(aux, DP_EDP_BACKLIGHT_ADJUSTMENT_CAP);
> +	DUMP_REG(aux, DP_EDP_GENERAL_CAP_2);
> +	DUMP_REG(aux, DP_EDP_GENERAL_CAP_3);
> +	DUMP_REG(aux, DP_EDP_DISPLAY_CONTROL_REGISTER);
> +	DUMP_REG(aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER);
> +	DUMP_REG(aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB);
> +	DUMP_REG(aux, DP_EDP_BACKLIGHT_BRIGHTNESS_LSB);
> +	DUMP_REG(aux, DP_EDP_PWMGEN_BIT_COUNT);
> +	DUMP_REG(aux, DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN);
> +	DUMP_REG(aux, DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX);
> +	DUMP_REG(aux, DP_EDP_BACKLIGHT_CONTROL_STATUS);
> +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_SET);
> +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MSB);
> +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MID);
> +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MIN_LSB);
> +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MAX_MSB);
> +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MAX_MID);
> +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MAX_LSB);
> +	DUMP_REG(aux, DP_EDP_DBC_MINIMUM_BRIGHTNESS_SET);
> +	DUMP_REG(aux, DP_EDP_DBC_MAXIMUM_BRIGHTNESS_SET);
> +	DUMP_REG(aux, DP_EDP_REGIONAL_BACKLIGHT_BASE);
> +	DUMP_REG(aux, DP_EDP_REGIONAL_BACKLIGHT_0);
> +
> +#undef DUMP_REG
> +}
> +EXPORT_SYMBOL(drm_dp_dpcd_dump);
> +
>  static void drm_dp_link_reset(struct drm_dp_link *link)
>  {
>  	if (!link)
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index d041bb00d6a0..089d274f857d 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -754,6 +754,8 @@ static inline ssize_t drm_dp_dpcd_writeb(struct drm_dp_aux *aux,
>  int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
>  				 u8 status[DP_LINK_STATUS_SIZE]);
>  
> +void drm_dp_dpcd_dump(struct drm_dp_aux *aux, struct seq_file *s);
> +
>  /**
>   * struct drm_dp_link_train_set - link training settings
>   * @voltage_swing: per-lane voltage swing
> -- 
> 2.4.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [RFC 11/13] drm/dp: Add helper to dump DPCD

[Rafael Antognolli]

On Fri, Aug 14, 2015 at 02:56:55PM +0300, Jani Nikula wrote:
> On Wed, 12 Aug 2015, Thierry Reding <thierry.reding@gmail.com> wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > The new drm_dp_dpcd_dump() helper dumps the contents of a DPCD to a
> > seq_file and can be used to make the DPCD available via debugfs for
> > example.
> 
> See i915/i915_debugfs.c for one DPCD dump implementation.
> 
> Around the time that was added, there was also some discussion (and
> patches [1]) to expose a read/write debugfs interface to DPCD, letting
> userspace access arbitrary DPCD registers.
> 
> Just this week there was some discussion about revisiting that. It was
> about accessing some proprietary panel features, but there's also the
> ease of debugging without having to keep updating the kernel to dump
> more.
> 
> I think it would be great to agree on a common debugfs interface to
> access DPCD arbitrarily. Last time I checked, the blocker to that was
> access to the aux channel from generic code; it's always driver
> specific. SMOP. ;)

Do you mean it would require the generic code/interface to somehow route
this to the driver specific code? I am not sure yet how this works (if
there's something like it around), but I'll take a look.

> I could put some effort into this (maybe Rafael too?), as long as we
> could agree on the interface. As I wrote in the referenced thread, I
> wasn't thrilled about what was proposed.
> 

Yes, I'm willing to put effort into this, for sure. Any help pointing to
which direction to follow is greatly appreciated.

Thanks,
Rafael

> 
> 
> [1] http://mid.gmane.org/1428493301-20293-1-git-send-email-durgadoss.r@intel.com
> 
> 
> 
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/gpu/drm/drm_dp_helper.c | 146 ++++++++++++++++++++++++++++++++++++++++
> >  include/drm/drm_dp_helper.h     |   2 +
> >  2 files changed, 148 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> > index 8968201ea93c..ea74884c9cb3 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -27,6 +27,7 @@
> >  #include <linux/errno.h>
> >  #include <linux/sched.h>
> >  #include <linux/i2c.h>
> > +#include <linux/seq_file.h>
> >  #include <drm/drm_dp_helper.h>
> >  #include <drm/drmP.h>
> >  
> > @@ -292,6 +293,151 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
> >  }
> >  EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
> >  
> > +/**
> > + * drm_dp_dpcd_dump() - dump DPCD content
> > + * @aux: DisplayPort AUX channel
> > + * @s: destination for DPCD dump
> > + *
> > + * Reads registers from the DPCD via a DisplayPort AUX channel and dumps them
> > + * to a seq_file.
> > + */
> > +void drm_dp_dpcd_dump(struct drm_dp_aux *aux, struct seq_file *s)
> > +{
> > +#define DUMP_REG(aux, offset) ({					\
> > +		u8 value;						\
> > +		int err;						\
> > +		err = drm_dp_dpcd_readb(aux, offset, &value);		\
> > +		if (err < 0) {						\
> > +			dev_err((aux)->dev, "failed to read %s: %d\n",	\
> > +				#offset, err);				\
> > +			return;						\
> > +		}							\
> > +		seq_printf(s, "%-35s 0x%04x 0x%02x\n", #offset, offset,	\
> > +			   value);					\
> > +	})
> > +
> > +	DUMP_REG(aux, DP_DPCD_REV);
> > +	DUMP_REG(aux, DP_MAX_LINK_RATE);
> > +	DUMP_REG(aux, DP_MAX_LANE_COUNT);
> > +	DUMP_REG(aux, DP_MAX_DOWNSPREAD);
> > +	DUMP_REG(aux, DP_NORP);
> > +	DUMP_REG(aux, DP_DOWNSTREAMPORT_PRESENT);
> > +	DUMP_REG(aux, DP_MAIN_LINK_CHANNEL_CODING);
> > +	DUMP_REG(aux, DP_DOWN_STREAM_PORT_COUNT);
> > +	DUMP_REG(aux, DP_RECEIVE_PORT_0_CAP_0);
> > +	DUMP_REG(aux, DP_RECEIVE_PORT_0_BUFFER_SIZE);
> > +	DUMP_REG(aux, DP_RECEIVE_PORT_1_CAP_0);
> > +	DUMP_REG(aux, DP_RECEIVE_PORT_1_BUFFER_SIZE);
> > +	DUMP_REG(aux, DP_I2C_SPEED_CAP);
> > +	DUMP_REG(aux, DP_EDP_CONFIGURATION_CAP);
> > +	DUMP_REG(aux, DP_TRAINING_AUX_RD_INTERVAL);
> > +	DUMP_REG(aux, DP_ADAPTER_CAP);
> > +	DUMP_REG(aux, DP_SUPPORTED_LINK_RATES);
> > +	DUMP_REG(aux, DP_FAUX_CAP);
> > +	DUMP_REG(aux, DP_MSTM_CAP);
> > +	DUMP_REG(aux, DP_NUMBER_OF_AUDIO_ENDPOINTS);
> > +	DUMP_REG(aux, DP_AV_GRANULARITY);
> > +	DUMP_REG(aux, DP_AUD_DEC_LAT0);
> > +	DUMP_REG(aux, DP_AUD_DEC_LAT1);
> > +	DUMP_REG(aux, DP_AUD_PP_LAT0);
> > +	DUMP_REG(aux, DP_AUD_PP_LAT1);
> > +	DUMP_REG(aux, DP_VID_INTER_LAT);
> > +	DUMP_REG(aux, DP_VID_PROG_LAT);
> > +	DUMP_REG(aux, DP_REP_LAT);
> > +	DUMP_REG(aux, DP_AUD_DEL_INS0);
> > +	DUMP_REG(aux, DP_AUD_DEL_INS1);
> > +	DUMP_REG(aux, DP_AUD_DEL_INS2);
> > +	DUMP_REG(aux, DP_RECEIVER_ALPM_CAP);
> > +	DUMP_REG(aux, DP_AUD_DEL_INS0);
> > +	DUMP_REG(aux, DP_GUID);
> > +	DUMP_REG(aux, DP_PSR_SUPPORT);
> > +	DUMP_REG(aux, DP_PSR_CAPS);
> > +	DUMP_REG(aux, DP_DOWNSTREAM_PORT_0);
> > +	DUMP_REG(aux, DP_LINK_BW_SET);
> > +	DUMP_REG(aux, DP_LANE_COUNT_SET);
> > +	DUMP_REG(aux, DP_TRAINING_PATTERN_SET);
> > +	DUMP_REG(aux, DP_TRAINING_LANE0_SET);
> > +	DUMP_REG(aux, DP_TRAINING_LANE1_SET);
> > +	DUMP_REG(aux, DP_TRAINING_LANE2_SET);
> > +	DUMP_REG(aux, DP_TRAINING_LANE3_SET);
> > +	DUMP_REG(aux, DP_DOWNSPREAD_CTRL);
> > +	DUMP_REG(aux, DP_MAIN_LINK_CHANNEL_CODING_SET);
> > +	DUMP_REG(aux, DP_I2C_SPEED_CONTROL_STATUS);
> > +	DUMP_REG(aux, DP_EDP_CONFIGURATION_SET);
> > +	DUMP_REG(aux, DP_LINK_QUAL_LANE0_SET);
> > +	DUMP_REG(aux, DP_LINK_QUAL_LANE1_SET);
> > +	DUMP_REG(aux, DP_LINK_QUAL_LANE2_SET);
> > +	DUMP_REG(aux, DP_LINK_QUAL_LANE3_SET);
> > +	DUMP_REG(aux, DP_TRAINING_LANE0_1_SET2);
> > +	DUMP_REG(aux, DP_TRAINING_LANE2_3_SET2);
> > +	DUMP_REG(aux, DP_MSTM_CTRL);
> > +	DUMP_REG(aux, DP_AUDIO_DELAY0);
> > +	DUMP_REG(aux, DP_AUDIO_DELAY1);
> > +	DUMP_REG(aux, DP_AUDIO_DELAY2);
> > +	DUMP_REG(aux, DP_LINK_RATE_SET);
> > +	DUMP_REG(aux, DP_RECEIVER_ALPM_CONFIG);
> > +	DUMP_REG(aux, DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF);
> > +	DUMP_REG(aux, DP_UPSTREAM_DEVICE_DP_PWR_NEED);
> > +	DUMP_REG(aux, DP_AUX_FRAME_SYNC_VALUE);
> > +	DUMP_REG(aux, DP_PSR_EN_CFG);
> > +	DUMP_REG(aux, DP_ADAPTER_CTRL);
> > +	DUMP_REG(aux, DP_BRANCH_DEVICE_CTRL);
> > +	DUMP_REG(aux, DP_PAYLOAD_ALLOCATE_SET);
> > +	DUMP_REG(aux, DP_PAYLOAD_ALLOCATE_START_TIME_SLOT);
> > +	DUMP_REG(aux, DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT);
> > +	DUMP_REG(aux, DP_SINK_COUNT);
> > +	DUMP_REG(aux, DP_DEVICE_SERVICE_IRQ_VECTOR);
> > +	DUMP_REG(aux, DP_LANE0_1_STATUS);
> > +	DUMP_REG(aux, DP_LANE2_3_STATUS);
> > +	DUMP_REG(aux, DP_LANE_ALIGN_STATUS_UPDATED);
> > +	DUMP_REG(aux, DP_SINK_STATUS);
> > +	DUMP_REG(aux, DP_ADJUST_REQUEST_LANE0_1);
> > +	DUMP_REG(aux, DP_ADJUST_REQUEST_LANE2_3);
> > +	DUMP_REG(aux, DP_TEST_REQUEST);
> > +	DUMP_REG(aux, DP_TEST_LINK_RATE);
> > +	DUMP_REG(aux, DP_TEST_LANE_COUNT);
> > +	DUMP_REG(aux, DP_TEST_CRC_R_CR);
> > +	DUMP_REG(aux, DP_TEST_CRC_G_Y);
> > +	DUMP_REG(aux, DP_TEST_CRC_B_CB);
> > +	DUMP_REG(aux, DP_TEST_SINK_MISC);
> > +	DUMP_REG(aux, DP_TEST_RESPONSE);
> > +	DUMP_REG(aux, DP_TEST_EDID_CHECKSUM);
> > +	DUMP_REG(aux, DP_TEST_SINK);
> > +	DUMP_REG(aux, DP_PAYLOAD_TABLE_UPDATE_STATUS);
> > +	DUMP_REG(aux, DP_VC_PAYLOAD_ID_SLOT_1);
> > +	DUMP_REG(aux, DP_SOURCE_OUI);
> > +	DUMP_REG(aux, DP_SINK_OUI);
> > +	DUMP_REG(aux, DP_BRANCH_OUI);
> > +	DUMP_REG(aux, DP_SET_POWER);
> > +	DUMP_REG(aux, DP_EDP_DPCD_REV);
> > +	DUMP_REG(aux, DP_EDP_GENERAL_CAP_1);
> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_ADJUSTMENT_CAP);
> > +	DUMP_REG(aux, DP_EDP_GENERAL_CAP_2);
> > +	DUMP_REG(aux, DP_EDP_GENERAL_CAP_3);
> > +	DUMP_REG(aux, DP_EDP_DISPLAY_CONTROL_REGISTER);
> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER);
> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB);
> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_BRIGHTNESS_LSB);
> > +	DUMP_REG(aux, DP_EDP_PWMGEN_BIT_COUNT);
> > +	DUMP_REG(aux, DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN);
> > +	DUMP_REG(aux, DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX);
> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_CONTROL_STATUS);
> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_SET);
> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MSB);
> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MID);
> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MIN_LSB);
> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MAX_MSB);
> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MAX_MID);
> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MAX_LSB);
> > +	DUMP_REG(aux, DP_EDP_DBC_MINIMUM_BRIGHTNESS_SET);
> > +	DUMP_REG(aux, DP_EDP_DBC_MAXIMUM_BRIGHTNESS_SET);
> > +	DUMP_REG(aux, DP_EDP_REGIONAL_BACKLIGHT_BASE);
> > +	DUMP_REG(aux, DP_EDP_REGIONAL_BACKLIGHT_0);
> > +
> > +#undef DUMP_REG
> > +}
> > +EXPORT_SYMBOL(drm_dp_dpcd_dump);
> > +
> >  static void drm_dp_link_reset(struct drm_dp_link *link)
> >  {
> >  	if (!link)
> > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> > index d041bb00d6a0..089d274f857d 100644
> > --- a/include/drm/drm_dp_helper.h
> > +++ b/include/drm/drm_dp_helper.h
> > @@ -754,6 +754,8 @@ static inline ssize_t drm_dp_dpcd_writeb(struct drm_dp_aux *aux,
> >  int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
> >  				 u8 status[DP_LINK_STATUS_SIZE]);
> >  
> > +void drm_dp_dpcd_dump(struct drm_dp_aux *aux, struct seq_file *s);
> > +
> >  /**
> >   * struct drm_dp_link_train_set - link training settings
> >   * @voltage_swing: per-lane voltage swing
> > -- 
> > 2.4.5
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 11/13] drm/dp: Add helper to dump DPCD

[Jani Nikula]

On Fri, 14 Aug 2015, Rafael Antognolli <rafael.antognolli@intel.com> wrote:
> On Fri, Aug 14, 2015 at 02:56:55PM +0300, Jani Nikula wrote:
>> On Wed, 12 Aug 2015, Thierry Reding <thierry.reding@gmail.com> wrote:
>> > From: Thierry Reding <treding@nvidia.com>
>> >
>> > The new drm_dp_dpcd_dump() helper dumps the contents of a DPCD to a
>> > seq_file and can be used to make the DPCD available via debugfs for
>> > example.
>> 
>> See i915/i915_debugfs.c for one DPCD dump implementation.
>> 
>> Around the time that was added, there was also some discussion (and
>> patches [1]) to expose a read/write debugfs interface to DPCD, letting
>> userspace access arbitrary DPCD registers.
>> 
>> Just this week there was some discussion about revisiting that. It was
>> about accessing some proprietary panel features, but there's also the
>> ease of debugging without having to keep updating the kernel to dump
>> more.
>> 
>> I think it would be great to agree on a common debugfs interface to
>> access DPCD arbitrarily. Last time I checked, the blocker to that was
>> access to the aux channel from generic code; it's always driver
>> specific. SMOP. ;)
>
> Do you mean it would require the generic code/interface to somehow route
> this to the driver specific code? I am not sure yet how this works (if
> there's something like it around), but I'll take a look.

Drivers can choose to support DP AUX any way they like, and they don't
have to use the common helpers to do so. It's pretty much an
implementation detail. I think we could require the use of the common
helpers to support generic DPCD access from debugfs, but we'd still have
to come up with a way to get hold of struct drm_dp_aux (again, driver
internals) given a drm_connector in generic debugfs code.

Thierry, do you see any problems with having a connector function to get
hold of its drm_dp_aux?

>> I could put some effort into this (maybe Rafael too?), as long as we
>> could agree on the interface. As I wrote in the referenced thread, I
>> wasn't thrilled about what was proposed.
>> 
>
> Yes, I'm willing to put effort into this, for sure. Any help pointing to
> which direction to follow is greatly appreciated.

Great!

BR,
Jani.


>
> Thanks,
> Rafael
>
>> 
>> 
>> [1] http://mid.gmane.org/1428493301-20293-1-git-send-email-durgadoss.r@intel.com
>> 
>> 
>> 
>> >
>> > Signed-off-by: Thierry Reding <treding@nvidia.com>
>> > ---
>> >  drivers/gpu/drm/drm_dp_helper.c | 146 ++++++++++++++++++++++++++++++++++++++++
>> >  include/drm/drm_dp_helper.h     |   2 +
>> >  2 files changed, 148 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
>> > index 8968201ea93c..ea74884c9cb3 100644
>> > --- a/drivers/gpu/drm/drm_dp_helper.c
>> > +++ b/drivers/gpu/drm/drm_dp_helper.c
>> > @@ -27,6 +27,7 @@
>> >  #include <linux/errno.h>
>> >  #include <linux/sched.h>
>> >  #include <linux/i2c.h>
>> > +#include <linux/seq_file.h>
>> >  #include <drm/drm_dp_helper.h>
>> >  #include <drm/drmP.h>
>> >  
>> > @@ -292,6 +293,151 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
>> >  }
>> >  EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
>> >  
>> > +/**
>> > + * drm_dp_dpcd_dump() - dump DPCD content
>> > + * @aux: DisplayPort AUX channel
>> > + * @s: destination for DPCD dump
>> > + *
>> > + * Reads registers from the DPCD via a DisplayPort AUX channel and dumps them
>> > + * to a seq_file.
>> > + */
>> > +void drm_dp_dpcd_dump(struct drm_dp_aux *aux, struct seq_file *s)
>> > +{
>> > +#define DUMP_REG(aux, offset) ({					\
>> > +		u8 value;						\
>> > +		int err;						\
>> > +		err = drm_dp_dpcd_readb(aux, offset, &value);		\
>> > +		if (err < 0) {						\
>> > +			dev_err((aux)->dev, "failed to read %s: %d\n",	\
>> > +				#offset, err);				\
>> > +			return;						\
>> > +		}							\
>> > +		seq_printf(s, "%-35s 0x%04x 0x%02x\n", #offset, offset,	\
>> > +			   value);					\
>> > +	})
>> > +
>> > +	DUMP_REG(aux, DP_DPCD_REV);
>> > +	DUMP_REG(aux, DP_MAX_LINK_RATE);
>> > +	DUMP_REG(aux, DP_MAX_LANE_COUNT);
>> > +	DUMP_REG(aux, DP_MAX_DOWNSPREAD);
>> > +	DUMP_REG(aux, DP_NORP);
>> > +	DUMP_REG(aux, DP_DOWNSTREAMPORT_PRESENT);
>> > +	DUMP_REG(aux, DP_MAIN_LINK_CHANNEL_CODING);
>> > +	DUMP_REG(aux, DP_DOWN_STREAM_PORT_COUNT);
>> > +	DUMP_REG(aux, DP_RECEIVE_PORT_0_CAP_0);
>> > +	DUMP_REG(aux, DP_RECEIVE_PORT_0_BUFFER_SIZE);
>> > +	DUMP_REG(aux, DP_RECEIVE_PORT_1_CAP_0);
>> > +	DUMP_REG(aux, DP_RECEIVE_PORT_1_BUFFER_SIZE);
>> > +	DUMP_REG(aux, DP_I2C_SPEED_CAP);
>> > +	DUMP_REG(aux, DP_EDP_CONFIGURATION_CAP);
>> > +	DUMP_REG(aux, DP_TRAINING_AUX_RD_INTERVAL);
>> > +	DUMP_REG(aux, DP_ADAPTER_CAP);
>> > +	DUMP_REG(aux, DP_SUPPORTED_LINK_RATES);
>> > +	DUMP_REG(aux, DP_FAUX_CAP);
>> > +	DUMP_REG(aux, DP_MSTM_CAP);
>> > +	DUMP_REG(aux, DP_NUMBER_OF_AUDIO_ENDPOINTS);
>> > +	DUMP_REG(aux, DP_AV_GRANULARITY);
>> > +	DUMP_REG(aux, DP_AUD_DEC_LAT0);
>> > +	DUMP_REG(aux, DP_AUD_DEC_LAT1);
>> > +	DUMP_REG(aux, DP_AUD_PP_LAT0);
>> > +	DUMP_REG(aux, DP_AUD_PP_LAT1);
>> > +	DUMP_REG(aux, DP_VID_INTER_LAT);
>> > +	DUMP_REG(aux, DP_VID_PROG_LAT);
>> > +	DUMP_REG(aux, DP_REP_LAT);
>> > +	DUMP_REG(aux, DP_AUD_DEL_INS0);
>> > +	DUMP_REG(aux, DP_AUD_DEL_INS1);
>> > +	DUMP_REG(aux, DP_AUD_DEL_INS2);
>> > +	DUMP_REG(aux, DP_RECEIVER_ALPM_CAP);
>> > +	DUMP_REG(aux, DP_AUD_DEL_INS0);
>> > +	DUMP_REG(aux, DP_GUID);
>> > +	DUMP_REG(aux, DP_PSR_SUPPORT);
>> > +	DUMP_REG(aux, DP_PSR_CAPS);
>> > +	DUMP_REG(aux, DP_DOWNSTREAM_PORT_0);
>> > +	DUMP_REG(aux, DP_LINK_BW_SET);
>> > +	DUMP_REG(aux, DP_LANE_COUNT_SET);
>> > +	DUMP_REG(aux, DP_TRAINING_PATTERN_SET);
>> > +	DUMP_REG(aux, DP_TRAINING_LANE0_SET);
>> > +	DUMP_REG(aux, DP_TRAINING_LANE1_SET);
>> > +	DUMP_REG(aux, DP_TRAINING_LANE2_SET);
>> > +	DUMP_REG(aux, DP_TRAINING_LANE3_SET);
>> > +	DUMP_REG(aux, DP_DOWNSPREAD_CTRL);
>> > +	DUMP_REG(aux, DP_MAIN_LINK_CHANNEL_CODING_SET);
>> > +	DUMP_REG(aux, DP_I2C_SPEED_CONTROL_STATUS);
>> > +	DUMP_REG(aux, DP_EDP_CONFIGURATION_SET);
>> > +	DUMP_REG(aux, DP_LINK_QUAL_LANE0_SET);
>> > +	DUMP_REG(aux, DP_LINK_QUAL_LANE1_SET);
>> > +	DUMP_REG(aux, DP_LINK_QUAL_LANE2_SET);
>> > +	DUMP_REG(aux, DP_LINK_QUAL_LANE3_SET);
>> > +	DUMP_REG(aux, DP_TRAINING_LANE0_1_SET2);
>> > +	DUMP_REG(aux, DP_TRAINING_LANE2_3_SET2);
>> > +	DUMP_REG(aux, DP_MSTM_CTRL);
>> > +	DUMP_REG(aux, DP_AUDIO_DELAY0);
>> > +	DUMP_REG(aux, DP_AUDIO_DELAY1);
>> > +	DUMP_REG(aux, DP_AUDIO_DELAY2);
>> > +	DUMP_REG(aux, DP_LINK_RATE_SET);
>> > +	DUMP_REG(aux, DP_RECEIVER_ALPM_CONFIG);
>> > +	DUMP_REG(aux, DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF);
>> > +	DUMP_REG(aux, DP_UPSTREAM_DEVICE_DP_PWR_NEED);
>> > +	DUMP_REG(aux, DP_AUX_FRAME_SYNC_VALUE);
>> > +	DUMP_REG(aux, DP_PSR_EN_CFG);
>> > +	DUMP_REG(aux, DP_ADAPTER_CTRL);
>> > +	DUMP_REG(aux, DP_BRANCH_DEVICE_CTRL);
>> > +	DUMP_REG(aux, DP_PAYLOAD_ALLOCATE_SET);
>> > +	DUMP_REG(aux, DP_PAYLOAD_ALLOCATE_START_TIME_SLOT);
>> > +	DUMP_REG(aux, DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT);
>> > +	DUMP_REG(aux, DP_SINK_COUNT);
>> > +	DUMP_REG(aux, DP_DEVICE_SERVICE_IRQ_VECTOR);
>> > +	DUMP_REG(aux, DP_LANE0_1_STATUS);
>> > +	DUMP_REG(aux, DP_LANE2_3_STATUS);
>> > +	DUMP_REG(aux, DP_LANE_ALIGN_STATUS_UPDATED);
>> > +	DUMP_REG(aux, DP_SINK_STATUS);
>> > +	DUMP_REG(aux, DP_ADJUST_REQUEST_LANE0_1);
>> > +	DUMP_REG(aux, DP_ADJUST_REQUEST_LANE2_3);
>> > +	DUMP_REG(aux, DP_TEST_REQUEST);
>> > +	DUMP_REG(aux, DP_TEST_LINK_RATE);
>> > +	DUMP_REG(aux, DP_TEST_LANE_COUNT);
>> > +	DUMP_REG(aux, DP_TEST_CRC_R_CR);
>> > +	DUMP_REG(aux, DP_TEST_CRC_G_Y);
>> > +	DUMP_REG(aux, DP_TEST_CRC_B_CB);
>> > +	DUMP_REG(aux, DP_TEST_SINK_MISC);
>> > +	DUMP_REG(aux, DP_TEST_RESPONSE);
>> > +	DUMP_REG(aux, DP_TEST_EDID_CHECKSUM);
>> > +	DUMP_REG(aux, DP_TEST_SINK);
>> > +	DUMP_REG(aux, DP_PAYLOAD_TABLE_UPDATE_STATUS);
>> > +	DUMP_REG(aux, DP_VC_PAYLOAD_ID_SLOT_1);
>> > +	DUMP_REG(aux, DP_SOURCE_OUI);
>> > +	DUMP_REG(aux, DP_SINK_OUI);
>> > +	DUMP_REG(aux, DP_BRANCH_OUI);
>> > +	DUMP_REG(aux, DP_SET_POWER);
>> > +	DUMP_REG(aux, DP_EDP_DPCD_REV);
>> > +	DUMP_REG(aux, DP_EDP_GENERAL_CAP_1);
>> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_ADJUSTMENT_CAP);
>> > +	DUMP_REG(aux, DP_EDP_GENERAL_CAP_2);
>> > +	DUMP_REG(aux, DP_EDP_GENERAL_CAP_3);
>> > +	DUMP_REG(aux, DP_EDP_DISPLAY_CONTROL_REGISTER);
>> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER);
>> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB);
>> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_BRIGHTNESS_LSB);
>> > +	DUMP_REG(aux, DP_EDP_PWMGEN_BIT_COUNT);
>> > +	DUMP_REG(aux, DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN);
>> > +	DUMP_REG(aux, DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX);
>> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_CONTROL_STATUS);
>> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_SET);
>> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MSB);
>> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MID);
>> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MIN_LSB);
>> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MAX_MSB);
>> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MAX_MID);
>> > +	DUMP_REG(aux, DP_EDP_BACKLIGHT_FREQ_CAP_MAX_LSB);
>> > +	DUMP_REG(aux, DP_EDP_DBC_MINIMUM_BRIGHTNESS_SET);
>> > +	DUMP_REG(aux, DP_EDP_DBC_MAXIMUM_BRIGHTNESS_SET);
>> > +	DUMP_REG(aux, DP_EDP_REGIONAL_BACKLIGHT_BASE);
>> > +	DUMP_REG(aux, DP_EDP_REGIONAL_BACKLIGHT_0);
>> > +
>> > +#undef DUMP_REG
>> > +}
>> > +EXPORT_SYMBOL(drm_dp_dpcd_dump);
>> > +
>> >  static void drm_dp_link_reset(struct drm_dp_link *link)
>> >  {
>> >  	if (!link)
>> > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
>> > index d041bb00d6a0..089d274f857d 100644
>> > --- a/include/drm/drm_dp_helper.h
>> > +++ b/include/drm/drm_dp_helper.h
>> > @@ -754,6 +754,8 @@ static inline ssize_t drm_dp_dpcd_writeb(struct drm_dp_aux *aux,
>> >  int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
>> >  				 u8 status[DP_LINK_STATUS_SIZE]);
>> >  
>> > +void drm_dp_dpcd_dump(struct drm_dp_aux *aux, struct seq_file *s);
>> > +
>> >  /**
>> >   * struct drm_dp_link_train_set - link training settings
>> >   * @voltage_swing: per-lane voltage swing
>> > -- 
>> > 2.4.5
>> >
>> > _______________________________________________
>> > dri-devel mailing list
>> > dri-devel@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>> 
>> -- 
>> Jani Nikula, Intel Open Source Technology Center

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

Re: [RFC 11/13] drm/dp: Add helper to dump DPCD

[Rafael Antognolli]

On Mon, Aug 17, 2015 at 10:02:04AM +0300, Jani Nikula wrote:
> On Fri, 14 Aug 2015, Rafael Antognolli <rafael.antognolli@intel.com> wrote:
> > On Fri, Aug 14, 2015 at 02:56:55PM +0300, Jani Nikula wrote:
> >> On Wed, 12 Aug 2015, Thierry Reding <thierry.reding@gmail.com> wrote:
> >> > From: Thierry Reding <treding@nvidia.com>
> >> >
> >> > The new drm_dp_dpcd_dump() helper dumps the contents of a DPCD to a
> >> > seq_file and can be used to make the DPCD available via debugfs for
> >> > example.
> >> 
> >> See i915/i915_debugfs.c for one DPCD dump implementation.
> >> 
> >> Around the time that was added, there was also some discussion (and
> >> patches [1]) to expose a read/write debugfs interface to DPCD, letting
> >> userspace access arbitrary DPCD registers.
> >> 
> >> Just this week there was some discussion about revisiting that. It was
> >> about accessing some proprietary panel features, but there's also the
> >> ease of debugging without having to keep updating the kernel to dump
> >> more.
> >> 
> >> I think it would be great to agree on a common debugfs interface to
> >> access DPCD arbitrarily. Last time I checked, the blocker to that was
> >> access to the aux channel from generic code; it's always driver
> >> specific. SMOP. ;)
> >
> > Do you mean it would require the generic code/interface to somehow route
> > this to the driver specific code? I am not sure yet how this works (if
> > there's something like it around), but I'll take a look.
> 
> Drivers can choose to support DP AUX any way they like, and they don't
> have to use the common helpers to do so. It's pretty much an
> implementation detail. I think we could require the use of the common
> helpers to support generic DPCD access from debugfs, but we'd still have
> to come up with a way to get hold of struct drm_dp_aux (again, driver
> internals) given a drm_connector in generic debugfs code.

I was under the assumption they always used the helpers, but I
understand that's not always the case.

Anyway, I was going to propose a new helper to "store" the drm_dp_aux
inside the drm_connector. And just expose something on debugfs if it was
used. Something like:

int drm_dp_aux_store(struct drm_dp_aux *aux,
                     struct drm_connector *connector);

The helpers don't seem to know about drm_connector's though, so I'm not sure
this is a good approach.

> Thierry, do you see any problems with having a connector function to get
> hold of its drm_dp_aux?

Would this be a new function pointer inside struct drm_connector_funcs?
If so, I'm fine with this approach too.

Regarding the interface, you mentioned that you didn't like it because
it had a state. What about just dumping the content of the register into
dmesg when one tries to read it, and use a different sintaxe for
writing, passing both the register addr and the value?

Daniel had another suggestion, regarding an ioctl in debugfs, but I'm
not sure I clearly understand that yet.

Thanks,
Rafael
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 11/13] drm/dp: Add helper to dump DPCD

[Rafael Antognolli]

Hi, I'm back from vacation, so I'll be looking at this again.

On Thu, Aug 20, 2015 at 04:26:42PM -0700, Rafael Antognolli wrote:
> On Mon, Aug 17, 2015 at 10:02:04AM +0300, Jani Nikula wrote:
> > On Fri, 14 Aug 2015, Rafael Antognolli <rafael.antognolli@intel.com> wrote:
> > > On Fri, Aug 14, 2015 at 02:56:55PM +0300, Jani Nikula wrote:
> > >> On Wed, 12 Aug 2015, Thierry Reding <thierry.reding@gmail.com> wrote:
> > >> > From: Thierry Reding <treding@nvidia.com>
> > >> >
> > >> > The new drm_dp_dpcd_dump() helper dumps the contents of a DPCD to a
> > >> > seq_file and can be used to make the DPCD available via debugfs for
> > >> > example.
> > >> 
> > >> See i915/i915_debugfs.c for one DPCD dump implementation.
> > >> 
> > >> Around the time that was added, there was also some discussion (and
> > >> patches [1]) to expose a read/write debugfs interface to DPCD, letting
> > >> userspace access arbitrary DPCD registers.
> > >> 
> > >> Just this week there was some discussion about revisiting that. It was
> > >> about accessing some proprietary panel features, but there's also the
> > >> ease of debugging without having to keep updating the kernel to dump
> > >> more.
> > >> 
> > >> I think it would be great to agree on a common debugfs interface to
> > >> access DPCD arbitrarily. Last time I checked, the blocker to that was
> > >> access to the aux channel from generic code; it's always driver
> > >> specific. SMOP. ;)
> > >
> > > Do you mean it would require the generic code/interface to somehow route
> > > this to the driver specific code? I am not sure yet how this works (if
> > > there's something like it around), but I'll take a look.
> > 
> > Drivers can choose to support DP AUX any way they like, and they don't
> > have to use the common helpers to do so. It's pretty much an
> > implementation detail. I think we could require the use of the common
> > helpers to support generic DPCD access from debugfs, but we'd still have
> > to come up with a way to get hold of struct drm_dp_aux (again, driver
> > internals) given a drm_connector in generic debugfs code.
> 
> I was under the assumption they always used the helpers, but I
> understand that's not always the case.
> 
> Anyway, I was going to propose a new helper to "store" the drm_dp_aux
> inside the drm_connector. And just expose something on debugfs if it was
> used. Something like:
> 
> int drm_dp_aux_store(struct drm_dp_aux *aux,
>                      struct drm_connector *connector);
> 
> The helpers don't seem to know about drm_connector's though, so I'm not sure
> this is a good approach.
> 
> > Thierry, do you see any problems with having a connector function to get
> > hold of its drm_dp_aux?
> 
> Would this be a new function pointer inside struct drm_connector_funcs?
> If so, I'm fine with this approach too.
> 
> Regarding the interface, you mentioned that you didn't like it because
> it had a state. What about just dumping the content of the register into
> dmesg when one tries to read it, and use a different sintaxe for
> writing, passing both the register addr and the value?
> 
> Daniel had another suggestion, regarding an ioctl in debugfs, but I'm
> not sure I clearly understand that yet.

Regarding the interface, this is the comment Daniel did a while ago:

"As mentioned in another thread I think the right approach here is to
expose the dp aux interface through some ioctls in debugfs or dev
somewhere, and then add simple tools on top of that. Similar to what we
have with i2c. That would allow us to implement additional things on top
like mst inspection or using the i2c-over-dp-aux sidechannel."

If exposing through an ioctl, shouldn't it be yet another DRM ioctl? And
if not, then I should create another /dev node/file specifically for this
purpose, right?

And if we are going with ioctls, I still don't understand exactly how it
relates to debugfs.

Thanks,
Rafael
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 11/13] drm/dp: Add helper to dump DPCD

[Daniel Vetter]

On Thu, Sep 03, 2015 at 10:49:34AM -0700, Rafael Antognolli wrote:
> Hi, I'm back from vacation, so I'll be looking at this again.
> 
> On Thu, Aug 20, 2015 at 04:26:42PM -0700, Rafael Antognolli wrote:
> > On Mon, Aug 17, 2015 at 10:02:04AM +0300, Jani Nikula wrote:
> > > On Fri, 14 Aug 2015, Rafael Antognolli <rafael.antognolli@intel.com> wrote:
> > > > On Fri, Aug 14, 2015 at 02:56:55PM +0300, Jani Nikula wrote:
> > > >> On Wed, 12 Aug 2015, Thierry Reding <thierry.reding@gmail.com> wrote:
> > > >> > From: Thierry Reding <treding@nvidia.com>
> > > >> >
> > > >> > The new drm_dp_dpcd_dump() helper dumps the contents of a DPCD to a
> > > >> > seq_file and can be used to make the DPCD available via debugfs for
> > > >> > example.
> > > >> 
> > > >> See i915/i915_debugfs.c for one DPCD dump implementation.
> > > >> 
> > > >> Around the time that was added, there was also some discussion (and
> > > >> patches [1]) to expose a read/write debugfs interface to DPCD, letting
> > > >> userspace access arbitrary DPCD registers.
> > > >> 
> > > >> Just this week there was some discussion about revisiting that. It was
> > > >> about accessing some proprietary panel features, but there's also the
> > > >> ease of debugging without having to keep updating the kernel to dump
> > > >> more.
> > > >> 
> > > >> I think it would be great to agree on a common debugfs interface to
> > > >> access DPCD arbitrarily. Last time I checked, the blocker to that was
> > > >> access to the aux channel from generic code; it's always driver
> > > >> specific. SMOP. ;)
> > > >
> > > > Do you mean it would require the generic code/interface to somehow route
> > > > this to the driver specific code? I am not sure yet how this works (if
> > > > there's something like it around), but I'll take a look.
> > > 
> > > Drivers can choose to support DP AUX any way they like, and they don't
> > > have to use the common helpers to do so. It's pretty much an
> > > implementation detail. I think we could require the use of the common
> > > helpers to support generic DPCD access from debugfs, but we'd still have
> > > to come up with a way to get hold of struct drm_dp_aux (again, driver
> > > internals) given a drm_connector in generic debugfs code.
> > 
> > I was under the assumption they always used the helpers, but I
> > understand that's not always the case.
> > 
> > Anyway, I was going to propose a new helper to "store" the drm_dp_aux
> > inside the drm_connector. And just expose something on debugfs if it was
> > used. Something like:
> > 
> > int drm_dp_aux_store(struct drm_dp_aux *aux,
> >                      struct drm_connector *connector);
> > 
> > The helpers don't seem to know about drm_connector's though, so I'm not sure
> > this is a good approach.
> > 
> > > Thierry, do you see any problems with having a connector function to get
> > > hold of its drm_dp_aux?
> > 
> > Would this be a new function pointer inside struct drm_connector_funcs?
> > If so, I'm fine with this approach too.
> > 
> > Regarding the interface, you mentioned that you didn't like it because
> > it had a state. What about just dumping the content of the register into
> > dmesg when one tries to read it, and use a different sintaxe for
> > writing, passing both the register addr and the value?
> > 
> > Daniel had another suggestion, regarding an ioctl in debugfs, but I'm
> > not sure I clearly understand that yet.
> 
> Regarding the interface, this is the comment Daniel did a while ago:
> 
> "As mentioned in another thread I think the right approach here is to
> expose the dp aux interface through some ioctls in debugfs or dev
> somewhere, and then add simple tools on top of that. Similar to what we
> have with i2c. That would allow us to implement additional things on top
> like mst inspection or using the i2c-over-dp-aux sidechannel."
> 
> If exposing through an ioctl, shouldn't it be yet another DRM ioctl? And
> if not, then I should create another /dev node/file specifically for this
> purpose, right?
> 
> And if we are going with ioctls, I still don't understand exactly how it
> relates to debugfs.

ioctl on some file, either a /dev chardev or just something in debugfs.
And imo it doesn't make sense to have it as an ioctl on the drm device
node since then we'd need to add some form of multiplexing to select the
right dp aux bus (and with dp mst there's new ones for each port on a
downstream hub). I'd just go with copying the i2c design of a chardev for
each master.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC 12/13] drm/dp: link: Track capabilities alongside settings

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Store capabilities in max_* fields and add separate fields for the
currently selected settings.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c    | 35 +++++++++++++++++++++--------------
 drivers/gpu/drm/msm/edp/edp_ctrl.c |  6 +++---
 drivers/gpu/drm/tegra/dpaux.c      |  8 ++++----
 drivers/gpu/drm/tegra/sor.c        | 28 ++++++++++++++--------------
 include/drm/drm_dp_helper.h        |  6 ++++--
 5 files changed, 46 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index ea74884c9cb3..2f35aba8ef17 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -445,10 +445,13 @@ static void drm_dp_link_reset(struct drm_dp_link *link)
 
 	link->revision = 0;
 	link->edp = 0;
-	link->rate = 0;
-	link->num_lanes = 0;
+	link->max_lanes = 0;
+	link->max_rate = 0;
 	link->capabilities = 0;
 	link->aux_rd_interval = 0;
+
+	link->lanes = 0;
+	link->rate = 0;
 }
 
 /**
@@ -474,8 +477,8 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 		return err;
 
 	link->revision = values[0];
-	link->rate = drm_dp_bw_code_to_link_rate(values[1]);
-	link->num_lanes = values[2] & DP_MAX_LANE_COUNT_MASK;
+	link->max_rate = drm_dp_bw_code_to_link_rate(values[1]);
+	link->max_lanes = values[2] & DP_MAX_LANE_COUNT_MASK;
 
 	if (link->revision >= 0x11)
 		if (values[2] & DP_ENHANCED_FRAME_CAP)
@@ -525,6 +528,10 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	/* DP_TRAINING_AUX_RD_INTERVAL is in units of 4 milliseconds */
 	link->aux_rd_interval = values[14] * 4000;
 
+	/* use highest available configuration by default */
+	link->lanes = link->max_lanes;
+	link->rate = link->max_rate;
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_dp_link_probe);
@@ -619,7 +626,7 @@ int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	}
 
 	values[0] = drm_dp_link_rate_to_bw_code(link->rate);
-	values[1] = link->num_lanes;
+	values[1] = link->lanes;
 
 	if (link->capabilities & DP_LINK_CAP_ENHANCED_FRAMING)
 		values[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
@@ -936,7 +943,7 @@ static bool drm_dp_link_train_valid(const struct drm_dp_link_train *train)
 static int drm_dp_link_apply_training(struct drm_dp_link *link)
 {
 	struct drm_dp_link_train_set *request = &link->train.request;
-	unsigned int lanes = link->num_lanes, *vs, *pe, *pc, i;
+	unsigned int lanes = link->lanes, *vs, *pe, *pc, i;
 	struct drm_dp_aux *aux = link->aux;
 	u8 values[4], pattern = 0;
 	int err;
@@ -1024,7 +1031,7 @@ static void drm_dp_link_get_adjustments(struct drm_dp_link *link,
 	struct drm_dp_link_train_set *adjust = &link->train.adjust;
 	unsigned int i;
 
-	for (i = 0; i < link->num_lanes; i++) {
+	for (i = 0; i < link->lanes; i++) {
 		adjust->voltage_swing[i] =
 			drm_dp_get_adjust_request_voltage(status, i) >>
 				DP_TRAIN_VOLTAGE_SWING_SHIFT;
@@ -1074,7 +1081,7 @@ static int drm_dp_link_recover_clock(struct drm_dp_link *link)
 		return err;
 	}
 
-	if (!drm_dp_clock_recovery_ok(status, link->num_lanes))
+	if (!drm_dp_clock_recovery_ok(status, link->lanes))
 		drm_dp_link_get_adjustments(link, status);
 	else
 		link->train.clock_recovered = true;
@@ -1124,13 +1131,13 @@ static int drm_dp_link_equalize_channel(struct drm_dp_link *link)
 		return err;
 	}
 
-	if (!drm_dp_clock_recovery_ok(status, link->num_lanes)) {
+	if (!drm_dp_clock_recovery_ok(status, link->lanes)) {
 		DRM_ERROR("clock recovery lost while equalizing channel\n");
 		link->train.clock_recovered = false;
 		return 0;
 	}
 
-	if (!drm_dp_channel_eq_ok(status, link->num_lanes))
+	if (!drm_dp_channel_eq_ok(status, link->lanes))
 		drm_dp_link_get_adjustments(link, status);
 	else
 		link->train.channel_equalized = true;
@@ -1200,7 +1207,7 @@ static int drm_dp_link_train_full(struct drm_dp_link *link)
 
 retry:
 	DRM_DEBUG_KMS("full-training link: %u lane%s at %u MHz\n",
-		      link->num_lanes, (link->num_lanes > 1) ? "s" : "",
+		      link->lanes, (link->lanes > 1) ? "s" : "",
 		      link->rate / 100);
 
 	err = drm_dp_link_configure(link->aux, link);
@@ -1256,7 +1263,7 @@ static int drm_dp_link_train_fast(struct drm_dp_link *link)
 	int err;
 
 	DRM_DEBUG_KMS("fast-training link: %u lane%s at %u MHz\n",
-		      link->num_lanes, (link->num_lanes > 1) ? "s" : "",
+		      link->lanes, (link->lanes > 1) ? "s" : "",
 		      link->rate / 100);
 
 	err = drm_dp_link_configure(link->aux, link);
@@ -1292,12 +1299,12 @@ static int drm_dp_link_train_fast(struct drm_dp_link *link)
 		goto out;
 	}
 
-	if (!drm_dp_clock_recovery_ok(status, link->num_lanes)) {
+	if (!drm_dp_clock_recovery_ok(status, link->lanes)) {
 		DRM_ERROR("clock recovery failed\n");
 		err = -EIO;
 	}
 
-	if (!drm_dp_channel_eq_ok(status, link->num_lanes)) {
+	if (!drm_dp_channel_eq_ok(status, link->lanes)) {
 		DRM_ERROR("channel equalization failed\n");
 		err = -EIO;
 	}
diff --git a/drivers/gpu/drm/msm/edp/edp_ctrl.c b/drivers/gpu/drm/msm/edp/edp_ctrl.c
index 81200e9be382..28e3726999d3 100644
--- a/drivers/gpu/drm/msm/edp/edp_ctrl.c
+++ b/drivers/gpu/drm/msm/edp/edp_ctrl.c
@@ -416,7 +416,7 @@ static void edp_fill_link_cfg(struct edp_ctrl *ctrl)
 	u32 prate;
 	u32 lrate;
 	u32 bpp;
-	u8 max_lane = ctrl->dp_link.num_lanes;
+	u8 max_lane = ctrl->dp_link.max_lanes;
 	u8 lane;
 
 	prate = ctrl->pixel_rate;
@@ -714,7 +714,7 @@ static int edp_link_rate_down_shift(struct edp_ctrl *ctrl)
 
 	rate = ctrl->link_rate;
 	lane = ctrl->lane_cnt;
-	max_lane = ctrl->dp_link.num_lanes;
+	max_lane = ctrl->dp_link.lanes;
 
 	bpp = ctrl->color_depth * 3;
 	prate = ctrl->pixel_rate;
@@ -772,7 +772,7 @@ static int edp_do_link_train(struct edp_ctrl *ctrl)
 	 * Set the current link rate and lane cnt to panel. They may have been
 	 * adjusted and the values are different from them in DPCD CAP
 	 */
-	dp_link.num_lanes = ctrl->lane_cnt;
+	dp_link.lanes = ctrl->lane_cnt;
 	dp_link.rate = drm_dp_bw_code_to_link_rate(ctrl->link_rate);
 	dp_link.capabilities = ctrl->dp_link.capabilities;
 	if (drm_dp_link_configure(ctrl->drm_aux, &dp_link) < 0)
diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index 07b26972f487..59ec6e57ae6a 100644
--- a/drivers/gpu/drm/tegra/dpaux.c
+++ b/drivers/gpu/drm/tegra/dpaux.c
@@ -528,14 +528,14 @@ int tegra_dpaux_train(struct tegra_dpaux *dpaux, struct drm_dp_link *link,
 	if (tp == DP_TRAINING_PATTERN_DISABLE)
 		return 0;
 
-	for (i = 0; i < link->num_lanes; i++)
+	for (i = 0; i < link->lanes; i++)
 		values[i] = DP_TRAIN_MAX_PRE_EMPHASIS_REACHED |
 			    DP_TRAIN_PRE_EMPH_LEVEL_0 |
 			    DP_TRAIN_MAX_SWING_REACHED |
 			    DP_TRAIN_VOLTAGE_SWING_LEVEL_0;
 
 	err = drm_dp_dpcd_write(&dpaux->aux, DP_TRAINING_LANE0_SET, values,
-				link->num_lanes);
+				link->lanes);
 	if (err < 0)
 		return err;
 
@@ -547,13 +547,13 @@ int tegra_dpaux_train(struct tegra_dpaux *dpaux, struct drm_dp_link *link,
 
 	switch (tp) {
 	case DP_TRAINING_PATTERN_1:
-		if (!drm_dp_clock_recovery_ok(status, link->num_lanes))
+		if (!drm_dp_clock_recovery_ok(status, link->lanes))
 			return -EAGAIN;
 
 		break;
 
 	case DP_TRAINING_PATTERN_2:
-		if (!drm_dp_channel_eq_ok(status, link->num_lanes))
+		if (!drm_dp_channel_eq_ok(status, link->lanes))
 			return -EAGAIN;
 
 		break;
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index ee8ad0d4a0f2..fb1f830fb973 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -133,7 +133,7 @@ static int tegra_sor_dp_train_fast(struct tegra_sor *sor,
 	if (err < 0)
 		return err;
 
-	for (i = 0, value = 0; i < link->num_lanes; i++) {
+	for (i = 0, value = 0; i < link->lanes; i++) {
 		unsigned long lane = SOR_DP_TPG_CHANNEL_CODING |
 				     SOR_DP_TPG_SCRAMBLER_NONE |
 				     SOR_DP_TPG_PATTERN_TRAIN1;
@@ -154,7 +154,7 @@ static int tegra_sor_dp_train_fast(struct tegra_sor *sor,
 	value |= SOR_DP_SPARE_MACRO_SOR_CLK;
 	tegra_sor_writel(sor, value, SOR_DP_SPARE_0);
 
-	for (i = 0, value = 0; i < link->num_lanes; i++) {
+	for (i = 0, value = 0; i < link->lanes; i++) {
 		unsigned long lane = SOR_DP_TPG_CHANNEL_CODING |
 				     SOR_DP_TPG_SCRAMBLER_NONE |
 				     SOR_DP_TPG_PATTERN_TRAIN2;
@@ -169,7 +169,7 @@ static int tegra_sor_dp_train_fast(struct tegra_sor *sor,
 	if (err < 0)
 		return err;
 
-	for (i = 0, value = 0; i < link->num_lanes; i++) {
+	for (i = 0, value = 0; i < link->lanes; i++) {
 		unsigned long lane = SOR_DP_TPG_CHANNEL_CODING |
 				     SOR_DP_TPG_SCRAMBLER_GALIOS |
 				     SOR_DP_TPG_PATTERN_NONE;
@@ -396,11 +396,11 @@ static int tegra_sor_calc_config(struct tegra_sor *sor,
 	u32 num_syms_per_line;
 	unsigned int i;
 
-	if (!link_rate || !link->num_lanes || !pclk || !config->bits_per_pixel)
+	if (!link_rate || !link->lanes || !pclk || !config->bits_per_pixel)
 		return -EINVAL;
 
-	output = link_rate * 8 * link->num_lanes;
 	input = pclk * config->bits_per_pixel;
+	output = link_rate * 8 * link->lanes;
 
 	if (input >= output)
 		return -ERANGE;
@@ -443,7 +443,7 @@ static int tegra_sor_calc_config(struct tegra_sor *sor,
 	watermark = div_u64(watermark + params.error, f);
 	config->watermark = watermark + (config->bits_per_pixel / 8) + 2;
 	num_syms_per_line = (mode->hdisplay * config->bits_per_pixel) *
-			    (link->num_lanes * 8);
+			    (link->lanes * 8);
 
 	if (config->watermark > 30) {
 		config->watermark = 30;
@@ -463,12 +463,12 @@ static int tegra_sor_calc_config(struct tegra_sor *sor,
 	if (link->capabilities & DP_LINK_CAP_ENHANCED_FRAMING)
 		config->hblank_symbols -= 3;
 
-	config->hblank_symbols -= 12 / link->num_lanes;
+	config->hblank_symbols -= 12 / link->lanes;
 
 	/* compute the number of symbols per vertical blanking interval */
 	num = (mode->hdisplay - 25) * link_rate;
 	config->vblank_symbols = div_u64(num, pclk);
-	config->vblank_symbols -= 36 / link->num_lanes + 4;
+	config->vblank_symbols -= 36 / link->lanes + 4;
 
 	dev_dbg(sor->dev, "blank symbols: H:%u V:%u\n", config->hblank_symbols,
 		config->vblank_symbols);
@@ -1100,17 +1100,17 @@ static void tegra_sor_encoder_mode_set(struct drm_encoder *encoder,
 	/* power DP lanes */
 	value = tegra_sor_readl(sor, SOR_DP_PADCTL_0);
 
-	if (link.num_lanes <= 2)
+	if (link.lanes <= 2)
 		value &= ~(SOR_DP_PADCTL_PD_TXD_3 | SOR_DP_PADCTL_PD_TXD_2);
 	else
 		value |= SOR_DP_PADCTL_PD_TXD_3 | SOR_DP_PADCTL_PD_TXD_2;
 
-	if (link.num_lanes <= 1)
+	if (link.lanes <= 1)
 		value &= ~SOR_DP_PADCTL_PD_TXD_1;
 	else
 		value |= SOR_DP_PADCTL_PD_TXD_1;
 
-	if (link.num_lanes == 0)
+	if (link.lanes == 0)
 		value &= ~SOR_DP_PADCTL_PD_TXD_0;
 	else
 		value |= SOR_DP_PADCTL_PD_TXD_0;
@@ -1119,7 +1119,7 @@ static void tegra_sor_encoder_mode_set(struct drm_encoder *encoder,
 
 	value = tegra_sor_readl(sor, SOR_DP_LINKCTL_0);
 	value &= ~SOR_DP_LINKCTL_LANE_COUNT_MASK;
-	value |= SOR_DP_LINKCTL_LANE_COUNT(link.num_lanes);
+	value |= SOR_DP_LINKCTL_LANE_COUNT(link.lanes);
 	tegra_sor_writel(sor, value, SOR_DP_LINKCTL_0);
 
 	/* start lane sequencer */
@@ -1219,7 +1219,7 @@ static void tegra_sor_encoder_mode_set(struct drm_encoder *encoder,
 		}
 
 		rate = drm_dp_link_rate_to_bw_code(link.rate);
-		lanes = link.num_lanes;
+		lanes = link.lanes;
 
 		value = tegra_sor_readl(sor, SOR_CLK_CNTRL);
 		value &= ~SOR_CLK_CNTRL_DP_LINK_SPEED_MASK;
@@ -1237,7 +1237,7 @@ static void tegra_sor_encoder_mode_set(struct drm_encoder *encoder,
 
 		/* disable training pattern generator */
 
-		for (i = 0; i < link.num_lanes; i++) {
+		for (i = 0; i < lanes; i++) {
 			unsigned long lane = SOR_DP_TPG_CHANNEL_CODING |
 					     SOR_DP_TPG_SCRAMBLER_GALIOS |
 					     SOR_DP_TPG_PATTERN_NONE;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 089d274f857d..521f9d75771d 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -807,8 +807,8 @@ struct drm_dp_link_ops {
 struct drm_dp_link {
 	unsigned char revision;
 	unsigned char edp;
-	unsigned int rate;
-	unsigned int num_lanes;
+	unsigned int max_lanes;
+	unsigned int max_rate;
 	unsigned long capabilities;
 	unsigned long aux_rd_interval;
 
@@ -816,6 +816,8 @@ struct drm_dp_link {
 	struct drm_dp_aux *aux;
 
 	struct drm_dp_link_train train;
+	unsigned int lanes;
+	unsigned int rate;
 };
 
 int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link);
-- 
2.4.5

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

[RFC 13/13] drm/dp: Add drm_dp_link_choose() helper

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

This helper chooses an appropriate configuration, according to the
bitrate requirements of the video mode and the capabilities of the
DisplayPort sink.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 54 +++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_dp_helper.h     |  5 ++++
 2 files changed, 59 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 2f35aba8ef17..fd7c26e476d0 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -655,6 +655,60 @@ int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
 }
 EXPORT_SYMBOL(drm_dp_link_configure);
 
+/**
+ * drm_dp_link_choose() - choose the lowest possible configuration for a mode
+ * @link: DRM DP link object
+ * @mode: DRM display mode
+ * @info: DRM display information
+ *
+ * According to the eDP specification, a source should select a configuration
+ * with the lowest number of lanes and the lowest possible link rate that can
+ * match the bitrate requirements of a video mode. However it must ensure not
+ * to exceed the capabilities of the sink.
+ *
+ * Returns: 0 on success or a negative error code on failure.
+ */
+int drm_dp_link_choose(struct drm_dp_link *link,
+		       const struct drm_display_mode *mode,
+		       const struct drm_display_info *info)
+{
+	/* available link symbol clock rates */
+	static const unsigned int rates[3] = { 162000, 270000, 540000 };
+	/* available number of lanes */
+	static const unsigned int lanes[3] = { 1, 2, 4 };
+	unsigned long requirement, capacity;
+	unsigned int rate = link->max_rate;
+	unsigned int i, j;
+
+	/* bandwidth requirement */
+	requirement = mode->clock * info->bpc * 3;
+
+	for (i = 0; i < ARRAY_SIZE(lanes) && lanes[i] <= link->max_lanes; i++) {
+		for (j = 0; j < ARRAY_SIZE(rates) && rates[j] <= rate; j++) {
+			/*
+			 * Capacity for this combination of lanes and rate,
+			 * factoring in the ANSI 8B/10B encoding.
+			 *
+			 * Link rates in the DRM DP helpers are really link
+			 * symbol frequencies, so a tenth of the actual rate
+			 * of the link.
+			 */
+			capacity = lanes[i] * (rates[j] * 10) * 8 / 10;
+
+			if (capacity >= requirement) {
+				DRM_DEBUG_KMS("using %u lanes at %u kHz (%lu/%lu kbps)\n",
+					      lanes[i], rates[j], requirement,
+					      capacity);
+				link->lanes = lanes[i];
+				link->rate = rates[j];
+				return 0;
+			}
+		}
+	}
+
+	return -ERANGE;
+}
+
 /*
  * I2C-over-AUX implementation
  */
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 521f9d75771d..4c296e62b884 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -27,6 +27,8 @@
 #include <linux/i2c.h>
 #include <linux/delay.h>
 
+#include <drm/drm_crtc.h>
+
 /*
  * Unless otherwise noted, all values are from the DP 1.1a spec.  Note that
  * DP and DPCD versions are independent.  Differences from 1.0 are not noted,
@@ -824,6 +826,9 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link);
 int drm_dp_link_power_up(struct drm_dp_aux *aux, struct drm_dp_link *link);
 int drm_dp_link_power_down(struct drm_dp_aux *aux, struct drm_dp_link *link);
 int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link);
+int drm_dp_link_choose(struct drm_dp_link *link,
+		       const struct drm_display_mode *mode,
+		       const struct drm_display_info *info);
 
 int drm_dp_link_train(struct drm_dp_link *link);
 
-- 
2.4.5

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