Re: [PATCH 1/7] drm/gem: Provide drm_gem_fb_{begin, end}_cpu_access() helpers

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Jul 16, 2021 at 04:07:55PM +0200, Thomas Zimmermann wrote:
> Implement helpers drm_gem_fb_begin_cpu_access() and _end_cpu_access(),
> which call the rsp dma-buf functions for all GEM BOs of the given
> framebuffer.
> 
> Calls to dma_buf_end_cpu_access() can return an error code on failure,
> while drm_gem_fb_end_cpu_access() does not. The latter runs during DRM's
> atomic commit or during cleanup. Both cases don't allow for errors, so
> leave out the return value.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 89 ++++++++++++++++++++
>  include/drm/drm_gem_framebuffer_helper.h     |  6 ++
>  2 files changed, 95 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index e2c68822e05c..94a1c0b0edfd 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -306,6 +306,95 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
>  
> +/**
> + * drm_gem_fb_begin_cpu_access - prepares GEM buffer objects for CPU access
> + * @fb: the framebuffer
> + * @dir: access mode
> + *
> + * Prepares a framebuffer'S GEM buffer objects for CPU access. This function

			s/S/s/

> + * must be called before accessing the BO data within the kernel. For imported
> + * BOs, the function calls dma_buf_begin_cpu_access().
> + *
> + * See drm_gem_fb_end_cpu_access() for signalling the end of CPU access.
> + *
> + * Returns:
> + * 0 on success, or a negative errno code otherwise.
> + */
> +int drm_gem_fb_begin_cpu_access(struct drm_framebuffer *fb, enum dma_data_direction dir)
> +{
> +	struct dma_buf_attachment *import_attach;
> +	struct drm_gem_object *obj;
> +	size_t i;
> +	int ret, ret2;
> +
> +	for (i = 0; i < ARRAY_SIZE(fb->obj); ++i) {
> +		obj = drm_gem_fb_get_obj(fb, i);
> +		if (!obj)
> +			continue;
> +		import_attach = obj->import_attach;
> +		if (!import_attach)
> +			continue;
> +		ret = dma_buf_begin_cpu_access(import_attach->dmabuf, dir);
> +		if (ret)
> +			goto err_dma_buf_end_cpu_access;
> +	}
> +
> +	return 0;
> +
> +err_dma_buf_end_cpu_access:
> +	while (i) {
> +		--i;
> +		obj = drm_gem_fb_get_obj(fb, i);
> +		if (!obj)
> +			continue;
> +		import_attach = obj->import_attach;
> +		if (!import_attach)
> +			continue;
> +		ret2 = dma_buf_end_cpu_access(import_attach->dmabuf, dir);
> +		if (ret2) {
> +			drm_err(fb->dev,
> +				"dma_buf_end_cpu_access() failed during error handling: %d\n",
> +				ret2);
> +		}
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(drm_gem_fb_begin_cpu_access);
> +
> +/**
> + * drm_gem_fb_end_cpu_access - signals end of CPU access to GEM buffer objects
> + * @fb: the framebuffer
> + * @dir: access mode
> + *
> + * Signals the end of CPU access to the given framebuffer'S GEM buffer objects. This

							s/S/s/

> + * function must be paired with a corresponding call to drm_gem_fb_begin_cpu_access().
> + * For imported BOs, the function calls dma_buf_end_cpu_access().
> + *
> + * See also drm_gem_fb_begin_cpu_access().
> + */
> +void drm_gem_fb_end_cpu_access(struct drm_framebuffer *fb, enum dma_data_direction dir)
> +{
> +	size_t i = ARRAY_SIZE(fb->obj);
> +	struct dma_buf_attachment *import_attach;
> +	struct drm_gem_object *obj;
> +	int ret;
> +
> +	while (i) {
> +		--i;
> +		obj = drm_gem_fb_get_obj(fb, i);
> +		if (!obj)
> +			continue;
> +		import_attach = obj->import_attach;
> +		if (!import_attach)
> +			continue;
> +		ret = dma_buf_end_cpu_access(import_attach->dmabuf, dir);
> +		if (ret)
> +			drm_err(fb->dev, "dma_buf_end_cpu_access() failed: %d\n", ret);
> +	}
> +}
> +EXPORT_SYMBOL(drm_gem_fb_end_cpu_access);
> +
>  static __u32 drm_gem_afbc_get_bpp(struct drm_device *dev,
>  				  const struct drm_mode_fb_cmd2 *mode_cmd)
>  {
> diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
> index 6bdffc7aa124..5705722f0855 100644
> --- a/include/drm/drm_gem_framebuffer_helper.h
> +++ b/include/drm/drm_gem_framebuffer_helper.h
> @@ -1,6 +1,9 @@
>  #ifndef __DRM_GEM_FB_HELPER_H__
>  #define __DRM_GEM_FB_HELPER_H__
>  
> +#include <linux/dma-buf.h>
> +#include <linux/dma-buf-map.h>
> +
>  struct drm_afbc_framebuffer;
>  struct drm_device;
>  struct drm_fb_helper_surface_size;
> @@ -34,6 +37,9 @@ struct drm_framebuffer *
>  drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
>  			     const struct drm_mode_fb_cmd2 *mode_cmd);
>  
> +int drm_gem_fb_begin_cpu_access(struct drm_framebuffer *fb, enum dma_data_direction dir);
> +void drm_gem_fb_end_cpu_access(struct drm_framebuffer *fb, enum dma_data_direction dir);
> +
>  #define drm_is_afbc(modifier) \
>  	(((modifier) & AFBC_VENDOR_AND_TYPE_MASK) == DRM_FORMAT_MOD_ARM_AFBC(0))

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>  
> -- 
> 2.32.0
> 

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