Re: [RFC 01/10] drm/amdgpu: implement PMC ioctl framework

["Zhu, Lingshan" <lingshan.zhu@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 11472 bytes --]

On 9/24/2026 11:49 PM, James Zhu wrote:

>
> On 2026-09-23 23:38, Zhu, Lingshan wrote:
>> On 9/23/2026 10:17 PM, James Zhu wrote:
>>>
>>> On 2026-09-22 05:34, Zhu Lingshan wrote:
>>>> This commit implements basic ioctl framework for PMC,
>>>> including basic operations.
>>>>
>>>> This commit also implements AMDGPU_PROFILER_OP_VERSION
>>>> which report profiler version to the user space.
>>>>
>>>> Signed-off-by: Zhu Lingshan<lingshan.zhu@amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/Makefile          |  2 +-
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c      |  2 +
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_profiler.c | 44
>>>> ++++++++++++++++++++
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_profiler.h | 33 +++++++++++++++
>>>>   include/uapi/drm/amdgpu_drm.h                | 36 ++++++++++++++++
>>>>   5 files changed, 116 insertions(+), 1 deletion(-)
>>>>   create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_profiler.c
>>>>   create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_profiler.h
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile
>>>> b/drivers/gpu/drm/amd/amdgpu/Makefile
>>>> index 6fb0a913e33b..d7db69e9d6a0 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
>>>> @@ -72,7 +72,7 @@ amdgpu-y += amdgpu_device.o amdgpu_reg_access.o
>>>> amdgpu_doorbell_mgr.o amdgpu_kms
>>>>       amdgpu_eeprom.o amdgpu_mca.o amdgpu_psp_ta.o amdgpu_lsdma.o
>>>> amdgpu_lockdep.o \
>>>>       amdgpu_ring_mux.o amdgpu_xcp.o amdgpu_seq64.o
>>>> amdgpu_dev_coredump.o \
>>>>       amdgpu_cper.o amdgpu_userq_fence.o amdgpu_eviction_fence.o
>>>> amdgpu_ip.o \
>>>> -    amdgpu_wb.o amdgpu_cwsr.o amdgpu_events.o
>>>> +    amdgpu_wb.o amdgpu_cwsr.o amdgpu_events.o amdgpu_profiler.o
>>>>     amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
>>>>   diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> index 7c473f60bd28..a98c4a47e196 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> @@ -55,6 +55,7 @@
>>>>   #include "amdgpu_cwsr.h"
>>>>   #include "amdgpu_userq_fence.h"
>>>>   #include "../amdxcp/amdgpu_xcp_drv.h"
>>>> +#include "amdgpu_profiler.h"
>>>>     /*
>>>>    * KMS wrapper.
>>>> @@ -3096,6 +3097,7 @@ const struct drm_ioctl_desc
>>>> amdgpu_ioctls_kms[] = {
>>>>       DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_WAIT, amdgpu_userq_wait_ioctl,
>>>> DRM_AUTH|DRM_RENDER_ALLOW),
>>>>       DRM_IOCTL_DEF_DRV(AMDGPU_GEM_LIST_HANDLES,
>>>> amdgpu_gem_list_handles_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
>>>>       DRM_IOCTL_DEF_DRV(AMDGPU_PROC_OPTIONS,
>>>> amdgpu_proc_options_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
>>>> +    DRM_IOCTL_DEF_DRV(AMDGPU_PROFILER, amdgpu_profiler_ioctl,
>>>> DRM_AUTH|DRM_RENDER_ALLOW),
>>>>   };
>>>>     static const struct drm_driver amdgpu_kms_driver = {
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_profiler.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_profiler.c
>>>> new file mode 100644
>>>> index 000000000000..04902696adab
>>>> --- /dev/null
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_profiler.c
>>>> @@ -0,0 +1,44 @@
>>>> +// SPDX-License-Identifier: 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 <linux/errno.h>
>>>> +#include <drm/amdgpu_drm.h>
>>>> +#include "amdgpu_profiler.h"
>>>> +
>>>> +int amdgpu_profiler_ioctl(struct drm_device *dev, void *data,
>>>> struct drm_file *filp)
>>>> +{
>>>> +    struct drm_amdgpu_profiler_args *args = data;
>>>> +
>>>> +    switch (args->op) {
>>>> +    case AMDGPU_PROFILER_OP_VERSION:
>>>> +        args->version = AMDGPU_PROFILER_VERSION_NUM;
>>>> +        return 0;
>>>> +    case AMDGPU_PROFILER_OP_RESERVE:
>>>> +        return -EOPNOTSUPP;
>>>> +    case AMDGPU_PROFILER_OP_PMC:
>>>> +        return -EOPNOTSUPP;
>>>> +    default:
>>>> +        return -EINVAL;
>>>> +    }
>>>> +}
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_profiler.h
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_profiler.h
>>>> new file mode 100644
>>>> index 000000000000..c27b5f9b3ff0
>>>> --- /dev/null
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_profiler.h
>>>> @@ -0,0 +1,33 @@
>>>> +/* SPDX-License-Identifier: 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_PROFILER_H__
>>>> +#define __AMDGPU_PROFILER_H__
>>>> +
>>>> +#include <drm/drm_device.h>
>>>> +#include <drm/drm_file.h>
>>>> +
>>>> +int amdgpu_profiler_ioctl(struct drm_device *dev, void *data,
>>>> struct drm_file *filp);
>>>> +
>>>> +#endif /* __AMDGPU_PROFILER_H__ */
>>>> diff --git a/include/uapi/drm/amdgpu_drm.h
>>>> b/include/uapi/drm/amdgpu_drm.h
>>>> index 9222be9a6d2a..acc63804ba8c 100644
>>>> --- a/include/uapi/drm/amdgpu_drm.h
>>>> +++ b/include/uapi/drm/amdgpu_drm.h
>>>> @@ -60,6 +60,7 @@ extern "C" {
>>>>   #define DRM_AMDGPU_GEM_LIST_HANDLES    0x19
>>>>   #define DRM_AMDGPU_PROC_OPTIONS        0x1A
>>>>   #define DRM_AMDGPU_CWSR            0x1B
>>>> +#define DRM_AMDGPU_PROFILER        0x1C
>>>>     #define DRM_IOCTL_AMDGPU_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE +
>>>> DRM_AMDGPU_GEM_CREATE, union drm_amdgpu_gem_create)
>>>>   #define DRM_IOCTL_AMDGPU_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE +
>>>> DRM_AMDGPU_GEM_MMAP, union drm_amdgpu_gem_mmap)
>>>> @@ -83,6 +84,7 @@ extern "C" {
>>>>   #define DRM_IOCTL_AMDGPU_GEM_LIST_HANDLES
>>>> DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_LIST_HANDLES, struct
>>>> drm_amdgpu_gem_list_handles)
>>>>   #define DRM_IOCTL_AMDGPU_PROC_OPTIONS DRM_IOWR(DRM_COMMAND_BASE +
>>>> DRM_AMDGPU_PROC_OPTIONS, struct drm_amdgpu_proc_options)
>>>>   #define DRM_IOCTL_AMDGPU_CWSR    DRM_IOWR(DRM_COMMAND_BASE +
>>>> DRM_AMDGPU_CWSR, union drm_amdgpu_cwsr)
>>>> +#define DRM_IOCTL_AMDGPU_PROFILER DRM_IOWR(DRM_COMMAND_BASE +
>>>> DRM_AMDGPU_PROFILER, struct drm_amdgpu_profiler_args)
>>>>     /**
>>>>    * DOC: memory domains
>>>> @@ -1809,6 +1811,40 @@ struct drm_amdgpu_proc_options {
>>>>       } kfd_sigbus_delay;
>>>>   };
>>>>   +#define AMDGPU_PROFILER_VERSION_NUM    1
>>>> +
>>>> +enum amdgpu_profiler_ops {
>>>> +    AMDGPU_PROFILER_OP_VERSION = 1,
>>>> +    AMDGPU_PROFILER_OP_RESERVE = 2,
>>> [JZ] the implementation tells that it is
>>> AMDGPU_PROFILER_OP_PMC_RESERVE, it is better to put it under new PMC
>>> sub-ioctl op
>> Do you mean implement it like {bool pmc_enable, bool reserve} in a
>> struct? I am not sure it is a good idea
>> and this reservation design is a improvement. Because it separates
>> the reserve(lock) and unreserve(unlock),
>> so we don't need to keep the reservation during the PMC operations in
>> a complex way, and we won't meet
>> the problems like how to handle the situation where the user tries to
>> unlock but keeps enable = 1.
> [JZ] profiler includes PMC, PC sampling, SPM etc. do the other feature
> need reservation also?

Hello James,

I think we should implement reservations, but in different ways:
1) if it is a device scope profiler, then we should implement a reservation mechanism like this one.
2) if the profiler facility allows running multiple type of profiler concurrently(e.g, PMC and PC together),
and some of the profiler may collect correlate data, then we need reservation because we don't want a profiler to
sniff another.
3) if it is a dri node or any sub function profiler, then we should implement a proper reservation in that level.

I believe we need a device level reservation for PMC.

Thanks
Lingshan

>>
>> Thanks
>> Lingshan
>>>> +    AMDGPU_PROFILER_OP_PMC = 3,
>>>> +};
>>>> +
>>>> +enum amdgpu_profiler_reserve_ops {
>>>> +    /* release the device resertaion, only the reservation owner
>>>> do this */
>>>> +    AMDGPU_PROFILER_RELEASE = 0,
>>>> +    /* reserve the device profiler */
>>>> +    AMDGPU_PROFILER_RESERVE = 1,
>>>> +    /* Clear the device profiler reservation, in case the reservation
>>>> +     * owner can no longer release the reservation(e.g., it crashes),
>>>> +     * This operation does not take the reseration nor touch
>>>> profielr state
>>>> +     */
>>>> +    AMDGPU_PROFILER_CLEAR_RESERVATION = 2,
>>>> +};
>>>> +
>>>> +struct drm_amdgpu_profiler_args {
>>>> +    /* enum amdgpu_profiler_ops */
>>>> +    __u8 op;
>>>> +    __u8 pad[3];
>>>> +    union {
>>>> +        /* AMDGPU_PROFILER_OP_RESERVE: enum
>>>> amdgpu_profiler_reserve_ops */
>>>> +        __u8 reserve;
>>>> +        /* AMDGPU_PROFILER_OP_PMC: 1 enable PMC, 0 disable PMC */
>>>> +        __u8 perfcount_enable;
>>>> +        /* AMDGPU_PROFILER_OP_VERSION: AMDGPU_PROFILER_VERSION_NUM */
>>>> +        __u32 version;
>>>> +    };
>>>> +};
>>>> +
>>>>   #if defined(__cplusplus)
>>>>   }
>>>>   #endif

[-- Attachment #2: Type: text/html, Size: 18947 bytes --]