Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Danilo Krummrich]

On Wed Sep 2, 2026 at 4:38 AM CEST, Alistair Popple wrote:
> So I'm ok with providing either a decoded architecture and implementation
> xor an opaque chip-id. Or alternatively maybe architecture includes the
> implementation (ie. we rename the opaque chip_id to architecture). But treating
> the implementation and architecture values differently and only providing one
> directly doesn't make much sense IMHO.

I think this is you still thinking about this in terms of register encoding.

Look at nova-core, there we are caring about two things chipid and architecture,
but never about the implementation bits.

You won't see any

	if (arch == X && impl == Y)

checks anywhere, because it would be unnecessarily complicated and error prone.
You will instead find checks for the architecture or the chipid directly, such
as in:

	/// Returns the HAL corresponding to `chipset`.
	pub(super) fn fb_hal(chipset: Chipset) -> &'static dyn FbHal {
	    match chipset.arch() {
	        Architecture::Turing => tu102::TU102_HAL,
	        Architecture::Ampere if chipset == Chipset::GA100 => ga100::GA100_HAL,
	        Architecture::Ampere | Architecture::Ada => ga102::GA102_HAL,
	        Architecture::Hopper => gh100::GH100_HAL,
	        Architecture::BlackwellGB10x => gb100::GB100_HAL,
	        Architecture::BlackwellGB20x => gb202::GB202_HAL,
	    }
	}

See? We never care about the implementation part, as it is just an
implementation detail of the encoding of the chipid that no one should ever
bother with.

Yes, the check in fb_hal() could technically be

	arch == Architecture::Ampere && impl == 0

but that's arguably worse than what fb_hal() does today for many reasons.

To name just one of them: We'd entirely lose the guarantee that the combination
of arch and impl even exists in the first place. Even with a new type this
wouldn't go away, since not every Implementation would be valid for any
Architecture.

Also, look at the code that you had to write to even expose the implementation
bits in the first place.

	pub(crate) const fn implementation(self) -> u32 {
	    self as u32 & 0xf
	} 

Notice the tension it creates? You have to reimplement what the lower layer of
the register encoding already does and intentionally hides in favor of providing
a chipset() accessor.

Also note that with this you get a raw integer that is kinda ugly, because you
can't even make up a useful new type:

For Architecture the variants are obvious and meaningful (Turing, Ampere, etc.),
for Chipset the variants are obvious and meaningful too (TU102, AD107, etc.).
But what would the variants for Implementation look like? "Zero", "Two", etc.?

> Except the architecture/implementation tuple is exaclty what user-space needs to
> eg. figure out what SM to compile for.

It doesn't need an architecture/implementation tuple, it needs a chipid for this
lookup. (Although it might be questionable whether userspace should have this
lookup table in the first place; see below.)

Both the KMD and the UMD only ever care about the architecture or the chipid.
The fact that the chipid is defined by an architecture/implementation tuple is
an irrelevant implementation detail not even the kernel cares about.

> And to be clear we don't care about the specific encodings in this example. The
> point is the SM version can't be looked up from architecture alone, it needs the
> implementation as well and if the only way to get that is from opaque chip-id
> that's all user-space will look at. Eg:

Again, it doesn't need the implementation, it needs the chipid. Which also your
code below correctly considers.

That said, if we know that userspace will never need to do an architecture based
check, but always a chipid specific check, it is obviously pointless to expose
it in the first place. But otherwise it should just be chipid and architecture.

> static uint8_t
> sm_for_chipset(enum chip_id chip)
> {
> 	switch (chip) {
> 		case NOVA_GPU_CHIP_GA100:
> 			return 80;
> 		case NOVA_GPU_CHIP_GA101:
> 			return 86;
> 		case NOVA_GPU_CHIP_GA102:
> 			return 86;
> 		case NOVA_GPU_CHIP_GA10B:
> 			return 87;
> 		...
> 	}
> }

That looks reasonable and much better than what mesa has, but if we'd ever care
about the SM value in the kernel, then the kernel should be the single source of
truth for this value and expose it to userspace.

Now, in this case I don't think the kernel really needs the value, but I think
the value is provided by GSP through GR_INFO_INDEX_SM_VERSION?

Given that, the kernel should query it and provide it via its GPU info structure
rather than having userspace invent another lookup table?