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 3. I did not (yet) include all suggested changes as I'm waiting for some clarification. See my last emails.
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 --- PS.: I have no idea why git send-email is messing up this timestamp in "From" below.
From 42a4a3a7d9ea9948b4071f406e7fcae23bfa0bdf Mon Sep 17 00:00:00 2001
From: Werner Sembach wse@tuxedocomputers.com Date: Mon, 3 May 2021 14:35:39 +0200 Subject: [PATCH 1/3] New function to avoid duplicate code in upcomming commits
--- 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;
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(-)
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;
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-avoi... 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/66df58af8a9f5c112690fc80f77271b9f656... 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
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-avoi... 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/66df58af8a9f5c112690fc80f77271b9f656... 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
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 --- I did not include the suggested change at intel_hdmi_mode_valid, because as far as I can tell it doesn't make a difference.
From 882afbfd5de6858c01bcef9580305082ce811701 Mon Sep 17 00:00:00 2001
From: Werner Sembach wse@tuxedocomputers.com Date: Mon, 3 May 2021 16:23:17 +0200 Subject: [PATCH 3/3] Use YCbCr420 as fallback when RGB fails
--- drivers/gpu/drm/i915/display/intel_hdmi.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index 2c3b545d5f24..3770ea00930f 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -1913,7 +1913,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector, clock *= 2; }
- if (drm_mode_is_420_only(&connector->display_info, mode)) + if (connector->ycbcr_420_allowed && drm_mode_is_420(&connector->display_info, mode)) clock /= 2;
status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink); @@ -2125,6 +2125,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; }
dri-devel@lists.freedesktop.org