Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH RESEND 0/8] x86 platform framebuffers
From: David Herrmann @ 2013-08-02 12:05 UTC (permalink / raw)
  To: linux-kernel
  Cc: David Airlie, Geert Uytterhoeven, Stephen Warren, Peter Jones,
	Tomi Valkeinen, Jean-Christophe Plagniol-Villard, H. Peter Anvin,
	Ingo Molnar, Thomas Gleixner, x86, linux-fbdev, akpm,
	David Herrmann

Hi

I cut down my previous series to no longer include the SimpleDRM driver. If
anyone is interested, you can find it here:
  http://lwn.net/Articles/558104/
I will resend it once these preparation patches are in.

Changes since v2:
 - added common x86 formats (reported by hpa) (patch #5)

This whole series (including simpledrm) is tested by Stephen and me. I would be
glad if maintainers could ack/nack this so I can continue my work.

This series is pretty small and just converts x86 to use platform-devices
instead of global objects to pass framebuffer data to drivers. The commit
messages explain everything in detail.
The idea is to create a "platform-framebuffer" device which drivers can bind to.
If x86 boot code detectes efi or vesa framebuffers, it creates efi-framebuffer
or vesa-framebuffer devices instead.

Additionally, if the modes are compatible, "simple-framebuffer" devices are
created so simplefb can be used on x86. This feature is only enabled if
CONFIG_X86_SYSFB is selected (off by default) so users without simplefb still
get boot logs.

 @Stephen: I wasn't sure whether you tested the efi/vesa framebuffer changes,
too, so I didn't add your tested-by there. And I changed patch #5 so I dropped
it there, too. Thanks for testing!

Thanks
David

David Herrmann (8):
  fbdev: simplefb: add init through platform_data
  fbdev: simplefb: mark as fw and allocate apertures
  x86: provide platform-devices for boot-framebuffers
  x86: sysfb: move EFI quirks from efifb to sysfb
  fbdev: simplefb: add common x86 RGB formats
  fbdev: vesafb: bind to platform-framebuffer device
  fbdev: efifb: bind to efi-framebuffer
  fbdev: fbcon: select VT_HW_CONSOLE_BINDING

 arch/x86/Kconfig                       |  26 +++
 arch/x86/include/asm/sysfb.h           |  98 +++++++++++
 arch/x86/kernel/Makefile               |   3 +
 arch/x86/kernel/sysfb.c                |  74 ++++++++
 arch/x86/kernel/sysfb_efi.c            | 214 +++++++++++++++++++++++
 arch/x86/kernel/sysfb_simplefb.c       |  95 +++++++++++
 drivers/video/Kconfig                  |   5 +-
 drivers/video/console/Kconfig          |   3 +-
 drivers/video/efifb.c                  | 302 ++++-----------------------------
 drivers/video/simplefb.c               |  58 +++++--
 drivers/video/vesafb.c                 |  55 ++----
 include/linux/platform_data/simplefb.h |  63 +++++++
 12 files changed, 666 insertions(+), 330 deletions(-)
 create mode 100644 arch/x86/include/asm/sysfb.h
 create mode 100644 arch/x86/kernel/sysfb.c
 create mode 100644 arch/x86/kernel/sysfb_efi.c
 create mode 100644 arch/x86/kernel/sysfb_simplefb.c
 create mode 100644 include/linux/platform_data/simplefb.h

-- 
1.8.3.4


^ permalink raw reply

* [PATCH RESEND 1/8] fbdev: simplefb: add init through platform_data
From: David Herrmann @ 2013-08-02 12:05 UTC (permalink / raw)
  To: linux-kernel
  Cc: David Airlie, Geert Uytterhoeven, Stephen Warren, Peter Jones,
	Tomi Valkeinen, Jean-Christophe Plagniol-Villard, H. Peter Anvin,
	Ingo Molnar, Thomas Gleixner, x86, linux-fbdev, akpm,
	David Herrmann
In-Reply-To: <1375445127-15480-1-git-send-email-dh.herrmann@gmail.com>

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
parameters 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>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/video/Kconfig                  |  5 ++-
 drivers/video/simplefb.c               | 48 +++++++++++++++++++++--------
 include/linux/platform_data/simplefb.h | 56 ++++++++++++++++++++++++++++++++++
 3 files changed, 93 insertions(+), 16 deletions(-)
 create mode 100644 include/linux/platform_data/simplefb.h

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 4cf1e1d..34c3d96 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2457,7 +2457,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
@@ -2469,8 +2469,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..5886989 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,18 +74,7 @@ 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} },
-};
+static struct simplefb_format simplefb_formats[] = SIMPLEFB_FORMATS;
 
 struct simplefb_params {
 	u32 width;
@@ -139,6 +129,33 @@ 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;
+
+	params->format = NULL;
+	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 +166,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..5fa2c5e
--- /dev/null
+++ b/include/linux/platform_data/simplefb.h
@@ -0,0 +1,56 @@
+/*
+ * 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>
+
+/* format array, use it to initialize a "struct simplefb_format" array */
+#define SIMPLEFB_FORMATS \
+{ \
+	{ "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0}, DRM_FORMAT_RGB565 }, \
+}
+
+/*
+ * Data-Format for Simple-Framebuffers
+ * @name: unique 0-terminated name that can be used to identify the mode
+ * @red,green,blue: Offsets and sizes of the single RGB parts
+ * @transp: Offset and size of the alpha bits. length=0 means no alpha
+ * @fourcc: 32bit DRM four-CC code (see drm_fourcc.h)
+ */
+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;
+};
+
+/*
+ * Simple-Framebuffer description
+ * If the arch-boot code creates simple-framebuffers without DT support, it
+ * can pass the width, height, stride and format via this platform-data object.
+ * The framebuffer location must be given as IORESOURCE_MEM resource.
+ * @format must be a format as described in "struct simplefb_format" above.
+ */
+struct simplefb_platform_data {
+	u32 width;
+	u32 height;
+	u32 stride;
+	const char *format;
+};
+
+#endif /* __PLATFORM_DATA_SIMPLEFB_H__ */
-- 
1.8.3.4


^ permalink raw reply related

* [PATCH RESEND 2/8] fbdev: simplefb: mark as fw and allocate apertures
From: David Herrmann @ 2013-08-02 12:05 UTC (permalink / raw)
  To: linux-kernel
  Cc: David Airlie, Geert Uytterhoeven, Stephen Warren, Peter Jones,
	Tomi Valkeinen, Jean-Christophe Plagniol-Villard, H. Peter Anvin,
	Ingo Molnar, Thomas Gleixner, x86, linux-fbdev, akpm,
	David Herrmann
In-Reply-To: <1375445127-15480-1-git-send-email-dh.herrmann@gmail.com>

fbdev-core uses FBINFO_MISC_FIRMWARE to mark drivers that use firmware
framebuffers. Furthermore, we now allocate apertures for the fbinfo
device.
Both information is used by remove_conflicting_framebuffers() to remove a
fbdev device whenever a real driver is loaded. This is used heavily on x86
for VGA/vesa/EFI framebuffers, but is also of great use for all other
systems.

Especially with x86 support for simplefb, this information is needed to
unload simplefb before a real hw-driver (like i915, radeon, nouveau) is
loaded.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/video/simplefb.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
index 5886989..8d78106 100644
--- a/drivers/video/simplefb.c
+++ b/drivers/video/simplefb.c
@@ -202,8 +202,16 @@ static int simplefb_probe(struct platform_device *pdev)
 	info->var.blue = params.format->blue;
 	info->var.transp = params.format->transp;
 
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		framebuffer_release(info);
+		return -ENOMEM;
+	}
+	info->apertures->ranges[0].base = info->fix.smem_start;
+	info->apertures->ranges[0].size = info->fix.smem_len;
+
 	info->fbops = &simplefb_ops;
-	info->flags = FBINFO_DEFAULT;
+	info->flags = FBINFO_DEFAULT | FBINFO_MISC_FIRMWARE;
 	info->screen_base = devm_ioremap(&pdev->dev, info->fix.smem_start,
 					 info->fix.smem_len);
 	if (!info->screen_base) {
-- 
1.8.3.4


^ permalink raw reply related

* [PATCH RESEND 3/8] x86: provide platform-devices for boot-framebuffers
From: David Herrmann @ 2013-08-02 12:05 UTC (permalink / raw)
  To: linux-kernel
  Cc: David Airlie, Geert Uytterhoeven, Stephen Warren, Peter Jones,
	Tomi Valkeinen, Jean-Christophe Plagniol-Villard, H. Peter Anvin,
	Ingo Molnar, Thomas Gleixner, x86, linux-fbdev, akpm,
	David Herrmann
In-Reply-To: <1375445127-15480-1-git-send-email-dh.herrmann@gmail.com>

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 devices 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.

Different devices are created for VGA/VESA/EFI FBs to allow multiple
drivers to be loaded on distro kernels. We create:
 - "vesa-framebuffer" for VBE/VESA graphics FBs
 - "efi-framebuffer" for EFI FBs
 - "platform-framebuffer" for everything else
This allows to load vesafb, efifb and others simultaneously and each
picks up only the supported FB types.

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>
Tested-by: Stephen Warren <swarren@nvidia.com>
---
 arch/x86/Kconfig                 | 26 +++++++++++
 arch/x86/include/asm/sysfb.h     | 41 +++++++++++++++++
 arch/x86/kernel/Makefile         |  2 +
 arch/x86/kernel/sysfb.c          | 71 ++++++++++++++++++++++++++++++
 arch/x86/kernel/sysfb_simplefb.c | 95 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 235 insertions(+)
 create mode 100644 arch/x86/include/asm/sysfb.h
 create mode 100644 arch/x86/kernel/sysfb.c
 create mode 100644 arch/x86/kernel/sysfb_simplefb.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b32ebf9..5c56559 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2270,6 +2270,32 @@ 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.
+	  This option, if enabled, marks VGA/VBE/EFI framebuffers as generic
+	  framebuffers so the new generic system-framebuffer drivers can be
+	  used on x86. If the framebuffer is not compatible with the generic
+	  modes, it is adverticed as fallback platform framebuffer so legacy
+	  drivers like efifb, vesafb and uvesafb can pick it up.
+	  If this option is not selected, all system framebuffers are always
+	  marked as fallback platform framebuffers as usual.
+
+	  Note: Legacy fbdev drivers, including vesafb, efifb, uvesafb, will
+	  not be able to pick up generic system framebuffers if this option
+	  is selected. You are highly encouraged to enable simplefb as
+	  replacement if you select this option. simplefb can correctly deal
+	  with generic system framebuffers. But you should still keep vesafb
+	  and others enabled as fallback if a system framebuffer is
+	  incompatible with simplefb.
+
+	  If unsure, say Y.
+
 endmenu
 
 
diff --git a/arch/x86/include/asm/sysfb.h b/arch/x86/include/asm/sysfb.h
new file mode 100644
index 0000000..2395fe0
--- /dev/null
+++ b/arch/x86/include/asm/sysfb.h
@@ -0,0 +1,41 @@
+#ifndef _ARCH_X86_KERNEL_SYSFB_H
+#define _ARCH_X86_KERNEL_SYSFB_H
+
+/*
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/platform_data/simplefb.h>
+#include <linux/screen_info.h>
+
+#ifdef CONFIG_X86_SYSFB
+
+bool parse_mode(const struct screen_info *si,
+		struct simplefb_platform_data *mode);
+int create_simplefb(const struct screen_info *si,
+		    const struct simplefb_platform_data *mode);
+
+#else /* CONFIG_X86_SYSFB */
+
+static inline bool parse_mode(const struct screen_info *si,
+			      struct simplefb_platform_data *mode)
+{
+	return false;
+}
+
+static inline int create_simplefb(const struct screen_info *si,
+				  const struct simplefb_platform_data *mode)
+{
+	return -EINVAL;
+}
+
+#endif /* CONFIG_X86_SYSFB */
+
+#endif /* _ARCH_X86_KERNEL_SYSFB_H */
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 88d99ea..90ecdc5 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -103,6 +103,8 @@ 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_X86_SYSFB)			+= sysfb_simplefb.o
 
 obj-$(CONFIG_PERF_EVENTS)		+= perf_regs.o
 obj-$(CONFIG_TRACING)			+= tracepoint.o
diff --git a/arch/x86/kernel/sysfb.c b/arch/x86/kernel/sysfb.c
new file mode 100644
index 0000000..7f30e19
--- /dev/null
+++ b/arch/x86/kernel/sysfb.c
@@ -0,0 +1,71 @@
+/*
+ * 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 legacy
+ * "vesa-framebuffer", "efi-framebuffer" or "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.
+ * The global "screen_info" is still valid at all times.
+ *
+ * If CONFIG_X86_SYSFB is not selected, we never register "simple-framebuffer"
+ * platform devices, but only use legacy 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>
+#include <asm/sysfb.h>
+
+static __init int sysfb_init(void)
+{
+	struct screen_info *si = &screen_info;
+	struct simplefb_platform_data mode;
+	struct platform_device *pd;
+	const char *name;
+	bool compatible;
+	int ret;
+
+	/* try to create a simple-framebuffer device */
+	compatible = parse_mode(si, &mode);
+	if (compatible) {
+		ret = create_simplefb(si, &mode);
+		if (!ret)
+			return 0;
+	}
+
+	/* if the FB is incompatible, create a legacy framebuffer device */
+	if (si->orig_video_isVGA = VIDEO_TYPE_EFI)
+		name = "efi-framebuffer";
+	else if (si->orig_video_isVGA = VIDEO_TYPE_VLFB)
+		name = "vesa-framebuffer";
+	else
+		name = "platform-framebuffer";
+
+	pd = platform_device_register_resndata(NULL, name, 0,
+					       NULL, 0, si, sizeof(*si));
+	return IS_ERR(pd) ? PTR_ERR(pd) : 0;
+}
+
+device_initcall(sysfb_init);
diff --git a/arch/x86/kernel/sysfb_simplefb.c b/arch/x86/kernel/sysfb_simplefb.c
new file mode 100644
index 0000000..22513e9
--- /dev/null
+++ b/arch/x86/kernel/sysfb_simplefb.c
@@ -0,0 +1,95 @@
+/*
+ * 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 probing
+ * Try to convert "screen_info" into a "simple-framebuffer" compatible mode.
+ * If the mode is incompatible, we return "false" and let the caller create
+ * legacy nodes instead.
+ */
+
+#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>
+#include <asm/sysfb.h>
+
+static const char simplefb_resname[] = "BOOTFB";
+static const struct simplefb_format formats[] = SIMPLEFB_FORMATS;
+
+/* try parsing x86 screen_info into a simple-framebuffer mode struct */
+__init bool parse_mode(const struct screen_info *si,
+		       struct simplefb_platform_data *mode)
+{
+	const struct simplefb_format *f;
+	__u8 type;
+	unsigned int i;
+
+	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) {
+			mode->format = f->name;
+			mode->width = si->lfb_width;
+			mode->height = si->lfb_height;
+			mode->stride = si->lfb_linelength;
+			return true;
+		}
+	}
+
+	return false;
+}
+
+__init int create_simplefb(const struct screen_info *si,
+			   const struct simplefb_platform_data *mode)
+{
+	struct platform_device *pd;
+	struct resource res;
+	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(&res, 0, sizeof(res));
+	res.flags = IORESOURCE_MEM;
+	res.name = simplefb_resname;
+	res.start = si->lfb_base;
+	res.end = si->lfb_base + len - 1;
+	if (res.end <= res.start)
+		return -EINVAL;
+
+	pd = platform_device_register_resndata(NULL, "simple-framebuffer", 0,
+					       &res, 1, mode, sizeof(*mode));
+	if (IS_ERR(pd))
+		return PTR_ERR(pd);
+
+	return 0;
+}
-- 
1.8.3.4


^ permalink raw reply related

* [PATCH RESEND 4/8] x86: sysfb: move EFI quirks from efifb to sysfb
From: David Herrmann @ 2013-08-02 12:05 UTC (permalink / raw)
  To: linux-kernel
  Cc: David Airlie, Geert Uytterhoeven, Stephen Warren, Peter Jones,
	Tomi Valkeinen, Jean-Christophe Plagniol-Villard, H. Peter Anvin,
	Ingo Molnar, Thomas Gleixner, x86, linux-fbdev, akpm,
	David Herrmann
In-Reply-To: <1375445127-15480-1-git-send-email-dh.herrmann@gmail.com>

The EFI FB quirks from efifb.c are useful for simple-framebuffer devices
as well. Apply them by default so we can convert efifb.c to use
efi-framebuffer platform devices.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 arch/x86/include/asm/sysfb.h |  57 +++++++++++
 arch/x86/kernel/Makefile     |   1 +
 arch/x86/kernel/sysfb.c      |   3 +
 arch/x86/kernel/sysfb_efi.c  | 214 +++++++++++++++++++++++++++++++++++++++
 drivers/video/efifb.c        | 234 ++-----------------------------------------
 5 files changed, 282 insertions(+), 227 deletions(-)
 create mode 100644 arch/x86/kernel/sysfb_efi.c

diff --git a/arch/x86/include/asm/sysfb.h b/arch/x86/include/asm/sysfb.h
index 2395fe0..2aeb3e2 100644
--- a/arch/x86/include/asm/sysfb.h
+++ b/arch/x86/include/asm/sysfb.h
@@ -15,6 +15,63 @@
 #include <linux/platform_data/simplefb.h>
 #include <linux/screen_info.h>
 
+enum {
+	M_I17,		/* 17-Inch iMac */
+	M_I20,		/* 20-Inch iMac */
+	M_I20_SR,	/* 20-Inch iMac (Santa Rosa) */
+	M_I24,		/* 24-Inch iMac */
+	M_I24_8_1,	/* 24-Inch iMac, 8,1th gen */
+	M_I24_10_1,	/* 24-Inch iMac, 10,1th gen */
+	M_I27_11_1,	/* 27-Inch iMac, 11,1th gen */
+	M_MINI,		/* Mac Mini */
+	M_MINI_3_1,	/* Mac Mini, 3,1th gen */
+	M_MINI_4_1,	/* Mac Mini, 4,1th gen */
+	M_MB,		/* MacBook */
+	M_MB_2,		/* MacBook, 2nd rev. */
+	M_MB_3,		/* MacBook, 3rd rev. */
+	M_MB_5_1,	/* MacBook, 5th rev. */
+	M_MB_6_1,	/* MacBook, 6th rev. */
+	M_MB_7_1,	/* MacBook, 7th rev. */
+	M_MB_SR,	/* MacBook, 2nd gen, (Santa Rosa) */
+	M_MBA,		/* MacBook Air */
+	M_MBA_3,	/* Macbook Air, 3rd rev */
+	M_MBP,		/* MacBook Pro */
+	M_MBP_2,	/* MacBook Pro 2nd gen */
+	M_MBP_2_2,	/* MacBook Pro 2,2nd gen */
+	M_MBP_SR,	/* MacBook Pro (Santa Rosa) */
+	M_MBP_4,	/* MacBook Pro, 4th gen */
+	M_MBP_5_1,	/* MacBook Pro, 5,1th gen */
+	M_MBP_5_2,	/* MacBook Pro, 5,2th gen */
+	M_MBP_5_3,	/* MacBook Pro, 5,3rd gen */
+	M_MBP_6_1,	/* MacBook Pro, 6,1th gen */
+	M_MBP_6_2,	/* MacBook Pro, 6,2th gen */
+	M_MBP_7_1,	/* MacBook Pro, 7,1th gen */
+	M_MBP_8_2,	/* MacBook Pro, 8,2nd gen */
+	M_UNKNOWN	/* placeholder */
+};
+
+struct efifb_dmi_info {
+	char *optname;
+	unsigned long base;
+	int stride;
+	int width;
+	int height;
+	int flags;
+};
+
+#ifdef CONFIG_EFI
+
+extern struct efifb_dmi_info efifb_dmi_list[];
+void sysfb_apply_efi_quirks(void);
+
+#else /* CONFIG_EFI */
+
+static inline void sysfb_apply_efi_quirks(void)
+{
+}
+
+#endif /* CONFIG_EFI */
+
 #ifdef CONFIG_X86_SYSFB
 
 bool parse_mode(const struct screen_info *si,
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 90ecdc5..a5408b9 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -105,6 +105,7 @@ obj-$(CONFIG_OF)			+= devicetree.o
 obj-$(CONFIG_UPROBES)			+= uprobes.o
 obj-y					+= sysfb.o
 obj-$(CONFIG_X86_SYSFB)			+= sysfb_simplefb.o
+obj-$(CONFIG_EFI)			+= sysfb_efi.o
 
 obj-$(CONFIG_PERF_EVENTS)		+= perf_regs.o
 obj-$(CONFIG_TRACING)			+= tracepoint.o
diff --git a/arch/x86/kernel/sysfb.c b/arch/x86/kernel/sysfb.c
index 7f30e19..193ec2c 100644
--- a/arch/x86/kernel/sysfb.c
+++ b/arch/x86/kernel/sysfb.c
@@ -47,6 +47,8 @@ static __init int sysfb_init(void)
 	bool compatible;
 	int ret;
 
+	sysfb_apply_efi_quirks();
+
 	/* try to create a simple-framebuffer device */
 	compatible = parse_mode(si, &mode);
 	if (compatible) {
@@ -68,4 +70,5 @@ static __init int sysfb_init(void)
 	return IS_ERR(pd) ? PTR_ERR(pd) : 0;
 }
 
+/* must execute after PCI subsystem for EFI quirks */
 device_initcall(sysfb_init);
diff --git a/arch/x86/kernel/sysfb_efi.c b/arch/x86/kernel/sysfb_efi.c
new file mode 100644
index 0000000..b285d4e
--- /dev/null
+++ b/arch/x86/kernel/sysfb_efi.c
@@ -0,0 +1,214 @@
+/*
+ * Generic System Framebuffers on x86
+ * Copyright (c) 2012-2013 David Herrmann <dh.herrmann@gmail.com>
+ *
+ * EFI Quirks Copyright (c) 2006 Edgar Hucek <gimli@dark-green.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.
+ */
+
+/*
+ * EFI Quirks
+ * Several EFI systems do not correctly advertise their boot framebuffers.
+ * Hence, we use this static table of known broken machines and fix up the
+ * information so framebuffer drivers can load corectly.
+ */
+
+#include <linux/dmi.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/pci.h>
+#include <linux/screen_info.h>
+#include <video/vga.h>
+#include <asm/sysfb.h>
+
+enum {
+	OVERRIDE_NONE = 0x0,
+	OVERRIDE_BASE = 0x1,
+	OVERRIDE_STRIDE = 0x2,
+	OVERRIDE_HEIGHT = 0x4,
+	OVERRIDE_WIDTH = 0x8,
+};
+
+struct efifb_dmi_info efifb_dmi_list[] = {
+	[M_I17] = { "i17", 0x80010000, 1472 * 4, 1440, 900, OVERRIDE_NONE },
+	[M_I20] = { "i20", 0x80010000, 1728 * 4, 1680, 1050, OVERRIDE_NONE }, /* guess */
+	[M_I20_SR] = { "imac7", 0x40010000, 1728 * 4, 1680, 1050, OVERRIDE_NONE },
+	[M_I24] = { "i24", 0x80010000, 2048 * 4, 1920, 1200, OVERRIDE_NONE }, /* guess */
+	[M_I24_8_1] = { "imac8", 0xc0060000, 2048 * 4, 1920, 1200, OVERRIDE_NONE },
+	[M_I24_10_1] = { "imac10", 0xc0010000, 2048 * 4, 1920, 1080, OVERRIDE_NONE },
+	[M_I27_11_1] = { "imac11", 0xc0010000, 2560 * 4, 2560, 1440, OVERRIDE_NONE },
+	[M_MINI]= { "mini", 0x80000000, 2048 * 4, 1024, 768, OVERRIDE_NONE },
+	[M_MINI_3_1] = { "mini31", 0x40010000, 1024 * 4, 1024, 768, OVERRIDE_NONE },
+	[M_MINI_4_1] = { "mini41", 0xc0010000, 2048 * 4, 1920, 1200, OVERRIDE_NONE },
+	[M_MB] = { "macbook", 0x80000000, 2048 * 4, 1280, 800, OVERRIDE_NONE },
+	[M_MB_5_1] = { "macbook51", 0x80010000, 2048 * 4, 1280, 800, OVERRIDE_NONE },
+	[M_MB_6_1] = { "macbook61", 0x80010000, 2048 * 4, 1280, 800, OVERRIDE_NONE },
+	[M_MB_7_1] = { "macbook71", 0x80010000, 2048 * 4, 1280, 800, OVERRIDE_NONE },
+	[M_MBA] = { "mba", 0x80000000, 2048 * 4, 1280, 800, OVERRIDE_NONE },
+	/* 11" Macbook Air 3,1 passes the wrong stride */
+	[M_MBA_3] = { "mba3", 0, 2048 * 4, 0, 0, OVERRIDE_STRIDE },
+	[M_MBP] = { "mbp", 0x80010000, 1472 * 4, 1440, 900, OVERRIDE_NONE },
+	[M_MBP_2] = { "mbp2", 0, 0, 0, 0, OVERRIDE_NONE }, /* placeholder */
+	[M_MBP_2_2] = { "mbp22", 0x80010000, 1472 * 4, 1440, 900, OVERRIDE_NONE },
+	[M_MBP_SR] = { "mbp3", 0x80030000, 2048 * 4, 1440, 900, OVERRIDE_NONE },
+	[M_MBP_4] = { "mbp4", 0xc0060000, 2048 * 4, 1920, 1200, OVERRIDE_NONE },
+	[M_MBP_5_1] = { "mbp51", 0xc0010000, 2048 * 4, 1440, 900, OVERRIDE_NONE },
+	[M_MBP_5_2] = { "mbp52", 0xc0010000, 2048 * 4, 1920, 1200, OVERRIDE_NONE },
+	[M_MBP_5_3] = { "mbp53", 0xd0010000, 2048 * 4, 1440, 900, OVERRIDE_NONE },
+	[M_MBP_6_1] = { "mbp61", 0x90030000, 2048 * 4, 1920, 1200, OVERRIDE_NONE },
+	[M_MBP_6_2] = { "mbp62", 0x90030000, 2048 * 4, 1680, 1050, OVERRIDE_NONE },
+	[M_MBP_7_1] = { "mbp71", 0xc0010000, 2048 * 4, 1280, 800, OVERRIDE_NONE },
+	[M_MBP_8_2] = { "mbp82", 0x90010000, 1472 * 4, 1440, 900, OVERRIDE_NONE },
+	[M_UNKNOWN] = { NULL, 0, 0, 0, 0, OVERRIDE_NONE }
+};
+
+#define choose_value(dmivalue, fwvalue, field, flags) ({	\
+		typeof(fwvalue) _ret_ = fwvalue;		\
+		if ((flags) & (field))				\
+			_ret_ = dmivalue;			\
+		else if ((fwvalue) = 0)			\
+			_ret_ = dmivalue;			\
+		_ret_;						\
+	})
+
+static int __init efifb_set_system(const struct dmi_system_id *id)
+{
+	struct efifb_dmi_info *info = id->driver_data;
+
+	if (info->base = 0 && info->height = 0 && info->width = 0 &&
+	    info->stride = 0)
+		return 0;
+
+	/* Trust the bootloader over the DMI tables */
+	if (screen_info.lfb_base = 0) {
+#if defined(CONFIG_PCI)
+		struct pci_dev *dev = NULL;
+		int found_bar = 0;
+#endif
+		if (info->base) {
+			screen_info.lfb_base = choose_value(info->base,
+				screen_info.lfb_base, OVERRIDE_BASE,
+				info->flags);
+
+#if defined(CONFIG_PCI)
+			/* make sure that the address in the table is actually
+			 * on a VGA device's PCI BAR */
+
+			for_each_pci_dev(dev) {
+				int i;
+				if ((dev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
+					continue;
+				for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
+					resource_size_t start, end;
+
+					start = pci_resource_start(dev, i);
+					if (start = 0)
+						break;
+					end = pci_resource_end(dev, i);
+					if (screen_info.lfb_base >= start &&
+					    screen_info.lfb_base < end) {
+						found_bar = 1;
+					}
+				}
+			}
+			if (!found_bar)
+				screen_info.lfb_base = 0;
+#endif
+		}
+	}
+	if (screen_info.lfb_base) {
+		screen_info.lfb_linelength = choose_value(info->stride,
+			screen_info.lfb_linelength, OVERRIDE_STRIDE,
+			info->flags);
+		screen_info.lfb_width = choose_value(info->width,
+			screen_info.lfb_width, OVERRIDE_WIDTH,
+			info->flags);
+		screen_info.lfb_height = choose_value(info->height,
+			screen_info.lfb_height, OVERRIDE_HEIGHT,
+			info->flags);
+		if (screen_info.orig_video_isVGA = 0)
+			screen_info.orig_video_isVGA = VIDEO_TYPE_EFI;
+	} else {
+		screen_info.lfb_linelength = 0;
+		screen_info.lfb_width = 0;
+		screen_info.lfb_height = 0;
+		screen_info.orig_video_isVGA = 0;
+		return 0;
+	}
+
+	printk(KERN_INFO "efifb: dmi detected %s - framebuffer at 0x%08x "
+			 "(%dx%d, stride %d)\n", id->ident,
+			 screen_info.lfb_base, screen_info.lfb_width,
+			 screen_info.lfb_height, screen_info.lfb_linelength);
+
+	return 1;
+}
+
+#define EFIFB_DMI_SYSTEM_ID(vendor, name, enumid)		\
+	{							\
+		efifb_set_system,				\
+		name,						\
+		{						\
+			DMI_MATCH(DMI_BIOS_VENDOR, vendor),	\
+			DMI_MATCH(DMI_PRODUCT_NAME, name)	\
+		},						\
+		&efifb_dmi_list[enumid]				\
+	}
+
+static const struct dmi_system_id efifb_dmi_system_table[] __initconst = {
+	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "iMac4,1", M_I17),
+	/* At least one of these two will be right; maybe both? */
+	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "iMac5,1", M_I20),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "iMac5,1", M_I20),
+	/* At least one of these two will be right; maybe both? */
+	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "iMac6,1", M_I24),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "iMac6,1", M_I24),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "iMac7,1", M_I20_SR),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "iMac8,1", M_I24_8_1),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "iMac10,1", M_I24_10_1),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "iMac11,1", M_I27_11_1),
+	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "Macmini1,1", M_MINI),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "Macmini3,1", M_MINI_3_1),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "Macmini4,1", M_MINI_4_1),
+	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "MacBook1,1", M_MB),
+	/* At least one of these two will be right; maybe both? */
+	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "MacBook2,1", M_MB),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBook2,1", M_MB),
+	/* At least one of these two will be right; maybe both? */
+	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "MacBook3,1", M_MB),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBook3,1", M_MB),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBook4,1", M_MB),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBook5,1", M_MB_5_1),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBook6,1", M_MB_6_1),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBook7,1", M_MB_7_1),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookAir1,1", M_MBA),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookAir3,1", M_MBA_3),
+	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "MacBookPro1,1", M_MBP),
+	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "MacBookPro2,1", M_MBP_2),
+	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "MacBookPro2,2", M_MBP_2_2),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro2,1", M_MBP_2),
+	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "MacBookPro3,1", M_MBP_SR),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro3,1", M_MBP_SR),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro4,1", M_MBP_4),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro5,1", M_MBP_5_1),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro5,2", M_MBP_5_2),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro5,3", M_MBP_5_3),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro6,1", M_MBP_6_1),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro6,2", M_MBP_6_2),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro7,1", M_MBP_7_1),
+	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro8,2", M_MBP_8_2),
+	{},
+};
+
+__init void sysfb_apply_efi_quirks(void)
+{
+	if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI ||
+	    !(screen_info.capabilities & VIDEO_CAPABILITY_SKIP_QUIRKS))
+		dmi_check_system(efifb_dmi_system_table);
+}
diff --git a/drivers/video/efifb.c b/drivers/video/efifb.c
index 50fe668..e493bcb 100644
--- a/drivers/video/efifb.c
+++ b/drivers/video/efifb.c
@@ -15,6 +15,7 @@
 #include <linux/dmi.h>
 #include <linux/pci.h>
 #include <video/vga.h>
+#include <asm/sysfb.h>
 
 static bool request_mem_succeeded = false;
 
@@ -38,223 +39,6 @@ static struct fb_fix_screeninfo efifb_fix = {
 	.visual			= FB_VISUAL_TRUECOLOR,
 };
 
-enum {
-	M_I17,		/* 17-Inch iMac */
-	M_I20,		/* 20-Inch iMac */
-	M_I20_SR,	/* 20-Inch iMac (Santa Rosa) */
-	M_I24,		/* 24-Inch iMac */
-	M_I24_8_1,	/* 24-Inch iMac, 8,1th gen */
-	M_I24_10_1,	/* 24-Inch iMac, 10,1th gen */
-	M_I27_11_1,	/* 27-Inch iMac, 11,1th gen */
-	M_MINI,		/* Mac Mini */
-	M_MINI_3_1,	/* Mac Mini, 3,1th gen */
-	M_MINI_4_1,	/* Mac Mini, 4,1th gen */
-	M_MB,		/* MacBook */
-	M_MB_2,		/* MacBook, 2nd rev. */
-	M_MB_3,		/* MacBook, 3rd rev. */
-	M_MB_5_1,	/* MacBook, 5th rev. */
-	M_MB_6_1,	/* MacBook, 6th rev. */
-	M_MB_7_1,	/* MacBook, 7th rev. */
-	M_MB_SR,	/* MacBook, 2nd gen, (Santa Rosa) */
-	M_MBA,		/* MacBook Air */
-	M_MBA_3,	/* Macbook Air, 3rd rev */
-	M_MBP,		/* MacBook Pro */
-	M_MBP_2,	/* MacBook Pro 2nd gen */
-	M_MBP_2_2,	/* MacBook Pro 2,2nd gen */
-	M_MBP_SR,	/* MacBook Pro (Santa Rosa) */
-	M_MBP_4,	/* MacBook Pro, 4th gen */
-	M_MBP_5_1,    /* MacBook Pro, 5,1th gen */
-	M_MBP_5_2,	/* MacBook Pro, 5,2th gen */
-	M_MBP_5_3,	/* MacBook Pro, 5,3rd gen */
-	M_MBP_6_1,	/* MacBook Pro, 6,1th gen */
-	M_MBP_6_2,	/* MacBook Pro, 6,2th gen */
-	M_MBP_7_1,	/* MacBook Pro, 7,1th gen */
-	M_MBP_8_2,	/* MacBook Pro, 8,2nd gen */
-	M_UNKNOWN	/* placeholder */
-};
-
-#define OVERRIDE_NONE	0x0
-#define OVERRIDE_BASE	0x1
-#define OVERRIDE_STRIDE	0x2
-#define OVERRIDE_HEIGHT	0x4
-#define OVERRIDE_WIDTH	0x8
-
-static struct efifb_dmi_info {
-	char *optname;
-	unsigned long base;
-	int stride;
-	int width;
-	int height;
-	int flags;
-} dmi_list[] __initdata = {
-	[M_I17] = { "i17", 0x80010000, 1472 * 4, 1440, 900, OVERRIDE_NONE },
-	[M_I20] = { "i20", 0x80010000, 1728 * 4, 1680, 1050, OVERRIDE_NONE }, /* guess */
-	[M_I20_SR] = { "imac7", 0x40010000, 1728 * 4, 1680, 1050, OVERRIDE_NONE },
-	[M_I24] = { "i24", 0x80010000, 2048 * 4, 1920, 1200, OVERRIDE_NONE }, /* guess */
-	[M_I24_8_1] = { "imac8", 0xc0060000, 2048 * 4, 1920, 1200, OVERRIDE_NONE },
-	[M_I24_10_1] = { "imac10", 0xc0010000, 2048 * 4, 1920, 1080, OVERRIDE_NONE },
-	[M_I27_11_1] = { "imac11", 0xc0010000, 2560 * 4, 2560, 1440, OVERRIDE_NONE },
-	[M_MINI]= { "mini", 0x80000000, 2048 * 4, 1024, 768, OVERRIDE_NONE },
-	[M_MINI_3_1] = { "mini31", 0x40010000, 1024 * 4, 1024, 768, OVERRIDE_NONE },
-	[M_MINI_4_1] = { "mini41", 0xc0010000, 2048 * 4, 1920, 1200, OVERRIDE_NONE },
-	[M_MB] = { "macbook", 0x80000000, 2048 * 4, 1280, 800, OVERRIDE_NONE },
-	[M_MB_5_1] = { "macbook51", 0x80010000, 2048 * 4, 1280, 800, OVERRIDE_NONE },
-	[M_MB_6_1] = { "macbook61", 0x80010000, 2048 * 4, 1280, 800, OVERRIDE_NONE },
-	[M_MB_7_1] = { "macbook71", 0x80010000, 2048 * 4, 1280, 800, OVERRIDE_NONE },
-	[M_MBA] = { "mba", 0x80000000, 2048 * 4, 1280, 800, OVERRIDE_NONE },
-	/* 11" Macbook Air 3,1 passes the wrong stride */
-	[M_MBA_3] = { "mba3", 0, 2048 * 4, 0, 0, OVERRIDE_STRIDE },
-	[M_MBP] = { "mbp", 0x80010000, 1472 * 4, 1440, 900, OVERRIDE_NONE },
-	[M_MBP_2] = { "mbp2", 0, 0, 0, 0, OVERRIDE_NONE }, /* placeholder */
-	[M_MBP_2_2] = { "mbp22", 0x80010000, 1472 * 4, 1440, 900, OVERRIDE_NONE },
-	[M_MBP_SR] = { "mbp3", 0x80030000, 2048 * 4, 1440, 900, OVERRIDE_NONE },
-	[M_MBP_4] = { "mbp4", 0xc0060000, 2048 * 4, 1920, 1200, OVERRIDE_NONE },
-	[M_MBP_5_1] = { "mbp51", 0xc0010000, 2048 * 4, 1440, 900, OVERRIDE_NONE },
-	[M_MBP_5_2] = { "mbp52", 0xc0010000, 2048 * 4, 1920, 1200, OVERRIDE_NONE },
-	[M_MBP_5_3] = { "mbp53", 0xd0010000, 2048 * 4, 1440, 900, OVERRIDE_NONE },
-	[M_MBP_6_1] = { "mbp61", 0x90030000, 2048 * 4, 1920, 1200, OVERRIDE_NONE },
-	[M_MBP_6_2] = { "mbp62", 0x90030000, 2048 * 4, 1680, 1050, OVERRIDE_NONE },
-	[M_MBP_7_1] = { "mbp71", 0xc0010000, 2048 * 4, 1280, 800, OVERRIDE_NONE },
-	[M_MBP_8_2] = { "mbp82", 0x90010000, 1472 * 4, 1440, 900, OVERRIDE_NONE },
-	[M_UNKNOWN] = { NULL, 0, 0, 0, 0, OVERRIDE_NONE }
-};
-
-static int set_system(const struct dmi_system_id *id);
-
-#define EFIFB_DMI_SYSTEM_ID(vendor, name, enumid)		\
-	{ set_system, name, {					\
-		DMI_MATCH(DMI_BIOS_VENDOR, vendor),		\
-		DMI_MATCH(DMI_PRODUCT_NAME, name) },		\
-	  &dmi_list[enumid] }
-
-static const struct dmi_system_id dmi_system_table[] __initconst = {
-	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "iMac4,1", M_I17),
-	/* At least one of these two will be right; maybe both? */
-	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "iMac5,1", M_I20),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "iMac5,1", M_I20),
-	/* At least one of these two will be right; maybe both? */
-	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "iMac6,1", M_I24),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "iMac6,1", M_I24),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "iMac7,1", M_I20_SR),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "iMac8,1", M_I24_8_1),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "iMac10,1", M_I24_10_1),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "iMac11,1", M_I27_11_1),
-	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "Macmini1,1", M_MINI),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "Macmini3,1", M_MINI_3_1),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "Macmini4,1", M_MINI_4_1),
-	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "MacBook1,1", M_MB),
-	/* At least one of these two will be right; maybe both? */
-	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "MacBook2,1", M_MB),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBook2,1", M_MB),
-	/* At least one of these two will be right; maybe both? */
-	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "MacBook3,1", M_MB),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBook3,1", M_MB),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBook4,1", M_MB),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBook5,1", M_MB_5_1),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBook6,1", M_MB_6_1),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBook7,1", M_MB_7_1),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookAir1,1", M_MBA),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookAir3,1", M_MBA_3),
-	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "MacBookPro1,1", M_MBP),
-	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "MacBookPro2,1", M_MBP_2),
-	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "MacBookPro2,2", M_MBP_2_2),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro2,1", M_MBP_2),
-	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "MacBookPro3,1", M_MBP_SR),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro3,1", M_MBP_SR),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro4,1", M_MBP_4),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro5,1", M_MBP_5_1),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro5,2", M_MBP_5_2),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro5,3", M_MBP_5_3),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro6,1", M_MBP_6_1),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro6,2", M_MBP_6_2),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro7,1", M_MBP_7_1),
-	EFIFB_DMI_SYSTEM_ID("Apple Inc.", "MacBookPro8,2", M_MBP_8_2),
-	{},
-};
-
-#define choose_value(dmivalue, fwvalue, field, flags) ({	\
-		typeof(fwvalue) _ret_ = fwvalue;		\
-		if ((flags) & (field))				\
-			_ret_ = dmivalue;			\
-		else if ((fwvalue) = 0)			\
-			_ret_ = dmivalue;			\
-		_ret_;						\
-	})
-
-static int set_system(const struct dmi_system_id *id)
-{
-	struct efifb_dmi_info *info = id->driver_data;
-
-	if (info->base = 0 && info->height = 0 && info->width = 0
-			&& info->stride = 0)
-		return 0;
-
-	/* Trust the bootloader over the DMI tables */
-	if (screen_info.lfb_base = 0) {
-#if defined(CONFIG_PCI)
-		struct pci_dev *dev = NULL;
-		int found_bar = 0;
-#endif
-		if (info->base) {
-			screen_info.lfb_base = choose_value(info->base,
-				screen_info.lfb_base, OVERRIDE_BASE,
-				info->flags);
-
-#if defined(CONFIG_PCI)
-			/* make sure that the address in the table is actually
-			 * on a VGA device's PCI BAR */
-
-			for_each_pci_dev(dev) {
-				int i;
-				if ((dev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
-					continue;
-				for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
-					resource_size_t start, end;
-
-					start = pci_resource_start(dev, i);
-					if (start = 0)
-						break;
-					end = pci_resource_end(dev, i);
-					if (screen_info.lfb_base >= start &&
-					    screen_info.lfb_base < end) {
-						found_bar = 1;
-					}
-				}
-			}
-			if (!found_bar)
-				screen_info.lfb_base = 0;
-#endif
-		}
-	}
-	if (screen_info.lfb_base) {
-		screen_info.lfb_linelength = choose_value(info->stride,
-			screen_info.lfb_linelength, OVERRIDE_STRIDE,
-			info->flags);
-		screen_info.lfb_width = choose_value(info->width,
-			screen_info.lfb_width, OVERRIDE_WIDTH,
-			info->flags);
-		screen_info.lfb_height = choose_value(info->height,
-			screen_info.lfb_height, OVERRIDE_HEIGHT,
-			info->flags);
-		if (screen_info.orig_video_isVGA = 0)
-			screen_info.orig_video_isVGA = VIDEO_TYPE_EFI;
-	} else {
-		screen_info.lfb_linelength = 0;
-		screen_info.lfb_width = 0;
-		screen_info.lfb_height = 0;
-		screen_info.orig_video_isVGA = 0;
-		return 0;
-	}
-
-	printk(KERN_INFO "efifb: dmi detected %s - framebuffer at 0x%08x "
-			 "(%dx%d, stride %d)\n", id->ident,
-			 screen_info.lfb_base, screen_info.lfb_width,
-			 screen_info.lfb_height, screen_info.lfb_linelength);
-
-
-	return 1;
-}
-
 static int efifb_setcolreg(unsigned regno, unsigned red, unsigned green,
 			   unsigned blue, unsigned transp,
 			   struct fb_info *info)
@@ -323,12 +107,12 @@ static int __init efifb_setup(char *options)
 			if (!*this_opt) continue;
 
 			for (i = 0; i < M_UNKNOWN; i++) {
-				if (!strcmp(this_opt, dmi_list[i].optname) &&
-				    dmi_list[i].base != 0) {
-					screen_info.lfb_base = dmi_list[i].base;
-					screen_info.lfb_linelength = dmi_list[i].stride;
-					screen_info.lfb_width = dmi_list[i].width;
-					screen_info.lfb_height = dmi_list[i].height;
+				if (!strcmp(this_opt, efifb_dmi_list[i].optname) &&
+				    efifb_dmi_list[i].base != 0) {
+					screen_info.lfb_base = efifb_dmi_list[i].base;
+					screen_info.lfb_linelength = efifb_dmi_list[i].stride;
+					screen_info.lfb_width = efifb_dmi_list[i].width;
+					screen_info.lfb_height = efifb_dmi_list[i].height;
 				}
 			}
 			if (!strncmp(this_opt, "base:", 5))
@@ -553,10 +337,6 @@ static int __init efifb_init(void)
 	int ret;
 	char *option = NULL;
 
-	if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI ||
-	    !(screen_info.capabilities & VIDEO_CAPABILITY_SKIP_QUIRKS))
-		dmi_check_system(dmi_system_table);
-
 	if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI)
 		return -ENODEV;
 
-- 
1.8.3.4


^ permalink raw reply related

* [PATCH RESEND 5/8] fbdev: simplefb: add common x86 RGB formats
From: David Herrmann @ 2013-08-02 12:05 UTC (permalink / raw)
  To: linux-kernel
  Cc: David Airlie, Geert Uytterhoeven, Stephen Warren, Peter Jones,
	Tomi Valkeinen, Jean-Christophe Plagniol-Villard, H. Peter Anvin,
	Ingo Molnar, Thomas Gleixner, x86, linux-fbdev, akpm,
	David Herrmann
In-Reply-To: <1375445127-15480-1-git-send-email-dh.herrmann@gmail.com>

32bit XRGB and ARGB are used by modern x86 systems for EFI and VESA
framebuffers. The other formats were reported by hpa to be most common.
Add these so simplefb works on most common x86 systems.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 include/linux/platform_data/simplefb.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/platform_data/simplefb.h b/include/linux/platform_data/simplefb.h
index 5fa2c5e..53774b0 100644
--- a/include/linux/platform_data/simplefb.h
+++ b/include/linux/platform_data/simplefb.h
@@ -20,6 +20,13 @@
 #define SIMPLEFB_FORMATS \
 { \
 	{ "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0}, DRM_FORMAT_RGB565 }, \
+	{ "x1r5g5b5", 16, {10, 5}, {5, 5}, {0, 5}, {0, 0}, DRM_FORMAT_XRGB1555 }, \
+	{ "a1r5g5b5", 16, {10, 5}, {5, 5}, {0, 5}, {15, 1}, DRM_FORMAT_ARGB1555 }, \
+	{ "r8g8b8", 24, {16, 8}, {8, 8}, {0, 8}, {0, 0}, DRM_FORMAT_RGB888 }, \
+	{ "x8r8g8b8", 32, {16, 8}, {8, 8}, {0, 8}, {0, 0}, DRM_FORMAT_XRGB8888 }, \
+	{ "a8r8g8b8", 32, {16, 8}, {8, 8}, {0, 8}, {24, 8}, DRM_FORMAT_ARGB8888 }, \
+	{ "x2r10g10b10", 32, {20, 10}, {10, 10}, {0, 10}, {0, 0}, DRM_FORMAT_XRGB2101010 }, \
+	{ "a2r10g10b10", 32, {20, 10}, {10, 10}, {0, 10}, {30, 2}, DRM_FORMAT_ARGB2101010 }, \
 }
 
 /*
-- 
1.8.3.4


^ permalink raw reply related

* [PATCH RESEND 6/8] fbdev: vesafb: bind to platform-framebuffer device
From: David Herrmann @ 2013-08-02 12:05 UTC (permalink / raw)
  To: linux-kernel
  Cc: David Airlie, Geert Uytterhoeven, Stephen Warren, Peter Jones,
	Tomi Valkeinen, Jean-Christophe Plagniol-Villard, H. Peter Anvin,
	Ingo Molnar, Thomas Gleixner, x86, linux-fbdev, akpm,
	David Herrmann
In-Reply-To: <1375445127-15480-1-git-send-email-dh.herrmann@gmail.com>

x86 creates platform-framebuffer platform devices for every system
framebuffer. Use these instead of creating a dummy device.

This requires us to remove the __init annotations as hotplugging may occur
during runtime.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/video/vesafb.c | 55 +++++++++++++++-----------------------------------
 1 file changed, 16 insertions(+), 39 deletions(-)

diff --git a/drivers/video/vesafb.c b/drivers/video/vesafb.c
index 501b340..bd83233 100644
--- a/drivers/video/vesafb.c
+++ b/drivers/video/vesafb.c
@@ -29,7 +29,7 @@
 
 /* --------------------------------------------------------------------- */
 
-static struct fb_var_screeninfo vesafb_defined __initdata = {
+static struct fb_var_screeninfo vesafb_defined = {
 	.activate	= FB_ACTIVATE_NOW,
 	.height		= -1,
 	.width		= -1,
@@ -40,7 +40,7 @@ static struct fb_var_screeninfo vesafb_defined __initdata = {
 	.vmode		= FB_VMODE_NONINTERLACED,
 };
 
-static struct fb_fix_screeninfo vesafb_fix __initdata = {
+static struct fb_fix_screeninfo vesafb_fix = {
 	.id	= "VESA VGA",
 	.type	= FB_TYPE_PACKED_PIXELS,
 	.accel	= FB_ACCEL_NONE,
@@ -48,8 +48,8 @@ static struct fb_fix_screeninfo vesafb_fix __initdata = {
 
 static int   inverse    __read_mostly;
 static int   mtrr       __read_mostly;		/* disable mtrr */
-static int   vram_remap __initdata;		/* Set amount of memory to be used */
-static int   vram_total __initdata;		/* Set total amount of memory */
+static int   vram_remap;			/* Set amount of memory to be used */
+static int   vram_total;			/* Set total amount of memory */
 static int   pmi_setpal __read_mostly = 1;	/* pmi for palette changes ??? */
 static int   ypan       __read_mostly;		/* 0..nothing, 1..ypan, 2..ywrap */
 static void  (*pmi_start)(void) __read_mostly;
@@ -192,7 +192,7 @@ static struct fb_ops vesafb_ops = {
 	.fb_imageblit	= cfb_imageblit,
 };
 
-static int __init vesafb_setup(char *options)
+static int vesafb_setup(char *options)
 {
 	char *this_opt;
 	
@@ -226,13 +226,18 @@ static int __init vesafb_setup(char *options)
 	return 0;
 }
 
-static int __init vesafb_probe(struct platform_device *dev)
+static int vesafb_probe(struct platform_device *dev)
 {
 	struct fb_info *info;
 	int i, err;
 	unsigned int size_vmode;
 	unsigned int size_remap;
 	unsigned int size_total;
+	char *option = NULL;
+
+	/* ignore error return of fb_get_options */
+	fb_get_options("vesafb", &option);
+	vesafb_setup(option);
 
 	if (screen_info.orig_video_isVGA != VIDEO_TYPE_VLFB)
 		return -ENODEV;
@@ -496,40 +501,12 @@ err:
 }
 
 static struct platform_driver vesafb_driver = {
-	.driver	= {
-		.name	= "vesafb",
+	.driver = {
+		.name = "vesa-framebuffer",
+		.owner = THIS_MODULE,
 	},
+	.probe = vesafb_probe,
 };
 
-static struct platform_device *vesafb_device;
-
-static int __init vesafb_init(void)
-{
-	int ret;
-	char *option = NULL;
-
-	/* ignore error return of fb_get_options */
-	fb_get_options("vesafb", &option);
-	vesafb_setup(option);
-
-	vesafb_device = platform_device_alloc("vesafb", 0);
-	if (!vesafb_device)
-		return -ENOMEM;
-
-	ret = platform_device_add(vesafb_device);
-	if (!ret) {
-		ret = platform_driver_probe(&vesafb_driver, vesafb_probe);
-		if (ret)
-			platform_device_del(vesafb_device);
-	}
-
-	if (ret) {
-		platform_device_put(vesafb_device);
-		vesafb_device = NULL;
-	}
-
-	return ret;
-}
-module_init(vesafb_init);
-
+module_platform_driver(vesafb_driver);
 MODULE_LICENSE("GPL");
-- 
1.8.3.4


^ permalink raw reply related

* [PATCH RESEND 7/8] fbdev: efifb: bind to efi-framebuffer
From: David Herrmann @ 2013-08-02 12:05 UTC (permalink / raw)
  To: linux-kernel
  Cc: David Airlie, Geert Uytterhoeven, Stephen Warren, Peter Jones,
	Tomi Valkeinen, Jean-Christophe Plagniol-Villard, H. Peter Anvin,
	Ingo Molnar, Thomas Gleixner, x86, linux-fbdev, akpm,
	David Herrmann
In-Reply-To: <1375445127-15480-1-git-send-email-dh.herrmann@gmail.com>

Instead of creating a dummy device, we now bind to the efi-fb device
which is provided by x86 initialization code.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/video/efifb.c | 68 +++++++++++++++++----------------------------------
 1 file changed, 22 insertions(+), 46 deletions(-)

diff --git a/drivers/video/efifb.c b/drivers/video/efifb.c
index e493bcb..2a8286e 100644
--- a/drivers/video/efifb.c
+++ b/drivers/video/efifb.c
@@ -96,7 +96,7 @@ void vga_set_default_device(struct pci_dev *pdev)
 	default_vga = pdev;
 }
 
-static int __init efifb_setup(char *options)
+static int efifb_setup(char *options)
 {
 	char *this_opt;
 	int i;
@@ -153,13 +153,28 @@ static int __init efifb_setup(char *options)
 	return 0;
 }
 
-static int __init efifb_probe(struct platform_device *dev)
+static int efifb_probe(struct platform_device *dev)
 {
 	struct fb_info *info;
 	int err;
 	unsigned int size_vmode;
 	unsigned int size_remap;
 	unsigned int size_total;
+	char *option = NULL;
+
+	if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI)
+		return -ENODEV;
+
+	if (fb_get_options("efifb", &option))
+		return -ENODEV;
+	efifb_setup(option);
+
+	/* We don't get linelength from UGA Draw Protocol, only from
+	 * EFI Graphics Protocol.  So if it's not in DMI, and it's not
+	 * passed in from the user, we really can't use the framebuffer.
+	 */
+	if (!screen_info.lfb_linelength)
+		return -ENODEV;
 
 	if (!screen_info.lfb_depth)
 		screen_info.lfb_depth = 32;
@@ -323,51 +338,12 @@ err_release_mem:
 }
 
 static struct platform_driver efifb_driver = {
-	.driver	= {
-		.name	= "efifb",
+	.driver = {
+		.name = "efi-framebuffer",
+		.owner = THIS_MODULE,
 	},
+	.probe = efifb_probe,
 };
 
-static struct platform_device efifb_device = {
-	.name	= "efifb",
-};
-
-static int __init efifb_init(void)
-{
-	int ret;
-	char *option = NULL;
-
-	if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI)
-		return -ENODEV;
-
-	if (fb_get_options("efifb", &option))
-		return -ENODEV;
-	efifb_setup(option);
-
-	/* We don't get linelength from UGA Draw Protocol, only from
-	 * EFI Graphics Protocol.  So if it's not in DMI, and it's not
-	 * passed in from the user, we really can't use the framebuffer.
-	 */
-	if (!screen_info.lfb_linelength)
-		return -ENODEV;
-
-	ret = platform_device_register(&efifb_device);
-	if (ret)
-		return ret;
-
-	/*
-	 * This is not just an optimization.  We will interfere
-	 * with a real driver if we get reprobed, so don't allow
-	 * it.
-	 */
-	ret = platform_driver_probe(&efifb_driver, efifb_probe);
-	if (ret) {
-		platform_device_unregister(&efifb_device);
-		return ret;
-	}
-
-	return ret;
-}
-module_init(efifb_init);
-
+module_platform_driver(efifb_driver);
 MODULE_LICENSE("GPL");
-- 
1.8.3.4


^ permalink raw reply related

* [PATCH RESEND 8/8] fbdev: fbcon: select VT_HW_CONSOLE_BINDING
From: David Herrmann @ 2013-08-02 12:05 UTC (permalink / raw)
  To: linux-kernel
  Cc: David Airlie, Geert Uytterhoeven, Stephen Warren, Peter Jones,
	Tomi Valkeinen, Jean-Christophe Plagniol-Villard, H. Peter Anvin,
	Ingo Molnar, Thomas Gleixner, x86, linux-fbdev, akpm,
	David Herrmann
In-Reply-To: <1375445127-15480-1-git-send-email-dh.herrmann@gmail.com>

fbdev provides framebuffer hotplugging, hence, we need to allow fbcon to
unbind from framebuffers. Unfortunately, fbcon_fb_unbind() cannot unbind
from the last framebuffer, unless console-unbinding is supported.

Fixing fbcon_unbind() to return 0 caused some horrible NULL-derefs in the
VT layer and I couldn't figure out why. Hence, lets just require
console-unbinding so fbdev hotplugging works with fbcon.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/video/console/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index 8c30603..846caab 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -92,7 +92,8 @@ config DUMMY_CONSOLE_ROWS
 
 config FRAMEBUFFER_CONSOLE
 	tristate "Framebuffer Console support"
-	depends on FB
+	depends on FB && !UML
+	select VT_HW_CONSOLE_BINDING
 	select CRC32
 	select FONT_SUPPORT
 	help
-- 
1.8.3.4


^ permalink raw reply related

* Re: [PATCH RESEND 0/8] x86 platform framebuffers
From: Stephen Warren @ 2013-08-02 20:46 UTC (permalink / raw)
  To: David Herrmann
  Cc: linux-kernel, David Airlie, Geert Uytterhoeven, Peter Jones,
	Tomi Valkeinen, Jean-Christophe Plagniol-Villard, H. Peter Anvin,
	Ingo Molnar, Thomas Gleixner, x86, linux-fbdev, akpm
In-Reply-To: <1375445127-15480-1-git-send-email-dh.herrmann@gmail.com>

On 08/02/2013 06:05 AM, David Herrmann wrote:
> Hi
> 
> I cut down my previous series to no longer include the SimpleDRM driver. If
> anyone is interested, you can find it here:
>   http://lwn.net/Articles/558104/
> I will resend it once these preparation patches are in.
> 
> Changes since v2:
>  - added common x86 formats (reported by hpa) (patch #5)

So I assume this is V3; patch subject doesn't say so:-)

...
> @Stephen: I wasn't sure whether you tested the efi/vesa framebuffer changes,
> too, so I didn't add your tested-by there. And I changed patch #5 so I dropped
> it there, too. Thanks for testing!

Yes, I'm only testing on ARM platforms using simplefb, not any x86
platforms. This latest version seems to work fine, so feel free to
re-apply by tested-by where appropriate.

^ permalink raw reply

* Re: [PATCH v2 00/24] video/da8xx-fb fbdev driver enhance to support TI am335x SoC
From: Darren Etheridge @ 2013-08-02 22:14 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1375208791-15781-1-git-send-email-detheridge@ti.com>

Darren Etheridge <detheridge@ti.com> wrote on Thu [2013-Aug-01 09:06:50 -0500]:
> Tomi Valkeinen <tomi.valkeinen@ti.com> wrote on Wed [2013-Jul-31 13:04:33 +0300]:
> > Hi,
> > 
> > On 30/07/13 21:26, Darren Etheridge wrote:
> > > Changes in v2:
> > > Addressing review comments from Tomi Valkeinen:
> > > 	Dropped readl/writel patch
> > > 	Many cosmetic changes to make code easier to understand
> > > 
> > > 
> > > This is primarily a resend of a series of patches that were original
> > > submitted to linux-fbdev back in January of 2013 for 3.8 by Afzal
> > > Mohammed. I have rebased them on 3.10 and also made sure they
> > > apply cleanly to the 'for-next' branch of linux-fbdev git.
> > > The patches enable use of the current mainline da8xx-fb driver on the
> > > TI AM335x SOC along with some bug fixes and cleanup.
> > > 
> > > The original patch series can be found here:
> > > https://patchwork.kernel.org/project/linux-fbdev/list/?submitter9101
> > > if you want to see the history.
> > 
> > Comments on the whole series:
> > 
> > Most of the patches are originally from Afzal. I believe some of the
> > patches are unchanged, but some are changed by you. In cases like this
> > you should pick one of the following options for each patch:
> > 
> > - If the patch is unchanged, send the patch as it is, having From: Afzal
> > line there.
> > 
> > - If you have changed the patch, send the patch having From: Afzal line,
> > but marking in the description that you've changed it (and what you
> > did). This should be done if the changes are small.
> > 
> > - If you changed a lot in the patch, send the patch with yourself as the
> > author, signed off by only you, but mention that it's based on Afzal's work.
> > The point here is that if you change the patch, it's no longer Afzal's
> > original patch. Afzal hasn't reviewed it, so signed-off-by Afzal is not
> > correct. You could've introduced horrible bugs in the patch, and I'm
> > sure Afzal doesn't want to see that a patch in the kernel introducing
> > horrible bugs is from him (when it is not from him).
> Understood, and I have made the changes accordingly in the updated
> series I am preparing. 
> 
Tomi,

I will be sending the updated series on Monday.  I have also now
integrated Prabhakar Lad's devm_ changes, just doing testing now.

Thanks,
Darren

^ permalink raw reply

* Re: [PATCH RESEND 0/8] x86 platform framebuffers
From: H. Peter Anvin @ 2013-08-02 23:39 UTC (permalink / raw)
  To: David Herrmann
  Cc: linux-kernel, David Airlie, Geert Uytterhoeven, Stephen Warren,
	Peter Jones, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Ingo Molnar, Thomas Gleixner, x86, linux-fbdev, akpm
In-Reply-To: <1375445127-15480-1-git-send-email-dh.herrmann@gmail.com>

On 08/02/2013 05:05 AM, David Herrmann wrote:
> Hi
> 
> I cut down my previous series to no longer include the SimpleDRM driver. If
> anyone is interested, you can find it here:
>   http://lwn.net/Articles/558104/
> I will resend it once these preparation patches are in.
> 
> Changes since v2:
>  - added common x86 formats (reported by hpa) (patch #5)
> 
> This whole series (including simpledrm) is tested by Stephen and me. I would be
> glad if maintainers could ack/nack this so I can continue my work.
> 
> This series is pretty small and just converts x86 to use platform-devices
> instead of global objects to pass framebuffer data to drivers. The commit
> messages explain everything in detail.
> The idea is to create a "platform-framebuffer" device which drivers can bind to.
> If x86 boot code detectes efi or vesa framebuffers, it creates efi-framebuffer
> or vesa-framebuffer devices instead.
> 
> Additionally, if the modes are compatible, "simple-framebuffer" devices are
> created so simplefb can be used on x86. This feature is only enabled if
> CONFIG_X86_SYSFB is selected (off by default) so users without simplefb still
> get boot logs.
> 
>  @Stephen: I wasn't sure whether you tested the efi/vesa framebuffer changes,
> too, so I didn't add your tested-by there. And I changed patch #5 so I dropped
> it there, too. Thanks for testing!
> 

I am getting a bunch of new warnings with this patchset, typically of
the form:


/home/hpa/kernel/distwork/drivers/video/arkfb.c:1019:23: warning: cast
to pointer from integer of different size [-Wint-to-pointer-cast]
  par->state.vgabase = (void __iomem *) vga_res.start;
                       ^
/home/hpa/kernel/distwork/drivers/video/s3fb.c: In function ‘s3_pci_probe’:
/home/hpa/kernel/distwork/drivers/video/s3fb.c:1186:23: warning: cast to
pointer from integer of different size [-Wint-to-pointer-cast]
  par->state.vgabase = (void __iomem *) vga_res.start;
                       ^
I have pushed it out to a topic branch in the tip tree, partly to give
Fengguang's build bot a run at it (it is excellent at spotting the
origin of new warnings), but this needs to be fixed; we will not merge
this branch in its current form.

	-hpa


^ permalink raw reply

* Re: [PATCH RESEND 0/8] x86 platform framebuffers
From: David Herrmann @ 2013-08-03 15:46 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: linux-kernel, David Airlie, Geert Uytterhoeven, Stephen Warren,
	Peter Jones, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Ingo Molnar, Thomas Gleixner, x86, linux-fbdev, Andrew Morton
In-Reply-To: <51FC431F.1080405@zytor.com>

Hi

On Sat, Aug 3, 2013 at 1:39 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 08/02/2013 05:05 AM, David Herrmann wrote:
>> Hi
>>
>> I cut down my previous series to no longer include the SimpleDRM driver. If
>> anyone is interested, you can find it here:
>>   http://lwn.net/Articles/558104/
>> I will resend it once these preparation patches are in.
>>
>> Changes since v2:
>>  - added common x86 formats (reported by hpa) (patch #5)
>>
>> This whole series (including simpledrm) is tested by Stephen and me. I would be
>> glad if maintainers could ack/nack this so I can continue my work.
>>
>> This series is pretty small and just converts x86 to use platform-devices
>> instead of global objects to pass framebuffer data to drivers. The commit
>> messages explain everything in detail.
>> The idea is to create a "platform-framebuffer" device which drivers can bind to.
>> If x86 boot code detectes efi or vesa framebuffers, it creates efi-framebuffer
>> or vesa-framebuffer devices instead.
>>
>> Additionally, if the modes are compatible, "simple-framebuffer" devices are
>> created so simplefb can be used on x86. This feature is only enabled if
>> CONFIG_X86_SYSFB is selected (off by default) so users without simplefb still
>> get boot logs.
>>
>>  @Stephen: I wasn't sure whether you tested the efi/vesa framebuffer changes,
>> too, so I didn't add your tested-by there. And I changed patch #5 so I dropped
>> it there, too. Thanks for testing!
>>
>
> I am getting a bunch of new warnings with this patchset, typically of
> the form:
>
>
> /home/hpa/kernel/distwork/drivers/video/arkfb.c:1019:23: warning: cast
> to pointer from integer of different size [-Wint-to-pointer-cast]
>   par->state.vgabase = (void __iomem *) vga_res.start;
>                        ^
> /home/hpa/kernel/distwork/drivers/video/s3fb.c: In function ‘s3_pci_probe’:
> /home/hpa/kernel/distwork/drivers/video/s3fb.c:1186:23: warning: cast to
> pointer from integer of different size [-Wint-to-pointer-cast]
>   par->state.vgabase = (void __iomem *) vga_res.start;
>                        ^
> I have pushed it out to a topic branch in the tip tree, partly to give
> Fengguang's build bot a run at it (it is excellent at spotting the
> origin of new warnings), but this needs to be fixed; we will not merge
> this branch in its current form.

Thanks for looking at it. However, these warnings seem unrelated to my
patchset. I didn't change any vga/fb headers nor did I touch
arkfb/s3fb. Furthermore, I cannot reproduce these warnings. "vga_res"
is a "struct resource", "->start" is of type "resource_size_t" which
is typedef'ed to "phys_addr_t". Seems like the build-bot used some
random config with CONFIG_PHYS_ADDR_T_64BIT set but building on 32bit
(or something like that).

Is there any place where I can see the compiler log? Or the used .config file?

Thanks
David

^ permalink raw reply

* Re: [PATCH RESEND 0/8] x86 platform framebuffers
From: David Herrmann @ 2013-08-03 15:50 UTC (permalink / raw)
  To: Stephen Warren
  Cc: linux-kernel, David Airlie, Geert Uytterhoeven, Peter Jones,
	Tomi Valkeinen, Jean-Christophe Plagniol-Villard, H. Peter Anvin,
	Ingo Molnar, Thomas Gleixner, x86, linux-fbdev, Andrew Morton
In-Reply-To: <51FC1A91.3080401@wwwdotorg.org>

Hi

On Fri, Aug 2, 2013 at 10:46 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 08/02/2013 06:05 AM, David Herrmann wrote:
>> Hi
>>
>> I cut down my previous series to no longer include the SimpleDRM driver. If
>> anyone is interested, you can find it here:
>>   http://lwn.net/Articles/558104/
>> I will resend it once these preparation patches are in.
>>
>> Changes since v2:
>>  - added common x86 formats (reported by hpa) (patch #5)
>
> So I assume this is V3; patch subject doesn't say so:-)

Whoops, my bad, sorry. It's v3, yes.

> ...
>> @Stephen: I wasn't sure whether you tested the efi/vesa framebuffer changes,
>> too, so I didn't add your tested-by there. And I changed patch #5 so I dropped
>> it there, too. Thanks for testing!
>
> Yes, I'm only testing on ARM platforms using simplefb, not any x86
> platforms. This latest version seems to work fine, so feel free to
> re-apply by tested-by where appropriate.

That's what I thought. Thanks for verifying.

Cheers
David

^ permalink raw reply

* Re: [PATCH RESEND 0/8] x86 platform framebuffers
From: H. Peter Anvin @ 2013-08-03 15:53 UTC (permalink / raw)
  To: David Herrmann
  Cc: linux-kernel, David Airlie, Geert Uytterhoeven, Stephen Warren,
	Peter Jones, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Ingo Molnar, Thomas Gleixner, x86, linux-fbdev, Andrew Morton
In-Reply-To: <CANq1E4SjN-=3oyRp2zXaQ+aaJB8yEVmJzcPvbxQxu1MTc8JjiQ@mail.gmail.com>

When compiled for i386 PAE phys_addr_t is 64 bits but pointers are 32 bits.

David Herrmann <dh.herrmann@gmail.com> wrote:
>Hi
>
>On Sat, Aug 3, 2013 at 1:39 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>> On 08/02/2013 05:05 AM, David Herrmann wrote:
>>> Hi
>>>
>>> I cut down my previous series to no longer include the SimpleDRM
>driver. If
>>> anyone is interested, you can find it here:
>>>   http://lwn.net/Articles/558104/
>>> I will resend it once these preparation patches are in.
>>>
>>> Changes since v2:
>>>  - added common x86 formats (reported by hpa) (patch #5)
>>>
>>> This whole series (including simpledrm) is tested by Stephen and me.
>I would be
>>> glad if maintainers could ack/nack this so I can continue my work.
>>>
>>> This series is pretty small and just converts x86 to use
>platform-devices
>>> instead of global objects to pass framebuffer data to drivers. The
>commit
>>> messages explain everything in detail.
>>> The idea is to create a "platform-framebuffer" device which drivers
>can bind to.
>>> If x86 boot code detectes efi or vesa framebuffers, it creates
>efi-framebuffer
>>> or vesa-framebuffer devices instead.
>>>
>>> Additionally, if the modes are compatible, "simple-framebuffer"
>devices are
>>> created so simplefb can be used on x86. This feature is only enabled
>if
>>> CONFIG_X86_SYSFB is selected (off by default) so users without
>simplefb still
>>> get boot logs.
>>>
>>>  @Stephen: I wasn't sure whether you tested the efi/vesa framebuffer
>changes,
>>> too, so I didn't add your tested-by there. And I changed patch #5 so
>I dropped
>>> it there, too. Thanks for testing!
>>>
>>
>> I am getting a bunch of new warnings with this patchset, typically of
>> the form:
>>
>>
>> /home/hpa/kernel/distwork/drivers/video/arkfb.c:1019:23: warning:
>cast
>> to pointer from integer of different size [-Wint-to-pointer-cast]
>>   par->state.vgabase = (void __iomem *) vga_res.start;
>>                        ^
>> /home/hpa/kernel/distwork/drivers/video/s3fb.c: In function
>‘s3_pci_probe’:
>> /home/hpa/kernel/distwork/drivers/video/s3fb.c:1186:23: warning: cast
>to
>> pointer from integer of different size [-Wint-to-pointer-cast]
>>   par->state.vgabase = (void __iomem *) vga_res.start;
>>                        ^
>> I have pushed it out to a topic branch in the tip tree, partly to
>give
>> Fengguang's build bot a run at it (it is excellent at spotting the
>> origin of new warnings), but this needs to be fixed; we will not
>merge
>> this branch in its current form.
>
>Thanks for looking at it. However, these warnings seem unrelated to my
>patchset. I didn't change any vga/fb headers nor did I touch
>arkfb/s3fb. Furthermore, I cannot reproduce these warnings. "vga_res"
>is a "struct resource", "->start" is of type "resource_size_t" which
>is typedef'ed to "phys_addr_t". Seems like the build-bot used some
>random config with CONFIG_PHYS_ADDR_T_64BIT set but building on 32bit
>(or something like that).
>
>Is there any place where I can see the compiler log? Or the used
>.config file?
>
>Thanks
>David

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

^ permalink raw reply

* Re: [PATCH] omapfb: In omapfb_probe return -EPROBE_DEFER when display driver is not loaded yet
From: Pali Rohár @ 2013-08-04  8:36 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Pavel Machek, Jean-Christophe Plagniol-Villard, linux-omap,
	linux-fbdev, linux-kernel, Aaro Koskinen, Tony Lindgren
In-Reply-To: <51EE5483.7000201@ti.com>

[-- Attachment #1: Type: Text/Plain, Size: 3217 bytes --]

Hello,

On Tuesday 23 July 2013 12:01:39 Tomi Valkeinen wrote:
> On 13/07/13 21:27, Pavel Machek wrote:
> > On Wed 2013-07-10 15:08:59, Pali Rohár wrote:
> >> * On RX-51 probing for acx565akm driver is later then for
> >> omapfb which cause that omapfb probe fail and framebuffer
> >> is not working * EPROBE_DEFER causing that kernel try to
> >> probe for omapfb later again which fixing this problem
> >> 
> >> * Without this patch display on Nokia RX-51 (N900) phone
> >> not working
> >> 
> >> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > 
> > Tested-by: Pavel Machek <pavel@ucw.cz>
> 
> Which kernel version is this? Does it have
> dfbc32316c6991010328c21e6046b05bac57eb84 (OMAPFB: defer probe
> if no displays)?
> 

Kernel 3.10, so it have above commit.

> Then again, rx51 has tv-output, which probably gets probed
> early. So omapfb does see one functional display, and starts,
> even if the LCD is not available yet.
> 
> > (Actually, do we know which commit broke the ordering? We
> > may want to simply revert that one...)
> 
> Well, my understand that this is how it's supposed to work.
> There's no defined order with the driver probes, and the
> drivers just have to deal with their dependencies not being
> there yet.
> 
> I don't have a perfect solution for this problem for omapfb.
> omapfb doesn't support dynamically adding displays, so all
> the displays it uses have to be probed before omapfb. And
> omapfb doesn't know which displays will be probed.
> 
> The patch below was added for 3.11. Does it fix the issue for
> you? Perhaps it should be added for 3.10 also.
> 
>  Tomi
> 

I tested patch below and it fixing our problem too. So it could be 
added to 3.10.

> commit e9f322b4913e5d3e5c5d21dc462ca6f8a86e1df1
> Author: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Date:   Thu May 23 16:41:25 2013 +0300
> 
>     OMAPFB: use EPROBE_DEFER if default display is not present
> 
>     Currently omapfb returns EPROBE_DEFER if no displays have
> been probed at the time omapfb is probed. However, sometimes
> some of the displays have been probed at that time, but not
> all. We can't return EPROBE_DEFER in that case, because then
> one missing driver would cause omapfb to defer always,
> preventing any display from working.
> 
>     However, if the user has defined a default display, we can
> presume that the driver for that display is eventually
> loaded. Thus, this patch changes omapfb to return
> EPROBE_DEFER in case default display is not found.
> 
>     Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> diff --git a/drivers/video/omap2/omapfb/omapfb-main.c
> b/drivers/video/omap2/omapfb/omapfb-main.c index
> 528e453..27d6905 100644
> --- a/drivers/video/omap2/omapfb/omapfb-main.c
> +++ b/drivers/video/omap2/omapfb/omapfb-main.c
> @@ -2503,7 +2503,7 @@ static int omapfb_probe(struct
> platform_device *pdev)
> 
>         if (def_display == NULL) {
>                 dev_err(fbdev->dev, "failed to find default
> display\n"); -               r = -EINVAL;
> +               r = -EPROBE_DEFER;
>                 goto cleanup;
>         }

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* [PATCH] fbdev: suppress warning when assigning vga-save/restore base
From: David Herrmann @ 2013-08-04 17:25 UTC (permalink / raw)
  To: linux-fbdev
  Cc: linux-kernel, H. Peter Anvin, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen, Ondrej Zajicek, David Miller, David Herrmann

If drivers use "struct resource" objects to retrieve the vga-base, they
must correctly cast the integer to pointer. With x86+PAE we have 32bit
pointers but 64bit resource_size_t. Hence, cast it to "unsigned long"
before casting to "void*" to suppress warnings due to size differences.

As IO addresses are always low addresses, we can safely drop the higher
part of the address. This is what these drivers did before, anyway.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Reported-by: H. Peter Anvin <hpa@zytor.com>
---
Hi

hpa reported build-warnings on i386+PAE:
/home/hpa/kernel/distwork/drivers/video/arkfb.c:1019:23: warning: cast to
    pointer from integer of different size [-Wint-to-pointer-cast]
    par->state.vgabase = (void __iomem *) vga_res.start;
                         ^
/home/hpa/kernel/distwork/drivers/video/s3fb.c: In function ‘s3_pci_probe’:
/home/hpa/kernel/distwork/drivers/video/s3fb.c:1186:23: warning: cast to pointer
    from integer of different size [-Wint-to-pointer-cast]
    par->state.vgabase = (void __iomem *) vga_res.start;
                         ^

This is due to resource_size_t being 64bit but "void*" 32bit. This patch tries
to suppress these warnings but I am not really comfortable fixing this. I have
no idea whether my assumption (IO address are 32bit) is right. Please verify.

 @David: The following 3 commits of yours presumably introduced the warnings. I
would be glad if you could review this:

commit 94c322c30bd14ae6cdd369cb4a1f94c5c3809ac9
Author: David Miller <davem@davemloft.net>
Date:   Tue Jan 11 23:54:21 2011 +0000

    s3fb: Compute VGA base iomem pointer explicitly.

commit 892c24ca40ffebf6d0ca4cc1454e58db131a4f5a
Author: David Miller <davem@davemloft.net>
Date:   Tue Jan 11 23:54:35 2011 +0000

    arkfb: Compute VGA base iomem pointer explicitly.

commit 6a2f6d5e970afbc1b8b08bafae9d9138a3206960
Author: David Miller <davem@davemloft.net>
Date:   Tue Jan 11 23:55:17 2011 +0000

    vt8623fb: Compute VGA base iomem pointer explicitly.

Cheers
David

 drivers/video/arkfb.c    | 2 +-
 drivers/video/s3fb.c     | 2 +-
 drivers/video/vt8623fb.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/video/arkfb.c b/drivers/video/arkfb.c
index 94a51f1..a4e487c 100644
--- a/drivers/video/arkfb.c
+++ b/drivers/video/arkfb.c
@@ -1016,7 +1016,7 @@ static int ark_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	pcibios_bus_to_resource(dev, &vga_res, &bus_reg);
 
-	par->state.vgabase = (void __iomem *) vga_res.start;
+	par->state.vgabase = (void __iomem *)(unsigned long) vga_res.start;
 
 	/* FIXME get memsize */
 	regval = vga_rseq(par->state.vgabase, 0x10);
diff --git a/drivers/video/s3fb.c b/drivers/video/s3fb.c
index 47ca86c..fc2208b 100644
--- a/drivers/video/s3fb.c
+++ b/drivers/video/s3fb.c
@@ -1183,7 +1183,7 @@ static int s3_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	pcibios_bus_to_resource(dev, &vga_res, &bus_reg);
 
-	par->state.vgabase = (void __iomem *) vga_res.start;
+	par->state.vgabase = (void __iomem *)(unsigned long) vga_res.start;
 
 	/* Unlock regs */
 	cr38 = vga_rcrt(par->state.vgabase, 0x38);
diff --git a/drivers/video/vt8623fb.c b/drivers/video/vt8623fb.c
index e9557fa..3039d82 100644
--- a/drivers/video/vt8623fb.c
+++ b/drivers/video/vt8623fb.c
@@ -729,7 +729,7 @@ static int vt8623_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	pcibios_bus_to_resource(dev, &vga_res, &bus_reg);
 
-	par->state.vgabase = (void __iomem *) vga_res.start;
+	par->state.vgabase = (void __iomem *)(unsigned long) vga_res.start;
 
 	/* Find how many physical memory there is on card */
 	memsize1 = (vga_rseq(par->state.vgabase, 0x34) + 1) >> 1;
-- 
1.8.3.4


^ permalink raw reply related

* Re: [PATCH RESEND 0/8] x86 platform framebuffers
From: David Herrmann @ 2013-08-04 17:30 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: linux-kernel, David Airlie, Geert Uytterhoeven, Stephen Warren,
	Peter Jones, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Ingo Molnar, Thomas Gleixner, x86, linux-fbdev, Andrew Morton
In-Reply-To: <cf7f27cd-4b3e-41bf-ba26-0955a93b04ec@email.android.com>

Hi

On Sat, Aug 3, 2013 at 5:53 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> When compiled for i386 PAE phys_addr_t is 64 bits but pointers are 32 bits.

Yepp, that makes sense. I am not really comfortable fixing this as I
have no idea how PAE actually works, but I digged through git history
and sent a patch to linux-fbdev (I put you on CC). The (long)-cast
should be sufficient, I guess.

Were there any other build warnings you encountered?

Thanks
David

> David Herrmann <dh.herrmann@gmail.com> wrote:
>>Hi
>>
>>On Sat, Aug 3, 2013 at 1:39 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>>> On 08/02/2013 05:05 AM, David Herrmann wrote:
>>>> Hi
>>>>
>>>> I cut down my previous series to no longer include the SimpleDRM
>>driver. If
>>>> anyone is interested, you can find it here:
>>>>   http://lwn.net/Articles/558104/
>>>> I will resend it once these preparation patches are in.
>>>>
>>>> Changes since v2:
>>>>  - added common x86 formats (reported by hpa) (patch #5)
>>>>
>>>> This whole series (including simpledrm) is tested by Stephen and me.
>>I would be
>>>> glad if maintainers could ack/nack this so I can continue my work.
>>>>
>>>> This series is pretty small and just converts x86 to use
>>platform-devices
>>>> instead of global objects to pass framebuffer data to drivers. The
>>commit
>>>> messages explain everything in detail.
>>>> The idea is to create a "platform-framebuffer" device which drivers
>>can bind to.
>>>> If x86 boot code detectes efi or vesa framebuffers, it creates
>>efi-framebuffer
>>>> or vesa-framebuffer devices instead.
>>>>
>>>> Additionally, if the modes are compatible, "simple-framebuffer"
>>devices are
>>>> created so simplefb can be used on x86. This feature is only enabled
>>if
>>>> CONFIG_X86_SYSFB is selected (off by default) so users without
>>simplefb still
>>>> get boot logs.
>>>>
>>>>  @Stephen: I wasn't sure whether you tested the efi/vesa framebuffer
>>changes,
>>>> too, so I didn't add your tested-by there. And I changed patch #5 so
>>I dropped
>>>> it there, too. Thanks for testing!
>>>>
>>>
>>> I am getting a bunch of new warnings with this patchset, typically of
>>> the form:
>>>
>>>
>>> /home/hpa/kernel/distwork/drivers/video/arkfb.c:1019:23: warning:
>>cast
>>> to pointer from integer of different size [-Wint-to-pointer-cast]
>>>   par->state.vgabase = (void __iomem *) vga_res.start;
>>>                        ^
>>> /home/hpa/kernel/distwork/drivers/video/s3fb.c: In function
>>‘s3_pci_probe’:
>>> /home/hpa/kernel/distwork/drivers/video/s3fb.c:1186:23: warning: cast
>>to
>>> pointer from integer of different size [-Wint-to-pointer-cast]
>>>   par->state.vgabase = (void __iomem *) vga_res.start;
>>>                        ^
>>> I have pushed it out to a topic branch in the tip tree, partly to
>>give
>>> Fengguang's build bot a run at it (it is excellent at spotting the
>>> origin of new warnings), but this needs to be fixed; we will not
>>merge
>>> this branch in its current form.
>>
>>Thanks for looking at it. However, these warnings seem unrelated to my
>>patchset. I didn't change any vga/fb headers nor did I touch
>>arkfb/s3fb. Furthermore, I cannot reproduce these warnings. "vga_res"
>>is a "struct resource", "->start" is of type "resource_size_t" which
>>is typedef'ed to "phys_addr_t". Seems like the build-bot used some
>>random config with CONFIG_PHYS_ADDR_T_64BIT set but building on 32bit
>>(or something like that).
>>
>>Is there any place where I can see the compiler log? Or the used
>>.config file?
>>
>>Thanks
>>David
>
> --
> Sent from my mobile phone. Please excuse brevity and lack of formatting.

^ permalink raw reply

* Re: [PATCH] fbdev: suppress warning when assigning vga-save/restore base
From: H. Peter Anvin @ 2013-08-04 17:33 UTC (permalink / raw)
  To: David Herrmann
  Cc: linux-fbdev, linux-kernel, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen, Ondrej Zajicek, David Miller
In-Reply-To: <1375637141-2878-1-git-send-email-dh.herrmann@gmail.com>

On 08/04/2013 10:25 AM, David Herrmann wrote:
> If drivers use "struct resource" objects to retrieve the vga-base, they
> must correctly cast the integer to pointer. With x86+PAE we have 32bit
> pointers but 64bit resource_size_t. Hence, cast it to "unsigned long"
> before casting to "void*" to suppress warnings due to size differences.
> 
> As IO addresses are always low addresses, we can safely drop the higher
> part of the address. This is what these drivers did before, anyway.
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> Reported-by: H. Peter Anvin <hpa@zytor.com>

I'm still bothered here.  Casting between resource_size_t and void *
implies a confusion between physical and virtual addresses.  That may be
unavoidable for some reason, but I'd like to know what exactly is confused.

Anyone who can dig backwards and summarize?  In other words:

Where in the current code do we stuff a physical address in a pointer,
or a virtual address into a non-pointer?

	-hpa



^ permalink raw reply

* Re: [PATCH RESEND 0/8] x86 platform framebuffers
From: H. Peter Anvin @ 2013-08-04 17:34 UTC (permalink / raw)
  To: David Herrmann
  Cc: linux-kernel, David Airlie, Geert Uytterhoeven, Stephen Warren,
	Peter Jones, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Ingo Molnar, Thomas Gleixner, x86, linux-fbdev, Andrew Morton
In-Reply-To: <CANq1E4S+MSv2s+ikPuJ7+jzbQfQFxjdj7wBk54e=n57GtZ0eeA@mail.gmail.com>

On 08/04/2013 10:30 AM, David Herrmann wrote:
> Hi
> 
> On Sat, Aug 3, 2013 at 5:53 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> When compiled for i386 PAE phys_addr_t is 64 bits but pointers are 32 bits.
> 
> Yepp, that makes sense. I am not really comfortable fixing this as I
> have no idea how PAE actually works, but I digged through git history
> and sent a patch to linux-fbdev (I put you on CC). The (long)-cast
> should be sufficient, I guess.
> 
> Were there any other build warnings you encountered?
> 
> Thanks
> David
> 

It looks like Fengguang's bot hasn't run yet, so I don't have a summary.
 I asked Fengguang what I need to do to kick it off.

	-hpa



^ permalink raw reply

* Re: [PATCH] fbdev: suppress warning when assigning vga-save/restore base
From: David Miller @ 2013-08-05  1:51 UTC (permalink / raw)
  To: hpa
  Cc: dh.herrmann, linux-fbdev, linux-kernel, plagnioj, tomi.valkeinen,
	santiago
In-Reply-To: <51FE907A.2090201@zytor.com>

From: "H. Peter Anvin" <hpa@zytor.com>
Date: Sun, 04 Aug 2013 10:33:46 -0700

> Anyone who can dig backwards and summarize?  In other words:
> 
> Where in the current code do we stuff a physical address in a pointer,
> or a virtual address into a non-pointer?

The VGA register accessors try to accomodate iomem and ioport
accesses.

If they are given a non-NULL iomem pointer 'regbase' they use
iomem accesses, otherwise they do direct ISA port poking.

And yes the drivers in question are making some brash assumptions.
I suspect they should be using ioremap() or similar.

^ permalink raw reply

* RE: [PATCH 4/5] fb: Add DCU framebuffer driver for Vybrid VF610 platform
From: Wang Huan-B18965 @ 2013-08-05  9:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130729111457.GB3029@pengutronix.de>

SGksIFNhc2NoYSwNCg0KPiBPbiBGcmksIEp1bCAxMiwgMjAxMyBhdCAwMjowNzo1NVBNICswODAw
LCBBbGlzb24gV2FuZyB3cm90ZToNCj4gPiBUaGUgRGlzcGxheSBDb250cm9sbGVyIFVuaXQgKERD
VSkgbW9kdWxlIGlzIGEgc3lzdGVtIG1hc3RlciB0aGF0DQo+ID4gZmV0Y2hlcyBncmFwaGljcyBz
dG9yZWQgaW4gaW50ZXJuYWwgb3IgZXh0ZXJuYWwgbWVtb3J5IGFuZCBkaXNwbGF5cw0KPiA+IHRo
ZW0gb24gYSBURlQgTENEIHBhbmVsLiBBIHdpZGUgcmFuZ2Ugb2YgcGFuZWwgc2l6ZXMgaXMgc3Vw
cG9ydGVkIGFuZA0KPiA+IHRoZSB0aW1pbmcgb2YgdGhlIGludGVyZmFjZSBzaWduYWxzIGlzIGhp
Z2hseSBjb25maWd1cmFibGUuDQo+ID4gR3JhcGhpY3MgYXJlIHJlYWQgZGlyZWN0bHkgZnJvbSBt
ZW1vcnkgYW5kIHRoZW4gYmxlbmRlZCBpbiByZWFsLXRpbWUsDQo+ID4gd2hpY2ggYWxsb3dzIGZv
ciBkeW5hbWljIGNvbnRlbnQgY3JlYXRpb24gd2l0aCBtaW5pbWFsIENQVQ0KPiBpbnRlcnZlbnRp
b24uDQo+IA0KPiBPbmx5IGEgcmV2aWV3IG9mIHRoZSBjb2RlIGlubGluZS4NCj4gDQo+IE1heWJl
IHRoZSByZWFsIHF1ZXN0aW9uIGlzIHdoZXRoZXIgd2Ugd2FudCB0byBpbnRyb2R1Y2UgYW5vdGhl
cg0KPiBmcmFtZWJ1ZmZlciBkcml2ZXIgYXQgYWxsIGluc3RlYWQgb2YgbWFraW5nIGl0IGEgRFJN
IGRyaXZlci4NCltBbGlzb24gV2FuZ10gSSB0aGluayBEQ1UgbW9kdWxlIGlzIG1vcmUgc3VpdGFi
bGUgdG8gYmUgZGVzaWduZWQgYXMgYSBmcmFtZWJ1ZmZlciBkcml2ZXIgdGhhbiBhIERSTSBkcml2
ZXIuIEp1c3QgbGlrZSBESVUgZnJhbWVidWZmZXIgZHJpdmVyIGZvciBQb3dlclBDLg0KPiANCj4g
PiArDQo+ID4gKyNkZWZpbmUgRFJJVkVSX05BTUUJCQkiZnNsLWRjdS1mYiINCj4gPiArDQo+ID4g
KyNkZWZpbmUgRENVX0RDVV9NT0RFCQkJMHgwMDEwDQo+ID4gKyNkZWZpbmUgRENVX01PREVfQkxF
TkRfSVRFUih4KQkJKHggPDwgMjApDQo+IA0KPiBXaGF0J3MgdGhlIHJlc3VsdCBvZiBEQ1VfTU9E
RV9CTEVORF9JVEVSKDEgKyAxKT8NCltBbGlzb24gV2FuZ10gSXMgaXQgcmVhbGx5IG5lY2Vzc2Fy
eT8gSSBkb24ndCB1c2UgdGhpcyBtYWNybyBsaWtlIERDVV9NT0RFX0JMRU5EX0lURVIoMSArIDEp
LCBqdXN0IHVzZSBEQ1VfTU9ERV9CTEVORF9JVEVSKDIpLg0KPiANCj4gPiArc3RhdGljIHN0cnVj
dCBmYl92aWRlb21vZGUgZGN1X21vZGVfZGJbXSA9IHsNCj4gPiArCXsNCj4gPiArCQkubmFtZQkJ
PSAiNDgweDI3MiIsDQo+ID4gKwkJLnJlZnJlc2gJPSA3NSwNCj4gPiArCQkueHJlcwkJPSA0ODAs
DQo+ID4gKwkJLnlyZXMJCT0gMjcyLA0KPiA+ICsJCS5waXhjbG9jawk9IDkxOTk2LA0KPiA+ICsJ
CS5sZWZ0X21hcmdpbgk9IDIsDQo+ID4gKwkJLnJpZ2h0X21hcmdpbgk9IDIsDQo+ID4gKwkJLnVw
cGVyX21hcmdpbgk9IDEsDQo+ID4gKwkJLmxvd2VyX21hcmdpbgk9IDEsDQo+ID4gKwkJLmhzeW5j
X2xlbgk9IDQxLA0KPiA+ICsJCS52c3luY19sZW4JPSAyLA0KPiA+ICsJCS5zeW5jCQk9IEZCX1NZ
TkNfQ09NUF9ISUdIX0FDVCB8IEZCX1NZTkNfVkVSVF9ISUdIX0FDVCwNCj4gPiArCQkudm1vZGUJ
CT0gRkJfVk1PREVfTk9OSU5URVJMQUNFRCwNCj4gPiArCX0sDQo+ID4gK307DQo+IA0KPiBXZSBo
YXZlIHdheXMgdG8gZGVzY3JpYmUgYSBwYW5lbCBpbiBkdC4gVXNlIHRoZW0uDQpbQWxpc29uIFdh
bmddIE9rLiBJIGRvbid0IGtub3cgaXQgaXMgY29tbW9uIHRvIGRlc2NyaWJlIHRoZSBwYW5lbCBp
biBkdHMgZm9yIHRoZSBsYXRlc3Qga2VybmVsLiBUaGFua3MuDQo+IA0KPiA+ICsNCj4gPiArc3Rh
dGljIHN0cnVjdCBtZmJfaW5mbyBtZmJfdGVtcGxhdGVbXSA9IHsNCj4gPiArCXsNCj4gPiArCS5p
bmRleCA9IExBWUVSMCwNCj4gPiArCS5pZCA9ICJMYXllcjAiLA0KPiA+ICsJLmFscGhhID0gMHhm
ZiwNCj4gPiArCS5ibGVuZCA9IDAsDQo+ID4gKwkuY291bnQgPSAwLA0KPiA+ICsJLnhfbGF5ZXJf
ZCA9IDAsDQo+ID4gKwkueV9sYXllcl9kID0gMCwNCj4gPiArCX0sDQo+IA0KPiBXcm9uZyBpbmRl
bnRhdGlvbi4NCltBbGlzb24gV2FuZ10gSSB3aWxsIGZpeCBpdCBpbiBuZXh0IHZlcnNpb24uDQo+
IA0KPiA+ICsJZGVmYXVsdDoNCj4gPiArCQlwcmludGsoS0VSTl9FUlIgInVuc3VwcG9ydGVkIGNv
bG9yIGRlcHRoOiAldVxuIiwNCj4gPiArCQkJdmFyLT5iaXRzX3Blcl9waXhlbCk7DQo+IA0KPiBV
c2UgZGV2XyogZm9yIHByaW50aW5nIG1lc3NhZ2VzIGluIGRyaXZlcnMuDQpbQWxpc29uIFdhbmdd
IE9rLg0KPiANCj4gPiArc3RhdGljIGludCBmc2xfZGN1X2NoZWNrX3ZhcihzdHJ1Y3QgZmJfdmFy
X3NjcmVlbmluZm8gKnZhciwNCj4gPiArCQlzdHJ1Y3QgZmJfaW5mbyAqaW5mbykNCj4gPiArew0K
PiA+ICsJaWYgKHZhci0+eHJlc192aXJ0dWFsIDwgdmFyLT54cmVzKQ0KPiA+ICsJCXZhci0+eHJl
c192aXJ0dWFsID0gdmFyLT54cmVzOw0KPiA+ICsJaWYgKHZhci0+eXJlc192aXJ0dWFsIDwgdmFy
LT55cmVzKQ0KPiA+ICsJCXZhci0+eXJlc192aXJ0dWFsID0gdmFyLT55cmVzOw0KPiA+ICsNCj4g
PiArCWlmICh2YXItPnhvZmZzZXQgPCAwKQ0KPiA+ICsJCXZhci0+eG9mZnNldCA9IDA7DQo+ID4g
Kw0KPiA+ICsJaWYgKHZhci0+eW9mZnNldCA8IDApDQo+ID4gKwkJdmFyLT55b2Zmc2V0ID0gMDsN
Cj4gDQo+IEV2ZXIgc2VlbiBhbiB1bnNpZ25lZCB2YWx1ZSBnb2luZyBiZWxvdyB6ZXJvPw0KW0Fs
aXNvbiBXYW5nXSBZb3UgYXJlIHJpZ2h0LCBJIHdpbGwgcmVtb3ZlIHRoZW0gaW4gbmV4dCB2ZXJz
aW9uLg0KPiANCj4gPiArCWRlZmF1bHQ6DQo+ID4gKwkJcHJpbnRrKEtFUk5fRVJSICJ1bnN1cHBv
cnRlZCBjb2xvciBkZXB0aDogJXVcbiIsDQo+ID4gKwkJCXZhci0+Yml0c19wZXJfcGl4ZWwpOw0K
PiANCj4gQlVHKCkuIFRoaXMgY2FuJ3QgaGFwcGVuIHNpbmNlIHlvdSBtYWtlIHRoYXQgc3VyZSBh
Ym92ZS4NCltBbGlzb24gV2FuZ10gWW91IGFyZSByaWdodCwgSSB3aWxsIHJlbW92ZSBpdCBpbiBu
ZXh0IHZlcnNpb24uDQo+IA0KPiA+ICsJCXJldHVybiAtRUlOVkFMOw0KPiA+ICsJfQ0KPiA+ICsN
Cj4gPiArCXJldHVybiAwOw0KPiA+ICt9DQo+ID4gKw0KPiA+ICtzdGF0aWMgaW50IGNhbGNfZGl2
X3JhdGlvKHN0cnVjdCBmYl9pbmZvICppbmZvKSB7DQo+IA0KPiBVc2UgYSBjb25zaXN0ZW50IG5h
bWVzcGFjZSBmb3IgZnVuY3Rpb24gbmFtZXMgKGZzbF9kY3VfKQ0KW0FsaXNvbiBXYW5nXSBJcyBp
dCBuZWNlc3NhcnkgdG8gdXNlIGEgY29uc2lzdGVudCBuYW1lc3BhY2UgZm9yIHRoaXMgZ2VuZXJp
YyBmdW5jdGlvbj8gSSB0aGluayBpdCBpcyBuZWNlc3NhcnkgdG8gdXNlIGEgY29uc2lzdGVudCBu
YW1lc3BhY2UgKGZzbF9kY3VfKSBmb3IgdGhlIGZ1bmN0aW9uIG5hbWVzIGluIHN0cnVjdHVyZSBm
c2xfZGN1X29wcy4NCj4gDQo+ID4gKwl3cml0ZWwoRENVX1RIUkVTSE9MRF9MU19CRl9WUygweDMp
IHwNCj4gRENVX1RIUkVTSE9MRF9PVVRfQlVGX0hJR0goMHg3OCkgfA0KPiA+ICsJCURDVV9USFJF
U0hPTERfT1VUX0JVRl9MT1coMCksIGRjdWZiLT5yZWdfYmFzZSArDQo+IERDVV9USFJFU0hPTEQp
Ow0KPiA+ICsNCj4gPiArCWVuYWJsZV9jb250cm9sbGVyKGluZm8pOw0KPiA+ICt9DQo+IA0KPiBN
YWtlIHlvdXIgZnVuY3Rpb25zIHN5bW1ldHJpYy4gSWYgdGhlcmUncyB1cGRhdGVfY29udHJvbGxl
cigpLCB0aGUNCj4gZnVuY3Rpb24gc2hvdWxkIGRvIGV4YWN0bHkgdGhhdCwgaXQgc2hvdWxkICpu
b3QqIGVuYWJsZSB0aGUgY29udHJvbGxlci4NCj4gQ2FsbCB0aGlzIGZyb20gdGhlIHVzZXJzIG9m
IHRoaXMgZnVuY3Rpb24gaWYgbmVjZXNzYXJ5Lg0KW0FsaXNvbiBXYW5nXSBPaywgSSB3aWxsIG1v
dmUgdGhpcyBmdW5jdGlvbiBvdXQsIGFuZCBhZGQgaXQgaGVyZS4NCmlmIChtZmJpLT5pbmRleCA9
PSBMQVlFUjApIHsNCgl1cGRhdGVfY29udHJvbGxlcihpbmZvKTsNCisJZW5hYmxlX2NvbnRyb2xs
ZXIoaW5mbyk7DQp9DQo+IA0KPiA+ICsJCWlmIChjb3B5X3RvX3VzZXIoYnVmLCAmYWxwaGEsIHNp
emVvZihhbHBoYSkpKQ0KPiA+ICsJCQlyZXR1cm4gLUVGQVVMVDsNCj4gPiArCQlicmVhazsNCj4g
PiArCWNhc2UgTUZCX1NFVF9BTFBIQToNCj4gPiArCQlpZiAoY29weV9mcm9tX3VzZXIoJmFscGhh
LCBidWYsIHNpemVvZihhbHBoYSkpKQ0KPiA+ICsJCQlyZXR1cm4gLUVGQVVMVDsNCj4gPiArCQlt
ZmJpLT5ibGVuZCA9IDE7DQo+ID4gKwkJbWZiaS0+YWxwaGEgPSBhbHBoYTsNCj4gPiArCQlmc2xf
ZGN1X3NldF9wYXIoaW5mbyk7DQo+ID4gKwkJYnJlYWs7DQo+IA0KPiBJcyBpdCBzdGlsbCBzdGF0
ZSBvZiB0aGUgYXJ0IHRvIGludHJvZHVjZSBpb2N0bHMgaW4gdGhlIGZiIGZyYW1ld29yaw0KPiB3
aXRob3V0IGFueSBraW5kIG9mIGRvY3VtZW50YXRpb24/DQpbQWxpc29uIFdhbmddIERvIHlvdSBt
ZWFuIEkgbmVlZCB0byBhZGQgYSBkb2N1bWVudGF0aW9uIGluIERvY3VtZW50YXRpb24vZmIvPw0K
PiANCj4gPiArCWRlZmF1bHQ6DQo+ID4gKwkJcHJpbnRrKEtFUk5fRVJSICJ1bmtub3duIGlvY3Rs
IGNvbW1hbmQgKDB4JTA4WClcbiIsIGNtZCk7DQo+IA0KPiBXaGF0IHNoYWxsIGEgcmVhZGVyIG9m
IHRoZSBrZXJuZWwgbG9nIGRvIHdpdGggYSBtZXNzYWdlIGxpa2UgdGhpcz8gSXQncw0KPiB1dHRl
cmx5IHVzZWxlc3Mgd2hlbiBoZSBkb2Vzbid0IGV2ZW4gbm93IHdoaWNoIGRldmljZSBmYWlsZWQg
aGVyZS4gSnVzdA0KPiBkcm9wIHRoaXMuDQpbQWxpc29uIFdhbmddIE9rLg0KPiANCj4gPiArc3Rh
dGljIHZvaWQgZW5hYmxlX2ludGVycnVwdHMoc3RydWN0IGRjdV9mYl9kYXRhICpkY3VmYikgew0K
PiA+ICsJdTMyIGludF9tYXNrID0gcmVhZGwoZGN1ZmItPnJlZ19iYXNlICsgRENVX0lOVF9NQVNL
KTsNCj4gPiArDQo+ID4gKwl3cml0ZWwoaW50X21hc2sgJiB+RENVX0lOVF9NQVNLX1VORFJVTiwg
ZGN1ZmItPnJlZ19iYXNlICsNCj4gPiArRENVX0lOVF9NQVNLKTsgfQ0KPiANCj4gSW5saW5lIHRo
aXMgY29kZSB3aGVyZSB5b3UgbmVlZCBpdC4gSW50cm9kdWNpbmcgYSBmdW5jdGlvbiBmb3IgYSBz
aW5nbGUNCj4gcmVnaXN0ZXIgd3JpdGUgc2VlbXMgcXVpdGUgdXNlbGVzcy4NCltBbGlzb24gV2Fu
Z10gT2ssIEkgd2lsbCBpbmxpbmUgaXQuDQo+IA0KPiA+ICtzdGF0aWMgaW50IGluc3RhbGxfZnJh
bWVidWZmZXIoc3RydWN0IGZiX2luZm8gKmluZm8pIHsNCj4gPiArCXN0cnVjdCBtZmJfaW5mbyAq
bWZiaSA9IGluZm8tPnBhcjsNCj4gPiArCXN0cnVjdCBmYl92aWRlb21vZGUgKmRiID0gZGN1X21v
ZGVfZGI7DQo+ID4gKwl1bnNpZ25lZCBpbnQgZGJzaXplID0gQVJSQVlfU0laRShkY3VfbW9kZV9k
Yik7DQo+ID4gKwlpbnQgcmV0Ow0KPiA+ICsNCj4gPiArCWluZm8tPnZhci5hY3RpdmF0ZSA9IEZC
X0FDVElWQVRFX05PVzsNCj4gPiArCWluZm8tPmZib3BzID0gJmZzbF9kY3Vfb3BzOw0KPiA+ICsJ
aW5mby0+ZmxhZ3MgPSBGQklORk9fRkxBR19ERUZBVUxUOw0KPiA+ICsJaW5mby0+cHNldWRvX3Bh
bGV0dGUgPSAmbWZiaS0+cHNldWRvX3BhbGV0dGU7DQo+ID4gKw0KPiA+ICsJZmJfYWxsb2NfY21h
cCgmaW5mby0+Y21hcCwgMTYsIDApOw0KPiA+ICsNCj4gPiArCXJldCA9IGZiX2ZpbmRfbW9kZSgm
aW5mby0+dmFyLCBpbmZvLCBmYl9tb2RlLCBkYiwgZGJzaXplLA0KPiA+ICsJCQlOVUxMLCBkZWZh
dWx0X2JwcCk7DQo+ID4gKw0KPiA+ICsJaWYgKGZzbF9kY3VfY2hlY2tfdmFyKCZpbmZvLT52YXIs
IGluZm8pKSB7DQo+ID4gKwkJcmV0ID0gLUVJTlZBTDsNCj4gDQo+IFByb3BhZ2F0ZSB0aGUgZXJy
b3IuDQpbQWxpc29uIFdhbmddIEkgd2lsbCBmaXggaW4gbmV4dCB2ZXJzaW9uLg0KPiANCj4gPiAr
CQlnb3RvIGZhaWxlZF9jaGVja3ZhcjsNCj4gPiArCX0NCj4gPiArDQo+ID4gKwlpZiAocmVnaXN0
ZXJfZnJhbWVidWZmZXIoaW5mbykgPCAwKSB7DQo+ID4gKwkJcmV0ID0gLUVJTlZBTDsNCj4gDQo+
IGRpdHRvDQpbQWxpc29uIFdhbmddIEkgd2lsbCBmaXggaW4gbmV4dCB2ZXJzaW9uLg0KPiANCj4g
PiArc3RhdGljIGlycXJldHVybl90IGZzbF9kY3VfaXJxKGludCBpcnEsIHZvaWQgKmRldl9pZCkg
ew0KPiA+ICsJc3RydWN0IGRjdV9mYl9kYXRhICpkY3VmYiA9IGRldl9pZDsNCj4gPiArCXVuc2ln
bmVkIGludCBzdGF0dXMgPSByZWFkbChkY3VmYi0+cmVnX2Jhc2UgKyBEQ1VfSU5UX1NUQVRVUyk7
DQo+ID4gKwl1MzIgZGN1X21vZGU7DQo+ID4gKw0KPiA+ICsJaWYgKHN0YXR1cykgew0KPiANCj4g
U2F2ZSBpbmRlbmRhdGlvbiBsZXZlbCBieSBiYWlsaW5nIG91dCBlYXJseS4NCltBbGlzb24gV2Fu
Z10gV2hhdCBkbyB5b3UgbWVhbj8NCj4gDQo+ID4gKwkJaWYgKHN0YXR1cyAmIERDVV9JTlRfU1RB
VFVTX1VORFJVTikgew0KPiA+ICsJCQlkY3VfbW9kZSA9IHJlYWRsKGRjdWZiLT5yZWdfYmFzZSAr
IERDVV9EQ1VfTU9ERSk7DQo+ID4gKwkJCWRjdV9tb2RlICY9IH5EQ1VfTU9ERV9EQ1VfTU9ERV9N
QVNLOw0KPiA+ICsJCQl3cml0ZWwoZGN1X21vZGUgfCBEQ1VfTU9ERV9EQ1VfTU9ERShEQ1VfTU9E
RV9PRkYpLA0KPiA+ICsJCQkJZGN1ZmItPnJlZ19iYXNlICsgRENVX0RDVV9NT0RFKTsNCj4gPiAr
CQkJdWRlbGF5KDEpOw0KPiA+ICsJCQl3cml0ZWwoZGN1X21vZGUgfCBEQ1VfTU9ERV9EQ1VfTU9E
RShEQ1VfTU9ERV9OT1JNQUwpLA0KPiA+ICsJCQkJZGN1ZmItPnJlZ19iYXNlICsgRENVX0RDVV9N
T0RFKTsNCj4gPiArCQl9DQo+ID4gKwkJd3JpdGVsKHN0YXR1cywgZGN1ZmItPnJlZ19iYXNlICsg
RENVX0lOVF9TVEFUVVMpOw0KPiA+ICsJCXJldHVybiBJUlFfSEFORExFRDsNCj4gPiArCX0NCj4g
PiArCXJldHVybiBJUlFfTk9ORTsNCj4gPiArfQ0KPiA+ICsNCj4gPiArCWlmIChJU19FUlIodGNv
bl9jbGspKSB7DQo+ID4gKwkJcmV0ID0gUFRSX0VSUih0Y29uX2Nsayk7DQo+ID4gKwkJZ290byBm
YWlsZWRfZ2V0Y2xvY2s7DQo+ID4gKwl9DQo+ID4gKwljbGtfcHJlcGFyZV9lbmFibGUodGNvbl9j
bGspOw0KPiA+ICsNCj4gPiArCXRjb25fcmVnID0gb2ZfaW9tYXAodGNvbl9ucCwgMCk7DQo+IA0K
PiBVc2UgZGV2bV8qDQpbQWxpc29uIFdhbmddIE9rLg0KPiANCj4gPiArCWRjdWZiLT5pcnEgPSBw
bGF0Zm9ybV9nZXRfaXJxKHBkZXYsIDApOw0KPiA+ICsJaWYgKCFkY3VmYi0+aXJxKSB7DQo+ID4g
KwkJcmV0ID0gLUVJTlZBTDsNCj4gPiArCQlnb3RvIGZhaWxlZF9nZXRpcnE7DQo+ID4gKwl9DQo+
ID4gKw0KPiA+ICsJcmV0ID0gcmVxdWVzdF9pcnEoZGN1ZmItPmlycSwgZnNsX2RjdV9pcnEsIDAs
IERSSVZFUl9OQU1FLCBkY3VmYik7DQo+IA0KPiBVc2UgZGV2bV9yZXF1ZXN0X2lycQ0KW0FsaXNv
biBXYW5nXSBPay4NCj4gDQo+ID4gKwlpZiAocmV0KSB7DQo+ID4gKwkJZGV2X2VycigmcGRldi0+
ZGV2LCAiY291bGQgbm90IHJlcXVlc3QgaXJxXG4iKTsNCj4gPiArCQlnb3RvIGZhaWxlZF9yZXF1
ZXN0aXJxOw0KPiA+ICsJfQ0KPiA+ICsNCj4gPiArCS8qIFB1dCBUQ09OIGluIGJ5cGFzcyBtb2Rl
LCBzbyB0aGUgaW5wdXQgc2lnbmFscyBmcm9tIERDVSBhcmUNCj4gcGFzc2VkDQo+ID4gKwkgKiB0
aHJvdWdoIFRDT04gdW5jaGFuZ2VkICovDQo+ID4gKwlyZXQgPSBieXBhc3NfdGNvbihwZGV2LT5k
ZXYub2Zfbm9kZSk7DQo+ID4gKwlpZiAocmV0KSB7DQo+ID4gKwkJZGV2X2VycigmcGRldi0+ZGV2
LCAiY291bGQgbm90IGJ5cGFzcyBUQ09OXG4iKTsNCj4gPiArCQlnb3RvIGZhaWxlZF9ieXBhc3N0
Y29uOw0KPiA+ICsJfQ0KPiA+ICsNCj4gPiArCWRjdWZiLT5jbGsgPSBkZXZtX2Nsa19nZXQoJnBk
ZXYtPmRldiwgImRjdSIpOw0KPiA+ICsJaWYgKElTX0VSUihkY3VmYi0+Y2xrKSkgew0KPiA+ICsJ
CWRldl9lcnIoJnBkZXYtPmRldiwgImNvdWxkIG5vdCBnZXQgY2xvY2tcbiIpOw0KPiA+ICsJCWdv
dG8gZmFpbGVkX2dldGNsb2NrOw0KPiANCj4gWW91IHdpbGwgcmV0dXJuIDAgaGVyZS4NCltBbGlz
b24gV2FuZ10gWW91IGFyZSByaWdodCwgSSB3aWxsIGZpeCBpdCBpbiBuZXh0IHZlcnNpb24uDQo+
IA0KPiA+ICsJfQ0KPiA+ICsJY2xrX3ByZXBhcmVfZW5hYmxlKGRjdWZiLT5jbGspOw0KPiA+ICsN
Cj4gPiArCWZvciAoaSA9IDA7IGkgPCBBUlJBWV9TSVpFKGRjdWZiLT5mc2xfZGN1X2luZm8pOyBp
KyspIHsNCj4gPiArCQlkY3VmYi0+ZnNsX2RjdV9pbmZvW2ldID0NCj4gPiArCQkJZnJhbWVidWZm
ZXJfYWxsb2Moc2l6ZW9mKHN0cnVjdCBtZmJfaW5mbyksICZwZGV2LQ0KPiA+ZGV2KTsNCj4gPiAr
CQlpZiAoIWRjdWZiLT5mc2xfZGN1X2luZm9baV0pIHsNCj4gPiArCQkJcmV0ID0gRU5PTUVNOw0K
PiANCj4gLUVOT01FTQ0KPiANCj4gPiArZmFpbGVkX2FsbG9jX2ZyYW1lYnVmZmVyOg0KPiA+ICtm
YWlsZWRfZ2V0Y2xvY2s6DQo+ID4gK2ZhaWxlZF9ieXBhc3N0Y29uOg0KPiA+ICsJZnJlZV9pcnEo
ZGN1ZmItPmlycSwgZGN1ZmIpOw0KPiA+ICtmYWlsZWRfcmVxdWVzdGlycToNCj4gPiArZmFpbGVk
X2dldGlycToNCj4gPiArCWlvdW5tYXAoZGN1ZmItPnJlZ19iYXNlKTsNCj4gDQo+IFlvdSB1c2Vk
IGRldm1faW9yZW1hcCwgc28gZHJvcCB0aGlzLg0KW0FsaXNvbiBXYW5nXSBPay4NCj4gDQo+ID4g
K2ZhaWxlZF9pb3JlbWFwOg0KPiA+ICsJa2ZyZWUoZGN1ZmIpOw0KPiANCj4gVGhpcyBpcyBhbGxv
Y2F0ZWQgd2l0aCBkZXZtXyouIERyb3AgdGhpcy4NCltBbGlzb24gV2FuZ10gT2suDQoNClRoYW5r
cyBmb3IgeW91ciBjb21tZW50cy4NCg0KDQpCZXN0IFJlZ2FyZHMsDQpBbGlzb24gV2FuZw0KDQoN
Cg=


^ permalink raw reply

* Re: [PATCH 4/5] fb: Add DCU framebuffer driver for Vybrid VF610 platform
From: Sascha Hauer @ 2013-08-05 10:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <81BA6E5E0BC2344391CABCEE22D1B6D83CA6F0@039-SN1MPN1-002.039d.mgd.msft.net>

On Mon, Aug 05, 2013 at 09:51:40AM +0000, Wang Huan-B18965 wrote:
> Hi, Sascha,
> 
> > On Fri, Jul 12, 2013 at 02:07:55PM +0800, Alison Wang wrote:
> > > The Display Controller Unit (DCU) module is a system master that
> > > fetches graphics stored in internal or external memory and displays
> > > them on a TFT LCD panel. A wide range of panel sizes is supported and
> > > the timing of the interface signals is highly configurable.
> > > Graphics are read directly from memory and then blended in real-time,
> > > which allows for dynamic content creation with minimal CPU
> > intervention.
> > 
> > Only a review of the code inline.
> > 
> > Maybe the real question is whether we want to introduce another
> > framebuffer driver at all instead of making it a DRM driver.
> [Alison Wang] I think DCU module is more suitable to be designed as a framebuffer driver than a DRM driver. Just like DIU framebuffer driver for PowerPC.
> > 
> > > +
> > > +#define DRIVER_NAME			"fsl-dcu-fb"
> > > +
> > > +#define DCU_DCU_MODE			0x0010
> > > +#define DCU_MODE_BLEND_ITER(x)		(x << 20)
> > 
> > What's the result of DCU_MODE_BLEND_ITER(1 + 1)?
> [Alison Wang] Is it really necessary? I don't use this macro like DCU_MODE_BLEND_ITER(1 + 1), just use DCU_MODE_BLEND_ITER(2).

<irony>
No it's not necessary. We can just add incorrect macros everywhere and
fix them as necessary after a long bug squashing session
</irony>

YES! This is necessary.

> > > +	return 0;
> > > +}
> > > +
> > > +static int calc_div_ratio(struct fb_info *info) {
> > 
> > Use a consistent namespace for function names (fsl_dcu_)
> [Alison Wang] Is it necessary to use a consistent namespace for this generic function? I think it is necessary to use a consistent namespace (fsl_dcu_) for the function names in structure fsl_dcu_ops.

This function calculates some divider out of a struct fb_info. This is
by no means generic.

> > > +		if (copy_to_user(buf, &alpha, sizeof(alpha)))
> > > +			return -EFAULT;
> > > +		break;
> > > +	case MFB_SET_ALPHA:
> > > +		if (copy_from_user(&alpha, buf, sizeof(alpha)))
> > > +			return -EFAULT;
> > > +		mfbi->blend = 1;
> > > +		mfbi->alpha = alpha;
> > > +		fsl_dcu_set_par(info);
> > > +		break;
> > 
> > Is it still state of the art to introduce ioctls in the fb framework
> > without any kind of documentation?
> [Alison Wang] Do you mean I need to add a documentation in Documentation/fb/?

This was more a question to the fb maintainers.

> > > +static irqreturn_t fsl_dcu_irq(int irq, void *dev_id) {
> > > +	struct dcu_fb_data *dcufb = dev_id;
> > > +	unsigned int status = readl(dcufb->reg_base + DCU_INT_STATUS);
> > > +	u32 dcu_mode;
> > > +
> > > +	if (status) {
> > 
> > Save indendation level by bailing out early.
> [Alison Wang] What do you mean?


Instead of doing:

	if (bla) {
		dothis();
		dothat();
		return;
	}

	return;

it's easier to read:

	if (!bla)
		return;

	dothis();
	dothat();
	return;

Note that dothis() and dothat() are one indentation level to the left
and thus have more space per line.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCH 4/5] fb: Add DCU framebuffer driver for Vybrid VF610 platform
From: Robert Schwebel @ 2013-08-05 13:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <81BA6E5E0BC2344391CABCEE22D1B6D83CA6F0@039-SN1MPN1-002.039d.mgd.msft.net>

On Mon, Aug 05, 2013 at 09:51:40AM +0000, Wang Huan-B18965 wrote:
> > On Fri, Jul 12, 2013 at 02:07:55PM +0800, Alison Wang wrote:
> > > The Display Controller Unit (DCU) module is a system master that
> > > fetches graphics stored in internal or external memory and displays
> > > them on a TFT LCD panel. A wide range of panel sizes is supported and
> > > the timing of the interface signals is highly configurable.
> > > Graphics are read directly from memory and then blended in real-time,
> > > which allows for dynamic content creation with minimal CPU
> > intervention.
> > 
> > Only a review of the code inline.
> > 
> > Maybe the real question is whether we want to introduce another
> > framebuffer driver at all instead of making it a DRM driver.
> [Alison Wang] I think DCU module is more suitable to be designed as a framebuffer driver than a DRM driver. Just like DIU framebuffer driver for PowerPC.

We looked at the Vybrid datasheet and it's DCU section last week, and
with its 64 planes, the controller really wants to get a DRM driver.

rsc
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCH 4/5] fb: Add DCU framebuffer driver for Vybrid VF610 platform
From: Lucas Stach @ 2013-08-05 14:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130805130911.GF30920@pengutronix.de>

Am Montag, den 05.08.2013, 15:09 +0200 schrieb Robert Schwebel:
> On Mon, Aug 05, 2013 at 09:51:40AM +0000, Wang Huan-B18965 wrote:
> > > On Fri, Jul 12, 2013 at 02:07:55PM +0800, Alison Wang wrote:
> > > > The Display Controller Unit (DCU) module is a system master that
> > > > fetches graphics stored in internal or external memory and displays
> > > > them on a TFT LCD panel. A wide range of panel sizes is supported and
> > > > the timing of the interface signals is highly configurable.
> > > > Graphics are read directly from memory and then blended in real-time,
> > > > which allows for dynamic content creation with minimal CPU
> > > intervention.
> > > 
> > > Only a review of the code inline.
> > > 
> > > Maybe the real question is whether we want to introduce another
> > > framebuffer driver at all instead of making it a DRM driver.
> > [Alison Wang] I think DCU module is more suitable to be designed as a framebuffer driver than a DRM driver. Just like DIU framebuffer driver for PowerPC.
> 
> We looked at the Vybrid datasheet and it's DCU section last week, and
> with its 64 planes, the controller really wants to get a DRM driver.
> 
Exactly, with it's extensive hardware composition capabilities the
controller begs for being driven by a proper DRM driver. There is no way
in fbdev to properly support all those hardware planes without
introducing a lot of non standard ioctls, which in turn will force you
into writing a lot of custom userspace code instead of running proven
technology that sits on top of KMS.

Doing things in DRM might be slightly more work for the initial bring
up, but will pay off in the long run when you are going to really use
the hardware caps of the controller.

Regards,
Lucas
-- 
Pengutronix e.K.                           | Lucas Stach                 |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox