Re: [PATCH v5 08/12] gpu: nova-core: firmware: process and prepare the GSP firmware

Danilo Krummrich
2025-09-11T12:31:21Z

On 9/11/25 2:29 PM, Alexandre Courbot wrote:
> 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.

Ah, makes sense, so that's fine then.