On Fri Aug 21, 2026 at 7:10 AM CEST, Alistair Popple wrote:
> On 2026-08-18 at 06:18 +1000, Danilo Krummrich <[email protected]> wrote...
>> On Tue Aug 11, 2026 at 7:06 AM CEST, Alistair Popple wrote:
>> > diff --git a/include/uapi/drm/nova_drm.h b/include/uapi/drm/nova_drm.h
>> > index ea7665383644..2604e4d2698b 100644
>> > --- a/include/uapi/drm/nova_drm.h
>> > +++ b/include/uapi/drm/nova_drm.h
>> > @@ -118,9 +118,34 @@ struct drm_nova_gem_info {
>> > __u64 size;
>> > };
>> >
>> > +/**
>> > + * struct drm_nova_gpu_info - query DRM GPU info.
>> > + */
>> > +struct drm_nova_gpu_info {
>> > + /**
>> > + * @size: The amount of space allocated by userspace for this structure.
>> > + * The kernel will return the amount of data it did/could actually
>> > write.
>> > + * User space can use this to determine how much of the struct is valid
>> > + * when running against an older kernel.
>> > + */
>> > + __u64 size;
>> > +
>> > + /**
>> > + * @chipid: GPU chip identifier. See &enum drm_nova_chipid for currently
>> > + * known chip identifiers.
>> > + */
>> > + __u32 chipid;
I think we now also want to add a field for the architecture now that chipid is
considered opaque.
>> > +
>> > + /**
>> > + * @pad: 32 bit padding, must be 0.
>> > + */
>> > + __u32 pad;
>> > +};
>>
>> I think we should add the indirection we discussed in [1], i.e. have an
>> indirection via
>>
>> struct drm_nova_info {
>> __u32 id;
>> __u32 size;
>> __u64 info;
>> /* Revserved fields, just in case? */
>> };
>>
>> so we can easily add new info structures, or extend an existing one with a v2
>> without having to create new ioctls for this purpose.
>
> Sorry, I should have called this difference out more explicitly.
>
> Basically I ended up doing it this way because it didn't make much sense to me
> putting an ioctl interface within an ioctl interface when DRM ioctl handling
> can already deal with matching numbers and truncating/extending the struct as
> required. It just leads to more code comparing ID's, etc and I'm not really
> sure
> what the advantage is. Are we concerned about running out of ioctls if we have
> to add other types of info struct?
>
> Doing this as top-level ioctl makes the strace decoders simpler and means we
> can
> just rely on the existing DRM ioctl handling to get everything right rather
> than
> duplicating that in nova-drm. Or is there some other advantage to [1] that
> I've
> missed that isn't solved here?
I don't think we are really concerned about running out of ioctls, but it seems
cleaner and more self-contained than having N ioctls for different info structs
and in the worst case having v2...vN info ioctls.
It also allows us to define a new info type struct whenever we think something
is a new logical info group. Making it per ioctl will always raise the question
of "do we really need a new ioctl for this, can't we just fit it in X", which
over time tends to get messy.
I think eventually we will have a bunch of different info categories. OpenRM
seems to have quite some as well (not too many categories, but with lots of
fields), Xe and amdgpu have even more categories.
> Thanks for looking.
>
> - Alistair
>
>>
>> [1] https://lore.kernel.org/nova-gpu/[email protected]/
>>
>> > #define DRM_NOVA_GETPARAM 0x00
>> > #define DRM_NOVA_GEM_CREATE 0x01
>> > #define DRM_NOVA_GEM_INFO 0x02
>> > +#define DRM_NOVA_GPU_INFO 0x03
>> >
>> > /* Note: this is an enum so that it can be resolved by Rust bindgen. */
>> > enum {
>> > @@ -130,6 +155,8 @@ enum {
>> > struct drm_nova_gem_create),
>> > DRM_IOCTL_NOVA_GEM_INFO = DRM_IOWR(DRM_COMMAND_BASE +
>> > DRM_NOVA_GEM_INFO,
>> > struct drm_nova_gem_info),
>> > + DRM_IOCTL_NOVA_GPU_INFO = DRM_IOWR(DRM_COMMAND_BASE +
>> > DRM_NOVA_GPU_INFO,
>> > + struct drm_nova_gpu_info),
>> > };
>> >
>> > #if defined(__cplusplus)
>> > --
>> > 2.54.0
>>
