Re: [v14,06/28] drm/tests: Add output bpc tests

From: Sui Jingfeng
Date: Thu May 23 2024 - 13:01:12 EST

Hi, Maxime


I love you patch, yet it generates warnning calltrace. Despite it's
just a warning but it can overwhelm when we run kunit tests. Hence,
I suggest switch to the drm_atomic_connector_get_property() function.

Logs are pasted as below for easier to ready.


------------[ cut here ]------------
WARNING: CPU: 3 PID: 1264 at drivers/gpu/drm/drm_mode_object.c:354 drm_object_property_get_value+0x2c/0x34
Modules linked in: drm_connector_test drm_display_helper drm_kunit_helpers kunit rfkill ip_set nf_tables nfnetlink vfat fat uas usb_storage kvm efi_pstore pstore spi_loongson_pci spi_loongson_core fuse efivarfs [last unloaded: drm_connector_test]
CPU: 3 PID: 1264 Comm: kunit_try_catch Tainted: G N 6.9.0+ #443
Hardware name: Loongson Loongson-3A6000-HV-7A2000-XA61200/Loongson-3A6000-HV-7A2000-XA61200, BIOS Loongson-UDK2018-V4.0.05636-stable202311 12/
pc 9000000003469fec ra ffff80000225afdc tp 900000011fc54000 sp 900000011fc57d80
a0 900000010aa84658 a1 9000000104432a00 a2 900000011fc57d98 a3 900000011fc57d98
a4 9000000104432a4c a5 9000000003f14e98 a6 0000000000000008 a7 fffffffffffffffe
t0 0000000000000010 t1 900000010aa84000 t2 ffffffffffffffff t3 ffffffffc0c0c0c0
t4 ffffffffc0c0c0c0 t5 0000000000000220 t6 0000000000000001 t7 0000000000107203
t8 0000000000107303 u0 0000000000000008 s9 90000001000ebe60 s0 900000010aa84000
s1 90000001470679c8 s2 9000000104432a00 s3 ffff800002284000 s4 900000010aa84658
s5 900000010aa84618 s6 0000000000001000 s7 0000000000000001 s8 0000000000000000
ra: ffff80000225afdc drm_test_connector_hdmi_init_bpc_8+0xcc/0x2d0 [drm_connector_test]
ERA: 9000000003469fec drm_object_property_get_value+0x2c/0x34
CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
PRMD: 00000004 (PPLV0 +PIE -PWE)
EUEN: 00000000 (-FPE -SXE -ASXE -BTE)
ECFG: 00071c1c (LIE=2-4,10-12 VS=7)
ESTAT: 000c0000 [BRK] (IS= ECode=12 EsubCode=0)
PRID: 0014d000 (Loongson-64bit, Loongson-3A6000-HV)
CPU: 3 PID: 1264 Comm: kunit_try_catch Tainted: G N 6.9.0+ #443
Hardware name: Loongson Loongson-3A6000-HV-7A2000-XA61200/Loongson-3A6000-HV-7A2000-XA61200, BIOS Loongson-UDK2018-V4.0.05636-stable202311 12/
Stack : 9000000004065000 0000000000000000 9000000002ac339c 900000011fc54000
900000011fc579f0 900000011fc579f8 0000000000000000 900000011fc57b38
900000011fc57b30 900000011fc57b30 900000011fc57940 0000000000000001
0000000000000001 900000011fc579f8 18e7bf3ffb6e59df 9000000100328a00
0000000000000001 0000000000000003 0000000000000434 4c206e6f73676e6f
6f4c203a656d616e 00000000000d0ad3 000000000704c000 90000001000ebe60
0000000000000000 0000000000000000 9000000003ee6ab0 9000000004065000
0000000000000000 900000010aa84618 0000000000001000 0000000000000001
0000000000000000 0000000000000000 9000000002ac33b4 000055557dd80078
00000000000000b0 0000000000000004 0000000000000000 0000000000071c1c
...
Call Trace:
[<9000000002ac33b4>] show_stack+0x5c/0x180
[<9000000003b1ed2c>] dump_stack_lvl+0x70/0xa0
[<9000000003b01fd8>] __warn+0x84/0xc8
[<9000000003ad282c>] report_bug+0x19c/0x204
[<9000000003b1fe00>] do_bp+0x264/0x2b4
[<0000000000000000>] 0x0
[<9000000003469fec>] drm_object_property_get_value+0x2c/0x34
[<ffff80000225afdc>] drm_test_connector_hdmi_init_bpc_8+0xcc/0x2d0 [drm_connector_test]
[<ffff800002214f3c>] kunit_try_run_case+0x7c/0x18c [kunit]
[<ffff800002216de8>] kunit_generic_run_threadfn_adapter+0x1c/0x28 [kunit]
[<9000000002b06238>] kthread+0x124/0x130
[<9000000002ac1248>] ret_from_kernel_thread+0xc/0xa4

---[ end trace 0000000000000000 ]---
------------[ cut here ]------------


On 5/21/24 18:13, Maxime Ripard wrote:
Now that we're tracking the output bpc count in the connector state,
let's add a few tests to make sure it works as expected.

Reviewed-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
Signed-off-by: Maxime Ripard <mripard@xxxxxxxxxx>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
drivers/gpu/drm/Kconfig | 1 +
drivers/gpu/drm/tests/Makefile | 1 +
drivers/gpu/drm/tests/drm_connector_test.c | 140 +++++++
drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 438 +++++++++++++++++++++
drivers/gpu/drm/tests/drm_kunit_edid.h | 106 +++++
5 files changed, 686 insertions(+)


[...]

+
+/*
+ * Test that the registration of a connector with a maximum bpc count of
+ * 8 succeeds, registers the max bpc property, but doesn't register the
+ * HDR output metadata one.
+ */
+static void drm_test_connector_hdmi_init_bpc_8(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ struct drm_connector *connector = &priv->connector;
+ struct drm_property *prop;
+ uint64_t val;
+ int ret;
+
+ ret = drmm_connector_hdmi_init(&priv->drm, connector,
+ &dummy_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ &priv->ddc,
+ 8);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ prop = connector->max_bpc_property;
+ KUNIT_ASSERT_NOT_NULL(test, prop);
+ KUNIT_EXPECT_NOT_NULL(test, drm_mode_obj_find_prop_id(&connector->base, prop->base.id));
+
+ ret = drm_object_property_get_value(&connector->base, prop, &val);


Maybe we should switch to drm_atomic_connector_get_property() instead,
as the comments of the drm_object_property_get_value() told us that
atomic drivers should never call this function directly, otherwise it
will print warnings and call trace.

+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, val, 8);
+
+ prop = priv->drm.mode_config.hdr_output_metadata_property;
+ KUNIT_ASSERT_NOT_NULL(test, prop);
+ KUNIT_EXPECT_NULL(test, drm_mode_obj_find_prop_id(&connector->base, prop->base.id));
+}
+
+/*
+ * Test that the registration of a connector with a maximum bpc count of
+ * 10 succeeds and registers the max bpc and HDR output metadata
+ * properties.
+ */
+static void drm_test_connector_hdmi_init_bpc_10(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ struct drm_connector *connector = &priv->connector;
+ struct drm_property *prop;
+ uint64_t val;
+ int ret;
+
+ ret = drmm_connector_hdmi_init(&priv->drm, connector,
+ &dummy_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ &priv->ddc,
+ 10);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ prop = connector->max_bpc_property;
+ KUNIT_ASSERT_NOT_NULL(test, prop);
+ KUNIT_EXPECT_NOT_NULL(test, drm_mode_obj_find_prop_id(&connector->base, prop->base.id));
+
+ ret = drm_object_property_get_value(&connector->base, prop, &val);

Ditto

+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, val, 10);
+
+ prop = priv->drm.mode_config.hdr_output_metadata_property;
+ KUNIT_ASSERT_NOT_NULL(test, prop);
+ KUNIT_EXPECT_NOT_NULL(test, drm_mode_obj_find_prop_id(&connector->base, prop->base.id));
+}
+
+/*
+ * Test that the registration of a connector with a maximum bpc count of
+ * 12 succeeds and registers the max bpc and HDR output metadata
+ * properties.
+ */
+static void drm_test_connector_hdmi_init_bpc_12(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ struct drm_connector *connector = &priv->connector;
+ struct drm_property *prop;
+ uint64_t val;
+ int ret;
+
+ ret = drmm_connector_hdmi_init(&priv->drm, connector,
+ &dummy_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ &priv->ddc,
+ 12);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ prop = connector->max_bpc_property;
+ KUNIT_ASSERT_NOT_NULL(test, prop);
+ KUNIT_EXPECT_NOT_NULL(test, drm_mode_obj_find_prop_id(&connector->base, prop->base.id));
+
+ ret = drm_object_property_get_value(&connector->base, prop, &val);

ret = drm_atomic_connector_get_property(connector,
connector->state, prop, &val);

Note that this function is not exported, but I think you could export it
just like what you did in the patch 02. Thank you for the amazing works.

+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, val, 12);
+
+ prop = priv->drm.mode_config.hdr_output_metadata_property;
+ KUNIT_ASSERT_NOT_NULL(test, prop);
+ KUNIT_EXPECT_NOT_NULL(test, drm_mode_obj_find_prop_id(&connector->base, prop->base.id));
+}
+

--
Best regards
Sui