Re: [PATCH v1 15/20] drm/amdgpu: introduce amdgpu_sdma_set_vm_pte_scheds

["Christian König" <christian.koenig@amd.com>]

On 11/4/25 09:35, Pierre-Eric Pelloux-Prayer wrote:
> All sdma versions used the same logic, so add a helper and move the
> common code to a single place.
> 
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h      |  1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   | 15 +++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/cik_sdma.c    |  8 +-------
>  drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c   |  8 +-------
>  drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c   |  8 +-------
>  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c   | 12 +-----------
>  drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c | 12 +-----------
>  drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c   |  8 +-------
>  drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c   |  8 +-------
>  drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c   |  8 +-------
>  drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c   |  8 +-------
>  drivers/gpu/drm/amd/amdgpu/si_dma.c      |  8 +-------
>  12 files changed, 26 insertions(+), 78 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index a9dc13659899..dead938a59a4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1614,6 +1614,7 @@ bool amdgpu_device_has_display_hardware(struct amdgpu_device *adev);
>  ssize_t amdgpu_get_soft_full_reset_mask(struct amdgpu_ring *ring);
>  ssize_t amdgpu_show_reset_mask(char *buf, uint32_t supported_reset);
>  void amdgpu_sdma_set_buffer_funcs_rings(struct amdgpu_device *adev);
> +void amdgpu_sdma_set_vm_pte_scheds(struct amdgpu_device *adev);
>  
>  /* atpx handler */
>  #if defined(CONFIG_VGA_SWITCHEROO)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index bc11e212f08c..b66e41e979ad 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -3210,3 +3210,18 @@ void amdgpu_vm_print_task_info(struct amdgpu_device *adev,
>  		task_info->process_name, task_info->tgid,
>  		task_info->task.comm, task_info->task.pid);
>  }
> +
> +void amdgpu_sdma_set_vm_pte_scheds(struct amdgpu_device *adev)

Please also add the vm_pte_funcs as parameters since those should always be set at the same time as the scheduler instances.

Apart from that looks good to me.

Regards,
Christian.

> +{
> +	struct drm_gpu_scheduler *sched;
> +	int i;
> +
> +	for (i = 0; i < adev->sdma.num_instances; i++) {
> +		if (adev->sdma.has_page_queue)
> +			sched = &adev->sdma.instance[i].page.sched;
> +		else
> +			sched = &adev->sdma.instance[i].ring.sched;
> +		adev->vm_manager.vm_pte_scheds[i] = sched;
> +	}
> +	adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> index 25040997c367..f2515de65597 100644
> --- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> @@ -1347,14 +1347,8 @@ static const struct amdgpu_vm_pte_funcs cik_sdma_vm_pte_funcs = {
>  
>  static void cik_sdma_set_vm_pte_funcs(struct amdgpu_device *adev)
>  {
> -	unsigned i;
> -
>  	adev->vm_manager.vm_pte_funcs = &cik_sdma_vm_pte_funcs;
> -	for (i = 0; i < adev->sdma.num_instances; i++) {
> -		adev->vm_manager.vm_pte_scheds[i] =
> -			&adev->sdma.instance[i].ring.sched;
> -	}
> -	adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
> +	amdgpu_sdma_set_vm_pte_scheds(adev);
>  }
>  
>  const struct amdgpu_ip_block_version cik_sdma_ip_block =
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> index 149356c9346a..4b2c7f75eadb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> @@ -1242,14 +1242,8 @@ static const struct amdgpu_vm_pte_funcs sdma_v2_4_vm_pte_funcs = {
>  
>  static void sdma_v2_4_set_vm_pte_funcs(struct amdgpu_device *adev)
>  {
> -	unsigned i;
> -
>  	adev->vm_manager.vm_pte_funcs = &sdma_v2_4_vm_pte_funcs;
> -	for (i = 0; i < adev->sdma.num_instances; i++) {
> -		adev->vm_manager.vm_pte_scheds[i] =
> -			&adev->sdma.instance[i].ring.sched;
> -	}
> -	adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
> +	amdgpu_sdma_set_vm_pte_scheds(adev);
>  }
>  
>  const struct amdgpu_ip_block_version sdma_v2_4_ip_block = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> index 6b538b6bd18f..fe0c855a803a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> @@ -1684,14 +1684,8 @@ static const struct amdgpu_vm_pte_funcs sdma_v3_0_vm_pte_funcs = {
>  
>  static void sdma_v3_0_set_vm_pte_funcs(struct amdgpu_device *adev)
>  {
> -	unsigned i;
> -
>  	adev->vm_manager.vm_pte_funcs = &sdma_v3_0_vm_pte_funcs;
> -	for (i = 0; i < adev->sdma.num_instances; i++) {
> -		adev->vm_manager.vm_pte_scheds[i] =
> -			 &adev->sdma.instance[i].ring.sched;
> -	}
> -	adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
> +	amdgpu_sdma_set_vm_pte_scheds(adev);
>  }
>  
>  const struct amdgpu_ip_block_version sdma_v3_0_ip_block =
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 60a97d1a82f2..405ccdfbafad 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -2622,18 +2622,8 @@ static const struct amdgpu_vm_pte_funcs sdma_v4_0_vm_pte_funcs = {
>  
>  static void sdma_v4_0_set_vm_pte_funcs(struct amdgpu_device *adev)
>  {
> -	struct drm_gpu_scheduler *sched;
> -	unsigned i;
> -
>  	adev->vm_manager.vm_pte_funcs = &sdma_v4_0_vm_pte_funcs;
> -	for (i = 0; i < adev->sdma.num_instances; i++) {
> -		if (adev->sdma.has_page_queue)
> -			sched = &adev->sdma.instance[i].page.sched;
> -		else
> -			sched = &adev->sdma.instance[i].ring.sched;
> -		adev->vm_manager.vm_pte_scheds[i] = sched;
> -	}
> -	adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
> +	amdgpu_sdma_set_vm_pte_scheds(adev);
>  }
>  
>  static void sdma_v4_0_get_ras_error_count(uint32_t value,
> 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 d265157bc4e1..1b2868b16859 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
> @@ -2323,18 +2323,8 @@ static const struct amdgpu_vm_pte_funcs sdma_v4_4_2_vm_pte_funcs = {
>  
>  static void sdma_v4_4_2_set_vm_pte_funcs(struct amdgpu_device *adev)
>  {
> -	struct drm_gpu_scheduler *sched;
> -	unsigned i;
> -
>  	adev->vm_manager.vm_pte_funcs = &sdma_v4_4_2_vm_pte_funcs;
> -	for (i = 0; i < adev->sdma.num_instances; i++) {
> -		if (adev->sdma.has_page_queue)
> -			sched = &adev->sdma.instance[i].page.sched;
> -		else
> -			sched = &adev->sdma.instance[i].ring.sched;
> -		adev->vm_manager.vm_pte_scheds[i] = sched;
> -	}
> -	adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
> +	amdgpu_sdma_set_vm_pte_scheds(adev);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> index 127f001ebb5a..af920e100400 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> @@ -2081,15 +2081,9 @@ static const struct amdgpu_vm_pte_funcs sdma_v5_0_vm_pte_funcs = {
>  
>  static void sdma_v5_0_set_vm_pte_funcs(struct amdgpu_device *adev)
>  {
> -	unsigned i;
> -
>  	if (adev->vm_manager.vm_pte_funcs == NULL) {
>  		adev->vm_manager.vm_pte_funcs = &sdma_v5_0_vm_pte_funcs;
> -		for (i = 0; i < adev->sdma.num_instances; i++) {
> -			adev->vm_manager.vm_pte_scheds[i] =
> -				&adev->sdma.instance[i].ring.sched;
> -		}
> -		adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
> +		amdgpu_sdma_set_vm_pte_scheds(adev);
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> index 78654ac3047d..29f597d56bcb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> @@ -2091,15 +2091,9 @@ static const struct amdgpu_vm_pte_funcs sdma_v5_2_vm_pte_funcs = {
>  
>  static void sdma_v5_2_set_vm_pte_funcs(struct amdgpu_device *adev)
>  {
> -	unsigned i;
> -
>  	if (adev->vm_manager.vm_pte_funcs == NULL) {
>  		adev->vm_manager.vm_pte_funcs = &sdma_v5_2_vm_pte_funcs;
> -		for (i = 0; i < adev->sdma.num_instances; i++) {
> -			adev->vm_manager.vm_pte_scheds[i] =
> -				&adev->sdma.instance[i].ring.sched;
> -		}
> -		adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
> +		amdgpu_sdma_set_vm_pte_scheds(adev);
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> index d1a7eb6e7ce2..03365db70d9c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> @@ -1897,14 +1897,8 @@ static const struct amdgpu_vm_pte_funcs sdma_v6_0_vm_pte_funcs = {
>  
>  static void sdma_v6_0_set_vm_pte_funcs(struct amdgpu_device *adev)
>  {
> -	unsigned i;
> -
>  	adev->vm_manager.vm_pte_funcs = &sdma_v6_0_vm_pte_funcs;
> -	for (i = 0; i < adev->sdma.num_instances; i++) {
> -		adev->vm_manager.vm_pte_scheds[i] =
> -			&adev->sdma.instance[i].ring.sched;
> -	}
> -	adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
> +	amdgpu_sdma_set_vm_pte_scheds(adev);
>  }
>  
>  const struct amdgpu_ip_block_version sdma_v6_0_ip_block = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
> index 9f15aa1df636..90d291c304d3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
> @@ -1839,14 +1839,8 @@ static const struct amdgpu_vm_pte_funcs sdma_v7_0_vm_pte_funcs = {
>  
>  static void sdma_v7_0_set_vm_pte_funcs(struct amdgpu_device *adev)
>  {
> -	unsigned i;
> -
>  	adev->vm_manager.vm_pte_funcs = &sdma_v7_0_vm_pte_funcs;
> -	for (i = 0; i < adev->sdma.num_instances; i++) {
> -		adev->vm_manager.vm_pte_scheds[i] =
> -			&adev->sdma.instance[i].ring.sched;
> -	}
> -	adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
> +	amdgpu_sdma_set_vm_pte_scheds(adev);
>  }
>  
>  const struct amdgpu_ip_block_version sdma_v7_0_ip_block = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/si_dma.c b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> index 621c6c17e6dd..4422aaeb99e3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/si_dma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> @@ -840,14 +840,8 @@ static const struct amdgpu_vm_pte_funcs si_dma_vm_pte_funcs = {
>  
>  static void si_dma_set_vm_pte_funcs(struct amdgpu_device *adev)
>  {
> -	unsigned i;
> -
>  	adev->vm_manager.vm_pte_funcs = &si_dma_vm_pte_funcs;
> -	for (i = 0; i < adev->sdma.num_instances; i++) {
> -		adev->vm_manager.vm_pte_scheds[i] =
> -			&adev->sdma.instance[i].ring.sched;
> -	}
> -	adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
> +	amdgpu_sdma_set_vm_pte_scheds(adev);
>  }
>  
>  const struct amdgpu_ip_block_version si_dma_ip_block =