RE: [PATCH 07/10] drm/amdgpu/gfx8: switch to amdgpu_gfx_rlc_init_microcode

["Wang, Yang(Kevin)" <KevinYang.Wang@amd.com>]

[AMD Official Use Only - General]

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Hawking Zhang
Sent: Friday, September 16, 2022 1:02 AM
To: amd-gfx@lists.freedesktop.org; Gao, Likun <Likun.Gao@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>
Cc: Zhang, Hawking <Hawking.Zhang@amd.com>
Subject: [PATCH 07/10] drm/amdgpu/gfx8: switch to amdgpu_gfx_rlc_init_microcode

switch to common helper to initialize rlc firmware for gfx8

Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 60 +++++----------------------
 1 file changed, 11 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 606d0c873d71..09c63e2d4637 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -951,7 +951,7 @@ static int gfx_v8_0_init_microcode(struct amdgpu_device *adev)
        const struct common_firmware_header *header = NULL;
        const struct gfx_firmware_header_v1_0 *cp_hdr;
        const struct rlc_firmware_header_v2_0 *rlc_hdr;
-       unsigned int *tmp = NULL, i;
+       uint16_t version_major, version_minor;

        DRM_DEBUG("\n");

@@ -1064,49 +1064,18 @@ static int gfx_v8_0_init_microcode(struct amdgpu_device *adev)
        if (err)
                goto out;
        err = amdgpu_ucode_validate(adev->gfx.rlc_fw);
-       rlc_hdr = (const struct rlc_firmware_header_v2_0 *)adev->gfx.rlc_fw->data;
-       adev->gfx.rlc_fw_version = le32_to_cpu(rlc_hdr->header.ucode_version);
-       adev->gfx.rlc_feature_version = le32_to_cpu(rlc_hdr->ucode_feature_version);
-
-       adev->gfx.rlc.save_and_restore_offset =
-                       le32_to_cpu(rlc_hdr->save_and_restore_offset);
-       adev->gfx.rlc.clear_state_descriptor_offset =
-                       le32_to_cpu(rlc_hdr->clear_state_descriptor_offset);
-       adev->gfx.rlc.avail_scratch_ram_locations =
-                       le32_to_cpu(rlc_hdr->avail_scratch_ram_locations);
-       adev->gfx.rlc.reg_restore_list_size =
-                       le32_to_cpu(rlc_hdr->reg_restore_list_size);
-       adev->gfx.rlc.reg_list_format_start =
-                       le32_to_cpu(rlc_hdr->reg_list_format_start);
-       adev->gfx.rlc.reg_list_format_separate_start =
-                       le32_to_cpu(rlc_hdr->reg_list_format_separate_start);
-       adev->gfx.rlc.starting_offsets_start =
-                       le32_to_cpu(rlc_hdr->starting_offsets_start);
-       adev->gfx.rlc.reg_list_format_size_bytes =
-                       le32_to_cpu(rlc_hdr->reg_list_format_size_bytes);
-       adev->gfx.rlc.reg_list_size_bytes =
-                       le32_to_cpu(rlc_hdr->reg_list_size_bytes);
-
-       adev->gfx.rlc.register_list_format =
-                       kmalloc(adev->gfx.rlc.reg_list_format_size_bytes +
-                                       adev->gfx.rlc.reg_list_size_bytes, GFP_KERNEL);
-
-       if (!adev->gfx.rlc.register_list_format) {
-               err = -ENOMEM;
+       if (err)
                goto out;
+       rlc_hdr = (const struct rlc_firmware_header_v2_0 *)adev->gfx.rlc_fw->data;
+       if (!rlc_hdr) {
+               dev_err(adev->dev, "invalid rlc fw data\n");
+               return -EINVAL;
        }

[Kevin]:
If variable of rlc_hdr is "NULL", the driver will get a data abort in amdgpu_ucode_validate() first, so I think the function "dev_err()" should never be called.
May be this error has been returned in previous position. (maybe in err = request_firmware()).

Thanks.

Best Regards,
Kevin
-
-       tmp = (unsigned int *)((uintptr_t)rlc_hdr +
-                       le32_to_cpu(rlc_hdr->reg_list_format_array_offset_bytes));
-       for (i = 0 ; i < (adev->gfx.rlc.reg_list_format_size_bytes >> 2); i++)
-               adev->gfx.rlc.register_list_format[i] = le32_to_cpu(tmp[i]);
-
-       adev->gfx.rlc.register_restore = adev->gfx.rlc.register_list_format + i;
-
-       tmp = (unsigned int *)((uintptr_t)rlc_hdr +
-                       le32_to_cpu(rlc_hdr->reg_list_array_offset_bytes));
-       for (i = 0 ; i < (adev->gfx.rlc.reg_list_size_bytes >> 2); i++)
-               adev->gfx.rlc.register_restore[i] = le32_to_cpu(tmp[i]);
+       version_major = le16_to_cpu(rlc_hdr->header.header_version_major);
+       version_minor = le16_to_cpu(rlc_hdr->header.header_version_minor);
+       err = amdgpu_gfx_rlc_init_microcode(adev, version_major, version_minor);
+       if (err)
+               goto out;

        if (adev->asic_type >= CHIP_POLARIS10 && adev->asic_type <= CHIP_POLARIS12) {
                snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mec_2.bin", chip_name); @@ -1178,13 +1147,6 @@ static int gfx_v8_0_init_microcode(struct amdgpu_device *adev)
        adev->firmware.fw_size +=
                ALIGN(le32_to_cpu(header->ucode_size_bytes), PAGE_SIZE);

-       info = &adev->firmware.ucode[AMDGPU_UCODE_ID_RLC_G];
-       info->ucode_id = AMDGPU_UCODE_ID_RLC_G;
-       info->fw = adev->gfx.rlc_fw;
-       header = (const struct common_firmware_header *)info->fw->data;
-       adev->firmware.fw_size +=
-               ALIGN(le32_to_cpu(header->ucode_size_bytes), PAGE_SIZE);
-
        info = &adev->firmware.ucode[AMDGPU_UCODE_ID_CP_MEC1];
        info->ucode_id = AMDGPU_UCODE_ID_CP_MEC1;
        info->fw = adev->gfx.mec_fw;
--
2.17.1