Hi all, First, thank you all for the feedback and help.
Based on all the messages, I'll change the patch as follows: - Move read_indexed_register function to amdgpu_reg_access.c and rename it to amdgpu_read_indexed_register - Move program_aspm function to amdgpu_nbio.c and rename it to amdgpu_nbio_program_aspm - Remove the change in common_sw_init function - Delete the proposed amdgpu_common file Does it look good to you? Best regards, Gabriel Almeida Em qua., 1 de abr. de 2026 às 05:21, Christian König <[email protected]> escreveu: > > On 4/1/26 08:58, Lazar, Lijo wrote: > > > > > > On 01-Apr-26 2:51 AM, Alex Deucher wrote: > >> On Tue, Mar 31, 2026 at 5:07 PM Gabriel Almeida > >> <[email protected]> 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 > > Yeah I wanted to suggest something similar. > > In general the driver code files are mostly organized like this: > > * amdgpu_drv.[ch] Code for the whole driver, e.g. module_init and co. > * amdgpu_device.[ch] Code for a specific adev instance. > * amdgpu_$ip.[ch] Code for this specific HW IP which is independent of > the HW generation (e.g. amdgpu_sdma.c) > * $ip_$version.[ch] Code for a specific IP in a specific HW generation > (e.g. sdma_v6_0.c) > * $HW_gen.[ch] Code for a specififc HW generation (e.g. cik.c or > soc15.c....) > * $whatever.[ch] Code for interfacing with kernel subsystem, feature, > task, whatever (e.g. amdgpu_ttm.c, amdgpu_cs.c...) > > So when you have some common functionality that should either go into > amdgpu_device.c or amdgpu_$ip.[ch]. > > Regards, > Christian. Thanks for sharing this with me. I wasn't aware of this organization, and it will be very helpful to understand it. > > > > > 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 > >>> <[email protected]> escreveu: > >>>> > >>>> On Tue, Mar 31, 2026 at 7:34 AM Christian König > >>>> <[email protected]> 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 <[email protected]> > >>>>>> --- > >>>>>> 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 > >>>>>> > >>>>> > > >
