<div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Aug 1, 2023 at 4:37 AM Karol Herbst <<a href="mailto:kherbst@redhat.com">kherbst@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, Jul 31, 2023 at 9:16 PM Dave Airlie <<a href="mailto:airlied@gmail.com" target="_blank">airlied@gmail.com</a>> wrote:<br>
><br>
> From: Dave Airlie <<a href="mailto:airlied@redhat.com" target="_blank">airlied@redhat.com</a>><br>
><br>
> nouveau > 10 years ago had a plan for new multiplexer inside a multiplexer<br>
> API using nvif. It never fully reached fruition, fast forward 10 years,<br>
> and the new vulkan driver is avoiding libdrm and calling ioctls, and<br>
> these 3 ioctls, getparam, channel alloc + free don't seem to be things<br>
> we'd want to use nvif for.<br>
><br>
> Undeprecate and put them into the uapi header so we can just copy it<br>
> into mesa later.<br>
><br>
> Signed-off-by: Dave Airlie <<a href="mailto:airlied@redhat.com" target="_blank">airlied@redhat.com</a>><br>
> ---<br>
> drivers/gpu/drm/nouveau/nouveau_abi16.h | 41 ---------------------<br>
> include/uapi/drm/nouveau_drm.h | 48 +++++++++++++++++++++++--<br>
> 2 files changed, 45 insertions(+), 44 deletions(-)<br>
><br>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.h b/drivers/gpu/drm/nouveau/nouveau_abi16.h<br>
> index 27eae85f33e6..d5d80d0d9011 100644<br>
> --- a/drivers/gpu/drm/nouveau/nouveau_abi16.h<br>
> +++ b/drivers/gpu/drm/nouveau/nouveau_abi16.h<br>
> @@ -43,28 +43,6 @@ int nouveau_abi16_usif(struct drm_file *, void *data, u32 size);<br>
> #define NOUVEAU_GEM_DOMAIN_VRAM (1 << 1)<br>
> #define NOUVEAU_GEM_DOMAIN_GART (1 << 2)<br>
><br>
> -struct drm_nouveau_channel_alloc {<br>
> - uint32_t fb_ctxdma_handle;<br>
> - uint32_t tt_ctxdma_handle;<br>
> -<br>
> - int channel;<br>
> - uint32_t pushbuf_domains;<br>
> -<br>
> - /* Notifier memory */<br>
> - uint32_t notifier_handle;<br>
> -<br>
> - /* DRM-enforced subchannel assignments */<br>
> - struct {<br>
> - uint32_t handle;<br>
> - uint32_t grclass;<br>
> - } subchan[8];<br>
> - uint32_t nr_subchan;<br>
> -};<br>
> -<br>
> -struct drm_nouveau_channel_free {<br>
> - int channel;<br>
> -};<br>
> -<br>
> struct drm_nouveau_grobj_alloc {<br>
> int channel;<br>
> uint32_t handle;<br>
> @@ -83,31 +61,12 @@ struct drm_nouveau_gpuobj_free {<br>
> uint32_t handle;<br>
> };<br>
><br>
> -#define NOUVEAU_GETPARAM_PCI_VENDOR 3<br>
> -#define NOUVEAU_GETPARAM_PCI_DEVICE 4<br>
> -#define NOUVEAU_GETPARAM_BUS_TYPE 5<br>
> -#define NOUVEAU_GETPARAM_FB_SIZE 8<br>
> -#define NOUVEAU_GETPARAM_AGP_SIZE 9<br>
> -#define NOUVEAU_GETPARAM_CHIPSET_ID 11<br>
> -#define NOUVEAU_GETPARAM_VM_VRAM_BASE 12<br>
> -#define NOUVEAU_GETPARAM_GRAPH_UNITS 13<br>
> -#define NOUVEAU_GETPARAM_PTIMER_TIME 14<br>
> -#define NOUVEAU_GETPARAM_HAS_BO_USAGE 15<br>
> -#define NOUVEAU_GETPARAM_HAS_PAGEFLIP 16<br>
> -struct drm_nouveau_getparam {<br>
> - uint64_t param;<br>
> - uint64_t value;<br>
> -};<br>
> -<br>
> struct drm_nouveau_setparam {<br>
> uint64_t param;<br>
> uint64_t value;<br>
> };<br>
><br>
> -#define DRM_IOCTL_NOUVEAU_GETPARAM DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_GETPARAM, struct drm_nouveau_getparam)<br>
> #define DRM_IOCTL_NOUVEAU_SETPARAM DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_SETPARAM, struct drm_nouveau_setparam)<br>
> -#define DRM_IOCTL_NOUVEAU_CHANNEL_ALLOC DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_CHANNEL_ALLOC, struct drm_nouveau_channel_alloc)<br>
> -#define DRM_IOCTL_NOUVEAU_CHANNEL_FREE DRM_IOW (DRM_COMMAND_BASE + DRM_NOUVEAU_CHANNEL_FREE, struct drm_nouveau_channel_free)<br>
> #define DRM_IOCTL_NOUVEAU_GROBJ_ALLOC DRM_IOW (DRM_COMMAND_BASE + DRM_NOUVEAU_GROBJ_ALLOC, struct drm_nouveau_grobj_alloc)<br>
> #define DRM_IOCTL_NOUVEAU_NOTIFIEROBJ_ALLOC DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_NOTIFIEROBJ_ALLOC, struct drm_nouveau_notifierobj_alloc)<br>
> #define DRM_IOCTL_NOUVEAU_GPUOBJ_FREE DRM_IOW (DRM_COMMAND_BASE + DRM_NOUVEAU_GPUOBJ_FREE, struct drm_nouveau_gpuobj_free)<br>
> diff --git a/include/uapi/drm/nouveau_drm.h b/include/uapi/drm/nouveau_drm.h<br>
> index 853a327433d3..1cd97b3d8eda 100644<br>
> --- a/include/uapi/drm/nouveau_drm.h<br>
> +++ b/include/uapi/drm/nouveau_drm.h<br>
> @@ -33,6 +33,44 @@<br>
> extern "C" {<br>
> #endif<br>
><br>
> +#define NOUVEAU_GETPARAM_PCI_VENDOR 3<br>
> +#define NOUVEAU_GETPARAM_PCI_DEVICE 4<br>
> +#define NOUVEAU_GETPARAM_BUS_TYPE 5<br>
> +#define NOUVEAU_GETPARAM_FB_SIZE 8<br>
> +#define NOUVEAU_GETPARAM_AGP_SIZE 9<br>
> +#define NOUVEAU_GETPARAM_CHIPSET_ID 11<br>
> +#define NOUVEAU_GETPARAM_VM_VRAM_BASE 12<br>
> +#define NOUVEAU_GETPARAM_GRAPH_UNITS 13<br>
> +#define NOUVEAU_GETPARAM_PTIMER_TIME 14<br>
> +#define NOUVEAU_GETPARAM_HAS_BO_USAGE 15<br>
> +#define NOUVEAU_GETPARAM_HAS_PAGEFLIP 16<br>
> +struct drm_nouveau_getparam {<br>
> + uint64_t param;<br>
> + uint64_t value;<br>
> +};<br>
> +<br>
> +struct drm_nouveau_channel_alloc {<br>
> + uint32_t fb_ctxdma_handle;<br></blockquote><div><br></div><div>Do we want to use `uint32_t` or `__u32` here? It looks like the original headers used `uint32_t` and then it got switched to `__u32` after the deprecation happened. We probably want `__u32` given that this is a uapi header.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> + uint32_t tt_ctxdma_handle;<br>
> +<br>
> + int channel;<br></blockquote><div><br></div><div>IDK what to do about this one. I want to make it __s32. I think that should be safe on all the hardware we care about. Having an unsized type in a UAPI header is concerning.</div><div><br></div><div>~Faith<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> + uint32_t pushbuf_domains;<br>
> +<br>
> + /* Notifier memory */<br>
> + uint32_t notifier_handle;<br>
> +<br>
> + /* DRM-enforced subchannel assignments */<br>
> + struct {<br>
> + uint32_t handle;<br>
> + uint32_t grclass;<br>
> + } subchan[8];<br>
> + uint32_t nr_subchan;<br>
> +};<br>
> +<br>
> +struct drm_nouveau_channel_free {<br>
> + int channel;<br>
> +};<br>
> +<br>
> #define NOUVEAU_GEM_DOMAIN_CPU (1 << 0)<br>
> #define NOUVEAU_GEM_DOMAIN_VRAM (1 << 1)<br>
> #define NOUVEAU_GEM_DOMAIN_GART (1 << 2)<br>
> @@ -126,10 +164,10 @@ struct drm_nouveau_gem_cpu_fini {<br>
> __u32 handle;<br>
> };<br>
><br>
> -#define DRM_NOUVEAU_GETPARAM 0x00 /* deprecated */<br>
> +#define DRM_NOUVEAU_GETPARAM 0x00<br>
> #define DRM_NOUVEAU_SETPARAM 0x01 /* deprecated */<br>
> -#define DRM_NOUVEAU_CHANNEL_ALLOC 0x02 /* deprecated */<br>
> -#define DRM_NOUVEAU_CHANNEL_FREE 0x03 /* deprecated */<br>
> +#define DRM_NOUVEAU_CHANNEL_ALLOC 0x02<br>
> +#define DRM_NOUVEAU_CHANNEL_FREE 0x03<br>
> #define DRM_NOUVEAU_GROBJ_ALLOC 0x04 /* deprecated */<br>
> #define DRM_NOUVEAU_NOTIFIEROBJ_ALLOC 0x05 /* deprecated */<br>
> #define DRM_NOUVEAU_GPUOBJ_FREE 0x06 /* deprecated */<br>
> @@ -188,6 +226,10 @@ struct drm_nouveau_svm_bind {<br>
> #define NOUVEAU_SVM_BIND_TARGET__GPU_VRAM (1UL << 31)<br>
><br>
><br>
> +#define DRM_IOCTL_NOUVEAU_GETPARAM DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_GETPARAM, struct drm_nouveau_getparam)<br>
> +#define DRM_IOCTL_NOUVEAU_CHANNEL_ALLOC DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_CHANNEL_ALLOC, struct drm_nouveau_channel_alloc)<br>
> +#define DRM_IOCTL_NOUVEAU_CHANNEL_FREE DRM_IOW (DRM_COMMAND_BASE + DRM_NOUVEAU_CHANNEL_FREE, struct drm_nouveau_channel_free)<br>
> +<br>
> #define DRM_IOCTL_NOUVEAU_SVM_INIT DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_SVM_INIT, struct drm_nouveau_svm_init)<br>
> #define DRM_IOCTL_NOUVEAU_SVM_BIND DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_SVM_BIND, struct drm_nouveau_svm_bind)<br>
><br>
> --<br>
> 2.41.0<br>
><br>
<br>
Reviewed-by: Karol Herbst <<a href="mailto:kherbst@redhat.com" target="_blank">kherbst@redhat.com</a>><br>
<br>
</blockquote></div></div>