| Message ID | 20200511093554.211493-4-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | shmem helper untangling | expand |
Am 11.05.20 um 11:35 schrieb Daniel Vetter: > - Move the shmem helper section to the drm-mm.rst file, next to the > vram helpers. Makes a lot more sense there with the now wider scope. > Also, that's where the all the other backing storage stuff resides. > It's just the framebuffer helpers that should be in the kms helper > section. > > - Try to clarify which functiosn are for implementing > drm_gem_object_funcs, and which for drivers to call directly. At > least one driver screwed that up a bit. > > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Rob Herring <robh@kernel.org> > Cc: Noralf Trønnes <noralf@tronnes.org> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> See below for a suggestion on the help text. > --- > Documentation/gpu/drm-kms-helpers.rst | 12 -------- > Documentation/gpu/drm-mm.rst | 12 ++++++++ > drivers/gpu/drm/drm_gem_shmem_helper.c | 39 +++++++++++++++++++++----- > 3 files changed, 44 insertions(+), 19 deletions(-) > > diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst > index ee730457bf4e..b89ddd06dabb 100644 > --- a/Documentation/gpu/drm-kms-helpers.rst > +++ b/Documentation/gpu/drm-kms-helpers.rst > @@ -411,15 +411,3 @@ Legacy CRTC/Modeset Helper Functions Reference > > .. kernel-doc:: drivers/gpu/drm/drm_crtc_helper.c > :export: > - > -SHMEM GEM Helper Reference > -========================== > - > -.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c > - :doc: overview > - > -.. kernel-doc:: include/drm/drm_gem_shmem_helper.h > - :internal: > - > -.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c > - :export: > diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst > index 1839762044be..c01757b0ac25 100644 > --- a/Documentation/gpu/drm-mm.rst > +++ b/Documentation/gpu/drm-mm.rst > @@ -373,6 +373,18 @@ GEM CMA Helper Functions Reference > .. kernel-doc:: drivers/gpu/drm/drm_gem_cma_helper.c > :export: > > +GEM SHMEM Helper Function Reference > +----------------------------------- > + > +.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c > + :doc: overview > + > +.. kernel-doc:: include/drm/drm_gem_shmem_helper.h > + :internal: > + > +.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c > + :export: > + > GEM VRAM Helper Functions Reference > ----------------------------------- > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c > index df31e5782eed..2a70159d50ef 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -103,7 +103,8 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_create); > * @obj: GEM object to free > * > * This function cleans up the GEM object state and frees the memory used to > - * store the object itself. > + * store the object itself. It should be used to implement > + * &drm_gem_object_funcs.free. It should 'only' be used? Or maybe you can say that it should be used by drivers that don't implement struct drm_driver.gem_create_object. > */ > void drm_gem_shmem_free_object(struct drm_gem_object *obj) > { > @@ -214,7 +215,8 @@ EXPORT_SYMBOL(drm_gem_shmem_put_pages); > * @obj: GEM object > * > * This function makes sure the backing pages are pinned in memory while the > - * buffer is exported. > + * buffer is exported. It should only be used to implement > + * &drm_gem_object_funcs.pin. > * > * Returns: > * 0 on success or a negative error code on failure. > @@ -232,7 +234,7 @@ EXPORT_SYMBOL(drm_gem_shmem_pin); > * @obj: GEM object > * > * This function removes the requirement that the backing pages are pinned in > - * memory. > + * memory. It should only be used to implement &drm_gem_object_funcs.unpin. > */ > void drm_gem_shmem_unpin(struct drm_gem_object *obj) > { > @@ -285,8 +287,14 @@ static void *drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem) > * drm_gem_shmem_vmap - Create a virtual mapping for a shmem GEM object > * @shmem: shmem GEM object > * > - * This function makes sure that a virtual address exists for the buffer backing > - * the shmem GEM object. > + * This function makes sure that a contiguous kernel virtual address mapping > + * exists for the buffer backing the shmem GEM object. > + * > + * This function can be used to implement &drm_gem_object_funcs.vmap. But it can > + * also be called by drivers directly, in which case it will hide the > + * differences between dma-buf imported and natively allocated objects. > + * > + * Acquired mappings should be cleaned up by calling drm_gem_shmem_vunmap(). > * > * Returns: > * 0 on success or a negative error code on failure. > @@ -330,7 +338,13 @@ static void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem) > * drm_gem_shmem_vunmap - Unmap a virtual mapping fo a shmem GEM object > * @shmem: shmem GEM object > * > - * This function removes the virtual address when use count drops to zero. > + * This function cleans up a kernel virtual address mapping acquired by > + * drm_gem_shmem_vmap(). The mapping is only removed when the use count drops to > + * zero. > + * > + * This function can be used to implement &drm_gem_object_funcs.vmap. But it can > + * also be called by drivers directly, in which case it will hide the > + * differences between dma-buf imported and natively allocated objects. > */ > void drm_gem_shmem_vunmap(struct drm_gem_object *obj, void *vaddr) > { > @@ -559,6 +573,8 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_mmap); > * @p: DRM printer > * @indent: Tab indentation level > * @obj: GEM object > + * > + * This implements the &drm_gem_object_funcs.info callback. > */ > void drm_gem_shmem_print_info(struct drm_printer *p, unsigned int indent, > const struct drm_gem_object *obj) > @@ -577,7 +593,12 @@ EXPORT_SYMBOL(drm_gem_shmem_print_info); > * @obj: GEM object > * > * This function exports a scatter/gather table suitable for PRIME usage by > - * calling the standard DMA mapping API. > + * calling the standard DMA mapping API. Drivers should not call this function > + * directly, instead it should only be used as an implementation for > + * &drm_gem_object_funcs.get_sg_table. > + * > + * Drivers who need to acquire an scatter/gather table for objects need to call > + * drm_gem_shmem_get_pages_sgt() instead. > * > * Returns: > * A pointer to the scatter/gather table of pinned pages or NULL on failure. > @@ -599,6 +620,10 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_get_sg_table); > * the sg table doesn't exist, the pages are pinned, dma-mapped, and a sg > * table created. > * > + * This is the main function for drivers to get at backing storage, and it hides > + * and difference between dma-buf imported and natively allocated objects. > + * drm_gem_shmem_get_sg_table() should not be directly called by drivers. > + * > * Returns: > * A pointer to the scatter/gather table of pinned pages or errno on failure. > */ >
On Mon, May 11, 2020 at 01:12:49PM +0200, Thomas Zimmermann wrote: > > > Am 11.05.20 um 11:35 schrieb Daniel Vetter: > > - Move the shmem helper section to the drm-mm.rst file, next to the > > vram helpers. Makes a lot more sense there with the now wider scope. > > Also, that's where the all the other backing storage stuff resides. > > It's just the framebuffer helpers that should be in the kms helper > > section. > > > > - Try to clarify which functiosn are for implementing > > drm_gem_object_funcs, and which for drivers to call directly. At > > least one driver screwed that up a bit. > > > > Cc: Gerd Hoffmann <kraxel@redhat.com> > > Cc: Rob Herring <robh@kernel.org> > > Cc: Noralf Trønnes <noralf@tronnes.org> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> > > See below for a suggestion on the help text. > > > --- > > Documentation/gpu/drm-kms-helpers.rst | 12 -------- > > Documentation/gpu/drm-mm.rst | 12 ++++++++ > > drivers/gpu/drm/drm_gem_shmem_helper.c | 39 +++++++++++++++++++++----- > > 3 files changed, 44 insertions(+), 19 deletions(-) > > > > diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst > > index ee730457bf4e..b89ddd06dabb 100644 > > --- a/Documentation/gpu/drm-kms-helpers.rst > > +++ b/Documentation/gpu/drm-kms-helpers.rst > > @@ -411,15 +411,3 @@ Legacy CRTC/Modeset Helper Functions Reference > > > > .. kernel-doc:: drivers/gpu/drm/drm_crtc_helper.c > > :export: > > - > > -SHMEM GEM Helper Reference > > -========================== > > - > > -.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c > > - :doc: overview > > - > > -.. kernel-doc:: include/drm/drm_gem_shmem_helper.h > > - :internal: > > - > > -.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c > > - :export: > > diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst > > index 1839762044be..c01757b0ac25 100644 > > --- a/Documentation/gpu/drm-mm.rst > > +++ b/Documentation/gpu/drm-mm.rst > > @@ -373,6 +373,18 @@ GEM CMA Helper Functions Reference > > .. kernel-doc:: drivers/gpu/drm/drm_gem_cma_helper.c > > :export: > > > > +GEM SHMEM Helper Function Reference > > +----------------------------------- > > + > > +.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c > > + :doc: overview > > + > > +.. kernel-doc:: include/drm/drm_gem_shmem_helper.h > > + :internal: > > + > > +.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c > > + :export: > > + > > GEM VRAM Helper Functions Reference > > ----------------------------------- > > > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c > > index df31e5782eed..2a70159d50ef 100644 > > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > > @@ -103,7 +103,8 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_create); > > * @obj: GEM object to free > > * > > * This function cleans up the GEM object state and frees the memory used to > > - * store the object itself. > > + * store the object itself. It should be used to implement > > + * &drm_gem_object_funcs.free. > > It should 'only' be used? Or maybe you can say that it should be used by > drivers that don't implement struct drm_driver.gem_create_object. Just looked at this, and I'm not clear what you're aiming for. There doesn't seem to be any misuse for this for other places than the free hook. And I can't really come up with ideas where that would even work. What kind of confusion are you trying to clarify with your suggestion? Maybe I can then reword that into something that also makes sense for me. Thanks, Daniel > > > */ > > void drm_gem_shmem_free_object(struct drm_gem_object *obj) > > { > > @@ -214,7 +215,8 @@ EXPORT_SYMBOL(drm_gem_shmem_put_pages); > > * @obj: GEM object > > * > > * This function makes sure the backing pages are pinned in memory while the > > - * buffer is exported. > > + * buffer is exported. It should only be used to implement > > + * &drm_gem_object_funcs.pin. > > * > > * Returns: > > * 0 on success or a negative error code on failure. > > @@ -232,7 +234,7 @@ EXPORT_SYMBOL(drm_gem_shmem_pin); > > * @obj: GEM object > > * > > * This function removes the requirement that the backing pages are pinned in > > - * memory. > > + * memory. It should only be used to implement &drm_gem_object_funcs.unpin. > > */ > > void drm_gem_shmem_unpin(struct drm_gem_object *obj) > > { > > @@ -285,8 +287,14 @@ static void *drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem) > > * drm_gem_shmem_vmap - Create a virtual mapping for a shmem GEM object > > * @shmem: shmem GEM object > > * > > - * This function makes sure that a virtual address exists for the buffer backing > > - * the shmem GEM object. > > + * This function makes sure that a contiguous kernel virtual address mapping > > + * exists for the buffer backing the shmem GEM object. > > + * > > + * This function can be used to implement &drm_gem_object_funcs.vmap. But it can > > + * also be called by drivers directly, in which case it will hide the > > + * differences between dma-buf imported and natively allocated objects. > > + * > > + * Acquired mappings should be cleaned up by calling drm_gem_shmem_vunmap(). > > * > > * Returns: > > * 0 on success or a negative error code on failure. > > @@ -330,7 +338,13 @@ static void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem) > > * drm_gem_shmem_vunmap - Unmap a virtual mapping fo a shmem GEM object > > * @shmem: shmem GEM object > > * > > - * This function removes the virtual address when use count drops to zero. > > + * This function cleans up a kernel virtual address mapping acquired by > > + * drm_gem_shmem_vmap(). The mapping is only removed when the use count drops to > > + * zero. > > + * > > + * This function can be used to implement &drm_gem_object_funcs.vmap. But it can > > + * also be called by drivers directly, in which case it will hide the > > + * differences between dma-buf imported and natively allocated objects. > > */ > > void drm_gem_shmem_vunmap(struct drm_gem_object *obj, void *vaddr) > > { > > @@ -559,6 +573,8 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_mmap); > > * @p: DRM printer > > * @indent: Tab indentation level > > * @obj: GEM object > > + * > > + * This implements the &drm_gem_object_funcs.info callback. > > */ > > void drm_gem_shmem_print_info(struct drm_printer *p, unsigned int indent, > > const struct drm_gem_object *obj) > > @@ -577,7 +593,12 @@ EXPORT_SYMBOL(drm_gem_shmem_print_info); > > * @obj: GEM object > > * > > * This function exports a scatter/gather table suitable for PRIME usage by > > - * calling the standard DMA mapping API. > > + * calling the standard DMA mapping API. Drivers should not call this function > > + * directly, instead it should only be used as an implementation for > > + * &drm_gem_object_funcs.get_sg_table. > > + * > > + * Drivers who need to acquire an scatter/gather table for objects need to call > > + * drm_gem_shmem_get_pages_sgt() instead. > > * > > * Returns: > > * A pointer to the scatter/gather table of pinned pages or NULL on failure. > > @@ -599,6 +620,10 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_get_sg_table); > > * the sg table doesn't exist, the pages are pinned, dma-mapped, and a sg > > * table created. > > * > > + * This is the main function for drivers to get at backing storage, and it hides > > + * and difference between dma-buf imported and natively allocated objects. > > + * drm_gem_shmem_get_sg_table() should not be directly called by drivers. > > + * > > * Returns: > > * A pointer to the scatter/gather table of pinned pages or errno on failure. > > */ > > > > -- > 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 >
Hi Am 14.05.20 um 22:05 schrieb Daniel Vetter: > On Mon, May 11, 2020 at 01:12:49PM +0200, Thomas Zimmermann wrote: >> >> >> Am 11.05.20 um 11:35 schrieb Daniel Vetter: >>> - Move the shmem helper section to the drm-mm.rst file, next to the >>> vram helpers. Makes a lot more sense there with the now wider scope. >>> Also, that's where the all the other backing storage stuff resides. >>> It's just the framebuffer helpers that should be in the kms helper >>> section. >>> >>> - Try to clarify which functiosn are for implementing >>> drm_gem_object_funcs, and which for drivers to call directly. At >>> least one driver screwed that up a bit. >>> >>> Cc: Gerd Hoffmann <kraxel@redhat.com> >>> Cc: Rob Herring <robh@kernel.org> >>> Cc: Noralf Trønnes <noralf@tronnes.org> >>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >> >> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> >> >> See below for a suggestion on the help text. >> >>> --- >>> Documentation/gpu/drm-kms-helpers.rst | 12 -------- >>> Documentation/gpu/drm-mm.rst | 12 ++++++++ >>> drivers/gpu/drm/drm_gem_shmem_helper.c | 39 +++++++++++++++++++++----- >>> 3 files changed, 44 insertions(+), 19 deletions(-) >>> >>> diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst >>> index ee730457bf4e..b89ddd06dabb 100644 >>> --- a/Documentation/gpu/drm-kms-helpers.rst >>> +++ b/Documentation/gpu/drm-kms-helpers.rst >>> @@ -411,15 +411,3 @@ Legacy CRTC/Modeset Helper Functions Reference >>> >>> .. kernel-doc:: drivers/gpu/drm/drm_crtc_helper.c >>> :export: >>> - >>> -SHMEM GEM Helper Reference >>> -========================== >>> - >>> -.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c >>> - :doc: overview >>> - >>> -.. kernel-doc:: include/drm/drm_gem_shmem_helper.h >>> - :internal: >>> - >>> -.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c >>> - :export: >>> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst >>> index 1839762044be..c01757b0ac25 100644 >>> --- a/Documentation/gpu/drm-mm.rst >>> +++ b/Documentation/gpu/drm-mm.rst >>> @@ -373,6 +373,18 @@ GEM CMA Helper Functions Reference >>> .. kernel-doc:: drivers/gpu/drm/drm_gem_cma_helper.c >>> :export: >>> >>> +GEM SHMEM Helper Function Reference >>> +----------------------------------- >>> + >>> +.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c >>> + :doc: overview >>> + >>> +.. kernel-doc:: include/drm/drm_gem_shmem_helper.h >>> + :internal: >>> + >>> +.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c >>> + :export: >>> + >>> GEM VRAM Helper Functions Reference >>> ----------------------------------- >>> >>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c >>> index df31e5782eed..2a70159d50ef 100644 >>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c >>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c >>> @@ -103,7 +103,8 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_create); >>> * @obj: GEM object to free >>> * >>> * This function cleans up the GEM object state and frees the memory used to >>> - * store the object itself. >>> + * store the object itself. It should be used to implement >>> + * &drm_gem_object_funcs.free. >> >> It should 'only' be used? Or maybe you can say that it should be used by >> drivers that don't implement struct drm_driver.gem_create_object. > > Just looked at this, and I'm not clear what you're aiming for. There > doesn't seem to be any misuse for this for other places than the free > hook. And I can't really come up with ideas where that would even work. > > What kind of confusion are you trying to clarify with your suggestion? > Maybe I can then reword that into something that also makes sense for me. It was just nit picking. I just worried that drivers might try to call this function for cleaning up an embedded instance of the structure; although the function's name clearly indicates otherwise. Kind of related, I think we should be more strict to use the abstract GEM interfaces. For example, several drivers call drm_gem_shmem_vmap() instead of drm_gem_vmap(). For this free function, we should require drivers to call drm_gem_object_free() instead of the shmem function. Best regards Thomas > > Thanks, Daniel > >> >>> */ >>> void drm_gem_shmem_free_object(struct drm_gem_object *obj) >>> { >>> @@ -214,7 +215,8 @@ EXPORT_SYMBOL(drm_gem_shmem_put_pages); >>> * @obj: GEM object >>> * >>> * This function makes sure the backing pages are pinned in memory while the >>> - * buffer is exported. >>> + * buffer is exported. It should only be used to implement >>> + * &drm_gem_object_funcs.pin. >>> * >>> * Returns: >>> * 0 on success or a negative error code on failure. >>> @@ -232,7 +234,7 @@ EXPORT_SYMBOL(drm_gem_shmem_pin); >>> * @obj: GEM object >>> * >>> * This function removes the requirement that the backing pages are pinned in >>> - * memory. >>> + * memory. It should only be used to implement &drm_gem_object_funcs.unpin. >>> */ >>> void drm_gem_shmem_unpin(struct drm_gem_object *obj) >>> { >>> @@ -285,8 +287,14 @@ static void *drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem) >>> * drm_gem_shmem_vmap - Create a virtual mapping for a shmem GEM object >>> * @shmem: shmem GEM object >>> * >>> - * This function makes sure that a virtual address exists for the buffer backing >>> - * the shmem GEM object. >>> + * This function makes sure that a contiguous kernel virtual address mapping >>> + * exists for the buffer backing the shmem GEM object. >>> + * >>> + * This function can be used to implement &drm_gem_object_funcs.vmap. But it can >>> + * also be called by drivers directly, in which case it will hide the >>> + * differences between dma-buf imported and natively allocated objects. >>> + * >>> + * Acquired mappings should be cleaned up by calling drm_gem_shmem_vunmap(). >>> * >>> * Returns: >>> * 0 on success or a negative error code on failure. >>> @@ -330,7 +338,13 @@ static void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem) >>> * drm_gem_shmem_vunmap - Unmap a virtual mapping fo a shmem GEM object >>> * @shmem: shmem GEM object >>> * >>> - * This function removes the virtual address when use count drops to zero. >>> + * This function cleans up a kernel virtual address mapping acquired by >>> + * drm_gem_shmem_vmap(). The mapping is only removed when the use count drops to >>> + * zero. >>> + * >>> + * This function can be used to implement &drm_gem_object_funcs.vmap. But it can >>> + * also be called by drivers directly, in which case it will hide the >>> + * differences between dma-buf imported and natively allocated objects. >>> */ >>> void drm_gem_shmem_vunmap(struct drm_gem_object *obj, void *vaddr) >>> { >>> @@ -559,6 +573,8 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_mmap); >>> * @p: DRM printer >>> * @indent: Tab indentation level >>> * @obj: GEM object >>> + * >>> + * This implements the &drm_gem_object_funcs.info callback. >>> */ >>> void drm_gem_shmem_print_info(struct drm_printer *p, unsigned int indent, >>> const struct drm_gem_object *obj) >>> @@ -577,7 +593,12 @@ EXPORT_SYMBOL(drm_gem_shmem_print_info); >>> * @obj: GEM object >>> * >>> * This function exports a scatter/gather table suitable for PRIME usage by >>> - * calling the standard DMA mapping API. >>> + * calling the standard DMA mapping API. Drivers should not call this function >>> + * directly, instead it should only be used as an implementation for >>> + * &drm_gem_object_funcs.get_sg_table. >>> + * >>> + * Drivers who need to acquire an scatter/gather table for objects need to call >>> + * drm_gem_shmem_get_pages_sgt() instead. >>> * >>> * Returns: >>> * A pointer to the scatter/gather table of pinned pages or NULL on failure. >>> @@ -599,6 +620,10 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_get_sg_table); >>> * the sg table doesn't exist, the pages are pinned, dma-mapped, and a sg >>> * table created. >>> * >>> + * This is the main function for drivers to get at backing storage, and it hides >>> + * and difference between dma-buf imported and natively allocated objects. >>> + * drm_gem_shmem_get_sg_table() should not be directly called by drivers. >>> + * >>> * Returns: >>> * A pointer to the scatter/gather table of pinned pages or errno on failure. >>> */ >>> >> >> -- >> 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 >> > > > >
diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst index ee730457bf4e..b89ddd06dabb 100644 --- a/Documentation/gpu/drm-kms-helpers.rst +++ b/Documentation/gpu/drm-kms-helpers.rst @@ -411,15 +411,3 @@ Legacy CRTC/Modeset Helper Functions Reference .. kernel-doc:: drivers/gpu/drm/drm_crtc_helper.c :export: - -SHMEM GEM Helper Reference -========================== - -.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c - :doc: overview - -.. kernel-doc:: include/drm/drm_gem_shmem_helper.h - :internal: - -.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c - :export: diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst index 1839762044be..c01757b0ac25 100644 --- a/Documentation/gpu/drm-mm.rst +++ b/Documentation/gpu/drm-mm.rst @@ -373,6 +373,18 @@ GEM CMA Helper Functions Reference .. kernel-doc:: drivers/gpu/drm/drm_gem_cma_helper.c :export: +GEM SHMEM Helper Function Reference +----------------------------------- + +.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c + :doc: overview + +.. kernel-doc:: include/drm/drm_gem_shmem_helper.h + :internal: + +.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c + :export: + GEM VRAM Helper Functions Reference ----------------------------------- diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index df31e5782eed..2a70159d50ef 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -103,7 +103,8 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_create); * @obj: GEM object to free * * This function cleans up the GEM object state and frees the memory used to - * store the object itself. + * store the object itself. It should be used to implement + * &drm_gem_object_funcs.free. */ void drm_gem_shmem_free_object(struct drm_gem_object *obj) { @@ -214,7 +215,8 @@ EXPORT_SYMBOL(drm_gem_shmem_put_pages); * @obj: GEM object * * This function makes sure the backing pages are pinned in memory while the - * buffer is exported. + * buffer is exported. It should only be used to implement + * &drm_gem_object_funcs.pin. * * Returns: * 0 on success or a negative error code on failure. @@ -232,7 +234,7 @@ EXPORT_SYMBOL(drm_gem_shmem_pin); * @obj: GEM object * * This function removes the requirement that the backing pages are pinned in - * memory. + * memory. It should only be used to implement &drm_gem_object_funcs.unpin. */ void drm_gem_shmem_unpin(struct drm_gem_object *obj) { @@ -285,8 +287,14 @@ static void *drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem) * drm_gem_shmem_vmap - Create a virtual mapping for a shmem GEM object * @shmem: shmem GEM object * - * This function makes sure that a virtual address exists for the buffer backing - * the shmem GEM object. + * This function makes sure that a contiguous kernel virtual address mapping + * exists for the buffer backing the shmem GEM object. + * + * This function can be used to implement &drm_gem_object_funcs.vmap. But it can + * also be called by drivers directly, in which case it will hide the + * differences between dma-buf imported and natively allocated objects. + * + * Acquired mappings should be cleaned up by calling drm_gem_shmem_vunmap(). * * Returns: * 0 on success or a negative error code on failure. @@ -330,7 +338,13 @@ static void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem) * drm_gem_shmem_vunmap - Unmap a virtual mapping fo a shmem GEM object * @shmem: shmem GEM object * - * This function removes the virtual address when use count drops to zero. + * This function cleans up a kernel virtual address mapping acquired by + * drm_gem_shmem_vmap(). The mapping is only removed when the use count drops to + * zero. + * + * This function can be used to implement &drm_gem_object_funcs.vmap. But it can + * also be called by drivers directly, in which case it will hide the + * differences between dma-buf imported and natively allocated objects. */ void drm_gem_shmem_vunmap(struct drm_gem_object *obj, void *vaddr) { @@ -559,6 +573,8 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_mmap); * @p: DRM printer * @indent: Tab indentation level * @obj: GEM object + * + * This implements the &drm_gem_object_funcs.info callback. */ void drm_gem_shmem_print_info(struct drm_printer *p, unsigned int indent, const struct drm_gem_object *obj) @@ -577,7 +593,12 @@ EXPORT_SYMBOL(drm_gem_shmem_print_info); * @obj: GEM object * * This function exports a scatter/gather table suitable for PRIME usage by - * calling the standard DMA mapping API. + * calling the standard DMA mapping API. Drivers should not call this function + * directly, instead it should only be used as an implementation for + * &drm_gem_object_funcs.get_sg_table. + * + * Drivers who need to acquire an scatter/gather table for objects need to call + * drm_gem_shmem_get_pages_sgt() instead. * * Returns: * A pointer to the scatter/gather table of pinned pages or NULL on failure. @@ -599,6 +620,10 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_get_sg_table); * the sg table doesn't exist, the pages are pinned, dma-mapped, and a sg * table created. * + * This is the main function for drivers to get at backing storage, and it hides + * and difference between dma-buf imported and natively allocated objects. + * drm_gem_shmem_get_sg_table() should not be directly called by drivers. + * * Returns: * A pointer to the scatter/gather table of pinned pages or errno on failure. */
- Move the shmem helper section to the drm-mm.rst file, next to the vram helpers. Makes a lot more sense there with the now wider scope. Also, that's where the all the other backing storage stuff resides. It's just the framebuffer helpers that should be in the kms helper section. - Try to clarify which functiosn are for implementing drm_gem_object_funcs, and which for drivers to call directly. At least one driver screwed that up a bit. Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Rob Herring <robh@kernel.org> Cc: Noralf Trønnes <noralf@tronnes.org> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- Documentation/gpu/drm-kms-helpers.rst | 12 -------- Documentation/gpu/drm-mm.rst | 12 ++++++++ drivers/gpu/drm/drm_gem_shmem_helper.c | 39 +++++++++++++++++++++----- 3 files changed, 44 insertions(+), 19 deletions(-)