Re: [PATCH 2/2] drm/prime: Update docs
On Wed, Jun 19, 2019 at 02:43:20PM +0200, Noralf Trønnes wrote:
>
>
> Den 18.06.2019 11.20, skrev Daniel Vetter:
> > Yes this is a bit a big patch, but since it's essentially a complete
> > rewrite of all the prime docs I didn't see how to better split it up.
> >
> > Changes:
> > - Consistently point to drm_gem_object_funcs as the preferred hooks,
> > where applicable.
> >
> > - Document all the hooks in _driver that lacked kerneldoc.
> >
> > - Completely new overview section, which now also includes the cleaned
> > up lifetime/reference counting subchapter. I also mentioned the weak
> > references in there due to the lookup caches.
> >
> > - Completely rewritten helper intro section, highlight the
> > import/export related functionality.
> >
> > - Polish for all the functions and more cross references.
> >
> > I also sprinkled a bunch of todos all over.
> >
> > Most important: 0 code changes in here. The cleanup motivated by
> > reading and improving all this will follow later on.
> >
> > v2: Actually update the prime helper docs. Plus add a few FIXMEs that
> > I won't address right away in subsequent cleanup patches.
> >
> > v3:
> > - Split out the function moving. This patch is now exclusively
> > documentation changes.
> > - Typos and nits (Sam).
> >
> > Cc: Sam Ravnborg
> > Cc: Eric Anholt
> > Cc: Emil Velikov
> > Signed-off-by: Daniel Vetter
> > ---
> > Documentation/gpu/drm-mm.rst | 40 +--
> > drivers/gpu/drm/drm_prime.c | 197 +--
> > include/drm/drm_drv.h| 104 +++---
> > include/drm/drm_gem.h| 18 ++--
> > 4 files changed, 226 insertions(+), 133 deletions(-)
> >
>
>
>
> > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> > index 68b4de85370c..2234206288fa 100644
> > --- a/drivers/gpu/drm/drm_prime.c
> > +++ b/drivers/gpu/drm/drm_prime.c
> > @@ -38,47 +38,53 @@
> >
> > #include "drm_internal.h"
> >
> > -/*
> > - * DMA-BUF/GEM Object references and lifetime overview:
> > - *
> > - * On the export the dma_buf holds a reference to the exporting GEM
> > - * object. It takes this reference in handle_to_fd_ioctl, when it
> > - * first calls .prime_export and stores the exporting GEM object in
> > - * the dma_buf priv. This reference needs to be released when the
> > - * final reference to the _buf itself is dropped and its
> > - * _buf_ops.release function is called. For GEM-based drivers,
> > - * the dma_buf should be exported using drm_gem_dmabuf_export() and
> > - * then released by drm_gem_dmabuf_release().
> > - *
> > - * On the import the importing GEM object holds a reference to the
> > - * dma_buf (which in turn holds a ref to the exporting GEM object).
> > - * It takes that reference in the fd_to_handle ioctl.
> > - * It calls dma_buf_get, creates an attachment to it and stores the
> > - * attachment in the GEM object. When this attachment is destroyed
> > - * when the imported object is destroyed, we remove the attachment
> > - * and drop the reference to the dma_buf.
> > - *
> > - * When all the references to the _buf are dropped, i.e. when
> > - * userspace has closed both handles to the imported GEM object (through
> > the
> > - * FD_TO_HANDLE IOCTL) and closed the file descriptor of the exported
> > - * (through the HANDLE_TO_FD IOCTL) dma_buf, and all kernel-internal
> > references
> > - * are also gone, then the dma_buf gets destroyed. This can also happen
> > as a
> > - * part of the clean up procedure in the drm_release() function if
> > userspace
> > - * fails to properly clean up. Note that both the kernel and userspace (by
> > - * keeeping the PRIME file descriptors open) can hold references onto a
> > - * _buf.
> > - *
> > - * Thus the chain of references always flows in one direction
> > - * (avoiding loops): importing_gem -> dmabuf -> exporting_gem
> > - *
> > - * Self-importing: if userspace is using PRIME as a replacement for flink
> > - * then it will get a fd->handle request for a GEM object that it created.
> > - * Drivers should detect this situation and return back the gem object
> > - * from the dma-buf private. Prime will do this automatically for drivers
> > that
> > - * use the drm_gem_prime_{import,export} helpers.
> > - *
> > - * GEM struct _buf_ops symbols are now exported. They can be resued by
> > - * drivers which implement GEM interface.
> > +/**
> > + * DOC: overview and lifetime rules
> > + *
> > + * Similar to GEM global names, PRIME file descriptors are also used to
> > share
> > + * buffer objects across processes. They offer additional security: as file
> > + * descriptors must be explicitly sent over UNIX domain sockets to be
> > shared
> > + * between applications, they can't be guessed like the globally unique GEM
> > + * names.
> > + *
> > + * Drivers that support the PRIME API must set the DRIVER_PRIME bit in the
> > + * _driver.driver_features field, and implement the
> > + * _driver.prime_handle_to_fd and
Re: [PATCH 2/2] drm/prime: Update docs
Den 18.06.2019 11.20, skrev Daniel Vetter:
> Yes this is a bit a big patch, but since it's essentially a complete
> rewrite of all the prime docs I didn't see how to better split it up.
>
> Changes:
> - Consistently point to drm_gem_object_funcs as the preferred hooks,
> where applicable.
>
> - Document all the hooks in _driver that lacked kerneldoc.
>
> - Completely new overview section, which now also includes the cleaned
> up lifetime/reference counting subchapter. I also mentioned the weak
> references in there due to the lookup caches.
>
> - Completely rewritten helper intro section, highlight the
> import/export related functionality.
>
> - Polish for all the functions and more cross references.
>
> I also sprinkled a bunch of todos all over.
>
> Most important: 0 code changes in here. The cleanup motivated by
> reading and improving all this will follow later on.
>
> v2: Actually update the prime helper docs. Plus add a few FIXMEs that
> I won't address right away in subsequent cleanup patches.
>
> v3:
> - Split out the function moving. This patch is now exclusively
> documentation changes.
> - Typos and nits (Sam).
>
> Cc: Sam Ravnborg
> Cc: Eric Anholt
> Cc: Emil Velikov
> Signed-off-by: Daniel Vetter
> ---
> Documentation/gpu/drm-mm.rst | 40 +--
> drivers/gpu/drm/drm_prime.c | 197 +--
> include/drm/drm_drv.h| 104 +++---
> include/drm/drm_gem.h| 18 ++--
> 4 files changed, 226 insertions(+), 133 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 68b4de85370c..2234206288fa 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -38,47 +38,53 @@
>
> #include "drm_internal.h"
>
> -/*
> - * DMA-BUF/GEM Object references and lifetime overview:
> - *
> - * On the export the dma_buf holds a reference to the exporting GEM
> - * object. It takes this reference in handle_to_fd_ioctl, when it
> - * first calls .prime_export and stores the exporting GEM object in
> - * the dma_buf priv. This reference needs to be released when the
> - * final reference to the _buf itself is dropped and its
> - * _buf_ops.release function is called. For GEM-based drivers,
> - * the dma_buf should be exported using drm_gem_dmabuf_export() and
> - * then released by drm_gem_dmabuf_release().
> - *
> - * On the import the importing GEM object holds a reference to the
> - * dma_buf (which in turn holds a ref to the exporting GEM object).
> - * It takes that reference in the fd_to_handle ioctl.
> - * It calls dma_buf_get, creates an attachment to it and stores the
> - * attachment in the GEM object. When this attachment is destroyed
> - * when the imported object is destroyed, we remove the attachment
> - * and drop the reference to the dma_buf.
> - *
> - * When all the references to the _buf are dropped, i.e. when
> - * userspace has closed both handles to the imported GEM object (through the
> - * FD_TO_HANDLE IOCTL) and closed the file descriptor of the exported
> - * (through the HANDLE_TO_FD IOCTL) dma_buf, and all kernel-internal
> references
> - * are also gone, then the dma_buf gets destroyed. This can also happen as a
> - * part of the clean up procedure in the drm_release() function if userspace
> - * fails to properly clean up. Note that both the kernel and userspace (by
> - * keeeping the PRIME file descriptors open) can hold references onto a
> - * _buf.
> - *
> - * Thus the chain of references always flows in one direction
> - * (avoiding loops): importing_gem -> dmabuf -> exporting_gem
> - *
> - * Self-importing: if userspace is using PRIME as a replacement for flink
> - * then it will get a fd->handle request for a GEM object that it created.
> - * Drivers should detect this situation and return back the gem object
> - * from the dma-buf private. Prime will do this automatically for drivers
> that
> - * use the drm_gem_prime_{import,export} helpers.
> - *
> - * GEM struct _buf_ops symbols are now exported. They can be resued by
> - * drivers which implement GEM interface.
> +/**
> + * DOC: overview and lifetime rules
> + *
> + * Similar to GEM global names, PRIME file descriptors are also used to share
> + * buffer objects across processes. They offer additional security: as file
> + * descriptors must be explicitly sent over UNIX domain sockets to be shared
> + * between applications, they can't be guessed like the globally unique GEM
> + * names.
> + *
> + * Drivers that support the PRIME API must set the DRIVER_PRIME bit in the
> + * _driver.driver_features field, and implement the
> + * _driver.prime_handle_to_fd and _driver.prime_fd_to_handle
> operations.
> + * GEM based drivers must use drm_gem_prime_handle_to_fd() and
> + * drm_gem_prime_fd_to_handle() to implement these. For GEM based drivers the
> + * actual driver interfaces is provided through the
> _gem_object_funcs.export
> + * and _driver.gem_prime_import hooks.
> + *
Re: [PATCH 2/2] drm/prime: Update docs
On 2019/06/18, Daniel Vetter wrote: > Yes this is a bit a big patch, but since it's essentially a complete > rewrite of all the prime docs I didn't see how to better split it up. > > Changes: > - Consistently point to drm_gem_object_funcs as the preferred hooks, > where applicable. > > - Document all the hooks in _driver that lacked kerneldoc. > > - Completely new overview section, which now also includes the cleaned > up lifetime/reference counting subchapter. I also mentioned the weak > references in there due to the lookup caches. > > - Completely rewritten helper intro section, highlight the > import/export related functionality. > > - Polish for all the functions and more cross references. > > I also sprinkled a bunch of todos all over. > > Most important: 0 code changes in here. The cleanup motivated by > reading and improving all this will follow later on. > > v2: Actually update the prime helper docs. Plus add a few FIXMEs that > I won't address right away in subsequent cleanup patches. > > v3: > - Split out the function moving. This patch is now exclusively > documentation changes. > - Typos and nits (Sam). > > Cc: Sam Ravnborg > Cc: Eric Anholt > Cc: Emil Velikov > Signed-off-by: Daniel Vetter > --- Had a quick read and it looks reasonable. Acked-by: Emil Velikov HTH Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/prime: Update docs
Hi, > - Polish for all the functions and more cross references. That is nice, helps figurung how all this works together without wading through the source code (or at least less of it). For that kind of changes it is probably helpful for review to generate the diff with more context (say --unified=10), so it is easier to spot the function where this doc update belongs to. Acked-by: Gerd Hoffmann cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/prime: Update docs
On Tue, Jun 18, 2019 at 11:20 AM Daniel Vetter wrote: > > Yes this is a bit a big patch, but since it's essentially a complete > rewrite of all the prime docs I didn't see how to better split it up. > > Changes: > - Consistently point to drm_gem_object_funcs as the preferred hooks, > where applicable. > > - Document all the hooks in _driver that lacked kerneldoc. > > - Completely new overview section, which now also includes the cleaned > up lifetime/reference counting subchapter. I also mentioned the weak > references in there due to the lookup caches. > > - Completely rewritten helper intro section, highlight the > import/export related functionality. > > - Polish for all the functions and more cross references. > > I also sprinkled a bunch of todos all over. > > Most important: 0 code changes in here. The cleanup motivated by > reading and improving all this will follow later on. > > v2: Actually update the prime helper docs. Plus add a few FIXMEs that > I won't address right away in subsequent cleanup patches. > > v3: > - Split out the function moving. This patch is now exclusively > documentation changes. > - Typos and nits (Sam). > > Cc: Sam Ravnborg > Cc: Eric Anholt > Cc: Emil Velikov > Signed-off-by: Daniel Vetter Adding Thomas, Gerd & Noralf. I think you folks have looked most closely at this recently, I'd much appreciate some review on this and the previous patch. Thanks, Daniel > --- > Documentation/gpu/drm-mm.rst | 40 +-- > drivers/gpu/drm/drm_prime.c | 197 +-- > include/drm/drm_drv.h| 104 +++--- > include/drm/drm_gem.h| 18 ++-- > 4 files changed, 226 insertions(+), 133 deletions(-) > > diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst > index c8ebd4f66a6a..b664f054c259 100644 > --- a/Documentation/gpu/drm-mm.rst > +++ b/Documentation/gpu/drm-mm.rst > @@ -433,43 +433,11 @@ PRIME is the cross device buffer sharing framework in > drm, originally > created for the OPTIMUS range of multi-gpu platforms. To userspace PRIME > buffers are dma-buf based file descriptors. > > -Overview and Driver Interface > -- > +Overview and Lifetime Rules > +--- > > -Similar to GEM global names, PRIME file descriptors are also used to > -share buffer objects across processes. They offer additional security: > -as file descriptors must be explicitly sent over UNIX domain sockets to > -be shared between applications, they can't be guessed like the globally > -unique GEM names. > - > -Drivers that support the PRIME API must set the DRIVER_PRIME bit in the > -struct :c:type:`struct drm_driver ` > -driver_features field, and implement the prime_handle_to_fd and > -prime_fd_to_handle operations. > - > -int (\*prime_handle_to_fd)(struct drm_device \*dev, struct drm_file > -\*file_priv, uint32_t handle, uint32_t flags, int \*prime_fd); int > -(\*prime_fd_to_handle)(struct drm_device \*dev, struct drm_file > -\*file_priv, int prime_fd, uint32_t \*handle); Those two operations > -convert a handle to a PRIME file descriptor and vice versa. Drivers must > -use the kernel dma-buf buffer sharing framework to manage the PRIME file > -descriptors. Similar to the mode setting API PRIME is agnostic to the > -underlying buffer object manager, as long as handles are 32bit unsigned > -integers. > - > -While non-GEM drivers must implement the operations themselves, GEM > -drivers must use the :c:func:`drm_gem_prime_handle_to_fd()` and > -:c:func:`drm_gem_prime_fd_to_handle()` helper functions. Those > -helpers rely on the driver gem_prime_export and gem_prime_import > -operations to create a dma-buf instance from a GEM object (dma-buf > -exporter role) and to create a GEM object from a dma-buf instance > -(dma-buf importer role). > - > -struct dma_buf \* (\*gem_prime_export)(struct drm_device \*dev, > -struct drm_gem_object \*obj, int flags); struct drm_gem_object \* > -(\*gem_prime_import)(struct drm_device \*dev, struct dma_buf > -\*dma_buf); These two operations are mandatory for GEM drivers that > -support PRIME. > +.. kernel-doc:: drivers/gpu/drm/drm_prime.c > + :doc: overview and lifetime rules > > PRIME Helper Functions > -- > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index 68b4de85370c..2234206288fa 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -38,47 +38,53 @@ > > #include "drm_internal.h" > > -/* > - * DMA-BUF/GEM Object references and lifetime overview: > - * > - * On the export the dma_buf holds a reference to the exporting GEM > - * object. It takes this reference in handle_to_fd_ioctl, when it > - * first calls .prime_export and stores the exporting GEM object in > - * the dma_buf priv. This reference needs to be released when the > - * final reference to the _buf itself is dropped and its > - * _buf_ops.release function is called. For GEM-based drivers, > - * the
[PATCH 2/2] drm/prime: Update docs
Yes this is a bit a big patch, but since it's essentially a complete rewrite of all the prime docs I didn't see how to better split it up. Changes: - Consistently point to drm_gem_object_funcs as the preferred hooks, where applicable. - Document all the hooks in _driver that lacked kerneldoc. - Completely new overview section, which now also includes the cleaned up lifetime/reference counting subchapter. I also mentioned the weak references in there due to the lookup caches. - Completely rewritten helper intro section, highlight the import/export related functionality. - Polish for all the functions and more cross references. I also sprinkled a bunch of todos all over. Most important: 0 code changes in here. The cleanup motivated by reading and improving all this will follow later on. v2: Actually update the prime helper docs. Plus add a few FIXMEs that I won't address right away in subsequent cleanup patches. v3: - Split out the function moving. This patch is now exclusively documentation changes. - Typos and nits (Sam). Cc: Sam Ravnborg Cc: Eric Anholt Cc: Emil Velikov Signed-off-by: Daniel Vetter --- Documentation/gpu/drm-mm.rst | 40 +-- drivers/gpu/drm/drm_prime.c | 197 +-- include/drm/drm_drv.h| 104 +++--- include/drm/drm_gem.h| 18 ++-- 4 files changed, 226 insertions(+), 133 deletions(-) diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst index c8ebd4f66a6a..b664f054c259 100644 --- a/Documentation/gpu/drm-mm.rst +++ b/Documentation/gpu/drm-mm.rst @@ -433,43 +433,11 @@ PRIME is the cross device buffer sharing framework in drm, originally created for the OPTIMUS range of multi-gpu platforms. To userspace PRIME buffers are dma-buf based file descriptors. -Overview and Driver Interface -- +Overview and Lifetime Rules +--- -Similar to GEM global names, PRIME file descriptors are also used to -share buffer objects across processes. They offer additional security: -as file descriptors must be explicitly sent over UNIX domain sockets to -be shared between applications, they can't be guessed like the globally -unique GEM names. - -Drivers that support the PRIME API must set the DRIVER_PRIME bit in the -struct :c:type:`struct drm_driver ` -driver_features field, and implement the prime_handle_to_fd and -prime_fd_to_handle operations. - -int (\*prime_handle_to_fd)(struct drm_device \*dev, struct drm_file -\*file_priv, uint32_t handle, uint32_t flags, int \*prime_fd); int -(\*prime_fd_to_handle)(struct drm_device \*dev, struct drm_file -\*file_priv, int prime_fd, uint32_t \*handle); Those two operations -convert a handle to a PRIME file descriptor and vice versa. Drivers must -use the kernel dma-buf buffer sharing framework to manage the PRIME file -descriptors. Similar to the mode setting API PRIME is agnostic to the -underlying buffer object manager, as long as handles are 32bit unsigned -integers. - -While non-GEM drivers must implement the operations themselves, GEM -drivers must use the :c:func:`drm_gem_prime_handle_to_fd()` and -:c:func:`drm_gem_prime_fd_to_handle()` helper functions. Those -helpers rely on the driver gem_prime_export and gem_prime_import -operations to create a dma-buf instance from a GEM object (dma-buf -exporter role) and to create a GEM object from a dma-buf instance -(dma-buf importer role). - -struct dma_buf \* (\*gem_prime_export)(struct drm_device \*dev, -struct drm_gem_object \*obj, int flags); struct drm_gem_object \* -(\*gem_prime_import)(struct drm_device \*dev, struct dma_buf -\*dma_buf); These two operations are mandatory for GEM drivers that -support PRIME. +.. kernel-doc:: drivers/gpu/drm/drm_prime.c + :doc: overview and lifetime rules PRIME Helper Functions -- diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 68b4de85370c..2234206288fa 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -38,47 +38,53 @@ #include "drm_internal.h" -/* - * DMA-BUF/GEM Object references and lifetime overview: - * - * On the export the dma_buf holds a reference to the exporting GEM - * object. It takes this reference in handle_to_fd_ioctl, when it - * first calls .prime_export and stores the exporting GEM object in - * the dma_buf priv. This reference needs to be released when the - * final reference to the _buf itself is dropped and its - * _buf_ops.release function is called. For GEM-based drivers, - * the dma_buf should be exported using drm_gem_dmabuf_export() and - * then released by drm_gem_dmabuf_release(). - * - * On the import the importing GEM object holds a reference to the - * dma_buf (which in turn holds a ref to the exporting GEM object). - * It takes that reference in the fd_to_handle ioctl. - * It calls dma_buf_get, creates an attachment to it and stores the - * attachment in the GEM object. When this attachment is
