[PATCH v2 1/3] drm/amdgpu: don't disable ttm buffer funcs on reset

[PATCH v2 1/3] drm/amdgpu: don't disable ttm buffer funcs on reset

[Pierre-Eric Pelloux-Prayer]

Access to hw is already suspended so there are no reasons to
disable ttm buffer funcs.

Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 6608780ffef2..57e10afb4714 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3167,8 +3167,6 @@ static int amdgpu_device_ip_suspend(struct amdgpu_device *adev)
 		amdgpu_virt_request_full_gpu(adev, false);
 	}
 
-	amdgpu_ttm_disable_buffer_funcs(adev);
-
 	r = amdgpu_device_ip_suspend_phase1(adev);
 	if (r)
 		return r;
-- 
2.43.0

[PATCH v2 2/3] drm/amdgpu: delay ttm buffer func enablement on xgmi

[Pierre-Eric Pelloux-Prayer]

When amdgpu_init_minimal_xgmi is used, SDMA engines init
is delayed so amdgpu_ttm_enable_buffer_funcs must be
called later.

Without this, the check for num_buffer_funcs_scheds will
fail and using ttm buffer funcs later will fail.

---
v2: no need to add a new call. Instead move down the one from
    amdgpu_device_reinit_after_reset after amdgpu_ib_ring_tests
---

Fixes: 3a5da695c8d1 ("drm/amdgpu: only use working sdma schedulers for ttm")
Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 57e10afb4714..53150fe55350 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2479,7 +2479,8 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
 	if (r)
 		goto init_failed;
 
-	amdgpu_ttm_enable_buffer_funcs(adev);
+	if (amdgpu_ip_member_of_hwini(adev, AMD_IP_BLOCK_TYPE_SDMA))
+		amdgpu_ttm_enable_buffer_funcs(adev);
 
 	/* Don't init kfd if whole hive need to be reset during init */
 	if (adev->init_lvl->level != AMDGPU_INIT_LEVEL_MINIMAL_XGMI) {
@@ -5253,8 +5254,6 @@ int amdgpu_device_reinit_after_reset(struct amdgpu_reset_context *reset_context)
 				if (r)
 					goto out;
 
-				amdgpu_ttm_enable_buffer_funcs(tmp_adev);
-
 				r = amdgpu_device_ip_resume_phase3(tmp_adev);
 				if (r)
 					goto out;
@@ -5320,6 +5319,8 @@ int amdgpu_device_reinit_after_reset(struct amdgpu_reset_context *reset_context)
 				r = -EAGAIN;
 				goto end;
 			}
+
+			amdgpu_ttm_enable_buffer_funcs(tmp_adev);
 		}
 
 		if (r)
-- 
2.43.0

Re: [PATCH v2 2/3] drm/amdgpu: delay ttm buffer func enablement on xgmi

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The relocated call to amdgpu_ttm_enable_buffer_funcs() silently fails due to the amdgpu_in_reset() guard, leaving TTM DMA permanently disabled.
--

commit 61a1713a716e72510eef36a97d31d040637c47f6
Author: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>

drm/amdgpu: delay ttm buffer func enablement on xgmi

When amdgpu_init_minimal_xgmi is used, SDMA engines init is delayed so
amdgpu_ttm_enable_buffer_funcs must be called later. This commit moves
the call into amdgpu_device_reinit_after_reset to avoid initialization
failures.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 57e10afb4714d..53150fe553508 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
[ ... ]
> @@ -5320,6 +5319,8 @@ int amdgpu_device_reinit_after_reset(struct amdgpu_reset_context *reset_context)
>  				r = -EAGAIN;
>  				goto end;
>  			}
> +
> +			amdgpu_ttm_enable_buffer_funcs(tmp_adev);

[Severity: High]
This isn't a bug introduced by this patch, but doesn't this relocated call
silently fail due to the amdgpu_in_reset() guard?

Because amdgpu_device_reinit_after_reset() executes within a hardware reset
context, the in_gpu_reset flag is set. Looking at
amdgpu_ttm_enable_buffer_funcs() in drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c,
there is an early return guard for this:

	if (!adev->mman.initialized || amdgpu_in_reset(adev) ||
	    adev->mman.buffer_funcs_enabled || adev->gmc.is_app_apu)
		return;

Will this cause the function to silently abort, leaving TTM DMA permanently
disabled and forcing all VRAM migrations to fallback to slow CPU copies?

Also, by placing the call at the very end of amdgpu_device_reinit_after_reset(),
it executes after drm_client_dev_resume(). Would any VRAM migrations triggered
by fbdev restore run without hardware DMA even if the early return was bypassed?

>  		}
>  
>  		if (r)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617132835.12306-1-pierre-eric.pelloux-prayer@amd.com?part=2

[PATCH v2 3/3] drm/amdgpu: remove useless enable/disable of ttm buffer funcs

[Pierre-Eric Pelloux-Prayer]

amdgpu_ttm_disable_buffer_funcs and amdgpu_ttm_enable_buffer_funcs should
each only be called once per amdgpu_device: at startup and teardown.

Therefore the other calls are redundant and can be safely dropped.

Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 5 -----
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 1 -
 2 files changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 53150fe55350..6abcde8a1a00 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3381,8 +3381,6 @@ static int amdgpu_device_ip_resume(struct amdgpu_device *adev)
 
 	r = amdgpu_device_ip_resume_phase2(adev);
 
-	amdgpu_ttm_enable_buffer_funcs(adev);
-
 	if (r)
 		return r;
 
@@ -4505,8 +4503,6 @@ int amdgpu_device_suspend(struct drm_device *dev, bool notify_clients)
 	if (r)
 		goto unwind_userq;
 
-	amdgpu_ttm_disable_buffer_funcs(adev);
-
 	amdgpu_fence_driver_hw_fini(adev);
 
 	r = amdgpu_device_ip_suspend_phase2(adev);
@@ -4519,7 +4515,6 @@ int amdgpu_device_suspend(struct drm_device *dev, bool notify_clients)
 	return 0;
 
 unwind_evict:
-	amdgpu_ttm_enable_buffer_funcs(adev);
 	amdgpu_fence_driver_hw_init(adev);
 
 unwind_userq:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 6c6ab4dd6ea9..95f879e723ea 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -2099,7 +2099,6 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
 	}
 
 	/* Change the size here instead of the init above so only lpfn is affected */
-	amdgpu_ttm_disable_buffer_funcs(adev);
 #ifdef CONFIG_64BIT
 #ifdef CONFIG_X86
 	if (adev->gmc.xgmi.connected_to_cpu)
-- 
2.43.0