Re:[PATCH v13 15/28] drm/connector: hdmi: Compute bpc and format automatically

["Andy Yan" <andyshrk@163.com>]

Hi Maxime，

在 2024-05-07 21:17:33，"Maxime Ripard" <mripard@kernel.org> 写道：
>Now that we have all the infrastructure needed, we can add some code
>that will, for a given connector state and mode, compute the best output
>format and bpc.
>
>The algorithm is equivalent to the one already found in i915 and vc4.
>
>Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>Signed-off-by: Maxime Ripard <mripard@kernel.org>
>---
> drivers/gpu/drm/display/drm_hdmi_state_helper.c    | 199 ++++++++++++++++++++-
> drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c |  25 ++-
> 2 files changed, 212 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
>index 063421835dba..f20dcfecb6b8 100644
>--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
>+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
>@@ -1,9 +1,11 @@
> // SPDX-License-Identifier: MIT
> 
> #include <drm/drm_atomic.h>
> #include <drm/drm_connector.h>
>+#include <drm/drm_edid.h>
>+#include <drm/drm_print.h>
> 
> #include <drm/display/drm_hdmi_helper.h>
> #include <drm/display/drm_hdmi_state_helper.h>
> 
> /**
>@@ -46,10 +48,112 @@ connector_state_get_mode(const struct drm_connector_state *conn_state)
> 		return NULL;
> 
> 	return &crtc_state->mode;
> }
> 
>+static bool
>+sink_supports_format_bpc(const struct drm_connector *connector,
>+			 const struct drm_display_info *info,
>+			 const struct drm_display_mode *mode,
>+			 unsigned int format, unsigned int bpc)
>+{
>+	struct drm_device *dev = connector->dev;
>+	u8 vic = drm_match_cea_mode(mode);
>+
>+	/*
>+	 * CTA-861-F, section 5.4 - Color Coding & Quantization states
>+	 * that the bpc must be 8, 10, 12 or 16 except for the default
>+	 * 640x480 VIC1 where the value must be 8.
>+	 *
>+	 * The definition of default here is ambiguous but the spec
>+	 * refers to VIC1 being the default timing in several occasions
>+	 * so our understanding is that for the default timing (ie,
>+	 * VIC1), the bpc must be 8.
>+	 */
>+	if (vic == 1 && bpc != 8) {
>+		drm_dbg_kms(dev, "VIC1 requires a bpc of 8, got %u\n", bpc);
>+		return false;
>+	}
>+
>+	if (!info->is_hdmi &&
>+	    (format != HDMI_COLORSPACE_RGB || bpc != 8)) {
>+		drm_dbg_kms(dev, "DVI Monitors require an RGB output at 8 bpc\n");
>+		return false;
>+	}
>+
>+	if (!(connector->hdmi.supported_formats & BIT(format))) {
>+		drm_dbg_kms(dev, "%s format unsupported by the connector.\n",
>+			    drm_hdmi_connector_get_output_format_name(format));
>+		return false;
>+	}
>+
>+	switch (format) {
>+	case HDMI_COLORSPACE_RGB:
>+		drm_dbg_kms(dev, "RGB Format, checking the constraints.\n");
>+
>+		if (!(info->color_formats & DRM_COLOR_FORMAT_RGB444)) {
>+			drm_dbg_kms(dev, "Sink doesn't support RGB.\n");
>+			return false;
>+		}
>+
As I reported in V12,  the HDMI output on my rk3036-kylin was lost after apply this series.
This is because there is something wrong with the DDC on my board, the edid read always failed
on first bootup. That means inno_hdmi_connector_get_modes will return 0.

and in function drm_helper_probe_single_connector_modes:

         count = drm_helper_probe_get_modes(connector);

         if (count == 0 && (connector->status == connector_status_connected ||
                            connector->status == connector_status_unknown)) {
                 count = drm_add_modes_noedid(connector, 1024, 768);

                 /*
                  * Section 4.2.2.6 (EDID Corruption Detection) of the DP 1.4a
                  * Link CTS specifies that 640x480 (the official "failsafe"
                  * mode) needs to be the default if there's no EDID.
                  */
                 if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort)
                         drm_set_preferred_mode(connector, 640, 480);
         }
drm_add_modes_noedid will not initialize display_info. So the check about display info will always failed here:

[    4.205368] rockchip-drm display-subsystem: [drm:drm_atomic_check_only] checking (ptrval)
[    4.205410] rockchip-drm display-subsystem: [drm:drm_atomic_helper_check_modeset] [CRTC:35:crtc-0] mode changed
[    4.205439] rockchip-drm display-subsystem: [drm:drm_atomic_helper_check_modeset] [CRTC:35:crtc-0] enable changed
[    4.205464] rockchip-drm display-subsystem: [drm:drm_atomic_helper_check_modeset] [CRTC:35:crtc-0] active changed
[    4.205490] rockchip-drm display-subsystem: [drm:drm_atomic_helper_check_modeset] Updating routing for [CONNECTOR:37:HDMI-A-1]
[    4.205517] rockchip-drm display-subsystem: [drm:drm_atomic_helper_check_modeset] [CONNECTOR:37:HDMI-A-1] using [ENCODER:36:TMDS-36] on [CRTC:35:crtc-0]
[    4.205545] rockchip-drm display-subsystem: [drm:drm_atomic_helper_connector_hdmi_check] Trying with a 8 bpc output
[    4.205575] rockchip-drm display-subsystem: [drm:drm_atomic_helper_connector_hdmi_check] Trying RGB output format
[    4.205670] rockchip-drm display-subsystem: [drm:drm_atomic_helper_connector_hdmi_check] RGB Format, checking the constraints.
[    4.205696] rockchip-drm display-subsystem: [drm:drm_atomic_helper_connector_hdmi_check] Sink doesn't support RGB.
[    4.205720] rockchip-drm display-subsystem: [drm:drm_atomic_helper_connector_hdmi_check] RGB output format not supported with 8 bpc
[    4.205747] rockchip-drm display-subsystem: [drm:drm_atomic_helper_connector_hdmi_check] Failed. No Format Supported for that bpc count.
[    4.205772] rockchip-drm display-subsystem: [drm:drm_atomic_helper_check_modeset] [CONNECTOR:37:HDMI-A-1] driver check failed
[    4.205796] rockchip-drm display-subsystem: [drm:drm_atomic_check_only] atomic driver check for (ptrval) failed: -22

My reply for your email in V12[0] was bounced, so I think you didn't read it.

[0]https://patchwork.kernel.org/project/linux-rockchip/patch/20240423-kms-hdmi-connector-state-v12-27-3338e4c0b189@kernel.org/


>+		if (bpc == 10 && !(info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_30)) {
>+			drm_dbg_kms(dev, "10 BPC but sink doesn't support Deep Color 30.\n");
>+			return false;
>+		}
>+
>+		if (bpc == 12 && !(info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_36)) {
>+			drm_dbg_kms(dev, "12 BPC but sink doesn't support Deep Color 36.\n");
>+			return false;
>+		}
>+
>+		drm_dbg_kms(dev, "RGB format supported in that configuration.\n");
>+
>+		return true;
>+
>+	case HDMI_COLORSPACE_YUV422:
>+		drm_dbg_kms(dev, "YUV422 format, checking the constraints.\n");
>+
>+		if (!(info->color_formats & DRM_COLOR_FORMAT_YCBCR422)) {
>+			drm_dbg_kms(dev, "Sink doesn't support YUV422.\n");
>+			return false;
>+		}
>+
>+		if (bpc != 12) {
>+			drm_dbg_kms(dev, "YUV422 only supports 12 bpc.\n");
>+			return false;
>+		}
>+
>+		drm_dbg_kms(dev, "YUV422 format supported in that configuration.\n");
>+
>+		return true;
>+
>+	case HDMI_COLORSPACE_YUV444:
>+		drm_dbg_kms(dev, "YUV444 format, checking the constraints.\n");
>+
>+		if (!(info->color_formats & DRM_COLOR_FORMAT_YCBCR444)) {
>+			drm_dbg_kms(dev, "Sink doesn't support YUV444.\n");
>+			return false;
>+		}
>+
>+		if (bpc == 10 && !(info->edid_hdmi_ycbcr444_dc_modes & DRM_EDID_HDMI_DC_30)) {
>+			drm_dbg_kms(dev, "10 BPC but sink doesn't support Deep Color 30.\n");
>+			return false;
>+		}
>+
>+		if (bpc == 12 && !(info->edid_hdmi_ycbcr444_dc_modes & DRM_EDID_HDMI_DC_36)) {
>+			drm_dbg_kms(dev, "12 BPC but sink doesn't support Deep Color 36.\n");
>+			return false;
>+		}
>+
>+		drm_dbg_kms(dev, "YUV444 format supported in that configuration.\n");
>+
>+		return true;
>+	}
>+
>+	return false;
>+}
>+

>-- 
>2.45.0
>
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel