| Message ID | 1480511958-10266-1-git-send-email-benjamin.gaignard@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Benjamin, Thank you for the patch. On Wednesday 30 Nov 2016 14:19:18 Benjamin Gaignard wrote: > Some SoC without MMU have display driver where a drm/kms driver > could be implemented. > Before doing such kind of thing drm/kms must allow to use mmuless > devices. > This patch propose to remove MMU configuration flag and add some > cma helpers functions to help implementing mmuless display driver > > version 2: > - define get_fb_unmapped_area() in drivers/video/fbdev/core/fbmem.c > if HAVE_ARCH_FB_UNMAPPED_AREA is set and get_fb_unmapped_area not > already defined in the architecture > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> > --- > drivers/gpu/drm/Kconfig | 4 +-- > drivers/gpu/drm/drm_fb_cma_helper.c | 4 +++ > drivers/gpu/drm/drm_gem_cma_helper.c | 62 +++++++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_vm.c | 4 +++ > drivers/video/fbdev/core/fbmem.c | 15 +++++++++ > include/drm/drm_gem_cma_helper.h | 6 ++++ > 6 files changed, 93 insertions(+), 2 deletions(-) [snip] > diff --git a/drivers/video/fbdev/core/fbmem.c > b/drivers/video/fbdev/core/fbmem.c index 76c1ad9..54488ee 100644 > --- a/drivers/video/fbdev/core/fbmem.c > +++ b/drivers/video/fbdev/core/fbmem.c > @@ -1492,6 +1492,21 @@ static long fb_compat_ioctl(struct file *file, > unsigned int cmd, return 0; > } > > +#if defined(HAVE_ARCH_FB_UNMAPPED_AREA) && !defined(get_fb_unmapped_area) Only blackfin and sparc define HAVE_ARCH_FB_UNMAPPED_AREA, and both of them provide an implementation of get_fb_unmapped_area(). > +unsigned long get_fb_unmapped_area(struct file *filp, > + unsigned long addr, unsigned long len, > + unsigned long pgoff, unsigned long flags) > +{ > + struct fb_info * const info = filp->private_data; > + unsigned long fb_size = PAGE_ALIGN(info->fix.smem_len); > + > + if (pgoff > fb_size || len > fb_size - pgoff) > + return -EINVAL; > + > + return (unsigned long)info->screen_base + pgoff; > +} > +#endif > + > static const struct file_operations fb_fops = { > .owner = THIS_MODULE, > .read = fb_read, [snip]
On Wed, Nov 30, 2016 at 02:19:18PM +0100, Benjamin Gaignard wrote: > Some SoC without MMU have display driver where a drm/kms driver > could be implemented. > Before doing such kind of thing drm/kms must allow to use mmuless > devices. > This patch propose to remove MMU configuration flag and add some > cma helpers functions to help implementing mmuless display driver > > version 2: > - define get_fb_unmapped_area() in drivers/video/fbdev/core/fbmem.c > if HAVE_ARCH_FB_UNMAPPED_AREA is set and get_fb_unmapped_area not > already defined in the architecture > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> > --- > drivers/gpu/drm/Kconfig | 4 +-- > drivers/gpu/drm/drm_fb_cma_helper.c | 4 +++ > drivers/gpu/drm/drm_gem_cma_helper.c | 62 ++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_vm.c | 4 +++ > drivers/video/fbdev/core/fbmem.c | 15 +++++++++ > include/drm/drm_gem_cma_helper.h | 6 ++++ > 6 files changed, 93 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 8d9cf73..36c102b 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -6,7 +6,7 @@ > # > menuconfig DRM > tristate "Direct Rendering Manager (XFree86 4.1.0 and higher DRI support)" > - depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && MMU && HAS_DMA > + depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && HAS_DMA > select HDMI > select FB_CMDLINE > select I2C > @@ -98,7 +98,7 @@ config DRM_LOAD_EDID_FIRMWARE > > config DRM_TTM > tristate > - depends on DRM > + depends on DRM && MMU > help > GPU memory management subsystem for devices with multiple > GPU memory types. Will be enabled automatically if a device driver > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c > index 81b3558..b1414b6 100644 > --- a/drivers/gpu/drm/drm_fb_cma_helper.c > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c > @@ -343,8 +343,12 @@ int drm_fb_cma_debugfs_show(struct seq_file *m, void *arg) > > static int drm_fb_cma_mmap(struct fb_info *info, struct vm_area_struct *vma) > { > +#ifdef CONFIG_MMU > return dma_mmap_writecombine(info->device, vma, info->screen_base, > info->fix.smem_start, info->fix.smem_len); > +#else > + return vm_iomap_memory(vma, vma->vm_start, info->fix.smem_len); > +#endif > } > > static struct fb_ops drm_fbdev_cma_ops = { > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c > index 1d6c335..de13535 100644 > --- a/drivers/gpu/drm/drm_gem_cma_helper.c > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c > @@ -318,11 +318,16 @@ static int drm_gem_cma_mmap_obj(struct drm_gem_cma_object *cma_obj, > vma->vm_flags &= ~VM_PFNMAP; > vma->vm_pgoff = 0; > > +#ifdef CONFIG_MMU > ret = dma_mmap_wc(cma_obj->base.dev->dev, vma, cma_obj->vaddr, > cma_obj->paddr, vma->vm_end - vma->vm_start); > if (ret) > drm_gem_vm_close(vma); > > +#else > + ret = vm_iomap_memory(vma, vma->vm_start, > + (vma->vm_end - vma->vm_start)); > +#endif Ok, me asking for a separate helper was probably too much. The #ifdefs here are still ugly, but on 2nd thought this just looks like the dma api is broken on no-mmu? Shouldn't we instead have a patch somewhere which gives us a working dma_mmap_wc on nommu platforms? > return ret; > } > > @@ -358,6 +363,63 @@ int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma) > } > EXPORT_SYMBOL_GPL(drm_gem_cma_mmap); > > +#ifdef CONFIG_MMU > +#define drm_gem_cma_get_unmapped_area NULL Depending upon how it's used it might be better to stub it out ... > +#else > +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp, > + unsigned long addr, > + unsigned long len, > + unsigned long pgoff, > + unsigned long flags) With no user and no kerneldoc I kinda have no idea what this is even for. The name also doesn't help, it says "get_unmapped_area", but very clearly you're returning a kernel vaddr cast to a long (and with ERR_PTR semantics on top, but non-standard). Why is this not called get_vaddr? -Daniel > +{ > + struct drm_gem_cma_object *cma_obj; > + struct drm_gem_object *obj = NULL; > + struct drm_file *priv = filp->private_data; > + struct drm_device *dev = priv->minor->dev; > + struct drm_vma_offset_node *node; > + > + if (drm_device_is_unplugged(dev)) > + return -ENODEV; > + > + drm_vma_offset_lock_lookup(dev->vma_offset_manager); > + node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager, > + pgoff, > + len >> PAGE_SHIFT); > + if (likely(node)) { > + obj = container_of(node, struct drm_gem_object, vma_node); > + /* > + * When the object is being freed, after it hits 0-refcnt it > + * proceeds to tear down the object. In the process it will > + * attempt to remove the VMA offset and so acquire this > + * mgr->vm_lock. Therefore if we find an object with a 0-refcnt > + * that matches our range, we know it is in the process of being > + * destroyed and will be freed as soon as we release the lock - > + * so we have to check for the 0-refcnted object and treat it as > + * invalid. > + */ > + if (!kref_get_unless_zero(&obj->refcount)) > + obj = NULL; > + } > + > + drm_vma_offset_unlock_lookup(dev->vma_offset_manager); > + > + if (!obj) > + return -EINVAL; > + > + if (!drm_vma_node_is_allowed(node, priv)) { > + drm_gem_object_unreference_unlocked(obj); > + return -EACCES; > + } > + > + cma_obj = to_drm_gem_cma_obj(obj); > + > + drm_gem_object_unreference_unlocked(obj); > + > + return cma_obj->vaddr ? (unsigned long)cma_obj->vaddr : -EINVAL; > +} > +#endif > +EXPORT_SYMBOL_GPL(drm_gem_cma_get_unmapped_area); > + > #ifdef CONFIG_DEBUG_FS > /** > * drm_gem_cma_describe - describe a CMA GEM object for debugfs > diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c > index caa4e4c..974e02b 100644 > --- a/drivers/gpu/drm/drm_vm.c > +++ b/drivers/gpu/drm/drm_vm.c > @@ -489,6 +489,7 @@ static int drm_mmap_dma(struct file *filp, struct vm_area_struct *vma) > #if defined(__i386__) || defined(__x86_64__) > pgprot_val(vma->vm_page_prot) &= ~_PAGE_RW; > #else > +#ifdef CONFIG_MMU > /* Ye gads this is ugly. With more thought > we could move this up higher and use > `protection_map' instead. */ > @@ -497,6 +498,7 @@ static int drm_mmap_dma(struct file *filp, struct vm_area_struct *vma) > (pte_wrprotect > (__pte(pgprot_val(vma->vm_page_prot))))); > #endif > +#endif > } > > vma->vm_ops = &drm_vm_dma_ops; > @@ -573,6 +575,7 @@ static int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma) > #if defined(__i386__) || defined(__x86_64__) > pgprot_val(vma->vm_page_prot) &= ~_PAGE_RW; > #else > +#ifdef CONFIG_MMU > /* Ye gads this is ugly. With more thought > we could move this up higher and use > `protection_map' instead. */ > @@ -581,6 +584,7 @@ static int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma) > (pte_wrprotect > (__pte(pgprot_val(vma->vm_page_prot))))); > #endif > +#endif > } > > switch (map->type) { > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c > index 76c1ad9..54488ee 100644 > --- a/drivers/video/fbdev/core/fbmem.c > +++ b/drivers/video/fbdev/core/fbmem.c > @@ -1492,6 +1492,21 @@ static long fb_compat_ioctl(struct file *file, unsigned int cmd, > return 0; > } > > +#if defined(HAVE_ARCH_FB_UNMAPPED_AREA) && !defined(get_fb_unmapped_area) > +unsigned long get_fb_unmapped_area(struct file *filp, > + unsigned long addr, unsigned long len, > + unsigned long pgoff, unsigned long flags) > +{ > + struct fb_info * const info = filp->private_data; > + unsigned long fb_size = PAGE_ALIGN(info->fix.smem_len); > + > + if (pgoff > fb_size || len > fb_size - pgoff) > + return -EINVAL; > + > + return (unsigned long)info->screen_base + pgoff; > +} > +#endif > + > static const struct file_operations fb_fops = { > .owner = THIS_MODULE, > .read = fb_read, > diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h > index acd6af8..177a20d 100644 > --- a/include/drm/drm_gem_cma_helper.h > +++ b/include/drm/drm_gem_cma_helper.h > @@ -53,6 +53,12 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, > > extern const struct vm_operations_struct drm_gem_cma_vm_ops; > > +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp, > + unsigned long addr, > + unsigned long len, > + unsigned long pgoff, > + unsigned long flags); > + > #ifdef CONFIG_DEBUG_FS > void drm_gem_cma_describe(struct drm_gem_cma_object *obj, struct seq_file *m); > #endif > -- > 1.9.1 >
2016-12-01 7:56 GMT+01:00 Daniel Vetter <daniel@ffwll.ch>: > On Wed, Nov 30, 2016 at 02:19:18PM +0100, Benjamin Gaignard wrote: >> Some SoC without MMU have display driver where a drm/kms driver >> could be implemented. >> Before doing such kind of thing drm/kms must allow to use mmuless >> devices. >> This patch propose to remove MMU configuration flag and add some >> cma helpers functions to help implementing mmuless display driver >> >> version 2: >> - define get_fb_unmapped_area() in drivers/video/fbdev/core/fbmem.c >> if HAVE_ARCH_FB_UNMAPPED_AREA is set and get_fb_unmapped_area not >> already defined in the architecture >> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> >> --- >> drivers/gpu/drm/Kconfig | 4 +-- >> drivers/gpu/drm/drm_fb_cma_helper.c | 4 +++ >> drivers/gpu/drm/drm_gem_cma_helper.c | 62 ++++++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/drm_vm.c | 4 +++ >> drivers/video/fbdev/core/fbmem.c | 15 +++++++++ >> include/drm/drm_gem_cma_helper.h | 6 ++++ >> 6 files changed, 93 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >> index 8d9cf73..36c102b 100644 >> --- a/drivers/gpu/drm/Kconfig >> +++ b/drivers/gpu/drm/Kconfig >> @@ -6,7 +6,7 @@ >> # >> menuconfig DRM >> tristate "Direct Rendering Manager (XFree86 4.1.0 and higher DRI support)" >> - depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && MMU && HAS_DMA >> + depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && HAS_DMA >> select HDMI >> select FB_CMDLINE >> select I2C >> @@ -98,7 +98,7 @@ config DRM_LOAD_EDID_FIRMWARE >> >> config DRM_TTM >> tristate >> - depends on DRM >> + depends on DRM && MMU >> help >> GPU memory management subsystem for devices with multiple >> GPU memory types. Will be enabled automatically if a device driver >> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c >> index 81b3558..b1414b6 100644 >> --- a/drivers/gpu/drm/drm_fb_cma_helper.c >> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c >> @@ -343,8 +343,12 @@ int drm_fb_cma_debugfs_show(struct seq_file *m, void *arg) >> >> static int drm_fb_cma_mmap(struct fb_info *info, struct vm_area_struct *vma) >> { >> +#ifdef CONFIG_MMU >> return dma_mmap_writecombine(info->device, vma, info->screen_base, >> info->fix.smem_start, info->fix.smem_len); >> +#else >> + return vm_iomap_memory(vma, vma->vm_start, info->fix.smem_len); >> +#endif >> } >> >> static struct fb_ops drm_fbdev_cma_ops = { >> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c >> index 1d6c335..de13535 100644 >> --- a/drivers/gpu/drm/drm_gem_cma_helper.c >> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c >> @@ -318,11 +318,16 @@ static int drm_gem_cma_mmap_obj(struct drm_gem_cma_object *cma_obj, >> vma->vm_flags &= ~VM_PFNMAP; >> vma->vm_pgoff = 0; >> >> +#ifdef CONFIG_MMU >> ret = dma_mmap_wc(cma_obj->base.dev->dev, vma, cma_obj->vaddr, >> cma_obj->paddr, vma->vm_end - vma->vm_start); >> if (ret) >> drm_gem_vm_close(vma); >> >> +#else >> + ret = vm_iomap_memory(vma, vma->vm_start, >> + (vma->vm_end - vma->vm_start)); >> +#endif > > Ok, me asking for a separate helper was probably too much. The #ifdefs > here are still ugly, but on 2nd thought this just looks like the dma api > is broken on no-mmu? Shouldn't we instead have a patch somewhere which > gives us a working dma_mmap_wc on nommu platforms? > >> return ret; >> } >> >> @@ -358,6 +363,63 @@ int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma) >> } >> EXPORT_SYMBOL_GPL(drm_gem_cma_mmap); >> >> +#ifdef CONFIG_MMU >> +#define drm_gem_cma_get_unmapped_area NULL > > Depending upon how it's used it might be better to stub it out ... > >> +#else >> +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp, >> + unsigned long addr, >> + unsigned long len, >> + unsigned long pgoff, >> + unsigned long flags) > > With no user and no kerneldoc I kinda have no idea what this is even for. > The name also doesn't help, it says "get_unmapped_area", but very clearly > you're returning a kernel vaddr cast to a long (and with ERR_PTR > semantics on top, but non-standard). Why is this not called get_vaddr? > -Daniel > >> +{ >> + struct drm_gem_cma_object *cma_obj; >> + struct drm_gem_object *obj = NULL; >> + struct drm_file *priv = filp->private_data; >> + struct drm_device *dev = priv->minor->dev; >> + struct drm_vma_offset_node *node; >> + >> + if (drm_device_is_unplugged(dev)) >> + return -ENODEV; >> + >> + drm_vma_offset_lock_lookup(dev->vma_offset_manager); >> + node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager, >> + pgoff, >> + len >> PAGE_SHIFT); >> + if (likely(node)) { >> + obj = container_of(node, struct drm_gem_object, vma_node); >> + /* >> + * When the object is being freed, after it hits 0-refcnt it >> + * proceeds to tear down the object. In the process it will >> + * attempt to remove the VMA offset and so acquire this >> + * mgr->vm_lock. Therefore if we find an object with a 0-refcnt >> + * that matches our range, we know it is in the process of being >> + * destroyed and will be freed as soon as we release the lock - >> + * so we have to check for the 0-refcnted object and treat it as >> + * invalid. >> + */ >> + if (!kref_get_unless_zero(&obj->refcount)) >> + obj = NULL; >> + } >> + >> + drm_vma_offset_unlock_lookup(dev->vma_offset_manager); >> + >> + if (!obj) >> + return -EINVAL; >> + >> + if (!drm_vma_node_is_allowed(node, priv)) { >> + drm_gem_object_unreference_unlocked(obj); >> + return -EACCES; >> + } >> + >> + cma_obj = to_drm_gem_cma_obj(obj); >> + >> + drm_gem_object_unreference_unlocked(obj); >> + >> + return cma_obj->vaddr ? (unsigned long)cma_obj->vaddr : -EINVAL; >> +} >> +#endif >> +EXPORT_SYMBOL_GPL(drm_gem_cma_get_unmapped_area); >> + >> #ifdef CONFIG_DEBUG_FS >> /** >> * drm_gem_cma_describe - describe a CMA GEM object for debugfs >> diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c >> index caa4e4c..974e02b 100644 >> --- a/drivers/gpu/drm/drm_vm.c >> +++ b/drivers/gpu/drm/drm_vm.c >> @@ -489,6 +489,7 @@ static int drm_mmap_dma(struct file *filp, struct vm_area_struct *vma) >> #if defined(__i386__) || defined(__x86_64__) >> pgprot_val(vma->vm_page_prot) &= ~_PAGE_RW; >> #else >> +#ifdef CONFIG_MMU >> /* Ye gads this is ugly. With more thought >> we could move this up higher and use >> `protection_map' instead. */ >> @@ -497,6 +498,7 @@ static int drm_mmap_dma(struct file *filp, struct vm_area_struct *vma) >> (pte_wrprotect >> (__pte(pgprot_val(vma->vm_page_prot))))); >> #endif >> +#endif >> } >> >> vma->vm_ops = &drm_vm_dma_ops; >> @@ -573,6 +575,7 @@ static int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma) >> #if defined(__i386__) || defined(__x86_64__) >> pgprot_val(vma->vm_page_prot) &= ~_PAGE_RW; >> #else >> +#ifdef CONFIG_MMU >> /* Ye gads this is ugly. With more thought >> we could move this up higher and use >> `protection_map' instead. */ >> @@ -581,6 +584,7 @@ static int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma) >> (pte_wrprotect >> (__pte(pgprot_val(vma->vm_page_prot))))); >> #endif >> +#endif >> } >> >> switch (map->type) { >> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c >> index 76c1ad9..54488ee 100644 >> --- a/drivers/video/fbdev/core/fbmem.c >> +++ b/drivers/video/fbdev/core/fbmem.c >> @@ -1492,6 +1492,21 @@ static long fb_compat_ioctl(struct file *file, unsigned int cmd, >> return 0; >> } >> >> +#if defined(HAVE_ARCH_FB_UNMAPPED_AREA) && !defined(get_fb_unmapped_area) >> +unsigned long get_fb_unmapped_area(struct file *filp, >> + unsigned long addr, unsigned long len, >> + unsigned long pgoff, unsigned long flags) >> +{ >> + struct fb_info * const info = filp->private_data; >> + unsigned long fb_size = PAGE_ALIGN(info->fix.smem_len); >> + >> + if (pgoff > fb_size || len > fb_size - pgoff) >> + return -EINVAL; >> + >> + return (unsigned long)info->screen_base + pgoff; >> +} >> +#endif >> + >> static const struct file_operations fb_fops = { >> .owner = THIS_MODULE, >> .read = fb_read, >> diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h >> index acd6af8..177a20d 100644 >> --- a/include/drm/drm_gem_cma_helper.h >> +++ b/include/drm/drm_gem_cma_helper.h >> @@ -53,6 +53,12 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, >> >> extern const struct vm_operations_struct drm_gem_cma_vm_ops; >> >> +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp, >> + unsigned long addr, >> + unsigned long len, >> + unsigned long pgoff, >> + unsigned long flags); >> + >> #ifdef CONFIG_DEBUG_FS >> void drm_gem_cma_describe(struct drm_gem_cma_object *obj, struct seq_file *m); >> #endif >> -- >> 1.9.1 >> > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
2016-12-01 7:56 GMT+01:00 Daniel Vetter <daniel@ffwll.ch>: > On Wed, Nov 30, 2016 at 02:19:18PM +0100, Benjamin Gaignard wrote: >> Some SoC without MMU have display driver where a drm/kms driver >> could be implemented. >> Before doing such kind of thing drm/kms must allow to use mmuless >> devices. >> This patch propose to remove MMU configuration flag and add some >> cma helpers functions to help implementing mmuless display driver >> >> version 2: >> - define get_fb_unmapped_area() in drivers/video/fbdev/core/fbmem.c >> if HAVE_ARCH_FB_UNMAPPED_AREA is set and get_fb_unmapped_area not >> already defined in the architecture >> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> >> --- >> drivers/gpu/drm/Kconfig | 4 +-- >> drivers/gpu/drm/drm_fb_cma_helper.c | 4 +++ >> drivers/gpu/drm/drm_gem_cma_helper.c | 62 ++++++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/drm_vm.c | 4 +++ >> drivers/video/fbdev/core/fbmem.c | 15 +++++++++ >> include/drm/drm_gem_cma_helper.h | 6 ++++ >> 6 files changed, 93 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >> index 8d9cf73..36c102b 100644 >> --- a/drivers/gpu/drm/Kconfig >> +++ b/drivers/gpu/drm/Kconfig >> @@ -6,7 +6,7 @@ >> # >> menuconfig DRM >> tristate "Direct Rendering Manager (XFree86 4.1.0 and higher DRI support)" >> - depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && MMU && HAS_DMA >> + depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && HAS_DMA >> select HDMI >> select FB_CMDLINE >> select I2C >> @@ -98,7 +98,7 @@ config DRM_LOAD_EDID_FIRMWARE >> >> config DRM_TTM >> tristate >> - depends on DRM >> + depends on DRM && MMU >> help >> GPU memory management subsystem for devices with multiple >> GPU memory types. Will be enabled automatically if a device driver >> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c >> index 81b3558..b1414b6 100644 >> --- a/drivers/gpu/drm/drm_fb_cma_helper.c >> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c >> @@ -343,8 +343,12 @@ int drm_fb_cma_debugfs_show(struct seq_file *m, void *arg) >> >> static int drm_fb_cma_mmap(struct fb_info *info, struct vm_area_struct *vma) >> { >> +#ifdef CONFIG_MMU >> return dma_mmap_writecombine(info->device, vma, info->screen_base, >> info->fix.smem_start, info->fix.smem_len); >> +#else >> + return vm_iomap_memory(vma, vma->vm_start, info->fix.smem_len); >> +#endif >> } >> >> static struct fb_ops drm_fbdev_cma_ops = { >> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c >> index 1d6c335..de13535 100644 >> --- a/drivers/gpu/drm/drm_gem_cma_helper.c >> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c >> @@ -318,11 +318,16 @@ static int drm_gem_cma_mmap_obj(struct drm_gem_cma_object *cma_obj, >> vma->vm_flags &= ~VM_PFNMAP; >> vma->vm_pgoff = 0; >> >> +#ifdef CONFIG_MMU >> ret = dma_mmap_wc(cma_obj->base.dev->dev, vma, cma_obj->vaddr, >> cma_obj->paddr, vma->vm_end - vma->vm_start); >> if (ret) >> drm_gem_vm_close(vma); >> >> +#else >> + ret = vm_iomap_memory(vma, vma->vm_start, >> + (vma->vm_end - vma->vm_start)); >> +#endif > > Ok, me asking for a separate helper was probably too much. The #ifdefs > here are still ugly, but on 2nd thought this just looks like the dma api > is broken on no-mmu? Shouldn't we instead have a patch somewhere which > gives us a working dma_mmap_wc on nommu platforms? > I not sure that dma_mmap_wc is broken for all nommu platforms, but at least for ARM nommu it return an error: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/mm/dma-mapping.c#n851 Could it be an alternative to call vm_iomap_memory if dma_map_wc has failed and !CONFIG_MMU ? >> return ret; >> } >> >> @@ -358,6 +363,63 @@ int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma) >> } >> EXPORT_SYMBOL_GPL(drm_gem_cma_mmap); >> >> +#ifdef CONFIG_MMU >> +#define drm_gem_cma_get_unmapped_area NULL > > Depending upon how it's used it might be better to stub it out ... This function shouldn't be called in MMU case but I can remove the flag and compile it for any case > >> +#else >> +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp, >> + unsigned long addr, >> + unsigned long len, >> + unsigned long pgoff, >> + unsigned long flags) > > With no user and no kerneldoc I kinda have no idea what this is even for. > The name also doesn't help, it says "get_unmapped_area", but very clearly > you're returning a kernel vaddr cast to a long (and with ERR_PTR > semantics on top, but non-standard). Why is this not called get_vaddr? This function will be used in driver file_operations structure to set get_unmapped_area field. For example v4l2 driver use it: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/media/v4l2-core/v4l2-dev.c#n462 I don't have found so much examples of that but each time they have keep "get_unmapped_area" in function name so I just do the same. In Documentation/nommu-mmap.txt you have few words about get_unmapped_area() > -Daniel > >> +{ >> + struct drm_gem_cma_object *cma_obj; >> + struct drm_gem_object *obj = NULL; >> + struct drm_file *priv = filp->private_data; >> + struct drm_device *dev = priv->minor->dev; >> + struct drm_vma_offset_node *node; >> + >> + if (drm_device_is_unplugged(dev)) >> + return -ENODEV; >> + >> + drm_vma_offset_lock_lookup(dev->vma_offset_manager); >> + node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager, >> + pgoff, >> + len >> PAGE_SHIFT); >> + if (likely(node)) { >> + obj = container_of(node, struct drm_gem_object, vma_node); >> + /* >> + * When the object is being freed, after it hits 0-refcnt it >> + * proceeds to tear down the object. In the process it will >> + * attempt to remove the VMA offset and so acquire this >> + * mgr->vm_lock. Therefore if we find an object with a 0-refcnt >> + * that matches our range, we know it is in the process of being >> + * destroyed and will be freed as soon as we release the lock - >> + * so we have to check for the 0-refcnted object and treat it as >> + * invalid. >> + */ >> + if (!kref_get_unless_zero(&obj->refcount)) >> + obj = NULL; >> + } >> + >> + drm_vma_offset_unlock_lookup(dev->vma_offset_manager); >> + >> + if (!obj) >> + return -EINVAL; >> + >> + if (!drm_vma_node_is_allowed(node, priv)) { >> + drm_gem_object_unreference_unlocked(obj); >> + return -EACCES; >> + } >> + >> + cma_obj = to_drm_gem_cma_obj(obj); >> + >> + drm_gem_object_unreference_unlocked(obj); >> + >> + return cma_obj->vaddr ? (unsigned long)cma_obj->vaddr : -EINVAL; >> +} >> +#endif >> +EXPORT_SYMBOL_GPL(drm_gem_cma_get_unmapped_area); >> + >> #ifdef CONFIG_DEBUG_FS >> /** >> * drm_gem_cma_describe - describe a CMA GEM object for debugfs >> diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c >> index caa4e4c..974e02b 100644 >> --- a/drivers/gpu/drm/drm_vm.c >> +++ b/drivers/gpu/drm/drm_vm.c >> @@ -489,6 +489,7 @@ static int drm_mmap_dma(struct file *filp, struct vm_area_struct *vma) >> #if defined(__i386__) || defined(__x86_64__) >> pgprot_val(vma->vm_page_prot) &= ~_PAGE_RW; >> #else >> +#ifdef CONFIG_MMU >> /* Ye gads this is ugly. With more thought >> we could move this up higher and use >> `protection_map' instead. */ >> @@ -497,6 +498,7 @@ static int drm_mmap_dma(struct file *filp, struct vm_area_struct *vma) >> (pte_wrprotect >> (__pte(pgprot_val(vma->vm_page_prot))))); >> #endif >> +#endif >> } >> >> vma->vm_ops = &drm_vm_dma_ops; >> @@ -573,6 +575,7 @@ static int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma) >> #if defined(__i386__) || defined(__x86_64__) >> pgprot_val(vma->vm_page_prot) &= ~_PAGE_RW; >> #else >> +#ifdef CONFIG_MMU >> /* Ye gads this is ugly. With more thought >> we could move this up higher and use >> `protection_map' instead. */ >> @@ -581,6 +584,7 @@ static int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma) >> (pte_wrprotect >> (__pte(pgprot_val(vma->vm_page_prot))))); >> #endif >> +#endif >> } >> >> switch (map->type) { >> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c >> index 76c1ad9..54488ee 100644 >> --- a/drivers/video/fbdev/core/fbmem.c >> +++ b/drivers/video/fbdev/core/fbmem.c >> @@ -1492,6 +1492,21 @@ static long fb_compat_ioctl(struct file *file, unsigned int cmd, >> return 0; >> } >> >> +#if defined(HAVE_ARCH_FB_UNMAPPED_AREA) && !defined(get_fb_unmapped_area) >> +unsigned long get_fb_unmapped_area(struct file *filp, >> + unsigned long addr, unsigned long len, >> + unsigned long pgoff, unsigned long flags) >> +{ >> + struct fb_info * const info = filp->private_data; >> + unsigned long fb_size = PAGE_ALIGN(info->fix.smem_len); >> + >> + if (pgoff > fb_size || len > fb_size - pgoff) >> + return -EINVAL; >> + >> + return (unsigned long)info->screen_base + pgoff; >> +} >> +#endif >> + >> static const struct file_operations fb_fops = { >> .owner = THIS_MODULE, >> .read = fb_read, >> diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h >> index acd6af8..177a20d 100644 >> --- a/include/drm/drm_gem_cma_helper.h >> +++ b/include/drm/drm_gem_cma_helper.h >> @@ -53,6 +53,12 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, >> >> extern const struct vm_operations_struct drm_gem_cma_vm_ops; >> >> +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp, >> + unsigned long addr, >> + unsigned long len, >> + unsigned long pgoff, >> + unsigned long flags); >> + >> #ifdef CONFIG_DEBUG_FS >> void drm_gem_cma_describe(struct drm_gem_cma_object *obj, struct seq_file *m); >> #endif >> -- >> 1.9.1 >> > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Thu, Dec 01, 2016 at 08:21:11AM +0100, Benjamin Gaignard wrote: > 2016-12-01 7:56 GMT+01:00 Daniel Vetter <daniel@ffwll.ch>: > > On Wed, Nov 30, 2016 at 02:19:18PM +0100, Benjamin Gaignard wrote: > >> Some SoC without MMU have display driver where a drm/kms driver > >> could be implemented. > >> Before doing such kind of thing drm/kms must allow to use mmuless > >> devices. > >> This patch propose to remove MMU configuration flag and add some > >> cma helpers functions to help implementing mmuless display driver > >> > >> version 2: > >> - define get_fb_unmapped_area() in drivers/video/fbdev/core/fbmem.c > >> if HAVE_ARCH_FB_UNMAPPED_AREA is set and get_fb_unmapped_area not > >> already defined in the architecture > >> > >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> > >> --- > >> drivers/gpu/drm/Kconfig | 4 +-- > >> drivers/gpu/drm/drm_fb_cma_helper.c | 4 +++ > >> drivers/gpu/drm/drm_gem_cma_helper.c | 62 ++++++++++++++++++++++++++++++++++++ > >> drivers/gpu/drm/drm_vm.c | 4 +++ > >> drivers/video/fbdev/core/fbmem.c | 15 +++++++++ > >> include/drm/drm_gem_cma_helper.h | 6 ++++ > >> 6 files changed, 93 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > >> index 8d9cf73..36c102b 100644 > >> --- a/drivers/gpu/drm/Kconfig > >> +++ b/drivers/gpu/drm/Kconfig > >> @@ -6,7 +6,7 @@ > >> # > >> menuconfig DRM > >> tristate "Direct Rendering Manager (XFree86 4.1.0 and higher DRI support)" > >> - depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && MMU && HAS_DMA > >> + depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && HAS_DMA > >> select HDMI > >> select FB_CMDLINE > >> select I2C > >> @@ -98,7 +98,7 @@ config DRM_LOAD_EDID_FIRMWARE > >> > >> config DRM_TTM > >> tristate > >> - depends on DRM > >> + depends on DRM && MMU > >> help > >> GPU memory management subsystem for devices with multiple > >> GPU memory types. Will be enabled automatically if a device driver > >> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c > >> index 81b3558..b1414b6 100644 > >> --- a/drivers/gpu/drm/drm_fb_cma_helper.c > >> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c > >> @@ -343,8 +343,12 @@ int drm_fb_cma_debugfs_show(struct seq_file *m, void *arg) > >> > >> static int drm_fb_cma_mmap(struct fb_info *info, struct vm_area_struct *vma) > >> { > >> +#ifdef CONFIG_MMU > >> return dma_mmap_writecombine(info->device, vma, info->screen_base, > >> info->fix.smem_start, info->fix.smem_len); > >> +#else > >> + return vm_iomap_memory(vma, vma->vm_start, info->fix.smem_len); > >> +#endif > >> } > >> > >> static struct fb_ops drm_fbdev_cma_ops = { > >> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c > >> index 1d6c335..de13535 100644 > >> --- a/drivers/gpu/drm/drm_gem_cma_helper.c > >> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c > >> @@ -318,11 +318,16 @@ static int drm_gem_cma_mmap_obj(struct drm_gem_cma_object *cma_obj, > >> vma->vm_flags &= ~VM_PFNMAP; > >> vma->vm_pgoff = 0; > >> > >> +#ifdef CONFIG_MMU > >> ret = dma_mmap_wc(cma_obj->base.dev->dev, vma, cma_obj->vaddr, > >> cma_obj->paddr, vma->vm_end - vma->vm_start); > >> if (ret) > >> drm_gem_vm_close(vma); > >> > >> +#else > >> + ret = vm_iomap_memory(vma, vma->vm_start, > >> + (vma->vm_end - vma->vm_start)); > >> +#endif > > > > Ok, me asking for a separate helper was probably too much. The #ifdefs > > here are still ugly, but on 2nd thought this just looks like the dma api > > is broken on no-mmu? Shouldn't we instead have a patch somewhere which > > gives us a working dma_mmap_wc on nommu platforms? > > > > I not sure that dma_mmap_wc is broken for all nommu platforms, but at least > for ARM nommu it return an error: > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/mm/dma-mapping.c#n851 > > Could it be an alternative to call vm_iomap_memory if dma_map_wc has > failed and !CONFIG_MMU ? Sprinkle #ifdefs all over the place is imo the problem, not what exactly it looks like. Imo this should be fixed in dma-mapping.c. And if core maintainers hate that, then we need to define a dma_mmap_wc in drm headers which works better than the core one ;-) Something like: #ifdef NOMMU #define dma_mmap_wc drm_dma_mmap_wc static inline drm_dma_mmap_wc( .... ) { vm_remap_memory( ....); } #endif We should probably create a drm-core.h to collect all these (we have a metric pile of them in i915 for various things that are too hard to get merged for no good reason). > >> return ret; > >> } > >> > >> @@ -358,6 +363,63 @@ int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma) > >> } > >> EXPORT_SYMBOL_GPL(drm_gem_cma_mmap); > >> > >> +#ifdef CONFIG_MMU > >> +#define drm_gem_cma_get_unmapped_area NULL > > > > Depending upon how it's used it might be better to stub it out ... > > This function shouldn't be called in MMU case but I can remove the > flag and compile it for any case > > > > >> +#else > >> +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp, > >> + unsigned long addr, > >> + unsigned long len, > >> + unsigned long pgoff, > >> + unsigned long flags) > > > > With no user and no kerneldoc I kinda have no idea what this is even for. > > The name also doesn't help, it says "get_unmapped_area", but very clearly > > you're returning a kernel vaddr cast to a long (and with ERR_PTR > > semantics on top, but non-standard). Why is this not called get_vaddr? > > This function will be used in driver file_operations structure to set > get_unmapped_area field. > For example v4l2 driver use it: > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/media/v4l2-core/v4l2-dev.c#n462 > I don't have found so much examples of that but each time they have > keep "get_unmapped_area" > in function name so I just do the same. > > In Documentation/nommu-mmap.txt you have few words about get_unmapped_area() Hm ok. I think it's time to digg out an old idea I've had: #define DRM_GEM_CMA_FOPS(driver) \ static const struct file_operations driver ## _drm_fops = { \ .owner = THIS_MODULE, \ .open = drm_open, \ .release = drm_release, \ .unlocked_ioctl = drm_ioctl, \ .compat_ioctl = drm_compat_ioctl, \ .poll = drm_poll, \ .read = drm_read, \ .llseek = no_llseek, \ .mmap = drm_gem_cma_mmap, \ }; \ Then you have one nice place to put the NOMMU #ifdeffery. Would be great to roll this out to drivers. Still need kerneldoc for this new hook, with a reference to Documentation/nommu-mmap.txt. I think with that bit of polish&abstraction it makes sense to keep this all in cma helpers itself. -Daniel > > > -Daniel > > > >> +{ > >> + struct drm_gem_cma_object *cma_obj; > >> + struct drm_gem_object *obj = NULL; > >> + struct drm_file *priv = filp->private_data; > >> + struct drm_device *dev = priv->minor->dev; > >> + struct drm_vma_offset_node *node; > >> + > >> + if (drm_device_is_unplugged(dev)) > >> + return -ENODEV; > >> + > >> + drm_vma_offset_lock_lookup(dev->vma_offset_manager); > >> + node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager, > >> + pgoff, > >> + len >> PAGE_SHIFT); > >> + if (likely(node)) { > >> + obj = container_of(node, struct drm_gem_object, vma_node); > >> + /* > >> + * When the object is being freed, after it hits 0-refcnt it > >> + * proceeds to tear down the object. In the process it will > >> + * attempt to remove the VMA offset and so acquire this > >> + * mgr->vm_lock. Therefore if we find an object with a 0-refcnt > >> + * that matches our range, we know it is in the process of being > >> + * destroyed and will be freed as soon as we release the lock - > >> + * so we have to check for the 0-refcnted object and treat it as > >> + * invalid. > >> + */ > >> + if (!kref_get_unless_zero(&obj->refcount)) > >> + obj = NULL; > >> + } > >> + > >> + drm_vma_offset_unlock_lookup(dev->vma_offset_manager); > >> + > >> + if (!obj) > >> + return -EINVAL; > >> + > >> + if (!drm_vma_node_is_allowed(node, priv)) { > >> + drm_gem_object_unreference_unlocked(obj); > >> + return -EACCES; > >> + } > >> + > >> + cma_obj = to_drm_gem_cma_obj(obj); > >> + > >> + drm_gem_object_unreference_unlocked(obj); > >> + > >> + return cma_obj->vaddr ? (unsigned long)cma_obj->vaddr : -EINVAL; > >> +} > >> +#endif > >> +EXPORT_SYMBOL_GPL(drm_gem_cma_get_unmapped_area); > >> + > >> #ifdef CONFIG_DEBUG_FS > >> /** > >> * drm_gem_cma_describe - describe a CMA GEM object for debugfs > >> diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c > >> index caa4e4c..974e02b 100644 > >> --- a/drivers/gpu/drm/drm_vm.c > >> +++ b/drivers/gpu/drm/drm_vm.c > >> @@ -489,6 +489,7 @@ static int drm_mmap_dma(struct file *filp, struct vm_area_struct *vma) > >> #if defined(__i386__) || defined(__x86_64__) > >> pgprot_val(vma->vm_page_prot) &= ~_PAGE_RW; > >> #else > >> +#ifdef CONFIG_MMU > >> /* Ye gads this is ugly. With more thought > >> we could move this up higher and use > >> `protection_map' instead. */ > >> @@ -497,6 +498,7 @@ static int drm_mmap_dma(struct file *filp, struct vm_area_struct *vma) > >> (pte_wrprotect > >> (__pte(pgprot_val(vma->vm_page_prot))))); > >> #endif > >> +#endif > >> } > >> > >> vma->vm_ops = &drm_vm_dma_ops; > >> @@ -573,6 +575,7 @@ static int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma) > >> #if defined(__i386__) || defined(__x86_64__) > >> pgprot_val(vma->vm_page_prot) &= ~_PAGE_RW; > >> #else > >> +#ifdef CONFIG_MMU > >> /* Ye gads this is ugly. With more thought > >> we could move this up higher and use > >> `protection_map' instead. */ > >> @@ -581,6 +584,7 @@ static int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma) > >> (pte_wrprotect > >> (__pte(pgprot_val(vma->vm_page_prot))))); > >> #endif > >> +#endif > >> } > >> > >> switch (map->type) { > >> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c > >> index 76c1ad9..54488ee 100644 > >> --- a/drivers/video/fbdev/core/fbmem.c > >> +++ b/drivers/video/fbdev/core/fbmem.c > >> @@ -1492,6 +1492,21 @@ static long fb_compat_ioctl(struct file *file, unsigned int cmd, > >> return 0; > >> } > >> > >> +#if defined(HAVE_ARCH_FB_UNMAPPED_AREA) && !defined(get_fb_unmapped_area) > >> +unsigned long get_fb_unmapped_area(struct file *filp, > >> + unsigned long addr, unsigned long len, > >> + unsigned long pgoff, unsigned long flags) > >> +{ > >> + struct fb_info * const info = filp->private_data; > >> + unsigned long fb_size = PAGE_ALIGN(info->fix.smem_len); > >> + > >> + if (pgoff > fb_size || len > fb_size - pgoff) > >> + return -EINVAL; > >> + > >> + return (unsigned long)info->screen_base + pgoff; > >> +} > >> +#endif > >> + > >> static const struct file_operations fb_fops = { > >> .owner = THIS_MODULE, > >> .read = fb_read, > >> diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h > >> index acd6af8..177a20d 100644 > >> --- a/include/drm/drm_gem_cma_helper.h > >> +++ b/include/drm/drm_gem_cma_helper.h > >> @@ -53,6 +53,12 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, > >> > >> extern const struct vm_operations_struct drm_gem_cma_vm_ops; > >> > >> +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp, > >> + unsigned long addr, > >> + unsigned long len, > >> + unsigned long pgoff, > >> + unsigned long flags); > >> + > >> #ifdef CONFIG_DEBUG_FS > >> void drm_gem_cma_describe(struct drm_gem_cma_object *obj, struct seq_file *m); > >> #endif > >> -- > >> 1.9.1 > >> > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > > > -- > Benjamin Gaignard > > Graphic Study Group > > Linaro.org │ Open source software for ARM SoCs > > Follow Linaro: Facebook | Twitter | Blog
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 8d9cf73..36c102b 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -6,7 +6,7 @@ # menuconfig DRM tristate "Direct Rendering Manager (XFree86 4.1.0 and higher DRI support)" - depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && MMU && HAS_DMA + depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && HAS_DMA select HDMI select FB_CMDLINE select I2C @@ -98,7 +98,7 @@ config DRM_LOAD_EDID_FIRMWARE config DRM_TTM tristate - depends on DRM + depends on DRM && MMU help GPU memory management subsystem for devices with multiple GPU memory types. Will be enabled automatically if a device driver diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index 81b3558..b1414b6 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -343,8 +343,12 @@ int drm_fb_cma_debugfs_show(struct seq_file *m, void *arg) static int drm_fb_cma_mmap(struct fb_info *info, struct vm_area_struct *vma) { +#ifdef CONFIG_MMU return dma_mmap_writecombine(info->device, vma, info->screen_base, info->fix.smem_start, info->fix.smem_len); +#else + return vm_iomap_memory(vma, vma->vm_start, info->fix.smem_len); +#endif } static struct fb_ops drm_fbdev_cma_ops = { diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index 1d6c335..de13535 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -318,11 +318,16 @@ static int drm_gem_cma_mmap_obj(struct drm_gem_cma_object *cma_obj, vma->vm_flags &= ~VM_PFNMAP; vma->vm_pgoff = 0; +#ifdef CONFIG_MMU ret = dma_mmap_wc(cma_obj->base.dev->dev, vma, cma_obj->vaddr, cma_obj->paddr, vma->vm_end - vma->vm_start); if (ret) drm_gem_vm_close(vma); +#else + ret = vm_iomap_memory(vma, vma->vm_start, + (vma->vm_end - vma->vm_start)); +#endif return ret; } @@ -358,6 +363,63 @@ int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma) } EXPORT_SYMBOL_GPL(drm_gem_cma_mmap); +#ifdef CONFIG_MMU +#define drm_gem_cma_get_unmapped_area NULL +#else +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp, + unsigned long addr, + unsigned long len, + unsigned long pgoff, + unsigned long flags) +{ + struct drm_gem_cma_object *cma_obj; + struct drm_gem_object *obj = NULL; + struct drm_file *priv = filp->private_data; + struct drm_device *dev = priv->minor->dev; + struct drm_vma_offset_node *node; + + if (drm_device_is_unplugged(dev)) + return -ENODEV; + + drm_vma_offset_lock_lookup(dev->vma_offset_manager); + node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager, + pgoff, + len >> PAGE_SHIFT); + if (likely(node)) { + obj = container_of(node, struct drm_gem_object, vma_node); + /* + * When the object is being freed, after it hits 0-refcnt it + * proceeds to tear down the object. In the process it will + * attempt to remove the VMA offset and so acquire this + * mgr->vm_lock. Therefore if we find an object with a 0-refcnt + * that matches our range, we know it is in the process of being + * destroyed and will be freed as soon as we release the lock - + * so we have to check for the 0-refcnted object and treat it as + * invalid. + */ + if (!kref_get_unless_zero(&obj->refcount)) + obj = NULL; + } + + drm_vma_offset_unlock_lookup(dev->vma_offset_manager); + + if (!obj) + return -EINVAL; + + if (!drm_vma_node_is_allowed(node, priv)) { + drm_gem_object_unreference_unlocked(obj); + return -EACCES; + } + + cma_obj = to_drm_gem_cma_obj(obj); + + drm_gem_object_unreference_unlocked(obj); + + return cma_obj->vaddr ? (unsigned long)cma_obj->vaddr : -EINVAL; +} +#endif +EXPORT_SYMBOL_GPL(drm_gem_cma_get_unmapped_area); + #ifdef CONFIG_DEBUG_FS /** * drm_gem_cma_describe - describe a CMA GEM object for debugfs diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c index caa4e4c..974e02b 100644 --- a/drivers/gpu/drm/drm_vm.c +++ b/drivers/gpu/drm/drm_vm.c @@ -489,6 +489,7 @@ static int drm_mmap_dma(struct file *filp, struct vm_area_struct *vma) #if defined(__i386__) || defined(__x86_64__) pgprot_val(vma->vm_page_prot) &= ~_PAGE_RW; #else +#ifdef CONFIG_MMU /* Ye gads this is ugly. With more thought we could move this up higher and use `protection_map' instead. */ @@ -497,6 +498,7 @@ static int drm_mmap_dma(struct file *filp, struct vm_area_struct *vma) (pte_wrprotect (__pte(pgprot_val(vma->vm_page_prot))))); #endif +#endif } vma->vm_ops = &drm_vm_dma_ops; @@ -573,6 +575,7 @@ static int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma) #if defined(__i386__) || defined(__x86_64__) pgprot_val(vma->vm_page_prot) &= ~_PAGE_RW; #else +#ifdef CONFIG_MMU /* Ye gads this is ugly. With more thought we could move this up higher and use `protection_map' instead. */ @@ -581,6 +584,7 @@ static int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma) (pte_wrprotect (__pte(pgprot_val(vma->vm_page_prot))))); #endif +#endif } switch (map->type) { diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 76c1ad9..54488ee 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1492,6 +1492,21 @@ static long fb_compat_ioctl(struct file *file, unsigned int cmd, return 0; } +#if defined(HAVE_ARCH_FB_UNMAPPED_AREA) && !defined(get_fb_unmapped_area) +unsigned long get_fb_unmapped_area(struct file *filp, + unsigned long addr, unsigned long len, + unsigned long pgoff, unsigned long flags) +{ + struct fb_info * const info = filp->private_data; + unsigned long fb_size = PAGE_ALIGN(info->fix.smem_len); + + if (pgoff > fb_size || len > fb_size - pgoff) + return -EINVAL; + + return (unsigned long)info->screen_base + pgoff; +} +#endif + static const struct file_operations fb_fops = { .owner = THIS_MODULE, .read = fb_read, diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h index acd6af8..177a20d 100644 --- a/include/drm/drm_gem_cma_helper.h +++ b/include/drm/drm_gem_cma_helper.h @@ -53,6 +53,12 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, extern const struct vm_operations_struct drm_gem_cma_vm_ops; +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp, + unsigned long addr, + unsigned long len, + unsigned long pgoff, + unsigned long flags); + #ifdef CONFIG_DEBUG_FS void drm_gem_cma_describe(struct drm_gem_cma_object *obj, struct seq_file *m); #endif
Some SoC without MMU have display driver where a drm/kms driver could be implemented. Before doing such kind of thing drm/kms must allow to use mmuless devices. This patch propose to remove MMU configuration flag and add some cma helpers functions to help implementing mmuless display driver version 2: - define get_fb_unmapped_area() in drivers/video/fbdev/core/fbmem.c if HAVE_ARCH_FB_UNMAPPED_AREA is set and get_fb_unmapped_area not already defined in the architecture Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> --- drivers/gpu/drm/Kconfig | 4 +-- drivers/gpu/drm/drm_fb_cma_helper.c | 4 +++ drivers/gpu/drm/drm_gem_cma_helper.c | 62 ++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_vm.c | 4 +++ drivers/video/fbdev/core/fbmem.c | 15 +++++++++ include/drm/drm_gem_cma_helper.h | 6 ++++ 6 files changed, 93 insertions(+), 2 deletions(-)