<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<p style="font-family:Arial;font-size:10pt;color:#0000FF;margin:5pt;" align="Left">
[AMD Official Use Only]<br>
</p>
<br>
<div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div id="appendonsend"></div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
<br>
</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Lazar, Lijo <Lijo.Lazar@amd.com><br>
<b>Sent:</b> Thursday, August 12, 2021 2:27 PM<br>
<b>To:</b> Wang, Kevin(Yang) <Kevin1.Wang@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org><br>
<b>Cc:</b> Feng, Kenneth <Kenneth.Feng@amd.com>; Min, Frank <Frank.Min@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com><br>
<b>Subject:</b> Re: [PATCH v2 1/2] drm/amd/pm: skip to load smu microcode on sriov for aldebaran</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt">
<div class="PlainText"><br>
<br>
On 8/12/2021 11:46 AM, Kevin Wang wrote:<br>
> v1:<br>
> 1. skip to load smu firmware in sriov mode for aldebaran chip<br>
> 2. using vbios pptable if in sriov mode.<br>
> <br>
> v2:<br>
> clean up smu driver code in sriov code path<br>
> <br>
> Signed-off-by: Kevin Wang <kevin1.wang@amd.com><br>
> ---<br>
>   .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c    | 102 ++++++++++++------<br>
>   1 file changed, 70 insertions(+), 32 deletions(-)<br>
> <br>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c<br>
> index a421ba85bd6d..3765624d8fd6 100644<br>
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c<br>
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c<br>
> @@ -85,6 +85,10 @@ int smu_v13_0_init_microcode(struct smu_context *smu)<br>
>        const struct common_firmware_header *header;<br>
>        struct amdgpu_firmware_info *ucode = NULL;<br>
>   <br>
> +     /* doesn't need to load smu firmware in IOV mode */<br>
> +     if (amdgpu_sriov_vf(adev))<br>
> +             return 0;<br>
> +<br>
>        switch (adev->asic_type) {<br>
>        case CHIP_ALDEBARAN:<br>
>                chip_name = "aldebaran";<br>
> @@ -268,52 +272,86 @@ static int smu_v13_0_set_pptable_v2_1(struct smu_context *smu, void **table,<br>
>        return 0;<br>
>   }<br>
>   <br>
> -int smu_v13_0_setup_pptable(struct smu_context *smu)<br>
> +static int smu_v13_0_get_pptable_from_vbios(struct smu_context *smu, void **table, uint32_t *size)<br>
>   {<br>
>        struct amdgpu_device *adev = smu->adev;<br>
> -     const struct smc_firmware_header_v1_0 *hdr;<br>
> -     int ret, index;<br>
> -     uint32_t size = 0;<br>
>        uint16_t atom_table_size;<br>
>        uint8_t frev, crev;<br>
> -     void *table;<br>
> -     uint16_t version_major, version_minor;<br>
> +     int ret, index;<br>
>   <br>
> +     dev_info(adev->dev, "use vbios provided pptable\n");<br>
> +     index = get_index_into_master_table(atom_master_list_of_data_tables_v2_1,<br>
> +                                         powerplayinfo);<br>
>   <br>
> -     if (amdgpu_smu_pptable_id >= 0) {<br>
> -             smu->smu_table.boot_values.pp_table_id = amdgpu_smu_pptable_id;<br>
> -             dev_info(adev->dev, "override pptable id %d\n", amdgpu_smu_pptable_id);<br>
> -     }<br>
> +     ret = amdgpu_atombios_get_data_table(adev, index, &atom_table_size, &frev, &crev,<br>
> +                                          (uint8_t **)&table);<br>
> +     if (ret)<br>
> +             return ret;<br>
> +<br>
> +     if (size)<br>
> +             *size = atom_table_size;<br>
> +<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static int smu_v13_0_get_pptable_from_firmware(struct smu_context *smu, void **table, uint32_t *size,<br>
> +                                            uint32_t pptable_id)<br>
> +{<br>
> +     const struct smc_firmware_header_v1_0 *hdr;<br>
> +     struct amdgpu_device *adev = smu->adev;<br>
> +     uint16_t version_major, version_minor;<br>
> +     int ret;<br>
>   <br>
>        hdr = (const struct smc_firmware_header_v1_0 *) adev->pm.fw->data;<br>
> +     if (!hdr)<br>
> +             return -EINVAL;<br>
> +<br>
> +     dev_info(adev->dev, "use driver provided pptable %d\n", pptable_id);<br>
> +<br>
>        version_major = le16_to_cpu(hdr->header.header_version_major);<br>
>        version_minor = le16_to_cpu(hdr->header.header_version_minor);<br>
> -     if (version_major == 2 && smu->smu_table.boot_values.pp_table_id > 0) {<br>
> -             dev_info(adev->dev, "use driver provided pptable %d\n", smu->smu_table.boot_values.pp_table_id);<br>
> -             switch (version_minor) {<br>
> -             case 1:<br>
> -                     ret = smu_v13_0_set_pptable_v2_1(smu, &table, &size,<br>
> -                                                      smu->smu_table.boot_values.pp_table_id);<br>
> -                     break;<br>
> -             default:<br>
> -                     ret = -EINVAL;<br>
> -                     break;<br>
> -             }<br>
> -             if (ret)<br>
> -                     return ret;<br>
> +     if (version_major != 2) {<br>
> +             dev_err(adev->dev, "Unsupported smu firwmare version %d.%d\n",<br>
> +                     version_major, version_minor);<br>
> +             return -EINVAL;<br>
> +     }<br>
>   <br>
> -     } else {<br>
> -             dev_info(adev->dev, "use vbios provided pptable\n");<br>
> -             index = get_index_into_master_table(atom_master_list_of_data_tables_v2_1,<br>
> -                                                 powerplayinfo);<br>
> +     switch (version_minor) {<br>
> +     case 1:<br>
> +             ret = smu_v13_0_set_pptable_v2_1(smu, table, size, pptable_id);<br>
> +             break;<br>
> +     default:<br>
> +             ret = -EINVAL;<br>
> +             break;<br>
> +     }<br>
>   <br>
> -             ret = amdgpu_atombios_get_data_table(adev, index, &atom_table_size, &frev, &crev,<br>
> -                                                  (uint8_t **)&table);<br>
> -             if (ret)<br>
> -                     return ret;<br>
> -             size = atom_table_size;<br>
> +     return 0;<br>
<br>
Probably, this should be<br>
        return ret;<br>
<br>
Fix it before submit. Apart from that series is<br>
        Reviewed-by: Lijo Lazar <lijo.lazar@amd.com></div>
<div class="PlainText"><br>
</div>
<div class="PlainText">[kevin]:</div>
<div class="PlainText"><br>
</div>
<div class="PlainText">good catch, thanks lijo.</div>
<div class="PlainText"><br>
</div>
<div class="PlainText">Best Regards,</div>
<div class="PlainText">Kevin</div>
<div class="PlainText"><br>
> +}<br>
> +<br>
> +int smu_v13_0_setup_pptable(struct smu_context *smu)<br>
> +{<br>
> +     struct amdgpu_device *adev = smu->adev;<br>
> +     uint32_t size = 0, pptable_id = 0;<br>
> +     void *table;<br>
> +     int ret = 0;<br>
> +<br>
> +     /* override pptable_id from driver parameter */<br>
> +     if (amdgpu_smu_pptable_id >= 0) {<br>
> +             pptable_id = amdgpu_smu_pptable_id;<br>
> +             dev_info(adev->dev, "override pptable id %d\n", pptable_id);<br>
> +     } else {<br>
> +             pptable_id = smu->smu_table.boot_values.pp_table_id;<br>
>        }<br>
>   <br>
> +     /* force using vbios pptable in sriov mode */<br>
> +     if (amdgpu_sriov_vf(adev) || !pptable_id)<br>
> +             ret = smu_v13_0_get_pptable_from_vbios(smu, &table, &size);<br>
> +     else<br>
> +             ret = smu_v13_0_get_pptable_from_firmware(smu, &table, &size, pptable_id);<br>
> +<br>
> +     if (ret)<br>
> +             return ret;<br>
> +<br>
>        if (!smu->smu_table.power_play_table)<br>
>                smu->smu_table.power_play_table = table;<br>
>        if (!smu->smu_table.power_play_table_size)<br>
> <br>
</div>
</span></font></div>
</div>
</body>
</html>