[PATCH 1/2] drm/amdgpu: Use fw vram offset when allocating stolen vga memory.

[Alex Deucher]

On Mon, Mar 14, 2022 at 3:35 PM Christian König
<ckoenig.leichtzumerken at gmail.com> wrote:
>
> Am 14.03.22 um 19:54 schrieb Yongqiang Sun:
> > [Why]
> > Memory access violation will happen in case of allocate stolen vga
> > memory with size isn't 0.
> >
> > [How]
> > when allocating stolen vga memory, use fw vram offset as the start point
> > instead of hard code value 0.
>
> Please stop separating commit message into [Why] and [How], that is not
> well received everywhere.
>
> Apart from that the patch is a certain NAK, you are messing things quite
> up here.
>
> >
> > Signed-off-by: Yongqiang Sun <yongqiang.sun at amd.com>
> > Change-Id: I7c555a6c1fb4b3fa8685753b4bdcbe215f89ea1e
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 31 +++++++++++++++++++------
> >   1 file changed, 24 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > index 41d6f604813d..1f635fdb0395 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > @@ -1708,6 +1708,7 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
> >       uint64_t gtt_size;
> >       int r;
> >       u64 vis_vram_limit;
> > +     u64 memory_offset = adev->mman.fw_vram_usage_start_offset + adev->mman.fw_vram_usage_size;
> >
> >       mutex_init(&adev->mman.gtt_window_lock);
> >
> > @@ -1774,24 +1775,40 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
> >        * This is used for VGA emulation and pre-OS scanout buffers to
> >        * avoid display artifacts while transitioning between pre-OS
> >        * and driver.  */
> > -     r = amdgpu_bo_create_kernel_at(adev, 0, adev->mman.stolen_vga_size,
> > +     r = amdgpu_bo_create_kernel_at(adev,
> > +                                        memory_offset,
> > +                                adev->mman.stolen_vga_size,
>
> That is certainly incorrect. See function amdgpu_ttm_fw_reserve_vram_init().
>
> The stolen VGA buffer always started at offset 0 and is independent of
> the range defined by fw_vram_usage_start_offset and fw_vram_usage_size.
>

Yeah these are separate ranges.  I just had a chat with Yongqiang and
there was some confusion around this, but this patch can be dropped.

Alex


> Regards,
> Christian.
>
> >                                      AMDGPU_GEM_DOMAIN_VRAM,
> >                                      &adev->mman.stolen_vga_memory,
> >                                      NULL);
> >       if (r)
> >               return r;
> > -     r = amdgpu_bo_create_kernel_at(adev, adev->mman.stolen_vga_size,
> > +
> > +     memory_offset += adev->mman.stolen_vga_size;
> > +
> > +     r = amdgpu_bo_create_kernel_at(adev,
> > +                                        memory_offset,
> >                                      adev->mman.stolen_extended_size,
> >                                      AMDGPU_GEM_DOMAIN_VRAM,
> >                                      &adev->mman.stolen_extended_memory,
> >                                      NULL);
> >       if (r)
> >               return r;
> > -     r = amdgpu_bo_create_kernel_at(adev, adev->mman.stolen_reserved_offset,
> > -                                    adev->mman.stolen_reserved_size,
> > -                                    AMDGPU_GEM_DOMAIN_VRAM,
> > -                                    &adev->mman.stolen_reserved_memory,
> > -                                    NULL);
> > +
> > +     memory_offset += adev->mman.stolen_extended_size;
> > +
> > +     if (adev->mman.stolen_reserved_offset > memory_offset)
> > +             r = amdgpu_bo_create_kernel_at(adev, adev->mman.stolen_reserved_offset,
> > +                                                adev->mman.stolen_reserved_size,
> > +                                                AMDGPU_GEM_DOMAIN_VRAM,
> > +                                                &adev->mman.stolen_reserved_memory,
> > +                                                NULL);
> > +     else if (adev->mman.stolen_reserved_offset + adev->mman.stolen_reserved_size > memory_offset)
> > +             r = amdgpu_bo_create_kernel_at(adev, memory_offset,
> > +                                             adev->mman.stolen_reserved_offset + adev->mman.stolen_reserved_size - memory_offset,
> > +                                                AMDGPU_GEM_DOMAIN_VRAM,
> > +                                                &adev->mman.stolen_reserved_memory,
> > +                                                NULL);
> >       if (r)
> >               return r;
> >
>