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@xxxxxxx> 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@xxxxxxx>
---
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.
