All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/4] DRM: allow to use mmuless devices
@ 2016-12-07 10:06 Benjamin Gaignard
  2016-12-07 10:06 ` [PATCH v4 1/4] nommu: allow mmap when !CONFIG_MMU Benjamin Gaignard
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Benjamin Gaignard @ 2016-12-07 10:06 UTC (permalink / raw)
  To: dri-devel, airlied, daniel, laurent.pinchart, michel
  Cc: yannick.fertre, linaro-kernel, philippe.cornu

version 4:
- add documentation about drm_gem_cma_get_unmapped_area()
- introduce FB_PROVIDE_GET_FB_UNMAPPED_AREA configuration flag for get_fb_unmapped_area()
- I have also send the first patch (https://lkml.org/lkml/2016/12/1/308) to make dma_mmap_wc
  works on ARM noMMU platform, assuming that is patch will be accepted there is no more
  needs to call vm_ioremap in cma helpers.

version 3:
- split the original patch in 3 parts, it should be more easy to review like
  this.
- duplicate drm_fb_cma_helper.c and drm_gem_cma_helper.c into nommu version
- add a configuration flag for drm_vm.c 

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 (4):
  nommu: allow mmap when !CONFIG_MMU
  fbmem: add a default get_fb_unmapped_area function
  drm: compile drm_vm.c only when needed
  drm: allow to use mmuless SoC

 Documentation/gpu/drm-mm.rst         | 11 ++++++
 arch/arm/mm/dma-mapping.c            |  3 ++
 drivers/gpu/drm/Kconfig              |  9 +++--
 drivers/gpu/drm/Makefile             |  3 +-
 drivers/gpu/drm/drm_gem_cma_helper.c | 69 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_legacy.h         |  7 ++++
 drivers/gpu/drm/nouveau/Kconfig      |  1 +
 drivers/video/fbdev/Kconfig          |  8 +++++
 drivers/video/fbdev/core/fbmem.c     | 15 ++++++++
 include/drm/drm_gem_cma_helper.h     | 17 +++++++++
 10 files changed, 140 insertions(+), 3 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] 17+ messages in thread

* [PATCH v4 1/4] nommu: allow mmap when !CONFIG_MMU
  2016-12-07 10:06 [PATCH v4 0/4] DRM: allow to use mmuless devices Benjamin Gaignard
@ 2016-12-07 10:06 ` Benjamin Gaignard
  2016-12-07 10:06 ` [PATCH v4 2/4] fbmem: add a default get_fb_unmapped_area function Benjamin Gaignard
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 17+ messages in thread
From: Benjamin Gaignard @ 2016-12-07 10:06 UTC (permalink / raw)
  To: dri-devel, airlied, daniel, laurent.pinchart, michel
  Cc: linaro-kernel, arnd, Catalin Marinas, philippe.cornu,
	yannick.fertre

commit ab6494f0c96f ("nommu: Add noMMU support to the DMA API") have
add CONFIG_MMU compilation flag but that prohibit to use dma_mmap_wc()
when the platform doesn't have MMU.

This patch call vm_iomap_memory() in noMMU case to test if addresses
are correct and set wma->vm_flags rather than all return an error.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: arnd@arndb.de
---
 arch/arm/mm/dma-mapping.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index ab4f745..230875e 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -868,6 +868,9 @@ static int __arm_dma_mmap(struct device *dev, struct vm_area_struct *vma,
 				      vma->vm_end - vma->vm_start,
 				      vma->vm_page_prot);
 	}
+#else
+	ret = vm_iomap_memory(vma, vma->vm_start,
+			      (vma->vm_end - vma->vm_start));
 #endif	/* CONFIG_MMU */
 
 	return ret;
-- 
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] 17+ messages in thread

* [PATCH v4 2/4] fbmem: add a default get_fb_unmapped_area function
  2016-12-07 10:06 [PATCH v4 0/4] DRM: allow to use mmuless devices Benjamin Gaignard
  2016-12-07 10:06 ` [PATCH v4 1/4] nommu: allow mmap when !CONFIG_MMU Benjamin Gaignard
@ 2016-12-07 10:06 ` Benjamin Gaignard
  2016-12-07 14:35   ` Laurent Pinchart
  2016-12-07 10:06 ` [PATCH v4 3/4] drm: compile drm_vm.c only when needed Benjamin Gaignard
  2016-12-07 10:06 ` [PATCH v4 4/4] drm: allow to use mmuless SoC Benjamin Gaignard
  3 siblings, 1 reply; 17+ messages in thread
From: Benjamin Gaignard @ 2016-12-07 10:06 UTC (permalink / raw)
  To: dri-devel, airlied, daniel, laurent.pinchart, michel
  Cc: yannick.fertre, linaro-kernel, philippe.cornu

Allow generic frame-buffer to provide a default
get_fb_unmapped_area function if specific devices need it.

Usually this function is defined in architecture directories but
define it here may limit code duplication especially for all ARM
platforms without MMU.

version 4:
- introdude a configuration flag to be independent of architecture

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 drivers/video/fbdev/Kconfig      |  8 ++++++++
 drivers/video/fbdev/core/fbmem.c | 15 +++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 5d3b0db..922e4ea 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -138,6 +138,14 @@ config FB_SYS_IMAGEBLIT
 	  blitting. This is used by drivers that don't provide their own
 	  (accelerated) version and the framebuffer is in system RAM.
 
+config FB_PROVIDE_GET_FB_UNMAPPED_AREA
+	bool
+	depends on FB
+	default n
+	---help---
+	  Allow generic frame-buffer to provide get_fb_unmapped_area
+	  function.
+
 menuconfig FB_FOREIGN_ENDIAN
 	bool "Framebuffer foreign endianness support"
 	depends on FB
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 76c1ad9..22321a2 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;
 }
 
+#ifdef CONFIG_FB_PROVIDE_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,
-- 
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] 17+ messages in thread

* [PATCH v4 3/4] drm: compile drm_vm.c only when needed
  2016-12-07 10:06 [PATCH v4 0/4] DRM: allow to use mmuless devices Benjamin Gaignard
  2016-12-07 10:06 ` [PATCH v4 1/4] nommu: allow mmap when !CONFIG_MMU Benjamin Gaignard
  2016-12-07 10:06 ` [PATCH v4 2/4] fbmem: add a default get_fb_unmapped_area function Benjamin Gaignard
@ 2016-12-07 10:06 ` Benjamin Gaignard
  2016-12-07 14:34   ` Laurent Pinchart
  2016-12-07 10:06 ` [PATCH v4 4/4] drm: allow to use mmuless SoC Benjamin Gaignard
  3 siblings, 1 reply; 17+ messages in thread
From: Benjamin Gaignard @ 2016-12-07 10:06 UTC (permalink / raw)
  To: dri-devel, airlied, daniel, laurent.pinchart, michel
  Cc: yannick.fertre, linaro-kernel, philippe.cornu

drm_vm.c functions are only need for DRM_LEGACY and DRM_NOUVEAU.
Use a new DRM_VM to define when drm_vm.c in needed.

stub drm_legacy_vma_flush() to avoid compilation issues

version 4:
- a "config DRM_VM" in Kconfig

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 drivers/gpu/drm/Kconfig         | 5 +++++
 drivers/gpu/drm/Makefile        | 3 ++-
 drivers/gpu/drm/drm_legacy.h    | 7 +++++++
 drivers/gpu/drm/nouveau/Kconfig | 1 +
 4 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 8d9cf73..83ac815 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -121,6 +121,10 @@ config DRM_KMS_CMA_HELPER
 	help
 	  Choose this if you need the KMS CMA helper functions
 
+config DRM_VM
+	bool
+	depends on DRM
+
 source "drivers/gpu/drm/i2c/Kconfig"
 
 source "drivers/gpu/drm/arm/Kconfig"
@@ -246,6 +250,7 @@ source "drivers/gpu/drm/zte/Kconfig"
 menuconfig DRM_LEGACY
 	bool "Enable legacy drivers (DANGEROUS)"
 	depends on DRM
+	select DRM_VM
 	help
 	  Enable legacy DRI1 drivers. Those drivers expose unsafe and dangerous
 	  APIs to user-space, which can be used to circumvent access
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index e10e935..5b73b16 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -5,7 +5,7 @@
 drm-y       :=	drm_auth.o drm_bufs.o drm_cache.o \
 		drm_context.o drm_dma.o \
 		drm_fops.o drm_gem.o drm_ioctl.o drm_irq.o \
-		drm_lock.o drm_memory.o drm_drv.o drm_vm.o \
+		drm_lock.o drm_memory.o drm_drv.o \
 		drm_scatter.o drm_pci.o \
 		drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
 		drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \
@@ -18,6 +18,7 @@ drm-y       :=	drm_auth.o drm_bufs.o drm_cache.o \
 		drm_plane.o drm_color_mgmt.o drm_print.o \
 		drm_dumb_buffers.o drm_mode_config.o
 
+drm-$(CONFIG_DRM_VM) += drm_vm.o
 drm-$(CONFIG_COMPAT) += drm_ioc32.o
 drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
 drm-$(CONFIG_PCI) += ati_pcigart.o
diff --git a/drivers/gpu/drm/drm_legacy.h b/drivers/gpu/drm/drm_legacy.h
index c6f422e..e4bb5ad 100644
--- a/drivers/gpu/drm/drm_legacy.h
+++ b/drivers/gpu/drm/drm_legacy.h
@@ -74,7 +74,14 @@ int drm_legacy_getmap_ioctl(struct drm_device *dev, void *data,
 int drm_legacy_mapbufs(struct drm_device *d, void *v, struct drm_file *f);
 int drm_legacy_dma_ioctl(struct drm_device *d, void *v, struct drm_file *f);
 
+#ifdef CONFIG_DRM_VM
 void drm_legacy_vma_flush(struct drm_device *d);
+#else
+static inline void drm_legacy_vma_flush(struct drm_device *d)
+{
+	/* do nothing */
+}
+#endif
 
 /*
  * AGP Support
diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index 2922a82..0f2f0af 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -16,6 +16,7 @@ config DRM_NOUVEAU
 	select INPUT if ACPI && X86
 	select THERMAL if ACPI && X86
 	select ACPI_VIDEO if ACPI && X86
+	select DRM_VM
 	help
 	  Choose this option for open-source NVIDIA support.
 
-- 
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] 17+ messages in thread

* [PATCH v4 4/4] drm: allow to use mmuless SoC
  2016-12-07 10:06 [PATCH v4 0/4] DRM: allow to use mmuless devices Benjamin Gaignard
                   ` (2 preceding siblings ...)
  2016-12-07 10:06 ` [PATCH v4 3/4] drm: compile drm_vm.c only when needed Benjamin Gaignard
@ 2016-12-07 10:06 ` Benjamin Gaignard
  2016-12-07 10:27   ` Daniel Vetter
                     ` (2 more replies)
  3 siblings, 3 replies; 17+ messages in thread
From: Benjamin Gaignard @ 2016-12-07 10:06 UTC (permalink / raw)
  To: dri-devel, airlied, daniel, laurent.pinchart, michel
  Cc: yannick.fertre, linaro-kernel, philippe.cornu

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 a cma helper
function to help implementing mmuless display driver

version 4:
- add documentation about drm_gem_cma_get_unmapped_area()
- stub it MMU case

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 Documentation/gpu/drm-mm.rst         | 11 ++++++
 drivers/gpu/drm/Kconfig              |  4 +--
 drivers/gpu/drm/drm_gem_cma_helper.c | 69 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_gem_cma_helper.h     | 17 +++++++++
 4 files changed, 99 insertions(+), 2 deletions(-)

diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index bca8085..6e7efab 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -303,6 +303,17 @@ created.
 Drivers that want to map the GEM object upfront instead of handling page
 faults can implement their own mmap file operation handler.
 
+For platforms without MMU GEM care provides a helper method
+:c:func:`drm_gem_cma_get_unmapped_area`. The mmap() routines will call
+this to get a proposed address for the mapping.
+
+To use :c:func:`drm_gem_cma_get_unmapped_area`, drivers must fill the
+struct :c:type:`struct file_operations <file_operations>` get_unmapped_area
+field with a pointer on :c:func:`drm_gem_cma_get_unmapped_area`.
+
+More detailed information about get_unmapped_area can be found in
+Documentation/nommu-mmap.txt
+
 Memory Coherency
 ----------------
 
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 83ac815..0eae4ad 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_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index 1d6c335..01016b1 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -358,6 +358,75 @@ int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma)
 }
 EXPORT_SYMBOL_GPL(drm_gem_cma_mmap);
 
+#ifndef CONFIG_MMU
+/**
+ * drm_gem_cma_get_unmapped_area - propose address for mapping in noMMU cases
+ * @filp: file object
+ * @addr: memory address
+ * @len: buffer size
+ * @pgoff: page offset
+ * @flags: memory flags
+ *
+ * This function is used in noMMU platforms to propose address mapping
+ * for a given buffer.
+ *
+ * Returns:
+ * mapping address on success or a negative error code on failure
+ */
+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;
+}
+EXPORT_SYMBOL_GPL(drm_gem_cma_get_unmapped_area);
+#endif
+
 #ifdef CONFIG_DEBUG_FS
 /**
  * drm_gem_cma_describe - describe a CMA GEM object for debugfs
diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h
index acd6af8..180b586 100644
--- a/include/drm/drm_gem_cma_helper.h
+++ b/include/drm/drm_gem_cma_helper.h
@@ -53,6 +53,23 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
 
 extern const struct vm_operations_struct drm_gem_cma_vm_ops;
 
+#ifndef CONFIG_MMU
+unsigned long drm_gem_cma_get_unmapped_area(struct file *filp,
+					    unsigned long addr,
+					    unsigned long len,
+					    unsigned long pgoff,
+					    unsigned long flags);
+#else
+unsigned long drm_gem_cma_get_unmapped_area(struct file *filp,
+					    unsigned long addr,
+					    unsigned long len,
+					    unsigned long pgoff,
+					    unsigned long flags)
+{
+	return -EINVAL;
+}
+#endif
+
 #ifdef CONFIG_DEBUG_FS
 void drm_gem_cma_describe(struct drm_gem_cma_object *obj, struct seq_file *m);
 #endif
-- 
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] 17+ messages in thread

* Re: [PATCH v4 4/4] drm: allow to use mmuless SoC
  2016-12-07 10:06 ` [PATCH v4 4/4] drm: allow to use mmuless SoC Benjamin Gaignard
@ 2016-12-07 10:27   ` Daniel Vetter
  2016-12-07 10:42     ` Benjamin Gaignard
  2016-12-07 14:32   ` Laurent Pinchart
  2016-12-07 15:19   ` Benjamin Gaignard
  2 siblings, 1 reply; 17+ messages in thread
From: Daniel Vetter @ 2016-12-07 10:27 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: linaro-kernel, michel, philippe.cornu, dri-devel, yannick.fertre,
	laurent.pinchart, airlied

On Wed, Dec 07, 2016 at 11:06:51AM +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 a cma helper
> function to help implementing mmuless display driver
> 
> version 4:
> - add documentation about drm_gem_cma_get_unmapped_area()
> - stub it MMU case
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>

Shouldn't we also update DRM_FB_HELPER_DEFAULT_OPS? With that addressed
this all looks good to me.
-Daniel

> ---
>  Documentation/gpu/drm-mm.rst         | 11 ++++++
>  drivers/gpu/drm/Kconfig              |  4 +--
>  drivers/gpu/drm/drm_gem_cma_helper.c | 69 ++++++++++++++++++++++++++++++++++++
>  include/drm/drm_gem_cma_helper.h     | 17 +++++++++
>  4 files changed, 99 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> index bca8085..6e7efab 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -303,6 +303,17 @@ created.
>  Drivers that want to map the GEM object upfront instead of handling page
>  faults can implement their own mmap file operation handler.
>  
> +For platforms without MMU GEM care provides a helper method
> +:c:func:`drm_gem_cma_get_unmapped_area`. The mmap() routines will call
> +this to get a proposed address for the mapping.
> +
> +To use :c:func:`drm_gem_cma_get_unmapped_area`, drivers must fill the
> +struct :c:type:`struct file_operations <file_operations>` get_unmapped_area
> +field with a pointer on :c:func:`drm_gem_cma_get_unmapped_area`.
> +
> +More detailed information about get_unmapped_area can be found in
> +Documentation/nommu-mmap.txt
> +
>  Memory Coherency
>  ----------------
>  
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 83ac815..0eae4ad 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_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
> index 1d6c335..01016b1 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -358,6 +358,75 @@ int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma)
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_cma_mmap);
>  
> +#ifndef CONFIG_MMU
> +/**
> + * drm_gem_cma_get_unmapped_area - propose address for mapping in noMMU cases
> + * @filp: file object
> + * @addr: memory address
> + * @len: buffer size
> + * @pgoff: page offset
> + * @flags: memory flags
> + *
> + * This function is used in noMMU platforms to propose address mapping
> + * for a given buffer.
> + *
> + * Returns:
> + * mapping address on success or a negative error code on failure
> + */
> +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;
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_cma_get_unmapped_area);
> +#endif
> +
>  #ifdef CONFIG_DEBUG_FS
>  /**
>   * drm_gem_cma_describe - describe a CMA GEM object for debugfs
> diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h
> index acd6af8..180b586 100644
> --- a/include/drm/drm_gem_cma_helper.h
> +++ b/include/drm/drm_gem_cma_helper.h
> @@ -53,6 +53,23 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
>  
>  extern const struct vm_operations_struct drm_gem_cma_vm_ops;
>  
> +#ifndef CONFIG_MMU
> +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp,
> +					    unsigned long addr,
> +					    unsigned long len,
> +					    unsigned long pgoff,
> +					    unsigned long flags);
> +#else
> +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp,
> +					    unsigned long addr,
> +					    unsigned long len,
> +					    unsigned long pgoff,
> +					    unsigned long flags)
> +{
> +	return -EINVAL;
> +}
> +#endif
> +
>  #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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 4/4] drm: allow to use mmuless SoC
  2016-12-07 10:27   ` Daniel Vetter
@ 2016-12-07 10:42     ` Benjamin Gaignard
  0 siblings, 0 replies; 17+ messages in thread
From: Benjamin Gaignard @ 2016-12-07 10:42 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Linaro Kernel Mailman List, Michel Dänzer, Philippe Cornu,
	dri-devel@lists.freedesktop.org, Yannick Fertre, Laurent Pinchart,
	Dave Airlie

2016-12-07 11:27 GMT+01:00 Daniel Vetter <daniel@ffwll.ch>:
> On Wed, Dec 07, 2016 at 11:06:51AM +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 a cma helper
>> function to help implementing mmuless display driver
>>
>> version 4:
>> - add documentation about drm_gem_cma_get_unmapped_area()
>> - stub it MMU case
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>
> Shouldn't we also update DRM_FB_HELPER_DEFAULT_OPS? With that addressed
> this all looks good to me.

Those changes shouldn't impact fb_ops but only fbdev file_operations
like it is done in fbmem patch

> -Daniel
>
[snip]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 4/4] drm: allow to use mmuless SoC
  2016-12-07 10:06 ` [PATCH v4 4/4] drm: allow to use mmuless SoC Benjamin Gaignard
  2016-12-07 10:27   ` Daniel Vetter
@ 2016-12-07 14:32   ` Laurent Pinchart
  2016-12-07 14:49     ` Daniel Vetter
  2016-12-07 15:19   ` Benjamin Gaignard
  2 siblings, 1 reply; 17+ messages in thread
From: Laurent Pinchart @ 2016-12-07 14:32 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: linaro-kernel, michel, philippe.cornu, dri-devel, yannick.fertre,
	airlied

Hi Benjamin,

Thank you for the patch.

On Wednesday 07 Dec 2016 11:06:51 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 a cma helper
> function to help implementing mmuless display driver
> 
> version 4:
> - add documentation about drm_gem_cma_get_unmapped_area()
> - stub it MMU case
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> ---
>  Documentation/gpu/drm-mm.rst         | 11 ++++++
>  drivers/gpu/drm/Kconfig              |  4 +--
>  drivers/gpu/drm/drm_gem_cma_helper.c | 69 +++++++++++++++++++++++++++++++++
>  include/drm/drm_gem_cma_helper.h     | 17 +++++++++
>  4 files changed, 99 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> index bca8085..6e7efab 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -303,6 +303,17 @@ created.
>  Drivers that want to map the GEM object upfront instead of handling page
>  faults can implement their own mmap file operation handler.
> 
> +For platforms without MMU GEM care provides a helper method

s/GEM care/the GEM core/

> +:c:func:`drm_gem_cma_get_unmapped_area`. The mmap() routines will call
> +this to get a proposed address for the mapping.
> +
> +To use :c:func:`drm_gem_cma_get_unmapped_area`, drivers must fill the
> +struct :c:type:`struct file_operations <file_operations>` get_unmapped_area
> +field with a pointer on :c:func:`drm_gem_cma_get_unmapped_area`.
> +
> +More detailed information about get_unmapped_area can be found in
> +Documentation/nommu-mmap.txt
> +
>  Memory Coherency
>  ----------------
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 83ac815..0eae4ad 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_gem_cma_helper.c
> b/drivers/gpu/drm/drm_gem_cma_helper.c index 1d6c335..01016b1 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -358,6 +358,75 @@ int drm_gem_cma_mmap(struct file *filp, struct
> vm_area_struct *vma) }
>  EXPORT_SYMBOL_GPL(drm_gem_cma_mmap);
> 
> +#ifndef CONFIG_MMU
> +/**
> + * drm_gem_cma_get_unmapped_area - propose address for mapping in noMMU
> cases
> + * @filp: file object
> + * @addr: memory address
> + * @len: buffer size
> + * @pgoff: page offset
> + * @flags: memory flags
> + *
> + * This function is used in noMMU platforms to propose address mapping
> + * for a given buffer.

I would add

"It's intended to be used as a direct handler for the :c:type:`struct 
file_operations <file_operations>` .get_unmapped_area() operation."

Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> + * Returns:
> + * mapping address on success or a negative error code on failure
> + */
> +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;
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_cma_get_unmapped_area);
> +#endif
> +
>  #ifdef CONFIG_DEBUG_FS
>  /**
>   * drm_gem_cma_describe - describe a CMA GEM object for debugfs
> diff --git a/include/drm/drm_gem_cma_helper.h
> b/include/drm/drm_gem_cma_helper.h index acd6af8..180b586 100644
> --- a/include/drm/drm_gem_cma_helper.h
> +++ b/include/drm/drm_gem_cma_helper.h
> @@ -53,6 +53,23 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct
> drm_device *drm,
> 
>  extern const struct vm_operations_struct drm_gem_cma_vm_ops;
> 
> +#ifndef CONFIG_MMU
> +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp,
> +					    unsigned long addr,
> +					    unsigned long len,
> +					    unsigned long pgoff,
> +					    unsigned long flags);
> +#else
> +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp,
> +					    unsigned long addr,
> +					    unsigned long len,
> +					    unsigned long pgoff,
> +					    unsigned long flags)
> +{
> +	return -EINVAL;
> +}
> +#endif
> +
>  #ifdef CONFIG_DEBUG_FS
>  void drm_gem_cma_describe(struct drm_gem_cma_object *obj, struct seq_file
> *m); #endif

-- 
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] 17+ messages in thread

* Re: [PATCH v4 3/4] drm: compile drm_vm.c only when needed
  2016-12-07 10:06 ` [PATCH v4 3/4] drm: compile drm_vm.c only when needed Benjamin Gaignard
@ 2016-12-07 14:34   ` Laurent Pinchart
  0 siblings, 0 replies; 17+ messages in thread
From: Laurent Pinchart @ 2016-12-07 14:34 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: linaro-kernel, michel, philippe.cornu, dri-devel, yannick.fertre,
	airlied

Hi Benjamin,

Thank you for the patch.

On Wednesday 07 Dec 2016 11:06:50 Benjamin Gaignard wrote:
> drm_vm.c functions are only need for DRM_LEGACY and DRM_NOUVEAU.
> Use a new DRM_VM to define when drm_vm.c in needed.
> 
> stub drm_legacy_vma_flush() to avoid compilation issues
> 
> version 4:
> - a "config DRM_VM" in Kconfig
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/Kconfig         | 5 +++++
>  drivers/gpu/drm/Makefile        | 3 ++-
>  drivers/gpu/drm/drm_legacy.h    | 7 +++++++
>  drivers/gpu/drm/nouveau/Kconfig | 1 +
>  4 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 8d9cf73..83ac815 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -121,6 +121,10 @@ config DRM_KMS_CMA_HELPER
>  	help
>  	  Choose this if you need the KMS CMA helper functions
> 
> +config DRM_VM
> +	bool
> +	depends on DRM
> +
>  source "drivers/gpu/drm/i2c/Kconfig"
> 
>  source "drivers/gpu/drm/arm/Kconfig"
> @@ -246,6 +250,7 @@ source "drivers/gpu/drm/zte/Kconfig"
>  menuconfig DRM_LEGACY
>  	bool "Enable legacy drivers (DANGEROUS)"
>  	depends on DRM
> +	select DRM_VM
>  	help
>  	  Enable legacy DRI1 drivers. Those drivers expose unsafe and 
dangerous
>  	  APIs to user-space, which can be used to circumvent access
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index e10e935..5b73b16 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -5,7 +5,7 @@
>  drm-y       :=	drm_auth.o drm_bufs.o drm_cache.o \
>  		drm_context.o drm_dma.o \
>  		drm_fops.o drm_gem.o drm_ioctl.o drm_irq.o \
> -		drm_lock.o drm_memory.o drm_drv.o drm_vm.o \
> +		drm_lock.o drm_memory.o drm_drv.o \
>  		drm_scatter.o drm_pci.o \
>  		drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
>  		drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \
> @@ -18,6 +18,7 @@ drm-y       :=	drm_auth.o drm_bufs.o drm_cache.o \
>  		drm_plane.o drm_color_mgmt.o drm_print.o \
>  		drm_dumb_buffers.o drm_mode_config.o
> 
> +drm-$(CONFIG_DRM_VM) += drm_vm.o
>  drm-$(CONFIG_COMPAT) += drm_ioc32.o
>  drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
>  drm-$(CONFIG_PCI) += ati_pcigart.o
> diff --git a/drivers/gpu/drm/drm_legacy.h b/drivers/gpu/drm/drm_legacy.h
> index c6f422e..e4bb5ad 100644
> --- a/drivers/gpu/drm/drm_legacy.h
> +++ b/drivers/gpu/drm/drm_legacy.h
> @@ -74,7 +74,14 @@ int drm_legacy_getmap_ioctl(struct drm_device *dev, void
> *data, int drm_legacy_mapbufs(struct drm_device *d, void *v, struct
> drm_file *f); int drm_legacy_dma_ioctl(struct drm_device *d, void *v,
> struct drm_file *f);
> 
> +#ifdef CONFIG_DRM_VM
>  void drm_legacy_vma_flush(struct drm_device *d);
> +#else
> +static inline void drm_legacy_vma_flush(struct drm_device *d)
> +{
> +	/* do nothing */
> +}
> +#endif
> 
>  /*
>   * AGP Support
> diff --git a/drivers/gpu/drm/nouveau/Kconfig
> b/drivers/gpu/drm/nouveau/Kconfig index 2922a82..0f2f0af 100644
> --- a/drivers/gpu/drm/nouveau/Kconfig
> +++ b/drivers/gpu/drm/nouveau/Kconfig
> @@ -16,6 +16,7 @@ config DRM_NOUVEAU
>  	select INPUT if ACPI && X86
>  	select THERMAL if ACPI && X86
>  	select ACPI_VIDEO if ACPI && X86
> +	select DRM_VM
>  	help
>  	  Choose this option for open-source NVIDIA support.

-- 
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] 17+ messages in thread

* Re: [PATCH v4 2/4] fbmem: add a default get_fb_unmapped_area function
  2016-12-07 10:06 ` [PATCH v4 2/4] fbmem: add a default get_fb_unmapped_area function Benjamin Gaignard
@ 2016-12-07 14:35   ` Laurent Pinchart
  2016-12-07 14:57     ` Benjamin Gaignard
  0 siblings, 1 reply; 17+ messages in thread
From: Laurent Pinchart @ 2016-12-07 14:35 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: linaro-kernel, michel, philippe.cornu, dri-devel, yannick.fertre,
	airlied

Hi Benjamin,

Thank you for the patch.

On Wednesday 07 Dec 2016 11:06:49 Benjamin Gaignard wrote:
> Allow generic frame-buffer to provide a default
> get_fb_unmapped_area function if specific devices need it.
> 
> Usually this function is defined in architecture directories but
> define it here may limit code duplication especially for all ARM
> platforms without MMU.

I still would like to see an explanation why an architecture-specific version 
is sometimes needed, and why this implementation is a reasonable default. 
Furthermore, it looks very similar to the blackfin implementation, so if you 
can't unify the two I'd like to know why.

> version 4:
> - introdude a configuration flag to be independent of architecture
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> ---
>  drivers/video/fbdev/Kconfig      |  8 ++++++++
>  drivers/video/fbdev/core/fbmem.c | 15 +++++++++++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index 5d3b0db..922e4ea 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -138,6 +138,14 @@ config FB_SYS_IMAGEBLIT
>  	  blitting. This is used by drivers that don't provide their own
>  	  (accelerated) version and the framebuffer is in system RAM.
> 
> +config FB_PROVIDE_GET_FB_UNMAPPED_AREA
> +	bool
> +	depends on FB
> +	default n
> +	---help---
> +	  Allow generic frame-buffer to provide get_fb_unmapped_area
> +	  function.
> +
>  menuconfig FB_FOREIGN_ENDIAN
>  	bool "Framebuffer foreign endianness support"
>  	depends on FB
> diff --git a/drivers/video/fbdev/core/fbmem.c
> b/drivers/video/fbdev/core/fbmem.c index 76c1ad9..22321a2 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;
>  }
> 
> +#ifdef CONFIG_FB_PROVIDE_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,

-- 
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] 17+ messages in thread

* Re: [PATCH v4 4/4] drm: allow to use mmuless SoC
  2016-12-07 14:32   ` Laurent Pinchart
@ 2016-12-07 14:49     ` Daniel Vetter
  0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2016-12-07 14:49 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linaro-kernel, michel, philippe.cornu, dri-devel, yannick.fertre,
	airlied

On Wed, Dec 07, 2016 at 04:32:44PM +0200, Laurent Pinchart wrote:
> Hi Benjamin,
> 
> Thank you for the patch.
> 
> On Wednesday 07 Dec 2016 11:06:51 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 a cma helper
> > function to help implementing mmuless display driver
> > 
> > version 4:
> > - add documentation about drm_gem_cma_get_unmapped_area()
> > - stub it MMU case
> > 
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> > ---
> >  Documentation/gpu/drm-mm.rst         | 11 ++++++
> >  drivers/gpu/drm/Kconfig              |  4 +--
> >  drivers/gpu/drm/drm_gem_cma_helper.c | 69 +++++++++++++++++++++++++++++++++
> >  include/drm/drm_gem_cma_helper.h     | 17 +++++++++
> >  4 files changed, 99 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> > index bca8085..6e7efab 100644
> > --- a/Documentation/gpu/drm-mm.rst
> > +++ b/Documentation/gpu/drm-mm.rst
> > @@ -303,6 +303,17 @@ created.
> >  Drivers that want to map the GEM object upfront instead of handling page
> >  faults can implement their own mmap file operation handler.
> > 
> > +For platforms without MMU GEM care provides a helper method
> 
> s/GEM care/the GEM core/
> 
> > +:c:func:`drm_gem_cma_get_unmapped_area`. The mmap() routines will call
> > +this to get a proposed address for the mapping.
> > +
> > +To use :c:func:`drm_gem_cma_get_unmapped_area`, drivers must fill the
> > +struct :c:type:`struct file_operations <file_operations>` get_unmapped_area
> > +field with a pointer on :c:func:`drm_gem_cma_get_unmapped_area`.
> > +
> > +More detailed information about get_unmapped_area can be found in
> > +Documentation/nommu-mmap.txt
> > +
> >  Memory Coherency
> >  ----------------
> > 
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index 83ac815..0eae4ad 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_gem_cma_helper.c
> > b/drivers/gpu/drm/drm_gem_cma_helper.c index 1d6c335..01016b1 100644
> > --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> > @@ -358,6 +358,75 @@ int drm_gem_cma_mmap(struct file *filp, struct
> > vm_area_struct *vma) }
> >  EXPORT_SYMBOL_GPL(drm_gem_cma_mmap);
> > 
> > +#ifndef CONFIG_MMU
> > +/**
> > + * drm_gem_cma_get_unmapped_area - propose address for mapping in noMMU
> > cases
> > + * @filp: file object
> > + * @addr: memory address
> > + * @len: buffer size
> > + * @pgoff: page offset
> > + * @flags: memory flags
> > + *
> > + * This function is used in noMMU platforms to propose address mapping
> > + * for a given buffer.
> 
> I would add
> 
> "It's intended to be used as a direct handler for the :c:type:`struct 
> file_operations <file_operations>` .get_unmapped_area() operation."

If' it's kernel-doc you can use shortcuts to autogenerate the links. Only
works with kernel-doc comments though, not with .rst in general:

"It's intended to be used as a direct handler for the struct 
&file_operations .get_unmapped_area() operation."

And +1 from me on sprinkling links all over docs. We might want to do that
for all the fbdev fops, for consistency.

Cheers, 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] 17+ messages in thread

* Re: [PATCH v4 2/4] fbmem: add a default get_fb_unmapped_area function
  2016-12-07 14:35   ` Laurent Pinchart
@ 2016-12-07 14:57     ` Benjamin Gaignard
  2016-12-07 15:19       ` Laurent Pinchart
  0 siblings, 1 reply; 17+ messages in thread
From: Benjamin Gaignard @ 2016-12-07 14:57 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Linaro Kernel Mailman List, Michel Dänzer, Philippe Cornu,
	dri-devel@lists.freedesktop.org, Yannick Fertre, Dave Airlie

2016-12-07 15:35 GMT+01:00 Laurent Pinchart <laurent.pinchart@ideasonboard.com>:
> Hi Benjamin,
>
> Thank you for the patch.
>
> On Wednesday 07 Dec 2016 11:06:49 Benjamin Gaignard wrote:
>> Allow generic frame-buffer to provide a default
>> get_fb_unmapped_area function if specific devices need it.
>>
>> Usually this function is defined in architecture directories but
>> define it here may limit code duplication especially for all ARM
>> platforms without MMU.
>
> I still would like to see an explanation why an architecture-specific version
> is sometimes needed, and why this implementation is a reasonable default.
> Furthermore, it looks very similar to the blackfin implementation, so if you
> can't unify the two I'd like to know why.

Obviously I don't know all the legacy behind this function but it is
definitively link
to architectures memory mapping strategies in no MMU cases.

I think this function is a reasonable default when the architecture is
doing a direct
mapping (no translation) of the memory like it is done in ARM.

Unlike what I propose blackfin implementation doesn't do any check on pgoff and
length and always return fb base address even is an offset has been requested.
I don't know if it is on purpose or just because nobody has never try to mmap
blackfin framebuffer with an offset...

>
>> version 4:
>> - introdude a configuration flag to be independent of architecture
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>> ---
>>  drivers/video/fbdev/Kconfig      |  8 ++++++++
>>  drivers/video/fbdev/core/fbmem.c | 15 +++++++++++++++
>>  2 files changed, 23 insertions(+)
>>
>> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
>> index 5d3b0db..922e4ea 100644
>> --- a/drivers/video/fbdev/Kconfig
>> +++ b/drivers/video/fbdev/Kconfig
>> @@ -138,6 +138,14 @@ config FB_SYS_IMAGEBLIT
>>         blitting. This is used by drivers that don't provide their own
>>         (accelerated) version and the framebuffer is in system RAM.
>>
>> +config FB_PROVIDE_GET_FB_UNMAPPED_AREA
>> +     bool
>> +     depends on FB
>> +     default n
>> +     ---help---
>> +       Allow generic frame-buffer to provide get_fb_unmapped_area
>> +       function.
>> +
>>  menuconfig FB_FOREIGN_ENDIAN
>>       bool "Framebuffer foreign endianness support"
>>       depends on FB
>> diff --git a/drivers/video/fbdev/core/fbmem.c
>> b/drivers/video/fbdev/core/fbmem.c index 76c1ad9..22321a2 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;
>>  }
>>
>> +#ifdef CONFIG_FB_PROVIDE_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,
>
> --
> 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] 17+ messages in thread

* Re: [PATCH v4 2/4] fbmem: add a default get_fb_unmapped_area function
  2016-12-07 14:57     ` Benjamin Gaignard
@ 2016-12-07 15:19       ` Laurent Pinchart
  2016-12-07 15:31         ` Benjamin Gaignard
  0 siblings, 1 reply; 17+ messages in thread
From: Laurent Pinchart @ 2016-12-07 15:19 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: Linaro Kernel Mailman List, Michel Dänzer, Philippe Cornu,
	dri-devel@lists.freedesktop.org, Yannick Fertre, Dave Airlie

Hi Benjamin,

On Wednesday 07 Dec 2016 15:57:49 Benjamin Gaignard wrote:
> 2016-12-07 15:35 GMT+01:00 Laurent Pinchart:
> > On Wednesday 07 Dec 2016 11:06:49 Benjamin Gaignard wrote:
> >> Allow generic frame-buffer to provide a default
> >> get_fb_unmapped_area function if specific devices need it.
> >> 
> >> Usually this function is defined in architecture directories but
> >> define it here may limit code duplication especially for all ARM
> >> platforms without MMU.
> > 
> > I still would like to see an explanation why an architecture-specific
> > version is sometimes needed, and why this implementation is a reasonable
> > default. Furthermore, it looks very similar to the blackfin
> > implementation, so if you can't unify the two I'd like to know why.
> 
> Obviously I don't know all the legacy behind this function but it is
> definitively link to architectures memory mapping strategies in no MMU
> cases.
> 
> I think this function is a reasonable default when the architecture is
> doing a direct mapping (no translation) of the memory like it is done in
> ARM.
> 
> Unlike what I propose blackfin implementation doesn't do any check on pgoff
> and length and always return fb base address even is an offset has been
> requested. I don't know if it is on purpose or just because nobody has
> never try to mmap blackfin framebuffer with an offset...

And that's exactly what I'd like you to try and find out :-) git blame and 
contacting the original authors of that code could be a first step.

> >> version 4:
> >> - introdude a configuration flag to be independent of architecture
> >> 
> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> >> ---
> >> 
> >>  drivers/video/fbdev/Kconfig      |  8 ++++++++
> >>  drivers/video/fbdev/core/fbmem.c | 15 +++++++++++++++
> >>  2 files changed, 23 insertions(+)
> >> 
> >> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> >> index 5d3b0db..922e4ea 100644
> >> --- a/drivers/video/fbdev/Kconfig
> >> +++ b/drivers/video/fbdev/Kconfig
> >> @@ -138,6 +138,14 @@ config FB_SYS_IMAGEBLIT
> >> 
> >>         blitting. This is used by drivers that don't provide their own
> >>         (accelerated) version and the framebuffer is in system RAM.
> >> 
> >> +config FB_PROVIDE_GET_FB_UNMAPPED_AREA
> >> +     bool
> >> +     depends on FB
> >> +     default n
> >> +     ---help---
> >> +       Allow generic frame-buffer to provide get_fb_unmapped_area
> >> +       function.
> >> +
> >> 
> >>  menuconfig FB_FOREIGN_ENDIAN
> >>  
> >>       bool "Framebuffer foreign endianness support"
> >>       depends on FB
> >> 
> >> diff --git a/drivers/video/fbdev/core/fbmem.c
> >> b/drivers/video/fbdev/core/fbmem.c index 76c1ad9..22321a2 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;
> >> 
> >>  }
> >> 
> >> +#ifdef CONFIG_FB_PROVIDE_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,
> > 
> > --
> > Regards,
> > 
> > Laurent Pinchart

-- 
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] 17+ messages in thread

* [PATCH v4 4/4] drm: allow to use mmuless SoC
@ 2016-12-07 15:19   ` Benjamin Gaignard
  2016-12-07 16:17     ` Eric Engestrom
  2016-12-07 20:44     ` Arnd Bergmann
  0 siblings, 2 replies; 17+ messages in thread
From: Benjamin Gaignard @ 2016-12-07 15:19 UTC (permalink / raw)
  To: dri-devel, airlied, daniel, laurent.pinchart, michel
  Cc: yannick.fertre, linaro-kernel, philippe.cornu

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 a cma helper
function to help implementing mmuless display driver

version 4:
- add documentation about drm_gem_cma_get_unmapped_area()
- stub it MMU case

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 Documentation/gpu/drm-mm.rst         | 11 ++++++
 drivers/gpu/drm/Kconfig              |  4 +-
 drivers/gpu/drm/drm_gem_cma_helper.c | 71 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_gem_cma_helper.h     | 17 +++++++++
 4 files changed, 101 insertions(+), 2 deletions(-)

diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index bca8085..9d4aa11 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -303,6 +303,17 @@ created.
 Drivers that want to map the GEM object upfront instead of handling page
 faults can implement their own mmap file operation handler.
 
+For platforms without MMU the GEM core provides a helper method
+:c:func:`drm_gem_cma_get_unmapped_area`. The mmap() routines will call
+this to get a proposed address for the mapping.
+
+To use :c:func:`drm_gem_cma_get_unmapped_area`, drivers must fill the
+struct :c:type:`struct file_operations <file_operations>` get_unmapped_area
+field with a pointer on :c:func:`drm_gem_cma_get_unmapped_area`.
+
+More detailed information about get_unmapped_area can be found in
+Documentation/nommu-mmap.txt
+
 Memory Coherency
 ----------------
 
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 83ac815..0eae4ad 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_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index 1d6c335..19908bb 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -358,6 +358,77 @@ int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma)
 }
 EXPORT_SYMBOL_GPL(drm_gem_cma_mmap);
 
+#ifndef CONFIG_MMU
+/**
+ * drm_gem_cma_get_unmapped_area - propose address for mapping in noMMU cases
+ * @filp: file object
+ * @addr: memory address
+ * @len: buffer size
+ * @pgoff: page offset
+ * @flags: memory flags
+ *
+ * This function is used in noMMU platforms to propose address mapping
+ * for a given buffer.
+ * It's intended to be used as a direct handler for the struct &file_operations
+ * .get_unmapped_area() operation.
+ *
+ * Returns:
+ * mapping address on success or a negative error code on failure.
+ */
+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;
+}
+EXPORT_SYMBOL_GPL(drm_gem_cma_get_unmapped_area);
+#endif
+
 #ifdef CONFIG_DEBUG_FS
 /**
  * drm_gem_cma_describe - describe a CMA GEM object for debugfs
diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h
index acd6af8..180b586 100644
--- a/include/drm/drm_gem_cma_helper.h
+++ b/include/drm/drm_gem_cma_helper.h
@@ -53,6 +53,23 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
 
 extern const struct vm_operations_struct drm_gem_cma_vm_ops;
 
+#ifndef CONFIG_MMU
+unsigned long drm_gem_cma_get_unmapped_area(struct file *filp,
+					    unsigned long addr,
+					    unsigned long len,
+					    unsigned long pgoff,
+					    unsigned long flags);
+#else
+unsigned long drm_gem_cma_get_unmapped_area(struct file *filp,
+					    unsigned long addr,
+					    unsigned long len,
+					    unsigned long pgoff,
+					    unsigned long flags)
+{
+	return -EINVAL;
+}
+#endif
+
 #ifdef CONFIG_DEBUG_FS
 void drm_gem_cma_describe(struct drm_gem_cma_object *obj, struct seq_file *m);
 #endif
-- 
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] 17+ messages in thread

* Re: [PATCH v4 2/4] fbmem: add a default get_fb_unmapped_area function
  2016-12-07 15:19       ` Laurent Pinchart
@ 2016-12-07 15:31         ` Benjamin Gaignard
  0 siblings, 0 replies; 17+ messages in thread
From: Benjamin Gaignard @ 2016-12-07 15:31 UTC (permalink / raw)
  To: Laurent Pinchart, thomas
  Cc: Linaro Kernel Mailman List, Michel Dänzer, Philippe Cornu,
	dri-devel@lists.freedesktop.org, Yannick Fertre, Dave Airlie

Hi Thomas,

in commit 59bd00c8 (Blackfin: fix framebuffer mmap bug for nommu) you
have introduce
get_fb_unmapped_area() for blackfin architecture.

I'm proposing a patch to have a default function in fbmem which
slightly does the same.

Do you think is new function could also fit with blackfin architecture needs ?
or does this architecture have specific requirements ?

Regards,
Benjamin

2016-12-07 16:19 GMT+01:00 Laurent Pinchart <laurent.pinchart@ideasonboard.com>:
> Hi Benjamin,
>
> On Wednesday 07 Dec 2016 15:57:49 Benjamin Gaignard wrote:
>> 2016-12-07 15:35 GMT+01:00 Laurent Pinchart:
>> > On Wednesday 07 Dec 2016 11:06:49 Benjamin Gaignard wrote:
>> >> Allow generic frame-buffer to provide a default
>> >> get_fb_unmapped_area function if specific devices need it.
>> >>
>> >> Usually this function is defined in architecture directories but
>> >> define it here may limit code duplication especially for all ARM
>> >> platforms without MMU.
>> >
>> > I still would like to see an explanation why an architecture-specific
>> > version is sometimes needed, and why this implementation is a reasonable
>> > default. Furthermore, it looks very similar to the blackfin
>> > implementation, so if you can't unify the two I'd like to know why.
>>
>> Obviously I don't know all the legacy behind this function but it is
>> definitively link to architectures memory mapping strategies in no MMU
>> cases.
>>
>> I think this function is a reasonable default when the architecture is
>> doing a direct mapping (no translation) of the memory like it is done in
>> ARM.
>>
>> Unlike what I propose blackfin implementation doesn't do any check on pgoff
>> and length and always return fb base address even is an offset has been
>> requested. I don't know if it is on purpose or just because nobody has
>> never try to mmap blackfin framebuffer with an offset...
>
> And that's exactly what I'd like you to try and find out :-) git blame and
> contacting the original authors of that code could be a first step.
>
>> >> version 4:
>> >> - introdude a configuration flag to be independent of architecture
>> >>
>> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>> >> ---
>> >>
>> >>  drivers/video/fbdev/Kconfig      |  8 ++++++++
>> >>  drivers/video/fbdev/core/fbmem.c | 15 +++++++++++++++
>> >>  2 files changed, 23 insertions(+)
>> >>
>> >> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
>> >> index 5d3b0db..922e4ea 100644
>> >> --- a/drivers/video/fbdev/Kconfig
>> >> +++ b/drivers/video/fbdev/Kconfig
>> >> @@ -138,6 +138,14 @@ config FB_SYS_IMAGEBLIT
>> >>
>> >>         blitting. This is used by drivers that don't provide their own
>> >>         (accelerated) version and the framebuffer is in system RAM.
>> >>
>> >> +config FB_PROVIDE_GET_FB_UNMAPPED_AREA
>> >> +     bool
>> >> +     depends on FB
>> >> +     default n
>> >> +     ---help---
>> >> +       Allow generic frame-buffer to provide get_fb_unmapped_area
>> >> +       function.
>> >> +
>> >>
>> >>  menuconfig FB_FOREIGN_ENDIAN
>> >>
>> >>       bool "Framebuffer foreign endianness support"
>> >>       depends on FB
>> >>
>> >> diff --git a/drivers/video/fbdev/core/fbmem.c
>> >> b/drivers/video/fbdev/core/fbmem.c index 76c1ad9..22321a2 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;
>> >>
>> >>  }
>> >>
>> >> +#ifdef CONFIG_FB_PROVIDE_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,
>> >
>> > --
>> > Regards,
>> >
>> > Laurent Pinchart
>
> --
> 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] 17+ messages in thread

* Re: [PATCH v4 4/4] drm: allow to use mmuless SoC
  2016-12-07 15:19   ` Benjamin Gaignard
@ 2016-12-07 16:17     ` Eric Engestrom
  2016-12-07 20:44     ` Arnd Bergmann
  1 sibling, 0 replies; 17+ messages in thread
From: Eric Engestrom @ 2016-12-07 16:17 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: linaro-kernel, michel, philippe.cornu, dri-devel, yannick.fertre,
	laurent.pinchart, airlied

On Wednesday, 2016-12-07 16:19:52 +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 a cma helper
> function to help implementing mmuless display driver
> 
> version 4:
> - add documentation about drm_gem_cma_get_unmapped_area()
> - stub it MMU case
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> ---
>  Documentation/gpu/drm-mm.rst         | 11 ++++++
>  drivers/gpu/drm/Kconfig              |  4 +-
>  drivers/gpu/drm/drm_gem_cma_helper.c | 71 ++++++++++++++++++++++++++++++++++++
>  include/drm/drm_gem_cma_helper.h     | 17 +++++++++
>  4 files changed, 101 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> index bca8085..9d4aa11 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -303,6 +303,17 @@ created.
>  Drivers that want to map the GEM object upfront instead of handling page
>  faults can implement their own mmap file operation handler.
>  
> +For platforms without MMU the GEM core provides a helper method
> +:c:func:`drm_gem_cma_get_unmapped_area`. The mmap() routines will call
> +this to get a proposed address for the mapping.
> +
> +To use :c:func:`drm_gem_cma_get_unmapped_area`, drivers must fill the
> +struct :c:type:`struct file_operations <file_operations>` get_unmapped_area
> +field with a pointer on :c:func:`drm_gem_cma_get_unmapped_area`.
> +
> +More detailed information about get_unmapped_area can be found in
> +Documentation/nommu-mmap.txt
> +
>  Memory Coherency
>  ----------------
>  
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 83ac815..0eae4ad 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_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
> index 1d6c335..19908bb 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -358,6 +358,77 @@ int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma)
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_cma_mmap);
>  
> +#ifndef CONFIG_MMU
> +/**
> + * drm_gem_cma_get_unmapped_area - propose address for mapping in noMMU cases
> + * @filp: file object
> + * @addr: memory address
> + * @len: buffer size
> + * @pgoff: page offset
> + * @flags: memory flags
> + *
> + * This function is used in noMMU platforms to propose address mapping
> + * for a given buffer.
> + * It's intended to be used as a direct handler for the struct &file_operations
> + * .get_unmapped_area() operation.
> + *
> + * Returns:
> + * mapping address on success or a negative error code on failure.
> + */
> +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;
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_cma_get_unmapped_area);
> +#endif
> +
>  #ifdef CONFIG_DEBUG_FS
>  /**
>   * drm_gem_cma_describe - describe a CMA GEM object for debugfs
> diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h
> index acd6af8..180b586 100644
> --- a/include/drm/drm_gem_cma_helper.h
> +++ b/include/drm/drm_gem_cma_helper.h
> @@ -53,6 +53,23 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
>  
>  extern const struct vm_operations_struct drm_gem_cma_vm_ops;
>  
> +#ifndef CONFIG_MMU
> +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp,
> +					    unsigned long addr,
> +					    unsigned long len,
> +					    unsigned long pgoff,
> +					    unsigned long flags);
> +#else

+static inline

> +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp,
> +					    unsigned long addr,
> +					    unsigned long len,
> +					    unsigned long pgoff,
> +					    unsigned long flags)
> +{
> +	return -EINVAL;
> +}

`static` is required to be able to include this header into multiple
c files that get linked into the same object, and `inline` because
there's no point having a function call for this :)

> +#endif
> +
>  #ifdef CONFIG_DEBUG_FS
>  void drm_gem_cma_describe(struct drm_gem_cma_object *obj, struct seq_file *m);
>  #endif
> -- 
> 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] 17+ messages in thread

* Re: [PATCH v4 4/4] drm: allow to use mmuless SoC
  2016-12-07 15:19   ` Benjamin Gaignard
  2016-12-07 16:17     ` Eric Engestrom
@ 2016-12-07 20:44     ` Arnd Bergmann
  1 sibling, 0 replies; 17+ messages in thread
From: Arnd Bergmann @ 2016-12-07 20:44 UTC (permalink / raw)
  To: linaro-kernel
  Cc: michel, philippe.cornu, dri-devel, yannick.fertre,
	laurent.pinchart, airlied

On Wednesday, December 7, 2016 4:19:52 PM CET Benjamin Gaignard wrote:
> +unsigned long drm_gem_cma_get_unmapped_area(struct file *filp,
> +                                           unsigned long addr,
> +                                           unsigned long len,
> +                                           unsigned long pgoff,
> +                                           unsigned long flags)
> +{
> +       return -EINVAL;
> +}
> 

This must be 'static inline'.

	Arnd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2016-12-07 20:44 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-07 10:06 [PATCH v4 0/4] DRM: allow to use mmuless devices Benjamin Gaignard
2016-12-07 10:06 ` [PATCH v4 1/4] nommu: allow mmap when !CONFIG_MMU Benjamin Gaignard
2016-12-07 10:06 ` [PATCH v4 2/4] fbmem: add a default get_fb_unmapped_area function Benjamin Gaignard
2016-12-07 14:35   ` Laurent Pinchart
2016-12-07 14:57     ` Benjamin Gaignard
2016-12-07 15:19       ` Laurent Pinchart
2016-12-07 15:31         ` Benjamin Gaignard
2016-12-07 10:06 ` [PATCH v4 3/4] drm: compile drm_vm.c only when needed Benjamin Gaignard
2016-12-07 14:34   ` Laurent Pinchart
2016-12-07 10:06 ` [PATCH v4 4/4] drm: allow to use mmuless SoC Benjamin Gaignard
2016-12-07 10:27   ` Daniel Vetter
2016-12-07 10:42     ` Benjamin Gaignard
2016-12-07 14:32   ` Laurent Pinchart
2016-12-07 14:49     ` Daniel Vetter
2016-12-07 15:19   ` Benjamin Gaignard
2016-12-07 16:17     ` Eric Engestrom
2016-12-07 20:44     ` Arnd Bergmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.