All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND PATCH v2 v4.1-rc8 0/2] drm: prime: Allow exported dma-bufs to be mapped
@ 2015-06-19 13:52 ` Daniel Thompson
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Thompson @ 2015-06-19 13:52 UTC (permalink / raw)
  To: David Airlie
  Cc: Daniel Thompson, dri-devel, Rob Clark, Benjamin Gaignard,
	Damien Hobson-Garcia, linux-kernel, patches, linaro-kernel,
	John Stultz, Sumit Semwal

This patch set started out as a single patch with a trivial bit of
boilerplate to add dmabuf mmap support to the msm driver. However Rob
Clark pointed out that, rather than keep one of the tricks I had used, it
would be better to change the helpers resulting in this series.

I've tested this both with a rather hacked about Android userspace
and with a fairly small test case run from debian. Both bits of code
currently use dumb buffers.

Thanks to Benjamin Gaignard for his help in finding this bit of code and
to Damien Hobson-Garcia for pointing out that I'd forgotten (since 3.18)
to RESEND these patches.

Dave: I guess its probably too late in the dev. cycle to take this code
      but don't worry, I will try really hard to remember to RESEND it
      for 4.2. ;-)

v2:

* Modified DRM_PRIME_HANDLE_TO_FD to honour the O_RDRW from the user
  and removed code to workaround this from the sti driver (Rob Clark).

* Added a patch to (rather spartanly) document gem_prime_mmap. Only
  tacked into this series 'cos I spotted it was missing when I was
  checking whether I needed to describe DRM_RDRW anywhere.


Daniel Thompson (2):
  drm: prime: Honour O_RDWR during prime-handle-to-fd
  drm: prime: Document gem_prime_mmap

 drivers/gpu/drm/drm_prime.c | 13 ++++++-------
 include/uapi/drm/drm.h      |  1 +
 2 files changed, 7 insertions(+), 7 deletions(-)

--
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

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

* [RESEND PATCH v2 v4.1-rc8 0/2] drm: prime: Allow exported dma-bufs to be mapped
@ 2015-06-19 13:52 ` Daniel Thompson
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Thompson @ 2015-06-19 13:52 UTC (permalink / raw)
  To: David Airlie
  Cc: Daniel Thompson, dri-devel, Rob Clark, Benjamin Gaignard,
	Damien Hobson-Garcia, linux-kernel, patches, linaro-kernel,
	John Stultz, Sumit Semwal

This patch set started out as a single patch with a trivial bit of
boilerplate to add dmabuf mmap support to the msm driver. However Rob
Clark pointed out that, rather than keep one of the tricks I had used, it
would be better to change the helpers resulting in this series.

I've tested this both with a rather hacked about Android userspace
and with a fairly small test case run from debian. Both bits of code
currently use dumb buffers.

Thanks to Benjamin Gaignard for his help in finding this bit of code and
to Damien Hobson-Garcia for pointing out that I'd forgotten (since 3.18)
to RESEND these patches.

Dave: I guess its probably too late in the dev. cycle to take this code
      but don't worry, I will try really hard to remember to RESEND it
      for 4.2. ;-)

v2:

* Modified DRM_PRIME_HANDLE_TO_FD to honour the O_RDRW from the user
  and removed code to workaround this from the sti driver (Rob Clark).

* Added a patch to (rather spartanly) document gem_prime_mmap. Only
  tacked into this series 'cos I spotted it was missing when I was
  checking whether I needed to describe DRM_RDRW anywhere.


Daniel Thompson (2):
  drm: prime: Honour O_RDWR during prime-handle-to-fd
  drm: prime: Document gem_prime_mmap

 drivers/gpu/drm/drm_prime.c | 13 ++++++-------
 include/uapi/drm/drm.h      |  1 +
 2 files changed, 7 insertions(+), 7 deletions(-)

--
2.4.3

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

* [RESEND PATCH v2 v4.1-rc8 1/2] drm: prime: Honour O_RDWR during prime-handle-to-fd
  2015-06-19 13:52 ` Daniel Thompson
@ 2015-06-19 13:52   ` Daniel Thompson
  -1 siblings, 0 replies; 14+ messages in thread
From: Daniel Thompson @ 2015-06-19 13:52 UTC (permalink / raw)
  To: David Airlie
  Cc: Daniel Thompson, dri-devel, Rob Clark, Benjamin Gaignard,
	Damien Hobson-Garcia, linux-kernel, patches, linaro-kernel,
	John Stultz, Sumit Semwal

Currently DRM_IOCTL_PRIME_HANDLE_TO_FD rejects all flags except
(DRM|O)_CLOEXEC making it difficult (maybe impossible) for userspace
to mmap() the resulting dma-buf even when this is supported by the
DRM driver.

It is trivial to relax the restriction and permit read/write access.
This is safe because the flags are seldom touched by drm; mostly they
are passed verbatim to dma_buf calls.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---
 drivers/gpu/drm/drm_prime.c | 9 +++------
 include/uapi/drm/drm.h      | 1 +
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 7fec191b45f7..6d2cf4fb4038 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -331,7 +331,7 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
  * drm_gem_prime_export - helper library implementation of the export callback
  * @dev: drm_device to export from
  * @obj: GEM object to export
- * @flags: flags like DRM_CLOEXEC
+ * @flags: flags like DRM_CLOEXEC and DRM_RDWR
  *
  * This is the implementation of the gem_prime_export functions for GEM drivers
  * using the PRIME helpers.
@@ -639,14 +639,11 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
 		return -ENOSYS;
 
 	/* check flags are valid */
-	if (args->flags & ~DRM_CLOEXEC)
+	if (args->flags & ~(DRM_CLOEXEC | DRM_RDWR))
 		return -EINVAL;
 
-	/* we only want to pass DRM_CLOEXEC which is == O_CLOEXEC */
-	flags = args->flags & DRM_CLOEXEC;
-
 	return dev->driver->prime_handle_to_fd(dev, file_priv,
-			args->handle, flags, &args->fd);
+			args->handle, args->flags, &args->fd);
 }
 
 int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index ff6ef62d084b..092fe3fa8ec0 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -668,6 +668,7 @@ struct drm_set_client_cap {
 	__u64 value;
 };
 
+#define DRM_RDWR O_RDWR
 #define DRM_CLOEXEC O_CLOEXEC
 struct drm_prime_handle {
 	__u32 handle;
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

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

* [RESEND PATCH v2 v4.1-rc8 1/2] drm: prime: Honour O_RDWR during prime-handle-to-fd
@ 2015-06-19 13:52   ` Daniel Thompson
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Thompson @ 2015-06-19 13:52 UTC (permalink / raw)
  To: David Airlie
  Cc: Daniel Thompson, dri-devel, Rob Clark, Benjamin Gaignard,
	Damien Hobson-Garcia, linux-kernel, patches, linaro-kernel,
	John Stultz, Sumit Semwal

Currently DRM_IOCTL_PRIME_HANDLE_TO_FD rejects all flags except
(DRM|O)_CLOEXEC making it difficult (maybe impossible) for userspace
to mmap() the resulting dma-buf even when this is supported by the
DRM driver.

It is trivial to relax the restriction and permit read/write access.
This is safe because the flags are seldom touched by drm; mostly they
are passed verbatim to dma_buf calls.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---
 drivers/gpu/drm/drm_prime.c | 9 +++------
 include/uapi/drm/drm.h      | 1 +
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 7fec191b45f7..6d2cf4fb4038 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -331,7 +331,7 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
  * drm_gem_prime_export - helper library implementation of the export callback
  * @dev: drm_device to export from
  * @obj: GEM object to export
- * @flags: flags like DRM_CLOEXEC
+ * @flags: flags like DRM_CLOEXEC and DRM_RDWR
  *
  * This is the implementation of the gem_prime_export functions for GEM drivers
  * using the PRIME helpers.
@@ -639,14 +639,11 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
 		return -ENOSYS;
 
 	/* check flags are valid */
-	if (args->flags & ~DRM_CLOEXEC)
+	if (args->flags & ~(DRM_CLOEXEC | DRM_RDWR))
 		return -EINVAL;
 
-	/* we only want to pass DRM_CLOEXEC which is == O_CLOEXEC */
-	flags = args->flags & DRM_CLOEXEC;
-
 	return dev->driver->prime_handle_to_fd(dev, file_priv,
-			args->handle, flags, &args->fd);
+			args->handle, args->flags, &args->fd);
 }
 
 int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index ff6ef62d084b..092fe3fa8ec0 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -668,6 +668,7 @@ struct drm_set_client_cap {
 	__u64 value;
 };
 
+#define DRM_RDWR O_RDWR
 #define DRM_CLOEXEC O_CLOEXEC
 struct drm_prime_handle {
 	__u32 handle;
-- 
2.4.3

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

* [RESEND PATCH v2 v4.1-rc8 2/2] drm: prime: Document gem_prime_mmap
  2015-06-19 13:52 ` Daniel Thompson
@ 2015-06-19 13:52   ` Daniel Thompson
  -1 siblings, 0 replies; 14+ messages in thread
From: Daniel Thompson @ 2015-06-19 13:52 UTC (permalink / raw)
  To: David Airlie
  Cc: Daniel Thompson, dri-devel, Rob Clark, Benjamin Gaignard,
	Damien Hobson-Garcia, linux-kernel, patches, linaro-kernel,
	John Stultz, Sumit Semwal

gem_prime_map is not currently described in the DRM manual, lets document
it.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---
 drivers/gpu/drm/drm_prime.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 6d2cf4fb4038..3b40a415f45d 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -309,7 +309,7 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
  * Drivers can implement @gem_prime_export and @gem_prime_import in terms of
  * simpler APIs by using the helper functions @drm_gem_prime_export and
  * @drm_gem_prime_import.  These functions implement dma-buf support in terms of
- * five lower-level driver callbacks:
+ * six lower-level driver callbacks:
  *
  * Export callbacks:
  *
@@ -321,6 +321,8 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
  *
  *  - @gem_prime_vunmap: vunmap a buffer exported by your driver
  *
+ *  - @gem_prime_mmap (optional): mmap a buffer exported by your driver
+ *
  * Import callback:
  *
  *  - @gem_prime_import_sg_table (import): produce a GEM object from another
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

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

* [RESEND PATCH v2 v4.1-rc8 2/2] drm: prime: Document gem_prime_mmap
@ 2015-06-19 13:52   ` Daniel Thompson
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Thompson @ 2015-06-19 13:52 UTC (permalink / raw)
  To: David Airlie
  Cc: Daniel Thompson, dri-devel, Rob Clark, Benjamin Gaignard,
	Damien Hobson-Garcia, linux-kernel, patches, linaro-kernel,
	John Stultz, Sumit Semwal

gem_prime_map is not currently described in the DRM manual, lets document
it.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---
 drivers/gpu/drm/drm_prime.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 6d2cf4fb4038..3b40a415f45d 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -309,7 +309,7 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
  * Drivers can implement @gem_prime_export and @gem_prime_import in terms of
  * simpler APIs by using the helper functions @drm_gem_prime_export and
  * @drm_gem_prime_import.  These functions implement dma-buf support in terms of
- * five lower-level driver callbacks:
+ * six lower-level driver callbacks:
  *
  * Export callbacks:
  *
@@ -321,6 +321,8 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
  *
  *  - @gem_prime_vunmap: vunmap a buffer exported by your driver
  *
+ *  - @gem_prime_mmap (optional): mmap a buffer exported by your driver
+ *
  * Import callback:
  *
  *  - @gem_prime_import_sg_table (import): produce a GEM object from another
-- 
2.4.3

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

* Re: [RESEND PATCH v2 v4.1-rc8 0/2] drm: prime: Allow exported dma-bufs to be mapped
  2015-06-19 13:52 ` Daniel Thompson
@ 2015-06-19 15:48   ` Daniel Vetter
  -1 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2015-06-19 15:48 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: David Airlie, linaro-kernel, patches, linux-kernel, dri-devel,
	Benjamin Gaignard, Damien Hobson-Garcia

On Fri, Jun 19, 2015 at 02:52:27PM +0100, Daniel Thompson wrote:
> This patch set started out as a single patch with a trivial bit of
> boilerplate to add dmabuf mmap support to the msm driver. However Rob
> Clark pointed out that, rather than keep one of the tricks I had used, it
> would be better to change the helpers resulting in this series.
> 
> I've tested this both with a rather hacked about Android userspace
> and with a fairly small test case run from debian. Both bits of code
> currently use dumb buffers.
> 
> Thanks to Benjamin Gaignard for his help in finding this bit of code and
> to Damien Hobson-Garcia for pointing out that I'd forgotten (since 3.18)
> to RESEND these patches.
> 
> Dave: I guess its probably too late in the dev. cycle to take this code
>       but don't worry, I will try really hard to remember to RESEND it
>       for 4.2. ;-)
> 
> v2:
> 
> * Modified DRM_PRIME_HANDLE_TO_FD to honour the O_RDRW from the user
>   and removed code to workaround this from the sti driver (Rob Clark).
> 
> * Added a patch to (rather spartanly) document gem_prime_mmap. Only
>   tacked into this series 'cos I spotted it was missing when I was
>   checking whether I needed to describe DRM_RDRW anywhere.

Oh hornets nest since I just screamed around again against drm prime mmap
support ;-) Imo before we expose this for real we really need to somehow
figure out what to do about cache coherency. Some intel folks are looking
into adding suitable ioctls to the dma-buf fd to make this possible. I
think we should wait with enabling drm prime mmaping before that's
resolved somehow. I'll point them at your patches though to make sure they
don't reinvent this wheel here.
-Daniel

> 
> 
> Daniel Thompson (2):
>   drm: prime: Honour O_RDWR during prime-handle-to-fd
>   drm: prime: Document gem_prime_mmap
> 
>  drivers/gpu/drm/drm_prime.c | 13 ++++++-------
>  include/uapi/drm/drm.h      |  1 +
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> --
> 2.4.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

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

* Re: [RESEND PATCH v2 v4.1-rc8 0/2] drm: prime: Allow exported dma-bufs to be mapped
@ 2015-06-19 15:48   ` Daniel Vetter
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2015-06-19 15:48 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: linaro-kernel, patches, linux-kernel, dri-devel,
	Benjamin Gaignard, Damien Hobson-Garcia

On Fri, Jun 19, 2015 at 02:52:27PM +0100, Daniel Thompson wrote:
> This patch set started out as a single patch with a trivial bit of
> boilerplate to add dmabuf mmap support to the msm driver. However Rob
> Clark pointed out that, rather than keep one of the tricks I had used, it
> would be better to change the helpers resulting in this series.
> 
> I've tested this both with a rather hacked about Android userspace
> and with a fairly small test case run from debian. Both bits of code
> currently use dumb buffers.
> 
> Thanks to Benjamin Gaignard for his help in finding this bit of code and
> to Damien Hobson-Garcia for pointing out that I'd forgotten (since 3.18)
> to RESEND these patches.
> 
> Dave: I guess its probably too late in the dev. cycle to take this code
>       but don't worry, I will try really hard to remember to RESEND it
>       for 4.2. ;-)
> 
> v2:
> 
> * Modified DRM_PRIME_HANDLE_TO_FD to honour the O_RDRW from the user
>   and removed code to workaround this from the sti driver (Rob Clark).
> 
> * Added a patch to (rather spartanly) document gem_prime_mmap. Only
>   tacked into this series 'cos I spotted it was missing when I was
>   checking whether I needed to describe DRM_RDRW anywhere.

Oh hornets nest since I just screamed around again against drm prime mmap
support ;-) Imo before we expose this for real we really need to somehow
figure out what to do about cache coherency. Some intel folks are looking
into adding suitable ioctls to the dma-buf fd to make this possible. I
think we should wait with enabling drm prime mmaping before that's
resolved somehow. I'll point them at your patches though to make sure they
don't reinvent this wheel here.
-Daniel

> 
> 
> Daniel Thompson (2):
>   drm: prime: Honour O_RDWR during prime-handle-to-fd
>   drm: prime: Document gem_prime_mmap
> 
>  drivers/gpu/drm/drm_prime.c | 13 ++++++-------
>  include/uapi/drm/drm.h      |  1 +
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> --
> 2.4.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [RESEND PATCH v2 v4.1-rc8 1/2] drm: prime: Honour O_RDWR during prime-handle-to-fd
  2015-06-19 13:52   ` Daniel Thompson
@ 2015-06-19 15:49     ` Daniel Vetter
  -1 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2015-06-19 15:49 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: David Airlie, linaro-kernel, patches, linux-kernel, dri-devel,
	Benjamin Gaignard, Damien Hobson-Garcia

On Fri, Jun 19, 2015 at 02:52:28PM +0100, Daniel Thompson wrote:
> Currently DRM_IOCTL_PRIME_HANDLE_TO_FD rejects all flags except
> (DRM|O)_CLOEXEC making it difficult (maybe impossible) for userspace
> to mmap() the resulting dma-buf even when this is supported by the
> DRM driver.
> 
> It is trivial to relax the restriction and permit read/write access.
> This is safe because the flags are seldom touched by drm; mostly they
> are passed verbatim to dma_buf calls.
> 
> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---
>  drivers/gpu/drm/drm_prime.c | 9 +++------
>  include/uapi/drm/drm.h      | 1 +
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 7fec191b45f7..6d2cf4fb4038 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -331,7 +331,7 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
>   * drm_gem_prime_export - helper library implementation of the export callback
>   * @dev: drm_device to export from
>   * @obj: GEM object to export
> - * @flags: flags like DRM_CLOEXEC
> + * @flags: flags like DRM_CLOEXEC and DRM_RDWR
>   *
>   * This is the implementation of the gem_prime_export functions for GEM drivers
>   * using the PRIME helpers.
> @@ -639,14 +639,11 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
>  		return -ENOSYS;
>  
>  	/* check flags are valid */
> -	if (args->flags & ~DRM_CLOEXEC)
> +	if (args->flags & ~(DRM_CLOEXEC | DRM_RDWR))
>  		return -EINVAL;

I think we should reject DRM_RDWR if there's no mmap implementation in the
underlying dma-buf vfunc table. Or in the gem version of those. Otherwise
looks ok to me, if we first resolve the dma-buf userspace mmap coherency
issue.
-Daniel

>  
> -	/* we only want to pass DRM_CLOEXEC which is == O_CLOEXEC */
> -	flags = args->flags & DRM_CLOEXEC;
> -
>  	return dev->driver->prime_handle_to_fd(dev, file_priv,
> -			args->handle, flags, &args->fd);
> +			args->handle, args->flags, &args->fd);
>  }
>  
>  int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index ff6ef62d084b..092fe3fa8ec0 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -668,6 +668,7 @@ struct drm_set_client_cap {
>  	__u64 value;
>  };
>  
> +#define DRM_RDWR O_RDWR
>  #define DRM_CLOEXEC O_CLOEXEC
>  struct drm_prime_handle {
>  	__u32 handle;
> -- 
> 2.4.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

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

* Re: [RESEND PATCH v2 v4.1-rc8 1/2] drm: prime: Honour O_RDWR during prime-handle-to-fd
@ 2015-06-19 15:49     ` Daniel Vetter
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2015-06-19 15:49 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: linaro-kernel, patches, linux-kernel, dri-devel,
	Benjamin Gaignard, Damien Hobson-Garcia

On Fri, Jun 19, 2015 at 02:52:28PM +0100, Daniel Thompson wrote:
> Currently DRM_IOCTL_PRIME_HANDLE_TO_FD rejects all flags except
> (DRM|O)_CLOEXEC making it difficult (maybe impossible) for userspace
> to mmap() the resulting dma-buf even when this is supported by the
> DRM driver.
> 
> It is trivial to relax the restriction and permit read/write access.
> This is safe because the flags are seldom touched by drm; mostly they
> are passed verbatim to dma_buf calls.
> 
> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---
>  drivers/gpu/drm/drm_prime.c | 9 +++------
>  include/uapi/drm/drm.h      | 1 +
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 7fec191b45f7..6d2cf4fb4038 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -331,7 +331,7 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
>   * drm_gem_prime_export - helper library implementation of the export callback
>   * @dev: drm_device to export from
>   * @obj: GEM object to export
> - * @flags: flags like DRM_CLOEXEC
> + * @flags: flags like DRM_CLOEXEC and DRM_RDWR
>   *
>   * This is the implementation of the gem_prime_export functions for GEM drivers
>   * using the PRIME helpers.
> @@ -639,14 +639,11 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
>  		return -ENOSYS;
>  
>  	/* check flags are valid */
> -	if (args->flags & ~DRM_CLOEXEC)
> +	if (args->flags & ~(DRM_CLOEXEC | DRM_RDWR))
>  		return -EINVAL;

I think we should reject DRM_RDWR if there's no mmap implementation in the
underlying dma-buf vfunc table. Or in the gem version of those. Otherwise
looks ok to me, if we first resolve the dma-buf userspace mmap coherency
issue.
-Daniel

>  
> -	/* we only want to pass DRM_CLOEXEC which is == O_CLOEXEC */
> -	flags = args->flags & DRM_CLOEXEC;
> -
>  	return dev->driver->prime_handle_to_fd(dev, file_priv,
> -			args->handle, flags, &args->fd);
> +			args->handle, args->flags, &args->fd);
>  }
>  
>  int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index ff6ef62d084b..092fe3fa8ec0 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -668,6 +668,7 @@ struct drm_set_client_cap {
>  	__u64 value;
>  };
>  
> +#define DRM_RDWR O_RDWR
>  #define DRM_CLOEXEC O_CLOEXEC
>  struct drm_prime_handle {
>  	__u32 handle;
> -- 
> 2.4.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [RESEND PATCH v2 v4.1-rc8 2/2] drm: prime: Document gem_prime_mmap
  2015-06-19 13:52   ` Daniel Thompson
@ 2015-06-19 15:50     ` Daniel Vetter
  -1 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2015-06-19 15:50 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: David Airlie, linaro-kernel, patches, linux-kernel, dri-devel,
	Benjamin Gaignard, Damien Hobson-Garcia

On Fri, Jun 19, 2015 at 02:52:29PM +0100, Daniel Thompson wrote:
> gem_prime_map is not currently described in the DRM manual, lets document
> it.
> 
> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>

Applied to drm-misc, thanks.
-Daniel

> ---
>  drivers/gpu/drm/drm_prime.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 6d2cf4fb4038..3b40a415f45d 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -309,7 +309,7 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
>   * Drivers can implement @gem_prime_export and @gem_prime_import in terms of
>   * simpler APIs by using the helper functions @drm_gem_prime_export and
>   * @drm_gem_prime_import.  These functions implement dma-buf support in terms of
> - * five lower-level driver callbacks:
> + * six lower-level driver callbacks:
>   *
>   * Export callbacks:
>   *
> @@ -321,6 +321,8 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
>   *
>   *  - @gem_prime_vunmap: vunmap a buffer exported by your driver
>   *
> + *  - @gem_prime_mmap (optional): mmap a buffer exported by your driver
> + *
>   * Import callback:
>   *
>   *  - @gem_prime_import_sg_table (import): produce a GEM object from another
> -- 
> 2.4.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

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

* Re: [RESEND PATCH v2 v4.1-rc8 2/2] drm: prime: Document gem_prime_mmap
@ 2015-06-19 15:50     ` Daniel Vetter
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2015-06-19 15:50 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: linaro-kernel, patches, linux-kernel, dri-devel,
	Benjamin Gaignard, Damien Hobson-Garcia

On Fri, Jun 19, 2015 at 02:52:29PM +0100, Daniel Thompson wrote:
> gem_prime_map is not currently described in the DRM manual, lets document
> it.
> 
> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>

Applied to drm-misc, thanks.
-Daniel

> ---
>  drivers/gpu/drm/drm_prime.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 6d2cf4fb4038..3b40a415f45d 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -309,7 +309,7 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
>   * Drivers can implement @gem_prime_export and @gem_prime_import in terms of
>   * simpler APIs by using the helper functions @drm_gem_prime_export and
>   * @drm_gem_prime_import.  These functions implement dma-buf support in terms of
> - * five lower-level driver callbacks:
> + * six lower-level driver callbacks:
>   *
>   * Export callbacks:
>   *
> @@ -321,6 +321,8 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
>   *
>   *  - @gem_prime_vunmap: vunmap a buffer exported by your driver
>   *
> + *  - @gem_prime_mmap (optional): mmap a buffer exported by your driver
> + *
>   * Import callback:
>   *
>   *  - @gem_prime_import_sg_table (import): produce a GEM object from another
> -- 
> 2.4.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [RESEND PATCH v2 v4.1-rc8 0/2] drm: prime: Allow exported dma-bufs to be mapped
  2015-06-19 15:48   ` Daniel Vetter
@ 2015-06-19 20:24     ` Benjamin Gaignard
  -1 siblings, 0 replies; 14+ messages in thread
From: Benjamin Gaignard @ 2015-06-19 20:24 UTC (permalink / raw)
  To: Daniel Thompson, David Airlie, Linaro Kernel Mailman List,
	Linux Kernel Mailing List, dri-devel, Benjamin Gaignard,
	Damien Hobson-Garcia

I'm far of being an expert in cache coherency but, from past experiences,
I have notice that ION cache management give good performances and is
simple to understand.

ION mark as "dirty" the mapped pages in wm_fault function and check this
flag while mapping the buffer in kernel space.
If the flag is set it call dma_sync_sg_for_device() with the good direction.

ION allow with an iotcl to do explicit or implicit cache management but, for
me, it add complexity to the code when implicit mode should be the
preferred one.

Benjamin


2015-06-19 17:48 GMT+02:00 Daniel Vetter <daniel@ffwll.ch>:
> On Fri, Jun 19, 2015 at 02:52:27PM +0100, Daniel Thompson wrote:
>> This patch set started out as a single patch with a trivial bit of
>> boilerplate to add dmabuf mmap support to the msm driver. However Rob
>> Clark pointed out that, rather than keep one of the tricks I had used, it
>> would be better to change the helpers resulting in this series.
>>
>> I've tested this both with a rather hacked about Android userspace
>> and with a fairly small test case run from debian. Both bits of code
>> currently use dumb buffers.
>>
>> Thanks to Benjamin Gaignard for his help in finding this bit of code and
>> to Damien Hobson-Garcia for pointing out that I'd forgotten (since 3.18)
>> to RESEND these patches.
>>
>> Dave: I guess its probably too late in the dev. cycle to take this code
>>       but don't worry, I will try really hard to remember to RESEND it
>>       for 4.2. ;-)
>>
>> v2:
>>
>> * Modified DRM_PRIME_HANDLE_TO_FD to honour the O_RDRW from the user
>>   and removed code to workaround this from the sti driver (Rob Clark).
>>
>> * Added a patch to (rather spartanly) document gem_prime_mmap. Only
>>   tacked into this series 'cos I spotted it was missing when I was
>>   checking whether I needed to describe DRM_RDRW anywhere.
>
> Oh hornets nest since I just screamed around again against drm prime mmap
> support ;-) Imo before we expose this for real we really need to somehow
> figure out what to do about cache coherency. Some intel folks are looking
> into adding suitable ioctls to the dma-buf fd to make this possible. I
> think we should wait with enabling drm prime mmaping before that's
> resolved somehow. I'll point them at your patches though to make sure they
> don't reinvent this wheel here.
> -Daniel
>
>>
>>
>> Daniel Thompson (2):
>>   drm: prime: Honour O_RDWR during prime-handle-to-fd
>>   drm: prime: Document gem_prime_mmap
>>
>>  drivers/gpu/drm/drm_prime.c | 13 ++++++-------
>>  include/uapi/drm/drm.h      |  1 +
>>  2 files changed, 7 insertions(+), 7 deletions(-)
>>
>> --
>> 2.4.3
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch



-- 
Benjamin Gaignard

Graphic Working Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

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

* Re: [RESEND PATCH v2 v4.1-rc8 0/2] drm: prime: Allow exported dma-bufs to be mapped
@ 2015-06-19 20:24     ` Benjamin Gaignard
  0 siblings, 0 replies; 14+ messages in thread
From: Benjamin Gaignard @ 2015-06-19 20:24 UTC (permalink / raw)
  To: Daniel Thompson, David Airlie, Linaro Kernel Mailman List,
	Linux Kernel Mailing List, dri-devel, Benjamin Gaignard,
	Damien Hobson-Garcia

I'm far of being an expert in cache coherency but, from past experiences,
I have notice that ION cache management give good performances and is
simple to understand.

ION mark as "dirty" the mapped pages in wm_fault function and check this
flag while mapping the buffer in kernel space.
If the flag is set it call dma_sync_sg_for_device() with the good direction.

ION allow with an iotcl to do explicit or implicit cache management but, for
me, it add complexity to the code when implicit mode should be the
preferred one.

Benjamin


2015-06-19 17:48 GMT+02:00 Daniel Vetter <daniel@ffwll.ch>:
> On Fri, Jun 19, 2015 at 02:52:27PM +0100, Daniel Thompson wrote:
>> This patch set started out as a single patch with a trivial bit of
>> boilerplate to add dmabuf mmap support to the msm driver. However Rob
>> Clark pointed out that, rather than keep one of the tricks I had used, it
>> would be better to change the helpers resulting in this series.
>>
>> I've tested this both with a rather hacked about Android userspace
>> and with a fairly small test case run from debian. Both bits of code
>> currently use dumb buffers.
>>
>> Thanks to Benjamin Gaignard for his help in finding this bit of code and
>> to Damien Hobson-Garcia for pointing out that I'd forgotten (since 3.18)
>> to RESEND these patches.
>>
>> Dave: I guess its probably too late in the dev. cycle to take this code
>>       but don't worry, I will try really hard to remember to RESEND it
>>       for 4.2. ;-)
>>
>> v2:
>>
>> * Modified DRM_PRIME_HANDLE_TO_FD to honour the O_RDRW from the user
>>   and removed code to workaround this from the sti driver (Rob Clark).
>>
>> * Added a patch to (rather spartanly) document gem_prime_mmap. Only
>>   tacked into this series 'cos I spotted it was missing when I was
>>   checking whether I needed to describe DRM_RDRW anywhere.
>
> Oh hornets nest since I just screamed around again against drm prime mmap
> support ;-) Imo before we expose this for real we really need to somehow
> figure out what to do about cache coherency. Some intel folks are looking
> into adding suitable ioctls to the dma-buf fd to make this possible. I
> think we should wait with enabling drm prime mmaping before that's
> resolved somehow. I'll point them at your patches though to make sure they
> don't reinvent this wheel here.
> -Daniel
>
>>
>>
>> Daniel Thompson (2):
>>   drm: prime: Honour O_RDWR during prime-handle-to-fd
>>   drm: prime: Document gem_prime_mmap
>>
>>  drivers/gpu/drm/drm_prime.c | 13 ++++++-------
>>  include/uapi/drm/drm.h      |  1 +
>>  2 files changed, 7 insertions(+), 7 deletions(-)
>>
>> --
>> 2.4.3
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch



-- 
Benjamin Gaignard

Graphic Working Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

end of thread, other threads:[~2015-06-19 20:25 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-19 13:52 [RESEND PATCH v2 v4.1-rc8 0/2] drm: prime: Allow exported dma-bufs to be mapped Daniel Thompson
2015-06-19 13:52 ` Daniel Thompson
2015-06-19 13:52 ` [RESEND PATCH v2 v4.1-rc8 1/2] drm: prime: Honour O_RDWR during prime-handle-to-fd Daniel Thompson
2015-06-19 13:52   ` Daniel Thompson
2015-06-19 15:49   ` Daniel Vetter
2015-06-19 15:49     ` Daniel Vetter
2015-06-19 13:52 ` [RESEND PATCH v2 v4.1-rc8 2/2] drm: prime: Document gem_prime_mmap Daniel Thompson
2015-06-19 13:52   ` Daniel Thompson
2015-06-19 15:50   ` Daniel Vetter
2015-06-19 15:50     ` Daniel Vetter
2015-06-19 15:48 ` [RESEND PATCH v2 v4.1-rc8 0/2] drm: prime: Allow exported dma-bufs to be mapped Daniel Vetter
2015-06-19 15:48   ` Daniel Vetter
2015-06-19 20:24   ` Benjamin Gaignard
2015-06-19 20:24     ` Benjamin Gaignard

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.