On Sat May 23, 2026 at 9:13 AM JST, John Hubbard wrote: > On 5/18/26 7:55 PM, Eliot Courtney wrote: >> `FwSecBiosBuilder` now only contains `falcon_ucode_offset` which just >> gets passed directly into `FwSecBiosImage`. Remove `FwSecBiosBuilder` >> and construct `FwSecBiosImage` directly, as a simplification. >> >> Reviewed-by: Joel Fernandes <[email protected]> >> Signed-off-by: Eliot Courtney <[email protected]> >> --- >> drivers/gpu/nova-core/vbios.rs | 98 >> +++++++++++++++++------------------------- >> 1 file changed, 39 insertions(+), 59 deletions(-) > ...> @@ -353,15 +349,23 @@ pub(crate) fn new(dev: &device::Device, bar0: > &Bar0) -> Result<Vbios> { >> } >> >> // Using all the images, setup the falcon data pointer in Fwsec. >> - if let (Some(mut second), Some(first), Some(pci_at)) = >> + if let (Some(second), Some(first), Some(pci_at)) = >> (second_fwsec_image, first_fwsec_image, pci_at_image) >> { >> - second >> - .setup_falcon_data(&pci_at, &first) >> + let fwsec_image = FwSecBiosImage::new(pci_at, first, second) >> .inspect_err(|e| dev_err!(dev, "Falcon data setup failed: >> {:?}\n", e))?; >> - Ok(Vbios { >> - fwsec_image: second.build()?, >> - }) >> + >> + if cfg!(debug_assertions) { >> + // Print the desc header for debugging > > Both this patch, and patch 16/20 are doing a tiny bit of work to > preserve this printing. And that looks good. > > However, after careful consideration over several months, I have > come to believe that this printing is no longer earning its keep, > even for debug-level printing. > > A significant fraction of the dmesg debug level output is consumed > by this one print, for example: > > nova-core 0000:01:00.0: PmuLookupTableEntry desc: V3( > FalconUCodeDescV3 { > hdr: 78381825, > stored_size: 59904, > pkc_data_offset: 1444, > interface_offset: 28, > imem_phys_base: 0, > imem_load_size: 57856, > imem_virt_base: 0, > dmem_phys_base: 0, > dmem_load_size: 2048, > engine_id_mask: 1024, > ucode_id: 9, > signature_count: 3, > signature_versions: 7, > _reserved: 37449, > }, > ) > > ...and yet it is exceedingly rare to make use of that particular > data, even when debugging. > > Let's just delete it. As always, bringup people can add it back in > temporarily if they need it. But they likely never will, because new > hardware doesn't hit this path anyway.
Yeah, good idea - I am glad to hear tbh. I wanted to delete these when working on this but I wasn't completely sure what people find useful for debugging etc so I left it alone. Thanks!
