Re: [RFC v2 7/8] drm/fb-helper: Add generic fbdev emulation

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jan 03, 2018 at 11:21:09PM +0100, Noralf Trønnes wrote:
> Add generic fbdev emulation which uses a drm_file to get a dumb_buffer
> and drm_framebuffer. The buffer is exported and vmap/mmap called on
> the dma-buf.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 301 +++++++++++++++++++++++++++++++++++++++-
>  include/drm/drm_fb_helper.h     |  33 +++++
>  2 files changed, 333 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index f9dcc7a5761f..270ff6dc8045 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -30,12 +30,15 @@
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
>  #include <linux/console.h>
> +#include <linux/dma-buf.h>
>  #include <linux/kernel.h>
>  #include <linux/sysrq.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
>  #include <drm/drmP.h>
> +#include <drm/drm_auth.h>
>  #include <drm/drm_crtc.h>
> +#include <drm/drm_dumb_buffers.h>
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_atomic.h>
> @@ -1951,7 +1954,9 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>  	if (fb_helper->fbdev->fbops->fb_open == drm_fb_helper_fb_open)
>  		atomic_set(&fb_helper->open_count, 0);
>  
> -	strcpy(fb_helper->fb->comm, "[fbcon]");
> +	if (fb_helper->fb)
> +		strcpy(fb_helper->fb->comm, "[fbcon]");
> +
>  	return 0;
>  }
>  
> @@ -2975,6 +2980,300 @@ void drm_fb_helper_output_poll_changed(struct drm_device *dev)
>  }
>  EXPORT_SYMBOL(drm_fb_helper_output_poll_changed);
>  
> +static struct fb_deferred_io drm_fb_helper_generic_defio = {
> +	.delay		= HZ / 20,
> +	.deferred_io	= drm_fb_helper_deferred_io,
> +};
> +
> +static int drm_fb_helper_generic_alloc_buf(struct drm_fb_helper *fb_helper)
> +{
> +	struct drm_fb_helper_surface_size *sizes = &fb_helper->sizes;
> +	struct drm_mode_create_dumb dumb_args = { 0 };
> +	struct drm_prime_handle prime_args = { 0 };
> +	struct drm_mode_fb_cmd2 fb_args = { 0 };
> +	struct drm_device *dev = fb_helper->dev;
> +	struct fb_info *fbi = fb_helper->fbdev;
> +	struct drm_framebuffer *fb;
> +	struct dma_buf *dma_buf;
> +	struct drm_file *file;
> +	void *vaddr;
> +	int ret;
> +
> +	file = drm_file_alloc(dev->primary);
> +	if (IS_ERR(file))
> +		return PTR_ERR(file);
> +
> +	drm_dropmaster_ioctl(dev, NULL, file);

Hm .... why do we need this? Feels a bit like drm_file_alloc shouldn't do
the entire master dance for us ...

Otherwise this looks awesome, I really like it.

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

> +
> +	dumb_args.width = sizes->surface_width;
> +	dumb_args.height = sizes->surface_height;
> +	dumb_args.bpp = sizes->surface_bpp;
> +	ret = drm_mode_create_dumb_ioctl(dev, &dumb_args, file);
> +	if (ret)
> +		goto err_free_file;
> +
> +	fb_args.width = dumb_args.width;
> +	fb_args.height = dumb_args.height;
> +	fb_args.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
> +							 sizes->surface_depth);
> +	fb_args.handles[0] = dumb_args.handle;
> +	fb_args.pitches[0] = dumb_args.pitch;
> +	ret = drm_mode_addfb2(dev, &fb_args, file);
> +	if (ret)
> +		goto err_free_file;
> +
> +	fb = drm_framebuffer_lookup(dev, file, fb_args.fb_id);
> +	if (!fb) {
> +		ret = -ENOENT;
> +		goto err_free_file;
> +	}
> +
> +	/* drop the reference we picked up in framebuffer lookup */
> +	drm_framebuffer_put(fb);
> +
> +	strcpy(fb->comm, "[fbcon]");
> +
> +	prime_args.handle = dumb_args.handle;
> +	ret = drm_prime_handle_to_fd_ioctl(dev, &prime_args, file);
> +	if (ret)
> +		goto err_free_file;
> +
> +	dma_buf = dma_buf_get(prime_args.fd);
> +	if (WARN_ON(IS_ERR(dma_buf))) {
> +		ret = PTR_ERR(dma_buf);
> +		goto err_free_file;
> +	}
> +
> +	vaddr = dma_buf_vmap(dma_buf);
> +	if (!vaddr) {
> +		ret = -ENOMEM;
> +		goto err_put_dmabuf;
> +	}
> +
> +	if (fb->funcs->dirty) {
> +		fbi->fbdefio = &drm_fb_helper_generic_defio;
> +		fb_deferred_io_init(fbi);
> +	}
> +
> +	fbi->screen_size = fb->height * fb->pitches[0];
> +	fbi->fix.smem_len = fbi->screen_size;
> +	fbi->screen_buffer = vaddr;
> +
> +	fb_helper->dma_buf = dma_buf;
> +	fb_helper->file = file;
> +
> +	mutex_lock(&fb_helper->lock);
> +	fb_helper->fb = fb;
> +	drm_setup_crtcs_fb(fb_helper);
> +	mutex_unlock(&fb_helper->lock);
> +
> +	/* First time setup */
> +	if (!fbi->var.bits_per_pixel) {
> +		struct fb_videomode mode;
> +
> +		drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->format->depth);
> +		drm_fb_helper_fill_var(fbi, fb_helper, sizes->fb_width, sizes->fb_height);
> +
> +		/* Drop the mode added by register_framebuffer() */
> +		fb_destroy_modelist(&fb_helper->fbdev->modelist);
> +
> +		fb_var_to_videomode(&mode, &fbi->var);
> +		fb_add_videomode(&mode, &fbi->modelist);
> +	}
> +
> +	return 0;
> +
> +err_put_dmabuf:
> +	dma_buf_put(dma_buf);
> +err_free_file:
> +	drm_file_free(file);
> +
> +	return ret;
> +}
> +
> +static void drm_fb_helper_generic_free_buf(struct drm_fb_helper *fb_helper)
> +{
> +	mutex_lock(&fb_helper->lock);
> +	fb_helper->fb = NULL;
> +	drm_setup_crtcs_fb(fb_helper);
> +	mutex_unlock(&fb_helper->lock);
> +
> +	if (fb_helper->fbdev->fbdefio) {
> +		cancel_delayed_work_sync(&fb_helper->fbdev->deferred_work);
> +		cancel_work_sync(&fb_helper->dirty_work);
> +		fb_deferred_io_cleanup(fb_helper->fbdev);
> +	}
> +
> +	dma_buf_vunmap(fb_helper->dma_buf, fb_helper->fbdev->screen_buffer);
> +	dma_buf_put(fb_helper->dma_buf);
> +	drm_file_free(fb_helper->file);
> +
> +	fb_helper->fbdev->screen_buffer = NULL;
> +	fb_helper->dma_buf = NULL;
> +	fb_helper->file = NULL;
> +}
> +
> +static int drm_fb_helper_generic_fb_open(struct fb_info *info, int user)
> +{
> +	struct drm_fb_helper *fb_helper = info->par;
> +	int ret;
> +
> +	ret = drm_fb_helper_fb_open(info, user);
> +	if (ret)
> +		return ret;
> +
> +	if (!fb_helper->fbdev->screen_buffer) {
> +		/*
> +		 * Exporting a buffer to get a virtual address results in
> +		 * dma-buf pinning the driver module. This means that we have
> +		 * to defer this to open/close in order to unload the driver
> +		 * module.
> +		 */
> +		ret = drm_fb_helper_generic_alloc_buf(fb_helper);
> +		if (ret) {
> +			DRM_ERROR("fbdev: Failed to allocate buffer: %d\n", ret);
> +			drm_fb_helper_fb_release(info, user);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int drm_fb_helper_generic_fb_release(struct fb_info *info, int user)
> +{
> +	struct drm_fb_helper *fb_helper = info->par;
> +
> +	drm_fb_helper_fb_release(info, user);
> +
> +	if (!atomic_read(&fb_helper->open_count))
> +		drm_fb_helper_generic_free_buf(fb_helper);
> +
> +	return 0;
> +}
> +
> +static int drm_fb_helper_generic_fb_mmap(struct fb_info *info,
> +					 struct vm_area_struct *vma)
> +{
> +	struct drm_fb_helper *fb_helper = info->par;
> +
> +	return dma_buf_mmap(fb_helper->dma_buf, vma, 0);
> +}
> +
> +static struct fb_ops drm_fb_helper_generic_fbdev_ops = {
> +	.owner		= THIS_MODULE,
> +	DRM_FB_HELPER_DEFAULT_OPS,
> +	.fb_open	= drm_fb_helper_generic_fb_open,
> +	.fb_release	= drm_fb_helper_generic_fb_release,
> +	.fb_mmap	= drm_fb_helper_generic_fb_mmap,
> +	.fb_read	= drm_fb_helper_sys_read,
> +	.fb_write	= drm_fb_helper_sys_write,
> +	.fb_fillrect	= drm_fb_helper_sys_fillrect,
> +	.fb_copyarea	= drm_fb_helper_sys_copyarea,
> +	.fb_imageblit	= drm_fb_helper_sys_imageblit,
> +};
> +
> +static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
> +				       struct drm_fb_helper_surface_size *sizes)
> +{
> +	struct fb_ops *fbops;
> +	struct fb_info *fbi;
> +
> +	DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n",
> +		      sizes->surface_width, sizes->surface_height,
> +		      sizes->surface_bpp);
> +
> +	fb_helper->sizes = *sizes;
> +
> +	/*
> +	 * fb_deferred_io_cleanup() clears &fbops->fb_mmap so a per instance
> +	 * version is necessary. We do it for all users since we don't know
> +	 * yet if the fb has a dirty callback. This also gives us the
> +	 * opportunity to set the correct owner.
> +	 */
> +	fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
> +	if (!fbops)
> +		return -ENOMEM;
> +
> +	*fbops = drm_fb_helper_generic_fbdev_ops;
> +	fbops->owner = fb_helper->dev->driver->fops->owner;
> +
> +	fbi = drm_fb_helper_alloc_fbi(fb_helper);
> +	if (IS_ERR(fbi)) {
> +		kfree(fbops);
> +		return PTR_ERR(fbi);
> +	}
> +
> +	fbi->par = fb_helper;
> +	fbi->fbops = fbops;
> +	strcpy(fbi->fix.id, "generic");
> +
> +	/* The rest of the setup is deferred to fb_open */
> +
> +	atomic_set(&fb_helper->open_count, 0);
> +
> +	return 0;
> +}
> +
> +static void drm_fb_helper_generic_release(struct drm_fb_helper *fb_helper)
> +{
> +	struct fb_ops *fbops = fb_helper->fbdev->fbops;
> +
> +	drm_fb_helper_fini(fb_helper);
> +	kfree(fb_helper);
> +	kfree(fbops);
> +}
> +
> +static const struct drm_fb_helper_funcs drm_fb_helper_generic_funcs = {
> +	.fb_probe	= drm_fb_helper_generic_probe,
> +	.restore	= drm_fb_helper_restore_fbdev_mode_unlocked,
> +	.hotplug_event	= drm_fb_helper_hotplug_event,
> +	.unregister	= drm_fb_helper_unregister_fbi,
> +	.release	= drm_fb_helper_generic_release,
> +};
> +
> +/**
> + * drm_fb_helper_generic_fbdev_setup() - Setup generic fbdev emulation
> + * @dev: DRM device
> + * @preferred_bpp: Preferred bits per pixel for the device.
> + *                 @dev->mode_config.preferred_depth is used if this is zero.
> + * @max_conn_count: Maximum number of connectors.
> + *                  @dev->mode_config.num_connector is used if this is zero.
> + *
> + * This function sets up generic fbdev emulation for drivers that supports
> + * dumb buffers which can be exported. The driver doesn't have to do anything
> + * else than to call this function, restore, hotplug events and teardown are
> + * all taken care of.
> + *
> + * Returns:
> + * Zero on success or negative error code on failure.
> + */
> +int drm_fb_helper_generic_fbdev_setup(struct drm_device *dev,
> +				      unsigned int preferred_bpp,
> +				      unsigned int max_conn_count)
> +{
> +	struct drm_fb_helper *fb_helper;
> +	int ret;
> +
> +	if (!drm_fbdev_emulation)
> +		return 0;
> +
> +	fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
> +	if (!fb_helper)
> +		return -ENOMEM;
> +
> +	ret = drm_fb_helper_fbdev_setup(dev, fb_helper,
> +					&drm_fb_helper_generic_funcs,
> +					preferred_bpp, max_conn_count);
> +	if (ret) {
> +		kfree(fb_helper);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_fb_helper_generic_fbdev_setup);
> +
>  /* The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT)
>   * but the module doesn't depend on any fb console symbols.  At least
>   * attempt to load fbcon to avoid leaving the system without a usable console.
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 385f967c3552..c6940ab6ffac 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -279,6 +279,28 @@ struct drm_fb_helper {
>  	 * initial value to 0 themselves.
>  	 */
>  	atomic_t open_count;
> +
> +	/**
> +	 * @file:
> +	 *
> +	 * Optional DRM file. Used by the generic fbdev code.
> +	 */
> +	struct drm_file *file;
> +
> +	/**
> +	 * @dma_buf:
> +	 *
> +	 * Optional pointer to a DMA buffer object.
> +	 * Used by the generic fbdev code.
> +	 */
> +	struct dma_buf *dma_buf;
> +
> +	/**
> +	 * @sizes:
> +	 *
> +	 * Optional surface sizes. Used by the generic fbdev code.
> +	 */
> +	struct drm_fb_helper_surface_size sizes;
>  };
>  
>  /**
> @@ -380,6 +402,10 @@ void drm_fb_helper_fbdev_teardown(struct drm_device *dev);
>  
>  void drm_fb_helper_lastclose(struct drm_device *dev);
>  void drm_fb_helper_output_poll_changed(struct drm_device *dev);
> +
> +int drm_fb_helper_generic_fbdev_setup(struct drm_device *dev,
> +				      unsigned int preferred_bpp,
> +				      unsigned int max_conn_count);
>  #else
>  static inline void drm_fb_helper_prepare(struct drm_device *dev,
>  					struct drm_fb_helper *helper,
> @@ -624,6 +650,13 @@ static inline void drm_fb_helper_output_poll_changed(struct drm_device *dev)
>  {
>  }
>  
> +static inline int
> +drm_fb_helper_generic_fbdev_setup(struct drm_device *dev,
> +				  unsigned int preferred_bpp,
> +				  unsigned int max_conn_count)
> +{
> +	return 0;
> +}
>  #endif
>  
>  static inline int
> -- 
> 2.14.2
> 

-- 
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