Messages in this thread | | | Date | Wed, 27 Oct 2021 20:54:58 +0000 | From | Yazen Ghannam <> | Subject | Re: [PATCH v5 1/5] x86/amd_nb: Add support for northbridges on Aldebaran |
| |
On Mon, Oct 25, 2021 at 08:20:14PM +0530, Naveen Krishna Chatradhi wrote: ... > +/* GPU Data Fabric ID Device 24 Function 1 */ > +#define PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F1 0x14d1 > + > +static struct pci_dev *get_gpu_df_f1(void) > +{ > + return pci_get_device(PCI_VENDOR_ID_AMD, > + PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F1, NULL); > +} > + > +/* DF18xF1 registers on Aldebaran GPU */ > +#define REG_LOCAL_NODE_TYPE_MAP 0x144 > +#define REG_RMT_NODE_TYPE_MAP 0x148 > + > +/* > + * Newer AMD CPUs and GPUs whose data fabrics can be connected via custom xGMI > + * links, comes with registers to gather local and remote node type map info. > + * > + * "Local Node Type" refers to nodes with the same type as that from which the > + * register is read, and "Remote Node Type" refers to nodes with a different type. > + * > + * This function, reads the registers from GPU DF function 1. > + * Hence, local nodes are GPU and remote nodes are CPUs. > + */ > +static int amd_get_node_map(struct pci_dev *pdev) > +{ > + struct amd_node_map *nodemap; > + u32 tmp; > + > + nodemap = kmalloc(sizeof(*nodemap), GFP_KERNEL); > + if (!nodemap) > + return -ENOMEM; > + > + pci_read_config_dword(pdev, REG_LOCAL_NODE_TYPE_MAP, &tmp); > + nodemap->gpu_node_start_id = tmp & 0xFFF; > + > + pci_read_config_dword(pdev, REG_RMT_NODE_TYPE_MAP, &tmp); > + nodemap->cpu_node_count = tmp >> 16 & 0xFFF; > + > + amd_northbridges.nodemap = nodemap; > + return 0; > +} > + ... > int amd_cache_northbridges(void) > { > const struct pci_device_id *misc_ids = amd_nb_misc_ids; > const struct pci_device_id *link_ids = amd_nb_link_ids; > const struct pci_device_id *root_ids = amd_root_ids; > - struct pci_dev *root, *misc, *link; > + struct pci_dev *root, *misc, *link, *dff1; ... > + /* > + * The number of miscs, roots and roots_per_misc might vary on different > + * nodes of a heterogeneous system. > + * Calculate roots_per_misc accordingly in order to skip the redundant > + * roots and map the DF/SMN interfaces to correct PCI roots. > + */ > + if (gpu_root_count && gpu_misc_count) { > + dff1 = get_gpu_df_f1();
dff1 is only used in this section, so it can be declared here.
> + if (!dff1) { > + pr_debug("Failed to gather GPU node info.\n");
This message doesn't make sense to me. The failure is that a particular PCI device was not found. Gathering node info hasn't even been attempted yet.
> + return -ENODEV; > + } > + > + if (amd_get_node_map(dff1)) > + return -ENOMEM; > +
Also, why do these functions need to be split up? If it was because of the return values, then you could have done the following.
int ret = amd_get_node_map();
if (ret) return ret;
Thanks, Yazen
| |