* [PATCH v3 1/6] drm/amdgpu: Implement a new 64bit sequence memory driver
2023-04-20 14:47 [PATCH v3 0/6] Usermode queue fencing synchronization Arunpravin Paneer Selvam
@ 2023-04-20 14:47 ` Arunpravin Paneer Selvam
2023-04-21 12:24 ` Christian König
2023-04-20 14:47 ` [PATCH v3 2/6] drm/amdgpu: Implement a new userqueue fence driver Arunpravin Paneer Selvam
` (4 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Arunpravin Paneer Selvam @ 2023-04-20 14:47 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, christian.koenig, Arunpravin Paneer Selvam
Developed a new driver which allocates a 64bit memory on
each request in sequence order. At the moment, user queue
fence memory is the main consumer of this seq64 driver.
v2: Worked on review comments from Christian for the following
modifications
- Move driver name from "semaphore" to "seq64"
- Remove unnecessary PT/PD mapping
- Move enable_mes check into init/fini functions.
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
drivers/gpu/drm/amd/amdgpu/Makefile | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 5 +
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 +
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 13 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c | 158 +++++++++++++++++++++
drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h | 48 +++++++
6 files changed, 232 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c
create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h
diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index 2cc7897de7e6..d39504e65db1 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -59,7 +59,7 @@ amdgpu-y += amdgpu_device.o amdgpu_doorbell_mgr.o amdgpu_kms.o \
amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
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_ring_mux.o amdgpu_seq64.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 c5f9af0e74ee..3bc8a2d35bb3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -109,6 +109,7 @@
#include "amdgpu_fdinfo.h"
#include "amdgpu_mca.h"
#include "amdgpu_ras.h"
+#include "amdgpu_seq64.h"
#define MAX_GPU_INSTANCE 16
@@ -486,6 +487,7 @@ struct amdgpu_fpriv {
struct amdgpu_vm vm;
struct amdgpu_bo_va *prt_va;
struct amdgpu_bo_va *csa_va;
+ struct amdgpu_bo_va *seq64_va;
struct mutex bo_list_lock;
struct idr bo_list_handles;
struct amdgpu_ctx_mgr ctx_mgr;
@@ -950,6 +952,9 @@ struct amdgpu_device {
/* GDS */
struct amdgpu_gds gds;
+ /* for userq and VM fences */
+ struct amdgpu_seq64 seq64;
+
/* KFD */
struct amdgpu_kfd_dev kfd;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 7f8fcac4f18b..828d0dd1455b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2253,6 +2253,12 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
goto init_failed;
}
}
+
+ r = amdgpu_seq64_init(adev);
+ if (r) {
+ DRM_ERROR("allocate seq64 failed %d\n", r);
+ goto init_failed;
+ }
}
}
@@ -2709,6 +2715,7 @@ static int amdgpu_device_ip_fini(struct amdgpu_device *adev)
amdgpu_device_wb_fini(adev);
amdgpu_device_vram_scratch_fini(adev);
amdgpu_ib_pool_fini(adev);
+ amdgpu_seq64_fini(adev);
}
r = adev->ip_blocks[i].version->funcs->sw_fini((void *)adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index b16b8155a157..d89f321304df 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1183,6 +1183,12 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
goto error_vm;
}
+ r = amdgpu_seq64_map(adev, &fpriv->vm, &fpriv->seq64_va,
+ AMDGPU_SEQ64_VADDR_START,
+ AMDGPU_SEQ64_SIZE);
+ if (r)
+ goto error_vm;
+
mutex_init(&fpriv->bo_list_lock);
idr_init_base(&fpriv->bo_list_handles, 1);
@@ -1250,6 +1256,13 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
amdgpu_bo_unreserve(adev->virt.csa_obj);
}
+ if (fpriv->seq64_va) {
+ WARN_ON(amdgpu_bo_reserve(adev->seq64.sbo, true));
+ amdgpu_vm_bo_del(adev, fpriv->seq64_va);
+ fpriv->seq64_va = NULL;
+ amdgpu_bo_unreserve(adev->seq64.sbo);
+ }
+
pasid = fpriv->vm.pasid;
pd = amdgpu_bo_ref(fpriv->vm.root.bo);
if (!WARN_ON(amdgpu_bo_reserve(pd, true))) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c
new file mode 100644
index 000000000000..bf43856cebbc
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright 2023 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_seq64.h"
+
+void amdgpu_seq64_fini(struct amdgpu_device *adev)
+{
+ if (!adev->enable_mes)
+ return;
+
+ amdgpu_bo_free_kernel(&adev->seq64.sbo,
+ NULL,
+ (void **)&adev->seq64.cpu_base_addr);
+}
+
+int amdgpu_seq64_init(struct amdgpu_device *adev)
+{
+ int r;
+
+ if (!adev->enable_mes)
+ return -EINVAL;
+
+ if (adev->seq64.sbo)
+ return 0;
+
+ /*
+ * AMDGPU_MAX_SEQ64_SLOTS * sizeof(u64) * 8 = AMDGPU_MAX_SEQ64_SLOTS
+ * 64bit slots
+ */
+ r = amdgpu_bo_create_kernel(adev, AMDGPU_SEQ64_SIZE,
+ PAGE_SIZE, AMDGPU_GEM_DOMAIN_GTT,
+ &adev->seq64.sbo, NULL,
+ (void **)&adev->seq64.cpu_base_addr);
+ if (r) {
+ dev_warn(adev->dev, "(%d) create seq64 failed\n", r);
+ return r;
+ }
+
+ memset(adev->seq64.cpu_base_addr, 0, AMDGPU_SEQ64_SIZE);
+
+ adev->seq64.num_sem = AMDGPU_MAX_SEQ64_SLOTS;
+ memset(&adev->seq64.used, 0, sizeof(adev->seq64.used));
+
+ return 0;
+}
+
+int amdgpu_seq64_map(struct amdgpu_device *adev, struct amdgpu_vm *vm,
+ struct amdgpu_bo_va **bo_va, u64 seq64_addr,
+ uint32_t size)
+{
+ struct ttm_validate_buffer seq64_tv;
+ struct amdgpu_bo_list_entry pd;
+ struct ww_acquire_ctx ticket;
+ struct list_head list;
+ struct amdgpu_bo *bo;
+ int r;
+
+ bo = adev->seq64.sbo;
+ if (!bo)
+ return -EINVAL;
+
+ INIT_LIST_HEAD(&list);
+ INIT_LIST_HEAD(&seq64_tv.head);
+
+ seq64_tv.bo = &bo->tbo;
+ seq64_tv.num_shared = 1;
+
+ list_add(&seq64_tv.head, &list);
+ amdgpu_vm_get_pd_bo(vm, &list, &pd);
+
+ r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL);
+ if (r)
+ return r;
+
+ *bo_va = amdgpu_vm_bo_add(adev, vm, bo);
+ if (!*bo_va) {
+ r = -ENOMEM;
+ goto error_vm;
+ }
+
+ r = amdgpu_vm_bo_map(adev, *bo_va, seq64_addr, 0, size,
+ AMDGPU_PTE_READABLE | AMDGPU_PTE_WRITEABLE |
+ AMDGPU_PTE_EXECUTABLE);
+ if (r) {
+ DRM_ERROR("failed to do bo_map on userq sem, err=%d\n", r);
+ goto error_map;
+ }
+
+ r = amdgpu_vm_bo_update(adev, *bo_va, false);
+ if (r) {
+ DRM_ERROR("failed to do vm_bo_update on userq sem\n");
+ goto error_map;
+ }
+
+ ttm_eu_backoff_reservation(&ticket, &list);
+
+ return 0;
+
+error_map:
+ amdgpu_vm_bo_del(adev, *bo_va);
+error_vm:
+ ttm_eu_backoff_reservation(&ticket, &list);
+ return r;
+}
+
+int amdgpu_seq64_get(struct amdgpu_device *adev, u64 *gpu_addr,
+ u64 **cpu_addr)
+{
+ unsigned long bit_pos;
+ u32 offset;
+
+ bit_pos = find_first_zero_bit(adev->seq64.used, adev->seq64.num_sem);
+
+ if (bit_pos < adev->seq64.num_sem) {
+ __set_bit(bit_pos, adev->seq64.used);
+ offset = bit_pos << 6; /* convert to qw offset */
+ } else {
+ return -EINVAL;
+ }
+
+ *gpu_addr = offset + AMDGPU_SEQ64_VADDR_START;
+ *cpu_addr = offset + adev->seq64.cpu_base_addr;
+
+ return 0;
+}
+
+void amdgpu_seq64_free(struct amdgpu_device *adev, u64 gpu_addr)
+{
+ u32 offset;
+
+ offset = gpu_addr - AMDGPU_SEQ64_VADDR_START;
+
+ offset >>= 6;
+ if (offset < adev->seq64.num_sem)
+ __clear_bit(offset, adev->seq64.used);
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h
new file mode 100644
index 000000000000..e9b0afa9c5aa
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright 2023 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_SEQ64_H__
+#define __AMDGPU_SEQ64_H__
+
+#define AMDGPU_SEQ64_SIZE (2ULL << 20)
+#define AMDGPU_MAX_SEQ64_SLOTS (AMDGPU_SEQ64_SIZE / (sizeof(u64) * 8))
+#define AMDGPU_SEQ64_VADDR_OFFSET 0x50000
+#define AMDGPU_SEQ64_VADDR_START (AMDGPU_VA_RESERVED_SIZE + AMDGPU_SEQ64_VADDR_OFFSET)
+
+struct amdgpu_seq64 {
+ struct amdgpu_bo *sbo;
+ u32 num_sem;
+ u64 *cpu_base_addr;
+ unsigned long used[DIV_ROUND_UP(AMDGPU_MAX_SEQ64_SLOTS, BITS_PER_LONG)];
+};
+
+void amdgpu_seq64_fini(struct amdgpu_device *adev);
+int amdgpu_seq64_init(struct amdgpu_device *adev);
+int amdgpu_seq64_get(struct amdgpu_device *adev, u64 *gpu_addr, u64 **cpu_addr);
+void amdgpu_seq64_free(struct amdgpu_device *adev, u64 gpu_addr);
+int amdgpu_seq64_map(struct amdgpu_device *adev, struct amdgpu_vm *vm,
+ struct amdgpu_bo_va **bo_va, u64 seq64_addr, uint32_t size);
+
+#endif
+
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 1/6] drm/amdgpu: Implement a new 64bit sequence memory driver
2023-04-20 14:47 ` [PATCH v3 1/6] drm/amdgpu: Implement a new 64bit sequence memory driver Arunpravin Paneer Selvam
@ 2023-04-21 12:24 ` Christian König
0 siblings, 0 replies; 11+ messages in thread
From: Christian König @ 2023-04-21 12:24 UTC (permalink / raw)
To: Arunpravin Paneer Selvam, amd-gfx; +Cc: alexander.deucher
Am 20.04.23 um 16:47 schrieb Arunpravin Paneer Selvam:
> Developed a new driver which allocates a 64bit memory on
> each request in sequence order. At the moment, user queue
> fence memory is the main consumer of this seq64 driver.
>
> v2: Worked on review comments from Christian for the following
> modifications
>
> - Move driver name from "semaphore" to "seq64"
> - Remove unnecessary PT/PD mapping
> - Move enable_mes check into init/fini functions.
Please just drop the enable_mes check. We need this functionality for
the TLB counter as well and that is completely independent of MES.
One more technical comment below.
Apart from that looks good to me, but could use a little bit kerneldoc.
>
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/Makefile | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 5 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 13 ++
> drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c | 158 +++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h | 48 +++++++
> 6 files changed, 232 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c
> create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
> index 2cc7897de7e6..d39504e65db1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -59,7 +59,7 @@ amdgpu-y += amdgpu_device.o amdgpu_doorbell_mgr.o amdgpu_kms.o \
> amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
> 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_ring_mux.o amdgpu_seq64.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 c5f9af0e74ee..3bc8a2d35bb3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -109,6 +109,7 @@
> #include "amdgpu_fdinfo.h"
> #include "amdgpu_mca.h"
> #include "amdgpu_ras.h"
> +#include "amdgpu_seq64.h"
>
> #define MAX_GPU_INSTANCE 16
>
> @@ -486,6 +487,7 @@ struct amdgpu_fpriv {
> struct amdgpu_vm vm;
> struct amdgpu_bo_va *prt_va;
> struct amdgpu_bo_va *csa_va;
> + struct amdgpu_bo_va *seq64_va;
> struct mutex bo_list_lock;
> struct idr bo_list_handles;
> struct amdgpu_ctx_mgr ctx_mgr;
> @@ -950,6 +952,9 @@ struct amdgpu_device {
> /* GDS */
> struct amdgpu_gds gds;
>
> + /* for userq and VM fences */
> + struct amdgpu_seq64 seq64;
> +
> /* KFD */
> struct amdgpu_kfd_dev kfd;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 7f8fcac4f18b..828d0dd1455b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2253,6 +2253,12 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
> goto init_failed;
> }
> }
> +
> + r = amdgpu_seq64_init(adev);
> + if (r) {
> + DRM_ERROR("allocate seq64 failed %d\n", r);
> + goto init_failed;
> + }
> }
> }
>
> @@ -2709,6 +2715,7 @@ static int amdgpu_device_ip_fini(struct amdgpu_device *adev)
> amdgpu_device_wb_fini(adev);
> amdgpu_device_vram_scratch_fini(adev);
> amdgpu_ib_pool_fini(adev);
> + amdgpu_seq64_fini(adev);
> }
>
> r = adev->ip_blocks[i].version->funcs->sw_fini((void *)adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index b16b8155a157..d89f321304df 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -1183,6 +1183,12 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
> goto error_vm;
> }
>
> + r = amdgpu_seq64_map(adev, &fpriv->vm, &fpriv->seq64_va,
> + AMDGPU_SEQ64_VADDR_START,
> + AMDGPU_SEQ64_SIZE);
> + if (r)
> + goto error_vm;
> +
> mutex_init(&fpriv->bo_list_lock);
> idr_init_base(&fpriv->bo_list_handles, 1);
>
> @@ -1250,6 +1256,13 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
> amdgpu_bo_unreserve(adev->virt.csa_obj);
> }
>
> + if (fpriv->seq64_va) {
> + WARN_ON(amdgpu_bo_reserve(adev->seq64.sbo, true));
> + amdgpu_vm_bo_del(adev, fpriv->seq64_va);
> + fpriv->seq64_va = NULL;
> + amdgpu_bo_unreserve(adev->seq64.sbo);
> + }
> +
> pasid = fpriv->vm.pasid;
> pd = amdgpu_bo_ref(fpriv->vm.root.bo);
> if (!WARN_ON(amdgpu_bo_reserve(pd, true))) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c
> new file mode 100644
> index 000000000000..bf43856cebbc
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c
> @@ -0,0 +1,158 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright 2023 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_seq64.h"
> +
> +void amdgpu_seq64_fini(struct amdgpu_device *adev)
> +{
> + if (!adev->enable_mes)
> + return;
> +
> + amdgpu_bo_free_kernel(&adev->seq64.sbo,
> + NULL,
> + (void **)&adev->seq64.cpu_base_addr);
> +}
> +
> +int amdgpu_seq64_init(struct amdgpu_device *adev)
> +{
> + int r;
> +
> + if (!adev->enable_mes)
> + return -EINVAL;
> +
> + if (adev->seq64.sbo)
> + return 0;
> +
> + /*
> + * AMDGPU_MAX_SEQ64_SLOTS * sizeof(u64) * 8 = AMDGPU_MAX_SEQ64_SLOTS
> + * 64bit slots
> + */
> + r = amdgpu_bo_create_kernel(adev, AMDGPU_SEQ64_SIZE,
> + PAGE_SIZE, AMDGPU_GEM_DOMAIN_GTT,
> + &adev->seq64.sbo, NULL,
> + (void **)&adev->seq64.cpu_base_addr);
> + if (r) {
> + dev_warn(adev->dev, "(%d) create seq64 failed\n", r);
> + return r;
> + }
> +
> + memset(adev->seq64.cpu_base_addr, 0, AMDGPU_SEQ64_SIZE);
> +
> + adev->seq64.num_sem = AMDGPU_MAX_SEQ64_SLOTS;
> + memset(&adev->seq64.used, 0, sizeof(adev->seq64.used));
> +
> + return 0;
> +}
> +
> +int amdgpu_seq64_map(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> + struct amdgpu_bo_va **bo_va, u64 seq64_addr,
> + uint32_t size)
> +{
> + struct ttm_validate_buffer seq64_tv;
> + struct amdgpu_bo_list_entry pd;
> + struct ww_acquire_ctx ticket;
> + struct list_head list;
> + struct amdgpu_bo *bo;
> + int r;
> +
> + bo = adev->seq64.sbo;
> + if (!bo)
> + return -EINVAL;
> +
> + INIT_LIST_HEAD(&list);
> + INIT_LIST_HEAD(&seq64_tv.head);
> +
> + seq64_tv.bo = &bo->tbo;
> + seq64_tv.num_shared = 1;
> +
> + list_add(&seq64_tv.head, &list);
> + amdgpu_vm_get_pd_bo(vm, &list, &pd);
> +
> + r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL);
> + if (r)
> + return r;
> +
> + *bo_va = amdgpu_vm_bo_add(adev, vm, bo);
> + if (!*bo_va) {
> + r = -ENOMEM;
> + goto error_vm;
> + }
> +
> + r = amdgpu_vm_bo_map(adev, *bo_va, seq64_addr, 0, size,
> + AMDGPU_PTE_READABLE | AMDGPU_PTE_WRITEABLE |
> + AMDGPU_PTE_EXECUTABLE);
> + if (r) {
> + DRM_ERROR("failed to do bo_map on userq sem, err=%d\n", r);
> + goto error_map;
> + }
> +
> + r = amdgpu_vm_bo_update(adev, *bo_va, false);
> + if (r) {
> + DRM_ERROR("failed to do vm_bo_update on userq sem\n");
> + goto error_map;
> + }
> +
> + ttm_eu_backoff_reservation(&ticket, &list);
> +
> + return 0;
> +
> +error_map:
> + amdgpu_vm_bo_del(adev, *bo_va);
> +error_vm:
> + ttm_eu_backoff_reservation(&ticket, &list);
> + return r;
> +}
> +
> +int amdgpu_seq64_get(struct amdgpu_device *adev, u64 *gpu_addr,
> + u64 **cpu_addr)
> +{
> + unsigned long bit_pos;
> + u32 offset;
> +
> + bit_pos = find_first_zero_bit(adev->seq64.used, adev->seq64.num_sem);
> +
> + if (bit_pos < adev->seq64.num_sem) {
> + __set_bit(bit_pos, adev->seq64.used);
> + offset = bit_pos << 6; /* convert to qw offset */
> + } else {
> + return -EINVAL;
> + }
> +
> + *gpu_addr = offset + AMDGPU_SEQ64_VADDR_START;
> + *cpu_addr = offset + adev->seq64.cpu_base_addr;
> +
> + return 0;
> +}
> +
> +void amdgpu_seq64_free(struct amdgpu_device *adev, u64 gpu_addr)
> +{
> + u32 offset;
> +
> + offset = gpu_addr - AMDGPU_SEQ64_VADDR_START;
> +
> + offset >>= 6;
> + if (offset < adev->seq64.num_sem)
> + __clear_bit(offset, adev->seq64.used);
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h
> new file mode 100644
> index 000000000000..e9b0afa9c5aa
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright 2023 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_SEQ64_H__
> +#define __AMDGPU_SEQ64_H__
> +
> +#define AMDGPU_SEQ64_SIZE (2ULL << 20)
> +#define AMDGPU_MAX_SEQ64_SLOTS (AMDGPU_SEQ64_SIZE / (sizeof(u64) * 8))
> +#define AMDGPU_SEQ64_VADDR_OFFSET 0x50000
> +#define AMDGPU_SEQ64_VADDR_START (AMDGPU_VA_RESERVED_SIZE + AMDGPU_SEQ64_VADDR_OFFSET)
> +
> +struct amdgpu_seq64 {
> + struct amdgpu_bo *sbo;
> + u32 num_sem;
> + u64 *cpu_base_addr;
> + unsigned long used[DIV_ROUND_UP(AMDGPU_MAX_SEQ64_SLOTS, BITS_PER_LONG)];
Please use DECLARE_BITMAP() instead of open coding this here.
Christian.
> +};
> +
> +void amdgpu_seq64_fini(struct amdgpu_device *adev);
> +int amdgpu_seq64_init(struct amdgpu_device *adev);
> +int amdgpu_seq64_get(struct amdgpu_device *adev, u64 *gpu_addr, u64 **cpu_addr);
> +void amdgpu_seq64_free(struct amdgpu_device *adev, u64 gpu_addr);
> +int amdgpu_seq64_map(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> + struct amdgpu_bo_va **bo_va, u64 seq64_addr, uint32_t size);
> +
> +#endif
> +
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 2/6] drm/amdgpu: Implement a new userqueue fence driver
2023-04-20 14:47 [PATCH v3 0/6] Usermode queue fencing synchronization Arunpravin Paneer Selvam
2023-04-20 14:47 ` [PATCH v3 1/6] drm/amdgpu: Implement a new 64bit sequence memory driver Arunpravin Paneer Selvam
@ 2023-04-20 14:47 ` Arunpravin Paneer Selvam
2023-04-21 12:37 ` Christian König
2023-04-20 14:47 ` [PATCH v3 3/6] drm/amdgpu: Add mqd support for the fence address Arunpravin Paneer Selvam
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Arunpravin Paneer Selvam @ 2023-04-20 14:47 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, christian.koenig, Arunpravin Paneer Selvam
Developed a userqueue fence driver for the userqueue process shared
BO synchronization.
Create a dma fence having write pointer as the seqno and allocate a
seq64 memory for each user queue process and feed this memory address
into the firmware/hardware, thus the firmware writes the read pointer
into the given address when the process completes it execution.
Compare wptr and rptr, if rptr >= wptr, signal the fences for the waiting
process to consume the buffers.
v2: Worked on review comments from Christian for the following
modifications
- Add wptr as sequence number into the fence
- Add a reference count for the fence driver
- Add dma_fence_put below the list_del as it might frees the userq fence.
- Trim unnecessary code in interrupt handler.
- Check dma fence signaled state in dma fence creation function for a
potential problem of hardware completing the job processing beforehand.
- Add necessary locks.
- Create a list and process all the unsignaled fences.
- clean up fences in destroy function.
- implement .signaled callback function
v3: Worked on review comments from Christian
- Modify naming convention for reference counted objects
- Fix fence driver reference drop issue
- Drop amdgpu_userq_fence_driver_process() function return value
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
build.sh | 11 +
drivers/gpu/drm/amd/amdgpu/Makefile | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 +
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 253 ++++++++++++++++++
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 64 +++++
drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 19 ++
.../gpu/drm/amd/include/amdgpu_userqueue.h | 1 +
7 files changed, 355 insertions(+), 1 deletion(-)
create mode 100755 build.sh
create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
diff --git a/build.sh b/build.sh
new file mode 100755
index 000000000000..fddaff75dd2a
--- /dev/null
+++ b/build.sh
@@ -0,0 +1,11 @@
+#make -j16 modules M=drivers/gpu/drm/
+#make -j16 modules M=drivers/gpu/drm/selftests/
+make -j16 modules M=drivers/gpu/drm/amd/amdgpu/
+#make -j16 modules M=drivers/gpu/drm/ttm/
+#cp drivers/gpu/drm/drm.ko /lib/modules/$(uname -r)/kernel/drivers/gpu/drm/
+#cp drivers/gpu/drm/selftests/test-drm_buddy.ko /lib/modules/$(uname -r)/kernel/drivers/gpu/drm/selftests/
+#cp drivers/gpu/drm/amd/amdgpu/amdgpu.ko /lib/modules/$(uname -r)/kernel/drivers/gpu/drm/amd/amdgpu/
+#cp drivers/gpu/drm/ttm/ttm.ko /lib/modules/$(uname -r)/kernel/drivers/gpu/drm/ttm/
+#update-initramfs -c -k $(uname -r)
+#reboot
+
diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index d39504e65db1..8ed9be0d4818 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -59,7 +59,7 @@ amdgpu-y += amdgpu_device.o amdgpu_doorbell_mgr.o amdgpu_kms.o \
amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
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_seq64.o
+ amdgpu_ring_mux.o amdgpu_seq64.o amdgpu_userq_fence.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 229976a2d0e7..e9c5047087d0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -53,6 +53,7 @@
#include "amdgpu_xgmi.h"
#include "amdgpu_reset.h"
#include "amdgpu_userqueue.h"
+#include "amdgpu_userq_fence.h"
/*
* KMS wrapper.
@@ -2826,6 +2827,10 @@ static int __init amdgpu_init(void)
if (r)
goto error_fence;
+ r = amdgpu_userq_fence_slab_init();
+ if (r)
+ goto error_fence;
+
DRM_INFO("amdgpu kernel modesetting enabled.\n");
amdgpu_register_atpx_handler();
amdgpu_acpi_detect();
@@ -2850,6 +2855,7 @@ static void __exit amdgpu_exit(void)
amdgpu_unregister_atpx_handler();
amdgpu_sync_fini();
amdgpu_fence_slab_fini();
+ amdgpu_userq_fence_slab_fini();
mmu_notifier_synchronize();
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
new file mode 100644
index 000000000000..a03d12f83147
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright 2023 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/kref.h>
+#include <linux/slab.h>
+
+#include <drm/drm_syncobj.h>
+
+#include "amdgpu.h"
+#include "amdgpu_userq_fence.h"
+
+static struct kmem_cache *amdgpu_userq_fence_slab;
+
+int amdgpu_userq_fence_slab_init(void)
+{
+ amdgpu_userq_fence_slab = kmem_cache_create("amdgpu_userq_fence",
+ sizeof(struct amdgpu_userq_fence),
+ 0,
+ SLAB_HWCACHE_ALIGN,
+ NULL);
+ if (!amdgpu_userq_fence_slab)
+ return -ENOMEM;
+
+ return 0;
+}
+
+void amdgpu_userq_fence_slab_fini(void)
+{
+ rcu_barrier();
+ kmem_cache_destroy(amdgpu_userq_fence_slab);
+}
+
+static inline struct amdgpu_userq_fence *to_amdgpu_userq_fence(struct dma_fence *f)
+{
+ struct amdgpu_userq_fence *__f = container_of(f, struct amdgpu_userq_fence, base);
+
+ if (!__f)
+ return NULL;
+
+ if (__f->base.ops == &amdgpu_userq_fence_ops)
+ return __f;
+
+ return NULL;
+}
+
+static u64 amdgpu_userq_fence_read(struct amdgpu_userq_fence_driver *fence_drv)
+{
+ return le64_to_cpu(*fence_drv->cpu_addr);
+}
+
+int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
+ struct amdgpu_usermode_queue *userq)
+{
+ struct amdgpu_userq_fence_driver *fence_drv;
+ int r;
+
+ fence_drv = userq->fence_drv;
+ if (!fence_drv)
+ return -EINVAL;
+
+ /* Acquire seq64 memory */
+ r = amdgpu_seq64_get(adev, &fence_drv->gpu_addr,
+ &fence_drv->cpu_addr);
+ if (r)
+ return -ENOMEM;
+
+ kref_init(&fence_drv->refcount);
+ INIT_LIST_HEAD(&fence_drv->fences);
+ spin_lock_init(&fence_drv->fence_list_lock);
+
+ fence_drv->adev = adev;
+ fence_drv->context = dma_fence_context_alloc(1);
+
+ get_task_comm(fence_drv->timeline_name, current);
+
+ return 0;
+}
+
+void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_drv)
+{
+ struct amdgpu_userq_fence *userq_fence, *tmp;
+ struct dma_fence *fence;
+
+ if (!fence_drv)
+ return;
+
+ spin_lock(&fence_drv->fence_list_lock);
+ list_for_each_entry_safe(userq_fence, tmp, &fence_drv->fences, link) {
+ fence = &userq_fence->base;
+
+ if (amdgpu_userq_fence_read(fence_drv) >= fence->seqno) {
+ dma_fence_signal(fence);
+ list_del(&userq_fence->link);
+
+ dma_fence_put(fence);
+ } else {
+ break;
+ }
+ }
+ spin_unlock(&fence_drv->fence_list_lock);
+}
+
+void amdgpu_userq_fence_driver_destroy(struct kref *ref)
+{
+ struct amdgpu_userq_fence_driver *fence_drv = container_of(ref,
+ struct amdgpu_userq_fence_driver,
+ refcount);
+ struct amdgpu_device *adev = fence_drv->adev;
+ struct amdgpu_userq_fence *fence, *tmp;
+ struct dma_fence *f;
+
+ spin_lock(&fence_drv->fence_list_lock);
+ list_for_each_entry_safe(fence, tmp, &fence_drv->fences, link) {
+ f = &fence->base;
+
+ if (!dma_fence_is_signaled(f)) {
+ dma_fence_set_error(f, -ECANCELED);
+ dma_fence_signal(f);
+ }
+
+ list_del(&fence->link);
+ dma_fence_put(f);
+ }
+
+ WARN_ON_ONCE(!list_empty(&fence_drv->fences));
+ spin_unlock(&fence_drv->fence_list_lock);
+
+ /* Free seq64 memory */
+ amdgpu_seq64_free(adev, fence_drv->gpu_addr);
+ kfree(fence_drv);
+}
+
+void amdgpu_userq_fence_driver_get(struct amdgpu_userq_fence_driver *fence_drv)
+{
+ kref_get(&fence_drv->refcount);
+}
+
+void amdgpu_userq_fence_driver_put(struct amdgpu_userq_fence_driver *fence_drv)
+{
+ kref_put(&fence_drv->refcount, amdgpu_userq_fence_driver_destroy);
+}
+
+int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
+ u64 seq, struct dma_fence **f)
+{
+ struct amdgpu_userq_fence_driver *fence_drv;
+ struct amdgpu_userq_fence *userq_fence;
+ struct dma_fence *fence;
+
+ fence_drv = userq->fence_drv;
+ if (!fence_drv)
+ return -EINVAL;
+
+ userq_fence = kmem_cache_alloc(amdgpu_userq_fence_slab, GFP_ATOMIC);
+ if (!userq_fence)
+ return -ENOMEM;
+
+ spin_lock_init(&userq_fence->lock);
+ INIT_LIST_HEAD(&userq_fence->link);
+ fence = &userq_fence->base;
+ userq_fence->fence_drv = fence_drv;
+
+ dma_fence_init(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
+ fence_drv->context, seq);
+
+ amdgpu_userq_fence_driver_get(fence_drv);
+ dma_fence_get(fence);
+
+ spin_lock(&fence_drv->fence_list_lock);
+ /* Check if hardware has already processed the job */
+ if (!dma_fence_is_signaled(fence)) {
+ list_add_tail(&userq_fence->link, &fence_drv->fences);
+ } else {
+ dma_fence_put(fence);
+ }
+ spin_unlock(&fence_drv->fence_list_lock);
+
+ *f = fence;
+
+ return 0;
+}
+
+static const char *amdgpu_userq_fence_get_driver_name(struct dma_fence *f)
+{
+ return "amdgpu_userqueue_fence";
+}
+
+static const char *amdgpu_userq_fence_get_timeline_name(struct dma_fence *f)
+{
+ struct amdgpu_userq_fence *fence = to_amdgpu_userq_fence(f);
+
+ return fence->fence_drv->timeline_name;
+}
+
+static bool amdgpu_userq_fence_signaled(struct dma_fence *f)
+{
+ struct amdgpu_userq_fence *fence = to_amdgpu_userq_fence(f);
+ struct amdgpu_userq_fence_driver *fence_drv = fence->fence_drv;
+ u64 rptr, wptr;
+
+ rptr = amdgpu_userq_fence_read(fence_drv);
+ wptr = fence->base.seqno;
+
+ if (rptr >= wptr)
+ return true;
+
+ return false;
+}
+
+static void amdgpu_userq_fence_free(struct rcu_head *rcu)
+{
+ struct dma_fence *fence = container_of(rcu, struct dma_fence, rcu);
+ struct amdgpu_userq_fence *userq_fence = to_amdgpu_userq_fence(fence);
+ struct amdgpu_userq_fence_driver *fence_drv = userq_fence->fence_drv;
+
+ /* Release the fence driver reference */
+ amdgpu_userq_fence_driver_put(fence_drv);
+ kmem_cache_free(amdgpu_userq_fence_slab, userq_fence);
+}
+
+static void amdgpu_userq_fence_release(struct dma_fence *f)
+{
+ call_rcu(&f->rcu, amdgpu_userq_fence_free);
+}
+
+static const struct dma_fence_ops amdgpu_userq_fence_ops = {
+ .use_64bit_seqno = true,
+ .get_driver_name = amdgpu_userq_fence_get_driver_name,
+ .get_timeline_name = amdgpu_userq_fence_get_timeline_name,
+ .signaled = amdgpu_userq_fence_signaled,
+ .release = amdgpu_userq_fence_release,
+};
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
new file mode 100644
index 000000000000..7329b4e5dd30
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
@@ -0,0 +1,64 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright 2023 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_USERQ_FENCE_H__
+#define __AMDGPU_USERQ_FENCE_H__
+
+#include <linux/types.h>
+
+#include "amdgpu_userqueue.h"
+
+struct amdgpu_userq_fence {
+ struct dma_fence base;
+ /* userq fence lock */
+ spinlock_t lock;
+ struct list_head link;
+ struct amdgpu_userq_fence_driver *fence_drv;
+};
+
+struct amdgpu_userq_fence_driver {
+ struct kref refcount;
+ u64 gpu_addr;
+ u64 *cpu_addr;
+ u64 context;
+ /* fence list lock */
+ spinlock_t fence_list_lock;
+ struct list_head fences;
+ struct amdgpu_device *adev;
+ char timeline_name[TASK_COMM_LEN];
+};
+
+static const struct dma_fence_ops amdgpu_userq_fence_ops;
+
+int amdgpu_userq_fence_slab_init(void);
+void amdgpu_userq_fence_slab_fini(void);
+int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
+ u64 seq, struct dma_fence **f);
+void amdgpu_userq_fence_driver_get(struct amdgpu_userq_fence_driver *fence_drv);
+void amdgpu_userq_fence_driver_put(struct amdgpu_userq_fence_driver *fence_drv);
+int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev, struct amdgpu_usermode_queue *userq);
+void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_drv);
+void amdgpu_userq_fence_driver_destroy(struct kref *ref);
+
+#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
index fd4a2ca3302d..8918b176fdcb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
@@ -24,6 +24,7 @@
#include "amdgpu.h"
#include "amdgpu_vm.h"
#include "amdgpu_userqueue.h"
+#include "amdgpu_userq_fence.h"
static inline int
amdgpu_userqueue_index(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermode_queue *queue)
@@ -158,6 +159,8 @@ static int amdgpu_userqueue_create(struct drm_file *filp, union drm_amdgpu_userq
struct amdgpu_fpriv *fpriv = filp->driver_priv;
struct amdgpu_userq_mgr *uq_mgr = &fpriv->userq_mgr;
struct drm_amdgpu_userq_mqd *mqd_in = &args->in.mqd;
+ struct amdgpu_userq_fence_driver *fence_drv;
+ struct amdgpu_device *adev = uq_mgr->adev;
uint64_t index;
int r;
@@ -173,6 +176,12 @@ static int amdgpu_userqueue_create(struct drm_file *filp, union drm_amdgpu_userq
return -ENOMEM;
}
+ fence_drv = kzalloc(sizeof(struct amdgpu_userq_fence_driver), GFP_KERNEL);
+ if (!fence_drv) {
+ DRM_ERROR("Failed to allocate memory for fence driver\n");
+ return -ENOMEM;
+ }
+
mutex_lock(&uq_mgr->userq_mutex);
queue->userq_prop.wptr_gpu_addr = mqd_in->wptr_va;
queue->userq_prop.rptr_gpu_addr = mqd_in->rptr_va;
@@ -188,6 +197,13 @@ static int amdgpu_userqueue_create(struct drm_file *filp, union drm_amdgpu_userq
goto free_queue;
}
+ queue->fence_drv = fence_drv;
+ r = amdgpu_userq_fence_driver_alloc(adev, queue);
+ if (r) {
+ DRM_ERROR("Failed to alloc fence driver\n");
+ goto free_fence_drv;
+ }
+
queue->userq_prop.doorbell_index = index;
queue->shadow_ctx_gpu_addr = mqd_in->shadow_va;
queue->queue_type = mqd_in->ip_type;
@@ -217,6 +233,8 @@ static int amdgpu_userqueue_create(struct drm_file *filp, union drm_amdgpu_userq
mutex_unlock(&uq_mgr->userq_mutex);
return 0;
+free_fence_drv:
+ amdgpu_userq_fence_driver_put(queue->fence_drv);
free_queue:
mutex_unlock(&uq_mgr->userq_mutex);
kfree(queue);
@@ -238,6 +256,7 @@ static void amdgpu_userqueue_destroy(struct drm_file *filp, int queue_id)
mutex_lock(&uq_mgr->userq_mutex);
uq_mgr->userq_funcs[queue->queue_type]->mqd_destroy(uq_mgr, queue);
amdgpu_userqueue_free_index(uq_mgr, queue->queue_id);
+ amdgpu_userq_fence_driver_put(queue->fence_drv);
mutex_unlock(&uq_mgr->userq_mutex);
kfree(queue);
}
diff --git a/drivers/gpu/drm/amd/include/amdgpu_userqueue.h b/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
index eaab7cf5fff6..eaea88539007 100644
--- a/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
+++ b/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
@@ -50,6 +50,7 @@ struct amdgpu_usermode_queue {
struct amdgpu_mqd_prop userq_prop;
struct amdgpu_userq_ctx_space mqd;
struct amdgpu_userq_ctx_space fw_space;
+ struct amdgpu_userq_fence_driver *fence_drv;
};
struct amdgpu_userq_funcs {
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 2/6] drm/amdgpu: Implement a new userqueue fence driver
2023-04-20 14:47 ` [PATCH v3 2/6] drm/amdgpu: Implement a new userqueue fence driver Arunpravin Paneer Selvam
@ 2023-04-21 12:37 ` Christian König
2023-04-24 19:09 ` Arunpravin Paneer Selvam
0 siblings, 1 reply; 11+ messages in thread
From: Christian König @ 2023-04-21 12:37 UTC (permalink / raw)
To: Arunpravin Paneer Selvam, amd-gfx; +Cc: alexander.deucher
Am 20.04.23 um 16:47 schrieb Arunpravin Paneer Selvam:
> Developed a userqueue fence driver for the userqueue process shared
> BO synchronization.
>
> Create a dma fence having write pointer as the seqno and allocate a
> seq64 memory for each user queue process and feed this memory address
> into the firmware/hardware, thus the firmware writes the read pointer
> into the given address when the process completes it execution.
> Compare wptr and rptr, if rptr >= wptr, signal the fences for the waiting
> process to consume the buffers.
>
> v2: Worked on review comments from Christian for the following
> modifications
>
> - Add wptr as sequence number into the fence
> - Add a reference count for the fence driver
> - Add dma_fence_put below the list_del as it might frees the userq fence.
> - Trim unnecessary code in interrupt handler.
> - Check dma fence signaled state in dma fence creation function for a
> potential problem of hardware completing the job processing beforehand.
> - Add necessary locks.
> - Create a list and process all the unsignaled fences.
> - clean up fences in destroy function.
> - implement .signaled callback function
>
> v3: Worked on review comments from Christian
> - Modify naming convention for reference counted objects
> - Fix fence driver reference drop issue
> - Drop amdgpu_userq_fence_driver_process() function return value
That looks really good, just two more comments below.
>
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
> ---
> build.sh | 11 +
> drivers/gpu/drm/amd/amdgpu/Makefile | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 +
> .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 253 ++++++++++++++++++
> .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 64 +++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 19 ++
> .../gpu/drm/amd/include/amdgpu_userqueue.h | 1 +
> 7 files changed, 355 insertions(+), 1 deletion(-)
> create mode 100755 build.sh
> create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
>
> diff --git a/build.sh b/build.sh
> new file mode 100755
> index 000000000000..fddaff75dd2a
> --- /dev/null
> +++ b/build.sh
> @@ -0,0 +1,11 @@
> +#make -j16 modules M=drivers/gpu/drm/
> +#make -j16 modules M=drivers/gpu/drm/selftests/
> +make -j16 modules M=drivers/gpu/drm/amd/amdgpu/
> +#make -j16 modules M=drivers/gpu/drm/ttm/
> +#cp drivers/gpu/drm/drm.ko /lib/modules/$(uname -r)/kernel/drivers/gpu/drm/
> +#cp drivers/gpu/drm/selftests/test-drm_buddy.ko /lib/modules/$(uname -r)/kernel/drivers/gpu/drm/selftests/
> +#cp drivers/gpu/drm/amd/amdgpu/amdgpu.ko /lib/modules/$(uname -r)/kernel/drivers/gpu/drm/amd/amdgpu/
> +#cp drivers/gpu/drm/ttm/ttm.ko /lib/modules/$(uname -r)/kernel/drivers/gpu/drm/ttm/
> +#update-initramfs -c -k $(uname -r)
> +#reboot
> +
I strongly assume you didn't intentionally committed that, did you?
Anyway, please remove :)
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
> index d39504e65db1..8ed9be0d4818 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -59,7 +59,7 @@ amdgpu-y += amdgpu_device.o amdgpu_doorbell_mgr.o amdgpu_kms.o \
> amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
> 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_seq64.o
> + amdgpu_ring_mux.o amdgpu_seq64.o amdgpu_userq_fence.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 229976a2d0e7..e9c5047087d0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -53,6 +53,7 @@
> #include "amdgpu_xgmi.h"
> #include "amdgpu_reset.h"
> #include "amdgpu_userqueue.h"
> +#include "amdgpu_userq_fence.h"
>
> /*
> * KMS wrapper.
> @@ -2826,6 +2827,10 @@ static int __init amdgpu_init(void)
> if (r)
> goto error_fence;
>
> + r = amdgpu_userq_fence_slab_init();
> + if (r)
> + goto error_fence;
> +
> DRM_INFO("amdgpu kernel modesetting enabled.\n");
> amdgpu_register_atpx_handler();
> amdgpu_acpi_detect();
> @@ -2850,6 +2855,7 @@ static void __exit amdgpu_exit(void)
> amdgpu_unregister_atpx_handler();
> amdgpu_sync_fini();
> amdgpu_fence_slab_fini();
> + amdgpu_userq_fence_slab_fini();
> mmu_notifier_synchronize();
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> new file mode 100644
> index 000000000000..a03d12f83147
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> @@ -0,0 +1,253 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright 2023 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/kref.h>
> +#include <linux/slab.h>
> +
> +#include <drm/drm_syncobj.h>
> +
> +#include "amdgpu.h"
> +#include "amdgpu_userq_fence.h"
> +
> +static struct kmem_cache *amdgpu_userq_fence_slab;
> +
> +int amdgpu_userq_fence_slab_init(void)
> +{
> + amdgpu_userq_fence_slab = kmem_cache_create("amdgpu_userq_fence",
> + sizeof(struct amdgpu_userq_fence),
> + 0,
> + SLAB_HWCACHE_ALIGN,
> + NULL);
> + if (!amdgpu_userq_fence_slab)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> +void amdgpu_userq_fence_slab_fini(void)
> +{
> + rcu_barrier();
> + kmem_cache_destroy(amdgpu_userq_fence_slab);
> +}
> +
> +static inline struct amdgpu_userq_fence *to_amdgpu_userq_fence(struct dma_fence *f)
> +{
> + struct amdgpu_userq_fence *__f = container_of(f, struct amdgpu_userq_fence, base);
> +
> + if (!__f)
> + return NULL;
> +
> + if (__f->base.ops == &amdgpu_userq_fence_ops)
> + return __f;
> +
> + return NULL;
> +}
> +
> +static u64 amdgpu_userq_fence_read(struct amdgpu_userq_fence_driver *fence_drv)
> +{
> + return le64_to_cpu(*fence_drv->cpu_addr);
> +}
> +
> +int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
> + struct amdgpu_usermode_queue *userq)
> +{
> + struct amdgpu_userq_fence_driver *fence_drv;
> + int r;
> +
> + fence_drv = userq->fence_drv;
> + if (!fence_drv)
> + return -EINVAL;
> +
> + /* Acquire seq64 memory */
> + r = amdgpu_seq64_get(adev, &fence_drv->gpu_addr,
> + &fence_drv->cpu_addr);
> + if (r)
> + return -ENOMEM;
> +
> + kref_init(&fence_drv->refcount);
> + INIT_LIST_HEAD(&fence_drv->fences);
> + spin_lock_init(&fence_drv->fence_list_lock);
> +
> + fence_drv->adev = adev;
> + fence_drv->context = dma_fence_context_alloc(1);
> +
> + get_task_comm(fence_drv->timeline_name, current);
> +
> + return 0;
> +}
> +
> +void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_drv)
> +{
> + struct amdgpu_userq_fence *userq_fence, *tmp;
> + struct dma_fence *fence;
> +
> + if (!fence_drv)
> + return;
> +
> + spin_lock(&fence_drv->fence_list_lock);
> + list_for_each_entry_safe(userq_fence, tmp, &fence_drv->fences, link) {
> + fence = &userq_fence->base;
> +
> + if (amdgpu_userq_fence_read(fence_drv) >= fence->seqno) {
> + dma_fence_signal(fence);
> + list_del(&userq_fence->link);
> +
> + dma_fence_put(fence);
> + } else {
> + break;
> + }
> + }
> + spin_unlock(&fence_drv->fence_list_lock);
> +}
> +
> +void amdgpu_userq_fence_driver_destroy(struct kref *ref)
> +{
> + struct amdgpu_userq_fence_driver *fence_drv = container_of(ref,
> + struct amdgpu_userq_fence_driver,
> + refcount);
> + struct amdgpu_device *adev = fence_drv->adev;
> + struct amdgpu_userq_fence *fence, *tmp;
> + struct dma_fence *f;
> +
> + spin_lock(&fence_drv->fence_list_lock);
> + list_for_each_entry_safe(fence, tmp, &fence_drv->fences, link) {
> + f = &fence->base;
> +
> + if (!dma_fence_is_signaled(f)) {
> + dma_fence_set_error(f, -ECANCELED);
> + dma_fence_signal(f);
> + }
> +
> + list_del(&fence->link);
> + dma_fence_put(f);
> + }
> +
> + WARN_ON_ONCE(!list_empty(&fence_drv->fences));
> + spin_unlock(&fence_drv->fence_list_lock);
> +
> + /* Free seq64 memory */
> + amdgpu_seq64_free(adev, fence_drv->gpu_addr);
> + kfree(fence_drv);
> +}
> +
> +void amdgpu_userq_fence_driver_get(struct amdgpu_userq_fence_driver *fence_drv)
> +{
> + kref_get(&fence_drv->refcount);
> +}
> +
> +void amdgpu_userq_fence_driver_put(struct amdgpu_userq_fence_driver *fence_drv)
> +{
> + kref_put(&fence_drv->refcount, amdgpu_userq_fence_driver_destroy);
> +}
> +
> +int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
> + u64 seq, struct dma_fence **f)
> +{
> + struct amdgpu_userq_fence_driver *fence_drv;
> + struct amdgpu_userq_fence *userq_fence;
> + struct dma_fence *fence;
> +
> + fence_drv = userq->fence_drv;
> + if (!fence_drv)
> + return -EINVAL;
> +
> + userq_fence = kmem_cache_alloc(amdgpu_userq_fence_slab, GFP_ATOMIC);
> + if (!userq_fence)
> + return -ENOMEM;
> +
> + spin_lock_init(&userq_fence->lock);
> + INIT_LIST_HEAD(&userq_fence->link);
> + fence = &userq_fence->base;
> + userq_fence->fence_drv = fence_drv;
> +
> + dma_fence_init(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
> + fence_drv->context, seq);
> +
> + amdgpu_userq_fence_driver_get(fence_drv);
> + dma_fence_get(fence);
> +
> + spin_lock(&fence_drv->fence_list_lock);
> + /* Check if hardware has already processed the job */
> + if (!dma_fence_is_signaled(fence)) {
> + list_add_tail(&userq_fence->link, &fence_drv->fences);
> + } else {
> + dma_fence_put(fence);
> + }
> + spin_unlock(&fence_drv->fence_list_lock);
> +
> + *f = fence;
> +
> + return 0;
> +}
> +
> +static const char *amdgpu_userq_fence_get_driver_name(struct dma_fence *f)
> +{
> + return "amdgpu_userqueue_fence";
> +}
> +
> +static const char *amdgpu_userq_fence_get_timeline_name(struct dma_fence *f)
> +{
> + struct amdgpu_userq_fence *fence = to_amdgpu_userq_fence(f);
> +
> + return fence->fence_drv->timeline_name;
> +}
> +
> +static bool amdgpu_userq_fence_signaled(struct dma_fence *f)
> +{
> + struct amdgpu_userq_fence *fence = to_amdgpu_userq_fence(f);
> + struct amdgpu_userq_fence_driver *fence_drv = fence->fence_drv;
> + u64 rptr, wptr;
> +
> + rptr = amdgpu_userq_fence_read(fence_drv);
> + wptr = fence->base.seqno;
> +
> + if (rptr >= wptr)
> + return true;
> +
> + return false;
> +}
> +
> +static void amdgpu_userq_fence_free(struct rcu_head *rcu)
> +{
> + struct dma_fence *fence = container_of(rcu, struct dma_fence, rcu);
> + struct amdgpu_userq_fence *userq_fence = to_amdgpu_userq_fence(fence);
> + struct amdgpu_userq_fence_driver *fence_drv = userq_fence->fence_drv;
> +
> + /* Release the fence driver reference */
> + amdgpu_userq_fence_driver_put(fence_drv);
> + kmem_cache_free(amdgpu_userq_fence_slab, userq_fence);
> +}
> +
> +static void amdgpu_userq_fence_release(struct dma_fence *f)
> +{
> + call_rcu(&f->rcu, amdgpu_userq_fence_free);
> +}
> +
> +static const struct dma_fence_ops amdgpu_userq_fence_ops = {
> + .use_64bit_seqno = true,
> + .get_driver_name = amdgpu_userq_fence_get_driver_name,
> + .get_timeline_name = amdgpu_userq_fence_get_timeline_name,
> + .signaled = amdgpu_userq_fence_signaled,
> + .release = amdgpu_userq_fence_release,
> +};
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
> new file mode 100644
> index 000000000000..7329b4e5dd30
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
> @@ -0,0 +1,64 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright 2023 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_USERQ_FENCE_H__
> +#define __AMDGPU_USERQ_FENCE_H__
> +
> +#include <linux/types.h>
> +
> +#include "amdgpu_userqueue.h"
> +
> +struct amdgpu_userq_fence {
> + struct dma_fence base;
> + /* userq fence lock */
> + spinlock_t lock;
This one.
> + struct list_head link;
> + struct amdgpu_userq_fence_driver *fence_drv;
> +};
> +
> +struct amdgpu_userq_fence_driver {
> + struct kref refcount;
> + u64 gpu_addr;
> + u64 *cpu_addr;
> + u64 context;
> + /* fence list lock */
> + spinlock_t fence_list_lock;
And that one here should have better documentation.
It's obvious what they are, so the existing doc doesn't help at all.
What you need to document is why they are separate.
> + struct list_head fences;
> + struct amdgpu_device *adev;
> + char timeline_name[TASK_COMM_LEN];
> +};
> +
> +static const struct dma_fence_ops amdgpu_userq_fence_ops;
> +
> +int amdgpu_userq_fence_slab_init(void);
> +void amdgpu_userq_fence_slab_fini(void);
> +int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
> + u64 seq, struct dma_fence **f);
> +void amdgpu_userq_fence_driver_get(struct amdgpu_userq_fence_driver *fence_drv);
> +void amdgpu_userq_fence_driver_put(struct amdgpu_userq_fence_driver *fence_drv);
> +int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev, struct amdgpu_usermode_queue *userq);
> +void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_drv);
> +void amdgpu_userq_fence_driver_destroy(struct kref *ref);
> +
> +#endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
> index fd4a2ca3302d..8918b176fdcb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
> @@ -24,6 +24,7 @@
> #include "amdgpu.h"
> #include "amdgpu_vm.h"
> #include "amdgpu_userqueue.h"
> +#include "amdgpu_userq_fence.h"
>
> static inline int
> amdgpu_userqueue_index(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermode_queue *queue)
> @@ -158,6 +159,8 @@ static int amdgpu_userqueue_create(struct drm_file *filp, union drm_amdgpu_userq
> struct amdgpu_fpriv *fpriv = filp->driver_priv;
> struct amdgpu_userq_mgr *uq_mgr = &fpriv->userq_mgr;
> struct drm_amdgpu_userq_mqd *mqd_in = &args->in.mqd;
> + struct amdgpu_userq_fence_driver *fence_drv;
> + struct amdgpu_device *adev = uq_mgr->adev;
> uint64_t index;
> int r;
>
> @@ -173,6 +176,12 @@ static int amdgpu_userqueue_create(struct drm_file *filp, union drm_amdgpu_userq
> return -ENOMEM;
> }
>
> + fence_drv = kzalloc(sizeof(struct amdgpu_userq_fence_driver), GFP_KERNEL);
> + if (!fence_drv) {
> + DRM_ERROR("Failed to allocate memory for fence driver\n");
> + return -ENOMEM;
> + }
> +
Why don't we have this kzalloc() inside amdgpu_userq_fence_driver_alloc() ?
Christian.
> mutex_lock(&uq_mgr->userq_mutex);
> queue->userq_prop.wptr_gpu_addr = mqd_in->wptr_va;
> queue->userq_prop.rptr_gpu_addr = mqd_in->rptr_va;
> @@ -188,6 +197,13 @@ static int amdgpu_userqueue_create(struct drm_file *filp, union drm_amdgpu_userq
> goto free_queue;
> }
>
> + queue->fence_drv = fence_drv;
> + r = amdgpu_userq_fence_driver_alloc(adev, queue);
> + if (r) {
> + DRM_ERROR("Failed to alloc fence driver\n");
> + goto free_fence_drv;
> + }
> +
> queue->userq_prop.doorbell_index = index;
> queue->shadow_ctx_gpu_addr = mqd_in->shadow_va;
> queue->queue_type = mqd_in->ip_type;
> @@ -217,6 +233,8 @@ static int amdgpu_userqueue_create(struct drm_file *filp, union drm_amdgpu_userq
> mutex_unlock(&uq_mgr->userq_mutex);
> return 0;
>
> +free_fence_drv:
> + amdgpu_userq_fence_driver_put(queue->fence_drv);
> free_queue:
> mutex_unlock(&uq_mgr->userq_mutex);
> kfree(queue);
> @@ -238,6 +256,7 @@ static void amdgpu_userqueue_destroy(struct drm_file *filp, int queue_id)
> mutex_lock(&uq_mgr->userq_mutex);
> uq_mgr->userq_funcs[queue->queue_type]->mqd_destroy(uq_mgr, queue);
> amdgpu_userqueue_free_index(uq_mgr, queue->queue_id);
> + amdgpu_userq_fence_driver_put(queue->fence_drv);
> mutex_unlock(&uq_mgr->userq_mutex);
> kfree(queue);
> }
> diff --git a/drivers/gpu/drm/amd/include/amdgpu_userqueue.h b/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
> index eaab7cf5fff6..eaea88539007 100644
> --- a/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
> +++ b/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
> @@ -50,6 +50,7 @@ struct amdgpu_usermode_queue {
> struct amdgpu_mqd_prop userq_prop;
> struct amdgpu_userq_ctx_space mqd;
> struct amdgpu_userq_ctx_space fw_space;
> + struct amdgpu_userq_fence_driver *fence_drv;
> };
>
> struct amdgpu_userq_funcs {
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3 2/6] drm/amdgpu: Implement a new userqueue fence driver
2023-04-21 12:37 ` Christian König
@ 2023-04-24 19:09 ` Arunpravin Paneer Selvam
0 siblings, 0 replies; 11+ messages in thread
From: Arunpravin Paneer Selvam @ 2023-04-24 19:09 UTC (permalink / raw)
To: Christian König, amd-gfx; +Cc: alexander.deucher
On 4/21/2023 6:07 PM, Christian König wrote:
> Am 20.04.23 um 16:47 schrieb Arunpravin Paneer Selvam:
>> Developed a userqueue fence driver for the userqueue process shared
>> BO synchronization.
>>
>> Create a dma fence having write pointer as the seqno and allocate a
>> seq64 memory for each user queue process and feed this memory address
>> into the firmware/hardware, thus the firmware writes the read pointer
>> into the given address when the process completes it execution.
>> Compare wptr and rptr, if rptr >= wptr, signal the fences for the
>> waiting
>> process to consume the buffers.
>>
>> v2: Worked on review comments from Christian for the following
>> modifications
>>
>> - Add wptr as sequence number into the fence
>> - Add a reference count for the fence driver
>> - Add dma_fence_put below the list_del as it might frees the
>> userq fence.
>> - Trim unnecessary code in interrupt handler.
>> - Check dma fence signaled state in dma fence creation function
>> for a
>> potential problem of hardware completing the job processing
>> beforehand.
>> - Add necessary locks.
>> - Create a list and process all the unsignaled fences.
>> - clean up fences in destroy function.
>> - implement .signaled callback function
>>
>> v3: Worked on review comments from Christian
>> - Modify naming convention for reference counted objects
>> - Fix fence driver reference drop issue
>> - Drop amdgpu_userq_fence_driver_process() function return value
>
> That looks really good, just two more comments below.
>
>>
>> Signed-off-by: Arunpravin Paneer Selvam
>> <Arunpravin.PaneerSelvam@amd.com>
>> ---
>> build.sh | 11 +
>> drivers/gpu/drm/amd/amdgpu/Makefile | 2 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 +
>> .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 253 ++++++++++++++++++
>> .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 64 +++++
>> drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 19 ++
>> .../gpu/drm/amd/include/amdgpu_userqueue.h | 1 +
>> 7 files changed, 355 insertions(+), 1 deletion(-)
>> create mode 100755 build.sh
>> create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>> create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
>>
>> diff --git a/build.sh b/build.sh
>> new file mode 100755
>> index 000000000000..fddaff75dd2a
>> --- /dev/null
>> +++ b/build.sh
>> @@ -0,0 +1,11 @@
>> +#make -j16 modules M=drivers/gpu/drm/
>> +#make -j16 modules M=drivers/gpu/drm/selftests/
>> +make -j16 modules M=drivers/gpu/drm/amd/amdgpu/
>> +#make -j16 modules M=drivers/gpu/drm/ttm/
>> +#cp drivers/gpu/drm/drm.ko /lib/modules/$(uname
>> -r)/kernel/drivers/gpu/drm/
>> +#cp drivers/gpu/drm/selftests/test-drm_buddy.ko /lib/modules/$(uname
>> -r)/kernel/drivers/gpu/drm/selftests/
>> +#cp drivers/gpu/drm/amd/amdgpu/amdgpu.ko /lib/modules/$(uname
>> -r)/kernel/drivers/gpu/drm/amd/amdgpu/
>> +#cp drivers/gpu/drm/ttm/ttm.ko /lib/modules/$(uname
>> -r)/kernel/drivers/gpu/drm/ttm/
>> +#update-initramfs -c -k $(uname -r)
>> +#reboot
>> +
>
> I strongly assume you didn't intentionally committed that, did you?
>
> Anyway, please remove :)
I committed this script by mistake :D I will remove it
>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile
>> b/drivers/gpu/drm/amd/amdgpu/Makefile
>> index d39504e65db1..8ed9be0d4818 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
>> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
>> @@ -59,7 +59,7 @@ amdgpu-y += amdgpu_device.o amdgpu_doorbell_mgr.o
>> amdgpu_kms.o \
>> amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
>> 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_seq64.o
>> + amdgpu_ring_mux.o amdgpu_seq64.o amdgpu_userq_fence.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 229976a2d0e7..e9c5047087d0 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -53,6 +53,7 @@
>> #include "amdgpu_xgmi.h"
>> #include "amdgpu_reset.h"
>> #include "amdgpu_userqueue.h"
>> +#include "amdgpu_userq_fence.h"
>> /*
>> * KMS wrapper.
>> @@ -2826,6 +2827,10 @@ static int __init amdgpu_init(void)
>> if (r)
>> goto error_fence;
>> + r = amdgpu_userq_fence_slab_init();
>> + if (r)
>> + goto error_fence;
>> +
>> DRM_INFO("amdgpu kernel modesetting enabled.\n");
>> amdgpu_register_atpx_handler();
>> amdgpu_acpi_detect();
>> @@ -2850,6 +2855,7 @@ static void __exit amdgpu_exit(void)
>> amdgpu_unregister_atpx_handler();
>> amdgpu_sync_fini();
>> amdgpu_fence_slab_fini();
>> + amdgpu_userq_fence_slab_fini();
>> mmu_notifier_synchronize();
>> }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>> new file mode 100644
>> index 000000000000..a03d12f83147
>> --- /dev/null
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>> @@ -0,0 +1,253 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright 2023 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/kref.h>
>> +#include <linux/slab.h>
>> +
>> +#include <drm/drm_syncobj.h>
>> +
>> +#include "amdgpu.h"
>> +#include "amdgpu_userq_fence.h"
>> +
>> +static struct kmem_cache *amdgpu_userq_fence_slab;
>> +
>> +int amdgpu_userq_fence_slab_init(void)
>> +{
>> + amdgpu_userq_fence_slab = kmem_cache_create("amdgpu_userq_fence",
>> + sizeof(struct amdgpu_userq_fence),
>> + 0,
>> + SLAB_HWCACHE_ALIGN,
>> + NULL);
>> + if (!amdgpu_userq_fence_slab)
>> + return -ENOMEM;
>> +
>> + return 0;
>> +}
>> +
>> +void amdgpu_userq_fence_slab_fini(void)
>> +{
>> + rcu_barrier();
>> + kmem_cache_destroy(amdgpu_userq_fence_slab);
>> +}
>> +
>> +static inline struct amdgpu_userq_fence
>> *to_amdgpu_userq_fence(struct dma_fence *f)
>> +{
>> + struct amdgpu_userq_fence *__f = container_of(f, struct
>> amdgpu_userq_fence, base);
>> +
>> + if (!__f)
>> + return NULL;
>> +
>> + if (__f->base.ops == &amdgpu_userq_fence_ops)
>> + return __f;
>> +
>> + return NULL;
>> +}
>> +
>> +static u64 amdgpu_userq_fence_read(struct amdgpu_userq_fence_driver
>> *fence_drv)
>> +{
>> + return le64_to_cpu(*fence_drv->cpu_addr);
>> +}
>> +
>> +int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
>> + struct amdgpu_usermode_queue *userq)
>> +{
>> + struct amdgpu_userq_fence_driver *fence_drv;
>> + int r;
>> +
>> + fence_drv = userq->fence_drv;
>> + if (!fence_drv)
>> + return -EINVAL;
>> +
>> + /* Acquire seq64 memory */
>> + r = amdgpu_seq64_get(adev, &fence_drv->gpu_addr,
>> + &fence_drv->cpu_addr);
>> + if (r)
>> + return -ENOMEM;
>> +
>> + kref_init(&fence_drv->refcount);
>> + INIT_LIST_HEAD(&fence_drv->fences);
>> + spin_lock_init(&fence_drv->fence_list_lock);
>> +
>> + fence_drv->adev = adev;
>> + fence_drv->context = dma_fence_context_alloc(1);
>> +
>> + get_task_comm(fence_drv->timeline_name, current);
>> +
>> + return 0;
>> +}
>> +
>> +void amdgpu_userq_fence_driver_process(struct
>> amdgpu_userq_fence_driver *fence_drv)
>> +{
>> + struct amdgpu_userq_fence *userq_fence, *tmp;
>> + struct dma_fence *fence;
>> +
>> + if (!fence_drv)
>> + return;
>> +
>> + spin_lock(&fence_drv->fence_list_lock);
>> + list_for_each_entry_safe(userq_fence, tmp, &fence_drv->fences,
>> link) {
>> + fence = &userq_fence->base;
>> +
>> + if (amdgpu_userq_fence_read(fence_drv) >= fence->seqno) {
>> + dma_fence_signal(fence);
>> + list_del(&userq_fence->link);
>> +
>> + dma_fence_put(fence);
>> + } else {
>> + break;
>> + }
>> + }
>> + spin_unlock(&fence_drv->fence_list_lock);
>> +}
>> +
>> +void amdgpu_userq_fence_driver_destroy(struct kref *ref)
>> +{
>> + struct amdgpu_userq_fence_driver *fence_drv = container_of(ref,
>> + struct amdgpu_userq_fence_driver,
>> + refcount);
>> + struct amdgpu_device *adev = fence_drv->adev;
>> + struct amdgpu_userq_fence *fence, *tmp;
>> + struct dma_fence *f;
>> +
>> + spin_lock(&fence_drv->fence_list_lock);
>> + list_for_each_entry_safe(fence, tmp, &fence_drv->fences, link) {
>> + f = &fence->base;
>> +
>> + if (!dma_fence_is_signaled(f)) {
>> + dma_fence_set_error(f, -ECANCELED);
>> + dma_fence_signal(f);
>> + }
>> +
>> + list_del(&fence->link);
>> + dma_fence_put(f);
>> + }
>> +
>> + WARN_ON_ONCE(!list_empty(&fence_drv->fences));
>> + spin_unlock(&fence_drv->fence_list_lock);
>> +
>> + /* Free seq64 memory */
>> + amdgpu_seq64_free(adev, fence_drv->gpu_addr);
>> + kfree(fence_drv);
>> +}
>> +
>> +void amdgpu_userq_fence_driver_get(struct amdgpu_userq_fence_driver
>> *fence_drv)
>> +{
>> + kref_get(&fence_drv->refcount);
>> +}
>> +
>> +void amdgpu_userq_fence_driver_put(struct amdgpu_userq_fence_driver
>> *fence_drv)
>> +{
>> + kref_put(&fence_drv->refcount, amdgpu_userq_fence_driver_destroy);
>> +}
>> +
>> +int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
>> + u64 seq, struct dma_fence **f)
>> +{
>> + struct amdgpu_userq_fence_driver *fence_drv;
>> + struct amdgpu_userq_fence *userq_fence;
>> + struct dma_fence *fence;
>> +
>> + fence_drv = userq->fence_drv;
>> + if (!fence_drv)
>> + return -EINVAL;
>> +
>> + userq_fence = kmem_cache_alloc(amdgpu_userq_fence_slab,
>> GFP_ATOMIC);
>> + if (!userq_fence)
>> + return -ENOMEM;
>> +
>> + spin_lock_init(&userq_fence->lock);
>> + INIT_LIST_HEAD(&userq_fence->link);
>> + fence = &userq_fence->base;
>> + userq_fence->fence_drv = fence_drv;
>> +
>> + dma_fence_init(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
>> + fence_drv->context, seq);
>> +
>> + amdgpu_userq_fence_driver_get(fence_drv);
>> + dma_fence_get(fence);
>> +
>> + spin_lock(&fence_drv->fence_list_lock);
>> + /* Check if hardware has already processed the job */
>> + if (!dma_fence_is_signaled(fence)) {
>> + list_add_tail(&userq_fence->link, &fence_drv->fences);
>> + } else {
>> + dma_fence_put(fence);
>> + }
>> + spin_unlock(&fence_drv->fence_list_lock);
>> +
>> + *f = fence;
>> +
>> + return 0;
>> +}
>> +
>> +static const char *amdgpu_userq_fence_get_driver_name(struct
>> dma_fence *f)
>> +{
>> + return "amdgpu_userqueue_fence";
>> +}
>> +
>> +static const char *amdgpu_userq_fence_get_timeline_name(struct
>> dma_fence *f)
>> +{
>> + struct amdgpu_userq_fence *fence = to_amdgpu_userq_fence(f);
>> +
>> + return fence->fence_drv->timeline_name;
>> +}
>> +
>> +static bool amdgpu_userq_fence_signaled(struct dma_fence *f)
>> +{
>> + struct amdgpu_userq_fence *fence = to_amdgpu_userq_fence(f);
>> + struct amdgpu_userq_fence_driver *fence_drv = fence->fence_drv;
>> + u64 rptr, wptr;
>> +
>> + rptr = amdgpu_userq_fence_read(fence_drv);
>> + wptr = fence->base.seqno;
>> +
>> + if (rptr >= wptr)
>> + return true;
>> +
>> + return false;
>> +}
>> +
>> +static void amdgpu_userq_fence_free(struct rcu_head *rcu)
>> +{
>> + struct dma_fence *fence = container_of(rcu, struct dma_fence, rcu);
>> + struct amdgpu_userq_fence *userq_fence =
>> to_amdgpu_userq_fence(fence);
>> + struct amdgpu_userq_fence_driver *fence_drv =
>> userq_fence->fence_drv;
>> +
>> + /* Release the fence driver reference */
>> + amdgpu_userq_fence_driver_put(fence_drv);
>> + kmem_cache_free(amdgpu_userq_fence_slab, userq_fence);
>> +}
>> +
>> +static void amdgpu_userq_fence_release(struct dma_fence *f)
>> +{
>> + call_rcu(&f->rcu, amdgpu_userq_fence_free);
>> +}
>> +
>> +static const struct dma_fence_ops amdgpu_userq_fence_ops = {
>> + .use_64bit_seqno = true,
>> + .get_driver_name = amdgpu_userq_fence_get_driver_name,
>> + .get_timeline_name = amdgpu_userq_fence_get_timeline_name,
>> + .signaled = amdgpu_userq_fence_signaled,
>> + .release = amdgpu_userq_fence_release,
>> +};
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
>> new file mode 100644
>> index 000000000000..7329b4e5dd30
>> --- /dev/null
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
>> @@ -0,0 +1,64 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright 2023 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_USERQ_FENCE_H__
>> +#define __AMDGPU_USERQ_FENCE_H__
>> +
>> +#include <linux/types.h>
>> +
>> +#include "amdgpu_userqueue.h"
>> +
>> +struct amdgpu_userq_fence {
>> + struct dma_fence base;
>> + /* userq fence lock */
>> + spinlock_t lock;
>
> This one.
>
>> + struct list_head link;
>> + struct amdgpu_userq_fence_driver *fence_drv;
>> +};
>> +
>> +struct amdgpu_userq_fence_driver {
>> + struct kref refcount;
>> + u64 gpu_addr;
>> + u64 *cpu_addr;
>> + u64 context;
>> + /* fence list lock */
>> + spinlock_t fence_list_lock;
>
> And that one here should have better documentation.
>
> It's obvious what they are, so the existing doc doesn't help at all.
>
> What you need to document is why they are separate.
I will add the required documentation.
>
>> + struct list_head fences;
>> + struct amdgpu_device *adev;
>> + char timeline_name[TASK_COMM_LEN];
>> +};
>> +
>> +static const struct dma_fence_ops amdgpu_userq_fence_ops;
>> +
>> +int amdgpu_userq_fence_slab_init(void);
>> +void amdgpu_userq_fence_slab_fini(void);
>> +int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
>> + u64 seq, struct dma_fence **f);
>> +void amdgpu_userq_fence_driver_get(struct amdgpu_userq_fence_driver
>> *fence_drv);
>> +void amdgpu_userq_fence_driver_put(struct amdgpu_userq_fence_driver
>> *fence_drv);
>> +int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
>> struct amdgpu_usermode_queue *userq);
>> +void amdgpu_userq_fence_driver_process(struct
>> amdgpu_userq_fence_driver *fence_drv);
>> +void amdgpu_userq_fence_driver_destroy(struct kref *ref);
>> +
>> +#endif
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
>> index fd4a2ca3302d..8918b176fdcb 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
>> @@ -24,6 +24,7 @@
>> #include "amdgpu.h"
>> #include "amdgpu_vm.h"
>> #include "amdgpu_userqueue.h"
>> +#include "amdgpu_userq_fence.h"
>> static inline int
>> amdgpu_userqueue_index(struct amdgpu_userq_mgr *uq_mgr, struct
>> amdgpu_usermode_queue *queue)
>> @@ -158,6 +159,8 @@ static int amdgpu_userqueue_create(struct
>> drm_file *filp, union drm_amdgpu_userq
>> struct amdgpu_fpriv *fpriv = filp->driver_priv;
>> struct amdgpu_userq_mgr *uq_mgr = &fpriv->userq_mgr;
>> struct drm_amdgpu_userq_mqd *mqd_in = &args->in.mqd;
>> + struct amdgpu_userq_fence_driver *fence_drv;
>> + struct amdgpu_device *adev = uq_mgr->adev;
>> uint64_t index;
>> int r;
>> @@ -173,6 +176,12 @@ static int amdgpu_userqueue_create(struct
>> drm_file *filp, union drm_amdgpu_userq
>> return -ENOMEM;
>> }
>> + fence_drv = kzalloc(sizeof(struct amdgpu_userq_fence_driver),
>> GFP_KERNEL);
>> + if (!fence_drv) {
>> + DRM_ERROR("Failed to allocate memory for fence driver\n");
>> + return -ENOMEM;
>> + }
>> +
>
> Why don't we have this kzalloc() inside
> amdgpu_userq_fence_driver_alloc() ?
Christian.
That's better. I moved kzalloc() inside amdgpu_userq_fence_driver_alloc().
Thanks,
Arun
>
>
>
>> mutex_lock(&uq_mgr->userq_mutex);
>> queue->userq_prop.wptr_gpu_addr = mqd_in->wptr_va;
>> queue->userq_prop.rptr_gpu_addr = mqd_in->rptr_va;
>> @@ -188,6 +197,13 @@ static int amdgpu_userqueue_create(struct
>> drm_file *filp, union drm_amdgpu_userq
>> goto free_queue;
>> }
>> + queue->fence_drv = fence_drv;
>> + r = amdgpu_userq_fence_driver_alloc(adev, queue);
>> + if (r) {
>> + DRM_ERROR("Failed to alloc fence driver\n");
>> + goto free_fence_drv;
>> + }
>> +
>> queue->userq_prop.doorbell_index = index;
>> queue->shadow_ctx_gpu_addr = mqd_in->shadow_va;
>> queue->queue_type = mqd_in->ip_type;
>> @@ -217,6 +233,8 @@ static int amdgpu_userqueue_create(struct
>> drm_file *filp, union drm_amdgpu_userq
>> mutex_unlock(&uq_mgr->userq_mutex);
>> return 0;
>> +free_fence_drv:
>> + amdgpu_userq_fence_driver_put(queue->fence_drv);
>> free_queue:
>> mutex_unlock(&uq_mgr->userq_mutex);
>> kfree(queue);
>> @@ -238,6 +256,7 @@ static void amdgpu_userqueue_destroy(struct
>> drm_file *filp, int queue_id)
>> mutex_lock(&uq_mgr->userq_mutex);
>> uq_mgr->userq_funcs[queue->queue_type]->mqd_destroy(uq_mgr, queue);
>> amdgpu_userqueue_free_index(uq_mgr, queue->queue_id);
>> + amdgpu_userq_fence_driver_put(queue->fence_drv);
>> mutex_unlock(&uq_mgr->userq_mutex);
>> kfree(queue);
>> }
>> diff --git a/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
>> b/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
>> index eaab7cf5fff6..eaea88539007 100644
>> --- a/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
>> +++ b/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
>> @@ -50,6 +50,7 @@ struct amdgpu_usermode_queue {
>> struct amdgpu_mqd_prop userq_prop;
>> struct amdgpu_userq_ctx_space mqd;
>> struct amdgpu_userq_ctx_space fw_space;
>> + struct amdgpu_userq_fence_driver *fence_drv;
>> };
>> struct amdgpu_userq_funcs {
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 3/6] drm/amdgpu: Add mqd support for the fence address
2023-04-20 14:47 [PATCH v3 0/6] Usermode queue fencing synchronization Arunpravin Paneer Selvam
2023-04-20 14:47 ` [PATCH v3 1/6] drm/amdgpu: Implement a new 64bit sequence memory driver Arunpravin Paneer Selvam
2023-04-20 14:47 ` [PATCH v3 2/6] drm/amdgpu: Implement a new userqueue fence driver Arunpravin Paneer Selvam
@ 2023-04-20 14:47 ` Arunpravin Paneer Selvam
2023-04-20 14:47 ` [PATCH v3 4/6] drm/amdgpu: UAPI headers for userqueue Secure semaphore Arunpravin Paneer Selvam
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Arunpravin Paneer Selvam @ 2023-04-20 14:47 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, christian.koenig, Arunpravin Paneer Selvam
- Add a field in struct v11_gfx_mqd for userqueue
fence address.
- Assign fence gpu VA address to the userqueue mqd
fence address fields.
v2: Remove the mask and replace with lower_32_bits (Christian)
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
.../gpu/drm/amd/amdgpu/amdgpu_userqueue_gfx_v11.c | 12 ++++++++++++
drivers/gpu/drm/amd/include/v11_structs.h | 4 ++--
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue_gfx_v11.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue_gfx_v11.c
index 274e78826334..a9d41407bab1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue_gfx_v11.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue_gfx_v11.c
@@ -22,6 +22,7 @@
*/
#include "amdgpu.h"
#include "amdgpu_userqueue.h"
+#include "amdgpu_userq_fence.h"
#include "v11_structs.h"
#include "amdgpu_mes.h"
@@ -131,6 +132,16 @@ static void amdgpu_userq_gfx_v11_destroy_ctx_space(struct amdgpu_userq_mgr *uq_m
&ctx->cpu_ptr);
}
+static void
+amdgpu_userq_set_fence_space(struct amdgpu_userq_mgr *uq_mgr,
+ struct amdgpu_usermode_queue *queue)
+{
+ struct v11_gfx_mqd *mqd = queue->mqd.cpu_ptr;
+
+ mqd->fenceaddress_lo = lower_32_bits(queue->fence_drv->gpu_addr);
+ mqd->fenceaddress_hi = upper_32_bits(queue->fence_drv->gpu_addr);
+}
+
static void
amdgpu_userq_set_ctx_space(struct amdgpu_userq_mgr *uq_mgr,
struct amdgpu_usermode_queue *queue)
@@ -190,6 +201,7 @@ amdgpu_userq_gfx_v11_mqd_create(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_u
}
amdgpu_userq_set_ctx_space(uq_mgr, queue);
+ amdgpu_userq_set_fence_space(uq_mgr, queue);
amdgpu_bo_unreserve(mqd->obj);
/* Map the queue in HW using MES ring */
diff --git a/drivers/gpu/drm/amd/include/v11_structs.h b/drivers/gpu/drm/amd/include/v11_structs.h
index f8008270f813..797ce6a1e56e 100644
--- a/drivers/gpu/drm/amd/include/v11_structs.h
+++ b/drivers/gpu/drm/amd/include/v11_structs.h
@@ -535,8 +535,8 @@ struct v11_gfx_mqd {
uint32_t reserved_507; // offset: 507 (0x1FB)
uint32_t reserved_508; // offset: 508 (0x1FC)
uint32_t reserved_509; // offset: 509 (0x1FD)
- uint32_t reserved_510; // offset: 510 (0x1FE)
- uint32_t reserved_511; // offset: 511 (0x1FF)
+ uint32_t fenceaddress_lo; // offset: 510 (0x1FE)
+ uint32_t fenceaddress_hi; // offset: 511 (0x1FF)
};
struct v11_sdma_mqd {
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 4/6] drm/amdgpu: UAPI headers for userqueue Secure semaphore
2023-04-20 14:47 [PATCH v3 0/6] Usermode queue fencing synchronization Arunpravin Paneer Selvam
` (2 preceding siblings ...)
2023-04-20 14:47 ` [PATCH v3 3/6] drm/amdgpu: Add mqd support for the fence address Arunpravin Paneer Selvam
@ 2023-04-20 14:47 ` Arunpravin Paneer Selvam
2023-04-20 14:47 ` [PATCH v3 5/6] drm/amdgpu: Implement userqueue signal/wait IOCTL functions Arunpravin Paneer Selvam
2023-04-20 14:47 ` [PATCH v3 6/6] drm/amdgpu: Enable userqueue fence interrupt handling support Arunpravin Paneer Selvam
5 siblings, 0 replies; 11+ messages in thread
From: Arunpravin Paneer Selvam @ 2023-04-20 14:47 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, christian.koenig, Arunpravin Paneer Selvam
- Add UAPI header support for userqueue Secure semaphore
v2: Worked on review comments from Christian for the
following modifications
- Add bo handles, bo flags and padding fields.
- Include value/va in a combined array.
v3: Worked on review comments from Christian
- Fix padding issues
- Add a number variable (num_fences) to get how much
space we need to allocate for userq_fence_info
- Write better name for the variables
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
include/uapi/drm/amdgpu_drm.h | 47 +++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index e4943099b9d2..39a9203908d8 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -55,6 +55,8 @@ extern "C" {
#define DRM_AMDGPU_FENCE_TO_HANDLE 0x14
#define DRM_AMDGPU_SCHED 0x15
#define DRM_AMDGPU_USERQ 0x16
+#define DRM_AMDGPU_USERQ_SIGNAL 0x17
+#define DRM_AMDGPU_USERQ_WAIT 0x18
#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)
@@ -73,6 +75,8 @@ extern "C" {
#define DRM_IOCTL_AMDGPU_FENCE_TO_HANDLE DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_FENCE_TO_HANDLE, union drm_amdgpu_fence_to_handle)
#define DRM_IOCTL_AMDGPU_SCHED DRM_IOW(DRM_COMMAND_BASE + DRM_AMDGPU_SCHED, union drm_amdgpu_sched)
#define DRM_IOCTL_AMDGPU_USERQ DRM_IOW(DRM_COMMAND_BASE + DRM_AMDGPU_USERQ, union drm_amdgpu_userq)
+#define DRM_IOCTL_AMDGPU_USERQ_SIGNAL DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_USERQ_SIGNAL, struct drm_amdgpu_userq_signal)
+#define DRM_IOCTL_AMDGPU_USERQ_WAIT DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_USERQ_WAIT, struct drm_amdgpu_userq_wait)
/**
* DOC: memory domains
@@ -362,6 +366,49 @@ union drm_amdgpu_userq {
struct drm_amdgpu_userq_out out;
};
+/* userq signal/wait ioctl */
+struct drm_amdgpu_userq_signal {
+ /** Queue ID */
+ __u32 queue_id;
+ /** Flags */
+ __u32 flags;
+ /** Sync obj handle */
+ __u32 syncobj_handle;
+ __u32 pad;
+ /* Sync obj timeline */
+ __u64 syncobj_point;
+ /** array of BO handles */
+ __u64 bo_handles_array;
+ /** number of BO handles */
+ __u32 num_bo_handles;
+ /** bo flags */
+ __u32 bo_flags;
+};
+
+struct drm_amdgpu_userq_fence_info {
+ __u64 va;
+ __u64 value;
+};
+
+struct drm_amdgpu_userq_wait {
+ /** Flags */
+ __u32 flags;
+ /** bo flags */
+ __u32 bo_wait_flags;
+ /** array of Sync obj handles */
+ __u64 syncobj_handles_array;
+ /** array of BO handles */
+ __u64 bo_handles_array;
+ /** number of Sync obj handles */
+ __u32 num_syncobj_handles;
+ /** number of BO handles */
+ __u32 num_bo_handles;
+ /** array of addr/values */
+ __u64 userq_fence_info;
+ /** number of fences */
+ __u64 num_fences;
+};
+
/* vm ioctl */
#define AMDGPU_VM_OP_RESERVE_VMID 1
#define AMDGPU_VM_OP_UNRESERVE_VMID 2
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 5/6] drm/amdgpu: Implement userqueue signal/wait IOCTL functions
2023-04-20 14:47 [PATCH v3 0/6] Usermode queue fencing synchronization Arunpravin Paneer Selvam
` (3 preceding siblings ...)
2023-04-20 14:47 ` [PATCH v3 4/6] drm/amdgpu: UAPI headers for userqueue Secure semaphore Arunpravin Paneer Selvam
@ 2023-04-20 14:47 ` Arunpravin Paneer Selvam
2023-04-21 13:20 ` Christian König
2023-04-20 14:47 ` [PATCH v3 6/6] drm/amdgpu: Enable userqueue fence interrupt handling support Arunpravin Paneer Selvam
5 siblings, 1 reply; 11+ messages in thread
From: Arunpravin Paneer Selvam @ 2023-04-20 14:47 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, christian.koenig, Arunpravin Paneer Selvam
This patch introduces new IOCTL for userqueue secure semaphore.
The signal IOCTL called from userspace application creates a drm
syncobj and array of bo GEM handles and passed in as parameter to
the driver to install the fence into it.
The wait IOCTL gets an array of drm syncobjs, finds the fences
attached to the drm syncobjs and obtain the array of
memory_address/fence_value combintion which are returned to
userspace.
v2: Worked on review comments from Christian for the following
modifications
- Install fence into GEM BO object.
- Lock all BO's using the dma resv subsystem
- Reorder the sequence in signal IOCTL function.
- Get write pointer from the shadow wptr
- use userq_fence to fetch the va/value in wait IOCTL.
v3: Worked on review comments from Christian
- Use drm_exec helper for the proper BO drm reserve and avoid BO
lock/unlock issues.
- fence/fence driver reference count logic for signal/wait IOCTLs.
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 312 +++++++++++++++++-
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 6 +
drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 1 +
5 files changed, 315 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 3bc8a2d35bb3..1d8a762f43c6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -970,6 +970,8 @@ struct amdgpu_device {
struct amdgpu_mes mes;
struct amdgpu_mqd mqds[AMDGPU_HW_IP_NUM];
+ struct amdgpu_userq_mgr *userq_mgr;
+
/* df */
struct amdgpu_df df;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index e9c5047087d0..b59e31845568 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2751,6 +2751,8 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = {
DRM_IOCTL_DEF_DRV(AMDGPU_GEM_OP, amdgpu_gem_op_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(AMDGPU_GEM_USERPTR, amdgpu_gem_userptr_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(AMDGPU_USERQ, amdgpu_userq_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_SIGNAL, amdgpu_userq_signal_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_WAIT, amdgpu_userq_wait_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
};
static const struct drm_driver amdgpu_kms_driver = {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index a03d12f83147..1c455b7ebcd6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -25,6 +25,7 @@
#include <linux/kref.h>
#include <linux/slab.h>
+#include <drm/drm_exec.h>
#include <drm/drm_syncobj.h>
#include "amdgpu.h"
@@ -84,7 +85,7 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
&fence_drv->cpu_addr);
if (r)
return -ENOMEM;
-
+
kref_init(&fence_drv->refcount);
INIT_LIST_HEAD(&fence_drv->fences);
spin_lock_init(&fence_drv->fence_list_lock);
@@ -124,28 +125,27 @@ void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_d
void amdgpu_userq_fence_driver_destroy(struct kref *ref)
{
struct amdgpu_userq_fence_driver *fence_drv = container_of(ref,
- struct amdgpu_userq_fence_driver,
- refcount);
+ struct amdgpu_userq_fence_driver,
+ refcount);
struct amdgpu_device *adev = fence_drv->adev;
struct amdgpu_userq_fence *fence, *tmp;
struct dma_fence *f;
-
+
spin_lock(&fence_drv->fence_list_lock);
list_for_each_entry_safe(fence, tmp, &fence_drv->fences, link) {
f = &fence->base;
-
+
if (!dma_fence_is_signaled(f)) {
dma_fence_set_error(f, -ECANCELED);
dma_fence_signal(f);
}
-
+
list_del(&fence->link);
dma_fence_put(f);
}
-
+
WARN_ON_ONCE(!list_empty(&fence_drv->fences));
spin_unlock(&fence_drv->fence_list_lock);
-
/* Free seq64 memory */
amdgpu_seq64_free(adev, fence_drv->gpu_addr);
kfree(fence_drv);
@@ -161,6 +161,11 @@ void amdgpu_userq_fence_driver_put(struct amdgpu_userq_fence_driver *fence_drv)
kref_put(&fence_drv->refcount, amdgpu_userq_fence_driver_destroy);
}
+static inline bool amdgpu_userq_fence_test_signaled(struct dma_fence *fence)
+{
+ return test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
+}
+
int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
u64 seq, struct dma_fence **f)
{
@@ -251,3 +256,294 @@ static const struct dma_fence_ops amdgpu_userq_fence_ops = {
.signaled = amdgpu_userq_fence_signaled,
.release = amdgpu_userq_fence_release,
};
+
+static int amdgpu_userq_fence_read_wptr(struct drm_file *filp,
+ struct amdgpu_usermode_queue *queue,
+ u64 *wptr)
+{
+ struct amdgpu_fpriv *fpriv = filp->driver_priv;
+ struct amdgpu_bo_va_mapping *mapping;
+ struct amdgpu_vm *vm = &fpriv->vm;
+ struct amdgpu_bo *bo;
+ u64 addr, *ptr;
+ int r;
+
+ addr = queue->userq_prop.wptr_gpu_addr >> PAGE_SHIFT;
+
+ mapping = amdgpu_vm_bo_lookup_mapping(vm, addr);
+ if (!mapping)
+ return -EINVAL;
+
+ bo = mapping->bo_va->base.bo;
+ r = amdgpu_bo_reserve(bo, true);
+ if (r) {
+ DRM_ERROR("Failed to reserve userqueue wptr bo");
+ return r;
+ }
+
+ r = amdgpu_bo_kmap(bo, (void **)&ptr);
+ if (r) {
+ DRM_ERROR("Failed mapping the userqueue wptr bo");
+ goto map_error;
+ }
+
+ *wptr = le64_to_cpu(*ptr);
+
+ amdgpu_bo_kunmap(bo);
+ amdgpu_bo_unreserve(bo);
+
+ return 0;
+
+map_error:
+ amdgpu_bo_unreserve(bo);
+ return r;
+}
+
+int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *filp)
+{
+ struct drm_amdgpu_userq_signal *args = data;
+ struct amdgpu_device *adev = drm_to_adev(dev);
+ struct amdgpu_userq_mgr *userq_mgr = adev->userq_mgr;
+ struct amdgpu_usermode_queue *queue;
+ struct drm_gem_object **gobj = NULL;
+ struct drm_syncobj *syncobj = NULL;
+ u32 *bo_handles, num_bo_handles;
+ struct dma_fence *fence;
+ struct drm_exec exec;
+ u64 wptr;
+ int r, i;
+
+ /* Retrieve the user queue */
+ queue = idr_find(&userq_mgr->userq_idr, args->queue_id);
+ if (!queue)
+ return -ENOENT;
+
+ r = amdgpu_userq_fence_read_wptr(filp, queue, &wptr);
+ if (r)
+ return -EINVAL;
+
+ /* Find Syncobj if any */
+ syncobj = drm_syncobj_find(filp, args->syncobj_handle);
+
+ /* Array of bo handles */
+ num_bo_handles = args->num_bo_handles;
+ bo_handles = kmalloc_array(num_bo_handles, sizeof(*bo_handles), GFP_KERNEL);
+ if (!bo_handles)
+ return -ENOMEM;
+
+ if (copy_from_user(bo_handles, u64_to_user_ptr(args->bo_handles_array),
+ sizeof(u32) * num_bo_handles)) {
+ r = -EFAULT;
+ goto cleanup_bo_handles;
+ }
+
+ /* Create a new fence */
+ r = amdgpu_userq_fence_create(queue, wptr, &fence);
+ if (!fence)
+ goto cleanup_bo_handles;
+
+ if (amdgpu_userq_fence_test_signaled(fence))
+ goto signaled;
+
+ /* Add the created fence to syncobj/BO's */
+ if (syncobj) {
+ drm_syncobj_replace_fence(syncobj, fence);
+ dma_fence_put(fence);
+ }
+
+ /* Array of GEM object handles */
+ gobj = kmalloc_array(num_bo_handles, sizeof(*gobj), GFP_KERNEL);
+ if (!gobj) {
+ r = -ENOMEM;
+ goto cleanup_bo_handles;
+ }
+
+ drm_exec_init(&exec, true);
+ drm_exec_while_not_all_locked(&exec) {
+ for (i = 0; i < num_bo_handles; i++) {
+ /* Retrieve GEM object */
+ gobj[i] = drm_gem_object_lookup(filp, bo_handles[i]);
+ if (!gobj[i]) {
+ r = -ENOENT;
+ goto exec_fini;
+ }
+
+ r = drm_exec_prepare_obj(&exec, gobj[i], 1);
+ drm_exec_continue_on_contention(&exec);
+ if (unlikely(r))
+ goto exec_fini;
+
+ dma_resv_add_fence(gobj[i]->resv, fence,
+ args->bo_flags & AMDGPU_USERQ_BO_READ ?
+ DMA_RESV_USAGE_READ :
+ DMA_RESV_USAGE_WRITE);
+ }
+ }
+
+exec_fini:
+ drm_exec_fini(&exec);
+signaled:
+ dma_fence_put(fence);
+ kfree(gobj);
+cleanup_bo_handles:
+ kfree(bo_handles);
+
+ return r;
+}
+
+int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *filp)
+{
+ struct drm_amdgpu_userq_fence_info *fence_info = NULL;
+ struct drm_amdgpu_userq_wait *wait_info = data;
+ u32 *syncobj_handles, *bo_handles;
+ struct dma_resv_iter resv_cursor;
+ u32 num_syncobj, num_bo_handles;
+ struct drm_gem_object **gobj;
+ u64 num_fences = 0;
+ int i, j, r, cur;
+ bool bo_flag;
+
+ num_bo_handles = wait_info->num_bo_handles;
+ /* Array of GEM BO handles */
+ bo_handles = kmalloc_array(num_bo_handles, sizeof(*bo_handles), GFP_KERNEL);
+ if (!bo_handles)
+ return -ENOMEM;
+
+ if (copy_from_user(bo_handles, u64_to_user_ptr(wait_info->bo_handles_array),
+ sizeof(u32) * num_bo_handles)) {
+ r = -EFAULT;
+ goto err_free_bo_handles;
+ }
+
+ /* Array of GEM object handles */
+ gobj = kmalloc_array(num_bo_handles, sizeof(*gobj), GFP_KERNEL);
+ if (!gobj) {
+ r = -ENOMEM;
+ goto err_free_fence_info;
+ }
+
+ if (wait_info->num_fences) {
+ /* Array of fence gpu address */
+ fence_info = kmalloc_array(wait_info->num_fences, sizeof(*fence_info), GFP_KERNEL);
+ if (!fence_info) {
+ r = -ENOMEM;
+ goto err_free_bo_handles;
+ }
+ }
+
+ /* Track fence_info index value */
+ cur = num_fences;
+
+ /* Retrieve GEM objects's fence */
+ bo_flag = wait_info->bo_wait_flags & AMDGPU_USERQ_BO_READ;
+ for (i = 0; i < num_bo_handles; i++) {
+ struct dma_fence *bo_fence;
+
+ gobj[i] = drm_gem_object_lookup(filp, bo_handles[i]);
+ if (!gobj[i]) {
+ r = -ENOENT;
+ goto err_put_gobj;
+ }
+
+ dma_resv_for_each_fence(&resv_cursor, gobj[i]->resv,
+ bo_flag ?
+ DMA_RESV_USAGE_READ :
+ DMA_RESV_USAGE_WRITE,
+ bo_fence) {
+ if (!wait_info->num_fences) {
+ ++num_fences;
+ dma_fence_get(bo_fence);
+ continue;
+ }
+
+ /* Store drm syncobj's gpu va address and value */
+ fence_info[num_fences - cur].va =
+ to_amdgpu_userq_fence(bo_fence)->fence_drv->gpu_addr;
+ fence_info[num_fences - cur].value = bo_fence->seqno;
+
+ cur--;
+ dma_fence_put(bo_fence);
+ }
+
+ drm_gem_object_put(gobj[i]);
+ }
+
+ num_syncobj = wait_info->num_syncobj_handles;
+ /* Array of Syncobj handles */
+ syncobj_handles = kmalloc_array(num_syncobj, sizeof(*syncobj_handles), GFP_KERNEL);
+ if (!syncobj_handles)
+ goto err_put_gobj;
+
+ if (copy_from_user(syncobj_handles, u64_to_user_ptr(wait_info->syncobj_handles_array),
+ sizeof(u32) * num_syncobj)) {
+ r = -EFAULT;
+ goto err_free_syncobj_handles;
+ }
+
+ /* Retrieve syncobj's fence */
+ for (j = 0; j < num_syncobj; j++) {
+ struct dma_fence *fence = NULL;
+ struct drm_syncobj *syncobj;
+
+ syncobj = drm_syncobj_find(filp, syncobj_handles[j]);
+ fence = drm_syncobj_fence_get(syncobj);
+ if (!fence)
+ continue;
+
+ if (!wait_info->num_fences) {
+ ++num_fences;
+ continue;
+ }
+
+ /* Store drm syncobj's gpu va address and value */
+ fence_info[num_fences - cur].va = to_amdgpu_userq_fence(fence)->fence_drv->gpu_addr;
+ fence_info[num_fences - cur].value = fence->seqno;
+ dma_fence_put(fence);
+
+ cur--;
+ dma_fence_put(fence);
+ }
+
+ /*
+ * Passing num_fences = 0 means that userspace doen't want to
+ * retrieve userq_fence_info. If num_fences = 0 we skip filling
+ * userq_fence_info and return the actual number of fences on
+ * args->num_fences.
+ */
+
+ if (!wait_info->num_fences)
+ goto no_fences;
+
+ if (copy_to_user(u64_to_user_ptr(wait_info->userq_fence_info),
+ fence_info, wait_info->num_fences * sizeof(*fence_info))) {
+ r = -EFAULT;
+ goto err_free_gobj;
+ }
+
+ /* Free all handles */
+ kfree(syncobj_handles);
+ kfree(gobj);
+ kfree(fence_info);
+ kfree(bo_handles);
+
+ return 0;
+
+err_free_syncobj_handles:
+ kfree(syncobj_handles);
+err_put_gobj:
+ while (i-- > 0)
+ drm_gem_object_put(gobj[i]);
+no_fences:
+ if (!wait_info->num_fences)
+ wait_info->num_fences = num_fences;
+err_free_gobj:
+ kfree(gobj);
+err_free_fence_info:
+ kfree(fence_info);
+err_free_bo_handles:
+ kfree(bo_handles);
+
+ return r;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
index 7329b4e5dd30..2b2f52296d76 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
@@ -29,6 +29,8 @@
#include "amdgpu_userqueue.h"
+#define AMDGPU_USERQ_BO_READ 0x1
+
struct amdgpu_userq_fence {
struct dma_fence base;
/* userq fence lock */
@@ -60,5 +62,9 @@ void amdgpu_userq_fence_driver_put(struct amdgpu_userq_fence_driver *fence_drv);
int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev, struct amdgpu_usermode_queue *userq);
void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_drv);
void amdgpu_userq_fence_driver_destroy(struct kref *ref);
+int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *filp);
+int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *filp);
#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
index 8918b176fdcb..7022afc17d8d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
@@ -305,6 +305,7 @@ int amdgpu_userq_mgr_init(struct amdgpu_userq_mgr *userq_mgr, struct amdgpu_devi
mutex_init(&userq_mgr->userq_mutex);
idr_init_base(&userq_mgr->userq_idr, 1);
userq_mgr->adev = adev;
+ adev->userq_mgr = userq_mgr;
amdgpu_userqueue_setup_ip_funcs(userq_mgr);
return 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 5/6] drm/amdgpu: Implement userqueue signal/wait IOCTL functions
2023-04-20 14:47 ` [PATCH v3 5/6] drm/amdgpu: Implement userqueue signal/wait IOCTL functions Arunpravin Paneer Selvam
@ 2023-04-21 13:20 ` Christian König
0 siblings, 0 replies; 11+ messages in thread
From: Christian König @ 2023-04-21 13:20 UTC (permalink / raw)
To: Arunpravin Paneer Selvam, amd-gfx; +Cc: alexander.deucher
Am 20.04.23 um 16:47 schrieb Arunpravin Paneer Selvam:
> This patch introduces new IOCTL for userqueue secure semaphore.
>
> The signal IOCTL called from userspace application creates a drm
> syncobj and array of bo GEM handles and passed in as parameter to
> the driver to install the fence into it.
>
> The wait IOCTL gets an array of drm syncobjs, finds the fences
> attached to the drm syncobjs and obtain the array of
> memory_address/fence_value combintion which are returned to
> userspace.
>
> v2: Worked on review comments from Christian for the following
> modifications
>
> - Install fence into GEM BO object.
> - Lock all BO's using the dma resv subsystem
> - Reorder the sequence in signal IOCTL function.
> - Get write pointer from the shadow wptr
> - use userq_fence to fetch the va/value in wait IOCTL.
>
> v3: Worked on review comments from Christian
> - Use drm_exec helper for the proper BO drm reserve and avoid BO
> lock/unlock issues.
> - fence/fence driver reference count logic for signal/wait IOCTLs.
>
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +
> .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 312 +++++++++++++++++-
> .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 6 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 1 +
> 5 files changed, 315 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 3bc8a2d35bb3..1d8a762f43c6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -970,6 +970,8 @@ struct amdgpu_device {
> struct amdgpu_mes mes;
> struct amdgpu_mqd mqds[AMDGPU_HW_IP_NUM];
>
> + struct amdgpu_userq_mgr *userq_mgr;
> +
> /* df */
> struct amdgpu_df df;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index e9c5047087d0..b59e31845568 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2751,6 +2751,8 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = {
> DRM_IOCTL_DEF_DRV(AMDGPU_GEM_OP, amdgpu_gem_op_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(AMDGPU_GEM_USERPTR, amdgpu_gem_userptr_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(AMDGPU_USERQ, amdgpu_userq_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> + DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_SIGNAL, amdgpu_userq_signal_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> + DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_WAIT, amdgpu_userq_wait_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> };
>
> static const struct drm_driver amdgpu_kms_driver = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> index a03d12f83147..1c455b7ebcd6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> @@ -25,6 +25,7 @@
> #include <linux/kref.h>
> #include <linux/slab.h>
>
> +#include <drm/drm_exec.h>
> #include <drm/drm_syncobj.h>
>
> #include "amdgpu.h"
> @@ -84,7 +85,7 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
> &fence_drv->cpu_addr);
> if (r)
> return -ENOMEM;
> -
> +
> kref_init(&fence_drv->refcount);
> INIT_LIST_HEAD(&fence_drv->fences);
> spin_lock_init(&fence_drv->fence_list_lock);
> @@ -124,28 +125,27 @@ void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_d
> void amdgpu_userq_fence_driver_destroy(struct kref *ref)
> {
> struct amdgpu_userq_fence_driver *fence_drv = container_of(ref,
> - struct amdgpu_userq_fence_driver,
> - refcount);
> + struct amdgpu_userq_fence_driver,
> + refcount);
> struct amdgpu_device *adev = fence_drv->adev;
> struct amdgpu_userq_fence *fence, *tmp;
> struct dma_fence *f;
> -
> +
> spin_lock(&fence_drv->fence_list_lock);
> list_for_each_entry_safe(fence, tmp, &fence_drv->fences, link) {
> f = &fence->base;
> -
> +
> if (!dma_fence_is_signaled(f)) {
> dma_fence_set_error(f, -ECANCELED);
> dma_fence_signal(f);
> }
> -
> +
> list_del(&fence->link);
> dma_fence_put(f);
> }
> -
> +
> WARN_ON_ONCE(!list_empty(&fence_drv->fences));
> spin_unlock(&fence_drv->fence_list_lock);
> -
> /* Free seq64 memory */
> amdgpu_seq64_free(adev, fence_drv->gpu_addr);
> kfree(fence_drv);
This whole block here are just unrelated white space changes. Please
cleanup.
> @@ -161,6 +161,11 @@ void amdgpu_userq_fence_driver_put(struct amdgpu_userq_fence_driver *fence_drv)
> kref_put(&fence_drv->refcount, amdgpu_userq_fence_driver_destroy);
> }
>
> +static inline bool amdgpu_userq_fence_test_signaled(struct dma_fence *fence)
> +{
> + return test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
> +}
> +
Oh, that's messing with dma_fence internals and you need a very good
explanation why this is needed!
> int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
> u64 seq, struct dma_fence **f)
> {
> @@ -251,3 +256,294 @@ static const struct dma_fence_ops amdgpu_userq_fence_ops = {
> .signaled = amdgpu_userq_fence_signaled,
> .release = amdgpu_userq_fence_release,
> };
> +
> +static int amdgpu_userq_fence_read_wptr(struct drm_file *filp,
> + struct amdgpu_usermode_queue *queue,
> + u64 *wptr)
> +{
> + struct amdgpu_fpriv *fpriv = filp->driver_priv;
> + struct amdgpu_bo_va_mapping *mapping;
> + struct amdgpu_vm *vm = &fpriv->vm;
> + struct amdgpu_bo *bo;
> + u64 addr, *ptr;
> + int r;
> +
> + addr = queue->userq_prop.wptr_gpu_addr >> PAGE_SHIFT;
> +
> + mapping = amdgpu_vm_bo_lookup_mapping(vm, addr);
> + if (!mapping)
> + return -EINVAL;
> +
> + bo = mapping->bo_va->base.bo;
> + r = amdgpu_bo_reserve(bo, true);
> + if (r) {
> + DRM_ERROR("Failed to reserve userqueue wptr bo");
> + return r;
> + }
> +
> + r = amdgpu_bo_kmap(bo, (void **)&ptr);
> + if (r) {
> + DRM_ERROR("Failed mapping the userqueue wptr bo");
> + goto map_error;
> + }
> +
> + *wptr = le64_to_cpu(*ptr);
> +
> + amdgpu_bo_kunmap(bo);
> + amdgpu_bo_unreserve(bo);
> +
> + return 0;
> +
> +map_error:
> + amdgpu_bo_unreserve(bo);
> + return r;
> +}
> +
> +int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *filp)
> +{
> + struct drm_amdgpu_userq_signal *args = data;
> + struct amdgpu_device *adev = drm_to_adev(dev);
> + struct amdgpu_userq_mgr *userq_mgr = adev->userq_mgr;
> + struct amdgpu_usermode_queue *queue;
> + struct drm_gem_object **gobj = NULL;
> + struct drm_syncobj *syncobj = NULL;
> + u32 *bo_handles, num_bo_handles;
> + struct dma_fence *fence;
> + struct drm_exec exec;
> + u64 wptr;
> + int r, i;
> +
> + /* Retrieve the user queue */
> + queue = idr_find(&userq_mgr->userq_idr, args->queue_id);
> + if (!queue)
> + return -ENOENT;
> +
> + r = amdgpu_userq_fence_read_wptr(filp, queue, &wptr);
> + if (r)
> + return -EINVAL;
> +
> + /* Find Syncobj if any */
> + syncobj = drm_syncobj_find(filp, args->syncobj_handle);
> +
> + /* Array of bo handles */
> + num_bo_handles = args->num_bo_handles;
> + bo_handles = kmalloc_array(num_bo_handles, sizeof(*bo_handles), GFP_KERNEL);
> + if (!bo_handles)
> + return -ENOMEM;
> +
> + if (copy_from_user(bo_handles, u64_to_user_ptr(args->bo_handles_array),
> + sizeof(u32) * num_bo_handles)) {
> + r = -EFAULT;
> + goto cleanup_bo_handles;
> + }
> +
> + /* Create a new fence */
> + r = amdgpu_userq_fence_create(queue, wptr, &fence);
> + if (!fence)
> + goto cleanup_bo_handles;
> +
> + if (amdgpu_userq_fence_test_signaled(fence))
> + goto signaled;
> +
> + /* Add the created fence to syncobj/BO's */
> + if (syncobj) {
> + drm_syncobj_replace_fence(syncobj, fence);
> + dma_fence_put(fence);
> + }
> +
> + /* Array of GEM object handles */
> + gobj = kmalloc_array(num_bo_handles, sizeof(*gobj), GFP_KERNEL);
> + if (!gobj) {
> + r = -ENOMEM;
> + goto cleanup_bo_handles;
> + }
> +
> + drm_exec_init(&exec, true);
> + drm_exec_while_not_all_locked(&exec) {
> + for (i = 0; i < num_bo_handles; i++) {
> + /* Retrieve GEM object */
> + gobj[i] = drm_gem_object_lookup(filp, bo_handles[i]);
Looks like a step into the right direction, e.g. the drm_exec usage
looks correct for example.
But it's illegal to lockup some object when you are already holding
locks of another object.
What you need to do is this:
1. Lockup all BOs (using drm_gem_object_lookup).
2. Lock all BOs (using drm_exec).
3. Add fence to the syncobj and BOs
4. Unlock all BOs.
5. Drop the reference grabbed during lockup.
> + if (!gobj[i]) {
> + r = -ENOENT;
> + goto exec_fini;
> + }
> +
> + r = drm_exec_prepare_obj(&exec, gobj[i], 1);
> + drm_exec_continue_on_contention(&exec);
> + if (unlikely(r))
> + goto exec_fini;
> +
> + dma_resv_add_fence(gobj[i]->resv, fence,
> + args->bo_flags & AMDGPU_USERQ_BO_READ ?
> + DMA_RESV_USAGE_READ :
> + DMA_RESV_USAGE_WRITE);
> + }
> + }
> +
> +exec_fini:
> + drm_exec_fini(&exec);
> +signaled:
> + dma_fence_put(fence);
> + kfree(gobj);
> +cleanup_bo_handles:
> + kfree(bo_handles);
> +
> + return r;
> +}
> +
> +int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *filp)
> +{
> + struct drm_amdgpu_userq_fence_info *fence_info = NULL;
> + struct drm_amdgpu_userq_wait *wait_info = data;
> + u32 *syncobj_handles, *bo_handles;
> + struct dma_resv_iter resv_cursor;
> + u32 num_syncobj, num_bo_handles;
> + struct drm_gem_object **gobj;
> + u64 num_fences = 0;
> + int i, j, r, cur;
> + bool bo_flag;
> +
> + num_bo_handles = wait_info->num_bo_handles;
> + /* Array of GEM BO handles */
> + bo_handles = kmalloc_array(num_bo_handles, sizeof(*bo_handles), GFP_KERNEL);
> + if (!bo_handles)
> + return -ENOMEM;
> +
> + if (copy_from_user(bo_handles, u64_to_user_ptr(wait_info->bo_handles_array),
> + sizeof(u32) * num_bo_handles)) {
> + r = -EFAULT;
> + goto err_free_bo_handles;
> + }
> +
> + /* Array of GEM object handles */
> + gobj = kmalloc_array(num_bo_handles, sizeof(*gobj), GFP_KERNEL);
> + if (!gobj) {
> + r = -ENOMEM;
> + goto err_free_fence_info;
> + }
> +
> + if (wait_info->num_fences) {
> + /* Array of fence gpu address */
> + fence_info = kmalloc_array(wait_info->num_fences, sizeof(*fence_info), GFP_KERNEL);
> + if (!fence_info) {
> + r = -ENOMEM;
> + goto err_free_bo_handles;
> + }
> + }
> +
> + /* Track fence_info index value */
> + cur = num_fences;
> +
> + /* Retrieve GEM objects's fence */
> + bo_flag = wait_info->bo_wait_flags & AMDGPU_USERQ_BO_READ;
> + for (i = 0; i < num_bo_handles; i++) {
> + struct dma_fence *bo_fence;
> +
> + gobj[i] = drm_gem_object_lookup(filp, bo_handles[i]);
> + if (!gobj[i]) {
> + r = -ENOENT;
> + goto err_put_gobj;
> + }
> +
> + dma_resv_for_each_fence(&resv_cursor, gobj[i]->resv,
> + bo_flag ?
> + DMA_RESV_USAGE_READ :
> + DMA_RESV_USAGE_WRITE,
> + bo_fence) {
When you don't lock the BO (and that's ok) you need to use
dma_resv_for_each_fence_unlock()!
This also requires resetting the fence when dma_resv_iter_is_restarted()
returns true.
> + if (!wait_info->num_fences) {
> + ++num_fences;
> + dma_fence_get(bo_fence);
> + continue;
> + }
> +
> + /* Store drm syncobj's gpu va address and value */
> + fence_info[num_fences - cur].va =
> + to_amdgpu_userq_fence(bo_fence)->fence_drv->gpu_addr;
> + fence_info[num_fences - cur].value = bo_fence->seqno;
You need to double check if that is really an amdgpu_userq_fence,
otherwise the cast will just return garbage here.
> +
> + cur--;
Hui? Filling the array from the end won't work. You can't predict how
many fences you will have when you start.
So you need to some handling which prevents adding to many fences and
overwriting things in userspace.
Christian.
> + dma_fence_put(bo_fence);
> + }
> +
> + drm_gem_object_put(gobj[i]);
> + }
> +
> + num_syncobj = wait_info->num_syncobj_handles;
> + /* Array of Syncobj handles */
> + syncobj_handles = kmalloc_array(num_syncobj, sizeof(*syncobj_handles), GFP_KERNEL);
> + if (!syncobj_handles)
> + goto err_put_gobj;
> +
> + if (copy_from_user(syncobj_handles, u64_to_user_ptr(wait_info->syncobj_handles_array),
> + sizeof(u32) * num_syncobj)) {
> + r = -EFAULT;
> + goto err_free_syncobj_handles;
> + }
> +
> + /* Retrieve syncobj's fence */
> + for (j = 0; j < num_syncobj; j++) {
> + struct dma_fence *fence = NULL;
> + struct drm_syncobj *syncobj;
> +
> + syncobj = drm_syncobj_find(filp, syncobj_handles[j]);
> + fence = drm_syncobj_fence_get(syncobj);
> + if (!fence)
> + continue;
> +
> + if (!wait_info->num_fences) {
> + ++num_fences;
> + continue;
> + }
> +
> + /* Store drm syncobj's gpu va address and value */
> + fence_info[num_fences - cur].va = to_amdgpu_userq_fence(fence)->fence_drv->gpu_addr;
> + fence_info[num_fences - cur].value = fence->seqno;
> + dma_fence_put(fence);
> +
> + cur--;
> + dma_fence_put(fence);
> + }
> +
> + /*
> + * Passing num_fences = 0 means that userspace doen't want to
> + * retrieve userq_fence_info. If num_fences = 0 we skip filling
> + * userq_fence_info and return the actual number of fences on
> + * args->num_fences.
> + */
> +
> + if (!wait_info->num_fences)
> + goto no_fences;
> +
> + if (copy_to_user(u64_to_user_ptr(wait_info->userq_fence_info),
> + fence_info, wait_info->num_fences * sizeof(*fence_info))) {
> + r = -EFAULT;
> + goto err_free_gobj;
> + }
> +
> + /* Free all handles */
> + kfree(syncobj_handles);
> + kfree(gobj);
> + kfree(fence_info);
> + kfree(bo_handles);
> +
> + return 0;
> +
> +err_free_syncobj_handles:
> + kfree(syncobj_handles);
> +err_put_gobj:
> + while (i-- > 0)
> + drm_gem_object_put(gobj[i]);
> +no_fences:
> + if (!wait_info->num_fences)
> + wait_info->num_fences = num_fences;
> +err_free_gobj:
> + kfree(gobj);
> +err_free_fence_info:
> + kfree(fence_info);
> +err_free_bo_handles:
> + kfree(bo_handles);
> +
> + return r;
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
> index 7329b4e5dd30..2b2f52296d76 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
> @@ -29,6 +29,8 @@
>
> #include "amdgpu_userqueue.h"
>
> +#define AMDGPU_USERQ_BO_READ 0x1
> +
> struct amdgpu_userq_fence {
> struct dma_fence base;
> /* userq fence lock */
> @@ -60,5 +62,9 @@ void amdgpu_userq_fence_driver_put(struct amdgpu_userq_fence_driver *fence_drv);
> int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev, struct amdgpu_usermode_queue *userq);
> void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_drv);
> void amdgpu_userq_fence_driver_destroy(struct kref *ref);
> +int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *filp);
> +int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *filp);
>
> #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
> index 8918b176fdcb..7022afc17d8d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
> @@ -305,6 +305,7 @@ int amdgpu_userq_mgr_init(struct amdgpu_userq_mgr *userq_mgr, struct amdgpu_devi
> mutex_init(&userq_mgr->userq_mutex);
> idr_init_base(&userq_mgr->userq_idr, 1);
> userq_mgr->adev = adev;
> + adev->userq_mgr = userq_mgr;
>
> amdgpu_userqueue_setup_ip_funcs(userq_mgr);
> return 0;
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 6/6] drm/amdgpu: Enable userqueue fence interrupt handling support
2023-04-20 14:47 [PATCH v3 0/6] Usermode queue fencing synchronization Arunpravin Paneer Selvam
` (4 preceding siblings ...)
2023-04-20 14:47 ` [PATCH v3 5/6] drm/amdgpu: Implement userqueue signal/wait IOCTL functions Arunpravin Paneer Selvam
@ 2023-04-20 14:47 ` Arunpravin Paneer Selvam
5 siblings, 0 replies; 11+ messages in thread
From: Arunpravin Paneer Selvam @ 2023-04-20 14:47 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, christian.koenig, Arunpravin Paneer Selvam
- Added support to handle the userqueue protected fence signal
hardware interrupt.
- Create a hash table which maps va address to the fence driver.
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 3 +++
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 1 +
drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 20 ++++++++++++++++++-
5 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 1d8a762f43c6..58e8c72706a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -971,6 +971,7 @@ struct amdgpu_device {
struct amdgpu_mqd mqds[AMDGPU_HW_IP_NUM];
struct amdgpu_userq_mgr *userq_mgr;
+ DECLARE_HASHTABLE(userq_fence_table, 5);
/* df */
struct amdgpu_df df;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 828d0dd1455b..c89d5fd4a23b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3431,6 +3431,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
mutex_init(&adev->mn_lock);
mutex_init(&adev->virt.vf_errors.lock);
hash_init(adev->mn_hash);
+ hash_init(adev->userq_fence_table);
mutex_init(&adev->psp.mutex);
mutex_init(&adev->notifier_lock);
mutex_init(&adev->pm.stable_pstate_ctx_lock);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index 1c455b7ebcd6..a183d04bb98e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -90,6 +90,9 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
INIT_LIST_HEAD(&fence_drv->fences);
spin_lock_init(&fence_drv->fence_list_lock);
+ hash_add(adev->userq_fence_table, &fence_drv->node,
+ fence_drv->gpu_addr);
+
fence_drv->adev = adev;
fence_drv->context = dma_fence_context_alloc(1);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
index 2b2f52296d76..cf714247dc05 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
@@ -41,6 +41,7 @@ struct amdgpu_userq_fence {
struct amdgpu_userq_fence_driver {
struct kref refcount;
+ struct hlist_node node;
u64 gpu_addr;
u64 *cpu_addr;
u64 context;
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
index a56c6e106d00..b52b90bd6edd 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
@@ -30,6 +30,7 @@
#include "amdgpu_psp.h"
#include "amdgpu_smu.h"
#include "amdgpu_atomfirmware.h"
+#include "amdgpu_userq_fence.h"
#include "imu_v11_0.h"
#include "soc21.h"
#include "nvd.h"
@@ -5870,10 +5871,27 @@ static int gfx_v11_0_eop_irq(struct amdgpu_device *adev,
u8 me_id, pipe_id, queue_id;
struct amdgpu_ring *ring;
uint32_t mes_queue_id = entry->src_data[0];
+ struct hlist_node *tmp;
+ struct amdgpu_userq_fence_driver *f;
+ u32 upper32 = entry->src_data[1];
+ u32 lower32 = entry->src_data[2];
+ u64 fence_address = ((u64)upper32 << 32) | lower32;
DRM_DEBUG("IH: CP EOP\n");
- if (adev->enable_mes && (mes_queue_id & AMDGPU_FENCE_MES_QUEUE_FLAG)) {
+ if (adev->enable_mes && fence_address) {
+ hash_for_each_safe(adev->userq_fence_table, i, tmp, f, node) {
+ if (fence_address == f->gpu_addr) {
+ hash_del(&f->node);
+ break;
+ }
+ }
+
+ if (f) {
+ DRM_DEBUG("user queue fence address %llu\n", fence_address);
+ amdgpu_userq_fence_driver_process(f);
+ }
+ } else if (adev->enable_mes && (mes_queue_id & AMDGPU_FENCE_MES_QUEUE_FLAG)) {
struct amdgpu_mes_queue *queue;
mes_queue_id &= AMDGPU_FENCE_MES_QUEUE_ID_MASK;
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread