linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] drm/mediatek: Add MTK Framebuffer-Device (mt7623)
       [not found] <20190102084917.4049-1-frank-w@public-files.de>
@ 2019-01-07 10:36 ` Daniel Vetter
  2019-01-08 13:40   ` Daniel Vetter
  0 siblings, 1 reply; 14+ messages in thread
From: Daniel Vetter @ 2019-01-07 10:36 UTC (permalink / raw)
  To: Frank Wunderlich
  Cc: Philipp Zabel, David Airlie, Alexander Ryabchenko, linux-kernel,
	dri-devel, Matthias Brugger, linux-mediatek, Daniel Vetter,
	CK Hu, linux-arm-kernel

On Wed, Jan 02, 2019 at 09:49:17AM +0100, Frank Wunderlich wrote:
> From: CK Hu <ck.hu@mediatek.com>
> 
> This patch adds Framebuffer-Driver for Mediatek
> 
> currently tested on mt7623, maybe works on other platforms
> MTK-FBDev written by CK Hu and ported from 4.4
> 
> based on patchset drm/hdmi for 7623 v5 (except last part included in 4.20)
>   https://patchwork.kernel.org/project/linux-mediatek/list/?series=25989
> 
> depends on
>   dts-patch (resend of 5/5, parts 1-4 already in 4.20):
>     "arm: dts: mt7623: add display subsystem related device nodes"
>     https://patchwork.kernel.org/patch/10588951/
>   2 bugfix-patches from bibby hsieh
>     fix-boot-up-for-720-and-480-but-1080
>     using-different-round-rate-for-mt7623
>   1 Patch from Ryder Lee
>     http://forum.banana-pi.org/t/kernel-4-19-rc1-for-testers/6618/52
> 
> full working tree here for reference:
> https://github.com/frank-w/BPI-R2-4.14/commits/4.20-hdmiv5
> 
> Signed-off-by: CK Hu <ck.hu@mediatek.com>
> Signed-off-by: Alexander Ryabchenko <d3adme4t@gmail.com>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> Tested-by: Frank Wunderlich <frank-w@public-files.de>

Would be good to use the new generic fbdev emulation code here, for even
less code. Or at least know why this isn't possible to use for mtk (and
maybe address that in the core code). Hand-rolling fbdev code shouldn't be
needed anymore.
-Daniel
> ---
>  drivers/gpu/drm/mediatek/Makefile        |   1 +
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c   |   6 +
>  drivers/gpu/drm/mediatek/mtk_drm_drv.h   |   4 +
>  drivers/gpu/drm/mediatek/mtk_drm_fb.c    |  13 ++
>  drivers/gpu/drm/mediatek/mtk_drm_fb.h    |   3 +
>  drivers/gpu/drm/mediatek/mtk_drm_fbdev.c | 178 +++++++++++++++++++++++
>  drivers/gpu/drm/mediatek/mtk_drm_fbdev.h |  25 ++++
>  7 files changed, 230 insertions(+)
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_fbdev.c
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_fbdev.h
> 
> diff --git a/drivers/gpu/drm/mediatek/Makefile b/drivers/gpu/drm/mediatek/Makefile
> index 82ae49c64221..d71e57dea77a 100644
> --- a/drivers/gpu/drm/mediatek/Makefile
> +++ b/drivers/gpu/drm/mediatek/Makefile
> @@ -14,6 +14,7 @@ mediatek-drm-y := mtk_disp_color.o \
>  		  mtk_mipi_tx.o \
>  		  mtk_dpi.o
>  
> +mediatek-drm-$(CONFIG_DRM_FBDEV_EMULATION) += mtk_drm_fbdev.o
>  obj-$(CONFIG_DRM_MEDIATEK) += mediatek-drm.o
>  
>  mediatek-drm-hdmi-objs := mtk_cec.o \
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 7800822b4db1..51f1ec3412d3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -32,6 +32,7 @@
>  #include "mtk_drm_ddp_comp.h"
>  #include "mtk_drm_drv.h"
>  #include "mtk_drm_fb.h"
> +#include "mtk_drm_fbdev.h"
>  #include "mtk_drm_gem.h"
>  
>  #define DRIVER_NAME "mediatek"
> @@ -299,6 +300,10 @@ static int mtk_drm_kms_init(struct drm_device *drm)
>  	drm_kms_helper_poll_init(drm);
>  	drm_mode_config_reset(drm);
>  
> +	ret = mtk_fbdev_init(drm);
> +	if (ret)
> +		goto err_component_unbind;
> +
>  	return 0;
>  
>  err_component_unbind:
> @@ -311,6 +316,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
>  
>  static void mtk_drm_kms_deinit(struct drm_device *drm)
>  {
> +	mtk_fbdev_fini(drm);
>  	drm_kms_helper_poll_fini(drm);
>  
>  	component_unbind_all(drm->dev, drm);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> index 256a3ff2e66e..56129a21fb2b 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> @@ -14,6 +14,7 @@
>  #ifndef MTK_DRM_DRV_H
>  #define MTK_DRM_DRV_H
>  
> +#include <drm/drm_fb_helper.h>
>  #include <linux/io.h>
>  #include "mtk_drm_ddp_comp.h"
>  
> @@ -43,6 +44,7 @@ struct mtk_drm_private {
>  	struct drm_device *drm;
>  	struct device *dma_dev;
>  
> +	struct drm_crtc *crtc[MAX_CRTC];
>  	unsigned int num_pipes;
>  
>  	struct device_node *mutex_node;
> @@ -59,6 +61,8 @@ struct mtk_drm_private {
>  	} commit;
>  
>  	struct drm_atomic_state *suspend_state;
> +	struct drm_fb_helper fb_helper;
> +	struct drm_gem_object *fbdev_bo;
>  };
>  
>  extern struct platform_driver mtk_ddp_driver;
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fb.c b/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> index be5f6f1daf55..7533aa4733d2 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> @@ -56,6 +56,19 @@ static struct drm_framebuffer *mtk_drm_framebuffer_init(struct drm_device *dev,
>  	return fb;
>  }
>  
> +struct drm_framebuffer *mtk_drm_framebuffer_create(struct drm_device *dev,
> +					const struct drm_mode_fb_cmd2 *mode,
> +					struct drm_gem_object *obj)
> +{
> +	struct drm_framebuffer *mtk_fb;
> +
> +	mtk_fb = mtk_drm_framebuffer_init(dev, mode, obj);
> +	if (IS_ERR(mtk_fb))
> +		return ERR_CAST(mtk_fb);
> +
> +	return mtk_fb;
> +}
> +
>  /*
>   * Wait for any exclusive fence in fb's gem object's reservation object.
>   *
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fb.h b/drivers/gpu/drm/mediatek/mtk_drm_fb.h
> index 7f976b196a15..e49e015380d0 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_fb.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_fb.h
> @@ -18,5 +18,8 @@ int mtk_fb_wait(struct drm_framebuffer *fb);
>  struct drm_framebuffer *mtk_drm_mode_fb_create(struct drm_device *dev,
>  					       struct drm_file *file,
>  					       const struct drm_mode_fb_cmd2 *cmd);
> +struct drm_framebuffer *mtk_drm_framebuffer_create(struct drm_device *dev,
> +		const struct drm_mode_fb_cmd2 *mode,
> +		struct drm_gem_object *obj);
>  
>  #endif /* MTK_DRM_FB_H */
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fbdev.c b/drivers/gpu/drm/mediatek/mtk_drm_fbdev.c
> new file mode 100644
> index 000000000000..9670f50d1247
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_fbdev.c
> @@ -0,0 +1,178 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
> + * Author: CK Hu <ck.hu@mediatek.com>
> + */
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_fb_helper.h>
> +#include <drm/drm_gem.h>
> +
> +#include "mtk_drm_drv.h"
> +#include "mtk_drm_fb.h"
> +#include "mtk_drm_gem.h"
> +#include "mtk_drm_fbdev.h"
> +
> +#define to_drm_private(x) \
> +		container_of(x, struct mtk_drm_private, fb_helper)
> +
> +static int mtk_drm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
> +{
> +	struct drm_fb_helper *helper = info->par;
> +	struct mtk_drm_private *private = to_drm_private(helper);
> +
> +	return mtk_drm_gem_mmap_buf(private->fbdev_bo, vma);
> +}
> +
> +static struct fb_ops mtk_fbdev_ops = {
> +	.owner = THIS_MODULE,
> +	DRM_FB_HELPER_DEFAULT_OPS,
> +	.fb_fillrect = drm_fb_helper_cfb_fillrect,
> +	.fb_copyarea = drm_fb_helper_cfb_copyarea,
> +	.fb_imageblit = drm_fb_helper_cfb_imageblit,
> +	.fb_check_var = drm_fb_helper_check_var,
> +	.fb_set_par = drm_fb_helper_set_par,
> +	.fb_blank = drm_fb_helper_blank,
> +	.fb_pan_display = drm_fb_helper_pan_display,
> +	.fb_setcmap = drm_fb_helper_setcmap,
> +	.fb_mmap = mtk_drm_fbdev_mmap,
> +};
> +
> +static int mtk_fbdev_probe(struct drm_fb_helper *helper,
> +			   struct drm_fb_helper_surface_size *sizes)
> +{
> +	struct drm_device *dev = helper->dev;
> +	struct mtk_drm_private *private = to_drm_private(helper);
> +	struct drm_mode_fb_cmd2 mode = { 0 };
> +	struct mtk_drm_gem_obj *mtk_gem;
> +	struct fb_info *info;
> +	struct drm_framebuffer *fb;
> +	unsigned int bytes_per_pixel;
> +	unsigned long offset;
> +	size_t size;
> +	int err = 0;
> +
> +	bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
> +
> +	mode.width = sizes->surface_width;
> +	mode.height = sizes->surface_height;
> +	mode.pitches[0] = sizes->surface_width * bytes_per_pixel;
> +	mode.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
> +						      sizes->surface_depth);
> +
> +	size = mode.pitches[0] * mode.height;
> +
> +	mtk_gem = mtk_drm_gem_create(dev, size, true);
> +	if (IS_ERR(mtk_gem))
> +		return PTR_ERR(mtk_gem);
> +
> +	private->fbdev_bo = &mtk_gem->base;
> +
> +	info = drm_fb_helper_alloc_fbi(helper);
> +	if (IS_ERR(info)) {
> +		DRM_DEV_ERROR(dev->dev,
> +			"failed to allocate framebuffer info, %d\n",
> +			err);
> +		err = PTR_ERR(info);
> +		goto out;
> +	}
> +
> +	fb = mtk_drm_framebuffer_create(dev, &mode, private->fbdev_bo);
> +	if (IS_ERR(fb)) {
> +		DRM_DEV_ERROR(dev->dev,
> +			"failed to allocate DRM framebuffer, %d\n",
> +			err);
> +		err = PTR_ERR(fb);
> +		goto out;
> +	}
> +	helper->fb = fb;
> +
> +	info->par = helper;
> +	info->flags = FBINFO_FLAG_DEFAULT;
> +	info->fbops = &mtk_fbdev_ops;
> +
> +	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format->depth);
> +	drm_fb_helper_fill_var(info, helper, sizes->fb_width, sizes->fb_height);
> +
> +	offset = info->var.xoffset * bytes_per_pixel;
> +	offset += info->var.yoffset * fb->pitches[0];
> +
> +	dev->mode_config.fb_base = 0;
> +	info->screen_base = mtk_gem->kvaddr + offset;
> +	info->screen_size = size;
> +	info->fix.smem_len = size;
> +
> +	DRM_DEBUG_KMS("FB [%ux%u]-%u offset=%lu size=%zd\n",
> +		      fb->width, fb->height, fb->format->depth, offset, size);
> +
> +	info->skip_vt_switch = true;
> +
> +	return 0;
> +
> +out:
> +
> +
> +	mtk_drm_gem_free_object(&mtk_gem->base);
> +	return err;
> +}
> +
> +static const struct drm_fb_helper_funcs mtk_drm_fb_helper_funcs = {
> +	.fb_probe = mtk_fbdev_probe,
> +};
> +
> +int mtk_fbdev_init(struct drm_device *dev)
> +{
> +	struct mtk_drm_private *priv = dev->dev_private;
> +	struct drm_fb_helper *helper = &priv->fb_helper;
> +	int ret;
> +
> +	if (!dev->mode_config.num_crtc || !dev->mode_config.num_connector)
> +		return -EINVAL;
> +
> +	drm_fb_helper_prepare(dev, helper, &mtk_drm_fb_helper_funcs);
> +
> +	ret = drm_fb_helper_init(dev, helper, dev->mode_config.num_connector);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(dev->dev,
> +			"failed to initialize DRM FB helper, %d\n",
> +			ret);
> +//		goto fini;
> +		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 fini;
> +	}
> +
> +	ret = drm_fb_helper_initial_config(helper, 32);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(dev->dev,
> +			"failed to set initial configuration, %d\n",
> +			ret);
> +		goto fini;
> +	}
> +
> +	return 0;
> +
> +fini:
> +	drm_fb_helper_fini(helper);
> +	return ret;
> +}
> +
> +void mtk_fbdev_fini(struct drm_device *dev)
> +{
> +	struct mtk_drm_private *priv = dev->dev_private;
> +	struct drm_fb_helper *helper = &priv->fb_helper;
> +
> +	drm_fb_helper_unregister_fbi(helper);
> +
> +	if (helper->fb) {
> +		drm_framebuffer_unregister_private(helper->fb);
> +		drm_framebuffer_remove(helper->fb);
> +	}
> +
> +	drm_fb_helper_fini(helper);
> +}
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fbdev.h b/drivers/gpu/drm/mediatek/mtk_drm_fbdev.h
> new file mode 100644
> index 000000000000..45717b642dc5
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_fbdev.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
> + * Author: CK Hu <ck.hu@mediatek.com>
> + */
> +
> +#ifndef MTK_DRM_FBDEV_H
> +#define MTK_DRM_FBDEV_H
> +
> +#ifdef CONFIG_DRM_FBDEV_EMULATION
> +int mtk_fbdev_init(struct drm_device *dev);
> +void mtk_fbdev_fini(struct drm_device *dev);
> +#else
> +int mtk_fbdev_init(struct drm_device *dev)
> +{
> +	return 0;
> +}
> +
> +void mtk_fbdev_fini(struct drm_device *dev)
> +{
> +
> +}
> +#endif /* CONFIG_DRM_FBDEV_EMULATION */
> +
> +#endif /* MTK_DRM_FBDEV_H */
> -- 
> 2.17.1
> 

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/mediatek: Add MTK Framebuffer-Device (mt7623)
  2019-01-07 10:36 ` [PATCH] drm/mediatek: Add MTK Framebuffer-Device (mt7623) Daniel Vetter
@ 2019-01-08 13:40   ` Daniel Vetter
  2019-01-10 19:01     ` Aw: " Frank Wunderlich
  0 siblings, 1 reply; 14+ messages in thread
From: Daniel Vetter @ 2019-01-08 13:40 UTC (permalink / raw)
  To: Frank Wunderlich
  Cc: David Airlie, Alexander Ryabchenko, Linux Kernel Mailing List,
	dri-devel, Matthias Brugger,
	moderated list:ARM/Mediatek SoC support, Philipp Zabel, CK Hu,
	Linux ARM

On Mon, Jan 7, 2019 at 11:36 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Wed, Jan 02, 2019 at 09:49:17AM +0100, Frank Wunderlich wrote:
> > From: CK Hu <ck.hu@mediatek.com>
> >
> > This patch adds Framebuffer-Driver for Mediatek
> >
> > currently tested on mt7623, maybe works on other platforms
> > MTK-FBDev written by CK Hu and ported from 4.4
> >
> > based on patchset drm/hdmi for 7623 v5 (except last part included in 4.20)
> >   https://patchwork.kernel.org/project/linux-mediatek/list/?series=25989
> >
> > depends on
> >   dts-patch (resend of 5/5, parts 1-4 already in 4.20):
> >     "arm: dts: mt7623: add display subsystem related device nodes"
> >     https://patchwork.kernel.org/patch/10588951/
> >   2 bugfix-patches from bibby hsieh
> >     fix-boot-up-for-720-and-480-but-1080
> >     using-different-round-rate-for-mt7623
> >   1 Patch from Ryder Lee
> >     http://forum.banana-pi.org/t/kernel-4-19-rc1-for-testers/6618/52
> >
> > full working tree here for reference:
> > https://github.com/frank-w/BPI-R2-4.14/commits/4.20-hdmiv5
> >
> > Signed-off-by: CK Hu <ck.hu@mediatek.com>
> > Signed-off-by: Alexander Ryabchenko <d3adme4t@gmail.com>
> > Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> > Tested-by: Frank Wunderlich <frank-w@public-files.de>
>
> Would be good to use the new generic fbdev emulation code here, for even
> less code. Or at least know why this isn't possible to use for mtk (and
> maybe address that in the core code). Hand-rolling fbdev code shouldn't be
> needed anymore.

Back on the mailing list, no private replies please:

For examples please grep for drm_fbdev_generic_setup(). There's also a
still in-flight series from Gerd Hoffmann to convert over bochs. That,
plus all the kerneldoc linked from there should get you started.
-Daniel


> -Daniel
> > ---
> >  drivers/gpu/drm/mediatek/Makefile        |   1 +
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.c   |   6 +
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.h   |   4 +
> >  drivers/gpu/drm/mediatek/mtk_drm_fb.c    |  13 ++
> >  drivers/gpu/drm/mediatek/mtk_drm_fb.h    |   3 +
> >  drivers/gpu/drm/mediatek/mtk_drm_fbdev.c | 178 +++++++++++++++++++++++
> >  drivers/gpu/drm/mediatek/mtk_drm_fbdev.h |  25 ++++
> >  7 files changed, 230 insertions(+)
> >  create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_fbdev.c
> >  create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_fbdev.h
> >
> > diff --git a/drivers/gpu/drm/mediatek/Makefile b/drivers/gpu/drm/mediatek/Makefile
> > index 82ae49c64221..d71e57dea77a 100644
> > --- a/drivers/gpu/drm/mediatek/Makefile
> > +++ b/drivers/gpu/drm/mediatek/Makefile
> > @@ -14,6 +14,7 @@ mediatek-drm-y := mtk_disp_color.o \
> >                 mtk_mipi_tx.o \
> >                 mtk_dpi.o
> >
> > +mediatek-drm-$(CONFIG_DRM_FBDEV_EMULATION) += mtk_drm_fbdev.o
> >  obj-$(CONFIG_DRM_MEDIATEK) += mediatek-drm.o
> >
> >  mediatek-drm-hdmi-objs := mtk_cec.o \
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > index 7800822b4db1..51f1ec3412d3 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > @@ -32,6 +32,7 @@
> >  #include "mtk_drm_ddp_comp.h"
> >  #include "mtk_drm_drv.h"
> >  #include "mtk_drm_fb.h"
> > +#include "mtk_drm_fbdev.h"
> >  #include "mtk_drm_gem.h"
> >
> >  #define DRIVER_NAME "mediatek"
> > @@ -299,6 +300,10 @@ static int mtk_drm_kms_init(struct drm_device *drm)
> >       drm_kms_helper_poll_init(drm);
> >       drm_mode_config_reset(drm);
> >
> > +     ret = mtk_fbdev_init(drm);
> > +     if (ret)
> > +             goto err_component_unbind;
> > +
> >       return 0;
> >
> >  err_component_unbind:
> > @@ -311,6 +316,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
> >
> >  static void mtk_drm_kms_deinit(struct drm_device *drm)
> >  {
> > +     mtk_fbdev_fini(drm);
> >       drm_kms_helper_poll_fini(drm);
> >
> >       component_unbind_all(drm->dev, drm);
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > index 256a3ff2e66e..56129a21fb2b 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > @@ -14,6 +14,7 @@
> >  #ifndef MTK_DRM_DRV_H
> >  #define MTK_DRM_DRV_H
> >
> > +#include <drm/drm_fb_helper.h>
> >  #include <linux/io.h>
> >  #include "mtk_drm_ddp_comp.h"
> >
> > @@ -43,6 +44,7 @@ struct mtk_drm_private {
> >       struct drm_device *drm;
> >       struct device *dma_dev;
> >
> > +     struct drm_crtc *crtc[MAX_CRTC];
> >       unsigned int num_pipes;
> >
> >       struct device_node *mutex_node;
> > @@ -59,6 +61,8 @@ struct mtk_drm_private {
> >       } commit;
> >
> >       struct drm_atomic_state *suspend_state;
> > +     struct drm_fb_helper fb_helper;
> > +     struct drm_gem_object *fbdev_bo;
> >  };
> >
> >  extern struct platform_driver mtk_ddp_driver;
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fb.c b/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> > index be5f6f1daf55..7533aa4733d2 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> > @@ -56,6 +56,19 @@ static struct drm_framebuffer *mtk_drm_framebuffer_init(struct drm_device *dev,
> >       return fb;
> >  }
> >
> > +struct drm_framebuffer *mtk_drm_framebuffer_create(struct drm_device *dev,
> > +                                     const struct drm_mode_fb_cmd2 *mode,
> > +                                     struct drm_gem_object *obj)
> > +{
> > +     struct drm_framebuffer *mtk_fb;
> > +
> > +     mtk_fb = mtk_drm_framebuffer_init(dev, mode, obj);
> > +     if (IS_ERR(mtk_fb))
> > +             return ERR_CAST(mtk_fb);
> > +
> > +     return mtk_fb;
> > +}
> > +
> >  /*
> >   * Wait for any exclusive fence in fb's gem object's reservation object.
> >   *
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fb.h b/drivers/gpu/drm/mediatek/mtk_drm_fb.h
> > index 7f976b196a15..e49e015380d0 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_fb.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_fb.h
> > @@ -18,5 +18,8 @@ int mtk_fb_wait(struct drm_framebuffer *fb);
> >  struct drm_framebuffer *mtk_drm_mode_fb_create(struct drm_device *dev,
> >                                              struct drm_file *file,
> >                                              const struct drm_mode_fb_cmd2 *cmd);
> > +struct drm_framebuffer *mtk_drm_framebuffer_create(struct drm_device *dev,
> > +             const struct drm_mode_fb_cmd2 *mode,
> > +             struct drm_gem_object *obj);
> >
> >  #endif /* MTK_DRM_FB_H */
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fbdev.c b/drivers/gpu/drm/mediatek/mtk_drm_fbdev.c
> > new file mode 100644
> > index 000000000000..9670f50d1247
> > --- /dev/null
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_fbdev.c
> > @@ -0,0 +1,178 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2018 MediaTek Inc.
> > + * Author: CK Hu <ck.hu@mediatek.com>
> > + */
> > +
> > +#include <drm/drmP.h>
> > +#include <drm/drm_crtc_helper.h>
> > +#include <drm/drm_fb_helper.h>
> > +#include <drm/drm_gem.h>
> > +
> > +#include "mtk_drm_drv.h"
> > +#include "mtk_drm_fb.h"
> > +#include "mtk_drm_gem.h"
> > +#include "mtk_drm_fbdev.h"
> > +
> > +#define to_drm_private(x) \
> > +             container_of(x, struct mtk_drm_private, fb_helper)
> > +
> > +static int mtk_drm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
> > +{
> > +     struct drm_fb_helper *helper = info->par;
> > +     struct mtk_drm_private *private = to_drm_private(helper);
> > +
> > +     return mtk_drm_gem_mmap_buf(private->fbdev_bo, vma);
> > +}
> > +
> > +static struct fb_ops mtk_fbdev_ops = {
> > +     .owner = THIS_MODULE,
> > +     DRM_FB_HELPER_DEFAULT_OPS,
> > +     .fb_fillrect = drm_fb_helper_cfb_fillrect,
> > +     .fb_copyarea = drm_fb_helper_cfb_copyarea,
> > +     .fb_imageblit = drm_fb_helper_cfb_imageblit,
> > +     .fb_check_var = drm_fb_helper_check_var,
> > +     .fb_set_par = drm_fb_helper_set_par,
> > +     .fb_blank = drm_fb_helper_blank,
> > +     .fb_pan_display = drm_fb_helper_pan_display,
> > +     .fb_setcmap = drm_fb_helper_setcmap,
> > +     .fb_mmap = mtk_drm_fbdev_mmap,
> > +};
> > +
> > +static int mtk_fbdev_probe(struct drm_fb_helper *helper,
> > +                        struct drm_fb_helper_surface_size *sizes)
> > +{
> > +     struct drm_device *dev = helper->dev;
> > +     struct mtk_drm_private *private = to_drm_private(helper);
> > +     struct drm_mode_fb_cmd2 mode = { 0 };
> > +     struct mtk_drm_gem_obj *mtk_gem;
> > +     struct fb_info *info;
> > +     struct drm_framebuffer *fb;
> > +     unsigned int bytes_per_pixel;
> > +     unsigned long offset;
> > +     size_t size;
> > +     int err = 0;
> > +
> > +     bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
> > +
> > +     mode.width = sizes->surface_width;
> > +     mode.height = sizes->surface_height;
> > +     mode.pitches[0] = sizes->surface_width * bytes_per_pixel;
> > +     mode.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
> > +                                                   sizes->surface_depth);
> > +
> > +     size = mode.pitches[0] * mode.height;
> > +
> > +     mtk_gem = mtk_drm_gem_create(dev, size, true);
> > +     if (IS_ERR(mtk_gem))
> > +             return PTR_ERR(mtk_gem);
> > +
> > +     private->fbdev_bo = &mtk_gem->base;
> > +
> > +     info = drm_fb_helper_alloc_fbi(helper);
> > +     if (IS_ERR(info)) {
> > +             DRM_DEV_ERROR(dev->dev,
> > +                     "failed to allocate framebuffer info, %d\n",
> > +                     err);
> > +             err = PTR_ERR(info);
> > +             goto out;
> > +     }
> > +
> > +     fb = mtk_drm_framebuffer_create(dev, &mode, private->fbdev_bo);
> > +     if (IS_ERR(fb)) {
> > +             DRM_DEV_ERROR(dev->dev,
> > +                     "failed to allocate DRM framebuffer, %d\n",
> > +                     err);
> > +             err = PTR_ERR(fb);
> > +             goto out;
> > +     }
> > +     helper->fb = fb;
> > +
> > +     info->par = helper;
> > +     info->flags = FBINFO_FLAG_DEFAULT;
> > +     info->fbops = &mtk_fbdev_ops;
> > +
> > +     drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format->depth);
> > +     drm_fb_helper_fill_var(info, helper, sizes->fb_width, sizes->fb_height);
> > +
> > +     offset = info->var.xoffset * bytes_per_pixel;
> > +     offset += info->var.yoffset * fb->pitches[0];
> > +
> > +     dev->mode_config.fb_base = 0;
> > +     info->screen_base = mtk_gem->kvaddr + offset;
> > +     info->screen_size = size;
> > +     info->fix.smem_len = size;
> > +
> > +     DRM_DEBUG_KMS("FB [%ux%u]-%u offset=%lu size=%zd\n",
> > +                   fb->width, fb->height, fb->format->depth, offset, size);
> > +
> > +     info->skip_vt_switch = true;
> > +
> > +     return 0;
> > +
> > +out:
> > +
> > +
> > +     mtk_drm_gem_free_object(&mtk_gem->base);
> > +     return err;
> > +}
> > +
> > +static const struct drm_fb_helper_funcs mtk_drm_fb_helper_funcs = {
> > +     .fb_probe = mtk_fbdev_probe,
> > +};
> > +
> > +int mtk_fbdev_init(struct drm_device *dev)
> > +{
> > +     struct mtk_drm_private *priv = dev->dev_private;
> > +     struct drm_fb_helper *helper = &priv->fb_helper;
> > +     int ret;
> > +
> > +     if (!dev->mode_config.num_crtc || !dev->mode_config.num_connector)
> > +             return -EINVAL;
> > +
> > +     drm_fb_helper_prepare(dev, helper, &mtk_drm_fb_helper_funcs);
> > +
> > +     ret = drm_fb_helper_init(dev, helper, dev->mode_config.num_connector);
> > +     if (ret < 0) {
> > +             DRM_DEV_ERROR(dev->dev,
> > +                     "failed to initialize DRM FB helper, %d\n",
> > +                     ret);
> > +//           goto fini;
> > +             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 fini;
> > +     }
> > +
> > +     ret = drm_fb_helper_initial_config(helper, 32);
> > +     if (ret < 0) {
> > +             DRM_DEV_ERROR(dev->dev,
> > +                     "failed to set initial configuration, %d\n",
> > +                     ret);
> > +             goto fini;
> > +     }
> > +
> > +     return 0;
> > +
> > +fini:
> > +     drm_fb_helper_fini(helper);
> > +     return ret;
> > +}
> > +
> > +void mtk_fbdev_fini(struct drm_device *dev)
> > +{
> > +     struct mtk_drm_private *priv = dev->dev_private;
> > +     struct drm_fb_helper *helper = &priv->fb_helper;
> > +
> > +     drm_fb_helper_unregister_fbi(helper);
> > +
> > +     if (helper->fb) {
> > +             drm_framebuffer_unregister_private(helper->fb);
> > +             drm_framebuffer_remove(helper->fb);
> > +     }
> > +
> > +     drm_fb_helper_fini(helper);
> > +}
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fbdev.h b/drivers/gpu/drm/mediatek/mtk_drm_fbdev.h
> > new file mode 100644
> > index 000000000000..45717b642dc5
> > --- /dev/null
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_fbdev.h
> > @@ -0,0 +1,25 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (c) 2018 MediaTek Inc.
> > + * Author: CK Hu <ck.hu@mediatek.com>
> > + */
> > +
> > +#ifndef MTK_DRM_FBDEV_H
> > +#define MTK_DRM_FBDEV_H
> > +
> > +#ifdef CONFIG_DRM_FBDEV_EMULATION
> > +int mtk_fbdev_init(struct drm_device *dev);
> > +void mtk_fbdev_fini(struct drm_device *dev);
> > +#else
> > +int mtk_fbdev_init(struct drm_device *dev)
> > +{
> > +     return 0;
> > +}
> > +
> > +void mtk_fbdev_fini(struct drm_device *dev)
> > +{
> > +
> > +}
> > +#endif /* CONFIG_DRM_FBDEV_EMULATION */
> > +
> > +#endif /* MTK_DRM_FBDEV_H */
> > --
> > 2.17.1
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Aw: Re: [PATCH] drm/mediatek: Add MTK Framebuffer-Device (mt7623)
  2019-01-08 13:40   ` Daniel Vetter
@ 2019-01-10 19:01     ` Frank Wunderlich
  2019-01-10 20:02       ` Daniel Vetter
  2019-01-11  1:31       ` Aw: " CK Hu
  0 siblings, 2 replies; 14+ messages in thread
From: Frank Wunderlich @ 2019-01-10 19:01 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: David Airlie, Alexander Ryabchenko, Linux Kernel Mailing List,
	dri-devel, Matthias Brugger,
	moderated list:ARM/Mediatek SoC support, Philipp Zabel, CK Hu,
	Linux ARM

Hi Daniel,

> > Would be good to use the new generic fbdev emulation code here, for even
> > less code. Or at least know why this isn't possible to use for mtk (and
> > maybe address that in the core code). Hand-rolling fbdev code shouldn't be
> > needed anymore.
> 
> Back on the mailing list, no private replies please:

i don't wanted to spam all people with dumb questions ;)

> For examples please grep for drm_fbdev_generic_setup(). There's also a
> still in-flight series from Gerd Hoffmann to convert over bochs. That,
> plus all the kerneldoc linked from there should get you started.
> -Daniel

this is one of google best founds if i search for drm_fbdev_generic_setup:

https://lkml.org/lkml/2018/12/19/305

not very helpful...

so i tried kernel-doc

https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html?highlight=drm_fbdev_generic_setup#c.drm_fbdev_generic_setup

which is nice function-reference but i've found no generic workflow

as the posted driver is "only" a driver ported from kernel 4.4 by Alexander, i don't know if this new framework can be used and which parts need to be changed. I only try to bring his code Mainline....
Maybe CK Hu can help here because driver is originally from him and he knows internals. Or maybe you can help here?

i personally make my first steps as spare-time kernel-developer :)

regards Frank

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Re: [PATCH] drm/mediatek: Add MTK Framebuffer-Device (mt7623)
  2019-01-10 19:01     ` Aw: " Frank Wunderlich
@ 2019-01-10 20:02       ` Daniel Vetter
  2019-01-11  3:20         ` CK Hu
  2019-01-11  1:31       ` Aw: " CK Hu
  1 sibling, 1 reply; 14+ messages in thread
From: Daniel Vetter @ 2019-01-10 20:02 UTC (permalink / raw)
  To: Frank Wunderlich
  Cc: Daniel Vetter, David Airlie, Alexander Ryabchenko,
	Linux Kernel Mailing List, dri-devel, Matthias Brugger,
	moderated list:ARM/Mediatek SoC support, Philipp Zabel, CK Hu,
	Linux ARM

On Thu, Jan 10, 2019 at 08:01:37PM +0100, Frank Wunderlich wrote:
> Hi Daniel,
> 
> > > Would be good to use the new generic fbdev emulation code here, for even
> > > less code. Or at least know why this isn't possible to use for mtk (and
> > > maybe address that in the core code). Hand-rolling fbdev code shouldn't be
> > > needed anymore.
> > 
> > Back on the mailing list, no private replies please:
> 
> i don't wanted to spam all people with dumb questions ;)

There's no dumb questions, only insufficient documentation :-)

> > For examples please grep for drm_fbdev_generic_setup(). There's also a
> > still in-flight series from Gerd Hoffmann to convert over bochs. That,
> > plus all the kerneldoc linked from there should get you started.
> > -Daniel
> 
> this is one of google best founds if i search for drm_fbdev_generic_setup:
> 
> https://lkml.org/lkml/2018/12/19/305
> 
> not very helpful...
> 
> so i tried kernel-doc
> 
> https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html?highlight=drm_fbdev_generic_setup#c.drm_fbdev_generic_setup
> 
> which is nice function-reference but i've found no generic workflow
> 
> as the posted driver is "only" a driver ported from kernel 4.4 by Alexander, i don't know if this new framework can be used and which parts need to be changed. I only try to bring his code Mainline....
> Maybe CK Hu can help here because driver is originally from him and he knows internals. Or maybe you can help here?
> 
> i personally make my first steps as spare-time kernel-developer :)

There's a ton of in-kernel users of that function already, I meant you can
use those to serve as examples ... If those + the kerneldoc aren't
good enough, then we need to improve them.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Aw: Re: [PATCH] drm/mediatek: Add MTK Framebuffer-Device (mt7623)
  2019-01-10 19:01     ` Aw: " Frank Wunderlich
  2019-01-10 20:02       ` Daniel Vetter
@ 2019-01-11  1:31       ` CK Hu
  1 sibling, 0 replies; 14+ messages in thread
From: CK Hu @ 2019-01-11  1:31 UTC (permalink / raw)
  To: Frank Wunderlich
  Cc: Philipp Zabel, David Airlie, Alexander Ryabchenko,
	Linux Kernel Mailing List, dri-devel,
	moderated list:ARM/Mediatek SoC support, Daniel Vetter,
	Matthias Brugger, Linux ARM

Hi, Frank:

On Thu, 2019-01-10 at 20:01 +0100, Frank Wunderlich wrote:
> Hi Daniel,
> 
> > > Would be good to use the new generic fbdev emulation code here, for even
> > > less code. Or at least know why this isn't possible to use for mtk (and
> > > maybe address that in the core code). Hand-rolling fbdev code shouldn't be
> > > needed anymore.
> > 
> > Back on the mailing list, no private replies please:
> 
> i don't wanted to spam all people with dumb questions ;)
> 
> > For examples please grep for drm_fbdev_generic_setup(). There's also a
> > still in-flight series from Gerd Hoffmann to convert over bochs. That,
> > plus all the kerneldoc linked from there should get you started.
> > -Daniel
> 
> this is one of google best founds if i search for drm_fbdev_generic_setup:
> 
> https://lkml.org/lkml/2018/12/19/305
> 
> not very helpful...
> 
> so i tried kernel-doc
> 
> https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html?highlight=drm_fbdev_generic_setup#c.drm_fbdev_generic_setup
> 
> which is nice function-reference but i've found no generic workflow
> 
> as the posted driver is "only" a driver ported from kernel 4.4 by Alexander, i don't know if this new framework can be used and which parts need to be changed. I only try to bring his code Mainline....
> Maybe CK Hu can help here because driver is originally from him and he knows internals. Or maybe you can help here?

I could help on this but I'm a little busy now, so I'm not sure how long
this process takes.

Regards,
CK

> 
> i personally make my first steps as spare-time kernel-developer :)
> 
> regards Frank
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Re: [PATCH] drm/mediatek: Add MTK Framebuffer-Device (mt7623)
  2019-01-10 20:02       ` Daniel Vetter
@ 2019-01-11  3:20         ` CK Hu
  2019-01-11  9:20           ` Daniel Vetter
  0 siblings, 1 reply; 14+ messages in thread
From: CK Hu @ 2019-01-11  3:20 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Frank Wunderlich, David Airlie, Alexander Ryabchenko,
	Linux Kernel Mailing List, dri-devel,
	moderated list:ARM/Mediatek SoC support, Philipp Zabel,
	Matthias Brugger, Linux ARM

Hi, Daniel:

On Thu, 2019-01-10 at 21:02 +0100, Daniel Vetter wrote:
> On Thu, Jan 10, 2019 at 08:01:37PM +0100, Frank Wunderlich wrote:
> > Hi Daniel,
> > 
> > > > Would be good to use the new generic fbdev emulation code here, for even
> > > > less code. Or at least know why this isn't possible to use for mtk (and
> > > > maybe address that in the core code). Hand-rolling fbdev code shouldn't be
> > > > needed anymore.
> > > 
> > > Back on the mailing list, no private replies please:
> > 
> > i don't wanted to spam all people with dumb questions ;)
> 
> There's no dumb questions, only insufficient documentation :-)
> 
> > > For examples please grep for drm_fbdev_generic_setup(). There's also a
> > > still in-flight series from Gerd Hoffmann to convert over bochs. That,
> > > plus all the kerneldoc linked from there should get you started.
> > > -Daniel
> > 
> > this is one of google best founds if i search for drm_fbdev_generic_setup:
> > 
> > https://lkml.org/lkml/2018/12/19/305
> > 
> > not very helpful...
> > 
> > so i tried kernel-doc
> > 
> > https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html?highlight=drm_fbdev_generic_setup#c.drm_fbdev_generic_setup
> > 
> > which is nice function-reference but i've found no generic workflow
> > 
> > as the posted driver is "only" a driver ported from kernel 4.4 by Alexander, i don't know if this new framework can be used and which parts need to be changed. I only try to bring his code Mainline....
> > Maybe CK Hu can help here because driver is originally from him and he knows internals. Or maybe you can help here?
> > 
> > i personally make my first steps as spare-time kernel-developer :)
> 
> There's a ton of in-kernel users of that function already, I meant you can
> use those to serve as examples ... If those + the kerneldoc aren't
> good enough, then we need to improve them.
> -Daniel

I've tried with drm_fbdev_generic_setup() and it works fine with simple
modification. The patch is

--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -16,6 +16,7 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_gem.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_of.h>
@@ -379,6 +380,7 @@ static void mtk_drm_kms_deinit(struct drm_device
*drm)
        .gem_prime_get_sg_table = mtk_gem_prime_get_sg_table,
        .gem_prime_import_sg_table = mtk_gem_prime_import_sg_table,
        .gem_prime_mmap = mtk_drm_gem_mmap_buf,
+       .gem_prime_vmap = mtk_drm_gem_prime_vmap,
        .ioctls = mtk_ioctls,
        .num_ioctls = ARRAY_SIZE(mtk_ioctls),
        .fops = &mtk_drm_fops,
@@ -416,6 +418,8 @@ static int mtk_drm_bind(struct device *dev)
        if (ret < 0)
                goto err_deinit;

+       drm_fbdev_generic_setup(drm, 32);
+
        return 0;


But I implement .gem_prime_vmap with a workaround,


--- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
@@ -280,3 +280,8 @@ int mtk_gem_create_ioctl(struct drm_device *dev,
void *data,
        mtk_drm_gem_free_object(&mtk_gem->base);
        return ret;
 }
+
+void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj)
+{
+       return (void *)1;
+}

Current drm_fb_helper depends on drm_client to create fbdev. When
drm_client create drm_client_buffer, it need to vmap to get kernel
vaddr. But I think for fbdev, the vaddr is useless. Do you agree that I
temporarily implement .gem_prime_vmap in such way?

Regards,
CK



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Re: [PATCH] drm/mediatek: Add MTK Framebuffer-Device (mt7623)
  2019-01-11  3:20         ` CK Hu
@ 2019-01-11  9:20           ` Daniel Vetter
  2019-01-11  9:49             ` CK Hu
  0 siblings, 1 reply; 14+ messages in thread
From: Daniel Vetter @ 2019-01-11  9:20 UTC (permalink / raw)
  To: CK Hu, Noralf Trønnes
  Cc: Philipp Zabel, Frank Wunderlich, David Airlie,
	Alexander Ryabchenko, Linux Kernel Mailing List, dri-devel,
	moderated list:ARM/Mediatek SoC support, Daniel Vetter,
	Matthias Brugger, Linux ARM

On Fri, Jan 11, 2019 at 11:20:09AM +0800, CK Hu wrote:
> Hi, Daniel:
> 
> On Thu, 2019-01-10 at 21:02 +0100, Daniel Vetter wrote:
> > On Thu, Jan 10, 2019 at 08:01:37PM +0100, Frank Wunderlich wrote:
> > > Hi Daniel,
> > > 
> > > > > Would be good to use the new generic fbdev emulation code here, for even
> > > > > less code. Or at least know why this isn't possible to use for mtk (and
> > > > > maybe address that in the core code). Hand-rolling fbdev code shouldn't be
> > > > > needed anymore.
> > > > 
> > > > Back on the mailing list, no private replies please:
> > > 
> > > i don't wanted to spam all people with dumb questions ;)
> > 
> > There's no dumb questions, only insufficient documentation :-)
> > 
> > > > For examples please grep for drm_fbdev_generic_setup(). There's also a
> > > > still in-flight series from Gerd Hoffmann to convert over bochs. That,
> > > > plus all the kerneldoc linked from there should get you started.
> > > > -Daniel
> > > 
> > > this is one of google best founds if i search for drm_fbdev_generic_setup:
> > > 
> > > https://lkml.org/lkml/2018/12/19/305
> > > 
> > > not very helpful...
> > > 
> > > so i tried kernel-doc
> > > 
> > > https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html?highlight=drm_fbdev_generic_setup#c.drm_fbdev_generic_setup
> > > 
> > > which is nice function-reference but i've found no generic workflow
> > > 
> > > as the posted driver is "only" a driver ported from kernel 4.4 by Alexander, i don't know if this new framework can be used and which parts need to be changed. I only try to bring his code Mainline....
> > > Maybe CK Hu can help here because driver is originally from him and he knows internals. Or maybe you can help here?
> > > 
> > > i personally make my first steps as spare-time kernel-developer :)
> > 
> > There's a ton of in-kernel users of that function already, I meant you can
> > use those to serve as examples ... If those + the kerneldoc aren't
> > good enough, then we need to improve them.
> > -Daniel
> 
> I've tried with drm_fbdev_generic_setup() and it works fine with simple
> modification. The patch is
> 
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -16,6 +16,7 @@
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_crtc_helper.h>
> +#include <drm/drm_fb_helper.h>
>  #include <drm/drm_gem.h>
>  #include <drm/drm_gem_cma_helper.h>
>  #include <drm/drm_of.h>
> @@ -379,6 +380,7 @@ static void mtk_drm_kms_deinit(struct drm_device
> *drm)
>         .gem_prime_get_sg_table = mtk_gem_prime_get_sg_table,
>         .gem_prime_import_sg_table = mtk_gem_prime_import_sg_table,
>         .gem_prime_mmap = mtk_drm_gem_mmap_buf,
> +       .gem_prime_vmap = mtk_drm_gem_prime_vmap,
>         .ioctls = mtk_ioctls,
>         .num_ioctls = ARRAY_SIZE(mtk_ioctls),
>         .fops = &mtk_drm_fops,
> @@ -416,6 +418,8 @@ static int mtk_drm_bind(struct device *dev)
>         if (ret < 0)
>                 goto err_deinit;
> 
> +       drm_fbdev_generic_setup(drm, 32);
> +
>         return 0;
> 
> 
> But I implement .gem_prime_vmap with a workaround,
> 
> 
> --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> @@ -280,3 +280,8 @@ int mtk_gem_create_ioctl(struct drm_device *dev,
> void *data,
>         mtk_drm_gem_free_object(&mtk_gem->base);
>         return ret;
>  }
> +
> +void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj)
> +{
> +       return (void *)1;
> +}
> 
> Current drm_fb_helper depends on drm_client to create fbdev. When
> drm_client create drm_client_buffer, it need to vmap to get kernel
> vaddr. But I think for fbdev, the vaddr is useless. Do you agree that I
> temporarily implement .gem_prime_vmap in such way?

The vmap is used by fbcon, without that fbdev won't really work I think.
So I'm rather confused on what you tested ...

Adding Noralf, he's the expert here.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Re: [PATCH] drm/mediatek: Add MTK Framebuffer-Device (mt7623)
  2019-01-11  9:20           ` Daniel Vetter
@ 2019-01-11  9:49             ` CK Hu
  2019-01-11 10:25               ` Daniel Vetter
  2019-01-11 18:23               ` Aw: Re: " Frank Wunderlich
  0 siblings, 2 replies; 14+ messages in thread
From: CK Hu @ 2019-01-11  9:49 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Frank Wunderlich, David Airlie, Alexander Ryabchenko,
	Linux Kernel Mailing List, dri-devel, Noralf Trønnes,
	moderated list:ARM/Mediatek SoC support, Philipp Zabel,
	Matthias Brugger, Linux ARM

Hi, Daniel:

On Fri, 2019-01-11 at 10:20 +0100, Daniel Vetter wrote:
> On Fri, Jan 11, 2019 at 11:20:09AM +0800, CK Hu wrote:
> > Hi, Daniel:
> > 
> > On Thu, 2019-01-10 at 21:02 +0100, Daniel Vetter wrote:
> > > On Thu, Jan 10, 2019 at 08:01:37PM +0100, Frank Wunderlich wrote:
> > > > Hi Daniel,
> > > > 
> > > > > > Would be good to use the new generic fbdev emulation code here, for even
> > > > > > less code. Or at least know why this isn't possible to use for mtk (and
> > > > > > maybe address that in the core code). Hand-rolling fbdev code shouldn't be
> > > > > > needed anymore.
> > > > > 
> > > > > Back on the mailing list, no private replies please:
> > > > 
> > > > i don't wanted to spam all people with dumb questions ;)
> > > 
> > > There's no dumb questions, only insufficient documentation :-)
> > > 
> > > > > For examples please grep for drm_fbdev_generic_setup(). There's also a
> > > > > still in-flight series from Gerd Hoffmann to convert over bochs. That,
> > > > > plus all the kerneldoc linked from there should get you started.
> > > > > -Daniel
> > > > 
> > > > this is one of google best founds if i search for drm_fbdev_generic_setup:
> > > > 
> > > > https://lkml.org/lkml/2018/12/19/305
> > > > 
> > > > not very helpful...
> > > > 
> > > > so i tried kernel-doc
> > > > 
> > > > https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html?highlight=drm_fbdev_generic_setup#c.drm_fbdev_generic_setup
> > > > 
> > > > which is nice function-reference but i've found no generic workflow
> > > > 
> > > > as the posted driver is "only" a driver ported from kernel 4.4 by Alexander, i don't know if this new framework can be used and which parts need to be changed. I only try to bring his code Mainline....
> > > > Maybe CK Hu can help here because driver is originally from him and he knows internals. Or maybe you can help here?
> > > > 
> > > > i personally make my first steps as spare-time kernel-developer :)
> > > 
> > > There's a ton of in-kernel users of that function already, I meant you can
> > > use those to serve as examples ... If those + the kerneldoc aren't
> > > good enough, then we need to improve them.
> > > -Daniel
> > 
> > I've tried with drm_fbdev_generic_setup() and it works fine with simple
> > modification. The patch is
> > 
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > @@ -16,6 +16,7 @@
> >  #include <drm/drm_atomic.h>
> >  #include <drm/drm_atomic_helper.h>
> >  #include <drm/drm_crtc_helper.h>
> > +#include <drm/drm_fb_helper.h>
> >  #include <drm/drm_gem.h>
> >  #include <drm/drm_gem_cma_helper.h>
> >  #include <drm/drm_of.h>
> > @@ -379,6 +380,7 @@ static void mtk_drm_kms_deinit(struct drm_device
> > *drm)
> >         .gem_prime_get_sg_table = mtk_gem_prime_get_sg_table,
> >         .gem_prime_import_sg_table = mtk_gem_prime_import_sg_table,
> >         .gem_prime_mmap = mtk_drm_gem_mmap_buf,
> > +       .gem_prime_vmap = mtk_drm_gem_prime_vmap,
> >         .ioctls = mtk_ioctls,
> >         .num_ioctls = ARRAY_SIZE(mtk_ioctls),
> >         .fops = &mtk_drm_fops,
> > @@ -416,6 +418,8 @@ static int mtk_drm_bind(struct device *dev)
> >         if (ret < 0)
> >                 goto err_deinit;
> > 
> > +       drm_fbdev_generic_setup(drm, 32);
> > +
> >         return 0;
> > 
> > 
> > But I implement .gem_prime_vmap with a workaround,
> > 
> > 
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> > @@ -280,3 +280,8 @@ int mtk_gem_create_ioctl(struct drm_device *dev,
> > void *data,
> >         mtk_drm_gem_free_object(&mtk_gem->base);
> >         return ret;
> >  }
> > +
> > +void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj)
> > +{
> > +       return (void *)1;
> > +}
> > 
> > Current drm_fb_helper depends on drm_client to create fbdev. When
> > drm_client create drm_client_buffer, it need to vmap to get kernel
> > vaddr. But I think for fbdev, the vaddr is useless. Do you agree that I
> > temporarily implement .gem_prime_vmap in such way?
> 
> The vmap is used by fbcon, without that fbdev won't really work I think.
> So I'm rather confused on what you tested ...
> 
> Adding Noralf, he's the expert here.
> -Daniel

I test with fb_test [1], it is a user space program just open /dev/fb0
and mmap it to user space. I does not turn on fbcon so everything looks
well. If support fbcon is essential, I would implement vmap correctly.
If it's not essential, I think I could return 0 when
CONFIG_FRAMBUFFER_CONSOLE is defined.

Regards,
CK

[1]
https://android.googlesource.com/platform/system/extras/+/master/tests/framebuffer/fb_test.c



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Re: [PATCH] drm/mediatek: Add MTK Framebuffer-Device (mt7623)
  2019-01-11  9:49             ` CK Hu
@ 2019-01-11 10:25               ` Daniel Vetter
  2019-01-11 16:07                 ` Noralf Trønnes
  2019-01-11 18:23               ` Aw: Re: " Frank Wunderlich
  1 sibling, 1 reply; 14+ messages in thread
From: Daniel Vetter @ 2019-01-11 10:25 UTC (permalink / raw)
  To: CK Hu
  Cc: Philipp Zabel, Frank Wunderlich, David Airlie,
	Alexander Ryabchenko, Linux Kernel Mailing List, dri-devel,
	Noralf Trønnes, moderated list:ARM/Mediatek SoC support,
	Daniel Vetter, Matthias Brugger, Linux ARM

On Fri, Jan 11, 2019 at 05:49:15PM +0800, CK Hu wrote:
> Hi, Daniel:
> 
> On Fri, 2019-01-11 at 10:20 +0100, Daniel Vetter wrote:
> > On Fri, Jan 11, 2019 at 11:20:09AM +0800, CK Hu wrote:
> > > Hi, Daniel:
> > > 
> > > On Thu, 2019-01-10 at 21:02 +0100, Daniel Vetter wrote:
> > > > On Thu, Jan 10, 2019 at 08:01:37PM +0100, Frank Wunderlich wrote:
> > > > > Hi Daniel,
> > > > > 
> > > > > > > Would be good to use the new generic fbdev emulation code here, for even
> > > > > > > less code. Or at least know why this isn't possible to use for mtk (and
> > > > > > > maybe address that in the core code). Hand-rolling fbdev code shouldn't be
> > > > > > > needed anymore.
> > > > > > 
> > > > > > Back on the mailing list, no private replies please:
> > > > > 
> > > > > i don't wanted to spam all people with dumb questions ;)
> > > > 
> > > > There's no dumb questions, only insufficient documentation :-)
> > > > 
> > > > > > For examples please grep for drm_fbdev_generic_setup(). There's also a
> > > > > > still in-flight series from Gerd Hoffmann to convert over bochs. That,
> > > > > > plus all the kerneldoc linked from there should get you started.
> > > > > > -Daniel
> > > > > 
> > > > > this is one of google best founds if i search for drm_fbdev_generic_setup:
> > > > > 
> > > > > https://lkml.org/lkml/2018/12/19/305
> > > > > 
> > > > > not very helpful...
> > > > > 
> > > > > so i tried kernel-doc
> > > > > 
> > > > > https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html?highlight=drm_fbdev_generic_setup#c.drm_fbdev_generic_setup
> > > > > 
> > > > > which is nice function-reference but i've found no generic workflow
> > > > > 
> > > > > as the posted driver is "only" a driver ported from kernel 4.4 by Alexander, i don't know if this new framework can be used and which parts need to be changed. I only try to bring his code Mainline....
> > > > > Maybe CK Hu can help here because driver is originally from him and he knows internals. Or maybe you can help here?
> > > > > 
> > > > > i personally make my first steps as spare-time kernel-developer :)
> > > > 
> > > > There's a ton of in-kernel users of that function already, I meant you can
> > > > use those to serve as examples ... If those + the kerneldoc aren't
> > > > good enough, then we need to improve them.
> > > > -Daniel
> > > 
> > > I've tried with drm_fbdev_generic_setup() and it works fine with simple
> > > modification. The patch is
> > > 
> > > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > > @@ -16,6 +16,7 @@
> > >  #include <drm/drm_atomic.h>
> > >  #include <drm/drm_atomic_helper.h>
> > >  #include <drm/drm_crtc_helper.h>
> > > +#include <drm/drm_fb_helper.h>
> > >  #include <drm/drm_gem.h>
> > >  #include <drm/drm_gem_cma_helper.h>
> > >  #include <drm/drm_of.h>
> > > @@ -379,6 +380,7 @@ static void mtk_drm_kms_deinit(struct drm_device
> > > *drm)
> > >         .gem_prime_get_sg_table = mtk_gem_prime_get_sg_table,
> > >         .gem_prime_import_sg_table = mtk_gem_prime_import_sg_table,
> > >         .gem_prime_mmap = mtk_drm_gem_mmap_buf,
> > > +       .gem_prime_vmap = mtk_drm_gem_prime_vmap,
> > >         .ioctls = mtk_ioctls,
> > >         .num_ioctls = ARRAY_SIZE(mtk_ioctls),
> > >         .fops = &mtk_drm_fops,
> > > @@ -416,6 +418,8 @@ static int mtk_drm_bind(struct device *dev)
> > >         if (ret < 0)
> > >                 goto err_deinit;
> > > 
> > > +       drm_fbdev_generic_setup(drm, 32);
> > > +
> > >         return 0;
> > > 
> > > 
> > > But I implement .gem_prime_vmap with a workaround,
> > > 
> > > 
> > > --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> > > @@ -280,3 +280,8 @@ int mtk_gem_create_ioctl(struct drm_device *dev,
> > > void *data,
> > >         mtk_drm_gem_free_object(&mtk_gem->base);
> > >         return ret;
> > >  }
> > > +
> > > +void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj)
> > > +{
> > > +       return (void *)1;
> > > +}
> > > 
> > > Current drm_fb_helper depends on drm_client to create fbdev. When
> > > drm_client create drm_client_buffer, it need to vmap to get kernel
> > > vaddr. But I think for fbdev, the vaddr is useless. Do you agree that I
> > > temporarily implement .gem_prime_vmap in such way?
> > 
> > The vmap is used by fbcon, without that fbdev won't really work I think.
> > So I'm rather confused on what you tested ...
> > 
> > Adding Noralf, he's the expert here.
> > -Daniel
> 
> I test with fb_test [1], it is a user space program just open /dev/fb0
> and mmap it to user space. I does not turn on fbcon so everything looks
> well. If support fbcon is essential, I would implement vmap correctly.
> If it's not essential, I think I could return 0 when
> CONFIG_FRAMBUFFER_CONSOLE is defined.

I think fbdev emulation without working fbcon is fairly useless. And it
shouldn't be that much work to make it work, we have quite a few more
helers for gem bo nowadays.
-Daniel

> 
> Regards,
> CK
> 
> [1]
> https://android.googlesource.com/platform/system/extras/+/master/tests/framebuffer/fb_test.c
> 
> 

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/mediatek: Add MTK Framebuffer-Device (mt7623)
  2019-01-11 10:25               ` Daniel Vetter
@ 2019-01-11 16:07                 ` Noralf Trønnes
  2019-01-14  2:02                   ` CK Hu
  0 siblings, 1 reply; 14+ messages in thread
From: Noralf Trønnes @ 2019-01-11 16:07 UTC (permalink / raw)
  To: CK Hu, Frank Wunderlich, David Airlie, Alexander Ryabchenko,
	Linux Kernel Mailing List, dri-devel, Matthias Brugger,
	moderated list:ARM/Mediatek SoC support, Philipp Zabel,
	Linux ARM



Den 11.01.2019 11.25, skrev Daniel Vetter:
> On Fri, Jan 11, 2019 at 05:49:15PM +0800, CK Hu wrote:
>> Hi, Daniel:
>>
>> On Fri, 2019-01-11 at 10:20 +0100, Daniel Vetter wrote:
>>> On Fri, Jan 11, 2019 at 11:20:09AM +0800, CK Hu wrote:
>>>> Hi, Daniel:
>>>>
>>>> On Thu, 2019-01-10 at 21:02 +0100, Daniel Vetter wrote:
>>>>> On Thu, Jan 10, 2019 at 08:01:37PM +0100, Frank Wunderlich wrote:
>>>>>> Hi Daniel,
>>>>>>
>>>>>>>> Would be good to use the new generic fbdev emulation code here, for even
>>>>>>>> less code. Or at least know why this isn't possible to use for mtk (and
>>>>>>>> maybe address that in the core code). Hand-rolling fbdev code shouldn't be
>>>>>>>> needed anymore.
>>>>>>>
>>>>>>> Back on the mailing list, no private replies please:
>>>>>>
>>>>>> i don't wanted to spam all people with dumb questions ;)
>>>>>
>>>>> There's no dumb questions, only insufficient documentation :-)
>>>>>
>>>>>>> For examples please grep for drm_fbdev_generic_setup(). There's also a
>>>>>>> still in-flight series from Gerd Hoffmann to convert over bochs. That,
>>>>>>> plus all the kerneldoc linked from there should get you started.
>>>>>>> -Daniel
>>>>>>
>>>>>> this is one of google best founds if i search for drm_fbdev_generic_setup:
>>>>>>
>>>>>> https://lkml.org/lkml/2018/12/19/305
>>>>>>
>>>>>> not very helpful...
>>>>>>
>>>>>> so i tried kernel-doc
>>>>>>
>>>>>> https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html?highlight=drm_fbdev_generic_setup#c.drm_fbdev_generic_setup
>>>>>>
>>>>>> which is nice function-reference but i've found no generic workflow
>>>>>>
>>>>>> as the posted driver is "only" a driver ported from kernel 4.4 by Alexander, i don't know if this new framework can be used and which parts need to be changed. I only try to bring his code Mainline....
>>>>>> Maybe CK Hu can help here because driver is originally from him and he knows internals. Or maybe you can help here?
>>>>>>
>>>>>> i personally make my first steps as spare-time kernel-developer :)
>>>>>
>>>>> There's a ton of in-kernel users of that function already, I meant you can
>>>>> use those to serve as examples ... If those + the kerneldoc aren't
>>>>> good enough, then we need to improve them.
>>>>> -Daniel
>>>>
>>>> I've tried with drm_fbdev_generic_setup() and it works fine with simple
>>>> modification. The patch is
>>>>
>>>> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
>>>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
>>>> @@ -16,6 +16,7 @@
>>>>  #include <drm/drm_atomic.h>
>>>>  #include <drm/drm_atomic_helper.h>
>>>>  #include <drm/drm_crtc_helper.h>
>>>> +#include <drm/drm_fb_helper.h>
>>>>  #include <drm/drm_gem.h>
>>>>  #include <drm/drm_gem_cma_helper.h>
>>>>  #include <drm/drm_of.h>
>>>> @@ -379,6 +380,7 @@ static void mtk_drm_kms_deinit(struct drm_device
>>>> *drm)
>>>>         .gem_prime_get_sg_table = mtk_gem_prime_get_sg_table,
>>>>         .gem_prime_import_sg_table = mtk_gem_prime_import_sg_table,
>>>>         .gem_prime_mmap = mtk_drm_gem_mmap_buf,
>>>> +       .gem_prime_vmap = mtk_drm_gem_prime_vmap,
>>>>         .ioctls = mtk_ioctls,
>>>>         .num_ioctls = ARRAY_SIZE(mtk_ioctls),
>>>>         .fops = &mtk_drm_fops,
>>>> @@ -416,6 +418,8 @@ static int mtk_drm_bind(struct device *dev)
>>>>         if (ret < 0)
>>>>                 goto err_deinit;
>>>>
>>>> +       drm_fbdev_generic_setup(drm, 32);
>>>> +
>>>>         return 0;
>>>>
>>>>
>>>> But I implement .gem_prime_vmap with a workaround,
>>>>
>>>>
>>>> --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
>>>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
>>>> @@ -280,3 +280,8 @@ int mtk_gem_create_ioctl(struct drm_device *dev,
>>>> void *data,
>>>>         mtk_drm_gem_free_object(&mtk_gem->base);
>>>>         return ret;
>>>>  }
>>>> +
>>>> +void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj)
>>>> +{
>>>> +       return (void *)1;
>>>> +}
>>>>
>>>> Current drm_fb_helper depends on drm_client to create fbdev. When
>>>> drm_client create drm_client_buffer, it need to vmap to get kernel
>>>> vaddr. But I think for fbdev, the vaddr is useless. Do you agree that I
>>>> temporarily implement .gem_prime_vmap in such way?
>>>
>>> The vmap is used by fbcon, without that fbdev won't really work I think.
>>> So I'm rather confused on what you tested ...
>>>
>>> Adding Noralf, he's the expert here.
>>> -Daniel
>>
>> I test with fb_test [1], it is a user space program just open /dev/fb0
>> and mmap it to user space. I does not turn on fbcon so everything looks
>> well. If support fbcon is essential, I would implement vmap correctly.
>> If it's not essential, I think I could return 0 when
>> CONFIG_FRAMBUFFER_CONSOLE is defined.
> 
> I think fbdev emulation without working fbcon is fairly useless. And it
> shouldn't be that much work to make it work, we have quite a few more
> helers for gem bo nowadays.

If the virtual address is not set, fbcon can't be used and that means it
has to be disabled using something like this in the driver Kconfig:
	depends on !FRAMEBUFFER_CONSOLE

Otherwise it will crash if someone tries to use it.

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.

Noralf.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Aw: Re: Re: [PATCH] drm/mediatek: Add MTK Framebuffer-Device (mt7623)
  2019-01-11  9:49             ` CK Hu
  2019-01-11 10:25               ` Daniel Vetter
@ 2019-01-11 18:23               ` Frank Wunderlich
  1 sibling, 0 replies; 14+ messages in thread
From: Frank Wunderlich @ 2019-01-11 18:23 UTC (permalink / raw)
  To: CK Hu
  Cc: Philipp Zabel, David Airlie, Alexander Ryabchenko,
	Linux Kernel Mailing List, dri-devel,
	"Noralf Trønnes",
	moderated list:ARM/Mediatek SoC support, Daniel Vetter,
	Matthias Brugger, Linux ARM

like Daniel guessed, fb-console does not work with this Patch.

i tried your Patch (additional patch for headerfile): https://github.com/frank-w/BPI-R2-4.14/commits/4.20-fbdev

and R2 hangs  (no crash and no further boot)

[    5.435075] mediatek-drm 14000000.dispsys: bound 14012000.rdma (ops mtk_disp)
[    5.435134] mediatek-drm 14000000.dispsys: bound 14014000.dpi (ops mtk_dpi_c)
[    5.459277] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).      
[    5.465863] [drm] No driver support for vblank timestamp query.              
[    5.472331] [drm] Initialized mediatek 1.0.0 20150513 for 14000000.dispsys o0

my monitor is switched on, but no output....we had added fbdev-driver to get FB-console over hdmi, so it's currently useless

regards Frank

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/mediatek: Add MTK Framebuffer-Device (mt7623)
  2019-01-11 16:07                 ` Noralf Trønnes
@ 2019-01-14  2:02                   ` CK Hu
  0 siblings, 0 replies; 14+ messages in thread
From: CK Hu @ 2019-01-14  2:02 UTC (permalink / raw)
  To: Noralf Trønnes
  Cc: Frank Wunderlich, David Airlie, Alexander Ryabchenko,
	Linux Kernel Mailing List, dri-devel,
	moderated list:ARM/Mediatek SoC support, Philipp Zabel,
	Matthias Brugger, Linux ARM

On Fri, 2019-01-11 at 17:07 +0100, Noralf Trønnes wrote:
> 
> Den 11.01.2019 11.25, skrev Daniel Vetter:
> > On Fri, Jan 11, 2019 at 05:49:15PM +0800, CK Hu wrote:
> >> Hi, Daniel:
> >>
> >> On Fri, 2019-01-11 at 10:20 +0100, Daniel Vetter wrote:
> >>> On Fri, Jan 11, 2019 at 11:20:09AM +0800, CK Hu wrote:
> >>>> Hi, Daniel:
> >>>>
> >>>> On Thu, 2019-01-10 at 21:02 +0100, Daniel Vetter wrote:
> >>>>> On Thu, Jan 10, 2019 at 08:01:37PM +0100, Frank Wunderlich wrote:
> >>>>>> Hi Daniel,
> >>>>>>
> >>>>>>>> Would be good to use the new generic fbdev emulation code here, for even
> >>>>>>>> less code. Or at least know why this isn't possible to use for mtk (and
> >>>>>>>> maybe address that in the core code). Hand-rolling fbdev code shouldn't be
> >>>>>>>> needed anymore.
> >>>>>>>
> >>>>>>> Back on the mailing list, no private replies please:
> >>>>>>
> >>>>>> i don't wanted to spam all people with dumb questions ;)
> >>>>>
> >>>>> There's no dumb questions, only insufficient documentation :-)
> >>>>>
> >>>>>>> For examples please grep for drm_fbdev_generic_setup(). There's also a
> >>>>>>> still in-flight series from Gerd Hoffmann to convert over bochs. That,
> >>>>>>> plus all the kerneldoc linked from there should get you started.
> >>>>>>> -Daniel
> >>>>>>
> >>>>>> this is one of google best founds if i search for drm_fbdev_generic_setup:
> >>>>>>
> >>>>>> https://lkml.org/lkml/2018/12/19/305
> >>>>>>
> >>>>>> not very helpful...
> >>>>>>
> >>>>>> so i tried kernel-doc
> >>>>>>
> >>>>>> https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html?highlight=drm_fbdev_generic_setup#c.drm_fbdev_generic_setup
> >>>>>>
> >>>>>> which is nice function-reference but i've found no generic workflow
> >>>>>>
> >>>>>> as the posted driver is "only" a driver ported from kernel 4.4 by Alexander, i don't know if this new framework can be used and which parts need to be changed. I only try to bring his code Mainline....
> >>>>>> Maybe CK Hu can help here because driver is originally from him and he knows internals. Or maybe you can help here?
> >>>>>>
> >>>>>> i personally make my first steps as spare-time kernel-developer :)
> >>>>>
> >>>>> There's a ton of in-kernel users of that function already, I meant you can
> >>>>> use those to serve as examples ... If those + the kerneldoc aren't
> >>>>> good enough, then we need to improve them.
> >>>>> -Daniel
> >>>>
> >>>> I've tried with drm_fbdev_generic_setup() and it works fine with simple
> >>>> modification. The patch is
> >>>>
> >>>> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> >>>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> >>>> @@ -16,6 +16,7 @@
> >>>>  #include <drm/drm_atomic.h>
> >>>>  #include <drm/drm_atomic_helper.h>
> >>>>  #include <drm/drm_crtc_helper.h>
> >>>> +#include <drm/drm_fb_helper.h>
> >>>>  #include <drm/drm_gem.h>
> >>>>  #include <drm/drm_gem_cma_helper.h>
> >>>>  #include <drm/drm_of.h>
> >>>> @@ -379,6 +380,7 @@ static void mtk_drm_kms_deinit(struct drm_device
> >>>> *drm)
> >>>>         .gem_prime_get_sg_table = mtk_gem_prime_get_sg_table,
> >>>>         .gem_prime_import_sg_table = mtk_gem_prime_import_sg_table,
> >>>>         .gem_prime_mmap = mtk_drm_gem_mmap_buf,
> >>>> +       .gem_prime_vmap = mtk_drm_gem_prime_vmap,
> >>>>         .ioctls = mtk_ioctls,
> >>>>         .num_ioctls = ARRAY_SIZE(mtk_ioctls),
> >>>>         .fops = &mtk_drm_fops,
> >>>> @@ -416,6 +418,8 @@ static int mtk_drm_bind(struct device *dev)
> >>>>         if (ret < 0)
> >>>>                 goto err_deinit;
> >>>>
> >>>> +       drm_fbdev_generic_setup(drm, 32);
> >>>> +
> >>>>         return 0;
> >>>>
> >>>>
> >>>> But I implement .gem_prime_vmap with a workaround,
> >>>>
> >>>>
> >>>> --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> >>>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> >>>> @@ -280,3 +280,8 @@ int mtk_gem_create_ioctl(struct drm_device *dev,
> >>>> void *data,
> >>>>         mtk_drm_gem_free_object(&mtk_gem->base);
> >>>>         return ret;
> >>>>  }
> >>>> +
> >>>> +void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj)
> >>>> +{
> >>>> +       return (void *)1;
> >>>> +}
> >>>>
> >>>> Current drm_fb_helper depends on drm_client to create fbdev. When
> >>>> drm_client create drm_client_buffer, it need to vmap to get kernel
> >>>> vaddr. But I think for fbdev, the vaddr is useless. Do you agree that I
> >>>> temporarily implement .gem_prime_vmap in such way?
> >>>
> >>> The vmap is used by fbcon, without that fbdev won't really work I think.
> >>> So I'm rather confused on what you tested ...
> >>>
> >>> Adding Noralf, he's the expert here.
> >>> -Daniel
> >>
> >> I test with fb_test [1], it is a user space program just open /dev/fb0
> >> and mmap it to user space. I does not turn on fbcon so everything looks
> >> well. If support fbcon is essential, I would implement vmap correctly.
> >> If it's not essential, I think I could return 0 when
> >> CONFIG_FRAMBUFFER_CONSOLE is defined.
> > 
> > I think fbdev emulation without working fbcon is fairly useless. And it
> > shouldn't be that much work to make it work, we have quite a few more
> > helers for gem bo nowadays.
> 
> If the virtual address is not set, fbcon can't be used and that means it
> has to be disabled using something like this in the driver Kconfig:
> 	depends on !FRAMEBUFFER_CONSOLE
> 
> Otherwise it will crash if someone tries to use it.
> 
> 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.



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/mediatek: Add MTK Framebuffer-Device (mt7623)
  2019-01-02 12:25 ` Frank Wunderlich
@ 2019-01-04 10:01   ` CK Hu
  0 siblings, 0 replies; 14+ messages in thread
From: CK Hu @ 2019-01-04 10:01 UTC (permalink / raw)
  To: Frank Wunderlich; +Cc: Alexander Ryabchenko, linux-mediatek, linux-arm-kernel

Hi, Frank:

On Wed, 2019-01-02 at 13:25 +0100, Frank Wunderlich wrote:
> From: CK Hu <ck.hu@mediatek.com>
> 
> This patch adds Framebuffer-Driver for Mediatek
> 
> currently tested on mt7623, maybe works on other platforms
> MTK-FBDev written by CK Hu and ported from 4.4
> 
> based on patchset drm/hdmi for 7623 v5 (except last part included in 4.20)
>   https://patchwork.kernel.org/project/linux-mediatek/list/?series=25989
> 
> depends on
>   dts-patch (resend of 5/5, parts 1-4 already in 4.20):
>     "arm: dts: mt7623: add display subsystem related device nodes"
>     https://patchwork.kernel.org/patch/10588951/
>   2 bugfix-patches from bibby hsieh
>     fix-boot-up-for-720-and-480-but-1080
>     using-different-round-rate-for-mt7623
>   1 Patch from Ryder Lee
>     http://forum.banana-pi.org/t/kernel-4-19-rc1-for-testers/6618/52
> 
> full working tree here for reference:
> https://github.com/frank-w/BPI-R2-4.14/commits/4.20-hdmiv5
> 
> Signed-off-by: CK Hu <ck.hu@mediatek.com>
> Signed-off-by: Alexander Ryabchenko <d3adme4t@gmail.com>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> Tested-by: Frank Wunderlich <frank-w@public-files.de>
> ---
>  drivers/gpu/drm/mediatek/Makefile        |   1 +
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c   |   6 +
>  drivers/gpu/drm/mediatek/mtk_drm_drv.h   |   4 +
>  drivers/gpu/drm/mediatek/mtk_drm_fb.c    |  13 ++
>  drivers/gpu/drm/mediatek/mtk_drm_fb.h    |   3 +
>  drivers/gpu/drm/mediatek/mtk_drm_fbdev.c | 178 +++++++++++++++++++++++
>  drivers/gpu/drm/mediatek/mtk_drm_fbdev.h |  25 ++++
>  7 files changed, 230 insertions(+)
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_fbdev.c
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_fbdev.h
> 
> diff --git a/drivers/gpu/drm/mediatek/Makefile b/drivers/gpu/drm/mediatek/Makefile
> index 82ae49c64221..d71e57dea77a 100644
> --- a/drivers/gpu/drm/mediatek/Makefile
> +++ b/drivers/gpu/drm/mediatek/Makefile
> @@ -14,6 +14,7 @@ mediatek-drm-y := mtk_disp_color.o \
>  		  mtk_mipi_tx.o \
>  		  mtk_dpi.o
>  
> +mediatek-drm-$(CONFIG_DRM_FBDEV_EMULATION) += mtk_drm_fbdev.o
>  obj-$(CONFIG_DRM_MEDIATEK) += mediatek-drm.o
>  
>  mediatek-drm-hdmi-objs := mtk_cec.o \
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 7800822b4db1..51f1ec3412d3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -32,6 +32,7 @@
>  #include "mtk_drm_ddp_comp.h"
>  #include "mtk_drm_drv.h"
>  #include "mtk_drm_fb.h"
> +#include "mtk_drm_fbdev.h"
>  #include "mtk_drm_gem.h"
>  
>  #define DRIVER_NAME "mediatek"
> @@ -299,6 +300,10 @@ static int mtk_drm_kms_init(struct drm_device *drm)
>  	drm_kms_helper_poll_init(drm);
>  	drm_mode_config_reset(drm);
>  
> +	ret = mtk_fbdev_init(drm);
> +	if (ret)
> +		goto err_component_unbind;
> +
>  	return 0;
>  
>  err_component_unbind:
> @@ -311,6 +316,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
>  
>  static void mtk_drm_kms_deinit(struct drm_device *drm)
>  {
> +	mtk_fbdev_fini(drm);
>  	drm_kms_helper_poll_fini(drm);
>  
>  	component_unbind_all(drm->dev, drm);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> index 256a3ff2e66e..56129a21fb2b 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> @@ -14,6 +14,7 @@
>  #ifndef MTK_DRM_DRV_H
>  #define MTK_DRM_DRV_H
>  
> +#include <drm/drm_fb_helper.h>
>  #include <linux/io.h>
>  #include "mtk_drm_ddp_comp.h"
>  
> @@ -43,6 +44,7 @@ struct mtk_drm_private {
>  	struct drm_device *drm;
>  	struct device *dma_dev;
>  
> +	struct drm_crtc *crtc[MAX_CRTC];

Useless, so remove it.

>  	unsigned int num_pipes;
>  
>  	struct device_node *mutex_node;
> @@ -59,6 +61,8 @@ struct mtk_drm_private {
>  	} commit;
>  
>  	struct drm_atomic_state *suspend_state;
> +	struct drm_fb_helper fb_helper;
> +	struct drm_gem_object *fbdev_bo;
>  };
>  
>  extern struct platform_driver mtk_ddp_driver;
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fb.c b/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> index be5f6f1daf55..7533aa4733d2 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> @@ -56,6 +56,19 @@ static struct drm_framebuffer *mtk_drm_framebuffer_init(struct drm_device *dev,
>  	return fb;
>  }
>  
> +struct drm_framebuffer *mtk_drm_framebuffer_create(struct drm_device *dev,
> +					const struct drm_mode_fb_cmd2 *mode,
> +					struct drm_gem_object *obj)
> +{
> +	struct drm_framebuffer *mtk_fb;
> +
> +	mtk_fb = mtk_drm_framebuffer_init(dev, mode, obj);
> +	if (IS_ERR(mtk_fb))
> +		return ERR_CAST(mtk_fb);
> +
> +	return mtk_fb;
> +}

mtk_drm_framebuffer_create() is exactly the same as
mtk_drm_framebuffer_init(), so remove this function and use
mtk_drm_framebuffer_init() directly.

> +
>  /*
>   * Wait for any exclusive fence in fb's gem object's reservation object.
>   *
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fb.h b/drivers/gpu/drm/mediatek/mtk_drm_fb.h
> index 7f976b196a15..e49e015380d0 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_fb.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_fb.h
> @@ -18,5 +18,8 @@ int mtk_fb_wait(struct drm_framebuffer *fb);
>  struct drm_framebuffer *mtk_drm_mode_fb_create(struct drm_device *dev,
>  					       struct drm_file *file,
>  					       const struct drm_mode_fb_cmd2 *cmd);
> +struct drm_framebuffer *mtk_drm_framebuffer_create(struct drm_device *dev,
> +		const struct drm_mode_fb_cmd2 *mode,
> +		struct drm_gem_object *obj);
>  
>  #endif /* MTK_DRM_FB_H */
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fbdev.c b/drivers/gpu/drm/mediatek/mtk_drm_fbdev.c
> new file mode 100644
> index 000000000000..9670f50d1247
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_fbdev.c
> @@ -0,0 +1,178 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
> + * Author: CK Hu <ck.hu@mediatek.com>
> + */
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_fb_helper.h>
> +#include <drm/drm_gem.h>
> +
> +#include "mtk_drm_drv.h"
> +#include "mtk_drm_fb.h"
> +#include "mtk_drm_gem.h"
> +#include "mtk_drm_fbdev.h"

Alphabetic order.

> +
> +#define to_drm_private(x) \
> +		container_of(x, struct mtk_drm_private, fb_helper)
> +
> +static int mtk_drm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
> +{
> +	struct drm_fb_helper *helper = info->par;
> +	struct mtk_drm_private *private = to_drm_private(helper);
> +
> +	return mtk_drm_gem_mmap_buf(private->fbdev_bo, vma);
> +}
> +
> +static struct fb_ops mtk_fbdev_ops = {
> +	.owner = THIS_MODULE,
> +	DRM_FB_HELPER_DEFAULT_OPS,
> +	.fb_fillrect = drm_fb_helper_cfb_fillrect,
> +	.fb_copyarea = drm_fb_helper_cfb_copyarea,
> +	.fb_imageblit = drm_fb_helper_cfb_imageblit,
> +	.fb_check_var = drm_fb_helper_check_var,
> +	.fb_set_par = drm_fb_helper_set_par,
> +	.fb_blank = drm_fb_helper_blank,
> +	.fb_pan_display = drm_fb_helper_pan_display,
> +	.fb_setcmap = drm_fb_helper_setcmap,
> +	.fb_mmap = mtk_drm_fbdev_mmap,
> +};

DRM_FB_HELPER_DEFAULT_OPS is defined as follow, so you should remove the
same definition part.

#define DRM_FB_HELPER_DEFAULT_OPS \
	.fb_check_var	= drm_fb_helper_check_var, \
	.fb_set_par	= drm_fb_helper_set_par, \
	.fb_setcmap	= drm_fb_helper_setcmap, \
	.fb_blank	= drm_fb_helper_blank, \
	.fb_pan_display	= drm_fb_helper_pan_display, \
	.fb_debug_enter = drm_fb_helper_debug_enter, \
	.fb_debug_leave = drm_fb_helper_debug_leave, \
	.fb_ioctl	= drm_fb_helper_ioctl

> +
> +static int mtk_fbdev_probe(struct drm_fb_helper *helper,
> +			   struct drm_fb_helper_surface_size *sizes)
> +{
> +	struct drm_device *dev = helper->dev;
> +	struct mtk_drm_private *private = to_drm_private(helper);
> +	struct drm_mode_fb_cmd2 mode = { 0 };
> +	struct mtk_drm_gem_obj *mtk_gem;
> +	struct fb_info *info;
> +	struct drm_framebuffer *fb;
> +	unsigned int bytes_per_pixel;
> +	unsigned long offset;
> +	size_t size;
> +	int err = 0;

Need not initialize to zero.

> +
> +	bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
> +
> +	mode.width = sizes->surface_width;
> +	mode.height = sizes->surface_height;
> +	mode.pitches[0] = sizes->surface_width * bytes_per_pixel;
> +	mode.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
> +						      sizes->surface_depth);
> +
> +	size = mode.pitches[0] * mode.height;
> +
> +	mtk_gem = mtk_drm_gem_create(dev, size, true);
> +	if (IS_ERR(mtk_gem))
> +		return PTR_ERR(mtk_gem);
> +
> +	private->fbdev_bo = &mtk_gem->base;
> +
> +	info = drm_fb_helper_alloc_fbi(helper);
> +	if (IS_ERR(info)) {
> +		DRM_DEV_ERROR(dev->dev,
> +			"failed to allocate framebuffer info, %d\n",

Align to the right of '('.

> +			err);

Why print err before assign it?

> +		err = PTR_ERR(info);
> +		goto out;
> +	}
> +
> +	fb = mtk_drm_framebuffer_create(dev, &mode, private->fbdev_bo);
> +	if (IS_ERR(fb)) {
> +		DRM_DEV_ERROR(dev->dev,
> +			"failed to allocate DRM framebuffer, %d\n",

Align to the right of '('.

> +			err);

Why print err before assign it?

> +		err = PTR_ERR(fb);
> +		goto out;

I think here should goto somewhere to undo drm_fb_helper_alloc_fbi().

> +	}
> +	helper->fb = fb;
> +
> +	info->par = helper;
> +	info->flags = FBINFO_FLAG_DEFAULT;
> +	info->fbops = &mtk_fbdev_ops;
> +
> +	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format->depth);
> +	drm_fb_helper_fill_var(info, helper, sizes->fb_width, sizes->fb_height);
> +
> +	offset = info->var.xoffset * bytes_per_pixel;
> +	offset += info->var.yoffset * fb->pitches[0];
> +
> +	dev->mode_config.fb_base = 0;
> +	info->screen_base = mtk_gem->kvaddr + offset;
> +	info->screen_size = size;
> +	info->fix.smem_len = size;
> +
> +	DRM_DEBUG_KMS("FB [%ux%u]-%u offset=%lu size=%zd\n",
> +		      fb->width, fb->height, fb->format->depth, offset, size);
> +
> +	info->skip_vt_switch = true;
> +
> +	return 0;
> +
> +out:

I would like the label naming as 'err_gem_free_object'

> +
> +
> +	mtk_drm_gem_free_object(&mtk_gem->base);
> +	return err;
> +}
> +
> +static const struct drm_fb_helper_funcs mtk_drm_fb_helper_funcs = {
> +	.fb_probe = mtk_fbdev_probe,
> +};
> +
> +int mtk_fbdev_init(struct drm_device *dev)
> +{
> +	struct mtk_drm_private *priv = dev->dev_private;
> +	struct drm_fb_helper *helper = &priv->fb_helper;
> +	int ret;
> +
> +	if (!dev->mode_config.num_crtc || !dev->mode_config.num_connector)
> +		return -EINVAL;
> +
> +	drm_fb_helper_prepare(dev, helper, &mtk_drm_fb_helper_funcs);
> +
> +	ret = drm_fb_helper_init(dev, helper, dev->mode_config.num_connector);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(dev->dev,
> +			"failed to initialize DRM FB helper, %d\n",
> +			ret);
> +//		goto fini;

Remove useless line.

> +		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 fini;
> +	}
> +
> +	ret = drm_fb_helper_initial_config(helper, 32);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(dev->dev,
> +			"failed to set initial configuration, %d\n",

Align to the right of '('.

> +			ret);
> +		goto fini;
> +	}
> +
> +	return 0;
> +
> +fini:

I would like the label naming as err_fini.

Regards,
CK

> +	drm_fb_helper_fini(helper);
> +	return ret;
> +}
> +
> +void mtk_fbdev_fini(struct drm_device *dev)
> +{
> +	struct mtk_drm_private *priv = dev->dev_private;
> +	struct drm_fb_helper *helper = &priv->fb_helper;
> +
> +	drm_fb_helper_unregister_fbi(helper);
> +
> +	if (helper->fb) {
> +		drm_framebuffer_unregister_private(helper->fb);
> +		drm_framebuffer_remove(helper->fb);
> +	}
> +
> +	drm_fb_helper_fini(helper);
> +}
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fbdev.h b/drivers/gpu/drm/mediatek/mtk_drm_fbdev.h
> new file mode 100644
> index 000000000000..45717b642dc5
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_fbdev.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
> + * Author: CK Hu <ck.hu@mediatek.com>
> + */
> +
> +#ifndef MTK_DRM_FBDEV_H
> +#define MTK_DRM_FBDEV_H
> +
> +#ifdef CONFIG_DRM_FBDEV_EMULATION
> +int mtk_fbdev_init(struct drm_device *dev);
> +void mtk_fbdev_fini(struct drm_device *dev);
> +#else
> +int mtk_fbdev_init(struct drm_device *dev)
> +{
> +	return 0;
> +}
> +
> +void mtk_fbdev_fini(struct drm_device *dev)
> +{
> +
> +}
> +#endif /* CONFIG_DRM_FBDEV_EMULATION */
> +
> +#endif /* MTK_DRM_FBDEV_H */



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] drm/mediatek: Add MTK Framebuffer-Device (mt7623)
       [not found] <20190102122540.7002-1-frank-w@public-files.de>
@ 2019-01-02 12:25 ` Frank Wunderlich
  2019-01-04 10:01   ` CK Hu
  0 siblings, 1 reply; 14+ messages in thread
From: Frank Wunderlich @ 2019-01-02 12:25 UTC (permalink / raw)
  To: linux-arm-kernel, linux-mediatek
  Cc: CK Hu, Alexander Ryabchenko, Frank Wunderlich

From: CK Hu <ck.hu@mediatek.com>

This patch adds Framebuffer-Driver for Mediatek

currently tested on mt7623, maybe works on other platforms
MTK-FBDev written by CK Hu and ported from 4.4

based on patchset drm/hdmi for 7623 v5 (except last part included in 4.20)
  https://patchwork.kernel.org/project/linux-mediatek/list/?series=25989

depends on
  dts-patch (resend of 5/5, parts 1-4 already in 4.20):
    "arm: dts: mt7623: add display subsystem related device nodes"
    https://patchwork.kernel.org/patch/10588951/
  2 bugfix-patches from bibby hsieh
    fix-boot-up-for-720-and-480-but-1080
    using-different-round-rate-for-mt7623
  1 Patch from Ryder Lee
    http://forum.banana-pi.org/t/kernel-4-19-rc1-for-testers/6618/52

full working tree here for reference:
https://github.com/frank-w/BPI-R2-4.14/commits/4.20-hdmiv5

Signed-off-by: CK Hu <ck.hu@mediatek.com>
Signed-off-by: Alexander Ryabchenko <d3adme4t@gmail.com>
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
Tested-by: Frank Wunderlich <frank-w@public-files.de>
---
 drivers/gpu/drm/mediatek/Makefile        |   1 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.c   |   6 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.h   |   4 +
 drivers/gpu/drm/mediatek/mtk_drm_fb.c    |  13 ++
 drivers/gpu/drm/mediatek/mtk_drm_fb.h    |   3 +
 drivers/gpu/drm/mediatek/mtk_drm_fbdev.c | 178 +++++++++++++++++++++++
 drivers/gpu/drm/mediatek/mtk_drm_fbdev.h |  25 ++++
 7 files changed, 230 insertions(+)
 create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_fbdev.c
 create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_fbdev.h

diff --git a/drivers/gpu/drm/mediatek/Makefile b/drivers/gpu/drm/mediatek/Makefile
index 82ae49c64221..d71e57dea77a 100644
--- a/drivers/gpu/drm/mediatek/Makefile
+++ b/drivers/gpu/drm/mediatek/Makefile
@@ -14,6 +14,7 @@ mediatek-drm-y := mtk_disp_color.o \
 		  mtk_mipi_tx.o \
 		  mtk_dpi.o
 
+mediatek-drm-$(CONFIG_DRM_FBDEV_EMULATION) += mtk_drm_fbdev.o
 obj-$(CONFIG_DRM_MEDIATEK) += mediatek-drm.o
 
 mediatek-drm-hdmi-objs := mtk_cec.o \
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 7800822b4db1..51f1ec3412d3 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -32,6 +32,7 @@
 #include "mtk_drm_ddp_comp.h"
 #include "mtk_drm_drv.h"
 #include "mtk_drm_fb.h"
+#include "mtk_drm_fbdev.h"
 #include "mtk_drm_gem.h"
 
 #define DRIVER_NAME "mediatek"
@@ -299,6 +300,10 @@ static int mtk_drm_kms_init(struct drm_device *drm)
 	drm_kms_helper_poll_init(drm);
 	drm_mode_config_reset(drm);
 
+	ret = mtk_fbdev_init(drm);
+	if (ret)
+		goto err_component_unbind;
+
 	return 0;
 
 err_component_unbind:
@@ -311,6 +316,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
 
 static void mtk_drm_kms_deinit(struct drm_device *drm)
 {
+	mtk_fbdev_fini(drm);
 	drm_kms_helper_poll_fini(drm);
 
 	component_unbind_all(drm->dev, drm);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index 256a3ff2e66e..56129a21fb2b 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -14,6 +14,7 @@
 #ifndef MTK_DRM_DRV_H
 #define MTK_DRM_DRV_H
 
+#include <drm/drm_fb_helper.h>
 #include <linux/io.h>
 #include "mtk_drm_ddp_comp.h"
 
@@ -43,6 +44,7 @@ struct mtk_drm_private {
 	struct drm_device *drm;
 	struct device *dma_dev;
 
+	struct drm_crtc *crtc[MAX_CRTC];
 	unsigned int num_pipes;
 
 	struct device_node *mutex_node;
@@ -59,6 +61,8 @@ struct mtk_drm_private {
 	} commit;
 
 	struct drm_atomic_state *suspend_state;
+	struct drm_fb_helper fb_helper;
+	struct drm_gem_object *fbdev_bo;
 };
 
 extern struct platform_driver mtk_ddp_driver;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fb.c b/drivers/gpu/drm/mediatek/mtk_drm_fb.c
index be5f6f1daf55..7533aa4733d2 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_fb.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_fb.c
@@ -56,6 +56,19 @@ static struct drm_framebuffer *mtk_drm_framebuffer_init(struct drm_device *dev,
 	return fb;
 }
 
+struct drm_framebuffer *mtk_drm_framebuffer_create(struct drm_device *dev,
+					const struct drm_mode_fb_cmd2 *mode,
+					struct drm_gem_object *obj)
+{
+	struct drm_framebuffer *mtk_fb;
+
+	mtk_fb = mtk_drm_framebuffer_init(dev, mode, obj);
+	if (IS_ERR(mtk_fb))
+		return ERR_CAST(mtk_fb);
+
+	return mtk_fb;
+}
+
 /*
  * Wait for any exclusive fence in fb's gem object's reservation object.
  *
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fb.h b/drivers/gpu/drm/mediatek/mtk_drm_fb.h
index 7f976b196a15..e49e015380d0 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_fb.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_fb.h
@@ -18,5 +18,8 @@ int mtk_fb_wait(struct drm_framebuffer *fb);
 struct drm_framebuffer *mtk_drm_mode_fb_create(struct drm_device *dev,
 					       struct drm_file *file,
 					       const struct drm_mode_fb_cmd2 *cmd);
+struct drm_framebuffer *mtk_drm_framebuffer_create(struct drm_device *dev,
+		const struct drm_mode_fb_cmd2 *mode,
+		struct drm_gem_object *obj);
 
 #endif /* MTK_DRM_FB_H */
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fbdev.c b/drivers/gpu/drm/mediatek/mtk_drm_fbdev.c
new file mode 100644
index 000000000000..9670f50d1247
--- /dev/null
+++ b/drivers/gpu/drm/mediatek/mtk_drm_fbdev.c
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 MediaTek Inc.
+ * Author: CK Hu <ck.hu@mediatek.com>
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_gem.h>
+
+#include "mtk_drm_drv.h"
+#include "mtk_drm_fb.h"
+#include "mtk_drm_gem.h"
+#include "mtk_drm_fbdev.h"
+
+#define to_drm_private(x) \
+		container_of(x, struct mtk_drm_private, fb_helper)
+
+static int mtk_drm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
+{
+	struct drm_fb_helper *helper = info->par;
+	struct mtk_drm_private *private = to_drm_private(helper);
+
+	return mtk_drm_gem_mmap_buf(private->fbdev_bo, vma);
+}
+
+static struct fb_ops mtk_fbdev_ops = {
+	.owner = THIS_MODULE,
+	DRM_FB_HELPER_DEFAULT_OPS,
+	.fb_fillrect = drm_fb_helper_cfb_fillrect,
+	.fb_copyarea = drm_fb_helper_cfb_copyarea,
+	.fb_imageblit = drm_fb_helper_cfb_imageblit,
+	.fb_check_var = drm_fb_helper_check_var,
+	.fb_set_par = drm_fb_helper_set_par,
+	.fb_blank = drm_fb_helper_blank,
+	.fb_pan_display = drm_fb_helper_pan_display,
+	.fb_setcmap = drm_fb_helper_setcmap,
+	.fb_mmap = mtk_drm_fbdev_mmap,
+};
+
+static int mtk_fbdev_probe(struct drm_fb_helper *helper,
+			   struct drm_fb_helper_surface_size *sizes)
+{
+	struct drm_device *dev = helper->dev;
+	struct mtk_drm_private *private = to_drm_private(helper);
+	struct drm_mode_fb_cmd2 mode = { 0 };
+	struct mtk_drm_gem_obj *mtk_gem;
+	struct fb_info *info;
+	struct drm_framebuffer *fb;
+	unsigned int bytes_per_pixel;
+	unsigned long offset;
+	size_t size;
+	int err = 0;
+
+	bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
+
+	mode.width = sizes->surface_width;
+	mode.height = sizes->surface_height;
+	mode.pitches[0] = sizes->surface_width * bytes_per_pixel;
+	mode.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
+						      sizes->surface_depth);
+
+	size = mode.pitches[0] * mode.height;
+
+	mtk_gem = mtk_drm_gem_create(dev, size, true);
+	if (IS_ERR(mtk_gem))
+		return PTR_ERR(mtk_gem);
+
+	private->fbdev_bo = &mtk_gem->base;
+
+	info = drm_fb_helper_alloc_fbi(helper);
+	if (IS_ERR(info)) {
+		DRM_DEV_ERROR(dev->dev,
+			"failed to allocate framebuffer info, %d\n",
+			err);
+		err = PTR_ERR(info);
+		goto out;
+	}
+
+	fb = mtk_drm_framebuffer_create(dev, &mode, private->fbdev_bo);
+	if (IS_ERR(fb)) {
+		DRM_DEV_ERROR(dev->dev,
+			"failed to allocate DRM framebuffer, %d\n",
+			err);
+		err = PTR_ERR(fb);
+		goto out;
+	}
+	helper->fb = fb;
+
+	info->par = helper;
+	info->flags = FBINFO_FLAG_DEFAULT;
+	info->fbops = &mtk_fbdev_ops;
+
+	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format->depth);
+	drm_fb_helper_fill_var(info, helper, sizes->fb_width, sizes->fb_height);
+
+	offset = info->var.xoffset * bytes_per_pixel;
+	offset += info->var.yoffset * fb->pitches[0];
+
+	dev->mode_config.fb_base = 0;
+	info->screen_base = mtk_gem->kvaddr + offset;
+	info->screen_size = size;
+	info->fix.smem_len = size;
+
+	DRM_DEBUG_KMS("FB [%ux%u]-%u offset=%lu size=%zd\n",
+		      fb->width, fb->height, fb->format->depth, offset, size);
+
+	info->skip_vt_switch = true;
+
+	return 0;
+
+out:
+
+
+	mtk_drm_gem_free_object(&mtk_gem->base);
+	return err;
+}
+
+static const struct drm_fb_helper_funcs mtk_drm_fb_helper_funcs = {
+	.fb_probe = mtk_fbdev_probe,
+};
+
+int mtk_fbdev_init(struct drm_device *dev)
+{
+	struct mtk_drm_private *priv = dev->dev_private;
+	struct drm_fb_helper *helper = &priv->fb_helper;
+	int ret;
+
+	if (!dev->mode_config.num_crtc || !dev->mode_config.num_connector)
+		return -EINVAL;
+
+	drm_fb_helper_prepare(dev, helper, &mtk_drm_fb_helper_funcs);
+
+	ret = drm_fb_helper_init(dev, helper, dev->mode_config.num_connector);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev->dev,
+			"failed to initialize DRM FB helper, %d\n",
+			ret);
+//		goto fini;
+		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 fini;
+	}
+
+	ret = drm_fb_helper_initial_config(helper, 32);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev->dev,
+			"failed to set initial configuration, %d\n",
+			ret);
+		goto fini;
+	}
+
+	return 0;
+
+fini:
+	drm_fb_helper_fini(helper);
+	return ret;
+}
+
+void mtk_fbdev_fini(struct drm_device *dev)
+{
+	struct mtk_drm_private *priv = dev->dev_private;
+	struct drm_fb_helper *helper = &priv->fb_helper;
+
+	drm_fb_helper_unregister_fbi(helper);
+
+	if (helper->fb) {
+		drm_framebuffer_unregister_private(helper->fb);
+		drm_framebuffer_remove(helper->fb);
+	}
+
+	drm_fb_helper_fini(helper);
+}
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fbdev.h b/drivers/gpu/drm/mediatek/mtk_drm_fbdev.h
new file mode 100644
index 000000000000..45717b642dc5
--- /dev/null
+++ b/drivers/gpu/drm/mediatek/mtk_drm_fbdev.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2018 MediaTek Inc.
+ * Author: CK Hu <ck.hu@mediatek.com>
+ */
+
+#ifndef MTK_DRM_FBDEV_H
+#define MTK_DRM_FBDEV_H
+
+#ifdef CONFIG_DRM_FBDEV_EMULATION
+int mtk_fbdev_init(struct drm_device *dev);
+void mtk_fbdev_fini(struct drm_device *dev);
+#else
+int mtk_fbdev_init(struct drm_device *dev)
+{
+	return 0;
+}
+
+void mtk_fbdev_fini(struct drm_device *dev)
+{
+
+}
+#endif /* CONFIG_DRM_FBDEV_EMULATION */
+
+#endif /* MTK_DRM_FBDEV_H */
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-01-14  2:02 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20190102084917.4049-1-frank-w@public-files.de>
2019-01-07 10:36 ` [PATCH] drm/mediatek: Add MTK Framebuffer-Device (mt7623) Daniel Vetter
2019-01-08 13:40   ` Daniel Vetter
2019-01-10 19:01     ` Aw: " Frank Wunderlich
2019-01-10 20:02       ` Daniel Vetter
2019-01-11  3:20         ` CK Hu
2019-01-11  9:20           ` Daniel Vetter
2019-01-11  9:49             ` CK Hu
2019-01-11 10:25               ` Daniel Vetter
2019-01-11 16:07                 ` Noralf Trønnes
2019-01-14  2:02                   ` CK Hu
2019-01-11 18:23               ` Aw: Re: " Frank Wunderlich
2019-01-11  1:31       ` Aw: " CK Hu
     [not found] <20190102122540.7002-1-frank-w@public-files.de>
2019-01-02 12:25 ` Frank Wunderlich
2019-01-04 10:01   ` CK Hu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).