On Mon, Apr 09, 2018 at 09:50:19PM +0800, 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.
> 

Yes. Andrey, Christian. That's why I reserve 8M stolen size bo at the first
of the vram. I will forward the history information to you ;-)

And nevermind, let me find a vega10 card to test whether these two patches
impact the case that I encountered before. Will let you know the result
later.

Thanks,
Ray

> 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.hu...@amd.com>
> > Sent: 09 April 2018 04:23:06
> > To: Alex Deucher; Grodzovsky, Andrey
> > Cc: amd-gfx@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.grodzov...@amd.com> (v1)
> >> Signed-off-by: Alex Deucher <alexander.deuc...@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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to