From: Alex Hung <[email protected]>
[WHAT]
Add KUnit tests for dm_dmub_sw_init(), covering the ASIC to DMUB service
mapping for all 19 supported IP versions, the memory domain choice, a
complete successful init, the BSS data region variant and the PSP
firmware load registration.
[HOW]
The framebuffer allocation needs a live TTM device, so add
amdgpu_bo_create_kernel() to the indirection table introduced for the
firmware request. The ASIC mapping and memory domain tests force that
allocation to fail, which walks every ASIC through DMUB service creation
and region calculation without a fake framebuffer.
Grow the fake firmware so its instruction constant region is larger than
two PSP footers. The footer probing in
dmub_srv_get_fw_meta_info_from_raw_fw() walks back from the end of that
region twice, so a region smaller than the footers underflows the size
and reads out of bounds.
Assisted-by: Copilot:Claude-Opus-5
Reviewed-by: Bhawanpreet Lakha <[email protected]>
Signed-off-by: Alex Hung <[email protected]>
Signed-off-by: Ivan Lipski <[email protected]>
---
.../amd/display/amdgpu_dm/amdgpu_dm_dmub.c | 13 +-
.../amd/display/amdgpu_dm/amdgpu_dm_dmub.h | 4 +
.../amdgpu_dm/tests/amdgpu_dm_dmub_test.c | 185 +++++++++++++++++-
3 files changed, 193 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_dmub.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_dmub.c
index 2b34d5c36b48d..f4b0ced7d391c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_dmub.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_dmub.c
@@ -67,6 +67,7 @@ MODULE_FIRMWARE(FIRMWARE_DCN_60_DMUB);
#if IS_ENABLED(CONFIG_DRM_AMD_DC_KUNIT_TEST)
static const struct amdgpu_dm_dmub_kunit_ops amdgpu_dm_dmub_default_ops = {
+ .bo_create_kernel = amdgpu_bo_create_kernel,
.ucode_request = amdgpu_ucode_request,
};
@@ -79,10 +80,12 @@ void amdgpu_dm_dmub_kunit_set_ops(const struct amdgpu_dm_dmub_kunit_ops *ops)
}
EXPORT_IF_KUNIT(amdgpu_dm_dmub_kunit_set_ops);
+#define dmub_bo_create_kernel amdgpu_dm_dmub_ops->bo_create_kernel
#define dmub_ucode_request amdgpu_dm_dmub_ops->ucode_request
#else
+#define dmub_bo_create_kernel amdgpu_bo_create_kernel
#define dmub_ucode_request amdgpu_ucode_request
#endif
@@ -683,11 +686,11 @@ int dm_dmub_sw_init(struct amdgpu_device *adev)
* Allocate a framebuffer based on the total size of all the regions.
* TODO: Move this into GART.
*/
- r = amdgpu_bo_create_kernel(adev, region_info.fb_size, PAGE_SIZE,
- mem_domain,
- &adev->dm.dmub_bo,
- &adev->dm.dmub_bo_gpu_addr,
- &adev->dm.dmub_bo_cpu_addr);
+ r = dmub_bo_create_kernel(adev, region_info.fb_size, PAGE_SIZE,
+ mem_domain,
+ &adev->dm.dmub_bo,
+ &adev->dm.dmub_bo_gpu_addr,
+ &adev->dm.dmub_bo_cpu_addr);
if (r)
return r;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_dmub.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_dmub.h
index d178e1bf4dd6b..df3e776c1ad14 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_dmub.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_dmub.h
@@ -78,6 +78,10 @@ void amdgpu_dm_dmub_reg_write(void *ctx, uint32_t address, uint32_t value);
/* Signatures are verbatim copies so an upstream change breaks the default ops. */
struct amdgpu_dm_dmub_kunit_ops {
+ int (*bo_create_kernel)(struct amdgpu_device *adev,
+ unsigned long size, int align,
+ u32 domain, struct amdgpu_bo **bo_ptr,
+ u64 *gpu_addr, void **cpu_addr);
__printf(4, 5)
int (*ucode_request)(struct amdgpu_device *adev, const struct firmware **fw,
enum amdgpu_ucode_required required, const char *fmt, ...);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_dmub_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_dmub_test.c
index fa81413be05f5..1e499acbcb908 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_dmub_test.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_dmub_test.c
@@ -19,7 +19,13 @@
#include "dmub/dmub_srv.h"
#include "amdgpu_dm_dmub.h"
-#define DM_TEST_FW_SIZE 512
+/*
+ * The PSP footer probing in dmub_srv_get_fw_meta_info_from_raw_fw() walks back
+ * from the end of the instruction constant region twice, so the fake firmware
+ * needs an instruction constant region larger than two footers.
+ */
+#define DM_TEST_FW_INST_CONST_BYTES (PSP_HEADER_BYTES_256 + 1024)
+#define DM_TEST_FW_SIZE (DM_TEST_FW_INST_CONST_BYTES + 1024)
/* Tests for dm_register_dmub_notify_callback() */
@@ -112,7 +118,7 @@ static const struct firmware *dm_test_alloc_dmub_fw(struct kunit *test)
hdr = (struct dmcub_firmware_header_v1_0 *)data;
hdr->header.ucode_array_offset_bytes = cpu_to_le32(0);
hdr->header.ucode_version = cpu_to_le32(DMUB_FW_VERSION(9, 9, 9));
- hdr->inst_const_bytes = cpu_to_le32(PSP_HEADER_BYTES_256);
+ hdr->inst_const_bytes = cpu_to_le32(DM_TEST_FW_INST_CONST_BYTES);
hdr->bss_data_bytes = cpu_to_le32(0);
fw->size = DM_TEST_FW_SIZE;
@@ -1398,8 +1404,8 @@ static const struct cgs_ops dm_test_cgs_ops = {
/*
* Fake buffer object allocator: amdgpu_bo_create_kernel() and
- * amdgpu_bo_free_kernel() need a live TTM device, so dm_allocate_gpu_mem() is
- * routed through this fake.
+ * amdgpu_bo_free_kernel() need a live TTM device, so both the DMUB framebuffer
+ * allocation and dm_allocate_gpu_mem() are routed through this fake.
*/
#define DM_TEST_FAKE_GPU_ADDR 0x1234ABCD0000ULL
@@ -1469,6 +1475,7 @@ static __printf(4, 5) int dm_test_ucode_request(struct amdgpu_device *adev,
}
static const struct amdgpu_dm_dmub_kunit_ops dm_test_dmub_ops = {
+ .bo_create_kernel = dm_test_bo_create_kernel,
.ucode_request = dm_test_ucode_request,
};
@@ -1720,6 +1727,170 @@ static void dm_test_init_microcode_request_fails(struct kunit *test)
KUNIT_EXPECT_EQ(test, dm_init_microcode(adev), -ENOENT);
}
+/* Tests for dm_dmub_sw_init() */
+
+static struct amdgpu_device *dm_test_alloc_adev_for_sw_init(struct kunit *test)
+{
+ struct amdgpu_device *adev = dm_test_alloc_adev_with_cgs(test);
+
+ adev->dm.dmub_fw = dm_test_alloc_dmub_fw(test);
+ adev->bios = kunit_kzalloc(test, 4, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, adev->bios);
+ adev->bios_size = 4;
+
+ return adev;
+}
+
+static void dm_test_free_sw_init(struct amdgpu_device *adev)
+{
+ kfree(adev->dm.dmub_srv);
+ kfree(adev->dm.dmub_fb_info);
+ adev->dm.dmub_srv = NULL;
+ adev->dm.dmub_fb_info = NULL;
+}
+
+/**
+ * dm_test_dmub_sw_init_asic_mapping - Test the ASIC to DMUB service mapping
+ * @test: The KUnit test context
+ *
+ * Every supported ASIC must get past DMUB service creation and region
+ * calculation. The framebuffer allocation is forced to fail so the tests stop
+ * before the memory layout is rebased onto a fake buffer.
+ */
+static void dm_test_dmub_sw_init_asic_mapping(struct kunit *test)
+{
+ static const u32 ip_versions[] = {
+ IP_VERSION(2, 1, 0), IP_VERSION(3, 0, 0), IP_VERSION(3, 0, 1),
+ IP_VERSION(3, 0, 2), IP_VERSION(3, 0, 3), IP_VERSION(3, 1, 2),
+ IP_VERSION(3, 1, 3), IP_VERSION(3, 1, 4), IP_VERSION(3, 1, 5),
+ IP_VERSION(3, 1, 6), IP_VERSION(3, 2, 0), IP_VERSION(3, 2, 1),
+ IP_VERSION(3, 5, 0), IP_VERSION(3, 5, 1), IP_VERSION(3, 6, 0),
+ IP_VERSION(4, 0, 1), IP_VERSION(4, 2, 0), IP_VERSION(4, 2, 1),
+ IP_VERSION(6, 0, 0),
+ };
+ struct amdgpu_device *adev = dm_test_alloc_adev_for_sw_init(test);
+ unsigned int i;
+
+ dm_test_bo.create_ret = -ENOMEM;
+
+ for (i = 0; i < ARRAY_SIZE(ip_versions); i++) {
+ adev->ip_versions[DCE_HWIP][0] = ip_versions[i];
+
+ KUNIT_EXPECT_EQ_MSG(test, dm_dmub_sw_init(adev), -ENOMEM,
+ "IP version 0x%08x", ip_versions[i]);
+
+ dm_test_free_sw_init(adev);
+ }
+
+ KUNIT_EXPECT_EQ(test, dm_test_bo.create_calls, (unsigned int)ARRAY_SIZE(ip_versions));
+}
+
+/**
+ * dm_test_dmub_sw_init_gtt_only_asic - Test the GTT-only memory domain
+ * @test: The KUnit test context
+ *
+ * DCN32 and DCN321 keep the DMUB framebuffer in GTT; every other ASIC also
+ * allows VRAM.
+ */
+static void dm_test_dmub_sw_init_gtt_only_asic(struct kunit *test)
+{
+ struct amdgpu_device *adev = dm_test_alloc_adev_for_sw_init(test);
+
+ adev->ip_versions[DCE_HWIP][0] = IP_VERSION(3, 2, 0);
+ dm_test_bo.create_ret = -ENOMEM;
+
+ KUNIT_EXPECT_EQ(test, dm_dmub_sw_init(adev), -ENOMEM);
+ KUNIT_EXPECT_EQ(test, dm_test_bo.create_domain, (u32)AMDGPU_GEM_DOMAIN_GTT);
+
+ dm_test_free_sw_init(adev);
+
+ adev->ip_versions[DCE_HWIP][0] = IP_VERSION(3, 5, 0);
+
+ KUNIT_EXPECT_EQ(test, dm_dmub_sw_init(adev), -ENOMEM);
+ KUNIT_EXPECT_EQ(test, dm_test_bo.create_domain,
+ (u32)(AMDGPU_GEM_DOMAIN_GTT | AMDGPU_GEM_DOMAIN_VRAM));
+
+ dm_test_free_sw_init(adev);
+}
+
+/**
+ * dm_test_dmub_sw_init_success - Test a complete software init
+ * @test: The KUnit test context
+ *
+ * With a fake framebuffer allocator and a fake CGS device, software init
+ * should publish the framebuffer info, the bounding box and the instruction
+ * constant size.
+ */
+static void dm_test_dmub_sw_init_success(struct kunit *test)
+{
+ struct amdgpu_device *adev = dm_test_alloc_adev_for_sw_init(test);
+
+ adev->ip_versions[DCE_HWIP][0] = IP_VERSION(6, 0, 0);
+
+ KUNIT_EXPECT_EQ(test, dm_dmub_sw_init(adev), 0);
+ KUNIT_EXPECT_NOT_NULL(test, adev->dm.dmub_srv);
+ KUNIT_EXPECT_NOT_NULL(test, adev->dm.dmub_fb_info);
+ KUNIT_EXPECT_NOT_NULL(test, adev->dm.bb_from_dmub);
+ KUNIT_EXPECT_EQ(test, adev->dm.dmcub_fw_version, DMUB_FW_VERSION(9, 9, 9));
+ /* Meta info lookup fails on the fake firmware and trims a PSP footer. */
+ KUNIT_EXPECT_EQ(test, adev->dm.fw_inst_size,
+ (u32)(DM_TEST_FW_INST_CONST_BYTES - PSP_HEADER_BYTES_256 -
+ PSP_FOOTER_BYTES_256));
+
+ dm_free_gpu_mem(adev, DC_MEM_ALLOC_TYPE_GART, adev->dm.bb_from_dmub);
+ dm_test_free_sw_init(adev);
+}
+
+/**
+ * dm_test_dmub_sw_init_bss_data - Test software init with a BSS data region
+ * @test: The KUnit test context
+ *
+ * A non-zero BSS data size makes software init point the firmware meta info
+ * lookup at the legacy metadata region instead of the instruction constants.
+ */
+static void dm_test_dmub_sw_init_bss_data(struct kunit *test)
+{
+ struct amdgpu_device *adev = dm_test_alloc_adev_for_sw_init(test);
+ struct dmcub_firmware_header_v1_0 *hdr;
+
+ hdr = (struct dmcub_firmware_header_v1_0 *)adev->dm.dmub_fw->data;
+ hdr->bss_data_bytes = cpu_to_le32(512);
+
+ adev->ip_versions[DCE_HWIP][0] = IP_VERSION(6, 0, 0);
+
+ KUNIT_EXPECT_EQ(test, dm_dmub_sw_init(adev), 0);
+ KUNIT_EXPECT_NOT_NULL(test, adev->dm.dmub_fb_info);
+
+ dm_free_gpu_mem(adev, DC_MEM_ALLOC_TYPE_GART, adev->dm.bb_from_dmub);
+ dm_test_free_sw_init(adev);
+}
+
+/**
+ * dm_test_dmub_sw_init_psp_load - Test the PSP firmware load registration
+ * @test: The KUnit test context
+ *
+ * When the firmware is loaded by the PSP, software init must register the
+ * DMCUB microcode with the AMDGPU firmware loader.
+ */
+static void dm_test_dmub_sw_init_psp_load(struct kunit *test)
+{
+ struct amdgpu_device *adev = dm_test_alloc_adev_for_sw_init(test);
+
+ adev->ip_versions[DCE_HWIP][0] = IP_VERSION(6, 0, 0);
+ adev->firmware.load_type = AMDGPU_FW_LOAD_PSP;
+
+ KUNIT_EXPECT_EQ(test, dm_dmub_sw_init(adev), 0);
+ KUNIT_EXPECT_EQ(test, adev->firmware.ucode[AMDGPU_UCODE_ID_DMCUB].ucode_id,
+ AMDGPU_UCODE_ID_DMCUB);
+ KUNIT_EXPECT_PTR_EQ(test, adev->firmware.ucode[AMDGPU_UCODE_ID_DMCUB].fw,
+ adev->dm.dmub_fw);
+ KUNIT_EXPECT_EQ(test, adev->firmware.fw_size,
+ (u32)ALIGN(DM_TEST_FW_INST_CONST_BYTES, PAGE_SIZE));
+
+ dm_free_gpu_mem(adev, DC_MEM_ALLOC_TYPE_GART, adev->dm.bb_from_dmub);
+ dm_test_free_sw_init(adev);
+}
+
static struct kunit_case amdgpu_dm_dmub_tests[] = {
/* dm_register_dmub_notify_callback() */
KUNIT_CASE(dm_test_register_dmub_notify_callback_null_callback),
@@ -1796,6 +1967,12 @@ static struct kunit_case amdgpu_dm_dmub_hw_access_tests[] = {
/* dm_init_microcode() */
KUNIT_CASE(dm_test_init_microcode_fw_names),
KUNIT_CASE(dm_test_init_microcode_request_fails),
+ /* dm_dmub_sw_init() */
+ KUNIT_CASE(dm_test_dmub_sw_init_asic_mapping),
+ KUNIT_CASE(dm_test_dmub_sw_init_gtt_only_asic),
+ KUNIT_CASE(dm_test_dmub_sw_init_success),
+ KUNIT_CASE(dm_test_dmub_sw_init_bss_data),
+ KUNIT_CASE(dm_test_dmub_sw_init_psp_load),
{}
};
--
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.