On 04/12/2018 12:32 AM, Alex Deucher wrote: > On Thu, Apr 12, 2018 at 12:08 AM, Andrey Grodzovsky > <andrey.grodzovsky at amd.com> wrote: >> Reserved VRAM is used to avoid overriding pre OS FB. >> Once our display stack takes over we don't need the reserved >> VRAM anymore. >> >> v2: >> Remove comment, we know actually why we need to reserve the stolen VRAM. >> Fix return type for amdgpu_ttm_late_init. >> v3: >> Return 0 in amdgpu_bo_late_init, rebase on changes to previous patch >> v4: >> Don't release stolen memory for GMC9 ASICs untill GART corruption >> on S3 resume is resolved. >> >> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky at amd.com> >> Signed-off-by: Alex Deucher <alexander.deucher at amd.com> > Looks like you used the original version of this patch as well. > Updated version here: > https://patchwork.freedesktop.org/patch/215567/ > more comments below. > >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 7 +++++++ >> drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 + >> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 ++++++-- >> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 + >> drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 2 ++ >> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 2 ++ >> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 2 ++ >> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 14 ++++++++++---- >> 8 files changed, 31 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >> index 9e23d6f..a160ef0 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >> @@ -852,6 +852,13 @@ int amdgpu_bo_init(struct amdgpu_device *adev) >> return amdgpu_ttm_init(adev); >> } >> >> +int amdgpu_bo_late_init(struct amdgpu_device *adev) >> +{ >> + amdgpu_ttm_late_init(adev); >> + >> + return 0; >> +} >> + >> void amdgpu_bo_fini(struct amdgpu_device *adev) >> { >> amdgpu_ttm_fini(adev); >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h >> index 3bee133..1e9fe85 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h >> @@ -251,6 +251,7 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain, >> int amdgpu_bo_unpin(struct amdgpu_bo *bo); >> int amdgpu_bo_evict_vram(struct amdgpu_device *adev); >> int amdgpu_bo_init(struct amdgpu_device *adev); >> +int amdgpu_bo_late_init(struct amdgpu_device *adev); >> void amdgpu_bo_fini(struct amdgpu_device *adev); >> int amdgpu_bo_fbdev_mmap(struct amdgpu_bo *bo, >> struct vm_area_struct *vma); >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >> index 0555821..7a608cf 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >> @@ -1517,14 +1517,18 @@ int amdgpu_ttm_init(struct amdgpu_device *adev) >> return 0; >> } >> >> +void amdgpu_ttm_late_init(struct amdgpu_device *adev) >> +{ >> + if (adev->gmc.stolen_size) > no need to check for NULL here. amdgpu_bo_free_kernel() will do the > right thing even if stolen_vga_memory is NULL. > >> + amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL, NULL); >> +} >> + >> void amdgpu_ttm_fini(struct amdgpu_device *adev) >> { >> if (!adev->mman.initialized) >> return; >> >> amdgpu_ttm_debugfs_fini(adev); >> - if (adev->gmc.stolen_size) >> - amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL, NULL); >> amdgpu_ttm_fw_reserve_vram_fini(adev); >> if (adev->mman.aper_base_kaddr) >> iounmap(adev->mman.aper_base_kaddr); >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h >> index 6ea7de8..e969c87 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h >> @@ -77,6 +77,7 @@ uint64_t amdgpu_vram_mgr_usage(struct ttm_mem_type_manager *man); >> uint64_t amdgpu_vram_mgr_vis_usage(struct ttm_mem_type_manager *man); >> >> int amdgpu_ttm_init(struct amdgpu_device *adev); >> +void amdgpu_ttm_late_init(struct amdgpu_device *adev); >> void amdgpu_ttm_fini(struct amdgpu_device *adev); >> void amdgpu_ttm_set_buffer_funcs_status(struct amdgpu_device *adev, >> bool enable); >> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c >> index 63f0b65..4a8f9bd 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c >> @@ -819,6 +819,8 @@ static int gmc_v6_0_late_init(void *handle) >> { >> struct amdgpu_device *adev = (struct amdgpu_device *)handle; >> >> + amdgpu_bo_late_init(adev); >> + >> if (amdgpu_vm_fault_stop != AMDGPU_VM_FAULT_STOP_ALWAYS) >> return amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0); >> else >> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c >> index 2deb5c9..189fdf9 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c >> @@ -958,6 +958,8 @@ static int gmc_v7_0_late_init(void *handle) >> { >> struct amdgpu_device *adev = (struct amdgpu_device *)handle; >> >> + amdgpu_bo_late_init(adev); >> + >> if (amdgpu_vm_fault_stop != AMDGPU_VM_FAULT_STOP_ALWAYS) >> return amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0); >> else >> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c >> index 04b00df..19e153f 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c >> @@ -1049,6 +1049,8 @@ static int gmc_v8_0_late_init(void *handle) >> { >> struct amdgpu_device *adev = (struct amdgpu_device *)handle; >> >> + amdgpu_bo_late_init(adev); >> + >> if (amdgpu_vm_fault_stop != AMDGPU_VM_FAULT_STOP_ALWAYS) >> return amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0); >> else >> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c >> index 252a6c69..099e3ce5 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c >> @@ -659,6 +659,16 @@ static int gmc_v9_0_late_init(void *handle) >> unsigned i; >> int r; >> >> + /* >> + * TODO: >> + * Currently there is a bug where some memory client outside >> + * of the driver writes to first 8M of VRAM on S3 resume, >> + * this overrides GART which by default gets placed in first 8M and >> + * causes VM_FAULTS once GTT is accessed. >> + * Keep the stolen memory reservation until this solved. >> + */ >> + /* amdgpu_bo_late_init(adev); / >> + > We still need to free this somewhere. I'd suggest calling it in > gmc_v9_0_sw_fini() and add a comment there about moving it when we fix > the issue. > >> for(i = 0; i < adev->num_rings; ++i) { >> struct amdgpu_ring *ring = adev->rings[i]; >> unsigned vmhub = ring->funcs->vmhub; >> @@ -884,10 +894,6 @@ static int gmc_v9_0_sw_init(void *handle) >> */ >> adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */ >> >> - /* >> - * It needs to reserve 8M stolen memory for vega10 >> - * TODO: Figure out how to avoid that... >> - */ >> adev->gmc.stolen_size = gmc_v9_0_get_vbios_fb_size(adev); > We may also just want to return 8MB or 9MB temporarily in > gmc_v9_0_get_vbios_fb_size until we sort out the root cause of the S3 > issue otherwise we're potentially wasting a lot more memory. > > Alex But what if we have 4k display ? In this case returning 9M probably will not hide the corruption we were originally dealing with. I remember in that case pre OS FB size would be 32M. Andrey > > >> /* set DMA mask + need_dma32 flags. >> -- >> 2.7.4 >> >> _______________________________________________ >> amd-gfx mailing list >> amd-gfx at lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
