[PATCH 1/2] Revert "drm/amdgpu: fix modprobe failure of the secondary GPU when GDDR6 training enabled(V5)"

[PATCH 1/2] Revert "drm/amdgpu: fix modprobe failure of the secondary GPU when GDDR6 training enabled(V5)"

[Tianci Yin]

From: "Tianci.Yin" <tianci.yin@amd.com>

This reverts commit 2ad857d7b82081736c078997ba0542acfdd50099.

The patch will be replaced with a better solution, revert it.
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  5 -----
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 27 +------------------------
 2 files changed, 1 insertion(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index 86267baca07c..d3c27a3c43f6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -60,11 +60,6 @@
  */
 #define AMDGPU_GMC_FAULT_TIMEOUT	5000ULL
 
-/*
- * Default stolen memory size, 1024 * 768 * 4
- */
-#define AMDGPU_STOLEN_BIST_TRAINING_DEFAULT_SIZE	0x300000ULL
-
 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 86f4ffe408e7..0c5bf3bd640f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -641,12 +641,7 @@ 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);
+	amdgpu_bo_late_init(adev);
 
 	r = amdgpu_gmc_allocate_vm_inv_eng(adev);
 	if (r)
@@ -830,19 +825,6 @@ 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_BIST_TRAINING_DEFAULT_SIZE);
-
 	/* Memory manager */
 	r = amdgpu_bo_init(adev);
 	if (r)
@@ -882,13 +864,6 @@ 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 memory if it wasn't already freed in late_init
-	 * because of memory training.
-	 */
-	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

[PATCH 2/2] drm/amdgpu: fix VRAM partially encroached issue in GDDR6 memory training

[Tianci Yin]

From: "Tianci.Yin" <tianci.yin@amd.com>

[why]
In GDDR6 BIST training, a certain mount of bottom VRAM will be encroached by
UMC, that causes problems(like GTT corrupted and page fault observed).

[how]
Saving the content of this bottom VRAM to system memory before training, and
restoring it after training to avoid VRAM corruption.

Change-Id: I04a8a6e8e63b3619f7c693fe67883b229cbf3c53
Signed-off-by: Tianci.Yin <tianci.yin@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  2 ++
 drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  | 32 ++++++++++++++++++++++++-
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index 3265487b859f..611021514c52 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -172,6 +172,8 @@ struct psp_dtm_context {
 #define MEM_TRAIN_SYSTEM_SIGNATURE		0x54534942
 #define GDDR6_MEM_TRAINING_DATA_SIZE_IN_BYTES	0x1000
 #define GDDR6_MEM_TRAINING_OFFSET		0x8000
+/*Define the VRAM size that will be encroached by BIST training.*/
+#define GDDR6_MEM_TRAINING_ENCROACHED_SIZE	0x2000000
 
 enum psp_memory_training_init_flag {
 	PSP_MEM_TRAIN_NOT_SUPPORT	= 0x0,
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
index 685dd9754c67..51011b661ba8 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
@@ -972,7 +972,10 @@ static int psp_v11_0_memory_training_init(struct psp_context *psp)
 static int psp_v11_0_memory_training(struct psp_context *psp, uint32_t ops)
 {
 	int ret;
+	void *buf;
+	uint32_t sz;
 	uint32_t p2c_header[4];
+	struct amdgpu_device *adev = psp->adev;
 	struct psp_memory_training_context *ctx = &psp->mem_train_ctx;
 	uint32_t *pcache = (uint32_t*)ctx->sys_cache;
 
@@ -989,7 +992,7 @@ static int psp_v11_0_memory_training(struct psp_context *psp, uint32_t ops)
 		return 0;
 	}
 
-	amdgpu_device_vram_access(psp->adev, ctx->p2c_train_data_offset, p2c_header, sizeof(p2c_header), false);
+	amdgpu_device_vram_access(adev, ctx->p2c_train_data_offset, p2c_header, sizeof(p2c_header), false);
 	DRM_DEBUG("sys_cache[%08x,%08x,%08x,%08x] p2c_header[%08x,%08x,%08x,%08x]\n",
 		  pcache[0], pcache[1], pcache[2], pcache[3],
 		  p2c_header[0], p2c_header[1], p2c_header[2], p2c_header[3]);
@@ -1026,11 +1029,38 @@ static int psp_v11_0_memory_training(struct psp_context *psp, uint32_t ops)
 	DRM_DEBUG("Memory training ops:%x.\n", ops);
 
 	if (ops & PSP_MEM_TRAIN_SEND_LONG_MSG) {
+		/*
+		 * Long traing will encroach certain mount of bottom VRAM,
+		 * saving the content of this bottom VRAM to system memory
+		 * before training, and restoring it after training to avoid
+		 * VRAM corruption.
+		 */
+		sz = GDDR6_MEM_TRAINING_ENCROACHED_SIZE;
+
+		if (adev->gmc.visible_vram_size < sz || !adev->mman.aper_base_kaddr) {
+			DRM_ERROR("visible_vram_size %llx or aper_base_kaddr %p is not initialized.\n",
+				  adev->gmc.visible_vram_size,
+				  adev->mman.aper_base_kaddr);
+			return -EINVAL;
+		}
+
+		buf = vmalloc(sz);
+		if (!buf) {
+			DRM_ERROR("failed to allocate system memory.\n");
+			return -ENOMEM;
+		}
+
+		memcpy_fromio(buf, adev->mman.aper_base_kaddr, sz);
 		ret = psp_v11_0_memory_training_send_msg(psp, PSP_BL__DRAM_LONG_TRAIN);
 		if (ret) {
 			DRM_ERROR("Send long training msg failed.\n");
+			vfree(buf);
 			return ret;
 		}
+
+		memcpy_toio(adev->mman.aper_base_kaddr, buf, sz);
+		adev->nbio.funcs->hdp_flush(adev, NULL);
+		vfree(buf);
 	}
 
 	if (ops & PSP_MEM_TRAIN_SAVE) {
-- 
2.17.1

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

Re: [PATCH 2/2] drm/amdgpu: fix VRAM partially encroached issue in GDDR6 memory training

[Christian König]

Am 20.01.20 um 12:08 schrieb Tianci Yin:
> From: "Tianci.Yin" <tianci.yin@amd.com>
>
> [why]
> In GDDR6 BIST training, a certain mount of bottom VRAM will be encroached by
> UMC, that causes problems(like GTT corrupted and page fault observed).
>
> [how]
> Saving the content of this bottom VRAM to system memory before training, and
> restoring it after training to avoid VRAM corruption.

You need to re-order the patches, this one should come first and the 
other one last.

One more style nit pick below.

>
> Change-Id: I04a8a6e8e63b3619f7c693fe67883b229cbf3c53
> Signed-off-by: Tianci.Yin <tianci.yin@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  2 ++
>   drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  | 32 ++++++++++++++++++++++++-
>   2 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> index 3265487b859f..611021514c52 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> @@ -172,6 +172,8 @@ struct psp_dtm_context {
>   #define MEM_TRAIN_SYSTEM_SIGNATURE		0x54534942
>   #define GDDR6_MEM_TRAINING_DATA_SIZE_IN_BYTES	0x1000
>   #define GDDR6_MEM_TRAINING_OFFSET		0x8000
> +/*Define the VRAM size that will be encroached by BIST training.*/
> +#define GDDR6_MEM_TRAINING_ENCROACHED_SIZE	0x2000000
>   
>   enum psp_memory_training_init_flag {
>   	PSP_MEM_TRAIN_NOT_SUPPORT	= 0x0,
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> index 685dd9754c67..51011b661ba8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> @@ -972,7 +972,10 @@ static int psp_v11_0_memory_training_init(struct psp_context *psp)
>   static int psp_v11_0_memory_training(struct psp_context *psp, uint32_t ops)
>   {
>   	int ret;
> +	void *buf;
> +	uint32_t sz;
>   	uint32_t p2c_header[4];
> +	struct amdgpu_device *adev = psp->adev;
>   	struct psp_memory_training_context *ctx = &psp->mem_train_ctx;
>   	uint32_t *pcache = (uint32_t*)ctx->sys_cache;

In general it is preferred to order the lines in reverse xmas tree.

E.g. long lines with pre-initializes variables such as adev, ctx, pcache 
first. And temporary stuff like i, ret, buf etc last.

Apart from that this looks good to me,
Christian.

>   
> @@ -989,7 +992,7 @@ static int psp_v11_0_memory_training(struct psp_context *psp, uint32_t ops)
>   		return 0;
>   	}
>   
> -	amdgpu_device_vram_access(psp->adev, ctx->p2c_train_data_offset, p2c_header, sizeof(p2c_header), false);
> +	amdgpu_device_vram_access(adev, ctx->p2c_train_data_offset, p2c_header, sizeof(p2c_header), false);
>   	DRM_DEBUG("sys_cache[%08x,%08x,%08x,%08x] p2c_header[%08x,%08x,%08x,%08x]\n",
>   		  pcache[0], pcache[1], pcache[2], pcache[3],
>   		  p2c_header[0], p2c_header[1], p2c_header[2], p2c_header[3]);
> @@ -1026,11 +1029,38 @@ static int psp_v11_0_memory_training(struct psp_context *psp, uint32_t ops)
>   	DRM_DEBUG("Memory training ops:%x.\n", ops);
>   
>   	if (ops & PSP_MEM_TRAIN_SEND_LONG_MSG) {
> +		/*
> +		 * Long traing will encroach certain mount of bottom VRAM,
> +		 * saving the content of this bottom VRAM to system memory
> +		 * before training, and restoring it after training to avoid
> +		 * VRAM corruption.
> +		 */
> +		sz = GDDR6_MEM_TRAINING_ENCROACHED_SIZE;
> +
> +		if (adev->gmc.visible_vram_size < sz || !adev->mman.aper_base_kaddr) {
> +			DRM_ERROR("visible_vram_size %llx or aper_base_kaddr %p is not initialized.\n",
> +				  adev->gmc.visible_vram_size,
> +				  adev->mman.aper_base_kaddr);
> +			return -EINVAL;
> +		}
> +
> +		buf = vmalloc(sz);
> +		if (!buf) {
> +			DRM_ERROR("failed to allocate system memory.\n");
> +			return -ENOMEM;
> +		}
> +
> +		memcpy_fromio(buf, adev->mman.aper_base_kaddr, sz);
>   		ret = psp_v11_0_memory_training_send_msg(psp, PSP_BL__DRAM_LONG_TRAIN);
>   		if (ret) {
>   			DRM_ERROR("Send long training msg failed.\n");
> +			vfree(buf);
>   			return ret;
>   		}
> +
> +		memcpy_toio(adev->mman.aper_base_kaddr, buf, sz);
> +		adev->nbio.funcs->hdp_flush(adev, NULL);
> +		vfree(buf);
>   	}
>   
>   	if (ops & PSP_MEM_TRAIN_SAVE) {

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

Re: [PATCH 2/2] drm/amdgpu: fix VRAM partially encroached issue in GDDR6 memory training

[Yin, Tianci (Rico)]

[-- Attachment #1.1: Type: text/plain, Size: 5587 bytes --]

[AMD Official Use Only - Internal Distribution Only]

Thanks very much Christian!

Please review again.

Rico
________________________________
From: Christian König <ckoenig.leichtzumerken@gmail.com>
Sent: Monday, January 20, 2020 19:58
To: Yin, Tianci (Rico) <Tianci.Yin@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Long, Gang <Gang.Long@amd.com>; Xu, Feifei <Feifei.Xu@amd.com>; Wang, Kevin(Yang) <Kevin1.Wang@amd.com>; Tuikov, Luben <Luben.Tuikov@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Yuan, Xiaojie <Xiaojie.Yuan@amd.com>
Subject: Re: [PATCH 2/2] drm/amdgpu: fix VRAM partially encroached issue in GDDR6 memory training

Am 20.01.20 um 12:08 schrieb Tianci Yin:
> From: "Tianci.Yin" <tianci.yin@amd.com>
>
> [why]
> In GDDR6 BIST training, a certain mount of bottom VRAM will be encroached by
> UMC, that causes problems(like GTT corrupted and page fault observed).
>
> [how]
> Saving the content of this bottom VRAM to system memory before training, and
> restoring it after training to avoid VRAM corruption.

You need to re-order the patches, this one should come first and the
other one last.

One more style nit pick below.

>
> Change-Id: I04a8a6e8e63b3619f7c693fe67883b229cbf3c53
> Signed-off-by: Tianci.Yin <tianci.yin@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  2 ++
>   drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  | 32 ++++++++++++++++++++++++-
>   2 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> index 3265487b859f..611021514c52 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> @@ -172,6 +172,8 @@ struct psp_dtm_context {
>   #define MEM_TRAIN_SYSTEM_SIGNATURE          0x54534942
>   #define GDDR6_MEM_TRAINING_DATA_SIZE_IN_BYTES       0x1000
>   #define GDDR6_MEM_TRAINING_OFFSET           0x8000
> +/*Define the VRAM size that will be encroached by BIST training.*/
> +#define GDDR6_MEM_TRAINING_ENCROACHED_SIZE   0x2000000
>
>   enum psp_memory_training_init_flag {
>        PSP_MEM_TRAIN_NOT_SUPPORT       = 0x0,
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> index 685dd9754c67..51011b661ba8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> @@ -972,7 +972,10 @@ static int psp_v11_0_memory_training_init(struct psp_context *psp)
>   static int psp_v11_0_memory_training(struct psp_context *psp, uint32_t ops)
>   {
>        int ret;
> +     void *buf;
> +     uint32_t sz;
>        uint32_t p2c_header[4];
> +     struct amdgpu_device *adev = psp->adev;
>        struct psp_memory_training_context *ctx = &psp->mem_train_ctx;
>        uint32_t *pcache = (uint32_t*)ctx->sys_cache;

In general it is preferred to order the lines in reverse xmas tree.

E.g. long lines with pre-initializes variables such as adev, ctx, pcache
first. And temporary stuff like i, ret, buf etc last.

Apart from that this looks good to me,
Christian.

>
> @@ -989,7 +992,7 @@ static int psp_v11_0_memory_training(struct psp_context *psp, uint32_t ops)
>                return 0;
>        }
>
> -     amdgpu_device_vram_access(psp->adev, ctx->p2c_train_data_offset, p2c_header, sizeof(p2c_header), false);
> +     amdgpu_device_vram_access(adev, ctx->p2c_train_data_offset, p2c_header, sizeof(p2c_header), false);
>        DRM_DEBUG("sys_cache[%08x,%08x,%08x,%08x] p2c_header[%08x,%08x,%08x,%08x]\n",
>                  pcache[0], pcache[1], pcache[2], pcache[3],
>                  p2c_header[0], p2c_header[1], p2c_header[2], p2c_header[3]);
> @@ -1026,11 +1029,38 @@ static int psp_v11_0_memory_training(struct psp_context *psp, uint32_t ops)
>        DRM_DEBUG("Memory training ops:%x.\n", ops);
>
>        if (ops & PSP_MEM_TRAIN_SEND_LONG_MSG) {
> +             /*
> +              * Long traing will encroach certain mount of bottom VRAM,
> +              * saving the content of this bottom VRAM to system memory
> +              * before training, and restoring it after training to avoid
> +              * VRAM corruption.
> +              */
> +             sz = GDDR6_MEM_TRAINING_ENCROACHED_SIZE;
> +
> +             if (adev->gmc.visible_vram_size < sz || !adev->mman.aper_base_kaddr) {
> +                     DRM_ERROR("visible_vram_size %llx or aper_base_kaddr %p is not initialized.\n",
> +                               adev->gmc.visible_vram_size,
> +                               adev->mman.aper_base_kaddr);
> +                     return -EINVAL;
> +             }
> +
> +             buf = vmalloc(sz);
> +             if (!buf) {
> +                     DRM_ERROR("failed to allocate system memory.\n");
> +                     return -ENOMEM;
> +             }
> +
> +             memcpy_fromio(buf, adev->mman.aper_base_kaddr, sz);
>                ret = psp_v11_0_memory_training_send_msg(psp, PSP_BL__DRAM_LONG_TRAIN);
>                if (ret) {
>                        DRM_ERROR("Send long training msg failed.\n");
> +                     vfree(buf);
>                        return ret;
>                }
> +
> +             memcpy_toio(adev->mman.aper_base_kaddr, buf, sz);
> +             adev->nbio.funcs->hdp_flush(adev, NULL);
> +             vfree(buf);
>        }
>
>        if (ops & PSP_MEM_TRAIN_SAVE) {


[-- Attachment #1.2: Type: text/html, Size: 11376 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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