On Mon Jun 29, 2026 at 11:09 PM JST, Alexandre Courbot wrote:
> The construction of the unload bundle is currently a bit convoluted and
> could be done in one function instead of two.
>
> Additionally, turn that function into a method of `Tu102`. A following
> patch will turn the "use FWSEC bootloader" property into a flag of the
> TU102 HAL itself, and making this a method will allow the code to access
> it instead of querying `Chipset`.
>
> Signed-off-by: Alexandre Courbot <[email protected]>
> ---
> drivers/gpu/nova-core/gsp/hal/tu102.rs | 87
> ++++++++++++++++------------------
> 1 file changed, 41 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp/hal/tu102.rs
> b/drivers/gpu/nova-core/gsp/hal/tu102.rs
> index ef465b99af05..0505a5beee6e 100644
> --- a/drivers/gpu/nova-core/gsp/hal/tu102.rs
> +++ b/drivers/gpu/nova-core/gsp/hal/tu102.rs
> @@ -56,22 +56,6 @@ enum FwsecUnloadFirmware {
> }
>
> impl FwsecUnloadFirmware {
> - /// Loads the FWSEC SB firmware, as well as its bootloader if `chipset`
> requires it.
> - fn new(
> - dev: &device::Device<device::Bound>,
> - chipset: Chipset,
> - bios: &Vbios,
> - gsp_falcon: &Falcon<'_, GspEngine>,
> - ) -> Result<Self> {
> - let fwsec_sb = FwsecFirmware::new(dev, gsp_falcon, bios,
> FwsecCommand::Sb)?;
> -
> - Ok(if chipset.needs_fwsec_bootloader() {
> - Self::WithBl(FwsecFirmwareWithBl::new(fwsec_sb, dev, chipset)?)
> - } else {
> - Self::WithoutBl(fwsec_sb)
> - })
> - }
> -
> /// Runs the FWSEC SB firmware.
> fn run(
> &self,
> @@ -93,33 +77,6 @@ struct Sec2UnloadBundle {
> booter_unloader: BooterFirmware,
> }
>
> -impl Sec2UnloadBundle {
> - /// Load and prepare the resources required to properly reset the GSP
> after it has been stopped.
> - fn build(
> - dev: &device::Device<device::Bound>,
> - chipset: Chipset,
> - bios: &Vbios,
> - gsp_falcon: &Falcon<'_, GspEngine>,
> - sec2_falcon: &Falcon<'_, Sec2>,
> - ) -> Result<KBox<dyn UnloadBundle>> {
> - KBox::new(
> - Self {
> - fwsec_sb: FwsecUnloadFirmware::new(dev, chipset, bios,
> gsp_falcon)?,
> - booter_unloader: BooterFirmware::new(
> - dev,
> - BooterKind::Unloader,
> - chipset,
> - FIRMWARE_VERSION,
> - sec2_falcon,
> - )?,
> - },
> - GFP_KERNEL,
> - )
> - .map(|b| b as KBox<dyn UnloadBundle>)
> - .map_err(Into::into)
> - }
> -}
> -
> impl UnloadBundle for Sec2UnloadBundle {
> fn run(&self, ctx: &GspBootContext<'_>) -> Result {
> let dev = ctx.dev();
> @@ -257,6 +214,44 @@ fn run_fwsec_frts(
>
> struct Tu102;
>
> +impl Tu102 {
> + /// Load and prepare the resources required to properly reset the GSP
> after it has been stopped.
> + fn build_unload_bundle(
> + &self,
> + dev: &device::Device<device::Bound>,
> + chipset: Chipset,
> + bios: &Vbios,
> + gsp_falcon: &Falcon<'_, GspEngine>,
> + sec2_falcon: &Falcon<'_, Sec2>,
> + ) -> Result<crate::gsp::UnloadBundle> {
> + // Load the FWSEC SB firmware, as well as its bootloader if required.
> + let fwsec_sb =
> + FwsecFirmware::new(dev, gsp_falcon, bios,
> FwsecCommand::Sb).and_then(|fwsec_sb| {
> + Ok(if chipset.needs_fwsec_bootloader() {
> +
> FwsecUnloadFirmware::WithBl(FwsecFirmwareWithBl::new(fwsec_sb, dev, chipset)?)
> + } else {
> + FwsecUnloadFirmware::WithoutBl(fwsec_sb)
> + })
> + })?;
nit: Maybe nicer to write this like:
```
// Load the FWSEC SB firmware, as well as its bootloader if required.
let fwsec_sb = FwsecFirmware::new(dev, gsp_falcon, bios, FwsecCommand::Sb)?;
let fwsec_sb = if self.needs_fwsec_bootloader {
FwsecUnloadFirmware::WithBl(FwsecFirmwareWithBl::new(fwsec_sb, dev,
chipset)?)
} else {
FwsecUnloadFirmware::WithoutBl(fwsec_sb)
};
```
With that,
Reviewed-by: Eliot Courtney <[email protected]>
> +
> + KBox::new(
> + Sec2UnloadBundle {
> + fwsec_sb,
> + booter_unloader: BooterFirmware::new(
> + dev,
> + BooterKind::Unloader,
> + chipset,
> + FIRMWARE_VERSION,
> + sec2_falcon,
> + )?,
> + },
> + GFP_KERNEL,
> + )
> + .map(|b| crate::gsp::UnloadBundle(b))
> + .map_err(Into::into)
> + }
> +}
> +
> impl GspHal for Tu102 {
> fn boot(
> &self,
> @@ -277,10 +272,10 @@ fn boot(
> //
> // If the unload bundle creation fails, the GPU will need to be
> reset before the driver can
> // be probed again.
> - let unload_bundle = Sec2UnloadBundle::build(dev, chipset, &bios,
> gsp_falcon, sec2_falcon)
> + let unload_bundle = self
> + .build_unload_bundle(dev, chipset, &bios, gsp_falcon,
> sec2_falcon)
> .inspect_err(|e| dev_warn!(dev, "Failed to prepare unload
> firmware: {:?}\n", e))
> - .ok()
> - .map(crate::gsp::UnloadBundle);
> + .ok();
>
> // Run the unload bundle to try and recover the GSP if an error
> occurs.
> let unload_guard = ScopeGuard::new_with_data(unload_bundle,
> |unload_bundle| {
