[PATCH 0/4] Fix handling of DP bridge devices

[PATCH 0/4] Fix handling of DP bridge devices

[Adam Jackson]

Connection sense for DP bridge devices (like DP->VGA) currently only looks
at DPCD presence.  This is wrong.  However we would still like to avoid
DDC reads if possible, since they're much slower than AUXCH.

This series enhances i915 DP detection to know about branch devices, and
attempts to discover downstream port status and reflect that back to KMS.
It does not handle the case of multiple downstream ports very well, but
then neither does the existing code; if anyone has an example of such a
device I'll be happy to investigate.

- ajax

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

[Adam Jackson]

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.

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]

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

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

[Adam Jackson]

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 92939bd..9809c53 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2088,11 +2088,37 @@ 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 && (intel_dp->link_configuration[0] & DP_SINK_COUNT_MASK))
+		return connector_status_connected;
+
+	/* 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: [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 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

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