[PATCH 0/9] MST AUX Devices (v3)

[PATCH 0/9] MST AUX Devices (v3)

[sunpeng.li]

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

Here's what changed in v3:

* Dropped patch to expose the mst-path device attribute, in lieu of
  ongoing discussion for defining a better connector path property
* Change WARN_ON_ONCE -> DRM_ERROR on MST dpcd read errors to avoid
  tainting the kernel
* Unwind intel_connector_register() on
  drm_dp_mst_connector_late_register() failure.
* Docstring and cosmetic fixes

Leo Li (8):
  drm/dp: Use non-cyclic idr
  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 (v2)
  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              |  18 ++-
 drivers/gpu/drm/drm_dp_mst_topology.c         | 144 ++++++++++++++++--
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |  33 +++-
 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 ++
 10 files changed, 272 insertions(+), 30 deletions(-)

-- 
2.22.0

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

[PATCH 1/9] 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 2/9 v3] drm/dp_mst: Enable registration of AUX devices for MST ports

[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.

v3 changes:
* Change WARN_ON_ONCE -> DRM_ERROR on dpcd read errors
* Docstring and cosmetic fixes

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>
Reviewed-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/drm_dp_aux_dev.c      |  15 ++-
 drivers/gpu/drm/drm_dp_mst_topology.c | 144 ++++++++++++++++++++++++--
 include/drm/drm_dp_helper.h           |   4 +
 include/drm/drm_dp_mst_helper.h       |  11 ++
 4 files changed, 162 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..0cfb386754c3 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"
@@ -162,7 +163,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 +215,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..4733d3350ede 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,8 @@ 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 +1490,52 @@ 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.
+ *
+ * Return: Number of bytes read, or negative error code on failure.
+ */
+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.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+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 +1579,46 @@ 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.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+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 +1641,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 +1910,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 +1922,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 +2533,58 @@ 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 (txmsg->reply.reply_type == 1) {
+		DRM_ERROR("mstb %p port %d: DPCD read on addr 0x%x for %d bytes NAKed\n",
+			  mstb, port->port_num, offset, size);
+		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 +2613,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..8364502f92cf 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 3/9] 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>
Reviewed-by: Lyude Paul <lyude@redhat.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 8f15281faa79..330d7d29a6e3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1349,7 +1349,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 4/9] 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 5/9] 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>
Cc: Harry Wentland <harry.wentland@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 6/9 v2] drm/i915: Implement MST Aux device registration

[sunpeng.li]

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>

v2 changes:
Unwind intel_connector_register on mst late register failure.
---
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 33 +++++++++++++++++++--
 1 file changed, 31 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..0633ebf3f1bf 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -400,13 +400,42 @@ 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;
+
+	ret = drm_dp_mst_connector_late_register(connector, port);
+	if (ret)
+		intel_connector_unregister(connector);
+
+	return ret;
+}
+
+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

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

[PATCH 7/9] 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 8497768f1b41..0d6e9350ba44 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1024,6 +1024,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,
@@ -1034,6 +1052,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 8/9] drm/radeon: Implement MST Aux device registration

[sunpeng.li]

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>
Cc: Harry Wentland <harry.wentland@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

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

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

[sunpeng.li]

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>
Cc: Harry Wentland <harry.wentland@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

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

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

[Kazlauskas, Nicholas]

On 7/23/19 7:28 PM, 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: Jerry Zuo <Jerry.Zuo@amd.com>
> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Signed-off-by: Leo Li <sunpeng.li@amd.com>

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

BTW: I already reviewed patch 5, feel free to add my R-B.

Nicholas Kazlauskas

> ---
>   .../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)
> 

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

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

[Li, Sun peng (Leo)]

On 2019-07-24 4:05 p.m., Kazlauskas, Nicholas wrote:
> On 7/23/19 7:28 PM, 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: Jerry Zuo <Jerry.Zuo@amd.com>
>> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>> Signed-off-by: Leo Li <sunpeng.li@amd.com>
> 
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> 
> BTW: I already reviewed patch 5, feel free to add my R-B.
> 
> Nicholas Kazlauskas

Sorry, will do.

Thanks!
Leo

> 
>> ---
>>   .../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)
>>
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/9 v3] drm/dp_mst: Enable registration of AUX devices for MST ports

[Lyude Paul]

Noticed something! important note below

On Tue, 2019-07-23 at 19:28 -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.
> 
> v3 changes:
> * Change WARN_ON_ONCE -> DRM_ERROR on dpcd read errors
> * Docstring and cosmetic fixes
> 
> 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>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> ---
>  drivers/gpu/drm/drm_dp_aux_dev.c      |  15 ++-
>  drivers/gpu/drm/drm_dp_mst_topology.c | 144 ++++++++++++++++++++++++--
>  include/drm/drm_dp_helper.h           |   4 +
>  include/drm/drm_dp_mst_helper.h       |  11 ++
>  4 files changed, 162 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..0cfb386754c3 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"
> @@ -162,7 +163,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 +215,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..4733d3350ede 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,8 @@ 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;
> +

This looks like some rebase ditritus you probably don't want in the final
patch ;)

Other then that, everything still looks good to me

>  		kfree(port->cached_edid);
>  
>  		/*
> @@ -1483,6 +1490,52 @@ 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.
> + *
> + * Return: Number of bytes read, or negative error code on failure.
> + */
> +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.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +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 +1579,46 @@ 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.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +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 +1641,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 +1910,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 +1922,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 +2533,58 @@ 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 (txmsg->reply.reply_type == 1) {
> +		DRM_ERROR("mstb %p port %d: DPCD read on addr 0x%x for %d
> bytes NAKed\n",
> +			  mstb, port->port_num, offset, size);
> +		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 +2613,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..8364502f92cf 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
-- 
Cheers,
	Lyude Paul

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

Re: [PATCH 7/9] drm/nouveau/kms/nv50: Implement MST Aux device registration

[Lyude Paul]

Works perfectly here, and no warnings (at least not any that are relevant to
this patch series :) when reloading nouveau.

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

On Tue, 2019-07-23 at 19:28 -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 8497768f1b41..0d6e9350ba44 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -1024,6 +1024,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,
> @@ -1034,6 +1052,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

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