[Intel-gfx] [PATCH 00/11] drm/i915: MST+DSC nukage and state stuff

[Intel-gfx] [PATCH 00/11] drm/i915: MST+DSC nukage and state stuff

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The big one here is removal of the defunct i915 MST DSC code.
That one clearly needs a lot more love, and the big issue
there (FEC) probably can't be done in a way that can be
easily backported. So IMO we just need to nuke the whole
MST+DSC thing for now, or else we'll end up with impossible
to debug bug reports.

The rest is mainly improvements around state
readout/check/dumping.

Ville Syrjälä (11):
  drm/dp_mst: Fix fractional DSC bpp handling
  drm/i915/mst: Remove broken MST DSC support
  drm/i915/mst: Read out FEC state
  drm/i915: Fix FEC pipe A vs. DDI A mixup
  drm/i915: Check lane count when determining FEC support
  drm/i915: Fix FEC state dump
  drm/i915: Split some long lines
  drm/i915: Introduce crtc_state->enhanced_framing
  drm/i915: Stop spamming the logs with PLL state
  drm/i915: Drop some redundant eDP checks
  drm/i915: Reduce combo PHY log spam

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   2 +-
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |   2 +-
 drivers/gpu/drm/display/drm_dp_mst_topology.c |  20 +-
 drivers/gpu/drm/i915/display/g4x_dp.c         |  10 +-
 .../gpu/drm/i915/display/intel_combo_phy.c    |  17 +-
 drivers/gpu/drm/i915/display/intel_crt.c      |   2 +
 .../drm/i915/display/intel_crtc_state_dump.c  |   3 +
 drivers/gpu/drm/i915/display/intel_ddi.c      |  29 +--
 drivers/gpu/drm/i915/display/intel_display.c  |   1 +
 .../drm/i915/display/intel_display_types.h    |   2 +
 drivers/gpu/drm/i915/display/intel_dp.c       |  29 +--
 .../drm/i915/display/intel_dp_link_training.c |   2 +-
 drivers/gpu/drm/i915/display/intel_dp_mst.c   | 181 +-----------------
 drivers/gpu/drm/i915/display/intel_fdi.c      |   9 +-
 drivers/gpu/drm/nouveau/dispnv50/disp.c       |   3 +-
 .../gpu/drm/tests/drm_dp_mst_helper_test.c    |   2 +-
 include/drm/display/drm_dp_mst_helper.h       |   2 +-
 17 files changed, 80 insertions(+), 236 deletions(-)

-- 
2.39.2

[Intel-gfx] [PATCH 01/11] drm/dp_mst: Fix fractional DSC bpp handling

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The current code does '(bpp << 4) / 16' in the MST PBN
calculation, but that is just the same as 'bpp' so the
DSC codepath achieves absolutely nothing. Fix it up so that
the fractional part of the bpp value is actually used instead
of truncated away. 64*1006 has enough zero lsbs that we can
just shift that down in the dividend and thus still manage
to stick to a 32bit divisor.

And while touching this, let's just make the whole thing more
straightforward by making the passed in bpp value .4 binary
fixed point always, instead of having to pass in different
things based on whether DSC is enabled or not.

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: David Francis <David.Francis@amd.com>
Cc: Mikita Lipski <mikita.lipski@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Fixes: dc48529fb14e ("drm/dp_mst: Add PBN calculation for DSC modes")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 +-
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  2 +-
 drivers/gpu/drm/display/drm_dp_mst_topology.c | 20 +++++--------------
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |  5 ++---
 drivers/gpu/drm/nouveau/dispnv50/disp.c       |  3 +--
 .../gpu/drm/tests/drm_dp_mst_helper_test.c    |  2 +-
 include/drm/display/drm_dp_mst_helper.h       |  2 +-
 7 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 6cacb76f389e..7d58f08a5444 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6763,7 +6763,7 @@ static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder,
 								    max_bpc);
 		bpp = convert_dc_color_depth_into_bpc(color_depth) * 3;
 		clock = adjusted_mode->clock;
-		dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp, false);
+		dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp << 4);
 	}
 
 	dm_new_connector_state->vcpi_slots =
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 994ba426ca66..eb4b666e50e8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -1515,7 +1515,7 @@ enum dc_status dm_dp_mst_is_port_support_mode(
 	} else {
 		/* check if mode could be supported within full_pbn */
 		bpp = convert_dc_color_depth_into_bpc(stream->timing.display_color_depth) * 3;
-		pbn = drm_dp_calc_pbn_mode(stream->timing.pix_clk_100hz / 10, bpp, false);
+		pbn = drm_dp_calc_pbn_mode(stream->timing.pix_clk_100hz / 10, bpp << 4);
 
 		if (pbn > aconnector->mst_output_port->full_pbn)
 			return DC_FAIL_BANDWIDTH_VALIDATE;
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 38dab76ae69e..cd4c4f22c903 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -4619,13 +4619,12 @@ EXPORT_SYMBOL(drm_dp_check_act_status);
 
 /**
  * drm_dp_calc_pbn_mode() - Calculate the PBN for a mode.
- * @clock: dot clock for the mode
- * @bpp: bpp for the mode.
- * @dsc: DSC mode. If true, bpp has units of 1/16 of a bit per pixel
+ * @clock: dot clock
+ * @bpp: bpp as .4 binary fixed point
  *
  * This uses the formula in the spec to calculate the PBN value for a mode.
  */
-int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc)
+int drm_dp_calc_pbn_mode(int clock, int bpp)
 {
 	/*
 	 * margin 5300ppm + 300ppm ~ 0.6% as per spec, factor is 1.006
@@ -4636,18 +4635,9 @@ int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc)
 	 * peak_kbps *= (1006/1000)
 	 * peak_kbps *= (64/54)
 	 * peak_kbps *= 8    convert to bytes
-	 *
-	 * If the bpp is in units of 1/16, further divide by 16. Put this
-	 * factor in the numerator rather than the denominator to avoid
-	 * integer overflow
 	 */
-
-	if (dsc)
-		return DIV_ROUND_UP_ULL(mul_u32_u32(clock * (bpp / 16), 64 * 1006),
-					8 * 54 * 1000 * 1000);
-
-	return DIV_ROUND_UP_ULL(mul_u32_u32(clock * bpp, 64 * 1006),
-				8 * 54 * 1000 * 1000);
+	return DIV_ROUND_UP_ULL(mul_u32_u32(clock * bpp, 64 * 1006 >> 4),
+				1000 * 8 * 54 * 1000);
 }
 EXPORT_SYMBOL(drm_dp_calc_pbn_mode);
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 2c49d9ab86a2..44c15d6faac4 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -109,8 +109,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
 			continue;
 
 		crtc_state->pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock,
-						       dsc ? bpp << 4 : bpp,
-						       dsc);
+						       bpp << 4);
 
 		slots = drm_dp_atomic_find_time_slots(state, &intel_dp->mst_mgr,
 						      connector->port,
@@ -936,7 +935,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
 		return ret;
 
 	if (mode_rate > max_rate || mode->clock > max_dotclk ||
-	    drm_dp_calc_pbn_mode(mode->clock, min_bpp, false) > port->full_pbn) {
+	    drm_dp_calc_pbn_mode(mode->clock, min_bpp << 4) > port->full_pbn) {
 		*status = MODE_CLOCK_HIGH;
 		return 0;
 	}
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 5bb777ff1313..d896cbb8cf3d 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -961,8 +961,7 @@ nv50_msto_atomic_check(struct drm_encoder *encoder,
 		const int clock = crtc_state->adjusted_mode.clock;
 
 		asyh->or.bpc = connector->display_info.bpc;
-		asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, asyh->or.bpc * 3,
-						    false);
+		asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, asyh->or.bpc * 3 << 4);
 	}
 
 	mst_state = drm_atomic_get_mst_topology_state(state, &mstm->mgr);
diff --git a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
index 545beea33e8c..39fc449148e1 100644
--- a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
@@ -56,7 +56,7 @@ static void drm_test_dp_mst_calc_pbn_mode(struct kunit *test)
 {
 	const struct drm_dp_mst_calc_pbn_mode_test *params = test->param_value;
 
-	KUNIT_EXPECT_EQ(test, drm_dp_calc_pbn_mode(params->clock, params->bpp, params->dsc),
+	KUNIT_EXPECT_EQ(test, drm_dp_calc_pbn_mode(params->clock, params->bpp << 4),
 			params->expected);
 }
 
diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h
index 32c764fb9cb5..c254500b4507 100644
--- a/include/drm/display/drm_dp_mst_helper.h
+++ b/include/drm/display/drm_dp_mst_helper.h
@@ -829,7 +829,7 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_
 int drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr,
 			     int link_rate, int link_lane_count);
 
-int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc);
+int drm_dp_calc_pbn_mode(int clock, int bpp);
 
 void drm_dp_mst_update_slots(struct drm_dp_mst_topology_state *mst_state, uint8_t link_encoding_cap);
 
-- 
2.39.2

[Intel-gfx] [PATCH 02/11] drm/i915/mst: Remove broken MST DSC support

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The MST DSC code has a myriad of issues:
- Platform checks are wrong (MST+DSC is TGL+ only IIRC)
- Return values of .mode_valid_ctx() are wrong
- .mode_valid_ctx() assumes bigjoiner might be used, but ther rest
  of the code doesn't agree
- compressed bpp calculations don't make sense
- FEC handling needs to consider the entire link as opposed to just
  the single stream. Currently FEC would only get enabled if the
  first enabled stream is compressed. Also I'm not seeing anything
  that would account for the FEC overhead in any bandwidth calculations
- PPS SDP is only handled for the first stream via the dig_port
  hooks, other streams will not be transmittitng any PPS SDPs
- PPS SDP readout is missing (also missing for SST!)
- VDSC readout is missing (also missing for SST!)

The FEC issues is really the big one since we have no way currently
to apply such link wide configuration constraints. Changing that is
going to require a much bigger rework of the higher level modeset
.compute_config() logic. We will also need such a rework to properly
distribute the available bandwidth across all the streams on the
same link (which is a must to eg. enable deep color).

Cc: stable@vger.kernel.org
Cc: Vinod Govindapillai <vinod.govindapillai@intel.com>
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Fixes: d51f25eb479a ("drm/i915: Add DSC support to MST path")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 176 +-------------------
 1 file changed, 5 insertions(+), 171 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 44c15d6faac4..d762f37fafb5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -72,8 +72,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
 						int min_bpp,
 						struct link_config_limits *limits,
 						struct drm_connector_state *conn_state,
-						int step,
-						bool dsc)
+						int step)
 {
 	struct drm_atomic_state *state = crtc_state->uapi.state;
 	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
@@ -104,7 +103,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
 	for (bpp = max_bpp; bpp >= min_bpp; bpp -= step) {
 		drm_dbg_kms(&i915->drm, "Trying bpp %d\n", bpp);
 
-		ret = intel_dp_mst_check_constraints(i915, bpp, adjusted_mode, crtc_state, dsc);
+		ret = intel_dp_mst_check_constraints(i915, bpp, adjusted_mode, crtc_state, false);
 		if (ret)
 			continue;
 
@@ -136,11 +135,8 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
 		drm_dbg_kms(&i915->drm, "failed finding vcpi slots:%d\n",
 			    slots);
 	} else {
-		if (!dsc)
-			crtc_state->pipe_bpp = bpp;
-		else
-			crtc_state->dsc.compressed_bpp = bpp;
-		drm_dbg_kms(&i915->drm, "Got %d slots for pipe bpp %d dsc %d\n", slots, bpp, dsc);
+		crtc_state->pipe_bpp = bpp;
+		drm_dbg_kms(&i915->drm, "Got %d slots for pipe bpp %d\n", slots, bpp);
 	}
 
 	return slots;
@@ -157,7 +153,7 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
 
 	slots = intel_dp_mst_find_vcpi_slots_for_bpp(encoder, crtc_state, limits->max_bpp,
 						     limits->min_bpp, limits,
-						     conn_state, 2 * 3, false);
+						     conn_state, 2 * 3);
 
 	if (slots < 0)
 		return slots;
@@ -173,99 +169,6 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
 	return 0;
 }
 
-static int intel_dp_dsc_mst_compute_link_config(struct intel_encoder *encoder,
-						struct intel_crtc_state *crtc_state,
-						struct drm_connector_state *conn_state,
-						struct link_config_limits *limits)
-{
-	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
-	struct intel_dp *intel_dp = &intel_mst->primary->dp;
-	struct intel_connector *connector =
-		to_intel_connector(conn_state->connector);
-	struct drm_i915_private *i915 = to_i915(connector->base.dev);
-	const struct drm_display_mode *adjusted_mode =
-		&crtc_state->hw.adjusted_mode;
-	int slots = -EINVAL;
-	int i, num_bpc;
-	u8 dsc_bpc[3] = {0};
-	int min_bpp, max_bpp, sink_min_bpp, sink_max_bpp;
-	u8 dsc_max_bpc;
-	bool need_timeslot_recalc = false;
-	u32 last_compressed_bpp;
-
-	/* Max DSC Input BPC for ICL is 10 and for TGL+ is 12 */
-	if (DISPLAY_VER(i915) >= 12)
-		dsc_max_bpc = min_t(u8, 12, conn_state->max_requested_bpc);
-	else
-		dsc_max_bpc = min_t(u8, 10, conn_state->max_requested_bpc);
-
-	max_bpp = min_t(u8, dsc_max_bpc * 3, limits->max_bpp);
-	min_bpp = limits->min_bpp;
-
-	num_bpc = drm_dp_dsc_sink_supported_input_bpcs(intel_dp->dsc_dpcd,
-						       dsc_bpc);
-
-	drm_dbg_kms(&i915->drm, "DSC Source supported min bpp %d max bpp %d\n",
-		    min_bpp, max_bpp);
-
-	sink_max_bpp = dsc_bpc[0] * 3;
-	sink_min_bpp = sink_max_bpp;
-
-	for (i = 1; i < num_bpc; i++) {
-		if (sink_min_bpp > dsc_bpc[i] * 3)
-			sink_min_bpp = dsc_bpc[i] * 3;
-		if (sink_max_bpp < dsc_bpc[i] * 3)
-			sink_max_bpp = dsc_bpc[i] * 3;
-	}
-
-	drm_dbg_kms(&i915->drm, "DSC Sink supported min bpp %d max bpp %d\n",
-		    sink_min_bpp, sink_max_bpp);
-
-	if (min_bpp < sink_min_bpp)
-		min_bpp = sink_min_bpp;
-
-	if (max_bpp > sink_max_bpp)
-		max_bpp = sink_max_bpp;
-
-	slots = intel_dp_mst_find_vcpi_slots_for_bpp(encoder, crtc_state, max_bpp,
-						     min_bpp, limits,
-						     conn_state, 2 * 3, true);
-
-	if (slots < 0)
-		return slots;
-
-	last_compressed_bpp = crtc_state->dsc.compressed_bpp;
-
-	crtc_state->dsc.compressed_bpp = intel_dp_dsc_nearest_valid_bpp(i915,
-									last_compressed_bpp,
-									crtc_state->pipe_bpp);
-
-	if (crtc_state->dsc.compressed_bpp != last_compressed_bpp)
-		need_timeslot_recalc = true;
-
-	/*
-	 * Apparently some MST hubs dislike if vcpi slots are not matching precisely
-	 * the actual compressed bpp we use.
-	 */
-	if (need_timeslot_recalc) {
-		slots = intel_dp_mst_find_vcpi_slots_for_bpp(encoder, crtc_state,
-							     crtc_state->dsc.compressed_bpp,
-							     crtc_state->dsc.compressed_bpp,
-							     limits, conn_state, 2 * 3, true);
-		if (slots < 0)
-			return slots;
-	}
-
-	intel_link_compute_m_n(crtc_state->dsc.compressed_bpp,
-			       crtc_state->lane_count,
-			       adjusted_mode->crtc_clock,
-			       crtc_state->port_clock,
-			       &crtc_state->dp_m_n,
-			       crtc_state->fec_enable);
-	crtc_state->dp_m_n.tu = slots;
-
-	return 0;
-}
 static int intel_dp_mst_update_slots(struct intel_encoder *encoder,
 				     struct intel_crtc_state *crtc_state,
 				     struct drm_connector_state *conn_state)
@@ -349,29 +252,6 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
 
 	ret = intel_dp_mst_compute_link_config(encoder, pipe_config,
 					       conn_state, &limits);
-
-	if (ret == -EDEADLK)
-		return ret;
-
-	/* enable compression if the mode doesn't fit available BW */
-	drm_dbg_kms(&dev_priv->drm, "Force DSC en = %d\n", intel_dp->force_dsc_en);
-	if (ret || intel_dp->force_dsc_en) {
-		/*
-		 * Try to get at least some timeslots and then see, if
-		 * we can fit there with DSC.
-		 */
-		drm_dbg_kms(&dev_priv->drm, "Trying to find VCPI slots in DSC mode\n");
-
-		ret = intel_dp_dsc_mst_compute_link_config(encoder, pipe_config,
-							   conn_state, &limits);
-		if (ret < 0)
-			return ret;
-
-		ret = intel_dp_dsc_compute_config(intel_dp, pipe_config,
-						  conn_state, &limits,
-						  pipe_config->dp_m_n.tu, false);
-	}
-
 	if (ret)
 		return ret;
 
@@ -909,10 +789,6 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
 	int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
 	int max_rate, mode_rate, max_lanes, max_link_clock;
 	int ret;
-	bool dsc = false, bigjoiner = false;
-	u16 dsc_max_output_bpp = 0;
-	u8 dsc_slice_count = 0;
-	int target_clock = mode->clock;
 
 	if (drm_connector_is_unregistered(connector)) {
 		*status = MODE_ERROR;
@@ -950,48 +826,6 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
 		return 0;
 	}
 
-	if (intel_dp_need_bigjoiner(intel_dp, mode->hdisplay, target_clock)) {
-		bigjoiner = true;
-		max_dotclk *= 2;
-	}
-
-	if (DISPLAY_VER(dev_priv) >= 10 &&
-	    drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)) {
-		/*
-		 * TBD pass the connector BPC,
-		 * for now U8_MAX so that max BPC on that platform would be picked
-		 */
-		int pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, U8_MAX);
-
-		if (drm_dp_sink_supports_fec(intel_dp->fec_capable)) {
-			dsc_max_output_bpp =
-				intel_dp_dsc_get_output_bpp(dev_priv,
-							    max_link_clock,
-							    max_lanes,
-							    target_clock,
-							    mode->hdisplay,
-							    bigjoiner,
-							    pipe_bpp, 64) >> 4;
-			dsc_slice_count =
-				intel_dp_dsc_get_slice_count(intel_dp,
-							     target_clock,
-							     mode->hdisplay,
-							     bigjoiner);
-		}
-
-		dsc = dsc_max_output_bpp && dsc_slice_count;
-	}
-
-	/*
-	 * Big joiner configuration needs DSC for TGL which is not true for
-	 * XE_LPD where uncompressed joiner is supported.
-	 */
-	if (DISPLAY_VER(dev_priv) < 13 && bigjoiner && !dsc)
-		return MODE_CLOCK_HIGH;
-
-	if (mode_rate > max_rate && !dsc)
-		return MODE_CLOCK_HIGH;
-
 	*status = intel_mode_valid_max_plane_size(dev_priv, mode, false);
 	return 0;
 }
-- 
2.39.2

[Intel-gfx] [PATCH 03/11] drm/i915/mst: Read out FEC state

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The MST codepath is missing FEC readout. Add it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 55f36d9d509c..41cfa28166e4 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3763,6 +3763,11 @@ static void intel_ddi_read_func_ctl(struct intel_encoder *encoder,
 		intel_cpu_transcoder_get_m1_n1(crtc, cpu_transcoder,
 					       &pipe_config->dp_m_n);
 
+		if (DISPLAY_VER(dev_priv) >= 11)
+			pipe_config->fec_enable =
+				intel_de_read(dev_priv,
+					      dp_tp_ctl_reg(encoder, pipe_config)) & DP_TP_CTL_FEC_ENABLE;
+
 		pipe_config->infoframes.enable |=
 			intel_hdmi_infoframes_enabled(encoder, pipe_config);
 		break;
-- 
2.39.2

[Intel-gfx] [PATCH 04/11] drm/i915: Fix FEC pipe A vs. DDI A mixup

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

On pre-TGL FEC is a port level feature, not a transcoder
level features, and it's DDI A which doesn't have it, not
trancodere A. Check for the correct thing when determining
whether FEC is supported or not.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 4361c1ac65c3..b27b4fb71ed7 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1212,13 +1212,13 @@ void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
 static bool intel_dp_source_supports_fec(struct intel_dp *intel_dp,
 					 const struct intel_crtc_state *pipe_config)
 {
+	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
 
-	/* On TGL, FEC is supported on all Pipes */
 	if (DISPLAY_VER(dev_priv) >= 12)
 		return true;
 
-	if (DISPLAY_VER(dev_priv) == 11 && pipe_config->cpu_transcoder != TRANSCODER_A)
+	if (DISPLAY_VER(dev_priv) == 11 && encoder->port != PORT_A)
 		return true;
 
 	return false;
-- 
2.39.2

[Intel-gfx] [PATCH 05/11] drm/i915: Check lane count when determining FEC support

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

ICL doesn't support FEC with a x1 DP link. Make sure
we don't try to enable FEC in such cases.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index b27b4fb71ed7..9ac199444155 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1218,7 +1218,8 @@ static bool intel_dp_source_supports_fec(struct intel_dp *intel_dp,
 	if (DISPLAY_VER(dev_priv) >= 12)
 		return true;
 
-	if (DISPLAY_VER(dev_priv) == 11 && encoder->port != PORT_A)
+	if (DISPLAY_VER(dev_priv) == 11 &&
+	    encoder->port != PORT_A && pipe_config->lane_count != 1)
 		return true;
 
 	return false;
@@ -1234,7 +1235,7 @@ static bool intel_dp_supports_fec(struct intel_dp *intel_dp,
 static bool intel_dp_supports_dsc(struct intel_dp *intel_dp,
 				  const struct intel_crtc_state *crtc_state)
 {
-	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP) && !crtc_state->fec_enable)
+	if (!intel_dp_is_edp(intel_dp) && !crtc_state->fec_enable)
 		return false;
 
 	return intel_dsc_source_support(crtc_state) &&
@@ -1580,15 +1581,6 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
 	int pipe_bpp;
 	int ret;
 
-	pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
-		intel_dp_supports_fec(intel_dp, pipe_config);
-
-	if (!intel_dp_supports_dsc(intel_dp, pipe_config))
-		return -EINVAL;
-
-	if (!intel_dp_dsc_supports_format(intel_dp, pipe_config->output_format))
-		return -EINVAL;
-
 	if (compute_pipe_bpp)
 		pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, conn_state->max_requested_bpc);
 	else
@@ -1615,6 +1607,15 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
 	pipe_config->port_clock = limits->max_rate;
 	pipe_config->lane_count = limits->max_lane_count;
 
+	pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
+		intel_dp_supports_fec(intel_dp, pipe_config);
+
+	if (!intel_dp_supports_dsc(intel_dp, pipe_config))
+		return -EINVAL;
+
+	if (!intel_dp_dsc_supports_format(intel_dp, pipe_config->output_format))
+		return -EINVAL;
+
 	if (intel_dp_is_edp(intel_dp)) {
 		pipe_config->dsc.compressed_bpp =
 			min_t(u16, drm_edp_dsc_sink_output_bpp(intel_dp->dsc_dpcd) >> 4,
-- 
2.39.2

[Intel-gfx] [PATCH 06/11] drm/i915: Fix FEC state dump

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Stop dumping state while reading it out. We have a proper
place for that stuff.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../gpu/drm/i915/display/intel_crtc_state_dump.c    |  2 ++
 drivers/gpu/drm/i915/display/intel_ddi.c            | 13 +++----------
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
index 0cdcaa49656f..91242ffe0768 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
@@ -257,6 +257,8 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
 		intel_dump_m_n_config(pipe_config, "dp m2_n2",
 				      pipe_config->lane_count,
 				      &pipe_config->dp_m2_n2);
+		drm_dbg_kms(&i915->drm, "fec: %s\n",
+			    str_enabled_disabled(pipe_config->fec_enable));
 	}
 
 	drm_dbg_kms(&i915->drm, "framestart delay: %d, MSA timing delay: %d\n",
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 41cfa28166e4..4246133950fd 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3725,17 +3725,10 @@ static void intel_ddi_read_func_ctl(struct intel_encoder *encoder,
 		intel_cpu_transcoder_get_m2_n2(crtc, cpu_transcoder,
 					       &pipe_config->dp_m2_n2);
 
-		if (DISPLAY_VER(dev_priv) >= 11) {
-			i915_reg_t dp_tp_ctl = dp_tp_ctl_reg(encoder, pipe_config);
-
+		if (DISPLAY_VER(dev_priv) >= 11)
 			pipe_config->fec_enable =
-				intel_de_read(dev_priv, dp_tp_ctl) & DP_TP_CTL_FEC_ENABLE;
-
-			drm_dbg_kms(&dev_priv->drm,
-				    "[ENCODER:%d:%s] Fec status: %u\n",
-				    encoder->base.base.id, encoder->base.name,
-				    pipe_config->fec_enable);
-		}
+				intel_de_read(dev_priv,
+					      dp_tp_ctl_reg(encoder, pipe_config)) & DP_TP_CTL_FEC_ENABLE;
 
 		if (dig_port->lspcon.active && dig_port->dp.has_hdmi_sink)
 			pipe_config->infoframes.enable |=
-- 
2.39.2

[Intel-gfx] [PATCH 07/11] drm/i915: Split some long lines

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Split some overly long lines.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_fdi.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fdi.c b/drivers/gpu/drm/i915/display/intel_fdi.c
index 55283677c45a..19ee78ba3936 100644
--- a/drivers/gpu/drm/i915/display/intel_fdi.c
+++ b/drivers/gpu/drm/i915/display/intel_fdi.c
@@ -765,7 +765,10 @@ void hsw_fdi_link_train(struct intel_encoder *encoder,
 	 * WaFDIAutoLinkSetTimingOverrride:hsw
 	 */
 	intel_de_write(dev_priv, FDI_RX_MISC(PIPE_A),
-		       FDI_RX_PWRDN_LANE1_VAL(2) | FDI_RX_PWRDN_LANE0_VAL(2) | FDI_RX_TP1_TO_TP2_48 | FDI_RX_FDI_DELAY_90);
+		       FDI_RX_PWRDN_LANE1_VAL(2) |
+		       FDI_RX_PWRDN_LANE0_VAL(2) |
+		       FDI_RX_TP1_TO_TP2_48 |
+		       FDI_RX_FDI_DELAY_90);
 
 	/* Enable the PCH Receiver FDI PLL */
 	rx_ctl_val = dev_priv->display.fdi.rx_config | FDI_RX_ENHANCE_FRAME_ENABLE |
@@ -798,7 +801,9 @@ void hsw_fdi_link_train(struct intel_encoder *encoder,
 		 * achieved on the PCH side in FDI_RX_CTL, so no need to set the
 		 * port reversal bit */
 		intel_de_write(dev_priv, DDI_BUF_CTL(PORT_E),
-			       DDI_BUF_CTL_ENABLE | ((crtc_state->fdi_lanes - 1) << 1) | DDI_BUF_TRANS_SELECT(i / 2));
+			       DDI_BUF_CTL_ENABLE |
+			       ((crtc_state->fdi_lanes - 1) << 1) |
+			       DDI_BUF_TRANS_SELECT(i / 2));
 		intel_de_posting_read(dev_priv, DDI_BUF_CTL(PORT_E));
 
 		udelay(600);
-- 
2.39.2

[Intel-gfx] [PATCH 08/11] drm/i915: Introduce crtc_state->enhanced_framing

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Track DP enhanced framing properly in the crtc state instead
of relying just on the cached DPCD everywhere, and hook it
up into the state check and dump.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/g4x_dp.c                 | 10 ++++++++--
 drivers/gpu/drm/i915/display/intel_crt.c              |  2 ++
 drivers/gpu/drm/i915/display/intel_crtc_state_dump.c  |  5 +++--
 drivers/gpu/drm/i915/display/intel_ddi.c              | 11 +++++++++--
 drivers/gpu/drm/i915/display/intel_display.c          |  1 +
 drivers/gpu/drm/i915/display/intel_display_types.h    |  2 ++
 drivers/gpu/drm/i915/display/intel_dp_link_training.c |  2 +-
 7 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c
index 920d570f7594..534546ea7d0b 100644
--- a/drivers/gpu/drm/i915/display/g4x_dp.c
+++ b/drivers/gpu/drm/i915/display/g4x_dp.c
@@ -141,7 +141,7 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
 
 		intel_de_rmw(dev_priv, TRANS_DP_CTL(crtc->pipe),
 			     TRANS_DP_ENH_FRAMING,
-			     drm_dp_enhanced_frame_cap(intel_dp->dpcd) ?
+			     pipe_config->enhanced_framing ?
 			     TRANS_DP_ENH_FRAMING : 0);
 	} else {
 		if (IS_G4X(dev_priv) && pipe_config->limited_color_range)
@@ -153,7 +153,7 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
 			intel_dp->DP |= DP_SYNC_VS_HIGH;
 		intel_dp->DP |= DP_LINK_TRAIN_OFF;
 
-		if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
+		if (pipe_config->enhanced_framing)
 			intel_dp->DP |= DP_ENHANCED_FRAMING;
 
 		if (IS_CHERRYVIEW(dev_priv))
@@ -351,6 +351,9 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 		u32 trans_dp = intel_de_read(dev_priv,
 					     TRANS_DP_CTL(crtc->pipe));
 
+		if (trans_dp & TRANS_DP_ENH_FRAMING)
+			pipe_config->enhanced_framing = true;
+
 		if (trans_dp & TRANS_DP_HSYNC_ACTIVE_HIGH)
 			flags |= DRM_MODE_FLAG_PHSYNC;
 		else
@@ -361,6 +364,9 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 		else
 			flags |= DRM_MODE_FLAG_NVSYNC;
 	} else {
+		if (tmp & DP_ENHANCED_FRAMING)
+			pipe_config->enhanced_framing = true;
+
 		if (tmp & DP_SYNC_HS_HIGH)
 			flags |= DRM_MODE_FLAG_PHSYNC;
 		else
diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
index 13519f78cf9f..52af64aa9953 100644
--- a/drivers/gpu/drm/i915/display/intel_crt.c
+++ b/drivers/gpu/drm/i915/display/intel_crt.c
@@ -449,6 +449,8 @@ static int hsw_crt_compute_config(struct intel_encoder *encoder,
 	/* FDI must always be 2.7 GHz */
 	pipe_config->port_clock = 135000 * 2;
 
+	pipe_config->enhanced_framing = true;
+
 	adjusted_mode->crtc_clock = lpt_iclkip(pipe_config);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
index 91242ffe0768..14db2b481ff1 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
@@ -257,8 +257,9 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
 		intel_dump_m_n_config(pipe_config, "dp m2_n2",
 				      pipe_config->lane_count,
 				      &pipe_config->dp_m2_n2);
-		drm_dbg_kms(&i915->drm, "fec: %s\n",
-			    str_enabled_disabled(pipe_config->fec_enable));
+		drm_dbg_kms(&i915->drm, "fec: %s, enhanced framing: %s\n",
+			    str_enabled_disabled(pipe_config->fec_enable),
+			    str_enabled_disabled(pipe_config->enhanced_framing));
 	}
 
 	drm_dbg_kms(&i915->drm, "framestart delay: %d, MSA timing delay: %d\n",
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 4246133950fd..51ae1aad7cc7 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3434,7 +3434,7 @@ static void mtl_ddi_prepare_link_retrain(struct intel_dp *intel_dp,
 		dp_tp_ctl |= DP_TP_CTL_MODE_MST;
 	} else {
 		dp_tp_ctl |= DP_TP_CTL_MODE_SST;
-		if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
+		if (crtc_state->enhanced_framing)
 			dp_tp_ctl |= DP_TP_CTL_ENHANCED_FRAME_ENABLE;
 	}
 	intel_de_write(dev_priv, dp_tp_ctl_reg(encoder, crtc_state), dp_tp_ctl);
@@ -3491,7 +3491,7 @@ static void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp,
 		dp_tp_ctl |= DP_TP_CTL_MODE_MST;
 	} else {
 		dp_tp_ctl |= DP_TP_CTL_MODE_SST;
-		if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
+		if (crtc_state->enhanced_framing)
 			dp_tp_ctl |= DP_TP_CTL_ENHANCED_FRAME_ENABLE;
 	}
 	intel_de_write(dev_priv, dp_tp_ctl_reg(encoder, crtc_state), dp_tp_ctl);
@@ -3725,6 +3725,10 @@ static void intel_ddi_read_func_ctl(struct intel_encoder *encoder,
 		intel_cpu_transcoder_get_m2_n2(crtc, cpu_transcoder,
 					       &pipe_config->dp_m2_n2);
 
+		pipe_config->enhanced_framing =
+			intel_de_read(dev_priv, dp_tp_ctl_reg(encoder, pipe_config)) &
+			DP_TP_CTL_ENHANCED_FRAME_ENABLE;
+
 		if (DISPLAY_VER(dev_priv) >= 11)
 			pipe_config->fec_enable =
 				intel_de_read(dev_priv,
@@ -3741,6 +3745,9 @@ static void intel_ddi_read_func_ctl(struct intel_encoder *encoder,
 		if (!HAS_DP20(dev_priv)) {
 			/* FDI */
 			pipe_config->output_types |= BIT(INTEL_OUTPUT_ANALOG);
+			pipe_config->enhanced_framing =
+				intel_de_read(dev_priv, dp_tp_ctl_reg(encoder, pipe_config)) &
+				DP_TP_CTL_ENHANCED_FRAME_ENABLE;
 			break;
 		}
 		fallthrough; /* 128b/132b */
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 3d3483e6f836..b95eb031abf2 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5226,6 +5226,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 	PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
 	PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
 	PIPE_CONF_CHECK_BOOL(has_infoframe);
+	PIPE_CONF_CHECK_BOOL(enhanced_framing);
 	PIPE_CONF_CHECK_BOOL(fec_enable);
 
 	PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 96a3183675be..9ea96eb19ddd 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1338,6 +1338,8 @@ struct intel_crtc_state {
 	u16 linetime;
 	u16 ips_linetime;
 
+	bool enhanced_framing;
+
 	/* Forward Error correction State */
 	bool fec_enable;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index e92c62bcc9b8..47a212a84fec 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -659,7 +659,7 @@ intel_dp_update_link_bw_set(struct intel_dp *intel_dp,
 	/* Write the link configuration data */
 	link_config[0] = link_bw;
 	link_config[1] = crtc_state->lane_count;
-	if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
+	if (crtc_state->enhanced_framing)
 		link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
 	drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config, 2);
 
-- 
2.39.2

[Intel-gfx] [PATCH 09/11] drm/i915: Stop spamming the logs with PLL state

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

encoder->get_config() is not the place where the state
should be dumped. Get rid of the spam.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 51ae1aad7cc7..65e031ff740c 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3863,11 +3863,9 @@ static void mtl_ddi_get_config(struct intel_encoder *encoder,
 		crtc_state->port_clock = intel_mtl_tbt_calc_port_clock(encoder);
 	} else if (intel_is_c10phy(i915, phy)) {
 		intel_c10pll_readout_hw_state(encoder, &crtc_state->cx0pll_state.c10);
-		intel_c10pll_dump_hw_state(i915, &crtc_state->cx0pll_state.c10);
 		crtc_state->port_clock = intel_c10pll_calc_port_clock(encoder, &crtc_state->cx0pll_state.c10);
 	} else {
 		intel_c20pll_readout_hw_state(encoder, &crtc_state->cx0pll_state.c20);
-		intel_c20pll_dump_hw_state(i915, &crtc_state->cx0pll_state.c20);
 		crtc_state->port_clock = intel_c20pll_calc_port_clock(encoder, &crtc_state->cx0pll_state.c20);
 	}
 
-- 
2.39.2

[Intel-gfx] [PATCH 10/11] drm/i915: Drop some redundant eDP checks

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

There's no need to check for both eDP and fixed_mode when
deciding whether to do the pfit calculations or not.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 9ac199444155..6bc7ff0c4320 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1044,7 +1044,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
 		return MODE_H_ILLEGAL;
 
 	fixed_mode = intel_panel_fixed_mode(connector, mode);
-	if (intel_dp_is_edp(intel_dp) && fixed_mode) {
+	if (fixed_mode) {
 		status = intel_panel_mode_valid(connector, mode);
 		if (status != MODE_OK)
 			return status;
@@ -2175,7 +2175,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 		intel_audio_compute_config(encoder, pipe_config, conn_state);
 
 	fixed_mode = intel_panel_fixed_mode(connector, adjusted_mode);
-	if (intel_dp_is_edp(intel_dp) && fixed_mode) {
+	if (fixed_mode) {
 		ret = intel_panel_compute_config(connector, adjusted_mode);
 		if (ret)
 			return ret;
-- 
2.39.2

[Intel-gfx] [PATCH 11/11] drm/i915: Reduce combo PHY log spam

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We always check whether combo PHYs need to be re-initialized
after disabling DC states, which leads to log spam. Switch things
around so that we only log something when we actually have to
re-initialized a PHY.

The log spam was exacerbated by commit 41b4c7fe72b6 ("drm/i915:
Disable DC states for all commits") since we now disable DC
states far more often.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_combo_phy.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_combo_phy.c b/drivers/gpu/drm/i915/display/intel_combo_phy.c
index 922a6d87b553..ee843f2b1af1 100644
--- a/drivers/gpu/drm/i915/display/intel_combo_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_combo_phy.c
@@ -114,10 +114,6 @@ static bool icl_verify_procmon_ref_values(struct drm_i915_private *dev_priv,
 
 	procmon = icl_get_procmon_ref_values(dev_priv, phy);
 
-	drm_dbg_kms(&dev_priv->drm,
-		    "Combo PHY %c Voltage/Process Info : %s\n",
-		    phy_name(phy), procmon->name);
-
 	ret = check_phy_reg(dev_priv, phy, ICL_PORT_COMP_DW1(phy),
 			    (0xff << 16) | 0xff, procmon->dw1);
 	ret &= check_phy_reg(dev_priv, phy, ICL_PORT_COMP_DW9(phy),
@@ -312,14 +308,17 @@ static void icl_combo_phys_init(struct drm_i915_private *dev_priv)
 	enum phy phy;
 
 	for_each_combo_phy(dev_priv, phy) {
+		const struct icl_procmon *procmon;
 		u32 val;
 
-		if (icl_combo_phy_verify_state(dev_priv, phy)) {
-			drm_dbg(&dev_priv->drm,
-				"Combo PHY %c already enabled, won't reprogram it.\n",
-				phy_name(phy));
+		if (icl_combo_phy_verify_state(dev_priv, phy))
 			continue;
-		}
+
+		procmon = icl_get_procmon_ref_values(dev_priv, phy);
+
+		drm_dbg(&dev_priv->drm,
+			"Initializing combo PHY %c (Voltage/Process Info : %s)\n",
+			phy_name(phy), procmon->name);
 
 		if (!has_phy_misc(dev_priv, phy))
 			goto skip_phy_misc;
-- 
2.39.2

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: MST+DSC nukage and state stuff

[Patchwork]

== Series Details ==

Series: drm/i915: MST+DSC nukage and state stuff
URL   : https://patchwork.freedesktop.org/series/117201/
State : warning

== Summary ==

Error: dim checkpatch failed
b98a8e6872f9 drm/dp_mst: Fix fractional DSC bpp handling
81bfa5ca1aa8 drm/i915/mst: Remove broken MST DSC support
7287f3ff6156 drm/i915/mst: Read out FEC state
-:24: WARNING:LONG_LINE: line length of 106 exceeds 100 columns
#24: FILE: drivers/gpu/drm/i915/display/intel_ddi.c:3769:
+					      dp_tp_ctl_reg(encoder, pipe_config)) & DP_TP_CTL_FEC_ENABLE;

total: 0 errors, 1 warnings, 0 checks, 11 lines checked
8b161765a004 drm/i915: Fix FEC pipe A vs. DDI A mixup
020a66619ead drm/i915: Check lane count when determining FEC support
ce9fd402683b drm/i915: Fix FEC state dump
-:48: WARNING:LONG_LINE: line length of 106 exceeds 100 columns
#48: FILE: drivers/gpu/drm/i915/display/intel_ddi.c:3731:
+					      dp_tp_ctl_reg(encoder, pipe_config)) & DP_TP_CTL_FEC_ENABLE;

total: 0 errors, 1 warnings, 0 checks, 28 lines checked
a63fb1144f6f drm/i915: Split some long lines
e0509a0619cd drm/i915: Introduce crtc_state->enhanced_framing
5cbdfdb85cd6 drm/i915: Stop spamming the logs with PLL state
124fa6dba488 drm/i915: Drop some redundant eDP checks
5588b8335ec1 drm/i915: Reduce combo PHY log spam

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: MST+DSC nukage and state stuff

[Patchwork]

== Series Details ==

Series: drm/i915: MST+DSC nukage and state stuff
URL   : https://patchwork.freedesktop.org/series/117201/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: MST+DSC nukage and state stuff

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 7083 bytes --]

== Series Details ==

Series: drm/i915: MST+DSC nukage and state stuff
URL   : https://patchwork.freedesktop.org/series/117201/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13097 -> Patchwork_117201v1
====================================================

Summary
-------

  **FAILURE**

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

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

Participating hosts (38 -> 38)
------------------------------

  Additional (1): fi-kbl-soraka 
  Missing    (1): fi-snb-2520m 

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_chamelium_frames@hdmi-crc-fast:
    - fi-kbl-7567u:       [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13097/fi-kbl-7567u/igt@kms_chamelium_frames@hdmi-crc-fast.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v1/fi-kbl-7567u/igt@kms_chamelium_frames@hdmi-crc-fast.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#2190])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v1/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#4613]) +3 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v1/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html

  * igt@i915_selftest@live@gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][5] ([i915#1886] / [i915#7913])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v1/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@hangcheck:
    - bat-dg1-7:          [PASS][6] -> [ABORT][7] ([i915#4983])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13097/bat-dg1-7/igt@i915_selftest@live@hangcheck.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v1/bat-dg1-7/igt@i915_selftest@live@hangcheck.html

  * igt@kms_chamelium_frames@hdmi-crc-fast:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][8] ([fdo#109271]) +16 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v1/fi-kbl-soraka/igt@kms_chamelium_frames@hdmi-crc-fast.html

  
#### Possible fixes ####

  * igt@gem_exec_parallel@engines@fds:
    - {bat-mtlp-8}:       [ABORT][9] -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13097/bat-mtlp-8/igt@gem_exec_parallel@engines@fds.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v1/bat-mtlp-8/igt@gem_exec_parallel@engines@fds.html

  * igt@i915_selftest@live@requests:
    - {bat-mtlp-6}:       [ABORT][11] ([i915#4983] / [i915#7920]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13097/bat-mtlp-6/igt@i915_selftest@live@requests.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v1/bat-mtlp-6/igt@i915_selftest@live@requests.html

  * igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1:
    - bat-dg2-8:          [FAIL][13] ([i915#7932]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13097/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v1/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#3595]: https://gitlab.freedesktop.org/drm/intel/issues/3595
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5190]: https://gitlab.freedesktop.org/drm/intel/issues/5190
  [i915#5274]: https://gitlab.freedesktop.org/drm/intel/issues/5274
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
  [i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7920]: https://gitlab.freedesktop.org/drm/intel/issues/7920
  [i915#7932]: https://gitlab.freedesktop.org/drm/intel/issues/7932
  [i915#8368]: https://gitlab.freedesktop.org/drm/intel/issues/8368
  [i915#8379]: https://gitlab.freedesktop.org/drm/intel/issues/8379


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

  * Linux: CI_DRM_13097 -> Patchwork_117201v1

  CI-20190529: 20190529
  CI_DRM_13097: 1413856e3770380da743e274a06896acabf49e0e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7277: 1cb3507f3ff28d11bd5cfabcde576fe78ddab571 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_117201v1: 1413856e3770380da743e274a06896acabf49e0e @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

64144f54c478 drm/i915: Reduce combo PHY log spam
1497222bf809 drm/i915: Drop some redundant eDP checks
99b827268f93 drm/i915: Stop spamming the logs with PLL state
520376fc9123 drm/i915: Introduce crtc_state->enhanced_framing
3bda143ed335 drm/i915: Split some long lines
18164c0d9115 drm/i915: Fix FEC state dump
ce9df9e342e3 drm/i915: Check lane count when determining FEC support
d73abf197659 drm/i915: Fix FEC pipe A vs. DDI A mixup
26935dadddfb drm/i915/mst: Read out FEC state
231af8fdaf87 drm/i915/mst: Remove broken MST DSC support
5f53dcd0d3d4 drm/dp_mst: Fix fractional DSC bpp handling

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v1/index.html

[-- Attachment #2: Type: text/html, Size: 6834 bytes --]

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: MST+DSC nukage and state stuff (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: MST+DSC nukage and state stuff (rev2)
URL   : https://patchwork.freedesktop.org/series/117201/
State : warning

== Summary ==

Error: dim checkpatch failed
787df893c7bf drm/dp_mst: Fix fractional DSC bpp handling
48761d3260fa drm/i915/mst: Remove broken MST DSC support
73450cd02b6a drm/i915/mst: Read out FEC state
-:24: WARNING:LONG_LINE: line length of 106 exceeds 100 columns
#24: FILE: drivers/gpu/drm/i915/display/intel_ddi.c:3769:
+					      dp_tp_ctl_reg(encoder, pipe_config)) & DP_TP_CTL_FEC_ENABLE;

total: 0 errors, 1 warnings, 0 checks, 11 lines checked
50007bdbb762 drm/i915: Fix FEC pipe A vs. DDI A mixup
01ea92f6872e drm/i915: Check lane count when determining FEC support
ab38c2184256 drm/i915: Fix FEC state dump
-:48: WARNING:LONG_LINE: line length of 106 exceeds 100 columns
#48: FILE: drivers/gpu/drm/i915/display/intel_ddi.c:3731:
+					      dp_tp_ctl_reg(encoder, pipe_config)) & DP_TP_CTL_FEC_ENABLE;

total: 0 errors, 1 warnings, 0 checks, 28 lines checked
a3bd69f5f3f9 drm/i915: Split some long lines
9f30735de6f9 drm/i915: Introduce crtc_state->enhanced_framing
306c9b6c8c78 drm/i915: Stop spamming the logs with PLL state
d93bee0be9e3 drm/i915: Drop some redundant eDP checks
cb750ff5a04c drm/i915: Reduce combo PHY log spam

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: MST+DSC nukage and state stuff (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: MST+DSC nukage and state stuff (rev2)
URL   : https://patchwork.freedesktop.org/series/117201/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: MST+DSC nukage and state stuff (rev2)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 6994 bytes --]

== Series Details ==

Series: drm/i915: MST+DSC nukage and state stuff (rev2)
URL   : https://patchwork.freedesktop.org/series/117201/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13097 -> Patchwork_117201v2
====================================================

Summary
-------

  **FAILURE**

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

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

Participating hosts (38 -> 37)
------------------------------

  Missing    (1): fi-snb-2520m 

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_chamelium_frames@hdmi-crc-fast:
    - fi-kbl-7567u:       [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13097/fi-kbl-7567u/igt@kms_chamelium_frames@hdmi-crc-fast.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v2/fi-kbl-7567u/igt@kms_chamelium_frames@hdmi-crc-fast.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@hangcheck:
    - bat-dg2-11:         [PASS][3] -> [ABORT][4] ([i915#7913] / [i915#7979])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13097/bat-dg2-11/igt@i915_selftest@live@hangcheck.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v2/bat-dg2-11/igt@i915_selftest@live@hangcheck.html

  * igt@kms_pipe_crc_basic@nonblocking-crc@pipe-c-dp-1:
    - bat-dg2-8:          [PASS][5] -> [FAIL][6] ([i915#7932])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13097/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-c-dp-1.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v2/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-c-dp-1.html

  * igt@kms_pipe_crc_basic@read-crc:
    - bat-dg2-11:         NOTRUN -> [SKIP][7] ([i915#1845] / [i915#5354])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v2/bat-dg2-11/igt@kms_pipe_crc_basic@read-crc.html

  
#### Possible fixes ####

  * igt@gem_exec_parallel@engines@fds:
    - {bat-mtlp-8}:       [ABORT][8] -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13097/bat-mtlp-8/igt@gem_exec_parallel@engines@fds.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v2/bat-mtlp-8/igt@gem_exec_parallel@engines@fds.html

  * igt@i915_selftest@live@requests:
    - {bat-mtlp-6}:       [ABORT][10] ([i915#4983] / [i915#7920]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13097/bat-mtlp-6/igt@i915_selftest@live@requests.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v2/bat-mtlp-6/igt@i915_selftest@live@requests.html

  * igt@i915_selftest@live@slpc:
    - bat-rpls-1:         [DMESG-WARN][12] ([i915#6367]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13097/bat-rpls-1/igt@i915_selftest@live@slpc.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v2/bat-rpls-1/igt@i915_selftest@live@slpc.html

  * igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1:
    - bat-dg2-8:          [FAIL][14] ([i915#7932]) -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13097/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v2/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1.html

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

  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#3595]: https://gitlab.freedesktop.org/drm/intel/issues/3595
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5190]: https://gitlab.freedesktop.org/drm/intel/issues/5190
  [i915#5274]: https://gitlab.freedesktop.org/drm/intel/issues/5274
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
  [i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7920]: https://gitlab.freedesktop.org/drm/intel/issues/7920
  [i915#7932]: https://gitlab.freedesktop.org/drm/intel/issues/7932
  [i915#7979]: https://gitlab.freedesktop.org/drm/intel/issues/7979
  [i915#8368]: https://gitlab.freedesktop.org/drm/intel/issues/8368
  [i915#8379]: https://gitlab.freedesktop.org/drm/intel/issues/8379


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

  * Linux: CI_DRM_13097 -> Patchwork_117201v2

  CI-20190529: 20190529
  CI_DRM_13097: 1413856e3770380da743e274a06896acabf49e0e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7277: 1cb3507f3ff28d11bd5cfabcde576fe78ddab571 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_117201v2: 1413856e3770380da743e274a06896acabf49e0e @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

5f06d044ec06 drm/i915: Reduce combo PHY log spam
c98886f5cb23 drm/i915: Drop some redundant eDP checks
3a97fbcbb09a drm/i915: Stop spamming the logs with PLL state
76202a2de2d3 drm/i915: Introduce crtc_state->enhanced_framing
c54710ec08e4 drm/i915: Split some long lines
5061ba051ca1 drm/i915: Fix FEC state dump
28049d44b66f drm/i915: Check lane count when determining FEC support
763215feeb0b drm/i915: Fix FEC pipe A vs. DDI A mixup
e102b1ab3c7d drm/i915/mst: Read out FEC state
b3de6dbe728f drm/i915/mst: Remove broken MST DSC support
ac969de60ffd drm/dp_mst: Fix fractional DSC bpp handling

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v2/index.html

[-- Attachment #2: Type: text/html, Size: 6649 bytes --]

Re: [Intel-gfx] [PATCH 02/11] drm/i915/mst: Remove broken MST DSC support

[Lisovskiy, Stanislav]

On Tue, May 02, 2023 at 05:38:57PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The MST DSC code has a myriad of issues:
> - Platform checks are wrong (MST+DSC is TGL+ only IIRC)
> - Return values of .mode_valid_ctx() are wrong
> - .mode_valid_ctx() assumes bigjoiner might be used, but ther rest
>   of the code doesn't agree
> - compressed bpp calculations don't make sense
> - FEC handling needs to consider the entire link as opposed to just
>   the single stream. Currently FEC would only get enabled if the
>   first enabled stream is compressed. Also I'm not seeing anything
>   that would account for the FEC overhead in any bandwidth calculations
> - PPS SDP is only handled for the first stream via the dig_port
>   hooks, other streams will not be transmittitng any PPS SDPs
> - PPS SDP readout is missing (also missing for SST!)
> - VDSC readout is missing (also missing for SST!)
> 
> The FEC issues is really the big one since we have no way currently
> to apply such link wide configuration constraints. Changing that is
> going to require a much bigger rework of the higher level modeset
> .compute_config() logic. We will also need such a rework to properly
> distribute the available bandwidth across all the streams on the
> same link (which is a must to eg. enable deep color).

Hi Ville,

I'm not going to argue even that this code has some issues or not,
sure it has. However you are currently totally removing everything
without proposing what fixes have to be done or could have been done
instead.
We had multiple DP MST hubs which started to work because of this code,
also we had a public gitlab issues which were solved. So now they will loose
that support. I mean if this goes through, then I guess its up to you to 
handle this. Why not just fix instead of remove? Just add fixing patches on
top of those which remove the wrong code.
Now we are simply removing all that works at least,
instead of proposing how to fix, leaving no support at all. 
I have 2 MST hubs on my desk which won't work after this code is removed,
so if this patch goes through I recommend taking them.
But at least from my side, I think this is kinda unconstructive.

Stan


> 
> Cc: stable@vger.kernel.org
> Cc: Vinod Govindapillai <vinod.govindapillai@intel.com>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Fixes: d51f25eb479a ("drm/i915: Add DSC support to MST path")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 176 +-------------------
>  1 file changed, 5 insertions(+), 171 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 44c15d6faac4..d762f37fafb5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -72,8 +72,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
>  						int min_bpp,
>  						struct link_config_limits *limits,
>  						struct drm_connector_state *conn_state,
> -						int step,
> -						bool dsc)
> +						int step)
>  {
>  	struct drm_atomic_state *state = crtc_state->uapi.state;
>  	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
> @@ -104,7 +103,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
>  	for (bpp = max_bpp; bpp >= min_bpp; bpp -= step) {
>  		drm_dbg_kms(&i915->drm, "Trying bpp %d\n", bpp);
>  
> -		ret = intel_dp_mst_check_constraints(i915, bpp, adjusted_mode, crtc_state, dsc);
> +		ret = intel_dp_mst_check_constraints(i915, bpp, adjusted_mode, crtc_state, false);
>  		if (ret)
>  			continue;
>  
> @@ -136,11 +135,8 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
>  		drm_dbg_kms(&i915->drm, "failed finding vcpi slots:%d\n",
>  			    slots);
>  	} else {
> -		if (!dsc)
> -			crtc_state->pipe_bpp = bpp;
> -		else
> -			crtc_state->dsc.compressed_bpp = bpp;
> -		drm_dbg_kms(&i915->drm, "Got %d slots for pipe bpp %d dsc %d\n", slots, bpp, dsc);
> +		crtc_state->pipe_bpp = bpp;
> +		drm_dbg_kms(&i915->drm, "Got %d slots for pipe bpp %d\n", slots, bpp);
>  	}
>  
>  	return slots;
> @@ -157,7 +153,7 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
>  
>  	slots = intel_dp_mst_find_vcpi_slots_for_bpp(encoder, crtc_state, limits->max_bpp,
>  						     limits->min_bpp, limits,
> -						     conn_state, 2 * 3, false);
> +						     conn_state, 2 * 3);
>  
>  	if (slots < 0)
>  		return slots;
> @@ -173,99 +169,6 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
>  	return 0;
>  }
>  
> -static int intel_dp_dsc_mst_compute_link_config(struct intel_encoder *encoder,
> -						struct intel_crtc_state *crtc_state,
> -						struct drm_connector_state *conn_state,
> -						struct link_config_limits *limits)
> -{
> -	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
> -	struct intel_dp *intel_dp = &intel_mst->primary->dp;
> -	struct intel_connector *connector =
> -		to_intel_connector(conn_state->connector);
> -	struct drm_i915_private *i915 = to_i915(connector->base.dev);
> -	const struct drm_display_mode *adjusted_mode =
> -		&crtc_state->hw.adjusted_mode;
> -	int slots = -EINVAL;
> -	int i, num_bpc;
> -	u8 dsc_bpc[3] = {0};
> -	int min_bpp, max_bpp, sink_min_bpp, sink_max_bpp;
> -	u8 dsc_max_bpc;
> -	bool need_timeslot_recalc = false;
> -	u32 last_compressed_bpp;
> -
> -	/* Max DSC Input BPC for ICL is 10 and for TGL+ is 12 */
> -	if (DISPLAY_VER(i915) >= 12)
> -		dsc_max_bpc = min_t(u8, 12, conn_state->max_requested_bpc);
> -	else
> -		dsc_max_bpc = min_t(u8, 10, conn_state->max_requested_bpc);
> -
> -	max_bpp = min_t(u8, dsc_max_bpc * 3, limits->max_bpp);
> -	min_bpp = limits->min_bpp;
> -
> -	num_bpc = drm_dp_dsc_sink_supported_input_bpcs(intel_dp->dsc_dpcd,
> -						       dsc_bpc);
> -
> -	drm_dbg_kms(&i915->drm, "DSC Source supported min bpp %d max bpp %d\n",
> -		    min_bpp, max_bpp);
> -
> -	sink_max_bpp = dsc_bpc[0] * 3;
> -	sink_min_bpp = sink_max_bpp;
> -
> -	for (i = 1; i < num_bpc; i++) {
> -		if (sink_min_bpp > dsc_bpc[i] * 3)
> -			sink_min_bpp = dsc_bpc[i] * 3;
> -		if (sink_max_bpp < dsc_bpc[i] * 3)
> -			sink_max_bpp = dsc_bpc[i] * 3;
> -	}
> -
> -	drm_dbg_kms(&i915->drm, "DSC Sink supported min bpp %d max bpp %d\n",
> -		    sink_min_bpp, sink_max_bpp);
> -
> -	if (min_bpp < sink_min_bpp)
> -		min_bpp = sink_min_bpp;
> -
> -	if (max_bpp > sink_max_bpp)
> -		max_bpp = sink_max_bpp;
> -
> -	slots = intel_dp_mst_find_vcpi_slots_for_bpp(encoder, crtc_state, max_bpp,
> -						     min_bpp, limits,
> -						     conn_state, 2 * 3, true);
> -
> -	if (slots < 0)
> -		return slots;
> -
> -	last_compressed_bpp = crtc_state->dsc.compressed_bpp;
> -
> -	crtc_state->dsc.compressed_bpp = intel_dp_dsc_nearest_valid_bpp(i915,
> -									last_compressed_bpp,
> -									crtc_state->pipe_bpp);
> -
> -	if (crtc_state->dsc.compressed_bpp != last_compressed_bpp)
> -		need_timeslot_recalc = true;
> -
> -	/*
> -	 * Apparently some MST hubs dislike if vcpi slots are not matching precisely
> -	 * the actual compressed bpp we use.
> -	 */
> -	if (need_timeslot_recalc) {
> -		slots = intel_dp_mst_find_vcpi_slots_for_bpp(encoder, crtc_state,
> -							     crtc_state->dsc.compressed_bpp,
> -							     crtc_state->dsc.compressed_bpp,
> -							     limits, conn_state, 2 * 3, true);
> -		if (slots < 0)
> -			return slots;
> -	}
> -
> -	intel_link_compute_m_n(crtc_state->dsc.compressed_bpp,
> -			       crtc_state->lane_count,
> -			       adjusted_mode->crtc_clock,
> -			       crtc_state->port_clock,
> -			       &crtc_state->dp_m_n,
> -			       crtc_state->fec_enable);
> -	crtc_state->dp_m_n.tu = slots;
> -
> -	return 0;
> -}
>  static int intel_dp_mst_update_slots(struct intel_encoder *encoder,
>  				     struct intel_crtc_state *crtc_state,
>  				     struct drm_connector_state *conn_state)
> @@ -349,29 +252,6 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  
>  	ret = intel_dp_mst_compute_link_config(encoder, pipe_config,
>  					       conn_state, &limits);
> -
> -	if (ret == -EDEADLK)
> -		return ret;
> -
> -	/* enable compression if the mode doesn't fit available BW */
> -	drm_dbg_kms(&dev_priv->drm, "Force DSC en = %d\n", intel_dp->force_dsc_en);
> -	if (ret || intel_dp->force_dsc_en) {
> -		/*
> -		 * Try to get at least some timeslots and then see, if
> -		 * we can fit there with DSC.
> -		 */
> -		drm_dbg_kms(&dev_priv->drm, "Trying to find VCPI slots in DSC mode\n");
> -
> -		ret = intel_dp_dsc_mst_compute_link_config(encoder, pipe_config,
> -							   conn_state, &limits);
> -		if (ret < 0)
> -			return ret;
> -
> -		ret = intel_dp_dsc_compute_config(intel_dp, pipe_config,
> -						  conn_state, &limits,
> -						  pipe_config->dp_m_n.tu, false);
> -	}
> -
>  	if (ret)
>  		return ret;
>  
> @@ -909,10 +789,6 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
>  	int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
>  	int max_rate, mode_rate, max_lanes, max_link_clock;
>  	int ret;
> -	bool dsc = false, bigjoiner = false;
> -	u16 dsc_max_output_bpp = 0;
> -	u8 dsc_slice_count = 0;
> -	int target_clock = mode->clock;
>  
>  	if (drm_connector_is_unregistered(connector)) {
>  		*status = MODE_ERROR;
> @@ -950,48 +826,6 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
>  		return 0;
>  	}
>  
> -	if (intel_dp_need_bigjoiner(intel_dp, mode->hdisplay, target_clock)) {
> -		bigjoiner = true;
> -		max_dotclk *= 2;
> -	}
> -
> -	if (DISPLAY_VER(dev_priv) >= 10 &&
> -	    drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)) {
> -		/*
> -		 * TBD pass the connector BPC,
> -		 * for now U8_MAX so that max BPC on that platform would be picked
> -		 */
> -		int pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, U8_MAX);
> -
> -		if (drm_dp_sink_supports_fec(intel_dp->fec_capable)) {
> -			dsc_max_output_bpp =
> -				intel_dp_dsc_get_output_bpp(dev_priv,
> -							    max_link_clock,
> -							    max_lanes,
> -							    target_clock,
> -							    mode->hdisplay,
> -							    bigjoiner,
> -							    pipe_bpp, 64) >> 4;
> -			dsc_slice_count =
> -				intel_dp_dsc_get_slice_count(intel_dp,
> -							     target_clock,
> -							     mode->hdisplay,
> -							     bigjoiner);
> -		}
> -
> -		dsc = dsc_max_output_bpp && dsc_slice_count;
> -	}
> -
> -	/*
> -	 * Big joiner configuration needs DSC for TGL which is not true for
> -	 * XE_LPD where uncompressed joiner is supported.
> -	 */
> -	if (DISPLAY_VER(dev_priv) < 13 && bigjoiner && !dsc)
> -		return MODE_CLOCK_HIGH;
> -
> -	if (mode_rate > max_rate && !dsc)
> -		return MODE_CLOCK_HIGH;
> -
>  	*status = intel_mode_valid_max_plane_size(dev_priv, mode, false);
>  	return 0;
>  }
> -- 
> 2.39.2
> 

Re: [Intel-gfx] [PATCH 02/11] drm/i915/mst: Remove broken MST DSC support

[Lisovskiy, Stanislav]

On Tue, May 02, 2023 at 05:38:57PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The MST DSC code has a myriad of issues:
> - Platform checks are wrong (MST+DSC is TGL+ only IIRC)
> - Return values of .mode_valid_ctx() are wrong
> - .mode_valid_ctx() assumes bigjoiner might be used, but ther rest
>   of the code doesn't agree
> - compressed bpp calculations don't make sense
> - FEC handling needs to consider the entire link as opposed to just
>   the single stream. Currently FEC would only get enabled if the
>   first enabled stream is compressed. Also I'm not seeing anything
>   that would account for the FEC overhead in any bandwidth calculations
> - PPS SDP is only handled for the first stream via the dig_port
>   hooks, other streams will not be transmittitng any PPS SDPs
> - PPS SDP readout is missing (also missing for SST!)
> - VDSC readout is missing (also missing for SST!)
> 
> The FEC issues is really the big one since we have no way currently
> to apply such link wide configuration constraints. Changing that is
> going to require a much bigger rework of the higher level modeset
> .compute_config() logic. We will also need such a rework to properly
> distribute the available bandwidth across all the streams on the
> same link (which is a must to eg. enable deep color).

Also all the things you mentioned are subject for discussion, for example
I see that FEC overhead is actually accounted for bpp calculation for instance.
We usually improve things by gradually fixing, because if we act same way towards
all wrong code in the driver, we could end up removing the whole i915.
So from my side I would nack it, at least until you have a code which handles
all of this better - I have no doubt you probably have some ideas in your mind, so lets be constructive at least and propose something better first.
This code doesn't cause any regressions, but still provides "some" support to DP MST DSC to say the least and even if that would be removed, if some of those users 
refer to me, I would probably then just point to this mail discussion everytime.

Stan


> 
> Cc: stable@vger.kernel.org
> Cc: Vinod Govindapillai <vinod.govindapillai@intel.com>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Fixes: d51f25eb479a ("drm/i915: Add DSC support to MST path")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 176 +-------------------
>  1 file changed, 5 insertions(+), 171 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 44c15d6faac4..d762f37fafb5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -72,8 +72,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
>  						int min_bpp,
>  						struct link_config_limits *limits,
>  						struct drm_connector_state *conn_state,
> -						int step,
> -						bool dsc)
> +						int step)
>  {
>  	struct drm_atomic_state *state = crtc_state->uapi.state;
>  	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
> @@ -104,7 +103,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
>  	for (bpp = max_bpp; bpp >= min_bpp; bpp -= step) {
>  		drm_dbg_kms(&i915->drm, "Trying bpp %d\n", bpp);
>  
> -		ret = intel_dp_mst_check_constraints(i915, bpp, adjusted_mode, crtc_state, dsc);
> +		ret = intel_dp_mst_check_constraints(i915, bpp, adjusted_mode, crtc_state, false);
>  		if (ret)
>  			continue;
>  
> @@ -136,11 +135,8 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
>  		drm_dbg_kms(&i915->drm, "failed finding vcpi slots:%d\n",
>  			    slots);
>  	} else {
> -		if (!dsc)
> -			crtc_state->pipe_bpp = bpp;
> -		else
> -			crtc_state->dsc.compressed_bpp = bpp;
> -		drm_dbg_kms(&i915->drm, "Got %d slots for pipe bpp %d dsc %d\n", slots, bpp, dsc);
> +		crtc_state->pipe_bpp = bpp;
> +		drm_dbg_kms(&i915->drm, "Got %d slots for pipe bpp %d\n", slots, bpp);
>  	}
>  
>  	return slots;
> @@ -157,7 +153,7 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
>  
>  	slots = intel_dp_mst_find_vcpi_slots_for_bpp(encoder, crtc_state, limits->max_bpp,
>  						     limits->min_bpp, limits,
> -						     conn_state, 2 * 3, false);
> +						     conn_state, 2 * 3);
>  
>  	if (slots < 0)
>  		return slots;
> @@ -173,99 +169,6 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
>  	return 0;
>  }
>  
> -static int intel_dp_dsc_mst_compute_link_config(struct intel_encoder *encoder,
> -						struct intel_crtc_state *crtc_state,
> -						struct drm_connector_state *conn_state,
> -						struct link_config_limits *limits)
> -{
> -	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
> -	struct intel_dp *intel_dp = &intel_mst->primary->dp;
> -	struct intel_connector *connector =
> -		to_intel_connector(conn_state->connector);
> -	struct drm_i915_private *i915 = to_i915(connector->base.dev);
> -	const struct drm_display_mode *adjusted_mode =
> -		&crtc_state->hw.adjusted_mode;
> -	int slots = -EINVAL;
> -	int i, num_bpc;
> -	u8 dsc_bpc[3] = {0};
> -	int min_bpp, max_bpp, sink_min_bpp, sink_max_bpp;
> -	u8 dsc_max_bpc;
> -	bool need_timeslot_recalc = false;
> -	u32 last_compressed_bpp;
> -
> -	/* Max DSC Input BPC for ICL is 10 and for TGL+ is 12 */
> -	if (DISPLAY_VER(i915) >= 12)
> -		dsc_max_bpc = min_t(u8, 12, conn_state->max_requested_bpc);
> -	else
> -		dsc_max_bpc = min_t(u8, 10, conn_state->max_requested_bpc);
> -
> -	max_bpp = min_t(u8, dsc_max_bpc * 3, limits->max_bpp);
> -	min_bpp = limits->min_bpp;
> -
> -	num_bpc = drm_dp_dsc_sink_supported_input_bpcs(intel_dp->dsc_dpcd,
> -						       dsc_bpc);
> -
> -	drm_dbg_kms(&i915->drm, "DSC Source supported min bpp %d max bpp %d\n",
> -		    min_bpp, max_bpp);
> -
> -	sink_max_bpp = dsc_bpc[0] * 3;
> -	sink_min_bpp = sink_max_bpp;
> -
> -	for (i = 1; i < num_bpc; i++) {
> -		if (sink_min_bpp > dsc_bpc[i] * 3)
> -			sink_min_bpp = dsc_bpc[i] * 3;
> -		if (sink_max_bpp < dsc_bpc[i] * 3)
> -			sink_max_bpp = dsc_bpc[i] * 3;
> -	}
> -
> -	drm_dbg_kms(&i915->drm, "DSC Sink supported min bpp %d max bpp %d\n",
> -		    sink_min_bpp, sink_max_bpp);
> -
> -	if (min_bpp < sink_min_bpp)
> -		min_bpp = sink_min_bpp;
> -
> -	if (max_bpp > sink_max_bpp)
> -		max_bpp = sink_max_bpp;
> -
> -	slots = intel_dp_mst_find_vcpi_slots_for_bpp(encoder, crtc_state, max_bpp,
> -						     min_bpp, limits,
> -						     conn_state, 2 * 3, true);
> -
> -	if (slots < 0)
> -		return slots;
> -
> -	last_compressed_bpp = crtc_state->dsc.compressed_bpp;
> -
> -	crtc_state->dsc.compressed_bpp = intel_dp_dsc_nearest_valid_bpp(i915,
> -									last_compressed_bpp,
> -									crtc_state->pipe_bpp);
> -
> -	if (crtc_state->dsc.compressed_bpp != last_compressed_bpp)
> -		need_timeslot_recalc = true;
> -
> -	/*
> -	 * Apparently some MST hubs dislike if vcpi slots are not matching precisely
> -	 * the actual compressed bpp we use.
> -	 */
> -	if (need_timeslot_recalc) {
> -		slots = intel_dp_mst_find_vcpi_slots_for_bpp(encoder, crtc_state,
> -							     crtc_state->dsc.compressed_bpp,
> -							     crtc_state->dsc.compressed_bpp,
> -							     limits, conn_state, 2 * 3, true);
> -		if (slots < 0)
> -			return slots;
> -	}
> -
> -	intel_link_compute_m_n(crtc_state->dsc.compressed_bpp,
> -			       crtc_state->lane_count,
> -			       adjusted_mode->crtc_clock,
> -			       crtc_state->port_clock,
> -			       &crtc_state->dp_m_n,
> -			       crtc_state->fec_enable);
> -	crtc_state->dp_m_n.tu = slots;
> -
> -	return 0;
> -}
>  static int intel_dp_mst_update_slots(struct intel_encoder *encoder,
>  				     struct intel_crtc_state *crtc_state,
>  				     struct drm_connector_state *conn_state)
> @@ -349,29 +252,6 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  
>  	ret = intel_dp_mst_compute_link_config(encoder, pipe_config,
>  					       conn_state, &limits);
> -
> -	if (ret == -EDEADLK)
> -		return ret;
> -
> -	/* enable compression if the mode doesn't fit available BW */
> -	drm_dbg_kms(&dev_priv->drm, "Force DSC en = %d\n", intel_dp->force_dsc_en);
> -	if (ret || intel_dp->force_dsc_en) {
> -		/*
> -		 * Try to get at least some timeslots and then see, if
> -		 * we can fit there with DSC.
> -		 */
> -		drm_dbg_kms(&dev_priv->drm, "Trying to find VCPI slots in DSC mode\n");
> -
> -		ret = intel_dp_dsc_mst_compute_link_config(encoder, pipe_config,
> -							   conn_state, &limits);
> -		if (ret < 0)
> -			return ret;
> -
> -		ret = intel_dp_dsc_compute_config(intel_dp, pipe_config,
> -						  conn_state, &limits,
> -						  pipe_config->dp_m_n.tu, false);
> -	}
> -
>  	if (ret)
>  		return ret;
>  
> @@ -909,10 +789,6 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
>  	int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
>  	int max_rate, mode_rate, max_lanes, max_link_clock;
>  	int ret;
> -	bool dsc = false, bigjoiner = false;
> -	u16 dsc_max_output_bpp = 0;
> -	u8 dsc_slice_count = 0;
> -	int target_clock = mode->clock;
>  
>  	if (drm_connector_is_unregistered(connector)) {
>  		*status = MODE_ERROR;
> @@ -950,48 +826,6 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
>  		return 0;
>  	}
>  
> -	if (intel_dp_need_bigjoiner(intel_dp, mode->hdisplay, target_clock)) {
> -		bigjoiner = true;
> -		max_dotclk *= 2;
> -	}
> -
> -	if (DISPLAY_VER(dev_priv) >= 10 &&
> -	    drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)) {
> -		/*
> -		 * TBD pass the connector BPC,
> -		 * for now U8_MAX so that max BPC on that platform would be picked
> -		 */
> -		int pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, U8_MAX);
> -
> -		if (drm_dp_sink_supports_fec(intel_dp->fec_capable)) {
> -			dsc_max_output_bpp =
> -				intel_dp_dsc_get_output_bpp(dev_priv,
> -							    max_link_clock,
> -							    max_lanes,
> -							    target_clock,
> -							    mode->hdisplay,
> -							    bigjoiner,
> -							    pipe_bpp, 64) >> 4;
> -			dsc_slice_count =
> -				intel_dp_dsc_get_slice_count(intel_dp,
> -							     target_clock,
> -							     mode->hdisplay,
> -							     bigjoiner);
> -		}
> -
> -		dsc = dsc_max_output_bpp && dsc_slice_count;
> -	}
> -
> -	/*
> -	 * Big joiner configuration needs DSC for TGL which is not true for
> -	 * XE_LPD where uncompressed joiner is supported.
> -	 */
> -	if (DISPLAY_VER(dev_priv) < 13 && bigjoiner && !dsc)
> -		return MODE_CLOCK_HIGH;
> -
> -	if (mode_rate > max_rate && !dsc)
> -		return MODE_CLOCK_HIGH;
> -
>  	*status = intel_mode_valid_max_plane_size(dev_priv, mode, false);
>  	return 0;
>  }
> -- 
> 2.39.2
> 

Re: [Intel-gfx] [PATCH 02/11] drm/i915/mst: Remove broken MST DSC support

[Ville Syrjälä]

On Wed, May 03, 2023 at 10:36:42AM +0300, Lisovskiy, Stanislav wrote:
> On Tue, May 02, 2023 at 05:38:57PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The MST DSC code has a myriad of issues:
> > - Platform checks are wrong (MST+DSC is TGL+ only IIRC)
> > - Return values of .mode_valid_ctx() are wrong
> > - .mode_valid_ctx() assumes bigjoiner might be used, but ther rest
> >   of the code doesn't agree
> > - compressed bpp calculations don't make sense
> > - FEC handling needs to consider the entire link as opposed to just
> >   the single stream. Currently FEC would only get enabled if the
> >   first enabled stream is compressed. Also I'm not seeing anything
> >   that would account for the FEC overhead in any bandwidth calculations
> > - PPS SDP is only handled for the first stream via the dig_port
> >   hooks, other streams will not be transmittitng any PPS SDPs
> > - PPS SDP readout is missing (also missing for SST!)
> > - VDSC readout is missing (also missing for SST!)
> > 
> > The FEC issues is really the big one since we have no way currently
> > to apply such link wide configuration constraints. Changing that is
> > going to require a much bigger rework of the higher level modeset
> > .compute_config() logic. We will also need such a rework to properly
> > distribute the available bandwidth across all the streams on the
> > same link (which is a must to eg. enable deep color).
> 
> Also all the things you mentioned are subject for discussion, for example
> I see that FEC overhead is actually accounted for bpp calculation for instance.

AFAICS FEC is only accounted for in the data M/N calculations,
assuming that particular stream happened to be compressed. I'm
not sure if that actually matters since at least the link M/N
are not even used by the MST sink. I suppose the data M/N might
still be used for something though. For any uncompressed stream
on the same link the data M/N values will be calculated
incorrectly without FEC.

And as mentioned, the FEC bandwidth overhead doesn't seem to
be accounted anywhere so no guarantee that we won't try to
oversubcribe the link.

And FEC will only be enabled if the first stream to be enabled
is compressed, otherwise we will enable the link without FEC
and still try to cram other compressed streams through it
(albeit without the PPS SDP so who knows what will happen)
and that is illegal.

> We usually improve things by gradually fixing, because if we act same way towards
> all wrong code in the driver, we could end up removing the whole i915.

We ususally don't merge code that has this many obvious and/or
fundemental issues.

Now, most of the issues I listed above are probably fixable
in a way that could be backported to stable kernels, but
unfortunately the FEC issue is not one of those. That one
will likely need massive amounts of work all over the driver
modeset code, making a backport impossible.

> So from my side I would nack it, at least until you have a code which handles
> all of this better - I have no doubt you probably have some ideas in your mind, so lets be constructive at least and propose something better first.
> This code doesn't cause any regressions, but still provides "some" support to DP MST DSC to say the least and even if that would be removed, if some of those users 
> refer to me, I would probably then just point to this mail discussion everytime.

It seems very likely that it will cause regressions at some point,
it just needs a specific multi-display MST setup. The resulting
problems will be very confusing to debug since the order in which
you enable/disable the outputs will have an impact on what actually
goes wrong on account of the FEC and PPS SDP issues. The longer
we wait disabling this the harder it will be to deal with those
regressions since we the probably can't revert anymore (a straight
revert was already not possible) but also can't fix it in a way
that can be backported (due to the FEC issues in particular).

-- 
Ville Syrjälä
Intel

[Intel-gfx] [PATCH v2 08/11] drm/i915: Introduce crtc_state->enhanced_framing

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Track DP enhanced framing properly in the crtc state instead
of relying just on the cached DPCD everywhere, and hook it
up into the state check and dump.

v2: Actually set enhanced_framing in .compute_config()

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/g4x_dp.c                 | 10 ++++++++--
 drivers/gpu/drm/i915/display/intel_crt.c              |  2 ++
 drivers/gpu/drm/i915/display/intel_crtc_state_dump.c  |  5 +++--
 drivers/gpu/drm/i915/display/intel_ddi.c              | 11 +++++++++--
 drivers/gpu/drm/i915/display/intel_display.c          |  1 +
 drivers/gpu/drm/i915/display/intel_display_types.h    |  2 ++
 drivers/gpu/drm/i915/display/intel_dp.c               |  3 +++
 drivers/gpu/drm/i915/display/intel_dp_link_training.c |  2 +-
 8 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c
index 920d570f7594..534546ea7d0b 100644
--- a/drivers/gpu/drm/i915/display/g4x_dp.c
+++ b/drivers/gpu/drm/i915/display/g4x_dp.c
@@ -141,7 +141,7 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
 
 		intel_de_rmw(dev_priv, TRANS_DP_CTL(crtc->pipe),
 			     TRANS_DP_ENH_FRAMING,
-			     drm_dp_enhanced_frame_cap(intel_dp->dpcd) ?
+			     pipe_config->enhanced_framing ?
 			     TRANS_DP_ENH_FRAMING : 0);
 	} else {
 		if (IS_G4X(dev_priv) && pipe_config->limited_color_range)
@@ -153,7 +153,7 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
 			intel_dp->DP |= DP_SYNC_VS_HIGH;
 		intel_dp->DP |= DP_LINK_TRAIN_OFF;
 
-		if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
+		if (pipe_config->enhanced_framing)
 			intel_dp->DP |= DP_ENHANCED_FRAMING;
 
 		if (IS_CHERRYVIEW(dev_priv))
@@ -351,6 +351,9 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 		u32 trans_dp = intel_de_read(dev_priv,
 					     TRANS_DP_CTL(crtc->pipe));
 
+		if (trans_dp & TRANS_DP_ENH_FRAMING)
+			pipe_config->enhanced_framing = true;
+
 		if (trans_dp & TRANS_DP_HSYNC_ACTIVE_HIGH)
 			flags |= DRM_MODE_FLAG_PHSYNC;
 		else
@@ -361,6 +364,9 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 		else
 			flags |= DRM_MODE_FLAG_NVSYNC;
 	} else {
+		if (tmp & DP_ENHANCED_FRAMING)
+			pipe_config->enhanced_framing = true;
+
 		if (tmp & DP_SYNC_HS_HIGH)
 			flags |= DRM_MODE_FLAG_PHSYNC;
 		else
diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
index 13519f78cf9f..52af64aa9953 100644
--- a/drivers/gpu/drm/i915/display/intel_crt.c
+++ b/drivers/gpu/drm/i915/display/intel_crt.c
@@ -449,6 +449,8 @@ static int hsw_crt_compute_config(struct intel_encoder *encoder,
 	/* FDI must always be 2.7 GHz */
 	pipe_config->port_clock = 135000 * 2;
 
+	pipe_config->enhanced_framing = true;
+
 	adjusted_mode->crtc_clock = lpt_iclkip(pipe_config);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
index 91242ffe0768..14db2b481ff1 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
@@ -257,8 +257,9 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
 		intel_dump_m_n_config(pipe_config, "dp m2_n2",
 				      pipe_config->lane_count,
 				      &pipe_config->dp_m2_n2);
-		drm_dbg_kms(&i915->drm, "fec: %s\n",
-			    str_enabled_disabled(pipe_config->fec_enable));
+		drm_dbg_kms(&i915->drm, "fec: %s, enhanced framing: %s\n",
+			    str_enabled_disabled(pipe_config->fec_enable),
+			    str_enabled_disabled(pipe_config->enhanced_framing));
 	}
 
 	drm_dbg_kms(&i915->drm, "framestart delay: %d, MSA timing delay: %d\n",
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 4246133950fd..51ae1aad7cc7 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3434,7 +3434,7 @@ static void mtl_ddi_prepare_link_retrain(struct intel_dp *intel_dp,
 		dp_tp_ctl |= DP_TP_CTL_MODE_MST;
 	} else {
 		dp_tp_ctl |= DP_TP_CTL_MODE_SST;
-		if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
+		if (crtc_state->enhanced_framing)
 			dp_tp_ctl |= DP_TP_CTL_ENHANCED_FRAME_ENABLE;
 	}
 	intel_de_write(dev_priv, dp_tp_ctl_reg(encoder, crtc_state), dp_tp_ctl);
@@ -3491,7 +3491,7 @@ static void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp,
 		dp_tp_ctl |= DP_TP_CTL_MODE_MST;
 	} else {
 		dp_tp_ctl |= DP_TP_CTL_MODE_SST;
-		if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
+		if (crtc_state->enhanced_framing)
 			dp_tp_ctl |= DP_TP_CTL_ENHANCED_FRAME_ENABLE;
 	}
 	intel_de_write(dev_priv, dp_tp_ctl_reg(encoder, crtc_state), dp_tp_ctl);
@@ -3725,6 +3725,10 @@ static void intel_ddi_read_func_ctl(struct intel_encoder *encoder,
 		intel_cpu_transcoder_get_m2_n2(crtc, cpu_transcoder,
 					       &pipe_config->dp_m2_n2);
 
+		pipe_config->enhanced_framing =
+			intel_de_read(dev_priv, dp_tp_ctl_reg(encoder, pipe_config)) &
+			DP_TP_CTL_ENHANCED_FRAME_ENABLE;
+
 		if (DISPLAY_VER(dev_priv) >= 11)
 			pipe_config->fec_enable =
 				intel_de_read(dev_priv,
@@ -3741,6 +3745,9 @@ static void intel_ddi_read_func_ctl(struct intel_encoder *encoder,
 		if (!HAS_DP20(dev_priv)) {
 			/* FDI */
 			pipe_config->output_types |= BIT(INTEL_OUTPUT_ANALOG);
+			pipe_config->enhanced_framing =
+				intel_de_read(dev_priv, dp_tp_ctl_reg(encoder, pipe_config)) &
+				DP_TP_CTL_ENHANCED_FRAME_ENABLE;
 			break;
 		}
 		fallthrough; /* 128b/132b */
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 3d3483e6f836..b95eb031abf2 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5226,6 +5226,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 	PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
 	PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
 	PIPE_CONF_CHECK_BOOL(has_infoframe);
+	PIPE_CONF_CHECK_BOOL(enhanced_framing);
 	PIPE_CONF_CHECK_BOOL(fec_enable);
 
 	PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 96a3183675be..9ea96eb19ddd 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1338,6 +1338,8 @@ struct intel_crtc_state {
 	u16 linetime;
 	u16 ips_linetime;
 
+	bool enhanced_framing;
+
 	/* Forward Error correction State */
 	bool fec_enable;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 9ac199444155..9b44ba99fd1d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2214,6 +2214,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	pipe_config->limited_color_range =
 		intel_dp_limited_color_range(pipe_config, conn_state);
 
+	pipe_config->enhanced_framing =
+		drm_dp_enhanced_frame_cap(intel_dp->dpcd);
+
 	if (pipe_config->dsc.compression_enable)
 		output_bpp = pipe_config->dsc.compressed_bpp;
 	else
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index e92c62bcc9b8..47a212a84fec 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -659,7 +659,7 @@ intel_dp_update_link_bw_set(struct intel_dp *intel_dp,
 	/* Write the link configuration data */
 	link_config[0] = link_bw;
 	link_config[1] = crtc_state->lane_count;
-	if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
+	if (crtc_state->enhanced_framing)
 		link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
 	drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config, 2);
 
-- 
2.39.2

Re: [Intel-gfx] [PATCH 02/11] drm/i915/mst: Remove broken MST DSC support

[Lisovskiy, Stanislav]

On Wed, May 03, 2023 at 02:07:04PM +0300, Ville Syrjälä wrote:
> On Wed, May 03, 2023 at 10:36:42AM +0300, Lisovskiy, Stanislav wrote:
> > On Tue, May 02, 2023 at 05:38:57PM +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > The MST DSC code has a myriad of issues:
> > > - Platform checks are wrong (MST+DSC is TGL+ only IIRC)
> > > - Return values of .mode_valid_ctx() are wrong
> > > - .mode_valid_ctx() assumes bigjoiner might be used, but ther rest
> > >   of the code doesn't agree
> > > - compressed bpp calculations don't make sense
> > > - FEC handling needs to consider the entire link as opposed to just
> > >   the single stream. Currently FEC would only get enabled if the
> > >   first enabled stream is compressed. Also I'm not seeing anything
> > >   that would account for the FEC overhead in any bandwidth calculations
> > > - PPS SDP is only handled for the first stream via the dig_port
> > >   hooks, other streams will not be transmittitng any PPS SDPs
> > > - PPS SDP readout is missing (also missing for SST!)
> > > - VDSC readout is missing (also missing for SST!)
> > > 
> > > The FEC issues is really the big one since we have no way currently
> > > to apply such link wide configuration constraints. Changing that is
> > > going to require a much bigger rework of the higher level modeset
> > > .compute_config() logic. We will also need such a rework to properly
> > > distribute the available bandwidth across all the streams on the
> > > same link (which is a must to eg. enable deep color).
> > 
> > Also all the things you mentioned are subject for discussion, for example
> > I see that FEC overhead is actually accounted for bpp calculation for instance.
> 
> AFAICS FEC is only accounted for in the data M/N calculations,
> assuming that particular stream happened to be compressed. I'm
> not sure if that actually matters since at least the link M/N
> are not even used by the MST sink. I suppose the data M/N might
> still be used for something though. For any uncompressed stream
> on the same link the data M/N values will be calculated
> incorrectly without FEC.
> 
> And as mentioned, the FEC bandwidth overhead doesn't seem to
> be accounted anywhere so no guarantee that we won't try to
> oversubcribe the link.
> 
> And FEC will only be enabled if the first stream to be enabled
> is compressed, otherwise we will enable the link without FEC
> and still try to cram other compressed streams through it
> (albeit without the PPS SDP so who knows what will happen)
> and that is illegal.
> 
> > We usually improve things by gradually fixing, because if we act same way towards
> > all wrong code in the driver, we could end up removing the whole i915.
> 
> We ususally don't merge code that has this many obvious and/or
> fundemental issues.

Well, this is arguable and subjective judgement. Fact is that, so far we had more MST hubs
working with that code than without. Also no regressions or anything like that.
Moreover we usually merge code after code review, in particular those patches
did spend lots of time in review, where you could comment also.

Regarding merging code with fundamental issues, just recently you had admitted yourself
that bigjoiner issue for instance, we had recently, was partly caused by your code, because
we don't anymore copy the pll state to slave crtc. 
I would say that words like "obvious" and "fundamental"
issues can be applied to many things, however I thought that we always fix things in constructive,
but not destructive/negative way. 
Should I call also all code completely broken and remove it, once we discover some flaws 
there? Oh, we had many regressions, where I could say the same.

And once again I'm completely okay, if you did introduce better functionality instead
AND I know you have some valid points there, but now we are just removing everything completely,
without providing anything better.

But okay, I've mentioned what I think about this and from side this is nak. 
And once the guys to whom those patches helped will pop up from gitlab,
asking why their MST hubs stopped working - I will just refer them here.

> 
> Now, most of the issues I listed above are probably fixable
> in a way that could be backported to stable kernels, but
> unfortunately the FEC issue is not one of those. That one
> will likely need massive amounts of work all over the driver
> modeset code, making a backport impossible.
> 
> > So from my side I would nack it, at least until you have a code which handles
> > all of this better - I have no doubt you probably have some ideas in your mind, so lets be constructive at least and propose something better first.
> > This code doesn't cause any regressions, but still provides "some" support to DP MST DSC to say the least and even if that would be removed, if some of those users 
> > refer to me, I would probably then just point to this mail discussion everytime.
> 
> It seems very likely that it will cause regressions at some point,
> it just needs a specific multi-display MST setup. The resulting
> problems will be very confusing to debug since the order in which
> you enable/disable the outputs will have an impact on what actually
> goes wrong on account of the FEC and PPS SDP issues. The longer
> we wait disabling this the harder it will be to deal with those
> regressions since we the probably can't revert anymore (a straight
> revert was already not possible) but also can't fix it in a way
> that can be backported (due to the FEC issues in particular).
> 
> -- 
> Ville Syrjälä
> Intel

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: MST+DSC nukage and state stuff (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: MST+DSC nukage and state stuff (rev3)
URL   : https://patchwork.freedesktop.org/series/117201/
State : warning

== Summary ==

Error: dim checkpatch failed
be284779422a drm/dp_mst: Fix fractional DSC bpp handling
af5f8eb69814 drm/i915/mst: Remove broken MST DSC support
adc050d4ed5d drm/i915/mst: Read out FEC state
-:24: WARNING:LONG_LINE: line length of 106 exceeds 100 columns
#24: FILE: drivers/gpu/drm/i915/display/intel_ddi.c:3769:
+					      dp_tp_ctl_reg(encoder, pipe_config)) & DP_TP_CTL_FEC_ENABLE;

total: 0 errors, 1 warnings, 0 checks, 11 lines checked
6d2b9d213014 drm/i915: Fix FEC pipe A vs. DDI A mixup
6fe709863b2a drm/i915: Check lane count when determining FEC support
385e0ee93525 drm/i915: Fix FEC state dump
-:48: WARNING:LONG_LINE: line length of 106 exceeds 100 columns
#48: FILE: drivers/gpu/drm/i915/display/intel_ddi.c:3731:
+					      dp_tp_ctl_reg(encoder, pipe_config)) & DP_TP_CTL_FEC_ENABLE;

total: 0 errors, 1 warnings, 0 checks, 28 lines checked
143d86dca1ff drm/i915: Split some long lines
3270c23c4494 drm/i915: Introduce crtc_state->enhanced_framing
c775af3a458f drm/i915: Stop spamming the logs with PLL state
90bb1913f03d drm/i915: Drop some redundant eDP checks
491e97f4dd89 drm/i915: Reduce combo PHY log spam

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: MST+DSC nukage and state stuff (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: MST+DSC nukage and state stuff (rev3)
URL   : https://patchwork.freedesktop.org/series/117201/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: MST+DSC nukage and state stuff (rev3)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 10388 bytes --]

== Series Details ==

Series: drm/i915: MST+DSC nukage and state stuff (rev3)
URL   : https://patchwork.freedesktop.org/series/117201/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13103 -> Patchwork_117201v3
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (37 -> 38)
------------------------------

  Additional (2): fi-kbl-soraka bat-atsm-1 
  Missing    (1): fi-snb-2520m 

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][1] ([fdo#109271] / [i915#2190])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][2] ([fdo#109271] / [i915#4613]) +3 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html

  * igt@gem_mmap@basic:
    - bat-atsm-1:         NOTRUN -> [SKIP][3] ([i915#4083])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-atsm-1/igt@gem_mmap@basic.html

  * igt@gem_tiled_fence_blits@basic:
    - bat-atsm-1:         NOTRUN -> [SKIP][4] ([i915#4077]) +2 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-atsm-1/igt@gem_tiled_fence_blits@basic.html

  * igt@gem_tiled_pread_basic:
    - bat-atsm-1:         NOTRUN -> [SKIP][5] ([i915#4079]) +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-atsm-1/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_rps@basic-api:
    - bat-atsm-1:         NOTRUN -> [SKIP][6] ([i915#6621])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-atsm-1/igt@i915_pm_rps@basic-api.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][7] ([i915#5334] / [i915#7872])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][8] ([i915#1886] / [i915#7913])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@migrate:
    - bat-dg2-11:         [PASS][9] -> [DMESG-WARN][10] ([i915#7699])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13103/bat-dg2-11/igt@i915_selftest@live@migrate.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-dg2-11/igt@i915_selftest@live@migrate.html

  * igt@i915_suspend@basic-s3-without-i915:
    - bat-atsm-1:         NOTRUN -> [SKIP][11] ([i915#6645])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-atsm-1/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_addfb_basic@size-max:
    - bat-atsm-1:         NOTRUN -> [SKIP][12] ([i915#6077]) +36 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-atsm-1/igt@kms_addfb_basic@size-max.html

  * igt@kms_chamelium_frames@hdmi-crc-fast:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][13] ([fdo#109271]) +16 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/fi-kbl-soraka/igt@kms_chamelium_frames@hdmi-crc-fast.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-atomic:
    - bat-atsm-1:         NOTRUN -> [SKIP][14] ([i915#6078]) +19 similar issues
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-atsm-1/igt@kms_cursor_legacy@basic-flip-after-cursor-atomic.html

  * igt@kms_flip@basic-plain-flip:
    - bat-atsm-1:         NOTRUN -> [SKIP][15] ([i915#6166]) +3 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-atsm-1/igt@kms_flip@basic-plain-flip.html

  * igt@kms_force_connector_basic@prune-stale-modes:
    - bat-atsm-1:         NOTRUN -> [SKIP][16] ([i915#6093]) +3 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-atsm-1/igt@kms_force_connector_basic@prune-stale-modes.html

  * igt@kms_pipe_crc_basic@hang-read-crc:
    - bat-atsm-1:         NOTRUN -> [SKIP][17] ([i915#1836]) +6 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-atsm-1/igt@kms_pipe_crc_basic@hang-read-crc.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence:
    - bat-dg2-11:         NOTRUN -> [SKIP][18] ([i915#1845] / [i915#5354]) +1 similar issue
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-dg2-11/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html

  * igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1:
    - bat-dg2-8:          [PASS][19] -> [FAIL][20] ([i915#7932])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13103/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1.html

  * igt@kms_prop_blob@basic:
    - bat-atsm-1:         NOTRUN -> [SKIP][21] ([i915#7357])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-atsm-1/igt@kms_prop_blob@basic.html

  * igt@kms_psr@sprite_plane_onoff:
    - bat-atsm-1:         NOTRUN -> [SKIP][22] ([i915#1072]) +3 similar issues
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-atsm-1/igt@kms_psr@sprite_plane_onoff.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-atsm-1:         NOTRUN -> [SKIP][23] ([i915#6094])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-atsm-1/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-flip:
    - bat-atsm-1:         NOTRUN -> [SKIP][24] ([fdo#109295] / [i915#6078])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-atsm-1/igt@prime_vgem@basic-fence-flip.html

  * igt@prime_vgem@basic-fence-mmap:
    - bat-atsm-1:         NOTRUN -> [SKIP][25] ([fdo#109295] / [i915#4077]) +1 similar issue
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-atsm-1/igt@prime_vgem@basic-fence-mmap.html

  * igt@prime_vgem@basic-write:
    - bat-atsm-1:         NOTRUN -> [SKIP][26] ([fdo#109295]) +3 similar issues
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-atsm-1/igt@prime_vgem@basic-write.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-kbl-7567u:       [DMESG-FAIL][27] ([i915#5334] / [i915#7872]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13103/fi-kbl-7567u/igt@i915_selftest@live@gt_heartbeat.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/fi-kbl-7567u/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@slpc:
    - {bat-mtlp-8}:       [DMESG-WARN][29] ([i915#6367]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13103/bat-mtlp-8/igt@i915_selftest@live@slpc.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/bat-mtlp-8/igt@i915_selftest@live@slpc.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1836]: https://gitlab.freedesktop.org/drm/intel/issues/1836
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#6077]: https://gitlab.freedesktop.org/drm/intel/issues/6077
  [i915#6078]: https://gitlab.freedesktop.org/drm/intel/issues/6078
  [i915#6093]: https://gitlab.freedesktop.org/drm/intel/issues/6093
  [i915#6094]: https://gitlab.freedesktop.org/drm/intel/issues/6094
  [i915#6166]: https://gitlab.freedesktop.org/drm/intel/issues/6166
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
  [i915#7357]: https://gitlab.freedesktop.org/drm/intel/issues/7357
  [i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699
  [i915#7872]: https://gitlab.freedesktop.org/drm/intel/issues/7872
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7932]: https://gitlab.freedesktop.org/drm/intel/issues/7932


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

  * Linux: CI_DRM_13103 -> Patchwork_117201v3

  CI-20190529: 20190529
  CI_DRM_13103: 6b5e2f67878dc507c30fadd8a136de074e756efc @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7277: 1cb3507f3ff28d11bd5cfabcde576fe78ddab571 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_117201v3: 6b5e2f67878dc507c30fadd8a136de074e756efc @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

b53626a243be drm/i915: Reduce combo PHY log spam
3a2526b72d03 drm/i915: Drop some redundant eDP checks
dfe657cda011 drm/i915: Stop spamming the logs with PLL state
d22b4294e052 drm/i915: Introduce crtc_state->enhanced_framing
ea2017369931 drm/i915: Split some long lines
ad2d0e792402 drm/i915: Fix FEC state dump
6521648f033f drm/i915: Check lane count when determining FEC support
dd9a81a7717a drm/i915: Fix FEC pipe A vs. DDI A mixup
27d618428962 drm/i915/mst: Read out FEC state
5fa9609f6ae7 drm/i915/mst: Remove broken MST DSC support
18477cf1cb92 drm/dp_mst: Fix fractional DSC bpp handling

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/index.html

[-- Attachment #2: Type: text/html, Size: 12252 bytes --]

[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: MST+DSC nukage and state stuff (rev3)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 6762 bytes --]

== Series Details ==

Series: drm/i915: MST+DSC nukage and state stuff (rev3)
URL   : https://patchwork.freedesktop.org/series/117201/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13103_full -> Patchwork_117201v3_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_fair@basic-none-solo@rcs0:
    - shard-apl:          [PASS][1] -> [FAIL][2] ([i915#2842]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13103/shard-apl4/igt@gem_exec_fair@basic-none-solo@rcs0.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/shard-apl6/igt@gem_exec_fair@basic-none-solo@rcs0.html

  * igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ab-hdmi-a1-hdmi-a2:
    - shard-glk:          [PASS][3] -> [FAIL][4] ([i915#2122])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13103/shard-glk8/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ab-hdmi-a1-hdmi-a2.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/shard-glk7/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ab-hdmi-a1-hdmi-a2.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a2:
    - shard-glk:          [PASS][5] -> [FAIL][6] ([i915#79])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13103/shard-glk1/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a2.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/shard-glk3/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a2.html

  * igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-25@pipe-b-hdmi-a-1:
    - shard-snb:          NOTRUN -> [SKIP][7] ([fdo#109271]) +33 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/shard-snb1/igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-25@pipe-b-hdmi-a-1.html

  * igt@kms_setmode@basic@pipe-a-hdmi-a-1:
    - shard-snb:          NOTRUN -> [FAIL][8] ([i915#5465]) +1 similar issue
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/shard-snb1/igt@kms_setmode@basic@pipe-a-hdmi-a-1.html

  
#### Possible fixes ####

  * igt@gem_ctx_exec@basic-nohangcheck:
    - {shard-tglu}:       [FAIL][9] ([i915#6268]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13103/shard-tglu-4/igt@gem_ctx_exec@basic-nohangcheck.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/shard-tglu-10/igt@gem_ctx_exec@basic-nohangcheck.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-glk:          [FAIL][11] ([i915#2842]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13103/shard-glk1/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/shard-glk3/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_exec_whisper@basic-fds-priority-all:
    - {shard-tglu}:       [INCOMPLETE][13] ([i915#6755] / [i915#7392]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13103/shard-tglu-7/igt@gem_exec_whisper@basic-fds-priority-all.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/shard-tglu-5/igt@gem_exec_whisper@basic-fds-priority-all.html

  * igt@i915_selftest@live@gt_heartbeat:
    - shard-apl:          [DMESG-FAIL][15] ([i915#5334]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13103/shard-apl7/igt@i915_selftest@live@gt_heartbeat.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/shard-apl2/igt@i915_selftest@live@gt_heartbeat.html

  * {igt@kms_plane_scaling@intel-max-src-size@pipe-a-hdmi-a-2}:
    - {shard-rkl}:        [FAIL][17] ([i915#8292]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13103/shard-rkl-6/igt@kms_plane_scaling@intel-max-src-size@pipe-a-hdmi-a-2.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/shard-rkl-4/igt@kms_plane_scaling@intel-max-src-size@pipe-a-hdmi-a-2.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109300]: https://bugs.freedesktop.org/show_bug.cgi?id=109300
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#5465]: https://gitlab.freedesktop.org/drm/intel/issues/5465
  [i915#5784]: https://gitlab.freedesktop.org/drm/intel/issues/5784
  [i915#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268
  [i915#6755]: https://gitlab.freedesktop.org/drm/intel/issues/6755
  [i915#6786]: https://gitlab.freedesktop.org/drm/intel/issues/6786
  [i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
  [i915#7392]: https://gitlab.freedesktop.org/drm/intel/issues/7392
  [i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#8211]: https://gitlab.freedesktop.org/drm/intel/issues/8211
  [i915#8292]: https://gitlab.freedesktop.org/drm/intel/issues/8292


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

  * Linux: CI_DRM_13103 -> Patchwork_117201v3

  CI-20190529: 20190529
  CI_DRM_13103: 6b5e2f67878dc507c30fadd8a136de074e756efc @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7277: 1cb3507f3ff28d11bd5cfabcde576fe78ddab571 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_117201v3: 6b5e2f67878dc507c30fadd8a136de074e756efc @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117201v3/index.html

[-- Attachment #2: Type: text/html, Size: 6658 bytes --]

Re: [Intel-gfx] [PATCH 01/11] drm/dp_mst: Fix fractional DSC bpp handling

[Lyude Paul]

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

Thanks!

On Tue, 2023-05-02 at 17:38 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The current code does '(bpp << 4) / 16' in the MST PBN
> calculation, but that is just the same as 'bpp' so the
> DSC codepath achieves absolutely nothing. Fix it up so that
> the fractional part of the bpp value is actually used instead
> of truncated away. 64*1006 has enough zero lsbs that we can
> just shift that down in the dividend and thus still manage
> to stick to a 32bit divisor.
> 
> And while touching this, let's just make the whole thing more
> straightforward by making the passed in bpp value .4 binary
> fixed point always, instead of having to pass in different
> things based on whether DSC is enabled or not.
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: David Francis <David.Francis@amd.com>
> Cc: Mikita Lipski <mikita.lipski@amd.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Fixes: dc48529fb14e ("drm/dp_mst: Add PBN calculation for DSC modes")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 +-
>  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  2 +-
>  drivers/gpu/drm/display/drm_dp_mst_topology.c | 20 +++++--------------
>  drivers/gpu/drm/i915/display/intel_dp_mst.c   |  5 ++---
>  drivers/gpu/drm/nouveau/dispnv50/disp.c       |  3 +--
>  .../gpu/drm/tests/drm_dp_mst_helper_test.c    |  2 +-
>  include/drm/display/drm_dp_mst_helper.h       |  2 +-
>  7 files changed, 12 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 6cacb76f389e..7d58f08a5444 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -6763,7 +6763,7 @@ static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder,
>  								    max_bpc);
>  		bpp = convert_dc_color_depth_into_bpc(color_depth) * 3;
>  		clock = adjusted_mode->clock;
> -		dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp, false);
> +		dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp << 4);
>  	}
>  
>  	dm_new_connector_state->vcpi_slots =
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> index 994ba426ca66..eb4b666e50e8 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> @@ -1515,7 +1515,7 @@ enum dc_status dm_dp_mst_is_port_support_mode(
>  	} else {
>  		/* check if mode could be supported within full_pbn */
>  		bpp = convert_dc_color_depth_into_bpc(stream->timing.display_color_depth) * 3;
> -		pbn = drm_dp_calc_pbn_mode(stream->timing.pix_clk_100hz / 10, bpp, false);
> +		pbn = drm_dp_calc_pbn_mode(stream->timing.pix_clk_100hz / 10, bpp << 4);
>  
>  		if (pbn > aconnector->mst_output_port->full_pbn)
>  			return DC_FAIL_BANDWIDTH_VALIDATE;
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index 38dab76ae69e..cd4c4f22c903 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -4619,13 +4619,12 @@ EXPORT_SYMBOL(drm_dp_check_act_status);
>  
>  /**
>   * drm_dp_calc_pbn_mode() - Calculate the PBN for a mode.
> - * @clock: dot clock for the mode
> - * @bpp: bpp for the mode.
> - * @dsc: DSC mode. If true, bpp has units of 1/16 of a bit per pixel
> + * @clock: dot clock
> + * @bpp: bpp as .4 binary fixed point
>   *
>   * This uses the formula in the spec to calculate the PBN value for a mode.
>   */
> -int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc)
> +int drm_dp_calc_pbn_mode(int clock, int bpp)
>  {
>  	/*
>  	 * margin 5300ppm + 300ppm ~ 0.6% as per spec, factor is 1.006
> @@ -4636,18 +4635,9 @@ int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc)
>  	 * peak_kbps *= (1006/1000)
>  	 * peak_kbps *= (64/54)
>  	 * peak_kbps *= 8    convert to bytes
> -	 *
> -	 * If the bpp is in units of 1/16, further divide by 16. Put this
> -	 * factor in the numerator rather than the denominator to avoid
> -	 * integer overflow
>  	 */
> -
> -	if (dsc)
> -		return DIV_ROUND_UP_ULL(mul_u32_u32(clock * (bpp / 16), 64 * 1006),
> -					8 * 54 * 1000 * 1000);
> -
> -	return DIV_ROUND_UP_ULL(mul_u32_u32(clock * bpp, 64 * 1006),
> -				8 * 54 * 1000 * 1000);
> +	return DIV_ROUND_UP_ULL(mul_u32_u32(clock * bpp, 64 * 1006 >> 4),
> +				1000 * 8 * 54 * 1000);
>  }
>  EXPORT_SYMBOL(drm_dp_calc_pbn_mode);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 2c49d9ab86a2..44c15d6faac4 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -109,8 +109,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
>  			continue;
>  
>  		crtc_state->pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock,
> -						       dsc ? bpp << 4 : bpp,
> -						       dsc);
> +						       bpp << 4);
>  
>  		slots = drm_dp_atomic_find_time_slots(state, &intel_dp->mst_mgr,
>  						      connector->port,
> @@ -936,7 +935,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
>  		return ret;
>  
>  	if (mode_rate > max_rate || mode->clock > max_dotclk ||
> -	    drm_dp_calc_pbn_mode(mode->clock, min_bpp, false) > port->full_pbn) {
> +	    drm_dp_calc_pbn_mode(mode->clock, min_bpp << 4) > port->full_pbn) {
>  		*status = MODE_CLOCK_HIGH;
>  		return 0;
>  	}
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 5bb777ff1313..d896cbb8cf3d 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -961,8 +961,7 @@ nv50_msto_atomic_check(struct drm_encoder *encoder,
>  		const int clock = crtc_state->adjusted_mode.clock;
>  
>  		asyh->or.bpc = connector->display_info.bpc;
> -		asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, asyh->or.bpc * 3,
> -						    false);
> +		asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, asyh->or.bpc * 3 << 4);
>  	}
>  
>  	mst_state = drm_atomic_get_mst_topology_state(state, &mstm->mgr);
> diff --git a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
> index 545beea33e8c..39fc449148e1 100644
> --- a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
> +++ b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
> @@ -56,7 +56,7 @@ static void drm_test_dp_mst_calc_pbn_mode(struct kunit *test)
>  {
>  	const struct drm_dp_mst_calc_pbn_mode_test *params = test->param_value;
>  
> -	KUNIT_EXPECT_EQ(test, drm_dp_calc_pbn_mode(params->clock, params->bpp, params->dsc),
> +	KUNIT_EXPECT_EQ(test, drm_dp_calc_pbn_mode(params->clock, params->bpp << 4),
>  			params->expected);
>  }
>  
> diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h
> index 32c764fb9cb5..c254500b4507 100644
> --- a/include/drm/display/drm_dp_mst_helper.h
> +++ b/include/drm/display/drm_dp_mst_helper.h
> @@ -829,7 +829,7 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_
>  int drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr,
>  			     int link_rate, int link_lane_count);
>  
> -int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc);
> +int drm_dp_calc_pbn_mode(int clock, int bpp);
>  
>  void drm_dp_mst_update_slots(struct drm_dp_mst_topology_state *mst_state, uint8_t link_encoding_cap);
>  

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

Re: [Intel-gfx] [PATCH 03/11] drm/i915/mst: Read out FEC state

[Luca Coelho]

On Tue, 2023-05-02 at 17:38 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The MST codepath is missing FEC readout. Add it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 55f36d9d509c..41cfa28166e4 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3763,6 +3763,11 @@ static void intel_ddi_read_func_ctl(struct intel_encoder *encoder,
>  		intel_cpu_transcoder_get_m1_n1(crtc, cpu_transcoder,
>  					       &pipe_config->dp_m_n);
>  
> +		if (DISPLAY_VER(dev_priv) >= 11)
> +			pipe_config->fec_enable =
> +				intel_de_read(dev_priv,
> +					      dp_tp_ctl_reg(encoder, pipe_config)) & DP_TP_CTL_FEC_ENABLE;
> +
>  		pipe_config->infoframes.enable |=
>  			intel_hdmi_infoframes_enabled(encoder, pipe_config);
>  		break;

LGTM.

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

Re: [Intel-gfx] [PATCH 04/11] drm/i915: Fix FEC pipe A vs. DDI A mixup

[Luca Coelho]

On Tue, 2023-05-02 at 17:38 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On pre-TGL FEC is a port level feature, not a transcoder
> level features, and it's DDI A which doesn't have it, not
> trancodere A.

A couple of typos: "level feature" and "transcoder A".


>  Check for the correct thing when determining
> whether FEC is supported or not.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 4361c1ac65c3..b27b4fb71ed7 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1212,13 +1212,13 @@ void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
>  static bool intel_dp_source_supports_fec(struct intel_dp *intel_dp,
>  					 const struct intel_crtc_state *pipe_config)
>  {
> +	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>  
> -	/* On TGL, FEC is supported on all Pipes */
>  	if (DISPLAY_VER(dev_priv) >= 12)
>  		return true;
>  
> -	if (DISPLAY_VER(dev_priv) == 11 && pipe_config->cpu_transcoder != TRANSCODER_A)
> +	if (DISPLAY_VER(dev_priv) == 11 && encoder->port != PORT_A)
>  		return true;
>  
>  	return false;

Other than that, looks good:

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

Re: [Intel-gfx] [PATCH 05/11] drm/i915: Check lane count when determining FEC support

[Luca Coelho]

On Tue, 2023-05-02 at 17:39 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> ICL doesn't support FEC with a x1 DP link. Make sure
> we don't try to enable FEC in such cases.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index b27b4fb71ed7..9ac199444155 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1218,7 +1218,8 @@ static bool intel_dp_source_supports_fec(struct intel_dp *intel_dp,
>  	if (DISPLAY_VER(dev_priv) >= 12)
>  		return true;
>  
> -	if (DISPLAY_VER(dev_priv) == 11 && encoder->port != PORT_A)
> +	if (DISPLAY_VER(dev_priv) == 11 &&
> +	    encoder->port != PORT_A && pipe_config->lane_count != 1)
>  		return true;
>  
>  	return false;
> @@ -1234,7 +1235,7 @@ static bool intel_dp_supports_fec(struct intel_dp *intel_dp,
>  static bool intel_dp_supports_dsc(struct intel_dp *intel_dp,
>  				  const struct intel_crtc_state *crtc_state)
>  {
> -	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP) && !crtc_state->fec_enable)
> +	if (!intel_dp_is_edp(intel_dp) && !crtc_state->fec_enable)

I'm probably missing something, but this change...


>  		return false;
>  
>  	return intel_dsc_source_support(crtc_state) &&
> @@ -1580,15 +1581,6 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>  	int pipe_bpp;
>  	int ret;
>  
> -	pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> -		intel_dp_supports_fec(intel_dp, pipe_config);
> -
> -	if (!intel_dp_supports_dsc(intel_dp, pipe_config))
> -		return -EINVAL;
> -
> -	if (!intel_dp_dsc_supports_format(intel_dp, pipe_config->output_format))
> -		return -EINVAL;
> -
>  	if (compute_pipe_bpp)
>  		pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, conn_state->max_requested_bpc);
>  	else
> @@ -1615,6 +1607,15 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>  	pipe_config->port_clock = limits->max_rate;
>  	pipe_config->lane_count = limits->max_lane_count;
>  
> +	pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> +		intel_dp_supports_fec(intel_dp, pipe_config);
> +
> +	if (!intel_dp_supports_dsc(intel_dp, pipe_config))
> +		return -EINVAL;
> +
> +	if (!intel_dp_dsc_supports_format(intel_dp, pipe_config->output_format))
> +		return -EINVAL;
> +
>  	if (intel_dp_is_edp(intel_dp)) {
>  		pipe_config->dsc.compressed_bpp =
>  			min_t(u16, drm_edp_dsc_sink_output_bpp(intel_dp->dsc_dpcd) >> 4,

...and this code move are not explained in the commit message? How are
they related?

--
Cheers,
Luca.

Re: [Intel-gfx] [PATCH 06/11] drm/i915: Fix FEC state dump

[Luca Coelho]

On Tue, 2023-05-02 at 17:39 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Stop dumping state while reading it out. We have a proper
> place for that stuff.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  .../gpu/drm/i915/display/intel_crtc_state_dump.c    |  2 ++
>  drivers/gpu/drm/i915/display/intel_ddi.c            | 13 +++----------
>  2 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> index 0cdcaa49656f..91242ffe0768 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> @@ -257,6 +257,8 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
>  		intel_dump_m_n_config(pipe_config, "dp m2_n2",
>  				      pipe_config->lane_count,
>  				      &pipe_config->dp_m2_n2);
> +		drm_dbg_kms(&i915->drm, "fec: %s\n",
> +			    str_enabled_disabled(pipe_config->fec_enable));
>  	}
>  
>  	drm_dbg_kms(&i915->drm, "framestart delay: %d, MSA timing delay: %d\n",
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 41cfa28166e4..4246133950fd 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3725,17 +3725,10 @@ static void intel_ddi_read_func_ctl(struct intel_encoder *encoder,
>  		intel_cpu_transcoder_get_m2_n2(crtc, cpu_transcoder,
>  					       &pipe_config->dp_m2_n2);
>  
> -		if (DISPLAY_VER(dev_priv) >= 11) {
> -			i915_reg_t dp_tp_ctl = dp_tp_ctl_reg(encoder, pipe_config);
> -
> +		if (DISPLAY_VER(dev_priv) >= 11)
>  			pipe_config->fec_enable =
> -				intel_de_read(dev_priv, dp_tp_ctl) & DP_TP_CTL_FEC_ENABLE;
> -
> -			drm_dbg_kms(&dev_priv->drm,
> -				    "[ENCODER:%d:%s] Fec status: %u\n",
> -				    encoder->base.base.id, encoder->base.name,
> -				    pipe_config->fec_enable);
> -		}
> +				intel_de_read(dev_priv,
> +					      dp_tp_ctl_reg(encoder, pipe_config)) & DP_TP_CTL_FEC_ENABLE;
>  
>  		if (dig_port->lspcon.active && dig_port->dp.has_hdmi_sink)
>  			pipe_config->infoframes.enable |=

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

Re: [Intel-gfx] [PATCH 07/11] drm/i915: Split some long lines

[Luca Coelho]

On Tue, 2023-05-02 at 17:39 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Split some overly long lines.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fdi.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

It doesn't matter much, but it would be nice to say where you're
splitting the long lines? If nothing else, it would at least make the
commit message more unique:

"drm/i915: Split some long lines in hsw_fdi_link_train()"

Other than this nitpick:

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

Re: [Intel-gfx] [PATCH v2 08/11] drm/i915: Introduce crtc_state->enhanced_framing

[Luca Coelho]

On Wed, 2023-05-03 at 14:36 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Track DP enhanced framing properly in the crtc state instead
> of relying just on the cached DPCD everywhere, and hook it
> up into the state check and dump.
> 
> v2: Actually set enhanced_framing in .compute_config()
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/g4x_dp.c                 | 10 ++++++++--
>  drivers/gpu/drm/i915/display/intel_crt.c              |  2 ++
>  drivers/gpu/drm/i915/display/intel_crtc_state_dump.c  |  5 +++--
>  drivers/gpu/drm/i915/display/intel_ddi.c              | 11 +++++++++--
>  drivers/gpu/drm/i915/display/intel_display.c          |  1 +
>  drivers/gpu/drm/i915/display/intel_display_types.h    |  2 ++
>  drivers/gpu/drm/i915/display/intel_dp.c               |  3 +++
>  drivers/gpu/drm/i915/display/intel_dp_link_training.c |  2 +-
>  8 files changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c
> index 920d570f7594..534546ea7d0b 100644
> --- a/drivers/gpu/drm/i915/display/g4x_dp.c
> +++ b/drivers/gpu/drm/i915/display/g4x_dp.c
> @@ -141,7 +141,7 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
>  
>  		intel_de_rmw(dev_priv, TRANS_DP_CTL(crtc->pipe),
>  			     TRANS_DP_ENH_FRAMING,
> -			     drm_dp_enhanced_frame_cap(intel_dp->dpcd) ?
> +			     pipe_config->enhanced_framing ?
>  			     TRANS_DP_ENH_FRAMING : 0);
>  	} else {
>  		if (IS_G4X(dev_priv) && pipe_config->limited_color_range)
> @@ -153,7 +153,7 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
>  			intel_dp->DP |= DP_SYNC_VS_HIGH;
>  		intel_dp->DP |= DP_LINK_TRAIN_OFF;
>  
> -		if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
> +		if (pipe_config->enhanced_framing)
>  			intel_dp->DP |= DP_ENHANCED_FRAMING;
>  
>  		if (IS_CHERRYVIEW(dev_priv))
> @@ -351,6 +351,9 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>  		u32 trans_dp = intel_de_read(dev_priv,
>  					     TRANS_DP_CTL(crtc->pipe));
>  
> +		if (trans_dp & TRANS_DP_ENH_FRAMING)
> +			pipe_config->enhanced_framing = true;
> +
>  		if (trans_dp & TRANS_DP_HSYNC_ACTIVE_HIGH)
>  			flags |= DRM_MODE_FLAG_PHSYNC;
>  		else
> @@ -361,6 +364,9 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>  		else
>  			flags |= DRM_MODE_FLAG_NVSYNC;
>  	} else {
> +		if (tmp & DP_ENHANCED_FRAMING)
> +			pipe_config->enhanced_framing = true;
> +
>  		if (tmp & DP_SYNC_HS_HIGH)
>  			flags |= DRM_MODE_FLAG_PHSYNC;
>  		else
> diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
> index 13519f78cf9f..52af64aa9953 100644
> --- a/drivers/gpu/drm/i915/display/intel_crt.c
> +++ b/drivers/gpu/drm/i915/display/intel_crt.c
> @@ -449,6 +449,8 @@ static int hsw_crt_compute_config(struct intel_encoder *encoder,
>  	/* FDI must always be 2.7 GHz */
>  	pipe_config->port_clock = 135000 * 2;
>  
> +	pipe_config->enhanced_framing = true;
> +

Just curious, why are you setting it to true by default here?

Otherwise, the changes look reasonable:

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

Re: [Intel-gfx] [PATCH 09/11] drm/i915: Stop spamming the logs with PLL state

[Luca Coelho]

On Tue, 2023-05-02 at 17:39 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> encoder->get_config() is not the place where the state
> should be dumped. Get rid of the spam.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 51ae1aad7cc7..65e031ff740c 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3863,11 +3863,9 @@ static void mtl_ddi_get_config(struct intel_encoder *encoder,
>  		crtc_state->port_clock = intel_mtl_tbt_calc_port_clock(encoder);
>  	} else if (intel_is_c10phy(i915, phy)) {
>  		intel_c10pll_readout_hw_state(encoder, &crtc_state->cx0pll_state.c10);
> -		intel_c10pll_dump_hw_state(i915, &crtc_state->cx0pll_state.c10);
>  		crtc_state->port_clock = intel_c10pll_calc_port_clock(encoder, &crtc_state->cx0pll_state.c10);
>  	} else {
>  		intel_c20pll_readout_hw_state(encoder, &crtc_state->cx0pll_state.c20);
> -		intel_c20pll_dump_hw_state(i915, &crtc_state->cx0pll_state.c20);
>  		crtc_state->port_clock = intel_c20pll_calc_port_clock(encoder, &crtc_state->cx0pll_state.c20);
>  	}
>  

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

Re: [Intel-gfx] [PATCH 10/11] drm/i915: Drop some redundant eDP checks

[Luca Coelho]

On Tue, 2023-05-02 at 17:39 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> There's no need to check for both eDP and fixed_mode when
> deciding whether to do the pfit calculations or not.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---

It would be nice to explain _why_ this is not needed.  Is it because
fixed_mode is always eDP?

--
Cheers,
Luca.

Re: [Intel-gfx] [PATCH 11/11] drm/i915: Reduce combo PHY log spam

[Luca Coelho]

On Tue, 2023-05-02 at 17:39 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We always check whether combo PHYs need to be re-initialized
> after disabling DC states, which leads to log spam. Switch things
> around so that we only log something when we actually have to
> re-initialized a PHY.
> 
> The log spam was exacerbated by commit 41b4c7fe72b6 ("drm/i915:
> Disable DC states for all commits") since we now disable DC
> states far more often.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_combo_phy.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_combo_phy.c b/drivers/gpu/drm/i915/display/intel_combo_phy.c
> index 922a6d87b553..ee843f2b1af1 100644
> --- a/drivers/gpu/drm/i915/display/intel_combo_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_combo_phy.c
> @@ -114,10 +114,6 @@ static bool icl_verify_procmon_ref_values(struct drm_i915_private *dev_priv,
>  
>  	procmon = icl_get_procmon_ref_values(dev_priv, phy);
>  
> -	drm_dbg_kms(&dev_priv->drm,
> -		    "Combo PHY %c Voltage/Process Info : %s\n",
> -		    phy_name(phy), procmon->name);
> -
>  	ret = check_phy_reg(dev_priv, phy, ICL_PORT_COMP_DW1(phy),
>  			    (0xff << 16) | 0xff, procmon->dw1);
>  	ret &= check_phy_reg(dev_priv, phy, ICL_PORT_COMP_DW9(phy),
> @@ -312,14 +308,17 @@ static void icl_combo_phys_init(struct drm_i915_private *dev_priv)
>  	enum phy phy;
>  
>  	for_each_combo_phy(dev_priv, phy) {
> +		const struct icl_procmon *procmon;
>  		u32 val;
>  
> -		if (icl_combo_phy_verify_state(dev_priv, phy)) {
> -			drm_dbg(&dev_priv->drm,
> -				"Combo PHY %c already enabled, won't reprogram it.\n",
> -				phy_name(phy));
> +		if (icl_combo_phy_verify_state(dev_priv, phy))
>  			continue;
> -		}
> +
> +		procmon = icl_get_procmon_ref_values(dev_priv, phy);
> +
> +		drm_dbg(&dev_priv->drm,
> +			"Initializing combo PHY %c (Voltage/Process Info : %s)\n",
> +			phy_name(phy), procmon->name);
>  
>  		if (!has_phy_misc(dev_priv, phy))
>  			goto skip_phy_misc;

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

Re: [Intel-gfx] [PATCH 02/11] drm/i915/mst: Remove broken MST DSC support

[Dave Airlie]

On Wed, 3 May 2023 at 22:23, Lisovskiy, Stanislav
<stanislav.lisovskiy@intel.com> wrote:
>
> On Wed, May 03, 2023 at 02:07:04PM +0300, Ville Syrjälä wrote:
> > On Wed, May 03, 2023 at 10:36:42AM +0300, Lisovskiy, Stanislav wrote:
> > > On Tue, May 02, 2023 at 05:38:57PM +0300, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > >
> > > > The MST DSC code has a myriad of issues:
> > > > - Platform checks are wrong (MST+DSC is TGL+ only IIRC)
> > > > - Return values of .mode_valid_ctx() are wrong
> > > > - .mode_valid_ctx() assumes bigjoiner might be used, but ther rest
> > > >   of the code doesn't agree
> > > > - compressed bpp calculations don't make sense
> > > > - FEC handling needs to consider the entire link as opposed to just
> > > >   the single stream. Currently FEC would only get enabled if the
> > > >   first enabled stream is compressed. Also I'm not seeing anything
> > > >   that would account for the FEC overhead in any bandwidth calculations
> > > > - PPS SDP is only handled for the first stream via the dig_port
> > > >   hooks, other streams will not be transmittitng any PPS SDPs
> > > > - PPS SDP readout is missing (also missing for SST!)
> > > > - VDSC readout is missing (also missing for SST!)
> > > >
> > > > The FEC issues is really the big one since we have no way currently
> > > > to apply such link wide configuration constraints. Changing that is
> > > > going to require a much bigger rework of the higher level modeset
> > > > .compute_config() logic. We will also need such a rework to properly
> > > > distribute the available bandwidth across all the streams on the
> > > > same link (which is a must to eg. enable deep color).
> > >
> > > Also all the things you mentioned are subject for discussion, for example
> > > I see that FEC overhead is actually accounted for bpp calculation for instance.
> >
> > AFAICS FEC is only accounted for in the data M/N calculations,
> > assuming that particular stream happened to be compressed. I'm
> > not sure if that actually matters since at least the link M/N
> > are not even used by the MST sink. I suppose the data M/N might
> > still be used for something though. For any uncompressed stream
> > on the same link the data M/N values will be calculated
> > incorrectly without FEC.
> >
> > And as mentioned, the FEC bandwidth overhead doesn't seem to
> > be accounted anywhere so no guarantee that we won't try to
> > oversubcribe the link.
> >
> > And FEC will only be enabled if the first stream to be enabled
> > is compressed, otherwise we will enable the link without FEC
> > and still try to cram other compressed streams through it
> > (albeit without the PPS SDP so who knows what will happen)
> > and that is illegal.
> >
> > > We usually improve things by gradually fixing, because if we act same way towards
> > > all wrong code in the driver, we could end up removing the whole i915.
> >
> > We ususally don't merge code that has this many obvious and/or
> > fundemental issues.
>
> Well, this is arguable and subjective judgement. Fact is that, so far we had more MST hubs
> working with that code than without. Also no regressions or anything like that.
> Moreover we usually merge code after code review, in particular those patches
> did spend lots of time in review, where you could comment also.
>
> Regarding merging code with fundamental issues, just recently you had admitted yourself
> that bigjoiner issue for instance, we had recently, was partly caused by your code, because
> we don't anymore copy the pll state to slave crtc.
> I would say that words like "obvious" and "fundamental"
> issues can be applied to many things, however I thought that we always fix things in constructive,
> but not destructive/negative way.
> Should I call also all code completely broken and remove it, once we discover some flaws
> there? Oh, we had many regressions, where I could say the same.
>
> And once again I'm completely okay, if you did introduce better functionality instead
> AND I know you have some valid points there, but now we are just removing everything completely,
> without providing anything better.
>
> But okay, I've mentioned what I think about this and from side this is nak.
> And once the guys to whom those patches helped will pop up from gitlab,
> asking why their MST hubs stopped working - I will just refer them here.
>
> >
> > Now, most of the issues I listed above are probably fixable
> > in a way that could be backported to stable kernels, but
> > unfortunately the FEC issue is not one of those. That one
> > will likely need massive amounts of work all over the driver
> > modeset code, making a backport impossible.
> >
> > > So from my side I would nack it, at least until you have a code which handles
> > > all of this better - I have no doubt you probably have some ideas in your mind, so lets be constructive at least and propose something better first.
> > > This code doesn't cause any regressions, but still provides "some" support to DP MST DSC to say the least and even if that would be removed, if some of those users
> > > refer to me, I would probably then just point to this mail discussion everytime.
> >
> > It seems very likely that it will cause regressions at some point,
> > it just needs a specific multi-display MST setup. The resulting
> > problems will be very confusing to debug since the order in which
> > you enable/disable the outputs will have an impact on what actually
> > goes wrong on account of the FEC and PPS SDP issues. The longer
> > we wait disabling this the harder it will be to deal with those
> > regressions since we the probably can't revert anymore (a straight
> > revert was already not possible) but also can't fix it in a way
> > that can be backported (due to the FEC issues in particular).

I don't think we can reasonably disable this either.

If there are any existant systems which work due to this even if it's
crap, merging this will regress them and Linus will make us revert it.

Sorry this isn't an engineering solution to the problem, going to have
to go around the long way.

Dave.

Re: [Intel-gfx] [PATCH v2 08/11] drm/i915: Introduce crtc_state->enhanced_framing

[Ville Syrjälä]

On Thu, May 25, 2023 at 12:51:28PM +0300, Luca Coelho wrote:
> On Wed, 2023-05-03 at 14:36 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Track DP enhanced framing properly in the crtc state instead
> > of relying just on the cached DPCD everywhere, and hook it
> > up into the state check and dump.
> > 
> > v2: Actually set enhanced_framing in .compute_config()
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/g4x_dp.c                 | 10 ++++++++--
> >  drivers/gpu/drm/i915/display/intel_crt.c              |  2 ++
> >  drivers/gpu/drm/i915/display/intel_crtc_state_dump.c  |  5 +++--
> >  drivers/gpu/drm/i915/display/intel_ddi.c              | 11 +++++++++--
> >  drivers/gpu/drm/i915/display/intel_display.c          |  1 +
> >  drivers/gpu/drm/i915/display/intel_display_types.h    |  2 ++
> >  drivers/gpu/drm/i915/display/intel_dp.c               |  3 +++
> >  drivers/gpu/drm/i915/display/intel_dp_link_training.c |  2 +-
> >  8 files changed, 29 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c
> > index 920d570f7594..534546ea7d0b 100644
> > --- a/drivers/gpu/drm/i915/display/g4x_dp.c
> > +++ b/drivers/gpu/drm/i915/display/g4x_dp.c
> > @@ -141,7 +141,7 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
> >  
> >  		intel_de_rmw(dev_priv, TRANS_DP_CTL(crtc->pipe),
> >  			     TRANS_DP_ENH_FRAMING,
> > -			     drm_dp_enhanced_frame_cap(intel_dp->dpcd) ?
> > +			     pipe_config->enhanced_framing ?
> >  			     TRANS_DP_ENH_FRAMING : 0);
> >  	} else {
> >  		if (IS_G4X(dev_priv) && pipe_config->limited_color_range)
> > @@ -153,7 +153,7 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
> >  			intel_dp->DP |= DP_SYNC_VS_HIGH;
> >  		intel_dp->DP |= DP_LINK_TRAIN_OFF;
> >  
> > -		if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
> > +		if (pipe_config->enhanced_framing)
> >  			intel_dp->DP |= DP_ENHANCED_FRAMING;
> >  
> >  		if (IS_CHERRYVIEW(dev_priv))
> > @@ -351,6 +351,9 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
> >  		u32 trans_dp = intel_de_read(dev_priv,
> >  					     TRANS_DP_CTL(crtc->pipe));
> >  
> > +		if (trans_dp & TRANS_DP_ENH_FRAMING)
> > +			pipe_config->enhanced_framing = true;
> > +
> >  		if (trans_dp & TRANS_DP_HSYNC_ACTIVE_HIGH)
> >  			flags |= DRM_MODE_FLAG_PHSYNC;
> >  		else
> > @@ -361,6 +364,9 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
> >  		else
> >  			flags |= DRM_MODE_FLAG_NVSYNC;
> >  	} else {
> > +		if (tmp & DP_ENHANCED_FRAMING)
> > +			pipe_config->enhanced_framing = true;
> > +
> >  		if (tmp & DP_SYNC_HS_HIGH)
> >  			flags |= DRM_MODE_FLAG_PHSYNC;
> >  		else
> > diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
> > index 13519f78cf9f..52af64aa9953 100644
> > --- a/drivers/gpu/drm/i915/display/intel_crt.c
> > +++ b/drivers/gpu/drm/i915/display/intel_crt.c
> > @@ -449,6 +449,8 @@ static int hsw_crt_compute_config(struct intel_encoder *encoder,
> >  	/* FDI must always be 2.7 GHz */
> >  	pipe_config->port_clock = 135000 * 2;
> >  
> > +	pipe_config->enhanced_framing = true;
> > +
> 
> Just curious, why are you setting it to true by default here?

We always want to use enhanced framing with FDI. Setting it here
and doing the readout allows us to also state check it also for FDI.

> 
> Otherwise, the changes look reasonable:
> 
> Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
> 
> --
> Cheers,
> Luca.

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 05/11] drm/i915: Check lane count when determining FEC support

[Ville Syrjälä]

On Thu, May 25, 2023 at 11:09:30AM +0300, Luca Coelho wrote:
> On Tue, 2023-05-02 at 17:39 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > ICL doesn't support FEC with a x1 DP link. Make sure
> > we don't try to enable FEC in such cases.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 23 ++++++++++++-----------
> >  1 file changed, 12 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index b27b4fb71ed7..9ac199444155 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -1218,7 +1218,8 @@ static bool intel_dp_source_supports_fec(struct intel_dp *intel_dp,
> >  	if (DISPLAY_VER(dev_priv) >= 12)
> >  		return true;
> >  
> > -	if (DISPLAY_VER(dev_priv) == 11 && encoder->port != PORT_A)
> > +	if (DISPLAY_VER(dev_priv) == 11 &&
> > +	    encoder->port != PORT_A && pipe_config->lane_count != 1)
> >  		return true;
> >  
> >  	return false;
> > @@ -1234,7 +1235,7 @@ static bool intel_dp_supports_fec(struct intel_dp *intel_dp,
> >  static bool intel_dp_supports_dsc(struct intel_dp *intel_dp,
> >  				  const struct intel_crtc_state *crtc_state)
> >  {
> > -	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP) && !crtc_state->fec_enable)
> > +	if (!intel_dp_is_edp(intel_dp) && !crtc_state->fec_enable)
> 
> I'm probably missing something, but this change...

This should have been a separate change I suppose. What this is
currently asserting is DP-SST needs FEC to use DSC, but so does DP-MST
which this is totally forgetting to check. eDP is only case where we
can skip FEC.

> 
> 
> >  		return false;
> >  
> >  	return intel_dsc_source_support(crtc_state) &&
> > @@ -1580,15 +1581,6 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> >  	int pipe_bpp;
> >  	int ret;
> >  
> > -	pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> > -		intel_dp_supports_fec(intel_dp, pipe_config);
> > -
> > -	if (!intel_dp_supports_dsc(intel_dp, pipe_config))
> > -		return -EINVAL;
> > -
> > -	if (!intel_dp_dsc_supports_format(intel_dp, pipe_config->output_format))
> > -		return -EINVAL;
> > -
> >  	if (compute_pipe_bpp)
> >  		pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, conn_state->max_requested_bpc);
> >  	else
> > @@ -1615,6 +1607,15 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> >  	pipe_config->port_clock = limits->max_rate;
> >  	pipe_config->lane_count = limits->max_lane_count;
> >  
> > +	pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> > +		intel_dp_supports_fec(intel_dp, pipe_config);
> > +
> > +	if (!intel_dp_supports_dsc(intel_dp, pipe_config))
> > +		return -EINVAL;
> > +
> > +	if (!intel_dp_dsc_supports_format(intel_dp, pipe_config->output_format))
> > +		return -EINVAL;
> > +
> >  	if (intel_dp_is_edp(intel_dp)) {
> >  		pipe_config->dsc.compressed_bpp =
> >  			min_t(u16, drm_edp_dsc_sink_output_bpp(intel_dp->dsc_dpcd) >> 4,
> 
> ...and this code move are not explained in the commit message? How are
> they related?

This is moved becaue we need to compute lanel_count before we can
actually check it.

-- 
Ville Syrjälä
Intel