Re: [PATCH 2/3] drm/amdgpu: Add peer-to-peer support among PCIe connected AMD GPUs

[Felix Kuehling <felix.kuehling@xxxxxxx>]

Am 2022-06-06 um 16:04 schrieb Felix Kuehling:
> Am 2022-06-06 um 14:07 schrieb Ramesh Errabolu:
> > Add support for peer-to-peer communication among AMD GPUs over PCIe
> > bus. Support REQUIRES enablement of config HSA_AMD_P2P.
> >
> > Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu@xxxxxxx>
>
> Sorry, one more nit-pick inline. With that fixed, the patch is
>
> Reviewed-by: Felix Kuehling <Felix.Kuehling@xxxxxxx>

Really inline now ...


> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index be0baacc5942..8e88e7c88d26 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -34,6 +34,7 @@
> >   #include <linux/pci.h>
> >   #include <linux/devcoredump.h>
> >   #include <generated/utsrelease.h>
> > +#include <linux/pci-p2pdma.h>
> >     #include <drm/drm_atomic_helper.h>
> >   #include <drm/drm_probe_helper.h>
> > @@ -129,6 +130,8 @@ const char *amdgpu_asic_name[] = {
> >       "LAST",
> >   };
> >   +extern bool pcie_p2p;
> > +

This should be declared in amdgpu.h along with other module parameters.


> >   /**
> >    * DOC: pcie_replay_count
> >    *
> > @@ -5498,6 +5501,36 @@ static void amdgpu_device_get_pcie_info(struct 
> > amdgpu_device *adev)
> >       }
> >   }
> >   +/**
> > + * amdgpu_device_is_peer_accessible - Check peer access through PCIe 
> > BAR
> > + *
> > + * @adev: amdgpu_device pointer
> > + * @peer_adev: amdgpu_device pointer for peer device trying to 
> > access @adev
> > + *
> > + * Return true if @peer_adev can access (DMA) @adev through the PCIe
> > + * BAR, i.e. @adev is "large BAR" and the BAR matches the DMA mask of
> > + * @peer_adev.
> > + */
> > +bool amdgpu_device_is_peer_accessible(struct amdgpu_device *adev,
> > +                      struct amdgpu_device *peer_adev)
> > +{
> > +#ifdef CONFIG_HSA_AMD_P2P
> > +    uint64_t address_mask = peer_adev->dev->dma_mask ?
> > +        ~*peer_adev->dev->dma_mask : ~((1ULL << 32) - 1);
> > +    resource_size_t aper_limit =
> > +        adev->gmc.aper_base + adev->gmc.aper_size - 1;
> > +    bool p2p_access = !(pci_p2pdma_distance_many(adev->pdev,
> > +                    &peer_adev->dev, 1, true) < 0);
> > +
> > +    return pcie_p2p && p2p_access && (adev->gmc.visible_vram_size &&
> > +        adev->gmc.real_vram_size == adev->gmc.visible_vram_size &&
> > +        !(adev->gmc.aper_base & address_mask ||
> > +          aper_limit & address_mask));
> > +#else
> > +    return false;
> > +#endif
> > +}
> > +
> >   int amdgpu_device_baco_enter(struct drm_device *dev)
> >   {
> >       struct amdgpu_device *adev = drm_to_adev(dev);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index bed4ed88951f..d1c82a9e8569 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -802,6 +802,14 @@ MODULE_PARM_DESC(no_queue_eviction_on_vm_fault, 
> > "No queue eviction on VM fault (
> >   module_param_named(no_queue_eviction_on_vm_fault, 
> > amdgpu_no_queue_eviction_on_vm_fault, int, 0444);
> >   #endif
> >   +/**
> > + * DOC: pcie_p2p (bool)
> > + * Enable PCIe P2P (requires large-BAR). Default value: true (on)
> > + */
> > +bool pcie_p2p = true;
> > +module_param(pcie_p2p, bool, 0444);
> > +MODULE_PARM_DESC(pcie_p2p, "Enable PCIe P2P (requires large-BAR). (N 
> > = off, Y = on(default))");
> > +
> >   /**
> >    * DOC: dcfeaturemask (uint)
> >    * Override display features enabled. See enum DC_FEATURE_MASK in 
> > drivers/gpu/drm/amd/include/amd_shared.h.