Re: [PATCH v6 20/45] drm/amd: Parse both v1 and v2 TA microcode headers using same function

Thu, 05 Jan 2023 05:22:58 -0800 



On 1/5/2023 9:12 AM, Mario Limonciello wrote:

Several IP versions duplicate code and can't use the common helpers.
Move this code into a single function so that the helpers can be used.

Reviewed-by: Alex Deucher <alexander.deuc...@amd.com>
Signed-off-by: Mario Limonciello <mario.limoncie...@amd.com>
---
v5->v6:
  * Rebase on earlier patches
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 120 ++++++++++++++++++------
  drivers/gpu/drm/amd/amdgpu/psp_v10_0.c  |  64 +------------
  drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  |  77 ++-------------
  drivers/gpu/drm/amd/amdgpu/psp_v12_0.c  |  62 +-----------
  4 files changed, 109 insertions(+), 214 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 7a2fc920739b..d971e3785eaf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -3272,41 +3272,75 @@ static int parse_ta_bin_descriptor(struct psp_context 
*psp,
        return 0;
  }

  
-int psp_init_ta_microcode(struct psp_context *psp,

-                         const char *chip_name)
+static int parse_ta_v1_microcode(struct psp_context *psp)
  {
+       const struct ta_firmware_header_v1_0 *ta_hdr;
        struct amdgpu_device *adev = psp->adev;
-       char fw_name[PSP_FW_NAME_LEN];
-       const struct ta_firmware_header_v2_0 *ta_hdr;
-       int err = 0;
-       int ta_index = 0;

  
-	if (!chip_name) {

-               dev_err(adev->dev, "invalid chip name for ta microcode\n");
+       ta_hdr = (const struct ta_firmware_header_v1_0 *)
+                        adev->psp.ta_fw->data;
+
+       if (le16_to_cpu(ta_hdr->header.header_version_major) != 1)
                return -EINVAL;
+
+       adev->psp.xgmi_context.context.bin_desc.fw_version =
+               le32_to_cpu(ta_hdr->xgmi.fw_version);
+       adev->psp.xgmi_context.context.bin_desc.size_bytes =
+               le32_to_cpu(ta_hdr->xgmi.size_bytes);
+       adev->psp.xgmi_context.context.bin_desc.start_addr =
+               (uint8_t *)ta_hdr +
+               le32_to_cpu(ta_hdr->header.ucode_array_offset_bytes);
+       adev->psp.ta_fw_version = le32_to_cpu(ta_hdr->header.ucode_version);
+       adev->psp.ras_context.context.bin_desc.fw_version =
+               le32_to_cpu(ta_hdr->ras.fw_version);
+       adev->psp.ras_context.context.bin_desc.size_bytes =
+               le32_to_cpu(ta_hdr->ras.size_bytes);
+       adev->psp.ras_context.context.bin_desc.start_addr =
+               (uint8_t *)adev->psp.xgmi_context.context.bin_desc.start_addr +
+               le32_to_cpu(ta_hdr->ras.offset_bytes);
+       adev->psp.hdcp_context.context.bin_desc.fw_version =
+               le32_to_cpu(ta_hdr->hdcp.fw_version);
+       adev->psp.hdcp_context.context.bin_desc.size_bytes =
+               le32_to_cpu(ta_hdr->hdcp.size_bytes);
+       adev->psp.hdcp_context.context.bin_desc.start_addr =
+               (uint8_t *)ta_hdr +
+               le32_to_cpu(ta_hdr->header.ucode_array_offset_bytes);
+       adev->psp.ta_fw_version = le32_to_cpu(ta_hdr->header.ucode_version);
+       adev->psp.dtm_context.context.bin_desc.fw_version =
+               le32_to_cpu(ta_hdr->dtm.fw_version);
+       adev->psp.dtm_context.context.bin_desc.size_bytes =
+               le32_to_cpu(ta_hdr->dtm.size_bytes);
+       adev->psp.dtm_context.context.bin_desc.start_addr =
+               (uint8_t *)adev->psp.hdcp_context.context.bin_desc.start_addr +
+               le32_to_cpu(ta_hdr->dtm.offset_bytes);
+       if (adev->apu_flags & AMD_APU_IS_RENOIR) {
+               adev->psp.securedisplay_context.context.bin_desc.fw_version =
+                       le32_to_cpu(ta_hdr->securedisplay.fw_version);
+               adev->psp.securedisplay_context.context.bin_desc.size_bytes =
+                       le32_to_cpu(ta_hdr->securedisplay.size_bytes);
+               adev->psp.securedisplay_context.context.bin_desc.start_addr =
+                       (uint8_t 
*)adev->psp.hdcp_context.context.bin_desc.start_addr +
+                       le32_to_cpu(ta_hdr->securedisplay.offset_bytes);
        }



psp_v10_0_init_microcode used to get securedisplay_context 
unconditionally and now this is restricted to RENOIR following the logic 
in psp v12. Better is to fetch all FW details unconditionally and make 
the size_bytes to 0 (just to be sure) in specific PSP versions to 
prevent their load.


Thanks,
Lijo


  
-	snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ta.bin", chip_name);

-       err = request_firmware(&adev->psp.ta_fw, fw_name, adev->dev);
-       if (err)
-               goto out;
+       return 0;
+}

  
-	err = amdgpu_ucode_validate(adev->psp.ta_fw);

-       if (err)
-               goto out;
+static int parse_ta_v2_microcode(struct psp_context *psp)
+{
+       const struct ta_firmware_header_v2_0 *ta_hdr;
+       struct amdgpu_device *adev = psp->adev;
+       int err = 0;
+       int ta_index = 0;

  
  	ta_hdr = (const struct ta_firmware_header_v2_0 *)adev->psp.ta_fw->data;
  
-	if (le16_to_cpu(ta_hdr->header.header_version_major) != 2) {

-               dev_err(adev->dev, "unsupported TA header version\n");
-               err = -EINVAL;
-               goto out;
-       }
+       if (le16_to_cpu(ta_hdr->header.header_version_major) != 2)
+               return -EINVAL;

  
  	if (le32_to_cpu(ta_hdr->ta_fw_bin_count) >= UCODE_MAX_PSP_PACKAGING) {

                dev_err(adev->dev, "packed TA count exceeds maximum limit\n");
-               err = -EINVAL;
-               goto out;
+               return -EINVAL;
        }

  
  	for (ta_index = 0; ta_index < le32_to_cpu(ta_hdr->ta_fw_bin_count); ta_index++) {

@@ -3314,14 +3348,46 @@ int psp_init_ta_microcode(struct psp_context *psp,
                                              &ta_hdr->ta_fw_bin[ta_index],
                                              ta_hdr);
                if (err)
-                       goto out;
+                       return err;
        }

  
  	return 0;

-out:
-       dev_err(adev->dev, "fail to initialize ta microcode\n");
-       release_firmware(adev->psp.ta_fw);
-       adev->psp.ta_fw = NULL;
+}
+
+int psp_init_ta_microcode(struct psp_context *psp, const char *chip_name)
+{
+       const struct common_firmware_header *hdr;
+       struct amdgpu_device *adev = psp->adev;
+       char fw_name[PSP_FW_NAME_LEN];
+       int err;
+
+       snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ta.bin", chip_name);
+       err = request_firmware(&adev->psp.ta_fw, fw_name, adev->dev);
+       if (err)
+               return err;
+       err = amdgpu_ucode_validate(adev->psp.ta_fw);
+       if (err)
+               return err;
+
+       hdr = (const struct common_firmware_header *)adev->psp.ta_fw->data;
+       switch (le16_to_cpu(hdr->header_version_major)) {
+       case 1:
+               err = parse_ta_v1_microcode(psp);
+               break;
+       case 2:
+               err = parse_ta_v2_microcode(psp);
+               break;
+       default:
+               dev_err(adev->dev, "unsupported TA header version\n");
+               err = -EINVAL;
+       }
+
+       if (err) {
+               dev_err(adev->dev, "fail to initialize ta microcode\n");
+               release_firmware(adev->psp.ta_fw);
+               adev->psp.ta_fw = NULL;
+       }
+
        return err;
  }

  
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c

index 9de46fa8f46c..f14fcfb9c425 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
@@ -48,9 +48,8 @@ static int psp_v10_0_init_microcode(struct psp_context *psp)
  {
        struct amdgpu_device *adev = psp->adev;
        const char *chip_name;
-       char fw_name[30];
+       char ucode_prefix[30];
        int err = 0;
-       const struct ta_firmware_header_v1_0 *ta_hdr;
        DRM_DEBUG("\n");

  
  	switch (adev->asic_type) {

@@ -64,66 +63,13 @@ static int psp_v10_0_init_microcode(struct psp_context *psp)
                break;
        default: BUG();
        }
+       amdgpu_ucode_ip_version_decode(adev, MP0_HWIP, ucode_prefix, 
sizeof(ucode_prefix));

  
-	err = psp_init_asd_microcode(psp, chip_name);

+       err = psp_init_asd_microcode(psp, ucode_prefix);
        if (err)
-               goto out;
-
-       snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ta.bin", chip_name);
-       err = request_firmware(&adev->psp.ta_fw, fw_name, adev->dev);
-       if (err) {
-               release_firmware(adev->psp.ta_fw);
-               adev->psp.ta_fw = NULL;
-               dev_info(adev->dev,
-                        "psp v10.0: Failed to load firmware \"%s\"\n",
-                        fw_name);
-       } else {
-               err = amdgpu_ucode_validate(adev->psp.ta_fw);
-               if (err)
-                       goto out2;
-
-               ta_hdr = (const struct ta_firmware_header_v1_0 *)
-                                adev->psp.ta_fw->data;
-               adev->psp.hdcp_context.context.bin_desc.fw_version =
-                       le32_to_cpu(ta_hdr->hdcp.fw_version);
-               adev->psp.hdcp_context.context.bin_desc.size_bytes =
-                       le32_to_cpu(ta_hdr->hdcp.size_bytes);
-               adev->psp.hdcp_context.context.bin_desc.start_addr =
-                       (uint8_t *)ta_hdr +
-                       le32_to_cpu(ta_hdr->header.ucode_array_offset_bytes);
-
-               adev->psp.dtm_context.context.bin_desc.fw_version =
-                       le32_to_cpu(ta_hdr->dtm.fw_version);
-               adev->psp.dtm_context.context.bin_desc.size_bytes =
-                       le32_to_cpu(ta_hdr->dtm.size_bytes);
-               adev->psp.dtm_context.context.bin_desc.start_addr =
-                       (uint8_t 
*)adev->psp.hdcp_context.context.bin_desc.start_addr +
-                       le32_to_cpu(ta_hdr->dtm.offset_bytes);
-
-               adev->psp.securedisplay_context.context.bin_desc.fw_version =
-                       le32_to_cpu(ta_hdr->securedisplay.fw_version);
-               adev->psp.securedisplay_context.context.bin_desc.size_bytes =
-                       le32_to_cpu(ta_hdr->securedisplay.size_bytes);
-               adev->psp.securedisplay_context.context.bin_desc.start_addr =
-                       (uint8_t 
*)adev->psp.hdcp_context.context.bin_desc.start_addr +
-                       le32_to_cpu(ta_hdr->securedisplay.offset_bytes);
-
-               adev->psp.ta_fw_version = 
le32_to_cpu(ta_hdr->header.ucode_version);
-       }
-
-       return 0;
-
-out2:
-       release_firmware(adev->psp.ta_fw);
-       adev->psp.ta_fw = NULL;
-out:
-       if (err) {
-               dev_err(adev->dev,
-                       "psp v10.0: Failed to load firmware \"%s\"\n",
-                       fw_name);
-       }
+               return err;

  
-	return err;

+       return psp_init_ta_microcode(psp, ucode_prefix);
  }

  
  static int psp_v10_0_ring_create(struct psp_context *psp,

diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c 
b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
index bd3e3e23a939..21d20ca2377a 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
@@ -89,9 +89,8 @@ static int psp_v11_0_init_microcode(struct psp_context *psp)
  {
        struct amdgpu_device *adev = psp->adev;
        const char *chip_name;
-       char fw_name[PSP_FW_NAME_LEN];
+       char ucode_prefix[30];
        int err = 0;
-       const struct ta_firmware_header_v1_0 *ta_hdr;

  
  	DRM_DEBUG("\n");
  
@@ -129,6 +128,7 @@ static int psp_v11_0_init_microcode(struct psp_context *psp)

        default:
                BUG();
        }
+       amdgpu_ucode_ip_version_decode(adev, MP0_HWIP, ucode_prefix, 
sizeof(ucode_prefix));

  
  
  	switch (adev->ip_versions[MP0_HWIP][0]) {

@@ -140,35 +140,9 @@ static int psp_v11_0_init_microcode(struct psp_context 
*psp)
                err = psp_init_asd_microcode(psp, chip_name);
                if (err)
                        return err;
-               snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ta.bin", 
chip_name);
-               err = request_firmware(&adev->psp.ta_fw, fw_name, adev->dev);
-               if (err) {
-                       release_firmware(adev->psp.ta_fw);
-                       adev->psp.ta_fw = NULL;
-                       dev_info(adev->dev,
-                                "psp v11.0: Failed to load firmware \"%s\"\n", 
fw_name);
-               } else {
-                       err = amdgpu_ucode_validate(adev->psp.ta_fw);
-                       if (err)
-                               goto out2;
-
-                       ta_hdr = (const struct ta_firmware_header_v1_0 
*)adev->psp.ta_fw->data;
-                       adev->psp.xgmi_context.context.bin_desc.fw_version =
-                               le32_to_cpu(ta_hdr->xgmi.fw_version);
-                       adev->psp.xgmi_context.context.bin_desc.size_bytes =
-                               le32_to_cpu(ta_hdr->xgmi.size_bytes);
-                       adev->psp.xgmi_context.context.bin_desc.start_addr =
-                               (uint8_t *)ta_hdr +
-                               
le32_to_cpu(ta_hdr->header.ucode_array_offset_bytes);
-                       adev->psp.ta_fw_version = 
le32_to_cpu(ta_hdr->header.ucode_version);
-                       adev->psp.ras_context.context.bin_desc.fw_version =
-                               le32_to_cpu(ta_hdr->ras.fw_version);
-                       adev->psp.ras_context.context.bin_desc.size_bytes =
-                               le32_to_cpu(ta_hdr->ras.size_bytes);
-                       adev->psp.ras_context.context.bin_desc.start_addr =
-                               (uint8_t 
*)adev->psp.xgmi_context.context.bin_desc.start_addr +
-                               le32_to_cpu(ta_hdr->ras.offset_bytes);
-               }
+               err = psp_init_ta_microcode(psp, ucode_prefix);
+               if (err)
+                       return err;
                break;
        case IP_VERSION(11, 0, 0):
        case IP_VERSION(11, 0, 5):
@@ -179,39 +153,9 @@ static int psp_v11_0_init_microcode(struct psp_context 
*psp)
                err = psp_init_asd_microcode(psp, chip_name);
                if (err)
                        return err;
-               snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ta.bin", 
chip_name);
-               err = request_firmware(&adev->psp.ta_fw, fw_name, adev->dev);
-               if (err) {
-                       release_firmware(adev->psp.ta_fw);
-                       adev->psp.ta_fw = NULL;
-                       dev_info(adev->dev,
-                                "psp v11.0: Failed to load firmware \"%s\"\n", 
fw_name);
-               } else {
-                       err = amdgpu_ucode_validate(adev->psp.ta_fw);
-                       if (err)
-                               goto out2;
-
-                       ta_hdr = (const struct ta_firmware_header_v1_0 
*)adev->psp.ta_fw->data;
-                       adev->psp.hdcp_context.context.bin_desc.fw_version =
-                               le32_to_cpu(ta_hdr->hdcp.fw_version);
-                       adev->psp.hdcp_context.context.bin_desc.size_bytes =
-                               le32_to_cpu(ta_hdr->hdcp.size_bytes);
-                       adev->psp.hdcp_context.context.bin_desc.start_addr =
-                               (uint8_t *)ta_hdr +
-                               le32_to_cpu(
-                                       
ta_hdr->header.ucode_array_offset_bytes);
-
-                       adev->psp.ta_fw_version = 
le32_to_cpu(ta_hdr->header.ucode_version);
-
-                       adev->psp.dtm_context.context.bin_desc.fw_version =
-                               le32_to_cpu(ta_hdr->dtm.fw_version);
-                       adev->psp.dtm_context.context.bin_desc.size_bytes =
-                               le32_to_cpu(ta_hdr->dtm.size_bytes);
-                       adev->psp.dtm_context.context.bin_desc.start_addr =
-                               (uint8_t *)adev->psp.hdcp_context.context
-                                       .bin_desc.start_addr +
-                               le32_to_cpu(ta_hdr->dtm.offset_bytes);
-               }
+               err = psp_init_ta_microcode(psp, ucode_prefix);
+               if (err)
+                       return err;
                break;
        case IP_VERSION(11, 0, 7):
        case IP_VERSION(11, 0, 11):
@@ -237,11 +181,6 @@ static int psp_v11_0_init_microcode(struct psp_context 
*psp)
        }

  
  	return 0;

-
-out2:
-       release_firmware(adev->psp.ta_fw);
-       adev->psp.ta_fw = NULL;
-       return err;
  }

  
  static int psp_v11_0_wait_for_bootloader(struct psp_context *psp)

diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c 
b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
index 8ed2281b6557..634fa2822d8a 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
@@ -49,9 +49,8 @@ static int psp_v12_0_init_microcode(struct psp_context *psp)
  {
        struct amdgpu_device *adev = psp->adev;
        const char *chip_name;
-       char fw_name[30];
+       char ucode_prefix[30];
        int err = 0;
-       const struct ta_firmware_header_v1_0 *ta_hdr;
        DRM_DEBUG("\n");

  
  	switch (adev->asic_type) {

@@ -64,67 +63,12 @@ static int psp_v12_0_init_microcode(struct psp_context *psp)
        default:
                BUG();
        }
+       amdgpu_ucode_ip_version_decode(adev, MP0_HWIP, ucode_prefix, 
sizeof(ucode_prefix));

  
  	err = psp_init_asd_microcode(psp, chip_name);

        if (err)
                return err;
-
-       snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ta.bin", chip_name);
-       err = request_firmware(&adev->psp.ta_fw, fw_name, adev->dev);
-       if (err) {
-               release_firmware(adev->psp.ta_fw);
-               adev->psp.ta_fw = NULL;
-               dev_info(adev->dev,
-                        "psp v12.0: Failed to load firmware \"%s\"\n",
-                        fw_name);
-       } else {
-               err = amdgpu_ucode_validate(adev->psp.ta_fw);
-               if (err)
-                       goto out;
-
-               ta_hdr = (const struct ta_firmware_header_v1_0 *)
-                                adev->psp.ta_fw->data;
-               adev->psp.hdcp_context.context.bin_desc.fw_version =
-                       le32_to_cpu(ta_hdr->hdcp.fw_version);
-               adev->psp.hdcp_context.context.bin_desc.size_bytes =
-                       le32_to_cpu(ta_hdr->hdcp.size_bytes);
-               adev->psp.hdcp_context.context.bin_desc.start_addr =
-                       (uint8_t *)ta_hdr +
-                       le32_to_cpu(ta_hdr->header.ucode_array_offset_bytes);
-
-               adev->psp.ta_fw_version = 
le32_to_cpu(ta_hdr->header.ucode_version);
-
-               adev->psp.dtm_context.context.bin_desc.fw_version =
-                       le32_to_cpu(ta_hdr->dtm.fw_version);
-               adev->psp.dtm_context.context.bin_desc.size_bytes =
-                       le32_to_cpu(ta_hdr->dtm.size_bytes);
-               adev->psp.dtm_context.context.bin_desc.start_addr =
-                       (uint8_t 
*)adev->psp.hdcp_context.context.bin_desc.start_addr +
-                       le32_to_cpu(ta_hdr->dtm.offset_bytes);
-
-               if (adev->apu_flags & AMD_APU_IS_RENOIR) {
-                       
adev->psp.securedisplay_context.context.bin_desc.fw_version =
-                               le32_to_cpu(ta_hdr->securedisplay.fw_version);
-                       
adev->psp.securedisplay_context.context.bin_desc.size_bytes =
-                               le32_to_cpu(ta_hdr->securedisplay.size_bytes);
-                       
adev->psp.securedisplay_context.context.bin_desc.start_addr =
-                               (uint8_t 
*)adev->psp.hdcp_context.context.bin_desc.start_addr +
-                               le32_to_cpu(ta_hdr->securedisplay.offset_bytes);
-               }
-       }
-
-       return 0;
-
-out:
-       release_firmware(adev->psp.ta_fw);
-       adev->psp.ta_fw = NULL;
-       if (err) {
-               dev_err(adev->dev,
-                       "psp v12.0: Failed to load firmware \"%s\"\n",
-                       fw_name);
-       }
-
-       return err;
+       return psp_init_ta_microcode(psp, ucode_prefix);
  }

  
  static int psp_v12_0_bootloader_load_sysdrv(struct psp_context *psp)























					Reply via email to