Hi
On Tue, Jul 30, 2013 at 9:52 AM, Sedat Dilek sedat.dilek@gmail.com wrote:
On Tue, Jul 30, 2013 at 9:41 AM, Sedat Dilek sedat.dilek@gmail.com wrote:
On Fri, Jul 26, 2013 at 10:15 PM, Daniel Vetter daniel@ffwll.ch wrote:
On Fri, Jul 26, 2013 at 12:09:32PM +0200, David Herrmann wrote:
The VMA manager is page-size based so drm_vma_node_size() returns the size in pages. However, drm_gem_mmap_obj() requires the size in bytes. Apply PAGE_SHIFT so we no longer get EINVAL during mmaps due to too small buffers.
This bug was introduced in commit: 0de23977cfeb5b357ec884ba15417ae118ff9e9b "drm/gem: convert to new unified vma manager"
Fixes i915 gtt mmap failure reported by Sedat Dilek in: Re: linux-next: Tree for Jul 25 [ call-trace: drm | drm-intel related? ]
Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Chris Wilson chris@chris-wilson.co.uk Signed-off-by: David Herrmann dh.herrmann@gmail.com Reported-by: Sedat Dilek sedat.dilek@gmail.com Tested-by: Sedat Dilek sedat.dilek@gmail.com
I remember that I even checked whether v4 was consistent with pages vs. bytes ;-) So
Hi David, Daniel, and Dave,
Just reading quickly "drm: add unified vma offset manager"... is that below in the docs?
"The VMA manager is page-size based so drm_vma_node_size() returns the size in pages. However, drm_gem_mmap_obj() requires the size in bytes. Apply PAGE_SHIFT so we no longer get EINVAL during mmaps due to too small buffers."
If not, do you mind adding it?
To quote this two:
[ include/drm/drm_vma_manager.h ]
/**
- drm_vma_node_size() - Return size (page-based)
- @node: Node to inspect
- Return the size as number of pages for the given node. This is the same size
- that was passed to drm_vma_offset_add(). If no offset is allocated for the
- node, this is 0.
- RETURNS:
- Size of @node as number of pages. 0 if the node does not have an offset
- allocated.
*/
[ drivers/gpu/drm/drm_gem.c ]
It's actually "drm_gem_mmap_obj" which we have to look at and it says: * @obj_size: the object size to be mapped, in bytes
So both are already documented.
Regards David
/**
- drm_gem_mmap - memory map routine for GEM objects
- @filp: DRM file pointer
- @vma: VMA for the area to be mapped
- If a driver supports GEM object mapping, mmap calls on the DRM file
- descriptor will end up here.
- Look up the GEM object based on the offset passed in (vma->vm_pgoff will
- contain the fake offset we created when the GTT map ioctl was called on
- the object) and map it with a call to drm_gem_mmap_obj().
*/
- Sedat -
Thanks in advance!
- Sedat -
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
on this little fixup. -Daniel
drivers/gpu/drm/drm_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 3613b50..1f76572 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -666,7 +666,7 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) }
obj = container_of(node, struct drm_gem_object, vma_node);
ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node), vma);
ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node) << PAGE_SHIFT, vma); mutex_unlock(&dev->struct_mutex);
-- 1.8.3.4
-- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch