[Skiboot] [PATCH 2/2] nvlink: Associate NPUs with GPUs using slots instead of PBCQ

[Russell Currey]

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 at 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;
> >  }
> >