AMD General > -----Original Message----- > From: Timur Kristóf <[email protected]> > Sent: Wednesday, June 24, 2026 1:08 PM > To: [email protected]; Deucher, Alexander > <[email protected]>; Koenig, Christian > <[email protected]>; Natalie Vock <[email protected]>; Limonciello, > Mario <[email protected]>; Marek Olšák <[email protected]>; Tvrtko > Ursulin <[email protected]>; Lazar, Lijo <[email protected]>; Martin > Roukala <[email protected]>; SHANMUGAM, SRINIVASAN > <[email protected]> > Cc: Timur Kristóf <[email protected]> > Subject: [PATCH 1/2] drm/amdgpu: Simplify filtering rings during IP block > soft reset > > Instead of storing pointers to affected rings in an array, just iterate over > all rings of > the device and filter the affected rings by type using the type mask. > > This is done to save memory used by the array of affected rings which was > sized > AMDGPU_MAX_RINGS. > > Suggested-by: Srinivasan Shanmugam <[email protected]> > Signed-off-by: Timur Kristóf <[email protected]>
This looks good to me. The new logic uses the same ring_type_mask filtering condition as the old amdgpu_filter_rings() helper, but avoids the temporary AMDGPU_MAX_RINGS-sized array. Reviewed-by: Srinivasan Shanmugam <[email protected]> # for the series Thanks! Srini > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c | 30 ++------------ > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 53 ++++++++++++++++-------- > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 4 +- > 3 files changed, 40 insertions(+), 47 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c > index 65505bc50399a..99ed0b0d82e94 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c > @@ -481,28 +481,6 @@ static u32 amdgpu_ring_mask_from_ip(const enum > amd_ip_block_type ip_type) > } > } > > -/** > - * amdgpu_filter_rings() - Filter rings according to a mask. > - * > - * @adev: amdgpu_device pointer > - * @ring_type_mask: Mask of ring types you are looking for > - * @out_rings: Array of rings which is going to be filled > - * @out_num_rings: Number of rings which were filtered > - */ > -static void amdgpu_filter_rings(struct amdgpu_device *adev, const u32 > ring_type_mask, > - struct amdgpu_ring **out_rings, u32 > *out_num_rings) > -{ > - u32 num_rings = 0; > - int i; > - > - for (i = 0; i < adev->num_rings; ++i) { > - if (BIT(adev->rings[i]->funcs->type) & ring_type_mask) > - out_rings[num_rings++] = adev->rings[i]; > - } > - > - *out_num_rings = num_rings; > -} > - > /** > * amdgpu_device_ip_soft_reset() - Perform a graceful soft reset on an IP > block. > * > @@ -524,10 +502,9 @@ int amdgpu_device_ip_soft_reset(struct amdgpu_ring > *guilty_ring, > struct amdgpu_fence *guilty_fence) > { > struct amdgpu_device *adev = guilty_ring->adev; > - struct amdgpu_ring *rings[AMDGPU_MAX_RINGS]; > struct amdgpu_ip_block *ip_block; > enum amd_ip_block_type ip_type; > - u32 num_rings, ring_type_mask; > + u32 ring_type_mask; > int r; > > ip_type = amdgpu_ip_from_ring(guilty_ring->funcs->type); > @@ -543,14 +520,13 @@ int amdgpu_device_ip_soft_reset(struct amdgpu_ring > *guilty_ring, > ip_block->version->funcs->name); > > ring_type_mask = amdgpu_ring_mask_from_ip(ip_type); > - amdgpu_filter_rings(adev, ring_type_mask, rings, &num_rings); > > amdgpu_device_lock_reset_domain(adev->reset_domain); > - amdgpu_multi_ring_reset_helper_begin(rings, num_rings, guilty_ring, > guilty_fence); > + amdgpu_multi_ring_reset_helper_begin(ring_type_mask, guilty_ring, > +guilty_fence); > > r = ip_block->version->funcs->soft_reset(ip_block); > > - r = amdgpu_multi_ring_reset_helper_end(rings, num_rings, guilty_ring, > r); > + r = amdgpu_multi_ring_reset_helper_end(ring_type_mask, guilty_ring, > +r); > amdgpu_device_unlock_reset_domain(adev->reset_domain); > > if (r) { > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c > index 8062135e73a1d..ab5df854c1d20 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c > @@ -884,8 +884,7 @@ int amdgpu_ring_reset_helper_end(struct amdgpu_ring > *ring, > /** > * amdgpu_multi_ring_reset_helper_begin() - Prepare multiple rings for a > reset. > * > - * @rings: Pointer to an array of amdgpu rings that are affected. > - * @num_rings: Number of rings in the array. > + * @ring_type_mask: Bitmask of affected ring types > * @guilty_ring: The ring which is guilty of causing a reset. > * @guilty_fence: The fence which didn't signal on the guilty ring. > * > @@ -904,7 +903,7 @@ int amdgpu_ring_reset_helper_end(struct amdgpu_ring > *ring, > * After the reset is complete, the caller should then call > * amdgpu_multi_ring_reset_helper_end() to restore the rings. > */ > -void amdgpu_multi_ring_reset_helper_begin(struct amdgpu_ring **rings, u32 > num_rings, > +void amdgpu_multi_ring_reset_helper_begin(const u32 ring_type_mask, > struct amdgpu_ring *guilty_ring, > struct amdgpu_fence *guilty_fence) { > @@ - > 915,8 +914,11 @@ void amdgpu_multi_ring_reset_helper_begin(struct > amdgpu_ring **rings, u32 num_ri > int i; > u32 t; > > - for (i = 0; i < num_rings; ++i) { > - ring = rings[i]; > + for (i = 0; i < adev->num_rings; ++i) { > + ring = adev->rings[i]; > + > + if (!(BIT(ring->funcs->type) & ring_type_mask)) > + continue; > > /* Don't accept new submissions on the ring. */ > if (amdgpu_ring_sched_ready(ring) && > !drm_sched_is_stopped(&ring->sched)) > @@ -949,8 +951,11 @@ void amdgpu_multi_ring_reset_helper_begin(struct > amdgpu_ring **rings, u32 num_ri > rings_busy = false; > > /* Check if any of the non-guilty rings are busy */ > - for (i = 0; i < num_rings; ++i) { > - ring = rings[i]; > + for (i = 0; i < adev->num_rings; ++i) { > + ring = adev->rings[i]; > + > + if (!(BIT(ring->funcs->type) & ring_type_mask)) > + continue; > > if (ring == guilty_ring) > continue; > @@ -966,8 +971,11 @@ void amdgpu_multi_ring_reset_helper_begin(struct > amdgpu_ring **rings, u32 num_ri > mdelay(10); > } > > - for (i = 0; i < num_rings; ++i) { > - ring = rings[i]; > + for (i = 0; i < adev->num_rings; ++i) { > + ring = adev->rings[i]; > + > + if (!(BIT(ring->funcs->type) & ring_type_mask)) > + continue; > > /* > * Find guilty fences, ie. the fences that didn't signal @@ > -991,8 > +999,7 @@ void amdgpu_multi_ring_reset_helper_begin(struct amdgpu_ring > **rings, u32 num_ri > /** > * amdgpu_multi_ring_reset_helper_end() - Prepare multiple rings for a reset. > * > - * @rings: Pointer to an array of amdgpu rings that are affected. > - * @num_rings: Number of rings in the array. > + * @ring_type_mask: Bitmask of affected ring types > * @guilty_ring: The ring which is guilty of causing a reset. > * @ret: Return code from the reset function. > * > @@ -1004,7 +1011,7 @@ void amdgpu_multi_ring_reset_helper_begin(struct > amdgpu_ring **rings, u32 num_ri > * be called to restore some state, but it won't attempt to > * fully restore the ring contents. > */ > -int amdgpu_multi_ring_reset_helper_end(struct amdgpu_ring **rings, u32 > num_rings, > +int amdgpu_multi_ring_reset_helper_end(const u32 ring_type_mask, > struct amdgpu_ring *guilty_ring, int > ret) { > struct amdgpu_device *adev = guilty_ring->adev; @@ -1012,8 +1019,11 > @@ int amdgpu_multi_ring_reset_helper_end(struct amdgpu_ring **rings, u32 > num_rings > int i, r; > > /* Set preempt condition, rings are now allowed to execute submissions > */ > - for (i = 0; i < num_rings; ++i) { > - ring = rings[i]; > + for (i = 0; i < adev->num_rings; ++i) { > + ring = adev->rings[i]; > + > + if (!(BIT(ring->funcs->type) & ring_type_mask)) > + continue; > > if (ring->funcs->init_cond_exec) > amdgpu_ring_set_preempt_cond_exec(ring, true); @@ - > 1027,9 +1037,13 @@ int amdgpu_multi_ring_reset_helper_end(struct amdgpu_ring > **rings, u32 num_rings > return ret; > > /* Restore contents of all rings */ > - for (i = 0; i < num_rings; ++i) { > - ring = rings[i]; > + for (i = 0; i < adev->num_rings; ++i) { > + ring = adev->rings[i]; > + > + if (!(BIT(ring->funcs->type) & ring_type_mask)) > + continue; > > + /* Restore contents of the ring */ > r = amdgpu_ring_reset_helper_end(ring, ring->guilty_fence); > if (r) { > dev_err(adev->dev, > @@ -1040,8 +1054,11 @@ int amdgpu_multi_ring_reset_helper_end(struct > amdgpu_ring **rings, u32 num_rings > } > > /* Accept submissions on all rings again */ > - for (i = 0; i < num_rings; ++i) { > - ring = rings[i]; > + for (i = 0; i < adev->num_rings; ++i) { > + ring = adev->rings[i]; > + > + if (!(BIT(ring->funcs->type) & ring_type_mask)) > + continue; > > if (!amdgpu_ring_sched_ready(ring)) > continue; > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h > index c272e0b028ad8..9d3934b4f1069 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h > @@ -595,10 +595,10 @@ void amdgpu_ring_reset_helper_begin(struct > amdgpu_ring *ring, > struct amdgpu_fence *guilty_fence); int > amdgpu_ring_reset_helper_end(struct amdgpu_ring *ring, > struct amdgpu_fence *guilty_fence); -void > amdgpu_multi_ring_reset_helper_begin(struct amdgpu_ring **rings, u32 > num_rings, > +void amdgpu_multi_ring_reset_helper_begin(const u32 ring_type_mask, > struct amdgpu_ring *guilty_ring, > struct amdgpu_fence *guilty_fence); > -int > amdgpu_multi_ring_reset_helper_end(struct amdgpu_ring **rings, u32 num_rings, > +int amdgpu_multi_ring_reset_helper_end(const u32 ring_type_mask, > struct amdgpu_ring *guilty_ring, int > ret); bool > amdgpu_ring_is_reset_type_supported(struct amdgpu_ring *ring, > u32 reset_type); > -- > 2.54.0
