[PATCH 0/6] Remove all bad dp_mst_port uses and hide struct def

[PATCH 0/6] Remove all bad dp_mst_port uses and hide struct def

[Lyude Paul]

So we don't ever have to worry about drivers touching drm_dp_mst_port
structs without verifying them and crashing again.

Lyude Paul (6):
  drm/dp_mst: Add drm_dp_get_payload_info()
  drm/nouveau: Use drm_dp_get_payload_info() for getting payload/vcpi
  drm/nouveau: Stop reading port->mgr in nv50_mstc_get_modes()
  drm/nouveau: Stop reading port->mgr in nv50_mstc_detect()
  drm/dp_mst: Hide drm_dp_mst_port contents from drivers
  drm/i915: Start using struct drm_dp_mst_port again

 drivers/gpu/drm/drm_dp_mst_topology.c   | 115 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp_mst.c     |   2 +-
 drivers/gpu/drm/i915/intel_drv.h        |   2 +-
 drivers/gpu/drm/nouveau/dispnv50/disp.c |  60 +++++--------
 include/drm/drm_dp_mst_helper.h         |  65 ++------------
 5 files changed, 146 insertions(+), 98 deletions(-)

-- 
2.19.1

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

[PATCH 1/6] drm/dp_mst: Add drm_dp_get_payload_info()

[Lyude Paul]

Some hardware (nvidia hardware in particular) needs to be notified of
the exact VCPI and payload settings that the topology manager decided on
for each mstb port. Since there isn't currently any way to get this
information without going through port (which drivers are very much not
supposed to do by themselves, ever), let's add one.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 56 +++++++++++++++++++++++++++
 include/drm/drm_dp_mst_helper.h       |  5 ++-
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 529414556962..4336d17ce904 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1982,6 +1982,62 @@ int drm_dp_update_payload_part2(struct drm_dp_mst_topology_mgr *mgr)
 }
 EXPORT_SYMBOL(drm_dp_update_payload_part2);
 
+/**
+ * drm_dp_get_payload_info() - Retrieve payload/vcpi information for the given
+ *                             @port
+ * @mgr: manager to use
+ * @port: the port to get the relevant payload information for
+ * @vcpi_out: where to copy the port's VCPI information to
+ * @payload_out: where to copy the port's payload information to
+ *
+ * Searches the current payloads for @mgr and finds the relevant payload and
+ * VCPI information that was programmed by the topology mgr, then copies it
+ * into @vcpi_out and @payload_out. Drivers which need to know this
+ * information must use this helper as opposed to checking @port themselves,
+ * as this helper will ensure the port reference is still valid and grab the
+ * appropriate locks in @mgr.
+ *
+ * Returns:
+ * 0 on success, negative error code if the port is no longer valid or a
+ * programmed payload could not be found for @port.
+ */
+int drm_dp_get_payload_info(struct drm_dp_mst_topology_mgr *mgr,
+			    struct drm_dp_mst_port *port,
+			    struct drm_dp_vcpi *vcpi_out,
+			    struct drm_dp_payload *payload_out)
+{
+	struct drm_dp_payload *payload = NULL;
+	int i;
+	int ret = 0;
+
+	port = drm_dp_get_validated_port_ref(mgr, port);
+	if (!port)
+		return -EINVAL;
+
+	mutex_lock(&mgr->payload_lock);
+	/* Figure out which of the payloads belongs to this port */
+	for (i = 0; i < mgr->max_payloads; i++) {
+		if (mgr->payloads[i].vcpi == port->vcpi.vcpi) {
+			payload = &mgr->payloads[i];
+			break;
+		}
+	}
+
+	if (!payload) {
+		DRM_DEBUG_KMS("Failed to find payload for port %p\n", port);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	*payload_out = *payload;
+	*vcpi_out = port->vcpi;
+out:
+	mutex_unlock(&mgr->payload_lock);
+	drm_dp_put_port(port);
+	return ret;
+}
+EXPORT_SYMBOL(drm_dp_get_payload_info);
+
 #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,
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 59f005b419cf..9cc93ea60e7e 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -592,7 +592,10 @@ bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
 			      struct drm_dp_mst_port *port, int pbn, int slots);
 
 int drm_dp_mst_get_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
-
+int drm_dp_get_payload_info(struct drm_dp_mst_topology_mgr *mgr,
+			    struct drm_dp_mst_port *port,
+			    struct drm_dp_vcpi *vcpi_out,
+			    struct drm_dp_payload *payload_out);
 
 void drm_dp_mst_reset_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
 
-- 
2.19.1

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

[PATCH 2/6] drm/nouveau: Use drm_dp_get_payload_info() for getting payload/vcpi

[Lyude Paul]

Currently, nouveau tries to go through the drm_dp_mst_port structures
itself in order to retrieve the relevant payload and VCPI information
that it needs to report to the GPU. This is wrong: mstc->port could be
destroyed at any point, and additionally the payload could be changed at
any point because it doesn't bother trying to grab the payload lock. So;
remove nv50_msto_payload entirely and use the new
drm_dp_get_payload_info() helper.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 55 ++++++++++---------------
 1 file changed, 21 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 6cbbae3f438b..e6f72ca0b1fa 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -665,41 +665,24 @@ struct nv50_msto {
 	bool disabled;
 };
 
-static struct drm_dp_payload *
-nv50_msto_payload(struct nv50_msto *msto)
-{
-	struct nouveau_drm *drm = nouveau_drm(msto->encoder.dev);
-	struct nv50_mstc *mstc = msto->mstc;
-	struct nv50_mstm *mstm = mstc->mstm;
-	int vcpi = mstc->port->vcpi.vcpi, i;
-
-	NV_ATOMIC(drm, "%s: vcpi %d\n", msto->encoder.name, vcpi);
-	for (i = 0; i < mstm->mgr.max_payloads; i++) {
-		struct drm_dp_payload *payload = &mstm->mgr.payloads[i];
-		NV_ATOMIC(drm, "%s: %d: vcpi %d start 0x%02x slots 0x%02x\n",
-			  mstm->outp->base.base.name, i, payload->vcpi,
-			  payload->start_slot, payload->num_slots);
-	}
-
-	for (i = 0; i < mstm->mgr.max_payloads; i++) {
-		struct drm_dp_payload *payload = &mstm->mgr.payloads[i];
-		if (payload->vcpi == vcpi)
-			return payload;
-	}
-
-	return NULL;
-}
-
 static void
 nv50_msto_cleanup(struct nv50_msto *msto)
 {
 	struct nouveau_drm *drm = nouveau_drm(msto->encoder.dev);
 	struct nv50_mstc *mstc = msto->mstc;
 	struct nv50_mstm *mstm = mstc->mstm;
+	struct drm_dp_payload payload;
+	struct drm_dp_vcpi vcpi;
+	int ret;
 
 	NV_ATOMIC(drm, "%s: msto cleanup\n", msto->encoder.name);
-	if (mstc->port && mstc->port->vcpi.vcpi > 0 && !nv50_msto_payload(msto))
-		drm_dp_mst_deallocate_vcpi(&mstm->mgr, mstc->port);
+	if (mstc->port) {
+		ret = drm_dp_get_payload_info(&mstm->mgr, mstc->port,
+					      &vcpi, &payload);
+		if (!ret)
+			drm_dp_mst_deallocate_vcpi(&mstm->mgr, mstc->port);
+	}
+
 	if (msto->disabled) {
 		msto->mstc = NULL;
 		msto->head = NULL;
@@ -713,6 +696,9 @@ nv50_msto_prepare(struct nv50_msto *msto)
 	struct nouveau_drm *drm = nouveau_drm(msto->encoder.dev);
 	struct nv50_mstc *mstc = msto->mstc;
 	struct nv50_mstm *mstm = mstc->mstm;
+	struct drm_dp_payload payload;
+	struct drm_dp_vcpi vcpi;
+	int ret;
 	struct {
 		struct nv50_disp_mthd_v1 base;
 		struct nv50_disp_sor_dp_mst_vcpi_v0 vcpi;
@@ -725,13 +711,14 @@ nv50_msto_prepare(struct nv50_msto *msto)
 	};
 
 	NV_ATOMIC(drm, "%s: msto prepare\n", msto->encoder.name);
-	if (mstc->port && mstc->port->vcpi.vcpi > 0) {
-		struct drm_dp_payload *payload = nv50_msto_payload(msto);
-		if (payload) {
-			args.vcpi.start_slot = payload->start_slot;
-			args.vcpi.num_slots = payload->num_slots;
-			args.vcpi.pbn = mstc->port->vcpi.pbn;
-			args.vcpi.aligned_pbn = mstc->port->vcpi.aligned_pbn;
+	if (mstc->port) {
+		ret = drm_dp_get_payload_info(&mstm->mgr, mstc->port,
+					      &vcpi, &payload);
+		if (!ret) {
+			args.vcpi.start_slot = payload.start_slot;
+			args.vcpi.num_slots = payload.num_slots;
+			args.vcpi.pbn = vcpi.pbn;
+			args.vcpi.aligned_pbn = vcpi.aligned_pbn;
 		}
 	}
 
-- 
2.19.1

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH 3/6] drm/nouveau: Stop reading port->mgr in nv50_mstc_get_modes()

[Lyude Paul]

mstc->port isn't validated here so it could be null or worse when we
access it. And drivers aren't ever supposed to be looking at it's
contents anyway. Plus, we can already get the MST manager from
&mstc->mstm->mgr.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index e6f72ca0b1fa..66c40b56a0cb 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -893,7 +893,8 @@ nv50_mstc_get_modes(struct drm_connector *connector)
 	struct nv50_mstc *mstc = nv50_mstc(connector);
 	int ret = 0;
 
-	mstc->edid = drm_dp_mst_get_edid(&mstc->connector, mstc->port->mgr, mstc->port);
+	mstc->edid = drm_dp_mst_get_edid(&mstc->connector,
+					 &mstc->mstm->mgr, mstc->port);
 	drm_connector_update_edid_property(&mstc->connector, mstc->edid);
 	if (mstc->edid)
 		ret = drm_add_edid_modes(&mstc->connector, mstc->edid);
-- 
2.19.1

[PATCH 4/6] drm/nouveau: Stop reading port->mgr in nv50_mstc_detect()

[Lyude Paul]

Same as the previous commit, but for nv50_mstc_detect() this time.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 66c40b56a0cb..a08dd827e892 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -930,7 +930,7 @@ nv50_mstc_detect(struct drm_connector *connector, bool force)
 	if (ret < 0 && ret != -EACCES)
 		return connector_status_disconnected;
 
-	conn_status = drm_dp_mst_detect_port(connector, mstc->port->mgr,
+	conn_status = drm_dp_mst_detect_port(connector, &mstc->mstm->mgr,
 					     mstc->port);
 
 	pm_runtime_mark_last_busy(connector->dev->dev);
-- 
2.19.1

[PATCH 5/6] drm/dp_mst: Hide drm_dp_mst_port contents from drivers

[Lyude Paul]

It hasn't been OK to access any of the contents of struct
drm_dp_mst_port without validating the port first for quite a long time
now, since a drm_dp_mst_port structure can be freed at any given moment
in time outside of the driver's contorl. Any kind of information a
driver needs from drm_dp_mst_port should be exposed through a helper
function instead that handles validating the port pointer, along with
anything else that's needed.

Since we've removed the last dangerous remanents of ->port accesses in
the DRM tree, let's finish this off and move the struct drm_dp_mst_port
definition out of drm_dp_mst_helper.h, into drm_dp_mst_topology.c, and
then replace it's header definition with an incomplete struct type. This
way drivers can still use the struct type, and no one else will make the
mistake of trying to access the contents of port.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 59 ++++++++++++++++++++++++++
 include/drm/drm_dp_mst_helper.h       | 60 +--------------------------
 2 files changed, 60 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 4336d17ce904..5fa898a8a64d 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -41,6 +41,65 @@
  * protocol. The helpers contain a topology manager and bandwidth manager.
  * The helpers encapsulate the sending and received of sideband msgs.
  */
+
+/**
+ * struct drm_dp_mst_port - MST port
+ * @kref: reference count for this port.
+ * @port_num: port number
+ * @input: if this port is an input port.
+ * @mcs: message capability status - DP 1.2 spec.
+ * @ddps: DisplayPort Device Plug Status - DP 1.2
+ * @pdt: Peer Device Type
+ * @ldps: Legacy Device Plug Status
+ * @dpcd_rev: DPCD revision of device on this port
+ * @num_sdp_streams: Number of simultaneous streams
+ * @num_sdp_stream_sinks: Number of stream sinks
+ * @available_pbn: Available bandwidth for this port.
+ * @next: link to next port on this branch device
+ * @mstb: branch device attach below this port
+ * @aux: i2c aux transport to talk to device connected to this port.
+ * @parent: branch device parent of this port
+ * @vcpi: Virtual Channel Payload info for this port.
+ * @connector: DRM connector this port is connected to.
+ * @mgr: topology manager this port lives under.
+ *
+ * This structure represents an MST port endpoint on a device somewhere
+ * in the MST topology.
+ */
+struct drm_dp_mst_port {
+	struct kref kref;
+
+	u8 port_num;
+	bool input;
+	bool mcs;
+	bool ddps;
+	u8 pdt;
+	bool ldps;
+	u8 dpcd_rev;
+	u8 num_sdp_streams;
+	u8 num_sdp_stream_sinks;
+	uint16_t available_pbn;
+	struct list_head next;
+	struct drm_dp_mst_branch *mstb; /* pointer to an mstb if this port has one */
+	struct drm_dp_aux aux; /* i2c bus for this port? */
+	struct drm_dp_mst_branch *parent;
+
+	struct drm_dp_vcpi vcpi;
+	struct drm_connector *connector;
+	struct drm_dp_mst_topology_mgr *mgr;
+
+	/**
+	 * @cached_edid: for DP logical ports - make tiling work by ensuring
+	 * that the EDID for all connectors is read immediately.
+	 */
+	struct edid *cached_edid;
+	/**
+	 * @has_audio: Tracks whether the sink connector to this port is
+	 * audio-capable.
+	 */
+	bool has_audio;
+};
+
 static bool dump_dp_payload_table(struct drm_dp_mst_topology_mgr *mgr,
 				  char *buf);
 static int test_calc_pbn_mode(void);
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 9cc93ea60e7e..3076a45aef4d 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -26,7 +26,7 @@
 #include <drm/drm_dp_helper.h>
 #include <drm/drm_atomic.h>
 
-struct drm_dp_mst_branch;
+struct drm_dp_mst_port;
 
 /**
  * struct drm_dp_vcpi - Virtual Channel Payload Identifier
@@ -42,64 +42,6 @@ struct drm_dp_vcpi {
 	int num_slots;
 };
 
-/**
- * struct drm_dp_mst_port - MST port
- * @kref: reference count for this port.
- * @port_num: port number
- * @input: if this port is an input port.
- * @mcs: message capability status - DP 1.2 spec.
- * @ddps: DisplayPort Device Plug Status - DP 1.2
- * @pdt: Peer Device Type
- * @ldps: Legacy Device Plug Status
- * @dpcd_rev: DPCD revision of device on this port
- * @num_sdp_streams: Number of simultaneous streams
- * @num_sdp_stream_sinks: Number of stream sinks
- * @available_pbn: Available bandwidth for this port.
- * @next: link to next port on this branch device
- * @mstb: branch device attach below this port
- * @aux: i2c aux transport to talk to device connected to this port.
- * @parent: branch device parent of this port
- * @vcpi: Virtual Channel Payload info for this port.
- * @connector: DRM connector this port is connected to.
- * @mgr: topology manager this port lives under.
- *
- * This structure represents an MST port endpoint on a device somewhere
- * in the MST topology.
- */
-struct drm_dp_mst_port {
-	struct kref kref;
-
-	u8 port_num;
-	bool input;
-	bool mcs;
-	bool ddps;
-	u8 pdt;
-	bool ldps;
-	u8 dpcd_rev;
-	u8 num_sdp_streams;
-	u8 num_sdp_stream_sinks;
-	uint16_t available_pbn;
-	struct list_head next;
-	struct drm_dp_mst_branch *mstb; /* pointer to an mstb if this port has one */
-	struct drm_dp_aux aux; /* i2c bus for this port? */
-	struct drm_dp_mst_branch *parent;
-
-	struct drm_dp_vcpi vcpi;
-	struct drm_connector *connector;
-	struct drm_dp_mst_topology_mgr *mgr;
-
-	/**
-	 * @cached_edid: for DP logical ports - make tiling work by ensuring
-	 * that the EDID for all connectors is read immediately.
-	 */
-	struct edid *cached_edid;
-	/**
-	 * @has_audio: Tracks whether the sink connector to this port is
-	 * audio-capable.
-	 */
-	bool has_audio;
-};
-
 /**
  * struct drm_dp_mst_branch - MST branch device.
  * @kref: reference count for this port.
-- 
2.19.1

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH 6/6] drm/i915: Start using struct drm_dp_mst_port again

[Lyude Paul]

Originally we started storing pointers to the drm_dp_mst_port struct for
each intel_connector as void* to stop people from trying to dereference
them. Now that we've removed the public struct definition for
drm_dp_mst_port however, it's no longer possible to dereference the port
structure even when using the proper type. So, move back to struct
drm_dp_mst_port from void* for clarity.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/i915/intel_dp_mst.c | 2 +-
 drivers/gpu/drm/i915/intel_drv.h    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 4de247ddf05f..3408efe67694 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -39,7 +39,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
 	struct intel_digital_port *intel_dig_port = intel_mst->primary;
 	struct intel_dp *intel_dp = &intel_dig_port->dp;
 	struct drm_connector *connector = conn_state->connector;
-	void *port = to_intel_connector(connector)->port;
+	struct drm_dp_mst_port *port = to_intel_connector(connector)->port;
 	struct drm_atomic_state *state = pipe_config->base.state;
 	int bpp;
 	int lane_count, slots = 0;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f575ba2a59da..1bb69097d6cb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -415,7 +415,7 @@ struct intel_connector {
 	   state of connector->polled in case hotplug storm detection changes it */
 	u8 polled;
 
-	void *port; /* store this opaque as its illegal to dereference it */
+	struct drm_dp_mst_port *port;
 
 	struct intel_dp *mst_port;
 
-- 
2.19.1

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

✓ Fi.CI.BAT: success for Remove all bad dp_mst_port uses and hide struct def

[Patchwork]

== Series Details ==

Series: Remove all bad dp_mst_port uses and hide struct def
URL   : https://patchwork.freedesktop.org/series/52636/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5154 -> Patchwork_10844 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/52636/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10844 that come from known issues:

  === IGT changes ===

    ==== Possible fixes ====

    igt@gem_ctx_create@basic-files:
      fi-bsw-kefka:       FAIL (fdo#108656) -> PASS

    igt@i915_module_load@reload:
      fi-blb-e6850:       INCOMPLETE (fdo#107718) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
      fi-byt-clapper:     FAIL (fdo#103191, fdo#107362) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS

    
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#108656 https://bugs.freedesktop.org/show_bug.cgi?id=108656


== Participating hosts (50 -> 46) ==

  Missing    (4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_5154 -> Patchwork_10844

  CI_DRM_5154: 2ba924da1cb3a684f538e0c25e2409b942a5a35a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4718: 8ac5cfb4db9c7bc593beec18a6be1e2ff163106c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10844: af900c99ec9037164b46a79abadd6d22d02af78e @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

af900c99ec90 drm/i915: Start using struct drm_dp_mst_port again
f7e93f99d599 drm/dp_mst: Hide drm_dp_mst_port contents from drivers
1affa2235cf2 drm/nouveau: Stop reading port->mgr in nv50_mstc_detect()
010aca1e996b drm/nouveau: Stop reading port->mgr in nv50_mstc_get_modes()
81eb3aab81da drm/nouveau: Use drm_dp_get_payload_info() for getting payload/vcpi
268300d13d06 drm/dp_mst: Add drm_dp_get_payload_info()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10844/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for Remove all bad dp_mst_port uses and hide struct def

[Patchwork]

== Series Details ==

Series: Remove all bad dp_mst_port uses and hide struct def
URL   : https://patchwork.freedesktop.org/series/52636/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5154_full -> Patchwork_10844_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10844_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10844_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10844_full:

  === IGT changes ===

    ==== Warnings ====

    igt@i915_suspend@shrink:
      shard-skl:          INCOMPLETE (fdo#106886) -> DMESG-WARN

    igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-shrfb-draw-mmap-wc:
      shard-hsw:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_10844_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_schedule@pi-ringfull-bsd:
      shard-skl:          NOTRUN -> FAIL (fdo#103158)

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-skl:          NOTRUN -> TIMEOUT (fdo#108039)

    igt@kms_chv_cursor_fail@pipe-a-128x128-left-edge:
      shard-skl:          NOTRUN -> FAIL (fdo#104671)

    igt@kms_cursor_crc@cursor-128x128-random:
      shard-glk:          PASS -> FAIL (fdo#103232)

    igt@kms_cursor_crc@cursor-256x256-offscreen:
      shard-skl:          NOTRUN -> FAIL (fdo#103232) +1

    igt@kms_cursor_legacy@cursora-vs-flipa-toggle:
      shard-glk:          PASS -> DMESG-WARN (fdo#105763, fdo#106538)

    igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-untiled:
      shard-glk:          PASS -> DMESG-FAIL (fdo#103184, fdo#105763, fdo#106538)

    igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-xtiled:
      shard-skl:          PASS -> FAIL (fdo#103184)

    igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-untiled:
      shard-glk:          PASS -> FAIL (fdo#103184, fdo#108472)

    igt@kms_flip@2x-flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#105363)

    igt@kms_flip@flip-vs-modeset-vs-hang-interruptible:
      shard-apl:          PASS -> INCOMPLETE (fdo#103927)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
      shard-skl:          NOTRUN -> FAIL (fdo#103167) +4

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
      shard-glk:          PASS -> FAIL (fdo#103167) +3

    igt@kms_frontbuffer_tracking@fbcpsr-modesetfrombusy:
      shard-skl:          NOTRUN -> FAIL (fdo#105682)

    igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-mmap-cpu:
      shard-skl:          PASS -> FAIL (fdo#103167)

    igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb:
      shard-skl:          NOTRUN -> FAIL (fdo#108145) +2

    igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
      shard-glk:          PASS -> FAIL (fdo#103166) +2

    igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
      shard-apl:          PASS -> FAIL (fdo#103166)

    igt@kms_properties@connector-properties-atomic:
      shard-skl:          NOTRUN -> FAIL (fdo#108642)

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)

    igt@kms_sysfs_edid_timing:
      shard-skl:          NOTRUN -> FAIL (fdo#100047)

    igt@kms_universal_plane@universal-plane-gen9-features-pipe-a:
      shard-kbl:          PASS -> DMESG-WARN (fdo#105602, fdo#103558) +13

    igt@kms_universal_plane@universal-plane-pipe-a-sanity:
      shard-glk:          PASS -> INCOMPLETE (k.org#198133, fdo#103359)

    igt@perf@polling:
      shard-hsw:          PASS -> FAIL (fdo#102252)

    igt@pm_rpm@system-suspend-devices:
      shard-skl:          PASS -> INCOMPLETE (fdo#107807)

    
    ==== Possible fixes ====

    igt@kms_chv_cursor_fail@pipe-c-128x128-top-edge:
      shard-skl:          FAIL (fdo#104671) -> PASS

    igt@kms_cursor_crc@cursor-64x21-random:
      shard-apl:          FAIL (fdo#103232) -> PASS +3

    igt@kms_cursor_crc@cursor-64x64-offscreen:
      shard-skl:          FAIL (fdo#103232) -> PASS

    igt@kms_flip_tiling@flip-y-tiled:
      shard-skl:          FAIL (fdo#108303) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff:
      shard-glk:          FAIL (fdo#103167) -> PASS +1

    igt@kms_plane@plane-position-covered-pipe-c-planes:
      shard-apl:          FAIL (fdo#103166) -> PASS +1

    igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
      shard-skl:          FAIL (fdo#107815) -> PASS

    igt@kms_universal_plane@universal-plane-pipe-a-functional:
      shard-glk:          FAIL (fdo#103166) -> PASS +1

    igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
      shard-skl:          INCOMPLETE (fdo#104108, fdo#107773) -> PASS

    igt@pm_rpm@gem-execbuf-stress-pc8:
      shard-skl:          INCOMPLETE (fdo#107807) -> SKIP

    
    ==== Warnings ====

    igt@kms_setmode@basic:
      shard-kbl:          FAIL (fdo#99912) -> DMESG-WARN (fdo#105602, fdo#103558)

    
  fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815
  fdo#108039 https://bugs.freedesktop.org/show_bug.cgi?id=108039
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108303 https://bugs.freedesktop.org/show_bug.cgi?id=108303
  fdo#108472 https://bugs.freedesktop.org/show_bug.cgi?id=108472
  fdo#108642 https://bugs.freedesktop.org/show_bug.cgi?id=108642
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (7 -> 6) ==

  Missing    (1): shard-iclb 


== Build changes ==

    * Linux: CI_DRM_5154 -> Patchwork_10844

  CI_DRM_5154: 2ba924da1cb3a684f538e0c25e2409b942a5a35a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4718: 8ac5cfb4db9c7bc593beec18a6be1e2ff163106c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10844: af900c99ec9037164b46a79abadd6d22d02af78e @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10844/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/6] drm/nouveau: Stop reading port->mgr in nv50_mstc_get_modes()

[Sasha Levin]

Hi,

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v4.19.2, v4.18.19, v4.14.81, v4.9.137, v4.4.163, v3.18.125, 

v4.19.2: Build OK!
v4.18.19: Failed to apply! Possible dependencies:
    Unable to calculate

v4.14.81: Failed to apply! Possible dependencies:
    Unable to calculate

v4.9.137: Failed to apply! Possible dependencies:
    f479c0ba4a17 ("drm/nouveau/kms/nv50: initial support for DP 1.2 multi-stream")

v4.4.163: Failed to apply! Possible dependencies:
    f479c0ba4a17 ("drm/nouveau/kms/nv50: initial support for DP 1.2 multi-stream")

v3.18.125: Failed to apply! Possible dependencies:
    f479c0ba4a17 ("drm/nouveau/kms/nv50: initial support for DP 1.2 multi-stream")


How should we proceed with this patch?

--
Thanks,
Sasha

Re: [PATCH 3/6] drm/nouveau: Stop reading port->mgr in nv50_mstc_get_modes()

[Sasha Levin]

Hi,

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v4.19.2, v4.18.19, v4.14.81, v4.9.137, v4.4.163, v3.18.125, 

v4.19.2: Build OK!
v4.18.19: Failed to apply! Possible dependencies:
    Unable to calculate

v4.14.81: Failed to apply! Possible dependencies:
    Unable to calculate

v4.9.137: Failed to apply! Possible dependencies:
    f479c0ba4a17 ("drm/nouveau/kms/nv50: initial support for DP 1.2 multi-stream")

v4.4.163: Failed to apply! Possible dependencies:
    f479c0ba4a17 ("drm/nouveau/kms/nv50: initial support for DP 1.2 multi-stream")

v3.18.125: Failed to apply! Possible dependencies:
    f479c0ba4a17 ("drm/nouveau/kms/nv50: initial support for DP 1.2 multi-stream")


How should we proceed with this patch?

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

Re: [PATCH 4/6] drm/nouveau: Stop reading port->mgr in nv50_mstc_detect()

[Sasha Levin]

Hi,

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v4.19.2, v4.18.19, v4.14.81, v4.9.137, v4.4.163, v3.18.125, 

v4.19.2: Build OK!
v4.18.19: Build OK!
v4.14.81: Failed to apply! Possible dependencies:
    e46368cf77f2 ("drm/nouveau/drm/nouveau: Grab runtime PM ref in nv50_mstc_detect()")

v4.9.137: Failed to apply! Possible dependencies:
    e46368cf77f2 ("drm/nouveau/drm/nouveau: Grab runtime PM ref in nv50_mstc_detect()")

v4.4.163: Failed to apply! Possible dependencies:
    e46368cf77f2 ("drm/nouveau/drm/nouveau: Grab runtime PM ref in nv50_mstc_detect()")

v3.18.125: Failed to apply! Possible dependencies:
    e46368cf77f2 ("drm/nouveau/drm/nouveau: Grab runtime PM ref in nv50_mstc_detect()")


How should we proceed with this patch?

--
Thanks,
Sasha

Re: [PATCH 4/6] drm/nouveau: Stop reading port->mgr in nv50_mstc_detect()

[Sasha Levin]

Hi,

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v4.19.2, v4.18.19, v4.14.81, v4.9.137, v4.4.163, v3.18.125, 

v4.19.2: Build OK!
v4.18.19: Build OK!
v4.14.81: Failed to apply! Possible dependencies:
    e46368cf77f2 ("drm/nouveau/drm/nouveau: Grab runtime PM ref in nv50_mstc_detect()")

v4.9.137: Failed to apply! Possible dependencies:
    e46368cf77f2 ("drm/nouveau/drm/nouveau: Grab runtime PM ref in nv50_mstc_detect()")

v4.4.163: Failed to apply! Possible dependencies:
    e46368cf77f2 ("drm/nouveau/drm/nouveau: Grab runtime PM ref in nv50_mstc_detect()")

v3.18.125: Failed to apply! Possible dependencies:
    e46368cf77f2 ("drm/nouveau/drm/nouveau: Grab runtime PM ref in nv50_mstc_detect()")


How should we proceed with this patch?

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

Re: [PATCH 0/6] Remove all bad dp_mst_port uses and hide struct def

[Karol Herbst]

Patches 1 and 5 are Acked-by: Karol Herbst <kherbst@redhat.com>
Patches 2-4 are Reviewed-by: Karol Herbst <kherbst@redhat.com>
On Sat, Nov 17, 2018 at 1:21 AM Lyude Paul <lyude@redhat.com> wrote:
>
> So we don't ever have to worry about drivers touching drm_dp_mst_port
> structs without verifying them and crashing again.
>
> Lyude Paul (6):
>   drm/dp_mst: Add drm_dp_get_payload_info()
>   drm/nouveau: Use drm_dp_get_payload_info() for getting payload/vcpi
>   drm/nouveau: Stop reading port->mgr in nv50_mstc_get_modes()
>   drm/nouveau: Stop reading port->mgr in nv50_mstc_detect()
>   drm/dp_mst: Hide drm_dp_mst_port contents from drivers
>   drm/i915: Start using struct drm_dp_mst_port again
>
>  drivers/gpu/drm/drm_dp_mst_topology.c   | 115 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dp_mst.c     |   2 +-
>  drivers/gpu/drm/i915/intel_drv.h        |   2 +-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c |  60 +++++--------
>  include/drm/drm_dp_mst_helper.h         |  65 ++------------
>  5 files changed, 146 insertions(+), 98 deletions(-)
>
> --
> 2.19.1
>
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH 4/6] drm/nouveau: Stop reading port->mgr in nv50_mstc_detect()

[Lyude Paul]

On Sat, 2018-11-17 at 12:24 +0000, Sasha Levin wrote:
> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: all
> 
> The bot has tested the following trees: v4.19.2, v4.18.19, v4.14.81,
> v4.9.137, v4.4.163, v3.18.125, 
> 
> v4.19.2: Build OK!
> v4.18.19: Build OK!
> v4.14.81: Failed to apply! Possible dependencies:
>     e46368cf77f2 ("drm/nouveau/drm/nouveau: Grab runtime PM ref in
> nv50_mstc_detect()")
> 
> v4.9.137: Failed to apply! Possible dependencies:
>     e46368cf77f2 ("drm/nouveau/drm/nouveau: Grab runtime PM ref in
> nv50_mstc_detect()")
> 
> v4.4.163: Failed to apply! Possible dependencies:
>     e46368cf77f2 ("drm/nouveau/drm/nouveau: Grab runtime PM ref in
> nv50_mstc_detect()")
> 
> v3.18.125: Failed to apply! Possible dependencies:
>     e46368cf77f2 ("drm/nouveau/drm/nouveau: Grab runtime PM ref in
> nv50_mstc_detect()")
> 
> 
> How should we proceed with this patch?

e46368cf77f2 ("drm/nouveau/drm/nouveau: Grab runtime PM ref in nv50_mstc_detect()")

Should also be backported for v4.9+, and then this patch should be applied on
top of that.
> 
> --
> Thanks,
> Sasha
-- 
Cheers,
	Lyude Paul

Re: [PATCH 1/6] drm/dp_mst: Add drm_dp_get_payload_info()

[Daniel Vetter]

On Fri, Nov 16, 2018 at 07:21:15PM -0500, Lyude Paul wrote:
> Some hardware (nvidia hardware in particular) needs to be notified of
> the exact VCPI and payload settings that the topology manager decided on
> for each mstb port. Since there isn't currently any way to get this
> information without going through port (which drivers are very much not
> supposed to do by themselves, ever), let's add one.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 56 +++++++++++++++++++++++++++
>  include/drm/drm_dp_mst_helper.h       |  5 ++-
>  2 files changed, 60 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 529414556962..4336d17ce904 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1982,6 +1982,62 @@ int drm_dp_update_payload_part2(struct drm_dp_mst_topology_mgr *mgr)
>  }
>  EXPORT_SYMBOL(drm_dp_update_payload_part2);
>  
> +/**
> + * drm_dp_get_payload_info() - Retrieve payload/vcpi information for the given
> + *                             @port
> + * @mgr: manager to use
> + * @port: the port to get the relevant payload information for
> + * @vcpi_out: where to copy the port's VCPI information to
> + * @payload_out: where to copy the port's payload information to
> + *
> + * Searches the current payloads for @mgr and finds the relevant payload and
> + * VCPI information that was programmed by the topology mgr, then copies it
> + * into @vcpi_out and @payload_out. Drivers which need to know this
> + * information must use this helper as opposed to checking @port themselves,
> + * as this helper will ensure the port reference is still valid and grab the
> + * appropriate locks in @mgr.
> + *
> + * Returns:
> + * 0 on success, negative error code if the port is no longer valid or a
> + * programmed payload could not be found for @port.
> + */
> +int drm_dp_get_payload_info(struct drm_dp_mst_topology_mgr *mgr,
> +			    struct drm_dp_mst_port *port,
> +			    struct drm_dp_vcpi *vcpi_out,
> +			    struct drm_dp_payload *payload_out)
> +{
> +	struct drm_dp_payload *payload = NULL;
> +	int i;
> +	int ret = 0;
> +
> +	port = drm_dp_get_validated_port_ref(mgr, port);
> +	if (!port)
> +		return -EINVAL;

This is the part that I mean in our other/irc discussions. The
dp_get_validated_port here could fail when it's going to surprise the
driver. With the dp_port_malloc_get stuff we could instead just grab a
port_malloc_kref when storing the port in mgr->payload, which would
guarantee that the port based lookup below still works.

> +
> +	mutex_lock(&mgr->payload_lock);
> +	/* Figure out which of the payloads belongs to this port */
> +	for (i = 0; i < mgr->max_payloads; i++) {
> +		if (mgr->payloads[i].vcpi == port->vcpi.vcpi) {

Or maybe even rework the lookup here to use the port pointer (as an
abstract key) instead of port->vcpi.vcpi. With port_malloc_kref we could
guarantee that it keeps working, even after the port has been destroyed.

And (without checking) I think that's needed anyway to clean up the
payload update hacks in the connector destroy work ...
-Daniel

> +			payload = &mgr->payloads[i];
> +			break;
> +		}
> +	}
> +
> +	if (!payload) {
> +		DRM_DEBUG_KMS("Failed to find payload for port %p\n", port);
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	*payload_out = *payload;
> +	*vcpi_out = port->vcpi;
> +out:
> +	mutex_unlock(&mgr->payload_lock);
> +	drm_dp_put_port(port);
> +	return ret;
> +}
> +EXPORT_SYMBOL(drm_dp_get_payload_info);
> +
>  #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,
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index 59f005b419cf..9cc93ea60e7e 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -592,7 +592,10 @@ bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
>  			      struct drm_dp_mst_port *port, int pbn, int slots);
>  
>  int drm_dp_mst_get_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
> -
> +int drm_dp_get_payload_info(struct drm_dp_mst_topology_mgr *mgr,
> +			    struct drm_dp_mst_port *port,
> +			    struct drm_dp_vcpi *vcpi_out,
> +			    struct drm_dp_payload *payload_out);
>  
>  void drm_dp_mst_reset_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
>  
> -- 
> 2.19.1
> 
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH 1/6] drm/dp_mst: Add drm_dp_get_payload_info()

[Lyude Paul]

On Tue, 2018-11-27 at 22:23 +0100, Daniel Vetter wrote:
> On Fri, Nov 16, 2018 at 07:21:15PM -0500, Lyude Paul wrote:
> > Some hardware (nvidia hardware in particular) needs to be notified of
> > the exact VCPI and payload settings that the topology manager decided on
> > for each mstb port. Since there isn't currently any way to get this
> > information without going through port (which drivers are very much not
> > supposed to do by themselves, ever), let's add one.
> > 
> > Signed-off-by: Lyude Paul <lyude@redhat.com>
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c | 56 +++++++++++++++++++++++++++
> >  include/drm/drm_dp_mst_helper.h       |  5 ++-
> >  2 files changed, 60 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 529414556962..4336d17ce904 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -1982,6 +1982,62 @@ int drm_dp_update_payload_part2(struct
> > drm_dp_mst_topology_mgr *mgr)
> >  }
> >  EXPORT_SYMBOL(drm_dp_update_payload_part2);
> >  
> > +/**
> > + * drm_dp_get_payload_info() - Retrieve payload/vcpi information for the
> > given
> > + *                             @port
> > + * @mgr: manager to use
> > + * @port: the port to get the relevant payload information for
> > + * @vcpi_out: where to copy the port's VCPI information to
> > + * @payload_out: where to copy the port's payload information to
> > + *
> > + * Searches the current payloads for @mgr and finds the relevant payload
> > and
> > + * VCPI information that was programmed by the topology mgr, then copies
> > it
> > + * into @vcpi_out and @payload_out. Drivers which need to know this
> > + * information must use this helper as opposed to checking @port
> > themselves,
> > + * as this helper will ensure the port reference is still valid and grab
> > the
> > + * appropriate locks in @mgr.
> > + *
> > + * Returns:
> > + * 0 on success, negative error code if the port is no longer valid or a
> > + * programmed payload could not be found for @port.
> > + */
> > +int drm_dp_get_payload_info(struct drm_dp_mst_topology_mgr *mgr,
> > +			    struct drm_dp_mst_port *port,
> > +			    struct drm_dp_vcpi *vcpi_out,
> > +			    struct drm_dp_payload *payload_out)
> > +{
> > +	struct drm_dp_payload *payload = NULL;
> > +	int i;
> > +	int ret = 0;
> > +
> > +	port = drm_dp_get_validated_port_ref(mgr, port);
> > +	if (!port)
> > +		return -EINVAL;
> 
> This is the part that I mean in our other/irc discussions. The
> dp_get_validated_port here could fail when it's going to surprise the
> driver. With the dp_port_malloc_get stuff we could instead just grab a
> port_malloc_kref when storing the port in mgr->payload, which would
> guarantee that the port based lookup below still works.
Yeah, that makes more sense now that there's context :P, sgtm.

> 
> > +
> > +	mutex_lock(&mgr->payload_lock);
> > +	/* Figure out which of the payloads belongs to this port */
> > +	for (i = 0; i < mgr->max_payloads; i++) {
> > +		if (mgr->payloads[i].vcpi == port->vcpi.vcpi) {
> 
> Or maybe even rework the lookup here to use the port pointer (as an
> abstract key) instead of port->vcpi.vcpi. With port_malloc_kref we could
> guarantee that it keeps working, even after the port has been destroyed.
> 
> And (without checking) I think that's needed anyway to clean up the
> payload update hacks in the connector destroy work ...
> -Daniel
> 
> > +			payload = &mgr->payloads[i];
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (!payload) {
> > +		DRM_DEBUG_KMS("Failed to find payload for port %p\n", port);
> > +		ret = -EINVAL;
> > +		goto out;
> > +	}
> > +
> > +	*payload_out = *payload;
> > +	*vcpi_out = port->vcpi;
> > +out:
> > +	mutex_unlock(&mgr->payload_lock);
> > +	drm_dp_put_port(port);
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(drm_dp_get_payload_info);
> > +
> >  #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,
> > diff --git a/include/drm/drm_dp_mst_helper.h
> > b/include/drm/drm_dp_mst_helper.h
> > index 59f005b419cf..9cc93ea60e7e 100644
> > --- a/include/drm/drm_dp_mst_helper.h
> > +++ b/include/drm/drm_dp_mst_helper.h
> > @@ -592,7 +592,10 @@ bool drm_dp_mst_allocate_vcpi(struct
> > drm_dp_mst_topology_mgr *mgr,
> >  			      struct drm_dp_mst_port *port, int pbn, int
> > slots);
> >  
> >  int drm_dp_mst_get_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr, struct
> > drm_dp_mst_port *port);
> > -
> > +int drm_dp_get_payload_info(struct drm_dp_mst_topology_mgr *mgr,
> > +			    struct drm_dp_mst_port *port,
> > +			    struct drm_dp_vcpi *vcpi_out,
> > +			    struct drm_dp_payload *payload_out);
> >  
> >  void drm_dp_mst_reset_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
> > struct drm_dp_mst_port *port);
> >  
> > -- 
> > 2.19.1
> > 
> > _______________________________________________
> > Nouveau mailing list
> > Nouveau@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/nouveau
-- 
Cheers,
	Lyude Paul

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

Re: [PATCH 0/6] Remove all bad dp_mst_port uses and hide struct def

[Ben Skeggs]

For the series:

Acked-by: Ben Skeggs <bskeggs@redhat.com>
On Sat, 17 Nov 2018 at 10:21, Lyude Paul <lyude@redhat.com> wrote:
>
> So we don't ever have to worry about drivers touching drm_dp_mst_port
> structs without verifying them and crashing again.
>
> Lyude Paul (6):
>   drm/dp_mst: Add drm_dp_get_payload_info()
>   drm/nouveau: Use drm_dp_get_payload_info() for getting payload/vcpi
>   drm/nouveau: Stop reading port->mgr in nv50_mstc_get_modes()
>   drm/nouveau: Stop reading port->mgr in nv50_mstc_detect()
>   drm/dp_mst: Hide drm_dp_mst_port contents from drivers
>   drm/i915: Start using struct drm_dp_mst_port again
>
>  drivers/gpu/drm/drm_dp_mst_topology.c   | 115 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dp_mst.c     |   2 +-
>  drivers/gpu/drm/i915/intel_drv.h        |   2 +-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c |  60 +++++--------
>  include/drm/drm_dp_mst_helper.h         |  65 ++------------
>  5 files changed, 146 insertions(+), 98 deletions(-)
>
> --
> 2.19.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel