[PATCH v2 0/7] drm/connector: hdmi: Allow using the YUV420 output format

[PATCH v2 0/7] drm/connector: hdmi: Allow using the YUV420 output format

[Cristian Ciocaltea]

Provide the basic support to enable using YUV420 as an RGB fallback when
computing the best output format and color depth.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
Changes in v2:
- Provided the missing Fixes tag on first patch (Dmitry)
- Added patch "drm/connector: hdmi: Improve debug message for supported
  format"
- Reworked "drm/connector: hdmi: Use YUV420 output format as an RGB
  fallback" so that the fallback to YUV420 will be attempted only when
  RGB cannot be supported for any of the available color depths (Maxime)
- Added a patch to simplify EDID setup in KUnit cases
- Added two patches extending KUnit coverage for YUV420 by providing
  tests for limited range and max TMDS rate fallback
- Rebased series onto drm-misc-next-2025-03-06
- Link to v1: https://lore.kernel.org/r/20241130-hdmi-conn-yuv-v1-0-254279a08671@collabora.com

---
Cristian Ciocaltea (7):
      drm/connector: hdmi: Evaluate limited range after computing format
      drm/connector: hdmi: Add support for YUV420 format verification
      drm/connector: hdmi: Improve debug message for supported format
      drm/connector: hdmi: Use YUV420 output format as an RGB fallback
      drm/tests: hdmi: Add macros to simplify EDID setup
      drm/tests: hdmi: Add limited range tests for YUV420 mode
      drm/tests: hdmi: Add max TMDS rate fallback tests for YUV420 mode

 drivers/gpu/drm/display/drm_hdmi_state_helper.c    | 115 +++--
 drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 479 ++++++++++++++-------
 drivers/gpu/drm/tests/drm_kunit_edid.h             | 222 ++++++++++
 3 files changed, 615 insertions(+), 201 deletions(-)
---
base-commit: 4423e607ff50157aaf088854b145936cbab4d560
change-id: 20241130-hdmi-conn-yuv-e1fa596df768

[PATCH v2 1/7] drm/connector: hdmi: Evaluate limited range after computing format

[Cristian Ciocaltea]

Evaluating the requirement to use a limited RGB quantization range
involves a verification of the output format, among others, but this is
currently performed before actually computing the format, hence relying
on the old connector state.

Move the call to hdmi_is_limited_range() after hdmi_compute_config() to
ensure the verification is done on the updated output format.

Fixes: 027d43590649 ("drm/connector: hdmi: Add RGB Quantization Range to the connector state")
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/display/drm_hdmi_state_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
index c205f37da1e12b11c384670db83e43613e031340..6bc96d5d1ab9115989e208d9899e16cd22254fb6 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -506,12 +506,12 @@ int drm_atomic_helper_connector_hdmi_check(struct drm_connector *connector,
 	if (!new_conn_state->crtc || !new_conn_state->best_encoder)
 		return 0;
 
-	new_conn_state->hdmi.is_limited_range = hdmi_is_limited_range(connector, new_conn_state);
-
 	ret = hdmi_compute_config(connector, new_conn_state, mode);
 	if (ret)
 		return ret;
 
+	new_conn_state->hdmi.is_limited_range = hdmi_is_limited_range(connector, new_conn_state);
+
 	ret = hdmi_generate_infoframes(connector, new_conn_state);
 	if (ret)
 		return ret;

-- 
2.48.1

[PATCH v2 2/7] drm/connector: hdmi: Add support for YUV420 format verification

[Cristian Ciocaltea]

Provide the necessary constraints verification in
sink_supports_format_bpc() in order to support handling of YUV420
output format.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/display/drm_hdmi_state_helper.c | 40 +++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
index 6bc96d5d1ab9115989e208d9899e16cd22254fb6..e99d868edc1854eddc5ebf8692ccffb9e2338268 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -3,6 +3,7 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_connector.h>
 #include <drm/drm_edid.h>
+#include <drm/drm_modes.h>
 #include <drm/drm_print.h>
 
 #include <drm/display/drm_hdmi_audio_helper.h>
@@ -115,6 +116,12 @@ sink_supports_format_bpc(const struct drm_connector *connector,
 		return false;
 	}
 
+	if (drm_mode_is_420_only(info, mode) && format != HDMI_COLORSPACE_YUV420) {
+		drm_dbg_kms(dev, "%s format unsupported by the sink for VIC%u.\n",
+			    drm_hdmi_connector_get_output_format_name(format), vic);
+		return false;
+	}
+
 	switch (format) {
 	case HDMI_COLORSPACE_RGB:
 		drm_dbg_kms(dev, "RGB Format, checking the constraints.\n");
@@ -145,9 +152,36 @@ sink_supports_format_bpc(const struct drm_connector *connector,
 		return true;
 
 	case HDMI_COLORSPACE_YUV420:
-		/* TODO: YUV420 is unsupported at the moment. */
-		drm_dbg_kms(dev, "YUV420 format isn't supported yet.\n");
-		return false;
+		drm_dbg_kms(dev, "YUV420 format, checking the constraints.\n");
+
+		if (!(info->color_formats & DRM_COLOR_FORMAT_YCBCR420)) {
+			drm_dbg_kms(dev, "Sink doesn't support YUV420.\n");
+			return false;
+		}
+
+		if (!drm_mode_is_420(info, mode)) {
+			drm_dbg_kms(dev, "Sink doesn't support YUV420 for VIC%u.\n", vic);
+			return false;
+		}
+
+		if (bpc == 10 && !(info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_30)) {
+			drm_dbg_kms(dev, "10 BPC but sink doesn't support Deep Color 30.\n");
+			return false;
+		}
+
+		if (bpc == 12 && !(info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_36)) {
+			drm_dbg_kms(dev, "12 BPC but sink doesn't support Deep Color 36.\n");
+			return false;
+		}
+
+		if (bpc == 16 && !(info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_48)) {
+			drm_dbg_kms(dev, "16 BPC but sink doesn't support Deep Color 48.\n");
+			return false;
+		}
+
+		drm_dbg_kms(dev, "YUV420 format supported in that configuration.\n");
+
+		return true;
 
 	case HDMI_COLORSPACE_YUV422:
 		drm_dbg_kms(dev, "YUV422 format, checking the constraints.\n");

-- 
2.48.1

[PATCH v2 3/7] drm/connector: hdmi: Improve debug message for supported format

[Cristian Ciocaltea]

Add the missing 'bpc' string to the debug message indicating the
supported format identified within hdmi_try_format_bpc() helper.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/display/drm_hdmi_state_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
index e99d868edc1854eddc5ebf8692ccffb9e2338268..a70e204a8df3ac1c2d7318e81cde87a83267dd21 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -305,7 +305,7 @@ hdmi_try_format_bpc(const struct drm_connector *connector,
 		return false;
 	}
 
-	drm_dbg_kms(dev, "%s output format supported with %u (TMDS char rate: %llu Hz)\n",
+	drm_dbg_kms(dev, "%s output format supported with %u bpc (TMDS char rate: %llu Hz)\n",
 		    drm_hdmi_connector_get_output_format_name(fmt),
 		    bpc, conn_state->hdmi.tmds_char_rate);
 

-- 
2.48.1

[PATCH v2 4/7] drm/connector: hdmi: Use YUV420 output format as an RGB fallback

[Cristian Ciocaltea]

Try to make use of YUV420 when computing the best output format and
RGB cannot be supported for any of the available color depths.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/display/drm_hdmi_state_helper.c | 69 +++++++++++++------------
 1 file changed, 35 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
index a70e204a8df3ac1c2d7318e81cde87a83267dd21..f2052781b797dd09b41127e33d98fe25408a9b23 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -287,8 +287,9 @@ hdmi_try_format_bpc(const struct drm_connector *connector,
 	struct drm_device *dev = connector->dev;
 	int ret;
 
-	drm_dbg_kms(dev, "Trying %s output format\n",
-		    drm_hdmi_connector_get_output_format_name(fmt));
+	drm_dbg_kms(dev, "Trying %s output format with %u bpc\n",
+		    drm_hdmi_connector_get_output_format_name(fmt),
+		    bpc);
 
 	if (!sink_supports_format_bpc(connector, info, mode, fmt, bpc)) {
 		drm_dbg_kms(dev, "%s output format not supported with %u bpc\n",
@@ -313,47 +314,22 @@ hdmi_try_format_bpc(const struct drm_connector *connector,
 }
 
 static int
-hdmi_compute_format(const struct drm_connector *connector,
-		    struct drm_connector_state *conn_state,
-		    const struct drm_display_mode *mode,
-		    unsigned int bpc)
-{
-	struct drm_device *dev = connector->dev;
-
-	/*
-	 * TODO: Add support for YCbCr420 output for HDMI 2.0 capable
-	 * devices, for modes that only support YCbCr420.
-	 */
-	if (hdmi_try_format_bpc(connector, conn_state, mode, bpc, HDMI_COLORSPACE_RGB)) {
-		conn_state->hdmi.output_format = HDMI_COLORSPACE_RGB;
-		return 0;
-	}
-
-	drm_dbg_kms(dev, "Failed. No Format Supported for that bpc count.\n");
-
-	return -EINVAL;
-}
-
-static int
-hdmi_compute_config(const struct drm_connector *connector,
-		    struct drm_connector_state *conn_state,
-		    const struct drm_display_mode *mode)
+hdmi_try_format(const struct drm_connector *connector,
+		struct drm_connector_state *conn_state,
+		const struct drm_display_mode *mode,
+		unsigned int max_bpc, enum hdmi_colorspace fmt)
 {
 	struct drm_device *dev = connector->dev;
-	unsigned int max_bpc = clamp_t(unsigned int,
-				       conn_state->max_bpc,
-				       8, connector->max_bpc);
 	unsigned int bpc;
 	int ret;
 
 	for (bpc = max_bpc; bpc >= 8; bpc -= 2) {
-		drm_dbg_kms(dev, "Trying with a %d bpc output\n", bpc);
-
-		ret = hdmi_compute_format(connector, conn_state, mode, bpc);
-		if (ret)
+		ret = hdmi_try_format_bpc(connector, conn_state, mode, bpc, fmt);
+		if (!ret)
 			continue;
 
 		conn_state->hdmi.output_bpc = bpc;
+		conn_state->hdmi.output_format = fmt;
 
 		drm_dbg_kms(dev,
 			    "Mode %ux%u @ %uHz: Found configuration: bpc: %u, fmt: %s, clock: %llu\n",
@@ -368,6 +344,31 @@ hdmi_compute_config(const struct drm_connector *connector,
 	return -EINVAL;
 }
 
+static int
+hdmi_compute_config(const struct drm_connector *connector,
+		    struct drm_connector_state *conn_state,
+		    const struct drm_display_mode *mode)
+{
+	unsigned int max_bpc = clamp_t(unsigned int,
+				       conn_state->max_bpc,
+				       8, connector->max_bpc);
+	int ret;
+
+	ret = hdmi_try_format(connector, conn_state, mode, max_bpc,
+			      HDMI_COLORSPACE_RGB);
+	if (!ret)
+		return 0;
+
+	if (connector->ycbcr_420_allowed)
+		ret = hdmi_try_format(connector, conn_state, mode, max_bpc,
+				      HDMI_COLORSPACE_YUV420);
+	else
+		drm_dbg_kms(connector->dev,
+			    "%s output format not allowed for connector\n",
+			    drm_hdmi_connector_get_output_format_name(HDMI_COLORSPACE_YUV420));
+	return ret;
+}
+
 static int hdmi_generate_avi_infoframe(const struct drm_connector *connector,
 				       struct drm_connector_state *conn_state)
 {

-- 
2.48.1

[PATCH v2 5/7] drm/tests: hdmi: Add macros to simplify EDID setup

[Cristian Ciocaltea]

Introduce a few macros to facilitate setting custom (i.e. non-default)
EDID data during connector initialization.

This helps reducing boilerplate code while also drops some redundant
calls to set_connector_edid().

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 245 ++++++++-------------
 1 file changed, 93 insertions(+), 152 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
index e97efd3af9ed18e6cf8ee66b4923dfc805b34e19..a3f7f3ce31c73335c2c2643bdc5395b6ceb6f071 100644
--- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
@@ -183,10 +183,12 @@ static const struct drm_connector_funcs dummy_connector_funcs = {
 
 static
 struct drm_atomic_helper_connector_hdmi_priv *
-drm_kunit_helper_connector_hdmi_init_funcs(struct kunit *test,
-					   unsigned int formats,
-					   unsigned int max_bpc,
-					   const struct drm_connector_hdmi_funcs *hdmi_funcs)
+connector_hdmi_init_funcs_set_edid(struct kunit *test,
+				   unsigned int formats,
+				   unsigned int max_bpc,
+				   const struct drm_connector_hdmi_funcs *hdmi_funcs,
+				   const char *edid_data,
+				   size_t edid_len)
 {
 	struct drm_atomic_helper_connector_hdmi_priv *priv;
 	struct drm_connector *conn;
@@ -240,30 +242,27 @@ drm_kunit_helper_connector_hdmi_init_funcs(struct kunit *test,
 
 	drm_mode_config_reset(drm);
 
+	if (edid_data && edid_len) {
+		ret = set_connector_edid(test, &priv->connector, edid_data, edid_len);
+		KUNIT_ASSERT_GT(test, ret, 0);
+	}
+
 	return priv;
 }
 
-static
-struct drm_atomic_helper_connector_hdmi_priv *
-drm_kunit_helper_connector_hdmi_init(struct kunit *test,
-				     unsigned int formats,
-				     unsigned int max_bpc)
-{
-	struct drm_atomic_helper_connector_hdmi_priv *priv;
-	int ret;
+#define drm_kunit_helper_connector_hdmi_init_funcs_set_edid(test, formats, max_bpc, funcs, edid) \
+	connector_hdmi_init_funcs_set_edid(test, formats, max_bpc, funcs, edid, ARRAY_SIZE(edid))
 
-	priv = drm_kunit_helper_connector_hdmi_init_funcs(test,
-							  formats, max_bpc,
-							  &dummy_connector_hdmi_funcs);
-	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
+#define drm_kunit_helper_connector_hdmi_init_funcs(test, formats, max_bpc, funcs)		\
+	connector_hdmi_init_funcs_set_edid(test, formats, max_bpc, funcs, NULL, 0)
 
-	ret = set_connector_edid(test, &priv->connector,
-				 test_edid_hdmi_1080p_rgb_max_200mhz,
-				 ARRAY_SIZE(test_edid_hdmi_1080p_rgb_max_200mhz));
-	KUNIT_ASSERT_GT(test, ret, 0);
+#define drm_kunit_helper_connector_hdmi_init_set_edid(test, formats, max_bpc, edid)		\
+	drm_kunit_helper_connector_hdmi_init_funcs_set_edid(test, formats, max_bpc,		\
+							    &dummy_connector_hdmi_funcs, edid)
 
-	return priv;
-}
+#define drm_kunit_helper_connector_hdmi_init(test, formats, max_bpc)				\
+	drm_kunit_helper_connector_hdmi_init_set_edid(test, formats, max_bpc,			\
+						      test_edid_hdmi_1080p_rgb_max_200mhz)
 
 /*
  * Test that if we change the RGB quantization property to a different
@@ -771,19 +770,15 @@ static void drm_test_check_output_bpc_crtc_mode_changed(struct kunit *test)
 	struct drm_crtc *crtc;
 	int ret;
 
-	priv = drm_kunit_helper_connector_hdmi_init(test,
-						    BIT(HDMI_COLORSPACE_RGB),
-						    10);
+	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
+				BIT(HDMI_COLORSPACE_RGB),
+				10,
+				test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
 
 	drm = &priv->drm;
 	crtc = priv->crtc;
 	conn = &priv->connector;
-	ret = set_connector_edid(test, conn,
-				 test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz,
-				 ARRAY_SIZE(test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz));
-	KUNIT_ASSERT_GT(test, ret, 0);
-
 	preferred = find_preferred_mode(conn);
 	KUNIT_ASSERT_NOT_NULL(test, preferred);
 
@@ -847,19 +842,15 @@ static void drm_test_check_output_bpc_crtc_mode_not_changed(struct kunit *test)
 	struct drm_crtc *crtc;
 	int ret;
 
-	priv = drm_kunit_helper_connector_hdmi_init(test,
-						    BIT(HDMI_COLORSPACE_RGB),
-						    10);
+	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
+				BIT(HDMI_COLORSPACE_RGB),
+				10,
+				test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
 
 	drm = &priv->drm;
 	crtc = priv->crtc;
 	conn = &priv->connector;
-	ret = set_connector_edid(test, conn,
-				 test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz,
-				 ARRAY_SIZE(test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz));
-	KUNIT_ASSERT_GT(test, ret, 0);
-
 	preferred = find_preferred_mode(conn);
 	KUNIT_ASSERT_NOT_NULL(test, preferred);
 
@@ -918,21 +909,17 @@ static void drm_test_check_output_bpc_dvi(struct kunit *test)
 	struct drm_crtc *crtc;
 	int ret;
 
-	priv = drm_kunit_helper_connector_hdmi_init(test,
-						    BIT(HDMI_COLORSPACE_RGB) |
-						    BIT(HDMI_COLORSPACE_YUV422) |
-						    BIT(HDMI_COLORSPACE_YUV444),
-						    12);
+	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
+					BIT(HDMI_COLORSPACE_RGB) |
+					BIT(HDMI_COLORSPACE_YUV422) |
+					BIT(HDMI_COLORSPACE_YUV444),
+					12,
+					test_edid_dvi_1080p);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
 
 	drm = &priv->drm;
 	crtc = priv->crtc;
 	conn = &priv->connector;
-	ret = set_connector_edid(test, conn,
-				 test_edid_dvi_1080p,
-				 ARRAY_SIZE(test_edid_dvi_1080p));
-	KUNIT_ASSERT_GT(test, ret, 0);
-
 	info = &conn->display_info;
 	KUNIT_ASSERT_FALSE(test, info->is_hdmi);
 
@@ -969,19 +956,15 @@ static void drm_test_check_tmds_char_rate_rgb_8bpc(struct kunit *test)
 	struct drm_crtc *crtc;
 	int ret;
 
-	priv = drm_kunit_helper_connector_hdmi_init(test,
-						    BIT(HDMI_COLORSPACE_RGB),
-						    8);
+	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
+				BIT(HDMI_COLORSPACE_RGB),
+				8,
+				test_edid_hdmi_1080p_rgb_max_200mhz);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
 
 	drm = &priv->drm;
 	crtc = priv->crtc;
 	conn = &priv->connector;
-	ret = set_connector_edid(test, conn,
-				 test_edid_hdmi_1080p_rgb_max_200mhz,
-				 ARRAY_SIZE(test_edid_hdmi_1080p_rgb_max_200mhz));
-	KUNIT_ASSERT_GT(test, ret, 0);
-
 	preferred = find_preferred_mode(conn);
 	KUNIT_ASSERT_NOT_NULL(test, preferred);
 	KUNIT_ASSERT_FALSE(test, preferred->flags & DRM_MODE_FLAG_DBLCLK);
@@ -1018,19 +1001,15 @@ static void drm_test_check_tmds_char_rate_rgb_10bpc(struct kunit *test)
 	struct drm_crtc *crtc;
 	int ret;
 
-	priv = drm_kunit_helper_connector_hdmi_init(test,
-						    BIT(HDMI_COLORSPACE_RGB),
-						    10);
+	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
+				BIT(HDMI_COLORSPACE_RGB),
+				10,
+				test_edid_hdmi_1080p_rgb_yuv_dc_max_340mhz);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
 
 	drm = &priv->drm;
 	crtc = priv->crtc;
 	conn = &priv->connector;
-	ret = set_connector_edid(test, conn,
-				 test_edid_hdmi_1080p_rgb_yuv_dc_max_340mhz,
-				 ARRAY_SIZE(test_edid_hdmi_1080p_rgb_yuv_dc_max_340mhz));
-	KUNIT_ASSERT_GT(test, ret, 0);
-
 	preferred = find_preferred_mode(conn);
 	KUNIT_ASSERT_NOT_NULL(test, preferred);
 	KUNIT_ASSERT_FALSE(test, preferred->flags & DRM_MODE_FLAG_DBLCLK);
@@ -1067,19 +1046,15 @@ static void drm_test_check_tmds_char_rate_rgb_12bpc(struct kunit *test)
 	struct drm_crtc *crtc;
 	int ret;
 
-	priv = drm_kunit_helper_connector_hdmi_init(test,
-						    BIT(HDMI_COLORSPACE_RGB),
-						    12);
+	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
+				BIT(HDMI_COLORSPACE_RGB),
+				12,
+				test_edid_hdmi_1080p_rgb_yuv_dc_max_340mhz);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
 
 	drm = &priv->drm;
 	crtc = priv->crtc;
 	conn = &priv->connector;
-	ret = set_connector_edid(test, conn,
-				 test_edid_hdmi_1080p_rgb_yuv_dc_max_340mhz,
-				 ARRAY_SIZE(test_edid_hdmi_1080p_rgb_yuv_dc_max_340mhz));
-	KUNIT_ASSERT_GT(test, ret, 0);
-
 	preferred = find_preferred_mode(conn);
 	KUNIT_ASSERT_NOT_NULL(test, preferred);
 	KUNIT_ASSERT_FALSE(test, preferred->flags & DRM_MODE_FLAG_DBLCLK);
@@ -1179,19 +1154,15 @@ static void drm_test_check_max_tmds_rate_bpc_fallback(struct kunit *test)
 	struct drm_crtc *crtc;
 	int ret;
 
-	priv = drm_kunit_helper_connector_hdmi_init(test,
-						    BIT(HDMI_COLORSPACE_RGB),
-						    12);
+	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
+				BIT(HDMI_COLORSPACE_RGB),
+				12,
+				test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
 
 	drm = &priv->drm;
 	crtc = priv->crtc;
 	conn = &priv->connector;
-	ret = set_connector_edid(test, conn,
-				 test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz,
-				 ARRAY_SIZE(test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz));
-	KUNIT_ASSERT_GT(test, ret, 0);
-
 	info = &conn->display_info;
 	KUNIT_ASSERT_TRUE(test, info->is_hdmi);
 	KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
@@ -1248,21 +1219,17 @@ static void drm_test_check_max_tmds_rate_format_fallback(struct kunit *test)
 	struct drm_crtc *crtc;
 	int ret;
 
-	priv = drm_kunit_helper_connector_hdmi_init(test,
-						    BIT(HDMI_COLORSPACE_RGB) |
-						    BIT(HDMI_COLORSPACE_YUV422) |
-						    BIT(HDMI_COLORSPACE_YUV444),
-						    12);
+	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
+				BIT(HDMI_COLORSPACE_RGB) |
+				BIT(HDMI_COLORSPACE_YUV422) |
+				BIT(HDMI_COLORSPACE_YUV444),
+				12,
+				test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
 
 	drm = &priv->drm;
 	crtc = priv->crtc;
 	conn = &priv->connector;
-	ret = set_connector_edid(test, conn,
-				 test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz,
-				 ARRAY_SIZE(test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz));
-	KUNIT_ASSERT_GT(test, ret, 0);
-
 	info = &conn->display_info;
 	KUNIT_ASSERT_TRUE(test, info->is_hdmi);
 	KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
@@ -1313,20 +1280,16 @@ static void drm_test_check_output_bpc_format_vic_1(struct kunit *test)
 	struct drm_crtc *crtc;
 	int ret;
 
-	priv = drm_kunit_helper_connector_hdmi_init(test,
-						    BIT(HDMI_COLORSPACE_RGB) |
-						    BIT(HDMI_COLORSPACE_YUV422) |
-						    BIT(HDMI_COLORSPACE_YUV444),
-						    12);
+	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
+				BIT(HDMI_COLORSPACE_RGB) |
+				BIT(HDMI_COLORSPACE_YUV422) |
+				BIT(HDMI_COLORSPACE_YUV444),
+				12,
+				test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
 
 	drm = &priv->drm;
 	conn = &priv->connector;
-	ret = set_connector_edid(test, conn,
-				 test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz,
-				 ARRAY_SIZE(test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz));
-	KUNIT_ASSERT_GT(test, ret, 0);
-
 	info = &conn->display_info;
 	KUNIT_ASSERT_TRUE(test, info->is_hdmi);
 	KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
@@ -1377,19 +1340,15 @@ static void drm_test_check_output_bpc_format_driver_rgb_only(struct kunit *test)
 	struct drm_crtc *crtc;
 	int ret;
 
-	priv = drm_kunit_helper_connector_hdmi_init(test,
-						    BIT(HDMI_COLORSPACE_RGB),
-						    12);
+	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
+				BIT(HDMI_COLORSPACE_RGB),
+				12,
+				test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
 
 	drm = &priv->drm;
 	crtc = priv->crtc;
 	conn = &priv->connector;
-	ret = set_connector_edid(test, conn,
-				 test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz,
-				 ARRAY_SIZE(test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz));
-	KUNIT_ASSERT_GT(test, ret, 0);
-
 	info = &conn->display_info;
 	KUNIT_ASSERT_TRUE(test, info->is_hdmi);
 	KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
@@ -1444,21 +1403,17 @@ static void drm_test_check_output_bpc_format_display_rgb_only(struct kunit *test
 	struct drm_crtc *crtc;
 	int ret;
 
-	priv = drm_kunit_helper_connector_hdmi_init(test,
-						    BIT(HDMI_COLORSPACE_RGB) |
-						    BIT(HDMI_COLORSPACE_YUV422) |
-						    BIT(HDMI_COLORSPACE_YUV444),
-						    12);
+	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
+				BIT(HDMI_COLORSPACE_RGB) |
+				BIT(HDMI_COLORSPACE_YUV422) |
+				BIT(HDMI_COLORSPACE_YUV444),
+				12,
+				test_edid_hdmi_1080p_rgb_max_200mhz);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
 
 	drm = &priv->drm;
 	crtc = priv->crtc;
 	conn = &priv->connector;
-	ret = set_connector_edid(test, conn,
-				 test_edid_hdmi_1080p_rgb_max_200mhz,
-				 ARRAY_SIZE(test_edid_hdmi_1080p_rgb_max_200mhz));
-	KUNIT_ASSERT_GT(test, ret, 0);
-
 	info = &conn->display_info;
 	KUNIT_ASSERT_TRUE(test, info->is_hdmi);
 	KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
@@ -1514,19 +1469,15 @@ static void drm_test_check_output_bpc_format_driver_8bpc_only(struct kunit *test
 	struct drm_crtc *crtc;
 	int ret;
 
-	priv = drm_kunit_helper_connector_hdmi_init(test,
-						    BIT(HDMI_COLORSPACE_RGB),
-						    8);
+	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
+				BIT(HDMI_COLORSPACE_RGB),
+				8,
+				test_edid_hdmi_1080p_rgb_yuv_dc_max_340mhz);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
 
 	drm = &priv->drm;
 	crtc = priv->crtc;
 	conn = &priv->connector;
-	ret = set_connector_edid(test, conn,
-				 test_edid_hdmi_1080p_rgb_yuv_dc_max_340mhz,
-				 ARRAY_SIZE(test_edid_hdmi_1080p_rgb_yuv_dc_max_340mhz));
-	KUNIT_ASSERT_GT(test, ret, 0);
-
 	info = &conn->display_info;
 	KUNIT_ASSERT_TRUE(test, info->is_hdmi);
 	KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
@@ -1574,21 +1525,17 @@ static void drm_test_check_output_bpc_format_display_8bpc_only(struct kunit *tes
 	struct drm_crtc *crtc;
 	int ret;
 
-	priv = drm_kunit_helper_connector_hdmi_init(test,
-						    BIT(HDMI_COLORSPACE_RGB) |
-						    BIT(HDMI_COLORSPACE_YUV422) |
-						    BIT(HDMI_COLORSPACE_YUV444),
-						    12);
+	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
+				BIT(HDMI_COLORSPACE_RGB) |
+				BIT(HDMI_COLORSPACE_YUV422) |
+				BIT(HDMI_COLORSPACE_YUV444),
+				12,
+				test_edid_hdmi_1080p_rgb_max_340mhz);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
 
 	drm = &priv->drm;
 	crtc = priv->crtc;
 	conn = &priv->connector;
-	ret = set_connector_edid(test, conn,
-				 test_edid_hdmi_1080p_rgb_max_340mhz,
-				 ARRAY_SIZE(test_edid_hdmi_1080p_rgb_max_340mhz));
-	KUNIT_ASSERT_GT(test, ret, 0);
-
 	info = &conn->display_info;
 	KUNIT_ASSERT_TRUE(test, info->is_hdmi);
 	KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
@@ -1906,9 +1853,9 @@ static void drm_test_check_mode_valid_reject_rate(struct kunit *test)
 	int ret;
 
 	priv = drm_kunit_helper_connector_hdmi_init_funcs(test,
-							  BIT(HDMI_COLORSPACE_RGB),
-							  8,
-							  &reject_100_MHz_connector_hdmi_funcs);
+					BIT(HDMI_COLORSPACE_RGB),
+					8,
+					&reject_100_MHz_connector_hdmi_funcs);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
 
 	conn = &priv->connector;
@@ -1943,9 +1890,9 @@ static void drm_test_check_mode_valid_reject(struct kunit *test)
 	int ret;
 
 	priv = drm_kunit_helper_connector_hdmi_init_funcs(test,
-							  BIT(HDMI_COLORSPACE_RGB),
-							  8,
-							  &reject_connector_hdmi_funcs);
+					BIT(HDMI_COLORSPACE_RGB),
+					8,
+					&reject_connector_hdmi_funcs);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
 
 	conn = &priv->connector;
@@ -1970,20 +1917,14 @@ static void drm_test_check_mode_valid_reject_max_clock(struct kunit *test)
 	struct drm_atomic_helper_connector_hdmi_priv *priv;
 	struct drm_connector *conn;
 	struct drm_display_mode *preferred;
-	int ret;
 
-	priv = drm_kunit_helper_connector_hdmi_init(test,
-						    BIT(HDMI_COLORSPACE_RGB),
-						    8);
+	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
+				BIT(HDMI_COLORSPACE_RGB),
+				8,
+				test_edid_hdmi_1080p_rgb_max_100mhz);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
 
 	conn = &priv->connector;
-
-	ret = set_connector_edid(test, conn,
-				 test_edid_hdmi_1080p_rgb_max_100mhz,
-				 ARRAY_SIZE(test_edid_hdmi_1080p_rgb_max_100mhz));
-	KUNIT_ASSERT_GT(test, ret, 0);
-
 	KUNIT_ASSERT_EQ(test, conn->display_info.max_tmds_clock, 100 * 1000);
 
 	preferred = find_preferred_mode(conn);

-- 
2.48.1

[PATCH v2 6/7] drm/tests: hdmi: Add limited range tests for YUV420 mode

[Cristian Ciocaltea]

Provide tests to verify that drm_atomic_helper_connector_hdmi_check()
helper behaviour when using YUV420 output format is to always set the
limited RGB quantization range to 'limited', no matter what the value of
Broadcast RGB property is.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c |  89 +++++++++++++++-
 drivers/gpu/drm/tests/drm_kunit_edid.h             | 112 +++++++++++++++++++++
 2 files changed, 196 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
index a3f7f3ce31c73335c2c2643bdc5395b6ceb6f071..1df12c0b7768e4f85f4c943840d9b4dcb6e079e0 100644
--- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
@@ -227,6 +227,8 @@ connector_hdmi_init_funcs_set_edid(struct kunit *test,
 	enc->possible_crtcs = drm_crtc_mask(priv->crtc);
 
 	conn = &priv->connector;
+	conn->ycbcr_420_allowed = !!(formats & BIT(HDMI_COLORSPACE_YUV420));
+
 	ret = drmm_connector_hdmi_init(drm, conn,
 				       "Vendor", "Product",
 				       &dummy_connector_funcs,
@@ -751,6 +753,86 @@ static void drm_test_check_broadcast_rgb_limited_cea_mode_vic_1(struct kunit *te
 	drm_modeset_acquire_fini(&ctx);
 }
 
+/*
+ * Test that for an HDMI connector, with an HDMI monitor, we will
+ * get a limited RGB Quantization Range with a YUV420 mode, no
+ * matter what the value of the Broadcast RGB property is set to.
+ */
+static void drm_test_check_broadcast_rgb_cea_mode_yuv420(struct kunit *test)
+{
+	struct drm_atomic_helper_connector_hdmi_priv *priv;
+	enum drm_hdmi_broadcast_rgb broadcast_rgb;
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_connector_state *conn_state;
+	struct drm_atomic_state *state;
+	struct drm_display_mode *mode;
+	struct drm_connector *conn;
+	struct drm_device *drm;
+	struct drm_crtc *crtc;
+	int ret;
+
+	broadcast_rgb = *(enum drm_hdmi_broadcast_rgb *)test->param_value;
+
+	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
+				BIT(HDMI_COLORSPACE_RGB) |
+				BIT(HDMI_COLORSPACE_YUV420),
+				8,
+				test_edid_hdmi_1080p_rgb_yuv_4k_yuv420_dc_max_200mhz);
+	KUNIT_ASSERT_NOT_NULL(test, priv);
+
+	drm = &priv->drm;
+	crtc = priv->crtc;
+	conn = &priv->connector;
+	KUNIT_ASSERT_TRUE(test, conn->display_info.is_hdmi);
+
+	mode = drm_kunit_display_mode_from_cea_vic(test, drm, 95);
+	KUNIT_ASSERT_NOT_NULL(test, mode);
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+	ret = light_up_connector(test, drm, crtc, conn, mode, &ctx);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	state = drm_kunit_helper_atomic_state_alloc(test, drm, &ctx);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+	conn_state = drm_atomic_get_connector_state(state, conn);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+	conn_state->hdmi.broadcast_rgb = broadcast_rgb;
+
+	ret = drm_atomic_check_only(state);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	conn_state = drm_atomic_get_connector_state(state, conn);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+	KUNIT_ASSERT_EQ(test, conn_state->hdmi.broadcast_rgb, broadcast_rgb);
+	KUNIT_ASSERT_EQ(test, conn_state->hdmi.output_format, HDMI_COLORSPACE_YUV420);
+
+	KUNIT_EXPECT_TRUE(test, conn_state->hdmi.is_limited_range);
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+}
+
+static const enum drm_hdmi_broadcast_rgb check_broadcast_rgb_cea_mode_yuv420_tests[] = {
+	DRM_HDMI_BROADCAST_RGB_AUTO,
+	DRM_HDMI_BROADCAST_RGB_FULL,
+	DRM_HDMI_BROADCAST_RGB_LIMITED,
+};
+
+static void
+check_broadcast_rgb_cea_mode_yuv420_desc(const enum drm_hdmi_broadcast_rgb *broadcast_rgb,
+					 char *desc)
+{
+	sprintf(desc, "%s", drm_hdmi_connector_get_broadcast_rgb_name(*broadcast_rgb));
+}
+
+KUNIT_ARRAY_PARAM(check_broadcast_rgb_cea_mode_yuv420,
+		  check_broadcast_rgb_cea_mode_yuv420_tests,
+		  check_broadcast_rgb_cea_mode_yuv420_desc);
+
 /*
  * Test that if we change the maximum bpc property to a different value,
  * we trigger a mode change on the connector's CRTC, which will in turn
@@ -1625,11 +1707,8 @@ static struct kunit_case drm_atomic_helper_connector_hdmi_check_tests[] = {
 	KUNIT_CASE(drm_test_check_broadcast_rgb_full_cea_mode_vic_1),
 	KUNIT_CASE(drm_test_check_broadcast_rgb_limited_cea_mode),
 	KUNIT_CASE(drm_test_check_broadcast_rgb_limited_cea_mode_vic_1),
-	/*
-	 * TODO: When we'll have YUV output support, we need to check
-	 * that the limited range is always set to limited no matter
-	 * what the value of Broadcast RGB is.
-	 */
+	KUNIT_CASE_PARAM(drm_test_check_broadcast_rgb_cea_mode_yuv420,
+			 check_broadcast_rgb_cea_mode_yuv420_gen_params),
 	KUNIT_CASE(drm_test_check_broadcast_rgb_crtc_mode_changed),
 	KUNIT_CASE(drm_test_check_broadcast_rgb_crtc_mode_not_changed),
 	KUNIT_CASE(drm_test_check_disable_connector),
diff --git a/drivers/gpu/drm/tests/drm_kunit_edid.h b/drivers/gpu/drm/tests/drm_kunit_edid.h
index 6358397a5d7ab0bcdea1c824fc9fd382560f4b0e..ff316e6114d65c96b1338cd83bc0d8d9e6e143e9 100644
--- a/drivers/gpu/drm/tests/drm_kunit_edid.h
+++ b/drivers/gpu/drm/tests/drm_kunit_edid.h
@@ -583,4 +583,116 @@ static const unsigned char test_edid_hdmi_1080p_rgb_yuv_dc_max_340mhz[] = {
 	0x00, 0x00, 0x00, 0x8c
 };
 
+/*
+ * edid-decode (hex):
+ *
+ * 00 ff ff ff ff ff ff 00 31 d8 34 00 00 00 00 00
+ * ff 23 01 03 80 60 36 78 0f ee 91 a3 54 4c 99 26
+ * 0f 50 54 20 00 00 01 01 01 01 01 01 01 01 01 01
+ * 01 01 01 01 01 01 02 3a 80 18 71 38 2d 40 58 2c
+ * 45 00 c0 1c 32 00 00 1e 00 00 00 fc 00 54 65 73
+ * 74 20 45 44 49 44 0a 20 20 20 00 00 00 fd 00 18
+ * 55 18 5e 11 00 0a 20 20 20 20 20 20 00 00 00 10
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 bb
+ *
+ * 02 03 29 31 42 90 5f 6c 03 0c 00 10 00 78 28 20
+ * 00 00 01 03 6d d8 5d c4 01 28 80 07 00 00 00 00
+ * 00 00 e3 0f 00 00 e2 0e 5f 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ca
+ *
+ * ----------------
+ *
+ * Block 0, Base EDID:
+ *   EDID Structure Version & Revision: 1.3
+ *   Vendor & Product Identification:
+ *     Manufacturer: LNX
+ *     Model: 52
+ *     Model year: 2025
+ *   Basic Display Parameters & Features:
+ *     Digital display
+ *     Maximum image size: 96 cm x 54 cm
+ *     Gamma: 2.20
+ *     RGB color display
+ *     Default (sRGB) color space is primary color space
+ *     First detailed timing is the preferred timing
+ *     Supports GTF timings within operating range
+ *   Color Characteristics:
+ *     Red  : 0.6396, 0.3300
+ *     Green: 0.2998, 0.5996
+ *     Blue : 0.1503, 0.0595
+ *     White: 0.3125, 0.3291
+ *   Established Timings I & II:
+ *     DMT 0x04:   640x480    59.940476 Hz   4:3     31.469 kHz     25.175000 MHz
+ *   Standard Timings: none
+ *   Detailed Timing Descriptors:
+ *     DTD 1:  1920x1080   60.000000 Hz  16:9     67.500 kHz    148.500000 MHz (960 mm x 540 mm)
+ *                  Hfront   88 Hsync  44 Hback  148 Hpol P
+ *                  Vfront    4 Vsync   5 Vback   36 Vpol P
+ *     Display Product Name: 'Test EDID'
+ *     Display Range Limits:
+ *       Monitor ranges (GTF): 24-85 Hz V, 24-94 kHz H, max dotclock 170 MHz
+ *     Dummy Descriptor:
+ *   Extension blocks: 1
+ * Checksum: 0xbb
+ *
+ * ----------------
+ *
+ * Block 1, CTA-861 Extension Block:
+ *   Revision: 3
+ *   Supports YCbCr 4:4:4
+ *   Supports YCbCr 4:2:2
+ *   Native detailed modes: 1
+ *   Video Data Block:
+ *     VIC  16:  1920x1080   60.000000 Hz  16:9     67.500 kHz    148.500000 MHz (native)
+ *     VIC  95:  3840x2160   30.000000 Hz  16:9     67.500 kHz    297.000000 MHz
+ *   Vendor-Specific Data Block (HDMI), OUI 00-0C-03:
+ *     Source physical address: 1.0.0.0
+ *     DC_48bit
+ *     DC_36bit
+ *     DC_30bit
+ *     DC_Y444
+ *     Maximum TMDS clock: 200 MHz
+ *     Extended HDMI video details:
+ *   Vendor-Specific Data Block (HDMI Forum), OUI C4-5D-D8:
+ *     Version: 1
+ *     Maximum TMDS Character Rate: 200 MHz
+ *     SCDC Present
+ *     Supports 16-bits/component Deep Color 4:2:0 Pixel Encoding
+ *     Supports 12-bits/component Deep Color 4:2:0 Pixel Encoding
+ *     Supports 10-bits/component Deep Color 4:2:0 Pixel Encoding
+ *   YCbCr 4:2:0 Capability Map Data Block:
+ *     Empty Capability Map
+ *   YCbCr 4:2:0 Video Data Block:
+ *     VIC  95:  3840x2160   30.000000 Hz  16:9     67.500 kHz    297.000000 MHz
+ * Checksum: 0xca
+ */
+static const unsigned char test_edid_hdmi_1080p_rgb_yuv_4k_yuv420_dc_max_200mhz[] = {
+	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x31, 0xd8, 0x34, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0xff, 0x23, 0x01, 0x03, 0x80, 0x60, 0x36, 0x78,
+	0x0f, 0xee, 0x91, 0xa3, 0x54, 0x4c, 0x99, 0x26, 0x0f, 0x50, 0x54, 0x20,
+	0x00, 0x00, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
+	0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x02, 0x3a, 0x80, 0x18, 0x71, 0x38,
+	0x2d, 0x40, 0x58, 0x2c, 0x45, 0x00, 0xc0, 0x1c, 0x32, 0x00, 0x00, 0x1e,
+	0x00, 0x00, 0x00, 0xfc, 0x00, 0x54, 0x65, 0x73, 0x74, 0x20, 0x45, 0x44,
+	0x49, 0x44, 0x0a, 0x20, 0x20, 0x20, 0x00, 0x00, 0x00, 0xfd, 0x00, 0x18,
+	0x55, 0x18, 0x5e, 0x11, 0x00, 0x0a, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+	0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0xbb, 0x02, 0x03, 0x29, 0x31,
+	0x42, 0x90, 0x5f, 0x6c, 0x03, 0x0c, 0x00, 0x10, 0x00, 0x78, 0x28, 0x20,
+	0x00, 0x00, 0x01, 0x03, 0x6d, 0xd8, 0x5d, 0xc4, 0x01, 0x28, 0x80, 0x07,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xe3, 0x0f, 0x00, 0x00, 0xe2, 0x0e,
+	0x5f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0xca
+};
+
 #endif // DRM_KUNIT_EDID_H_

-- 
2.48.1

[PATCH v2 7/7] drm/tests: hdmi: Add max TMDS rate fallback tests for YUV420 mode

[Cristian Ciocaltea]

Provide tests to verify drm_atomic_helper_connector_hdmi_check() helper
fallback behavior when using YUV420 output.

Also rename drm_test_check_max_tmds_rate_{bpc|format}_fallback() to
better differentiate from the newly introduced *_yuv420() variants.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 145 ++++++++++++++++++++-
 drivers/gpu/drm/tests/drm_kunit_edid.h             | 110 ++++++++++++++++
 2 files changed, 251 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
index 1df12c0b7768e4f85f4c943840d9b4dcb6e079e0..77da5b88c4cbd5bba22f4f0ce1ef2928042d7d50 100644
--- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
@@ -1223,7 +1223,7 @@ static void drm_test_check_hdmi_funcs_reject_rate(struct kunit *test)
  * Then we will pick the latter, and the computed TMDS character rate
  * will be equal to 1.25 times the mode pixel clock.
  */
-static void drm_test_check_max_tmds_rate_bpc_fallback(struct kunit *test)
+static void drm_test_check_max_tmds_rate_bpc_fallback_rgb(struct kunit *test)
 {
 	struct drm_atomic_helper_connector_hdmi_priv *priv;
 	struct drm_modeset_acquire_ctx ctx;
@@ -1275,6 +1275,72 @@ static void drm_test_check_max_tmds_rate_bpc_fallback(struct kunit *test)
 	drm_modeset_acquire_fini(&ctx);
 }
 
+/*
+ * Test that if:
+ * - We have an HDMI connector supporting both RGB and YUV420
+ * - The chosen mode can be supported in YUV420 output format only
+ * - The chosen mode has a TMDS character rate higher than the display
+ *   supports in YUV420/12bpc
+ * - The chosen mode has a TMDS character rate lower than the display
+ *   supports in YUV420/10bpc.
+ *
+ * Then we will pick the latter, and the computed TMDS character rate
+ * will be equal to 1.25 * 0.5 times the mode pixel clock.
+ */
+static void drm_test_check_max_tmds_rate_bpc_fallback_yuv420(struct kunit *test)
+{
+	struct drm_atomic_helper_connector_hdmi_priv *priv;
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_connector_state *conn_state;
+	struct drm_display_info *info;
+	struct drm_display_mode *yuv420_only_mode;
+	unsigned long long rate;
+	struct drm_connector *conn;
+	struct drm_device *drm;
+	struct drm_crtc *crtc;
+	int ret;
+
+	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
+				BIT(HDMI_COLORSPACE_RGB) |
+				BIT(HDMI_COLORSPACE_YUV420),
+				12,
+				test_edid_hdmi_1080p_rgb_yuv_4k_yuv420_dc_max_200mhz);
+	KUNIT_ASSERT_NOT_NULL(test, priv);
+
+	drm = &priv->drm;
+	crtc = priv->crtc;
+	conn = &priv->connector;
+	info = &conn->display_info;
+	KUNIT_ASSERT_TRUE(test, info->is_hdmi);
+	KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
+
+	yuv420_only_mode = drm_kunit_display_mode_from_cea_vic(test, drm, 95);
+	KUNIT_ASSERT_NOT_NULL(test, yuv420_only_mode);
+	KUNIT_ASSERT_TRUE(test, drm_mode_is_420_only(info, yuv420_only_mode));
+
+	rate = drm_hdmi_compute_mode_clock(yuv420_only_mode, 12, HDMI_COLORSPACE_YUV420);
+	KUNIT_ASSERT_GT(test, rate, info->max_tmds_clock * 1000);
+
+	rate = drm_hdmi_compute_mode_clock(yuv420_only_mode, 10, HDMI_COLORSPACE_YUV420);
+	KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+	ret = light_up_connector(test, drm, crtc, conn, yuv420_only_mode, &ctx);
+	KUNIT_EXPECT_EQ(test, ret, 0);
+
+	conn_state = conn->state;
+	KUNIT_ASSERT_NOT_NULL(test, conn_state);
+
+	KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_bpc, 10);
+	KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, HDMI_COLORSPACE_YUV420);
+	KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
+			yuv420_only_mode->clock * 1250 * 0.5);
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+}
+
 /*
  * Test that if:
  * - We have an HDMI connector supporting both RGB and YUV422 and up to
@@ -1288,7 +1354,7 @@ static void drm_test_check_max_tmds_rate_bpc_fallback(struct kunit *test)
  * Then we will prefer to keep the RGB format with a lower bpc over
  * picking YUV422.
  */
-static void drm_test_check_max_tmds_rate_format_fallback(struct kunit *test)
+static void drm_test_check_max_tmds_rate_format_fallback_yuv422(struct kunit *test)
 {
 	struct drm_atomic_helper_connector_hdmi_priv *priv;
 	struct drm_modeset_acquire_ctx ctx;
@@ -1344,6 +1410,75 @@ static void drm_test_check_max_tmds_rate_format_fallback(struct kunit *test)
 	drm_modeset_acquire_fini(&ctx);
 }
 
+/*
+ * Test that if:
+ * - We have an HDMI connector supporting both RGB and YUV420 and up to
+ *   12 bpc
+ * - The chosen mode has a TMDS character rate higher than the display
+ *   supports in RGB/10bpc but lower than the display supports in
+ *   RGB/8bpc
+ * - The chosen mode has a TMDS character rate lower than the display
+ *   supports in YUV420/12bpc.
+ *
+ * Then we will prefer to keep the RGB format with a lower bpc over
+ * picking YUV420.
+ */
+static void drm_test_check_max_tmds_rate_format_fallback_yuv420(struct kunit *test)
+{
+	struct drm_atomic_helper_connector_hdmi_priv *priv;
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_connector_state *conn_state;
+	struct drm_display_info *info;
+	struct drm_display_mode *preferred;
+	unsigned long long rate;
+	struct drm_connector *conn;
+	struct drm_device *drm;
+	struct drm_crtc *crtc;
+	int ret;
+
+	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
+				BIT(HDMI_COLORSPACE_RGB) |
+				BIT(HDMI_COLORSPACE_YUV420),
+				12,
+				test_edid_hdmi_4k_rgb_yuv420_dc_max_340mhz);
+	KUNIT_ASSERT_NOT_NULL(test, priv);
+
+	drm = &priv->drm;
+	crtc = priv->crtc;
+	conn = &priv->connector;
+	info = &conn->display_info;
+	KUNIT_ASSERT_TRUE(test, info->is_hdmi);
+	KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
+
+	preferred = find_preferred_mode(conn);
+	KUNIT_ASSERT_NOT_NULL(test, preferred);
+	KUNIT_ASSERT_FALSE(test, preferred->flags & DRM_MODE_FLAG_DBLCLK);
+	KUNIT_ASSERT_TRUE(test, drm_mode_is_420_also(info, preferred));
+
+	rate = drm_hdmi_compute_mode_clock(preferred, 8, HDMI_COLORSPACE_RGB);
+	KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
+
+	rate = drm_hdmi_compute_mode_clock(preferred, 10, HDMI_COLORSPACE_RGB);
+	KUNIT_ASSERT_GT(test, rate, info->max_tmds_clock * 1000);
+
+	rate = drm_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_YUV420);
+	KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+	ret = light_up_connector(test, drm, crtc, conn, preferred, &ctx);
+	KUNIT_EXPECT_EQ(test, ret, 0);
+
+	conn_state = conn->state;
+	KUNIT_ASSERT_NOT_NULL(test, conn_state);
+
+	KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_bpc, 8);
+	KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, HDMI_COLORSPACE_RGB);
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+}
+
 /*
  * Test that if a driver and screen supports RGB and YUV formats, and we
  * try to set the VIC 1 mode, we end up with 8bpc RGB even if we could
@@ -1713,8 +1848,10 @@ static struct kunit_case drm_atomic_helper_connector_hdmi_check_tests[] = {
 	KUNIT_CASE(drm_test_check_broadcast_rgb_crtc_mode_not_changed),
 	KUNIT_CASE(drm_test_check_disable_connector),
 	KUNIT_CASE(drm_test_check_hdmi_funcs_reject_rate),
-	KUNIT_CASE(drm_test_check_max_tmds_rate_bpc_fallback),
-	KUNIT_CASE(drm_test_check_max_tmds_rate_format_fallback),
+	KUNIT_CASE(drm_test_check_max_tmds_rate_bpc_fallback_rgb),
+	KUNIT_CASE(drm_test_check_max_tmds_rate_bpc_fallback_yuv420),
+	KUNIT_CASE(drm_test_check_max_tmds_rate_format_fallback_yuv422),
+	KUNIT_CASE(drm_test_check_max_tmds_rate_format_fallback_yuv420),
 	KUNIT_CASE(drm_test_check_output_bpc_crtc_mode_changed),
 	KUNIT_CASE(drm_test_check_output_bpc_crtc_mode_not_changed),
 	KUNIT_CASE(drm_test_check_output_bpc_dvi),
diff --git a/drivers/gpu/drm/tests/drm_kunit_edid.h b/drivers/gpu/drm/tests/drm_kunit_edid.h
index ff316e6114d65c96b1338cd83bc0d8d9e6e143e9..8e9086df20c690f34623d7858c716032d77d0c26 100644
--- a/drivers/gpu/drm/tests/drm_kunit_edid.h
+++ b/drivers/gpu/drm/tests/drm_kunit_edid.h
@@ -695,4 +695,114 @@ static const unsigned char test_edid_hdmi_1080p_rgb_yuv_4k_yuv420_dc_max_200mhz[
 	0x00, 0x00, 0x00, 0xca
 };
 
+/*
+ * edid-decode (hex):
+ *
+ * 00 ff ff ff ff ff ff 00 31 d8 34 00 00 00 00 00
+ * ff 23 01 03 80 60 36 78 0f ee 91 a3 54 4c 99 26
+ * 0f 50 54 20 00 00 01 01 01 01 01 01 01 01 01 01
+ * 01 01 01 01 01 01 04 74 00 30 f2 70 5a 80 b0 58
+ * 8a 00 40 84 63 00 00 1e 00 00 00 fc 00 54 65 73
+ * 74 20 45 44 49 44 0a 20 20 20 00 00 00 fd 00 18
+ * 55 18 5e 22 00 0a 20 20 20 20 20 20 00 00 00 10
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 ce
+ *
+ * 02 03 27 31 41 5f 6c 03 0c 00 10 00 78 44 20 00
+ * 00 01 03 6d d8 5d c4 01 44 80 07 00 00 00 00 00
+ * 00 e3 0f 01 00 e1 0e 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 84
+ *
+ * ----------------
+ *
+ * Block 0, Base EDID:
+ *   EDID Structure Version & Revision: 1.3
+ *   Vendor & Product Identification:
+ *     Manufacturer: LNX
+ *     Model: 52
+ *     Model year: 2025
+ *   Basic Display Parameters & Features:
+ *     Digital display
+ *     Maximum image size: 96 cm x 54 cm
+ *     Gamma: 2.20
+ *     RGB color display
+ *     Default (sRGB) color space is primary color space
+ *     First detailed timing is the preferred timing
+ *     Supports GTF timings within operating range
+ *   Color Characteristics:
+ *     Red  : 0.6396, 0.3300
+ *     Green: 0.2998, 0.5996
+ *     Blue : 0.1503, 0.0595
+ *     White: 0.3125, 0.3291
+ *   Established Timings I & II:
+ *     DMT 0x04:   640x480    59.940476 Hz   4:3     31.469 kHz     25.175000 MHz
+ *   Standard Timings: none
+ *   Detailed Timing Descriptors:
+ *     DTD 1:  3840x2160   30.000000 Hz  16:9     67.500 kHz    297.000000 MHz (1600 mm x 900 mm)
+ *                  Hfront  176 Hsync  88 Hback  296 Hpol P
+ *                  Vfront    8 Vsync  10 Vback   72 Vpol P
+ *     Display Product Name: 'Test EDID'
+ *     Display Range Limits:
+ *       Monitor ranges (GTF): 24-85 Hz V, 24-94 kHz H, max dotclock 340 MHz
+ *     Dummy Descriptor:
+ *   Extension blocks: 1
+ * Checksum: 0xce
+ *
+ * ----------------
+ *
+ * Block 1, CTA-861 Extension Block:
+ *   Revision: 3
+ *   Supports YCbCr 4:4:4
+ *   Supports YCbCr 4:2:2
+ *   Native detailed modes: 1
+ *   Video Data Block:
+ *     VIC  95:  3840x2160   30.000000 Hz  16:9     67.500 kHz    297.000000 MHz
+ *   Vendor-Specific Data Block (HDMI), OUI 00-0C-03:
+ *     Source physical address: 1.0.0.0
+ *     DC_48bit
+ *     DC_36bit
+ *     DC_30bit
+ *     DC_Y444
+ *     Maximum TMDS clock: 340 MHz
+ *     Extended HDMI video details:
+ *   Vendor-Specific Data Block (HDMI Forum), OUI C4-5D-D8:
+ *     Version: 1
+ *     Maximum TMDS Character Rate: 340 MHz
+ *     SCDC Present
+ *     Supports 16-bits/component Deep Color 4:2:0 Pixel Encoding
+ *     Supports 12-bits/component Deep Color 4:2:0 Pixel Encoding
+ *     Supports 10-bits/component Deep Color 4:2:0 Pixel Encoding
+ *   YCbCr 4:2:0 Capability Map Data Block:
+ *     VIC  95:  3840x2160   30.000000 Hz  16:9     67.500 kHz    297.000000 MHz
+ *   YCbCr 4:2:0 Video Data Block:
+ * Checksum: 0x84
+ */
+static const unsigned char test_edid_hdmi_4k_rgb_yuv420_dc_max_340mhz[] = {
+	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x31, 0xd8, 0x34, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0xff, 0x23, 0x01, 0x03, 0x80, 0x60, 0x36, 0x78,
+	0x0f, 0xee, 0x91, 0xa3, 0x54, 0x4c, 0x99, 0x26, 0x0f, 0x50, 0x54, 0x20,
+	0x00, 0x00, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
+	0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x04, 0x74, 0x00, 0x30, 0xf2, 0x70,
+	0x5a, 0x80, 0xb0, 0x58, 0x8a, 0x00, 0x40, 0x84, 0x63, 0x00, 0x00, 0x1e,
+	0x00, 0x00, 0x00, 0xfc, 0x00, 0x54, 0x65, 0x73, 0x74, 0x20, 0x45, 0x44,
+	0x49, 0x44, 0x0a, 0x20, 0x20, 0x20, 0x00, 0x00, 0x00, 0xfd, 0x00, 0x18,
+	0x55, 0x18, 0x5e, 0x22, 0x00, 0x0a, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+	0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0xce, 0x02, 0x03, 0x27, 0x31,
+	0x41, 0x5f, 0x6c, 0x03, 0x0c, 0x00, 0x10, 0x00, 0x78, 0x44, 0x20, 0x00,
+	0x00, 0x01, 0x03, 0x6d, 0xd8, 0x5d, 0xc4, 0x01, 0x44, 0x80, 0x07, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0xe3, 0x0f, 0x01, 0x00, 0xe1, 0x0e, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x84
+};
+
 #endif // DRM_KUNIT_EDID_H_

-- 
2.48.1

Re: [PATCH v2 2/7] drm/connector: hdmi: Add support for YUV420 format verification

[Maxime Ripard]

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

On Tue, Mar 11, 2025 at 12:57:34PM +0200, Cristian Ciocaltea wrote:
> Provide the necessary constraints verification in
> sink_supports_format_bpc() in order to support handling of YUV420
> output format.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/gpu/drm/display/drm_hdmi_state_helper.c | 40 +++++++++++++++++++++++--
>  1 file changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> index 6bc96d5d1ab9115989e208d9899e16cd22254fb6..e99d868edc1854eddc5ebf8692ccffb9e2338268 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> @@ -3,6 +3,7 @@
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_connector.h>
>  #include <drm/drm_edid.h>
> +#include <drm/drm_modes.h>
>  #include <drm/drm_print.h>
>  
>  #include <drm/display/drm_hdmi_audio_helper.h>
> @@ -115,6 +116,12 @@ sink_supports_format_bpc(const struct drm_connector *connector,
>  		return false;
>  	}
>  
> +	if (drm_mode_is_420_only(info, mode) && format != HDMI_COLORSPACE_YUV420) {
> +		drm_dbg_kms(dev, "%s format unsupported by the sink for VIC%u.\n",
> +			    drm_hdmi_connector_get_output_format_name(format), vic);

We don't necessarily have a VIC for the mode we pass, so it's not super
useful to pass it. I'd rather mention that the mode is supposed to be
YUV420 only, but the format isn't YUV420.

> +		return false;
> +	}
> +
>  	switch (format) {
>  	case HDMI_COLORSPACE_RGB:
>  		drm_dbg_kms(dev, "RGB Format, checking the constraints.\n");
> @@ -145,9 +152,36 @@ sink_supports_format_bpc(const struct drm_connector *connector,
>  		return true;
>  
>  	case HDMI_COLORSPACE_YUV420:
> -		/* TODO: YUV420 is unsupported at the moment. */
> -		drm_dbg_kms(dev, "YUV420 format isn't supported yet.\n");
> -		return false;
> +		drm_dbg_kms(dev, "YUV420 format, checking the constraints.\n");
> +
> +		if (!(info->color_formats & DRM_COLOR_FORMAT_YCBCR420)) {
> +			drm_dbg_kms(dev, "Sink doesn't support YUV420.\n");
> +			return false;
> +		}
> +
> +		if (!drm_mode_is_420(info, mode)) {
> +			drm_dbg_kms(dev, "Sink doesn't support YUV420 for VIC%u.\n", vic);

Again, we shouldn't print the VIC here. There's a printk format we can
use to print drm_display_mode if you want to, but we should keep things
consistent.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 3/7] drm/connector: hdmi: Improve debug message for supported format

[Maxime Ripard]

On Tue, 11 Mar 2025 12:57:35 +0200, Cristian Ciocaltea wrote:
> Add the missing 'bpc' string to the debug message indicating the
> supported format identified within hdmi_try_format_bpc() helper.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

Re: [PATCH v2 4/7] drm/connector: hdmi: Use YUV420 output format as an RGB fallback

[Maxime Ripard]

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

Hi,

I think the first thing we need to address is that we will need to
differentiate between HDMI 1.4 devices and HDMI 2.0.

It applies to YUV420, which is HDMI 2.0-only, and I guess your patches
are good enough if you consider YUV420 support only, but scrambler setup
for example is a thing we want to support in that infrastructure
eventually, and is conditioned on HDMI 2.0 as well.

On Tue, Mar 11, 2025 at 12:57:36PM +0200, Cristian Ciocaltea wrote:
> Try to make use of YUV420 when computing the best output format and
> RGB cannot be supported for any of the available color depths.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/gpu/drm/display/drm_hdmi_state_helper.c | 69 +++++++++++++------------
>  1 file changed, 35 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> index a70e204a8df3ac1c2d7318e81cde87a83267dd21..f2052781b797dd09b41127e33d98fe25408a9b23 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> @@ -287,8 +287,9 @@ hdmi_try_format_bpc(const struct drm_connector *connector,
>  	struct drm_device *dev = connector->dev;
>  	int ret;
>  
> -	drm_dbg_kms(dev, "Trying %s output format\n",
> -		    drm_hdmi_connector_get_output_format_name(fmt));
> +	drm_dbg_kms(dev, "Trying %s output format with %u bpc\n",
> +		    drm_hdmi_connector_get_output_format_name(fmt),
> +		    bpc);

That part should be in a separate patch, it's independant of the rest.

>  	if (!sink_supports_format_bpc(connector, info, mode, fmt, bpc)) {
>  		drm_dbg_kms(dev, "%s output format not supported with %u bpc\n",
> @@ -313,47 +314,22 @@ hdmi_try_format_bpc(const struct drm_connector *connector,
>  }
>  
>  static int
> -hdmi_compute_format(const struct drm_connector *connector,
> -		    struct drm_connector_state *conn_state,
> -		    const struct drm_display_mode *mode,
> -		    unsigned int bpc)
> -{
> -	struct drm_device *dev = connector->dev;
> -
> -	/*
> -	 * TODO: Add support for YCbCr420 output for HDMI 2.0 capable
> -	 * devices, for modes that only support YCbCr420.
> -	 */
> -	if (hdmi_try_format_bpc(connector, conn_state, mode, bpc, HDMI_COLORSPACE_RGB)) {
> -		conn_state->hdmi.output_format = HDMI_COLORSPACE_RGB;
> -		return 0;
> -	}
> -
> -	drm_dbg_kms(dev, "Failed. No Format Supported for that bpc count.\n");
> -
> -	return -EINVAL;
> -}
> -
> -static int
> -hdmi_compute_config(const struct drm_connector *connector,
> -		    struct drm_connector_state *conn_state,
> -		    const struct drm_display_mode *mode)
> +hdmi_try_format(const struct drm_connector *connector,
> +		struct drm_connector_state *conn_state,
> +		const struct drm_display_mode *mode,
> +		unsigned int max_bpc, enum hdmi_colorspace fmt)
>  {
>  	struct drm_device *dev = connector->dev;
> -	unsigned int max_bpc = clamp_t(unsigned int,
> -				       conn_state->max_bpc,
> -				       8, connector->max_bpc);
>  	unsigned int bpc;
>  	int ret;
>  
>  	for (bpc = max_bpc; bpc >= 8; bpc -= 2) {
> -		drm_dbg_kms(dev, "Trying with a %d bpc output\n", bpc);
> -
> -		ret = hdmi_compute_format(connector, conn_state, mode, bpc);
> -		if (ret)
> +		ret = hdmi_try_format_bpc(connector, conn_state, mode, bpc, fmt);
> +		if (!ret)
>  			continue;
>  
>  		conn_state->hdmi.output_bpc = bpc;
> +		conn_state->hdmi.output_format = fmt;

I guess it's a matter of semantics, but if it sets the value in the
state, it doesn't try. Maybe the function should be named
hdmi_compute_format_bpc then?

That renaming should be in a separate patch too (possibly several).

>  		drm_dbg_kms(dev,
>  			    "Mode %ux%u @ %uHz: Found configuration: bpc: %u, fmt: %s, clock: %llu\n",
> @@ -368,6 +344,31 @@ hdmi_compute_config(const struct drm_connector *connector,
>  	return -EINVAL;
>  }
>  
> +static int
> +hdmi_compute_config(const struct drm_connector *connector,
> +		    struct drm_connector_state *conn_state,
> +		    const struct drm_display_mode *mode)
> +{
> +	unsigned int max_bpc = clamp_t(unsigned int,
> +				       conn_state->max_bpc,
> +				       8, connector->max_bpc);
> +	int ret;
> +
> +	ret = hdmi_try_format(connector, conn_state, mode, max_bpc,
> +			      HDMI_COLORSPACE_RGB);
> +	if (!ret)
> +		return 0;
> +
> +	if (connector->ycbcr_420_allowed)
> +		ret = hdmi_try_format(connector, conn_state, mode, max_bpc,
> +				      HDMI_COLORSPACE_YUV420);

I think that's conditioned on a few more things:
  - That the driver supports HDMI 2.0
  - That the display is an HDMI output
  - That the mode is allowed YUV420 by the sink EDIDs

> +	else
> +		drm_dbg_kms(connector->dev,
> +			    "%s output format not allowed for connector\n",
> +			    drm_hdmi_connector_get_output_format_name(HDMI_COLORSPACE_YUV420));

And I think we should keep the catch-all failure message we had.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 5/7] drm/tests: hdmi: Add macros to simplify EDID setup

[Maxime Ripard]

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

On Tue, Mar 11, 2025 at 12:57:37PM +0200, Cristian Ciocaltea wrote:
> Introduce a few macros to facilitate setting custom (i.e. non-default)
> EDID data during connector initialization.
> 
> This helps reducing boilerplate code while also drops some redundant
> calls to set_connector_edid().
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 245 ++++++++-------------
>  1 file changed, 93 insertions(+), 152 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> index e97efd3af9ed18e6cf8ee66b4923dfc805b34e19..a3f7f3ce31c73335c2c2643bdc5395b6ceb6f071 100644
> --- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> +++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> @@ -183,10 +183,12 @@ static const struct drm_connector_funcs dummy_connector_funcs = {
>  
>  static
>  struct drm_atomic_helper_connector_hdmi_priv *
> -drm_kunit_helper_connector_hdmi_init_funcs(struct kunit *test,
> -					   unsigned int formats,
> -					   unsigned int max_bpc,
> -					   const struct drm_connector_hdmi_funcs *hdmi_funcs)
> +connector_hdmi_init_funcs_set_edid(struct kunit *test,
> +				   unsigned int formats,
> +				   unsigned int max_bpc,
> +				   const struct drm_connector_hdmi_funcs *hdmi_funcs,
> +				   const char *edid_data,
> +				   size_t edid_len)
>  {
>  	struct drm_atomic_helper_connector_hdmi_priv *priv;
>  	struct drm_connector *conn;
> @@ -240,30 +242,27 @@ drm_kunit_helper_connector_hdmi_init_funcs(struct kunit *test,
>  
>  	drm_mode_config_reset(drm);
>  
> +	if (edid_data && edid_len) {
> +		ret = set_connector_edid(test, &priv->connector, edid_data, edid_len);
> +		KUNIT_ASSERT_GT(test, ret, 0);
> +	}
> +
>  	return priv;
>  }
>  
> -static
> -struct drm_atomic_helper_connector_hdmi_priv *
> -drm_kunit_helper_connector_hdmi_init(struct kunit *test,
> -				     unsigned int formats,
> -				     unsigned int max_bpc)
> -{
> -	struct drm_atomic_helper_connector_hdmi_priv *priv;
> -	int ret;
> +#define drm_kunit_helper_connector_hdmi_init_funcs_set_edid(test, formats, max_bpc, funcs, edid) \
> +	connector_hdmi_init_funcs_set_edid(test, formats, max_bpc, funcs, edid, ARRAY_SIZE(edid))
>  
> -	priv = drm_kunit_helper_connector_hdmi_init_funcs(test,
> -							  formats, max_bpc,
> -							  &dummy_connector_hdmi_funcs);
> -	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
> +#define drm_kunit_helper_connector_hdmi_init_funcs(test, formats, max_bpc, funcs)		\
> +	connector_hdmi_init_funcs_set_edid(test, formats, max_bpc, funcs, NULL, 0)
>  
> -	ret = set_connector_edid(test, &priv->connector,
> -				 test_edid_hdmi_1080p_rgb_max_200mhz,
> -				 ARRAY_SIZE(test_edid_hdmi_1080p_rgb_max_200mhz));
> -	KUNIT_ASSERT_GT(test, ret, 0);
> +#define drm_kunit_helper_connector_hdmi_init_set_edid(test, formats, max_bpc, edid)		\
> +	drm_kunit_helper_connector_hdmi_init_funcs_set_edid(test, formats, max_bpc,		\
> +							    &dummy_connector_hdmi_funcs, edid)
>  
> -	return priv;
> -}
> +#define drm_kunit_helper_connector_hdmi_init(test, formats, max_bpc)				\
> +	drm_kunit_helper_connector_hdmi_init_set_edid(test, formats, max_bpc,			\
> +						      test_edid_hdmi_1080p_rgb_max_200mhz)

I'd really prefer to have functions to macros here. They are easier to
read, extend, and don't have any particular drawbacks.

I also don't think we need that many, looking at the tests:

  - We need drm_kunit_helper_connector_hdmi_init() to setup a connector
    with test_edid_hdmi_1080p_rgb_max_200mhz and
    dummy_connector_hdmi_funcs()

  - We need to create a
    drm_kunit_helper_connector_hdmi_init_with_edid_funcs() to pass both
    the funcs and edid pointers

And that's it, right?

>  /*
>   * Test that if we change the RGB quantization property to a different
> @@ -771,19 +770,15 @@ static void drm_test_check_output_bpc_crtc_mode_changed(struct kunit *test)
>  	struct drm_crtc *crtc;
>  	int ret;
>  
> -	priv = drm_kunit_helper_connector_hdmi_init(test,
> -						    BIT(HDMI_COLORSPACE_RGB),
> -						    10);
> +	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
> +				BIT(HDMI_COLORSPACE_RGB),
> +				10,
> +				test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz);

I think that convertion should be part of another patch.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 6/7] drm/tests: hdmi: Add limited range tests for YUV420 mode

[Maxime Ripard]

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

On Tue, Mar 11, 2025 at 12:57:38PM +0200, Cristian Ciocaltea wrote:
> Provide tests to verify that drm_atomic_helper_connector_hdmi_check()
> helper behaviour when using YUV420 output format is to always set the
> limited RGB quantization range to 'limited', no matter what the value of
> Broadcast RGB property is.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c |  89 +++++++++++++++-
>  drivers/gpu/drm/tests/drm_kunit_edid.h             | 112 +++++++++++++++++++++
>  2 files changed, 196 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> index a3f7f3ce31c73335c2c2643bdc5395b6ceb6f071..1df12c0b7768e4f85f4c943840d9b4dcb6e079e0 100644
> --- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> +++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> @@ -227,6 +227,8 @@ connector_hdmi_init_funcs_set_edid(struct kunit *test,
>  	enc->possible_crtcs = drm_crtc_mask(priv->crtc);
>  
>  	conn = &priv->connector;
> +	conn->ycbcr_420_allowed = !!(formats & BIT(HDMI_COLORSPACE_YUV420));
> +
>  	ret = drmm_connector_hdmi_init(drm, conn,
>  				       "Vendor", "Product",
>  				       &dummy_connector_funcs,
> @@ -751,6 +753,86 @@ static void drm_test_check_broadcast_rgb_limited_cea_mode_vic_1(struct kunit *te
>  	drm_modeset_acquire_fini(&ctx);
>  }
>  
> +/*
> + * Test that for an HDMI connector, with an HDMI monitor, we will
> + * get a limited RGB Quantization Range with a YUV420 mode, no
> + * matter what the value of the Broadcast RGB property is set to.
> + */
> +static void drm_test_check_broadcast_rgb_cea_mode_yuv420(struct kunit *test)
> +{
> +	struct drm_atomic_helper_connector_hdmi_priv *priv;
> +	enum drm_hdmi_broadcast_rgb broadcast_rgb;
> +	struct drm_modeset_acquire_ctx ctx;
> +	struct drm_connector_state *conn_state;
> +	struct drm_atomic_state *state;
> +	struct drm_display_mode *mode;
> +	struct drm_connector *conn;
> +	struct drm_device *drm;
> +	struct drm_crtc *crtc;
> +	int ret;
> +
> +	broadcast_rgb = *(enum drm_hdmi_broadcast_rgb *)test->param_value;
> +
> +	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
> +				BIT(HDMI_COLORSPACE_RGB) |
> +				BIT(HDMI_COLORSPACE_YUV420),
> +				8,
> +				test_edid_hdmi_1080p_rgb_yuv_4k_yuv420_dc_max_200mhz);
> +	KUNIT_ASSERT_NOT_NULL(test, priv);
> +
> +	drm = &priv->drm;
> +	crtc = priv->crtc;
> +	conn = &priv->connector;
> +	KUNIT_ASSERT_TRUE(test, conn->display_info.is_hdmi);
> +
> +	mode = drm_kunit_display_mode_from_cea_vic(test, drm, 95);
> +	KUNIT_ASSERT_NOT_NULL(test, mode);
> +
> +	drm_modeset_acquire_init(&ctx, 0);
> +
> +	ret = light_up_connector(test, drm, crtc, conn, mode, &ctx);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	state = drm_kunit_helper_atomic_state_alloc(test, drm, &ctx);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
> +
> +	conn_state = drm_atomic_get_connector_state(state, conn);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
> +
> +	conn_state->hdmi.broadcast_rgb = broadcast_rgb;
> +
> +	ret = drm_atomic_check_only(state);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	conn_state = drm_atomic_get_connector_state(state, conn);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
> +
> +	KUNIT_ASSERT_EQ(test, conn_state->hdmi.broadcast_rgb, broadcast_rgb);
> +	KUNIT_ASSERT_EQ(test, conn_state->hdmi.output_format, HDMI_COLORSPACE_YUV420);
> +
> +	KUNIT_EXPECT_TRUE(test, conn_state->hdmi.is_limited_range);
> +
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
> +}
> +
> +static const enum drm_hdmi_broadcast_rgb check_broadcast_rgb_cea_mode_yuv420_tests[] = {
> +	DRM_HDMI_BROADCAST_RGB_AUTO,
> +	DRM_HDMI_BROADCAST_RGB_FULL,
> +	DRM_HDMI_BROADCAST_RGB_LIMITED,
> +};
> +
> +static void
> +check_broadcast_rgb_cea_mode_yuv420_desc(const enum drm_hdmi_broadcast_rgb *broadcast_rgb,
> +					 char *desc)
> +{
> +	sprintf(desc, "%s", drm_hdmi_connector_get_broadcast_rgb_name(*broadcast_rgb));
> +}
> +
> +KUNIT_ARRAY_PARAM(check_broadcast_rgb_cea_mode_yuv420,
> +		  check_broadcast_rgb_cea_mode_yuv420_tests,
> +		  check_broadcast_rgb_cea_mode_yuv420_desc);
> +

We need more tests than that to test the various combinations, whether
the fallback to YUV420 should work or not depending on the EDID, the
driver capabilities, YUV420-only vs YUV420-also, etc.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/7] drm/connector: hdmi: Add support for YUV420 format verification

[Cristian Ciocaltea]

On 3/11/25 5:30 PM, Maxime Ripard wrote:
> On Tue, Mar 11, 2025 at 12:57:34PM +0200, Cristian Ciocaltea wrote:
>> Provide the necessary constraints verification in
>> sink_supports_format_bpc() in order to support handling of YUV420
>> output format.
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>  drivers/gpu/drm/display/drm_hdmi_state_helper.c | 40 +++++++++++++++++++++++--
>>  1 file changed, 37 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
>> index 6bc96d5d1ab9115989e208d9899e16cd22254fb6..e99d868edc1854eddc5ebf8692ccffb9e2338268 100644
>> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
>> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
>> @@ -3,6 +3,7 @@
>>  #include <drm/drm_atomic.h>
>>  #include <drm/drm_connector.h>
>>  #include <drm/drm_edid.h>
>> +#include <drm/drm_modes.h>
>>  #include <drm/drm_print.h>
>>  
>>  #include <drm/display/drm_hdmi_audio_helper.h>
>> @@ -115,6 +116,12 @@ sink_supports_format_bpc(const struct drm_connector *connector,
>>  		return false;
>>  	}
>>  
>> +	if (drm_mode_is_420_only(info, mode) && format != HDMI_COLORSPACE_YUV420) {
>> +		drm_dbg_kms(dev, "%s format unsupported by the sink for VIC%u.\n",
>> +			    drm_hdmi_connector_get_output_format_name(format), vic);
> 
> We don't necessarily have a VIC for the mode we pass, so it's not super
> useful to pass it. I'd rather mention that the mode is supposed to be
> YUV420 only, but the format isn't YUV420.

Ack, I'll change the message as suggested.

>> +		return false;
>> +	}
>> +
>>  	switch (format) {
>>  	case HDMI_COLORSPACE_RGB:
>>  		drm_dbg_kms(dev, "RGB Format, checking the constraints.\n");
>> @@ -145,9 +152,36 @@ sink_supports_format_bpc(const struct drm_connector *connector,
>>  		return true;
>>  
>>  	case HDMI_COLORSPACE_YUV420:
>> -		/* TODO: YUV420 is unsupported at the moment. */
>> -		drm_dbg_kms(dev, "YUV420 format isn't supported yet.\n");
>> -		return false;
>> +		drm_dbg_kms(dev, "YUV420 format, checking the constraints.\n");
>> +
>> +		if (!(info->color_formats & DRM_COLOR_FORMAT_YCBCR420)) {
>> +			drm_dbg_kms(dev, "Sink doesn't support YUV420.\n");
>> +			return false;
>> +		}
>> +
>> +		if (!drm_mode_is_420(info, mode)) {
>> +			drm_dbg_kms(dev, "Sink doesn't support YUV420 for VIC%u.\n", vic);
> 
> Again, we shouldn't print the VIC here. There's a printk format we can
> use to print drm_display_mode if you want to, but we should keep things
> consistent.

Agree, will proceed as above.

Thanks for the review,
Cristian

Re: [PATCH v2 4/7] drm/connector: hdmi: Use YUV420 output format as an RGB fallback

[Cristian Ciocaltea]

Hi Maxime,

On 3/11/25 5:55 PM, Maxime Ripard wrote:
> Hi,
> 
> I think the first thing we need to address is that we will need to
> differentiate between HDMI 1.4 devices and HDMI 2.0.
> 
> It applies to YUV420, which is HDMI 2.0-only, and I guess your patches
> are good enough if you consider YUV420 support only

Yes, my intention was to get the very basic support for now.

, but scrambler setup
> for example is a thing we want to support in that infrastructure
> eventually, and is conditioned on HDMI 2.0 as well.

Right, the scrambler setup is actually among the next tasks I'll focus on,
e.g. this is still missing on dw-hdmi-qp side and I got it reworked a bit
according to your initial review.  It would probably make sense for me to
submit that and get some feedback before attempting to go for a generic
approach (still need to do a few more checks/improvements before).

> On Tue, Mar 11, 2025 at 12:57:36PM +0200, Cristian Ciocaltea wrote:
>> Try to make use of YUV420 when computing the best output format and
>> RGB cannot be supported for any of the available color depths.
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>  drivers/gpu/drm/display/drm_hdmi_state_helper.c | 69 +++++++++++++------------
>>  1 file changed, 35 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
>> index a70e204a8df3ac1c2d7318e81cde87a83267dd21..f2052781b797dd09b41127e33d98fe25408a9b23 100644
>> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
>> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
>> @@ -287,8 +287,9 @@ hdmi_try_format_bpc(const struct drm_connector *connector,
>>  	struct drm_device *dev = connector->dev;
>>  	int ret;
>>  
>> -	drm_dbg_kms(dev, "Trying %s output format\n",
>> -		    drm_hdmi_connector_get_output_format_name(fmt));
>> +	drm_dbg_kms(dev, "Trying %s output format with %u bpc\n",
>> +		    drm_hdmi_connector_get_output_format_name(fmt),
>> +		    bpc);
> 
> That part should be in a separate patch, it's independant of the rest.

Ack.

> 
>>  	if (!sink_supports_format_bpc(connector, info, mode, fmt, bpc)) {
>>  		drm_dbg_kms(dev, "%s output format not supported with %u bpc\n",
>> @@ -313,47 +314,22 @@ hdmi_try_format_bpc(const struct drm_connector *connector,
>>  }
>>  
>>  static int
>> -hdmi_compute_format(const struct drm_connector *connector,
>> -		    struct drm_connector_state *conn_state,
>> -		    const struct drm_display_mode *mode,
>> -		    unsigned int bpc)
>> -{
>> -	struct drm_device *dev = connector->dev;
>> -
>> -	/*
>> -	 * TODO: Add support for YCbCr420 output for HDMI 2.0 capable
>> -	 * devices, for modes that only support YCbCr420.
>> -	 */
>> -	if (hdmi_try_format_bpc(connector, conn_state, mode, bpc, HDMI_COLORSPACE_RGB)) {
>> -		conn_state->hdmi.output_format = HDMI_COLORSPACE_RGB;
>> -		return 0;
>> -	}
>> -
>> -	drm_dbg_kms(dev, "Failed. No Format Supported for that bpc count.\n");
>> -
>> -	return -EINVAL;
>> -}
>> -
>> -static int
>> -hdmi_compute_config(const struct drm_connector *connector,
>> -		    struct drm_connector_state *conn_state,
>> -		    const struct drm_display_mode *mode)
>> +hdmi_try_format(const struct drm_connector *connector,
>> +		struct drm_connector_state *conn_state,
>> +		const struct drm_display_mode *mode,
>> +		unsigned int max_bpc, enum hdmi_colorspace fmt)
>>  {
>>  	struct drm_device *dev = connector->dev;
>> -	unsigned int max_bpc = clamp_t(unsigned int,
>> -				       conn_state->max_bpc,
>> -				       8, connector->max_bpc);
>>  	unsigned int bpc;
>>  	int ret;
>>  
>>  	for (bpc = max_bpc; bpc >= 8; bpc -= 2) {
>> -		drm_dbg_kms(dev, "Trying with a %d bpc output\n", bpc);
>> -
>> -		ret = hdmi_compute_format(connector, conn_state, mode, bpc);
>> -		if (ret)
>> +		ret = hdmi_try_format_bpc(connector, conn_state, mode, bpc, fmt);
>> +		if (!ret)
>>  			continue;
>>  
>>  		conn_state->hdmi.output_bpc = bpc;
>> +		conn_state->hdmi.output_format = fmt;
> 
> I guess it's a matter of semantics, but if it sets the value in the
> state, it doesn't try. Maybe the function should be named
> hdmi_compute_format_bpc then?

Good point!

> 
> That renaming should be in a separate patch too (possibly several).

Yes, I'll move all these preparatory changes into separate patch(es).

> 
>>  		drm_dbg_kms(dev,
>>  			    "Mode %ux%u @ %uHz: Found configuration: bpc: %u, fmt: %s, clock: %llu\n",
>> @@ -368,6 +344,31 @@ hdmi_compute_config(const struct drm_connector *connector,
>>  	return -EINVAL;
>>  }
>>  
>> +static int
>> +hdmi_compute_config(const struct drm_connector *connector,
>> +		    struct drm_connector_state *conn_state,
>> +		    const struct drm_display_mode *mode)
>> +{
>> +	unsigned int max_bpc = clamp_t(unsigned int,
>> +				       conn_state->max_bpc,
>> +				       8, connector->max_bpc);
>> +	int ret;
>> +
>> +	ret = hdmi_try_format(connector, conn_state, mode, max_bpc,
>> +			      HDMI_COLORSPACE_RGB);
>> +	if (!ret)
>> +		return 0;
>> +
>> +	if (connector->ycbcr_420_allowed)
>> +		ret = hdmi_try_format(connector, conn_state, mode, max_bpc,
>> +				      HDMI_COLORSPACE_YUV420);
> 
> I think that's conditioned on a few more things:

I've actually expected this! :-)

You've already raised some points during v1, but I preferred to restart the
discussion on updated code instead - sorry for taking so long to respin the
series.  In particular, I worked on [1] to improve handling of
ycbcr_420_allowed flag and fix some consistency issues with
HDMI_COLORSPACE_YUV420 advertised in drm_bridge->supported_formats.  Hence
I assumed it's now safe to rely exclusively on this flag to indicate the
connector is YUV420 capable, without doing any additional checks.

>   - That the driver supports HDMI 2.0

Probably I'm missing something obvious here, but is this necessary to
actually double-check ycbcr_420_allowed has been set correctly?

E.g. for bridges with DRM_BRIDGE_OP_HDMI set in drm_bridge->ops, the
framework does already adjust ycbcr_420_allowed, hence any additional
verification would be redundant.  When not making use of the framework,
drivers are not expected to set the flag if they are not HDMI 2.0 compliant
or not supporting YUV420, right? Are there any other use cases we need to
handle?

>   - That the display is an HDMI output

I think this should be handled by sink_supports_format_bpc() via:

    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;
    }

>   - That the mode is allowed YUV420 by the sink EDIDs

And that would be handled via the changes introduced by "drm/connector:
hdmi: Add support for YUV420 format verification".

>> +	else
>> +		drm_dbg_kms(connector->dev,
>> +			    "%s output format not allowed for connector\n",
>> +			    drm_hdmi_connector_get_output_format_name(HDMI_COLORSPACE_YUV420));
> 
> And I think we should keep the catch-all failure message we had.

IIRC, the rational for the change was to get rid of some redundancy, but
I'll recheck and make sure to keep that message in place.

Thanks,
Cristian

[1] https://patchwork.freedesktop.org/series/142679/

Re: [PATCH v2 1/7] drm/connector: hdmi: Evaluate limited range after computing format

[Dmitry Baryshkov]

On Tue, Mar 11, 2025 at 12:57:33PM +0200, Cristian Ciocaltea wrote:
> Evaluating the requirement to use a limited RGB quantization range
> involves a verification of the output format, among others, but this is
> currently performed before actually computing the format, hence relying
> on the old connector state.
> 
> Move the call to hdmi_is_limited_range() after hdmi_compute_config() to
> ensure the verification is done on the updated output format.
> 
> Fixes: 027d43590649 ("drm/connector: hdmi: Add RGB Quantization Range to the connector state")
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/gpu/drm/display/drm_hdmi_state_helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <lumag@kernel.org>

-- 
With best wishes
Dmitry

Re: [PATCH v2 4/7] drm/connector: hdmi: Use YUV420 output format as an RGB fallback

[Dmitry Baryshkov]

On Tue, Mar 11, 2025 at 04:55:17PM +0100, Maxime Ripard wrote:
> Hi,
> 
> I think the first thing we need to address is that we will need to
> differentiate between HDMI 1.4 devices and HDMI 2.0.
> 
> It applies to YUV420, which is HDMI 2.0-only, and I guess your patches
> are good enough if you consider YUV420 support only, but scrambler setup
> for example is a thing we want to support in that infrastructure
> eventually, and is conditioned on HDMI 2.0 as well.
> 
> On Tue, Mar 11, 2025 at 12:57:36PM +0200, Cristian Ciocaltea wrote:
> > Try to make use of YUV420 when computing the best output format and
> > RGB cannot be supported for any of the available color depths.
> > 
> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> > ---
> >  drivers/gpu/drm/display/drm_hdmi_state_helper.c | 69 +++++++++++++------------
> >  1 file changed, 35 insertions(+), 34 deletions(-)
> > 

[...]

> >  	return -EINVAL;
> >  }
> >  
> > +static int
> > +hdmi_compute_config(const struct drm_connector *connector,
> > +		    struct drm_connector_state *conn_state,
> > +		    const struct drm_display_mode *mode)
> > +{
> > +	unsigned int max_bpc = clamp_t(unsigned int,
> > +				       conn_state->max_bpc,
> > +				       8, connector->max_bpc);
> > +	int ret;
> > +
> > +	ret = hdmi_try_format(connector, conn_state, mode, max_bpc,
> > +			      HDMI_COLORSPACE_RGB);
> > +	if (!ret)
> > +		return 0;
> > +
> > +	if (connector->ycbcr_420_allowed)
> > +		ret = hdmi_try_format(connector, conn_state, mode, max_bpc,
> > +				      HDMI_COLORSPACE_YUV420);
> 
> I think that's conditioned on a few more things:
>   - That the driver supports HDMI 2.0

Isn't that included into connector->ycbcr_420_allowed? I'd expect that
HDMI 1.4-only drivers don't set that flag.

>   - That the display is an HDMI output
>   - That the mode is allowed YUV420 by the sink EDIDs
> 
> > +	else
> > +		drm_dbg_kms(connector->dev,
> > +			    "%s output format not allowed for connector\n",
> > +			    drm_hdmi_connector_get_output_format_name(HDMI_COLORSPACE_YUV420));
> 
> And I think we should keep the catch-all failure message we had.
> 
> Maxime



-- 
With best wishes
Dmitry

Re: [PATCH v2 5/7] drm/tests: hdmi: Add macros to simplify EDID setup

[Cristian Ciocaltea]

On 3/11/25 6:12 PM, Maxime Ripard wrote:
> On Tue, Mar 11, 2025 at 12:57:37PM +0200, Cristian Ciocaltea wrote:
>> Introduce a few macros to facilitate setting custom (i.e. non-default)
>> EDID data during connector initialization.
>>
>> This helps reducing boilerplate code while also drops some redundant
>> calls to set_connector_edid().
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>  drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 245 ++++++++-------------
>>  1 file changed, 93 insertions(+), 152 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
>> index e97efd3af9ed18e6cf8ee66b4923dfc805b34e19..a3f7f3ce31c73335c2c2643bdc5395b6ceb6f071 100644
>> --- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
>> +++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
>> @@ -183,10 +183,12 @@ static const struct drm_connector_funcs dummy_connector_funcs = {
>>  
>>  static
>>  struct drm_atomic_helper_connector_hdmi_priv *
>> -drm_kunit_helper_connector_hdmi_init_funcs(struct kunit *test,
>> -					   unsigned int formats,
>> -					   unsigned int max_bpc,
>> -					   const struct drm_connector_hdmi_funcs *hdmi_funcs)
>> +connector_hdmi_init_funcs_set_edid(struct kunit *test,
>> +				   unsigned int formats,
>> +				   unsigned int max_bpc,
>> +				   const struct drm_connector_hdmi_funcs *hdmi_funcs,
>> +				   const char *edid_data,
>> +				   size_t edid_len)
>>  {
>>  	struct drm_atomic_helper_connector_hdmi_priv *priv;
>>  	struct drm_connector *conn;
>> @@ -240,30 +242,27 @@ drm_kunit_helper_connector_hdmi_init_funcs(struct kunit *test,
>>  
>>  	drm_mode_config_reset(drm);
>>  
>> +	if (edid_data && edid_len) {
>> +		ret = set_connector_edid(test, &priv->connector, edid_data, edid_len);
>> +		KUNIT_ASSERT_GT(test, ret, 0);
>> +	}
>> +
>>  	return priv;
>>  }
>>  
>> -static
>> -struct drm_atomic_helper_connector_hdmi_priv *
>> -drm_kunit_helper_connector_hdmi_init(struct kunit *test,
>> -				     unsigned int formats,
>> -				     unsigned int max_bpc)
>> -{
>> -	struct drm_atomic_helper_connector_hdmi_priv *priv;
>> -	int ret;
>> +#define drm_kunit_helper_connector_hdmi_init_funcs_set_edid(test, formats, max_bpc, funcs, edid) \
>> +	connector_hdmi_init_funcs_set_edid(test, formats, max_bpc, funcs, edid, ARRAY_SIZE(edid))
>>  
>> -	priv = drm_kunit_helper_connector_hdmi_init_funcs(test,
>> -							  formats, max_bpc,
>> -							  &dummy_connector_hdmi_funcs);
>> -	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
>> +#define drm_kunit_helper_connector_hdmi_init_funcs(test, formats, max_bpc, funcs)		\
>> +	connector_hdmi_init_funcs_set_edid(test, formats, max_bpc, funcs, NULL, 0)
>>  
>> -	ret = set_connector_edid(test, &priv->connector,
>> -				 test_edid_hdmi_1080p_rgb_max_200mhz,
>> -				 ARRAY_SIZE(test_edid_hdmi_1080p_rgb_max_200mhz));
>> -	KUNIT_ASSERT_GT(test, ret, 0);
>> +#define drm_kunit_helper_connector_hdmi_init_set_edid(test, formats, max_bpc, edid)		\
>> +	drm_kunit_helper_connector_hdmi_init_funcs_set_edid(test, formats, max_bpc,		\
>> +							    &dummy_connector_hdmi_funcs, edid)
>>  
>> -	return priv;
>> -}
>> +#define drm_kunit_helper_connector_hdmi_init(test, formats, max_bpc)				\
>> +	drm_kunit_helper_connector_hdmi_init_set_edid(test, formats, max_bpc,			\
>> +						      test_edid_hdmi_1080p_rgb_max_200mhz)
> 
> I'd really prefer to have functions to macros here. They are easier to
> read, extend, and don't have any particular drawbacks.

Yeah, the main reason I opted for macros was to allow dropping
ARRAY_SIZE(edid) from the caller side, hence making the API as simple as
possible.

> I also don't think we need that many, looking at the tests:
> 
>   - We need drm_kunit_helper_connector_hdmi_init() to setup a connector
>     with test_edid_hdmi_1080p_rgb_max_200mhz and
>     dummy_connector_hdmi_funcs()

Correct.

>   - We need to create a
>     drm_kunit_helper_connector_hdmi_init_with_edid_funcs() to pass both
>     the funcs and edid pointers

That's drm_kunit_helper_connector_hdmi_init_funcs_set_edid(), but I can
rename it if you prefer - I've just tried to keep the name length under
control, as there are already some indentation challenges when calling
the helpers.

Currently it's only used to implement
drm_kunit_helper_connector_hdmi_init_set_edid() by passing
&dummy_connector_hdmi_funcs, but there are a few test cases that can
benefit of it and help extend the cleanup - will do for v3.

drm_kunit_helper_connector_hdmi_init_set_edid() should also stay as it's
already being used to drop boilerplate code from a lot of places.

> And that's it, right?

There's also drm_kunit_helper_connector_hdmi_init_funcs() which has been
used for a few times, but it can be further optimized out via
drm_kunit_helper_connector_hdmi_init_funcs_set_edid(), so I'll drop it.

That means we could end up with just the following:

- drm_kunit_helper_connector_hdmi_init()
- drm_kunit_helper_connector_hdmi_init_set_edid()
- drm_kunit_helper_connector_hdmi_init_funcs_set_edid()

>>  /*
>>   * Test that if we change the RGB quantization property to a different
>> @@ -771,19 +770,15 @@ static void drm_test_check_output_bpc_crtc_mode_changed(struct kunit *test)
>>  	struct drm_crtc *crtc;
>>  	int ret;
>>  
>> -	priv = drm_kunit_helper_connector_hdmi_init(test,
>> -						    BIT(HDMI_COLORSPACE_RGB),
>> -						    10);
>> +	priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
>> +				BIT(HDMI_COLORSPACE_RGB),
>> +				10,
>> +				test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz);
> 
> I think that convertion should be part of another patch.

Ack, will move all conversions to a dedicated patch.

Thanks,
Cristian

Re: [PATCH v2 6/7] drm/tests: hdmi: Add limited range tests for YUV420 mode

[Cristian Ciocaltea]

On 3/11/25 6:17 PM, Maxime Ripard wrote:
> On Tue, Mar 11, 2025 at 12:57:38PM +0200, Cristian Ciocaltea wrote:
>> Provide tests to verify that drm_atomic_helper_connector_hdmi_check()
>> helper behaviour when using YUV420 output format is to always set the
>> limited RGB quantization range to 'limited', no matter what the value of
>> Broadcast RGB property is.

[...]

> We need more tests than that to test the various combinations, whether
> the fallback to YUV420 should work or not depending on the EDID, the
> driver capabilities, YUV420-only vs YUV420-also, etc.

Some fallback tests were provided in the next patch, including checks like:

  KUNIT_ASSERT_TRUE(test, drm_mode_is_420_only(info, yuv420_only_mode));
  KUNIT_ASSERT_TRUE(test, drm_mode_is_420_also(info, preferred));

I'll try to further extend the test coverage.

Regards,
Cristian

Re: [PATCH v2 7/7] drm/tests: hdmi: Add max TMDS rate fallback tests for YUV420 mode

[kernel test robot]

Hi Cristian,

kernel test robot noticed the following build errors:

[auto build test ERROR on 4423e607ff50157aaf088854b145936cbab4d560]

url:    https://github.com/intel-lab-lkp/linux/commits/Cristian-Ciocaltea/drm-connector-hdmi-Evaluate-limited-range-after-computing-format/20250311-190150
base:   4423e607ff50157aaf088854b145936cbab4d560
patch link:    https://lore.kernel.org/r/20250311-hdmi-conn-yuv-v2-7-fbdb94f02562%40collabora.com
patch subject: [PATCH v2 7/7] drm/tests: hdmi: Add max TMDS rate fallback tests for YUV420 mode
config: x86_64-randconfig-004-20250312 (https://download.01.org/0day-ci/archive/20250312/202503121909.5IeaFEUt-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250312/202503121909.5IeaFEUt-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503121909.5IeaFEUt-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/drm/drm_kunit_helpers.h:10,
                    from drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c:14:
   drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c: In function 'drm_test_check_max_tmds_rate_bpc_fallback_yuv420':
>> include/kunit/test.h:776:29: error: SSE register return with SSE disabled
     776 |         const typeof(right) __right = (right);                                 \
         |                             ^~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
--
   In file included from include/drm/drm_kunit_helpers.h:10,
                    from drm_hdmi_state_helper_test.c:14:
   drm_hdmi_state_helper_test.c: In function 'drm_test_check_max_tmds_rate_bpc_fallback_yuv420':
>> include/kunit/test.h:776:29: error: SSE register return with SSE disabled
     776 |         const typeof(right) __right = (right);                                 \
         |                             ^~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~


vim +776 include/kunit/test.h

73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  734  
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  735  #define KUNIT_TRUE_MSG_ASSERTION(test, assert_type, condition, fmt, ...)       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  736  	KUNIT_UNARY_ASSERTION(test,					       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  737  			      assert_type,				       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  738  			      condition,				       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  739  			      true,					       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  740  			      fmt,					       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  741  			      ##__VA_ARGS__)
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  742  
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  743  #define KUNIT_FALSE_MSG_ASSERTION(test, assert_type, condition, fmt, ...)      \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  744  	KUNIT_UNARY_ASSERTION(test,					       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  745  			      assert_type,				       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  746  			      condition,				       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  747  			      false,					       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  748  			      fmt,					       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  749  			      ##__VA_ARGS__)
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  750  
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  751  /*
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  752   * A factory macro for defining the assertions and expectations for the basic
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  753   * comparisons defined for the built in types.
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  754   *
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  755   * Unfortunately, there is no common type that all types can be promoted to for
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  756   * which all the binary operators behave the same way as for the actual types
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  757   * (for example, there is no type that long long and unsigned long long can
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  758   * both be cast to where the comparison result is preserved for all values). So
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  759   * the best we can do is do the comparison in the original types and then coerce
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  760   * everything to long long for printing; this way, the comparison behaves
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  761   * correctly and the printed out value usually makes sense without
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  762   * interpretation, but can always be interpreted to figure out the actual
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  763   * value.
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  764   */
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  765  #define KUNIT_BASE_BINARY_ASSERTION(test,				       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  766  				    assert_class,			       \
064ff292aca500d Daniel Latypov  2022-01-25  767  				    format_func,			       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  768  				    assert_type,			       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  769  				    left,				       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  770  				    op,					       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  771  				    right,				       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  772  				    fmt,				       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  773  				    ...)				       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  774  do {									       \
c2741453478badf Daniel Latypov  2022-01-27  775  	const typeof(left) __left = (left);				       \
c2741453478badf Daniel Latypov  2022-01-27 @776  	const typeof(right) __right = (right);				       \
2b6861e2372bac6 Daniel Latypov  2022-01-25  777  	static const struct kunit_binary_assert_text __text = {		       \
2b6861e2372bac6 Daniel Latypov  2022-01-25  778  		.operation = #op,					       \
2b6861e2372bac6 Daniel Latypov  2022-01-25  779  		.left_text = #left,					       \
2b6861e2372bac6 Daniel Latypov  2022-01-25  780  		.right_text = #right,					       \
2b6861e2372bac6 Daniel Latypov  2022-01-25  781  	};								       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  782  									       \
8bd5d74babc9255 Mickaël Salaün  2024-04-08  783  	_KUNIT_SAVE_LOC(test);						       \
97d453bc4007d4a Daniel Latypov  2022-09-30  784  	if (likely(__left op __right))					       \
97d453bc4007d4a Daniel Latypov  2022-09-30  785  		break;							       \
97d453bc4007d4a Daniel Latypov  2022-09-30  786  									       \
97d453bc4007d4a Daniel Latypov  2022-09-30  787  	_KUNIT_FAILED(test,						       \
21957f90b28f6bc Daniel Latypov  2022-01-13  788  		      assert_type,					       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  789  		      assert_class,					       \
a8495ad8e973cb6 Daniel Latypov  2022-09-30  790  		      format_func,					       \
697365c08679137 Daniel Latypov  2022-09-30  791  		      KUNIT_INIT_ASSERT(.text = &__text,		       \
697365c08679137 Daniel Latypov  2022-09-30  792  					.left_value = __left,		       \
697365c08679137 Daniel Latypov  2022-09-30  793  					.right_value = __right),	       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  794  		      fmt,						       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  795  		      ##__VA_ARGS__);					       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  796  } while (0)
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  797  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 7/7] drm/tests: hdmi: Add max TMDS rate fallback tests for YUV420 mode

[kernel test robot]

Hi Cristian,

kernel test robot noticed the following build errors:

[auto build test ERROR on 4423e607ff50157aaf088854b145936cbab4d560]

url:    https://github.com/intel-lab-lkp/linux/commits/Cristian-Ciocaltea/drm-connector-hdmi-Evaluate-limited-range-after-computing-format/20250311-190150
base:   4423e607ff50157aaf088854b145936cbab4d560
patch link:    https://lore.kernel.org/r/20250311-hdmi-conn-yuv-v2-7-fbdb94f02562%40collabora.com
patch subject: [PATCH v2 7/7] drm/tests: hdmi: Add max TMDS rate fallback tests for YUV420 mode
config: arm64-randconfig-004-20250312 (https://download.01.org/0day-ci/archive/20250313/202503130136.AnTvw0Cj-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250313/202503130136.AnTvw0Cj-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503130136.AnTvw0Cj-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/drm/drm_kunit_helpers.h:10,
                    from drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c:14:
   drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c: In function 'drm_test_check_max_tmds_rate_bpc_fallback_yuv420':
>> include/kunit/test.h:776:29: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
     776 |         const typeof(right) __right = (right);                                 \
         |                             ^~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
>> include/kunit/test.h:776:29: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
     776 |         const typeof(right) __right = (right);                                 \
         |                             ^~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
>> include/kunit/test.h:776:29: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
     776 |         const typeof(right) __right = (right);                                 \
         |                             ^~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
>> include/kunit/test.h:776:29: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
     776 |         const typeof(right) __right = (right);                                 \
         |                             ^~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
>> include/kunit/test.h:776:29: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
     776 |         const typeof(right) __right = (right);                                 \
         |                             ^~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
>> include/kunit/test.h:776:29: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
     776 |         const typeof(right) __right = (right);                                 \
         |                             ^~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
>> include/kunit/test.h:776:29: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
     776 |         const typeof(right) __right = (right);                                 \
         |                             ^~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
   In file included from include/linux/export.h:5,
                    from include/linux/linkage.h:7,
                    from include/linux/preempt.h:10,
                    from include/linux/spinlock.h:56,
                    from include/drm/drm_crtc.h:28,
                    from include/drm/drm_atomic.h:31,
                    from drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c:7:
>> include/linux/compiler.h:32:35: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
      32 |                         ______r = __builtin_expect(!!(x), expect);      \
         |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:44:26: note: in expansion of macro '__branch_check__'
      44 | #  define likely(x)     (__branch_check__(x, 1, __builtin_constant_p(x)))
         |                          ^~~~~~~~~~~~~~~~
   include/kunit/test.h:784:13: note: in expansion of macro 'likely'
     784 |         if (likely(__left op __right))                                         \
         |             ^~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
   include/kunit/test.h:670:35: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
     670 |         const struct assert_class __assertion = INITIALIZER;                   \
         |                                   ^~~~~~~~~~~
   include/kunit/test.h:787:9: note: in expansion of macro '_KUNIT_FAILED'
     787 |         _KUNIT_FAILED(test,                                                    \
         |         ^~~~~~~~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
   include/kunit/test.h:670:35: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
     670 |         const struct assert_class __assertion = INITIALIZER;                   \
         |                                   ^~~~~~~~~~~
   include/kunit/test.h:787:9: note: in expansion of macro '_KUNIT_FAILED'
     787 |         _KUNIT_FAILED(test,                                                    \
         |         ^~~~~~~~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
--
   In file included from include/drm/drm_kunit_helpers.h:10,
                    from drm_hdmi_state_helper_test.c:14:
   drm_hdmi_state_helper_test.c: In function 'drm_test_check_max_tmds_rate_bpc_fallback_yuv420':
>> include/kunit/test.h:776:29: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
     776 |         const typeof(right) __right = (right);                                 \
         |                             ^~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
>> include/kunit/test.h:776:29: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
     776 |         const typeof(right) __right = (right);                                 \
         |                             ^~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
>> include/kunit/test.h:776:29: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
     776 |         const typeof(right) __right = (right);                                 \
         |                             ^~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
>> include/kunit/test.h:776:29: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
     776 |         const typeof(right) __right = (right);                                 \
         |                             ^~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
>> include/kunit/test.h:776:29: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
     776 |         const typeof(right) __right = (right);                                 \
         |                             ^~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
>> include/kunit/test.h:776:29: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
     776 |         const typeof(right) __right = (right);                                 \
         |                             ^~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
>> include/kunit/test.h:776:29: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
     776 |         const typeof(right) __right = (right);                                 \
         |                             ^~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
   In file included from include/linux/export.h:5,
                    from include/linux/linkage.h:7,
                    from include/linux/preempt.h:10,
                    from include/linux/spinlock.h:56,
                    from include/drm/drm_crtc.h:28,
                    from include/drm/drm_atomic.h:31,
                    from drm_hdmi_state_helper_test.c:7:
>> include/linux/compiler.h:32:35: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
      32 |                         ______r = __builtin_expect(!!(x), expect);      \
         |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:44:26: note: in expansion of macro '__branch_check__'
      44 | #  define likely(x)     (__branch_check__(x, 1, __builtin_constant_p(x)))
         |                          ^~~~~~~~~~~~~~~~
   include/kunit/test.h:784:13: note: in expansion of macro 'likely'
     784 |         if (likely(__left op __right))                                         \
         |             ^~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
   include/kunit/test.h:670:35: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
     670 |         const struct assert_class __assertion = INITIALIZER;                   \
         |                                   ^~~~~~~~~~~
   include/kunit/test.h:787:9: note: in expansion of macro '_KUNIT_FAILED'
     787 |         _KUNIT_FAILED(test,                                                    \
         |         ^~~~~~~~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~
   include/kunit/test.h:670:35: error: '-mgeneral-regs-only' is incompatible with the use of floating-point types
     670 |         const struct assert_class __assertion = INITIALIZER;                   \
         |                                   ^~~~~~~~~~~
   include/kunit/test.h:787:9: note: in expansion of macro '_KUNIT_FAILED'
     787 |         _KUNIT_FAILED(test,                                                    \
         |         ^~~~~~~~~~~~~
   include/kunit/test.h:805:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
     805 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:971:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
     971 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/test.h:968:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
     968 |         KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
         |         ^~~~~~~~~~~~~~~~~~~
   drm_hdmi_state_helper_test.c:1337:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
    1337 |         KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate,
         |         ^~~~~~~~~~~~~~~


vim +776 include/kunit/test.h

73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  734  
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  735  #define KUNIT_TRUE_MSG_ASSERTION(test, assert_type, condition, fmt, ...)       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  736  	KUNIT_UNARY_ASSERTION(test,					       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  737  			      assert_type,				       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  738  			      condition,				       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  739  			      true,					       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  740  			      fmt,					       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  741  			      ##__VA_ARGS__)
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  742  
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  743  #define KUNIT_FALSE_MSG_ASSERTION(test, assert_type, condition, fmt, ...)      \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  744  	KUNIT_UNARY_ASSERTION(test,					       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  745  			      assert_type,				       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  746  			      condition,				       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  747  			      false,					       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  748  			      fmt,					       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  749  			      ##__VA_ARGS__)
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  750  
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  751  /*
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  752   * A factory macro for defining the assertions and expectations for the basic
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  753   * comparisons defined for the built in types.
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  754   *
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  755   * Unfortunately, there is no common type that all types can be promoted to for
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  756   * which all the binary operators behave the same way as for the actual types
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  757   * (for example, there is no type that long long and unsigned long long can
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  758   * both be cast to where the comparison result is preserved for all values). So
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  759   * the best we can do is do the comparison in the original types and then coerce
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  760   * everything to long long for printing; this way, the comparison behaves
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  761   * correctly and the printed out value usually makes sense without
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  762   * interpretation, but can always be interpreted to figure out the actual
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  763   * value.
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  764   */
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  765  #define KUNIT_BASE_BINARY_ASSERTION(test,				       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  766  				    assert_class,			       \
064ff292aca500d Daniel Latypov  2022-01-25  767  				    format_func,			       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  768  				    assert_type,			       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  769  				    left,				       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  770  				    op,					       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  771  				    right,				       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  772  				    fmt,				       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  773  				    ...)				       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  774  do {									       \
c2741453478badf Daniel Latypov  2022-01-27  775  	const typeof(left) __left = (left);				       \
c2741453478badf Daniel Latypov  2022-01-27 @776  	const typeof(right) __right = (right);				       \
2b6861e2372bac6 Daniel Latypov  2022-01-25  777  	static const struct kunit_binary_assert_text __text = {		       \
2b6861e2372bac6 Daniel Latypov  2022-01-25  778  		.operation = #op,					       \
2b6861e2372bac6 Daniel Latypov  2022-01-25  779  		.left_text = #left,					       \
2b6861e2372bac6 Daniel Latypov  2022-01-25  780  		.right_text = #right,					       \
2b6861e2372bac6 Daniel Latypov  2022-01-25  781  	};								       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  782  									       \
8bd5d74babc9255 Mickaël Salaün  2024-04-08  783  	_KUNIT_SAVE_LOC(test);						       \
97d453bc4007d4a Daniel Latypov  2022-09-30  784  	if (likely(__left op __right))					       \
97d453bc4007d4a Daniel Latypov  2022-09-30  785  		break;							       \
97d453bc4007d4a Daniel Latypov  2022-09-30  786  									       \
97d453bc4007d4a Daniel Latypov  2022-09-30  787  	_KUNIT_FAILED(test,						       \
21957f90b28f6bc Daniel Latypov  2022-01-13  788  		      assert_type,					       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  789  		      assert_class,					       \
a8495ad8e973cb6 Daniel Latypov  2022-09-30  790  		      format_func,					       \
697365c08679137 Daniel Latypov  2022-09-30  791  		      KUNIT_INIT_ASSERT(.text = &__text,		       \
697365c08679137 Daniel Latypov  2022-09-30  792  					.left_value = __left,		       \
697365c08679137 Daniel Latypov  2022-09-30  793  					.right_value = __right),	       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  794  		      fmt,						       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  795  		      ##__VA_ARGS__);					       \
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  796  } while (0)
73cda7bb8bfb1d4 Brendan Higgins 2019-09-23  797  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 4/7] drm/connector: hdmi: Use YUV420 output format as an RGB fallback

[Maxime Ripard]

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

On Tue, Mar 11, 2025 at 09:46:39PM +0200, Dmitry Baryshkov wrote:
> On Tue, Mar 11, 2025 at 04:55:17PM +0100, Maxime Ripard wrote:
> > Hi,
> > 
> > I think the first thing we need to address is that we will need to
> > differentiate between HDMI 1.4 devices and HDMI 2.0.
> > 
> > It applies to YUV420, which is HDMI 2.0-only, and I guess your patches
> > are good enough if you consider YUV420 support only, but scrambler setup
> > for example is a thing we want to support in that infrastructure
> > eventually, and is conditioned on HDMI 2.0 as well.
> > 
> > On Tue, Mar 11, 2025 at 12:57:36PM +0200, Cristian Ciocaltea wrote:
> > > Try to make use of YUV420 when computing the best output format and
> > > RGB cannot be supported for any of the available color depths.
> > > 
> > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> > > ---
> > >  drivers/gpu/drm/display/drm_hdmi_state_helper.c | 69 +++++++++++++------------
> > >  1 file changed, 35 insertions(+), 34 deletions(-)
> > > 
> 
> [...]
> 
> > >  	return -EINVAL;
> > >  }
> > >  
> > > +static int
> > > +hdmi_compute_config(const struct drm_connector *connector,
> > > +		    struct drm_connector_state *conn_state,
> > > +		    const struct drm_display_mode *mode)
> > > +{
> > > +	unsigned int max_bpc = clamp_t(unsigned int,
> > > +				       conn_state->max_bpc,
> > > +				       8, connector->max_bpc);
> > > +	int ret;
> > > +
> > > +	ret = hdmi_try_format(connector, conn_state, mode, max_bpc,
> > > +			      HDMI_COLORSPACE_RGB);
> > > +	if (!ret)
> > > +		return 0;
> > > +
> > > +	if (connector->ycbcr_420_allowed)
> > > +		ret = hdmi_try_format(connector, conn_state, mode, max_bpc,
> > > +				      HDMI_COLORSPACE_YUV420);
> > 
> > I think that's conditioned on a few more things:
> >   - That the driver supports HDMI 2.0
> 
> Isn't that included into connector->ycbcr_420_allowed? I'd expect that
> HDMI 1.4-only drivers don't set that flag.

Yeah, I guess that's one way to do it, but we don't have any way to
express it at the moment

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH v2 6/7] drm/tests: hdmi: Add limited range tests for YUV420 mode

[Maxime Ripard]

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

On Wed, Mar 12, 2025 at 12:54:51AM +0200, Cristian Ciocaltea wrote:
> On 3/11/25 6:17 PM, Maxime Ripard wrote:
> > On Tue, Mar 11, 2025 at 12:57:38PM +0200, Cristian Ciocaltea wrote:
> >> Provide tests to verify that drm_atomic_helper_connector_hdmi_check()
> >> helper behaviour when using YUV420 output format is to always set the
> >> limited RGB quantization range to 'limited', no matter what the value of
> >> Broadcast RGB property is.
> 
> [...]
> 
> > We need more tests than that to test the various combinations, whether
> > the fallback to YUV420 should work or not depending on the EDID, the
> > driver capabilities, YUV420-only vs YUV420-also, etc.
> 
> Some fallback tests were provided in the next patch, including checks like:
> 
>   KUNIT_ASSERT_TRUE(test, drm_mode_is_420_only(info, yuv420_only_mode));
>   KUNIT_ASSERT_TRUE(test, drm_mode_is_420_also(info, preferred));
> 
> I'll try to further extend the test coverage.

Yeah, sorry, I saw them after reviewing this patch. Still, I think we'll
need more, especially to deal with the fallback cases. IIRC, you were
testing the cases where you're forced to take YUV420 because of the
monitors (or drivers) limit, but we should also try the tests where
you're forced to take YUV420 but the driver implements HDMI 1.4 only for
example, and thus we can't.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH v2 4/7] drm/connector: hdmi: Use YUV420 output format as an RGB fallback

[Maxime Ripard]

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

On Tue, Mar 11, 2025 at 08:59:00PM +0200, Cristian Ciocaltea wrote:
> >> +static int
> >> +hdmi_compute_config(const struct drm_connector *connector,
> >> +		    struct drm_connector_state *conn_state,
> >> +		    const struct drm_display_mode *mode)
> >> +{
> >> +	unsigned int max_bpc = clamp_t(unsigned int,
> >> +				       conn_state->max_bpc,
> >> +				       8, connector->max_bpc);
> >> +	int ret;
> >> +
> >> +	ret = hdmi_try_format(connector, conn_state, mode, max_bpc,
> >> +			      HDMI_COLORSPACE_RGB);
> >> +	if (!ret)
> >> +		return 0;
> >> +
> >> +	if (connector->ycbcr_420_allowed)
> >> +		ret = hdmi_try_format(connector, conn_state, mode, max_bpc,
> >> +				      HDMI_COLORSPACE_YUV420);
> > 
> > I think that's conditioned on a few more things:
> 
> I've actually expected this! :-)
> 
> You've already raised some points during v1, but I preferred to restart the
> discussion on updated code instead - sorry for taking so long to respin the
> series.  In particular, I worked on [1] to improve handling of
> ycbcr_420_allowed flag and fix some consistency issues with
> HDMI_COLORSPACE_YUV420 advertised in drm_bridge->supported_formats.  Hence
> I assumed it's now safe to rely exclusively on this flag to indicate the
> connector is YUV420 capable, without doing any additional checks.
> 
> >   - That the driver supports HDMI 2.0
> 
> Probably I'm missing something obvious here, but is this necessary to
> actually double-check ycbcr_420_allowed has been set correctly?
>
> E.g. for bridges with DRM_BRIDGE_OP_HDMI set in drm_bridge->ops, the
> framework does already adjust ycbcr_420_allowed, hence any additional
> verification would be redundant.  When not making use of the framework,
> drivers are not expected to set the flag if they are not HDMI 2.0 compliant
> or not supporting YUV420, right? Are there any other use cases we need to
> handle?

That's what I answered Dmitry as well, we can definitely make
ycbcr_420_allowed conditional on HDMI 2.0 support being implemented.
We'd have to check that it's properly set through
drmm_connector_hdmi_init tests too I guess.

> >   - That the display is an HDMI output
> 
> I think this should be handled by sink_supports_format_bpc() via:
> 
>     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;
>     }

Yeah, that makes sense.

> >   - That the mode is allowed YUV420 by the sink EDIDs
> 
> And that would be handled via the changes introduced by "drm/connector:
> hdmi: Add support for YUV420 format verification".

ACK

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH v2 4/7] drm/connector: hdmi: Use YUV420 output format as an RGB fallback

[Dmitry Baryshkov]

On Fri, Mar 14, 2025 at 02:47:53PM +0100, Maxime Ripard wrote:
> On Tue, Mar 11, 2025 at 09:46:39PM +0200, Dmitry Baryshkov wrote:
> > On Tue, Mar 11, 2025 at 04:55:17PM +0100, Maxime Ripard wrote:
> > > Hi,
> > > 
> > > I think the first thing we need to address is that we will need to
> > > differentiate between HDMI 1.4 devices and HDMI 2.0.
> > > 
> > > It applies to YUV420, which is HDMI 2.0-only, and I guess your patches
> > > are good enough if you consider YUV420 support only, but scrambler setup
> > > for example is a thing we want to support in that infrastructure
> > > eventually, and is conditioned on HDMI 2.0 as well.
> > > 
> > > On Tue, Mar 11, 2025 at 12:57:36PM +0200, Cristian Ciocaltea wrote:
> > > > Try to make use of YUV420 when computing the best output format and
> > > > RGB cannot be supported for any of the available color depths.
> > > > 
> > > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> > > > ---
> > > >  drivers/gpu/drm/display/drm_hdmi_state_helper.c | 69 +++++++++++++------------
> > > >  1 file changed, 35 insertions(+), 34 deletions(-)
> > > > 
> > 
> > [...]
> > 
> > > >  	return -EINVAL;
> > > >  }
> > > >  
> > > > +static int
> > > > +hdmi_compute_config(const struct drm_connector *connector,
> > > > +		    struct drm_connector_state *conn_state,
> > > > +		    const struct drm_display_mode *mode)
> > > > +{
> > > > +	unsigned int max_bpc = clamp_t(unsigned int,
> > > > +				       conn_state->max_bpc,
> > > > +				       8, connector->max_bpc);
> > > > +	int ret;
> > > > +
> > > > +	ret = hdmi_try_format(connector, conn_state, mode, max_bpc,
> > > > +			      HDMI_COLORSPACE_RGB);
> > > > +	if (!ret)
> > > > +		return 0;
> > > > +
> > > > +	if (connector->ycbcr_420_allowed)
> > > > +		ret = hdmi_try_format(connector, conn_state, mode, max_bpc,
> > > > +				      HDMI_COLORSPACE_YUV420);
> > > 
> > > I think that's conditioned on a few more things:
> > >   - That the driver supports HDMI 2.0
> > 
> > Isn't that included into connector->ycbcr_420_allowed? I'd expect that
> > HDMI 1.4-only drivers don't set that flag.
> 
> Yeah, I guess that's one way to do it, but we don't have any way to
> express it at the moment

Yes, we do not have a way to specify that the connector is HDMI 1.x or
2.0. However I think the code that we currently have ensures that the
flag is set if and only if the HDMI Host and all the chain after it
actually supports YUV 420, which would imply HDMI 2.0.

I know that drm_bridge_connector has one deficiency wrt. YCbCr 420 flag:
it is impossible to "inject" YUV420 in the middle of the chain (e.g. DSI
host outputs RGB data, but then comes DSI2HDMI bridge which can convert
RGB data to YUV). I kept that in mind, but I'd like to see an actual
usecase first. And anyway, this currently limits YUV support rather than
enabing it in the unwanted cases.

-- 
With best wishes
Dmitry

Re: [PATCH v2 5/7] drm/tests: hdmi: Add macros to simplify EDID setup

[Maxime Ripard]

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

On Wed, Mar 12, 2025 at 12:44:26AM +0200, Cristian Ciocaltea wrote:
> On 3/11/25 6:12 PM, Maxime Ripard wrote:
> > On Tue, Mar 11, 2025 at 12:57:37PM +0200, Cristian Ciocaltea wrote:
> >> Introduce a few macros to facilitate setting custom (i.e. non-default)
> >> EDID data during connector initialization.
> >>
> >> This helps reducing boilerplate code while also drops some redundant
> >> calls to set_connector_edid().
> >>
> >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> >> ---
> >>  drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 245 ++++++++-------------
> >>  1 file changed, 93 insertions(+), 152 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> >> index e97efd3af9ed18e6cf8ee66b4923dfc805b34e19..a3f7f3ce31c73335c2c2643bdc5395b6ceb6f071 100644
> >> --- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> >> +++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> >> @@ -183,10 +183,12 @@ static const struct drm_connector_funcs dummy_connector_funcs = {
> >>  
> >>  static
> >>  struct drm_atomic_helper_connector_hdmi_priv *
> >> -drm_kunit_helper_connector_hdmi_init_funcs(struct kunit *test,
> >> -					   unsigned int formats,
> >> -					   unsigned int max_bpc,
> >> -					   const struct drm_connector_hdmi_funcs *hdmi_funcs)
> >> +connector_hdmi_init_funcs_set_edid(struct kunit *test,
> >> +				   unsigned int formats,
> >> +				   unsigned int max_bpc,
> >> +				   const struct drm_connector_hdmi_funcs *hdmi_funcs,
> >> +				   const char *edid_data,
> >> +				   size_t edid_len)
> >>  {
> >>  	struct drm_atomic_helper_connector_hdmi_priv *priv;
> >>  	struct drm_connector *conn;
> >> @@ -240,30 +242,27 @@ drm_kunit_helper_connector_hdmi_init_funcs(struct kunit *test,
> >>  
> >>  	drm_mode_config_reset(drm);
> >>  
> >> +	if (edid_data && edid_len) {
> >> +		ret = set_connector_edid(test, &priv->connector, edid_data, edid_len);
> >> +		KUNIT_ASSERT_GT(test, ret, 0);
> >> +	}
> >> +
> >>  	return priv;
> >>  }
> >>  
> >> -static
> >> -struct drm_atomic_helper_connector_hdmi_priv *
> >> -drm_kunit_helper_connector_hdmi_init(struct kunit *test,
> >> -				     unsigned int formats,
> >> -				     unsigned int max_bpc)
> >> -{
> >> -	struct drm_atomic_helper_connector_hdmi_priv *priv;
> >> -	int ret;
> >> +#define drm_kunit_helper_connector_hdmi_init_funcs_set_edid(test, formats, max_bpc, funcs, edid) \
> >> +	connector_hdmi_init_funcs_set_edid(test, formats, max_bpc, funcs, edid, ARRAY_SIZE(edid))
> >>  
> >> -	priv = drm_kunit_helper_connector_hdmi_init_funcs(test,
> >> -							  formats, max_bpc,
> >> -							  &dummy_connector_hdmi_funcs);
> >> -	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
> >> +#define drm_kunit_helper_connector_hdmi_init_funcs(test, formats, max_bpc, funcs)		\
> >> +	connector_hdmi_init_funcs_set_edid(test, formats, max_bpc, funcs, NULL, 0)
> >>  
> >> -	ret = set_connector_edid(test, &priv->connector,
> >> -				 test_edid_hdmi_1080p_rgb_max_200mhz,
> >> -				 ARRAY_SIZE(test_edid_hdmi_1080p_rgb_max_200mhz));
> >> -	KUNIT_ASSERT_GT(test, ret, 0);
> >> +#define drm_kunit_helper_connector_hdmi_init_set_edid(test, formats, max_bpc, edid)		\
> >> +	drm_kunit_helper_connector_hdmi_init_funcs_set_edid(test, formats, max_bpc,		\
> >> +							    &dummy_connector_hdmi_funcs, edid)
> >>  
> >> -	return priv;
> >> -}
> >> +#define drm_kunit_helper_connector_hdmi_init(test, formats, max_bpc)				\
> >> +	drm_kunit_helper_connector_hdmi_init_set_edid(test, formats, max_bpc,			\
> >> +						      test_edid_hdmi_1080p_rgb_max_200mhz)
> > 
> > I'd really prefer to have functions to macros here. They are easier to
> > read, extend, and don't have any particular drawbacks.
> 
> Yeah, the main reason I opted for macros was to allow dropping
> ARRAY_SIZE(edid) from the caller side, hence making the API as simple as
> possible.
> 
> > I also don't think we need that many, looking at the tests:
> > 
> >   - We need drm_kunit_helper_connector_hdmi_init() to setup a connector
> >     with test_edid_hdmi_1080p_rgb_max_200mhz and
> >     dummy_connector_hdmi_funcs()
> 
> Correct.
> 
> >   - We need to create a
> >     drm_kunit_helper_connector_hdmi_init_with_edid_funcs() to pass both
> >     the funcs and edid pointers
> 
> That's drm_kunit_helper_connector_hdmi_init_funcs_set_edid(), but I can
> rename it if you prefer - I've just tried to keep the name length under
> control, as there are already some indentation challenges when calling
> the helpers.
> 
> Currently it's only used to implement
> drm_kunit_helper_connector_hdmi_init_set_edid() by passing
> &dummy_connector_hdmi_funcs, but there are a few test cases that can
> benefit of it and help extend the cleanup - will do for v3.
> 
> drm_kunit_helper_connector_hdmi_init_set_edid() should also stay as it's
> already being used to drop boilerplate code from a lot of places.
> 
> > And that's it, right?
> 
> There's also drm_kunit_helper_connector_hdmi_init_funcs() which has been
> used for a few times, but it can be further optimized out via
> drm_kunit_helper_connector_hdmi_init_funcs_set_edid(), so I'll drop it.
> 
> That means we could end up with just the following:
> 
> - drm_kunit_helper_connector_hdmi_init()
> - drm_kunit_helper_connector_hdmi_init_set_edid()
> - drm_kunit_helper_connector_hdmi_init_funcs_set_edid()

I'm not even sure we need init_set_edid. We just have to provide the
dummy_connector_hdmi_funcs pointer to
drm_kunit_helper_connector_hdmi_init_funcs_set_edid and that's it.

And yeah, I'd really prefer to use
drm_kunit_helper_connector_hdmi_init_with_edid_funcs() to remain
consistent with the rest of KMS.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH v2 5/7] drm/tests: hdmi: Add macros to simplify EDID setup

[Cristian Ciocaltea]

On 3/19/25 5:32 PM, Maxime Ripard wrote:
> On Wed, Mar 12, 2025 at 12:44:26AM +0200, Cristian Ciocaltea wrote:
>> On 3/11/25 6:12 PM, Maxime Ripard wrote:
>>> On Tue, Mar 11, 2025 at 12:57:37PM +0200, Cristian Ciocaltea wrote:
>>>> Introduce a few macros to facilitate setting custom (i.e. non-default)
>>>> EDID data during connector initialization.
>>>>
>>>> This helps reducing boilerplate code while also drops some redundant
>>>> calls to set_connector_edid().
>>>>
>>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>>> ---
>>>>  drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 245 ++++++++-------------
>>>>  1 file changed, 93 insertions(+), 152 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
>>>> index e97efd3af9ed18e6cf8ee66b4923dfc805b34e19..a3f7f3ce31c73335c2c2643bdc5395b6ceb6f071 100644
>>>> --- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
>>>> +++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
>>>> @@ -183,10 +183,12 @@ static const struct drm_connector_funcs dummy_connector_funcs = {
>>>>  
>>>>  static
>>>>  struct drm_atomic_helper_connector_hdmi_priv *
>>>> -drm_kunit_helper_connector_hdmi_init_funcs(struct kunit *test,
>>>> -					   unsigned int formats,
>>>> -					   unsigned int max_bpc,
>>>> -					   const struct drm_connector_hdmi_funcs *hdmi_funcs)
>>>> +connector_hdmi_init_funcs_set_edid(struct kunit *test,
>>>> +				   unsigned int formats,
>>>> +				   unsigned int max_bpc,
>>>> +				   const struct drm_connector_hdmi_funcs *hdmi_funcs,
>>>> +				   const char *edid_data,
>>>> +				   size_t edid_len)
>>>>  {
>>>>  	struct drm_atomic_helper_connector_hdmi_priv *priv;
>>>>  	struct drm_connector *conn;
>>>> @@ -240,30 +242,27 @@ drm_kunit_helper_connector_hdmi_init_funcs(struct kunit *test,
>>>>  
>>>>  	drm_mode_config_reset(drm);
>>>>  
>>>> +	if (edid_data && edid_len) {
>>>> +		ret = set_connector_edid(test, &priv->connector, edid_data, edid_len);
>>>> +		KUNIT_ASSERT_GT(test, ret, 0);
>>>> +	}
>>>> +
>>>>  	return priv;
>>>>  }
>>>>  
>>>> -static
>>>> -struct drm_atomic_helper_connector_hdmi_priv *
>>>> -drm_kunit_helper_connector_hdmi_init(struct kunit *test,
>>>> -				     unsigned int formats,
>>>> -				     unsigned int max_bpc)
>>>> -{
>>>> -	struct drm_atomic_helper_connector_hdmi_priv *priv;
>>>> -	int ret;
>>>> +#define drm_kunit_helper_connector_hdmi_init_funcs_set_edid(test, formats, max_bpc, funcs, edid) \
>>>> +	connector_hdmi_init_funcs_set_edid(test, formats, max_bpc, funcs, edid, ARRAY_SIZE(edid))
>>>>  
>>>> -	priv = drm_kunit_helper_connector_hdmi_init_funcs(test,
>>>> -							  formats, max_bpc,
>>>> -							  &dummy_connector_hdmi_funcs);
>>>> -	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
>>>> +#define drm_kunit_helper_connector_hdmi_init_funcs(test, formats, max_bpc, funcs)		\
>>>> +	connector_hdmi_init_funcs_set_edid(test, formats, max_bpc, funcs, NULL, 0)
>>>>  
>>>> -	ret = set_connector_edid(test, &priv->connector,
>>>> -				 test_edid_hdmi_1080p_rgb_max_200mhz,
>>>> -				 ARRAY_SIZE(test_edid_hdmi_1080p_rgb_max_200mhz));
>>>> -	KUNIT_ASSERT_GT(test, ret, 0);
>>>> +#define drm_kunit_helper_connector_hdmi_init_set_edid(test, formats, max_bpc, edid)		\
>>>> +	drm_kunit_helper_connector_hdmi_init_funcs_set_edid(test, formats, max_bpc,		\
>>>> +							    &dummy_connector_hdmi_funcs, edid)
>>>>  
>>>> -	return priv;
>>>> -}
>>>> +#define drm_kunit_helper_connector_hdmi_init(test, formats, max_bpc)				\
>>>> +	drm_kunit_helper_connector_hdmi_init_set_edid(test, formats, max_bpc,			\
>>>> +						      test_edid_hdmi_1080p_rgb_max_200mhz)
>>>
>>> I'd really prefer to have functions to macros here. They are easier to
>>> read, extend, and don't have any particular drawbacks.
>>
>> Yeah, the main reason I opted for macros was to allow dropping
>> ARRAY_SIZE(edid) from the caller side, hence making the API as simple as
>> possible.
>>
>>> I also don't think we need that many, looking at the tests:
>>>
>>>   - We need drm_kunit_helper_connector_hdmi_init() to setup a connector
>>>     with test_edid_hdmi_1080p_rgb_max_200mhz and
>>>     dummy_connector_hdmi_funcs()
>>
>> Correct.
>>
>>>   - We need to create a
>>>     drm_kunit_helper_connector_hdmi_init_with_edid_funcs() to pass both
>>>     the funcs and edid pointers
>>
>> That's drm_kunit_helper_connector_hdmi_init_funcs_set_edid(), but I can
>> rename it if you prefer - I've just tried to keep the name length under
>> control, as there are already some indentation challenges when calling
>> the helpers.
>>
>> Currently it's only used to implement
>> drm_kunit_helper_connector_hdmi_init_set_edid() by passing
>> &dummy_connector_hdmi_funcs, but there are a few test cases that can
>> benefit of it and help extend the cleanup - will do for v3.
>>
>> drm_kunit_helper_connector_hdmi_init_set_edid() should also stay as it's
>> already being used to drop boilerplate code from a lot of places.
>>
>>> And that's it, right?
>>
>> There's also drm_kunit_helper_connector_hdmi_init_funcs() which has been
>> used for a few times, but it can be further optimized out via
>> drm_kunit_helper_connector_hdmi_init_funcs_set_edid(), so I'll drop it.
>>
>> That means we could end up with just the following:
>>
>> - drm_kunit_helper_connector_hdmi_init()
>> - drm_kunit_helper_connector_hdmi_init_set_edid()
>> - drm_kunit_helper_connector_hdmi_init_funcs_set_edid()
> 
> I'm not even sure we need init_set_edid. We just have to provide the
> dummy_connector_hdmi_funcs pointer to
> drm_kunit_helper_connector_hdmi_init_funcs_set_edid and that's it.

Right, it can be done, but my point is that there are currently ~20 test
cases that benefit from this simplification, and the number is likely to
grow while extending the test suite.

I would rename it to drm_kunit_helper_connector_hdmi_init_with_edid(),
to be consistent with your preference below, but yeah, if you're still
not convinced we should keep the helper, pls let me know and I'll drop it.

> And yeah, I'd really prefer to use
> drm_kunit_helper_connector_hdmi_init_with_edid_funcs() to remain
> consistent with the rest of KMS.

Sure, will do.

Thanks,
Cristian