Re: [PATCH 1/2] drm/amdgpu: add the IP information of the soc

["Khatri, Sunil" <sukhatri@amd.com>]

On 3/14/2024 1:58 AM, Alex Deucher wrote:
> On Tue, Mar 12, 2024 at 8:41 AM Sunil Khatri <sunil.khatri@amd.com> wrote:
>> Add all the IP's information on a SOC to the
>> devcoredump.
>>
>> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c | 19 +++++++++++++++++++
>>   1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
>> index a0dbccad2f53..611fdb90a1fc 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
>> @@ -196,6 +196,25 @@ amdgpu_devcoredump_read(char *buffer, loff_t offset, size_t count,
>>                             coredump->reset_task_info.process_name,
>>                             coredump->reset_task_info.pid);
>>
>> +       /* GPU IP's information of the SOC */
>> +       if (coredump->adev) {
>> +               drm_printf(&p, "\nIP Information\n");
>> +               drm_printf(&p, "SOC Family: %d\n", coredump->adev->family);
>> +               drm_printf(&p, "SOC Revision id: %d\n", coredump->adev->rev_id);
>> +
>> +               for (int i = 0; i < coredump->adev->num_ip_blocks; i++) {
>> +                       struct amdgpu_ip_block *ip =
>> +                               &coredump->adev->ip_blocks[i];
>> +                       drm_printf(&p, "IP type: %d IP name: %s\n",
>> +                                  ip->version->type,
>> +                                  ip->version->funcs->name);
>> +                       drm_printf(&p, "IP version: (%d,%d,%d)\n\n",
>> +                                  ip->version->major,
>> +                                  ip->version->minor,
>> +                                  ip->version->rev);
>> +               }
>> +       }
> I think the IP discovery table would be more useful.  Either walk the
> adev->ip_versions structure, or just include the IP discovery binary.

I did explore the adev->ip_versions and if i just go through the array 
it doesn't give any useful information directly.
There are no ways to find directly from adev->ip_versions below things 
until i also reparse the discovery binary again like done the discovery 
amdgpu_discovery_reg_base_init and walk through the headers of various 
ips using discovery binary.
a. Which IP is available on soc or not.
b. How many instances are there
Also i again have to change back to major, minor and rev convention for 
this information to be useful. I am exploring it more if i find some 
other information i will update.

adev->ip_block[] is derived from ip discovery only for each block which 
is there on the SOC, so we are not reading information which isnt 
applicable for the soc. We have name , type and version no of the IPs 
available on the soc. If you want i could add no of instances of each IP 
too if you think that's useful information here. Could you share what 
information is missing in this approach so i can include that.

For dumping the IP discovery binary, i dont understand how that 
information would be useful directly and needs to be decoded like we are 
doing in discovery init. Please correct me if my understanding is wrong 
here.
> Alex
>
>> +
>>          if (coredump->ring) {
>>                  drm_printf(&p, "\nRing timed out details\n");
>>                  drm_printf(&p, "IP Type: %d Ring Name: %s\n",
>> --
>> 2.34.1
>>