* [Intel-gfx] [PATCH 1/4] drm/i915/tgl: Select master transcoder for MST stream
@ 2019-12-07 1:18 José Roberto de Souza
2019-12-07 1:18 ` [Intel-gfx] [PATCH 2/4] drm/i915/display: Always enables MST master pipe first José Roberto de Souza
` (5 more replies)
0 siblings, 6 replies; 16+ messages in thread
From: José Roberto de Souza @ 2019-12-07 1:18 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi
On TGL the blending of all the streams have moved from DDI to
transcoder, so now every transcoder working over the same MST port must
send its stream to a master transcoder and master will send to DDI
respecting the time slots.
So here adding all the CRTCs that shares the same MST stream if
needed and computing their state again, it will pick the first
transcoder among the ones in the same stream to be master.
Most of the time skl_commit_modeset_enables() enables pipes in a
crescent order but due DDB overlapping it might not happen, this
scenario will be handled in the next patch.
BSpec: 50493
BSpec: 49190
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/display/intel_ddi.c | 14 +-
drivers/gpu/drm/i915/display/intel_display.c | 15 +-
.../drm/i915/display/intel_display_types.h | 3 +
drivers/gpu/drm/i915/display/intel_dp_mst.c | 163 ++++++++++++++++++
drivers/gpu/drm/i915/display/intel_dp_mst.h | 5 +
5 files changed, 196 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 3cacb1e279c1..be5bc506d4d3 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1903,8 +1903,13 @@ intel_ddi_transcoder_func_reg_val_get(const struct intel_crtc_state *crtc_state)
temp |= TRANS_DDI_MODE_SELECT_DP_MST;
temp |= DDI_PORT_WIDTH(crtc_state->lane_count);
- if (INTEL_GEN(dev_priv) >= 12)
- temp |= TRANS_DDI_MST_TRANSPORT_SELECT(crtc_state->cpu_transcoder);
+ if (INTEL_GEN(dev_priv) >= 12) {
+ enum transcoder master;
+
+ master = crtc_state->mst_master_transcoder;
+ WARN_ON(master == INVALID_TRANSCODER);
+ temp |= TRANS_DDI_MST_TRANSPORT_SELECT(master);
+ }
} else {
temp |= TRANS_DDI_MODE_SELECT_DP_SST;
temp |= DDI_PORT_WIDTH(crtc_state->lane_count);
@@ -4372,6 +4377,11 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
pipe_config->lane_count =
((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
+
+ if (INTEL_GEN(dev_priv) >= 12)
+ pipe_config->mst_master_transcoder =
+ REG_FIELD_GET(TRANS_DDI_MST_TRANSPORT_SELECT_MASK, temp);
+
intel_dp_get_m_n(intel_crtc, pipe_config);
break;
default:
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 821ba8053f9d..f89494c849ce 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -46,6 +46,7 @@
#include "display/intel_crt.h"
#include "display/intel_ddi.h"
#include "display/intel_dp.h"
+#include "display/intel_dp_mst.h"
#include "display/intel_dsi.h"
#include "display/intel_dvo.h"
#include "display/intel_gmbus.h"
@@ -12542,6 +12543,11 @@ static void intel_dump_pipe_config(const struct intel_crtc_state *pipe_config,
pipe_config->csc_mode, pipe_config->gamma_mode,
pipe_config->gamma_enable, pipe_config->csc_enable);
+ if (INTEL_GEN(dev_priv) >= 12 &&
+ intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST))
+ DRM_DEBUG_KMS("MST master transcoder: %s\n",
+ transcoder_name(pipe_config->mst_master_transcoder));
+
dump_planes:
if (!state)
return;
@@ -13318,6 +13324,10 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
PIPE_CONF_CHECK_I(sync_mode_slaves_mask);
PIPE_CONF_CHECK_I(master_transcoder);
+ if (INTEL_GEN(dev_priv) >= 12 &&
+ intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST))
+ PIPE_CONF_CHECK_I(mst_master_transcoder);
+
#undef PIPE_CONF_CHECK_X
#undef PIPE_CONF_CHECK_I
#undef PIPE_CONF_CHECK_BOOL
@@ -14406,7 +14416,7 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
u32 handled = 0;
int i;
- /* Only disable port sync slaves */
+ /* Only disable port sync and MST slaves */
for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state, i) {
if (!needs_modeset(new_crtc_state))
@@ -14420,7 +14430,8 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
* slave CRTCs are disabled first and then master CRTC since
* Slave vblanks are masked till Master Vblanks.
*/
- if (!is_trans_port_sync_slave(old_crtc_state))
+ if (!is_trans_port_sync_slave(old_crtc_state) &&
+ !intel_dp_mst_is_slave_trans(old_crtc_state))
continue;
intel_pre_plane_update(state, crtc);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 83ea04149b77..630a94892b7b 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1054,6 +1054,9 @@ struct intel_crtc_state {
/* Bitmask to indicate slaves attached */
u8 sync_mode_slaves_mask;
+
+ /* Only valid on TGL+ */
+ enum transcoder mst_master_transcoder;
};
struct intel_crtc {
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 926e49f449a6..49f1518e3ab9 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -87,6 +87,49 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
return 0;
}
+/*
+ * Iterate over all the CRTCs and set mst_master_transcoder to the first active
+ * transcoder that shares the same MST connector.
+ *
+ * TODO: maybe this could be optimized to keep the old master transcoder
+ */
+static int
+intel_dp_mst_master_trans_compute(struct intel_encoder *encoder,
+ struct intel_crtc_state *pipe_config,
+ struct drm_connector_state *conn_state)
+{
+ struct intel_atomic_state *state = to_intel_atomic_state(pipe_config->uapi.state);
+ struct intel_connector *conn = to_intel_connector(conn_state->connector);
+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+ struct intel_digital_connector_state *iter_conn_state;
+ struct intel_connector *iter_conn;
+ int i;
+
+ if (INTEL_GEN(dev_priv) < 12)
+ return 0;
+
+ for_each_new_intel_connector_in_state(state, iter_conn,
+ iter_conn_state, i) {
+ struct intel_crtc_state *iter_crtc_state;
+ struct intel_crtc *iter_crtc;
+
+ if (conn->mst_port != iter_conn->mst_port ||
+ !iter_conn_state->base.crtc)
+ continue;
+
+ iter_crtc = to_intel_crtc(iter_conn_state->base.crtc);
+ iter_crtc_state = intel_atomic_get_new_crtc_state(state,
+ iter_crtc);
+ if (!iter_crtc_state->uapi.active)
+ continue;
+
+ pipe_config->mst_master_transcoder = iter_crtc_state->cpu_transcoder;
+ break;
+ }
+
+ return 0;
+}
+
static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
struct intel_crtc_state *pipe_config,
struct drm_connector_state *conn_state)
@@ -154,6 +197,98 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
+ ret = intel_dp_mst_master_trans_compute(encoder, pipe_config,
+ conn_state);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int
+intel_dp_mst_atomic_master_trans_check(struct drm_connector *connector,
+ struct drm_atomic_state *state)
+{
+ struct drm_i915_private *dev_priv = to_i915(connector->dev);
+ struct drm_connector_state *new_conn_state, *old_conn_state;
+ struct drm_connector_list_iter conn_list_iter;
+ struct intel_crtc_state *intel_crtc_state;
+ struct intel_connector *intel_conn;
+ struct drm_crtc_state *crtc_state;
+ struct drm_connector *conn_iter;
+
+ if (INTEL_GEN(dev_priv) < 12)
+ return 0;
+
+ intel_conn = to_intel_connector(connector);
+ new_conn_state = drm_atomic_get_new_connector_state(state, connector);
+ old_conn_state = drm_atomic_get_old_connector_state(state, connector);
+
+ if (!old_conn_state->crtc && !new_conn_state->crtc)
+ return 0;
+
+ /*
+ * 3 cases that needs be handled here:
+ * - connector going from disabled to enabled:
+ * nothing else need to be done, drm helpers already set mode_changed in
+ * the CRTCs needed
+ * - connector going from enabled to disabled:
+ * if this transcoder was the master, all slaves needs a modeset
+ * - connector going from enabled to enabled but it needs a modeset:
+ * if this transcoder was the master, all slaves also needs a modeset
+ */
+
+ /* disabled -> enabled */
+ if (!old_conn_state->crtc && new_conn_state->crtc)
+ return 0;
+
+ /* enabled -> enabled(modeset)? */
+ if (new_conn_state->crtc) {
+ crtc_state = drm_atomic_get_new_crtc_state(state,
+ new_conn_state->crtc);
+ if (!drm_atomic_crtc_needs_modeset(crtc_state))
+ return 0;
+ }
+
+ /* handling enabled -> enabled(modeset) and enabled -> disabled */
+ crtc_state = drm_atomic_get_old_crtc_state(state, old_conn_state->crtc);
+ intel_crtc_state = to_intel_crtc_state(crtc_state);
+
+ /* If not master, nothing else needs to be handled */
+ if (!intel_dp_mst_is_master_trans(intel_crtc_state))
+ return 0;
+
+ /* It is master, add and mark all other CRTCs in the stream as changed */
+ drm_connector_list_iter_begin(state->dev, &conn_list_iter);
+ drm_for_each_connector_iter(conn_iter, &conn_list_iter) {
+ struct drm_connector_state *conn_iter_state;
+ struct intel_connector *intel_conn_iter;
+
+ intel_conn_iter = to_intel_connector(conn_iter);
+ if (intel_conn_iter->mst_port != intel_conn->mst_port)
+ continue;
+
+ conn_iter_state = drm_atomic_get_connector_state(state,
+ conn_iter);
+ if (IS_ERR(conn_iter_state)) {
+ drm_connector_list_iter_end(&conn_list_iter);
+ return PTR_ERR(conn_iter_state);
+ }
+
+ if (!conn_iter_state->crtc)
+ continue;
+
+ crtc_state = drm_atomic_get_crtc_state(state,
+ conn_iter_state->crtc);
+ if (IS_ERR(crtc_state)) {
+ drm_connector_list_iter_end(&conn_list_iter);
+ return PTR_ERR(conn_iter_state);
+ }
+
+ crtc_state->mode_changed = true;
+ }
+ drm_connector_list_iter_end(&conn_list_iter);
+
return 0;
}
@@ -175,6 +310,10 @@ intel_dp_mst_atomic_check(struct drm_connector *connector,
if (ret)
return ret;
+ ret = intel_dp_mst_atomic_master_trans_check(connector, state);
+ if (ret)
+ return ret;
+
if (!old_conn_state->crtc)
return 0;
@@ -241,6 +380,9 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
intel_dp->active_mst_links--;
last_mst_stream = intel_dp->active_mst_links == 0;
+ WARN_ON(INTEL_GEN(dev_priv) >= 12 && last_mst_stream &&
+ !intel_dp_mst_is_master_trans(old_crtc_state));
+
/*
* From TGL spec: "If multi-stream slave transcoder: Configure
* Transcoder Clock Select to direct no clock to the transcoder"
@@ -321,6 +463,9 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
intel_mst->connector = connector;
first_mst_stream = intel_dp->active_mst_links == 0;
+ WARN_ON(INTEL_GEN(dev_priv) >= 12 && first_mst_stream &&
+ !intel_dp_mst_is_master_trans(pipe_config));
+
DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
if (first_mst_stream)
@@ -726,3 +871,21 @@ intel_dp_mst_encoder_cleanup(struct intel_digital_port *intel_dig_port)
drm_dp_mst_topology_mgr_destroy(&intel_dp->mst_mgr);
/* encoders will get killed by normal cleanup */
}
+
+bool intel_dp_mst_is_master_trans(const struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
+
+ return INTEL_GEN(dev_priv) >= 12 &&
+ intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST) &&
+ crtc_state->mst_master_transcoder == crtc_state->cpu_transcoder;
+}
+
+bool intel_dp_mst_is_slave_trans(const struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
+
+ return INTEL_GEN(dev_priv) >= 12 &&
+ intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST) &&
+ crtc_state->mst_master_transcoder != crtc_state->cpu_transcoder;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.h b/drivers/gpu/drm/i915/display/intel_dp_mst.h
index f660ad80db04..e40767f78343 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.h
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.h
@@ -6,10 +6,15 @@
#ifndef __INTEL_DP_MST_H__
#define __INTEL_DP_MST_H__
+#include <stdbool.h>
+
struct intel_digital_port;
+struct intel_crtc_state;
int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
void intel_dp_mst_encoder_cleanup(struct intel_digital_port *intel_dig_port);
int intel_dp_mst_encoder_active_links(struct intel_digital_port *intel_dig_port);
+bool intel_dp_mst_is_master_trans(const struct intel_crtc_state *crtc_state);
+bool intel_dp_mst_is_slave_trans(const struct intel_crtc_state *crtc_state);
#endif /* __INTEL_DP_MST_H__ */
--
2.24.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Intel-gfx] [PATCH 2/4] drm/i915/display: Always enables MST master pipe first
2019-12-07 1:18 [Intel-gfx] [PATCH 1/4] drm/i915/tgl: Select master transcoder for MST stream José Roberto de Souza
@ 2019-12-07 1:18 ` José Roberto de Souza
2019-12-09 17:19 ` Ville Syrjälä
2019-12-07 1:18 ` [Intel-gfx] [PATCH 3/4] drm/i915/dp: Fix MST disable sequences José Roberto de Souza
` (4 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: José Roberto de Souza @ 2019-12-07 1:18 UTC (permalink / raw)
To: intel-gfx
Due to DDB overlaps the pipe enabling sequence is not always crescent.
As the previous patch selects the first pipe/transcoder in the MST
stream to the MST master and it needs to be enabled first this
changes were needed to guarantee that.
So here it will first loop through all the MST masters and other
pipes that do not have pipe dependencies and enabling then, as when
the master is being enabled all the slaves are also going to a full
modeset they will not overlap with each other.
Then on the second loop it will enable all the MST slaves.
I have tried to put port sync pipes into those two loops but
intel_update_trans_port_sync_crtcs() is doing way more than just
enable pipes, reading spec I guess it could be accomplish but I will
leave it to people working on port sync.
At least now the port sync pipes are enabled by last so the slave DDB
allocation will not overlap with other pipes.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/display/intel_display.c | 120 ++++++++++++++++---
1 file changed, 105 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index f89494c849ce..2f74c0bfb2a8 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14576,6 +14576,39 @@ static void intel_update_trans_port_sync_crtcs(struct intel_crtc *crtc,
state);
}
+static void
+skl_commit_modeset_enable_pipe(struct intel_crtc *crtc,
+ struct intel_crtc_state *old_crtc_state,
+ struct intel_crtc_state *new_crtc_state,
+ unsigned int *updated, bool *progress,
+ struct skl_ddb_entry *entry)
+{
+ struct intel_atomic_state *state = to_intel_atomic_state(old_crtc_state->uapi.state);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ bool vbl_wait = false;
+
+ *updated = *updated | BIT(crtc->pipe);
+ *progress = true;
+ *entry = new_crtc_state->wm.skl.ddb;
+
+ /*
+ * If this is an already active pipe, it's DDB changed,
+ * and this isn't the last pipe that needs updating
+ * then we need to wait for a vblank to pass for the
+ * new ddb allocation to take effect.
+ */
+ if (!needs_modeset(new_crtc_state) &&
+ state->wm_results.dirty_pipes != *updated &&
+ !skl_ddb_entry_equal(&new_crtc_state->wm.skl.ddb,
+ &old_crtc_state->wm.skl.ddb))
+ vbl_wait = true;
+
+ intel_update_crtc(crtc, state, old_crtc_state, new_crtc_state);
+
+ if (vbl_wait)
+ intel_wait_for_vblank(dev_priv, crtc->pipe);
+}
+
static void skl_commit_modeset_enables(struct intel_atomic_state *state)
{
struct drm_i915_private *dev_priv = to_i915(state->base.dev);
@@ -14600,18 +14633,84 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
/*
* Whenever the number of active pipes changes, we need to make sure we
* update the pipes in the right order so that their ddb allocations
- * never overlap with eachother inbetween CRTC updates. Otherwise we'll
+ * never overlap with each other between CRTC updates. Otherwise we'll
* cause pipe underruns and other bad stuff.
+ *
+ * First enable all the pipes that do not depends on other pipes while
+ * respecting the DDB allocation overlaps.
+ *
+ * TODO: integrate port sync to the loops bellow.
+ * Port sync is not respecting the DDB allocation overlaps as it
+ * was not checking for the slave port overlaps and there is more than
+ * just a pipe enable in intel_update_trans_port_sync_crtcs()
*/
do {
progress = false;
- for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+ for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+ new_crtc_state, i) {
+ if (updated & BIT(crtc->pipe) ||
+ !new_crtc_state->hw.active)
+ continue;
+
+ if (intel_dp_mst_is_slave_trans(new_crtc_state) ||
+ is_trans_port_sync_mode(new_crtc_state))
+ continue;
+
+ if (skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
+ entries,
+ INTEL_NUM_PIPES(dev_priv), i))
+ continue;
+
+ skl_commit_modeset_enable_pipe(crtc, old_crtc_state,
+ new_crtc_state, &updated,
+ &progress, &entries[i]);
+ }
+ } while (progress);
+
+ /*
+ * Now enable all the pipes that depends on other pipe aka MST slaves
+ */
+ do {
+ progress = false;
+
+ for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+ new_crtc_state, i) {
+ if (updated & BIT(crtc->pipe) ||
+ !new_crtc_state->hw.active)
+ continue;
+
+ if (is_trans_port_sync_mode(new_crtc_state))
+ continue;
+
+ if (skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
+ entries,
+ INTEL_NUM_PIPES(dev_priv), i))
+ continue;
+
+ skl_commit_modeset_enable_pipe(crtc, old_crtc_state,
+ new_crtc_state, &updated,
+ &progress, &entries[i]);
+ }
+ } while (progress);
+
+ /* Port sync loop */
+ do {
+ progress = false;
+
+ for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+ new_crtc_state, i) {
enum pipe pipe = crtc->pipe;
bool vbl_wait = false;
bool modeset = needs_modeset(new_crtc_state);
- if (updated & BIT(crtc->pipe) || !new_crtc_state->hw.active)
+ if (updated & BIT(pipe) || !new_crtc_state->hw.active)
+ continue;
+
+ if (!is_trans_port_sync_master(new_crtc_state))
+ continue;
+
+ if (!modeset)
continue;
if (skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
@@ -14634,18 +14733,9 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
state->wm_results.dirty_pipes != updated)
vbl_wait = true;
- if (modeset && is_trans_port_sync_mode(new_crtc_state)) {
- if (is_trans_port_sync_master(new_crtc_state))
- intel_update_trans_port_sync_crtcs(crtc,
- state,
- old_crtc_state,
- new_crtc_state);
- else
- continue;
- } else {
- intel_update_crtc(crtc, state, old_crtc_state,
- new_crtc_state);
- }
+ intel_update_trans_port_sync_crtcs(crtc, state,
+ old_crtc_state,
+ new_crtc_state);
if (vbl_wait)
intel_wait_for_vblank(dev_priv, pipe);
--
2.24.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Intel-gfx] [PATCH 3/4] drm/i915/dp: Fix MST disable sequences
2019-12-07 1:18 [Intel-gfx] [PATCH 1/4] drm/i915/tgl: Select master transcoder for MST stream José Roberto de Souza
2019-12-07 1:18 ` [Intel-gfx] [PATCH 2/4] drm/i915/display: Always enables MST master pipe first José Roberto de Souza
@ 2019-12-07 1:18 ` José Roberto de Souza
2019-12-10 21:38 ` Ville Syrjälä
2019-12-07 1:18 ` [Intel-gfx] [PATCH 4/4] drm/i915/display: Add comment to a function that probably can be removed José Roberto de Souza
` (3 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: José Roberto de Souza @ 2019-12-07 1:18 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi
The disable sequence after wait for transcoder off was not correctly
implemented.
The MST disable sequence is basically the same for HSW, SKL, ICL and
TGL, with just minor changes for TGL.
So here calling a new MST function to do the MST sequences, most of
the steps just moved from the post disable hook.
With this last patch we finally fixed the hotplugs triggered by MST
sinks during the disable/enable sequence, those were causing source
to try to do a link training while it was not ready causing CPU pipe
FIFO underrrus on TGL.
BSpec: 4231
BSpec: 4163
BSpec: 22243
BSpec: 49190
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/display/intel_ddi.c | 17 +++--
drivers/gpu/drm/i915/display/intel_display.c | 2 +
drivers/gpu/drm/i915/display/intel_dp_mst.c | 79 ++++++++++++++++----
drivers/gpu/drm/i915/display/intel_dp_mst.h | 1 +
4 files changed, 79 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index be5bc506d4d3..f06c6dfec888 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -34,6 +34,7 @@
#include "intel_ddi.h"
#include "intel_display_types.h"
#include "intel_dp.h"
+#include "intel_dp_mst.h"
#include "intel_dp_link_training.h"
#include "intel_dpio_phy.h"
#include "intel_dsi.h"
@@ -1953,17 +1954,19 @@ void intel_ddi_disable_transcoder_func(const struct intel_crtc_state *crtc_state
struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
- i915_reg_t reg = TRANS_DDI_FUNC_CTL(cpu_transcoder);
- u32 val = I915_READ(reg);
+ u32 val;
+
+ val = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
+ val &= ~TRANS_DDI_FUNC_ENABLE;
if (INTEL_GEN(dev_priv) >= 12) {
- val &= ~(TRANS_DDI_FUNC_ENABLE | TGL_TRANS_DDI_PORT_MASK |
- TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
+ if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST) ||
+ intel_dp_mst_is_slave_trans(crtc_state))
+ val &= ~TGL_TRANS_DDI_PORT_MASK;
} else {
- val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_PORT_MASK |
- TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
+ val &= ~TRANS_DDI_PORT_MASK;
}
- I915_WRITE(reg, val);
+ I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), val);
if (dev_priv->quirks & QUIRK_INCREASE_DDI_DISABLED_TIME &&
intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 2f74c0bfb2a8..d3eefb271fa4 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6776,6 +6776,8 @@ static void haswell_crtc_disable(struct intel_atomic_state *state,
if (!transcoder_is_dsi(cpu_transcoder))
intel_disable_pipe(old_crtc_state);
+ intel_dp_mst_post_trans_disabled(old_crtc_state);
+
if (INTEL_GEN(dev_priv) >= 11)
icl_disable_transcoder_port_sync(old_crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 49f1518e3ab9..3c98a25e6308 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -365,6 +365,57 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder,
old_crtc_state, old_conn_state);
}
+static void
+dp_mst_post_trans_disabled(struct intel_encoder *encoder,
+ const struct intel_crtc_state *old_crtc_state,
+ const struct drm_connector_state *old_conn_state)
+{
+ struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
+ struct intel_digital_port *intel_dig_port = intel_mst->primary;
+ struct intel_dp *intel_dp = &intel_dig_port->dp;
+ struct intel_connector *connector =
+ to_intel_connector(old_conn_state->connector);
+ struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
+ u32 val;
+
+ drm_dp_update_payload_part2(&intel_dp->mst_mgr);
+
+ val = I915_READ(TRANS_DDI_FUNC_CTL(old_crtc_state->cpu_transcoder));
+ val &= ~TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
+ I915_WRITE(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_ERROR("Timed out waiting for ACT sent when disabling\n");
+
+ drm_dp_check_act_status(&intel_dp->mst_mgr);
+
+ drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, connector->port);
+}
+
+void
+intel_dp_mst_post_trans_disabled(const struct intel_crtc_state *old_crtc_state)
+{
+ struct drm_atomic_state *state = old_crtc_state->uapi.state;
+ struct drm_connector_state *old_conn_state;
+ struct drm_connector *conn;
+ int i;
+
+ if (!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST))
+ return;
+
+ for_each_old_connector_in_state(state, conn, old_conn_state, i) {
+ struct intel_encoder *encoder;
+
+ if (old_conn_state->crtc != old_crtc_state->uapi.crtc)
+ continue;
+
+ encoder = to_intel_encoder(old_conn_state->best_encoder);
+ dp_mst_post_trans_disabled(encoder, old_crtc_state,
+ old_conn_state);
+ }
+}
+
static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
const struct intel_crtc_state *old_crtc_state,
const struct drm_connector_state *old_conn_state)
@@ -383,6 +434,12 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
WARN_ON(INTEL_GEN(dev_priv) >= 12 && last_mst_stream &&
!intel_dp_mst_is_master_trans(old_crtc_state));
+ /*
+ * Power down mst path before disabling the port, otherwise we end
+ * up getting interrupts from the sink upon detecting link loss.
+ */
+ drm_dp_send_power_updown_phy(&intel_dp->mst_mgr, connector->port,
+ false);
/*
* From TGL spec: "If multi-stream slave transcoder: Configure
* Transcoder Clock Select to direct no clock to the transcoder"
@@ -393,24 +450,20 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
if (INTEL_GEN(dev_priv) < 12 || !last_mst_stream)
intel_ddi_disable_pipe_clock(old_crtc_state);
- /* this can fail */
- drm_dp_check_act_status(&intel_dp->mst_mgr);
- /* and this can also fail */
- drm_dp_update_payload_part2(&intel_dp->mst_mgr);
- drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, connector->port);
+ intel_mst->connector = NULL;
+ if (last_mst_stream) {
+ enum transcoder cpu_transcoder;
+ u32 val;
- /*
- * Power down mst path before disabling the port, otherwise we end
- * up getting interrupts from the sink upon detecting link loss.
- */
- drm_dp_send_power_updown_phy(&intel_dp->mst_mgr, connector->port,
- false);
+ cpu_transcoder = old_crtc_state->cpu_transcoder;
+ val = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
+ val &= ~TGL_TRANS_DDI_PORT_MASK;
+ I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), val);
- intel_mst->connector = NULL;
- if (last_mst_stream)
intel_dig_port->base.post_disable(&intel_dig_port->base,
old_crtc_state, NULL);
+ }
DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
}
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.h b/drivers/gpu/drm/i915/display/intel_dp_mst.h
index e40767f78343..87f32fab90fc 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.h
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.h
@@ -16,5 +16,6 @@ void intel_dp_mst_encoder_cleanup(struct intel_digital_port *intel_dig_port);
int intel_dp_mst_encoder_active_links(struct intel_digital_port *intel_dig_port);
bool intel_dp_mst_is_master_trans(const struct intel_crtc_state *crtc_state);
bool intel_dp_mst_is_slave_trans(const struct intel_crtc_state *crtc_state);
+void intel_dp_mst_post_trans_disabled(const struct intel_crtc_state *old_crtc_state);
#endif /* __INTEL_DP_MST_H__ */
--
2.24.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Intel-gfx] [PATCH 4/4] drm/i915/display: Add comment to a function that probably can be removed
2019-12-07 1:18 [Intel-gfx] [PATCH 1/4] drm/i915/tgl: Select master transcoder for MST stream José Roberto de Souza
2019-12-07 1:18 ` [Intel-gfx] [PATCH 2/4] drm/i915/display: Always enables MST master pipe first José Roberto de Souza
2019-12-07 1:18 ` [Intel-gfx] [PATCH 3/4] drm/i915/dp: Fix MST disable sequences José Roberto de Souza
@ 2019-12-07 1:18 ` José Roberto de Souza
2019-12-07 2:31 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915/tgl: Select master transcoder for MST stream Patchwork
` (2 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: José Roberto de Souza @ 2019-12-07 1:18 UTC (permalink / raw)
To: intel-gfx
This function is only called from port sync and it is identical to
what will be executed again in intel_update_crtc() over port sync
pipes.
If it is really necessary it at least deserves a better name and a
comment, leaving it to people working on port sync.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/display/intel_display.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index d3eefb271fa4..4b498bd25325 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14498,6 +14498,10 @@ static void intel_set_dp_tp_ctl_normal(struct intel_crtc *crtc,
intel_dp_stop_link_train(intel_dp);
}
+/*
+ * TODO: This is only called from port sync and it is identical to what will be
+ * executed again in intel_update_crtc() over port sync pipes
+ */
static void intel_post_crtc_enable_updates(struct intel_crtc *crtc,
struct intel_atomic_state *state)
{
--
2.24.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915/tgl: Select master transcoder for MST stream
2019-12-07 1:18 [Intel-gfx] [PATCH 1/4] drm/i915/tgl: Select master transcoder for MST stream José Roberto de Souza
` (2 preceding siblings ...)
2019-12-07 1:18 ` [Intel-gfx] [PATCH 4/4] drm/i915/display: Add comment to a function that probably can be removed José Roberto de Souza
@ 2019-12-07 2:31 ` Patchwork
2019-12-07 16:59 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2019-12-09 16:40 ` [Intel-gfx] [PATCH 1/4] " Ville Syrjälä
5 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2019-12-07 2:31 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/4] drm/i915/tgl: Select master transcoder for MST stream
URL : https://patchwork.freedesktop.org/series/70581/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_7506 -> Patchwork_15636
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/index.html
Known issues
------------
Here are the changes found in Patchwork_15636 that come from known issues:
### IGT changes ###
#### Possible fixes ####
* igt@gem_exec_gttfill@basic:
- {fi-tgl-u}: [INCOMPLETE][1] ([fdo#111593]) -> [PASS][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/fi-tgl-u/igt@gem_exec_gttfill@basic.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/fi-tgl-u/igt@gem_exec_gttfill@basic.html
* igt@i915_selftest@live_blt:
- fi-hsw-4770r: [DMESG-FAIL][3] ([i915#725]) -> [PASS][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/fi-hsw-4770r/igt@i915_selftest@live_blt.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/fi-hsw-4770r/igt@i915_selftest@live_blt.html
- fi-ivb-3770: [DMESG-FAIL][5] ([i915#725]) -> [PASS][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/fi-ivb-3770/igt@i915_selftest@live_blt.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/fi-ivb-3770/igt@i915_selftest@live_blt.html
- fi-hsw-4770: [DMESG-FAIL][7] ([i915#553] / [i915#725]) -> [PASS][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/fi-hsw-4770/igt@i915_selftest@live_blt.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/fi-hsw-4770/igt@i915_selftest@live_blt.html
* igt@i915_selftest@live_gem_contexts:
- fi-byt-j1900: [DMESG-FAIL][9] ([i915#722]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/fi-byt-j1900/igt@i915_selftest@live_gem_contexts.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/fi-byt-j1900/igt@i915_selftest@live_gem_contexts.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#111593]: https://bugs.freedesktop.org/show_bug.cgi?id=111593
[i915#553]: https://gitlab.freedesktop.org/drm/intel/issues/553
[i915#710]: https://gitlab.freedesktop.org/drm/intel/issues/710
[i915#722]: https://gitlab.freedesktop.org/drm/intel/issues/722
[i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725
[i915#726]: https://gitlab.freedesktop.org/drm/intel/issues/726
Participating hosts (38 -> 33)
------------------------------
Missing (5): fi-ilk-m540 fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_7506 -> Patchwork_15636
CI-20190529: 20190529
CI_DRM_7506: ebfb3e0542800b63ac73a4546fb7d486f9ade587 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5335: 06aa2c377ed40df1e246fca009c441fa18e53825 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_15636: 9e7f7e227edbbcc07fbe10523260728cc417c0ed @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
9e7f7e227edb drm/i915/display: Add comment to a function that probably can be removed
cd0f982622a1 drm/i915/dp: Fix MST disable sequences
9a1567310e28 drm/i915/display: Always enables MST master pipe first
70d76ac77f13 drm/i915/tgl: Select master transcoder for MST stream
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/4] drm/i915/tgl: Select master transcoder for MST stream
2019-12-07 1:18 [Intel-gfx] [PATCH 1/4] drm/i915/tgl: Select master transcoder for MST stream José Roberto de Souza
` (3 preceding siblings ...)
2019-12-07 2:31 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915/tgl: Select master transcoder for MST stream Patchwork
@ 2019-12-07 16:59 ` Patchwork
2019-12-09 16:40 ` [Intel-gfx] [PATCH 1/4] " Ville Syrjälä
5 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2019-12-07 16:59 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/4] drm/i915/tgl: Select master transcoder for MST stream
URL : https://patchwork.freedesktop.org/series/70581/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_7506_full -> Patchwork_15636_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_15636_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_15636_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_15636_full:
### IGT changes ###
#### Possible regressions ####
* igt@gem_ctx_isolation@vcs1-clean:
- shard-kbl: NOTRUN -> [FAIL][1]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-kbl2/igt@gem_ctx_isolation@vcs1-clean.html
* igt@gem_ctx_isolation@vecs0-dirty-switch:
- shard-skl: NOTRUN -> [FAIL][2]
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl7/igt@gem_ctx_isolation@vecs0-dirty-switch.html
* igt@gem_exec_parse_blt@allowed-all:
- shard-glk: [PASS][3] -> [DMESG-WARN][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-glk8/igt@gem_exec_parse_blt@allowed-all.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-glk3/igt@gem_exec_parse_blt@allowed-all.html
* igt@i915_selftest@mock_sanitycheck:
- shard-kbl: [PASS][5] -> [DMESG-WARN][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-kbl7/igt@i915_selftest@mock_sanitycheck.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-kbl7/igt@i915_selftest@mock_sanitycheck.html
- shard-skl: [PASS][7] -> [DMESG-WARN][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-skl6/igt@i915_selftest@mock_sanitycheck.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl2/igt@i915_selftest@mock_sanitycheck.html
Known issues
------------
Here are the changes found in Patchwork_15636_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_busy@extended-parallel-vcs1:
- shard-iclb: [PASS][9] -> [SKIP][10] ([fdo#112080]) +6 similar issues
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-iclb4/igt@gem_busy@extended-parallel-vcs1.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-iclb6/igt@gem_busy@extended-parallel-vcs1.html
* igt@gem_ctx_isolation@rcs0-s3:
- shard-tglb: [PASS][11] -> [INCOMPLETE][12] ([i915#456])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-tglb2/igt@gem_ctx_isolation@rcs0-s3.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-tglb7/igt@gem_ctx_isolation@rcs0-s3.html
* igt@gem_ctx_shared@exec-shared-gtt-bsd2:
- shard-iclb: [PASS][13] -> [SKIP][14] ([fdo#109276]) +3 similar issues
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-iclb1/igt@gem_ctx_shared@exec-shared-gtt-bsd2.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-iclb6/igt@gem_ctx_shared@exec-shared-gtt-bsd2.html
* igt@gem_exec_await@wide-all:
- shard-tglb: [PASS][15] -> [INCOMPLETE][16] ([fdo#111736])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-tglb4/igt@gem_exec_await@wide-all.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-tglb7/igt@gem_exec_await@wide-all.html
* igt@gem_exec_schedule@preempt-queue-bsd2:
- shard-tglb: [PASS][17] -> [INCOMPLETE][18] ([fdo#111606] / [fdo#111677])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-tglb1/igt@gem_exec_schedule@preempt-queue-bsd2.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-tglb6/igt@gem_exec_schedule@preempt-queue-bsd2.html
* igt@gem_exec_schedule@preempt-self-bsd:
- shard-iclb: [PASS][19] -> [SKIP][20] ([fdo#112146])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-iclb6/igt@gem_exec_schedule@preempt-self-bsd.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-iclb2/igt@gem_exec_schedule@preempt-self-bsd.html
* igt@gem_ppgtt@flink-and-close-vma-leak:
- shard-glk: [PASS][21] -> [FAIL][22] ([i915#644])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-glk1/igt@gem_ppgtt@flink-and-close-vma-leak.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-glk2/igt@gem_ppgtt@flink-and-close-vma-leak.html
* igt@gem_softpin@noreloc-s3:
- shard-skl: [PASS][23] -> [INCOMPLETE][24] ([i915#69]) +1 similar issue
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-skl7/igt@gem_softpin@noreloc-s3.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl3/igt@gem_softpin@noreloc-s3.html
* igt@gem_sync@basic-all:
- shard-tglb: [PASS][25] -> [INCOMPLETE][26] ([i915#470] / [i915#472])
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-tglb5/igt@gem_sync@basic-all.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-tglb6/igt@gem_sync@basic-all.html
* igt@gem_userptr_blits@sync-unmap-cycles:
- shard-snb: [PASS][27] -> [DMESG-WARN][28] ([fdo#111870]) +2 similar issues
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-snb1/igt@gem_userptr_blits@sync-unmap-cycles.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-snb6/igt@gem_userptr_blits@sync-unmap-cycles.html
* igt@i915_selftest@live_requests:
- shard-tglb: [PASS][29] -> [INCOMPLETE][30] ([fdo#112057])
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-tglb7/igt@i915_selftest@live_requests.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-tglb7/igt@i915_selftest@live_requests.html
* igt@i915_suspend@debugfs-reader:
- shard-tglb: [PASS][31] -> [INCOMPLETE][32] ([i915#460])
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-tglb2/igt@i915_suspend@debugfs-reader.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-tglb2/igt@i915_suspend@debugfs-reader.html
* igt@i915_suspend@fence-restore-tiled2untiled:
- shard-tglb: [PASS][33] -> [INCOMPLETE][34] ([i915#456] / [i915#460])
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-tglb1/igt@i915_suspend@fence-restore-tiled2untiled.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-tglb3/igt@i915_suspend@fence-restore-tiled2untiled.html
* igt@kms_color@pipe-a-ctm-blue-to-red:
- shard-skl: [PASS][35] -> [DMESG-WARN][36] ([i915#109])
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-skl2/igt@kms_color@pipe-a-ctm-blue-to-red.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl1/igt@kms_color@pipe-a-ctm-blue-to-red.html
* igt@kms_cursor_crc@pipe-a-cursor-64x64-sliding:
- shard-skl: [PASS][37] -> [FAIL][38] ([i915#54]) +1 similar issue
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-skl8/igt@kms_cursor_crc@pipe-a-cursor-64x64-sliding.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl9/igt@kms_cursor_crc@pipe-a-cursor-64x64-sliding.html
* igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-gtt:
- shard-kbl: [PASS][39] -> [DMESG-WARN][40] ([i915#728])
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-kbl2/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-gtt.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-kbl2/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-gtt.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-wc:
- shard-iclb: [PASS][41] -> [INCOMPLETE][42] ([i915#140])
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-wc.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-wc.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-blt:
- shard-tglb: [PASS][43] -> [FAIL][44] ([i915#49])
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-tglb7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-blt.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-tglb5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-blt.html
* igt@kms_frontbuffer_tracking@fbc-suspend:
- shard-apl: [PASS][45] -> [DMESG-WARN][46] ([i915#180]) +2 similar issues
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-apl4/igt@kms_frontbuffer_tracking@fbc-suspend.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-apl6/igt@kms_frontbuffer_tracking@fbc-suspend.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-blt:
- shard-iclb: [PASS][47] -> [FAIL][48] ([i915#49]) +1 similar issue
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-blt.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-blt.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt:
- shard-iclb: [PASS][49] -> [DMESG-WARN][50] ([i915#728]) +2 similar issues
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-iclb3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-iclb3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-move:
- shard-tglb: [PASS][51] -> [DMESG-WARN][52] ([i915#728]) +2 similar issues
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-tglb3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-move.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-tglb3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-move.html
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-shrfb-plflip-blt:
- shard-skl: [PASS][53] -> [DMESG-WARN][54] ([i915#728])
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-skl8/igt@kms_frontbuffer_tracking@psr-1p-primscrn-shrfb-plflip-blt.html
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl9/igt@kms_frontbuffer_tracking@psr-1p-primscrn-shrfb-plflip-blt.html
* igt@kms_frontbuffer_tracking@psr-shrfb-scaledprimary:
- shard-skl: [PASS][55] -> [INCOMPLETE][56] ([i915#123])
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-skl9/igt@kms_frontbuffer_tracking@psr-shrfb-scaledprimary.html
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl6/igt@kms_frontbuffer_tracking@psr-shrfb-scaledprimary.html
* igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
- shard-kbl: [PASS][57] -> [DMESG-WARN][58] ([i915#180]) +6 similar issues
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-kbl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-kbl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
* igt@kms_psr@no_drrs:
- shard-iclb: [PASS][59] -> [FAIL][60] ([i915#173])
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-iclb4/igt@kms_psr@no_drrs.html
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-iclb1/igt@kms_psr@no_drrs.html
* igt@kms_psr@suspend:
- shard-skl: [PASS][61] -> [INCOMPLETE][62] ([i915#198])
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-skl7/igt@kms_psr@suspend.html
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl3/igt@kms_psr@suspend.html
* igt@kms_setmode@basic:
- shard-kbl: [PASS][63] -> [FAIL][64] ([i915#31])
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-kbl4/igt@kms_setmode@basic.html
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-kbl6/igt@kms_setmode@basic.html
* igt@perf@disabled-read-error:
- shard-hsw: [PASS][65] -> [INCOMPLETE][66] ([i915#61])
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-hsw7/igt@perf@disabled-read-error.html
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-hsw1/igt@perf@disabled-read-error.html
#### Possible fixes ####
* igt@gem_ctx_persistence@rcs0-mixed-process:
- shard-skl: [FAIL][67] ([i915#679]) -> [PASS][68]
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-skl9/igt@gem_ctx_persistence@rcs0-mixed-process.html
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl6/igt@gem_ctx_persistence@rcs0-mixed-process.html
* igt@gem_ctx_shared@q-smoketest-all:
- shard-tglb: [INCOMPLETE][69] ([fdo#111735]) -> [PASS][70]
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-tglb4/igt@gem_ctx_shared@q-smoketest-all.html
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-tglb5/igt@gem_ctx_shared@q-smoketest-all.html
* igt@gem_eio@in-flight-suspend:
- shard-tglb: [INCOMPLETE][71] ([i915#456] / [i915#460] / [i915#534]) -> [PASS][72]
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-tglb7/igt@gem_eio@in-flight-suspend.html
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-tglb8/igt@gem_eio@in-flight-suspend.html
* igt@gem_eio@suspend:
- shard-tglb: [INCOMPLETE][73] ([i915#460]) -> [PASS][74]
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-tglb2/igt@gem_eio@suspend.html
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-tglb8/igt@gem_eio@suspend.html
* igt@gem_exec_balancer@smoke:
- shard-iclb: [SKIP][75] ([fdo#110854]) -> [PASS][76]
[75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-iclb6/igt@gem_exec_balancer@smoke.html
[76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-iclb2/igt@gem_exec_balancer@smoke.html
* igt@gem_exec_basic@basic-vcs1:
- shard-iclb: [SKIP][77] ([fdo#112080]) -> [PASS][78] +2 similar issues
[77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-iclb3/igt@gem_exec_basic@basic-vcs1.html
[78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-iclb4/igt@gem_exec_basic@basic-vcs1.html
* igt@gem_exec_parallel@rcs0-fds:
- shard-hsw: [FAIL][79] -> [PASS][80]
[79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-hsw6/igt@gem_exec_parallel@rcs0-fds.html
[80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-hsw5/igt@gem_exec_parallel@rcs0-fds.html
* igt@gem_exec_schedule@in-order-bsd:
- shard-iclb: [SKIP][81] ([fdo#112146]) -> [PASS][82] +3 similar issues
[81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-iclb4/igt@gem_exec_schedule@in-order-bsd.html
[82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-iclb6/igt@gem_exec_schedule@in-order-bsd.html
* {igt@gem_exec_schedule@pi-userfault-bsd}:
- shard-iclb: [SKIP][83] ([i915#677]) -> [PASS][84]
[83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-iclb1/igt@gem_exec_schedule@pi-userfault-bsd.html
[84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-iclb3/igt@gem_exec_schedule@pi-userfault-bsd.html
* igt@gem_exec_schedule@preempt-queue-contexts-chain-bsd2:
- shard-iclb: [SKIP][85] ([fdo#109276]) -> [PASS][86] +3 similar issues
[85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-iclb3/igt@gem_exec_schedule@preempt-queue-contexts-chain-bsd2.html
[86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-iclb4/igt@gem_exec_schedule@preempt-queue-contexts-chain-bsd2.html
* igt@gem_persistent_relocs@forked-faulting-reloc-thrash-inactive:
- shard-tglb: [TIMEOUT][87] ([i915#530]) -> [PASS][88]
[87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-tglb5/igt@gem_persistent_relocs@forked-faulting-reloc-thrash-inactive.html
[88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-tglb1/igt@gem_persistent_relocs@forked-faulting-reloc-thrash-inactive.html
* igt@gem_ppgtt@flink-and-close-vma-leak:
- shard-apl: [FAIL][89] ([i915#644]) -> [PASS][90]
[89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-apl1/igt@gem_ppgtt@flink-and-close-vma-leak.html
[90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-apl7/igt@gem_ppgtt@flink-and-close-vma-leak.html
* igt@i915_pm_rpm@system-suspend-execbuf:
- shard-tglb: [INCOMPLETE][91] ([i915#456] / [i915#460]) -> [PASS][92] +1 similar issue
[91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-tglb4/igt@i915_pm_rpm@system-suspend-execbuf.html
[92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-tglb6/igt@i915_pm_rpm@system-suspend-execbuf.html
* igt@i915_suspend@forcewake:
- shard-kbl: [DMESG-WARN][93] ([i915#180]) -> [PASS][94] +2 similar issues
[93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-kbl7/igt@i915_suspend@forcewake.html
[94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-kbl6/igt@i915_suspend@forcewake.html
* igt@kms_big_fb@x-tiled-8bpp-rotate-180:
- shard-snb: [SKIP][95] ([fdo#109271]) -> [PASS][96]
[95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-snb1/igt@kms_big_fb@x-tiled-8bpp-rotate-180.html
[96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-snb6/igt@kms_big_fb@x-tiled-8bpp-rotate-180.html
* igt@kms_big_fb@yf-tiled-32bpp-rotate-270:
- shard-kbl: [DMESG-WARN][97] ([i915#728]) -> [PASS][98] +2 similar issues
[97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-kbl7/igt@kms_big_fb@yf-tiled-32bpp-rotate-270.html
[98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-kbl2/igt@kms_big_fb@yf-tiled-32bpp-rotate-270.html
* igt@kms_ccs@pipe-a-crc-primary-rotation-180:
- shard-skl: [INCOMPLETE][99] ([fdo#112347]) -> [PASS][100]
[99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-skl3/igt@kms_ccs@pipe-a-crc-primary-rotation-180.html
[100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl5/igt@kms_ccs@pipe-a-crc-primary-rotation-180.html
* igt@kms_cursor_crc@pipe-a-cursor-128x42-onscreen:
- shard-skl: [FAIL][101] ([i915#54]) -> [PASS][102] +3 similar issues
[101]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-skl10/igt@kms_cursor_crc@pipe-a-cursor-128x42-onscreen.html
[102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl9/igt@kms_cursor_crc@pipe-a-cursor-128x42-onscreen.html
* igt@kms_cursor_crc@pipe-b-cursor-suspend:
- shard-apl: [DMESG-WARN][103] ([i915#180]) -> [PASS][104] +1 similar issue
[103]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-apl4/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
[104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-apl6/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
* igt@kms_cursor_crc@pipe-c-cursor-suspend:
- shard-skl: [INCOMPLETE][105] ([i915#300]) -> [PASS][106] +1 similar issue
[105]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-skl9/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
[106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl3/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
* igt@kms_draw_crc@draw-method-xrgb2101010-render-untiled:
- shard-skl: [INCOMPLETE][107] ([i915#646]) -> [PASS][108]
[107]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-skl4/igt@kms_draw_crc@draw-method-xrgb2101010-render-untiled.html
[108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl5/igt@kms_draw_crc@draw-method-xrgb2101010-render-untiled.html
* igt@kms_draw_crc@draw-method-xrgb8888-pwrite-xtiled:
- shard-skl: [FAIL][109] ([i915#52] / [i915#54]) -> [PASS][110]
[109]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-skl3/igt@kms_draw_crc@draw-method-xrgb8888-pwrite-xtiled.html
[110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl10/igt@kms_draw_crc@draw-method-xrgb8888-pwrite-xtiled.html
* igt@kms_fbcon_fbt@fbc-suspend:
- shard-tglb: [INCOMPLETE][111] ([i915#435] / [i915#456] / [i915#460]) -> [PASS][112]
[111]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-tglb2/igt@kms_fbcon_fbt@fbc-suspend.html
[112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-tglb4/igt@kms_fbcon_fbt@fbc-suspend.html
* igt@kms_flip@flip-vs-expired-vblank:
- shard-skl: [FAIL][113] ([i915#79]) -> [PASS][114]
[113]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-skl1/igt@kms_flip@flip-vs-expired-vblank.html
[114]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl4/igt@kms_flip@flip-vs-expired-vblank.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-pwrite:
- shard-iclb: [FAIL][115] ([i915#49]) -> [PASS][116]
[115]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-pwrite.html
[116]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-iclb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-pwrite.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-wc:
- shard-tglb: [FAIL][117] ([i915#49]) -> [PASS][118] +2 similar issues
[117]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-wc.html
[118]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-wc.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-render:
- shard-iclb: [DMESG-WARN][119] ([i915#728]) -> [PASS][120] +2 similar issues
[119]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-iclb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-render.html
[120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-iclb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-render.html
- shard-tglb: [DMESG-WARN][121] ([i915#728]) -> [PASS][122] +1 similar issue
[121]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-render.html
[122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-tglb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-render.html
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-pgflip-blt:
- shard-skl: [DMESG-WARN][123] ([i915#728]) -> [PASS][124] +1 similar issue
[123]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-skl8/igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-pgflip-blt.html
[124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl9/igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-pgflip-blt.html
* igt@kms_plane@pixel-format-pipe-a-planes:
- shard-skl: [INCOMPLETE][125] ([fdo#112347] / [fdo#112391] / [i915#648]) -> [PASS][126]
[125]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-skl5/igt@kms_plane@pixel-format-pipe-a-planes.html
[126]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl7/igt@kms_plane@pixel-format-pipe-a-planes.html
* igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
- shard-skl: [FAIL][127] ([fdo#108145]) -> [PASS][128]
[127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-skl2/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
[128]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl1/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
* igt@kms_psr@psr2_sprite_plane_onoff:
- shard-iclb: [SKIP][129] ([fdo#109441]) -> [PASS][130]
[129]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-iclb6/igt@kms_psr@psr2_sprite_plane_onoff.html
[130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-iclb2/igt@kms_psr@psr2_sprite_plane_onoff.html
#### Warnings ####
* igt@gem_eio@kms:
- shard-snb: [DMESG-WARN][131] ([i915#444] / [i915#502]) -> [DMESG-WARN][132] ([i915#444])
[131]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-snb4/igt@gem_eio@kms.html
[132]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-snb1/igt@gem_eio@kms.html
* igt@kms_plane@pixel-format-pipe-a-planes:
- shard-kbl: [INCOMPLETE][133] ([fdo#103665]) -> [DMESG-WARN][134] ([i915#728])
[133]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-kbl7/igt@kms_plane@pixel-format-pipe-a-planes.html
[134]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-kbl7/igt@kms_plane@pixel-format-pipe-a-planes.html
* igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
- shard-skl: [DMESG-WARN][135] ([i915#728]) -> [INCOMPLETE][136] ([i915#648])
[135]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-skl4/igt@kms_plane@pixel-format-pipe-a-planes-source-clamping.html
[136]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl6/igt@kms_plane@pixel-format-pipe-a-planes-source-clamping.html
* igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
- shard-skl: [INCOMPLETE][137] ([fdo#112347] / [i915#648]) -> [INCOMPLETE][138] ([fdo#112347] / [fdo#112391] / [i915#648])
[137]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7506/shard-skl7/igt@kms_plane@pixel-format-pipe-b-planes-source-clamping.html
[138]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/shard-skl4/igt@kms_plane@pixel-format-pipe-b-planes-source-clamping.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
[fdo#111606]: https://bugs.freedesktop.org/show_bug.cgi?id=111606
[fdo#111677]: https://bugs.freedesktop.org/show_bug.cgi?id=111677
[fdo#111735]: https://bugs.freedesktop.org/show_bug.cgi?id=111735
[fdo#111736]: https://bugs.freedesktop.org/show_bug.cgi?id=111736
[fdo#111870]: https://bugs.freedesktop.org/show_bug.cgi?id=111870
[fdo#112057]: https://bugs.freedesktop.org/show_bug.cgi?id=112057
[fdo#112080]: https://bugs.freedesktop.org/show_bug.cgi?id=112080
[fdo#112146]: https://bugs.freedesktop.org/show_bug.cgi?id=112146
[fdo#112347]: https://bugs.freedesktop.org/show_bug.cgi?id=112347
[fdo#112391]: https://bugs.freedesktop.org/show_bug.cgi?id=112391
[i915#109]: https://gitlab.freedesktop.org/drm/intel/issues/109
[i915#123]: https://gitlab.freedesktop.org/drm/intel/issues/123
[i915#140]: https://gitlab.freedesktop.org/drm/intel/issues/140
[i915#173]: https://gitlab.freedesktop.org/drm/intel/issues/173
[i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
[i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
[i915#300]: https://gitlab.freedesktop.org/drm/intel/issues/300
[i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
[i915#435]: https://gitlab.freedesktop.org/drm/intel/issues/435
[i915#444]: https://gitlab.freedesktop.org/drm/intel/issues/444
[i915#456]: https://gitlab.freedesktop.org/drm/intel/issues/456
[i915#460]: https://gitlab.freedesktop.org/drm/intel/issues/460
[i915#470]: https://gitlab.freedesktop.org/drm/intel/issues/470
[i915#472]: https://gitlab.freedesktop.org/drm/intel/issues/472
[i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
[i915#502]: https://gitlab.freedesktop.org/drm/intel/issues/502
[i915#52]: https://gitlab.freedesktop.org/drm/intel/issues/52
[i915#530]: https://gitlab.freedesktop.org/drm/intel/issues/530
[i915#534]: https://gitlab.freedesktop.org/drm/intel/issues/534
[i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
[i915#61]: https://gitlab.freedesktop.org/drm/intel/issues/61
[i915#644]: https://gitlab.freedesktop.org/drm/intel/issues/644
[i915#646]: https://gitlab.freedesktop.org/drm/intel/issues/646
[i915#648]: https://gitlab.freedesktop.org/drm/intel/issues/648
[i915#677]: https://gitlab.freedesktop.org/drm/intel/issues/677
[i915#679]: https://gitlab.freedesktop.org/drm/intel/issues/679
[i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
[i915#728]: https://gitlab.freedesktop.org/drm/intel/issues/728
[i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
Participating hosts (11 -> 11)
------------------------------
No changes in participating hosts
Build changes
-------------
* CI: CI-20190529 -> None
* Linu
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15636/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 1/4] drm/i915/tgl: Select master transcoder for MST stream
2019-12-07 1:18 [Intel-gfx] [PATCH 1/4] drm/i915/tgl: Select master transcoder for MST stream José Roberto de Souza
` (4 preceding siblings ...)
2019-12-07 16:59 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
@ 2019-12-09 16:40 ` Ville Syrjälä
2019-12-09 18:45 ` Souza, Jose
5 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2019-12-09 16:40 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Lucas De Marchi
On Fri, Dec 06, 2019 at 05:18:29PM -0800, José Roberto de Souza wrote:
> On TGL the blending of all the streams have moved from DDI to
> transcoder, so now every transcoder working over the same MST port must
> send its stream to a master transcoder and master will send to DDI
> respecting the time slots.
>
> So here adding all the CRTCs that shares the same MST stream if
> needed and computing their state again, it will pick the first
> transcoder among the ones in the same stream to be master.
>
> Most of the time skl_commit_modeset_enables() enables pipes in a
> crescent order but due DDB overlapping it might not happen, this
> scenario will be handled in the next patch.
>
> BSpec: 50493
> BSpec: 49190
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 14 +-
> drivers/gpu/drm/i915/display/intel_display.c | 15 +-
> .../drm/i915/display/intel_display_types.h | 3 +
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 163 ++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_dp_mst.h | 5 +
> 5 files changed, 196 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 3cacb1e279c1..be5bc506d4d3 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1903,8 +1903,13 @@ intel_ddi_transcoder_func_reg_val_get(const struct intel_crtc_state *crtc_state)
> temp |= TRANS_DDI_MODE_SELECT_DP_MST;
> temp |= DDI_PORT_WIDTH(crtc_state->lane_count);
>
> - if (INTEL_GEN(dev_priv) >= 12)
> - temp |= TRANS_DDI_MST_TRANSPORT_SELECT(crtc_state->cpu_transcoder);
> + if (INTEL_GEN(dev_priv) >= 12) {
> + enum transcoder master;
> +
> + master = crtc_state->mst_master_transcoder;
> + WARN_ON(master == INVALID_TRANSCODER);
I'd drop the WARN_ON(). If we keep adding these for every little thing
we'll drown in them.
> + temp |= TRANS_DDI_MST_TRANSPORT_SELECT(master);
> + }
> } else {
> temp |= TRANS_DDI_MODE_SELECT_DP_SST;
> temp |= DDI_PORT_WIDTH(crtc_state->lane_count);
> @@ -4372,6 +4377,11 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
> pipe_config->lane_count =
> ((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
> +
> + if (INTEL_GEN(dev_priv) >= 12)
> + pipe_config->mst_master_transcoder =
> + REG_FIELD_GET(TRANS_DDI_MST_TRANSPORT_SELECT_MASK, temp);
> +
> intel_dp_get_m_n(intel_crtc, pipe_config);
> break;
> default:
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 821ba8053f9d..f89494c849ce 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -46,6 +46,7 @@
> #include "display/intel_crt.h"
> #include "display/intel_ddi.h"
> #include "display/intel_dp.h"
> +#include "display/intel_dp_mst.h"
> #include "display/intel_dsi.h"
> #include "display/intel_dvo.h"
> #include "display/intel_gmbus.h"
> @@ -12542,6 +12543,11 @@ static void intel_dump_pipe_config(const struct intel_crtc_state *pipe_config,
> pipe_config->csc_mode, pipe_config->gamma_mode,
> pipe_config->gamma_enable, pipe_config->csc_enable);
>
> + if (INTEL_GEN(dev_priv) >= 12 &&
> + intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST))
Could probably print this unconditionally to keep the code less messy.
> + DRM_DEBUG_KMS("MST master transcoder: %s\n",
> + transcoder_name(pipe_config->mst_master_transcoder));
> +
> dump_planes:
> if (!state)
> return;
> @@ -13318,6 +13324,10 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> PIPE_CONF_CHECK_I(sync_mode_slaves_mask);
> PIPE_CONF_CHECK_I(master_transcoder);
>
> + if (INTEL_GEN(dev_priv) >= 12 &&
> + intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST))
These checks are definitely pointless.
> + PIPE_CONF_CHECK_I(mst_master_transcoder);
> +
> #undef PIPE_CONF_CHECK_X
> #undef PIPE_CONF_CHECK_I
> #undef PIPE_CONF_CHECK_BOOL
> @@ -14406,7 +14416,7 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
> u32 handled = 0;
> int i;
>
> - /* Only disable port sync slaves */
> + /* Only disable port sync and MST slaves */
> for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> new_crtc_state, i) {
> if (!needs_modeset(new_crtc_state))
> @@ -14420,7 +14430,8 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
> * slave CRTCs are disabled first and then master CRTC since
> * Slave vblanks are masked till Master Vblanks.
> */
> - if (!is_trans_port_sync_slave(old_crtc_state))
> + if (!is_trans_port_sync_slave(old_crtc_state) &&
> + !intel_dp_mst_is_slave_trans(old_crtc_state))
> continue;
>
> intel_pre_plane_update(state, crtc);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 83ea04149b77..630a94892b7b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1054,6 +1054,9 @@ struct intel_crtc_state {
>
> /* Bitmask to indicate slaves attached */
> u8 sync_mode_slaves_mask;
> +
> + /* Only valid on TGL+ */
> + enum transcoder mst_master_transcoder;
> };
>
> struct intel_crtc {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 926e49f449a6..49f1518e3ab9 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -87,6 +87,49 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
> return 0;
> }
>
> +/*
> + * Iterate over all the CRTCs and set mst_master_transcoder to the first active
> + * transcoder that shares the same MST connector.
> + *
> + * TODO: maybe this could be optimized to keep the old master transcoder
> + */
> +static int
> +intel_dp_mst_master_trans_compute(struct intel_encoder *encoder,
> + struct intel_crtc_state *pipe_config,
s/pipe_config/crtc_state/
> + struct drm_connector_state *conn_state)
> +{
> + struct intel_atomic_state *state = to_intel_atomic_state(pipe_config->uapi.state);
> + struct intel_connector *conn = to_intel_connector(conn_state->connector);
ocd: s/conn/connector/ all over.
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_digital_connector_state *iter_conn_state;
> + struct intel_connector *iter_conn;
> + int i;
> +
> + if (INTEL_GEN(dev_priv) < 12)
> + return 0;
> +
> + for_each_new_intel_connector_in_state(state, iter_conn,
> + iter_conn_state, i) {
> + struct intel_crtc_state *iter_crtc_state;
> + struct intel_crtc *iter_crtc;
> +
> + if (conn->mst_port != iter_conn->mst_port ||
> + !iter_conn_state->base.crtc)
> + continue;
> +
> + iter_crtc = to_intel_crtc(iter_conn_state->base.crtc);
> + iter_crtc_state = intel_atomic_get_new_crtc_state(state,
> + iter_crtc);
> + if (!iter_crtc_state->uapi.active)
> + continue;
> +
> + pipe_config->mst_master_transcoder = iter_crtc_state->cpu_transcoder;
> + break;
So we're going to pick this based on the connector order. How does that
fit in with the port sync, or did we not have port sync for MST yet?
To keep everything consistent and easy my idea was to pick the first
pipe as the master for both MST and port sync.
Although I can easily imagine topologies where even that would
land us in some interesting mess. For example:
pipe A -> MST port B -> ... -> whatever : MST master
pipe B -> MST port B -> ... -> tile 0 : port sync master / MST slave
pipe C -> MST port C -> ... -> tile 1 : port sync slave / MST master
pipe D -> MST port C -> ... -> whatever : MST slave
pipe A -> MST port B -> ... -> whatever : MST master
pipe B -> MST port B -> ... -> tile 0 : port sync master / MST slave
pipe C -> MST port C -> ... -> whatever : MST master
pipe D -> MST port C -> ... -> tile 1 : port sync slave / MST slave
No idea if the hw can even do something like that. Needs more thought
if/when we enable port sync with MST (assuming we're not already doing
that).
> + }
> +
> + return 0;
> +}
> +
> static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
> struct intel_crtc_state *pipe_config,
> struct drm_connector_state *conn_state)
> @@ -154,6 +197,98 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
>
> intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
>
> + ret = intel_dp_mst_master_trans_compute(encoder, pipe_config,
> + conn_state);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int
> +intel_dp_mst_atomic_master_trans_check(struct drm_connector *connector,
> + struct drm_atomic_state *state)
Just pass in intel_ types pls.
> +{
> + struct drm_i915_private *dev_priv = to_i915(connector->dev);
> + struct drm_connector_state *new_conn_state, *old_conn_state;
> + struct drm_connector_list_iter conn_list_iter;
ocd: s/conn_list_iter/conn_iter/
> + struct intel_crtc_state *intel_crtc_state;
Just 'crtc_state'
> + struct intel_connector *intel_conn;
> + struct drm_crtc_state *crtc_state;
> + struct drm_connector *conn_iter;
iter_foo vs. foo_iter in this vs. the previous function. Not that I
really like that naming convention since it can get confusing with
the 'struct drm_connector_list_iter conn_iter'.
> +
> + if (INTEL_GEN(dev_priv) < 12)
> + return 0;
> +
> + intel_conn = to_intel_connector(connector);
> + new_conn_state = drm_atomic_get_new_connector_state(state, connector);
> + old_conn_state = drm_atomic_get_old_connector_state(state, connector);
These could be done when declaring the variables.
> +
> + if (!old_conn_state->crtc && !new_conn_state->crtc)
> + return 0;
> +
> + /*
> + * 3 cases that needs be handled here:
> + * - connector going from disabled to enabled:
> + * nothing else need to be done, drm helpers already set mode_changed in
> + * the CRTCs needed
> + * - connector going from enabled to disabled:
> + * if this transcoder was the master, all slaves needs a modeset
> + * - connector going from enabled to enabled but it needs a modeset:
> + * if this transcoder was the master, all slaves also needs a modeset
> + */
Too may special cases IMO. I suggest just blindly marking
everything on the same mst_port for modeset if this connector
needs a modeset. IIRC we have a helper for that check even...
Ah yes, intel_connector_needs_modeset().
> +
> + /* disabled -> enabled */
> + if (!old_conn_state->crtc && new_conn_state->crtc)
> + return 0;
> +
> + /* enabled -> enabled(modeset)? */
> + if (new_conn_state->crtc) {
> + crtc_state = drm_atomic_get_new_crtc_state(state,
> + new_conn_state->crtc);
> + if (!drm_atomic_crtc_needs_modeset(crtc_state))
> + return 0;
> + }
> +
> + /* handling enabled -> enabled(modeset) and enabled -> disabled */
> + crtc_state = drm_atomic_get_old_crtc_state(state, old_conn_state->crtc);
> + intel_crtc_state = to_intel_crtc_state(crtc_state);
> +
> + /* If not master, nothing else needs to be handled */
> + if (!intel_dp_mst_is_master_trans(intel_crtc_state))
> + return 0;
> +
> + /* It is master, add and mark all other CRTCs in the stream as changed */
> + drm_connector_list_iter_begin(state->dev, &conn_list_iter);
> + drm_for_each_connector_iter(conn_iter, &conn_list_iter) {
> + struct drm_connector_state *conn_iter_state;
> + struct intel_connector *intel_conn_iter;
> +
> + intel_conn_iter = to_intel_connector(conn_iter);
> + if (intel_conn_iter->mst_port != intel_conn->mst_port)
> + continue;
> +
> + conn_iter_state = drm_atomic_get_connector_state(state,
> + conn_iter);
> + if (IS_ERR(conn_iter_state)) {
> + drm_connector_list_iter_end(&conn_list_iter);
> + return PTR_ERR(conn_iter_state);
> + }
> +
> + if (!conn_iter_state->crtc)
> + continue;
> +
> + crtc_state = drm_atomic_get_crtc_state(state,
> + conn_iter_state->crtc);
> + if (IS_ERR(crtc_state)) {
> + drm_connector_list_iter_end(&conn_list_iter);
> + return PTR_ERR(conn_iter_state);
> + }
> +
> + crtc_state->mode_changed = true;
> + }
> + drm_connector_list_iter_end(&conn_list_iter);
> +
> return 0;
> }
>
> @@ -175,6 +310,10 @@ intel_dp_mst_atomic_check(struct drm_connector *connector,
> if (ret)
> return ret;
>
> + ret = intel_dp_mst_atomic_master_trans_check(connector, state);
> + if (ret)
> + return ret;
> +
> if (!old_conn_state->crtc)
> return 0;
>
> @@ -241,6 +380,9 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
> intel_dp->active_mst_links--;
> last_mst_stream = intel_dp->active_mst_links == 0;
>
> + WARN_ON(INTEL_GEN(dev_priv) >= 12 && last_mst_stream &&
> + !intel_dp_mst_is_master_trans(old_crtc_state));
> +
> /*
> * From TGL spec: "If multi-stream slave transcoder: Configure
> * Transcoder Clock Select to direct no clock to the transcoder"
> @@ -321,6 +463,9 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
> intel_mst->connector = connector;
> first_mst_stream = intel_dp->active_mst_links == 0;
>
> + WARN_ON(INTEL_GEN(dev_priv) >= 12 && first_mst_stream &&
> + !intel_dp_mst_is_master_trans(pipe_config));
> +
> DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
>
> if (first_mst_stream)
> @@ -726,3 +871,21 @@ intel_dp_mst_encoder_cleanup(struct intel_digital_port *intel_dig_port)
> drm_dp_mst_topology_mgr_destroy(&intel_dp->mst_mgr);
> /* encoders will get killed by normal cleanup */
> }
> +
> +bool intel_dp_mst_is_master_trans(const struct intel_crtc_state *crtc_state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
> +
> + return INTEL_GEN(dev_priv) >= 12 &&
> + intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST) &&
> + crtc_state->mst_master_transcoder == crtc_state->cpu_transcoder;
All but the last line seem redundant.
> +}
> +
> +bool intel_dp_mst_is_slave_trans(const struct intel_crtc_state *crtc_state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
> +
> + return INTEL_GEN(dev_priv) >= 12 &&
> + intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST) &&
> + crtc_state->mst_master_transcoder != crtc_state->cpu_transcoder;
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.h b/drivers/gpu/drm/i915/display/intel_dp_mst.h
> index f660ad80db04..e40767f78343 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.h
> @@ -6,10 +6,15 @@
> #ifndef __INTEL_DP_MST_H__
> #define __INTEL_DP_MST_H__
>
> +#include <stdbool.h>
> +
> struct intel_digital_port;
> +struct intel_crtc_state;
>
> int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
> void intel_dp_mst_encoder_cleanup(struct intel_digital_port *intel_dig_port);
> int intel_dp_mst_encoder_active_links(struct intel_digital_port *intel_dig_port);
> +bool intel_dp_mst_is_master_trans(const struct intel_crtc_state *crtc_state);
> +bool intel_dp_mst_is_slave_trans(const struct intel_crtc_state *crtc_state);
>
> #endif /* __INTEL_DP_MST_H__ */
> --
> 2.24.0
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/display: Always enables MST master pipe first
2019-12-07 1:18 ` [Intel-gfx] [PATCH 2/4] drm/i915/display: Always enables MST master pipe first José Roberto de Souza
@ 2019-12-09 17:19 ` Ville Syrjälä
2019-12-09 18:47 ` Souza, Jose
0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2019-12-09 17:19 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
On Fri, Dec 06, 2019 at 05:18:30PM -0800, José Roberto de Souza wrote:
> Due to DDB overlaps the pipe enabling sequence is not always crescent.
> As the previous patch selects the first pipe/transcoder in the MST
> stream to the MST master and it needs to be enabled first this
> changes were needed to guarantee that.
>
> So here it will first loop through all the MST masters and other
> pipes that do not have pipe dependencies and enabling then, as when
> the master is being enabled all the slaves are also going to a full
> modeset they will not overlap with each other.
> Then on the second loop it will enable all the MST slaves.
>
> I have tried to put port sync pipes into those two loops but
> intel_update_trans_port_sync_crtcs() is doing way more than just
> enable pipes, reading spec I guess it could be accomplish but I will
> leave it to people working on port sync.
> At least now the port sync pipes are enabled by last so the slave DDB
> allocation will not overlap with other pipes.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 120 ++++++++++++++++---
> 1 file changed, 105 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index f89494c849ce..2f74c0bfb2a8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14576,6 +14576,39 @@ static void intel_update_trans_port_sync_crtcs(struct intel_crtc *crtc,
> state);
> }
>
> +static void
> +skl_commit_modeset_enable_pipe(struct intel_crtc *crtc,
> + struct intel_crtc_state *old_crtc_state,
> + struct intel_crtc_state *new_crtc_state,
> + unsigned int *updated, bool *progress,
> + struct skl_ddb_entry *entry)
> +{
> + struct intel_atomic_state *state = to_intel_atomic_state(old_crtc_state->uapi.state);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + bool vbl_wait = false;
> +
> + *updated = *updated | BIT(crtc->pipe);
> + *progress = true;
> + *entry = new_crtc_state->wm.skl.ddb;
> +
> + /*
> + * If this is an already active pipe, it's DDB changed,
> + * and this isn't the last pipe that needs updating
> + * then we need to wait for a vblank to pass for the
> + * new ddb allocation to take effect.
> + */
> + if (!needs_modeset(new_crtc_state) &&
> + state->wm_results.dirty_pipes != *updated &&
> + !skl_ddb_entry_equal(&new_crtc_state->wm.skl.ddb,
> + &old_crtc_state->wm.skl.ddb))
> + vbl_wait = true;
> +
> + intel_update_crtc(crtc, state, old_crtc_state, new_crtc_state);
> +
> + if (vbl_wait)
> + intel_wait_for_vblank(dev_priv, crtc->pipe);
> +}
> +
> static void skl_commit_modeset_enables(struct intel_atomic_state *state)
> {
> struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> @@ -14600,18 +14633,84 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
> /*
> * Whenever the number of active pipes changes, we need to make sure we
> * update the pipes in the right order so that their ddb allocations
> - * never overlap with eachother inbetween CRTC updates. Otherwise we'll
> + * never overlap with each other between CRTC updates. Otherwise we'll
> * cause pipe underruns and other bad stuff.
> + *
> + * First enable all the pipes that do not depends on other pipes while
> + * respecting the DDB allocation overlaps.
> + *
> + * TODO: integrate port sync to the loops bellow.
> + * Port sync is not respecting the DDB allocation overlaps as it
> + * was not checking for the slave port overlaps and there is more than
> + * just a pipe enable in intel_update_trans_port_sync_crtcs()
> */
> do {
> progress = false;
>
> - for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> + for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> + new_crtc_state, i) {
> + if (updated & BIT(crtc->pipe) ||
> + !new_crtc_state->hw.active)
> + continue;
> +
> + if (intel_dp_mst_is_slave_trans(new_crtc_state) ||
> + is_trans_port_sync_mode(new_crtc_state))
> + continue;
> +
> + if (skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
> + entries,
> + INTEL_NUM_PIPES(dev_priv), i))
> + continue;
> +
> + skl_commit_modeset_enable_pipe(crtc, old_crtc_state,
> + new_crtc_state, &updated,
> + &progress, &entries[i]);
> + }
> + } while (progress);
> +
> + /*
> + * Now enable all the pipes that depends on other pipe aka MST slaves
> + */
> + do {
> + progress = false;
I think we probably want to split the modeset from the other updates
now so that we can avoid repeating all this stuff for modesets.
Something like:
do {
for_each_crtc() {
if (needs_modeset()
continue;
// current thing
}
} while (progress);
for_each_crtc() {
if (!needs_modeset())
continue;
if (port_sync || mst_slave)
continue;
WARN_ON(allocaiton_overlaps());
enable_crtc();
}
for_each_crtc() {
if (!needs_modeset())
continue;
if (port_sync)
continue;
WARN_ON(allocaiton_overlaps());
enable_crtc();
}
for_each_crtc() {
if (!needs_modeset())
continue;
if (!port_sync_master)
continue;
WARN_ON(allocaiton_overlaps());
enable_port_sync();
}
Still repetitive, but at leeast we don't have to repeat all the ddb
overlap avoidance stuff.
> +
> + for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> + new_crtc_state, i) {
> + if (updated & BIT(crtc->pipe) ||
> + !new_crtc_state->hw.active)
> + continue;
> +
> + if (is_trans_port_sync_mode(new_crtc_state))
> + continue;
> +
> + if (skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
> + entries,
> + INTEL_NUM_PIPES(dev_priv), i))
> + continue;
> +
> + skl_commit_modeset_enable_pipe(crtc, old_crtc_state,
> + new_crtc_state, &updated,
> + &progress, &entries[i]);
> + }
> + } while (progress);
> +
> + /* Port sync loop */
> + do {
> + progress = false;
> +
> + for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> + new_crtc_state, i) {
> enum pipe pipe = crtc->pipe;
> bool vbl_wait = false;
> bool modeset = needs_modeset(new_crtc_state);
>
> - if (updated & BIT(crtc->pipe) || !new_crtc_state->hw.active)
> + if (updated & BIT(pipe) || !new_crtc_state->hw.active)
> + continue;
> +
> + if (!is_trans_port_sync_master(new_crtc_state))
> + continue;
> +
> + if (!modeset)
> continue;
>
> if (skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
> @@ -14634,18 +14733,9 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
> state->wm_results.dirty_pipes != updated)
> vbl_wait = true;
>
> - if (modeset && is_trans_port_sync_mode(new_crtc_state)) {
> - if (is_trans_port_sync_master(new_crtc_state))
> - intel_update_trans_port_sync_crtcs(crtc,
> - state,
> - old_crtc_state,
> - new_crtc_state);
> - else
> - continue;
> - } else {
> - intel_update_crtc(crtc, state, old_crtc_state,
> - new_crtc_state);
> - }
> + intel_update_trans_port_sync_crtcs(crtc, state,
> + old_crtc_state,
> + new_crtc_state);
>
> if (vbl_wait)
> intel_wait_for_vblank(dev_priv, pipe);
> --
> 2.24.0
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 1/4] drm/i915/tgl: Select master transcoder for MST stream
2019-12-09 16:40 ` [Intel-gfx] [PATCH 1/4] " Ville Syrjälä
@ 2019-12-09 18:45 ` Souza, Jose
2019-12-09 19:43 ` Ville Syrjälä
0 siblings, 1 reply; 16+ messages in thread
From: Souza, Jose @ 2019-12-09 18:45 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, De Marchi, Lucas
On Mon, 2019-12-09 at 18:40 +0200, Ville Syrjälä wrote:
> On Fri, Dec 06, 2019 at 05:18:29PM -0800, José Roberto de Souza
> wrote:
> > On TGL the blending of all the streams have moved from DDI to
> > transcoder, so now every transcoder working over the same MST port
> > must
> > send its stream to a master transcoder and master will send to DDI
> > respecting the time slots.
> >
> > So here adding all the CRTCs that shares the same MST stream if
> > needed and computing their state again, it will pick the first
> > transcoder among the ones in the same stream to be master.
> >
> > Most of the time skl_commit_modeset_enables() enables pipes in a
> > crescent order but due DDB overlapping it might not happen, this
> > scenario will be handled in the next patch.
> >
> > BSpec: 50493
> > BSpec: 49190
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c | 14 +-
> > drivers/gpu/drm/i915/display/intel_display.c | 15 +-
> > .../drm/i915/display/intel_display_types.h | 3 +
> > drivers/gpu/drm/i915/display/intel_dp_mst.c | 163
> > ++++++++++++++++++
> > drivers/gpu/drm/i915/display/intel_dp_mst.h | 5 +
> > 5 files changed, 196 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 3cacb1e279c1..be5bc506d4d3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -1903,8 +1903,13 @@ intel_ddi_transcoder_func_reg_val_get(const
> > struct intel_crtc_state *crtc_state)
> > temp |= TRANS_DDI_MODE_SELECT_DP_MST;
> > temp |= DDI_PORT_WIDTH(crtc_state->lane_count);
> >
> > - if (INTEL_GEN(dev_priv) >= 12)
> > - temp |=
> > TRANS_DDI_MST_TRANSPORT_SELECT(crtc_state->cpu_transcoder);
> > + if (INTEL_GEN(dev_priv) >= 12) {
> > + enum transcoder master;
> > +
> > + master = crtc_state->mst_master_transcoder;
> > + WARN_ON(master == INVALID_TRANSCODER);
>
> I'd drop the WARN_ON(). If we keep adding these for every little
> thing
> we'll drown in them.
>
> > + temp |= TRANS_DDI_MST_TRANSPORT_SELECT(master);
> > + }
> > } else {
> > temp |= TRANS_DDI_MODE_SELECT_DP_SST;
> > temp |= DDI_PORT_WIDTH(crtc_state->lane_count);
> > @@ -4372,6 +4377,11 @@ void intel_ddi_get_config(struct
> > intel_encoder *encoder,
> > pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
> > pipe_config->lane_count =
> > ((temp & DDI_PORT_WIDTH_MASK) >>
> > DDI_PORT_WIDTH_SHIFT) + 1;
> > +
> > + if (INTEL_GEN(dev_priv) >= 12)
> > + pipe_config->mst_master_transcoder =
> > + REG_FIELD_GET(TRANS_DDI_MST_TRA
> > NSPORT_SELECT_MASK, temp);
> > +
> > intel_dp_get_m_n(intel_crtc, pipe_config);
> > break;
> > default:
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 821ba8053f9d..f89494c849ce 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -46,6 +46,7 @@
> > #include "display/intel_crt.h"
> > #include "display/intel_ddi.h"
> > #include "display/intel_dp.h"
> > +#include "display/intel_dp_mst.h"
> > #include "display/intel_dsi.h"
> > #include "display/intel_dvo.h"
> > #include "display/intel_gmbus.h"
> > @@ -12542,6 +12543,11 @@ static void intel_dump_pipe_config(const
> > struct intel_crtc_state *pipe_config,
> > pipe_config->csc_mode, pipe_config-
> > >gamma_mode,
> > pipe_config->gamma_enable, pipe_config-
> > >csc_enable);
> >
> > + if (INTEL_GEN(dev_priv) >= 12 &&
> > + intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST))
>
> Could probably print this unconditionally to keep the code less
> messy.
I'm not setting mst_master_transcoder = INVALID_TRANSCODER in the other
code paths, so it would print transcoder A for HDMI, DP SST and to DP
MST in gen < 12.
That is fine? Should I add mst_master_transcoder = INVALID_TRANSCODER
to all those code paths? For me the best option is keep this checks.
>
> > + DRM_DEBUG_KMS("MST master transcoder: %s\n",
> > + transcoder_name(pipe_config-
> > >mst_master_transcoder));
> > +
> > dump_planes:
> > if (!state)
> > return;
> > @@ -13318,6 +13324,10 @@ intel_pipe_config_compare(const struct
> > intel_crtc_state *current_config,
> > PIPE_CONF_CHECK_I(sync_mode_slaves_mask);
> > PIPE_CONF_CHECK_I(master_transcoder);
> >
> > + if (INTEL_GEN(dev_priv) >= 12 &&
> > + intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST))
>
> These checks are definitely pointless.
Similar with the above, it would not cause mismatch because for non gen
12 DP MST it would compare trans A with trans A, will change that
depending on your answer to the above.
>
> > + PIPE_CONF_CHECK_I(mst_master_transcoder);
> > +
> > #undef PIPE_CONF_CHECK_X
> > #undef PIPE_CONF_CHECK_I
> > #undef PIPE_CONF_CHECK_BOOL
> > @@ -14406,7 +14416,7 @@ static void
> > intel_commit_modeset_disables(struct intel_atomic_state *state)
> > u32 handled = 0;
> > int i;
> >
> > - /* Only disable port sync slaves */
> > + /* Only disable port sync and MST slaves */
> > for_each_oldnew_intel_crtc_in_state(state, crtc,
> > old_crtc_state,
> > new_crtc_state, i) {
> > if (!needs_modeset(new_crtc_state))
> > @@ -14420,7 +14430,8 @@ static void
> > intel_commit_modeset_disables(struct intel_atomic_state *state)
> > * slave CRTCs are disabled first and then master CRTC
> > since
> > * Slave vblanks are masked till Master Vblanks.
> > */
> > - if (!is_trans_port_sync_slave(old_crtc_state))
> > + if (!is_trans_port_sync_slave(old_crtc_state) &&
> > + !intel_dp_mst_is_slave_trans(old_crtc_state))
> > continue;
> >
> > intel_pre_plane_update(state, crtc);
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 83ea04149b77..630a94892b7b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1054,6 +1054,9 @@ struct intel_crtc_state {
> >
> > /* Bitmask to indicate slaves attached */
> > u8 sync_mode_slaves_mask;
> > +
> > + /* Only valid on TGL+ */
> > + enum transcoder mst_master_transcoder;
> > };
> >
> > struct intel_crtc {
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index 926e49f449a6..49f1518e3ab9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -87,6 +87,49 @@ static int
> > intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
> > return 0;
> > }
> >
> > +/*
> > + * Iterate over all the CRTCs and set mst_master_transcoder to the
> > first active
> > + * transcoder that shares the same MST connector.
> > + *
> > + * TODO: maybe this could be optimized to keep the old master
> > transcoder
> > + */
> > +static int
> > +intel_dp_mst_master_trans_compute(struct intel_encoder *encoder,
> > + struct intel_crtc_state *pipe_config,
>
> s/pipe_config/crtc_state/
Okay
>
> > + struct drm_connector_state
> > *conn_state)
> > +{
> > + struct intel_atomic_state *state =
> > to_intel_atomic_state(pipe_config->uapi.state);
> > + struct intel_connector *conn = to_intel_connector(conn_state-
> > >connector);
>
> ocd: s/conn/connector/ all over.
Okay
>
> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > + struct intel_digital_connector_state *iter_conn_state;
> > + struct intel_connector *iter_conn;
> > + int i;
> > +
> > + if (INTEL_GEN(dev_priv) < 12)
> > + return 0;
> > +
> > + for_each_new_intel_connector_in_state(state, iter_conn,
> > + iter_conn_state, i) {
> > + struct intel_crtc_state *iter_crtc_state;
> > + struct intel_crtc *iter_crtc;
> > +
> > + if (conn->mst_port != iter_conn->mst_port ||
> > + !iter_conn_state->base.crtc)
> > + continue;
> > +
> > + iter_crtc = to_intel_crtc(iter_conn_state->base.crtc);
> > + iter_crtc_state =
> > intel_atomic_get_new_crtc_state(state,
> > + iter_
> > crtc);
> > + if (!iter_crtc_state->uapi.active)
> > + continue;
> > +
> > + pipe_config->mst_master_transcoder = iter_crtc_state-
> > >cpu_transcoder;
> > + break;
>
> So we're going to pick this based on the connector order. How does
> that
> fit in with the port sync, or did we not have port sync for MST yet?
>
> To keep everything consistent and easy my idea was to pick the first
> pipe as the master for both MST and port sync.
>
> Although I can easily imagine topologies where even that would
> land us in some interesting mess. For example:
> pipe A -> MST port B -> ... -> whatever : MST master
> pipe B -> MST port B -> ... -> tile 0 : port sync master / MST slave
> pipe C -> MST port C -> ... -> tile 1 : port sync slave / MST master
> pipe D -> MST port C -> ... -> whatever : MST slave
>
> pipe A -> MST port B -> ... -> whatever : MST master
> pipe B -> MST port B -> ... -> tile 0 : port sync master / MST slave
> pipe C -> MST port C -> ... -> whatever : MST master
> pipe D -> MST port C -> ... -> tile 1 : port sync slave / MST slave
>
> No idea if the hw can even do something like that. Needs more thought
> if/when we enable port sync with MST (assuming we're not already
> doing
> that).
According to BSpec is possible to have port sync with MST but it is not
implemented. But sure it can be easily fixed, will loop throgh all the
connectors updating mst_master_transcoder with the smallest pipe.
>
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int intel_dp_mst_compute_config(struct intel_encoder
> > *encoder,
> > struct intel_crtc_state
> > *pipe_config,
> > struct drm_connector_state
> > *conn_state)
> > @@ -154,6 +197,98 @@ static int intel_dp_mst_compute_config(struct
> > intel_encoder *encoder,
> >
> > intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
> >
> > + ret = intel_dp_mst_master_trans_compute(encoder, pipe_config,
> > + conn_state);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > +static int
> > +intel_dp_mst_atomic_master_trans_check(struct drm_connector
> > *connector,
> > + struct drm_atomic_state *state)
>
> Just pass in intel_ types pls.
Okay
>
> > +{
> > + struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > + struct drm_connector_state *new_conn_state, *old_conn_state;
> > + struct drm_connector_list_iter conn_list_iter;
>
> ocd: s/conn_list_iter/conn_iter/
Okay
>
> > + struct intel_crtc_state *intel_crtc_state;
>
> Just 'crtc_state'
Okay
>
> > + struct intel_connector *intel_conn;
> > + struct drm_crtc_state *crtc_state;
> > + struct drm_connector *conn_iter;
>
> iter_foo vs. foo_iter in this vs. the previous function. Not that I
> really like that naming convention since it can get confusing with
> the 'struct drm_connector_list_iter conn_iter'.
>
> > +
> > + if (INTEL_GEN(dev_priv) < 12)
> > + return 0;
> > +
> > + intel_conn = to_intel_connector(connector);
> > + new_conn_state = drm_atomic_get_new_connector_state(state,
> > connector);
> > + old_conn_state = drm_atomic_get_old_connector_state(state,
> > connector);
>
> These could be done when declaring the variables.
>
> > +
> > + if (!old_conn_state->crtc && !new_conn_state->crtc)
> > + return 0;
> > +
> > + /*
> > + * 3 cases that needs be handled here:
> > + * - connector going from disabled to enabled:
> > + * nothing else need to be done, drm helpers already set
> > mode_changed in
> > + * the CRTCs needed
> > + * - connector going from enabled to disabled:
> > + * if this transcoder was the master, all slaves needs a
> > modeset
> > + * - connector going from enabled to enabled but it needs a
> > modeset:
> > + * if this transcoder was the master, all slaves also needs a
> > modeset
> > + */
>
> Too may special cases IMO. I suggest just blindly marking
> everything on the same mst_port for modeset if this connector
> needs a modeset. IIRC we have a helper for that check even...
> Ah yes, intel_connector_needs_modeset().
Oooh cool, will use it.
>
> > +
> > + /* disabled -> enabled */
> > + if (!old_conn_state->crtc && new_conn_state->crtc)
> > + return 0;
> > +
> > + /* enabled -> enabled(modeset)? */
> > + if (new_conn_state->crtc) {
> > + crtc_state = drm_atomic_get_new_crtc_state(state,
> > + new_conn_sta
> > te->crtc);
> > + if (!drm_atomic_crtc_needs_modeset(crtc_state))
> > + return 0;
> > + }
> > +
> > + /* handling enabled -> enabled(modeset) and enabled -> disabled
> > */
> > + crtc_state = drm_atomic_get_old_crtc_state(state,
> > old_conn_state->crtc);
> > + intel_crtc_state = to_intel_crtc_state(crtc_state);
> > +
> > + /* If not master, nothing else needs to be handled */
> > + if (!intel_dp_mst_is_master_trans(intel_crtc_state))
> > + return 0;
> > +
> > + /* It is master, add and mark all other CRTCs in the stream as
> > changed */
> > + drm_connector_list_iter_begin(state->dev, &conn_list_iter);
> > + drm_for_each_connector_iter(conn_iter, &conn_list_iter) {
> > + struct drm_connector_state *conn_iter_state;
> > + struct intel_connector *intel_conn_iter;
> > +
> > + intel_conn_iter = to_intel_connector(conn_iter);
> > + if (intel_conn_iter->mst_port != intel_conn->mst_port)
> > + continue;
> > +
> > + conn_iter_state = drm_atomic_get_connector_state(state,
> > + conn_i
> > ter);
> > + if (IS_ERR(conn_iter_state)) {
> > + drm_connector_list_iter_end(&conn_list_iter);
> > + return PTR_ERR(conn_iter_state);
> > + }
> > +
> > + if (!conn_iter_state->crtc)
> > + continue;
> > +
> > + crtc_state = drm_atomic_get_crtc_state(state,
> > + conn_iter_state-
> > >crtc);
> > + if (IS_ERR(crtc_state)) {
> > + drm_connector_list_iter_end(&conn_list_iter);
> > + return PTR_ERR(conn_iter_state);
> > + }
> > +
> > + crtc_state->mode_changed = true;
> > + }
> > + drm_connector_list_iter_end(&conn_list_iter);
> > +
> > return 0;
> > }
> >
> > @@ -175,6 +310,10 @@ intel_dp_mst_atomic_check(struct drm_connector
> > *connector,
> > if (ret)
> > return ret;
> >
> > + ret = intel_dp_mst_atomic_master_trans_check(connector, state);
> > + if (ret)
> > + return ret;
> > +
> > if (!old_conn_state->crtc)
> > return 0;
> >
> > @@ -241,6 +380,9 @@ static void intel_mst_post_disable_dp(struct
> > intel_encoder *encoder,
> > intel_dp->active_mst_links--;
> > last_mst_stream = intel_dp->active_mst_links == 0;
> >
> > + WARN_ON(INTEL_GEN(dev_priv) >= 12 && last_mst_stream &&
> > + !intel_dp_mst_is_master_trans(old_crtc_state));
> > +
> > /*
> > * From TGL spec: "If multi-stream slave transcoder: Configure
> > * Transcoder Clock Select to direct no clock to the
> > transcoder"
> > @@ -321,6 +463,9 @@ static void intel_mst_pre_enable_dp(struct
> > intel_encoder *encoder,
> > intel_mst->connector = connector;
> > first_mst_stream = intel_dp->active_mst_links == 0;
> >
> > + WARN_ON(INTEL_GEN(dev_priv) >= 12 && first_mst_stream &&
> > + !intel_dp_mst_is_master_trans(pipe_config));
> > +
> > DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
> >
> > if (first_mst_stream)
> > @@ -726,3 +871,21 @@ intel_dp_mst_encoder_cleanup(struct
> > intel_digital_port *intel_dig_port)
> > drm_dp_mst_topology_mgr_destroy(&intel_dp->mst_mgr);
> > /* encoders will get killed by normal cleanup */
> > }
> > +
> > +bool intel_dp_mst_is_master_trans(const struct intel_crtc_state
> > *crtc_state)
> > +{
> > + struct drm_i915_private *dev_priv = to_i915(crtc_state-
> > >uapi.crtc->dev);
> > +
> > + return INTEL_GEN(dev_priv) >= 12 &&
> > + intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST) &&
> > + crtc_state->mst_master_transcoder == crtc_state-
> > >cpu_transcoder;
>
> All but the last line seem redundant.
Similar to my first answer and even if INVALID_TRANSCODER is set in
other code paths, removing the other checks would cause us to need to
check for gen 12 and MST when enabling or disabling pipes, that is why
I kept everything here.
>
> > +}
> > +
> > +bool intel_dp_mst_is_slave_trans(const struct intel_crtc_state
> > *crtc_state)
> > +{
> > + struct drm_i915_private *dev_priv = to_i915(crtc_state-
> > >uapi.crtc->dev);
> > +
> > + return INTEL_GEN(dev_priv) >= 12 &&
> > + intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST) &&
> > + crtc_state->mst_master_transcoder != crtc_state-
> > >cpu_transcoder;
> > +}
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.h
> > b/drivers/gpu/drm/i915/display/intel_dp_mst.h
> > index f660ad80db04..e40767f78343 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.h
> > @@ -6,10 +6,15 @@
> > #ifndef __INTEL_DP_MST_H__
> > #define __INTEL_DP_MST_H__
> >
> > +#include <stdbool.h>
> > +
> > struct intel_digital_port;
> > +struct intel_crtc_state;
> >
> > int intel_dp_mst_encoder_init(struct intel_digital_port
> > *intel_dig_port, int conn_id);
> > void intel_dp_mst_encoder_cleanup(struct intel_digital_port
> > *intel_dig_port);
> > int intel_dp_mst_encoder_active_links(struct intel_digital_port
> > *intel_dig_port);
> > +bool intel_dp_mst_is_master_trans(const struct intel_crtc_state
> > *crtc_state);
> > +bool intel_dp_mst_is_slave_trans(const struct intel_crtc_state
> > *crtc_state);
> >
> > #endif /* __INTEL_DP_MST_H__ */
> > --
> > 2.24.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/display: Always enables MST master pipe first
2019-12-09 17:19 ` Ville Syrjälä
@ 2019-12-09 18:47 ` Souza, Jose
0 siblings, 0 replies; 16+ messages in thread
From: Souza, Jose @ 2019-12-09 18:47 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Mon, 2019-12-09 at 19:19 +0200, Ville Syrjälä wrote:
> On Fri, Dec 06, 2019 at 05:18:30PM -0800, José Roberto de Souza
> wrote:
> > Due to DDB overlaps the pipe enabling sequence is not always
> > crescent.
> > As the previous patch selects the first pipe/transcoder in the MST
> > stream to the MST master and it needs to be enabled first this
> > changes were needed to guarantee that.
> >
> > So here it will first loop through all the MST masters and other
> > pipes that do not have pipe dependencies and enabling then, as when
> > the master is being enabled all the slaves are also going to a full
> > modeset they will not overlap with each other.
> > Then on the second loop it will enable all the MST slaves.
> >
> > I have tried to put port sync pipes into those two loops but
> > intel_update_trans_port_sync_crtcs() is doing way more than just
> > enable pipes, reading spec I guess it could be accomplish but I
> > will
> > leave it to people working on port sync.
> > At least now the port sync pipes are enabled by last so the slave
> > DDB
> > allocation will not overlap with other pipes.
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 120
> > ++++++++++++++++---
> > 1 file changed, 105 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index f89494c849ce..2f74c0bfb2a8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -14576,6 +14576,39 @@ static void
> > intel_update_trans_port_sync_crtcs(struct intel_crtc *crtc,
> > state);
> > }
> >
> > +static void
> > +skl_commit_modeset_enable_pipe(struct intel_crtc *crtc,
> > + struct intel_crtc_state *old_crtc_state,
> > + struct intel_crtc_state *new_crtc_state,
> > + unsigned int *updated, bool *progress,
> > + struct skl_ddb_entry *entry)
> > +{
> > + struct intel_atomic_state *state =
> > to_intel_atomic_state(old_crtc_state->uapi.state);
> > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > + bool vbl_wait = false;
> > +
> > + *updated = *updated | BIT(crtc->pipe);
> > + *progress = true;
> > + *entry = new_crtc_state->wm.skl.ddb;
> > +
> > + /*
> > + * If this is an already active pipe, it's DDB changed,
> > + * and this isn't the last pipe that needs updating
> > + * then we need to wait for a vblank to pass for the
> > + * new ddb allocation to take effect.
> > + */
> > + if (!needs_modeset(new_crtc_state) &&
> > + state->wm_results.dirty_pipes != *updated &&
> > + !skl_ddb_entry_equal(&new_crtc_state->wm.skl.ddb,
> > + &old_crtc_state->wm.skl.ddb))
> > + vbl_wait = true;
> > +
> > + intel_update_crtc(crtc, state, old_crtc_state, new_crtc_state);
> > +
> > + if (vbl_wait)
> > + intel_wait_for_vblank(dev_priv, crtc->pipe);
> > +}
> > +
> > static void skl_commit_modeset_enables(struct intel_atomic_state
> > *state)
> > {
> > struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> > @@ -14600,18 +14633,84 @@ static void
> > skl_commit_modeset_enables(struct intel_atomic_state *state)
> > /*
> > * Whenever the number of active pipes changes, we need to make
> > sure we
> > * update the pipes in the right order so that their ddb
> > allocations
> > - * never overlap with eachother inbetween CRTC updates.
> > Otherwise we'll
> > + * never overlap with each other between CRTC updates.
> > Otherwise we'll
> > * cause pipe underruns and other bad stuff.
> > + *
> > + * First enable all the pipes that do not depends on other
> > pipes while
> > + * respecting the DDB allocation overlaps.
> > + *
> > + * TODO: integrate port sync to the loops bellow.
> > + * Port sync is not respecting the DDB allocation overlaps as
> > it
> > + * was not checking for the slave port overlaps and there is
> > more than
> > + * just a pipe enable in intel_update_trans_port_sync_crtcs()
> > */
> > do {
> > progress = false;
> >
> > - for_each_oldnew_intel_crtc_in_state(state, crtc,
> > old_crtc_state, new_crtc_state, i) {
> > + for_each_oldnew_intel_crtc_in_state(state, crtc,
> > old_crtc_state,
> > + new_crtc_state, i)
> > {
> > + if (updated & BIT(crtc->pipe) ||
> > + !new_crtc_state->hw.active)
> > + continue;
> > +
> > + if (intel_dp_mst_is_slave_trans(new_crtc_state)
> > ||
> > + is_trans_port_sync_mode(new_crtc_state))
> > + continue;
> > +
> > + if
> > (skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
> > + entries,
> > + INTEL_NUM_PIPES
> > (dev_priv), i))
> > + continue;
> > +
> > + skl_commit_modeset_enable_pipe(crtc,
> > old_crtc_state,
> > + new_crtc_state,
> > &updated,
> > + &progress,
> > &entries[i]);
> > + }
> > + } while (progress);
> > +
> > + /*
> > + * Now enable all the pipes that depends on other pipe aka MST
> > slaves
> > + */
> > + do {
> > + progress = false;
>
> I think we probably want to split the modeset from the other updates
> now so that we can avoid repeating all this stuff for modesets.
>
> Something like:
>
> do {
> for_each_crtc() {
> if (needs_modeset()
> continue;
> // current thing
> }
> } while (progress);
>
> for_each_crtc() {
> if (!needs_modeset())
> continue;
>
> if (port_sync || mst_slave)
> continue;
>
> WARN_ON(allocaiton_overlaps());
> enable_crtc();
> }
>
> for_each_crtc() {
> if (!needs_modeset())
> continue;
>
> if (port_sync)
> continue;
>
> WARN_ON(allocaiton_overlaps());
> enable_crtc();
> }
>
> for_each_crtc() {
> if (!needs_modeset())
> continue;
>
> if (!port_sync_master)
> continue;
>
> WARN_ON(allocaiton_overlaps());
> enable_port_sync();
> }
>
> Still repetitive, but at leeast we don't have to repeat all the ddb
> overlap avoidance stuff.
Looks good, will update it.
>
> > +
> > + for_each_oldnew_intel_crtc_in_state(state, crtc,
> > old_crtc_state,
> > + new_crtc_state, i)
> > {
> > + if (updated & BIT(crtc->pipe) ||
> > + !new_crtc_state->hw.active)
> > + continue;
> > +
> > + if (is_trans_port_sync_mode(new_crtc_state))
> > + continue;
> > +
> > + if
> > (skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
> > + entries,
> > + INTEL_NUM_PIPES
> > (dev_priv), i))
> > + continue;
> > +
> > + skl_commit_modeset_enable_pipe(crtc,
> > old_crtc_state,
> > + new_crtc_state,
> > &updated,
> > + &progress,
> > &entries[i]);
> > + }
> > + } while (progress);
> > +
> > + /* Port sync loop */
> > + do {
> > + progress = false;
> > +
> > + for_each_oldnew_intel_crtc_in_state(state, crtc,
> > old_crtc_state,
> > + new_crtc_state, i)
> > {
> > enum pipe pipe = crtc->pipe;
> > bool vbl_wait = false;
> > bool modeset = needs_modeset(new_crtc_state);
> >
> > - if (updated & BIT(crtc->pipe) ||
> > !new_crtc_state->hw.active)
> > + if (updated & BIT(pipe) || !new_crtc_state-
> > >hw.active)
> > + continue;
> > +
> > + if (!is_trans_port_sync_master(new_crtc_state))
> > + continue;
> > +
> > + if (!modeset)
> > continue;
> >
> > if
> > (skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
> > @@ -14634,18 +14733,9 @@ static void
> > skl_commit_modeset_enables(struct intel_atomic_state *state)
> > state->wm_results.dirty_pipes != updated)
> > vbl_wait = true;
> >
> > - if (modeset &&
> > is_trans_port_sync_mode(new_crtc_state)) {
> > - if
> > (is_trans_port_sync_master(new_crtc_state))
> > - intel_update_trans_port_sync_cr
> > tcs(crtc,
> > -
> > state,
> > -
> > old_crtc_state,
> > -
> > new_crtc_state);
> > - else
> > - continue;
> > - } else {
> > - intel_update_crtc(crtc, state,
> > old_crtc_state,
> > - new_crtc_state);
> > - }
> > + intel_update_trans_port_sync_crtcs(crtc, state,
> > + old_crtc_sta
> > te,
> > + new_crtc_sta
> > te);
> >
> > if (vbl_wait)
> > intel_wait_for_vblank(dev_priv, pipe);
> > --
> > 2.24.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 1/4] drm/i915/tgl: Select master transcoder for MST stream
2019-12-09 18:45 ` Souza, Jose
@ 2019-12-09 19:43 ` Ville Syrjälä
2019-12-10 2:16 ` Souza, Jose
0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2019-12-09 19:43 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx, De Marchi, Lucas
On Mon, Dec 09, 2019 at 06:45:43PM +0000, Souza, Jose wrote:
> On Mon, 2019-12-09 at 18:40 +0200, Ville Syrjälä wrote:
> > On Fri, Dec 06, 2019 at 05:18:29PM -0800, José Roberto de Souza
> > wrote:
> > > On TGL the blending of all the streams have moved from DDI to
> > > transcoder, so now every transcoder working over the same MST port
> > > must
> > > send its stream to a master transcoder and master will send to DDI
> > > respecting the time slots.
> > >
> > > So here adding all the CRTCs that shares the same MST stream if
> > > needed and computing their state again, it will pick the first
> > > transcoder among the ones in the same stream to be master.
> > >
> > > Most of the time skl_commit_modeset_enables() enables pipes in a
> > > crescent order but due DDB overlapping it might not happen, this
> > > scenario will be handled in the next patch.
> > >
> > > BSpec: 50493
> > > BSpec: 49190
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/display/intel_ddi.c | 14 +-
> > > drivers/gpu/drm/i915/display/intel_display.c | 15 +-
> > > .../drm/i915/display/intel_display_types.h | 3 +
> > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 163
> > > ++++++++++++++++++
> > > drivers/gpu/drm/i915/display/intel_dp_mst.h | 5 +
> > > 5 files changed, 196 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index 3cacb1e279c1..be5bc506d4d3 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -1903,8 +1903,13 @@ intel_ddi_transcoder_func_reg_val_get(const
> > > struct intel_crtc_state *crtc_state)
> > > temp |= TRANS_DDI_MODE_SELECT_DP_MST;
> > > temp |= DDI_PORT_WIDTH(crtc_state->lane_count);
> > >
> > > - if (INTEL_GEN(dev_priv) >= 12)
> > > - temp |=
> > > TRANS_DDI_MST_TRANSPORT_SELECT(crtc_state->cpu_transcoder);
> > > + if (INTEL_GEN(dev_priv) >= 12) {
> > > + enum transcoder master;
> > > +
> > > + master = crtc_state->mst_master_transcoder;
> > > + WARN_ON(master == INVALID_TRANSCODER);
> >
> > I'd drop the WARN_ON(). If we keep adding these for every little
> > thing
> > we'll drown in them.
> >
> > > + temp |= TRANS_DDI_MST_TRANSPORT_SELECT(master);
> > > + }
> > > } else {
> > > temp |= TRANS_DDI_MODE_SELECT_DP_SST;
> > > temp |= DDI_PORT_WIDTH(crtc_state->lane_count);
> > > @@ -4372,6 +4377,11 @@ void intel_ddi_get_config(struct
> > > intel_encoder *encoder,
> > > pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
> > > pipe_config->lane_count =
> > > ((temp & DDI_PORT_WIDTH_MASK) >>
> > > DDI_PORT_WIDTH_SHIFT) + 1;
> > > +
> > > + if (INTEL_GEN(dev_priv) >= 12)
> > > + pipe_config->mst_master_transcoder =
> > > + REG_FIELD_GET(TRANS_DDI_MST_TRA
> > > NSPORT_SELECT_MASK, temp);
> > > +
> > > intel_dp_get_m_n(intel_crtc, pipe_config);
> > > break;
> > > default:
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 821ba8053f9d..f89494c849ce 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -46,6 +46,7 @@
> > > #include "display/intel_crt.h"
> > > #include "display/intel_ddi.h"
> > > #include "display/intel_dp.h"
> > > +#include "display/intel_dp_mst.h"
> > > #include "display/intel_dsi.h"
> > > #include "display/intel_dvo.h"
> > > #include "display/intel_gmbus.h"
> > > @@ -12542,6 +12543,11 @@ static void intel_dump_pipe_config(const
> > > struct intel_crtc_state *pipe_config,
> > > pipe_config->csc_mode, pipe_config-
> > > >gamma_mode,
> > > pipe_config->gamma_enable, pipe_config-
> > > >csc_enable);
> > >
> > > + if (INTEL_GEN(dev_priv) >= 12 &&
> > > + intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST))
> >
> > Could probably print this unconditionally to keep the code less
> > messy.
>
> I'm not setting mst_master_transcoder = INVALID_TRANSCODER in the other
> code paths, so it would print transcoder A for HDMI, DP SST and to DP
> MST in gen < 12.
> That is fine? Should I add mst_master_transcoder = INVALID_TRANSCODER
> to all those code paths? For me the best option is keep this checks.
I think setting to invalid would be nice.
With https://patchwork.freedesktop.org/series/69129/
we could even do it in a nice central place just the once.
>
>
> >
> > > + DRM_DEBUG_KMS("MST master transcoder: %s\n",
> > > + transcoder_name(pipe_config-
> > > >mst_master_transcoder));
> > > +
> > > dump_planes:
> > > if (!state)
> > > return;
> > > @@ -13318,6 +13324,10 @@ intel_pipe_config_compare(const struct
> > > intel_crtc_state *current_config,
> > > PIPE_CONF_CHECK_I(sync_mode_slaves_mask);
> > > PIPE_CONF_CHECK_I(master_transcoder);
> > >
> > > + if (INTEL_GEN(dev_priv) >= 12 &&
> > > + intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST))
> >
> > These checks are definitely pointless.
>
> Similar with the above, it would not cause mismatch because for non gen
> 12 DP MST it would compare trans A with trans A, will change that
> depending on your answer to the above.
>
>
> >
> > > + PIPE_CONF_CHECK_I(mst_master_transcoder);
> > > +
> > > #undef PIPE_CONF_CHECK_X
> > > #undef PIPE_CONF_CHECK_I
> > > #undef PIPE_CONF_CHECK_BOOL
> > > @@ -14406,7 +14416,7 @@ static void
> > > intel_commit_modeset_disables(struct intel_atomic_state *state)
> > > u32 handled = 0;
> > > int i;
> > >
> > > - /* Only disable port sync slaves */
> > > + /* Only disable port sync and MST slaves */
> > > for_each_oldnew_intel_crtc_in_state(state, crtc,
> > > old_crtc_state,
> > > new_crtc_state, i) {
> > > if (!needs_modeset(new_crtc_state))
> > > @@ -14420,7 +14430,8 @@ static void
> > > intel_commit_modeset_disables(struct intel_atomic_state *state)
> > > * slave CRTCs are disabled first and then master CRTC
> > > since
> > > * Slave vblanks are masked till Master Vblanks.
> > > */
> > > - if (!is_trans_port_sync_slave(old_crtc_state))
> > > + if (!is_trans_port_sync_slave(old_crtc_state) &&
> > > + !intel_dp_mst_is_slave_trans(old_crtc_state))
> > > continue;
> > >
> > > intel_pre_plane_update(state, crtc);
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > index 83ea04149b77..630a94892b7b 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > @@ -1054,6 +1054,9 @@ struct intel_crtc_state {
> > >
> > > /* Bitmask to indicate slaves attached */
> > > u8 sync_mode_slaves_mask;
> > > +
> > > + /* Only valid on TGL+ */
> > > + enum transcoder mst_master_transcoder;
> > > };
> > >
> > > struct intel_crtc {
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > index 926e49f449a6..49f1518e3ab9 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > @@ -87,6 +87,49 @@ static int
> > > intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
> > > return 0;
> > > }
> > >
> > > +/*
> > > + * Iterate over all the CRTCs and set mst_master_transcoder to the
> > > first active
> > > + * transcoder that shares the same MST connector.
> > > + *
> > > + * TODO: maybe this could be optimized to keep the old master
> > > transcoder
> > > + */
> > > +static int
> > > +intel_dp_mst_master_trans_compute(struct intel_encoder *encoder,
> > > + struct intel_crtc_state *pipe_config,
> >
> > s/pipe_config/crtc_state/
>
> Okay
>
> >
> > > + struct drm_connector_state
> > > *conn_state)
> > > +{
> > > + struct intel_atomic_state *state =
> > > to_intel_atomic_state(pipe_config->uapi.state);
> > > + struct intel_connector *conn = to_intel_connector(conn_state-
> > > >connector);
> >
> > ocd: s/conn/connector/ all over.
>
> Okay
>
> >
> > > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > + struct intel_digital_connector_state *iter_conn_state;
> > > + struct intel_connector *iter_conn;
> > > + int i;
> > > +
> > > + if (INTEL_GEN(dev_priv) < 12)
> > > + return 0;
> > > +
> > > + for_each_new_intel_connector_in_state(state, iter_conn,
> > > + iter_conn_state, i) {
> > > + struct intel_crtc_state *iter_crtc_state;
> > > + struct intel_crtc *iter_crtc;
> > > +
> > > + if (conn->mst_port != iter_conn->mst_port ||
> > > + !iter_conn_state->base.crtc)
> > > + continue;
> > > +
> > > + iter_crtc = to_intel_crtc(iter_conn_state->base.crtc);
> > > + iter_crtc_state =
> > > intel_atomic_get_new_crtc_state(state,
> > > + iter_
> > > crtc);
> > > + if (!iter_crtc_state->uapi.active)
> > > + continue;
> > > +
> > > + pipe_config->mst_master_transcoder = iter_crtc_state-
> > > >cpu_transcoder;
> > > + break;
> >
> > So we're going to pick this based on the connector order. How does
> > that
> > fit in with the port sync, or did we not have port sync for MST yet?
> >
> > To keep everything consistent and easy my idea was to pick the first
> > pipe as the master for both MST and port sync.
> >
> > Although I can easily imagine topologies where even that would
> > land us in some interesting mess. For example:
> > pipe A -> MST port B -> ... -> whatever : MST master
> > pipe B -> MST port B -> ... -> tile 0 : port sync master / MST slave
> > pipe C -> MST port C -> ... -> tile 1 : port sync slave / MST master
> > pipe D -> MST port C -> ... -> whatever : MST slave
> >
> > pipe A -> MST port B -> ... -> whatever : MST master
> > pipe B -> MST port B -> ... -> tile 0 : port sync master / MST slave
> > pipe C -> MST port C -> ... -> whatever : MST master
> > pipe D -> MST port C -> ... -> tile 1 : port sync slave / MST slave
> >
> > No idea if the hw can even do something like that. Needs more thought
> > if/when we enable port sync with MST (assuming we're not already
> > doing
> > that).
>
> According to BSpec is possible to have port sync with MST but it is not
> implemented. But sure it can be easily fixed, will loop throgh all the
> connectors updating mst_master_transcoder with the smallest pipe.
I don't think it's quite that easy since the ordering constraints of
MST vs. port sync might conflict as in my examples above. But let's
just ignore that particular can of worms for now.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 1/4] drm/i915/tgl: Select master transcoder for MST stream
2019-12-09 19:43 ` Ville Syrjälä
@ 2019-12-10 2:16 ` Souza, Jose
2019-12-10 13:02 ` Ville Syrjälä
0 siblings, 1 reply; 16+ messages in thread
From: Souza, Jose @ 2019-12-10 2:16 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, De Marchi, Lucas
On Mon, 2019-12-09 at 21:43 +0200, Ville Syrjälä wrote:
> On Mon, Dec 09, 2019 at 06:45:43PM +0000, Souza, Jose wrote:
> > On Mon, 2019-12-09 at 18:40 +0200, Ville Syrjälä wrote:
> > > On Fri, Dec 06, 2019 at 05:18:29PM -0800, José Roberto de Souza
> > > wrote:
> > > > On TGL the blending of all the streams have moved from DDI to
> > > > transcoder, so now every transcoder working over the same MST
> > > > port
> > > > must
> > > > send its stream to a master transcoder and master will send to
> > > > DDI
> > > > respecting the time slots.
> > > >
> > > > So here adding all the CRTCs that shares the same MST stream if
> > > > needed and computing their state again, it will pick the first
> > > > transcoder among the ones in the same stream to be master.
> > > >
> > > > Most of the time skl_commit_modeset_enables() enables pipes in
> > > > a
> > > > crescent order but due DDB overlapping it might not happen,
> > > > this
> > > > scenario will be handled in the next patch.
> > > >
> > > > BSpec: 50493
> > > > BSpec: 49190
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_ddi.c | 14 +-
> > > > drivers/gpu/drm/i915/display/intel_display.c | 15 +-
> > > > .../drm/i915/display/intel_display_types.h | 3 +
> > > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 163
> > > > ++++++++++++++++++
> > > > drivers/gpu/drm/i915/display/intel_dp_mst.h | 5 +
> > > > 5 files changed, 196 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > index 3cacb1e279c1..be5bc506d4d3 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > @@ -1903,8 +1903,13 @@
> > > > intel_ddi_transcoder_func_reg_val_get(const
> > > > struct intel_crtc_state *crtc_state)
> > > > temp |= TRANS_DDI_MODE_SELECT_DP_MST;
> > > > temp |= DDI_PORT_WIDTH(crtc_state->lane_count);
> > > >
> > > > - if (INTEL_GEN(dev_priv) >= 12)
> > > > - temp |=
> > > > TRANS_DDI_MST_TRANSPORT_SELECT(crtc_state->cpu_transcoder);
> > > > + if (INTEL_GEN(dev_priv) >= 12) {
> > > > + enum transcoder master;
> > > > +
> > > > + master = crtc_state-
> > > > >mst_master_transcoder;
> > > > + WARN_ON(master == INVALID_TRANSCODER);
> > >
> > > I'd drop the WARN_ON(). If we keep adding these for every little
> > > thing
> > > we'll drown in them.
> > >
> > > > + temp |=
> > > > TRANS_DDI_MST_TRANSPORT_SELECT(master);
> > > > + }
> > > > } else {
> > > > temp |= TRANS_DDI_MODE_SELECT_DP_SST;
> > > > temp |= DDI_PORT_WIDTH(crtc_state->lane_count);
> > > > @@ -4372,6 +4377,11 @@ void intel_ddi_get_config(struct
> > > > intel_encoder *encoder,
> > > > pipe_config->output_types |=
> > > > BIT(INTEL_OUTPUT_DP_MST);
> > > > pipe_config->lane_count =
> > > > ((temp & DDI_PORT_WIDTH_MASK) >>
> > > > DDI_PORT_WIDTH_SHIFT) + 1;
> > > > +
> > > > + if (INTEL_GEN(dev_priv) >= 12)
> > > > + pipe_config->mst_master_transcoder =
> > > > + REG_FIELD_GET(TRANS_DDI
> > > > _MST_TRA
> > > > NSPORT_SELECT_MASK, temp);
> > > > +
> > > > intel_dp_get_m_n(intel_crtc, pipe_config);
> > > > break;
> > > > default:
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > index 821ba8053f9d..f89494c849ce 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > @@ -46,6 +46,7 @@
> > > > #include "display/intel_crt.h"
> > > > #include "display/intel_ddi.h"
> > > > #include "display/intel_dp.h"
> > > > +#include "display/intel_dp_mst.h"
> > > > #include "display/intel_dsi.h"
> > > > #include "display/intel_dvo.h"
> > > > #include "display/intel_gmbus.h"
> > > > @@ -12542,6 +12543,11 @@ static void
> > > > intel_dump_pipe_config(const
> > > > struct intel_crtc_state *pipe_config,
> > > > pipe_config->csc_mode,
> > > > pipe_config-
> > > > > gamma_mode,
> > > > pipe_config->gamma_enable,
> > > > pipe_config-
> > > > > csc_enable);
> > > >
> > > > + if (INTEL_GEN(dev_priv) >= 12 &&
> > > > + intel_crtc_has_type(pipe_config,
> > > > INTEL_OUTPUT_DP_MST))
> > >
> > > Could probably print this unconditionally to keep the code less
> > > messy.
> >
> > I'm not setting mst_master_transcoder = INVALID_TRANSCODER in the
> > other
> > code paths, so it would print transcoder A for HDMI, DP SST and to
> > DP
> > MST in gen < 12.
> > That is fine? Should I add mst_master_transcoder =
> > INVALID_TRANSCODER
> > to all those code paths? For me the best option is keep this
> > checks.
>
> I think setting to invalid would be nice.
>
> With https://patchwork.freedesktop.org/series/69129/
> we could even do it in a nice central place just the once.
Awesome, just rvb it.
It still apply without conflicts and compile, moving this MST patches
on top.
>
> >
> > > > + DRM_DEBUG_KMS("MST master transcoder: %s\n",
> > > > + transcoder_name(pipe_config-
> > > > > mst_master_transcoder));
> > > > +
> > > > dump_planes:
> > > > if (!state)
> > > > return;
> > > > @@ -13318,6 +13324,10 @@ intel_pipe_config_compare(const struct
> > > > intel_crtc_state *current_config,
> > > > PIPE_CONF_CHECK_I(sync_mode_slaves_mask);
> > > > PIPE_CONF_CHECK_I(master_transcoder);
> > > >
> > > > + if (INTEL_GEN(dev_priv) >= 12 &&
> > > > + intel_crtc_has_type(pipe_config,
> > > > INTEL_OUTPUT_DP_MST))
> > >
> > > These checks are definitely pointless.
> >
> > Similar with the above, it would not cause mismatch because for non
> > gen
> > 12 DP MST it would compare trans A with trans A, will change that
> > depending on your answer to the above.
> >
> >
> > > > + PIPE_CONF_CHECK_I(mst_master_transcoder);
> > > > +
> > > > #undef PIPE_CONF_CHECK_X
> > > > #undef PIPE_CONF_CHECK_I
> > > > #undef PIPE_CONF_CHECK_BOOL
> > > > @@ -14406,7 +14416,7 @@ static void
> > > > intel_commit_modeset_disables(struct intel_atomic_state *state)
> > > > u32 handled = 0;
> > > > int i;
> > > >
> > > > - /* Only disable port sync slaves */
> > > > + /* Only disable port sync and MST slaves */
> > > > for_each_oldnew_intel_crtc_in_state(state, crtc,
> > > > old_crtc_state,
> > > > new_crtc_state, i)
> > > > {
> > > > if (!needs_modeset(new_crtc_state))
> > > > @@ -14420,7 +14430,8 @@ static void
> > > > intel_commit_modeset_disables(struct intel_atomic_state *state)
> > > > * slave CRTCs are disabled first and then
> > > > master CRTC
> > > > since
> > > > * Slave vblanks are masked till Master
> > > > Vblanks.
> > > > */
> > > > - if (!is_trans_port_sync_slave(old_crtc_state))
> > > > + if (!is_trans_port_sync_slave(old_crtc_state)
> > > > &&
> > > > + !intel_dp_mst_is_slave_trans(old_crtc_state
> > > > ))
> > > > continue;
> > > >
> > > > intel_pre_plane_update(state, crtc);
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > index 83ea04149b77..630a94892b7b 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > @@ -1054,6 +1054,9 @@ struct intel_crtc_state {
> > > >
> > > > /* Bitmask to indicate slaves attached */
> > > > u8 sync_mode_slaves_mask;
> > > > +
> > > > + /* Only valid on TGL+ */
> > > > + enum transcoder mst_master_transcoder;
> > > > };
> > > >
> > > > struct intel_crtc {
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > index 926e49f449a6..49f1518e3ab9 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > @@ -87,6 +87,49 @@ static int
> > > > intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
> > > > return 0;
> > > > }
> > > >
> > > > +/*
> > > > + * Iterate over all the CRTCs and set mst_master_transcoder to
> > > > the
> > > > first active
> > > > + * transcoder that shares the same MST connector.
> > > > + *
> > > > + * TODO: maybe this could be optimized to keep the old master
> > > > transcoder
> > > > + */
> > > > +static int
> > > > +intel_dp_mst_master_trans_compute(struct intel_encoder
> > > > *encoder,
> > > > + struct intel_crtc_state
> > > > *pipe_config,
> > >
> > > s/pipe_config/crtc_state/
> >
> > Okay
> >
> > > > + struct drm_connector_state
> > > > *conn_state)
> > > > +{
> > > > + struct intel_atomic_state *state =
> > > > to_intel_atomic_state(pipe_config->uapi.state);
> > > > + struct intel_connector *conn =
> > > > to_intel_connector(conn_state-
> > > > > connector);
> > >
> > > ocd: s/conn/connector/ all over.
> >
> > Okay
> >
> > > > + struct drm_i915_private *dev_priv = to_i915(encoder-
> > > > >base.dev);
> > > > + struct intel_digital_connector_state *iter_conn_state;
> > > > + struct intel_connector *iter_conn;
> > > > + int i;
> > > > +
> > > > + if (INTEL_GEN(dev_priv) < 12)
> > > > + return 0;
> > > > +
> > > > + for_each_new_intel_connector_in_state(state, iter_conn,
> > > > + iter_conn_state,
> > > > i) {
> > > > + struct intel_crtc_state *iter_crtc_state;
> > > > + struct intel_crtc *iter_crtc;
> > > > +
> > > > + if (conn->mst_port != iter_conn->mst_port ||
> > > > + !iter_conn_state->base.crtc)
> > > > + continue;
> > > > +
> > > > + iter_crtc = to_intel_crtc(iter_conn_state-
> > > > >base.crtc);
> > > > + iter_crtc_state =
> > > > intel_atomic_get_new_crtc_state(state,
> > > > +
> > > > iter_
> > > > crtc);
> > > > + if (!iter_crtc_state->uapi.active)
> > > > + continue;
> > > > +
> > > > + pipe_config->mst_master_transcoder =
> > > > iter_crtc_state-
> > > > > cpu_transcoder;
> > > > + break;
> > >
> > > So we're going to pick this based on the connector order. How
> > > does
> > > that
> > > fit in with the port sync, or did we not have port sync for MST
> > > yet?
> > >
> > > To keep everything consistent and easy my idea was to pick the
> > > first
> > > pipe as the master for both MST and port sync.
> > >
> > > Although I can easily imagine topologies where even that would
> > > land us in some interesting mess. For example:
> > > pipe A -> MST port B -> ... -> whatever : MST master
> > > pipe B -> MST port B -> ... -> tile 0 : port sync master / MST
> > > slave
> > > pipe C -> MST port C -> ... -> tile 1 : port sync slave / MST
> > > master
> > > pipe D -> MST port C -> ... -> whatever : MST slave
> > >
> > > pipe A -> MST port B -> ... -> whatever : MST master
> > > pipe B -> MST port B -> ... -> tile 0 : port sync master / MST
> > > slave
> > > pipe C -> MST port C -> ... -> whatever : MST master
> > > pipe D -> MST port C -> ... -> tile 1 : port sync slave / MST
> > > slave
> > >
> > > No idea if the hw can even do something like that. Needs more
> > > thought
> > > if/when we enable port sync with MST (assuming we're not already
> > > doing
> > > that).
> >
> > According to BSpec is possible to have port sync with MST but it is
> > not
> > implemented. But sure it can be easily fixed, will loop throgh all
> > the
> > connectors updating mst_master_transcoder with the smallest pipe.
>
> I don't think it's quite that easy since the ordering constraints of
> MST vs. port sync might conflict as in my examples above. But let's
> just ignore that particular can of worms for now.
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 1/4] drm/i915/tgl: Select master transcoder for MST stream
2019-12-10 2:16 ` Souza, Jose
@ 2019-12-10 13:02 ` Ville Syrjälä
0 siblings, 0 replies; 16+ messages in thread
From: Ville Syrjälä @ 2019-12-10 13:02 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx, De Marchi, Lucas
On Tue, Dec 10, 2019 at 02:16:23AM +0000, Souza, Jose wrote:
> On Mon, 2019-12-09 at 21:43 +0200, Ville Syrjälä wrote:
> > On Mon, Dec 09, 2019 at 06:45:43PM +0000, Souza, Jose wrote:
> > > On Mon, 2019-12-09 at 18:40 +0200, Ville Syrjälä wrote:
> > > > On Fri, Dec 06, 2019 at 05:18:29PM -0800, José Roberto de Souza
> > > > wrote:
> > > > > On TGL the blending of all the streams have moved from DDI to
> > > > > transcoder, so now every transcoder working over the same MST
> > > > > port
> > > > > must
> > > > > send its stream to a master transcoder and master will send to
> > > > > DDI
> > > > > respecting the time slots.
> > > > >
> > > > > So here adding all the CRTCs that shares the same MST stream if
> > > > > needed and computing their state again, it will pick the first
> > > > > transcoder among the ones in the same stream to be master.
> > > > >
> > > > > Most of the time skl_commit_modeset_enables() enables pipes in
> > > > > a
> > > > > crescent order but due DDB overlapping it might not happen,
> > > > > this
> > > > > scenario will be handled in the next patch.
> > > > >
> > > > > BSpec: 50493
> > > > > BSpec: 49190
> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > > ---
> > > > > drivers/gpu/drm/i915/display/intel_ddi.c | 14 +-
> > > > > drivers/gpu/drm/i915/display/intel_display.c | 15 +-
> > > > > .../drm/i915/display/intel_display_types.h | 3 +
> > > > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 163
> > > > > ++++++++++++++++++
> > > > > drivers/gpu/drm/i915/display/intel_dp_mst.h | 5 +
> > > > > 5 files changed, 196 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > > index 3cacb1e279c1..be5bc506d4d3 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > > @@ -1903,8 +1903,13 @@
> > > > > intel_ddi_transcoder_func_reg_val_get(const
> > > > > struct intel_crtc_state *crtc_state)
> > > > > temp |= TRANS_DDI_MODE_SELECT_DP_MST;
> > > > > temp |= DDI_PORT_WIDTH(crtc_state->lane_count);
> > > > >
> > > > > - if (INTEL_GEN(dev_priv) >= 12)
> > > > > - temp |=
> > > > > TRANS_DDI_MST_TRANSPORT_SELECT(crtc_state->cpu_transcoder);
> > > > > + if (INTEL_GEN(dev_priv) >= 12) {
> > > > > + enum transcoder master;
> > > > > +
> > > > > + master = crtc_state-
> > > > > >mst_master_transcoder;
> > > > > + WARN_ON(master == INVALID_TRANSCODER);
> > > >
> > > > I'd drop the WARN_ON(). If we keep adding these for every little
> > > > thing
> > > > we'll drown in them.
> > > >
> > > > > + temp |=
> > > > > TRANS_DDI_MST_TRANSPORT_SELECT(master);
> > > > > + }
> > > > > } else {
> > > > > temp |= TRANS_DDI_MODE_SELECT_DP_SST;
> > > > > temp |= DDI_PORT_WIDTH(crtc_state->lane_count);
> > > > > @@ -4372,6 +4377,11 @@ void intel_ddi_get_config(struct
> > > > > intel_encoder *encoder,
> > > > > pipe_config->output_types |=
> > > > > BIT(INTEL_OUTPUT_DP_MST);
> > > > > pipe_config->lane_count =
> > > > > ((temp & DDI_PORT_WIDTH_MASK) >>
> > > > > DDI_PORT_WIDTH_SHIFT) + 1;
> > > > > +
> > > > > + if (INTEL_GEN(dev_priv) >= 12)
> > > > > + pipe_config->mst_master_transcoder =
> > > > > + REG_FIELD_GET(TRANS_DDI
> > > > > _MST_TRA
> > > > > NSPORT_SELECT_MASK, temp);
> > > > > +
> > > > > intel_dp_get_m_n(intel_crtc, pipe_config);
> > > > > break;
> > > > > default:
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > index 821ba8053f9d..f89494c849ce 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > @@ -46,6 +46,7 @@
> > > > > #include "display/intel_crt.h"
> > > > > #include "display/intel_ddi.h"
> > > > > #include "display/intel_dp.h"
> > > > > +#include "display/intel_dp_mst.h"
> > > > > #include "display/intel_dsi.h"
> > > > > #include "display/intel_dvo.h"
> > > > > #include "display/intel_gmbus.h"
> > > > > @@ -12542,6 +12543,11 @@ static void
> > > > > intel_dump_pipe_config(const
> > > > > struct intel_crtc_state *pipe_config,
> > > > > pipe_config->csc_mode,
> > > > > pipe_config-
> > > > > > gamma_mode,
> > > > > pipe_config->gamma_enable,
> > > > > pipe_config-
> > > > > > csc_enable);
> > > > >
> > > > > + if (INTEL_GEN(dev_priv) >= 12 &&
> > > > > + intel_crtc_has_type(pipe_config,
> > > > > INTEL_OUTPUT_DP_MST))
> > > >
> > > > Could probably print this unconditionally to keep the code less
> > > > messy.
> > >
> > > I'm not setting mst_master_transcoder = INVALID_TRANSCODER in the
> > > other
> > > code paths, so it would print transcoder A for HDMI, DP SST and to
> > > DP
> > > MST in gen < 12.
> > > That is fine? Should I add mst_master_transcoder =
> > > INVALID_TRANSCODER
> > > to all those code paths? For me the best option is keep this
> > > checks.
> >
> > I think setting to invalid would be nice.
> >
> > With https://patchwork.freedesktop.org/series/69129/
> > we could even do it in a nice central place just the once.
>
> Awesome, just rvb it.
> It still apply without conflicts and compile, moving this MST patches
> on top.
Thanks. Oh, now I remember. That series didn't pass BAT. The fix for
that is the remainder of https://patchwork.freedesktop.org/series/69366/
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/i915/dp: Fix MST disable sequences
2019-12-07 1:18 ` [Intel-gfx] [PATCH 3/4] drm/i915/dp: Fix MST disable sequences José Roberto de Souza
@ 2019-12-10 21:38 ` Ville Syrjälä
2019-12-11 0:30 ` James Ausmus
0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2019-12-10 21:38 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Lucas De Marchi
On Fri, Dec 06, 2019 at 05:18:31PM -0800, José Roberto de Souza wrote:
> The disable sequence after wait for transcoder off was not correctly
> implemented.
> The MST disable sequence is basically the same for HSW, SKL, ICL and
> TGL, with just minor changes for TGL.
>
> So here calling a new MST function to do the MST sequences, most of
> the steps just moved from the post disable hook.
>
> With this last patch we finally fixed the hotplugs triggered by MST
> sinks during the disable/enable sequence, those were causing source
> to try to do a link training while it was not ready causing CPU pipe
> FIFO underrrus on TGL.
>
> BSpec: 4231
> BSpec: 4163
> BSpec: 22243
> BSpec: 49190
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 17 +++--
> drivers/gpu/drm/i915/display/intel_display.c | 2 +
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 79 ++++++++++++++++----
> drivers/gpu/drm/i915/display/intel_dp_mst.h | 1 +
> 4 files changed, 79 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index be5bc506d4d3..f06c6dfec888 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -34,6 +34,7 @@
> #include "intel_ddi.h"
> #include "intel_display_types.h"
> #include "intel_dp.h"
> +#include "intel_dp_mst.h"
> #include "intel_dp_link_training.h"
> #include "intel_dpio_phy.h"
> #include "intel_dsi.h"
> @@ -1953,17 +1954,19 @@ void intel_ddi_disable_transcoder_func(const struct intel_crtc_state *crtc_state
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> - i915_reg_t reg = TRANS_DDI_FUNC_CTL(cpu_transcoder);
> - u32 val = I915_READ(reg);
> + u32 val;
> +
> + val = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> + val &= ~TRANS_DDI_FUNC_ENABLE;
>
> if (INTEL_GEN(dev_priv) >= 12) {
> - val &= ~(TRANS_DDI_FUNC_ENABLE | TGL_TRANS_DDI_PORT_MASK |
> - TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
> + if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST) ||
> + intel_dp_mst_is_slave_trans(crtc_state))
> + val &= ~TGL_TRANS_DDI_PORT_MASK;
> } else {
> - val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_PORT_MASK |
> - TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
> + val &= ~TRANS_DDI_PORT_MASK;
> }
> - I915_WRITE(reg, val);
> + I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), val);
>
> if (dev_priv->quirks & QUIRK_INCREASE_DDI_DISABLED_TIME &&
> intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 2f74c0bfb2a8..d3eefb271fa4 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6776,6 +6776,8 @@ static void haswell_crtc_disable(struct intel_atomic_state *state,
> if (!transcoder_is_dsi(cpu_transcoder))
> intel_disable_pipe(old_crtc_state);
>
> + intel_dp_mst_post_trans_disabled(old_crtc_state);
> +
Basically a new ad-hoc encoder hook :(
I think we either want to suck in more crap from the crtc_enable/disable
into the encoder hooks, or we try to move everything from the current
.post_disable() into .post_pll_disable() and relocate .post_disabel()
to a better place in the sequence. Though the whole .post_pll_disable()
is a farily poor match for these platforms as there's nothing to do after
disabling the PLL. So from the hook naming POV I'd kinda want move things
the other way.
So I think moving more things into the encoder hooks sounds like a
better plan. Eg. in this case I think we could probably shovel everything
after intel_pipe_disable() into .post_disable(). Would also help us
eliminate some of those 'if !dsi' things.
I'm also thinking intel_disable_ddi_buf() should be split up and
just inlined into the proper place(s). Would help in making the sequence
more easily comparable with the spec.
> if (INTEL_GEN(dev_priv) >= 11)
> icl_disable_transcoder_port_sync(old_crtc_state);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 49f1518e3ab9..3c98a25e6308 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -365,6 +365,57 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder,
> old_crtc_state, old_conn_state);
> }
>
> +static void
> +dp_mst_post_trans_disabled(struct intel_encoder *encoder,
> + const struct intel_crtc_state *old_crtc_state,
> + const struct drm_connector_state *old_conn_state)
> +{
> + struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
> + struct intel_digital_port *intel_dig_port = intel_mst->primary;
> + struct intel_dp *intel_dp = &intel_dig_port->dp;
> + struct intel_connector *connector =
> + to_intel_connector(old_conn_state->connector);
> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> + u32 val;
> +
> + drm_dp_update_payload_part2(&intel_dp->mst_mgr);
> +
> + val = I915_READ(TRANS_DDI_FUNC_CTL(old_crtc_state->cpu_transcoder));
> + val &= ~TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> + I915_WRITE(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_ERROR("Timed out waiting for ACT sent when disabling\n");
> +
> + drm_dp_check_act_status(&intel_dp->mst_mgr);
> +
> + drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, connector->port);
> +}
> +
> +void
> +intel_dp_mst_post_trans_disabled(const struct intel_crtc_state *old_crtc_state)
> +{
> + struct drm_atomic_state *state = old_crtc_state->uapi.state;
> + struct drm_connector_state *old_conn_state;
> + struct drm_connector *conn;
> + int i;
> +
> + if (!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST))
> + return;
> +
> + for_each_old_connector_in_state(state, conn, old_conn_state, i) {
> + struct intel_encoder *encoder;
> +
> + if (old_conn_state->crtc != old_crtc_state->uapi.crtc)
> + continue;
> +
> + encoder = to_intel_encoder(old_conn_state->best_encoder);
> + dp_mst_post_trans_disabled(encoder, old_crtc_state,
> + old_conn_state);
> + }
> +}
> +
> static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
> const struct intel_crtc_state *old_crtc_state,
> const struct drm_connector_state *old_conn_state)
> @@ -383,6 +434,12 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
> WARN_ON(INTEL_GEN(dev_priv) >= 12 && last_mst_stream &&
> !intel_dp_mst_is_master_trans(old_crtc_state));
>
> + /*
> + * Power down mst path before disabling the port, otherwise we end
> + * up getting interrupts from the sink upon detecting link loss.
> + */
> + drm_dp_send_power_updown_phy(&intel_dp->mst_mgr, connector->port,
> + false);
> /*
> * From TGL spec: "If multi-stream slave transcoder: Configure
> * Transcoder Clock Select to direct no clock to the transcoder"
> @@ -393,24 +450,20 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
> if (INTEL_GEN(dev_priv) < 12 || !last_mst_stream)
> intel_ddi_disable_pipe_clock(old_crtc_state);
>
> - /* this can fail */
> - drm_dp_check_act_status(&intel_dp->mst_mgr);
> - /* and this can also fail */
> - drm_dp_update_payload_part2(&intel_dp->mst_mgr);
>
> - drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, connector->port);
> + intel_mst->connector = NULL;
> + if (last_mst_stream) {
> + enum transcoder cpu_transcoder;
> + u32 val;
>
> - /*
> - * Power down mst path before disabling the port, otherwise we end
> - * up getting interrupts from the sink upon detecting link loss.
> - */
> - drm_dp_send_power_updown_phy(&intel_dp->mst_mgr, connector->port,
> - false);
> + cpu_transcoder = old_crtc_state->cpu_transcoder;
> + val = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> + val &= ~TGL_TRANS_DDI_PORT_MASK;
Unconditional TGL stuff?
> + I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), val);
>
> - intel_mst->connector = NULL;
> - if (last_mst_stream)
> intel_dig_port->base.post_disable(&intel_dig_port->base,
> old_crtc_state, NULL);
> + }
>
> DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.h b/drivers/gpu/drm/i915/display/intel_dp_mst.h
> index e40767f78343..87f32fab90fc 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.h
> @@ -16,5 +16,6 @@ void intel_dp_mst_encoder_cleanup(struct intel_digital_port *intel_dig_port);
> int intel_dp_mst_encoder_active_links(struct intel_digital_port *intel_dig_port);
> bool intel_dp_mst_is_master_trans(const struct intel_crtc_state *crtc_state);
> bool intel_dp_mst_is_slave_trans(const struct intel_crtc_state *crtc_state);
> +void intel_dp_mst_post_trans_disabled(const struct intel_crtc_state *old_crtc_state);
>
> #endif /* __INTEL_DP_MST_H__ */
> --
> 2.24.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/i915/dp: Fix MST disable sequences
2019-12-10 21:38 ` Ville Syrjälä
@ 2019-12-11 0:30 ` James Ausmus
2019-12-11 2:07 ` Souza, Jose
0 siblings, 1 reply; 16+ messages in thread
From: James Ausmus @ 2019-12-11 0:30 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Lucas De Marchi
On Tue, Dec 10, 2019 at 11:38:50PM +0200, Ville Syrjälä wrote:
> On Fri, Dec 06, 2019 at 05:18:31PM -0800, José Roberto de Souza wrote:
> > The disable sequence after wait for transcoder off was not correctly
> > implemented.
> > The MST disable sequence is basically the same for HSW, SKL, ICL and
> > TGL, with just minor changes for TGL.
> >
> > So here calling a new MST function to do the MST sequences, most of
> > the steps just moved from the post disable hook.
> >
> > With this last patch we finally fixed the hotplugs triggered by MST
> > sinks during the disable/enable sequence, those were causing source
> > to try to do a link training while it was not ready causing CPU pipe
> > FIFO underrrus on TGL.
> >
> > BSpec: 4231
> > BSpec: 4163
> > BSpec: 22243
> > BSpec: 49190
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c | 17 +++--
> > drivers/gpu/drm/i915/display/intel_display.c | 2 +
> > drivers/gpu/drm/i915/display/intel_dp_mst.c | 79 ++++++++++++++++----
> > drivers/gpu/drm/i915/display/intel_dp_mst.h | 1 +
> > 4 files changed, 79 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index be5bc506d4d3..f06c6dfec888 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -34,6 +34,7 @@
> > #include "intel_ddi.h"
> > #include "intel_display_types.h"
> > #include "intel_dp.h"
> > +#include "intel_dp_mst.h"
> > #include "intel_dp_link_training.h"
> > #include "intel_dpio_phy.h"
> > #include "intel_dsi.h"
> > @@ -1953,17 +1954,19 @@ void intel_ddi_disable_transcoder_func(const struct intel_crtc_state *crtc_state
> > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > - i915_reg_t reg = TRANS_DDI_FUNC_CTL(cpu_transcoder);
> > - u32 val = I915_READ(reg);
> > + u32 val;
> > +
> > + val = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> > + val &= ~TRANS_DDI_FUNC_ENABLE;
> >
> > if (INTEL_GEN(dev_priv) >= 12) {
> > - val &= ~(TRANS_DDI_FUNC_ENABLE | TGL_TRANS_DDI_PORT_MASK |
> > - TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
> > + if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST) ||
> > + intel_dp_mst_is_slave_trans(crtc_state))
> > + val &= ~TGL_TRANS_DDI_PORT_MASK;
> > } else {
> > - val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_PORT_MASK |
> > - TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
> > + val &= ~TRANS_DDI_PORT_MASK;
> > }
> > - I915_WRITE(reg, val);
> > + I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), val);
> >
> > if (dev_priv->quirks & QUIRK_INCREASE_DDI_DISABLED_TIME &&
> > intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 2f74c0bfb2a8..d3eefb271fa4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -6776,6 +6776,8 @@ static void haswell_crtc_disable(struct intel_atomic_state *state,
> > if (!transcoder_is_dsi(cpu_transcoder))
> > intel_disable_pipe(old_crtc_state);
> >
> > + intel_dp_mst_post_trans_disabled(old_crtc_state);
> > +
>
> Basically a new ad-hoc encoder hook :(
>
> I think we either want to suck in more crap from the crtc_enable/disable
> into the encoder hooks, or we try to move everything from the current
> .post_disable() into .post_pll_disable() and relocate .post_disabel()
> to a better place in the sequence. Though the whole .post_pll_disable()
> is a farily poor match for these platforms as there's nothing to do after
> disabling the PLL. So from the hook naming POV I'd kinda want move things
> the other way.
>
> So I think moving more things into the encoder hooks sounds like a
> better plan. Eg. in this case I think we could probably shovel everything
> after intel_pipe_disable() into .post_disable(). Would also help us
> eliminate some of those 'if !dsi' things.
>
> I'm also thinking intel_disable_ddi_buf() should be split up and
> just inlined into the proper place(s). Would help in making the sequence
> more easily comparable with the spec.
Does this refactoring and cleanup need to happen for the immediate goal
of fixing MST, or can it land as a cleanup after we get this series
merged and MST is functional?
Thanks!
-James
>
>
> > if (INTEL_GEN(dev_priv) >= 11)
> > icl_disable_transcoder_port_sync(old_crtc_state);
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index 49f1518e3ab9..3c98a25e6308 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -365,6 +365,57 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder,
> > old_crtc_state, old_conn_state);
> > }
> >
> > +static void
> > +dp_mst_post_trans_disabled(struct intel_encoder *encoder,
> > + const struct intel_crtc_state *old_crtc_state,
> > + const struct drm_connector_state *old_conn_state)
> > +{
> > + struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
> > + struct intel_digital_port *intel_dig_port = intel_mst->primary;
> > + struct intel_dp *intel_dp = &intel_dig_port->dp;
> > + struct intel_connector *connector =
> > + to_intel_connector(old_conn_state->connector);
> > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > + u32 val;
> > +
> > + drm_dp_update_payload_part2(&intel_dp->mst_mgr);
> > +
> > + val = I915_READ(TRANS_DDI_FUNC_CTL(old_crtc_state->cpu_transcoder));
> > + val &= ~TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> > + I915_WRITE(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_ERROR("Timed out waiting for ACT sent when disabling\n");
> > +
> > + drm_dp_check_act_status(&intel_dp->mst_mgr);
> > +
> > + drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, connector->port);
> > +}
> > +
> > +void
> > +intel_dp_mst_post_trans_disabled(const struct intel_crtc_state *old_crtc_state)
> > +{
> > + struct drm_atomic_state *state = old_crtc_state->uapi.state;
> > + struct drm_connector_state *old_conn_state;
> > + struct drm_connector *conn;
> > + int i;
> > +
> > + if (!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST))
> > + return;
> > +
> > + for_each_old_connector_in_state(state, conn, old_conn_state, i) {
> > + struct intel_encoder *encoder;
> > +
> > + if (old_conn_state->crtc != old_crtc_state->uapi.crtc)
> > + continue;
> > +
> > + encoder = to_intel_encoder(old_conn_state->best_encoder);
> > + dp_mst_post_trans_disabled(encoder, old_crtc_state,
> > + old_conn_state);
> > + }
> > +}
> > +
> > static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
> > const struct intel_crtc_state *old_crtc_state,
> > const struct drm_connector_state *old_conn_state)
> > @@ -383,6 +434,12 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
> > WARN_ON(INTEL_GEN(dev_priv) >= 12 && last_mst_stream &&
> > !intel_dp_mst_is_master_trans(old_crtc_state));
> >
> > + /*
> > + * Power down mst path before disabling the port, otherwise we end
> > + * up getting interrupts from the sink upon detecting link loss.
> > + */
> > + drm_dp_send_power_updown_phy(&intel_dp->mst_mgr, connector->port,
> > + false);
> > /*
> > * From TGL spec: "If multi-stream slave transcoder: Configure
> > * Transcoder Clock Select to direct no clock to the transcoder"
> > @@ -393,24 +450,20 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
> > if (INTEL_GEN(dev_priv) < 12 || !last_mst_stream)
> > intel_ddi_disable_pipe_clock(old_crtc_state);
> >
> > - /* this can fail */
> > - drm_dp_check_act_status(&intel_dp->mst_mgr);
> > - /* and this can also fail */
> > - drm_dp_update_payload_part2(&intel_dp->mst_mgr);
> >
> > - drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, connector->port);
> > + intel_mst->connector = NULL;
> > + if (last_mst_stream) {
> > + enum transcoder cpu_transcoder;
> > + u32 val;
> >
> > - /*
> > - * Power down mst path before disabling the port, otherwise we end
> > - * up getting interrupts from the sink upon detecting link loss.
> > - */
> > - drm_dp_send_power_updown_phy(&intel_dp->mst_mgr, connector->port,
> > - false);
> > + cpu_transcoder = old_crtc_state->cpu_transcoder;
> > + val = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> > + val &= ~TGL_TRANS_DDI_PORT_MASK;
>
> Unconditional TGL stuff?
>
> > + I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), val);
> >
> > - intel_mst->connector = NULL;
> > - if (last_mst_stream)
> > intel_dig_port->base.post_disable(&intel_dig_port->base,
> > old_crtc_state, NULL);
> > + }
> >
> > DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
> > }
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.h b/drivers/gpu/drm/i915/display/intel_dp_mst.h
> > index e40767f78343..87f32fab90fc 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.h
> > @@ -16,5 +16,6 @@ void intel_dp_mst_encoder_cleanup(struct intel_digital_port *intel_dig_port);
> > int intel_dp_mst_encoder_active_links(struct intel_digital_port *intel_dig_port);
> > bool intel_dp_mst_is_master_trans(const struct intel_crtc_state *crtc_state);
> > bool intel_dp_mst_is_slave_trans(const struct intel_crtc_state *crtc_state);
> > +void intel_dp_mst_post_trans_disabled(const struct intel_crtc_state *old_crtc_state);
> >
> > #endif /* __INTEL_DP_MST_H__ */
> > --
> > 2.24.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/i915/dp: Fix MST disable sequences
2019-12-11 0:30 ` James Ausmus
@ 2019-12-11 2:07 ` Souza, Jose
0 siblings, 0 replies; 16+ messages in thread
From: Souza, Jose @ 2019-12-11 2:07 UTC (permalink / raw)
To: ville.syrjala, Ausmus, James; +Cc: intel-gfx, De Marchi, Lucas
On Tue, 2019-12-10 at 16:30 -0800, James Ausmus wrote:
> On Tue, Dec 10, 2019 at 11:38:50PM +0200, Ville Syrjälä wrote:
> > On Fri, Dec 06, 2019 at 05:18:31PM -0800, José Roberto de Souza
> > wrote:
> > > The disable sequence after wait for transcoder off was not
> > > correctly
> > > implemented.
> > > The MST disable sequence is basically the same for HSW, SKL, ICL
> > > and
> > > TGL, with just minor changes for TGL.
> > >
> > > So here calling a new MST function to do the MST sequences, most
> > > of
> > > the steps just moved from the post disable hook.
> > >
> > > With this last patch we finally fixed the hotplugs triggered by
> > > MST
> > > sinks during the disable/enable sequence, those were causing
> > > source
> > > to try to do a link training while it was not ready causing CPU
> > > pipe
> > > FIFO underrrus on TGL.
> > >
> > > BSpec: 4231
> > > BSpec: 4163
> > > BSpec: 22243
> > > BSpec: 49190
> > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/display/intel_ddi.c | 17 +++--
> > > drivers/gpu/drm/i915/display/intel_display.c | 2 +
> > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 79
> > > ++++++++++++++++----
> > > drivers/gpu/drm/i915/display/intel_dp_mst.h | 1 +
> > > 4 files changed, 79 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index be5bc506d4d3..f06c6dfec888 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -34,6 +34,7 @@
> > > #include "intel_ddi.h"
> > > #include "intel_display_types.h"
> > > #include "intel_dp.h"
> > > +#include "intel_dp_mst.h"
> > > #include "intel_dp_link_training.h"
> > > #include "intel_dpio_phy.h"
> > > #include "intel_dsi.h"
> > > @@ -1953,17 +1954,19 @@ void
> > > intel_ddi_disable_transcoder_func(const struct intel_crtc_state
> > > *crtc_state
> > > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > > - i915_reg_t reg = TRANS_DDI_FUNC_CTL(cpu_transcoder);
> > > - u32 val = I915_READ(reg);
> > > + u32 val;
> > > +
> > > + val = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> > > + val &= ~TRANS_DDI_FUNC_ENABLE;
> > >
> > > if (INTEL_GEN(dev_priv) >= 12) {
> > > - val &= ~(TRANS_DDI_FUNC_ENABLE |
> > > TGL_TRANS_DDI_PORT_MASK |
> > > - TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
> > > + if (!intel_crtc_has_type(crtc_state,
> > > INTEL_OUTPUT_DP_MST) ||
> > > + intel_dp_mst_is_slave_trans(crtc_state))
> > > + val &= ~TGL_TRANS_DDI_PORT_MASK;
> > > } else {
> > > - val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_PORT_MASK |
> > > - TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
> > > + val &= ~TRANS_DDI_PORT_MASK;
> > > }
> > > - I915_WRITE(reg, val);
> > > + I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), val);
> > >
> > > if (dev_priv->quirks & QUIRK_INCREASE_DDI_DISABLED_TIME &&
> > > intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 2f74c0bfb2a8..d3eefb271fa4 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -6776,6 +6776,8 @@ static void haswell_crtc_disable(struct
> > > intel_atomic_state *state,
> > > if (!transcoder_is_dsi(cpu_transcoder))
> > > intel_disable_pipe(old_crtc_state);
> > >
> > > + intel_dp_mst_post_trans_disabled(old_crtc_state);
> > > +
> >
> > Basically a new ad-hoc encoder hook :(
> >
> > I think we either want to suck in more crap from the
> > crtc_enable/disable
> > into the encoder hooks, or we try to move everything from the
> > current
> > .post_disable() into .post_pll_disable() and relocate
> > .post_disabel()
> > to a better place in the sequence. Though the whole
> > .post_pll_disable()
> > is a farily poor match for these platforms as there's nothing to do
> > after
> > disabling the PLL. So from the hook naming POV I'd kinda want move
> > things
> > the other way.
> >
> > So I think moving more things into the encoder hooks sounds like a
> > better plan. Eg. in this case I think we could probably shovel
> > everything
> > after intel_pipe_disable() into .post_disable(). Would also help us
> > eliminate some of those 'if !dsi' things.
> >
> > I'm also thinking intel_disable_ddi_buf() should be split up and
> > just inlined into the proper place(s). Would help in making the
> > sequence
> > more easily comparable with the spec.
>
> Does this refactoring and cleanup need to happen for the immediate
> goal
> of fixing MST, or can it land as a cleanup after we get this series
> merged and MST is functional?
>
> Thanks!
I'm about to send a new version without this changes but with the
"Unconditional TGL stuff?" fixed.
>
> -James
>
> >
> > > if (INTEL_GEN(dev_priv) >= 11)
> > > icl_disable_transcoder_port_sync(old_crtc_state);
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > index 49f1518e3ab9..3c98a25e6308 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > @@ -365,6 +365,57 @@ static void intel_mst_disable_dp(struct
> > > intel_encoder *encoder,
> > > old_crtc_state,
> > > old_conn_state);
> > > }
> > >
> > > +static void
> > > +dp_mst_post_trans_disabled(struct intel_encoder *encoder,
> > > + const struct intel_crtc_state
> > > *old_crtc_state,
> > > + const struct drm_connector_state
> > > *old_conn_state)
> > > +{
> > > + struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder-
> > > >base);
> > > + struct intel_digital_port *intel_dig_port = intel_mst->primary;
> > > + struct intel_dp *intel_dp = &intel_dig_port->dp;
> > > + struct intel_connector *connector =
> > > + to_intel_connector(old_conn_state->connector);
> > > + struct drm_i915_private *dev_priv = to_i915(connector-
> > > >base.dev);
> > > + u32 val;
> > > +
> > > + drm_dp_update_payload_part2(&intel_dp->mst_mgr);
> > > +
> > > + val = I915_READ(TRANS_DDI_FUNC_CTL(old_crtc_state-
> > > >cpu_transcoder));
> > > + val &= ~TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> > > + I915_WRITE(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_ERROR("Timed out waiting for ACT sent when
> > > disabling\n");
> > > +
> > > + drm_dp_check_act_status(&intel_dp->mst_mgr);
> > > +
> > > + drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, connector-
> > > >port);
> > > +}
> > > +
> > > +void
> > > +intel_dp_mst_post_trans_disabled(const struct intel_crtc_state
> > > *old_crtc_state)
> > > +{
> > > + struct drm_atomic_state *state = old_crtc_state->uapi.state;
> > > + struct drm_connector_state *old_conn_state;
> > > + struct drm_connector *conn;
> > > + int i;
> > > +
> > > + if (!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST))
> > > + return;
> > > +
> > > + for_each_old_connector_in_state(state, conn, old_conn_state, i)
> > > {
> > > + struct intel_encoder *encoder;
> > > +
> > > + if (old_conn_state->crtc != old_crtc_state->uapi.crtc)
> > > + continue;
> > > +
> > > + encoder = to_intel_encoder(old_conn_state-
> > > >best_encoder);
> > > + dp_mst_post_trans_disabled(encoder, old_crtc_state,
> > > + old_conn_state);
> > > + }
> > > +}
> > > +
> > > static void intel_mst_post_disable_dp(struct intel_encoder
> > > *encoder,
> > > const struct intel_crtc_state
> > > *old_crtc_state,
> > > const struct drm_connector_state
> > > *old_conn_state)
> > > @@ -383,6 +434,12 @@ static void intel_mst_post_disable_dp(struct
> > > intel_encoder *encoder,
> > > WARN_ON(INTEL_GEN(dev_priv) >= 12 && last_mst_stream &&
> > > !intel_dp_mst_is_master_trans(old_crtc_state));
> > >
> > > + /*
> > > + * Power down mst path before disabling the port, otherwise we
> > > end
> > > + * up getting interrupts from the sink upon detecting link
> > > loss.
> > > + */
> > > + drm_dp_send_power_updown_phy(&intel_dp->mst_mgr, connector-
> > > >port,
> > > + false);
> > > /*
> > > * From TGL spec: "If multi-stream slave transcoder: Configure
> > > * Transcoder Clock Select to direct no clock to the
> > > transcoder"
> > > @@ -393,24 +450,20 @@ static void
> > > intel_mst_post_disable_dp(struct intel_encoder *encoder,
> > > if (INTEL_GEN(dev_priv) < 12 || !last_mst_stream)
> > > intel_ddi_disable_pipe_clock(old_crtc_state);
> > >
> > > - /* this can fail */
> > > - drm_dp_check_act_status(&intel_dp->mst_mgr);
> > > - /* and this can also fail */
> > > - drm_dp_update_payload_part2(&intel_dp->mst_mgr);
> > >
> > > - drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, connector-
> > > >port);
> > > + intel_mst->connector = NULL;
> > > + if (last_mst_stream) {
> > > + enum transcoder cpu_transcoder;
> > > + u32 val;
> > >
> > > - /*
> > > - * Power down mst path before disabling the port, otherwise we
> > > end
> > > - * up getting interrupts from the sink upon detecting link
> > > loss.
> > > - */
> > > - drm_dp_send_power_updown_phy(&intel_dp->mst_mgr, connector-
> > > >port,
> > > - false);
> > > + cpu_transcoder = old_crtc_state->cpu_transcoder;
> > > + val = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> > > + val &= ~TGL_TRANS_DDI_PORT_MASK;
> >
> > Unconditional TGL stuff?
> >
> > > + I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), val);
> > >
> > > - intel_mst->connector = NULL;
> > > - if (last_mst_stream)
> > > intel_dig_port->base.post_disable(&intel_dig_port-
> > > >base,
> > > old_crtc_state,
> > > NULL);
> > > + }
> > >
> > > DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
> > > }
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.h
> > > b/drivers/gpu/drm/i915/display/intel_dp_mst.h
> > > index e40767f78343..87f32fab90fc 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.h
> > > @@ -16,5 +16,6 @@ void intel_dp_mst_encoder_cleanup(struct
> > > intel_digital_port *intel_dig_port);
> > > int intel_dp_mst_encoder_active_links(struct intel_digital_port
> > > *intel_dig_port);
> > > bool intel_dp_mst_is_master_trans(const struct intel_crtc_state
> > > *crtc_state);
> > > bool intel_dp_mst_is_slave_trans(const struct intel_crtc_state
> > > *crtc_state);
> > > +void intel_dp_mst_post_trans_disabled(const struct
> > > intel_crtc_state *old_crtc_state);
> > >
> > > #endif /* __INTEL_DP_MST_H__ */
> > > --
> > > 2.24.0
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Ville Syrjälä
> > Intel
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-12-11 2:07 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-07 1:18 [Intel-gfx] [PATCH 1/4] drm/i915/tgl: Select master transcoder for MST stream José Roberto de Souza
2019-12-07 1:18 ` [Intel-gfx] [PATCH 2/4] drm/i915/display: Always enables MST master pipe first José Roberto de Souza
2019-12-09 17:19 ` Ville Syrjälä
2019-12-09 18:47 ` Souza, Jose
2019-12-07 1:18 ` [Intel-gfx] [PATCH 3/4] drm/i915/dp: Fix MST disable sequences José Roberto de Souza
2019-12-10 21:38 ` Ville Syrjälä
2019-12-11 0:30 ` James Ausmus
2019-12-11 2:07 ` Souza, Jose
2019-12-07 1:18 ` [Intel-gfx] [PATCH 4/4] drm/i915/display: Add comment to a function that probably can be removed José Roberto de Souza
2019-12-07 2:31 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915/tgl: Select master transcoder for MST stream Patchwork
2019-12-07 16:59 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2019-12-09 16:40 ` [Intel-gfx] [PATCH 1/4] " Ville Syrjälä
2019-12-09 18:45 ` Souza, Jose
2019-12-09 19:43 ` Ville Syrjälä
2019-12-10 2:16 ` Souza, Jose
2019-12-10 13:02 ` Ville Syrjälä
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.