* [PATCH v3 1/2] drm/amdgpu: move program_aspm to amdgpu_nbio
@ 2026-04-24 0:49 Gabriel Almeida
2026-04-24 0:49 ` [PATCH v3 2/2] drm/amdgpu: move read_indexed_register to amdgpu_reg_access Gabriel Almeida
0 siblings, 1 reply; 4+ messages in thread
From: Gabriel Almeida @ 2026-04-24 0:49 UTC (permalink / raw)
To: alexander.deucher, christian.koenig, airlied, simona
Cc: linux-kernel, amd-gfx, dri-devel, gabrielsousa230
The program_aspm helper is duplicated across multiple files with
identical logic.
Move it to amdgpu_nbio.c as amdgpu_nbio_program_aspm and update
all users accordingly.
Signed-off-by: Gabriel Almeida <gabrielsousa230@gmail.com>
---
v3:
- split from previous combined patch
v2:
- move program_aspm to amdgpu_nbio.c
- drop amdgpu_common
drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.c | 10 ++++++++++
drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.h | 2 ++
drivers/gpu/drm/amd/amdgpu/nv.c | 12 +-----------
drivers/gpu/drm/amd/amdgpu/soc15.c | 11 +----------
drivers/gpu/drm/amd/amdgpu/soc21.c | 11 +----------
5 files changed, 15 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.c
index a97426583..e4c8e9872 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.c
@@ -84,3 +84,13 @@ int amdgpu_nbio_ras_late_init(struct amdgpu_device *adev, struct ras_common_if *
amdgpu_ras_block_late_fini(adev, ras_block);
return r;
}
+
+
+void amdgpu_nbio_program_aspm(struct amdgpu_device *adev)
+{
+ if (!amdgpu_device_should_use_aspm(adev))
+ return;
+
+ if (adev->nbio.funcs->program_aspm)
+ adev->nbio.funcs->program_aspm(adev);
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.h
index b528de6a0..a61f3a6e8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.h
@@ -121,4 +121,6 @@ u64 amdgpu_nbio_get_pcie_replay_count(struct amdgpu_device *adev);
bool amdgpu_nbio_is_replay_cnt_supported(struct amdgpu_device *adev);
+void amdgpu_nbio_program_aspm(struct amdgpu_device *adev);
+
#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
index 7ce1a1b95..030d80664 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -511,16 +511,6 @@ static int nv_set_vce_clocks(struct amdgpu_device *adev, u32 evclk, u32 ecclk)
return 0;
}
-static void nv_program_aspm(struct amdgpu_device *adev)
-{
- if (!amdgpu_device_should_use_aspm(adev))
- return;
-
- if (adev->nbio.funcs->program_aspm)
- adev->nbio.funcs->program_aspm(adev);
-
-}
-
const struct amdgpu_ip_block_version nv_common_ip_block = {
.type = AMD_IP_BLOCK_TYPE_COMMON,
.major = 1,
@@ -984,7 +974,7 @@ static int nv_common_hw_init(struct amdgpu_ip_block *ip_block)
adev->nbio.funcs->apply_l1_link_width_reconfig_wa(adev);
/* enable aspm */
- nv_program_aspm(adev);
+ amdgpu_nbio_program_aspm(adev);
/* setup nbio registers */
adev->nbio.funcs->init_registers(adev);
/* remap HDP registers to a hole in mmio space,
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
index b456e4541..27bcbbae5 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -695,15 +695,6 @@ static int soc15_set_vce_clocks(struct amdgpu_device *adev, u32 evclk, u32 ecclk
return 0;
}
-static void soc15_program_aspm(struct amdgpu_device *adev)
-{
- if (!amdgpu_device_should_use_aspm(adev))
- return;
-
- if (adev->nbio.funcs->program_aspm)
- adev->nbio.funcs->program_aspm(adev);
-}
-
const struct amdgpu_ip_block_version vega10_common_ip_block =
{
.type = AMD_IP_BLOCK_TYPE_COMMON,
@@ -1284,7 +1275,7 @@ static int soc15_common_hw_init(struct amdgpu_ip_block *ip_block)
struct amdgpu_device *adev = ip_block->adev;
/* enable aspm */
- soc15_program_aspm(adev);
+ amdgpu_nbio_program_aspm(adev);
/* setup nbio registers */
adev->nbio.funcs->init_registers(adev);
/* remap HDP registers to a hole in mmio space,
diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c b/drivers/gpu/drm/amd/amdgpu/soc21.c
index fbd1d97f3..7e4353d0c 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc21.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
@@ -470,15 +470,6 @@ static int soc21_set_vce_clocks(struct amdgpu_device *adev, u32 evclk, u32 ecclk
return 0;
}
-static void soc21_program_aspm(struct amdgpu_device *adev)
-{
- if (!amdgpu_device_should_use_aspm(adev))
- return;
-
- if (adev->nbio.funcs->program_aspm)
- adev->nbio.funcs->program_aspm(adev);
-}
-
const struct amdgpu_ip_block_version soc21_common_ip_block = {
.type = AMD_IP_BLOCK_TYPE_COMMON,
.major = 1,
@@ -925,7 +916,7 @@ static int soc21_common_hw_init(struct amdgpu_ip_block *ip_block)
struct amdgpu_device *adev = ip_block->adev;
/* enable aspm */
- soc21_program_aspm(adev);
+ amdgpu_nbio_program_aspm(adev);
/* setup nbio registers */
adev->nbio.funcs->init_registers(adev);
/* remap HDP registers to a hole in mmio space,
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v3 2/2] drm/amdgpu: move read_indexed_register to amdgpu_reg_access
2026-04-24 0:49 [PATCH v3 1/2] drm/amdgpu: move program_aspm to amdgpu_nbio Gabriel Almeida
@ 2026-04-24 0:49 ` Gabriel Almeida
2026-04-24 7:24 ` Christian König
0 siblings, 1 reply; 4+ messages in thread
From: Gabriel Almeida @ 2026-04-24 0:49 UTC (permalink / raw)
To: alexander.deucher, christian.koenig, airlied, simona
Cc: linux-kernel, amd-gfx, dri-devel, gabrielsousa230
The read_indexed_register helper is duplicated across multiple files
with identical logic.
Move it to amdgpu_reg_access.c as
amdgpu_read_indexed_register and update all users accordingly.
No functional changes intended.
Signed-off-by: Gabriel Almeida <gabrielsousa230@gmail.com>
---
v3:
- split into two patches as requested
v2:
- move read_indexed_register to amdgpu_reg_access.c
- drop amdgpu_common
.../gpu/drm/amd/amdgpu/amdgpu_reg_access.c | 18 ++++++++++++++++
.../gpu/drm/amd/amdgpu/amdgpu_reg_access.h | 3 +++
drivers/gpu/drm/amd/amdgpu/nv.c | 19 +----------------
drivers/gpu/drm/amd/amdgpu/soc15.c | 19 +----------------
drivers/gpu/drm/amd/amdgpu/soc21.c | 19 +----------------
drivers/gpu/drm/amd/amdgpu/soc24.c | 21 +------------------
drivers/gpu/drm/amd/amdgpu/soc_v1_0.c | 20 +-----------------
7 files changed, 26 insertions(+), 93 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.c
index 540040c76..daefbeeee 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.c
@@ -956,3 +956,21 @@ uint32_t amdgpu_device_wait_on_rreg(struct amdgpu_device *adev, uint32_t inst,
}
return ret;
}
+
+
+uint32_t amdgpu_read_indexed_register(struct amdgpu_device *adev,
+ u32 se_num, u32 sh_num, u32 reg_offset)
+{
+ uint32_t val;
+
+ mutex_lock(&adev->grbm_idx_mutex);
+ if (se_num != 0xffffffff || sh_num != 0xffffffff)
+ amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff, 0);
+
+ val = RREG32(reg_offset);
+
+ if (se_num != 0xffffffff || sh_num != 0xffffffff)
+ amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff, 0);
+ mutex_unlock(&adev->grbm_idx_mutex);
+ return val;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.h
index 4d88e5cd1..a1011af6b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.h
@@ -160,4 +160,7 @@ uint32_t amdgpu_device_wait_on_rreg(struct amdgpu_device *adev, uint32_t inst,
uint32_t reg_addr, char reg_name[],
uint32_t expected_value, uint32_t mask);
+uint32_t amdgpu_read_indexed_register(struct amdgpu_device *adev,
+ u32 se_num, u32 sh_num, u32 reg_offset);
+
#endif /* __AMDGPU_REG_ACCESS_H__ */
diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
index 030d80664..72edf5326 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -354,29 +354,12 @@ static struct soc15_allowed_register_entry nv_allowed_read_registers[] = {
{ SOC15_REG_ENTRY(GC, 0, mmGB_ADDR_CONFIG)},
};
-static uint32_t nv_read_indexed_register(struct amdgpu_device *adev, u32 se_num,
- u32 sh_num, u32 reg_offset)
-{
- uint32_t val;
-
- mutex_lock(&adev->grbm_idx_mutex);
- if (se_num != 0xffffffff || sh_num != 0xffffffff)
- amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff, 0);
-
- val = RREG32(reg_offset);
-
- if (se_num != 0xffffffff || sh_num != 0xffffffff)
- amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff, 0);
- mutex_unlock(&adev->grbm_idx_mutex);
- return val;
-}
-
static uint32_t nv_get_register_value(struct amdgpu_device *adev,
bool indexed, u32 se_num,
u32 sh_num, u32 reg_offset)
{
if (indexed) {
- return nv_read_indexed_register(adev, se_num, sh_num, reg_offset);
+ return amdgpu_read_indexed_register(adev, se_num, sh_num, reg_offset);
} else {
if (reg_offset == SOC15_REG_OFFSET(GC, 0, mmGB_ADDR_CONFIG))
return adev->gfx.config.gb_addr_config;
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
index 27bcbbae5..87b398dd0 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -401,29 +401,12 @@ static struct soc15_allowed_register_entry soc15_allowed_read_registers[] = {
{ SOC15_REG_ENTRY(GC, 0, mmDB_DEBUG2)},
};
-static uint32_t soc15_read_indexed_register(struct amdgpu_device *adev, u32 se_num,
- u32 sh_num, u32 reg_offset)
-{
- uint32_t val;
-
- mutex_lock(&adev->grbm_idx_mutex);
- if (se_num != 0xffffffff || sh_num != 0xffffffff)
- amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff, 0);
-
- val = RREG32(reg_offset);
-
- if (se_num != 0xffffffff || sh_num != 0xffffffff)
- amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff, 0);
- mutex_unlock(&adev->grbm_idx_mutex);
- return val;
-}
-
static uint32_t soc15_get_register_value(struct amdgpu_device *adev,
bool indexed, u32 se_num,
u32 sh_num, u32 reg_offset)
{
if (indexed) {
- return soc15_read_indexed_register(adev, se_num, sh_num, reg_offset);
+ return amdgpu_read_indexed_register(adev, se_num, sh_num, reg_offset);
} else {
if (reg_offset == SOC15_REG_OFFSET(GC, 0, mmGB_ADDR_CONFIG))
return adev->gfx.config.gb_addr_config;
diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c b/drivers/gpu/drm/amd/amdgpu/soc21.c
index 7e4353d0c..93c002e51 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc21.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
@@ -306,29 +306,12 @@ static struct soc15_allowed_register_entry soc21_allowed_read_registers[] = {
{ SOC15_REG_ENTRY(GC, 0, regGB_ADDR_CONFIG)},
};
-static uint32_t soc21_read_indexed_register(struct amdgpu_device *adev, u32 se_num,
- u32 sh_num, u32 reg_offset)
-{
- uint32_t val;
-
- mutex_lock(&adev->grbm_idx_mutex);
- if (se_num != 0xffffffff || sh_num != 0xffffffff)
- amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff, 0);
-
- val = RREG32(reg_offset);
-
- if (se_num != 0xffffffff || sh_num != 0xffffffff)
- amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff, 0);
- mutex_unlock(&adev->grbm_idx_mutex);
- return val;
-}
-
static uint32_t soc21_get_register_value(struct amdgpu_device *adev,
bool indexed, u32 se_num,
u32 sh_num, u32 reg_offset)
{
if (indexed) {
- return soc21_read_indexed_register(adev, se_num, sh_num, reg_offset);
+ return amdgpu_read_indexed_register(adev, se_num, sh_num, reg_offset);
} else {
if (reg_offset == SOC15_REG_OFFSET(GC, 0, regGB_ADDR_CONFIG) && adev->gfx.config.gb_addr_config)
return adev->gfx.config.gb_addr_config;
diff --git a/drivers/gpu/drm/amd/amdgpu/soc24.c b/drivers/gpu/drm/amd/amdgpu/soc24.c
index d1adf19a5..265db9331 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc24.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc24.c
@@ -132,31 +132,12 @@ static struct soc15_allowed_register_entry soc24_allowed_read_registers[] = {
{ SOC15_REG_ENTRY(GC, 0, regGB_ADDR_CONFIG)},
};
-static uint32_t soc24_read_indexed_register(struct amdgpu_device *adev,
- u32 se_num,
- u32 sh_num,
- u32 reg_offset)
-{
- uint32_t val;
-
- mutex_lock(&adev->grbm_idx_mutex);
- if (se_num != 0xffffffff || sh_num != 0xffffffff)
- amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff, 0);
-
- val = RREG32(reg_offset);
-
- if (se_num != 0xffffffff || sh_num != 0xffffffff)
- amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff, 0);
- mutex_unlock(&adev->grbm_idx_mutex);
- return val;
-}
-
static uint32_t soc24_get_register_value(struct amdgpu_device *adev,
bool indexed, u32 se_num,
u32 sh_num, u32 reg_offset)
{
if (indexed) {
- return soc24_read_indexed_register(adev, se_num, sh_num, reg_offset);
+ return amdgpu_read_indexed_register(adev, se_num, sh_num, reg_offset);
} else {
if (reg_offset == SOC15_REG_OFFSET(GC, 0, regGB_ADDR_CONFIG) &&
adev->gfx.config.gb_addr_config)
diff --git a/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c b/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c
index 709b1669b..4a5fe8e9d 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c
@@ -184,31 +184,13 @@ static struct soc15_allowed_register_entry soc_v1_0_allowed_read_registers[] = {
{ SOC15_REG_ENTRY(GC, 0, regGB_ADDR_CONFIG_1) },
};
-static uint32_t soc_v1_0_read_indexed_register(struct amdgpu_device *adev,
- u32 se_num,
- u32 sh_num,
- u32 reg_offset)
-{
- uint32_t val;
-
- mutex_lock(&adev->grbm_idx_mutex);
- if (se_num != 0xffffffff || sh_num != 0xffffffff)
- amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff, 0);
-
- val = RREG32(reg_offset);
-
- if (se_num != 0xffffffff || sh_num != 0xffffffff)
- amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff, 0);
- mutex_unlock(&adev->grbm_idx_mutex);
- return val;
-}
static uint32_t soc_v1_0_get_register_value(struct amdgpu_device *adev,
bool indexed, u32 se_num,
u32 sh_num, u32 reg_offset)
{
if (indexed) {
- return soc_v1_0_read_indexed_register(adev, se_num, sh_num, reg_offset);
+ return amdgpu_read_indexed_register(adev, se_num, sh_num, reg_offset);
} else {
if (reg_offset == SOC15_REG_OFFSET(GC, 0, regGB_ADDR_CONFIG_1) &&
adev->gfx.config.gb_addr_config)
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3 2/2] drm/amdgpu: move read_indexed_register to amdgpu_reg_access
2026-04-24 0:49 ` [PATCH v3 2/2] drm/amdgpu: move read_indexed_register to amdgpu_reg_access Gabriel Almeida
@ 2026-04-24 7:24 ` Christian König
2026-04-24 14:27 ` Alex Deucher
0 siblings, 1 reply; 4+ messages in thread
From: Christian König @ 2026-04-24 7:24 UTC (permalink / raw)
To: Gabriel Almeida, alexander.deucher, airlied, simona
Cc: linux-kernel, amd-gfx, dri-devel
On 4/24/26 02:49, Gabriel Almeida wrote:
> The read_indexed_register helper is duplicated across multiple files
> with identical logic.
>
> Move it to amdgpu_reg_access.c as
> amdgpu_read_indexed_register and update all users accordingly.
>
> No functional changes intended.
Yeah, as far as I can see we have already abstracted the HW generation depending parts in amdgpu_gfx_select_se_sh(), so the change is most likely a valid cleanup.
> Signed-off-by: Gabriel Almeida <gabrielsousa230@gmail.com>
> ---
> v3:
> - split into two patches as requested
>
> v2:
> - move read_indexed_register to amdgpu_reg_access.c
> - drop amdgpu_common
>
> .../gpu/drm/amd/amdgpu/amdgpu_reg_access.c | 18 ++++++++++++++++
> .../gpu/drm/amd/amdgpu/amdgpu_reg_access.h | 3 +++
> drivers/gpu/drm/amd/amdgpu/nv.c | 19 +----------------
> drivers/gpu/drm/amd/amdgpu/soc15.c | 19 +----------------
> drivers/gpu/drm/amd/amdgpu/soc21.c | 19 +----------------
> drivers/gpu/drm/amd/amdgpu/soc24.c | 21 +------------------
> drivers/gpu/drm/amd/amdgpu/soc_v1_0.c | 20 +-----------------
> 7 files changed, 26 insertions(+), 93 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.c
> index 540040c76..daefbeeee 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.c
> @@ -956,3 +956,21 @@ uint32_t amdgpu_device_wait_on_rreg(struct amdgpu_device *adev, uint32_t inst,
> }
> return ret;
> }
> +
> +
A bit kerneldoc here would be nice to have, but not mandatory.
With that done Reviewed-by: Christian König <christian.koenig@amd.com>
Regards,
Christian.
> +uint32_t amdgpu_read_indexed_register(struct amdgpu_device *adev,
> + u32 se_num, u32 sh_num, u32 reg_offset)
> +{
> + uint32_t val;
> +
> + mutex_lock(&adev->grbm_idx_mutex);
> + if (se_num != 0xffffffff || sh_num != 0xffffffff)
> + amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff, 0);
> +
> + val = RREG32(reg_offset);
> +
> + if (se_num != 0xffffffff || sh_num != 0xffffffff)
> + amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff, 0);
> + mutex_unlock(&adev->grbm_idx_mutex);
> + return val;
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.h
> index 4d88e5cd1..a1011af6b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.h
> @@ -160,4 +160,7 @@ uint32_t amdgpu_device_wait_on_rreg(struct amdgpu_device *adev, uint32_t inst,
> uint32_t reg_addr, char reg_name[],
> uint32_t expected_value, uint32_t mask);
>
> +uint32_t amdgpu_read_indexed_register(struct amdgpu_device *adev,
> + u32 se_num, u32 sh_num, u32 reg_offset);
> +
> #endif /* __AMDGPU_REG_ACCESS_H__ */
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
> index 030d80664..72edf5326 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> @@ -354,29 +354,12 @@ static struct soc15_allowed_register_entry nv_allowed_read_registers[] = {
> { SOC15_REG_ENTRY(GC, 0, mmGB_ADDR_CONFIG)},
> };
>
> -static uint32_t nv_read_indexed_register(struct amdgpu_device *adev, u32 se_num,
> - u32 sh_num, u32 reg_offset)
> -{
> - uint32_t val;
> -
> - mutex_lock(&adev->grbm_idx_mutex);
> - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> - amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff, 0);
> -
> - val = RREG32(reg_offset);
> -
> - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> - amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff, 0);
> - mutex_unlock(&adev->grbm_idx_mutex);
> - return val;
> -}
> -
> static uint32_t nv_get_register_value(struct amdgpu_device *adev,
> bool indexed, u32 se_num,
> u32 sh_num, u32 reg_offset)
> {
> if (indexed) {
> - return nv_read_indexed_register(adev, se_num, sh_num, reg_offset);
> + return amdgpu_read_indexed_register(adev, se_num, sh_num, reg_offset);
> } else {
> if (reg_offset == SOC15_REG_OFFSET(GC, 0, mmGB_ADDR_CONFIG))
> return adev->gfx.config.gb_addr_config;
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
> index 27bcbbae5..87b398dd0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> @@ -401,29 +401,12 @@ static struct soc15_allowed_register_entry soc15_allowed_read_registers[] = {
> { SOC15_REG_ENTRY(GC, 0, mmDB_DEBUG2)},
> };
>
> -static uint32_t soc15_read_indexed_register(struct amdgpu_device *adev, u32 se_num,
> - u32 sh_num, u32 reg_offset)
> -{
> - uint32_t val;
> -
> - mutex_lock(&adev->grbm_idx_mutex);
> - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> - amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff, 0);
> -
> - val = RREG32(reg_offset);
> -
> - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> - amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff, 0);
> - mutex_unlock(&adev->grbm_idx_mutex);
> - return val;
> -}
> -
> static uint32_t soc15_get_register_value(struct amdgpu_device *adev,
> bool indexed, u32 se_num,
> u32 sh_num, u32 reg_offset)
> {
> if (indexed) {
> - return soc15_read_indexed_register(adev, se_num, sh_num, reg_offset);
> + return amdgpu_read_indexed_register(adev, se_num, sh_num, reg_offset);
> } else {
> if (reg_offset == SOC15_REG_OFFSET(GC, 0, mmGB_ADDR_CONFIG))
> return adev->gfx.config.gb_addr_config;
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c b/drivers/gpu/drm/amd/amdgpu/soc21.c
> index 7e4353d0c..93c002e51 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc21.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
> @@ -306,29 +306,12 @@ static struct soc15_allowed_register_entry soc21_allowed_read_registers[] = {
> { SOC15_REG_ENTRY(GC, 0, regGB_ADDR_CONFIG)},
> };
>
> -static uint32_t soc21_read_indexed_register(struct amdgpu_device *adev, u32 se_num,
> - u32 sh_num, u32 reg_offset)
> -{
> - uint32_t val;
> -
> - mutex_lock(&adev->grbm_idx_mutex);
> - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> - amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff, 0);
> -
> - val = RREG32(reg_offset);
> -
> - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> - amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff, 0);
> - mutex_unlock(&adev->grbm_idx_mutex);
> - return val;
> -}
> -
> static uint32_t soc21_get_register_value(struct amdgpu_device *adev,
> bool indexed, u32 se_num,
> u32 sh_num, u32 reg_offset)
> {
> if (indexed) {
> - return soc21_read_indexed_register(adev, se_num, sh_num, reg_offset);
> + return amdgpu_read_indexed_register(adev, se_num, sh_num, reg_offset);
> } else {
> if (reg_offset == SOC15_REG_OFFSET(GC, 0, regGB_ADDR_CONFIG) && adev->gfx.config.gb_addr_config)
> return adev->gfx.config.gb_addr_config;
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc24.c b/drivers/gpu/drm/amd/amdgpu/soc24.c
> index d1adf19a5..265db9331 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc24.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc24.c
> @@ -132,31 +132,12 @@ static struct soc15_allowed_register_entry soc24_allowed_read_registers[] = {
> { SOC15_REG_ENTRY(GC, 0, regGB_ADDR_CONFIG)},
> };
>
> -static uint32_t soc24_read_indexed_register(struct amdgpu_device *adev,
> - u32 se_num,
> - u32 sh_num,
> - u32 reg_offset)
> -{
> - uint32_t val;
> -
> - mutex_lock(&adev->grbm_idx_mutex);
> - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> - amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff, 0);
> -
> - val = RREG32(reg_offset);
> -
> - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> - amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff, 0);
> - mutex_unlock(&adev->grbm_idx_mutex);
> - return val;
> -}
> -
> static uint32_t soc24_get_register_value(struct amdgpu_device *adev,
> bool indexed, u32 se_num,
> u32 sh_num, u32 reg_offset)
> {
> if (indexed) {
> - return soc24_read_indexed_register(adev, se_num, sh_num, reg_offset);
> + return amdgpu_read_indexed_register(adev, se_num, sh_num, reg_offset);
> } else {
> if (reg_offset == SOC15_REG_OFFSET(GC, 0, regGB_ADDR_CONFIG) &&
> adev->gfx.config.gb_addr_config)
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c b/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c
> index 709b1669b..4a5fe8e9d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c
> @@ -184,31 +184,13 @@ static struct soc15_allowed_register_entry soc_v1_0_allowed_read_registers[] = {
> { SOC15_REG_ENTRY(GC, 0, regGB_ADDR_CONFIG_1) },
> };
>
> -static uint32_t soc_v1_0_read_indexed_register(struct amdgpu_device *adev,
> - u32 se_num,
> - u32 sh_num,
> - u32 reg_offset)
> -{
> - uint32_t val;
> -
> - mutex_lock(&adev->grbm_idx_mutex);
> - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> - amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff, 0);
> -
> - val = RREG32(reg_offset);
> -
> - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> - amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff, 0);
> - mutex_unlock(&adev->grbm_idx_mutex);
> - return val;
> -}
>
> static uint32_t soc_v1_0_get_register_value(struct amdgpu_device *adev,
> bool indexed, u32 se_num,
> u32 sh_num, u32 reg_offset)
> {
> if (indexed) {
> - return soc_v1_0_read_indexed_register(adev, se_num, sh_num, reg_offset);
> + return amdgpu_read_indexed_register(adev, se_num, sh_num, reg_offset);
> } else {
> if (reg_offset == SOC15_REG_OFFSET(GC, 0, regGB_ADDR_CONFIG_1) &&
> adev->gfx.config.gb_addr_config)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3 2/2] drm/amdgpu: move read_indexed_register to amdgpu_reg_access
2026-04-24 7:24 ` Christian König
@ 2026-04-24 14:27 ` Alex Deucher
0 siblings, 0 replies; 4+ messages in thread
From: Alex Deucher @ 2026-04-24 14:27 UTC (permalink / raw)
To: Christian König
Cc: Gabriel Almeida, alexander.deucher, airlied, simona,
linux-kernel, amd-gfx, dri-devel
Applied the series. Thanks!
Alex
On Fri, Apr 24, 2026 at 3:49 AM Christian König
<christian.koenig@amd.com> wrote:
>
> On 4/24/26 02:49, Gabriel Almeida wrote:
> > The read_indexed_register helper is duplicated across multiple files
> > with identical logic.
> >
> > Move it to amdgpu_reg_access.c as
> > amdgpu_read_indexed_register and update all users accordingly.
> >
> > No functional changes intended.
>
> Yeah, as far as I can see we have already abstracted the HW generation depending parts in amdgpu_gfx_select_se_sh(), so the change is most likely a valid cleanup.
>
> > Signed-off-by: Gabriel Almeida <gabrielsousa230@gmail.com>
> > ---
> > v3:
> > - split into two patches as requested
> >
> > v2:
> > - move read_indexed_register to amdgpu_reg_access.c
> > - drop amdgpu_common
> >
> > .../gpu/drm/amd/amdgpu/amdgpu_reg_access.c | 18 ++++++++++++++++
> > .../gpu/drm/amd/amdgpu/amdgpu_reg_access.h | 3 +++
> > drivers/gpu/drm/amd/amdgpu/nv.c | 19 +----------------
> > drivers/gpu/drm/amd/amdgpu/soc15.c | 19 +----------------
> > drivers/gpu/drm/amd/amdgpu/soc21.c | 19 +----------------
> > drivers/gpu/drm/amd/amdgpu/soc24.c | 21 +------------------
> > drivers/gpu/drm/amd/amdgpu/soc_v1_0.c | 20 +-----------------
> > 7 files changed, 26 insertions(+), 93 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.c
> > index 540040c76..daefbeeee 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.c
> > @@ -956,3 +956,21 @@ uint32_t amdgpu_device_wait_on_rreg(struct amdgpu_device *adev, uint32_t inst,
> > }
> > return ret;
> > }
> > +
> > +
>
> A bit kerneldoc here would be nice to have, but not mandatory.
>
> With that done Reviewed-by: Christian König <christian.koenig@amd.com>
>
> Regards,
> Christian.
>
> > +uint32_t amdgpu_read_indexed_register(struct amdgpu_device *adev,
> > + u32 se_num, u32 sh_num, u32 reg_offset)
> > +{
> > + uint32_t val;
> > +
> > + mutex_lock(&adev->grbm_idx_mutex);
> > + if (se_num != 0xffffffff || sh_num != 0xffffffff)
> > + amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff, 0);
> > +
> > + val = RREG32(reg_offset);
> > +
> > + if (se_num != 0xffffffff || sh_num != 0xffffffff)
> > + amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff, 0);
> > + mutex_unlock(&adev->grbm_idx_mutex);
> > + return val;
> > +}
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.h
> > index 4d88e5cd1..a1011af6b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.h
> > @@ -160,4 +160,7 @@ uint32_t amdgpu_device_wait_on_rreg(struct amdgpu_device *adev, uint32_t inst,
> > uint32_t reg_addr, char reg_name[],
> > uint32_t expected_value, uint32_t mask);
> >
> > +uint32_t amdgpu_read_indexed_register(struct amdgpu_device *adev,
> > + u32 se_num, u32 sh_num, u32 reg_offset);
> > +
> > #endif /* __AMDGPU_REG_ACCESS_H__ */
> > diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
> > index 030d80664..72edf5326 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> > @@ -354,29 +354,12 @@ static struct soc15_allowed_register_entry nv_allowed_read_registers[] = {
> > { SOC15_REG_ENTRY(GC, 0, mmGB_ADDR_CONFIG)},
> > };
> >
> > -static uint32_t nv_read_indexed_register(struct amdgpu_device *adev, u32 se_num,
> > - u32 sh_num, u32 reg_offset)
> > -{
> > - uint32_t val;
> > -
> > - mutex_lock(&adev->grbm_idx_mutex);
> > - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> > - amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff, 0);
> > -
> > - val = RREG32(reg_offset);
> > -
> > - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> > - amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff, 0);
> > - mutex_unlock(&adev->grbm_idx_mutex);
> > - return val;
> > -}
> > -
> > static uint32_t nv_get_register_value(struct amdgpu_device *adev,
> > bool indexed, u32 se_num,
> > u32 sh_num, u32 reg_offset)
> > {
> > if (indexed) {
> > - return nv_read_indexed_register(adev, se_num, sh_num, reg_offset);
> > + return amdgpu_read_indexed_register(adev, se_num, sh_num, reg_offset);
> > } else {
> > if (reg_offset == SOC15_REG_OFFSET(GC, 0, mmGB_ADDR_CONFIG))
> > return adev->gfx.config.gb_addr_config;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
> > index 27bcbbae5..87b398dd0 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> > @@ -401,29 +401,12 @@ static struct soc15_allowed_register_entry soc15_allowed_read_registers[] = {
> > { SOC15_REG_ENTRY(GC, 0, mmDB_DEBUG2)},
> > };
> >
> > -static uint32_t soc15_read_indexed_register(struct amdgpu_device *adev, u32 se_num,
> > - u32 sh_num, u32 reg_offset)
> > -{
> > - uint32_t val;
> > -
> > - mutex_lock(&adev->grbm_idx_mutex);
> > - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> > - amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff, 0);
> > -
> > - val = RREG32(reg_offset);
> > -
> > - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> > - amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff, 0);
> > - mutex_unlock(&adev->grbm_idx_mutex);
> > - return val;
> > -}
> > -
> > static uint32_t soc15_get_register_value(struct amdgpu_device *adev,
> > bool indexed, u32 se_num,
> > u32 sh_num, u32 reg_offset)
> > {
> > if (indexed) {
> > - return soc15_read_indexed_register(adev, se_num, sh_num, reg_offset);
> > + return amdgpu_read_indexed_register(adev, se_num, sh_num, reg_offset);
> > } else {
> > if (reg_offset == SOC15_REG_OFFSET(GC, 0, mmGB_ADDR_CONFIG))
> > return adev->gfx.config.gb_addr_config;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c b/drivers/gpu/drm/amd/amdgpu/soc21.c
> > index 7e4353d0c..93c002e51 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/soc21.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
> > @@ -306,29 +306,12 @@ static struct soc15_allowed_register_entry soc21_allowed_read_registers[] = {
> > { SOC15_REG_ENTRY(GC, 0, regGB_ADDR_CONFIG)},
> > };
> >
> > -static uint32_t soc21_read_indexed_register(struct amdgpu_device *adev, u32 se_num,
> > - u32 sh_num, u32 reg_offset)
> > -{
> > - uint32_t val;
> > -
> > - mutex_lock(&adev->grbm_idx_mutex);
> > - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> > - amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff, 0);
> > -
> > - val = RREG32(reg_offset);
> > -
> > - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> > - amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff, 0);
> > - mutex_unlock(&adev->grbm_idx_mutex);
> > - return val;
> > -}
> > -
> > static uint32_t soc21_get_register_value(struct amdgpu_device *adev,
> > bool indexed, u32 se_num,
> > u32 sh_num, u32 reg_offset)
> > {
> > if (indexed) {
> > - return soc21_read_indexed_register(adev, se_num, sh_num, reg_offset);
> > + return amdgpu_read_indexed_register(adev, se_num, sh_num, reg_offset);
> > } else {
> > if (reg_offset == SOC15_REG_OFFSET(GC, 0, regGB_ADDR_CONFIG) && adev->gfx.config.gb_addr_config)
> > return adev->gfx.config.gb_addr_config;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/soc24.c b/drivers/gpu/drm/amd/amdgpu/soc24.c
> > index d1adf19a5..265db9331 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/soc24.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/soc24.c
> > @@ -132,31 +132,12 @@ static struct soc15_allowed_register_entry soc24_allowed_read_registers[] = {
> > { SOC15_REG_ENTRY(GC, 0, regGB_ADDR_CONFIG)},
> > };
> >
> > -static uint32_t soc24_read_indexed_register(struct amdgpu_device *adev,
> > - u32 se_num,
> > - u32 sh_num,
> > - u32 reg_offset)
> > -{
> > - uint32_t val;
> > -
> > - mutex_lock(&adev->grbm_idx_mutex);
> > - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> > - amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff, 0);
> > -
> > - val = RREG32(reg_offset);
> > -
> > - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> > - amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff, 0);
> > - mutex_unlock(&adev->grbm_idx_mutex);
> > - return val;
> > -}
> > -
> > static uint32_t soc24_get_register_value(struct amdgpu_device *adev,
> > bool indexed, u32 se_num,
> > u32 sh_num, u32 reg_offset)
> > {
> > if (indexed) {
> > - return soc24_read_indexed_register(adev, se_num, sh_num, reg_offset);
> > + return amdgpu_read_indexed_register(adev, se_num, sh_num, reg_offset);
> > } else {
> > if (reg_offset == SOC15_REG_OFFSET(GC, 0, regGB_ADDR_CONFIG) &&
> > adev->gfx.config.gb_addr_config)
> > diff --git a/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c b/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c
> > index 709b1669b..4a5fe8e9d 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c
> > @@ -184,31 +184,13 @@ static struct soc15_allowed_register_entry soc_v1_0_allowed_read_registers[] = {
> > { SOC15_REG_ENTRY(GC, 0, regGB_ADDR_CONFIG_1) },
> > };
> >
> > -static uint32_t soc_v1_0_read_indexed_register(struct amdgpu_device *adev,
> > - u32 se_num,
> > - u32 sh_num,
> > - u32 reg_offset)
> > -{
> > - uint32_t val;
> > -
> > - mutex_lock(&adev->grbm_idx_mutex);
> > - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> > - amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff, 0);
> > -
> > - val = RREG32(reg_offset);
> > -
> > - if (se_num != 0xffffffff || sh_num != 0xffffffff)
> > - amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff, 0);
> > - mutex_unlock(&adev->grbm_idx_mutex);
> > - return val;
> > -}
> >
> > static uint32_t soc_v1_0_get_register_value(struct amdgpu_device *adev,
> > bool indexed, u32 se_num,
> > u32 sh_num, u32 reg_offset)
> > {
> > if (indexed) {
> > - return soc_v1_0_read_indexed_register(adev, se_num, sh_num, reg_offset);
> > + return amdgpu_read_indexed_register(adev, se_num, sh_num, reg_offset);
> > } else {
> > if (reg_offset == SOC15_REG_OFFSET(GC, 0, regGB_ADDR_CONFIG_1) &&
> > adev->gfx.config.gb_addr_config)
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-04-24 14:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-04-24 0:49 [PATCH v3 1/2] drm/amdgpu: move program_aspm to amdgpu_nbio Gabriel Almeida
2026-04-24 0:49 ` [PATCH v3 2/2] drm/amdgpu: move read_indexed_register to amdgpu_reg_access Gabriel Almeida
2026-04-24 7:24 ` Christian König
2026-04-24 14:27 ` Alex Deucher
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®