Re: [PATCH v3 3/7] drm: nova: Add chipid enum to nova-drm UAPI

On Tue Jul 28, 2026 at 11:49 PM CEST, John Hubbard wrote:
> On 7/28/26 2:29 PM, Danilo Krummrich wrote:
>> On Tue Jul 28, 2026 at 10:00 AM CEST, Alistair Popple wrote:
>>> On 2026-07-26 at 01:23 +1000, Danilo Krummrich <[email protected]> wrote.=
..
>>>> On Thu Jul 23, 2026 at 8:30 AM CEST, Alistair Popple wrote:
> ...
>>>> For instance, what's valid for userspace to extract from this? Given c=
hipid is a
>>>> composed value, is userspace e.g. allowed to make assumptions on extra=
cting the
>>>> architecture? Or is this something we want to expose separately and te=
ll
>>>> userspace that the chipid is an opaque value identifying a specific ch=
ip only?
>>>
>>> I _think_ it makes sense for this to just be an opaque value, but it's =
a good
>>> question that I should probably try and get some internal feedback on a=
s well.
>>> Reading the arch would then be a separate parameter.
>>=20
>> I'd prefer exposing it as an opaque value and provide other information
>> separately; the kernel already does the work anyway, so why duplicate it=
 in
>> userspace. It would also remain much cleaner if encoding details ever ch=
ange.
>>=20
>
> Just to orient myself, I'm assuming that by "opaque values", you mean usi=
ng
> something that just counts up from, say, 1, instead of using real boot42
> values below, right?
>
> That seems like a good approach to me, too. Because I can't imagine anyth=
ing
> particularly good coming from providing unnecessary details to user space
> here. :)

I'd probably not be too worried about keeping it as is, but just document i=
t as
being an opaque value that shouldn't be messed with.

If we expose the encoded information separately I don't see a huge incentiv=
e for
userspace to redo the work and mess with it anyway.

The advantage of keeping the values as they are is that we avoid having to =
deal
with mapping values back and forth. I kind of like that nova-core uses the
values from the uAPI header to define the Chipset struct, passes it to nova=
-drm
and nova-drm does use it to fill in the userspace exposed data.

Something else worth to consider in this context is whether we really want =
to go
with the key-value pair GETPARAM approach, which can become a bit tedious.

I think for the basic GPU information we could have a struct drm_nova_gpu_i=
nfo,
which contains all the relevant information obtained at probe time, such as
chipid, architecture, VRAM size, etc.

If everything is available in a single struct there should be no reason for
userspace to mess with the chipid value. However, I'd also not object to ma=
ke it
a "real" opaque value.

In order to deal with additions to the struct we could take a two stage app=
roach
to obtain the information from userspace; probe size first, then allocate w=
ith
the returned size:

We can have a DRM_NOVA_INFO ioctl, which takes a struct containing the info
identifier (e.g. DRM_NOVA_GPU_INFO), a size and a pointer to the actual inf=
o
structure.

	struct drm_nova_info {
		__u32 id;
		__u32 size;
		__u64 info;
	};

Userspace can call the DRM_NOVA_INFO with the identifier (e.g.
DRM_NOVA_GPU_INFO), size and info pointer set to zero, and the kernel fills=
 in
the size of the info struct (e.g. struct drm_nova_gpu_info).

Subsequently, userspace can allocate memory with the size reported by the k=
ernel
and call the same ioctl with the info pointer set to the allocated memory.

This way existing info structs remain extensible (as long as the layout isn=
't
changed of course) and new ones can be added at any time.