All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Fixes for MST (daisy-chain and 4k tiles)
@ 2016-01-22 22:07 Harry Wentland
  2016-01-22 22:07 ` [PATCH 1/5] drm: Add drm_fixp_from_fraction and drm_fixp2int_ceil Harry Wentland
                   ` (5 more replies)
  0 siblings, 6 replies; 22+ messages in thread
From: Harry Wentland @ 2016-01-22 22:07 UTC (permalink / raw)
  To: dri-devel

A couple of MST fixes to bugs in the framework that we encountered when
testing with 
  - three-display daisy-chain configurations
  - 4k tiled displays

Andrey Grodzovsky (1):
  drm/dp/mst: Reverse order of MST enable and clearing VC payload table.

Harry Wentland (2):
  drm: Add drm_fixp_from_fraction and drm_fixp2int_ceil
  drm/dp/mst: Calculate MST PBN with 31.32 fixed point

Hersen Wu (1):
  drm/dp/mst: move GUID storage from mgr, port to only mst branch

Mykola Lysenko (1):
  drm/dp/mst: change MST detection scheme

 drivers/gpu/drm/drm_dp_mst_topology.c | 180 +++++++++++++++++-----------------
 include/drm/drm_dp_mst_helper.h       |  25 ++---
 include/drm/drm_fixed.h               |  54 +++++++++-
 3 files changed, 154 insertions(+), 105 deletions(-)

-- 
2.1.4

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/5] drm: Add drm_fixp_from_fraction and drm_fixp2int_ceil
  2016-01-22 22:07 [PATCH 0/5] Fixes for MST (daisy-chain and 4k tiles) Harry Wentland
@ 2016-01-22 22:07 ` Harry Wentland
  2016-01-22 22:07 ` [PATCH 2/5] drm/dp/mst: Calculate MST PBN with 31.32 fixed point Harry Wentland
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 22+ messages in thread
From: Harry Wentland @ 2016-01-22 22:07 UTC (permalink / raw)
  To: dri-devel

drm_fixp_from_fraction allows us to create a fixed point directly
from a fraction, rather than creating fixed point values and dividing
later. This avoids overflow of our 64 bit value for large numbers.

drm_fixp2int_ceil allows us to return the ceiling of our fixed point
value.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 include/drm/drm_fixed.h | 54 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/include/drm/drm_fixed.h b/include/drm/drm_fixed.h
index d639049a613d..16e725c42422 100644
--- a/include/drm/drm_fixed.h
+++ b/include/drm/drm_fixed.h
@@ -73,18 +73,28 @@ static inline u32 dfixed_div(fixed20_12 A, fixed20_12 B)
 #define DRM_FIXED_ONE		(1ULL << DRM_FIXED_POINT)
 #define DRM_FIXED_DECIMAL_MASK	(DRM_FIXED_ONE - 1)
 #define DRM_FIXED_DIGITS_MASK	(~DRM_FIXED_DECIMAL_MASK)
+#define DRM_FIXED_EPSILON	1LL
+#define DRM_FIXED_ALMOST_ONE	(DRM_FIXED_ONE - DRM_FIXED_EPSILON)
 
 static inline s64 drm_int2fixp(int a)
 {
 	return ((s64)a) << DRM_FIXED_POINT;
 }
 
-static inline int drm_fixp2int(int64_t a)
+static inline int drm_fixp2int(s64 a)
 {
 	return ((s64)a) >> DRM_FIXED_POINT;
 }
 
-static inline unsigned drm_fixp_msbset(int64_t a)
+static inline int drm_fixp2int_ceil(s64 a)
+{
+	if (a > 0)
+		return drm_fixp2int(a + DRM_FIXED_ALMOST_ONE);
+	else
+		return drm_fixp2int(a - DRM_FIXED_ALMOST_ONE);
+}
+
+static inline unsigned drm_fixp_msbset(s64 a)
 {
 	unsigned shift, sign = (a >> 63) & 1;
 
@@ -136,6 +146,46 @@ static inline s64 drm_fixp_div(s64 a, s64 b)
 	return result;
 }
 
+static inline s64 drm_fixp_from_fraction(s64 a, s64 b)
+{
+	s64 res;
+	bool a_neg = a < 0;
+	bool b_neg = b < 0;
+	u64 a_abs = a_neg ? -a : a;
+	u64 b_abs = b_neg ? -b : b;
+	u64 rem;
+
+	/* determine integer part */
+	u64 res_abs = a_abs / b_abs;
+	rem = a_abs % b_abs;
+
+	/* determine fractional part */
+	{
+		u32 i = DRM_FIXED_POINT;
+
+		do {
+			rem <<= 1;
+			res_abs <<= 1;
+			if (rem >= b_abs) {
+				res_abs |= 1;
+				rem -= b_abs;
+			}
+		} while (--i != 0);
+	}
+
+	/* round up LSB */
+	{
+		u64 summand = (rem << 1) >= b_abs;
+
+		res_abs += summand;
+	}
+
+	res = (s64) res_abs;
+	if (a_neg ^ b_neg)
+		res = -res;
+	return res;
+}
+
 static inline s64 drm_fixp_exp(s64 x)
 {
 	s64 tolerance = div64_s64(DRM_FIXED_ONE, 1000000);
-- 
2.1.4

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/5] drm/dp/mst: Calculate MST PBN with 31.32 fixed point
  2016-01-22 22:07 [PATCH 0/5] Fixes for MST (daisy-chain and 4k tiles) Harry Wentland
  2016-01-22 22:07 ` [PATCH 1/5] drm: Add drm_fixp_from_fraction and drm_fixp2int_ceil Harry Wentland
@ 2016-01-22 22:07 ` Harry Wentland
  2016-01-22 22:07 ` [PATCH 3/5] drm/dp/mst: change MST detection scheme Harry Wentland
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 22+ messages in thread
From: Harry Wentland @ 2016-01-22 22:07 UTC (permalink / raw)
  To: dri-devel

Our PBN value overflows the 20 bits integer part of the 20.12
fixed point. We need to use 31.32 fixed point to avoid this.

This happens with display clocks larger than 293122 (at 24 bpp),
which we see with the Sharp (and similar) 4k tiled displays.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 67 ++++++++++++++++++++---------------
 1 file changed, 39 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 6ed90a2437e5..041597b7a7c6 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2605,32 +2605,31 @@ EXPORT_SYMBOL(drm_dp_check_act_status);
  */
 int drm_dp_calc_pbn_mode(int clock, int bpp)
 {
-	fixed20_12 pix_bw;
-	fixed20_12 fbpp;
-	fixed20_12 result;
-	fixed20_12 margin, tmp;
-	u32 res;
-
-	pix_bw.full = dfixed_const(clock);
-	fbpp.full = dfixed_const(bpp);
-	tmp.full = dfixed_const(8);
-	fbpp.full = dfixed_div(fbpp, tmp);
-
-	result.full = dfixed_mul(pix_bw, fbpp);
-	margin.full = dfixed_const(54);
-	tmp.full = dfixed_const(64);
-	margin.full = dfixed_div(margin, tmp);
-	result.full = dfixed_div(result, margin);
-
-	margin.full = dfixed_const(1006);
-	tmp.full = dfixed_const(1000);
-	margin.full = dfixed_div(margin, tmp);
-	result.full = dfixed_mul(result, margin);
-
-	result.full = dfixed_div(result, tmp);
-	result.full = dfixed_ceil(result);
-	res = dfixed_trunc(result);
-	return res;
+	u64 kbps;
+	s64 peak_kbps;
+	u32 numerator;
+	u32 denominator;
+
+	kbps = clock * bpp;
+
+	/*
+	 * margin 5300ppm + 300ppm ~ 0.6% as per spec, factor is 1.006
+	 * The unit of 54/64Mbytes/sec is an arbitrary unit chosen based on
+	 * common multiplier to render an integer PBN for all link rate/lane
+	 * counts combinations
+	 * calculate
+	 * peak_kbps *= (1006/1000)
+	 * peak_kbps *= (64/54)
+	 * peak_kbps *= 8    convert to bytes
+	 */
+
+	numerator = 64 * 1006;
+	denominator = 54 * 8 * 1000 * 1000;
+
+	kbps *= numerator;
+	peak_kbps = drm_fixp_from_fraction(kbps, denominator);
+
+	return drm_fixp2int_ceil(peak_kbps);
 }
 EXPORT_SYMBOL(drm_dp_calc_pbn_mode);
 
@@ -2638,11 +2637,23 @@ static int test_calc_pbn_mode(void)
 {
 	int ret;
 	ret = drm_dp_calc_pbn_mode(154000, 30);
-	if (ret != 689)
+	if (ret != 689) {
+		DRM_ERROR("PBN calculation test failed - clock %d, bpp %d, expected PBN %d, actual PBN %d.\n",
+				154000, 30, 689, ret);
 		return -EINVAL;
+	}
 	ret = drm_dp_calc_pbn_mode(234000, 30);
-	if (ret != 1047)
+	if (ret != 1047) {
+		DRM_ERROR("PBN calculation test failed - clock %d, bpp %d, expected PBN %d, actual PBN %d.\n",
+				234000, 30, 1047, ret);
 		return -EINVAL;
+	}
+	ret = drm_dp_calc_pbn_mode(297000, 24);
+	if (ret != 1063) {
+		DRM_ERROR("PBN calculation test failed - clock %d, bpp %d, expected PBN %d, actual PBN %d.\n",
+				297000, 24, 1063, ret);
+		return -EINVAL;
+	}
 	return 0;
 }
 
-- 
2.1.4

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 3/5] drm/dp/mst: change MST detection scheme
  2016-01-22 22:07 [PATCH 0/5] Fixes for MST (daisy-chain and 4k tiles) Harry Wentland
  2016-01-22 22:07 ` [PATCH 1/5] drm: Add drm_fixp_from_fraction and drm_fixp2int_ceil Harry Wentland
  2016-01-22 22:07 ` [PATCH 2/5] drm/dp/mst: Calculate MST PBN with 31.32 fixed point Harry Wentland
@ 2016-01-22 22:07 ` Harry Wentland
  2016-02-17  5:45   ` Dave Airlie
  2016-01-22 22:07 ` [PATCH 4/5] drm/dp/mst: move GUID storage from mgr, port to only mst branch Harry Wentland
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 22+ messages in thread
From: Harry Wentland @ 2016-01-22 22:07 UTC (permalink / raw)
  To: dri-devel; +Cc: Mykola Lysenko

From: Mykola Lysenko <Mykola.Lysenko@amd.com>

1. Get edid for all connected MST displays, not only on logical ports,
   in the same thread as MST topology detection is done:
     There are displays that have branches inside w/o logical ports.
     So in case another SST display connected downstream system can
     end-up in situation when 3 DOWN requests sent: two for
    ‘remote i2c read’ and one for ‘enum path resources’, making slots full.

2. Call notification callback in one place in the end of topology discovery/update:
     This is done to reduce number of events sent to userspace in case complex
     topology discovery is going, adding multiple number of connectors;

3. Remove notification callback call from short pulse interrupt processing function:
     This is done in order not to block interrupt processing function, in case any
     MST request will be made from it. Notification will be send from topology
     discovery/update work item.

Signed-off-by: Mykola Lysenko <Mykola.Lysenko@amd.com>
Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
Acked-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 37 ++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 041597b7a7c6..052c20ca35ee 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1130,13 +1130,11 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
 			drm_dp_put_port(port);
 			goto out;
 		}
-		if (port->port_num >= DP_MST_LOGICAL_PORT_0) {
-			port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc);
-			drm_mode_connector_set_tile_property(port->connector);
-		}
+
+		drm_mode_connector_set_tile_property(port->connector);
+
 		(*mstb->mgr->cbs->register_connector)(port->connector);
 	}
-
 out:
 	/* put reference to this port */
 	drm_dp_put_port(port);
@@ -1161,9 +1159,9 @@ static void drm_dp_update_port(struct drm_dp_mst_branch *mstb,
 	port->ddps = conn_stat->displayport_device_plug_status;
 
 	if (old_ddps != port->ddps) {
+		dowork = true;
 		if (port->ddps) {
 			drm_dp_check_port_guid(mstb, port);
-			dowork = true;
 		} else {
 			port->guid_valid = false;
 			port->available_pbn = 0;
@@ -1271,8 +1269,13 @@ static void drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *m
 		if (port->input)
 			continue;
 
-		if (!port->ddps)
+		if (!port->ddps) {
+			if (port->cached_edid) {
+				kfree(port->cached_edid);
+				port->cached_edid = NULL;
+			}
 			continue;
+		}
 
 		if (!port->available_pbn)
 			drm_dp_send_enum_path_resources(mgr, mstb, port);
@@ -1283,6 +1286,12 @@ static void drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *m
 				drm_dp_check_and_send_link_address(mgr, mstb_child);
 				drm_dp_put_mst_branch_device(mstb_child);
 			}
+		} else if (port->pdt == DP_PEER_DEVICE_SST_SINK ||
+			port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV) {
+			if (!port->cached_edid) {
+				port->cached_edid =
+					drm_get_edid(port->connector, &port->aux.ddc);
+			}
 		}
 	}
 }
@@ -1302,6 +1311,8 @@ static void drm_dp_mst_link_probe_work(struct work_struct *work)
 		drm_dp_check_and_send_link_address(mgr, mstb);
 		drm_dp_put_mst_branch_device(mstb);
 	}
+
+	(*mgr->cbs->hotplug)(mgr);
 }
 
 static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
@@ -1558,7 +1569,6 @@ static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
 			for (i = 0; i < txmsg->reply.u.link_addr.nports; i++) {
 				drm_dp_add_port(mstb, mgr->dev, &txmsg->reply.u.link_addr.ports[i]);
 			}
-			(*mgr->cbs->hotplug)(mgr);
 		}
 	} else {
 		mstb->link_address_sent = false;
@@ -2232,8 +2242,6 @@ static int drm_dp_mst_handle_up_req(struct drm_dp_mst_topology_mgr *mgr)
 
 			drm_dp_update_port(mstb, &msg.u.conn_stat);
 			DRM_DEBUG_KMS("Got CSN: pn: %d ldps:%d ddps: %d mcs: %d ip: %d pdt: %d\n", msg.u.conn_stat.port_number, msg.u.conn_stat.legacy_device_plug_status, msg.u.conn_stat.displayport_device_plug_status, msg.u.conn_stat.message_capability_status, msg.u.conn_stat.input_port, msg.u.conn_stat.peer_device_type);
-			(*mgr->cbs->hotplug)(mgr);
-
 		} else if (msg.req_type == DP_RESOURCE_STATUS_NOTIFY) {
 			drm_dp_send_up_ack_reply(mgr, mgr->mst_primary, msg.req_type, seqno, false);
 			if (!mstb)
@@ -2320,10 +2328,6 @@ enum drm_connector_status drm_dp_mst_detect_port(struct drm_connector *connector
 
 	case DP_PEER_DEVICE_SST_SINK:
 		status = connector_status_connected;
-		/* for logical ports - cache the EDID */
-		if (port->port_num >= 8 && !port->cached_edid) {
-			port->cached_edid = drm_get_edid(connector, &port->aux.ddc);
-		}
 		break;
 	case DP_PEER_DEVICE_DP_LEGACY_CONV:
 		if (port->ldps)
@@ -2378,10 +2382,7 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_
 
 	if (port->cached_edid)
 		edid = drm_edid_duplicate(port->cached_edid);
-	else {
-		edid = drm_get_edid(connector, &port->aux.ddc);
-		drm_mode_connector_set_tile_property(connector);
-	}
+
 	port->has_audio = drm_detect_monitor_audio(edid);
 	drm_dp_put_port(port);
 	return edid;
-- 
2.1.4

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 4/5] drm/dp/mst: move GUID storage from mgr, port to only mst branch
  2016-01-22 22:07 [PATCH 0/5] Fixes for MST (daisy-chain and 4k tiles) Harry Wentland
                   ` (2 preceding siblings ...)
  2016-01-22 22:07 ` [PATCH 3/5] drm/dp/mst: change MST detection scheme Harry Wentland
@ 2016-01-22 22:07 ` Harry Wentland
  2016-01-22 22:07 ` [PATCH 5/5] drm/dp/mst: Reverse order of MST enable and clearing VC payload table Harry Wentland
  2016-01-25  4:53 ` [PATCH 0/5] Fixes for MST (daisy-chain and 4k tiles) Dave Airlie
  5 siblings, 0 replies; 22+ messages in thread
From: Harry Wentland @ 2016-01-22 22:07 UTC (permalink / raw)
  To: dri-devel; +Cc: Hersen Wu

From: Hersen Wu <hersenxs.wu@amd.com>

Previous implementation does not handle case below: boot up one MST branch
to DP connector of ASIC. After boot up, hot plug 2nd MST branch to DP output
of 1st MST, GUID is not created for 2nd MST branch. When downstream port of
2nd MST branch send upstream request, it fails because 2nd MST branch GUID
is not available.

New Implementation: only create GUID for MST branch and save it within Branch.

Signed-off-by: Hersen Wu <hersenxs.wu@amd.com>
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Acked-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 64 +++++++++++++++--------------------
 include/drm/drm_dp_mst_helper.h       | 25 ++++++--------
 2 files changed, 38 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 052c20ca35ee..01a7d0198e15 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1018,18 +1018,27 @@ static bool drm_dp_port_setup_pdt(struct drm_dp_mst_port *port)
 	return send_link;
 }
 
-static void drm_dp_check_port_guid(struct drm_dp_mst_branch *mstb,
-				   struct drm_dp_mst_port *port)
+static void drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb, u8 *guid)
 {
 	int ret;
-	if (port->dpcd_rev >= 0x12) {
-		port->guid_valid = drm_dp_validate_guid(mstb->mgr, port->guid);
-		if (!port->guid_valid) {
-			ret = drm_dp_send_dpcd_write(mstb->mgr,
-						     port,
-						     DP_GUID,
-						     16, port->guid);
-			port->guid_valid = true;
+
+	memcpy(mstb->guid, guid, 16);
+
+	if (!drm_dp_validate_guid(mstb->mgr, mstb->guid)) {
+		if (mstb->port_parent) {
+			ret = drm_dp_send_dpcd_write(
+					mstb->mgr,
+					mstb->port_parent,
+					DP_GUID,
+					16,
+					mstb->guid);
+		} else {
+
+			ret = drm_dp_dpcd_write(
+					mstb->mgr->aux,
+					DP_GUID,
+					mstb->guid,
+					16);
 		}
 	}
 }
@@ -1086,7 +1095,6 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
 	port->dpcd_rev = port_msg->dpcd_revision;
 	port->num_sdp_streams = port_msg->num_sdp_streams;
 	port->num_sdp_stream_sinks = port_msg->num_sdp_stream_sinks;
-	memcpy(port->guid, port_msg->peer_guid, 16);
 
 	/* manage mstb port lists with mgr lock - take a reference
 	   for this list */
@@ -1099,11 +1107,9 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
 
 	if (old_ddps != port->ddps) {
 		if (port->ddps) {
-			drm_dp_check_port_guid(mstb, port);
 			if (!port->input)
 				drm_dp_send_enum_path_resources(mstb->mgr, mstb, port);
 		} else {
-			port->guid_valid = false;
 			port->available_pbn = 0;
 			}
 	}
@@ -1161,9 +1167,7 @@ static void drm_dp_update_port(struct drm_dp_mst_branch *mstb,
 	if (old_ddps != port->ddps) {
 		dowork = true;
 		if (port->ddps) {
-			drm_dp_check_port_guid(mstb, port);
 		} else {
-			port->guid_valid = false;
 			port->available_pbn = 0;
 		}
 	}
@@ -1220,13 +1224,14 @@ static struct drm_dp_mst_branch *get_mst_branch_device_by_guid_helper(
 	struct drm_dp_mst_branch *found_mstb;
 	struct drm_dp_mst_port *port;
 
+	if (memcmp(mstb->guid, guid, 16) == 0)
+		return mstb;
+
+
 	list_for_each_entry(port, &mstb->ports, next) {
 		if (!port->mstb)
 			continue;
 
-		if (port->guid_valid && memcmp(port->guid, guid, 16) == 0)
-			return port->mstb;
-
 		found_mstb = get_mst_branch_device_by_guid_helper(port->mstb, guid);
 
 		if (found_mstb)
@@ -1245,10 +1250,7 @@ static struct drm_dp_mst_branch *drm_dp_get_mst_branch_device_by_guid(
 	/* find the port by iterating down */
 	mutex_lock(&mgr->lock);
 
-	if (mgr->guid_valid && memcmp(mgr->guid, guid, 16) == 0)
-		mstb = mgr->mst_primary;
-	else
-		mstb = get_mst_branch_device_by_guid_helper(mgr->mst_primary, guid);
+	mstb = get_mst_branch_device_by_guid_helper(mgr->mst_primary, guid);
 
 	if (mstb)
 		kref_get(&mstb->kref);
@@ -1566,6 +1568,9 @@ static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
 				       txmsg->reply.u.link_addr.ports[i].num_sdp_streams,
 				       txmsg->reply.u.link_addr.ports[i].num_sdp_stream_sinks);
 			}
+
+			drm_dp_check_mstb_guid(mstb, txmsg->reply.u.link_addr.guid);
+
 			for (i = 0; i < txmsg->reply.u.link_addr.nports; i++) {
 				drm_dp_add_port(mstb, mgr->dev, &txmsg->reply.u.link_addr.ports[i]);
 			}
@@ -2006,20 +2011,6 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms
 			goto out_unlock;
 		}
 
-
-		/* sort out guid */
-		ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, mgr->guid, 16);
-		if (ret != 16) {
-			DRM_DEBUG_KMS("failed to read DP GUID %d\n", ret);
-			goto out_unlock;
-		}
-
-		mgr->guid_valid = drm_dp_validate_guid(mgr, mgr->guid);
-		if (!mgr->guid_valid) {
-			ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, mgr->guid, 16);
-			mgr->guid_valid = true;
-		}
-
 		queue_work(system_long_wq, &mgr->work);
 
 		ret = 0;
@@ -2241,6 +2232,7 @@ static int drm_dp_mst_handle_up_req(struct drm_dp_mst_topology_mgr *mgr)
 			}
 
 			drm_dp_update_port(mstb, &msg.u.conn_stat);
+
 			DRM_DEBUG_KMS("Got CSN: pn: %d ldps:%d ddps: %d mcs: %d ip: %d pdt: %d\n", msg.u.conn_stat.port_number, msg.u.conn_stat.legacy_device_plug_status, msg.u.conn_stat.displayport_device_plug_status, msg.u.conn_stat.message_capability_status, msg.u.conn_stat.input_port, msg.u.conn_stat.peer_device_type);
 		} else if (msg.req_type == DP_RESOURCE_STATUS_NOTIFY) {
 			drm_dp_send_up_ack_reply(mgr, mgr->mst_primary, msg.req_type, seqno, false);
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 24ab1787b771..fdb47051d549 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -44,8 +44,6 @@ struct drm_dp_vcpi {
 /**
  * struct drm_dp_mst_port - MST port
  * @kref: reference count for this port.
- * @guid_valid: for DP 1.2 devices if we have validated the GUID.
- * @guid: guid for DP 1.2 device on this port.
  * @port_num: port number
  * @input: if this port is an input port.
  * @mcs: message capability status - DP 1.2 spec.
@@ -70,10 +68,6 @@ struct drm_dp_vcpi {
 struct drm_dp_mst_port {
 	struct kref kref;
 
-	/* if dpcd 1.2 device is on this port - its GUID info */
-	bool guid_valid;
-	u8 guid[16];
-
 	u8 port_num;
 	bool input;
 	bool mcs;
@@ -110,10 +104,12 @@ struct drm_dp_mst_port {
  * @tx_slots: transmission slots for this device.
  * @last_seqno: last sequence number used to talk to this.
  * @link_address_sent: if a link address message has been sent to this device yet.
+ * @guid: guid for DP 1.2 branch device. port under this branch can be
+ * identified by port #.
  *
  * This structure represents an MST branch device, there is one
- * primary branch device at the root, along with any others connected
- * to downstream ports
+ * primary branch device at the root, along with any other branches connected
+ * to downstream port of parent branches.
  */
 struct drm_dp_mst_branch {
 	struct kref kref;
@@ -132,6 +128,9 @@ struct drm_dp_mst_branch {
 	struct drm_dp_sideband_msg_tx *tx_slots[2];
 	int last_seqno;
 	bool link_address_sent;
+
+	/* global unique identifier to identify branch devices */
+	u8 guid[16];
 };
 
 
@@ -406,11 +405,9 @@ struct drm_dp_payload {
  * @conn_base_id: DRM connector ID this mgr is connected to.
  * @down_rep_recv: msg receiver state for down replies.
  * @up_req_recv: msg receiver state for up requests.
- * @lock: protects mst state, primary, guid, dpcd.
+ * @lock: protects mst state, primary, dpcd.
  * @mst_state: if this manager is enabled for an MST capable port.
  * @mst_primary: pointer to the primary branch device.
- * @guid_valid: GUID valid for the primary branch device.
- * @guid: GUID for primary port.
  * @dpcd: cache of DPCD for primary port.
  * @pbn_div: PBN to slots divisor.
  *
@@ -432,13 +429,11 @@ struct drm_dp_mst_topology_mgr {
 	struct drm_dp_sideband_msg_rx up_req_recv;
 
 	/* pointer to info about the initial MST device */
-	struct mutex lock; /* protects mst_state + primary + guid + dpcd */
+	struct mutex lock; /* protects mst_state + primary + dpcd */
 
 	bool mst_state;
 	struct drm_dp_mst_branch *mst_primary;
-	/* primary MST device GUID */
-	bool guid_valid;
-	u8 guid[16];
+
 	u8 dpcd[DP_RECEIVER_CAP_SIZE];
 	u8 sink_count;
 	int pbn_div;
-- 
2.1.4

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 5/5] drm/dp/mst: Reverse order of MST enable and clearing VC payload table.
  2016-01-22 22:07 [PATCH 0/5] Fixes for MST (daisy-chain and 4k tiles) Harry Wentland
                   ` (3 preceding siblings ...)
  2016-01-22 22:07 ` [PATCH 4/5] drm/dp/mst: move GUID storage from mgr, port to only mst branch Harry Wentland
@ 2016-01-22 22:07 ` Harry Wentland
  2016-01-25  4:53 ` [PATCH 0/5] Fixes for MST (daisy-chain and 4k tiles) Dave Airlie
  5 siblings, 0 replies; 22+ messages in thread
From: Harry Wentland @ 2016-01-22 22:07 UTC (permalink / raw)
  To: dri-devel; +Cc: Andrey Grodzovsky

From: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com>

On DELL U3014 if you clear the table before enabling MST it sometimes
hangs the receiver.

Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com>
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Acked-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 01a7d0198e15..fc589265068b 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1998,6 +1998,12 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms
 		mgr->mst_primary = mstb;
 		kref_get(&mgr->mst_primary->kref);
 
+		ret = drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL,
+							 DP_MST_EN | DP_UP_REQ_EN | DP_UPSTREAM_IS_SRC);
+		if (ret < 0) {
+			goto out_unlock;
+		}
+
 		{
 			struct drm_dp_payload reset_pay;
 			reset_pay.start_slot = 0;
@@ -2005,12 +2011,6 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms
 			drm_dp_dpcd_write_payload(mgr, 0, &reset_pay);
 		}
 
-		ret = drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL,
-					 DP_MST_EN | DP_UP_REQ_EN | DP_UPSTREAM_IS_SRC);
-		if (ret < 0) {
-			goto out_unlock;
-		}
-
 		queue_work(system_long_wq, &mgr->work);
 
 		ret = 0;
-- 
2.1.4

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/5] Fixes for MST (daisy-chain and 4k tiles)
  2016-01-22 22:07 [PATCH 0/5] Fixes for MST (daisy-chain and 4k tiles) Harry Wentland
                   ` (4 preceding siblings ...)
  2016-01-22 22:07 ` [PATCH 5/5] drm/dp/mst: Reverse order of MST enable and clearing VC payload table Harry Wentland
@ 2016-01-25  4:53 ` Dave Airlie
  2016-01-25 18:55   ` Wentland, Harry
  5 siblings, 1 reply; 22+ messages in thread
From: Dave Airlie @ 2016-01-25  4:53 UTC (permalink / raw)
  To: Harry Wentland; +Cc: dri-devel

On 23 January 2016 at 08:07, Harry Wentland <harry.wentland@amd.com> wrote:
> A couple of MST fixes to bugs in the framework that we encountered when
> testing with
>   - three-display daisy-chain configurations
>   - 4k tiled displays

Hi Harry,

these all look pretty good to me, have you tested them with lockdep enabled?

I had started to fix the CSN/GUID one here a little while back but it's sitting
on a laptop I can't access at the moment.

But if you can test with lockdep, and it doesn't deadlock, then I'm happy
to accept these..

Reviewed-by: Dave Airlie <airlied@redhat.com>

Dave.
>
> Andrey Grodzovsky (1):
>   drm/dp/mst: Reverse order of MST enable and clearing VC payload table.
>
> Harry Wentland (2):
>   drm: Add drm_fixp_from_fraction and drm_fixp2int_ceil
>   drm/dp/mst: Calculate MST PBN with 31.32 fixed point
>
> Hersen Wu (1):
>   drm/dp/mst: move GUID storage from mgr, port to only mst branch
>
> Mykola Lysenko (1):
>   drm/dp/mst: change MST detection scheme
>
>  drivers/gpu/drm/drm_dp_mst_topology.c | 180 +++++++++++++++++-----------------
>  include/drm/drm_dp_mst_helper.h       |  25 ++---
>  include/drm/drm_fixed.h               |  54 +++++++++-
>  3 files changed, 154 insertions(+), 105 deletions(-)
>
> --
> 2.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/5] Fixes for MST (daisy-chain and 4k tiles)
  2016-01-25  4:53 ` [PATCH 0/5] Fixes for MST (daisy-chain and 4k tiles) Dave Airlie
@ 2016-01-25 18:55   ` Wentland, Harry
  2016-02-02 21:20     ` Alex Deucher
  0 siblings, 1 reply; 22+ messages in thread
From: Wentland, Harry @ 2016-01-25 18:55 UTC (permalink / raw)
  To: Dave Airlie; +Cc: Deucher, Alexander, dri-devel

Hi Dave,

I've been running with lockdep with these changes for over a week now. Just ran another test with our daisy-chain displays and the 4k tiled display with no deadlocks or lockdep prints (other than "RCU lockdep checking is enabled.").

Thanks,
Harry

________________________________________
From: Dave Airlie <airlied@gmail.com>
Sent: Sunday, January 24, 2016 11:53 PM
To: Wentland, Harry
Cc: dri-devel
Subject: Re: [PATCH 0/5] Fixes for MST (daisy-chain and 4k tiles)

On 23 January 2016 at 08:07, Harry Wentland <harry.wentland@amd.com> wrote:
> A couple of MST fixes to bugs in the framework that we encountered when
> testing with
>   - three-display daisy-chain configurations
>   - 4k tiled displays

Hi Harry,

these all look pretty good to me, have you tested them with lockdep enabled?

I had started to fix the CSN/GUID one here a little while back but it's sitting
on a laptop I can't access at the moment.

But if you can test with lockdep, and it doesn't deadlock, then I'm happy
to accept these..

Reviewed-by: Dave Airlie <airlied@redhat.com>

Dave.
>
> Andrey Grodzovsky (1):
>   drm/dp/mst: Reverse order of MST enable and clearing VC payload table.
>
> Harry Wentland (2):
>   drm: Add drm_fixp_from_fraction and drm_fixp2int_ceil
>   drm/dp/mst: Calculate MST PBN with 31.32 fixed point
>
> Hersen Wu (1):
>   drm/dp/mst: move GUID storage from mgr, port to only mst branch
>
> Mykola Lysenko (1):
>   drm/dp/mst: change MST detection scheme
>
>  drivers/gpu/drm/drm_dp_mst_topology.c | 180 +++++++++++++++++-----------------
>  include/drm/drm_dp_mst_helper.h       |  25 ++---
>  include/drm/drm_fixed.h               |  54 +++++++++-
>  3 files changed, 154 insertions(+), 105 deletions(-)
>
> --
> 2.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/5] Fixes for MST (daisy-chain and 4k tiles)
  2016-01-25 18:55   ` Wentland, Harry
@ 2016-02-02 21:20     ` Alex Deucher
  2016-02-04  0:35       ` Dave Airlie
  0 siblings, 1 reply; 22+ messages in thread
From: Alex Deucher @ 2016-02-02 21:20 UTC (permalink / raw)
  To: Wentland, Harry; +Cc: Deucher, Alexander, dri-devel

On Mon, Jan 25, 2016 at 1:55 PM, Wentland, Harry <Harry.Wentland@amd.com> wrote:
> Hi Dave,
>
> I've been running with lockdep with these changes for over a week now. Just ran another test with our daisy-chain displays and the 4k tiled display with no deadlocks or lockdep prints (other than "RCU lockdep checking is enabled.").
>

Dave, do you want to pick these up directly or should I pull them into
my -fixes tree?

Alex

> Thanks,
> Harry
>
> ________________________________________
> From: Dave Airlie <airlied@gmail.com>
> Sent: Sunday, January 24, 2016 11:53 PM
> To: Wentland, Harry
> Cc: dri-devel
> Subject: Re: [PATCH 0/5] Fixes for MST (daisy-chain and 4k tiles)
>
> On 23 January 2016 at 08:07, Harry Wentland <harry.wentland@amd.com> wrote:
>> A couple of MST fixes to bugs in the framework that we encountered when
>> testing with
>>   - three-display daisy-chain configurations
>>   - 4k tiled displays
>
> Hi Harry,
>
> these all look pretty good to me, have you tested them with lockdep enabled?
>
> I had started to fix the CSN/GUID one here a little while back but it's sitting
> on a laptop I can't access at the moment.
>
> But if you can test with lockdep, and it doesn't deadlock, then I'm happy
> to accept these..
>
> Reviewed-by: Dave Airlie <airlied@redhat.com>
>
> Dave.
>>
>> Andrey Grodzovsky (1):
>>   drm/dp/mst: Reverse order of MST enable and clearing VC payload table.
>>
>> Harry Wentland (2):
>>   drm: Add drm_fixp_from_fraction and drm_fixp2int_ceil
>>   drm/dp/mst: Calculate MST PBN with 31.32 fixed point
>>
>> Hersen Wu (1):
>>   drm/dp/mst: move GUID storage from mgr, port to only mst branch
>>
>> Mykola Lysenko (1):
>>   drm/dp/mst: change MST detection scheme
>>
>>  drivers/gpu/drm/drm_dp_mst_topology.c | 180 +++++++++++++++++-----------------
>>  include/drm/drm_dp_mst_helper.h       |  25 ++---
>>  include/drm/drm_fixed.h               |  54 +++++++++-
>>  3 files changed, 154 insertions(+), 105 deletions(-)
>>
>> --
>> 2.1.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/5] Fixes for MST (daisy-chain and 4k tiles)
  2016-02-02 21:20     ` Alex Deucher
@ 2016-02-04  0:35       ` Dave Airlie
  0 siblings, 0 replies; 22+ messages in thread
From: Dave Airlie @ 2016-02-04  0:35 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Deucher, Alexander, dri-devel

On 3 February 2016 at 07:20, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Mon, Jan 25, 2016 at 1:55 PM, Wentland, Harry <Harry.Wentland@amd.com> wrote:
>> Hi Dave,
>>
>> I've been running with lockdep with these changes for over a week now. Just ran another test with our daisy-chain displays and the 4k tiled display with no deadlocks or lockdep prints (other than "RCU lockdep checking is enabled.").
>>
>
> Dave, do you want to pick these up directly or should I pull them into
> my -fixes tree?
>
I'm going to pull them in myself for this round I think, there are a
few other MST bits around I want to test before pushing.

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/5] drm/dp/mst: change MST detection scheme
  2016-01-22 22:07 ` [PATCH 3/5] drm/dp/mst: change MST detection scheme Harry Wentland
@ 2016-02-17  5:45   ` Dave Airlie
  2016-02-18  4:53     ` Lysenko, Mykola
  2016-02-23  3:09     ` Lysenko, Mykola
  0 siblings, 2 replies; 22+ messages in thread
From: Dave Airlie @ 2016-02-17  5:45 UTC (permalink / raw)
  To: Harry Wentland; +Cc: Mykola Lysenko, dri-devel

On 23 January 2016 at 08:07, Harry Wentland <harry.wentland@amd.com> wrote:
> From: Mykola Lysenko <Mykola.Lysenko@amd.com>
>
> 1. Get edid for all connected MST displays, not only on logical ports,
>    in the same thread as MST topology detection is done:
>      There are displays that have branches inside w/o logical ports.
>      So in case another SST display connected downstream system can
>      end-up in situation when 3 DOWN requests sent: two for
>     ‘remote i2c read’ and one for ‘enum path resources’, making slots full.
>
> 2. Call notification callback in one place in the end of topology discovery/update:
>      This is done to reduce number of events sent to userspace in case complex
>      topology discovery is going, adding multiple number of connectors;
>
> 3. Remove notification callback call from short pulse interrupt processing function:
>      This is done in order not to block interrupt processing function, in case any
>      MST request will be made from it. Notification will be send from topology
>      discovery/update work item.

I've had to pull this out, as I did some more indepth testing with
i915 and a Dell 30"
and this broke things.

The main thing it broke is setting the tiling property that userspace
needs to see
those dual-panel monitors as one.

you should be able to see xrandr --props if the tile property is
correct if you test.

I'm also not sure about some other bits in this patch, so I probably need to dig
a bit deeper into it.

Dave.

>
> Signed-off-by: Mykola Lysenko <Mykola.Lysenko@amd.com>
> Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 37 ++++++++++++++++++-----------------
>  1 file changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 041597b7a7c6..052c20ca35ee 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1130,13 +1130,11 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
>                         drm_dp_put_port(port);
>                         goto out;
>                 }
> -               if (port->port_num >= DP_MST_LOGICAL_PORT_0) {
> -                       port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc);
> -                       drm_mode_connector_set_tile_property(port->connector);
> -               }
> +
> +               drm_mode_connector_set_tile_property(port->connector);
> +
>                 (*mstb->mgr->cbs->register_connector)(port->connector);
>         }
> -
>  out:
>         /* put reference to this port */
>         drm_dp_put_port(port);
> @@ -1161,9 +1159,9 @@ static void drm_dp_update_port(struct drm_dp_mst_branch *mstb,
>         port->ddps = conn_stat->displayport_device_plug_status;
>
>         if (old_ddps != port->ddps) {
> +               dowork = true;
>                 if (port->ddps) {
>                         drm_dp_check_port_guid(mstb, port);
> -                       dowork = true;
>                 } else {
>                         port->guid_valid = false;
>                         port->available_pbn = 0;
> @@ -1271,8 +1269,13 @@ static void drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *m
>                 if (port->input)
>                         continue;
>
> -               if (!port->ddps)
> +               if (!port->ddps) {
> +                       if (port->cached_edid) {
> +                               kfree(port->cached_edid);
> +                               port->cached_edid = NULL;
> +                       }
>                         continue;
> +               }
>
>                 if (!port->available_pbn)
>                         drm_dp_send_enum_path_resources(mgr, mstb, port);
> @@ -1283,6 +1286,12 @@ static void drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *m
>                                 drm_dp_check_and_send_link_address(mgr, mstb_child);
>                                 drm_dp_put_mst_branch_device(mstb_child);
>                         }
> +               } else if (port->pdt == DP_PEER_DEVICE_SST_SINK ||
> +                       port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV) {
> +                       if (!port->cached_edid) {
> +                               port->cached_edid =
> +                                       drm_get_edid(port->connector, &port->aux.ddc);
> +                       }
>                 }
>         }
>  }
> @@ -1302,6 +1311,8 @@ static void drm_dp_mst_link_probe_work(struct work_struct *work)
>                 drm_dp_check_and_send_link_address(mgr, mstb);
>                 drm_dp_put_mst_branch_device(mstb);
>         }
> +
> +       (*mgr->cbs->hotplug)(mgr);
>  }
>
>  static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
> @@ -1558,7 +1569,6 @@ static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
>                         for (i = 0; i < txmsg->reply.u.link_addr.nports; i++) {
>                                 drm_dp_add_port(mstb, mgr->dev, &txmsg->reply.u.link_addr.ports[i]);
>                         }
> -                       (*mgr->cbs->hotplug)(mgr);
>                 }
>         } else {
>                 mstb->link_address_sent = false;
> @@ -2232,8 +2242,6 @@ static int drm_dp_mst_handle_up_req(struct drm_dp_mst_topology_mgr *mgr)
>
>                         drm_dp_update_port(mstb, &msg.u.conn_stat);
>                         DRM_DEBUG_KMS("Got CSN: pn: %d ldps:%d ddps: %d mcs: %d ip: %d pdt: %d\n", msg.u.conn_stat.port_number, msg.u.conn_stat.legacy_device_plug_status, msg.u.conn_stat.displayport_device_plug_status, msg.u.conn_stat.message_capability_status, msg.u.conn_stat.input_port, msg.u.conn_stat.peer_device_type);
> -                       (*mgr->cbs->hotplug)(mgr);
> -
>                 } else if (msg.req_type == DP_RESOURCE_STATUS_NOTIFY) {
>                         drm_dp_send_up_ack_reply(mgr, mgr->mst_primary, msg.req_type, seqno, false);
>                         if (!mstb)
> @@ -2320,10 +2328,6 @@ enum drm_connector_status drm_dp_mst_detect_port(struct drm_connector *connector
>
>         case DP_PEER_DEVICE_SST_SINK:
>                 status = connector_status_connected;
> -               /* for logical ports - cache the EDID */
> -               if (port->port_num >= 8 && !port->cached_edid) {
> -                       port->cached_edid = drm_get_edid(connector, &port->aux.ddc);
> -               }
>                 break;
>         case DP_PEER_DEVICE_DP_LEGACY_CONV:
>                 if (port->ldps)
> @@ -2378,10 +2382,7 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_
>
>         if (port->cached_edid)
>                 edid = drm_edid_duplicate(port->cached_edid);
> -       else {
> -               edid = drm_get_edid(connector, &port->aux.ddc);
> -               drm_mode_connector_set_tile_property(connector);
> -       }
> +
>         port->has_audio = drm_detect_monitor_audio(edid);
>         drm_dp_put_port(port);
>         return edid;
> --
> 2.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 3/5] drm/dp/mst: change MST detection scheme
  2016-02-17  5:45   ` Dave Airlie
@ 2016-02-18  4:53     ` Lysenko, Mykola
  2016-02-23  3:09     ` Lysenko, Mykola
  1 sibling, 0 replies; 22+ messages in thread
From: Lysenko, Mykola @ 2016-02-18  4:53 UTC (permalink / raw)
  To: Dave Airlie, Wentland, Harry; +Cc: dri-devel

Hi Dave,

it seems to be missed call to 

drm_mode_connector_set_tile_property(port->connector);

here:

> +               } else if (port->pdt == DP_PEER_DEVICE_SST_SINK ||
> +                       port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV) {
> +                       if (!port->cached_edid) {
> +                               port->cached_edid =
> +                                       drm_get_edid(port->connector, &port->aux.ddc);
	                     drm_mode_connector_set_tile_property(port->connector);
> +                       }

I will test with tile display to see if it solves the issue.

Thanks,
Mykola


-----Original Message-----
From: Dave Airlie [mailto:airlied@gmail.com] 
Sent: Wednesday, February 17, 2016 1:46 PM
To: Wentland, Harry <Harry.Wentland@amd.com>
Cc: dri-devel <dri-devel@lists.freedesktop.org>; Lysenko, Mykola <Mykola.Lysenko@amd.com>
Subject: Re: [PATCH 3/5] drm/dp/mst: change MST detection scheme

On 23 January 2016 at 08:07, Harry Wentland <harry.wentland@amd.com> wrote:
> From: Mykola Lysenko <Mykola.Lysenko@amd.com>
>
> 1. Get edid for all connected MST displays, not only on logical ports,
>    in the same thread as MST topology detection is done:
>      There are displays that have branches inside w/o logical ports.
>      So in case another SST display connected downstream system can
>      end-up in situation when 3 DOWN requests sent: two for
>     ‘remote i2c read’ and one for ‘enum path resources’, making slots full.
>
> 2. Call notification callback in one place in the end of topology discovery/update:
>      This is done to reduce number of events sent to userspace in case complex
>      topology discovery is going, adding multiple number of connectors;
>
> 3. Remove notification callback call from short pulse interrupt processing function:
>      This is done in order not to block interrupt processing function, in case any
>      MST request will be made from it. Notification will be send from topology
>      discovery/update work item.

I've had to pull this out, as I did some more indepth testing with
i915 and a Dell 30"
and this broke things.

The main thing it broke is setting the tiling property that userspace
needs to see
those dual-panel monitors as one.

you should be able to see xrandr --props if the tile property is
correct if you test.

I'm also not sure about some other bits in this patch, so I probably need to dig
a bit deeper into it.

Dave.

>
> Signed-off-by: Mykola Lysenko <Mykola.Lysenko@amd.com>
> Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 37 ++++++++++++++++++-----------------
>  1 file changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 041597b7a7c6..052c20ca35ee 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1130,13 +1130,11 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
>                         drm_dp_put_port(port);
>                         goto out;
>                 }
> -               if (port->port_num >= DP_MST_LOGICAL_PORT_0) {
> -                       port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc);
> -                       drm_mode_connector_set_tile_property(port->connector);
> -               }
> +
> +               drm_mode_connector_set_tile_property(port->connector);
> +
>                 (*mstb->mgr->cbs->register_connector)(port->connector);
>         }
> -
>  out:
>         /* put reference to this port */
>         drm_dp_put_port(port);
> @@ -1161,9 +1159,9 @@ static void drm_dp_update_port(struct drm_dp_mst_branch *mstb,
>         port->ddps = conn_stat->displayport_device_plug_status;
>
>         if (old_ddps != port->ddps) {
> +               dowork = true;
>                 if (port->ddps) {
>                         drm_dp_check_port_guid(mstb, port);
> -                       dowork = true;
>                 } else {
>                         port->guid_valid = false;
>                         port->available_pbn = 0;
> @@ -1271,8 +1269,13 @@ static void drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *m
>                 if (port->input)
>                         continue;
>
> -               if (!port->ddps)
> +               if (!port->ddps) {
> +                       if (port->cached_edid) {
> +                               kfree(port->cached_edid);
> +                               port->cached_edid = NULL;
> +                       }
>                         continue;
> +               }
>
>                 if (!port->available_pbn)
>                         drm_dp_send_enum_path_resources(mgr, mstb, port);
> @@ -1283,6 +1286,12 @@ static void drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *m
>                                 drm_dp_check_and_send_link_address(mgr, mstb_child);
>                                 drm_dp_put_mst_branch_device(mstb_child);
>                         }
> +               } else if (port->pdt == DP_PEER_DEVICE_SST_SINK ||
> +                       port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV) {
> +                       if (!port->cached_edid) {
> +                               port->cached_edid =
> +                                       drm_get_edid(port->connector, &port->aux.ddc);
> +                       }
>                 }
>         }
>  }
> @@ -1302,6 +1311,8 @@ static void drm_dp_mst_link_probe_work(struct work_struct *work)
>                 drm_dp_check_and_send_link_address(mgr, mstb);
>                 drm_dp_put_mst_branch_device(mstb);
>         }
> +
> +       (*mgr->cbs->hotplug)(mgr);
>  }
>
>  static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
> @@ -1558,7 +1569,6 @@ static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
>                         for (i = 0; i < txmsg->reply.u.link_addr.nports; i++) {
>                                 drm_dp_add_port(mstb, mgr->dev, &txmsg->reply.u.link_addr.ports[i]);
>                         }
> -                       (*mgr->cbs->hotplug)(mgr);
>                 }
>         } else {
>                 mstb->link_address_sent = false;
> @@ -2232,8 +2242,6 @@ static int drm_dp_mst_handle_up_req(struct drm_dp_mst_topology_mgr *mgr)
>
>                         drm_dp_update_port(mstb, &msg.u.conn_stat);
>                         DRM_DEBUG_KMS("Got CSN: pn: %d ldps:%d ddps: %d mcs: %d ip: %d pdt: %d\n", msg.u.conn_stat.port_number, msg.u.conn_stat.legacy_device_plug_status, msg.u.conn_stat.displayport_device_plug_status, msg.u.conn_stat.message_capability_status, msg.u.conn_stat.input_port, msg.u.conn_stat.peer_device_type);
> -                       (*mgr->cbs->hotplug)(mgr);
> -
>                 } else if (msg.req_type == DP_RESOURCE_STATUS_NOTIFY) {
>                         drm_dp_send_up_ack_reply(mgr, mgr->mst_primary, msg.req_type, seqno, false);
>                         if (!mstb)
> @@ -2320,10 +2328,6 @@ enum drm_connector_status drm_dp_mst_detect_port(struct drm_connector *connector
>
>         case DP_PEER_DEVICE_SST_SINK:
>                 status = connector_status_connected;
> -               /* for logical ports - cache the EDID */
> -               if (port->port_num >= 8 && !port->cached_edid) {
> -                       port->cached_edid = drm_get_edid(connector, &port->aux.ddc);
> -               }
>                 break;
>         case DP_PEER_DEVICE_DP_LEGACY_CONV:
>                 if (port->ldps)
> @@ -2378,10 +2382,7 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_
>
>         if (port->cached_edid)
>                 edid = drm_edid_duplicate(port->cached_edid);
> -       else {
> -               edid = drm_get_edid(connector, &port->aux.ddc);
> -               drm_mode_connector_set_tile_property(connector);
> -       }
> +
>         port->has_audio = drm_detect_monitor_audio(edid);
>         drm_dp_put_port(port);
>         return edid;
> --
> 2.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 3/5] drm/dp/mst: change MST detection scheme
  2016-02-17  5:45   ` Dave Airlie
  2016-02-18  4:53     ` Lysenko, Mykola
@ 2016-02-23  3:09     ` Lysenko, Mykola
  2016-05-12 22:41       ` Grodzovsky, Andrey
  1 sibling, 1 reply; 22+ messages in thread
From: Lysenko, Mykola @ 2016-02-23  3:09 UTC (permalink / raw)
  To: Dave Airlie, Wentland, Harry; +Cc: dri-devel

[-- Attachment #1: Type: text/plain, Size: 9234 bytes --]

Hi Dave,

Attached patch should fix the issue.

I have one question though: should we add drm_mode_connector_set_tile_property to the drm_get_displayid function, so it will be updated in case of regular DP displays as well?

As an example of such usage could be 5k displays with two tiles 2560x2880 that could not be driven on one MST connector because of bandwidth limitations, and are driven in SST mode.

Thanks,
Mykola

-----Original Message-----
From: Lysenko, Mykola 
Sent: Thursday, February 18, 2016 12:53 PM
To: 'Dave Airlie' <airlied@gmail.com>; Wentland, Harry <Harry.Wentland@amd.com>
Cc: dri-devel <dri-devel@lists.freedesktop.org>
Subject: RE: [PATCH 3/5] drm/dp/mst: change MST detection scheme

Hi Dave,

it seems to be missed call to 

drm_mode_connector_set_tile_property(port->connector);

here:

> +               } else if (port->pdt == DP_PEER_DEVICE_SST_SINK ||
> +                       port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV) {
> +                       if (!port->cached_edid) {
> +                               port->cached_edid =
> +                                       drm_get_edid(port->connector, 
> + &port->aux.ddc);
	                     drm_mode_connector_set_tile_property(port->connector);
> +                       }

I will test with tile display to see if it solves the issue.

Thanks,
Mykola


-----Original Message-----
From: Dave Airlie [mailto:airlied@gmail.com]
Sent: Wednesday, February 17, 2016 1:46 PM
To: Wentland, Harry <Harry.Wentland@amd.com>
Cc: dri-devel <dri-devel@lists.freedesktop.org>; Lysenko, Mykola <Mykola.Lysenko@amd.com>
Subject: Re: [PATCH 3/5] drm/dp/mst: change MST detection scheme

On 23 January 2016 at 08:07, Harry Wentland <harry.wentland@amd.com> wrote:
> From: Mykola Lysenko <Mykola.Lysenko@amd.com>
>
> 1. Get edid for all connected MST displays, not only on logical ports,
>    in the same thread as MST topology detection is done:
>      There are displays that have branches inside w/o logical ports.
>      So in case another SST display connected downstream system can
>      end-up in situation when 3 DOWN requests sent: two for
>     ‘remote i2c read’ and one for ‘enum path resources’, making slots full.
>
> 2. Call notification callback in one place in the end of topology discovery/update:
>      This is done to reduce number of events sent to userspace in case complex
>      topology discovery is going, adding multiple number of 
> connectors;
>
> 3. Remove notification callback call from short pulse interrupt processing function:
>      This is done in order not to block interrupt processing function, in case any
>      MST request will be made from it. Notification will be send from topology
>      discovery/update work item.

I've had to pull this out, as I did some more indepth testing with
i915 and a Dell 30"
and this broke things.

The main thing it broke is setting the tiling property that userspace needs to see those dual-panel monitors as one.

you should be able to see xrandr --props if the tile property is correct if you test.

I'm also not sure about some other bits in this patch, so I probably need to dig a bit deeper into it.

Dave.

>
> Signed-off-by: Mykola Lysenko <Mykola.Lysenko@amd.com>
> Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 37 
> ++++++++++++++++++-----------------
>  1 file changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 041597b7a7c6..052c20ca35ee 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1130,13 +1130,11 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
>                         drm_dp_put_port(port);
>                         goto out;
>                 }
> -               if (port->port_num >= DP_MST_LOGICAL_PORT_0) {
> -                       port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc);
> -                       drm_mode_connector_set_tile_property(port->connector);
> -               }
> +
> +               drm_mode_connector_set_tile_property(port->connector);
> +
>                 (*mstb->mgr->cbs->register_connector)(port->connector);
>         }
> -
>  out:
>         /* put reference to this port */
>         drm_dp_put_port(port);
> @@ -1161,9 +1159,9 @@ static void drm_dp_update_port(struct drm_dp_mst_branch *mstb,
>         port->ddps = conn_stat->displayport_device_plug_status;
>
>         if (old_ddps != port->ddps) {
> +               dowork = true;
>                 if (port->ddps) {
>                         drm_dp_check_port_guid(mstb, port);
> -                       dowork = true;
>                 } else {
>                         port->guid_valid = false;
>                         port->available_pbn = 0; @@ -1271,8 +1269,13 
> @@ static void drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *m
>                 if (port->input)
>                         continue;
>
> -               if (!port->ddps)
> +               if (!port->ddps) {
> +                       if (port->cached_edid) {
> +                               kfree(port->cached_edid);
> +                               port->cached_edid = NULL;
> +                       }
>                         continue;
> +               }
>
>                 if (!port->available_pbn)
>                         drm_dp_send_enum_path_resources(mgr, mstb, 
> port); @@ -1283,6 +1286,12 @@ static void drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *m
>                                 drm_dp_check_and_send_link_address(mgr, mstb_child);
>                                 drm_dp_put_mst_branch_device(mstb_child);
>                         }
> +               } else if (port->pdt == DP_PEER_DEVICE_SST_SINK ||
> +                       port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV) {
> +                       if (!port->cached_edid) {
> +                               port->cached_edid =
> +                                       drm_get_edid(port->connector, &port->aux.ddc);
> +                       }
>                 }
>         }
>  }
> @@ -1302,6 +1311,8 @@ static void drm_dp_mst_link_probe_work(struct work_struct *work)
>                 drm_dp_check_and_send_link_address(mgr, mstb);
>                 drm_dp_put_mst_branch_device(mstb);
>         }
> +
> +       (*mgr->cbs->hotplug)(mgr);
>  }
>
>  static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr, 
> @@ -1558,7 +1569,6 @@ static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
>                         for (i = 0; i < txmsg->reply.u.link_addr.nports; i++) {
>                                 drm_dp_add_port(mstb, mgr->dev, &txmsg->reply.u.link_addr.ports[i]);
>                         }
> -                       (*mgr->cbs->hotplug)(mgr);
>                 }
>         } else {
>                 mstb->link_address_sent = false; @@ -2232,8 +2242,6 @@ 
> static int drm_dp_mst_handle_up_req(struct drm_dp_mst_topology_mgr 
> *mgr)
>
>                         drm_dp_update_port(mstb, &msg.u.conn_stat);
>                         DRM_DEBUG_KMS("Got CSN: pn: %d ldps:%d ddps: %d mcs: %d ip: %d pdt: %d\n", msg.u.conn_stat.port_number, msg.u.conn_stat.legacy_device_plug_status, msg.u.conn_stat.displayport_device_plug_status, msg.u.conn_stat.message_capability_status, msg.u.conn_stat.input_port, msg.u.conn_stat.peer_device_type);
> -                       (*mgr->cbs->hotplug)(mgr);
> -
>                 } else if (msg.req_type == DP_RESOURCE_STATUS_NOTIFY) {
>                         drm_dp_send_up_ack_reply(mgr, mgr->mst_primary, msg.req_type, seqno, false);
>                         if (!mstb)
> @@ -2320,10 +2328,6 @@ enum drm_connector_status 
> drm_dp_mst_detect_port(struct drm_connector *connector
>
>         case DP_PEER_DEVICE_SST_SINK:
>                 status = connector_status_connected;
> -               /* for logical ports - cache the EDID */
> -               if (port->port_num >= 8 && !port->cached_edid) {
> -                       port->cached_edid = drm_get_edid(connector, &port->aux.ddc);
> -               }
>                 break;
>         case DP_PEER_DEVICE_DP_LEGACY_CONV:
>                 if (port->ldps)
> @@ -2378,10 +2382,7 @@ struct edid *drm_dp_mst_get_edid(struct 
> drm_connector *connector, struct drm_dp_
>
>         if (port->cached_edid)
>                 edid = drm_edid_duplicate(port->cached_edid);
> -       else {
> -               edid = drm_get_edid(connector, &port->aux.ddc);
> -               drm_mode_connector_set_tile_property(connector);
> -       }
> +
>         port->has_audio = drm_detect_monitor_audio(edid);
>         drm_dp_put_port(port);
>         return edid;
> --
> 2.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[-- Attachment #2: 0001-drm-dp-mst-update-tile-property-after-edid-read.patch --]
[-- Type: application/octet-stream, Size: 1160 bytes --]

From 999795026a4ac6b61fdb06d000fb295b65f21261 Mon Sep 17 00:00:00 2001
From: Mykola Lysenko <Mykola.Lysenko@amd.com>
Date: Mon, 22 Feb 2016 21:43:32 -0500
Subject: drm/dp/mst: update tile property after edid read

Fix of tiled display issue cased by previous commit

Signed-off-by: Mykola Lysenko <Mykola.Lysenko@amd.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index c27f448..b91360d 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1141,8 +1141,6 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
 			goto out;
 		}
 
-		drm_mode_connector_set_tile_property(port->connector);
-
 		(*mstb->mgr->cbs->register_connector)(port->connector);
 	}
 out:
@@ -1297,6 +1295,7 @@ static void drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *m
 			if (!port->cached_edid) {
 				port->cached_edid =
 					drm_get_edid(port->connector, &port->aux.ddc);
+				drm_mode_connector_set_tile_property(port->connector);
 			}
 		}
 	}
-- 
1.9.1


[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* RE: [PATCH 3/5] drm/dp/mst: change MST detection scheme
  2016-02-23  3:09     ` Lysenko, Mykola
@ 2016-05-12 22:41       ` Grodzovsky, Andrey
  2016-06-15  3:49         ` Dave Airlie
  0 siblings, 1 reply; 22+ messages in thread
From: Grodzovsky, Andrey @ 2016-05-12 22:41 UTC (permalink / raw)
  To: Dave Airlie; +Cc: Lysenko, Mykola, dri-devel

Hi Dave,

Have you had a chance to see if Mykola's latest patch addresses the issue you observed with tiled MST display ?

Thanks.
-----Original Message-----
From: Lysenko, Mykola 
Sent: Monday, February 22, 2016 10:09 PM
To: Dave Airlie; Wentland, Harry
Cc: dri-devel
Subject: RE: [PATCH 3/5] drm/dp/mst: change MST detection scheme

Hi Dave,

Attached patch should fix the issue.

I have one question though: should we add drm_mode_connector_set_tile_property to the drm_get_displayid function, so it will be updated in case of regular DP displays as well?

As an example of such usage could be 5k displays with two tiles 2560x2880 that could not be driven on one MST connector because of bandwidth limitations, and are driven in SST mode.

Thanks,
Mykola

-----Original Message-----
From: Lysenko, Mykola
Sent: Thursday, February 18, 2016 12:53 PM
To: 'Dave Airlie' <airlied@gmail.com>; Wentland, Harry <Harry.Wentland@amd.com>
Cc: dri-devel <dri-devel@lists.freedesktop.org>
Subject: RE: [PATCH 3/5] drm/dp/mst: change MST detection scheme

Hi Dave,

it seems to be missed call to 

drm_mode_connector_set_tile_property(port->connector);

here:

> +               } else if (port->pdt == DP_PEER_DEVICE_SST_SINK ||
> +                       port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV) {
> +                       if (!port->cached_edid) {
> +                               port->cached_edid =
> +                                       drm_get_edid(port->connector, 
> + &port->aux.ddc);
	                     drm_mode_connector_set_tile_property(port->connector);
> +                       }

I will test with tile display to see if it solves the issue.

Thanks,
Mykola


-----Original Message-----
From: Dave Airlie [mailto:airlied@gmail.com]
Sent: Wednesday, February 17, 2016 1:46 PM
To: Wentland, Harry <Harry.Wentland@amd.com>
Cc: dri-devel <dri-devel@lists.freedesktop.org>; Lysenko, Mykola <Mykola.Lysenko@amd.com>
Subject: Re: [PATCH 3/5] drm/dp/mst: change MST detection scheme

On 23 January 2016 at 08:07, Harry Wentland <harry.wentland@amd.com> wrote:
> From: Mykola Lysenko <Mykola.Lysenko@amd.com>
>
> 1. Get edid for all connected MST displays, not only on logical ports,
>    in the same thread as MST topology detection is done:
>      There are displays that have branches inside w/o logical ports.
>      So in case another SST display connected downstream system can
>      end-up in situation when 3 DOWN requests sent: two for
>     ‘remote i2c read’ and one for ‘enum path resources’, making slots full.
>
> 2. Call notification callback in one place in the end of topology discovery/update:
>      This is done to reduce number of events sent to userspace in case complex
>      topology discovery is going, adding multiple number of 
> connectors;
>
> 3. Remove notification callback call from short pulse interrupt processing function:
>      This is done in order not to block interrupt processing function, in case any
>      MST request will be made from it. Notification will be send from topology
>      discovery/update work item.

I've had to pull this out, as I did some more indepth testing with
i915 and a Dell 30"
and this broke things.

The main thing it broke is setting the tiling property that userspace needs to see those dual-panel monitors as one.

you should be able to see xrandr --props if the tile property is correct if you test.

I'm also not sure about some other bits in this patch, so I probably need to dig a bit deeper into it.

Dave.

>
> Signed-off-by: Mykola Lysenko <Mykola.Lysenko@amd.com>
> Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 37
> ++++++++++++++++++-----------------
>  1 file changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 041597b7a7c6..052c20ca35ee 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1130,13 +1130,11 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
>                         drm_dp_put_port(port);
>                         goto out;
>                 }
> -               if (port->port_num >= DP_MST_LOGICAL_PORT_0) {
> -                       port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc);
> -                       drm_mode_connector_set_tile_property(port->connector);
> -               }
> +
> +               drm_mode_connector_set_tile_property(port->connector);
> +
>                 (*mstb->mgr->cbs->register_connector)(port->connector);
>         }
> -
>  out:
>         /* put reference to this port */
>         drm_dp_put_port(port);
> @@ -1161,9 +1159,9 @@ static void drm_dp_update_port(struct drm_dp_mst_branch *mstb,
>         port->ddps = conn_stat->displayport_device_plug_status;
>
>         if (old_ddps != port->ddps) {
> +               dowork = true;
>                 if (port->ddps) {
>                         drm_dp_check_port_guid(mstb, port);
> -                       dowork = true;
>                 } else {
>                         port->guid_valid = false;
>                         port->available_pbn = 0; @@ -1271,8 +1269,13 
> @@ static void drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *m
>                 if (port->input)
>                         continue;
>
> -               if (!port->ddps)
> +               if (!port->ddps) {
> +                       if (port->cached_edid) {
> +                               kfree(port->cached_edid);
> +                               port->cached_edid = NULL;
> +                       }
>                         continue;
> +               }
>
>                 if (!port->available_pbn)
>                         drm_dp_send_enum_path_resources(mgr, mstb, 
> port); @@ -1283,6 +1286,12 @@ static void drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *m
>                                 drm_dp_check_and_send_link_address(mgr, mstb_child);
>                                 drm_dp_put_mst_branch_device(mstb_child);
>                         }
> +               } else if (port->pdt == DP_PEER_DEVICE_SST_SINK ||
> +                       port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV) {
> +                       if (!port->cached_edid) {
> +                               port->cached_edid =
> +                                       drm_get_edid(port->connector, &port->aux.ddc);
> +                       }
>                 }
>         }
>  }
> @@ -1302,6 +1311,8 @@ static void drm_dp_mst_link_probe_work(struct work_struct *work)
>                 drm_dp_check_and_send_link_address(mgr, mstb);
>                 drm_dp_put_mst_branch_device(mstb);
>         }
> +
> +       (*mgr->cbs->hotplug)(mgr);
>  }
>
>  static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr, 
> @@ -1558,7 +1569,6 @@ static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
>                         for (i = 0; i < txmsg->reply.u.link_addr.nports; i++) {
>                                 drm_dp_add_port(mstb, mgr->dev, &txmsg->reply.u.link_addr.ports[i]);
>                         }
> -                       (*mgr->cbs->hotplug)(mgr);
>                 }
>         } else {
>                 mstb->link_address_sent = false; @@ -2232,8 +2242,6 @@ 
> static int drm_dp_mst_handle_up_req(struct drm_dp_mst_topology_mgr
> *mgr)
>
>                         drm_dp_update_port(mstb, &msg.u.conn_stat);
>                         DRM_DEBUG_KMS("Got CSN: pn: %d ldps:%d ddps: %d mcs: %d ip: %d pdt: %d\n", msg.u.conn_stat.port_number, msg.u.conn_stat.legacy_device_plug_status, msg.u.conn_stat.displayport_device_plug_status, msg.u.conn_stat.message_capability_status, msg.u.conn_stat.input_port, msg.u.conn_stat.peer_device_type);
> -                       (*mgr->cbs->hotplug)(mgr);
> -
>                 } else if (msg.req_type == DP_RESOURCE_STATUS_NOTIFY) {
>                         drm_dp_send_up_ack_reply(mgr, mgr->mst_primary, msg.req_type, seqno, false);
>                         if (!mstb)
> @@ -2320,10 +2328,6 @@ enum drm_connector_status 
> drm_dp_mst_detect_port(struct drm_connector *connector
>
>         case DP_PEER_DEVICE_SST_SINK:
>                 status = connector_status_connected;
> -               /* for logical ports - cache the EDID */
> -               if (port->port_num >= 8 && !port->cached_edid) {
> -                       port->cached_edid = drm_get_edid(connector, &port->aux.ddc);
> -               }
>                 break;
>         case DP_PEER_DEVICE_DP_LEGACY_CONV:
>                 if (port->ldps)
> @@ -2378,10 +2382,7 @@ struct edid *drm_dp_mst_get_edid(struct 
> drm_connector *connector, struct drm_dp_
>
>         if (port->cached_edid)
>                 edid = drm_edid_duplicate(port->cached_edid);
> -       else {
> -               edid = drm_get_edid(connector, &port->aux.ddc);
> -               drm_mode_connector_set_tile_property(connector);
> -       }
> +
>         port->has_audio = drm_detect_monitor_audio(edid);
>         drm_dp_put_port(port);
>         return edid;
> --
> 2.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/5] drm/dp/mst: change MST detection scheme
  2016-05-12 22:41       ` Grodzovsky, Andrey
@ 2016-06-15  3:49         ` Dave Airlie
  2016-06-15  4:14           ` Dave Airlie
  0 siblings, 1 reply; 22+ messages in thread
From: Dave Airlie @ 2016-06-15  3:49 UTC (permalink / raw)
  To: Grodzovsky, Andrey; +Cc: Lysenko, Mykola, dri-devel

Excuse me for top posting.

So I finally got back to this patch, still don't like it.

Apart from the doing 3 things in once which is just annoying, current
userspace for tiled monitors
relies on the behaviour that the cached edids are retrieved before the
connector is show to userspace.

This is so that for tiled monitors both connectors are shown with
tiling properties so userspace can pick the correct monitor size.

Now I realise this isn't great, since at least for the two cable 5K
monitors you won't ever plug them in at once.

So we should probably discuss more what userspace should do in the
presence of a tiled monitor where it can only see one tile,

If we go ahead with something like this, things will look kinda ugly
as you plug in a 4k monitor and it'll try and set a mode
on the first tile when it gets the hotplug (or it could be racing and
notice the connectors before the hotplug). So it then
sets the 1920x1080 mode on one connector and gets another hotplug to
set the tiled mode up.

Dave.

On 13 May 2016 at 08:41, Grodzovsky, Andrey <Andrey.Grodzovsky@amd.com> wrote:
> Hi Dave,
>
> Have you had a chance to see if Mykola's latest patch addresses the issue you observed with tiled MST display ?
>
> Thanks.
> -----Original Message-----
> From: Lysenko, Mykola
> Sent: Monday, February 22, 2016 10:09 PM
> To: Dave Airlie; Wentland, Harry
> Cc: dri-devel
> Subject: RE: [PATCH 3/5] drm/dp/mst: change MST detection scheme
>
> Hi Dave,
>
> Attached patch should fix the issue.
>
> I have one question though: should we add drm_mode_connector_set_tile_property to the drm_get_displayid function, so it will be updated in case of regular DP displays as well?
>
> As an example of such usage could be 5k displays with two tiles 2560x2880 that could not be driven on one MST connector because of bandwidth limitations, and are driven in SST mode.
>
> Thanks,
> Mykola
>
> -----Original Message-----
> From: Lysenko, Mykola
> Sent: Thursday, February 18, 2016 12:53 PM
> To: 'Dave Airlie' <airlied@gmail.com>; Wentland, Harry <Harry.Wentland@amd.com>
> Cc: dri-devel <dri-devel@lists.freedesktop.org>
> Subject: RE: [PATCH 3/5] drm/dp/mst: change MST detection scheme
>
> Hi Dave,
>
> it seems to be missed call to
>
> drm_mode_connector_set_tile_property(port->connector);
>
> here:
>
>> +               } else if (port->pdt == DP_PEER_DEVICE_SST_SINK ||
>> +                       port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV) {
>> +                       if (!port->cached_edid) {
>> +                               port->cached_edid =
>> +                                       drm_get_edid(port->connector,
>> + &port->aux.ddc);
>                              drm_mode_connector_set_tile_property(port->connector);
>> +                       }
>
> I will test with tile display to see if it solves the issue.
>
> Thanks,
> Mykola
>
>
> -----Original Message-----
> From: Dave Airlie [mailto:airlied@gmail.com]
> Sent: Wednesday, February 17, 2016 1:46 PM
> To: Wentland, Harry <Harry.Wentland@amd.com>
> Cc: dri-devel <dri-devel@lists.freedesktop.org>; Lysenko, Mykola <Mykola.Lysenko@amd.com>
> Subject: Re: [PATCH 3/5] drm/dp/mst: change MST detection scheme
>
> On 23 January 2016 at 08:07, Harry Wentland <harry.wentland@amd.com> wrote:
>> From: Mykola Lysenko <Mykola.Lysenko@amd.com>
>>
>> 1. Get edid for all connected MST displays, not only on logical ports,
>>    in the same thread as MST topology detection is done:
>>      There are displays that have branches inside w/o logical ports.
>>      So in case another SST display connected downstream system can
>>      end-up in situation when 3 DOWN requests sent: two for
>>     ‘remote i2c read’ and one for ‘enum path resources’, making slots full.
>>
>> 2. Call notification callback in one place in the end of topology discovery/update:
>>      This is done to reduce number of events sent to userspace in case complex
>>      topology discovery is going, adding multiple number of
>> connectors;
>>
>> 3. Remove notification callback call from short pulse interrupt processing function:
>>      This is done in order not to block interrupt processing function, in case any
>>      MST request will be made from it. Notification will be send from topology
>>      discovery/update work item.
>
> I've had to pull this out, as I did some more indepth testing with
> i915 and a Dell 30"
> and this broke things.
>
> The main thing it broke is setting the tiling property that userspace needs to see those dual-panel monitors as one.
>
> you should be able to see xrandr --props if the tile property is correct if you test.
>
> I'm also not sure about some other bits in this patch, so I probably need to dig a bit deeper into it.
>
> Dave.
>
>>
>> Signed-off-by: Mykola Lysenko <Mykola.Lysenko@amd.com>
>> Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
>> Acked-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>>  drivers/gpu/drm/drm_dp_mst_topology.c | 37
>> ++++++++++++++++++-----------------
>>  1 file changed, 19 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
>> b/drivers/gpu/drm/drm_dp_mst_topology.c
>> index 041597b7a7c6..052c20ca35ee 100644
>> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
>> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
>> @@ -1130,13 +1130,11 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
>>                         drm_dp_put_port(port);
>>                         goto out;
>>                 }
>> -               if (port->port_num >= DP_MST_LOGICAL_PORT_0) {
>> -                       port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc);
>> -                       drm_mode_connector_set_tile_property(port->connector);
>> -               }
>> +
>> +               drm_mode_connector_set_tile_property(port->connector);
>> +
>>                 (*mstb->mgr->cbs->register_connector)(port->connector);
>>         }
>> -
>>  out:
>>         /* put reference to this port */
>>         drm_dp_put_port(port);
>> @@ -1161,9 +1159,9 @@ static void drm_dp_update_port(struct drm_dp_mst_branch *mstb,
>>         port->ddps = conn_stat->displayport_device_plug_status;
>>
>>         if (old_ddps != port->ddps) {
>> +               dowork = true;
>>                 if (port->ddps) {
>>                         drm_dp_check_port_guid(mstb, port);
>> -                       dowork = true;
>>                 } else {
>>                         port->guid_valid = false;
>>                         port->available_pbn = 0; @@ -1271,8 +1269,13
>> @@ static void drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *m
>>                 if (port->input)
>>                         continue;
>>
>> -               if (!port->ddps)
>> +               if (!port->ddps) {
>> +                       if (port->cached_edid) {
>> +                               kfree(port->cached_edid);
>> +                               port->cached_edid = NULL;
>> +                       }
>>                         continue;
>> +               }
>>
>>                 if (!port->available_pbn)
>>                         drm_dp_send_enum_path_resources(mgr, mstb,
>> port); @@ -1283,6 +1286,12 @@ static void drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *m
>>                                 drm_dp_check_and_send_link_address(mgr, mstb_child);
>>                                 drm_dp_put_mst_branch_device(mstb_child);
>>                         }
>> +               } else if (port->pdt == DP_PEER_DEVICE_SST_SINK ||
>> +                       port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV) {
>> +                       if (!port->cached_edid) {
>> +                               port->cached_edid =
>> +                                       drm_get_edid(port->connector, &port->aux.ddc);
>> +                       }
>>                 }
>>         }
>>  }
>> @@ -1302,6 +1311,8 @@ static void drm_dp_mst_link_probe_work(struct work_struct *work)
>>                 drm_dp_check_and_send_link_address(mgr, mstb);
>>                 drm_dp_put_mst_branch_device(mstb);
>>         }
>> +
>> +       (*mgr->cbs->hotplug)(mgr);
>>  }
>>
>>  static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
>> @@ -1558,7 +1569,6 @@ static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
>>                         for (i = 0; i < txmsg->reply.u.link_addr.nports; i++) {
>>                                 drm_dp_add_port(mstb, mgr->dev, &txmsg->reply.u.link_addr.ports[i]);
>>                         }
>> -                       (*mgr->cbs->hotplug)(mgr);
>>                 }
>>         } else {
>>                 mstb->link_address_sent = false; @@ -2232,8 +2242,6 @@
>> static int drm_dp_mst_handle_up_req(struct drm_dp_mst_topology_mgr
>> *mgr)
>>
>>                         drm_dp_update_port(mstb, &msg.u.conn_stat);
>>                         DRM_DEBUG_KMS("Got CSN: pn: %d ldps:%d ddps: %d mcs: %d ip: %d pdt: %d\n", msg.u.conn_stat.port_number, msg.u.conn_stat.legacy_device_plug_status, msg.u.conn_stat.displayport_device_plug_status, msg.u.conn_stat.message_capability_status, msg.u.conn_stat.input_port, msg.u.conn_stat.peer_device_type);
>> -                       (*mgr->cbs->hotplug)(mgr);
>> -
>>                 } else if (msg.req_type == DP_RESOURCE_STATUS_NOTIFY) {
>>                         drm_dp_send_up_ack_reply(mgr, mgr->mst_primary, msg.req_type, seqno, false);
>>                         if (!mstb)
>> @@ -2320,10 +2328,6 @@ enum drm_connector_status
>> drm_dp_mst_detect_port(struct drm_connector *connector
>>
>>         case DP_PEER_DEVICE_SST_SINK:
>>                 status = connector_status_connected;
>> -               /* for logical ports - cache the EDID */
>> -               if (port->port_num >= 8 && !port->cached_edid) {
>> -                       port->cached_edid = drm_get_edid(connector, &port->aux.ddc);
>> -               }
>>                 break;
>>         case DP_PEER_DEVICE_DP_LEGACY_CONV:
>>                 if (port->ldps)
>> @@ -2378,10 +2382,7 @@ struct edid *drm_dp_mst_get_edid(struct
>> drm_connector *connector, struct drm_dp_
>>
>>         if (port->cached_edid)
>>                 edid = drm_edid_duplicate(port->cached_edid);
>> -       else {
>> -               edid = drm_get_edid(connector, &port->aux.ddc);
>> -               drm_mode_connector_set_tile_property(connector);
>> -       }
>> +
>>         port->has_audio = drm_detect_monitor_audio(edid);
>>         drm_dp_put_port(port);
>>         return edid;
>> --
>> 2.1.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/5] drm/dp/mst: change MST detection scheme
  2016-06-15  3:49         ` Dave Airlie
@ 2016-06-15  4:14           ` Dave Airlie
  2017-01-25 17:16             ` Krzysztof Nowicki
  0 siblings, 1 reply; 22+ messages in thread
From: Dave Airlie @ 2016-06-15  4:14 UTC (permalink / raw)
  To: Grodzovsky, Andrey; +Cc: Lysenko, Mykola, dri-devel

On 15 June 2016 at 13:49, Dave Airlie <airlied@gmail.com> wrote:
> Excuse me for top posting.
>
> So I finally got back to this patch, still don't like it.
>
> Apart from the doing 3 things in once which is just annoying, current
> userspace for tiled monitors
> relies on the behaviour that the cached edids are retrieved before the
> connector is show to userspace.
>
> This is so that for tiled monitors both connectors are shown with
> tiling properties so userspace can pick the correct monitor size.
>
> Now I realise this isn't great, since at least for the two cable 5K
> monitors you won't ever plug them in at once.
>
> So we should probably discuss more what userspace should do in the
> presence of a tiled monitor where it can only see one tile,
>
> If we go ahead with something like this, things will look kinda ugly
> as you plug in a 4k monitor and it'll try and set a mode
> on the first tile when it gets the hotplug (or it could be racing and
> notice the connectors before the hotplug). So it then
> sets the 1920x1080 mode on one connector and gets another hotplug to
> set the tiled mode up.

Though I think the fix is to leave the cached edid test in add port,

And maybe the rest of the patch is okay then.

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/5] drm/dp/mst: change MST detection scheme
  2016-06-15  4:14           ` Dave Airlie
@ 2017-01-25 17:16             ` Krzysztof Nowicki
  2017-01-25 17:24               ` Alex Deucher
  0 siblings, 1 reply; 22+ messages in thread
From: Krzysztof Nowicki @ 2017-01-25 17:16 UTC (permalink / raw)
  To: dri-devel

Hi,

On środa, 15 czerwca 2016 14:14:01 CET  wrote:
> On 15 June 2016 at 13:49, Dave Airlie <airlied at gmail.com> wrote:
> > Excuse me for top posting.
> > 
> > So I finally got back to this patch, still don't like it.
> > 
> > Apart from the doing 3 things in once which is just annoying, current
> > userspace for tiled monitors
> > relies on the behaviour that the cached edids are retrieved before the
> > connector is show to userspace.
> > 
> > This is so that for tiled monitors both connectors are shown with
> > tiling properties so userspace can pick the correct monitor size.
> > 
> > Now I realise this isn't great, since at least for the two cable 5K
> > monitors you won't ever plug them in at once.
> > 
> > So we should probably discuss more what userspace should do in the
> > presence of a tiled monitor where it can only see one tile,
> > 
> > If we go ahead with something like this, things will look kinda ugly
> > as you plug in a 4k monitor and it'll try and set a mode
> > on the first tile when it gets the hotplug (or it could be racing and
> > notice the connectors before the hotplug). So it then
> > sets the 1920x1080 mode on one connector and gets another hotplug to
> > set the tiled mode up.
> 
> Though I think the fix is to leave the cached edid test in add port,
> 
> And maybe the rest of the patch is okay then.
> 
> Dave.

Is this patch still being considered for pulling in?

Recently I've been fighting with a HP UltraSlim docking station with a MST hub 
inside in order to get it working with the amdgpu DAL/DC code. I couldn't get 
it to work until I stumbled upon this patch. Applying it solved the issue 
instantly.

What I have found during debugging is that the MST hub would return SST sink 
port numbers < 8 which resulted in no EDID being retrieved and in turn the 
status of the outputs was 'disconnected'.

Regards
Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/5] drm/dp/mst: change MST detection scheme
  2017-01-25 17:16             ` Krzysztof Nowicki
@ 2017-01-25 17:24               ` Alex Deucher
  2017-01-25 18:32                 ` Harry Wentland
  2017-01-27 18:21                 ` Krzysztof Nowicki
  0 siblings, 2 replies; 22+ messages in thread
From: Alex Deucher @ 2017-01-25 17:24 UTC (permalink / raw)
  To: Krzysztof Nowicki, Wentland, Harry; +Cc: Maling list - DRI developers

On Wed, Jan 25, 2017 at 12:16 PM, Krzysztof Nowicki
<krzysztof.a.nowicki@gmail.com> wrote:
> Hi,
>
> On środa, 15 czerwca 2016 14:14:01 CET  wrote:
>> On 15 June 2016 at 13:49, Dave Airlie <airlied at gmail.com> wrote:
>> > Excuse me for top posting.
>> >
>> > So I finally got back to this patch, still don't like it.
>> >
>> > Apart from the doing 3 things in once which is just annoying, current
>> > userspace for tiled monitors
>> > relies on the behaviour that the cached edids are retrieved before the
>> > connector is show to userspace.
>> >
>> > This is so that for tiled monitors both connectors are shown with
>> > tiling properties so userspace can pick the correct monitor size.
>> >
>> > Now I realise this isn't great, since at least for the two cable 5K
>> > monitors you won't ever plug them in at once.
>> >
>> > So we should probably discuss more what userspace should do in the
>> > presence of a tiled monitor where it can only see one tile,
>> >
>> > If we go ahead with something like this, things will look kinda ugly
>> > as you plug in a 4k monitor and it'll try and set a mode
>> > on the first tile when it gets the hotplug (or it could be racing and
>> > notice the connectors before the hotplug). So it then
>> > sets the 1920x1080 mode on one connector and gets another hotplug to
>> > set the tiled mode up.
>>
>> Though I think the fix is to leave the cached edid test in add port,
>>
>> And maybe the rest of the patch is okay then.
>>
>> Dave.
>
> Is this patch still being considered for pulling in?
>
> Recently I've been fighting with a HP UltraSlim docking station with a MST hub
> inside in order to get it working with the amdgpu DAL/DC code. I couldn't get
> it to work until I stumbled upon this patch. Applying it solved the issue
> instantly.
>
> What I have found during debugging is that the MST hub would return SST sink
> port numbers < 8 which resulted in no EDID being retrieved and in turn the
> status of the outputs was 'disconnected'.

Harry,

Any chance you could re-spin this against something more recent and re-send?

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/5] drm/dp/mst: change MST detection scheme
  2017-01-25 17:24               ` Alex Deucher
@ 2017-01-25 18:32                 ` Harry Wentland
  2017-01-25 22:46                   ` Alex Deucher
  2017-01-27 18:21                 ` Krzysztof Nowicki
  1 sibling, 1 reply; 22+ messages in thread
From: Harry Wentland @ 2017-01-25 18:32 UTC (permalink / raw)
  To: Alex Deucher, Krzysztof Nowicki; +Cc: Maling list - DRI developers

You mean rebase dal/dc on drm-next with these patches?

I'll see if I find some time today or tomorrow to do that. We'll
probably need that anyways for our atomic rework.

Do you still rebase amd-staging on drm-next?
https://cgit.freedesktop.org/~agd5f/linux/log/?h=amd-staging-drm-next
looks a few weeks old now.

Harry

On 2017-01-25 12:24 PM, Alex Deucher wrote:
> On Wed, Jan 25, 2017 at 12:16 PM, Krzysztof Nowicki
> <krzysztof.a.nowicki@gmail.com> wrote:
>> Hi,
>>
>> On środa, 15 czerwca 2016 14:14:01 CET  wrote:
>>> On 15 June 2016 at 13:49, Dave Airlie <airlied at gmail.com> wrote:
>>>> Excuse me for top posting.
>>>>
>>>> So I finally got back to this patch, still don't like it.
>>>>
>>>> Apart from the doing 3 things in once which is just annoying, current
>>>> userspace for tiled monitors
>>>> relies on the behaviour that the cached edids are retrieved before the
>>>> connector is show to userspace.
>>>>
>>>> This is so that for tiled monitors both connectors are shown with
>>>> tiling properties so userspace can pick the correct monitor size.
>>>>
>>>> Now I realise this isn't great, since at least for the two cable 5K
>>>> monitors you won't ever plug them in at once.
>>>>
>>>> So we should probably discuss more what userspace should do in the
>>>> presence of a tiled monitor where it can only see one tile,
>>>>
>>>> If we go ahead with something like this, things will look kinda ugly
>>>> as you plug in a 4k monitor and it'll try and set a mode
>>>> on the first tile when it gets the hotplug (or it could be racing and
>>>> notice the connectors before the hotplug). So it then
>>>> sets the 1920x1080 mode on one connector and gets another hotplug to
>>>> set the tiled mode up.
>>> Though I think the fix is to leave the cached edid test in add port,
>>>
>>> And maybe the rest of the patch is okay then.
>>>
>>> Dave.
>> Is this patch still being considered for pulling in?
>>
>> Recently I've been fighting with a HP UltraSlim docking station with a MST hub
>> inside in order to get it working with the amdgpu DAL/DC code. I couldn't get
>> it to work until I stumbled upon this patch. Applying it solved the issue
>> instantly.
>>
>> What I have found during debugging is that the MST hub would return SST sink
>> port numbers < 8 which resulted in no EDID being retrieved and in turn the
>> status of the outputs was 'disconnected'.
> Harry,
>
> Any chance you could re-spin this against something more recent and re-send?
>
> Alex

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/5] drm/dp/mst: change MST detection scheme
  2017-01-25 18:32                 ` Harry Wentland
@ 2017-01-25 22:46                   ` Alex Deucher
  2017-01-25 23:07                     ` Harry Wentland
  0 siblings, 1 reply; 22+ messages in thread
From: Alex Deucher @ 2017-01-25 22:46 UTC (permalink / raw)
  To: Harry Wentland; +Cc: Krzysztof Nowicki, Maling list - DRI developers

On Wed, Jan 25, 2017 at 1:32 PM, Harry Wentland <harry.wentland@amd.com> wrote:
> You mean rebase dal/dc on drm-next with these patches?

This is a core MST fix, so it would be good to get upstream for all drivers.

>
> I'll see if I find some time today or tomorrow to do that. We'll
> probably need that anyways for our atomic rework.
>
> Do you still rebase amd-staging on drm-next?
> https://cgit.freedesktop.org/~agd5f/linux/log/?h=amd-staging-drm-next
> looks a few weeks old now.

I haven't had a chance to rebase that branch in quite a while.

Alex

>
> Harry
>
> On 2017-01-25 12:24 PM, Alex Deucher wrote:
>> On Wed, Jan 25, 2017 at 12:16 PM, Krzysztof Nowicki
>> <krzysztof.a.nowicki@gmail.com> wrote:
>>> Hi,
>>>
>>> On środa, 15 czerwca 2016 14:14:01 CET  wrote:
>>>> On 15 June 2016 at 13:49, Dave Airlie <airlied at gmail.com> wrote:
>>>>> Excuse me for top posting.
>>>>>
>>>>> So I finally got back to this patch, still don't like it.
>>>>>
>>>>> Apart from the doing 3 things in once which is just annoying, current
>>>>> userspace for tiled monitors
>>>>> relies on the behaviour that the cached edids are retrieved before the
>>>>> connector is show to userspace.
>>>>>
>>>>> This is so that for tiled monitors both connectors are shown with
>>>>> tiling properties so userspace can pick the correct monitor size.
>>>>>
>>>>> Now I realise this isn't great, since at least for the two cable 5K
>>>>> monitors you won't ever plug them in at once.
>>>>>
>>>>> So we should probably discuss more what userspace should do in the
>>>>> presence of a tiled monitor where it can only see one tile,
>>>>>
>>>>> If we go ahead with something like this, things will look kinda ugly
>>>>> as you plug in a 4k monitor and it'll try and set a mode
>>>>> on the first tile when it gets the hotplug (or it could be racing and
>>>>> notice the connectors before the hotplug). So it then
>>>>> sets the 1920x1080 mode on one connector and gets another hotplug to
>>>>> set the tiled mode up.
>>>> Though I think the fix is to leave the cached edid test in add port,
>>>>
>>>> And maybe the rest of the patch is okay then.
>>>>
>>>> Dave.
>>> Is this patch still being considered for pulling in?
>>>
>>> Recently I've been fighting with a HP UltraSlim docking station with a MST hub
>>> inside in order to get it working with the amdgpu DAL/DC code. I couldn't get
>>> it to work until I stumbled upon this patch. Applying it solved the issue
>>> instantly.
>>>
>>> What I have found during debugging is that the MST hub would return SST sink
>>> port numbers < 8 which resulted in no EDID being retrieved and in turn the
>>> status of the outputs was 'disconnected'.
>> Harry,
>>
>> Any chance you could re-spin this against something more recent and re-send?
>>
>> Alex
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/5] drm/dp/mst: change MST detection scheme
  2017-01-25 22:46                   ` Alex Deucher
@ 2017-01-25 23:07                     ` Harry Wentland
  0 siblings, 0 replies; 22+ messages in thread
From: Harry Wentland @ 2017-01-25 23:07 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Krzysztof Nowicki, Maling list - DRI developers

On 2017-01-25 05:46 PM, Alex Deucher wrote:
> On Wed, Jan 25, 2017 at 1:32 PM, Harry Wentland <harry.wentland@amd.com> wrote:
>> You mean rebase dal/dc on drm-next with these patches?
> 
> This is a core MST fix, so it would be good to get upstream for all drivers.
> 

Ignore my previous message. Somehow I though this was something else.

It didn't make it upstream because it broke tiled display. We'll need to
find some time to take a second look. It's definitely still on our list.

Harry


>>
>> I'll see if I find some time today or tomorrow to do that. We'll
>> probably need that anyways for our atomic rework.
>>
>> Do you still rebase amd-staging on drm-next?
>> https://cgit.freedesktop.org/~agd5f/linux/log/?h=amd-staging-drm-next
>> looks a few weeks old now.
> 
> I haven't had a chance to rebase that branch in quite a while.
> 
> Alex
> 
>>
>> Harry
>>
>> On 2017-01-25 12:24 PM, Alex Deucher wrote:
>>> On Wed, Jan 25, 2017 at 12:16 PM, Krzysztof Nowicki
>>> <krzysztof.a.nowicki@gmail.com> wrote:
>>>> Hi,
>>>>
>>>> On środa, 15 czerwca 2016 14:14:01 CET  wrote:
>>>>> On 15 June 2016 at 13:49, Dave Airlie <airlied at gmail.com> wrote:
>>>>>> Excuse me for top posting.
>>>>>>
>>>>>> So I finally got back to this patch, still don't like it.
>>>>>>
>>>>>> Apart from the doing 3 things in once which is just annoying, current
>>>>>> userspace for tiled monitors
>>>>>> relies on the behaviour that the cached edids are retrieved before the
>>>>>> connector is show to userspace.
>>>>>>
>>>>>> This is so that for tiled monitors both connectors are shown with
>>>>>> tiling properties so userspace can pick the correct monitor size.
>>>>>>
>>>>>> Now I realise this isn't great, since at least for the two cable 5K
>>>>>> monitors you won't ever plug them in at once.
>>>>>>
>>>>>> So we should probably discuss more what userspace should do in the
>>>>>> presence of a tiled monitor where it can only see one tile,
>>>>>>
>>>>>> If we go ahead with something like this, things will look kinda ugly
>>>>>> as you plug in a 4k monitor and it'll try and set a mode
>>>>>> on the first tile when it gets the hotplug (or it could be racing and
>>>>>> notice the connectors before the hotplug). So it then
>>>>>> sets the 1920x1080 mode on one connector and gets another hotplug to
>>>>>> set the tiled mode up.
>>>>> Though I think the fix is to leave the cached edid test in add port,
>>>>>
>>>>> And maybe the rest of the patch is okay then.
>>>>>
>>>>> Dave.
>>>> Is this patch still being considered for pulling in?
>>>>
>>>> Recently I've been fighting with a HP UltraSlim docking station with a MST hub
>>>> inside in order to get it working with the amdgpu DAL/DC code. I couldn't get
>>>> it to work until I stumbled upon this patch. Applying it solved the issue
>>>> instantly.
>>>>
>>>> What I have found during debugging is that the MST hub would return SST sink
>>>> port numbers < 8 which resulted in no EDID being retrieved and in turn the
>>>> status of the outputs was 'disconnected'.
>>> Harry,
>>>
>>> Any chance you could re-spin this against something more recent and re-send?
>>>
>>> Alex
>>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/5] drm/dp/mst: change MST detection scheme
  2017-01-25 17:24               ` Alex Deucher
  2017-01-25 18:32                 ` Harry Wentland
@ 2017-01-27 18:21                 ` Krzysztof Nowicki
  1 sibling, 0 replies; 22+ messages in thread
From: Krzysztof Nowicki @ 2017-01-27 18:21 UTC (permalink / raw)
  To: dri-devel

On środa, 25 stycznia 2017 12:24:35 CET Alex Deucher wrote:
> On Wed, Jan 25, 2017 at 12:16 PM, Krzysztof Nowicki
> 
> <krzysztof.a.nowicki@gmail.com> wrote:
> > Hi,
> > 
> > 
> > Is this patch still being considered for pulling in?
> > 
> > Recently I've been fighting with a HP UltraSlim docking station with a MST
> > hub inside in order to get it working with the amdgpu DAL/DC code. I
> > couldn't get it to work until I stumbled upon this patch. Applying it
> > solved the issue instantly.
> > 
> > What I have found during debugging is that the MST hub would return SST
> > sink port numbers < 8 which resulted in no EDID being retrieved and in
> > turn the status of the outputs was 'disconnected'.
> 
> Harry,
> 
> Any chance you could re-spin this against something more recent and re-send?
> 
> Alex

Hi,

I have actually investigated this problem further and noticed that it's a 
problem in the amdgpu DAL/DC driver, which makes an incorrect assumption about 
the presence of a cached EDID. The fact that it started to work was only a 
side-effect of this patch.

I have submitted a patch to amd-gfx with what I believe is the proper fix.

Regards
Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2017-01-27 18:22 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-22 22:07 [PATCH 0/5] Fixes for MST (daisy-chain and 4k tiles) Harry Wentland
2016-01-22 22:07 ` [PATCH 1/5] drm: Add drm_fixp_from_fraction and drm_fixp2int_ceil Harry Wentland
2016-01-22 22:07 ` [PATCH 2/5] drm/dp/mst: Calculate MST PBN with 31.32 fixed point Harry Wentland
2016-01-22 22:07 ` [PATCH 3/5] drm/dp/mst: change MST detection scheme Harry Wentland
2016-02-17  5:45   ` Dave Airlie
2016-02-18  4:53     ` Lysenko, Mykola
2016-02-23  3:09     ` Lysenko, Mykola
2016-05-12 22:41       ` Grodzovsky, Andrey
2016-06-15  3:49         ` Dave Airlie
2016-06-15  4:14           ` Dave Airlie
2017-01-25 17:16             ` Krzysztof Nowicki
2017-01-25 17:24               ` Alex Deucher
2017-01-25 18:32                 ` Harry Wentland
2017-01-25 22:46                   ` Alex Deucher
2017-01-25 23:07                     ` Harry Wentland
2017-01-27 18:21                 ` Krzysztof Nowicki
2016-01-22 22:07 ` [PATCH 4/5] drm/dp/mst: move GUID storage from mgr, port to only mst branch Harry Wentland
2016-01-22 22:07 ` [PATCH 5/5] drm/dp/mst: Reverse order of MST enable and clearing VC payload table Harry Wentland
2016-01-25  4:53 ` [PATCH 0/5] Fixes for MST (daisy-chain and 4k tiles) Dave Airlie
2016-01-25 18:55   ` Wentland, Harry
2016-02-02 21:20     ` Alex Deucher
2016-02-04  0:35       ` Dave Airlie

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.