For nouveau, radeon, and amdgpu attempting to scanout of a bo will cause the driver to migrate it to VRAM.
When the bo in question is an imported dma-buf, this results in a buffer that appears to be shared but is actually device-private.
Consensus on #dri-devel seemed to be that it would be appropriate for addfb to fail when attempted on an imported dma-buf.
It *might* also be appropriate to fail addfb on an exported dma-buf (as that is likewise pinned to GTT), but #dri-devel thought it possible the buffer could be unshared between addfb and actually trying to scanout. An imported dma-buf doesn't have the same problem.
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Any use of the framebuffer will migrate it to VRAM, which is not sensible for an imported dma-buf.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com CC: nouveau@lists.freedesktop.org --- drivers/gpu/drm/nouveau/nouveau_display.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index 72fdba1a1c5d..e8e6bc7b6d51 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -286,6 +286,10 @@ nouveau_user_framebuffer_create(struct drm_device *dev, return ERR_PTR(-ENOENT); nvbo = nouveau_gem_object(gem);
+ /* Handle is an imported dma-buf, so cannot be migrated to VRAM */ + if (gem->import_attach) + return ERR_PTR(-EINVAL); + ret = nouveau_framebuffer_new(dev, mode_cmd, nvbo, &fb); if (ret == 0) return &fb->base;
Op 29-03-17 om 02:27 schreef raof@ubuntu.com:
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Any use of the framebuffer will migrate it to VRAM, which is not sensible for an imported dma-buf.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com CC: nouveau@lists.freedesktop.org
drivers/gpu/drm/nouveau/nouveau_display.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index 72fdba1a1c5d..e8e6bc7b6d51 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -286,6 +286,10 @@ nouveau_user_framebuffer_create(struct drm_device *dev, return ERR_PTR(-ENOENT); nvbo = nouveau_gem_object(gem);
- /* Handle is an imported dma-buf, so cannot be migrated to VRAM */
- if (gem->import_attach)
return ERR_PTR(-EINVAL);- ret = nouveau_framebuffer_new(dev, mode_cmd, nvbo, &fb); if (ret == 0) return &fb->base;
I don't know if there's really a hard requirement for nouveau to scan out from GART on nv50+, but it might be a bigger problem on earlier platforms.
Acked-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Attempting to migrate the bo will break the sharing of the buffer.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com CC: nouveau@lists.freedesktop.org --- drivers/gpu/drm/nouveau/nouveau_prime.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_prime.c b/drivers/gpu/drm/nouveau/nouveau_prime.c index 1fefc93af1d7..5f474ebb4d6d 100644 --- a/drivers/gpu/drm/nouveau/nouveau_prime.c +++ b/drivers/gpu/drm/nouveau/nouveau_prime.c @@ -76,6 +76,8 @@ struct drm_gem_object *nouveau_gem_prime_import_sg_table(struct drm_device *dev, return ERR_PTR(ret);
nvbo->valid_domains = NOUVEAU_GEM_DOMAIN_GART; + /* pin imported buffer to GTT */ + nouveau_bo_pin(nvbo, TTM_PL_FLAG_TT, false);
/* Initialize the embedded gem-object. We return a single gem-reference * to the caller, instead of a normal nouveau_bo ttm reference. */
Op 29-03-17 om 02:27 schreef raof@ubuntu.com:
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Attempting to migrate the bo will break the sharing of the buffer.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com CC: nouveau@lists.freedesktop.org
drivers/gpu/drm/nouveau/nouveau_prime.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_prime.c b/drivers/gpu/drm/nouveau/nouveau_prime.c index 1fefc93af1d7..5f474ebb4d6d 100644 --- a/drivers/gpu/drm/nouveau/nouveau_prime.c +++ b/drivers/gpu/drm/nouveau/nouveau_prime.c @@ -76,6 +76,8 @@ struct drm_gem_object *nouveau_gem_prime_import_sg_table(struct drm_device *dev, return ERR_PTR(ret);
nvbo->valid_domains = NOUVEAU_GEM_DOMAIN_GART;
/* pin imported buffer to GTT */
nouveau_bo_pin(nvbo, TTM_PL_FLAG_TT, false);
/* Initialize the embedded gem-object. We return a single gem-reference
- to the caller, instead of a normal nouveau_bo ttm reference. */
Missing error handling here?
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Attempting to migrate the bo will break the sharing of the buffer.
v2: Error-check the nouveau_bo_pin call; it can fail.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com --- drivers/gpu/drm/nouveau/nouveau_prime.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_prime.c b/drivers/gpu/drm/nouveau/nouveau_prime.c index 1fefc93af1d7..ad5eaf70c8e2 100644 --- a/drivers/gpu/drm/nouveau/nouveau_prime.c +++ b/drivers/gpu/drm/nouveau/nouveau_prime.c @@ -76,6 +76,12 @@ struct drm_gem_object *nouveau_gem_prime_import_sg_table(struct drm_device *dev, return ERR_PTR(ret);
nvbo->valid_domains = NOUVEAU_GEM_DOMAIN_GART; + /* pin imported buffer to GTT */ + ret = nouveau_bo_pin(nvbo, TTM_PL_FLAG_TT, false); + if (ret) { + nouveau_bo_ref(NULL, &nvbo); + return ERR_PTR(ret); + }
/* Initialize the embedded gem-object. We return a single gem-reference * to the caller, instead of a normal nouveau_bo ttm reference. */
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Any use of the framebuffer will migrate it to VRAM, which is not sensible for an imported dma-buf.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com CC: amd-gfx@lists.freedesktop.org --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index 39fc388f222a..e7c3cc5b7d62 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -612,6 +612,12 @@ amdgpu_user_framebuffer_create(struct drm_device *dev, return ERR_PTR(-ENOENT); }
+ /* Handle is imported dma-buf, so cannot be migrated to VRAM for scanout */ + if (obj->import_attach) { + dev_err(&dev->pdev->dev, "Cannot create framebuffer from imported dma_buf\n"); + return ERR_PTR(-EINVAL); + } + amdgpu_fb = kzalloc(sizeof(*amdgpu_fb), GFP_KERNEL); if (amdgpu_fb == NULL) { drm_gem_object_unreference_unlocked(obj);
On 29/03/17 09:27 AM, raof@ubuntu.com wrote:
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Any use of the framebuffer will migrate it to VRAM, which is not sensible for an imported dma-buf.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com CC: amd-gfx@lists.freedesktop.org
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index 39fc388f222a..e7c3cc5b7d62 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -612,6 +612,12 @@ amdgpu_user_framebuffer_create(struct drm_device *dev, return ERR_PTR(-ENOENT); }
- /* Handle is imported dma-buf, so cannot be migrated to VRAM for scanout */
Newer APUs support scanout from GTT, though they still impose some restrictions which will probably not be satisfied in general by BOs imported from other drivers. So this is probably okay for now.
- if (obj->import_attach) {
dev_err(&dev->pdev->dev, "Cannot create framebuffer from imported dma_buf\n");
This should probably be something like DRM_DEBUG, so userspace can't spam dmesg by default. Same for patch 5.
return ERR_PTR(-EINVAL);- }
- amdgpu_fb = kzalloc(sizeof(*amdgpu_fb), GFP_KERNEL); if (amdgpu_fb == NULL) { drm_gem_object_unreference_unlocked(obj);
On Wed, 29 Mar 2017 at 13:04 Michel Dänzer michel@daenzer.net wrote:
On 29/03/17 09:27 AM, raof@ubuntu.com wrote:
From: Christopher James Halse Rogers <
christopher.halse.rogers@canonical.com>
Any use of the framebuffer will migrate it to VRAM, which is not
sensible for
an imported dma-buf.
Signed-off-by: Christopher James Halse Rogers <
christopher.halse.rogers@canonical.com>
CC: amd-gfx@lists.freedesktop.org
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 39fc388f222a..e7c3cc5b7d62 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -612,6 +612,12 @@ amdgpu_user_framebuffer_create(struct drm_device
*dev,
return ERR_PTR(-ENOENT); }
/* Handle is imported dma-buf, so cannot be migrated to VRAM forscanout */
Newer APUs support scanout from GTT, though they still impose some restrictions which will probably not be satisfied in general by BOs imported from other drivers. So this is probably okay for now.
As far as I can tell amdgpu unconditionally migrates to VRAM when trying to scanout at the moment. When that changes, so can this check :).
if (obj->import_attach) {dev_err(&dev->pdev->dev, "Cannot create framebuffer fromimported dma_buf\n");
This should probably be something like DRM_DEBUG, so userspace can't spam dmesg by default. Same for patch 5.
Ta. v2 incoming.
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Any use of the framebuffer will migrate it to VRAM, which is not sensible for an imported dma-buf.
v2: Use DRM_DEBUG_KMS to prevent userspace accidentally spamming dmesg.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com CC: amd-gfx@lists.freedesktop.org --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index 39fc388f222a..2f2e9b5c8a6a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -612,6 +612,12 @@ amdgpu_user_framebuffer_create(struct drm_device *dev, return ERR_PTR(-ENOENT); }
+ /* Handle is imported dma-buf, so cannot be migrated to VRAM for scanout */ + if (obj->import_attach) { + DRM_DEBUG_KMS("Cannot create framebuffer from imported dma_buf\n"); + return ERR_PTR(-EINVAL); + } + amdgpu_fb = kzalloc(sizeof(*amdgpu_fb), GFP_KERNEL); if (amdgpu_fb == NULL) { drm_gem_object_unreference_unlocked(obj);
On 29/03/17 01:02 PM, raof@ubuntu.com wrote:
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Any use of the framebuffer will migrate it to VRAM, which is not sensible for an imported dma-buf.
v2: Use DRM_DEBUG_KMS to prevent userspace accidentally spamming dmesg.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com CC: amd-gfx@lists.freedesktop.org
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index 39fc388f222a..2f2e9b5c8a6a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -612,6 +612,12 @@ amdgpu_user_framebuffer_create(struct drm_device *dev, return ERR_PTR(-ENOENT); }
- /* Handle is imported dma-buf, so cannot be migrated to VRAM for scanout */
- if (obj->import_attach) {
DRM_DEBUG_KMS("Cannot create framebuffer from imported dma_buf\n");return ERR_PTR(-EINVAL);- }
- amdgpu_fb = kzalloc(sizeof(*amdgpu_fb), GFP_KERNEL); if (amdgpu_fb == NULL) { drm_gem_object_unreference_unlocked(obj);
This patch and v2 of patch 5 are
Reviewed-by: Michel Dänzer michel.daenzer@amd.com
On Wed, Mar 29, 2017 at 2:07 AM, Michel Dänzer michel@daenzer.net wrote:
On 29/03/17 01:02 PM, raof@ubuntu.com wrote:
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Any use of the framebuffer will migrate it to VRAM, which is not sensible for an imported dma-buf.
v2: Use DRM_DEBUG_KMS to prevent userspace accidentally spamming dmesg.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com CC: amd-gfx@lists.freedesktop.org
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index 39fc388f222a..2f2e9b5c8a6a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -612,6 +612,12 @@ amdgpu_user_framebuffer_create(struct drm_device *dev, return ERR_PTR(-ENOENT); }
/* Handle is imported dma-buf, so cannot be migrated to VRAM for scanout */if (obj->import_attach) {DRM_DEBUG_KMS("Cannot create framebuffer from imported dma_buf\n");return ERR_PTR(-EINVAL);}amdgpu_fb = kzalloc(sizeof(*amdgpu_fb), GFP_KERNEL); if (amdgpu_fb == NULL) { drm_gem_object_unreference_unlocked(obj);This patch and v2 of patch 5 are
Reviewed-by: Michel Dänzer michel.daenzer@amd.com
Applied these two. Thanks!
Alex
-- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Attempting to migrate the bo will break the sharing of the buffer.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com CC: amd-gfx@lists.freedesktop.org --- drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c index 3826d5aea0a6..3c84ec5c6ac8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c @@ -74,6 +74,17 @@ amdgpu_gem_prime_import_sg_table(struct drm_device *dev, if (ret) return ERR_PTR(ret);
+ /* Imported bo must be pinned to GTT, as moving it breaks sharing */ + ret = amdgpu_bo_reserve(bo, false); + if (ret) + return ERR_PTR(ret); + + ret = amdgpu_bo_pin(bo, AMDGPU_GEM_DOMAIN_GTT, NULL); + if (ret) + return ERR_PTR(ret); + + amdgpu_bo_unreserve(bo); + bo->prime_shared_count = 1; return &bo->gem_base; }
On 29/03/17 09:27 AM, raof@ubuntu.com wrote:
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Attempting to migrate the bo will break the sharing of the buffer.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com CC: amd-gfx@lists.freedesktop.org
drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c index 3826d5aea0a6..3c84ec5c6ac8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c @@ -74,6 +74,17 @@ amdgpu_gem_prime_import_sg_table(struct drm_device *dev, if (ret) return ERR_PTR(ret);
- /* Imported bo must be pinned to GTT, as moving it breaks sharing */
- ret = amdgpu_bo_reserve(bo, false);
- if (ret)
return ERR_PTR(ret);- ret = amdgpu_bo_pin(bo, AMDGPU_GEM_DOMAIN_GTT, NULL);
- if (ret)
return ERR_PTR(ret);- amdgpu_bo_unreserve(bo);
- bo->prime_shared_count = 1; return &bo->gem_base;
}
Thanks for beating me to this! :) This patch and patch 6 are
Reviewed-by: Michel Dänzer michel.daenzer@amd.com
Am 29.03.2017 um 02:27 schrieb raof@ubuntu.com:
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Attempting to migrate the bo will break the sharing of the buffer.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com CC: amd-gfx@lists.freedesktop.org
NAK on this one and the radeon version.
We can't migrate the buffers to VRAM, but we shouldn't pin it either cause that will lock down the GTT space used for it.
Instead you should modify amdgpu_bo_pin() and fail if anybody tries to migrate a BO which has prime_shared_count != 0 to VRAM (you migth need to add this for radeon).
Regards, Christian.
drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c index 3826d5aea0a6..3c84ec5c6ac8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c @@ -74,6 +74,17 @@ amdgpu_gem_prime_import_sg_table(struct drm_device *dev, if (ret) return ERR_PTR(ret);
- /* Imported bo must be pinned to GTT, as moving it breaks sharing */
- ret = amdgpu_bo_reserve(bo, false);
- if (ret)
return ERR_PTR(ret);- ret = amdgpu_bo_pin(bo, AMDGPU_GEM_DOMAIN_GTT, NULL);
- if (ret)
return ERR_PTR(ret);- amdgpu_bo_unreserve(bo);
- bo->prime_shared_count = 1; return &bo->gem_base; }
On 29/03/17 04:14 PM, Christian König wrote:
Am 29.03.2017 um 02:27 schrieb raof@ubuntu.com:
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Attempting to migrate the bo will break the sharing of the buffer.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com CC: amd-gfx@lists.freedesktop.org
NAK on this one and the radeon version.
We can't migrate the buffers to VRAM, but we shouldn't pin it either cause that will lock down the GTT space used for it.
Ah, good point, didn't think of that.
Instead you should modify amdgpu_bo_pin() and fail if anybody tries to migrate a BO which has prime_shared_count != 0 to VRAM (you migth need to add this for radeon).
This would also need to be checked in the AMDGPU_GEM_OP_SET_PLACEMENT case of amdgpu_gem_op_ioctl, otherwise userspace can change prefered_domains to AMDGPU_GEM_DOMAIN_VRAM, and using the BO for GPU operations may move it to VRAM.
Or maybe there's a central place which can catch both of these cases (and any others).
Am 29.03.2017 um 11:07 schrieb Michel Dänzer:
On 29/03/17 04:14 PM, Christian König wrote:
Am 29.03.2017 um 02:27 schrieb raof@ubuntu.com:
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Attempting to migrate the bo will break the sharing of the buffer.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com CC: amd-gfx@lists.freedesktop.org
NAK on this one and the radeon version.
We can't migrate the buffers to VRAM, but we shouldn't pin it either cause that will lock down the GTT space used for it.
Ah, good point, didn't think of that.
Instead you should modify amdgpu_bo_pin() and fail if anybody tries to migrate a BO which has prime_shared_count != 0 to VRAM (you migth need to add this for radeon).
This would also need to be checked in the AMDGPU_GEM_OP_SET_PLACEMENT case of amdgpu_gem_op_ioctl, otherwise userspace can change prefered_domains to AMDGPU_GEM_DOMAIN_VRAM, and using the BO for GPU operations may move it to VRAM.
Good point as well, that indeed also needs to be forbidden.
Or maybe there's a central place which can catch both of these cases (and any others).
At least of hand I can't think of any. But catching those two cases should be sufficient as far as I can see.
Christian.
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Migration to VRAM will break the sharing, resulting in rendering on the exporting GPU never becoming visible on the importing GPU.
v2: Don't pin BOs to GTT. Instead, refuse to migrate them out of GTT.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 5 +++++ drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 4 ++++ 2 files changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index 106cf83c2e6b..bc5356d0a13d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -679,6 +679,11 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data, break; } case AMDGPU_GEM_OP_SET_PLACEMENT: + if (robj->prime_shared_count && (args->value & AMDGPU_GEM_DOMAIN_VRAM)) { + r = -EINVAL; + amdgpu_bo_unreserve(robj); + break; + } if (amdgpu_ttm_tt_get_usermm(robj->tbo.ttm)) { r = -EPERM; amdgpu_bo_unreserve(robj); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index be80a4a68d7b..323580740a74 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -665,6 +665,10 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain, if (WARN_ON_ONCE(min_offset > max_offset)) return -EINVAL;
+ /* A shared bo cannot be migrated to VRAM */ + if (bo->prime_shared_count && (domain == AMDGPU_GEM_DOMAIN_VRAM)) + return -EINVAL; + if (bo->pin_count) { uint32_t mem_type = bo->tbo.mem.mem_type;
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Any use of the framebuffer will migrate it to VRAM, which is not sensible for an imported dma-buf.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com CC: amd-gfx@lists.freedesktop.org --- drivers/gpu/drm/radeon/radeon_display.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index aea8b62835a4..289c9cbbe7d5 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -1352,6 +1352,12 @@ radeon_user_framebuffer_create(struct drm_device *dev, return ERR_PTR(-ENOENT); }
+ /* Handle is imported dma-buf, so cannot be migrated to VRAM for scanout */ + if (obj->import_attach) { + dev_err(&dev->pdev->dev, "Cannot create framebuffer from imported dma_buf\n"); + return ERR_PTR(-EINVAL); + } + radeon_fb = kzalloc(sizeof(*radeon_fb), GFP_KERNEL); if (radeon_fb == NULL) { drm_gem_object_unreference_unlocked(obj);
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Any use of the framebuffer will migrate it to VRAM, which is not sensible for an imported dma-buf.
v2: Use DRM_DEBUG_KMS to prevent userspace accidentally spamming dmesg.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com CC: amd-gfx@lists.freedesktop.org --- drivers/gpu/drm/radeon/radeon_display.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index aea8b62835a4..f85e9c1eaa47 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -1352,6 +1352,12 @@ radeon_user_framebuffer_create(struct drm_device *dev, return ERR_PTR(-ENOENT); }
+ /* Handle is imported dma-buf, so cannot be migrated to VRAM for scanout */ + if (obj->import_attach) { + DRM_DEBUG_KMS("Cannot create framebuffer from imported dma_buf\n"); + return ERR_PTR(-EINVAL); + } + radeon_fb = kzalloc(sizeof(*radeon_fb), GFP_KERNEL); if (radeon_fb == NULL) { drm_gem_object_unreference_unlocked(obj);
Am 29.03.2017 um 06:00 schrieb raof@ubuntu.com:
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Any use of the framebuffer will migrate it to VRAM, which is not sensible for an imported dma-buf.
v2: Use DRM_DEBUG_KMS to prevent userspace accidentally spamming dmesg.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com CC: amd-gfx@lists.freedesktop.org
For this one and the amdgpu variant: Reviewed-by: Christian König christian.koenig@amd.com.
drivers/gpu/drm/radeon/radeon_display.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index aea8b62835a4..f85e9c1eaa47 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -1352,6 +1352,12 @@ radeon_user_framebuffer_create(struct drm_device *dev, return ERR_PTR(-ENOENT); }
- /* Handle is imported dma-buf, so cannot be migrated to VRAM for scanout */
- if (obj->import_attach) {
DRM_DEBUG_KMS("Cannot create framebuffer from imported dma_buf\n");return ERR_PTR(-EINVAL);- }
- radeon_fb = kzalloc(sizeof(*radeon_fb), GFP_KERNEL); if (radeon_fb == NULL) { drm_gem_object_unreference_unlocked(obj);
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Attempting to migrate the bo will break the sharing of the buffer.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com CC: amd-gfx@lists.freedesktop.org --- drivers/gpu/drm/radeon/radeon_prime.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_prime.c b/drivers/gpu/drm/radeon/radeon_prime.c index f3609c97496b..2dbbb4bd47b1 100644 --- a/drivers/gpu/drm/radeon/radeon_prime.c +++ b/drivers/gpu/drm/radeon/radeon_prime.c @@ -73,6 +73,16 @@ struct drm_gem_object *radeon_gem_prime_import_sg_table(struct drm_device *dev, if (ret) return ERR_PTR(ret);
+ ret = radeon_bo_reserve(bo, false); + if (ret) + return ERR_PTR(ret); + + ret = radeon_bo_pin(bo, RADEON_GEM_DOMAIN_GTT, NULL); + radeon_bo_unreserve(bo); + + if (ret) + return ERR_PTR(ret); + mutex_lock(&rdev->gem.mutex); list_add_tail(&bo->list, &rdev->gem.objects); mutex_unlock(&rdev->gem.mutex);
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com --- drivers/gpu/drm/radeon/radeon.h | 1 + drivers/gpu/drm/radeon/radeon_prime.c | 6 ++++++ 2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 44e0c5ed6418..1094325c1877 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -499,6 +499,7 @@ struct radeon_bo { u32 tiling_flags; u32 pitch; int surface_reg; + unsigned prime_shared_count; /* list of all virtual address to which this bo * is associated to */ diff --git a/drivers/gpu/drm/radeon/radeon_prime.c b/drivers/gpu/drm/radeon/radeon_prime.c index f3609c97496b..7110d403322c 100644 --- a/drivers/gpu/drm/radeon/radeon_prime.c +++ b/drivers/gpu/drm/radeon/radeon_prime.c @@ -77,6 +77,7 @@ struct drm_gem_object *radeon_gem_prime_import_sg_table(struct drm_device *dev, list_add_tail(&bo->list, &rdev->gem.objects); mutex_unlock(&rdev->gem.mutex);
+ bo->prime_shared_count = 1; return &bo->gem_base; }
@@ -91,6 +92,9 @@ int radeon_gem_prime_pin(struct drm_gem_object *obj)
/* pin buffer into GTT */ ret = radeon_bo_pin(bo, RADEON_GEM_DOMAIN_GTT, NULL); + if (likely(ret == 0)) + bo->prime_shared_count++; + radeon_bo_unreserve(bo); return ret; } @@ -105,6 +109,8 @@ void radeon_gem_prime_unpin(struct drm_gem_object *obj) return;
radeon_bo_unpin(bo); + if (bo->prime_shared_count) + bo->prime_shared_count--; radeon_bo_unreserve(bo); }
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
BOs shared via dma-buf, either imported or exported, cannot sensibly be migrated to VRAM without breaking the dma-buf sharing. Refuse userspace requests to migrate to VRAM, ensure such BOs are not migrated during command submission, and refuse to pin them to VRAM.
v2: Don't pin BOs in GTT. Instead, refuse to migrate BOs to VRAM.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com --- drivers/gpu/drm/radeon/radeon_cs.c | 10 ++++++++++ drivers/gpu/drm/radeon/radeon_gem.c | 4 ++++ drivers/gpu/drm/radeon/radeon_object.c | 5 +++++ 3 files changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c index a8442f7196d6..df6b58c08544 100644 --- a/drivers/gpu/drm/radeon/radeon_cs.c +++ b/drivers/gpu/drm/radeon/radeon_cs.c @@ -164,6 +164,16 @@ static int radeon_cs_parser_relocs(struct radeon_cs_parser *p) p->relocs[i].allowed_domains = domain; }
+ /* Objects shared as dma-bufs cannot be moved to VRAM */ + if (p->relocs[i].robj->prime_shared_count) { + p->relocs[i].allowed_domains &= ~RADEON_GEM_DOMAIN_VRAM; + if (!p->relocs[i].allowed_domains) { + DRM_ERROR("BO associated with dma-buf cannot " + "be moved to VRAM\n"); + return -EINVAL; + } + } + p->relocs[i].tv.bo = &p->relocs[i].robj->tbo; p->relocs[i].tv.shared = !r->write_domain;
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c index 96683f5b2b1b..067ea7d7658a 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -120,6 +120,10 @@ static int radeon_gem_set_domain(struct drm_gem_object *gobj, return r; } } + if (domain == RADEON_GEM_DOMAIN_VRAM && robj->prime_shared_count) { + /* A BO that is associated with a dma-buf cannot be sensibly migrated to VRAM */ + return -EINVAL; + } return 0; }
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 74b276060c20..bec2ec056de4 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -352,6 +352,11 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset,
return 0; } + if (bo->prime_shared_count && domain == RADEON_GEM_DOMAIN_VRAM) { + /* A BO shared as a dma-buf cannot be sensibly migrated to VRAM */ + return -EINVAL; + } + radeon_ttm_placement_from_domain(bo, domain); for (i = 0; i < bo->placement.num_placement; i++) { /* force to pin into visible video ram */
Am 03.04.2017 um 05:35 schrieb raof@ubuntu.com:
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
BOs shared via dma-buf, either imported or exported, cannot sensibly be migrated to VRAM without breaking the dma-buf sharing. Refuse userspace requests to migrate to VRAM, ensure such BOs are not migrated during command submission, and refuse to pin them to VRAM.
v2: Don't pin BOs in GTT. Instead, refuse to migrate BOs to VRAM.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Patches #4, #6 and this one (#7) are Reviewed-by: Christian König christian.koenig@amd.com.
drivers/gpu/drm/radeon/radeon_cs.c | 10 ++++++++++ drivers/gpu/drm/radeon/radeon_gem.c | 4 ++++ drivers/gpu/drm/radeon/radeon_object.c | 5 +++++ 3 files changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c index a8442f7196d6..df6b58c08544 100644 --- a/drivers/gpu/drm/radeon/radeon_cs.c +++ b/drivers/gpu/drm/radeon/radeon_cs.c @@ -164,6 +164,16 @@ static int radeon_cs_parser_relocs(struct radeon_cs_parser *p) p->relocs[i].allowed_domains = domain; }
/* Objects shared as dma-bufs cannot be moved to VRAM */if (p->relocs[i].robj->prime_shared_count) {p->relocs[i].allowed_domains &= ~RADEON_GEM_DOMAIN_VRAM;if (!p->relocs[i].allowed_domains) {DRM_ERROR("BO associated with dma-buf cannot ""be moved to VRAM\n");return -EINVAL;}}- p->relocs[i].tv.bo = &p->relocs[i].robj->tbo; p->relocs[i].tv.shared = !r->write_domain;
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c index 96683f5b2b1b..067ea7d7658a 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -120,6 +120,10 @@ static int radeon_gem_set_domain(struct drm_gem_object *gobj, return r; } }
- if (domain == RADEON_GEM_DOMAIN_VRAM && robj->prime_shared_count) {
/* A BO that is associated with a dma-buf cannot be sensibly migrated to VRAM */return -EINVAL;- } return 0; }
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 74b276060c20..bec2ec056de4 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -352,6 +352,11 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset,
return 0;}
- if (bo->prime_shared_count && domain == RADEON_GEM_DOMAIN_VRAM) {
/* A BO shared as a dma-buf cannot be sensibly migrated to VRAM */return -EINVAL;- }
- radeon_ttm_placement_from_domain(bo, domain); for (i = 0; i < bo->placement.num_placement; i++) { /* force to pin into visible video ram */
On Mon, Apr 3, 2017 at 4:24 AM, Christian König deathsimple@vodafone.de wrote:
Am 03.04.2017 um 05:35 schrieb raof@ubuntu.com:
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
BOs shared via dma-buf, either imported or exported, cannot sensibly be migrated to VRAM without breaking the dma-buf sharing. Refuse userspace requests to migrate to VRAM, ensure such BOs are not migrated during command submission, and refuse to pin them to VRAM.
v2: Don't pin BOs in GTT. Instead, refuse to migrate BOs to VRAM.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Patches #4, #6 and this one (#7) are Reviewed-by: Christian König christian.koenig@amd.com.
Will this cause any issues with self sharing? E.g., the standard use case with a single GPU and DRI3?
Alex
drivers/gpu/drm/radeon/radeon_cs.c | 10 ++++++++++ drivers/gpu/drm/radeon/radeon_gem.c | 4 ++++ drivers/gpu/drm/radeon/radeon_object.c | 5 +++++ 3 files changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c index a8442f7196d6..df6b58c08544 100644 --- a/drivers/gpu/drm/radeon/radeon_cs.c +++ b/drivers/gpu/drm/radeon/radeon_cs.c @@ -164,6 +164,16 @@ static int radeon_cs_parser_relocs(struct radeon_cs_parser *p) p->relocs[i].allowed_domains = domain; }
/* Objects shared as dma-bufs cannot be moved to VRAM */if (p->relocs[i].robj->prime_shared_count) {p->relocs[i].allowed_domains &=~RADEON_GEM_DOMAIN_VRAM;
if (!p->relocs[i].allowed_domains) {DRM_ERROR("BO associated with dma-bufcannot "
"be moved to VRAM\n");return -EINVAL;}}p->relocs[i].tv.bo = &p->relocs[i].robj->tbo; p->relocs[i].tv.shared = !r->write_domain;b/drivers/gpu/drm/radeon/radeon_gem.c index 96683f5b2b1b..067ea7d7658a 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -120,6 +120,10 @@ static int radeon_gem_set_domain(struct drm_gem_object *gobj, return r; } }
if (domain == RADEON_GEM_DOMAIN_VRAM && robj->prime_shared_count){
/* A BO that is associated with a dma-buf cannot besensibly migrated to VRAM */
return -EINVAL;} return 0;b/drivers/gpu/drm/radeon/radeon_object.c index 74b276060c20..bec2ec056de4 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -352,6 +352,11 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset, return 0; }
if (bo->prime_shared_count && domain == RADEON_GEM_DOMAIN_VRAM) {/* A BO shared as a dma-buf cannot be sensibly migrated toVRAM */
return -EINVAL;}radeon_ttm_placement_from_domain(bo, domain); for (i = 0; i < bo->placement.num_placement; i++) { /* force to pin into visible video ram */
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Am 03.04.2017 um 17:51 schrieb Alex Deucher:
On Mon, Apr 3, 2017 at 4:24 AM, Christian König deathsimple@vodafone.de wrote:
Am 03.04.2017 um 05:35 schrieb raof@ubuntu.com:
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
BOs shared via dma-buf, either imported or exported, cannot sensibly be migrated to VRAM without breaking the dma-buf sharing. Refuse userspace requests to migrate to VRAM, ensure such BOs are not migrated during command submission, and refuse to pin them to VRAM.
v2: Don't pin BOs in GTT. Instead, refuse to migrate BOs to VRAM.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Patches #4, #6 and this one (#7) are Reviewed-by: Christian König christian.koenig@amd.com.
Will this cause any issues with self sharing? E.g., the standard use case with a single GPU and DRI3?
No, the code from amdgpu to count the number of times we really shared a BO with somebody was ported over to radeon as far as I can see.
So that should work fine, but I haven't double checked or tested it.
Christian.
Alex
drivers/gpu/drm/radeon/radeon_cs.c | 10 ++++++++++ drivers/gpu/drm/radeon/radeon_gem.c | 4 ++++ drivers/gpu/drm/radeon/radeon_object.c | 5 +++++ 3 files changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c index a8442f7196d6..df6b58c08544 100644 --- a/drivers/gpu/drm/radeon/radeon_cs.c +++ b/drivers/gpu/drm/radeon/radeon_cs.c @@ -164,6 +164,16 @@ static int radeon_cs_parser_relocs(struct radeon_cs_parser *p) p->relocs[i].allowed_domains = domain; }
/* Objects shared as dma-bufs cannot be moved to VRAM */if (p->relocs[i].robj->prime_shared_count) {p->relocs[i].allowed_domains &=~RADEON_GEM_DOMAIN_VRAM;
if (!p->relocs[i].allowed_domains) {DRM_ERROR("BO associated with dma-bufcannot "
"be moved to VRAM\n");return -EINVAL;}}p->relocs[i].tv.bo = &p->relocs[i].robj->tbo; p->relocs[i].tv.shared = !r->write_domain;b/drivers/gpu/drm/radeon/radeon_gem.c index 96683f5b2b1b..067ea7d7658a 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -120,6 +120,10 @@ static int radeon_gem_set_domain(struct drm_gem_object *gobj, return r; } }
if (domain == RADEON_GEM_DOMAIN_VRAM && robj->prime_shared_count){
/* A BO that is associated with a dma-buf cannot besensibly migrated to VRAM */
return -EINVAL;} return 0;b/drivers/gpu/drm/radeon/radeon_object.c index 74b276060c20..bec2ec056de4 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -352,6 +352,11 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset, return 0; }
if (bo->prime_shared_count && domain == RADEON_GEM_DOMAIN_VRAM) {/* A BO shared as a dma-buf cannot be sensibly migrated toVRAM */
return -EINVAL;}radeon_ttm_placement_from_domain(bo, domain); for (i = 0; i < bo->placement.num_placement; i++) { /* force to pin into visible video ram */
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
I also haven't tested it on hardware (all my hardware seems to be supported by amdgpu), but the only times we touch prime_shared_count are: 1) It's set to 1 in radeon_gem_prime_import_sg_table. This shouldn't be called in the self-import case (and isn't, as far as I can tell), and 2) It's incremented in radeon_gem_prime_pin, right after the bo is pinned to GTT, so cannot be migrated to VRAM anyway (unless there's an unmatched radeon_bo_unpin call somewhere else?).
On Tue, 4 Apr 2017 at 02:30 Christian König deathsimple@vodafone.de wrote:
Am 03.04.2017 um 17:51 schrieb Alex Deucher:
On Mon, Apr 3, 2017 at 4:24 AM, Christian König deathsimple@vodafone.de
wrote:
Am 03.04.2017 um 05:35 schrieb raof@ubuntu.com:
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
BOs shared via dma-buf, either imported or exported, cannot sensibly be migrated to VRAM without breaking the dma-buf sharing. Refuse userspace requests to
migrate
to VRAM, ensure such BOs are not migrated during command submission, and refuse
to
pin them to VRAM.
v2: Don't pin BOs in GTT. Instead, refuse to migrate BOs to VRAM.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Patches #4, #6 and this one (#7) are Reviewed-by: Christian König christian.koenig@amd.com.
Will this cause any issues with self sharing? E.g., the standard use case with a single GPU and DRI3?
No, the code from amdgpu to count the number of times we really shared a BO with somebody was ported over to radeon as far as I can see.
So that should work fine, but I haven't double checked or tested it.
Christian.
Alex
drivers/gpu/drm/radeon/radeon_cs.c | 10 ++++++++++ drivers/gpu/drm/radeon/radeon_gem.c | 4 ++++ drivers/gpu/drm/radeon/radeon_object.c | 5 +++++ 3 files changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c index a8442f7196d6..df6b58c08544 100644 --- a/drivers/gpu/drm/radeon/radeon_cs.c +++ b/drivers/gpu/drm/radeon/radeon_cs.c @@ -164,6 +164,16 @@ static int radeon_cs_parser_relocs(struct radeon_cs_parser *p) p->relocs[i].allowed_domains = domain; }
/* Objects shared as dma-bufs cannot be moved to VRAM*/
if (p->relocs[i].robj->prime_shared_count) {p->relocs[i].allowed_domains &=~RADEON_GEM_DOMAIN_VRAM;
if (!p->relocs[i].allowed_domains) {DRM_ERROR("BO associated with dma-bufcannot "
"be moved to VRAM\n");return -EINVAL;}}p->relocs[i].tv.bo = &p->relocs[i].robj->tbo; p->relocs[i].tv.shared = !r->write_domain;b/drivers/gpu/drm/radeon/radeon_gem.c index 96683f5b2b1b..067ea7d7658a 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -120,6 +120,10 @@ static int radeon_gem_set_domain(struct drm_gem_object *gobj, return r; } }
if (domain == RADEON_GEM_DOMAIN_VRAM &&robj->prime_shared_count)
{
/* A BO that is associated with a dma-buf cannot besensibly migrated to VRAM */
return -EINVAL;} return 0;b/drivers/gpu/drm/radeon/radeon_object.c index 74b276060c20..bec2ec056de4 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -352,6 +352,11 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset, return 0; }
if (bo->prime_shared_count && domain ==RADEON_GEM_DOMAIN_VRAM) {
/* A BO shared as a dma-buf cannot be sensiblymigrated to
VRAM */
return -EINVAL;}radeon_ttm_placement_from_domain(bo, domain); for (i = 0; i < bo->placement.num_placement; i++) { /* force to pin into visible video ram */
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On Mon, Apr 3, 2017 at 4:24 AM, Christian König deathsimple@vodafone.de wrote:
Am 03.04.2017 um 05:35 schrieb raof@ubuntu.com:
From: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
BOs shared via dma-buf, either imported or exported, cannot sensibly be migrated to VRAM without breaking the dma-buf sharing. Refuse userspace requests to migrate to VRAM, ensure such BOs are not migrated during command submission, and refuse to pin them to VRAM.
v2: Don't pin BOs in GTT. Instead, refuse to migrate BOs to VRAM.
Signed-off-by: Christopher James Halse Rogers christopher.halse.rogers@canonical.com
Patches #4, #6 and this one (#7) are Reviewed-by: Christian König christian.koenig@amd.com.
Applied 4, 6, 7.
Thanks,
Alex
drivers/gpu/drm/radeon/radeon_cs.c | 10 ++++++++++ drivers/gpu/drm/radeon/radeon_gem.c | 4 ++++ drivers/gpu/drm/radeon/radeon_object.c | 5 +++++ 3 files changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c index a8442f7196d6..df6b58c08544 100644 --- a/drivers/gpu/drm/radeon/radeon_cs.c +++ b/drivers/gpu/drm/radeon/radeon_cs.c @@ -164,6 +164,16 @@ static int radeon_cs_parser_relocs(struct radeon_cs_parser *p) p->relocs[i].allowed_domains = domain; }
/* Objects shared as dma-bufs cannot be moved to VRAM */if (p->relocs[i].robj->prime_shared_count) {p->relocs[i].allowed_domains &=~RADEON_GEM_DOMAIN_VRAM;
if (!p->relocs[i].allowed_domains) {DRM_ERROR("BO associated with dma-bufcannot "
"be moved to VRAM\n");return -EINVAL;}}p->relocs[i].tv.bo = &p->relocs[i].robj->tbo; p->relocs[i].tv.shared = !r->write_domain;b/drivers/gpu/drm/radeon/radeon_gem.c index 96683f5b2b1b..067ea7d7658a 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -120,6 +120,10 @@ static int radeon_gem_set_domain(struct drm_gem_object *gobj, return r; } }
if (domain == RADEON_GEM_DOMAIN_VRAM && robj->prime_shared_count){
/* A BO that is associated with a dma-buf cannot besensibly migrated to VRAM */
return -EINVAL;} return 0;b/drivers/gpu/drm/radeon/radeon_object.c index 74b276060c20..bec2ec056de4 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -352,6 +352,11 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset, return 0; }
if (bo->prime_shared_count && domain == RADEON_GEM_DOMAIN_VRAM) {/* A BO shared as a dma-buf cannot be sensibly migrated toVRAM */
return -EINVAL;}radeon_ttm_placement_from_domain(bo, domain); for (i = 0; i < bo->placement.num_placement; i++) { /* force to pin into visible video ram */
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Christian König -
Christopher James Halse Rogers -
Maarten Lankhorst -
Michel Dänzer -
raof＠ubuntu.com