[PATCH 1/2] drm/amdgpu/gmc: steal the appropriate amount of vram for fw hand-over (v2)
Andrey Grodzovsky
Andrey.Grodzovsky at amd.com
Mon Apr 9 15:17:58 UTC 2018
OK, tested with DC disabled , no issues on resume (no visible corruption
on display or errors in log). Now the display itself freezes after
amdgpu is loaded with DC disabled, this happens only when BIOS in VGA
mode , in console mode no such problem. Happens before my and Alex
patches, looks like a separate issue.
So anyway, if corruption would be there (beginning of VRAM and hence
scanout FB corrupted) , i should have seen it with grub in console mode
where display is fine and not freezing.
Andrey
On 04/09/2018 09:50 AM, Christian König wrote:
> Hi Andrey,
>
> I think the problem Ray wants to point to is that we now release the
> stolen memory after device initialization.
>
> So during S3 we might run into issues because the first 8MB of VRAM
> are corrupted after startup.
>
> Christian.
>
> Am 09.04.2018 um 15:26 schrieb Grodzovsky, Andrey:
>> Top posting (mobile)
>>
>> I tested S3 with DC enabled only. Even if I disable DC I need a
>> device with less then 8M VRAM to reproduce it, don't I ? Otherwise we
>> just gonna reserve pre OS FB size of VRAM and not corrupt it. Right ?
>> Should probably test it with forcing VRAM size to less then 8M...
>>
>> Andrey
>>
>> ________________________________________
>> From: Huang Rui <ray.huang at amd.com>
>> Sent: 09 April 2018 04:23:06
>> To: Alex Deucher; Grodzovsky, Andrey
>> Cc: amd-gfx at lists.freedesktop.org; Deucher, Alexander
>> Subject: Re: [PATCH 1/2] drm/amdgpu/gmc: steal the appropriate amount
>> of vram for fw hand-over (v2)
>>
>> On Fri, Apr 06, 2018 at 02:54:09PM -0500, Alex Deucher wrote:
>>> Steal 9 MB for vga emulation and fb if vga is enabled, otherwise,
>>> steal enough to cover the current display size as set by the vbios.
>>>
>>> If no memory is used (e.g., secondary or headless card), skip
>>> stolen memory reserve.
>>>
>>> v2: skip reservation if vram is limited, address Christian's comments
>>>
>>> Reviewed-and-Tested-by: Andrey Grodzovsky
>>> <andrey.grodzovsky at amd.com> (v1)
>>> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 14 +++++----
>>> drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 23 +++++++++++++--
>>> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 23 +++++++++++++--
>>> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 23 +++++++++++++--
>>> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 51
>>> +++++++++++++++++++++++++++++----
>>> 5 files changed, 116 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> index 205da3ff9cd0..46c69ad34461 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> @@ -1454,12 +1454,14 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>>> return r;
>>> }
>>>
>>> - r = amdgpu_bo_create_kernel(adev, adev->gmc.stolen_size,
>>> PAGE_SIZE,
>>> - AMDGPU_GEM_DOMAIN_VRAM,
>>> - &adev->stolen_vga_memory,
>>> - NULL, NULL);
>>> - if (r)
>>> - return r;
>>> + if (adev->gmc.stolen_size) {
>>> + r = amdgpu_bo_create_kernel(adev,
>>> adev->gmc.stolen_size, PAGE_SIZE,
>>> + AMDGPU_GEM_DOMAIN_VRAM,
>>> + &adev->stolen_vga_memory,
>>> + NULL, NULL);
>>> + if (r)
>>> + return r;
>>> + }
>>> DRM_INFO("amdgpu: %uM of VRAM memory ready\n",
>>> (unsigned) (adev->gmc.real_vram_size / (1024 * 1024)));
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>>> index 5617cf62c566..24e1ea36b454 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>>> @@ -825,6 +825,25 @@ static int gmc_v6_0_late_init(void *handle)
>>> return 0;
>>> }
>>>
>>> +static unsigned gmc_v6_0_get_vbios_fb_size(struct amdgpu_device *adev)
>>> +{
>>> + u32 d1vga_control = RREG32(mmD1VGA_CONTROL);
>>> + unsigned size;
>>> +
>>> + if (REG_GET_FIELD(d1vga_control, D1VGA_CONTROL,
>>> D1VGA_MODE_ENABLE)) {
>>> + size = 9 * 1024 * 1024; /* reserve 8MB for vga
>>> emulator and 1 MB for FB */
>>> + } else {
>>> + u32 viewport = RREG32(mmVIEWPORT_SIZE);
>>> + size = (REG_GET_FIELD(viewport, VIEWPORT_SIZE,
>>> VIEWPORT_HEIGHT) *
>>> + REG_GET_FIELD(viewport, VIEWPORT_SIZE,
>>> VIEWPORT_WIDTH) *
>>> + 4);
>>> + }
>>> + /* return 0 if the pre-OS buffer uses up most of vram */
>>> + if ((adev->gmc.real_vram_size - size) < (8 * 1024 * 1024))
>>> + return 0;
>>> + return size;
>>> +}
>>> +
>>> static int gmc_v6_0_sw_init(void *handle)
>>> {
>>> int r;
>>> @@ -851,8 +870,6 @@ static int gmc_v6_0_sw_init(void *handle)
>>>
>>> adev->gmc.mc_mask = 0xffffffffffULL;
>>>
>>> - adev->gmc.stolen_size = 256 * 1024;
>>> -
>>> adev->need_dma32 = false;
>>> dma_bits = adev->need_dma32 ? 32 : 40;
>>> r = pci_set_dma_mask(adev->pdev, DMA_BIT_MASK(dma_bits));
>>> @@ -878,6 +895,8 @@ static int gmc_v6_0_sw_init(void *handle)
>>> if (r)
>>> return r;
>>>
>>> + adev->gmc.stolen_size = gmc_v6_0_get_vbios_fb_size(adev);
>>> +
>>> r = amdgpu_bo_init(adev);
>>> if (r)
>>> return r;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> index 80054f36e487..93861f9c7773 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> @@ -964,6 +964,25 @@ static int gmc_v7_0_late_init(void *handle)
>>> return 0;
>>> }
>>>
>>> +static unsigned gmc_v7_0_get_vbios_fb_size(struct amdgpu_device *adev)
>>> +{
>>> + u32 d1vga_control = RREG32(mmD1VGA_CONTROL);
>>> + unsigned size;
>>> +
>>> + if (REG_GET_FIELD(d1vga_control, D1VGA_CONTROL,
>>> D1VGA_MODE_ENABLE)) {
>>> + size = 9 * 1024 * 1024; /* reserve 8MB for vga
>>> emulator and 1 MB for FB */
>>> + } else {
>>> + u32 viewport = RREG32(mmVIEWPORT_SIZE);
>>> + size = (REG_GET_FIELD(viewport, VIEWPORT_SIZE,
>>> VIEWPORT_HEIGHT) *
>>> + REG_GET_FIELD(viewport, VIEWPORT_SIZE,
>>> VIEWPORT_WIDTH) *
>>> + 4);
>>> + }
>>> + /* return 0 if the pre-OS buffer uses up most of vram */
>>> + if ((adev->gmc.real_vram_size - size) < (8 * 1024 * 1024))
>>> + return 0;
>>> + return size;
>>> +}
>>> +
>>> static int gmc_v7_0_sw_init(void *handle)
>>> {
>>> int r;
>>> @@ -998,8 +1017,6 @@ static int gmc_v7_0_sw_init(void *handle)
>>> */
>>> adev->gmc.mc_mask = 0xffffffffffULL; /* 40 bit MC */
>>>
>>> - adev->gmc.stolen_size = 256 * 1024;
>>> -
>>> /* set DMA mask + need_dma32 flags.
>>> * PCIE - can handle 40-bits.
>>> * IGP - can handle 40-bits
>>> @@ -1030,6 +1047,8 @@ static int gmc_v7_0_sw_init(void *handle)
>>> if (r)
>>> return r;
>>>
>>> + adev->gmc.stolen_size = gmc_v7_0_get_vbios_fb_size(adev);
>>> +
>>> /* Memory manager */
>>> r = amdgpu_bo_init(adev);
>>> if (r)
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> index d71d4cb68f9c..fbd8f56c70f3 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> @@ -1055,6 +1055,25 @@ static int gmc_v8_0_late_init(void *handle)
>>> return 0;
>>> }
>>>
>>> +static unsigned gmc_v8_0_get_vbios_fb_size(struct amdgpu_device *adev)
>>> +{
>>> + u32 d1vga_control = RREG32(mmD1VGA_CONTROL);
>>> + unsigned size;
>>> +
>>> + if (REG_GET_FIELD(d1vga_control, D1VGA_CONTROL,
>>> D1VGA_MODE_ENABLE)) {
>>> + size = 9 * 1024 * 1024; /* reserve 8MB for vga
>>> emulator and 1 MB for FB */
>>> + } else {
>>> + u32 viewport = RREG32(mmVIEWPORT_SIZE);
>>> + size = (REG_GET_FIELD(viewport, VIEWPORT_SIZE,
>>> VIEWPORT_HEIGHT) *
>>> + REG_GET_FIELD(viewport, VIEWPORT_SIZE,
>>> VIEWPORT_WIDTH) *
>>> + 4);
>>> + }
>>> + /* return 0 if the pre-OS buffer uses up most of vram */
>>> + if ((adev->gmc.real_vram_size - size) < (8 * 1024 * 1024))
>>> + return 0;
>>> + return size;
>>> +}
>>> +
>>> #define mmMC_SEQ_MISC0_FIJI 0xA71
>>>
>>> static int gmc_v8_0_sw_init(void *handle)
>>> @@ -1096,8 +1115,6 @@ static int gmc_v8_0_sw_init(void *handle)
>>> */
>>> adev->gmc.mc_mask = 0xffffffffffULL; /* 40 bit MC */
>>>
>>> - adev->gmc.stolen_size = 256 * 1024;
>>> -
>>> /* set DMA mask + need_dma32 flags.
>>> * PCIE - can handle 40-bits.
>>> * IGP - can handle 40-bits
>>> @@ -1128,6 +1145,8 @@ static int gmc_v8_0_sw_init(void *handle)
>>> if (r)
>>> return r;
>>>
>>> + adev->gmc.stolen_size = gmc_v8_0_get_vbios_fb_size(adev);
>>> +
>>> /* Memory manager */
>>> r = amdgpu_bo_init(adev);
>>> if (r)
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> index 070946e1e4a7..fcc11a29c027 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> @@ -57,6 +57,14 @@
>>> #define DF_CS_AON0_DramBaseAddress0__IntLvAddrSel_MASK 0x00000700L
>>> #define DF_CS_AON0_DramBaseAddress0__DramBaseAddr_MASK 0xFFFFF000L
>>>
>>> +/* add these here since we already include dce12 headers and these
>>> are for DCN */
>>> +#define mmHUBP0_DCSURF_PRI_VIEWPORT_DIMENSION 0x055d
>>> +#define mmHUBP0_DCSURF_PRI_VIEWPORT_DIMENSION_BASE_IDX 2
>>> +#define
>>> HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION__PRI_VIEWPORT_WIDTH__SHIFT 0x0
>>> +#define
>>> HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION__PRI_VIEWPORT_HEIGHT__SHIFT 0x10
>>> +#define
>>> HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION__PRI_VIEWPORT_WIDTH_MASK
>>> 0x00003FFFL
>>> +#define
>>> HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION__PRI_VIEWPORT_HEIGHT_MASK
>>> 0x3FFF0000L
>>> +
>>> /* XXX Move this macro to VEGA10 header file, which is like vid.h
>>> for VI.*/
>>> #define AMDGPU_NUM_OF_VMIDS 8
>>>
>>> @@ -793,6 +801,41 @@ static int gmc_v9_0_gart_init(struct
>>> amdgpu_device *adev)
>>> return amdgpu_gart_table_vram_alloc(adev);
>>> }
>>>
>>> +static unsigned gmc_v9_0_get_vbios_fb_size(struct amdgpu_device *adev)
>>> +{
>>> + u32 d1vga_control = RREG32_SOC15(DCE, 0, mmD1VGA_CONTROL);
>>> + unsigned size;
>>> +
>>> + if (REG_GET_FIELD(d1vga_control, D1VGA_CONTROL,
>>> D1VGA_MODE_ENABLE)) {
>>> + size = 9 * 1024 * 1024; /* reserve 8MB for vga
>>> emulator and 1 MB for FB */
>>> + } else {
>>> + u32 viewport;
>>> +
>>> + switch (adev->asic_type) {
>>> + case CHIP_RAVEN:
>>> + viewport = RREG32_SOC15(DCE, 0,
>>> mmHUBP0_DCSURF_PRI_VIEWPORT_DIMENSION);
>>> + size = (REG_GET_FIELD(viewport,
>>> + HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION, PRI_VIEWPORT_HEIGHT) *
>>> + REG_GET_FIELD(viewport,
>>> + HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION, PRI_VIEWPORT_WIDTH) *
>>> + 4);
>>> + break;
>>> + case CHIP_VEGA10:
>>> + case CHIP_VEGA12:
>>> + default:
>>> + viewport = RREG32_SOC15(DCE, 0,
>>> mmSCL0_VIEWPORT_SIZE);
>>> + size = (REG_GET_FIELD(viewport,
>>> SCL0_VIEWPORT_SIZE, VIEWPORT_HEIGHT) *
>>> + REG_GET_FIELD(viewport,
>>> SCL0_VIEWPORT_SIZE, VIEWPORT_WIDTH) *
>>> + 4);
>>> + break;
>>> + }
>>> + }
>>> + /* return 0 if the pre-OS buffer uses up most of vram */
>>> + if ((adev->gmc.real_vram_size - size) < (8 * 1024 * 1024))
>>> + return 0;
>>> + return size;
>>> +}
>> I recall when I was bringing up the S3 suspend/resume and found
>> abount 8M
>> corruption at start of vram. But at that time, I didn't find the root
>> cause. The corruption will be encountered when we disabled DC ip block.
>> Alex, Andrey, have you tried the case that resume back from S3 with DC
>> disable?
>>
>> Thanks,
>> Ray
>> _______________________________________________
>> 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