Hi On Tue, Jul 30, 2013 at 9:52 AM, Sedat Dilek <sedat.dilek@xxxxxxxxx> wrote: > On Tue, Jul 30, 2013 at 9:41 AM, Sedat Dilek <sedat.dilek@xxxxxxxxx> wrote: >> On Fri, Jul 26, 2013 at 10:15 PM, Daniel Vetter <daniel@xxxxxxxx> 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@xxxxxxxx> >>>> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> >>>> Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxx> >>>> Reported-by: Sedat Dilek <sedat.dilek@xxxxxxxxx> >>>> Tested-by: Sedat Dilek <sedat.dilek@xxxxxxxxx> >>> >>> 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@xxxxxxxx> >>> >>> 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
