[PATCH v4 2/2] drm/amdgpu: Free VGA stolen memory as soon as possible.
Alex Deucher
alexdeucher at gmail.com
Thu Apr 12 13:33:30 UTC 2018
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
>
>
More information about the amd-gfx
mailing list