drm/rockchip: Use drm_fbdev_generic_setup()

Other than using a rockchip_gem_object directly, the Rockchip fbdev
setup has nothing special and can be converted to use the generic fbdev
emulation instead.

This patch makes full use of the generic fbdev emulation by using its
drm_client callbacks. This means that
drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
now handled by the emulation code. Additionally fbdev unregister happens
automatically on drm_dev_unregister().

The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
driver. This is done to highlight the fact that fbdev emulation is an
internal client that makes use of the driver, it is not part of the
driver as such. If fbdev setup fails, an error is printed, but the driver
succeeds probing.

Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: Sandy Huang <hjc@rock-chips.com>
Cc: "Heiko Stübner" <heiko@sntech.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-rockchip@lists.infradead.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/gpu/drm/rockchip/Makefile             |   1 -
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c   |  10 +-
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h   |   2 -
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c    |  14 --
 drivers/gpu/drm/rockchip/rockchip_drm_fb.h    |   6 -
 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 183 ------------------
 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h |  32 ---
 7 files changed, 2 insertions(+), 246 deletions(-)
 delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c
 delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h

Den 02.02.2019 18.07, skrev Rob Herring:
> Other than using a rockchip_gem_object directly, the Rockchip fbdev
> setup has nothing special and can be converted to use the generic fbdev
> emulation instead.
> 
> This patch makes full use of the generic fbdev emulation by using its
> drm_client callbacks. This means that
> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> now handled by the emulation code. Additionally fbdev unregister happens
> automatically on drm_dev_unregister().
> 
> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
> driver. This is done to highlight the fact that fbdev emulation is an
> internal client that makes use of the driver, it is not part of the
> driver as such. If fbdev setup fails, an error is printed, but the driver
> succeeds probing.
> 
> Cc: Noralf Trønnes <noralf@tronnes.org>
> Cc: Sandy Huang <hjc@rock-chips.com>
> Cc: "Heiko Stübner" <heiko@sntech.de>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-rockchip@lists.infradead.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/gpu/drm/rockchip/Makefile             |   1 -
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c   |  10 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.h   |   2 -
>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c    |  14 --
>  drivers/gpu/drm/rockchip/rockchip_drm_fb.h    |   6 -
>  drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 183 ------------------
>  drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h |  32 ---
>  7 files changed, 2 insertions(+), 246 deletions(-)
>  delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c
>  delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h
> 
> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
> index f6fc9d5dd0ad..dff1d0962ff7 100644
> --- a/drivers/gpu/drm/rockchip/Makefile
> +++ b/drivers/gpu/drm/rockchip/Makefile
> @@ -6,7 +6,6 @@
>  rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
>  		rockchip_drm_gem.o rockchip_drm_psr.o \
>  		rockchip_drm_vop.o rockchip_vop_reg.o
> -rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
>  
>  rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
>  rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index be6c2573039a..7089f04970ab 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -31,7 +31,6 @@
>  
>  #include "rockchip_drm_drv.h"
>  #include "rockchip_drm_fb.h"
> -#include "rockchip_drm_fbdev.h"
>  #include "rockchip_drm_gem.h"
>  
>  #define DRIVER_NAME	"rockchip"
> @@ -162,10 +161,6 @@ static int rockchip_drm_bind(struct device *dev)
>  	 */
>  	drm_dev->irq_enabled = true;
>  
> -	ret = rockchip_drm_fbdev_init(drm_dev);
> -	if (ret)
> -		goto err_unbind_all;
> -
>  	/* init kms poll for handling hpd */
>  	drm_kms_helper_poll_init(drm_dev);
>  
> @@ -173,10 +168,11 @@ static int rockchip_drm_bind(struct device *dev)
>  	if (ret)
>  		goto err_kms_helper_poll_fini;
>  
> +	drm_fbdev_generic_setup(drm_dev, 32);
> +
>  	return 0;
>  err_kms_helper_poll_fini:
>  	drm_kms_helper_poll_fini(drm_dev);
> -	rockchip_drm_fbdev_fini(drm_dev);
>  err_unbind_all:
>  	component_unbind_all(dev, drm_dev);
>  err_mode_config_cleanup:
> @@ -195,7 +191,6 @@ static void rockchip_drm_unbind(struct device *dev)
>  
>  	drm_dev_unregister(drm_dev);
>  
> -	rockchip_drm_fbdev_fini(drm_dev);
>  	drm_kms_helper_poll_fini(drm_dev);
>  
>  	drm_atomic_helper_shutdown(drm_dev);
> @@ -222,7 +217,6 @@ static const struct file_operations rockchip_drm_driver_fops = {
>  static struct drm_driver rockchip_drm_driver = {
>  	.driver_features	= DRIVER_MODESET | DRIVER_GEM |
>  				  DRIVER_PRIME | DRIVER_ATOMIC,
> -	.lastclose		= drm_fb_helper_lastclose,
>  	.gem_vm_ops		= &drm_gem_cma_vm_ops,
>  	.gem_free_object_unlocked = rockchip_gem_free_object,
>  	.dumb_create		= rockchip_gem_dumb_create,
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index ce48568ec8a0..7f4ebd682ad2 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -50,8 +50,6 @@ struct rockchip_crtc_state {
>   * @mm_lock: protect drm_mm on multi-threads.
>   */
>  struct rockchip_drm_private {
> -	struct drm_fb_helper fbdev_helper;
> -	struct drm_gem_object *fbdev_bo;
>  	struct iommu_domain *domain;
>  	struct mutex mm_lock;
>  	struct drm_mm mm;
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> index ea18cb2a76c0..5a008efa1e7d 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> @@ -197,20 +197,6 @@ static const struct drm_mode_config_funcs rockchip_drm_mode_config_funcs = {
>  	.atomic_commit = drm_atomic_helper_commit,
>  };
>  
> -struct drm_framebuffer *
> -rockchip_drm_framebuffer_init(struct drm_device *dev,
> -			      const struct drm_mode_fb_cmd2 *mode_cmd,
> -			      struct drm_gem_object *obj)
> -{
> -	struct drm_framebuffer *fb;
> -
> -	fb = rockchip_fb_alloc(dev, mode_cmd, &obj, 1);
> -	if (IS_ERR(fb))
> -		return ERR_CAST(fb);
> -
> -	return fb;
> -}
> -
>  void rockchip_drm_mode_config_init(struct drm_device *dev)
>  {
>  	dev->mode_config.min_width = 0;
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.h b/drivers/gpu/drm/rockchip/rockchip_drm_fb.h
> index f1265cb1aee8..69271df96fa6 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.h
> @@ -15,11 +15,5 @@
>  #ifndef _ROCKCHIP_DRM_FB_H
>  #define _ROCKCHIP_DRM_FB_H
>  
> -struct drm_framebuffer *
> -rockchip_drm_framebuffer_init(struct drm_device *dev,
> -			      const struct drm_mode_fb_cmd2 *mode_cmd,
> -			      struct drm_gem_object *obj);
> -void rockchip_drm_framebuffer_fini(struct drm_framebuffer *fb);
> -
>  void rockchip_drm_mode_config_init(struct drm_device *dev);
>  #endif /* _ROCKCHIP_DRM_FB_H */
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c
> deleted file mode 100644
> index e6650553f5d6..000000000000
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c
> +++ /dev/null
> @@ -1,183 +0,0 @@
> -/*
> - * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
> - * Author:Mark Yao <mark.yao@rock-chips.com>
> - *
> - * This software is licensed under the terms of the GNU General Public
> - * License version 2, as published by the Free Software Foundation, and
> - * may be copied, distributed, and modified under those terms.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
> - */
> -
> -#include <drm/drm.h>
> -#include <drm/drmP.h>
> -#include <drm/drm_fb_helper.h>
> -#include <drm/drm_crtc_helper.h>
> -
> -#include "rockchip_drm_drv.h"
> -#include "rockchip_drm_gem.h"
> -#include "rockchip_drm_fb.h"
> -#include "rockchip_drm_fbdev.h"
> -
> -#define PREFERRED_BPP		32
> -#define to_drm_private(x) \
> -		container_of(x, struct rockchip_drm_private, fbdev_helper)
> -
> -static int rockchip_fbdev_mmap(struct fb_info *info,
> -			       struct vm_area_struct *vma)
> -{
> -	struct drm_fb_helper *helper = info->par;
> -	struct rockchip_drm_private *private = to_drm_private(helper);
> -
> -	return rockchip_gem_mmap_buf(private->fbdev_bo, vma);
> -}
> -
> -static struct fb_ops rockchip_drm_fbdev_ops = {
> -	.owner		= THIS_MODULE,
> -	DRM_FB_HELPER_DEFAULT_OPS,
> -	.fb_mmap	= rockchip_fbdev_mmap,
> -	.fb_fillrect	= drm_fb_helper_cfb_fillrect,
> -	.fb_copyarea	= drm_fb_helper_cfb_copyarea,
> -	.fb_imageblit	= drm_fb_helper_cfb_imageblit,
> -};
> -
> -static int rockchip_drm_fbdev_create(struct drm_fb_helper *helper,
> -				     struct drm_fb_helper_surface_size *sizes)
> -{
> -	struct rockchip_drm_private *private = to_drm_private(helper);
> -	struct drm_mode_fb_cmd2 mode_cmd = { 0 };
> -	struct drm_device *dev = helper->dev;
> -	struct rockchip_gem_object *rk_obj;
> -	struct drm_framebuffer *fb;
> -	unsigned int bytes_per_pixel;
> -	unsigned long offset;
> -	struct fb_info *fbi;
> -	size_t size;
> -	int ret;
> -
> -	bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
> -
> -	mode_cmd.width = sizes->surface_width;
> -	mode_cmd.height = sizes->surface_height;
> -	mode_cmd.pitches[0] = sizes->surface_width * bytes_per_pixel;
> -	mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
> -		sizes->surface_depth);
> -
> -	size = mode_cmd.pitches[0] * mode_cmd.height;
> -
> -	rk_obj = rockchip_gem_create_object(dev, size, true);

I don't think the generic emulation in it's current form will work for
rockchip. rockchip treats fbdev buffers and dumb buffers differently. If
it uses DMA buffers, then the dumb buffer can't get a virtual address.

From rockchip_gem_alloc_dma():

	if (!alloc_kmap)
		rk_obj->dma_attrs |= DMA_ATTR_NO_KERNEL_MAPPING;

From rockchip_gem_prime_vmap():

	if (rk_obj->dma_attrs & DMA_ATTR_NO_KERNEL_MAPPING)
		return NULL;

Maybe it's possible to vmap a buffer created using dma_alloc_attrs()
after the allocation?

Noralf.

> -	if (IS_ERR(rk_obj))
> -		return -ENOMEM;
> -
> -	private->fbdev_bo = &rk_obj->base;
> -
> -	fbi = drm_fb_helper_alloc_fbi(helper);
> -	if (IS_ERR(fbi)) {
> -		DRM_DEV_ERROR(dev->dev, "Failed to create framebuffer info.\n");
> -		ret = PTR_ERR(fbi);
> -		goto out;
> -	}
> -
> -	helper->fb = rockchip_drm_framebuffer_init(dev, &mode_cmd,
> -						   private->fbdev_bo);
> -	if (IS_ERR(helper->fb)) {
> -		DRM_DEV_ERROR(dev->dev,
> -			      "Failed to allocate DRM framebuffer.\n");
> -		ret = PTR_ERR(helper->fb);
> -		goto out;
> -	}
> -
> -	fbi->par = helper;
> -	fbi->flags = FBINFO_FLAG_DEFAULT;
> -	fbi->fbops = &rockchip_drm_fbdev_ops;
> -
> -	fb = helper->fb;
> -	drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->format->depth);
> -	drm_fb_helper_fill_var(fbi, helper, sizes->fb_width, sizes->fb_height);
> -
> -	offset = fbi->var.xoffset * bytes_per_pixel;
> -	offset += fbi->var.yoffset * fb->pitches[0];
> -
> -	dev->mode_config.fb_base = 0;
> -	fbi->screen_base = rk_obj->kvaddr + offset;
> -	fbi->screen_size = rk_obj->base.size;
> -	fbi->fix.smem_len = rk_obj->base.size;
> -
> -	DRM_DEBUG_KMS("FB [%dx%d]-%d kvaddr=%p offset=%ld size=%zu\n",
> -		      fb->width, fb->height, fb->format->depth,
> -		      rk_obj->kvaddr,
> -		      offset, size);
> -
> -	fbi->skip_vt_switch = true;
> -
> -	return 0;
> -
> -out:
> -	rockchip_gem_free_object(&rk_obj->base);
> -	return ret;
> -}
> -
> -static const struct drm_fb_helper_funcs rockchip_drm_fb_helper_funcs = {
> -	.fb_probe = rockchip_drm_fbdev_create,
> -};
> -
> -int rockchip_drm_fbdev_init(struct drm_device *dev)
> -{
> -	struct rockchip_drm_private *private = dev->dev_private;
> -	struct drm_fb_helper *helper;
> -	int ret;
> -
> -	if (!dev->mode_config.num_crtc || !dev->mode_config.num_connector)
> -		return -EINVAL;
> -
> -	helper = &private->fbdev_helper;
> -
> -	drm_fb_helper_prepare(dev, helper, &rockchip_drm_fb_helper_funcs);
> -
> -	ret = drm_fb_helper_init(dev, helper, ROCKCHIP_MAX_CONNECTOR);
> -	if (ret < 0) {
> -		DRM_DEV_ERROR(dev->dev,
> -			      "Failed to initialize drm fb helper - %d.\n",
> -			      ret);
> -		return ret;
> -	}
> -
> -	ret = drm_fb_helper_single_add_all_connectors(helper);
> -	if (ret < 0) {
> -		DRM_DEV_ERROR(dev->dev,
> -			      "Failed to add connectors - %d.\n", ret);
> -		goto err_drm_fb_helper_fini;
> -	}
> -
> -	ret = drm_fb_helper_initial_config(helper, PREFERRED_BPP);
> -	if (ret < 0) {
> -		DRM_DEV_ERROR(dev->dev,
> -			      "Failed to set initial hw config - %d.\n",
> -			      ret);
> -		goto err_drm_fb_helper_fini;
> -	}
> -
> -	return 0;
> -
> -err_drm_fb_helper_fini:
> -	drm_fb_helper_fini(helper);
> -	return ret;
> -}
> -
> -void rockchip_drm_fbdev_fini(struct drm_device *dev)
> -{
> -	struct rockchip_drm_private *private = dev->dev_private;
> -	struct drm_fb_helper *helper;
> -
> -	helper = &private->fbdev_helper;
> -
> -	drm_fb_helper_unregister_fbi(helper);
> -
> -	if (helper->fb)
> -		drm_framebuffer_put(helper->fb);
> -
> -	drm_fb_helper_fini(helper);
> -}
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h
> deleted file mode 100644
> index 73718c5f5bbf..000000000000
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -/*
> - * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
> - * Author:Mark Yao <mark.yao@rock-chips.com>
> - *
> - * This software is licensed under the terms of the GNU General Public
> - * License version 2, as published by the Free Software Foundation, and
> - * may be copied, distributed, and modified under those terms.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
> - */
> -
> -#ifndef _ROCKCHIP_DRM_FBDEV_H
> -#define _ROCKCHIP_DRM_FBDEV_H
> -
> -#ifdef CONFIG_DRM_FBDEV_EMULATION
> -int rockchip_drm_fbdev_init(struct drm_device *dev);
> -void rockchip_drm_fbdev_fini(struct drm_device *dev);
> -#else
> -static inline int rockchip_drm_fbdev_init(struct drm_device *dev)
> -{
> -	return 0;
> -}
> -
> -static inline void rockchip_drm_fbdev_fini(struct drm_device *dev)
> -{
> -}
> -#endif
> -
> -#endif /* _ROCKCHIP_DRM_FBDEV_H */
>

On Sat, Feb 2, 2019 at 12:22 PM Noralf Trønnes <noralf@tronnes.org> wrote:
> Den 02.02.2019 18.07, skrev Rob Herring:
> > Other than using a rockchip_gem_object directly, the Rockchip fbdev
> > setup has nothing special and can be converted to use the generic fbdev
> > emulation instead.

[...]

> > -static int rockchip_drm_fbdev_create(struct drm_fb_helper *helper,
> > -                                  struct drm_fb_helper_surface_size *sizes)
> > -{
> > -     struct rockchip_drm_private *private = to_drm_private(helper);
> > -     struct drm_mode_fb_cmd2 mode_cmd = { 0 };
> > -     struct drm_device *dev = helper->dev;
> > -     struct rockchip_gem_object *rk_obj;
> > -     struct drm_framebuffer *fb;
> > -     unsigned int bytes_per_pixel;
> > -     unsigned long offset;
> > -     struct fb_info *fbi;
> > -     size_t size;
> > -     int ret;
> > -
> > -     bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
> > -
> > -     mode_cmd.width = sizes->surface_width;
> > -     mode_cmd.height = sizes->surface_height;
> > -     mode_cmd.pitches[0] = sizes->surface_width * bytes_per_pixel;
> > -     mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
> > -             sizes->surface_depth);
> > -
> > -     size = mode_cmd.pitches[0] * mode_cmd.height;
> > -
> > -     rk_obj = rockchip_gem_create_object(dev, size, true);
>
> I don't think the generic emulation in it's current form will work for
> rockchip. rockchip treats fbdev buffers and dumb buffers differently. If
> it uses DMA buffers, then the dumb buffer can't get a virtual address.

Yes, you are right. I tested the iommu case.

> From rockchip_gem_alloc_dma():
>
>         if (!alloc_kmap)
>                 rk_obj->dma_attrs |= DMA_ATTR_NO_KERNEL_MAPPING;
>
> From rockchip_gem_prime_vmap():
>
>         if (rk_obj->dma_attrs & DMA_ATTR_NO_KERNEL_MAPPING)
>                 return NULL;
>
> Maybe it's possible to vmap a buffer created using dma_alloc_attrs()
> after the allocation?

It should be possible I think as that is what
drm_gem_cma_prime_import_sg_table_vmap() does. One problem though is
you may already have a mapping because DMA_ATTR_NO_KERNEL_MAPPING is
just a suggestion (only 32-bit ARM implements) and there's not a way
to tell if you got a mapping or not. Maybe it's not all that important
if there are 2 mappings because I think only fbcon needs a kernel
mapping.

My follow-up to this was going to be converting Rockchip to use CMA
and the shmem helpers. So I was already wondering about what to do
with DMA_ATTR_NO_KERNEL_MAPPING. There's several drivers not using CMA
just to set DMA_ATTR_NO_KERNEL_MAPPING. So it would be good to come up
with a solution here.

Rob

Den 04.02.2019 16.01, skrev Rob Herring:
> On Sat, Feb 2, 2019 at 12:22 PM Noralf Trønnes <noralf@tronnes.org> wrote:
>> Den 02.02.2019 18.07, skrev Rob Herring:
>>> Other than using a rockchip_gem_object directly, the Rockchip fbdev
>>> setup has nothing special and can be converted to use the generic fbdev
>>> emulation instead.
> 
> [...]
> 
>>> -static int rockchip_drm_fbdev_create(struct drm_fb_helper *helper,
>>> -                                  struct drm_fb_helper_surface_size *sizes)
>>> -{
>>> -     struct rockchip_drm_private *private = to_drm_private(helper);
>>> -     struct drm_mode_fb_cmd2 mode_cmd = { 0 };
>>> -     struct drm_device *dev = helper->dev;
>>> -     struct rockchip_gem_object *rk_obj;
>>> -     struct drm_framebuffer *fb;
>>> -     unsigned int bytes_per_pixel;
>>> -     unsigned long offset;
>>> -     struct fb_info *fbi;
>>> -     size_t size;
>>> -     int ret;
>>> -
>>> -     bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
>>> -
>>> -     mode_cmd.width = sizes->surface_width;
>>> -     mode_cmd.height = sizes->surface_height;
>>> -     mode_cmd.pitches[0] = sizes->surface_width * bytes_per_pixel;
>>> -     mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
>>> -             sizes->surface_depth);
>>> -
>>> -     size = mode_cmd.pitches[0] * mode_cmd.height;
>>> -
>>> -     rk_obj = rockchip_gem_create_object(dev, size, true);
>>
>> I don't think the generic emulation in it's current form will work for
>> rockchip. rockchip treats fbdev buffers and dumb buffers differently. If
>> it uses DMA buffers, then the dumb buffer can't get a virtual address.
> 
> Yes, you are right. I tested the iommu case.
> 
>> From rockchip_gem_alloc_dma():
>>
>>         if (!alloc_kmap)
>>                 rk_obj->dma_attrs |= DMA_ATTR_NO_KERNEL_MAPPING;
>>
>> From rockchip_gem_prime_vmap():
>>
>>         if (rk_obj->dma_attrs & DMA_ATTR_NO_KERNEL_MAPPING)
>>                 return NULL;
>>
>> Maybe it's possible to vmap a buffer created using dma_alloc_attrs()
>> after the allocation?
> 
> It should be possible I think as that is what
> drm_gem_cma_prime_import_sg_table_vmap() does. One problem though is
> you may already have a mapping because DMA_ATTR_NO_KERNEL_MAPPING is
> just a suggestion (only 32-bit ARM implements) and there's not a way
> to tell if you got a mapping or not. Maybe it's not all that important
> if there are 2 mappings because I think only fbcon needs a kernel
> mapping.
> 
> My follow-up to this was going to be converting Rockchip to use CMA
> and the shmem helpers. So I was already wondering about what to do
> with DMA_ATTR_NO_KERNEL_MAPPING. There's several drivers not using CMA
> just to set DMA_ATTR_NO_KERNEL_MAPPING. So it would be good to come up
> with a solution here.
> 

Here's part of a discussion concerning the mediatek driver (couldn't
find it in the dri-devel archives):

Re: [PATCH] drm/mediatek: Add MTK Framebuffer-Device (mt7623)

> On Fri, 2019-01-11 at 17:07 +0100, Noralf Trønnes wrote:
...
> > The problem here is that this driver doesn't map a virtual address for
> > its buffers:
> > 	mtk_gem->dma_attrs |= DMA_ATTR_NO_KERNEL_MAPPING;
> >
> > Probably because of limited virtual address space.
> > rockchip is in the same situation.
> >
> > I'm aware of this shortcoming of the generic emulation, but I don't see
> > how it can be solved without using somekind of flag attached to the dumb
> > buffer creation request.
>
> The virtual address space is limited so we do not map it by default. I
> also see the shortcoming of the generic emulation, so I would refer to
> rockchip to implement this mapping.
>
> Regards,
> CK

Noralf.

On Mon, Feb 4, 2019 at 9:01 AM Rob Herring <robh@kernel.org> wrote:
>
> On Sat, Feb 2, 2019 at 12:22 PM Noralf Trønnes <noralf@tronnes.org> wrote:
> > Den 02.02.2019 18.07, skrev Rob Herring:
> > > Other than using a rockchip_gem_object directly, the Rockchip fbdev
> > > setup has nothing special and can be converted to use the generic fbdev
> > > emulation instead.
>
> [...]
>
> > > -static int rockchip_drm_fbdev_create(struct drm_fb_helper *helper,
> > > -                                  struct drm_fb_helper_surface_size *sizes)
> > > -{
> > > -     struct rockchip_drm_private *private = to_drm_private(helper);
> > > -     struct drm_mode_fb_cmd2 mode_cmd = { 0 };
> > > -     struct drm_device *dev = helper->dev;
> > > -     struct rockchip_gem_object *rk_obj;
> > > -     struct drm_framebuffer *fb;
> > > -     unsigned int bytes_per_pixel;
> > > -     unsigned long offset;
> > > -     struct fb_info *fbi;
> > > -     size_t size;
> > > -     int ret;
> > > -
> > > -     bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
> > > -
> > > -     mode_cmd.width = sizes->surface_width;
> > > -     mode_cmd.height = sizes->surface_height;
> > > -     mode_cmd.pitches[0] = sizes->surface_width * bytes_per_pixel;
> > > -     mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
> > > -             sizes->surface_depth);
> > > -
> > > -     size = mode_cmd.pitches[0] * mode_cmd.height;
> > > -
> > > -     rk_obj = rockchip_gem_create_object(dev, size, true);
> >
> > I don't think the generic emulation in it's current form will work for
> > rockchip. rockchip treats fbdev buffers and dumb buffers differently. If
> > it uses DMA buffers, then the dumb buffer can't get a virtual address.
>
> Yes, you are right. I tested the iommu case.
>
> > From rockchip_gem_alloc_dma():
> >
> >         if (!alloc_kmap)
> >                 rk_obj->dma_attrs |= DMA_ATTR_NO_KERNEL_MAPPING;
> >
> > From rockchip_gem_prime_vmap():
> >
> >         if (rk_obj->dma_attrs & DMA_ATTR_NO_KERNEL_MAPPING)
> >                 return NULL;
> >
> > Maybe it's possible to vmap a buffer created using dma_alloc_attrs()
> > after the allocation?
>
> It should be possible I think as that is what
> drm_gem_cma_prime_import_sg_table_vmap() does. One problem though is
> you may already have a mapping because DMA_ATTR_NO_KERNEL_MAPPING is
> just a suggestion (only 32-bit ARM implements) and there's not a way
> to tell if you got a mapping or not. Maybe it's not all that important
> if there are 2 mappings because I think only fbcon needs a kernel
> mapping.

As another data point, exynos always uses DMA_ATTR_NO_KERNEL_MAPPING
and then vmap's its fbdev buffer.

Rob