* [PATCH 2/2] drm/amdgpu: Use guard() for spin locks
@ 2026-03-11 9:13 renpanpan
2026-03-11 9:18 ` Christian König
0 siblings, 1 reply; 2+ messages in thread
From: renpanpan @ 2026-03-11 9:13 UTC (permalink / raw)
To: linux-kernel; +Cc: alexander.deucher, christian.koenig, airlied, renpanpan
Clean up the code using guard() for spin locks.
Merely code refactoring, and no behavior change.
Signed-off-by: renpanpan <renpanpan@kylinos.cn>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 71 ++++++++--------------
drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 8 +--
2 files changed, 28 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 3e19b51a2763..f9ba1fb36762 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -708,7 +708,6 @@ bool amdgpu_device_supports_smart_shift(struct amdgpu_device *adev)
void amdgpu_device_mm_access(struct amdgpu_device *adev, loff_t pos,
void *buf, size_t size, bool write)
{
- unsigned long flags;
uint32_t hi = ~0, tmp = 0;
uint32_t *data = buf;
uint64_t last;
@@ -719,7 +718,7 @@ void amdgpu_device_mm_access(struct amdgpu_device *adev, loff_t pos,
BUG_ON(!IS_ALIGNED(pos, 4) || !IS_ALIGNED(size, 4));
- spin_lock_irqsave(&adev->mmio_idx_lock, flags);
+ guard(spinlock_irqsave)(&adev->mmio_idx_lock);
for (last = pos + size; pos < last; pos += 4) {
tmp = pos >> 31;
@@ -734,7 +733,6 @@ void amdgpu_device_mm_access(struct amdgpu_device *adev, loff_t pos,
*data++ = RREG32_NO_KIQ(mmMM_DATA);
}
- spin_unlock_irqrestore(&adev->mmio_idx_lock, flags);
drm_dev_exit(idx);
}
@@ -1087,7 +1085,7 @@ void amdgpu_device_xcc_wreg(struct amdgpu_device *adev,
u32 amdgpu_device_indirect_rreg(struct amdgpu_device *adev,
u32 reg_addr)
{
- unsigned long flags, pcie_index, pcie_data;
+ unsigned long pcie_index, pcie_data;
void __iomem *pcie_index_offset;
void __iomem *pcie_data_offset;
u32 r;
@@ -1095,14 +1093,13 @@ u32 amdgpu_device_indirect_rreg(struct amdgpu_device *adev,
pcie_index = adev->nbio.funcs->get_pcie_index_offset(adev);
pcie_data = adev->nbio.funcs->get_pcie_data_offset(adev);
- spin_lock_irqsave(&adev->pcie_idx_lock, flags);
+ guard(spinlock_irqsave)(&adev->pcie_idx_lock);
pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
pcie_data_offset = (void __iomem *)adev->rmmio + pcie_data * 4;
writel(reg_addr, pcie_index_offset);
readl(pcie_index_offset);
r = readl(pcie_data_offset);
- spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
return r;
}
@@ -1110,7 +1107,7 @@ u32 amdgpu_device_indirect_rreg(struct amdgpu_device *adev,
u32 amdgpu_device_indirect_rreg_ext(struct amdgpu_device *adev,
u64 reg_addr)
{
- unsigned long flags, pcie_index, pcie_index_hi, pcie_data;
+ unsigned long pcie_index, pcie_index_hi, pcie_data;
u32 r;
void __iomem *pcie_index_offset;
void __iomem *pcie_index_hi_offset;
@@ -1133,7 +1130,7 @@ u32 amdgpu_device_indirect_rreg_ext(struct amdgpu_device *adev,
pcie_index_hi = 0;
}
- spin_lock_irqsave(&adev->pcie_idx_lock, flags);
+ guard(spinlock_irqsave)(&adev->pcie_idx_lock);
pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
pcie_data_offset = (void __iomem *)adev->rmmio + pcie_data * 4;
if (pcie_index_hi != 0)
@@ -1154,8 +1151,6 @@ u32 amdgpu_device_indirect_rreg_ext(struct amdgpu_device *adev,
readl(pcie_index_hi_offset);
}
- spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
-
return r;
}
@@ -1170,7 +1165,7 @@ u32 amdgpu_device_indirect_rreg_ext(struct amdgpu_device *adev,
u64 amdgpu_device_indirect_rreg64(struct amdgpu_device *adev,
u32 reg_addr)
{
- unsigned long flags, pcie_index, pcie_data;
+ unsigned long pcie_index, pcie_data;
void __iomem *pcie_index_offset;
void __iomem *pcie_data_offset;
u64 r;
@@ -1178,7 +1173,7 @@ u64 amdgpu_device_indirect_rreg64(struct amdgpu_device *adev,
pcie_index = adev->nbio.funcs->get_pcie_index_offset(adev);
pcie_data = adev->nbio.funcs->get_pcie_data_offset(adev);
- spin_lock_irqsave(&adev->pcie_idx_lock, flags);
+ guard(spinlock_irqsave)(&adev->pcie_idx_lock);
pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
pcie_data_offset = (void __iomem *)adev->rmmio + pcie_data * 4;
@@ -1190,7 +1185,6 @@ u64 amdgpu_device_indirect_rreg64(struct amdgpu_device *adev,
writel(reg_addr + 4, pcie_index_offset);
readl(pcie_index_offset);
r |= ((u64)readl(pcie_data_offset) << 32);
- spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
return r;
}
@@ -1198,7 +1192,7 @@ u64 amdgpu_device_indirect_rreg64(struct amdgpu_device *adev,
u64 amdgpu_device_indirect_rreg64_ext(struct amdgpu_device *adev,
u64 reg_addr)
{
- unsigned long flags, pcie_index, pcie_data;
+ unsigned long pcie_index, pcie_data;
unsigned long pcie_index_hi = 0;
void __iomem *pcie_index_offset;
void __iomem *pcie_index_hi_offset;
@@ -1210,7 +1204,7 @@ u64 amdgpu_device_indirect_rreg64_ext(struct amdgpu_device *adev,
if ((reg_addr >> 32) && (adev->nbio.funcs->get_pcie_index_hi_offset))
pcie_index_hi = adev->nbio.funcs->get_pcie_index_hi_offset(adev);
- spin_lock_irqsave(&adev->pcie_idx_lock, flags);
+ guard(spinlock_irqsave)(&adev->pcie_idx_lock);
pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
pcie_data_offset = (void __iomem *)adev->rmmio + pcie_data * 4;
if (pcie_index_hi != 0)
@@ -1240,8 +1234,6 @@ u64 amdgpu_device_indirect_rreg64_ext(struct amdgpu_device *adev,
readl(pcie_index_hi_offset);
}
- spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
-
return r;
}
@@ -1256,14 +1248,14 @@ u64 amdgpu_device_indirect_rreg64_ext(struct amdgpu_device *adev,
void amdgpu_device_indirect_wreg(struct amdgpu_device *adev,
u32 reg_addr, u32 reg_data)
{
- unsigned long flags, pcie_index, pcie_data;
+ unsigned long pcie_index, pcie_data;
void __iomem *pcie_index_offset;
void __iomem *pcie_data_offset;
pcie_index = adev->nbio.funcs->get_pcie_index_offset(adev);
pcie_data = adev->nbio.funcs->get_pcie_data_offset(adev);
- spin_lock_irqsave(&adev->pcie_idx_lock, flags);
+ guard(spinlock_irqsave)(&adev->pcie_idx_lock);
pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
pcie_data_offset = (void __iomem *)adev->rmmio + pcie_data * 4;
@@ -1271,13 +1263,12 @@ void amdgpu_device_indirect_wreg(struct amdgpu_device *adev,
readl(pcie_index_offset);
writel(reg_data, pcie_data_offset);
readl(pcie_data_offset);
- spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
}
void amdgpu_device_indirect_wreg_ext(struct amdgpu_device *adev,
u64 reg_addr, u32 reg_data)
{
- unsigned long flags, pcie_index, pcie_index_hi, pcie_data;
+ unsigned long pcie_index, pcie_index_hi, pcie_data;
void __iomem *pcie_index_offset;
void __iomem *pcie_index_hi_offset;
void __iomem *pcie_data_offset;
@@ -1289,7 +1280,7 @@ void amdgpu_device_indirect_wreg_ext(struct amdgpu_device *adev,
else
pcie_index_hi = 0;
- spin_lock_irqsave(&adev->pcie_idx_lock, flags);
+ guard(spinlock_irqsave)(&adev->pcie_idx_lock);
pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
pcie_data_offset = (void __iomem *)adev->rmmio + pcie_data * 4;
if (pcie_index_hi != 0)
@@ -1311,7 +1302,6 @@ void amdgpu_device_indirect_wreg_ext(struct amdgpu_device *adev,
readl(pcie_index_hi_offset);
}
- spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
}
/**
@@ -1325,14 +1315,14 @@ void amdgpu_device_indirect_wreg_ext(struct amdgpu_device *adev,
void amdgpu_device_indirect_wreg64(struct amdgpu_device *adev,
u32 reg_addr, u64 reg_data)
{
- unsigned long flags, pcie_index, pcie_data;
+ unsigned long pcie_index, pcie_data;
void __iomem *pcie_index_offset;
void __iomem *pcie_data_offset;
pcie_index = adev->nbio.funcs->get_pcie_index_offset(adev);
pcie_data = adev->nbio.funcs->get_pcie_data_offset(adev);
- spin_lock_irqsave(&adev->pcie_idx_lock, flags);
+ guard(spinlock_irqsave)(&adev->pcie_idx_lock);
pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
pcie_data_offset = (void __iomem *)adev->rmmio + pcie_data * 4;
@@ -1346,13 +1336,12 @@ void amdgpu_device_indirect_wreg64(struct amdgpu_device *adev,
readl(pcie_index_offset);
writel((u32)(reg_data >> 32), pcie_data_offset);
readl(pcie_data_offset);
- spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
}
void amdgpu_device_indirect_wreg64_ext(struct amdgpu_device *adev,
u64 reg_addr, u64 reg_data)
{
- unsigned long flags, pcie_index, pcie_data;
+ unsigned long pcie_index, pcie_data;
unsigned long pcie_index_hi = 0;
void __iomem *pcie_index_offset;
void __iomem *pcie_index_hi_offset;
@@ -1363,7 +1352,7 @@ void amdgpu_device_indirect_wreg64_ext(struct amdgpu_device *adev,
if ((reg_addr >> 32) && (adev->nbio.funcs->get_pcie_index_hi_offset))
pcie_index_hi = adev->nbio.funcs->get_pcie_index_hi_offset(adev);
- spin_lock_irqsave(&adev->pcie_idx_lock, flags);
+ guard(spinlock_irqsave)(&adev->pcie_idx_lock);
pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
pcie_data_offset = (void __iomem *)adev->rmmio + pcie_data * 4;
if (pcie_index_hi != 0)
@@ -1395,7 +1384,6 @@ void amdgpu_device_indirect_wreg64_ext(struct amdgpu_device *adev,
readl(pcie_index_hi_offset);
}
- spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
}
/**
@@ -1764,17 +1752,15 @@ static int amdgpu_device_wb_init(struct amdgpu_device *adev)
*/
int amdgpu_device_wb_get(struct amdgpu_device *adev, u32 *wb)
{
- unsigned long flags, offset;
+ unsigned long offset;
- spin_lock_irqsave(&adev->wb.lock, flags);
+ guard(spinlock_irqsave)(&adev->wb.lock);
offset = find_first_zero_bit(adev->wb.used, adev->wb.num_wb);
if (offset < adev->wb.num_wb) {
__set_bit(offset, adev->wb.used);
- spin_unlock_irqrestore(&adev->wb.lock, flags);
*wb = offset << 3; /* convert to dw offset */
return 0;
} else {
- spin_unlock_irqrestore(&adev->wb.lock, flags);
return -EINVAL;
}
}
@@ -1789,13 +1775,11 @@ int amdgpu_device_wb_get(struct amdgpu_device *adev, u32 *wb)
*/
void amdgpu_device_wb_free(struct amdgpu_device *adev, u32 wb)
{
- unsigned long flags;
wb >>= 3;
- spin_lock_irqsave(&adev->wb.lock, flags);
+ guard(spinlock_irqsave)(&adev->wb.lock);
if (wb < adev->wb.num_wb)
__clear_bit(wb, adev->wb.used);
- spin_unlock_irqrestore(&adev->wb.lock, flags);
}
/**
@@ -7379,34 +7363,31 @@ void amdgpu_device_halt(struct amdgpu_device *adev)
u32 amdgpu_device_pcie_port_rreg(struct amdgpu_device *adev,
u32 reg)
{
- unsigned long flags, address, data;
- u32 r;
+ unsigned long address, data;
address = adev->nbio.funcs->get_pcie_port_index_offset(adev);
data = adev->nbio.funcs->get_pcie_port_data_offset(adev);
- spin_lock_irqsave(&adev->pcie_idx_lock, flags);
+ guard(spinlock_irqsave)(&adev->pcie_idx_lock);
WREG32(address, reg * 4);
(void)RREG32(address);
- r = RREG32(data);
- spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
- return r;
+
+ return RREG32(data);
}
void amdgpu_device_pcie_port_wreg(struct amdgpu_device *adev,
u32 reg, u32 v)
{
- unsigned long flags, address, data;
+ unsigned long address, data;
address = adev->nbio.funcs->get_pcie_port_index_offset(adev);
data = adev->nbio.funcs->get_pcie_port_data_offset(adev);
- spin_lock_irqsave(&adev->pcie_idx_lock, flags);
+ guard(spinlock_irqsave)(&adev->pcie_idx_lock);
WREG32(address, reg * 4);
(void)RREG32(address);
WREG32(data, v);
(void)RREG32(data);
- spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
}
/**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 1054d66c54fa..8eb2c917d622 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -664,9 +664,8 @@ void amdgpu_fence_driver_hw_init(struct amdgpu_device *adev)
void amdgpu_fence_driver_set_error(struct amdgpu_ring *ring, int error)
{
struct amdgpu_fence_driver *drv = &ring->fence_drv;
- unsigned long flags;
- spin_lock_irqsave(&drv->lock, flags);
+ guard(spinlock_irqsave)(&drv->lock);
for (unsigned int i = 0; i <= drv->num_fences_mask; ++i) {
struct dma_fence *fence;
@@ -675,7 +674,6 @@ void amdgpu_fence_driver_set_error(struct amdgpu_ring *ring, int error)
if (fence && !dma_fence_is_signaled_locked(fence))
dma_fence_set_error(fence, error);
}
- spin_unlock_irqrestore(&drv->lock, flags);
}
/**
@@ -719,7 +717,6 @@ void amdgpu_fence_driver_update_timedout_fence_state(struct amdgpu_fence *af)
struct dma_fence __rcu **ptr;
struct amdgpu_fence *fence;
struct amdgpu_ring *ring = af->ring;
- unsigned long flags;
u32 seq, last_seq;
bool reemitted = false;
@@ -727,7 +724,7 @@ void amdgpu_fence_driver_update_timedout_fence_state(struct amdgpu_fence *af)
seq = ring->fence_drv.sync_seq & ring->fence_drv.num_fences_mask;
/* mark all fences from the guilty context with an error */
- spin_lock_irqsave(&ring->fence_drv.lock, flags);
+ guard(spinlock_irqsave)(&ring->fence_drv.lock);
do {
last_seq++;
last_seq &= ring->fence_drv.num_fences_mask;
@@ -748,7 +745,6 @@ void amdgpu_fence_driver_update_timedout_fence_state(struct amdgpu_fence *af)
}
rcu_read_unlock();
} while (last_seq != seq);
- spin_unlock_irqrestore(&ring->fence_drv.lock, flags);
if (reemitted) {
/* if we've already reemitted once then just cancel everything */
--
2.25.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: Use guard() for spin locks
2026-03-11 9:13 [PATCH 2/2] drm/amdgpu: Use guard() for spin locks renpanpan
@ 2026-03-11 9:18 ` Christian König
0 siblings, 0 replies; 2+ messages in thread
From: Christian König @ 2026-03-11 9:18 UTC (permalink / raw)
To: renpanpan, linux-kernel; +Cc: alexander.deucher, airlied
On 3/11/26 10:13, renpanpan wrote:
>
> Clean up the code using guard() for spin locks.
>
> Merely code refactoring, and no behavior change.
Well you clearly have unintended behavior changes in this patch. So absolutely clear NAK to that!
>
> Signed-off-by: renpanpan <renpanpan@kylinos.cn>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 71 ++++++++--------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 8 +--
> 2 files changed, 28 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 3e19b51a2763..f9ba1fb36762 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -708,7 +708,6 @@ bool amdgpu_device_supports_smart_shift(struct amdgpu_device *adev)
> void amdgpu_device_mm_access(struct amdgpu_device *adev, loff_t pos,
> void *buf, size_t size, bool write)
> {
> - unsigned long flags;
> uint32_t hi = ~0, tmp = 0;
> uint32_t *data = buf;
> uint64_t last;
> @@ -719,7 +718,7 @@ void amdgpu_device_mm_access(struct amdgpu_device *adev, loff_t pos,
>
> BUG_ON(!IS_ALIGNED(pos, 4) || !IS_ALIGNED(size, 4));
>
> - spin_lock_irqsave(&adev->mmio_idx_lock, flags);
> + guard(spinlock_irqsave)(&adev->mmio_idx_lock);
> for (last = pos + size; pos < last; pos += 4) {
> tmp = pos >> 31;
>
> @@ -734,7 +733,6 @@ void amdgpu_device_mm_access(struct amdgpu_device *adev, loff_t pos,
> *data++ = RREG32_NO_KIQ(mmMM_DATA);
> }
>
> - spin_unlock_irqrestore(&adev->mmio_idx_lock, flags);
> drm_dev_exit(idx);
This suddenly holds the spinlock while calling drm_dev_exit() because the goard is still in scope.
Same thing on a bunch of other places in the rest of the patch.
Regards,
Christian
> }
>
> @@ -1087,7 +1085,7 @@ void amdgpu_device_xcc_wreg(struct amdgpu_device *adev,
> u32 amdgpu_device_indirect_rreg(struct amdgpu_device *adev,
> u32 reg_addr)
> {
> - unsigned long flags, pcie_index, pcie_data;
> + unsigned long pcie_index, pcie_data;
> void __iomem *pcie_index_offset;
> void __iomem *pcie_data_offset;
> u32 r;
> @@ -1095,14 +1093,13 @@ u32 amdgpu_device_indirect_rreg(struct amdgpu_device *adev,
> pcie_index = adev->nbio.funcs->get_pcie_index_offset(adev);
> pcie_data = adev->nbio.funcs->get_pcie_data_offset(adev);
>
> - spin_lock_irqsave(&adev->pcie_idx_lock, flags);
> + guard(spinlock_irqsave)(&adev->pcie_idx_lock);
> pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
> pcie_data_offset = (void __iomem *)adev->rmmio + pcie_data * 4;
>
> writel(reg_addr, pcie_index_offset);
> readl(pcie_index_offset);
> r = readl(pcie_data_offset);
> - spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
>
> return r;
> }
> @@ -1110,7 +1107,7 @@ u32 amdgpu_device_indirect_rreg(struct amdgpu_device *adev,
> u32 amdgpu_device_indirect_rreg_ext(struct amdgpu_device *adev,
> u64 reg_addr)
> {
> - unsigned long flags, pcie_index, pcie_index_hi, pcie_data;
> + unsigned long pcie_index, pcie_index_hi, pcie_data;
> u32 r;
> void __iomem *pcie_index_offset;
> void __iomem *pcie_index_hi_offset;
> @@ -1133,7 +1130,7 @@ u32 amdgpu_device_indirect_rreg_ext(struct amdgpu_device *adev,
> pcie_index_hi = 0;
> }
>
> - spin_lock_irqsave(&adev->pcie_idx_lock, flags);
> + guard(spinlock_irqsave)(&adev->pcie_idx_lock);
> pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
> pcie_data_offset = (void __iomem *)adev->rmmio + pcie_data * 4;
> if (pcie_index_hi != 0)
> @@ -1154,8 +1151,6 @@ u32 amdgpu_device_indirect_rreg_ext(struct amdgpu_device *adev,
> readl(pcie_index_hi_offset);
> }
>
> - spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
> -
> return r;
> }
>
> @@ -1170,7 +1165,7 @@ u32 amdgpu_device_indirect_rreg_ext(struct amdgpu_device *adev,
> u64 amdgpu_device_indirect_rreg64(struct amdgpu_device *adev,
> u32 reg_addr)
> {
> - unsigned long flags, pcie_index, pcie_data;
> + unsigned long pcie_index, pcie_data;
> void __iomem *pcie_index_offset;
> void __iomem *pcie_data_offset;
> u64 r;
> @@ -1178,7 +1173,7 @@ u64 amdgpu_device_indirect_rreg64(struct amdgpu_device *adev,
> pcie_index = adev->nbio.funcs->get_pcie_index_offset(adev);
> pcie_data = adev->nbio.funcs->get_pcie_data_offset(adev);
>
> - spin_lock_irqsave(&adev->pcie_idx_lock, flags);
> + guard(spinlock_irqsave)(&adev->pcie_idx_lock);
> pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
> pcie_data_offset = (void __iomem *)adev->rmmio + pcie_data * 4;
>
> @@ -1190,7 +1185,6 @@ u64 amdgpu_device_indirect_rreg64(struct amdgpu_device *adev,
> writel(reg_addr + 4, pcie_index_offset);
> readl(pcie_index_offset);
> r |= ((u64)readl(pcie_data_offset) << 32);
> - spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
>
> return r;
> }
> @@ -1198,7 +1192,7 @@ u64 amdgpu_device_indirect_rreg64(struct amdgpu_device *adev,
> u64 amdgpu_device_indirect_rreg64_ext(struct amdgpu_device *adev,
> u64 reg_addr)
> {
> - unsigned long flags, pcie_index, pcie_data;
> + unsigned long pcie_index, pcie_data;
> unsigned long pcie_index_hi = 0;
> void __iomem *pcie_index_offset;
> void __iomem *pcie_index_hi_offset;
> @@ -1210,7 +1204,7 @@ u64 amdgpu_device_indirect_rreg64_ext(struct amdgpu_device *adev,
> if ((reg_addr >> 32) && (adev->nbio.funcs->get_pcie_index_hi_offset))
> pcie_index_hi = adev->nbio.funcs->get_pcie_index_hi_offset(adev);
>
> - spin_lock_irqsave(&adev->pcie_idx_lock, flags);
> + guard(spinlock_irqsave)(&adev->pcie_idx_lock);
> pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
> pcie_data_offset = (void __iomem *)adev->rmmio + pcie_data * 4;
> if (pcie_index_hi != 0)
> @@ -1240,8 +1234,6 @@ u64 amdgpu_device_indirect_rreg64_ext(struct amdgpu_device *adev,
> readl(pcie_index_hi_offset);
> }
>
> - spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
> -
> return r;
> }
>
> @@ -1256,14 +1248,14 @@ u64 amdgpu_device_indirect_rreg64_ext(struct amdgpu_device *adev,
> void amdgpu_device_indirect_wreg(struct amdgpu_device *adev,
> u32 reg_addr, u32 reg_data)
> {
> - unsigned long flags, pcie_index, pcie_data;
> + unsigned long pcie_index, pcie_data;
> void __iomem *pcie_index_offset;
> void __iomem *pcie_data_offset;
>
> pcie_index = adev->nbio.funcs->get_pcie_index_offset(adev);
> pcie_data = adev->nbio.funcs->get_pcie_data_offset(adev);
>
> - spin_lock_irqsave(&adev->pcie_idx_lock, flags);
> + guard(spinlock_irqsave)(&adev->pcie_idx_lock);
> pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
> pcie_data_offset = (void __iomem *)adev->rmmio + pcie_data * 4;
>
> @@ -1271,13 +1263,12 @@ void amdgpu_device_indirect_wreg(struct amdgpu_device *adev,
> readl(pcie_index_offset);
> writel(reg_data, pcie_data_offset);
> readl(pcie_data_offset);
> - spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
> }
>
> void amdgpu_device_indirect_wreg_ext(struct amdgpu_device *adev,
> u64 reg_addr, u32 reg_data)
> {
> - unsigned long flags, pcie_index, pcie_index_hi, pcie_data;
> + unsigned long pcie_index, pcie_index_hi, pcie_data;
> void __iomem *pcie_index_offset;
> void __iomem *pcie_index_hi_offset;
> void __iomem *pcie_data_offset;
> @@ -1289,7 +1280,7 @@ void amdgpu_device_indirect_wreg_ext(struct amdgpu_device *adev,
> else
> pcie_index_hi = 0;
>
> - spin_lock_irqsave(&adev->pcie_idx_lock, flags);
> + guard(spinlock_irqsave)(&adev->pcie_idx_lock);
> pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
> pcie_data_offset = (void __iomem *)adev->rmmio + pcie_data * 4;
> if (pcie_index_hi != 0)
> @@ -1311,7 +1302,6 @@ void amdgpu_device_indirect_wreg_ext(struct amdgpu_device *adev,
> readl(pcie_index_hi_offset);
> }
>
> - spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
> }
>
> /**
> @@ -1325,14 +1315,14 @@ void amdgpu_device_indirect_wreg_ext(struct amdgpu_device *adev,
> void amdgpu_device_indirect_wreg64(struct amdgpu_device *adev,
> u32 reg_addr, u64 reg_data)
> {
> - unsigned long flags, pcie_index, pcie_data;
> + unsigned long pcie_index, pcie_data;
> void __iomem *pcie_index_offset;
> void __iomem *pcie_data_offset;
>
> pcie_index = adev->nbio.funcs->get_pcie_index_offset(adev);
> pcie_data = adev->nbio.funcs->get_pcie_data_offset(adev);
>
> - spin_lock_irqsave(&adev->pcie_idx_lock, flags);
> + guard(spinlock_irqsave)(&adev->pcie_idx_lock);
> pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
> pcie_data_offset = (void __iomem *)adev->rmmio + pcie_data * 4;
>
> @@ -1346,13 +1336,12 @@ void amdgpu_device_indirect_wreg64(struct amdgpu_device *adev,
> readl(pcie_index_offset);
> writel((u32)(reg_data >> 32), pcie_data_offset);
> readl(pcie_data_offset);
> - spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
> }
>
> void amdgpu_device_indirect_wreg64_ext(struct amdgpu_device *adev,
> u64 reg_addr, u64 reg_data)
> {
> - unsigned long flags, pcie_index, pcie_data;
> + unsigned long pcie_index, pcie_data;
> unsigned long pcie_index_hi = 0;
> void __iomem *pcie_index_offset;
> void __iomem *pcie_index_hi_offset;
> @@ -1363,7 +1352,7 @@ void amdgpu_device_indirect_wreg64_ext(struct amdgpu_device *adev,
> if ((reg_addr >> 32) && (adev->nbio.funcs->get_pcie_index_hi_offset))
> pcie_index_hi = adev->nbio.funcs->get_pcie_index_hi_offset(adev);
>
> - spin_lock_irqsave(&adev->pcie_idx_lock, flags);
> + guard(spinlock_irqsave)(&adev->pcie_idx_lock);
> pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
> pcie_data_offset = (void __iomem *)adev->rmmio + pcie_data * 4;
> if (pcie_index_hi != 0)
> @@ -1395,7 +1384,6 @@ void amdgpu_device_indirect_wreg64_ext(struct amdgpu_device *adev,
> readl(pcie_index_hi_offset);
> }
>
> - spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
> }
>
> /**
> @@ -1764,17 +1752,15 @@ static int amdgpu_device_wb_init(struct amdgpu_device *adev)
> */
> int amdgpu_device_wb_get(struct amdgpu_device *adev, u32 *wb)
> {
> - unsigned long flags, offset;
> + unsigned long offset;
>
> - spin_lock_irqsave(&adev->wb.lock, flags);
> + guard(spinlock_irqsave)(&adev->wb.lock);
> offset = find_first_zero_bit(adev->wb.used, adev->wb.num_wb);
> if (offset < adev->wb.num_wb) {
> __set_bit(offset, adev->wb.used);
> - spin_unlock_irqrestore(&adev->wb.lock, flags);
> *wb = offset << 3; /* convert to dw offset */
> return 0;
> } else {
> - spin_unlock_irqrestore(&adev->wb.lock, flags);
> return -EINVAL;
> }
> }
> @@ -1789,13 +1775,11 @@ int amdgpu_device_wb_get(struct amdgpu_device *adev, u32 *wb)
> */
> void amdgpu_device_wb_free(struct amdgpu_device *adev, u32 wb)
> {
> - unsigned long flags;
>
> wb >>= 3;
> - spin_lock_irqsave(&adev->wb.lock, flags);
> + guard(spinlock_irqsave)(&adev->wb.lock);
> if (wb < adev->wb.num_wb)
> __clear_bit(wb, adev->wb.used);
> - spin_unlock_irqrestore(&adev->wb.lock, flags);
> }
>
> /**
> @@ -7379,34 +7363,31 @@ void amdgpu_device_halt(struct amdgpu_device *adev)
> u32 amdgpu_device_pcie_port_rreg(struct amdgpu_device *adev,
> u32 reg)
> {
> - unsigned long flags, address, data;
> - u32 r;
> + unsigned long address, data;
>
> address = adev->nbio.funcs->get_pcie_port_index_offset(adev);
> data = adev->nbio.funcs->get_pcie_port_data_offset(adev);
>
> - spin_lock_irqsave(&adev->pcie_idx_lock, flags);
> + guard(spinlock_irqsave)(&adev->pcie_idx_lock);
> WREG32(address, reg * 4);
> (void)RREG32(address);
> - r = RREG32(data);
> - spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
> - return r;
> +
> + return RREG32(data);
> }
>
> void amdgpu_device_pcie_port_wreg(struct amdgpu_device *adev,
> u32 reg, u32 v)
> {
> - unsigned long flags, address, data;
> + unsigned long address, data;
>
> address = adev->nbio.funcs->get_pcie_port_index_offset(adev);
> data = adev->nbio.funcs->get_pcie_port_data_offset(adev);
>
> - spin_lock_irqsave(&adev->pcie_idx_lock, flags);
> + guard(spinlock_irqsave)(&adev->pcie_idx_lock);
> WREG32(address, reg * 4);
> (void)RREG32(address);
> WREG32(data, v);
> (void)RREG32(data);
> - spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
> }
>
> /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> index 1054d66c54fa..8eb2c917d622 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> @@ -664,9 +664,8 @@ void amdgpu_fence_driver_hw_init(struct amdgpu_device *adev)
> void amdgpu_fence_driver_set_error(struct amdgpu_ring *ring, int error)
> {
> struct amdgpu_fence_driver *drv = &ring->fence_drv;
> - unsigned long flags;
>
> - spin_lock_irqsave(&drv->lock, flags);
> + guard(spinlock_irqsave)(&drv->lock);
> for (unsigned int i = 0; i <= drv->num_fences_mask; ++i) {
> struct dma_fence *fence;
>
> @@ -675,7 +674,6 @@ void amdgpu_fence_driver_set_error(struct amdgpu_ring *ring, int error)
> if (fence && !dma_fence_is_signaled_locked(fence))
> dma_fence_set_error(fence, error);
> }
> - spin_unlock_irqrestore(&drv->lock, flags);
> }
>
> /**
> @@ -719,7 +717,6 @@ void amdgpu_fence_driver_update_timedout_fence_state(struct amdgpu_fence *af)
> struct dma_fence __rcu **ptr;
> struct amdgpu_fence *fence;
> struct amdgpu_ring *ring = af->ring;
> - unsigned long flags;
> u32 seq, last_seq;
> bool reemitted = false;
>
> @@ -727,7 +724,7 @@ void amdgpu_fence_driver_update_timedout_fence_state(struct amdgpu_fence *af)
> seq = ring->fence_drv.sync_seq & ring->fence_drv.num_fences_mask;
>
> /* mark all fences from the guilty context with an error */
> - spin_lock_irqsave(&ring->fence_drv.lock, flags);
> + guard(spinlock_irqsave)(&ring->fence_drv.lock);
> do {
> last_seq++;
> last_seq &= ring->fence_drv.num_fences_mask;
> @@ -748,7 +745,6 @@ void amdgpu_fence_driver_update_timedout_fence_state(struct amdgpu_fence *af)
> }
> rcu_read_unlock();
> } while (last_seq != seq);
> - spin_unlock_irqrestore(&ring->fence_drv.lock, flags);
>
> if (reemitted) {
> /* if we've already reemitted once then just cancel everything */
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-03-11 9:19 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-11 9:13 [PATCH 2/2] drm/amdgpu: Use guard() for spin locks renpanpan
2026-03-11 9:18 ` Christian König
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®