Re: [PATCH v3 11/11] gpu: nova-core: vbios: use the first PCI-AT and FWSEC images

[Eliot Courtney]

On Thu Apr 30, 2026 at 2:49 AM JST, Gary Guo wrote:
> On Wed Apr 29, 2026 at 3:32 PM BST, Alexandre Courbot wrote:
>> On Tue Apr 21, 2026 at 5:20 PM JST, Eliot Courtney wrote:
>>> Currently, PCI-AT takes the final image if multiple exist. For FWSEC, it
>>> takes the first one and the last one. Align both of these to nouveau
>>> behavior by taking the first ones.
>>>
>>> Signed-off-by: Eliot Courtney <ecourtney@xxxxxxxxxx>
>>> ---
>>>  drivers/gpu/nova-core/vbios.rs | 8 ++++++--
>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
>>> index 5cc251c73800..8cfc75b1184f 100644
>>> --- a/drivers/gpu/nova-core/vbios.rs
>>> +++ b/drivers/gpu/nova-core/vbios.rs
>>> @@ -251,12 +251,16 @@ pub(crate) fn new(dev: &device::Device, bar0: &Bar0) -> Result<Vbios> {
>>>              // Convert to a specific image type
>>>              match BiosImageType::try_from(image.pcir.code_type) {
>>>                  Ok(BiosImageType::PciAt) => {
>>> -                    pci_at_image = Some(PciAtBiosImage::try_from(image)?);
>>> +                    // Silently ignore any extra PCI-AT images.
>>> +                    if pci_at_image.is_none() {
>>> +                        pci_at_image = Some(PciAtBiosImage::try_from(image)?);
>>> +                    }
>>>                  }
>>
>> I am getting a Clippy here:
>>
>> warning: this `if` can be collapsed into the outer `match`
>>    --> ../drivers/gpu/nova-core/vbios.rs:338:21
>>     |
>> 338 | /                     if pci_at_image.is_none() {
>> 339 | |                         pci_at_image = Some(PciAtBiosImage::try_from(image)?);
>> 340 | |                     }
>>     | |_____________________^
>>     |
>>     = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.95.0/index.html#collapsible_match
>>     = note: `-W clippy::collapsible-match` implied by `-W clippy::all`
>>     = help: to override `-W clippy::all` add `#[allow(clippy::collapsible_match)]`
>> help: collapse nested if block
>>     |
>> 336 ~                 Ok(BiosImageType::PciAt)
>> 337 |                     // Silently ignore any extra PCI-AT images.
>> 338 ~                     if pci_at_image.is_none() => {
>> 339 |                         pci_at_image = Some(PciAtBiosImage::try_from(image)?);
>> 340 ~                     }
>>
>> I have tested this series on Turing and probe completed successfully.


Thank you for testing (and reviewing) this!
>
> Be aware of false positives and the suggested code changes the behaviour. See
> https://lore.kernel.org/rust-for-linux/20260426144201.227108-1-ojeda@xxxxxxxxxx/.
>
> Best,
> Gary

I have to say I am not a fan of this lint in this particular case, since
it moves the "ignore this" semantics to be match level rather than match
branch level. In this particular case it's fine since the _ branch also
silently ignores, but I have been bitten by this kind of match case
lifting making it easy for semantics to be accidentally changed before
IIRC.