Re: [PATCH v2 4/8] drm/amdgpu: rename doorbell variables

[Shashank Sharma <shashank.sharma@xxxxxxx>]

On 14/02/2023 19:27, Christian König wrote:
> Am 14.02.23 um 17:15 schrieb Shashank Sharma:
> > From: Alex Deucher <alexander.deucher@xxxxxxx>
> >
> > This patch:
> > - renames the adev->doorbell.base to adev->doorbell.doorbell_aper_base
> > - renames the adev->doorbell.size to adev->doorbell.doorbell_aper_size
> > - moves the adev->doorbell.ptr to adev->mman.doorbell_aper_base_kaddr
> >
> > rest of the changes are just to accommodate these variable name changes.
> >
> > V2: Mered 2 patches into this one doorbell clean-up patch.
> >
> > Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> > Cc: Christian Koenig <christian.koenig@xxxxxxx>
> > Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
> > Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxx>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c   |  8 ++---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c   | 34 ++++++++++----------
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h |  5 ++-
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c      |  2 +-
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h      |  1 +
> >   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c        |  2 +-
> >   drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c       |  4 +--
> >   drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c       |  4 +--
> >   drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c       |  4 +--
> >   drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c       |  4 +--
> >   drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c       |  4 +--
> >   drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c       |  4 +--
> >   12 files changed, 38 insertions(+), 38 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > index 58689b2a2d1c..0493c64e9d0a 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > @@ -106,13 +106,13 @@ static void amdgpu_doorbell_get_kfd_info(struct 
> > amdgpu_device *adev,
> >            * not initialized as AMDGPU manages the whole
> >            * doorbell space.
> >            */
> > -        *aperture_base = adev->doorbell.base;
> > +        *aperture_base = adev->doorbell.doorbell_aper_base;
> >           *aperture_size = 0;
> >           *start_offset = 0;
> > -    } else if (adev->doorbell.size > adev->doorbell.num_doorbells *
> > +    } else if (adev->doorbell.doorbell_aper_size > 
> > adev->doorbell.num_doorbells *
> >                           sizeof(u32)) {
> > -        *aperture_base = adev->doorbell.base;
> > -        *aperture_size = adev->doorbell.size;
> > +        *aperture_base = adev->doorbell.doorbell_aper_base;
> > +        *aperture_size = adev->doorbell.doorbell_aper_size;
>
> Well that now looks a bit duplicated and you completely remove 
> doorbell_aper_base_kaddr later on, right?
>
> Christian.
Yeah, I still kept it as this cleanup is a bit peculiar and not easy to 
follow on review, but your right, lets keep it cleaner.

I will take remove doorbell_aper_base_kaddr.

- Shashank

> >           *start_offset = adev->doorbell.num_doorbells * sizeof(u32);
> >       } else {
> >           *aperture_base = 0;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 45588b7919fe..43c1b67c2778 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -597,7 +597,7 @@ u32 amdgpu_mm_rdoorbell(struct amdgpu_device 
> > *adev, u32 index)
> >           return 0;
> >         if (index < adev->doorbell.num_doorbells) {
> > -        return readl(adev->doorbell.ptr + index);
> > +        return readl(adev->mman.doorbell_aper_base_kaddr + index);
> >       } else {
> >           DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n", 
> > index);
> >           return 0;
> > @@ -620,7 +620,7 @@ void amdgpu_mm_wdoorbell(struct amdgpu_device 
> > *adev, u32 index, u32 v)
> >           return;
> >         if (index < adev->doorbell.num_doorbells) {
> > -        writel(v, adev->doorbell.ptr + index);
> > +        writel(v, adev->mman.doorbell_aper_base_kaddr + index);
> >       } else {
> >           DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", 
> > index);
> >       }
> > @@ -641,7 +641,7 @@ u64 amdgpu_mm_rdoorbell64(struct amdgpu_device 
> > *adev, u32 index)
> >           return 0;
> >         if (index < adev->doorbell.num_doorbells) {
> > -        return atomic64_read((atomic64_t *)(adev->doorbell.ptr + 
> > index));
> > +        return atomic64_read((atomic64_t 
> > *)(adev->mman.doorbell_aper_base_kaddr + index));
> >       } else {
> >           DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n", 
> > index);
> >           return 0;
> > @@ -664,7 +664,7 @@ void amdgpu_mm_wdoorbell64(struct amdgpu_device 
> > *adev, u32 index, u64 v)
> >           return;
> >         if (index < adev->doorbell.num_doorbells) {
> > -        atomic64_set((atomic64_t *)(adev->doorbell.ptr + index), v);
> > +        atomic64_set((atomic64_t 
> > *)(adev->mman.doorbell_aper_base_kaddr + index), v);
> >       } else {
> >           DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", 
> > index);
> >       }
> > @@ -1035,10 +1035,10 @@ static int amdgpu_device_doorbell_init(struct 
> > amdgpu_device *adev)
> >         /* No doorbell on SI hardware generation */
> >       if (adev->asic_type < CHIP_BONAIRE) {
> > -        adev->doorbell.base = 0;
> > -        adev->doorbell.size = 0;
> > +        adev->doorbell.doorbell_aper_base = 0;
> > +        adev->doorbell.doorbell_aper_size = 0;
> >           adev->doorbell.num_doorbells = 0;
> > -        adev->doorbell.ptr = NULL;
> > +        adev->mman.doorbell_aper_base_kaddr = NULL;
> >           return 0;
> >       }
> >   @@ -1048,15 +1048,15 @@ static int 
> > amdgpu_device_doorbell_init(struct amdgpu_device *adev)
> >       amdgpu_asic_init_doorbell_index(adev);
> >         /* doorbell bar mapping */
> > -    adev->doorbell.base = pci_resource_start(adev->pdev, 2);
> > -    adev->doorbell.size = pci_resource_len(adev->pdev, 2);
> > +    adev->doorbell.doorbell_aper_base = 
> > pci_resource_start(adev->pdev, 2);
> > +    adev->doorbell.doorbell_aper_size = pci_resource_len(adev->pdev, 
> > 2);
> >         if (adev->enable_mes) {
> >           adev->doorbell.num_doorbells =
> > -            adev->doorbell.size / sizeof(u32);
> > +            adev->doorbell.doorbell_aper_size / sizeof(u32);
> >       } else {
> >           adev->doorbell.num_doorbells =
> > -            min_t(u32, adev->doorbell.size / sizeof(u32),
> > +            min_t(u32, adev->doorbell.doorbell_aper_size / sizeof(u32),
> >                     adev->doorbell_index.max_assignment+1);
> >           if (adev->doorbell.num_doorbells == 0)
> >               return -EINVAL;
> > @@ -1071,10 +1071,10 @@ static int amdgpu_device_doorbell_init(struct 
> > amdgpu_device *adev)
> >               adev->doorbell.num_doorbells += 0x400;
> >       }
> >   -    adev->doorbell.ptr = ioremap(adev->doorbell.base,
> > -                     adev->doorbell.num_doorbells *
> > -                     sizeof(u32));
> > -    if (adev->doorbell.ptr == NULL)
> > +    adev->mman.doorbell_aper_base_kaddr = 
> > ioremap(adev->doorbell.doorbell_aper_base,
> > +                              adev->doorbell.num_doorbells *
> > +                              sizeof(u32));
> > +    if (adev->mman.doorbell_aper_base_kaddr == NULL)
> >           return -ENOMEM;
> >         return 0;
> > @@ -1089,8 +1089,8 @@ static int amdgpu_device_doorbell_init(struct 
> > amdgpu_device *adev)
> >    */
> >   static void amdgpu_device_doorbell_fini(struct amdgpu_device *adev)
> >   {
> > -    iounmap(adev->doorbell.ptr);
> > -    adev->doorbell.ptr = NULL;
> > +    iounmap(adev->mman.doorbell_aper_base_kaddr);
> > +    adev->mman.doorbell_aper_base_kaddr = NULL;
> >   }
> >     diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> > index 7199b6b0be81..526b6b4a86dd 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> > @@ -26,9 +26,8 @@
> >    */
> >   struct amdgpu_doorbell {
> >       /* doorbell mmio */
> > -    resource_size_t        base;
> > -    resource_size_t        size;
> > -    u32 __iomem        *ptr;
> > +    resource_size_t        doorbell_aper_base;
> > +    resource_size_t        doorbell_aper_size;
> >       u32            num_doorbells;    /* Number of doorbells 
> > actually reserved for amdgpu. */
> >   };
> >   diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> > index 0c546245793b..b79fb369f0f6 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> > @@ -126,7 +126,7 @@ static int amdgpu_mes_doorbell_init(struct 
> > amdgpu_device *adev)
> >           roundup(doorbell_start_offset,
> >               amdgpu_mes_doorbell_process_slice(adev));
> >   -    doorbell_aperture_size = adev->doorbell.size;
> > +    doorbell_aperture_size = adev->doorbell.doorbell_aper_size;
> >       doorbell_aperture_size =
> >               rounddown(doorbell_aperture_size,
> >                     amdgpu_mes_doorbell_process_slice(adev));
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> > index 929bc8abac28..967b265dbfa1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> > @@ -51,6 +51,7 @@ struct amdgpu_mman {
> >       struct ttm_device        bdev;
> >       bool                initialized;
> >       void __iomem            *vram_aper_base_kaddr;
> > +    u32 __iomem            *doorbell_aper_base_kaddr;
> >         /* buffer handling */
> >       const struct amdgpu_buffer_funcs    *buffer_funcs;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > index f202b45c413c..7722da8e7cb4 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > @@ -3526,7 +3526,7 @@ static int gfx_v9_0_kiq_init_register(struct 
> > amdgpu_ring *ring)
> >            */
> >           if (check_if_enlarge_doorbell_range(adev))
> >               WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_UPPER,
> > -                    (adev->doorbell.size - 4));
> > +                    (adev->doorbell.doorbell_aper_size - 4));
> >           else
> >               WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_UPPER,
> >                       (adev->doorbell_index.userqueue_end * 2) << 2);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c 
> > b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> > index aa761ff3a5fa..c5fd58d5fef9 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> > @@ -173,9 +173,9 @@ static void 
> > nbio_v2_3_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
> >                       DOORBELL_SELFRING_GPA_APER_SIZE, 0);
> >             WREG32_SOC15(NBIO, 0, 
> > mmBIF_BX_PF_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
> > -                 lower_32_bits(adev->doorbell.base));
> > + lower_32_bits(adev->doorbell.doorbell_aper_base));
> >           WREG32_SOC15(NBIO, 0, 
> > mmBIF_BX_PF_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
> > -                 upper_32_bits(adev->doorbell.base));
> > + upper_32_bits(adev->doorbell.doorbell_aper_base));
> >       }
> >         WREG32_SOC15(NBIO, 0, 
> > mmBIF_BX_PF_DOORBELL_SELFRING_GPA_APER_CNTL,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c 
> > b/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
> > index 15eb3658d70e..9d716ec71f28 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
> > @@ -169,9 +169,9 @@ static void 
> > nbio_v4_3_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
> >                       DOORBELL_SELFRING_GPA_APER_SIZE, 0);
> >             WREG32_SOC15(NBIO, 0, 
> > regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
> > -                 lower_32_bits(adev->doorbell.base));
> > + lower_32_bits(adev->doorbell.doorbell_aper_base));
> >           WREG32_SOC15(NBIO, 0, 
> > regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
> > -                 upper_32_bits(adev->doorbell.base));
> > + upper_32_bits(adev->doorbell.doorbell_aper_base));
> >       }
> >         WREG32_SOC15(NBIO, 0, 
> > regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c 
> > b/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
> > index 37615a77287b..19e175cc7340 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
> > @@ -121,9 +121,9 @@ static void 
> > nbio_v6_1_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
> >                 REG_SET_FIELD(tmp, 
> > BIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL, 
> > DOORBELL_SELFRING_GPA_APER_SIZE, 0);
> >             WREG32_SOC15(NBIO, 0, 
> > mmBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
> > -                 lower_32_bits(adev->doorbell.base));
> > + lower_32_bits(adev->doorbell.doorbell_aper_base));
> >           WREG32_SOC15(NBIO, 0, 
> > mmBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
> > -                 upper_32_bits(adev->doorbell.base));
> > + upper_32_bits(adev->doorbell.doorbell_aper_base));
> >       }
> >         WREG32_SOC15(NBIO, 0, 
> > mmBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL, tmp);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c 
> > b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
> > index 31776b12e4c4..bb2f1857b1e2 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
> > @@ -175,10 +175,10 @@ static void 
> > nbio_v7_2_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
> >             WREG32_SOC15(NBIO, 0,
> >               regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
> > -            lower_32_bits(adev->doorbell.base));
> > + lower_32_bits(adev->doorbell.doorbell_aper_base));
> >           WREG32_SOC15(NBIO, 0,
> > regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
> > -            upper_32_bits(adev->doorbell.base));
> > + upper_32_bits(adev->doorbell.doorbell_aper_base));
> >       }
> >         WREG32_SOC15(NBIO, 0, 
> > regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c 
> > b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> > index 19455a725939..ee1982bb06aa 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> > @@ -223,9 +223,9 @@ static void 
> > nbio_v7_4_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
> >                 REG_SET_FIELD(tmp, DOORBELL_SELFRING_GPA_APER_CNTL, 
> > DOORBELL_SELFRING_GPA_APER_SIZE, 0);
> >             WREG32_SOC15(NBIO, 0, mmDOORBELL_SELFRING_GPA_APER_BASE_LOW,
> > -                 lower_32_bits(adev->doorbell.base));
> > + lower_32_bits(adev->doorbell.doorbell_aper_base));
> >           WREG32_SOC15(NBIO, 0, mmDOORBELL_SELFRING_GPA_APER_BASE_HIGH,
> > -                 upper_32_bits(adev->doorbell.base));
> > + upper_32_bits(adev->doorbell.doorbell_aper_base));
> >       }
> >         WREG32_SOC15(NBIO, 0, mmDOORBELL_SELFRING_GPA_APER_CNTL, tmp);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c 
> > b/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c
> > index def89379b51a..180d50bcb40f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c
> > @@ -132,10 +132,10 @@ static void 
> > nbio_v7_7_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
> >             WREG32_SOC15(NBIO, 0,
> >               regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
> > -            lower_32_bits(adev->doorbell.base));
> > + lower_32_bits(adev->doorbell.doorbell_aper_base));
> >           WREG32_SOC15(NBIO, 0,
> > regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
> > -            upper_32_bits(adev->doorbell.base));
> > + upper_32_bits(adev->doorbell.doorbell_aper_base));
> >       }
> >         WREG32_SOC15(NBIO, 0, 
> > regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL,