[Intel-gfx] [PATCH 1/6] drm/i915/tgl+: Use the correct DP_TP_* register instances in MST encoders

[Intel-gfx] [PATCH 1/6] drm/i915/tgl+: Use the correct DP_TP_* register instances in MST encoders

[Imre Deak]

MST encoders must use the master MST transcoder's DP_TP_STATUS and
DP_TP_CONTROL registers. Atm, during the HW readout of a slave
transcoder's CRTC state we reset these register addresses in
intel_dp::regs.dp_tp_* to the slave transcoder's DP_TP_* register
addresses incorrectly; fix this.

This issue led at least to
'Timed out waiting for ACT sent when disabling'
errors during output disabling in a multiple MST stream config.

This change replaces
https://patchwork.freedesktop.org/patch/369577/?series=78193&rev=1
which just papered over the problem.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index ca7bb2294d2b..73d6cc29291a 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4193,11 +4193,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 	if (drm_WARN_ON(&dev_priv->drm, transcoder_is_dsi(cpu_transcoder)))
 		return;
 
-	if (INTEL_GEN(dev_priv) >= 12) {
-		intel_dp->regs.dp_tp_ctl = TGL_DP_TP_CTL(cpu_transcoder);
-		intel_dp->regs.dp_tp_status = TGL_DP_TP_STATUS(cpu_transcoder);
-	}
-
 	intel_dsc_get_config(encoder, pipe_config);
 
 	temp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder));
@@ -4299,6 +4294,16 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 		break;
 	}
 
+	if (INTEL_GEN(dev_priv) >= 12) {
+		enum transcoder transcoder =
+			intel_dp_mst_is_slave_trans(pipe_config) ?
+			pipe_config->mst_master_transcoder :
+			pipe_config->cpu_transcoder;
+
+		intel_dp->regs.dp_tp_ctl = TGL_DP_TP_CTL(transcoder);
+		intel_dp->regs.dp_tp_status = TGL_DP_TP_STATUS(transcoder);
+	}
+
 	pipe_config->has_audio =
 		intel_ddi_is_audio_enabled(dev_priv, cpu_transcoder);
 
-- 
2.23.1

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

[Intel-gfx] [PATCH 2/6] drm/i915/dp_mst: Disable link training fallback on MST links

[Imre Deak]

We calculate the MST available bandwidth using the link's maximum rate
and lane count. This bandwidth won't be recalculated in response to a
link training error and fallback setting, so modesets following a link
training error will calculate incorrect timing parameters (like the
transcoder's data M/N params or the number of MST TUs).

Prevent the above problem by disabling the link training fallback on MST
links for now, until the MST compute config can deal with changing link
parameters.

The misconfigured timing lead at least to a
'Timed out waiting for DP idle patterns'
error.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 42589cae766d..c585b002783a 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -468,6 +468,13 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
 	int index;
 
+	/*
+	 * TODO: Enable fallback on MST links once MST link compute can handle
+	 * the fallback params.
+	 */
+	if (intel_dp->is_mst)
+		return -1;
+
 	index = intel_dp_rate_index(intel_dp->common_rates,
 				    intel_dp->num_common_rates,
 				    link_rate);
@@ -6163,7 +6170,17 @@ intel_dp_detect(struct drm_connector *connector,
 		goto out;
 	}
 
-	if (intel_dp->reset_link_params) {
+	/* Read DP Sink DSC Cap DPCD regs for DP v1.4 */
+	if (INTEL_GEN(dev_priv) >= 11)
+		intel_dp_get_dsc_sink_cap(intel_dp);
+
+	intel_dp_configure_mst(intel_dp);
+
+	/*
+	 * TODO: Reset link params when switching to MST mode, until MST
+	 * supports link training fallback params.
+	 */
+	if (intel_dp->reset_link_params || intel_dp->is_mst) {
 		/* Initial max link lane count */
 		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
 
@@ -6175,12 +6192,6 @@ intel_dp_detect(struct drm_connector *connector,
 
 	intel_dp_print_rates(intel_dp);
 
-	/* Read DP Sink DSC Cap DPCD regs for DP v1.4 */
-	if (INTEL_GEN(dev_priv) >= 11)
-		intel_dp_get_dsc_sink_cap(intel_dp);
-
-	intel_dp_configure_mst(intel_dp);
-
 	if (intel_dp->is_mst) {
 		/*
 		 * If we are in MST mode then this connector
-- 
2.23.1

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

[Intel-gfx] [PATCH 3/6] drm/i915/dp_mst: Move clearing the ACT sent flag closer to its polling

[Imre Deak]

During transcoder enabling we'll configure the transcoder in MST mode
and enable the VC payload allocation, which will start the ACT sequence.
Before waiting for the ACT sequence completion, we need to clear the ACT
sent flag, but based on the above we can do this right before enabling
the transcoder.

For clarity, move the flag clearing closer to where we wait for it.

While at it also factor out some common code.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 36 +++++++++++++--------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 2e6c6375a23b..3977ee4f7176 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -317,6 +317,25 @@ intel_dp_mst_atomic_check(struct drm_connector *connector,
 	return ret;
 }
 
+static void clear_act_sent(struct intel_dp *intel_dp)
+{
+	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+
+	intel_de_write(i915, intel_dp->regs.dp_tp_status,
+		       intel_de_read(i915, intel_dp->regs.dp_tp_status));
+}
+
+static void wait_for_act_sent(struct intel_dp *intel_dp)
+{
+	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+
+	if (intel_de_wait_for_set(i915, intel_dp->regs.dp_tp_status,
+				  DP_TP_STATUS_ACT_SENT, 1))
+		drm_err(&i915->drm, "Timed out waiting for ACT sent\n");
+
+	drm_dp_check_act_status(&intel_dp->mst_mgr);
+}
+
 static void intel_mst_disable_dp(struct intel_atomic_state *state,
 				 struct intel_encoder *encoder,
 				 const struct intel_crtc_state *old_crtc_state,
@@ -377,11 +396,7 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
 		       TRANS_DDI_FUNC_CTL(old_crtc_state->cpu_transcoder),
 		       val);
 
-	if (intel_de_wait_for_set(dev_priv, intel_dp->regs.dp_tp_status,
-				  DP_TP_STATUS_ACT_SENT, 1))
-		drm_err(&dev_priv->drm,
-			"Timed out waiting for ACT sent when disabling\n");
-	drm_dp_check_act_status(&intel_dp->mst_mgr);
+	wait_for_act_sent(intel_dp);
 
 	drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, connector->port);
 
@@ -452,7 +467,6 @@ static void intel_mst_pre_enable_dp(struct intel_atomic_state *state,
 	struct intel_connector *connector =
 		to_intel_connector(conn_state->connector);
 	int ret;
-	u32 temp;
 	bool first_mst_stream;
 
 	/* MST encoders are bound to a crtc, not to a connector,
@@ -485,8 +499,6 @@ static void intel_mst_pre_enable_dp(struct intel_atomic_state *state,
 		drm_err(&dev_priv->drm, "failed to allocate vcpi\n");
 
 	intel_dp->active_mst_links++;
-	temp = intel_de_read(dev_priv, intel_dp->regs.dp_tp_status);
-	intel_de_write(dev_priv, intel_dp->regs.dp_tp_status, temp);
 
 	ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
 
@@ -517,16 +529,14 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
 
 	drm_WARN_ON(&dev_priv->drm, pipe_config->has_pch_encoder);
 
+	clear_act_sent(intel_dp);
+
 	intel_ddi_enable_transcoder_func(encoder, pipe_config);
 
 	drm_dbg_kms(&dev_priv->drm, "active links %d\n",
 		    intel_dp->active_mst_links);
 
-	if (intel_de_wait_for_set(dev_priv, intel_dp->regs.dp_tp_status,
-				  DP_TP_STATUS_ACT_SENT, 1))
-		drm_err(&dev_priv->drm, "Timed out waiting for ACT sent\n");
-
-	drm_dp_check_act_status(&intel_dp->mst_mgr);
+	wait_for_act_sent(intel_dp);
 
 	drm_dp_update_payload_part2(&intel_dp->mst_mgr);
 
-- 
2.23.1

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

[Intel-gfx] [PATCH 4/6] drm/i915/dp_mst: Clear only the ACT sent flag from DP_TP_STATUS

[Imre Deak]

It's not clear if the DP_TP_STATUS flags other than the ACT sent flag
have some side-effect, so don't clear those; we don't depend on the
state of these flags anyway.

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 3977ee4f7176..b66b56a070e5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -322,7 +322,7 @@ static void clear_act_sent(struct intel_dp *intel_dp)
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
 
 	intel_de_write(i915, intel_dp->regs.dp_tp_status,
-		       intel_de_read(i915, intel_dp->regs.dp_tp_status));
+		       DP_TP_STATUS_ACT_SENT);
 }
 
 static void wait_for_act_sent(struct intel_dp *intel_dp)
-- 
2.23.1

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

[Intel-gfx] [PATCH 5/6] drm/i915/dp_mst: Clear the ACT sent flag during encoder disabling too

[Imre Deak]

During encoder enabling we clear the flag before starting the ACT
sequence and wait for the flag, but the clearing is missing during
encoder disabling, add it there too. Since nothing cleared the flag
automatically we could've run subsequent disabling steps too early.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index b66b56a070e5..9308b5920780 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -389,6 +389,8 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
 
 	drm_dp_update_payload_part2(&intel_dp->mst_mgr);
 
+	clear_act_sent(intel_dp);
+
 	val = intel_de_read(dev_priv,
 			    TRANS_DDI_FUNC_CTL(old_crtc_state->cpu_transcoder));
 	val &= ~TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
-- 
2.23.1

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

[PATCH 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

[Imre Deak]

Atm, we clear the ACT sent flag in the sink's DPCD before updating the
sink's payload table, along clearing the payload table updated flag.
The sink is supposed to set this flag once it detects that the source
has completed the ACT sequence (after detecting the 4 required ACT MTPH
symbols sent by the source). As opposed to this 2 DELL monitors I have
set the flag already along the payload table updated flag, which is not
quite correct.

To be sure that the sink has detected the ACT MTPH symbols before
continuing enabling the encoder, clear the ACT sent flag before enabling
or disabling the transcoder VC payload allocation (which is what starts
the ACT sequence).

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

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index b2f5a84b4cfb..e3bf8c9c8267 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4377,6 +4377,34 @@ void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
 }
 EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
 
+/**
+ * drm_dp_clear_payload_status() - Clears the payload table status flags
+ * @mgr: manager to use
+ *
+ * Clears the payload table ACT handled and table updated flags in the MST hub's
+ * DPCD. This function must be called before updating the payload table or
+ * starting the ACT sequence and waiting for the corresponding flags to get
+ * set by the hub.
+ *
+ * Returns:
+ * 0 if the flag got cleared successfully, otherwise a negative error code.
+ */
+int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
+{
+	int ret;
+
+	ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
+				 DP_PAYLOAD_ACT_HANDLED);
+	if (ret < 0) {
+		DRM_DEBUG_DRIVER("Can't clear the ACT sent flag (%d)\n", ret);
+		return ret;
+	}
+	WARN_ON(ret != 1);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_dp_clear_payload_status);
+
 static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
 				     int id, struct drm_dp_payload *payload)
 {
@@ -4384,8 +4412,7 @@ static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
 	int ret;
 	int retries = 0;
 
-	drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
-			   DP_PAYLOAD_TABLE_UPDATED);
+	drm_dp_clear_payload_status(mgr);
 
 	payload_alloc[0] = id;
 	payload_alloc[1] = payload->start_slot;
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 9308b5920780..3c4b0fb10d8b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
 
 	intel_de_write(i915, intel_dp->regs.dp_tp_status,
 		       DP_TP_STATUS_ACT_SENT);
+
+	drm_dp_clear_payload_status(&intel_dp->mst_mgr);
 }
 
 static void wait_for_act_sent(struct intel_dp *intel_dp)
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 8b9eb4db3381..2facb87624bf 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
 			   int pbn);
 
 
+int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
+
 int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
 
 
-- 
2.23.1

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

[Intel-gfx] [PATCH 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

[Imre Deak]

Atm, we clear the ACT sent flag in the sink's DPCD before updating the
sink's payload table, along clearing the payload table updated flag.
The sink is supposed to set this flag once it detects that the source
has completed the ACT sequence (after detecting the 4 required ACT MTPH
symbols sent by the source). As opposed to this 2 DELL monitors I have
set the flag already along the payload table updated flag, which is not
quite correct.

To be sure that the sink has detected the ACT MTPH symbols before
continuing enabling the encoder, clear the ACT sent flag before enabling
or disabling the transcoder VC payload allocation (which is what starts
the ACT sequence).

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

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index b2f5a84b4cfb..e3bf8c9c8267 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4377,6 +4377,34 @@ void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
 }
 EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
 
+/**
+ * drm_dp_clear_payload_status() - Clears the payload table status flags
+ * @mgr: manager to use
+ *
+ * Clears the payload table ACT handled and table updated flags in the MST hub's
+ * DPCD. This function must be called before updating the payload table or
+ * starting the ACT sequence and waiting for the corresponding flags to get
+ * set by the hub.
+ *
+ * Returns:
+ * 0 if the flag got cleared successfully, otherwise a negative error code.
+ */
+int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
+{
+	int ret;
+
+	ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
+				 DP_PAYLOAD_ACT_HANDLED);
+	if (ret < 0) {
+		DRM_DEBUG_DRIVER("Can't clear the ACT sent flag (%d)\n", ret);
+		return ret;
+	}
+	WARN_ON(ret != 1);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_dp_clear_payload_status);
+
 static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
 				     int id, struct drm_dp_payload *payload)
 {
@@ -4384,8 +4412,7 @@ static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
 	int ret;
 	int retries = 0;
 
-	drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
-			   DP_PAYLOAD_TABLE_UPDATED);
+	drm_dp_clear_payload_status(mgr);
 
 	payload_alloc[0] = id;
 	payload_alloc[1] = payload->start_slot;
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 9308b5920780..3c4b0fb10d8b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
 
 	intel_de_write(i915, intel_dp->regs.dp_tp_status,
 		       DP_TP_STATUS_ACT_SENT);
+
+	drm_dp_clear_payload_status(&intel_dp->mst_mgr);
 }
 
 static void wait_for_act_sent(struct intel_dp *intel_dp)
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 8b9eb4db3381..2facb87624bf 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
 			   int pbn);
 
 
+int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
+
 int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
 
 
-- 
2.23.1

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

Re: [Intel-gfx] [PATCH 2/6] drm/i915/dp_mst: Disable link training fallback on MST links

[Ville Syrjälä]

On Tue, Jun 16, 2020 at 05:18:51PM +0300, Imre Deak wrote:
> We calculate the MST available bandwidth using the link's maximum rate
> and lane count. This bandwidth won't be recalculated in response to a

Thw wording here is a bit ambiguousr as to who "we" is, and what exactly
"link's maximum rate and lane count". I would try to clarify a bit that
it's drm_dp_mst_topology.c who is mostly in error here by directly
interpreting the max data rate/lanes from the DPCD.

Althoguh the i915 code is not wihtout faults, as it lacks any logic
to modeset all the MST streams for this link when the link params
change (except on tgl+ where the master/slave stuff should force
all streams to do a modeset anyway).

> link training error and fallback setting, so modesets following a link
> training error will calculate incorrect timing parameters (like the
> transcoder's data M/N params or the number of MST TUs).
> 
> Prevent the above problem by disabling the link training fallback on MST
> links for now, until the MST compute config can deal with changing link
> parameters.
> 
> The misconfigured timing lead at least to a
> 'Timed out waiting for DP idle patterns'
> error.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 42589cae766d..c585b002783a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -468,6 +468,13 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
>  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>  	int index;
>  
> +	/*
> +	 * TODO: Enable fallback on MST links once MST link compute can handle
> +	 * the fallback params.
> +	 */
> +	if (intel_dp->is_mst)
> +		return -1;

Should we duplicate the drm_error() from the other path here maybe?

> +
>  	index = intel_dp_rate_index(intel_dp->common_rates,
>  				    intel_dp->num_common_rates,
>  				    link_rate);
> @@ -6163,7 +6170,17 @@ intel_dp_detect(struct drm_connector *connector,
>  		goto out;
>  	}
>  
> -	if (intel_dp->reset_link_params) {
> +	/* Read DP Sink DSC Cap DPCD regs for DP v1.4 */
> +	if (INTEL_GEN(dev_priv) >= 11)
> +		intel_dp_get_dsc_sink_cap(intel_dp);
> +
> +	intel_dp_configure_mst(intel_dp);
> +
> +	/*
> +	 * TODO: Reset link params when switching to MST mode, until MST
> +	 * supports link training fallback params.
> +	 */
> +	if (intel_dp->reset_link_params || intel_dp->is_mst) {

/me confused. Why do we need to touch this code?

>  		/* Initial max link lane count */
>  		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
>  
> @@ -6175,12 +6192,6 @@ intel_dp_detect(struct drm_connector *connector,
>  
>  	intel_dp_print_rates(intel_dp);
>  
> -	/* Read DP Sink DSC Cap DPCD regs for DP v1.4 */
> -	if (INTEL_GEN(dev_priv) >= 11)
> -		intel_dp_get_dsc_sink_cap(intel_dp);
> -
> -	intel_dp_configure_mst(intel_dp);
> -
>  	if (intel_dp->is_mst) {
>  		/*
>  		 * If we are in MST mode then this connector
> -- 
> 2.23.1

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

Re: [Intel-gfx] [PATCH 2/6] drm/i915/dp_mst: Disable link training fallback on MST links

[Imre Deak]

On Tue, Jun 16, 2020 at 06:22:51PM +0300, Ville Syrjälä wrote:
> On Tue, Jun 16, 2020 at 05:18:51PM +0300, Imre Deak wrote:
> > We calculate the MST available bandwidth using the link's maximum rate
> > and lane count. This bandwidth won't be recalculated in response to a
> 
> Thw wording here is a bit ambiguousr as to who "we" is, and what exactly
> "link's maximum rate and lane count". I would try to clarify a bit that
> it's drm_dp_mst_topology.c who is mostly in error here by directly
> interpreting the max data rate/lanes from the DPCD.
> 
> Althoguh the i915 code is not wihtout faults, as it lacks any logic
> to modeset all the MST streams for this link when the link params
> change (except on tgl+ where the master/slave stuff should force
> all streams to do a modeset anyway).
> 
> > link training error and fallback setting, so modesets following a link
> > training error will calculate incorrect timing parameters (like the
> > transcoder's data M/N params or the number of MST TUs).
> > 
> > Prevent the above problem by disabling the link training fallback on MST
> > links for now, until the MST compute config can deal with changing link
> > parameters.
> > 
> > The misconfigured timing lead at least to a
> > 'Timed out waiting for DP idle patterns'
> > error.
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 25 ++++++++++++++++++-------
> >  1 file changed, 18 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 42589cae766d..c585b002783a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -468,6 +468,13 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> >  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> >  	int index;
> >  
> > +	/*
> > +	 * TODO: Enable fallback on MST links once MST link compute can handle
> > +	 * the fallback params.
> > +	 */
> > +	if (intel_dp->is_mst)
> > +		return -1;
> 
> Should we duplicate the drm_error() from the other path here maybe?

Yes, will add it.

> 
> > +
> >  	index = intel_dp_rate_index(intel_dp->common_rates,
> >  				    intel_dp->num_common_rates,
> >  				    link_rate);
> > @@ -6163,7 +6170,17 @@ intel_dp_detect(struct drm_connector *connector,
> >  		goto out;
> >  	}
> >  
> > -	if (intel_dp->reset_link_params) {
> > +	/* Read DP Sink DSC Cap DPCD regs for DP v1.4 */
> > +	if (INTEL_GEN(dev_priv) >= 11)
> > +		intel_dp_get_dsc_sink_cap(intel_dp);
> > +
> > +	intel_dp_configure_mst(intel_dp);
> > +
> > +	/*
> > +	 * TODO: Reset link params when switching to MST mode, until MST
> > +	 * supports link training fallback params.
> > +	 */
> > +	if (intel_dp->reset_link_params || intel_dp->is_mst) {
> 
> /me confused. Why do we need to touch this code?

It's possible to switch to MST mode after the link rate/lane count got
reduced by a link training error in SST mode.

> >  		/* Initial max link lane count */
> >  		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> >  
> > @@ -6175,12 +6192,6 @@ intel_dp_detect(struct drm_connector *connector,
> >  
> >  	intel_dp_print_rates(intel_dp);
> >  
> > -	/* Read DP Sink DSC Cap DPCD regs for DP v1.4 */
> > -	if (INTEL_GEN(dev_priv) >= 11)
> > -		intel_dp_get_dsc_sink_cap(intel_dp);
> > -
> > -	intel_dp_configure_mst(intel_dp);
> > -
> >  	if (intel_dp->is_mst) {
> >  		/*
> >  		 * If we are in MST mode then this connector
> > -- 
> > 2.23.1
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/6] drm/i915/dp_mst: Disable link training fallback on MST links

[Ville Syrjälä]

On Tue, Jun 16, 2020 at 06:30:55PM +0300, Imre Deak wrote:
> On Tue, Jun 16, 2020 at 06:22:51PM +0300, Ville Syrjälä wrote:
> > On Tue, Jun 16, 2020 at 05:18:51PM +0300, Imre Deak wrote:
> > > We calculate the MST available bandwidth using the link's maximum rate
> > > and lane count. This bandwidth won't be recalculated in response to a
> > 
> > Thw wording here is a bit ambiguousr as to who "we" is, and what exactly
> > "link's maximum rate and lane count". I would try to clarify a bit that
> > it's drm_dp_mst_topology.c who is mostly in error here by directly
> > interpreting the max data rate/lanes from the DPCD.
> > 
> > Althoguh the i915 code is not wihtout faults, as it lacks any logic
> > to modeset all the MST streams for this link when the link params
> > change (except on tgl+ where the master/slave stuff should force
> > all streams to do a modeset anyway).
> > 
> > > link training error and fallback setting, so modesets following a link
> > > training error will calculate incorrect timing parameters (like the
> > > transcoder's data M/N params or the number of MST TUs).
> > > 
> > > Prevent the above problem by disabling the link training fallback on MST
> > > links for now, until the MST compute config can deal with changing link
> > > parameters.
> > > 
> > > The misconfigured timing lead at least to a
> > > 'Timed out waiting for DP idle patterns'
> > > error.
> > > 
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_dp.c | 25 ++++++++++++++++++-------
> > >  1 file changed, 18 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index 42589cae766d..c585b002783a 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -468,6 +468,13 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> > >  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > >  	int index;
> > >  
> > > +	/*
> > > +	 * TODO: Enable fallback on MST links once MST link compute can handle
> > > +	 * the fallback params.
> > > +	 */
> > > +	if (intel_dp->is_mst)
> > > +		return -1;
> > 
> > Should we duplicate the drm_error() from the other path here maybe?
> 
> Yes, will add it.
> 
> > 
> > > +
> > >  	index = intel_dp_rate_index(intel_dp->common_rates,
> > >  				    intel_dp->num_common_rates,
> > >  				    link_rate);
> > > @@ -6163,7 +6170,17 @@ intel_dp_detect(struct drm_connector *connector,
> > >  		goto out;
> > >  	}
> > >  
> > > -	if (intel_dp->reset_link_params) {
> > > +	/* Read DP Sink DSC Cap DPCD regs for DP v1.4 */
> > > +	if (INTEL_GEN(dev_priv) >= 11)
> > > +		intel_dp_get_dsc_sink_cap(intel_dp);
> > > +
> > > +	intel_dp_configure_mst(intel_dp);
> > > +
> > > +	/*
> > > +	 * TODO: Reset link params when switching to MST mode, until MST
> > > +	 * supports link training fallback params.
> > > +	 */
> > > +	if (intel_dp->reset_link_params || intel_dp->is_mst) {
> > 
> > /me confused. Why do we need to touch this code?
> 
> It's possible to switch to MST mode after the link rate/lane count got
> reduced by a link training error in SST mode.

But then we should have a long hpd and reset_link_params should be set
anyway no?

> 
> > >  		/* Initial max link lane count */
> > >  		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> > >  
> > > @@ -6175,12 +6192,6 @@ intel_dp_detect(struct drm_connector *connector,
> > >  
> > >  	intel_dp_print_rates(intel_dp);
> > >  
> > > -	/* Read DP Sink DSC Cap DPCD regs for DP v1.4 */
> > > -	if (INTEL_GEN(dev_priv) >= 11)
> > > -		intel_dp_get_dsc_sink_cap(intel_dp);
> > > -
> > > -	intel_dp_configure_mst(intel_dp);
> > > -
> > >  	if (intel_dp->is_mst) {
> > >  		/*
> > >  		 * If we are in MST mode then this connector
> > > -- 
> > > 2.23.1
> > 
> > -- 
> > Ville Syrjälä
> > Intel

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

Re: [PATCH 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

[Ville Syrjälä]

On Tue, Jun 16, 2020 at 05:18:55PM +0300, Imre Deak wrote:
> Atm, we clear the ACT sent flag in the sink's DPCD before updating the
> sink's payload table, along clearing the payload table updated flag.
> The sink is supposed to set this flag once it detects that the source
> has completed the ACT sequence (after detecting the 4 required ACT MTPH
> symbols sent by the source). As opposed to this 2 DELL monitors I have
> set the flag already along the payload table updated flag, which is not
> quite correct.
> 
> To be sure that the sink has detected the ACT MTPH symbols before
> continuing enabling the encoder, clear the ACT sent flag before enabling
> or disabling the transcoder VC payload allocation (which is what starts
> the ACT sequence).
> 
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c       | 31 +++++++++++++++++++--
>  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
>  include/drm/drm_dp_mst_helper.h             |  2 ++
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index b2f5a84b4cfb..e3bf8c9c8267 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4377,6 +4377,34 @@ void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
>  }
>  EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
>  
> +/**
> + * drm_dp_clear_payload_status() - Clears the payload table status flags
> + * @mgr: manager to use
> + *
> + * Clears the payload table ACT handled and table updated flags in the MST hub's
> + * DPCD. This function must be called before updating the payload table or
> + * starting the ACT sequence and waiting for the corresponding flags to get
> + * set by the hub.
> + *
> + * Returns:
> + * 0 if the flag got cleared successfully, otherwise a negative error code.
> + */
> +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
> +{
> +	int ret;
> +
> +	ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> +				 DP_PAYLOAD_ACT_HANDLED);
> +	if (ret < 0) {
> +		DRM_DEBUG_DRIVER("Can't clear the ACT sent flag (%d)\n", ret);
> +		return ret;
> +	}
> +	WARN_ON(ret != 1);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_dp_clear_payload_status);
> +
>  static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
>  				     int id, struct drm_dp_payload *payload)
>  {
> @@ -4384,8 +4412,7 @@ static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
>  	int ret;
>  	int retries = 0;
>  
> -	drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> -			   DP_PAYLOAD_TABLE_UPDATED);

We used to clear DP_PAYLOAD_TABLE_UPDATED but now we clear
DP_PAYLOAD_ACT_HANDLED ?

> +	drm_dp_clear_payload_status(mgr);
>  
>  	payload_alloc[0] = id;
>  	payload_alloc[1] = payload->start_slot;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 9308b5920780..3c4b0fb10d8b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
>  
>  	intel_de_write(i915, intel_dp->regs.dp_tp_status,
>  		       DP_TP_STATUS_ACT_SENT);
> +
> +	drm_dp_clear_payload_status(&intel_dp->mst_mgr);
>  }
>  
>  static void wait_for_act_sent(struct intel_dp *intel_dp)
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index 8b9eb4db3381..2facb87624bf 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
>  			   int pbn);
>  
>  
> +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
> +
>  int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
>  
>  
> -- 
> 2.23.1

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

Re: [Intel-gfx] [PATCH 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

[Ville Syrjälä]

On Tue, Jun 16, 2020 at 05:18:55PM +0300, Imre Deak wrote:
> Atm, we clear the ACT sent flag in the sink's DPCD before updating the
> sink's payload table, along clearing the payload table updated flag.
> The sink is supposed to set this flag once it detects that the source
> has completed the ACT sequence (after detecting the 4 required ACT MTPH
> symbols sent by the source). As opposed to this 2 DELL monitors I have
> set the flag already along the payload table updated flag, which is not
> quite correct.
> 
> To be sure that the sink has detected the ACT MTPH symbols before
> continuing enabling the encoder, clear the ACT sent flag before enabling
> or disabling the transcoder VC payload allocation (which is what starts
> the ACT sequence).
> 
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c       | 31 +++++++++++++++++++--
>  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
>  include/drm/drm_dp_mst_helper.h             |  2 ++
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index b2f5a84b4cfb..e3bf8c9c8267 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4377,6 +4377,34 @@ void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
>  }
>  EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
>  
> +/**
> + * drm_dp_clear_payload_status() - Clears the payload table status flags
> + * @mgr: manager to use
> + *
> + * Clears the payload table ACT handled and table updated flags in the MST hub's
> + * DPCD. This function must be called before updating the payload table or
> + * starting the ACT sequence and waiting for the corresponding flags to get
> + * set by the hub.
> + *
> + * Returns:
> + * 0 if the flag got cleared successfully, otherwise a negative error code.
> + */
> +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
> +{
> +	int ret;
> +
> +	ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> +				 DP_PAYLOAD_ACT_HANDLED);
> +	if (ret < 0) {
> +		DRM_DEBUG_DRIVER("Can't clear the ACT sent flag (%d)\n", ret);
> +		return ret;
> +	}
> +	WARN_ON(ret != 1);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_dp_clear_payload_status);
> +
>  static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
>  				     int id, struct drm_dp_payload *payload)
>  {
> @@ -4384,8 +4412,7 @@ static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
>  	int ret;
>  	int retries = 0;
>  
> -	drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> -			   DP_PAYLOAD_TABLE_UPDATED);

We used to clear DP_PAYLOAD_TABLE_UPDATED but now we clear
DP_PAYLOAD_ACT_HANDLED ?

> +	drm_dp_clear_payload_status(mgr);
>  
>  	payload_alloc[0] = id;
>  	payload_alloc[1] = payload->start_slot;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 9308b5920780..3c4b0fb10d8b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
>  
>  	intel_de_write(i915, intel_dp->regs.dp_tp_status,
>  		       DP_TP_STATUS_ACT_SENT);
> +
> +	drm_dp_clear_payload_status(&intel_dp->mst_mgr);
>  }
>  
>  static void wait_for_act_sent(struct intel_dp *intel_dp)
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index 8b9eb4db3381..2facb87624bf 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
>  			   int pbn);
>  
>  
> +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
> +
>  int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
>  
>  
> -- 
> 2.23.1

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

Re: [Intel-gfx] [PATCH 1/6] drm/i915/tgl+: Use the correct DP_TP_* register instances in MST encoders

[Ville Syrjälä]

On Tue, Jun 16, 2020 at 05:18:50PM +0300, Imre Deak wrote:
> MST encoders must use the master MST transcoder's DP_TP_STATUS and
> DP_TP_CONTROL registers. Atm, during the HW readout of a slave
> transcoder's CRTC state we reset these register addresses in
> intel_dp::regs.dp_tp_* to the slave transcoder's DP_TP_* register
> addresses incorrectly; fix this.
> 
> This issue led at least to
> 'Timed out waiting for ACT sent when disabling'
> errors during output disabling in a multiple MST stream config.
> 
> This change replaces
> https://patchwork.freedesktop.org/patch/369577/?series=78193&rev=1
> which just papered over the problem.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

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

> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index ca7bb2294d2b..73d6cc29291a 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4193,11 +4193,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>  	if (drm_WARN_ON(&dev_priv->drm, transcoder_is_dsi(cpu_transcoder)))
>  		return;
>  
> -	if (INTEL_GEN(dev_priv) >= 12) {
> -		intel_dp->regs.dp_tp_ctl = TGL_DP_TP_CTL(cpu_transcoder);
> -		intel_dp->regs.dp_tp_status = TGL_DP_TP_STATUS(cpu_transcoder);
> -	}
> -
>  	intel_dsc_get_config(encoder, pipe_config);
>  
>  	temp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder));
> @@ -4299,6 +4294,16 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>  		break;
>  	}
>  
> +	if (INTEL_GEN(dev_priv) >= 12) {
> +		enum transcoder transcoder =
> +			intel_dp_mst_is_slave_trans(pipe_config) ?
> +			pipe_config->mst_master_transcoder :
> +			pipe_config->cpu_transcoder;
> +
> +		intel_dp->regs.dp_tp_ctl = TGL_DP_TP_CTL(transcoder);
> +		intel_dp->regs.dp_tp_status = TGL_DP_TP_STATUS(transcoder);
> +	}
> +
>  	pipe_config->has_audio =
>  		intel_ddi_is_audio_enabled(dev_priv, cpu_transcoder);
>  
> -- 
> 2.23.1

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

Re: [Intel-gfx] [PATCH 3/6] drm/i915/dp_mst: Move clearing the ACT sent flag closer to its polling

[Ville Syrjälä]

On Tue, Jun 16, 2020 at 05:18:52PM +0300, Imre Deak wrote:
> During transcoder enabling we'll configure the transcoder in MST mode
> and enable the VC payload allocation, which will start the ACT sequence.
> Before waiting for the ACT sequence completion, we need to clear the ACT
> sent flag, but based on the above we can do this right before enabling
> the transcoder.
> 
> For clarity, move the flag clearing closer to where we wait for it.
> 
> While at it also factor out some common code.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

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

> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 36 +++++++++++++--------
>  1 file changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 2e6c6375a23b..3977ee4f7176 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -317,6 +317,25 @@ intel_dp_mst_atomic_check(struct drm_connector *connector,
>  	return ret;
>  }
>  
> +static void clear_act_sent(struct intel_dp *intel_dp)
> +{
> +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> +
> +	intel_de_write(i915, intel_dp->regs.dp_tp_status,
> +		       intel_de_read(i915, intel_dp->regs.dp_tp_status));
> +}
> +
> +static void wait_for_act_sent(struct intel_dp *intel_dp)
> +{
> +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> +
> +	if (intel_de_wait_for_set(i915, intel_dp->regs.dp_tp_status,
> +				  DP_TP_STATUS_ACT_SENT, 1))
> +		drm_err(&i915->drm, "Timed out waiting for ACT sent\n");
> +
> +	drm_dp_check_act_status(&intel_dp->mst_mgr);
> +}
> +
>  static void intel_mst_disable_dp(struct intel_atomic_state *state,
>  				 struct intel_encoder *encoder,
>  				 const struct intel_crtc_state *old_crtc_state,
> @@ -377,11 +396,7 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
>  		       TRANS_DDI_FUNC_CTL(old_crtc_state->cpu_transcoder),
>  		       val);
>  
> -	if (intel_de_wait_for_set(dev_priv, intel_dp->regs.dp_tp_status,
> -				  DP_TP_STATUS_ACT_SENT, 1))
> -		drm_err(&dev_priv->drm,
> -			"Timed out waiting for ACT sent when disabling\n");
> -	drm_dp_check_act_status(&intel_dp->mst_mgr);
> +	wait_for_act_sent(intel_dp);
>  
>  	drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, connector->port);
>  
> @@ -452,7 +467,6 @@ static void intel_mst_pre_enable_dp(struct intel_atomic_state *state,
>  	struct intel_connector *connector =
>  		to_intel_connector(conn_state->connector);
>  	int ret;
> -	u32 temp;
>  	bool first_mst_stream;
>  
>  	/* MST encoders are bound to a crtc, not to a connector,
> @@ -485,8 +499,6 @@ static void intel_mst_pre_enable_dp(struct intel_atomic_state *state,
>  		drm_err(&dev_priv->drm, "failed to allocate vcpi\n");
>  
>  	intel_dp->active_mst_links++;
> -	temp = intel_de_read(dev_priv, intel_dp->regs.dp_tp_status);
> -	intel_de_write(dev_priv, intel_dp->regs.dp_tp_status, temp);
>  
>  	ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
>  
> @@ -517,16 +529,14 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
>  
>  	drm_WARN_ON(&dev_priv->drm, pipe_config->has_pch_encoder);
>  
> +	clear_act_sent(intel_dp);
> +
>  	intel_ddi_enable_transcoder_func(encoder, pipe_config);
>  
>  	drm_dbg_kms(&dev_priv->drm, "active links %d\n",
>  		    intel_dp->active_mst_links);
>  
> -	if (intel_de_wait_for_set(dev_priv, intel_dp->regs.dp_tp_status,
> -				  DP_TP_STATUS_ACT_SENT, 1))
> -		drm_err(&dev_priv->drm, "Timed out waiting for ACT sent\n");
> -
> -	drm_dp_check_act_status(&intel_dp->mst_mgr);
> +	wait_for_act_sent(intel_dp);
>  
>  	drm_dp_update_payload_part2(&intel_dp->mst_mgr);
>  
> -- 
> 2.23.1

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

Re: [Intel-gfx] [PATCH 4/6] drm/i915/dp_mst: Clear only the ACT sent flag from DP_TP_STATUS

[Ville Syrjälä]

On Tue, Jun 16, 2020 at 05:18:53PM +0300, Imre Deak wrote:
> It's not clear if the DP_TP_STATUS flags other than the ACT sent flag
> have some side-effect, so don't clear those; we don't depend on the
> state of these flags anyway.
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

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

> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 3977ee4f7176..b66b56a070e5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -322,7 +322,7 @@ static void clear_act_sent(struct intel_dp *intel_dp)
>  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>  
>  	intel_de_write(i915, intel_dp->regs.dp_tp_status,
> -		       intel_de_read(i915, intel_dp->regs.dp_tp_status));
> +		       DP_TP_STATUS_ACT_SENT);
>  }
>  
>  static void wait_for_act_sent(struct intel_dp *intel_dp)
> -- 
> 2.23.1

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

Re: [Intel-gfx] [PATCH 5/6] drm/i915/dp_mst: Clear the ACT sent flag during encoder disabling too

[Ville Syrjälä]

On Tue, Jun 16, 2020 at 05:18:54PM +0300, Imre Deak wrote:
> During encoder enabling we clear the flag before starting the ACT
> sequence and wait for the flag, but the clearing is missing during
> encoder disabling, add it there too. Since nothing cleared the flag
> automatically we could've run subsequent disabling steps too early.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

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

> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index b66b56a070e5..9308b5920780 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -389,6 +389,8 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
>  
>  	drm_dp_update_payload_part2(&intel_dp->mst_mgr);
>  
> +	clear_act_sent(intel_dp);
> +
>  	val = intel_de_read(dev_priv,
>  			    TRANS_DDI_FUNC_CTL(old_crtc_state->cpu_transcoder));
>  	val &= ~TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> -- 
> 2.23.1

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

Re: [Intel-gfx] [PATCH 2/6] drm/i915/dp_mst: Disable link training fallback on MST links

[Imre Deak]

On Tue, Jun 16, 2020 at 06:39:30PM +0300, Ville Syrjälä wrote:
> On Tue, Jun 16, 2020 at 06:30:55PM +0300, Imre Deak wrote:
> > On Tue, Jun 16, 2020 at 06:22:51PM +0300, Ville Syrjälä wrote:
> > > On Tue, Jun 16, 2020 at 05:18:51PM +0300, Imre Deak wrote:
> > > > We calculate the MST available bandwidth using the link's maximum rate
> > > > and lane count. This bandwidth won't be recalculated in response to a
> > > 
> > > Thw wording here is a bit ambiguousr as to who "we" is, and what exactly
> > > "link's maximum rate and lane count". I would try to clarify a bit that
> > > it's drm_dp_mst_topology.c who is mostly in error here by directly
> > > interpreting the max data rate/lanes from the DPCD.
> > > 
> > > Althoguh the i915 code is not wihtout faults, as it lacks any logic
> > > to modeset all the MST streams for this link when the link params
> > > change (except on tgl+ where the master/slave stuff should force
> > > all streams to do a modeset anyway).
> > > 
> > > > link training error and fallback setting, so modesets following a link
> > > > training error will calculate incorrect timing parameters (like the
> > > > transcoder's data M/N params or the number of MST TUs).
> > > > 
> > > > Prevent the above problem by disabling the link training fallback on MST
> > > > links for now, until the MST compute config can deal with changing link
> > > > parameters.
> > > > 
> > > > The misconfigured timing lead at least to a
> > > > 'Timed out waiting for DP idle patterns'
> > > > error.
> > > > 
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_dp.c | 25 ++++++++++++++++++-------
> > > >  1 file changed, 18 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > index 42589cae766d..c585b002783a 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > @@ -468,6 +468,13 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> > > >  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > > >  	int index;
> > > >  
> > > > +	/*
> > > > +	 * TODO: Enable fallback on MST links once MST link compute can handle
> > > > +	 * the fallback params.
> > > > +	 */
> > > > +	if (intel_dp->is_mst)
> > > > +		return -1;
> > > 
> > > Should we duplicate the drm_error() from the other path here maybe?
> > 
> > Yes, will add it.
> > 
> > > 
> > > > +
> > > >  	index = intel_dp_rate_index(intel_dp->common_rates,
> > > >  				    intel_dp->num_common_rates,
> > > >  				    link_rate);
> > > > @@ -6163,7 +6170,17 @@ intel_dp_detect(struct drm_connector *connector,
> > > >  		goto out;
> > > >  	}
> > > >  
> > > > -	if (intel_dp->reset_link_params) {
> > > > +	/* Read DP Sink DSC Cap DPCD regs for DP v1.4 */
> > > > +	if (INTEL_GEN(dev_priv) >= 11)
> > > > +		intel_dp_get_dsc_sink_cap(intel_dp);
> > > > +
> > > > +	intel_dp_configure_mst(intel_dp);
> > > > +
> > > > +	/*
> > > > +	 * TODO: Reset link params when switching to MST mode, until MST
> > > > +	 * supports link training fallback params.
> > > > +	 */
> > > > +	if (intel_dp->reset_link_params || intel_dp->is_mst) {
> > > 
> > > /me confused. Why do we need to touch this code?
> > 
> > It's possible to switch to MST mode after the link rate/lane count got
> > reduced by a link training error in SST mode.
> 
> But then we should have a long hpd and reset_link_params should be set
> anyway no?

I meant switching the mode for the same sink as it would change its
DP_MST_CAP. I'm not sure if a long HPD is required in that case. Also if
we had a long HPD after a mode change couldn't a modeset run before the
next intel_dp_detect() call could reset the link params?

> > 
> > > >  		/* Initial max link lane count */
> > > >  		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> > > >  
> > > > @@ -6175,12 +6192,6 @@ intel_dp_detect(struct drm_connector *connector,
> > > >  
> > > >  	intel_dp_print_rates(intel_dp);
> > > >  
> > > > -	/* Read DP Sink DSC Cap DPCD regs for DP v1.4 */
> > > > -	if (INTEL_GEN(dev_priv) >= 11)
> > > > -		intel_dp_get_dsc_sink_cap(intel_dp);
> > > > -
> > > > -	intel_dp_configure_mst(intel_dp);
> > > > -
> > > >  	if (intel_dp->is_mst) {
> > > >  		/*
> > > >  		 * If we are in MST mode then this connector
> > > > -- 
> > > > 2.23.1
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

[Imre Deak]

On Tue, Jun 16, 2020 at 06:45:46PM +0300, Ville Syrjälä wrote:
> On Tue, Jun 16, 2020 at 05:18:55PM +0300, Imre Deak wrote:
> > Atm, we clear the ACT sent flag in the sink's DPCD before updating the
> > sink's payload table, along clearing the payload table updated flag.
> > The sink is supposed to set this flag once it detects that the source
> > has completed the ACT sequence (after detecting the 4 required ACT MTPH
> > symbols sent by the source). As opposed to this 2 DELL monitors I have
> > set the flag already along the payload table updated flag, which is not
> > quite correct.
> > 
> > To be sure that the sink has detected the ACT MTPH symbols before
> > continuing enabling the encoder, clear the ACT sent flag before enabling
> > or disabling the transcoder VC payload allocation (which is what starts
> > the ACT sequence).
> > 
> > Cc: Lyude Paul <lyude@redhat.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c       | 31 +++++++++++++++++++--
> >  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
> >  include/drm/drm_dp_mst_helper.h             |  2 ++
> >  3 files changed, 33 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index b2f5a84b4cfb..e3bf8c9c8267 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -4377,6 +4377,34 @@ void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
> >  }
> >  EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
> >  
> > +/**
> > + * drm_dp_clear_payload_status() - Clears the payload table status flags
> > + * @mgr: manager to use
> > + *
> > + * Clears the payload table ACT handled and table updated flags in the MST hub's
> > + * DPCD. This function must be called before updating the payload table or
> > + * starting the ACT sequence and waiting for the corresponding flags to get
> > + * set by the hub.
> > + *
> > + * Returns:
> > + * 0 if the flag got cleared successfully, otherwise a negative error code.
> > + */
> > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
> > +{
> > +	int ret;
> > +
> > +	ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > +				 DP_PAYLOAD_ACT_HANDLED);
> > +	if (ret < 0) {
> > +		DRM_DEBUG_DRIVER("Can't clear the ACT sent flag (%d)\n", ret);
> > +		return ret;
> > +	}
> > +	WARN_ON(ret != 1);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(drm_dp_clear_payload_status);
> > +
> >  static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
> >  				     int id, struct drm_dp_payload *payload)
> >  {
> > @@ -4384,8 +4412,7 @@ static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
> >  	int ret;
> >  	int retries = 0;
> >  
> > -	drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > -			   DP_PAYLOAD_TABLE_UPDATED);
> 
> We used to clear DP_PAYLOAD_TABLE_UPDATED but now we clear
> DP_PAYLOAD_ACT_HANDLED ?

Eek. We should write DP_PAYLOAD_TABLE_UPDATED which is the only way to
clear both the act-handled and the table-updated flags. I tested things
that way but managed to send an old version. Thanks for catching it.

> 
> > +	drm_dp_clear_payload_status(mgr);
> >  
> >  	payload_alloc[0] = id;
> >  	payload_alloc[1] = payload->start_slot;
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index 9308b5920780..3c4b0fb10d8b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
> >  
> >  	intel_de_write(i915, intel_dp->regs.dp_tp_status,
> >  		       DP_TP_STATUS_ACT_SENT);
> > +
> > +	drm_dp_clear_payload_status(&intel_dp->mst_mgr);
> >  }
> >  
> >  static void wait_for_act_sent(struct intel_dp *intel_dp)
> > diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> > index 8b9eb4db3381..2facb87624bf 100644
> > --- a/include/drm/drm_dp_mst_helper.h
> > +++ b/include/drm/drm_dp_mst_helper.h
> > @@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
> >  			   int pbn);
> >  
> >  
> > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
> > +
> >  int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
> >  
> >  
> > -- 
> > 2.23.1
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

[Imre Deak]

On Tue, Jun 16, 2020 at 06:45:46PM +0300, Ville Syrjälä wrote:
> On Tue, Jun 16, 2020 at 05:18:55PM +0300, Imre Deak wrote:
> > Atm, we clear the ACT sent flag in the sink's DPCD before updating the
> > sink's payload table, along clearing the payload table updated flag.
> > The sink is supposed to set this flag once it detects that the source
> > has completed the ACT sequence (after detecting the 4 required ACT MTPH
> > symbols sent by the source). As opposed to this 2 DELL monitors I have
> > set the flag already along the payload table updated flag, which is not
> > quite correct.
> > 
> > To be sure that the sink has detected the ACT MTPH symbols before
> > continuing enabling the encoder, clear the ACT sent flag before enabling
> > or disabling the transcoder VC payload allocation (which is what starts
> > the ACT sequence).
> > 
> > Cc: Lyude Paul <lyude@redhat.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c       | 31 +++++++++++++++++++--
> >  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
> >  include/drm/drm_dp_mst_helper.h             |  2 ++
> >  3 files changed, 33 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index b2f5a84b4cfb..e3bf8c9c8267 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -4377,6 +4377,34 @@ void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
> >  }
> >  EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
> >  
> > +/**
> > + * drm_dp_clear_payload_status() - Clears the payload table status flags
> > + * @mgr: manager to use
> > + *
> > + * Clears the payload table ACT handled and table updated flags in the MST hub's
> > + * DPCD. This function must be called before updating the payload table or
> > + * starting the ACT sequence and waiting for the corresponding flags to get
> > + * set by the hub.
> > + *
> > + * Returns:
> > + * 0 if the flag got cleared successfully, otherwise a negative error code.
> > + */
> > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
> > +{
> > +	int ret;
> > +
> > +	ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > +				 DP_PAYLOAD_ACT_HANDLED);
> > +	if (ret < 0) {
> > +		DRM_DEBUG_DRIVER("Can't clear the ACT sent flag (%d)\n", ret);
> > +		return ret;
> > +	}
> > +	WARN_ON(ret != 1);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(drm_dp_clear_payload_status);
> > +
> >  static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
> >  				     int id, struct drm_dp_payload *payload)
> >  {
> > @@ -4384,8 +4412,7 @@ static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
> >  	int ret;
> >  	int retries = 0;
> >  
> > -	drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > -			   DP_PAYLOAD_TABLE_UPDATED);
> 
> We used to clear DP_PAYLOAD_TABLE_UPDATED but now we clear
> DP_PAYLOAD_ACT_HANDLED ?

Eek. We should write DP_PAYLOAD_TABLE_UPDATED which is the only way to
clear both the act-handled and the table-updated flags. I tested things
that way but managed to send an old version. Thanks for catching it.

> 
> > +	drm_dp_clear_payload_status(mgr);
> >  
> >  	payload_alloc[0] = id;
> >  	payload_alloc[1] = payload->start_slot;
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index 9308b5920780..3c4b0fb10d8b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
> >  
> >  	intel_de_write(i915, intel_dp->regs.dp_tp_status,
> >  		       DP_TP_STATUS_ACT_SENT);
> > +
> > +	drm_dp_clear_payload_status(&intel_dp->mst_mgr);
> >  }
> >  
> >  static void wait_for_act_sent(struct intel_dp *intel_dp)
> > diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> > index 8b9eb4db3381..2facb87624bf 100644
> > --- a/include/drm/drm_dp_mst_helper.h
> > +++ b/include/drm/drm_dp_mst_helper.h
> > @@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
> >  			   int pbn);
> >  
> >  
> > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
> > +
> >  int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
> >  
> >  
> > -- 
> > 2.23.1
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/6] drm/i915/dp_mst: Disable link training fallback on MST links

[Ville Syrjälä]

On Tue, Jun 16, 2020 at 06:49:20PM +0300, Imre Deak wrote:
> On Tue, Jun 16, 2020 at 06:39:30PM +0300, Ville Syrjälä wrote:
> > On Tue, Jun 16, 2020 at 06:30:55PM +0300, Imre Deak wrote:
> > > On Tue, Jun 16, 2020 at 06:22:51PM +0300, Ville Syrjälä wrote:
> > > > On Tue, Jun 16, 2020 at 05:18:51PM +0300, Imre Deak wrote:
> > > > > We calculate the MST available bandwidth using the link's maximum rate
> > > > > and lane count. This bandwidth won't be recalculated in response to a
> > > > 
> > > > Thw wording here is a bit ambiguousr as to who "we" is, and what exactly
> > > > "link's maximum rate and lane count". I would try to clarify a bit that
> > > > it's drm_dp_mst_topology.c who is mostly in error here by directly
> > > > interpreting the max data rate/lanes from the DPCD.
> > > > 
> > > > Althoguh the i915 code is not wihtout faults, as it lacks any logic
> > > > to modeset all the MST streams for this link when the link params
> > > > change (except on tgl+ where the master/slave stuff should force
> > > > all streams to do a modeset anyway).
> > > > 
> > > > > link training error and fallback setting, so modesets following a link
> > > > > training error will calculate incorrect timing parameters (like the
> > > > > transcoder's data M/N params or the number of MST TUs).
> > > > > 
> > > > > Prevent the above problem by disabling the link training fallback on MST
> > > > > links for now, until the MST compute config can deal with changing link
> > > > > parameters.
> > > > > 
> > > > > The misconfigured timing lead at least to a
> > > > > 'Timed out waiting for DP idle patterns'
> > > > > error.
> > > > > 
> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/display/intel_dp.c | 25 ++++++++++++++++++-------
> > > > >  1 file changed, 18 insertions(+), 7 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > index 42589cae766d..c585b002783a 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > @@ -468,6 +468,13 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> > > > >  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > > > >  	int index;
> > > > >  
> > > > > +	/*
> > > > > +	 * TODO: Enable fallback on MST links once MST link compute can handle
> > > > > +	 * the fallback params.
> > > > > +	 */
> > > > > +	if (intel_dp->is_mst)
> > > > > +		return -1;
> > > > 
> > > > Should we duplicate the drm_error() from the other path here maybe?
> > > 
> > > Yes, will add it.
> > > 
> > > > 
> > > > > +
> > > > >  	index = intel_dp_rate_index(intel_dp->common_rates,
> > > > >  				    intel_dp->num_common_rates,
> > > > >  				    link_rate);
> > > > > @@ -6163,7 +6170,17 @@ intel_dp_detect(struct drm_connector *connector,
> > > > >  		goto out;
> > > > >  	}
> > > > >  
> > > > > -	if (intel_dp->reset_link_params) {
> > > > > +	/* Read DP Sink DSC Cap DPCD regs for DP v1.4 */
> > > > > +	if (INTEL_GEN(dev_priv) >= 11)
> > > > > +		intel_dp_get_dsc_sink_cap(intel_dp);
> > > > > +
> > > > > +	intel_dp_configure_mst(intel_dp);
> > > > > +
> > > > > +	/*
> > > > > +	 * TODO: Reset link params when switching to MST mode, until MST
> > > > > +	 * supports link training fallback params.
> > > > > +	 */
> > > > > +	if (intel_dp->reset_link_params || intel_dp->is_mst) {
> > > > 
> > > > /me confused. Why do we need to touch this code?
> > > 
> > > It's possible to switch to MST mode after the link rate/lane count got
> > > reduced by a link training error in SST mode.
> > 
> > But then we should have a long hpd and reset_link_params should be set
> > anyway no?
> 
> I meant switching the mode for the same sink as it would change its
> DP_MST_CAP. I'm not sure if a long HPD is required in that case. 

I would expect so. I don't think there's a requirement in the spec to
re-evaluate this sort of stuff for a short hpd.

> Also if
> we had a long HPD after a mode change couldn't a modeset run before the
> next intel_dp_detect() call could reset the link params?

This is detect() we're talking about here, so I guess you mean
detect() running before the hpd gets to flag reset_link_params=true ?
Not sure I'd worry about that, but I guess there should be no harm
in reordering these things a bit.

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

> 
> > > 
> > > > >  		/* Initial max link lane count */
> > > > >  		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> > > > >  
> > > > > @@ -6175,12 +6192,6 @@ intel_dp_detect(struct drm_connector *connector,
> > > > >  
> > > > >  	intel_dp_print_rates(intel_dp);
> > > > >  
> > > > > -	/* Read DP Sink DSC Cap DPCD regs for DP v1.4 */
> > > > > -	if (INTEL_GEN(dev_priv) >= 11)
> > > > > -		intel_dp_get_dsc_sink_cap(intel_dp);
> > > > > -
> > > > > -	intel_dp_configure_mst(intel_dp);
> > > > > -
> > > > >  	if (intel_dp->is_mst) {
> > > > >  		/*
> > > > >  		 * If we are in MST mode then this connector
> > > > > -- 
> > > > > 2.23.1
> > > > 
> > > > -- 
> > > > Ville Syrjälä
> > > > Intel
> > 
> > -- 
> > Ville Syrjälä
> > Intel

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

Re: [PATCH 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

[Ville Syrjälä]

On Tue, Jun 16, 2020 at 06:54:41PM +0300, Imre Deak wrote:
> On Tue, Jun 16, 2020 at 06:45:46PM +0300, Ville Syrjälä wrote:
> > On Tue, Jun 16, 2020 at 05:18:55PM +0300, Imre Deak wrote:
> > > Atm, we clear the ACT sent flag in the sink's DPCD before updating the
> > > sink's payload table, along clearing the payload table updated flag.
> > > The sink is supposed to set this flag once it detects that the source
> > > has completed the ACT sequence (after detecting the 4 required ACT MTPH
> > > symbols sent by the source). As opposed to this 2 DELL monitors I have
> > > set the flag already along the payload table updated flag, which is not
> > > quite correct.
> > > 
> > > To be sure that the sink has detected the ACT MTPH symbols before
> > > continuing enabling the encoder, clear the ACT sent flag before enabling
> > > or disabling the transcoder VC payload allocation (which is what starts
> > > the ACT sequence).
> > > 
> > > Cc: Lyude Paul <lyude@redhat.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: dri-devel@lists.freedesktop.org
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_dp_mst_topology.c       | 31 +++++++++++++++++++--
> > >  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
> > >  include/drm/drm_dp_mst_helper.h             |  2 ++
> > >  3 files changed, 33 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > index b2f5a84b4cfb..e3bf8c9c8267 100644
> > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > @@ -4377,6 +4377,34 @@ void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
> > >  }
> > >  EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
> > >  
> > > +/**
> > > + * drm_dp_clear_payload_status() - Clears the payload table status flags
> > > + * @mgr: manager to use
> > > + *
> > > + * Clears the payload table ACT handled and table updated flags in the MST hub's
> > > + * DPCD. This function must be called before updating the payload table or
> > > + * starting the ACT sequence and waiting for the corresponding flags to get
> > > + * set by the hub.
> > > + *
> > > + * Returns:
> > > + * 0 if the flag got cleared successfully, otherwise a negative error code.
> > > + */
> > > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
> > > +{
> > > +	int ret;
> > > +
> > > +	ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > > +				 DP_PAYLOAD_ACT_HANDLED);
> > > +	if (ret < 0) {
> > > +		DRM_DEBUG_DRIVER("Can't clear the ACT sent flag (%d)\n", ret);
> > > +		return ret;
> > > +	}
> > > +	WARN_ON(ret != 1);
> > > +
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL(drm_dp_clear_payload_status);
> > > +
> > >  static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
> > >  				     int id, struct drm_dp_payload *payload)
> > >  {
> > > @@ -4384,8 +4412,7 @@ static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
> > >  	int ret;
> > >  	int retries = 0;
> > >  
> > > -	drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > > -			   DP_PAYLOAD_TABLE_UPDATED);
> > 
> > We used to clear DP_PAYLOAD_TABLE_UPDATED but now we clear
> > DP_PAYLOAD_ACT_HANDLED ?
> 
> Eek. We should write DP_PAYLOAD_TABLE_UPDATED which is the only way to
> clear both the act-handled and the table-updated flags.

Huh. That's a bit crazy. But it is what the spec says.

> I tested things
> that way but managed to send an old version. Thanks for catching it.
> 
> > 
> > > +	drm_dp_clear_payload_status(mgr);
> > >  
> > >  	payload_alloc[0] = id;
> > >  	payload_alloc[1] = payload->start_slot;
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > index 9308b5920780..3c4b0fb10d8b 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > @@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
> > >  
> > >  	intel_de_write(i915, intel_dp->regs.dp_tp_status,
> > >  		       DP_TP_STATUS_ACT_SENT);
> > > +
> > > +	drm_dp_clear_payload_status(&intel_dp->mst_mgr);
> > >  }
> > >  
> > >  static void wait_for_act_sent(struct intel_dp *intel_dp)
> > > diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> > > index 8b9eb4db3381..2facb87624bf 100644
> > > --- a/include/drm/drm_dp_mst_helper.h
> > > +++ b/include/drm/drm_dp_mst_helper.h
> > > @@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
> > >  			   int pbn);
> > >  
> > >  
> > > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
> > > +
> > >  int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
> > >  
> > >  
> > > -- 
> > > 2.23.1
> > 
> > -- 
> > Ville Syrjälä
> > Intel

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

Re: [Intel-gfx] [PATCH 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

[Ville Syrjälä]

On Tue, Jun 16, 2020 at 06:54:41PM +0300, Imre Deak wrote:
> On Tue, Jun 16, 2020 at 06:45:46PM +0300, Ville Syrjälä wrote:
> > On Tue, Jun 16, 2020 at 05:18:55PM +0300, Imre Deak wrote:
> > > Atm, we clear the ACT sent flag in the sink's DPCD before updating the
> > > sink's payload table, along clearing the payload table updated flag.
> > > The sink is supposed to set this flag once it detects that the source
> > > has completed the ACT sequence (after detecting the 4 required ACT MTPH
> > > symbols sent by the source). As opposed to this 2 DELL monitors I have
> > > set the flag already along the payload table updated flag, which is not
> > > quite correct.
> > > 
> > > To be sure that the sink has detected the ACT MTPH symbols before
> > > continuing enabling the encoder, clear the ACT sent flag before enabling
> > > or disabling the transcoder VC payload allocation (which is what starts
> > > the ACT sequence).
> > > 
> > > Cc: Lyude Paul <lyude@redhat.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: dri-devel@lists.freedesktop.org
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_dp_mst_topology.c       | 31 +++++++++++++++++++--
> > >  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
> > >  include/drm/drm_dp_mst_helper.h             |  2 ++
> > >  3 files changed, 33 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > index b2f5a84b4cfb..e3bf8c9c8267 100644
> > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > @@ -4377,6 +4377,34 @@ void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
> > >  }
> > >  EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
> > >  
> > > +/**
> > > + * drm_dp_clear_payload_status() - Clears the payload table status flags
> > > + * @mgr: manager to use
> > > + *
> > > + * Clears the payload table ACT handled and table updated flags in the MST hub's
> > > + * DPCD. This function must be called before updating the payload table or
> > > + * starting the ACT sequence and waiting for the corresponding flags to get
> > > + * set by the hub.
> > > + *
> > > + * Returns:
> > > + * 0 if the flag got cleared successfully, otherwise a negative error code.
> > > + */
> > > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
> > > +{
> > > +	int ret;
> > > +
> > > +	ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > > +				 DP_PAYLOAD_ACT_HANDLED);
> > > +	if (ret < 0) {
> > > +		DRM_DEBUG_DRIVER("Can't clear the ACT sent flag (%d)\n", ret);
> > > +		return ret;
> > > +	}
> > > +	WARN_ON(ret != 1);
> > > +
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL(drm_dp_clear_payload_status);
> > > +
> > >  static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
> > >  				     int id, struct drm_dp_payload *payload)
> > >  {
> > > @@ -4384,8 +4412,7 @@ static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
> > >  	int ret;
> > >  	int retries = 0;
> > >  
> > > -	drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > > -			   DP_PAYLOAD_TABLE_UPDATED);
> > 
> > We used to clear DP_PAYLOAD_TABLE_UPDATED but now we clear
> > DP_PAYLOAD_ACT_HANDLED ?
> 
> Eek. We should write DP_PAYLOAD_TABLE_UPDATED which is the only way to
> clear both the act-handled and the table-updated flags.

Huh. That's a bit crazy. But it is what the spec says.

> I tested things
> that way but managed to send an old version. Thanks for catching it.
> 
> > 
> > > +	drm_dp_clear_payload_status(mgr);
> > >  
> > >  	payload_alloc[0] = id;
> > >  	payload_alloc[1] = payload->start_slot;
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > index 9308b5920780..3c4b0fb10d8b 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > @@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
> > >  
> > >  	intel_de_write(i915, intel_dp->regs.dp_tp_status,
> > >  		       DP_TP_STATUS_ACT_SENT);
> > > +
> > > +	drm_dp_clear_payload_status(&intel_dp->mst_mgr);
> > >  }
> > >  
> > >  static void wait_for_act_sent(struct intel_dp *intel_dp)
> > > diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> > > index 8b9eb4db3381..2facb87624bf 100644
> > > --- a/include/drm/drm_dp_mst_helper.h
> > > +++ b/include/drm/drm_dp_mst_helper.h
> > > @@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
> > >  			   int pbn);
> > >  
> > >  
> > > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
> > > +
> > >  int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
> > >  
> > >  
> > > -- 
> > > 2.23.1
> > 
> > -- 
> > Ville Syrjälä
> > Intel

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

Re: [Intel-gfx] [PATCH 1/6] drm/i915/tgl+: Use the correct DP_TP_* register instances in MST encoders

[Souza, Jose]

On Tue, 2020-06-16 at 17:18 +0300, Imre Deak wrote:
> MST encoders must use the master MST transcoder's DP_TP_STATUS and
> DP_TP_CONTROL registers. Atm, during the HW readout of a slave
> transcoder's CRTC state we reset these register addresses in
> intel_dp::regs.dp_tp_* to the slave transcoder's DP_TP_* register
> addresses incorrectly; fix this.
> 
> This issue led at least to
> 'Timed out waiting for ACT sent when disabling'
> errors during output disabling in a multiple MST stream config.

Can you point to place where dp_tp_ctl is used and cause this?
All the MST code paths uses the dp_tp_ctl of the main intel_dp(the one that is not a mst connector).

> 
> This change replaces
> https://patchwork.freedesktop.org/patch/369577/?series=78193&rev=1
> which just papered over the problem.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index ca7bb2294d2b..73d6cc29291a 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4193,11 +4193,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>  	if (drm_WARN_ON(&dev_priv->drm, transcoder_is_dsi(cpu_transcoder)))
>  		return;
>  
> -	if (INTEL_GEN(dev_priv) >= 12) {
> -		intel_dp->regs.dp_tp_ctl = TGL_DP_TP_CTL(cpu_transcoder);
> -		intel_dp->regs.dp_tp_status = TGL_DP_TP_STATUS(cpu_transcoder);
> -	}
> -
>  	intel_dsc_get_config(encoder, pipe_config);
>  
>  	temp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder));
> @@ -4299,6 +4294,16 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>  		break;
>  	}
>  
> +	if (INTEL_GEN(dev_priv) >= 12) {
> +		enum transcoder transcoder =
> +			intel_dp_mst_is_slave_trans(pipe_config) ?
> +			pipe_config->mst_master_transcoder :
> +			pipe_config->cpu_transcoder;
> +
> +		intel_dp->regs.dp_tp_ctl = TGL_DP_TP_CTL(transcoder);
> +		intel_dp->regs.dp_tp_status = TGL_DP_TP_STATUS(transcoder);
> +	}

Also not sure how change only in the config readout would fix the issue, IFWI don't enable MST so when i915 takes over a full modeset will happen to
enable MST and only dp_tp_ctl of the main intel_dp(the one that is not a mst connector) will be set, check tgl_ddi_pre_enable_dp().

> +
>  	pipe_config->has_audio =
>  		intel_ddi_is_audio_enabled(dev_priv, cpu_transcoder);
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

[Ville Syrjälä]

On Tue, Jun 16, 2020 at 07:23:21PM +0300, Ville Syrjälä wrote:
> On Tue, Jun 16, 2020 at 06:54:41PM +0300, Imre Deak wrote:
> > On Tue, Jun 16, 2020 at 06:45:46PM +0300, Ville Syrjälä wrote:
> > > On Tue, Jun 16, 2020 at 05:18:55PM +0300, Imre Deak wrote:
> > > > Atm, we clear the ACT sent flag in the sink's DPCD before updating the
> > > > sink's payload table, along clearing the payload table updated flag.
> > > > The sink is supposed to set this flag once it detects that the source
> > > > has completed the ACT sequence (after detecting the 4 required ACT MTPH
> > > > symbols sent by the source). As opposed to this 2 DELL monitors I have
> > > > set the flag already along the payload table updated flag, which is not
> > > > quite correct.
> > > > 
> > > > To be sure that the sink has detected the ACT MTPH symbols before
> > > > continuing enabling the encoder, clear the ACT sent flag before enabling
> > > > or disabling the transcoder VC payload allocation (which is what starts
> > > > the ACT sequence).
> > > > 
> > > > Cc: Lyude Paul <lyude@redhat.com>
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: dri-devel@lists.freedesktop.org
> > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/drm_dp_mst_topology.c       | 31 +++++++++++++++++++--
> > > >  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
> > > >  include/drm/drm_dp_mst_helper.h             |  2 ++
> > > >  3 files changed, 33 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > index b2f5a84b4cfb..e3bf8c9c8267 100644
> > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > @@ -4377,6 +4377,34 @@ void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
> > > >  }
> > > >  EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
> > > >  
> > > > +/**
> > > > + * drm_dp_clear_payload_status() - Clears the payload table status flags
> > > > + * @mgr: manager to use
> > > > + *
> > > > + * Clears the payload table ACT handled and table updated flags in the MST hub's
> > > > + * DPCD. This function must be called before updating the payload table or
> > > > + * starting the ACT sequence and waiting for the corresponding flags to get
> > > > + * set by the hub.
> > > > + *
> > > > + * Returns:
> > > > + * 0 if the flag got cleared successfully, otherwise a negative error code.
> > > > + */
> > > > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
> > > > +{
> > > > +	int ret;
> > > > +
> > > > +	ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > > > +				 DP_PAYLOAD_ACT_HANDLED);
> > > > +	if (ret < 0) {
> > > > +		DRM_DEBUG_DRIVER("Can't clear the ACT sent flag (%d)\n", ret);
> > > > +		return ret;
> > > > +	}
> > > > +	WARN_ON(ret != 1);
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +EXPORT_SYMBOL(drm_dp_clear_payload_status);
> > > > +
> > > >  static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
> > > >  				     int id, struct drm_dp_payload *payload)
> > > >  {
> > > > @@ -4384,8 +4412,7 @@ static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
> > > >  	int ret;
> > > >  	int retries = 0;
> > > >  
> > > > -	drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > > > -			   DP_PAYLOAD_TABLE_UPDATED);
> > > 
> > > We used to clear DP_PAYLOAD_TABLE_UPDATED but now we clear
> > > DP_PAYLOAD_ACT_HANDLED ?
> > 
> > Eek. We should write DP_PAYLOAD_TABLE_UPDATED which is the only way to
> > clear both the act-handled and the table-updated flags.
> 
> Huh. That's a bit crazy. But it is what the spec says.

In fact, I'd suggest adding a comment explaining this crazyness
so that the next person doesn't have to wonder why we're never
clearing the ACT bit.

> 
> > I tested things
> > that way but managed to send an old version. Thanks for catching it.
> > 
> > > 
> > > > +	drm_dp_clear_payload_status(mgr);
> > > >  
> > > >  	payload_alloc[0] = id;
> > > >  	payload_alloc[1] = payload->start_slot;
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > index 9308b5920780..3c4b0fb10d8b 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > @@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
> > > >  
> > > >  	intel_de_write(i915, intel_dp->regs.dp_tp_status,
> > > >  		       DP_TP_STATUS_ACT_SENT);
> > > > +
> > > > +	drm_dp_clear_payload_status(&intel_dp->mst_mgr);
> > > >  }
> > > >  
> > > >  static void wait_for_act_sent(struct intel_dp *intel_dp)
> > > > diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> > > > index 8b9eb4db3381..2facb87624bf 100644
> > > > --- a/include/drm/drm_dp_mst_helper.h
> > > > +++ b/include/drm/drm_dp_mst_helper.h
> > > > @@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
> > > >  			   int pbn);
> > > >  
> > > >  
> > > > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
> > > > +
> > > >  int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
> > > >  
> > > >  
> > > > -- 
> > > > 2.23.1
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel
> 
> -- 
> Ville Syrjälä
> Intel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [Intel-gfx] [PATCH 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

[Ville Syrjälä]

On Tue, Jun 16, 2020 at 07:23:21PM +0300, Ville Syrjälä wrote:
> On Tue, Jun 16, 2020 at 06:54:41PM +0300, Imre Deak wrote:
> > On Tue, Jun 16, 2020 at 06:45:46PM +0300, Ville Syrjälä wrote:
> > > On Tue, Jun 16, 2020 at 05:18:55PM +0300, Imre Deak wrote:
> > > > Atm, we clear the ACT sent flag in the sink's DPCD before updating the
> > > > sink's payload table, along clearing the payload table updated flag.
> > > > The sink is supposed to set this flag once it detects that the source
> > > > has completed the ACT sequence (after detecting the 4 required ACT MTPH
> > > > symbols sent by the source). As opposed to this 2 DELL monitors I have
> > > > set the flag already along the payload table updated flag, which is not
> > > > quite correct.
> > > > 
> > > > To be sure that the sink has detected the ACT MTPH symbols before
> > > > continuing enabling the encoder, clear the ACT sent flag before enabling
> > > > or disabling the transcoder VC payload allocation (which is what starts
> > > > the ACT sequence).
> > > > 
> > > > Cc: Lyude Paul <lyude@redhat.com>
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: dri-devel@lists.freedesktop.org
> > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/drm_dp_mst_topology.c       | 31 +++++++++++++++++++--
> > > >  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
> > > >  include/drm/drm_dp_mst_helper.h             |  2 ++
> > > >  3 files changed, 33 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > index b2f5a84b4cfb..e3bf8c9c8267 100644
> > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > @@ -4377,6 +4377,34 @@ void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
> > > >  }
> > > >  EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
> > > >  
> > > > +/**
> > > > + * drm_dp_clear_payload_status() - Clears the payload table status flags
> > > > + * @mgr: manager to use
> > > > + *
> > > > + * Clears the payload table ACT handled and table updated flags in the MST hub's
> > > > + * DPCD. This function must be called before updating the payload table or
> > > > + * starting the ACT sequence and waiting for the corresponding flags to get
> > > > + * set by the hub.
> > > > + *
> > > > + * Returns:
> > > > + * 0 if the flag got cleared successfully, otherwise a negative error code.
> > > > + */
> > > > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
> > > > +{
> > > > +	int ret;
> > > > +
> > > > +	ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > > > +				 DP_PAYLOAD_ACT_HANDLED);
> > > > +	if (ret < 0) {
> > > > +		DRM_DEBUG_DRIVER("Can't clear the ACT sent flag (%d)\n", ret);
> > > > +		return ret;
> > > > +	}
> > > > +	WARN_ON(ret != 1);
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +EXPORT_SYMBOL(drm_dp_clear_payload_status);
> > > > +
> > > >  static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
> > > >  				     int id, struct drm_dp_payload *payload)
> > > >  {
> > > > @@ -4384,8 +4412,7 @@ static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
> > > >  	int ret;
> > > >  	int retries = 0;
> > > >  
> > > > -	drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > > > -			   DP_PAYLOAD_TABLE_UPDATED);
> > > 
> > > We used to clear DP_PAYLOAD_TABLE_UPDATED but now we clear
> > > DP_PAYLOAD_ACT_HANDLED ?
> > 
> > Eek. We should write DP_PAYLOAD_TABLE_UPDATED which is the only way to
> > clear both the act-handled and the table-updated flags.
> 
> Huh. That's a bit crazy. But it is what the spec says.

In fact, I'd suggest adding a comment explaining this crazyness
so that the next person doesn't have to wonder why we're never
clearing the ACT bit.

> 
> > I tested things
> > that way but managed to send an old version. Thanks for catching it.
> > 
> > > 
> > > > +	drm_dp_clear_payload_status(mgr);
> > > >  
> > > >  	payload_alloc[0] = id;
> > > >  	payload_alloc[1] = payload->start_slot;
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > index 9308b5920780..3c4b0fb10d8b 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > @@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
> > > >  
> > > >  	intel_de_write(i915, intel_dp->regs.dp_tp_status,
> > > >  		       DP_TP_STATUS_ACT_SENT);
> > > > +
> > > > +	drm_dp_clear_payload_status(&intel_dp->mst_mgr);
> > > >  }
> > > >  
> > > >  static void wait_for_act_sent(struct intel_dp *intel_dp)
> > > > diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> > > > index 8b9eb4db3381..2facb87624bf 100644
> > > > --- a/include/drm/drm_dp_mst_helper.h
> > > > +++ b/include/drm/drm_dp_mst_helper.h
> > > > @@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
> > > >  			   int pbn);
> > > >  
> > > >  
> > > > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
> > > > +
> > > >  int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
> > > >  
> > > >  
> > > > -- 
> > > > 2.23.1
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel
> 
> -- 
> Ville Syrjälä
> Intel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [Intel-gfx] [PATCH 1/6] drm/i915/tgl+: Use the correct DP_TP_* register instances in MST encoders

[Imre Deak]

On Tue, Jun 16, 2020 at 07:32:46PM +0300, Souza, Jose wrote:
> On Tue, 2020-06-16 at 17:18 +0300, Imre Deak wrote:
> > MST encoders must use the master MST transcoder's DP_TP_STATUS and
> > DP_TP_CONTROL registers. Atm, during the HW readout of a slave
> > transcoder's CRTC state we reset these register addresses in
> > intel_dp::regs.dp_tp_* to the slave transcoder's DP_TP_* register
> > addresses incorrectly; fix this.
> > 
> > This issue led at least to
> > 'Timed out waiting for ACT sent when disabling'
> > errors during output disabling in a multiple MST stream config.
> 
> Can you point to place where dp_tp_ctl is used and cause this?  All
> the MST code paths uses the dp_tp_ctl of the main intel_dp(the one
> that is not a mst connector).

During a slave stream disabling when waiting for the ACT sent flag for
that stream.

> 
> > 
> > This change replaces
> > https://patchwork.freedesktop.org/patch/369577/?series=78193&rev=1
> > which just papered over the problem.
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c | 15 ++++++++++-----
> >  1 file changed, 10 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index ca7bb2294d2b..73d6cc29291a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -4193,11 +4193,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >  	if (drm_WARN_ON(&dev_priv->drm, transcoder_is_dsi(cpu_transcoder)))
> >  		return;
> >  
> > -	if (INTEL_GEN(dev_priv) >= 12) {
> > -		intel_dp->regs.dp_tp_ctl = TGL_DP_TP_CTL(cpu_transcoder);
> > -		intel_dp->regs.dp_tp_status = TGL_DP_TP_STATUS(cpu_transcoder);
> > -	}
> > -
> >  	intel_dsc_get_config(encoder, pipe_config);
> >  
> >  	temp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder));
> > @@ -4299,6 +4294,16 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >  		break;
> >  	}
> >  
> > +	if (INTEL_GEN(dev_priv) >= 12) {
> > +		enum transcoder transcoder =
> > +			intel_dp_mst_is_slave_trans(pipe_config) ?
> > +			pipe_config->mst_master_transcoder :
> > +			pipe_config->cpu_transcoder;
> > +
> > +		intel_dp->regs.dp_tp_ctl = TGL_DP_TP_CTL(transcoder);
> > +		intel_dp->regs.dp_tp_status = TGL_DP_TP_STATUS(transcoder);
> > +	}
> 
> Also not sure how change only in the config readout would fix the issue, 

After a modeset we'll verify the HW state. The readout for a slave
stream CRTC (get_pipe_config) running after the master CRTC's readout
will overwrite the dp_tp reg addresses. The other instance of dp_tp
register address init (in tgl_ddi_pre_enable_dp()) is correct.

> IFWI don't enable MST so when i915 takes over a full modeset will
> happen to enable MST and only dp_tp_ctl of the main intel_dp(the one
> that is not a mst connector) will be set, check
> tgl_ddi_pre_enable_dp().
> 
> > +
> >  	pipe_config->has_audio =
> >  		intel_ddi_is_audio_enabled(dev_priv, cpu_transcoder);
> >  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

[Imre Deak]

On Tue, Jun 16, 2020 at 07:40:47PM +0300, Ville Syrjälä wrote:
> On Tue, Jun 16, 2020 at 07:23:21PM +0300, Ville Syrjälä wrote:
> > On Tue, Jun 16, 2020 at 06:54:41PM +0300, Imre Deak wrote:
> > > On Tue, Jun 16, 2020 at 06:45:46PM +0300, Ville Syrjälä wrote:
> > > > On Tue, Jun 16, 2020 at 05:18:55PM +0300, Imre Deak wrote:
> > > > > Atm, we clear the ACT sent flag in the sink's DPCD before updating the
> > > > > sink's payload table, along clearing the payload table updated flag.
> > > > > The sink is supposed to set this flag once it detects that the source
> > > > > has completed the ACT sequence (after detecting the 4 required ACT MTPH
> > > > > symbols sent by the source). As opposed to this 2 DELL monitors I have
> > > > > set the flag already along the payload table updated flag, which is not
> > > > > quite correct.
> > > > > 
> > > > > To be sure that the sink has detected the ACT MTPH symbols before
> > > > > continuing enabling the encoder, clear the ACT sent flag before enabling
> > > > > or disabling the transcoder VC payload allocation (which is what starts
> > > > > the ACT sequence).
> > > > > 
> > > > > Cc: Lyude Paul <lyude@redhat.com>
> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Cc: dri-devel@lists.freedesktop.org
> > > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/drm_dp_mst_topology.c       | 31 +++++++++++++++++++--
> > > > >  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
> > > > >  include/drm/drm_dp_mst_helper.h             |  2 ++
> > > > >  3 files changed, 33 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > index b2f5a84b4cfb..e3bf8c9c8267 100644
> > > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > @@ -4377,6 +4377,34 @@ void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
> > > > >  }
> > > > >  EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
> > > > >  
> > > > > +/**
> > > > > + * drm_dp_clear_payload_status() - Clears the payload table status flags
> > > > > + * @mgr: manager to use
> > > > > + *
> > > > > + * Clears the payload table ACT handled and table updated flags in the MST hub's
> > > > > + * DPCD. This function must be called before updating the payload table or
> > > > > + * starting the ACT sequence and waiting for the corresponding flags to get
> > > > > + * set by the hub.
> > > > > + *
> > > > > + * Returns:
> > > > > + * 0 if the flag got cleared successfully, otherwise a negative error code.
> > > > > + */
> > > > > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
> > > > > +{
> > > > > +	int ret;
> > > > > +
> > > > > +	ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > > > > +				 DP_PAYLOAD_ACT_HANDLED);
> > > > > +	if (ret < 0) {
> > > > > +		DRM_DEBUG_DRIVER("Can't clear the ACT sent flag (%d)\n", ret);
> > > > > +		return ret;
> > > > > +	}
> > > > > +	WARN_ON(ret != 1);
> > > > > +
> > > > > +	return 0;
> > > > > +}
> > > > > +EXPORT_SYMBOL(drm_dp_clear_payload_status);
> > > > > +
> > > > >  static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
> > > > >  				     int id, struct drm_dp_payload *payload)
> > > > >  {
> > > > > @@ -4384,8 +4412,7 @@ static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
> > > > >  	int ret;
> > > > >  	int retries = 0;
> > > > >  
> > > > > -	drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > > > > -			   DP_PAYLOAD_TABLE_UPDATED);
> > > > 
> > > > We used to clear DP_PAYLOAD_TABLE_UPDATED but now we clear
> > > > DP_PAYLOAD_ACT_HANDLED ?
> > > 
> > > Eek. We should write DP_PAYLOAD_TABLE_UPDATED which is the only way to
> > > clear both the act-handled and the table-updated flags.
> > 
> > Huh. That's a bit crazy. But it is what the spec says.
> 
> In fact, I'd suggest adding a comment explaining this crazyness
> so that the next person doesn't have to wonder why we're never
> clearing the ACT bit.

Ok.

> 
> > 
> > > I tested things
> > > that way but managed to send an old version. Thanks for catching it.
> > > 
> > > > 
> > > > > +	drm_dp_clear_payload_status(mgr);
> > > > >  
> > > > >  	payload_alloc[0] = id;
> > > > >  	payload_alloc[1] = payload->start_slot;
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > > index 9308b5920780..3c4b0fb10d8b 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > > @@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
> > > > >  
> > > > >  	intel_de_write(i915, intel_dp->regs.dp_tp_status,
> > > > >  		       DP_TP_STATUS_ACT_SENT);
> > > > > +
> > > > > +	drm_dp_clear_payload_status(&intel_dp->mst_mgr);
> > > > >  }
> > > > >  
> > > > >  static void wait_for_act_sent(struct intel_dp *intel_dp)
> > > > > diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> > > > > index 8b9eb4db3381..2facb87624bf 100644
> > > > > --- a/include/drm/drm_dp_mst_helper.h
> > > > > +++ b/include/drm/drm_dp_mst_helper.h
> > > > > @@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
> > > > >  			   int pbn);
> > > > >  
> > > > >  
> > > > > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
> > > > > +
> > > > >  int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
> > > > >  
> > > > >  
> > > > > -- 
> > > > > 2.23.1
> > > > 
> > > > -- 
> > > > Ville Syrjälä
> > > > Intel
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

[Imre Deak]

On Tue, Jun 16, 2020 at 07:40:47PM +0300, Ville Syrjälä wrote:
> On Tue, Jun 16, 2020 at 07:23:21PM +0300, Ville Syrjälä wrote:
> > On Tue, Jun 16, 2020 at 06:54:41PM +0300, Imre Deak wrote:
> > > On Tue, Jun 16, 2020 at 06:45:46PM +0300, Ville Syrjälä wrote:
> > > > On Tue, Jun 16, 2020 at 05:18:55PM +0300, Imre Deak wrote:
> > > > > Atm, we clear the ACT sent flag in the sink's DPCD before updating the
> > > > > sink's payload table, along clearing the payload table updated flag.
> > > > > The sink is supposed to set this flag once it detects that the source
> > > > > has completed the ACT sequence (after detecting the 4 required ACT MTPH
> > > > > symbols sent by the source). As opposed to this 2 DELL monitors I have
> > > > > set the flag already along the payload table updated flag, which is not
> > > > > quite correct.
> > > > > 
> > > > > To be sure that the sink has detected the ACT MTPH symbols before
> > > > > continuing enabling the encoder, clear the ACT sent flag before enabling
> > > > > or disabling the transcoder VC payload allocation (which is what starts
> > > > > the ACT sequence).
> > > > > 
> > > > > Cc: Lyude Paul <lyude@redhat.com>
> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Cc: dri-devel@lists.freedesktop.org
> > > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/drm_dp_mst_topology.c       | 31 +++++++++++++++++++--
> > > > >  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
> > > > >  include/drm/drm_dp_mst_helper.h             |  2 ++
> > > > >  3 files changed, 33 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > index b2f5a84b4cfb..e3bf8c9c8267 100644
> > > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > @@ -4377,6 +4377,34 @@ void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
> > > > >  }
> > > > >  EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
> > > > >  
> > > > > +/**
> > > > > + * drm_dp_clear_payload_status() - Clears the payload table status flags
> > > > > + * @mgr: manager to use
> > > > > + *
> > > > > + * Clears the payload table ACT handled and table updated flags in the MST hub's
> > > > > + * DPCD. This function must be called before updating the payload table or
> > > > > + * starting the ACT sequence and waiting for the corresponding flags to get
> > > > > + * set by the hub.
> > > > > + *
> > > > > + * Returns:
> > > > > + * 0 if the flag got cleared successfully, otherwise a negative error code.
> > > > > + */
> > > > > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
> > > > > +{
> > > > > +	int ret;
> > > > > +
> > > > > +	ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > > > > +				 DP_PAYLOAD_ACT_HANDLED);
> > > > > +	if (ret < 0) {
> > > > > +		DRM_DEBUG_DRIVER("Can't clear the ACT sent flag (%d)\n", ret);
> > > > > +		return ret;
> > > > > +	}
> > > > > +	WARN_ON(ret != 1);
> > > > > +
> > > > > +	return 0;
> > > > > +}
> > > > > +EXPORT_SYMBOL(drm_dp_clear_payload_status);
> > > > > +
> > > > >  static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
> > > > >  				     int id, struct drm_dp_payload *payload)
> > > > >  {
> > > > > @@ -4384,8 +4412,7 @@ static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
> > > > >  	int ret;
> > > > >  	int retries = 0;
> > > > >  
> > > > > -	drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > > > > -			   DP_PAYLOAD_TABLE_UPDATED);
> > > > 
> > > > We used to clear DP_PAYLOAD_TABLE_UPDATED but now we clear
> > > > DP_PAYLOAD_ACT_HANDLED ?
> > > 
> > > Eek. We should write DP_PAYLOAD_TABLE_UPDATED which is the only way to
> > > clear both the act-handled and the table-updated flags.
> > 
> > Huh. That's a bit crazy. But it is what the spec says.
> 
> In fact, I'd suggest adding a comment explaining this crazyness
> so that the next person doesn't have to wonder why we're never
> clearing the ACT bit.

Ok.

> 
> > 
> > > I tested things
> > > that way but managed to send an old version. Thanks for catching it.
> > > 
> > > > 
> > > > > +	drm_dp_clear_payload_status(mgr);
> > > > >  
> > > > >  	payload_alloc[0] = id;
> > > > >  	payload_alloc[1] = payload->start_slot;
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > > index 9308b5920780..3c4b0fb10d8b 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > > @@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
> > > > >  
> > > > >  	intel_de_write(i915, intel_dp->regs.dp_tp_status,
> > > > >  		       DP_TP_STATUS_ACT_SENT);
> > > > > +
> > > > > +	drm_dp_clear_payload_status(&intel_dp->mst_mgr);
> > > > >  }
> > > > >  
> > > > >  static void wait_for_act_sent(struct intel_dp *intel_dp)
> > > > > diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> > > > > index 8b9eb4db3381..2facb87624bf 100644
> > > > > --- a/include/drm/drm_dp_mst_helper.h
> > > > > +++ b/include/drm/drm_dp_mst_helper.h
> > > > > @@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
> > > > >  			   int pbn);
> > > > >  
> > > > >  
> > > > > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
> > > > > +
> > > > >  int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
> > > > >  
> > > > >  
> > > > > -- 
> > > > > 2.23.1
> > > > 
> > > > -- 
> > > > Ville Syrjälä
> > > > Intel
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/6] drm/i915/tgl+: Use the correct DP_TP_* register instances in MST encoders

[Souza, Jose]

On Tue, 2020-06-16 at 19:42 +0300, Imre Deak wrote:
> On Tue, Jun 16, 2020 at 07:32:46PM +0300, Souza, Jose wrote:
> > On Tue, 2020-06-16 at 17:18 +0300, Imre Deak wrote:
> > > MST encoders must use the master MST transcoder's DP_TP_STATUS and
> > > DP_TP_CONTROL registers. Atm, during the HW readout of a slave
> > > transcoder's CRTC state we reset these register addresses in
> > > intel_dp::regs.dp_tp_* to the slave transcoder's DP_TP_* register
> > > addresses incorrectly; fix this.
> > > 
> > > This issue led at least to
> > > 'Timed out waiting for ACT sent when disabling'
> > > errors during output disabling in a multiple MST stream config.
> > 
> > Can you point to place where dp_tp_ctl is used and cause this?  All
> > the MST code paths uses the dp_tp_ctl of the main intel_dp(the one
> > that is not a mst connector).
> 
> During a slave stream disabling when waiting for the ACT sent flag for
> that stream.
> 
> > > This change replaces
> > > https://patchwork.freedesktop.org/patch/369577/?series=78193&rev=1
> > > which just papered over the problem.
> > > 
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: José Roberto de Souza <jose.souza@intel.com>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_ddi.c | 15 ++++++++++-----
> > >  1 file changed, 10 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index ca7bb2294d2b..73d6cc29291a 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -4193,11 +4193,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> > >  	if (drm_WARN_ON(&dev_priv->drm, transcoder_is_dsi(cpu_transcoder)))
> > >  		return;
> > >  
> > > -	if (INTEL_GEN(dev_priv) >= 12) {
> > > -		intel_dp->regs.dp_tp_ctl = TGL_DP_TP_CTL(cpu_transcoder);
> > > -		intel_dp->regs.dp_tp_status = TGL_DP_TP_STATUS(cpu_transcoder);
> > > -	}
> > > -
> > >  	intel_dsc_get_config(encoder, pipe_config);
> > >  
> > >  	temp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder));
> > > @@ -4299,6 +4294,16 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> > >  		break;
> > >  	}
> > >  
> > > +	if (INTEL_GEN(dev_priv) >= 12) {
> > > +		enum transcoder transcoder =
> > > +			intel_dp_mst_is_slave_trans(pipe_config) ?
> > > +			pipe_config->mst_master_transcoder :
> > > +			pipe_config->cpu_transcoder;
> > > +
> > > +		intel_dp->regs.dp_tp_ctl = TGL_DP_TP_CTL(transcoder);
> > > +		intel_dp->regs.dp_tp_status = TGL_DP_TP_STATUS(transcoder);
> > > +	}
> > 
> > Also not sure how change only in the config readout would fix the issue, 
> 
> After a modeset we'll verify the HW state. The readout for a slave
> stream CRTC (get_pipe_config) running after the master CRTC's readout
> will overwrite the dp_tp reg addresses. The other instance of dp_tp
> register address init (in tgl_ddi_pre_enable_dp()) is correct.

intel_mst_post_disable_dp()
	struct intel_digital_port *intel_dig_port = intel_mst->primary;
	struct intel_dp *intel_dp = &intel_dig_port->dp;
	
...
	
	if (intel_de_wait_for_set(dev_priv, intel_dp->regs.dp_tp_status,
				  DP_TP_STATUS_ACT_SENT, 1))
		drm_err(&dev_priv->drm, "Timed out waiting for ACT sent when disabling\n");


Until here is right, but yeah bellow is the problem:

static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
					struct intel_crtc_state *pipe_config)
{
	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
	struct intel_digital_port *intel_dig_port = intel_mst->primary;

	intel_ddi_get_config(&intel_dig_port->base, pipe_config);
}

It will be overwritten with the transcoder of the last crtc read.Would suggest to add something about intel_dp_mst_enc_get_config() to the commit
description but the change looks good now.


> 
> > IFWI don't enable MST so when i915 takes over a full modeset will
> > happen to enable MST and only dp_tp_ctl of the main intel_dp(the one
> > that is not a mst connector) will be set, check
> > tgl_ddi_pre_enable_dp().
> > 
> > > +
> > >  	pipe_config->has_audio =
> > >  		intel_ddi_is_audio_enabled(dev_priv, cpu_transcoder);
> > >  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/6] drm/i915/tgl+: Use the correct DP_TP_* register instances in MST encoders

[Imre Deak]

On Tue, Jun 16, 2020 at 08:02:10PM +0300, Souza, Jose wrote:
> On Tue, 2020-06-16 at 19:42 +0300, Imre Deak wrote:
> > On Tue, Jun 16, 2020 at 07:32:46PM +0300, Souza, Jose wrote:
> > > On Tue, 2020-06-16 at 17:18 +0300, Imre Deak wrote:
> > > > MST encoders must use the master MST transcoder's DP_TP_STATUS and
> > > > DP_TP_CONTROL registers. Atm, during the HW readout of a slave
> > > > transcoder's CRTC state we reset these register addresses in
> > > > intel_dp::regs.dp_tp_* to the slave transcoder's DP_TP_* register
> > > > addresses incorrectly; fix this.
> > > > 
> > > > This issue led at least to
> > > > 'Timed out waiting for ACT sent when disabling'
> > > > errors during output disabling in a multiple MST stream config.
> > > 
> > > Can you point to place where dp_tp_ctl is used and cause this?  All
> > > the MST code paths uses the dp_tp_ctl of the main intel_dp(the one
> > > that is not a mst connector).
> > 
> > During a slave stream disabling when waiting for the ACT sent flag for
> > that stream.
> > 
> > > > This change replaces
> > > > https://patchwork.freedesktop.org/patch/369577/?series=78193&rev=1
> > > > which just papered over the problem.
> > > > 
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: José Roberto de Souza <jose.souza@intel.com>
> > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_ddi.c | 15 ++++++++++-----
> > > >  1 file changed, 10 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > index ca7bb2294d2b..73d6cc29291a 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > @@ -4193,11 +4193,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> > > >  	if (drm_WARN_ON(&dev_priv->drm, transcoder_is_dsi(cpu_transcoder)))
> > > >  		return;
> > > >  
> > > > -	if (INTEL_GEN(dev_priv) >= 12) {
> > > > -		intel_dp->regs.dp_tp_ctl = TGL_DP_TP_CTL(cpu_transcoder);
> > > > -		intel_dp->regs.dp_tp_status = TGL_DP_TP_STATUS(cpu_transcoder);
> > > > -	}
> > > > -
> > > >  	intel_dsc_get_config(encoder, pipe_config);
> > > >  
> > > >  	temp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder));
> > > > @@ -4299,6 +4294,16 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> > > >  		break;
> > > >  	}
> > > >  
> > > > +	if (INTEL_GEN(dev_priv) >= 12) {
> > > > +		enum transcoder transcoder =
> > > > +			intel_dp_mst_is_slave_trans(pipe_config) ?
> > > > +			pipe_config->mst_master_transcoder :
> > > > +			pipe_config->cpu_transcoder;
> > > > +
> > > > +		intel_dp->regs.dp_tp_ctl = TGL_DP_TP_CTL(transcoder);
> > > > +		intel_dp->regs.dp_tp_status = TGL_DP_TP_STATUS(transcoder);
> > > > +	}
> > > 
> > > Also not sure how change only in the config readout would fix the issue, 
> > 
> > After a modeset we'll verify the HW state. The readout for a slave
> > stream CRTC (get_pipe_config) running after the master CRTC's readout
> > will overwrite the dp_tp reg addresses. The other instance of dp_tp
> > register address init (in tgl_ddi_pre_enable_dp()) is correct.
> 
> intel_mst_post_disable_dp()
> 	struct intel_digital_port *intel_dig_port = intel_mst->primary;
> 	struct intel_dp *intel_dp = &intel_dig_port->dp;
> 	
> ...
> 	
> 	if (intel_de_wait_for_set(dev_priv, intel_dp->regs.dp_tp_status,
> 				  DP_TP_STATUS_ACT_SENT, 1))
> 		drm_err(&dev_priv->drm, "Timed out waiting for ACT sent when disabling\n");
> 
> 
> Until here is right, but yeah bellow is the problem:
> 
> static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
> 					struct intel_crtc_state *pipe_config)
> {
> 	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
> 	struct intel_digital_port *intel_dig_port = intel_mst->primary;
> 
> 	intel_ddi_get_config(&intel_dig_port->base, pipe_config);
> }
> 
>
> It will be overwritten with the transcoder of the last crtc read.Would
> suggest to add something about intel_dp_mst_enc_get_config() to the
> commit description but the change looks good now.

Hm yea, it's the encoder not the CRTC readout where the overwrite
happens. Will update this in the commit log. 

> > > IFWI don't enable MST so when i915 takes over a full modeset will
> > > happen to enable MST and only dp_tp_ctl of the main intel_dp(the one
> > > that is not a mst connector) will be set, check
> > > tgl_ddi_pre_enable_dp().
> > > 
> > > > +
> > > >  	pipe_config->has_audio =
> > > >  		intel_ddi_is_audio_enabled(dev_priv, cpu_transcoder);
> > > >  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/6] drm/i915/tgl+: Use the correct DP_TP_* register instances in MST encoders

[Patchwork]

== Series Details ==

Series: series starting with [1/6] drm/i915/tgl+: Use the correct DP_TP_* register instances in MST encoders
URL   : https://patchwork.freedesktop.org/series/78423/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8635 -> Patchwork_17964
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/index.html

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_cursor_legacy@basic-flip-before-cursor-atomic:
    - fi-icl-guc:         [PASS][1] -> [DMESG-WARN][2] ([i915#1982])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/fi-icl-guc/igt@kms_cursor_legacy@basic-flip-before-cursor-atomic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/fi-icl-guc/igt@kms_cursor_legacy@basic-flip-before-cursor-atomic.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1:
    - fi-icl-u2:          [PASS][3] -> [DMESG-WARN][4] ([i915#1982])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html

  
#### Possible fixes ####

  * igt@kms_busy@basic@flip:
    - fi-kbl-x1275:       [DMESG-WARN][5] ([i915#62] / [i915#92] / [i915#95]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/fi-kbl-x1275/igt@kms_busy@basic@flip.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/fi-kbl-x1275/igt@kms_busy@basic@flip.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - fi-icl-guc:         [DMESG-WARN][7] ([i915#1982]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/fi-icl-guc/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/fi-icl-guc/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  
#### Warnings ####

  * igt@i915_module_load@reload:
    - fi-kbl-x1275:       [DMESG-WARN][9] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][10] ([i915#62] / [i915#92])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/fi-kbl-x1275/igt@i915_module_load@reload.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/fi-kbl-x1275/igt@i915_module_load@reload.html

  * igt@i915_pm_rpm@basic-rte:
    - fi-kbl-guc:         [SKIP][11] ([fdo#109271]) -> [FAIL][12] ([i915#665] / [i915#704])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/fi-kbl-guc/igt@i915_pm_rpm@basic-rte.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/fi-kbl-guc/igt@i915_pm_rpm@basic-rte.html

  * igt@prime_vgem@basic-fence-flip:
    - fi-kbl-x1275:       [DMESG-WARN][13] ([i915#62] / [i915#92]) -> [DMESG-WARN][14] ([i915#62] / [i915#92] / [i915#95])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/fi-kbl-x1275/igt@prime_vgem@basic-fence-flip.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/fi-kbl-x1275/igt@prime_vgem@basic-fence-flip.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1569]: https://gitlab.freedesktop.org/drm/intel/issues/1569
  [i915#192]: https://gitlab.freedesktop.org/drm/intel/issues/192
  [i915#193]: https://gitlab.freedesktop.org/drm/intel/issues/193
  [i915#194]: https://gitlab.freedesktop.org/drm/intel/issues/194
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2029]: https://gitlab.freedesktop.org/drm/intel/issues/2029
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#665]: https://gitlab.freedesktop.org/drm/intel/issues/665
  [i915#704]: https://gitlab.freedesktop.org/drm/intel/issues/704
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (47 -> 42)
------------------------------

  Missing    (5): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_8635 -> Patchwork_17964

  CI-20190529: 20190529
  CI_DRM_8635: f9acdb898773f94ac1bcb9a8826596f88412a53b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5711: 90611a0c90afa4a46496c78a4faf9638a1538ac3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17964: d302e0a23522a808ab7073bf458e7b70df70def3 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

d302e0a23522 drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it
005be72f6fe4 drm/i915/dp_mst: Clear the ACT sent flag during encoder disabling too
1c1441217489 drm/i915/dp_mst: Clear only the ACT sent flag from DP_TP_STATUS
23865500149b drm/i915/dp_mst: Move clearing the ACT sent flag closer to its polling
94a0f7ad711f drm/i915/dp_mst: Disable link training fallback on MST links
15358e434948 drm/i915/tgl+: Use the correct DP_TP_* register instances in MST encoders

== Logs ==

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

[Intel-gfx] [PATCH v2 1/6] drm/i915/tgl+: Use the correct DP_TP_* register instances in MST encoders

[Imre Deak]

MST encoders must use the master MST transcoder's DP_TP_STATUS and
DP_TP_CONTROL registers. Atm, during the HW readout of an MST encoder
connected to a slave transcoder we reset these register addresses in
intel_dp::regs.dp_tp_* to the slave transcoder's DP_TP_* register
addresses incorrectly; fix this.

One example where the above overwite happens is the encoder HW state
validation after enabling multiple streams; see
intel_dp_mst_enc_get_config(). After that during disabling any stream
we'll get a

'Timed out waiting for ACT sent when disabling'

error, due to reading from the incorrect DP_TP_STATUS register.

This change replaces
https://patchwork.freedesktop.org/patch/369577/?series=78193&rev=1
which just papered over the problem.

v2:
- Correct the failure scenario in the commit log. (José)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index ca7bb2294d2b..73d6cc29291a 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4193,11 +4193,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 	if (drm_WARN_ON(&dev_priv->drm, transcoder_is_dsi(cpu_transcoder)))
 		return;
 
-	if (INTEL_GEN(dev_priv) >= 12) {
-		intel_dp->regs.dp_tp_ctl = TGL_DP_TP_CTL(cpu_transcoder);
-		intel_dp->regs.dp_tp_status = TGL_DP_TP_STATUS(cpu_transcoder);
-	}
-
 	intel_dsc_get_config(encoder, pipe_config);
 
 	temp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder));
@@ -4299,6 +4294,16 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 		break;
 	}
 
+	if (INTEL_GEN(dev_priv) >= 12) {
+		enum transcoder transcoder =
+			intel_dp_mst_is_slave_trans(pipe_config) ?
+			pipe_config->mst_master_transcoder :
+			pipe_config->cpu_transcoder;
+
+		intel_dp->regs.dp_tp_ctl = TGL_DP_TP_CTL(transcoder);
+		intel_dp->regs.dp_tp_status = TGL_DP_TP_STATUS(transcoder);
+	}
+
 	pipe_config->has_audio =
 		intel_ddi_is_audio_enabled(dev_priv, cpu_transcoder);
 
-- 
2.23.1

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

[Intel-gfx] [PATCH v2 2/6] drm/i915/dp_mst: Disable link training fallback on MST links

[Imre Deak]

During the initial probing of an MST sink, MST core will determine the
sink's link bandwidth based on its own version of the sink link
rate/lane count caps it reads from the DPCD. At a later point (after
probing and 1 or more modesets) i915 may limit the link parameters wrt.
the original source/sink common caps above due to link training failures
during a modeset and the resulting link training fallback logic.

Based on the above a modeset following another modeset with a link
training error will compute the i915 HW specific and DP protocol timing
parameters (data/link M/N and MST TU values) taking into account only
the unlimited source/sink common caps, but not taking into account the
fallback limits. This will also let DRM core oversubscribe the actual
link bandwidth during the MST payload allocation.

Prevent the above problem by disabling the link training fallback on MST
links for now, until the MST probe time initialization and the MST
compute config logic can deal with changing link parameters.

The misconfigured timings lead at least to a
'Timed out waiting for DP idle patterns'
error.

v2: (Ville)
- Print link training error message on the MST path too.
- Clarify the problem in the commit log.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 27 ++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 42589cae766d..66d9ee94cdd0 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -468,6 +468,15 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
 	int index;
 
+	/*
+	 * TODO: Enable fallback on MST links once MST link compute can handle
+	 * the fallback params.
+	 */
+	if (intel_dp->is_mst) {
+		drm_err(&i915->drm, "Link Training Unsuccessful\n");
+		return -1;
+	}
+
 	index = intel_dp_rate_index(intel_dp->common_rates,
 				    intel_dp->num_common_rates,
 				    link_rate);
@@ -6163,7 +6172,17 @@ intel_dp_detect(struct drm_connector *connector,
 		goto out;
 	}
 
-	if (intel_dp->reset_link_params) {
+	/* Read DP Sink DSC Cap DPCD regs for DP v1.4 */
+	if (INTEL_GEN(dev_priv) >= 11)
+		intel_dp_get_dsc_sink_cap(intel_dp);
+
+	intel_dp_configure_mst(intel_dp);
+
+	/*
+	 * TODO: Reset link params when switching to MST mode, until MST
+	 * supports link training fallback params.
+	 */
+	if (intel_dp->reset_link_params || intel_dp->is_mst) {
 		/* Initial max link lane count */
 		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
 
@@ -6175,12 +6194,6 @@ intel_dp_detect(struct drm_connector *connector,
 
 	intel_dp_print_rates(intel_dp);
 
-	/* Read DP Sink DSC Cap DPCD regs for DP v1.4 */
-	if (INTEL_GEN(dev_priv) >= 11)
-		intel_dp_get_dsc_sink_cap(intel_dp);
-
-	intel_dp_configure_mst(intel_dp);
-
 	if (intel_dp->is_mst) {
 		/*
 		 * If we are in MST mode then this connector
-- 
2.23.1

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

[PATCH v2 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

[Imre Deak]

Atm, we clear the ACT sent flag in the sink's DPCD before updating the
sink's payload table, along clearing the payload table updated flag.
The sink is supposed to set this flag once it detects that the source
has completed the ACT sequence (after detecting the 4 required ACT MTPH
symbols sent by the source). As opposed to this 2 DELL monitors I have
set the flag already along the payload table updated flag, which is not
quite correct.

To be sure that the sink has detected the ACT MTPH symbols before
continuing enabling the encoder, clear the ACT sent flag before enabling
or disabling the transcoder VC payload allocation (which is what starts
the ACT sequence).

v2 (Ville):
- Use the correct bit to clear the flags.
- Add code comment explaining the clearing semantics of the ACT handled
  flag.

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

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index b2f5a84b4cfb..1f5d14128c1a 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4377,6 +4377,41 @@ void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
 }
 EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
 
+/**
+ * drm_dp_clear_payload_status() - Clears the payload table status flags
+ * @mgr: manager to use
+ *
+ * Clears the payload table ACT handled and table updated flags in the MST hub's
+ * DPCD. This function must be called before updating the payload table or
+ * starting the ACT sequence and waiting for the corresponding flags to get
+ * set by the hub.
+ *
+ * Returns:
+ * 0 if the flags got cleared successfully, otherwise a negative error code.
+ */
+int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
+{
+	int ret;
+
+	/*
+	 * Note that the following is based on the DP Standard stating that
+	 * writing the DP_PAYLOAD_TABLE_UPDATED bit alone will clear both the
+	 * DP_PAYLOAD_TABLE_UPDATED and the DP_PAYLOAD_ACT_HANDLED flags. This
+	 * seems to be also the only way to clear DP_PAYLOAD_ACT_HANDLED.
+	 */
+	ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
+				 DP_PAYLOAD_TABLE_UPDATED);
+	if (ret < 0) {
+		DRM_DEBUG_DRIVER("Can't clear the ACT handled/table updated flags (%d)\n",
+				 ret);
+		return ret;
+	}
+	WARN_ON(ret != 1);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_dp_clear_payload_status);
+
 static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
 				     int id, struct drm_dp_payload *payload)
 {
@@ -4384,8 +4419,7 @@ static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
 	int ret;
 	int retries = 0;
 
-	drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
-			   DP_PAYLOAD_TABLE_UPDATED);
+	drm_dp_clear_payload_status(mgr);
 
 	payload_alloc[0] = id;
 	payload_alloc[1] = payload->start_slot;
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 9308b5920780..3c4b0fb10d8b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
 
 	intel_de_write(i915, intel_dp->regs.dp_tp_status,
 		       DP_TP_STATUS_ACT_SENT);
+
+	drm_dp_clear_payload_status(&intel_dp->mst_mgr);
 }
 
 static void wait_for_act_sent(struct intel_dp *intel_dp)
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 8b9eb4db3381..2facb87624bf 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
 			   int pbn);
 
 
+int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
+
 int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
 
 
-- 
2.23.1

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

[Intel-gfx] [PATCH v2 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

[Imre Deak]

Atm, we clear the ACT sent flag in the sink's DPCD before updating the
sink's payload table, along clearing the payload table updated flag.
The sink is supposed to set this flag once it detects that the source
has completed the ACT sequence (after detecting the 4 required ACT MTPH
symbols sent by the source). As opposed to this 2 DELL monitors I have
set the flag already along the payload table updated flag, which is not
quite correct.

To be sure that the sink has detected the ACT MTPH symbols before
continuing enabling the encoder, clear the ACT sent flag before enabling
or disabling the transcoder VC payload allocation (which is what starts
the ACT sequence).

v2 (Ville):
- Use the correct bit to clear the flags.
- Add code comment explaining the clearing semantics of the ACT handled
  flag.

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

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index b2f5a84b4cfb..1f5d14128c1a 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4377,6 +4377,41 @@ void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
 }
 EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
 
+/**
+ * drm_dp_clear_payload_status() - Clears the payload table status flags
+ * @mgr: manager to use
+ *
+ * Clears the payload table ACT handled and table updated flags in the MST hub's
+ * DPCD. This function must be called before updating the payload table or
+ * starting the ACT sequence and waiting for the corresponding flags to get
+ * set by the hub.
+ *
+ * Returns:
+ * 0 if the flags got cleared successfully, otherwise a negative error code.
+ */
+int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
+{
+	int ret;
+
+	/*
+	 * Note that the following is based on the DP Standard stating that
+	 * writing the DP_PAYLOAD_TABLE_UPDATED bit alone will clear both the
+	 * DP_PAYLOAD_TABLE_UPDATED and the DP_PAYLOAD_ACT_HANDLED flags. This
+	 * seems to be also the only way to clear DP_PAYLOAD_ACT_HANDLED.
+	 */
+	ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
+				 DP_PAYLOAD_TABLE_UPDATED);
+	if (ret < 0) {
+		DRM_DEBUG_DRIVER("Can't clear the ACT handled/table updated flags (%d)\n",
+				 ret);
+		return ret;
+	}
+	WARN_ON(ret != 1);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_dp_clear_payload_status);
+
 static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
 				     int id, struct drm_dp_payload *payload)
 {
@@ -4384,8 +4419,7 @@ static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
 	int ret;
 	int retries = 0;
 
-	drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
-			   DP_PAYLOAD_TABLE_UPDATED);
+	drm_dp_clear_payload_status(mgr);
 
 	payload_alloc[0] = id;
 	payload_alloc[1] = payload->start_slot;
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 9308b5920780..3c4b0fb10d8b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
 
 	intel_de_write(i915, intel_dp->regs.dp_tp_status,
 		       DP_TP_STATUS_ACT_SENT);
+
+	drm_dp_clear_payload_status(&intel_dp->mst_mgr);
 }
 
 static void wait_for_act_sent(struct intel_dp *intel_dp)
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 8b9eb4db3381..2facb87624bf 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
 			   int pbn);
 
 
+int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
+
 int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
 
 
-- 
2.23.1

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

[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/6] drm/i915/tgl+: Use the correct DP_TP_* register instances in MST encoders

[Patchwork]

== Series Details ==

Series: series starting with [1/6] drm/i915/tgl+: Use the correct DP_TP_* register instances in MST encoders
URL   : https://patchwork.freedesktop.org/series/78423/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8635_full -> Patchwork_17964_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_persistence@engines-mixed-process@vcs0:
    - shard-apl:          [PASS][1] -> [FAIL][2] ([i915#1528])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-apl7/igt@gem_ctx_persistence@engines-mixed-process@vcs0.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-apl1/igt@gem_ctx_persistence@engines-mixed-process@vcs0.html

  * igt@gem_ctx_persistence@process:
    - shard-kbl:          [PASS][3] -> [DMESG-WARN][4] ([i915#93] / [i915#95]) +2 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-kbl4/igt@gem_ctx_persistence@process.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-kbl7/igt@gem_ctx_persistence@process.html

  * igt@gem_exec_whisper@basic-contexts-priority-all:
    - shard-glk:          [PASS][5] -> [DMESG-WARN][6] ([i915#118] / [i915#95])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-glk4/igt@gem_exec_whisper@basic-contexts-priority-all.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-glk1/igt@gem_exec_whisper@basic-contexts-priority-all.html

  * igt@gem_workarounds@basic-read:
    - shard-tglb:         [PASS][7] -> [DMESG-WARN][8] ([i915#402])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-tglb3/igt@gem_workarounds@basic-read.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-tglb7/igt@gem_workarounds@basic-read.html

  * igt@i915_pm_rps@waitboost:
    - shard-glk:          [PASS][9] -> [FAIL][10] ([i915#39])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-glk7/igt@i915_pm_rps@waitboost.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-glk6/igt@i915_pm_rps@waitboost.html

  * igt@kms_big_fb@x-tiled-64bpp-rotate-180:
    - shard-glk:          [PASS][11] -> [DMESG-FAIL][12] ([i915#118] / [i915#95])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-glk2/igt@kms_big_fb@x-tiled-64bpp-rotate-180.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-glk8/igt@kms_big_fb@x-tiled-64bpp-rotate-180.html

  * igt@kms_draw_crc@draw-method-xrgb2101010-mmap-gtt-xtiled:
    - shard-apl:          [PASS][13] -> [DMESG-WARN][14] ([i915#1982])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-apl1/igt@kms_draw_crc@draw-method-xrgb2101010-mmap-gtt-xtiled.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-apl4/igt@kms_draw_crc@draw-method-xrgb2101010-mmap-gtt-xtiled.html

  * igt@kms_flip@flip-vs-blocking-wf-vblank@b-edp1:
    - shard-skl:          [PASS][15] -> [FAIL][16] ([i915#1928])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-skl1/igt@kms_flip@flip-vs-blocking-wf-vblank@b-edp1.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-skl2/igt@kms_flip@flip-vs-blocking-wf-vblank@b-edp1.html

  * igt@kms_flip_tiling@flip-changes-tiling:
    - shard-skl:          [PASS][17] -> [DMESG-WARN][18] ([i915#1982]) +10 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-skl8/igt@kms_flip_tiling@flip-changes-tiling.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-skl2/igt@kms_flip_tiling@flip-changes-tiling.html

  * igt@kms_flip_tiling@flip-changes-tiling-y:
    - shard-apl:          [PASS][19] -> [DMESG-FAIL][20] ([i915#95])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-apl2/igt@kms_flip_tiling@flip-changes-tiling-y.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-apl2/igt@kms_flip_tiling@flip-changes-tiling-y.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite:
    - shard-apl:          [PASS][21] -> [DMESG-WARN][22] ([i915#95]) +10 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-apl8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-apl7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@psr-farfromfence:
    - shard-tglb:         [PASS][23] -> [SKIP][24] ([i915#668]) +5 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-tglb3/igt@kms_frontbuffer_tracking@psr-farfromfence.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-tglb6/igt@kms_frontbuffer_tracking@psr-farfromfence.html

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([fdo#108145] / [i915#265])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-skl10/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-skl1/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html

  * igt@kms_psr@psr2_cursor_plane_move:
    - shard-iclb:         [PASS][27] -> [SKIP][28] ([fdo#109441]) +1 similar issue
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-iclb2/igt@kms_psr@psr2_cursor_plane_move.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-iclb4/igt@kms_psr@psr2_cursor_plane_move.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-kbl:          [PASS][29] -> [DMESG-WARN][30] ([i915#180]) +6 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-kbl2/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-kbl3/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  * igt@perf_pmu@semaphore-busy@rcs0:
    - shard-kbl:          [PASS][31] -> [FAIL][32] ([i915#1820])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-kbl6/igt@perf_pmu@semaphore-busy@rcs0.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-kbl7/igt@perf_pmu@semaphore-busy@rcs0.html

  
#### Possible fixes ####

  * igt@gem_ctx_freq@sysfs:
    - shard-apl:          [DMESG-WARN][33] ([i915#95]) -> [PASS][34] +18 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-apl7/igt@gem_ctx_freq@sysfs.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-apl1/igt@gem_ctx_freq@sysfs.html

  * igt@gem_exec_reloc@basic-concurrent0:
    - shard-glk:          [FAIL][35] ([i915#1930]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-glk8/igt@gem_exec_reloc@basic-concurrent0.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-glk4/igt@gem_exec_reloc@basic-concurrent0.html

  * igt@gem_exec_schedule@implicit-read-write@rcs0:
    - shard-snb:          [INCOMPLETE][37] ([i915#82]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-snb1/igt@gem_exec_schedule@implicit-read-write@rcs0.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-snb4/igt@gem_exec_schedule@implicit-read-write@rcs0.html

  * igt@gem_exec_schedule@smoketest@bcs0:
    - shard-tglb:         [INCOMPLETE][39] ([i915#1829]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-tglb1/igt@gem_exec_schedule@smoketest@bcs0.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-tglb1/igt@gem_exec_schedule@smoketest@bcs0.html

  * igt@gem_exec_whisper@basic-queues-priority:
    - shard-glk:          [DMESG-WARN][41] ([i915#118] / [i915#95]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-glk2/igt@gem_exec_whisper@basic-queues-priority.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-glk8/igt@gem_exec_whisper@basic-queues-priority.html

  * igt@i915_module_load@reload:
    - shard-tglb:         [DMESG-WARN][43] ([i915#402]) -> [PASS][44] +2 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-tglb8/igt@i915_module_load@reload.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-tglb2/igt@i915_module_load@reload.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic:
    - shard-skl:          [FAIL][45] ([IGT#5]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-skl6/igt@kms_cursor_legacy@flip-vs-cursor-atomic.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-skl10/igt@kms_cursor_legacy@flip-vs-cursor-atomic.html

  * igt@kms_flip@flip-vs-absolute-wf_vblank@a-edp1:
    - shard-tglb:         [FAIL][47] ([i915#1928]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-tglb3/igt@kms_flip@flip-vs-absolute-wf_vblank@a-edp1.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-tglb6/igt@kms_flip@flip-vs-absolute-wf_vblank@a-edp1.html

  * igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a1:
    - shard-glk:          [FAIL][49] ([i915#79]) -> [PASS][50] +1 similar issue
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-glk7/igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a1.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-glk6/igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a1.html

  * igt@kms_flip@flip-vs-suspend-interruptible@a-dp1:
    - shard-kbl:          [DMESG-WARN][51] ([i915#180]) -> [PASS][52] +2 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-kbl2/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-kbl3/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html

  * igt@kms_flip@modeset-vs-vblank-race-interruptible@a-dp1:
    - shard-kbl:          [DMESG-WARN][53] ([i915#165] / [i915#78]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-kbl2/igt@kms_flip@modeset-vs-vblank-race-interruptible@a-dp1.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-kbl3/igt@kms_flip@modeset-vs-vblank-race-interruptible@a-dp1.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-pgflip-blt:
    - shard-apl:          [DMESG-WARN][55] ([i915#1982]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-apl7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-pgflip-blt.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-apl1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-pgflip-blt.html

  * igt@kms_plane@plane-position-covered-pipe-b-planes:
    - shard-skl:          [DMESG-WARN][57] ([i915#1982]) -> [PASS][58] +6 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-skl4/igt@kms_plane@plane-position-covered-pipe-b-planes.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-skl5/igt@kms_plane@plane-position-covered-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [FAIL][59] ([fdo#108145] / [i915#265]) -> [PASS][60] +2 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-skl5/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [SKIP][61] ([fdo#109441]) -> [PASS][62] +1 similar issue
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-iclb5/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html

  * igt@perf@blocking-parameterized:
    - shard-iclb:         [FAIL][63] ([i915#1542]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-iclb8/igt@perf@blocking-parameterized.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-iclb5/igt@perf@blocking-parameterized.html

  
#### Warnings ####

  * igt@i915_suspend@fence-restore-untiled:
    - shard-kbl:          [DMESG-WARN][65] ([i915#180]) -> [INCOMPLETE][66] ([i915#155])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-kbl7/igt@i915_suspend@fence-restore-untiled.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-kbl4/igt@i915_suspend@fence-restore-untiled.html

  * igt@kms_color@pipe-c-ctm-0-25:
    - shard-tglb:         [DMESG-FAIL][67] ([i915#1149] / [i915#1982]) -> [FAIL][68] ([i915#1149] / [i915#315])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-tglb6/igt@kms_color@pipe-c-ctm-0-25.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-tglb6/igt@kms_color@pipe-c-ctm-0-25.html

  * igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
    - shard-skl:          [FAIL][69] ([fdo#108145] / [i915#265]) -> [DMESG-FAIL][70] ([fdo#108145] / [i915#1982])
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-skl8/igt@kms_plane_alpha_blend@pipe-b-alpha-basic.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-skl2/igt@kms_plane_alpha_blend@pipe-b-alpha-basic.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [IGT#5]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/5
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [i915#1149]: https://gitlab.freedesktop.org/drm/intel/issues/1149
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1528]: https://gitlab.freedesktop.org/drm/intel/issues/1528
  [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
  [i915#155]: https://gitlab.freedesktop.org/drm/intel/issues/155
  [i915#165]: https://gitlab.freedesktop.org/drm/intel/issues/165
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1820]: https://gitlab.freedesktop.org/drm/intel/issues/1820
  [i915#1829]: https://gitlab.freedesktop.org/drm/intel/issues/1829
  [i915#1928]: https://gitlab.freedesktop.org/drm/intel/issues/1928
  [i915#1930]: https://gitlab.freedesktop.org/drm/intel/issues/1930
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#315]: https://gitlab.freedesktop.org/drm/intel/issues/315
  [i915#39]: https://gitlab.freedesktop.org/drm/intel/issues/39
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#668]: https://gitlab.freedesktop.org/drm/intel/issues/668
  [i915#78]: https://gitlab.freedesktop.org/drm/intel/issues/78
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
  [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (11 -> 10)
------------------------------

  Missing    (1): pig-icl-1065g7 


Build changes
-------------

  * Linux: CI_DRM_8635 -> Patchwork_17964

  CI-20190529: 20190529
  CI_DRM_8635: f9acdb898773f94ac1bcb9a8826596f88412a53b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5711: 90611a0c90afa4a46496c78a4faf9638a1538ac3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17964: d302e0a23522a808ab7073bf458e7b70df70def3 @ 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_17964/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 1/6] drm/i915/tgl+: Use the correct DP_TP_* register instances in MST encoders

[Souza, Jose]

On Wed, 2020-06-17 at 00:11 +0300, Imre Deak wrote:
> MST encoders must use the master MST transcoder's DP_TP_STATUS and
> DP_TP_CONTROL registers. Atm, during the HW readout of an MST encoder
> connected to a slave transcoder we reset these register addresses in
> intel_dp::regs.dp_tp_* to the slave transcoder's DP_TP_* register
> addresses incorrectly; fix this.
> 
> One example where the above overwite happens is the encoder HW state
> validation after enabling multiple streams; see
> intel_dp_mst_enc_get_config(). After that during disabling any stream
> we'll get a
> 
> 'Timed out waiting for ACT sent when disabling'
> 
> error, due to reading from the incorrect DP_TP_STATUS register.
> 
> This change replaces
> https://patchwork.freedesktop.org/patch/369577/?series=78193&rev=1
> which just papered over the problem.
> 
> v2:
> - Correct the failure scenario in the commit log. (José)
> 

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index ca7bb2294d2b..73d6cc29291a 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4193,11 +4193,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>  	if (drm_WARN_ON(&dev_priv->drm, transcoder_is_dsi(cpu_transcoder)))
>  		return;
>  
> -	if (INTEL_GEN(dev_priv) >= 12) {
> -		intel_dp->regs.dp_tp_ctl = TGL_DP_TP_CTL(cpu_transcoder);
> -		intel_dp->regs.dp_tp_status = TGL_DP_TP_STATUS(cpu_transcoder);
> -	}
> -
>  	intel_dsc_get_config(encoder, pipe_config);
>  
>  	temp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder));
> @@ -4299,6 +4294,16 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>  		break;
>  	}
>  
> +	if (INTEL_GEN(dev_priv) >= 12) {
> +		enum transcoder transcoder =
> +			intel_dp_mst_is_slave_trans(pipe_config) ?
> +			pipe_config->mst_master_transcoder :
> +			pipe_config->cpu_transcoder;
> +
> +		intel_dp->regs.dp_tp_ctl = TGL_DP_TP_CTL(transcoder);
> +		intel_dp->regs.dp_tp_status = TGL_DP_TP_STATUS(transcoder);
> +	}
> +
>  	pipe_config->has_audio =
>  		intel_ddi_is_audio_enabled(dev_priv, cpu_transcoder);
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/6] drm/i915/tgl+: Use the correct DP_TP_* register instances in MST encoders (rev4)

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/6] drm/i915/tgl+: Use the correct DP_TP_* register instances in MST encoders (rev4)
URL   : https://patchwork.freedesktop.org/series/78423/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8636 -> Patchwork_17971
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17971/index.html

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-bsw-kefka:       [PASS][1] -> [DMESG-WARN][2] ([i915#1982])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8636/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17971/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-icl-u2:          [PASS][3] -> [FAIL][4] ([i915#262])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8636/fi-icl-u2/igt@kms_chamelium@dp-crc-fast.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17971/fi-icl-u2/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - fi-icl-guc:         [PASS][5] -> [DMESG-WARN][6] ([i915#1982])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8636/fi-icl-guc/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17971/fi-icl-guc/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_cursor_legacy@basic-flip-before-cursor-atomic:
    - fi-icl-u2:          [PASS][7] -> [DMESG-WARN][8] ([i915#1982]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8636/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-before-cursor-atomic.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17971/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-before-cursor-atomic.html

  
#### Possible fixes ####

  * igt@i915_pm_backlight@basic-brightness:
    - fi-whl-u:           [DMESG-WARN][9] ([i915#95]) -> [PASS][10] +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8636/fi-whl-u/igt@i915_pm_backlight@basic-brightness.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17971/fi-whl-u/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_pm_rpm@module-reload:
    - fi-glk-dsi:         [DMESG-WARN][11] ([i915#1982]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8636/fi-glk-dsi/igt@i915_pm_rpm@module-reload.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17971/fi-glk-dsi/igt@i915_pm_rpm@module-reload.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][13] ([i915#227]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8636/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17971/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-icl-u2:          [DMESG-WARN][15] ([i915#1982]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8636/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17971/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  
#### Warnings ####

  * igt@i915_pm_rpm@basic-rte:
    - fi-kbl-guc:         [FAIL][17] ([i915#665] / [i915#704]) -> [SKIP][18] ([fdo#109271])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8636/fi-kbl-guc/igt@i915_pm_rpm@basic-rte.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17971/fi-kbl-guc/igt@i915_pm_rpm@basic-rte.html

  * igt@kms_cursor_legacy@basic-flip-before-cursor-varying-size:
    - fi-kbl-x1275:       [DMESG-WARN][19] ([i915#62] / [i915#92]) -> [DMESG-WARN][20] ([i915#62] / [i915#92] / [i915#95])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8636/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-before-cursor-varying-size.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17971/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-before-cursor-varying-size.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@a-dp1:
    - fi-kbl-x1275:       [DMESG-WARN][21] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][22] ([i915#62] / [i915#92]) +2 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8636/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-wf_vblank@a-dp1.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17971/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-wf_vblank@a-dp1.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1569]: https://gitlab.freedesktop.org/drm/intel/issues/1569
  [i915#192]: https://gitlab.freedesktop.org/drm/intel/issues/192
  [i915#193]: https://gitlab.freedesktop.org/drm/intel/issues/193
  [i915#194]: https://gitlab.freedesktop.org/drm/intel/issues/194
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2029]: https://gitlab.freedesktop.org/drm/intel/issues/2029
  [i915#227]: https://gitlab.freedesktop.org/drm/intel/issues/227
  [i915#262]: https://gitlab.freedesktop.org/drm/intel/issues/262
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#665]: https://gitlab.freedesktop.org/drm/intel/issues/665
  [i915#704]: https://gitlab.freedesktop.org/drm/intel/issues/704
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (47 -> 42)
------------------------------

  Missing    (5): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_8636 -> Patchwork_17971

  CI-20190529: 20190529
  CI_DRM_8636: dd73f1f0cf1ea35520ff8267e59159be8c884e23 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5711: 90611a0c90afa4a46496c78a4faf9638a1538ac3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17971: 448ae11c4b10098f824f972c5e23f8e35845e27e @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

448ae11c4b10 drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it
c836c00b27b7 drm/i915/dp_mst: Clear the ACT sent flag during encoder disabling too
129b4b528789 drm/i915/dp_mst: Clear only the ACT sent flag from DP_TP_STATUS
8ded6cb7fc67 drm/i915/dp_mst: Move clearing the ACT sent flag closer to its polling
d5591a33514f drm/i915/dp_mst: Disable link training fallback on MST links
6182299efa2a drm/i915/tgl+: Use the correct DP_TP_* register instances in MST encoders

== Logs ==

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

Re: [PATCH v2 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

[Lyude Paul]

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

Thanks for all the subtle fixes for broken MST displays, these are always my
favorite to find :)

On Wed, 2020-06-17 at 00:11 +0300, Imre Deak wrote:
> Atm, we clear the ACT sent flag in the sink's DPCD before updating the
> sink's payload table, along clearing the payload table updated flag.
> The sink is supposed to set this flag once it detects that the source
> has completed the ACT sequence (after detecting the 4 required ACT MTPH
> symbols sent by the source). As opposed to this 2 DELL monitors I have
> set the flag already along the payload table updated flag, which is not
> quite correct.
> 
> To be sure that the sink has detected the ACT MTPH symbols before
> continuing enabling the encoder, clear the ACT sent flag before enabling
> or disabling the transcoder VC payload allocation (which is what starts
> the ACT sequence).
> 
> v2 (Ville):
> - Use the correct bit to clear the flags.
> - Add code comment explaining the clearing semantics of the ACT handled
>   flag.
> 
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c       | 38 +++++++++++++++++++--
>  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
>  include/drm/drm_dp_mst_helper.h             |  2 ++
>  3 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index b2f5a84b4cfb..1f5d14128c1a 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4377,6 +4377,41 @@ void drm_dp_mst_deallocate_vcpi(struct
> drm_dp_mst_topology_mgr *mgr,
>  }
>  EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
>  
> +/**
> + * drm_dp_clear_payload_status() - Clears the payload table status flags
> + * @mgr: manager to use
> + *
> + * Clears the payload table ACT handled and table updated flags in the MST
> hub's
> + * DPCD. This function must be called before updating the payload table or
> + * starting the ACT sequence and waiting for the corresponding flags to get
> + * set by the hub.
> + *
> + * Returns:
> + * 0 if the flags got cleared successfully, otherwise a negative error
> code.
> + */
> +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
> +{
> +	int ret;
> +
> +	/*
> +	 * Note that the following is based on the DP Standard stating that
> +	 * writing the DP_PAYLOAD_TABLE_UPDATED bit alone will clear both the
> +	 * DP_PAYLOAD_TABLE_UPDATED and the DP_PAYLOAD_ACT_HANDLED flags. This
> +	 * seems to be also the only way to clear DP_PAYLOAD_ACT_HANDLED.
> +	 */
> +	ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> +				 DP_PAYLOAD_TABLE_UPDATED);
> +	if (ret < 0) {
> +		DRM_DEBUG_DRIVER("Can't clear the ACT handled/table updated
> flags (%d)\n",
> +				 ret);
> +		return ret;
> +	}
> +	WARN_ON(ret != 1);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_dp_clear_payload_status);
> +
>  static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
>  				     int id, struct drm_dp_payload *payload)
>  {
> @@ -4384,8 +4419,7 @@ static int drm_dp_dpcd_write_payload(struct
> drm_dp_mst_topology_mgr *mgr,
>  	int ret;
>  	int retries = 0;
>  
> -	drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> -			   DP_PAYLOAD_TABLE_UPDATED);
> +	drm_dp_clear_payload_status(mgr);
>  
>  	payload_alloc[0] = id;
>  	payload_alloc[1] = payload->start_slot;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 9308b5920780..3c4b0fb10d8b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
>  
>  	intel_de_write(i915, intel_dp->regs.dp_tp_status,
>  		       DP_TP_STATUS_ACT_SENT);
> +
> +	drm_dp_clear_payload_status(&intel_dp->mst_mgr);
>  }
>  
>  static void wait_for_act_sent(struct intel_dp *intel_dp)
> diff --git a/include/drm/drm_dp_mst_helper.h
> b/include/drm/drm_dp_mst_helper.h
> index 8b9eb4db3381..2facb87624bf 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct
> drm_dp_mst_topology_mgr *mgr,
>  			   int pbn);
>  
>  
> +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
> +
>  int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
>  
>  
-- 
Cheers,
	Lyude Paul (she/her)
	Associate Software Engineer at Red Hat

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

Re: [Intel-gfx] [PATCH v2 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

[Lyude Paul]

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

Thanks for all the subtle fixes for broken MST displays, these are always my
favorite to find :)

On Wed, 2020-06-17 at 00:11 +0300, Imre Deak wrote:
> Atm, we clear the ACT sent flag in the sink's DPCD before updating the
> sink's payload table, along clearing the payload table updated flag.
> The sink is supposed to set this flag once it detects that the source
> has completed the ACT sequence (after detecting the 4 required ACT MTPH
> symbols sent by the source). As opposed to this 2 DELL monitors I have
> set the flag already along the payload table updated flag, which is not
> quite correct.
> 
> To be sure that the sink has detected the ACT MTPH symbols before
> continuing enabling the encoder, clear the ACT sent flag before enabling
> or disabling the transcoder VC payload allocation (which is what starts
> the ACT sequence).
> 
> v2 (Ville):
> - Use the correct bit to clear the flags.
> - Add code comment explaining the clearing semantics of the ACT handled
>   flag.
> 
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c       | 38 +++++++++++++++++++--
>  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
>  include/drm/drm_dp_mst_helper.h             |  2 ++
>  3 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index b2f5a84b4cfb..1f5d14128c1a 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4377,6 +4377,41 @@ void drm_dp_mst_deallocate_vcpi(struct
> drm_dp_mst_topology_mgr *mgr,
>  }
>  EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
>  
> +/**
> + * drm_dp_clear_payload_status() - Clears the payload table status flags
> + * @mgr: manager to use
> + *
> + * Clears the payload table ACT handled and table updated flags in the MST
> hub's
> + * DPCD. This function must be called before updating the payload table or
> + * starting the ACT sequence and waiting for the corresponding flags to get
> + * set by the hub.
> + *
> + * Returns:
> + * 0 if the flags got cleared successfully, otherwise a negative error
> code.
> + */
> +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
> +{
> +	int ret;
> +
> +	/*
> +	 * Note that the following is based on the DP Standard stating that
> +	 * writing the DP_PAYLOAD_TABLE_UPDATED bit alone will clear both the
> +	 * DP_PAYLOAD_TABLE_UPDATED and the DP_PAYLOAD_ACT_HANDLED flags. This
> +	 * seems to be also the only way to clear DP_PAYLOAD_ACT_HANDLED.
> +	 */
> +	ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> +				 DP_PAYLOAD_TABLE_UPDATED);
> +	if (ret < 0) {
> +		DRM_DEBUG_DRIVER("Can't clear the ACT handled/table updated
> flags (%d)\n",
> +				 ret);
> +		return ret;
> +	}
> +	WARN_ON(ret != 1);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_dp_clear_payload_status);
> +
>  static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
>  				     int id, struct drm_dp_payload *payload)
>  {
> @@ -4384,8 +4419,7 @@ static int drm_dp_dpcd_write_payload(struct
> drm_dp_mst_topology_mgr *mgr,
>  	int ret;
>  	int retries = 0;
>  
> -	drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> -			   DP_PAYLOAD_TABLE_UPDATED);
> +	drm_dp_clear_payload_status(mgr);
>  
>  	payload_alloc[0] = id;
>  	payload_alloc[1] = payload->start_slot;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 9308b5920780..3c4b0fb10d8b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
>  
>  	intel_de_write(i915, intel_dp->regs.dp_tp_status,
>  		       DP_TP_STATUS_ACT_SENT);
> +
> +	drm_dp_clear_payload_status(&intel_dp->mst_mgr);
>  }
>  
>  static void wait_for_act_sent(struct intel_dp *intel_dp)
> diff --git a/include/drm/drm_dp_mst_helper.h
> b/include/drm/drm_dp_mst_helper.h
> index 8b9eb4db3381..2facb87624bf 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct
> drm_dp_mst_topology_mgr *mgr,
>  			   int pbn);
>  
>  
> +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
> +
>  int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
>  
>  
-- 
Cheers,
	Lyude Paul (she/her)
	Associate Software Engineer at Red Hat

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

Re: [Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/6] drm/i915/tgl+: Use the correct DP_TP_* register instances in MST encoders

[Imre Deak]

On Tue, Jun 16, 2020 at 10:16:29PM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/6] drm/i915/tgl+: Use the correct DP_TP_* register instances in MST encoders
> URL   : https://patchwork.freedesktop.org/series/78423/
> State : success

Patches 1-5 pushed to -dinq, thanks for the reviews.

> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_8635_full -> Patchwork_17964_full
> ====================================================
> 
> Summary
> -------
> 
>   **SUCCESS**
> 
>   No regressions found.
> 
>   
> 
> Known issues
> ------------
> 
>   Here are the changes found in Patchwork_17964_full that come from known issues:
> 
> ### IGT changes ###
> 
> #### Issues hit ####
> 
>   * igt@gem_ctx_persistence@engines-mixed-process@vcs0:
>     - shard-apl:          [PASS][1] -> [FAIL][2] ([i915#1528])
>    [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-apl7/igt@gem_ctx_persistence@engines-mixed-process@vcs0.html
>    [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-apl1/igt@gem_ctx_persistence@engines-mixed-process@vcs0.html
> 
>   * igt@gem_ctx_persistence@process:
>     - shard-kbl:          [PASS][3] -> [DMESG-WARN][4] ([i915#93] / [i915#95]) +2 similar issues
>    [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-kbl4/igt@gem_ctx_persistence@process.html
>    [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-kbl7/igt@gem_ctx_persistence@process.html
> 
>   * igt@gem_exec_whisper@basic-contexts-priority-all:
>     - shard-glk:          [PASS][5] -> [DMESG-WARN][6] ([i915#118] / [i915#95])
>    [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-glk4/igt@gem_exec_whisper@basic-contexts-priority-all.html
>    [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-glk1/igt@gem_exec_whisper@basic-contexts-priority-all.html
> 
>   * igt@gem_workarounds@basic-read:
>     - shard-tglb:         [PASS][7] -> [DMESG-WARN][8] ([i915#402])
>    [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-tglb3/igt@gem_workarounds@basic-read.html
>    [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-tglb7/igt@gem_workarounds@basic-read.html
> 
>   * igt@i915_pm_rps@waitboost:
>     - shard-glk:          [PASS][9] -> [FAIL][10] ([i915#39])
>    [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-glk7/igt@i915_pm_rps@waitboost.html
>    [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-glk6/igt@i915_pm_rps@waitboost.html
> 
>   * igt@kms_big_fb@x-tiled-64bpp-rotate-180:
>     - shard-glk:          [PASS][11] -> [DMESG-FAIL][12] ([i915#118] / [i915#95])
>    [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-glk2/igt@kms_big_fb@x-tiled-64bpp-rotate-180.html
>    [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-glk8/igt@kms_big_fb@x-tiled-64bpp-rotate-180.html
> 
>   * igt@kms_draw_crc@draw-method-xrgb2101010-mmap-gtt-xtiled:
>     - shard-apl:          [PASS][13] -> [DMESG-WARN][14] ([i915#1982])
>    [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-apl1/igt@kms_draw_crc@draw-method-xrgb2101010-mmap-gtt-xtiled.html
>    [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-apl4/igt@kms_draw_crc@draw-method-xrgb2101010-mmap-gtt-xtiled.html
> 
>   * igt@kms_flip@flip-vs-blocking-wf-vblank@b-edp1:
>     - shard-skl:          [PASS][15] -> [FAIL][16] ([i915#1928])
>    [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-skl1/igt@kms_flip@flip-vs-blocking-wf-vblank@b-edp1.html
>    [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-skl2/igt@kms_flip@flip-vs-blocking-wf-vblank@b-edp1.html
> 
>   * igt@kms_flip_tiling@flip-changes-tiling:
>     - shard-skl:          [PASS][17] -> [DMESG-WARN][18] ([i915#1982]) +10 similar issues
>    [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-skl8/igt@kms_flip_tiling@flip-changes-tiling.html
>    [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-skl2/igt@kms_flip_tiling@flip-changes-tiling.html
> 
>   * igt@kms_flip_tiling@flip-changes-tiling-y:
>     - shard-apl:          [PASS][19] -> [DMESG-FAIL][20] ([i915#95])
>    [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-apl2/igt@kms_flip_tiling@flip-changes-tiling-y.html
>    [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-apl2/igt@kms_flip_tiling@flip-changes-tiling-y.html
> 
>   * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite:
>     - shard-apl:          [PASS][21] -> [DMESG-WARN][22] ([i915#95]) +10 similar issues
>    [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-apl8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite.html
>    [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-apl7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite.html
> 
>   * igt@kms_frontbuffer_tracking@psr-farfromfence:
>     - shard-tglb:         [PASS][23] -> [SKIP][24] ([i915#668]) +5 similar issues
>    [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-tglb3/igt@kms_frontbuffer_tracking@psr-farfromfence.html
>    [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-tglb6/igt@kms_frontbuffer_tracking@psr-farfromfence.html
> 
>   * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
>     - shard-skl:          [PASS][25] -> [FAIL][26] ([fdo#108145] / [i915#265])
>    [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-skl10/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
>    [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-skl1/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
> 
>   * igt@kms_psr@psr2_cursor_plane_move:
>     - shard-iclb:         [PASS][27] -> [SKIP][28] ([fdo#109441]) +1 similar issue
>    [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-iclb2/igt@kms_psr@psr2_cursor_plane_move.html
>    [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-iclb4/igt@kms_psr@psr2_cursor_plane_move.html
> 
>   * igt@kms_vblank@pipe-a-ts-continuation-suspend:
>     - shard-kbl:          [PASS][29] -> [DMESG-WARN][30] ([i915#180]) +6 similar issues
>    [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-kbl2/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
>    [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-kbl3/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
> 
>   * igt@perf_pmu@semaphore-busy@rcs0:
>     - shard-kbl:          [PASS][31] -> [FAIL][32] ([i915#1820])
>    [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-kbl6/igt@perf_pmu@semaphore-busy@rcs0.html
>    [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-kbl7/igt@perf_pmu@semaphore-busy@rcs0.html
> 
>   
> #### Possible fixes ####
> 
>   * igt@gem_ctx_freq@sysfs:
>     - shard-apl:          [DMESG-WARN][33] ([i915#95]) -> [PASS][34] +18 similar issues
>    [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-apl7/igt@gem_ctx_freq@sysfs.html
>    [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-apl1/igt@gem_ctx_freq@sysfs.html
> 
>   * igt@gem_exec_reloc@basic-concurrent0:
>     - shard-glk:          [FAIL][35] ([i915#1930]) -> [PASS][36]
>    [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-glk8/igt@gem_exec_reloc@basic-concurrent0.html
>    [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-glk4/igt@gem_exec_reloc@basic-concurrent0.html
> 
>   * igt@gem_exec_schedule@implicit-read-write@rcs0:
>     - shard-snb:          [INCOMPLETE][37] ([i915#82]) -> [PASS][38]
>    [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-snb1/igt@gem_exec_schedule@implicit-read-write@rcs0.html
>    [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-snb4/igt@gem_exec_schedule@implicit-read-write@rcs0.html
> 
>   * igt@gem_exec_schedule@smoketest@bcs0:
>     - shard-tglb:         [INCOMPLETE][39] ([i915#1829]) -> [PASS][40]
>    [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-tglb1/igt@gem_exec_schedule@smoketest@bcs0.html
>    [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-tglb1/igt@gem_exec_schedule@smoketest@bcs0.html
> 
>   * igt@gem_exec_whisper@basic-queues-priority:
>     - shard-glk:          [DMESG-WARN][41] ([i915#118] / [i915#95]) -> [PASS][42]
>    [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-glk2/igt@gem_exec_whisper@basic-queues-priority.html
>    [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-glk8/igt@gem_exec_whisper@basic-queues-priority.html
> 
>   * igt@i915_module_load@reload:
>     - shard-tglb:         [DMESG-WARN][43] ([i915#402]) -> [PASS][44] +2 similar issues
>    [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-tglb8/igt@i915_module_load@reload.html
>    [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-tglb2/igt@i915_module_load@reload.html
> 
>   * igt@kms_cursor_legacy@flip-vs-cursor-atomic:
>     - shard-skl:          [FAIL][45] ([IGT#5]) -> [PASS][46]
>    [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-skl6/igt@kms_cursor_legacy@flip-vs-cursor-atomic.html
>    [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-skl10/igt@kms_cursor_legacy@flip-vs-cursor-atomic.html
> 
>   * igt@kms_flip@flip-vs-absolute-wf_vblank@a-edp1:
>     - shard-tglb:         [FAIL][47] ([i915#1928]) -> [PASS][48]
>    [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-tglb3/igt@kms_flip@flip-vs-absolute-wf_vblank@a-edp1.html
>    [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-tglb6/igt@kms_flip@flip-vs-absolute-wf_vblank@a-edp1.html
> 
>   * igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a1:
>     - shard-glk:          [FAIL][49] ([i915#79]) -> [PASS][50] +1 similar issue
>    [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-glk7/igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a1.html
>    [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-glk6/igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a1.html
> 
>   * igt@kms_flip@flip-vs-suspend-interruptible@a-dp1:
>     - shard-kbl:          [DMESG-WARN][51] ([i915#180]) -> [PASS][52] +2 similar issues
>    [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-kbl2/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
>    [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-kbl3/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
> 
>   * igt@kms_flip@modeset-vs-vblank-race-interruptible@a-dp1:
>     - shard-kbl:          [DMESG-WARN][53] ([i915#165] / [i915#78]) -> [PASS][54]
>    [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-kbl2/igt@kms_flip@modeset-vs-vblank-race-interruptible@a-dp1.html
>    [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-kbl3/igt@kms_flip@modeset-vs-vblank-race-interruptible@a-dp1.html
> 
>   * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-pgflip-blt:
>     - shard-apl:          [DMESG-WARN][55] ([i915#1982]) -> [PASS][56]
>    [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-apl7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-pgflip-blt.html
>    [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-apl1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-pgflip-blt.html
> 
>   * igt@kms_plane@plane-position-covered-pipe-b-planes:
>     - shard-skl:          [DMESG-WARN][57] ([i915#1982]) -> [PASS][58] +6 similar issues
>    [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-skl4/igt@kms_plane@plane-position-covered-pipe-b-planes.html
>    [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-skl5/igt@kms_plane@plane-position-covered-pipe-b-planes.html
> 
>   * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
>     - shard-skl:          [FAIL][59] ([fdo#108145] / [i915#265]) -> [PASS][60] +2 similar issues
>    [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-skl5/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
>    [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
> 
>   * igt@kms_psr@psr2_sprite_mmap_gtt:
>     - shard-iclb:         [SKIP][61] ([fdo#109441]) -> [PASS][62] +1 similar issue
>    [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-iclb5/igt@kms_psr@psr2_sprite_mmap_gtt.html
>    [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html
> 
>   * igt@perf@blocking-parameterized:
>     - shard-iclb:         [FAIL][63] ([i915#1542]) -> [PASS][64]
>    [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-iclb8/igt@perf@blocking-parameterized.html
>    [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-iclb5/igt@perf@blocking-parameterized.html
> 
>   
> #### Warnings ####
> 
>   * igt@i915_suspend@fence-restore-untiled:
>     - shard-kbl:          [DMESG-WARN][65] ([i915#180]) -> [INCOMPLETE][66] ([i915#155])
>    [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-kbl7/igt@i915_suspend@fence-restore-untiled.html
>    [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-kbl4/igt@i915_suspend@fence-restore-untiled.html
> 
>   * igt@kms_color@pipe-c-ctm-0-25:
>     - shard-tglb:         [DMESG-FAIL][67] ([i915#1149] / [i915#1982]) -> [FAIL][68] ([i915#1149] / [i915#315])
>    [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-tglb6/igt@kms_color@pipe-c-ctm-0-25.html
>    [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-tglb6/igt@kms_color@pipe-c-ctm-0-25.html
> 
>   * igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
>     - shard-skl:          [FAIL][69] ([fdo#108145] / [i915#265]) -> [DMESG-FAIL][70] ([fdo#108145] / [i915#1982])
>    [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8635/shard-skl8/igt@kms_plane_alpha_blend@pipe-b-alpha-basic.html
>    [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17964/shard-skl2/igt@kms_plane_alpha_blend@pipe-b-alpha-basic.html
> 
>   
>   {name}: This element is suppressed. This means it is ignored when computing
>           the status of the difference (SUCCESS, WARNING, or FAILURE).
> 
>   [IGT#5]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/5
>   [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
>   [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
>   [i915#1149]: https://gitlab.freedesktop.org/drm/intel/issues/1149
>   [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
>   [i915#1528]: https://gitlab.freedesktop.org/drm/intel/issues/1528
>   [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
>   [i915#155]: https://gitlab.freedesktop.org/drm/intel/issues/155
>   [i915#165]: https://gitlab.freedesktop.org/drm/intel/issues/165
>   [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
>   [i915#1820]: https://gitlab.freedesktop.org/drm/intel/issues/1820
>   [i915#1829]: https://gitlab.freedesktop.org/drm/intel/issues/1829
>   [i915#1928]: https://gitlab.freedesktop.org/drm/intel/issues/1928
>   [i915#1930]: https://gitlab.freedesktop.org/drm/intel/issues/1930
>   [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
>   [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
>   [i915#315]: https://gitlab.freedesktop.org/drm/intel/issues/315
>   [i915#39]: https://gitlab.freedesktop.org/drm/intel/issues/39
>   [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
>   [i915#668]: https://gitlab.freedesktop.org/drm/intel/issues/668
>   [i915#78]: https://gitlab.freedesktop.org/drm/intel/issues/78
>   [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
>   [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
>   [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
>   [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
> 
> 
> Participating hosts (11 -> 10)
> ------------------------------
> 
>   Missing    (1): pig-icl-1065g7 
> 
> 
> Build changes
> -------------
> 
>   * Linux: CI_DRM_8635 -> Patchwork_17964
> 
>   CI-20190529: 20190529
>   CI_DRM_8635: f9acdb898773f94ac1bcb9a8826596f88412a53b @ git://anongit.freedesktop.org/gfx-ci/linux
>   IGT_5711: 90611a0c90afa4a46496c78a4faf9638a1538ac3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>   Patchwork_17964: d302e0a23522a808ab7073bf458e7b70df70def3 @ 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_17964/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

[Imre Deak]

On Wed, Jun 17, 2020 at 12:11:46AM +0300, Imre Deak wrote:
> Atm, we clear the ACT sent flag in the sink's DPCD before updating the
> sink's payload table, along clearing the payload table updated flag.
> The sink is supposed to set this flag once it detects that the source
> has completed the ACT sequence (after detecting the 4 required ACT MTPH
> symbols sent by the source). As opposed to this 2 DELL monitors I have
> set the flag already along the payload table updated flag, which is not
> quite correct.
> 
> To be sure that the sink has detected the ACT MTPH symbols before
> continuing enabling the encoder, clear the ACT sent flag before enabling
> or disabling the transcoder VC payload allocation (which is what starts
> the ACT sequence).
> 
> v2 (Ville):
> - Use the correct bit to clear the flags.
> - Add code comment explaining the clearing semantics of the ACT handled
>   flag.
> 
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Dropping this patch because clearing the ACT handled flag from DPCD
causes a problem for some sinks, which set this flag only once when the
VC payload table is updated and do not set it when the ACT symbols are
actually sent by the source.

> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c       | 38 +++++++++++++++++++--
>  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
>  include/drm/drm_dp_mst_helper.h             |  2 ++
>  3 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index b2f5a84b4cfb..1f5d14128c1a 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4377,6 +4377,41 @@ void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
>  }
>  EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
>  
> +/**
> + * drm_dp_clear_payload_status() - Clears the payload table status flags
> + * @mgr: manager to use
> + *
> + * Clears the payload table ACT handled and table updated flags in the MST hub's
> + * DPCD. This function must be called before updating the payload table or
> + * starting the ACT sequence and waiting for the corresponding flags to get
> + * set by the hub.
> + *
> + * Returns:
> + * 0 if the flags got cleared successfully, otherwise a negative error code.
> + */
> +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
> +{
> +	int ret;
> +
> +	/*
> +	 * Note that the following is based on the DP Standard stating that
> +	 * writing the DP_PAYLOAD_TABLE_UPDATED bit alone will clear both the
> +	 * DP_PAYLOAD_TABLE_UPDATED and the DP_PAYLOAD_ACT_HANDLED flags. This
> +	 * seems to be also the only way to clear DP_PAYLOAD_ACT_HANDLED.
> +	 */
> +	ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> +				 DP_PAYLOAD_TABLE_UPDATED);
> +	if (ret < 0) {
> +		DRM_DEBUG_DRIVER("Can't clear the ACT handled/table updated flags (%d)\n",
> +				 ret);
> +		return ret;
> +	}
> +	WARN_ON(ret != 1);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_dp_clear_payload_status);
> +
>  static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
>  				     int id, struct drm_dp_payload *payload)
>  {
> @@ -4384,8 +4419,7 @@ static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
>  	int ret;
>  	int retries = 0;
>  
> -	drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> -			   DP_PAYLOAD_TABLE_UPDATED);
> +	drm_dp_clear_payload_status(mgr);
>  
>  	payload_alloc[0] = id;
>  	payload_alloc[1] = payload->start_slot;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 9308b5920780..3c4b0fb10d8b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
>  
>  	intel_de_write(i915, intel_dp->regs.dp_tp_status,
>  		       DP_TP_STATUS_ACT_SENT);
> +
> +	drm_dp_clear_payload_status(&intel_dp->mst_mgr);
>  }
>  
>  static void wait_for_act_sent(struct intel_dp *intel_dp)
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index 8b9eb4db3381..2facb87624bf 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
>  			   int pbn);
>  
>  
> +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
> +
>  int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
>  
>  
> -- 
> 2.23.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

Re: [Intel-gfx] [PATCH v2 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

[Imre Deak]

On Wed, Jun 17, 2020 at 12:11:46AM +0300, Imre Deak wrote:
> Atm, we clear the ACT sent flag in the sink's DPCD before updating the
> sink's payload table, along clearing the payload table updated flag.
> The sink is supposed to set this flag once it detects that the source
> has completed the ACT sequence (after detecting the 4 required ACT MTPH
> symbols sent by the source). As opposed to this 2 DELL monitors I have
> set the flag already along the payload table updated flag, which is not
> quite correct.
> 
> To be sure that the sink has detected the ACT MTPH symbols before
> continuing enabling the encoder, clear the ACT sent flag before enabling
> or disabling the transcoder VC payload allocation (which is what starts
> the ACT sequence).
> 
> v2 (Ville):
> - Use the correct bit to clear the flags.
> - Add code comment explaining the clearing semantics of the ACT handled
>   flag.
> 
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Dropping this patch because clearing the ACT handled flag from DPCD
causes a problem for some sinks, which set this flag only once when the
VC payload table is updated and do not set it when the ACT symbols are
actually sent by the source.

> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c       | 38 +++++++++++++++++++--
>  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
>  include/drm/drm_dp_mst_helper.h             |  2 ++
>  3 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index b2f5a84b4cfb..1f5d14128c1a 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4377,6 +4377,41 @@ void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
>  }
>  EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
>  
> +/**
> + * drm_dp_clear_payload_status() - Clears the payload table status flags
> + * @mgr: manager to use
> + *
> + * Clears the payload table ACT handled and table updated flags in the MST hub's
> + * DPCD. This function must be called before updating the payload table or
> + * starting the ACT sequence and waiting for the corresponding flags to get
> + * set by the hub.
> + *
> + * Returns:
> + * 0 if the flags got cleared successfully, otherwise a negative error code.
> + */
> +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
> +{
> +	int ret;
> +
> +	/*
> +	 * Note that the following is based on the DP Standard stating that
> +	 * writing the DP_PAYLOAD_TABLE_UPDATED bit alone will clear both the
> +	 * DP_PAYLOAD_TABLE_UPDATED and the DP_PAYLOAD_ACT_HANDLED flags. This
> +	 * seems to be also the only way to clear DP_PAYLOAD_ACT_HANDLED.
> +	 */
> +	ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> +				 DP_PAYLOAD_TABLE_UPDATED);
> +	if (ret < 0) {
> +		DRM_DEBUG_DRIVER("Can't clear the ACT handled/table updated flags (%d)\n",
> +				 ret);
> +		return ret;
> +	}
> +	WARN_ON(ret != 1);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_dp_clear_payload_status);
> +
>  static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
>  				     int id, struct drm_dp_payload *payload)
>  {
> @@ -4384,8 +4419,7 @@ static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
>  	int ret;
>  	int retries = 0;
>  
> -	drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> -			   DP_PAYLOAD_TABLE_UPDATED);
> +	drm_dp_clear_payload_status(mgr);
>  
>  	payload_alloc[0] = id;
>  	payload_alloc[1] = payload->start_slot;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 9308b5920780..3c4b0fb10d8b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
>  
>  	intel_de_write(i915, intel_dp->regs.dp_tp_status,
>  		       DP_TP_STATUS_ACT_SENT);
> +
> +	drm_dp_clear_payload_status(&intel_dp->mst_mgr);
>  }
>  
>  static void wait_for_act_sent(struct intel_dp *intel_dp)
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index 8b9eb4db3381..2facb87624bf 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
>  			   int pbn);
>  
>  
> +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
> +
>  int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
>  
>  
> -- 
> 2.23.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx