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 where relevant.
Let me know what you think, Maxime
Signed-off-by: Maxime Ripard mripard@kernel.org --- Changes in v2: - Fix some typos - Use plaltform_device_del instead of removing the call to platform_device_put after calling platform_device_add - Link to v1: https://lore.kernel.org/r/20230710-kms-kunit-actions-rework-v1-0-722c58d72c7...
--- 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 a helper to allocate a locking ctx drm/tests: helpers: Create a 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 | 108 +++++++++++++++++++++- 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, 158 insertions(+), 101 deletions(-) --- base-commit: c58c49dd89324b18a812762a2bfa5a0458e4f252 change-id: 20230710-kms-kunit-actions-rework-5d163762c93b
Best regards,
Reviewed-by: Javier Martinez Canillas javierm@redhat.com Signed-off-by: Maxime Ripard mripard@kernel.org --- drivers/gpu/drm/tests/drm_kunit_helpers.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c b/drivers/gpu/drm/tests/drm_kunit_helpers.c index 4df47071dc88..5856beb7f7d7 100644 --- a/drivers/gpu/drm/tests/drm_kunit_helpers.c +++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c @@ -35,8 +35,8 @@ static struct platform_driver fake_platform_driver = { * able to leverage the usual infrastructure and most notably the * device-managed resources just like a "real" device. * - * Callers need to make sure drm_kunit_helper_free_device() on the - * device when done. + * Resources will be cleaned up automatically, but the removal can be + * forced using @drm_kunit_helper_free_device. * * Returns: * A pointer to the new device, or an ERR_PTR() otherwise. @@ -49,12 +49,27 @@ struct device *drm_kunit_helper_alloc_device(struct kunit *test) ret = platform_driver_register(&fake_platform_driver); KUNIT_ASSERT_EQ(test, ret, 0);
+ ret = kunit_add_action_or_reset(test, + (kunit_action_t *)platform_driver_unregister, + &fake_platform_driver); + KUNIT_ASSERT_EQ(test, ret, 0); + pdev = platform_device_alloc(KUNIT_DEVICE_NAME, PLATFORM_DEVID_NONE); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
+ ret = kunit_add_action_or_reset(test, + (kunit_action_t *)platform_device_put, + pdev); + KUNIT_ASSERT_EQ(test, ret, 0); + ret = platform_device_add(pdev); KUNIT_ASSERT_EQ(test, ret, 0);
+ ret = kunit_add_action_or_reset(test, + (kunit_action_t *)platform_device_del, + pdev); + KUNIT_ASSERT_EQ(test, ret, 0); + return &pdev->dev; } EXPORT_SYMBOL_GPL(drm_kunit_helper_alloc_device); @@ -70,8 +85,13 @@ void drm_kunit_helper_free_device(struct kunit *test, struct device *dev) { struct platform_device *pdev = to_platform_device(dev);
- platform_device_unregister(pdev); - platform_driver_unregister(&fake_platform_driver); + kunit_release_action(test, + (kunit_action_t *)platform_device_unregister, + pdev); + + kunit_release_action(test, + (kunit_action_t *)platform_driver_unregister, + &fake_platform_driver); } EXPORT_SYMBOL_GPL(drm_kunit_helper_free_device);
Hi Maxime,
kernel test robot noticed the following build warnings:
[auto build test WARNING on c58c49dd89324b18a812762a2bfa5a0458e4f252]
url: https://github.com/intel-lab-lkp/linux/commits/Maxime-Ripard/drm-tests-helpe... base: c58c49dd89324b18a812762a2bfa5a0458e4f252 patch link: https://lore.kernel.org/r/20230720-kms-kunit-actions-rework-v2-1-175017bd56a... patch subject: [PATCH v2 01/11] drm/tests: helpers: Switch to kunit actions config: arm64-randconfig-r022-20230720 (https://download.01.org/0day-ci/archive/20230721/202307210148.7gWzLOtn-lkp@i...) compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a) reproduce: (https://download.01.org/0day-ci/archive/20230721/202307210148.7gWzLOtn-lkp@i...)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202307210148.7gWzLOtn-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/gpu/drm/tests/drm_kunit_helpers.c:53:6: warning: cast from 'void (*)(struct platform_driver *)' to 'kunit_action_t *' (aka 'void (*)(void *)') converts to incompatible function type [-Wcast-function-type-strict]
53 | (kunit_action_t *)platform_driver_unregister, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/tests/drm_kunit_helpers.c:61:6: warning: cast from 'void (*)(struct platform_device *)' to 'kunit_action_t *' (aka 'void (*)(void *)') converts to incompatible function type [-Wcast-function-type-strict]
61 | (kunit_action_t *)platform_device_put, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/tests/drm_kunit_helpers.c:69:6: warning: cast from 'void (*)(struct platform_device *)' to 'kunit_action_t *' (aka 'void (*)(void *)') converts to incompatible function type [-Wcast-function-type-strict] 69 | (kunit_action_t *)platform_device_del, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/tests/drm_kunit_helpers.c:89:9: warning: cast from 'void (*)(struct platform_device *)' to 'kunit_action_t *' (aka 'void (*)(void *)') converts to incompatible function type [-Wcast-function-type-strict] 89 | (kunit_action_t *)platform_device_unregister, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/tests/drm_kunit_helpers.c:93:9: warning: cast from 'void (*)(struct platform_driver *)' to 'kunit_action_t *' (aka 'void (*)(void *)') converts to incompatible function type [-Wcast-function-type-strict] 93 | (kunit_action_t *)platform_driver_unregister, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 5 warnings generated.
vim +53 drivers/gpu/drm/tests/drm_kunit_helpers.c
28 29 /** 30 * drm_kunit_helper_alloc_device - Allocate a mock device for a KUnit test 31 * @test: The test context object 32 * 33 * This allocates a fake struct &device to create a mock for a KUnit 34 * test. The device will also be bound to a fake driver. It will thus be 35 * able to leverage the usual infrastructure and most notably the 36 * device-managed resources just like a "real" device. 37 * 38 * Resources will be cleaned up automatically, but the removal can be 39 * forced using @drm_kunit_helper_free_device. 40 * 41 * Returns: 42 * A pointer to the new device, or an ERR_PTR() otherwise. 43 */ 44 struct device *drm_kunit_helper_alloc_device(struct kunit *test) 45 { 46 struct platform_device *pdev; 47 int ret; 48 49 ret = platform_driver_register(&fake_platform_driver); 50 KUNIT_ASSERT_EQ(test, ret, 0); 51 52 ret = kunit_add_action_or_reset(test,
53 (kunit_action_t *)platform_driver_unregister,
54 &fake_platform_driver); 55 KUNIT_ASSERT_EQ(test, ret, 0); 56 57 pdev = platform_device_alloc(KUNIT_DEVICE_NAME, PLATFORM_DEVID_NONE); 58 KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev); 59 60 ret = kunit_add_action_or_reset(test,
61 (kunit_action_t *)platform_device_put,
62 pdev); 63 KUNIT_ASSERT_EQ(test, ret, 0); 64 65 ret = platform_device_add(pdev); 66 KUNIT_ASSERT_EQ(test, ret, 0); 67 68 ret = kunit_add_action_or_reset(test, 69 (kunit_action_t *)platform_device_del, 70 pdev); 71 KUNIT_ASSERT_EQ(test, ret, 0); 72 73 return &pdev->dev; 74 } 75 EXPORT_SYMBOL_GPL(drm_kunit_helper_alloc_device); 76
Hi David, Brendan,
On Fri, Jul 21, 2023 at 01:14:41AM +0800, kernel test robot wrote:
Hi Maxime,
kernel test robot noticed the following build warnings:
[auto build test WARNING on c58c49dd89324b18a812762a2bfa5a0458e4f252]
url: https://github.com/intel-lab-lkp/linux/commits/Maxime-Ripard/drm-tests-helpe... base: c58c49dd89324b18a812762a2bfa5a0458e4f252 patch link: https://lore.kernel.org/r/20230720-kms-kunit-actions-rework-v2-1-175017bd56a... patch subject: [PATCH v2 01/11] drm/tests: helpers: Switch to kunit actions config: arm64-randconfig-r022-20230720 (https://download.01.org/0day-ci/archive/20230721/202307210148.7gWzLOtn-lkp@i...) compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a) reproduce: (https://download.01.org/0day-ci/archive/20230721/202307210148.7gWzLOtn-lkp@i...)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202307210148.7gWzLOtn-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/gpu/drm/tests/drm_kunit_helpers.c:53:6: warning: cast from 'void (*)(struct platform_driver *)' to 'kunit_action_t *' (aka 'void (*)(void *)') converts to incompatible function type [-Wcast-function-type-strict]
53 | (kunit_action_t *)platform_driver_unregister, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I missed that warning before, and I don't think we can address that easily. Should we give up on kunit_action_t entirely?
Maxime
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.
Reviewed-by: Javier Martinez Canillas javierm@redhat.com Signed-off-by: Maxime Ripard mripard@kernel.org --- 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 };
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.
Reviewed-by: Javier Martinez Canillas javierm@redhat.com Signed-off-by: Maxime Ripard mripard@kernel.org --- drivers/gpu/drm/tests/drm_modes_test.c | 8 -------- 1 file changed, 8 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_modes_test.c b/drivers/gpu/drm/tests/drm_modes_test.c index bc4aa2ce78be..1e9f63fbfead 100644 --- a/drivers/gpu/drm/tests/drm_modes_test.c +++ b/drivers/gpu/drm/tests/drm_modes_test.c @@ -36,13 +36,6 @@ static int drm_test_modes_init(struct kunit *test) return 0; }
-static void drm_test_modes_exit(struct kunit *test) -{ - struct drm_test_modes_priv *priv = test->priv; - - drm_kunit_helper_free_device(test, priv->dev); -} - static void drm_test_modes_analog_tv_ntsc_480i(struct kunit *test) { struct drm_test_modes_priv *priv = test->priv; @@ -148,7 +141,6 @@ static struct kunit_case drm_modes_analog_tv_tests[] = { static struct kunit_suite drm_modes_analog_tv_test_suite = { .name = "drm_modes_analog_tv", .init = drm_test_modes_init, - .exit = drm_test_modes_exit, .test_cases = drm_modes_analog_tv_tests, };
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.
Reviewed-by: Javier Martinez Canillas javierm@redhat.com Signed-off-by: Maxime Ripard mripard@kernel.org --- drivers/gpu/drm/tests/drm_probe_helper_test.c | 8 -------- 1 file changed, 8 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_probe_helper_test.c b/drivers/gpu/drm/tests/drm_probe_helper_test.c index 0ee65828623e..1a2044070a6c 100644 --- a/drivers/gpu/drm/tests/drm_probe_helper_test.c +++ b/drivers/gpu/drm/tests/drm_probe_helper_test.c @@ -60,13 +60,6 @@ static int drm_probe_helper_test_init(struct kunit *test) return 0; }
-static void drm_probe_helper_test_exit(struct kunit *test) -{ - struct drm_probe_helper_test_priv *priv = test->priv; - - drm_kunit_helper_free_device(test, priv->dev); -} - typedef struct drm_display_mode *(*expected_mode_func_t)(struct drm_device *);
struct drm_connector_helper_tv_get_modes_test { @@ -208,7 +201,6 @@ static struct kunit_case drm_test_connector_helper_tv_get_modes_tests[] = { static struct kunit_suite drm_test_connector_helper_tv_get_modes_suite = { .name = "drm_connector_helper_tv_get_modes", .init = drm_probe_helper_test_init, - .exit = drm_probe_helper_test_exit, .test_cases = drm_test_connector_helper_tv_get_modes_tests, };
As we get more and more tests, the locking context initialisation creates more and more boilerplate, both at creation and destruction.
Let's create a helper that will allocate, initialise a context, and register kunit actions to clean up once the test is done.
Reviewed-by: Javier Martinez Canillas javierm@redhat.com Signed-off-by: Maxime Ripard mripard@kernel.org --- drivers/gpu/drm/tests/drm_kunit_helpers.c | 41 +++++++++++++++++++++++++++++++ include/drm/drm_kunit_helpers.h | 2 ++ 2 files changed, 43 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c b/drivers/gpu/drm/tests/drm_kunit_helpers.c index 5856beb7f7d7..5130d4553262 100644 --- a/drivers/gpu/drm/tests/drm_kunit_helpers.c +++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c @@ -120,5 +120,46 @@ __drm_kunit_helper_alloc_drm_device_with_driver(struct kunit *test, } EXPORT_SYMBOL_GPL(__drm_kunit_helper_alloc_drm_device_with_driver);
+static void action_drm_release_context(void *ptr) +{ + struct drm_modeset_acquire_ctx *ctx = ptr; + + drm_modeset_drop_locks(ctx); + drm_modeset_acquire_fini(ctx); +} + +/** + * drm_kunit_helper_context_alloc - Allocates an acquire context + * @test: The test context object + * + * Allocates and initializes a modeset acquire context. + * + * The context is tied to the kunit test context, so we must not call + * drm_modeset_acquire_fini() on it, it will be done so automatically. + * + * Returns: + * An ERR_PTR on error, a pointer to the newly allocated context otherwise + */ +struct drm_modeset_acquire_ctx * +drm_kunit_helper_acquire_ctx_alloc(struct kunit *test) +{ + struct drm_modeset_acquire_ctx *ctx; + int ret; + + ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, ctx); + + drm_modeset_acquire_init(ctx, 0); + + ret = kunit_add_action_or_reset(test, + action_drm_release_context, + ctx); + if (ret) + return ERR_PTR(ret); + + return ctx; +} +EXPORT_SYMBOL_GPL(drm_kunit_helper_acquire_ctx_alloc); + MODULE_AUTHOR("Maxime Ripard maxime@cerno.tech"); MODULE_LICENSE("GPL"); diff --git a/include/drm/drm_kunit_helpers.h b/include/drm/drm_kunit_helpers.h index ed013fdcc1ff..4ba5e10653c6 100644 --- a/include/drm/drm_kunit_helpers.h +++ b/include/drm/drm_kunit_helpers.h @@ -87,5 +87,7 @@ __drm_kunit_helper_alloc_drm_device(struct kunit *test, sizeof(_type), \ offsetof(_type, _member), \ _feat)) +struct drm_modeset_acquire_ctx * +drm_kunit_helper_acquire_ctx_alloc(struct kunit *test);
#endif // DRM_KUNIT_HELPERS_H_
Hi Maxime,
kernel test robot noticed the following build warnings:
[auto build test WARNING on c58c49dd89324b18a812762a2bfa5a0458e4f252]
url: https://github.com/intel-lab-lkp/linux/commits/Maxime-Ripard/drm-tests-helpe... base: c58c49dd89324b18a812762a2bfa5a0458e4f252 patch link: https://lore.kernel.org/r/20230720-kms-kunit-actions-rework-v2-5-175017bd56a... patch subject: [PATCH v2 05/11] drm/tests: helpers: Create a helper to allocate a locking ctx config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20230720/202307202244.26VyeZKj-lkp@i...) compiler: alpha-linux-gcc (GCC) 12.3.0 reproduce: (https://download.01.org/0day-ci/archive/20230720/202307202244.26VyeZKj-lkp@i...)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202307202244.26VyeZKj-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/gpu/drm/tests/drm_kunit_helpers.c:145: warning: expecting prototype for drm_kunit_helper_context_alloc(). Prototype was for drm_kunit_helper_acquire_ctx_alloc() instead
vim +145 drivers/gpu/drm/tests/drm_kunit_helpers.c
130 131 /** 132 * drm_kunit_helper_context_alloc - Allocates an acquire context 133 * @test: The test context object 134 * 135 * Allocates and initializes a modeset acquire context. 136 * 137 * The context is tied to the kunit test context, so we must not call 138 * drm_modeset_acquire_fini() on it, it will be done so automatically. 139 * 140 * Returns: 141 * An ERR_PTR on error, a pointer to the newly allocated context otherwise 142 */ 143 struct drm_modeset_acquire_ctx * 144 drm_kunit_helper_acquire_ctx_alloc(struct kunit *test)
145 {
146 struct drm_modeset_acquire_ctx *ctx; 147 int ret; 148 149 ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL); 150 KUNIT_ASSERT_NOT_NULL(test, ctx); 151 152 drm_modeset_acquire_init(ctx, 0); 153 154 ret = kunit_add_action_or_reset(test, 155 action_drm_release_context, 156 ctx); 157 if (ret) 158 return ERR_PTR(ret); 159 160 return ctx; 161 } 162 EXPORT_SYMBOL_GPL(drm_kunit_helper_acquire_ctx_alloc); 163
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 a helper to create that atomic state, and call an action when the test is done. This will also clean up the exit path.
Reviewed-by: Javier Martinez Canillas javierm@redhat.com Signed-off-by: Maxime Ripard mripard@kernel.org --- 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 5130d4553262..d3b2c6b48163 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> @@ -161,5 +162,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 maxime@cerno.tech"); 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_
Hi Maxime,
kernel test robot noticed the following build warnings:
[auto build test WARNING on c58c49dd89324b18a812762a2bfa5a0458e4f252]
url: https://github.com/intel-lab-lkp/linux/commits/Maxime-Ripard/drm-tests-helpe... base: c58c49dd89324b18a812762a2bfa5a0458e4f252 patch link: https://lore.kernel.org/r/20230720-kms-kunit-actions-rework-v2-6-175017bd56a... patch subject: [PATCH v2 06/11] drm/tests: helpers: Create a helper to allocate an atomic state config: arm64-randconfig-r022-20230720 (https://download.01.org/0day-ci/archive/20230721/202307210124.Ur3UNuxZ-lkp@i...) compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a) reproduce: (https://download.01.org/0day-ci/archive/20230721/202307210124.Ur3UNuxZ-lkp@i...)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202307210124.Ur3UNuxZ-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/gpu/drm/tests/drm_kunit_helpers.c:54:6: warning: cast from 'void (*)(struct platform_driver *)' to 'kunit_action_t *' (aka 'void (*)(void *)') converts to incompatible function type [-Wcast-function-type-strict] 54 | (kunit_action_t *)platform_driver_unregister, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/tests/drm_kunit_helpers.c:62:6: warning: cast from 'void (*)(struct platform_device *)' to 'kunit_action_t *' (aka 'void (*)(void *)') converts to incompatible function type [-Wcast-function-type-strict] 62 | (kunit_action_t *)platform_device_put, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/tests/drm_kunit_helpers.c:70:6: warning: cast from 'void (*)(struct platform_device *)' to 'kunit_action_t *' (aka 'void (*)(void *)') converts to incompatible function type [-Wcast-function-type-strict] 70 | (kunit_action_t *)platform_device_del, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/tests/drm_kunit_helpers.c:90:9: warning: cast from 'void (*)(struct platform_device *)' to 'kunit_action_t *' (aka 'void (*)(void *)') converts to incompatible function type [-Wcast-function-type-strict] 90 | (kunit_action_t *)platform_device_unregister, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/tests/drm_kunit_helpers.c:94:9: warning: cast from 'void (*)(struct platform_driver *)' to 'kunit_action_t *' (aka 'void (*)(void *)') converts to incompatible function type [-Wcast-function-type-strict] 94 | (kunit_action_t *)platform_driver_unregister, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/tests/drm_kunit_helpers.c:192:6: warning: cast from 'void (*)(struct drm_atomic_state *)' to 'kunit_action_t *' (aka 'void (*)(void *)') converts to incompatible function type [-Wcast-function-type-strict]
192 | (kunit_action_t *)drm_atomic_state_put, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 6 warnings generated.
vim +192 drivers/gpu/drm/tests/drm_kunit_helpers.c
164 165 /** 166 * drm_kunit_helper_atomic_state_alloc - Allocates an atomic state 167 * @test: The test context object 168 * @drm: The device to alloc the state for 169 * @ctx: Locking context for that atomic update 170 * 171 * Allocates a empty atomic state. 172 * 173 * The state is tied to the kunit test context, so we must not call 174 * drm_atomic_state_put() on it, it will be done so automatically. 175 * 176 * Returns: 177 * An ERR_PTR on error, a pointer to the newly allocated state otherwise 178 */ 179 struct drm_atomic_state * 180 drm_kunit_helper_atomic_state_alloc(struct kunit *test, 181 struct drm_device *drm, 182 struct drm_modeset_acquire_ctx *ctx) 183 { 184 struct drm_atomic_state *state; 185 int ret; 186 187 state = drm_atomic_state_alloc(drm); 188 if (!state) 189 return ERR_PTR(-ENOMEM); 190 191 ret = kunit_add_action_or_reset(test,
192 (kunit_action_t *)drm_atomic_state_put,
193 state); 194 if (ret) 195 return ERR_PTR(ret); 196 197 state->acquire_ctx = ctx; 198 199 return state; 200 } 201 EXPORT_SYMBOL_GPL(drm_kunit_helper_atomic_state_alloc); 202
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.
Reviewed-by: Javier Martinez Canillas javierm@redhat.com Signed-off-by: Maxime Ripard mripard@kernel.org --- 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[] = {
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.
Reviewed-by: Javier Martinez Canillas javierm@redhat.com Signed-off-by: Maxime Ripard mripard@kernel.org --- 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[] = {
The new helper to init the locking context allows to remove some boilerplate.
Reviewed-by: Javier Martinez Canillas javierm@redhat.com Signed-off-by: Maxime Ripard mripard@kernel.org --- drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c | 42 ++++++++++++-------------- 1 file changed, 19 insertions(+), 23 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 776a7b01608f..ff1deaed0cab 100644 --- a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c +++ b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c @@ -20,7 +20,6 @@
struct pv_muxing_priv { struct vc4_dev *vc4; - struct drm_modeset_acquire_ctx ctx; struct drm_atomic_state *state; };
@@ -725,6 +724,7 @@ static void drm_vc4_test_pv_muxing_invalid(struct kunit *test) 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; @@ -738,13 +738,14 @@ static int vc4_pv_muxing_test_init(struct kunit *test) KUNIT_ASSERT_NOT_ERR_OR_NULL(test, vc4); priv->vc4 = vc4;
- drm_modeset_acquire_init(&priv->ctx, 0); + ctx = drm_kunit_helper_acquire_ctx_alloc(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 = &priv->ctx; + state->acquire_ctx = ctx;
priv->state = state;
@@ -757,8 +758,6 @@ static void vc4_pv_muxing_test_exit(struct kunit *test) struct drm_atomic_state *state = priv->state;
drm_atomic_state_put(state); - drm_modeset_drop_locks(&priv->ctx); - drm_modeset_acquire_fini(&priv->ctx); }
static struct kunit_case vc4_pv_muxing_tests[] = { @@ -798,7 +797,7 @@ static struct kunit_suite vc5_pv_muxing_test_suite = { */ static void drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable(struct kunit *test) { - struct drm_modeset_acquire_ctx ctx; + struct drm_modeset_acquire_ctx *ctx; struct drm_atomic_state *state; struct vc4_crtc_state *new_vc4_crtc_state; struct vc4_hvs_state *new_hvs_state; @@ -811,13 +810,14 @@ static void drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable(struct kunit *tes vc4 = vc5_mock_device(test); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, vc4);
- drm_modeset_acquire_init(&ctx, 0); + ctx = drm_kunit_helper_acquire_ctx_alloc(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; + state->acquire_ctx = ctx;
ret = vc4_mock_atomic_add_output(test, state, VC4_ENCODER_TYPE_HDMI0); KUNIT_ASSERT_EQ(test, ret, 0); @@ -844,7 +844,7 @@ static void drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable(struct kunit *tes state = drm_atomic_state_alloc(drm); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
- state->acquire_ctx = &ctx; + state->acquire_ctx = ctx;
ret = vc4_mock_atomic_add_output(test, state, VC4_ENCODER_TYPE_HDMI1); KUNIT_ASSERT_EQ(test, ret, 0); @@ -866,13 +866,11 @@ static void drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable(struct kunit *tes KUNIT_EXPECT_NE(test, hdmi0_channel, hdmi1_channel);
drm_atomic_state_put(state); - drm_modeset_drop_locks(&ctx); - drm_modeset_acquire_fini(&ctx); }
static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test) { - struct drm_modeset_acquire_ctx ctx; + struct drm_modeset_acquire_ctx *ctx; struct drm_atomic_state *state; struct vc4_crtc_state *new_vc4_crtc_state; struct vc4_hvs_state *new_hvs_state; @@ -885,13 +883,14 @@ static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test) vc4 = vc5_mock_device(test); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, vc4);
- drm_modeset_acquire_init(&ctx, 0); + ctx = drm_kunit_helper_acquire_ctx_alloc(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; + state->acquire_ctx = ctx;
ret = vc4_mock_atomic_add_output(test, state, VC4_ENCODER_TYPE_HDMI0); KUNIT_ASSERT_EQ(test, ret, 0); @@ -929,7 +928,7 @@ static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test) state = drm_atomic_state_alloc(drm); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
- state->acquire_ctx = &ctx; + state->acquire_ctx = ctx;
ret = vc4_mock_atomic_del_output(test, state, VC4_ENCODER_TYPE_HDMI0); KUNIT_ASSERT_EQ(test, ret, 0); @@ -954,14 +953,12 @@ 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); }
static void drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable_too_many_crtc_state(struct kunit *test) { - struct drm_modeset_acquire_ctx ctx; + struct drm_modeset_acquire_ctx *ctx; struct drm_atomic_state *state; struct vc4_crtc_state *new_vc4_crtc_state; struct drm_device *drm; @@ -971,13 +968,14 @@ drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable_too_many_crtc_state(struct ku vc4 = vc5_mock_device(test); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, vc4);
- drm_modeset_acquire_init(&ctx, 0); + ctx = drm_kunit_helper_acquire_ctx_alloc(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; + state->acquire_ctx = ctx;
ret = vc4_mock_atomic_add_output(test, state, VC4_ENCODER_TYPE_HDMI0); KUNIT_ASSERT_EQ(test, ret, 0); @@ -993,7 +991,7 @@ drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable_too_many_crtc_state(struct ku state = drm_atomic_state_alloc(drm); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
- state->acquire_ctx = &ctx; + state->acquire_ctx = ctx;
ret = vc4_mock_atomic_add_output(test, state, VC4_ENCODER_TYPE_HDMI1); KUNIT_ASSERT_EQ(test, ret, 0); @@ -1006,8 +1004,6 @@ drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable_too_many_crtc_state(struct ku KUNIT_EXPECT_NULL(test, new_vc4_crtc_state);
drm_atomic_state_put(state); - drm_modeset_drop_locks(&ctx); - drm_modeset_acquire_fini(&ctx); }
static struct kunit_case vc5_pv_muxing_bugs_tests[] = {
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.
Reviewed-by: Javier Martinez Canillas javierm@redhat.com Signed-off-by: Maxime Ripard mripard@kernel.org --- 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[] = {
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.
Reviewed-by: Javier Martinez Canillas javierm@redhat.com Signed-off-by: Maxime Ripard mripard@kernel.org --- 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) {
Hi Maxime,
On 7/20/23 08:15, 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 where relevant.
Let me know what you think,
With the problems pointed out by kernel test bot fixed, the whole series is:
Reviewed-by: Maíra Canal mairacanal@riseup.net
Best Regards, - Maíra
Maxime
Signed-off-by: Maxime Ripard mripard@kernel.org
Changes in v2:
- Fix some typos
- Use plaltform_device_del instead of removing the call to platform_device_put after calling platform_device_add
- Link to v1: https://lore.kernel.org/r/20230710-kms-kunit-actions-rework-v1-0-722c58d72c7...
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 a helper to allocate a locking ctx drm/tests: helpers: Create a 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 | 108 +++++++++++++++++++++- 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, 158 insertions(+), 101 deletions(-)
base-commit: c58c49dd89324b18a812762a2bfa5a0458e4f252 change-id: 20230710-kms-kunit-actions-rework-5d163762c93b
Best regards,
On Thu, 20 Jul 2023 13:15:45 +0200, Maxime Ripard wrote:
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 where relevant.
Let me know what you think, Maxime
[...]
Applied to drm/drm-misc (drm-misc-next).
Thanks! Maxime
linux-kselftest-mirror@lists.linaro.org
-
kernel test robot -
Maira Canal -
Maxime Ripard