linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/4] drm: add pgprot callback to drm_gem_object_funcs
       [not found] <20191211121957.18637-1-kraxel@redhat.com>
@ 2019-12-11 12:19 ` Gerd Hoffmann
  2019-12-11 12:38   ` Daniel Vetter
  2019-12-11 12:19 ` [PATCH v3 2/4] drm/shmem: add support for per object caching flags Gerd Hoffmann
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Gerd Hoffmann @ 2019-12-11 12:19 UTC (permalink / raw)
  To: dri-devel
  Cc: tzimmermann, gurchetansingh, Gerd Hoffmann, Maarten Lankhorst,
	Maxime Ripard, David Airlie, Daniel Vetter, open list

The callback allows drivers and helpers to tweak pgprot for mappings.
This is especially helpful when using shmem helpers.  It allows drivers
to switch mappings from writecombine (default) to something else (cached
for example) on a per-object base without having to supply their own
mmap() and vmap() functions.

The patch also adds two implementations for the callback, for cached and
writecombine mappings, and the drm_gem_pgprot() function to update
pgprot for a given object, using the new &drm_gem_object_funcs.pgprot
callback if available.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/drm/drm_gem.h     | 15 +++++++++++++
 drivers/gpu/drm/drm_gem.c | 46 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 0b375069cd48..5beef7226e69 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -163,6 +163,17 @@ struct drm_gem_object_funcs {
 	 */
 	int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma);
 
+	/**
+	 * @pgprot:
+	 *
+	 * Tweak pgprot as needed, typically used to set cache bits.
+	 *
+	 * This callback is optional.
+	 *
+	 * If unset drm_gem_pgprot_wc() will be used.
+	 */
+	pgprot_t (*pgprot)(struct drm_gem_object *obj, pgprot_t prot);
+
 	/**
 	 * @vm_ops:
 	 *
@@ -350,6 +361,10 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
 		     struct vm_area_struct *vma);
 int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
 
+pgprot_t drm_gem_pgprot_cached(struct drm_gem_object *obj, pgprot_t prot);
+pgprot_t drm_gem_pgprot_wc(struct drm_gem_object *obj, pgprot_t prot);
+pgprot_t drm_gem_pgprot(struct drm_gem_object *obj, pgprot_t prot);
+
 /**
  * drm_gem_object_get - acquire a GEM buffer object reference
  * @obj: GEM buffer object
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 56f42e0f2584..1c468fe8e342 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1119,7 +1119,8 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
 			return -EINVAL;
 
 		vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
-		vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+		vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
+		vma->vm_page_prot = drm_gem_pgprot(obj, vma->vm_page_prot);
 		vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
 	}
 
@@ -1210,6 +1211,49 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 }
 EXPORT_SYMBOL(drm_gem_mmap);
 
+/**
+ * drm_gem_mmap - update pgprot for objects needing a cachable mapping.
+ * @obj: the GEM object.
+ * @prot: page attributes.
+ *
+ * This function can be used as &drm_gem_object_funcs.pgprot callback.
+ */
+pgprot_t drm_gem_pgprot_cached(struct drm_gem_object *obj, pgprot_t prot)
+{
+	return prot;
+}
+EXPORT_SYMBOL(drm_gem_pgprot_cached);
+
+/**
+ * drm_gem_mmap - update pgprot for objects needing a wc mapping.
+ * @obj: the GEM object.
+ * @prot: page attributes.
+ *
+ * This function can be used as &drm_gem_object_funcs.pgprot callback.
+ */
+pgprot_t drm_gem_pgprot_wc(struct drm_gem_object *obj, pgprot_t prot)
+{
+	return pgprot_writecombine(prot);
+}
+EXPORT_SYMBOL(drm_gem_pgprot_wc);
+
+/**
+ * drm_gem_mmap - update pgprot for a given gem object.
+ * @obj: the GEM object.
+ * @prot: page attributes.
+ *
+ * This function updates pgprot according to the needs of the given
+ * object.  If present &drm_gem_object_funcs.pgprot callback will be
+ * used, otherwise drm_gem_pgprot_wc() is called.
+ */
+pgprot_t drm_gem_pgprot(struct drm_gem_object *obj, pgprot_t prot)
+{
+	if (obj->funcs->pgprot)
+		return obj->funcs->pgprot(obj, prot);
+	return drm_gem_pgprot_wc(obj, prot);
+}
+EXPORT_SYMBOL(drm_gem_pgprot);
+
 void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
 			const struct drm_gem_object *obj)
 {
-- 
2.18.1


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

* [PATCH v3 2/4] drm/shmem: add support for per object caching flags.
       [not found] <20191211121957.18637-1-kraxel@redhat.com>
  2019-12-11 12:19 ` [PATCH v3 1/4] drm: add pgprot callback to drm_gem_object_funcs Gerd Hoffmann
@ 2019-12-11 12:19 ` Gerd Hoffmann
  2019-12-11 12:19 ` [PATCH v3 3/4] drm/virtio: fix mmap page attributes Gerd Hoffmann
  2019-12-11 12:19 ` [PATCH v3 4/4] drm/udl: simplify gem object mapping Gerd Hoffmann
  3 siblings, 0 replies; 7+ messages in thread
From: Gerd Hoffmann @ 2019-12-11 12:19 UTC (permalink / raw)
  To: dri-devel
  Cc: tzimmermann, gurchetansingh, Gerd Hoffmann, Maarten Lankhorst,
	Maxime Ripard, David Airlie, Daniel Vetter, open list

Use drm_gem_pgprot_wc() as pgprot callback in drm_gem_shmem_funcs.
Use drm_gem_pgprot() to update pgprot caching flags.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/drm_gem_shmem_helper.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index a421a2eed48a..2a662ed77115 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -33,6 +33,7 @@ static const struct drm_gem_object_funcs drm_gem_shmem_funcs = {
 	.vmap = drm_gem_shmem_vmap,
 	.vunmap = drm_gem_shmem_vunmap,
 	.mmap = drm_gem_shmem_mmap,
+	.pgprot = drm_gem_pgprot_wc,
 };
 
 /**
@@ -258,7 +259,7 @@ static void *drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem)
 		shmem->vaddr = dma_buf_vmap(obj->import_attach->dmabuf);
 	else
 		shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT,
-				    VM_MAP, pgprot_writecombine(PAGE_KERNEL));
+				    VM_MAP, drm_gem_pgprot(obj, PAGE_KERNEL));
 
 	if (!shmem->vaddr) {
 		DRM_DEBUG_KMS("Failed to vmap pages\n");
@@ -540,7 +541,8 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
 	}
 
 	vma->vm_flags |= VM_MIXEDMAP | VM_DONTEXPAND;
-	vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+	vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
+	vma->vm_page_prot = drm_gem_pgprot(obj, vma->vm_page_prot);
 	vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
 	vma->vm_ops = &drm_gem_shmem_vm_ops;
 
-- 
2.18.1


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

* [PATCH v3 3/4] drm/virtio: fix mmap page attributes
       [not found] <20191211121957.18637-1-kraxel@redhat.com>
  2019-12-11 12:19 ` [PATCH v3 1/4] drm: add pgprot callback to drm_gem_object_funcs Gerd Hoffmann
  2019-12-11 12:19 ` [PATCH v3 2/4] drm/shmem: add support for per object caching flags Gerd Hoffmann
@ 2019-12-11 12:19 ` Gerd Hoffmann
  2019-12-11 12:19 ` [PATCH v3 4/4] drm/udl: simplify gem object mapping Gerd Hoffmann
  3 siblings, 0 replies; 7+ messages in thread
From: Gerd Hoffmann @ 2019-12-11 12:19 UTC (permalink / raw)
  To: dri-devel
  Cc: tzimmermann, gurchetansingh, Gerd Hoffmann, David Airlie,
	Daniel Vetter, open list:VIRTIO GPU DRIVER, open list

virtio-gpu uses cached mappings, set virtio_gpu_gem_funcs.pgprot
accordingly.

Reported-by: Gurchetan Singh <gurchetansingh@chromium.org>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/virtio/virtgpu_object.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
index 017a9e0fc3bb..0b754c5bbcce 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -87,6 +87,7 @@ static const struct drm_gem_object_funcs virtio_gpu_gem_funcs = {
 	.vmap = drm_gem_shmem_vmap,
 	.vunmap = drm_gem_shmem_vunmap,
 	.mmap = &drm_gem_shmem_mmap,
+	.pgprot = &drm_gem_pgprot_cached,
 };
 
 struct drm_gem_object *virtio_gpu_create_object(struct drm_device *dev,
-- 
2.18.1


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

* [PATCH v3 4/4] drm/udl: simplify gem object mapping.
       [not found] <20191211121957.18637-1-kraxel@redhat.com>
                   ` (2 preceding siblings ...)
  2019-12-11 12:19 ` [PATCH v3 3/4] drm/virtio: fix mmap page attributes Gerd Hoffmann
@ 2019-12-11 12:19 ` Gerd Hoffmann
  3 siblings, 0 replies; 7+ messages in thread
From: Gerd Hoffmann @ 2019-12-11 12:19 UTC (permalink / raw)
  To: dri-devel
  Cc: tzimmermann, gurchetansingh, Gerd Hoffmann, Dave Airlie,
	Sean Paul, David Airlie, Daniel Vetter, open list

With shmem helpers allowing to update pgprot caching flags via
drm_gem_object_funcs.pgprot we can just use that and ditch our own
implementations of mmap() and vmap().

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/udl/udl_gem.c | 62 ++++-------------------------------
 1 file changed, 7 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
index b6e26f98aa0a..b82a4a921f1b 100644
--- a/drivers/gpu/drm/udl/udl_gem.c
+++ b/drivers/gpu/drm/udl/udl_gem.c
@@ -17,61 +17,12 @@
  * GEM object funcs
  */
 
-static int udl_gem_object_mmap(struct drm_gem_object *obj,
-			       struct vm_area_struct *vma)
+static pgprot_t udl_gem_object_pgprot(struct drm_gem_object *obj,
+				      pgprot_t pgprot)
 {
-	int ret;
-
-	ret = drm_gem_shmem_mmap(obj, vma);
-	if (ret)
-		return ret;
-
-	vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
 	if (obj->import_attach)
-		vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
-	vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
-
-	return 0;
-}
-
-static void *udl_gem_object_vmap(struct drm_gem_object *obj)
-{
-	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
-	int ret;
-
-	ret = mutex_lock_interruptible(&shmem->vmap_lock);
-	if (ret)
-		return ERR_PTR(ret);
-
-	if (shmem->vmap_use_count++ > 0)
-		goto out;
-
-	ret = drm_gem_shmem_get_pages(shmem);
-	if (ret)
-		goto err_zero_use;
-
-	if (obj->import_attach)
-		shmem->vaddr = dma_buf_vmap(obj->import_attach->dmabuf);
-	else
-		shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT,
-				    VM_MAP, PAGE_KERNEL);
-
-	if (!shmem->vaddr) {
-		DRM_DEBUG_KMS("Failed to vmap pages\n");
-		ret = -ENOMEM;
-		goto err_put_pages;
-	}
-
-out:
-	mutex_unlock(&shmem->vmap_lock);
-	return shmem->vaddr;
-
-err_put_pages:
-	drm_gem_shmem_put_pages(shmem);
-err_zero_use:
-	shmem->vmap_use_count = 0;
-	mutex_unlock(&shmem->vmap_lock);
-	return ERR_PTR(ret);
+		pgprot = pgprot_writecombine(pgprot);
+	return pgprot;
 }
 
 static const struct drm_gem_object_funcs udl_gem_object_funcs = {
@@ -80,9 +31,10 @@ static const struct drm_gem_object_funcs udl_gem_object_funcs = {
 	.pin = drm_gem_shmem_pin,
 	.unpin = drm_gem_shmem_unpin,
 	.get_sg_table = drm_gem_shmem_get_sg_table,
-	.vmap = udl_gem_object_vmap,
+	.vmap = drm_gem_shmem_vmap,
 	.vunmap = drm_gem_shmem_vunmap,
-	.mmap = udl_gem_object_mmap,
+	.mmap = drm_gem_shmem_mmap,
+	.pgprot = udl_gem_object_pgprot,
 };
 
 /*
-- 
2.18.1


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

* Re: [PATCH v3 1/4] drm: add pgprot callback to drm_gem_object_funcs
  2019-12-11 12:19 ` [PATCH v3 1/4] drm: add pgprot callback to drm_gem_object_funcs Gerd Hoffmann
@ 2019-12-11 12:38   ` Daniel Vetter
  2019-12-11 13:10     ` Gerd Hoffmann
  2019-12-11 13:21     ` Thomas Zimmermann
  0 siblings, 2 replies; 7+ messages in thread
From: Daniel Vetter @ 2019-12-11 12:38 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: dri-devel, tzimmermann, gurchetansingh, Maarten Lankhorst,
	Maxime Ripard, David Airlie, Daniel Vetter, open list

On Wed, Dec 11, 2019 at 01:19:53PM +0100, Gerd Hoffmann wrote:
> The callback allows drivers and helpers to tweak pgprot for mappings.
> This is especially helpful when using shmem helpers.  It allows drivers
> to switch mappings from writecombine (default) to something else (cached
> for example) on a per-object base without having to supply their own
> mmap() and vmap() functions.
> 
> The patch also adds two implementations for the callback, for cached and
> writecombine mappings, and the drm_gem_pgprot() function to update
> pgprot for a given object, using the new &drm_gem_object_funcs.pgprot
> callback if available.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/drm/drm_gem.h     | 15 +++++++++++++
>  drivers/gpu/drm/drm_gem.c | 46 ++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 60 insertions(+), 1 deletion(-)
> 
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index 0b375069cd48..5beef7226e69 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -163,6 +163,17 @@ struct drm_gem_object_funcs {
>  	 */
>  	int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma);
>  
> +	/**
> +	 * @pgprot:
> +	 *
> +	 * Tweak pgprot as needed, typically used to set cache bits.
> +	 *
> +	 * This callback is optional.
> +	 *
> +	 * If unset drm_gem_pgprot_wc() will be used.
> +	 */
> +	pgprot_t (*pgprot)(struct drm_gem_object *obj, pgprot_t prot);

I kinda prefer v1, mostly because this is a huge can of worms, and solving
this properly is going to be real hard (and will necessarily involve
dma-buf and dma-api and probably more). Charging ahead here just risks
that we dig ourselves into a corner. You're v1 is maybe not the most
clean, but just a few code bits here&there should be more flexible and
easier to hack on and experiment around with.
-Daniel

> +
>  	/**
>  	 * @vm_ops:
>  	 *
> @@ -350,6 +361,10 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
>  		     struct vm_area_struct *vma);
>  int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
>  
> +pgprot_t drm_gem_pgprot_cached(struct drm_gem_object *obj, pgprot_t prot);
> +pgprot_t drm_gem_pgprot_wc(struct drm_gem_object *obj, pgprot_t prot);
> +pgprot_t drm_gem_pgprot(struct drm_gem_object *obj, pgprot_t prot);
> +
>  /**
>   * drm_gem_object_get - acquire a GEM buffer object reference
>   * @obj: GEM buffer object
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 56f42e0f2584..1c468fe8e342 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1119,7 +1119,8 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
>  			return -EINVAL;
>  
>  		vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
> -		vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
> +		vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
> +		vma->vm_page_prot = drm_gem_pgprot(obj, vma->vm_page_prot);
>  		vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
>  	}
>  
> @@ -1210,6 +1211,49 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>  }
>  EXPORT_SYMBOL(drm_gem_mmap);
>  
> +/**
> + * drm_gem_mmap - update pgprot for objects needing a cachable mapping.
> + * @obj: the GEM object.
> + * @prot: page attributes.
> + *
> + * This function can be used as &drm_gem_object_funcs.pgprot callback.
> + */
> +pgprot_t drm_gem_pgprot_cached(struct drm_gem_object *obj, pgprot_t prot)
> +{
> +	return prot;
> +}
> +EXPORT_SYMBOL(drm_gem_pgprot_cached);
> +
> +/**
> + * drm_gem_mmap - update pgprot for objects needing a wc mapping.
> + * @obj: the GEM object.
> + * @prot: page attributes.
> + *
> + * This function can be used as &drm_gem_object_funcs.pgprot callback.
> + */
> +pgprot_t drm_gem_pgprot_wc(struct drm_gem_object *obj, pgprot_t prot)
> +{
> +	return pgprot_writecombine(prot);
> +}
> +EXPORT_SYMBOL(drm_gem_pgprot_wc);
> +
> +/**
> + * drm_gem_mmap - update pgprot for a given gem object.
> + * @obj: the GEM object.
> + * @prot: page attributes.
> + *
> + * This function updates pgprot according to the needs of the given
> + * object.  If present &drm_gem_object_funcs.pgprot callback will be
> + * used, otherwise drm_gem_pgprot_wc() is called.
> + */
> +pgprot_t drm_gem_pgprot(struct drm_gem_object *obj, pgprot_t prot)
> +{
> +	if (obj->funcs->pgprot)
> +		return obj->funcs->pgprot(obj, prot);
> +	return drm_gem_pgprot_wc(obj, prot);
> +}
> +EXPORT_SYMBOL(drm_gem_pgprot);
> +
>  void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
>  			const struct drm_gem_object *obj)
>  {
> -- 
> 2.18.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [PATCH v3 1/4] drm: add pgprot callback to drm_gem_object_funcs
  2019-12-11 12:38   ` Daniel Vetter
@ 2019-12-11 13:10     ` Gerd Hoffmann
  2019-12-11 13:21     ` Thomas Zimmermann
  1 sibling, 0 replies; 7+ messages in thread
From: Gerd Hoffmann @ 2019-12-11 13:10 UTC (permalink / raw)
  To: dri-devel, tzimmermann, gurchetansingh, Maarten Lankhorst,
	Maxime Ripard, David Airlie, open list

> > +	/**
> > +	 * @pgprot:
> > +	 *
> > +	 * Tweak pgprot as needed, typically used to set cache bits.
> > +	 *
> > +	 * This callback is optional.
> > +	 *
> > +	 * If unset drm_gem_pgprot_wc() will be used.
> > +	 */
> > +	pgprot_t (*pgprot)(struct drm_gem_object *obj, pgprot_t prot);
> 
> I kinda prefer v1, mostly because this is a huge can of worms, and solving
> this properly is going to be real hard (and will necessarily involve
> dma-buf and dma-api and probably more). Charging ahead here just risks
> that we dig ourselves into a corner. You're v1 is maybe not the most
> clean, but just a few code bits here&there should be more flexible and
> easier to hack on and experiment around with.

Hmm.  Second vote for v1.

Problem with v1 is that it covers mmap() only, not vmap() ...

cheers,
  Gerd


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

* Re: [PATCH v3 1/4] drm: add pgprot callback to drm_gem_object_funcs
  2019-12-11 12:38   ` Daniel Vetter
  2019-12-11 13:10     ` Gerd Hoffmann
@ 2019-12-11 13:21     ` Thomas Zimmermann
  1 sibling, 0 replies; 7+ messages in thread
From: Thomas Zimmermann @ 2019-12-11 13:21 UTC (permalink / raw)
  To: Gerd Hoffmann, dri-devel, gurchetansingh, Maarten Lankhorst,
	Maxime Ripard, David Airlie, open list


[-- Attachment #1.1: Type: text/plain, Size: 5364 bytes --]

Hi

Am 11.12.19 um 13:38 schrieb Daniel Vetter:
> On Wed, Dec 11, 2019 at 01:19:53PM +0100, Gerd Hoffmann wrote:
>> The callback allows drivers and helpers to tweak pgprot for mappings.
>> This is especially helpful when using shmem helpers.  It allows drivers
>> to switch mappings from writecombine (default) to something else (cached
>> for example) on a per-object base without having to supply their own
>> mmap() and vmap() functions.
>>
>> The patch also adds two implementations for the callback, for cached and
>> writecombine mappings, and the drm_gem_pgprot() function to update
>> pgprot for a given object, using the new &drm_gem_object_funcs.pgprot
>> callback if available.
>>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>>  include/drm/drm_gem.h     | 15 +++++++++++++
>>  drivers/gpu/drm/drm_gem.c | 46 ++++++++++++++++++++++++++++++++++++++-
>>  2 files changed, 60 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
>> index 0b375069cd48..5beef7226e69 100644
>> --- a/include/drm/drm_gem.h
>> +++ b/include/drm/drm_gem.h
>> @@ -163,6 +163,17 @@ struct drm_gem_object_funcs {
>>  	 */
>>  	int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma);
>>  
>> +	/**
>> +	 * @pgprot:
>> +	 *
>> +	 * Tweak pgprot as needed, typically used to set cache bits.
>> +	 *
>> +	 * This callback is optional.
>> +	 *
>> +	 * If unset drm_gem_pgprot_wc() will be used.
>> +	 */
>> +	pgprot_t (*pgprot)(struct drm_gem_object *obj, pgprot_t prot);
> 
> I kinda prefer v1, mostly because this is a huge can of worms, and solving
> this properly is going to be real hard (and will necessarily involve
> dma-buf and dma-api and probably more). Charging ahead here just risks
> that we dig ourselves into a corner. You're v1 is maybe not the most
> clean, but just a few code bits here&there should be more flexible and
> easier to hack on and experiment around with.
> -Daniel

I agree; at least patch v1 is known to be a sound approach. The others
might fall on our feet at some point. Sorry, Gerd, if my proposals added
lots of work for you.

Best regards
Thomas

> 
>> +
>>  	/**
>>  	 * @vm_ops:
>>  	 *
>> @@ -350,6 +361,10 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
>>  		     struct vm_area_struct *vma);
>>  int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
>>  
>> +pgprot_t drm_gem_pgprot_cached(struct drm_gem_object *obj, pgprot_t prot);
>> +pgprot_t drm_gem_pgprot_wc(struct drm_gem_object *obj, pgprot_t prot);
>> +pgprot_t drm_gem_pgprot(struct drm_gem_object *obj, pgprot_t prot);
>> +
>>  /**
>>   * drm_gem_object_get - acquire a GEM buffer object reference
>>   * @obj: GEM buffer object
>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
>> index 56f42e0f2584..1c468fe8e342 100644
>> --- a/drivers/gpu/drm/drm_gem.c
>> +++ b/drivers/gpu/drm/drm_gem.c
>> @@ -1119,7 +1119,8 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
>>  			return -EINVAL;
>>  
>>  		vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
>> -		vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
>> +		vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
>> +		vma->vm_page_prot = drm_gem_pgprot(obj, vma->vm_page_prot);
>>  		vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
>>  	}
>>  
>> @@ -1210,6 +1211,49 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>>  }
>>  EXPORT_SYMBOL(drm_gem_mmap);
>>  
>> +/**
>> + * drm_gem_mmap - update pgprot for objects needing a cachable mapping.
>> + * @obj: the GEM object.
>> + * @prot: page attributes.
>> + *
>> + * This function can be used as &drm_gem_object_funcs.pgprot callback.
>> + */
>> +pgprot_t drm_gem_pgprot_cached(struct drm_gem_object *obj, pgprot_t prot)
>> +{
>> +	return prot;
>> +}
>> +EXPORT_SYMBOL(drm_gem_pgprot_cached);
>> +
>> +/**
>> + * drm_gem_mmap - update pgprot for objects needing a wc mapping.
>> + * @obj: the GEM object.
>> + * @prot: page attributes.
>> + *
>> + * This function can be used as &drm_gem_object_funcs.pgprot callback.
>> + */
>> +pgprot_t drm_gem_pgprot_wc(struct drm_gem_object *obj, pgprot_t prot)
>> +{
>> +	return pgprot_writecombine(prot);
>> +}
>> +EXPORT_SYMBOL(drm_gem_pgprot_wc);
>> +
>> +/**
>> + * drm_gem_mmap - update pgprot for a given gem object.
>> + * @obj: the GEM object.
>> + * @prot: page attributes.
>> + *
>> + * This function updates pgprot according to the needs of the given
>> + * object.  If present &drm_gem_object_funcs.pgprot callback will be
>> + * used, otherwise drm_gem_pgprot_wc() is called.
>> + */
>> +pgprot_t drm_gem_pgprot(struct drm_gem_object *obj, pgprot_t prot)
>> +{
>> +	if (obj->funcs->pgprot)
>> +		return obj->funcs->pgprot(obj, prot);
>> +	return drm_gem_pgprot_wc(obj, prot);
>> +}
>> +EXPORT_SYMBOL(drm_gem_pgprot);
>> +
>>  void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
>>  			const struct drm_gem_object *obj)
>>  {
>> -- 
>> 2.18.1
>>
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

end of thread, other threads:[~2019-12-11 13:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20191211121957.18637-1-kraxel@redhat.com>
2019-12-11 12:19 ` [PATCH v3 1/4] drm: add pgprot callback to drm_gem_object_funcs Gerd Hoffmann
2019-12-11 12:38   ` Daniel Vetter
2019-12-11 13:10     ` Gerd Hoffmann
2019-12-11 13:21     ` Thomas Zimmermann
2019-12-11 12:19 ` [PATCH v3 2/4] drm/shmem: add support for per object caching flags Gerd Hoffmann
2019-12-11 12:19 ` [PATCH v3 3/4] drm/virtio: fix mmap page attributes Gerd Hoffmann
2019-12-11 12:19 ` [PATCH v3 4/4] drm/udl: simplify gem object mapping Gerd Hoffmann

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).