Re: [PATCH 2/4] drm/hisilicon/hibmc: Replace struct hibmc_framebuffer with generic code

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Nov 22, 2019 at 09:30:42AM +0100, Thomas Zimmermann wrote:
> The hibmc driver's struct hibmc_framebuffer stores a DRM framebuffer
> with an associated GEM object. This functionality is also provided by
> generic code. Switch hibmc over.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Aside from the one silly compile fail I managed to do and not spot this
matches what I've done.

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

> ---
>  .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c    |  4 +-
>  .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   | 11 ---
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c   | 69 +------------------
>  3 files changed, 3 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> index 6527a97f68a3..7fa7d4933f60 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> @@ -99,14 +99,12 @@ static void hibmc_plane_atomic_update(struct drm_plane *plane,
>  	s64 gpu_addr = 0;
>  	unsigned int line_l;
>  	struct hibmc_drm_private *priv = plane->dev->dev_private;
> -	struct hibmc_framebuffer *hibmc_fb;
>  	struct drm_gem_vram_object *gbo;
>  
>  	if (!state->fb)
>  		return;
>  
> -	hibmc_fb = to_hibmc_framebuffer(state->fb);
> -	gbo = drm_gem_vram_of_gem(hibmc_fb->obj);
> +	gbo = drm_gem_vram_of_gem(state->fb->obj[0]);
>  
>  	gpu_addr = drm_gem_vram_offset(gbo);
>  	if (WARN_ON_ONCE(gpu_addr < 0))
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> index b34493ead30b..8eb7258b236a 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> @@ -20,11 +20,6 @@
>  struct drm_device;
>  struct drm_gem_object;
>  
> -struct hibmc_framebuffer {
> -	struct drm_framebuffer fb;
> -	struct drm_gem_object *obj;
> -};
> -
>  struct hibmc_drm_private {
>  	/* hw */
>  	void __iomem   *mmio;
> @@ -38,8 +33,6 @@ struct hibmc_drm_private {
>  	bool mode_config_initialized;
>  };
>  
> -#define to_hibmc_framebuffer(x) container_of(x, struct hibmc_framebuffer, fb)
> -
>  void hibmc_set_power_mode(struct hibmc_drm_private *priv,
>  			  unsigned int power_mode);
>  void hibmc_set_current_gate(struct hibmc_drm_private *priv,
> @@ -50,10 +43,6 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv);
>  
>  int hibmc_gem_create(struct drm_device *dev, u32 size, bool iskernel,
>  		     struct drm_gem_object **obj);
> -struct hibmc_framebuffer *
> -hibmc_framebuffer_init(struct drm_device *dev,
> -		       const struct drm_mode_fb_cmd2 *mode_cmd,
> -		       struct drm_gem_object *obj);
>  
>  int hibmc_mm_init(struct hibmc_drm_private *hibmc);
>  void hibmc_mm_fini(struct hibmc_drm_private *hibmc);
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
> index 21b684eab5c9..f6d25b85c209 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
> @@ -15,6 +15,7 @@
>  
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_gem.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
>  #include <drm/drm_gem_vram_helper.h>
>  #include <drm/drm_print.h>
>  
> @@ -97,74 +98,8 @@ int hibmc_dumb_create(struct drm_file *file, struct drm_device *dev,
>  	return 0;
>  }
>  
> -static void hibmc_user_framebuffer_destroy(struct drm_framebuffer *fb)
> -{
> -	struct hibmc_framebuffer *hibmc_fb = to_hibmc_framebuffer(fb);
> -
> -	drm_gem_object_put_unlocked(hibmc_fb->obj);
> -	drm_framebuffer_cleanup(fb);
> -	kfree(hibmc_fb);
> -}
> -
> -static const struct drm_framebuffer_funcs hibmc_fb_funcs = {
> -	.destroy = hibmc_user_framebuffer_destroy,
> -};
> -
> -struct hibmc_framebuffer *
> -hibmc_framebuffer_init(struct drm_device *dev,
> -		       const struct drm_mode_fb_cmd2 *mode_cmd,
> -		       struct drm_gem_object *obj)
> -{
> -	struct hibmc_framebuffer *hibmc_fb;
> -	int ret;
> -
> -	hibmc_fb = kzalloc(sizeof(*hibmc_fb), GFP_KERNEL);
> -	if (!hibmc_fb) {
> -		DRM_ERROR("failed to allocate hibmc_fb\n");
> -		return ERR_PTR(-ENOMEM);
> -	}
> -
> -	drm_helper_mode_fill_fb_struct(dev, &hibmc_fb->fb, mode_cmd);
> -	hibmc_fb->obj = obj;
> -	ret = drm_framebuffer_init(dev, &hibmc_fb->fb, &hibmc_fb_funcs);
> -	if (ret) {
> -		DRM_ERROR("drm_framebuffer_init failed: %d\n", ret);
> -		kfree(hibmc_fb);
> -		return ERR_PTR(ret);
> -	}
> -
> -	return hibmc_fb;
> -}
> -
> -static struct drm_framebuffer *
> -hibmc_user_framebuffer_create(struct drm_device *dev,
> -			      struct drm_file *filp,
> -			      const struct drm_mode_fb_cmd2 *mode_cmd)
> -{
> -	struct drm_gem_object *obj;
> -	struct hibmc_framebuffer *hibmc_fb;
> -
> -	DRM_DEBUG_DRIVER("%dx%d, format %c%c%c%c\n",
> -			 mode_cmd->width, mode_cmd->height,
> -			 (mode_cmd->pixel_format) & 0xff,
> -			 (mode_cmd->pixel_format >> 8)  & 0xff,
> -			 (mode_cmd->pixel_format >> 16) & 0xff,
> -			 (mode_cmd->pixel_format >> 24) & 0xff);
> -
> -	obj = drm_gem_object_lookup(filp, mode_cmd->handles[0]);
> -	if (!obj)
> -		return ERR_PTR(-ENOENT);
> -
> -	hibmc_fb = hibmc_framebuffer_init(dev, mode_cmd, obj);
> -	if (IS_ERR(hibmc_fb)) {
> -		drm_gem_object_put_unlocked(obj);
> -		return ERR_PTR((long)hibmc_fb);
> -	}
> -	return &hibmc_fb->fb;
> -}
> -
>  const struct drm_mode_config_funcs hibmc_mode_funcs = {
>  	.atomic_check = drm_atomic_helper_check,
>  	.atomic_commit = drm_atomic_helper_commit,
> -	.fb_create = hibmc_user_framebuffer_create,
> +	.fb_create = drm_gem_fb_create,
>  };
> -- 
> 2.23.0
> 

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