All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] drm/i915/display: Try YCbCr420 color when RGB fails
@ 2021-05-06 17:23 ` Werner Sembach
  0 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-06 17:23 UTC (permalink / raw)
  To: ville.syrjala, airlied, daniel, intel-gfx, dri-devel, linux-kernel

When encoder validation of a display mode fails, retry with less bandwidth
heavy YCbCr420 color mode, if available. This enables some HDMI 1.4 setups
to support 4k60Hz output, which previously failed silently.

AMDGPU had nearly the exact same issue. This problem description is
therefore copied from my commit message of the AMDGPU patch.

On some setups, while the monitor and the gpu support display modes with
pixel clocks of up to 600MHz, the link encoder might not. This prevents
YCbCr444 and RGB encoding for 4k60Hz, but YCbCr420 encoding might still be
possible. However, which color mode is used is decided before the link
encoder capabilities are checked. This patch fixes the problem by retrying
to find a display mode with YCbCr420 enforced and using it, if it is
valid.

This patchset is revision 5. Only change to 4 is a small whitespace error fix.
Resend because automation tools did not work with wrong e-mail format used
before.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Intel-gfx] [PATCH 0/3] drm/i915/display: Try YCbCr420 color when RGB fails
@ 2021-05-06 17:23 ` Werner Sembach
  0 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-06 17:23 UTC (permalink / raw)
  To: ville.syrjala, airlied, daniel, intel-gfx, dri-devel, linux-kernel

When encoder validation of a display mode fails, retry with less bandwidth
heavy YCbCr420 color mode, if available. This enables some HDMI 1.4 setups
to support 4k60Hz output, which previously failed silently.

AMDGPU had nearly the exact same issue. This problem description is
therefore copied from my commit message of the AMDGPU patch.

On some setups, while the monitor and the gpu support display modes with
pixel clocks of up to 600MHz, the link encoder might not. This prevents
YCbCr444 and RGB encoding for 4k60Hz, but YCbCr420 encoding might still be
possible. However, which color mode is used is decided before the link
encoder capabilities are checked. This patch fixes the problem by retrying
to find a display mode with YCbCr420 enforced and using it, if it is
valid.

This patchset is revision 5. Only change to 4 is a small whitespace error fix.
Resend because automation tools did not work with wrong e-mail format used
before.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 1/3] drm/i915/display: New function to avoid duplicate code in upcomming commits
  2021-05-06 17:23 ` [Intel-gfx] " Werner Sembach
@ 2021-05-06 17:23   ` Werner Sembach
  -1 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-06 17:23 UTC (permalink / raw)
  To: ville.syrjala, airlied, daniel, intel-gfx, dri-devel, linux-kernel
  Cc: Werner Sembach

Moves some checks that later will be performed 2 times to an own fuction. This
avoids duplicate code later on.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 41 ++++++++++++++---------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 46de56af33db..576d3d910d06 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1861,6 +1861,31 @@ static int intel_hdmi_port_clock(int clock, int bpc)
 	return clock * bpc / 8;
 }
 
+static enum drm_mode_status
+intel_hdmi_mode_clock_valid(struct intel_hdmi *hdmi, int clock, bool has_hdmi_sink)
+{
+	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	enum drm_mode_status status;
+
+	/* check if we can do 8bpc */
+	status = hdmi_port_clock_valid(hdmi, clock, true, has_hdmi_sink);
+
+	if (has_hdmi_sink) {
+		/* if we can't do 8bpc we may still be able to do 12bpc */
+		if (status != MODE_OK && !HAS_GMCH(dev_priv))
+			status = hdmi_port_clock_valid(hdmi, clock * 3 / 2,
+						       true, has_hdmi_sink);
+
+		/* if we can't do 8,12bpc we may still be able to do 10bpc */
+		if (status != MODE_OK && INTEL_GEN(dev_priv) >= 11)
+			status = hdmi_port_clock_valid(hdmi, clock * 5 / 4,
+						       true, has_hdmi_sink);
+	}
+
+	return status;
+}
+
 static enum drm_mode_status
 intel_hdmi_mode_valid(struct drm_connector *connector,
 		      struct drm_display_mode *mode)
@@ -1891,21 +1916,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 	if (drm_mode_is_420_only(&connector->display_info, mode))
 		clock /= 2;
 
-	/* check if we can do 8bpc */
-	status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 8),
-				       true, has_hdmi_sink);
-
-	if (has_hdmi_sink) {
-		/* if we can't do 8bpc we may still be able to do 12bpc */
-		if (status != MODE_OK && !HAS_GMCH(dev_priv))
-			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 12),
-						       true, has_hdmi_sink);
-
-		/* if we can't do 8,12bpc we may still be able to do 10bpc */
-		if (status != MODE_OK && DISPLAY_VER(dev_priv) >= 11)
-			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 10),
-						       true, has_hdmi_sink);
-	}
+	status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
 	if (status != MODE_OK)
 		return status;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Intel-gfx] [PATCH 1/3] drm/i915/display: New function to avoid duplicate code in upcomming commits
@ 2021-05-06 17:23   ` Werner Sembach
  0 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-06 17:23 UTC (permalink / raw)
  To: ville.syrjala, airlied, daniel, intel-gfx, dri-devel, linux-kernel
  Cc: Werner Sembach

Moves some checks that later will be performed 2 times to an own fuction. This
avoids duplicate code later on.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 41 ++++++++++++++---------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 46de56af33db..576d3d910d06 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1861,6 +1861,31 @@ static int intel_hdmi_port_clock(int clock, int bpc)
 	return clock * bpc / 8;
 }
 
+static enum drm_mode_status
+intel_hdmi_mode_clock_valid(struct intel_hdmi *hdmi, int clock, bool has_hdmi_sink)
+{
+	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	enum drm_mode_status status;
+
+	/* check if we can do 8bpc */
+	status = hdmi_port_clock_valid(hdmi, clock, true, has_hdmi_sink);
+
+	if (has_hdmi_sink) {
+		/* if we can't do 8bpc we may still be able to do 12bpc */
+		if (status != MODE_OK && !HAS_GMCH(dev_priv))
+			status = hdmi_port_clock_valid(hdmi, clock * 3 / 2,
+						       true, has_hdmi_sink);
+
+		/* if we can't do 8,12bpc we may still be able to do 10bpc */
+		if (status != MODE_OK && INTEL_GEN(dev_priv) >= 11)
+			status = hdmi_port_clock_valid(hdmi, clock * 5 / 4,
+						       true, has_hdmi_sink);
+	}
+
+	return status;
+}
+
 static enum drm_mode_status
 intel_hdmi_mode_valid(struct drm_connector *connector,
 		      struct drm_display_mode *mode)
@@ -1891,21 +1916,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 	if (drm_mode_is_420_only(&connector->display_info, mode))
 		clock /= 2;
 
-	/* check if we can do 8bpc */
-	status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 8),
-				       true, has_hdmi_sink);
-
-	if (has_hdmi_sink) {
-		/* if we can't do 8bpc we may still be able to do 12bpc */
-		if (status != MODE_OK && !HAS_GMCH(dev_priv))
-			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 12),
-						       true, has_hdmi_sink);
-
-		/* if we can't do 8,12bpc we may still be able to do 10bpc */
-		if (status != MODE_OK && DISPLAY_VER(dev_priv) >= 11)
-			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 10),
-						       true, has_hdmi_sink);
-	}
+	status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
 	if (status != MODE_OK)
 		return status;
 
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 2/3] drm/i915/display: Restructure output format computation for better expandability
  2021-05-06 17:23 ` [Intel-gfx] " Werner Sembach
@ 2021-05-06 17:23   ` Werner Sembach
  -1 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-06 17:23 UTC (permalink / raw)
  To: ville.syrjala, airlied, daniel, intel-gfx, dri-devel, linux-kernel
  Cc: Werner Sembach

Couples the decission between RGB and YCbCr420 mode and the check if the port
clock can archive the required frequency. Other checks and configuration steps
that where previously done in between can also be done before or after.

This allows for are cleaner implementation of retrying different color
encodings.

Slight change in behaviour: If YCbCr420 is not allowed but display is YCbCr420
only it no longer fails, but just prints an error and tries to fallback on RGB.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 65 ++++++++++++-----------
 1 file changed, 34 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 576d3d910d06..b0201d4f27eb 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1999,29 +1999,6 @@ static bool hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
 					      INTEL_OUTPUT_FORMAT_YCBCR420);
 }
 
-static int
-intel_hdmi_ycbcr420_config(struct intel_crtc_state *crtc_state,
-			   const struct drm_connector_state *conn_state)
-{
-	struct drm_connector *connector = conn_state->connector;
-	struct drm_i915_private *i915 = to_i915(connector->dev);
-	const struct drm_display_mode *adjusted_mode =
-		&crtc_state->hw.adjusted_mode;
-
-	if (!drm_mode_is_420_only(&connector->display_info, adjusted_mode))
-		return 0;
-
-	if (!connector->ycbcr_420_allowed) {
-		drm_err(&i915->drm,
-			"Platform doesn't support YCBCR420 output\n");
-		return -EINVAL;
-	}
-
-	crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
-
-	return intel_pch_panel_fitting(crtc_state, conn_state);
-}
-
 static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
 				  struct intel_crtc_state *crtc_state,
 				  int clock)
@@ -2128,6 +2105,30 @@ static bool intel_hdmi_has_audio(struct intel_encoder *encoder,
 		return intel_conn_state->force_audio == HDMI_AUDIO_ON;
 }
 
+static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
+				     struct intel_crtc_state *crtc_state,
+				     const struct drm_connector_state *conn_state)
+{
+	struct drm_connector *connector = conn_state->connector;
+	struct drm_i915_private *i915 = to_i915(connector->dev);
+	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
+	int ret;
+	bool ycbcr_420_only;
+
+	ycbcr_420_only = drm_mode_is_420_only(&connector->display_info, adjusted_mode);
+	if (connector->ycbcr_420_allowed && ycbcr_420_only)
+		crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+	else {
+		if (!connector->ycbcr_420_allowed && ycbcr_420_only)
+			drm_err(&i915->drm, "Display only supports YCbCr420 output, but connector does not allow it. Fallback to RGB, but this will likely fail.\n");
+		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
+	}
+
+	ret = intel_hdmi_compute_clock(encoder, crtc_state);
+
+	return ret;
+}
+
 int intel_hdmi_compute_config(struct intel_encoder *encoder,
 			      struct intel_crtc_state *pipe_config,
 			      struct drm_connector_state *conn_state)
@@ -2152,23 +2153,25 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
 		pipe_config->pixel_multiplier = 2;
 
-	ret = intel_hdmi_ycbcr420_config(pipe_config, conn_state);
-	if (ret)
-		return ret;
-
-	pipe_config->limited_color_range =
-		intel_hdmi_limited_color_range(pipe_config, conn_state);
-
 	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv))
 		pipe_config->has_pch_encoder = true;
 
 	pipe_config->has_audio =
 		intel_hdmi_has_audio(encoder, pipe_config, conn_state);
 
-	ret = intel_hdmi_compute_clock(encoder, pipe_config);
+	ret = intel_hdmi_compute_output_format(encoder, pipe_config, conn_state);
 	if (ret)
 		return ret;
 
+	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
+		ret = intel_pch_panel_fitting(pipe_config, conn_state);
+		if (ret)
+			return ret;
+	}
+
+	pipe_config->limited_color_range =
+		intel_hdmi_limited_color_range(pipe_config, conn_state);
+
 	if (conn_state->picture_aspect_ratio)
 		adjusted_mode->picture_aspect_ratio =
 			conn_state->picture_aspect_ratio;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Intel-gfx] [PATCH 2/3] drm/i915/display: Restructure output format computation for better expandability
@ 2021-05-06 17:23   ` Werner Sembach
  0 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-06 17:23 UTC (permalink / raw)
  To: ville.syrjala, airlied, daniel, intel-gfx, dri-devel, linux-kernel
  Cc: Werner Sembach

Couples the decission between RGB and YCbCr420 mode and the check if the port
clock can archive the required frequency. Other checks and configuration steps
that where previously done in between can also be done before or after.

This allows for are cleaner implementation of retrying different color
encodings.

Slight change in behaviour: If YCbCr420 is not allowed but display is YCbCr420
only it no longer fails, but just prints an error and tries to fallback on RGB.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 65 ++++++++++++-----------
 1 file changed, 34 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 576d3d910d06..b0201d4f27eb 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1999,29 +1999,6 @@ static bool hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
 					      INTEL_OUTPUT_FORMAT_YCBCR420);
 }
 
-static int
-intel_hdmi_ycbcr420_config(struct intel_crtc_state *crtc_state,
-			   const struct drm_connector_state *conn_state)
-{
-	struct drm_connector *connector = conn_state->connector;
-	struct drm_i915_private *i915 = to_i915(connector->dev);
-	const struct drm_display_mode *adjusted_mode =
-		&crtc_state->hw.adjusted_mode;
-
-	if (!drm_mode_is_420_only(&connector->display_info, adjusted_mode))
-		return 0;
-
-	if (!connector->ycbcr_420_allowed) {
-		drm_err(&i915->drm,
-			"Platform doesn't support YCBCR420 output\n");
-		return -EINVAL;
-	}
-
-	crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
-
-	return intel_pch_panel_fitting(crtc_state, conn_state);
-}
-
 static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
 				  struct intel_crtc_state *crtc_state,
 				  int clock)
@@ -2128,6 +2105,30 @@ static bool intel_hdmi_has_audio(struct intel_encoder *encoder,
 		return intel_conn_state->force_audio == HDMI_AUDIO_ON;
 }
 
+static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
+				     struct intel_crtc_state *crtc_state,
+				     const struct drm_connector_state *conn_state)
+{
+	struct drm_connector *connector = conn_state->connector;
+	struct drm_i915_private *i915 = to_i915(connector->dev);
+	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
+	int ret;
+	bool ycbcr_420_only;
+
+	ycbcr_420_only = drm_mode_is_420_only(&connector->display_info, adjusted_mode);
+	if (connector->ycbcr_420_allowed && ycbcr_420_only)
+		crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+	else {
+		if (!connector->ycbcr_420_allowed && ycbcr_420_only)
+			drm_err(&i915->drm, "Display only supports YCbCr420 output, but connector does not allow it. Fallback to RGB, but this will likely fail.\n");
+		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
+	}
+
+	ret = intel_hdmi_compute_clock(encoder, crtc_state);
+
+	return ret;
+}
+
 int intel_hdmi_compute_config(struct intel_encoder *encoder,
 			      struct intel_crtc_state *pipe_config,
 			      struct drm_connector_state *conn_state)
@@ -2152,23 +2153,25 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
 		pipe_config->pixel_multiplier = 2;
 
-	ret = intel_hdmi_ycbcr420_config(pipe_config, conn_state);
-	if (ret)
-		return ret;
-
-	pipe_config->limited_color_range =
-		intel_hdmi_limited_color_range(pipe_config, conn_state);
-
 	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv))
 		pipe_config->has_pch_encoder = true;
 
 	pipe_config->has_audio =
 		intel_hdmi_has_audio(encoder, pipe_config, conn_state);
 
-	ret = intel_hdmi_compute_clock(encoder, pipe_config);
+	ret = intel_hdmi_compute_output_format(encoder, pipe_config, conn_state);
 	if (ret)
 		return ret;
 
+	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
+		ret = intel_pch_panel_fitting(pipe_config, conn_state);
+		if (ret)
+			return ret;
+	}
+
+	pipe_config->limited_color_range =
+		intel_hdmi_limited_color_range(pipe_config, conn_state);
+
 	if (conn_state->picture_aspect_ratio)
 		adjusted_mode->picture_aspect_ratio =
 			conn_state->picture_aspect_ratio;
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 3/3] drm/i915/display: Use YCbCr420 as fallback when RGB fails
  2021-05-06 17:23 ` [Intel-gfx] " Werner Sembach
@ 2021-05-06 17:23   ` Werner Sembach
  -1 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-06 17:23 UTC (permalink / raw)
  To: ville.syrjala, airlied, daniel, intel-gfx, dri-devel, linux-kernel
  Cc: Werner Sembach

When encoder validation of a display mode fails, retry with less bandwidth
heavy YCbCr420 color mode, if available. This enables some HDMI 1.4 setups
to support 4k60Hz output, which previously failed silently.

AMDGPU had nearly the exact same issue. This problem description is
therefore copied from my commit message of the AMDGPU patch.

On some setups, while the monitor and the gpu support display modes with
pixel clocks of up to 600MHz, the link encoder might not. This prevents
YCbCr444 and RGB encoding for 4k60Hz, but YCbCr420 encoding might still be
possible. However, which color mode is used is decided before the link
encoder capabilities are checked. This patch fixes the problem by retrying
to find a display mode with YCbCr420 enforced and using it, if it is
valid.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index b0201d4f27eb..7815569267e3 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1897,6 +1897,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 	int clock = mode->clock;
 	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;
 
 	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
 		return MODE_NO_DBLESCAN;
@@ -1913,12 +1914,20 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 		clock *= 2;
 	}
 
-	if (drm_mode_is_420_only(&connector->display_info, mode))
+	ycbcr_420_only = drm_mode_is_420_only(&connector->display_info, mode);
+	if (ycbcr_420_only)
 		clock /= 2;
 
 	status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
-	if (status != MODE_OK)
-		return status;
+	if (status != MODE_OK) {
+		if (ycbcr_420_only || !connector->ycbcr_420_allowed || !drm_mode_is_420_also(&connector->display_info, mode))
+			return status;
+
+		clock /= 2;
+		status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
+		if (status != MODE_OK)
+			return status;
+	}
 
 	return intel_mode_valid_max_plane_size(dev_priv, mode, false);
 }
@@ -2125,6 +2134,14 @@ static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
 	}
 
 	ret = intel_hdmi_compute_clock(encoder, crtc_state);
+	if (ret) {
+		if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420 &&
+				connector->ycbcr_420_allowed &&
+				drm_mode_is_420_also(&connector->display_info, adjusted_mode)) {
+			crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+			ret = intel_hdmi_compute_clock(encoder, crtc_state);
+		}
+	}
 
 	return ret;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Intel-gfx] [PATCH 3/3] drm/i915/display: Use YCbCr420 as fallback when RGB fails
@ 2021-05-06 17:23   ` Werner Sembach
  0 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-06 17:23 UTC (permalink / raw)
  To: ville.syrjala, airlied, daniel, intel-gfx, dri-devel, linux-kernel
  Cc: Werner Sembach

When encoder validation of a display mode fails, retry with less bandwidth
heavy YCbCr420 color mode, if available. This enables some HDMI 1.4 setups
to support 4k60Hz output, which previously failed silently.

AMDGPU had nearly the exact same issue. This problem description is
therefore copied from my commit message of the AMDGPU patch.

On some setups, while the monitor and the gpu support display modes with
pixel clocks of up to 600MHz, the link encoder might not. This prevents
YCbCr444 and RGB encoding for 4k60Hz, but YCbCr420 encoding might still be
possible. However, which color mode is used is decided before the link
encoder capabilities are checked. This patch fixes the problem by retrying
to find a display mode with YCbCr420 enforced and using it, if it is
valid.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index b0201d4f27eb..7815569267e3 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1897,6 +1897,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 	int clock = mode->clock;
 	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;
 
 	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
 		return MODE_NO_DBLESCAN;
@@ -1913,12 +1914,20 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 		clock *= 2;
 	}
 
-	if (drm_mode_is_420_only(&connector->display_info, mode))
+	ycbcr_420_only = drm_mode_is_420_only(&connector->display_info, mode);
+	if (ycbcr_420_only)
 		clock /= 2;
 
 	status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
-	if (status != MODE_OK)
-		return status;
+	if (status != MODE_OK) {
+		if (ycbcr_420_only || !connector->ycbcr_420_allowed || !drm_mode_is_420_also(&connector->display_info, mode))
+			return status;
+
+		clock /= 2;
+		status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
+		if (status != MODE_OK)
+			return status;
+	}
 
 	return intel_mode_valid_max_plane_size(dev_priv, mode, false);
 }
@@ -2125,6 +2134,14 @@ static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
 	}
 
 	ret = intel_hdmi_compute_clock(encoder, crtc_state);
+	if (ret) {
+		if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420 &&
+				connector->ycbcr_420_allowed &&
+				drm_mode_is_420_also(&connector->display_info, adjusted_mode)) {
+			crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+			ret = intel_hdmi_compute_clock(encoder, crtc_state);
+		}
+	}
 
 	return ret;
 }
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/display: Try YCbCr420 color when RGB fails
  2021-05-06 17:23 ` [Intel-gfx] " Werner Sembach
                   ` (3 preceding siblings ...)
  (?)
@ 2021-05-06 17:35 ` Patchwork
  -1 siblings, 0 replies; 34+ messages in thread
From: Patchwork @ 2021-05-06 17:35 UTC (permalink / raw)
  To: Werner Sembach; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/display: Try YCbCr420 color when RGB fails
URL   : https://patchwork.freedesktop.org/series/89842/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
00bf5b4bbe7e drm/i915/display: New function to avoid duplicate code in upcomming commits
-:7: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#7: 
Moves some checks that later will be performed 2 times to an own fuction. This

-:7: WARNING:TYPO_SPELLING: 'fuction' may be misspelled - perhaps 'function'?
#7: 
Moves some checks that later will be performed 2 times to an own fuction. This
                                                                 ^^^^^^^

total: 0 errors, 2 warnings, 0 checks, 53 lines checked
6cff290a93e1 drm/i915/display: Restructure output format computation for better expandability
-:7: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#7: 
Couples the decission between RGB and YCbCr420 mode and the check if the port

-:58: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#58: FILE: drivers/gpu/drm/i915/display/intel_hdmi.c:2110:
+static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
+				     struct intel_crtc_state *crtc_state,

-:68: CHECK:BRACES: braces {} should be used on all arms of this statement
#68: FILE: drivers/gpu/drm/i915/display/intel_hdmi.c:2120:
+	if (connector->ycbcr_420_allowed && ycbcr_420_only)
[...]
+	else {
[...]

-:70: CHECK:BRACES: Unbalanced braces around else statement
#70: FILE: drivers/gpu/drm/i915/display/intel_hdmi.c:2122:
+	else {

total: 0 errors, 1 warnings, 3 checks, 92 lines checked
fa1ae5bd837f drm/i915/display: Use YCbCr420 as fallback when RGB fails
-:48: WARNING:LONG_LINE: line length of 125 exceeds 100 columns
#48: FILE: drivers/gpu/drm/i915/display/intel_hdmi.c:1924:
+		if (ycbcr_420_only || !connector->ycbcr_420_allowed || !drm_mode_is_420_also(&connector->display_info, mode))

-:65: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#65: FILE: drivers/gpu/drm/i915/display/intel_hdmi.c:2140:
+		if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420 &&
+				connector->ycbcr_420_allowed &&

total: 0 errors, 1 warnings, 1 checks, 44 lines checked


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/display: Try YCbCr420 color when RGB fails
  2021-05-06 17:23 ` [Intel-gfx] " Werner Sembach
                   ` (4 preceding siblings ...)
  (?)
@ 2021-05-06 18:04 ` Patchwork
  -1 siblings, 0 replies; 34+ messages in thread
From: Patchwork @ 2021-05-06 18:04 UTC (permalink / raw)
  To: Werner Sembach; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 2291 bytes --]

== Series Details ==

Series: drm/i915/display: Try YCbCr420 color when RGB fails
URL   : https://patchwork.freedesktop.org/series/89842/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10053 -> Patchwork_20079
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@hangcheck:
    - fi-snb-2600:        [PASS][1] -> [INCOMPLETE][2] ([i915#2782])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/fi-snb-2600/igt@i915_selftest@live@hangcheck.html

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

  [i915#2782]: https://gitlab.freedesktop.org/drm/intel/issues/2782
  [i915#3277]: https://gitlab.freedesktop.org/drm/intel/issues/3277
  [i915#3283]: https://gitlab.freedesktop.org/drm/intel/issues/3283
  [k.org#205379]: https://bugzilla.kernel.org/show_bug.cgi?id=205379


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

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-bdw-gvtdvm fi-ctg-p8600 fi-bsw-kefka fi-bdw-samus 


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

  * Linux: CI_DRM_10053 -> Patchwork_20079

  CI-20190529: 20190529
  CI_DRM_10053: 3e000bbf311ad04f734843e1ba6396b28ba44399 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6080: 1c450c3d4df19cf1087b8ccff3b62cb51addacae @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_20079: fa1ae5bd837fe78b388ca039c1a80403536eb20f @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

fa1ae5bd837f drm/i915/display: Use YCbCr420 as fallback when RGB fails
6cff290a93e1 drm/i915/display: Restructure output format computation for better expandability
00bf5b4bbe7e drm/i915/display: New function to avoid duplicate code in upcomming commits

== Logs ==

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

[-- Attachment #1.2: Type: text/html, Size: 2700 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/display: Try YCbCr420 color when RGB fails
  2021-05-06 17:23 ` [Intel-gfx] " Werner Sembach
                   ` (5 preceding siblings ...)
  (?)
@ 2021-05-06 19:21 ` Patchwork
  -1 siblings, 0 replies; 34+ messages in thread
From: Patchwork @ 2021-05-06 19:21 UTC (permalink / raw)
  To: Werner Sembach; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 30274 bytes --]

== Series Details ==

Series: drm/i915/display: Try YCbCr420 color when RGB fails
URL   : https://patchwork.freedesktop.org/series/89842/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10053_full -> Patchwork_20079_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@feature_discovery@display-2x:
    - shard-iclb:         NOTRUN -> [SKIP][1] ([i915#1839])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@feature_discovery@display-2x.html

  * igt@feature_discovery@psr2:
    - shard-iclb:         NOTRUN -> [SKIP][2] ([i915#658])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@feature_discovery@psr2.html

  * igt@gem_create@create-clear:
    - shard-glk:          [PASS][3] -> [FAIL][4] ([i915#1888] / [i915#3160])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-glk5/igt@gem_create@create-clear.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-glk7/igt@gem_create@create-clear.html

  * igt@gem_ctx_persistence@engines-mixed-process:
    - shard-snb:          NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#1099]) +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-snb5/igt@gem_ctx_persistence@engines-mixed-process.html

  * igt@gem_eio@unwedge-stress:
    - shard-tglb:         [PASS][6] -> [TIMEOUT][7] ([i915#2369] / [i915#3063])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-tglb1/igt@gem_eio@unwedge-stress.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-tglb3/igt@gem_eio@unwedge-stress.html
    - shard-iclb:         [PASS][8] -> [TIMEOUT][9] ([i915#2369] / [i915#2481] / [i915#3070])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-iclb6/igt@gem_eio@unwedge-stress.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb4/igt@gem_eio@unwedge-stress.html
    - shard-snb:          NOTRUN -> [FAIL][10] ([i915#3354])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-snb5/igt@gem_eio@unwedge-stress.html

  * igt@gem_exec_fair@basic-deadline:
    - shard-tglb:         [PASS][11] -> [FAIL][12] ([i915#2846])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-tglb3/igt@gem_exec_fair@basic-deadline.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-tglb2/igt@gem_exec_fair@basic-deadline.html

  * igt@gem_exec_fair@basic-none-vip@rcs0:
    - shard-kbl:          NOTRUN -> [FAIL][13] ([i915#2842])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-kbl6/igt@gem_exec_fair@basic-none-vip@rcs0.html

  * igt@gem_exec_fair@basic-none@vcs1:
    - shard-iclb:         NOTRUN -> [FAIL][14] ([i915#2842])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb2/igt@gem_exec_fair@basic-none@vcs1.html

  * igt@gem_exec_fair@basic-none@vecs0:
    - shard-apl:          NOTRUN -> [FAIL][15] ([i915#2842])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-apl1/igt@gem_exec_fair@basic-none@vecs0.html

  * igt@gem_exec_fair@basic-pace-solo@rcs0:
    - shard-iclb:         [PASS][16] -> [FAIL][17] ([i915#2842])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-iclb3/igt@gem_exec_fair@basic-pace-solo@rcs0.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb3/igt@gem_exec_fair@basic-pace-solo@rcs0.html

  * igt@gem_exec_fair@basic-pace@vecs0:
    - shard-kbl:          [PASS][18] -> [SKIP][19] ([fdo#109271])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-kbl1/igt@gem_exec_fair@basic-pace@vecs0.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-kbl1/igt@gem_exec_fair@basic-pace@vecs0.html

  * igt@gem_exec_reloc@basic-wide-active@vcs1:
    - shard-iclb:         NOTRUN -> [FAIL][20] ([i915#2389])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb2/igt@gem_exec_reloc@basic-wide-active@vcs1.html

  * igt@gem_mmap_gtt@big-copy-xy:
    - shard-skl:          [PASS][21] -> [FAIL][22] ([i915#307])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-skl9/igt@gem_mmap_gtt@big-copy-xy.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl9/igt@gem_mmap_gtt@big-copy-xy.html

  * igt@gem_pwrite@basic-exhaustion:
    - shard-apl:          NOTRUN -> [WARN][23] ([i915#2658])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-apl2/igt@gem_pwrite@basic-exhaustion.html

  * igt@gem_render_copy@y-tiled-ccs-to-yf-tiled-mc-ccs:
    - shard-iclb:         NOTRUN -> [SKIP][24] ([i915#768])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@gem_render_copy@y-tiled-ccs-to-yf-tiled-mc-ccs.html

  * igt@gem_userptr_blits@input-checking:
    - shard-apl:          NOTRUN -> [DMESG-WARN][25] ([i915#3002])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-apl8/igt@gem_userptr_blits@input-checking.html

  * igt@gem_userptr_blits@vma-merge:
    - shard-snb:          NOTRUN -> [FAIL][26] ([i915#2724])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-snb5/igt@gem_userptr_blits@vma-merge.html

  * igt@gem_workarounds@suspend-resume-fd:
    - shard-kbl:          [PASS][27] -> [DMESG-WARN][28] ([i915#180]) +4 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-kbl1/igt@gem_workarounds@suspend-resume-fd.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-kbl1/igt@gem_workarounds@suspend-resume-fd.html

  * igt@gen7_exec_parse@batch-without-end:
    - shard-iclb:         NOTRUN -> [SKIP][29] ([fdo#109289])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@gen7_exec_parse@batch-without-end.html

  * igt@gen9_exec_parse@allowed-single:
    - shard-skl:          [PASS][30] -> [DMESG-WARN][31] ([i915#1436] / [i915#716])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-skl8/igt@gen9_exec_parse@allowed-single.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl9/igt@gen9_exec_parse@allowed-single.html

  * igt@gen9_exec_parse@batch-invalid-length:
    - shard-snb:          NOTRUN -> [SKIP][32] ([fdo#109271]) +241 similar issues
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-snb5/igt@gen9_exec_parse@batch-invalid-length.html

  * igt@i915_pm_sseu@full-enable:
    - shard-iclb:         NOTRUN -> [SKIP][33] ([fdo#109288])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@i915_pm_sseu@full-enable.html

  * igt@i915_selftest@live@hangcheck:
    - shard-snb:          NOTRUN -> [INCOMPLETE][34] ([i915#2782])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-snb7/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@mock@requests:
    - shard-skl:          [PASS][35] -> [INCOMPLETE][36] ([i915#198])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-skl4/igt@i915_selftest@mock@requests.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl2/igt@i915_selftest@mock@requests.html

  * igt@kms_atomic_transition@plane-all-modeset-transition:
    - shard-iclb:         NOTRUN -> [SKIP][37] ([i915#1769])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@kms_atomic_transition@plane-all-modeset-transition.html

  * igt@kms_big_fb@yf-tiled-8bpp-rotate-0:
    - shard-iclb:         NOTRUN -> [SKIP][38] ([fdo#110723])
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@kms_big_fb@yf-tiled-8bpp-rotate-0.html

  * igt@kms_big_joiner@invalid-modeset:
    - shard-apl:          NOTRUN -> [SKIP][39] ([fdo#109271] / [i915#2705])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-apl2/igt@kms_big_joiner@invalid-modeset.html

  * igt@kms_ccs@pipe-c-bad-pixel-format:
    - shard-skl:          NOTRUN -> [SKIP][40] ([fdo#109271] / [fdo#111304])
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl7/igt@kms_ccs@pipe-c-bad-pixel-format.html

  * igt@kms_chamelium@dp-mode-timings:
    - shard-apl:          NOTRUN -> [SKIP][41] ([fdo#109271] / [fdo#111827]) +20 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-apl8/igt@kms_chamelium@dp-mode-timings.html

  * igt@kms_chamelium@vga-hpd-after-suspend:
    - shard-skl:          NOTRUN -> [SKIP][42] ([fdo#109271] / [fdo#111827]) +10 similar issues
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl1/igt@kms_chamelium@vga-hpd-after-suspend.html

  * igt@kms_chamelium@vga-hpd-without-ddc:
    - shard-kbl:          NOTRUN -> [SKIP][43] ([fdo#109271] / [fdo#111827]) +5 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-kbl6/igt@kms_chamelium@vga-hpd-without-ddc.html

  * igt@kms_color@pipe-a-ctm-max:
    - shard-skl:          [PASS][44] -> [DMESG-WARN][45] ([i915#1982])
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-skl8/igt@kms_color@pipe-a-ctm-max.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl8/igt@kms_color@pipe-a-ctm-max.html

  * igt@kms_color_chamelium@pipe-a-ctm-0-25:
    - shard-snb:          NOTRUN -> [SKIP][46] ([fdo#109271] / [fdo#111827]) +14 similar issues
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-snb5/igt@kms_color_chamelium@pipe-a-ctm-0-25.html

  * igt@kms_color_chamelium@pipe-a-ctm-blue-to-red:
    - shard-iclb:         NOTRUN -> [SKIP][47] ([fdo#109284] / [fdo#111827]) +2 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@kms_color_chamelium@pipe-a-ctm-blue-to-red.html

  * igt@kms_color_chamelium@pipe-d-ctm-max:
    - shard-iclb:         NOTRUN -> [SKIP][48] ([fdo#109278] / [fdo#109284] / [fdo#111827])
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@kms_color_chamelium@pipe-d-ctm-max.html

  * igt@kms_content_protection@atomic-dpms:
    - shard-apl:          NOTRUN -> [TIMEOUT][49] ([i915#1319]) +1 similar issue
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-apl3/igt@kms_content_protection@atomic-dpms.html
    - shard-kbl:          NOTRUN -> [TIMEOUT][50] ([i915#1319])
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-kbl6/igt@kms_content_protection@atomic-dpms.html

  * igt@kms_content_protection@mei_interface:
    - shard-iclb:         NOTRUN -> [SKIP][51] ([fdo#109300] / [fdo#111066])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@kms_content_protection@mei_interface.html

  * igt@kms_cursor_crc@pipe-a-cursor-512x170-random:
    - shard-iclb:         NOTRUN -> [SKIP][52] ([fdo#109278] / [fdo#109279])
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@kms_cursor_crc@pipe-a-cursor-512x170-random.html

  * igt@kms_cursor_crc@pipe-c-cursor-128x42-sliding:
    - shard-skl:          [PASS][53] -> [FAIL][54] ([i915#3444])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-skl10/igt@kms_cursor_crc@pipe-c-cursor-128x42-sliding.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl4/igt@kms_cursor_crc@pipe-c-cursor-128x42-sliding.html

  * igt@kms_cursor_legacy@2x-flip-vs-cursor-legacy:
    - shard-iclb:         NOTRUN -> [SKIP][55] ([fdo#109274] / [fdo#109278]) +1 similar issue
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@kms_cursor_legacy@2x-flip-vs-cursor-legacy.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-skl:          NOTRUN -> [FAIL][56] ([i915#2346] / [i915#533])
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl10/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_cursor_legacy@pipe-d-torture-bo:
    - shard-apl:          NOTRUN -> [SKIP][57] ([fdo#109271] / [i915#533]) +1 similar issue
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-apl2/igt@kms_cursor_legacy@pipe-d-torture-bo.html

  * igt@kms_flip@flip-vs-suspend@b-dp1:
    - shard-apl:          [PASS][58] -> [DMESG-WARN][59] ([i915#180])
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-apl8/igt@kms_flip@flip-vs-suspend@b-dp1.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-apl8/igt@kms_flip@flip-vs-suspend@b-dp1.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-blt:
    - shard-skl:          NOTRUN -> [SKIP][60] ([fdo#109271]) +80 similar issues
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl10/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbc-stridechange:
    - shard-glk:          [PASS][61] -> [FAIL][62] ([i915#49])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-glk7/igt@kms_frontbuffer_tracking@fbc-stridechange.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-glk1/igt@kms_frontbuffer_tracking@fbc-stridechange.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-blt:
    - shard-apl:          NOTRUN -> [SKIP][63] ([fdo#109271]) +182 similar issues
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-apl2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-cur-indfb-draw-mmap-cpu:
    - shard-iclb:         NOTRUN -> [SKIP][64] ([fdo#109280]) +9 similar issues
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-cur-indfb-draw-mmap-cpu.html

  * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-wc:
    - shard-kbl:          NOTRUN -> [SKIP][65] ([fdo#109271]) +41 similar issues
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-kbl7/igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-wc.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - shard-iclb:         NOTRUN -> [SKIP][66] ([fdo#109278]) +7 similar issues
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-d:
    - shard-skl:          NOTRUN -> [SKIP][67] ([fdo#109271] / [i915#533]) +1 similar issue
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl10/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-d.html
    - shard-kbl:          NOTRUN -> [SKIP][68] ([fdo#109271] / [i915#533])
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-kbl6/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-d.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-basic:
    - shard-skl:          NOTRUN -> [FAIL][69] ([fdo#108145] / [i915#265])
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl7/igt@kms_plane_alpha_blend@pipe-a-alpha-basic.html

  * igt@kms_plane_alpha_blend@pipe-b-alpha-transparent-fb:
    - shard-apl:          NOTRUN -> [FAIL][70] ([i915#265])
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-apl3/igt@kms_plane_alpha_blend@pipe-b-alpha-transparent-fb.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [PASS][71] -> [FAIL][72] ([fdo#108145] / [i915#265]) +1 similar issue
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_properties@connector-properties-atomic:
    - shard-glk:          [PASS][73] -> [DMESG-WARN][74] ([i915#118] / [i915#95]) +2 similar issues
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-glk7/igt@kms_properties@connector-properties-atomic.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-glk1/igt@kms_properties@connector-properties-atomic.html

  * igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-2:
    - shard-apl:          NOTRUN -> [SKIP][75] ([fdo#109271] / [i915#658]) +4 similar issues
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-apl3/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-2.html
    - shard-kbl:          NOTRUN -> [SKIP][76] ([fdo#109271] / [i915#658]) +1 similar issue
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-kbl6/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-2.html
    - shard-skl:          NOTRUN -> [SKIP][77] ([fdo#109271] / [i915#658]) +1 similar issue
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl10/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-2.html

  * igt@kms_psr@psr2_cursor_mmap_cpu:
    - shard-iclb:         NOTRUN -> [SKIP][78] ([fdo#109441]) +1 similar issue
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@kms_psr@psr2_cursor_mmap_cpu.html

  * igt@kms_psr@psr2_cursor_plane_onoff:
    - shard-iclb:         [PASS][79] -> [SKIP][80] ([fdo#109441]) +1 similar issue
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb8/igt@kms_psr@psr2_cursor_plane_onoff.html

  * igt@kms_writeback@writeback-pixel-formats:
    - shard-skl:          NOTRUN -> [SKIP][81] ([fdo#109271] / [i915#2437])
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl10/igt@kms_writeback@writeback-pixel-formats.html

  * igt@nouveau_crc@pipe-a-ctx-flip-detection:
    - shard-iclb:         NOTRUN -> [SKIP][82] ([i915#2530])
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@nouveau_crc@pipe-a-ctx-flip-detection.html

  * igt@prime_nv_api@i915_nv_double_import:
    - shard-iclb:         NOTRUN -> [SKIP][83] ([fdo#109291])
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@prime_nv_api@i915_nv_double_import.html

  * igt@prime_vgem@basic-userptr:
    - shard-iclb:         NOTRUN -> [SKIP][84] ([i915#3301])
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@prime_vgem@basic-userptr.html

  * igt@prime_vgem@fence-read-hang:
    - shard-iclb:         NOTRUN -> [SKIP][85] ([fdo#109295])
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@prime_vgem@fence-read-hang.html

  * igt@sysfs_clients@create:
    - shard-iclb:         NOTRUN -> [SKIP][86] ([i915#2994])
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb5/igt@sysfs_clients@create.html

  * igt@sysfs_clients@sema-10:
    - shard-skl:          NOTRUN -> [SKIP][87] ([fdo#109271] / [i915#2994]) +3 similar issues
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl10/igt@sysfs_clients@sema-10.html

  * igt@sysfs_clients@sema-50:
    - shard-kbl:          NOTRUN -> [SKIP][88] ([fdo#109271] / [i915#2994]) +1 similar issue
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-kbl6/igt@sysfs_clients@sema-50.html
    - shard-apl:          NOTRUN -> [SKIP][89] ([fdo#109271] / [i915#2994]) +2 similar issues
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-apl3/igt@sysfs_clients@sema-50.html

  
#### Possible fixes ####

  * igt@gem_ctx_shared@q-smoketest-all:
    - shard-glk:          [DMESG-WARN][90] ([i915#118] / [i915#95]) -> [PASS][91]
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-glk9/igt@gem_ctx_shared@q-smoketest-all.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-glk5/igt@gem_ctx_shared@q-smoketest-all.html

  * igt@gem_exec_fair@basic-none-share@rcs0:
    - shard-iclb:         [FAIL][92] ([i915#2842]) -> [PASS][93]
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-iclb5/igt@gem_exec_fair@basic-none-share@rcs0.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb3/igt@gem_exec_fair@basic-none-share@rcs0.html

  * igt@gem_exec_fair@basic-none@rcs0:
    - shard-glk:          [FAIL][94] ([i915#2842]) -> [PASS][95]
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-glk9/igt@gem_exec_fair@basic-none@rcs0.html
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-glk5/igt@gem_exec_fair@basic-none@rcs0.html

  * igt@gem_exec_fair@basic-pace@vcs0:
    - shard-tglb:         [FAIL][96] ([i915#2842]) -> [PASS][97]
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-tglb2/igt@gem_exec_fair@basic-pace@vcs0.html
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-tglb5/igt@gem_exec_fair@basic-pace@vcs0.html

  * igt@gem_ppgtt@flink-and-close-vma-leak:
    - shard-skl:          [FAIL][98] ([i915#644]) -> [PASS][99]
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-skl1/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl5/igt@gem_ppgtt@flink-and-close-vma-leak.html

  * igt@gem_vm_create@destroy-race:
    - shard-tglb:         [TIMEOUT][100] ([i915#2795]) -> [PASS][101]
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-tglb5/igt@gem_vm_create@destroy-race.html
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-tglb7/igt@gem_vm_create@destroy-race.html

  * igt@i915_pm_dc@dc5-psr:
    - shard-skl:          [INCOMPLETE][102] ([i915#198]) -> [PASS][103]
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-skl2/igt@i915_pm_dc@dc5-psr.html
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl10/igt@i915_pm_dc@dc5-psr.html

  * igt@i915_pm_rpm@system-suspend-execbuf:
    - shard-skl:          [INCOMPLETE][104] ([i915#151]) -> [PASS][105]
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-skl7/igt@i915_pm_rpm@system-suspend-execbuf.html
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl1/igt@i915_pm_rpm@system-suspend-execbuf.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-kbl:          [DMESG-WARN][106] ([i915#180]) -> [PASS][107] +1 similar issue
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-kbl3/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-kbl6/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_draw_crc@draw-method-xrgb8888-mmap-gtt-untiled:
    - shard-iclb:         [FAIL][108] -> [PASS][109]
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-iclb7/igt@kms_draw_crc@draw-method-xrgb8888-mmap-gtt-untiled.html
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb2/igt@kms_draw_crc@draw-method-xrgb8888-mmap-gtt-untiled.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1:
    - shard-skl:          [FAIL][110] ([i915#79]) -> [PASS][111]
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-skl9/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl9/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html

  * igt@kms_hdr@bpc-switch:
    - shard-skl:          [FAIL][112] ([i915#1188]) -> [PASS][113]
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-skl10/igt@kms_hdr@bpc-switch.html
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl4/igt@kms_hdr@bpc-switch.html

  * igt@kms_hdr@bpc-switch-suspend:
    - shard-apl:          [DMESG-WARN][114] ([i915#180]) -> [PASS][115] +2 similar issues
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-apl6/igt@kms_hdr@bpc-switch-suspend.html
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-apl3/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
    - shard-skl:          [FAIL][116] ([i915#53]) -> [PASS][117]
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-skl6/igt@kms_pipe_crc_basic@hang-read-crc-pipe-a.html
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl6/igt@kms_pipe_crc_basic@hang-read-crc-pipe-a.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [FAIL][118] ([fdo#108145] / [i915#265]) -> [PASS][119] +1 similar issue
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-skl4/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-skl6/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_plane_lowres@pipe-c-tiling-none:
    - shard-glk:          [FAIL][120] ([i915#899]) -> [PASS][121]
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-glk7/igt@kms_plane_lowres@pipe-c-tiling-none.html
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-glk1/igt@kms_plane_lowres@pipe-c-tiling-none.html

  * igt@kms_psr@psr2_sprite_plane_onoff:
    - shard-iclb:         [SKIP][122] ([fdo#109441]) -> [PASS][123]
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-iclb6/igt@kms_psr@psr2_sprite_plane_onoff.html
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb2/igt@kms_psr@psr2_sprite_plane_onoff.html

  
#### Warnings ####

  * igt@kms_cursor_crc@pipe-b-cursor-suspend:
    - shard-kbl:          [INCOMPLETE][124] ([i915#2405]) -> [INCOMPLETE][125] ([i915#155] / [i915#2405])
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-kbl3/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-kbl3/igt@kms_cursor_crc@pipe-b-cursor-suspend.html

  * igt@kms_psr2_sf@plane-move-sf-dmg-area-0:
    - shard-iclb:         [SKIP][126] ([i915#658]) -> [SKIP][127] ([i915#2920]) +2 similar issues
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-iclb7/igt@kms_psr2_sf@plane-move-sf-dmg-area-0.html
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb2/igt@kms_psr2_sf@plane-move-sf-dmg-area-0.html

  * igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-5:
    - shard-iclb:         [SKIP][128] ([i915#2920]) -> [SKIP][129] ([i915#658]) +1 similar issue
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-iclb2/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-5.html
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-iclb8/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-5.html

  * igt@runner@aborted:
    - shard-kbl:          ([FAIL][130], [FAIL][131], [FAIL][132], [FAIL][133], [FAIL][134]) ([i915#180] / [i915#1814] / [i915#2505] / [i915#3002] / [i915#3363] / [i915#92]) -> ([FAIL][135], [FAIL][136], [FAIL][137], [FAIL][138], [FAIL][139], [FAIL][140], [FAIL][141]) ([i915#180] / [i915#1814] / [i915#2292] / [i915#3002] / [i915#3363])
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-kbl1/igt@runner@aborted.html
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-kbl3/igt@runner@aborted.html
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-kbl3/igt@runner@aborted.html
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-kbl1/igt@runner@aborted.html
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-kbl6/igt@runner@aborted.html
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-kbl3/igt@runner@aborted.html
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-kbl4/igt@runner@aborted.html
   [137]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-kbl1/igt@runner@aborted.html
   [138]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-kbl1/igt@runner@aborted.html
   [139]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-kbl3/igt@runner@aborted.html
   [140]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-kbl3/igt@runner@aborted.html
   [141]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-kbl4/igt@runner@aborted.html
    - shard-apl:          ([FAIL][142], [FAIL][143], [FAIL][144], [FAIL][145]) ([fdo#109271] / [i915#180] / [i915#1814] / [i915#3002] / [i915#3363]) -> ([FAIL][146], [FAIL][147], [FAIL][148]) ([i915#180] / [i915#3002] / [i915#3363])
   [142]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-apl6/igt@runner@aborted.html
   [143]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-apl6/igt@runner@aborted.html
   [144]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-apl1/igt@runner@aborted.html
   [145]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-apl6/igt@runner@aborted.html
   [146]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-apl1/igt@runner@aborted.html
   [147]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-apl8/igt@runner@aborted.html
   [148]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20079/shard-apl8/igt@runner@aborted.html
    - shard-skl:          ([FAIL][149], [FAIL][150], [FAIL][151], [FAIL][152]) ([i915#1814] / [i915#2029] / [i915#3002] / [i915#3363]) -> ([FAIL][153], [FAIL][154], [FAIL][155], [FAIL][156], [FAIL][157]) ([i915#1436] / [i915#1814] / [i915#2029] / [i915#2722] / [i915#3002] / [i915#3363])
   [149]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-skl2/igt@runner@aborted.html
   [150]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-skl7/igt@runner@aborted.html
   [151]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10053/shard-skl4/igt@run

== Logs ==

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

[-- Attachment #1.2: Type: text/html, Size: 33957 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v6 0/3] drm/i915/display: Try YCbCr420 color when RGB fails
  2021-05-06 17:23 ` [Intel-gfx] " Werner Sembach
@ 2021-05-07  8:49   ` Werner Sembach
  -1 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-07  8:49 UTC (permalink / raw)
  To: ville.syrjala, airlied, daniel, intel-gfx, dri-devel, linux-kernel

When encoder validation of a display mode fails, retry with less bandwidth
heavy YCbCr420 color mode, if available. This enables some HDMI 1.4 setups
to support 4k60Hz output, which previously failed silently.

AMDGPU had nearly the exact same issue. This problem description is
therefore copied from my commit message of the AMDGPU patch.

On some setups, while the monitor and the gpu support display modes with
pixel clocks of up to 600MHz, the link encoder might not. This prevents
YCbCr444 and RGB encoding for 4k60Hz, but YCbCr420 encoding might still be
possible. However, which color mode is used is decided before the link
encoder capabilities are checked. This patch fixes the problem by retrying
to find a display mode with YCbCr420 enforced and using it, if it is
valid.

This patchset is revision 6. Fixed only some coding style issues.



^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Intel-gfx] [PATCH v6 0/3] drm/i915/display: Try YCbCr420 color when RGB fails
@ 2021-05-07  8:49   ` Werner Sembach
  0 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-07  8:49 UTC (permalink / raw)
  To: ville.syrjala, airlied, daniel, intel-gfx, dri-devel, linux-kernel

When encoder validation of a display mode fails, retry with less bandwidth
heavy YCbCr420 color mode, if available. This enables some HDMI 1.4 setups
to support 4k60Hz output, which previously failed silently.

AMDGPU had nearly the exact same issue. This problem description is
therefore copied from my commit message of the AMDGPU patch.

On some setups, while the monitor and the gpu support display modes with
pixel clocks of up to 600MHz, the link encoder might not. This prevents
YCbCr444 and RGB encoding for 4k60Hz, but YCbCr420 encoding might still be
possible. However, which color mode is used is decided before the link
encoder capabilities are checked. This patch fixes the problem by retrying
to find a display mode with YCbCr420 enforced and using it, if it is
valid.

This patchset is revision 6. Fixed only some coding style issues.


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v6 1/3] drm/i915/display: New function to avoid duplicate code in upcomming commits
  2021-05-07  8:49   ` [Intel-gfx] " Werner Sembach
@ 2021-05-07  8:49     ` Werner Sembach
  -1 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-07  8:49 UTC (permalink / raw)
  To: ville.syrjala, airlied, daniel, intel-gfx, dri-devel, linux-kernel
  Cc: Werner Sembach

Moves some checks that later will be performed 2 times to an own function.
This avoids duplicate code later on.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 41 ++++++++++++++---------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 46de56af33db..576d3d910d06 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1861,6 +1861,31 @@ static int intel_hdmi_port_clock(int clock, int bpc)
 	return clock * bpc / 8;
 }
 
+static enum drm_mode_status
+intel_hdmi_mode_clock_valid(struct intel_hdmi *hdmi, int clock, bool has_hdmi_sink)
+{
+	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	enum drm_mode_status status;
+
+	/* check if we can do 8bpc */
+	status = hdmi_port_clock_valid(hdmi, clock, true, has_hdmi_sink);
+
+	if (has_hdmi_sink) {
+		/* if we can't do 8bpc we may still be able to do 12bpc */
+		if (status != MODE_OK && !HAS_GMCH(dev_priv))
+			status = hdmi_port_clock_valid(hdmi, clock * 3 / 2,
+						       true, has_hdmi_sink);
+
+		/* if we can't do 8,12bpc we may still be able to do 10bpc */
+		if (status != MODE_OK && INTEL_GEN(dev_priv) >= 11)
+			status = hdmi_port_clock_valid(hdmi, clock * 5 / 4,
+						       true, has_hdmi_sink);
+	}
+
+	return status;
+}
+
 static enum drm_mode_status
 intel_hdmi_mode_valid(struct drm_connector *connector,
 		      struct drm_display_mode *mode)
@@ -1891,21 +1916,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 	if (drm_mode_is_420_only(&connector->display_info, mode))
 		clock /= 2;
 
-	/* check if we can do 8bpc */
-	status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 8),
-				       true, has_hdmi_sink);
-
-	if (has_hdmi_sink) {
-		/* if we can't do 8bpc we may still be able to do 12bpc */
-		if (status != MODE_OK && !HAS_GMCH(dev_priv))
-			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 12),
-						       true, has_hdmi_sink);
-
-		/* if we can't do 8,12bpc we may still be able to do 10bpc */
-		if (status != MODE_OK && DISPLAY_VER(dev_priv) >= 11)
-			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 10),
-						       true, has_hdmi_sink);
-	}
+	status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
 	if (status != MODE_OK)
 		return status;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Intel-gfx] [PATCH v6 1/3] drm/i915/display: New function to avoid duplicate code in upcomming commits
@ 2021-05-07  8:49     ` Werner Sembach
  0 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-07  8:49 UTC (permalink / raw)
  To: ville.syrjala, airlied, daniel, intel-gfx, dri-devel, linux-kernel
  Cc: Werner Sembach

Moves some checks that later will be performed 2 times to an own function.
This avoids duplicate code later on.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 41 ++++++++++++++---------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 46de56af33db..576d3d910d06 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1861,6 +1861,31 @@ static int intel_hdmi_port_clock(int clock, int bpc)
 	return clock * bpc / 8;
 }
 
+static enum drm_mode_status
+intel_hdmi_mode_clock_valid(struct intel_hdmi *hdmi, int clock, bool has_hdmi_sink)
+{
+	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	enum drm_mode_status status;
+
+	/* check if we can do 8bpc */
+	status = hdmi_port_clock_valid(hdmi, clock, true, has_hdmi_sink);
+
+	if (has_hdmi_sink) {
+		/* if we can't do 8bpc we may still be able to do 12bpc */
+		if (status != MODE_OK && !HAS_GMCH(dev_priv))
+			status = hdmi_port_clock_valid(hdmi, clock * 3 / 2,
+						       true, has_hdmi_sink);
+
+		/* if we can't do 8,12bpc we may still be able to do 10bpc */
+		if (status != MODE_OK && INTEL_GEN(dev_priv) >= 11)
+			status = hdmi_port_clock_valid(hdmi, clock * 5 / 4,
+						       true, has_hdmi_sink);
+	}
+
+	return status;
+}
+
 static enum drm_mode_status
 intel_hdmi_mode_valid(struct drm_connector *connector,
 		      struct drm_display_mode *mode)
@@ -1891,21 +1916,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 	if (drm_mode_is_420_only(&connector->display_info, mode))
 		clock /= 2;
 
-	/* check if we can do 8bpc */
-	status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 8),
-				       true, has_hdmi_sink);
-
-	if (has_hdmi_sink) {
-		/* if we can't do 8bpc we may still be able to do 12bpc */
-		if (status != MODE_OK && !HAS_GMCH(dev_priv))
-			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 12),
-						       true, has_hdmi_sink);
-
-		/* if we can't do 8,12bpc we may still be able to do 10bpc */
-		if (status != MODE_OK && DISPLAY_VER(dev_priv) >= 11)
-			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 10),
-						       true, has_hdmi_sink);
-	}
+	status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
 	if (status != MODE_OK)
 		return status;
 
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v6 2/3] drm/i915/display: Restructure output format computation for better expandability
  2021-05-07  8:49   ` [Intel-gfx] " Werner Sembach
@ 2021-05-07  8:49     ` Werner Sembach
  -1 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-07  8:49 UTC (permalink / raw)
  To: ville.syrjala, airlied, daniel, intel-gfx, dri-devel, linux-kernel
  Cc: Werner Sembach

Couples the decission between RGB and YCbCr420 mode and the check if the
port clock can archive the required frequency. Other checks and
configuration steps that where previously done in between can also be done
before or after.

This allows for are cleaner implementation of retrying different color
encodings.

A slight change in behaviour occurs with this patch: If YCbCr420 is not
allowed but display is YCbCr420 only it no longer fails, but just prints
an error and tries to fallback on RGB.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 65 ++++++++++++-----------
 1 file changed, 34 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 576d3d910d06..9f3da72dabee 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1999,29 +1999,6 @@ static bool hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
 					      INTEL_OUTPUT_FORMAT_YCBCR420);
 }
 
-static int
-intel_hdmi_ycbcr420_config(struct intel_crtc_state *crtc_state,
-			   const struct drm_connector_state *conn_state)
-{
-	struct drm_connector *connector = conn_state->connector;
-	struct drm_i915_private *i915 = to_i915(connector->dev);
-	const struct drm_display_mode *adjusted_mode =
-		&crtc_state->hw.adjusted_mode;
-
-	if (!drm_mode_is_420_only(&connector->display_info, adjusted_mode))
-		return 0;
-
-	if (!connector->ycbcr_420_allowed) {
-		drm_err(&i915->drm,
-			"Platform doesn't support YCBCR420 output\n");
-		return -EINVAL;
-	}
-
-	crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
-
-	return intel_pch_panel_fitting(crtc_state, conn_state);
-}
-
 static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
 				  struct intel_crtc_state *crtc_state,
 				  int clock)
@@ -2128,6 +2105,30 @@ static bool intel_hdmi_has_audio(struct intel_encoder *encoder,
 		return intel_conn_state->force_audio == HDMI_AUDIO_ON;
 }
 
+static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
+					    struct intel_crtc_state *crtc_state,
+					    const struct drm_connector_state *conn_state)
+{
+	struct drm_connector *connector = conn_state->connector;
+	struct drm_i915_private *i915 = to_i915(connector->dev);
+	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
+	int ret;
+	bool ycbcr_420_only;
+
+	ycbcr_420_only = drm_mode_is_420_only(&connector->display_info, adjusted_mode);
+	if (connector->ycbcr_420_allowed && ycbcr_420_only) {
+		crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+	} else {
+		if (!connector->ycbcr_420_allowed && ycbcr_420_only)
+			drm_err(&i915->drm, "Display only supports YCbCr420 output, but connector does not allow it. Fallback to RGB, but this will likely fail.\n");
+		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
+	}
+
+	ret = intel_hdmi_compute_clock(encoder, crtc_state);
+
+	return ret;
+}
+
 int intel_hdmi_compute_config(struct intel_encoder *encoder,
 			      struct intel_crtc_state *pipe_config,
 			      struct drm_connector_state *conn_state)
@@ -2152,23 +2153,25 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
 		pipe_config->pixel_multiplier = 2;
 
-	ret = intel_hdmi_ycbcr420_config(pipe_config, conn_state);
-	if (ret)
-		return ret;
-
-	pipe_config->limited_color_range =
-		intel_hdmi_limited_color_range(pipe_config, conn_state);
-
 	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv))
 		pipe_config->has_pch_encoder = true;
 
 	pipe_config->has_audio =
 		intel_hdmi_has_audio(encoder, pipe_config, conn_state);
 
-	ret = intel_hdmi_compute_clock(encoder, pipe_config);
+	ret = intel_hdmi_compute_output_format(encoder, pipe_config, conn_state);
 	if (ret)
 		return ret;
 
+	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
+		ret = intel_pch_panel_fitting(pipe_config, conn_state);
+		if (ret)
+			return ret;
+	}
+
+	pipe_config->limited_color_range =
+		intel_hdmi_limited_color_range(pipe_config, conn_state);
+
 	if (conn_state->picture_aspect_ratio)
 		adjusted_mode->picture_aspect_ratio =
 			conn_state->picture_aspect_ratio;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Intel-gfx] [PATCH v6 2/3] drm/i915/display: Restructure output format computation for better expandability
@ 2021-05-07  8:49     ` Werner Sembach
  0 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-07  8:49 UTC (permalink / raw)
  To: ville.syrjala, airlied, daniel, intel-gfx, dri-devel, linux-kernel
  Cc: Werner Sembach

Couples the decission between RGB and YCbCr420 mode and the check if the
port clock can archive the required frequency. Other checks and
configuration steps that where previously done in between can also be done
before or after.

This allows for are cleaner implementation of retrying different color
encodings.

A slight change in behaviour occurs with this patch: If YCbCr420 is not
allowed but display is YCbCr420 only it no longer fails, but just prints
an error and tries to fallback on RGB.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 65 ++++++++++++-----------
 1 file changed, 34 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 576d3d910d06..9f3da72dabee 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1999,29 +1999,6 @@ static bool hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
 					      INTEL_OUTPUT_FORMAT_YCBCR420);
 }
 
-static int
-intel_hdmi_ycbcr420_config(struct intel_crtc_state *crtc_state,
-			   const struct drm_connector_state *conn_state)
-{
-	struct drm_connector *connector = conn_state->connector;
-	struct drm_i915_private *i915 = to_i915(connector->dev);
-	const struct drm_display_mode *adjusted_mode =
-		&crtc_state->hw.adjusted_mode;
-
-	if (!drm_mode_is_420_only(&connector->display_info, adjusted_mode))
-		return 0;
-
-	if (!connector->ycbcr_420_allowed) {
-		drm_err(&i915->drm,
-			"Platform doesn't support YCBCR420 output\n");
-		return -EINVAL;
-	}
-
-	crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
-
-	return intel_pch_panel_fitting(crtc_state, conn_state);
-}
-
 static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
 				  struct intel_crtc_state *crtc_state,
 				  int clock)
@@ -2128,6 +2105,30 @@ static bool intel_hdmi_has_audio(struct intel_encoder *encoder,
 		return intel_conn_state->force_audio == HDMI_AUDIO_ON;
 }
 
+static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
+					    struct intel_crtc_state *crtc_state,
+					    const struct drm_connector_state *conn_state)
+{
+	struct drm_connector *connector = conn_state->connector;
+	struct drm_i915_private *i915 = to_i915(connector->dev);
+	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
+	int ret;
+	bool ycbcr_420_only;
+
+	ycbcr_420_only = drm_mode_is_420_only(&connector->display_info, adjusted_mode);
+	if (connector->ycbcr_420_allowed && ycbcr_420_only) {
+		crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+	} else {
+		if (!connector->ycbcr_420_allowed && ycbcr_420_only)
+			drm_err(&i915->drm, "Display only supports YCbCr420 output, but connector does not allow it. Fallback to RGB, but this will likely fail.\n");
+		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
+	}
+
+	ret = intel_hdmi_compute_clock(encoder, crtc_state);
+
+	return ret;
+}
+
 int intel_hdmi_compute_config(struct intel_encoder *encoder,
 			      struct intel_crtc_state *pipe_config,
 			      struct drm_connector_state *conn_state)
@@ -2152,23 +2153,25 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
 		pipe_config->pixel_multiplier = 2;
 
-	ret = intel_hdmi_ycbcr420_config(pipe_config, conn_state);
-	if (ret)
-		return ret;
-
-	pipe_config->limited_color_range =
-		intel_hdmi_limited_color_range(pipe_config, conn_state);
-
 	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv))
 		pipe_config->has_pch_encoder = true;
 
 	pipe_config->has_audio =
 		intel_hdmi_has_audio(encoder, pipe_config, conn_state);
 
-	ret = intel_hdmi_compute_clock(encoder, pipe_config);
+	ret = intel_hdmi_compute_output_format(encoder, pipe_config, conn_state);
 	if (ret)
 		return ret;
 
+	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
+		ret = intel_pch_panel_fitting(pipe_config, conn_state);
+		if (ret)
+			return ret;
+	}
+
+	pipe_config->limited_color_range =
+		intel_hdmi_limited_color_range(pipe_config, conn_state);
+
 	if (conn_state->picture_aspect_ratio)
 		adjusted_mode->picture_aspect_ratio =
 			conn_state->picture_aspect_ratio;
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v6 3/3] drm/i915/display: Use YCbCr420 as fallback when RGB fails
  2021-05-07  8:49   ` [Intel-gfx] " Werner Sembach
@ 2021-05-07  8:49     ` Werner Sembach
  -1 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-07  8:49 UTC (permalink / raw)
  To: ville.syrjala, airlied, daniel, intel-gfx, dri-devel, linux-kernel
  Cc: Werner Sembach

When encoder validation of a display mode fails, retry with less bandwidth
heavy YCbCr420 color mode, if available. This enables some HDMI 1.4 setups
to support 4k60Hz output, which previously failed silently.

AMDGPU had nearly the exact same issue. This problem description is
therefore copied from my commit message of the AMDGPU patch.

On some setups, while the monitor and the gpu support display modes with
pixel clocks of up to 600MHz, the link encoder might not. This prevents
YCbCr444 and RGB encoding for 4k60Hz, but YCbCr420 encoding might still be
possible. However, which color mode is used is decided before the link
encoder capabilities are checked. This patch fixes the problem by retrying
to find a display mode with YCbCr420 enforced and using it, if it is
valid.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 25 ++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 9f3da72dabee..3c4d7a3e0969 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1897,6 +1897,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 	int clock = mode->clock;
 	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;
 
 	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
 		return MODE_NO_DBLESCAN;
@@ -1913,12 +1914,22 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 		clock *= 2;
 	}
 
-	if (drm_mode_is_420_only(&connector->display_info, mode))
+	ycbcr_420_only = drm_mode_is_420_only(&connector->display_info, mode);
+	if (ycbcr_420_only)
 		clock /= 2;
 
 	status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
-	if (status != MODE_OK)
-		return status;
+	if (status != MODE_OK) {
+		if (ycbcr_420_only ||
+		    !connector->ycbcr_420_allowed ||
+		    !drm_mode_is_420_also(&connector->display_info, mode))
+			return status;
+
+		clock /= 2;
+		status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
+		if (status != MODE_OK)
+			return status;
+	}
 
 	return intel_mode_valid_max_plane_size(dev_priv, mode, false);
 }
@@ -2125,6 +2136,14 @@ static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
 	}
 
 	ret = intel_hdmi_compute_clock(encoder, crtc_state);
+	if (ret) {
+		if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420 &&
+		    connector->ycbcr_420_allowed &&
+		    drm_mode_is_420_also(&connector->display_info, adjusted_mode)) {
+			crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+			ret = intel_hdmi_compute_clock(encoder, crtc_state);
+		}
+	}
 
 	return ret;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [Intel-gfx] [PATCH v6 3/3] drm/i915/display: Use YCbCr420 as fallback when RGB fails
@ 2021-05-07  8:49     ` Werner Sembach
  0 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-07  8:49 UTC (permalink / raw)
  To: ville.syrjala, airlied, daniel, intel-gfx, dri-devel, linux-kernel
  Cc: Werner Sembach

When encoder validation of a display mode fails, retry with less bandwidth
heavy YCbCr420 color mode, if available. This enables some HDMI 1.4 setups
to support 4k60Hz output, which previously failed silently.

AMDGPU had nearly the exact same issue. This problem description is
therefore copied from my commit message of the AMDGPU patch.

On some setups, while the monitor and the gpu support display modes with
pixel clocks of up to 600MHz, the link encoder might not. This prevents
YCbCr444 and RGB encoding for 4k60Hz, but YCbCr420 encoding might still be
possible. However, which color mode is used is decided before the link
encoder capabilities are checked. This patch fixes the problem by retrying
to find a display mode with YCbCr420 enforced and using it, if it is
valid.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 25 ++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 9f3da72dabee..3c4d7a3e0969 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1897,6 +1897,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 	int clock = mode->clock;
 	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;
 
 	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
 		return MODE_NO_DBLESCAN;
@@ -1913,12 +1914,22 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 		clock *= 2;
 	}
 
-	if (drm_mode_is_420_only(&connector->display_info, mode))
+	ycbcr_420_only = drm_mode_is_420_only(&connector->display_info, mode);
+	if (ycbcr_420_only)
 		clock /= 2;
 
 	status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
-	if (status != MODE_OK)
-		return status;
+	if (status != MODE_OK) {
+		if (ycbcr_420_only ||
+		    !connector->ycbcr_420_allowed ||
+		    !drm_mode_is_420_also(&connector->display_info, mode))
+			return status;
+
+		clock /= 2;
+		status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
+		if (status != MODE_OK)
+			return status;
+	}
 
 	return intel_mode_valid_max_plane_size(dev_priv, mode, false);
 }
@@ -2125,6 +2136,14 @@ static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
 	}
 
 	ret = intel_hdmi_compute_clock(encoder, crtc_state);
+	if (ret) {
+		if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420 &&
+		    connector->ycbcr_420_allowed &&
+		    drm_mode_is_420_also(&connector->display_info, adjusted_mode)) {
+			crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+			ret = intel_hdmi_compute_clock(encoder, crtc_state);
+		}
+	}
 
 	return ret;
 }
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH v6 1/3] drm/i915/display: New function to avoid duplicate code in upcomming commits
  2021-05-07  8:49     ` [Intel-gfx] " Werner Sembach
  (?)
@ 2021-05-07 17:47       ` Ville Syrjälä
  -1 siblings, 0 replies; 34+ messages in thread
From: Ville Syrjälä @ 2021-05-07 17:47 UTC (permalink / raw)
  To: Werner Sembach; +Cc: airlied, daniel, intel-gfx, dri-devel, linux-kernel

On Fri, May 07, 2021 at 10:49:01AM +0200, Werner Sembach wrote:
> Moves some checks that later will be performed 2 times to an own function.
> This avoids duplicate code later on.
> 
> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 41 ++++++++++++++---------
>  1 file changed, 26 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 46de56af33db..576d3d910d06 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -1861,6 +1861,31 @@ static int intel_hdmi_port_clock(int clock, int bpc)
>  	return clock * bpc / 8;
>  }
>  
> +static enum drm_mode_status
> +intel_hdmi_mode_clock_valid(struct intel_hdmi *hdmi, int clock, bool has_hdmi_sink)
> +{
> +	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	enum drm_mode_status status;
> +
> +	/* check if we can do 8bpc */
> +	status = hdmi_port_clock_valid(hdmi, clock, true, has_hdmi_sink);
> +
> +	if (has_hdmi_sink) {
> +		/* if we can't do 8bpc we may still be able to do 12bpc */
> +		if (status != MODE_OK && !HAS_GMCH(dev_priv))
> +			status = hdmi_port_clock_valid(hdmi, clock * 3 / 2,

Seems we've lost intel_hdmi_port_clock() here somehow.

> +						       true, has_hdmi_sink);
> +
> +		/* if we can't do 8,12bpc we may still be able to do 10bpc */
> +		if (status != MODE_OK && INTEL_GEN(dev_priv) >= 11)
> +			status = hdmi_port_clock_valid(hdmi, clock * 5 / 4,

Also here.

> +						       true, has_hdmi_sink);
> +	}
> +
> +	return status;
> +}
> +
>  static enum drm_mode_status
>  intel_hdmi_mode_valid(struct drm_connector *connector,
>  		      struct drm_display_mode *mode)
> @@ -1891,21 +1916,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>  	if (drm_mode_is_420_only(&connector->display_info, mode))
>  		clock /= 2;
>  
> -	/* check if we can do 8bpc */
> -	status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 8),
> -				       true, has_hdmi_sink);
> -
> -	if (has_hdmi_sink) {
> -		/* if we can't do 8bpc we may still be able to do 12bpc */
> -		if (status != MODE_OK && !HAS_GMCH(dev_priv))
> -			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 12),
> -						       true, has_hdmi_sink);
> -
> -		/* if we can't do 8,12bpc we may still be able to do 10bpc */
> -		if (status != MODE_OK && DISPLAY_VER(dev_priv) >= 11)
> -			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 10),
> -						       true, has_hdmi_sink);
> -	}
> +	status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
>  	if (status != MODE_OK)
>  		return status;
>  
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v6 1/3] drm/i915/display: New function to avoid duplicate code in upcomming commits
@ 2021-05-07 17:47       ` Ville Syrjälä
  0 siblings, 0 replies; 34+ messages in thread
From: Ville Syrjälä @ 2021-05-07 17:47 UTC (permalink / raw)
  To: Werner Sembach; +Cc: airlied, intel-gfx, dri-devel, linux-kernel

On Fri, May 07, 2021 at 10:49:01AM +0200, Werner Sembach wrote:
> Moves some checks that later will be performed 2 times to an own function.
> This avoids duplicate code later on.
> 
> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 41 ++++++++++++++---------
>  1 file changed, 26 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 46de56af33db..576d3d910d06 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -1861,6 +1861,31 @@ static int intel_hdmi_port_clock(int clock, int bpc)
>  	return clock * bpc / 8;
>  }
>  
> +static enum drm_mode_status
> +intel_hdmi_mode_clock_valid(struct intel_hdmi *hdmi, int clock, bool has_hdmi_sink)
> +{
> +	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	enum drm_mode_status status;
> +
> +	/* check if we can do 8bpc */
> +	status = hdmi_port_clock_valid(hdmi, clock, true, has_hdmi_sink);
> +
> +	if (has_hdmi_sink) {
> +		/* if we can't do 8bpc we may still be able to do 12bpc */
> +		if (status != MODE_OK && !HAS_GMCH(dev_priv))
> +			status = hdmi_port_clock_valid(hdmi, clock * 3 / 2,

Seems we've lost intel_hdmi_port_clock() here somehow.

> +						       true, has_hdmi_sink);
> +
> +		/* if we can't do 8,12bpc we may still be able to do 10bpc */
> +		if (status != MODE_OK && INTEL_GEN(dev_priv) >= 11)
> +			status = hdmi_port_clock_valid(hdmi, clock * 5 / 4,

Also here.

> +						       true, has_hdmi_sink);
> +	}
> +
> +	return status;
> +}
> +
>  static enum drm_mode_status
>  intel_hdmi_mode_valid(struct drm_connector *connector,
>  		      struct drm_display_mode *mode)
> @@ -1891,21 +1916,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>  	if (drm_mode_is_420_only(&connector->display_info, mode))
>  		clock /= 2;
>  
> -	/* check if we can do 8bpc */
> -	status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 8),
> -				       true, has_hdmi_sink);
> -
> -	if (has_hdmi_sink) {
> -		/* if we can't do 8bpc we may still be able to do 12bpc */
> -		if (status != MODE_OK && !HAS_GMCH(dev_priv))
> -			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 12),
> -						       true, has_hdmi_sink);
> -
> -		/* if we can't do 8,12bpc we may still be able to do 10bpc */
> -		if (status != MODE_OK && DISPLAY_VER(dev_priv) >= 11)
> -			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 10),
> -						       true, has_hdmi_sink);
> -	}
> +	status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
>  	if (status != MODE_OK)
>  		return status;
>  
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Intel-gfx] [PATCH v6 1/3] drm/i915/display: New function to avoid duplicate code in upcomming commits
@ 2021-05-07 17:47       ` Ville Syrjälä
  0 siblings, 0 replies; 34+ messages in thread
From: Ville Syrjälä @ 2021-05-07 17:47 UTC (permalink / raw)
  To: Werner Sembach; +Cc: airlied, intel-gfx, dri-devel, linux-kernel

On Fri, May 07, 2021 at 10:49:01AM +0200, Werner Sembach wrote:
> Moves some checks that later will be performed 2 times to an own function.
> This avoids duplicate code later on.
> 
> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 41 ++++++++++++++---------
>  1 file changed, 26 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 46de56af33db..576d3d910d06 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -1861,6 +1861,31 @@ static int intel_hdmi_port_clock(int clock, int bpc)
>  	return clock * bpc / 8;
>  }
>  
> +static enum drm_mode_status
> +intel_hdmi_mode_clock_valid(struct intel_hdmi *hdmi, int clock, bool has_hdmi_sink)
> +{
> +	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	enum drm_mode_status status;
> +
> +	/* check if we can do 8bpc */
> +	status = hdmi_port_clock_valid(hdmi, clock, true, has_hdmi_sink);
> +
> +	if (has_hdmi_sink) {
> +		/* if we can't do 8bpc we may still be able to do 12bpc */
> +		if (status != MODE_OK && !HAS_GMCH(dev_priv))
> +			status = hdmi_port_clock_valid(hdmi, clock * 3 / 2,

Seems we've lost intel_hdmi_port_clock() here somehow.

> +						       true, has_hdmi_sink);
> +
> +		/* if we can't do 8,12bpc we may still be able to do 10bpc */
> +		if (status != MODE_OK && INTEL_GEN(dev_priv) >= 11)
> +			status = hdmi_port_clock_valid(hdmi, clock * 5 / 4,

Also here.

> +						       true, has_hdmi_sink);
> +	}
> +
> +	return status;
> +}
> +
>  static enum drm_mode_status
>  intel_hdmi_mode_valid(struct drm_connector *connector,
>  		      struct drm_display_mode *mode)
> @@ -1891,21 +1916,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>  	if (drm_mode_is_420_only(&connector->display_info, mode))
>  		clock /= 2;
>  
> -	/* check if we can do 8bpc */
> -	status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 8),
> -				       true, has_hdmi_sink);
> -
> -	if (has_hdmi_sink) {
> -		/* if we can't do 8bpc we may still be able to do 12bpc */
> -		if (status != MODE_OK && !HAS_GMCH(dev_priv))
> -			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 12),
> -						       true, has_hdmi_sink);
> -
> -		/* if we can't do 8,12bpc we may still be able to do 10bpc */
> -		if (status != MODE_OK && DISPLAY_VER(dev_priv) >= 11)
> -			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 10),
> -						       true, has_hdmi_sink);
> -	}
> +	status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
>  	if (status != MODE_OK)
>  		return status;
>  
> -- 
> 2.25.1

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v6 2/3] drm/i915/display: Restructure output format computation for better expandability
  2021-05-07  8:49     ` [Intel-gfx] " Werner Sembach
  (?)
@ 2021-05-07 17:52       ` Ville Syrjälä
  -1 siblings, 0 replies; 34+ messages in thread
From: Ville Syrjälä @ 2021-05-07 17:52 UTC (permalink / raw)
  To: Werner Sembach; +Cc: airlied, daniel, intel-gfx, dri-devel, linux-kernel

On Fri, May 07, 2021 at 10:49:02AM +0200, Werner Sembach wrote:
> Couples the decission between RGB and YCbCr420 mode and the check if the
> port clock can archive the required frequency. Other checks and
> configuration steps that where previously done in between can also be done
> before or after.
> 
> This allows for are cleaner implementation of retrying different color
> encodings.
> 
> A slight change in behaviour occurs with this patch: If YCbCr420 is not
> allowed but display is YCbCr420 only it no longer fails, but just prints
> an error and tries to fallback on RGB.
> 
> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 65 ++++++++++++-----------
>  1 file changed, 34 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 576d3d910d06..9f3da72dabee 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -1999,29 +1999,6 @@ static bool hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
>  					      INTEL_OUTPUT_FORMAT_YCBCR420);
>  }
>  
> -static int
> -intel_hdmi_ycbcr420_config(struct intel_crtc_state *crtc_state,
> -			   const struct drm_connector_state *conn_state)
> -{
> -	struct drm_connector *connector = conn_state->connector;
> -	struct drm_i915_private *i915 = to_i915(connector->dev);
> -	const struct drm_display_mode *adjusted_mode =
> -		&crtc_state->hw.adjusted_mode;
> -
> -	if (!drm_mode_is_420_only(&connector->display_info, adjusted_mode))
> -		return 0;
> -
> -	if (!connector->ycbcr_420_allowed) {
> -		drm_err(&i915->drm,
> -			"Platform doesn't support YCBCR420 output\n");
> -		return -EINVAL;
> -	}
> -
> -	crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> -
> -	return intel_pch_panel_fitting(crtc_state, conn_state);
> -}
> -
>  static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
>  				  struct intel_crtc_state *crtc_state,
>  				  int clock)
> @@ -2128,6 +2105,30 @@ static bool intel_hdmi_has_audio(struct intel_encoder *encoder,
>  		return intel_conn_state->force_audio == HDMI_AUDIO_ON;
>  }
>  
> +static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
> +					    struct intel_crtc_state *crtc_state,
> +					    const struct drm_connector_state *conn_state)
> +{
> +	struct drm_connector *connector = conn_state->connector;
> +	struct drm_i915_private *i915 = to_i915(connector->dev);
> +	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
> +	int ret;
> +	bool ycbcr_420_only;
> +
> +	ycbcr_420_only = drm_mode_is_420_only(&connector->display_info, adjusted_mode);
> +	if (connector->ycbcr_420_allowed && ycbcr_420_only) {
> +		crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> +	} else {
> +		if (!connector->ycbcr_420_allowed && ycbcr_420_only)
> +			drm_err(&i915->drm, "Display only supports YCbCr420 output, but connector does not allow it. Fallback to RGB, but this will likely fail.\n");

We can't let the user spam dmesg with errors freely. So this needs
to be a drm_dbg_kms(). Also a bit long, so going to annoyingly wrap
always. Could maybe shorten a bit to something like:
"YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible. Falling back to RGB."

With that sorted, and the intel_hdmi_port_clock() stuff restored,
I believe this series is good to go.

I think you confused our CI by replying to the old patch with a whole
new series. It can generally deal with a whole new series as a new
thread or replies to individual patches with updated versions of
exactly that patch, but not full series as a reply to a patch.
So I suggest just posting the final versions as a new series. Thanks.

> +		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
> +	}
> +
> +	ret = intel_hdmi_compute_clock(encoder, crtc_state);
> +
> +	return ret;
> +}
> +
>  int intel_hdmi_compute_config(struct intel_encoder *encoder,
>  			      struct intel_crtc_state *pipe_config,
>  			      struct drm_connector_state *conn_state)
> @@ -2152,23 +2153,25 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
>  		pipe_config->pixel_multiplier = 2;
>  
> -	ret = intel_hdmi_ycbcr420_config(pipe_config, conn_state);
> -	if (ret)
> -		return ret;
> -
> -	pipe_config->limited_color_range =
> -		intel_hdmi_limited_color_range(pipe_config, conn_state);
> -
>  	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv))
>  		pipe_config->has_pch_encoder = true;
>  
>  	pipe_config->has_audio =
>  		intel_hdmi_has_audio(encoder, pipe_config, conn_state);
>  
> -	ret = intel_hdmi_compute_clock(encoder, pipe_config);
> +	ret = intel_hdmi_compute_output_format(encoder, pipe_config, conn_state);
>  	if (ret)
>  		return ret;
>  
> +	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
> +		ret = intel_pch_panel_fitting(pipe_config, conn_state);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	pipe_config->limited_color_range =
> +		intel_hdmi_limited_color_range(pipe_config, conn_state);
> +
>  	if (conn_state->picture_aspect_ratio)
>  		adjusted_mode->picture_aspect_ratio =
>  			conn_state->picture_aspect_ratio;
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v6 2/3] drm/i915/display: Restructure output format computation for better expandability
@ 2021-05-07 17:52       ` Ville Syrjälä
  0 siblings, 0 replies; 34+ messages in thread
From: Ville Syrjälä @ 2021-05-07 17:52 UTC (permalink / raw)
  To: Werner Sembach; +Cc: airlied, intel-gfx, dri-devel, linux-kernel

On Fri, May 07, 2021 at 10:49:02AM +0200, Werner Sembach wrote:
> Couples the decission between RGB and YCbCr420 mode and the check if the
> port clock can archive the required frequency. Other checks and
> configuration steps that where previously done in between can also be done
> before or after.
> 
> This allows for are cleaner implementation of retrying different color
> encodings.
> 
> A slight change in behaviour occurs with this patch: If YCbCr420 is not
> allowed but display is YCbCr420 only it no longer fails, but just prints
> an error and tries to fallback on RGB.
> 
> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 65 ++++++++++++-----------
>  1 file changed, 34 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 576d3d910d06..9f3da72dabee 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -1999,29 +1999,6 @@ static bool hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
>  					      INTEL_OUTPUT_FORMAT_YCBCR420);
>  }
>  
> -static int
> -intel_hdmi_ycbcr420_config(struct intel_crtc_state *crtc_state,
> -			   const struct drm_connector_state *conn_state)
> -{
> -	struct drm_connector *connector = conn_state->connector;
> -	struct drm_i915_private *i915 = to_i915(connector->dev);
> -	const struct drm_display_mode *adjusted_mode =
> -		&crtc_state->hw.adjusted_mode;
> -
> -	if (!drm_mode_is_420_only(&connector->display_info, adjusted_mode))
> -		return 0;
> -
> -	if (!connector->ycbcr_420_allowed) {
> -		drm_err(&i915->drm,
> -			"Platform doesn't support YCBCR420 output\n");
> -		return -EINVAL;
> -	}
> -
> -	crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> -
> -	return intel_pch_panel_fitting(crtc_state, conn_state);
> -}
> -
>  static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
>  				  struct intel_crtc_state *crtc_state,
>  				  int clock)
> @@ -2128,6 +2105,30 @@ static bool intel_hdmi_has_audio(struct intel_encoder *encoder,
>  		return intel_conn_state->force_audio == HDMI_AUDIO_ON;
>  }
>  
> +static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
> +					    struct intel_crtc_state *crtc_state,
> +					    const struct drm_connector_state *conn_state)
> +{
> +	struct drm_connector *connector = conn_state->connector;
> +	struct drm_i915_private *i915 = to_i915(connector->dev);
> +	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
> +	int ret;
> +	bool ycbcr_420_only;
> +
> +	ycbcr_420_only = drm_mode_is_420_only(&connector->display_info, adjusted_mode);
> +	if (connector->ycbcr_420_allowed && ycbcr_420_only) {
> +		crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> +	} else {
> +		if (!connector->ycbcr_420_allowed && ycbcr_420_only)
> +			drm_err(&i915->drm, "Display only supports YCbCr420 output, but connector does not allow it. Fallback to RGB, but this will likely fail.\n");

We can't let the user spam dmesg with errors freely. So this needs
to be a drm_dbg_kms(). Also a bit long, so going to annoyingly wrap
always. Could maybe shorten a bit to something like:
"YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible. Falling back to RGB."

With that sorted, and the intel_hdmi_port_clock() stuff restored,
I believe this series is good to go.

I think you confused our CI by replying to the old patch with a whole
new series. It can generally deal with a whole new series as a new
thread or replies to individual patches with updated versions of
exactly that patch, but not full series as a reply to a patch.
So I suggest just posting the final versions as a new series. Thanks.

> +		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
> +	}
> +
> +	ret = intel_hdmi_compute_clock(encoder, crtc_state);
> +
> +	return ret;
> +}
> +
>  int intel_hdmi_compute_config(struct intel_encoder *encoder,
>  			      struct intel_crtc_state *pipe_config,
>  			      struct drm_connector_state *conn_state)
> @@ -2152,23 +2153,25 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
>  		pipe_config->pixel_multiplier = 2;
>  
> -	ret = intel_hdmi_ycbcr420_config(pipe_config, conn_state);
> -	if (ret)
> -		return ret;
> -
> -	pipe_config->limited_color_range =
> -		intel_hdmi_limited_color_range(pipe_config, conn_state);
> -
>  	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv))
>  		pipe_config->has_pch_encoder = true;
>  
>  	pipe_config->has_audio =
>  		intel_hdmi_has_audio(encoder, pipe_config, conn_state);
>  
> -	ret = intel_hdmi_compute_clock(encoder, pipe_config);
> +	ret = intel_hdmi_compute_output_format(encoder, pipe_config, conn_state);
>  	if (ret)
>  		return ret;
>  
> +	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
> +		ret = intel_pch_panel_fitting(pipe_config, conn_state);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	pipe_config->limited_color_range =
> +		intel_hdmi_limited_color_range(pipe_config, conn_state);
> +
>  	if (conn_state->picture_aspect_ratio)
>  		adjusted_mode->picture_aspect_ratio =
>  			conn_state->picture_aspect_ratio;
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Intel-gfx] [PATCH v6 2/3] drm/i915/display: Restructure output format computation for better expandability
@ 2021-05-07 17:52       ` Ville Syrjälä
  0 siblings, 0 replies; 34+ messages in thread
From: Ville Syrjälä @ 2021-05-07 17:52 UTC (permalink / raw)
  To: Werner Sembach; +Cc: airlied, intel-gfx, dri-devel, linux-kernel

On Fri, May 07, 2021 at 10:49:02AM +0200, Werner Sembach wrote:
> Couples the decission between RGB and YCbCr420 mode and the check if the
> port clock can archive the required frequency. Other checks and
> configuration steps that where previously done in between can also be done
> before or after.
> 
> This allows for are cleaner implementation of retrying different color
> encodings.
> 
> A slight change in behaviour occurs with this patch: If YCbCr420 is not
> allowed but display is YCbCr420 only it no longer fails, but just prints
> an error and tries to fallback on RGB.
> 
> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 65 ++++++++++++-----------
>  1 file changed, 34 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 576d3d910d06..9f3da72dabee 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -1999,29 +1999,6 @@ static bool hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
>  					      INTEL_OUTPUT_FORMAT_YCBCR420);
>  }
>  
> -static int
> -intel_hdmi_ycbcr420_config(struct intel_crtc_state *crtc_state,
> -			   const struct drm_connector_state *conn_state)
> -{
> -	struct drm_connector *connector = conn_state->connector;
> -	struct drm_i915_private *i915 = to_i915(connector->dev);
> -	const struct drm_display_mode *adjusted_mode =
> -		&crtc_state->hw.adjusted_mode;
> -
> -	if (!drm_mode_is_420_only(&connector->display_info, adjusted_mode))
> -		return 0;
> -
> -	if (!connector->ycbcr_420_allowed) {
> -		drm_err(&i915->drm,
> -			"Platform doesn't support YCBCR420 output\n");
> -		return -EINVAL;
> -	}
> -
> -	crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> -
> -	return intel_pch_panel_fitting(crtc_state, conn_state);
> -}
> -
>  static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
>  				  struct intel_crtc_state *crtc_state,
>  				  int clock)
> @@ -2128,6 +2105,30 @@ static bool intel_hdmi_has_audio(struct intel_encoder *encoder,
>  		return intel_conn_state->force_audio == HDMI_AUDIO_ON;
>  }
>  
> +static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
> +					    struct intel_crtc_state *crtc_state,
> +					    const struct drm_connector_state *conn_state)
> +{
> +	struct drm_connector *connector = conn_state->connector;
> +	struct drm_i915_private *i915 = to_i915(connector->dev);
> +	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
> +	int ret;
> +	bool ycbcr_420_only;
> +
> +	ycbcr_420_only = drm_mode_is_420_only(&connector->display_info, adjusted_mode);
> +	if (connector->ycbcr_420_allowed && ycbcr_420_only) {
> +		crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> +	} else {
> +		if (!connector->ycbcr_420_allowed && ycbcr_420_only)
> +			drm_err(&i915->drm, "Display only supports YCbCr420 output, but connector does not allow it. Fallback to RGB, but this will likely fail.\n");

We can't let the user spam dmesg with errors freely. So this needs
to be a drm_dbg_kms(). Also a bit long, so going to annoyingly wrap
always. Could maybe shorten a bit to something like:
"YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible. Falling back to RGB."

With that sorted, and the intel_hdmi_port_clock() stuff restored,
I believe this series is good to go.

I think you confused our CI by replying to the old patch with a whole
new series. It can generally deal with a whole new series as a new
thread or replies to individual patches with updated versions of
exactly that patch, but not full series as a reply to a patch.
So I suggest just posting the final versions as a new series. Thanks.

> +		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
> +	}
> +
> +	ret = intel_hdmi_compute_clock(encoder, crtc_state);
> +
> +	return ret;
> +}
> +
>  int intel_hdmi_compute_config(struct intel_encoder *encoder,
>  			      struct intel_crtc_state *pipe_config,
>  			      struct drm_connector_state *conn_state)
> @@ -2152,23 +2153,25 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
>  		pipe_config->pixel_multiplier = 2;
>  
> -	ret = intel_hdmi_ycbcr420_config(pipe_config, conn_state);
> -	if (ret)
> -		return ret;
> -
> -	pipe_config->limited_color_range =
> -		intel_hdmi_limited_color_range(pipe_config, conn_state);
> -
>  	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv))
>  		pipe_config->has_pch_encoder = true;
>  
>  	pipe_config->has_audio =
>  		intel_hdmi_has_audio(encoder, pipe_config, conn_state);
>  
> -	ret = intel_hdmi_compute_clock(encoder, pipe_config);
> +	ret = intel_hdmi_compute_output_format(encoder, pipe_config, conn_state);
>  	if (ret)
>  		return ret;
>  
> +	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
> +		ret = intel_pch_panel_fitting(pipe_config, conn_state);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	pipe_config->limited_color_range =
> +		intel_hdmi_limited_color_range(pipe_config, conn_state);
> +
>  	if (conn_state->picture_aspect_ratio)
>  		adjusted_mode->picture_aspect_ratio =
>  			conn_state->picture_aspect_ratio;
> -- 
> 2.25.1

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v6 2/3] drm/i915/display: Restructure output format computation for better expandability
  2021-05-07 17:52       ` Ville Syrjälä
  (?)
@ 2021-05-07 17:56         ` Randy Dunlap
  -1 siblings, 0 replies; 34+ messages in thread
From: Randy Dunlap @ 2021-05-07 17:56 UTC (permalink / raw)
  To: Ville Syrjälä, Werner Sembach
  Cc: airlied, daniel, intel-gfx, dri-devel, linux-kernel

On 5/7/21 10:52 AM, Ville Syrjälä wrote:
> On Fri, May 07, 2021 at 10:49:02AM +0200, Werner Sembach wrote:
>> Couples the decission between RGB and YCbCr420 mode and the check if the
>> port clock can archive the required frequency. Other checks and
>> configuration steps that where previously done in between can also be done
>> before or after.
>>
>> This allows for are cleaner implementation of retrying different color
>> encodings.
>>
>> A slight change in behaviour occurs with this patch: If YCbCr420 is not
>> allowed but display is YCbCr420 only it no longer fails, but just prints
>> an error and tries to fallback on RGB.
>>
>> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_hdmi.c | 65 ++++++++++++-----------
>>  1 file changed, 34 insertions(+), 31 deletions(-)
>>

> 
> We can't let the user spam dmesg with errors freely. So this needs
> to be a drm_dbg_kms(). Also a bit long, so going to annoyingly wrap
> always. Could maybe shorten a bit to something like:
> "YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible. Falling back to RGB."
> 
> With that sorted, and the intel_hdmi_port_clock() stuff restored,
> I believe this series is good to go.
> 
> I think you confused our CI by replying to the old patch with a whole
> new series. It can generally deal with a whole new series as a new
> thread or replies to individual patches with updated versions of
> exactly that patch, but not full series as a reply to a patch.
> So I suggest just posting the final versions as a new series. Thanks.
> 

Yeah, we try to say "don't do that," but maybe we need to say that more
strongly. See Documentation/process/submitting-patches.rst:

<<<
However, for a multi-patch series, it is generally
best to avoid using In-Reply-To: to link to older versions of the
series.  This way multiple versions of the patch don't become an
unmanageable forest of references in email clients.  If a link is
helpful, you can use the https://lkml.kernel.org/ redirector (e.g., in
the cover email text) to link to an earlier version of the patch series.
>>>


-- 
~Randy


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v6 2/3] drm/i915/display: Restructure output format computation for better expandability
@ 2021-05-07 17:56         ` Randy Dunlap
  0 siblings, 0 replies; 34+ messages in thread
From: Randy Dunlap @ 2021-05-07 17:56 UTC (permalink / raw)
  To: Ville Syrjälä, Werner Sembach
  Cc: airlied, intel-gfx, dri-devel, linux-kernel

On 5/7/21 10:52 AM, Ville Syrjälä wrote:
> On Fri, May 07, 2021 at 10:49:02AM +0200, Werner Sembach wrote:
>> Couples the decission between RGB and YCbCr420 mode and the check if the
>> port clock can archive the required frequency. Other checks and
>> configuration steps that where previously done in between can also be done
>> before or after.
>>
>> This allows for are cleaner implementation of retrying different color
>> encodings.
>>
>> A slight change in behaviour occurs with this patch: If YCbCr420 is not
>> allowed but display is YCbCr420 only it no longer fails, but just prints
>> an error and tries to fallback on RGB.
>>
>> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_hdmi.c | 65 ++++++++++++-----------
>>  1 file changed, 34 insertions(+), 31 deletions(-)
>>

> 
> We can't let the user spam dmesg with errors freely. So this needs
> to be a drm_dbg_kms(). Also a bit long, so going to annoyingly wrap
> always. Could maybe shorten a bit to something like:
> "YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible. Falling back to RGB."
> 
> With that sorted, and the intel_hdmi_port_clock() stuff restored,
> I believe this series is good to go.
> 
> I think you confused our CI by replying to the old patch with a whole
> new series. It can generally deal with a whole new series as a new
> thread or replies to individual patches with updated versions of
> exactly that patch, but not full series as a reply to a patch.
> So I suggest just posting the final versions as a new series. Thanks.
> 

Yeah, we try to say "don't do that," but maybe we need to say that more
strongly. See Documentation/process/submitting-patches.rst:

<<<
However, for a multi-patch series, it is generally
best to avoid using In-Reply-To: to link to older versions of the
series.  This way multiple versions of the patch don't become an
unmanageable forest of references in email clients.  If a link is
helpful, you can use the https://lkml.kernel.org/ redirector (e.g., in
the cover email text) to link to an earlier version of the patch series.
>>>


-- 
~Randy


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Intel-gfx] [PATCH v6 2/3] drm/i915/display: Restructure output format computation for better expandability
@ 2021-05-07 17:56         ` Randy Dunlap
  0 siblings, 0 replies; 34+ messages in thread
From: Randy Dunlap @ 2021-05-07 17:56 UTC (permalink / raw)
  To: Ville Syrjälä, Werner Sembach
  Cc: airlied, intel-gfx, dri-devel, linux-kernel

On 5/7/21 10:52 AM, Ville Syrjälä wrote:
> On Fri, May 07, 2021 at 10:49:02AM +0200, Werner Sembach wrote:
>> Couples the decission between RGB and YCbCr420 mode and the check if the
>> port clock can archive the required frequency. Other checks and
>> configuration steps that where previously done in between can also be done
>> before or after.
>>
>> This allows for are cleaner implementation of retrying different color
>> encodings.
>>
>> A slight change in behaviour occurs with this patch: If YCbCr420 is not
>> allowed but display is YCbCr420 only it no longer fails, but just prints
>> an error and tries to fallback on RGB.
>>
>> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_hdmi.c | 65 ++++++++++++-----------
>>  1 file changed, 34 insertions(+), 31 deletions(-)
>>

> 
> We can't let the user spam dmesg with errors freely. So this needs
> to be a drm_dbg_kms(). Also a bit long, so going to annoyingly wrap
> always. Could maybe shorten a bit to something like:
> "YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible. Falling back to RGB."
> 
> With that sorted, and the intel_hdmi_port_clock() stuff restored,
> I believe this series is good to go.
> 
> I think you confused our CI by replying to the old patch with a whole
> new series. It can generally deal with a whole new series as a new
> thread or replies to individual patches with updated versions of
> exactly that patch, but not full series as a reply to a patch.
> So I suggest just posting the final versions as a new series. Thanks.
> 

Yeah, we try to say "don't do that," but maybe we need to say that more
strongly. See Documentation/process/submitting-patches.rst:

<<<
However, for a multi-patch series, it is generally
best to avoid using In-Reply-To: to link to older versions of the
series.  This way multiple versions of the patch don't become an
unmanageable forest of references in email clients.  If a link is
helpful, you can use the https://lkml.kernel.org/ redirector (e.g., in
the cover email text) to link to an earlier version of the patch series.
>>>


-- 
~Randy

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v6 1/3] drm/i915/display: New function to avoid duplicate code in upcomming commits
  2021-05-07 17:47       ` Ville Syrjälä
  (?)
@ 2021-05-07 20:33         ` Werner Sembach
  -1 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-07 20:33 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: airlied, daniel, intel-gfx, dri-devel, linux-kernel

Am 07.05.21 um 19:47 schrieb Ville Syrjälä:

> On Fri, May 07, 2021 at 10:49:01AM +0200, Werner Sembach wrote:
>> Moves some checks that later will be performed 2 times to an own function.
>> This avoids duplicate code later on.
>>
>> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_hdmi.c | 41 ++++++++++++++---------
>>  1 file changed, 26 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> index 46de56af33db..576d3d910d06 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> @@ -1861,6 +1861,31 @@ static int intel_hdmi_port_clock(int clock, int bpc)
>>  	return clock * bpc / 8;
>>  }
>>  
>> +static enum drm_mode_status
>> +intel_hdmi_mode_clock_valid(struct intel_hdmi *hdmi, int clock, bool has_hdmi_sink)
>> +{
>> +	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
>> +	struct drm_i915_private *dev_priv = to_i915(dev);
>> +	enum drm_mode_status status;
>> +
>> +	/* check if we can do 8bpc */
>> +	status = hdmi_port_clock_valid(hdmi, clock, true, has_hdmi_sink);
>> +
>> +	if (has_hdmi_sink) {
>> +		/* if we can't do 8bpc we may still be able to do 12bpc */
>> +		if (status != MODE_OK && !HAS_GMCH(dev_priv))
>> +			status = hdmi_port_clock_valid(hdmi, clock * 3 / 2,
> Seems we've lost intel_hdmi_port_clock() here somehow.

Yes, I think it happened when I rebased from torvalds/master to drm-tip/drm-tip.

Thanks for pointing it out. Fixed.

>
>> +						       true, has_hdmi_sink);
>> +
>> +		/* if we can't do 8,12bpc we may still be able to do 10bpc */
>> +		if (status != MODE_OK && INTEL_GEN(dev_priv) >= 11)
>> +			status = hdmi_port_clock_valid(hdmi, clock * 5 / 4,
> Also here.
>
>> +						       true, has_hdmi_sink);
>> +	}
>> +
>> +	return status;
>> +}
>> +
>>  static enum drm_mode_status
>>  intel_hdmi_mode_valid(struct drm_connector *connector,
>>  		      struct drm_display_mode *mode)
>> @@ -1891,21 +1916,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>>  	if (drm_mode_is_420_only(&connector->display_info, mode))
>>  		clock /= 2;
>>  
>> -	/* check if we can do 8bpc */
>> -	status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 8),
>> -				       true, has_hdmi_sink);
>> -
>> -	if (has_hdmi_sink) {
>> -		/* if we can't do 8bpc we may still be able to do 12bpc */
>> -		if (status != MODE_OK && !HAS_GMCH(dev_priv))
>> -			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 12),
>> -						       true, has_hdmi_sink);
>> -
>> -		/* if we can't do 8,12bpc we may still be able to do 10bpc */
>> -		if (status != MODE_OK && DISPLAY_VER(dev_priv) >= 11)
>> -			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 10),
>> -						       true, has_hdmi_sink);
>> -	}
>> +	status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
>>  	if (status != MODE_OK)
>>  		return status;
>>  
>> -- 
>> 2.25.1

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v6 1/3] drm/i915/display: New function to avoid duplicate code in upcomming commits
@ 2021-05-07 20:33         ` Werner Sembach
  0 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-07 20:33 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: airlied, intel-gfx, dri-devel, linux-kernel

Am 07.05.21 um 19:47 schrieb Ville Syrjälä:

> On Fri, May 07, 2021 at 10:49:01AM +0200, Werner Sembach wrote:
>> Moves some checks that later will be performed 2 times to an own function.
>> This avoids duplicate code later on.
>>
>> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_hdmi.c | 41 ++++++++++++++---------
>>  1 file changed, 26 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> index 46de56af33db..576d3d910d06 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> @@ -1861,6 +1861,31 @@ static int intel_hdmi_port_clock(int clock, int bpc)
>>  	return clock * bpc / 8;
>>  }
>>  
>> +static enum drm_mode_status
>> +intel_hdmi_mode_clock_valid(struct intel_hdmi *hdmi, int clock, bool has_hdmi_sink)
>> +{
>> +	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
>> +	struct drm_i915_private *dev_priv = to_i915(dev);
>> +	enum drm_mode_status status;
>> +
>> +	/* check if we can do 8bpc */
>> +	status = hdmi_port_clock_valid(hdmi, clock, true, has_hdmi_sink);
>> +
>> +	if (has_hdmi_sink) {
>> +		/* if we can't do 8bpc we may still be able to do 12bpc */
>> +		if (status != MODE_OK && !HAS_GMCH(dev_priv))
>> +			status = hdmi_port_clock_valid(hdmi, clock * 3 / 2,
> Seems we've lost intel_hdmi_port_clock() here somehow.

Yes, I think it happened when I rebased from torvalds/master to drm-tip/drm-tip.

Thanks for pointing it out. Fixed.

>
>> +						       true, has_hdmi_sink);
>> +
>> +		/* if we can't do 8,12bpc we may still be able to do 10bpc */
>> +		if (status != MODE_OK && INTEL_GEN(dev_priv) >= 11)
>> +			status = hdmi_port_clock_valid(hdmi, clock * 5 / 4,
> Also here.
>
>> +						       true, has_hdmi_sink);
>> +	}
>> +
>> +	return status;
>> +}
>> +
>>  static enum drm_mode_status
>>  intel_hdmi_mode_valid(struct drm_connector *connector,
>>  		      struct drm_display_mode *mode)
>> @@ -1891,21 +1916,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>>  	if (drm_mode_is_420_only(&connector->display_info, mode))
>>  		clock /= 2;
>>  
>> -	/* check if we can do 8bpc */
>> -	status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 8),
>> -				       true, has_hdmi_sink);
>> -
>> -	if (has_hdmi_sink) {
>> -		/* if we can't do 8bpc we may still be able to do 12bpc */
>> -		if (status != MODE_OK && !HAS_GMCH(dev_priv))
>> -			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 12),
>> -						       true, has_hdmi_sink);
>> -
>> -		/* if we can't do 8,12bpc we may still be able to do 10bpc */
>> -		if (status != MODE_OK && DISPLAY_VER(dev_priv) >= 11)
>> -			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 10),
>> -						       true, has_hdmi_sink);
>> -	}
>> +	status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
>>  	if (status != MODE_OK)
>>  		return status;
>>  
>> -- 
>> 2.25.1

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Intel-gfx] [PATCH v6 1/3] drm/i915/display: New function to avoid duplicate code in upcomming commits
@ 2021-05-07 20:33         ` Werner Sembach
  0 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-07 20:33 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: airlied, intel-gfx, dri-devel, linux-kernel

Am 07.05.21 um 19:47 schrieb Ville Syrjälä:

> On Fri, May 07, 2021 at 10:49:01AM +0200, Werner Sembach wrote:
>> Moves some checks that later will be performed 2 times to an own function.
>> This avoids duplicate code later on.
>>
>> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_hdmi.c | 41 ++++++++++++++---------
>>  1 file changed, 26 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> index 46de56af33db..576d3d910d06 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> @@ -1861,6 +1861,31 @@ static int intel_hdmi_port_clock(int clock, int bpc)
>>  	return clock * bpc / 8;
>>  }
>>  
>> +static enum drm_mode_status
>> +intel_hdmi_mode_clock_valid(struct intel_hdmi *hdmi, int clock, bool has_hdmi_sink)
>> +{
>> +	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
>> +	struct drm_i915_private *dev_priv = to_i915(dev);
>> +	enum drm_mode_status status;
>> +
>> +	/* check if we can do 8bpc */
>> +	status = hdmi_port_clock_valid(hdmi, clock, true, has_hdmi_sink);
>> +
>> +	if (has_hdmi_sink) {
>> +		/* if we can't do 8bpc we may still be able to do 12bpc */
>> +		if (status != MODE_OK && !HAS_GMCH(dev_priv))
>> +			status = hdmi_port_clock_valid(hdmi, clock * 3 / 2,
> Seems we've lost intel_hdmi_port_clock() here somehow.

Yes, I think it happened when I rebased from torvalds/master to drm-tip/drm-tip.

Thanks for pointing it out. Fixed.

>
>> +						       true, has_hdmi_sink);
>> +
>> +		/* if we can't do 8,12bpc we may still be able to do 10bpc */
>> +		if (status != MODE_OK && INTEL_GEN(dev_priv) >= 11)
>> +			status = hdmi_port_clock_valid(hdmi, clock * 5 / 4,
> Also here.
>
>> +						       true, has_hdmi_sink);
>> +	}
>> +
>> +	return status;
>> +}
>> +
>>  static enum drm_mode_status
>>  intel_hdmi_mode_valid(struct drm_connector *connector,
>>  		      struct drm_display_mode *mode)
>> @@ -1891,21 +1916,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>>  	if (drm_mode_is_420_only(&connector->display_info, mode))
>>  		clock /= 2;
>>  
>> -	/* check if we can do 8bpc */
>> -	status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 8),
>> -				       true, has_hdmi_sink);
>> -
>> -	if (has_hdmi_sink) {
>> -		/* if we can't do 8bpc we may still be able to do 12bpc */
>> -		if (status != MODE_OK && !HAS_GMCH(dev_priv))
>> -			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 12),
>> -						       true, has_hdmi_sink);
>> -
>> -		/* if we can't do 8,12bpc we may still be able to do 10bpc */
>> -		if (status != MODE_OK && DISPLAY_VER(dev_priv) >= 11)
>> -			status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 10),
>> -						       true, has_hdmi_sink);
>> -	}
>> +	status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
>>  	if (status != MODE_OK)
>>  		return status;
>>  
>> -- 
>> 2.25.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v6 2/3] drm/i915/display: Restructure output format computation for better expandability
  2021-05-07 17:52       ` Ville Syrjälä
  (?)
@ 2021-05-07 21:24         ` Werner Sembach
  -1 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-07 21:24 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: airlied, daniel, intel-gfx, dri-devel, linux-kernel

Am 07.05.21 um 19:52 schrieb Ville Syrjälä:
> On Fri, May 07, 2021 at 10:49:02AM +0200, Werner Sembach wrote:
>> Couples the decission between RGB and YCbCr420 mode and the check if the
>> port clock can archive the required frequency. Other checks and
>> configuration steps that where previously done in between can also be done
>> before or after.
>>
>> This allows for are cleaner implementation of retrying different color
>> encodings.
>>
>> A slight change in behaviour occurs with this patch: If YCbCr420 is not
>> allowed but display is YCbCr420 only it no longer fails, but just prints
>> an error and tries to fallback on RGB.
>>
>> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_hdmi.c | 65 ++++++++++++-----------
>>  1 file changed, 34 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> index 576d3d910d06..9f3da72dabee 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> @@ -1999,29 +1999,6 @@ static bool hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
>>  					      INTEL_OUTPUT_FORMAT_YCBCR420);
>>  }
>>  
>> -static int
>> -intel_hdmi_ycbcr420_config(struct intel_crtc_state *crtc_state,
>> -			   const struct drm_connector_state *conn_state)
>> -{
>> -	struct drm_connector *connector = conn_state->connector;
>> -	struct drm_i915_private *i915 = to_i915(connector->dev);
>> -	const struct drm_display_mode *adjusted_mode =
>> -		&crtc_state->hw.adjusted_mode;
>> -
>> -	if (!drm_mode_is_420_only(&connector->display_info, adjusted_mode))
>> -		return 0;
>> -
>> -	if (!connector->ycbcr_420_allowed) {
>> -		drm_err(&i915->drm,
>> -			"Platform doesn't support YCBCR420 output\n");
>> -		return -EINVAL;
>> -	}
>> -
>> -	crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>> -
>> -	return intel_pch_panel_fitting(crtc_state, conn_state);
>> -}
>> -
>>  static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
>>  				  struct intel_crtc_state *crtc_state,
>>  				  int clock)
>> @@ -2128,6 +2105,30 @@ static bool intel_hdmi_has_audio(struct intel_encoder *encoder,
>>  		return intel_conn_state->force_audio == HDMI_AUDIO_ON;
>>  }
>>  
>> +static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
>> +					    struct intel_crtc_state *crtc_state,
>> +					    const struct drm_connector_state *conn_state)
>> +{
>> +	struct drm_connector *connector = conn_state->connector;
>> +	struct drm_i915_private *i915 = to_i915(connector->dev);
>> +	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
>> +	int ret;
>> +	bool ycbcr_420_only;
>> +
>> +	ycbcr_420_only = drm_mode_is_420_only(&connector->display_info, adjusted_mode);
>> +	if (connector->ycbcr_420_allowed && ycbcr_420_only) {
>> +		crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>> +	} else {
>> +		if (!connector->ycbcr_420_allowed && ycbcr_420_only)
>> +			drm_err(&i915->drm, "Display only supports YCbCr420 output, but connector does not allow it. Fallback to RGB, but this will likely fail.\n");
> We can't let the user spam dmesg with errors freely. So this needs
> to be a drm_dbg_kms(). Also a bit long, so going to annoyingly wrap
> always. Could maybe shorten a bit to something like:
> "YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible. Falling back to RGB."
Done, but I don't have time anymore to test it today. I will test and submit v7 on Monday.
>
> With that sorted, and the intel_hdmi_port_clock() stuff restored,
> I believe this series is good to go.
>
> I think you confused our CI by replying to the old patch with a whole
> new series. It can generally deal with a whole new series as a new
> thread or replies to individual patches with updated versions of
> exactly that patch, but not full series as a reply to a patch.
> So I suggest just posting the final versions as a new series. Thanks.
>
>> +		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
>> +	}
>> +
>> +	ret = intel_hdmi_compute_clock(encoder, crtc_state);
>> +
>> +	return ret;
>> +}
>> +
>>  int intel_hdmi_compute_config(struct intel_encoder *encoder,
>>  			      struct intel_crtc_state *pipe_config,
>>  			      struct drm_connector_state *conn_state)
>> @@ -2152,23 +2153,25 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
>>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
>>  		pipe_config->pixel_multiplier = 2;
>>  
>> -	ret = intel_hdmi_ycbcr420_config(pipe_config, conn_state);
>> -	if (ret)
>> -		return ret;
>> -
>> -	pipe_config->limited_color_range =
>> -		intel_hdmi_limited_color_range(pipe_config, conn_state);
>> -
>>  	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv))
>>  		pipe_config->has_pch_encoder = true;
>>  
>>  	pipe_config->has_audio =
>>  		intel_hdmi_has_audio(encoder, pipe_config, conn_state);
>>  
>> -	ret = intel_hdmi_compute_clock(encoder, pipe_config);
>> +	ret = intel_hdmi_compute_output_format(encoder, pipe_config, conn_state);
>>  	if (ret)
>>  		return ret;
>>  
>> +	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
>> +		ret = intel_pch_panel_fitting(pipe_config, conn_state);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	pipe_config->limited_color_range =
>> +		intel_hdmi_limited_color_range(pipe_config, conn_state);
>> +
>>  	if (conn_state->picture_aspect_ratio)
>>  		adjusted_mode->picture_aspect_ratio =
>>  			conn_state->picture_aspect_ratio;
>> -- 
>> 2.25.1

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v6 2/3] drm/i915/display: Restructure output format computation for better expandability
@ 2021-05-07 21:24         ` Werner Sembach
  0 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-07 21:24 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: airlied, intel-gfx, dri-devel, linux-kernel

Am 07.05.21 um 19:52 schrieb Ville Syrjälä:
> On Fri, May 07, 2021 at 10:49:02AM +0200, Werner Sembach wrote:
>> Couples the decission between RGB and YCbCr420 mode and the check if the
>> port clock can archive the required frequency. Other checks and
>> configuration steps that where previously done in between can also be done
>> before or after.
>>
>> This allows for are cleaner implementation of retrying different color
>> encodings.
>>
>> A slight change in behaviour occurs with this patch: If YCbCr420 is not
>> allowed but display is YCbCr420 only it no longer fails, but just prints
>> an error and tries to fallback on RGB.
>>
>> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_hdmi.c | 65 ++++++++++++-----------
>>  1 file changed, 34 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> index 576d3d910d06..9f3da72dabee 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> @@ -1999,29 +1999,6 @@ static bool hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
>>  					      INTEL_OUTPUT_FORMAT_YCBCR420);
>>  }
>>  
>> -static int
>> -intel_hdmi_ycbcr420_config(struct intel_crtc_state *crtc_state,
>> -			   const struct drm_connector_state *conn_state)
>> -{
>> -	struct drm_connector *connector = conn_state->connector;
>> -	struct drm_i915_private *i915 = to_i915(connector->dev);
>> -	const struct drm_display_mode *adjusted_mode =
>> -		&crtc_state->hw.adjusted_mode;
>> -
>> -	if (!drm_mode_is_420_only(&connector->display_info, adjusted_mode))
>> -		return 0;
>> -
>> -	if (!connector->ycbcr_420_allowed) {
>> -		drm_err(&i915->drm,
>> -			"Platform doesn't support YCBCR420 output\n");
>> -		return -EINVAL;
>> -	}
>> -
>> -	crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>> -
>> -	return intel_pch_panel_fitting(crtc_state, conn_state);
>> -}
>> -
>>  static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
>>  				  struct intel_crtc_state *crtc_state,
>>  				  int clock)
>> @@ -2128,6 +2105,30 @@ static bool intel_hdmi_has_audio(struct intel_encoder *encoder,
>>  		return intel_conn_state->force_audio == HDMI_AUDIO_ON;
>>  }
>>  
>> +static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
>> +					    struct intel_crtc_state *crtc_state,
>> +					    const struct drm_connector_state *conn_state)
>> +{
>> +	struct drm_connector *connector = conn_state->connector;
>> +	struct drm_i915_private *i915 = to_i915(connector->dev);
>> +	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
>> +	int ret;
>> +	bool ycbcr_420_only;
>> +
>> +	ycbcr_420_only = drm_mode_is_420_only(&connector->display_info, adjusted_mode);
>> +	if (connector->ycbcr_420_allowed && ycbcr_420_only) {
>> +		crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>> +	} else {
>> +		if (!connector->ycbcr_420_allowed && ycbcr_420_only)
>> +			drm_err(&i915->drm, "Display only supports YCbCr420 output, but connector does not allow it. Fallback to RGB, but this will likely fail.\n");
> We can't let the user spam dmesg with errors freely. So this needs
> to be a drm_dbg_kms(). Also a bit long, so going to annoyingly wrap
> always. Could maybe shorten a bit to something like:
> "YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible. Falling back to RGB."
Done, but I don't have time anymore to test it today. I will test and submit v7 on Monday.
>
> With that sorted, and the intel_hdmi_port_clock() stuff restored,
> I believe this series is good to go.
>
> I think you confused our CI by replying to the old patch with a whole
> new series. It can generally deal with a whole new series as a new
> thread or replies to individual patches with updated versions of
> exactly that patch, but not full series as a reply to a patch.
> So I suggest just posting the final versions as a new series. Thanks.
>
>> +		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
>> +	}
>> +
>> +	ret = intel_hdmi_compute_clock(encoder, crtc_state);
>> +
>> +	return ret;
>> +}
>> +
>>  int intel_hdmi_compute_config(struct intel_encoder *encoder,
>>  			      struct intel_crtc_state *pipe_config,
>>  			      struct drm_connector_state *conn_state)
>> @@ -2152,23 +2153,25 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
>>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
>>  		pipe_config->pixel_multiplier = 2;
>>  
>> -	ret = intel_hdmi_ycbcr420_config(pipe_config, conn_state);
>> -	if (ret)
>> -		return ret;
>> -
>> -	pipe_config->limited_color_range =
>> -		intel_hdmi_limited_color_range(pipe_config, conn_state);
>> -
>>  	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv))
>>  		pipe_config->has_pch_encoder = true;
>>  
>>  	pipe_config->has_audio =
>>  		intel_hdmi_has_audio(encoder, pipe_config, conn_state);
>>  
>> -	ret = intel_hdmi_compute_clock(encoder, pipe_config);
>> +	ret = intel_hdmi_compute_output_format(encoder, pipe_config, conn_state);
>>  	if (ret)
>>  		return ret;
>>  
>> +	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
>> +		ret = intel_pch_panel_fitting(pipe_config, conn_state);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	pipe_config->limited_color_range =
>> +		intel_hdmi_limited_color_range(pipe_config, conn_state);
>> +
>>  	if (conn_state->picture_aspect_ratio)
>>  		adjusted_mode->picture_aspect_ratio =
>>  			conn_state->picture_aspect_ratio;
>> -- 
>> 2.25.1

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Intel-gfx] [PATCH v6 2/3] drm/i915/display: Restructure output format computation for better expandability
@ 2021-05-07 21:24         ` Werner Sembach
  0 siblings, 0 replies; 34+ messages in thread
From: Werner Sembach @ 2021-05-07 21:24 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: airlied, intel-gfx, dri-devel, linux-kernel

Am 07.05.21 um 19:52 schrieb Ville Syrjälä:
> On Fri, May 07, 2021 at 10:49:02AM +0200, Werner Sembach wrote:
>> Couples the decission between RGB and YCbCr420 mode and the check if the
>> port clock can archive the required frequency. Other checks and
>> configuration steps that where previously done in between can also be done
>> before or after.
>>
>> This allows for are cleaner implementation of retrying different color
>> encodings.
>>
>> A slight change in behaviour occurs with this patch: If YCbCr420 is not
>> allowed but display is YCbCr420 only it no longer fails, but just prints
>> an error and tries to fallback on RGB.
>>
>> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_hdmi.c | 65 ++++++++++++-----------
>>  1 file changed, 34 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> index 576d3d910d06..9f3da72dabee 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> @@ -1999,29 +1999,6 @@ static bool hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
>>  					      INTEL_OUTPUT_FORMAT_YCBCR420);
>>  }
>>  
>> -static int
>> -intel_hdmi_ycbcr420_config(struct intel_crtc_state *crtc_state,
>> -			   const struct drm_connector_state *conn_state)
>> -{
>> -	struct drm_connector *connector = conn_state->connector;
>> -	struct drm_i915_private *i915 = to_i915(connector->dev);
>> -	const struct drm_display_mode *adjusted_mode =
>> -		&crtc_state->hw.adjusted_mode;
>> -
>> -	if (!drm_mode_is_420_only(&connector->display_info, adjusted_mode))
>> -		return 0;
>> -
>> -	if (!connector->ycbcr_420_allowed) {
>> -		drm_err(&i915->drm,
>> -			"Platform doesn't support YCBCR420 output\n");
>> -		return -EINVAL;
>> -	}
>> -
>> -	crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>> -
>> -	return intel_pch_panel_fitting(crtc_state, conn_state);
>> -}
>> -
>>  static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
>>  				  struct intel_crtc_state *crtc_state,
>>  				  int clock)
>> @@ -2128,6 +2105,30 @@ static bool intel_hdmi_has_audio(struct intel_encoder *encoder,
>>  		return intel_conn_state->force_audio == HDMI_AUDIO_ON;
>>  }
>>  
>> +static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
>> +					    struct intel_crtc_state *crtc_state,
>> +					    const struct drm_connector_state *conn_state)
>> +{
>> +	struct drm_connector *connector = conn_state->connector;
>> +	struct drm_i915_private *i915 = to_i915(connector->dev);
>> +	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
>> +	int ret;
>> +	bool ycbcr_420_only;
>> +
>> +	ycbcr_420_only = drm_mode_is_420_only(&connector->display_info, adjusted_mode);
>> +	if (connector->ycbcr_420_allowed && ycbcr_420_only) {
>> +		crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>> +	} else {
>> +		if (!connector->ycbcr_420_allowed && ycbcr_420_only)
>> +			drm_err(&i915->drm, "Display only supports YCbCr420 output, but connector does not allow it. Fallback to RGB, but this will likely fail.\n");
> We can't let the user spam dmesg with errors freely. So this needs
> to be a drm_dbg_kms(). Also a bit long, so going to annoyingly wrap
> always. Could maybe shorten a bit to something like:
> "YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible. Falling back to RGB."
Done, but I don't have time anymore to test it today. I will test and submit v7 on Monday.
>
> With that sorted, and the intel_hdmi_port_clock() stuff restored,
> I believe this series is good to go.
>
> I think you confused our CI by replying to the old patch with a whole
> new series. It can generally deal with a whole new series as a new
> thread or replies to individual patches with updated versions of
> exactly that patch, but not full series as a reply to a patch.
> So I suggest just posting the final versions as a new series. Thanks.
>
>> +		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
>> +	}
>> +
>> +	ret = intel_hdmi_compute_clock(encoder, crtc_state);
>> +
>> +	return ret;
>> +}
>> +
>>  int intel_hdmi_compute_config(struct intel_encoder *encoder,
>>  			      struct intel_crtc_state *pipe_config,
>>  			      struct drm_connector_state *conn_state)
>> @@ -2152,23 +2153,25 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
>>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
>>  		pipe_config->pixel_multiplier = 2;
>>  
>> -	ret = intel_hdmi_ycbcr420_config(pipe_config, conn_state);
>> -	if (ret)
>> -		return ret;
>> -
>> -	pipe_config->limited_color_range =
>> -		intel_hdmi_limited_color_range(pipe_config, conn_state);
>> -
>>  	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv))
>>  		pipe_config->has_pch_encoder = true;
>>  
>>  	pipe_config->has_audio =
>>  		intel_hdmi_has_audio(encoder, pipe_config, conn_state);
>>  
>> -	ret = intel_hdmi_compute_clock(encoder, pipe_config);
>> +	ret = intel_hdmi_compute_output_format(encoder, pipe_config, conn_state);
>>  	if (ret)
>>  		return ret;
>>  
>> +	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
>> +		ret = intel_pch_panel_fitting(pipe_config, conn_state);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	pipe_config->limited_color_range =
>> +		intel_hdmi_limited_color_range(pipe_config, conn_state);
>> +
>>  	if (conn_state->picture_aspect_ratio)
>>  		adjusted_mode->picture_aspect_ratio =
>>  			conn_state->picture_aspect_ratio;
>> -- 
>> 2.25.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2021-05-07 21:25 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-06 17:23 [PATCH 0/3] drm/i915/display: Try YCbCr420 color when RGB fails Werner Sembach
2021-05-06 17:23 ` [Intel-gfx] " Werner Sembach
2021-05-06 17:23 ` [PATCH 1/3] drm/i915/display: New function to avoid duplicate code in upcomming commits Werner Sembach
2021-05-06 17:23   ` [Intel-gfx] " Werner Sembach
2021-05-06 17:23 ` [PATCH 2/3] drm/i915/display: Restructure output format computation for better expandability Werner Sembach
2021-05-06 17:23   ` [Intel-gfx] " Werner Sembach
2021-05-06 17:23 ` [PATCH 3/3] drm/i915/display: Use YCbCr420 as fallback when RGB fails Werner Sembach
2021-05-06 17:23   ` [Intel-gfx] " Werner Sembach
2021-05-06 17:35 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/display: Try YCbCr420 color " Patchwork
2021-05-06 18:04 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-05-06 19:21 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2021-05-07  8:49 ` [PATCH v6 0/3] " Werner Sembach
2021-05-07  8:49   ` [Intel-gfx] " Werner Sembach
2021-05-07  8:49   ` [PATCH v6 1/3] drm/i915/display: New function to avoid duplicate code in upcomming commits Werner Sembach
2021-05-07  8:49     ` [Intel-gfx] " Werner Sembach
2021-05-07 17:47     ` Ville Syrjälä
2021-05-07 17:47       ` [Intel-gfx] " Ville Syrjälä
2021-05-07 17:47       ` Ville Syrjälä
2021-05-07 20:33       ` Werner Sembach
2021-05-07 20:33         ` [Intel-gfx] " Werner Sembach
2021-05-07 20:33         ` Werner Sembach
2021-05-07  8:49   ` [PATCH v6 2/3] drm/i915/display: Restructure output format computation for better expandability Werner Sembach
2021-05-07  8:49     ` [Intel-gfx] " Werner Sembach
2021-05-07 17:52     ` Ville Syrjälä
2021-05-07 17:52       ` [Intel-gfx] " Ville Syrjälä
2021-05-07 17:52       ` Ville Syrjälä
2021-05-07 17:56       ` Randy Dunlap
2021-05-07 17:56         ` [Intel-gfx] " Randy Dunlap
2021-05-07 17:56         ` Randy Dunlap
2021-05-07 21:24       ` Werner Sembach
2021-05-07 21:24         ` [Intel-gfx] " Werner Sembach
2021-05-07 21:24         ` Werner Sembach
2021-05-07  8:49   ` [PATCH v6 3/3] drm/i915/display: Use YCbCr420 as fallback when RGB fails Werner Sembach
2021-05-07  8:49     ` [Intel-gfx] " Werner Sembach

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.