| Message ID | 1463978605-30091-2-git-send-email-ruscur@russell.cc |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Russell, We also need to update the bdfn allocation - see npu_allocate_bdfn(). At the moment we compare pbcq's to determine if nvlink devices go to the same GPU or not. Now that we have slot information we need to update the bdfn allocator to use that instead to determine if nvlinks are going to the same GPU or not, similar to what you've done below for the device binding. A final patch to remove the ibm,npu-pbcq properties and associated code from platforms/astbmc/garrison.c would also be great if you have time. Thanks! Regards, Alistair On Mon, 23 May 2016 14:43:25 Russell Currey wrote: > NPU links (npu_dev) need to be associated with their corresponding > physical GPU's PCI devices. On Garrison systems, this works by comparing > the PBCQ handle of the NPU and GPU. However, this will fail if a system > has multiple GPUs behind a single PHB. Instead, associate GPUs and NPUs > by comparing slot locations. > > Signed-off-by: Russell Currey <ruscur@russell.cc> > --- > hw/npu.c | 18 ++++++++---------- > 1 file changed, 8 insertions(+), 10 deletions(-) > > diff --git a/hw/npu.c b/hw/npu.c > index 476a1d2..ac5e0d7 100644 > --- a/hw/npu.c > +++ b/hw/npu.c > @@ -29,6 +29,7 @@ > #include <npu-regs.h> > #include <npu.h> > #include <xscom.h> > +#include <string.h> > > /* > * Terminology: > @@ -502,25 +503,22 @@ static int __npu_dev_bind_pci_dev(struct phb *phb __unused, > { > struct npu_dev *dev = data; > struct dt_node *pci_dt_node; > - uint32_t npu_npcq_phandle; > > /* Ignore non-nvidia PCI devices */ > if ((pd->vdid & 0xffff) != 0x10de) > return 0; > > - /* Find the PCI devices pbcq */ > - for (pci_dt_node = pd->dn->parent; > - pci_dt_node && !dt_find_property(pci_dt_node, "ibm,pbcq"); > + /* Find the PCI device's slot location */ > + for (pci_dt_node = pd->dn; > + pci_dt_node && !dt_find_property(pci_dt_node, "ibm,slot-label"); > pci_dt_node = pci_dt_node->parent); > > - if (!pci_dt_node) > + if (!pci_dt_node || !dt_find_property(pci_dt_node, "ibm,slot-label")) > return 0; > > - npu_npcq_phandle = dt_prop_get_u32(dev->dt_node, "ibm,npu-pbcq"); > - > - if (dt_prop_get_u32(pci_dt_node, "ibm,pbcq") == npu_npcq_phandle && > - (pd->vdid & 0xffff) == 0x10de) > - return 1; > + if (streq((char *)dt_prop_get(pci_dt_node, "ibm,slot-label"), > + dev->slot_info->label)) > + return 1; > > return 0; > } >
On Thu, 2016-05-26 at 15:01 +1000, Alistair Popple wrote: > Hi Russell, > > We also need to update the bdfn allocation - see npu_allocate_bdfn(). At the > moment we compare pbcq's to determine if nvlink devices go to the same GPU or > not. Now that we have slot information we need to update the bdfn allocator > to > use that instead to determine if nvlinks are going to the same GPU or not, > similar to what you've done below for the device binding. > > A final patch to remove the ibm,npu-pbcq properties and associated code from > platforms/astbmc/garrison.c would also be great if you have time. Thanks! > Makes sense, will do. > Regards, > > Alistair > > On Mon, 23 May 2016 14:43:25 Russell Currey wrote: > > NPU links (npu_dev) need to be associated with their corresponding > > physical GPU's PCI devices. On Garrison systems, this works by comparing > > the PBCQ handle of the NPU and GPU. However, this will fail if a system > > has multiple GPUs behind a single PHB. Instead, associate GPUs and NPUs > > by comparing slot locations. > > > > Signed-off-by: Russell Currey <ruscur@russell.cc> > > --- > > hw/npu.c | 18 ++++++++---------- > > 1 file changed, 8 insertions(+), 10 deletions(-) > > > > diff --git a/hw/npu.c b/hw/npu.c > > index 476a1d2..ac5e0d7 100644 > > --- a/hw/npu.c > > +++ b/hw/npu.c > > @@ -29,6 +29,7 @@ > > #include <npu-regs.h> > > #include <npu.h> > > #include <xscom.h> > > +#include <string.h> > > > > /* > > * Terminology: > > @@ -502,25 +503,22 @@ static int __npu_dev_bind_pci_dev(struct phb *phb > __unused, > > { > > struct npu_dev *dev = data; > > struct dt_node *pci_dt_node; > > - uint32_t npu_npcq_phandle; > > > > /* Ignore non-nvidia PCI devices */ > > if ((pd->vdid & 0xffff) != 0x10de) > > return 0; > > > > - /* Find the PCI devices pbcq */ > > - for (pci_dt_node = pd->dn->parent; > > - pci_dt_node && !dt_find_property(pci_dt_node, "ibm,pbcq"); > > + /* Find the PCI device's slot location */ > > + for (pci_dt_node = pd->dn; > > + pci_dt_node && !dt_find_property(pci_dt_node, "ibm,slot- > > label"); > > pci_dt_node = pci_dt_node->parent); > > > > - if (!pci_dt_node) > > + if (!pci_dt_node || !dt_find_property(pci_dt_node, "ibm,slot- > > label")) > > return 0; > > > > - npu_npcq_phandle = dt_prop_get_u32(dev->dt_node, "ibm,npu-pbcq"); > > - > > - if (dt_prop_get_u32(pci_dt_node, "ibm,pbcq") == npu_npcq_phandle && > > - (pd->vdid & 0xffff) == 0x10de) > > - return 1; > > + if (streq((char *)dt_prop_get(pci_dt_node, "ibm,slot-label"), > > + dev->slot_info->label)) > > + return 1; > > > > return 0; > > } > >
diff --git a/hw/npu.c b/hw/npu.c index 476a1d2..ac5e0d7 100644 --- a/hw/npu.c +++ b/hw/npu.c @@ -29,6 +29,7 @@ #include <npu-regs.h> #include <npu.h> #include <xscom.h> +#include <string.h> /* * Terminology: @@ -502,25 +503,22 @@ static int __npu_dev_bind_pci_dev(struct phb *phb __unused, { struct npu_dev *dev = data; struct dt_node *pci_dt_node; - uint32_t npu_npcq_phandle; /* Ignore non-nvidia PCI devices */ if ((pd->vdid & 0xffff) != 0x10de) return 0; - /* Find the PCI devices pbcq */ - for (pci_dt_node = pd->dn->parent; - pci_dt_node && !dt_find_property(pci_dt_node, "ibm,pbcq"); + /* Find the PCI device's slot location */ + for (pci_dt_node = pd->dn; + pci_dt_node && !dt_find_property(pci_dt_node, "ibm,slot-label"); pci_dt_node = pci_dt_node->parent); - if (!pci_dt_node) + if (!pci_dt_node || !dt_find_property(pci_dt_node, "ibm,slot-label")) return 0; - npu_npcq_phandle = dt_prop_get_u32(dev->dt_node, "ibm,npu-pbcq"); - - if (dt_prop_get_u32(pci_dt_node, "ibm,pbcq") == npu_npcq_phandle && - (pd->vdid & 0xffff) == 0x10de) - return 1; + if (streq((char *)dt_prop_get(pci_dt_node, "ibm,slot-label"), + dev->slot_info->label)) + return 1; return 0; }
NPU links (npu_dev) need to be associated with their corresponding physical GPU's PCI devices. On Garrison systems, this works by comparing the PBCQ handle of the NPU and GPU. However, this will fail if a system has multiple GPUs behind a single PHB. Instead, associate GPUs and NPUs by comparing slot locations. Signed-off-by: Russell Currey <ruscur@russell.cc> --- hw/npu.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)