On 6/17/26 21:14, Timur Kristóf wrote: > Soft reset means resetting IP blocks individually using > a hardware interconnect (SRBM or GRBM) without assistance > from firmware. > > Soft reset is a useful tool for implementing GPU recovery, > eg. it is already successfully used for SDMA queue resets. > It should be used by a GPU recovery method instead of > being called directly from the ASIC reset code path. > > Currently, this is only used on Carrizo and Stoney, > but doesn't work well and fails on those chips. > A subsequent commit will add a working GFX8 recovery > implementation after the cleanups. > > Note that this commit only cleans up the ASIC reset path, > which also unblocks more opportunities for cleanup for > the various IP blocks. Those will be done in subsequent commits. > > Signed-off-by: Timur Kristóf <[email protected]>
amdgpu_device_pre_asic_reset() looks like it could be cleaned up even more, but this is clearly a step in the right direction. Reviewed-by: Christian König <[email protected]> Regards, Christian. > --- > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 - > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 173 +-------------------- > drivers/gpu/drm/amd/amdgpu/cik.c | 7 - > drivers/gpu/drm/amd/amdgpu/nv.c | 6 - > drivers/gpu/drm/amd/amdgpu/si.c | 7 - > drivers/gpu/drm/amd/amdgpu/soc15.c | 9 -- > drivers/gpu/drm/amd/amdgpu/soc21.c | 12 -- > drivers/gpu/drm/amd/amdgpu/soc24.c | 11 -- > drivers/gpu/drm/amd/amdgpu/soc_v1_0.c | 10 -- > drivers/gpu/drm/amd/amdgpu/vi.c | 22 --- > 10 files changed, 2 insertions(+), 258 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > index cb4fa9c111eb..7c5ca9e44d54 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > @@ -586,8 +586,6 @@ struct amdgpu_asic_funcs { > /* invalidate hdp read cache */ > void (*invalidate_hdp)(struct amdgpu_device *adev, > struct amdgpu_ring *ring); > - /* check if the asic needs a full reset of if soft reset will work */ > - bool (*need_full_reset)(struct amdgpu_device *adev); > /* initialize doorbell layout for specific asic*/ > void (*init_doorbell_index)(struct amdgpu_device *adev); > /* PCIe bandwidth usage */ > @@ -1355,7 +1353,6 @@ int emu_soc_asic_init(struct amdgpu_device *adev); > #define amdgpu_asic_read_bios_from_rom(adev, b, l) > (adev)->asic_funcs->read_bios_from_rom((adev), (b), (l)) > #define amdgpu_asic_read_register(adev, se, sh, offset, > v)((adev)->asic_funcs->read_register((adev), (se), (sh), (offset), (v))) > #define amdgpu_asic_get_config_memsize(adev) > (adev)->asic_funcs->get_config_memsize((adev)) > -#define amdgpu_asic_need_full_reset(adev) > (adev)->asic_funcs->need_full_reset((adev)) > #define amdgpu_asic_init_doorbell_index(adev) > (adev)->asic_funcs->init_doorbell_index((adev)) > #define amdgpu_asic_get_pcie_usage(adev, cnt0, cnt1) > ((adev)->asic_funcs->get_pcie_usage((adev), (cnt0), (cnt1))) > #define amdgpu_asic_need_reset_on_init(adev) > (adev)->asic_funcs->need_reset_on_init((adev)) > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 71a6b18ccf23..df7e0f2b3ccc 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -4685,161 +4685,6 @@ int amdgpu_device_resume(struct drm_device *dev, bool > notify_clients) > return 0; > } > > -/** > - * amdgpu_device_ip_check_soft_reset - did soft reset succeed > - * > - * @adev: amdgpu_device pointer > - * > - * The list of all the hardware IPs that make up the asic is walked and > - * the check_soft_reset callbacks are run. check_soft_reset determines > - * if the asic is still hung or not. > - * Returns true if any of the IPs are still in a hung state, false if not. > - */ > -static bool amdgpu_device_ip_check_soft_reset(struct amdgpu_device *adev) > -{ > - int i; > - bool asic_hang = false; > - > - if (amdgpu_sriov_vf(adev)) > - return true; > - > - if (amdgpu_asic_need_full_reset(adev)) > - return true; > - > - for (i = 0; i < adev->num_ip_blocks; i++) { > - if (!adev->ip_blocks[i].status.valid) > - continue; > - if (adev->ip_blocks[i].version->funcs->check_soft_reset) > - adev->ip_blocks[i].status.hang = > - > adev->ip_blocks[i].version->funcs->check_soft_reset( > - &adev->ip_blocks[i]); > - if (adev->ip_blocks[i].status.hang) { > - dev_info(adev->dev, "IP block:%s is hung!\n", > adev->ip_blocks[i].version->funcs->name); > - asic_hang = true; > - } > - } > - return asic_hang; > -} > - > -/** > - * amdgpu_device_ip_pre_soft_reset - prepare for soft reset > - * > - * @adev: amdgpu_device pointer > - * > - * The list of all the hardware IPs that make up the asic is walked and the > - * pre_soft_reset callbacks are run if the block is hung. pre_soft_reset > - * handles any IP specific hardware or software state changes that are > - * necessary for a soft reset to succeed. > - * Returns 0 on success, negative error code on failure. > - */ > -static int amdgpu_device_ip_pre_soft_reset(struct amdgpu_device *adev) > -{ > - int i, r = 0; > - > - for (i = 0; i < adev->num_ip_blocks; i++) { > - if (!adev->ip_blocks[i].status.valid) > - continue; > - if (adev->ip_blocks[i].status.hang && > - adev->ip_blocks[i].version->funcs->pre_soft_reset) { > - r = > adev->ip_blocks[i].version->funcs->pre_soft_reset(&adev->ip_blocks[i]); > - if (r) > - return r; > - } > - } > - > - return 0; > -} > - > -/** > - * amdgpu_device_ip_need_full_reset - check if a full asic reset is needed > - * > - * @adev: amdgpu_device pointer > - * > - * Some hardware IPs cannot be soft reset. If they are hung, a full gpu > - * reset is necessary to recover. > - * Returns true if a full asic reset is required, false if not. > - */ > -static bool amdgpu_device_ip_need_full_reset(struct amdgpu_device *adev) > -{ > - int i; > - > - if (amdgpu_asic_need_full_reset(adev)) > - return true; > - > - for (i = 0; i < adev->num_ip_blocks; i++) { > - if (!adev->ip_blocks[i].status.valid) > - continue; > - if ((adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_GMC) > || > - (adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_SMC) > || > - (adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_ACP) > || > - (adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_DCE) > || > - adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_PSP) > { > - if (adev->ip_blocks[i].status.hang) { > - dev_info(adev->dev, "Some block need full > reset!\n"); > - return true; > - } > - } > - } > - return false; > -} > - > -/** > - * amdgpu_device_ip_soft_reset - do a soft reset > - * > - * @adev: amdgpu_device pointer > - * > - * The list of all the hardware IPs that make up the asic is walked and the > - * soft_reset callbacks are run if the block is hung. soft_reset handles any > - * IP specific hardware or software state changes that are necessary to soft > - * reset the IP. > - * Returns 0 on success, negative error code on failure. > - */ > -static int amdgpu_device_ip_soft_reset(struct amdgpu_device *adev) > -{ > - int i, r = 0; > - > - for (i = 0; i < adev->num_ip_blocks; i++) { > - if (!adev->ip_blocks[i].status.valid) > - continue; > - if (adev->ip_blocks[i].status.hang && > - adev->ip_blocks[i].version->funcs->soft_reset) { > - r = > adev->ip_blocks[i].version->funcs->soft_reset(&adev->ip_blocks[i]); > - if (r) > - return r; > - } > - } > - > - return 0; > -} > - > -/** > - * amdgpu_device_ip_post_soft_reset - clean up from soft reset > - * > - * @adev: amdgpu_device pointer > - * > - * The list of all the hardware IPs that make up the asic is walked and the > - * post_soft_reset callbacks are run if the asic was hung. post_soft_reset > - * handles any IP specific hardware or software state changes that are > - * necessary after the IP has been soft reset. > - * Returns 0 on success, negative error code on failure. > - */ > -static int amdgpu_device_ip_post_soft_reset(struct amdgpu_device *adev) > -{ > - int i, r = 0; > - > - for (i = 0; i < adev->num_ip_blocks; i++) { > - if (!adev->ip_blocks[i].status.valid) > - continue; > - if (adev->ip_blocks[i].status.hang && > - adev->ip_blocks[i].version->funcs->post_soft_reset) > - r = > adev->ip_blocks[i].version->funcs->post_soft_reset(&adev->ip_blocks[i]); > - if (r) > - return r; > - } > - > - return 0; > -} > - > /** > * amdgpu_device_reset_sriov - reset ASIC for SR-IOV vf > * > @@ -5134,20 +4979,7 @@ int amdgpu_device_pre_asic_reset(struct amdgpu_device > *adev, > > /* Don't suspend on bare metal if we are not going to HW reset the ASIC > */ > if (!amdgpu_sriov_vf(adev)) { > - > - if (!need_full_reset) > - need_full_reset = > amdgpu_device_ip_need_full_reset(adev); > - > - if (!need_full_reset && amdgpu_gpu_recovery && > - amdgpu_device_ip_check_soft_reset(adev)) { > - amdgpu_device_ip_pre_soft_reset(adev); > - r = amdgpu_device_ip_soft_reset(adev); > - amdgpu_device_ip_post_soft_reset(adev); > - if (r || amdgpu_device_ip_check_soft_reset(adev)) { > - dev_info(adev->dev, "soft reset failed, will > fallback to full reset!\n"); > - need_full_reset = true; > - } > - } > + need_full_reset = true; > > if (!test_bit(AMDGPU_SKIP_COREDUMP, &reset_context->flags)) { > dev_info(tmp_adev->dev, "Dumping IP State\n"); > @@ -5600,8 +5432,7 @@ static void amdgpu_device_halt_activities(struct > amdgpu_device *adev, > drm_client_dev_suspend(adev_to_drm(tmp_adev)); > > /* disable ras on ALL IPs */ > - if (!need_emergency_restart && !amdgpu_reset_in_dpc(adev) && > - amdgpu_device_ip_need_full_reset(tmp_adev)) > + if (!need_emergency_restart && !amdgpu_reset_in_dpc(adev)) > amdgpu_ras_suspend(tmp_adev); > > amdgpu_userq_pre_reset(tmp_adev); > diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c > b/drivers/gpu/drm/amd/amdgpu/cik.c > index 29954c7d61b0..77e120a72815 100644 > --- a/drivers/gpu/drm/amd/amdgpu/cik.c > +++ b/drivers/gpu/drm/amd/amdgpu/cik.c > @@ -1876,12 +1876,6 @@ static void cik_invalidate_hdp(struct amdgpu_device > *adev, > } > } > > -static bool cik_need_full_reset(struct amdgpu_device *adev) > -{ > - /* change this when we support soft reset */ > - return true; > -} > - > static void cik_get_pcie_usage(struct amdgpu_device *adev, uint64_t *count0, > uint64_t *count1) > { > @@ -1971,7 +1965,6 @@ static const struct amdgpu_asic_funcs cik_asic_funcs = > .get_config_memsize = &cik_get_config_memsize, > .flush_hdp = &cik_flush_hdp, > .invalidate_hdp = &cik_invalidate_hdp, > - .need_full_reset = &cik_need_full_reset, > .init_doorbell_index = &legacy_doorbell_index_init, > .get_pcie_usage = &cik_get_pcie_usage, > .need_reset_on_init = &cik_need_reset_on_init, > diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c > index 72edf5326b05..77557ee3ca16 100644 > --- a/drivers/gpu/drm/amd/amdgpu/nv.c > +++ b/drivers/gpu/drm/amd/amdgpu/nv.c > @@ -507,11 +507,6 @@ void nv_set_virt_ops(struct amdgpu_device *adev) > adev->virt.ops = &xgpu_nv_virt_ops; > } > > -static bool nv_need_full_reset(struct amdgpu_device *adev) > -{ > - return true; > -} > - > static bool nv_need_reset_on_init(struct amdgpu_device *adev) > { > u32 sol_reg; > @@ -595,7 +590,6 @@ static const struct amdgpu_asic_funcs nv_asic_funcs = { > .set_vce_clocks = &nv_set_vce_clocks, > .get_config_memsize = &nv_get_config_memsize, > .init_doorbell_index = &nv_init_doorbell_index, > - .need_full_reset = &nv_need_full_reset, > .need_reset_on_init = &nv_need_reset_on_init, > .get_pcie_replay_count = &amdgpu_nbio_get_pcie_replay_count, > .supports_baco = &amdgpu_dpm_is_baco_supported, > diff --git a/drivers/gpu/drm/amd/amdgpu/si.c b/drivers/gpu/drm/amd/amdgpu/si.c > index c26cb3e8bff6..b104469c38ec 100644 > --- a/drivers/gpu/drm/amd/amdgpu/si.c > +++ b/drivers/gpu/drm/amd/amdgpu/si.c > @@ -1509,12 +1509,6 @@ static void si_invalidate_hdp(struct amdgpu_device > *adev, > } > } > > -static bool si_need_full_reset(struct amdgpu_device *adev) > -{ > - /* change this when we support soft reset */ > - return true; > -} > - > static bool si_need_reset_on_init(struct amdgpu_device *adev) > { > return false; > @@ -2019,7 +2013,6 @@ static const struct amdgpu_asic_funcs si_asic_funcs = > .get_config_memsize = &si_get_config_memsize, > .flush_hdp = &si_flush_hdp, > .invalidate_hdp = &si_invalidate_hdp, > - .need_full_reset = &si_need_full_reset, > .get_pcie_usage = &si_get_pcie_usage, > .need_reset_on_init = &si_need_reset_on_init, > .get_pcie_replay_count = &si_get_pcie_replay_count, > diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c > b/drivers/gpu/drm/amd/amdgpu/soc15.c > index 87b398dd0769..ed3fd58b78d0 100644 > --- a/drivers/gpu/drm/amd/amdgpu/soc15.c > +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c > @@ -721,12 +721,6 @@ void soc15_set_virt_ops(struct amdgpu_device *adev) > soc15_reg_base_init(adev); > } > > -static bool soc15_need_full_reset(struct amdgpu_device *adev) > -{ > - /* change this when we implement soft reset */ > - return true; > -} > - > static void soc15_get_pcie_usage(struct amdgpu_device *adev, uint64_t > *count0, > uint64_t *count1) > { > @@ -878,7 +872,6 @@ static const struct amdgpu_asic_funcs soc15_asic_funcs = > .set_uvd_clocks = &soc15_set_uvd_clocks, > .set_vce_clocks = &soc15_set_vce_clocks, > .get_config_memsize = &soc15_get_config_memsize, > - .need_full_reset = &soc15_need_full_reset, > .init_doorbell_index = &vega10_doorbell_index_init, > .get_pcie_usage = &soc15_get_pcie_usage, > .need_reset_on_init = &soc15_need_reset_on_init, > @@ -899,7 +892,6 @@ static const struct amdgpu_asic_funcs vega20_asic_funcs = > .set_uvd_clocks = &soc15_set_uvd_clocks, > .set_vce_clocks = &soc15_set_vce_clocks, > .get_config_memsize = &soc15_get_config_memsize, > - .need_full_reset = &soc15_need_full_reset, > .init_doorbell_index = &vega20_doorbell_index_init, > .get_pcie_usage = &vega20_get_pcie_usage, > .need_reset_on_init = &soc15_need_reset_on_init, > @@ -920,7 +912,6 @@ static const struct amdgpu_asic_funcs > aqua_vanjaram_asic_funcs = > .set_uvd_clocks = &soc15_set_uvd_clocks, > .set_vce_clocks = &soc15_set_vce_clocks, > .get_config_memsize = &soc15_get_config_memsize, > - .need_full_reset = &soc15_need_full_reset, > .init_doorbell_index = &aqua_vanjaram_doorbell_index_init, > .need_reset_on_init = &soc15_need_reset_on_init, > .get_pcie_replay_count = &amdgpu_nbio_get_pcie_replay_count, > diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c > b/drivers/gpu/drm/amd/amdgpu/soc21.c > index 93c002e511c7..6af81c27b986 100644 > --- a/drivers/gpu/drm/amd/amdgpu/soc21.c > +++ b/drivers/gpu/drm/amd/amdgpu/soc21.c > @@ -461,17 +461,6 @@ const struct amdgpu_ip_block_version > soc21_common_ip_block = { > .funcs = &soc21_common_ip_funcs, > }; > > -static bool soc21_need_full_reset(struct amdgpu_device *adev) > -{ > - switch (amdgpu_ip_version(adev, GC_HWIP, 0)) { > - case IP_VERSION(11, 0, 0): > - case IP_VERSION(11, 0, 2): > - case IP_VERSION(11, 0, 3): > - default: > - return true; > - } > -} > - > static bool soc21_need_reset_on_init(struct amdgpu_device *adev) > { > u32 sol_reg; > @@ -550,7 +539,6 @@ static const struct amdgpu_asic_funcs soc21_asic_funcs = { > .set_vce_clocks = &soc21_set_vce_clocks, > .get_config_memsize = &soc21_get_config_memsize, > .init_doorbell_index = &soc21_init_doorbell_index, > - .need_full_reset = &soc21_need_full_reset, > .need_reset_on_init = &soc21_need_reset_on_init, > .get_pcie_replay_count = &amdgpu_nbio_get_pcie_replay_count, > .supports_baco = &amdgpu_dpm_is_baco_supported, > diff --git a/drivers/gpu/drm/amd/amdgpu/soc24.c > b/drivers/gpu/drm/amd/amdgpu/soc24.c > index 265db9331d0b..620970de6eb8 100644 > --- a/drivers/gpu/drm/amd/amdgpu/soc24.c > +++ b/drivers/gpu/drm/amd/amdgpu/soc24.c > @@ -238,16 +238,6 @@ const struct amdgpu_ip_block_version > soc24_common_ip_block = { > .funcs = &soc24_common_ip_funcs, > }; > > -static bool soc24_need_full_reset(struct amdgpu_device *adev) > -{ > - switch (amdgpu_ip_version(adev, GC_HWIP, 0)) { > - case IP_VERSION(12, 0, 0): > - case IP_VERSION(12, 0, 1): > - default: > - return true; > - } > -} > - > static bool soc24_need_reset_on_init(struct amdgpu_device *adev) > { > u32 sol_reg; > @@ -330,7 +320,6 @@ static const struct amdgpu_asic_funcs soc24_asic_funcs = { > .get_xclk = &soc24_get_xclk, > .get_config_memsize = &soc24_get_config_memsize, > .init_doorbell_index = &soc24_init_doorbell_index, > - .need_full_reset = &soc24_need_full_reset, > .need_reset_on_init = &soc24_need_reset_on_init, > .get_pcie_replay_count = &soc24_get_pcie_replay_count, > .supports_baco = &amdgpu_dpm_is_baco_supported, > diff --git a/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c > b/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c > index 5f05c8e68297..4d93f28145d0 100644 > --- a/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c > @@ -223,15 +223,6 @@ static int soc_v1_0_read_register(struct amdgpu_device > *adev, > return -EINVAL; > } > > -static bool soc_v1_0_need_full_reset(struct amdgpu_device *adev) > -{ > - switch (amdgpu_ip_version(adev, GC_HWIP, 0)) { > - case IP_VERSION(12, 1, 0): > - default: > - return true; > - } > -} > - > static bool soc_v1_0_need_reset_on_init(struct amdgpu_device *adev) > { > > @@ -271,7 +262,6 @@ static const struct amdgpu_asic_funcs soc_v1_0_asic_funcs > = { > .read_register = &soc_v1_0_read_register, > .get_config_memsize = &soc_v1_0_get_config_memsize, > .get_xclk = &soc_v1_0_get_xclk, > - .need_full_reset = &soc_v1_0_need_full_reset, > .init_doorbell_index = &soc_v1_0_doorbell_index_init, > .need_reset_on_init = &soc_v1_0_need_reset_on_init, > .encode_ext_smn_addressing = &soc_v1_0_encode_ext_smn_addressing, > diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c > index a256320b92f3..5715b6b596af 100644 > --- a/drivers/gpu/drm/amd/amdgpu/vi.c > +++ b/drivers/gpu/drm/amd/amdgpu/vi.c > @@ -1328,27 +1328,6 @@ static void vi_invalidate_hdp(struct amdgpu_device > *adev, > } > } > > -static bool vi_need_full_reset(struct amdgpu_device *adev) > -{ > - switch (adev->asic_type) { > - case CHIP_CARRIZO: > - case CHIP_STONEY: > - /* CZ has hang issues with full reset at the moment */ > - return false; > - case CHIP_FIJI: > - case CHIP_TONGA: > - /* XXX: soft reset should work on fiji and tonga */ > - return true; > - case CHIP_POLARIS10: > - case CHIP_POLARIS11: > - case CHIP_POLARIS12: > - case CHIP_TOPAZ: > - default: > - /* change this when we support soft reset */ > - return true; > - } > -} > - > static void vi_get_pcie_usage(struct amdgpu_device *adev, uint64_t *count0, > uint64_t *count1) > { > @@ -1437,7 +1416,6 @@ static const struct amdgpu_asic_funcs vi_asic_funcs = > .get_config_memsize = &vi_get_config_memsize, > .flush_hdp = &vi_flush_hdp, > .invalidate_hdp = &vi_invalidate_hdp, > - .need_full_reset = &vi_need_full_reset, > .init_doorbell_index = &legacy_doorbell_index_init, > .get_pcie_usage = &vi_get_pcie_usage, > .need_reset_on_init = &vi_need_reset_on_init,
