[PATCH 12/12] drm/amdgpu: enable foreign DMA-buf objects

[Felix Kuehling]

On 17-07-04 03:32 AM, Christian König wrote:
> Am 03.07.2017 um 23:11 schrieb Felix Kuehling:
>> +
>> +    list_add(&gobj->list, &bo->gem_objects);
>> +    gobj->bo = amdgpu_bo_ref(bo);
>> +    bo->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
>
> It's a bit more tricker than that. IIRC my original patch limited the
> BO to GTT space as well.

I dug out your original patch for reference (attached), which was
originally applied on a 4.1-based KFD staging branch. Your original
patch series also included a change for VRAM VM mappings with the system
bit (also attached). So your original intention was clearly to also
allow VRAM P2P access. The VM patch didn't apply after some VM changes
(probably related to the vram manager) and was later replaced by Amber's
patch.

>
> VRAM peer to peer access doesn't work with most PCIe chipsets.
>
> At bare minimum we need to put this behind a config option or add a
> white list for the chipset or only enable it if
> "pci=pcie_bus_peer2peer" is set or something like this.

Well we're using it without any special pci= flags.
pci=pcie_bus_peer2peer can reduce performance, so we should not require
it if it's not needed on all systems.

There are other issues that can prevent P2P access between some pairs of
devices. For example on Intel dual-socket boards the QPI link between
the sockets doesn't work for P2P traffic. So P2P only works between
devices connected to the same socket.

I think it's impractical to check all those chipset-specific limitations
at this level. Importing and mapping a foreign BO should be no problem
either way. If remote access is limited, that's something the
application can figure out on its own. In case of KFD, this is done
based on IO-link topology information.

Regards,
  Felix


>
> BTW: If you modify a patch as severely as that please add your
> Signed-of-by line as well.
>
> Regards,
> Christian.
>
>> +
>> +    ww_mutex_unlock(&bo->tbo.resv->lock);
>> +
>> +    return &gobj->base;
>> +}
>> +
>> +struct drm_gem_object *amdgpu_gem_prime_import(struct drm_device *dev,
>> +                           struct dma_buf *dma_buf)
>> +{
>> +    struct amdgpu_device *adev = dev->dev_private;
>> +
>> +    if (dma_buf->ops == &drm_gem_prime_dmabuf_ops) {
>> +        struct drm_gem_object *obj = dma_buf->priv;
>> +
>> +        if (obj->dev != dev && obj->dev->driver == dev->driver) {
>> +            /* It's a amdgpu_bo from a different driver instance */
>> +            struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
>> +
>> +            return amdgpu_gem_prime_foreign_bo(adev, bo);
>> +        }
>> +    }
>> +
>> +    return drm_gem_prime_import(dev, dma_buf);
>> +}
>
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-drm-amdgpu-enable-foreign-DMA-buf-objects.patch
Type: text/x-patch
Size: 3956 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20170704/dc6ba3b6/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-drm-amdgpu-handle-foreign-BOs-in-the-VM-mapping-code.patch
Type: text/x-patch
Size: 1896 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20170704/dc6ba3b6/attachment-0001.bin>