[WHAT] Add KUnit tests for the failure paths of amdgpu_dm_build_default_pipeline(): a failed allocation and a failed initialization at each of the eight pipeline steps.
[HOW] Add struct amdgpu_dm_colorop_kunit_ops, a KUnit-only indirection table for the colorop allocation and the five DRM colorop constructors. The constructors cannot be made to fail through their arguments, so tests install a fake allocator and spies that delegate to the real constructors and override the return value of one selected call. Non-KUnit builds call the same functions directly. Assisted-by: Copilot:Claude-Opus-5 Reviewed-by: Bhawanpreet Lakha <[email protected]> Signed-off-by: Alex Hung <[email protected]> --- .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 110 ++++++--- .../amd/display/amdgpu_dm/amdgpu_dm_colorop.h | 28 +++ .../amdgpu_dm/tests/amdgpu_dm_colorop_test.c | 232 ++++++++++++++++-- 3 files changed, 312 insertions(+), 58 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c index fae9007b9998..d7260518a23e 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c @@ -63,6 +63,48 @@ static const struct drm_colorop_funcs dm_colorop_funcs = { .destroy = drm_colorop_destroy, }; +#if IS_ENABLED(CONFIG_DRM_AMD_DC_KUNIT_TEST) +static struct drm_colorop *amdgpu_dm_colorop_kzalloc(void) +{ + return kzalloc_obj(struct drm_colorop); +} + +static const struct amdgpu_dm_colorop_kunit_ops amdgpu_dm_colorop_default_ops = { + .colorop_kzalloc_obj = amdgpu_dm_colorop_kzalloc, + .curve_1d_init = drm_plane_colorop_curve_1d_init, + .curve_1d_lut_init = drm_plane_colorop_curve_1d_lut_init, + .ctm_3x4_init = drm_plane_colorop_ctm_3x4_init, + .mult_init = drm_plane_colorop_mult_init, + .lut3d_init = drm_plane_colorop_3dlut_init, +}; + +static const struct amdgpu_dm_colorop_kunit_ops *amdgpu_dm_colorop_ops = + &amdgpu_dm_colorop_default_ops; + +void amdgpu_dm_colorop_kunit_set_ops(const struct amdgpu_dm_colorop_kunit_ops *ops) +{ + amdgpu_dm_colorop_ops = ops ? ops : &amdgpu_dm_colorop_default_ops; +} +EXPORT_IF_KUNIT(amdgpu_dm_colorop_kunit_set_ops); + +#define colorop_kzalloc_obj amdgpu_dm_colorop_ops->colorop_kzalloc_obj +#define colorop_curve_1d_init amdgpu_dm_colorop_ops->curve_1d_init +#define colorop_curve_1d_lut_init amdgpu_dm_colorop_ops->curve_1d_lut_init +#define colorop_ctm_3x4_init amdgpu_dm_colorop_ops->ctm_3x4_init +#define colorop_mult_init amdgpu_dm_colorop_ops->mult_init +#define colorop_3dlut_init amdgpu_dm_colorop_ops->lut3d_init + +#else + +#define colorop_kzalloc_obj() kzalloc_obj(struct drm_colorop) +#define colorop_curve_1d_init drm_plane_colorop_curve_1d_init +#define colorop_curve_1d_lut_init drm_plane_colorop_curve_1d_lut_init +#define colorop_ctm_3x4_init drm_plane_colorop_ctm_3x4_init +#define colorop_mult_init drm_plane_colorop_mult_init +#define colorop_3dlut_init drm_plane_colorop_3dlut_init + +#endif + STATIC_IFN_KUNIT int amdgpu_dm_build_default_pipeline(struct drm_device *dev, struct drm_plane *plane, bool hw_3d_lut, struct drm_prop_enum_list *list) @@ -74,15 +116,15 @@ amdgpu_dm_build_default_pipeline(struct drm_device *dev, struct drm_plane *plane memset(ops, 0, sizeof(ops)); /* 1D curve - DEGAM TF */ - ops[i] = kzalloc_obj(*ops[0]); + ops[i] = colorop_kzalloc_obj(); if (!ops[i]) { ret = -ENOMEM; goto cleanup; } - ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, &dm_colorop_funcs, - amdgpu_dm_supported_degam_tfs, - DRM_COLOROP_FLAG_ALLOW_BYPASS); + ret = colorop_curve_1d_init(dev, ops[i], plane, &dm_colorop_funcs, + amdgpu_dm_supported_degam_tfs, + DRM_COLOROP_FLAG_ALLOW_BYPASS); if (ret) goto cleanup; @@ -91,14 +133,14 @@ amdgpu_dm_build_default_pipeline(struct drm_device *dev, struct drm_plane *plane i++; /* Multiplier */ - ops[i] = kzalloc_obj(struct drm_colorop); + ops[i] = colorop_kzalloc_obj(); if (!ops[i]) { ret = -ENOMEM; goto cleanup; } - ret = drm_plane_colorop_mult_init(dev, ops[i], plane, &dm_colorop_funcs, - DRM_COLOROP_FLAG_ALLOW_BYPASS); + ret = colorop_mult_init(dev, ops[i], plane, &dm_colorop_funcs, + DRM_COLOROP_FLAG_ALLOW_BYPASS); if (ret) goto cleanup; @@ -107,15 +149,14 @@ amdgpu_dm_build_default_pipeline(struct drm_device *dev, struct drm_plane *plane i++; /* 3x4 matrix */ - ops[i] = kzalloc_obj(struct drm_colorop); + ops[i] = colorop_kzalloc_obj(); if (!ops[i]) { ret = -ENOMEM; goto cleanup; } - ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, - &dm_colorop_funcs, - DRM_COLOROP_FLAG_ALLOW_BYPASS); + ret = colorop_ctm_3x4_init(dev, ops[i], plane, &dm_colorop_funcs, + DRM_COLOROP_FLAG_ALLOW_BYPASS); if (ret) goto cleanup; @@ -125,15 +166,15 @@ amdgpu_dm_build_default_pipeline(struct drm_device *dev, struct drm_plane *plane if (hw_3d_lut) { /* 1D curve - SHAPER TF */ - ops[i] = kzalloc_obj(*ops[0]); + ops[i] = colorop_kzalloc_obj(); if (!ops[i]) { ret = -ENOMEM; goto cleanup; } - ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, &dm_colorop_funcs, - amdgpu_dm_supported_shaper_tfs, - DRM_COLOROP_FLAG_ALLOW_BYPASS); + ret = colorop_curve_1d_init(dev, ops[i], plane, &dm_colorop_funcs, + amdgpu_dm_supported_shaper_tfs, + DRM_COLOROP_FLAG_ALLOW_BYPASS); if (ret) goto cleanup; @@ -142,17 +183,16 @@ amdgpu_dm_build_default_pipeline(struct drm_device *dev, struct drm_plane *plane i++; /* 1D LUT - SHAPER LUT */ - ops[i] = kzalloc_obj(*ops[0]); + ops[i] = colorop_kzalloc_obj(); if (!ops[i]) { ret = -ENOMEM; goto cleanup; } - ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, - &dm_colorop_funcs, - MAX_COLOR_LUT_ENTRIES, - DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR, - DRM_COLOROP_FLAG_ALLOW_BYPASS); + ret = colorop_curve_1d_lut_init(dev, ops[i], plane, &dm_colorop_funcs, + MAX_COLOR_LUT_ENTRIES, + DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR, + DRM_COLOROP_FLAG_ALLOW_BYPASS); if (ret) goto cleanup; @@ -161,16 +201,16 @@ amdgpu_dm_build_default_pipeline(struct drm_device *dev, struct drm_plane *plane i++; /* 3D LUT */ - ops[i] = kzalloc_obj(*ops[0]); + ops[i] = colorop_kzalloc_obj(); if (!ops[i]) { ret = -ENOMEM; goto cleanup; } - ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane, - &dm_colorop_funcs, LUT3D_SIZE, - DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL, - DRM_COLOROP_FLAG_ALLOW_BYPASS); + ret = colorop_3dlut_init(dev, ops[i], plane, &dm_colorop_funcs, + LUT3D_SIZE, + DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL, + DRM_COLOROP_FLAG_ALLOW_BYPASS); if (ret) goto cleanup; @@ -180,15 +220,15 @@ amdgpu_dm_build_default_pipeline(struct drm_device *dev, struct drm_plane *plane } /* 1D curve - BLND TF */ - ops[i] = kzalloc_obj(*ops[0]); + ops[i] = colorop_kzalloc_obj(); if (!ops[i]) { ret = -ENOMEM; goto cleanup; } - ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, &dm_colorop_funcs, - amdgpu_dm_supported_blnd_tfs, - DRM_COLOROP_FLAG_ALLOW_BYPASS); + ret = colorop_curve_1d_init(dev, ops[i], plane, &dm_colorop_funcs, + amdgpu_dm_supported_blnd_tfs, + DRM_COLOROP_FLAG_ALLOW_BYPASS); if (ret) goto cleanup; @@ -197,16 +237,16 @@ amdgpu_dm_build_default_pipeline(struct drm_device *dev, struct drm_plane *plane i++; /* 1D LUT - BLND LUT */ - ops[i] = kzalloc_obj(struct drm_colorop); + ops[i] = colorop_kzalloc_obj(); if (!ops[i]) { ret = -ENOMEM; goto cleanup; } - ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, &dm_colorop_funcs, - MAX_COLOR_LUT_ENTRIES, - DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR, - DRM_COLOROP_FLAG_ALLOW_BYPASS); + ret = colorop_curve_1d_lut_init(dev, ops[i], plane, &dm_colorop_funcs, + MAX_COLOR_LUT_ENTRIES, + DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR, + DRM_COLOROP_FLAG_ALLOW_BYPASS); if (ret) goto cleanup; diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h index 77364d954d3b..ff46c4f7b126 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h @@ -34,8 +34,36 @@ extern const u64 amdgpu_dm_supported_blnd_tfs; int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_prop_enum_list *list); #if IS_ENABLED(CONFIG_DRM_AMD_DC_KUNIT_TEST) +#include <drm/drm_colorop.h> + +struct amdgpu_dm_colorop_kunit_ops { + struct drm_colorop *(*colorop_kzalloc_obj)(void); + int (*curve_1d_init)(struct drm_device *dev, struct drm_colorop *colorop, + struct drm_plane *plane, const struct drm_colorop_funcs *funcs, + u64 supported_tfs, uint32_t flags); + int (*curve_1d_lut_init)(struct drm_device *dev, struct drm_colorop *colorop, + struct drm_plane *plane, + const struct drm_colorop_funcs *funcs, + uint32_t lut_size, + enum drm_colorop_lut1d_interpolation_type interpolation, + uint32_t flags); + int (*ctm_3x4_init)(struct drm_device *dev, struct drm_colorop *colorop, + struct drm_plane *plane, const struct drm_colorop_funcs *funcs, + uint32_t flags); + int (*mult_init)(struct drm_device *dev, struct drm_colorop *colorop, + struct drm_plane *plane, const struct drm_colorop_funcs *funcs, + uint32_t flags); + int (*lut3d_init)(struct drm_device *dev, struct drm_colorop *colorop, + struct drm_plane *plane, + const struct drm_colorop_funcs *funcs, + uint32_t lut_size, + enum drm_colorop_lut3d_interpolation_type interpolation, + uint32_t flags); +}; + int amdgpu_dm_build_default_pipeline(struct drm_device *dev, struct drm_plane *plane, bool hw_3d_lut, struct drm_prop_enum_list *list); +void amdgpu_dm_colorop_kunit_set_ops(const struct amdgpu_dm_colorop_kunit_ops *ops); #endif #endif /* __AMDGPU_DM_COLOROP_H__*/ diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_colorop_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_colorop_test.c index 2e557ff66818..90cc5592ef5d 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_colorop_test.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_colorop_test.c @@ -136,6 +136,30 @@ static void kunit_colorop_pipeline_destroy(void *drm) drm_colorop_pipeline_destroy((struct drm_device *)drm); } +/* + * A plain drm_device (not an amdgpu_device) is enough: building the pipeline + * only needs the DRM mode-config infrastructure. The pipeline is destroyed + * before the DRM device so the colorops are freed while the device is valid. + */ +static struct drm_plane *dm_colorop_alloc_plane(struct kunit *test, struct drm_device **drm) +{ + struct drm_plane *plane; + struct device *dev; + + dev = drm_kunit_helper_alloc_device(test); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev); + + *drm = __drm_kunit_helper_alloc_drm_device(test, dev, sizeof(**drm), 0, 0); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, *drm); + + plane = drm_kunit_helper_create_primary_plane(test, *drm, NULL, NULL, NULL, 0, NULL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, plane); + + kunit_add_action(test, kunit_colorop_pipeline_destroy, *drm); + + return plane; +} + static void dm_expect_colorop_pipeline(struct kunit *test, struct drm_device *drm, const struct drm_prop_enum_list *list, const enum drm_colorop_type *expected, @@ -177,34 +201,12 @@ static void dm_test_initialize_default_pipeline(struct kunit *test) DRM_COLOROP_1D_CURVE, /* blnd TF */ DRM_COLOROP_1D_LUT, /* blnd LUT */ }; - struct device *dev; struct drm_device *drm; struct drm_plane *plane; struct drm_prop_enum_list list = {}; int ret; - dev = drm_kunit_helper_alloc_device(test); - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev); - - /* - * Allocate a plain drm_device (not an amdgpu_device) — sufficient - * because amdgpu_dm_build_default_pipeline() only needs the DRM - * mode-config infrastructure, not the amdgpu device wrapper. - */ - drm = __drm_kunit_helper_alloc_drm_device(test, dev, - sizeof(*drm), 0, 0); - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, drm); - - plane = drm_kunit_helper_create_primary_plane(test, drm, - NULL, NULL, NULL, 0, NULL); - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, plane); - - /* - * Destroy the pipeline before the DRM device is cleaned up so - * that the colorop objects (kzalloc'd inside the function) are - * freed while the device is still valid. - */ - kunit_add_action(test, kunit_colorop_pipeline_destroy, drm); + plane = dm_colorop_alloc_plane(test, &drm); ret = amdgpu_dm_build_default_pipeline(drm, plane, true, &list); KUNIT_ASSERT_EQ(test, ret, 0); @@ -308,6 +310,187 @@ static void dm_test_initialize_default_pipeline_no_3d_lut(struct kunit *test) expected, ARRAY_SIZE(expected)); } +/* Failure paths of amdgpu_dm_build_default_pipeline() */ + +/* + * The DRM colorop constructors cannot be made to fail through their arguments, + * so failures are injected with a fake allocator and with spies that delegate + * to the real constructors and override the return value of one call. + */ +struct dm_colorop_fault { + int alloc_fail_at; /* allocation index returning NULL, -1 to disable */ + int init_fail_at; /* init call index returning an error, -1 to disable */ + int allocs; + int inits; +}; + +static struct dm_colorop_fault dm_colorop_fault; + +static struct drm_colorop *dm_colorop_test_alloc(void) +{ + if (dm_colorop_fault.allocs++ == dm_colorop_fault.alloc_fail_at) + return NULL; + + return kzalloc_obj(struct drm_colorop); +} + +static int dm_colorop_test_init_ret(int ret) +{ + if (ret) + return ret; + + if (dm_colorop_fault.inits++ == dm_colorop_fault.init_fail_at) + return -EINVAL; + + return 0; +} + +static int dm_colorop_test_curve_1d_init(struct drm_device *dev, struct drm_colorop *colorop, + struct drm_plane *plane, + const struct drm_colorop_funcs *funcs, + u64 supported_tfs, uint32_t flags) +{ + return dm_colorop_test_init_ret(drm_plane_colorop_curve_1d_init(dev, colorop, plane, + funcs, supported_tfs, + flags)); +} + +static int dm_colorop_test_1d_lut_init(struct drm_device *dev, struct drm_colorop *colorop, + struct drm_plane *plane, + const struct drm_colorop_funcs *funcs, + uint32_t lut_size, + enum drm_colorop_lut1d_interpolation_type interpolation, + uint32_t flags) +{ + return dm_colorop_test_init_ret(drm_plane_colorop_curve_1d_lut_init(dev, colorop, plane, + funcs, lut_size, + interpolation, + flags)); +} + +static int dm_colorop_test_ctm_3x4_init(struct drm_device *dev, struct drm_colorop *colorop, + struct drm_plane *plane, + const struct drm_colorop_funcs *funcs, uint32_t flags) +{ + return dm_colorop_test_init_ret(drm_plane_colorop_ctm_3x4_init(dev, colorop, plane, + funcs, flags)); +} + +static int dm_colorop_test_mult_init(struct drm_device *dev, struct drm_colorop *colorop, + struct drm_plane *plane, + const struct drm_colorop_funcs *funcs, uint32_t flags) +{ + return dm_colorop_test_init_ret(drm_plane_colorop_mult_init(dev, colorop, plane, + funcs, flags)); +} + +static int dm_colorop_test_3dlut_init(struct drm_device *dev, struct drm_colorop *colorop, + struct drm_plane *plane, + const struct drm_colorop_funcs *funcs, uint32_t lut_size, + enum drm_colorop_lut3d_interpolation_type interpolation, + uint32_t flags) +{ + return dm_colorop_test_init_ret(drm_plane_colorop_3dlut_init(dev, colorop, plane, funcs, + lut_size, interpolation, + flags)); +} + +static const struct amdgpu_dm_colorop_kunit_ops dm_colorop_test_ops = { + .colorop_kzalloc_obj = dm_colorop_test_alloc, + .curve_1d_init = dm_colorop_test_curve_1d_init, + .curve_1d_lut_init = dm_colorop_test_1d_lut_init, + .ctm_3x4_init = dm_colorop_test_ctm_3x4_init, + .mult_init = dm_colorop_test_mult_init, + .lut3d_init = dm_colorop_test_3dlut_init, +}; + +static void dm_colorop_test_reset_ops(void *unused) +{ + amdgpu_dm_colorop_kunit_set_ops(NULL); +} + +/* + * Build the full (3D LUT) pipeline with one injected failure and return the + * error reported by amdgpu_dm_build_default_pipeline(). + */ +static int dm_colorop_build_with_fault(struct kunit *test, struct drm_device **drm, + int alloc_fail_at, int init_fail_at) +{ + struct drm_prop_enum_list list = {}; + struct drm_plane *plane; + int ret; + + plane = dm_colorop_alloc_plane(test, drm); + + dm_colorop_fault = (struct dm_colorop_fault){ + .alloc_fail_at = alloc_fail_at, + .init_fail_at = init_fail_at, + }; + + amdgpu_dm_colorop_kunit_set_ops(&dm_colorop_test_ops); + KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, dm_colorop_test_reset_ops, NULL), 0); + + ret = amdgpu_dm_build_default_pipeline(*drm, plane, true, &list); + kfree(list.name); + + return ret; +} + +struct dm_colorop_step_param { + const char *name; + int index; +}; + +/* Pipeline steps in creation order when the 3D LUT block is built. */ +static const struct dm_colorop_step_param dm_colorop_step_params[] = { + { "degam_tf", 0 }, + { "multiplier", 1 }, + { "ctm_3x4", 2 }, + { "shaper_tf", 3 }, + { "shaper_lut", 4 }, + { "lut3d", 5 }, + { "blnd_tf", 6 }, + { "blnd_lut", 7 }, +}; + +KUNIT_ARRAY_PARAM_DESC(dm_colorop_step, dm_colorop_step_params, name); + +/** + * dm_test_build_pipeline_alloc_fail() - Verify a failed alloc returns -ENOMEM and cleans up. + * @test: KUnit test context. + */ +static void dm_test_build_pipeline_alloc_fail(struct kunit *test) +{ + const struct dm_colorop_step_param *param = test->param_value; + struct drm_device *drm; + int ret; + + ret = dm_colorop_build_with_fault(test, &drm, param->index, -1); + + KUNIT_EXPECT_EQ(test, ret, -ENOMEM); + KUNIT_EXPECT_EQ(test, dm_colorop_fault.allocs, param->index + 1); + KUNIT_EXPECT_EQ(test, dm_colorop_fault.inits, param->index); + KUNIT_EXPECT_EQ(test, drm->mode_config.num_colorop, 0); +} + +/** + * dm_test_build_pipeline_init_fail() - Verify a failed init propagates the error and cleans up. + * @test: KUnit test context. + */ +static void dm_test_build_pipeline_init_fail(struct kunit *test) +{ + const struct dm_colorop_step_param *param = test->param_value; + struct drm_device *drm; + int ret; + + ret = dm_colorop_build_with_fault(test, &drm, -1, param->index); + + KUNIT_EXPECT_EQ(test, ret, -EINVAL); + KUNIT_EXPECT_EQ(test, dm_colorop_fault.allocs, param->index + 1); + KUNIT_EXPECT_EQ(test, dm_colorop_fault.inits, param->index + 1); + KUNIT_EXPECT_EQ(test, drm->mode_config.num_colorop, 0); +} + static struct kunit_case dm_colorop_test_cases[] = { /* degam TFs */ KUNIT_CASE(dm_test_supported_degam_tfs_has_srgb_eotf), @@ -334,6 +517,9 @@ static struct kunit_case dm_colorop_test_cases[] = { KUNIT_CASE(dm_test_initialize_default_pipeline_dpp_3d_lut), KUNIT_CASE(dm_test_initialize_default_pipeline_mpc_preblend), KUNIT_CASE(dm_test_initialize_default_pipeline_no_3d_lut), + /* amdgpu_dm_build_default_pipeline failure paths */ + KUNIT_CASE_PARAM(dm_test_build_pipeline_alloc_fail, dm_colorop_step_gen_params), + KUNIT_CASE_PARAM(dm_test_build_pipeline_init_fail, dm_colorop_step_gen_params), {} }; -- 2.43.0
