On Thu Sep 11, 2025 at 8:27 PM JST, Danilo Krummrich wrote:
> On 9/11/25 1:04 PM, Alexandre Courbot wrote:
>> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
>> index
>> 06a7ee8f4195759fb55ad483852724bb1ab46793..8505ee81c43e7628d1f099aff285244f8908c633
>> 100644
>> --- a/drivers/gpu/nova-core/gpu.rs
>> +++ b/drivers/gpu/nova-core/gpu.rs
>> @@ -8,6 +8,7 @@
>> use crate::fb::SysmemFlush;
>> use crate::firmware::booter::{BooterFirmware, BooterKind};
>> use crate::firmware::fwsec::{FwsecCommand, FwsecFirmware};
>> +use crate::firmware::gsp::GspFirmware;
>> use crate::firmware::{Firmware, FIRMWARE_VERSION};
>> use crate::gfw;
>> use crate::regs;
>> @@ -285,6 +286,11 @@ pub(crate) fn start_gsp(
>>
>> let bios = Vbios::new(dev, bar)?;
>>
>> + let _gsp_fw = KBox::pin_init(
>> + GspFirmware::new(dev, chipset, FIRMWARE_VERSION)?,
>> + GFP_KERNEL,
>> + )?;
>
> Since we now have the infrastructure in place and the change is trival, I'd
> prefer to make this a member of struct Gsp and make it part of the Gpu
> structure
> directly (without separate allocation).
>
> Should we need dynamic dispatch in the future, it's also trivial to make it
> its
> own allocation again, but maybe we also get around the dyn dispatch. :)
Ah, my previous talk about dynamic dispatch is a bit obsolete now that
the `Firmware` struct is gone. :) Sorry if that created confusion.
Truth is, this object is not supposed to survive `start_gsp`, and we can
dispose of it after the GSP is started as the bootloader will have
validated and copied it into the WPR region. So we don't want to store
it into `Gpu`, now or ever.
I guess we could technically store it on the stack, but IIRC I haven't
found a pin initializer for that in the kernel. And the structure might
be a little bit too big for that (several owned SGTables and a couple of
CoherentAllocations - we're talking hundreds of bytes).
