[CI 1/4] drm/i915/dp_mst: Add the MST topology state for modesetted CRTCs

[CI 1/4] drm/i915/dp_mst: Add the MST topology state for modesetted CRTCs

[Imre Deak]

Add the MST topology for a CRTC to the atomic state if the driver
needs to force a modeset on the CRTC after the encoder compute config
functions are called.

Later the MST encoder's disable hook also adds the state, but that isn't
guaranteed to work (since in that hook getting the state may fail, which
can't be handled there). This should fix that, while a later patch fixes
the use of the MST state in the disable hook.

v2: Add missing forward struct declartions, caught by hdrtest.
v3: Factor out intel_dp_mst_add_topology_state_for_connector() used
    later in the patchset.

Cc: Lyude Paul <lyude@redhat.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: stable@vger.kernel.org # 6.1
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> # v2
Reviewed-by: Lyude Paul <lyude@redhat.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c |  4 ++
 drivers/gpu/drm/i915/display/intel_dp_mst.c  | 61 ++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dp_mst.h  |  4 ++
 3 files changed, 69 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 166662ade593c..38106cf63b3b9 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5936,6 +5936,10 @@ int intel_modeset_all_pipes(struct intel_atomic_state *state,
 		if (ret)
 			return ret;
 
+		ret = intel_dp_mst_add_topology_state_for_crtc(state, crtc);
+		if (ret)
+			return ret;
+
 		ret = intel_atomic_add_affected_planes(state, crtc);
 		if (ret)
 			return ret;
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 8b0e4defa3f10..f3cb12dcfe0a7 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -1223,3 +1223,64 @@ bool intel_dp_mst_is_slave_trans(const struct intel_crtc_state *crtc_state)
 	return crtc_state->mst_master_transcoder != INVALID_TRANSCODER &&
 	       crtc_state->mst_master_transcoder != crtc_state->cpu_transcoder;
 }
+
+/**
+ * intel_dp_mst_add_topology_state_for_connector - add MST topology state for a connector
+ * @state: atomic state
+ * @connector: connector to add the state for
+ * @crtc: the CRTC @connector is attached to
+ *
+ * Add the MST topology state for @connector to @state.
+ *
+ * Returns 0 on success, negative error code on failure.
+ */
+static int
+intel_dp_mst_add_topology_state_for_connector(struct intel_atomic_state *state,
+					      struct intel_connector *connector,
+					      struct intel_crtc *crtc)
+{
+	struct drm_dp_mst_topology_state *mst_state;
+
+	if (!connector->mst_port)
+		return 0;
+
+	mst_state = drm_atomic_get_mst_topology_state(&state->base,
+						      &connector->mst_port->mst_mgr);
+	if (IS_ERR(mst_state))
+		return PTR_ERR(mst_state);
+
+	mst_state->pending_crtc_mask |= drm_crtc_mask(&crtc->base);
+
+	return 0;
+}
+
+/**
+ * intel_dp_mst_add_topology_state_for_crtc - add MST topology state for a CRTC
+ * @state: atomic state
+ * @crtc: CRTC to add the state for
+ *
+ * Add the MST topology state for @crtc to @state.
+ *
+ * Returns 0 on success, negative error code on failure.
+ */
+int intel_dp_mst_add_topology_state_for_crtc(struct intel_atomic_state *state,
+					     struct intel_crtc *crtc)
+{
+	struct drm_connector *_connector;
+	struct drm_connector_state *conn_state;
+	int i;
+
+	for_each_new_connector_in_state(&state->base, _connector, conn_state, i) {
+		struct intel_connector *connector = to_intel_connector(_connector);
+		int ret;
+
+		if (conn_state->crtc != &crtc->base)
+			continue;
+
+		ret = intel_dp_mst_add_topology_state_for_connector(state, connector, crtc);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.h b/drivers/gpu/drm/i915/display/intel_dp_mst.h
index f7301de6cdfb3..f1815bb722672 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.h
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.h
@@ -8,6 +8,8 @@
 
 #include <linux/types.h>
 
+struct intel_atomic_state;
+struct intel_crtc;
 struct intel_crtc_state;
 struct intel_digital_port;
 struct intel_dp;
@@ -18,5 +20,7 @@ int intel_dp_mst_encoder_active_links(struct intel_digital_port *dig_port);
 bool intel_dp_mst_is_master_trans(const struct intel_crtc_state *crtc_state);
 bool intel_dp_mst_is_slave_trans(const struct intel_crtc_state *crtc_state);
 bool intel_dp_mst_source_support(struct intel_dp *intel_dp);
+int intel_dp_mst_add_topology_state_for_crtc(struct intel_atomic_state *state,
+					     struct intel_crtc *crtc);
 
 #endif /* __INTEL_DP_MST_H__ */
-- 
2.37.1

[CI 2/4] drm/display/dp_mst: Handle old/new payload states in drm_dp_remove_payload()

[Imre Deak]

Atm, drm_dp_remove_payload() uses the same payload state to both get the
vc_start_slot required for the payload removal DPCD message and to
deduct time_slots from vc_start_slot of all payloads after the one being
removed.

The above isn't always correct, as vc_start_slot must be the up-to-date
version contained in the new payload state, but time_slots must be the
one used when the payload was previously added, contained in the old
payload state. The new payload's time_slots can change vs. the old one
if the current atomic commit changes the corresponding mode.

This patch let's drivers pass the old and new payload states to
drm_dp_remove_payload(), but keeps these the same for now in all drivers
not to change the behavior. A follow-up i915 patch will pass in that
driver the correct old and new states to the function.

Cc: Lyude Paul <lyude@redhat.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Karol Herbst <kherbst@redhat.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Wayne Lin <Wayne.Lin@amd.com>
Cc: stable@vger.kernel.org # 6.1
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Lyude Paul <lyude@redhat.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c |  2 +-
 drivers/gpu/drm/display/drm_dp_mst_topology.c | 26 ++++++++++---------
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |  4 ++-
 drivers/gpu/drm/nouveau/dispnv50/disp.c       |  2 +-
 include/drm/display/drm_dp_mst_helper.h       |  3 ++-
 5 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index a50319fc42b11..180d3893b68da 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -208,7 +208,7 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
 	if (enable)
 		drm_dp_add_payload_part1(mst_mgr, mst_state, payload);
 	else
-		drm_dp_remove_payload(mst_mgr, mst_state, payload);
+		drm_dp_remove_payload(mst_mgr, mst_state, payload, payload);
 
 	/* mst_mgr->->payloads are VC payload notify MST branch using DPCD or
 	 * AUX message. The sequence is slot 1-63 allocated sequence for each
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 847c10aa2098c..1990ff5dc7ddd 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -3342,7 +3342,8 @@ EXPORT_SYMBOL(drm_dp_add_payload_part1);
  * drm_dp_remove_payload() - Remove an MST payload
  * @mgr: Manager to use.
  * @mst_state: The MST atomic state
- * @payload: The payload to write
+ * @old_payload: The payload with its old state
+ * @new_payload: The payload to write
  *
  * Removes a payload from an MST topology if it was successfully assigned a start slot. Also updates
  * the starting time slots of all other payloads which would have been shifted towards the start of
@@ -3350,36 +3351,37 @@ EXPORT_SYMBOL(drm_dp_add_payload_part1);
  */
 void drm_dp_remove_payload(struct drm_dp_mst_topology_mgr *mgr,
 			   struct drm_dp_mst_topology_state *mst_state,
-			   struct drm_dp_mst_atomic_payload *payload)
+			   const struct drm_dp_mst_atomic_payload *old_payload,
+			   struct drm_dp_mst_atomic_payload *new_payload)
 {
 	struct drm_dp_mst_atomic_payload *pos;
 	bool send_remove = false;
 
 	/* We failed to make the payload, so nothing to do */
-	if (payload->vc_start_slot == -1)
+	if (new_payload->vc_start_slot == -1)
 		return;
 
 	mutex_lock(&mgr->lock);
-	send_remove = drm_dp_mst_port_downstream_of_branch(payload->port, mgr->mst_primary);
+	send_remove = drm_dp_mst_port_downstream_of_branch(new_payload->port, mgr->mst_primary);
 	mutex_unlock(&mgr->lock);
 
 	if (send_remove)
-		drm_dp_destroy_payload_step1(mgr, mst_state, payload);
+		drm_dp_destroy_payload_step1(mgr, mst_state, new_payload);
 	else
 		drm_dbg_kms(mgr->dev, "Payload for VCPI %d not in topology, not sending remove\n",
-			    payload->vcpi);
+			    new_payload->vcpi);
 
 	list_for_each_entry(pos, &mst_state->payloads, next) {
-		if (pos != payload && pos->vc_start_slot > payload->vc_start_slot)
-			pos->vc_start_slot -= payload->time_slots;
+		if (pos != new_payload && pos->vc_start_slot > new_payload->vc_start_slot)
+			pos->vc_start_slot -= old_payload->time_slots;
 	}
-	payload->vc_start_slot = -1;
+	new_payload->vc_start_slot = -1;
 
 	mgr->payload_count--;
-	mgr->next_start_slot -= payload->time_slots;
+	mgr->next_start_slot -= old_payload->time_slots;
 
-	if (payload->delete)
-		drm_dp_mst_put_port_malloc(payload->port);
+	if (new_payload->delete)
+		drm_dp_mst_put_port_malloc(new_payload->port);
 }
 EXPORT_SYMBOL(drm_dp_remove_payload);
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index f3cb12dcfe0a7..dc4e5ff1dbb31 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -526,6 +526,8 @@ static void intel_mst_disable_dp(struct intel_atomic_state *state,
 		to_intel_connector(old_conn_state->connector);
 	struct drm_dp_mst_topology_state *mst_state =
 		drm_atomic_get_mst_topology_state(&state->base, &intel_dp->mst_mgr);
+	struct drm_dp_mst_atomic_payload *payload =
+		drm_atomic_get_mst_payload_state(mst_state, connector->port);
 	struct drm_i915_private *i915 = to_i915(connector->base.dev);
 
 	drm_dbg_kms(&i915->drm, "active links %d\n",
@@ -534,7 +536,7 @@ static void intel_mst_disable_dp(struct intel_atomic_state *state,
 	intel_hdcp_disable(intel_mst->connector);
 
 	drm_dp_remove_payload(&intel_dp->mst_mgr, mst_state,
-			      drm_atomic_get_mst_payload_state(mst_state, connector->port));
+			      payload, payload);
 
 	intel_audio_codec_disable(encoder, old_crtc_state, old_conn_state);
 }
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index edcb2529b4025..ed9d374147b8d 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -885,7 +885,7 @@ nv50_msto_prepare(struct drm_atomic_state *state,
 
 	// TODO: Figure out if we want to do a better job of handling VCPI allocation failures here?
 	if (msto->disabled) {
-		drm_dp_remove_payload(mgr, mst_state, payload);
+		drm_dp_remove_payload(mgr, mst_state, payload, payload);
 
 		nvif_outp_dp_mst_vcpi(&mstm->outp->outp, msto->head->base.index, 0, 0, 0, 0);
 	} else {
diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h
index 41fd8352ab656..f5eb9aa152b14 100644
--- a/include/drm/display/drm_dp_mst_helper.h
+++ b/include/drm/display/drm_dp_mst_helper.h
@@ -841,7 +841,8 @@ int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr,
 			     struct drm_dp_mst_atomic_payload *payload);
 void drm_dp_remove_payload(struct drm_dp_mst_topology_mgr *mgr,
 			   struct drm_dp_mst_topology_state *mst_state,
-			   struct drm_dp_mst_atomic_payload *payload);
+			   const struct drm_dp_mst_atomic_payload *old_payload,
+			   struct drm_dp_mst_atomic_payload *new_payload);
 
 int drm_dp_check_act_status(struct drm_dp_mst_topology_mgr *mgr);
 
-- 
2.37.1

[CI 3/4] drm/display/dp_mst: Add drm_atomic_get_old_mst_topology_state()

[Imre Deak]

Add a function to get the old MST topology state, required by a
follow-up i915 patch.

While at it clarify the code comment of
drm_atomic_get_new_mst_topology_state() and add _new prefix
to the new state pointer to remind about its difference from the old
state.

v2: Use old_/new_ prefixes for the state pointers. (Ville)

Cc: Lyude Paul <lyude@redhat.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: stable@vger.kernel.org # 6.1
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Lyude Paul <lyude@redhat.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/display/drm_dp_mst_topology.c | 33 ++++++++++++++++---
 include/drm/display/drm_dp_mst_helper.h       |  3 ++
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 1990ff5dc7ddd..38dab76ae69ea 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -5364,28 +5364,53 @@ struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct drm_a
 }
 EXPORT_SYMBOL(drm_atomic_get_mst_topology_state);
 
+/**
+ * drm_atomic_get_old_mst_topology_state: get old MST topology state in atomic state, if any
+ * @state: global atomic state
+ * @mgr: MST topology manager, also the private object in this case
+ *
+ * This function wraps drm_atomic_get_old_private_obj_state() passing in the MST atomic
+ * state vtable so that the private object state returned is that of a MST
+ * topology object.
+ *
+ * Returns:
+ *
+ * The old MST topology state, or NULL if there's no topology state for this MST mgr
+ * in the global atomic state
+ */
+struct drm_dp_mst_topology_state *
+drm_atomic_get_old_mst_topology_state(struct drm_atomic_state *state,
+				      struct drm_dp_mst_topology_mgr *mgr)
+{
+	struct drm_private_state *old_priv_state =
+		drm_atomic_get_old_private_obj_state(state, &mgr->base);
+
+	return old_priv_state ? to_dp_mst_topology_state(old_priv_state) : NULL;
+}
+EXPORT_SYMBOL(drm_atomic_get_old_mst_topology_state);
+
 /**
  * drm_atomic_get_new_mst_topology_state: get new MST topology state in atomic state, if any
  * @state: global atomic state
  * @mgr: MST topology manager, also the private object in this case
  *
- * This function wraps drm_atomic_get_priv_obj_state() passing in the MST atomic
+ * This function wraps drm_atomic_get_new_private_obj_state() passing in the MST atomic
  * state vtable so that the private object state returned is that of a MST
  * topology object.
  *
  * Returns:
  *
- * The MST topology state, or NULL if there's no topology state for this MST mgr
+ * The new MST topology state, or NULL if there's no topology state for this MST mgr
  * in the global atomic state
  */
 struct drm_dp_mst_topology_state *
 drm_atomic_get_new_mst_topology_state(struct drm_atomic_state *state,
 				      struct drm_dp_mst_topology_mgr *mgr)
 {
-	struct drm_private_state *priv_state =
+	struct drm_private_state *new_priv_state =
 		drm_atomic_get_new_private_obj_state(state, &mgr->base);
 
-	return priv_state ? to_dp_mst_topology_state(priv_state) : NULL;
+	return new_priv_state ? to_dp_mst_topology_state(new_priv_state) : NULL;
 }
 EXPORT_SYMBOL(drm_atomic_get_new_mst_topology_state);
 
diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h
index f5eb9aa152b14..32c764fb9cb56 100644
--- a/include/drm/display/drm_dp_mst_helper.h
+++ b/include/drm/display/drm_dp_mst_helper.h
@@ -868,6 +868,9 @@ struct drm_dp_mst_topology_state *
 drm_atomic_get_mst_topology_state(struct drm_atomic_state *state,
 				  struct drm_dp_mst_topology_mgr *mgr);
 struct drm_dp_mst_topology_state *
+drm_atomic_get_old_mst_topology_state(struct drm_atomic_state *state,
+				      struct drm_dp_mst_topology_mgr *mgr);
+struct drm_dp_mst_topology_state *
 drm_atomic_get_new_mst_topology_state(struct drm_atomic_state *state,
 				      struct drm_dp_mst_topology_mgr *mgr);
 struct drm_dp_mst_atomic_payload *
-- 
2.37.1

[CI 4/4] drm/i915/dp_mst: Fix payload removal during output disabling

[Imre Deak]

Use the correct old/new topology and payload states in
intel_mst_disable_dp(). So far drm_atomic_get_mst_topology_state() it
used returned either the old state, in case the state was added already
earlier during the atomic check phase or otherwise the new state (but
the latter could fail, which can't be handled in the enable/disable
hooks). After the first patch in the patchset, the state should always
get added already during the check phase, so here we can get the
old/new states without a failure.

drm_dp_remove_payload() should use time_slots from the old payload state
and vc_start_slot in the new one. It should update the new payload
states to reflect the sink's current payload table after the payload is
removed. Pass the new topology state and the old and new payload states
accordingly.

This also fixes a problem where the payload allocations for multiple MST
streams on the same link got inconsistent after a few commits, as
during payload removal the old instead of the new payload state got
updated, so the subsequent enabling sequence and commits used a stale
payload state.

v2: Constify the old payload state pointer. (Ville)

Cc: Lyude Paul <lyude@redhat.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: stable@vger.kernel.org # 6.1
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Lyude Paul <lyude@redhat.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index dc4e5ff1dbb31..054a009e800d7 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -524,10 +524,14 @@ static void intel_mst_disable_dp(struct intel_atomic_state *state,
 	struct intel_dp *intel_dp = &dig_port->dp;
 	struct intel_connector *connector =
 		to_intel_connector(old_conn_state->connector);
-	struct drm_dp_mst_topology_state *mst_state =
-		drm_atomic_get_mst_topology_state(&state->base, &intel_dp->mst_mgr);
-	struct drm_dp_mst_atomic_payload *payload =
-		drm_atomic_get_mst_payload_state(mst_state, connector->port);
+	struct drm_dp_mst_topology_state *old_mst_state =
+		drm_atomic_get_old_mst_topology_state(&state->base, &intel_dp->mst_mgr);
+	struct drm_dp_mst_topology_state *new_mst_state =
+		drm_atomic_get_new_mst_topology_state(&state->base, &intel_dp->mst_mgr);
+	const struct drm_dp_mst_atomic_payload *old_payload =
+		drm_atomic_get_mst_payload_state(old_mst_state, connector->port);
+	struct drm_dp_mst_atomic_payload *new_payload =
+		drm_atomic_get_mst_payload_state(new_mst_state, connector->port);
 	struct drm_i915_private *i915 = to_i915(connector->base.dev);
 
 	drm_dbg_kms(&i915->drm, "active links %d\n",
@@ -535,8 +539,8 @@ static void intel_mst_disable_dp(struct intel_atomic_state *state,
 
 	intel_hdcp_disable(intel_mst->connector);
 
-	drm_dp_remove_payload(&intel_dp->mst_mgr, mst_state,
-			      payload, payload);
+	drm_dp_remove_payload(&intel_dp->mst_mgr, new_mst_state,
+			      old_payload, new_payload);
 
 	intel_audio_codec_disable(encoder, old_crtc_state, old_conn_state);
 }
-- 
2.37.1

Re: [Intel-gfx] [CI 1/4] drm/i915/dp_mst: Add the MST topology state for modesetted CRTCs

[Imre Deak]

Hi all,

On Mon, Feb 06, 2023 at 01:48:53PM +0200, Imre Deak wrote:
> Add the MST topology for a CRTC to the atomic state if the driver
> needs to force a modeset on the CRTC after the encoder compute config
> functions are called.
> 
> Later the MST encoder's disable hook also adds the state, but that isn't
> guaranteed to work (since in that hook getting the state may fail, which
> can't be handled there). This should fix that, while a later patch fixes
> the use of the MST state in the disable hook.
> 
> v2: Add missing forward struct declartions, caught by hdrtest.
> v3: Factor out intel_dp_mst_add_topology_state_for_connector() used
>     later in the patchset.
> 
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: stable@vger.kernel.org # 6.1
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> # v2
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Is it ok to merge these 4 patches (also at [1]), via the i915 tree?

If so could it be also acked from the AMD and Nouveau side?

[1] https://patchwork.freedesktop.org/series/113703/

> ---
>  drivers/gpu/drm/i915/display/intel_display.c |  4 ++
>  drivers/gpu/drm/i915/display/intel_dp_mst.c  | 61 ++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_dp_mst.h  |  4 ++
>  3 files changed, 69 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 166662ade593c..38106cf63b3b9 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5936,6 +5936,10 @@ int intel_modeset_all_pipes(struct intel_atomic_state *state,
>  		if (ret)
>  			return ret;
>  
> +		ret = intel_dp_mst_add_topology_state_for_crtc(state, crtc);
> +		if (ret)
> +			return ret;
> +
>  		ret = intel_atomic_add_affected_planes(state, crtc);
>  		if (ret)
>  			return ret;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 8b0e4defa3f10..f3cb12dcfe0a7 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -1223,3 +1223,64 @@ bool intel_dp_mst_is_slave_trans(const struct intel_crtc_state *crtc_state)
>  	return crtc_state->mst_master_transcoder != INVALID_TRANSCODER &&
>  	       crtc_state->mst_master_transcoder != crtc_state->cpu_transcoder;
>  }
> +
> +/**
> + * intel_dp_mst_add_topology_state_for_connector - add MST topology state for a connector
> + * @state: atomic state
> + * @connector: connector to add the state for
> + * @crtc: the CRTC @connector is attached to
> + *
> + * Add the MST topology state for @connector to @state.
> + *
> + * Returns 0 on success, negative error code on failure.
> + */
> +static int
> +intel_dp_mst_add_topology_state_for_connector(struct intel_atomic_state *state,
> +					      struct intel_connector *connector,
> +					      struct intel_crtc *crtc)
> +{
> +	struct drm_dp_mst_topology_state *mst_state;
> +
> +	if (!connector->mst_port)
> +		return 0;
> +
> +	mst_state = drm_atomic_get_mst_topology_state(&state->base,
> +						      &connector->mst_port->mst_mgr);
> +	if (IS_ERR(mst_state))
> +		return PTR_ERR(mst_state);
> +
> +	mst_state->pending_crtc_mask |= drm_crtc_mask(&crtc->base);
> +
> +	return 0;
> +}
> +
> +/**
> + * intel_dp_mst_add_topology_state_for_crtc - add MST topology state for a CRTC
> + * @state: atomic state
> + * @crtc: CRTC to add the state for
> + *
> + * Add the MST topology state for @crtc to @state.
> + *
> + * Returns 0 on success, negative error code on failure.
> + */
> +int intel_dp_mst_add_topology_state_for_crtc(struct intel_atomic_state *state,
> +					     struct intel_crtc *crtc)
> +{
> +	struct drm_connector *_connector;
> +	struct drm_connector_state *conn_state;
> +	int i;
> +
> +	for_each_new_connector_in_state(&state->base, _connector, conn_state, i) {
> +		struct intel_connector *connector = to_intel_connector(_connector);
> +		int ret;
> +
> +		if (conn_state->crtc != &crtc->base)
> +			continue;
> +
> +		ret = intel_dp_mst_add_topology_state_for_connector(state, connector, crtc);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.h b/drivers/gpu/drm/i915/display/intel_dp_mst.h
> index f7301de6cdfb3..f1815bb722672 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.h
> @@ -8,6 +8,8 @@
>  
>  #include <linux/types.h>
>  
> +struct intel_atomic_state;
> +struct intel_crtc;
>  struct intel_crtc_state;
>  struct intel_digital_port;
>  struct intel_dp;
> @@ -18,5 +20,7 @@ int intel_dp_mst_encoder_active_links(struct intel_digital_port *dig_port);
>  bool intel_dp_mst_is_master_trans(const struct intel_crtc_state *crtc_state);
>  bool intel_dp_mst_is_slave_trans(const struct intel_crtc_state *crtc_state);
>  bool intel_dp_mst_source_support(struct intel_dp *intel_dp);
> +int intel_dp_mst_add_topology_state_for_crtc(struct intel_atomic_state *state,
> +					     struct intel_crtc *crtc);
>  
>  #endif /* __INTEL_DP_MST_H__ */
> -- 
> 2.37.1
> 

[Cc: drm-misc folks] Re: [Intel-gfx] [CI 1/4] drm/i915/dp_mst: Add the MST topology state for modesetted CRTCs

[Lyude Paul]

On Tue, 2023-02-07 at 14:59 +0200, Imre Deak wrote:
> Hi all,
> 
> On Mon, Feb 06, 2023 at 01:48:53PM +0200, Imre Deak wrote:
> > Add the MST topology for a CRTC to the atomic state if the driver
> > needs to force a modeset on the CRTC after the encoder compute config
> > functions are called.
> > 
> > Later the MST encoder's disable hook also adds the state, but that isn't
> > guaranteed to work (since in that hook getting the state may fail, which
> > can't be handled there). This should fix that, while a later patch fixes
> > the use of the MST state in the disable hook.
> > 
> > v2: Add missing forward struct declartions, caught by hdrtest.
> > v3: Factor out intel_dp_mst_add_topology_state_for_connector() used
> >     later in the patchset.
> > 
> > Cc: Lyude Paul <lyude@redhat.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: stable@vger.kernel.org # 6.1
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> # v2
> > Reviewed-by: Lyude Paul <lyude@redhat.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> 
> Is it ok to merge these 4 patches (also at [1]), via the i915 tree?
> 
> If so could it be also acked from the AMD and Nouveau side?

Whichever branch works best for y'all is fine by me, if it's via i915's tree I
guess we might need to back-merge drm-misc at some point so I can write up
equivalent fixes for nouveau as well.

(Added Thomas Zimmermann to Cc)

> 
> [1] https://patchwork.freedesktop.org/series/113703/
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c |  4 ++
> >  drivers/gpu/drm/i915/display/intel_dp_mst.c  | 61 ++++++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_dp_mst.h  |  4 ++
> >  3 files changed, 69 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 166662ade593c..38106cf63b3b9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -5936,6 +5936,10 @@ int intel_modeset_all_pipes(struct intel_atomic_state *state,
> >  		if (ret)
> >  			return ret;
> >  
> > +		ret = intel_dp_mst_add_topology_state_for_crtc(state, crtc);
> > +		if (ret)
> > +			return ret;
> > +
> >  		ret = intel_atomic_add_affected_planes(state, crtc);
> >  		if (ret)
> >  			return ret;
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index 8b0e4defa3f10..f3cb12dcfe0a7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -1223,3 +1223,64 @@ bool intel_dp_mst_is_slave_trans(const struct intel_crtc_state *crtc_state)
> >  	return crtc_state->mst_master_transcoder != INVALID_TRANSCODER &&
> >  	       crtc_state->mst_master_transcoder != crtc_state->cpu_transcoder;
> >  }
> > +
> > +/**
> > + * intel_dp_mst_add_topology_state_for_connector - add MST topology state for a connector
> > + * @state: atomic state
> > + * @connector: connector to add the state for
> > + * @crtc: the CRTC @connector is attached to
> > + *
> > + * Add the MST topology state for @connector to @state.
> > + *
> > + * Returns 0 on success, negative error code on failure.
> > + */
> > +static int
> > +intel_dp_mst_add_topology_state_for_connector(struct intel_atomic_state *state,
> > +					      struct intel_connector *connector,
> > +					      struct intel_crtc *crtc)
> > +{
> > +	struct drm_dp_mst_topology_state *mst_state;
> > +
> > +	if (!connector->mst_port)
> > +		return 0;
> > +
> > +	mst_state = drm_atomic_get_mst_topology_state(&state->base,
> > +						      &connector->mst_port->mst_mgr);
> > +	if (IS_ERR(mst_state))
> > +		return PTR_ERR(mst_state);
> > +
> > +	mst_state->pending_crtc_mask |= drm_crtc_mask(&crtc->base);
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * intel_dp_mst_add_topology_state_for_crtc - add MST topology state for a CRTC
> > + * @state: atomic state
> > + * @crtc: CRTC to add the state for
> > + *
> > + * Add the MST topology state for @crtc to @state.
> > + *
> > + * Returns 0 on success, negative error code on failure.
> > + */
> > +int intel_dp_mst_add_topology_state_for_crtc(struct intel_atomic_state *state,
> > +					     struct intel_crtc *crtc)
> > +{
> > +	struct drm_connector *_connector;
> > +	struct drm_connector_state *conn_state;
> > +	int i;
> > +
> > +	for_each_new_connector_in_state(&state->base, _connector, conn_state, i) {
> > +		struct intel_connector *connector = to_intel_connector(_connector);
> > +		int ret;
> > +
> > +		if (conn_state->crtc != &crtc->base)
> > +			continue;
> > +
> > +		ret = intel_dp_mst_add_topology_state_for_connector(state, connector, crtc);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.h b/drivers/gpu/drm/i915/display/intel_dp_mst.h
> > index f7301de6cdfb3..f1815bb722672 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.h
> > @@ -8,6 +8,8 @@
> >  
> >  #include <linux/types.h>
> >  
> > +struct intel_atomic_state;
> > +struct intel_crtc;
> >  struct intel_crtc_state;
> >  struct intel_digital_port;
> >  struct intel_dp;
> > @@ -18,5 +20,7 @@ int intel_dp_mst_encoder_active_links(struct intel_digital_port *dig_port);
> >  bool intel_dp_mst_is_master_trans(const struct intel_crtc_state *crtc_state);
> >  bool intel_dp_mst_is_slave_trans(const struct intel_crtc_state *crtc_state);
> >  bool intel_dp_mst_source_support(struct intel_dp *intel_dp);
> > +int intel_dp_mst_add_topology_state_for_crtc(struct intel_atomic_state *state,
> > +					     struct intel_crtc *crtc);
> >  
> >  #endif /* __INTEL_DP_MST_H__ */
> > -- 
> > 2.37.1
> > 
> 

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat

Re: [Cc: drm-misc folks] Re: [Intel-gfx] [CI 1/4] drm/i915/dp_mst: Add the MST topology state for modesetted CRTCs

[Jani Nikula]

On Thu, 09 Feb 2023, Lyude Paul <lyude@redhat.com> wrote:
> On Tue, 2023-02-07 at 14:59 +0200, Imre Deak wrote:
>> Hi all,
>> 
>> On Mon, Feb 06, 2023 at 01:48:53PM +0200, Imre Deak wrote:
>> > Add the MST topology for a CRTC to the atomic state if the driver
>> > needs to force a modeset on the CRTC after the encoder compute config
>> > functions are called.
>> > 
>> > Later the MST encoder's disable hook also adds the state, but that isn't
>> > guaranteed to work (since in that hook getting the state may fail, which
>> > can't be handled there). This should fix that, while a later patch fixes
>> > the use of the MST state in the disable hook.
>> > 
>> > v2: Add missing forward struct declartions, caught by hdrtest.
>> > v3: Factor out intel_dp_mst_add_topology_state_for_connector() used
>> >     later in the patchset.
>> > 
>> > Cc: Lyude Paul <lyude@redhat.com>
>> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > Cc: stable@vger.kernel.org # 6.1
>> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> # v2
>> > Reviewed-by: Lyude Paul <lyude@redhat.com>
>> > Signed-off-by: Imre Deak <imre.deak@intel.com>
>> 
>> Is it ok to merge these 4 patches (also at [1]), via the i915 tree?
>> 
>> If so could it be also acked from the AMD and Nouveau side?
>
> Whichever branch works best for y'all is fine by me, if it's via i915's tree I
> guess we might need to back-merge drm-misc at some point so I can write up
> equivalent fixes for nouveau as well.
>
> (Added Thomas Zimmermann to Cc)

I suggest merging the series via drm-misc-next-fixes branch, to get them
to Linus' tree in the upcoming merge window. They all apply cleanly
there. The drivers can backmerge them from drm-next in the mean time, or
wait for v6.3-rc1.

Daniel acked this (well, any -next-fixes branch) on IRC yesterday,
obviously ack from me too.

I take the above as Lyude's ack for nouveau.

Harry, Wayne, do you agree with this, ack for merging the AMD part via
drm-misc-next-fixes? (Alex suggested to get your input.)


BR,
Jani.


>
>> 
>> [1] https://patchwork.freedesktop.org/series/113703/
>> 
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_display.c |  4 ++
>> >  drivers/gpu/drm/i915/display/intel_dp_mst.c  | 61 ++++++++++++++++++++
>> >  drivers/gpu/drm/i915/display/intel_dp_mst.h  |  4 ++
>> >  3 files changed, 69 insertions(+)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> > index 166662ade593c..38106cf63b3b9 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> > @@ -5936,6 +5936,10 @@ int intel_modeset_all_pipes(struct intel_atomic_state *state,
>> >  		if (ret)
>> >  			return ret;
>> >  
>> > +		ret = intel_dp_mst_add_topology_state_for_crtc(state, crtc);
>> > +		if (ret)
>> > +			return ret;
>> > +
>> >  		ret = intel_atomic_add_affected_planes(state, crtc);
>> >  		if (ret)
>> >  			return ret;
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > index 8b0e4defa3f10..f3cb12dcfe0a7 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > @@ -1223,3 +1223,64 @@ bool intel_dp_mst_is_slave_trans(const struct intel_crtc_state *crtc_state)
>> >  	return crtc_state->mst_master_transcoder != INVALID_TRANSCODER &&
>> >  	       crtc_state->mst_master_transcoder != crtc_state->cpu_transcoder;
>> >  }
>> > +
>> > +/**
>> > + * intel_dp_mst_add_topology_state_for_connector - add MST topology state for a connector
>> > + * @state: atomic state
>> > + * @connector: connector to add the state for
>> > + * @crtc: the CRTC @connector is attached to
>> > + *
>> > + * Add the MST topology state for @connector to @state.
>> > + *
>> > + * Returns 0 on success, negative error code on failure.
>> > + */
>> > +static int
>> > +intel_dp_mst_add_topology_state_for_connector(struct intel_atomic_state *state,
>> > +					      struct intel_connector *connector,
>> > +					      struct intel_crtc *crtc)
>> > +{
>> > +	struct drm_dp_mst_topology_state *mst_state;
>> > +
>> > +	if (!connector->mst_port)
>> > +		return 0;
>> > +
>> > +	mst_state = drm_atomic_get_mst_topology_state(&state->base,
>> > +						      &connector->mst_port->mst_mgr);
>> > +	if (IS_ERR(mst_state))
>> > +		return PTR_ERR(mst_state);
>> > +
>> > +	mst_state->pending_crtc_mask |= drm_crtc_mask(&crtc->base);
>> > +
>> > +	return 0;
>> > +}
>> > +
>> > +/**
>> > + * intel_dp_mst_add_topology_state_for_crtc - add MST topology state for a CRTC
>> > + * @state: atomic state
>> > + * @crtc: CRTC to add the state for
>> > + *
>> > + * Add the MST topology state for @crtc to @state.
>> > + *
>> > + * Returns 0 on success, negative error code on failure.
>> > + */
>> > +int intel_dp_mst_add_topology_state_for_crtc(struct intel_atomic_state *state,
>> > +					     struct intel_crtc *crtc)
>> > +{
>> > +	struct drm_connector *_connector;
>> > +	struct drm_connector_state *conn_state;
>> > +	int i;
>> > +
>> > +	for_each_new_connector_in_state(&state->base, _connector, conn_state, i) {
>> > +		struct intel_connector *connector = to_intel_connector(_connector);
>> > +		int ret;
>> > +
>> > +		if (conn_state->crtc != &crtc->base)
>> > +			continue;
>> > +
>> > +		ret = intel_dp_mst_add_topology_state_for_connector(state, connector, crtc);
>> > +		if (ret)
>> > +			return ret;
>> > +	}
>> > +
>> > +	return 0;
>> > +}
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.h b/drivers/gpu/drm/i915/display/intel_dp_mst.h
>> > index f7301de6cdfb3..f1815bb722672 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.h
>> > @@ -8,6 +8,8 @@
>> >  
>> >  #include <linux/types.h>
>> >  
>> > +struct intel_atomic_state;
>> > +struct intel_crtc;
>> >  struct intel_crtc_state;
>> >  struct intel_digital_port;
>> >  struct intel_dp;
>> > @@ -18,5 +20,7 @@ int intel_dp_mst_encoder_active_links(struct intel_digital_port *dig_port);
>> >  bool intel_dp_mst_is_master_trans(const struct intel_crtc_state *crtc_state);
>> >  bool intel_dp_mst_is_slave_trans(const struct intel_crtc_state *crtc_state);
>> >  bool intel_dp_mst_source_support(struct intel_dp *intel_dp);
>> > +int intel_dp_mst_add_topology_state_for_crtc(struct intel_atomic_state *state,
>> > +					     struct intel_crtc *crtc);
>> >  
>> >  #endif /* __INTEL_DP_MST_H__ */
>> > -- 
>> > 2.37.1
>> > 
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center

RE: [Cc: drm-misc folks] Re: [Intel-gfx] [CI 1/4] drm/i915/dp_mst: Add the MST topology state for modesetted CRTCs

[Lin, Wayne]

[Public]

Add Mario for awareness.

> -----Original Message-----
> From: Jani Nikula <jani.nikula@intel.com>
> Sent: Friday, February 10, 2023 6:48 PM
> To: Lyude Paul <lyude@redhat.com>; imre.deak@intel.com; Wentland,
> Harry <Harry.Wentland@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: stable@vger.kernel.org; intel-gfx@lists.freedesktop.org; dri-
> devel@lists.freedesktop.org; Ville Syrjälä <ville.syrjala@linux.intel.com>; Ben
> Skeggs <bskeggs@redhat.com>; Lin, Wayne <Wayne.Lin@amd.com>; Karol
> Herbst <kherbst@redhat.com>; Thomas Zimmermann
> <tzimmermann@suse.de>
> Subject: Re: [Cc: drm-misc folks] Re: [Intel-gfx] [CI 1/4] drm/i915/dp_mst:
> Add the MST topology state for modesetted CRTCs
> 
> On Thu, 09 Feb 2023, Lyude Paul <lyude@redhat.com> wrote:
> > On Tue, 2023-02-07 at 14:59 +0200, Imre Deak wrote:
> >> Hi all,
> >>
> >> On Mon, Feb 06, 2023 at 01:48:53PM +0200, Imre Deak wrote:
> >> > Add the MST topology for a CRTC to the atomic state if the driver
> >> > needs to force a modeset on the CRTC after the encoder compute
> >> > config functions are called.
> >> >
> >> > Later the MST encoder's disable hook also adds the state, but that
> >> > isn't guaranteed to work (since in that hook getting the state may
> >> > fail, which can't be handled there). This should fix that, while a
> >> > later patch fixes the use of the MST state in the disable hook.
> >> >
> >> > v2: Add missing forward struct declartions, caught by hdrtest.
> >> > v3: Factor out intel_dp_mst_add_topology_state_for_connector() used
> >> >     later in the patchset.
> >> >
> >> > Cc: Lyude Paul <lyude@redhat.com>
> >> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> > Cc: stable@vger.kernel.org # 6.1
> >> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> # v2
> >> > Reviewed-by: Lyude Paul <lyude@redhat.com>
> >> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> >>
> >> Is it ok to merge these 4 patches (also at [1]), via the i915 tree?
> >>
> >> If so could it be also acked from the AMD and Nouveau side?
> >
> > Whichever branch works best for y'all is fine by me, if it's via
> > i915's tree I guess we might need to back-merge drm-misc at some point
> > so I can write up equivalent fixes for nouveau as well.
> >
> > (Added Thomas Zimmermann to Cc)
> 
> I suggest merging the series via drm-misc-next-fixes branch, to get them to
> Linus' tree in the upcoming merge window. They all apply cleanly there. The
> drivers can backmerge them from drm-next in the mean time, or wait for
> v6.3-rc1.
> 
> Daniel acked this (well, any -next-fixes branch) on IRC yesterday, obviously
> ack from me too.
> 
> I take the above as Lyude's ack for nouveau.
> 
> Harry, Wayne, do you agree with this, ack for merging the AMD part via drm-
> misc-next-fixes? (Alex suggested to get your input.)
> 

Thank you Imre, Lyude and Jani.
That looks good to me and I agree with that.

Thanks!

Regards,
Wayne
> 
> BR,
> Jani.
> 
> 
> >
> >>
> >> [1] https://patchwork.freedesktop.org/series/113703/
> >>
> >> > ---
> >> >  drivers/gpu/drm/i915/display/intel_display.c |  4 ++
> >> > drivers/gpu/drm/i915/display/intel_dp_mst.c  | 61
> >> > ++++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_dp_mst.h
> >> > |  4 ++
> >> >  3 files changed, 69 insertions(+)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> >> > b/drivers/gpu/drm/i915/display/intel_display.c
> >> > index 166662ade593c..38106cf63b3b9 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> >> > @@ -5936,6 +5936,10 @@ int intel_modeset_all_pipes(struct
> intel_atomic_state *state,
> >> >  		if (ret)
> >> >  			return ret;
> >> >
> >> > +		ret = intel_dp_mst_add_topology_state_for_crtc(state, crtc);
> >> > +		if (ret)
> >> > +			return ret;
> >> > +
> >> >  		ret = intel_atomic_add_affected_planes(state, crtc);
> >> >  		if (ret)
> >> >  			return ret;
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> >> > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> >> > index 8b0e4defa3f10..f3cb12dcfe0a7 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> >> > @@ -1223,3 +1223,64 @@ bool intel_dp_mst_is_slave_trans(const
> struct intel_crtc_state *crtc_state)
> >> >  	return crtc_state->mst_master_transcoder !=
> INVALID_TRANSCODER &&
> >> >  	       crtc_state->mst_master_transcoder !=
> >> > crtc_state->cpu_transcoder;  }
> >> > +
> >> > +/**
> >> > + * intel_dp_mst_add_topology_state_for_connector - add MST
> >> > +topology state for a connector
> >> > + * @state: atomic state
> >> > + * @connector: connector to add the state for
> >> > + * @crtc: the CRTC @connector is attached to
> >> > + *
> >> > + * Add the MST topology state for @connector to @state.
> >> > + *
> >> > + * Returns 0 on success, negative error code on failure.
> >> > + */
> >> > +static int
> >> > +intel_dp_mst_add_topology_state_for_connector(struct
> intel_atomic_state *state,
> >> > +					      struct intel_connector *connector,
> >> > +					      struct intel_crtc *crtc) {
> >> > +	struct drm_dp_mst_topology_state *mst_state;
> >> > +
> >> > +	if (!connector->mst_port)
> >> > +		return 0;
> >> > +
> >> > +	mst_state = drm_atomic_get_mst_topology_state(&state->base,
> >> > +						      &connector->mst_port-
> >mst_mgr);
> >> > +	if (IS_ERR(mst_state))
> >> > +		return PTR_ERR(mst_state);
> >> > +
> >> > +	mst_state->pending_crtc_mask |= drm_crtc_mask(&crtc->base);
> >> > +
> >> > +	return 0;
> >> > +}
> >> > +
> >> > +/**
> >> > + * intel_dp_mst_add_topology_state_for_crtc - add MST topology
> >> > +state for a CRTC
> >> > + * @state: atomic state
> >> > + * @crtc: CRTC to add the state for
> >> > + *
> >> > + * Add the MST topology state for @crtc to @state.
> >> > + *
> >> > + * Returns 0 on success, negative error code on failure.
> >> > + */
> >> > +int intel_dp_mst_add_topology_state_for_crtc(struct
> intel_atomic_state *state,
> >> > +					     struct intel_crtc *crtc) {
> >> > +	struct drm_connector *_connector;
> >> > +	struct drm_connector_state *conn_state;
> >> > +	int i;
> >> > +
> >> > +	for_each_new_connector_in_state(&state->base, _connector,
> conn_state, i) {
> >> > +		struct intel_connector *connector =
> to_intel_connector(_connector);
> >> > +		int ret;
> >> > +
> >> > +		if (conn_state->crtc != &crtc->base)
> >> > +			continue;
> >> > +
> >> > +		ret =
> intel_dp_mst_add_topology_state_for_connector(state, connector, crtc);
> >> > +		if (ret)
> >> > +			return ret;
> >> > +	}
> >> > +
> >> > +	return 0;
> >> > +}
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.h
> >> > b/drivers/gpu/drm/i915/display/intel_dp_mst.h
> >> > index f7301de6cdfb3..f1815bb722672 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.h
> >> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.h
> >> > @@ -8,6 +8,8 @@
> >> >
> >> >  #include <linux/types.h>
> >> >
> >> > +struct intel_atomic_state;
> >> > +struct intel_crtc;
> >> >  struct intel_crtc_state;
> >> >  struct intel_digital_port;
> >> >  struct intel_dp;
> >> > @@ -18,5 +20,7 @@ int intel_dp_mst_encoder_active_links(struct
> >> > intel_digital_port *dig_port);  bool
> >> > intel_dp_mst_is_master_trans(const struct intel_crtc_state
> >> > *crtc_state);  bool intel_dp_mst_is_slave_trans(const struct
> >> > intel_crtc_state *crtc_state);  bool
> >> > intel_dp_mst_source_support(struct intel_dp *intel_dp);
> >> > +int intel_dp_mst_add_topology_state_for_crtc(struct
> intel_atomic_state *state,
> >> > +					     struct intel_crtc *crtc);
> >> >
> >> >  #endif /* __INTEL_DP_MST_H__ */
> >> > --
> >> > 2.37.1
> >> >
> >>
> 
> --
> Jani Nikula, Intel Open Source Graphics Center

Re: [Cc: drm-misc folks] Re: [Intel-gfx] [CI 1/4] drm/i915/dp_mst: Add the MST topology state for modesetted CRTCs

[Imre Deak]

On Mon, Feb 13, 2023 at 10:41:32AM +0000, Lin, Wayne wrote:
> [Public]
> 
> Add Mario for awareness.
> 
> > -----Original Message-----
> > From: Jani Nikula <jani.nikula@intel.com>
> > Sent: Friday, February 10, 2023 6:48 PM
> > To: Lyude Paul <lyude@redhat.com>; imre.deak@intel.com; Wentland,
> > Harry <Harry.Wentland@amd.com>; Deucher, Alexander
> > <Alexander.Deucher@amd.com>; Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: stable@vger.kernel.org; intel-gfx@lists.freedesktop.org; dri-
> > devel@lists.freedesktop.org; Ville Syrjälä <ville.syrjala@linux.intel.com>; Ben
> > Skeggs <bskeggs@redhat.com>; Lin, Wayne <Wayne.Lin@amd.com>; Karol
> > Herbst <kherbst@redhat.com>; Thomas Zimmermann
> > <tzimmermann@suse.de>
> > Subject: Re: [Cc: drm-misc folks] Re: [Intel-gfx] [CI 1/4] drm/i915/dp_mst:
> > Add the MST topology state for modesetted CRTCs
> > 
> > On Thu, 09 Feb 2023, Lyude Paul <lyude@redhat.com> wrote:
> > > On Tue, 2023-02-07 at 14:59 +0200, Imre Deak wrote:
> > >> Hi all,
> > >>
> > >> On Mon, Feb 06, 2023 at 01:48:53PM +0200, Imre Deak wrote:
> > >> > Add the MST topology for a CRTC to the atomic state if the driver
> > >> > needs to force a modeset on the CRTC after the encoder compute
> > >> > config functions are called.
> > >> >
> > >> > Later the MST encoder's disable hook also adds the state, but that
> > >> > isn't guaranteed to work (since in that hook getting the state may
> > >> > fail, which can't be handled there). This should fix that, while a
> > >> > later patch fixes the use of the MST state in the disable hook.
> > >> >
> > >> > v2: Add missing forward struct declartions, caught by hdrtest.
> > >> > v3: Factor out intel_dp_mst_add_topology_state_for_connector() used
> > >> >     later in the patchset.
> > >> >
> > >> > Cc: Lyude Paul <lyude@redhat.com>
> > >> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >> > Cc: stable@vger.kernel.org # 6.1
> > >> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> # v2
> > >> > Reviewed-by: Lyude Paul <lyude@redhat.com>
> > >> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > >>
> > >> Is it ok to merge these 4 patches (also at [1]), via the i915 tree?
> > >>
> > >> If so could it be also acked from the AMD and Nouveau side?
> > >
> > > Whichever branch works best for y'all is fine by me, if it's via
> > > i915's tree I guess we might need to back-merge drm-misc at some point
> > > so I can write up equivalent fixes for nouveau as well.
> > >
> > > (Added Thomas Zimmermann to Cc)
> > 
> > I suggest merging the series via drm-misc-next-fixes branch, to get them to
> > Linus' tree in the upcoming merge window. They all apply cleanly there. The
> > drivers can backmerge them from drm-next in the mean time, or wait for
> > v6.3-rc1.
> > 
> > Daniel acked this (well, any -next-fixes branch) on IRC yesterday, obviously
> > ack from me too.
> > 
> > I take the above as Lyude's ack for nouveau.
> > 
> > Harry, Wayne, do you agree with this, ack for merging the AMD part via drm-
> > misc-next-fixes? (Alex suggested to get your input.)
> 
> Thank you Imre, Lyude and Jani.
> That looks good to me and I agree with that.

Ok, thanks all, pushed the patchset to drm-misc-next-fixes.

> Thanks!
> 
> Regards,
> Wayne
> > 
> > BR,
> > Jani.
> > 
> > 
> > >
> > >>
> > >> [1] https://patchwork.freedesktop.org/series/113703/
> > >>
> > >> > ---
> > >> >  drivers/gpu/drm/i915/display/intel_display.c |  4 ++
> > >> > drivers/gpu/drm/i915/display/intel_dp_mst.c  | 61
> > >> > ++++++++++++++++++++
> > drivers/gpu/drm/i915/display/intel_dp_mst.h
> > >> > |  4 ++
> > >> >  3 files changed, 69 insertions(+)
> > >> >
> > >> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > >> > b/drivers/gpu/drm/i915/display/intel_display.c
> > >> > index 166662ade593c..38106cf63b3b9 100644
> > >> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > >> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > >> > @@ -5936,6 +5936,10 @@ int intel_modeset_all_pipes(struct
> > intel_atomic_state *state,
> > >> >  		if (ret)
> > >> >  			return ret;
> > >> >
> > >> > +		ret = intel_dp_mst_add_topology_state_for_crtc(state, crtc);
> > >> > +		if (ret)
> > >> > +			return ret;
> > >> > +
> > >> >  		ret = intel_atomic_add_affected_planes(state, crtc);
> > >> >  		if (ret)
> > >> >  			return ret;
> > >> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > >> > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > >> > index 8b0e4defa3f10..f3cb12dcfe0a7 100644
> > >> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > >> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > >> > @@ -1223,3 +1223,64 @@ bool intel_dp_mst_is_slave_trans(const
> > struct intel_crtc_state *crtc_state)
> > >> >  	return crtc_state->mst_master_transcoder !=
> > INVALID_TRANSCODER &&
> > >> >  	       crtc_state->mst_master_transcoder !=
> > >> > crtc_state->cpu_transcoder;  }
> > >> > +
> > >> > +/**
> > >> > + * intel_dp_mst_add_topology_state_for_connector - add MST
> > >> > +topology state for a connector
> > >> > + * @state: atomic state
> > >> > + * @connector: connector to add the state for
> > >> > + * @crtc: the CRTC @connector is attached to
> > >> > + *
> > >> > + * Add the MST topology state for @connector to @state.
> > >> > + *
> > >> > + * Returns 0 on success, negative error code on failure.
> > >> > + */
> > >> > +static int
> > >> > +intel_dp_mst_add_topology_state_for_connector(struct
> > intel_atomic_state *state,
> > >> > +					      struct intel_connector *connector,
> > >> > +					      struct intel_crtc *crtc) {
> > >> > +	struct drm_dp_mst_topology_state *mst_state;
> > >> > +
> > >> > +	if (!connector->mst_port)
> > >> > +		return 0;
> > >> > +
> > >> > +	mst_state = drm_atomic_get_mst_topology_state(&state->base,
> > >> > +						      &connector->mst_port-
> > >mst_mgr);
> > >> > +	if (IS_ERR(mst_state))
> > >> > +		return PTR_ERR(mst_state);
> > >> > +
> > >> > +	mst_state->pending_crtc_mask |= drm_crtc_mask(&crtc->base);
> > >> > +
> > >> > +	return 0;
> > >> > +}
> > >> > +
> > >> > +/**
> > >> > + * intel_dp_mst_add_topology_state_for_crtc - add MST topology
> > >> > +state for a CRTC
> > >> > + * @state: atomic state
> > >> > + * @crtc: CRTC to add the state for
> > >> > + *
> > >> > + * Add the MST topology state for @crtc to @state.
> > >> > + *
> > >> > + * Returns 0 on success, negative error code on failure.
> > >> > + */
> > >> > +int intel_dp_mst_add_topology_state_for_crtc(struct
> > intel_atomic_state *state,
> > >> > +					     struct intel_crtc *crtc) {
> > >> > +	struct drm_connector *_connector;
> > >> > +	struct drm_connector_state *conn_state;
> > >> > +	int i;
> > >> > +
> > >> > +	for_each_new_connector_in_state(&state->base, _connector,
> > conn_state, i) {
> > >> > +		struct intel_connector *connector =
> > to_intel_connector(_connector);
> > >> > +		int ret;
> > >> > +
> > >> > +		if (conn_state->crtc != &crtc->base)
> > >> > +			continue;
> > >> > +
> > >> > +		ret =
> > intel_dp_mst_add_topology_state_for_connector(state, connector, crtc);
> > >> > +		if (ret)
> > >> > +			return ret;
> > >> > +	}
> > >> > +
> > >> > +	return 0;
> > >> > +}
> > >> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.h
> > >> > b/drivers/gpu/drm/i915/display/intel_dp_mst.h
> > >> > index f7301de6cdfb3..f1815bb722672 100644
> > >> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.h
> > >> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.h
> > >> > @@ -8,6 +8,8 @@
> > >> >
> > >> >  #include <linux/types.h>
> > >> >
> > >> > +struct intel_atomic_state;
> > >> > +struct intel_crtc;
> > >> >  struct intel_crtc_state;
> > >> >  struct intel_digital_port;
> > >> >  struct intel_dp;
> > >> > @@ -18,5 +20,7 @@ int intel_dp_mst_encoder_active_links(struct
> > >> > intel_digital_port *dig_port);  bool
> > >> > intel_dp_mst_is_master_trans(const struct intel_crtc_state
> > >> > *crtc_state);  bool intel_dp_mst_is_slave_trans(const struct
> > >> > intel_crtc_state *crtc_state);  bool
> > >> > intel_dp_mst_source_support(struct intel_dp *intel_dp);
> > >> > +int intel_dp_mst_add_topology_state_for_crtc(struct
> > intel_atomic_state *state,
> > >> > +					     struct intel_crtc *crtc);
> > >> >
> > >> >  #endif /* __INTEL_DP_MST_H__ */
> > >> > --
> > >> > 2.37.1
> > >> >
> > >>
> > 
> > --
> > Jani Nikula, Intel Open Source Graphics Center