[PATCH 00/10] Enable MST Aux devices (v2)

[PATCH 00/10] Enable MST Aux devices (v2)

[sunpeng.li-5C7GfCeVMHo]

From: Leo Li <sunpeng.li@amd.com>

Hi all,

Here's the second revision of patches to enable mst aux devices.

v2 fixes an aux device unregistration issue during driver unload. See
patch 2/10 for details. Consequently, drivers supporting mst are
modified to use the new mst connector late register and early unregister
helpers.

Thanks,
Leo

Leo Li (9):
  drm/dp: Use non-cyclic idr
  drm/sysfs: Add mstpath attribute to connector devices
  drm/nouveau: Use connector kdev as aux device parent
  drm/bridge/analogix-anx78xx: Use connector kdev as aux device parent
  drm/amd/display: Use connector kdev as aux device parent
  drm/i915: Implement MST Aux device registration
  drm/nouveau/kms/nv50: Implement MST Aux device registration
  drm/radeon: Implement MST Aux device registration
  drm/amd/display: Implement MST Aux device registration

Ville Syrjälä (1):
  drm/dp_mst: Enable registration of AUX devices for MST ports

 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  26 +++-
 drivers/gpu/drm/bridge/analogix-anx78xx.c     |  22 +--
 drivers/gpu/drm/drm_dp_aux_dev.c              |  19 ++-
 drivers/gpu/drm/drm_dp_mst_topology.c         | 137 ++++++++++++++++--
 drivers/gpu/drm/drm_sysfs.c                   |  23 +++
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |  29 +++-
 drivers/gpu/drm/nouveau/dispnv50/disp.c       |  20 +++
 drivers/gpu/drm/nouveau/nouveau_connector.c   |   2 +-
 drivers/gpu/drm/radeon/radeon_dp_mst.c        |  22 +++
 include/drm/drm_dp_helper.h                   |   4 +
 include/drm/drm_dp_mst_helper.h               |  11 ++
 11 files changed, 285 insertions(+), 30 deletions(-)

-- 
2.22.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 01/10] drm/dp: Use non-cyclic idr

[sunpeng.li-5C7GfCeVMHo]

From: Leo Li <sunpeng.li@amd.com>

In preparation for adding aux devices for DP MST, make the IDR
non-cyclic. That way, hotplug cycling MST devices won't needlessly
increment the minor version index.

Signed-off-by: Leo Li <sunpeng.li@amd.com>
Reviewed-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_dp_aux_dev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c b/drivers/gpu/drm/drm_dp_aux_dev.c
index 5be28e3295f3..26e38dacf654 100644
--- a/drivers/gpu/drm/drm_dp_aux_dev.c
+++ b/drivers/gpu/drm/drm_dp_aux_dev.c
@@ -82,8 +82,7 @@ static struct drm_dp_aux_dev *alloc_drm_dp_aux_dev(struct drm_dp_aux *aux)
 	kref_init(&aux_dev->refcount);
 
 	mutex_lock(&aux_idr_mutex);
-	index = idr_alloc_cyclic(&aux_idr, aux_dev, 0, DRM_AUX_MINORS,
-				 GFP_KERNEL);
+	index = idr_alloc(&aux_idr, aux_dev, 0, DRM_AUX_MINORS, GFP_KERNEL);
 	mutex_unlock(&aux_idr_mutex);
 	if (index < 0) {
 		kfree(aux_dev);
-- 
2.22.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 02/10] drm/dp_mst: Enable registration of AUX devices for MST ports (v2)

[sunpeng.li-5C7GfCeVMHo]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

All available downstream ports - physical and logical - are exposed for
each MST device. They are listed in /dev/, following the same naming
scheme as SST devices by appending an incremental ID.

Although all downstream ports are exposed, only some will work as
expected. Consider the following topology:

               +---------+
               |  ASIC   |
               +---------+
              Conn-0|
                    |
               +----v----+
          +----| MST HUB |----+
          |    +---------+    |
          |                   |
          |Port-1       Port-2|
    +-----v-----+       +-----v-----+
    |  MST      |       |  SST      |
    |  Display  |       |  Display  |
    +-----------+       +-----------+
          |Port-1
          x

 MST Path  | MST Device
 ----------+----------------------------------
 sst:0     | MST Hub
 mst:0-1   | MST Display
 mst:0-1-1 | MST Display's disconnected DP out
 mst:0-1-8 | MST Display's internal sink
 mst:0-2   | SST Display

On certain MST displays, the upstream physical port will ACK DPCD reads.
However, reads on the local logical port to the internal sink will
*NAK*. i.e. reading mst:0-1 ACKs, but mst:0-1-8 NAKs.

There may also be duplicates. Some displays will return the same GUID
when reading DPCD from both mst:0-1 and mst:0-1-8.

There are some device-dependent behavior as well. The MST hub used
during testing will actually *ACK* read requests on a disconnected
physical port, whereas the MST displays will NAK.

In light of these discrepancies, it's simpler to expose all downstream
ports - both physical and logical - and let the user decide what to use.

(v2) changes:

Moved remote aux device (un)registration to new mst connector late
register and early unregister helpers. Drivers should call these from
their own mst connector function hooks.

This is to solve an issue during driver unload, where mst connector
devices are unregistered before the remote aux devices are. In a setup
where aux devices are created as children of connector devices, the aux
device would be removed too early, and uncleanly. Doing so in
early_unregister solves this issue, as that is called before connector
unregistration.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/drm_dp_aux_dev.c      |  16 ++-
 drivers/gpu/drm/drm_dp_mst_topology.c | 137 ++++++++++++++++++++++++--
 include/drm/drm_dp_helper.h           |   4 +
 include/drm/drm_dp_mst_helper.h       |  11 +++
 4 files changed, 156 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c b/drivers/gpu/drm/drm_dp_aux_dev.c
index 26e38dacf654..4aa5e455e894 100644
--- a/drivers/gpu/drm/drm_dp_aux_dev.c
+++ b/drivers/gpu/drm/drm_dp_aux_dev.c
@@ -37,6 +37,7 @@
 
 #include <drm/drm_crtc.h>
 #include <drm/drm_dp_helper.h>
+#include <drm/drm_dp_mst_helper.h>
 #include <drm/drm_print.h>
 
 #include "drm_crtc_helper_internal.h"
@@ -116,6 +117,7 @@ static ssize_t name_show(struct device *dev,
 
 	return res;
 }
+
 static DEVICE_ATTR_RO(name);
 
 static struct attribute *drm_dp_aux_attrs[] = {
@@ -162,7 +164,12 @@ static ssize_t auxdev_read_iter(struct kiocb *iocb, struct iov_iter *to)
 			break;
 		}
 
-		res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo);
+		if (aux_dev->aux->is_remote)
+			res = drm_dp_mst_dpcd_read(aux_dev->aux, pos, buf,
+						   todo);
+		else
+			res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo);
+
 		if (res <= 0)
 			break;
 
@@ -209,7 +216,12 @@ static ssize_t auxdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
 			break;
 		}
 
-		res = drm_dp_dpcd_write(aux_dev->aux, pos, buf, todo);
+		if (aux_dev->aux->is_remote)
+			res = drm_dp_mst_dpcd_write(aux_dev->aux, pos, buf,
+						    todo);
+		else
+			res = drm_dp_dpcd_write(aux_dev->aux, pos, buf, todo);
+
 		if (res <= 0)
 			break;
 
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 0984b9a34d55..dde79c44b625 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -36,6 +36,8 @@
 #include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
 
+#include "drm_crtc_helper_internal.h"
+
 /**
  * DOC: dp mst helper
  *
@@ -53,6 +55,9 @@ static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
 				     int id,
 				     struct drm_dp_payload *payload);
 
+static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
+				 struct drm_dp_mst_port *port,
+				 int offset, int size, u8 *bytes);
 static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
 				  struct drm_dp_mst_port *port,
 				  int offset, int size, u8 *bytes);
@@ -1238,6 +1243,9 @@ static void drm_dp_destroy_port(struct kref *kref)
 	struct drm_dp_mst_topology_mgr *mgr = port->mgr;
 
 	if (!port->input) {
+
+		port->vcpi.num_slots = 0;
+
 		kfree(port->cached_edid);
 
 		/*
@@ -1483,6 +1491,48 @@ static bool drm_dp_port_setup_pdt(struct drm_dp_mst_port *port)
 	return send_link;
 }
 
+/**
+ * drm_dp_mst_dpcd_read() - read a series of bytes from the DPCD via sideband
+ * @aux: Fake sideband AUX CH
+ * @offset: address of the (first) register to read
+ * @buffer: buffer to store the register values
+ * @size: number of bytes in @buffer
+ *
+ * Performs the same functionality for remote devices via
+ * sideband messaging as drm_dp_dpcd_read() does for local
+ * devices via actual AUX CH.
+ */
+ssize_t drm_dp_mst_dpcd_read(struct drm_dp_aux *aux,
+			     unsigned int offset, void *buffer, size_t size)
+{
+	struct drm_dp_mst_port *port = container_of(aux, struct drm_dp_mst_port,
+						    aux);
+
+	return drm_dp_send_dpcd_read(port->mgr, port,
+				     offset, size, buffer);
+}
+
+/**
+ * drm_dp_mst_dpcd_write() - write a series of bytes to the DPCD via sideband
+ * @aux: Fake sideband AUX CH
+ * @offset: address of the (first) register to write
+ * @buffer: buffer containing the values to write
+ * @size: number of bytes in @buffer
+ *
+ * Performs the same functionality for remote devices via
+ * sideband messaging as drm_dp_dpcd_write() does for local
+ * devices via actual AUX CH.
+ */
+ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
+			      unsigned int offset, void *buffer, size_t size)
+{
+	struct drm_dp_mst_port *port = container_of(aux, struct drm_dp_mst_port,
+						    aux);
+
+	return drm_dp_send_dpcd_write(port->mgr, port,
+				      offset, size, buffer);
+}
+
 static void drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb, u8 *guid)
 {
 	int ret;
@@ -1526,6 +1576,44 @@ static void build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
 	strlcat(proppath, temp, proppath_size);
 }
 
+/**
+ * drm_dp_mst_connector_late_register() - Late MST connector registration
+ * @drm_connector: The MST connector
+ * @port: The MST port for this connector
+ *
+ * Helper to register the remote aux device for this MST port. Drivers should
+ * call this from their mst connector's late_register hook to enable MST aux
+ * devices.
+ */
+int drm_dp_mst_connector_late_register(struct drm_connector *connector,
+				       struct drm_dp_mst_port *port)
+{
+	DRM_DEBUG_KMS("registering %s remote bus for %s\n",
+		      port->aux.name, connector->kdev->kobj.name);
+
+	port->aux.dev = connector->kdev;
+	return drm_dp_aux_register_devnode(&port->aux);
+}
+EXPORT_SYMBOL(drm_dp_mst_connector_late_register);
+
+/**
+ * drm_dp_mst_connector_early_unregister() - Early MST connector unregistration
+ * @drm_connector: The MST connector
+ * @port: The MST port for this connector
+ *
+ * Helper to unregister the remote aux device for this MST port, registered by
+ * drm_dp_mst_connector_late_register(). Drivers should call this from their mst
+ * connector's early_unregister hook.
+ */
+void drm_dp_mst_connector_early_unregister(struct drm_connector *connector,
+					   struct drm_dp_mst_port *port)
+{
+	DRM_DEBUG_KMS("unregistering %s remote bus for %s\n",
+		      port->aux.name, connector->kdev->kobj.name);
+	drm_dp_aux_unregister_devnode(&port->aux);
+}
+EXPORT_SYMBOL(drm_dp_mst_connector_early_unregister);
+
 static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
 			    struct drm_device *dev,
 			    struct drm_dp_link_addr_reply_port *port_msg)
@@ -1548,6 +1636,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
 		port->mgr = mstb->mgr;
 		port->aux.name = "DPMST";
 		port->aux.dev = dev->dev;
+		port->aux.is_remote = true;
 
 		/*
 		 * Make sure the memory allocation for our parent branch stays
@@ -1816,7 +1905,6 @@ static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
 	return false;
 }
 
-#if 0
 static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes)
 {
 	struct drm_dp_sideband_msg_req_body req;
@@ -1829,7 +1917,6 @@ static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32
 
 	return 0;
 }
-#endif
 
 static int drm_dp_send_sideband_msg(struct drm_dp_mst_topology_mgr *mgr,
 				    bool up, u8 *msg, int len)
@@ -2441,26 +2528,56 @@ int drm_dp_update_payload_part2(struct drm_dp_mst_topology_mgr *mgr)
 }
 EXPORT_SYMBOL(drm_dp_update_payload_part2);
 
-#if 0 /* unused as of yet */
 static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
 				 struct drm_dp_mst_port *port,
-				 int offset, int size)
+				 int offset, int size, u8 *bytes)
 {
 	int len;
+	int ret = 0;
 	struct drm_dp_sideband_msg_tx *txmsg;
+	struct drm_dp_mst_branch *mstb;
+
+	mstb = drm_dp_mst_topology_get_mstb_validated(mgr, port->parent);
+	if (!mstb)
+		return -EINVAL;
 
 	txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
-	if (!txmsg)
-		return -ENOMEM;
+	if (!txmsg) {
+		ret = -ENOMEM;
+		goto fail_put;
+	}
 
-	len = build_dpcd_read(txmsg, port->port_num, 0, 8);
+	len = build_dpcd_read(txmsg, port->port_num, offset, size);
 	txmsg->dst = port->parent;
 
 	drm_dp_queue_down_tx(mgr, txmsg);
 
-	return 0;
+	ret = drm_dp_mst_wait_tx_reply(mstb, txmsg);
+	if (ret < 0)
+		goto fail_free;
+
+	/* DPCD read should never be NACKed */
+	if (WARN_ON_ONCE(txmsg->reply.reply_type == 1)) {
+		ret = -EIO;
+		goto fail_free;
+	}
+
+	if (txmsg->reply.u.remote_dpcd_read_ack.num_bytes != size) {
+		ret = -EPROTO;
+		goto fail_free;
+	}
+
+	ret = min_t(size_t, txmsg->reply.u.remote_dpcd_read_ack.num_bytes,
+		    size);
+	memcpy(bytes, txmsg->reply.u.remote_dpcd_read_ack.bytes, ret);
+
+fail_free:
+	kfree(txmsg);
+fail_put:
+	drm_dp_mst_topology_put_mstb(mstb);
+
+	return ret;
 }
-#endif
 
 static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
 				  struct drm_dp_mst_port *port,
@@ -2489,7 +2606,7 @@ static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
 	ret = drm_dp_mst_wait_tx_reply(mstb, txmsg);
 	if (ret > 0) {
 		if (txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK)
-			ret = -EINVAL;
+			ret = -EIO;
 		else
 			ret = 0;
 	}
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 397896b5b21a..cb2deface950 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1309,6 +1309,10 @@ struct drm_dp_aux {
 	 * @cec: struct containing fields used for CEC-Tunneling-over-AUX.
 	 */
 	struct drm_dp_aux_cec cec;
+	/**
+	 * @is_remote: Is this "AUX CH" actually using sideband messaging.
+	 */
+	bool is_remote;
 };
 
 ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 8c97a5f92c47..2ba6253ea6d3 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -643,6 +643,17 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
 void drm_dp_mst_topology_mgr_suspend(struct drm_dp_mst_topology_mgr *mgr);
 int __must_check
 drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr);
+
+ssize_t drm_dp_mst_dpcd_read(struct drm_dp_aux *aux,
+			     unsigned int offset, void *buffer, size_t size);
+ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
+			      unsigned int offset, void *buffer, size_t size);
+
+int drm_dp_mst_connector_late_register(struct drm_connector *connector,
+				       struct drm_dp_mst_port *port);
+void drm_dp_mst_connector_early_unregister(struct drm_connector *connector,
+					   struct drm_dp_mst_port *port);
+
 struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct drm_atomic_state *state,
 								    struct drm_dp_mst_topology_mgr *mgr);
 int __must_check
-- 
2.22.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 03/10] drm/sysfs: Add mstpath attribute to connector devices

[sunpeng.li]

From: Leo Li <sunpeng.li@amd.com>

This can be used to create more descriptive symlinks for MST aux
devices. Consider the following udev rule:

SUBSYSTEM=="drm_dp_aux_dev", SUBSYSTEMS=="drm", ATTRS{mstpath}=="?*",
	SYMLINK+="drm_dp_aux/by-path/$attr{mstpath}"

The following symlinks will be created (depending on your MST topology):

$ ls /dev/drm_dp_aux/by-path/
card0-mst:0-1  card0-mst:0-1-1  card0-mst:0-1-8  card0-mst:0-8

Signed-off-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/drm_sysfs.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index ad10810bc972..53fd1f71e900 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -236,16 +236,39 @@ static ssize_t modes_show(struct device *device,
 	return written;
 }
 
+static ssize_t mstpath_show(struct device *device,
+			    struct device_attribute *attr,
+			    char *buf)
+{
+	struct drm_connector *connector = to_drm_connector(device);
+	ssize_t ret = 0;
+	char *path;
+
+	mutex_lock(&connector->dev->mode_config.mutex);
+	if (!connector->path_blob_ptr)
+		goto unlock;
+
+	path = connector->path_blob_ptr->data;
+	ret = snprintf(buf, PAGE_SIZE, "card%d-%s\n",
+		       connector->dev->primary->index, path);
+
+unlock:
+	mutex_unlock(&connector->dev->mode_config.mutex);
+	return ret;
+}
+
 static DEVICE_ATTR_RW(status);
 static DEVICE_ATTR_RO(enabled);
 static DEVICE_ATTR_RO(dpms);
 static DEVICE_ATTR_RO(modes);
+static DEVICE_ATTR_RO(mstpath);
 
 static struct attribute *connector_dev_attrs[] = {
 	&dev_attr_status.attr,
 	&dev_attr_enabled.attr,
 	&dev_attr_dpms.attr,
 	&dev_attr_modes.attr,
+	&dev_attr_mstpath.attr,
 	NULL
 };
 
-- 
2.22.0

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

[PATCH 04/10] drm/nouveau: Use connector kdev as aux device parent

[sunpeng.li-5C7GfCeVMHo]

From: Leo Li <sunpeng.li@amd.com>

Set the connector's kernel device as the parent for the aux kernel
device. This allows udev rules to access connector attributes when
creating symlinks to aux devices.

Cc: Ben Skeggs <bskeggs@redhat.com>
Signed-off-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/nouveau/nouveau_connector.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 4116ee62adaf..e32def09ef89 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1346,7 +1346,7 @@ nouveau_connector_create(struct drm_device *dev,
 		break;
 	case DRM_MODE_CONNECTOR_DisplayPort:
 	case DRM_MODE_CONNECTOR_eDP:
-		nv_connector->aux.dev = dev->dev;
+		nv_connector->aux.dev = connector->kdev;
 		nv_connector->aux.transfer = nouveau_connector_aux_xfer;
 		snprintf(aux_name, sizeof(aux_name), "sor-%04x-%04x",
 			 dcbe->hasht, dcbe->hashm);
-- 
2.22.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 05/10] drm/bridge/analogix-anx78xx: Use connector kdev as aux device parent

[sunpeng.li-5C7GfCeVMHo]

From: Leo Li <sunpeng.li@amd.com>

Set the connector's kernel device as the parent for the aux kernel
device. This allows udev rules to access connector attributes when
creating symlinks to aux devices.

To do so, the connector needs to be registered beforehand. Therefore,
shift aux registration to be after connector registration.

Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Cc: Nicolas Boichat <drinkcat@chromium.org>
Signed-off-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/bridge/analogix-anx78xx.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
index 3c7cc5af735c..c2800cd3e2ee 100644
--- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
@@ -1008,17 +1008,6 @@ static int anx78xx_bridge_attach(struct drm_bridge *bridge)
 		return -ENODEV;
 	}
 
-	/* Register aux channel */
-	anx78xx->aux.name = "DP-AUX";
-	anx78xx->aux.dev = &anx78xx->client->dev;
-	anx78xx->aux.transfer = anx78xx_aux_transfer;
-
-	err = drm_dp_aux_register(&anx78xx->aux);
-	if (err < 0) {
-		DRM_ERROR("Failed to register aux channel: %d\n", err);
-		return err;
-	}
-
 	err = drm_connector_init(bridge->dev, &anx78xx->connector,
 				 &anx78xx_connector_funcs,
 				 DRM_MODE_CONNECTOR_DisplayPort);
@@ -1038,6 +1027,17 @@ static int anx78xx_bridge_attach(struct drm_bridge *bridge)
 
 	anx78xx->connector.polled = DRM_CONNECTOR_POLL_HPD;
 
+	/* Register aux channel */
+	anx78xx->aux.name = "DP-AUX";
+	anx78xx->aux.dev = anx78xx->connector.kdev;
+	anx78xx->aux.transfer = anx78xx_aux_transfer;
+
+	err = drm_dp_aux_register(&anx78xx->aux);
+	if (err < 0) {
+		DRM_ERROR("Failed to register aux channel: %d\n", err);
+		return err;
+	}
+
 	err = drm_connector_attach_encoder(&anx78xx->connector,
 					   bridge->encoder);
 	if (err) {
-- 
2.22.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 06/10] drm/amd/display: Use connector kdev as aux device parent

[sunpeng.li-5C7GfCeVMHo]

From: Leo Li <sunpeng.li@amd.com>

Set the connector's kernel device as the parent for the aux kernel
device. This allows udev rules to access connector attributes when
creating symlinks to aux devices.

For example, the following udev rule:

SUBSYSTEM=="drm_dp_aux_dev", SUBSYSTEMS=="drm", ATTRS{edid}=="*",
	SYMLINK+="drm_dp_aux/by-name/$id"

Will create the following symlinks using the connector's name:

$ ls /dev/drm_dp_aux/by-name/
card0-DP-1  card0-DP-2  card0-DP-3

Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Cc: Jerry (Fangzhi) Zuo <Jerry.Zuo@amd.com>
Signed-off-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 6e205ee36ac3..53d2cfe62e13 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -388,7 +388,7 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm,
 				       struct amdgpu_dm_connector *aconnector)
 {
 	aconnector->dm_dp_aux.aux.name = "dmdc";
-	aconnector->dm_dp_aux.aux.dev = dm->adev->dev;
+	aconnector->dm_dp_aux.aux.dev = aconnector->base.kdev;
 	aconnector->dm_dp_aux.aux.transfer = dm_dp_aux_transfer;
 	aconnector->dm_dp_aux.ddc_service = aconnector->dc_link->ddc;
 
-- 
2.22.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 07/10] drm/i915: Implement MST Aux device registration

[sunpeng.li-5C7GfCeVMHo]

From: Leo Li <sunpeng.li@amd.com>

Implement late_register and early_unregister hooks for MST connectors.
Call drm helpers for MST connector registration, which registers the
AUX devices.

Signed-off-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 29 +++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 60652ebbdf61..be309016f746 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -400,13 +400,38 @@ intel_dp_mst_detect(struct drm_connector *connector, bool force)
 				      intel_connector->port);
 }
 
+static int
+intel_dp_mst_connector_late_register(struct drm_connector *connector)
+{
+	struct intel_connector *intel_connector = to_intel_connector(connector);
+	struct drm_dp_mst_port *port = intel_connector->port;
+
+	int ret;
+
+	ret = intel_connector_register(connector);
+	if (ret)
+		return ret;
+
+	return drm_dp_mst_connector_late_register(connector, port);
+}
+
+static void
+intel_dp_mst_connector_early_unregister(struct drm_connector *connector)
+{
+	struct intel_connector *intel_connector = to_intel_connector(connector);
+	struct drm_dp_mst_port *port = intel_connector->port;
+
+	drm_dp_mst_connector_early_unregister(connector, port);
+	intel_connector_unregister(connector);
+}
+
 static const struct drm_connector_funcs intel_dp_mst_connector_funcs = {
 	.detect = intel_dp_mst_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.atomic_get_property = intel_digital_connector_atomic_get_property,
 	.atomic_set_property = intel_digital_connector_atomic_set_property,
-	.late_register = intel_connector_register,
-	.early_unregister = intel_connector_unregister,
+	.late_register = intel_dp_mst_connector_late_register,
+	.early_unregister = intel_dp_mst_connector_early_unregister,
 	.destroy = intel_connector_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 	.atomic_duplicate_state = intel_digital_connector_duplicate_state,
-- 
2.22.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 08/10] drm/nouveau/kms/nv50: Implement MST Aux device registration

[sunpeng.li-5C7GfCeVMHo]

From: Leo Li <sunpeng.li@amd.com>

Implement late_register and early_unregister hooks for MST connectors.
Call drm helpers for MST connector registration, which registers the
AUX devices.

Cc: Ben Skeggs <bskeggs@redhat.com>
Signed-off-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 7ba373f493b2..6d0fbb6036cf 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1019,6 +1019,24 @@ nv50_mstc_destroy(struct drm_connector *connector)
 	kfree(mstc);
 }
 
+static int
+nv50_mstc_late_register(struct drm_connector *connector)
+{
+	struct nv50_mstc *mstc = nv50_mstc(connector);
+	struct drm_dp_mst_port *port = mstc->port;
+
+	return drm_dp_mst_connector_late_register(connector, port);
+}
+
+static void
+nv50_mstc_early_unregister(struct drm_connector *connector)
+{
+	struct nv50_mstc *mstc = nv50_mstc(connector);
+	struct drm_dp_mst_port *port = mstc->port;
+
+	drm_dp_mst_connector_early_unregister(connector, port);
+}
+
 static const struct drm_connector_funcs
 nv50_mstc = {
 	.reset = nouveau_conn_reset,
@@ -1029,6 +1047,8 @@ nv50_mstc = {
 	.atomic_destroy_state = nouveau_conn_atomic_destroy_state,
 	.atomic_set_property = nouveau_conn_atomic_set_property,
 	.atomic_get_property = nouveau_conn_atomic_get_property,
+	.late_register = nv50_mstc_late_register,
+	.early_unregister = nv50_mstc_early_unregister,
 };
 
 static int
-- 
2.22.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 09/10] drm/radeon: Implement MST Aux device registration

[sunpeng.li-5C7GfCeVMHo]

From: Leo Li <sunpeng.li@amd.com>

Implement late_register and early_unregister hooks for MST connectors.
Call drm helpers for MST connector registration, which registers the
AUX devices.

Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/radeon/radeon_dp_mst.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c
index 2994f07fbad9..2d53699734fb 100644
--- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
@@ -260,11 +260,33 @@ radeon_dp_mst_connector_destroy(struct drm_connector *connector)
 	kfree(radeon_connector);
 }
 
+static int
+radeon_dp_mst_connector_late_register(struct drm_connector *connector)
+{
+	struct radeon_connector *radeon_connector =
+		to_radeon_connector(connector);
+	struct drm_dp_mst_port *port = radeon_connector->port;
+
+	return drm_dp_mst_connector_late_register(connector, port);
+}
+
+static void
+radeon_dp_mst_connector_early_unregister(struct drm_connector *connector)
+{
+	struct radeon_connector *radeon_connector =
+		to_radeon_connector(connector);
+	struct drm_dp_mst_port *port = radeon_connector->port;
+
+	drm_dp_mst_connector_early_unregister(connector, port);
+}
+
 static const struct drm_connector_funcs radeon_dp_mst_connector_funcs = {
 	.dpms = drm_helper_connector_dpms,
 	.detect = radeon_dp_mst_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.destroy = radeon_dp_mst_connector_destroy,
+	.late_register = radeon_dp_mst_connector_late_register,
+	.early_unregister = radeon_dp_mst_connector_early_unregister,
 };
 
 static struct drm_connector *radeon_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
-- 
2.22.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 10/10] drm/amd/display: Implement MST Aux device registration

[sunpeng.li-5C7GfCeVMHo]

From: Leo Li <sunpeng.li@amd.com>

Implement late_register and early_unregister hooks for MST connectors.
Call drm helpers for MST connector registration, which registers the
AUX devices.

Cc: Jerry Zuo <Jerry.Zuo@amd.com>
Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Signed-off-by: Leo Li <sunpeng.li@amd.com>
---
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   | 24 ++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 53d2cfe62e13..16218a202b59 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -156,6 +156,26 @@ dm_dp_mst_connector_destroy(struct drm_connector *connector)
 	kfree(amdgpu_dm_connector);
 }
 
+static int
+amdgpu_dm_mst_connector_late_register(struct drm_connector *connector)
+{
+	struct amdgpu_dm_connector *amdgpu_dm_connector =
+		to_amdgpu_dm_connector(connector);
+	struct drm_dp_mst_port *port = amdgpu_dm_connector->port;
+
+	return drm_dp_mst_connector_late_register(connector, port);
+}
+
+static void
+amdgpu_dm_mst_connector_early_unregister(struct drm_connector *connector)
+{
+	struct amdgpu_dm_connector *amdgpu_dm_connector =
+		to_amdgpu_dm_connector(connector);
+	struct drm_dp_mst_port *port = amdgpu_dm_connector->port;
+
+	drm_dp_mst_connector_early_unregister(connector, port);
+}
+
 static const struct drm_connector_funcs dm_dp_mst_connector_funcs = {
 	.detect = dm_dp_mst_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
@@ -164,7 +184,9 @@ static const struct drm_connector_funcs dm_dp_mst_connector_funcs = {
 	.atomic_duplicate_state = amdgpu_dm_connector_atomic_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 	.atomic_set_property = amdgpu_dm_connector_atomic_set_property,
-	.atomic_get_property = amdgpu_dm_connector_atomic_get_property
+	.atomic_get_property = amdgpu_dm_connector_atomic_get_property,
+	.late_register = amdgpu_dm_mst_connector_late_register,
+	.early_unregister = amdgpu_dm_mst_connector_early_unregister,
 };
 
 static int dm_dp_mst_get_modes(struct drm_connector *connector)
-- 
2.22.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 03/10] drm/sysfs: Add mstpath attribute to connector devices

[Ville Syrjälä]

On Thu, Jul 04, 2019 at 03:05:12PM -0400, sunpeng.li@amd.com wrote:
> From: Leo Li <sunpeng.li@amd.com>
> 
> This can be used to create more descriptive symlinks for MST aux
> devices. Consider the following udev rule:
> 
> SUBSYSTEM=="drm_dp_aux_dev", SUBSYSTEMS=="drm", ATTRS{mstpath}=="?*",
> 	SYMLINK+="drm_dp_aux/by-path/$attr{mstpath}"
> 
> The following symlinks will be created (depending on your MST topology):
> 
> $ ls /dev/drm_dp_aux/by-path/
> card0-mst:0-1  card0-mst:0-1-1  card0-mst:0-1-8  card0-mst:0-8
> 
> Signed-off-by: Leo Li <sunpeng.li@amd.com>
> ---
>  drivers/gpu/drm/drm_sysfs.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index ad10810bc972..53fd1f71e900 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -236,16 +236,39 @@ static ssize_t modes_show(struct device *device,
>  	return written;
>  }
>  
> +static ssize_t mstpath_show(struct device *device,
> +			    struct device_attribute *attr,
> +			    char *buf)
> +{
> +	struct drm_connector *connector = to_drm_connector(device);
> +	ssize_t ret = 0;
> +	char *path;
> +
> +	mutex_lock(&connector->dev->mode_config.mutex);

The blob is populated when the connector is created so I don't think
this lock is actually doing anything for us.

One would also hope that device_unregister() protects us from racing
with the removal of the attribute. Eg. if you hold a file descriptor
open to the sysfs file does device_unregister() block until the fd is
closed?

> +	if (!connector->path_blob_ptr)
> +		goto unlock;
> +
> +	path = connector->path_blob_ptr->data;
> +	ret = snprintf(buf, PAGE_SIZE, "card%d-%s\n",
> +		       connector->dev->primary->index, path);
> +
> +unlock:
> +	mutex_unlock(&connector->dev->mode_config.mutex);
> +	return ret;
> +}
> +
>  static DEVICE_ATTR_RW(status);
>  static DEVICE_ATTR_RO(enabled);
>  static DEVICE_ATTR_RO(dpms);
>  static DEVICE_ATTR_RO(modes);
> +static DEVICE_ATTR_RO(mstpath);
>  
>  static struct attribute *connector_dev_attrs[] = {
>  	&dev_attr_status.attr,
>  	&dev_attr_enabled.attr,
>  	&dev_attr_dpms.attr,
>  	&dev_attr_modes.attr,
> +	&dev_attr_mstpath.attr,
>  	NULL
>  };
>  
> -- 
> 2.22.0

-- 
Ville Syrjälä
Intel
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 03/10] drm/sysfs: Add mstpath attribute to connector devices

[Li, Sun peng (Leo)]

On 2019-07-04 3:33 p.m., Ville Syrjälä wrote:
> On Thu, Jul 04, 2019 at 03:05:12PM -0400, sunpeng.li@amd.com wrote:
>> From: Leo Li <sunpeng.li@amd.com>
>>
>> This can be used to create more descriptive symlinks for MST aux
>> devices. Consider the following udev rule:
>>
>> SUBSYSTEM=="drm_dp_aux_dev", SUBSYSTEMS=="drm", ATTRS{mstpath}=="?*",
>> 	SYMLINK+="drm_dp_aux/by-path/$attr{mstpath}"
>>
>> The following symlinks will be created (depending on your MST topology):
>>
>> $ ls /dev/drm_dp_aux/by-path/
>> card0-mst:0-1  card0-mst:0-1-1  card0-mst:0-1-8  card0-mst:0-8
>>
>> Signed-off-by: Leo Li <sunpeng.li@amd.com>
>> ---
>>  drivers/gpu/drm/drm_sysfs.c | 23 +++++++++++++++++++++++
>>  1 file changed, 23 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
>> index ad10810bc972..53fd1f71e900 100644
>> --- a/drivers/gpu/drm/drm_sysfs.c
>> +++ b/drivers/gpu/drm/drm_sysfs.c
>> @@ -236,16 +236,39 @@ static ssize_t modes_show(struct device *device,
>>  	return written;
>>  }
>>  
>> +static ssize_t mstpath_show(struct device *device,
>> +			    struct device_attribute *attr,
>> +			    char *buf)
>> +{
>> +	struct drm_connector *connector = to_drm_connector(device);
>> +	ssize_t ret = 0;
>> +	char *path;
>> +
>> +	mutex_lock(&connector->dev->mode_config.mutex);
> 
> The blob is populated when the connector is created so I don't think
> this lock is actually doing anything for us.

Right, will drop this.

> 
> One would also hope that device_unregister() protects us from racing
> with the removal of the attribute. Eg. if you hold a file descriptor
> open to the sysfs file does device_unregister() block until the fd is
> closed?

For dpcd transactions, as long as the aux device is unregistered through
drm_dp_aux_unregister_devnode(), we should be good. There's an atomic_t
use_count that syncs against reads and writes.

Although I'm not sure what would happen if the device was ripped out
from underneath us, say, if the parent connector device is removed
before calling aux_unregister_devnode(). If this does happen, I think
it's more of an order-of-operations issue from the driver. V2 of patch 2
(and 7-10) actually addresses a specific occurence of this during driver
unload.

Regarding sysfs attrs, the kernfs underneath does seem to guarantee that
any r/w is completed before removal. See kernfs_drain(), called as part
of kernfs_remove().

Leo

> 
>> +	if (!connector->path_blob_ptr)
>> +		goto unlock;
>> +
>> +	path = connector->path_blob_ptr->data;
>> +	ret = snprintf(buf, PAGE_SIZE, "card%d-%s\n",
>> +		       connector->dev->primary->index, path);
>> +
>> +unlock:
>> +	mutex_unlock(&connector->dev->mode_config.mutex);
>> +	return ret;
>> +}
>> +
>>  static DEVICE_ATTR_RW(status);
>>  static DEVICE_ATTR_RO(enabled);
>>  static DEVICE_ATTR_RO(dpms);
>>  static DEVICE_ATTR_RO(modes);
>> +static DEVICE_ATTR_RO(mstpath);
>>  
>>  static struct attribute *connector_dev_attrs[] = {
>>  	&dev_attr_status.attr,
>>  	&dev_attr_enabled.attr,
>>  	&dev_attr_dpms.attr,
>>  	&dev_attr_modes.attr,
>> +	&dev_attr_mstpath.attr,
>>  	NULL
>>  };
>>  
>> -- 
>> 2.22.0
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2] drm/sysfs: Add mstpath attribute to connector devices

[sunpeng.li-5C7GfCeVMHo]

From: Leo Li <sunpeng.li@amd.com>

This can be used to create more descriptive symlinks for MST aux
devices. Consider the following udev rule:

SUBSYSTEM=="drm_dp_aux_dev", SUBSYSTEMS=="drm", ATTRS{mstpath}=="?*",
	SYMLINK+="drm_dp_aux/by-path/$attr{mstpath}"

The following symlinks will be created (depending on your MST topology):

$ ls /dev/drm_dp_aux/by-path/
card0-mst:0-1  card0-mst:0-1-1  card0-mst:0-1-8  card0-mst:0-8

v2: remove unnecessary locking of mode_config.mutex

Signed-off-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/drm_sysfs.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index ad10810bc972..7d483ab684a0 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -236,16 +236,36 @@ static ssize_t modes_show(struct device *device,
 	return written;
 }
 
+static ssize_t mstpath_show(struct device *device,
+			    struct device_attribute *attr,
+			    char *buf)
+{
+	struct drm_connector *connector = to_drm_connector(device);
+	ssize_t ret = 0;
+	char *path;
+
+	if (!connector->path_blob_ptr)
+		return ret;
+
+	path = connector->path_blob_ptr->data;
+	ret = snprintf(buf, PAGE_SIZE, "card%d-%s\n",
+		       connector->dev->primary->index, path);
+
+	return ret;
+}
+
 static DEVICE_ATTR_RW(status);
 static DEVICE_ATTR_RO(enabled);
 static DEVICE_ATTR_RO(dpms);
 static DEVICE_ATTR_RO(modes);
+static DEVICE_ATTR_RO(mstpath);
 
 static struct attribute *connector_dev_attrs[] = {
 	&dev_attr_status.attr,
 	&dev_attr_enabled.attr,
 	&dev_attr_dpms.attr,
 	&dev_attr_modes.attr,
+	&dev_attr_mstpath.attr,
 	NULL
 };
 
-- 
2.22.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 00/10] Enable MST Aux devices (v2)

[Li, Sun peng (Leo)]

Hi Lyude, sorry - just realized I forgot to CC you on this series! Let
me know if I should resend them.

Adding some additional reviewers as well.

Thanks,
Leo

On 2019-07-04 3:05 p.m., sunpeng.li@amd.com wrote:
> From: Leo Li <sunpeng.li@amd.com>
> 
> Hi all,
> 
> Here's the second revision of patches to enable mst aux devices.
> 
> v2 fixes an aux device unregistration issue during driver unload. See
> patch 2/10 for details. Consequently, drivers supporting mst are
> modified to use the new mst connector late register and early unregister
> helpers.
> 
> Thanks,
> Leo
> 
> Leo Li (9):
>   drm/dp: Use non-cyclic idr
>   drm/sysfs: Add mstpath attribute to connector devices
>   drm/nouveau: Use connector kdev as aux device parent
>   drm/bridge/analogix-anx78xx: Use connector kdev as aux device parent
>   drm/amd/display: Use connector kdev as aux device parent
>   drm/i915: Implement MST Aux device registration
>   drm/nouveau/kms/nv50: Implement MST Aux device registration
>   drm/radeon: Implement MST Aux device registration
>   drm/amd/display: Implement MST Aux device registration
> 
> Ville Syrjälä (1):
>   drm/dp_mst: Enable registration of AUX devices for MST ports
> 
>  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  26 +++-
>  drivers/gpu/drm/bridge/analogix-anx78xx.c     |  22 +--
>  drivers/gpu/drm/drm_dp_aux_dev.c              |  19 ++-
>  drivers/gpu/drm/drm_dp_mst_topology.c         | 137 ++++++++++++++++--
>  drivers/gpu/drm/drm_sysfs.c                   |  23 +++
>  drivers/gpu/drm/i915/display/intel_dp_mst.c   |  29 +++-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c       |  20 +++
>  drivers/gpu/drm/nouveau/nouveau_connector.c   |   2 +-
>  drivers/gpu/drm/radeon/radeon_dp_mst.c        |  22 +++
>  include/drm/drm_dp_helper.h                   |   4 +
>  include/drm/drm_dp_mst_helper.h               |  11 ++
>  11 files changed, 285 insertions(+), 30 deletions(-)
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 07/10] drm/i915: Implement MST Aux device registration

[Ville Syrjälä]

On Thu, Jul 04, 2019 at 03:05:16PM -0400, sunpeng.li@amd.com wrote:
> From: Leo Li <sunpeng.li@amd.com>
> 
> Implement late_register and early_unregister hooks for MST connectors.
> Call drm helpers for MST connector registration, which registers the
> AUX devices.
> 
> Signed-off-by: Leo Li <sunpeng.li@amd.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 29 +++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 60652ebbdf61..be309016f746 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -400,13 +400,38 @@ intel_dp_mst_detect(struct drm_connector *connector, bool force)
>  				      intel_connector->port);
>  }
>  
> +static int
> +intel_dp_mst_connector_late_register(struct drm_connector *connector)
> +{
> +	struct intel_connector *intel_connector = to_intel_connector(connector);
> +	struct drm_dp_mst_port *port = intel_connector->port;
> +
> +	int ret;
> +
> +	ret = intel_connector_register(connector);
> +	if (ret)
> +		return ret;
> +
> +	return drm_dp_mst_connector_late_register(connector, port);

We should probably unwind properly in case of an error from this guy.
Currently it won't matter since intel_connector_register() only sets
up the backlight and that doesn't exist for MST connectors, but if and
when someone adds more stuff to intel_connector_register() they may not
notice that the caller is leaking.

> +}
> +
> +static void
> +intel_dp_mst_connector_early_unregister(struct drm_connector *connector)
> +{
> +	struct intel_connector *intel_connector = to_intel_connector(connector);
> +	struct drm_dp_mst_port *port = intel_connector->port;
> +
> +	drm_dp_mst_connector_early_unregister(connector, port);
> +	intel_connector_unregister(connector);
> +}
> +
>  static const struct drm_connector_funcs intel_dp_mst_connector_funcs = {
>  	.detect = intel_dp_mst_detect,
>  	.fill_modes = drm_helper_probe_single_connector_modes,
>  	.atomic_get_property = intel_digital_connector_atomic_get_property,
>  	.atomic_set_property = intel_digital_connector_atomic_set_property,
> -	.late_register = intel_connector_register,
> -	.early_unregister = intel_connector_unregister,
> +	.late_register = intel_dp_mst_connector_late_register,
> +	.early_unregister = intel_dp_mst_connector_early_unregister,
>  	.destroy = intel_connector_destroy,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  	.atomic_duplicate_state = intel_digital_connector_duplicate_state,
> -- 
> 2.22.0

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/sysfs: Add mstpath attribute to connector devices

[Ville Syrjälä]

On Fri, Jul 05, 2019 at 10:32:20AM -0400, sunpeng.li@amd.com wrote:
> From: Leo Li <sunpeng.li@amd.com>
> 
> This can be used to create more descriptive symlinks for MST aux
> devices. Consider the following udev rule:
> 
> SUBSYSTEM=="drm_dp_aux_dev", SUBSYSTEMS=="drm", ATTRS{mstpath}=="?*",
> 	SYMLINK+="drm_dp_aux/by-path/$attr{mstpath}"
> 
> The following symlinks will be created (depending on your MST topology):
> 
> $ ls /dev/drm_dp_aux/by-path/
> card0-mst:0-1  card0-mst:0-1-1  card0-mst:0-1-8  card0-mst:0-8
> 
> v2: remove unnecessary locking of mode_config.mutex
> 
> Signed-off-by: Leo Li <sunpeng.li@amd.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/drm_sysfs.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index ad10810bc972..7d483ab684a0 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -236,16 +236,36 @@ static ssize_t modes_show(struct device *device,
>  	return written;
>  }
>  
> +static ssize_t mstpath_show(struct device *device,
> +			    struct device_attribute *attr,
> +			    char *buf)
> +{
> +	struct drm_connector *connector = to_drm_connector(device);
> +	ssize_t ret = 0;
> +	char *path;
> +
> +	if (!connector->path_blob_ptr)
> +		return ret;
> +
> +	path = connector->path_blob_ptr->data;
> +	ret = snprintf(buf, PAGE_SIZE, "card%d-%s\n",
> +		       connector->dev->primary->index, path);
> +
> +	return ret;
> +}
> +
>  static DEVICE_ATTR_RW(status);
>  static DEVICE_ATTR_RO(enabled);
>  static DEVICE_ATTR_RO(dpms);
>  static DEVICE_ATTR_RO(modes);
> +static DEVICE_ATTR_RO(mstpath);
>  
>  static struct attribute *connector_dev_attrs[] = {
>  	&dev_attr_status.attr,
>  	&dev_attr_enabled.attr,
>  	&dev_attr_dpms.attr,
>  	&dev_attr_modes.attr,
> +	&dev_attr_mstpath.attr,
>  	NULL
>  };
>  
> -- 
> 2.22.0

-- 
Ville Syrjälä
Intel
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 02/10] drm/dp_mst: Enable registration of AUX devices for MST ports (v2)

[Lyude Paul]

Some small nitpicks

On Thu, 2019-07-04 at 15:05 -0400, sunpeng.li@amd.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> All available downstream ports - physical and logical - are exposed for
> each MST device. They are listed in /dev/, following the same naming
> scheme as SST devices by appending an incremental ID.
> 
> Although all downstream ports are exposed, only some will work as
> expected. Consider the following topology:
> 
>                +---------+
>                |  ASIC   |
>                +---------+
>               Conn-0|
>                     |
>                +----v----+
>           +----| MST HUB |----+
>           |    +---------+    |
>           |                   |
>           |Port-1       Port-2|
>     +-----v-----+       +-----v-----+
>     |  MST      |       |  SST      |
>     |  Display  |       |  Display  |
>     +-----------+       +-----------+
>           |Port-1
>           x
> 
>  MST Path  | MST Device
>  ----------+----------------------------------
>  sst:0     | MST Hub
>  mst:0-1   | MST Display
>  mst:0-1-1 | MST Display's disconnected DP out
>  mst:0-1-8 | MST Display's internal sink
>  mst:0-2   | SST Display
> 
> On certain MST displays, the upstream physical port will ACK DPCD reads.
> However, reads on the local logical port to the internal sink will
> *NAK*. i.e. reading mst:0-1 ACKs, but mst:0-1-8 NAKs.
> 
> There may also be duplicates. Some displays will return the same GUID
> when reading DPCD from both mst:0-1 and mst:0-1-8.
> 
> There are some device-dependent behavior as well. The MST hub used
> during testing will actually *ACK* read requests on a disconnected
> physical port, whereas the MST displays will NAK.
> 
> In light of these discrepancies, it's simpler to expose all downstream
> ports - both physical and logical - and let the user decide what to use.
> 
> (v2) changes:
> 
> Moved remote aux device (un)registration to new mst connector late
> register and early unregister helpers. Drivers should call these from
> their own mst connector function hooks.
> 
> This is to solve an issue during driver unload, where mst connector
> devices are unregistered before the remote aux devices are. In a setup
> where aux devices are created as children of connector devices, the aux
> device would be removed too early, and uncleanly. Doing so in
> early_unregister solves this issue, as that is called before connector
> unregistration.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Leo Li <sunpeng.li@amd.com>
> ---
>  drivers/gpu/drm/drm_dp_aux_dev.c      |  16 ++-
>  drivers/gpu/drm/drm_dp_mst_topology.c | 137 ++++++++++++++++++++++++--
>  include/drm/drm_dp_helper.h           |   4 +
>  include/drm/drm_dp_mst_helper.h       |  11 +++
>  4 files changed, 156 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c
> b/drivers/gpu/drm/drm_dp_aux_dev.c
> index 26e38dacf654..4aa5e455e894 100644
> --- a/drivers/gpu/drm/drm_dp_aux_dev.c
> +++ b/drivers/gpu/drm/drm_dp_aux_dev.c
> @@ -37,6 +37,7 @@
>  
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_dp_helper.h>
> +#include <drm/drm_dp_mst_helper.h>
>  #include <drm/drm_print.h>
>  
>  #include "drm_crtc_helper_internal.h"
> @@ -116,6 +117,7 @@ static ssize_t name_show(struct device *dev,
>  
>  	return res;
>  }
> +
>  static DEVICE_ATTR_RO(name);
>  
>  static struct attribute *drm_dp_aux_attrs[] = {
> @@ -162,7 +164,12 @@ static ssize_t auxdev_read_iter(struct kiocb *iocb,
> struct iov_iter *to)
>  			break;
>  		}
>  
> -		res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo);
> +		if (aux_dev->aux->is_remote)
> +			res = drm_dp_mst_dpcd_read(aux_dev->aux, pos, buf,
> +						   todo);
> +		else
> +			res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo);
> +
>  		if (res <= 0)
>  			break;
>  
> @@ -209,7 +216,12 @@ static ssize_t auxdev_write_iter(struct kiocb *iocb,
> struct iov_iter *from)
>  			break;
>  		}
>  
> -		res = drm_dp_dpcd_write(aux_dev->aux, pos, buf, todo);
> +		if (aux_dev->aux->is_remote)
> +			res = drm_dp_mst_dpcd_write(aux_dev->aux, pos, buf,
> +						    todo);
> +		else
> +			res = drm_dp_dpcd_write(aux_dev->aux, pos, buf, todo);
> +
>  		if (res <= 0)
>  			break;
>  
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 0984b9a34d55..dde79c44b625 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -36,6 +36,8 @@
>  #include <drm/drm_print.h>
>  #include <drm/drm_probe_helper.h>
>  
> +#include "drm_crtc_helper_internal.h"
> +
>  /**
>   * DOC: dp mst helper
>   *
> @@ -53,6 +55,9 @@ static int drm_dp_dpcd_write_payload(struct
> drm_dp_mst_topology_mgr *mgr,
>  				     int id,
>  				     struct drm_dp_payload *payload);
>  
> +static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
> +				 struct drm_dp_mst_port *port,
> +				 int offset, int size, u8 *bytes);
>  static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
>  				  struct drm_dp_mst_port *port,
>  				  int offset, int size, u8 *bytes);
> @@ -1238,6 +1243,9 @@ static void drm_dp_destroy_port(struct kref *kref)
>  	struct drm_dp_mst_topology_mgr *mgr = port->mgr;
>  
>  	if (!port->input) {
> +

We should get rid of the extra newline right here ^

> +		port->vcpi.num_slots = 0;
> +
>  		kfree(port->cached_edid);
>  
>  		/*
> @@ -1483,6 +1491,48 @@ static bool drm_dp_port_setup_pdt(struct
> drm_dp_mst_port *port)
>  	return send_link;
>  }
>  
> +/**
> + * drm_dp_mst_dpcd_read() - read a series of bytes from the DPCD via
> sideband
> + * @aux: Fake sideband AUX CH
> + * @offset: address of the (first) register to read
> + * @buffer: buffer to store the register values
> + * @size: number of bytes in @buffer
> + *
> + * Performs the same functionality for remote devices via
> + * sideband messaging as drm_dp_dpcd_read() does for local
> + * devices via actual AUX CH.
> + */

You forgot to document the return value

> +ssize_t drm_dp_mst_dpcd_read(struct drm_dp_aux *aux,
> +			     unsigned int offset, void *buffer, size_t size)
> +{
> +	struct drm_dp_mst_port *port = container_of(aux, struct
> drm_dp_mst_port,
> +						    aux);
> +
> +	return drm_dp_send_dpcd_read(port->mgr, port,
> +				     offset, size, buffer);
> +}
> +
> +/**
> + * drm_dp_mst_dpcd_write() - write a series of bytes to the DPCD via
> sideband
> + * @aux: Fake sideband AUX CH
> + * @offset: address of the (first) register to write
> + * @buffer: buffer containing the values to write
> + * @size: number of bytes in @buffer
> + *
> + * Performs the same functionality for remote devices via
> + * sideband messaging as drm_dp_dpcd_write() does for local
> + * devices via actual AUX CH.
> + */

Same for this one…

> +ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
> +			      unsigned int offset, void *buffer, size_t size)
> +{
> +	struct drm_dp_mst_port *port = container_of(aux, struct
> drm_dp_mst_port,
> +						    aux);
> +
> +	return drm_dp_send_dpcd_write(port->mgr, port,
> +				      offset, size, buffer);
> +}
> +
>  static void drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb, u8
> *guid)
>  {
>  	int ret;
> @@ -1526,6 +1576,44 @@ static void build_mst_prop_path(const struct
> drm_dp_mst_branch *mstb,
>  	strlcat(proppath, temp, proppath_size);
>  }
>  
> +/**
> + * drm_dp_mst_connector_late_register() - Late MST connector registration
> + * @drm_connector: The MST connector
> + * @port: The MST port for this connector
> + *
> + * Helper to register the remote aux device for this MST port. Drivers
> should
> + * call this from their mst connector's late_register hook to enable MST
> aux
> + * devices.
> + */

…and this one…

> +int drm_dp_mst_connector_late_register(struct drm_connector *connector,
> +				       struct drm_dp_mst_port *port)
> +{
> +	DRM_DEBUG_KMS("registering %s remote bus for %s\n",
> +		      port->aux.name, connector->kdev->kobj.name);
> +
> +	port->aux.dev = connector->kdev;
> +	return drm_dp_aux_register_devnode(&port->aux);
> +}
> +EXPORT_SYMBOL(drm_dp_mst_connector_late_register);
> +
> +/**
> + * drm_dp_mst_connector_early_unregister() - Early MST connector
> unregistration
> + * @drm_connector: The MST connector
> + * @port: The MST port for this connector
> + *
> + * Helper to unregister the remote aux device for this MST port, registered
> by
> + * drm_dp_mst_connector_late_register(). Drivers should call this from
> their mst
> + * connector's early_unregister hook.
> + */

…and this one.

> +void drm_dp_mst_connector_early_unregister(struct drm_connector *connector,
> +					   struct drm_dp_mst_port *port)
> +{
> +	DRM_DEBUG_KMS("unregistering %s remote bus for %s\n",
> +		      port->aux.name, connector->kdev->kobj.name);
> +	drm_dp_aux_unregister_devnode(&port->aux);
> +}
> +EXPORT_SYMBOL(drm_dp_mst_connector_early_unregister);
> +
>  static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
>  			    struct drm_device *dev,
>  			    struct drm_dp_link_addr_reply_port *port_msg)
> @@ -1548,6 +1636,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch
> *mstb,
>  		port->mgr = mstb->mgr;
>  		port->aux.name = "DPMST";
>  		port->aux.dev = dev->dev;
> +		port->aux.is_remote = true;
>  
>  		/*
>  		 * Make sure the memory allocation for our parent branch stays
> @@ -1816,7 +1905,6 @@ static bool drm_dp_validate_guid(struct
> drm_dp_mst_topology_mgr *mgr,
>  	return false;
>  }
>  
> -#if 0
>  static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num,
> u32 offset, u8 num_bytes)
>  {
>  	struct drm_dp_sideband_msg_req_body req;
> @@ -1829,7 +1917,6 @@ static int build_dpcd_read(struct
> drm_dp_sideband_msg_tx *msg, u8 port_num, u32
>  
>  	return 0;
>  }
> -#endif
>  
>  static int drm_dp_send_sideband_msg(struct drm_dp_mst_topology_mgr *mgr,
>  				    bool up, u8 *msg, int len)
> @@ -2441,26 +2528,56 @@ int drm_dp_update_payload_part2(struct
> drm_dp_mst_topology_mgr *mgr)
>  }
>  EXPORT_SYMBOL(drm_dp_update_payload_part2);
>  
> -#if 0 /* unused as of yet */
>  static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
>  				 struct drm_dp_mst_port *port,
> -				 int offset, int size)
> +				 int offset, int size, u8 *bytes)
>  {
>  	int len;
> +	int ret = 0;
>  	struct drm_dp_sideband_msg_tx *txmsg;
> +	struct drm_dp_mst_branch *mstb;
> +
> +	mstb = drm_dp_mst_topology_get_mstb_validated(mgr, port->parent);
> +	if (!mstb)
> +		return -EINVAL;
>  
>  	txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> -	if (!txmsg)
> -		return -ENOMEM;
> +	if (!txmsg) {
> +		ret = -ENOMEM;
> +		goto fail_put;
> +	}
>  
> -	len = build_dpcd_read(txmsg, port->port_num, 0, 8);
> +	len = build_dpcd_read(txmsg, port->port_num, offset, size);
>  	txmsg->dst = port->parent;
>  
>  	drm_dp_queue_down_tx(mgr, txmsg);
>  
> -	return 0;
> +	ret = drm_dp_mst_wait_tx_reply(mstb, txmsg);
> +	if (ret < 0)
> +		goto fail_free;
> +
> +	/* DPCD read should never be NACKed */
> +	if (WARN_ON_ONCE(txmsg->reply.reply_type == 1)) {
> +		ret = -EIO;
> +		goto fail_free;
> +	}
> +
> +	if (txmsg->reply.u.remote_dpcd_read_ack.num_bytes != size) {
> +		ret = -EPROTO;
> +		goto fail_free;
> +	}
> +
> +	ret = min_t(size_t, txmsg->reply.u.remote_dpcd_read_ack.num_bytes,
> +		    size);
> +	memcpy(bytes, txmsg->reply.u.remote_dpcd_read_ack.bytes, ret);
> +
> +fail_free:
> +	kfree(txmsg);
> +fail_put:
> +	drm_dp_mst_topology_put_mstb(mstb);
> +
> +	return ret;
>  }
> -#endif
>  
>  static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
>  				  struct drm_dp_mst_port *port,
> @@ -2489,7 +2606,7 @@ static int drm_dp_send_dpcd_write(struct
> drm_dp_mst_topology_mgr *mgr,
>  	ret = drm_dp_mst_wait_tx_reply(mstb, txmsg);
>  	if (ret > 0) {
>  		if (txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK)
> -			ret = -EINVAL;
> +			ret = -EIO;
>  		else
>  			ret = 0;
>  	}
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 397896b5b21a..cb2deface950 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1309,6 +1309,10 @@ struct drm_dp_aux {
>  	 * @cec: struct containing fields used for CEC-Tunneling-over-AUX.
>  	 */
>  	struct drm_dp_aux_cec cec;
> +	/**
> +	 * @is_remote: Is this "AUX CH" actually using sideband messaging.

I'd remove the quotations around AUX CH, but that's up to you. With those
small whitespace and kernel doc changes addressed:

Reviewed-by: Lyude Paul <lyude@redhat.com>

Thanks for the great work with this!

> +	 */
> +	bool is_remote;
>  };
>  
>  ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
> diff --git a/include/drm/drm_dp_mst_helper.h
> b/include/drm/drm_dp_mst_helper.h
> index 8c97a5f92c47..2ba6253ea6d3 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -643,6 +643,17 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
>  void drm_dp_mst_topology_mgr_suspend(struct drm_dp_mst_topology_mgr *mgr);
>  int __must_check
>  drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr);
> +
> +ssize_t drm_dp_mst_dpcd_read(struct drm_dp_aux *aux,
> +			     unsigned int offset, void *buffer, size_t size);
> +ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
> +			      unsigned int offset, void *buffer, size_t size);
> +
> +int drm_dp_mst_connector_late_register(struct drm_connector *connector,
> +				       struct drm_dp_mst_port *port);
> +void drm_dp_mst_connector_early_unregister(struct drm_connector *connector,
> +					   struct drm_dp_mst_port *port);
> +
>  struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct
> drm_atomic_state *state,
>  								    struct
> drm_dp_mst_topology_mgr *mgr);
>  int __must_check
-- 
Cheers,
	Lyude Paul

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2] drm/sysfs: Add mstpath attribute to connector devices

[Lyude Paul]

gah. So, I was originally going to ask "why didn't we add the connector name
into this?", but then I realized we're doing the right thing already and just
doing card%d-%s % (card_number, path_prop). Which means we probably really don't
want to add a property called mstpath, since it's hardly different from path
(whoops!).

Additionally, after some thinking I realized I may have made a mistake as I'm
not entirely sure if we would need to specify the DRM card in the path prop for
udev, considering that's specified in the sysfs path all ready. Even if I'm
wrong on that though, I think it might be better not to add an mstpath property
and just go the route of just adding a new path_v2 property that we could use
for both MST and non-MST connector paths. (I cc'd you on the email thread about
this, so you can read more about this there.

So, I would actually suggest we just drop this patch entirely for now. We should
be fine without it, even though the dp_aux_dev paths will be kind of ugly for a
little while. I'd rather the rest of this series get upstream first, and try to
do the path prop stuff separately.


On Fri, 2019-07-05 at 10:32 -0400, sunpeng.li@amd.com wrote:
> From: Leo Li <sunpeng.li@amd.com>
> 
> This can be used to create more descriptive symlinks for MST aux
> devices. Consider the following udev rule:
> 
> SUBSYSTEM=="drm_dp_aux_dev", SUBSYSTEMS=="drm", ATTRS{mstpath}=="?*",
> 	SYMLINK+="drm_dp_aux/by-path/$attr{mstpath}"
> 
> The following symlinks will be created (depending on your MST topology):
> 
> $ ls /dev/drm_dp_aux/by-path/
> card0-mst:0-1  card0-mst:0-1-1  card0-mst:0-1-8  card0-mst:0-8
> 
> v2: remove unnecessary locking of mode_config.mutex
> 
> Signed-off-by: Leo Li <sunpeng.li@amd.com>
> ---
>  drivers/gpu/drm/drm_sysfs.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index ad10810bc972..7d483ab684a0 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -236,16 +236,36 @@ static ssize_t modes_show(struct device *device,
>  	return written;
>  }
>  
> +static ssize_t mstpath_show(struct device *device,
> +			    struct device_attribute *attr,
> +			    char *buf)
> +{
> +	struct drm_connector *connector = to_drm_connector(device);
> +	ssize_t ret = 0;
> +	char *path;
> +
> +	if (!connector->path_blob_ptr)
> +		return ret;
> +
> +	path = connector->path_blob_ptr->data;
> +	ret = snprintf(buf, PAGE_SIZE, "card%d-%s\n",
> +		       connector->dev->primary->index, path);
> +
> +	return ret;
> +}
> +
>  static DEVICE_ATTR_RW(status);
>  static DEVICE_ATTR_RO(enabled);
>  static DEVICE_ATTR_RO(dpms);
>  static DEVICE_ATTR_RO(modes);
> +static DEVICE_ATTR_RO(mstpath);
>  
>  static struct attribute *connector_dev_attrs[] = {
>  	&dev_attr_status.attr,
>  	&dev_attr_enabled.attr,
>  	&dev_attr_dpms.attr,
>  	&dev_attr_modes.attr,
> +	&dev_attr_mstpath.attr,
>  	NULL
>  };
>  

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 02/10] drm/dp_mst: Enable registration of AUX devices for MST ports (v2)

[Lyude Paul]

On Thu, 2019-07-04 at 15:05 -0400, sunpeng.li@amd.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> All available downstream ports - physical and logical - are exposed for
> each MST device. They are listed in /dev/, following the same naming
> scheme as SST devices by appending an incremental ID.
> 
> Although all downstream ports are exposed, only some will work as
> expected. Consider the following topology:
> 
>                +---------+
>                |  ASIC   |
>                +---------+
>               Conn-0|
>                     |
>                +----v----+
>           +----| MST HUB |----+
>           |    +---------+    |
>           |                   |
>           |Port-1       Port-2|
>     +-----v-----+       +-----v-----+
>     |  MST      |       |  SST      |
>     |  Display  |       |  Display  |
>     +-----------+       +-----------+
>           |Port-1
>           x
> 
>  MST Path  | MST Device
>  ----------+----------------------------------
>  sst:0     | MST Hub
>  mst:0-1   | MST Display
>  mst:0-1-1 | MST Display's disconnected DP out
>  mst:0-1-8 | MST Display's internal sink
>  mst:0-2   | SST Display
> 
> On certain MST displays, the upstream physical port will ACK DPCD reads.
> However, reads on the local logical port to the internal sink will
> *NAK*. i.e. reading mst:0-1 ACKs, but mst:0-1-8 NAKs.
> 
> There may also be duplicates. Some displays will return the same GUID
> when reading DPCD from both mst:0-1 and mst:0-1-8.
> 
> There are some device-dependent behavior as well. The MST hub used
> during testing will actually *ACK* read requests on a disconnected
> physical port, whereas the MST displays will NAK.
> 
> In light of these discrepancies, it's simpler to expose all downstream
> ports - both physical and logical - and let the user decide what to use.
> 
> (v2) changes:
> 
> Moved remote aux device (un)registration to new mst connector late
> register and early unregister helpers. Drivers should call these from
> their own mst connector function hooks.
> 
> This is to solve an issue during driver unload, where mst connector
> devices are unregistered before the remote aux devices are. In a setup
> where aux devices are created as children of connector devices, the aux
> device would be removed too early, and uncleanly. Doing so in
> early_unregister solves this issue, as that is called before connector
> unregistration.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Leo Li <sunpeng.li@amd.com>
> ---
>  drivers/gpu/drm/drm_dp_aux_dev.c      |  16 ++-
>  drivers/gpu/drm/drm_dp_mst_topology.c | 137 ++++++++++++++++++++++++--
>  include/drm/drm_dp_helper.h           |   4 +
>  include/drm/drm_dp_mst_helper.h       |  11 +++
>  4 files changed, 156 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c
> b/drivers/gpu/drm/drm_dp_aux_dev.c
> index 26e38dacf654..4aa5e455e894 100644
> --- a/drivers/gpu/drm/drm_dp_aux_dev.c
> +++ b/drivers/gpu/drm/drm_dp_aux_dev.c
> @@ -37,6 +37,7 @@
>  
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_dp_helper.h>
> +#include <drm/drm_dp_mst_helper.h>
>  #include <drm/drm_print.h>
>  
>  #include "drm_crtc_helper_internal.h"
> @@ -116,6 +117,7 @@ static ssize_t name_show(struct device *dev,
>  
>  	return res;
>  }
> +
>  static DEVICE_ATTR_RO(name);
>  
>  static struct attribute *drm_dp_aux_attrs[] = {
> @@ -162,7 +164,12 @@ static ssize_t auxdev_read_iter(struct kiocb *iocb,
> struct iov_iter *to)
>  			break;
>  		}
>  
> -		res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo);
> +		if (aux_dev->aux->is_remote)
> +			res = drm_dp_mst_dpcd_read(aux_dev->aux, pos, buf,
> +						   todo);
> +		else
> +			res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo);
> +
>  		if (res <= 0)
>  			break;
>  
> @@ -209,7 +216,12 @@ static ssize_t auxdev_write_iter(struct kiocb *iocb,
> struct iov_iter *from)
>  			break;
>  		}
>  
> -		res = drm_dp_dpcd_write(aux_dev->aux, pos, buf, todo);
> +		if (aux_dev->aux->is_remote)
> +			res = drm_dp_mst_dpcd_write(aux_dev->aux, pos, buf,
> +						    todo);
> +		else
> +			res = drm_dp_dpcd_write(aux_dev->aux, pos, buf, todo);
> +
>  		if (res <= 0)
>  			break;
>  
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 0984b9a34d55..dde79c44b625 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -36,6 +36,8 @@
>  #include <drm/drm_print.h>
>  #include <drm/drm_probe_helper.h>
>  
> +#include "drm_crtc_helper_internal.h"
> +
>  /**
>   * DOC: dp mst helper
>   *
> @@ -53,6 +55,9 @@ static int drm_dp_dpcd_write_payload(struct
> drm_dp_mst_topology_mgr *mgr,
>  				     int id,
>  				     struct drm_dp_payload *payload);
>  
> +static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
> +				 struct drm_dp_mst_port *port,
> +				 int offset, int size, u8 *bytes);
>  static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
>  				  struct drm_dp_mst_port *port,
>  				  int offset, int size, u8 *bytes);
> @@ -1238,6 +1243,9 @@ static void drm_dp_destroy_port(struct kref *kref)
>  	struct drm_dp_mst_topology_mgr *mgr = port->mgr;
>  
>  	if (!port->input) {
> +
> +		port->vcpi.num_slots = 0;
> +
>  		kfree(port->cached_edid);
>  
>  		/*
> @@ -1483,6 +1491,48 @@ static bool drm_dp_port_setup_pdt(struct
> drm_dp_mst_port *port)
>  	return send_link;
>  }
>  
> +/**
> + * drm_dp_mst_dpcd_read() - read a series of bytes from the DPCD via sideband
> + * @aux: Fake sideband AUX CH
> + * @offset: address of the (first) register to read
> + * @buffer: buffer to store the register values
> + * @size: number of bytes in @buffer
> + *
> + * Performs the same functionality for remote devices via
> + * sideband messaging as drm_dp_dpcd_read() does for local
> + * devices via actual AUX CH.
> + */
> +ssize_t drm_dp_mst_dpcd_read(struct drm_dp_aux *aux,
> +			     unsigned int offset, void *buffer, size_t size)
> +{
> +	struct drm_dp_mst_port *port = container_of(aux, struct drm_dp_mst_port,
> +						    aux);
> +
> +	return drm_dp_send_dpcd_read(port->mgr, port,
> +				     offset, size, buffer);
> +}
> +
> +/**
> + * drm_dp_mst_dpcd_write() - write a series of bytes to the DPCD via sideband
> + * @aux: Fake sideband AUX CH
> + * @offset: address of the (first) register to write
> + * @buffer: buffer containing the values to write
> + * @size: number of bytes in @buffer
> + *
> + * Performs the same functionality for remote devices via
> + * sideband messaging as drm_dp_dpcd_write() does for local
> + * devices via actual AUX CH.
> + */
> +ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
> +			      unsigned int offset, void *buffer, size_t size)
> +{
> +	struct drm_dp_mst_port *port = container_of(aux, struct drm_dp_mst_port,
> +						    aux);
> +
> +	return drm_dp_send_dpcd_write(port->mgr, port,
> +				      offset, size, buffer);
> +}
> +
>  static void drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb, u8 *guid)
>  {
>  	int ret;
> @@ -1526,6 +1576,44 @@ static void build_mst_prop_path(const struct
> drm_dp_mst_branch *mstb,
>  	strlcat(proppath, temp, proppath_size);
>  }
>  
> +/**
> + * drm_dp_mst_connector_late_register() - Late MST connector registration
> + * @drm_connector: The MST connector
> + * @port: The MST port for this connector
> + *
> + * Helper to register the remote aux device for this MST port. Drivers should
> + * call this from their mst connector's late_register hook to enable MST aux
> + * devices.
> + */
> +int drm_dp_mst_connector_late_register(struct drm_connector *connector,
> +				       struct drm_dp_mst_port *port)
> +{
> +	DRM_DEBUG_KMS("registering %s remote bus for %s\n",
> +		      port->aux.name, connector->kdev->kobj.name);
> +
> +	port->aux.dev = connector->kdev;
> +	return drm_dp_aux_register_devnode(&port->aux);
> +}
> +EXPORT_SYMBOL(drm_dp_mst_connector_late_register);
> +
> +/**
> + * drm_dp_mst_connector_early_unregister() - Early MST connector
> unregistration
> + * @drm_connector: The MST connector
> + * @port: The MST port for this connector
> + *
> + * Helper to unregister the remote aux device for this MST port, registered
> by
> + * drm_dp_mst_connector_late_register(). Drivers should call this from their
> mst
> + * connector's early_unregister hook.
> + */
> +void drm_dp_mst_connector_early_unregister(struct drm_connector *connector,
> +					   struct drm_dp_mst_port *port)
> +{
> +	DRM_DEBUG_KMS("unregistering %s remote bus for %s\n",
> +		      port->aux.name, connector->kdev->kobj.name);
> +	drm_dp_aux_unregister_devnode(&port->aux);
> +}
> +EXPORT_SYMBOL(drm_dp_mst_connector_early_unregister);
> +
>  static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
>  			    struct drm_device *dev,
>  			    struct drm_dp_link_addr_reply_port *port_msg)
> @@ -1548,6 +1636,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch
> *mstb,
>  		port->mgr = mstb->mgr;
>  		port->aux.name = "DPMST";
>  		port->aux.dev = dev->dev;
> +		port->aux.is_remote = true;
>  
>  		/*
>  		 * Make sure the memory allocation for our parent branch stays
> @@ -1816,7 +1905,6 @@ static bool drm_dp_validate_guid(struct
> drm_dp_mst_topology_mgr *mgr,
>  	return false;
>  }
>  
> -#if 0
>  static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num,
> u32 offset, u8 num_bytes)
>  {
>  	struct drm_dp_sideband_msg_req_body req;
> @@ -1829,7 +1917,6 @@ static int build_dpcd_read(struct drm_dp_sideband_msg_tx
> *msg, u8 port_num, u32
>  
>  	return 0;
>  }
> -#endif
>  
>  static int drm_dp_send_sideband_msg(struct drm_dp_mst_topology_mgr *mgr,
>  				    bool up, u8 *msg, int len)
> @@ -2441,26 +2528,56 @@ int drm_dp_update_payload_part2(struct
> drm_dp_mst_topology_mgr *mgr)
>  }
>  EXPORT_SYMBOL(drm_dp_update_payload_part2);
>  
> -#if 0 /* unused as of yet */
>  static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
>  				 struct drm_dp_mst_port *port,
> -				 int offset, int size)
> +				 int offset, int size, u8 *bytes)
>  {
>  	int len;
> +	int ret = 0;
>  	struct drm_dp_sideband_msg_tx *txmsg;
> +	struct drm_dp_mst_branch *mstb;
> +
> +	mstb = drm_dp_mst_topology_get_mstb_validated(mgr, port->parent);
> +	if (!mstb)
> +		return -EINVAL;
>  
>  	txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> -	if (!txmsg)
> -		return -ENOMEM;
> +	if (!txmsg) {
> +		ret = -ENOMEM;
> +		goto fail_put;
> +	}
>  
> -	len = build_dpcd_read(txmsg, port->port_num, 0, 8);
> +	len = build_dpcd_read(txmsg, port->port_num, offset, size);
>  	txmsg->dst = port->parent;
>  
>  	drm_dp_queue_down_tx(mgr, txmsg);
>  
> -	return 0;
> +	ret = drm_dp_mst_wait_tx_reply(mstb, txmsg);
> +	if (ret < 0)
> +		goto fail_free;
> +
> +	/* DPCD read should never be NACKed */
> +	if (WARN_ON_ONCE(txmsg->reply.reply_type == 1)) {

agh-sorry, one last nitpick: I think we might want to change this from a
WARN_ON_ONCE() to a simple DRM_ERROR. On my lenovo P50's laptop dock, trying to 
read from non-native DP devices actually results in NAKs. No reason to taint
someone's kernel for this.

> +		ret = -EIO;
> +		goto fail_free;
> +	}
> +
> +	if (txmsg->reply.u.remote_dpcd_read_ack.num_bytes != size) {
> +		ret = -EPROTO;
> +		goto fail_free;
> +	}
> +
> +	ret = min_t(size_t, txmsg->reply.u.remote_dpcd_read_ack.num_bytes,
> +		    size);
> +	memcpy(bytes, txmsg->reply.u.remote_dpcd_read_ack.bytes, ret);
> +
> +fail_free:
> +	kfree(txmsg);
> +fail_put:
> +	drm_dp_mst_topology_put_mstb(mstb);
> +
> +	return ret;
>  }
> -#endif
>  
>  static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
>  				  struct drm_dp_mst_port *port,
> @@ -2489,7 +2606,7 @@ static int drm_dp_send_dpcd_write(struct
> drm_dp_mst_topology_mgr *mgr,
>  	ret = drm_dp_mst_wait_tx_reply(mstb, txmsg);
>  	if (ret > 0) {
>  		if (txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK)
> -			ret = -EINVAL;
> +			ret = -EIO;
>  		else
>  			ret = 0;
>  	}
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 397896b5b21a..cb2deface950 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1309,6 +1309,10 @@ struct drm_dp_aux {
>  	 * @cec: struct containing fields used for CEC-Tunneling-over-AUX.
>  	 */
>  	struct drm_dp_aux_cec cec;
> +	/**
> +	 * @is_remote: Is this "AUX CH" actually using sideband messaging.
> +	 */
> +	bool is_remote;
>  };
>  
>  ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index 8c97a5f92c47..2ba6253ea6d3 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -643,6 +643,17 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
>  void drm_dp_mst_topology_mgr_suspend(struct drm_dp_mst_topology_mgr *mgr);
>  int __must_check
>  drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr);
> +
> +ssize_t drm_dp_mst_dpcd_read(struct drm_dp_aux *aux,
> +			     unsigned int offset, void *buffer, size_t size);
> +ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
> +			      unsigned int offset, void *buffer, size_t size);
> +
> +int drm_dp_mst_connector_late_register(struct drm_connector *connector,
> +				       struct drm_dp_mst_port *port);
> +void drm_dp_mst_connector_early_unregister(struct drm_connector *connector,
> +					   struct drm_dp_mst_port *port);
> +
>  struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct
> drm_atomic_state *state,
>  								    struct
> drm_dp_mst_topology_mgr *mgr);
>  int __must_check

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

Re: [PATCH 07/10] drm/i915: Implement MST Aux device registration

[Lyude Paul]

BTW, I just tried these patches on my T450s (using i915) and I'm seeing some
kernel warnings with them when adding DP aux devices after connecting a new
MST topology to the system: 

[  296.511130] kauditd_printk_skb: 2 callbacks suppressed
[  296.511135] audit: type=1305 audit(1562960713.086:245): op=set audit_pid=0 old=743 auid=4294967295 ses=4294967295 res=1
[  296.513594] audit: type=1131 audit(1562960713.088:246): pid=1 uid=0 auid=4294967295 ses=4294967295 msg='unit=auditd comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? res=success'
[  298.506015] audit: type=1305 audit(1562960715.080:247): op=set audit_enabled=1 old=1 auid=4294967295 ses=4294967295 res=1
[  298.506057] audit: type=1305 audit(1562960715.080:248): op=set audit_pid=7270 old=0 auid=4294967295 ses=4294967295 res=1
[  367.725603] [drm:intel_get_hpd_pins.isra.0 [i915]] hotplug event received, stat 0x00400000, dig 0x10101210, pins 0x00000040, long 0x00000040
[  367.725684] [drm:intel_hpd_irq_handler [i915]] digital hpd port C - long
[  367.725760] [drm:intel_hpd_irq_handler [i915]] Received HPD interrupt on PIN 6 - cnt: 10
[  367.725888] [drm:intel_dp_hpd_pulse [i915]] got hpd irq on port C - long
[  367.725977] [drm:i915_hotplug_work_func [i915]] running encoder hotplug functions
[  367.726068] [drm:i915_hotplug_work_func [i915]] Connector DP-2 (pin 6) received hotplug event.
[  367.726165] [drm:intel_dp_detect [i915]] [CONNECTOR:86:DP-2]
[  367.726679] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x00000 AUX -> (ret= 15) 12 14 c4 01 00 05 01 83 02 00 00 00 00 00 04
[  367.726748] [drm:intel_dp_read_dpcd [i915]] DPCD: 12 14 c4 01 00 05 01 83 02 00 00 00 00 00 04
[  367.727117] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x00200 AUX -> (ret=  1) 01
[  367.727787] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x00080 AUX -> (ret= 16) 08 0d 0d 00 00 00 00 00 00 00 00 00 00 00 00 00
[  367.727886] [drm:intel_dp_print_rates [i915]] source rates: 162000, 270000, 540000
[  367.727974] [drm:intel_dp_print_rates [i915]] sink rates: 162000, 270000, 540000
[  367.728047] [drm:intel_dp_print_rates [i915]] common rates: 162000, 270000, 540000
[  367.728548] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x00500 AUX -> (ret= 12) 90 cc 24 53 59 4e 41 23 22 10 02 1d
[  367.728597] [drm:drm_dp_read_desc [drm_kms_helper]] DP branch: OUI 90-cc-24 dev-ID SYNA#" HW-rev 1.0 SW-rev 2.29 quirks 0x0000
[  367.729069] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x00021 AUX -> (ret=  1) 01
[  367.729159] [drm:intel_dp_detect [i915]] MST support? port C: yes, sink: yes, modparam: yes
[  367.729805] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x00000 AUX -> (ret= 15) 12 14 c4 01 00 05 01 83 02 00 00 00 00 00 04
[  367.729830] [drm:drm_dp_mst_topology_mgr_set_mst [drm_kms_helper]] mstb 000000001b4abbfa (2)
[  367.730301] [drm:drm_dp_dpcd_write [drm_kms_helper]] DPDDC-C: 0x00111 AUX <- (ret=  1) 07
[  367.730868] [drm:drm_dp_dpcd_write [drm_kms_helper]] DPDDC-C: 0x002c0 AUX <- (ret=  1) 01
[  367.731086] [drm:drm_dp_dpcd_write [drm_kms_helper]] DPDDC-C: 0x001c0 AUX <- (ret=  3) 00 00 3f
[  367.731432] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x002c0 AUX -> (ret=  1) 01
[  367.731448] [drm:drm_dp_mst_topology_put_mstb [drm_kms_helper]] mstb 000000001b4abbfa (1)
[  367.731521] [drm:i915_hotplug_work_func [i915]] Connector HDMI-A-2 (pin 6) received hotplug event.
[  367.731587] [drm:intel_hdmi_detect [i915]] [CONNECTOR:91:HDMI-A-2]
[  367.731784] [drm:drm_dp_mst_topology_try_get_mstb.part.0 [drm_kms_helper]] mstb 000000001b4abbfa (2)
[  367.732133] [drm:drm_dp_dpcd_write [drm_kms_helper]] DPDDC-C: 0x01000 AUX <- (ret=  5) 10 02 cb 01 d5
[  367.732249] [drm:do_gmbus_xfer [i915]] GMBUS [i915 gmbus dpc] NAK for addr: 0050 w(1)
[  367.732328] [drm:do_gmbus_xfer [i915]] GMBUS [i915 gmbus dpc] NAK on first message, retry
[  367.732568] [drm:do_gmbus_xfer [i915]] GMBUS [i915 gmbus dpc] NAK for addr: 0050 w(1)
[  367.732630] [drm:drm_do_probe_ddc_edid [drm]] drm: skipping non-existent adapter i915 gmbus dpc
[  367.732726] [drm:intel_hdmi_set_edid [i915]] HDMI GMBUS EDID read failed, retry using GPIO bit-banging
[  367.732820] [drm:intel_gmbus_force_bit [i915]] enabling bit-banging on i915 gmbus dpc. force bit now 1
[  367.733406] [drm:intel_get_hpd_pins.isra.0 [i915]] hotplug event received, stat 0x00400000, dig 0x10101110, pins 0x00000040, long 0x00000000
[  367.733491] [drm:intel_hpd_irq_handler [i915]] digital hpd port C - short
[  367.733616] [drm:intel_dp_hpd_pulse [i915]] got hpd irq on port C - short
[  367.733711] [drm:drm_do_probe_ddc_edid [drm]] drm: skipping non-existent adapter i915 gmbus dpc
[  367.733826] [drm:intel_gmbus_force_bit [i915]] disabling bit-banging on i915 gmbus dpc. force bit now 0
[  367.734104] [drm:do_gmbus_xfer [i915]] GMBUS [i915 gmbus dpc] NAK for addr: 0040 w(1)
[  367.734197] [drm:do_gmbus_xfer [i915]] GMBUS [i915 gmbus dpc] NAK on first message, retry
[  367.734234] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x02002 AUX -> (ret= 14) 01 10 00 00 00 00 00 00 00 00 00 00 80 00
[  367.734335] [drm:intel_dp_hpd_pulse [i915]] got esi 01 10 00
[  367.734510] [drm:do_gmbus_xfer [i915]] GMBUS [i915 gmbus dpc] NAK for addr: 0040 w(1)
[  367.734539] [drm:drm_dp_dual_mode_detect [drm_kms_helper]] DP dual mode HDMI ID:  (err -6)
[  367.734893] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x01400 AUX -> (ret= 16) 10 2d 8c 01 00 00 00 00 00 00 00 00 00 00 00 00
[  367.735446] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x01410 AUX -> (ret= 16) 00 00 00 00 04 90 c0 31 40 11 00 00 00 00 00 00
[  367.735994] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x01420 AUX -> (ret= 16) 00 00 00 00 00 00 00 00 00 00 11 02 00 00 00 16
[  367.736229] [drm:drm_dp_dpcd_write [drm_kms_helper]] DPDDC-C: 0x02003 AUX <- (ret=  3) 10 00 00
[  367.736713] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x02002 AUX -> (ret= 14) 01 00 00 00 00 00 00 00 00 00 00 00 80 00
[  367.736737] [drm:intel_dp_hpd_pulse [i915]] got esi2 01 00 00
[  367.736756] [drm:intel_dp_hpd_pulse [i915]] got esi 01 00 00
[  367.737420] [drm:intel_get_hpd_pins.isra.0 [i915]] hotplug event received, stat 0x00400000, dig 0x10101110, pins 0x00000040, long 0x00000000
[  367.737441] [drm:intel_hpd_irq_handler [i915]] digital hpd port C - short
[  367.737468] [drm:intel_dp_hpd_pulse [i915]] got hpd irq on port C - short
[  367.737939] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x02002 AUX -> (ret= 14) 01 10 00 00 00 00 00 00 00 00 00 00 80 00
[  367.737961] [drm:intel_dp_hpd_pulse [i915]] got esi 01 10 00
[  367.738452] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x01400 AUX -> (ret= 16) 10 25 45 00 00 00 00 00 00 00 00 00 00 00 00 00
[  367.738940] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x01410 AUX -> (ret= 16) 00 00 00 03 00 00 00 00 00 00 00 00 00 00 00 00
[  367.739331] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x01420 AUX -> (ret=  8) 00 00 00 00 00 00 00 a9
[  367.739336] [drm:drm_dp_mst_topology_try_get_mstb.part.0 [drm_kms_helper]] mstb 000000001b4abbfa (3)
[  367.739340] [drm:drm_dp_mst_topology_put_mstb [drm_kms_helper]] mstb 000000001b4abbfa (2)
[  367.739372] [drm:drm_dp_send_link_address [drm_kms_helper]] link address reply: 4
[  367.739378] [drm:drm_dp_send_link_address [drm_kms_helper]] port 0: input 1, pdt: 1, pn: 0, dpcd_rev: 00, mcs: 1, ddps: 1, ldps 0, sdp 0/0
[  367.739384] [drm:drm_dp_send_link_address [drm_kms_helper]] port 1: input 0, pdt: 3, pn: 1, dpcd_rev: 11, mcs: 0, ddps: 1, ldps 0, sdp 1/1
[  367.739390] [drm:drm_dp_send_link_address [drm_kms_helper]] port 2: input 0, pdt: 0, pn: 2, dpcd_rev: 00, mcs: 0, ddps: 0, ldps 0, sdp 0/0
[  367.739396] [drm:drm_dp_send_link_address [drm_kms_helper]] port 3: input 0, pdt: 0, pn: 3, dpcd_rev: 00, mcs: 0, ddps: 0, ldps 0, sdp 0/0
[  367.739531] [drm:drm_dp_dpcd_write [drm_kms_helper]] DPDDC-C: 0x02003 AUX <- (ret=  3) 10 00 00
[  367.739986] [drm:drm_dp_dpcd_write [drm_kms_helper]] DPDDC-C: 0x00030 AUX <- (ret= 16) 31 08 01 00 01 00 00 00 31 08 01 00 01 00 00 00
[  367.739991] [drm:drm_dp_add_port [drm_kms_helper]] mstb 000000001b4abbfa (2)
[  367.739997] [drm:drm_dp_add_port [drm_kms_helper]] port 000000007ac74091 (2)
[  367.740002] [drm:drm_dp_mst_topology_put_port [drm_kms_helper]] port 000000007ac74091 (1)
[  367.740009] [drm:drm_dp_add_port [drm_kms_helper]] mstb 000000001b4abbfa (3)
[  367.740014] [drm:drm_dp_add_port [drm_kms_helper]] port 000000006e23aef3 (2)
[  367.740289] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x02002 AUX -> (ret= 14) 01 00 00 00 00 00 00 00 00 00 00 00 80 00
[  367.740314] [drm:intel_dp_hpd_pulse [i915]] got esi2 01 00 00
[  367.740337] [drm:intel_dp_hpd_pulse [i915]] got esi 01 00 00
[  367.740497] [drm:drm_dp_dpcd_write [drm_kms_helper]] DPDDC-C: 0x01000 AUX <- (ret=  6) 10 43 d4 10 10 e2
[  367.741445] [drm:intel_get_hpd_pins.isra.0 [i915]] hotplug event received, stat 0x00400000, dig 0x10101110, pins 0x00000040, long 0x00000000
[  367.741462] [drm:intel_hpd_irq_handler [i915]] digital hpd port C - short
[  367.741482] [drm:intel_dp_hpd_pulse [i915]] got hpd irq on port C - short
[  367.741939] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x02002 AUX -> (ret= 14) 01 10 00 00 00 00 00 00 00 00 00 00 80 00
[  367.741958] [drm:intel_dp_hpd_pulse [i915]] got esi 01 10 00
[  367.742447] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x01400 AUX -> (ret= 16) 10 47 d3 10 10 05 00 05 00 3c 00 00 00 00 00 00
[  367.742452] [drm:drm_dp_mst_topology_try_get_mstb.part.0 [drm_kms_helper]] mstb 000000001b4abbfa (3)
[  367.742456] [drm:drm_dp_mst_topology_put_mstb [drm_kms_helper]] mstb 000000001b4abbfa (2)
[  367.742471] [drm:drm_dp_send_enum_path_resources.isra.0 [drm_kms_helper]] enum path resources 1: 1280 1280
[  367.742526] [drm:drm_dp_mst_get_port_malloc [drm_kms_helper]] port 000000006e23aef3 (2)
[  367.742571] WARNING: CPU: 2 PID: 442 at drivers/gpu/drm/drm_mode_object.c:45 __drm_mode_object_add+0xaa/0xb0 [drm]
[  367.742651] Modules linked in: vfat fat joydev rmi_smbus rmi_core i915 wmi_bmof iTCO_wdt intel_rapl x86_pkg_temp_thermal i2c_algo_bit coretemp drm_kms_helper crc32_pclmul syscopyarea sysfillrect sysimgblt psmouse fb_sys_fops i2c_i801 drm i2c_core lpc_ich mei_me mfd_core thinkpad_acpi mei ledtrig_audio wmi tpm_tis tpm_tis_core tpm video pcc_cpufreq crc32c_intel serio_raw ehci_pci ehci_hcd xhci_pci xhci_hcd
[  367.742859] CPU: 2 PID: 442 Comm: kworker/2:3 Kdump: loaded Not tainted 5.2.0Lyude-Test+ #1
[  367.742905] Hardware name: LENOVO 20BWS1KY00/20BWS1KY00, BIOS JBET71WW (1.35 ) 09/14/2018
[  367.742957] Workqueue: events_long drm_dp_mst_link_probe_work [drm_kms_helper]
[  367.743011] RIP: 0010:__drm_mode_object_add+0xaa/0xb0 [drm]
[  367.743044] Code: 41 c7 44 24 10 01 00 00 00 4c 89 ff e8 4f 58 6b e1 85 db b8 00 00 00 00 0f 4e c3 48 83 c4 08 5b 41 5c 41 5d 41 5e 41 5f 5d c3 <0f> 0b eb 82 66 90 0f 1f 44 00 00 55 45 31 c0 b9 01 00 00 00 48 89
[  367.743143] RSP: 0018:ffffc90000447b68 EFLAGS: 00010246
[  367.743174] RAX: ffff888319350df0 RBX: ffff88830a840000 RCX: 0000000000000001
[  367.743214] RDX: 00000000b0b0b0b0 RSI: ffff888313ad2e90 RDI: ffff88830a840000
[  367.743254] RBP: ffffc90000447b98 R08: 0000000000000000 R09: ffff888319350df0
[  367.743294] R10: 0000000000000000 R11: 0000000000000000 R12: ffff888313ad2e90
[  367.743334] R13: 0000000000000001 R14: 0000000000000000 R15: ffffffffa01b0d7a
[  367.743375] FS:  0000000000000000(0000) GS:ffff88831da80000(0000) knlGS:0000000000000000
[  367.743421] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  367.743453] CR2: 00007f733dd19d88 CR3: 000000000220a006 CR4: 00000000003606e0
[  367.743494] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  367.743534] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[  367.743575] Call Trace:
[  367.743604]  drm_mode_object_add+0x16/0x20 [drm]
[  367.743643]  drm_property_create+0xd4/0x1b0 [drm]
[  367.743682]  drm_property_create_range+0x1f/0x40 [drm]
[  367.743722]  drm_connector_attach_max_bpc_property+0x69/0x90 [drm]
[  367.743784]  intel_dp_add_mst_connector+0x130/0x140 [i915]
[  367.743822]  drm_dp_add_port+0x222/0x490 [drm_kms_helper]
[  367.743856]  ? memory_bm_create+0x1d1/0x4a0
[  367.743886]  ? drm_dp_dpcd_write+0x61/0xb0 [drm_kms_helper]
[  367.743930]  ? drm_dbg+0x87/0x90 [drm]
[  367.743958]  ? drm_dp_dpcd_write+0x61/0xb0 [drm_kms_helper]
[  367.743996]  drm_dp_send_link_address+0x162/0x1c0 [drm_kms_helper]
[  367.744037]  drm_dp_check_and_send_link_address+0xa2/0xb0 [drm_kms_helper]
[  367.744081]  drm_dp_mst_link_probe_work+0x69/0x90 [drm_kms_helper]
[  367.744118]  process_one_work+0x198/0x380
[  367.744143]  worker_thread+0x4d/0x3a0
[  367.744166]  kthread+0x106/0x140
[  367.744188]  ? process_one_work+0x380/0x380
[  367.744213]  ? kthread_park+0x90/0x90
[  367.744237]  ret_from_fork+0x35/0x40
[  367.744272] WARNING: CPU: 2 PID: 442 at drivers/gpu/drm/drm_mode_object.c:45 __drm_mode_object_add+0xaa/0xb0 [drm]
[  367.744330] ---[ end trace 314c8adc7c8ca583 ]---
[  367.744396] [drm:drm_sysfs_connector_add [drm]] adding "DP-3" to sysfs
[  367.744406] [drm:drm_sysfs_hotplug_event [drm]] generating hotplug event
[  367.744426] [drm:drm_dp_mst_connector_late_register [drm_kms_helper]] registering DPMST remote bus for card0-DP-3
[  367.744454] [drm:drm_dp_dpcd_write [drm_kms_helper]] DPDDC-C: 0x02003 AUX <- (ret=  3) 10 00 00
[  367.744908] [drm:drm_dp_dpcd_read [drm_kms_helper]] DPDDC-C: 0x02002 AUX -> (ret= 14) 01 00 00 00 00 00 00 00 00 00 00 00 80 00
[  367.744927] [drm:intel_dp_hpd_pulse [i915]] got esi2 01 00 00
[  367.744944] [drm:intel_dp_hpd_pulse [i915]] got esi 01 00 00
[  367.745595] [drm:drm_dp_aux_register_devnode [drm_kms_helper]] drm_dp_aux_dev: aux [DPMST] registered as minor 3
[  367.745601] [drm:drm_dp_mst_topology_put_port [drm_kms_helper]] port 000000006e23aef3 (1)
[  367.745605] [drm:drm_dp_add_port [drm_kms_helper]] mstb 000000001b4abbfa (4)
[  367.745609] [drm:drm_dp_add_port [drm_kms_helper]] port 0000000024a17a86 (2)
[  367.745613] [drm:drm_dp_mst_get_port_malloc [drm_kms_helper]] port 0000000024a17a86 (2)
[  367.745635] WARNING: CPU: 2 PID: 442 at drivers/gpu/drm/drm_mode_object.c:45 __drm_mode_object_add+0xaa/0xb0 [drm]
[  367.745668] Modules linked in: vfat fat joydev rmi_smbus rmi_core i915 wmi_bmof iTCO_wdt intel_rapl x86_pkg_temp_thermal i2c_algo_bit coretemp drm_kms_helper crc32_pclmul syscopyarea sysfillrect sysimgblt psmouse fb_sys_fops i2c_i801 drm i2c_core lpc_ich mei_me mfd_core thinkpad_acpi mei ledtrig_audio wmi tpm_tis tpm_tis_core tpm video pcc_cpufreq crc32c_intel serio_raw ehci_pci ehci_hcd xhci_pci xhci_hcd
[  367.745774] CPU: 2 PID: 442 Comm: kworker/2:3 Kdump: loaded Tainted: G        W         5.2.0Lyude-Test+ #1
[  367.745802] Hardware name: LENOVO 20BWS1KY00/20BWS1KY00, BIOS JBET71WW (1.35 ) 09/14/2018
[  367.745847] Workqueue: events_long drm_dp_mst_link_probe_work [drm_kms_helper]
[  367.745899] RIP: 0010:__drm_mode_object_add+0xaa/0xb0 [drm]
[  367.745931] Code: 41 c7 44 24 10 01 00 00 00 4c 89 ff e8 4f 58 6b e1 85 db b8 00 00 00 00 0f 4e c3 48 83 c4 08 5b 41 5c 41 5d 41 5e 41 5f 5d c3 <0f> 0b eb 82 66 90 0f 1f 44 00 00 55 45 31 c0 b9 01 00 00 00 48 89
[  367.746022] RSP: 0018:ffffc90000447b68 EFLAGS: 00010246
[  367.746052] RAX: ffff888319350d90 RBX: ffff88830a840000 RCX: 0000000000000001
[  367.746090] RDX: 00000000b0b0b0b0 RSI: ffff888313ad2c90 RDI: ffff88830a840000
[  367.746128] RBP: ffffc90000447b98 R08: 0000000000000000 R09: ffff888319350d90
[  367.746165] R10: 0000000000000000 R11: 0000000000000000 R12: ffff888313ad2c90
[  367.746204] R13: 0000000000000001 R14: 0000000000000000 R15: ffffffffa01b0d7a
[  367.746242] FS:  0000000000000000(0000) GS:ffff88831da80000(0000) knlGS:0000000000000000
[  367.746285] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  367.748602] CR2: 00007f733dd19d88 CR3: 00000003190ba003 CR4: 00000000003606e0
[  367.750927] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  367.753249] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[  367.755581] Call Trace:
[  367.757571]  drm_mode_object_add+0x16/0x20 [drm]
[  367.759275]  drm_property_create+0xd4/0x1b0 [drm]
[  367.761414]  drm_property_create_range+0x1f/0x40 [drm]
[  367.763605]  drm_connector_attach_max_bpc_property+0x69/0x90 [drm]
[  367.765862]  intel_dp_add_mst_connector+0x130/0x140 [i915]
[  367.768061]  drm_dp_add_port+0x222/0x490 [drm_kms_helper]
[  367.770256]  ? memory_bm_create+0x1d2/0x4a0
[  367.772428]  ? drm_dp_dpcd_write+0x61/0xb0 [drm_kms_helper]
[  367.774552]  ? drm_dbg+0x87/0x90 [drm]
[  367.776554]  ? drm_dp_dpcd_write+0x61/0xb0 [drm_kms_helper]
[  367.777972]  drm_dp_send_link_address+0x162/0x1c0 [drm_kms_helper]
[  367.779153]  drm_dp_check_and_send_link_address+0xa2/0xb0 [drm_kms_helper]
[  367.780406]  drm_dp_mst_link_probe_work+0x69/0x90 [drm_kms_helper]
[  367.781512]  process_one_work+0x198/0x380
[  367.782566]  worker_thread+0x4d/0x3a0
[  367.783619]  kthread+0x106/0x140
[  367.784656]  ? process_one_work+0x380/0x380
[  367.785693]  ? kthread_park+0x90/0x90
[  367.786728]  ret_from_fork+0x35/0x40
[  367.787775] WARNING: CPU: 2 PID: 442 at drivers/gpu/drm/drm_mode_object.c:45 __drm_mode_object_add+0xaa/0xb0 [drm]
[  367.788839] ---[ end trace 314c8adc7c8ca584 ]---
[  367.790031] [drm:drm_sysfs_connector_add [drm]] adding "DP-4" to sysfs
[  367.790043] [drm:drm_sysfs_hotplug_event [drm]] generating hotplug event
[  367.790066] [drm:drm_dp_mst_connector_late_register [drm_kms_helper]] registering DPMST remote bus for card0-DP-4
[  367.790107] [drm:drm_dp_aux_register_devnode [drm_kms_helper]] drm_dp_aux_dev: aux [DPMST] registered as minor 4
[  367.790115] [drm:drm_dp_mst_topology_put_port [drm_kms_helper]] port 0000000024a17a86 (1)
[  367.790124] [drm:drm_dp_add_port [drm_kms_helper]] mstb 000000001b4abbfa (5)
[  367.790130] [drm:drm_dp_add_port [drm_kms_helper]] port 00000000b8b9df74 (2)
[  367.790137] [drm:drm_dp_mst_get_port_malloc [drm_kms_helper]] port 00000000b8b9df74 (2)
[  367.790169] WARNING: CPU: 2 PID: 442 at drivers/gpu/drm/drm_mode_object.c:45 __drm_mode_object_add+0xaa/0xb0 [drm]
[  367.791436] Modules linked in: vfat fat joydev rmi_smbus rmi_core i915 wmi_bmof iTCO_wdt intel_rapl x86_pkg_temp_thermal i2c_algo_bit coretemp drm_kms_helper crc32_pclmul syscopyarea sysfillrect sysimgblt psmouse fb_sys_fops i2c_i801 drm i2c_core lpc_ich mei_me mfd_core thinkpad_acpi mei ledtrig_audio wmi tpm_tis tpm_tis_core tpm video pcc_cpufreq crc32c_intel serio_raw ehci_pci ehci_hcd xhci_pci xhci_hcd
[  367.793914] CPU: 2 PID: 442 Comm: kworker/2:3 Kdump: loaded Tainted: G        W         5.2.0Lyude-Test+ #1
[  367.795127] Hardware name: LENOVO 20BWS1KY00/20BWS1KY00, BIOS JBET71WW (1.35 ) 09/14/2018
[  367.796330] Workqueue: events_long drm_dp_mst_link_probe_work [drm_kms_helper]
[  367.797554] RIP: 0010:__drm_mode_object_add+0xaa/0xb0 [drm]
[  367.798774] Code: 41 c7 44 24 10 01 00 00 00 4c 89 ff e8 4f 58 6b e1 85 db b8 00 00 00 00 0f 4e c3 48 83 c4 08 5b 41 5c 41 5d 41 5e 41 5f 5d c3 <0f> 0b eb 82 66 90 0f 1f 44 00 00 55 45 31 c0 b9 01 00 00 00 48 89
[  367.800047] RSP: 0018:ffffc90000447b68 EFLAGS: 00010246
[  367.801322] RAX: ffff8883193421a0 RBX: ffff88830a840000 RCX: 0000000000000001
[  367.802629] RDX: 00000000b0b0b0b0 RSI: ffff888313ad2890 RDI: ffff88830a840000
[  367.803915] RBP: ffffc90000447b98 R08: 0000000000000000 R09: ffff8883193421a0
[  367.805202] R10: 0000000000000000 R11: 0000000000000000 R12: ffff888313ad2890
[  367.806489] R13: 0000000000000001 R14: 0000000000000000 R15: ffffffffa01b0d7a
[  367.807793] FS:  0000000000000000(0000) GS:ffff88831da80000(0000) knlGS:0000000000000000
[  367.809080] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  367.810363] CR2: 00007f733dd19d88 CR3: 00000003190ba003 CR4: 00000000003606e0
[  367.811675] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  367.812962] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[  367.814248] Call Trace:
[  367.815554]  drm_mode_object_add+0x16/0x20 [drm]
[  367.816862]  drm_property_create+0xd4/0x1b0 [drm]
[  367.818167]  drm_property_create_range+0x1f/0x40 [drm]
[  367.819466]  drm_connector_attach_max_bpc_property+0x69/0x90 [drm]
[  367.820790]  intel_dp_add_mst_connector+0x130/0x140 [i915]
[  367.822190]  drm_dp_add_port+0x222/0x490 [drm_kms_helper]
[  367.823451]  ? memory_bm_create+0x1d3/0x4a0
[  367.824692]  ? drm_dp_dpcd_write+0x61/0xb0 [drm_kms_helper]
[  367.825877]  ? drm_dbg+0x87/0x90 [drm]
[  367.827026]  ? drm_dp_dpcd_write+0x61/0xb0 [drm_kms_helper]
[  367.828168]  drm_dp_send_link_address+0x162/0x1c0 [drm_kms_helper]
[  367.829305]  drm_dp_check_and_send_link_address+0xa2/0xb0 [drm_kms_helper]
[  367.830440]  drm_dp_mst_link_probe_work+0x69/0x90 [drm_kms_helper]
[  367.831552]  process_one_work+0x198/0x380
[  367.832620]  worker_thread+0x4d/0x3a0
[  367.833671]  kthread+0x106/0x140
[  367.834714]  ? process_one_work+0x380/0x380
[  367.835756]  ? kthread_park+0x90/0x90
[  367.836799]  ret_from_fork+0x35/0x40
[  367.837851] WARNING: CPU: 2 PID: 442 at drivers/gpu/drm/drm_mode_object.c:45 __drm_mode_object_add+0xaa/0xb0 [drm]
[  367.838919] ---[ end trace 314c8adc7c8ca585 ]---
[  367.840066] [drm:drm_sysfs_connector_add [drm]] adding "DP-5" to sysfs
[  367.840078] [drm:drm_sysfs_hotplug_event [drm]] generating hotplug event
[  367.840101] [drm:drm_dp_mst_connector_late_register [drm_kms_helper]] registering DPMST remote bus for card0-DP-5
[  367.840152] [drm:drm_dp_aux_register_devnode [drm_kms_helper]] drm_dp_aux_dev: aux [DPMST] registered as minor 5
[  367.840164] [drm:drm_dp_mst_topology_put_port [drm_kms_helper]] port 00000000b8b9df74 (1)
[  367.840176] [drm:drm_sysfs_hotplug_event [drm]] generating hotplug event
[  367.840189] [drm:drm_fb_helper_hotplug_event.part.0 [drm_kms_helper]] 
[  367.840202] [drm:drm_client_modeset_probe [drm]] 
[  367.840223] [drm:drm_mode_object_get [drm]] OBJ ID: 68 (4)
[  367.840237] [drm:drm_mode_object_get [drm]] OBJ ID: 74 (2)
[  367.840250] [drm:drm_mode_object_get [drm]] OBJ ID: 80 (2)
[  367.840266] [drm:drm_mode_object_get [drm]] OBJ ID: 86 (2)
[  367.840279] [drm:drm_mode_object_get [drm]] OBJ ID: 91 (2)
[  367.840291] [drm:drm_mode_object_get [drm]] OBJ ID: 94 (2)
[  367.840304] [drm:drm_mode_object_get [drm]] OBJ ID: 99 (2)
[  367.840329] [drm:drm_mode_object_get [drm]] OBJ ID: 102 (2)

On Thu, 2019-07-04 at 15:05 -0400, sunpeng.li@amd.com wrote:
> From: Leo Li <sunpeng.li@amd.com>
> 
> Implement late_register and early_unregister hooks for MST connectors.
> Call drm helpers for MST connector registration, which registers the
> AUX devices.
> 
> Signed-off-by: Leo Li <sunpeng.li@amd.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 29 +++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 60652ebbdf61..be309016f746 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -400,13 +400,38 @@ intel_dp_mst_detect(struct drm_connector *connector,
> bool force)
>  				      intel_connector->port);
>  }
>  
> +static int
> +intel_dp_mst_connector_late_register(struct drm_connector *connector)
> +{
> +	struct intel_connector *intel_connector =
> to_intel_connector(connector);
> +	struct drm_dp_mst_port *port = intel_connector->port;
> +
> +	int ret;
> +
> +	ret = intel_connector_register(connector);
> +	if (ret)
> +		return ret;
> +
> +	return drm_dp_mst_connector_late_register(connector, port);
> +}
> +
> +static void
> +intel_dp_mst_connector_early_unregister(struct drm_connector *connector)
> +{
> +	struct intel_connector *intel_connector =
> to_intel_connector(connector);
> +	struct drm_dp_mst_port *port = intel_connector->port;
> +
> +	drm_dp_mst_connector_early_unregister(connector, port);
> +	intel_connector_unregister(connector);
> +}
> +
>  static const struct drm_connector_funcs intel_dp_mst_connector_funcs = {
>  	.detect = intel_dp_mst_detect,
>  	.fill_modes = drm_helper_probe_single_connector_modes,
>  	.atomic_get_property = intel_digital_connector_atomic_get_property,
>  	.atomic_set_property = intel_digital_connector_atomic_set_property,
> -	.late_register = intel_connector_register,
> -	.early_unregister = intel_connector_unregister,
> +	.late_register = intel_dp_mst_connector_late_register,
> +	.early_unregister = intel_dp_mst_connector_early_unregister,
>  	.destroy = intel_connector_destroy,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  	.atomic_duplicate_state = intel_digital_connector_duplicate_state,
-- 
Cheers,
	Lyude Paul

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

Re: [PATCH 08/10] drm/nouveau/kms/nv50: Implement MST Aux device registration

[Lyude Paul]

Patch looks fine to me, but I'm seeing the same warnings that I mentioned on
patch 7 with this as well

On Thu, 2019-07-04 at 15:05 -0400, sunpeng.li@amd.com wrote:
> From: Leo Li <sunpeng.li@amd.com>
> 
> Implement late_register and early_unregister hooks for MST connectors.
> Call drm helpers for MST connector registration, which registers the
> AUX devices.
> 
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Signed-off-by: Leo Li <sunpeng.li@amd.com>
> ---
>  drivers/gpu/drm/nouveau/dispnv50/disp.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 7ba373f493b2..6d0fbb6036cf 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -1019,6 +1019,24 @@ nv50_mstc_destroy(struct drm_connector *connector)
>  	kfree(mstc);
>  }
>  
> +static int
> +nv50_mstc_late_register(struct drm_connector *connector)
> +{
> +	struct nv50_mstc *mstc = nv50_mstc(connector);
> +	struct drm_dp_mst_port *port = mstc->port;
> +
> +	return drm_dp_mst_connector_late_register(connector, port);
> +}
> +
> +static void
> +nv50_mstc_early_unregister(struct drm_connector *connector)
> +{
> +	struct nv50_mstc *mstc = nv50_mstc(connector);
> +	struct drm_dp_mst_port *port = mstc->port;
> +
> +	drm_dp_mst_connector_early_unregister(connector, port);
> +}
> +
>  static const struct drm_connector_funcs
>  nv50_mstc = {
>  	.reset = nouveau_conn_reset,
> @@ -1029,6 +1047,8 @@ nv50_mstc = {
>  	.atomic_destroy_state = nouveau_conn_atomic_destroy_state,
>  	.atomic_set_property = nouveau_conn_atomic_set_property,
>  	.atomic_get_property = nouveau_conn_atomic_get_property,
> +	.late_register = nv50_mstc_late_register,
> +	.early_unregister = nv50_mstc_early_unregister,
>  };
>  
>  static int
-- 
Cheers,
	Lyude Paul

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

Re: [PATCH 04/10] drm/nouveau: Use connector kdev as aux device parent

[Lyude Paul]

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Thu, 2019-07-04 at 15:05 -0400, sunpeng.li@amd.com wrote:
> From: Leo Li <sunpeng.li@amd.com>
> 
> Set the connector's kernel device as the parent for the aux kernel
> device. This allows udev rules to access connector attributes when
> creating symlinks to aux devices.
> 
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Signed-off-by: Leo Li <sunpeng.li@amd.com>
> ---
>  drivers/gpu/drm/nouveau/nouveau_connector.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c
> b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 4116ee62adaf..e32def09ef89 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -1346,7 +1346,7 @@ nouveau_connector_create(struct drm_device *dev,
>  		break;
>  	case DRM_MODE_CONNECTOR_DisplayPort:
>  	case DRM_MODE_CONNECTOR_eDP:
> -		nv_connector->aux.dev = dev->dev;
> +		nv_connector->aux.dev = connector->kdev;
>  		nv_connector->aux.transfer = nouveau_connector_aux_xfer;
>  		snprintf(aux_name, sizeof(aux_name), "sor-%04x-%04x",
>  			 dcbe->hasht, dcbe->hashm);
-- 
Cheers,
	Lyude Paul

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

Re: [PATCH 07/10] drm/i915: Implement MST Aux device registration

[Ville Syrjälä]

On Fri, Jul 12, 2019 at 03:48:53PM -0400, Lyude Paul wrote:
> BTW, I just tried these patches on my T450s (using i915) and I'm seeing some
> kernel warnings with them when adding DP aux devices after connecting a new
> MST topology to the system: 
> 
> [  367.742571] WARNING: CPU: 2 PID: 442 at drivers/gpu/drm/drm_mode_object.c:45 __drm_mode_object_add+0xaa/0xb0 [drm]

Looks like Daniel added that particular WARN in
commit 4f5368b5541a  ("drm/kms: Catch mode_object lifetime errors").

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 07/10] drm/i915: Implement MST Aux device registration

[Ville Syrjälä]

On Fri, Jul 12, 2019 at 11:05:59PM +0300, Ville Syrjälä wrote:
> On Fri, Jul 12, 2019 at 03:48:53PM -0400, Lyude Paul wrote:
> > BTW, I just tried these patches on my T450s (using i915) and I'm seeing some
> > kernel warnings with them when adding DP aux devices after connecting a new
> > MST topology to the system: 
> > 
> > [  367.742571] WARNING: CPU: 2 PID: 442 at drivers/gpu/drm/drm_mode_object.c:45 __drm_mode_object_add+0xaa/0xb0 [drm]
> 
> Looks like Daniel added that particular WARN in
> commit 4f5368b5541a  ("drm/kms: Catch mode_object lifetime errors").

And I'm the one who added the max_bpc prop to the mst connectors, which
is a per-connector property (ie. a new one gets created for every
connecotor). So that could be a problem I suppose. I guess we may need
to create just one of these for MST and reuse it for every connector.
Could just point at the prop of the corresponding SST connector I
suppose...

+       /* Reuse the prop because blah */
+       connector->max_bpc_property =
+               intel_dp->attached_connector->base.max_bpc_property;
        drm_connector_attach_max_bpc_property(connector, 6, 12);


-- 
Ville Syrjälä
Intel
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2] drm/sysfs: Add mstpath attribute to connector devices

[Li, Sun peng (Leo)]

On 2019-07-10 6:50 p.m., Lyude Paul wrote:
> gah. So, I was originally going to ask "why didn't we add the connector name
> into this?", but then I realized we're doing the right thing already and just
> doing card%d-%s % (card_number, path_prop). Which means we probably really don't
> want to add a property called mstpath, since it's hardly different from path
> (whoops!).
> 
> Additionally, after some thinking I realized I may have made a mistake as I'm
> not entirely sure if we would need to specify the DRM card in the path prop for
> udev, considering that's specified in the sysfs path all ready. Even if I'm
> wrong on that though, I think it might be better not to add an mstpath property
> and just go the route of just adding a new path_v2 property that we could use
> for both MST and non-MST connector paths. (I cc'd you on the email thread about
> this, so you can read more about this there.

Funny enough, I was originally trying to make this work for SST devices.
It didn't make sense to have by-name and by-path, but only have SST
exist in the by-name symlinks. The question there was "what to use for
sst paths?" Eventually I settled with keeping this purely for user
friendliness. But since discussion is already underway for a better
'path', it makes sense to delay this.

> 
> So, I would actually suggest we just drop this patch entirely for now. We should
> be fine without it, even though the dp_aux_dev paths will be kind of ugly for a
> little while. I'd rather the rest of this series get upstream first, and try to
> do the path prop stuff separately.>

Sounds fair, going to spin up v3.

Thanks!
Leo

> 
> On Fri, 2019-07-05 at 10:32 -0400, sunpeng.li@amd.com wrote:
>> From: Leo Li <sunpeng.li@amd.com>
>>
>> This can be used to create more descriptive symlinks for MST aux
>> devices. Consider the following udev rule:
>>
>> SUBSYSTEM=="drm_dp_aux_dev", SUBSYSTEMS=="drm", ATTRS{mstpath}=="?*",
>> 	SYMLINK+="drm_dp_aux/by-path/$attr{mstpath}"
>>
>> The following symlinks will be created (depending on your MST topology):
>>
>> $ ls /dev/drm_dp_aux/by-path/
>> card0-mst:0-1  card0-mst:0-1-1  card0-mst:0-1-8  card0-mst:0-8
>>
>> v2: remove unnecessary locking of mode_config.mutex
>>
>> Signed-off-by: Leo Li <sunpeng.li@amd.com>
>> ---
>>  drivers/gpu/drm/drm_sysfs.c | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
>> index ad10810bc972..7d483ab684a0 100644
>> --- a/drivers/gpu/drm/drm_sysfs.c
>> +++ b/drivers/gpu/drm/drm_sysfs.c
>> @@ -236,16 +236,36 @@ static ssize_t modes_show(struct device *device,
>>  	return written;
>>  }
>>  
>> +static ssize_t mstpath_show(struct device *device,
>> +			    struct device_attribute *attr,
>> +			    char *buf)
>> +{
>> +	struct drm_connector *connector = to_drm_connector(device);
>> +	ssize_t ret = 0;
>> +	char *path;
>> +
>> +	if (!connector->path_blob_ptr)
>> +		return ret;
>> +
>> +	path = connector->path_blob_ptr->data;
>> +	ret = snprintf(buf, PAGE_SIZE, "card%d-%s\n",
>> +		       connector->dev->primary->index, path);
>> +
>> +	return ret;
>> +}
>> +
>>  static DEVICE_ATTR_RW(status);
>>  static DEVICE_ATTR_RO(enabled);
>>  static DEVICE_ATTR_RO(dpms);
>>  static DEVICE_ATTR_RO(modes);
>> +static DEVICE_ATTR_RO(mstpath);
>>  
>>  static struct attribute *connector_dev_attrs[] = {
>>  	&dev_attr_status.attr,
>>  	&dev_attr_enabled.attr,
>>  	&dev_attr_dpms.attr,
>>  	&dev_attr_modes.attr,
>> +	&dev_attr_mstpath.attr,
>>  	NULL
>>  };
>>  
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2] drm/sysfs: Add mstpath attribute to connector devices

[Lyude Paul]

On Tue, 2019-07-16 at 18:28 +0000, Li, Sun peng (Leo) wrote:
> 
> 
> On 2019-07-10 6:50 p.m., Lyude Paul wrote:
> > gah. So, I was originally going to ask "why didn't we add the connector
> > name
> > into this?", but then I realized we're doing the right thing already and
> > just
> > doing card%d-%s % (card_number, path_prop). Which means we probably really
> > don't
> > want to add a property called mstpath, since it's hardly different from
> > path
> > (whoops!).
> > 
> > Additionally, after some thinking I realized I may have made a mistake as
> > I'm
> > not entirely sure if we would need to specify the DRM card in the path
> > prop for
> > udev, considering that's specified in the sysfs path all ready. Even if
> > I'm
> > wrong on that though, I think it might be better not to add an mstpath
> > property
> > and just go the route of just adding a new path_v2 property that we could
> > use
> > for both MST and non-MST connector paths. (I cc'd you on the email thread
> > about
> > this, so you can read more about this there.
> 
> Funny enough, I was originally trying to make this work for SST devices.
> It didn't make sense to have by-name and by-path, but only have SST
> exist in the by-name symlinks. The question there was "what to use for
> sst paths?" Eventually I settled with keeping this purely for user
> friendliness. But since discussion is already underway for a better
> 'path', it makes sense to delay this.

Yeah-I think Ville's suggestion of making object IDs global instead of per-
device might be what we want to  go with, but getting the aux dev stuff in the
kernel first is priority imo

> 
> > So, I would actually suggest we just drop this patch entirely for now. We
> > should
> > be fine without it, even though the dp_aux_dev paths will be kind of ugly
> > for a
> > little while. I'd rather the rest of this series get upstream first, and
> > try to
> > do the path prop stuff separately.>
> 
> Sounds fair, going to spin up v3.
> 
> Thanks!
> Leo
> 
> > On Fri, 2019-07-05 at 10:32 -0400, sunpeng.li@amd.com wrote:
> > > From: Leo Li <sunpeng.li@amd.com>
> > > 
> > > This can be used to create more descriptive symlinks for MST aux
> > > devices. Consider the following udev rule:
> > > 
> > > SUBSYSTEM=="drm_dp_aux_dev", SUBSYSTEMS=="drm", ATTRS{mstpath}=="?*",
> > > 	SYMLINK+="drm_dp_aux/by-path/$attr{mstpath}"
> > > 
> > > The following symlinks will be created (depending on your MST topology):
> > > 
> > > $ ls /dev/drm_dp_aux/by-path/
> > > card0-mst:0-1  card0-mst:0-1-1  card0-mst:0-1-8  card0-mst:0-8
> > > 
> > > v2: remove unnecessary locking of mode_config.mutex
> > > 
> > > Signed-off-by: Leo Li <sunpeng.li@amd.com>
> > > ---
> > >  drivers/gpu/drm/drm_sysfs.c | 20 ++++++++++++++++++++
> > >  1 file changed, 20 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> > > index ad10810bc972..7d483ab684a0 100644
> > > --- a/drivers/gpu/drm/drm_sysfs.c
> > > +++ b/drivers/gpu/drm/drm_sysfs.c
> > > @@ -236,16 +236,36 @@ static ssize_t modes_show(struct device *device,
> > >  	return written;
> > >  }
> > >  
> > > +static ssize_t mstpath_show(struct device *device,
> > > +			    struct device_attribute *attr,
> > > +			    char *buf)
> > > +{
> > > +	struct drm_connector *connector = to_drm_connector(device);
> > > +	ssize_t ret = 0;
> > > +	char *path;
> > > +
> > > +	if (!connector->path_blob_ptr)
> > > +		return ret;
> > > +
> > > +	path = connector->path_blob_ptr->data;
> > > +	ret = snprintf(buf, PAGE_SIZE, "card%d-%s\n",
> > > +		       connector->dev->primary->index, path);
> > > +
> > > +	return ret;
> > > +}
> > > +
> > >  static DEVICE_ATTR_RW(status);
> > >  static DEVICE_ATTR_RO(enabled);
> > >  static DEVICE_ATTR_RO(dpms);
> > >  static DEVICE_ATTR_RO(modes);
> > > +static DEVICE_ATTR_RO(mstpath);
> > >  
> > >  static struct attribute *connector_dev_attrs[] = {
> > >  	&dev_attr_status.attr,
> > >  	&dev_attr_enabled.attr,
> > >  	&dev_attr_dpms.attr,
> > >  	&dev_attr_modes.attr,
> > > +	&dev_attr_mstpath.attr,
> > >  	NULL
> > >  };
> > >  
-- 
Cheers,
	Lyude Paul

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

Re: [PATCH 07/10] drm/i915: Implement MST Aux device registration

[Li, Sun peng (Leo)]

On 2019-07-12 4:15 p.m., Ville Syrjälä wrote:
> On Fri, Jul 12, 2019 at 11:05:59PM +0300, Ville Syrjälä wrote:
>> On Fri, Jul 12, 2019 at 03:48:53PM -0400, Lyude Paul wrote:
>>> BTW, I just tried these patches on my T450s (using i915) and I'm seeing some
>>> kernel warnings with them when adding DP aux devices after connecting a new
>>> MST topology to the system: 
>>>
>>> [  367.742571] WARNING: CPU: 2 PID: 442 at drivers/gpu/drm/drm_mode_object.c:45 __drm_mode_object_add+0xaa/0xb0 [drm]
>>
>> Looks like Daniel added that particular WARN in
>> commit 4f5368b5541a  ("drm/kms: Catch mode_object lifetime errors").
> 
> And I'm the one who added the max_bpc prop to the mst connectors, which
> is a per-connector property (ie. a new one gets created for every
> connecotor). So that could be a problem I suppose. I guess we may need
> to create just one of these for MST and reuse it for every connector.
> Could just point at the prop of the corresponding SST connector I
> suppose...
> 
> +       /* Reuse the prop because blah */
> +       connector->max_bpc_property =
> +               intel_dp->attached_connector->base.max_bpc_property;
>         drm_connector_attach_max_bpc_property(connector, 6, 12);

I'd prefer to address this separately, if that's alright.
It isn't related to this series, and no new warnings are introduced by
this either.

Leo

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