On Wed Aug 20, 2025 at 12:08 PM JST, John Hubbard wrote:
> NovaCore has so far been too imprecise about figuring out if .probe()
> has found a supported PCI PF (Physical Function). By that I mean:
> .probe() sets up BAR0 (which involves a lot of very careful devres and
> Device<Bound> details behind the scenes). And then if it is dealing with
> a non-supported device such as the .1 audio PF on many GPUs, it fails
> out due to an unexpected BAR0 size. We have been fortunate that the BAR0
> sizes are different.
>
> Really, we should be filtering on PCI class ID instead. These days I
> think we can confidently pick out Nova's supported PF's via PCI class
> ID. And if not, then we'll revisit.
>
> The approach here is to filter on "Display VGA" or "Display 3D", which
> is how PCI class IDs express "this is a modern GPU's PF".
>
> Cc: Danilo Krummrich <dakr@xxxxxxxxxx>
> Cc: Alexandre Courbot <acourbot@xxxxxxxxxx>
> Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
> ---
> drivers/gpu/nova-core/driver.rs | 28 +++++++++++++++++++++++-----
> 1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> index 274989ea1fb4..b60c9defa9d1 100644
> --- a/drivers/gpu/nova-core/driver.rs
> +++ b/drivers/gpu/nova-core/driver.rs
> @@ -1,6 +1,9 @@
> // SPDX-License-Identifier: GPL-2.0
>
> -use kernel::{auxiliary, bindings, c_str, device::Core, pci, prelude::*, sizes::SZ_16M, sync::Arc};
> +use kernel::{
> + auxiliary, bindings, c_str, device::Core, pci, pci::Class, pci::ClassMask, prelude::*,
> + sizes::SZ_16M, sync::Arc,
> +};
>
> use crate::gpu::Gpu;
>
> @@ -18,10 +21,25 @@ pub(crate) struct NovaCore {
> PCI_TABLE,
> MODULE_PCI_TABLE,
> <NovaCore as pci::Driver>::IdInfo,
> - [(
> - pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_NVIDIA, bindings::PCI_ANY_ID as u32),
> - ()
> - )]
> + [
> + // Modern NVIDIA GPUs will show up as either VGA or 3D controllers.
> + (
> + pci::DeviceId::from_class_and_vendor(
> + Class::DISPLAY_VGA,
> + ClassMask::ClassSubclass,
> + bindings::PCI_VENDOR_ID_NVIDIA
> + ),
> + ()
> + ),
> + (
> + pci::DeviceId::from_class_and_vendor(
> + Class::DISPLAY_3D,
> + ClassMask::ClassSubclass,
> + bindings::PCI_VENDOR_ID_NVIDIA
> + ),
This is making use of `from_class_and_vendor`, which is modified in the
next patch, requiring to modify this part of the file again. How about
switching this patch with 3/3 so we only modify the nova-core code once?
I also wonder if we want to merge 1/3 and (the current) 3/3, since 1/3
alone leaves `from_class_and_vendor` into some intermediate state that
nobody will ever get a chance to use anyway, and one doesn't really make
sense without the other. WDYT?
