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

[Alexandre Courbot]

On Mon Aug 31, 2026 at 9:50 PM JST, Danilo Krummrich wrote:
> 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.

Got it - let's keep nova-core's `num` for one more cycle then. This will
make for a larger conversion, but the silver lining is that this means
less rebasing for the patches currently in-flight.

I'll revisit this series for 7.5 (?), although I guess patch 2 is still
relevant as long as it keeps using nova-core's `num` crate.