Re: [PATCH v2 2/3] drm: simpledrm: add fbdev fallback support

[Paul Gortmaker]

On Fri, Aug 5, 2016 at 11:44 AM, Noralf TrÃnnes <noralf@xxxxxxxxxxx> wrote:
> Create a simple fbdev device during SimpleDRM setup so legacy user-space
> and fbcon can use it.
>
> Original work by David Herrmann.
>
> Cc: dh.herrmann@xxxxxxxxx
> Signed-off-by: Noralf TrÃnnes <noralf@xxxxxxxxxxx>
> ---
>
> Changes from version 1:
>   No changes
>
> Changes from previous version:
> - Remove the DRM_SIMPLEDRM_FBDEV kconfig option and use DRM_FBDEV_EMULATION
> - Suspend fbcon/fbdev when the pipeline is enabled, resume in lastclose
> - Add FBINFO_CAN_FORCE_OUTPUT flag so we get oops'es on the console
>
>  drivers/gpu/drm/simpledrm/Kconfig           |   3 +
>  drivers/gpu/drm/simpledrm/Makefile          |   1 +
>  drivers/gpu/drm/simpledrm/simpledrm.h       |  24 +++++
>  drivers/gpu/drm/simpledrm/simpledrm_drv.c   |   4 +
>  drivers/gpu/drm/simpledrm/simpledrm_fbdev.c | 160 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/simpledrm/simpledrm_kms.c   |  12 +++
>  6 files changed, 204 insertions(+)
>  create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
>
> diff --git a/drivers/gpu/drm/simpledrm/Kconfig b/drivers/gpu/drm/simpledrm/Kconfig
> index f45b25d..9454536 100644
> --- a/drivers/gpu/drm/simpledrm/Kconfig
> +++ b/drivers/gpu/drm/simpledrm/Kconfig
> @@ -13,6 +13,9 @@ config DRM_SIMPLEDRM
>           SimpleDRM supports "simple-framebuffer" DeviceTree objects and
>           compatible platform framebuffers.
>
> +         If fbdev support is enabled, this driver will also provide an fbdev
> +         compatibility layer.
> +
>           If unsure, say Y.
>
>           To compile this driver as a module, choose M here: the
> diff --git a/drivers/gpu/drm/simpledrm/Makefile b/drivers/gpu/drm/simpledrm/Makefile
> index f6a62dc..7087245 100644
> --- a/drivers/gpu/drm/simpledrm/Makefile
> +++ b/drivers/gpu/drm/simpledrm/Makefile
> @@ -1,4 +1,5 @@
>  simpledrm-y := simpledrm_drv.o simpledrm_kms.o simpledrm_gem.o \
>                 simpledrm_damage.o
> +simpledrm-$(CONFIG_DRM_FBDEV_EMULATION) += simpledrm_fbdev.o
>
>  obj-$(CONFIG_DRM_SIMPLEDRM) := simpledrm.o
> diff --git a/drivers/gpu/drm/simpledrm/simpledrm.h b/drivers/gpu/drm/simpledrm/simpledrm.h
> index f9f082c..eb18d59 100644
> --- a/drivers/gpu/drm/simpledrm/simpledrm.h
> +++ b/drivers/gpu/drm/simpledrm/simpledrm.h
> @@ -30,6 +30,7 @@ struct sdrm_device {
>         struct drm_device *ddev;
>         struct drm_simple_display_pipe pipe;
>         struct drm_connector conn;
> +       struct fb_info *fbdev;
>
>         /* framebuffer information */
>         const struct simplefb_format *fb_sformat;
> @@ -52,6 +53,7 @@ struct sdrm_device {
>  #endif
>  };
>
> +void sdrm_lastclose(struct drm_device *ddev);
>  int sdrm_drm_modeset_init(struct sdrm_device *sdrm);
>  int sdrm_drm_mmap(struct file *filp, struct vm_area_struct *vma);
>
> @@ -93,4 +95,26 @@ struct sdrm_framebuffer {
>
>  #define to_sdrm_fb(x) container_of(x, struct sdrm_framebuffer, base)
>
> +#ifdef CONFIG_DRM_FBDEV_EMULATION
> +
> +void sdrm_fbdev_init(struct sdrm_device *sdrm);
> +void sdrm_fbdev_cleanup(struct sdrm_device *sdrm);
> +void sdrm_fbdev_set_suspend(struct sdrm_device *sdrm, int state);
> +
> +#else
> +
> +static inline void sdrm_fbdev_init(struct sdrm_device *sdrm)
> +{
> +}
> +
> +static inline void sdrm_fbdev_cleanup(struct sdrm_device *sdrm)
> +{
> +}
> +
> +static inline void sdrm_fbdev_set_suspend(struct sdrm_device *sdrm, int state)
> +{
> +}
> +
> +#endif
> +
>  #endif /* SDRM_DRV_H */
> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_drv.c b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
> index 35296d2..88ad717c 100644
> --- a/drivers/gpu/drm/simpledrm/simpledrm_drv.c
> +++ b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
> @@ -41,6 +41,7 @@ static struct drm_driver sdrm_drm_driver = {
>         .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
>                            DRIVER_ATOMIC,
>         .fops = &sdrm_drm_fops,
> +       .lastclose = sdrm_lastclose,
>
>         .gem_free_object = sdrm_gem_free_object,
>         .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> @@ -447,6 +448,8 @@ static int sdrm_simplefb_probe(struct platform_device *pdev)
>         if (ret)
>                 goto err_regulators;
>
> +       sdrm_fbdev_init(ddev->dev_private);
> +
>         DRM_INFO("Initialized %s on minor %d\n", ddev->driver->name,
>                  ddev->primary->index);
>
> @@ -472,6 +475,7 @@ static int sdrm_simplefb_remove(struct platform_device *pdev)
>         struct drm_device *ddev = platform_get_drvdata(pdev);
>         struct sdrm_device *sdrm = ddev->dev_private;
>
> +       sdrm_fbdev_cleanup(sdrm);
>         drm_dev_unregister(ddev);
>         drm_mode_config_cleanup(ddev);
>
> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
> new file mode 100644
> index 0000000..b83646b
> --- /dev/null
> +++ b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
> @@ -0,0 +1,160 @@
> +/*
> + * SimpleDRM firmware framebuffer driver
> + * Copyright (c) 2012-2014 David Herrmann <dh.herrmann@xxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + */
> +
> +/*
> + * fbdev compatibility layer
> + * We provide a basic fbdev device for the same framebuffer that is used for
> + * the pseudo CRTC.
> + */
> +
> +#include <linux/console.h>
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> +#include <linux/mm.h>
> +#include <linux/module.h>

You should not need module.h  here since this file is not doing the
module_init or module_exit or MODULE_ALIAS etc etc.

An empty file with just module.h in it outputs about 750k of goo
from cpp, so it is best avoided wherever not strictly needed.

Thanks,
Paul.
--

> +#include <linux/string.h>
> +#include <linux/fb.h>
> +#include "simpledrm.h"
> +