[Nouveau] [PATCH] drm/nouveau: fixup the uapi header file.

Karol Herbst kherbst at redhat.com
Tue Aug 1 16:04:11 UTC 2023


On Tue, Aug 1, 2023 at 5:15 PM Faith Ekstrand <faith at gfxstrand.net> wrote:

> On Tue, Aug 1, 2023 at 4:37 AM Karol Herbst <kherbst at redhat.com> wrote:
>
>> On Mon, Jul 31, 2023 at 9:16 PM Dave Airlie <airlied at gmail.com> wrote:
>> >
>> > From: Dave Airlie <airlied at redhat.com>
>> >
>> > 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 <airlied at redhat.com>
>> > ---
>> >  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;
>>
>
> 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.
>
>
>> > +       uint32_t     tt_ctxdma_handle;
>> > +
>> > +       int          channel;
>>
>
> 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.
>
>
Do we have any architectures we care about where `int` isn't `__s32`? I
think on all 32/64 bit x86, ppc and arm it's that way and I don't think we
care about anything else?



> ~Faith
>
>
>> > +       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 <kherbst at redhat.com>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20230801/078398e3/attachment-0001.htm>


More information about the Nouveau mailing list