[PATCH 3/4] drm/i915/dp: Fetch downstream port info if needed during DPCD fetch

[PATCH 3/4] drm/i915/dp: Fetch downstream port info if needed during DPCD fetch

[Adam Jackson]

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 drivers/gpu/drm/i915/intel_dp.c |   25 ++++++++++++++++++++-----
 1 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ace757a..92939bd 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -39,6 +39,7 @@
 #include "drm_dp_helper.h"
 
 #define DP_RECEIVER_CAP_SIZE	0xf
+#define DP_MAX_DOWNSTREAM_PORTS 0xf
 #define DP_LINK_STATUS_SIZE	6
 #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
 
@@ -56,6 +57,7 @@ struct intel_dp {
 	uint8_t link_bw;
 	uint8_t lane_count;
 	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
+	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
 	struct i2c_adapter adapter;
 	struct i2c_algo_dp_aux_data algo;
 	bool is_pch_edp;
@@ -1968,12 +1970,25 @@ static bool
 intel_dp_get_dpcd(struct intel_dp *intel_dp)
 {
 	if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
-					   sizeof(intel_dp->dpcd)) &&
-	    (intel_dp->dpcd[DP_DPCD_REV] != 0)) {
-		return true;
-	}
+					   sizeof(intel_dp->dpcd) == 0))
+		return false; /* aux transfer failed */
 
-	return false;
+	if (intel_dp->dpcd[DP_DPCD_REV] == 0)
+		return false; /* DPCD not present */
+
+	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
+	      DP_DWN_STRM_PORT_PRESENT))
+		return true; /* native DP sink */
+
+	if (intel_dp->dpcd[DP_DPCD_REV] == 0x10)
+		return true; /* no per-port downstream info */
+
+	if (intel_dp_aux_native_read_retry(intel_dp, DP_DOWNSTREAM_PORT_0,
+					   intel_dp->downstream_ports,
+					   DP_MAX_DOWNSTREAM_PORTS) == 0)
+		return false; /* downstream port status fetch failed */
+
+	return true;
 }
 
 static void
-- 
1.7.7.6

Re: [PATCH 3/4] drm/i915/dp: Fetch downstream port info if needed during DPCD fetch

[Jani Nikula]

On Mon, 17 Sep 2012, Adam Jackson <ajax@redhat.com> wrote:
> Signed-off-by: Adam Jackson <ajax@redhat.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |   25 ++++++++++++++++++++-----
>  1 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index ace757a..92939bd 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -39,6 +39,7 @@
>  #include "drm_dp_helper.h"
>  
>  #define DP_RECEIVER_CAP_SIZE	0xf
> +#define DP_MAX_DOWNSTREAM_PORTS 0xf
>  #define DP_LINK_STATUS_SIZE	6
>  #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
>  
> @@ -56,6 +57,7 @@ struct intel_dp {
>  	uint8_t link_bw;
>  	uint8_t lane_count;
>  	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
> +	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
>  	struct i2c_adapter adapter;
>  	struct i2c_algo_dp_aux_data algo;
>  	bool is_pch_edp;
> @@ -1968,12 +1970,25 @@ static bool
>  intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  {
>  	if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
> -					   sizeof(intel_dp->dpcd)) &&
> -	    (intel_dp->dpcd[DP_DPCD_REV] != 0)) {
> -		return true;
> -	}
> +					   sizeof(intel_dp->dpcd) == 0))

Misplaced brace, isn't it? You're passing "sizeof(intel_dp->dpcd) == 0"
as length.

BR,
Jani.

> +		return false; /* aux transfer failed */
>  
> -	return false;
> +	if (intel_dp->dpcd[DP_DPCD_REV] == 0)
> +		return false; /* DPCD not present */
> +
> +	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> +	      DP_DWN_STRM_PORT_PRESENT))
> +		return true; /* native DP sink */
> +
> +	if (intel_dp->dpcd[DP_DPCD_REV] == 0x10)
> +		return true; /* no per-port downstream info */
> +
> +	if (intel_dp_aux_native_read_retry(intel_dp, DP_DOWNSTREAM_PORT_0,
> +					   intel_dp->downstream_ports,
> +					   DP_MAX_DOWNSTREAM_PORTS) == 0)
> +		return false; /* downstream port status fetch failed */
> +
> +	return true;
>  }
>  
>  static void
> -- 
> 1.7.7.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/4] drm: Export drm_probe_ddc()

[Adam Jackson]

Tested-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 drivers/gpu/drm/drm_edid.c |    3 ++-
 include/drm/drm_crtc.h     |    1 +
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index b7ee230..a1895ba 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -376,13 +376,14 @@ out:
  * \param adapter : i2c device adaptor
  * \return 1 on success
  */
-static bool
+bool
 drm_probe_ddc(struct i2c_adapter *adapter)
 {
 	unsigned char out;
 
 	return (drm_do_probe_ddc_edid(adapter, &out, 0, 1) == 0);
 }
+EXPORT_SYMBOL(drm_probe_ddc);
 
 /**
  * drm_get_edid - get EDID data, if available
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index bfacf0d..f5d434b 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -862,6 +862,7 @@ extern char *drm_get_tv_subconnector_name(int val);
 extern char *drm_get_tv_select_name(int val);
 extern void drm_fb_release(struct drm_file *file_priv);
 extern int drm_mode_group_init_legacy_group(struct drm_device *dev, struct drm_mode_group *group);
+extern bool drm_probe_ddc(struct i2c_adapter *adapter);
 extern struct edid *drm_get_edid(struct drm_connector *connector,
 				 struct i2c_adapter *adapter);
 extern int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid);
-- 
1.7.7.6

[PATCH 2/4] drm/dp: Update DPCD defines

[Adam Jackson]

Sources: DP, eDP, and DP interop specs, and a VESA slideshow about DP
1.2 for the MST bits.

Tested-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 include/drm/drm_dp_helper.h |   60 ++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 1744b18c..f9888c3 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -69,16 +69,30 @@
 /* 10b = TMDS or HDMI */
 /* 11b = Other */
 # define DP_FORMAT_CONVERSION               (1 << 3)
+# define DP_DETAILED_CAP_INFO_AVAILABLE	    (1 << 4)
 
 #define DP_MAIN_LINK_CHANNEL_CODING         0x006
 
 #define DP_DOWN_STREAM_PORT_COUNT	    0x007
-#define  DP_PORT_COUNT_MASK		    0x0f
-#define  DP_OUI_SUPPORT			    (1 << 7)
+# define DP_PORT_COUNT_MASK		    0x0f
+# define DP_MSA_TIMING_PAR_IGNORED	    (1 << 6)
+# define DP_OUI_SUPPORT			    (1 << 7)
+
+#define DP_I2C_SPEED_CAP		    0x00c
+# define DP_I2C_SPEED_1K		    0x01
+# define DP_I2C_SPEED_5K		    0x02
+# define DP_I2C_SPEED_10K		    0x04
+# define DP_I2C_SPEED_100K		    0x08
+# define DP_I2C_SPEED_400K		    0x10
+# define DP_I2C_SPEED_1M		    0x20
 
 #define DP_EDP_CONFIGURATION_CAP            0x00d
 #define DP_TRAINING_AUX_RD_INTERVAL         0x00e
 
+/* Multiple stream transport */
+#define DP_MSTM_CAP			    0x021
+# define DP_MST_CAP			    (1 << 0)
+
 #define DP_PSR_SUPPORT                      0x070
 # define DP_PSR_IS_SUPPORTED                1
 #define DP_PSR_CAPS                         0x071
@@ -93,6 +107,31 @@
 # define DP_PSR_SETUP_TIME_MASK             (7 << 1)
 # define DP_PSR_SETUP_TIME_SHIFT            1
 
+/*
+ * 0x80-0x8f describe downstream port capabilities, but there are two layouts
+ * based on whether DP_DETAILED_CAP_INFO_AVAILABLE was set.  If it was not,
+ * each port's descriptor is one byte wide.  If it was set, each port's is
+ * four bytes wide, starting with the one byte from the base info.  As of
+ * DP interop v1.1a only VGA defines additional detail.
+ */
+
+/* offset 0 */
+#define DP_DOWNSTREAM_PORT_0		    0x80
+# define DP_DS_PORT_TYPE_MASK		    (7 << 0)
+# define DP_DS_PORT_TYPE_DP		    0
+# define DP_DS_PORT_TYPE_VGA		    1
+# define DP_DS_PORT_TYPE_DVI		    2
+# define DP_DS_PORT_TYPE_HDMI		    3
+# define DP_DS_PORT_TYPE_NON_EDID	    4
+# define DP_DS_PORT_HPD			    (1 << 3)
+/* offset 1 for VGA is maximum megapixels per second / 8 */
+/* offset 2 */
+# define DP_DS_VGA_MAX_BPC_MASK		    (3 << 0)
+# define DP_DS_VGA_8BPC			    0
+# define DP_DS_VGA_10BPC		    1
+# define DP_DS_VGA_12BPC		    2
+# define DP_DS_VGA_16BPC		    3
+
 /* link configuration */
 #define	DP_LINK_BW_SET		            0x100
 # define DP_LINK_BW_1_62		    0x06
@@ -148,24 +187,37 @@
 
 #define DP_DOWNSPREAD_CTRL		    0x107
 # define DP_SPREAD_AMP_0_5		    (1 << 4)
+# define DP_MSA_TIMING_PAR_IGNORE_EN	    (1 << 7)
 
 #define DP_MAIN_LINK_CHANNEL_CODING_SET	    0x108
 # define DP_SET_ANSI_8B10B		    (1 << 0)
 
+#define DP_I2C_SPEED_CONTROL_STATUS	    0x109
+/* bitmask as for DP_I2C_SPEED_CAP */
+
+#define DP_EDP_CONFIGURATION_SET            0x10a
+
+#define DP_MSTM_CTRL			    0x111
+# define DP_MST_EN			    (1 << 0)
+# define DP_UP_REQ_EN			    (1 << 1)
+# define DP_UPSTREAM_IS_SRC		    (1 << 2)
+
 #define DP_PSR_EN_CFG			    0x170
 # define DP_PSR_ENABLE			    (1 << 0)
 # define DP_PSR_MAIN_LINK_ACTIVE	    (1 << 1)
 # define DP_PSR_CRC_VERIFICATION	    (1 << 2)
 # define DP_PSR_FRAME_CAPTURE		    (1 << 3)
 
+#define DP_SINK_COUNT			    0x200
+# define DP_SINK_COUNT_MASK		    (31 << 0)
+# define DP_SINK_CP_READY		    (1 << 6)
+
 #define DP_DEVICE_SERVICE_IRQ_VECTOR	    0x201
 # define DP_REMOTE_CONTROL_COMMAND_PENDING  (1 << 0)
 # define DP_AUTOMATED_TEST_REQUEST	    (1 << 1)
 # define DP_CP_IRQ			    (1 << 2)
 # define DP_SINK_SPECIFIC_IRQ		    (1 << 6)
 
-#define DP_EDP_CONFIGURATION_SET            0x10a
-
 #define DP_LANE0_1_STATUS		    0x202
 #define DP_LANE2_3_STATUS		    0x203
 # define DP_LANE_CR_DONE		    (1 << 0)
-- 
1.7.7.6

[PATCH 3/4] drm/i915/dp: Fetch downstream port info if needed during DPCD fetch

[Adam Jackson]

v2: Fix parenthesis mismatch, spotted by Jani Nikula

Tested-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 drivers/gpu/drm/i915/intel_dp.c |   25 ++++++++++++++++++++-----
 1 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ace757a..098119e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -39,6 +39,7 @@
 #include "drm_dp_helper.h"
 
 #define DP_RECEIVER_CAP_SIZE	0xf
+#define DP_MAX_DOWNSTREAM_PORTS 0xf
 #define DP_LINK_STATUS_SIZE	6
 #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
 
@@ -56,6 +57,7 @@ struct intel_dp {
 	uint8_t link_bw;
 	uint8_t lane_count;
 	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
+	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
 	struct i2c_adapter adapter;
 	struct i2c_algo_dp_aux_data algo;
 	bool is_pch_edp;
@@ -1968,12 +1970,25 @@ static bool
 intel_dp_get_dpcd(struct intel_dp *intel_dp)
 {
 	if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
-					   sizeof(intel_dp->dpcd)) &&
-	    (intel_dp->dpcd[DP_DPCD_REV] != 0)) {
-		return true;
-	}
+					   sizeof(intel_dp->dpcd)) == 0)
+		return false; /* aux transfer failed */
 
-	return false;
+	if (intel_dp->dpcd[DP_DPCD_REV] == 0)
+		return false; /* DPCD not present */
+
+	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
+	      DP_DWN_STRM_PORT_PRESENT))
+		return true; /* native DP sink */
+
+	if (intel_dp->dpcd[DP_DPCD_REV] == 0x10)
+		return true; /* no per-port downstream info */
+
+	if (intel_dp_aux_native_read_retry(intel_dp, DP_DOWNSTREAM_PORT_0,
+					   intel_dp->downstream_ports,
+					   DP_MAX_DOWNSTREAM_PORTS) == 0)
+		return false; /* downstream port status fetch failed */
+
+	return true;
 }
 
 static void
-- 
1.7.7.6

[PATCH 4/4] drm/i915/dp: Be smarter about connection sense for branch devices

[Adam Jackson]

If there's no downstream device, DPCD success is good enough.  If
there's a hotplug-capable downstream device, count the number of
connected sinks in DP_SINK_STATUS and return success if it's non-zero.
Otherwise, probe DDC and report appropriately.

v2: Check DP_SINK_STATUS instead of something unrelated to sink status.

Tested-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 drivers/gpu/drm/i915/intel_dp.c |   35 ++++++++++++++++++++++++++++++++++-
 1 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 098119e..813b771 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2088,11 +2088,44 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 	}
 }
 
+/* XXX this is probably wrong for multiple downstream ports */
 static enum drm_connector_status
 intel_dp_detect_dpcd(struct intel_dp *intel_dp)
 {
-	if (intel_dp_get_dpcd(intel_dp))
+	uint8_t *dpcd = intel_dp->dpcd;
+	bool hpd;
+	uint8_t type;
+
+	if (!intel_dp_get_dpcd(intel_dp))
+		return connector_status_disconnected;
+
+	/* if there's no downstream port, we're done */
+	if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT))
+		return connector_status_connected;
+
+	/* If we're HPD-aware, SINK_COUNT changes dynamically */
+	hpd = !!(intel_dp->downstream_ports[0] & DP_DS_PORT_HPD);
+	if (hpd) {
+		uint8_t sink_count;
+		if (!intel_dp_aux_native_read_retry(intel_dp, DP_SINK_COUNT,
+						    &sink_count, 1))
+			return connector_status_unknown;
+		sink_count &= DP_SINK_COUNT_MASK;
+		return sink_count ? connector_status_connected
+				  : connector_status_disconnected;
+	}
+
+	/* If no HPD, poke DDC gently */
+	if (drm_probe_ddc(&intel_dp->adapter))
 		return connector_status_connected;
+
+	/* Well we tried, say unknown for unreliable port types */
+	type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
+	if (type == DP_DS_PORT_TYPE_VGA || type == DP_DS_PORT_TYPE_NON_EDID)
+		return connector_status_unknown;
+
+	/* Anything else is out of spec, warn and ignore */
+	DRM_DEBUG_KMS("Broken DP branch device, ignoring\n");
 	return connector_status_disconnected;
 }
 
-- 
1.7.7.6

Re: [Intel-gfx] [PATCH 4/4] drm/i915/dp: Be smarter about connection sense for branch devices

[Adam Jackson]

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

On Tue, 2012-09-18 at 10:58 -0400, Adam Jackson wrote:
> If there's no downstream device, DPCD success is good enough.  If
> there's a hotplug-capable downstream device, count the number of
> connected sinks in DP_SINK_STATUS and return success if it's non-zero.
> Otherwise, probe DDC and report appropriately.
> 
> v2: Check DP_SINK_STATUS instead of something unrelated to sink status.

Ugh, s/STATUS/COUNT/g in the summary, sorry.

- ajax

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

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

Re: [Intel-gfx] [PATCH 3/4] drm/i915/dp: Fetch downstream port info if needed during DPCD fetch

[Jani Nikula]

On Tue, 18 Sep 2012, Adam Jackson <ajax@redhat.com> wrote:
> v2: Fix parenthesis mismatch, spotted by Jani Nikula
>
> Tested-by: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Adam Jackson <ajax@redhat.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |   25 ++++++++++++++++++++-----
>  1 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index ace757a..098119e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -39,6 +39,7 @@
>  #include "drm_dp_helper.h"
>  
>  #define DP_RECEIVER_CAP_SIZE	0xf
> +#define DP_MAX_DOWNSTREAM_PORTS 0xf

That should be 0x10. It doesn't matter in this series, but would be nice
to fix it for correctness. Sorry I didn't spot this the first time
around.

Otherwise, on the series,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>  #define DP_LINK_STATUS_SIZE	6
>  #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
>  
> @@ -56,6 +57,7 @@ struct intel_dp {
>  	uint8_t link_bw;
>  	uint8_t lane_count;
>  	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
> +	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
>  	struct i2c_adapter adapter;
>  	struct i2c_algo_dp_aux_data algo;
>  	bool is_pch_edp;
> @@ -1968,12 +1970,25 @@ static bool
>  intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  {
>  	if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
> -					   sizeof(intel_dp->dpcd)) &&
> -	    (intel_dp->dpcd[DP_DPCD_REV] != 0)) {
> -		return true;
> -	}
> +					   sizeof(intel_dp->dpcd)) == 0)
> +		return false; /* aux transfer failed */
>  
> -	return false;
> +	if (intel_dp->dpcd[DP_DPCD_REV] == 0)
> +		return false; /* DPCD not present */
> +
> +	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> +	      DP_DWN_STRM_PORT_PRESENT))
> +		return true; /* native DP sink */
> +
> +	if (intel_dp->dpcd[DP_DPCD_REV] == 0x10)
> +		return true; /* no per-port downstream info */
> +
> +	if (intel_dp_aux_native_read_retry(intel_dp, DP_DOWNSTREAM_PORT_0,
> +					   intel_dp->downstream_ports,
> +					   DP_MAX_DOWNSTREAM_PORTS) == 0)
> +		return false; /* downstream port status fetch failed */
> +
> +	return true;
>  }
>  
>  static void
> -- 
> 1.7.7.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/4] drm/dp: Update DPCD defines

[Paulo Zanoni]

Hi

See below:

2012/9/18 Adam Jackson <ajax@redhat.com>:
> Sources: DP, eDP, and DP interop specs, and a VESA slideshow about DP
> 1.2 for the MST bits.

All I needed to review every bit was DP spec version 1.2.

>
> Tested-by: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Adam Jackson <ajax@redhat.com>
> ---
>  include/drm/drm_dp_helper.h |   60 ++++++++++++++++++++++++++++++++++++++++---
>  1 files changed, 56 insertions(+), 4 deletions(-)
>
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 1744b18c..f9888c3 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -69,16 +69,30 @@
>  /* 10b = TMDS or HDMI */
>  /* 11b = Other */
>  # define DP_FORMAT_CONVERSION               (1 << 3)
> +# define DP_DETAILED_CAP_INFO_AVAILABLE            (1 << 4)
>
>  #define DP_MAIN_LINK_CHANNEL_CODING         0x006
>
>  #define DP_DOWN_STREAM_PORT_COUNT          0x007
> -#define  DP_PORT_COUNT_MASK                0x0f
> -#define  DP_OUI_SUPPORT                            (1 << 7)
> +# define DP_PORT_COUNT_MASK                0x0f
> +# define DP_MSA_TIMING_PAR_IGNORED         (1 << 6)
> +# define DP_OUI_SUPPORT                            (1 << 7)
> +
> +#define DP_I2C_SPEED_CAP                   0x00c
> +# define DP_I2C_SPEED_1K                   0x01
> +# define DP_I2C_SPEED_5K                   0x02
> +# define DP_I2C_SPEED_10K                  0x04
> +# define DP_I2C_SPEED_100K                 0x08
> +# define DP_I2C_SPEED_400K                 0x10
> +# define DP_I2C_SPEED_1M                   0x20
>
>  #define DP_EDP_CONFIGURATION_CAP            0x00d
>  #define DP_TRAINING_AUX_RD_INTERVAL         0x00e
>
> +/* Multiple stream transport */
> +#define DP_MSTM_CAP                        0x021
> +# define DP_MST_CAP                        (1 << 0)
> +
>  #define DP_PSR_SUPPORT                      0x070
>  # define DP_PSR_IS_SUPPORTED                1
>  #define DP_PSR_CAPS                         0x071
> @@ -93,6 +107,31 @@
>  # define DP_PSR_SETUP_TIME_MASK             (7 << 1)
>  # define DP_PSR_SETUP_TIME_SHIFT            1
>
> +/*
> + * 0x80-0x8f describe downstream port capabilities, but there are two layouts
> + * based on whether DP_DETAILED_CAP_INFO_AVAILABLE was set.  If it was not,
> + * each port's descriptor is one byte wide.  If it was set, each port's is
> + * four bytes wide, starting with the one byte from the base info.  As of
> + * DP interop v1.1a only VGA defines additional detail.
> + */
> +
> +/* offset 0 */
> +#define DP_DOWNSTREAM_PORT_0               0x80
> +# define DP_DS_PORT_TYPE_MASK              (7 << 0)
> +# define DP_DS_PORT_TYPE_DP                0
> +# define DP_DS_PORT_TYPE_VGA               1
> +# define DP_DS_PORT_TYPE_DVI               2
> +# define DP_DS_PORT_TYPE_HDMI              3
> +# define DP_DS_PORT_TYPE_NON_EDID          4
> +# define DP_DS_PORT_HPD                            (1 << 3)
> +/* offset 1 for VGA is maximum megapixels per second / 8 */
> +/* offset 2 */
> +# define DP_DS_VGA_MAX_BPC_MASK                    (3 << 0)
> +# define DP_DS_VGA_8BPC                            0
> +# define DP_DS_VGA_10BPC                   1
> +# define DP_DS_VGA_12BPC                   2
> +# define DP_DS_VGA_16BPC                   3
> +
>  /* link configuration */
>  #define        DP_LINK_BW_SET                      0x100
>  # define DP_LINK_BW_1_62                   0x06
> @@ -148,24 +187,37 @@
>
>  #define DP_DOWNSPREAD_CTRL                 0x107
>  # define DP_SPREAD_AMP_0_5                 (1 << 4)
> +# define DP_MSA_TIMING_PAR_IGNORE_EN       (1 << 7)
>
>  #define DP_MAIN_LINK_CHANNEL_CODING_SET            0x108
>  # define DP_SET_ANSI_8B10B                 (1 << 0)
>
> +#define DP_I2C_SPEED_CONTROL_STATUS        0x109
> +/* bitmask as for DP_I2C_SPEED_CAP */
> +
> +#define DP_EDP_CONFIGURATION_SET            0x10a
> +
> +#define DP_MSTM_CTRL                       0x111
> +# define DP_MST_EN                         (1 << 0)
> +# define DP_UP_REQ_EN                      (1 << 1)
> +# define DP_UPSTREAM_IS_SRC                (1 << 2)
> +
>  #define DP_PSR_EN_CFG                      0x170
>  # define DP_PSR_ENABLE                     (1 << 0)
>  # define DP_PSR_MAIN_LINK_ACTIVE           (1 << 1)
>  # define DP_PSR_CRC_VERIFICATION           (1 << 2)
>  # define DP_PSR_FRAME_CAPTURE              (1 << 3)
>
> +#define DP_SINK_COUNT                      0x200
> +# define DP_SINK_COUNT_MASK                (31 << 0)

My DP spec version 1.2 says "bits 7 and 5:0", but the DP 1.1 spec says
it's just 5:0 and "Bits 7 = RESERVED". So should we treat bit 7 as the
most-significant-bit? Notice that this will affect patch 4 of this
series.

Idea for a follow-up patch: maybe we should try to add some comments
explaining which bits appeared only in some specific DPCD x.y
revision?

With the "bit 7" problem fixed somehow:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> +# define DP_SINK_CP_READY                  (1 << 6)
> +
>  #define DP_DEVICE_SERVICE_IRQ_VECTOR       0x201
>  # define DP_REMOTE_CONTROL_COMMAND_PENDING  (1 << 0)
>  # define DP_AUTOMATED_TEST_REQUEST         (1 << 1)
>  # define DP_CP_IRQ                         (1 << 2)
>  # define DP_SINK_SPECIFIC_IRQ              (1 << 6)
>
> -#define DP_EDP_CONFIGURATION_SET            0x10a
> -
>  #define DP_LANE0_1_STATUS                  0x202
>  #define DP_LANE2_3_STATUS                  0x203
>  # define DP_LANE_CR_DONE                   (1 << 0)
> --
> 1.7.7.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [Intel-gfx] [PATCH 2/4] drm/dp: Update DPCD defines

[Adam Jackson]

On 9/20/12 10:10 AM, Paulo Zanoni wrote:
> 2012/9/18 Adam Jackson <ajax@redhat.com>:
>> Sources: DP, eDP, and DP interop specs, and a VESA slideshow about DP
>> 1.2 for the MST bits.
>
> All I needed to review every bit was DP spec version 1.2.

Lucky you!  I don't have a copy.

>> +#define DP_SINK_COUNT                      0x200
>> +# define DP_SINK_COUNT_MASK                (31 << 0)
>
> My DP spec version 1.2 says "bits 7 and 5:0", but the DP 1.1 spec says
> it's just 5:0 and "Bits 7 = RESERVED". So should we treat bit 7 as the
> most-significant-bit? Notice that this will affect patch 4 of this
> series.

Oh, wild.  I guess they did that so they could have twice as many 
downstream devices?

> Idea for a follow-up patch: maybe we should try to add some comments
> explaining which bits appeared only in some specific DPCD x.y
> revision?

That's a good idea.

I'll send follow-ups for that, and to make a DP_GET_SINK_COUNT() that 
does the right math.

- ajax

Re: [PATCH 3/4] drm/i915/dp: Fetch downstream port info if needed during DPCD fetch

[Daniel Vetter]

On Thu, Sep 20, 2012 at 03:28:45PM +0300, Jani Nikula wrote:
> On Tue, 18 Sep 2012, Adam Jackson <ajax@redhat.com> wrote:
> > v2: Fix parenthesis mismatch, spotted by Jani Nikula
> >
> > Tested-by: Takashi Iwai <tiwai@suse.de>
> > Signed-off-by: Adam Jackson <ajax@redhat.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c |   25 ++++++++++++++++++++-----
> >  1 files changed, 20 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index ace757a..098119e 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -39,6 +39,7 @@
> >  #include "drm_dp_helper.h"
> >  
> >  #define DP_RECEIVER_CAP_SIZE	0xf
> > +#define DP_MAX_DOWNSTREAM_PORTS 0xf
> 
> That should be 0x10. It doesn't matter in this series, but would be nice
> to fix it for correctness. Sorry I didn't spot this the first time
> around.
> 
> Otherwise, on the series,
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Ok, I've merged these 4 patches, with the #define fixed and a small merge
conflict resolved. Please yell if I've botched things up ...

Thanks, Daniel
> 
> 
> >  #define DP_LINK_STATUS_SIZE	6
> >  #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
> >  
> > @@ -56,6 +57,7 @@ struct intel_dp {
> >  	uint8_t link_bw;
> >  	uint8_t lane_count;
> >  	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
> > +	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
> >  	struct i2c_adapter adapter;
> >  	struct i2c_algo_dp_aux_data algo;
> >  	bool is_pch_edp;
> > @@ -1968,12 +1970,25 @@ static bool
> >  intel_dp_get_dpcd(struct intel_dp *intel_dp)
> >  {
> >  	if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
> > -					   sizeof(intel_dp->dpcd)) &&
> > -	    (intel_dp->dpcd[DP_DPCD_REV] != 0)) {
> > -		return true;
> > -	}
> > +					   sizeof(intel_dp->dpcd)) == 0)
> > +		return false; /* aux transfer failed */
> >  
> > -	return false;
> > +	if (intel_dp->dpcd[DP_DPCD_REV] == 0)
> > +		return false; /* DPCD not present */
> > +
> > +	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> > +	      DP_DWN_STRM_PORT_PRESENT))
> > +		return true; /* native DP sink */
> > +
> > +	if (intel_dp->dpcd[DP_DPCD_REV] == 0x10)
> > +		return true; /* no per-port downstream info */
> > +
> > +	if (intel_dp_aux_native_read_retry(intel_dp, DP_DOWNSTREAM_PORT_0,
> > +					   intel_dp->downstream_ports,
> > +					   DP_MAX_DOWNSTREAM_PORTS) == 0)
> > +		return false; /* downstream port status fetch failed */
> > +
> > +	return true;
> >  }
> >  
> >  static void
> > -- 
> > 1.7.7.6
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch