Re: [PATCH 1/2] gpu: nova-core: use kernel lossless integer conversion module

[Danilo Krummrich]

On Mon Aug 31, 2026 at 3:08 AM CEST, Alexandre Courbot wrote:
> On Sun Aug 30, 2026 at 4:14 AM JST, Danilo Krummrich wrote:
>> On Sat Aug 29, 2026 at 5:34 AM CEST, Alexandre Courbot wrote:
>>> On Fri Aug 28, 2026 at 9:09 PM JST, Danilo Krummrich wrote:
>>>> (Cc: Miguel)
>>>>
>>>> On Fri Aug 28, 2026 at 7:33 AM CEST, Alexandre Courbot wrote:
>>>>> The `kernel` crate now features a replacement for our lossless integer
>>>>> conversion routines. Switch to the kernel version and remove our own.
>>>>>
>>>>> Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
>>>>> Reviewed-by: Danilo Krummrich <dakr@xxxxxxxxxx>
>>>>> Reviewed-by: Eliot Courtney <ecourtney@xxxxxxxxxx>
>>>>> ---
>>>>>  drivers/gpu/nova-core/falcon.rs                    |  12 +-
>>>>>  drivers/gpu/nova-core/falcon/fsp.rs                |   4 +-
>>>>>  drivers/gpu/nova-core/fb.rs                        |   2 +-
>>>>>  drivers/gpu/nova-core/fb/hal/gb100.rs              |   6 +-
>>>>>  drivers/gpu/nova-core/firmware.rs                  |   4 +-
>>>>>  drivers/gpu/nova-core/firmware/booter.rs           |   4 +-
>>>>>  drivers/gpu/nova-core/firmware/fwsec.rs            |   2 +-
>>>>>  drivers/gpu/nova-core/firmware/fwsec/bootloader.rs |   4 +-
>>>>>  drivers/gpu/nova-core/firmware/gsp.rs              |   9 +-
>>>>>  drivers/gpu/nova-core/firmware/tlv.rs              |  11 +-
>>>>>  drivers/gpu/nova-core/fsp.rs                       |   8 +-
>>>>>  drivers/gpu/nova-core/gsp.rs                       |   4 +-
>>>>>  drivers/gpu/nova-core/gsp/cmdq.rs                  |  22 +--
>>>>>  drivers/gpu/nova-core/gsp/fw.rs                    |  42 ++--
>>>>>  drivers/gpu/nova-core/gsp/fw/commands.rs           |   4 +-
>>>>>  drivers/gpu/nova-core/gsp/sequencer.rs             |   2 +-
>>>>>  drivers/gpu/nova-core/mctp.rs                      |   8 +-
>>>>>  drivers/gpu/nova-core/num.rs                       | 211 ---------------------
>>>>>  drivers/gpu/nova-core/vbios.rs                     |   2 +-
>>>>>  19 files changed, 78 insertions(+), 283 deletions(-)
>>>>
>>>> Please see the discussion in [1].
>>>>
>>>> If we make this change, then the subsequent conversion to const_as!() would need
>>>> to go through the Rust tree next cycle, which could be a bit of a mess, as I'd
>>>> expect a bunch of conflicts.
>>>>
>>>> Alternatively, we could the the full three cycle dance, or have a signed tag for
>>>> const_as!() and use it right away.
>>>>
>>>> But honestly, the former would just be unnecessary noise. If we don't do the
>>>> latter, let's just keep the nova-core num module until the dust has been
>>>> settled.
>>>>
>>>> [1] https://lore.kernel.org/all/CANiq72k+-fNWZCwGuQ=FTchxgm-X-f6WR=tG=Ercn19ic1edQg@xxxxxxxxxxxxxx/
>>>
>>> Maybe I am missing something, but wouldn't the following work?
>>>
>>> - rc1 gets tagged, `drm-rust-next` gets the `num::casts` module,
>>> - This series gets applied to `drm-rust-next`, then `cv!` (without the
>>>   nova-core conversion patch) in `rust-next`,
>>> - We wait one cycle for `cv!` to trickle down to `drm-rust-next` before
>>>   converting nova-core to use `cv!`.
>>>
>>> Would that work? This series doesn't overlap with the const conversions
>>> that `cv!` covers on purpose, and it would have the benefit of getting
>>> rid of the local `num` module in nova-core quickly.
>>
>> Take casts::usize_into_u32() for instance, drm-rust-next replaces the usage of
>> it next cycle, but rust-next wants to remove the API from
>> rust/kernel/num/casts.rs.
>>
>> In this case rust-next would need to get rid of the users (including nova-core),
>> but drm-rust-next will likely do different changes, so we'd get unnecessary
>> merge conflicts.
>>
>> You can resolve this either by keeping casts::usize_into_u32 and friends around
>> for one additional cycle, so that all users we introduce now are gone. Or, since
>> there's no rush anyways, we just wait one cycle and use the real thing right
>> away.
>
> Sorry, I should have explicitly mentioned that `*_into_*` would remain
> alongside `cv!` for the next cycle.

Maybe we have a different understanding of what "next cycle" means. As of today
we are in the v7.4 cycle, so I call this the current cycle.

In the next cycle, as discussed in [1], rust-next will drop casts::*_into_*(),
and so would drm-rust-next, if we start using casts::*_into_*() in nova-core
this cycle.

I don't want this situation, so we can either keep casts::*_into_*() around for
even longer, or just never start using it.

> The reason why I think it's preferable to use `casts` in nova-core
> sooner is that some of the series currently under review (and that will
> likely be merged next cycle) add more uses of the local `num` methods,
> making the eentual conversion more painful. If we move to use `casts`
> early we can at least avoid the churn on those cases that do not use
> `cv!`.

It would come at the price of adding cross tree dependencies and converting the
cv! parts twice, which is much worse than one clean and independent conversion.