On 6/8/26 21:47, Shahyan Soltani wrote:
> Move struct amdgpu_wb and helpers out of the monolithic header amdgpu.h
> into its own dedicated header amdgpu_wb.h.
>
> Move functions amdgpu_device_wb_get() and amdgpu_device_wb_free out of
> amdgpu_device.c into new dedicated amdgpu_wb.c file.
>
> Update amdgpu/Makefile to build amdgpu_wb.o.
>
> This is part of the ongoing effort to reduce the size of amdgpu.h into
> their own respective separate headers.
>
> Signed-off-by: Shahyan Soltani <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/Makefile | 3 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 68 +-------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 45 ----------
> drivers/gpu/drm/amd/amdgpu/amdgpu_wb.c | 69 ++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_wb.h | 100 +++++++++++++++++++++
> 5 files changed, 172 insertions(+), 113 deletions(-)
> create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_wb.c
> create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_wb.h
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile
> b/drivers/gpu/drm/amd/amdgpu/Makefile
> index ee3574797bc2..e6deb24f73bb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -71,7 +71,8 @@ amdgpu-y += amdgpu_device.o amdgpu_reg_access.o
> amdgpu_doorbell_mgr.o amdgpu_kms
> amdgpu_fw_attestation.o amdgpu_securedisplay.o \
> amdgpu_eeprom.o amdgpu_mca.o amdgpu_psp_ta.o amdgpu_lsdma.o \
> 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_cper.o amdgpu_userq_fence.o amdgpu_eviction_fence.o amdgpu_ip.o \
> + amdgpu_wb.o
>
> amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index d7d8664854fd..9a714b4b59bc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -112,6 +112,7 @@
> #include "amdgpu_userq.h"
> #include "amdgpu_eviction_fence.h"
> #include "amdgpu_sa.h"
> +#include "amdgpu_wb.h"
> #include "amdgpu_ip.h"
> #if defined(CONFIG_DRM_AMD_ISP)
> #include "amdgpu_isp.h"
> @@ -429,73 +430,6 @@ struct amdgpu_fpriv {
>
> int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv);
>
> -/*
> - * Writeback
> - */
> -#define AMDGPU_MAX_WB 1024 /* Reserve at most 1024 WB slots for
> amdgpu-owned rings. */
> -
> -/**
> - * struct amdgpu_wb - This struct is used for small GPU memory allocation.
> - *
> - * This struct is used to allocate a small amount of GPU memory that can be
> - * used to shadow certain states into the memory. This is especially useful
> for
> - * providing easy CPU access to some states without requiring register access
> - * (e.g., if some block is power gated, reading register may be problematic).
> - *
> - * Note: the term writeback was initially used because many of the amdgpu
> - * components had some level of writeback memory, and this struct initially
> - * described those components.
> - */
> -struct amdgpu_wb {
> -
> - /**
> - * @wb_obj:
> - *
> - * Buffer Object used for the writeback memory.
> - */
> - struct amdgpu_bo *wb_obj;
> -
> - /**
> - * @wb:
> - *
> - * Pointer to the first writeback slot. In terms of CPU address
> - * this value can be accessed directly by using the offset as an index.
> - * For the GPU address, it is necessary to use gpu_addr and the offset.
> - */
> - uint32_t *wb;
> -
> - /**
> - * @gpu_addr:
> - *
> - * Writeback base address in the GPU.
> - */
> - uint64_t gpu_addr;
> -
> - /**
> - * @num_wb:
> - *
> - * Number of writeback slots reserved for amdgpu.
> - */
> - u32 num_wb;
> -
> - /**
> - * @used:
> - *
> - * Track the writeback slot already used.
> - */
> - unsigned long used[DIV_ROUND_UP(AMDGPU_MAX_WB,
> BITS_PER_LONG)];
> -
> - /**
> - * @lock:
> - *
> - * Protects read and write of the used field array.
> - */
> - spinlock_t lock;
> -};
> -
> -int amdgpu_device_wb_get(struct amdgpu_device *adev, u32 *wb);
> -void amdgpu_device_wb_free(struct amdgpu_device *adev, u32 wb);
> -
> /*
> * Benchmarking
> */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 5ff224163bab..15a6a9010fc8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1058,51 +1058,6 @@ static int amdgpu_device_wb_init(struct amdgpu_device
> *adev)
> return 0;
> }
>
> -/**
> - * amdgpu_device_wb_get - Allocate a wb entry
> - *
> - * @adev: amdgpu_device pointer
> - * @wb: wb index
> - *
> - * Allocate a wb slot for use by the driver (all asics).
> - * Returns 0 on success or -EINVAL on failure.
> - */
> -int amdgpu_device_wb_get(struct amdgpu_device *adev, u32 *wb)
> -{
> - unsigned long flags, offset;
> -
> - spin_lock_irqsave(&adev->wb.lock, flags);
> - 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;
> - }
> -}
> -
> -/**
> - * amdgpu_device_wb_free - Free a wb entry
> - *
> - * @adev: amdgpu_device pointer
> - * @wb: wb index
> - *
> - * Free a wb slot allocated for use by the driver (all asics)
> - */
> -void amdgpu_device_wb_free(struct amdgpu_device *adev, u32 wb)
> -{
> - unsigned long flags;
> -
> - wb >>= 3;
> - spin_lock_irqsave(&adev->wb.lock, flags);
> - if (wb < adev->wb.num_wb)
> - __clear_bit(wb, adev->wb.used);
> - spin_unlock_irqrestore(&adev->wb.lock, flags);
> -}
> -
> /**
> * amdgpu_device_resize_fb_bar - try to resize FB BAR
> *
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_wb.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_wb.c
> new file mode 100644
> index 000000000000..8e5f572077f3
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_wb.c
> @@ -0,0 +1,69 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Copyright 2026 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +#include "amdgpu.h"
> +#include "amdgpu_wb.h"
> +#include <linux/spinlock.h>
> +/**
> + * amdgpu_device_wb_get - Allocate a wb entry
> + *
> + * @adev: amdgpu_device pointer
> + * @wb: wb index
> + *
> + * Allocate a wb slot for use by the driver (all asics).
> + * Returns 0 on success or -EINVAL on failure.
> + */
> +int amdgpu_device_wb_get(struct amdgpu_device *adev, u32 *wb)
Drop the _device_ part from the function name now that this is a complete
separate component.
You might also want to change the parameter from struct amdgpu_device to struct
amdgpu_wb, not a must have but I think it would be cleaner.
Apart from those nit picks looks good to me.
Regards,
Christian.
> +{
> + unsigned long flags, offset;
> +
> + spin_lock_irqsave(&adev->wb.lock, flags);
> + 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;
> + }
> +}
> +
> +/**
> + * amdgpu_device_wb_free - Free a wb entry
> + *
> + * @adev: amdgpu_device pointer
> + * @wb: wb index
> + *
> + * Free a wb slot allocated for use by the driver (all asics)
> + */
> +void amdgpu_device_wb_free(struct amdgpu_device *adev, u32 wb)
> +{
> + unsigned long flags;
> +
> + wb >>= 3;
> + spin_lock_irqsave(&adev->wb.lock, flags);
> + if (wb < adev->wb.num_wb)
> + __clear_bit(wb, adev->wb.used);
> + spin_unlock_irqrestore(&adev->wb.lock, flags);
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_wb.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_wb.h
> new file mode 100644
> index 000000000000..dac9fc3f0004
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_wb.h
> @@ -0,0 +1,100 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR MIT
> + *
> + * Copyright 2026 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +#ifndef __AMDGPU_WB_H__
> +#define __AMDGPU_WB_H__
> +
> +#include <linux/types.h>
> +#include <linux/spinlock_types.h>
> +#include <linux/math.h>
> +
> +/*
> + * Writeback
> + */
> +#define AMDGPU_MAX_WB 1024 /* Reserve at most 1024 WB slots for
> amdgpu-owned rings. */
> +
> +/**
> + * struct amdgpu_wb - This struct is used for small GPU memory allocation.
> + *
> + * This struct is used to allocate a small amount of GPU memory that can be
> + * used to shadow certain states into the memory. This is especially useful
> for
> + * providing easy CPU access to some states without requiring register access
> + * (e.g., if some block is power gated, reading register may be problematic).
> + *
> + * Note: the term writeback was initially used because many of the amdgpu
> + * components had some level of writeback memory, and this struct initially
> + * described those components.
> + */
> +
> +struct amdgpu_bo;
> +struct amdgpu_device;
> +
> +struct amdgpu_wb {
> +
> + /**
> + * @wb_obj:
> + *
> + * Buffer Object used for the writeback memory.
> + */
> + struct amdgpu_bo *wb_obj;
> +
> + /**
> + * @wb:
> + *
> + * Pointer to the first writeback slot. In terms of CPU address
> + * this value can be accessed directly by using the offset as an index.
> + * For the GPU address, it is necessary to use gpu_addr and the offset.
> + */
> + uint32_t *wb;
> +
> + /**
> + * @gpu_addr:
> + *
> + * Writeback base address in the GPU.
> + */
> + uint64_t gpu_addr;
> +
> + /**
> + * @num_wb:
> + *
> + * Number of writeback slots reserved for amdgpu.
> + */
> + u32 num_wb;
> +
> + /**
> + * @used:
> + *
> + * Track the writeback slot already used.
> + */
> + unsigned long used[DIV_ROUND_UP(AMDGPU_MAX_WB,
> BITS_PER_LONG)];
> +
> + /**
> + * @lock:
> + *
> + * Protects read and write of the used field array.
> + */
> + spinlock_t lock;
> +};
> +
> +int amdgpu_device_wb_get(struct amdgpu_device *adev, u32 *wb);
> +void amdgpu_device_wb_free(struct amdgpu_device *adev, u32 wb);
> +#endif
