On Thu, Apr 12, 2018 at 7:17 AM, Andrey Grodzovsky <Andrey.Grodzovsky at amd.com> wrote: > > > 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. I guess it's a trade off, possible garbage monentary during bios to driver transition vs. wasting an additional 24 MB of CPU accessible vram for the life of the driver. Alex > > 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 > >
