diff mbox series

[2/3] Restructure output format computation for better expandability

Message ID 20210505141052.3467-3-wse@tuxedocomputers.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/display Try YCbCr420 color when RGB fails | expand

Commit Message

Werner Sembach May 5, 2021, 2:10 p.m. UTC 
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>
---
Imho an error message in when YCbCR420 not allowed meets YCbCr420 only can be
a usefull bugfinding tool for cases of blackscreen on exotic configurations.

I'm unsure if this should be a warning instead.

From 50f0f396c3363dc05291d8a0873a9c9f23fa5b4b Mon Sep 17 00:00:00 2001
From: Werner Sembach <wse@tuxedocomputers.com>
Date: Mon, 3 May 2021 15:30:40 +0200
Subject: [PATCH 2/3] Restructure output format computation for better
 expandability

---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 65 ++++++++++++-----------
 1 file changed, 34 insertions(+), 31 deletions(-)

Comments

kernel test robot May 5, 2021, 7:03 p.m. UTC | #1 
Hi Werner,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on next-20210505]
[cannot apply to v5.12]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Werner-Sembach/New-function-to-avoid-duplicate-code-in-upcomming-commits/20210505-231103
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-defconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/66df58af8a9f5c112690fc80f77271b9f6567765
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Werner-Sembach/New-function-to-avoid-duplicate-code-in-upcomming-commits/20210505-231103
        git checkout 66df58af8a9f5c112690fc80f77271b9f6567765
        # save the attached .config to linux build tree
        make W=1 W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/gpu/drm/i915/display/intel_hdmi.c: In function 'intel_hdmi_compute_config':
>> drivers/gpu/drm/i915/display/intel_hdmi.c:2166:6: error: 'crtc_state' undeclared (first use in this function); did you mean 'ctx_state'?
    2166 |  if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
         |      ^~~~~~~~~~
         |      ctx_state
   drivers/gpu/drm/i915/display/intel_hdmi.c:2166:6: note: each undeclared identifier is reported only once for each function it appears in


vim +2166 drivers/gpu/drm/i915/display/intel_hdmi.c

  2131	
  2132	int intel_hdmi_compute_config(struct intel_encoder *encoder,
  2133				      struct intel_crtc_state *pipe_config,
  2134				      struct drm_connector_state *conn_state)
  2135	{
  2136		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
  2137		struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
  2138		struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
  2139		struct drm_connector *connector = conn_state->connector;
  2140		struct drm_scdc *scdc = &connector->display_info.hdmi.scdc;
  2141		int ret;
  2142	
  2143		if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
  2144			return -EINVAL;
  2145	
  2146		pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
  2147		pipe_config->has_hdmi_sink = intel_has_hdmi_sink(intel_hdmi,
  2148								 conn_state);
  2149	
  2150		if (pipe_config->has_hdmi_sink)
  2151			pipe_config->has_infoframe = true;
  2152	
  2153		if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
  2154			pipe_config->pixel_multiplier = 2;
  2155	
  2156		if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv))
  2157			pipe_config->has_pch_encoder = true;
  2158	
  2159		pipe_config->has_audio =
  2160			intel_hdmi_has_audio(encoder, pipe_config, conn_state);
  2161	
  2162		ret = intel_hdmi_compute_output_format(encoder, pipe_config, conn_state);
  2163		if (ret)
  2164			return ret;
  2165	
> 2166		if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
  2167			ret = intel_pch_panel_fitting(pipe_config, conn_state);
  2168			if (ret)
  2169				return ret;
  2170		}
  2171	
  2172		pipe_config->limited_color_range =
  2173			intel_hdmi_limited_color_range(pipe_config, conn_state);
  2174	
  2175		if (conn_state->picture_aspect_ratio)
  2176			adjusted_mode->picture_aspect_ratio =
  2177				conn_state->picture_aspect_ratio;
  2178	
  2179		pipe_config->lane_count = 4;
  2180	
  2181		if (scdc->scrambling.supported && DISPLAY_VER(dev_priv) >= 10) {
  2182			if (scdc->scrambling.low_rates)
  2183				pipe_config->hdmi_scrambling = true;
  2184	
  2185			if (pipe_config->port_clock > 340000) {
  2186				pipe_config->hdmi_scrambling = true;
  2187				pipe_config->hdmi_high_tmds_clock_ratio = true;
  2188			}
  2189		}
  2190	
  2191		intel_hdmi_compute_gcp_infoframe(encoder, pipe_config,
  2192						 conn_state);
  2193	
  2194		if (!intel_hdmi_compute_avi_infoframe(encoder, pipe_config, conn_state)) {
  2195			drm_dbg_kms(&dev_priv->drm, "bad AVI infoframe\n");
  2196			return -EINVAL;
  2197		}
  2198	
  2199		if (!intel_hdmi_compute_spd_infoframe(encoder, pipe_config, conn_state)) {
  2200			drm_dbg_kms(&dev_priv->drm, "bad SPD infoframe\n");
  2201			return -EINVAL;
  2202		}
  2203	
  2204		if (!intel_hdmi_compute_hdmi_infoframe(encoder, pipe_config, conn_state)) {
  2205			drm_dbg_kms(&dev_priv->drm, "bad HDMI infoframe\n");
  2206			return -EINVAL;
  2207		}
  2208	
  2209		if (!intel_hdmi_compute_drm_infoframe(encoder, pipe_config, conn_state)) {
  2210			drm_dbg_kms(&dev_priv->drm, "bad DRM infoframe\n");
  2211			return -EINVAL;
  2212		}
  2213	
  2214		return 0;
  2215	}
  2216	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot May 5, 2021, 8:17 p.m. UTC | #2 
Hi Werner,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on next-20210505]
[cannot apply to v5.12]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Werner-Sembach/New-function-to-avoid-duplicate-code-in-upcomming-commits/20210505-231103
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-a012-20210505 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 8f5a2a5836cc8e4c1def2bdeb022e7b496623439)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/66df58af8a9f5c112690fc80f77271b9f6567765
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Werner-Sembach/New-function-to-avoid-duplicate-code-in-upcomming-commits/20210505-231103
        git checkout 66df58af8a9f5c112690fc80f77271b9f6567765
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/i915/display/intel_hdmi.c:2166:6: error: use of undeclared identifier 'crtc_state'
           if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
               ^
   1 error generated.


vim +/crtc_state +2166 drivers/gpu/drm/i915/display/intel_hdmi.c

  2131	
  2132	int intel_hdmi_compute_config(struct intel_encoder *encoder,
  2133				      struct intel_crtc_state *pipe_config,
  2134				      struct drm_connector_state *conn_state)
  2135	{
  2136		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
  2137		struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
  2138		struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
  2139		struct drm_connector *connector = conn_state->connector;
  2140		struct drm_scdc *scdc = &connector->display_info.hdmi.scdc;
  2141		int ret;
  2142	
  2143		if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
  2144			return -EINVAL;
  2145	
  2146		pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
  2147		pipe_config->has_hdmi_sink = intel_has_hdmi_sink(intel_hdmi,
  2148								 conn_state);
  2149	
  2150		if (pipe_config->has_hdmi_sink)
  2151			pipe_config->has_infoframe = true;
  2152	
  2153		if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
  2154			pipe_config->pixel_multiplier = 2;
  2155	
  2156		if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv))
  2157			pipe_config->has_pch_encoder = true;
  2158	
  2159		pipe_config->has_audio =
  2160			intel_hdmi_has_audio(encoder, pipe_config, conn_state);
  2161	
  2162		ret = intel_hdmi_compute_output_format(encoder, pipe_config, conn_state);
  2163		if (ret)
  2164			return ret;
  2165	
> 2166		if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
  2167			ret = intel_pch_panel_fitting(pipe_config, conn_state);
  2168			if (ret)
  2169				return ret;
  2170		}
  2171	
  2172		pipe_config->limited_color_range =
  2173			intel_hdmi_limited_color_range(pipe_config, conn_state);
  2174	
  2175		if (conn_state->picture_aspect_ratio)
  2176			adjusted_mode->picture_aspect_ratio =
  2177				conn_state->picture_aspect_ratio;
  2178	
  2179		pipe_config->lane_count = 4;
  2180	
  2181		if (scdc->scrambling.supported && DISPLAY_VER(dev_priv) >= 10) {
  2182			if (scdc->scrambling.low_rates)
  2183				pipe_config->hdmi_scrambling = true;
  2184	
  2185			if (pipe_config->port_clock > 340000) {
  2186				pipe_config->hdmi_scrambling = true;
  2187				pipe_config->hdmi_high_tmds_clock_ratio = true;
  2188			}
  2189		}
  2190	
  2191		intel_hdmi_compute_gcp_infoframe(encoder, pipe_config,
  2192						 conn_state);
  2193	
  2194		if (!intel_hdmi_compute_avi_infoframe(encoder, pipe_config, conn_state)) {
  2195			drm_dbg_kms(&dev_priv->drm, "bad AVI infoframe\n");
  2196			return -EINVAL;
  2197		}
  2198	
  2199		if (!intel_hdmi_compute_spd_infoframe(encoder, pipe_config, conn_state)) {
  2200			drm_dbg_kms(&dev_priv->drm, "bad SPD infoframe\n");
  2201			return -EINVAL;
  2202		}
  2203	
  2204		if (!intel_hdmi_compute_hdmi_infoframe(encoder, pipe_config, conn_state)) {
  2205			drm_dbg_kms(&dev_priv->drm, "bad HDMI infoframe\n");
  2206			return -EINVAL;
  2207		}
  2208	
  2209		if (!intel_hdmi_compute_drm_infoframe(encoder, pipe_config, conn_state)) {
  2210			drm_dbg_kms(&dev_priv->drm, "bad DRM infoframe\n");
  2211			return -EINVAL;
  2212		}
  2213	
  2214		return 0;
  2215	}
  2216	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 576d3d910d06..2c3b545d5f24 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 (crtc_state->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;