[RFC 0/6] SimpleDRM Driver (was: dvbe driver)

[RFC 0/6] SimpleDRM Driver (was: dvbe driver)

[David Herrmann]

Hi

This is my second revision of the dvbe driver. I renamed it to SimpleDRM to
show the resemblence with the recently introduced simplefb.c fbdev driver. The
driver is supposed to be the most basic DRM driver similar to efifb.c, vesafb.c,
offb.c, simplefb.c, ...
It provides a single virtual CRTC+encoder+connector and allows user-space to
create one dumb-buffer at a time and attach it.

The setup changed slightly. It no longer uses shadow buffers but instead maps
the framebuffer directly into userspace. Furthermore, a new infrastructure is
used to unload firmware drivers during real hardware drivers probe cycles. Only
nouveau was changed to use it, yet.

I still have an odd problem when unloading DRM drivers (not just SimpleDRM) with
an fbdev fallback. If I call printk() directly after unregister_framebufer(), I
get a NULL-deref somewhere in the VT layer (most times hide_cursor()). I haven't
figured out exactly where that happens, but I am also very reluctant to spend
more time debugging the VT layer.

Anyhow, comments welcome. If someone wants to test it, you probably need to add
a line to ./include/linux/platform_data/simplefb.h and add the modeline of your
VESA/EFI framebuffer.

Cheers
David

David Herrmann (6):
  fbdev: simplefb: add init through platform_data
  x86: provide platform-devices for boot-framebuffers
  drm: add SimpleDRM driver
  drm: simpledrm: add fbdev fallback support
  drm: add helpers to kick out firmware drivers
  drm: nouveau: kick out firmware drivers during probe

 MAINTAINERS                                 |   8 +
 arch/x86/Kconfig                            |  18 ++
 arch/x86/kernel/Makefile                    |   1 +
 arch/x86/kernel/sysfb.c                     | 157 ++++++++++++
 drivers/gpu/drm/Kconfig                     |   2 +
 drivers/gpu/drm/Makefile                    |   1 +
 drivers/gpu/drm/drm_pci.c                   |   1 +
 drivers/gpu/drm/drm_platform.c              |   1 +
 drivers/gpu/drm/drm_stub.c                  | 107 ++++++++
 drivers/gpu/drm/drm_usb.c                   |   1 +
 drivers/gpu/drm/nouveau/nouveau_drm.c       |  29 ++-
 drivers/gpu/drm/simpledrm/Kconfig           |  29 +++
 drivers/gpu/drm/simpledrm/Makefile          |   9 +
 drivers/gpu/drm/simpledrm/simpledrm.h       | 114 +++++++++
 drivers/gpu/drm/simpledrm/simpledrm_drv.c   | 231 ++++++++++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_fbdev.c | 180 ++++++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_main.c  | 366 ++++++++++++++++++++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_mem.c   | 254 +++++++++++++++++++
 drivers/video/Kconfig                       |   5 +-
 drivers/video/simplefb.c                    |  45 +++-
 include/drm/drmP.h                          |  26 ++
 include/linux/platform_data/simplefb.h      |  40 +++
 22 files changed, 1604 insertions(+), 21 deletions(-)
 create mode 100644 arch/x86/kernel/sysfb.c
 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_drv.c
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_main.c
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_mem.c
 create mode 100644 include/linux/platform_data/simplefb.h

-- 
1.8.3.1

[RFC 1/6] fbdev: simplefb: add init through platform_data

[David Herrmann]

If we create proper platform-devices in x86 boot-code, we can use simplefb
for VBE or EFI framebuffers, too. However, there is normally no OF support
so we introduce a platform_data object so x86 boot-code can pass the
paramaters via plain old platform-data.

This also removes the OF dependency as it is not needed. The headers
provide proper dummies for the case OF is disabled.

Furthermore, we move the FORMAT-definitions to the common platform header
so initialization code can use it to transform "struct screen_info" to
the right format-name.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/video/Kconfig                  |  5 ++--
 drivers/video/simplefb.c               | 45 +++++++++++++++++++++++++---------
 include/linux/platform_data/simplefb.h | 40 ++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+), 14 deletions(-)
 create mode 100644 include/linux/platform_data/simplefb.h

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 2e937bd..22586ee 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2455,7 +2455,7 @@ config FB_HYPERV
 
 config FB_SIMPLE
 	bool "Simple framebuffer support"
-	depends on (FB = y) && OF
+	depends on (FB = y)
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
@@ -2467,8 +2467,7 @@ config FB_SIMPLE
 	  pre-allocated frame buffer surface.
 
 	  Configuration re: surface address, size, and format must be provided
-	  through device tree, or potentially plain old platform data in the
-	  future.
+	  through device tree, or plain old platform data.
 
 source "drivers/video/omap/Kconfig"
 source "drivers/video/omap2/Kconfig"
diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
index e2e9e3e..35e36c5 100644
--- a/drivers/video/simplefb.c
+++ b/drivers/video/simplefb.c
@@ -24,6 +24,7 @@
 #include <linux/fb.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/platform_data/simplefb.h>
 #include <linux/platform_device.h>
 
 static struct fb_fix_screeninfo simplefb_fix = {
@@ -73,17 +74,8 @@ static struct fb_ops simplefb_ops = {
 	.fb_imageblit	= cfb_imageblit,
 };
 
-struct simplefb_format {
-	const char *name;
-	u32 bits_per_pixel;
-	struct fb_bitfield red;
-	struct fb_bitfield green;
-	struct fb_bitfield blue;
-	struct fb_bitfield transp;
-};
-
 static struct simplefb_format simplefb_formats[] = {
-	{ "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} },
+	SIMPLEFB_FORMATS
 };
 
 struct simplefb_params {
@@ -139,6 +131,32 @@ static int simplefb_parse_dt(struct platform_device *pdev,
 	return 0;
 }
 
+static int simplefb_parse_pd(struct platform_device *pdev,
+			     struct simplefb_params *params)
+{
+	struct simplefb_platform_data *pd = pdev->dev.platform_data;
+	int i;
+
+	params->width = pd->width;
+	params->height = pd->height;
+	params->stride = pd->stride;
+
+	for (i = 0; i < ARRAY_SIZE(simplefb_formats); i++) {
+		if (strcmp(pd->format, simplefb_formats[i].name))
+			continue;
+
+		params->format = &simplefb_formats[i];
+		break;
+	}
+
+	if (!params->format) {
+		dev_err(&pdev->dev, "Invalid format value\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int simplefb_probe(struct platform_device *pdev)
 {
 	int ret;
@@ -149,7 +167,12 @@ static int simplefb_probe(struct platform_device *pdev)
 	if (fb_get_options("simplefb", NULL))
 		return -ENODEV;
 
-	ret = simplefb_parse_dt(pdev, &params);
+	ret = -ENODEV;
+	if (pdev->dev.platform_data)
+		ret = simplefb_parse_pd(pdev, &params);
+	else if (pdev->dev.of_node)
+		ret = simplefb_parse_dt(pdev, &params);
+
 	if (ret)
 		return ret;
 
diff --git a/include/linux/platform_data/simplefb.h b/include/linux/platform_data/simplefb.h
new file mode 100644
index 0000000..a18353d
--- /dev/null
+++ b/include/linux/platform_data/simplefb.h
@@ -0,0 +1,40 @@
+/*
+ * simplefb.h - Simple Framebuffer Device
+ *
+ * Copyright (C) 2013 David Herrmann <dh.herrmann@gmail.com>
+ *
+ * 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 __PLATFORM_DATA_SIMPLEFB_H__
+#define __PLATFORM_DATA_SIMPLEFB_H__
+
+#include <drm/drm_fourcc.h>
+#include <linux/fb.h>
+#include <linux/kernel.h>
+
+#define SIMPLEFB_FORMATS \
+	{ "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0}, DRM_FORMAT_RGB565 }
+
+struct simplefb_format {
+	const char *name;
+	u32 bits_per_pixel;
+	struct fb_bitfield red;
+	struct fb_bitfield green;
+	struct fb_bitfield blue;
+	struct fb_bitfield transp;
+	u32 fourcc;
+};
+
+/* the framebuffer size and location is available as IORESOURCE_MEM */
+struct simplefb_platform_data {
+	u32 width;
+	u32 height;
+	u32 stride;
+	char format[64];
+};
+
+#endif /* __PLATFORM_DATA_SIMPLEFB_H__ */
-- 
1.8.3.1

Re: [RFC 1/6] fbdev: simplefb: add init through platform_data

[Stephen Warren]

On 06/24/2013 04:27 PM, David Herrmann wrote:
> If we create proper platform-devices in x86 boot-code, we can use simplefb
> for VBE or EFI framebuffers, too. However, there is normally no OF support
> so we introduce a platform_data object so x86 boot-code can pass the
> paramaters via plain old platform-data.
> 
> This also removes the OF dependency as it is not needed. The headers
> provide proper dummies for the case OF is disabled.
> 
> Furthermore, we move the FORMAT-definitions to the common platform header
> so initialization code can use it to transform "struct screen_info" to
> the right format-name.

> diff --git a/include/linux/platform_data/simplefb.h b/include/linux/platform_data/simplefb.h

> +/* the framebuffer size and location is available as IORESOURCE_MEM */
> +struct simplefb_platform_data {
> +	u32 width;
> +	u32 height;
> +	u32 stride;
> +	char format[64];
> +};

Any reason not to make format:

const char *format;

You should be able to initialize that just as easily in platform code,
either as static data or at runtime, I think.

Re: [RFC 1/6] fbdev: simplefb: add init through platform_data

[David Herrmann]

Hi

On Wed, Jun 26, 2013 at 10:39 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 06/24/2013 04:27 PM, David Herrmann wrote:
>> If we create proper platform-devices in x86 boot-code, we can use simplefb
>> for VBE or EFI framebuffers, too. However, there is normally no OF support
>> so we introduce a platform_data object so x86 boot-code can pass the
>> paramaters via plain old platform-data.
>>
>> This also removes the OF dependency as it is not needed. The headers
>> provide proper dummies for the case OF is disabled.
>>
>> Furthermore, we move the FORMAT-definitions to the common platform header
>> so initialization code can use it to transform "struct screen_info" to
>> the right format-name.
>
>> diff --git a/include/linux/platform_data/simplefb.h b/include/linux/platform_data/simplefb.h
>
>> +/* the framebuffer size and location is available as IORESOURCE_MEM */
>> +struct simplefb_platform_data {
>> +     u32 width;
>> +     u32 height;
>> +     u32 stride;
>> +     char format[64];
>> +};
>
> Any reason not to make format:
>
> const char *format;
>
> You should be able to initialize that just as easily in platform code,
> either as static data or at runtime, I think.

That makes sense. I fixed it up.

Thanks
David

[RFC 2/6] x86: provide platform-devices for boot-framebuffers

[David Herrmann]

The current situation regarding boot-framebuffers (VGA, VESA/VBE, EFI) on
x86 causes troubles when loading multiple fbdev drivers. The global
"struct screen_info" does not provide any state-tracking about which
drivers use the FBs. request_mem_region() theoretically works, but
unfortunately vesafb/efifb ignore it due to quirks for broken boards.

Avoid this by creating a "platform-framebuffer" device with a pointer
to the "struct screen_info" as platform-data. Drivers can now create
platform-drivers and the driver-core will refuse multiple drivers being
active simultaneously.

We keep the screen_info available for backwards-compatibility. Drivers
can be converted in follow-up patches.

Apart from "platform-framebuffer" devices, this also introduces a
compatibility option for "simple-framebuffer" drivers which recently got
introduced for OF based systems. If CONFIG_X86_SYSFB is selected, we
try to match the screen_info against a simple-framebuffer supported
format. If we succeed, we create a "simple-framebuffer" device instead
of a platform-framebuffer.
This allows to reuse the simplefb.c driver across architectures and also
to introduce a SimpleDRM driver. There is no need to have vesafb.c,
efifb.c, simplefb.c and more just to have architecture specific quirks
in their setup-routines.

Instead, we now move the architecture specific quirks into x86-setup and
provide a generic simple-framebuffer. For backwards-compatibility (if
strange formats are used), we still allow vesafb/efifb to be loaded
simultaneously and pick up all remaining devices.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 arch/x86/Kconfig         |  18 ++++++
 arch/x86/kernel/Makefile |   1 +
 arch/x86/kernel/sysfb.c  | 157 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 176 insertions(+)
 create mode 100644 arch/x86/kernel/sysfb.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index fe120da..8eb06b5 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2255,6 +2255,24 @@ config RAPIDIO
 
 source "drivers/rapidio/Kconfig"
 
+config X86_SYSFB
+	bool "Mark VGA/VBE/EFI FB as generic system framebuffer"
+	help
+	  Firmwares often provide initial graphics framebuffers so the BIOS,
+	  bootloader or kernel can show basic video-output during boot for
+	  user-guidance and debugging. Historically, x86 used the VESA BIOS
+	  Extensions and EFI-framebuffers for this, which are mostly limited
+	  to x86. However, a generic system-framebuffer initialization emerged
+	  recently on some non-x86 architectures.
+	  This option, if enabled, marks VGA/VBE/EFI framebuffers as generic
+	  framebuffers so the new generic system-framebuffer drivers can be
+	  used on x86.
+
+	  This breaks any x86-only driver like efifb, vesafb, uvesafb, which
+	  will not work if this is selected.
+
+	  If unsure, say N.
+
 endmenu
 
 
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 7bd3bd3..1e1005a 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -100,6 +100,7 @@ obj-$(CONFIG_X86_CHECK_BIOS_CORRUPTION) += check.o
 obj-$(CONFIG_SWIOTLB)			+= pci-swiotlb.o
 obj-$(CONFIG_OF)			+= devicetree.o
 obj-$(CONFIG_UPROBES)			+= uprobes.o
+obj-y					+= sysfb.o
 
 obj-$(CONFIG_PERF_EVENTS)		+= perf_regs.o
 
diff --git a/arch/x86/kernel/sysfb.c b/arch/x86/kernel/sysfb.c
new file mode 100644
index 0000000..8272958
--- /dev/null
+++ b/arch/x86/kernel/sysfb.c
@@ -0,0 +1,157 @@
+/*
+ * Generic System Framebuffers on x86
+ * Copyright (c) 2012-2013 David Herrmann <dh.herrmann@gmail.com>
+ */
+
+/*
+ * 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.
+ */
+
+/*
+ * Simple-Framebuffer support for x86 systems
+ * Create a platform-device for any available boot framebuffer. The
+ * simple-framebuffer platform device is already available on DT systems, so
+ * this module parses the global "screen_info" object and creates a suitable
+ * platform device compatible with the "simple-framebuffer" DT object. If
+ * the framebuffer is incompatible, we instead create a "platform-framebuffer"
+ * device and pass the screen_info as platform_data. This allows legacy drivers
+ * to pick these devices up without messing with simple-framebuffer drivers.
+ *
+ * If CONFIG_X86_SYSFB is not selected, we never register "simple-framebuffer"
+ * platform devices, but only use "platform-framebuffer" devices for
+ * backwards compatibility.
+ *
+ * TODO: We set the dev_id field of all platform-devices to 0. This allows
+ * other x86 OF/DT parsers to create such devices, too. However, they must
+ * start at offset 1 for this to work.
+ */
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/platform_data/simplefb.h>
+#include <linux/platform_device.h>
+#include <linux/screen_info.h>
+
+#ifdef CONFIG_X86_SYSFB
+
+static const char simplefb_resname[] = "BOOTFB";
+
+static const struct simplefb_format formats[] = {
+	SIMPLEFB_FORMATS
+};
+
+static bool parse_mode(const struct screen_info *si,
+		       struct simplefb_platform_data *mode)
+{
+	const struct simplefb_format *f;
+	__u8 type;
+	unsigned int i;
+
+	/* TODO: apply quirks from efifb.c here before probing the devices */
+
+	type = si->orig_video_isVGA;
+	if (type != VIDEO_TYPE_VLFB && type != VIDEO_TYPE_EFI)
+		return false;
+
+	for (i = 0; i < ARRAY_SIZE(formats); ++i) {
+		f = &formats[i];
+		if (si->lfb_depth == f->bits_per_pixel &&
+		    si->red_size == f->red.length &&
+		    si->red_pos == f->red.offset &&
+		    si->green_size == f->green.length &&
+		    si->green_pos == f->green.offset &&
+		    si->blue_size == f->blue.length &&
+		    si->blue_pos == f->blue.offset &&
+		    si->rsvd_size == f->transp.length &&
+		    si->rsvd_pos == f->transp.offset) {
+			strlcpy(mode->format, f->name, sizeof(mode->format));
+			mode->width = si->lfb_width;
+			mode->height = si->lfb_height;
+			mode->stride = si->lfb_linelength;
+			return true;
+		}
+	}
+
+	return false;
+}
+
+static int create_simplefb(const struct screen_info *si,
+			   const struct simplefb_platform_data *mode)
+{
+	struct platform_device *pd;
+	struct resource memres;
+	unsigned long len;
+
+	/* don't use lfb_size as it may contain the whole VMEM instead of only
+	 * the part that is occupied by the framebuffer */
+	len = mode->height * mode->stride;
+	len = PAGE_ALIGN(len);
+	if (len > si->lfb_size << 16) {
+		printk(KERN_WARNING "sysfb: VRAM smaller than advertised\n");
+		return -EINVAL;
+	}
+
+	/* setup IORESOURCE_MEM as framebuffer memory */
+	memset(&memres, 0, sizeof(memres));
+	memres.flags = IORESOURCE_MEM;
+	memres.name = simplefb_resname;
+	memres.start = si->lfb_base;
+	memres.end = si->lfb_base + len - 1;
+	if (memres.end <= memres.start)
+		return -EINVAL;
+
+	pd = platform_device_register_resndata(NULL,
+					       "simple-framebuffer", 0,
+					       &memres, 1,
+					       mode, sizeof(*mode));
+	if (IS_ERR(pd))
+		return PTR_ERR(pd);
+
+	return 0;
+}
+
+#else /* CONFIG_X86_SYSFB */
+
+static bool parse_mode(const struct screen_info *si,
+		       struct simplefb_platform_data *mode)
+{
+	return false;
+}
+
+static int create_simplefb(const struct screen_info *si,
+			   const struct simplefb_platform_data *mode)
+{
+	return -EINVAL;
+}
+
+#endif /* CONFIG_X86_SYSFB */
+
+static __init int add_sysfb(void)
+{
+	const struct screen_info *si = &screen_info;
+	struct simplefb_platform_data mode;
+	struct platform_device *pd;
+	bool compatible = false;
+	int ret;
+
+	compatible = parse_mode(si, &mode);
+
+	ret = -ENODEV;
+	if (compatible)
+		ret = create_simplefb(si, &mode);
+
+	if (ret) {
+		pd = platform_device_register_resndata(NULL,
+						"platform-framebuffer", 0,
+						NULL, 0, si, sizeof(*si));
+		ret = IS_ERR(pd) ? PTR_ERR(pd) : 0;
+	}
+
+	return ret;
+}
+device_initcall(add_sysfb);
-- 
1.8.3.1

Re: [RFC 2/6] x86: provide platform-devices for boot-framebuffers

[Stephen Warren]

On 06/24/2013 04:27 PM, David Herrmann wrote:
> The current situation regarding boot-framebuffers (VGA, VESA/VBE, EFI) on
> x86 causes troubles when loading multiple fbdev drivers. The global
> "struct screen_info" does not provide any state-tracking about which
> drivers use the FBs. request_mem_region() theoretically works, but
> unfortunately vesafb/efifb ignore it due to quirks for broken boards.
> 
> Avoid this by creating a "platform-framebuffer" device with a pointer
> to the "struct screen_info" as platform-data. Drivers can now create
> platform-drivers and the driver-core will refuse multiple drivers being
> active simultaneously.
> 
> We keep the screen_info available for backwards-compatibility. Drivers
> can be converted in follow-up patches.
> 
> Apart from "platform-framebuffer" devices, this also introduces a
> compatibility option for "simple-framebuffer" drivers which recently got
> introduced for OF based systems. If CONFIG_X86_SYSFB is selected, we
> try to match the screen_info against a simple-framebuffer supported
> format. If we succeed, we create a "simple-framebuffer" device instead
> of a platform-framebuffer.
> This allows to reuse the simplefb.c driver across architectures and also
> to introduce a SimpleDRM driver. There is no need to have vesafb.c,
> efifb.c, simplefb.c and more just to have architecture specific quirks
> in their setup-routines.
> 
> Instead, we now move the architecture specific quirks into x86-setup and
> provide a generic simple-framebuffer. For backwards-compatibility (if
> strange formats are used), we still allow vesafb/efifb to be loaded
> simultaneously and pick up all remaining devices.

> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig

> +config X86_SYSFB
> +	bool "Mark VGA/VBE/EFI FB as generic system framebuffer"
> +	help
> +	  Firmwares often provide initial graphics framebuffers so the BIOS,
> +	  bootloader or kernel can show basic video-output during boot for
> +	  user-guidance and debugging. Historically, x86 used the VESA BIOS
> +	  Extensions and EFI-framebuffers for this, which are mostly limited
> +	  to x86. However, a generic system-framebuffer initialization emerged
> +	  recently on some non-x86 architectures.

After this patch has been in the kernel a while, that very last won't
really be true; simplefb won't have been introduced recently. Perhaps
just delete that one sentence?

> +	  This option, if enabled, marks VGA/VBE/EFI framebuffers as generic
> +	  framebuffers so the new generic system-framebuffer drivers can be
> +	  used on x86.
> +
> +	  This breaks any x86-only driver like efifb, vesafb, uvesafb, which
> +	  will not work if this is selected.

Doesn't that imply that some form of conflicts or depends ! statement
should be added here?

> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile

> +obj-y					+= sysfb.o

I suspect that should be obj-$(CONFIG_X86_SYSFB) += sysfb.o.

> diff --git a/arch/x86/kernel/sysfb.c b/arch/x86/kernel/sysfb.c

> +#ifdef CONFIG_X86_SYSFB

Rather than ifdef'ing the body of this file, why not create a dummy
static inline version of add_sysfb() and put that into a header file
that users include. There should be a header file to prototype the
function anyway. That way, you avoid all of the ifdefs and static inline
functions in this file.

> +static bool parse_mode(const struct screen_info *si,
> +		       struct simplefb_platform_data *mode)

> +			strlcpy(mode->format, f->name, sizeof(mode->format));

Per my comments about the type of mode->format, I think that could just be:

mode->format = f->name;

... since formats[] (i.e. f) isn't initdata.

> +#else /* CONFIG_X86_SYSFB */
> +
> +static bool parse_mode(const struct screen_info *si,
> +		       struct simplefb_platform_data *mode)
> +{
> +	return false;
> +}
> +
> +static int create_simplefb(const struct screen_info *si,
> +			   const struct simplefb_platform_data *mode)
> +{
> +	return -EINVAL;
> +}
> +
> +#endif /* CONFIG_X86_SYSFB */

Following on from my ifdef comment above, I believe those versions of
those functions will always cause add_sysfb() to return -ENODEV, so you
may as well provide a static inline for add_sysfb() instead.

Re: [RFC 2/6] x86: provide platform-devices for boot-framebuffers

[David Herrmann]

Hi

On Wed, Jun 26, 2013 at 10:49 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 06/24/2013 04:27 PM, David Herrmann wrote:
>> The current situation regarding boot-framebuffers (VGA, VESA/VBE, EFI) on
>> x86 causes troubles when loading multiple fbdev drivers. The global
>> "struct screen_info" does not provide any state-tracking about which
>> drivers use the FBs. request_mem_region() theoretically works, but
>> unfortunately vesafb/efifb ignore it due to quirks for broken boards.
>>
>> Avoid this by creating a "platform-framebuffer" device with a pointer
>> to the "struct screen_info" as platform-data. Drivers can now create
>> platform-drivers and the driver-core will refuse multiple drivers being
>> active simultaneously.
>>
>> We keep the screen_info available for backwards-compatibility. Drivers
>> can be converted in follow-up patches.
>>
>> Apart from "platform-framebuffer" devices, this also introduces a
>> compatibility option for "simple-framebuffer" drivers which recently got
>> introduced for OF based systems. If CONFIG_X86_SYSFB is selected, we
>> try to match the screen_info against a simple-framebuffer supported
>> format. If we succeed, we create a "simple-framebuffer" device instead
>> of a platform-framebuffer.
>> This allows to reuse the simplefb.c driver across architectures and also
>> to introduce a SimpleDRM driver. There is no need to have vesafb.c,
>> efifb.c, simplefb.c and more just to have architecture specific quirks
>> in their setup-routines.
>>
>> Instead, we now move the architecture specific quirks into x86-setup and
>> provide a generic simple-framebuffer. For backwards-compatibility (if
>> strange formats are used), we still allow vesafb/efifb to be loaded
>> simultaneously and pick up all remaining devices.
>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>
>> +config X86_SYSFB
>> +     bool "Mark VGA/VBE/EFI FB as generic system framebuffer"
>> +     help
>> +       Firmwares often provide initial graphics framebuffers so the BIOS,
>> +       bootloader or kernel can show basic video-output during boot for
>> +       user-guidance and debugging. Historically, x86 used the VESA BIOS
>> +       Extensions and EFI-framebuffers for this, which are mostly limited
>> +       to x86. However, a generic system-framebuffer initialization emerged
>> +       recently on some non-x86 architectures.
>
> After this patch has been in the kernel a while, that very last won't
> really be true; simplefb won't have been introduced recently. Perhaps
> just delete that one sentence?

It rather belongs in the commit message, right. I will rephrase that.

>> +       This option, if enabled, marks VGA/VBE/EFI framebuffers as generic
>> +       framebuffers so the new generic system-framebuffer drivers can be
>> +       used on x86.
>> +
>> +       This breaks any x86-only driver like efifb, vesafb, uvesafb, which
>> +       will not work if this is selected.
>
> Doesn't that imply that some form of conflicts or depends ! statement
> should be added here?

There is no real conflict here. You still can use vesafb/... with this
option, but they will not pick up the device. I intend to fix these up
to use "platform-framebuffer" devices instead of globally binding to
"struct screen_info". This way, framebuffers either end up as
simple-framebuffers or platform-framebuffers. This option selects
which device they end up as.

As all non-compatible framebuffers (with incompatible pixel-formats)
always end up as "platform-framebuffer", it still makes sense to use
vesafb as fallback. Hence, I'd not introduce any "conflicts"
dependency here.
Maybe I should rephrase the warning to something that makes clear that
if this option is selected, you need simplefb.c or simpledrm to make
use of these devices.

>> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
>
>> +obj-y                                        += sysfb.o
>
> I suspect that should be obj-$(CONFIG_X86_SYSFB) += sysfb.o.

No. This patch tries to solve two things: First of all, every
system-framebuffer now gets a "platform-framebuffer" platform-device.
Iff X86_SYSFB is selected, it additionally tries to parse the
framebuffer information as "simple-framebuffer". If it succeeds, it
created a "simple-framebuffer" object, if it doesn't, a fallback
"platform-framebuffer" is provided.

This series is missing vesafb/efifb/.. patches, which should now bind
to "platform-framebuffer" devices instead of using "struct
screen_info" directly. I intend to add these in the next round.

>> diff --git a/arch/x86/kernel/sysfb.c b/arch/x86/kernel/sysfb.c
>
>> +#ifdef CONFIG_X86_SYSFB
>
> Rather than ifdef'ing the body of this file, why not create a dummy
> static inline version of add_sysfb() and put that into a header file
> that users include. There should be a header file to prototype the
> function anyway. That way, you avoid all of the ifdefs and static inline
> functions in this file.
>
>> +static bool parse_mode(const struct screen_info *si,
>> +                    struct simplefb_platform_data *mode)
>
>> +                     strlcpy(mode->format, f->name, sizeof(mode->format));
>
> Per my comments about the type of mode->format, I think that could just be:
>
> mode->format = f->name;
>
> ... since formats[] (i.e. f) isn't initdata.
>
>> +#else /* CONFIG_X86_SYSFB */
>> +
>> +static bool parse_mode(const struct screen_info *si,
>> +                    struct simplefb_platform_data *mode)
>> +{
>> +     return false;
>> +}
>> +
>> +static int create_simplefb(const struct screen_info *si,
>> +                        const struct simplefb_platform_data *mode)
>> +{
>> +     return -EINVAL;
>> +}
>> +
>> +#endif /* CONFIG_X86_SYSFB */
>
> Following on from my ifdef comment above, I believe those versions of
> those functions will always cause add_sysfb() to return -ENODEV, so you
> may as well provide a static inline for add_sysfb() instead.

No. add_sysfb() is supposed to always succeed. However, if
parse_mode/create_simplefb fail, it creates a "platform-framebuffer"
as fallback. I don't see any way to avoid these ifdefs. Considering
the explanation above, could you elaborate how you think this should
work?

Thanks
David

Re: [RFC 2/6] x86: provide platform-devices for boot-framebuffers

[Stephen Warren]

On 06/28/2013 04:11 AM, David Herrmann wrote:
> Hi
> 
> On Wed, Jun 26, 2013 at 10:49 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 06/24/2013 04:27 PM, David Herrmann wrote:
>>> The current situation regarding boot-framebuffers (VGA, VESA/VBE, EFI) on
>>> x86 causes troubles when loading multiple fbdev drivers. The global
>>> "struct screen_info" does not provide any state-tracking about which
>>> drivers use the FBs. request_mem_region() theoretically works, but
>>> unfortunately vesafb/efifb ignore it due to quirks for broken boards.
>>>
>>> Avoid this by creating a "platform-framebuffer" device with a pointer
>>> to the "struct screen_info" as platform-data. Drivers can now create
>>> platform-drivers and the driver-core will refuse multiple drivers being
>>> active simultaneously.
>>>
>>> We keep the screen_info available for backwards-compatibility. Drivers
>>> can be converted in follow-up patches.
>>>
>>> Apart from "platform-framebuffer" devices, this also introduces a
>>> compatibility option for "simple-framebuffer" drivers which recently got
>>> introduced for OF based systems. If CONFIG_X86_SYSFB is selected, we
>>> try to match the screen_info against a simple-framebuffer supported
>>> format. If we succeed, we create a "simple-framebuffer" device instead
>>> of a platform-framebuffer.
...

>>> diff --git a/arch/x86/kernel/sysfb.c b/arch/x86/kernel/sysfb.c
...
>>> +#else /* CONFIG_X86_SYSFB */
>>> +
>>> +static bool parse_mode(const struct screen_info *si,
>>> +                    struct simplefb_platform_data *mode)
>>> +{
>>> +     return false;
>>> +}
>>> +
>>> +static int create_simplefb(const struct screen_info *si,
>>> +                        const struct simplefb_platform_data *mode)
>>> +{
>>> +     return -EINVAL;
>>> +}
>>> +
>>> +#endif /* CONFIG_X86_SYSFB */
>>
>> Following on from my ifdef comment above, I believe those versions of
>> those functions will always cause add_sysfb() to return -ENODEV, so you
>> may as well provide a static inline for add_sysfb() instead.
> 
> No. add_sysfb() is supposed to always succeed. However, if
> parse_mode/create_simplefb fail, it creates a "platform-framebuffer"
> as fallback. I don't see any way to avoid these ifdefs. Considering
> the explanation above, could you elaborate how you think this should
> work?

Ah, I wasn't getting the fallback mechanism; that if creating a simplefb
wasn't possible or didn't succeed, a platformfb device would be created
instead.

Perhaps the following might be slightly clearer; there are certainly
fewer nesting levels:

static __init int add_sysfb(void)
{
	const struct screen_info *si = &screen_info;
	struct simplefb_platform_data mode;
	struct platform_device *pd;
	bool compatible = false;
	int ret;

	compatible = parse_mode(si, &mode);
	if (compatible) {
		ret = create_simplefb(si, &mode);
		if (!ret)
			return 0;
	}

	pd = platform_device_register_resndata(NULL,
					"platform-framebuffer", 0,
					NULL, 0, si, sizeof(*si));
	ret = IS_ERR(pd) ? PTR_ERR(pd) : 0;

	return ret;
}

[RFC 3/6] drm: add SimpleDRM driver

[David Herrmann]

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 one dumb-buffer and attach it to the CRTC. Only if
the buffer is destroyed, a new buffer can be created. The buffer is
directly mapped into user-space, so we have only resources for a single
buffer. Otherwise, shadow buffers plus damage-request would be needed.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 MAINTAINERS                                |   8 +
 drivers/gpu/drm/Kconfig                    |   2 +
 drivers/gpu/drm/Makefile                   |   1 +
 drivers/gpu/drm/simpledrm/Kconfig          |  18 ++
 drivers/gpu/drm/simpledrm/Makefile         |   5 +
 drivers/gpu/drm/simpledrm/simpledrm.h      |  91 ++++++++
 drivers/gpu/drm/simpledrm/simpledrm_drv.c  | 230 ++++++++++++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_main.c | 330 +++++++++++++++++++++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_mem.c  | 254 ++++++++++++++++++++++
 9 files changed, 939 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_drv.c
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_main.c
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_mem.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 5be702c..0f651845 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7305,6 +7305,14 @@ S:	Odd Fixes
 F:	drivers/media/platform/sh_vou.c
 F:	include/media/sh_vou.h
 
+SIMPLE DRM DRIVER
+M:	David Herrmann <dh.herrmann@gmail.com>
+L:	dri-devel@lists.freedesktop.org
+T:	git git://people.freedesktop.org/~dvdhrm/linux
+S:	Maintained
+F:	drivers/gpu/drm/simpledrm
+F:	include/linux/platform_data/simpledrm.h
+
 SIMPLE FIRMWARE INTERFACE (SFI)
 M:	Len Brown <lenb@kernel.org>
 L:	sfi-devel@simplefirmware.org
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index b16c50e..b3ccef6 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -220,3 +220,5 @@ source "drivers/gpu/drm/omapdrm/Kconfig"
 source "drivers/gpu/drm/tilcdc/Kconfig"
 
 source "drivers/gpu/drm/qxl/Kconfig"
+
+source "drivers/gpu/drm/simpledrm/Kconfig"
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 1c9f2439..59c424d 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -52,4 +52,5 @@ obj-$(CONFIG_DRM_SHMOBILE) +=shmobile/
 obj-$(CONFIG_DRM_OMAP)	+= omapdrm/
 obj-$(CONFIG_DRM_TILCDC)	+= tilcdc/
 obj-$(CONFIG_DRM_QXL) += qxl/
+obj-$(CONFIG_DRM_SIMPLEDRM) += simpledrm/
 obj-y			+= i2c/
diff --git a/drivers/gpu/drm/simpledrm/Kconfig b/drivers/gpu/drm/simpledrm/Kconfig
new file mode 100644
index 0000000..1d4f38e
--- /dev/null
+++ b/drivers/gpu/drm/simpledrm/Kconfig
@@ -0,0 +1,18 @@
+config DRM_SIMPLEDRM
+	tristate "Simple firmware framebuffer DRM driver"
+	depends on DRM && !FB_SIMPLE
+	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, x86 VESA
+	  BIOS Extensions (VBE), EFI 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..2d474a5
--- /dev/null
+++ b/drivers/gpu/drm/simpledrm/Makefile
@@ -0,0 +1,5 @@
+ccflags-y := -Iinclude/drm
+
+simpledrm-y := simpledrm_drv.o simpledrm_main.o simpledrm_mem.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..279d847
--- /dev/null
+++ b/drivers/gpu/drm/simpledrm/simpledrm.h
@@ -0,0 +1,91 @@
+/*
+ * SimpleDRM firmware framebuffer driver
+ * Copyright (c) 2012-2013 David Herrmann <dh.herrmann@gmail.com>
+ */
+
+/*
+ * 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 <linux/errno.h>
+#include <linux/fb.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/platform_data/simplefb.h>
+#include <linux/string.h>
+#include <drm/drmP.h>
+
+struct sdrm_device;
+struct sdrm_gem_object;
+struct sdrm_framebuffer;
+
+/* simpledrm devices */
+
+struct sdrm_device {
+	struct drm_device *ddev;
+
+	/* 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;
+
+	/* mode-setting objects */
+	struct sdrm_gem_object *fb_obj;
+	struct drm_crtc crtc;
+	struct drm_encoder enc;
+	struct drm_connector conn;
+	struct drm_display_mode *mode;
+};
+
+int sdrm_drm_load(struct drm_device *ddev, unsigned long flags);
+int sdrm_drm_unload(struct drm_device *ddev);
+int sdrm_drm_mmap(struct file *filp, struct vm_area_struct *vma);
+int sdrm_pdev_init(struct sdrm_device *sdrm);
+void sdrm_pdev_destroy(struct sdrm_device *sdrm);
+
+/* simpledrm gem objects */
+
+struct sdrm_gem_object {
+	struct drm_gem_object base;
+	unsigned long fb_base;
+	unsigned long fb_size;
+};
+
+#define to_sdrm_bo(x) container_of(x, struct sdrm_gem_object, base)
+
+int sdrm_gem_init_object(struct drm_gem_object *obj);
+void sdrm_gem_free_object(struct drm_gem_object *obj);
+void sdrm_gem_unmap_object(struct sdrm_gem_object *obj);
+
+/* dumb buffers */
+
+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);
+
+/* simpledrm framebuffers */
+
+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_drv.c b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
new file mode 100644
index 0000000..282752c
--- /dev/null
+++ b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
@@ -0,0 +1,230 @@
+/*
+ * SimpleDRM firmware framebuffer driver
+ * Copyright (c) 2012-2013 David Herrmann <dh.herrmann@gmail.com>
+ */
+
+/*
+ * 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 <linux/errno.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/platform_data/simplefb.h>
+#include <linux/string.h>
+#include <drm/drmP.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,
+	.fasync = drm_fasync,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl = drm_compat_ioctl,
+#endif
+	.llseek = noop_llseek,
+};
+
+static struct drm_driver sdrm_drm_driver = {
+	.driver_features = DRIVER_MODESET | DRIVER_GEM,
+	.load = sdrm_drm_load,
+	.unload = sdrm_drm_unload,
+	.fops = &sdrm_drm_fops,
+
+	.gem_init_object = sdrm_gem_init_object,
+	.gem_free_object = sdrm_gem_free_object,
+
+	.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,
+};
+
+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;
+	}
+	strlcpy(mode->format, format, sizeof(mode->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;
+	}
+
+	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;
+
+	if (!request_mem_region(sdrm->fb_base, sdrm->fb_size,
+				"simple-framebuffer")) {
+		dev_err(sdrm->ddev->dev, "cannot reserve VMEM\n");
+		return -EIO;
+	}
+
+	sdrm->fb_map = ioremap(sdrm->fb_base, sdrm->fb_size);
+	if (!sdrm->fb_map) {
+		dev_err(sdrm->ddev->dev, "cannot remap VMEM\n");
+		ret = -EIO;
+		goto err_region;
+	}
+
+	return 0;
+
+err_region:
+	release_mem_region(sdrm->fb_base, sdrm->fb_size);
+	return ret;
+}
+
+void sdrm_pdev_destroy(struct sdrm_device *sdrm)
+{
+	if (sdrm->fb_map) {
+		iounmap(sdrm->fb_map);
+		release_mem_region(sdrm->fb_base, sdrm->fb_size);
+		sdrm->fb_map = NULL;
+	}
+}
+
+static int sdrm_simplefb_probe(struct platform_device *pdev)
+{
+	return drm_platform_init(&sdrm_drm_driver, pdev);
+}
+
+static int sdrm_simplefb_remove(struct platform_device *pdev)
+{
+	drm_platform_exit(&sdrm_drm_driver, pdev);
+
+	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)
+{
+	return platform_driver_register(&sdrm_simplefb_driver);
+}
+
+static void __exit sdrm_exit(void)
+{
+	platform_driver_unregister(&sdrm_simplefb_driver);
+}
+
+module_init(sdrm_init);
+module_exit(sdrm_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("David Herrmann <dh.herrmann@gmail.com>");
+MODULE_DESCRIPTION("Simple firmware framebuffer DRM driver");
diff --git a/drivers/gpu/drm/simpledrm/simpledrm_main.c b/drivers/gpu/drm/simpledrm/simpledrm_main.c
new file mode 100644
index 0000000..dcc3d0a
--- /dev/null
+++ b/drivers/gpu/drm/simpledrm/simpledrm_main.c
@@ -0,0 +1,330 @@
+/*
+ * SimpleDRM firmware framebuffer driver
+ * Copyright (c) 2012-2013 David Herrmann <dh.herrmann@gmail.com>
+ */
+
+/*
+ * 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 <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/string.h>
+#include <drm/drmP.h>
+#include <drm/drm_crtc.h>
+#include "simpledrm.h"
+
+/* crtcs */
+
+static int sdrm_crtc_set_config(struct drm_mode_set *set)
+{
+	struct drm_device *ddev;
+	struct sdrm_device *sdrm;
+	struct sdrm_framebuffer *fb;
+
+	if (!set || !set->crtc)
+		return -EINVAL;
+
+	ddev = set->crtc->dev;
+	sdrm = ddev->dev_private;
+
+	if (set->crtc != &sdrm->crtc)
+		return -EINVAL;
+
+	if (!set->mode || !set->fb || !set->num_connectors) {
+		sdrm->conn.encoder = NULL;
+		sdrm->conn.dpms = DRM_MODE_DPMS_OFF;
+		sdrm->enc.crtc = NULL;
+		sdrm->crtc.fb = NULL;
+		sdrm->crtc.enabled = false;
+		return 0;
+	}
+
+	fb = to_sdrm_fb(set->fb);
+
+	if (set->num_connectors != 1 || set->connectors[0] != &sdrm->conn)
+		return -EINVAL;
+	if (set->x || set->y)
+		return -EINVAL;
+	if (set->mode->hdisplay != sdrm->fb_width ||
+	    set->mode->vdisplay != sdrm->fb_height)
+		return -EINVAL;
+
+	sdrm->conn.encoder = &sdrm->enc;
+	sdrm->conn.dpms = DRM_MODE_DPMS_ON;
+	sdrm->enc.crtc = &sdrm->crtc;
+	sdrm->crtc.fb = set->fb;
+	sdrm->crtc.enabled = true;
+	sdrm->crtc.mode = *set->mode;
+	sdrm->crtc.hwmode = *set->mode;
+	sdrm->crtc.x = 0;
+	sdrm->crtc.y = 0;
+
+	drm_calc_timestamping_constants(&sdrm->crtc);
+	return 0;
+}
+
+static const struct drm_crtc_funcs sdrm_crtc_ops = {
+	.set_config = sdrm_crtc_set_config,
+	.destroy = drm_crtc_cleanup,
+};
+
+/* encoders */
+
+static const struct drm_encoder_funcs sdrm_enc_ops = {
+	.destroy = drm_encoder_cleanup,
+};
+
+/* connectors */
+
+static void sdrm_conn_dpms(struct drm_connector *conn, int mode)
+{
+	conn->dpms = mode;
+}
+
+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 int sdrm_conn_fill_modes(struct drm_connector *conn, uint32_t max_x,
+				uint32_t max_y)
+{
+	struct sdrm_device *sdrm = conn->dev->dev_private;
+	struct drm_display_mode *mode;
+	int ret;
+
+	if (conn->force == DRM_FORCE_ON)
+		conn->status = connector_status_connected;
+	else if (conn->force)
+		conn->status = connector_status_disconnected;
+	else
+		conn->status = connector_status_connected;
+
+	list_for_each_entry(mode, &conn->modes, head)
+		mode->status = MODE_UNVERIFIED;
+
+	mode = drm_gtf_mode(sdrm->ddev, sdrm->fb_width, sdrm->fb_height,
+			    60, 0, 0);
+	if (mode) {
+		mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+		drm_mode_probed_add(conn, mode);
+		sdrm->mode = mode;
+		drm_mode_connector_list_update(conn);
+		ret = 1;
+	} else {
+		ret = 0;
+	}
+
+	if (max_x && max_y)
+		drm_mode_validate_size(conn->dev, &conn->modes,
+				       max_x, max_y, 0);
+
+	drm_mode_prune_invalid(conn->dev, &conn->modes, false);
+	if (list_empty(&conn->modes))
+		return 0;
+
+	drm_mode_sort(&conn->modes);
+
+	list_for_each_entry(mode, &conn->modes, head) {
+		mode->vrefresh = drm_mode_vrefresh(mode);
+		drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V);
+	}
+
+	return ret;
+}
+
+static void sdrm_conn_destroy(struct drm_connector *conn)
+{
+	/* Remove the fake-connector from sysfs and then let the DRM core
+	 * clean up all associated resources. */
+	if (device_is_registered(&conn->kdev))
+		drm_sysfs_connector_remove(conn);
+	drm_connector_cleanup(conn);
+}
+
+static const struct drm_connector_funcs sdrm_conn_ops = {
+	.dpms = sdrm_conn_dpms,
+	.detect = sdrm_conn_detect,
+	.fill_modes = sdrm_conn_fill_modes,
+	.destroy = sdrm_conn_destroy,
+};
+
+/* framebuffers */
+
+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,
+	.destroy = sdrm_fb_destroy,
+};
+
+static struct drm_framebuffer *sdrm_fb_create(struct drm_device *ddev,
+					      struct drm_file *dfile,
+					      struct drm_mode_fb_cmd2 *cmd)
+{
+	struct sdrm_device *sdrm = ddev->dev_private;
+	struct sdrm_framebuffer *fb;
+	struct drm_gem_object *gobj;
+	int ret, i;
+	void *err;
+
+	if (cmd->flags || cmd->pixel_format != sdrm->fb_format)
+		return ERR_PTR(-EINVAL);
+	if (cmd->height != sdrm->fb_height || cmd->width != sdrm->fb_width)
+		return ERR_PTR(-EINVAL);
+	if (cmd->offsets[0] || cmd->pitches[0] != sdrm->fb_stride)
+		return ERR_PTR(-EINVAL);
+
+	gobj = drm_gem_object_lookup(ddev, 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];
+	for (i = 1; i < 4; i++) {
+		fb->base.pitches[i] = 0;
+		fb->base.offsets[i] = 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);
+
+	ret = drm_framebuffer_init(ddev, &fb->base, &sdrm_fb_ops);
+	if (ret < 0) {
+		err = ERR_PTR(ret);
+		goto err_free;
+	}
+
+	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,
+};
+
+/* initialization */
+
+int sdrm_drm_load(struct drm_device *ddev, unsigned long flags)
+{
+	struct sdrm_device *sdrm;
+	int ret;
+
+	sdrm = kzalloc(sizeof(*sdrm), GFP_KERNEL);
+	if (!sdrm)
+		return -ENOMEM;
+
+	sdrm->ddev = ddev;
+	ddev->dev_private = sdrm;
+
+	ddev->devname = kstrdup("simpledrm", GFP_KERNEL);
+	if (!ddev->devname) {
+		ret = -ENOMEM;
+		goto err_free;
+	}
+
+	ret = sdrm_pdev_init(sdrm);
+	if (ret)
+		goto err_name;
+
+	drm_mode_config_init(ddev);
+	ddev->mode_config.min_width = 0;
+	ddev->mode_config.min_height = 0;
+	ddev->mode_config.max_width = 8192;
+	ddev->mode_config.max_height = 8192;
+	ddev->mode_config.funcs = &sdrm_mode_config_ops;
+
+	ret = drm_crtc_init(ddev, &sdrm->crtc, &sdrm_crtc_ops);
+	if (ret)
+		goto err_cleanup;
+
+	sdrm->enc.possible_crtcs = 1;
+	sdrm->enc.possible_clones = 0;
+	ret = drm_encoder_init(ddev, &sdrm->enc, &sdrm_enc_ops,
+			       DRM_MODE_ENCODER_VIRTUAL);
+	if (ret)
+		goto err_cleanup;
+
+	sdrm->conn.display_info.width_mm = 0;
+	sdrm->conn.display_info.height_mm = 0;
+	sdrm->conn.interlace_allowed = false;
+	sdrm->conn.doublescan_allowed = false;
+	sdrm->conn.polled = 0;
+	ret = drm_connector_init(ddev, &sdrm->conn, &sdrm_conn_ops,
+				 DRM_MODE_CONNECTOR_VIRTUAL);
+	if (ret)
+		goto err_cleanup;
+
+	ret = drm_mode_connector_attach_encoder(&sdrm->conn, &sdrm->enc);
+	if (ret)
+		goto err_cleanup;
+
+	ret = drm_sysfs_connector_add(&sdrm->conn);
+	if (ret)
+		goto err_cleanup;
+
+	return 0;
+
+err_cleanup:
+	drm_mode_config_cleanup(ddev);
+	sdrm_pdev_destroy(sdrm);
+err_name:
+	kfree(ddev->devname);
+	ddev->devname = NULL;
+err_free:
+	kfree(sdrm);
+	return ret;
+}
+
+int sdrm_drm_unload(struct drm_device *ddev)
+{
+	struct sdrm_device *sdrm = ddev->dev_private;
+
+	drm_mode_config_cleanup(ddev);
+	sdrm_pdev_destroy(sdrm);
+	kfree(sdrm);
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/simpledrm/simpledrm_mem.c b/drivers/gpu/drm/simpledrm/simpledrm_mem.c
new file mode 100644
index 0000000..afadcf5
--- /dev/null
+++ b/drivers/gpu/drm/simpledrm/simpledrm_mem.c
@@ -0,0 +1,254 @@
+/*
+ * SimpleDRM firmware framebuffer driver
+ * Copyright (c) 2012-2013 David Herrmann <dh.herrmann@gmail.com>
+ */
+
+/*
+ * 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 <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/string.h>
+#include <drm/drmP.h>
+#include "simpledrm.h"
+
+/*
+ * Create GEM Object
+ * Allocates a new GEM object to manage the physical memory at @fb_base with
+ * size @fb_size. Both parameters must be page-aligned and point to the
+ * physical memory of the framebuffer to manage. They must not have a
+ * "struct page" and have to be reserved before.
+ * It is the callers responsibility to create only one object per framebuffer.
+ */
+static struct sdrm_gem_object *sdrm_gem_alloc_object(struct drm_device *ddev,
+						     unsigned long fb_base,
+						     unsigned long fb_size)
+{
+	struct sdrm_gem_object *obj;
+
+	WARN_ON((fb_base & ~PAGE_MASK) != 0);
+	WARN_ON((fb_size & ~PAGE_MASK) != 0);
+
+	/* align to page-size */
+	fb_size = fb_size + (fb_base & ~PAGE_MASK);
+	fb_base = fb_base & PAGE_MASK;
+	fb_size = PAGE_ALIGN(fb_size);
+
+	if (fb_base + fb_size < fb_base)
+		return NULL;
+
+	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
+	if (!obj)
+		return NULL;
+	obj->fb_base = fb_base;
+	obj->fb_size = fb_size;
+
+	if (drm_gem_private_object_init(ddev, &obj->base, fb_size)) {
+		kfree(obj);
+		return NULL;
+	}
+
+	return obj;
+}
+
+/* drm_gem_object_alloc() is not supported */
+int sdrm_gem_init_object(struct drm_gem_object *gobj)
+{
+	return -EINVAL;
+}
+
+/*
+ * Unmap GEM Object
+ * Destroy any memory-mappings that user-space created on this object. Note
+ * that this will cause SIGBUS errors if user-space continues writing to it.
+ * There is no way to remap the pages in fault-handlers as this is not what
+ * we want. You should destroy the mappings only when destroying the object
+ * so no remapping will be needed.
+ * It's the callers responsibility to prevent any further mappings. This only
+ * destroys all current mappings.
+ */
+void sdrm_gem_unmap_object(struct sdrm_gem_object *obj)
+{
+	struct drm_device *ddev = obj->base.dev;
+	loff_t off;
+
+	off = obj->base.map_list.hash.key << PAGE_SHIFT;
+	unmap_mapping_range(ddev->dev_mapping, off, obj->fb_size, 1);
+}
+
+/*
+ * Free GEM Object
+ * Frees the given GEM object. It does not release the framebuffer memory that
+ * was passed during allocation, but destroys all user-space mappings.
+ */
+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;
+	struct sdrm_device *sdrm = ddev->dev_private;
+
+	if (sdrm->fb_obj == obj)
+		sdrm->fb_obj = NULL;
+
+	sdrm_gem_unmap_object(obj);
+
+	if (gobj->map_list.map)
+		drm_gem_free_mmap_offset(gobj);
+	drm_gem_object_release(gobj);
+	kfree(obj);
+}
+
+/*
+ * Create Dumb Buffer
+ * IOCTL backend for dumb-buffers. We only support one framebuffer per
+ * simple-DRM device so this function fails if there is already a framebuffer
+ * allocated. If not, an initial GEM-object plus framebuffer is created and
+ * forwarded to the caller.
+ *
+ * We could try to kill off the previous framebuffer and create a new one for
+ * the caller. However, user-space often allocates two buffers in a row to
+ * allow double-buffering. If we kill the previous buffer, user-space would
+ * have no chance to notice that only one buffer is available.
+ *
+ * So user-space must make sure they either destroy their buffer when dropping
+ * DRM-Master or leave the CRTC intact and let others share the buffer via
+ * drmModeGetFB().
+ *
+ * The buffer parameters must be the same as from the default-mode of the CRTC.
+ * No other sizes can be supported!
+ */
+int sdrm_dumb_create(struct drm_file *dfile, struct drm_device *ddev,
+		     struct drm_mode_create_dumb *args)
+{
+	struct drm_device *dev = dfile->minor->dev;
+	struct sdrm_device *sdrm = dev->dev_private;
+	struct sdrm_gem_object *obj;
+	int ret;
+
+	/* only allow one framebuffer at a time */
+	if (sdrm->fb_obj)
+		return -ENOMEM;
+
+	if (args->width != sdrm->fb_width ||
+	    args->height != sdrm->fb_height ||
+	    args->bpp != sdrm->fb_bpp ||
+	    args->flags)
+		return -EINVAL;
+
+	args->pitch = sdrm->fb_stride;
+	args->size = sdrm->fb_size;
+	obj = sdrm_gem_alloc_object(ddev, sdrm->fb_base, sdrm->fb_size);
+	if (!obj)
+		return -ENOMEM;
+
+	ret = drm_gem_handle_create(dfile, &obj->base, &args->handle);
+	if (ret) {
+		drm_gem_object_unreference(&obj->base);
+		return ret;
+	}
+
+	/* fb_obj is cleared by sdrm_gem_free_object() */
+	sdrm->fb_obj = obj;
+	drm_gem_object_unreference(&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 ret;
+
+	mutex_lock(&ddev->struct_mutex);
+
+	gobj = drm_gem_object_lookup(ddev, dfile, handle);
+	if (!gobj) {
+		ret = -ENOENT;
+		goto out_unlock;
+	}
+
+	if (!gobj->map_list.map) {
+		ret = drm_gem_create_mmap_offset(gobj);
+		if (ret)
+			goto out_unref;
+	}
+
+	*offset = gobj->map_list.hash.key << PAGE_SHIFT;
+
+out_unref:
+	drm_gem_object_unreference(gobj);
+out_unlock:
+	mutex_unlock(&ddev->struct_mutex);
+	return ret;
+}
+
+/*
+ * mmap ioctl
+ * We simply map the physical range of the FB into user-space as requested. We
+ * perform few sanity-checks and then let io_remap_pfn_range() do all the work.
+ * No vma_ops are needed this way as pages are either cleared or present.
+ */
+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_gem_mm *mm = dev->mm_private;
+	struct drm_local_map *map;
+	struct drm_hash_item *item;
+	struct drm_gem_object *gobj;
+	struct sdrm_gem_object *obj;
+	unsigned long len;
+	int ret;
+
+	mutex_lock(&dev->struct_mutex);
+
+	if (drm_ht_find_item(&mm->offset_hash, vma->vm_pgoff, &item)) {
+		mutex_unlock(&dev->struct_mutex);
+		return drm_mmap(filp, vma);
+	}
+
+	map = drm_hash_entry(item, struct drm_map_list, hash)->map;
+	if (!map ||
+	    ((map->flags & _DRM_RESTRICTED) && !capable(CAP_SYS_ADMIN))) {
+		ret =  -EPERM;
+		goto out_unlock;
+	}
+
+	gobj = map->handle;
+	obj = to_sdrm_bo(gobj);
+
+	vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
+	vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
+	vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
+	/* FIXME: do we need fb_pgprotect() here? */
+
+	/* verify mapping size */
+	len = vma->vm_end - vma->vm_start;
+	if (len > obj->fb_size) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
+	/* This object is _not_ referenced here. Therefore, we _must_ destroy
+	 * the mapping before destroying the bo! */
+
+	ret = io_remap_pfn_range(vma, vma->vm_start, obj->fb_base >> PAGE_SHIFT,
+				 obj->fb_size, vma->vm_page_prot);
+
+out_unlock:
+	mutex_unlock(&dev->struct_mutex);
+	return ret;
+}
-- 
1.8.3.1

Re: [RFC 3/6] drm: add SimpleDRM driver

[Andy Lutomirski]

On 06/24/2013 03:27 PM, David Herrmann wrote:
> +	sdrm->fb_map = ioremap(sdrm->fb_base, sdrm->fb_size);

This should probably be ioremap_wc.  Otherwise it will be *really* slow
if used in legacy mode and it may cause conflicts with the
pgprot_writecombine mode for mmap.

(Watching boot messages go by on fbcon on efifb was like using an old
2400 baud modem before I made the corresponding change to efifb.)

--Andy

Re: [RFC 3/6] drm: add SimpleDRM driver

[David Herrmann]

Hi

On Tue, Jun 25, 2013 at 3:05 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On 06/24/2013 03:27 PM, David Herrmann wrote:
>> +     sdrm->fb_map = ioremap(sdrm->fb_base, sdrm->fb_size);
>
> This should probably be ioremap_wc.  Otherwise it will be *really* slow
> if used in legacy mode and it may cause conflicts with the
> pgprot_writecombine mode for mmap.

Whoops, yepp, I fixed that.

Thanks
David

Re: [RFC 3/6] drm: add SimpleDRM driver

[Stephen Warren]

On 06/24/2013 04:27 PM, David Herrmann 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 one dumb-buffer and attach it to the CRTC. Only if
> the buffer is destroyed, a new buffer can be created. The buffer is
> directly mapped into user-space, so we have only resources for a single
> buffer. Otherwise, shadow buffers plus damage-request would be needed.

> diff --git a/drivers/gpu/drm/simpledrm/Kconfig b/drivers/gpu/drm/simpledrm/Kconfig

> +config DRM_SIMPLEDRM
> +	tristate "Simple firmware framebuffer DRM driver"
> +	depends on DRM && !FB_SIMPLE
> +	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, x86 VESA
> +	  BIOS Extensions (VBE), EFI framebuffers

DT objects, yes. I'm not sure it's quite true to say it actually
directly supports VBE or EFI FBs; it's more the code in patch 2/6 that
supports those. I guess this is a bit nit-picky of a distinction though.

> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_drv.c b/drivers/gpu/drm/simpledrm/simpledrm_drv.c

> +static int parse_dt(struct platform_device *pdev,
> +		    struct simplefb_platform_data *mode)

> +	strlcpy(mode->format, format, sizeof(mode->format));

Even here, I believe the DT data sticks around so just copying the
pointer should be safe. It'd be worth validating that for sure though.

I didn't review the DRM stuff here since I'm not at all familiar with DRM.

Re: [RFC 3/6] drm: add SimpleDRM driver

[David Herrmann]

Hi

On Wed, Jun 26, 2013 at 10:58 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 06/24/2013 04:27 PM, David Herrmann 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 one dumb-buffer and attach it to the CRTC. Only if
>> the buffer is destroyed, a new buffer can be created. The buffer is
>> directly mapped into user-space, so we have only resources for a single
>> buffer. Otherwise, shadow buffers plus damage-request would be needed.
>
>> diff --git a/drivers/gpu/drm/simpledrm/Kconfig b/drivers/gpu/drm/simpledrm/Kconfig
>
>> +config DRM_SIMPLEDRM
>> +     tristate "Simple firmware framebuffer DRM driver"
>> +     depends on DRM && !FB_SIMPLE
>> +     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, x86 VESA
>> +       BIOS Extensions (VBE), EFI framebuffers
>
> DT objects, yes. I'm not sure it's quite true to say it actually
> directly supports VBE or EFI FBs; it's more the code in patch 2/6 that
> supports those. I guess this is a bit nit-picky of a distinction though.

I initially intended to add support for "platform-framebuffer"
objects, too. This would make vesafb/... obsolete but I dropped that
idea for now. I will fix the Kconfig description.

>> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_drv.c b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
>
>> +static int parse_dt(struct platform_device *pdev,
>> +                 struct simplefb_platform_data *mode)
>
>> +     strlcpy(mode->format, format, sizeof(mode->format));
>
> Even here, I believe the DT data sticks around so just copying the
> pointer should be safe. It'd be worth validating that for sure though.

Yep, indeed, I fixed that.

Thanks!
David

> I didn't review the DRM stuff here since I'm not at all familiar with DRM.

[RFC 4/6] drm: simpledrm: add fbdev fallback support

[David Herrmann]

Create a simple fbdev device during SimpleDRM setup so legacy user-space
and fbcon can use it.

fbdev deletion is quite buggy. A unregister_framebuffer() call followed by
a printk() causes NULL-derefs in hide_cursor() and other places in the VT
layer. Hence, we leak the fbdev device currently to make the VT layer
happy. This needs to be fixed soon! Otherwise, we need a "depends !VT"
line for SimpleDRM.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/simpledrm/Kconfig           |  11 ++
 drivers/gpu/drm/simpledrm/Makefile          |   4 +
 drivers/gpu/drm/simpledrm/simpledrm.h       |  22 ++++
 drivers/gpu/drm/simpledrm/simpledrm_fbdev.c | 180 ++++++++++++++++++++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_main.c  |   2 +
 5 files changed, 219 insertions(+)
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_fbdev.c

diff --git a/drivers/gpu/drm/simpledrm/Kconfig b/drivers/gpu/drm/simpledrm/Kconfig
index 1d4f38e..7936211 100644
--- a/drivers/gpu/drm/simpledrm/Kconfig
+++ b/drivers/gpu/drm/simpledrm/Kconfig
@@ -12,7 +12,18 @@ config DRM_SIMPLEDRM
 	  SimpleDRM supports: "simple-framebuffer" DeviceTree objects, x86 VESA
 	  BIOS Extensions (VBE), EFI framebuffers
 
+	  If fbdev support is enabled, this driver will also provide an fbdev
+	  compatibility layer.
+
 	  If unsure, say Y.
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called simpledrm.
+
+config DRM_SIMPLEDRM_FBDEV
+	bool
+	depends on DRM_SIMPLEDRM && FB
+	default y
+	select FB_CFB_FILLRECT
+	select FB_CFB_COPYAREA
+	select FB_CFB_IMAGEBLIT
diff --git a/drivers/gpu/drm/simpledrm/Makefile b/drivers/gpu/drm/simpledrm/Makefile
index 2d474a5..e77bd9b 100644
--- a/drivers/gpu/drm/simpledrm/Makefile
+++ b/drivers/gpu/drm/simpledrm/Makefile
@@ -2,4 +2,8 @@ ccflags-y := -Iinclude/drm
 
 simpledrm-y := simpledrm_drv.o simpledrm_main.o simpledrm_mem.o
 
+ifdef CONFIG_DRM_SIMPLEDRM_FBDEV
+	simpledrm-y += simpledrm_fbdev.o
+endif
+
 obj-$(CONFIG_DRM_SIMPLEDRM) := simpledrm.o
diff --git a/drivers/gpu/drm/simpledrm/simpledrm.h b/drivers/gpu/drm/simpledrm/simpledrm.h
index 279d847..cfd99f9 100644
--- a/drivers/gpu/drm/simpledrm/simpledrm.h
+++ b/drivers/gpu/drm/simpledrm/simpledrm.h
@@ -48,6 +48,9 @@ struct sdrm_device {
 	struct drm_encoder enc;
 	struct drm_connector conn;
 	struct drm_display_mode *mode;
+
+	/* fbdev */
+	struct fb_info *fbdev;
 };
 
 int sdrm_drm_load(struct drm_device *ddev, unsigned long flags);
@@ -88,4 +91,23 @@ struct sdrm_framebuffer {
 
 #define to_sdrm_fb(x) container_of(x, struct sdrm_framebuffer, base)
 
+/* simpledrm fbdev helpers */
+
+#ifdef CONFIG_DRM_SIMPLEDRM_FBDEV
+
+void sdrm_fbdev_init(struct sdrm_device *sdrm);
+void sdrm_fbdev_cleanup(struct sdrm_device *sdrm);
+
+#else /* CONFIG_DRM_SIMPLEDRM_FBDEV */
+
+static inline void sdrm_fbdev_init(struct sdrm_device *sdrm)
+{
+}
+
+static inline void sdrm_fbdev_cleanup(struct sdrm_device *sdrm)
+{
+}
+
+#endif /* CONFIG_DRM_SIMPLEDRM_FBDEV */
+
 #endif /* SDRM_DRV_H */
diff --git a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
new file mode 100644
index 0000000..40a2696
--- /dev/null
+++ b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
@@ -0,0 +1,180 @@
+/*
+ * SimpleDRM firmware framebuffer driver
+ * Copyright (c) 2012-2013 David Herrmann <dh.herrmann@gmail.com>
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+/*
+ * fbdev compatibility layer
+ * We provide a basic fbdev device for the same framebuffer that is used for
+ * the pseudo CRTC.
+ */
+
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/string.h>
+#include <linux/fb.h>
+#include "simpledrm.h"
+
+struct sdrm_fbdev {
+	u32 palette[256];
+};
+
+static int sdrm_fbdev_setcolreg(unsigned regno, unsigned red, unsigned green,
+				unsigned blue, unsigned transp,
+				struct fb_info *info)
+{
+	/*
+	 * Set a single color register. The values supplied are
+	 * already rounded down to the hardware's capabilities
+	 * (according to the entries in the `var' structure). Return != 0 for
+	 * invalid regno.
+	 */
+
+	if (regno >= info->cmap.len)
+		return 1;
+	if (info->var.bits_per_pixel == 8)
+		return -EINVAL;
+	if (regno >= 16)
+		return 0;
+
+	switch (info->var.bits_per_pixel) {
+	case 16:
+		if (info->var.red.offset == 10) {
+			/* 1:5:5:5 */
+			((u32*) (info->pseudo_palette))[regno] =
+				((red   & 0xf800) >>  1) |
+				((green & 0xf800) >>  6) |
+				((blue  & 0xf800) >> 11);
+		} else {
+			/* 0:5:6:5 */
+			((u32*) (info->pseudo_palette))[regno] =
+				((red   & 0xf800)      ) |
+				((green & 0xfc00) >>  5) |
+				((blue  & 0xf800) >> 11);
+		}
+		break;
+	case 24:
+	case 32:
+		red   >>= 8;
+		green >>= 8;
+		blue  >>= 8;
+		((u32*) (info->pseudo_palette))[regno] =
+			(red   << info->var.red.offset)   |
+			(green << info->var.green.offset) |
+			(blue  << info->var.blue.offset);
+		break;
+	}
+
+	return 0;
+}
+
+static struct fb_ops sdrm_fbdev_ops = {
+	.owner		= THIS_MODULE,
+	.fb_setcolreg	= sdrm_fbdev_setcolreg,
+	.fb_fillrect	= cfb_fillrect,
+	.fb_copyarea	= cfb_copyarea,
+	.fb_imageblit	= cfb_imageblit,
+};
+
+void sdrm_fbdev_init(struct sdrm_device *sdrm)
+{
+	struct sdrm_fbdev *fb;
+	struct fb_info *info;
+	int ret;
+
+	info = framebuffer_alloc(sizeof(struct sdrm_fbdev), sdrm->ddev->dev);
+	if (!info)
+		goto err_out;
+
+	fb = info->par;
+	info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE;
+	info->pseudo_palette = fb->palette;
+	info->fbops = &sdrm_fbdev_ops;
+	info->screen_base = sdrm->fb_map;
+
+	strncpy(info->fix.id, "simpledrmfb", 15);
+	info->fix.type = FB_TYPE_PACKED_PIXELS;
+	info->fix.visual = FB_VISUAL_TRUECOLOR;
+	info->fix.accel = FB_ACCEL_NONE;
+	info->fix.smem_start = (unsigned long)sdrm->fb_base;
+	info->fix.smem_len = sdrm->fb_size;
+	info->fix.line_length = sdrm->fb_stride;
+
+	info->var.activate = FB_ACTIVATE_NOW;
+	info->var.vmode = FB_VMODE_NONINTERLACED;
+	info->var.bits_per_pixel = sdrm->fb_bpp;
+	info->var.height = -1;
+	info->var.width = -1;
+	info->var.xres = sdrm->fb_width;
+	info->var.yres = sdrm->fb_height;
+	info->var.xres_virtual = info->var.xres;
+	info->var.yres_virtual = info->var.yres;
+	info->var.red = sdrm->fb_sformat->red;
+	info->var.green = sdrm->fb_sformat->green;
+	info->var.blue = sdrm->fb_sformat->blue;
+	info->var.transp = sdrm->fb_sformat->transp;
+
+	/* some dummy values for timing to make fbset happy */
+	info->var.pixclock = 10000000 / info->var.xres * 1000 / info->var.yres;
+	info->var.left_margin = (info->var.xres / 8) & 0xf8;
+	info->var.right_margin = 32;
+	info->var.upper_margin = 16;
+	info->var.lower_margin = 4;
+	info->var.hsync_len = (info->var.xres / 8) & 0xf8;
+	info->var.vsync_len = 4;
+
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures)
+		goto err_free;
+
+	info->apertures->ranges[0].base = (unsigned long)sdrm->fb_base;
+	info->apertures->ranges[0].size = sdrm->fb_size;
+
+	sdrm->fbdev = info;
+	ret = register_framebuffer(info);
+	if (ret < 0)
+		goto err_free;
+
+	dev_info(sdrm->ddev->dev, "fbdev frontend %s as fb%d\n",
+		 info->fix.id, info->node);
+
+	return;
+
+err_free:
+	framebuffer_release(info);
+err_out:
+	dev_warn(sdrm->ddev->dev, "cannot create fbdev frontend\n");
+}
+
+void sdrm_fbdev_cleanup(struct sdrm_device *sdrm)
+{
+	struct fb_info *info;
+
+	if (!sdrm->info)
+		return;
+
+	dev_info(sdrm->ddev->dev, "fbdev cleanup\n");
+	info = sdrm->fbdev;
+	sdrm->fbdev = NULL;
+
+	unregister_framebuffer(info);
+
+	/*
+	 * FIXME: unregister_framebuffer() may silently fail, which is odd
+	 * because there is no sane way for us to detect when to release the
+	 * framebuffer.
+	 * If we fix unbind_console() to not fail during framebuffer
+	 * unregistration, we end up with NULL-derefs in the VT layer
+	 * afterwards. So lets just leak the FB here for the VT layer's sake.
+	 */
+	/* framebuffer_release(info); */
+}
diff --git a/drivers/gpu/drm/simpledrm/simpledrm_main.c b/drivers/gpu/drm/simpledrm/simpledrm_main.c
index dcc3d0a..ffa1abb 100644
--- a/drivers/gpu/drm/simpledrm/simpledrm_main.c
+++ b/drivers/gpu/drm/simpledrm/simpledrm_main.c
@@ -305,6 +305,7 @@ int sdrm_drm_load(struct drm_device *ddev, unsigned long flags)
 	if (ret)
 		goto err_cleanup;
 
+	sdrm_fbdev_init(sdrm);
 	return 0;
 
 err_cleanup:
@@ -322,6 +323,7 @@ int sdrm_drm_unload(struct drm_device *ddev)
 {
 	struct sdrm_device *sdrm = ddev->dev_private;
 
+	sdrm_fbdev_cleanup(sdrm);
 	drm_mode_config_cleanup(ddev);
 	sdrm_pdev_destroy(sdrm);
 	kfree(sdrm);
-- 
1.8.3.1

Re: [RFC 4/6] drm: simpledrm: add fbdev fallback support

[Stephen Warren]

On 06/24/2013 04:27 PM, David Herrmann wrote:
> Create a simple fbdev device during SimpleDRM setup so legacy user-space
> and fbcon can use it.
> 
> fbdev deletion is quite buggy. A unregister_framebuffer() call followed by
> a printk() causes NULL-derefs in hide_cursor() and other places in the VT
> layer. Hence, we leak the fbdev device currently to make the VT layer
> happy. This needs to be fixed soon! Otherwise, we need a "depends !VT"
> line for SimpleDRM.

> diff --git a/drivers/gpu/drm/simpledrm/Makefile b/drivers/gpu/drm/simpledrm/Makefile

>  simpledrm-y := simpledrm_drv.o simpledrm_main.o simpledrm_mem.o
>  
> +ifdef CONFIG_DRM_SIMPLEDRM_FBDEV
> +	simpledrm-y += simpledrm_fbdev.o
> +endif

I think that's:

+ simpledrm-$(CONFIG_DRM_SIMPLEDRM_FBDEV) += simpledrm_fbdev.o

Re: [RFC 4/6] drm: simpledrm: add fbdev fallback support

[David Herrmann]

Hi

On Wed, Jun 26, 2013 at 10:59 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 06/24/2013 04:27 PM, David Herrmann wrote:
>> Create a simple fbdev device during SimpleDRM setup so legacy user-space
>> and fbcon can use it.
>>
>> fbdev deletion is quite buggy. A unregister_framebuffer() call followed by
>> a printk() causes NULL-derefs in hide_cursor() and other places in the VT
>> layer. Hence, we leak the fbdev device currently to make the VT layer
>> happy. This needs to be fixed soon! Otherwise, we need a "depends !VT"
>> line for SimpleDRM.
>
>> diff --git a/drivers/gpu/drm/simpledrm/Makefile b/drivers/gpu/drm/simpledrm/Makefile
>
>>  simpledrm-y := simpledrm_drv.o simpledrm_main.o simpledrm_mem.o
>>
>> +ifdef CONFIG_DRM_SIMPLEDRM_FBDEV
>> +     simpledrm-y += simpledrm_fbdev.o
>> +endif
>
> I think that's:
>
> + simpledrm-$(CONFIG_DRM_SIMPLEDRM_FBDEV) += simpledrm_fbdev.o

Ugh, I got errors trying that because SIMPLEDRM_FBDEV is a boolean but
SIMPLEDRM is a tristate. But I guess I tried:
obj-$(CONFIG_DRM_SIMPLEDRM_FBDEV) += simpledrm_fbdev.o
which obviously fails if SIMPLEDRM=m and SIMPLEDRM_FBDEV=y. I will try
to fix that.

Thanks
David

[RFC 5/6] drm: add helpers to kick out firmware drivers

[David Herrmann]

If we load a real hardware DRM driver, we want all firmware drivers to be
unloaded. Historically, this was done via
remove_conflicting_framebuffers(), but for DRM drivers (like SimpleDRM) we
need a new way.

This patch introduces DRIVER_FIRMWARE and DRM apertures to provide a quite
similar way to kick out firmware DRM drivers. Additionally, unlike the
fbdev equivalent, DRM firmware drivers can now query the system whether a
real hardware driver is already loaded and prevent loading themselves.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_pci.c                  |   1 +
 drivers/gpu/drm/drm_platform.c             |   1 +
 drivers/gpu/drm/drm_stub.c                 | 107 +++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_usb.c                  |   1 +
 drivers/gpu/drm/simpledrm/simpledrm.h      |   1 +
 drivers/gpu/drm/simpledrm/simpledrm_drv.c  |   3 +-
 drivers/gpu/drm/simpledrm/simpledrm_main.c |  34 +++++++++
 include/drm/drmP.h                         |  26 +++++++
 8 files changed, 173 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 14194b6..4dcb2a4 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -366,6 +366,7 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 	}
 
 	list_add_tail(&dev->driver_item, &driver->device_list);
+	list_add_tail(&dev->global_item, &drm_devlist);
 
 	DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
 		 driver->name, driver->major, driver->minor, driver->patchlevel,
diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
index b8a282e..94923c8 100644
--- a/drivers/gpu/drm/drm_platform.c
+++ b/drivers/gpu/drm/drm_platform.c
@@ -88,6 +88,7 @@ int drm_get_platform_dev(struct platform_device *platdev,
 	}
 
 	list_add_tail(&dev->driver_item, &driver->device_list);
+	list_add_tail(&dev->global_item, &drm_devlist);
 
 	mutex_unlock(&drm_global_mutex);
 
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 16f3ec5..a433ab0 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -46,6 +46,9 @@ EXPORT_SYMBOL(drm_vblank_offdelay);
 unsigned int drm_timestamp_precision = 20;  /* Default to 20 usecs. */
 EXPORT_SYMBOL(drm_timestamp_precision);
 
+LIST_HEAD(drm_devlist);		/* device list; protected by global lock */
+EXPORT_SYMBOL(drm_devlist);
+
 /*
  * Default to use monotonic timestamps for wait-for-vblank and page-flip
  * complete events.
@@ -484,7 +487,9 @@ void drm_put_dev(struct drm_device *dev)
 
 	drm_put_minor(&dev->primary);
 
+	list_del(&dev->global_item);
 	list_del(&dev->driver_item);
+	kfree(dev->apertures);
 	kfree(dev->devname);
 	kfree(dev);
 }
@@ -507,3 +512,105 @@ void drm_unplug_dev(struct drm_device *dev)
 	mutex_unlock(&drm_global_mutex);
 }
 EXPORT_SYMBOL(drm_unplug_dev);
+
+void drm_unplug_dev_locked(struct drm_device *dev)
+{
+	/* for a USB device */
+	if (drm_core_check_feature(dev, DRIVER_MODESET))
+		drm_unplug_minor(dev->control);
+	drm_unplug_minor(dev->primary);
+
+	drm_device_set_unplugged(dev);
+
+	/* TODO: schedule drm_put_dev if open_count == 0 */
+}
+EXPORT_SYMBOL(drm_unplug_dev_locked);
+
+#define VGA_FB_PHYS 0xa0000
+
+static bool apertures_overlap(struct drm_device *dev,
+			      struct apertures_struct *ap,
+			      bool boot)
+{
+	unsigned int i, j;
+	struct aperture *a, *b;
+
+	if (!dev->apertures)
+		return false;
+
+	for (i = 0; i < dev->apertures->count; ++i) {
+		a = &dev->apertures->ranges[i];
+
+		if (boot && a->base == VGA_FB_PHYS)
+			return true;
+
+		for (j = 0; ap && j < ap->count; ++j) {
+			b = &ap->ranges[j];
+			if (a->base <= b->base && a->base + a->size > b->base)
+				return true;
+			if (b->base <= a->base && b->base + b->size > a->base)
+				return true;
+		}
+	}
+
+	return false;
+}
+
+/**
+ * Kick out firmware
+ *
+ * Virtually unplug any firmware graphics devices which overlap the given
+ * region. This must be called with the global-drm-mutex locked.
+ * This calls the kick_out_firmware() callback on any firmware DRM driver and
+ * after that remove_conflicting_framebuffers() to remove legacy fbdev
+ * framebuffers.
+ */
+void drm_kick_out_firmware(struct apertures_struct *ap, bool boot)
+{
+	struct drm_device *dev;
+
+	list_for_each_entry(dev, &drm_devlist, global_item) {
+		if (!drm_core_check_feature(dev, DRIVER_FIRMWARE))
+			continue;
+		if (apertures_overlap(dev, ap, boot)) {
+			DRM_INFO("kicking out firmware %s\n",
+				 dev->devname);
+			dev->driver->kick_out_firmware(dev);
+		}
+	}
+
+	remove_conflicting_framebuffers(ap, "DRM", boot);
+}
+EXPORT_SYMBOL(drm_kick_out_firmware);
+
+/**
+ * Verify that no driver uses firmware FBs
+ *
+ * Whenever you register a firmware framebuffer driver, you should store the
+ * apertures in @ap and test whether any other registered driver already
+ * claimed this area. Hence, if this function returns true, you should _not_
+ * register your driver!
+ */
+bool drm_is_firmware_used(struct apertures_struct *ap)
+{
+	struct drm_device *dev;
+	unsigned int i;
+	bool boot = false;
+
+	for (i = 0; ap && i < ap->count; ++i) {
+		if (ap->ranges[i].base == VGA_FB_PHYS) {
+			boot = true;
+			break;
+		}
+	}
+
+	list_for_each_entry(dev, &drm_devlist, global_item) {
+		if (dev->apert_boot && boot)
+			return true;
+		if (apertures_overlap(dev, ap, false))
+			return true;
+	}
+
+	return false;
+}
+EXPORT_SYMBOL(drm_is_firmware_used);
diff --git a/drivers/gpu/drm/drm_usb.c b/drivers/gpu/drm/drm_usb.c
index 34a156f..5ad8dba 100644
--- a/drivers/gpu/drm/drm_usb.c
+++ b/drivers/gpu/drm/drm_usb.c
@@ -50,6 +50,7 @@ int drm_get_usb_dev(struct usb_interface *interface,
 		goto err_g3;
 
 	list_add_tail(&dev->driver_item, &driver->device_list);
+	list_add_tail(&dev->global_item, &drm_devlist);
 
 	mutex_unlock(&drm_global_mutex);
 
diff --git a/drivers/gpu/drm/simpledrm/simpledrm.h b/drivers/gpu/drm/simpledrm/simpledrm.h
index cfd99f9..03373fe 100644
--- a/drivers/gpu/drm/simpledrm/simpledrm.h
+++ b/drivers/gpu/drm/simpledrm/simpledrm.h
@@ -55,6 +55,7 @@ struct sdrm_device {
 
 int sdrm_drm_load(struct drm_device *ddev, unsigned long flags);
 int sdrm_drm_unload(struct drm_device *ddev);
+void sdrm_drm_kick_out_firmware(struct drm_device *ddev);
 int sdrm_drm_mmap(struct file *filp, struct vm_area_struct *vma);
 int sdrm_pdev_init(struct sdrm_device *sdrm);
 void sdrm_pdev_destroy(struct sdrm_device *sdrm);
diff --git a/drivers/gpu/drm/simpledrm/simpledrm_drv.c b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
index 282752c..e5d0ce0 100644
--- a/drivers/gpu/drm/simpledrm/simpledrm_drv.c
+++ b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
@@ -36,9 +36,10 @@ static const struct file_operations sdrm_drm_fops = {
 };
 
 static struct drm_driver sdrm_drm_driver = {
-	.driver_features = DRIVER_MODESET | DRIVER_GEM,
+	.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_FIRMWARE,
 	.load = sdrm_drm_load,
 	.unload = sdrm_drm_unload,
+	.kick_out_firmware = sdrm_drm_kick_out_firmware,
 	.fops = &sdrm_drm_fops,
 
 	.gem_init_object = sdrm_gem_init_object,
diff --git a/drivers/gpu/drm/simpledrm/simpledrm_main.c b/drivers/gpu/drm/simpledrm/simpledrm_main.c
index ffa1abb..6b7696e 100644
--- a/drivers/gpu/drm/simpledrm/simpledrm_main.c
+++ b/drivers/gpu/drm/simpledrm/simpledrm_main.c
@@ -269,6 +269,21 @@ int sdrm_drm_load(struct drm_device *ddev, unsigned long flags)
 	if (ret)
 		goto err_name;
 
+	ddev->apertures = alloc_apertures(1);
+	if (!ddev->apertures) {
+		ret = -ENOMEM;
+		goto err_pdev;
+	}
+
+	ddev->apertures->ranges[0].base = sdrm->fb_base;
+	ddev->apertures->ranges[0].size = sdrm->fb_size;
+
+	if (drm_is_firmware_used(ddev->apertures)) {
+		dev_info(ddev->dev, "firmware framebuffer is already in use\n");
+		ret = -EBUSY;
+		goto err_apert;
+	}
+
 	drm_mode_config_init(ddev);
 	ddev->mode_config.min_width = 0;
 	ddev->mode_config.min_height = 0;
@@ -310,6 +325,9 @@ int sdrm_drm_load(struct drm_device *ddev, unsigned long flags)
 
 err_cleanup:
 	drm_mode_config_cleanup(ddev);
+err_apert:
+	kfree(ddev->apertures);
+err_pdev:
 	sdrm_pdev_destroy(sdrm);
 err_name:
 	kfree(ddev->devname);
@@ -326,7 +344,23 @@ int sdrm_drm_unload(struct drm_device *ddev)
 	sdrm_fbdev_cleanup(sdrm);
 	drm_mode_config_cleanup(ddev);
 	sdrm_pdev_destroy(sdrm);
+	kfree(ddev->apertures);
 	kfree(sdrm);
 
 	return 0;
 }
+
+void sdrm_drm_kick_out_firmware(struct drm_device *ddev)
+{
+	struct sdrm_device *sdrm = ddev->dev_private;
+
+	mutex_lock(&ddev->struct_mutex);
+
+	sdrm_fbdev_cleanup(sdrm);
+	drm_unplug_dev_locked(ddev);
+	if (sdrm->fb_obj)
+		sdrm_gem_unmap_object(sdrm->fb_obj);
+	sdrm_pdev_destroy(sdrm);
+
+	mutex_unlock(&ddev->struct_mutex);
+}
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 63d17ee..a19e710 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -47,6 +47,7 @@
 #include <linux/fs.h>
 #include <linux/proc_fs.h>
 #include <linux/init.h>
+#include <linux/fb.h>
 #include <linux/file.h>
 #include <linux/platform_device.h>
 #include <linux/pci.h>
@@ -156,6 +157,7 @@ int drm_err(const char *func, const char *format, ...);
 #define DRIVER_GEM         0x1000
 #define DRIVER_MODESET     0x2000
 #define DRIVER_PRIME       0x4000
+#define DRIVER_FIRMWARE    0x8000
 
 #define DRIVER_BUS_PCI 0x1
 #define DRIVER_BUS_PLATFORM 0x2
@@ -954,6 +956,23 @@ struct drm_driver {
 			    struct drm_device *dev,
 			    uint32_t handle);
 
+	/**
+	 * kick_out_firmware - kick out firmware driver
+	 * @dev: DRM device
+	 *
+	 * Iff this driver has DRIVER_FIRMWARE set, this function is called
+	 * when a real hw-driver is loaded and claims the framebuffer memory
+	 * that is mapped by the firmware driver. This is called with the
+	 * global drm mutex held.
+	 * Drivers should unmap any memory-mappings, disable the device and
+	 * schedule a driver removal.
+	 *
+	 * Note that a driver setting DRIVER_FIRMWARE is supposed to also set
+	 * the "apertures" information on @dev. It is used to match the memory
+	 * regions that are used by firmware drivers.
+	 */
+	void (*kick_out_firmware) (struct drm_device *dev);
+
 	/* Driver private ops for this object */
 	const struct vm_operations_struct *gem_vm_ops;
 
@@ -1077,6 +1096,7 @@ struct drm_pending_vblank_event {
  */
 struct drm_device {
 	struct list_head driver_item;	/**< list of devices per driver */
+	struct list_head global_item;	/**< global list of devices */
 	char *devname;			/**< For /proc/interrupts */
 	int if_version;			/**< Highest interface version set */
 
@@ -1218,6 +1238,8 @@ struct drm_device {
 	int switch_power_state;
 
 	atomic_t unplugged; /* device has been unplugged or gone away */
+	struct apertures_struct *apertures;	/**< fbmem apertures */
+	bool apert_boot;			/**< true if mapped as boot fb */
 };
 
 #define DRM_SWITCH_POWER_ON 0
@@ -1532,6 +1554,10 @@ extern void drm_master_put(struct drm_master **master);
 extern void drm_put_dev(struct drm_device *dev);
 extern int drm_put_minor(struct drm_minor **minor);
 extern void drm_unplug_dev(struct drm_device *dev);
+extern void drm_unplug_dev_locked(struct drm_device *dev);
+extern void drm_kick_out_firmware(struct apertures_struct *ap, bool boot);
+extern bool drm_is_firmware_used(struct apertures_struct *ap);
+extern struct list_head drm_devlist;
 extern unsigned int drm_debug;
 
 extern unsigned int drm_vblank_offdelay;
-- 
1.8.3.1

[RFC 6/6] drm: nouveau: kick out firmware drivers during probe

[David Herrmann]

Use the new DRM infrastructure to kick out firmware drivers before probing
the real driver.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/nouveau/nouveau_drm.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 383f4e6..418867f 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -233,13 +233,11 @@ nouveau_accel_init(struct nouveau_drm *drm)
 	nouveau_bo_move_init(drm);
 }
 
-static int nouveau_drm_probe(struct pci_dev *pdev,
-			     const struct pci_device_id *pent)
+static int nouveau_kick_out_firmware(struct drm_device *dev)
 {
-	struct nouveau_device *device;
 	struct apertures_struct *aper;
 	bool boot = false;
-	int ret;
+	struct pci_dev *pdev = dev->pdev;
 
 	/* remove conflicting drivers (vesafb, efifb etc) */
 	aper = alloc_apertures(3);
@@ -265,8 +263,18 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
 #ifdef CONFIG_X86
 	boot = pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
 #endif
-	remove_conflicting_framebuffers(aper, "nouveaufb", boot);
-	kfree(aper);
+
+	drm_kick_out_firmware(aper, boot);
+	dev->apertures = aper;
+	dev->apert_boot = boot;
+	return 0;
+}
+
+static int nouveau_drm_probe(struct pci_dev *pdev,
+			     const struct pci_device_id *pent)
+{
+	struct nouveau_device *device;
+	int ret;
 
 	ret = nouveau_device_create(pdev, nouveau_name(pdev), pci_name(pdev),
 				    nouveau_config, nouveau_debug, &device);
@@ -294,9 +302,13 @@ nouveau_drm_load(struct drm_device *dev, unsigned long flags)
 	struct nouveau_drm *drm;
 	int ret;
 
-	ret = nouveau_cli_create(pdev, "DRM", sizeof(*drm), (void**)&drm);
+	ret = nouveau_kick_out_firmware(dev);
 	if (ret)
 		return ret;
+
+	ret = nouveau_cli_create(pdev, "DRM", sizeof(*drm), (void**)&drm);
+	if (ret)
+		goto fail_apert;
 	lockdep_set_class(&drm->client.mutex, &drm_client_lock_class_key);
 
 	dev->dev_private = drm;
@@ -392,6 +404,9 @@ fail_ttm:
 	nouveau_vga_fini(drm);
 fail_device:
 	nouveau_cli_destroy(&drm->client);
+fail_apert:
+	kfree(dev->apertures);
+	dev->apertures = NULL;
 	return ret;
 }
 
-- 
1.8.3.1

Re: [RFC 0/6] SimpleDRM Driver (was: dvbe driver)

[Stephen Warren]

On 06/24/2013 04:27 PM, David Herrmann wrote:
> Hi
> 
> This is my second revision of the dvbe driver. I renamed it to SimpleDRM to
> show the resemblence with the recently introduced simplefb.c fbdev driver. The
> driver is supposed to be the most basic DRM driver similar to efifb.c, vesafb.c,
> offb.c, simplefb.c, ...
> It provides a single virtual CRTC+encoder+connector and allows user-space to
> create one dumb-buffer at a time and attach it.
> 
> The setup changed slightly. It no longer uses shadow buffers but instead maps
> the framebuffer directly into userspace. Furthermore, a new infrastructure is
> used to unload firmware drivers during real hardware drivers probe cycles. Only
> nouveau was changed to use it, yet.
> 
> I still have an odd problem when unloading DRM drivers (not just SimpleDRM) with
> an fbdev fallback. If I call printk() directly after unregister_framebufer(), I
> get a NULL-deref somewhere in the VT layer (most times hide_cursor()). I haven't
> figured out exactly where that happens, but I am also very reluctant to spend
> more time debugging the VT layer.

I tested this on a Tegra ARM system, and it basically worked.

I have one question: With the simplefb driver, and console=tty1 on the
kernel command-line, I see both the penguins logo and Linux's boot
messages on the LCD panel that's hooked up through simplefb. However,
with simpledrm, I only see the penguins logo, but no boot messages. Is
that expected? How would I solve that if so?

Note: I needed to apply the following patch to get it to compile:

diff --git a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
index 40a2696..39885c8 100644
--- a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
+++ b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
@@ -159,7 +159,7 @@ void sdrm_fbdev_cleanup(struct sdrm_device *sdrm)
 {
        struct fb_info *info;

-       if (!sdrm->info)
+       if (!sdrm->fbdev)
                return;

        dev_info(sdrm->ddev->dev, "fbdev cleanup\n");

Re: [RFC 0/6] SimpleDRM Driver (was: dvbe driver)

[David Herrmann]

Hi

On Wed, Jun 26, 2013 at 11:30 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 06/24/2013 04:27 PM, David Herrmann wrote:
>> Hi
>>
>> This is my second revision of the dvbe driver. I renamed it to SimpleDRM to
>> show the resemblence with the recently introduced simplefb.c fbdev driver. The
>> driver is supposed to be the most basic DRM driver similar to efifb.c, vesafb.c,
>> offb.c, simplefb.c, ...
>> It provides a single virtual CRTC+encoder+connector and allows user-space to
>> create one dumb-buffer at a time and attach it.
>>
>> The setup changed slightly. It no longer uses shadow buffers but instead maps
>> the framebuffer directly into userspace. Furthermore, a new infrastructure is
>> used to unload firmware drivers during real hardware drivers probe cycles. Only
>> nouveau was changed to use it, yet.
>>
>> I still have an odd problem when unloading DRM drivers (not just SimpleDRM) with
>> an fbdev fallback. If I call printk() directly after unregister_framebufer(), I
>> get a NULL-deref somewhere in the VT layer (most times hide_cursor()). I haven't
>> figured out exactly where that happens, but I am also very reluctant to spend
>> more time debugging the VT layer.
>
> I tested this on a Tegra ARM system, and it basically worked.

Thanks a lot for the feedback!

> I have one question: With the simplefb driver, and console=tty1 on the
> kernel command-line, I see both the penguins logo and Linux's boot
> messages on the LCD panel that's hooked up through simplefb. However,
> with simpledrm, I only see the penguins logo, but no boot messages. Is
> that expected? How would I solve that if so?

No idea what is going wrong there. Somehow the simpledrm-fbdev device
is not picked up as primary device. I only got a black-cursor on
black-background (visible if painting something else on the fb0
device). And I get NULL-derefs in cursor_hide() during unregistration.
I digged through fbcon.c to find out what's going wrong but without
any success. I will see what I can do.

However, X-server or other apps work perfectly fine with it.

> Note: I needed to apply the following patch to get it to compile:
>
> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
> b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
> index 40a2696..39885c8 100644
> --- a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
> +++ b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
> @@ -159,7 +159,7 @@ void sdrm_fbdev_cleanup(struct sdrm_device *sdrm)
>  {
>         struct fb_info *info;
>
> -       if (!sdrm->info)
> +       if (!sdrm->fbdev)
>                 return;

Ugh, embarrassing, sorry. I fixed it up. It was a late fix to a avoid
fbcon from panicking.

Thanks!
David

>
>         dev_info(sdrm->ddev->dev, "fbdev cleanup\n");
>
>
>