[PATCH] drm/amd/amdgpu: Add missing parameter for amdgpu_sdma_register_on_reset_callbacks

[PATCH] drm/amd/amdgpu: Add missing parameter for amdgpu_sdma_register_on_reset_callbacks

[Srinivasan Shanmugam]

This commit updates the documentation for the function
amdgpu_sdma_register_on_reset_callbacks to include a description
for the 'adev' parameter.

The 'adev' parameter is a pointer to the amdgpu_device structure,
which is necessary for registering SDMA reset callbacks.

Fixes the below with gcc W=1:
drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c:474: warning: Function parameter or struct member 'adev' not described in 'amdgpu_sdma_register_on_reset_callbacks'

Cc: Christian König <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
index 42a7b86e41c3..82856592039b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
@@ -464,6 +464,7 @@ void amdgpu_sdma_sysfs_reset_mask_fini(struct amdgpu_device *adev)
 
 /**
  * amdgpu_sdma_register_on_reset_callbacks - Register SDMA reset callbacks
+ * @adev: Pointer to the amdgpu_device structure
  * @funcs: Pointer to the callback structure containing pre_reset and post_reset functions
  *
  * This function allows KFD and AMDGPU to register their own callbacks for handling
-- 
2.34.1

Re: [PATCH] drm/amd/amdgpu: Add missing parameter for amdgpu_sdma_register_on_reset_callbacks

[Christian König]

Am 24.02.25 um 12:45 schrieb Srinivasan Shanmugam:
> This commit updates the documentation for the function
> amdgpu_sdma_register_on_reset_callbacks to include a description
> for the 'adev' parameter.
>
> The 'adev' parameter is a pointer to the amdgpu_device structure,
> which is necessary for registering SDMA reset callbacks.
>
> Fixes the below with gcc W=1:
> drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c:474: warning: Function parameter or struct member 'adev' not described in 'amdgpu_sdma_register_on_reset_callbacks'
>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> index 42a7b86e41c3..82856592039b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> @@ -464,6 +464,7 @@ void amdgpu_sdma_sysfs_reset_mask_fini(struct amdgpu_device *adev)
>  
>  /**
>   * amdgpu_sdma_register_on_reset_callbacks - Register SDMA reset callbacks

Why do we have a function to dynamically register callbacks in the first place?

That doesn't seem to make much sense.

Regards,
Christian.

> + * @adev: Pointer to the amdgpu_device structure
>   * @funcs: Pointer to the callback structure containing pre_reset and post_reset functions
>   *
>   * This function allows KFD and AMDGPU to register their own callbacks for handling

Re: [PATCH] drm/amd/amdgpu: Add missing parameter for amdgpu_sdma_register_on_reset_callbacks

[SRINIVASAN SHANMUGAM]

On 2/24/2025 6:54 PM, Christian König wrote:
> Am 24.02.25 um 12:45 schrieb Srinivasan Shanmugam:
>> This commit updates the documentation for the function
>> amdgpu_sdma_register_on_reset_callbacks to include a description
>> for the 'adev' parameter.
>>
>> The 'adev' parameter is a pointer to the amdgpu_device structure,
>> which is necessary for registering SDMA reset callbacks.
>>
>> Fixes the below with gcc W=1:
>> drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c:474: warning: Function parameter or struct member 'adev' not described in 'amdgpu_sdma_register_on_reset_callbacks'
>>
>> Cc: Christian König <christian.koenig@amd.com>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
>> index 42a7b86e41c3..82856592039b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
>> @@ -464,6 +464,7 @@ void amdgpu_sdma_sysfs_reset_mask_fini(struct amdgpu_device *adev)
>>   
>>   /**
>>    * amdgpu_sdma_register_on_reset_callbacks - Register SDMA reset callbacks
> Why do we have a function to dynamically register callbacks in the first place?

Hi Christian,

Looks like this change comes from commit:

commit 938b123d7c85971328712c6173268d7de66ba843
Author: Jesse.zhang@amd.com <Jesse.zhang@amd.com>
Date:   Tue Jan 21 09:18:44 2025 +0800

     drm/amdgpu/kfd: Add shared SDMA reset functionality with callback 
support

     This patch introduces shared SDMA reset functionality between 
AMDGPU and KFD.
     The implementation includes the following key changes:

     1. Added `amdgpu_sdma_reset_queue`:
        - Resets a specific SDMA queue by instance ID.
        - Invokes registered pre-reset and post-reset callbacks to allow 
KFD and AMDGPU
          to save/restore their state during the reset process.

     2. Added `amdgpu_set_on_reset_callbacks`:
        - Allows KFD and AMDGPU to register callback functions for 
pre-reset and
          post-reset operations.
        - Callbacks are stored in a global linked list and invoked in 
the correct order
          during SDMA reset.

     This patch ensures that both AMDGPU and KFD can handle SDMA reset 
events
     gracefully, with proper state saving and restoration. It also 
provides a flexible
     callback mechanism for future extensions.

looks like may be design choice though not sure, where this callback 
mechanism is to provides flexibility in handling SDMA reset events. By 
allowing both KFD (Kernel Fusion Driver) and AMDGPU to register their 
own pre-reset and post-reset functions,

May I please have your suggestions, if we might further improve this 
mechanism or any other aspects of the implementation.

Thanks in advance,

Srini

>
> That doesn't seem to make much sense.
>
> Regards,
> Christian.
>
>> + * @adev: Pointer to the amdgpu_device structure
>>    * @funcs: Pointer to the callback structure containing pre_reset and post_reset functions
>>    *
>>    * This function allows KFD and AMDGPU to register their own callbacks for handling