Re: [v3 3/3] drm/amdgpu: Add soft reset callback to SDMA v4.4.x

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

On 6/11/2025 3:12 PM, Jesse Zhang wrote:
> Implement soft reset engine callback for SDMA 4.4.x IPs. This avoids IP
> version check in generic implementation.
> 
> V2: Correct physical instance ID calculation in soft_reset_engine (Jesse)
> 
> Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 34 +++---------------------
>  drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c | 10 +++++++
>  2 files changed, 14 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> index 9b54a1ece447..a1e54bcef495 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> @@ -534,37 +534,11 @@ bool amdgpu_sdma_is_shared_inv_eng(struct amdgpu_device *adev, struct amdgpu_rin
>  static int amdgpu_sdma_soft_reset(struct amdgpu_device *adev, u32 instance_id)
>  {
>  	struct amdgpu_sdma_instance *sdma_instance = &adev->sdma.instance[instance_id];
> -	int r = -EOPNOTSUPP;
> -
> -	switch (amdgpu_ip_version(adev, SDMA0_HWIP, 0)) {
> -	case IP_VERSION(4, 4, 2):
> -	case IP_VERSION(4, 4, 4):
> -	case IP_VERSION(4, 4, 5):
> -		/* For SDMA 4.x, use the existing DPM interface for backward compatibility,
> -		 * we need to convert the logical instance ID to physical instance ID before reset.
> -		 */
> -		r = amdgpu_dpm_reset_sdma(adev, 1 << GET_INST(SDMA0, instance_id));
> -		break;
> -	case IP_VERSION(5, 0, 0):
> -	case IP_VERSION(5, 0, 1):
> -	case IP_VERSION(5, 0, 2):
> -	case IP_VERSION(5, 0, 5):
> -	case IP_VERSION(5, 2, 0):
> -	case IP_VERSION(5, 2, 2):
> -	case IP_VERSION(5, 2, 4):
> -	case IP_VERSION(5, 2, 5):
> -	case IP_VERSION(5, 2, 6):
> -	case IP_VERSION(5, 2, 3):
> -	case IP_VERSION(5, 2, 1):
> -	case IP_VERSION(5, 2, 7):
> -		if (sdma_instance->funcs->soft_reset_kernel_queue)
> -			r = sdma_instance->funcs->soft_reset_kernel_queue(adev, instance_id);
> -		break;
> -	default:
> -		break;
> -	}
>  
> -	return r;
> +	if (sdma_instance->funcs->soft_reset_kernel_queue)
> +		return sdma_instance->funcs->soft_reset_kernel_queue(adev, instance_id);
> +
> +	return -EOPNOTSUPP;
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
> index 3de125062ee3..ea0eabd9166e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
> @@ -109,6 +109,8 @@ static void sdma_v4_4_2_set_ras_funcs(struct amdgpu_device *adev);
>  static void sdma_v4_4_2_update_reset_mask(struct amdgpu_device *adev);
>  static int sdma_v4_4_2_stop_queue(struct amdgpu_ring *ring);
>  static int sdma_v4_4_2_restore_queue(struct amdgpu_ring *ring);
> +static int sdma_v4_4_2_soft_reset_engine(struct amdgpu_device *adev,
> +					 u32 instance_id);
>  
>  static u32 sdma_v4_4_2_get_reg_offset(struct amdgpu_device *adev,
>  		u32 instance, u32 offset)
> @@ -1337,6 +1339,7 @@ static bool sdma_v4_4_2_fw_support_paging_queue(struct amdgpu_device *adev)
>  static const struct amdgpu_sdma_funcs sdma_v4_4_2_sdma_funcs = {
>  	.stop_kernel_queue = &sdma_v4_4_2_stop_queue,
>  	.start_kernel_queue = &sdma_v4_4_2_restore_queue,
> +	.soft_reset_kernel_queue = &sdma_v4_4_2_soft_reset_engine,
>  };
>  
>  static int sdma_v4_4_2_early_init(struct amdgpu_ip_block *ip_block)
> @@ -1745,6 +1748,13 @@ static int sdma_v4_4_2_restore_queue(struct amdgpu_ring *ring)
>  	return sdma_v4_4_2_inst_start(adev, inst_mask, true);
>  }
>  
> +static int sdma_v4_4_2_soft_reset_engine(struct amdgpu_device *adev,
> +					 u32 instance_id)
> +{
> +	/* For SDMA 4.x, use the existing DPM interface for backward compatibility */

For clarity, you may keep your original comments here (for that alone a
v4 is not required).

"we need to convert the logical instance ID to physical instance ID
before reset"

Thanks,
Lijo

> +	return amdgpu_dpm_reset_sdma(adev, 1 << GET_INST(SDMA0, instance_id));
> +}
> +
>  static int sdma_v4_4_2_set_trap_irq_state(struct amdgpu_device *adev,
>  					struct amdgpu_irq_src *source,
>  					unsigned type,