[PATCH v4 2/2] drm/amdgpu: Free VGA stolen memory as soon as possible.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux