[Intel-gfx] [PATCH v11 00/11] Handle BPC for HDMI2.1 PCON without DSC1.2 sink and other fixes

[Intel-gfx] [PATCH v11 00/11] Handle BPC for HDMI2.1 PCON without DSC1.2 sink and other fixes

[Ankit Nautiyal]

This series fixes issues faced when an HDMI2.1 sink that does not
support DSC is connected via HDMI2.1PCON. It also includes other minor
HDMI2.1 PCON fixes/refactoring.

Patch 1-2 Have minor fixes/cleanups.
Patch 3-6 Pull the decision making to use DFP conversion capabilities
for every mode during compute config, instead of having that decision
during DP initializing phase.
Patch 7-8 Calculate the max BPC that can be sufficient with either
RGB or YCbcr420 format for the maximum FRL rate supported.

Rev2: Split the refactoring of DFP RG->YCBCR conversion into smaller
patches, as suggested by Jani N.
Also dropped the unnecessary helper for DSC1.2 support for HDMI2.1 DFP.

Rev3: As suggested by Ville, added new member sink_format to store the
final format that the sink will be using, which might be different
than the output format, and thus might need color/format conversion
performed by the PCON.

Rev4: Fix typo in switch case as, reported by kernel test bot.

Rev5: Corrected order of setting sink_format and output_format. (Ville)
Avoided the flag ycbcr420_output and used the sink_format to facilitate
4:2:2 support at a later stage. (Ville)

Rev6: Added missing changes for sdvo. (Ville)
Added check for scaler and DSC constraints with YCbCr420.

Rev7: Split change to add scaler constraint in separate patch, and rebased.

Rev8: Rebased. Fixed check for mode rate with dsc in modevalid.
Fixed scaler constraint as per display version.

Rev9: Rebased.

Rev10: Addressed review comments from Ville.
Dropped patch to check for mode rate with dsc during modevalid, as the
compressed bpp is already selected with bandwidth considerations.

Rev11: Fixed the policy to use output format as RGB first if possible,
followed by YCbCr444, atlast YCbCr420. Also removed the scaler-constraints
with YCbCr420, as these are handled in scaler code. (Ville)

Ankit Nautiyal (11):
  drm/i915/display: Add new member to configure PCON color conversion
  drm/i915/display: Add new member in intel_dp to store ycbcr420
    passthrough cap
  drm/i915/dp: Replace intel_dp.dfp members with the new crtc_state
    sink_format
  drm/i915/display: Use sink_format instead of ycbcr420_output flag
  drm/i915/dp: Rearrange check for illegal mode and comments in
    mode_valid
  drm/i915/dp: Consider output_format while computing dsc bpp for
    mode_valid
  drm/i915/display: Add helper function to check if sink_format is 420
  drm/i915/dp: Avoid DSC with output_format YCBCR420
  drm/i915/dp: Handle BPP where HDMI2.1 DFP doesn't support DSC
  drm/i915/dp: Fix FRL BW check for HDMI2.1 DFP
  drm/i915/dp: Add a wrapper to check frl/tmds downstream constraints

 drivers/gpu/drm/i915/display/icl_dsi.c        |   1 +
 drivers/gpu/drm/i915/display/intel_crt.c      |   1 +
 .../drm/i915/display/intel_crtc_state_dump.c  |   5 +-
 drivers/gpu/drm/i915/display/intel_display.c  |   5 +
 .../drm/i915/display/intel_display_types.h    |  18 +-
 drivers/gpu/drm/i915/display/intel_dp.c       | 420 +++++++++++++-----
 drivers/gpu/drm/i915/display/intel_dp.h       |   4 +-
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |   3 +-
 drivers/gpu/drm/i915/display/intel_dvo.c      |   1 +
 drivers/gpu/drm/i915/display/intel_hdmi.c     |  73 +--
 drivers/gpu/drm/i915/display/intel_hdmi.h     |   5 +-
 drivers/gpu/drm/i915/display/intel_lvds.c     |   1 +
 drivers/gpu/drm/i915/display/intel_sdvo.c     |   1 +
 drivers/gpu/drm/i915/display/intel_tv.c       |   1 +
 drivers/gpu/drm/i915/display/vlv_dsi.c        |   1 +
 15 files changed, 380 insertions(+), 160 deletions(-)

-- 
2.25.1

[Intel-gfx] [PATCH v11 01/11] drm/i915/display: Add new member to configure PCON color conversion

[Ankit Nautiyal]

The decision to use DFP output format conversion capabilities should be
during compute_config phase.

This patch adds new member to crtc_state to represent the final
output_format to the sink. In case of a DFP this can be different than
the output_format, as per the format conversion done via the PCON.

This will help to store only the format conversion capabilities of the
DP device in intel_dp->dfp, and use crtc_state to compute and store the
configuration for color/format conversion for a given mode.

v2: modified the new member to crtc_state to represent the final
output_format that eaches the sink, after possible conversion by
PCON kind of devices. (Ville)

v3: Addressed comments from Ville:
-Added comments to clarify difference between sink_format and
output_format.
-Corrected the order of setting sink_format and output_format.
-Added readout for sink_format in get_pipe_config hooks.

v4: Set sink_format for intel_sdvo too. (Ville)

v5: Rebased.

v6: Fixed condition to go for YCbCr420 format for dp and hdmi. (Ville)

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (v3)
---
 drivers/gpu/drm/i915/display/icl_dsi.c        |  1 +
 drivers/gpu/drm/i915/display/intel_crt.c      |  1 +
 .../drm/i915/display/intel_crtc_state_dump.c  |  5 +--
 drivers/gpu/drm/i915/display/intel_display.c  |  5 +++
 .../drm/i915/display/intel_display_types.h    | 11 ++++++-
 drivers/gpu/drm/i915/display/intel_dp.c       | 33 +++++++++++++------
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |  1 +
 drivers/gpu/drm/i915/display/intel_dvo.c      |  1 +
 drivers/gpu/drm/i915/display/intel_hdmi.c     | 23 +++++++------
 drivers/gpu/drm/i915/display/intel_lvds.c     |  1 +
 drivers/gpu/drm/i915/display/intel_sdvo.c     |  1 +
 drivers/gpu/drm/i915/display/intel_tv.c       |  1 +
 drivers/gpu/drm/i915/display/vlv_dsi.c        |  1 +
 13 files changed, 62 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
index 50dcaa895854..e8ac4552e681 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -1593,6 +1593,7 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
 		&pipe_config->hw.adjusted_mode;
 	int ret;
 
+	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
 	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 
 	ret = intel_panel_compute_config(intel_connector, adjusted_mode);
diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
index 8f2ebead0826..e925e21d87fc 100644
--- a/drivers/gpu/drm/i915/display/intel_crt.c
+++ b/drivers/gpu/drm/i915/display/intel_crt.c
@@ -393,6 +393,7 @@ static int intel_crt_compute_config(struct intel_encoder *encoder,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
 		return -EINVAL;
 
+	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
 	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 
 	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 766633566fd6..185cd1971aa5 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
@@ -178,10 +178,11 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
 
 	snprintf_output_types(buf, sizeof(buf), pipe_config->output_types);
 	drm_dbg_kms(&i915->drm,
-		    "active: %s, output_types: %s (0x%x), output format: %s\n",
+		    "active: %s, output_types: %s (0x%x), output format: %s, sink format: %s\n",
 		    str_yes_no(pipe_config->hw.active),
 		    buf, pipe_config->output_types,
-		    output_formats(pipe_config->output_format));
+		    output_formats(pipe_config->output_format),
+		    output_formats(pipe_config->sink_format));
 
 	drm_dbg_kms(&i915->drm,
 		    "cpu_transcoder: %s, pipe bpp: %i, dithering: %i\n",
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 410c84fd905c..6fa86d1e0480 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3127,6 +3127,7 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 		return false;
 
 	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
+	pipe_config->sink_format = pipe_config->output_format;
 	pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
 	pipe_config->shared_dpll = NULL;
 
@@ -3586,6 +3587,8 @@ static bool ilk_get_pipe_config(struct intel_crtc *crtc,
 		break;
 	}
 
+	pipe_config->sink_format = pipe_config->output_format;
+
 	pipe_config->gamma_mode = REG_FIELD_GET(TRANSCONF_GAMMA_MODE_MASK_ILK, tmp);
 
 	pipe_config->framestart_delay = REG_FIELD_GET(TRANSCONF_FRAME_START_DELAY_MASK, tmp) + 1;
@@ -3984,6 +3987,8 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc,
 			bdw_get_pipemisc_output_format(crtc);
 	}
 
+	pipe_config->sink_format = pipe_config->output_format;
+
 	pipe_config->gamma_mode = intel_de_read(dev_priv,
 						GAMMA_MODE(crtc->pipe));
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index c32bfba06ca1..c9d1863d2765 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1274,9 +1274,18 @@ struct intel_crtc_state {
 	/* HDMI High TMDS char rate ratio */
 	bool hdmi_high_tmds_clock_ratio;
 
-	/* Output format RGB/YCBCR etc */
+	/*
+	 * Output format RGB/YCBCR etc., that is coming out
+	 * at the end of the pipe.
+	 */
 	enum intel_output_format output_format;
 
+	/*
+	 * Sink output format RGB/YCBCR etc., that is going
+	 * into the sink.
+	 */
+	enum intel_output_format sink_format;
+
 	/* enable pipe gamma? */
 	bool gamma_enable;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 8e16745275f6..e52e2ffc355c 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -819,11 +819,12 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
 
 static enum intel_output_format
 intel_dp_output_format(struct intel_connector *connector,
-		       bool ycbcr_420_output)
+		       enum intel_output_format sink_format)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
 
-	if (!connector->base.ycbcr_420_allowed || !ycbcr_420_output)
+	if (!connector->base.ycbcr_420_allowed ||
+	    sink_format != INTEL_OUTPUT_FORMAT_YCBCR420)
 		return INTEL_OUTPUT_FORMAT_RGB;
 
 	if (intel_dp->dfp.rgb_to_ycbcr &&
@@ -862,8 +863,14 @@ intel_dp_mode_min_output_bpp(struct intel_connector *connector,
 			     const struct drm_display_mode *mode)
 {
 	const struct drm_display_info *info = &connector->base.display_info;
-	enum intel_output_format output_format =
-		intel_dp_output_format(connector, drm_mode_is_420_only(info, mode));
+	enum intel_output_format output_format, sink_format;
+
+	if (drm_mode_is_420_only(info, mode))
+		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+	else
+		sink_format = INTEL_OUTPUT_FORMAT_RGB;
+
+	output_format = intel_dp_output_format(connector, sink_format);
 
 	return intel_dp_output_bpp(output_format, intel_dp_min_bpp(output_format));
 }
@@ -2040,23 +2047,29 @@ intel_dp_compute_output_format(struct intel_encoder *encoder,
 
 	ycbcr_420_only = drm_mode_is_420_only(info, adjusted_mode);
 
-	crtc_state->output_format = intel_dp_output_format(connector, ycbcr_420_only);
-
-	if (ycbcr_420_only && !intel_dp_is_ycbcr420(intel_dp, crtc_state)) {
+	if (ycbcr_420_only && !connector->base.ycbcr_420_allowed) {
 		drm_dbg_kms(&i915->drm,
 			    "YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible. Falling back to RGB.\n");
-		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
+		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
+	} else if (ycbcr_420_only) {
+		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+	} else {
+		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
 	}
 
+	crtc_state->output_format = intel_dp_output_format(connector, crtc_state->sink_format);
+
 	ret = intel_dp_compute_link_config(encoder, crtc_state, conn_state,
 					   respect_downstream_limits);
 	if (ret) {
-		if (intel_dp_is_ycbcr420(intel_dp, crtc_state) ||
+		if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
 		    !connector->base.ycbcr_420_allowed ||
 		    !drm_mode_is_420_also(info, adjusted_mode))
 			return ret;
 
-		crtc_state->output_format = intel_dp_output_format(connector, true);
+		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+		crtc_state->output_format = intel_dp_output_format(connector,
+								   crtc_state->sink_format);
 		ret = intel_dp_compute_link_config(encoder, crtc_state, conn_state,
 						   respect_downstream_limits);
 	}
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index a860cbc5dbea..ff0b821a901a 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -293,6 +293,7 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
 		return -EINVAL;
 
+	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
 	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 	pipe_config->has_pch_encoder = false;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dvo.c b/drivers/gpu/drm/i915/display/intel_dvo.c
index eb2dcd866cc8..9884678743b6 100644
--- a/drivers/gpu/drm/i915/display/intel_dvo.c
+++ b/drivers/gpu/drm/i915/display/intel_dvo.c
@@ -271,6 +271,7 @@ static int intel_dvo_compute_config(struct intel_encoder *encoder,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
 		return -EINVAL;
 
+	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
 	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index c7e9e1fbed37..1ad0540c13ee 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2171,13 +2171,13 @@ static bool intel_hdmi_has_audio(struct intel_encoder *encoder,
 
 static enum intel_output_format
 intel_hdmi_output_format(const struct intel_crtc_state *crtc_state,
-			 struct intel_connector *connector,
-			 bool ycbcr_420_output)
+			 struct intel_connector *connector)
 {
 	if (!crtc_state->has_hdmi_sink)
 		return INTEL_OUTPUT_FORMAT_RGB;
 
-	if (connector->base.ycbcr_420_allowed && ycbcr_420_output)
+	if (connector->base.ycbcr_420_allowed &&
+	    crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
 		return INTEL_OUTPUT_FORMAT_YCBCR420;
 	else
 		return INTEL_OUTPUT_FORMAT_RGB;
@@ -2195,23 +2195,26 @@ static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
 	bool ycbcr_420_only = drm_mode_is_420_only(info, adjusted_mode);
 	int ret;
 
-	crtc_state->output_format =
-		intel_hdmi_output_format(crtc_state, connector, ycbcr_420_only);
-
-	if (ycbcr_420_only && !intel_hdmi_is_ycbcr420(crtc_state)) {
+	if (ycbcr_420_only && !connector->base.ycbcr_420_allowed) {
 		drm_dbg_kms(&i915->drm,
 			    "YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible. Falling back to RGB.\n");
-		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
+		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
+	} else if (ycbcr_420_only) {
+		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+	} else {
+		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
 	}
 
+	crtc_state->output_format = intel_hdmi_output_format(crtc_state, connector);
 	ret = intel_hdmi_compute_clock(encoder, crtc_state, respect_downstream_limits);
 	if (ret) {
-		if (intel_hdmi_is_ycbcr420(crtc_state) ||
+		if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
 		    !connector->base.ycbcr_420_allowed ||
 		    !drm_mode_is_420_also(info, adjusted_mode))
 			return ret;
 
-		crtc_state->output_format = intel_hdmi_output_format(crtc_state, connector, true);
+		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+		crtc_state->output_format = intel_hdmi_output_format(crtc_state, connector);
 		ret = intel_hdmi_compute_clock(encoder, crtc_state, respect_downstream_limits);
 	}
 
diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
index a504b3a7fbd5..a7783da37dfd 100644
--- a/drivers/gpu/drm/i915/display/intel_lvds.c
+++ b/drivers/gpu/drm/i915/display/intel_lvds.c
@@ -436,6 +436,7 @@ static int intel_lvds_compute_config(struct intel_encoder *encoder,
 		crtc_state->pipe_bpp = lvds_bpp;
 	}
 
+	crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
 	crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
 
 	/*
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
index e12ba458636c..34ee9dd82a78 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -1351,6 +1351,7 @@ static int intel_sdvo_compute_config(struct intel_encoder *encoder,
 
 	DRM_DEBUG_KMS("forcing bpc to 8 for SDVO\n");
 	pipe_config->pipe_bpp = 8*3;
+	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
 	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 
 	if (HAS_PCH_SPLIT(to_i915(encoder->base.dev)))
diff --git a/drivers/gpu/drm/i915/display/intel_tv.c b/drivers/gpu/drm/i915/display/intel_tv.c
index 3b5ff84dc615..6f7ac225293e 100644
--- a/drivers/gpu/drm/i915/display/intel_tv.c
+++ b/drivers/gpu/drm/i915/display/intel_tv.c
@@ -1204,6 +1204,7 @@ intel_tv_compute_config(struct intel_encoder *encoder,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
 		return -EINVAL;
 
+	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
 	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 
 	drm_dbg_kms(&dev_priv->drm, "forcing bpc to 8 for TV\n");
diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
index 8d2e6e151ba0..82c30feb7a91 100644
--- a/drivers/gpu/drm/i915/display/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
@@ -280,6 +280,7 @@ static int intel_dsi_compute_config(struct intel_encoder *encoder,
 	int ret;
 
 	drm_dbg_kms(&dev_priv->drm, "\n");
+	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
 	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 
 	ret = intel_panel_compute_config(intel_connector, adjusted_mode);
-- 
2.25.1

[Intel-gfx] [PATCH v11 02/11] drm/i915/display: Add new member in intel_dp to store ycbcr420 passthrough cap

[Ankit Nautiyal]

New member to store the YCBCR20 Pass through capability of the DP sink.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_types.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index c9d1863d2765..1b65f062b001 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1736,6 +1736,7 @@ struct intel_dp {
 		int pcon_max_frl_bw;
 		u8 max_bpc;
 		bool ycbcr_444_to_420;
+		bool ycbcr420_passthrough;
 		bool rgb_to_ycbcr;
 	} dfp;
 
-- 
2.25.1

[Intel-gfx] [PATCH v11 03/11] drm/i915/dp: Replace intel_dp.dfp members with the new crtc_state sink_format

[Ankit Nautiyal]

The decision to use DFP output format conversion capabilities should be
during compute_config phase.

This patch uses the members of intel_dp->dfp to only store the
format conversion capabilities of the DP device and uses the crtc_state
sink_format member, to program the protocol-converter for
colorspace/format conversion.

v2: Use sink_format to determine the color conversion config for the
pcon (Ville).

v3: Fix typo: missing 'break' in switch case (lkp kernel test robot).

v4: Add helper to check if DP supports YCBCR420.

v5: Simplify logic for computing output_format, based on the given
sink_format. (Ville).
Added scaler constraint for YCbCr420 output.

v6: Split the patch for Scaler constraint for Ycbcr420.

v7: Simplify the policy for selecting output_format:
Always try for RGB first, followed by YCBCR444, and finally by YCBCR420.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 182 +++++++++++++++++-------
 1 file changed, 129 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index e52e2ffc355c..c31ec9c91c64 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -817,24 +817,82 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
 	return 0;
 }
 
+static bool source_can_output(struct intel_dp *intel_dp,
+			      enum intel_output_format format)
+{
+	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+	bool is_branch = drm_dp_is_branch(intel_dp->dpcd);
+
+	if (format == INTEL_OUTPUT_FORMAT_RGB)
+		return true;
+
+	/*
+	 * No YCbCr output support on gmch platforms.
+	 * Also, ILK doesn't seem capable of DP YCbCr output.
+	 * The displayed image is severly corrupted. SNB+ is fine.
+	 */
+	if (HAS_GMCH(i915) || IS_IRONLAKE(i915))
+		return false;
+
+	if (format == INTEL_OUTPUT_FORMAT_YCBCR444)
+		return true;
+
+	/* Platform < Gen 11 cannot output YCbCr420 format */
+	if (DISPLAY_VER(i915) < 11)
+		return false;
+
+	/* If branch device then PCONs should support YCbCr420 Passthrough */
+	if (format == INTEL_OUTPUT_FORMAT_YCBCR420)
+		return !is_branch || intel_dp->dfp.ycbcr420_passthrough;
+
+	return false;
+}
+
+static bool
+dfp_can_convert_from_rgb(struct intel_dp *intel_dp,
+			 enum intel_output_format sink_format)
+{
+	if (!drm_dp_is_branch(intel_dp->dpcd))
+		return false;
+
+	if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR444)
+		return intel_dp->dfp.rgb_to_ycbcr;
+
+	if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
+		return intel_dp->dfp.rgb_to_ycbcr &&
+			intel_dp->dfp.ycbcr_444_to_420;
+
+	return false;
+}
+
+static bool
+dfp_can_convert_from_ycbcr444(struct intel_dp *intel_dp,
+			      enum intel_output_format sink_format)
+{
+	if (!drm_dp_is_branch(intel_dp->dpcd))
+		return false;
+
+	if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
+		return intel_dp->dfp.ycbcr_444_to_420;
+
+	return false;
+}
+
 static enum intel_output_format
 intel_dp_output_format(struct intel_connector *connector,
 		       enum intel_output_format sink_format)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
 
-	if (!connector->base.ycbcr_420_allowed ||
-	    sink_format != INTEL_OUTPUT_FORMAT_YCBCR420)
-		return INTEL_OUTPUT_FORMAT_RGB;
-
-	if (intel_dp->dfp.rgb_to_ycbcr &&
-	    intel_dp->dfp.ycbcr_444_to_420)
+	if (sink_format == INTEL_OUTPUT_FORMAT_RGB ||
+	    dfp_can_convert_from_rgb(intel_dp, sink_format))
 		return INTEL_OUTPUT_FORMAT_RGB;
 
-	if (intel_dp->dfp.ycbcr_444_to_420)
+	if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR444 ||
+	    dfp_can_convert_from_ycbcr444(intel_dp, sink_format))
 		return INTEL_OUTPUT_FORMAT_YCBCR444;
-	else
-		return INTEL_OUTPUT_FORMAT_YCBCR420;
+
+	return INTEL_OUTPUT_FORMAT_YCBCR420;
 }
 
 int intel_dp_min_bpp(enum intel_output_format output_format)
@@ -2751,6 +2809,8 @@ void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
 					   const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+	bool ycbcr444_to_420 = false;
+	bool rgb_to_ycbcr = false;
 	u8 tmp;
 
 	if (intel_dp->dpcd[DP_DPCD_REV] < 0x13)
@@ -2767,8 +2827,35 @@ void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
 		drm_dbg_kms(&i915->drm, "Failed to %s protocol converter HDMI mode\n",
 			    str_enable_disable(intel_dp->has_hdmi_sink));
 
-	tmp = crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444 &&
-		intel_dp->dfp.ycbcr_444_to_420 ? DP_CONVERSION_TO_YCBCR420_ENABLE : 0;
+	if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
+		switch (crtc_state->output_format) {
+		case INTEL_OUTPUT_FORMAT_YCBCR420:
+			/*
+			 * sink_format is YCbCr420, output_format is also YCbCr420:
+			 * Passthrough.
+			 */
+			break;
+		case INTEL_OUTPUT_FORMAT_YCBCR444:
+			/*
+			 * sink_format is YCbCr420, output_format is YCbCr444:
+			 * Downsample.
+			 */
+			ycbcr444_to_420 = true;
+			break;
+		case INTEL_OUTPUT_FORMAT_RGB:
+			/*
+			 * sink_format is YCbCr420, output_format is RGB:
+			 * Convert to YCbCr444 and Downsample.
+			 */
+			rgb_to_ycbcr = true;
+			ycbcr444_to_420 = true;
+			break;
+		default:
+			break;
+		}
+	}
+
+	tmp = ycbcr444_to_420 ? DP_CONVERSION_TO_YCBCR420_ENABLE : 0;
 
 	if (drm_dp_dpcd_writeb(&intel_dp->aux,
 			       DP_PROTOCOL_CONVERTER_CONTROL_1, tmp) != 1)
@@ -2776,13 +2863,12 @@ void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
 			    "Failed to %s protocol converter YCbCr 4:2:0 conversion mode\n",
 			    str_enable_disable(intel_dp->dfp.ycbcr_444_to_420));
 
-	tmp = intel_dp->dfp.rgb_to_ycbcr ?
-		DP_CONVERSION_BT709_RGB_YCBCR_ENABLE : 0;
+	tmp = rgb_to_ycbcr ? DP_CONVERSION_BT709_RGB_YCBCR_ENABLE : 0;
 
 	if (drm_dp_pcon_convert_rgb_to_ycbcr(&intel_dp->aux, tmp) < 0)
 		drm_dbg_kms(&i915->drm,
-			   "Failed to %s protocol converter RGB->YCbCr conversion mode\n",
-			   str_enable_disable(tmp));
+			    "Failed to %s protocol converter RGB->YCbCr conversion mode\n",
+			    str_enable_disable(tmp));
 }
 
 bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp)
@@ -4572,57 +4658,47 @@ intel_dp_update_dfp(struct intel_dp *intel_dp,
 	intel_dp_get_pcon_dsc_cap(intel_dp);
 }
 
+static bool
+intel_dp_can_ycbcr420(struct intel_dp *intel_dp)
+{
+	if (source_can_output(intel_dp, INTEL_OUTPUT_FORMAT_YCBCR420))
+		return true;
+	/*
+	 * If source cannot support YCbCr420, and PCON has color conv. support:
+	 * Source sends YCbCr444, PCON converts YCbCr444->420 Or
+	 * Source sends RGB444, PCON converts RGB->YCbCr444 + YCbCr444->YCbCr420)
+	 */
+	if (source_can_output(intel_dp, INTEL_OUTPUT_FORMAT_RGB) &&
+	    dfp_can_convert_from_rgb(intel_dp, INTEL_OUTPUT_FORMAT_YCBCR420))
+		return true;
+
+	if (source_can_output(intel_dp, INTEL_OUTPUT_FORMAT_YCBCR444) &&
+	    dfp_can_convert_from_ycbcr444(intel_dp, INTEL_OUTPUT_FORMAT_YCBCR420))
+		return INTEL_OUTPUT_FORMAT_YCBCR444;
+
+	return false;
+}
+
 static void
 intel_dp_update_420(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
 	struct intel_connector *connector = intel_dp->attached_connector;
-	bool is_branch, ycbcr_420_passthrough, ycbcr_444_to_420, rgb_to_ycbcr;
-
-	/* No YCbCr output support on gmch platforms */
-	if (HAS_GMCH(i915))
-		return;
 
-	/*
-	 * ILK doesn't seem capable of DP YCbCr output. The
-	 * displayed image is severly corrupted. SNB+ is fine.
-	 */
-	if (IS_IRONLAKE(i915))
-		return;
-
-	is_branch = drm_dp_is_branch(intel_dp->dpcd);
-	ycbcr_420_passthrough =
+	intel_dp->dfp.ycbcr420_passthrough =
 		drm_dp_downstream_420_passthrough(intel_dp->dpcd,
 						  intel_dp->downstream_ports);
 	/* on-board LSPCON always assumed to support 4:4:4->4:2:0 conversion */
-	ycbcr_444_to_420 =
+	intel_dp->dfp.ycbcr_444_to_420 =
 		dp_to_dig_port(intel_dp)->lspcon.active ||
 		drm_dp_downstream_444_to_420_conversion(intel_dp->dpcd,
 							intel_dp->downstream_ports);
-	rgb_to_ycbcr = drm_dp_downstream_rgb_to_ycbcr_conversion(intel_dp->dpcd,
-								 intel_dp->downstream_ports,
-								 DP_DS_HDMI_BT709_RGB_YCBCR_CONV);
-
-	if (DISPLAY_VER(i915) >= 11) {
-		/* Let PCON convert from RGB->YCbCr if possible */
-		if (is_branch && rgb_to_ycbcr && ycbcr_444_to_420) {
-			intel_dp->dfp.rgb_to_ycbcr = true;
-			intel_dp->dfp.ycbcr_444_to_420 = true;
-			connector->base.ycbcr_420_allowed = true;
-		} else {
-		/* Prefer 4:2:0 passthrough over 4:4:4->4:2:0 conversion */
-			intel_dp->dfp.ycbcr_444_to_420 =
-				ycbcr_444_to_420 && !ycbcr_420_passthrough;
+	intel_dp->dfp.rgb_to_ycbcr =
+		drm_dp_downstream_rgb_to_ycbcr_conversion(intel_dp->dpcd,
+							  intel_dp->downstream_ports,
+							  DP_DS_HDMI_BT709_RGB_YCBCR_CONV);
 
-			connector->base.ycbcr_420_allowed =
-				!is_branch || ycbcr_444_to_420 || ycbcr_420_passthrough;
-		}
-	} else {
-		/* 4:4:4->4:2:0 conversion is the only way */
-		intel_dp->dfp.ycbcr_444_to_420 = ycbcr_444_to_420;
-
-		connector->base.ycbcr_420_allowed = ycbcr_444_to_420;
-	}
+	connector->base.ycbcr_420_allowed = intel_dp_can_ycbcr420(intel_dp);
 
 	drm_dbg_kms(&i915->drm,
 		    "[CONNECTOR:%d:%s] RGB->YcbCr conversion? %s, YCbCr 4:2:0 allowed? %s, YCbCr 4:4:4->4:2:0 conversion? %s\n",
-- 
2.25.1

[Intel-gfx] [PATCH v11 04/11] drm/i915/display: Use sink_format instead of ycbcr420_output flag

[Ankit Nautiyal]

Start passing the sink_format, to all functions that take a bool
ycbcr420_output as parameter. This will make the functions generic,
and will serve as a slight step towards 4:2:2 support later.

v2: Rebased.

Suggested-by: Ville Syrj_l_ <ville.syrjala@linux.intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c   | 33 +++++++--------
 drivers/gpu/drm/i915/display/intel_hdmi.c | 50 ++++++++++++-----------
 drivers/gpu/drm/i915/display/intel_hdmi.h |  5 ++-
 3 files changed, 44 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index c31ec9c91c64..b49d113357e4 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -967,7 +967,8 @@ static int intel_dp_max_tmds_clock(struct intel_dp *intel_dp)
 
 static enum drm_mode_status
 intel_dp_tmds_clock_valid(struct intel_dp *intel_dp,
-			  int clock, int bpc, bool ycbcr420_output,
+			  int clock, int bpc,
+			  enum intel_output_format sink_format,
 			  bool respect_downstream_limits)
 {
 	int tmds_clock, min_tmds_clock, max_tmds_clock;
@@ -975,7 +976,7 @@ intel_dp_tmds_clock_valid(struct intel_dp *intel_dp,
 	if (!respect_downstream_limits)
 		return MODE_OK;
 
-	tmds_clock = intel_hdmi_tmds_clock(clock, bpc, ycbcr420_output);
+	tmds_clock = intel_hdmi_tmds_clock(clock, bpc, sink_format);
 
 	min_tmds_clock = intel_dp->dfp.min_tmds_clock;
 	max_tmds_clock = intel_dp_max_tmds_clock(intel_dp);
@@ -998,6 +999,7 @@ intel_dp_mode_valid_downstream(struct intel_connector *connector,
 	const struct drm_display_info *info = &connector->base.display_info;
 	enum drm_mode_status status;
 	bool ycbcr_420_only;
+	enum intel_output_format sink_format;
 
 	/* If PCON supports FRL MODE, check FRL bandwidth constraints */
 	if (intel_dp->dfp.pcon_max_frl_bw) {
@@ -1024,18 +1026,22 @@ intel_dp_mode_valid_downstream(struct intel_connector *connector,
 
 	ycbcr_420_only = drm_mode_is_420_only(info, mode);
 
+	if (ycbcr_420_only && connector->base.ycbcr_420_allowed)
+		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+	else
+		sink_format = INTEL_OUTPUT_FORMAT_RGB;
+
 	/* Assume 8bpc for the DP++/HDMI/DVI TMDS clock check */
 	status = intel_dp_tmds_clock_valid(intel_dp, target_clock,
-					   8, ycbcr_420_only, true);
+					   8, sink_format, true);
 
 	if (status != MODE_OK) {
-		if (ycbcr_420_only ||
-		    !connector->base.ycbcr_420_allowed ||
+		if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
 		    !drm_mode_is_420_also(info, mode))
 			return status;
-
+		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
 		status = intel_dp_tmds_clock_valid(intel_dp, target_clock,
-						   8, true, true);
+						   8, sink_format, true);
 		if (status != MODE_OK)
 			return status;
 	}
@@ -1271,19 +1277,10 @@ static bool intel_dp_supports_dsc(struct intel_dp *intel_dp,
 		drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd);
 }
 
-static bool intel_dp_is_ycbcr420(struct intel_dp *intel_dp,
-				 const struct intel_crtc_state *crtc_state)
-{
-	return crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
-		(crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444 &&
-		 intel_dp->dfp.ycbcr_444_to_420);
-}
-
 static int intel_dp_hdmi_compute_bpc(struct intel_dp *intel_dp,
 				     const struct intel_crtc_state *crtc_state,
 				     int bpc, bool respect_downstream_limits)
 {
-	bool ycbcr420_output = intel_dp_is_ycbcr420(intel_dp, crtc_state);
 	int clock = crtc_state->hw.adjusted_mode.crtc_clock;
 
 	/*
@@ -1303,8 +1300,8 @@ static int intel_dp_hdmi_compute_bpc(struct intel_dp *intel_dp,
 
 	for (; bpc >= 8; bpc -= 2) {
 		if (intel_hdmi_bpc_possible(crtc_state, bpc,
-					    intel_dp->has_hdmi_sink, ycbcr420_output) &&
-		    intel_dp_tmds_clock_valid(intel_dp, clock, bpc, ycbcr420_output,
+					    intel_dp->has_hdmi_sink) &&
+		    intel_dp_tmds_clock_valid(intel_dp, clock, bpc, crtc_state->sink_format,
 					      respect_downstream_limits) == MODE_OK)
 			return bpc;
 	}
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 1ad0540c13ee..15bf64a217c2 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1793,11 +1793,6 @@ static bool intel_has_hdmi_sink(struct intel_hdmi *hdmi,
 		READ_ONCE(to_intel_digital_connector_state(conn_state)->force_audio) != HDMI_AUDIO_OFF_DVI;
 }
 
-static bool intel_hdmi_is_ycbcr420(const struct intel_crtc_state *crtc_state)
-{
-	return crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420;
-}
-
 static int hdmi_port_clock_limit(struct intel_hdmi *hdmi,
 				 bool respect_downstream_limits,
 				 bool has_hdmi_sink)
@@ -1871,10 +1866,11 @@ hdmi_port_clock_valid(struct intel_hdmi *hdmi,
 	return MODE_OK;
 }
 
-int intel_hdmi_tmds_clock(int clock, int bpc, bool ycbcr420_output)
+int intel_hdmi_tmds_clock(int clock, int bpc,
+			  enum intel_output_format sink_format)
 {
 	/* YCBCR420 TMDS rate requirement is half the pixel clock */
-	if (ycbcr420_output)
+	if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
 		clock /= 2;
 
 	/*
@@ -1901,7 +1897,8 @@ static bool intel_hdmi_source_bpc_possible(struct drm_i915_private *i915, int bp
 }
 
 static bool intel_hdmi_sink_bpc_possible(struct drm_connector *connector,
-					 int bpc, bool has_hdmi_sink, bool ycbcr420_output)
+					 int bpc, bool has_hdmi_sink,
+					 enum intel_output_format sink_format)
 {
 	const struct drm_display_info *info = &connector->display_info;
 	const struct drm_hdmi_info *hdmi = &info->hdmi;
@@ -1911,7 +1908,7 @@ static bool intel_hdmi_sink_bpc_possible(struct drm_connector *connector,
 		if (!has_hdmi_sink)
 			return false;
 
-		if (ycbcr420_output)
+		if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
 			return hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_36;
 		else
 			return info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_36;
@@ -1919,7 +1916,7 @@ static bool intel_hdmi_sink_bpc_possible(struct drm_connector *connector,
 		if (!has_hdmi_sink)
 			return false;
 
-		if (ycbcr420_output)
+		if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
 			return hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_30;
 		else
 			return info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_30;
@@ -1933,7 +1930,8 @@ static bool intel_hdmi_sink_bpc_possible(struct drm_connector *connector,
 
 static enum drm_mode_status
 intel_hdmi_mode_clock_valid(struct drm_connector *connector, int clock,
-			    bool has_hdmi_sink, bool ycbcr420_output)
+			    bool has_hdmi_sink,
+			    enum intel_output_format sink_format)
 {
 	struct drm_i915_private *i915 = to_i915(connector->dev);
 	struct intel_hdmi *hdmi = intel_attached_hdmi(to_intel_connector(connector));
@@ -1946,12 +1944,12 @@ intel_hdmi_mode_clock_valid(struct drm_connector *connector, int clock,
 	 * least one color depth is accepted.
 	 */
 	for (bpc = 12; bpc >= 8; bpc -= 2) {
-		int tmds_clock = intel_hdmi_tmds_clock(clock, bpc, ycbcr420_output);
+		int tmds_clock = intel_hdmi_tmds_clock(clock, bpc, sink_format);
 
 		if (!intel_hdmi_source_bpc_possible(i915, bpc))
 			continue;
 
-		if (!intel_hdmi_sink_bpc_possible(connector, bpc, has_hdmi_sink, ycbcr420_output))
+		if (!intel_hdmi_sink_bpc_possible(connector, bpc, has_hdmi_sink, sink_format))
 			continue;
 
 		status = hdmi_port_clock_valid(hdmi, tmds_clock, true, has_hdmi_sink);
@@ -1976,6 +1974,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 	int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
 	bool has_hdmi_sink = intel_has_hdmi_sink(hdmi, connector->state);
 	bool ycbcr_420_only;
+	enum intel_output_format sink_format;
 
 	if ((mode->flags & DRM_MODE_FLAG_3D_MASK) == DRM_MODE_FLAG_3D_FRAME_PACKING)
 		clock *= 2;
@@ -2000,14 +1999,17 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 
 	ycbcr_420_only = drm_mode_is_420_only(&connector->display_info, mode);
 
-	status = intel_hdmi_mode_clock_valid(connector, clock, has_hdmi_sink, ycbcr_420_only);
+	sink_format = ycbcr_420_only ? INTEL_OUTPUT_FORMAT_YCBCR420 : INTEL_OUTPUT_FORMAT_RGB;
+
+	status = intel_hdmi_mode_clock_valid(connector, clock, has_hdmi_sink, sink_format);
 	if (status != MODE_OK) {
 		if (ycbcr_420_only ||
 		    !connector->ycbcr_420_allowed ||
 		    !drm_mode_is_420_also(&connector->display_info, mode))
 			return status;
 
-		status = intel_hdmi_mode_clock_valid(connector, clock, has_hdmi_sink, true);
+		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+		status = intel_hdmi_mode_clock_valid(connector, clock, has_hdmi_sink, sink_format);
 		if (status != MODE_OK)
 			return status;
 	}
@@ -2016,7 +2018,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 }
 
 bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state,
-			     int bpc, bool has_hdmi_sink, bool ycbcr420_output)
+			     int bpc, bool has_hdmi_sink)
 {
 	struct drm_atomic_state *state = crtc_state->uapi.state;
 	struct drm_connector_state *connector_state;
@@ -2027,7 +2029,8 @@ bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state,
 		if (connector_state->crtc != crtc_state->uapi.crtc)
 			continue;
 
-		if (!intel_hdmi_sink_bpc_possible(connector, bpc, has_hdmi_sink, ycbcr420_output))
+		if (!intel_hdmi_sink_bpc_possible(connector, bpc, has_hdmi_sink,
+						  crtc_state->sink_format))
 			return false;
 	}
 
@@ -2045,14 +2048,13 @@ static bool hdmi_bpc_possible(const struct intel_crtc_state *crtc_state, int bpc
 		return false;
 
 	/* Display Wa_1405510057:icl,ehl */
-	if (intel_hdmi_is_ycbcr420(crtc_state) &&
+	if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
 	    bpc == 10 && DISPLAY_VER(dev_priv) == 11 &&
 	    (adjusted_mode->crtc_hblank_end -
 	     adjusted_mode->crtc_hblank_start) % 8 == 2)
 		return false;
 
-	return intel_hdmi_bpc_possible(crtc_state, bpc, crtc_state->has_hdmi_sink,
-				       intel_hdmi_is_ycbcr420(crtc_state));
+	return intel_hdmi_bpc_possible(crtc_state, bpc, crtc_state->has_hdmi_sink);
 }
 
 static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
@@ -2060,7 +2062,6 @@ static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
 				  int clock, bool respect_downstream_limits)
 {
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
-	bool ycbcr420_output = intel_hdmi_is_ycbcr420(crtc_state);
 	int bpc;
 
 	/*
@@ -2078,7 +2079,8 @@ static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
 		bpc = 8;
 
 	for (; bpc >= 8; bpc -= 2) {
-		int tmds_clock = intel_hdmi_tmds_clock(clock, bpc, ycbcr420_output);
+		int tmds_clock = intel_hdmi_tmds_clock(clock, bpc,
+						       crtc_state->sink_format);
 
 		if (hdmi_bpc_possible(crtc_state, bpc) &&
 		    hdmi_port_clock_valid(intel_hdmi, tmds_clock,
@@ -2108,7 +2110,7 @@ static int intel_hdmi_compute_clock(struct intel_encoder *encoder,
 		return bpc;
 
 	crtc_state->port_clock =
-		intel_hdmi_tmds_clock(clock, bpc, intel_hdmi_is_ycbcr420(crtc_state));
+		intel_hdmi_tmds_clock(clock, bpc, crtc_state->sink_format);
 
 	/*
 	 * pipe_bpp could already be below 8bpc due to
@@ -2293,7 +2295,7 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
 		return ret;
 	}
 
-	if (intel_hdmi_is_ycbcr420(pipe_config)) {
+	if (pipe_config->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
 		ret = intel_panel_fitting(pipe_config, conn_state);
 		if (ret)
 			return ret;
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.h b/drivers/gpu/drm/i915/display/intel_hdmi.h
index 774dda2376ed..d1e27247b657 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.h
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.h
@@ -9,6 +9,7 @@
 #include <linux/types.h>
 
 enum hdmi_infoframe_type;
+enum intel_output_format;
 enum port;
 struct drm_connector;
 struct drm_connector_state;
@@ -45,8 +46,8 @@ void intel_read_infoframe(struct intel_encoder *encoder,
 bool intel_hdmi_limited_color_range(const struct intel_crtc_state *crtc_state,
 				    const struct drm_connector_state *conn_state);
 bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state,
-			     int bpc, bool has_hdmi_sink, bool ycbcr420_output);
-int intel_hdmi_tmds_clock(int clock, int bpc, bool ycbcr420_output);
+			     int bpc, bool has_hdmi_sink);
+int intel_hdmi_tmds_clock(int clock, int bpc, enum intel_output_format sink_format);
 int intel_hdmi_dsc_get_bpp(int src_fractional_bpp, int slice_width,
 			   int num_slices, int output_format, bool hdmi_all_bpp,
 			   int hdmi_max_chunk_bytes);
-- 
2.25.1

[Intel-gfx] [PATCH v11 05/11] drm/i915/dp: Rearrange check for illegal mode and comments in mode_valid

[Ankit Nautiyal]

Check for MODE_H_ILLEGAL before calculating max rates, lanes etc.
Move comments about compressed bpp U6.4 format closer to where it is used.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index b49d113357e4..dcb3c2519041 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1098,6 +1098,9 @@ intel_dp_mode_valid(struct drm_connector *_connector,
 	if (target_clock > max_dotclk)
 		return MODE_CLOCK_HIGH;
 
+	if (intel_dp_hdisplay_bad(dev_priv, mode->hdisplay))
+		return MODE_H_ILLEGAL;
+
 	max_link_clock = intel_dp_max_link_rate(intel_dp);
 	max_lanes = intel_dp_max_lane_count(intel_dp);
 
@@ -1105,13 +1108,6 @@ intel_dp_mode_valid(struct drm_connector *_connector,
 	mode_rate = intel_dp_link_required(target_clock,
 					   intel_dp_mode_min_output_bpp(connector, mode));
 
-	if (intel_dp_hdisplay_bad(dev_priv, mode->hdisplay))
-		return MODE_H_ILLEGAL;
-
-	/*
-	 * Output bpp is stored in 6.4 format so right shift by 4 to get the
-	 * integer value since we support only integer values of bpp.
-	 */
 	if (HAS_DSC(dev_priv) &&
 	    drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)) {
 		/*
@@ -1120,6 +1116,10 @@ intel_dp_mode_valid(struct drm_connector *_connector,
 		 */
 		int pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, U8_MAX);
 
+		/*
+		 * Output bpp is stored in 6.4 format so right shift by 4 to get the
+		 * integer value since we support only integer values of bpp.
+		 */
 		if (intel_dp_is_edp(intel_dp)) {
 			dsc_max_output_bpp =
 				drm_edp_dsc_sink_output_bpp(intel_dp->dsc_dpcd) >> 4;
-- 
2.25.1

[Intel-gfx] [PATCH v11 06/11] drm/i915/dp: Consider output_format while computing dsc bpp for mode_valid

[Ankit Nautiyal]

During modevalid step, the pipe bpp is computed assuming RGB output
format. When checking with DSC, consider the output_format and compute
the input bpp for DSC appropriately.

v2: For DP-MST we currently use RGB output format only, so continue
using RGB while computing dsc_bpp for MST case.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c     | 28 ++++++++++++++++-----
 drivers/gpu/drm/i915/display/intel_dp.h     |  4 ++-
 drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 +-
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index dcb3c2519041..499390c519ca 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1110,11 +1110,21 @@ intel_dp_mode_valid(struct drm_connector *_connector,
 
 	if (HAS_DSC(dev_priv) &&
 	    drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)) {
+		int pipe_bpp;
+		enum intel_output_format output_format, sink_format;
+		const struct drm_display_info *info = &connector->base.display_info;
+
+		if (drm_mode_is_420_only(info, mode))
+			sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+		else
+			sink_format = INTEL_OUTPUT_FORMAT_RGB;
+
+		output_format = intel_dp_output_format(connector, sink_format);
 		/*
 		 * 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);
+		pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, output_format, U8_MAX);
 
 		/*
 		 * Output bpp is stored in 6.4 format so right shift by 4 to get the
@@ -1454,12 +1464,15 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
 	return -EINVAL;
 }
 
-int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8 max_req_bpc)
+int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp,
+			     enum intel_output_format output_format,
+			     u8 max_req_bpc)
 {
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
 	int i, num_bpc;
 	u8 dsc_bpc[3] = {0};
 	u8 dsc_max_bpc;
+	int pipe_bpp = 0;
 
 	/* Max DSC Input BPC for ICL is 10 and for TGL+ is 12 */
 	if (DISPLAY_VER(i915) >= 12)
@@ -1470,11 +1483,13 @@ int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8 max_req_bpc)
 	num_bpc = drm_dp_dsc_sink_supported_input_bpcs(intel_dp->dsc_dpcd,
 						       dsc_bpc);
 	for (i = 0; i < num_bpc; i++) {
-		if (dsc_max_bpc >= dsc_bpc[i])
-			return dsc_bpc[i] * 3;
+		if (dsc_max_bpc >= dsc_bpc[i]) {
+			pipe_bpp = dsc_bpc[i] * 3;
+			break;
+		}
 	}
 
-	return 0;
+	return intel_dp_output_bpp(output_format, pipe_bpp);
 }
 
 static int intel_dp_source_dsc_version_minor(struct intel_dp *intel_dp)
@@ -1588,7 +1603,8 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
 		return -EINVAL;
 
 	if (compute_pipe_bpp)
-		pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, conn_state->max_requested_bpc);
+		pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, pipe_config->output_format,
+						    conn_state->max_requested_bpc);
 	else
 		pipe_bpp = pipe_config->pipe_bpp;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index ef39e4f7a329..2f4136e43f38 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -102,7 +102,9 @@ void intel_read_dp_sdp(struct intel_encoder *encoder,
 		       struct intel_crtc_state *crtc_state,
 		       unsigned int type);
 bool intel_digital_port_connected(struct intel_encoder *encoder);
-int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8 dsc_max_bpc);
+int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp,
+			     enum intel_output_format output_format,
+			     u8 dsc_max_bpc);
 u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
 				u32 link_clock, u32 lane_count,
 				u32 mode_clock, u32 mode_hdisplay,
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index ff0b821a901a..bdc5c53ccd75 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -917,7 +917,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
 		 * 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);
+		int pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, INTEL_OUTPUT_FORMAT_RGB, U8_MAX);
 
 		if (drm_dp_sink_supports_fec(intel_dp->fec_capable)) {
 			dsc_max_output_bpp =
-- 
2.25.1

[Intel-gfx] [PATCH v11 07/11] drm/i915/display: Add helper function to check if sink_format is 420

[Ankit Nautiyal]

Add an inline helper function to check if the sink_format is set to
YCBCR420 format.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_types.h | 6 ++++++
 drivers/gpu/drm/i915/display/intel_dp.c            | 4 ++--
 drivers/gpu/drm/i915/display/intel_hdmi.c          | 6 +++---
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 1b65f062b001..77d8eb5e2ba9 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -2070,4 +2070,10 @@ to_intel_frontbuffer(struct drm_framebuffer *fb)
 	return fb ? to_intel_framebuffer(fb)->frontbuffer : NULL;
 }
 
+static inline bool
+intel_crtc_has_420_sink_format(const struct intel_crtc_state *crtc_state)
+{
+	return crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420;
+}
+
 #endif /*  __INTEL_DISPLAY_TYPES_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 499390c519ca..b1431ed175bc 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2133,7 +2133,7 @@ intel_dp_compute_output_format(struct intel_encoder *encoder,
 	ret = intel_dp_compute_link_config(encoder, crtc_state, conn_state,
 					   respect_downstream_limits);
 	if (ret) {
-		if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
+		if (intel_crtc_has_420_sink_format(crtc_state) ||
 		    !connector->base.ycbcr_420_allowed ||
 		    !drm_mode_is_420_also(info, adjusted_mode))
 			return ret;
@@ -2840,7 +2840,7 @@ void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
 		drm_dbg_kms(&i915->drm, "Failed to %s protocol converter HDMI mode\n",
 			    str_enable_disable(intel_dp->has_hdmi_sink));
 
-	if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
+	if (intel_crtc_has_420_sink_format(crtc_state)) {
 		switch (crtc_state->output_format) {
 		case INTEL_OUTPUT_FORMAT_YCBCR420:
 			/*
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 15bf64a217c2..73e314f66367 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2048,7 +2048,7 @@ static bool hdmi_bpc_possible(const struct intel_crtc_state *crtc_state, int bpc
 		return false;
 
 	/* Display Wa_1405510057:icl,ehl */
-	if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
+	if (intel_crtc_has_420_sink_format(crtc_state) &&
 	    bpc == 10 && DISPLAY_VER(dev_priv) == 11 &&
 	    (adjusted_mode->crtc_hblank_end -
 	     adjusted_mode->crtc_hblank_start) % 8 == 2)
@@ -2210,7 +2210,7 @@ static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
 	crtc_state->output_format = intel_hdmi_output_format(crtc_state, connector);
 	ret = intel_hdmi_compute_clock(encoder, crtc_state, respect_downstream_limits);
 	if (ret) {
-		if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
+		if (intel_crtc_has_420_sink_format(crtc_state) ||
 		    !connector->base.ycbcr_420_allowed ||
 		    !drm_mode_is_420_also(info, adjusted_mode))
 			return ret;
@@ -2295,7 +2295,7 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
 		return ret;
 	}
 
-	if (pipe_config->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
+	if (intel_crtc_has_420_sink_format(pipe_config)) {
 		ret = intel_panel_fitting(pipe_config, conn_state);
 		if (ret)
 			return ret;
-- 
2.25.1

[Intel-gfx] [PATCH v11 08/11] drm/i915/dp: Avoid DSC with output_format YCBCR420

[Ankit Nautiyal]

Currently, DSC with YCBCR420 is not supported.
Return -EINVAL when trying with DSC with output_format as YCBCR420.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index b1431ed175bc..99a5cd370c1f 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1602,6 +1602,10 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
 	if (!intel_dp_supports_dsc(intel_dp, pipe_config))
 		return -EINVAL;
 
+	/* Currently DSC with YCBCR420 format is not supported */
+	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
+		return -EINVAL;
+
 	if (compute_pipe_bpp)
 		pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, pipe_config->output_format,
 						    conn_state->max_requested_bpc);
-- 
2.25.1

[Intel-gfx] [PATCH v11 09/11] drm/i915/dp: Handle BPP where HDMI2.1 DFP doesn't support DSC

[Ankit Nautiyal]

Currently we use the highest input BPC supported by DP sink while using
DSC.In cases where PCON with HDMI2.1 as branch device, if PCON supports
DSC but HDMI2.1 sink does not supports DSC, The PCON tries to use same
input BPC that is used between Source and the PCON without DSC, which
might not work even with the maximum FRL rate supported by HDMI2.1
sink.

This patch calculates the max BPC that can be sufficient with either
RGB or YCBCR420 format for the maximum FRL rate supported.

v2: Rebase

v3: Use the sink_format in the functions instead of ycbcr420 flag.

v4: Rebase

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 61 +++++++++++++++++++++++--
 1 file changed, 58 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 99a5cd370c1f..6975d493481b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -116,6 +116,7 @@ bool intel_dp_is_edp(struct intel_dp *intel_dp)
 	return dig_port->base.type == INTEL_OUTPUT_EDP;
 }
 
+static bool intel_dp_is_hdmi_2_1_sink(struct intel_dp *intel_dp);
 static void intel_dp_unset_edid(struct intel_dp *intel_dp);
 
 /* Is link rate UHBR and thus 128b/132b? */
@@ -1582,6 +1583,39 @@ static int intel_dp_dsc_compute_params(struct intel_encoder *encoder,
 	return drm_dsc_compute_rc_parameters(vdsc_cfg);
 }
 
+static int
+intel_dp_pcon_hdmi21_get_bpp_nodsc(struct intel_dp *intel_dp,
+				   struct intel_crtc_state *pipe_config,
+				   int max_bpc)
+{
+	struct intel_connector *intel_connector = intel_dp->attached_connector;
+	struct drm_connector *connector = &intel_connector->base;
+	const struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
+	int i, num_bpc;
+	u8 dsc_bpc[3] = {0};
+	int req_rate_gbps;
+	int max_frl_rate = connector->display_info.hdmi.max_lanes *
+			   connector->display_info.hdmi.max_frl_rate_per_lane;
+
+	num_bpc = drm_dp_dsc_sink_supported_input_bpcs(intel_dp->dsc_dpcd,
+						       dsc_bpc);
+	for (i = 0; i < num_bpc; i++) {
+		if (dsc_bpc[i] > max_bpc)
+			continue;
+
+		req_rate_gbps = DIV_ROUND_UP(dsc_bpc[i] * 3 * adjusted_mode->clock, 1000000);
+
+		/* YCBCR420 reduces data rate by 2 */
+		if (intel_crtc_has_420_sink_format(pipe_config))
+			req_rate_gbps /= 2;
+
+		if (req_rate_gbps < max_frl_rate)
+			return dsc_bpc[i] * 3;
+	}
+
+	return 0;
+}
+
 int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
 				struct intel_crtc_state *pipe_config,
 				struct drm_connector_state *conn_state,
@@ -1590,6 +1624,8 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
 				bool compute_pipe_bpp)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	struct intel_connector *intel_connector = intel_dp->attached_connector;
+	struct drm_connector *connector = &intel_connector->base;
 	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
 	const struct drm_display_mode *adjusted_mode =
 		&pipe_config->hw.adjusted_mode;
@@ -1606,12 +1642,31 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
 	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
 		return -EINVAL;
 
-	if (compute_pipe_bpp)
+	/*
+	 * In cases where PCON with HDMI2.1 as branch device, if PCON supports
+	 * DSC but HDMI2.1 sink does not supports DSC, there can be issues due
+	 * to the bpc used.
+	 * With DSC, a source-PCON pair can support the mode with higher bpcs.
+	 * But PCON->Sink pair, cannot support the same bpc without sink having
+	 * DSC support.
+	 * So use the max BPC as input BPC that will be sufficient to show the
+	 * mode without DSC from PCON->HDMI2.1
+	 */
+	if (intel_dp_is_hdmi_2_1_sink(intel_dp) &&
+	    !connector->display_info.hdmi.dsc_cap.v_1p2) {
+		pipe_bpp = intel_dp_pcon_hdmi21_get_bpp_nodsc(intel_dp, pipe_config,
+							      conn_state->max_requested_bpc);
+		if (!pipe_bpp) {
+			drm_dbg_kms(&dev_priv->drm,
+				    "No BPC possible to support the mode without HDMI2.1 DSC\n");
+			return -EINVAL;
+		}
+	} else if (compute_pipe_bpp) {
 		pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, pipe_config->output_format,
 						    conn_state->max_requested_bpc);
-	else
+	} else {
 		pipe_bpp = pipe_config->pipe_bpp;
-
+	}
 	if (intel_dp->force_dsc_bpc) {
 		pipe_bpp = intel_dp->force_dsc_bpc * 3;
 		drm_dbg_kms(&dev_priv->drm, "Input DSC BPP forced to %d", pipe_bpp);
-- 
2.25.1

[Intel-gfx] [PATCH v11 10/11] drm/i915/dp: Fix FRL BW check for HDMI2.1 DFP

[Ankit Nautiyal]

During FRL bandwidth  check for downstream HDMI2.1 sink,
the min BPC supported is incorrectly taken for DP, and the check does
not consider ybcr420 only modes.
This patch fixes the bandwidth calculation similar to the TMDS case, by
taking min 8Bpc and considering Ycbcr420 only modes.

v2: Rebase

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 64 +++++++++++++++++--------
 1 file changed, 45 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 6975d493481b..0333fbbc8e60 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -118,6 +118,7 @@ bool intel_dp_is_edp(struct intel_dp *intel_dp)
 
 static bool intel_dp_is_hdmi_2_1_sink(struct intel_dp *intel_dp);
 static void intel_dp_unset_edid(struct intel_dp *intel_dp);
+static int intel_dp_hdmi_sink_max_frl(struct intel_dp *intel_dp);
 
 /* Is link rate UHBR and thus 128b/132b? */
 bool intel_dp_is_uhbr(const struct intel_crtc_state *crtc_state)
@@ -991,6 +992,32 @@ intel_dp_tmds_clock_valid(struct intel_dp *intel_dp,
 	return MODE_OK;
 }
 
+static enum drm_mode_status
+intel_dp_frl_bw_valid(struct intel_dp *intel_dp, int target_clock,
+		      int bpc, enum intel_output_format sink_format)
+{
+	int target_bw;
+	int max_frl_bw;
+	int bpp = bpc * 3;
+
+	if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
+		target_clock /= 2;
+
+	target_bw = bpp * target_clock;
+
+	/* check for MAX FRL BW for both PCON and HDMI2.1 sink */
+	max_frl_bw = min(intel_dp->dfp.pcon_max_frl_bw,
+			 intel_dp_hdmi_sink_max_frl(intel_dp));
+
+	/* converting bw from Gbps to Kbps*/
+	max_frl_bw = max_frl_bw * 1000000;
+
+	if (target_bw > max_frl_bw)
+		return MODE_CLOCK_HIGH;
+
+	return MODE_OK;
+}
+
 static enum drm_mode_status
 intel_dp_mode_valid_downstream(struct intel_connector *connector,
 			       const struct drm_display_mode *mode,
@@ -999,24 +1026,30 @@ intel_dp_mode_valid_downstream(struct intel_connector *connector,
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
 	const struct drm_display_info *info = &connector->base.display_info;
 	enum drm_mode_status status;
-	bool ycbcr_420_only;
+	bool ycbcr_420_only = drm_mode_is_420_only(info, mode);
 	enum intel_output_format sink_format;
 
+	ycbcr_420_only = drm_mode_is_420_only(info, mode);
+
 	/* If PCON supports FRL MODE, check FRL bandwidth constraints */
 	if (intel_dp->dfp.pcon_max_frl_bw) {
-		int target_bw;
-		int max_frl_bw;
-		int bpp = intel_dp_mode_min_output_bpp(connector, mode);
-
-		target_bw = bpp * target_clock;
 
-		max_frl_bw = intel_dp->dfp.pcon_max_frl_bw;
-
-		/* converting bw from Gbps to Kbps*/
-		max_frl_bw = max_frl_bw * 1000000;
+		if (ycbcr_420_only && connector->base.ycbcr_420_allowed)
+			sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+		else
+			sink_format = INTEL_OUTPUT_FORMAT_RGB;
 
-		if (target_bw > max_frl_bw)
-			return MODE_CLOCK_HIGH;
+		/* Assume 8bpc for the HDMI2.1 FRL BW check */
+		status = intel_dp_frl_bw_valid(intel_dp, target_clock, 8, sink_format);
+		if (status != MODE_OK) {
+			if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
+			    !drm_mode_is_420_also(info, mode))
+				return status;
+			sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+			status = intel_dp_frl_bw_valid(intel_dp, target_clock, 8, sink_format);
+			if (status != MODE_OK)
+				return status;
+		}
 
 		return MODE_OK;
 	}
@@ -1025,13 +1058,6 @@ intel_dp_mode_valid_downstream(struct intel_connector *connector,
 	    target_clock > intel_dp->dfp.max_dotclock)
 		return MODE_CLOCK_HIGH;
 
-	ycbcr_420_only = drm_mode_is_420_only(info, mode);
-
-	if (ycbcr_420_only && connector->base.ycbcr_420_allowed)
-		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
-	else
-		sink_format = INTEL_OUTPUT_FORMAT_RGB;
-
 	/* Assume 8bpc for the DP++/HDMI/DVI TMDS clock check */
 	status = intel_dp_tmds_clock_valid(intel_dp, target_clock,
 					   8, sink_format, true);
-- 
2.25.1

[Intel-gfx] [PATCH v11 11/11] drm/i915/dp: Add a wrapper to check frl/tmds downstream constraints

[Ankit Nautiyal]

Add a wrapper function to check dp_downstream clock/bandwidth
constraints. Based on whether the sink supports FRL/TMDS the wrapper
calls the appropriate FRL/TMDS functions.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 51 +++++++++++--------------
 1 file changed, 23 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 0333fbbc8e60..d83757f211be 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1018,6 +1018,18 @@ intel_dp_frl_bw_valid(struct intel_dp *intel_dp, int target_clock,
 	return MODE_OK;
 }
 
+static enum drm_mode_status
+intel_dp_hdmi_bw_check(struct intel_dp *intel_dp,
+		       int target_clock, int bpc,
+		       enum intel_output_format sink_format,
+		       bool is_frl)
+{
+	if (is_frl)
+		return intel_dp_frl_bw_valid(intel_dp, target_clock, 8, sink_format);
+
+	return intel_dp_tmds_clock_valid(intel_dp, target_clock, 8, sink_format, true);
+}
+
 static enum drm_mode_status
 intel_dp_mode_valid_downstream(struct intel_connector *connector,
 			       const struct drm_display_mode *mode,
@@ -1027,48 +1039,31 @@ intel_dp_mode_valid_downstream(struct intel_connector *connector,
 	const struct drm_display_info *info = &connector->base.display_info;
 	enum drm_mode_status status;
 	bool ycbcr_420_only = drm_mode_is_420_only(info, mode);
+	bool is_frl;
 	enum intel_output_format sink_format;
+	int bpc = 8; /* Assume 8bpc for the DP++/HDMI/DVI TMDS/FRL bw heck */
 
-	ycbcr_420_only = drm_mode_is_420_only(info, mode);
+	if (ycbcr_420_only && connector->base.ycbcr_420_allowed)
+		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+	else
+		sink_format = INTEL_OUTPUT_FORMAT_RGB;
 
 	/* If PCON supports FRL MODE, check FRL bandwidth constraints */
-	if (intel_dp->dfp.pcon_max_frl_bw) {
+	is_frl = intel_dp->dfp.pcon_max_frl_bw ? true : false;
 
-		if (ycbcr_420_only && connector->base.ycbcr_420_allowed)
-			sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
-		else
-			sink_format = INTEL_OUTPUT_FORMAT_RGB;
-
-		/* Assume 8bpc for the HDMI2.1 FRL BW check */
-		status = intel_dp_frl_bw_valid(intel_dp, target_clock, 8, sink_format);
-		if (status != MODE_OK) {
-			if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
-			    !drm_mode_is_420_also(info, mode))
-				return status;
-			sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
-			status = intel_dp_frl_bw_valid(intel_dp, target_clock, 8, sink_format);
-			if (status != MODE_OK)
-				return status;
-		}
-
-		return MODE_OK;
-	}
-
-	if (intel_dp->dfp.max_dotclock &&
+	if (!is_frl && intel_dp->dfp.max_dotclock &&
 	    target_clock > intel_dp->dfp.max_dotclock)
 		return MODE_CLOCK_HIGH;
 
-	/* Assume 8bpc for the DP++/HDMI/DVI TMDS clock check */
-	status = intel_dp_tmds_clock_valid(intel_dp, target_clock,
-					   8, sink_format, true);
+	status = intel_dp_hdmi_bw_check(intel_dp, target_clock, bpc, sink_format, is_frl);
 
 	if (status != MODE_OK) {
 		if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
 		    !drm_mode_is_420_also(info, mode))
 			return status;
 		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
-		status = intel_dp_tmds_clock_valid(intel_dp, target_clock,
-						   8, sink_format, true);
+		status = intel_dp_hdmi_bw_check(intel_dp, target_clock, bpc, sink_format, is_frl);
+	} else {
 		if (status != MODE_OK)
 			return status;
 	}
-- 
2.25.1

[Intel-gfx] ✓ Fi.CI.BAT: success for Handle BPC for HDMI2.1 PCON without DSC1.2 sink and other fixes (rev12)

[Patchwork]

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

== Series Details ==

Series: Handle BPC for HDMI2.1 PCON without DSC1.2 sink and other fixes (rev12)
URL   : https://patchwork.freedesktop.org/series/107550/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12857 -> Patchwork_107550v12
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (36 -> 35)
------------------------------

  Missing    (1): fi-snb-2520m 

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@workarounds:
    - bat-dg2-11:         [PASS][1] -> [INCOMPLETE][2] ([i915#7913])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/bat-dg2-11/igt@i915_selftest@live@workarounds.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/bat-dg2-11/igt@i915_selftest@live@workarounds.html

  * igt@kms_chamelium_hpd@common-hpd-after-suspend:
    - bat-adlp-9:         NOTRUN -> [SKIP][3] ([i915#7828])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/bat-adlp-9/igt@kms_chamelium_hpd@common-hpd-after-suspend.html
    - bat-rpls-1:         NOTRUN -> [SKIP][4] ([i915#7828])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/bat-rpls-1/igt@kms_chamelium_hpd@common-hpd-after-suspend.html

  * igt@kms_pipe_crc_basic@suspend-read-crc:
    - bat-rpls-1:         NOTRUN -> [SKIP][5] ([i915#1845])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/bat-rpls-1/igt@kms_pipe_crc_basic@suspend-read-crc.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-cfl-8109u:       [DMESG-FAIL][6] ([i915#5334]) -> [PASS][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/fi-cfl-8109u/igt@i915_selftest@live@gt_heartbeat.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/fi-cfl-8109u/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@requests:
    - bat-rpls-1:         [ABORT][8] ([i915#4983] / [i915#7694] / [i915#7911] / [i915#7981]) -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/bat-rpls-1/igt@i915_selftest@live@requests.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/bat-rpls-1/igt@i915_selftest@live@requests.html
    - bat-adlp-9:         [ABORT][10] ([i915#7982]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/bat-adlp-9/igt@i915_selftest@live@requests.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/bat-adlp-9/igt@i915_selftest@live@requests.html

  
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#7694]: https://gitlab.freedesktop.org/drm/intel/issues/7694
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7911]: https://gitlab.freedesktop.org/drm/intel/issues/7911
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7981]: https://gitlab.freedesktop.org/drm/intel/issues/7981
  [i915#7982]: https://gitlab.freedesktop.org/drm/intel/issues/7982


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

  * Linux: CI_DRM_12857 -> Patchwork_107550v12

  CI-20190529: 20190529
  CI_DRM_12857: 004fefbbf160569f80946d1e516d538b7ecb04f2 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7194: d22d66efd6211a22d301649b63d58c8c293e0817 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_107550v12: 004fefbbf160569f80946d1e516d538b7ecb04f2 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

0ce37c7b395d drm/i915/dp: Add a wrapper to check frl/tmds downstream constraints
85f651d81633 drm/i915/dp: Fix FRL BW check for HDMI2.1 DFP
af1d32738ec9 drm/i915/dp: Handle BPP where HDMI2.1 DFP doesn't support DSC
8898d36619b2 drm/i915/dp: Avoid DSC with output_format YCBCR420
ee214b9501cd drm/i915/display: Add helper function to check if sink_format is 420
eb129b35dc6c drm/i915/dp: Consider output_format while computing dsc bpp for mode_valid
8bc5dc0dd97f drm/i915/dp: Rearrange check for illegal mode and comments in mode_valid
8d7d37d62f43 drm/i915/display: Use sink_format instead of ycbcr420_output flag
af95e4103ee6 drm/i915/dp: Replace intel_dp.dfp members with the new crtc_state sink_format
1525d0fbf80d drm/i915/display: Add new member in intel_dp to store ycbcr420 passthrough cap
580877b9b9de drm/i915/display: Add new member to configure PCON color conversion

== Logs ==

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

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

Re: [Intel-gfx] [PATCH v11 08/11] drm/i915/dp: Avoid DSC with output_format YCBCR420

[Manasi Navare]

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

Since we cannot do DSC with this output format currently, can this check be
added as part of the intel_dp_supports_dsc() ?

Regards
Manasi


On Tue, Mar 14, 2023 at 4:07 AM Ankit Nautiyal <ankit.k.nautiyal@intel.com>
wrote:

> Currently, DSC with YCBCR420 is not supported.
> Return -EINVAL when trying with DSC with output_format as YCBCR420.
>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index b1431ed175bc..99a5cd370c1f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1602,6 +1602,10 @@ int intel_dp_dsc_compute_config(struct intel_dp
> *intel_dp,
>         if (!intel_dp_supports_dsc(intel_dp, pipe_config))
>                 return -EINVAL;
>
> +       /* Currently DSC with YCBCR420 format is not supported */
> +       if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> +               return -EINVAL;
> +
>         if (compute_pipe_bpp)
>                 pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp,
> pipe_config->output_format,
>
> conn_state->max_requested_bpc);
> --
> 2.25.1
>
>

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

[Intel-gfx] ✓ Fi.CI.IGT: success for Handle BPC for HDMI2.1 PCON without DSC1.2 sink and other fixes (rev12)

[Patchwork]

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

== Series Details ==

Series: Handle BPC for HDMI2.1 PCON without DSC1.2 sink and other fixes (rev12)
URL   : https://patchwork.freedesktop.org/series/107550/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12857_full -> Patchwork_107550v12_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (8 -> 8)
------------------------------

  Additional (1): shard-rkl0 
  Missing    (1): shard-tglu0 

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

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

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_suspend@basic-s3-without-i915:
    - {shard-tglu}:       NOTRUN -> [INCOMPLETE][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-tglu-9/igt@i915_suspend@basic-s3-without-i915.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gen9_exec_parse@allowed-all:
    - shard-apl:          [PASS][2] -> [ABORT][3] ([i915#5566])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-apl1/igt@gen9_exec_parse@allowed-all.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-apl1/igt@gen9_exec_parse@allowed-all.html

  * igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a1:
    - shard-glk:          [PASS][4] -> [FAIL][5] ([i915#79])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-glk7/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a1.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-glk3/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a1.html

  * igt@kms_vblank@pipe-b-ts-continuation-suspend:
    - shard-apl:          [PASS][6] -> [ABORT][7] ([i915#180])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-apl7/igt@kms_vblank@pipe-b-ts-continuation-suspend.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-apl6/igt@kms_vblank@pipe-b-ts-continuation-suspend.html

  
#### Possible fixes ####

  * igt@device_reset@unbind-reset-rebind:
    - {shard-rkl}:        [FAIL][8] ([i915#4778]) -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-rkl-5/igt@device_reset@unbind-reset-rebind.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-rkl-1/igt@device_reset@unbind-reset-rebind.html

  * igt@drm_fdinfo@most-busy-check-all@rcs0:
    - {shard-rkl}:        [FAIL][10] ([i915#7742]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-rkl-3/igt@drm_fdinfo@most-busy-check-all@rcs0.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-rkl-1/igt@drm_fdinfo@most-busy-check-all@rcs0.html

  * igt@fbdev@eof:
    - {shard-rkl}:        [SKIP][12] ([i915#2582]) -> [PASS][13] +1 similar issue
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-rkl-2/igt@fbdev@eof.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-rkl-6/igt@fbdev@eof.html

  * igt@gem_ctx_persistence@engines-hang@bcs0:
    - {shard-rkl}:        [SKIP][14] ([i915#6252]) -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-rkl-5/igt@gem_ctx_persistence@engines-hang@bcs0.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-rkl-1/igt@gem_ctx_persistence@engines-hang@bcs0.html

  * igt@gem_exec_fair@basic-deadline:
    - {shard-rkl}:        [FAIL][16] ([i915#2846]) -> [PASS][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-rkl-2/igt@gem_exec_fair@basic-deadline.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-rkl-4/igt@gem_exec_fair@basic-deadline.html

  * igt@gem_exec_reloc@basic-wc-read-noreloc:
    - {shard-rkl}:        [SKIP][18] ([i915#3281]) -> [PASS][19] +2 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-rkl-6/igt@gem_exec_reloc@basic-wc-read-noreloc.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-rkl-5/igt@gem_exec_reloc@basic-wc-read-noreloc.html

  * igt@gem_exec_suspend@basic-s4-devices@lmem0:
    - {shard-dg1}:        [ABORT][20] ([i915#7975]) -> [PASS][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-dg1-14/igt@gem_exec_suspend@basic-s4-devices@lmem0.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-dg1-18/igt@gem_exec_suspend@basic-s4-devices@lmem0.html

  * igt@gem_mmap_wc@set-cache-level:
    - {shard-tglu}:       [SKIP][22] ([i915#1850]) -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-tglu-9/igt@gem_mmap_wc@set-cache-level.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-tglu-1/igt@gem_mmap_wc@set-cache-level.html

  * igt@gem_userptr_blits@forbidden-operations:
    - {shard-rkl}:        [SKIP][24] ([i915#3282]) -> [PASS][25] +3 similar issues
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-rkl-6/igt@gem_userptr_blits@forbidden-operations.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-rkl-5/igt@gem_userptr_blits@forbidden-operations.html

  * igt@gen9_exec_parse@batch-invalid-length:
    - {shard-rkl}:        [SKIP][26] ([i915#2527]) -> [PASS][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-rkl-6/igt@gen9_exec_parse@batch-invalid-length.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-rkl-5/igt@gen9_exec_parse@batch-invalid-length.html

  * igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a:
    - {shard-tglu}:       [FAIL][28] ([i915#3825]) -> [PASS][29]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-tglu-5/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-tglu-1/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html

  * igt@i915_pm_rc6_residency@rc6-idle@bcs0:
    - {shard-dg1}:        [FAIL][30] ([i915#3591]) -> [PASS][31] +1 similar issue
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-dg1-14/igt@i915_pm_rc6_residency@rc6-idle@bcs0.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-dg1-18/igt@i915_pm_rc6_residency@rc6-idle@bcs0.html

  * igt@i915_pm_rpm@dpms-mode-unset-lpsp:
    - {shard-rkl}:        [SKIP][32] ([i915#1397]) -> [PASS][33]
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-rkl-2/igt@i915_pm_rpm@dpms-mode-unset-lpsp.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-rkl-6/igt@i915_pm_rpm@dpms-mode-unset-lpsp.html

  * igt@i915_pm_rpm@modeset-lpsp:
    - {shard-tglu}:       [SKIP][34] ([i915#1397]) -> [PASS][35]
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-tglu-9/igt@i915_pm_rpm@modeset-lpsp.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-tglu-7/igt@i915_pm_rpm@modeset-lpsp.html

  * igt@i915_pm_rpm@pm-tiling:
    - {shard-rkl}:        [SKIP][36] ([fdo#109308]) -> [PASS][37] +1 similar issue
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-rkl-2/igt@i915_pm_rpm@pm-tiling.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-rkl-6/igt@i915_pm_rpm@pm-tiling.html

  * igt@i915_pm_sseu@full-enable:
    - {shard-rkl}:        [SKIP][38] ([i915#4387]) -> [PASS][39]
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-rkl-6/igt@i915_pm_sseu@full-enable.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-rkl-5/igt@i915_pm_sseu@full-enable.html

  * igt@i915_selftest@live@dmabuf:
    - shard-apl:          [DMESG-FAIL][40] ([i915#7562]) -> [PASS][41]
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-apl2/igt@i915_selftest@live@dmabuf.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-apl4/igt@i915_selftest@live@dmabuf.html

  * igt@kms_ccs@pipe-c-bad-aux-stride-y_tiled_gen12_rc_ccs:
    - {shard-tglu}:       [SKIP][42] ([i915#1845] / [i915#7651]) -> [PASS][43] +26 similar issues
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-tglu-9/igt@kms_ccs@pipe-c-bad-aux-stride-y_tiled_gen12_rc_ccs.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-tglu-1/igt@kms_ccs@pipe-c-bad-aux-stride-y_tiled_gen12_rc_ccs.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-glk:          [FAIL][44] ([i915#2346]) -> [PASS][45]
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-glk4/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-glk6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_cursor_legacy@flip-vs-cursor-varying-size:
    - {shard-tglu}:       [SKIP][46] ([i915#1845]) -> [PASS][47] +1 similar issue
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-tglu-9/igt@kms_cursor_legacy@flip-vs-cursor-varying-size.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-tglu-7/igt@kms_cursor_legacy@flip-vs-cursor-varying-size.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-apl:          [FAIL][48] ([i915#4767]) -> [PASS][49]
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-apl6/igt@kms_fbcon_fbt@fbc-suspend.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-apl7/igt@kms_fbcon_fbt@fbc-suspend.html

  * igt@kms_fence_pin_leak:
    - {shard-tglu}:       [SKIP][50] ([fdo#109274] / [i915#1845]) -> [PASS][51]
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-tglu-9/igt@kms_fence_pin_leak.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-tglu-1/igt@kms_fence_pin_leak.html

  * igt@kms_frontbuffer_tracking@fbc-tiling-linear:
    - {shard-tglu}:       [SKIP][52] ([i915#1849]) -> [PASS][53] +3 similar issues
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-tglu-9/igt@kms_frontbuffer_tracking@fbc-tiling-linear.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-tglu-1/igt@kms_frontbuffer_tracking@fbc-tiling-linear.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-render:
    - {shard-rkl}:        [SKIP][54] ([i915#1849] / [i915#4098]) -> [PASS][55] +16 similar issues
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-rkl-5/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-render.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-rkl-6/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-render.html

  * igt@kms_properties@plane-properties-atomic:
    - {shard-rkl}:        [SKIP][56] ([i915#1849]) -> [PASS][57] +1 similar issue
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-rkl-2/igt@kms_properties@plane-properties-atomic.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-rkl-6/igt@kms_properties@plane-properties-atomic.html

  * igt@kms_psr@cursor_blt:
    - {shard-rkl}:        [SKIP][58] ([i915#1072]) -> [PASS][59] +2 similar issues
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-rkl-2/igt@kms_psr@cursor_blt.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-rkl-6/igt@kms_psr@cursor_blt.html

  * igt@kms_rotation_crc@exhaust-fences:
    - {shard-rkl}:        [SKIP][60] ([i915#1845] / [i915#4098]) -> [PASS][61] +29 similar issues
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-rkl-2/igt@kms_rotation_crc@exhaust-fences.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-rkl-6/igt@kms_rotation_crc@exhaust-fences.html

  * igt@kms_universal_plane@cursor-fb-leak-pipe-c:
    - {shard-tglu}:       [SKIP][62] ([fdo#109274]) -> [PASS][63] +1 similar issue
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-tglu-9/igt@kms_universal_plane@cursor-fb-leak-pipe-c.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-tglu-1/igt@kms_universal_plane@cursor-fb-leak-pipe-c.html

  * igt@prime_self_import@basic-with_one_bo:
    - {shard-rkl}:        [SKIP][64] ([fdo#109315]) -> [PASS][65] +3 similar issues
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-rkl-5/igt@prime_self_import@basic-with_one_bo.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-rkl-1/igt@prime_self_import@basic-with_one_bo.html

  * igt@prime_vgem@basic-read:
    - {shard-rkl}:        [SKIP][66] ([fdo#109295] / [i915#3291] / [i915#3708]) -> [PASS][67]
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-rkl-4/igt@prime_vgem@basic-read.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-rkl-5/igt@prime_vgem@basic-read.html

  * igt@syncobj_timeline@wait-all-for-submit-delayed-submit:
    - {shard-rkl}:        [SKIP][68] ([i915#2575]) -> [PASS][69] +2 similar issues
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12857/shard-rkl-5/igt@syncobj_timeline@wait-all-for-submit-delayed-submit.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107550v12/shard-rkl-1/igt@syncobj_timeline@wait-all-for-submit-delayed-submit.html

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

  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#109308]: https://bugs.freedesktop.org/show_bug.cgi?id=109308
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#110723]: https://bugs.freedesktop.org/show_bug.cgi?id=110723
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111656]: https://bugs.freedesktop.org/show_bug.cgi?id=111656
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [fdo#112054]: https://bugs.freedesktop.org/show_bug.cgi?id=112054
  [fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1257]: https://gitlab.freedesktop.org/drm/intel/issues/1257
  [i915#132]: https://gitlab.freedesktop.org/drm/intel/issues/132
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#1722]: https://gitlab.freedesktop.org/drm/intel/issues/1722
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
  [i915#1839]: https://gitlab.freedesktop.org/drm/intel/issues/1839
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#1850]: https://gitlab.freedesktop.org/drm/intel/issues/1850
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
  [i915#2532]: https://gitlab.freedesktop.org/drm/intel/issues/2532
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2658]: https://gitlab.freedesktop.org/drm/intel/issues/2658
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2705]: https://gitlab.freedesktop.org/drm/intel/issues/2705
  [i915#280]: https://gitlab.freedesktop.org/drm/intel/issues/280
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2846]: https://gitlab.freedesktop.org/drm/intel/issues/2846
  [i915#2856]: https://gitlab.freedesktop.org/drm/intel/issues/2856
  [i915#2920]: https://gitlab.freedesktop.org/drm/intel/issues/2920
  [i915#3116]: https://gitlab.freedesktop.org/drm/intel/issues/3116
  [i915#315]: https://gitlab.freedesktop.org/drm/intel/issues/315
  [i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3299]: https://gitlab.freedesktop.org/drm/intel/issues/3299
  [i915#3323]: https://gitlab.freedesktop.org/drm/intel/issues/3323
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3361]: https://gitlab.freedesktop.org/drm/intel/issues/3361
  [i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
  [i915#3469]: https://gitlab.freedesktop.org/drm/intel/issues/3469
  [i915#3539]: https://gitlab.freedesktop.org/drm/intel/issues/3539
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [i915#3547]: https://gitlab.freedesktop.org/drm/intel/issues/3547
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
  [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
  [i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3734]: https://gitlab.freedesktop.org/drm/intel/issues/3734
  [i915#3742]: https://gitlab.freedesktop.org/drm/intel/issues/3742
  [i915#3825]: https://gitlab.freedesktop.org/drm/intel/issues/3825
  [i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#3936]: https://gitlab.freedesktop.org/drm/intel/issues/3936
  [i915#3952]: https://gitlab.freedesktop.org/drm/intel/issues/3952
  [i915#3955]: https://gitlab.freedesktop.org/drm/intel/issues/3955
  [i915#3989]: https://gitlab.freedesktop.org/drm/intel/issues/3989
  [i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
  [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#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4258]: https://gitlab.freedesktop.org/drm/intel/issues/4258
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4281]: https://gitlab.freedesktop.org/drm/intel/issues/4281
  [i915#4387]: https://gitlab.freedesktop.org/drm/intel/issues/4387
  [i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#4565]: https://gitlab.freedesktop.org/drm/intel/issues/4565
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4767]: https://gitlab.freedesktop.org/drm/intel/issues/4767
  [i915#4771]: https://gitlab.freedesktop.org/drm/intel/issues/4771
  [i915#4778]: https://gitlab.freedesktop.org/drm/intel/issues/4778
  [i915#4812]: https://gitlab.freedesktop.org/drm/intel/issues/4812
  [i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
  [i915#4852]: https://gitlab.freedesktop.org/drm/intel/issues/4852
  [i915#4860]: https://gitlab.freedesktop.org/drm/intel/issues/4860
  [i915#5030]: https://gitlab.freedesktop.org/drm/intel/issues/5030
  [i915#5115]: https://gitlab.freedesktop.org/drm/intel/issues/5115
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5288]: https://gitlab.freedesktop.org/drm/intel/issues/5288
  [i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
  [i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5439]: https://gitlab.freedesktop.org/drm/intel/issues/5439
  [i915#5461]: https://gitlab.freedesktop.org/drm/intel/issues/5461
  [i915#5563]: https://gitlab.freedesktop.org/drm/intel/issues/5563
  [i915#5566]: https://gitlab.freedesktop.org/drm/intel/issues/5566
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6117]: https://gitlab.freedesktop.org/drm/intel/issues/6117
  [i915#6247]: https://gitlab.freedesktop.org/drm/intel/issues/6247
  [i915#6248]: https://gitlab.freedesktop.org/drm/intel/issues/6248
  [i915#6252]: https://gitlab.freedesktop.org/drm/intel/issues/6252
  [i915#6355]: https://gitlab.freedesktop.org/drm/intel/issues/6355
  [i915#6433]: https://gitlab.freedesktop.org/drm/intel/issues/6433
  [i915#6497]: https://gitlab.freedesktop.org/drm/intel/issues/6497
  [i915#6524]: https://gitlab.freedesktop.org/drm/intel/issues/6524
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#6768]: https://gitlab.freedesktop.org/drm/intel/issues/6768
  [i915#6944]: https://gitlab.freedesktop.org/drm/intel/issues/6944
  [i915#6946]: https://gitlab.freedesktop.org/drm/intel/issues/6946
  [i915#6953]: https://gitlab.freedesktop.org/drm/intel/issues/6953
  [i915#7037]: https://gitlab.freedesktop.org/drm/intel/issues/7037
  [i915#7052]: https://gitlab.freedesktop.org/drm/intel/issues/7052
  [i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
  [i915#7118]: https://gitlab.freedesktop.org/drm/intel/issues/7118
  [i915#7128]: https://gitlab.freedesktop.org/drm/intel/issues/7128
  [i915#7294]: https://gitlab.freedesktop.org/drm/intel/issues/7294
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7562]: https://gitlab.freedesktop.org/drm/intel/issues/7562
  [i915#7651]: https://gitlab.freedesktop.org/drm/intel/issues/7651
  [i915#7697]: https://gitlab.freedesktop.org/drm/intel/issues/7697
  [i915#7701]: https://gitlab.freedesktop.org/drm/intel/issues/7701
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
  [i915#7742]: https://gitlab.freedesktop.org/drm/intel/issues/7742
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#7949]: https://gitlab.freedesktop.org/drm/intel/issues/7949
  [i915#7957]: https://gitlab.freedesktop.org/drm/intel/issues/7957
  [i915#7975]: https://gitlab.freedesktop.org/drm/intel/issues/7975
  [i915#8151]: https://gitlab.freedesktop.org/drm/intel/issues/8151
  [i915#8152]: https://gitlab.freedesktop.org/drm/intel/issues/8152
  [i915#8154]: https://gitlab.freedesktop.org/drm/intel/issues/8154
  [i915#8211]: https://gitlab.freedesktop.org/drm/intel/issues/8211
  [i915#8228]: https://gitlab.freedesktop.org/drm/intel/issues/8228
  [i915#8234]: https://gitlab.freedesktop.org/drm/intel/issues/8234
  [i915#8282]: https://gitlab.freedesktop.org/drm/intel/issues/8282


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

  * Linux: CI_DRM_12857 -> Patchwork_107550v12

  CI-20190529: 20190529
  CI_DRM_12857: 004fefbbf160569f80946d1e516d538b7ecb04f2 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7194: d22d66efd6211a22d301649b63d58c8c293e0817 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_107550v12: 004fefbbf160569f80946d1e516d538b7ecb04f2 @ 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_107550v12/index.html

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

Re: [Intel-gfx] [PATCH v11 08/11] drm/i915/dp: Avoid DSC with output_format YCBCR420

[Nautiyal, Ankit K]

Hi Manasi,

I just realized that there is a newer version of the patch in another 
series for DSC 420 support [1].

I added this patch when I was debugging an issue with PCON + 8k YCbCr420 
only mode, and noticed that we set the output_format first and then 
check for DSC.

Later this patch was pulled in DSC420 series and then got some comments 
and modifications.

So, to avoid having similar change in 2 places, perhaps will drop this 
patch from this series, and collect reviews in [1] for the change.

Currently on [1], the check is outside of intel_dp_supports_dsc(), as a 
separate function explicitly checking for the given output_format, but 
we can discuss and have this inside the mentioned function.

Apologies for the inconvenience.

[1] https://patchwork.freedesktop.org/patch/525903/?series=114246&rev=3 
<https://patchwork.freedesktop.org/patch/525903/?series=114246&rev=3>

Thanks & Regards,

Ankit


On 3/14/2023 11:03 PM, Manasi Navare wrote:
> Since we cannot do DSC with this output format currently, can this 
> check be added as part of the intel_dp_supports_dsc() ?
>
> Regards
> Manasi
>
>
> On Tue, Mar 14, 2023 at 4:07 AM Ankit Nautiyal 
> <ankit.k.nautiyal@intel.com> wrote:
>
>     Currently, DSC with YCBCR420 is not supported.
>     Return -EINVAL when trying with DSC with output_format as YCBCR420.
>
>     Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>     ---
>      drivers/gpu/drm/i915/display/intel_dp.c | 4 ++++
>      1 file changed, 4 insertions(+)
>
>     diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>     b/drivers/gpu/drm/i915/display/intel_dp.c
>     index b1431ed175bc..99a5cd370c1f 100644
>     --- a/drivers/gpu/drm/i915/display/intel_dp.c
>     +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>     @@ -1602,6 +1602,10 @@ int intel_dp_dsc_compute_config(struct
>     intel_dp *intel_dp,
>             if (!(intel_dp, pipe_config))
>                     return -EINVAL;
>
>     +       /* Currently DSC with YCBCR420 format is not supported */
>     +       if (pipe_config->output_format ==
>     INTEL_OUTPUT_FORMAT_YCBCR420)
>     +               return -EINVAL;
>     +
>             if (compute_pipe_bpp)
>                     pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp,
>     pipe_config->output_format,
>     conn_state->max_requested_bpc);
>     -- 
>     2.25.1
>

Re: [Intel-gfx] [PATCH v11 03/11] drm/i915/dp: Replace intel_dp.dfp members with the new crtc_state sink_format

[Ville Syrjälä]

On Tue, Mar 14, 2023 at 04:34:07PM +0530, Ankit Nautiyal wrote:
> The decision to use DFP output format conversion capabilities should be
> during compute_config phase.
> 
> This patch uses the members of intel_dp->dfp to only store the
> format conversion capabilities of the DP device and uses the crtc_state
> sink_format member, to program the protocol-converter for
> colorspace/format conversion.
> 
> v2: Use sink_format to determine the color conversion config for the
> pcon (Ville).
> 
> v3: Fix typo: missing 'break' in switch case (lkp kernel test robot).
> 
> v4: Add helper to check if DP supports YCBCR420.
> 
> v5: Simplify logic for computing output_format, based on the given
> sink_format. (Ville).
> Added scaler constraint for YCbCr420 output.
> 
> v6: Split the patch for Scaler constraint for Ycbcr420.
> 
> v7: Simplify the policy for selecting output_format:
> Always try for RGB first, followed by YCBCR444, and finally by YCBCR420.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 182 +++++++++++++++++-------
>  1 file changed, 129 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index e52e2ffc355c..c31ec9c91c64 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -817,24 +817,82 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
>  	return 0;
>  }
>  
> +static bool source_can_output(struct intel_dp *intel_dp,
> +			      enum intel_output_format format)
> +{
> +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> +	bool is_branch = drm_dp_is_branch(intel_dp->dpcd);
> +
> +	if (format == INTEL_OUTPUT_FORMAT_RGB)
> +		return true;
> +
> +	/*
> +	 * No YCbCr output support on gmch platforms.
> +	 * Also, ILK doesn't seem capable of DP YCbCr output.
> +	 * The displayed image is severly corrupted. SNB+ is fine.
> +	 */
> +	if (HAS_GMCH(i915) || IS_IRONLAKE(i915))
> +		return false;
> +
> +	if (format == INTEL_OUTPUT_FORMAT_YCBCR444)
> +		return true;
> +
> +	/* Platform < Gen 11 cannot output YCbCr420 format */
> +	if (DISPLAY_VER(i915) < 11)
> +		return false;
> +
> +	/* If branch device then PCONs should support YCbCr420 Passthrough */
> +	if (format == INTEL_OUTPUT_FORMAT_YCBCR420)
> +		return !is_branch || intel_dp->dfp.ycbcr420_passthrough;

This part isn't really about the source capabilities.
I think it would be more appropriate to do these checks
in the caller.

> +
> +	return false;
> +}
> +
> +static bool
> +dfp_can_convert_from_rgb(struct intel_dp *intel_dp,
> +			 enum intel_output_format sink_format)
> +{
> +	if (!drm_dp_is_branch(intel_dp->dpcd))
> +		return false;
> +
> +	if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR444)
> +		return intel_dp->dfp.rgb_to_ycbcr;
> +
> +	if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> +		return intel_dp->dfp.rgb_to_ycbcr &&
> +			intel_dp->dfp.ycbcr_444_to_420;
> +
> +	return false;
> +}
> +
> +static bool
> +dfp_can_convert_from_ycbcr444(struct intel_dp *intel_dp,
> +			      enum intel_output_format sink_format)
> +{
> +	if (!drm_dp_is_branch(intel_dp->dpcd))
> +		return false;
> +
> +	if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> +		return intel_dp->dfp.ycbcr_444_to_420;
> +
> +	return false;
> +}
> +
>  static enum intel_output_format
>  intel_dp_output_format(struct intel_connector *connector,
>  		       enum intel_output_format sink_format)
>  {
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
>  
> -	if (!connector->base.ycbcr_420_allowed ||
> -	    sink_format != INTEL_OUTPUT_FORMAT_YCBCR420)
> -		return INTEL_OUTPUT_FORMAT_RGB;
> -
> -	if (intel_dp->dfp.rgb_to_ycbcr &&
> -	    intel_dp->dfp.ycbcr_444_to_420)
> +	if (sink_format == INTEL_OUTPUT_FORMAT_RGB ||
> +	    dfp_can_convert_from_rgb(intel_dp, sink_format))
>  		return INTEL_OUTPUT_FORMAT_RGB;
>  
> -	if (intel_dp->dfp.ycbcr_444_to_420)
> +	if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR444 ||
> +	    dfp_can_convert_from_ycbcr444(intel_dp, sink_format))
>  		return INTEL_OUTPUT_FORMAT_YCBCR444;
> -	else
> -		return INTEL_OUTPUT_FORMAT_YCBCR420;
> +
> +	return INTEL_OUTPUT_FORMAT_YCBCR420;
>  }

I'm thinking the caller of intel_dp_output_format() might want
to do a drm_WARN_ON(!source_can_output(output_format))
just to make sure we didn't screw things up too badly.

Or maybe we want to have that assert in intel_dp_output_format()
itself in case there are many callers.

>  
>  int intel_dp_min_bpp(enum intel_output_format output_format)
> @@ -2751,6 +2809,8 @@ void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
>  					   const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> +	bool ycbcr444_to_420 = false;
> +	bool rgb_to_ycbcr = false;
>  	u8 tmp;
>  
>  	if (intel_dp->dpcd[DP_DPCD_REV] < 0x13)
> @@ -2767,8 +2827,35 @@ void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
>  		drm_dbg_kms(&i915->drm, "Failed to %s protocol converter HDMI mode\n",
>  			    str_enable_disable(intel_dp->has_hdmi_sink));
>  
> -	tmp = crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444 &&
> -		intel_dp->dfp.ycbcr_444_to_420 ? DP_CONVERSION_TO_YCBCR420_ENABLE : 0;
> +	if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420) {

I was wondering where YCBCR444 went here, but then I
remembered that we don't have uapi for it. 

But even so, we could consider making this code already
handle it. Would make it that much easier to test
YCbCr 4:4:4 output. Should just take a one line hack
to intel_dp_sink_format() at that point. Could be
a followup patch though.

> +		switch (crtc_state->output_format) {
> +		case INTEL_OUTPUT_FORMAT_YCBCR420:
> +			/*
> +			 * sink_format is YCbCr420, output_format is also YCbCr420:
> +			 * Passthrough.
> +			 */

These comments seem a bit redundant.

> +			break;
> +		case INTEL_OUTPUT_FORMAT_YCBCR444:
> +			/*
> +			 * sink_format is YCbCr420, output_format is YCbCr444:
> +			 * Downsample.
> +			 */
> +			ycbcr444_to_420 = true;
> +			break;
> +		case INTEL_OUTPUT_FORMAT_RGB:
> +			/*
> +			 * sink_format is YCbCr420, output_format is RGB:
> +			 * Convert to YCbCr444 and Downsample.
> +			 */
> +			rgb_to_ycbcr = true;
> +			ycbcr444_to_420 = true;
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +
> +	tmp = ycbcr444_to_420 ? DP_CONVERSION_TO_YCBCR420_ENABLE : 0;
>  
>  	if (drm_dp_dpcd_writeb(&intel_dp->aux,
>  			       DP_PROTOCOL_CONVERTER_CONTROL_1, tmp) != 1)
> @@ -2776,13 +2863,12 @@ void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
>  			    "Failed to %s protocol converter YCbCr 4:2:0 conversion mode\n",
>  			    str_enable_disable(intel_dp->dfp.ycbcr_444_to_420));
>  
> -	tmp = intel_dp->dfp.rgb_to_ycbcr ?
> -		DP_CONVERSION_BT709_RGB_YCBCR_ENABLE : 0;
> +	tmp = rgb_to_ycbcr ? DP_CONVERSION_BT709_RGB_YCBCR_ENABLE : 0;
>  
>  	if (drm_dp_pcon_convert_rgb_to_ycbcr(&intel_dp->aux, tmp) < 0)
>  		drm_dbg_kms(&i915->drm,
> -			   "Failed to %s protocol converter RGB->YCbCr conversion mode\n",
> -			   str_enable_disable(tmp));
> +			    "Failed to %s protocol converter RGB->YCbCr conversion mode\n",
> +			    str_enable_disable(tmp));
>  }
>  
>  bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp)
> @@ -4572,57 +4658,47 @@ intel_dp_update_dfp(struct intel_dp *intel_dp,
>  	intel_dp_get_pcon_dsc_cap(intel_dp);
>  }
>  
> +static bool
> +intel_dp_can_ycbcr420(struct intel_dp *intel_dp)
> +{
> +	if (source_can_output(intel_dp, INTEL_OUTPUT_FORMAT_YCBCR420))
> +		return true;

Should have an empty line here.

> +	/*
> +	 * If source cannot support YCbCr420, and PCON has color conv. support:
> +	 * Source sends YCbCr444, PCON converts YCbCr444->420 Or
> +	 * Source sends RGB444, PCON converts RGB->YCbCr444 + YCbCr444->YCbCr420)
> +	 */

I think the code already explains that pretty well. Comment seems
a bit redundant.

> +	if (source_can_output(intel_dp, INTEL_OUTPUT_FORMAT_RGB) &&
> +	    dfp_can_convert_from_rgb(intel_dp, INTEL_OUTPUT_FORMAT_YCBCR420))
> +		return true;
> +
> +	if (source_can_output(intel_dp, INTEL_OUTPUT_FORMAT_YCBCR444) &&
> +	    dfp_can_convert_from_ycbcr444(intel_dp, INTEL_OUTPUT_FORMAT_YCBCR420))
> +		return INTEL_OUTPUT_FORMAT_YCBCR444;
> +
> +	return false;
> +}
> +
>  static void
>  intel_dp_update_420(struct intel_dp *intel_dp)
>  {
>  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>  	struct intel_connector *connector = intel_dp->attached_connector;
> -	bool is_branch, ycbcr_420_passthrough, ycbcr_444_to_420, rgb_to_ycbcr;
> -
> -	/* No YCbCr output support on gmch platforms */
> -	if (HAS_GMCH(i915))
> -		return;
>  
> -	/*
> -	 * ILK doesn't seem capable of DP YCbCr output. The
> -	 * displayed image is severly corrupted. SNB+ is fine.
> -	 */
> -	if (IS_IRONLAKE(i915))
> -		return;
> -
> -	is_branch = drm_dp_is_branch(intel_dp->dpcd);
> -	ycbcr_420_passthrough =
> +	intel_dp->dfp.ycbcr420_passthrough =
>  		drm_dp_downstream_420_passthrough(intel_dp->dpcd,
>  						  intel_dp->downstream_ports);
>  	/* on-board LSPCON always assumed to support 4:4:4->4:2:0 conversion */
> -	ycbcr_444_to_420 =
> +	intel_dp->dfp.ycbcr_444_to_420 =
>  		dp_to_dig_port(intel_dp)->lspcon.active ||
>  		drm_dp_downstream_444_to_420_conversion(intel_dp->dpcd,
>  							intel_dp->downstream_ports);
> -	rgb_to_ycbcr = drm_dp_downstream_rgb_to_ycbcr_conversion(intel_dp->dpcd,
> -								 intel_dp->downstream_ports,
> -								 DP_DS_HDMI_BT709_RGB_YCBCR_CONV);
> -
> -	if (DISPLAY_VER(i915) >= 11) {
> -		/* Let PCON convert from RGB->YCbCr if possible */
> -		if (is_branch && rgb_to_ycbcr && ycbcr_444_to_420) {
> -			intel_dp->dfp.rgb_to_ycbcr = true;
> -			intel_dp->dfp.ycbcr_444_to_420 = true;
> -			connector->base.ycbcr_420_allowed = true;
> -		} else {
> -		/* Prefer 4:2:0 passthrough over 4:4:4->4:2:0 conversion */
> -			intel_dp->dfp.ycbcr_444_to_420 =
> -				ycbcr_444_to_420 && !ycbcr_420_passthrough;
> +	intel_dp->dfp.rgb_to_ycbcr =
> +		drm_dp_downstream_rgb_to_ycbcr_conversion(intel_dp->dpcd,
> +							  intel_dp->downstream_ports,
> +							  DP_DS_HDMI_BT709_RGB_YCBCR_CONV);
>  
> -			connector->base.ycbcr_420_allowed =
> -				!is_branch || ycbcr_444_to_420 || ycbcr_420_passthrough;
> -		}
> -	} else {
> -		/* 4:4:4->4:2:0 conversion is the only way */
> -		intel_dp->dfp.ycbcr_444_to_420 = ycbcr_444_to_420;
> -
> -		connector->base.ycbcr_420_allowed = ycbcr_444_to_420;
> -	}
> +	connector->base.ycbcr_420_allowed = intel_dp_can_ycbcr420(intel_dp);

Looks good. With the minor issues sorted this is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
>  	drm_dbg_kms(&i915->drm,
>  		    "[CONNECTOR:%d:%s] RGB->YcbCr conversion? %s, YCbCr 4:2:0 allowed? %s, YCbCr 4:4:4->4:2:0 conversion? %s\n",
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH v11 01/11] drm/i915/display: Add new member to configure PCON color conversion

[Ville Syrjälä]

On Tue, Mar 14, 2023 at 04:34:05PM +0530, Ankit Nautiyal wrote:
> The decision to use DFP output format conversion capabilities should be
> during compute_config phase.
> 
> This patch adds new member to crtc_state to represent the final
> output_format to the sink. In case of a DFP this can be different than
> the output_format, as per the format conversion done via the PCON.
> 
> This will help to store only the format conversion capabilities of the
> DP device in intel_dp->dfp, and use crtc_state to compute and store the
> configuration for color/format conversion for a given mode.
> 
> v2: modified the new member to crtc_state to represent the final
> output_format that eaches the sink, after possible conversion by
> PCON kind of devices. (Ville)
> 
> v3: Addressed comments from Ville:
> -Added comments to clarify difference between sink_format and
> output_format.
> -Corrected the order of setting sink_format and output_format.
> -Added readout for sink_format in get_pipe_config hooks.
> 
> v4: Set sink_format for intel_sdvo too. (Ville)
> 
> v5: Rebased.
> 
> v6: Fixed condition to go for YCbCr420 format for dp and hdmi. (Ville)
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (v3)
> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c        |  1 +
>  drivers/gpu/drm/i915/display/intel_crt.c      |  1 +
>  .../drm/i915/display/intel_crtc_state_dump.c  |  5 +--
>  drivers/gpu/drm/i915/display/intel_display.c  |  5 +++
>  .../drm/i915/display/intel_display_types.h    | 11 ++++++-
>  drivers/gpu/drm/i915/display/intel_dp.c       | 33 +++++++++++++------
>  drivers/gpu/drm/i915/display/intel_dp_mst.c   |  1 +
>  drivers/gpu/drm/i915/display/intel_dvo.c      |  1 +
>  drivers/gpu/drm/i915/display/intel_hdmi.c     | 23 +++++++------
>  drivers/gpu/drm/i915/display/intel_lvds.c     |  1 +
>  drivers/gpu/drm/i915/display/intel_sdvo.c     |  1 +
>  drivers/gpu/drm/i915/display/intel_tv.c       |  1 +
>  drivers/gpu/drm/i915/display/vlv_dsi.c        |  1 +
>  13 files changed, 62 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 50dcaa895854..e8ac4552e681 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1593,6 +1593,7 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
>  		&pipe_config->hw.adjusted_mode;
>  	int ret;
>  
> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>  	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  
>  	ret = intel_panel_compute_config(intel_connector, adjusted_mode);
> diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
> index 8f2ebead0826..e925e21d87fc 100644
> --- a/drivers/gpu/drm/i915/display/intel_crt.c
> +++ b/drivers/gpu/drm/i915/display/intel_crt.c
> @@ -393,6 +393,7 @@ static int intel_crt_compute_config(struct intel_encoder *encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
>  		return -EINVAL;
>  
> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>  	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  
>  	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 766633566fd6..185cd1971aa5 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> @@ -178,10 +178,11 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
>  
>  	snprintf_output_types(buf, sizeof(buf), pipe_config->output_types);
>  	drm_dbg_kms(&i915->drm,
> -		    "active: %s, output_types: %s (0x%x), output format: %s\n",
> +		    "active: %s, output_types: %s (0x%x), output format: %s, sink format: %s\n",
>  		    str_yes_no(pipe_config->hw.active),
>  		    buf, pipe_config->output_types,
> -		    output_formats(pipe_config->output_format));
> +		    output_formats(pipe_config->output_format),
> +		    output_formats(pipe_config->sink_format));
>  
>  	drm_dbg_kms(&i915->drm,
>  		    "cpu_transcoder: %s, pipe bpp: %i, dithering: %i\n",
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 410c84fd905c..6fa86d1e0480 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -3127,6 +3127,7 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>  		return false;
>  
>  	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
> +	pipe_config->sink_format = pipe_config->output_format;
>  	pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
>  	pipe_config->shared_dpll = NULL;
>  
> @@ -3586,6 +3587,8 @@ static bool ilk_get_pipe_config(struct intel_crtc *crtc,
>  		break;
>  	}
>  
> +	pipe_config->sink_format = pipe_config->output_format;
> +
>  	pipe_config->gamma_mode = REG_FIELD_GET(TRANSCONF_GAMMA_MODE_MASK_ILK, tmp);
>  
>  	pipe_config->framestart_delay = REG_FIELD_GET(TRANSCONF_FRAME_START_DELAY_MASK, tmp) + 1;
> @@ -3984,6 +3987,8 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc,
>  			bdw_get_pipemisc_output_format(crtc);
>  	}
>  
> +	pipe_config->sink_format = pipe_config->output_format;
> +
>  	pipe_config->gamma_mode = intel_de_read(dev_priv,
>  						GAMMA_MODE(crtc->pipe));
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index c32bfba06ca1..c9d1863d2765 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1274,9 +1274,18 @@ struct intel_crtc_state {
>  	/* HDMI High TMDS char rate ratio */
>  	bool hdmi_high_tmds_clock_ratio;
>  
> -	/* Output format RGB/YCBCR etc */
> +	/*
> +	 * Output format RGB/YCBCR etc., that is coming out
> +	 * at the end of the pipe.
> +	 */
>  	enum intel_output_format output_format;
>  
> +	/*
> +	 * Sink output format RGB/YCBCR etc., that is going
> +	 * into the sink.
> +	 */
> +	enum intel_output_format sink_format;
> +
>  	/* enable pipe gamma? */
>  	bool gamma_enable;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 8e16745275f6..e52e2ffc355c 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -819,11 +819,12 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
>  
>  static enum intel_output_format
>  intel_dp_output_format(struct intel_connector *connector,
> -		       bool ycbcr_420_output)
> +		       enum intel_output_format sink_format)
>  {
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
>  
> -	if (!connector->base.ycbcr_420_allowed || !ycbcr_420_output)
> +	if (!connector->base.ycbcr_420_allowed ||
> +	    sink_format != INTEL_OUTPUT_FORMAT_YCBCR420)
>  		return INTEL_OUTPUT_FORMAT_RGB;
>  
>  	if (intel_dp->dfp.rgb_to_ycbcr &&
> @@ -862,8 +863,14 @@ intel_dp_mode_min_output_bpp(struct intel_connector *connector,
>  			     const struct drm_display_mode *mode)
>  {
>  	const struct drm_display_info *info = &connector->base.display_info;
> -	enum intel_output_format output_format =
> -		intel_dp_output_format(connector, drm_mode_is_420_only(info, mode));
> +	enum intel_output_format output_format, sink_format;
> +
> +	if (drm_mode_is_420_only(info, mode))
> +		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> +	else
> +		sink_format = INTEL_OUTPUT_FORMAT_RGB;
> +
> +	output_format = intel_dp_output_format(connector, sink_format);
>  
>  	return intel_dp_output_bpp(output_format, intel_dp_min_bpp(output_format));
>  }
> @@ -2040,23 +2047,29 @@ intel_dp_compute_output_format(struct intel_encoder *encoder,
>  
>  	ycbcr_420_only = drm_mode_is_420_only(info, adjusted_mode);
>  
> -	crtc_state->output_format = intel_dp_output_format(connector, ycbcr_420_only);
> -
> -	if (ycbcr_420_only && !intel_dp_is_ycbcr420(intel_dp, crtc_state)) {
> +	if (ycbcr_420_only && !connector->base.ycbcr_420_allowed) {
>  		drm_dbg_kms(&i915->drm,
>  			    "YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible. Falling back to RGB.\n");
> -		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
> +	} else if (ycbcr_420_only) {
> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> +	} else {
> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>  	}
>  
> +	crtc_state->output_format = intel_dp_output_format(connector, crtc_state->sink_format);
> +
>  	ret = intel_dp_compute_link_config(encoder, crtc_state, conn_state,
>  					   respect_downstream_limits);
>  	if (ret) {
> -		if (intel_dp_is_ycbcr420(intel_dp, crtc_state) ||
> +		if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
>  		    !connector->base.ycbcr_420_allowed ||
>  		    !drm_mode_is_420_also(info, adjusted_mode))
>  			return ret;
>  
> -		crtc_state->output_format = intel_dp_output_format(connector, true);
> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> +		crtc_state->output_format = intel_dp_output_format(connector,
> +								   crtc_state->sink_format);
>  		ret = intel_dp_compute_link_config(encoder, crtc_state, conn_state,
>  						   respect_downstream_limits);
>  	}
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index a860cbc5dbea..ff0b821a901a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -293,6 +293,7 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
>  		return -EINVAL;
>  
> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>  	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  	pipe_config->has_pch_encoder = false;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dvo.c b/drivers/gpu/drm/i915/display/intel_dvo.c
> index eb2dcd866cc8..9884678743b6 100644
> --- a/drivers/gpu/drm/i915/display/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_dvo.c
> @@ -271,6 +271,7 @@ static int intel_dvo_compute_config(struct intel_encoder *encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
>  		return -EINVAL;
>  
> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>  	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index c7e9e1fbed37..1ad0540c13ee 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -2171,13 +2171,13 @@ static bool intel_hdmi_has_audio(struct intel_encoder *encoder,
>  
>  static enum intel_output_format
>  intel_hdmi_output_format(const struct intel_crtc_state *crtc_state,
> -			 struct intel_connector *connector,
> -			 bool ycbcr_420_output)
> +			 struct intel_connector *connector)
>  {
>  	if (!crtc_state->has_hdmi_sink)
>  		return INTEL_OUTPUT_FORMAT_RGB;

I think we need to move this into the sink_format selection since
we can't have sink_format!=output_format with HDMI.

>  
> -	if (connector->base.ycbcr_420_allowed && ycbcr_420_output)
> +	if (connector->base.ycbcr_420_allowed &&

Similarly the 420_allowed check here doesn't seem correct.

In fact I guess what we need is simply:
 intel_hdmi_output_format()
 {
 	return sink_format;
 }

> +	    crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
>  		return INTEL_OUTPUT_FORMAT_YCBCR420;
>  	else
>  		return INTEL_OUTPUT_FORMAT_RGB;
> @@ -2195,23 +2195,26 @@ static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
>  	bool ycbcr_420_only = drm_mode_is_420_only(info, adjusted_mode);
>  	int ret;
>  
> -	crtc_state->output_format =
> -		intel_hdmi_output_format(crtc_state, connector, ycbcr_420_only);
> -
> -	if (ycbcr_420_only && !intel_hdmi_is_ycbcr420(crtc_state)) {
> +	if (ycbcr_420_only && !connector->base.ycbcr_420_allowed) {
>  		drm_dbg_kms(&i915->drm,
>  			    "YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible. Falling back to RGB.\n");
> -		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
> +	} else if (ycbcr_420_only) {
> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> +	} else {
> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>  	}
>  
> +	crtc_state->output_format = intel_hdmi_output_format(crtc_state, connector);
>  	ret = intel_hdmi_compute_clock(encoder, crtc_state, respect_downstream_limits);
>  	if (ret) {
> -		if (intel_hdmi_is_ycbcr420(crtc_state) ||
> +		if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||

With intel_hdmi_output_format() adjusted as mentined above this will
also need an explicit has_hdmi_sink check.

>  		    !connector->base.ycbcr_420_allowed ||
>  		    !drm_mode_is_420_also(info, adjusted_mode))
>  			return ret;
>  
> -		crtc_state->output_format = intel_hdmi_output_format(crtc_state, connector, true);
> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> +		crtc_state->output_format = intel_hdmi_output_format(crtc_state, connector);
>  		ret = intel_hdmi_compute_clock(encoder, crtc_state, respect_downstream_limits);
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
> index a504b3a7fbd5..a7783da37dfd 100644
> --- a/drivers/gpu/drm/i915/display/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/display/intel_lvds.c
> @@ -436,6 +436,7 @@ static int intel_lvds_compute_config(struct intel_encoder *encoder,
>  		crtc_state->pipe_bpp = lvds_bpp;
>  	}
>  
> +	crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>  	crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  
>  	/*
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index e12ba458636c..34ee9dd82a78 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -1351,6 +1351,7 @@ static int intel_sdvo_compute_config(struct intel_encoder *encoder,
>  
>  	DRM_DEBUG_KMS("forcing bpc to 8 for SDVO\n");
>  	pipe_config->pipe_bpp = 8*3;
> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>  	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  
>  	if (HAS_PCH_SPLIT(to_i915(encoder->base.dev)))
> diff --git a/drivers/gpu/drm/i915/display/intel_tv.c b/drivers/gpu/drm/i915/display/intel_tv.c
> index 3b5ff84dc615..6f7ac225293e 100644
> --- a/drivers/gpu/drm/i915/display/intel_tv.c
> +++ b/drivers/gpu/drm/i915/display/intel_tv.c
> @@ -1204,6 +1204,7 @@ intel_tv_compute_config(struct intel_encoder *encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
>  		return -EINVAL;
>  
> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>  	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  
>  	drm_dbg_kms(&dev_priv->drm, "forcing bpc to 8 for TV\n");
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index 8d2e6e151ba0..82c30feb7a91 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -280,6 +280,7 @@ static int intel_dsi_compute_config(struct intel_encoder *encoder,
>  	int ret;
>  
>  	drm_dbg_kms(&dev_priv->drm, "\n");
> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>  	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  
>  	ret = intel_panel_compute_config(intel_connector, adjusted_mode);
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH v11 04/11] drm/i915/display: Use sink_format instead of ycbcr420_output flag

[Ville Syrjälä]

On Tue, Mar 14, 2023 at 04:34:08PM +0530, Ankit Nautiyal wrote:
> Start passing the sink_format, to all functions that take a bool
> ycbcr420_output as parameter. This will make the functions generic,
> and will serve as a slight step towards 4:2:2 support later.
> 
> v2: Rebased.
> 
> Suggested-by: Ville Syrj_l_ <ville.syrjala@linux.intel.com>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c   | 33 +++++++--------
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 50 ++++++++++++-----------
>  drivers/gpu/drm/i915/display/intel_hdmi.h |  5 ++-
>  3 files changed, 44 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index c31ec9c91c64..b49d113357e4 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -967,7 +967,8 @@ static int intel_dp_max_tmds_clock(struct intel_dp *intel_dp)
>  
>  static enum drm_mode_status
>  intel_dp_tmds_clock_valid(struct intel_dp *intel_dp,
> -			  int clock, int bpc, bool ycbcr420_output,
> +			  int clock, int bpc,
> +			  enum intel_output_format sink_format,
>  			  bool respect_downstream_limits)
>  {
>  	int tmds_clock, min_tmds_clock, max_tmds_clock;
> @@ -975,7 +976,7 @@ intel_dp_tmds_clock_valid(struct intel_dp *intel_dp,
>  	if (!respect_downstream_limits)
>  		return MODE_OK;
>  
> -	tmds_clock = intel_hdmi_tmds_clock(clock, bpc, ycbcr420_output);
> +	tmds_clock = intel_hdmi_tmds_clock(clock, bpc, sink_format);
>  
>  	min_tmds_clock = intel_dp->dfp.min_tmds_clock;
>  	max_tmds_clock = intel_dp_max_tmds_clock(intel_dp);
> @@ -998,6 +999,7 @@ intel_dp_mode_valid_downstream(struct intel_connector *connector,
>  	const struct drm_display_info *info = &connector->base.display_info;
>  	enum drm_mode_status status;
>  	bool ycbcr_420_only;
> +	enum intel_output_format sink_format;
>  
>  	/* If PCON supports FRL MODE, check FRL bandwidth constraints */
>  	if (intel_dp->dfp.pcon_max_frl_bw) {
> @@ -1024,18 +1026,22 @@ intel_dp_mode_valid_downstream(struct intel_connector *connector,
>  
>  	ycbcr_420_only = drm_mode_is_420_only(info, mode);
>  
> +	if (ycbcr_420_only && connector->base.ycbcr_420_allowed)

drm_mode_validate_ycbcr420() already checks for this combination.
So I don't think we should need to redo it here, and we didn't
before.

> +		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> +	else
> +		sink_format = INTEL_OUTPUT_FORMAT_RGB;
> +
>  	/* Assume 8bpc for the DP++/HDMI/DVI TMDS clock check */
>  	status = intel_dp_tmds_clock_valid(intel_dp, target_clock,
> -					   8, ycbcr_420_only, true);
> +					   8, sink_format, true);
>  
>  	if (status != MODE_OK) {
> -		if (ycbcr_420_only ||
> -		    !connector->base.ycbcr_420_allowed ||

But here we do need to keep the allowed flag check since
drm_mode_validate_ycbcr420() doesn't care about 420_also() modes.

> +		if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
>  		    !drm_mode_is_420_also(info, mode))
>  			return status;
> -
> +		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>  		status = intel_dp_tmds_clock_valid(intel_dp, target_clock,
> -						   8, true, true);
> +						   8, sink_format, true);
>  		if (status != MODE_OK)
>  			return status;
>  	}
> @@ -1271,19 +1277,10 @@ static bool intel_dp_supports_dsc(struct intel_dp *intel_dp,
>  		drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd);
>  }
>  
> -static bool intel_dp_is_ycbcr420(struct intel_dp *intel_dp,
> -				 const struct intel_crtc_state *crtc_state)
> -{
> -	return crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
> -		(crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444 &&
> -		 intel_dp->dfp.ycbcr_444_to_420);
> -}
> -
>  static int intel_dp_hdmi_compute_bpc(struct intel_dp *intel_dp,
>  				     const struct intel_crtc_state *crtc_state,
>  				     int bpc, bool respect_downstream_limits)
>  {
> -	bool ycbcr420_output = intel_dp_is_ycbcr420(intel_dp, crtc_state);
>  	int clock = crtc_state->hw.adjusted_mode.crtc_clock;
>  
>  	/*
> @@ -1303,8 +1300,8 @@ static int intel_dp_hdmi_compute_bpc(struct intel_dp *intel_dp,
>  
>  	for (; bpc >= 8; bpc -= 2) {
>  		if (intel_hdmi_bpc_possible(crtc_state, bpc,
> -					    intel_dp->has_hdmi_sink, ycbcr420_output) &&
> -		    intel_dp_tmds_clock_valid(intel_dp, clock, bpc, ycbcr420_output,
> +					    intel_dp->has_hdmi_sink) &&
> +		    intel_dp_tmds_clock_valid(intel_dp, clock, bpc, crtc_state->sink_format,
>  					      respect_downstream_limits) == MODE_OK)
>  			return bpc;
>  	}
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 1ad0540c13ee..15bf64a217c2 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -1793,11 +1793,6 @@ static bool intel_has_hdmi_sink(struct intel_hdmi *hdmi,
>  		READ_ONCE(to_intel_digital_connector_state(conn_state)->force_audio) != HDMI_AUDIO_OFF_DVI;
>  }
>  
> -static bool intel_hdmi_is_ycbcr420(const struct intel_crtc_state *crtc_state)
> -{
> -	return crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420;
> -}
> -
>  static int hdmi_port_clock_limit(struct intel_hdmi *hdmi,
>  				 bool respect_downstream_limits,
>  				 bool has_hdmi_sink)
> @@ -1871,10 +1866,11 @@ hdmi_port_clock_valid(struct intel_hdmi *hdmi,
>  	return MODE_OK;
>  }
>  
> -int intel_hdmi_tmds_clock(int clock, int bpc, bool ycbcr420_output)
> +int intel_hdmi_tmds_clock(int clock, int bpc,
> +			  enum intel_output_format sink_format)
>  {
>  	/* YCBCR420 TMDS rate requirement is half the pixel clock */
> -	if (ycbcr420_output)
> +	if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
>  		clock /= 2;
>  
>  	/*
> @@ -1901,7 +1897,8 @@ static bool intel_hdmi_source_bpc_possible(struct drm_i915_private *i915, int bp
>  }
>  
>  static bool intel_hdmi_sink_bpc_possible(struct drm_connector *connector,
> -					 int bpc, bool has_hdmi_sink, bool ycbcr420_output)
> +					 int bpc, bool has_hdmi_sink,
> +					 enum intel_output_format sink_format)
>  {
>  	const struct drm_display_info *info = &connector->display_info;
>  	const struct drm_hdmi_info *hdmi = &info->hdmi;
> @@ -1911,7 +1908,7 @@ static bool intel_hdmi_sink_bpc_possible(struct drm_connector *connector,
>  		if (!has_hdmi_sink)
>  			return false;
>  
> -		if (ycbcr420_output)
> +		if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
>  			return hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_36;
>  		else
>  			return info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_36;
> @@ -1919,7 +1916,7 @@ static bool intel_hdmi_sink_bpc_possible(struct drm_connector *connector,
>  		if (!has_hdmi_sink)
>  			return false;
>  
> -		if (ycbcr420_output)
> +		if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
>  			return hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_30;
>  		else
>  			return info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_30;
> @@ -1933,7 +1930,8 @@ static bool intel_hdmi_sink_bpc_possible(struct drm_connector *connector,
>  
>  static enum drm_mode_status
>  intel_hdmi_mode_clock_valid(struct drm_connector *connector, int clock,
> -			    bool has_hdmi_sink, bool ycbcr420_output)
> +			    bool has_hdmi_sink,
> +			    enum intel_output_format sink_format)
>  {
>  	struct drm_i915_private *i915 = to_i915(connector->dev);
>  	struct intel_hdmi *hdmi = intel_attached_hdmi(to_intel_connector(connector));
> @@ -1946,12 +1944,12 @@ intel_hdmi_mode_clock_valid(struct drm_connector *connector, int clock,
>  	 * least one color depth is accepted.
>  	 */
>  	for (bpc = 12; bpc >= 8; bpc -= 2) {
> -		int tmds_clock = intel_hdmi_tmds_clock(clock, bpc, ycbcr420_output);
> +		int tmds_clock = intel_hdmi_tmds_clock(clock, bpc, sink_format);
>  
>  		if (!intel_hdmi_source_bpc_possible(i915, bpc))
>  			continue;
>  
> -		if (!intel_hdmi_sink_bpc_possible(connector, bpc, has_hdmi_sink, ycbcr420_output))
> +		if (!intel_hdmi_sink_bpc_possible(connector, bpc, has_hdmi_sink, sink_format))
>  			continue;
>  
>  		status = hdmi_port_clock_valid(hdmi, tmds_clock, true, has_hdmi_sink);
> @@ -1976,6 +1974,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>  	int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
>  	bool has_hdmi_sink = intel_has_hdmi_sink(hdmi, connector->state);
>  	bool ycbcr_420_only;
> +	enum intel_output_format sink_format;
>  
>  	if ((mode->flags & DRM_MODE_FLAG_3D_MASK) == DRM_MODE_FLAG_3D_FRAME_PACKING)
>  		clock *= 2;
> @@ -2000,14 +1999,17 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>  
>  	ycbcr_420_only = drm_mode_is_420_only(&connector->display_info, mode);
>  
> -	status = intel_hdmi_mode_clock_valid(connector, clock, has_hdmi_sink, ycbcr_420_only);
> +	sink_format = ycbcr_420_only ? INTEL_OUTPUT_FORMAT_YCBCR420 : INTEL_OUTPUT_FORMAT_RGB;
> +

Please try to stick to common style between here and
intel_dp_mode_valid_downstream() to avoid a some of the
head scratching when comparing the two.

> +	status = intel_hdmi_mode_clock_valid(connector, clock, has_hdmi_sink, sink_format);
>  	if (status != MODE_OK) {
>  		if (ycbcr_420_only ||
>  		    !connector->ycbcr_420_allowed ||
>  		    !drm_mode_is_420_also(&connector->display_info, mode))
>  			return status;
>  
> -		status = intel_hdmi_mode_clock_valid(connector, clock, has_hdmi_sink, true);
> +		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> +		status = intel_hdmi_mode_clock_valid(connector, clock, has_hdmi_sink, sink_format);
>  		if (status != MODE_OK)
>  			return status;
>  	}
> @@ -2016,7 +2018,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>  }
>  
>  bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state,
> -			     int bpc, bool has_hdmi_sink, bool ycbcr420_output)
> +			     int bpc, bool has_hdmi_sink)
>  {
>  	struct drm_atomic_state *state = crtc_state->uapi.state;
>  	struct drm_connector_state *connector_state;
> @@ -2027,7 +2029,8 @@ bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state,
>  		if (connector_state->crtc != crtc_state->uapi.crtc)
>  			continue;
>  
> -		if (!intel_hdmi_sink_bpc_possible(connector, bpc, has_hdmi_sink, ycbcr420_output))
> +		if (!intel_hdmi_sink_bpc_possible(connector, bpc, has_hdmi_sink,
> +						  crtc_state->sink_format))
>  			return false;
>  	}
>  
> @@ -2045,14 +2048,13 @@ static bool hdmi_bpc_possible(const struct intel_crtc_state *crtc_state, int bpc
>  		return false;
>  
>  	/* Display Wa_1405510057:icl,ehl */
> -	if (intel_hdmi_is_ycbcr420(crtc_state) &&
> +	if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
>  	    bpc == 10 && DISPLAY_VER(dev_priv) == 11 &&
>  	    (adjusted_mode->crtc_hblank_end -
>  	     adjusted_mode->crtc_hblank_start) % 8 == 2)
>  		return false;
>  
> -	return intel_hdmi_bpc_possible(crtc_state, bpc, crtc_state->has_hdmi_sink,
> -				       intel_hdmi_is_ycbcr420(crtc_state));
> +	return intel_hdmi_bpc_possible(crtc_state, bpc, crtc_state->has_hdmi_sink);
>  }
>  
>  static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
> @@ -2060,7 +2062,6 @@ static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
>  				  int clock, bool respect_downstream_limits)
>  {
>  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> -	bool ycbcr420_output = intel_hdmi_is_ycbcr420(crtc_state);
>  	int bpc;
>  
>  	/*
> @@ -2078,7 +2079,8 @@ static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
>  		bpc = 8;
>  
>  	for (; bpc >= 8; bpc -= 2) {
> -		int tmds_clock = intel_hdmi_tmds_clock(clock, bpc, ycbcr420_output);
> +		int tmds_clock = intel_hdmi_tmds_clock(clock, bpc,
> +						       crtc_state->sink_format);
>  
>  		if (hdmi_bpc_possible(crtc_state, bpc) &&
>  		    hdmi_port_clock_valid(intel_hdmi, tmds_clock,
> @@ -2108,7 +2110,7 @@ static int intel_hdmi_compute_clock(struct intel_encoder *encoder,
>  		return bpc;
>  
>  	crtc_state->port_clock =
> -		intel_hdmi_tmds_clock(clock, bpc, intel_hdmi_is_ycbcr420(crtc_state));
> +		intel_hdmi_tmds_clock(clock, bpc, crtc_state->sink_format);
>  
>  	/*
>  	 * pipe_bpp could already be below 8bpc due to
> @@ -2293,7 +2295,7 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
>  		return ret;
>  	}
>  
> -	if (intel_hdmi_is_ycbcr420(pipe_config)) {
> +	if (pipe_config->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420) {

While the sink_format vs. output_format distinction doesn't really
matter for HDMI it might be clearer to use output_format at
least in places like this where we're clearly 100% concerned with
what comes out the end of the pipe. That is also what the DP
counterpart code checks here.

For a lot of the other cases the in the HDMI code it's a lot less
clear currently which is the more appropriate choice, and maybe
in places neither is entirely correct with the way it gets used
currently.

>  		ret = intel_panel_fitting(pipe_config, conn_state);
>  		if (ret)
>  			return ret;
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.h b/drivers/gpu/drm/i915/display/intel_hdmi.h
> index 774dda2376ed..d1e27247b657 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.h
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.h
> @@ -9,6 +9,7 @@
>  #include <linux/types.h>
>  
>  enum hdmi_infoframe_type;
> +enum intel_output_format;
>  enum port;
>  struct drm_connector;
>  struct drm_connector_state;
> @@ -45,8 +46,8 @@ void intel_read_infoframe(struct intel_encoder *encoder,
>  bool intel_hdmi_limited_color_range(const struct intel_crtc_state *crtc_state,
>  				    const struct drm_connector_state *conn_state);
>  bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state,
> -			     int bpc, bool has_hdmi_sink, bool ycbcr420_output);
> -int intel_hdmi_tmds_clock(int clock, int bpc, bool ycbcr420_output);
> +			     int bpc, bool has_hdmi_sink);
> +int intel_hdmi_tmds_clock(int clock, int bpc, enum intel_output_format sink_format);
>  int intel_hdmi_dsc_get_bpp(int src_fractional_bpp, int slice_width,
>  			   int num_slices, int output_format, bool hdmi_all_bpp,
>  			   int hdmi_max_chunk_bytes);
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH v11 05/11] drm/i915/dp: Rearrange check for illegal mode and comments in mode_valid

[Ville Syrjälä]

On Tue, Mar 14, 2023 at 04:34:09PM +0530, Ankit Nautiyal wrote:
> Check for MODE_H_ILLEGAL before calculating max rates, lanes etc.
> Move comments about compressed bpp U6.4 format closer to where it is used.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

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

> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index b49d113357e4..dcb3c2519041 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1098,6 +1098,9 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>  	if (target_clock > max_dotclk)
>  		return MODE_CLOCK_HIGH;
>  
> +	if (intel_dp_hdisplay_bad(dev_priv, mode->hdisplay))
> +		return MODE_H_ILLEGAL;
> +
>  	max_link_clock = intel_dp_max_link_rate(intel_dp);
>  	max_lanes = intel_dp_max_lane_count(intel_dp);
>  
> @@ -1105,13 +1108,6 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>  	mode_rate = intel_dp_link_required(target_clock,
>  					   intel_dp_mode_min_output_bpp(connector, mode));
>  
> -	if (intel_dp_hdisplay_bad(dev_priv, mode->hdisplay))
> -		return MODE_H_ILLEGAL;
> -
> -	/*
> -	 * Output bpp is stored in 6.4 format so right shift by 4 to get the
> -	 * integer value since we support only integer values of bpp.
> -	 */
>  	if (HAS_DSC(dev_priv) &&
>  	    drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)) {
>  		/*
> @@ -1120,6 +1116,10 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>  		 */
>  		int pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, U8_MAX);
>  
> +		/*
> +		 * Output bpp is stored in 6.4 format so right shift by 4 to get the
> +		 * integer value since we support only integer values of bpp.
> +		 */
>  		if (intel_dp_is_edp(intel_dp)) {
>  			dsc_max_output_bpp =
>  				drm_edp_dsc_sink_output_bpp(intel_dp->dsc_dpcd) >> 4;
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH v11 06/11] drm/i915/dp: Consider output_format while computing dsc bpp for mode_valid

[Ville Syrjälä]

On Tue, Mar 14, 2023 at 04:34:10PM +0530, Ankit Nautiyal wrote:
> During modevalid step, the pipe bpp is computed assuming RGB output
> format. When checking with DSC, consider the output_format and compute
> the input bpp for DSC appropriately.
> 
> v2: For DP-MST we currently use RGB output format only, so continue
> using RGB while computing dsc_bpp for MST case.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c     | 28 ++++++++++++++++-----
>  drivers/gpu/drm/i915/display/intel_dp.h     |  4 ++-
>  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 +-
>  3 files changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index dcb3c2519041..499390c519ca 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1110,11 +1110,21 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>  
>  	if (HAS_DSC(dev_priv) &&
>  	    drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)) {
> +		int pipe_bpp;
> +		enum intel_output_format output_format, sink_format;
> +		const struct drm_display_info *info = &connector->base.display_info;
> +
> +		if (drm_mode_is_420_only(info, mode))
> +			sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> +		else
> +			sink_format = INTEL_OUTPUT_FORMAT_RGB;

I think I saw this same code duplicated somewhere else already.
Time for a intel_dp_sink_format()?

> +
> +		output_format = intel_dp_output_format(connector, sink_format);
>  		/*
>  		 * 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);
> +		pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, output_format, U8_MAX);
>  
>  		/*
>  		 * Output bpp is stored in 6.4 format so right shift by 4 to get the
> @@ -1454,12 +1464,15 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
>  	return -EINVAL;
>  }
>  
> -int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8 max_req_bpc)
> +int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp,
> +			     enum intel_output_format output_format,
> +			     u8 max_req_bpc)
>  {
>  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>  	int i, num_bpc;
>  	u8 dsc_bpc[3] = {0};
>  	u8 dsc_max_bpc;
> +	int pipe_bpp = 0;
>  
>  	/* Max DSC Input BPC for ICL is 10 and for TGL+ is 12 */
>  	if (DISPLAY_VER(i915) >= 12)
> @@ -1470,11 +1483,13 @@ int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8 max_req_bpc)
>  	num_bpc = drm_dp_dsc_sink_supported_input_bpcs(intel_dp->dsc_dpcd,
>  						       dsc_bpc);
>  	for (i = 0; i < num_bpc; i++) {
> -		if (dsc_max_bpc >= dsc_bpc[i])
> -			return dsc_bpc[i] * 3;
> +		if (dsc_max_bpc >= dsc_bpc[i]) {
> +			pipe_bpp = dsc_bpc[i] * 3;
> +			break;
> +		}
>  	}
>  
> -	return 0;
> +	return intel_dp_output_bpp(output_format, pipe_bpp);

The pipe_bpp vs. output_bpp terms seem a bit confused now in the dsc
code.

In the non-compressed cases pipe_bpp does not include any
subsampling, output_bpp is the subsampled version.

What this dsc code seems to want is an intermediate value which
is the subsampled pipe_bpp that is the input to dsc compressor?
And output_bpp/dsc.compressed_bpp is then the final bpp coming
out of the compressor.

I think we should invent a consistent set of names for each so that
it's clear which value the code is concerned with.

>  }
>  
>  static int intel_dp_source_dsc_version_minor(struct intel_dp *intel_dp)
> @@ -1588,7 +1603,8 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>  		return -EINVAL;
>  
>  	if (compute_pipe_bpp)
> -		pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, conn_state->max_requested_bpc);
> +		pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, pipe_config->output_format,
> +						    conn_state->max_requested_bpc);

This pipe_bpp gets plugged back into crtc_state->pipe_bpp later and
then it'll be the subsampled version. I don't think that is what we want
eg. for dithering setup and whatnot.

>  	else
>  		pipe_bpp = pipe_config->pipe_bpp;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index ef39e4f7a329..2f4136e43f38 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -102,7 +102,9 @@ void intel_read_dp_sdp(struct intel_encoder *encoder,
>  		       struct intel_crtc_state *crtc_state,
>  		       unsigned int type);
>  bool intel_digital_port_connected(struct intel_encoder *encoder);
> -int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8 dsc_max_bpc);
> +int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp,
> +			     enum intel_output_format output_format,
> +			     u8 dsc_max_bpc);
>  u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
>  				u32 link_clock, u32 lane_count,
>  				u32 mode_clock, u32 mode_hdisplay,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index ff0b821a901a..bdc5c53ccd75 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -917,7 +917,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
>  		 * 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);
> +		int pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, INTEL_OUTPUT_FORMAT_RGB, U8_MAX);
>  
>  		if (drm_dp_sink_supports_fec(intel_dp->fec_capable)) {
>  			dsc_max_output_bpp =
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH v11 01/11] drm/i915/display: Add new member to configure PCON color conversion

[Nautiyal, Ankit K]

On 3/17/2023 5:54 AM, Ville Syrjälä wrote:
> On Tue, Mar 14, 2023 at 04:34:05PM +0530, Ankit Nautiyal wrote:
>> The decision to use DFP output format conversion capabilities should be
>> during compute_config phase.
>>
>> This patch adds new member to crtc_state to represent the final
>> output_format to the sink. In case of a DFP this can be different than
>> the output_format, as per the format conversion done via the PCON.
>>
>> This will help to store only the format conversion capabilities of the
>> DP device in intel_dp->dfp, and use crtc_state to compute and store the
>> configuration for color/format conversion for a given mode.
>>
>> v2: modified the new member to crtc_state to represent the final
>> output_format that eaches the sink, after possible conversion by
>> PCON kind of devices. (Ville)
>>
>> v3: Addressed comments from Ville:
>> -Added comments to clarify difference between sink_format and
>> output_format.
>> -Corrected the order of setting sink_format and output_format.
>> -Added readout for sink_format in get_pipe_config hooks.
>>
>> v4: Set sink_format for intel_sdvo too. (Ville)
>>
>> v5: Rebased.
>>
>> v6: Fixed condition to go for YCbCr420 format for dp and hdmi. (Ville)
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (v3)
>> ---
>>   drivers/gpu/drm/i915/display/icl_dsi.c        |  1 +
>>   drivers/gpu/drm/i915/display/intel_crt.c      |  1 +
>>   .../drm/i915/display/intel_crtc_state_dump.c  |  5 +--
>>   drivers/gpu/drm/i915/display/intel_display.c  |  5 +++
>>   .../drm/i915/display/intel_display_types.h    | 11 ++++++-
>>   drivers/gpu/drm/i915/display/intel_dp.c       | 33 +++++++++++++------
>>   drivers/gpu/drm/i915/display/intel_dp_mst.c   |  1 +
>>   drivers/gpu/drm/i915/display/intel_dvo.c      |  1 +
>>   drivers/gpu/drm/i915/display/intel_hdmi.c     | 23 +++++++------
>>   drivers/gpu/drm/i915/display/intel_lvds.c     |  1 +
>>   drivers/gpu/drm/i915/display/intel_sdvo.c     |  1 +
>>   drivers/gpu/drm/i915/display/intel_tv.c       |  1 +
>>   drivers/gpu/drm/i915/display/vlv_dsi.c        |  1 +
>>   13 files changed, 62 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
>> index 50dcaa895854..e8ac4552e681 100644
>> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
>> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
>> @@ -1593,6 +1593,7 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
>>   		&pipe_config->hw.adjusted_mode;
>>   	int ret;
>>   
>> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>>   	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>>   
>>   	ret = intel_panel_compute_config(intel_connector, adjusted_mode);
>> diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
>> index 8f2ebead0826..e925e21d87fc 100644
>> --- a/drivers/gpu/drm/i915/display/intel_crt.c
>> +++ b/drivers/gpu/drm/i915/display/intel_crt.c
>> @@ -393,6 +393,7 @@ static int intel_crt_compute_config(struct intel_encoder *encoder,
>>   	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
>>   		return -EINVAL;
>>   
>> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>>   	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>>   
>>   	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 766633566fd6..185cd1971aa5 100644
>> --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
>> +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
>> @@ -178,10 +178,11 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
>>   
>>   	snprintf_output_types(buf, sizeof(buf), pipe_config->output_types);
>>   	drm_dbg_kms(&i915->drm,
>> -		    "active: %s, output_types: %s (0x%x), output format: %s\n",
>> +		    "active: %s, output_types: %s (0x%x), output format: %s, sink format: %s\n",
>>   		    str_yes_no(pipe_config->hw.active),
>>   		    buf, pipe_config->output_types,
>> -		    output_formats(pipe_config->output_format));
>> +		    output_formats(pipe_config->output_format),
>> +		    output_formats(pipe_config->sink_format));
>>   
>>   	drm_dbg_kms(&i915->drm,
>>   		    "cpu_transcoder: %s, pipe bpp: %i, dithering: %i\n",
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 410c84fd905c..6fa86d1e0480 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -3127,6 +3127,7 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>>   		return false;
>>   
>>   	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>> +	pipe_config->sink_format = pipe_config->output_format;
>>   	pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
>>   	pipe_config->shared_dpll = NULL;
>>   
>> @@ -3586,6 +3587,8 @@ static bool ilk_get_pipe_config(struct intel_crtc *crtc,
>>   		break;
>>   	}
>>   
>> +	pipe_config->sink_format = pipe_config->output_format;
>> +
>>   	pipe_config->gamma_mode = REG_FIELD_GET(TRANSCONF_GAMMA_MODE_MASK_ILK, tmp);
>>   
>>   	pipe_config->framestart_delay = REG_FIELD_GET(TRANSCONF_FRAME_START_DELAY_MASK, tmp) + 1;
>> @@ -3984,6 +3987,8 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc,
>>   			bdw_get_pipemisc_output_format(crtc);
>>   	}
>>   
>> +	pipe_config->sink_format = pipe_config->output_format;
>> +
>>   	pipe_config->gamma_mode = intel_de_read(dev_priv,
>>   						GAMMA_MODE(crtc->pipe));
>>   
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index c32bfba06ca1..c9d1863d2765 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -1274,9 +1274,18 @@ struct intel_crtc_state {
>>   	/* HDMI High TMDS char rate ratio */
>>   	bool hdmi_high_tmds_clock_ratio;
>>   
>> -	/* Output format RGB/YCBCR etc */
>> +	/*
>> +	 * Output format RGB/YCBCR etc., that is coming out
>> +	 * at the end of the pipe.
>> +	 */
>>   	enum intel_output_format output_format;
>>   
>> +	/*
>> +	 * Sink output format RGB/YCBCR etc., that is going
>> +	 * into the sink.
>> +	 */
>> +	enum intel_output_format sink_format;
>> +
>>   	/* enable pipe gamma? */
>>   	bool gamma_enable;
>>   
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 8e16745275f6..e52e2ffc355c 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -819,11 +819,12 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
>>   
>>   static enum intel_output_format
>>   intel_dp_output_format(struct intel_connector *connector,
>> -		       bool ycbcr_420_output)
>> +		       enum intel_output_format sink_format)
>>   {
>>   	struct intel_dp *intel_dp = intel_attached_dp(connector);
>>   
>> -	if (!connector->base.ycbcr_420_allowed || !ycbcr_420_output)
>> +	if (!connector->base.ycbcr_420_allowed ||
>> +	    sink_format != INTEL_OUTPUT_FORMAT_YCBCR420)
>>   		return INTEL_OUTPUT_FORMAT_RGB;
>>   
>>   	if (intel_dp->dfp.rgb_to_ycbcr &&
>> @@ -862,8 +863,14 @@ intel_dp_mode_min_output_bpp(struct intel_connector *connector,
>>   			     const struct drm_display_mode *mode)
>>   {
>>   	const struct drm_display_info *info = &connector->base.display_info;
>> -	enum intel_output_format output_format =
>> -		intel_dp_output_format(connector, drm_mode_is_420_only(info, mode));
>> +	enum intel_output_format output_format, sink_format;
>> +
>> +	if (drm_mode_is_420_only(info, mode))
>> +		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>> +	else
>> +		sink_format = INTEL_OUTPUT_FORMAT_RGB;
>> +
>> +	output_format = intel_dp_output_format(connector, sink_format);
>>   
>>   	return intel_dp_output_bpp(output_format, intel_dp_min_bpp(output_format));
>>   }
>> @@ -2040,23 +2047,29 @@ intel_dp_compute_output_format(struct intel_encoder *encoder,
>>   
>>   	ycbcr_420_only = drm_mode_is_420_only(info, adjusted_mode);
>>   
>> -	crtc_state->output_format = intel_dp_output_format(connector, ycbcr_420_only);
>> -
>> -	if (ycbcr_420_only && !intel_dp_is_ycbcr420(intel_dp, crtc_state)) {
>> +	if (ycbcr_420_only && !connector->base.ycbcr_420_allowed) {
>>   		drm_dbg_kms(&i915->drm,
>>   			    "YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible. Falling back to RGB.\n");
>> -		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
>> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>> +	} else if (ycbcr_420_only) {
>> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>> +	} else {
>> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>>   	}
>>   
>> +	crtc_state->output_format = intel_dp_output_format(connector, crtc_state->sink_format);
>> +
>>   	ret = intel_dp_compute_link_config(encoder, crtc_state, conn_state,
>>   					   respect_downstream_limits);
>>   	if (ret) {
>> -		if (intel_dp_is_ycbcr420(intel_dp, crtc_state) ||
>> +		if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
>>   		    !connector->base.ycbcr_420_allowed ||
>>   		    !drm_mode_is_420_also(info, adjusted_mode))
>>   			return ret;
>>   
>> -		crtc_state->output_format = intel_dp_output_format(connector, true);
>> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>> +		crtc_state->output_format = intel_dp_output_format(connector,
>> +								   crtc_state->sink_format);
>>   		ret = intel_dp_compute_link_config(encoder, crtc_state, conn_state,
>>   						   respect_downstream_limits);
>>   	}
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> index a860cbc5dbea..ff0b821a901a 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> @@ -293,6 +293,7 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
>>   	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
>>   		return -EINVAL;
>>   
>> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>>   	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>>   	pipe_config->has_pch_encoder = false;
>>   
>> diff --git a/drivers/gpu/drm/i915/display/intel_dvo.c b/drivers/gpu/drm/i915/display/intel_dvo.c
>> index eb2dcd866cc8..9884678743b6 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dvo.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dvo.c
>> @@ -271,6 +271,7 @@ static int intel_dvo_compute_config(struct intel_encoder *encoder,
>>   	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
>>   		return -EINVAL;
>>   
>> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>>   	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>>   
>>   	return 0;
>> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> index c7e9e1fbed37..1ad0540c13ee 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> @@ -2171,13 +2171,13 @@ static bool intel_hdmi_has_audio(struct intel_encoder *encoder,
>>   
>>   static enum intel_output_format
>>   intel_hdmi_output_format(const struct intel_crtc_state *crtc_state,
>> -			 struct intel_connector *connector,
>> -			 bool ycbcr_420_output)
>> +			 struct intel_connector *connector)
>>   {
>>   	if (!crtc_state->has_hdmi_sink)
>>   		return INTEL_OUTPUT_FORMAT_RGB;
> I think we need to move this into the sink_format selection since
> we can't have sink_format!=output_format with HDMI.
>
>>   
>> -	if (connector->base.ycbcr_420_allowed && ycbcr_420_output)
>> +	if (connector->base.ycbcr_420_allowed &&
> Similarly the 420_allowed check here doesn't seem correct.
>
> In fact I guess what we need is simply:
>   intel_hdmi_output_format()
>   {
>   	return sink_format;
>   }

Hmm alright I think I got it. As we are computing, what should be sent 
to the sink side, the sink_format selection should have all the condition.

Since for Hdmi what is sent from port, is what reaches the sink so 
output_format is same as sink_format:

static enum intel_output_format
intel_hdmi_sink_format(const struct intel_crtc_state *crtc_state,
                        struct intel_connector *connector,
                        bool ycbcr_420_output)
{
         if (!crtc_state->has_hdmi_sink)
                 return INTEL_OUTPUT_FORMAT_RGB;

         if (connector->base.ycbcr_420_allowed && ycbcr_420_output)
                 return INTEL_OUTPUT_FORMAT_YCBCR420;
         else
                 return INTEL_OUTPUT_FORMAT_RGB;
}

static enum intel_output_format
intel_hdmi_output_format(const struct intel_crtc_state *crtc_state)
{
         return crtc_state->sink_format;
}


>
>> +	    crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
>>   		return INTEL_OUTPUT_FORMAT_YCBCR420;
>>   	else
>>   		return INTEL_OUTPUT_FORMAT_RGB;
>> @@ -2195,23 +2195,26 @@ static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
>>   	bool ycbcr_420_only = drm_mode_is_420_only(info, adjusted_mode);
>>   	int ret;
>>   
>> -	crtc_state->output_format =
>> -		intel_hdmi_output_format(crtc_state, connector, ycbcr_420_only);
>> -
>> -	if (ycbcr_420_only && !intel_hdmi_is_ycbcr420(crtc_state)) {
>> +	if (ycbcr_420_only && !connector->base.ycbcr_420_allowed) {
>>   		drm_dbg_kms(&i915->drm,
>>   			    "YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible. Falling back to RGB.\n");
>> -		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
>> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>> +	} else if (ycbcr_420_only) {
>> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>> +	} else {
>> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>>   	}
>>   
>> +	crtc_state->output_format = intel_hdmi_output_format(crtc_state, connector);
>>   	ret = intel_hdmi_compute_clock(encoder, crtc_state, respect_downstream_limits);
>>   	if (ret) {
>> -		if (intel_hdmi_is_ycbcr420(crtc_state) ||
>> +		if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
> With intel_hdmi_output_format() adjusted as mentined above this will
> also need an explicit has_hdmi_sink check.

If we have sink_format as YCbCr420, only when crtc_state->has_hdmi_sink, 
as per above mentioned intel_hdmi_sink_format

then we might not need explicit has_hdmi_sink right?

Regards,

Ankit

>>   		    !connector->base.ycbcr_420_allowed ||
>>   		    !drm_mode_is_420_also(info, adjusted_mode))
>>   			return ret;
>>   
>> -		crtc_state->output_format = intel_hdmi_output_format(crtc_state, connector, true);
>> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>> +		crtc_state->output_format = intel_hdmi_output_format(crtc_state, connector);
>>   		ret = intel_hdmi_compute_clock(encoder, crtc_state, respect_downstream_limits);
>>   	}
>>   
>> diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
>> index a504b3a7fbd5..a7783da37dfd 100644
>> --- a/drivers/gpu/drm/i915/display/intel_lvds.c
>> +++ b/drivers/gpu/drm/i915/display/intel_lvds.c
>> @@ -436,6 +436,7 @@ static int intel_lvds_compute_config(struct intel_encoder *encoder,
>>   		crtc_state->pipe_bpp = lvds_bpp;
>>   	}
>>   
>> +	crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>>   	crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
>>   
>>   	/*
>> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
>> index e12ba458636c..34ee9dd82a78 100644
>> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
>> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
>> @@ -1351,6 +1351,7 @@ static int intel_sdvo_compute_config(struct intel_encoder *encoder,
>>   
>>   	DRM_DEBUG_KMS("forcing bpc to 8 for SDVO\n");
>>   	pipe_config->pipe_bpp = 8*3;
>> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>>   	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>>   
>>   	if (HAS_PCH_SPLIT(to_i915(encoder->base.dev)))
>> diff --git a/drivers/gpu/drm/i915/display/intel_tv.c b/drivers/gpu/drm/i915/display/intel_tv.c
>> index 3b5ff84dc615..6f7ac225293e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_tv.c
>> +++ b/drivers/gpu/drm/i915/display/intel_tv.c
>> @@ -1204,6 +1204,7 @@ intel_tv_compute_config(struct intel_encoder *encoder,
>>   	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
>>   		return -EINVAL;
>>   
>> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>>   	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>>   
>>   	drm_dbg_kms(&dev_priv->drm, "forcing bpc to 8 for TV\n");
>> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
>> index 8d2e6e151ba0..82c30feb7a91 100644
>> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
>> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
>> @@ -280,6 +280,7 @@ static int intel_dsi_compute_config(struct intel_encoder *encoder,
>>   	int ret;
>>   
>>   	drm_dbg_kms(&dev_priv->drm, "\n");
>> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>>   	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>>   
>>   	ret = intel_panel_compute_config(intel_connector, adjusted_mode);
>> -- 
>> 2.25.1

Re: [Intel-gfx] [PATCH v11 03/11] drm/i915/dp: Replace intel_dp.dfp members with the new crtc_state sink_format

[Nautiyal, Ankit K]

Hi Ville,

Thanks for the reviews and comments.

I agree to the suggested changes and will address them in next version 
of the patch.

Please find my responses inline.

On 3/17/2023 5:16 AM, Ville Syrjälä wrote:
> On Tue, Mar 14, 2023 at 04:34:07PM +0530, Ankit Nautiyal wrote:
>> The decision to use DFP output format conversion capabilities should be
>> during compute_config phase.
>>
>> This patch uses the members of intel_dp->dfp to only store the
>> format conversion capabilities of the DP device and uses the crtc_state
>> sink_format member, to program the protocol-converter for
>> colorspace/format conversion.
>>
>> v2: Use sink_format to determine the color conversion config for the
>> pcon (Ville).
>>
>> v3: Fix typo: missing 'break' in switch case (lkp kernel test robot).
>>
>> v4: Add helper to check if DP supports YCBCR420.
>>
>> v5: Simplify logic for computing output_format, based on the given
>> sink_format. (Ville).
>> Added scaler constraint for YCbCr420 output.
>>
>> v6: Split the patch for Scaler constraint for Ycbcr420.
>>
>> v7: Simplify the policy for selecting output_format:
>> Always try for RGB first, followed by YCBCR444, and finally by YCBCR420.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dp.c | 182 +++++++++++++++++-------
>>   1 file changed, 129 insertions(+), 53 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index e52e2ffc355c..c31ec9c91c64 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -817,24 +817,82 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
>>   	return 0;
>>   }
>>   
>> +static bool source_can_output(struct intel_dp *intel_dp,
>> +			      enum intel_output_format format)
>> +{
>> +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>> +	bool is_branch = drm_dp_is_branch(intel_dp->dpcd);
>> +
>> +	if (format == INTEL_OUTPUT_FORMAT_RGB)
>> +		return true;
>> +
>> +	/*
>> +	 * No YCbCr output support on gmch platforms.
>> +	 * Also, ILK doesn't seem capable of DP YCbCr output.
>> +	 * The displayed image is severly corrupted. SNB+ is fine.
>> +	 */
>> +	if (HAS_GMCH(i915) || IS_IRONLAKE(i915))
>> +		return false;
>> +
>> +	if (format == INTEL_OUTPUT_FORMAT_YCBCR444)
>> +		return true;
>> +
>> +	/* Platform < Gen 11 cannot output YCbCr420 format */
>> +	if (DISPLAY_VER(i915) < 11)
>> +		return false;
>> +
>> +	/* If branch device then PCONs should support YCbCr420 Passthrough */
>> +	if (format == INTEL_OUTPUT_FORMAT_YCBCR420)
>> +		return !is_branch || intel_dp->dfp.ycbcr420_passthrough;
> This part isn't really about the source capabilities.
> I think it would be more appropriate to do these checks
> in the caller.

Agreed, will move these in caller.


>> +
>> +	return false;
>> +}
>> +
>> +static bool
>> +dfp_can_convert_from_rgb(struct intel_dp *intel_dp,
>> +			 enum intel_output_format sink_format)
>> +{
>> +	if (!drm_dp_is_branch(intel_dp->dpcd))
>> +		return false;
>> +
>> +	if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR444)
>> +		return intel_dp->dfp.rgb_to_ycbcr;
>> +
>> +	if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
>> +		return intel_dp->dfp.rgb_to_ycbcr &&
>> +			intel_dp->dfp.ycbcr_444_to_420;
>> +
>> +	return false;
>> +}
>> +
>> +static bool
>> +dfp_can_convert_from_ycbcr444(struct intel_dp *intel_dp,
>> +			      enum intel_output_format sink_format)
>> +{
>> +	if (!drm_dp_is_branch(intel_dp->dpcd))
>> +		return false;
>> +
>> +	if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
>> +		return intel_dp->dfp.ycbcr_444_to_420;
>> +
>> +	return false;
>> +}
>> +
>>   static enum intel_output_format
>>   intel_dp_output_format(struct intel_connector *connector,
>>   		       enum intel_output_format sink_format)
>>   {
>>   	struct intel_dp *intel_dp = intel_attached_dp(connector);
>>   
>> -	if (!connector->base.ycbcr_420_allowed ||
>> -	    sink_format != INTEL_OUTPUT_FORMAT_YCBCR420)
>> -		return INTEL_OUTPUT_FORMAT_RGB;
>> -
>> -	if (intel_dp->dfp.rgb_to_ycbcr &&
>> -	    intel_dp->dfp.ycbcr_444_to_420)
>> +	if (sink_format == INTEL_OUTPUT_FORMAT_RGB ||
>> +	    dfp_can_convert_from_rgb(intel_dp, sink_format))
>>   		return INTEL_OUTPUT_FORMAT_RGB;
>>   
>> -	if (intel_dp->dfp.ycbcr_444_to_420)
>> +	if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR444 ||
>> +	    dfp_can_convert_from_ycbcr444(intel_dp, sink_format))
>>   		return INTEL_OUTPUT_FORMAT_YCBCR444;
>> -	else
>> -		return INTEL_OUTPUT_FORMAT_YCBCR420;
>> +
>> +	return INTEL_OUTPUT_FORMAT_YCBCR420;
>>   }
> I'm thinking the caller of intel_dp_output_format() might want
> to do a drm_WARN_ON(!source_can_output(output_format))
> just to make sure we didn't screw things up too badly.
>
> Or maybe we want to have that assert in intel_dp_output_format()
> itself in case there are many callers.

I think we can keep in intel_dp_output_format().

I had a check in earlier version I think, missed to add this in this 
version, but was defaulting to RGB without warn_on.

Will add a warn_on in the function as suggested.

>>   
>>   int intel_dp_min_bpp(enum intel_output_format output_format)
>> @@ -2751,6 +2809,8 @@ void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
>>   					   const struct intel_crtc_state *crtc_state)
>>   {
>>   	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>> +	bool ycbcr444_to_420 = false;
>> +	bool rgb_to_ycbcr = false;
>>   	u8 tmp;
>>   
>>   	if (intel_dp->dpcd[DP_DPCD_REV] < 0x13)
>> @@ -2767,8 +2827,35 @@ void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
>>   		drm_dbg_kms(&i915->drm, "Failed to %s protocol converter HDMI mode\n",
>>   			    str_enable_disable(intel_dp->has_hdmi_sink));
>>   
>> -	tmp = crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444 &&
>> -		intel_dp->dfp.ycbcr_444_to_420 ? DP_CONVERSION_TO_YCBCR420_ENABLE : 0;
>> +	if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
> I was wondering where YCBCR444 went here, but then I
> remembered that we don't have uapi for it.
>
> But even so, we could consider making this code already
> handle it. Would make it that much easier to test
> YCbCr 4:4:4 output. Should just take a one line hack
> to intel_dp_sink_format() at that point. Could be
> a followup patch though.

Makes sense, will add if (crtc_state->sink_format == 
INTEL_OUTPUT_FORMAT_YCBCR444) block.

Will try to test : going with YCbCr444 first instead on RGB, while 
selecting sink_format.

Let me add this in a follow up patch.
>
>> +		switch (crtc_state->output_format) {
>> +		case INTEL_OUTPUT_FORMAT_YCBCR420:
>> +			/*
>> +			 * sink_format is YCbCr420, output_format is also YCbCr420:
>> +			 * Passthrough.
>> +			 */
> These comments seem a bit redundant.

Alright, will remove the comments.


>
>> +			break;
>> +		case INTEL_OUTPUT_FORMAT_YCBCR444:
>> +			/*
>> +			 * sink_format is YCbCr420, output_format is YCbCr444:
>> +			 * Downsample.
>> +			 */
>> +			ycbcr444_to_420 = true;
>> +			break;
>> +		case INTEL_OUTPUT_FORMAT_RGB:
>> +			/*
>> +			 * sink_format is YCbCr420, output_format is RGB:
>> +			 * Convert to YCbCr444 and Downsample.
>> +			 */
>> +			rgb_to_ycbcr = true;
>> +			ycbcr444_to_420 = true;
>> +			break;
>> +		default:
>> +			break;
>> +		}
>> +	}
>> +
>> +	tmp = ycbcr444_to_420 ? DP_CONVERSION_TO_YCBCR420_ENABLE : 0;
>>   
>>   	if (drm_dp_dpcd_writeb(&intel_dp->aux,
>>   			       DP_PROTOCOL_CONVERTER_CONTROL_1, tmp) != 1)
>> @@ -2776,13 +2863,12 @@ void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
>>   			    "Failed to %s protocol converter YCbCr 4:2:0 conversion mode\n",
>>   			    str_enable_disable(intel_dp->dfp.ycbcr_444_to_420));
>>   
>> -	tmp = intel_dp->dfp.rgb_to_ycbcr ?
>> -		DP_CONVERSION_BT709_RGB_YCBCR_ENABLE : 0;
>> +	tmp = rgb_to_ycbcr ? DP_CONVERSION_BT709_RGB_YCBCR_ENABLE : 0;
>>   
>>   	if (drm_dp_pcon_convert_rgb_to_ycbcr(&intel_dp->aux, tmp) < 0)
>>   		drm_dbg_kms(&i915->drm,
>> -			   "Failed to %s protocol converter RGB->YCbCr conversion mode\n",
>> -			   str_enable_disable(tmp));
>> +			    "Failed to %s protocol converter RGB->YCbCr conversion mode\n",
>> +			    str_enable_disable(tmp));
>>   }
>>   
>>   bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp)
>> @@ -4572,57 +4658,47 @@ intel_dp_update_dfp(struct intel_dp *intel_dp,
>>   	intel_dp_get_pcon_dsc_cap(intel_dp);
>>   }
>>   
>> +static bool
>> +intel_dp_can_ycbcr420(struct intel_dp *intel_dp)
>> +{
>> +	if (source_can_output(intel_dp, INTEL_OUTPUT_FORMAT_YCBCR420))
>> +		return true;
> Should have an empty line here.


Noted. will add the line, in next version.

>
>> +	/*
>> +	 * If source cannot support YCbCr420, and PCON has color conv. support:
>> +	 * Source sends YCbCr444, PCON converts YCbCr444->420 Or
>> +	 * Source sends RGB444, PCON converts RGB->YCbCr444 + YCbCr444->YCbCr420)
>> +	 */
> I think the code already explains that pretty well. Comment seems
> a bit redundant.

Alright, will remove comments from here.


>> +	if (source_can_output(intel_dp, INTEL_OUTPUT_FORMAT_RGB) &&
>> +	    dfp_can_convert_from_rgb(intel_dp, INTEL_OUTPUT_FORMAT_YCBCR420))
>> +		return true;
>> +
>> +	if (source_can_output(intel_dp, INTEL_OUTPUT_FORMAT_YCBCR444) &&
>> +	    dfp_can_convert_from_ycbcr444(intel_dp, INTEL_OUTPUT_FORMAT_YCBCR420))
>> +		return INTEL_OUTPUT_FORMAT_YCBCR444;
>> +
>> +	return false;
>> +}
>> +
>>   static void
>>   intel_dp_update_420(struct intel_dp *intel_dp)
>>   {
>>   	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>>   	struct intel_connector *connector = intel_dp->attached_connector;
>> -	bool is_branch, ycbcr_420_passthrough, ycbcr_444_to_420, rgb_to_ycbcr;
>> -
>> -	/* No YCbCr output support on gmch platforms */
>> -	if (HAS_GMCH(i915))
>> -		return;
>>   
>> -	/*
>> -	 * ILK doesn't seem capable of DP YCbCr output. The
>> -	 * displayed image is severly corrupted. SNB+ is fine.
>> -	 */
>> -	if (IS_IRONLAKE(i915))
>> -		return;
>> -
>> -	is_branch = drm_dp_is_branch(intel_dp->dpcd);
>> -	ycbcr_420_passthrough =
>> +	intel_dp->dfp.ycbcr420_passthrough =
>>   		drm_dp_downstream_420_passthrough(intel_dp->dpcd,
>>   						  intel_dp->downstream_ports);
>>   	/* on-board LSPCON always assumed to support 4:4:4->4:2:0 conversion */
>> -	ycbcr_444_to_420 =
>> +	intel_dp->dfp.ycbcr_444_to_420 =
>>   		dp_to_dig_port(intel_dp)->lspcon.active ||
>>   		drm_dp_downstream_444_to_420_conversion(intel_dp->dpcd,
>>   							intel_dp->downstream_ports);
>> -	rgb_to_ycbcr = drm_dp_downstream_rgb_to_ycbcr_conversion(intel_dp->dpcd,
>> -								 intel_dp->downstream_ports,
>> -								 DP_DS_HDMI_BT709_RGB_YCBCR_CONV);
>> -
>> -	if (DISPLAY_VER(i915) >= 11) {
>> -		/* Let PCON convert from RGB->YCbCr if possible */
>> -		if (is_branch && rgb_to_ycbcr && ycbcr_444_to_420) {
>> -			intel_dp->dfp.rgb_to_ycbcr = true;
>> -			intel_dp->dfp.ycbcr_444_to_420 = true;
>> -			connector->base.ycbcr_420_allowed = true;
>> -		} else {
>> -		/* Prefer 4:2:0 passthrough over 4:4:4->4:2:0 conversion */
>> -			intel_dp->dfp.ycbcr_444_to_420 =
>> -				ycbcr_444_to_420 && !ycbcr_420_passthrough;
>> +	intel_dp->dfp.rgb_to_ycbcr =
>> +		drm_dp_downstream_rgb_to_ycbcr_conversion(intel_dp->dpcd,
>> +							  intel_dp->downstream_ports,
>> +							  DP_DS_HDMI_BT709_RGB_YCBCR_CONV);
>>   
>> -			connector->base.ycbcr_420_allowed =
>> -				!is_branch || ycbcr_444_to_420 || ycbcr_420_passthrough;
>> -		}
>> -	} else {
>> -		/* 4:4:4->4:2:0 conversion is the only way */
>> -		intel_dp->dfp.ycbcr_444_to_420 = ycbcr_444_to_420;
>> -
>> -		connector->base.ycbcr_420_allowed = ycbcr_444_to_420;
>> -	}
>> +	connector->base.ycbcr_420_allowed = intel_dp_can_ycbcr420(intel_dp);
> Looks good. With the minor issues sorted this is
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks again Ville for the suggestions and review.

Regards,

Ankit

>>   
>>   	drm_dbg_kms(&i915->drm,
>>   		    "[CONNECTOR:%d:%s] RGB->YcbCr conversion? %s, YCbCr 4:2:0 allowed? %s, YCbCr 4:4:4->4:2:0 conversion? %s\n",
>> -- 
>> 2.25.1

Re: [Intel-gfx] [PATCH v11 04/11] drm/i915/display: Use sink_format instead of ycbcr420_output flag

[Nautiyal, Ankit K]

On 3/17/2023 5:55 AM, Ville Syrjälä wrote:
> On Tue, Mar 14, 2023 at 04:34:08PM +0530, Ankit Nautiyal wrote:
>> Start passing the sink_format, to all functions that take a bool
>> ycbcr420_output as parameter. This will make the functions generic,
>> and will serve as a slight step towards 4:2:2 support later.
>>
>> v2: Rebased.
>>
>> Suggested-by: Ville Syrj_l_ <ville.syrjala@linux.intel.com>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dp.c   | 33 +++++++--------
>>   drivers/gpu/drm/i915/display/intel_hdmi.c | 50 ++++++++++++-----------
>>   drivers/gpu/drm/i915/display/intel_hdmi.h |  5 ++-
>>   3 files changed, 44 insertions(+), 44 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index c31ec9c91c64..b49d113357e4 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -967,7 +967,8 @@ static int intel_dp_max_tmds_clock(struct intel_dp *intel_dp)
>>   
>>   static enum drm_mode_status
>>   intel_dp_tmds_clock_valid(struct intel_dp *intel_dp,
>> -			  int clock, int bpc, bool ycbcr420_output,
>> +			  int clock, int bpc,
>> +			  enum intel_output_format sink_format,
>>   			  bool respect_downstream_limits)
>>   {
>>   	int tmds_clock, min_tmds_clock, max_tmds_clock;
>> @@ -975,7 +976,7 @@ intel_dp_tmds_clock_valid(struct intel_dp *intel_dp,
>>   	if (!respect_downstream_limits)
>>   		return MODE_OK;
>>   
>> -	tmds_clock = intel_hdmi_tmds_clock(clock, bpc, ycbcr420_output);
>> +	tmds_clock = intel_hdmi_tmds_clock(clock, bpc, sink_format);
>>   
>>   	min_tmds_clock = intel_dp->dfp.min_tmds_clock;
>>   	max_tmds_clock = intel_dp_max_tmds_clock(intel_dp);
>> @@ -998,6 +999,7 @@ intel_dp_mode_valid_downstream(struct intel_connector *connector,
>>   	const struct drm_display_info *info = &connector->base.display_info;
>>   	enum drm_mode_status status;
>>   	bool ycbcr_420_only;
>> +	enum intel_output_format sink_format;
>>   
>>   	/* If PCON supports FRL MODE, check FRL bandwidth constraints */
>>   	if (intel_dp->dfp.pcon_max_frl_bw) {
>> @@ -1024,18 +1026,22 @@ intel_dp_mode_valid_downstream(struct intel_connector *connector,
>>   
>>   	ycbcr_420_only = drm_mode_is_420_only(info, mode);
>>   
>> +	if (ycbcr_420_only && connector->base.ycbcr_420_allowed)
> drm_mode_validate_ycbcr420() already checks for this combination.
> So I don't think we should need to redo it here, and we didn't
> before.

I missed that. Yes indeed this is redundant.

So will just set sink_format as YCbCr420 based on ycbcr_420_only flag.


>
>> +		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>> +	else
>> +		sink_format = INTEL_OUTPUT_FORMAT_RGB;
>> +
>>   	/* Assume 8bpc for the DP++/HDMI/DVI TMDS clock check */
>>   	status = intel_dp_tmds_clock_valid(intel_dp, target_clock,
>> -					   8, ycbcr_420_only, true);
>> +					   8, sink_format, true);
>>   
>>   	if (status != MODE_OK) {
>> -		if (ycbcr_420_only ||
>> -		    !connector->base.ycbcr_420_allowed ||
> But here we do need to keep the allowed flag check since
> drm_mode_validate_ycbcr420() doesn't care about 420_also() modes.

Ok will retain the check for connector->base.ycbcr_420_allowed.


>
>> +		if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
>>   		    !drm_mode_is_420_also(info, mode))
>>   			return status;
>> -
>> +		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>>   		status = intel_dp_tmds_clock_valid(intel_dp, target_clock,
>> -						   8, true, true);
>> +						   8, sink_format, true);
>>   		if (status != MODE_OK)
>>   			return status;
>>   	}
>> @@ -1271,19 +1277,10 @@ static bool intel_dp_supports_dsc(struct intel_dp *intel_dp,
>>   		drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd);
>>   }
>>   
>> -static bool intel_dp_is_ycbcr420(struct intel_dp *intel_dp,
>> -				 const struct intel_crtc_state *crtc_state)
>> -{
>> -	return crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
>> -		(crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444 &&
>> -		 intel_dp->dfp.ycbcr_444_to_420);
>> -}
>> -
>>   static int intel_dp_hdmi_compute_bpc(struct intel_dp *intel_dp,
>>   				     const struct intel_crtc_state *crtc_state,
>>   				     int bpc, bool respect_downstream_limits)
>>   {
>> -	bool ycbcr420_output = intel_dp_is_ycbcr420(intel_dp, crtc_state);
>>   	int clock = crtc_state->hw.adjusted_mode.crtc_clock;
>>   
>>   	/*
>> @@ -1303,8 +1300,8 @@ static int intel_dp_hdmi_compute_bpc(struct intel_dp *intel_dp,
>>   
>>   	for (; bpc >= 8; bpc -= 2) {
>>   		if (intel_hdmi_bpc_possible(crtc_state, bpc,
>> -					    intel_dp->has_hdmi_sink, ycbcr420_output) &&
>> -		    intel_dp_tmds_clock_valid(intel_dp, clock, bpc, ycbcr420_output,
>> +					    intel_dp->has_hdmi_sink) &&
>> +		    intel_dp_tmds_clock_valid(intel_dp, clock, bpc, crtc_state->sink_format,
>>   					      respect_downstream_limits) == MODE_OK)
>>   			return bpc;
>>   	}
>> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> index 1ad0540c13ee..15bf64a217c2 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> @@ -1793,11 +1793,6 @@ static bool intel_has_hdmi_sink(struct intel_hdmi *hdmi,
>>   		READ_ONCE(to_intel_digital_connector_state(conn_state)->force_audio) != HDMI_AUDIO_OFF_DVI;
>>   }
>>   
>> -static bool intel_hdmi_is_ycbcr420(const struct intel_crtc_state *crtc_state)
>> -{
>> -	return crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420;
>> -}
>> -
>>   static int hdmi_port_clock_limit(struct intel_hdmi *hdmi,
>>   				 bool respect_downstream_limits,
>>   				 bool has_hdmi_sink)
>> @@ -1871,10 +1866,11 @@ hdmi_port_clock_valid(struct intel_hdmi *hdmi,
>>   	return MODE_OK;
>>   }
>>   
>> -int intel_hdmi_tmds_clock(int clock, int bpc, bool ycbcr420_output)
>> +int intel_hdmi_tmds_clock(int clock, int bpc,
>> +			  enum intel_output_format sink_format)
>>   {
>>   	/* YCBCR420 TMDS rate requirement is half the pixel clock */
>> -	if (ycbcr420_output)
>> +	if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
>>   		clock /= 2;
>>   
>>   	/*
>> @@ -1901,7 +1897,8 @@ static bool intel_hdmi_source_bpc_possible(struct drm_i915_private *i915, int bp
>>   }
>>   
>>   static bool intel_hdmi_sink_bpc_possible(struct drm_connector *connector,
>> -					 int bpc, bool has_hdmi_sink, bool ycbcr420_output)
>> +					 int bpc, bool has_hdmi_sink,
>> +					 enum intel_output_format sink_format)
>>   {
>>   	const struct drm_display_info *info = &connector->display_info;
>>   	const struct drm_hdmi_info *hdmi = &info->hdmi;
>> @@ -1911,7 +1908,7 @@ static bool intel_hdmi_sink_bpc_possible(struct drm_connector *connector,
>>   		if (!has_hdmi_sink)
>>   			return false;
>>   
>> -		if (ycbcr420_output)
>> +		if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
>>   			return hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_36;
>>   		else
>>   			return info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_36;
>> @@ -1919,7 +1916,7 @@ static bool intel_hdmi_sink_bpc_possible(struct drm_connector *connector,
>>   		if (!has_hdmi_sink)
>>   			return false;
>>   
>> -		if (ycbcr420_output)
>> +		if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
>>   			return hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_30;
>>   		else
>>   			return info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_30;
>> @@ -1933,7 +1930,8 @@ static bool intel_hdmi_sink_bpc_possible(struct drm_connector *connector,
>>   
>>   static enum drm_mode_status
>>   intel_hdmi_mode_clock_valid(struct drm_connector *connector, int clock,
>> -			    bool has_hdmi_sink, bool ycbcr420_output)
>> +			    bool has_hdmi_sink,
>> +			    enum intel_output_format sink_format)
>>   {
>>   	struct drm_i915_private *i915 = to_i915(connector->dev);
>>   	struct intel_hdmi *hdmi = intel_attached_hdmi(to_intel_connector(connector));
>> @@ -1946,12 +1944,12 @@ intel_hdmi_mode_clock_valid(struct drm_connector *connector, int clock,
>>   	 * least one color depth is accepted.
>>   	 */
>>   	for (bpc = 12; bpc >= 8; bpc -= 2) {
>> -		int tmds_clock = intel_hdmi_tmds_clock(clock, bpc, ycbcr420_output);
>> +		int tmds_clock = intel_hdmi_tmds_clock(clock, bpc, sink_format);
>>   
>>   		if (!intel_hdmi_source_bpc_possible(i915, bpc))
>>   			continue;
>>   
>> -		if (!intel_hdmi_sink_bpc_possible(connector, bpc, has_hdmi_sink, ycbcr420_output))
>> +		if (!intel_hdmi_sink_bpc_possible(connector, bpc, has_hdmi_sink, sink_format))
>>   			continue;
>>   
>>   		status = hdmi_port_clock_valid(hdmi, tmds_clock, true, has_hdmi_sink);
>> @@ -1976,6 +1974,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>>   	int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
>>   	bool has_hdmi_sink = intel_has_hdmi_sink(hdmi, connector->state);
>>   	bool ycbcr_420_only;
>> +	enum intel_output_format sink_format;
>>   
>>   	if ((mode->flags & DRM_MODE_FLAG_3D_MASK) == DRM_MODE_FLAG_3D_FRAME_PACKING)
>>   		clock *= 2;
>> @@ -2000,14 +1999,17 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>>   
>>   	ycbcr_420_only = drm_mode_is_420_only(&connector->display_info, mode);
>>   
>> -	status = intel_hdmi_mode_clock_valid(connector, clock, has_hdmi_sink, ycbcr_420_only);
>> +	sink_format = ycbcr_420_only ? INTEL_OUTPUT_FORMAT_YCBCR420 : INTEL_OUTPUT_FORMAT_RGB;
>> +
> Please try to stick to common style between here and
> intel_dp_mode_valid_downstream() to avoid a some of the
> head scratching when comparing the two.

Alright will just have similar style check as in DP case.


>
>> +	status = intel_hdmi_mode_clock_valid(connector, clock, has_hdmi_sink, sink_format);
>>   	if (status != MODE_OK) {
>>   		if (ycbcr_420_only ||
>>   		    !connector->ycbcr_420_allowed ||
>>   		    !drm_mode_is_420_also(&connector->display_info, mode))
>>   			return status;
>>   
>> -		status = intel_hdmi_mode_clock_valid(connector, clock, has_hdmi_sink, true);
>> +		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>> +		status = intel_hdmi_mode_clock_valid(connector, clock, has_hdmi_sink, sink_format);
>>   		if (status != MODE_OK)
>>   			return status;
>>   	}
>> @@ -2016,7 +2018,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>>   }
>>   
>>   bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state,
>> -			     int bpc, bool has_hdmi_sink, bool ycbcr420_output)
>> +			     int bpc, bool has_hdmi_sink)
>>   {
>>   	struct drm_atomic_state *state = crtc_state->uapi.state;
>>   	struct drm_connector_state *connector_state;
>> @@ -2027,7 +2029,8 @@ bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state,
>>   		if (connector_state->crtc != crtc_state->uapi.crtc)
>>   			continue;
>>   
>> -		if (!intel_hdmi_sink_bpc_possible(connector, bpc, has_hdmi_sink, ycbcr420_output))
>> +		if (!intel_hdmi_sink_bpc_possible(connector, bpc, has_hdmi_sink,
>> +						  crtc_state->sink_format))
>>   			return false;
>>   	}
>>   
>> @@ -2045,14 +2048,13 @@ static bool hdmi_bpc_possible(const struct intel_crtc_state *crtc_state, int bpc
>>   		return false;
>>   
>>   	/* Display Wa_1405510057:icl,ehl */
>> -	if (intel_hdmi_is_ycbcr420(crtc_state) &&
>> +	if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
>>   	    bpc == 10 && DISPLAY_VER(dev_priv) == 11 &&
>>   	    (adjusted_mode->crtc_hblank_end -
>>   	     adjusted_mode->crtc_hblank_start) % 8 == 2)
>>   		return false;
>>   
>> -	return intel_hdmi_bpc_possible(crtc_state, bpc, crtc_state->has_hdmi_sink,
>> -				       intel_hdmi_is_ycbcr420(crtc_state));
>> +	return intel_hdmi_bpc_possible(crtc_state, bpc, crtc_state->has_hdmi_sink);
>>   }
>>   
>>   static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
>> @@ -2060,7 +2062,6 @@ static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
>>   				  int clock, bool respect_downstream_limits)
>>   {
>>   	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
>> -	bool ycbcr420_output = intel_hdmi_is_ycbcr420(crtc_state);
>>   	int bpc;
>>   
>>   	/*
>> @@ -2078,7 +2079,8 @@ static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
>>   		bpc = 8;
>>   
>>   	for (; bpc >= 8; bpc -= 2) {
>> -		int tmds_clock = intel_hdmi_tmds_clock(clock, bpc, ycbcr420_output);
>> +		int tmds_clock = intel_hdmi_tmds_clock(clock, bpc,
>> +						       crtc_state->sink_format);
>>   
>>   		if (hdmi_bpc_possible(crtc_state, bpc) &&
>>   		    hdmi_port_clock_valid(intel_hdmi, tmds_clock,
>> @@ -2108,7 +2110,7 @@ static int intel_hdmi_compute_clock(struct intel_encoder *encoder,
>>   		return bpc;
>>   
>>   	crtc_state->port_clock =
>> -		intel_hdmi_tmds_clock(clock, bpc, intel_hdmi_is_ycbcr420(crtc_state));
>> +		intel_hdmi_tmds_clock(clock, bpc, crtc_state->sink_format);
>>   
>>   	/*
>>   	 * pipe_bpp could already be below 8bpc due to
>> @@ -2293,7 +2295,7 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
>>   		return ret;
>>   	}
>>   
>> -	if (intel_hdmi_is_ycbcr420(pipe_config)) {
>> +	if (pipe_config->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
> While the sink_format vs. output_format distinction doesn't really
> matter for HDMI it might be clearer to use output_format at
> least in places like this where we're clearly 100% concerned with
> what comes out the end of the pipe. That is also what the DP
> counterpart code checks here.
>
> For a lot of the other cases the in the HDMI code it's a lot less
> clear currently which is the more appropriate choice, and maybe
> in places neither is entirely correct with the way it gets used
> currently.

Yes indeed, I did get confuse. The things pertaining to : what comes out 
of pipe, should have output_format.

I will retain the function intel_hdmi_is_ycbcr420 and re-evaluate 
whether we mean to check output_format or sink_format.

Thanks & Regards,

Ankit


>
>>   		ret = intel_panel_fitting(pipe_config, conn_state);
>>   		if (ret)
>>   			return ret;
>> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.h b/drivers/gpu/drm/i915/display/intel_hdmi.h
>> index 774dda2376ed..d1e27247b657 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hdmi.h
>> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.h
>> @@ -9,6 +9,7 @@
>>   #include <linux/types.h>
>>   
>>   enum hdmi_infoframe_type;
>> +enum intel_output_format;
>>   enum port;
>>   struct drm_connector;
>>   struct drm_connector_state;
>> @@ -45,8 +46,8 @@ void intel_read_infoframe(struct intel_encoder *encoder,
>>   bool intel_hdmi_limited_color_range(const struct intel_crtc_state *crtc_state,
>>   				    const struct drm_connector_state *conn_state);
>>   bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state,
>> -			     int bpc, bool has_hdmi_sink, bool ycbcr420_output);
>> -int intel_hdmi_tmds_clock(int clock, int bpc, bool ycbcr420_output);
>> +			     int bpc, bool has_hdmi_sink);
>> +int intel_hdmi_tmds_clock(int clock, int bpc, enum intel_output_format sink_format);
>>   int intel_hdmi_dsc_get_bpp(int src_fractional_bpp, int slice_width,
>>   			   int num_slices, int output_format, bool hdmi_all_bpp,
>>   			   int hdmi_max_chunk_bytes);
>> -- 
>> 2.25.1

Re: [Intel-gfx] [PATCH v11 01/11] drm/i915/display: Add new member to configure PCON color conversion

[Ville Syrjälä]

On Fri, Mar 17, 2023 at 03:37:40PM +0530, Nautiyal, Ankit K wrote:
> 
> On 3/17/2023 5:54 AM, Ville Syrjälä wrote:
> > On Tue, Mar 14, 2023 at 04:34:05PM +0530, Ankit Nautiyal wrote:
> >> The decision to use DFP output format conversion capabilities should be
> >> during compute_config phase.
> >>
> >> This patch adds new member to crtc_state to represent the final
> >> output_format to the sink. In case of a DFP this can be different than
> >> the output_format, as per the format conversion done via the PCON.
> >>
> >> This will help to store only the format conversion capabilities of the
> >> DP device in intel_dp->dfp, and use crtc_state to compute and store the
> >> configuration for color/format conversion for a given mode.
> >>
> >> v2: modified the new member to crtc_state to represent the final
> >> output_format that eaches the sink, after possible conversion by
> >> PCON kind of devices. (Ville)
> >>
> >> v3: Addressed comments from Ville:
> >> -Added comments to clarify difference between sink_format and
> >> output_format.
> >> -Corrected the order of setting sink_format and output_format.
> >> -Added readout for sink_format in get_pipe_config hooks.
> >>
> >> v4: Set sink_format for intel_sdvo too. (Ville)
> >>
> >> v5: Rebased.
> >>
> >> v6: Fixed condition to go for YCbCr420 format for dp and hdmi. (Ville)
> >>
> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> >> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (v3)
> >> ---
> >>   drivers/gpu/drm/i915/display/icl_dsi.c        |  1 +
> >>   drivers/gpu/drm/i915/display/intel_crt.c      |  1 +
> >>   .../drm/i915/display/intel_crtc_state_dump.c  |  5 +--
> >>   drivers/gpu/drm/i915/display/intel_display.c  |  5 +++
> >>   .../drm/i915/display/intel_display_types.h    | 11 ++++++-
> >>   drivers/gpu/drm/i915/display/intel_dp.c       | 33 +++++++++++++------
> >>   drivers/gpu/drm/i915/display/intel_dp_mst.c   |  1 +
> >>   drivers/gpu/drm/i915/display/intel_dvo.c      |  1 +
> >>   drivers/gpu/drm/i915/display/intel_hdmi.c     | 23 +++++++------
> >>   drivers/gpu/drm/i915/display/intel_lvds.c     |  1 +
> >>   drivers/gpu/drm/i915/display/intel_sdvo.c     |  1 +
> >>   drivers/gpu/drm/i915/display/intel_tv.c       |  1 +
> >>   drivers/gpu/drm/i915/display/vlv_dsi.c        |  1 +
> >>   13 files changed, 62 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> >> index 50dcaa895854..e8ac4552e681 100644
> >> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> >> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> >> @@ -1593,6 +1593,7 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
> >>   		&pipe_config->hw.adjusted_mode;
> >>   	int ret;
> >>   
> >> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
> >>   	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
> >>   
> >>   	ret = intel_panel_compute_config(intel_connector, adjusted_mode);
> >> diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
> >> index 8f2ebead0826..e925e21d87fc 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_crt.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_crt.c
> >> @@ -393,6 +393,7 @@ static int intel_crt_compute_config(struct intel_encoder *encoder,
> >>   	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> >>   		return -EINVAL;
> >>   
> >> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
> >>   	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
> >>   
> >>   	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 766633566fd6..185cd1971aa5 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> >> @@ -178,10 +178,11 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
> >>   
> >>   	snprintf_output_types(buf, sizeof(buf), pipe_config->output_types);
> >>   	drm_dbg_kms(&i915->drm,
> >> -		    "active: %s, output_types: %s (0x%x), output format: %s\n",
> >> +		    "active: %s, output_types: %s (0x%x), output format: %s, sink format: %s\n",
> >>   		    str_yes_no(pipe_config->hw.active),
> >>   		    buf, pipe_config->output_types,
> >> -		    output_formats(pipe_config->output_format));
> >> +		    output_formats(pipe_config->output_format),
> >> +		    output_formats(pipe_config->sink_format));
> >>   
> >>   	drm_dbg_kms(&i915->drm,
> >>   		    "cpu_transcoder: %s, pipe bpp: %i, dithering: %i\n",
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> >> index 410c84fd905c..6fa86d1e0480 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> >> @@ -3127,6 +3127,7 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> >>   		return false;
> >>   
> >>   	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
> >> +	pipe_config->sink_format = pipe_config->output_format;
> >>   	pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
> >>   	pipe_config->shared_dpll = NULL;
> >>   
> >> @@ -3586,6 +3587,8 @@ static bool ilk_get_pipe_config(struct intel_crtc *crtc,
> >>   		break;
> >>   	}
> >>   
> >> +	pipe_config->sink_format = pipe_config->output_format;
> >> +
> >>   	pipe_config->gamma_mode = REG_FIELD_GET(TRANSCONF_GAMMA_MODE_MASK_ILK, tmp);
> >>   
> >>   	pipe_config->framestart_delay = REG_FIELD_GET(TRANSCONF_FRAME_START_DELAY_MASK, tmp) + 1;
> >> @@ -3984,6 +3987,8 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc,
> >>   			bdw_get_pipemisc_output_format(crtc);
> >>   	}
> >>   
> >> +	pipe_config->sink_format = pipe_config->output_format;
> >> +
> >>   	pipe_config->gamma_mode = intel_de_read(dev_priv,
> >>   						GAMMA_MODE(crtc->pipe));
> >>   
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> >> index c32bfba06ca1..c9d1863d2765 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> >> @@ -1274,9 +1274,18 @@ struct intel_crtc_state {
> >>   	/* HDMI High TMDS char rate ratio */
> >>   	bool hdmi_high_tmds_clock_ratio;
> >>   
> >> -	/* Output format RGB/YCBCR etc */
> >> +	/*
> >> +	 * Output format RGB/YCBCR etc., that is coming out
> >> +	 * at the end of the pipe.
> >> +	 */
> >>   	enum intel_output_format output_format;
> >>   
> >> +	/*
> >> +	 * Sink output format RGB/YCBCR etc., that is going
> >> +	 * into the sink.
> >> +	 */
> >> +	enum intel_output_format sink_format;
> >> +
> >>   	/* enable pipe gamma? */
> >>   	bool gamma_enable;
> >>   
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> >> index 8e16745275f6..e52e2ffc355c 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >> @@ -819,11 +819,12 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
> >>   
> >>   static enum intel_output_format
> >>   intel_dp_output_format(struct intel_connector *connector,
> >> -		       bool ycbcr_420_output)
> >> +		       enum intel_output_format sink_format)
> >>   {
> >>   	struct intel_dp *intel_dp = intel_attached_dp(connector);
> >>   
> >> -	if (!connector->base.ycbcr_420_allowed || !ycbcr_420_output)
> >> +	if (!connector->base.ycbcr_420_allowed ||
> >> +	    sink_format != INTEL_OUTPUT_FORMAT_YCBCR420)
> >>   		return INTEL_OUTPUT_FORMAT_RGB;
> >>   
> >>   	if (intel_dp->dfp.rgb_to_ycbcr &&
> >> @@ -862,8 +863,14 @@ intel_dp_mode_min_output_bpp(struct intel_connector *connector,
> >>   			     const struct drm_display_mode *mode)
> >>   {
> >>   	const struct drm_display_info *info = &connector->base.display_info;
> >> -	enum intel_output_format output_format =
> >> -		intel_dp_output_format(connector, drm_mode_is_420_only(info, mode));
> >> +	enum intel_output_format output_format, sink_format;
> >> +
> >> +	if (drm_mode_is_420_only(info, mode))
> >> +		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> >> +	else
> >> +		sink_format = INTEL_OUTPUT_FORMAT_RGB;
> >> +
> >> +	output_format = intel_dp_output_format(connector, sink_format);
> >>   
> >>   	return intel_dp_output_bpp(output_format, intel_dp_min_bpp(output_format));
> >>   }
> >> @@ -2040,23 +2047,29 @@ intel_dp_compute_output_format(struct intel_encoder *encoder,
> >>   
> >>   	ycbcr_420_only = drm_mode_is_420_only(info, adjusted_mode);
> >>   
> >> -	crtc_state->output_format = intel_dp_output_format(connector, ycbcr_420_only);
> >> -
> >> -	if (ycbcr_420_only && !intel_dp_is_ycbcr420(intel_dp, crtc_state)) {
> >> +	if (ycbcr_420_only && !connector->base.ycbcr_420_allowed) {
> >>   		drm_dbg_kms(&i915->drm,
> >>   			    "YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible. Falling back to RGB.\n");
> >> -		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
> >> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
> >> +	} else if (ycbcr_420_only) {
> >> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> >> +	} else {
> >> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
> >>   	}
> >>   
> >> +	crtc_state->output_format = intel_dp_output_format(connector, crtc_state->sink_format);
> >> +
> >>   	ret = intel_dp_compute_link_config(encoder, crtc_state, conn_state,
> >>   					   respect_downstream_limits);
> >>   	if (ret) {
> >> -		if (intel_dp_is_ycbcr420(intel_dp, crtc_state) ||
> >> +		if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
> >>   		    !connector->base.ycbcr_420_allowed ||
> >>   		    !drm_mode_is_420_also(info, adjusted_mode))
> >>   			return ret;
> >>   
> >> -		crtc_state->output_format = intel_dp_output_format(connector, true);
> >> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> >> +		crtc_state->output_format = intel_dp_output_format(connector,
> >> +								   crtc_state->sink_format);
> >>   		ret = intel_dp_compute_link_config(encoder, crtc_state, conn_state,
> >>   						   respect_downstream_limits);
> >>   	}
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> >> index a860cbc5dbea..ff0b821a901a 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> >> @@ -293,6 +293,7 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
> >>   	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> >>   		return -EINVAL;
> >>   
> >> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
> >>   	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
> >>   	pipe_config->has_pch_encoder = false;
> >>   
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dvo.c b/drivers/gpu/drm/i915/display/intel_dvo.c
> >> index eb2dcd866cc8..9884678743b6 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dvo.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dvo.c
> >> @@ -271,6 +271,7 @@ static int intel_dvo_compute_config(struct intel_encoder *encoder,
> >>   	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> >>   		return -EINVAL;
> >>   
> >> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
> >>   	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
> >>   
> >>   	return 0;
> >> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> >> index c7e9e1fbed37..1ad0540c13ee 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> >> @@ -2171,13 +2171,13 @@ static bool intel_hdmi_has_audio(struct intel_encoder *encoder,
> >>   
> >>   static enum intel_output_format
> >>   intel_hdmi_output_format(const struct intel_crtc_state *crtc_state,
> >> -			 struct intel_connector *connector,
> >> -			 bool ycbcr_420_output)
> >> +			 struct intel_connector *connector)
> >>   {
> >>   	if (!crtc_state->has_hdmi_sink)
> >>   		return INTEL_OUTPUT_FORMAT_RGB;
> > I think we need to move this into the sink_format selection since
> > we can't have sink_format!=output_format with HDMI.
> >
> >>   
> >> -	if (connector->base.ycbcr_420_allowed && ycbcr_420_output)
> >> +	if (connector->base.ycbcr_420_allowed &&
> > Similarly the 420_allowed check here doesn't seem correct.
> >
> > In fact I guess what we need is simply:
> >   intel_hdmi_output_format()
> >   {
> >   	return sink_format;
> >   }
> 
> Hmm alright I think I got it. As we are computing, what should be sent 
> to the sink side, the sink_format selection should have all the condition.
> 
> Since for Hdmi what is sent from port, is what reaches the sink so 
> output_format is same as sink_format:
> 
> static enum intel_output_format
> intel_hdmi_sink_format(const struct intel_crtc_state *crtc_state,
>                         struct intel_connector *connector,
>                         bool ycbcr_420_output)
> {
>          if (!crtc_state->has_hdmi_sink)
>                  return INTEL_OUTPUT_FORMAT_RGB;
> 
>          if (connector->base.ycbcr_420_allowed && ycbcr_420_output)
>                  return INTEL_OUTPUT_FORMAT_YCBCR420;
>          else
>                  return INTEL_OUTPUT_FORMAT_RGB;
> }
> 
> static enum intel_output_format
> intel_hdmi_output_format(const struct intel_crtc_state *crtc_state)
> {
>          return crtc_state->sink_format;
> }
> 
> 
> >
> >> +	    crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> >>   		return INTEL_OUTPUT_FORMAT_YCBCR420;
> >>   	else
> >>   		return INTEL_OUTPUT_FORMAT_RGB;
> >> @@ -2195,23 +2195,26 @@ static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
> >>   	bool ycbcr_420_only = drm_mode_is_420_only(info, adjusted_mode);
> >>   	int ret;
> >>   
> >> -	crtc_state->output_format =
> >> -		intel_hdmi_output_format(crtc_state, connector, ycbcr_420_only);
> >> -
> >> -	if (ycbcr_420_only && !intel_hdmi_is_ycbcr420(crtc_state)) {
> >> +	if (ycbcr_420_only && !connector->base.ycbcr_420_allowed) {
> >>   		drm_dbg_kms(&i915->drm,
> >>   			    "YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible. Falling back to RGB.\n");
> >> -		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
> >> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
> >> +	} else if (ycbcr_420_only) {
> >> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> >> +	} else {
> >> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
> >>   	}
> >>   
> >> +	crtc_state->output_format = intel_hdmi_output_format(crtc_state, connector);
> >>   	ret = intel_hdmi_compute_clock(encoder, crtc_state, respect_downstream_limits);
> >>   	if (ret) {
> >> -		if (intel_hdmi_is_ycbcr420(crtc_state) ||
> >> +		if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
> > With intel_hdmi_output_format() adjusted as mentined above this will
> > also need an explicit has_hdmi_sink check.
> 
> If we have sink_format as YCbCr420, only when crtc_state->has_hdmi_sink, 
> as per above mentioned intel_hdmi_sink_format
> 
> then we might not need explicit has_hdmi_sink right?

We are setting sink_format=YCBCR420 direcly below,
hence I think we need an explicit check here.

> 
> Regards,
> 
> Ankit
> 
> >>   		    !connector->base.ycbcr_420_allowed ||
> >>   		    !drm_mode_is_420_also(info, adjusted_mode))
> >>   			return ret;
> >>   
> >> -		crtc_state->output_format = intel_hdmi_output_format(crtc_state, connector, true);
> >> +		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> >> +		crtc_state->output_format = intel_hdmi_output_format(crtc_state, connector);
> >>   		ret = intel_hdmi_compute_clock(encoder, crtc_state, respect_downstream_limits);
> >>   	}
> >>   
> >> diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
> >> index a504b3a7fbd5..a7783da37dfd 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_lvds.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_lvds.c
> >> @@ -436,6 +436,7 @@ static int intel_lvds_compute_config(struct intel_encoder *encoder,
> >>   		crtc_state->pipe_bpp = lvds_bpp;
> >>   	}
> >>   
> >> +	crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
> >>   	crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
> >>   
> >>   	/*
> >> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> >> index e12ba458636c..34ee9dd82a78 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> >> @@ -1351,6 +1351,7 @@ static int intel_sdvo_compute_config(struct intel_encoder *encoder,
> >>   
> >>   	DRM_DEBUG_KMS("forcing bpc to 8 for SDVO\n");
> >>   	pipe_config->pipe_bpp = 8*3;
> >> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
> >>   	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
> >>   
> >>   	if (HAS_PCH_SPLIT(to_i915(encoder->base.dev)))
> >> diff --git a/drivers/gpu/drm/i915/display/intel_tv.c b/drivers/gpu/drm/i915/display/intel_tv.c
> >> index 3b5ff84dc615..6f7ac225293e 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_tv.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_tv.c
> >> @@ -1204,6 +1204,7 @@ intel_tv_compute_config(struct intel_encoder *encoder,
> >>   	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> >>   		return -EINVAL;
> >>   
> >> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
> >>   	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
> >>   
> >>   	drm_dbg_kms(&dev_priv->drm, "forcing bpc to 8 for TV\n");
> >> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> >> index 8d2e6e151ba0..82c30feb7a91 100644
> >> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> >> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> >> @@ -280,6 +280,7 @@ static int intel_dsi_compute_config(struct intel_encoder *encoder,
> >>   	int ret;
> >>   
> >>   	drm_dbg_kms(&dev_priv->drm, "\n");
> >> +	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
> >>   	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
> >>   
> >>   	ret = intel_panel_compute_config(intel_connector, adjusted_mode);
> >> -- 
> >> 2.25.1

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH v11 06/11] drm/i915/dp: Consider output_format while computing dsc bpp for mode_valid

[Nautiyal, Ankit K]

On 3/17/2023 6:30 AM, Ville Syrjälä wrote:
> On Tue, Mar 14, 2023 at 04:34:10PM +0530, Ankit Nautiyal wrote:
>> During modevalid step, the pipe bpp is computed assuming RGB output
>> format. When checking with DSC, consider the output_format and compute
>> the input bpp for DSC appropriately.
>>
>> v2: For DP-MST we currently use RGB output format only, so continue
>> using RGB while computing dsc_bpp for MST case.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dp.c     | 28 ++++++++++++++++-----
>>   drivers/gpu/drm/i915/display/intel_dp.h     |  4 ++-
>>   drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 +-
>>   3 files changed, 26 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index dcb3c2519041..499390c519ca 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -1110,11 +1110,21 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>>   
>>   	if (HAS_DSC(dev_priv) &&
>>   	    drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)) {
>> +		int pipe_bpp;
>> +		enum intel_output_format output_format, sink_format;
>> +		const struct drm_display_info *info = &connector->base.display_info;
>> +
>> +		if (	(info, mode))
>> +			sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>> +		else
>> +			sink_format = INTEL_OUTPUT_FORMAT_RGB;
> I think I saw this same code duplicated somewhere else already.
> Time for a intel_dp_sink_format()?

Yes this can be made as a common function, also used in 
intel_dp_mode_min_output_bpp.

Will add the helper intel_dp_sink_format for this.

>
>> +
>> +		output_format = intel_dp_output_format(connector, sink_format);
>>   		/*
>>   		 * 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);
>> +		pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, output_format, U8_MAX);
>>   
>>   		/*
>>   		 * Output bpp is stored in 6.4 format so right shift by 4 to get the
>> @@ -1454,12 +1464,15 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
>>   	return -EINVAL;
>>   }
>>   
>> -int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8 max_req_bpc)
>> +int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp,
>> +			     enum intel_output_format output_format,
>> +			     u8 max_req_bpc)
>>   {
>>   	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>>   	int i, num_bpc;
>>   	u8 dsc_bpc[3] = {0};
>>   	u8 dsc_max_bpc;
>> +	int pipe_bpp = 0;
>>   
>>   	/* Max DSC Input BPC for ICL is 10 and for TGL+ is 12 */
>>   	if (DISPLAY_VER(i915) >= 12)
>> @@ -1470,11 +1483,13 @@ int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8 max_req_bpc)
>>   	num_bpc = drm_dp_dsc_sink_supported_input_bpcs(intel_dp->dsc_dpcd,
>>   						       dsc_bpc);
>>   	for (i = 0; i < num_bpc; i++) {
>> -		if (dsc_max_bpc >= dsc_bpc[i])
>> -			return dsc_bpc[i] * 3;
>> +		if (dsc_max_bpc >= dsc_bpc[i]) {
>> +			pipe_bpp = dsc_bpc[i] * 3;
>> +			break;
>> +		}
>>   	}
>>   
>> -	return 0;
>> +	return intel_dp_output_bpp(output_format, pipe_bpp);
> The pipe_bpp vs. output_bpp terms seem a bit confused now in the dsc
> code.
>
> In the non-compressed cases pipe_bpp does not include any
> subsampling, output_bpp is the subsampled version.
>
> What this dsc code seems to want is an intermediate value which
> is the subsampled pipe_bpp that is the input to dsc compressor?
> And output_bpp/dsc.compressed_bpp is then the final bpp coming
> out of the compressor.


Yes I now realize, this is wrong, it should have been pipe_bpp not the 
subsampled pipe_bpp.

As I understand, we are using subsampled pipe_bpp only to compute the 
bandwidth/clock checks.

You are right, we need use pipe_bpp for programming, and the same is 
input to DSC, and not the subsampled pipe_bpp.

I Will remove this line from the function.

>
> I think we should invent a consistent set of names for each so that
> it's clear which value the code is concerned with.

Yes you are right output_bpp is used as compressed_bpp in some places, 
functions.

We can go with something like this perhaps:

pipe_bpp : the actual bits_per_pixel, that we program

output_bpp : In case of YcbCr420,422 output format bytes_per_pixel will 
be half the number of bytes of RGB pixel.

compressed_bpp : (Only for DSC case) the target BPP for the DSC encoder, 
after compression.

Does this make sense? I can try to have a separate patch for this.


>>   }
>>   
>>   static int intel_dp_source_dsc_version_minor(struct intel_dp *intel_dp)
>> @@ -1588,7 +1603,8 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>>   		return -EINVAL;
>>   
>>   	if (compute_pipe_bpp)
>> -		pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, conn_state->max_requested_bpc);
>> +		pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, pipe_config->output_format,
>> +						    conn_state->max_requested_bpc);
> This pipe_bpp gets plugged back into crtc_state->pipe_bpp later and
> then it'll be the subsampled version. I don't think that is what we want
> eg. for dithering setup and whatnot.

You  are right, this is incorrect. Removing the line as mentioned above 
will fix this.

Thanks for pointing out this issue. I will fix this in next version of 
the patch.


Regards,

Ankit

>
>>   	else
>>   		pipe_bpp = pipe_config->pipe_bpp;
>>   
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
>> index ef39e4f7a329..2f4136e43f38 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.h
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
>> @@ -102,7 +102,9 @@ void intel_read_dp_sdp(struct intel_encoder *encoder,
>>   		       struct intel_crtc_state *crtc_state,
>>   		       unsigned int type);
>>   bool intel_digital_port_connected(struct intel_encoder *encoder);
>> -int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8 dsc_max_bpc);
>> +int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp,
>> +			     enum intel_output_format output_format,
>> +			     u8 dsc_max_bpc);
>>   u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
>>   				u32 link_clock, u32 lane_count,
>>   				u32 mode_clock, u32 mode_hdisplay,
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> index ff0b821a901a..bdc5c53ccd75 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> @@ -917,7 +917,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
>>   		 * 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);
>> +		int pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, INTEL_OUTPUT_FORMAT_RGB, U8_MAX);
>>   
>>   		if (drm_dp_sink_supports_fec(intel_dp->fec_capable)) {
>>   			dsc_max_output_bpp =
>> -- 
>> 2.25.1

Re: [Intel-gfx] [PATCH v11 06/11] drm/i915/dp: Consider output_format while computing dsc bpp for mode_valid

[Ville Syrjälä]

On Mon, Mar 20, 2023 at 09:06:25AM +0530, Nautiyal, Ankit K wrote:
> 
> On 3/17/2023 6:30 AM, Ville Syrjälä wrote:
> > On Tue, Mar 14, 2023 at 04:34:10PM +0530, Ankit Nautiyal wrote:
> >> During modevalid step, the pipe bpp is computed assuming RGB output
> >> format. When checking with DSC, consider the output_format and compute
> >> the input bpp for DSC appropriately.
> >>
> >> v2: For DP-MST we currently use RGB output format only, so continue
> >> using RGB while computing dsc_bpp for MST case.
> >>
> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/display/intel_dp.c     | 28 ++++++++++++++++-----
> >>   drivers/gpu/drm/i915/display/intel_dp.h     |  4 ++-
> >>   drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 +-
> >>   3 files changed, 26 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> >> index dcb3c2519041..499390c519ca 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >> @@ -1110,11 +1110,21 @@ intel_dp_mode_valid(struct drm_connector *_connector,
> >>   
> >>   	if (HAS_DSC(dev_priv) &&
> >>   	    drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)) {
> >> +		int pipe_bpp;
> >> +		enum intel_output_format output_format, sink_format;
> >> +		const struct drm_display_info *info = &connector->base.display_info;
> >> +
> >> +		if (	(info, mode))
> >> +			sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> >> +		else
> >> +			sink_format = INTEL_OUTPUT_FORMAT_RGB;
> > I think I saw this same code duplicated somewhere else already.
> > Time for a intel_dp_sink_format()?
> 
> Yes this can be made as a common function, also used in 
> intel_dp_mode_min_output_bpp.
> 
> Will add the helper intel_dp_sink_format for this.
> 
> >
> >> +
> >> +		output_format = intel_dp_output_format(connector, sink_format);
> >>   		/*
> >>   		 * 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);
> >> +		pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, output_format, U8_MAX);
> >>   
> >>   		/*
> >>   		 * Output bpp is stored in 6.4 format so right shift by 4 to get the
> >> @@ -1454,12 +1464,15 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
> >>   	return -EINVAL;
> >>   }
> >>   
> >> -int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8 max_req_bpc)
> >> +int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp,
> >> +			     enum intel_output_format output_format,
> >> +			     u8 max_req_bpc)
> >>   {
> >>   	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> >>   	int i, num_bpc;
> >>   	u8 dsc_bpc[3] = {0};
> >>   	u8 dsc_max_bpc;
> >> +	int pipe_bpp = 0;
> >>   
> >>   	/* Max DSC Input BPC for ICL is 10 and for TGL+ is 12 */
> >>   	if (DISPLAY_VER(i915) >= 12)
> >> @@ -1470,11 +1483,13 @@ int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8 max_req_bpc)
> >>   	num_bpc = drm_dp_dsc_sink_supported_input_bpcs(intel_dp->dsc_dpcd,
> >>   						       dsc_bpc);
> >>   	for (i = 0; i < num_bpc; i++) {
> >> -		if (dsc_max_bpc >= dsc_bpc[i])
> >> -			return dsc_bpc[i] * 3;
> >> +		if (dsc_max_bpc >= dsc_bpc[i]) {
> >> +			pipe_bpp = dsc_bpc[i] * 3;
> >> +			break;
> >> +		}
> >>   	}
> >>   
> >> -	return 0;
> >> +	return intel_dp_output_bpp(output_format, pipe_bpp);
> > The pipe_bpp vs. output_bpp terms seem a bit confused now in the dsc
> > code.
> >
> > In the non-compressed cases pipe_bpp does not include any
> > subsampling, output_bpp is the subsampled version.
> >
> > What this dsc code seems to want is an intermediate value which
> > is the subsampled pipe_bpp that is the input to dsc compressor?
> > And output_bpp/dsc.compressed_bpp is then the final bpp coming
> > out of the compressor.
> 
> 
> Yes I now realize, this is wrong, it should have been pipe_bpp not the 
> subsampled pipe_bpp.
> 
> As I understand, we are using subsampled pipe_bpp only to compute the 
> bandwidth/clock checks.
> 
> You are right, we need use pipe_bpp for programming, and the same is 
> input to DSC, and not the subsampled pipe_bpp.
> 
> I Will remove this line from the function.
> 
> >
> > I think we should invent a consistent set of names for each so that
> > it's clear which value the code is concerned with.
> 
> Yes you are right output_bpp is used as compressed_bpp in some places, 
> functions.
> 
> We can go with something like this perhaps:
> 
> pipe_bpp : the actual bits_per_pixel, that we program
> 
> output_bpp : In case of YcbCr420,422 output format bytes_per_pixel will 
> be half the number of bytes of RGB pixel.
> 
> compressed_bpp : (Only for DSC case) the target BPP for the DSC encoder, 
> after compression.
> 
> Does this make sense? I can try to have a separate patch for this.

Then I think we need a new name for the final bpp in
intel_dp_compute_config() at least. Hmm, maybe link_bpp?
intel_dp_compute_link_config_wide() should perhaps use that
same name as well.

> 
> 
> >>   }
> >>   
> >>   static int intel_dp_source_dsc_version_minor(struct intel_dp *intel_dp)
> >> @@ -1588,7 +1603,8 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> >>   		return -EINVAL;
> >>   
> >>   	if (compute_pipe_bpp)
> >> -		pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, conn_state->max_requested_bpc);
> >> +		pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, pipe_config->output_format,
> >> +						    conn_state->max_requested_bpc);
> > This pipe_bpp gets plugged back into crtc_state->pipe_bpp later and
> > then it'll be the subsampled version. I don't think that is what we want
> > eg. for dithering setup and whatnot.
> 
> You  are right, this is incorrect. Removing the line as mentioned above 
> will fix this.
> 
> Thanks for pointing out this issue. I will fix this in next version of 
> the patch.
> 
> 
> Regards,
> 
> Ankit
> 
> >
> >>   	else
> >>   		pipe_bpp = pipe_config->pipe_bpp;
> >>   
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> >> index ef39e4f7a329..2f4136e43f38 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> >> @@ -102,7 +102,9 @@ void intel_read_dp_sdp(struct intel_encoder *encoder,
> >>   		       struct intel_crtc_state *crtc_state,
> >>   		       unsigned int type);
> >>   bool intel_digital_port_connected(struct intel_encoder *encoder);
> >> -int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8 dsc_max_bpc);
> >> +int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp,
> >> +			     enum intel_output_format output_format,
> >> +			     u8 dsc_max_bpc);
> >>   u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
> >>   				u32 link_clock, u32 lane_count,
> >>   				u32 mode_clock, u32 mode_hdisplay,
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> >> index ff0b821a901a..bdc5c53ccd75 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> >> @@ -917,7 +917,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
> >>   		 * 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);
> >> +		int pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, INTEL_OUTPUT_FORMAT_RGB, U8_MAX);
> >>   
> >>   		if (drm_dp_sink_supports_fec(intel_dp->fec_capable)) {
> >>   			dsc_max_output_bpp =
> >> -- 
> >> 2.25.1

-- 
Ville Syrjälä
Intel