diff mbox series

[v9,53/61] drm/tests: connector: Add HDMI max_tmds_char_rate init coverage

Message ID 20260723-dw-hdmi-qp-scramb-v9-53-4fb12ea22ac9@collabora.com (mailing list archive)
State New
Headers
Series Add HDMI 2.0 support to DW HDMI QP TX |

Commit Message

Cristian Ciocaltea July 23, 2026, 1:35 a.m. UTC 
Add KUnit tests for the new drmm_connector_hdmi_init() extension, which
infers the connector's max_tmds_char_rate from the HDMI specification
version advertised by the driver.  Also validate a driver-capped
max_tmds_char_rate against the version-inferred limit: override below,
accept at, reject above, reject when no version is advertised.

Since HDMI_VERSION_2_0 requires scrambler callbacks to be provided, add
a dummy_hdmi_funcs_scrambler instance for tests that initialize HDMI 2.0
connectors.

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

Comments

sashiko-bot@kernel.org July 23, 2026, 1:58 a.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Flawed KUnit test logic causes TMDS character rate limit rejection tests to pass for the wrong reason.
- [Medium] Dangling pointer created by storing the address of a stack-allocated structure into a persistent device-managed object.
--

--- Patch [53]: [PATCH v9 53/61] drm/tests: connector: Add HDMI max_tmds_char_rate init coverage ---
commit 7359a144a698a961aa27ac0fd6a0ec01ec7928f5
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/tests: connector: Add HDMI max_tmds_char_rate init coverage

This commit adds KUnit tests for the new drmm_connector_hdmi_init()
extension to infer and validate the connector's max_tmds_char_rate based
on the advertised HDMI specification version.

> diff --git a/drivers/gpu/drm/tests/drm_connector_test.c b/drivers/gpu/drm/tests/drm_connector_test.c
> index 74342880b3015..f14cd741b05e4 100644
> --- a/drivers/gpu/drm/tests/drm_connector_test.c
> +++ b/drivers/gpu/drm/tests/drm_connector_test.c

[ ... ]

> @@ -1236,6 +1264,154 @@ KUNIT_ARRAY_PARAM(drm_connector_hdmi_init_type_invalid,
> +static void drm_test_connector_hdmi_init_max_tmds_rate_inferred(struct kunit *test)
> +{
> +	struct drm_connector_init_priv *priv = test->priv;
> +	const struct drm_connector_hdmi_init_max_tmds_rate_inferred_case *params =
> +		test->param_value;
> +	struct drm_connector_hdmi_funcs hdmi_funcs;
> +	int ret;
> +
> +	hdmi_funcs = params->ver < HDMI_VERSION_2_0 ?
> +			dummy_hdmi_funcs : dummy_hdmi_funcs_scrambler;
> +	hdmi_funcs.supported_hdmi_ver = params->ver;
> +
> +	ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
> +				       &dummy_funcs,
> +				       &hdmi_funcs,
> +				       DRM_MODE_CONNECTOR_HDMIA,
> +				       &priv->ddc);

[Severity: Medium]
Does this create a dangling pointer?

drmm_connector_hdmi_init() stores the provided hdmi_funcs pointer in
connector->hdmi.funcs.

Since hdmi_funcs is allocated on the stack here (and in other new tests),
connector->hdmi.funcs becomes a dangling pointer when the test function
returns. Even if deferred device-managed cleanup doesn't dereference it
today, is it safe to leave this pointing to destroyed stack frames?

[ ... ]

> +/*
> + * Test that the registration of an HDMI connector providing a max TMDS
> + * character rate that exceeds the limit inferred from the advertised HDMI
> + * specification version fails.
> + */
> +static void drm_test_connector_hdmi_init_max_tmds_rate_exceeds(struct kunit *test)
> +{
> +	struct drm_connector_init_priv *priv = test->priv;
> +	struct drm_connector_hdmi_funcs hdmi_funcs = dummy_hdmi_funcs_scrambler;
> +	int ret;
> +
> +	hdmi_funcs.supported_hdmi_ver = HDMI_VERSION_1_4;
> +	hdmi_funcs.supported_tmds_char_rate = HDMI_2_0_TMDS_CHAR_RATE_MAX_HZ;
> +
> +	ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
> +				       &dummy_funcs,
> +				       &hdmi_funcs,
> +				       DRM_MODE_CONNECTOR_HDMIA,
> +				       &priv->ddc);
> +	KUNIT_EXPECT_LT(test, ret, 0);
> +}

[Severity: Medium]
Are these tests passing for the wrong reason?

Because dummy_hdmi_funcs_scrambler provides scrambler callbacks, setting
hdmi_funcs.supported_hdmi_ver = HDMI_VERSION_1_4 causes
drmm_connector_hdmi_init() to return -EINVAL early:

	if (hdmi_funcs->scrambler_enable || hdmi_funcs->scrambler_disable) {
		drm_err(dev, "Scrambler callbacks unexpected for HDMI 1.x\n");
		return -EINVAL;
	}

This bypasses the TMDS rate logic completely, meaning the expectation
KUNIT_EXPECT_LT(test, ret, 0) succeeds without actually testing the intended
rate limit logic.

[ ... ]

> +/*
> + * Test that the registration of an HDMI connector providing a non-zero max
> + * TMDS character rate without an HDMI specification version fails, as the
> + * version-inferred limit defaults to zero and any positive override would
> + * exceed it.
> + */
> +static void drm_test_connector_hdmi_init_max_tmds_rate_no_version(struct kunit *test)
> +{
> +	struct drm_connector_init_priv *priv = test->priv;
> +	struct drm_connector_hdmi_funcs hdmi_funcs = dummy_hdmi_funcs_scrambler;
> +	int ret;
> +
> +	hdmi_funcs.supported_hdmi_ver = HDMI_VERSION_UNKNOWN;
> +	hdmi_funcs.supported_tmds_char_rate = HDMI_1_0_TMDS_CHAR_RATE_MAX_HZ;
> +
> +	ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
> +				       &dummy_funcs,
> +				       &hdmi_funcs,
> +				       DRM_MODE_CONNECTOR_HDMIA,
> +				       &priv->ddc);
> +	KUNIT_EXPECT_LT(test, ret, 0);
> +}

[Severity: Medium]
Does this test have the exact same logic flaw as above? 

It uses dummy_hdmi_funcs_scrambler with HDMI_VERSION_UNKNOWN, causing early
rejection due to the scrambler checks rather than testing the TMDS rate
validation.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/tests/drm_connector_test.c b/drivers/gpu/drm/tests/drm_connector_test.c
index 74342880b301..f14cd741b05e 100644
--- a/drivers/gpu/drm/tests/drm_connector_test.c
+++ b/drivers/gpu/drm/tests/drm_connector_test.c
@@ -52,6 +52,34 @@  static const struct drm_connector_hdmi_funcs dummy_hdmi_funcs = {
 	},
 };
 
+static int accept_scrambler_enable(struct drm_connector *connector)
+{
+	return 0;
+}
+
+static int accept_scrambler_disable(struct drm_connector *connector)
+{
+	return 0;
+}
+
+static const struct drm_connector_hdmi_funcs dummy_hdmi_funcs_scrambler = {
+	.vendor = "Vendor",
+	.product = "Product",
+	.supported_hdmi_ver = HDMI_VERSION_2_0,
+	.supported_formats = BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444),
+	.max_bpc = 8,
+	.scrambler_enable = accept_scrambler_enable,
+	.scrambler_disable = accept_scrambler_disable,
+	.avi = {
+		.clear_infoframe = accept_infoframe_clear_infoframe,
+		.write_infoframe = accept_infoframe_write_infoframe,
+	},
+	.hdmi = {
+		.clear_infoframe = accept_infoframe_clear_infoframe,
+		.write_infoframe = accept_infoframe_write_infoframe,
+	},
+};
+
 static const struct drm_connector_funcs dummy_funcs = {
 	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
 	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
@@ -1236,6 +1264,154 @@  KUNIT_ARRAY_PARAM(drm_connector_hdmi_init_type_invalid,
 		  drm_connector_hdmi_init_type_invalid_tests,
 		  drm_connector_hdmi_init_type_desc);
 
+/*
+ * Test that the registration of an HDMI connector without an explicit max TMDS
+ * character rate being provided succeeds, and the connector limit is inferred
+ * from the advertised HDMI specification version.
+ */
+struct drm_connector_hdmi_init_max_tmds_rate_inferred_case {
+	const char *desc;
+	enum hdmi_version ver;
+	unsigned long long expected;
+};
+
+static void drm_test_connector_hdmi_init_max_tmds_rate_inferred(struct kunit *test)
+{
+	struct drm_connector_init_priv *priv = test->priv;
+	const struct drm_connector_hdmi_init_max_tmds_rate_inferred_case *params =
+		test->param_value;
+	struct drm_connector_hdmi_funcs hdmi_funcs;
+	int ret;
+
+	hdmi_funcs = params->ver < HDMI_VERSION_2_0 ?
+			dummy_hdmi_funcs : dummy_hdmi_funcs_scrambler;
+	hdmi_funcs.supported_hdmi_ver = params->ver;
+
+	ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
+				       &dummy_funcs,
+				       &hdmi_funcs,
+				       DRM_MODE_CONNECTOR_HDMIA,
+				       &priv->ddc);
+	KUNIT_EXPECT_EQ(test, ret, 0);
+	KUNIT_EXPECT_EQ(test, priv->connector.hdmi.max_tmds_char_rate,
+			params->expected);
+}
+
+static const struct drm_connector_hdmi_init_max_tmds_rate_inferred_case
+drm_connector_hdmi_init_max_tmds_rate_inferred_tests[] = {
+	{ "unknown",	HDMI_VERSION_UNKNOWN,	0 },
+	{ "1.0",	HDMI_VERSION_1_0,	HDMI_1_0_TMDS_CHAR_RATE_MAX_HZ },
+	{ "1.2",	HDMI_VERSION_1_2,	HDMI_1_0_TMDS_CHAR_RATE_MAX_HZ },
+	{ "1.3",	HDMI_VERSION_1_3,	HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ },
+	{ "1.4",	HDMI_VERSION_1_4,	HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ },
+	{ "2.0",	HDMI_VERSION_2_0,	HDMI_2_0_TMDS_CHAR_RATE_MAX_HZ },
+};
+
+static void drm_connector_hdmi_init_max_tmds_rate_inferred_desc(
+	const struct drm_connector_hdmi_init_max_tmds_rate_inferred_case *t,
+	char *desc)
+{
+	strscpy(desc, t->desc, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(drm_connector_hdmi_init_max_tmds_rate_inferred,
+		  drm_connector_hdmi_init_max_tmds_rate_inferred_tests,
+		  drm_connector_hdmi_init_max_tmds_rate_inferred_desc);
+
+/*
+ * Test that the registration of an HDMI connector providing a max TMDS
+ * character rate strictly below the version-inferred limit succeeds, and
+ * the connector limit is overridden.
+ */
+static void drm_test_connector_hdmi_init_max_tmds_rate_override(struct kunit *test)
+{
+	struct drm_connector_init_priv *priv = test->priv;
+	struct drm_connector_hdmi_funcs hdmi_funcs = dummy_hdmi_funcs_scrambler;
+	int ret;
+
+	KUNIT_ASSERT_EQ(test, hdmi_funcs.supported_hdmi_ver, HDMI_VERSION_2_0);
+
+	hdmi_funcs.supported_tmds_char_rate = HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ;
+
+	ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
+				       &dummy_funcs,
+				       &hdmi_funcs,
+				       DRM_MODE_CONNECTOR_HDMIA,
+				       &priv->ddc);
+	KUNIT_EXPECT_EQ(test, ret, 0);
+	KUNIT_EXPECT_EQ(test, priv->connector.hdmi.max_tmds_char_rate,
+			HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ);
+}
+
+/*
+ * Test that the registration of an HDMI connector providing a max TMDS
+ * character rate equal to the version-inferred limit succeeds.
+ */
+static void drm_test_connector_hdmi_init_max_tmds_rate_at_limit(struct kunit *test)
+{
+	struct drm_connector_init_priv *priv = test->priv;
+	struct drm_connector_hdmi_funcs hdmi_funcs = dummy_hdmi_funcs_scrambler;
+	int ret;
+
+	KUNIT_ASSERT_EQ(test, hdmi_funcs.supported_hdmi_ver, HDMI_VERSION_2_0);
+
+	hdmi_funcs.supported_tmds_char_rate = HDMI_2_0_TMDS_CHAR_RATE_MAX_HZ;
+
+	ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
+				       &dummy_funcs,
+				       &hdmi_funcs,
+				       DRM_MODE_CONNECTOR_HDMIA,
+				       &priv->ddc);
+	KUNIT_EXPECT_EQ(test, ret, 0);
+	KUNIT_EXPECT_EQ(test, priv->connector.hdmi.max_tmds_char_rate,
+			HDMI_2_0_TMDS_CHAR_RATE_MAX_HZ);
+}
+
+/*
+ * Test that the registration of an HDMI connector providing a max TMDS
+ * character rate that exceeds the limit inferred from the advertised HDMI
+ * specification version fails.
+ */
+static void drm_test_connector_hdmi_init_max_tmds_rate_exceeds(struct kunit *test)
+{
+	struct drm_connector_init_priv *priv = test->priv;
+	struct drm_connector_hdmi_funcs hdmi_funcs = dummy_hdmi_funcs_scrambler;
+	int ret;
+
+	hdmi_funcs.supported_hdmi_ver = HDMI_VERSION_1_4;
+	hdmi_funcs.supported_tmds_char_rate = HDMI_2_0_TMDS_CHAR_RATE_MAX_HZ;
+
+	ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
+				       &dummy_funcs,
+				       &hdmi_funcs,
+				       DRM_MODE_CONNECTOR_HDMIA,
+				       &priv->ddc);
+	KUNIT_EXPECT_LT(test, ret, 0);
+}
+
+/*
+ * Test that the registration of an HDMI connector providing a non-zero max
+ * TMDS character rate without an HDMI specification version fails, as the
+ * version-inferred limit defaults to zero and any positive override would
+ * exceed it.
+ */
+static void drm_test_connector_hdmi_init_max_tmds_rate_no_version(struct kunit *test)
+{
+	struct drm_connector_init_priv *priv = test->priv;
+	struct drm_connector_hdmi_funcs hdmi_funcs = dummy_hdmi_funcs_scrambler;
+	int ret;
+
+	hdmi_funcs.supported_hdmi_ver = HDMI_VERSION_UNKNOWN;
+	hdmi_funcs.supported_tmds_char_rate = HDMI_1_0_TMDS_CHAR_RATE_MAX_HZ;
+
+	ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
+				       &dummy_funcs,
+				       &hdmi_funcs,
+				       DRM_MODE_CONNECTOR_HDMIA,
+				       &priv->ddc);
+	KUNIT_EXPECT_LT(test, ret, 0);
+}
+
 static struct kunit_case drmm_connector_hdmi_init_tests[] = {
 	KUNIT_CASE(drm_test_connector_hdmi_init_valid),
 	KUNIT_CASE(drm_test_connector_hdmi_init_bpc_8),
@@ -1260,6 +1436,12 @@  static struct kunit_case drmm_connector_hdmi_init_tests[] = {
 			 drm_connector_hdmi_init_type_valid_gen_params),
 	KUNIT_CASE_PARAM(drm_test_connector_hdmi_init_type_invalid,
 			 drm_connector_hdmi_init_type_invalid_gen_params),
+	KUNIT_CASE_PARAM(drm_test_connector_hdmi_init_max_tmds_rate_inferred,
+			 drm_connector_hdmi_init_max_tmds_rate_inferred_gen_params),
+	KUNIT_CASE(drm_test_connector_hdmi_init_max_tmds_rate_override),
+	KUNIT_CASE(drm_test_connector_hdmi_init_max_tmds_rate_at_limit),
+	KUNIT_CASE(drm_test_connector_hdmi_init_max_tmds_rate_exceeds),
+	KUNIT_CASE(drm_test_connector_hdmi_init_max_tmds_rate_no_version),
 	{ }
 };