Re: [PATCH v4 09/20] gpu: nova-core: vbios: keep PmuLookupTable local in setup_falcon_data

[John Hubbard]

On 5/18/26 7:55 PM, Eliot Courtney wrote:
> This does not need to be stored in `FwSecBiosBuilder` so we can remove
> it from there, and just create and use it locally in
> `setup_falcon_data`.
> 
> Reviewed-by: Joel Fernandes <joelagnelf@xxxxxxxxxx>
> Signed-off-by: Eliot Courtney <ecourtney@xxxxxxxxxx>
> ---
>  drivers/gpu/nova-core/vbios.rs | 25 ++++++-------------------
>  1 file changed, 6 insertions(+), 19 deletions(-)

Reviewed-by: John Hubbard <jhubbard@xxxxxxxxxx>

thanks,
-- 
John Hubbard

> 
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index 8a0e16e6c9e8..cadc6dcffefb 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -338,7 +338,6 @@ pub(crate) fn new(dev: &device::Device, bar0: &Bar0) -> Result<Vbios> {
>                  Ok(BiosImageType::FwSec) => {
>                      let fwsec = FwSecBiosBuilder {
>                          base: image,
> -                        pmu_lookup_table: None,
>                          falcon_ucode_offset: None,
>                      };
>                      if first_fwsec_image.is_none() {
> @@ -711,8 +710,6 @@ struct FwSecBiosBuilder {
>      /// Once FwSecBiosBuilder is constructed, the `falcon_ucode_offset` will be copied into a new
>      /// [`FwSecBiosImage`].
>      ///
> -    /// The [`PmuLookupTable`] starts at the offset of the falcon data pointer.
> -    pmu_lookup_table: Option<PmuLookupTable>,
>      /// The offset of the Falcon ucode.
>      falcon_ucode_offset: Option<usize>,
>  }
> @@ -1012,24 +1009,14 @@ fn setup_falcon_data(
>              offset -= first_fwsec.base.data.len();
>          }
>  
> -        if pmu_in_first_fwsec {
> -            self.pmu_lookup_table = Some(PmuLookupTable::new(
> -                &self.base.dev,
> -                &first_fwsec.base.data[offset..],
> -            )?);
> +        let pmu_lookup_data = if pmu_in_first_fwsec {
> +            &first_fwsec.base.data[offset..]
>          } else {
> -            self.pmu_lookup_table = Some(PmuLookupTable::new(
> -                &self.base.dev,
> -                &self.base.data[offset..],
> -            )?);
> -        }
> +            self.base.data.get(offset..).ok_or(EINVAL)?
> +        };
> +        let pmu_lookup_table = PmuLookupTable::new(&self.base.dev, pmu_lookup_data)?;
>  
> -        match self
> -            .pmu_lookup_table
> -            .as_ref()
> -            .ok_or(EINVAL)?
> -            .find_entry_by_type(FALCON_UCODE_ENTRY_APPID_FWSEC_PROD)
> -        {
> +        match pmu_lookup_table.find_entry_by_type(FALCON_UCODE_ENTRY_APPID_FWSEC_PROD) {
>              Ok(entry) => {
>                  self.falcon_ucode_offset = Some(
>                      usize::from_safe_cast(entry.data)
>