Hi,
Since v6.5-rc1, kunit gained a devm/drmm-like mechanism that makes tests
resources much easier to cleanup.
This series converts the existing tests to use those new actions were
relevant.
Let me know what you think,
Maxime
Signed-off-by: Maxime Ripard <[email protected]>
---
Maxime Ripard (11):
drm/tests: helpers: Switch to kunit actions
drm/tests: client-modeset: Remove call to drm_kunit_helper_free_device()
drm/tests: modes: Remove call to drm_kunit_helper_free_device()
drm/tests: probe-helper: Remove call to drm_kunit_helper_free_device()
drm/tests: helpers: Create an helper to allocate a locking ctx
drm/tests: helpers: Create an helper to allocate an atomic state
drm/vc4: tests: pv-muxing: Remove call to drm_kunit_helper_free_device()
drm/vc4: tests: mock: Use a kunit action to unregister DRM device
drm/vc4: tests: pv-muxing: Switch to managed locking init
drm/vc4: tests: Switch to atomic state allocation helper
drm/vc4: tests: pv-muxing: Document test scenario
drivers/gpu/drm/tests/drm_client_modeset_test.c | 8 --
drivers/gpu/drm/tests/drm_kunit_helpers.c | 112 ++++++++++++++++++++++-
drivers/gpu/drm/tests/drm_modes_test.c | 8 --
drivers/gpu/drm/tests/drm_probe_helper_test.c | 8 --
drivers/gpu/drm/vc4/tests/vc4_mock.c | 5 ++
drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c | 115 +++++++++---------------
include/drm/drm_kunit_helpers.h | 7 ++
7 files changed, 162 insertions(+), 101 deletions(-)
---
base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
change-id: 20230710-kms-kunit-actions-rework-5d163762c93b
Best regards,
--
Maxime Ripard <[email protected]>
Calling drm_kunit_helper_free_device() to clean up the resources
allocated by drm_kunit_helper_alloc_device() is now optional and not
needed in most cases.
Remove it.
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/tests/drm_client_modeset_test.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c b/drivers/gpu/drm/tests/drm_client_modeset_test.c
index 416a279b6dae..7516f6cb36e4 100644
--- a/drivers/gpu/drm/tests/drm_client_modeset_test.c
+++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c
@@ -82,13 +82,6 @@ static int drm_client_modeset_test_init(struct kunit *test)
return 0;
}
-static void drm_client_modeset_test_exit(struct kunit *test)
-{
- struct drm_client_modeset_test_priv *priv = test->priv;
-
- drm_kunit_helper_free_device(test, priv->dev);
-}
-
static void drm_test_pick_cmdline_res_1920_1080_60(struct kunit *test)
{
struct drm_client_modeset_test_priv *priv = test->priv;
@@ -188,7 +181,6 @@ static struct kunit_case drm_test_pick_cmdline_tests[] = {
static struct kunit_suite drm_test_pick_cmdline_test_suite = {
.name = "drm_test_pick_cmdline",
.init = drm_client_modeset_test_init,
- .exit = drm_client_modeset_test_exit,
.test_cases = drm_test_pick_cmdline_tests
};
--
2.41.0
We've had a couple of tests that weren't really obvious, nor did they
document what they were supposed to test. Document that to make it
hopefully more obvious.
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
index 5f9f5626329d..61622e951031 100644
--- a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
+++ b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
@@ -845,6 +845,13 @@ static void drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable(struct kunit *tes
KUNIT_EXPECT_NE(test, hdmi0_channel, hdmi1_channel);
}
+/*
+ * This test makes sure that we never change the FIFO of an active HVS
+ * channel if we disable a FIFO with a lower index.
+ *
+ * Doing so would result in a FIFO stall and would disrupt an output
+ * supposed to be unaffected by the commit.
+ */
static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test)
{
struct drm_modeset_acquire_ctx *ctx;
@@ -924,6 +931,21 @@ static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test)
}
}
+/*
+ * Test that if we affect a single output, only the CRTC state of that
+ * output will be pulled in the global atomic state.
+ *
+ * This is relevant for two things:
+ *
+ * - If we don't have that state at all, we are unlikely to affect the
+ * FIFO muxing. This is somewhat redundant with
+ * drm_test_vc5_pv_muxing_bugs_stable_fifo()
+ *
+ * - KMS waits for page flips to occur on all the CRTC found in the
+ * CRTC state. Since the CRTC is unaffected, we would over-wait, but
+ * most importantly run into corner cases like waiting on an
+ * inactive CRTC that never completes.
+ */
static void
drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable_too_many_crtc_state(struct kunit *test)
{
--
2.41.0
As we gain more tests, boilerplate to allocate an atomic state and free
it starts to be there more and more as well.
In order to reduce the allocation boilerplate, we can create an helper
to create that atomic state, and call an action when the test is done.
This will also clean up the exit path.
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/tests/drm_kunit_helpers.c | 39 +++++++++++++++++++++++++++++++
include/drm/drm_kunit_helpers.h | 5 ++++
2 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c b/drivers/gpu/drm/tests/drm_kunit_helpers.c
index 40a27c78d692..3f3331bc389f 100644
--- a/drivers/gpu/drm/tests/drm_kunit_helpers.c
+++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
+#include <drm/drm_atomic.h>
#include <drm/drm_drv.h>
#include <drm/drm_kunit_helpers.h>
#include <drm/drm_managed.h>
@@ -165,5 +166,43 @@ drm_kunit_helper_acquire_ctx_alloc(struct kunit *test)
}
EXPORT_SYMBOL_GPL(drm_kunit_helper_acquire_ctx_alloc);
+/**
+ * drm_kunit_helper_atomic_state_alloc - Allocates an atomic state
+ * @test: The test context object
+ * @drm: The device to alloc the state for
+ * @ctx: Locking context for that atomic update
+ *
+ * Allocates a empty atomic state.
+ *
+ * The state is tied to the kunit test context, so we must not call
+ * drm_atomic_state_put() on it, it will be done so automatically.
+ *
+ * Returns:
+ * An ERR_PTR on error, a pointer to the newly allocated state otherwise
+ */
+struct drm_atomic_state *
+drm_kunit_helper_atomic_state_alloc(struct kunit *test,
+ struct drm_device *drm,
+ struct drm_modeset_acquire_ctx *ctx)
+{
+ struct drm_atomic_state *state;
+ int ret;
+
+ state = drm_atomic_state_alloc(drm);
+ if (!state)
+ return ERR_PTR(-ENOMEM);
+
+ ret = kunit_add_action_or_reset(test,
+ (kunit_action_t *)drm_atomic_state_put,
+ state);
+ if (ret)
+ return ERR_PTR(ret);
+
+ state->acquire_ctx = ctx;
+
+ return state;
+}
+EXPORT_SYMBOL_GPL(drm_kunit_helper_atomic_state_alloc);
+
MODULE_AUTHOR("Maxime Ripard <[email protected]>");
MODULE_LICENSE("GPL");
diff --git a/include/drm/drm_kunit_helpers.h b/include/drm/drm_kunit_helpers.h
index 4ba5e10653c6..514c8a7a32f0 100644
--- a/include/drm/drm_kunit_helpers.h
+++ b/include/drm/drm_kunit_helpers.h
@@ -90,4 +90,9 @@ __drm_kunit_helper_alloc_drm_device(struct kunit *test,
struct drm_modeset_acquire_ctx *
drm_kunit_helper_acquire_ctx_alloc(struct kunit *test);
+struct drm_atomic_state *
+drm_kunit_helper_atomic_state_alloc(struct kunit *test,
+ struct drm_device *drm,
+ struct drm_modeset_acquire_ctx *ctx);
+
#endif // DRM_KUNIT_HELPERS_H_
--
2.41.0
The *_mock_device functions allocate a DRM device that needs to be
released using drm_dev_unregister.
Now that we have a kunit release action API, we can switch to it and
don't require any kind of garbage collection from the caller.
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/tests/vc4_mock.c | 5 +++++
drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c | 6 ------
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock.c b/drivers/gpu/drm/vc4/tests/vc4_mock.c
index a4bed26af32f..00825ddc52f0 100644
--- a/drivers/gpu/drm/vc4/tests/vc4_mock.c
+++ b/drivers/gpu/drm/vc4/tests/vc4_mock.c
@@ -186,6 +186,11 @@ static struct vc4_dev *__mock_device(struct kunit *test, bool is_vc5)
ret = drm_dev_register(drm, 0);
KUNIT_ASSERT_EQ(test, ret, 0);
+ ret = kunit_add_action_or_reset(test,
+ (kunit_action_t *)drm_dev_unregister,
+ drm);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
return vc4;
}
diff --git a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
index 6c982e72cae8..776a7b01608f 100644
--- a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
+++ b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
@@ -754,14 +754,11 @@ static int vc4_pv_muxing_test_init(struct kunit *test)
static void vc4_pv_muxing_test_exit(struct kunit *test)
{
struct pv_muxing_priv *priv = test->priv;
- struct vc4_dev *vc4 = priv->vc4;
- struct drm_device *drm = &vc4->base;
struct drm_atomic_state *state = priv->state;
drm_atomic_state_put(state);
drm_modeset_drop_locks(&priv->ctx);
drm_modeset_acquire_fini(&priv->ctx);
- drm_dev_unregister(drm);
}
static struct kunit_case vc4_pv_muxing_tests[] = {
@@ -871,7 +868,6 @@ static void drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable(struct kunit *tes
drm_atomic_state_put(state);
drm_modeset_drop_locks(&ctx);
drm_modeset_acquire_fini(&ctx);
- drm_dev_unregister(drm);
}
static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test)
@@ -960,7 +956,6 @@ static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test)
drm_atomic_state_put(state);
drm_modeset_drop_locks(&ctx);
drm_modeset_acquire_fini(&ctx);
- drm_dev_unregister(drm);
}
static void
@@ -1013,7 +1008,6 @@ drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable_too_many_crtc_state(struct ku
drm_atomic_state_put(state);
drm_modeset_drop_locks(&ctx);
drm_modeset_acquire_fini(&ctx);
- drm_dev_unregister(drm);
}
static struct kunit_case vc5_pv_muxing_bugs_tests[] = {
--
2.41.0
Now that we have a helper that takes care of an atomic state allocation
and cleanup, we can migrate to it to simplify our tests.
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c | 55 ++++----------------------
1 file changed, 8 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
index ff1deaed0cab..5f9f5626329d 100644
--- a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
+++ b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
@@ -725,7 +725,6 @@ static int vc4_pv_muxing_test_init(struct kunit *test)
{
const struct pv_muxing_param *params = test->param_value;
struct drm_modeset_acquire_ctx *ctx;
- struct drm_atomic_state *state;
struct pv_muxing_priv *priv;
struct drm_device *drm;
struct vc4_dev *vc4;
@@ -742,24 +741,12 @@ static int vc4_pv_muxing_test_init(struct kunit *test)
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
drm = &vc4->base;
- state = drm_atomic_state_alloc(drm);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
-
- state->acquire_ctx = ctx;
-
- priv->state = state;
+ priv->state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->state);
return 0;
}
-static void vc4_pv_muxing_test_exit(struct kunit *test)
-{
- struct pv_muxing_priv *priv = test->priv;
- struct drm_atomic_state *state = priv->state;
-
- drm_atomic_state_put(state);
-}
-
static struct kunit_case vc4_pv_muxing_tests[] = {
KUNIT_CASE_PARAM(drm_vc4_test_pv_muxing,
vc4_test_pv_muxing_gen_params),
@@ -771,7 +758,6 @@ static struct kunit_case vc4_pv_muxing_tests[] = {
static struct kunit_suite vc4_pv_muxing_test_suite = {
.name = "vc4-pv-muxing-combinations",
.init = vc4_pv_muxing_test_init,
- .exit = vc4_pv_muxing_test_exit,
.test_cases = vc4_pv_muxing_tests,
};
@@ -786,7 +772,6 @@ static struct kunit_case vc5_pv_muxing_tests[] = {
static struct kunit_suite vc5_pv_muxing_test_suite = {
.name = "vc5-pv-muxing-combinations",
.init = vc4_pv_muxing_test_init,
- .exit = vc4_pv_muxing_test_exit,
.test_cases = vc5_pv_muxing_tests,
};
@@ -814,11 +799,9 @@ static void drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable(struct kunit *tes
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
drm = &vc4->base;
- state = drm_atomic_state_alloc(drm);
+ state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
- state->acquire_ctx = ctx;
-
ret = vc4_mock_atomic_add_output(test, state, VC4_ENCODER_TYPE_HDMI0);
KUNIT_ASSERT_EQ(test, ret, 0);
@@ -839,13 +822,9 @@ static void drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable(struct kunit *tes
ret = drm_atomic_helper_swap_state(state, false);
KUNIT_ASSERT_EQ(test, ret, 0);
- drm_atomic_state_put(state);
-
- state = drm_atomic_state_alloc(drm);
+ state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
- state->acquire_ctx = ctx;
-
ret = vc4_mock_atomic_add_output(test, state, VC4_ENCODER_TYPE_HDMI1);
KUNIT_ASSERT_EQ(test, ret, 0);
@@ -864,8 +843,6 @@ static void drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable(struct kunit *tes
KUNIT_ASSERT_TRUE(test, new_hvs_state->fifo_state[hdmi1_channel].in_use);
KUNIT_EXPECT_NE(test, hdmi0_channel, hdmi1_channel);
-
- drm_atomic_state_put(state);
}
static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test)
@@ -887,11 +864,9 @@ static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test)
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
drm = &vc4->base;
- state = drm_atomic_state_alloc(drm);
+ state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
- state->acquire_ctx = ctx;
-
ret = vc4_mock_atomic_add_output(test, state, VC4_ENCODER_TYPE_HDMI0);
KUNIT_ASSERT_EQ(test, ret, 0);
@@ -923,13 +898,9 @@ static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test)
ret = drm_atomic_helper_swap_state(state, false);
KUNIT_ASSERT_EQ(test, ret, 0);
- drm_atomic_state_put(state);
-
- state = drm_atomic_state_alloc(drm);
+ state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
- state->acquire_ctx = ctx;
-
ret = vc4_mock_atomic_del_output(test, state, VC4_ENCODER_TYPE_HDMI0);
KUNIT_ASSERT_EQ(test, ret, 0);
@@ -951,8 +922,6 @@ static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test)
KUNIT_EXPECT_EQ(test, old_hdmi1_channel, hdmi1_channel);
}
-
- drm_atomic_state_put(state);
}
static void
@@ -972,11 +941,9 @@ drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable_too_many_crtc_state(struct ku
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
drm = &vc4->base;
- state = drm_atomic_state_alloc(drm);
+ state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
- state->acquire_ctx = ctx;
-
ret = vc4_mock_atomic_add_output(test, state, VC4_ENCODER_TYPE_HDMI0);
KUNIT_ASSERT_EQ(test, ret, 0);
@@ -986,13 +953,9 @@ drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable_too_many_crtc_state(struct ku
ret = drm_atomic_helper_swap_state(state, false);
KUNIT_ASSERT_EQ(test, ret, 0);
- drm_atomic_state_put(state);
-
- state = drm_atomic_state_alloc(drm);
+ state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
- state->acquire_ctx = ctx;
-
ret = vc4_mock_atomic_add_output(test, state, VC4_ENCODER_TYPE_HDMI1);
KUNIT_ASSERT_EQ(test, ret, 0);
@@ -1002,8 +965,6 @@ drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable_too_many_crtc_state(struct ku
new_vc4_crtc_state = get_vc4_crtc_state_for_encoder(test, state,
VC4_ENCODER_TYPE_HDMI0);
KUNIT_EXPECT_NULL(test, new_vc4_crtc_state);
-
- drm_atomic_state_put(state);
}
static struct kunit_case vc5_pv_muxing_bugs_tests[] = {
--
2.41.0
Calling drm_kunit_helper_free_device() to clean up the resources
allocated by drm_kunit_helper_alloc_device() is now optional and not
needed in most cases.
Remove it.
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
index ae0bd0f81698..6c982e72cae8 100644
--- a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
+++ b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
@@ -762,7 +762,6 @@ static void vc4_pv_muxing_test_exit(struct kunit *test)
drm_modeset_drop_locks(&priv->ctx);
drm_modeset_acquire_fini(&priv->ctx);
drm_dev_unregister(drm);
- drm_kunit_helper_free_device(test, vc4->dev);
}
static struct kunit_case vc4_pv_muxing_tests[] = {
@@ -873,7 +872,6 @@ static void drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable(struct kunit *tes
drm_modeset_drop_locks(&ctx);
drm_modeset_acquire_fini(&ctx);
drm_dev_unregister(drm);
- drm_kunit_helper_free_device(test, vc4->dev);
}
static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test)
@@ -963,7 +961,6 @@ static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test)
drm_modeset_drop_locks(&ctx);
drm_modeset_acquire_fini(&ctx);
drm_dev_unregister(drm);
- drm_kunit_helper_free_device(test, vc4->dev);
}
static void
@@ -1017,7 +1014,6 @@ drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable_too_many_crtc_state(struct ku
drm_modeset_drop_locks(&ctx);
drm_modeset_acquire_fini(&ctx);
drm_dev_unregister(drm);
- drm_kunit_helper_free_device(test, vc4->dev);
}
static struct kunit_case vc5_pv_muxing_bugs_tests[] = {
--
2.41.0
Hi,
On 2023/7/10 15:47, Maxime Ripard wrote:
> Hi,
>
> Since v6.5-rc1, kunit gained a devm/drmm-like mechanism that makes tests
> resources much easier to cleanup.
>
> This series converts the existing tests to use those new actions were
> relevant.
Is the word 'were' here means that 'where' relevant ?
Or it is means that it were relevant, after applied you patch it is not
relevant anymore ?
> Let me know what you think,
> Maxime
>
> Signed-off-by: Maxime Ripard <[email protected]>
> ---
> Maxime Ripard (11):
> drm/tests: helpers: Switch to kunit actions
> drm/tests: client-modeset: Remove call to drm_kunit_helper_free_device()
> drm/tests: modes: Remove call to drm_kunit_helper_free_device()
> drm/tests: probe-helper: Remove call to drm_kunit_helper_free_device()
> drm/tests: helpers: Create an helper to allocate a locking ctx
> drm/tests: helpers: Create an helper to allocate an atomic state
a helper or an helper ?
Should this two patch be re-titled as following ?
I search it on the internet[1], mostly using a helper.
drm/tests: helpers: Create a helper to allocate a locking ctx
drm/tests: helpers: Create a helper to allocate an atomic state
[1] https://www.a-or-an.com/a_an/helper
Sorry about the noise if I'm wrong.
> drm/vc4: tests: pv-muxing: Remove call to drm_kunit_helper_free_device()
> drm/vc4: tests: mock: Use a kunit action to unregister DRM device
> drm/vc4: tests: pv-muxing: Switch to managed locking init
> drm/vc4: tests: Switch to atomic state allocation helper
> drm/vc4: tests: pv-muxing: Document test scenario
>
> drivers/gpu/drm/tests/drm_client_modeset_test.c | 8 --
> drivers/gpu/drm/tests/drm_kunit_helpers.c | 112 ++++++++++++++++++++++-
> drivers/gpu/drm/tests/drm_modes_test.c | 8 --
> drivers/gpu/drm/tests/drm_probe_helper_test.c | 8 --
> drivers/gpu/drm/vc4/tests/vc4_mock.c | 5 ++
> drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c | 115 +++++++++---------------
> include/drm/drm_kunit_helpers.h | 7 ++
> 7 files changed, 162 insertions(+), 101 deletions(-)
> ---
> base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
> change-id: 20230710-kms-kunit-actions-rework-5d163762c93b
>
> Best regards,
Maxime Ripard <[email protected]> writes:
> Calling drm_kunit_helper_free_device() to clean up the resources
> allocated by drm_kunit_helper_alloc_device() is now optional and not
> needed in most cases.
>
> Remove it.
>
> Signed-off-by: Maxime Ripard <[email protected]>
> ---
Reviewed-by: Javier Martinez Canillas <[email protected]>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Maxime Ripard <[email protected]> writes:
> Now that we have a helper that takes care of an atomic state allocation
> and cleanup, we can migrate to it to simplify our tests.
>
> Signed-off-by: Maxime Ripard <[email protected]>
> ---
Reviewed-by: Javier Martinez Canillas <[email protected]>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Maxime Ripard <[email protected]> writes:
> The *_mock_device functions allocate a DRM device that needs to be
> released using drm_dev_unregister.
>
> Now that we have a kunit release action API, we can switch to it and
> don't require any kind of garbage collection from the caller.
>
> Signed-off-by: Maxime Ripard <[email protected]>
> ---
Reviewed-by: Javier Martinez Canillas <[email protected]>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Maxime Ripard <[email protected]> writes:
> As we gain more tests, boilerplate to allocate an atomic state and free
> it starts to be there more and more as well.
>
> In order to reduce the allocation boilerplate, we can create an helper
> to create that atomic state, and call an action when the test is done.
> This will also clean up the exit path.
>
> Signed-off-by: Maxime Ripard <[email protected]>
> ---
Reviewed-by: Javier Martinez Canillas <[email protected]>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Maxime Ripard <[email protected]> writes:
> Calling drm_kunit_helper_free_device() to clean up the resources
> allocated by drm_kunit_helper_alloc_device() is now optional and not
> needed in most cases.
>
> Remove it.
>
> Signed-off-by: Maxime Ripard <[email protected]>
> ---
Reviewed-by: Javier Martinez Canillas <[email protected]>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Maxime Ripard <[email protected]> writes:
> We've had a couple of tests that weren't really obvious, nor did they
> document what they were supposed to test. Document that to make it
> hopefully more obvious.
>
> Signed-off-by: Maxime Ripard <[email protected]>
> ---
Reviewed-by: Javier Martinez Canillas <[email protected]>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Hi,
On Mon, Jul 17, 2023 at 11:24:13PM +0800, suijingfeng wrote:
> On 2023/7/10 15:47, Maxime Ripard wrote:
> > Hi,
> >
> > Since v6.5-rc1, kunit gained a devm/drmm-like mechanism that makes tests
> > resources much easier to cleanup.
> >
> > This series converts the existing tests to use those new actions were
> > relevant.
>
> Is the word 'were' here means that 'where' relevant ?
Yes :)
> Or it is means that it were relevant, after applied you patch it is not
> relevant anymore ?
>
> > Let me know what you think,
> > Maxime
> >
> > Signed-off-by: Maxime Ripard <[email protected]>
> > ---
> > Maxime Ripard (11):
> > drm/tests: helpers: Switch to kunit actions
> > drm/tests: client-modeset: Remove call to drm_kunit_helper_free_device()
> > drm/tests: modes: Remove call to drm_kunit_helper_free_device()
> > drm/tests: probe-helper: Remove call to drm_kunit_helper_free_device()
> > drm/tests: helpers: Create an helper to allocate a locking ctx
> > drm/tests: helpers: Create an helper to allocate an atomic state
>
> a helper or an helper ?
>
> Should this two patch be re-titled as following ?
>
> I search it on the internet[1], mostly using a helper.
>
>
> drm/tests: helpers: Create a helper to allocate a locking ctx
> drm/tests: helpers: Create a helper to allocate an atomic state
>
> [1] https://www.a-or-an.com/a_an/helper
>
> Sorry about the noise if I'm wrong.
You're right, I'll fix it
Thanks!
Maxime