* [RFC 0/1] DRM: allow to use mmuless devices @ 2016-11-30 11:21 Benjamin Gaignard 2016-11-30 11:21 ` [RFC 1/1] drm: allow to use mmuless SoC Benjamin Gaignard 0 siblings, 1 reply; 10+ messages in thread From: Benjamin Gaignard @ 2016-11-30 11:21 UTC (permalink / raw) To: dri-devel, airlied, daniel, robdclark, laurent.pinchart Cc: yannick.fertre, linaro-kernel, philippe.cornu Hello, The purpose of this RFC is to understand what is needed to allow to write drm/kms drivers for devices without MMU. There are some MCU platforms, like stm32f4, which don't have MMU but have hardware display IP where is it possible to implement a drm/kms driver. Obviously that start by removing MMU configuration flag from Kconfig. But while coding the driver for stm32f4 (on discovery board to have enough memory) we have already identify few other pieces of code that need to be change. We have been inspired by what already exist in v4l2 where using mmuless devices is possible. Since we have only use cma helpers we only have partial view of what could be needed for other part of drm/kms framework. Benjamin Gaignard (1): drm: allow to use mmuless SoC drivers/gpu/drm/Kconfig | 4 +-- drivers/gpu/drm/drm_fb_cma_helper.c | 20 ++++++++++++ drivers/gpu/drm/drm_gem_cma_helper.c | 62 ++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_vm.c | 4 +++ drivers/video/fbdev/core/fbmem.c | 8 +++++ include/drm/drm_gem_cma_helper.h | 6 ++++ 6 files changed, 102 insertions(+), 2 deletions(-) -- 1.9.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC 1/1] drm: allow to use mmuless SoC 2016-11-30 11:21 [RFC 0/1] DRM: allow to use mmuless devices Benjamin Gaignard @ 2016-11-30 11:21 ` Benjamin Gaignard 2016-11-30 11:35 ` Laurent Pinchart 2016-11-30 13:52 ` Daniel Vetter 0 siblings, 2 replies; 10+ messages in thread From: Benjamin Gaignard @ 2016-11-30 11:21 UTC (permalink / raw) To: dri-devel, airlied, daniel, robdclark, laurent.pinchart Cc: yannick.fertre, linaro-kernel, philippe.cornu Some platforms without MMU have display drivers where a drm/kms driver could be implemented. Before doing such kind of thing drm/kms must allow to use mmuless devices. This patch proposes to remove MMU configuration flag and add some cma helpers functions to help implementing mmuless display driver Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> --- drivers/gpu/drm/Kconfig | 4 +-- drivers/gpu/drm/drm_fb_cma_helper.c | 20 ++++++++++++ drivers/gpu/drm/drm_gem_cma_helper.c | 62 ++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_vm.c | 4 +++ drivers/video/fbdev/core/fbmem.c | 8 +++++ include/drm/drm_gem_cma_helper.h | 6 ++++ 6 files changed, 102 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..a3cef79 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -343,10 +343,30 @@ 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 } +#ifdef HAVE_ARCH_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; +} +EXPORT_SYMBOL_GPL(get_fb_unmapped_area); +#endif + static struct fb_ops drm_fbdev_cma_ops = { .owner = THIS_MODULE, DRM_FB_HELPER_DEFAULT_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..d6a83bd 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1492,6 +1492,14 @@ static long fb_compat_ioctl(struct file *file, unsigned int cmd, return 0; } +#ifdef HAVE_ARCH_FB_UNMAPPED_AREA +extern unsigned long get_fb_unmapped_area(struct file *filp, + unsigned long addr, + unsigned long len, + unsigned long pgoff, + unsigned long flags); +#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..79e9dbd 100644 --- a/include/drm/drm_gem_cma_helper.h +++ b/include/drm/drm_gem_cma_helper.h @@ -51,6 +51,12 @@ int drm_gem_cma_dumb_map_offset(struct drm_file *file_priv, struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, size_t size); +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp, + unsigned long addr, + unsigned long len, + unsigned long pgoff, + unsigned long flags); + extern const struct vm_operations_struct drm_gem_cma_vm_ops; #ifdef CONFIG_DEBUG_FS -- 1.9.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC 1/1] drm: allow to use mmuless SoC 2016-11-30 11:21 ` [RFC 1/1] drm: allow to use mmuless SoC Benjamin Gaignard @ 2016-11-30 11:35 ` Laurent Pinchart 2016-11-30 12:35 ` Benjamin Gaignard 2016-11-30 13:52 ` Daniel Vetter 1 sibling, 1 reply; 10+ messages in thread From: Laurent Pinchart @ 2016-11-30 11:35 UTC (permalink / raw) To: Benjamin Gaignard Cc: linaro-kernel, philippe.cornu, dri-devel, yannick.fertre, airlied Hi Benjamin, Thank you for the patch. On Wednesday 30 Nov 2016 12:21:24 Benjamin Gaignard wrote: > Some platforms without MMU have display drivers where a drm/kms driver > could be implemented. > Before doing such kind of thing drm/kms must allow to use mmuless > devices. > This patch proposes to remove MMU configuration flag and add some > cma helpers functions to help implementing mmuless display driver > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> > --- > drivers/gpu/drm/Kconfig | 4 +-- > drivers/gpu/drm/drm_fb_cma_helper.c | 20 ++++++++++++ > drivers/gpu/drm/drm_gem_cma_helper.c | 62 +++++++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_vm.c | 4 +++ I'll let someone more knowledgeable than me comment on these two files. > drivers/video/fbdev/core/fbmem.c | 8 +++++ > include/drm/drm_gem_cma_helper.h | 6 ++++ > 6 files changed, 102 insertions(+), 2 deletions(-) [snip] > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c > b/drivers/gpu/drm/drm_fb_cma_helper.c index 81b3558..a3cef79 100644 > --- a/drivers/gpu/drm/drm_fb_cma_helper.c > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c > @@ -343,10 +343,30 @@ 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 > } > > +#ifdef HAVE_ARCH_FB_UNMAPPED_AREA Shouldn't this be #ifndef ? When the symbol is defined the implementation is provided by the architecture. By the way, given that you provide a generic implementation here, what's the reason some architecture need to implement the function in a special way ? Couldn't we come up with a generic implementation across all architectures ? > +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; > +} > +EXPORT_SYMBOL_GPL(get_fb_unmapped_area); Shouldn't this be defined somewhere in drivers/video/ instead, as the code is fbdev-specific and only used in drivers/video/fbdev/core/fbmem.c ? > +#endif > + > static struct fb_ops drm_fbdev_cma_ops = { > .owner = THIS_MODULE, > DRM_FB_HELPER_DEFAULT_OPS, [snip] > diff --git a/drivers/video/fbdev/core/fbmem.c > b/drivers/video/fbdev/core/fbmem.c index 76c1ad9..d6a83bd 100644 > --- a/drivers/video/fbdev/core/fbmem.c > +++ b/drivers/video/fbdev/core/fbmem.c > @@ -1492,6 +1492,14 @@ static long fb_compat_ioctl(struct file *file, > unsigned int cmd, return 0; > } > > +#ifdef HAVE_ARCH_FB_UNMAPPED_AREA > +extern unsigned long get_fb_unmapped_area(struct file *filp, > + unsigned long addr, > + unsigned long len, > + unsigned long pgoff, > + unsigned long flags); > +#endif This should come from a header file. > 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..79e9dbd 100644 > --- a/include/drm/drm_gem_cma_helper.h > +++ b/include/drm/drm_gem_cma_helper.h > @@ -51,6 +51,12 @@ int drm_gem_cma_dumb_map_offset(struct drm_file > *file_priv, struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device > *drm, size_t size); > > +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp, > + unsigned long addr, > + unsigned long len, > + unsigned long pgoff, > + unsigned long flags); > + > extern const struct vm_operations_struct drm_gem_cma_vm_ops; > > #ifdef CONFIG_DEBUG_FS -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 1/1] drm: allow to use mmuless SoC 2016-11-30 11:35 ` Laurent Pinchart @ 2016-11-30 12:35 ` Benjamin Gaignard 0 siblings, 0 replies; 10+ messages in thread From: Benjamin Gaignard @ 2016-11-30 12:35 UTC (permalink / raw) To: Laurent Pinchart Cc: Linaro Kernel Mailman List, Philippe Cornu, dri-devel@lists.freedesktop.org, Yannick Fertre, Dave Airlie 2016-11-30 12:35 GMT+01:00 Laurent Pinchart <laurent.pinchart@ideasonboard.com>: > Hi Benjamin, > > Thank you for the patch. > > On Wednesday 30 Nov 2016 12:21:24 Benjamin Gaignard wrote: >> Some platforms without MMU have display drivers where a drm/kms driver >> could be implemented. >> Before doing such kind of thing drm/kms must allow to use mmuless >> devices. >> This patch proposes to remove MMU configuration flag and add some >> cma helpers functions to help implementing mmuless display driver >> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> >> --- >> drivers/gpu/drm/Kconfig | 4 +-- >> drivers/gpu/drm/drm_fb_cma_helper.c | 20 ++++++++++++ >> drivers/gpu/drm/drm_gem_cma_helper.c | 62 +++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/drm_vm.c | 4 +++ > > I'll let someone more knowledgeable than me comment on these two files. > >> drivers/video/fbdev/core/fbmem.c | 8 +++++ >> include/drm/drm_gem_cma_helper.h | 6 ++++ >> 6 files changed, 102 insertions(+), 2 deletions(-) > > [snip] > >> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c >> b/drivers/gpu/drm/drm_fb_cma_helper.c index 81b3558..a3cef79 100644 >> --- a/drivers/gpu/drm/drm_fb_cma_helper.c >> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c >> @@ -343,10 +343,30 @@ 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 >> } >> >> +#ifdef HAVE_ARCH_FB_UNMAPPED_AREA > > Shouldn't this be #ifndef ? When the symbol is defined the implementation is > provided by the architecture. > > By the way, given that you provide a generic implementation here, what's the > reason some architecture need to implement the function in a special way ? > Couldn't we come up with a generic implementation across all architectures ? > >> +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; >> +} >> +EXPORT_SYMBOL_GPL(get_fb_unmapped_area); > > Shouldn't this be defined somewhere in drivers/video/ instead, as the code is > fbdev-specific and only used in drivers/video/fbdev/core/fbmem.c ? > maybe I could migrate this code in drivers/video/fbdev/core/fbmem.c and play with HAVE_ARCH_FB_UNMAPPED_AREA and get_fb_unmapped_area to know if the symbol already exist or not. I think that would also fix your previous and next remark because I this case I will not have declare the prototype. >> +#endif >> + >> static struct fb_ops drm_fbdev_cma_ops = { >> .owner = THIS_MODULE, >> DRM_FB_HELPER_DEFAULT_OPS, > > [snip] > >> diff --git a/drivers/video/fbdev/core/fbmem.c >> b/drivers/video/fbdev/core/fbmem.c index 76c1ad9..d6a83bd 100644 >> --- a/drivers/video/fbdev/core/fbmem.c >> +++ b/drivers/video/fbdev/core/fbmem.c >> @@ -1492,6 +1492,14 @@ static long fb_compat_ioctl(struct file *file, >> unsigned int cmd, return 0; >> } >> >> +#ifdef HAVE_ARCH_FB_UNMAPPED_AREA >> +extern unsigned long get_fb_unmapped_area(struct file *filp, >> + unsigned long addr, >> + unsigned long len, >> + unsigned long pgoff, >> + unsigned long flags); >> +#endif > > This should come from a header file. > >> 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..79e9dbd 100644 >> --- a/include/drm/drm_gem_cma_helper.h >> +++ b/include/drm/drm_gem_cma_helper.h >> @@ -51,6 +51,12 @@ int drm_gem_cma_dumb_map_offset(struct drm_file >> *file_priv, struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device >> *drm, size_t size); >> >> +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp, >> + unsigned long addr, >> + unsigned long len, >> + unsigned long pgoff, >> + unsigned long flags); >> + >> extern const struct vm_operations_struct drm_gem_cma_vm_ops; >> >> #ifdef CONFIG_DEBUG_FS > > -- > Regards, > > Laurent Pinchart > -- Benjamin Gaignard Graphic Study Group Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 1/1] drm: allow to use mmuless SoC 2016-11-30 11:21 ` [RFC 1/1] drm: allow to use mmuless SoC Benjamin Gaignard 2016-11-30 11:35 ` Laurent Pinchart @ 2016-11-30 13:52 ` Daniel Vetter 2016-11-30 15:08 ` Benjamin Gaignard 1 sibling, 1 reply; 10+ messages in thread From: Daniel Vetter @ 2016-11-30 13:52 UTC (permalink / raw) To: Benjamin Gaignard Cc: linaro-kernel, philippe.cornu, dri-devel, yannick.fertre, laurent.pinchart, airlied On Wed, Nov 30, 2016 at 12:21:24PM +0100, Benjamin Gaignard wrote: > Some platforms without MMU have display drivers where a drm/kms driver > could be implemented. > Before doing such kind of thing drm/kms must allow to use mmuless > devices. > This patch proposes to remove MMU configuration flag and add some > cma helpers functions to help implementing mmuless display driver > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> > --- > drivers/gpu/drm/Kconfig | 4 +-- > drivers/gpu/drm/drm_fb_cma_helper.c | 20 ++++++++++++ > drivers/gpu/drm/drm_gem_cma_helper.c | 62 ++++++++++++++++++++++++++++++++++++ Hacking this into cma helpers feels very wrong to me. mmu-less looks like it works substantially differently enough that it warrants a new set of mmu-less gem helpers. And maybe that will showcase more code-refactoring opportunities between cma helpers, mmu-less helpers and shmem-based gem objects. And if there's none, then I think copypasting is better than ending up in a mess. > drivers/gpu/drm/drm_vm.c | 4 +++ > drivers/video/fbdev/core/fbmem.c | 8 +++++ > include/drm/drm_gem_cma_helper.h | 6 ++++ > 6 files changed, 102 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..a3cef79 100644 > --- a/drivers/gpu/drm/drm_fb_cma_helper.c > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c > @@ -343,10 +343,30 @@ 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 > } > > +#ifdef HAVE_ARCH_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; > +} > +EXPORT_SYMBOL_GPL(get_fb_unmapped_area); > +#endif > + > static struct fb_ops drm_fbdev_cma_ops = { > .owner = THIS_MODULE, > DRM_FB_HELPER_DEFAULT_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) { drm_vm.c is a legacy horror show. Instead of hacking even more garbage into this, can't we just not compile this for MMU-less platforms? A bunch of stubs in drm_internal.h is all that should be needed for this, since on MMU-less you should never be able to enable one of the legacy drivers which need the exported symbols from this file. Cheers, Daniel > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c > index 76c1ad9..d6a83bd 100644 > --- a/drivers/video/fbdev/core/fbmem.c > +++ b/drivers/video/fbdev/core/fbmem.c > @@ -1492,6 +1492,14 @@ static long fb_compat_ioctl(struct file *file, unsigned int cmd, > return 0; > } > > +#ifdef HAVE_ARCH_FB_UNMAPPED_AREA > +extern unsigned long get_fb_unmapped_area(struct file *filp, > + unsigned long addr, > + unsigned long len, > + unsigned long pgoff, > + unsigned long flags); > +#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..79e9dbd 100644 > --- a/include/drm/drm_gem_cma_helper.h > +++ b/include/drm/drm_gem_cma_helper.h > @@ -51,6 +51,12 @@ int drm_gem_cma_dumb_map_offset(struct drm_file *file_priv, > struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, > size_t size); > > +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp, > + unsigned long addr, > + unsigned long len, > + unsigned long pgoff, > + unsigned long flags); > + > extern const struct vm_operations_struct drm_gem_cma_vm_ops; > > #ifdef CONFIG_DEBUG_FS > -- > 1.9.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 1/1] drm: allow to use mmuless SoC 2016-11-30 13:52 ` Daniel Vetter @ 2016-11-30 15:08 ` Benjamin Gaignard 2016-11-30 15:19 ` Laurent Pinchart 0 siblings, 1 reply; 10+ messages in thread From: Benjamin Gaignard @ 2016-11-30 15:08 UTC (permalink / raw) To: Daniel Vetter Cc: Linaro Kernel Mailman List, Philippe Cornu, dri-devel@lists.freedesktop.org, Yannick Fertre, Laurent Pinchart, Dave Airlie 2016-11-30 14:52 GMT+01:00 Daniel Vetter <daniel@ffwll.ch>: > On Wed, Nov 30, 2016 at 12:21:24PM +0100, Benjamin Gaignard wrote: >> Some platforms without MMU have display drivers where a drm/kms driver >> could be implemented. >> Before doing such kind of thing drm/kms must allow to use mmuless >> devices. >> This patch proposes to remove MMU configuration flag and add some >> cma helpers functions to help implementing mmuless display driver >> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> >> --- >> drivers/gpu/drm/Kconfig | 4 +-- >> drivers/gpu/drm/drm_fb_cma_helper.c | 20 ++++++++++++ >> drivers/gpu/drm/drm_gem_cma_helper.c | 62 ++++++++++++++++++++++++++++++++++++ > > Hacking this into cma helpers feels very wrong to me. mmu-less looks like > it works substantially differently enough that it warrants a new set of > mmu-less gem helpers. And maybe that will showcase more code-refactoring > opportunities between cma helpers, mmu-less helpers and shmem-based gem > objects. And if there's none, then I think copypasting is better than > ending up in a mess. Only the 2 calls to dma_mmap_wc() are problematic within ARM nommu architecture. After Laurent remarks I have move get_fb_unmapped_area() to fbmem.c it really limits the modifications in cma helpers... > >> drivers/gpu/drm/drm_vm.c | 4 +++ >> drivers/video/fbdev/core/fbmem.c | 8 +++++ >> include/drm/drm_gem_cma_helper.h | 6 ++++ >> 6 files changed, 102 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..a3cef79 100644 >> --- a/drivers/gpu/drm/drm_fb_cma_helper.c >> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c >> @@ -343,10 +343,30 @@ 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 >> } >> >> +#ifdef HAVE_ARCH_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; >> +} >> +EXPORT_SYMBOL_GPL(get_fb_unmapped_area); >> +#endif >> + >> static struct fb_ops drm_fbdev_cma_ops = { >> .owner = THIS_MODULE, >> DRM_FB_HELPER_DEFAULT_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) { > > drm_vm.c is a legacy horror show. Instead of hacking even more garbage > into this, can't we just not compile this for MMU-less platforms? A bunch > of stubs in drm_internal.h is all that should be needed for this, since on > MMU-less you should never be able to enable one of the legacy drivers > which need the exported symbols from this file. Following your advice I have removed drm_vm.c from the build if CONFIG_MMU is not set I only had to stub drm_legacy_vma_flush() to compile. I will include that in v3 > > Cheers, Daniel > >> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c >> index 76c1ad9..d6a83bd 100644 >> --- a/drivers/video/fbdev/core/fbmem.c >> +++ b/drivers/video/fbdev/core/fbmem.c >> @@ -1492,6 +1492,14 @@ static long fb_compat_ioctl(struct file *file, unsigned int cmd, >> return 0; >> } >> >> +#ifdef HAVE_ARCH_FB_UNMAPPED_AREA >> +extern unsigned long get_fb_unmapped_area(struct file *filp, >> + unsigned long addr, >> + unsigned long len, >> + unsigned long pgoff, >> + unsigned long flags); >> +#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..79e9dbd 100644 >> --- a/include/drm/drm_gem_cma_helper.h >> +++ b/include/drm/drm_gem_cma_helper.h >> @@ -51,6 +51,12 @@ int drm_gem_cma_dumb_map_offset(struct drm_file *file_priv, >> struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, >> size_t size); >> >> +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp, >> + unsigned long addr, >> + unsigned long len, >> + unsigned long pgoff, >> + unsigned long flags); >> + >> extern const struct vm_operations_struct drm_gem_cma_vm_ops; >> >> #ifdef CONFIG_DEBUG_FS >> -- >> 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 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 1/1] drm: allow to use mmuless SoC 2016-11-30 15:08 ` Benjamin Gaignard @ 2016-11-30 15:19 ` Laurent Pinchart 2016-11-30 15:34 ` Benjamin Gaignard 0 siblings, 1 reply; 10+ messages in thread From: Laurent Pinchart @ 2016-11-30 15:19 UTC (permalink / raw) To: dri-devel Cc: Linaro Kernel Mailman List, Philippe Cornu, Yannick Fertre, Dave Airlie Hi Benjamin, On Wednesday 30 Nov 2016 16:08:23 Benjamin Gaignard wrote: > 2016-11-30 14:52 GMT+01:00 Daniel Vetter <daniel@ffwll.ch>: > > On Wed, Nov 30, 2016 at 12:21:24PM +0100, Benjamin Gaignard wrote: > >> Some platforms without MMU have display drivers where a drm/kms driver > >> could be implemented. > >> Before doing such kind of thing drm/kms must allow to use mmuless > >> devices. > >> This patch proposes to remove MMU configuration flag and add some > >> cma helpers functions to help implementing mmuless display driver > >> > >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> > >> --- > >> > >> drivers/gpu/drm/Kconfig | 4 +-- > >> drivers/gpu/drm/drm_fb_cma_helper.c | 20 ++++++++++++ > >> drivers/gpu/drm/drm_gem_cma_helper.c | 62 +++++++++++++++++++++++++++++ [snip] > > drm_vm.c is a legacy horror show. Instead of hacking even more garbage > > into this, can't we just not compile this for MMU-less platforms? A bunch > > of stubs in drm_internal.h is all that should be needed for this, since on > > MMU-less you should never be able to enable one of the legacy drivers > > which need the exported symbols from this file. > > Following your advice I have removed drm_vm.c from the build if > CONFIG_MMU is not set How about only including it if !DRM_LEGACY && !DRM_NOUVEAU ? > I only had to stub drm_legacy_vma_flush() to compile. > I will include that in v3 -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 1/1] drm: allow to use mmuless SoC 2016-11-30 15:19 ` Laurent Pinchart @ 2016-11-30 15:34 ` Benjamin Gaignard 2016-11-30 15:36 ` Laurent Pinchart 0 siblings, 1 reply; 10+ messages in thread From: Benjamin Gaignard @ 2016-11-30 15:34 UTC (permalink / raw) To: Laurent Pinchart Cc: Linaro Kernel Mailman List, Philippe Cornu, dri-devel@lists.freedesktop.org, Yannick Fertre, Dave Airlie 2016-11-30 16:19 GMT+01:00 Laurent Pinchart <laurent.pinchart@ideasonboard.com>: > Hi Benjamin, > > On Wednesday 30 Nov 2016 16:08:23 Benjamin Gaignard wrote: >> 2016-11-30 14:52 GMT+01:00 Daniel Vetter <daniel@ffwll.ch>: >> > On Wed, Nov 30, 2016 at 12:21:24PM +0100, Benjamin Gaignard wrote: >> >> Some platforms without MMU have display drivers where a drm/kms driver >> >> could be implemented. >> >> Before doing such kind of thing drm/kms must allow to use mmuless >> >> devices. >> >> This patch proposes to remove MMU configuration flag and add some >> >> cma helpers functions to help implementing mmuless display driver >> >> >> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> >> >> --- >> >> >> >> drivers/gpu/drm/Kconfig | 4 +-- >> >> drivers/gpu/drm/drm_fb_cma_helper.c | 20 ++++++++++++ >> >> drivers/gpu/drm/drm_gem_cma_helper.c | 62 +++++++++++++++++++++++++++++ > > [snip] > >> > drm_vm.c is a legacy horror show. Instead of hacking even more garbage >> > into this, can't we just not compile this for MMU-less platforms? A bunch >> > of stubs in drm_internal.h is all that should be needed for this, since on >> > MMU-less you should never be able to enable one of the legacy drivers >> > which need the exported symbols from this file. >> >> Following your advice I have removed drm_vm.c from the build if >> CONFIG_MMU is not set > > How about only including it if !DRM_LEGACY && !DRM_NOUVEAU ? I don't understand the link between !DRM_LEGACY && !DRM_NOUVEAU and MMU... By chance would you mean including only if DRM_LEGACY && DRM_NOUVEAU ? > >> I only had to stub drm_legacy_vma_flush() to compile. >> I will include that in v3 > > -- > Regards, > > Laurent Pinchart > -- Benjamin Gaignard Graphic Study Group Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 1/1] drm: allow to use mmuless SoC 2016-11-30 15:34 ` Benjamin Gaignard @ 2016-11-30 15:36 ` Laurent Pinchart 2016-11-30 15:56 ` Daniel Vetter 0 siblings, 1 reply; 10+ messages in thread From: Laurent Pinchart @ 2016-11-30 15:36 UTC (permalink / raw) To: Benjamin Gaignard Cc: Linaro Kernel Mailman List, Philippe Cornu, dri-devel@lists.freedesktop.org, Yannick Fertre, Dave Airlie Hi Benjamin, On Wednesday 30 Nov 2016 16:34:37 Benjamin Gaignard wrote: > 2016-11-30 16:19 GMT+01:00 Laurent Pinchart: > > On Wednesday 30 Nov 2016 16:08:23 Benjamin Gaignard wrote: > >> 2016-11-30 14:52 GMT+01:00 Daniel Vetter <daniel@ffwll.ch>: > >>> On Wed, Nov 30, 2016 at 12:21:24PM +0100, Benjamin Gaignard wrote: > >>>> Some platforms without MMU have display drivers where a drm/kms driver > >>>> could be implemented. > >>>> Before doing such kind of thing drm/kms must allow to use mmuless > >>>> devices. > >>>> This patch proposes to remove MMU configuration flag and add some > >>>> cma helpers functions to help implementing mmuless display driver > >>>> > >>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> > >>>> --- > >>>> > >>>> drivers/gpu/drm/Kconfig | 4 +-- > >>>> drivers/gpu/drm/drm_fb_cma_helper.c | 20 ++++++++++++ > >>>> drivers/gpu/drm/drm_gem_cma_helper.c | 62 +++++++++++++++++++++++++++ > > > > [snip] > > > >>> drm_vm.c is a legacy horror show. Instead of hacking even more garbage > >>> into this, can't we just not compile this for MMU-less platforms? A > >>> bunch of stubs in drm_internal.h is all that should be needed for this, > >>> since on MMU-less you should never be able to enable one of the legacy > >>> drivers which need the exported symbols from this file. > >> > >> Following your advice I have removed drm_vm.c from the build if > >> CONFIG_MMU is not set > > > > How about only including it if !DRM_LEGACY && !DRM_NOUVEAU ? > > I don't understand the link between !DRM_LEGACY && !DRM_NOUVEAU and MMU... > > By chance would you mean including only if DRM_LEGACY && DRM_NOUVEAU ? I meant excluding it if !DRM_LEGACY && !DRM_NOUVEAU, sorry, so including it if DRM_LEGACY || DRM_NOUVEAU. (Probably out of scope for this patch series, but why do we need it in nouveau ?) > >> I only had to stub drm_legacy_vma_flush() to compile. > >> I will include that in v3 -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 1/1] drm: allow to use mmuless SoC 2016-11-30 15:36 ` Laurent Pinchart @ 2016-11-30 15:56 ` Daniel Vetter 0 siblings, 0 replies; 10+ messages in thread From: Daniel Vetter @ 2016-11-30 15:56 UTC (permalink / raw) To: Laurent Pinchart Cc: Linaro Kernel Mailman List, Philippe Cornu, dri-devel@lists.freedesktop.org, Yannick Fertre, Dave Airlie On Wed, Nov 30, 2016 at 05:36:46PM +0200, Laurent Pinchart wrote: > Hi Benjamin, > > On Wednesday 30 Nov 2016 16:34:37 Benjamin Gaignard wrote: > > 2016-11-30 16:19 GMT+01:00 Laurent Pinchart: > > > On Wednesday 30 Nov 2016 16:08:23 Benjamin Gaignard wrote: > > >> 2016-11-30 14:52 GMT+01:00 Daniel Vetter <daniel@ffwll.ch>: > > >>> On Wed, Nov 30, 2016 at 12:21:24PM +0100, Benjamin Gaignard wrote: > > >>>> Some platforms without MMU have display drivers where a drm/kms driver > > >>>> could be implemented. > > >>>> Before doing such kind of thing drm/kms must allow to use mmuless > > >>>> devices. > > >>>> This patch proposes to remove MMU configuration flag and add some > > >>>> cma helpers functions to help implementing mmuless display driver > > >>>> > > >>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> > > >>>> --- > > >>>> > > >>>> drivers/gpu/drm/Kconfig | 4 +-- > > >>>> drivers/gpu/drm/drm_fb_cma_helper.c | 20 ++++++++++++ > > >>>> drivers/gpu/drm/drm_gem_cma_helper.c | 62 +++++++++++++++++++++++++++ > > > > > > [snip] > > > > > >>> drm_vm.c is a legacy horror show. Instead of hacking even more garbage > > >>> into this, can't we just not compile this for MMU-less platforms? A > > >>> bunch of stubs in drm_internal.h is all that should be needed for this, > > >>> since on MMU-less you should never be able to enable one of the legacy > > >>> drivers which need the exported symbols from this file. > > >> > > >> Following your advice I have removed drm_vm.c from the build if > > >> CONFIG_MMU is not set > > > > > > How about only including it if !DRM_LEGACY && !DRM_NOUVEAU ? > > > > I don't understand the link between !DRM_LEGACY && !DRM_NOUVEAU and MMU... > > > > By chance would you mean including only if DRM_LEGACY && DRM_NOUVEAU ? > > I meant excluding it if !DRM_LEGACY && !DRM_NOUVEAU, sorry, so including it if > DRM_LEGACY || DRM_NOUVEAU. > > (Probably out of scope for this patch series, but why do we need it in nouveau > ?) Don't ask, but you did: The very first version of the nouveau kms ddx was a partial conversion from ums to kms, and still create some of the old garabge. Among them a ctx and a few other things. Of course it was never used, but the init code was overlooked, and it checks for errors. Which means we can't just fail. The other problem is that rhel shipped that version of nouveau, which means we need to wait 10 years or so until we can nuke it all :( -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-11-30 15:56 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-11-30 11:21 [RFC 0/1] DRM: allow to use mmuless devices Benjamin Gaignard 2016-11-30 11:21 ` [RFC 1/1] drm: allow to use mmuless SoC Benjamin Gaignard 2016-11-30 11:35 ` Laurent Pinchart 2016-11-30 12:35 ` Benjamin Gaignard 2016-11-30 13:52 ` Daniel Vetter 2016-11-30 15:08 ` Benjamin Gaignard 2016-11-30 15:19 ` Laurent Pinchart 2016-11-30 15:34 ` Benjamin Gaignard 2016-11-30 15:36 ` Laurent Pinchart 2016-11-30 15:56 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).