[v3,1/3] drm: add SimpleDRM driver

Commit Message

Noralf Trønnes Aug. 14, 2016, 4:52 p.m. UTC

The SimpleDRM driver binds to simple-framebuffer devices and provides a
DRM/KMS API. It provides only a single CRTC+encoder+connector combination
plus one initial mode.

Userspace can create dumb-buffers which can be blit into the real
framebuffer similar to UDL. No access to the real framebuffer is allowed
(compared to earlier version of this driver) to avoid security issues.
Furthermore, this way we can support arbitrary modes as long as we have a
conversion-helper.

The driver was originally written by David Herrmann in 2014.
My main contribution is to make use of drm_simple_kms_helper and
rework the probe path to avoid use of the deprecated drm_platform_init()
and drm_driver.{load,unload}().
Additions have also been made for later changes to the Device Tree binding
document, like support for clocks, regulators and having the node under
/chosen. This code was lifted verbatim from simplefb.c.

Cc: [e-mail]
Cc: [e-mail]
Signed-off-by: Noralf Trønnes <[e-mail]>
---

Changes from version 2:
- Remove superfluos module.h includes
- Move includes from header to source files
- Set plane.fb before flushing in pipe update, or else the previous one
  gets flushed
- Added check for vblank event in sdrm_display_pipe_update()

Changes from version 1:
- Move platform_set_drvdata() before drm_dev_register()
- Remove drm_legacy_mmap() call.
- Set mode_config.{min,max}_{width,height} to the actual dimensions
  of the native framebuffer
- Remove plane positioning since it won't work with the simple display pipe,
  meaning sdrm_display_pipe_check() isn't necessary either
- Support the additions to the Device Tree binding document, including
  clocks, regulators and having the node under /chosen

Changes from previous version:
- Remove FB_SIMPLE=n dependency to avoid kconfig recursive error
- Changed module name to match kconfig help text: sdrm -> simpledrm
- Use drm_simple_display_pipe
- Replace deprecated drm_platform_init()
- sdrm_dumb_create(): drm_gem_object_unreference() -> *_unlocked()
- sdrm_dumb_map_offset(): drm_gem_object_lookup() remove drm_device parameter
- sdrm_drm_mmap() changes:
  Remove struct_mutex locking
  Add drm_vma_offset_{lock,unlock}_lookup()
  drm_mmap() -> drm_legacy_mmap()
- dma_buf_begin_cpu_access() doesn't require start and length anymore
- Use drm_cvt_mode() instead of open coding a mode
- Fix format conversion. In the intermediate step, store the 8/6/5 bit color
  value in the upper part of the 16-bit color variable, not the lower.
- Support clips == NULL in sdrm_dirty()
- Set mode_config.preferred_depth
- Attach mode_config.dirty_info_property to connector

 drivers/gpu/drm/Kconfig                      |   2 +
 drivers/gpu/drm/Makefile                     |   1 +
 drivers/gpu/drm/simpledrm/Kconfig            |  19 +
 drivers/gpu/drm/simpledrm/Makefile           |   4 +
 drivers/gpu/drm/simpledrm/simpledrm.h        |  90 +++++
 drivers/gpu/drm/simpledrm/simpledrm_damage.c | 302 +++++++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_drv.c    | 539 +++++++++++++++++++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_gem.c    | 274 ++++++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_kms.c    | 258 +++++++++++++
 9 files changed, 1489 insertions(+)
 create mode 100644 drivers/gpu/drm/simpledrm/Kconfig
 create mode 100644 drivers/gpu/drm/simpledrm/Makefile
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm.h
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_damage.c
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_drv.c
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_gem.c
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_kms.c

--
2.8.2

Comments

Daniel Vetter Aug. 15, 2016, 6:59 a.m. UTC | #1

On Sun, Aug 14, 2016 at 06:52:04PM +0200, Noralf Trønnes wrote:
> The SimpleDRM driver binds to simple-framebuffer devices and provides a
> DRM/KMS API. It provides only a single CRTC+encoder+connector combination
> plus one initial mode.
> 
> Userspace can create dumb-buffers which can be blit into the real
> framebuffer similar to UDL. No access to the real framebuffer is allowed
> (compared to earlier version of this driver) to avoid security issues.
> Furthermore, this way we can support arbitrary modes as long as we have a
> conversion-helper.
> 
> The driver was originally written by David Herrmann in 2014.
> My main contribution is to make use of drm_simple_kms_helper and
> rework the probe path to avoid use of the deprecated drm_platform_init()
> and drm_driver.{load,unload}().
> Additions have also been made for later changes to the Device Tree binding
> document, like support for clocks, regulators and having the node under
> /chosen. This code was lifted verbatim from simplefb.c.
> 
> Cc: [e-mail]
> Cc: [e-mail]
> Signed-off-by: Noralf Trønnes <[e-mail]>

Overall looks nice, but I think we can try to get rid of a bit more
boilerplate still. Comments below.
-Daniel

> ---
> 
> Changes from version 2:
> - Remove superfluos module.h includes
> - Move includes from header to source files
> - Set plane.fb before flushing in pipe update, or else the previous one
>   gets flushed
> - Added check for vblank event in sdrm_display_pipe_update()
> 
> Changes from version 1:
> - Move platform_set_drvdata() before drm_dev_register()
> - Remove drm_legacy_mmap() call.
> - Set mode_config.{min,max}_{width,height} to the actual dimensions
>   of the native framebuffer
> - Remove plane positioning since it won't work with the simple display pipe,
>   meaning sdrm_display_pipe_check() isn't necessary either
> - Support the additions to the Device Tree binding document, including
>   clocks, regulators and having the node under /chosen
> 
> Changes from previous version:
> - Remove FB_SIMPLE=n dependency to avoid kconfig recursive error
> - Changed module name to match kconfig help text: sdrm -> simpledrm
> - Use drm_simple_display_pipe
> - Replace deprecated drm_platform_init()
> - sdrm_dumb_create(): drm_gem_object_unreference() -> *_unlocked()
> - sdrm_dumb_map_offset(): drm_gem_object_lookup() remove drm_device parameter
> - sdrm_drm_mmap() changes:
>   Remove struct_mutex locking
>   Add drm_vma_offset_{lock,unlock}_lookup()
>   drm_mmap() -> drm_legacy_mmap()
> - dma_buf_begin_cpu_access() doesn't require start and length anymore
> - Use drm_cvt_mode() instead of open coding a mode
> - Fix format conversion. In the intermediate step, store the 8/6/5 bit color
>   value in the upper part of the 16-bit color variable, not the lower.
> - Support clips == NULL in sdrm_dirty()
> - Set mode_config.preferred_depth
> - Attach mode_config.dirty_info_property to connector

dirty_info is gone now in drm-misc.

> 
>  drivers/gpu/drm/Kconfig                      |   2 +
>  drivers/gpu/drm/Makefile                     |   1 +
>  drivers/gpu/drm/simpledrm/Kconfig            |  19 +
>  drivers/gpu/drm/simpledrm/Makefile           |   4 +
>  drivers/gpu/drm/simpledrm/simpledrm.h        |  90 +++++
>  drivers/gpu/drm/simpledrm/simpledrm_damage.c | 302 +++++++++++++++
>  drivers/gpu/drm/simpledrm/simpledrm_drv.c    | 539 +++++++++++++++++++++++++++
>  drivers/gpu/drm/simpledrm/simpledrm_gem.c    | 274 ++++++++++++++
>  drivers/gpu/drm/simpledrm/simpledrm_kms.c    | 258 +++++++++++++
>  9 files changed, 1489 insertions(+)
>  create mode 100644 drivers/gpu/drm/simpledrm/Kconfig
>  create mode 100644 drivers/gpu/drm/simpledrm/Makefile
>  create mode 100644 drivers/gpu/drm/simpledrm/simpledrm.h
>  create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_damage.c
>  create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_drv.c
>  create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_gem.c
>  create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_kms.c
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index fc35731..a54cc8d 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -290,3 +290,5 @@ source "drivers/gpu/drm/arc/Kconfig"
>  source "drivers/gpu/drm/hisilicon/Kconfig"
> 
>  source "drivers/gpu/drm/mediatek/Kconfig"
> +
> +source "drivers/gpu/drm/simpledrm/Kconfig"
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index e3dba6f..eba32ad 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -83,3 +83,4 @@ obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
>  obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
>  obj-$(CONFIG_DRM_ARCPGU)+= arc/
>  obj-y			+= hisilicon/
> +obj-$(CONFIG_DRM_SIMPLEDRM) += simpledrm/
> diff --git a/drivers/gpu/drm/simpledrm/Kconfig b/drivers/gpu/drm/simpledrm/Kconfig
> new file mode 100644
> index 0000000..f45b25d
> --- /dev/null
> +++ b/drivers/gpu/drm/simpledrm/Kconfig
> @@ -0,0 +1,19 @@
> +config DRM_SIMPLEDRM
> +	tristate "Simple firmware framebuffer DRM driver"
> +	depends on DRM
> +	select DRM_KMS_HELPER
> +	help
> +	  SimpleDRM can run on all systems with pre-initialized graphics
> +	  hardware. It uses a framebuffer that was initialized during
> +	  firmware boot. No page-flipping, modesetting or other advanced
> +	  features are available. However, other DRM drivers can be loaded
> +	  later and take over from SimpleDRM if they provide real hardware
> +	  support.
> +
> +	  SimpleDRM supports "simple-framebuffer" DeviceTree objects and
> +	  compatible platform framebuffers.
> +
> +	  If unsure, say Y.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called simpledrm.
> diff --git a/drivers/gpu/drm/simpledrm/Makefile b/drivers/gpu/drm/simpledrm/Makefile
> new file mode 100644
> index 0000000..f6a62dc
> --- /dev/null
> +++ b/drivers/gpu/drm/simpledrm/Makefile
> @@ -0,0 +1,4 @@
> +simpledrm-y :=	simpledrm_drv.o simpledrm_kms.o simpledrm_gem.o \
> +		simpledrm_damage.o
> +
> +obj-$(CONFIG_DRM_SIMPLEDRM) := simpledrm.o
> diff --git a/drivers/gpu/drm/simpledrm/simpledrm.h b/drivers/gpu/drm/simpledrm/simpledrm.h
> new file mode 100644
> index 0000000..481acc2
> --- /dev/null
> +++ b/drivers/gpu/drm/simpledrm/simpledrm.h
> @@ -0,0 +1,90 @@
> +/*
> + * SimpleDRM firmware framebuffer driver
> + * Copyright (c) 2012-2014 David Herrmann <[e-mail]>
> + *
> + * 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.
> + */
> +
> +#ifndef SDRM_DRV_H
> +#define SDRM_DRV_H
> +
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_gem.h>
> +#include <drm/drm_simple_kms_helper.h>
> +
> +struct simplefb_format;
> +struct regulator;
> +struct fb_info;
> +struct clk;
> +
> +struct sdrm_device {
> +	struct drm_device *ddev;
> +	struct drm_simple_display_pipe pipe;
> +	struct drm_connector conn;
> +
> +	/* framebuffer information */
> +	const struct simplefb_format *fb_sformat;
> +	u32 fb_format;
> +	u32 fb_width;
> +	u32 fb_height;
> +	u32 fb_stride;
> +	u32 fb_bpp;
> +	unsigned long fb_base;
> +	unsigned long fb_size;
> +	void *fb_map;
> +
> +#if defined CONFIG_OF && defined CONFIG_COMMON_CLK
> +	unsigned int clk_count;
> +	struct clk **clks;
> +#endif
> +#if defined CONFIG_OF && defined CONFIG_REGULATOR
> +	u32 regulator_count;
> +	struct regulator **regulators;
> +#endif
> +};
> +
> +int sdrm_drm_modeset_init(struct sdrm_device *sdrm);
> +int sdrm_drm_mmap(struct file *filp, struct vm_area_struct *vma);
> +
> +int sdrm_dirty(struct drm_framebuffer *fb,
> +	       struct drm_file *file,
> +	       unsigned int flags, unsigned int color,
> +	       struct drm_clip_rect *clips,
> +	       unsigned int num_clips);
> +int sdrm_dirty_all_locked(struct sdrm_device *sdrm);
> +int sdrm_dirty_all_unlocked(struct sdrm_device *sdrm);
> +
> +struct sdrm_gem_object {
> +	struct drm_gem_object base;
> +	struct sg_table *sg;
> +	struct page **pages;
> +	void *vmapping;
> +};
> +
> +#define to_sdrm_bo(x) container_of(x, struct sdrm_gem_object, base)
> +
> +struct sdrm_gem_object *sdrm_gem_alloc_object(struct drm_device *ddev,
> +					      size_t size);
> +struct drm_gem_object *sdrm_gem_prime_import(struct drm_device *ddev,
> +					     struct dma_buf *dma_buf);
> +void sdrm_gem_free_object(struct drm_gem_object *obj);
> +int sdrm_gem_get_pages(struct sdrm_gem_object *obj);
> +
> +int sdrm_dumb_create(struct drm_file *file_priv, struct drm_device *ddev,
> +		     struct drm_mode_create_dumb *arg);
> +int sdrm_dumb_destroy(struct drm_file *file_priv, struct drm_device *ddev,
> +		      uint32_t handle);
> +int sdrm_dumb_map_offset(struct drm_file *file_priv, struct drm_device *ddev,
> +			 uint32_t handle, uint64_t *offset);
> +
> +struct sdrm_framebuffer {
> +	struct drm_framebuffer base;
> +	struct sdrm_gem_object *obj;
> +};
> +
> +#define to_sdrm_fb(x) container_of(x, struct sdrm_framebuffer, base)
> +
> +#endif /* SDRM_DRV_H */
> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_damage.c b/drivers/gpu/drm/simpledrm/simpledrm_damage.c
> new file mode 100644
> index 0000000..ee3d465
> --- /dev/null
> +++ b/drivers/gpu/drm/simpledrm/simpledrm_damage.c
> @@ -0,0 +1,302 @@
> +/*
> + * SimpleDRM firmware framebuffer driver
> + * Copyright (c) 2012-2014 David Herrmann <[e-mail]>
> + *
> + * 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.
> + */
> +
> +#include <asm/unaligned.h>
> +#include <drm/drmP.h>
> +#include <drm/drm_crtc.h>
> +#include <linux/dma-buf.h>
> +#include <linux/kernel.h>
> +#include <linux/string.h>
> +
> +#include "simpledrm.h"
> +
> +static inline void sdrm_put(u8 *dst, u32 four_cc, u16 r, u16 g, u16 b)
> +{
> +	switch (four_cc) {
> +	case DRM_FORMAT_RGB565:
> +		r >>= 11;
> +		g >>= 10;
> +		b >>= 11;
> +		put_unaligned((u16)((r << 11) | (g << 5) | b), (u16 *)dst);
> +		break;
> +	case DRM_FORMAT_XRGB1555:
> +	case DRM_FORMAT_ARGB1555:
> +		r >>= 11;
> +		g >>= 11;
> +		b >>= 11;
> +		put_unaligned((u16)((r << 10) | (g << 5) | b), (u16 *)dst);
> +		break;
> +	case DRM_FORMAT_RGB888:
> +		r >>= 8;
> +		g >>= 8;
> +		b >>= 8;
> +#ifdef __LITTLE_ENDIAN
> +		dst[2] = r;
> +		dst[1] = g;
> +		dst[0] = b;
> +#elif defined(__BIG_ENDIAN)
> +		dst[0] = r;
> +		dst[1] = g;
> +		dst[2] = b;
> +#endif
> +		break;
> +	case DRM_FORMAT_XRGB8888:
> +	case DRM_FORMAT_ARGB8888:
> +		r >>= 8;
> +		g >>= 8;
> +		b >>= 8;
> +		put_unaligned((u32)((r << 16) | (g << 8) | b), (u32 *)dst);
> +		break;
> +	case DRM_FORMAT_ABGR8888:
> +		r >>= 8;
> +		g >>= 8;
> +		b >>= 8;
> +		put_unaligned((u32)((b << 16) | (g << 8) | r), (u32 *)dst);
> +		break;
> +	case DRM_FORMAT_XRGB2101010:
> +	case DRM_FORMAT_ARGB2101010:
> +		r >>= 4;
> +		g >>= 4;
> +		b >>= 4;
> +		put_unaligned((u32)((r << 20) | (g << 10) | b), (u32 *)dst);
> +		break;
> +	}
> +}
> +
> +static void sdrm_blit_from_xrgb8888(const u8 *src, u32 src_stride, u32 src_bpp,
> +				    u8 *dst, u32 dst_stride, u32 dst_bpp,
> +				    u32 dst_four_cc, u32 width, u32 height)
> +{
> +	u32 val, i;
> +
> +	while (height--) {
> +		for (i = 0; i < width; ++i) {
> +			val = get_unaligned((const u32 *)&src[i * src_bpp]);
> +			sdrm_put(&dst[i * dst_bpp], dst_four_cc,
> +				 (val & 0x00ff0000U) >> 8,
> +				 (val & 0x0000ff00U),
> +				 (val & 0x000000ffU) << 8);
> +		}
> +
> +		src += src_stride;
> +		dst += dst_stride;
> +	}
> +}
> +
> +static void sdrm_blit_from_rgb565(const u8 *src, u32 src_stride, u32 src_bpp,
> +				  u8 *dst, u32 dst_stride, u32 dst_bpp,
> +				  u32 dst_four_cc, u32 width, u32 height)
> +{
> +	u32 val, i;
> +
> +	while (height--) {
> +		for (i = 0; i < width; ++i) {
> +			val = get_unaligned((const u16 *)&src[i * src_bpp]);
> +			sdrm_put(&dst[i * dst_bpp], dst_four_cc,
> +				 (val & 0xf800),
> +				 (val & 0x07e0) << 5,
> +				 (val & 0x001f) << 11);
> +		}
> +
> +		src += src_stride;
> +		dst += dst_stride;
> +	}
> +}
> +
> +static void sdrm_blit_lines(const u8 *src, u32 src_stride,
> +			    u8 *dst, u32 dst_stride,
> +			    u32 bpp, u32 width, u32 height)
> +{
> +	u32 len;
> +
> +	len = width * bpp;
> +
> +	while (height--) {
> +		memcpy(dst, src, len);
> +		src += src_stride;
> +		dst += dst_stride;
> +	}
> +}
> +
> +static void sdrm_blit(struct sdrm_framebuffer *sfb, u32 x, u32 y,
> +		      u32 width, u32 height)
> +{
> +	struct drm_framebuffer *fb = &sfb->base;
> +	struct drm_device *ddev = fb->dev;
> +	struct sdrm_device *sdrm = ddev->dev_private;
> +	u32 src_bpp, dst_bpp, x2, y2;
> +	u8 *src, *dst;
> +
> +	/* already unmapped; ongoing handover? */
> +	if (!sdrm->fb_map)
> +		return;
> +
> +	/* empty dirty-region, nothing to do */
> +	if (!width || !height)
> +		return;
> +	if (x >= fb->width || y >= fb->height)
> +		return;
> +
> +	/* sanity checks */
> +	if (x + width < x)
> +		width = fb->width - x;
> +	if (y + height < y)
> +		height = fb->height - y;
> +
> +	/* get intersection of dirty region and scan-out region */
> +	x2 = min(x + width, sdrm->fb_width);
> +	y2 = min(y + height, sdrm->fb_height);
> +	if (x2 <= x || y2 <= y)
> +		return;
> +	width = x2 - x;
> +	height = y2 - y;
> +
> +	/* get buffer offsets */
> +	src = sfb->obj->vmapping;
> +	dst = sdrm->fb_map;
> +
> +	/* bo is guaranteed to be big enough; size checks not needed */
> +	src_bpp = (fb->bits_per_pixel + 7) / 8;
> +	src += fb->offsets[0] + y * fb->pitches[0] + x * src_bpp;
> +
> +	dst_bpp = (sdrm->fb_bpp + 7) / 8;
> +	dst += y * sdrm->fb_stride + x * dst_bpp;
> +
> +	/* if formats are identical, do a line-by-line copy.. */
> +	if (fb->pixel_format == sdrm->fb_format) {
> +		sdrm_blit_lines(src, fb->pitches[0],
> +				dst, sdrm->fb_stride,
> +				src_bpp, width, height);
> +		return;
> +	}
> +
> +	/* ..otherwise call slow blit-function */
> +	switch (fb->pixel_format) {
> +	case DRM_FORMAT_ARGB8888:
> +		/* fallthrough */
> +	case DRM_FORMAT_XRGB8888:
> +		sdrm_blit_from_xrgb8888(src, fb->pitches[0], src_bpp,
> +					dst, sdrm->fb_stride, dst_bpp,
> +					sdrm->fb_format, width, height);
> +		break;
> +	case DRM_FORMAT_RGB565:
> +		sdrm_blit_from_rgb565(src, fb->pitches[0], src_bpp,
> +				      dst, sdrm->fb_stride, dst_bpp,
> +				      sdrm->fb_format, width, height);
> +		break;
> +	}
> +}
> +
> +static int sdrm_begin_access(struct sdrm_framebuffer *sfb)
> +{
> +	int r;
> +
> +	r = sdrm_gem_get_pages(sfb->obj);
> +	if (r)
> +		return r;
> +
> +	if (!sfb->obj->base.import_attach)
> +		return 0;
> +
> +	return dma_buf_begin_cpu_access(sfb->obj->base.import_attach->dmabuf,
> +					DMA_FROM_DEVICE);
> +}
> +
> +static void sdrm_end_access(struct sdrm_framebuffer *sfb)
> +{
> +	if (!sfb->obj->base.import_attach)
> +		return;
> +
> +	dma_buf_end_cpu_access(sfb->obj->base.import_attach->dmabuf,
> +			       DMA_FROM_DEVICE);
> +}
> +
> +int sdrm_dirty(struct drm_framebuffer *fb,
> +	       struct drm_file *file,
> +	       unsigned int flags, unsigned int color,
> +	       struct drm_clip_rect *clips,
> +	       unsigned int num_clips)
> +{
> +	struct sdrm_framebuffer *sfb = to_sdrm_fb(fb);
> +	struct drm_device *ddev = fb->dev;
> +	struct sdrm_device *sdrm = ddev->dev_private;
> +	struct drm_clip_rect full_clip;
> +	unsigned int i;
> +	int r;
> +
> +	if (!clips || !num_clips) {
> +		full_clip.x1 = 0;
> +		full_clip.x2 = fb->width;
> +		full_clip.y1 = 0;
> +		full_clip.y2 = fb->height;
> +		clips = &full_clip;
> +		num_clips = 1;
> +	}
> +
> +	drm_modeset_lock_all(ddev);
> +
> +	if (sdrm->pipe.plane.fb != fb) {
> +		r = 0;
> +		goto unlock;
> +	}
> +
> +	r = sdrm_begin_access(sfb);
> +	if (r)
> +		goto unlock;
> +
> +	for (i = 0; i < num_clips; i++) {
> +		if (clips[i].x2 <= clips[i].x1 ||
> +		    clips[i].y2 <= clips[i].y1)
> +			continue;
> +
> +		sdrm_blit(sfb, clips[i].x1, clips[i].y1,
> +			  clips[i].x2 - clips[i].x1,
> +			  clips[i].y2 - clips[i].y1);
> +	}
> +
> +	sdrm_end_access(sfb);
> +
> +unlock:
> +	drm_modeset_unlock_all(ddev);
> +	return 0;
> +}
> +
> +int sdrm_dirty_all_locked(struct sdrm_device *sdrm)
> +{
> +	struct drm_framebuffer *fb;
> +	struct sdrm_framebuffer *sfb;
> +	int r;
> +
> +	fb = sdrm->pipe.plane.fb;
> +	if (!fb)
> +		return 0;
> +
> +	sfb = to_sdrm_fb(fb);
> +	r = sdrm_begin_access(sfb);
> +	if (r)
> +		return r;
> +
> +	sdrm_blit(sfb, 0, 0, fb->width, fb->height);
> +
> +	sdrm_end_access(sfb);
> +
> +	return 0;
> +}
> +
> +int sdrm_dirty_all_unlocked(struct sdrm_device *sdrm)
> +{
> +	int r;
> +
> +	drm_modeset_lock_all(sdrm->ddev);
> +	r = sdrm_dirty_all_locked(sdrm);
> +	drm_modeset_unlock_all(sdrm->ddev);
> +
> +	return r;
> +}

Seems unused?

> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_drv.c b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
> new file mode 100644
> index 0000000..e5b6f75
> --- /dev/null
> +++ b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
> @@ -0,0 +1,539 @@
> +/*
> + * SimpleDRM firmware framebuffer driver
> + * Copyright (c) 2012-2014 David Herrmann <[e-mail]>
> + *
> + * 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.
> + */
> +
> +#include <drm/drmP.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/errno.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/mm.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_data/simplefb.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/string.h>
> +
> +#include "simpledrm.h"
> +
> +static const struct file_operations sdrm_drm_fops = {
> +	.owner = THIS_MODULE,
> +	.open = drm_open,
> +	.mmap = sdrm_drm_mmap,
> +	.poll = drm_poll,
> +	.read = drm_read,
> +	.unlocked_ioctl = drm_ioctl,
> +	.release = drm_release,
> +#ifdef CONFIG_COMPAT
> +	.compat_ioctl = drm_compat_ioctl,
> +#endif
> +	.llseek = noop_llseek,
> +};
> +
> +static struct drm_driver sdrm_drm_driver = {
> +	.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
> +			   DRIVER_ATOMIC,
> +	.fops = &sdrm_drm_fops,
> +
> +	.gem_free_object = sdrm_gem_free_object,
> +	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> +	.gem_prime_import = sdrm_gem_prime_import,
> +
> +	.dumb_create = sdrm_dumb_create,
> +	.dumb_map_offset = sdrm_dumb_map_offset,
> +	.dumb_destroy = sdrm_dumb_destroy,
> +
> +	.name = "simpledrm",
> +	.desc = "Simple firmware framebuffer DRM driver",
> +	.date = "20130601",
> +	.major = 0,
> +	.minor = 0,
> +	.patchlevel = 1,
> +};
> +
> +#if defined CONFIG_OF && defined CONFIG_COMMON_CLK
> +/*
> + * Clock handling code.
> + *
> + * Here we handle the clocks property of our "simple-framebuffer" dt node.
> + * This is necessary so that we can make sure that any clocks needed by
> + * the display engine that the bootloader set up for us (and for which it
> + * provided a simplefb dt node), stay up, for the life of the simplefb
> + * driver.
> + *
> + * When the driver unloads, we cleanly disable, and then release the clocks.
> + *
> + * We only complain about errors here, no action is taken as the most likely
> + * error can only happen due to a mismatch between the bootloader which set
> + * up simplefb, and the clock definitions in the device tree. Chances are
> + * that there are no adverse effects, and if there are, a clean teardown of
> + * the fb probe will not help us much either. So just complain and carry on,
> + * and hope that the user actually gets a working fb at the end of things.
> + */
> +static int sdrm_clocks_init(struct sdrm_device *sdrm,
> +			    struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct clk *clock;
> +	int i, ret;
> +
> +	if (dev_get_platdata(&pdev->dev) || !np)
> +		return 0;
> +
> +	sdrm->clk_count = of_clk_get_parent_count(np);
> +	if (!sdrm->clk_count)
> +		return 0;
> +
> +	sdrm->clks = kcalloc(sdrm->clk_count, sizeof(struct clk *), GFP_KERNEL);
> +	if (!sdrm->clks)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < sdrm->clk_count; i++) {
> +		clock = of_clk_get(np, i);
> +		if (IS_ERR(clock)) {
> +			if (PTR_ERR(clock) == -EPROBE_DEFER) {
> +				while (--i >= 0) {
> +					if (sdrm->clks[i])
> +						clk_put(sdrm->clks[i]);
> +				}
> +				kfree(sdrm->clks);
> +				return -EPROBE_DEFER;
> +			}
> +			dev_err(&pdev->dev, "%s: clock %d not found: %ld\n",
> +				__func__, i, PTR_ERR(clock));
> +			continue;
> +		}
> +		sdrm->clks[i] = clock;
> +	}
> +
> +	for (i = 0; i < sdrm->clk_count; i++) {
> +		if (sdrm->clks[i]) {
> +			ret = clk_prepare_enable(sdrm->clks[i]);
> +			if (ret) {
> +				dev_err(&pdev->dev,
> +					"%s: failed to enable clock %d: %d\n",
> +					__func__, i, ret);
> +				clk_put(sdrm->clks[i]);
> +				sdrm->clks[i] = NULL;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static void sdrm_clocks_destroy(struct sdrm_device *sdrm)
> +{
> +	int i;
> +
> +	if (!sdrm->clks)
> +		return;
> +
> +	for (i = 0; i < sdrm->clk_count; i++) {
> +		if (sdrm->clks[i]) {
> +			clk_disable_unprepare(sdrm->clks[i]);
> +			clk_put(sdrm->clks[i]);
> +		}
> +	}
> +
> +	kfree(sdrm->clks);
> +}
> +#else
> +static int sdrm_clocks_init(struct sdrm_device *sdrm,
> +			    struct platform_device *pdev)
> +{
> +	return 0;
> +}
> +
> +static void sdrm_clocks_destroy(struct sdrm_device *sdrm)
> +{
> +}
> +#endif
> +
> +#if defined CONFIG_OF && defined CONFIG_REGULATOR
> +
> +#define SUPPLY_SUFFIX "-supply"
> +
> +/*
> + * Regulator handling code.
> + *
> + * Here we handle the num-supplies and vin*-supply properties of our
> + * "simple-framebuffer" dt node. This is necessary so that we can make sure
> + * that any regulators needed by the display hardware that the bootloader
> + * set up for us (and for which it provided a simplefb dt node), stay up,
> + * for the life of the simplefb driver.
> + *
> + * When the driver unloads, we cleanly disable, and then release the
> + * regulators.
> + *
> + * We only complain about errors here, no action is taken as the most likely
> + * error can only happen due to a mismatch between the bootloader which set
> + * up simplefb, and the regulator definitions in the device tree. Chances are
> + * that there are no adverse effects, and if there are, a clean teardown of
> + * the fb probe will not help us much either. So just complain and carry on,
> + * and hope that the user actually gets a working fb at the end of things.
> + */
> +static int sdrm_regulators_init(struct sdrm_device *sdrm,
> +				struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct regulator *regulator;
> +	int count = 0, i = 0, ret;
> +	struct property *prop;
> +	const char *p;
> +
> +	if (dev_get_platdata(&pdev->dev) || !np)
> +		return 0;
> +
> +	/* Count the number of regulator supplies */
> +	for_each_property_of_node(np, prop) {
> +		p = strstr(prop->name, SUPPLY_SUFFIX);
> +		if (p && p != prop->name)
> +			count++;
> +	}
> +
> +	if (!count)
> +		return 0;
> +
> +	sdrm->regulators = devm_kcalloc(&pdev->dev, count,
> +					sizeof(struct regulator *),
> +					GFP_KERNEL);
> +	if (!sdrm->regulators)
> +		return -ENOMEM;
> +
> +	/* Get all the regulators */
> +	for_each_property_of_node(np, prop) {
> +		char name[32]; /* 32 is max size of property name */
> +
> +		p = strstr(prop->name, SUPPLY_SUFFIX);
> +		if (!p || p == prop->name)
> +			continue;
> +
> +		strlcpy(name, prop->name,
> +			strlen(prop->name) - strlen(SUPPLY_SUFFIX) + 1);
> +		regulator = devm_regulator_get_optional(&pdev->dev, name);
> +		if (IS_ERR(regulator)) {
> +			if (PTR_ERR(regulator) == -EPROBE_DEFER)
> +				return -EPROBE_DEFER;
> +			dev_err(&pdev->dev, "regulator %s not found: %ld\n",
> +				name, PTR_ERR(regulator));
> +			continue;
> +		}
> +		sdrm->regulators[i++] = regulator;
> +	}
> +	sdrm->regulator_count = i;
> +
> +	/* Enable all the regulators */
> +	for (i = 0; i < sdrm->regulator_count; i++) {
> +		ret = regulator_enable(sdrm->regulators[i]);
> +		if (ret) {
> +			dev_err(&pdev->dev,
> +				"failed to enable regulator %d: %d\n",
> +				i, ret);
> +			devm_regulator_put(sdrm->regulators[i]);
> +			sdrm->regulators[i] = NULL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static void sdrm_regulators_destroy(struct sdrm_device *sdrm)
> +{
> +	int i;
> +
> +	if (!sdrm->regulators)
> +		return;
> +
> +	for (i = 0; i < sdrm->regulator_count; i++)
> +		if (sdrm->regulators[i])
> +			regulator_disable(sdrm->regulators[i]);
> +}
> +#else
> +static int sdrm_regulators_init(struct sdrm_device *sdrm,
> +				struct platform_device *pdev)
> +{
> +	return 0;
> +}
> +
> +static void sdrm_regulators_destroy(struct sdrm_device *sdrm)
> +{
> +}
> +#endif
> +
> +static int parse_dt(struct platform_device *pdev,
> +		    struct simplefb_platform_data *mode)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	const char *format;
> +	int ret;
> +
> +	if (!np)
> +		return -ENODEV;
> +
> +	ret = of_property_read_u32(np, "width", &mode->width);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Can't parse width property\n");
> +		return ret;
> +	}
> +
> +	ret = of_property_read_u32(np, "height", &mode->height);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Can't parse height property\n");
> +		return ret;
> +	}
> +
> +	ret = of_property_read_u32(np, "stride", &mode->stride);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Can't parse stride property\n");
> +		return ret;
> +	}
> +
> +	ret = of_property_read_string(np, "format", &format);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Can't parse format property\n");
> +		return ret;
> +	}
> +	mode->format = format;
> +
> +	return 0;
> +}
> +
> +static struct simplefb_format simplefb_formats[] = SIMPLEFB_FORMATS;
> +
> +int sdrm_pdev_init(struct sdrm_device *sdrm)
> +{
> +	struct platform_device *pdev = sdrm->ddev->platformdev;
> +	struct simplefb_platform_data *mode = pdev->dev.platform_data;
> +	struct simplefb_platform_data pmode;
> +	struct resource *mem;
> +	unsigned int depth;
> +	int ret, i, bpp;
> +
> +	if (!mode) {
> +		mode = &pmode;
> +		ret = parse_dt(pdev, mode);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!mem) {
> +		dev_err(sdrm->ddev->dev, "No memory resource\n");
> +		return -ENODEV;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(simplefb_formats); ++i) {
> +		if (strcmp(mode->format, simplefb_formats[i].name))
> +			continue;
> +
> +		sdrm->fb_sformat = &simplefb_formats[i];
> +		sdrm->fb_format = simplefb_formats[i].fourcc;
> +		sdrm->fb_width = mode->width;
> +		sdrm->fb_height = mode->height;
> +		sdrm->fb_stride = mode->stride;
> +		sdrm->fb_base = mem->start;
> +		sdrm->fb_size = resource_size(mem);
> +		break;
> +	}
> +
> +	if (i >= ARRAY_SIZE(simplefb_formats)) {
> +		dev_err(sdrm->ddev->dev, "Unknown format %s\n", mode->format);
> +		return -ENODEV;
> +	}
> +
> +	switch (sdrm->fb_format) {
> +	case DRM_FORMAT_RGB565:
> +	case DRM_FORMAT_XRGB1555:
> +	case DRM_FORMAT_ARGB1555:
> +	case DRM_FORMAT_RGB888:
> +	case DRM_FORMAT_XRGB8888:
> +	case DRM_FORMAT_ARGB8888:
> +	case DRM_FORMAT_ABGR8888:
> +	case DRM_FORMAT_XRGB2101010:
> +	case DRM_FORMAT_ARGB2101010:
> +		/*
> +		 * You must adjust sdrm_put() whenever you add a new format
> +		 * here, otherwise, blitting operations will not work.
> +		 * Furthermore, include/linux/platform_data/simplefb.h needs
> +		 * to be adjusted so the platform-device actually allows this
> +		 * format.
> +		 */
> +		break;
> +	default:
> +		dev_err(sdrm->ddev->dev, "Unsupported format %s\n",
> +			mode->format);
> +		return -ENODEV;
> +	}
> +
> +	drm_fb_get_bpp_depth(sdrm->fb_format, &depth, &bpp);
> +	if (!bpp) {
> +		dev_err(sdrm->ddev->dev, "Unknown format %s\n", mode->format);
> +		return -ENODEV;
> +	}
> +
> +	if (sdrm->fb_size < sdrm->fb_stride * sdrm->fb_height) {
> +		dev_err(sdrm->ddev->dev, "FB too small\n");
> +		return -ENODEV;
> +	} else if ((bpp + 7) / 8 * sdrm->fb_width > sdrm->fb_stride) {
> +		dev_err(sdrm->ddev->dev, "Invalid stride\n");
> +		return -ENODEV;
> +	}
> +
> +	sdrm->fb_bpp = bpp;
> +
> +	sdrm->fb_map = ioremap_wc(sdrm->fb_base, sdrm->fb_size);
> +	if (!sdrm->fb_map) {
> +		dev_err(sdrm->ddev->dev, "cannot remap VMEM\n");
> +		return -EIO;
> +	}
> +
> +	DRM_DEBUG_KMS("format: %s\n", drm_get_format_name(sdrm->fb_format));
> +
> +	return 0;
> +}
> +
> +void sdrm_pdev_destroy(struct sdrm_device *sdrm)
> +{
> +	if (sdrm->fb_map) {
> +		iounmap(sdrm->fb_map);
> +		sdrm->fb_map = NULL;
> +	}
> +}
> +
> +static int sdrm_simplefb_probe(struct platform_device *pdev)
> +{
> +	struct sdrm_device *sdrm;
> +	struct drm_device *ddev;
> +	int ret;
> +
> +	ddev = drm_dev_alloc(&sdrm_drm_driver, &pdev->dev);
> +	if (!ddev)
> +		return -ENOMEM;
> +
> +	sdrm = kzalloc(sizeof(*sdrm), GFP_KERNEL);
> +	if (!sdrm)
> +		goto err_free;
> +
> +	ddev->platformdev = pdev;

I'm kinda trying to deprecate drm_device->platformdev. If you want it, pls
store it in sdrm_device. Or pass it around explicitly.
-Daniel

> +	ddev->dev_private = sdrm;
> +	sdrm->ddev = ddev;
> +
> +	ret = sdrm_pdev_init(sdrm);
> +	if (ret)
> +		goto err_free;
> +
> +	ret = sdrm_drm_modeset_init(sdrm);
> +	if (ret)
> +		goto err_destroy;
> +
> +	ret = sdrm_clocks_init(sdrm, pdev);
> +	if (ret < 0)
> +		goto err_cleanup;
> +
> +	ret = sdrm_regulators_init(sdrm, pdev);
> +	if (ret < 0)
> +		goto err_clocks;
> +
> +	platform_set_drvdata(pdev, ddev);
> +	ret = drm_dev_register(ddev, 0);
> +	if (ret)
> +		goto err_regulators;
> +
> +	DRM_INFO("Initialized %s on minor %d\n", ddev->driver->name,
> +		 ddev->primary->index);
> +
> +	return 0;
> +
> +err_regulators:
> +	sdrm_regulators_destroy(sdrm);
> +err_clocks:
> +	sdrm_clocks_destroy(sdrm);
> +err_cleanup:
> +	drm_mode_config_cleanup(ddev);
> +err_destroy:
> +	sdrm_pdev_destroy(sdrm);
> +err_free:
> +	drm_dev_unref(ddev);
> +	kfree(sdrm);
> +
> +	return ret;
> +}
> +
> +static int sdrm_simplefb_remove(struct platform_device *pdev)
> +{
> +	struct drm_device *ddev = platform_get_drvdata(pdev);
> +	struct sdrm_device *sdrm = ddev->dev_private;
> +
> +	drm_dev_unregister(ddev);
> +	drm_mode_config_cleanup(ddev);
> +
> +	/* protect fb_map removal against sdrm_blit() */
> +	drm_modeset_lock_all(ddev);
> +	sdrm_pdev_destroy(sdrm);
> +	drm_modeset_unlock_all(ddev);
> +
> +	sdrm_regulators_destroy(sdrm);
> +	sdrm_clocks_destroy(sdrm);
> +
> +	drm_dev_unref(ddev);
> +	kfree(sdrm);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id simplefb_of_match[] = {
> +	{ .compatible = "simple-framebuffer", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, simplefb_of_match);
> +
> +static struct platform_driver sdrm_simplefb_driver = {
> +	.probe = sdrm_simplefb_probe,
> +	.remove = sdrm_simplefb_remove,
> +	.driver = {
> +		.name = "simple-framebuffer",
> +		.mod_name = KBUILD_MODNAME,
> +		.owner = THIS_MODULE,
> +		.of_match_table = simplefb_of_match,
> +	},
> +};
> +
> +static int __init sdrm_init(void)
> +{
> +	int ret;
> +	struct device_node *np;
> +
> +	ret = platform_driver_register(&sdrm_simplefb_driver);
> +	if (ret)
> +		return ret;
> +
> +	if (IS_ENABLED(CONFIG_OF_ADDRESS) && of_chosen) {
> +		for_each_child_of_node(of_chosen, np) {
> +			if (of_device_is_compatible(np, "simple-framebuffer"))
> +				of_platform_device_create(np, NULL, NULL);
> +		}
> +	}
> +
> +	return 0;
> +}
> +module_init(sdrm_init);
> +
> +static void __exit sdrm_exit(void)
> +{
> +	platform_driver_unregister(&sdrm_simplefb_driver);
> +}
> +module_exit(sdrm_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("David Herrmann <[e-mail]>");
> +MODULE_DESCRIPTION("Simple firmware framebuffer DRM driver");
> +MODULE_ALIAS("platform:simple-framebuffer");
> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_gem.c b/drivers/gpu/drm/simpledrm/simpledrm_gem.c
> new file mode 100644
> index 0000000..81bd7c5
> --- /dev/null
> +++ b/drivers/gpu/drm/simpledrm/simpledrm_gem.c
> @@ -0,0 +1,274 @@
> +/*
> + * SimpleDRM firmware framebuffer driver
> + * Copyright (c) 2012-2014 David Herrmann <[e-mail]>
> + *
> + * 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.
> + */
> +
> +#include <drm/drmP.h>
> +#include <linux/dma-buf.h>
> +#include <linux/errno.h>
> +#include <linux/mm.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +#include <linux/vmalloc.h>
> +
> +#include "simpledrm.h"
> +
> +int sdrm_gem_get_pages(struct sdrm_gem_object *obj)
> +{
> +	size_t num, i;
> +
> +	if (obj->vmapping)
> +		return 0;
> +
> +	if (obj->base.import_attach) {
> +		obj->vmapping = dma_buf_vmap(obj->base.import_attach->dmabuf);
> +		return !obj->vmapping ? -ENOMEM : 0;
> +	}
> +
> +	num = obj->base.size >> PAGE_SHIFT;
> +	obj->pages = drm_malloc_ab(num, sizeof(*obj->pages));
> +	if (!obj->pages)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < num; ++i) {
> +		obj->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO);
> +		if (!obj->pages[i])
> +			goto error;
> +	}
> +
> +	obj->vmapping = vmap(obj->pages, num, 0, PAGE_KERNEL);
> +	if (!obj->vmapping)
> +		goto error;
> +
> +	return 0;
> +
> +error:
> +	while (i > 0)
> +		__free_pages(obj->pages[--i], 0);
> +
> +	drm_free_large(obj->pages);
> +	obj->pages = NULL;
> +	return -ENOMEM;
> +}

There's a helper for this in drm_gem.c. Same for put_pages.
> +
> +static void sdrm_gem_put_pages(struct sdrm_gem_object *obj)
> +{
> +	size_t num, i;
> +
> +	if (!obj->vmapping)
> +		return;
> +
> +	if (obj->base.import_attach) {
> +		dma_buf_vunmap(obj->base.import_attach->dmabuf, obj->vmapping);
> +		obj->vmapping = NULL;
> +		return;
> +	}
> +
> +	vunmap(obj->vmapping);
> +	obj->vmapping = NULL;
> +
> +	num = obj->base.size >> PAGE_SHIFT;
> +	for (i = 0; i < num; ++i)
> +		__free_pages(obj->pages[i], 0);
> +
> +	drm_free_large(obj->pages);
> +	obj->pages = NULL;
> +}
> +
> +struct sdrm_gem_object *sdrm_gem_alloc_object(struct drm_device *ddev,
> +					      size_t size)
> +{
> +	struct sdrm_gem_object *obj;
> +
> +	WARN_ON(!size || (size & ~PAGE_MASK) != 0);
> +
> +	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
> +	if (!obj)
> +		return NULL;
> +
> +	drm_gem_private_object_init(ddev, &obj->base, size);
> +	return obj;
> +}
> +
> +void sdrm_gem_free_object(struct drm_gem_object *gobj)
> +{
> +	struct sdrm_gem_object *obj = to_sdrm_bo(gobj);
> +	struct drm_device *ddev = gobj->dev;
> +
> +	if (obj->pages) {
> +		/* kill all user-space mappings */
> +		drm_vma_node_unmap(&gobj->vma_node,
> +				   ddev->anon_inode->i_mapping);
> +		sdrm_gem_put_pages(obj);
> +	}
> +
> +	if (gobj->import_attach)
> +		drm_prime_gem_destroy(gobj, obj->sg);
> +
> +	drm_gem_free_mmap_offset(gobj);
> +	drm_gem_object_release(gobj);
> +	kfree(obj);
> +}
> +
> +int sdrm_dumb_create(struct drm_file *dfile, struct drm_device *ddev,
> +		     struct drm_mode_create_dumb *args)
> +{
> +	struct sdrm_gem_object *obj;
> +	int r;
> +
> +	if (args->flags)
> +		return -EINVAL;
> +
> +	/* overflow checks are done by DRM core */
> +	args->pitch = (args->bpp + 7) / 8 * args->width;
> +	args->size = PAGE_ALIGN(args->pitch * args->height);
> +
> +	obj = sdrm_gem_alloc_object(ddev, args->size);
> +	if (!obj)
> +		return -ENOMEM;
> +
> +	r = drm_gem_handle_create(dfile, &obj->base, &args->handle);
> +	if (r) {
> +		drm_gem_object_unreference_unlocked(&obj->base);
> +		return r;
> +	}
> +
> +	/* handle owns a reference */
> +	drm_gem_object_unreference_unlocked(&obj->base);
> +	return 0;
> +}
> +
> +int sdrm_dumb_destroy(struct drm_file *dfile, struct drm_device *ddev,
> +		      uint32_t handle)
> +{
> +	return drm_gem_handle_delete(dfile, handle);
> +}

I wonder whether some dumb+gem helpers wouldn't be useful to roll out. At
least drm_dumb_gem_destroy.c would be pretty simple.

> +
> +int sdrm_dumb_map_offset(struct drm_file *dfile, struct drm_device *ddev,
> +			 uint32_t handle, uint64_t *offset)
> +{
> +	struct drm_gem_object *gobj;
> +	int r;
> +
> +	mutex_lock(&ddev->struct_mutex);

There's still some struct mutex here.

> +
> +	gobj = drm_gem_object_lookup(dfile, handle);
> +	if (!gobj) {
> +		r = -ENOENT;
> +		goto out_unlock;
> +	}
> +
> +	r = drm_gem_create_mmap_offset(gobj);
> +	if (r)
> +		goto out_unref;
> +
> +	*offset = drm_vma_node_offset_addr(&gobj->vma_node);
> +
> +out_unref:
> +	drm_gem_object_unreference(gobj);
> +out_unlock:
> +	mutex_unlock(&ddev->struct_mutex);
> +	return r;
> +}
> +
> +int sdrm_drm_mmap(struct file *filp, struct vm_area_struct *vma)
> +{
> +	struct drm_file *priv = filp->private_data;
> +	struct drm_device *dev = priv->minor->dev;
> +	struct drm_vma_offset_node *node;
> +	struct drm_gem_object *gobj;
> +	struct sdrm_gem_object *obj;
> +	size_t size, i, num;
> +	int r;
> +
> +	if (drm_device_is_unplugged(dev))
> +		return -ENODEV;
> +
> +	drm_vma_offset_lock_lookup(dev->vma_offset_manager);
> +	node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
> +						  vma->vm_pgoff,
> +						  vma_pages(vma));
> +	drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
> +
> +	if (!drm_vma_node_is_allowed(node, filp))
> +		return -EACCES;
> +
> +	gobj = container_of(node, struct drm_gem_object, vma_node);
> +	obj = to_sdrm_bo(gobj);
> +	size = drm_vma_node_size(node) << PAGE_SHIFT;
> +	if (size < vma->vm_end - vma->vm_start)
> +		return r;
> +
> +	r = sdrm_gem_get_pages(obj);
> +	if (r < 0)
> +		return r;
> +
> +	/* prevent dmabuf-imported mmap to user-space */
> +	if (!obj->pages)
> +		return -EACCES;
> +
> +	vma->vm_flags |= VM_DONTEXPAND;
> +	vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
> +
> +	num = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> +	for (i = 0; i < num; ++i) {
> +		r = vm_insert_page(vma, vma->vm_start + i * PAGE_SIZE,
> +				   obj->pages[i]);
> +		if (r < 0) {
> +			if (i > 0)
> +				zap_vma_ptes(vma, vma->vm_start, i * PAGE_SIZE);
> +			return r;
> +		}
> +	}
> +
> +	return 0;
> +}

Why can't we just redirect to the underlying shmem mmap here (after
argument checking)?

> +
> +struct drm_gem_object *sdrm_gem_prime_import(struct drm_device *ddev,
> +					     struct dma_buf *dma_buf)
> +{
> +	struct dma_buf_attachment *attach;
> +	struct sdrm_gem_object *obj;
> +	struct sg_table *sg;
> +	int ret;
> +
> +	/* need to attach */
> +	attach = dma_buf_attach(dma_buf, ddev->dev);
> +	if (IS_ERR(attach))
> +		return ERR_CAST(attach);
> +
> +	get_dma_buf(dma_buf);
> +
> +	sg = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
> +	if (IS_ERR(sg)) {
> +		ret = PTR_ERR(sg);
> +		goto fail_detach;
> +	}
> +
> +	/*
> +	 * dma_buf_vmap() gives us a page-aligned mapping, so lets bump the
> +	 * size of the dma-buf to the next page-boundary
> +	 */

dma_buf should be page-aligned already. Not sure we enforce that, but it's
in practice a given.

> +	obj = sdrm_gem_alloc_object(ddev, PAGE_ALIGN(dma_buf->size));
> +	if (!obj) {
> +		ret = -ENOMEM;
> +		goto fail_unmap;
> +	}
> +
> +	obj->sg = sg;
> +	obj->base.import_attach = attach;
> +
> +	return &obj->base;
> +
> +fail_unmap:
> +	dma_buf_unmap_attachment(attach, sg, DMA_BIDIRECTIONAL);
> +fail_detach:
> +	dma_buf_detach(dma_buf, attach);
> +	dma_buf_put(dma_buf);
> +	return ERR_PTR(ret);
> +}
> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_kms.c b/drivers/gpu/drm/simpledrm/simpledrm_kms.c
> new file mode 100644
> index 0000000..00e50d9
> --- /dev/null
> +++ b/drivers/gpu/drm/simpledrm/simpledrm_kms.c
> @@ -0,0 +1,258 @@
> +/*
> + * SimpleDRM firmware framebuffer driver
> + * Copyright (c) 2012-2014 David Herrmann <[e-mail]>
> + *
> + * 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.
> + */
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_gem.h>
> +#include <drm/drm_simple_kms_helper.h>
> +#include <linux/slab.h>
> +
> +#include "simpledrm.h"
> +
> +static const uint32_t sdrm_formats[] = {
> +	DRM_FORMAT_RGB565,
> +	DRM_FORMAT_ARGB8888,
> +	DRM_FORMAT_XRGB8888,
> +};
> +
> +static int sdrm_conn_get_modes(struct drm_connector *conn)
> +{
> +	struct sdrm_device *sdrm = conn->dev->dev_private;
> +	struct drm_display_mode *mode;
> +
> +	mode = drm_cvt_mode(sdrm->ddev, sdrm->fb_width, sdrm->fb_height,
> +			    60, false, false, false);
> +	if (!mode)
> +		return 0;
> +
> +	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> +	drm_mode_set_name(mode);
> +	drm_mode_probed_add(conn, mode);
> +
> +	return 1;
> +}
> +
> +static const struct drm_connector_helper_funcs sdrm_conn_hfuncs = {
> +	.get_modes = sdrm_conn_get_modes,
> +	.best_encoder = drm_atomic_helper_best_encoder,
> +};
> +
> +static enum drm_connector_status sdrm_conn_detect(struct drm_connector *conn,
> +						  bool force)
> +{
> +	/*
> +	 * We simulate an always connected monitor. simple-fb doesn't
> +	 * provide any way to detect whether the connector is active. Hence,
> +	 * signal DRM core that it is always connected.
> +	 */
> +
> +	return connector_status_connected;
> +}
> +
> +static const struct drm_connector_funcs sdrm_conn_ops = {
> +	.dpms = drm_atomic_helper_connector_dpms,
> +	.reset = drm_atomic_helper_connector_reset,
> +	.detect = sdrm_conn_detect,
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.destroy = drm_connector_cleanup,
> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static inline struct sdrm_device *
> +pipe_to_sdrm(struct drm_simple_display_pipe *pipe)
> +{
> +	return container_of(pipe, struct sdrm_device, pipe);
> +}
> +
> +static void sdrm_crtc_send_vblank_event(struct drm_crtc *crtc)
> +{
> +	if (crtc->state && crtc->state->event) {
> +		spin_lock_irq(&crtc->dev->event_lock);
> +		drm_crtc_send_vblank_event(crtc, crtc->state->event);
> +		spin_unlock_irq(&crtc->dev->event_lock);
> +		crtc->state->event = NULL;
> +	}
> +}
> +
> +void sdrm_display_pipe_update(struct drm_simple_display_pipe *pipe,
> +			      struct drm_plane_state *plane_state)
> +{
> +	struct drm_framebuffer *fb = pipe->plane.state->fb;
> +	struct sdrm_device *sdrm = pipe_to_sdrm(pipe);
> +
> +	sdrm_crtc_send_vblank_event(&pipe->crtc);
> +
> +	if (fb) {
> +		pipe->plane.fb = fb;
> +		sdrm_dirty_all_locked(sdrm);
> +	}
> +}
> +
> +static void sdrm_display_pipe_enable(struct drm_simple_display_pipe *pipe,
> +				     struct drm_crtc_state *crtc_state)
> +{
> +	sdrm_crtc_send_vblank_event(&pipe->crtc);
> +}
> +
> +static void sdrm_display_pipe_disable(struct drm_simple_display_pipe *pipe)
> +{
> +	sdrm_crtc_send_vblank_event(&pipe->crtc);
> +}
> +
> +static const struct drm_simple_display_pipe_funcs sdrm_pipe_funcs = {
> +	.update = sdrm_display_pipe_update,
> +	.enable = sdrm_display_pipe_enable,
> +	.disable = sdrm_display_pipe_disable,
> +};
> +
> +static int sdrm_fb_create_handle(struct drm_framebuffer *fb,
> +				 struct drm_file *dfile,
> +				 unsigned int *handle)
> +{
> +	struct sdrm_framebuffer *sfb = to_sdrm_fb(fb);
> +
> +	return drm_gem_handle_create(dfile, &sfb->obj->base, handle);
> +}
> +
> +static void sdrm_fb_destroy(struct drm_framebuffer *fb)
> +{
> +	struct sdrm_framebuffer *sfb = to_sdrm_fb(fb);
> +
> +	drm_framebuffer_cleanup(fb);
> +	drm_gem_object_unreference_unlocked(&sfb->obj->base);
> +	kfree(sfb);
> +}
> +
> +static const struct drm_framebuffer_funcs sdrm_fb_ops = {
> +	.create_handle = sdrm_fb_create_handle,
> +	.dirty = sdrm_dirty,
> +	.destroy = sdrm_fb_destroy,
> +};
> +
> +static struct drm_framebuffer *sdrm_fb_create(struct drm_device *ddev,
> +					      struct drm_file *dfile,
> +					      const struct drm_mode_fb_cmd2 *cmd)
> +{
> +	struct sdrm_framebuffer *fb;
> +	struct drm_gem_object *gobj;
> +	u32 bpp, size;
> +	int ret;
> +	void *err;
> +
> +	if (cmd->flags)
> +		return ERR_PTR(-EINVAL);
> +
> +	gobj = drm_gem_object_lookup(dfile, cmd->handles[0]);
> +	if (!gobj)
> +		return ERR_PTR(-EINVAL);
> +
> +	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
> +	if (!fb) {
> +		err = ERR_PTR(-ENOMEM);
> +		goto err_unref;
> +	}
> +	fb->obj = to_sdrm_bo(gobj);
> +
> +	fb->base.pitches[0] = cmd->pitches[0];
> +	fb->base.offsets[0] = cmd->offsets[0];
> +	fb->base.width = cmd->width;
> +	fb->base.height = cmd->height;
> +	fb->base.pixel_format = cmd->pixel_format;
> +	drm_fb_get_bpp_depth(cmd->pixel_format, &fb->base.depth,
> +			     &fb->base.bits_per_pixel);
> +
> +	/*
> +	 * width/height are already clamped into min/max_width/height range,
> +	 * so overflows are not possible
> +	 */
> +
> +	bpp = (fb->base.bits_per_pixel + 7) / 8;
> +	size = cmd->pitches[0] * cmd->height;
> +	if (!bpp ||
> +	    bpp > 4 ||
> +	    cmd->pitches[0] < bpp * fb->base.width ||
> +	    cmd->pitches[0] > 0xffffU ||
> +	    size + fb->base.offsets[0] < size ||
> +	    size + fb->base.offsets[0] > fb->obj->base.size) {
> +		err = ERR_PTR(-EINVAL);
> +		goto err_free;
> +	}
> +
> +	ret = drm_framebuffer_init(ddev, &fb->base, &sdrm_fb_ops);
> +	if (ret < 0) {
> +		err = ERR_PTR(ret);
> +		goto err_free;
> +	}
> +
> +	DRM_DEBUG_KMS("[FB:%d] pixel_format: %s\n", fb->base.base.id,
> +		      drm_get_format_name(fb->base.pixel_format));
> +
> +	return &fb->base;
> +
> +err_free:
> +	kfree(fb);
> +err_unref:
> +	drm_gem_object_unreference_unlocked(gobj);
> +
> +	return err;
> +}
> +
> +static const struct drm_mode_config_funcs sdrm_mode_config_ops = {
> +	.fb_create = sdrm_fb_create,
> +	.atomic_check = drm_atomic_helper_check,
> +	.atomic_commit = drm_atomic_helper_commit,
> +};
> +
> +int sdrm_drm_modeset_init(struct sdrm_device *sdrm)
> +{
> +	struct drm_connector *conn = &sdrm->conn;
> +	struct drm_device *ddev = sdrm->ddev;
> +	int ret;
> +
> +	drm_mode_config_init(ddev);
> +	ddev->mode_config.min_width = sdrm->fb_width;
> +	ddev->mode_config.max_width = sdrm->fb_width;
> +	ddev->mode_config.min_height = sdrm->fb_height;
> +	ddev->mode_config.max_height = sdrm->fb_height;
> +	ddev->mode_config.preferred_depth = sdrm->fb_bpp;
> +	ddev->mode_config.funcs = &sdrm_mode_config_ops;
> +
> +	drm_connector_helper_add(conn, &sdrm_conn_hfuncs);
> +	ret = drm_connector_init(ddev, conn, &sdrm_conn_ops,
> +				 DRM_MODE_CONNECTOR_VIRTUAL);
> +	if (ret)
> +		goto err_cleanup;
> +
> +	ret = drm_mode_create_dirty_info_property(ddev);
> +	if (ret)
> +		goto err_cleanup;
> +
> +	drm_object_attach_property(&conn->base,
> +				   ddev->mode_config.dirty_info_property,
> +				   DRM_MODE_DIRTY_ON);

dirty_info_property is gone.

> +
> +	ret = drm_simple_display_pipe_init(ddev, &sdrm->pipe, &sdrm_pipe_funcs,
> +					   sdrm_formats,
> +					   ARRAY_SIZE(sdrm_formats), conn);
> +	if (ret)
> +		goto err_cleanup;
> +
> +	drm_mode_config_reset(ddev);
> +
> +	return 0;
> +
> +err_cleanup:
> +	drm_mode_config_cleanup(ddev);
> +
> +	return ret;
> +}
> --
> 2.8.2
> 
> _______________________________________________
> dri-devel mailing list
> [e-mail]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Noralf Trønnes Aug. 16, 2016, 12:58 p.m. UTC | #2

Den 15.08.2016 08:59, skrev Daniel Vetter:
> On Sun, Aug 14, 2016 at 06:52:04PM +0200, Noralf Trønnes wrote:
>> The SimpleDRM driver binds to simple-framebuffer devices and provides a
>> DRM/KMS API. It provides only a single CRTC+encoder+connector combination
>> plus one initial mode.
>>
>> Userspace can create dumb-buffers which can be blit into the real
>> framebuffer similar to UDL. No access to the real framebuffer is allowed
>> (compared to earlier version of this driver) to avoid security issues.
>> Furthermore, this way we can support arbitrary modes as long as we have a
>> conversion-helper.
>>
>> The driver was originally written by David Herrmann in 2014.
>> My main contribution is to make use of drm_simple_kms_helper and
>> rework the probe path to avoid use of the deprecated drm_platform_init()
>> and drm_driver.{load,unload}().
>> Additions have also been made for later changes to the Device Tree binding
>> document, like support for clocks, regulators and having the node under
>> /chosen. This code was lifted verbatim from simplefb.c.
>>
>> Cc: [e-mail]
>> Cc: [e-mail]
>> Signed-off-by: Noralf Trønnes <[e-mail]>

<snip>

>> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_gem.c b/drivers/gpu/drm/simpledrm/simpledrm_gem.c
>> new file mode 100644
>> index 0000000..81bd7c5
>> --- /dev/null
>> +++ b/drivers/gpu/drm/simpledrm/simpledrm_gem.c

<snip>

>> +struct sdrm_gem_object *sdrm_gem_alloc_object(struct drm_device *ddev,
>> +					      size_t size)
>> +{
>> +	struct sdrm_gem_object *obj;
>> +
>> +	WARN_ON(!size || (size & ~PAGE_MASK) != 0);
>> +
>> +	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
>> +	if (!obj)
>> +		return NULL;
>> +
>> +	drm_gem_private_object_init(ddev, &obj->base, size);
>> +	return obj;
>> +}
>> +
>> +void sdrm_gem_free_object(struct drm_gem_object *gobj)
>> +{
>> +	struct sdrm_gem_object *obj = to_sdrm_bo(gobj);
>> +	struct drm_device *ddev = gobj->dev;
>> +
>> +	if (obj->pages) {
>> +		/* kill all user-space mappings */
>> +		drm_vma_node_unmap(&gobj->vma_node,
>> +				   ddev->anon_inode->i_mapping);
>> +		sdrm_gem_put_pages(obj);
>> +	}
>> +
>> +	if (gobj->import_attach)
>> +		drm_prime_gem_destroy(gobj, obj->sg);
>> +
>> +	drm_gem_free_mmap_offset(gobj);
>> +	drm_gem_object_release(gobj);
>> +	kfree(obj);
>> +}
>> +
>> +int sdrm_dumb_create(struct drm_file *dfile, struct drm_device *ddev,
>> +		     struct drm_mode_create_dumb *args)
>> +{
>> +	struct sdrm_gem_object *obj;
>> +	int r;
>> +
>> +	if (args->flags)
>> +		return -EINVAL;
>> +
>> +	/* overflow checks are done by DRM core */
>> +	args->pitch = (args->bpp + 7) / 8 * args->width;
>> +	args->size = PAGE_ALIGN(args->pitch * args->height);
>> +
>> +	obj = sdrm_gem_alloc_object(ddev, args->size);
>> +	if (!obj)
>> +		return -ENOMEM;
>> +
>> +	r = drm_gem_handle_create(dfile, &obj->base, &args->handle);
>> +	if (r) {
>> +		drm_gem_object_unreference_unlocked(&obj->base);
>> +		return r;
>> +	}
>> +
>> +	/* handle owns a reference */
>> +	drm_gem_object_unreference_unlocked(&obj->base);
>> +	return 0;
>> +}
>> +
>> +int sdrm_dumb_destroy(struct drm_file *dfile, struct drm_device *ddev,
>> +		      uint32_t handle)
>> +{
>> +	return drm_gem_handle_delete(dfile, handle);
>> +}
> I wonder whether some dumb+gem helpers wouldn't be useful to roll out. At
> least drm_dumb_gem_destroy.c would be pretty simple.

There's already a drm_gem_dumb_destroy() in drm_gem.c

The drm_driver->gem_create_object callback makes it possible to do a
generic dumb create:

int drm_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
             struct drm_mode_create_dumb *args)
{
     struct drm_gem_object *obj;
     int ret;

     if (!dev->driver->gem_create_object)
         return -EINVAL;

     args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
     args->size = args->pitch * args->height;

     obj = dev->driver->gem_create_object(dev, args->size);
     if (!obj)
         return -ENOMEM;

     ret = drm_gem_handle_create(file, obj, &args->handle);
     drm_gem_object_unreference_unlocked(obj);

     return ret;
}
EXPORT_SYMBOL(drm_gem_dumb_create);

struct drm_gem_object *sdrm_gem_create_object(struct drm_device *ddev,
                           size_t size)
{
     struct sdrm_gem_object *sobj;

     sobj = sdrm_gem_alloc_object(ddev, size);
     if (!sobj)
         return ERR_PTR(-ENOMEM);

     return &sobj->base;
}

>> +
>> +int sdrm_dumb_map_offset(struct drm_file *dfile, struct drm_device *ddev,
>> +			 uint32_t handle, uint64_t *offset)
>> +{
>> +	struct drm_gem_object *gobj;
>> +	int r;
>> +
>> +	mutex_lock(&ddev->struct_mutex);
> There's still some struct mutex here.
>
>> +
>> +	gobj = drm_gem_object_lookup(dfile, handle);
>> +	if (!gobj) {
>> +		r = -ENOENT;
>> +		goto out_unlock;
>> +	}
>> +
>> +	r = drm_gem_create_mmap_offset(gobj);
>> +	if (r)
>> +		goto out_unref;
>> +
>> +	*offset = drm_vma_node_offset_addr(&gobj->vma_node);
>> +
>> +out_unref:
>> +	drm_gem_object_unreference(gobj);
>> +out_unlock:
>> +	mutex_unlock(&ddev->struct_mutex);
>> +	return r;
>> +}
>> +

Maybe this addition to drm_gem.c as well:

int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
                 uint32_t handle, uint64_t *offset)
{
     struct drm_gem_object *obj;
     int ret;

     obj = drm_gem_object_lookup(file, handle);
     if (!obj)
         return -ENOENT;

     ret = drm_gem_create_mmap_offset(obj);
     if (ret)
         goto out_unref;

     *offset = drm_vma_node_offset_addr(&obj->vma_node);

out_unref:
     drm_gem_object_unreference_unlocked(obj);

     return ret;
}

>> +int sdrm_drm_mmap(struct file *filp, struct vm_area_struct *vma)
>> +{
>> +	struct drm_file *priv = filp->private_data;
>> +	struct drm_device *dev = priv->minor->dev;
>> +	struct drm_vma_offset_node *node;
>> +	struct drm_gem_object *gobj;
>> +	struct sdrm_gem_object *obj;
>> +	size_t size, i, num;
>> +	int r;
>> +
>> +	if (drm_device_is_unplugged(dev))
>> +		return -ENODEV;
>> +
>> +	drm_vma_offset_lock_lookup(dev->vma_offset_manager);
>> +	node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
>> +						  vma->vm_pgoff,
>> +						  vma_pages(vma));
>> +	drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
>> +
>> +	if (!drm_vma_node_is_allowed(node, filp))
>> +		return -EACCES;
>> +
>> +	gobj = container_of(node, struct drm_gem_object, vma_node);
>> +	obj = to_sdrm_bo(gobj);
>> +	size = drm_vma_node_size(node) << PAGE_SHIFT;
>> +	if (size < vma->vm_end - vma->vm_start)
>> +		return r;
>> +
>> +	r = sdrm_gem_get_pages(obj);
>> +	if (r < 0)
>> +		return r;
>> +
>> +	/* prevent dmabuf-imported mmap to user-space */
>> +	if (!obj->pages)
>> +		return -EACCES;
>> +
>> +	vma->vm_flags |= VM_DONTEXPAND;
>> +	vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
>> +
>> +	num = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
>> +	for (i = 0; i < num; ++i) {
>> +		r = vm_insert_page(vma, vma->vm_start + i * PAGE_SIZE,
>> +				   obj->pages[i]);
>> +		if (r < 0) {
>> +			if (i > 0)
>> +				zap_vma_ptes(vma, vma->vm_start, i * PAGE_SIZE);
>> +			return r;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
> Why can't we just redirect to the underlying shmem mmap here (after
> argument checking)?

I don't know what that means, but looking at vc4 it seems I can use
drm_gem_mmap() to remove some boilerplate.

int sdrm_drm_mmap(struct file *filp, struct vm_area_struct *vma)
{
     unsigned long vm_flags = vma->vm_flags;
     struct sdrm_gem_object *sobj;
     struct drm_gem_object *obj;
     size_t i, num;
     int r;

     r = drm_gem_mmap(filp, vma);
     if (r)
         return r;

     obj = vma->vm_private_data;

     sobj = to_sdrm_bo(obj);

     r = sdrm_gem_get_pages(obj);
     if (r < 0)
         return r;

     /* prevent dmabuf-imported mmap to user-space */
     if (!obj->pages)
         return -EACCES;

     /* drm_gem_mmap() sets flags that we don't want */
     vma->vm_flags = vm_flags | VM_DONTEXPAND;
     vma->vm_page_prot = 
pgprot_writecombine(vm_get_page_prot(vma->vm_flags));

     num = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
     for (i = 0; i < num; ++i) {
         r = vm_insert_page(vma, vma->vm_start + i * PAGE_SIZE,
                    obj->pages[i]);
         if (r < 0) {
             if (i > 0)
                 zap_vma_ptes(vma, vma->vm_start, i * PAGE_SIZE);
             return r;
         }
     }

     return 0;
}

drm_gem_mmap() requires drm_driver->gem_vm_ops to be set:

const struct vm_operations_struct sdrm_gem_vm_ops = {
     .open = drm_gem_vm_open,
     .close = drm_gem_vm_close,
};

And browsing the code a bit more shows that tegra, udl, etnaviv and vgem
does the vm_insert_page() thing in the vm_operations_struct->fault() 
handler.

So maybe this makes sense for simpledrm as well:

static int sdrm_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
{
         struct drm_gem_object *obj = vma->vm_private_data;
     struct sdrm_gem_object *sobj = to_sdrm_bo(obj);
     loff_t num_pages;
     pgoff_t offset;
     int r;

     if (!sobj->pages)
         return VM_FAULT_SIGBUS;

     offset = ((unsigned long)vmf->virtual_address - vma->vm_start) >>
          PAGE_SHIFT;
     num_pages = DIV_ROUND_UP(obj->base.size, PAGE_SIZE);
     if (page_offset > num_pages)
         return VM_FAULT_SIGBUS;

     r = vm_insert_page(vma, (unsigned long)vmf->virtual_address,
                sobj->pages[offset]);
     switch (r) {
     case -EAGAIN:
     case 0:
     case -ERESTARTSYS:
     case -EINTR:
     case -EBUSY:
         return VM_FAULT_NOPAGE;

     case -ENOMEM:
         return VM_FAULT_OOM;
     }

     return VM_FAULT_SIGBUS;
}


Noralf.

>> +
>> +struct drm_gem_object *sdrm_gem_prime_import(struct drm_device *ddev,
>> +					     struct dma_buf *dma_buf)
>> +{
>> +	struct dma_buf_attachment *attach;
>> +	struct sdrm_gem_object *obj;
>> +	struct sg_table *sg;
>> +	int ret;
>> +
>> +	/* need to attach */
>> +	attach = dma_buf_attach(dma_buf, ddev->dev);
>> +	if (IS_ERR(attach))
>> +		return ERR_CAST(attach);
>> +
>> +	get_dma_buf(dma_buf);
>> +
>> +	sg = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
>> +	if (IS_ERR(sg)) {
>> +		ret = PTR_ERR(sg);
>> +		goto fail_detach;
>> +	}
>> +
>> +	/*
>> +	 * dma_buf_vmap() gives us a page-aligned mapping, so lets bump the
>> +	 * size of the dma-buf to the next page-boundary
>> +	 */
> dma_buf should be page-aligned already. Not sure we enforce that, but it's
> in practice a given.
>
>> +	obj = sdrm_gem_alloc_object(ddev, PAGE_ALIGN(dma_buf->size));
>> +	if (!obj) {
>> +		ret = -ENOMEM;
>> +		goto fail_unmap;
>> +	}
>> +
>> +	obj->sg = sg;
>> +	obj->base.import_attach = attach;
>> +
>> +	return &obj->base;
>> +
>> +fail_unmap:
>> +	dma_buf_unmap_attachment(attach, sg, DMA_BIDIRECTIONAL);
>> +fail_detach:
>> +	dma_buf_detach(dma_buf, attach);
>> +	dma_buf_put(dma_buf);
>> +	return ERR_PTR(ret);
>> +}
>> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_kms.c b/drivers/gpu/drm/simpledrm/simpledrm_kms.c
>> new file mode 100644
>> index 0000000..00e50d9
>> --- /dev/null
>> +++ b/drivers/gpu/drm/simpledrm/simpledrm_kms.c
>> @@ -0,0 +1,258 @@
>> +/*
>> + * SimpleDRM firmware framebuffer driver
>> + * Copyright (c) 2012-2014 David Herrmann <[e-mail]>
>> + *
>> + * 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.
>> + */
>> +
>> +#include <drm/drmP.h>
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_crtc.h>
>> +#include <drm/drm_crtc_helper.h>
>> +#include <drm/drm_gem.h>
>> +#include <drm/drm_simple_kms_helper.h>
>> +#include <linux/slab.h>
>> +
>> +#include "simpledrm.h"
>> +
>> +static const uint32_t sdrm_formats[] = {
>> +	DRM_FORMAT_RGB565,
>> +	DRM_FORMAT_ARGB8888,
>> +	DRM_FORMAT_XRGB8888,
>> +};
>> +
>> +static int sdrm_conn_get_modes(struct drm_connector *conn)
>> +{
>> +	struct sdrm_device *sdrm = conn->dev->dev_private;
>> +	struct drm_display_mode *mode;
>> +
>> +	mode = drm_cvt_mode(sdrm->ddev, sdrm->fb_width, sdrm->fb_height,
>> +			    60, false, false, false);
>> +	if (!mode)
>> +		return 0;
>> +
>> +	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
>> +	drm_mode_set_name(mode);
>> +	drm_mode_probed_add(conn, mode);
>> +
>> +	return 1;
>> +}
>> +
>> +static const struct drm_connector_helper_funcs sdrm_conn_hfuncs = {
>> +	.get_modes = sdrm_conn_get_modes,
>> +	.best_encoder = drm_atomic_helper_best_encoder,
>> +};
>> +
>> +static enum drm_connector_status sdrm_conn_detect(struct drm_connector *conn,
>> +						  bool force)
>> +{
>> +	/*
>> +	 * We simulate an always connected monitor. simple-fb doesn't
>> +	 * provide any way to detect whether the connector is active. Hence,
>> +	 * signal DRM core that it is always connected.
>> +	 */
>> +
>> +	return connector_status_connected;
>> +}
>> +
>> +static const struct drm_connector_funcs sdrm_conn_ops = {
>> +	.dpms = drm_atomic_helper_connector_dpms,
>> +	.reset = drm_atomic_helper_connector_reset,
>> +	.detect = sdrm_conn_detect,
>> +	.fill_modes = drm_helper_probe_single_connector_modes,
>> +	.destroy = drm_connector_cleanup,
>> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> +};
>> +
>> +static inline struct sdrm_device *
>> +pipe_to_sdrm(struct drm_simple_display_pipe *pipe)
>> +{
>> +	return container_of(pipe, struct sdrm_device, pipe);
>> +}
>> +
>> +static void sdrm_crtc_send_vblank_event(struct drm_crtc *crtc)
>> +{
>> +	if (crtc->state && crtc->state->event) {
>> +		spin_lock_irq(&crtc->dev->event_lock);
>> +		drm_crtc_send_vblank_event(crtc, crtc->state->event);
>> +		spin_unlock_irq(&crtc->dev->event_lock);
>> +		crtc->state->event = NULL;
>> +	}
>> +}
>> +
>> +void sdrm_display_pipe_update(struct drm_simple_display_pipe *pipe,
>> +			      struct drm_plane_state *plane_state)
>> +{
>> +	struct drm_framebuffer *fb = pipe->plane.state->fb;
>> +	struct sdrm_device *sdrm = pipe_to_sdrm(pipe);
>> +
>> +	sdrm_crtc_send_vblank_event(&pipe->crtc);
>> +
>> +	if (fb) {
>> +		pipe->plane.fb = fb;
>> +		sdrm_dirty_all_locked(sdrm);
>> +	}
>> +}
>> +
>> +static void sdrm_display_pipe_enable(struct drm_simple_display_pipe *pipe,
>> +				     struct drm_crtc_state *crtc_state)
>> +{
>> +	sdrm_crtc_send_vblank_event(&pipe->crtc);
>> +}
>> +
>> +static void sdrm_display_pipe_disable(struct drm_simple_display_pipe *pipe)
>> +{
>> +	sdrm_crtc_send_vblank_event(&pipe->crtc);
>> +}
>> +
>> +static const struct drm_simple_display_pipe_funcs sdrm_pipe_funcs = {
>> +	.update = sdrm_display_pipe_update,
>> +	.enable = sdrm_display_pipe_enable,
>> +	.disable = sdrm_display_pipe_disable,
>> +};
>> +
>> +static int sdrm_fb_create_handle(struct drm_framebuffer *fb,
>> +				 struct drm_file *dfile,
>> +				 unsigned int *handle)
>> +{
>> +	struct sdrm_framebuffer *sfb = to_sdrm_fb(fb);
>> +
>> +	return drm_gem_handle_create(dfile, &sfb->obj->base, handle);
>> +}
>> +
>> +static void sdrm_fb_destroy(struct drm_framebuffer *fb)
>> +{
>> +	struct sdrm_framebuffer *sfb = to_sdrm_fb(fb);
>> +
>> +	drm_framebuffer_cleanup(fb);
>> +	drm_gem_object_unreference_unlocked(&sfb->obj->base);
>> +	kfree(sfb);
>> +}
>> +
>> +static const struct drm_framebuffer_funcs sdrm_fb_ops = {
>> +	.create_handle = sdrm_fb_create_handle,
>> +	.dirty = sdrm_dirty,
>> +	.destroy = sdrm_fb_destroy,
>> +};
>> +
>> +static struct drm_framebuffer *sdrm_fb_create(struct drm_device *ddev,
>> +					      struct drm_file *dfile,
>> +					      const struct drm_mode_fb_cmd2 *cmd)
>> +{
>> +	struct sdrm_framebuffer *fb;
>> +	struct drm_gem_object *gobj;
>> +	u32 bpp, size;
>> +	int ret;
>> +	void *err;
>> +
>> +	if (cmd->flags)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	gobj = drm_gem_object_lookup(dfile, cmd->handles[0]);
>> +	if (!gobj)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
>> +	if (!fb) {
>> +		err = ERR_PTR(-ENOMEM);
>> +		goto err_unref;
>> +	}
>> +	fb->obj = to_sdrm_bo(gobj);
>> +
>> +	fb->base.pitches[0] = cmd->pitches[0];
>> +	fb->base.offsets[0] = cmd->offsets[0];
>> +	fb->base.width = cmd->width;
>> +	fb->base.height = cmd->height;
>> +	fb->base.pixel_format = cmd->pixel_format;
>> +	drm_fb_get_bpp_depth(cmd->pixel_format, &fb->base.depth,
>> +			     &fb->base.bits_per_pixel);
>> +
>> +	/*
>> +	 * width/height are already clamped into min/max_width/height range,
>> +	 * so overflows are not possible
>> +	 */
>> +
>> +	bpp = (fb->base.bits_per_pixel + 7) / 8;
>> +	size = cmd->pitches[0] * cmd->height;
>> +	if (!bpp ||
>> +	    bpp > 4 ||
>> +	    cmd->pitches[0] < bpp * fb->base.width ||
>> +	    cmd->pitches[0] > 0xffffU ||
>> +	    size + fb->base.offsets[0] < size ||
>> +	    size + fb->base.offsets[0] > fb->obj->base.size) {
>> +		err = ERR_PTR(-EINVAL);
>> +		goto err_free;
>> +	}
>> +
>> +	ret = drm_framebuffer_init(ddev, &fb->base, &sdrm_fb_ops);
>> +	if (ret < 0) {
>> +		err = ERR_PTR(ret);
>> +		goto err_free;
>> +	}
>> +
>> +	DRM_DEBUG_KMS("[FB:%d] pixel_format: %s\n", fb->base.base.id,
>> +		      drm_get_format_name(fb->base.pixel_format));
>> +
>> +	return &fb->base;
>> +
>> +err_free:
>> +	kfree(fb);
>> +err_unref:
>> +	drm_gem_object_unreference_unlocked(gobj);
>> +
>> +	return err;
>> +}
>> +
>> +static const struct drm_mode_config_funcs sdrm_mode_config_ops = {
>> +	.fb_create = sdrm_fb_create,
>> +	.atomic_check = drm_atomic_helper_check,
>> +	.atomic_commit = drm_atomic_helper_commit,
>> +};
>> +
>> +int sdrm_drm_modeset_init(struct sdrm_device *sdrm)
>> +{
>> +	struct drm_connector *conn = &sdrm->conn;
>> +	struct drm_device *ddev = sdrm->ddev;
>> +	int ret;
>> +
>> +	drm_mode_config_init(ddev);
>> +	ddev->mode_config.min_width = sdrm->fb_width;
>> +	ddev->mode_config.max_width = sdrm->fb_width;
>> +	ddev->mode_config.min_height = sdrm->fb_height;
>> +	ddev->mode_config.max_height = sdrm->fb_height;
>> +	ddev->mode_config.preferred_depth = sdrm->fb_bpp;
>> +	ddev->mode_config.funcs = &sdrm_mode_config_ops;
>> +
>> +	drm_connector_helper_add(conn, &sdrm_conn_hfuncs);
>> +	ret = drm_connector_init(ddev, conn, &sdrm_conn_ops,
>> +				 DRM_MODE_CONNECTOR_VIRTUAL);
>> +	if (ret)
>> +		goto err_cleanup;
>> +
>> +	ret = drm_mode_create_dirty_info_property(ddev);
>> +	if (ret)
>> +		goto err_cleanup;
>> +
>> +	drm_object_attach_property(&conn->base,
>> +				   ddev->mode_config.dirty_info_property,
>> +				   DRM_MODE_DIRTY_ON);
> dirty_info_property is gone.
>
>> +
>> +	ret = drm_simple_display_pipe_init(ddev, &sdrm->pipe, &sdrm_pipe_funcs,
>> +					   sdrm_formats,
>> +					   ARRAY_SIZE(sdrm_formats), conn);
>> +	if (ret)
>> +		goto err_cleanup;
>> +
>> +	drm_mode_config_reset(ddev);
>> +
>> +	return 0;
>> +
>> +err_cleanup:
>> +	drm_mode_config_cleanup(ddev);
>> +
>> +	return ret;
>> +}
>> --
>> 2.8.2
>>
>> _______________________________________________
>> dri-devel mailing list
>> [e-mail]
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Daniel Vetter Aug. 16, 2016, 3:25 p.m. UTC | #3

On Tue, Aug 16, 2016 at 02:58:38PM +0200, Noralf Trønnes wrote:
> 
> Den 15.08.2016 08:59, skrev Daniel Vetter:
> > On Sun, Aug 14, 2016 at 06:52:04PM +0200, Noralf Trønnes wrote:
> > > The SimpleDRM driver binds to simple-framebuffer devices and provides a
> > > DRM/KMS API. It provides only a single CRTC+encoder+connector combination
> > > plus one initial mode.
> > > 
> > > Userspace can create dumb-buffers which can be blit into the real
> > > framebuffer similar to UDL. No access to the real framebuffer is allowed
> > > (compared to earlier version of this driver) to avoid security issues.
> > > Furthermore, this way we can support arbitrary modes as long as we have a
> > > conversion-helper.
> > > 
> > > The driver was originally written by David Herrmann in 2014.
> > > My main contribution is to make use of drm_simple_kms_helper and
> > > rework the probe path to avoid use of the deprecated drm_platform_init()
> > > and drm_driver.{load,unload}().
> > > Additions have also been made for later changes to the Device Tree binding
> > > document, like support for clocks, regulators and having the node under
> > > /chosen. This code was lifted verbatim from simplefb.c.
> > > 
> > > Cc: [e-mail]
> > > Cc: [e-mail]
> > > Signed-off-by: Noralf Trønnes <[e-mail]>
> 
> <snip>
> 
> > > diff --git a/drivers/gpu/drm/simpledrm/simpledrm_gem.c b/drivers/gpu/drm/simpledrm/simpledrm_gem.c
> > > new file mode 100644
> > > index 0000000..81bd7c5
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/simpledrm/simpledrm_gem.c
> 
> <snip>
> 
> > > +struct sdrm_gem_object *sdrm_gem_alloc_object(struct drm_device *ddev,
> > > +					      size_t size)
> > > +{
> > > +	struct sdrm_gem_object *obj;
> > > +
> > > +	WARN_ON(!size || (size & ~PAGE_MASK) != 0);
> > > +
> > > +	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
> > > +	if (!obj)
> > > +		return NULL;
> > > +
> > > +	drm_gem_private_object_init(ddev, &obj->base, size);
> > > +	return obj;
> > > +}
> > > +
> > > +void sdrm_gem_free_object(struct drm_gem_object *gobj)
> > > +{
> > > +	struct sdrm_gem_object *obj = to_sdrm_bo(gobj);
> > > +	struct drm_device *ddev = gobj->dev;
> > > +
> > > +	if (obj->pages) {
> > > +		/* kill all user-space mappings */
> > > +		drm_vma_node_unmap(&gobj->vma_node,
> > > +				   ddev->anon_inode->i_mapping);
> > > +		sdrm_gem_put_pages(obj);
> > > +	}
> > > +
> > > +	if (gobj->import_attach)
> > > +		drm_prime_gem_destroy(gobj, obj->sg);
> > > +
> > > +	drm_gem_free_mmap_offset(gobj);
> > > +	drm_gem_object_release(gobj);
> > > +	kfree(obj);
> > > +}
> > > +
> > > +int sdrm_dumb_create(struct drm_file *dfile, struct drm_device *ddev,
> > > +		     struct drm_mode_create_dumb *args)
> > > +{
> > > +	struct sdrm_gem_object *obj;
> > > +	int r;
> > > +
> > > +	if (args->flags)
> > > +		return -EINVAL;
> > > +
> > > +	/* overflow checks are done by DRM core */
> > > +	args->pitch = (args->bpp + 7) / 8 * args->width;
> > > +	args->size = PAGE_ALIGN(args->pitch * args->height);
> > > +
> > > +	obj = sdrm_gem_alloc_object(ddev, args->size);
> > > +	if (!obj)
> > > +		return -ENOMEM;
> > > +
> > > +	r = drm_gem_handle_create(dfile, &obj->base, &args->handle);
> > > +	if (r) {
> > > +		drm_gem_object_unreference_unlocked(&obj->base);
> > > +		return r;
> > > +	}
> > > +
> > > +	/* handle owns a reference */
> > > +	drm_gem_object_unreference_unlocked(&obj->base);
> > > +	return 0;
> > > +}
> > > +
> > > +int sdrm_dumb_destroy(struct drm_file *dfile, struct drm_device *ddev,
> > > +		      uint32_t handle)
> > > +{
> > > +	return drm_gem_handle_delete(dfile, handle);
> > > +}
> > I wonder whether some dumb+gem helpers wouldn't be useful to roll out. At
> > least drm_dumb_gem_destroy.c would be pretty simple.
> 
> There's already a drm_gem_dumb_destroy() in drm_gem.c
> 
> The drm_driver->gem_create_object callback makes it possible to do a
> generic dumb create:
> 
> int drm_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
>             struct drm_mode_create_dumb *args)
> {
>     struct drm_gem_object *obj;
>     int ret;
> 
>     if (!dev->driver->gem_create_object)
>         return -EINVAL;
> 
>     args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
>     args->size = args->pitch * args->height;
> 
>     obj = dev->driver->gem_create_object(dev, args->size);
>     if (!obj)
>         return -ENOMEM;
> 
>     ret = drm_gem_handle_create(file, obj, &args->handle);
>     drm_gem_object_unreference_unlocked(obj);
> 
>     return ret;
> }
> EXPORT_SYMBOL(drm_gem_dumb_create);
> 
> struct drm_gem_object *sdrm_gem_create_object(struct drm_device *ddev,
>                           size_t size)
> {
>     struct sdrm_gem_object *sobj;
> 
>     sobj = sdrm_gem_alloc_object(ddev, size);
>     if (!sobj)
>         return ERR_PTR(-ENOMEM);
> 
>     return &sobj->base;
> }
> 
> > > +
> > > +int sdrm_dumb_map_offset(struct drm_file *dfile, struct drm_device *ddev,
> > > +			 uint32_t handle, uint64_t *offset)
> > > +{
> > > +	struct drm_gem_object *gobj;
> > > +	int r;
> > > +
> > > +	mutex_lock(&ddev->struct_mutex);
> > There's still some struct mutex here.
> > 
> > > +
> > > +	gobj = drm_gem_object_lookup(dfile, handle);
> > > +	if (!gobj) {
> > > +		r = -ENOENT;
> > > +		goto out_unlock;
> > > +	}
> > > +
> > > +	r = drm_gem_create_mmap_offset(gobj);
> > > +	if (r)
> > > +		goto out_unref;
> > > +
> > > +	*offset = drm_vma_node_offset_addr(&gobj->vma_node);
> > > +
> > > +out_unref:
> > > +	drm_gem_object_unreference(gobj);
> > > +out_unlock:
> > > +	mutex_unlock(&ddev->struct_mutex);
> > > +	return r;
> > > +}
> > > +
> 
> Maybe this addition to drm_gem.c as well:
> 
> int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
>                 uint32_t handle, uint64_t *offset)
> {
>     struct drm_gem_object *obj;
>     int ret;
> 
>     obj = drm_gem_object_lookup(file, handle);
>     if (!obj)
>         return -ENOENT;
> 
>     ret = drm_gem_create_mmap_offset(obj);
>     if (ret)
>         goto out_unref;
> 
>     *offset = drm_vma_node_offset_addr(&obj->vma_node);
> 
> out_unref:
>     drm_gem_object_unreference_unlocked(obj);
> 
>     return ret;
> }

Yeah, sounds good for adding the above two. Feel free to roll them out to
drivers (or not).
 
> > > +int sdrm_drm_mmap(struct file *filp, struct vm_area_struct *vma)
> > > +{
> > > +	struct drm_file *priv = filp->private_data;
> > > +	struct drm_device *dev = priv->minor->dev;
> > > +	struct drm_vma_offset_node *node;
> > > +	struct drm_gem_object *gobj;
> > > +	struct sdrm_gem_object *obj;
> > > +	size_t size, i, num;
> > > +	int r;
> > > +
> > > +	if (drm_device_is_unplugged(dev))
> > > +		return -ENODEV;
> > > +
> > > +	drm_vma_offset_lock_lookup(dev->vma_offset_manager);
> > > +	node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
> > > +						  vma->vm_pgoff,
> > > +						  vma_pages(vma));
> > > +	drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
> > > +
> > > +	if (!drm_vma_node_is_allowed(node, filp))
> > > +		return -EACCES;
> > > +
> > > +	gobj = container_of(node, struct drm_gem_object, vma_node);
> > > +	obj = to_sdrm_bo(gobj);
> > > +	size = drm_vma_node_size(node) << PAGE_SHIFT;
> > > +	if (size < vma->vm_end - vma->vm_start)
> > > +		return r;
> > > +
> > > +	r = sdrm_gem_get_pages(obj);
> > > +	if (r < 0)
> > > +		return r;
> > > +
> > > +	/* prevent dmabuf-imported mmap to user-space */
> > > +	if (!obj->pages)
> > > +		return -EACCES;
> > > +
> > > +	vma->vm_flags |= VM_DONTEXPAND;
> > > +	vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
> > > +
> > > +	num = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> > > +	for (i = 0; i < num; ++i) {
> > > +		r = vm_insert_page(vma, vma->vm_start + i * PAGE_SIZE,
> > > +				   obj->pages[i]);
> > > +		if (r < 0) {
> > > +			if (i > 0)
> > > +				zap_vma_ptes(vma, vma->vm_start, i * PAGE_SIZE);
> > > +			return r;
> > > +		}
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > Why can't we just redirect to the underlying shmem mmap here (after
> > argument checking)?
> 
> I don't know what that means, but looking at vc4 it seems I can use
> drm_gem_mmap() to remove some boilerplate.
> 
> int sdrm_drm_mmap(struct file *filp, struct vm_area_struct *vma)
> {
>     unsigned long vm_flags = vma->vm_flags;
>     struct sdrm_gem_object *sobj;
>     struct drm_gem_object *obj;
>     size_t i, num;
>     int r;
> 
>     r = drm_gem_mmap(filp, vma);
>     if (r)
>         return r;
> 
>     obj = vma->vm_private_data;
> 
>     sobj = to_sdrm_bo(obj);
> 
>     r = sdrm_gem_get_pages(obj);
>     if (r < 0)
>         return r;
> 
>     /* prevent dmabuf-imported mmap to user-space */
>     if (!obj->pages)
>         return -EACCES;
> 
>     /* drm_gem_mmap() sets flags that we don't want */
>     vma->vm_flags = vm_flags | VM_DONTEXPAND;
>     vma->vm_page_prot =
> pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
> 
>     num = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
>     for (i = 0; i < num; ++i) {
>         r = vm_insert_page(vma, vma->vm_start + i * PAGE_SIZE,
>                    obj->pages[i]);
>         if (r < 0) {
>             if (i > 0)
>                 zap_vma_ptes(vma, vma->vm_start, i * PAGE_SIZE);
>             return r;
>         }
>     }
> 
>     return 0;
> }
> 
> drm_gem_mmap() requires drm_driver->gem_vm_ops to be set:
> 
> const struct vm_operations_struct sdrm_gem_vm_ops = {
>     .open = drm_gem_vm_open,
>     .close = drm_gem_vm_close,
> };
> 
> And browsing the code a bit more shows that tegra, udl, etnaviv and vgem
> does the vm_insert_page() thing in the vm_operations_struct->fault()
> handler.
> 
> So maybe this makes sense for simpledrm as well:
> 
> static int sdrm_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
> {
>         struct drm_gem_object *obj = vma->vm_private_data;
>     struct sdrm_gem_object *sobj = to_sdrm_bo(obj);
>     loff_t num_pages;
>     pgoff_t offset;
>     int r;
> 
>     if (!sobj->pages)
>         return VM_FAULT_SIGBUS;
> 
>     offset = ((unsigned long)vmf->virtual_address - vma->vm_start) >>
>          PAGE_SHIFT;
>     num_pages = DIV_ROUND_UP(obj->base.size, PAGE_SIZE);
>     if (page_offset > num_pages)
>         return VM_FAULT_SIGBUS;
> 
>     r = vm_insert_page(vma, (unsigned long)vmf->virtual_address,
>                sobj->pages[offset]);
>     switch (r) {
>     case -EAGAIN:
>     case 0:
>     case -ERESTARTSYS:
>     case -EINTR:
>     case -EBUSY:
>         return VM_FAULT_NOPAGE;
> 
>     case -ENOMEM:
>         return VM_FAULT_OOM;
>     }
> 
>     return VM_FAULT_SIGBUS;
> }

That's still a lot for what amounts to reimplementing mmap on shmem, but
badly. What I mean with redirecting is pointing the entire ->mmap
operation to the mmap implementation for the underlying mmap. Roughly:

	/* normal gem mmap checks first */

	/* redirect to shmem mmap */
	vma->vm_file = obj->filp;
	vma->vm_pgoff = 0;

	return obj->filp->f_op->mmap(obj->filp, vma);

Much less code ;-)

Cheers, Daniel

Noralf Trønnes Aug. 16, 2016, 7:38 p.m. UTC | #4

Den 16.08.2016 17:25, skrev Daniel Vetter:
> On Tue, Aug 16, 2016 at 02:58:38PM +0200, Noralf Trønnes wrote:
>> Den 15.08.2016 08:59, skrev Daniel Vetter:
>>> On Sun, Aug 14, 2016 at 06:52:04PM +0200, Noralf Trønnes wrote:
>>>> The SimpleDRM driver binds to simple-framebuffer devices and provides a
>>>> DRM/KMS API. It provides only a single CRTC+encoder+connector combination
>>>> plus one initial mode.
>>>>
>>>> Userspace can create dumb-buffers which can be blit into the real
>>>> framebuffer similar to UDL. No access to the real framebuffer is allowed
>>>> (compared to earlier version of this driver) to avoid security issues.
>>>> Furthermore, this way we can support arbitrary modes as long as we have a
>>>> conversion-helper.
>>>>
>>>> The driver was originally written by David Herrmann in 2014.
>>>> My main contribution is to make use of drm_simple_kms_helper and
>>>> rework the probe path to avoid use of the deprecated drm_platform_init()
>>>> and drm_driver.{load,unload}().
>>>> Additions have also been made for later changes to the Device Tree binding
>>>> document, like support for clocks, regulators and having the node under
>>>> /chosen. This code was lifted verbatim from simplefb.c.
>>>>
>>>> Cc: [e-mail]
>>>> Cc: [e-mail]
>>>> Signed-off-by: Noralf Trønnes <[e-mail]>
>> <snip>
>>
>>>> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_gem.c b/drivers/gpu/drm/simpledrm/simpledrm_gem.c
>>>> new file mode 100644
>>>> index 0000000..81bd7c5
>>>> --- /dev/null
>>>> +++ b/drivers/gpu/drm/simpledrm/simpledrm_gem.c
>> <snip>
>>
>>>> +struct sdrm_gem_object *sdrm_gem_alloc_object(struct drm_device *ddev,
>>>> +					      size_t size)
>>>> +{
>>>> +	struct sdrm_gem_object *obj;
>>>> +
>>>> +	WARN_ON(!size || (size & ~PAGE_MASK) != 0);
>>>> +
>>>> +	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
>>>> +	if (!obj)
>>>> +		return NULL;
>>>> +
>>>> +	drm_gem_private_object_init(ddev, &obj->base, size);
>>>> +	return obj;
>>>> +}
>>>> +
>>>> +void sdrm_gem_free_object(struct drm_gem_object *gobj)
>>>> +{
>>>> +	struct sdrm_gem_object *obj = to_sdrm_bo(gobj);
>>>> +	struct drm_device *ddev = gobj->dev;
>>>> +
>>>> +	if (obj->pages) {
>>>> +		/* kill all user-space mappings */
>>>> +		drm_vma_node_unmap(&gobj->vma_node,
>>>> +				   ddev->anon_inode->i_mapping);
>>>> +		sdrm_gem_put_pages(obj);
>>>> +	}
>>>> +
>>>> +	if (gobj->import_attach)
>>>> +		drm_prime_gem_destroy(gobj, obj->sg);
>>>> +
>>>> +	drm_gem_free_mmap_offset(gobj);
>>>> +	drm_gem_object_release(gobj);
>>>> +	kfree(obj);
>>>> +}
>>>> +
>>>> +int sdrm_dumb_create(struct drm_file *dfile, struct drm_device *ddev,
>>>> +		     struct drm_mode_create_dumb *args)
>>>> +{
>>>> +	struct sdrm_gem_object *obj;
>>>> +	int r;
>>>> +
>>>> +	if (args->flags)
>>>> +		return -EINVAL;
>>>> +
>>>> +	/* overflow checks are done by DRM core */
>>>> +	args->pitch = (args->bpp + 7) / 8 * args->width;
>>>> +	args->size = PAGE_ALIGN(args->pitch * args->height);
>>>> +
>>>> +	obj = sdrm_gem_alloc_object(ddev, args->size);
>>>> +	if (!obj)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	r = drm_gem_handle_create(dfile, &obj->base, &args->handle);
>>>> +	if (r) {
>>>> +		drm_gem_object_unreference_unlocked(&obj->base);
>>>> +		return r;
>>>> +	}
>>>> +
>>>> +	/* handle owns a reference */
>>>> +	drm_gem_object_unreference_unlocked(&obj->base);
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +int sdrm_dumb_destroy(struct drm_file *dfile, struct drm_device *ddev,
>>>> +		      uint32_t handle)
>>>> +{
>>>> +	return drm_gem_handle_delete(dfile, handle);
>>>> +}
>>> I wonder whether some dumb+gem helpers wouldn't be useful to roll out. At
>>> least drm_dumb_gem_destroy.c would be pretty simple.
>> There's already a drm_gem_dumb_destroy() in drm_gem.c
>>
>> The drm_driver->gem_create_object callback makes it possible to do a
>> generic dumb create:
>>
>> int drm_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
>>              struct drm_mode_create_dumb *args)
>> {
>>      struct drm_gem_object *obj;
>>      int ret;
>>
>>      if (!dev->driver->gem_create_object)
>>          return -EINVAL;
>>
>>      args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
>>      args->size = args->pitch * args->height;
>>
>>      obj = dev->driver->gem_create_object(dev, args->size);
>>      if (!obj)
>>          return -ENOMEM;
>>
>>      ret = drm_gem_handle_create(file, obj, &args->handle);
>>      drm_gem_object_unreference_unlocked(obj);
>>
>>      return ret;
>> }
>> EXPORT_SYMBOL(drm_gem_dumb_create);
>>
>> struct drm_gem_object *sdrm_gem_create_object(struct drm_device *ddev,
>>                            size_t size)
>> {
>>      struct sdrm_gem_object *sobj;
>>
>>      sobj = sdrm_gem_alloc_object(ddev, size);
>>      if (!sobj)
>>          return ERR_PTR(-ENOMEM);
>>
>>      return &sobj->base;
>> }
>>
>>>> +
>>>> +int sdrm_dumb_map_offset(struct drm_file *dfile, struct drm_device *ddev,
>>>> +			 uint32_t handle, uint64_t *offset)
>>>> +{
>>>> +	struct drm_gem_object *gobj;
>>>> +	int r;
>>>> +
>>>> +	mutex_lock(&ddev->struct_mutex);
>>> There's still some struct mutex here.
>>>
>>>> +
>>>> +	gobj = drm_gem_object_lookup(dfile, handle);
>>>> +	if (!gobj) {
>>>> +		r = -ENOENT;
>>>> +		goto out_unlock;
>>>> +	}
>>>> +
>>>> +	r = drm_gem_create_mmap_offset(gobj);
>>>> +	if (r)
>>>> +		goto out_unref;
>>>> +
>>>> +	*offset = drm_vma_node_offset_addr(&gobj->vma_node);
>>>> +
>>>> +out_unref:
>>>> +	drm_gem_object_unreference(gobj);
>>>> +out_unlock:
>>>> +	mutex_unlock(&ddev->struct_mutex);
>>>> +	return r;
>>>> +}
>>>> +
>> Maybe this addition to drm_gem.c as well:
>>
>> int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
>>                  uint32_t handle, uint64_t *offset)
>> {
>>      struct drm_gem_object *obj;
>>      int ret;
>>
>>      obj = drm_gem_object_lookup(file, handle);
>>      if (!obj)
>>          return -ENOENT;
>>
>>      ret = drm_gem_create_mmap_offset(obj);
>>      if (ret)
>>          goto out_unref;
>>
>>      *offset = drm_vma_node_offset_addr(&obj->vma_node);
>>
>> out_unref:
>>      drm_gem_object_unreference_unlocked(obj);
>>
>>      return ret;
>> }
> Yeah, sounds good for adding the above two. Feel free to roll them out to
> drivers (or not).
>   
>>>> +int sdrm_drm_mmap(struct file *filp, struct vm_area_struct *vma)
>>>> +{
>>>> +	struct drm_file *priv = filp->private_data;
>>>> +	struct drm_device *dev = priv->minor->dev;
>>>> +	struct drm_vma_offset_node *node;
>>>> +	struct drm_gem_object *gobj;
>>>> +	struct sdrm_gem_object *obj;
>>>> +	size_t size, i, num;
>>>> +	int r;
>>>> +
>>>> +	if (drm_device_is_unplugged(dev))
>>>> +		return -ENODEV;
>>>> +
>>>> +	drm_vma_offset_lock_lookup(dev->vma_offset_manager);
>>>> +	node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
>>>> +						  vma->vm_pgoff,
>>>> +						  vma_pages(vma));
>>>> +	drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
>>>> +
>>>> +	if (!drm_vma_node_is_allowed(node, filp))
>>>> +		return -EACCES;
>>>> +
>>>> +	gobj = container_of(node, struct drm_gem_object, vma_node);
>>>> +	obj = to_sdrm_bo(gobj);
>>>> +	size = drm_vma_node_size(node) << PAGE_SHIFT;
>>>> +	if (size < vma->vm_end - vma->vm_start)
>>>> +		return r;
>>>> +
>>>> +	r = sdrm_gem_get_pages(obj);
>>>> +	if (r < 0)
>>>> +		return r;
>>>> +
>>>> +	/* prevent dmabuf-imported mmap to user-space */
>>>> +	if (!obj->pages)
>>>> +		return -EACCES;
>>>> +
>>>> +	vma->vm_flags |= VM_DONTEXPAND;
>>>> +	vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
>>>> +
>>>> +	num = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
>>>> +	for (i = 0; i < num; ++i) {
>>>> +		r = vm_insert_page(vma, vma->vm_start + i * PAGE_SIZE,
>>>> +				   obj->pages[i]);
>>>> +		if (r < 0) {
>>>> +			if (i > 0)
>>>> +				zap_vma_ptes(vma, vma->vm_start, i * PAGE_SIZE);
>>>> +			return r;
>>>> +		}
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +}
>>> Why can't we just redirect to the underlying shmem mmap here (after
>>> argument checking)?
>> I don't know what that means, but looking at vc4 it seems I can use
>> drm_gem_mmap() to remove some boilerplate.
>>
>> int sdrm_drm_mmap(struct file *filp, struct vm_area_struct *vma)
>> {
>>      unsigned long vm_flags = vma->vm_flags;
>>      struct sdrm_gem_object *sobj;
>>      struct drm_gem_object *obj;
>>      size_t i, num;
>>      int r;
>>
>>      r = drm_gem_mmap(filp, vma);
>>      if (r)
>>          return r;
>>
>>      obj = vma->vm_private_data;
>>
>>      sobj = to_sdrm_bo(obj);
>>
>>      r = sdrm_gem_get_pages(obj);
>>      if (r < 0)
>>          return r;
>>
>>      /* prevent dmabuf-imported mmap to user-space */
>>      if (!obj->pages)
>>          return -EACCES;
>>
>>      /* drm_gem_mmap() sets flags that we don't want */
>>      vma->vm_flags = vm_flags | VM_DONTEXPAND;
>>      vma->vm_page_prot =
>> pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
>>
>>      num = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
>>      for (i = 0; i < num; ++i) {
>>          r = vm_insert_page(vma, vma->vm_start + i * PAGE_SIZE,
>>                     obj->pages[i]);
>>          if (r < 0) {
>>              if (i > 0)
>>                  zap_vma_ptes(vma, vma->vm_start, i * PAGE_SIZE);
>>              return r;
>>          }
>>      }
>>
>>      return 0;
>> }
>>
>> drm_gem_mmap() requires drm_driver->gem_vm_ops to be set:
>>
>> const struct vm_operations_struct sdrm_gem_vm_ops = {
>>      .open = drm_gem_vm_open,
>>      .close = drm_gem_vm_close,
>> };
>>
>> And browsing the code a bit more shows that tegra, udl, etnaviv and vgem
>> does the vm_insert_page() thing in the vm_operations_struct->fault()
>> handler.
>>
>> So maybe this makes sense for simpledrm as well:
>>
>> static int sdrm_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>> {
>>          struct drm_gem_object *obj = vma->vm_private_data;
>>      struct sdrm_gem_object *sobj = to_sdrm_bo(obj);
>>      loff_t num_pages;
>>      pgoff_t offset;
>>      int r;
>>
>>      if (!sobj->pages)
>>          return VM_FAULT_SIGBUS;
>>
>>      offset = ((unsigned long)vmf->virtual_address - vma->vm_start) >>
>>           PAGE_SHIFT;
>>      num_pages = DIV_ROUND_UP(obj->base.size, PAGE_SIZE);
>>      if (page_offset > num_pages)
>>          return VM_FAULT_SIGBUS;
>>
>>      r = vm_insert_page(vma, (unsigned long)vmf->virtual_address,
>>                 sobj->pages[offset]);
>>      switch (r) {
>>      case -EAGAIN:
>>      case 0:
>>      case -ERESTARTSYS:
>>      case -EINTR:
>>      case -EBUSY:
>>          return VM_FAULT_NOPAGE;
>>
>>      case -ENOMEM:
>>          return VM_FAULT_OOM;
>>      }
>>
>>      return VM_FAULT_SIGBUS;
>> }
> That's still a lot for what amounts to reimplementing mmap on shmem, but
> badly. What I mean with redirecting is pointing the entire ->mmap
> operation to the mmap implementation for the underlying mmap. Roughly:
>
> 	/* normal gem mmap checks first */
>
> 	/* redirect to shmem mmap */
> 	vma->vm_file = obj->filp;
> 	vma->vm_pgoff = 0;
>
> 	return obj->filp->f_op->mmap(obj->filp, vma);
>
> Much less code ;-)

obj->filp is NULL in my case.

And looking at the docs, that's expected since I have driver specific 
backing?

/**
  * @filp:
  *
  * SHMEM file node used as backing storage for swappable buffer objects.
  * GEM also supports driver private objects with driver-specific backing
  * storage (contiguous CMA memory, special reserved blocks). In this
  * case @filp is NULL.
  */


Noralf.

Daniel Vetter Aug. 17, 2016, 9:30 a.m. UTC | #5

On Tue, Aug 16, 2016 at 9:38 PM, Noralf Trønnes <[e-mail]> wrote:
>> That's still a lot for what amounts to reimplementing mmap on shmem, but
>> badly. What I mean with redirecting is pointing the entire ->mmap
>> operation to the mmap implementation for the underlying mmap. Roughly:
>>
>>         /* normal gem mmap checks first */
>>
>>         /* redirect to shmem mmap */
>>         vma->vm_file = obj->filp;
>>         vma->vm_pgoff = 0;
>>
>>         return obj->filp->f_op->mmap(obj->filp, vma);
>>
>> Much less code ;-)
>
>
> obj->filp is NULL in my case.
>
> And looking at the docs, that's expected since I have driver specific
> backing?
>
> /**
>  * @filp:
>  *
>  * SHMEM file node used as backing storage for swappable buffer objects.
>  * GEM also supports driver private objects with driver-specific backing
>  * storage (contiguous CMA memory, special reserved blocks). In this
>  * case @filp is NULL.
>  */

Hm, I totally misread the driver code. I assumed that we'd just
allocate normal shmem gem objects, and then copy them on-demand onto
the frontbuffer (in the dirty or plane update callbacks). Essentially
treat the firmware fb area as a manual upload display, except that we
don't use i2c or spi to do the upload, but normal mmio writes. I think
that would greatly simplify the driver, and more important: It would
also work like any other kms driver. Currently sdrm is violiting the
spec a bit by aliasing all dumb buffers to the same underlying backing
storage, and that's a bit evil.

The other bit I noticed (and why I was confused): The prime import
code reinvents a lot of wheels, and it digs into the backing storage
directly. Instead it should just call dma_buf_vmap/dma_buf_vunmap and
let the exporter figure out how it works.

Sorry I was all confused here and didn't realize what's going on :(
-Daniel

Noralf Trønnes Aug. 17, 2016, 10:19 a.m. UTC | #6

Den 17.08.2016 11:30, skrev Daniel Vetter:
> On Tue, Aug 16, 2016 at 9:38 PM, Noralf Trønnes <[e-mail]> wrote:
>>> That's still a lot for what amounts to reimplementing mmap on shmem, but
>>> badly. What I mean with redirecting is pointing the entire ->mmap
>>> operation to the mmap implementation for the underlying mmap. Roughly:
>>>
>>>          /* normal gem mmap checks first */
>>>
>>>          /* redirect to shmem mmap */
>>>          vma->vm_file = obj->filp;
>>>          vma->vm_pgoff = 0;
>>>
>>>          return obj->filp->f_op->mmap(obj->filp, vma);
>>>
>>> Much less code ;-)
>>
>> obj->filp is NULL in my case.
>>
>> And looking at the docs, that's expected since I have driver specific
>> backing?
>>
>> /**
>>   * @filp:
>>   *
>>   * SHMEM file node used as backing storage for swappable buffer objects.
>>   * GEM also supports driver private objects with driver-specific backing
>>   * storage (contiguous CMA memory, special reserved blocks). In this
>>   * case @filp is NULL.
>>   */
> Hm, I totally misread the driver code. I assumed that we'd just
> allocate normal shmem gem objects, and then copy them on-demand onto
> the frontbuffer (in the dirty or plane update callbacks). Essentially
> treat the firmware fb area as a manual upload display, except that we
> don't use i2c or spi to do the upload, but normal mmio writes. I think
> that would greatly simplify the driver, and more important: It would
> also work like any other kms driver. Currently sdrm is violiting the
> spec a bit by aliasing all dumb buffers to the same underlying backing
> storage, and that's a bit evil.
>
> The other bit I noticed (and why I was confused): The prime import
> code reinvents a lot of wheels, and it digs into the backing storage
> directly. Instead it should just call dma_buf_vmap/dma_buf_vunmap and
> let the exporter figure out how it works.

Is there a driver I can look at that does it the way we want?
I'm afraid that this gem framework is still over my head.
Copying code or patterns is easy, starting from scratch demands 
understanding :-)


Noralf.

Daniel Vetter Aug. 17, 2016, 10:58 a.m. UTC | #7

On Wed, Aug 17, 2016 at 12:19:02PM +0200, Noralf Trønnes wrote:
> 
> Den 17.08.2016 11:30, skrev Daniel Vetter:
> > On Tue, Aug 16, 2016 at 9:38 PM, Noralf Trønnes <[e-mail]> wrote:
> > > > That's still a lot for what amounts to reimplementing mmap on shmem, but
> > > > badly. What I mean with redirecting is pointing the entire ->mmap
> > > > operation to the mmap implementation for the underlying mmap. Roughly:
> > > > 
> > > >          /* normal gem mmap checks first */
> > > > 
> > > >          /* redirect to shmem mmap */
> > > >          vma->vm_file = obj->filp;
> > > >          vma->vm_pgoff = 0;
> > > > 
> > > >          return obj->filp->f_op->mmap(obj->filp, vma);
> > > > 
> > > > Much less code ;-)
> > > 
> > > obj->filp is NULL in my case.
> > > 
> > > And looking at the docs, that's expected since I have driver specific
> > > backing?
> > > 
> > > /**
> > >   * @filp:
> > >   *
> > >   * SHMEM file node used as backing storage for swappable buffer objects.
> > >   * GEM also supports driver private objects with driver-specific backing
> > >   * storage (contiguous CMA memory, special reserved blocks). In this
> > >   * case @filp is NULL.
> > >   */
> > Hm, I totally misread the driver code. I assumed that we'd just
> > allocate normal shmem gem objects, and then copy them on-demand onto
> > the frontbuffer (in the dirty or plane update callbacks). Essentially
> > treat the firmware fb area as a manual upload display, except that we
> > don't use i2c or spi to do the upload, but normal mmio writes. I think
> > that would greatly simplify the driver, and more important: It would
> > also work like any other kms driver. Currently sdrm is violiting the
> > spec a bit by aliasing all dumb buffers to the same underlying backing
> > storage, and that's a bit evil.
> > 
> > The other bit I noticed (and why I was confused): The prime import
> > code reinvents a lot of wheels, and it digs into the backing storage
> > directly. Instead it should just call dma_buf_vmap/dma_buf_vunmap and
> > let the exporter figure out how it works.
> 
> Is there a driver I can look at that does it the way we want?
> I'm afraid that this gem framework is still over my head.
> Copying code or patterns is easy, starting from scratch demands
> understanding :-)

udl_gem.c is pretty much what I had in mind, including the handling of
dma-bufs and all that. Not sure how much point there is in sharing code,
at least if you can't test udl.

What udl doesn't do is directly redirect the mmap to the shmem node (and
reject the mmap for dma-buf imported buffers, since that's not a good idea
really).
-Daniel

Patch

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index fc35731..a54cc8d 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -290,3 +290,5 @@  source "drivers/gpu/drm/arc/Kconfig"
 source "drivers/gpu/drm/hisilicon/Kconfig"

 source "drivers/gpu/drm/mediatek/Kconfig"
+
+source "drivers/gpu/drm/simpledrm/Kconfig"
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index e3dba6f..eba32ad 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -83,3 +83,4 @@  obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
 obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
 obj-$(CONFIG_DRM_ARCPGU)+= arc/
 obj-y			+= hisilicon/
+obj-$(CONFIG_DRM_SIMPLEDRM) += simpledrm/
diff --git a/drivers/gpu/drm/simpledrm/Kconfig b/drivers/gpu/drm/simpledrm/Kconfig
new file mode 100644
index 0000000..f45b25d
--- /dev/null
+++ b/drivers/gpu/drm/simpledrm/Kconfig
@@ -0,0 +1,19 @@ 
+config DRM_SIMPLEDRM
+	tristate "Simple firmware framebuffer DRM driver"
+	depends on DRM
+	select DRM_KMS_HELPER
+	help
+	  SimpleDRM can run on all systems with pre-initialized graphics
+	  hardware. It uses a framebuffer that was initialized during
+	  firmware boot. No page-flipping, modesetting or other advanced
+	  features are available. However, other DRM drivers can be loaded
+	  later and take over from SimpleDRM if they provide real hardware
+	  support.
+
+	  SimpleDRM supports "simple-framebuffer" DeviceTree objects and
+	  compatible platform framebuffers.
+
+	  If unsure, say Y.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called simpledrm.
diff --git a/drivers/gpu/drm/simpledrm/Makefile b/drivers/gpu/drm/simpledrm/Makefile
new file mode 100644
index 0000000..f6a62dc
--- /dev/null
+++ b/drivers/gpu/drm/simpledrm/Makefile
@@ -0,0 +1,4 @@ 
+simpledrm-y :=	simpledrm_drv.o simpledrm_kms.o simpledrm_gem.o \
+		simpledrm_damage.o
+
+obj-$(CONFIG_DRM_SIMPLEDRM) := simpledrm.o
diff --git a/drivers/gpu/drm/simpledrm/simpledrm.h b/drivers/gpu/drm/simpledrm/simpledrm.h
new file mode 100644
index 0000000..481acc2
--- /dev/null
+++ b/drivers/gpu/drm/simpledrm/simpledrm.h
@@ -0,0 +1,90 @@ 
+/*
+ * SimpleDRM firmware framebuffer driver
+ * Copyright (c) 2012-2014 David Herrmann <[e-mail]>
+ *
+ * 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.
+ */
+
+#ifndef SDRM_DRV_H
+#define SDRM_DRV_H
+
+#include <drm/drm_crtc.h>
+#include <drm/drm_gem.h>
+#include <drm/drm_simple_kms_helper.h>
+
+struct simplefb_format;
+struct regulator;
+struct fb_info;
+struct clk;
+
+struct sdrm_device {
+	struct drm_device *ddev;
+	struct drm_simple_display_pipe pipe;
+	struct drm_connector conn;
+
+	/* framebuffer information */
+	const struct simplefb_format *fb_sformat;
+	u32 fb_format;
+	u32 fb_width;
+	u32 fb_height;
+	u32 fb_stride;
+	u32 fb_bpp;
+	unsigned long fb_base;
+	unsigned long fb_size;
+	void *fb_map;
+
+#if defined CONFIG_OF && defined CONFIG_COMMON_CLK
+	unsigned int clk_count;
+	struct clk **clks;
+#endif
+#if defined CONFIG_OF && defined CONFIG_REGULATOR
+	u32 regulator_count;
+	struct regulator **regulators;
+#endif
+};
+
+int sdrm_drm_modeset_init(struct sdrm_device *sdrm);
+int sdrm_drm_mmap(struct file *filp, struct vm_area_struct *vma);
+
+int sdrm_dirty(struct drm_framebuffer *fb,
+	       struct drm_file *file,
+	       unsigned int flags, unsigned int color,
+	       struct drm_clip_rect *clips,
+	       unsigned int num_clips);
+int sdrm_dirty_all_locked(struct sdrm_device *sdrm);
+int sdrm_dirty_all_unlocked(struct sdrm_device *sdrm);
+
+struct sdrm_gem_object {
+	struct drm_gem_object base;
+	struct sg_table *sg;
+	struct page **pages;
+	void *vmapping;
+};
+
+#define to_sdrm_bo(x) container_of(x, struct sdrm_gem_object, base)
+
+struct sdrm_gem_object *sdrm_gem_alloc_object(struct drm_device *ddev,
+					      size_t size);
+struct drm_gem_object *sdrm_gem_prime_import(struct drm_device *ddev,
+					     struct dma_buf *dma_buf);
+void sdrm_gem_free_object(struct drm_gem_object *obj);
+int sdrm_gem_get_pages(struct sdrm_gem_object *obj);
+
+int sdrm_dumb_create(struct drm_file *file_priv, struct drm_device *ddev,
+		     struct drm_mode_create_dumb *arg);
+int sdrm_dumb_destroy(struct drm_file *file_priv, struct drm_device *ddev,
+		      uint32_t handle);
+int sdrm_dumb_map_offset(struct drm_file *file_priv, struct drm_device *ddev,
+			 uint32_t handle, uint64_t *offset);
+
+struct sdrm_framebuffer {
+	struct drm_framebuffer base;
+	struct sdrm_gem_object *obj;
+};
+
+#define to_sdrm_fb(x) container_of(x, struct sdrm_framebuffer, base)
+
+#endif /* SDRM_DRV_H */
diff --git a/drivers/gpu/drm/simpledrm/simpledrm_damage.c b/drivers/gpu/drm/simpledrm/simpledrm_damage.c
new file mode 100644
index 0000000..ee3d465
--- /dev/null
+++ b/drivers/gpu/drm/simpledrm/simpledrm_damage.c
@@ -0,0 +1,302 @@ 
+/*
+ * SimpleDRM firmware framebuffer driver
+ * Copyright (c) 2012-2014 David Herrmann <[e-mail]>
+ *
+ * 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.
+ */
+
+#include <asm/unaligned.h>
+#include <drm/drmP.h>
+#include <drm/drm_crtc.h>
+#include <linux/dma-buf.h>
+#include <linux/kernel.h>
+#include <linux/string.h>
+
+#include "simpledrm.h"
+
+static inline void sdrm_put(u8 *dst, u32 four_cc, u16 r, u16 g, u16 b)
+{
+	switch (four_cc) {
+	case DRM_FORMAT_RGB565:
+		r >>= 11;
+		g >>= 10;
+		b >>= 11;
+		put_unaligned((u16)((r << 11) | (g << 5) | b), (u16 *)dst);
+		break;
+	case DRM_FORMAT_XRGB1555:
+	case DRM_FORMAT_ARGB1555:
+		r >>= 11;
+		g >>= 11;
+		b >>= 11;
+		put_unaligned((u16)((r << 10) | (g << 5) | b), (u16 *)dst);
+		break;
+	case DRM_FORMAT_RGB888:
+		r >>= 8;
+		g >>= 8;
+		b >>= 8;
+#ifdef __LITTLE_ENDIAN
+		dst[2] = r;
+		dst[1] = g;
+		dst[0] = b;
+#elif defined(__BIG_ENDIAN)
+		dst[0] = r;
+		dst[1] = g;
+		dst[2] = b;
+#endif
+		break;
+	case DRM_FORMAT_XRGB8888:
+	case DRM_FORMAT_ARGB8888:
+		r >>= 8;
+		g >>= 8;
+		b >>= 8;
+		put_unaligned((u32)((r << 16) | (g << 8) | b), (u32 *)dst);
+		break;
+	case DRM_FORMAT_ABGR8888:
+		r >>= 8;
+		g >>= 8;
+		b >>= 8;
+		put_unaligned((u32)((b << 16) | (g << 8) | r), (u32 *)dst);
+		break;
+	case DRM_FORMAT_XRGB2101010:
+	case DRM_FORMAT_ARGB2101010:
+		r >>= 4;
+		g >>= 4;
+		b >>= 4;
+		put_unaligned((u32)((r << 20) | (g << 10) | b), (u32 *)dst);
+		break;
+	}
+}
+
+static void sdrm_blit_from_xrgb8888(const u8 *src, u32 src_stride, u32 src_bpp,
+				    u8 *dst, u32 dst_stride, u32 dst_bpp,
+				    u32 dst_four_cc, u32 width, u32 height)
+{
+	u32 val, i;
+
+	while (height--) {
+		for (i = 0; i < width; ++i) {
+			val = get_unaligned((const u32 *)&src[i * src_bpp]);
+			sdrm_put(&dst[i * dst_bpp], dst_four_cc,
+				 (val & 0x00ff0000U) >> 8,
+				 (val & 0x0000ff00U),
+				 (val & 0x000000ffU) << 8);
+		}
+
+		src += src_stride;
+		dst += dst_stride;
+	}
+}
+
+static void sdrm_blit_from_rgb565(const u8 *src, u32 src_stride, u32 src_bpp,
+				  u8 *dst, u32 dst_stride, u32 dst_bpp,
+				  u32 dst_four_cc, u32 width, u32 height)
+{
+	u32 val, i;
+
+	while (height--) {
+		for (i = 0; i < width; ++i) {
+			val = get_unaligned((const u16 *)&src[i * src_bpp]);
+			sdrm_put(&dst[i * dst_bpp], dst_four_cc,
+				 (val & 0xf800),
+				 (val & 0x07e0) << 5,
+				 (val & 0x001f) << 11);
+		}
+
+		src += src_stride;
+		dst += dst_stride;
+	}
+}
+
+static void sdrm_blit_lines(const u8 *src, u32 src_stride,
+			    u8 *dst, u32 dst_stride,
+			    u32 bpp, u32 width, u32 height)
+{
+	u32 len;
+
+	len = width * bpp;
+
+	while (height--) {
+		memcpy(dst, src, len);
+		src += src_stride;
+		dst += dst_stride;
+	}
+}
+
+static void sdrm_blit(struct sdrm_framebuffer *sfb, u32 x, u32 y,
+		      u32 width, u32 height)
+{
+	struct drm_framebuffer *fb = &sfb->base;
+	struct drm_device *ddev = fb->dev;
+	struct sdrm_device *sdrm = ddev->dev_private;
+	u32 src_bpp, dst_bpp, x2, y2;
+	u8 *src, *dst;
+
+	/* already unmapped; ongoing handover? */
+	if (!sdrm->fb_map)
+		return;
+
+	/* empty dirty-region, nothing to do */
+	if (!width || !height)
+		return;
+	if (x >= fb->width || y >= fb->height)
+		return;
+
+	/* sanity checks */
+	if (x + width < x)
+		width = fb->width - x;
+	if (y + height < y)
+		height = fb->height - y;
+
+	/* get intersection of dirty region and scan-out region */
+	x2 = min(x + width, sdrm->fb_width);
+	y2 = min(y + height, sdrm->fb_height);
+	if (x2 <= x || y2 <= y)
+		return;
+	width = x2 - x;
+	height = y2 - y;
+
+	/* get buffer offsets */
+	src = sfb->obj->vmapping;
+	dst = sdrm->fb_map;
+
+	/* bo is guaranteed to be big enough; size checks not needed */
+	src_bpp = (fb->bits_per_pixel + 7) / 8;
+	src += fb->offsets[0] + y * fb->pitches[0] + x * src_bpp;
+
+	dst_bpp = (sdrm->fb_bpp + 7) / 8;
+	dst += y * sdrm->fb_stride + x * dst_bpp;
+
+	/* if formats are identical, do a line-by-line copy.. */
+	if (fb->pixel_format == sdrm->fb_format) {
+		sdrm_blit_lines(src, fb->pitches[0],
+				dst, sdrm->fb_stride,
+				src_bpp, width, height);
+		return;
+	}
+
+	/* ..otherwise call slow blit-function */
+	switch (fb->pixel_format) {
+	case DRM_FORMAT_ARGB8888:
+		/* fallthrough */
+	case DRM_FORMAT_XRGB8888:
+		sdrm_blit_from_xrgb8888(src, fb->pitches[0], src_bpp,
+					dst, sdrm->fb_stride, dst_bpp,
+					sdrm->fb_format, width, height);
+		break;
+	case DRM_FORMAT_RGB565:
+		sdrm_blit_from_rgb565(src, fb->pitches[0], src_bpp,
+				      dst, sdrm->fb_stride, dst_bpp,
+				      sdrm->fb_format, width, height);
+		break;
+	}
+}
+
+static int sdrm_begin_access(struct sdrm_framebuffer *sfb)
+{
+	int r;
+
+	r = sdrm_gem_get_pages(sfb->obj);
+	if (r)
+		return r;
+
+	if (!sfb->obj->base.import_attach)
+		return 0;
+
+	return dma_buf_begin_cpu_access(sfb->obj->base.import_attach->dmabuf,
+					DMA_FROM_DEVICE);
+}
+
+static void sdrm_end_access(struct sdrm_framebuffer *sfb)
+{
+	if (!sfb->obj->base.import_attach)
+		return;
+
+	dma_buf_end_cpu_access(sfb->obj->base.import_attach->dmabuf,
+			       DMA_FROM_DEVICE);
+}
+
+int sdrm_dirty(struct drm_framebuffer *fb,
+	       struct drm_file *file,
+	       unsigned int flags, unsigned int color,
+	       struct drm_clip_rect *clips,
+	       unsigned int num_clips)
+{
+	struct sdrm_framebuffer *sfb = to_sdrm_fb(fb);
+	struct drm_device *ddev = fb->dev;
+	struct sdrm_device *sdrm = ddev->dev_private;
+	struct drm_clip_rect full_clip;
+	unsigned int i;
+	int r;
+
+	if (!clips || !num_clips) {
+		full_clip.x1 = 0;
+		full_clip.x2 = fb->width;
+		full_clip.y1 = 0;
+		full_clip.y2 = fb->height;
+		clips = &full_clip;
+		num_clips = 1;
+	}
+
+	drm_modeset_lock_all(ddev);
+
+	if (sdrm->pipe.plane.fb != fb) {
+		r = 0;
+		goto unlock;
+	}
+
+	r = sdrm_begin_access(sfb);
+	if (r)
+		goto unlock;
+
+	for (i = 0; i < num_clips; i++) {
+		if (clips[i].x2 <= clips[i].x1 ||
+		    clips[i].y2 <= clips[i].y1)
+			continue;
+
+		sdrm_blit(sfb, clips[i].x1, clips[i].y1,
+			  clips[i].x2 - clips[i].x1,
+			  clips[i].y2 - clips[i].y1);
+	}
+
+	sdrm_end_access(sfb);
+
+unlock:
+	drm_modeset_unlock_all(ddev);
+	return 0;
+}
+
+int sdrm_dirty_all_locked(struct sdrm_device *sdrm)
+{
+	struct drm_framebuffer *fb;
+	struct sdrm_framebuffer *sfb;
+	int r;
+
+	fb = sdrm->pipe.plane.fb;
+	if (!fb)
+		return 0;
+
+	sfb = to_sdrm_fb(fb);
+	r = sdrm_begin_access(sfb);
+	if (r)
+		return r;
+
+	sdrm_blit(sfb, 0, 0, fb->width, fb->height);
+
+	sdrm_end_access(sfb);
+
+	return 0;
+}
+
+int sdrm_dirty_all_unlocked(struct sdrm_device *sdrm)
+{
+	int r;
+
+	drm_modeset_lock_all(sdrm->ddev);
+	r = sdrm_dirty_all_locked(sdrm);
+	drm_modeset_unlock_all(sdrm->ddev);
+
+	return r;
+}
diff --git a/drivers/gpu/drm/simpledrm/simpledrm_drv.c b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
new file mode 100644
index 0000000..e5b6f75
--- /dev/null
+++ b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
@@ -0,0 +1,539 @@ 
+/*
+ * SimpleDRM firmware framebuffer driver
+ * Copyright (c) 2012-2014 David Herrmann <[e-mail]>
+ *
+ * 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.
+ */
+
+#include <drm/drmP.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_data/simplefb.h>
+#include <linux/regulator/consumer.h>
+#include <linux/string.h>
+
+#include "simpledrm.h"
+
+static const struct file_operations sdrm_drm_fops = {
+	.owner = THIS_MODULE,
+	.open = drm_open,
+	.mmap = sdrm_drm_mmap,
+	.poll = drm_poll,
+	.read = drm_read,
+	.unlocked_ioctl = drm_ioctl,
+	.release = drm_release,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl = drm_compat_ioctl,
+#endif
+	.llseek = noop_llseek,
+};
+
+static struct drm_driver sdrm_drm_driver = {
+	.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
+			   DRIVER_ATOMIC,
+	.fops = &sdrm_drm_fops,
+
+	.gem_free_object = sdrm_gem_free_object,
+	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
+	.gem_prime_import = sdrm_gem_prime_import,
+
+	.dumb_create = sdrm_dumb_create,
+	.dumb_map_offset = sdrm_dumb_map_offset,
+	.dumb_destroy = sdrm_dumb_destroy,
+
+	.name = "simpledrm",
+	.desc = "Simple firmware framebuffer DRM driver",
+	.date = "20130601",
+	.major = 0,
+	.minor = 0,
+	.patchlevel = 1,
+};
+
+#if defined CONFIG_OF && defined CONFIG_COMMON_CLK
+/*
+ * Clock handling code.
+ *
+ * Here we handle the clocks property of our "simple-framebuffer" dt node.
+ * This is necessary so that we can make sure that any clocks needed by
+ * the display engine that the bootloader set up for us (and for which it
+ * provided a simplefb dt node), stay up, for the life of the simplefb
+ * driver.
+ *
+ * When the driver unloads, we cleanly disable, and then release the clocks.
+ *
+ * We only complain about errors here, no action is taken as the most likely
+ * error can only happen due to a mismatch between the bootloader which set
+ * up simplefb, and the clock definitions in the device tree. Chances are
+ * that there are no adverse effects, and if there are, a clean teardown of
+ * the fb probe will not help us much either. So just complain and carry on,
+ * and hope that the user actually gets a working fb at the end of things.
+ */
+static int sdrm_clocks_init(struct sdrm_device *sdrm,
+			    struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct clk *clock;
+	int i, ret;
+
+	if (dev_get_platdata(&pdev->dev) || !np)
+		return 0;
+
+	sdrm->clk_count = of_clk_get_parent_count(np);
+	if (!sdrm->clk_count)
+		return 0;
+
+	sdrm->clks = kcalloc(sdrm->clk_count, sizeof(struct clk *), GFP_KERNEL);
+	if (!sdrm->clks)
+		return -ENOMEM;
+
+	for (i = 0; i < sdrm->clk_count; i++) {
+		clock = of_clk_get(np, i);
+		if (IS_ERR(clock)) {
+			if (PTR_ERR(clock) == -EPROBE_DEFER) {
+				while (--i >= 0) {
+					if (sdrm->clks[i])
+						clk_put(sdrm->clks[i]);
+				}
+				kfree(sdrm->clks);
+				return -EPROBE_DEFER;
+			}
+			dev_err(&pdev->dev, "%s: clock %d not found: %ld\n",
+				__func__, i, PTR_ERR(clock));
+			continue;
+		}
+		sdrm->clks[i] = clock;
+	}
+
+	for (i = 0; i < sdrm->clk_count; i++) {
+		if (sdrm->clks[i]) {
+			ret = clk_prepare_enable(sdrm->clks[i]);
+			if (ret) {
+				dev_err(&pdev->dev,
+					"%s: failed to enable clock %d: %d\n",
+					__func__, i, ret);
+				clk_put(sdrm->clks[i]);
+				sdrm->clks[i] = NULL;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static void sdrm_clocks_destroy(struct sdrm_device *sdrm)
+{
+	int i;
+
+	if (!sdrm->clks)
+		return;
+
+	for (i = 0; i < sdrm->clk_count; i++) {
+		if (sdrm->clks[i]) {
+			clk_disable_unprepare(sdrm->clks[i]);
+			clk_put(sdrm->clks[i]);
+		}
+	}
+
+	kfree(sdrm->clks);
+}
+#else
+static int sdrm_clocks_init(struct sdrm_device *sdrm,
+			    struct platform_device *pdev)
+{
+	return 0;
+}
+
+static void sdrm_clocks_destroy(struct sdrm_device *sdrm)
+{
+}
+#endif
+
+#if defined CONFIG_OF && defined CONFIG_REGULATOR
+
+#define SUPPLY_SUFFIX "-supply"
+
+/*
+ * Regulator handling code.
+ *
+ * Here we handle the num-supplies and vin*-supply properties of our
+ * "simple-framebuffer" dt node. This is necessary so that we can make sure
+ * that any regulators needed by the display hardware that the bootloader
+ * set up for us (and for which it provided a simplefb dt node), stay up,
+ * for the life of the simplefb driver.
+ *
+ * When the driver unloads, we cleanly disable, and then release the
+ * regulators.
+ *
+ * We only complain about errors here, no action is taken as the most likely
+ * error can only happen due to a mismatch between the bootloader which set
+ * up simplefb, and the regulator definitions in the device tree. Chances are
+ * that there are no adverse effects, and if there are, a clean teardown of
+ * the fb probe will not help us much either. So just complain and carry on,
+ * and hope that the user actually gets a working fb at the end of things.
+ */
+static int sdrm_regulators_init(struct sdrm_device *sdrm,
+				struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct regulator *regulator;
+	int count = 0, i = 0, ret;
+	struct property *prop;
+	const char *p;
+
+	if (dev_get_platdata(&pdev->dev) || !np)
+		return 0;
+
+	/* Count the number of regulator supplies */
+	for_each_property_of_node(np, prop) {
+		p = strstr(prop->name, SUPPLY_SUFFIX);
+		if (p && p != prop->name)
+			count++;
+	}
+
+	if (!count)
+		return 0;
+
+	sdrm->regulators = devm_kcalloc(&pdev->dev, count,
+					sizeof(struct regulator *),
+					GFP_KERNEL);
+	if (!sdrm->regulators)
+		return -ENOMEM;
+
+	/* Get all the regulators */
+	for_each_property_of_node(np, prop) {
+		char name[32]; /* 32 is max size of property name */
+
+		p = strstr(prop->name, SUPPLY_SUFFIX);
+		if (!p || p == prop->name)
+			continue;
+
+		strlcpy(name, prop->name,
+			strlen(prop->name) - strlen(SUPPLY_SUFFIX) + 1);
+		regulator = devm_regulator_get_optional(&pdev->dev, name);
+		if (IS_ERR(regulator)) {
+			if (PTR_ERR(regulator) == -EPROBE_DEFER)
+				return -EPROBE_DEFER;
+			dev_err(&pdev->dev, "regulator %s not found: %ld\n",
+				name, PTR_ERR(regulator));
+			continue;
+		}
+		sdrm->regulators[i++] = regulator;
+	}
+	sdrm->regulator_count = i;
+
+	/* Enable all the regulators */
+	for (i = 0; i < sdrm->regulator_count; i++) {
+		ret = regulator_enable(sdrm->regulators[i]);
+		if (ret) {
+			dev_err(&pdev->dev,
+				"failed to enable regulator %d: %d\n",
+				i, ret);
+			devm_regulator_put(sdrm->regulators[i]);
+			sdrm->regulators[i] = NULL;
+		}
+	}
+
+	return 0;
+}
+
+static void sdrm_regulators_destroy(struct sdrm_device *sdrm)
+{
+	int i;
+
+	if (!sdrm->regulators)
+		return;
+
+	for (i = 0; i < sdrm->regulator_count; i++)
+		if (sdrm->regulators[i])
+			regulator_disable(sdrm->regulators[i]);
+}
+#else
+static int sdrm_regulators_init(struct sdrm_device *sdrm,
+				struct platform_device *pdev)
+{
+	return 0;
+}
+
+static void sdrm_regulators_destroy(struct sdrm_device *sdrm)
+{
+}
+#endif
+
+static int parse_dt(struct platform_device *pdev,
+		    struct simplefb_platform_data *mode)
+{
+	struct device_node *np = pdev->dev.of_node;
+	const char *format;
+	int ret;
+
+	if (!np)
+		return -ENODEV;
+
+	ret = of_property_read_u32(np, "width", &mode->width);
+	if (ret) {
+		dev_err(&pdev->dev, "Can't parse width property\n");
+		return ret;
+	}
+
+	ret = of_property_read_u32(np, "height", &mode->height);
+	if (ret) {
+		dev_err(&pdev->dev, "Can't parse height property\n");
+		return ret;
+	}
+
+	ret = of_property_read_u32(np, "stride", &mode->stride);
+	if (ret) {
+		dev_err(&pdev->dev, "Can't parse stride property\n");
+		return ret;
+	}
+
+	ret = of_property_read_string(np, "format", &format);
+	if (ret) {
+		dev_err(&pdev->dev, "Can't parse format property\n");
+		return ret;
+	}
+	mode->format = format;
+
+	return 0;
+}
+
+static struct simplefb_format simplefb_formats[] = SIMPLEFB_FORMATS;
+
+int sdrm_pdev_init(struct sdrm_device *sdrm)
+{
+	struct platform_device *pdev = sdrm->ddev->platformdev;
+	struct simplefb_platform_data *mode = pdev->dev.platform_data;
+	struct simplefb_platform_data pmode;
+	struct resource *mem;
+	unsigned int depth;
+	int ret, i, bpp;
+
+	if (!mode) {
+		mode = &pmode;
+		ret = parse_dt(pdev, mode);
+		if (ret)
+			return ret;
+	}
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem) {
+		dev_err(sdrm->ddev->dev, "No memory resource\n");
+		return -ENODEV;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(simplefb_formats); ++i) {
+		if (strcmp(mode->format, simplefb_formats[i].name))
+			continue;
+
+		sdrm->fb_sformat = &simplefb_formats[i];
+		sdrm->fb_format = simplefb_formats[i].fourcc;
+		sdrm->fb_width = mode->width;
+		sdrm->fb_height = mode->height;
+		sdrm->fb_stride = mode->stride;
+		sdrm->fb_base = mem->start;
+		sdrm->fb_size = resource_size(mem);
+		break;
+	}
+
+	if (i >= ARRAY_SIZE(simplefb_formats)) {
+		dev_err(sdrm->ddev->dev, "Unknown format %s\n", mode->format);
+		return -ENODEV;
+	}
+
+	switch (sdrm->fb_format) {
+	case DRM_FORMAT_RGB565:
+	case DRM_FORMAT_XRGB1555:
+	case DRM_FORMAT_ARGB1555:
+	case DRM_FORMAT_RGB888:
+	case DRM_FORMAT_XRGB8888:
+	case DRM_FORMAT_ARGB8888:
+	case DRM_FORMAT_ABGR8888:
+	case DRM_FORMAT_XRGB2101010:
+	case DRM_FORMAT_ARGB2101010:
+		/*
+		 * You must adjust sdrm_put() whenever you add a new format
+		 * here, otherwise, blitting operations will not work.
+		 * Furthermore, include/linux/platform_data/simplefb.h needs
+		 * to be adjusted so the platform-device actually allows this
+		 * format.
+		 */
+		break;
+	default:
+		dev_err(sdrm->ddev->dev, "Unsupported format %s\n",
+			mode->format);
+		return -ENODEV;
+	}
+
+	drm_fb_get_bpp_depth(sdrm->fb_format, &depth, &bpp);
+	if (!bpp) {
+		dev_err(sdrm->ddev->dev, "Unknown format %s\n", mode->format);
+		return -ENODEV;
+	}
+
+	if (sdrm->fb_size < sdrm->fb_stride * sdrm->fb_height) {
+		dev_err(sdrm->ddev->dev, "FB too small\n");
+		return -ENODEV;
+	} else if ((bpp + 7) / 8 * sdrm->fb_width > sdrm->fb_stride) {
+		dev_err(sdrm->ddev->dev, "Invalid stride\n");
+		return -ENODEV;
+	}
+
+	sdrm->fb_bpp = bpp;
+
+	sdrm->fb_map = ioremap_wc(sdrm->fb_base, sdrm->fb_size);
+	if (!sdrm->fb_map) {
+		dev_err(sdrm->ddev->dev, "cannot remap VMEM\n");
+		return -EIO;
+	}
+
+	DRM_DEBUG_KMS("format: %s\n", drm_get_format_name(sdrm->fb_format));
+
+	return 0;
+}
+
+void sdrm_pdev_destroy(struct sdrm_device *sdrm)
+{
+	if (sdrm->fb_map) {
+		iounmap(sdrm->fb_map);
+		sdrm->fb_map = NULL;
+	}
+}
+
+static int sdrm_simplefb_probe(struct platform_device *pdev)
+{
+	struct sdrm_device *sdrm;
+	struct drm_device *ddev;
+	int ret;
+
+	ddev = drm_dev_alloc(&sdrm_drm_driver, &pdev->dev);
+	if (!ddev)
+		return -ENOMEM;
+
+	sdrm = kzalloc(sizeof(*sdrm), GFP_KERNEL);
+	if (!sdrm)
+		goto err_free;
+
+	ddev->platformdev = pdev;
+	ddev->dev_private = sdrm;
+	sdrm->ddev = ddev;
+
+	ret = sdrm_pdev_init(sdrm);
+	if (ret)
+		goto err_free;
+
+	ret = sdrm_drm_modeset_init(sdrm);
+	if (ret)
+		goto err_destroy;
+
+	ret = sdrm_clocks_init(sdrm, pdev);
+	if (ret < 0)
+		goto err_cleanup;
+
+	ret = sdrm_regulators_init(sdrm, pdev);
+	if (ret < 0)
+		goto err_clocks;
+
+	platform_set_drvdata(pdev, ddev);
+	ret = drm_dev_register(ddev, 0);
+	if (ret)
+		goto err_regulators;
+
+	DRM_INFO("Initialized %s on minor %d\n", ddev->driver->name,
+		 ddev->primary->index);
+
+	return 0;
+
+err_regulators:
+	sdrm_regulators_destroy(sdrm);
+err_clocks:
+	sdrm_clocks_destroy(sdrm);
+err_cleanup:
+	drm_mode_config_cleanup(ddev);
+err_destroy:
+	sdrm_pdev_destroy(sdrm);
+err_free:
+	drm_dev_unref(ddev);
+	kfree(sdrm);
+
+	return ret;
+}
+
+static int sdrm_simplefb_remove(struct platform_device *pdev)
+{
+	struct drm_device *ddev = platform_get_drvdata(pdev);
+	struct sdrm_device *sdrm = ddev->dev_private;
+
+	drm_dev_unregister(ddev);
+	drm_mode_config_cleanup(ddev);
+
+	/* protect fb_map removal against sdrm_blit() */
+	drm_modeset_lock_all(ddev);
+	sdrm_pdev_destroy(sdrm);
+	drm_modeset_unlock_all(ddev);
+
+	sdrm_regulators_destroy(sdrm);
+	sdrm_clocks_destroy(sdrm);
+
+	drm_dev_unref(ddev);
+	kfree(sdrm);
+
+	return 0;
+}
+
+static const struct of_device_id simplefb_of_match[] = {
+	{ .compatible = "simple-framebuffer", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, simplefb_of_match);
+
+static struct platform_driver sdrm_simplefb_driver = {
+	.probe = sdrm_simplefb_probe,
+	.remove = sdrm_simplefb_remove,
+	.driver = {
+		.name = "simple-framebuffer",
+		.mod_name = KBUILD_MODNAME,
+		.owner = THIS_MODULE,
+		.of_match_table = simplefb_of_match,
+	},
+};
+
+static int __init sdrm_init(void)
+{
+	int ret;
+	struct device_node *np;
+
+	ret = platform_driver_register(&sdrm_simplefb_driver);
+	if (ret)
+		return ret;
+
+	if (IS_ENABLED(CONFIG_OF_ADDRESS) && of_chosen) {
+		for_each_child_of_node(of_chosen, np) {
+			if (of_device_is_compatible(np, "simple-framebuffer"))
+				of_platform_device_create(np, NULL, NULL);
+		}
+	}
+
+	return 0;
+}
+module_init(sdrm_init);
+
+static void __exit sdrm_exit(void)
+{
+	platform_driver_unregister(&sdrm_simplefb_driver);
+}
+module_exit(sdrm_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("David Herrmann <[e-mail]>");
+MODULE_DESCRIPTION("Simple firmware framebuffer DRM driver");
+MODULE_ALIAS("platform:simple-framebuffer");
diff --git a/drivers/gpu/drm/simpledrm/simpledrm_gem.c b/drivers/gpu/drm/simpledrm/simpledrm_gem.c
new file mode 100644
index 0000000..81bd7c5
--- /dev/null
+++ b/drivers/gpu/drm/simpledrm/simpledrm_gem.c
@@ -0,0 +1,274 @@ 
+/*
+ * SimpleDRM firmware framebuffer driver
+ * Copyright (c) 2012-2014 David Herrmann <[e-mail]>
+ *
+ * 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.
+ */
+
+#include <drm/drmP.h>
+#include <linux/dma-buf.h>
+#include <linux/errno.h>
+#include <linux/mm.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/vmalloc.h>
+
+#include "simpledrm.h"
+
+int sdrm_gem_get_pages(struct sdrm_gem_object *obj)
+{
+	size_t num, i;
+
+	if (obj->vmapping)
+		return 0;
+
+	if (obj->base.import_attach) {
+		obj->vmapping = dma_buf_vmap(obj->base.import_attach->dmabuf);
+		return !obj->vmapping ? -ENOMEM : 0;
+	}
+
+	num = obj->base.size >> PAGE_SHIFT;
+	obj->pages = drm_malloc_ab(num, sizeof(*obj->pages));
+	if (!obj->pages)
+		return -ENOMEM;
+
+	for (i = 0; i < num; ++i) {
+		obj->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO);
+		if (!obj->pages[i])
+			goto error;
+	}
+
+	obj->vmapping = vmap(obj->pages, num, 0, PAGE_KERNEL);
+	if (!obj->vmapping)
+		goto error;
+
+	return 0;
+
+error:
+	while (i > 0)
+		__free_pages(obj->pages[--i], 0);
+
+	drm_free_large(obj->pages);
+	obj->pages = NULL;
+	return -ENOMEM;
+}
+
+static void sdrm_gem_put_pages(struct sdrm_gem_object *obj)
+{
+	size_t num, i;
+
+	if (!obj->vmapping)
+		return;
+
+	if (obj->base.import_attach) {
+		dma_buf_vunmap(obj->base.import_attach->dmabuf, obj->vmapping);
+		obj->vmapping = NULL;
+		return;
+	}
+
+	vunmap(obj->vmapping);
+	obj->vmapping = NULL;
+
+	num = obj->base.size >> PAGE_SHIFT;
+	for (i = 0; i < num; ++i)
+		__free_pages(obj->pages[i], 0);
+
+	drm_free_large(obj->pages);
+	obj->pages = NULL;
+}
+
+struct sdrm_gem_object *sdrm_gem_alloc_object(struct drm_device *ddev,
+					      size_t size)
+{
+	struct sdrm_gem_object *obj;
+
+	WARN_ON(!size || (size & ~PAGE_MASK) != 0);
+
+	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
+	if (!obj)
+		return NULL;
+
+	drm_gem_private_object_init(ddev, &obj->base, size);
+	return obj;
+}
+
+void sdrm_gem_free_object(struct drm_gem_object *gobj)
+{
+	struct sdrm_gem_object *obj = to_sdrm_bo(gobj);
+	struct drm_device *ddev = gobj->dev;
+
+	if (obj->pages) {
+		/* kill all user-space mappings */
+		drm_vma_node_unmap(&gobj->vma_node,
+				   ddev->anon_inode->i_mapping);
+		sdrm_gem_put_pages(obj);
+	}
+
+	if (gobj->import_attach)
+		drm_prime_gem_destroy(gobj, obj->sg);
+
+	drm_gem_free_mmap_offset(gobj);
+	drm_gem_object_release(gobj);
+	kfree(obj);
+}
+
+int sdrm_dumb_create(struct drm_file *dfile, struct drm_device *ddev,
+		     struct drm_mode_create_dumb *args)
+{
+	struct sdrm_gem_object *obj;
+	int r;
+
+	if (args->flags)
+		return -EINVAL;
+
+	/* overflow checks are done by DRM core */
+	args->pitch = (args->bpp + 7) / 8 * args->width;
+	args->size = PAGE_ALIGN(args->pitch * args->height);
+
+	obj = sdrm_gem_alloc_object(ddev, args->size);
+	if (!obj)
+		return -ENOMEM;
+
+	r = drm_gem_handle_create(dfile, &obj->base, &args->handle);
+	if (r) {
+		drm_gem_object_unreference_unlocked(&obj->base);
+		return r;
+	}
+
+	/* handle owns a reference */
+	drm_gem_object_unreference_unlocked(&obj->base);
+	return 0;
+}
+
+int sdrm_dumb_destroy(struct drm_file *dfile, struct drm_device *ddev,
+		      uint32_t handle)
+{
+	return drm_gem_handle_delete(dfile, handle);
+}
+
+int sdrm_dumb_map_offset(struct drm_file *dfile, struct drm_device *ddev,
+			 uint32_t handle, uint64_t *offset)
+{
+	struct drm_gem_object *gobj;
+	int r;
+
+	mutex_lock(&ddev->struct_mutex);
+
+	gobj = drm_gem_object_lookup(dfile, handle);
+	if (!gobj) {
+		r = -ENOENT;
+		goto out_unlock;
+	}
+
+	r = drm_gem_create_mmap_offset(gobj);
+	if (r)
+		goto out_unref;
+
+	*offset = drm_vma_node_offset_addr(&gobj->vma_node);
+
+out_unref:
+	drm_gem_object_unreference(gobj);
+out_unlock:
+	mutex_unlock(&ddev->struct_mutex);
+	return r;
+}
+
+int sdrm_drm_mmap(struct file *filp, struct vm_area_struct *vma)
+{
+	struct drm_file *priv = filp->private_data;
+	struct drm_device *dev = priv->minor->dev;
+	struct drm_vma_offset_node *node;
+	struct drm_gem_object *gobj;
+	struct sdrm_gem_object *obj;
+	size_t size, i, num;
+	int r;
+
+	if (drm_device_is_unplugged(dev))
+		return -ENODEV;
+
+	drm_vma_offset_lock_lookup(dev->vma_offset_manager);
+	node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
+						  vma->vm_pgoff,
+						  vma_pages(vma));
+	drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
+
+	if (!drm_vma_node_is_allowed(node, filp))
+		return -EACCES;
+
+	gobj = container_of(node, struct drm_gem_object, vma_node);
+	obj = to_sdrm_bo(gobj);
+	size = drm_vma_node_size(node) << PAGE_SHIFT;
+	if (size < vma->vm_end - vma->vm_start)
+		return r;
+
+	r = sdrm_gem_get_pages(obj);
+	if (r < 0)
+		return r;
+
+	/* prevent dmabuf-imported mmap to user-space */
+	if (!obj->pages)
+		return -EACCES;
+
+	vma->vm_flags |= VM_DONTEXPAND;
+	vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+
+	num = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
+	for (i = 0; i < num; ++i) {
+		r = vm_insert_page(vma, vma->vm_start + i * PAGE_SIZE,
+				   obj->pages[i]);
+		if (r < 0) {
+			if (i > 0)
+				zap_vma_ptes(vma, vma->vm_start, i * PAGE_SIZE);
+			return r;
+		}
+	}
+
+	return 0;
+}
+
+struct drm_gem_object *sdrm_gem_prime_import(struct drm_device *ddev,
+					     struct dma_buf *dma_buf)
+{
+	struct dma_buf_attachment *attach;
+	struct sdrm_gem_object *obj;
+	struct sg_table *sg;
+	int ret;
+
+	/* need to attach */
+	attach = dma_buf_attach(dma_buf, ddev->dev);
+	if (IS_ERR(attach))
+		return ERR_CAST(attach);
+
+	get_dma_buf(dma_buf);
+
+	sg = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
+	if (IS_ERR(sg)) {
+		ret = PTR_ERR(sg);
+		goto fail_detach;
+	}
+
+	/*
+	 * dma_buf_vmap() gives us a page-aligned mapping, so lets bump the
+	 * size of the dma-buf to the next page-boundary
+	 */
+	obj = sdrm_gem_alloc_object(ddev, PAGE_ALIGN(dma_buf->size));
+	if (!obj) {
+		ret = -ENOMEM;
+		goto fail_unmap;
+	}
+
+	obj->sg = sg;
+	obj->base.import_attach = attach;
+
+	return &obj->base;
+
+fail_unmap:
+	dma_buf_unmap_attachment(attach, sg, DMA_BIDIRECTIONAL);
+fail_detach:
+	dma_buf_detach(dma_buf, attach);
+	dma_buf_put(dma_buf);
+	return ERR_PTR(ret);
+}
diff --git a/drivers/gpu/drm/simpledrm/simpledrm_kms.c b/drivers/gpu/drm/simpledrm/simpledrm_kms.c
new file mode 100644
index 0000000..00e50d9
--- /dev/null
+++ b/drivers/gpu/drm/simpledrm/simpledrm_kms.c
@@ -0,0 +1,258 @@ 
+/*
+ * SimpleDRM firmware framebuffer driver
+ * Copyright (c) 2012-2014 David Herrmann <[e-mail]>
+ *
+ * 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.
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_gem.h>
+#include <drm/drm_simple_kms_helper.h>
+#include <linux/slab.h>
+
+#include "simpledrm.h"
+
+static const uint32_t sdrm_formats[] = {
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_XRGB8888,
+};
+
+static int sdrm_conn_get_modes(struct drm_connector *conn)
+{
+	struct sdrm_device *sdrm = conn->dev->dev_private;
+	struct drm_display_mode *mode;
+
+	mode = drm_cvt_mode(sdrm->ddev, sdrm->fb_width, sdrm->fb_height,
+			    60, false, false, false);
+	if (!mode)
+		return 0;
+
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+	drm_mode_set_name(mode);
+	drm_mode_probed_add(conn, mode);
+
+	return 1;
+}
+
+static const struct drm_connector_helper_funcs sdrm_conn_hfuncs = {
+	.get_modes = sdrm_conn_get_modes,
+	.best_encoder = drm_atomic_helper_best_encoder,
+};
+
+static enum drm_connector_status sdrm_conn_detect(struct drm_connector *conn,
+						  bool force)
+{
+	/*
+	 * We simulate an always connected monitor. simple-fb doesn't
+	 * provide any way to detect whether the connector is active. Hence,
+	 * signal DRM core that it is always connected.
+	 */
+
+	return connector_status_connected;
+}
+
+static const struct drm_connector_funcs sdrm_conn_ops = {
+	.dpms = drm_atomic_helper_connector_dpms,
+	.reset = drm_atomic_helper_connector_reset,
+	.detect = sdrm_conn_detect,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = drm_connector_cleanup,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static inline struct sdrm_device *
+pipe_to_sdrm(struct drm_simple_display_pipe *pipe)
+{
+	return container_of(pipe, struct sdrm_device, pipe);
+}
+
+static void sdrm_crtc_send_vblank_event(struct drm_crtc *crtc)
+{
+	if (crtc->state && crtc->state->event) {
+		spin_lock_irq(&crtc->dev->event_lock);
+		drm_crtc_send_vblank_event(crtc, crtc->state->event);
+		spin_unlock_irq(&crtc->dev->event_lock);
+		crtc->state->event = NULL;
+	}
+}
+
+void sdrm_display_pipe_update(struct drm_simple_display_pipe *pipe,
+			      struct drm_plane_state *plane_state)
+{
+	struct drm_framebuffer *fb = pipe->plane.state->fb;
+	struct sdrm_device *sdrm = pipe_to_sdrm(pipe);
+
+	sdrm_crtc_send_vblank_event(&pipe->crtc);
+
+	if (fb) {
+		pipe->plane.fb = fb;
+		sdrm_dirty_all_locked(sdrm);
+	}
+}
+
+static void sdrm_display_pipe_enable(struct drm_simple_display_pipe *pipe,
+				     struct drm_crtc_state *crtc_state)
+{
+	sdrm_crtc_send_vblank_event(&pipe->crtc);
+}
+
+static void sdrm_display_pipe_disable(struct drm_simple_display_pipe *pipe)
+{
+	sdrm_crtc_send_vblank_event(&pipe->crtc);
+}
+
+static const struct drm_simple_display_pipe_funcs sdrm_pipe_funcs = {
+	.update = sdrm_display_pipe_update,
+	.enable = sdrm_display_pipe_enable,
+	.disable = sdrm_display_pipe_disable,
+};
+
+static int sdrm_fb_create_handle(struct drm_framebuffer *fb,
+				 struct drm_file *dfile,
+				 unsigned int *handle)
+{
+	struct sdrm_framebuffer *sfb = to_sdrm_fb(fb);
+
+	return drm_gem_handle_create(dfile, &sfb->obj->base, handle);
+}
+
+static void sdrm_fb_destroy(struct drm_framebuffer *fb)
+{
+	struct sdrm_framebuffer *sfb = to_sdrm_fb(fb);
+
+	drm_framebuffer_cleanup(fb);
+	drm_gem_object_unreference_unlocked(&sfb->obj->base);
+	kfree(sfb);
+}
+
+static const struct drm_framebuffer_funcs sdrm_fb_ops = {
+	.create_handle = sdrm_fb_create_handle,
+	.dirty = sdrm_dirty,
+	.destroy = sdrm_fb_destroy,
+};
+
+static struct drm_framebuffer *sdrm_fb_create(struct drm_device *ddev,
+					      struct drm_file *dfile,
+					      const struct drm_mode_fb_cmd2 *cmd)
+{
+	struct sdrm_framebuffer *fb;
+	struct drm_gem_object *gobj;
+	u32 bpp, size;
+	int ret;
+	void *err;
+
+	if (cmd->flags)
+		return ERR_PTR(-EINVAL);
+
+	gobj = drm_gem_object_lookup(dfile, cmd->handles[0]);
+	if (!gobj)
+		return ERR_PTR(-EINVAL);
+
+	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
+	if (!fb) {
+		err = ERR_PTR(-ENOMEM);
+		goto err_unref;
+	}
+	fb->obj = to_sdrm_bo(gobj);
+
+	fb->base.pitches[0] = cmd->pitches[0];
+	fb->base.offsets[0] = cmd->offsets[0];
+	fb->base.width = cmd->width;
+	fb->base.height = cmd->height;
+	fb->base.pixel_format = cmd->pixel_format;
+	drm_fb_get_bpp_depth(cmd->pixel_format, &fb->base.depth,
+			     &fb->base.bits_per_pixel);
+
+	/*
+	 * width/height are already clamped into min/max_width/height range,
+	 * so overflows are not possible
+	 */
+
+	bpp = (fb->base.bits_per_pixel + 7) / 8;
+	size = cmd->pitches[0] * cmd->height;
+	if (!bpp ||
+	    bpp > 4 ||
+	    cmd->pitches[0] < bpp * fb->base.width ||
+	    cmd->pitches[0] > 0xffffU ||
+	    size + fb->base.offsets[0] < size ||
+	    size + fb->base.offsets[0] > fb->obj->base.size) {
+		err = ERR_PTR(-EINVAL);
+		goto err_free;
+	}
+
+	ret = drm_framebuffer_init(ddev, &fb->base, &sdrm_fb_ops);
+	if (ret < 0) {
+		err = ERR_PTR(ret);
+		goto err_free;
+	}
+
+	DRM_DEBUG_KMS("[FB:%d] pixel_format: %s\n", fb->base.base.id,
+		      drm_get_format_name(fb->base.pixel_format));
+
+	return &fb->base;
+
+err_free:
+	kfree(fb);
+err_unref:
+	drm_gem_object_unreference_unlocked(gobj);
+
+	return err;
+}
+
+static const struct drm_mode_config_funcs sdrm_mode_config_ops = {
+	.fb_create = sdrm_fb_create,
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
+};
+
+int sdrm_drm_modeset_init(struct sdrm_device *sdrm)
+{
+	struct drm_connector *conn = &sdrm->conn;
+	struct drm_device *ddev = sdrm->ddev;
+	int ret;
+
+	drm_mode_config_init(ddev);
+	ddev->mode_config.min_width = sdrm->fb_width;
+	ddev->mode_config.max_width = sdrm->fb_width;
+	ddev->mode_config.min_height = sdrm->fb_height;
+	ddev->mode_config.max_height = sdrm->fb_height;
+	ddev->mode_config.preferred_depth = sdrm->fb_bpp;
+	ddev->mode_config.funcs = &sdrm_mode_config_ops;
+
+	drm_connector_helper_add(conn, &sdrm_conn_hfuncs);
+	ret = drm_connector_init(ddev, conn, &sdrm_conn_ops,
+				 DRM_MODE_CONNECTOR_VIRTUAL);
+	if (ret)
+		goto err_cleanup;
+
+	ret = drm_mode_create_dirty_info_property(ddev);
+	if (ret)
+		goto err_cleanup;
+
+	drm_object_attach_property(&conn->base,
+				   ddev->mode_config.dirty_info_property,
+				   DRM_MODE_DIRTY_ON);
+
+	ret = drm_simple_display_pipe_init(ddev, &sdrm->pipe, &sdrm_pipe_funcs,
+					   sdrm_formats,
+					   ARRAY_SIZE(sdrm_formats), conn);
+	if (ret)
+		goto err_cleanup;
+
+	drm_mode_config_reset(ddev);
+
+	return 0;
+
+err_cleanup:
+	drm_mode_config_cleanup(ddev);
+
+	return ret;
+}