* [PATCH v2 1/4] drm/tests: hdmi: Fix WW_MUTEX_SLOWPATH failures
2025-01-29 14:21 [PATCH v2 0/4] drm/tests: Fix locking issues (kind of) Maxime Ripard
@ 2025-01-29 14:21 ` Maxime Ripard
2025-02-04 11:01 ` Simona Vetter
2025-01-29 14:21 ` [PATCH v2 2/4] drm/tests: hdmi: Remove redundant assignments Maxime Ripard
` (4 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Maxime Ripard @ 2025-01-29 14:21 UTC (permalink / raw)
To: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-kernel, Maxime Ripard
The light_up_connector helper function in the HDMI infrastructure unit
tests uses drm_atomic_set_crtc_for_connector(), but fails when it
returns an error.
This function can return EDEADLK though if the sequence needs to be
restarted, and WW_MUTEX_SLOWPATH is meant to test that we handle it
properly.
Let's handle EDEADLK and restart the sequence in our tests as well.
Fixes: eb66d34d793e ("drm/tests: Add output bpc tests")
Reported-by: Dave Airlie <airlied@gmail.com>
Closes: https://lore.kernel.org/r/CAPM=9tzJ4-ERDxvuwrCyUPY0=+P44orhp1kLWVGL7MCfpQjMEQ@mail.gmail.com/
Link: https://lore.kernel.org/r/20241031091558.2435850-1-mripard@kernel.org
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 7 +++++++
1 file changed, 7 insertions(+)
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 c3b693bb966f1f8b04066d19f520bfa4bf11c23d..e814b538c46c4d342604436beca64ac0bdb31526 100644
--- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
@@ -68,14 +68,21 @@ static int light_up_connector(struct kunit *test,
int ret;
state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+retry:
conn_state = drm_atomic_get_connector_state(state, connector);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
ret = drm_atomic_set_crtc_for_connector(conn_state, crtc);
+ if (ret == -EDEADLK) {
+ drm_atomic_state_clear(state);
+ ret = drm_modeset_backoff(ctx);
+ if (!ret)
+ goto retry;
+ }
KUNIT_EXPECT_EQ(test, ret, 0);
crtc_state = drm_atomic_get_crtc_state(state, crtc);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
--
2.47.1
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 1/4] drm/tests: hdmi: Fix WW_MUTEX_SLOWPATH failures
2025-01-29 14:21 ` [PATCH v2 1/4] drm/tests: hdmi: Fix WW_MUTEX_SLOWPATH failures Maxime Ripard
@ 2025-02-04 11:01 ` Simona Vetter
0 siblings, 0 replies; 10+ messages in thread
From: Simona Vetter @ 2025-02-04 11:01 UTC (permalink / raw)
To: Maxime Ripard
Cc: Maarten Lankhorst, Thomas Zimmermann, David Airlie,
Simona Vetter, dri-devel, linux-kernel
On Wed, Jan 29, 2025 at 03:21:53PM +0100, Maxime Ripard wrote:
> The light_up_connector helper function in the HDMI infrastructure unit
> tests uses drm_atomic_set_crtc_for_connector(), but fails when it
> returns an error.
>
> This function can return EDEADLK though if the sequence needs to be
> restarted, and WW_MUTEX_SLOWPATH is meant to test that we handle it
> properly.
>
> Let's handle EDEADLK and restart the sequence in our tests as well.
>
> Fixes: eb66d34d793e ("drm/tests: Add output bpc tests")
> Reported-by: Dave Airlie <airlied@gmail.com>
> Closes: https://lore.kernel.org/r/CAPM=9tzJ4-ERDxvuwrCyUPY0=+P44orhp1kLWVGL7MCfpQjMEQ@mail.gmail.com/
> Link: https://lore.kernel.org/r/20241031091558.2435850-1-mripard@kernel.org
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
> drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> 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 c3b693bb966f1f8b04066d19f520bfa4bf11c23d..e814b538c46c4d342604436beca64ac0bdb31526 100644
> --- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> +++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> @@ -68,14 +68,21 @@ static int light_up_connector(struct kunit *test,
> int ret;
>
> state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
>
> +retry:
> conn_state = drm_atomic_get_connector_state(state, connector);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
>
> ret = drm_atomic_set_crtc_for_connector(conn_state, crtc);
> + if (ret == -EDEADLK) {
> + drm_atomic_state_clear(state);
> + ret = drm_modeset_backoff(ctx);
> + if (!ret)
> + goto retry;
> + }
Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch>
> KUNIT_EXPECT_EQ(test, ret, 0);
>
> crtc_state = drm_atomic_get_crtc_state(state, crtc);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
>
>
> --
> 2.47.1
>
--
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/4] drm/tests: hdmi: Remove redundant assignments
2025-01-29 14:21 [PATCH v2 0/4] drm/tests: Fix locking issues (kind of) Maxime Ripard
2025-01-29 14:21 ` [PATCH v2 1/4] drm/tests: hdmi: Fix WW_MUTEX_SLOWPATH failures Maxime Ripard
@ 2025-01-29 14:21 ` Maxime Ripard
2025-02-04 11:15 ` Simona Vetter
2025-01-29 14:21 ` [PATCH v2 3/4] drm/tests: hdmi: Reorder DRM entities variables assignment Maxime Ripard
` (3 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Maxime Ripard @ 2025-01-29 14:21 UTC (permalink / raw)
To: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-kernel, Maxime Ripard
Some tests have the drm pointer assigned multiple times to the same
value. Drop the redundant assignments.
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 4 ----
1 file changed, 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 e814b538c46c4d342604436beca64ac0bdb31526..32e0477deb45c1481a7d23bb6e6a74de52845d1c 100644
--- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
@@ -479,11 +479,10 @@ static void drm_test_check_broadcast_rgb_auto_cea_mode_vic_1(struct kunit *test)
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
mode = drm_kunit_display_mode_from_cea_vic(test, drm, 1);
KUNIT_ASSERT_NOT_NULL(test, mode);
- drm = &priv->drm;
crtc = priv->crtc;
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);
@@ -593,11 +592,10 @@ static void drm_test_check_broadcast_rgb_full_cea_mode_vic_1(struct kunit *test)
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
mode = drm_kunit_display_mode_from_cea_vic(test, drm, 1);
KUNIT_ASSERT_NOT_NULL(test, mode);
- drm = &priv->drm;
crtc = priv->crtc;
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);
@@ -709,11 +707,10 @@ static void drm_test_check_broadcast_rgb_limited_cea_mode_vic_1(struct kunit *te
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
mode = drm_kunit_display_mode_from_cea_vic(test, drm, 1);
KUNIT_ASSERT_NOT_NULL(test, mode);
- drm = &priv->drm;
crtc = priv->crtc;
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);
@@ -1311,11 +1308,10 @@ static void drm_test_check_output_bpc_format_vic_1(struct kunit *test)
* Thus, we have to calculate the rate by hand.
*/
rate = mode->clock * 1500;
KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
- drm = &priv->drm;
crtc = priv->crtc;
ret = light_up_connector(test, drm, crtc, conn, mode, ctx);
KUNIT_EXPECT_EQ(test, ret, 0);
conn_state = conn->state;
--
2.47.1
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 2/4] drm/tests: hdmi: Remove redundant assignments
2025-01-29 14:21 ` [PATCH v2 2/4] drm/tests: hdmi: Remove redundant assignments Maxime Ripard
@ 2025-02-04 11:15 ` Simona Vetter
0 siblings, 0 replies; 10+ messages in thread
From: Simona Vetter @ 2025-02-04 11:15 UTC (permalink / raw)
To: Maxime Ripard
Cc: Maarten Lankhorst, Thomas Zimmermann, David Airlie,
Simona Vetter, dri-devel, linux-kernel
On Wed, Jan 29, 2025 at 03:21:54PM +0100, Maxime Ripard wrote:
> Some tests have the drm pointer assigned multiple times to the same
> value. Drop the redundant assignments.
>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 4 ----
> 1 file changed, 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 e814b538c46c4d342604436beca64ac0bdb31526..32e0477deb45c1481a7d23bb6e6a74de52845d1c 100644
> --- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> +++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> @@ -479,11 +479,10 @@ static void drm_test_check_broadcast_rgb_auto_cea_mode_vic_1(struct kunit *test)
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
>
> mode = drm_kunit_display_mode_from_cea_vic(test, drm, 1);
> KUNIT_ASSERT_NOT_NULL(test, mode);
>
> - drm = &priv->drm;
> crtc = priv->crtc;
> 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);
> @@ -593,11 +592,10 @@ static void drm_test_check_broadcast_rgb_full_cea_mode_vic_1(struct kunit *test)
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
>
> mode = drm_kunit_display_mode_from_cea_vic(test, drm, 1);
> KUNIT_ASSERT_NOT_NULL(test, mode);
>
> - drm = &priv->drm;
> crtc = priv->crtc;
> 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);
> @@ -709,11 +707,10 @@ static void drm_test_check_broadcast_rgb_limited_cea_mode_vic_1(struct kunit *te
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
>
> mode = drm_kunit_display_mode_from_cea_vic(test, drm, 1);
> KUNIT_ASSERT_NOT_NULL(test, mode);
>
> - drm = &priv->drm;
> crtc = priv->crtc;
> 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);
> @@ -1311,11 +1308,10 @@ static void drm_test_check_output_bpc_format_vic_1(struct kunit *test)
> * Thus, we have to calculate the rate by hand.
> */
> rate = mode->clock * 1500;
> KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
>
> - drm = &priv->drm;
> crtc = priv->crtc;
> ret = light_up_connector(test, drm, crtc, conn, mode, ctx);
> KUNIT_EXPECT_EQ(test, ret, 0);
>
> conn_state = conn->state;
>
> --
> 2.47.1
>
--
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 3/4] drm/tests: hdmi: Reorder DRM entities variables assignment
2025-01-29 14:21 [PATCH v2 0/4] drm/tests: Fix locking issues (kind of) Maxime Ripard
2025-01-29 14:21 ` [PATCH v2 1/4] drm/tests: hdmi: Fix WW_MUTEX_SLOWPATH failures Maxime Ripard
2025-01-29 14:21 ` [PATCH v2 2/4] drm/tests: hdmi: Remove redundant assignments Maxime Ripard
@ 2025-01-29 14:21 ` Maxime Ripard
2025-01-29 14:21 ` [PATCH v2 4/4] drm/tests: hdmi: Fix recursive locking Maxime Ripard
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2025-01-29 14:21 UTC (permalink / raw)
To: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-kernel, Maxime Ripard
The tests all deviate slightly in how they assign their local pointers
to DRM entities. This makes refactoring pretty difficult, so let's just
move the assignment as soon as the entities are allocated.
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 81 +++++++++++-----------
1 file changed, 42 insertions(+), 39 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 32e0477deb45c1481a7d23bb6e6a74de52845d1c..c265d28631603f266ba606564e77b8f965b7f83b 100644
--- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
@@ -287,19 +287,20 @@ static void drm_test_check_broadcast_rgb_crtc_mode_changed(struct kunit *test)
priv = drm_kunit_helper_connector_hdmi_init(test,
BIT(HDMI_COLORSPACE_RGB),
8);
KUNIT_ASSERT_NOT_NULL(test, priv);
+ drm = &priv->drm;
+ crtc = priv->crtc;
+ conn = &priv->connector;
+
ctx = drm_kunit_helper_acquire_ctx_alloc(test);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
- conn = &priv->connector;
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
- drm = &priv->drm;
- crtc = priv->crtc;
ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
@@ -350,19 +351,20 @@ static void drm_test_check_broadcast_rgb_crtc_mode_not_changed(struct kunit *tes
priv = drm_kunit_helper_connector_hdmi_init(test,
BIT(HDMI_COLORSPACE_RGB),
8);
KUNIT_ASSERT_NOT_NULL(test, priv);
+ drm = &priv->drm;
+ crtc = priv->crtc;
+ conn = &priv->connector;
+
ctx = drm_kunit_helper_acquire_ctx_alloc(test);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
- conn = &priv->connector;
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
- drm = &priv->drm;
- crtc = priv->crtc;
ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
@@ -413,22 +415,22 @@ static void drm_test_check_broadcast_rgb_auto_cea_mode(struct kunit *test)
priv = drm_kunit_helper_connector_hdmi_init(test,
BIT(HDMI_COLORSPACE_RGB),
8);
KUNIT_ASSERT_NOT_NULL(test, priv);
+ drm = &priv->drm;
+ crtc = priv->crtc;
conn = &priv->connector;
KUNIT_ASSERT_TRUE(test, conn->display_info.is_hdmi);
ctx = drm_kunit_helper_acquire_ctx_alloc(test);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
KUNIT_ASSERT_NE(test, drm_match_cea_mode(preferred), 1);
- drm = &priv->drm;
- crtc = priv->crtc;
ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
@@ -524,22 +526,22 @@ static void drm_test_check_broadcast_rgb_full_cea_mode(struct kunit *test)
priv = drm_kunit_helper_connector_hdmi_init(test,
BIT(HDMI_COLORSPACE_RGB),
8);
KUNIT_ASSERT_NOT_NULL(test, priv);
+ drm = &priv->drm;
+ crtc = priv->crtc;
conn = &priv->connector;
KUNIT_ASSERT_TRUE(test, conn->display_info.is_hdmi);
ctx = drm_kunit_helper_acquire_ctx_alloc(test);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
KUNIT_ASSERT_NE(test, drm_match_cea_mode(preferred), 1);
- drm = &priv->drm;
- crtc = priv->crtc;
ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
@@ -639,22 +641,22 @@ static void drm_test_check_broadcast_rgb_limited_cea_mode(struct kunit *test)
priv = drm_kunit_helper_connector_hdmi_init(test,
BIT(HDMI_COLORSPACE_RGB),
8);
KUNIT_ASSERT_NOT_NULL(test, priv);
+ drm = &priv->drm;
+ crtc = priv->crtc;
conn = &priv->connector;
KUNIT_ASSERT_TRUE(test, conn->display_info.is_hdmi);
ctx = drm_kunit_helper_acquire_ctx_alloc(test);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
KUNIT_ASSERT_NE(test, drm_match_cea_mode(preferred), 1);
- drm = &priv->drm;
- crtc = priv->crtc;
ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
@@ -756,10 +758,12 @@ static void drm_test_check_output_bpc_crtc_mode_changed(struct kunit *test)
priv = drm_kunit_helper_connector_hdmi_init(test,
BIT(HDMI_COLORSPACE_RGB),
10);
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);
@@ -768,12 +772,10 @@ static void drm_test_check_output_bpc_crtc_mode_changed(struct kunit *test)
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
- drm = &priv->drm;
- crtc = priv->crtc;
ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
@@ -830,10 +832,12 @@ static void drm_test_check_output_bpc_crtc_mode_not_changed(struct kunit *test)
priv = drm_kunit_helper_connector_hdmi_init(test,
BIT(HDMI_COLORSPACE_RGB),
10);
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);
@@ -842,12 +846,10 @@ static void drm_test_check_output_bpc_crtc_mode_not_changed(struct kunit *test)
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
- drm = &priv->drm;
- crtc = priv->crtc;
ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
@@ -901,10 +903,12 @@ static void drm_test_check_output_bpc_dvi(struct kunit *test)
BIT(HDMI_COLORSPACE_YUV422) |
BIT(HDMI_COLORSPACE_YUV444),
12);
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);
@@ -916,12 +920,10 @@ static void drm_test_check_output_bpc_dvi(struct kunit *test)
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
- drm = &priv->drm;
- crtc = priv->crtc;
ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
KUNIT_ASSERT_EQ(test, ret, 0);
conn_state = conn->state;
KUNIT_ASSERT_NOT_NULL(test, conn_state);
@@ -948,10 +950,12 @@ static void drm_test_check_tmds_char_rate_rgb_8bpc(struct kunit *test)
priv = drm_kunit_helper_connector_hdmi_init(test,
BIT(HDMI_COLORSPACE_RGB),
8);
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);
@@ -961,12 +965,10 @@ static void drm_test_check_tmds_char_rate_rgb_8bpc(struct kunit *test)
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
KUNIT_ASSERT_FALSE(test, preferred->flags & DRM_MODE_FLAG_DBLCLK);
- drm = &priv->drm;
- crtc = priv->crtc;
ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
KUNIT_ASSERT_EQ(test, ret, 0);
conn_state = conn->state;
KUNIT_ASSERT_NOT_NULL(test, conn_state);
@@ -995,10 +997,12 @@ static void drm_test_check_tmds_char_rate_rgb_10bpc(struct kunit *test)
priv = drm_kunit_helper_connector_hdmi_init(test,
BIT(HDMI_COLORSPACE_RGB),
10);
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);
@@ -1008,12 +1012,10 @@ static void drm_test_check_tmds_char_rate_rgb_10bpc(struct kunit *test)
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
KUNIT_ASSERT_FALSE(test, preferred->flags & DRM_MODE_FLAG_DBLCLK);
- drm = &priv->drm;
- crtc = priv->crtc;
ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
KUNIT_ASSERT_EQ(test, ret, 0);
conn_state = conn->state;
KUNIT_ASSERT_NOT_NULL(test, conn_state);
@@ -1042,10 +1044,12 @@ static void drm_test_check_tmds_char_rate_rgb_12bpc(struct kunit *test)
priv = drm_kunit_helper_connector_hdmi_init(test,
BIT(HDMI_COLORSPACE_RGB),
12);
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);
@@ -1055,12 +1059,10 @@ static void drm_test_check_tmds_char_rate_rgb_12bpc(struct kunit *test)
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
KUNIT_ASSERT_FALSE(test, preferred->flags & DRM_MODE_FLAG_DBLCLK);
- drm = &priv->drm;
- crtc = priv->crtc;
ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
KUNIT_ASSERT_EQ(test, ret, 0);
conn_state = conn->state;
KUNIT_ASSERT_NOT_NULL(test, conn_state);
@@ -1093,19 +1095,20 @@ static void drm_test_check_hdmi_funcs_reject_rate(struct kunit *test)
priv = drm_kunit_helper_connector_hdmi_init(test,
BIT(HDMI_COLORSPACE_RGB),
8);
KUNIT_ASSERT_NOT_NULL(test, priv);
+ drm = &priv->drm;
+ crtc = priv->crtc;
+ conn = &priv->connector;
+
ctx = drm_kunit_helper_acquire_ctx_alloc(test);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
- conn = &priv->connector;
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
- drm = &priv->drm;
- crtc = priv->crtc;
ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
KUNIT_ASSERT_EQ(test, ret, 0);
/* You shouldn't be doing that at home. */
conn->hdmi.funcs = &reject_connector_hdmi_funcs;
@@ -1149,10 +1152,12 @@ static void drm_test_check_max_tmds_rate_bpc_fallback(struct kunit *test)
priv = drm_kunit_helper_connector_hdmi_init(test,
BIT(HDMI_COLORSPACE_RGB),
12);
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);
@@ -1172,12 +1177,10 @@ static void drm_test_check_max_tmds_rate_bpc_fallback(struct kunit *test)
KUNIT_ASSERT_GT(test, rate, info->max_tmds_clock * 1000);
rate = drm_hdmi_compute_mode_clock(preferred, 10, HDMI_COLORSPACE_RGB);
KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
- drm = &priv->drm;
- crtc = priv->crtc;
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);
@@ -1218,10 +1221,12 @@ static void drm_test_check_max_tmds_rate_format_fallback(struct kunit *test)
BIT(HDMI_COLORSPACE_YUV422) |
BIT(HDMI_COLORSPACE_YUV444),
12);
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);
@@ -1244,12 +1249,10 @@ static void drm_test_check_max_tmds_rate_format_fallback(struct kunit *test)
KUNIT_ASSERT_GT(test, rate, info->max_tmds_clock * 1000);
rate = drm_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_YUV422);
KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
- drm = &priv->drm;
- crtc = priv->crtc;
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);
@@ -1341,10 +1344,12 @@ static void drm_test_check_output_bpc_format_driver_rgb_only(struct kunit *test)
priv = drm_kunit_helper_connector_hdmi_init(test,
BIT(HDMI_COLORSPACE_RGB),
12);
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);
@@ -1372,12 +1377,10 @@ static void drm_test_check_output_bpc_format_driver_rgb_only(struct kunit *test)
KUNIT_ASSERT_GT(test, rate, info->max_tmds_clock * 1000);
rate = drm_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_YUV422);
KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
- drm = &priv->drm;
- crtc = priv->crtc;
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);
@@ -1408,10 +1411,12 @@ static void drm_test_check_output_bpc_format_display_rgb_only(struct kunit *test
BIT(HDMI_COLORSPACE_YUV422) |
BIT(HDMI_COLORSPACE_YUV444),
12);
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);
@@ -1439,12 +1444,10 @@ static void drm_test_check_output_bpc_format_display_rgb_only(struct kunit *test
KUNIT_ASSERT_GT(test, rate, info->max_tmds_clock * 1000);
rate = drm_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_YUV422);
KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
- drm = &priv->drm;
- crtc = priv->crtc;
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);
@@ -1474,10 +1477,12 @@ static void drm_test_check_output_bpc_format_driver_8bpc_only(struct kunit *test
priv = drm_kunit_helper_connector_hdmi_init(test,
BIT(HDMI_COLORSPACE_RGB),
8);
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);
@@ -1497,12 +1502,10 @@ static void drm_test_check_output_bpc_format_driver_8bpc_only(struct kunit *test
* clock to actually use 12bpc.
*/
rate = drm_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_RGB);
KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
- drm = &priv->drm;
- crtc = priv->crtc;
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);
@@ -1534,10 +1537,12 @@ static void drm_test_check_output_bpc_format_display_8bpc_only(struct kunit *tes
BIT(HDMI_COLORSPACE_YUV422) |
BIT(HDMI_COLORSPACE_YUV444),
12);
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);
@@ -1557,12 +1562,10 @@ static void drm_test_check_output_bpc_format_display_8bpc_only(struct kunit *tes
* clock to actually use 12bpc.
*/
rate = drm_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_RGB);
KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
- drm = &priv->drm;
- crtc = priv->crtc;
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);
--
2.47.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v2 4/4] drm/tests: hdmi: Fix recursive locking
2025-01-29 14:21 [PATCH v2 0/4] drm/tests: Fix locking issues (kind of) Maxime Ripard
` (2 preceding siblings ...)
2025-01-29 14:21 ` [PATCH v2 3/4] drm/tests: hdmi: Reorder DRM entities variables assignment Maxime Ripard
@ 2025-01-29 14:21 ` Maxime Ripard
2025-02-04 11:17 ` Simona Vetter
2025-02-04 11:00 ` [PATCH v2 0/4] drm/tests: Fix locking issues (kind of) Simona Vetter
2025-02-06 13:10 ` Maxime Ripard
5 siblings, 1 reply; 10+ messages in thread
From: Maxime Ripard @ 2025-01-29 14:21 UTC (permalink / raw)
To: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-kernel, Maxime Ripard
The find_preferred_mode() functions takes the mode_config mutex, but due
to the order most tests have, is called with the crtc_ww_class_mutex
taken. This raises a warning for a circular dependency when running the
tests with lockdep.
Reorder the tests to call find_preferred_mode before the acquire context
has been created to avoid the issue.
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 114 ++++++++++-----------
1 file changed, 57 insertions(+), 57 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 c265d28631603f266ba606564e77b8f965b7f83b..037c19875daf8b9b13e7702c8640dde1547f817c 100644
--- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
@@ -291,16 +291,16 @@ static void drm_test_check_broadcast_rgb_crtc_mode_changed(struct kunit *test)
drm = &priv->drm;
crtc = priv->crtc;
conn = &priv->connector;
- ctx = drm_kunit_helper_acquire_ctx_alloc(test);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
-
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
@@ -355,16 +355,16 @@ static void drm_test_check_broadcast_rgb_crtc_mode_not_changed(struct kunit *tes
drm = &priv->drm;
crtc = priv->crtc;
conn = &priv->connector;
- ctx = drm_kunit_helper_acquire_ctx_alloc(test);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
-
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
@@ -420,17 +420,17 @@ static void drm_test_check_broadcast_rgb_auto_cea_mode(struct kunit *test)
drm = &priv->drm;
crtc = priv->crtc;
conn = &priv->connector;
KUNIT_ASSERT_TRUE(test, conn->display_info.is_hdmi);
- ctx = drm_kunit_helper_acquire_ctx_alloc(test);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
-
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
KUNIT_ASSERT_NE(test, drm_match_cea_mode(preferred), 1);
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
@@ -531,17 +531,17 @@ static void drm_test_check_broadcast_rgb_full_cea_mode(struct kunit *test)
drm = &priv->drm;
crtc = priv->crtc;
conn = &priv->connector;
KUNIT_ASSERT_TRUE(test, conn->display_info.is_hdmi);
- ctx = drm_kunit_helper_acquire_ctx_alloc(test);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
-
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
KUNIT_ASSERT_NE(test, drm_match_cea_mode(preferred), 1);
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
@@ -646,17 +646,17 @@ static void drm_test_check_broadcast_rgb_limited_cea_mode(struct kunit *test)
drm = &priv->drm;
crtc = priv->crtc;
conn = &priv->connector;
KUNIT_ASSERT_TRUE(test, conn->display_info.is_hdmi);
- ctx = drm_kunit_helper_acquire_ctx_alloc(test);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
-
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
KUNIT_ASSERT_NE(test, drm_match_cea_mode(preferred), 1);
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
@@ -766,16 +766,16 @@ static void drm_test_check_output_bpc_crtc_mode_changed(struct kunit *test)
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);
- ctx = drm_kunit_helper_acquire_ctx_alloc(test);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
-
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
@@ -840,16 +840,16 @@ static void drm_test_check_output_bpc_crtc_mode_not_changed(struct kunit *test)
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);
- ctx = drm_kunit_helper_acquire_ctx_alloc(test);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
-
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
@@ -914,16 +914,16 @@ static void drm_test_check_output_bpc_dvi(struct kunit *test)
KUNIT_ASSERT_GT(test, ret, 0);
info = &conn->display_info;
KUNIT_ASSERT_FALSE(test, info->is_hdmi);
- ctx = drm_kunit_helper_acquire_ctx_alloc(test);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
-
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
KUNIT_ASSERT_EQ(test, ret, 0);
conn_state = conn->state;
KUNIT_ASSERT_NOT_NULL(test, conn_state);
@@ -958,17 +958,17 @@ static void drm_test_check_tmds_char_rate_rgb_8bpc(struct kunit *test)
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);
- ctx = drm_kunit_helper_acquire_ctx_alloc(test);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
-
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
KUNIT_ASSERT_FALSE(test, preferred->flags & DRM_MODE_FLAG_DBLCLK);
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
KUNIT_ASSERT_EQ(test, ret, 0);
conn_state = conn->state;
KUNIT_ASSERT_NOT_NULL(test, conn_state);
@@ -1005,17 +1005,17 @@ static void drm_test_check_tmds_char_rate_rgb_10bpc(struct kunit *test)
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);
- ctx = drm_kunit_helper_acquire_ctx_alloc(test);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
-
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
KUNIT_ASSERT_FALSE(test, preferred->flags & DRM_MODE_FLAG_DBLCLK);
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
KUNIT_ASSERT_EQ(test, ret, 0);
conn_state = conn->state;
KUNIT_ASSERT_NOT_NULL(test, conn_state);
@@ -1052,17 +1052,17 @@ static void drm_test_check_tmds_char_rate_rgb_12bpc(struct kunit *test)
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);
- ctx = drm_kunit_helper_acquire_ctx_alloc(test);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
-
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
KUNIT_ASSERT_FALSE(test, preferred->flags & DRM_MODE_FLAG_DBLCLK);
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
KUNIT_ASSERT_EQ(test, ret, 0);
conn_state = conn->state;
KUNIT_ASSERT_NOT_NULL(test, conn_state);
@@ -1099,16 +1099,16 @@ static void drm_test_check_hdmi_funcs_reject_rate(struct kunit *test)
drm = &priv->drm;
crtc = priv->crtc;
conn = &priv->connector;
- ctx = drm_kunit_helper_acquire_ctx_alloc(test);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
-
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
KUNIT_ASSERT_EQ(test, ret, 0);
/* You shouldn't be doing that at home. */
conn->hdmi.funcs = &reject_connector_hdmi_funcs;
@@ -1164,23 +1164,23 @@ static void drm_test_check_max_tmds_rate_bpc_fallback(struct kunit *test)
info = &conn->display_info;
KUNIT_ASSERT_TRUE(test, info->is_hdmi);
KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
- ctx = drm_kunit_helper_acquire_ctx_alloc(test);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
-
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
KUNIT_ASSERT_FALSE(test, preferred->flags & DRM_MODE_FLAG_DBLCLK);
rate = drm_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_RGB);
KUNIT_ASSERT_GT(test, rate, info->max_tmds_clock * 1000);
rate = drm_hdmi_compute_mode_clock(preferred, 10, HDMI_COLORSPACE_RGB);
KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
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);
@@ -1233,13 +1233,10 @@ static void drm_test_check_max_tmds_rate_format_fallback(struct kunit *test)
info = &conn->display_info;
KUNIT_ASSERT_TRUE(test, info->is_hdmi);
KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
- ctx = drm_kunit_helper_acquire_ctx_alloc(test);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
-
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
KUNIT_ASSERT_FALSE(test, preferred->flags & DRM_MODE_FLAG_DBLCLK);
rate = drm_hdmi_compute_mode_clock(preferred, 10, HDMI_COLORSPACE_RGB);
@@ -1249,10 +1246,13 @@ static void drm_test_check_max_tmds_rate_format_fallback(struct kunit *test)
KUNIT_ASSERT_GT(test, rate, info->max_tmds_clock * 1000);
rate = drm_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_YUV422);
KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
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);
@@ -1295,13 +1295,10 @@ static void drm_test_check_output_bpc_format_vic_1(struct kunit *test)
info = &conn->display_info;
KUNIT_ASSERT_TRUE(test, info->is_hdmi);
KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
- ctx = drm_kunit_helper_acquire_ctx_alloc(test);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
-
mode = drm_kunit_display_mode_from_cea_vic(test, drm, 1);
KUNIT_ASSERT_NOT_NULL(test, mode);
/*
* NOTE: We can't use drm_hdmi_compute_mode_clock()
@@ -1311,10 +1308,13 @@ static void drm_test_check_output_bpc_format_vic_1(struct kunit *test)
* Thus, we have to calculate the rate by hand.
*/
rate = mode->clock * 1500;
KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
crtc = priv->crtc;
ret = light_up_connector(test, drm, crtc, conn, mode, ctx);
KUNIT_EXPECT_EQ(test, ret, 0);
conn_state = conn->state;
@@ -1356,13 +1356,10 @@ static void drm_test_check_output_bpc_format_driver_rgb_only(struct kunit *test)
info = &conn->display_info;
KUNIT_ASSERT_TRUE(test, info->is_hdmi);
KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
- ctx = drm_kunit_helper_acquire_ctx_alloc(test);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
-
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
/*
* We're making sure that YUV422 would be the preferred option
@@ -1377,10 +1374,13 @@ static void drm_test_check_output_bpc_format_driver_rgb_only(struct kunit *test)
KUNIT_ASSERT_GT(test, rate, info->max_tmds_clock * 1000);
rate = drm_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_YUV422);
KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
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);
@@ -1423,13 +1423,10 @@ static void drm_test_check_output_bpc_format_display_rgb_only(struct kunit *test
info = &conn->display_info;
KUNIT_ASSERT_TRUE(test, info->is_hdmi);
KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
- ctx = drm_kunit_helper_acquire_ctx_alloc(test);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
-
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
/*
* We're making sure that YUV422 would be the preferred option
@@ -1444,10 +1441,13 @@ static void drm_test_check_output_bpc_format_display_rgb_only(struct kunit *test
KUNIT_ASSERT_GT(test, rate, info->max_tmds_clock * 1000);
rate = drm_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_YUV422);
KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
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);
@@ -1489,23 +1489,23 @@ static void drm_test_check_output_bpc_format_driver_8bpc_only(struct kunit *test
info = &conn->display_info;
KUNIT_ASSERT_TRUE(test, info->is_hdmi);
KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
- ctx = drm_kunit_helper_acquire_ctx_alloc(test);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
-
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
/*
* We're making sure that we have headroom on the TMDS character
* clock to actually use 12bpc.
*/
rate = drm_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_RGB);
KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
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);
@@ -1549,23 +1549,23 @@ static void drm_test_check_output_bpc_format_display_8bpc_only(struct kunit *tes
info = &conn->display_info;
KUNIT_ASSERT_TRUE(test, info->is_hdmi);
KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
- ctx = drm_kunit_helper_acquire_ctx_alloc(test);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
-
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
/*
* We're making sure that we have headroom on the TMDS character
* clock to actually use 12bpc.
*/
rate = drm_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_RGB);
KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
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);
--
2.47.1
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 4/4] drm/tests: hdmi: Fix recursive locking
2025-01-29 14:21 ` [PATCH v2 4/4] drm/tests: hdmi: Fix recursive locking Maxime Ripard
@ 2025-02-04 11:17 ` Simona Vetter
0 siblings, 0 replies; 10+ messages in thread
From: Simona Vetter @ 2025-02-04 11:17 UTC (permalink / raw)
To: Maxime Ripard
Cc: Maarten Lankhorst, Thomas Zimmermann, David Airlie,
Simona Vetter, dri-devel, linux-kernel
On Wed, Jan 29, 2025 at 03:21:56PM +0100, Maxime Ripard wrote:
> The find_preferred_mode() functions takes the mode_config mutex, but due
> to the order most tests have, is called with the crtc_ww_class_mutex
> taken. This raises a warning for a circular dependency when running the
> tests with lockdep.
>
> Reorder the tests to call find_preferred_mode before the acquire context
> has been created to avoid the issue.
>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
On the last two patches: Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 114 ++++++++++-----------
> 1 file changed, 57 insertions(+), 57 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 c265d28631603f266ba606564e77b8f965b7f83b..037c19875daf8b9b13e7702c8640dde1547f817c 100644
> --- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> +++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> @@ -291,16 +291,16 @@ static void drm_test_check_broadcast_rgb_crtc_mode_changed(struct kunit *test)
>
> drm = &priv->drm;
> crtc = priv->crtc;
> conn = &priv->connector;
>
> - ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> -
> preferred = find_preferred_mode(conn);
> KUNIT_ASSERT_NOT_NULL(test, preferred);
>
> + ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
> @@ -355,16 +355,16 @@ static void drm_test_check_broadcast_rgb_crtc_mode_not_changed(struct kunit *tes
>
> drm = &priv->drm;
> crtc = priv->crtc;
> conn = &priv->connector;
>
> - ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> -
> preferred = find_preferred_mode(conn);
> KUNIT_ASSERT_NOT_NULL(test, preferred);
>
> + ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
> @@ -420,17 +420,17 @@ static void drm_test_check_broadcast_rgb_auto_cea_mode(struct kunit *test)
> drm = &priv->drm;
> crtc = priv->crtc;
> conn = &priv->connector;
> KUNIT_ASSERT_TRUE(test, conn->display_info.is_hdmi);
>
> - ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> -
> preferred = find_preferred_mode(conn);
> KUNIT_ASSERT_NOT_NULL(test, preferred);
> KUNIT_ASSERT_NE(test, drm_match_cea_mode(preferred), 1);
>
> + ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
> @@ -531,17 +531,17 @@ static void drm_test_check_broadcast_rgb_full_cea_mode(struct kunit *test)
> drm = &priv->drm;
> crtc = priv->crtc;
> conn = &priv->connector;
> KUNIT_ASSERT_TRUE(test, conn->display_info.is_hdmi);
>
> - ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> -
> preferred = find_preferred_mode(conn);
> KUNIT_ASSERT_NOT_NULL(test, preferred);
> KUNIT_ASSERT_NE(test, drm_match_cea_mode(preferred), 1);
>
> + ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
> @@ -646,17 +646,17 @@ static void drm_test_check_broadcast_rgb_limited_cea_mode(struct kunit *test)
> drm = &priv->drm;
> crtc = priv->crtc;
> conn = &priv->connector;
> KUNIT_ASSERT_TRUE(test, conn->display_info.is_hdmi);
>
> - ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> -
> preferred = find_preferred_mode(conn);
> KUNIT_ASSERT_NOT_NULL(test, preferred);
> KUNIT_ASSERT_NE(test, drm_match_cea_mode(preferred), 1);
>
> + ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
> @@ -766,16 +766,16 @@ static void drm_test_check_output_bpc_crtc_mode_changed(struct kunit *test)
> 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);
>
> - ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> -
> preferred = find_preferred_mode(conn);
> KUNIT_ASSERT_NOT_NULL(test, preferred);
>
> + ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
> @@ -840,16 +840,16 @@ static void drm_test_check_output_bpc_crtc_mode_not_changed(struct kunit *test)
> 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);
>
> - ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> -
> preferred = find_preferred_mode(conn);
> KUNIT_ASSERT_NOT_NULL(test, preferred);
>
> + ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> ret = light_up_connector(test, drm, crtc, conn, preferred, 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);
> @@ -914,16 +914,16 @@ static void drm_test_check_output_bpc_dvi(struct kunit *test)
> KUNIT_ASSERT_GT(test, ret, 0);
>
> info = &conn->display_info;
> KUNIT_ASSERT_FALSE(test, info->is_hdmi);
>
> - ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> -
> preferred = find_preferred_mode(conn);
> KUNIT_ASSERT_NOT_NULL(test, preferred);
>
> + ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
> KUNIT_ASSERT_EQ(test, ret, 0);
>
> conn_state = conn->state;
> KUNIT_ASSERT_NOT_NULL(test, conn_state);
> @@ -958,17 +958,17 @@ static void drm_test_check_tmds_char_rate_rgb_8bpc(struct kunit *test)
> 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);
>
> - ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> -
> preferred = find_preferred_mode(conn);
> KUNIT_ASSERT_NOT_NULL(test, preferred);
> KUNIT_ASSERT_FALSE(test, preferred->flags & DRM_MODE_FLAG_DBLCLK);
>
> + ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
> KUNIT_ASSERT_EQ(test, ret, 0);
>
> conn_state = conn->state;
> KUNIT_ASSERT_NOT_NULL(test, conn_state);
> @@ -1005,17 +1005,17 @@ static void drm_test_check_tmds_char_rate_rgb_10bpc(struct kunit *test)
> 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);
>
> - ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> -
> preferred = find_preferred_mode(conn);
> KUNIT_ASSERT_NOT_NULL(test, preferred);
> KUNIT_ASSERT_FALSE(test, preferred->flags & DRM_MODE_FLAG_DBLCLK);
>
> + ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
> KUNIT_ASSERT_EQ(test, ret, 0);
>
> conn_state = conn->state;
> KUNIT_ASSERT_NOT_NULL(test, conn_state);
> @@ -1052,17 +1052,17 @@ static void drm_test_check_tmds_char_rate_rgb_12bpc(struct kunit *test)
> 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);
>
> - ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> -
> preferred = find_preferred_mode(conn);
> KUNIT_ASSERT_NOT_NULL(test, preferred);
> KUNIT_ASSERT_FALSE(test, preferred->flags & DRM_MODE_FLAG_DBLCLK);
>
> + ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
> KUNIT_ASSERT_EQ(test, ret, 0);
>
> conn_state = conn->state;
> KUNIT_ASSERT_NOT_NULL(test, conn_state);
> @@ -1099,16 +1099,16 @@ static void drm_test_check_hdmi_funcs_reject_rate(struct kunit *test)
>
> drm = &priv->drm;
> crtc = priv->crtc;
> conn = &priv->connector;
>
> - ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> -
> preferred = find_preferred_mode(conn);
> KUNIT_ASSERT_NOT_NULL(test, preferred);
>
> + ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
> KUNIT_ASSERT_EQ(test, ret, 0);
>
> /* You shouldn't be doing that at home. */
> conn->hdmi.funcs = &reject_connector_hdmi_funcs;
> @@ -1164,23 +1164,23 @@ static void drm_test_check_max_tmds_rate_bpc_fallback(struct kunit *test)
>
> info = &conn->display_info;
> KUNIT_ASSERT_TRUE(test, info->is_hdmi);
> KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
>
> - ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> -
> preferred = find_preferred_mode(conn);
> KUNIT_ASSERT_NOT_NULL(test, preferred);
> KUNIT_ASSERT_FALSE(test, preferred->flags & DRM_MODE_FLAG_DBLCLK);
>
> rate = drm_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_RGB);
> KUNIT_ASSERT_GT(test, rate, info->max_tmds_clock * 1000);
>
> rate = drm_hdmi_compute_mode_clock(preferred, 10, HDMI_COLORSPACE_RGB);
> KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
>
> + ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> 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);
> @@ -1233,13 +1233,10 @@ static void drm_test_check_max_tmds_rate_format_fallback(struct kunit *test)
>
> info = &conn->display_info;
> KUNIT_ASSERT_TRUE(test, info->is_hdmi);
> KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
>
> - ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> -
> preferred = find_preferred_mode(conn);
> KUNIT_ASSERT_NOT_NULL(test, preferred);
> KUNIT_ASSERT_FALSE(test, preferred->flags & DRM_MODE_FLAG_DBLCLK);
>
> rate = drm_hdmi_compute_mode_clock(preferred, 10, HDMI_COLORSPACE_RGB);
> @@ -1249,10 +1246,13 @@ static void drm_test_check_max_tmds_rate_format_fallback(struct kunit *test)
> KUNIT_ASSERT_GT(test, rate, info->max_tmds_clock * 1000);
>
> rate = drm_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_YUV422);
> KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
>
> + ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> 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);
> @@ -1295,13 +1295,10 @@ static void drm_test_check_output_bpc_format_vic_1(struct kunit *test)
>
> info = &conn->display_info;
> KUNIT_ASSERT_TRUE(test, info->is_hdmi);
> KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
>
> - ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> -
> mode = drm_kunit_display_mode_from_cea_vic(test, drm, 1);
> KUNIT_ASSERT_NOT_NULL(test, mode);
>
> /*
> * NOTE: We can't use drm_hdmi_compute_mode_clock()
> @@ -1311,10 +1308,13 @@ static void drm_test_check_output_bpc_format_vic_1(struct kunit *test)
> * Thus, we have to calculate the rate by hand.
> */
> rate = mode->clock * 1500;
> KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
>
> + ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> crtc = priv->crtc;
> ret = light_up_connector(test, drm, crtc, conn, mode, ctx);
> KUNIT_EXPECT_EQ(test, ret, 0);
>
> conn_state = conn->state;
> @@ -1356,13 +1356,10 @@ static void drm_test_check_output_bpc_format_driver_rgb_only(struct kunit *test)
>
> info = &conn->display_info;
> KUNIT_ASSERT_TRUE(test, info->is_hdmi);
> KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
>
> - ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> -
> preferred = find_preferred_mode(conn);
> KUNIT_ASSERT_NOT_NULL(test, preferred);
>
> /*
> * We're making sure that YUV422 would be the preferred option
> @@ -1377,10 +1374,13 @@ static void drm_test_check_output_bpc_format_driver_rgb_only(struct kunit *test)
> KUNIT_ASSERT_GT(test, rate, info->max_tmds_clock * 1000);
>
> rate = drm_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_YUV422);
> KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
>
> + ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> 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);
> @@ -1423,13 +1423,10 @@ static void drm_test_check_output_bpc_format_display_rgb_only(struct kunit *test
>
> info = &conn->display_info;
> KUNIT_ASSERT_TRUE(test, info->is_hdmi);
> KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
>
> - ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> -
> preferred = find_preferred_mode(conn);
> KUNIT_ASSERT_NOT_NULL(test, preferred);
>
> /*
> * We're making sure that YUV422 would be the preferred option
> @@ -1444,10 +1441,13 @@ static void drm_test_check_output_bpc_format_display_rgb_only(struct kunit *test
> KUNIT_ASSERT_GT(test, rate, info->max_tmds_clock * 1000);
>
> rate = drm_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_YUV422);
> KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
>
> + ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> 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);
> @@ -1489,23 +1489,23 @@ static void drm_test_check_output_bpc_format_driver_8bpc_only(struct kunit *test
>
> info = &conn->display_info;
> KUNIT_ASSERT_TRUE(test, info->is_hdmi);
> KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
>
> - ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> -
> preferred = find_preferred_mode(conn);
> KUNIT_ASSERT_NOT_NULL(test, preferred);
>
> /*
> * We're making sure that we have headroom on the TMDS character
> * clock to actually use 12bpc.
> */
> rate = drm_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_RGB);
> KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
>
> + ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> 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);
> @@ -1549,23 +1549,23 @@ static void drm_test_check_output_bpc_format_display_8bpc_only(struct kunit *tes
>
> info = &conn->display_info;
> KUNIT_ASSERT_TRUE(test, info->is_hdmi);
> KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
>
> - ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> -
> preferred = find_preferred_mode(conn);
> KUNIT_ASSERT_NOT_NULL(test, preferred);
>
> /*
> * We're making sure that we have headroom on the TMDS character
> * clock to actually use 12bpc.
> */
> rate = drm_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_RGB);
> KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
>
> + ctx = drm_kunit_helper_acquire_ctx_alloc(test);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> 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);
>
> --
> 2.47.1
>
--
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/4] drm/tests: Fix locking issues (kind of)
2025-01-29 14:21 [PATCH v2 0/4] drm/tests: Fix locking issues (kind of) Maxime Ripard
` (3 preceding siblings ...)
2025-01-29 14:21 ` [PATCH v2 4/4] drm/tests: hdmi: Fix recursive locking Maxime Ripard
@ 2025-02-04 11:00 ` Simona Vetter
2025-02-06 13:10 ` Maxime Ripard
5 siblings, 0 replies; 10+ messages in thread
From: Simona Vetter @ 2025-02-04 11:00 UTC (permalink / raw)
To: Maxime Ripard
Cc: Maarten Lankhorst, Thomas Zimmermann, David Airlie,
Simona Vetter, dri-devel, linux-kernel
On Wed, Jan 29, 2025 at 03:21:52PM +0100, Maxime Ripard wrote:
> Hi,
>
> Here's another attempt at fixing the current locking issues with the
> HDMI kunit tests.
>
> The initial issue was reported by Dave here:
> https://lore.kernel.org/all/CAPM=9tzJ4-ERDxvuwrCyUPY0=+P44orhp1kLWVGL7MCfpQjMEQ@mail.gmail.com/
>
> After fixing it, there was still a lockdep warning for a circular
> dependency. This series is also fixing the issue.
So this looks like it's a kthread_exit, which yes is broken. You cannot
acquire a lock in one thread and release it in another thread, that does
not work for lockdep and therefore is forbidden for mutexes.
It's kinda allowed for semaphore, but that's why semaphores cannot be
automatically checked by lockdep. So yeah we cannot use such a deferred
action, it would need to be a deferred action that's run synchronously.
-Sima
>
> There's still an issue though. When running the tests, I get:
>
> KTAP version 1
> 1..1
> KTAP version 1
> # Subtest: drm_atomic_helper_connector_hdmi_check
> # module: drm_hdmi_state_helper_test
> 1..1
>
> ====================================
> WARNING: kunit_try_catch/25 still has locks held!
> 6.13.0-rc2-00410-gbd9d16533367 #18 Tainted: G N
> ------------------------------------
> 2 locks held by kunit_try_catch/25:
> #0: fff00000021586f0 (crtc_ww_class_acquire){+.+.}-{0:0}, at: drm_kunit_helper_acquire_ctx_alloc+0x5c/0xf0
> #1: fff0000002158718 (crtc_ww_class_mutex){+.+.}-{0:0}, at: drm_kunit_helper_acquire_ctx_alloc+0x5c/0xf0
>
> stack backtrace:
> CPU: 0 UID: 0 PID: 25 Comm: kunit_try_catch Tainted: G N 6.13.0-rc2-00410-gbd9d16533367 #18
> Tainted: [N]=TEST
> Hardware name: linux,dummy-virt (DT)
> Call trace:
> show_stack+0x18/0x30 (C)
> dump_stack_lvl+0x70/0x98
> dump_stack+0x18/0x24
> debug_check_no_locks_held+0x9c/0xa4
> do_exit+0x52c/0x970
> kthread_exit+0x28/0x30
> kthread+0xdc/0xf0
> ret_from_fork+0x10/0x20
> ok 1 drm_test_check_hdmi_funcs_reject_rate
> ok 1 drm_atomic_helper_connector_hdmi_check
>
> I believe it's due to the fact that drm_kunit_helper_acquire_ctx_alloc()
> will acquire the lock directly, but will release it as a deferred kunit
> action. It's not unsafe, as the lock is eventually released, but I don't
> really know what the best course of action is here:
>
> * Forget about the idea of a context tied to the lifetime of a test
> * Make lockdep know that it's ok, and we know what to do
>
> I've tried the latter, using lockdep_pin/unpin_lock, but that didn't fix
> the issue so I must have done something wrong.
>
> Let me know what you think,
> Maxime
>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
> Changes in v2:
> - Fix circular dependency warning
> - Link to v1: https://lore.kernel.org/r/20241031091558.2435850-1-mripard@kernel.org
>
> ---
> Maxime Ripard (4):
> drm/tests: hdmi: Fix WW_MUTEX_SLOWPATH failures
> drm/tests: hdmi: Remove redundant assignments
> drm/tests: hdmi: Reorder DRM entities variables assignment
> drm/tests: hdmi: Fix recursive locking
>
> drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 200 +++++++++++----------
> 1 file changed, 103 insertions(+), 97 deletions(-)
> ---
> base-commit: e2a81c0cd7de6cb063058be304b18f200c64802b
> change-id: 20250129-test-kunit-5ba3c03bffb0
>
> Best regards,
> --
> Maxime Ripard <mripard@kernel.org>
>
--
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 0/4] drm/tests: Fix locking issues (kind of)
2025-01-29 14:21 [PATCH v2 0/4] drm/tests: Fix locking issues (kind of) Maxime Ripard
` (4 preceding siblings ...)
2025-02-04 11:00 ` [PATCH v2 0/4] drm/tests: Fix locking issues (kind of) Simona Vetter
@ 2025-02-06 13:10 ` Maxime Ripard
5 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2025-02-06 13:10 UTC (permalink / raw)
To: Maarten Lankhorst, Thomas Zimmermann, David Airlie,
Simona Vetter, Maxime Ripard
Cc: dri-devel, linux-kernel
On Wed, 29 Jan 2025 15:21:52 +0100, Maxime Ripard wrote:
> Here's another attempt at fixing the current locking issues with the
> HDMI kunit tests.
>
> The initial issue was reported by Dave here:
> https://lore.kernel.org/all/CAPM=9tzJ4-ERDxvuwrCyUPY0=+P44orhp1kLWVGL7MCfpQjMEQ@mail.gmail.com/
>
> After fixing it, there was still a lockdep warning for a circular
> dependency. This series is also fixing the issue.
>
> [...]
Applied to misc/kernel.git (drm-misc-fixes).
Thanks!
Maxime
^ permalink raw reply [flat|nested] 10+ messages in thread