Skip to contentSkip to conversation

drm + i915 DP MST code preview

RFC New Dave Airlie 4 participants 22 messages May 2, 2014 – May 8, 2014

Subsystems (3)
DRM drivers DRM misc Intel i915
lore
Share & tools
lore

Dave Airlie

author maintainer
#0 lore raw

Hi,

Branch: http://cgit.freedesktop.org/~airlied/linux/log/?h=drm-i915-mst-support

Okay I've gotten this to light up some monitors using -modesetting so it must be ready for an initial posting,

Tested on Haswell so far with Lenovo Ultradock and with Ancell MST Hub devices, (checked it can detect the Dell 32" Monitor - but that is another world of pain).

The main problem is documented in the i915 patch, state check backtraces abound.

There is still quite a bit of work todo for a compliant solution, but I don't have a compliance tester so I can only imagine the fail.

The helper interface can probably be cleaned up, the whole ACT area is bit of a mess, and I'd probably need an idea from other GPUs how best to do that,

Dave.

author maintainer
#1 lore raw

PATCH 1/8 drm/dp_helper: add defines for DP 1.2 and MST support.

From: Dave Airlie <[email protected]>

This just adds the defines from the DP 1.2 spec, which we will use later.

Signed-off-by Dave Airlie <[email protected]>
---
 include/drm/drm_dp_helper.h | 78 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)
include/drm/drm_dp_helper.h +78 -0
index cfcacec..879836d 100644
@@ -37,6 +37,7 @@
  * eDP: Embedded DisplayPort version 1
  * DPI: DisplayPort Interoperability Guideline v1.1a
  * 1.2: DisplayPort 1.2
+ * MST: Multistream Transport - part of DP 1.2a
  *
  * 1.2 formally includes both eDP and DPI definitions.
  */

@@ -103,9 +104,14 @@
 #define DP_TRAINING_AUX_RD_INTERVAL         0x00e   /* XXX 1.2? */
 
 /* Multiple stream transport */
+#define DP_FAUX_CAP			    0x020   /* 1.2 */
+# define DP_FAUX_CAP_1			    (1 << 0)
+
 #define DP_MSTM_CAP			    0x021   /* 1.2 */
 # define DP_MST_CAP			    (1 << 0)
 
+#define DP_GUID				    0x030   /* 1.2 */
+
 #define DP_PSR_SUPPORT                      0x070   /* XXX 1.2? */
 # define DP_PSR_IS_SUPPORTED                1
 #define DP_PSR_CAPS                         0x071   /* XXX 1.2? */

@@ -221,6 +227,16 @@
 # define DP_PSR_CRC_VERIFICATION	    (1 << 2)
 # define DP_PSR_FRAME_CAPTURE		    (1 << 3)
 
+#define DP_ADAPTER_CTRL			    0x1a0
+# define DP_ADAPTER_CTRL_FORCE_LOAD_SENSE   (1 << 0)
+
+#define DP_BRANCH_DEVICE_CTRL		    0x1a1
+# define DP_BRANCH_DEVICE_IRQ_HPD	    (1 << 0)
+
+#define DP_PAYLOAD_ALLOCATE_SET		    0x1c0
+#define DP_PAYLOAD_ALLOCATE_START_TIME_SLOT 0x1c1
+#define DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT 0x1c2
+
 #define DP_SINK_COUNT			    0x200
 /* prior to 1.2 bit 7 was reserved mbz */
 # define DP_GET_SINK_COUNT(x)		    ((((x) & 0x80) >> 1) | ((x) & 0x3f))

@@ -230,6 +246,9 @@
 # define DP_REMOTE_CONTROL_COMMAND_PENDING  (1 << 0)
 # define DP_AUTOMATED_TEST_REQUEST	    (1 << 1)
 # define DP_CP_IRQ			    (1 << 2)
+# define DP_MCCS_IRQ			    (1 << 3)
+# define DP_DOWN_REP_MSG_RDY		    (1 << 4) /* DP MST */
+# define DP_UP_REQ_MSG_RDY		    (1 << 5) /* DP MST */
 # define DP_SINK_SPECIFIC_IRQ		    (1 << 6)
 
 #define DP_LANE0_1_STATUS		    0x202

@@ -294,6 +313,13 @@
 #define DP_TEST_SINK			    0x270
 #define DP_TEST_SINK_START	    (1 << 0)
 
+#define DP_PAYLOAD_TABLE_UPDATE_STATUS      0x2c0   /* 1.2 MST */
+# define DP_PAYLOAD_TABLE_UPDATED           (1 << 0)
+# define DP_PAYLOAD_ACT_HANDLED             (1 << 1)
+
+#define DP_VC_PAYLOAD_ID_SLOT_1             0x2c1   /* 1.2 MST */
+/* up to ID_SLOT_63 at 0x2ff */
+
 #define DP_SOURCE_OUI			    0x300
 #define DP_SINK_OUI			    0x400
 #define DP_BRANCH_OUI			    0x500

@@ -303,6 +329,21 @@
 # define DP_SET_POWER_D3                    0x2
 # define DP_SET_POWER_MASK                  0x3
 
+#define DP_SIDEBAND_MSG_DOWN_REQ_BASE	    0x1000   /* 1.2 MST */
+#define DP_SIDEBAND_MSG_UP_REP_BASE	    0x1200   /* 1.2 MST */
+#define DP_SIDEBAND_MSG_DOWN_REP_BASE	    0x1400   /* 1.2 MST */
+#define DP_SIDEBAND_MSG_UP_REQ_BASE	    0x1600   /* 1.2 MST */
+
+#define DP_SINK_COUNT_ESI		    0x2002   /* 1.2 */
+/* 0-5 sink count */
+# define DP_SINK_COUNT_CP_READY             (1 << 6)
+
+#define DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0   0x2003   /* 1.2 */
+
+#define DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1   0x2004   /* 1.2 */
+
+#define DP_LINK_SERVICE_IRQ_VECTOR_ESI0     0x2005   /* 1.2 */
+
 #define DP_PSR_ERROR_STATUS                 0x2006  /* XXX 1.2? */
 # define DP_PSR_LINK_CRC_ERROR              (1 << 0)
 # define DP_PSR_RFB_STORAGE_ERROR           (1 << 1)

@@ -319,6 +360,43 @@
 # define DP_PSR_SINK_INTERNAL_ERROR         7
 # define DP_PSR_SINK_STATE_MASK             0x07
 
+/* DP 1.2 Sideband message defines */
+/* peer device type - DP 1.2a Table 2-92 */
+#define DP_PEER_DEVICE_NONE		0x0
+#define DP_PEER_DEVICE_SOURCE_OR_SST	0x1
+#define DP_PEER_DEVICE_MST_BRANCHING	0x2
+#define DP_PEER_DEVICE_SST_SINK		0x3
+#define DP_PEER_DEVICE_DP_LEGACY_CONV	0x4
+
+/* DP 1.2 MST sideband request names DP 1.2a Table 2-80 */
+#define DP_LINK_ADDRESS			0x01
+#define DP_CONNECTION_STATUS_NOTIFY	0x02
+#define DP_ENUM_PATH_RESOURCES		0x10
+#define DP_ALLOCATE_PAYLOAD		0x11
+#define DP_QUERY_PAYLOAD		0x12
+#define DP_RESOURCE_STATUS_NOTIFY	0x13
+#define DP_CLEAR_PAYLOAD_ID_TABLE	0x14
+#define DP_REMOTE_DPCD_READ		0x20
+#define DP_REMOTE_DPCD_WRITE		0x21
+#define DP_REMOTE_I2C_READ		0x22
+#define DP_REMOTE_I2C_WRITE		0x23
+#define DP_POWER_UP_PHY			0x24
+#define DP_POWER_DOWN_PHY		0x25
+#define DP_SINK_EVENT_NOTIFY		0x30
+#define DP_QUERY_STREAM_ENC_STATUS	0x38
+
+/* DP 1.2 MST sideband nak reasons - table 2.84 */
+#define DP_NAK_WRITE_FAILURE		0x01
+#define DP_NAK_INVALID_READ		0x02
+#define DP_NAK_CRC_FAILURE		0x03
+#define DP_NAK_BAD_PARAM		0x04
+#define DP_NAK_DEFER			0x05
+#define DP_NAK_LINK_FAILURE		0x06
+#define DP_NAK_NO_RESOURCES		0x07
+#define DP_NAK_DPCD_FAIL		0x08
+#define DP_NAK_I2C_NAK			0x09
+#define DP_NAK_ALLOCATE_FAIL		0x0a
+
 #define MODE_I2C_START	1
 #define MODE_I2C_WRITE	2
 #define MODE_I2C_READ	4
Signature · 1 line
1.9.0
author maintainer
#2 lore raw

PATCH 2/8 drm: add DP MST encoder type

In reply to Dave Airlie

From: Dave Airlie <[email protected]>

This adds an encoder type for DP MST encoders.

Signed-off-by Dave Airlie <[email protected]>
---
 drivers/gpu/drm/drm_crtc.c  | 1 +
 include/uapi/drm/drm_mode.h | 1 +
 2 files changed, 2 insertions(+)
drivers/gpu/drm/drm_crtc.c +1 -0
index a3fe324..f1753e6 100644
@@ -227,6 +227,7 @@ static const struct drm_prop_enum_list drm_encoder_enum_list[] =
 	{ DRM_MODE_ENCODER_TVDAC, "TV" },
 	{ DRM_MODE_ENCODER_VIRTUAL, "Virtual" },
 	{ DRM_MODE_ENCODER_DSI, "DSI" },
+	{ DRM_MODE_ENCODER_DPMST, "DP MST" },
 };
 
 static const struct drm_prop_enum_list drm_subpixel_enum_list[] =
include/uapi/drm/drm_mode.h +1 -0
index f104c26..719add4 100644
@@ -181,6 +181,7 @@ struct drm_mode_get_plane_res {
 #define DRM_MODE_ENCODER_TVDAC	4
 #define DRM_MODE_ENCODER_VIRTUAL 5
 #define DRM_MODE_ENCODER_DSI	6
+#define DRM_MODE_ENCODER_DPMST	7
 
 struct drm_mode_get_encoder {
 	__u32 encoder_id;
Signature · 1 line
1.9.0
author maintainer
#3 lore raw

PATCH 3/8 drm/i915: add some registers need for displayport MST support.

In reply to Dave Airlie

From: Dave Airlie <[email protected]>

These are just from the Haswell spec.

Signed-off-by Dave Airlie <[email protected]>
---
 drivers/gpu/drm/i915/i915_reg.h | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/i915_reg.h +9 -2
index 8f84555..557b37a 100644
@@ -5386,6 +5386,7 @@ enum punit_power_well {
 #define  TRANS_DDI_EDP_INPUT_A_ONOFF	(4<<12)
 #define  TRANS_DDI_EDP_INPUT_B_ONOFF	(5<<12)
 #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6<<12)
+#define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC	(1<<8)
 #define  TRANS_DDI_BFI_ENABLE		(1<<4)
 
 /* DisplayPort Transport Control */

@@ -5395,6 +5396,7 @@ enum punit_power_well {
 #define  DP_TP_CTL_ENABLE			(1<<31)
 #define  DP_TP_CTL_MODE_SST			(0<<27)
 #define  DP_TP_CTL_MODE_MST			(1<<27)
+#define  DP_TP_CTL_FORCE_ACT			(1<<25)
 #define  DP_TP_CTL_ENHANCED_FRAME_ENABLE	(1<<18)
 #define  DP_TP_CTL_FDI_AUTOTRAIN		(1<<15)
 #define  DP_TP_CTL_LINK_TRAIN_MASK		(7<<8)

@@ -5409,8 +5411,13 @@ enum punit_power_well {
 #define DP_TP_STATUS_A			0x64044
 #define DP_TP_STATUS_B			0x64144
 #define DP_TP_STATUS(port) _PORT(port, DP_TP_STATUS_A, DP_TP_STATUS_B)
-#define  DP_TP_STATUS_IDLE_DONE		(1<<25)
-#define  DP_TP_STATUS_AUTOTRAIN_DONE	(1<<12)
+#define  DP_TP_STATUS_IDLE_DONE			(1<<25)
+#define  DP_TP_STATUS_ACT_SENT			(1<<24)
+#define  DP_TP_STATUS_MODE_STATUS_MST		(1<<23)
+#define  DP_TP_STATUS_AUTOTRAIN_DONE		(1<<12)
+#define  DP_TP_STATUS_PAYLOAD_MAPPING_VC2	(3 << 8)
+#define  DP_TP_STATUS_PAYLOAD_MAPPING_VC1	(3 << 4)
+#define  DP_TP_STATUS_PAYLOAD_MAPPING_VC0	(3 << 0)
 
 /* DDI Buffer Control */
 #define DDI_BUF_CTL_A				0x64000
Signature · 1 line
1.9.0
author maintainer
#4 lore raw

PATCH 4/8 drm/crtc: add interface to reinitialise the legacy mode group

In reply to Dave Airlie

From: Dave Airlie <[email protected]>

This can be called to update things after dynamic connectors/encoders are created/deleted.

Signed-off-by Dave Airlie <[email protected]>
---
 drivers/gpu/drm/drm_crtc.c | 9 +++++++++
 include/drm/drm_crtc.h     | 1 +
 2 files changed, 10 insertions(+)
drivers/gpu/drm/drm_crtc.c +9 -0
index f1753e6..8bf87a6 100644
@@ -1421,6 +1421,15 @@ int drm_mode_group_init_legacy_group(struct drm_device *dev,
 }
 EXPORT_SYMBOL(drm_mode_group_init_legacy_group);
 
+void drm_reinit_primary_mode_group(struct drm_device *dev)
+{
+	drm_modeset_lock_all(dev);
+	drm_mode_group_destroy(&dev->primary->mode_group);
+	drm_mode_group_init_legacy_group(dev, &dev->primary->mode_group);
+	drm_modeset_unlock_all(dev);
+}
+EXPORT_SYMBOL(drm_reinit_primary_mode_group);
+
 /**
  * drm_crtc_convert_to_umode - convert a drm_display_mode into a modeinfo
  * @out: drm_mode_modeinfo struct to return to the user
include/drm/drm_crtc.h +1 -0
index c6b9e8a..55bc523 100644
@@ -916,6 +916,7 @@ extern const 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 void drm_mode_group_destroy(struct drm_mode_group *group);
+extern void drm_reinit_primary_mode_group(struct drm_device *dev);
 extern bool drm_probe_ddc(struct i2c_adapter *adapter);
 extern struct edid *drm_get_edid(struct drm_connector *connector,
 				 struct i2c_adapter *adapter);
Signature · 1 line
1.9.0
author maintainer
#5 lore raw

PATCH 5/8 drm/helper: add Displayport multi-stream helper

In reply to Dave Airlie

From: Dave Airlie <[email protected]>

This is the initial import of the helper for displayport multistream.

It consists of a topology manager, init/destroy/set mst state

It supports DP 1.2 MST sideband msg protocol handler - via hpd irqs

connector detect and edid retrieval interface.

It supports i2c device over DP 1.2 sideband msg protocol (EDID reads only)

bandwidth manager API via vcpi allocation and payload updating, along with a helper to check the ACT status.

Objects:
MST topology manager - one per toplevel MST capable GPU port - not sure if this should be higher level again MST branch unit - one instance per plugged branching unit - one at top of hierarchy - others hanging from ports MST port - one port per port reported by branching units, can have MST units hanging from them as well.

TODO:
cleanup
locking review
fix known bug with UP message when an MST device is connected to an MST device.

Signed-off-by Dave Airlie <[email protected]>
---
 drivers/gpu/drm/Makefile              |    2 +-
 drivers/gpu/drm/drm_dp_mst_topology.c | 2081 +++++++++++++++++++++++++++++++++
 include/drm/drm_dp_mst_helper.h       |  403 +++++++
 3 files changed, 2485 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/drm_dp_mst_topology.c
 create mode 100644 include/drm/drm_dp_mst_helper.h
drivers/gpu/drm/Makefile +1 -1
index 48e38ba..712b73e 100644
@@ -23,7 +23,7 @@ drm-$(CONFIG_DRM_PANEL) += drm_panel.o
 
 drm-usb-y   := drm_usb.o
 
-drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o
+drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o drm_dp_mst_topology.o
 drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
 drm_kms_helper-$(CONFIG_DRM_KMS_FB_HELPER) += drm_fb_helper.o
 drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
drivers/gpu/drm/drm_dp_mst_topology.c new file +2081 -0

2,081 lines not on this page · raw message

include/drm/drm_dp_mst_helper.h new file +403 -0

403 lines not on this page · raw message

Signature · 6 lines
1.9.0

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
author maintainer
#6 lore raw

PATCH 6/8 i915: split some DP modesetting code into a separate function

In reply to Dave Airlie

From: Dave Airlie <[email protected]>

this is just prep work for mst support.

Signed-off-by Dave Airlie <[email protected]>
---
 drivers/gpu/drm/i915/intel_ddi.c | 20 +++++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 2 files changed, 14 insertions(+), 7 deletions(-)
drivers/gpu/drm/i915/intel_ddi.c +13 -7
index 0ad4e96..a5b8b76 100644
@@ -364,6 +364,18 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
 	DRM_ERROR("FDI link training failed!\n");
 }
 
+void intel_ddi_mode_set_dp(struct intel_encoder *encoder)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+	struct intel_digital_port *intel_dig_port =
+		enc_to_dig_port(&encoder->base);
+
+	intel_dp->DP = intel_dig_port->saved_port_bits |
+		DDI_BUF_CTL_ENABLE | DDI_BUF_EMP_400MV_0DB_HSW;
+	intel_dp->DP |= DDI_PORT_WIDTH(intel_dp->lane_count);
+
+}
+
 static void intel_ddi_mode_set(struct intel_encoder *encoder)
 {
 	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);

@@ -378,13 +390,7 @@ static void intel_ddi_mode_set(struct intel_encoder *encoder)
 	crtc->eld_vld = false;
 	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
-		struct intel_digital_port *intel_dig_port =
-			enc_to_dig_port(&encoder->base);
-
-		intel_dp->DP = intel_dig_port->saved_port_bits |
-			       DDI_BUF_CTL_ENABLE | DDI_BUF_EMP_400MV_0DB_HSW;
-		intel_dp->DP |= DDI_PORT_WIDTH(intel_dp->lane_count);
-
+		intel_ddi_mode_set_dp(encoder);
 		if (intel_dp->has_audio) {
 			DRM_DEBUG_DRIVER("DP audio on pipe %c on DDI\n",
 					 pipe_name(crtc->pipe));
drivers/gpu/drm/i915/intel_drv.h +1 -0
index b885df1..8e41cdc 100644
@@ -683,6 +683,7 @@ void intel_ddi_fdi_disable(struct drm_crtc *crtc);
 void intel_ddi_get_config(struct intel_encoder *encoder,
 			  struct intel_crtc_config *pipe_config);
 
+void intel_ddi_mode_set_dp(struct intel_encoder *encoder);
 
 /* intel_display.c */
 const char *intel_output_name(int output);
Signature · 1 line
1.9.0
author maintainer
#7 lore raw

PATCH 7/8 drm/i915: check connector->encoder before using it.

In reply to Dave Airlie

From: Dave Airlie <[email protected]>

DP MST will need connectors that aren't connected to specific encoders, add some checks in advance to avoid oopses.

Signed-off-by Dave Airlie <[email protected]>
---
 drivers/gpu/drm/i915/i915_debugfs.c  | 16 +++++++++-------
 drivers/gpu/drm/i915/i915_irq.c      |  4 ++++
 drivers/gpu/drm/i915/intel_display.c | 25 ++++++++++++++-----------
 3 files changed, 27 insertions(+), 18 deletions(-)
drivers/gpu/drm/i915/i915_debugfs.c +9 -7
index 1e83ae4..88e944f 100644
@@ -2279,13 +2279,15 @@ static void intel_connector_info(struct seq_file *m,
 		seq_printf(m, "\tCEA rev: %d\n",
 			   connector->display_info.cea_rev);
 	}
-	if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
-	    intel_encoder->type == INTEL_OUTPUT_EDP)
-		intel_dp_info(m, intel_connector);
-	else if (intel_encoder->type == INTEL_OUTPUT_HDMI)
-		intel_hdmi_info(m, intel_connector);
-	else if (intel_encoder->type == INTEL_OUTPUT_LVDS)
-		intel_lvds_info(m, intel_connector);
+	if (intel_encoder) {
+		if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
+		    intel_encoder->type == INTEL_OUTPUT_EDP)
+			intel_dp_info(m, intel_connector);
+		else if (intel_encoder->type == INTEL_OUTPUT_HDMI)
+			intel_hdmi_info(m, intel_connector);
+		else if (intel_encoder->type == INTEL_OUTPUT_LVDS)
+			intel_lvds_info(m, intel_connector);
+	}
 
 	seq_printf(m, "\tmodes:\n");
 	list_for_each_entry(mode, &connector->modes, head)
drivers/gpu/drm/i915/i915_irq.c +4 -0
index afa5519..5852dee 100644
@@ -1016,6 +1016,8 @@ static void i915_hotplug_work_func(struct work_struct *work)
 	dev_priv->hpd_event_bits = 0;
 	list_for_each_entry(connector, &mode_config->connector_list, head) {
 		intel_connector = to_intel_connector(connector);
+		if (!intel_connector->encoder)
+			continue;
 		intel_encoder = intel_connector->encoder;
 		if (intel_encoder->hpd_pin > HPD_NONE &&
 		    dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == HPD_MARK_DISABLED &&

@@ -1046,6 +1048,8 @@ static void i915_hotplug_work_func(struct work_struct *work)
 
 	list_for_each_entry(connector, &mode_config->connector_list, head) {
 		intel_connector = to_intel_connector(connector);
+		if (!intel_connector->encoder)
+			continue;
 		intel_encoder = intel_connector->encoder;
 		if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
 			if (intel_encoder->hot_plug)
drivers/gpu/drm/i915/intel_display.c +14 -11
index b39d036..75b2aaf 100644
@@ -4600,20 +4600,23 @@ static void intel_connector_check_state(struct intel_connector *connector)
 		     "wrong connector dpms state\n");
 		WARN(connector->base.encoder != &encoder->base,
 		     "active connector not linked to encoder\n");
-		WARN(!encoder->connectors_active,
-		     "encoder->connectors_active not set\n");
 
-		encoder_enabled = encoder->get_hw_state(encoder, &pipe);
-		WARN(!encoder_enabled, "encoder not enabled\n");
-		if (WARN_ON(!encoder->base.crtc))
-			return;
+		if (encoder) {
+			WARN(!encoder->connectors_active,
+			     "encoder->connectors_active not set\n");
+
+			encoder_enabled = encoder->get_hw_state(encoder, &pipe);
+			WARN(!encoder_enabled, "encoder not enabled\n");
+			if (WARN_ON(!encoder->base.crtc))
+				return;
 
-		crtc = encoder->base.crtc;
+			crtc = encoder->base.crtc;
 
-		WARN(!crtc->enabled, "crtc not enabled\n");
-		WARN(!to_intel_crtc(crtc)->active, "crtc not active\n");
-		WARN(pipe != to_intel_crtc(crtc)->pipe,
-		     "encoder active on the wrong pipe\n");
+			WARN(!crtc->enabled, "crtc not enabled\n");
+			WARN(!to_intel_crtc(crtc)->active, "crtc not active\n");
+			WARN(pipe != to_intel_crtc(crtc)->pipe,
+			     "encoder active on the wrong pipe\n");
+		}
 	}
 }
Signature · 1 line
1.9.0
author maintainer
#8 lore raw

PATCH 8/8 i915: add DP 1.2 MST support (v0.1)

In reply to Dave Airlie

From: Dave Airlie <[email protected]>

This adds DP 1.2 MST support on Haswell systems.

Notes:
a) this reworks irq handling for DP MST ports, so that we can avoid the mode config locking in the current hpd handlers, as we need to process up/down msgs at a better time.

b) it introduces a new MST output type.

c) it really annoys the state checker, as the connector that is connected at the start, goes into disconnected, and its encoder it active, and things blow up, need to look into how to solve that.

d) the intel userspace driver needs changes to use this, -modesetting should work okay.

e) it might contain a race condition big enough to eat you.

f) its hardcoded to 4 lanes 5.4Ghz. - TODO.

TODO:
write TODOs.

Signed-off-by Dave Airlie <[email protected]>
---
 drivers/gpu/drm/i915/Makefile        |   1 +
 drivers/gpu/drm/i915/i915_drv.h      |   5 +
 drivers/gpu/drm/i915/i915_irq.c      | 105 +++++++-
 drivers/gpu/drm/i915/intel_ddi.c     | 106 ++++++--
 drivers/gpu/drm/i915/intel_display.c |  56 ++--
 drivers/gpu/drm/i915/intel_dp.c      | 174 +++++++++++-
 drivers/gpu/drm/i915/intel_dp_mst.c  | 502 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  39 ++-
 8 files changed, 942 insertions(+), 46 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_dp_mst.c
drivers/gpu/drm/i915/Makefile +1 -0
index b1445b7..6cf2b95 100644
@@ -52,6 +52,7 @@ i915-y += dvo_ch7017.o \
 	  intel_crt.o \
 	  intel_ddi.o \
 	  intel_dp.o \
+	  intel_dp_mst.o \
 	  intel_dsi_cmd.o \
 	  intel_dsi.o \
 	  intel_dsi_pll.o \
drivers/gpu/drm/i915/i915_drv.h +5 -0
index 7d6acb4..36a5c78 100644
@@ -1468,6 +1468,9 @@ struct drm_i915_private {
 
 	struct i915_runtime_pm pm;
 
+	struct intel_digital_port *hpd_irq_port[I915_MAX_PORTS];
+	u32 hpd_port;
+	struct work_struct dig_port_work;
 	/* Old dri1 support infrastructure, beware the dragons ya fools entering
 	 * here! */
 	struct i915_dri1_state dri1;

@@ -1999,6 +2002,8 @@ i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
 
 void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv);
 void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv);
+bool intel_hpd_irq_event(struct drm_device *dev,
+			 struct drm_connector *connector);
 
 /* i915_gem.c */
 int i915_gem_init_ioctl(struct drm_device *dev, void *data,
drivers/gpu/drm/i915/i915_irq.c +101 -4

154 lines not on this page · raw message

drivers/gpu/drm/i915/intel_ddi.c +84 -22

175 lines not on this page · raw message

drivers/gpu/drm/i915/intel_display.c +40 -16

98 lines not on this page · raw message

drivers/gpu/drm/i915/intel_dp.c +171 -3

243 lines not on this page · raw message

drivers/gpu/drm/i915/intel_dp_mst.c new file +502 -0

502 lines not on this page · raw message

drivers/gpu/drm/i915/intel_drv.h +38 -1
index 8e41cdc..98bb133 100644
@@ -32,7 +32,7 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_helper.h>
-#include <drm/drm_dp_helper.h>
+#include <drm/drm_dp_mst_helper.h>
 
 /**
  * _wait_for - magic (register) wait macro

@@ -100,6 +100,7 @@
 #define INTEL_OUTPUT_EDP 8
 #define INTEL_OUTPUT_DSI 9
 #define INTEL_OUTPUT_UNKNOWN 10
+#define INTEL_OUTPUT_DP_MST 11
 
 #define INTEL_DVO_CHIP_NONE 0
 #define INTEL_DVO_CHIP_LVDS 1

@@ -207,6 +208,9 @@ struct intel_connector {
 	/* since POLL and HPD connectors may use the same HPD line keep the native
 	   state of connector->polled in case hotplug storm detection changes it */
 	u8 polled;
+
+	struct drm_dp_mst_port *port;
+	struct intel_dp *mst_port;
 };
 
 typedef struct dpll {

@@ -340,6 +344,8 @@ struct intel_crtc_config {
 	bool ips_enabled;
 
 	bool double_wide;
+
+	bool dp_encoder_is_mst;
 };
 
 struct intel_pipe_wm {

@@ -487,6 +493,7 @@ struct intel_hdmi {
 			       struct drm_display_mode *adjusted_mode);
 };
 
+struct intel_dp_mst_encoder;
 #define DP_MAX_DOWNSTREAM_PORTS		0x10
 
 /**

@@ -527,8 +534,17 @@ struct intel_dp {
 	unsigned long last_backlight_off;
 	bool psr_setup_done;
 	bool use_tps3;
+	bool can_mst; /* this port supports mst */
+	bool is_mst;
+	int active_mst_links;
+	/* connector directly attached - won't be use for modeset in mst world */
 	struct intel_connector *attached_connector;
 
+	/* mst connector list */
+	struct intel_connector *mst_connectors;
+	struct intel_dp_mst_encoder *mst_encoders[I915_MAX_PIPES];
+	struct drm_dp_mst_topology_mgr mst_mgr;
+
 	uint32_t (*get_aux_clock_divider)(struct intel_dp *dp, int index);
 	/*
 	 * This function returns the value we have to program the AUX_CTL

@@ -554,6 +570,12 @@ struct intel_digital_port {
 	struct intel_hdmi hdmi;
 };
 
+struct intel_dp_mst_encoder {
+	struct intel_encoder base;
+	enum pipe pipe;
+	struct intel_digital_port *primary;
+};
+
 static inline int
 vlv_dport_to_channel(struct intel_digital_port *dport)
 {

@@ -621,6 +643,12 @@ enc_to_dig_port(struct drm_encoder *encoder)
 	return container_of(encoder, struct intel_digital_port, base.base);
 }
 
+static inline struct intel_dp_mst_encoder *
+enc_to_mst(struct drm_encoder *encoder)
+{
+	return container_of(encoder, struct intel_dp_mst_encoder, base.base);
+}
+
 static inline struct intel_dp *enc_to_intel_dp(struct drm_encoder *encoder)
 {
 	return &enc_to_dig_port(encoder)->dp;

@@ -684,6 +712,10 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 			  struct intel_crtc_config *pipe_config);
 
 void intel_ddi_mode_set_dp(struct intel_encoder *encoder);
+void intel_ddi_clock_get(struct intel_encoder *encoder,
+			 struct intel_crtc_config *pipe_config);
+void intel_ddi_force_act(struct intel_encoder *encoder, bool state);
+void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
 
 /* intel_display.c */
 const char *intel_output_name(int output);

@@ -800,6 +832,11 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp);
 void intel_edp_psr_update(struct drm_device *dev);
 void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate);
 
+int intel_dp_handle_hpd_irq(struct intel_digital_port *digport, bool long_hpd);
+void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector);
+
+/* intel_dp_mst.c */
+bool intel_dp_mst_init_encoder(struct intel_digital_port *intel_dig_port);
 /* intel_dsi.c */
 bool intel_dsi_init(struct drm_device *dev);
Signature · 6 lines
1.9.0

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
author maintainer
#9 lore raw
In reply to Dave Airlie

TODO: fix fbcon

I forgot to mention no fbcon yet :-)

Dave.

Quoted 25 lines · Dave Airlie

Hi,

Branch: http://cgit.freedesktop.org/~airlied/linux/log/?h=drm-i915-mst-support

Okay I've gotten this to light up some monitors using -modesetting so it must be ready for an initial posting,

Tested on Haswell so far with Lenovo Ultradock and with Ancell MST Hub devices, (checked it can detect the Dell 32" Monitor - but that is another world of pain).

The main problem is documented in the i915 patch, state check backtraces abound.

There is still quite a bit of work todo for a compliant solution, but I don't have a compliance tester so I can only imagine the fail.

The helper interface can probably be cleaned up, the whole ACT area is bit of a mess, and I'd probably need an idea from other GPUs how best to do that,

Dave.

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

#10 lore raw
In reply to Dave Airlie
Quoted 6 lines · Dave Airlie

Hi,

Branch: http://cgit.freedesktop.org/~airlied/linux/log/?h=drm-i915-mst-support

Okay I've gotten this to light up some monitors using -modesetting so it must be ready for an initial posting,

I see that connectors are added/removed dynamically based on branch discovery, but I haven't spotted how that is communicated to userspace. I was expecting to see a new uevent. Or do you simply want userspace to update it lists of connectors on every hotplug event? Is there anyway to retrieve the guid so that we can associate the same XID to each branch device for the server lifetime? (Is that even a sane idea?) Can we detect when a DP dongle is present splitting DVI/VGA using MST but since they use different encoders they already falsely appear as unique connectors to i915.ko?
-Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
author maintainer
#11 lore raw
Quoted 8 lines · Chris Wilson
Quoted 6 lines · Dave Airlie

Hi,

Branch: http://cgit.freedesktop.org/~airlied/linux/log/?h=drm-i915-mst-support

Okay I've gotten this to light up some monitors using -modesetting so it must be ready for an initial posting,

Quoted 9 lines

I see that connectors are added/removed dynamically based on branch discovery, but I haven't spotted how that is communicated to userspace. I was expecting to see a new uevent. Or do you simply want userspace to update it lists of connectors on every hotplug event? Is there anyway to retrieve the guid so that we can associate the same XID to each branch device for the server lifetime? (Is that even a sane idea?) Can we detect when a DP dongle is present splitting DVI/VGA using MST but since they use different encoders they already falsely appear as unique connectors to i915.ko?

I hadn't considered a new hotplug event but I suppose one makes sense for this actually,
however userspace will still get old school hotplug events anyways as the connectors transition to connected state.

I'm not sure how much overhead userspace would have calling drmGetResources on every hotplug it certainly shouldn't be a common event,

the GUID is only on DP 1.2 devices, so you don't get one for ever port, also GUIDs are wiped on powerdown on most devices, default GUID is 0 except where devices have USB hubs as well, so it probably doesn't make much sense to bother exposing them directly.

I'm not sure about the last question, MST is always active DP conversion, each port the protocol advertises will appear as a connector, the protocol doesn't offer much more detail, like my dock has a DP port and a HDMI port shared on one DP port, depending on what is plugged in the port is configured different, but it appears as the same connector.

Dave.

#12 lore raw
Quoted 19 lines · Dave Airlie
Quoted 8 lines · Chris Wilson
Quoted 6 lines · Dave Airlie

Hi,

Branch: http://cgit.freedesktop.org/~airlied/linux/log/?h=drm-i915-mst-support

Okay I've gotten this to light up some monitors using -modesetting so it must be ready for an initial posting,

Quoted 9 lines

I see that connectors are added/removed dynamically based on branch discovery, but I haven't spotted how that is communicated to userspace. I was expecting to see a new uevent. Or do you simply want userspace to update it lists of connectors on every hotplug event? Is there anyway to retrieve the guid so that we can associate the same XID to each branch device for the server lifetime? (Is that even a sane idea?) Can we detect when a DP dongle is present splitting DVI/VGA using MST but since they use different encoders they already falsely appear as unique connectors to i915.ko?

Quoted 8 lines

I hadn't considered a new hotplug event but I suppose one makes sense for this actually,
however userspace will still get old school hotplug events anyways as the connectors transition to connected state.

I'm not sure how much overhead userspace would have calling drmGetResources on every hotplug it certainly shouldn't be a common event,

I realised that GETRESOURCES is not too heavy later. I was thinking that the maintainance of the X state would be more troublesome, but it doesn't appear so. I still think having a separate event type for branch changes will be useful, if only for the purposes of diagnostics.

Quoted 4 lines

the GUID is only on DP 1.2 devices, so you don't get one for ever port, also GUIDs are wiped on powerdown on most devices, default GUID is 0 except where devices have USB hubs as well, so it probably doesn't make much sense to bother exposing them directly.

Ok. It looks like if we do attempt to maintain persistent naming, we need to do it in the kernel anyway. That is to make sure that a downstream device always has the same type-id upon reconnection - at least for the lifetime of module. Or maybe the output name is irrelevant for preserving extended desktop configurations?

Quoted 6 lines

I'm not sure about the last question, MST is always active DP conversion, each port the protocol advertises will appear as a connector, the protocol doesn't offer much more detail, like my dock has a DP port and a HDMI port shared on one DP port, depending on what is plugged in the port is configured different, but it appears as the same connector.

Hmm. I think I am confused. I thought we had a recent report of a DP-to-VGA/DVI splitter that on Windows you could drive the VGA + DVI separately, but on Linux they could only be cloned. That sounds like MST. However, I am pretty sure the xrandr was reporting VGA and HDMI outputs (behind the splitter) that the user had some control over. I am not too sure if I have the details right though.
-Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
#13 lore raw
Quoted 8 lines · Chris Wilson
Quoted 6 lines · Dave Airlie
Quoted 1 line · Chris Wilson

On Fri, May 02, 2014 at 02:39:37PM +1000, Dave Airlie wrote:

the GUID is only on DP 1.2 devices, so you don't get one for ever port, also GUIDs are wiped on powerdown on most devices, default GUID is 0 except where devices have USB hubs as well, so it probably doesn't make much sense to bother exposing them directly.

Quoted 5 lines

Ok. It looks like if we do attempt to maintain persistent naming, we need to do it in the kernel anyway. That is to make sure that a downstream device always has the same type-id upon reconnection - at least for the lifetime of module. Or maybe the output name is irrelevant for preserving extended desktop configurations?

Dunno if it helps, but for roughly similar reasons we ended up naming the outputs based on their topology paths in the NVIDIA driver. So for example a port named DP-3 that has a Dell UP2414Q attached will show up as two outputs named DP-3.1 and DP-3.8 since its internal bridge uses downstream ports 1 and 8. This has worked out fairly well in practice.

Here's how I described it in the README:

When DisplayPort 1.2 branch devices are present, display devices will be created with type- and connector-based names that are based on how they are connected to the branch device tree. For example, if a connector named DP-2 has a branch device attached and a DisplayPort device is connected to the branch device's first downstream port, a display device named DP-2.1 might be created. If another branch device is connected between the first branch device and the display device, the name might be DP-2.1.1.

To avoid cluttering the output list, DisplayPort 1.2 devices can be deleted when they are no longer connected and are not named in any MetaModes. This behavior can be enabled with the DeleteUnusedDP12Displays option.

http://us.download.nvidia.com/XFree86/Linux-x86/337.19/README/displaydevicenames.html

Signature · 1 line
Aaron
#14 lore raw
Quoted 33 lines · Aaron Plattner
Quoted 8 lines · Chris Wilson
Quoted 6 lines · Dave Airlie
Quoted 1 line · Chris Wilson

On Fri, May 02, 2014 at 02:39:37PM +1000, Dave Airlie wrote:

the GUID is only on DP 1.2 devices, so you don't get one for ever port, also GUIDs are wiped on powerdown on most devices, default GUID is 0 except where devices have USB hubs as well, so it probably doesn't make much sense to bother exposing them directly.

Quoted 5 lines

Ok. It looks like if we do attempt to maintain persistent naming, we need to do it in the kernel anyway. That is to make sure that a downstream device always has the same type-id upon reconnection - at least for the lifetime of module. Or maybe the output name is irrelevant for preserving extended desktop configurations?

Dunno if it helps, but for roughly similar reasons we ended up naming the outputs based on their topology paths in the NVIDIA driver. So for example a port named DP-3 that has a Dell UP2414Q attached will show up as two outputs named DP-3.1 and DP-3.8 since its internal bridge uses downstream ports 1 and 8. This has worked out fairly well in practice.

Here's how I described it in the README:

When DisplayPort 1.2 branch devices are present, display devices will be created with type- and connector-based names that are based on how they are connected to the branch device tree. For example, if a connector named DP-2 has a branch device attached and a DisplayPort device is connected to the branch device's first downstream port, a display device named DP-2.1 might be created. If another branch device is connected between the first branch device and the display device, the name might be DP-2.1.1.

Quoted 6 lines

To avoid cluttering the output list, DisplayPort 1.2 devices can be deleted when they are no longer connected and are not named in any MetaModes. This behavior can be enabled with the DeleteUnusedDP12Displays option.

http://us.download.nvidia.com/XFree86/Linux-x86/337.19/README/displaydevicenames.html

I'm unclear how you name non-DP branch devices. Do they show up as DP-2.1-HDMI or something similar? Or do you just keep the DP-2.1 name but set the connector type to HDMI?

In general I think your prosoal of adding branch downstream ports to name MST connectors sounds really good. The kms object ids will still be random, but at least if users connect the same topology to the same ports we should have stable names.

And of course userspace can still check the EDID serial. -Daniel

Signature · 3 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
#15 lore raw
Quoted 39 lines · Daniel Vetter
Quoted 38 lines · Aaron Plattner
Quoted 8 lines · Chris Wilson
Quoted 6 lines · Dave Airlie
Quoted 1 line · Chris Wilson

On Fri, May 02, 2014 at 02:39:37PM +1000, Dave Airlie wrote:

the GUID is only on DP 1.2 devices, so you don't get one for ever port, also GUIDs are wiped on powerdown on most devices, default GUID is 0 except where devices have USB hubs as well, so it probably doesn't make much sense to bother exposing them directly.

Quoted 5 lines

Ok. It looks like if we do attempt to maintain persistent naming, we need to do it in the kernel anyway. That is to make sure that a downstream device always has the same type-id upon reconnection - at least for the lifetime of module. Or maybe the output name is irrelevant for preserving extended desktop configurations?

Dunno if it helps, but for roughly similar reasons we ended up naming the outputs based on their topology paths in the NVIDIA driver. So for example a port named DP-3 that has a Dell UP2414Q attached will show up as two outputs named DP-3.1 and DP-3.8 since its internal bridge uses downstream ports 1 and 8. This has worked out fairly well in practice.

Here's how I described it in the README:

When DisplayPort 1.2 branch devices are present, display devices will be created with type- and connector-based names that are based on how they are connected to the branch device tree. For example, if a connector named DP-2 has a branch device attached and a DisplayPort device is connected to the branch device's first downstream port, a display device named DP-2.1 might be created. If another branch device is connected between the first branch device and the display device, the name might be DP-2.1.1.

To avoid cluttering the output list, DisplayPort 1.2 devices can be deleted when they are no longer connected and are not named in any MetaModes. This behavior can be enabled with the DeleteUnusedDP12Displays option.

Quoted 5 lines
Quoted 1 line

http://us.download.nvidia.com/XFree86/Linux-x86/337.19/README/displaydevicenames.html

I'm unclear how you name non-DP branch devices. Do they show up as DP-2.1-HDMI or something similar? Or do you just keep the DP-2.1 name but set the connector type to HDMI?

I don't have any branches that support passive adapters on downstream ports, so I'm not sure what happens with those.

Quoted 4 lines

In general I think your prosoal of adding branch downstream ports to name MST connectors sounds really good. The kms object ids will still be random, but at least if users connect the same topology to the same ports we should have stable names.

Yeah, the RandR output XIDs work the same way.

Quoted 2 lines

And of course userspace can still check the EDID serial. -Daniel

author maintainer
#16 lore raw
In reply to Daniel Vetter
Quoted 45 lines · Daniel Vetter
Quoted 33 lines · Aaron Plattner
Quoted 8 lines · Chris Wilson
Quoted 6 lines · Dave Airlie
Quoted 1 line · Chris Wilson

On Fri, May 02, 2014 at 02:39:37PM +1000, Dave Airlie wrote:

the GUID is only on DP 1.2 devices, so you don't get one for ever port, also GUIDs are wiped on powerdown on most devices, default GUID is 0 except where devices have USB hubs as well, so it probably doesn't make much sense to bother exposing them directly.

Quoted 5 lines

Ok. It looks like if we do attempt to maintain persistent naming, we need to do it in the kernel anyway. That is to make sure that a downstream device always has the same type-id upon reconnection - at least for the lifetime of module. Or maybe the output name is irrelevant for preserving extended desktop configurations?

Dunno if it helps, but for roughly similar reasons we ended up naming the outputs based on their topology paths in the NVIDIA driver. So for example a port named DP-3 that has a Dell UP2414Q attached will show up as two outputs named DP-3.1 and DP-3.8 since its internal bridge uses downstream ports 1 and 8. This has worked out fairly well in practice.

Here's how I described it in the README:

When DisplayPort 1.2 branch devices are present, display devices will be created with type- and connector-based names that are based on how they are connected to the branch device tree. For example, if a connector named DP-2 has a branch device attached and a DisplayPort device is connected to the branch device's first downstream port, a display device named DP-2.1 might be created. If another branch device is connected between the first branch device and the display device, the name might be DP-2.1.1.

Quoted 6 lines

To avoid cluttering the output list, DisplayPort 1.2 devices can be deleted when they are no longer connected and are not named in any MetaModes. This behavior can be enabled with the DeleteUnusedDP12Displays option.

http://us.download.nvidia.com/XFree86/Linux-x86/337.19/README/displaydevicenames.html

I'm unclear how you name non-DP branch devices. Do they show up as DP-2.1-HDMI or something similar? Or do you just keep the DP-2.1 name but set the connector type to HDMI?

Quoted 6 lines

In general I think your prosoal of adding branch downstream ports to name MST connectors sounds really good. The kms object ids will still be random, but at least if users connect the same topology to the same ports we should have stable names.

And of course userspace can still check the EDID serial.

I'm going to just add a property with the path and the userspace driver can reconstruct a name from that
property, seems like the best plan,

Also for non-DP outputs, you don't know what they are, at least on dock I have I can tell if I have
a legacy or a DP device plugged in to a port, but nothing else about it. When nothing is plugged in
the port isn't connected.

So on the dock there is a VGA port, HDMI and DP shared, DVI-D and DP shared, I see 3 ports
in the MST protocol, until something gets plugged in to something I get no more info.

Dave.

author maintainer
#19 lore raw
Quoted 8 lines · Chris Wilson
Quoted 3 lines · Dave Airlie

I've pushed a branch of -modesetting to

http://cgit.freedesktop.org/~airlied/xf86-video-modesetting/log/?h=dp-mst

In parse_path_blob():
char conn_id[5];

Quoted 6 lines

len = conn - (blob_data + 4);
if (len + 1> 5) return -1;

memcpy(conn_id, blob_data + 4, len);

conn_id[len + 1] = '\0';
        ^^^^^^^ should just be len.

Nice catch, thanks, I'll fix that,

The kernel side for this along with even more locking and refcounting fixes is in
http://cgit.freedesktop.org/~airlied/linux/log/?h=drm-i915-mst-support

Dave.

author maintainer
#20 lore raw
In reply to Aaron Plattner
Quoted 39 lines · Aaron Plattner
Quoted 13 lines · Chris Wilson
Quoted 9 lines · Dave Airlie
Quoted 2 lines · Chris Wilson

On Fri, May 02, 2014 at 02:39:37PM +1000, Dave Airlie wrote:

the GUID is only on DP 1.2 devices, so you don't get one for ever port, also GUIDs are wiped on powerdown on most devices, default GUID is 0 except where devices have USB hubs as well, so it probably doesn't make much sense to bother exposing them directly.

Quoted 5 lines

Ok. It looks like if we do attempt to maintain persistent naming, we need to do it in the kernel anyway. That is to make sure that a downstream device always has the same type-id upon reconnection - at least for the lifetime of module. Or maybe the output name is irrelevant for preserving extended desktop configurations?

Dunno if it helps, but for roughly similar reasons we ended up naming the outputs based on their topology paths in the NVIDIA driver. So for example a port named DP-3 that has a Dell UP2414Q attached will show up as two outputs named DP-3.1 and DP-3.8 since its internal bridge uses downstream ports 1 and 8. This has worked out fairly well in practice.

Here's how I described it in the README:

When DisplayPort 1.2 branch devices are present, display devices will be created with type- and connector-based names that are based on how they are connected to the branch device tree. For example, if a connector named DP-2 has a branch device attached and a DisplayPort device is connected to the branch device's first downstream port, a display device named DP-2.1 might be created. If another branch device is connected between the first branch device and the display device, the name might be DP-2.1.1.

Quoted 6 lines

To avoid cluttering the output list, DisplayPort 1.2 devices can be deleted when they are no longer connected and are not named in any MetaModes. This behavior can be enabled with the DeleteUnusedDP12Displays option.

http://us.download.nvidia.com/XFree86/Linux-x86/337.19/README/displaydevicenames.html

Is the cleaning up an option because it caused some problems?

I'm seeing some gnome-settings-daemon, gnome-shell crash because the XIDs are gone away and they get X errors, just wondering if this is what you were seeing,

Dave.

#21 lore raw
Quoted 45 lines · Dave Airlie
Quoted 44 lines · Aaron Plattner
Quoted 13 lines · Chris Wilson
Quoted 9 lines · Dave Airlie
Quoted 2 lines · Chris Wilson

On Fri, May 02, 2014 at 02:39:37PM +1000, Dave Airlie wrote:

the GUID is only on DP 1.2 devices, so you don't get one for ever port, also GUIDs are wiped on powerdown on most devices, default GUID is 0 except where devices have USB hubs as well, so it probably doesn't make much sense to bother exposing them directly.

Quoted 5 lines

Ok. It looks like if we do attempt to maintain persistent naming, we need to do it in the kernel anyway. That is to make sure that a downstream device always has the same type-id upon reconnection - at least for the lifetime of module. Or maybe the output name is irrelevant for preserving extended desktop configurations?

Dunno if it helps, but for roughly similar reasons we ended up naming the outputs based on their topology paths in the NVIDIA driver. So for example a port named DP-3 that has a Dell UP2414Q attached will show up as two outputs named DP-3.1 and DP-3.8 since its internal bridge uses downstream ports 1 and 8. This has worked out fairly well in practice.

Here's how I described it in the README:

When DisplayPort 1.2 branch devices are present, display devices will be created with type- and connector-based names that are based on how they are connected to the branch device tree. For example, if a connector named DP-2 has a branch device attached and a DisplayPort device is connected to the branch device's first downstream port, a display device named DP-2.1 might be created. If another branch device is connected between the first branch device and the display device, the name might be DP-2.1.1.

To avoid cluttering the output list, DisplayPort 1.2 devices can be deleted when they are no longer connected and are not named in any MetaModes. This behavior can be enabled with the DeleteUnusedDP12Displays option.

Quoted 7 lines
Quoted 1 line

http://us.download.nvidia.com/XFree86/Linux-x86/337.19/README/displaydevicenames.html

Is the cleaning up an option because it caused some problems?

I'm seeing some gnome-settings-daemon, gnome-shell crash because the XIDs are gone away and they get X errors, just wondering if this is what you were seeing,

Yeah, this exactly. RandR's timestamp mechanism doesn't help you avoid BadOutput errors, although we could probably extend the protocol to add that, if we decide it's necessary.

Since the topology-based IDs are relatively stable unless you get a whole pile of branch devices and swap them around in weird ways all day, it was easier to just leave them around. They'll get reused if a device with that topology path ever comes back.

Quoted 1 line

Dave.