Re: [PATCH v2 2/8] drm/amdgpu: replace aper_base_kaddr with vram_aper_base_kaddr

[Shashank Sharma <shashank.sharma@xxxxxxx>]

On 14/02/2023 19:24, Christian König wrote:
> Am 14.02.23 um 17:15 schrieb Shashank Sharma:
> > From: Alex Deucher <alexander.deucher@xxxxxxx>
> >
> > To differentiate it from the doorbell BAR.
>
> Since we removed the manual ioremap() for the doorbell BAR today we 
> don't really need that patch any more, don't we?
>
> On the other hand renaming the field still makes a lot of sense for 
> better documenting what it's good for.
>
> Christian.
Exactly, I was also in two minds about this patch, but then realized 
that if nothing, its making stuff more readable.

- Shashank


> > V2: Added Christian's A-B
> >
> > Acked-by: Christian Koenig <christian.koenig@xxxxxxxx>
> >
> > Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> > Cc: Christian Koenig <christian.koenig@xxxxxxx>
> > Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 +++++-----
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 14 +++++++-------
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h    |  2 +-
> >   drivers/gpu/drm/amd/amdgpu/psp_v11_0.c     | 10 +++++-----
> >   drivers/gpu/drm/amd/amdgpu/psp_v13_0.c     | 10 +++++-----
> >   5 files changed, 23 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 2f28a8c02f64..0b6a394e109b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -354,12 +354,12 @@ size_t amdgpu_device_aper_access(struct 
> > amdgpu_device *adev, loff_t pos,
> >       size_t count = 0;
> >       uint64_t last;
> >   -    if (!adev->mman.aper_base_kaddr)
> > +    if (!adev->mman.vram_aper_base_kaddr)
> >           return 0;
> >         last = min(pos + size, adev->gmc.visible_vram_size);
> >       if (last > pos) {
> > -        addr = adev->mman.aper_base_kaddr + pos;
> > +        addr = adev->mman.vram_aper_base_kaddr + pos;
> >           count = last - pos;
> >             if (write) {
> > @@ -3954,9 +3954,9 @@ static void amdgpu_device_unmap_mmio(struct 
> > amdgpu_device *adev)
> >         iounmap(adev->rmmio);
> >       adev->rmmio = NULL;
> > -    if (adev->mman.aper_base_kaddr)
> > -        iounmap(adev->mman.aper_base_kaddr);
> > -    adev->mman.aper_base_kaddr = NULL;
> > +    if (adev->mman.vram_aper_base_kaddr)
> > +        iounmap(adev->mman.vram_aper_base_kaddr);
> > +    adev->mman.vram_aper_base_kaddr = NULL;
> >         /* Memory manager related */
> >       if (!adev->gmc.xgmi.connected_to_cpu) {
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > index 55e0284b2bdd..73b831b47892 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > @@ -578,9 +578,9 @@ static int amdgpu_ttm_io_mem_reserve(struct 
> > ttm_device *bdev,
> >           if ((mem->bus.offset + bus_size) > 
> > adev->gmc.visible_vram_size)
> >               return -EINVAL;
> >   -        if (adev->mman.aper_base_kaddr &&
> > +        if (adev->mman.vram_aper_base_kaddr &&
> >               mem->placement & TTM_PL_FLAG_CONTIGUOUS)
> > -            mem->bus.addr = (u8 *)adev->mman.aper_base_kaddr +
> > +            mem->bus.addr = (u8 *)adev->mman.vram_aper_base_kaddr +
> >                       mem->bus.offset;
> >             mem->bus.offset += adev->gmc.aper_base;
> > @@ -1752,12 +1752,12 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
> >   #ifdef CONFIG_64BIT
> >   #ifdef CONFIG_X86
> >       if (adev->gmc.xgmi.connected_to_cpu)
> > -        adev->mman.aper_base_kaddr = ioremap_cache(adev->gmc.aper_base,
> > +        adev->mman.vram_aper_base_kaddr = 
> > ioremap_cache(adev->gmc.aper_base,
> >                   adev->gmc.visible_vram_size);
> >         else
> >   #endif
> > -        adev->mman.aper_base_kaddr = ioremap_wc(adev->gmc.aper_base,
> > +        adev->mman.vram_aper_base_kaddr = 
> > ioremap_wc(adev->gmc.aper_base,
> >                   adev->gmc.visible_vram_size);
> >   #endif
> >   @@ -1904,9 +1904,9 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
> >         if (drm_dev_enter(adev_to_drm(adev), &idx)) {
> >   -        if (adev->mman.aper_base_kaddr)
> > -            iounmap(adev->mman.aper_base_kaddr);
> > -        adev->mman.aper_base_kaddr = NULL;
> > +        if (adev->mman.vram_aper_base_kaddr)
> > +            iounmap(adev->mman.vram_aper_base_kaddr);
> > +        adev->mman.vram_aper_base_kaddr = NULL;
> >             drm_dev_exit(idx);
> >       }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> > index e2cd5894afc9..929bc8abac28 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> > @@ -50,7 +50,7 @@ struct amdgpu_gtt_mgr {
> >   struct amdgpu_mman {
> >       struct ttm_device        bdev;
> >       bool                initialized;
> > -    void __iomem            *aper_base_kaddr;
> > +    void __iomem            *vram_aper_base_kaddr;
> >         /* buffer handling */
> >       const struct amdgpu_buffer_funcs    *buffer_funcs;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c 
> > b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > index bd3e3e23a939..f39d4f593a2f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > @@ -611,10 +611,10 @@ static int psp_v11_0_memory_training(struct 
> > psp_context *psp, uint32_t ops)
> >            */
> >           sz = GDDR6_MEM_TRAINING_ENCROACHED_SIZE;
> >   -        if (adev->gmc.visible_vram_size < sz || 
> > !adev->mman.aper_base_kaddr) {
> > -            DRM_ERROR("visible_vram_size %llx or aper_base_kaddr %p 
> > is not initialized.\n",
> > +        if (adev->gmc.visible_vram_size < sz || 
> > !adev->mman.vram_aper_base_kaddr) {
> > +            DRM_ERROR("visible_vram_size %llx or 
> > vram_aper_base_kaddr %p is not initialized.\n",
> >                     adev->gmc.visible_vram_size,
> > -                  adev->mman.aper_base_kaddr);
> > +                  adev->mman.vram_aper_base_kaddr);
> >               return -EINVAL;
> >           }
> >   @@ -625,7 +625,7 @@ static int psp_v11_0_memory_training(struct 
> > psp_context *psp, uint32_t ops)
> >           }
> >             if (drm_dev_enter(adev_to_drm(adev), &idx)) {
> > -            memcpy_fromio(buf, adev->mman.aper_base_kaddr, sz);
> > +            memcpy_fromio(buf, adev->mman.vram_aper_base_kaddr, sz);
> >               ret = psp_v11_0_memory_training_send_msg(psp, 
> > PSP_BL__DRAM_LONG_TRAIN);
> >               if (ret) {
> >                   DRM_ERROR("Send long training msg failed.\n");
> > @@ -634,7 +634,7 @@ static int psp_v11_0_memory_training(struct 
> > psp_context *psp, uint32_t ops)
> >                   return ret;
> >               }
> >   -            memcpy_toio(adev->mman.aper_base_kaddr, buf, sz);
> > +            memcpy_toio(adev->mman.vram_aper_base_kaddr, buf, sz);
> >               adev->hdp.funcs->flush_hdp(adev, NULL);
> >               vfree(buf);
> >               drm_dev_exit(idx);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c 
> > b/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
> > index e6a26a7e5e5e..9605c0971c11 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
> > @@ -510,10 +510,10 @@ static int psp_v13_0_memory_training(struct 
> > psp_context *psp, uint32_t ops)
> >            */
> >           sz = GDDR6_MEM_TRAINING_ENCROACHED_SIZE;
> >   -        if (adev->gmc.visible_vram_size < sz || 
> > !adev->mman.aper_base_kaddr) {
> > -            dev_err(adev->dev, "visible_vram_size %llx or 
> > aper_base_kaddr %p is not initialized.\n",
> > +        if (adev->gmc.visible_vram_size < sz || 
> > !adev->mman.vram_aper_base_kaddr) {
> > +            dev_err(adev->dev, "visible_vram_size %llx or 
> > vram_aper_base_kaddr %p is not initialized.\n",
> >                     adev->gmc.visible_vram_size,
> > -                  adev->mman.aper_base_kaddr);
> > +                  adev->mman.vram_aper_base_kaddr);
> >               return -EINVAL;
> >           }
> >   @@ -524,7 +524,7 @@ static int psp_v13_0_memory_training(struct 
> > psp_context *psp, uint32_t ops)
> >           }
> >             if (drm_dev_enter(adev_to_drm(adev), &idx)) {
> > -            memcpy_fromio(buf, adev->mman.aper_base_kaddr, sz);
> > +            memcpy_fromio(buf, adev->mman.vram_aper_base_kaddr, sz);
> >               ret = psp_v13_0_memory_training_send_msg(psp, 
> > PSP_BL__DRAM_LONG_TRAIN);
> >               if (ret) {
> >                   DRM_ERROR("Send long training msg failed.\n");
> > @@ -533,7 +533,7 @@ static int psp_v13_0_memory_training(struct 
> > psp_context *psp, uint32_t ops)
> >                   return ret;
> >               }
> >   -            memcpy_toio(adev->mman.aper_base_kaddr, buf, sz);
> > +            memcpy_toio(adev->mman.vram_aper_base_kaddr, buf, sz);
> >               adev->hdp.funcs->flush_hdp(adev, NULL);
> >               vfree(buf);
> >               drm_dev_exit(idx);