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

Khatri, Sunil
2024-03-14T05:44:47Z

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
>>