Re: [PATCH] drm/amdgpu: fix modprobe failure of the secondary GPU when GDDR6 training enabled(V2)

2020-01-09 Thread Christian König

Am 09.01.20 um 11:17 schrieb Tianci Yin:

From: "Tianci.Yin" 

[why]
In dual GPUs scenario, stolen_size is assigned to zero on the secondary GPU,
since there is no pre-OS console using that memory. Then the bottom region of
VRAM was allocated as GTT, unfortunately a small region of bottom VRAM was
encroached by UMC firmware during GDDR6 BIST training, this cause page fault.

[how]
Forcing stolen_size to 3MB, then the bottom region of VRAM was
allocated as stolen memory, GTT corruption avoid.

Change-Id: I310a72ba0402994defbe50839842a8edb025a868
Signed-off-by: Tianci.Yin 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  5 +
  drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 28 +
  2 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index c91dd602d5f1..e4b2f9bcaeb7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -60,6 +60,11 @@
   */
  #define AMDGPU_GMC_FAULT_TIMEOUT  5000ULL
  
+/*

+ * Default stolen memory size, 1024 * 768 * 4
+ */
+#define AMDGPU_STOLEN_VGA_DEFAULT_SIZE 0x30ULL
+
  struct firmware;
  
  /*

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 7dc8c068c62a..0e0fdf7596eb 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -566,6 +566,13 @@ static int gmc_v10_0_late_init(void *handle)
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
int r;
  
+	/*

+* Can't free the stolen VGA memory when it might be used for memory
+* training again.
+*/
+   if (!adev->fw_vram_usage.mem_train_support)
+   amdgpu_bo_late_init(adev);
+
r = amdgpu_gmc_allocate_vm_inv_eng(adev);
if (r)
return r;
@@ -757,6 +764,19 @@ static int gmc_v10_0_sw_init(void *handle)
  
  	adev->gmc.stolen_size = gmc_v10_0_get_vbios_fb_size(adev);
  
+	/*

+* In dual GPUs scenario, stolen_size is assigned to zero on the
+* secondary GPU, since there is no pre-OS console using that memory.
+* Then the bottom region of VRAM was allocated as GTT, unfortunately a
+* small region of bottom VRAM was encroached by UMC firmware during
+* GDDR6 BIST training, this cause page fault.
+* The page fault can be fixed by forcing stolen_size to 3MB, then the
+* bottom region of VRAM was allocated as stolen memory, GTT corruption
+* avoid.
+*/
+   adev->gmc.stolen_size = max(adev->gmc.stolen_size,
+   AMDGPU_STOLEN_VGA_DEFAULT_SIZE);
+
/* Memory manager */
r = amdgpu_bo_init(adev);
if (r)
@@ -796,6 +816,14 @@ static void gmc_v10_0_gart_fini(struct amdgpu_device *adev)
  static int gmc_v10_0_sw_fini(void *handle)
  {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+   void *stolen_vga_buf;
+
+   /*ALU utilization.  Because of the way that predication is used to deal
with divergent control-flow, you can end up executing more instructions
than any single invocation needs.  As a worst-case example, suppose your
  shader starts with a big switch statement where each case is some
non-trivial piece of code.  Further suppose that you have some shader
thread where each invocation takes a different case in the switch.
Thanks to the way divergent control-flow has to be handled, you would
end up effectively executing each invocation separately and nothing
would be parallelized.
+* Free the stolen VGA memory when it might be used for memory training.
+*/
+   if (adev->fw_vram_usage.mem_train_support)


You can drop the "if", freeing the BO twice is harmless because we set 
the pointer to NULL after the first time.


You might also want to clearify the comment a bit, something like "Free 
the stolen memory if it wasn't already freed in late_init because of 
memory training".


Apart from that looks good to me as well, but you should sync with Alex 
who is going to do the patches.


Cheers,
Christian.


+   amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL,
+ &stolen_vga_buf);
  
  	amdgpu_vm_manager_fini(adev);

gmc_v10_0_gart_fini(adev);


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH] drm/amdgpu: fix modprobe failure of the secondary GPU when GDDR6 training enabled(V2)

2020-01-09 Thread Tianci Yin
From: "Tianci.Yin" 

[why]
In dual GPUs scenario, stolen_size is assigned to zero on the secondary GPU,
since there is no pre-OS console using that memory. Then the bottom region of
VRAM was allocated as GTT, unfortunately a small region of bottom VRAM was
encroached by UMC firmware during GDDR6 BIST training, this cause page fault.

[how]
Forcing stolen_size to 3MB, then the bottom region of VRAM was
allocated as stolen memory, GTT corruption avoid.

Change-Id: I310a72ba0402994defbe50839842a8edb025a868
Signed-off-by: Tianci.Yin 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  5 +
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 28 +
 2 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index c91dd602d5f1..e4b2f9bcaeb7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -60,6 +60,11 @@
  */
 #define AMDGPU_GMC_FAULT_TIMEOUT   5000ULL
 
+/*
+ * Default stolen memory size, 1024 * 768 * 4
+ */
+#define AMDGPU_STOLEN_VGA_DEFAULT_SIZE 0x30ULL
+
 struct firmware;
 
 /*
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 7dc8c068c62a..0e0fdf7596eb 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -566,6 +566,13 @@ static int gmc_v10_0_late_init(void *handle)
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
int r;
 
+   /*
+* Can't free the stolen VGA memory when it might be used for memory
+* training again.
+*/
+   if (!adev->fw_vram_usage.mem_train_support)
+   amdgpu_bo_late_init(adev);
+
r = amdgpu_gmc_allocate_vm_inv_eng(adev);
if (r)
return r;
@@ -757,6 +764,19 @@ static int gmc_v10_0_sw_init(void *handle)
 
adev->gmc.stolen_size = gmc_v10_0_get_vbios_fb_size(adev);
 
+   /*
+* In dual GPUs scenario, stolen_size is assigned to zero on the
+* secondary GPU, since there is no pre-OS console using that memory.
+* Then the bottom region of VRAM was allocated as GTT, unfortunately a
+* small region of bottom VRAM was encroached by UMC firmware during
+* GDDR6 BIST training, this cause page fault.
+* The page fault can be fixed by forcing stolen_size to 3MB, then the
+* bottom region of VRAM was allocated as stolen memory, GTT corruption
+* avoid.
+*/
+   adev->gmc.stolen_size = max(adev->gmc.stolen_size,
+   AMDGPU_STOLEN_VGA_DEFAULT_SIZE);
+
/* Memory manager */
r = amdgpu_bo_init(adev);
if (r)
@@ -796,6 +816,14 @@ static void gmc_v10_0_gart_fini(struct amdgpu_device *adev)
 static int gmc_v10_0_sw_fini(void *handle)
 {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+   void *stolen_vga_buf;
+
+   /*
+* Free the stolen VGA memory when it might be used for memory training.
+*/
+   if (adev->fw_vram_usage.mem_train_support)
+   amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL,
+ &stolen_vga_buf);
 
amdgpu_vm_manager_fini(adev);
gmc_v10_0_gart_fini(adev);
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx