On Thu, Sep 28, 2017 at 10:23 AM, Liviu Dudau Liviu.Dudau@arm.com wrote:
Hi Daniel,
On Thu, Sep 28, 2017 at 09:40:35AM +0200, Daniel Vetter wrote:
On Wed, Sep 27, 2017 at 4:01 PM, Steven Price Steven.Price@arm.com wrote:
From: Tu Vuong tu.vuong@arm.com
When a CMA GEM object is exported via DRM PRIME it should be possible to mmap the object using an offset. However drm_gem_cma_mmap_obj always zeroed vm_pgoff.
No, at least no one ever had run into this. Pretty much all drivers work like this.
Can you be a bit more specific with "work like this"? You mean all drivers expect mmap-ed calles to have zero offset or the fact that they all zero vm_pgoff therefore ignore the offset?
Iirc some drivers even outright reject the mmap if it's not aligned to the full obj.
Fix this by moving the zeroing of vm_pgoff to drm_gem_cma_mmap (which is only used for non-PRIME mmap) and correct the size parameter in the call to dma_mmap_wc as the offset may not be non-zero.
Since this is an uabi change we need the corresponding patch to upstream open source userspace. Reviewed and all that and ready for acceptance. See
If ignoring the offset is considered uabi, then I agree. Otherwise it looks to me more like a fix, rather than an uabi change.
Maybe we should have first a patch warning when vm_pgoff is not zero so that we can find out how many in the userspace use that?
Not sure warning is a good idea, because it still means we get to audit everything. Might as well go ahead an reject it with -EINVAL, and make sure that's consistent across drivers. But that's all work, so what exactly is the use-case you have and want to do sub-mmapping for? Once we have that it's much easier to discuss what to do, instead of abstraction discussions without the full stack at hand.
Thanks, Daniel
Best regards, Liviu
https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace...
Signed-off-by: Tu Vuong tu.vuong@arm.com Signed-off-by: Steven Price steven.price@arm.com Reviewed-by: Brian Starkey brian.starkey@arm.com CC: Brian Starkey brian.starkey@arm.com CC: Liviu Dudau Liviu.Dudau@arm.com
Thanks, Daniel
drivers/gpu/drm/drm_gem_cma_helper.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index 373e33f22be4..25828b33c5be 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -276,15 +276,12 @@ static int drm_gem_cma_mmap_obj(struct drm_gem_cma_object *cma_obj, int ret;
/*
* Clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the* vm_pgoff (used as a fake buffer offset by DRM) to 0 as we want to map* the whole buffer.
* Clear the VM_PFNMAP flag that was set by drm_gem_mmap() */ vma->vm_flags &= ~VM_PFNMAP;
vma->vm_pgoff = 0; ret = dma_mmap_wc(cma_obj->base.dev->dev, vma, cma_obj->vaddr,cma_obj->paddr, vma->vm_end - vma->vm_start);
cma_obj->paddr, cma_obj->base.size); if (ret) drm_gem_vm_close(vma);@@ -322,6 +319,12 @@ int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma) gem_obj = vma->vm_private_data; cma_obj = to_drm_gem_cma_obj(gem_obj);
/** Set the vm_pgoff (used as a fake buffer offset by DRM) to 0 as we* want to map the whole buffer.*/vma->vm_pgoff = 0;return drm_gem_cma_mmap_obj(cma_obj, vma);} EXPORT_SYMBOL_GPL(drm_gem_cma_mmap); -- 2.11.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
| I would like to | | fix the world, | | but they're not | | giving me the | \ source code! /
¯\_(ツ)_/¯