Re: [PATCH v2 1/2] drm/amdgpu: Add enum for PCIe BAR regions

Mon, 01 Jun 2026 22:44:11 -0700 



On 02-Jun-26 2:25 AM, Harish Kasiviswanathan wrote:

Use enum instead of hard coded values. There is no functional change.

v2:
- Add amdgpu_get_bar_idx() to map BAR roles to PCI BAR indices

Signed-off-by: Harish Kasiviswanathan <[email protected]>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h           | 17 ++++++
  drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c      |  6 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    | 56 ++++++++++++++-----
  .../gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c  |  9 ++-
  drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c        |  6 +-
  drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c        |  6 +-
  drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c        |  6 +-
  drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c         |  6 +-
  drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c         |  6 +-
  drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c         |  6 +-
  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c         |  9 ++-
  11 files changed, 100 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 5d7bfa59424a..1944d1bece86 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1405,6 +1405,23 @@ bool amdgpu_device_supports_boco(struct amdgpu_device 
*adev);
  bool amdgpu_device_supports_smart_shift(struct amdgpu_device *adev);
  int amdgpu_device_supports_baco(struct amdgpu_device *adev);
  void amdgpu_device_detect_runtime_pm_mode(struct amdgpu_device *adev);
+
+/**
+ * enum amdgpu_pcie_bar - PCIe BAR role identifiers
+ * @AMDGPU_PCIE_BAR_VRAM: VRAM aperture
+ * @AMDGPU_PCIE_BAR_DOORBELL: Doorbell aperture (Bonaire+)
+ * @AMDGPU_PCIE_BAR_MMIO: MMIO register aperture
+ *
+ * Use amdgpu_get_bar_idx() to map a role to the PCI BAR index on a given ASIC.
+ */
+enum amdgpu_pcie_bar {
+       AMDGPU_PCIE_BAR_VRAM = 0,
+       AMDGPU_PCIE_BAR_DOORBELL = 2,
+       AMDGPU_PCIE_BAR_MMIO = 5,
+};



This may just be treated as an enum, it's not required to designate 
numbers for BAR index.



+
+int amdgpu_get_bar_idx(struct amdgpu_device *adev, enum amdgpu_pcie_bar bar);
+
  bool amdgpu_device_is_peer_accessible(struct amdgpu_device *adev,
                                      struct amdgpu_device *peer_adev);
  int amdgpu_device_baco_enter(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
index aa039e148a5e..7e253bb35434 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
@@ -111,11 +111,13 @@ static bool amdgpu_read_bios_from_vram(struct 
amdgpu_device *adev)
                        return false;

  
  	/* FB BAR not enabled */

-       if (pci_resource_len(adev->pdev, 0) == 0)
+       if (pci_resource_len(adev->pdev,
+                              amdgpu_get_bar_idx(adev, AMDGPU_PCIE_BAR_VRAM)) 
== 0)
                return false;

  
  	adev->bios = NULL;

-       vram_base = pci_resource_start(adev->pdev, 0);
+       vram_base = pci_resource_start(adev->pdev,
+                                      amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_VRAM));

  
  	adev->bios = kmalloc(size, GFP_KERNEL);

        if (!adev->bios)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 5ff224163bab..2e1e5791f123 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1103,6 +1103,34 @@ void amdgpu_device_wb_free(struct amdgpu_device *adev, 
u32 wb)
        spin_unlock_irqrestore(&adev->wb.lock, flags);
  }

  
+/**

+ * amdgpu_get_bar_idx - map a BAR role to the PCI BAR index
+ * @adev: amdgpu_device pointer
+ * @bar: BAR role to look up
+ *
+ * Return the PCI BAR index for @bar on @adev.
+ *
+ * VRAM is always BAR 0. Doorbells were introduced at Bonaire (CIK): 
pre-Bonaire
+ * ASICs have no dedicated doorbell BAR, and BAR 2 is used for MMIO registers.
+ * Bonaire and newer expose a doorbell aperture at BAR 2 and MMIO at BAR 5.
+ */
+int amdgpu_get_bar_idx(struct amdgpu_device *adev, enum amdgpu_pcie_bar bar)
+{
+       switch (bar) {
+       case AMDGPU_PCIE_BAR_VRAM:
+               return AMDGPU_PCIE_BAR_VRAM;
+       case AMDGPU_PCIE_BAR_DOORBELL:
+               return AMDGPU_PCIE_BAR_DOORBELL;
+       case AMDGPU_PCIE_BAR_MMIO:
+               if (adev->asic_type >= CHIP_BONAIRE)
+                       return AMDGPU_PCIE_BAR_MMIO;
+               return AMDGPU_PCIE_BAR_DOORBELL;



Was not expecting this function to return the enum itself. Input is enum 
and output is a hardcoded index like 0, 2, 5.

+       default:
+               WARN_ON(1);


pci_resource_len() expects a valid bar number.

pci_resource_len(adev->pdev, amdgpu_get_bar_idx()) usage may invoke some 
static code analyzer warnings.



Thinking again, a one-time assignment of below ones somewhere in early 
init amdgpu_init_pci_bars() may be simpler to deal with.


gmc.aper_bar_idx
rmmio_bar_idx
doorbell.bar_idx

Thanks,
Lijo

+               return -EINVAL;
+       }
+}
+
  /**
   * amdgpu_device_resize_fb_bar - try to resize FB BAR
   *
@@ -1146,7 +1174,8 @@ int amdgpu_device_resize_fb_bar(struct amdgpu_device 
*adev)

  
  	/* skip if the bios has already enabled large BAR */

        if (adev->gmc.real_vram_size &&
-           (pci_resource_len(adev->pdev, 0) >= adev->gmc.real_vram_size))
+           (pci_resource_len(adev->pdev,
+                             amdgpu_get_bar_idx(adev, AMDGPU_PCIE_BAR_VRAM)) >= 
adev->gmc.real_vram_size))
                return 0;

  
  	/* Check if the root BUS has 64bit memory resources */

@@ -1165,7 +1194,8 @@ int amdgpu_device_resize_fb_bar(struct amdgpu_device 
*adev)
                return 0;

  
  	/* Limit the BAR size to what is available */

-       max_size = pci_rebar_get_max_size(adev->pdev, 0);
+       max_size = pci_rebar_get_max_size(adev->pdev,
+                                         amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_VRAM));
        if (max_size < 0)
                return 0;
        rbar_size = min(max_size, rbar_size);
@@ -1178,9 +1208,11 @@ int amdgpu_device_resize_fb_bar(struct amdgpu_device 
*adev)
        /* Tear down doorbell as resizing will release BARs */
        amdgpu_doorbell_fini(adev);

  
-	r = pci_resize_resource(adev->pdev, 0, rbar_size,

-                               (adev->asic_type >= CHIP_BONAIRE) ? 1 << 5
-                                                                 : 1 << 2);
+       /* Resize the VRAM BAR. Exclude the MMIO BAR from being released. */
+       r = pci_resize_resource(adev->pdev,
+                               amdgpu_get_bar_idx(adev, AMDGPU_PCIE_BAR_VRAM),
+                               rbar_size,
+                               BIT(amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_MMIO)));
        if (r == -ENOSPC)
                dev_info(adev->dev,
                         "Not enough PCI address space for a large BAR.");
@@ -1191,7 +1223,8 @@ int amdgpu_device_resize_fb_bar(struct amdgpu_device 
*adev)
         * using the device.
         */
        r = amdgpu_doorbell_init(adev);
-       if (r || (pci_resource_flags(adev->pdev, 0) & IORESOURCE_UNSET))
+       if (r || (pci_resource_flags(adev->pdev,
+                                      amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_VRAM)) & IORESOURCE_UNSET))
                return -ENODEV;

  
  	pci_write_config_word(adev->pdev, PCI_COMMAND, cmd);

@@ -3825,13 +3858,10 @@ int amdgpu_device_init(struct amdgpu_device *adev,

  
  	/* Registers mapping */

        /* TODO: block userspace mapping of io register */
-       if (adev->asic_type >= CHIP_BONAIRE) {
-               adev->rmmio_base = pci_resource_start(adev->pdev, 5);
-               adev->rmmio_size = pci_resource_len(adev->pdev, 5);
-       } else {
-               adev->rmmio_base = pci_resource_start(adev->pdev, 2);
-               adev->rmmio_size = pci_resource_len(adev->pdev, 2);
-       }
+       adev->rmmio_base = pci_resource_start(adev->pdev,
+                                             amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_MMIO));
+       adev->rmmio_size = pci_resource_len(adev->pdev,
+                                           amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_MMIO));

  
  	for (i = 0; i < AMD_IP_BLOCK_TYPE_NUM; i++)

                atomic_set(&adev->pm.pwr_state[i], POWER_STATE_UNKNOWN);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c
index bc7858567321..826b80481908 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c
@@ -201,14 +201,17 @@ int amdgpu_doorbell_init(struct amdgpu_device *adev)
                return 0;
        }

  
-	if (pci_resource_flags(adev->pdev, 2) & IORESOURCE_UNSET)

+       if (pci_resource_flags(adev->pdev,
+                              amdgpu_get_bar_idx(adev, AMDGPU_PCIE_BAR_DOORBELL)) 
& IORESOURCE_UNSET)
                return -EINVAL;

  
  	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.base = pci_resource_start(adev->pdev,
+                                                amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_DOORBELL));
+       adev->doorbell.size = pci_resource_len(adev->pdev,
+                                              amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_DOORBELL));

  
  	adev->doorbell.num_kernel_doorbells =

                min_t(u32, adev->doorbell.size / sizeof(u32),
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 8523833a74fb..728d93e96ea1 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -694,8 +694,10 @@ static int gmc_v10_0_mc_init(struct amdgpu_device *adev)
                if (r)
                        return r;
        }
-       adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);
-       adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
+       adev->gmc.aper_base = pci_resource_start(adev->pdev,
+                                              amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_VRAM));
+       adev->gmc.aper_size = pci_resource_len(adev->pdev,
+                                              amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_VRAM));

  
  #ifdef CONFIG_X86_64

        if ((adev->flags & AMD_IS_APU) && !amdgpu_passthrough(adev)) {
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
index 16388e3caea3..a16e681b4de0 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
@@ -694,8 +694,10 @@ static int gmc_v11_0_mc_init(struct amdgpu_device *adev)
                if (r)
                        return r;
        }
-       adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);
-       adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
+       adev->gmc.aper_base = pci_resource_start(adev->pdev,
+                                              amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_VRAM));
+       adev->gmc.aper_size = pci_resource_len(adev->pdev,
+                                              amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_VRAM));

  
  #ifdef CONFIG_X86_64

        if ((adev->flags & AMD_IS_APU) && !amdgpu_passthrough(adev)) {
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
index 586703ec0dfa..5c2b4d2f3f16 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
@@ -746,8 +746,10 @@ static int gmc_v12_0_mc_init(struct amdgpu_device *adev)
                        return r;
        }

  
-	adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);

-       adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
+       adev->gmc.aper_base = pci_resource_start(adev->pdev,
+                                              amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_VRAM));
+       adev->gmc.aper_size = pci_resource_len(adev->pdev,
+                                              amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_VRAM));

  
  #ifdef CONFIG_X86_64

        if (((adev->flags & AMD_IS_APU) && !amdgpu_passthrough(adev)) ||
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index af6944d2d330..5bdcc276244c 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -323,8 +323,10 @@ static int gmc_v6_0_mc_init(struct amdgpu_device *adev)
                if (r)
                        return r;
        }
-       adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);
-       adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
+       adev->gmc.aper_base = pci_resource_start(adev->pdev,
+                                              amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_VRAM));
+       adev->gmc.aper_size = pci_resource_len(adev->pdev,
+                                              amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_VRAM));
        adev->gmc.visible_vram_size = adev->gmc.aper_size;

  
  	/* set the gart size */

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index 93cf283191fa..d8b81d8ec954 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -379,8 +379,10 @@ static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
                if (r)
                        return r;
        }
-       adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);
-       adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
+       adev->gmc.aper_base = pci_resource_start(adev->pdev,
+                                              amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_VRAM));
+       adev->gmc.aper_size = pci_resource_len(adev->pdev,
+                                              amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_VRAM));

  
  #ifdef CONFIG_X86_64

        if ((adev->flags & AMD_IS_APU) &&
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index 1d3ddffd5a11..99bfd4e42977 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -572,8 +572,10 @@ static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
                if (r)
                        return r;
        }
-       adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);
-       adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
+       adev->gmc.aper_base = pci_resource_start(adev->pdev,
+                                              amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_VRAM));
+       adev->gmc.aper_size = pci_resource_len(adev->pdev,
+                                              amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_VRAM));

  
  #ifdef CONFIG_X86_64

        if ((adev->flags & AMD_IS_APU) && !amdgpu_passthrough(adev)) {
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index ced0f3941863..774e93590389 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1588,7 +1588,8 @@ static int gmc_v9_0_early_init(struct amdgpu_ip_block 
*ip_block)
                 * mode.
                 */
                adev->gmc.is_app_apu = (pkg_type == AMDGPU_PKG_TYPE_APU &&
-                                       !pci_resource_len(adev->pdev, 0));
+                                       !pci_resource_len(adev->pdev,
+                                                         
amdgpu_get_bar_idx(adev, AMDGPU_PCIE_BAR_VRAM)));
        }

  
  	gmc_v9_0_set_gmc_funcs(adev);

@@ -1700,8 +1701,10 @@ static int gmc_v9_0_mc_init(struct amdgpu_device *adev)
                if (r)
                        return r;
        }
-       adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);
-       adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
+       adev->gmc.aper_base = pci_resource_start(adev->pdev,
+                                              amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_VRAM));
+       adev->gmc.aper_size = pci_resource_len(adev->pdev,
+                                              amdgpu_get_bar_idx(adev, 
AMDGPU_PCIE_BAR_VRAM));

  
  #ifdef CONFIG_X86_64

        /*
























					Reply via email to