On Wed, Jul 31, 2013 at 6:46 PM, David Herrmann <dh.herrmann@xxxxxxxxx> wrote: > 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. > Good. I had only a quick view, you are the expert. > 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
