Re: [PATCH 2/2] drm/amdgpu: Use version to figure out harvest info

["Lazar, Lijo" <lijo.lazar@amd.com>]

On 1/28/2025 9:01 PM, Alex Deucher wrote:
> On Tue, Jan 28, 2025 at 1:42 AM Lijo Lazar <lijo.lazar@amd.com> wrote:
>>
>> IP tables with version <=2 may use harvest bit. For version 3 and above,
>> harvest bit is not applicable, instead uses harvest table. Fix the
>> logic accordingly.
>>
>> Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 27 +++++++++++--------
>>  1 file changed, 16 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>> index d34b97a081d8..e3afdf933dc8 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>> @@ -612,7 +612,7 @@ static void amdgpu_discovery_read_harvest_bit_per_ip(struct amdgpu_device *adev,
>>         struct binary_header *bhdr;
>>         struct ip_discovery_header *ihdr;
>>         struct die_header *dhdr;
>> -       struct ip_v4 *ip;
>> +       struct ip *ip;
>>         uint16_t die_offset, ip_offset, num_dies, num_ips;
>>         uint16_t hw_id;
>>         uint8_t inst;
>> @@ -631,13 +631,14 @@ static void amdgpu_discovery_read_harvest_bit_per_ip(struct amdgpu_device *adev,
>>                 ip_offset = die_offset + sizeof(*dhdr);
>>
>>                 for (j = 0; j < num_ips; j++) {
>> -                       ip = (struct ip_v4 *)(adev->mman.discovery_bin + ip_offset);
>> -                       inst = ip->instance_number;
>> +                       ip = (struct ip *)(adev->mman.discovery_bin +
>> +                                          ip_offset);
>> +                       inst = ip->number_instance;
>>                         hw_id = le16_to_cpu(ip->hw_id);
>>                         if (amdgpu_discovery_validate_ip(adev, inst, hw_id))
>>                                 goto next_ip;
>>
>> -                       if (le16_to_cpu(ip->variant) == 1) {
>> +                       if (le16_to_cpu(ip->harvest) == 1) {
> 
> ip->harvest is a uint8_t so it doesn't need byte swapping.  Other than
> that, the series is:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Thanks for the catch. Will make this change before commit.

Thanks,
Lijo

> 
> 
>>                                 switch (hw_id) {
>>                                 case VCN_HWID:
>>                                         (*vcn_harvest_count)++;
>> @@ -663,10 +664,8 @@ static void amdgpu_discovery_read_harvest_bit_per_ip(struct amdgpu_device *adev,
>>                                 }
>>                         }
>>  next_ip:
>> -                       if (ihdr->base_addr_64_bit)
>> -                               ip_offset += struct_size(ip, base_address_64, ip->num_base_address);
>> -                       else
>> -                               ip_offset += struct_size(ip, base_address, ip->num_base_address);
>> +                       ip_offset += struct_size(ip, base_address,
>> +                                                ip->num_base_address);
>>                 }
>>         }
>>  }
>> @@ -1474,18 +1473,24 @@ static int amdgpu_discovery_reg_base_init(struct amdgpu_device *adev)
>>
>>  static void amdgpu_discovery_harvest_ip(struct amdgpu_device *adev)
>>  {
>> +       struct ip_discovery_header *ihdr;
>> +       struct binary_header *bhdr;
>>         int vcn_harvest_count = 0;
>>         int umc_harvest_count = 0;
>> +       uint16_t offset, ihdr_ver;
>>
>> +       bhdr = (struct binary_header *)adev->mman.discovery_bin;
>> +       offset = le16_to_cpu(bhdr->table_list[IP_DISCOVERY].offset);
>> +       ihdr = (struct ip_discovery_header *)(adev->mman.discovery_bin +
>> +                                             offset);
>> +       ihdr_ver = le16_to_cpu(ihdr->version);
>>         /*
>>          * Harvest table does not fit Navi1x and legacy GPUs,
>>          * so read harvest bit per IP data structure to set
>>          * harvest configuration.
>>          */
>>         if (amdgpu_ip_version(adev, GC_HWIP, 0) < IP_VERSION(10, 2, 0) &&
>> -           amdgpu_ip_version(adev, GC_HWIP, 0) != IP_VERSION(9, 4, 3) &&
>> -           amdgpu_ip_version(adev, GC_HWIP, 0) != IP_VERSION(9, 4, 4) &&
>> -           amdgpu_ip_version(adev, GC_HWIP, 0) != IP_VERSION(9, 5, 0)) {
>> +           ihdr_ver <= 2) {
>>                 if ((adev->pdev->device == 0x731E &&
>>                         (adev->pdev->revision == 0xC6 ||
>>                          adev->pdev->revision == 0xC7)) ||
>> --
>> 2.25.1
>>