[PATCH] drm/amdgpu: fix modprobe failure of the secondary GPU when GDDR6 training enabled(V2)
Christian König
ckoenig.leichtzumerken at gmail.com
Thu Jan 9 10:38:07 UTC 2020
Am 09.01.20 um 11:17 schrieb Tianci Yin:
> From: "Tianci.Yin" <tianci.yin at amd.com>
>
> [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 <tianci.yin at amd.com>
> ---
> 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 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 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);
More information about the amd-gfx
mailing list