lkml.org 
[lkml]   [2021]   [Oct]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2021-10-27 22:55    [W:0.120 / U:0.136 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site