Re: [PATCH v2 02/22] drm/fb-cma-helper: Use drm_gem_framebuffer_helper

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 09, 2017 at 12:11:05PM +0200, Noralf Trønnes wrote:
> Use the new drm_gem_framebuffer_helper who's code was copied
> from this helper.
> 
> Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
> ---
>  drivers/gpu/drm/drm_fb_cma_helper.c | 172 +++++++-----------------------------
>  1 file changed, 30 insertions(+), 142 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
> index ade319d..d0c0110 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -18,27 +18,17 @@
>   */
>  
>  #include <drm/drmP.h>
> -#include <drm/drm_atomic.h>
> -#include <drm/drm_crtc.h>
>  #include <drm/drm_fb_helper.h>
> -#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_framebuffer.h>
>  #include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
>  #include <drm/drm_fb_cma_helper.h>
> -#include <linux/dma-buf.h>
> -#include <linux/dma-mapping.h>
>  #include <linux/module.h>
> -#include <linux/reservation.h>
>  
>  #define DEFAULT_FBDEFIO_DELAY_MS 50
>  
> -struct drm_fb_cma {
> -	struct drm_framebuffer		fb;
> -	struct drm_gem_cma_object	*obj[4];
> -};
> -
>  struct drm_fbdev_cma {
>  	struct drm_fb_helper	fb_helper;
> -	struct drm_fb_cma	*fb;
>  	const struct drm_framebuffer_funcs *fb_funcs;
>  };
>  
> @@ -90,69 +80,19 @@ static inline struct drm_fbdev_cma *to_fbdev_cma(struct drm_fb_helper *helper)
>  	return container_of(helper, struct drm_fbdev_cma, fb_helper);
>  }
>  
> -static inline struct drm_fb_cma *to_fb_cma(struct drm_framebuffer *fb)
> -{
> -	return container_of(fb, struct drm_fb_cma, fb);
> -}
> -
>  void drm_fb_cma_destroy(struct drm_framebuffer *fb)
>  {
> -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
> -	int i;
> -
> -	for (i = 0; i < 4; i++) {
> -		if (fb_cma->obj[i])
> -			drm_gem_object_put_unlocked(&fb_cma->obj[i]->base);
> -	}
> -
> -	drm_framebuffer_cleanup(fb);
> -	kfree(fb_cma);
> +	drm_gem_fb_destroy(fb);
>  }
>  EXPORT_SYMBOL(drm_fb_cma_destroy);
>  
>  int drm_fb_cma_create_handle(struct drm_framebuffer *fb,
>  	struct drm_file *file_priv, unsigned int *handle)
>  {
> -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
> -
> -	return drm_gem_handle_create(file_priv,
> -			&fb_cma->obj[0]->base, handle);
> +	return drm_gem_fb_create_handle(fb, file_priv, handle);
>  }
>  EXPORT_SYMBOL(drm_fb_cma_create_handle);
>  
> -static struct drm_framebuffer_funcs drm_fb_cma_funcs = {
> -	.destroy	= drm_fb_cma_destroy,
> -	.create_handle	= drm_fb_cma_create_handle,
> -};
> -
> -static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,
> -	const struct drm_mode_fb_cmd2 *mode_cmd,
> -	struct drm_gem_cma_object **obj,
> -	unsigned int num_planes, const struct drm_framebuffer_funcs *funcs)
> -{
> -	struct drm_fb_cma *fb_cma;
> -	int ret;
> -	int i;
> -
> -	fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL);
> -	if (!fb_cma)
> -		return ERR_PTR(-ENOMEM);
> -
> -	drm_helper_mode_fill_fb_struct(dev, &fb_cma->fb, mode_cmd);
> -
> -	for (i = 0; i < num_planes; i++)
> -		fb_cma->obj[i] = obj[i];
> -
> -	ret = drm_framebuffer_init(dev, &fb_cma->fb, funcs);
> -	if (ret) {
> -		dev_err(dev->dev, "Failed to initialize framebuffer: %d\n", ret);
> -		kfree(fb_cma);
> -		return ERR_PTR(ret);
> -	}
> -
> -	return fb_cma;
> -}
> -
>  /**
>   * drm_fb_cma_create_with_funcs() - helper function for the
>   *                                  &drm_mode_config_funcs.fb_create
> @@ -170,53 +110,7 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
>  	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd,
>  	const struct drm_framebuffer_funcs *funcs)
>  {
> -	const struct drm_format_info *info;
> -	struct drm_fb_cma *fb_cma;
> -	struct drm_gem_cma_object *objs[4];
> -	struct drm_gem_object *obj;
> -	int ret;
> -	int i;
> -
> -	info = drm_get_format_info(dev, mode_cmd);
> -	if (!info)
> -		return ERR_PTR(-EINVAL);
> -
> -	for (i = 0; i < info->num_planes; i++) {
> -		unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
> -		unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
> -		unsigned int min_size;
> -
> -		obj = drm_gem_object_lookup(file_priv, mode_cmd->handles[i]);
> -		if (!obj) {
> -			dev_err(dev->dev, "Failed to lookup GEM object\n");
> -			ret = -ENOENT;
> -			goto err_gem_object_put;
> -		}
> -
> -		min_size = (height - 1) * mode_cmd->pitches[i]
> -			 + width * info->cpp[i]
> -			 + mode_cmd->offsets[i];
> -
> -		if (obj->size < min_size) {
> -			drm_gem_object_put_unlocked(obj);
> -			ret = -EINVAL;
> -			goto err_gem_object_put;
> -		}
> -		objs[i] = to_drm_gem_cma_obj(obj);
> -	}
> -
> -	fb_cma = drm_fb_cma_alloc(dev, mode_cmd, objs, i, funcs);
> -	if (IS_ERR(fb_cma)) {
> -		ret = PTR_ERR(fb_cma);
> -		goto err_gem_object_put;
> -	}
> -
> -	return &fb_cma->fb;
> -
> -err_gem_object_put:
> -	for (i--; i >= 0; i--)
> -		drm_gem_object_put_unlocked(&objs[i]->base);
> -	return ERR_PTR(ret);
> +	return drm_gem_fb_create_with_funcs(dev, file_priv, mode_cmd, funcs);
>  }
>  EXPORT_SYMBOL_GPL(drm_fb_cma_create_with_funcs);
>  
> @@ -233,8 +127,7 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_create_with_funcs);
>  struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,
>  	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd)
>  {
> -	return drm_fb_cma_create_with_funcs(dev, file_priv, mode_cmd,
> -					    &drm_fb_cma_funcs);
> +	return drm_gem_fb_create(dev, file_priv, mode_cmd);
>  }
>  EXPORT_SYMBOL_GPL(drm_fb_cma_create);
>  
> @@ -250,12 +143,14 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_create);
>  struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb,
>  						  unsigned int plane)
>  {
> -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
> +	struct drm_gem_object *gem;
>  
> -	if (plane >= 4)
> +	gem = drm_gem_fb_get_obj(fb, plane);
> +	if (!gem)
>  		return NULL;
>  
> -	return fb_cma->obj[plane];
> +	return to_drm_gem_cma_obj(gem);
> +
>  }
>  EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj);
>  
> @@ -272,13 +167,14 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer *fb,
>  				   struct drm_plane_state *state,
>  				   unsigned int plane)
>  {
> -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
> +	struct drm_gem_cma_object *obj;
>  	dma_addr_t paddr;
>  
> -	if (plane >= 4)
> +	obj = drm_fb_cma_get_gem_obj(fb, plane);
> +	if (!obj)
>  		return 0;
>  
> -	paddr = fb_cma->obj[plane]->paddr + fb->offsets[plane];
> +	paddr = obj->paddr + fb->offsets[plane];
>  	paddr += fb->format->cpp[plane] * (state->src_x >> 16);
>  	paddr += fb->pitches[plane] * (state->src_y >> 16);
>  
> @@ -302,26 +198,13 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_addr);
>  int drm_fb_cma_prepare_fb(struct drm_plane *plane,
>  			  struct drm_plane_state *state)
>  {
> -	struct dma_buf *dma_buf;
> -	struct dma_fence *fence;
> -
> -	if ((plane->state->fb == state->fb) || !state->fb)
> -		return 0;
> -
> -	dma_buf = drm_fb_cma_get_gem_obj(state->fb, 0)->base.dma_buf;
> -	if (dma_buf) {
> -		fence = reservation_object_get_excl_rcu(dma_buf->resv);
> -		drm_atomic_set_fence_for_plane(state, fence);
> -	}
> -
> -	return 0;
> +	return drm_gem_fb_prepare_fb(plane, state);
>  }
>  EXPORT_SYMBOL_GPL(drm_fb_cma_prepare_fb);
>  
>  #ifdef CONFIG_DEBUG_FS
>  static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m)
>  {
> -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
>  	int i;
>  
>  	seq_printf(m, "fb: %dx%d@%4.4s\n", fb->width, fb->height,
> @@ -330,7 +213,7 @@ static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m)
>  	for (i = 0; i < fb->format->num_planes; i++) {
>  		seq_printf(m, "   %d: offset=%d pitch=%d, obj: ",
>  				i, fb->offsets[i], fb->pitches[i]);
> -		drm_gem_cma_describe(fb_cma->obj[i], m);
> +		drm_gem_cma_describe(drm_fb_cma_get_gem_obj(fb, i), m);
>  	}
>  }
>  
> @@ -434,6 +317,7 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper,
>  	struct drm_mode_fb_cmd2 mode_cmd = { 0 };
>  	struct drm_device *dev = helper->dev;
>  	struct drm_gem_cma_object *obj;
> +	struct drm_gem_object *gem;
>  	struct drm_framebuffer *fb;
>  	unsigned int bytes_per_pixel;
>  	unsigned long offset;
> @@ -464,15 +348,14 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper,
>  		goto err_gem_free_object;
>  	}
>  
> -	fbdev_cma->fb = drm_fb_cma_alloc(dev, &mode_cmd, &obj, 1,
> -					 fbdev_cma->fb_funcs);
> -	if (IS_ERR(fbdev_cma->fb)) {
> +	gem = &obj->base;
> +	fb = drm_gem_fb_alloc(dev, &mode_cmd, &gem, 1, fbdev_cma->fb_funcs);
> +	if (IS_ERR(fb)) {
>  		dev_err(dev->dev, "Failed to allocate DRM framebuffer.\n");
> -		ret = PTR_ERR(fbdev_cma->fb);
> +		ret = PTR_ERR(fb);
>  		goto err_fb_info_destroy;
>  	}
>  
> -	fb = &fbdev_cma->fb->fb;
>  	helper->fb = fb;
>  
>  	fbi->par = helper;
> @@ -500,7 +383,7 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper,
>  	return 0;
>  
>  err_cma_destroy:
> -	drm_framebuffer_remove(&fbdev_cma->fb->fb);
> +	drm_framebuffer_remove(fb);
>  err_fb_info_destroy:
>  	drm_fb_helper_fini(helper);
>  err_gem_free_object:
> @@ -570,6 +453,11 @@ struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
>  }
>  EXPORT_SYMBOL_GPL(drm_fbdev_cma_init_with_funcs);
>  
> +static const struct drm_framebuffer_funcs drm_fb_cma_funcs = {
> +	.destroy	= drm_gem_fb_destroy,
> +	.create_handle	= drm_gem_fb_create_handle,
> +};

I don't see you removing this in the very last patch. It exactly matches
the function table the gem fb helper provides, why do we need this dupe
here?
-Daniel

> +
>  /**
>   * drm_fbdev_cma_init() - Allocate and initializes a drm_fbdev_cma struct
>   * @dev: DRM device
> @@ -597,8 +485,8 @@ void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma)
>  	if (fbdev_cma->fb_helper.fbdev)
>  		drm_fbdev_cma_defio_fini(fbdev_cma->fb_helper.fbdev);
>  
> -	if (fbdev_cma->fb)
> -		drm_framebuffer_remove(&fbdev_cma->fb->fb);
> +	if (fbdev_cma->fb_helper.fb)
> +		drm_framebuffer_remove(fbdev_cma->fb_helper.fb);
>  
>  	drm_fb_helper_fini(&fbdev_cma->fb_helper);
>  	kfree(fbdev_cma);
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux