I don't think postpone RAS recovery init is not one reasonable proposal. What 
we do in recovery init is to read the retired page if there is, and retire 
corresponding memory, this will make sure these pages are reserved well before 
setting up memory manager and reserving other memory blocks, it will be safe.

Regards,
Guchun

-----Original Message-----
From: Andrey Grodzovsky <[email protected]> 
Sent: Saturday, October 19, 2019 4:49 AM
To: [email protected]
Cc: Chen, Guchun <[email protected]>; Zhou1, Tao <[email protected]>; 
Deucher, Alexander <[email protected]>; [email protected]; 
Quan, Evan <[email protected]>; Grodzovsky, Andrey <[email protected]>
Subject: [PATCH 4/4] drm/amdgpu: Move amdgpu_ras_recovery_init to after SMU 
ready.

For Arcturus the I2C traffic is done through SMU tables and so we must postpone 
RAS recovery init to after they are ready which is in 
amdgpu_device_ip_hw_init_phase2.

Signed-off-by: Andrey Grodzovsky <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 13 +++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 11 -----------
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 17cfdaf..c40e9a5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1850,6 +1850,19 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
*adev)
        if (r)
                goto init_failed;
 
+       /*
+        * retired pages will be loaded from eeprom and reserved here,
+        * it should be called after amdgpu_device_ip_hw_init_phase2  since
+        * for some ASICs the RAS EEPROM code relies on SMU fully functioning
+        * for I2C communication which only true at this point.
+        * recovery_init may fail, but it can free all resources allocated by
+        * itself and its failure should not stop amdgpu init process.
+        *
+        * Note: theoretically, this should be called before all vram 
allocations
+        * to protect retired page from abusing
+        */
+       amdgpu_ras_recovery_init(adev);
+
        if (adev->gmc.xgmi.num_physical_nodes > 1)
                amdgpu_xgmi_add_device(adev);
        amdgpu_amdkfd_device_init(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 2e85a51..1045c3f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1721,17 +1721,6 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)  #endif
 
        /*
-        * retired pages will be loaded from eeprom and reserved here,
-        * it should be called after ttm init since new bo may be created,
-        * recovery_init may fail, but it can free all resources allocated by
-        * itself and its failure should not stop amdgpu init process.
-        *
-        * Note: theoretically, this should be called before all vram 
allocations
-        * to protect retired page from abusing
-        */
-       amdgpu_ras_recovery_init(adev);
-
-       /*
         *The reserved vram for firmware must be pinned to the specified
         *place on the VRAM, so reserve it early.
         */
--
2.7.4

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to