<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: </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: > > From: Dave Airlie <<a href="mailto:airlied@redhat.com" target="_blank">airlied@redhat.com</a>> > > nouveau > 10 years ago had a plan for new multiplexer inside a multiplexer > API using nvif. It never fully reached fruition, fast forward 10 years, > and the new vulkan driver is avoiding libdrm and calling ioctls, and > these 3 ioctls, getparam, channel alloc + free don't seem to be things > we'd want to use nvif for. > > Undeprecate and put them into the uapi header so we can just copy it > into mesa later. > > Signed-off-by: Dave Airlie <<a href="mailto:airlied@redhat.com" target="_blank">airlied@redhat.com</a>> > --- >Â drivers/gpu/drm/nouveau/nouveau_abi16.h | 41 --------------------- >Â include/uapi/drm/nouveau_drm.hÂ Â Â Â Â | 48 +++++++++++++++++++++++-- >Â 2 files changed, 45 insertions(+), 44 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.h b/drivers/gpu/drm/nouveau/nouveau_abi16.h > index 27eae85f33e6..d5d80d0d9011 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_abi16.h > +++ b/drivers/gpu/drm/nouveau/nouveau_abi16.h > @@ -43,28 +43,6 @@ intÂ nouveau_abi16_usif(struct drm_file *, void *data, u32 size); >Â #define NOUVEAU_GEM_DOMAIN_VRAMÂ Â Â (1 << 1) >Â #define NOUVEAU_GEM_DOMAIN_GARTÂ Â Â (1 << 2) > > -struct drm_nouveau_channel_alloc { > -Â Â Â Â uint32_tÂ Â Â fb_ctxdma_handle; > -Â Â Â Â uint32_tÂ Â Â tt_ctxdma_handle; > - > -Â Â Â Â intÂ Â Â Â Â channel; > -Â Â Â Â uint32_tÂ Â Â pushbuf_domains; > - > -Â Â Â Â /* Notifier memory */ > -Â Â Â Â uint32_tÂ Â Â notifier_handle; > - > -Â Â Â Â /* DRM-enforced subchannel assignments */ > -Â Â Â Â struct { > -Â Â Â Â Â Â Â Â uint32_t handle; > -Â Â Â Â Â Â Â Â uint32_t grclass; > -Â Â Â Â } subchan[8]; > -Â Â Â Â uint32_t nr_subchan; > -}; > - > -struct drm_nouveau_channel_free { > -Â Â Â Â int channel; > -}; > - >Â struct drm_nouveau_grobj_alloc { >Â Â Â Â Â intÂ Â Â channel; >Â Â Â Â Â uint32_t handle; > @@ -83,31 +61,12 @@ struct drm_nouveau_gpuobj_free { >Â Â Â Â Â uint32_t handle; >Â }; > > -#define NOUVEAU_GETPARAM_PCI_VENDORÂ Â Â 3 > -#define NOUVEAU_GETPARAM_PCI_DEVICEÂ Â Â 4 > -#define NOUVEAU_GETPARAM_BUS_TYPEÂ Â Â Â 5 > -#define NOUVEAU_GETPARAM_FB_SIZEÂ Â Â Â Â 8 > -#define NOUVEAU_GETPARAM_AGP_SIZEÂ Â Â Â 9 > -#define NOUVEAU_GETPARAM_CHIPSET_IDÂ Â Â 11 > -#define NOUVEAU_GETPARAM_VM_VRAM_BASEÂ Â 12 > -#define NOUVEAU_GETPARAM_GRAPH_UNITSÂ Â Â 13 > -#define NOUVEAU_GETPARAM_PTIMER_TIMEÂ Â Â 14 > -#define NOUVEAU_GETPARAM_HAS_BO_USAGEÂ Â 15 > -#define NOUVEAU_GETPARAM_HAS_PAGEFLIPÂ Â 16 > -struct drm_nouveau_getparam { > -Â Â Â Â uint64_t param; > -Â Â Â Â uint64_t value; > -}; > - >Â struct drm_nouveau_setparam { >Â Â Â Â Â uint64_t param; >Â Â Â Â Â uint64_t value; >Â }; > > -#define DRM_IOCTL_NOUVEAU_GETPARAMÂ Â Â Â Â Â DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_GETPARAM, struct drm_nouveau_getparam) >Â #define DRM_IOCTL_NOUVEAU_SETPARAMÂ Â Â Â Â Â DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_SETPARAM, struct drm_nouveau_setparam) > -#define DRM_IOCTL_NOUVEAU_CHANNEL_ALLOCÂ Â Â DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_CHANNEL_ALLOC, struct drm_nouveau_channel_alloc) > -#define DRM_IOCTL_NOUVEAU_CHANNEL_FREEÂ Â Â Â DRM_IOW (DRM_COMMAND_BASE + DRM_NOUVEAU_CHANNEL_FREE, struct drm_nouveau_channel_free) >Â #define DRM_IOCTL_NOUVEAU_GROBJ_ALLOCÂ Â Â Â DRM_IOW (DRM_COMMAND_BASE + DRM_NOUVEAU_GROBJ_ALLOC, struct drm_nouveau_grobj_alloc) >Â #define DRM_IOCTL_NOUVEAU_NOTIFIEROBJ_ALLOCÂ DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_NOTIFIEROBJ_ALLOC, struct drm_nouveau_notifierobj_alloc) >Â #define DRM_IOCTL_NOUVEAU_GPUOBJ_FREEÂ Â Â Â DRM_IOW (DRM_COMMAND_BASE + DRM_NOUVEAU_GPUOBJ_FREE, struct drm_nouveau_gpuobj_free) > diff --git a/include/uapi/drm/nouveau_drm.h b/include/uapi/drm/nouveau_drm.h > index 853a327433d3..1cd97b3d8eda 100644 > --- a/include/uapi/drm/nouveau_drm.h > +++ b/include/uapi/drm/nouveau_drm.h > @@ -33,6 +33,44 @@ >Â extern "C" { >Â #endif > > +#define NOUVEAU_GETPARAM_PCI_VENDORÂ Â Â 3 > +#define NOUVEAU_GETPARAM_PCI_DEVICEÂ Â Â 4 > +#define NOUVEAU_GETPARAM_BUS_TYPEÂ Â Â Â 5 > +#define NOUVEAU_GETPARAM_FB_SIZEÂ Â Â Â Â 8 > +#define NOUVEAU_GETPARAM_AGP_SIZEÂ Â Â Â 9 > +#define NOUVEAU_GETPARAM_CHIPSET_IDÂ Â Â 11 > +#define NOUVEAU_GETPARAM_VM_VRAM_BASEÂ Â 12 > +#define NOUVEAU_GETPARAM_GRAPH_UNITSÂ Â Â 13 > +#define NOUVEAU_GETPARAM_PTIMER_TIMEÂ Â Â 14 > +#define NOUVEAU_GETPARAM_HAS_BO_USAGEÂ Â 15 > +#define NOUVEAU_GETPARAM_HAS_PAGEFLIPÂ Â 16 > +struct drm_nouveau_getparam { > +Â Â Â Â uint64_t param; > +Â Â Â Â uint64_t value; > +}; > + > +struct drm_nouveau_channel_alloc { > +Â Â Â Â uint32_tÂ Â Â fb_ctxdma_handle; </blockquote><div> </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. </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; > + > +Â Â Â Â intÂ Â Â Â Â channel; </blockquote><div> </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> </div><div>~Faith </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; > + > +Â Â Â Â /* Notifier memory */ > +Â Â Â Â uint32_tÂ Â Â notifier_handle; > + > +Â Â Â Â /* DRM-enforced subchannel assignments */ > +Â Â Â Â struct { > +Â Â Â Â Â Â Â Â uint32_t handle; > +Â Â Â Â Â Â Â Â uint32_t grclass; > +Â Â Â Â } subchan[8]; > +Â Â Â Â uint32_t nr_subchan; > +}; > + > +struct drm_nouveau_channel_free { > +Â Â Â Â int channel; > +}; > + >Â #define NOUVEAU_GEM_DOMAIN_CPUÂ Â Â Â (1 << 0) >Â #define NOUVEAU_GEM_DOMAIN_VRAMÂ Â Â (1 << 1) >Â #define NOUVEAU_GEM_DOMAIN_GARTÂ Â Â (1 << 2) > @@ -126,10 +164,10 @@ struct drm_nouveau_gem_cpu_fini { >Â Â Â Â Â __u32 handle; >Â }; > > -#define DRM_NOUVEAU_GETPARAMÂ Â Â Â Â Â 0x00 /* deprecated */ > +#define DRM_NOUVEAU_GETPARAMÂ Â Â Â Â Â 0x00 >Â #define DRM_NOUVEAU_SETPARAMÂ Â Â Â Â Â 0x01 /* deprecated */ > -#define DRM_NOUVEAU_CHANNEL_ALLOCÂ Â Â 0x02 /* deprecated */ > -#define DRM_NOUVEAU_CHANNEL_FREEÂ Â Â Â 0x03 /* deprecated */ > +#define DRM_NOUVEAU_CHANNEL_ALLOCÂ Â Â 0x02 > +#define DRM_NOUVEAU_CHANNEL_FREEÂ Â Â Â 0x03 >Â #define DRM_NOUVEAU_GROBJ_ALLOCÂ Â Â Â 0x04 /* deprecated */ >Â #define DRM_NOUVEAU_NOTIFIEROBJ_ALLOCÂ 0x05 /* deprecated */ >Â #define DRM_NOUVEAU_GPUOBJ_FREEÂ Â Â Â 0x06 /* deprecated */ > @@ -188,6 +226,10 @@ struct drm_nouveau_svm_bind { >Â #define NOUVEAU_SVM_BIND_TARGET__GPU_VRAMÂ Â Â Â Â Â Â Â (1UL << 31) > > > +#define DRM_IOCTL_NOUVEAU_GETPARAMÂ Â Â Â Â Â DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_GETPARAM, struct drm_nouveau_getparam) > +#define DRM_IOCTL_NOUVEAU_CHANNEL_ALLOCÂ Â Â DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_CHANNEL_ALLOC, struct drm_nouveau_channel_alloc) > +#define DRM_IOCTL_NOUVEAU_CHANNEL_FREEÂ Â Â Â DRM_IOW (DRM_COMMAND_BASE + DRM_NOUVEAU_CHANNEL_FREE, struct drm_nouveau_channel_free) > + >Â #define DRM_IOCTL_NOUVEAU_SVM_INITÂ Â Â Â Â Â DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_SVM_INIT, struct drm_nouveau_svm_init) >Â #define DRM_IOCTL_NOUVEAU_SVM_BINDÂ Â Â Â Â Â DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_SVM_BIND, struct drm_nouveau_svm_bind) > > -- > 2.41.0 > Reviewed-by: Karol Herbst <<a href="mailto:kherbst@redhat.com" target="_blank">kherbst@redhat.com</a>> </blockquote></div></div>