|
[AMD Official Use Only] From: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
Sent: Thursday, August 12, 2021 2:27 PM To: Wang, Kevin(Yang) <Kevin1.Wang@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx> Cc: Feng, Kenneth <Kenneth.Feng@xxxxxxx>; Min, Frank <Frank.Min@xxxxxxx>; Zhang, Hawking <Hawking.Zhang@xxxxxxx> Subject: Re: [PATCH v2 1/2] drm/amd/pm: skip to load smu microcode on sriov for aldebaran On 8/12/2021 11:46 AM, Kevin Wang wrote: > v1: > 1. skip to load smu firmware in sriov mode for aldebaran chip > 2. using vbios pptable if in sriov mode. > > v2: > clean up smu driver code in sriov code path > > Signed-off-by: Kevin Wang <kevin1.wang@xxxxxxx> > --- > .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c | 102 ++++++++++++------ > 1 file changed, 70 insertions(+), 32 deletions(-) > > 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 > index a421ba85bd6d..3765624d8fd6 100644 > --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c > @@ -85,6 +85,10 @@ int smu_v13_0_init_microcode(struct smu_context *smu) > const struct common_firmware_header *header; > struct amdgpu_firmware_info *ucode = NULL; > > + /* doesn't need to load smu firmware in IOV mode */ > + if (amdgpu_sriov_vf(adev)) > + return 0; > + > switch (adev->asic_type) { > case CHIP_ALDEBARAN: > chip_name = "aldebaran"; > @@ -268,52 +272,86 @@ static int smu_v13_0_set_pptable_v2_1(struct smu_context *smu, void **table, > return 0; > } > > -int smu_v13_0_setup_pptable(struct smu_context *smu) > +static int smu_v13_0_get_pptable_from_vbios(struct smu_context *smu, void **table, uint32_t *size) > { > struct amdgpu_device *adev = smu->adev; > - const struct smc_firmware_header_v1_0 *hdr; > - int ret, index; > - uint32_t size = 0; > uint16_t atom_table_size; > uint8_t frev, crev; > - void *table; > - uint16_t version_major, version_minor; > + int ret, index; > > + dev_info(adev->dev, "use vbios provided pptable\n"); > + index = get_index_into_master_table(atom_master_list_of_data_tables_v2_1, > + powerplayinfo); > > - if (amdgpu_smu_pptable_id >= 0) { > - smu->smu_table.boot_values.pp_table_id = amdgpu_smu_pptable_id; > - dev_info(adev->dev, "override pptable id %d\n", amdgpu_smu_pptable_id); > - } > + ret = amdgpu_atombios_get_data_table(adev, index, &atom_table_size, &frev, &crev, > + (uint8_t **)&table); > + if (ret) > + return ret; > + > + if (size) > + *size = atom_table_size; > + > + return 0; > +} > + > +static int smu_v13_0_get_pptable_from_firmware(struct smu_context *smu, void **table, uint32_t *size, > + uint32_t pptable_id) > +{ > + const struct smc_firmware_header_v1_0 *hdr; > + struct amdgpu_device *adev = smu->adev; > + uint16_t version_major, version_minor; > + int ret; > > hdr = (const struct smc_firmware_header_v1_0 *) adev->pm.fw->data; > + if (!hdr) > + return -EINVAL; > + > + dev_info(adev->dev, "use driver provided pptable %d\n", pptable_id); > + > version_major = le16_to_cpu(hdr->header.header_version_major); > version_minor = le16_to_cpu(hdr->header.header_version_minor); > - if (version_major == 2 && smu->smu_table.boot_values.pp_table_id > 0) { > - dev_info(adev->dev, "use driver provided pptable %d\n", smu->smu_table.boot_values.pp_table_id); > - switch (version_minor) { > - case 1: > - ret = smu_v13_0_set_pptable_v2_1(smu, &table, &size, > - smu->smu_table.boot_values.pp_table_id); > - break; > - default: > - ret = -EINVAL; > - break; > - } > - if (ret) > - return ret; > + if (version_major != 2) { > + dev_err(adev->dev, "Unsupported smu firwmare version %d.%d\n", > + version_major, version_minor); > + return -EINVAL; > + } > > - } else { > - dev_info(adev->dev, "use vbios provided pptable\n"); > - index = get_index_into_master_table(atom_master_list_of_data_tables_v2_1, > - powerplayinfo); > + switch (version_minor) { > + case 1: > + ret = smu_v13_0_set_pptable_v2_1(smu, table, size, pptable_id); > + break; > + default: > + ret = -EINVAL; > + break; > + } > > - ret = amdgpu_atombios_get_data_table(adev, index, &atom_table_size, &frev, &crev, > - (uint8_t **)&table); > - if (ret) > - return ret; > - size = atom_table_size; > + return 0; Probably, this should be return ret; Fix it before submit. Apart from that series is Reviewed-by: Lijo Lazar <lijo.lazar@xxxxxxx> [kevin]:
good catch, thanks lijo.
Best Regards,
Kevin
> +} > + > +int smu_v13_0_setup_pptable(struct smu_context *smu) > +{ > + struct amdgpu_device *adev = smu->adev; > + uint32_t size = 0, pptable_id = 0; > + void *table; > + int ret = 0; > + > + /* override pptable_id from driver parameter */ > + if (amdgpu_smu_pptable_id >= 0) { > + pptable_id = amdgpu_smu_pptable_id; > + dev_info(adev->dev, "override pptable id %d\n", pptable_id); > + } else { > + pptable_id = smu->smu_table.boot_values.pp_table_id; > } > > + /* force using vbios pptable in sriov mode */ > + if (amdgpu_sriov_vf(adev) || !pptable_id) > + ret = smu_v13_0_get_pptable_from_vbios(smu, &table, &size); > + else > + ret = smu_v13_0_get_pptable_from_firmware(smu, &table, &size, pptable_id); > + > + if (ret) > + return ret; > + > if (!smu->smu_table.power_play_table) > smu->smu_table.power_play_table = table; > if (!smu->smu_table.power_play_table_size) > |
