From: "Lazar, Lijo" <lijo.lazar@amd.com>
To: Alex Deucher <alexdeucher@gmail.com>,
Gabriel Almeida <gabrielsousa230@gmail.com>
Cc: "Christian König" <christian.koenig@amd.com>,
alexander.deucher@amd.com, airlied@gmail.com, simona@ffwll.ch,
linux-kernel@vger.kernel.org, amd-gfx@lists.freedesktop.org,
dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: deduplicate register access and helper routines
Date: Wed, 1 Apr 2026 12:28:09 +0530 [thread overview]
Message-ID: <11e4daaa-0f1e-4fd4-b435-d2fa9d058869@amd.com> (raw)
In-Reply-To: <CADnq5_Pf841oeJAwhLp=1Y_tOWmEX+gyhDW06Z=qr+FR4x4JEw@mail.gmail.com>
On 01-Apr-26 2:51 AM, Alex Deucher wrote:
> On Tue, Mar 31, 2026 at 5:07 PM Gabriel Almeida
> <gabrielsousa230@gmail.com> wrote:
>>
>> Hi Christian and Alex,
>>
>> Thank you both for your feedback.
>>
>> I understand that there can be differences between these functions due to
>> different macro values across hardware generations. I admit that I didn’t
>> fully take that into account in this patch.
>>
>> Among the functions I modified, `program_aspm` and `common_sw_init` seem
>> to have identical behavior regardless of those macros, so I thought they
>> could be good candidates for shared helper functions. That said,
>> `common_sw_init` is currently only identical across NV, SOC21 and SOC24,
>> so I’m not sure if you would consider it generic enough for such use.
>>
>> Regarding `read_indexed_register`, I’m still uncertain due to the use of
>> the `RREG32` macro. From what I’ve seen so far, it appears to behave
>> consistently across these implementations, but I may be missing some
>> subtleties.
>
> You are correct. the RREG32 and WREG32 macros are the same on all chips.
>
>>
>> Also, when Christian mentioned “move them a layer up”, do you mean moving
>> these helpers into an existing common file such as `amdgpu_device.c`
>> instead of introducing a new file like `amdgpu_common.c/h`? I can rework
>> the patch accordingly and drop the new files if that is the preferred
>> approach.
>
> I think something like amdgpu_common_helpers.c is fine, although
> thinking about it more, I think the program_aspm() function should
> probably end up in amdgpu_nbio.c as something like
> amdgpu_nbio_program_aspm(). read_indexed_register() could probably go
> in amdgpu_device.c as amdgpu_device_read_indexed_register_helper().
For the grbm register access one, consider keeping it in amdgpu_reg_access.c
Thanks,
Lijo
> And finally I'm not sure it's worth breaking out common_sw_init() as a
> separate function. Maybe drop that change.
>
> Alex
>
>>
>> I can also incorporate Alex’s suggestions regarding naming and licensing.
>>
>> Given these points, I’d like to better understand which direction you
>> would prefer for this change.
>>
>> Thanks again for your time and guidance.
>>
>> Best regards,
>> Gabriel Almeida
>>
>>
>> Em ter., 31 de mar. de 2026 às 10:31, Alex Deucher
>> <alexdeucher@gmail.com> escreveu:
>>>
>>> On Tue, Mar 31, 2026 at 7:34 AM Christian König
>>> <christian.koenig@amd.com> wrote:
>>>>
>>>> On 3/31/26 00:45, Gabriel Almeida wrote:
>>>>> Some helper functions are implemented multiple times with identical
>>>>> logic across different source files.
>>>>
>>>> And that is at least sometimes completely intentional.
>>>>
>>>> Background is that different headers are included which define macros with different values for each HW generation.
>>>>
>>>>>
>>>>> Extract these implementations into a shared helper file
>>>>> (amdgpu_common.c) and update existing code to reuse them.
>>>>
>>>> Please don't when they are functional identical then move them a layer up instead of messing up the backends.
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>>
>>>>> This simplifies the codebase and avoids duplication without
>>>>> changing behavior.
>>>>>
>>>>> No functional changes intended.
>>>>>
>>>>> Signed-off-by: Gabriel Almeida <gabrielsousa230@gmail.com>
>>>>> ---
>>>>> drivers/gpu/drm/amd/amdgpu/Makefile | 2 ++
>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_common.c | 42 ++++++++++++++++++++++
>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_common.h | 12 +++++++
>>>
>>> I think amdgpu_common_helper.c/h would be better.
>>>
>>>>> drivers/gpu/drm/amd/amdgpu/nv.c | 38 +++-----------------
>>>>> drivers/gpu/drm/amd/amdgpu/soc15.c | 31 ++--------------
>>>>> drivers/gpu/drm/amd/amdgpu/soc21.c | 38 +++-----------------
>>>>> drivers/gpu/drm/amd/amdgpu/soc24.c | 29 ++-------------
>>>>> drivers/gpu/drm/amd/amdgpu/soc_v1_0.c | 21 ++---------
>>>>> 8 files changed, 72 insertions(+), 141 deletions(-)
>>>>> create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_common.c
>>>>> create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_common.h
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
>>>>> index 6a7e9bfec..84cce03d7 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
>>>>> @@ -69,6 +69,8 @@ amdgpu-y += amdgpu_device.o amdgpu_reg_access.o amdgpu_doorbell_mgr.o amdgpu_kms
>>>>> amdgpu_ring_mux.o amdgpu_xcp.o amdgpu_seq64.o amdgpu_aca.o amdgpu_dev_coredump.o \
>>>>> amdgpu_cper.o amdgpu_userq_fence.o amdgpu_eviction_fence.o amdgpu_ip.o
>>>>>
>>>>> +amdgpu-y += amdgpu_common.o
>>>>> +
>>>>> amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
>>>>>
>>>>> amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_common.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_common.c
>>>>> new file mode 100644
>>>>> index 000000000..34ade6f63
>>>>> --- /dev/null
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_common.c
>>>>> @@ -0,0 +1,42 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>
>>> This should be MIT
>>>
>>>>> +#include <linux/module.h>
>>>>> +
>>>>> +#include "amdgpu.h"
>>>>> +#include "amdgpu_common.h"
>>>>> +#include "mxgpu_nv.h"
>>>>> +
>>>>> +uint32_t 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;
>>>>> +}
>>>>> +
>>>>> +void 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);
>>>>> +}
>>>>> +
>>>>> +int common_sw_init(struct amdgpu_ip_block *ip_block)
>>>
>>> Please prefix each of these functions with amdgpu_common_helper_
>>>
>>>>> +{
>>>>> + struct amdgpu_device *adev = ip_block->adev;
>>>>> +
>>>>> + if (amdgpu_sriov_vf(adev))
>>>>> + xgpu_nv_mailbox_add_irq_id(adev);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_common.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_common.h
>>>>> new file mode 100644
>>>>> index 000000000..314b3506b
>>>>> --- /dev/null
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_common.h
>>>>> @@ -0,0 +1,12 @@
>>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>
>>> This should be MIT
>>>
>>> Alex
>>>
>>>>> +#ifndef __AMDGPU_COMMON_H__
>>>>> +#define __AMDGPU_COMMON_H__
>>>>> +
>>>>> +uint32_t read_indexed_register(struct amdgpu_device *adev,
>>>>> + u32 se_num, u32 sh_num, u32 reg_offset);
>>>>> +
>>>>> +void program_aspm(struct amdgpu_device *adev);
>>>>> +
>>>>> +int common_sw_init(struct amdgpu_ip_block *ip_block);
>>>>> +
>>>>> +#endif
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
>>>>> index 7ce1a1b95..cf8052c73 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
>>>>> @@ -29,6 +29,7 @@
>>>>>
>>>>> #include "amdgpu.h"
>>>>> #include "amdgpu_atombios.h"
>>>>> +#include "amdgpu_common.h"
>>>>> #include "amdgpu_ih.h"
>>>>> #include "amdgpu_uvd.h"
>>>>> #include "amdgpu_vce.h"
>>>>> @@ -354,29 +355,13 @@ 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 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;
>>>>> @@ -511,16 +496,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,
>>>>> @@ -965,12 +940,7 @@ static int nv_common_late_init(struct amdgpu_ip_block *ip_block)
>>>>>
>>>>> static int nv_common_sw_init(struct amdgpu_ip_block *ip_block)
>>>>> {
>>>>> - struct amdgpu_device *adev = ip_block->adev;
>>>>> -
>>>>> - if (amdgpu_sriov_vf(adev))
>>>>> - xgpu_nv_mailbox_add_irq_id(adev);
>>>>> -
>>>>> - return 0;
>>>>> + return common_sw_init(ip_block);
>>>>> }
>>>>>
>>>>> static int nv_common_hw_init(struct amdgpu_ip_block *ip_block)
>>>>> @@ -984,7 +954,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);
>>>>> + 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..a6b91363d 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
>>>>> @@ -28,6 +28,7 @@
>>>>> #include <drm/amdgpu_drm.h>
>>>>>
>>>>> #include "amdgpu.h"
>>>>> +#include "amdgpu_common.h"
>>>>> #include "amdgpu_ih.h"
>>>>> #include "amdgpu_uvd.h"
>>>>> #include "amdgpu_vce.h"
>>>>> @@ -401,29 +402,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 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;
>>>>> @@ -695,15 +679,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 +1259,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);
>>>>> + 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..586d62202 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/soc21.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
>>>>> @@ -27,6 +27,7 @@
>>>>>
>>>>> #include "amdgpu.h"
>>>>> #include "amdgpu_atombios.h"
>>>>> +#include "amdgpu_common.h"
>>>>> #include "amdgpu_ih.h"
>>>>> #include "amdgpu_uvd.h"
>>>>> #include "amdgpu_vce.h"
>>>>> @@ -306,29 +307,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 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;
>>>>> @@ -470,15 +454,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,
>>>>> @@ -912,12 +887,7 @@ static int soc21_common_late_init(struct amdgpu_ip_block *ip_block)
>>>>>
>>>>> static int soc21_common_sw_init(struct amdgpu_ip_block *ip_block)
>>>>> {
>>>>> - struct amdgpu_device *adev = ip_block->adev;
>>>>> -
>>>>> - if (amdgpu_sriov_vf(adev))
>>>>> - xgpu_nv_mailbox_add_irq_id(adev);
>>>>> -
>>>>> - return 0;
>>>>> + return common_sw_init(ip_block);
>>>>> }
>>>>>
>>>>> static int soc21_common_hw_init(struct amdgpu_ip_block *ip_block)
>>>>> @@ -925,7 +895,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);
>>>>> + 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/soc24.c b/drivers/gpu/drm/amd/amdgpu/soc24.c
>>>>> index d1adf19a5..f9341c0e4 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/soc24.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/soc24.c
>>>>> @@ -26,6 +26,7 @@
>>>>> #include <linux/pci.h>
>>>>>
>>>>> #include "amdgpu.h"
>>>>> +#include "amdgpu_common.h"
>>>>> #include "amdgpu_ih.h"
>>>>> #include "amdgpu_uvd.h"
>>>>> #include "amdgpu_vce.h"
>>>>> @@ -132,31 +133,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 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)
>>>>> @@ -455,12 +437,7 @@ static int soc24_common_late_init(struct amdgpu_ip_block *ip_block)
>>>>>
>>>>> static int soc24_common_sw_init(struct amdgpu_ip_block *ip_block)
>>>>> {
>>>>> - struct amdgpu_device *adev = ip_block->adev;
>>>>> -
>>>>> - if (amdgpu_sriov_vf(adev))
>>>>> - xgpu_nv_mailbox_add_irq_id(adev);
>>>>> -
>>>>> - return 0;
>>>>> + return common_sw_init(ip_block);
>>>>> }
>>>>>
>>>>> static int soc24_common_hw_init(struct amdgpu_ip_block *ip_block)
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c b/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c
>>>>> index 709b1669b..2f77fb0b6 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c
>>>>> @@ -21,6 +21,7 @@
>>>>> *
>>>>> */
>>>>> #include "amdgpu.h"
>>>>> +#include "amdgpu_common.h"
>>>>> #include "soc15.h"
>>>>> #include "soc15_common.h"
>>>>> #include "soc_v1_0.h"
>>>>> @@ -184,31 +185,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 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
>>>>>
>>>>
next prev parent reply other threads:[~2026-04-01 6:58 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-30 22:45 Gabriel Almeida
2026-03-31 11:24 ` Christian König
2026-03-31 13:30 ` Alex Deucher
2026-03-31 21:07 ` Gabriel Almeida
2026-03-31 21:21 ` Alex Deucher
2026-04-01 6:58 ` Lazar, Lijo [this message]
2026-04-01 8:21 ` Christian König
2026-04-07 0:59 ` Gabriel Almeida
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=11e4daaa-0f1e-4fd4-b435-d2fa9d058869@amd.com \
--to=lijo.lazar@amd.com \
--cc=airlied@gmail.com \
--cc=alexander.deucher@amd.com \
--cc=alexdeucher@gmail.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=christian.koenig@amd.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=gabrielsousa230@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=simona@ffwll.ch \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome