Re: [PATCH v4 4/7] drm: nova: Add a GPU info ioctl

["Danilo Krummrich" <dakr@kernel.org>]

On Tue Aug 25, 2026 at 9:32 AM CEST, Alistair Popple wrote:
> On 2026-08-25 at 05:10 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
>> 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.
>
> But isn't v2...vN info ioctls dealt with in the usual way by extending the
> existing struct and bumping the size? That seems like a pretty clean and
> self-contained API to me.

To be clear, my main point is that having a single info ioctl with different
info types is more self-contained and provides more flexibility to introduce new
info types whenever we think it is warranted. Long term I expect it to be the
cleaner API.

>> 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.
>
> Doesn't that question also apply to adding GETPARAM N+1 though? If we're not
> worried about running out of top-level ioctls I don't understand why they
> are considered special enough to warrant the extra complexity of creating and
> decoding a hiearchy of sub-ioctls.

I think the code would be rather trivial:

	fn write_info<T: AsBytes>(info: &mut uapi::drm_nova_info, value: &T) -> Result {
	    let len = size_of_val(value).min(info.size);
	    let uptr = UserPtr::from_addr(info.data);
	    let mut writer = UserSlice::new(uptr, len).writer();

	    // Note: I made this up, as I think we want to add this method to
	    // `UserSliceWriter`, to avoid having to call `as_bytes()`.
	    writer.write_truncated(&value)?;
	    info.size = len;

	    Ok(())
	}

	match info.id {
	    uapi::DRM_NOVA_INFO_GPU => write_info(info, &uapi::drm_nova_gpu_info { ... })?,
	    uapi::DRM_NOVA_INFO_MEM => write_info(info, &uapi::drm_nova_mem_info { ... })?,
	    _ => return Err(EINVAL),
	}

Honestly, I think this is even less complicated that adding a new ioctl for a
new info struct.

And on the userspace side:

	fn query_info<T: Default>(fd: &DrmDevice, id: DrmNovaInfoId) -> Result<T> {
	    let mut value = T::default();
	    let mut info = drm_nova_info {
	        id: id.as_raw(),
	        size: size_of::<T>(),
	        data: ptr::from_mut(&mut value) as u64,
	    };

	    fd.ioctl(DRM_IOCTL_NOVA_INFO, &mut info)?;
	    Ok(value)
	}

	let gpu_info: drm_nova_gpu_info = query_info(&dev, DRM_NOVA_INFO_GPU)?;
	let mem_info: drm_nova_mem_info = query_info(&dev, DRM_NOVA_INFO_MEM)?;

(I pushed a few cleanups to drm-test, so this code should work.)