* [PATCH 1/2] drm/amdgpu: add userq specific kernel config for fence ioctls
@ 2024-10-24 12:10 Arunpravin Paneer Selvam
2024-10-24 12:10 ` [PATCH 2/2] drm/amdgpu: Add gpu_addr support to seq64 allocation Arunpravin Paneer Selvam
2024-10-29 8:42 ` [PATCH 1/2] drm/amdgpu: add userq specific kernel config for fence ioctls Christian König
0 siblings, 2 replies; 7+ messages in thread
From: Arunpravin Paneer Selvam @ 2024-10-24 12:10 UTC (permalink / raw)
To: amd-gfx, christian.koenig; +Cc: alexander.deucher, Arunpravin Paneer Selvam
Keep the user queue fence signal and wait IOCTLs in the
kernel config CONFIG_DRM_AMDGPU_NAVI3X_USERQ.
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++++
drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 16 ++++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 70cb3b794a8a..04eb6611d19b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2971,9 +2971,11 @@ static int __init amdgpu_init(void)
if (r)
goto error_sync;
+#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
r = amdgpu_fence_slab_init();
if (r)
goto error_fence;
+#endif
r = amdgpu_userq_fence_slab_init();
if (r)
@@ -3003,7 +3005,9 @@ static void __exit amdgpu_exit(void)
amdgpu_unregister_atpx_handler();
amdgpu_acpi_release();
amdgpu_sync_fini();
+#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
amdgpu_fence_slab_fini();
+#endif
amdgpu_userq_fence_slab_fini();
mmu_notifier_synchronize();
amdgpu_xcp_drv_release();
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index 279dece6f6d7..bec53776fe5f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -318,6 +318,7 @@ static const struct dma_fence_ops amdgpu_userq_fence_ops = {
.release = amdgpu_userq_fence_release,
};
+#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
/**
* amdgpu_userq_fence_read_wptr - Read the userq wptr value
*
@@ -502,7 +503,15 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
return r;
}
+#else
+int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *filp)
+{
+ return 0;
+}
+#endif
+#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
struct drm_file *filp)
{
@@ -797,3 +806,10 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
return r;
}
+#else
+int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *filp)
+{
+ return 0;
+}
+#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] drm/amdgpu: Add gpu_addr support to seq64 allocation
2024-10-24 12:10 [PATCH 1/2] drm/amdgpu: add userq specific kernel config for fence ioctls Arunpravin Paneer Selvam
@ 2024-10-24 12:10 ` Arunpravin Paneer Selvam
2024-10-29 8:45 ` Christian König
2024-10-29 8:42 ` [PATCH 1/2] drm/amdgpu: add userq specific kernel config for fence ioctls Christian König
1 sibling, 1 reply; 7+ messages in thread
From: Arunpravin Paneer Selvam @ 2024-10-24 12:10 UTC (permalink / raw)
To: amd-gfx, christian.koenig; +Cc: alexander.deucher, Arunpravin Paneer Selvam
Add gpu address support to seq64 alloc function.
v1:(Christian)
- Add the user of this new interface change to the same
patch.
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c | 10 ++++++++--
drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h | 3 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 8 ++++----
drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 1 +
4 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c
index 12cad4424fc0..2f01a209ec3f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c
@@ -166,7 +166,8 @@ void amdgpu_seq64_unmap(struct amdgpu_device *adev, struct amdgpu_fpriv *fpriv)
* Returns:
* 0 on success or a negative error code on failure
*/
-int amdgpu_seq64_alloc(struct amdgpu_device *adev, u64 *va, u64 **cpu_addr)
+int amdgpu_seq64_alloc(struct amdgpu_device *adev, u64 *va,
+ u64 *gpu_addr, u64 **cpu_addr)
{
unsigned long bit_pos;
@@ -175,7 +176,12 @@ int amdgpu_seq64_alloc(struct amdgpu_device *adev, u64 *va, u64 **cpu_addr)
return -ENOSPC;
__set_bit(bit_pos, adev->seq64.used);
+
*va = bit_pos * sizeof(u64) + amdgpu_seq64_get_va_base(adev);
+
+ if (gpu_addr)
+ *gpu_addr = bit_pos * sizeof(u64) + adev->seq64.gpu_addr;
+
*cpu_addr = bit_pos + adev->seq64.cpu_base_addr;
return 0;
@@ -236,7 +242,7 @@ int amdgpu_seq64_init(struct amdgpu_device *adev)
*/
r = amdgpu_bo_create_kernel(adev, AMDGPU_VA_RESERVED_SEQ64_SIZE,
PAGE_SIZE, AMDGPU_GEM_DOMAIN_GTT,
- &adev->seq64.sbo, NULL,
+ &adev->seq64.sbo, &adev->seq64.gpu_addr,
(void **)&adev->seq64.cpu_base_addr);
if (r) {
dev_warn(adev->dev, "(%d) create seq64 failed\n", r);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h
index 4203b2ab318d..26a249aaaee1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h
@@ -32,13 +32,14 @@
struct amdgpu_seq64 {
struct amdgpu_bo *sbo;
u32 num_sem;
+ u64 gpu_addr;
u64 *cpu_base_addr;
DECLARE_BITMAP(used, AMDGPU_MAX_SEQ64_SLOTS);
};
void amdgpu_seq64_fini(struct amdgpu_device *adev);
int amdgpu_seq64_init(struct amdgpu_device *adev);
-int amdgpu_seq64_alloc(struct amdgpu_device *adev, u64 *gpu_addr, u64 **cpu_addr);
+int amdgpu_seq64_alloc(struct amdgpu_device *adev, u64 *va, 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);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index bec53776fe5f..8cf169e7e893 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -82,7 +82,7 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
}
/* Acquire seq64 memory */
- r = amdgpu_seq64_alloc(adev, &fence_drv->gpu_addr,
+ r = amdgpu_seq64_alloc(adev, &fence_drv->va, &fence_drv->gpu_addr,
&fence_drv->cpu_addr);
if (r) {
kfree(fence_drv);
@@ -113,7 +113,7 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
return 0;
free_seq64:
- amdgpu_seq64_free(adev, fence_drv->gpu_addr);
+ amdgpu_seq64_free(adev, fence_drv->va);
free_fence_drv:
kfree(fence_drv);
@@ -183,7 +183,7 @@ void amdgpu_userq_fence_driver_destroy(struct kref *ref)
xa_unlock_irqrestore(xa, flags);
/* Free seq64 memory */
- amdgpu_seq64_free(adev, fence_drv->gpu_addr);
+ amdgpu_seq64_free(adev, fence_drv->va);
kfree(fence_drv);
}
@@ -751,7 +751,7 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
}
/* Store drm syncobj's gpu va address and value */
- fence_info[cnt].va = fence_drv->gpu_addr;
+ fence_info[cnt].va = fence_drv->va;
fence_info[cnt].value = fences[i]->seqno;
dma_fence_put(fences[i]);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
index 89c82ba38b50..f1a90840ac1f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
@@ -44,6 +44,7 @@ struct amdgpu_userq_fence {
struct amdgpu_userq_fence_driver {
struct kref refcount;
+ u64 va;
u64 gpu_addr;
u64 *cpu_addr;
u64 context;
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] drm/amdgpu: add userq specific kernel config for fence ioctls
2024-10-24 12:10 [PATCH 1/2] drm/amdgpu: add userq specific kernel config for fence ioctls Arunpravin Paneer Selvam
2024-10-24 12:10 ` [PATCH 2/2] drm/amdgpu: Add gpu_addr support to seq64 allocation Arunpravin Paneer Selvam
@ 2024-10-29 8:42 ` Christian König
2024-10-29 13:32 ` Alex Deucher
1 sibling, 1 reply; 7+ messages in thread
From: Christian König @ 2024-10-29 8:42 UTC (permalink / raw)
To: Arunpravin Paneer Selvam, amd-gfx; +Cc: alexander.deucher
Am 24.10.24 um 14:10 schrieb Arunpravin Paneer Selvam:
> Keep the user queue fence signal and wait IOCTLs in the
> kernel config CONFIG_DRM_AMDGPU_NAVI3X_USERQ.
>
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 16 ++++++++++++++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 70cb3b794a8a..04eb6611d19b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2971,9 +2971,11 @@ static int __init amdgpu_init(void)
> if (r)
> goto error_sync;
>
> +#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
> r = amdgpu_fence_slab_init();
> if (r)
> goto error_fence;
> +#endif
That here makes no sense. This is for the kernel queues and not for the
user queues.
>
> r = amdgpu_userq_fence_slab_init();
> if (r)
> @@ -3003,7 +3005,9 @@ static void __exit amdgpu_exit(void)
> amdgpu_unregister_atpx_handler();
> amdgpu_acpi_release();
> amdgpu_sync_fini();
> +#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
> amdgpu_fence_slab_fini();
> +#endif
> amdgpu_userq_fence_slab_fini();
> mmu_notifier_synchronize();
> amdgpu_xcp_drv_release();
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> index 279dece6f6d7..bec53776fe5f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> @@ -318,6 +318,7 @@ static const struct dma_fence_ops amdgpu_userq_fence_ops = {
> .release = amdgpu_userq_fence_release,
> };
>
> +#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
> /**
> * amdgpu_userq_fence_read_wptr - Read the userq wptr value
> *
> @@ -502,7 +503,15 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
>
> return r;
> }
> +#else
> +int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *filp)
> +{
> + return 0;
> +}
> +#endif
>
> +#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
> int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> struct drm_file *filp)
> {
> @@ -797,3 +806,10 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
>
> return r;
> }
> +#else
> +int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *filp)
> +{
> + return 0;
> +}
> +#endif
Not nice, but since CONFIG_DRM_AMDGPU_NAVI3X_USERQ depends on
CONFIG_BROKEN at the moment probably ok as intermediate step.
Regards,
Christian.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: Add gpu_addr support to seq64 allocation
2024-10-24 12:10 ` [PATCH 2/2] drm/amdgpu: Add gpu_addr support to seq64 allocation Arunpravin Paneer Selvam
@ 2024-10-29 8:45 ` Christian König
0 siblings, 0 replies; 7+ messages in thread
From: Christian König @ 2024-10-29 8:45 UTC (permalink / raw)
To: Arunpravin Paneer Selvam, amd-gfx; +Cc: alexander.deucher
Am 24.10.24 um 14:10 schrieb Arunpravin Paneer Selvam:
> Add gpu address support to seq64 alloc function.
>
> v1:(Christian)
> - Add the user of this new interface change to the same
> patch.
>
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c | 10 ++++++++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h | 3 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 8 ++++----
> drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 1 +
> 4 files changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c
> index 12cad4424fc0..2f01a209ec3f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.c
> @@ -166,7 +166,8 @@ void amdgpu_seq64_unmap(struct amdgpu_device *adev, struct amdgpu_fpriv *fpriv)
> * Returns:
> * 0 on success or a negative error code on failure
> */
> -int amdgpu_seq64_alloc(struct amdgpu_device *adev, u64 *va, u64 **cpu_addr)
> +int amdgpu_seq64_alloc(struct amdgpu_device *adev, u64 *va,
> + u64 *gpu_addr, u64 **cpu_addr)
> {
> unsigned long bit_pos;
>
> @@ -175,7 +176,12 @@ int amdgpu_seq64_alloc(struct amdgpu_device *adev, u64 *va, u64 **cpu_addr)
> return -ENOSPC;
>
> __set_bit(bit_pos, adev->seq64.used);
> +
> *va = bit_pos * sizeof(u64) + amdgpu_seq64_get_va_base(adev);
> +
> + if (gpu_addr)
> + *gpu_addr = bit_pos * sizeof(u64) + adev->seq64.gpu_addr;
> +
> *cpu_addr = bit_pos + adev->seq64.cpu_base_addr;
>
> return 0;
> @@ -236,7 +242,7 @@ int amdgpu_seq64_init(struct amdgpu_device *adev)
> */
> r = amdgpu_bo_create_kernel(adev, AMDGPU_VA_RESERVED_SEQ64_SIZE,
> PAGE_SIZE, AMDGPU_GEM_DOMAIN_GTT,
> - &adev->seq64.sbo, NULL,
> + &adev->seq64.sbo, &adev->seq64.gpu_addr,
> (void **)&adev->seq64.cpu_base_addr);
> if (r) {
> dev_warn(adev->dev, "(%d) create seq64 failed\n", r);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h
> index 4203b2ab318d..26a249aaaee1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_seq64.h
> @@ -32,13 +32,14 @@
> struct amdgpu_seq64 {
> struct amdgpu_bo *sbo;
> u32 num_sem;
> + u64 gpu_addr;
> u64 *cpu_base_addr;
> DECLARE_BITMAP(used, AMDGPU_MAX_SEQ64_SLOTS);
> };
>
> void amdgpu_seq64_fini(struct amdgpu_device *adev);
> int amdgpu_seq64_init(struct amdgpu_device *adev);
> -int amdgpu_seq64_alloc(struct amdgpu_device *adev, u64 *gpu_addr, u64 **cpu_addr);
> +int amdgpu_seq64_alloc(struct amdgpu_device *adev, u64 *va, 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);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> index bec53776fe5f..8cf169e7e893 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> @@ -82,7 +82,7 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
> }
>
> /* Acquire seq64 memory */
> - r = amdgpu_seq64_alloc(adev, &fence_drv->gpu_addr,
> + r = amdgpu_seq64_alloc(adev, &fence_drv->va, &fence_drv->gpu_addr,
> &fence_drv->cpu_addr);
> if (r) {
> kfree(fence_drv);
> @@ -113,7 +113,7 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
> return 0;
>
> free_seq64:
> - amdgpu_seq64_free(adev, fence_drv->gpu_addr);
> + amdgpu_seq64_free(adev, fence_drv->va);
> free_fence_drv:
> kfree(fence_drv);
>
> @@ -183,7 +183,7 @@ void amdgpu_userq_fence_driver_destroy(struct kref *ref)
> xa_unlock_irqrestore(xa, flags);
>
> /* Free seq64 memory */
> - amdgpu_seq64_free(adev, fence_drv->gpu_addr);
> + amdgpu_seq64_free(adev, fence_drv->va);
> kfree(fence_drv);
> }
>
> @@ -751,7 +751,7 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> }
>
> /* Store drm syncobj's gpu va address and value */
> - fence_info[cnt].va = fence_drv->gpu_addr;
> + fence_info[cnt].va = fence_drv->va;
> fence_info[cnt].value = fences[i]->seqno;
>
> dma_fence_put(fences[i]);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
> index 89c82ba38b50..f1a90840ac1f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
> @@ -44,6 +44,7 @@ struct amdgpu_userq_fence {
>
> struct amdgpu_userq_fence_driver {
> struct kref refcount;
> + u64 va;
> u64 gpu_addr;
> u64 *cpu_addr;
> u64 context;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] drm/amdgpu: add userq specific kernel config for fence ioctls
2024-10-29 8:42 ` [PATCH 1/2] drm/amdgpu: add userq specific kernel config for fence ioctls Christian König
@ 2024-10-29 13:32 ` Alex Deucher
2024-10-29 14:03 ` Christian König
0 siblings, 1 reply; 7+ messages in thread
From: Alex Deucher @ 2024-10-29 13:32 UTC (permalink / raw)
To: Christian König; +Cc: Arunpravin Paneer Selvam, amd-gfx, alexander.deucher
On Tue, Oct 29, 2024 at 5:38 AM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 24.10.24 um 14:10 schrieb Arunpravin Paneer Selvam:
> > Keep the user queue fence signal and wait IOCTLs in the
> > kernel config CONFIG_DRM_AMDGPU_NAVI3X_USERQ.
> >
> > Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++++
> > drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 16 ++++++++++++++++
> > 2 files changed, 20 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index 70cb3b794a8a..04eb6611d19b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -2971,9 +2971,11 @@ static int __init amdgpu_init(void)
> > if (r)
> > goto error_sync;
> >
> > +#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
> > r = amdgpu_fence_slab_init();
> > if (r)
> > goto error_fence;
> > +#endif
>
> That here makes no sense. This is for the kernel queues and not for the
> user queues.
>
> >
> > r = amdgpu_userq_fence_slab_init();
> > if (r)
> > @@ -3003,7 +3005,9 @@ static void __exit amdgpu_exit(void)
> > amdgpu_unregister_atpx_handler();
> > amdgpu_acpi_release();
> > amdgpu_sync_fini();
> > +#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
> > amdgpu_fence_slab_fini();
> > +#endif
> > amdgpu_userq_fence_slab_fini();
> > mmu_notifier_synchronize();
> > amdgpu_xcp_drv_release();
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> > index 279dece6f6d7..bec53776fe5f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> > @@ -318,6 +318,7 @@ static const struct dma_fence_ops amdgpu_userq_fence_ops = {
> > .release = amdgpu_userq_fence_release,
> > };
> >
>
>
> > +#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
> > /**
> > * amdgpu_userq_fence_read_wptr - Read the userq wptr value
> > *
> > @@ -502,7 +503,15 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
> >
> > return r;
> > }
> > +#else
> > +int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
> > + struct drm_file *filp)
> > +{
> > + return 0;
> > +}
> > +#endif
> >
> > +#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
> > int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> > struct drm_file *filp)
> > {
> > @@ -797,3 +806,10 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> >
> > return r;
> > }
> > +#else
> > +int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> > + struct drm_file *filp)
> > +{
> > + return 0;
> > +}
> > +#endif
>
> Not nice, but since CONFIG_DRM_AMDGPU_NAVI3X_USERQ depends on
> CONFIG_BROKEN at the moment probably ok as intermediate step.
Wouldn't it be better to return an error in these cases?
Alex
>
> Regards,
> Christian.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] drm/amdgpu: add userq specific kernel config for fence ioctls
2024-10-29 13:32 ` Alex Deucher
@ 2024-10-29 14:03 ` Christian König
2024-10-29 14:06 ` Paneer Selvam, Arunpravin
0 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2024-10-29 14:03 UTC (permalink / raw)
To: Alex Deucher; +Cc: Arunpravin Paneer Selvam, amd-gfx, alexander.deucher
Am 29.10.24 um 14:32 schrieb Alex Deucher:
> On Tue, Oct 29, 2024 at 5:38 AM Christian König
> <christian.koenig@amd.com> wrote:
>> Am 24.10.24 um 14:10 schrieb Arunpravin Paneer Selvam:
>>> Keep the user queue fence signal and wait IOCTLs in the
>>> kernel config CONFIG_DRM_AMDGPU_NAVI3X_USERQ.
>>>
>>> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++++
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 16 ++++++++++++++++
>>> 2 files changed, 20 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> index 70cb3b794a8a..04eb6611d19b 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> @@ -2971,9 +2971,11 @@ static int __init amdgpu_init(void)
>>> if (r)
>>> goto error_sync;
>>>
>>> +#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
>>> r = amdgpu_fence_slab_init();
>>> if (r)
>>> goto error_fence;
>>> +#endif
>> That here makes no sense. This is for the kernel queues and not for the
>> user queues.
>>
>>> r = amdgpu_userq_fence_slab_init();
>>> if (r)
>>> @@ -3003,7 +3005,9 @@ static void __exit amdgpu_exit(void)
>>> amdgpu_unregister_atpx_handler();
>>> amdgpu_acpi_release();
>>> amdgpu_sync_fini();
>>> +#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
>>> amdgpu_fence_slab_fini();
>>> +#endif
>>> amdgpu_userq_fence_slab_fini();
>>> mmu_notifier_synchronize();
>>> amdgpu_xcp_drv_release();
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>> index 279dece6f6d7..bec53776fe5f 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>> @@ -318,6 +318,7 @@ static const struct dma_fence_ops amdgpu_userq_fence_ops = {
>>> .release = amdgpu_userq_fence_release,
>>> };
>>>
>>
>>> +#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
>>> /**
>>> * amdgpu_userq_fence_read_wptr - Read the userq wptr value
>>> *
>>> @@ -502,7 +503,15 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
>>>
>>> return r;
>>> }
>>> +#else
>>> +int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
>>> + struct drm_file *filp)
>>> +{
>>> + return 0;
>>> +}
>>> +#endif
>>>
>>> +#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
>>> int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
>>> struct drm_file *filp)
>>> {
>>> @@ -797,3 +806,10 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
>>>
>>> return r;
>>> }
>>> +#else
>>> +int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
>>> + struct drm_file *filp)
>>> +{
>>> + return 0;
>>> +}
>>> +#endif
>> Not nice, but since CONFIG_DRM_AMDGPU_NAVI3X_USERQ depends on
>> CONFIG_BROKEN at the moment probably ok as intermediate step.
> Wouldn't it be better to return an error in these cases?
Good point, the functions should never be called in the first place but
better save than sorry.
Christian.
>
> Alex
>
>> Regards,
>> Christian.
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] drm/amdgpu: add userq specific kernel config for fence ioctls
2024-10-29 14:03 ` Christian König
@ 2024-10-29 14:06 ` Paneer Selvam, Arunpravin
0 siblings, 0 replies; 7+ messages in thread
From: Paneer Selvam, Arunpravin @ 2024-10-29 14:06 UTC (permalink / raw)
To: Christian König, Alex Deucher; +Cc: amd-gfx, alexander.deucher
On 10/29/2024 7:33 PM, Christian König wrote:
> Am 29.10.24 um 14:32 schrieb Alex Deucher:
>> On Tue, Oct 29, 2024 at 5:38 AM Christian König
>> <christian.koenig@amd.com> wrote:
>>> Am 24.10.24 um 14:10 schrieb Arunpravin Paneer Selvam:
>>>> Keep the user queue fence signal and wait IOCTLs in the
>>>> kernel config CONFIG_DRM_AMDGPU_NAVI3X_USERQ.
>>>>
>>>> Signed-off-by: Arunpravin Paneer Selvam
>>>> <Arunpravin.PaneerSelvam@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++++
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 16
>>>> ++++++++++++++++
>>>> 2 files changed, 20 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> index 70cb3b794a8a..04eb6611d19b 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> @@ -2971,9 +2971,11 @@ static int __init amdgpu_init(void)
>>>> if (r)
>>>> goto error_sync;
>>>>
>>>> +#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
>>>> r = amdgpu_fence_slab_init();
>>>> if (r)
>>>> goto error_fence;
>>>> +#endif
>>> That here makes no sense. This is for the kernel queues and not for the
>>> user queues.
>>>
>>>> r = amdgpu_userq_fence_slab_init();
>>>> if (r)
>>>> @@ -3003,7 +3005,9 @@ static void __exit amdgpu_exit(void)
>>>> amdgpu_unregister_atpx_handler();
>>>> amdgpu_acpi_release();
>>>> amdgpu_sync_fini();
>>>> +#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
>>>> amdgpu_fence_slab_fini();
>>>> +#endif
>>>> amdgpu_userq_fence_slab_fini();
>>>> mmu_notifier_synchronize();
>>>> amdgpu_xcp_drv_release();
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>> index 279dece6f6d7..bec53776fe5f 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>> @@ -318,6 +318,7 @@ static const struct dma_fence_ops
>>>> amdgpu_userq_fence_ops = {
>>>> .release = amdgpu_userq_fence_release,
>>>> };
>>>>
>>>
>>>> +#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
>>>> /**
>>>> * amdgpu_userq_fence_read_wptr - Read the userq wptr value
>>>> *
>>>> @@ -502,7 +503,15 @@ int amdgpu_userq_signal_ioctl(struct
>>>> drm_device *dev, void *data,
>>>>
>>>> return r;
>>>> }
>>>> +#else
>>>> +int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
>>>> + struct drm_file *filp)
>>>> +{
>>>> + return 0;
>>>> +}
>>>> +#endif
>>>>
>>>> +#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
>>>> int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
>>>> struct drm_file *filp)
>>>> {
>>>> @@ -797,3 +806,10 @@ int amdgpu_userq_wait_ioctl(struct drm_device
>>>> *dev, void *data,
>>>>
>>>> return r;
>>>> }
>>>> +#else
>>>> +int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
>>>> + struct drm_file *filp)
>>>> +{
>>>> + return 0;
>>>> +}
>>>> +#endif
>>> Not nice, but since CONFIG_DRM_AMDGPU_NAVI3X_USERQ depends on
>>> CONFIG_BROKEN at the moment probably ok as intermediate step.
>> Wouldn't it be better to return an error in these cases?
>
> Good point, the functions should never be called in the first place
> but better save than sorry.
Can I return -EINVAL instead of 0.
Thanks,
Arun.
>
> Christian.
>
>>
>> Alex
>>
>>> Regards,
>>> Christian.
>>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-10-29 14:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-24 12:10 [PATCH 1/2] drm/amdgpu: add userq specific kernel config for fence ioctls Arunpravin Paneer Selvam
2024-10-24 12:10 ` [PATCH 2/2] drm/amdgpu: Add gpu_addr support to seq64 allocation Arunpravin Paneer Selvam
2024-10-29 8:45 ` Christian König
2024-10-29 8:42 ` [PATCH 1/2] drm/amdgpu: add userq specific kernel config for fence ioctls Christian König
2024-10-29 13:32 ` Alex Deucher
2024-10-29 14:03 ` Christian König
2024-10-29 14:06 ` Paneer Selvam, Arunpravin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox