Received-SPF: pass (google.com: domain of linux-kernel+bounces-98994-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <34ed7275-e1a5-4d93-8ba1-ef3c597eaef3@amd.com>
Date: Mon, 11 Mar 2024 20:18:59 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] drm/amdgpu: add ring buffer information in devcoredump
Content-Language: en-US
To: =?UTF-8?Q?Christian_K=C3=B6nig?= <christian.koenig@amd.com>,
 Sunil Khatri <sunil.khatri@amd.com>, Alex Deucher
 <alexander.deucher@amd.com>, Shashank Sharma <shashank.sharma@amd.com>
Cc: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
 linux-kernel@vger.kernel.org
References: <20240311122212.13713-1-sunil.khatri@amd.com>
 <9e1dbcbc-f00e-417c-995e-d9c539292e03@amd.com>
From: "Khatri, Sunil" <sukhatri@amd.com>
In-Reply-To: <9e1dbcbc-f00e-417c-995e-d9c539292e03@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?WmhGQ3BGUVN1eEVySXd4aUlFU2JWcjYxRkQydzV6cnZhamV3NU0zT3ZKcDEx?=
 =?utf-8?B?bmVZWS9kWlhpWU9Eam03VUhDSkM1ZlJvbTZBYkZHN1NUV3drYXlDNGYydGg2?=
 =?utf-8?B?SUFRMG5VeUQrU0NHWUREbXlpcUpqN01qOWpIeG1aTzBPR2cvUG9JVi9NbFJS?=
 =?utf-8?B?UTQzeFh2SW55RTVpOWttY3VsRzFkUHpXMmVtMCszK0d0V2ttbVJ3eTkwMzBI?=
 =?utf-8?B?YkpGaWtwcEZLTzBxeE5uWVZ1aEp4SjJpVU9nYmozL1lOZGdvbkJ5Yk1oMW8y?=
 =?utf-8?B?WldCcVFnOVZWdERKNEN6YkpGL2oyS0FTblY2WXFQME9GK3pVRkoycWRreGlF?=
 =?utf-8?B?ZzkxT2NnTTFlZ2YxS3o2czNHTEdXbUVHWFVnci9MbHZqNCs5aXhpenBoS0pY?=
 =?utf-8?B?cHp4Q2pWQXlpckM2MmpVaTJjZjVNMjZLcTdmelRQbTg0N1M4dEFPTWZIQ0VQ?=
 =?utf-8?B?V1BOWjRWbDNuWlM4eit2cGZiYS9RM3hVLytPdnNqRm1LNWQ2Zmc3NkR2dDJy?=
 =?utf-8?B?Qzd4SlZnMnl4akI1TFRUOG9IWlFraE1DR01GUEdXczZQM3RxT3B4dkJDSlFM?=
 =?utf-8?B?dXRsSXNoWXJQTC9tamNuOHNieVpCOStzbXdDNitSL1JTNmZQdDk2cnlrRUE4?=
 =?utf-8?B?VXF6Tnc1SzVlNWVOTUthdUxLREoxd0NVQTIwUFRWZ05xb1AxbDBVek5QVmU2?=
 =?utf-8?B?WllNUVluWUU2L3RWQjExVmp4cjR6YzcvK2Q0Rkkyb0I3UmJkUnkvQjhpbXZO?=
 =?utf-8?B?bVhMQWhZODF1bTBBWnRMdzFaZFlvL3g4SXowaTYvK1MrVlBzL2crNTVpREdm?=
 =?utf-8?B?WWxhVlBYVWpCRjA3U3A4UWdsa1dXVkhTZHFpTHJEL3dyZUtZMEJkVnpVODdJ?=
 =?utf-8?B?RkZLZC80TzJpTC95alZvVi96cGs3SStZSmhCL2pDK3RnbW0rekRCODNHa29V?=
 =?utf-8?B?Y3hDUHBPUUZCUTlJQnZ4WHlTbVVkV0xrakg2QnpXakkzc3BUbytaVUVyZGh3?=
 =?utf-8?B?QVJxTkU3MGJLZzAxODI4KzlSZklndW1HM3JxME45RW83bk9VY3hEcWhxTzRN?=
 =?utf-8?B?WmhBYTEwb3dYaGVKaWx4VFJzTXgvaWZYRENpUkdVWlh3enplYmpwZlVXcFlt?=
 =?utf-8?B?R3pLa3pSbWJsV2ZtS08yWWZHNlh4cWV4Nmp4S21oQUtTcThNRU8yb0pwWmZr?=
 =?utf-8?B?cSswUXhZcFhhaGNIQzZkamY2UkNPZVl0MGFrd2c4VXM0UXlmdHEybzdEUTBJ?=
 =?utf-8?B?UGFyT3J4WEFjS25KZGZYUm9zajdVQk9VbTl6R3MwUnZiZDllMHB1U3o5VnB0?=
 =?utf-8?B?bGE4b2VBbExNWjJvUkhxMHBzZWp2YnVmcmFTV1BBLyt0SkVDVGhNR0o0Z0VM?=
 =?utf-8?B?UWJ2QWVVT2RiSzhaUGN6YzQ0dnJodEFJVkxkOEZrREVQMGszUkg4NG03TWlz?=
 =?utf-8?B?NXlDLzd1dDB3OEdPNEU3K284dTcwY3FEVW1nSTVIQVkxU0xxcFBWMExtTnpE?=
 =?utf-8?B?c0xTbVF1Zm0yOG40STdhT1dVczFRZnIvWnFrc0ZFZnliaWxDbWtBSnlyYWhh?=
 =?utf-8?B?SGxaYmVCcm01b0k1bzB5dUVQUnlvT2VmM3k2VGk0bERFdlVhdkkwelF0S2N1?=
 =?utf-8?B?TlREWm9JSDlzT3NXdDNpWlMya1VyZzc3WWdSMUdDWG5nY2RtaGtvOGNYMzJh?=
 =?utf-8?B?ekJrUjhMM0J4aG16MnN4NlZtQ0R3bXZlT3dXNDBOQzA4ODNZblUzdy9sbFVU?=
 =?utf-8?B?My9Ic1hmVXJSanJJMW95NTlEcm1yU1VLZkU3V0Ruejd2cXdTZTREbVhvS2hM?=
 =?utf-8?B?Y0g3SG1DMndneWlrNHFMdHBzd2hld2RBQXB4TFpUQ0RWM3FHdDcwTGRhTjVw?=
 =?utf-8?B?RnYyRnhRMTg3ckJ5dFBHMFo3REl1TGxRNVhNa2ovalovRUZFanlraGZBL1hx?=
 =?utf-8?B?Q05vbis2YXhpMEs4Slo2RTRWUEpBRTN1UXRNVjQ3UnNnajYzdm84SXJPRkFn?=
 =?utf-8?B?K3JHdGxna0E2dTMrbjlFbTNBZG9wZmJrUzhzWHdvaTQ0VjYzNnVVQStPN3ky?=
 =?utf-8?B?ZFZ6US9rWWp5OTRpY1A1YmRkQ0ptbTVrNi9HTE9MNHQ3d3RkeVNmcW5zU3Qr?=
 =?utf-8?Q?oPRIGq5GQxCRq1DrWL2qW2/PY?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c47cb122-4000-4b51-9211-08dc41da6864
X-MS-Exchange-CrossTenant-AuthSource: PH7PR12MB5596.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Mar 2024 14:49:09.3359
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: SKHF84dm9uZfN2VuuMu/FYZ8c/xCn5ND3z70DOjymupPfMH7Eebq9HGjy4YzoG/6bcBkM5t6PTTwshaNFxJI/Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR12MB8850


On 3/11/2024 7:29 PM, Christian König wrote:
>
>
> Am 11.03.24 um 13:22 schrieb Sunil Khatri:
>> Add relevant ringbuffer information such as
>> rptr, wptr, ring name, ring size and also
>> the ring contents for each ring on a gpu reset.
>>
>> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c | 21 +++++++++++++++++++++
>>   1 file changed, 21 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
>> index 6d059f853adc..1992760039da 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
>> @@ -215,6 +215,27 @@ amdgpu_devcoredump_read(char *buffer, loff_t 
>> offset, size_t count,
>>                  fault_info->status);
>>       }
>>   +    drm_printf(&p, "Ring buffer information\n");
>> +    for (int i = 0; i < coredump->adev->num_rings; i++) {
>> +        int j = 0;
>> +        struct amdgpu_ring *ring = coredump->adev->rings[i];
>> +
>> +        drm_printf(&p, "ring name: %s\n", ring->name);
>> +        drm_printf(&p, "Rptr: 0x%llx Wptr: 0x%llx\n",
>> +               amdgpu_ring_get_rptr(ring) & ring->buf_mask,
>> +               amdgpu_ring_get_wptr(ring) & ring->buf_mask);
>
> Don't apply the mask here. We do have some use cases where the rptr 
> and wptr are outside the ring buffer.
Sure i will remove the mask.
>
>> +        drm_printf(&p, "Ring size in dwords: %d\n",
>> +               ring->ring_size / 4);
>
> Rather print the mask as additional value here.
Does that help adding the mask value ?
>
>> +        drm_printf(&p, "Ring contents\n");
>> +        drm_printf(&p, "Offset \t Value\n");
>> +
>> +        while (j < ring->ring_size) {
>> +            drm_printf(&p, "0x%x \t 0x%x\n", j, ring->ring[j/4]);
>> +            j += 4;
>> +        }
>
>> +        drm_printf(&p, "Ring dumped\n");
>
> That seems superfluous.

Noted


Regards
Sunil

>
> Regards,
> Christian.
>
>> +    }
>> +
>>       if (coredump->reset_vram_lost)
>>           drm_printf(&p, "VRAM is lost due to GPU reset!\n");
>>       if (coredump->adev->reset_info.num_regs) {
>