linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 3/4] fbdev: uvesafb driver
@ 2007-06-23 10:52 Michal Januszewski
  2007-06-23 11:51 ` [Linux-fbdev-devel] " Bernhard Fischer
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Michal Januszewski @ 2007-06-23 10:52 UTC (permalink / raw)
  To: linux-fbdev-devel; +Cc: linux-kernel

Add the uvesafb driver, an enhanced version of vesafb that makes use of
a userspace helper to run x86 Video BIOS code.

Signed-off-by: Michal Januszewski <spock@gentoo.org>
---
 drivers/video/Kconfig   |   18 +
 drivers/video/Makefile  |    1 +
 drivers/video/uvesafb.c | 1902 +++++++++++++++++++++++++++++++++++++++++++++++
 include/video/Kbuild    |    2 +-
 include/video/uvesafb.h |  208 ++++++
 5 files changed, 2130 insertions(+), 1 deletions(-)

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 403dac7..5cc03f9 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -585,6 +585,24 @@ config FB_TGA
 
 	  Say Y if you have one of those.
 
+config FB_UVESA
+	tristate "Userspace VESA VGA graphics support"
+	depends on FB && CONNECTOR
+	select FB_CFB_FILLRECT
+	select FB_CFB_COPYAREA
+	select FB_CFB_IMAGEBLIT
+	select FB_MODE_HELPERS
+	help
+	  This is the frame buffer driver for generic VBE 2.0 compliant
+	  graphic cards. It can also take advantage of VBE 3.0 features,
+	  such as refresh rate adjustment.
+
+	  This driver generally provides more features than vesafb but
+	  requires a userspace helper application called 'v86d'. See
+	  <file:Documentation/fb/uvesafb.txt> for more information.
+
+	  If unsure, say N.
+
 config FB_VESA
 	bool "VESA VGA graphics support"
 	depends on (FB = y) && X86
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index bd8b052..b7da8c0 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -115,6 +115,7 @@ obj-$(CONFIG_FB_SM501)            += sm501fb.o
 obj-$(CONFIG_FB_XILINX)           += xilinxfb.o
 
 # Platform or fallback drivers go here
+obj-$(CONFIG_FB_UVESA)            += uvesafb.o
 obj-$(CONFIG_FB_VESA)             += vesafb.o
 obj-$(CONFIG_FB_IMAC)             += imacfb.o
 obj-$(CONFIG_FB_VGA16)            += vga16fb.o
diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c
new file mode 100644
index 0000000..020597f
--- /dev/null
+++ b/drivers/video/uvesafb.c
@@ -0,0 +1,1902 @@
+/*
+ * A framebuffer driver for VBE 2.0+ compliant video cards
+ *
+ * (c) 2007 Michal Januszewski <spock@gentoo.org>
+ *     Loosely based upon the vesafb driver.
+ *
+ */
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/skbuff.h>
+#include <linux/timer.h>
+#include <linux/completion.h>
+#include <linux/connector.h>
+#include <linux/random.h>
+#include <linux/platform_device.h>
+#include <linux/limits.h>
+#include <linux/fb.h>
+#include <video/edid.h>
+#include <video/vga.h>
+#include <video/uvesafb.h>
+#include <asm/io.h>
+#include <asm/mtrr.h>
+
+#include "edid.h"
+
+static struct cb_id uvesafb_cn_id = {
+	.idx = CN_IDX_V86D,
+	.val = CN_VAL_V86D_UVESAFB
+};
+static struct sock *nls;
+static char v86d_path[PATH_MAX] = "/sbin/v86d";
+static char v86d_started = 0;	/* has v86d been started by uvesafb? */
+
+static struct fb_fix_screeninfo uvesafb_fix __devinitdata = {
+	.id	= "VESA VGA",
+	.type	= FB_TYPE_PACKED_PIXELS,
+	.accel	= FB_ACCEL_NONE,
+};
+
+static int mtrr		__devinitdata = 3; /* enable mtrr by default */
+static int blank	__devinitdata = 1; /* enable blanking by default */
+static int ypan		__devinitdata = 1; /* 0: scroll, 1: ypan, 2: ywrap */
+static int pmi_setpal	__devinitdata = 1; /* use PMI for palette changes */
+static int nocrtc	__devinitdata = 0; /* ignore CRTC settings */
+static int noedid	__devinitdata = 0; /* don't try DDC transfers */
+static int vram_remap	__devinitdata = 0; /* set amt. of memory to be used */
+static int vram_total	__devinitdata = 0; /* set total amount of memory */
+static u16 maxclk	__devinitdata = 0; /* maximum pixel clock */
+static u16 maxvf	__devinitdata = 0; /* maximum vertical frequency */
+static u16 maxhf	__devinitdata = 0; /* maximum horizontal frequency */
+static u16 vbemode	__devinitdata = 0; /* force use of a specific VBE mode */
+static char *mode_option __devinitdata = NULL;
+
+static struct uvesafb_ktask* uvfb_tasks[UVESAFB_TASKS_MAX];
+
+static void uvesafb_cn_callback(void *data)
+{
+	struct cn_msg *msg = (struct cn_msg *)data;
+	struct uvesafb_task *utask = (struct uvesafb_task *)msg->data;
+	struct uvesafb_ktask *task;
+
+	if (msg->seq >= UVESAFB_TASKS_MAX)
+		return;
+
+	task = uvfb_tasks[msg->seq];
+
+	if (!task || msg->ack != task->ack)
+		return;
+
+	memcpy(&task->t, utask, sizeof(struct uvesafb_task));
+
+	if (task->t.buf_len && task->buf)
+		memcpy(task->buf, ((u8*)utask) + sizeof(struct uvesafb_task),
+						task->t.buf_len);
+
+	complete(task->done);
+	return;
+}
+
+static int uvesafb_helper_start(void)
+{
+	char *envp[] = {
+		"HOME=/",
+		"PATH=/sbin:/bin",
+		NULL,
+	};
+
+	char *argv[] = {
+		v86d_path,
+		NULL,
+	};
+
+	return call_usermodehelper(v86d_path, argv, envp, 1);
+}
+
+static struct uvesafb_ktask *uvesafb_prep(void)
+{
+	struct uvesafb_ktask *task;
+
+	task = kzalloc(sizeof(struct uvesafb_ktask), GFP_ATOMIC);
+	if (task) {
+		task->done = kzalloc(sizeof(struct completion), GFP_ATOMIC);
+		if (!task->done) {
+			kfree(task);
+			task = NULL;
+		}
+	}
+	return task;
+}
+
+/*
+ * Execute a uvesafb task.
+ *
+ * Returns 0 if the task is executed successfully.
+ *
+ * A message sent to the userspace consists of the uvesafb_task
+ * struct and (optionally) a buffer. The uvesafb_task struct is
+ * a simplified version of uvesafb_ktask (its kernel counterpart)
+ * containing only the register values, flags and the length of
+ * the buffer.
+ *
+ * Each message is assigned a sequence number (increased linearly)
+ * and a random ack number. The sequence number is used as a key
+ * for the uvfb_tasks array which holds pointers to uvesafb_ktask
+ * structs for all requests.
+ */
+static int uvesafb_exec(struct uvesafb_ktask *task)
+{
+	static int seq = 0;
+	struct cn_msg *m;
+	int err;
+	int len = sizeof(task->t) + task->t.buf_len;
+
+	/* Check whether the message isn't longer than the maximum
+	 * allowed by connector */
+	if (sizeof(*m) + len > CONNECTOR_MAX_MSG_SIZE) {
+		printk(KERN_WARNING "uvesafb: message too long (%d), "
+				"can't exec task\n", (int)(sizeof(*m) + len));
+		return -E2BIG;
+	}
+
+	m = kmalloc(sizeof(*m) + len, GFP_ATOMIC);
+	if (!m)
+		return -ENOMEM;
+
+	init_completion(task->done);
+
+	memset(m, 0, sizeof(*m) + len);
+	memcpy(&m->id, &uvesafb_cn_id, sizeof(m->id));
+	m->seq = seq;
+	m->len = len;
+	m->ack = random32();
+
+	/* uvesafb_task structure */
+	memcpy(m + 1, &task->t, sizeof(task->t));
+
+	/* Buffer */
+	memcpy((u8*)m + sizeof(task->t) + sizeof(*m),
+		task->buf, task->t.buf_len);
+
+	/* Save the message ack number so that we can find the kernel
+	 * part of this task when a reply is received from userspace. */
+	task->ack = m->ack;
+
+	/* If all slots are taken -- bail out. */
+	if (uvfb_tasks[seq])
+		return -EBUSY;
+
+	/* Save a pointer to the kernel part of the task struct. */
+	uvfb_tasks[seq] = task;
+
+	err = cn_netlink_send(m, 0, gfp_any());
+	if (err == -ESRCH) {
+		/* Try to start the userspace helper if sending
+		 * the request failed the first time. */
+		err = uvesafb_helper_start();
+		if (err) {
+			printk(KERN_ERR "uvesafb: failed to execute %s\n",
+					v86d_path);
+			printk(KERN_ERR "uvesafb: make sure that the v86d"
+					"helper is installed and executable\n");
+		} else {
+			v86d_started = 1;
+			err = cn_netlink_send(m, 0, gfp_any());
+		}
+	}
+	kfree(m);
+
+	if (!err && !(task->t.flags & TF_EXIT))
+		err = !wait_for_completion_timeout(task->done,
+				msecs_to_jiffies(UVESAFB_TIMEOUT));
+
+	uvfb_tasks[seq] = NULL;
+
+	seq++;
+	if (seq >= UVESAFB_TASKS_MAX)
+		seq = 0;
+
+	return err;
+}
+
+static void uvesafb_setup_var(struct fb_var_screeninfo *var,
+		struct fb_info *info, struct vbe_mode_ib *mode)
+{
+	struct uvesafb_par *par = info->par;
+
+	var->vmode = FB_VMODE_NONINTERLACED;
+	var->sync = FB_SYNC_VERT_HIGH_ACT;
+
+	var->xres = mode->x_res;
+	var->yres = mode->y_res;
+	var->xres_virtual = mode->x_res;
+	var->yres_virtual = (par->ypan) ?
+			      info->fix.smem_len / mode->bytes_per_scan_line :
+			      mode->y_res;
+	var->xoffset = 0;
+	var->yoffset = 0;
+	var->bits_per_pixel = mode->bits_per_pixel;
+
+	if (var->bits_per_pixel == 15)
+		var->bits_per_pixel = 16;
+
+	if (var->bits_per_pixel > 8) {
+		var->red.offset    = mode->red_off;
+		var->red.length    = mode->red_len;
+		var->green.offset  = mode->green_off;
+		var->green.length  = mode->green_len;
+		var->blue.offset   = mode->blue_off;
+		var->blue.length   = mode->blue_len;
+		var->transp.offset = mode->rsvd_off;
+		var->transp.length = mode->rsvd_len;
+	} else {
+		var->red.offset    = 0;
+		var->green.offset  = 0;
+		var->blue.offset   = 0;
+		var->transp.offset = 0;
+
+		/* We're assuming that we can switch the DAC to 8 bits. If
+		 * this proves to be incorrect, we'll update the fields
+		 * later in set_par(). */
+		if (par->vbe_ib.capabilities & VBE_CAP_CAN_SWITCH_DAC) {
+			var->red.length    = 8;
+			var->green.length  = 8;
+			var->blue.length   = 8;
+			var->transp.length = 0;
+		} else {
+			var->red.length    = 6;
+			var->green.length  = 6;
+			var->blue.length   = 6;
+			var->transp.length = 0;
+		}
+	}
+}
+
+static int uvesafb_vbe_find_mode(struct uvesafb_par *par,
+		int xres, int yres, int depth, unsigned char flags)
+{
+	int i, match = -1, h = 0, d = 0x7fffffff;
+
+	for (i = 0; i < par->vbe_modes_cnt; i++) {
+		h = abs(par->vbe_modes[i].x_res - xres) +
+		    abs(par->vbe_modes[i].y_res - yres) +
+		    abs(depth - par->vbe_modes[i].depth);
+
+		/* We have an exact match in terms of resolution
+		 * and depth. */
+		if (h == 0)
+			return i;
+
+		if (h < d || (h == d && par->vbe_modes[i].depth > depth)) {
+			d = h;
+			match = i;
+		}
+	}
+	i = 1;
+
+	if (flags & UVESAFB_NEED_EXACT_DEPTH &&
+			par->vbe_modes[match].depth != depth)
+		i = 0;
+
+	if (flags & UVESAFB_NEED_EXACT_RES && d > 24)
+		i = 0;
+
+	if (i != 0)
+		return match;
+	else
+		return -1;
+}
+
+static u8* uvesafb_vbe_state_save(struct uvesafb_par *par)
+{
+	struct uvesafb_ktask *task;
+	u8 *state;
+	int err;
+
+	if (!par->vbe_state_size)
+		return NULL;
+
+	state = kmalloc(par->vbe_state_size, GFP_KERNEL);
+	if (!state)
+		return NULL;
+
+	task = uvesafb_prep();
+	if (!task) {
+		kfree(state);
+		return NULL;
+	}
+
+	task->t.regs.eax = 0x4f04;
+	task->t.regs.ecx = 0x000f;
+	task->t.regs.edx = 0x0001;
+	task->t.flags = TF_BUF_RET | TF_BUF_ESBX;
+	task->t.buf_len = par->vbe_state_size;
+	task->buf = (void*)state;
+	err = uvesafb_exec(task);
+
+	if (err || (task->t.regs.eax & 0xffff) != 0x004f) {
+		printk(KERN_WARNING "uvesafb: VBE get state call "
+				"failed (eax=0x%x, err=%d)\n",
+				task->t.regs.eax, err);
+		kfree(state);
+		state = NULL;
+	}
+
+	uvesafb_free(task);
+	return state;
+}
+
+static void uvesafb_vbe_state_restore(struct uvesafb_par *par, u8 *state_buf)
+{
+	struct uvesafb_ktask *task;
+	int err;
+
+	if (!state_buf)
+		return;
+
+	task = uvesafb_prep();
+	if (!task)
+		return;
+
+	task->t.regs.eax = 0x4f04;
+	task->t.regs.ecx = 0x000f;
+	task->t.regs.edx = 0x0002;
+	task->t.buf_len = par->vbe_state_size;
+	task->t.flags = TF_BUF_ESBX;
+	task->buf = (void*)(state_buf);
+
+	err = uvesafb_exec(task);
+	if (err || (task->t.regs.eax & 0xffff) != 0x004f)
+		printk(KERN_WARNING "vesafb: VBE state restore call "
+				"failed (eax=0x%x, err=%d)\n",
+				task->t.regs.eax, err);
+
+	uvesafb_free(task);
+}
+
+static int __devinit uvesafb_vbe_getinfo(struct uvesafb_ktask *task,
+	struct uvesafb_par *par)
+{
+	int err;
+
+	task->t.regs.eax = 0x4f00;
+	task->t.flags = TF_VBEIB;
+	task->t.buf_len = sizeof(struct vbe_ib);
+	task->buf = (u8*) &par->vbe_ib;
+	strncpy(par->vbe_ib.vbe_signature, "VBE2", 4);
+
+	err = uvesafb_exec(task);
+	if (err || (task->t.regs.eax & 0xffff) != 0x004f) {
+		printk(KERN_ERR "uvesafb: Getting VBE info block failed "
+				"(eax=0x%x, err=%x)\n", (u32)task->t.regs.eax,
+				err);
+		return -EINVAL;
+	}
+
+	if (par->vbe_ib.vbe_version < 0x0200) {
+		printk(KERN_ERR "uvesafb: Sorry, pre-VBE 2.0 cards are "
+				"not supported.\n");
+		return -EINVAL;
+	}
+
+	if (!par->vbe_ib.mode_list_ptr) {
+	    printk(KERN_ERR "uvesafb: Missing mode list!\n");
+	    return -EINVAL;
+	}
+
+	printk(KERN_INFO "uvesafb: ");
+
+	/* Convert string pointers and the mode list pointer into
+	 * usable addresses. Print informational messages about the
+	 * video adapter and its vendor. */
+	if (par->vbe_ib.oem_vendor_name_ptr)
+		printk("%s, ",
+			(char*)(task->buf + par->vbe_ib.oem_vendor_name_ptr));
+
+	if (par->vbe_ib.oem_product_name_ptr)
+		printk("%s, ",
+			(char*)(task->buf + par->vbe_ib.oem_product_name_ptr));
+
+	if (par->vbe_ib.oem_product_rev_ptr)
+		printk("%s, ",
+			(char*)(task->buf + par->vbe_ib.oem_product_rev_ptr));
+
+	if (par->vbe_ib.oem_string_ptr)
+		printk("OEM: %s, ",
+			(char*)(task->buf + par->vbe_ib.oem_string_ptr));
+
+	printk("VBE v%d.%d\n", ((par->vbe_ib.vbe_version & 0xff00) >> 8),
+			par->vbe_ib.vbe_version & 0xff);
+
+	return 0;
+}
+
+static int __devinit uvesafb_vbe_getmodes(struct uvesafb_ktask *task,
+		struct uvesafb_par *par)
+{
+	int off = 0, err;
+	u16 *mode;
+
+	par->vbe_modes_cnt = 0;
+
+	/* Count available modes. */
+	mode = (u16*) (((u8*)&par->vbe_ib) + par->vbe_ib.mode_list_ptr);
+	while (*mode != 0xffff) {
+		par->vbe_modes_cnt++;
+		mode++;
+	}
+
+	par->vbe_modes = kzalloc(sizeof(struct vbe_mode_ib) *
+				par->vbe_modes_cnt, GFP_KERNEL);
+	if (!par->vbe_modes)
+		return -ENOMEM;
+
+	/* Get info about all available modes. */
+	mode = (u16*) (((u8*)&par->vbe_ib) + par->vbe_ib.mode_list_ptr);
+	while (*mode != 0xffff) {
+		struct vbe_mode_ib *mib;
+
+		uvesafb_reset(task);
+		task->t.regs.eax = 0x4f01;
+		task->t.regs.ecx = (u32) *mode;
+		task->t.flags = TF_BUF_RET | TF_BUF_ESDI;
+		task->t.buf_len = sizeof(struct vbe_mode_ib);
+		task->buf = (u8*) (par->vbe_modes + off);
+
+		err = uvesafb_exec(task);
+		if (err || (task->t.regs.eax & 0xffff) != 0x004f) {
+			printk(KERN_ERR "uvesafb: Getting mode info block "
+				"for mode 0x%x failed (eax=0x%x, err=%x)\n",
+				*mode, (u32)task->t.regs.eax, err);
+			return -EINVAL;
+		}
+
+		mib = (struct vbe_mode_ib*)task->buf;
+		mib->mode_id = *mode;
+
+		/* We only want modes that are supported with the current
+		 * hardware configuration, color, graphics and that have
+		 * support for the LFB. */
+		if ((mib->mode_attr & VBE_MODE_MASK) == VBE_MODE_MASK &&
+		     mib->bits_per_pixel >= 8) {
+			off++;
+		} else {
+			par->vbe_modes_cnt--;
+		}
+		mode++;
+		mib->depth = mib->red_len + mib->green_len + mib->blue_len;
+
+		/* Handle 8bpp modes and modes with broken color component
+		 * lengths. */
+		if (mib->depth == 0 || (mib->depth == 24 &&
+					mib->bits_per_pixel == 32))
+			mib->depth = mib->bits_per_pixel;
+	}
+
+	return 0;
+}
+
+#ifdef __i386__
+static int __devinit uvesafb_vbe_getpmi(struct uvesafb_ktask *task,
+		struct uvesafb_par *par)
+{
+	int i, err;
+
+	uvesafb_reset(task);
+	task->t.regs.eax = 0x4f0a;
+	task->t.regs.ebx = 0x0;
+	err = uvesafb_exec(task);
+
+	if ((task->t.regs.eax & 0xffff) != 0x4f || task->t.regs.es < 0xc000) {
+		par->pmi_setpal = par->ypan = 0;
+	} else {
+		par->pmi_base = (u16*)phys_to_virt(((u32)task->t.regs.es << 4)
+						+ task->t.regs.edi);
+		par->pmi_start = (void*)((char*)par->pmi_base +
+						par->pmi_base[1]);
+		par->pmi_pal = (void*)((char*)par->pmi_base +
+						par->pmi_base[2]);
+		printk(KERN_INFO "uvesafb: protected mode interface info at "
+				 "%04x:%04x\n",
+				 (u16)task->t.regs.es, (u16)task->t.regs.edi);
+		printk(KERN_INFO "uvesafb: pmi: set display start = %p, "
+				 "set palette = %p\n", par->pmi_start,
+				 par->pmi_pal);
+
+		if (par->pmi_base[3]) {
+			printk(KERN_INFO "uvesafb: pmi: ports = ");
+			for (i = par->pmi_base[3]/2;
+					par->pmi_base[i] != 0xffff; i++)
+				printk("%x ", par->pmi_base[i]);
+			printk("\n");
+
+			if (par->pmi_base[i] != 0xffff) {
+				printk(KERN_INFO "uvesafb: can't handle memory"
+						 " requests, pmi disabled\n");
+				par->ypan = par->pmi_setpal = 0;
+			}
+		}
+	}
+	return 0;
+}
+#endif
+
+/* Check whether a video mode is supported by the Video BIOS and is
+ * compatible with the monitor limits. */
+static int __devinit uvesafb_is_valid_mode(struct fb_videomode *mode,
+		struct fb_info *info)
+{
+	if (info->monspecs.gtf) {
+		fb_videomode_to_var(&info->var, mode);
+		if (fb_validate_mode(&info->var, info))
+			return -1;
+	}
+
+	if (uvesafb_vbe_find_mode(info->par, mode->xres, mode->yres, 8,
+				UVESAFB_NEED_EXACT_RES) == -1)
+		return -1;
+
+	return 0;
+}
+
+static int __devinit uvesafb_vbe_getedid(struct uvesafb_ktask *task,
+		struct fb_info *info)
+{
+	struct uvesafb_par *par = (struct uvesafb_par *)info->par;
+	int err = 0;
+
+	if (noedid || par->vbe_ib.vbe_version < 0x0300)
+		return -EINVAL;
+
+	task->t.regs.eax = 0x4f15;
+	task->t.regs.ebx = 0;
+	task->t.regs.ecx = 0;
+	task->t.buf_len = 0;
+	task->t.flags = 0;
+
+	err = uvesafb_exec(task);
+
+	if ((task->t.regs.eax & 0xffff) != 0x004f || err)
+		return -EINVAL;
+
+	if ((task->t.regs.ebx & 0x3) == 3) {
+		printk(KERN_INFO "uvesafb: VBIOS/hardware supports both "
+				 "DDC1 and DDC2 transfers\n");
+	} else if ((task->t.regs.ebx & 0x3) == 2) {
+		printk(KERN_INFO "uvesafb: VBIOS/hardware supports DDC2 "
+				 "transfers\n");
+	} else if ((task->t.regs.ebx & 0x3) == 1) {
+		printk(KERN_INFO "uvesafb: VBIOS/hardware supports DDC1 "
+				 "transfers\n");
+	} else {
+		printk(KERN_INFO "uvesafb: VBIOS/hardware doesn't support "
+				 "DDC transfers\n");
+		return -EINVAL;
+	}
+
+	task->t.regs.eax = 0x4f15;
+	task->t.regs.ebx = 1;
+	task->t.regs.ecx = task->t.regs.edx = 0;
+	task->t.flags = TF_BUF_RET | TF_BUF_ESDI;
+	task->t.buf_len = EDID_LENGTH;
+	task->buf = kzalloc(EDID_LENGTH, GFP_KERNEL);
+
+	err = uvesafb_exec(task);
+
+	if ((task->t.regs.eax & 0xffff) == 0x004f && !err) {
+		fb_edid_to_monspecs(task->buf, &info->monspecs);
+
+		if (info->monspecs.vfmax && info->monspecs.hfmax) {
+			/* If the maximum pixel clock wasn't specified in
+			 * the EDID block, set it to 300 MHz. */
+			if (info->monspecs.dclkmax == 0)
+				info->monspecs.dclkmax = 300 * 1000000;
+			info->monspecs.gtf = 1;
+		}
+	} else {
+		err = -EINVAL;
+	}
+
+	kfree(task->buf);
+	return err;
+}
+
+static void __devinit uvesafb_vbe_getmonspecs(struct uvesafb_ktask *task,
+		struct fb_info *info)
+{
+	struct uvesafb_par *par = info->par;
+	int i;
+	memset(&info->monspecs, 0, sizeof(struct fb_monspecs));
+
+	/* If we don't get all necessary data from the EDID block,
+	 * mark it as incompatible with the GTF. */
+	if (uvesafb_vbe_getedid(task, info))
+		info->monspecs.gtf = 0;
+
+	/* Kernel command line overrides. */
+	if (maxclk)
+		info->monspecs.dclkmax = maxclk * 1000000;
+	if (maxvf)
+		info->monspecs.vfmax = maxvf;
+	if (maxhf)
+		info->monspecs.hfmax = maxhf * 1000;
+
+	/* In case DDC transfers are not supported the user can provide
+	 * monitor limits manually. Lower limits are set to "safe" values. */
+	if (info->monspecs.gtf == 0 && maxclk && maxvf && maxhf) {
+		info->monspecs.dclkmin = 0;
+		info->monspecs.vfmin = 60;
+		info->monspecs.hfmin = 29000;
+		info->monspecs.gtf = 1;
+	}
+
+	if (info->monspecs.gtf)
+		printk(KERN_INFO
+			"uvesafb: monitor limits: vf = %d Hz, hf = %d kHz, "
+			"clk = %d MHz\n", info->monspecs.vfmax,
+			(int)(info->monspecs.hfmax / 1000),
+			(int)(info->monspecs.dclkmax / 1000000));
+	else
+		printk(KERN_INFO "uvesafb: no monitor limits have been set\n");
+
+	/* Add VBE modes to the modelist. */
+	for (i = 0; i < par->vbe_modes_cnt; i++) {
+		struct fb_var_screeninfo var;
+		struct vbe_mode_ib *mode;
+		struct fb_videomode vmode;
+
+		mode = &par->vbe_modes[i];
+		memset(&var, 0, sizeof(var));
+
+		var.xres = mode->x_res;
+		var.yres = mode->y_res;
+
+		fb_get_mode(FB_VSYNCTIMINGS | FB_IGNOREMON, 60, &var, info);
+		fb_var_to_videomode(&vmode, &var);
+		fb_add_videomode(&vmode, &info->modelist);
+	}
+
+	/* Add valid VESA modes to our modelist. */
+	for (i = 0; i < VESA_MODEDB_SIZE; i++) {
+		if (!uvesafb_is_valid_mode((struct fb_videomode*)
+						&vesa_modes[i], info))
+			fb_add_videomode(&vesa_modes[i], &info->modelist);
+	}
+
+	for (i = 0; i < info->monspecs.modedb_len; i++) {
+		if (!uvesafb_is_valid_mode(&info->monspecs.modedb[i], info))
+			fb_add_videomode(&info->monspecs.modedb[i],
+					&info->modelist);
+	}
+
+	return;
+}
+
+static void __devinit uvesafb_vbe_getstatesize(struct uvesafb_ktask *task,
+		struct uvesafb_par *par)
+{
+	int err;
+	uvesafb_reset(task);
+
+	/* Get the VBE state buffer size. We want all available
+	 * hardware state data (CL = 0x0f). */
+	task->t.regs.eax = 0x4f04;
+	task->t.regs.ecx = 0x000f;
+	task->t.regs.edx = 0x0000;
+	task->t.flags = 0;
+
+	err = uvesafb_exec(task);
+
+	if (err || (task->t.regs.eax & 0xffff) != 0x004f) {
+		printk(KERN_WARNING "uvesafb: VBE state buffer size "
+			"cannot be determined (eax=0x%x, err=%d)\n",
+			task->t.regs.eax, err);
+		par->vbe_state_size = 0;
+		return;
+	}
+
+	par->vbe_state_size = 64 * (task->t.regs.ebx & 0xffff);
+}
+
+static int __devinit uvesafb_vbe_init(struct fb_info *info)
+{
+	struct uvesafb_ktask *task = NULL;
+	struct uvesafb_par *par = info->par;
+	int err;
+
+	task = uvesafb_prep();
+	if (!task)
+		return -ENOMEM;
+
+	if ((err = uvesafb_vbe_getinfo(task, par)))
+		goto out;
+	if ((err = uvesafb_vbe_getmodes(task, par)))
+		goto out;
+	par->nocrtc = nocrtc;
+#ifdef __i386__
+	par->pmi_setpal = pmi_setpal;
+	par->ypan = ypan;
+
+	if (par->pmi_setpal || par->ypan)
+		uvesafb_vbe_getpmi(task, par);
+#else
+	/* The protected mode interface is not available on non-x86. */
+	par->pmi_setpal = par->ypan = 0;
+#endif
+
+	INIT_LIST_HEAD(&info->modelist);
+	uvesafb_vbe_getmonspecs(task, info);
+	uvesafb_vbe_getstatesize(task, par);
+
+out:	uvesafb_free(task);
+	return err;
+}
+
+static int __devinit uvesafb_vbe_init_mode(struct fb_info *info)
+{
+	struct list_head *pos;
+	struct fb_modelist *modelist;
+	struct fb_videomode *mode;
+	struct uvesafb_par *par = info->par;
+	int i, modeid;
+
+	/* Has the user requested a specific VESA mode? */
+	if (vbemode) {
+		for (i = 0; i < par->vbe_modes_cnt; i++) {
+			if (par->vbe_modes[i].mode_id == vbemode) {
+				fb_get_mode(FB_VSYNCTIMINGS | FB_IGNOREMON, 60,
+							&info->var, info);
+				/* With pixclock set to 0, the default BIOS
+				 * timings will be used in set_par(). */
+				info->var.pixclock = 0;
+				modeid = i;
+				goto gotmode;
+			}
+		}
+		printk(KERN_INFO "uvesafb: requested VBE mode 0x%x is "
+				 "unavailable\n", vbemode);
+		vbemode = 0;
+	}
+
+	i = 0;
+	list_for_each(pos, &info->modelist) {
+		i++;
+	}
+
+	mode = kzalloc(i * sizeof(*mode), GFP_KERNEL);
+	i = 0;
+	list_for_each(pos, &info->modelist) {
+		modelist = list_entry(pos, struct fb_modelist, list);
+		mode[i] = modelist->mode;
+		i++;
+	}
+
+	if (!mode_option)
+		mode_option = UVESAFB_DEFAULT_MODE;
+
+	i = fb_find_mode(&info->var, info, mode_option, mode, i,
+		NULL, 8);
+
+	kfree(mode);
+
+	/* fb_find_mode() failed */
+	if (i == 0 || i >= 3) {
+		info->var.xres = 640;
+		info->var.yres = 480;
+		mode = (struct fb_videomode*)
+				fb_find_best_mode(&info->var, &info->modelist);
+
+		if (mode) {
+			fb_videomode_to_var(&info->var, mode);
+		} else {
+			modeid = par->vbe_modes[0].mode_id;
+			fb_get_mode(FB_VSYNCTIMINGS | FB_IGNOREMON, 60,
+				    &info->var, info);
+			goto gotmode;
+		}
+	}
+
+	/* Look for a matching VBE mode. */
+	modeid = uvesafb_vbe_find_mode(par, info->var.xres, info->var.yres,
+			info->var.bits_per_pixel, UVESAFB_NEED_EXACT_RES);
+
+	if (modeid == -1)
+		return -EINVAL;
+
+gotmode:
+	uvesafb_setup_var(&info->var, info, &par->vbe_modes[modeid]);
+
+	/* If we are not VBE3.0+ compliant, we're done -- the BIOS will
+	 * ignore our mode timings anyway. */
+	if (par->vbe_ib.vbe_version < 0x0300)
+		fb_get_mode(FB_VSYNCTIMINGS | FB_IGNOREMON, 60,
+					&info->var, info);
+
+	return modeid;
+}
+
+static int uvesafb_setpalette(struct uvesafb_pal_entry *entries, int count,
+			     int start, struct fb_info *info)
+{
+	struct uvesafb_ktask *task;
+	struct uvesafb_par *par = info->par;
+	int i = par->mode_idx;
+	int err = 0;
+
+	/* We support palette modifications for 8 bpp modes only, so
+	 * there can never be more than 256 entries. */
+	if (start + count > 256)
+		return -EINVAL;
+
+	/* Use VGA registers if mode is VGA-compatible. */
+	if (i >= 0 && i < par->vbe_modes_cnt &&
+	    par->vbe_modes[i].mode_attr & VBE_MODE_VGACOMPAT) {
+		for (i = 0; i < count; i++) {
+			outb_p(start + i,        dac_reg);
+			outb_p(entries[i].red,   dac_val);
+			outb_p(entries[i].green, dac_val);
+			outb_p(entries[i].blue,  dac_val);
+		}
+	} else if (par->pmi_setpal) {
+		__asm__ __volatile__(
+		"call *(%%esi)"
+		: /* no return value */
+		: "a" (0x4f09),         /* EAX */
+		  "b" (0),              /* EBX */
+		  "c" (count),          /* ECX */
+		  "d" (start),          /* EDX */
+		  "D" (entries),        /* EDI */
+		  "S" (&par->pmi_pal)); /* ESI */
+	} else {
+		task = uvesafb_prep();
+		if (!task)
+			return -ENOMEM;
+
+		task->t.regs.eax = 0x4f09;
+		task->t.regs.ebx = 0x0;
+		task->t.regs.ecx = count;
+		task->t.regs.edx = start;
+		task->t.flags = TF_BUF_ESDI;
+		task->t.buf_len = sizeof(struct uvesafb_pal_entry) * count;
+		task->buf = (u8*) entries;
+
+		err = uvesafb_exec(task);
+		if ((task->t.regs.eax & 0xffff) != 0x004f)
+			err = 1;
+
+		uvesafb_free(task);
+	}
+	return err;
+}
+
+static int uvesafb_setcolreg(unsigned regno, unsigned red, unsigned green,
+			    unsigned blue, unsigned transp,
+			    struct fb_info *info)
+{
+	struct uvesafb_pal_entry entry;
+	int shift = 16 - info->var.green.length;
+	int err = 0;
+
+	if (regno >= info->cmap.len)
+		return -EINVAL;
+
+	if (info->var.bits_per_pixel == 8) {
+		entry.red   = red   >> shift;
+		entry.green = green >> shift;
+		entry.blue  = blue  >> shift;
+		entry.pad   = 0;
+
+		err = uvesafb_setpalette(&entry, 1, regno, info);
+	} else if (regno < 16) {
+		switch (info->var.bits_per_pixel) {
+		case 16:
+			if (info->var.red.offset == 10) {
+				/* 1:5:5:5 */
+				((u32*) (info->pseudo_palette))[regno] =
+						((red   & 0xf800) >>  1) |
+						((green & 0xf800) >>  6) |
+						((blue  & 0xf800) >> 11);
+			} else {
+				/* 0:5:6:5 */
+				((u32*) (info->pseudo_palette))[regno] =
+						((red   & 0xf800)      ) |
+						((green & 0xfc00) >>  5) |
+						((blue  & 0xf800) >> 11);
+			}
+			break;
+
+		case 24:
+		case 32:
+			red   >>= 8;
+			green >>= 8;
+			blue  >>= 8;
+			((u32 *)(info->pseudo_palette))[regno] =
+				(red   << info->var.red.offset)   |
+				(green << info->var.green.offset) |
+				(blue  << info->var.blue.offset);
+			break;
+		}
+	}
+	return err;
+}
+
+static int uvesafb_setcmap(struct fb_cmap *cmap, struct fb_info *info)
+{
+	struct uvesafb_pal_entry *entries;
+	int shift = 16 - info->var.green.length;
+	int i, err = 0;
+
+	if (info->var.bits_per_pixel == 8) {
+		if (cmap->start + cmap->len > info->cmap.start +
+		    info->cmap.len || cmap->start < info->cmap.start)
+			return -EINVAL;
+
+		entries = vmalloc(sizeof(struct uvesafb_pal_entry) * cmap->len);
+		if (!entries)
+			return -ENOMEM;
+		for (i = 0; i < cmap->len; i++) {
+			entries[i].red   = cmap->red[i]   >> shift;
+			entries[i].green = cmap->green[i] >> shift;
+			entries[i].blue  = cmap->blue[i]  >> shift;
+			entries[i].pad   = 0;
+		}
+		err = uvesafb_setpalette(entries, cmap->len, cmap->start, info);
+		vfree(entries);
+	} else {
+		/* For modes with bpp > 8, we only set the pseudo palette in
+		 * the fb_info struct. We rely on uvesafb_setcolreg to do all
+		 * sanity checking. */
+		for (i = 0; i < cmap->len; i++) {
+			err |= uvesafb_setcolreg(cmap->start + i, cmap->red[i],
+						cmap->green[i], cmap->blue[i],
+						0, info);
+		}
+	}
+	return err;
+}
+
+static int uvesafb_pan_display(struct fb_var_screeninfo *var,
+		struct fb_info *info)
+{
+	int offset;
+	struct uvesafb_par *par = info->par;
+
+	offset = (var->yoffset * info->fix.line_length + var->xoffset) / 4;
+
+	/* It turns out it's not the best idea to do panning via vm86,
+	 * so we only allow it if we have a PMI. */
+	if (par->pmi_start) {
+		__asm__ __volatile__(
+			"call *(%%edi)"
+			: /* no return value */
+			: "a" (0x4f07),         /* EAX */
+			  "b" (0),              /* EBX */
+			  "c" (offset),         /* ECX */
+			  "d" (offset >> 16),   /* EDX */
+			  "D" (&par->pmi_start));    /* EDI */
+	}
+	return 0;
+}
+
+static int uvesafb_blank(int blank, struct fb_info *info)
+{
+	struct uvesafb_par *par = info->par;
+	struct uvesafb_ktask *task;
+	int err = 1;
+
+	if (par->vbe_ib.capabilities & VBE_CAP_VGACOMPAT) {
+		int loop = 10000;
+		u8 seq = 0, crtc17 = 0;
+
+		if (blank == FB_BLANK_POWERDOWN) {
+			seq = 0x20;
+			crtc17 = 0x00;
+			err = 0;
+		} else {
+			seq = 0x00;
+			crtc17 = 0x80;
+			err = (blank == FB_BLANK_UNBLANK) ? 0 : -EINVAL;
+		}
+
+		vga_wseq(NULL, 0x00, 0x01);
+		seq |= vga_rseq(NULL, 0x01) & ~0x20;
+		vga_wseq(NULL, 0x00, seq);
+
+		crtc17 |= vga_rcrt(NULL, 0x17) & ~0x80;
+		while (loop--);
+		vga_wcrt(NULL, 0x17, crtc17);
+		vga_wseq(NULL, 0x00, 0x03);
+	} else {
+		task = uvesafb_prep();
+		if (!task)
+			return -ENOMEM;
+
+		task->t.regs.eax = 0x4f10;
+		switch (blank) {
+		case FB_BLANK_UNBLANK:
+			task->t.regs.ebx = 0x0001;
+			break;
+		case FB_BLANK_NORMAL:
+			task->t.regs.ebx = 0x0101;	/* standby */
+			break;
+		case FB_BLANK_POWERDOWN:
+			task->t.regs.ebx = 0x0401;	/* powerdown */
+			break;
+		default:
+			goto out;
+		}
+		task->t.flags = 0;
+
+		err = uvesafb_exec(task);
+		if (!err && (task->t.regs.eax & 0xffff) == 0x004f)
+			err = 0;
+out:		uvesafb_free(task);
+	}
+	return err;
+}
+
+static int uvesafb_open(struct fb_info *info, int user)
+{
+	struct uvesafb_par *par = info->par;
+	int cnt = atomic_read(&par->ref_count);
+
+	if (!cnt && par->vbe_state_size)
+		par->vbe_state_orig = uvesafb_vbe_state_save(par);
+
+	atomic_inc(&par->ref_count);
+	return 0;
+}
+
+static int uvesafb_release(struct fb_info *info, int user)
+{
+	struct uvesafb_ktask *task = NULL;
+	struct uvesafb_par *par = info->par;
+	int cnt = atomic_read(&par->ref_count);
+
+	if (!cnt)
+		return -EINVAL;
+
+	if (cnt != 1)
+		goto out;
+
+	task = uvesafb_prep();
+	if (!task)
+		goto out;
+
+	/* First, try to set the standard 80x25 text mode. */
+	task->t.regs.eax = 0x0003;
+	uvesafb_exec(task);
+
+	/* Now try to restore whatever hardware state we might have
+	 * saved when the fb device was first opened. */
+	uvesafb_vbe_state_restore(par, par->vbe_state_orig);
+out:
+	atomic_dec(&par->ref_count);
+	if (task)
+		uvesafb_free(task);
+	return 0;
+}
+
+static int uvesafb_set_par(struct fb_info *info)
+{
+	struct uvesafb_par *par = info->par;
+	struct uvesafb_ktask *task = NULL;
+	struct vbe_crtc_ib *crtc = NULL;
+	struct vbe_mode_ib *mode = NULL;
+	int i, err = 0, depth = info->var.bits_per_pixel;
+
+	if (depth > 8 && depth != 32)
+		depth = info->var.red.length + info->var.green.length +
+			info->var.blue.length;
+
+	i = uvesafb_vbe_find_mode(par, info->var.xres, info->var.yres, depth,
+				 UVESAFB_NEED_EXACT_RES |
+				 UVESAFB_NEED_EXACT_DEPTH);
+	if (i >= 0)
+		mode = &par->vbe_modes[i];
+	else
+		return -EINVAL;
+
+	task = uvesafb_prep();
+	if (!task)
+		return -ENOMEM;
+	task->t.regs.eax = 0x4f02;
+	task->t.regs.ebx = mode->mode_id | 0x4000;	/* use LFB */
+	task->t.flags = 0;
+
+	if (par->vbe_ib.vbe_version >= 0x0300 && !par->nocrtc &&
+	    info->var.pixclock != 0) {
+		task->t.regs.ebx |= 0x0800;		/* use CRTC data */
+		task->t.flags = TF_BUF_ESDI;
+		crtc = kzalloc(sizeof(struct vbe_crtc_ib), GFP_KERNEL);
+		if (!crtc) {
+			err = -ENOMEM;
+			goto out;
+		}
+		crtc->horiz_start = info->var.xres + info->var.right_margin;
+		crtc->horiz_end	  = crtc->horiz_start + info->var.hsync_len;
+		crtc->horiz_total = crtc->horiz_end + info->var.left_margin;
+
+		crtc->vert_start  = info->var.yres + info->var.lower_margin;
+		crtc->vert_end    = crtc->vert_start + info->var.vsync_len;
+		crtc->vert_total  = crtc->vert_end + info->var.upper_margin;
+
+		crtc->pixel_clock = PICOS2KHZ(info->var.pixclock) * 1000;
+		crtc->refresh_rate = (u16)(100 * (crtc->pixel_clock /
+				     (crtc->vert_total * crtc->horiz_total)));
+		crtc->flags = 0;
+
+		if (info->var.vmode & FB_VMODE_DOUBLE)
+			crtc->flags |= 0x1;
+		if (info->var.vmode & FB_VMODE_INTERLACED)
+			crtc->flags |= 0x2;
+		if (!(info->var.sync & FB_SYNC_HOR_HIGH_ACT))
+			crtc->flags |= 0x4;
+		if (!(info->var.sync & FB_SYNC_VERT_HIGH_ACT))
+			crtc->flags |= 0x8;
+		memcpy(&par->crtc, crtc, sizeof(struct vbe_crtc_ib));
+	} else {
+		memset(&par->crtc, 0, sizeof(struct vbe_crtc_ib));
+	}
+
+	task->t.buf_len = sizeof(struct vbe_crtc_ib);
+	task->buf = (u8*) &par->crtc;
+
+	err = uvesafb_exec(task);
+	if (err || (task->t.regs.eax & 0xffff) != 0x004f) {
+		printk(KERN_ERR "uvesafb: mode switch failed (eax=0x%x, "
+				"err=%x)\n", task->t.regs.eax, err);
+		err = -EINVAL;
+		goto out;
+	}
+	par->mode_idx = i;
+
+	/* For 8bpp modes, always try to set the DAC to 8 bits. */
+	if (par->vbe_ib.capabilities & VBE_CAP_CAN_SWITCH_DAC &&
+	    mode->bits_per_pixel <= 8) {
+		uvesafb_reset(task);
+		task->t.regs.eax = 0x4f08;
+		task->t.regs.ebx = 0x0800;
+
+		err = uvesafb_exec(task);
+		if (err || (task->t.regs.eax & 0xffff) != 0x004f ||
+		    ((task->t.regs.ebx & 0xff00) >> 8) != 8) {
+			/* We've failed to set the DAC palette format -
+			 * time to correct var. */
+			info->var.red.length    = 6;
+			info->var.green.length  = 6;
+			info->var.blue.length   = 6;
+		}
+	}
+
+	info->fix.visual = (info->var.bits_per_pixel == 8) ?
+		           FB_VISUAL_PSEUDOCOLOR : FB_VISUAL_TRUECOLOR;
+	info->fix.line_length = mode->bytes_per_scan_line;
+
+out:	if (crtc != NULL)
+		kfree(crtc);
+	uvesafb_free(task);
+
+	return err;
+}
+
+static void uvesafb_check_limits(struct fb_var_screeninfo *var,
+		struct fb_info *info)
+{
+	const struct fb_videomode *mode;
+	struct uvesafb_par *par = info->par;
+
+	/* If pixclock is set to 0, then we're using default BIOS timings
+	 * and thus don't have to perform any checks here. */
+	if (!var->pixclock)
+		return;
+
+	if (par->vbe_ib.vbe_version < 0x0300) {
+		fb_get_mode(FB_VSYNCTIMINGS | FB_IGNOREMON, 60, var, info);
+		return;
+	}
+
+	if (!fb_validate_mode(var, info))
+		return;
+
+	mode = fb_find_best_mode(var, &info->modelist);
+	if (mode) {
+		if (mode->xres == var->xres && mode->yres == var->yres &&
+		    !(mode->vmode & (FB_VMODE_INTERLACED | FB_VMODE_DOUBLE))) {
+			fb_videomode_to_var(var, mode);
+			return;
+		}
+	}
+
+	if (info->monspecs.gtf && !fb_get_mode(FB_MAXTIMINGS, 0, var, info))
+		return;
+	/* Use default refresh rate */
+	var->pixclock = 0;
+}
+
+static int uvesafb_check_var(struct fb_var_screeninfo *var,
+		struct fb_info *info)
+{
+	struct uvesafb_par *par = info->par;
+	struct vbe_mode_ib *mode = NULL;
+	int match = -1;
+	int depth = var->red.length + var->green.length + var->blue.length;
+
+	/* Various apps will use bits_per_pixel to set the color depth,
+	 * which is theoretically incorrect, but which we'll try to handle
+	 * here. */
+	if (depth == 0 || abs(depth - var->bits_per_pixel) >= 8)
+		depth = var->bits_per_pixel;
+
+	match = uvesafb_vbe_find_mode(par, var->xres, var->yres, depth,
+					UVESAFB_NEED_EXACT_RES);
+	if (match == -1)
+		return -EINVAL;
+
+	mode = &par->vbe_modes[match];
+	uvesafb_setup_var(var, info, mode);
+
+	/* Check whether we have remapped enough memory for this mode.
+	 * We might be called at an early stage, when we haven't remapped
+	 * any memory yet, in which case we simply skip the check. */
+	if (var->yres * mode->bytes_per_scan_line > info->fix.smem_len
+						&& info->fix.smem_len)
+		return -EINVAL;
+
+	if ((var->vmode & FB_VMODE_DOUBLE) &&
+				   !(par->vbe_modes[match].mode_attr & 0x100))
+		var->vmode &= ~FB_VMODE_DOUBLE;
+
+	if ((var->vmode & FB_VMODE_INTERLACED) &&
+				    !(par->vbe_modes[match].mode_attr & 0x200))
+		var->vmode &= ~FB_VMODE_INTERLACED;
+	uvesafb_check_limits(var, info);
+
+	var->xres_virtual = var->xres;
+	var->yres_virtual = (par->ypan) ?
+			      info->fix.smem_len / mode->bytes_per_scan_line :
+			      var->yres;
+	return 0;
+}
+
+static void uvesafb_save_state(struct fb_info *info)
+{
+	struct uvesafb_par *par = info->par;
+	if (par->vbe_state_saved)
+		kfree(par->vbe_state_saved);
+
+	par->vbe_state_saved = uvesafb_vbe_state_save(par);
+}
+
+static void uvesafb_restore_state(struct fb_info *info)
+{
+	struct uvesafb_par *par = info->par;
+	uvesafb_vbe_state_restore(par, par->vbe_state_saved);
+}
+
+static struct fb_ops uvesafb_ops = {
+	.owner		= THIS_MODULE,
+	.fb_open	= uvesafb_open,
+	.fb_release	= uvesafb_release,
+	.fb_setcolreg	= uvesafb_setcolreg,
+	.fb_setcmap	= uvesafb_setcmap,
+	.fb_pan_display	= uvesafb_pan_display,
+	.fb_blank       = uvesafb_blank,
+	.fb_fillrect	= cfb_fillrect,
+	.fb_copyarea	= cfb_copyarea,
+	.fb_imageblit	= cfb_imageblit,
+	.fb_check_var	= uvesafb_check_var,
+	.fb_set_par	= uvesafb_set_par,
+	.fb_save_state  = uvesafb_save_state,
+	.fb_restore_state = uvesafb_restore_state,
+};
+
+static void __devinit uvesafb_init_info(struct fb_info *info,
+		struct vbe_mode_ib *mode)
+{
+	unsigned int size_vmode;
+	unsigned int size_remap;
+	unsigned int size_total;
+	struct uvesafb_par *par = info->par;
+	int i, h;
+
+	info->pseudo_palette = ((u8*)info->par + sizeof(struct uvesafb_par));
+	info->fix = uvesafb_fix;
+	info->fix.ypanstep = par->ypan ? 1 : 0;
+	info->fix.ywrapstep = (par->ypan > 1) ? 1 : 0;
+
+	/* If we were unable to get the state buffer size, disable
+	 * functions for saving and restoring the hardware state. */
+	if (par->vbe_state_size == 0) {
+		info->fbops->fb_save_state = NULL;
+		info->fbops->fb_restore_state = NULL;
+	}
+
+	/* Disable blanking if the user requested so. */
+	if (!blank)
+		info->fbops->fb_blank = NULL;
+
+	/* Find out how much IO memory is required for the mode with
+	 * the highest resolution. */
+	size_remap = 0;
+	for (i = 0; i < par->vbe_modes_cnt; i++) {
+		h = par->vbe_modes[i].bytes_per_scan_line *
+					par->vbe_modes[i].y_res;
+		if (h > size_remap)
+			size_remap = h;
+	}
+	size_remap *= 2;
+
+	/*   size_vmode -- that is the amount of memory needed for the
+	 *                 used video mode, i.e. the minimum amount of
+	 *                 memory we need. */
+	if (mode != NULL) {
+		size_vmode = info->var.yres * mode->bytes_per_scan_line;
+	} else {
+		size_vmode = info->var.yres * info->var.xres *
+			     ((info->var.bits_per_pixel + 7) >> 3);
+	}
+
+	/*   size_total -- all video memory we have. Used for mtrr
+	 *                 entries, resource allocation and bounds
+	 *                 checking. */
+	size_total = par->vbe_ib.total_memory * 65536;
+	if (vram_total)
+		size_total = vram_total * 1024 * 1024;
+	if (size_total < size_vmode)
+		size_total = size_vmode;
+
+	/*   size_remap -- the amount of video memory we are going to
+	 *                 use for vesafb.  With modern cards it is no
+	 *                 option to simply use size_total as th
+	 *                 wastes plenty of kernel address space. */
+	if (vram_remap)
+		size_remap = vram_remap * 1024 * 1024;
+	if (size_remap < size_vmode)
+		size_remap = size_vmode;
+	if (size_remap > size_total)
+		size_remap = size_total;
+
+	info->fix.smem_len = size_remap;
+	info->fix.smem_start = mode->phys_base_ptr;
+
+	/* We have to set yres_virtual here because when setup_var() was
+	 * called, smem_len wasn't defined yet. */
+	info->var.yres_virtual = info->fix.smem_len /
+				 mode->bytes_per_scan_line;
+
+	if (par->ypan && info->var.yres_virtual > info->var.yres) {
+		printk(KERN_INFO "uvesafb: scrolling: %s "
+		       "using protected mode interface, "
+		       "yres_virtual=%d\n",
+		       (par->ypan > 1) ? "ywrap" : "ypan",
+		       info->var.yres_virtual);
+	} else {
+		printk(KERN_INFO "uvesafb: scrolling: redraw\n");
+		info->var.yres_virtual = info->var.yres;
+		par->ypan = 0;
+	}
+
+	info->flags = FBINFO_FLAG_DEFAULT |
+			(par->ypan) ? FBINFO_HWACCEL_YPAN : 0;
+
+	if (!par->ypan)
+		info->fbops->fb_pan_display = NULL;
+}
+
+static void uvesafb_init_mtrr(struct fb_info *info)
+{
+#ifdef CONFIG_MTRR
+	if (mtrr && !(info->fix.smem_start & (PAGE_SIZE - 1))) {
+		int temp_size = info->fix.smem_len;
+		unsigned int type = 0;
+
+		switch (mtrr) {
+		case 1:
+			type = MTRR_TYPE_UNCACHABLE;
+			break;
+		case 2:
+			type = MTRR_TYPE_WRBACK;
+			break;
+		case 3:
+			type = MTRR_TYPE_WRCOMB;
+			break;
+		case 4:
+			type = MTRR_TYPE_WRTHROUGH;
+			break;
+		default:
+			type = 0;
+			break;
+		}
+
+		if (type) {
+			int rc;
+
+			/* Find the largest power-of-two */
+			while (temp_size & (temp_size - 1))
+				temp_size &= (temp_size - 1);
+
+			/* Try and find a power of two to add */
+			do {
+				rc = mtrr_add(info->fix.smem_start,
+					      temp_size, type, 1);
+				temp_size >>= 1;
+			} while (temp_size >= PAGE_SIZE && rc == -EINVAL);
+		}
+	}
+#endif /* CONFIG_MTRR */
+}
+
+
+static ssize_t uvesafb_show_vbe_ver(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct fb_info *info = platform_get_drvdata(to_platform_device(dev));
+	struct uvesafb_par *par = info->par;
+	return snprintf(buf, PAGE_SIZE, "%.4x\n", par->vbe_ib.vbe_version);
+}
+
+static ssize_t uvesafb_show_vbe_modes(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct fb_info *info = platform_get_drvdata(to_platform_device(dev));
+	struct uvesafb_par *par = info->par;
+	int ret = 0, i;
+
+	for (i = 0; i < par->vbe_modes_cnt && ret < PAGE_SIZE; i++) {
+		ret += snprintf(buf + ret, PAGE_SIZE - ret,
+			"%dx%d-%d, 0x%.4x\n",
+			par->vbe_modes[i].x_res, par->vbe_modes[i].y_res,
+			par->vbe_modes[i].depth, par->vbe_modes[i].mode_id);
+	}
+
+	return ret;
+}
+
+static ssize_t uvesafb_show_vendor(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct fb_info *info = platform_get_drvdata(to_platform_device(dev));
+	struct uvesafb_par *par = info->par;
+	if (par->vbe_ib.oem_vendor_name_ptr)
+		return snprintf(buf, PAGE_SIZE, "%s\n", (char*)
+			(&par->vbe_ib) + par->vbe_ib.oem_vendor_name_ptr);
+	else
+		return 0;
+}
+
+static ssize_t uvesafb_show_product_name(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct fb_info *info = platform_get_drvdata(to_platform_device(dev));
+	struct uvesafb_par *par = info->par;
+	if (par->vbe_ib.oem_product_name_ptr)
+		return snprintf(buf, PAGE_SIZE, "%s\n", (char*)
+			(&par->vbe_ib) + par->vbe_ib.oem_product_name_ptr);
+	else
+		return 0;
+}
+
+static ssize_t uvesafb_show_product_rev(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct fb_info *info = platform_get_drvdata(to_platform_device(dev));
+	struct uvesafb_par *par = info->par;
+	if (par->vbe_ib.oem_product_rev_ptr)
+		return snprintf(buf, PAGE_SIZE, "%s\n", (char*)
+			(&par->vbe_ib) + par->vbe_ib.oem_product_rev_ptr);
+	else
+		return 0;
+}
+
+static ssize_t uvesafb_show_oem_string(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct fb_info *info = platform_get_drvdata(to_platform_device(dev));
+	struct uvesafb_par *par = info->par;
+	if (par->vbe_ib.oem_string_ptr)
+		return snprintf(buf, PAGE_SIZE, "%s\n",
+			(char*)(&par->vbe_ib) + par->vbe_ib.oem_string_ptr);
+	else
+		return 0;
+}
+
+static ssize_t uvesafb_show_nocrtc(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct fb_info *info = platform_get_drvdata(to_platform_device(dev));
+	struct uvesafb_par *par = info->par;
+	return snprintf(buf, PAGE_SIZE, "%d\n", par->nocrtc);
+}
+
+static ssize_t uvesafb_store_nocrtc(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct fb_info *info = platform_get_drvdata(to_platform_device(dev));
+	struct uvesafb_par *par = info->par;
+
+	if (count > 0) {
+		if (buf[0] == '0')
+			par->nocrtc = 0;
+		else
+			par->nocrtc = 1;
+	}
+	return count;
+}
+
+static struct device_attribute device_attrs[] = {
+	__ATTR(vbe_version, S_IRUGO, uvesafb_show_vbe_ver, NULL),
+	__ATTR(vbe_modes, S_IRUGO, uvesafb_show_vbe_modes, NULL),
+	__ATTR(oem_vendor, S_IRUGO, uvesafb_show_vendor, NULL),
+	__ATTR(oem_product_name, S_IRUGO, uvesafb_show_product_name, NULL),
+	__ATTR(oem_product_rev, S_IRUGO, uvesafb_show_product_rev, NULL),
+	__ATTR(oem_string, S_IRUGO, uvesafb_show_oem_string, NULL),
+	__ATTR(nocrtc, S_IRUGO | S_IWUSR, uvesafb_show_nocrtc,
+		uvesafb_store_nocrtc),
+};
+
+static void __devinit uvesafb_create_sysfs(struct device *dev,
+		struct fb_info *info)
+{
+	int i, err = 0;
+
+	for (i = 0; i < ARRAY_SIZE(device_attrs); i++) {
+		err |= device_create_file(dev, &device_attrs[i]);
+	}
+
+	if (err != 0)
+		printk(KERN_WARNING "fb%d: failed to register attributes\n",
+			info->node);
+}
+
+static void __devinit uvesafb_remove_sysfs(struct device *dev)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(device_attrs); i++) {
+		device_remove_file(dev, &device_attrs[i]);
+	}
+}
+
+static int __devinit uvesafb_probe(struct platform_device *dev)
+{
+	struct fb_info *info;
+	struct vbe_mode_ib *mode = NULL;
+	struct uvesafb_par *par;
+	int err = 0, i;
+
+	info = framebuffer_alloc(sizeof(*par) +	sizeof(u32) * 256, &dev->dev);
+	if (!info)
+		return -ENOMEM;
+
+	par = info->par;
+
+	if ((err = uvesafb_vbe_init(info))) {
+		printk(KERN_ERR "uvesafb: vbe_init() failed with %d\n", err);
+		goto out;
+	}
+
+	info->fbops = &uvesafb_ops;
+
+	i = uvesafb_vbe_init_mode(info);
+	if (i < 0) {
+		err = -EINVAL;
+		goto out;
+	} else {
+		mode = &par->vbe_modes[i];
+	}
+
+	if (fb_alloc_cmap(&info->cmap, 256, 0) < 0) {
+		err = -ENXIO;
+		goto out;
+	}
+
+	uvesafb_init_info(info, mode);
+
+	if (!request_mem_region(info->fix.smem_start, info->fix.smem_len,
+	    "uvesafb")) {
+		printk(KERN_WARNING "uvesafb: cannot reserve video memory at "
+		       "0x%lx\n", info->fix.smem_start);
+		/* We cannot make this fatal. Sometimes this comes from magic
+		   spaces our resource handlers simply don't know about. */
+	}
+
+	info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len);
+
+	if (!info->screen_base) {
+		printk(KERN_ERR
+		       "uvesafb: abort, cannot ioremap 0x%x bytes of video "
+		       "memory at 0x%lx\n",
+		       info->fix.smem_len, info->fix.smem_start);
+		err = -EIO;
+		goto out_mem;
+	}
+
+	if (!request_region(0x3c0, 32, "uvesafb")) {
+		printk(KERN_ERR "uvesafb: request region 0x3c0-0x3e0 failed\n");
+		err = -EIO;
+		goto out_unmap;
+	}
+
+	uvesafb_init_mtrr(info);
+	platform_set_drvdata(dev, info);
+
+	if (register_framebuffer(info) < 0) {
+		printk(KERN_ERR
+		       "uvesafb: failed to register framebuffer device\n");
+		err = -EINVAL;
+		goto out_reg;
+	}
+
+	printk(KERN_INFO "uvesafb: framebuffer at 0x%lx, mapped to 0x%p, "
+		       "using %dk, total %dk\n", info->fix.smem_start,
+		       info->screen_base, info->fix.smem_len/1024,
+		       par->vbe_ib.total_memory * 64);
+	printk(KERN_INFO "fb%d: %s frame buffer device\n", info->node,
+		       info->fix.id);
+
+	uvesafb_create_sysfs(&dev->dev, info);
+	return 0;
+
+out_reg:
+	release_region(0x3c0, 32);
+out_unmap:
+	iounmap(info->screen_base);
+out_mem:
+	release_mem_region(info->fix.smem_start, info->fix.smem_len);
+	if (!list_empty(&info->modelist))
+		fb_destroy_modelist(&info->modelist);
+	fb_destroy_modedb(info->monspecs.modedb);
+	fb_dealloc_cmap(&info->cmap);
+out:
+	if (par->vbe_modes)
+		kfree(par->vbe_modes);
+
+	framebuffer_release(info);
+	return err;
+}
+
+static int uvesafb_remove(struct platform_device *dev)
+{
+	struct fb_info *info = platform_get_drvdata(dev);
+
+	if (info) {
+		struct uvesafb_par *par = info->par;
+
+		uvesafb_remove_sysfs(&dev->dev);
+		unregister_framebuffer(info);
+		release_region(0x3c0, 32);
+		iounmap(info->screen_base);
+		release_mem_region(info->fix.smem_start, info->fix.smem_len);
+		fb_destroy_modedb(info->monspecs.modedb);
+		fb_dealloc_cmap(&info->cmap);
+
+		if (par) {
+			if (par->vbe_modes)
+				kfree(par->vbe_modes);
+			if (par->vbe_state_orig)
+				kfree(par->vbe_state_orig);
+			if (par->vbe_state_saved)
+				kfree(par->vbe_state_saved);
+		}
+
+		framebuffer_release(info);
+	}
+	return 0;
+}
+
+static struct platform_driver uvesafb_driver = {
+	.probe	= uvesafb_probe,
+	.remove = uvesafb_remove,
+	.driver	= {
+		.name	= "uvesafb",
+	},
+};
+
+static struct platform_device *uvesafb_device;
+
+#ifndef MODULE
+static int __devinit uvesafb_setup(char *options)
+{
+	char *this_opt;
+
+	if (!options || !*options)
+		return 0;
+
+	while ((this_opt = strsep(&options, ",")) != NULL) {
+		if (!*this_opt) continue;
+
+		if (!strcmp(this_opt, "redraw"))
+			ypan = 0;
+		else if (!strcmp(this_opt, "ypan"))
+			ypan = 1;
+		else if (!strcmp(this_opt, "ywrap"))
+			ypan = 2;
+		else if (!strcmp(this_opt, "vgapal"))
+			pmi_setpal = 0;
+		else if (!strcmp(this_opt, "pmipal"))
+			pmi_setpal = 1;
+		else if (!strncmp(this_opt, "mtrr:", 5))
+			mtrr = simple_strtoul(this_opt+5, NULL, 0);
+		else if (!strcmp(this_opt, "nomtrr"))
+			mtrr = 0;
+		else if (!strcmp(this_opt, "nocrtc"))
+			nocrtc = 1;
+		else if (!strcmp(this_opt, "noedid"))
+			noedid = 1;
+		else if (!strcmp(this_opt, "noblank"))
+			blank = 0;
+		else if (!strncmp(this_opt, "vtotal:", 7))
+			vram_total = simple_strtoul(this_opt + 7, NULL, 0);
+		else if (!strncmp(this_opt, "vremap:", 7))
+			vram_remap = simple_strtoul(this_opt + 7, NULL, 0);
+		else if (!strncmp(this_opt, "maxhf:", 6))
+			maxhf = simple_strtoul(this_opt + 6, NULL, 0);
+		else if (!strncmp(this_opt, "maxvf:", 6))
+			maxvf = simple_strtoul(this_opt + 6, NULL, 0);
+		else if (!strncmp(this_opt, "maxclk:", 7))
+			maxclk = simple_strtoul(this_opt + 7, NULL, 0);
+		else if (!strncmp(this_opt, "vbemode:", 8))
+			vbemode = simple_strtoul(this_opt + 8, NULL,0);
+		else if (this_opt[0] >= '0' && this_opt[0] <= '9') {
+			mode_option = this_opt;
+		} else {
+			printk(KERN_WARNING
+			       "uvesafb: unrecognized option %s\n", this_opt);
+		}
+	}
+
+	return 0;
+}
+#endif /* !MODULE */
+
+static ssize_t show_v86d(struct device_driver *dev, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%s\n", v86d_path);
+}
+
+static ssize_t store_v86d(struct device_driver *dev, const char *buf,
+		size_t count)
+{
+	strncpy(v86d_path, buf, PATH_MAX);
+	return count;
+}
+
+static DRIVER_ATTR(v86d, S_IRUGO | S_IWUSR, show_v86d, store_v86d);
+
+static int __devinit uvesafb_init(void)
+{
+	int err;
+
+#ifndef MODULE
+	char *option = NULL;
+
+	if (fb_get_options("uvesafb", &option))
+		return -ENODEV;
+	uvesafb_setup(option);
+#endif
+	err = cn_add_callback(&uvesafb_cn_id, "uvesafb", uvesafb_cn_callback);
+	if (err)
+		goto err_out;
+
+	err = platform_driver_register(&uvesafb_driver);
+
+	if (!err) {
+		uvesafb_device = platform_device_alloc("uvesafb", 0);
+		if (uvesafb_device)
+			err = platform_device_add(uvesafb_device);
+		else
+			err = -ENOMEM;
+
+		if (err) {
+			platform_device_put(uvesafb_device);
+			platform_driver_unregister(&uvesafb_driver);
+		}
+
+		err = driver_create_file(&uvesafb_driver.driver,
+				&driver_attr_v86d);
+		if (err) {
+			printk(KERN_WARNING "uvesafb: failed to register "
+					"attributes\n");
+			err = 0;
+		}
+	}
+	return err;
+
+err_out:
+	if (nls && nls->sk_socket)
+		sock_release(nls->sk_socket);
+	return err;
+}
+
+module_init(uvesafb_init);
+
+#ifdef MODULE
+static void __devexit uvesafb_exit(void)
+{
+	struct uvesafb_ktask *task;
+
+	if (v86d_started) {
+		task = uvesafb_prep();
+		if (task) {
+			task->t.flags = TF_EXIT;
+			uvesafb_exec(task);
+			uvesafb_free(task);
+		}
+	}
+
+	cn_del_callback(&uvesafb_cn_id);
+
+	driver_remove_file(&uvesafb_driver.driver, &driver_attr_v86d);
+	platform_device_unregister(uvesafb_device);
+	platform_driver_unregister(&uvesafb_driver);
+
+	if (nls && nls->sk_socket)
+		sock_release(nls->sk_socket);
+}
+
+module_exit(uvesafb_exit);
+
+static inline int param_get_scroll(char *buffer, struct kernel_param *kp)
+{
+	return 0;
+}
+
+static inline int param_set_scroll(const char *val, struct kernel_param *kp)
+{
+	ypan = 0;
+
+	if (!strcmp(val, "redraw"))
+		ypan = 0;
+	else if (!strcmp(val, "ypan"))
+		ypan = 1;
+	else if (!strcmp(val, "ywrap"))
+		ypan = 2;
+
+	return 0;
+}
+
+#define param_check_scroll(name, p) __param_check(name, p, void);
+
+module_param_named(scroll, ypan, scroll, 0);
+MODULE_PARM_DESC(scroll, "Scrolling mode, set to 'redraw', "
+	"'ypan' or 'ywrap'");
+module_param_named(vgapal, pmi_setpal, invbool, 0);
+MODULE_PARM_DESC(vgapal, "Set palette using VGA registers");
+module_param_named(pmipal, pmi_setpal, bool, 0);
+MODULE_PARM_DESC(pmipal, "Set palette using PMI calls");
+module_param(mtrr, uint, 0);
+MODULE_PARM_DESC(mtrr, "Memory Type Range Registers setting. "
+	"Use 0 to disable.");
+module_param(blank, bool, 1);
+MODULE_PARM_DESC(blank,"Enable hardware blanking");
+module_param(nocrtc, bool, 0);
+MODULE_PARM_DESC(nocrtc, "Ignore CRTC timings when setting modes");
+module_param(noedid, bool, 0);
+MODULE_PARM_DESC(noedid, "Ignore EDID-provided monitor limits "
+	"when setting modes");
+module_param(vram_remap, uint, 0);
+MODULE_PARM_DESC(vram_remap,"Set amount of video memory to be used [MiB]");
+module_param(vram_total, uint, 0);
+MODULE_PARM_DESC(vram_total,"Set total amount of video memoery [MiB]");
+module_param(maxclk, ushort, 0);
+MODULE_PARM_DESC(maxclk,"Maximum pixelclock [MHz], overrides EDID data");
+module_param(maxhf, ushort, 0);
+MODULE_PARM_DESC(maxhf, "Maximum horizontal frequency [kHz], "
+	"overrides EDID data");
+module_param(maxvf, ushort, 0);
+MODULE_PARM_DESC(maxvf, "Maximum vertical frequency [Hz], "
+	"overrides EDID data");
+module_param_named(mode, mode_option, charp, 0);
+MODULE_PARM_DESC(mode, "Specify initial video mode as "
+	"\"<xres>x<yres>[-<bpp>][@<refresh>]\"");
+module_param(vbemode, ushort, 0);
+MODULE_PARM_DESC(vbemode, "VBE mode number to set, "
+	"overrides 'mode' setting");
+#endif
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Michal Januszewski <spock@gentoo.org>");
+MODULE_DESCRIPTION("Framebuffer driver for VBE2.0+ compliant graphics boards");
+
diff --git a/include/video/Kbuild b/include/video/Kbuild
index a14f9c0..53a6c73 100644
--- a/include/video/Kbuild
+++ b/include/video/Kbuild
@@ -1 +1 @@
-unifdef-y += sisfb.h
+unifdef-y += sisfb.h uvesafb.h
diff --git a/include/video/uvesafb.h b/include/video/uvesafb.h
new file mode 100644
index 0000000..4fef5fc
--- /dev/null
+++ b/include/video/uvesafb.h
@@ -0,0 +1,208 @@
+#ifndef _UVESAFB_H
+#define _UVESAFB_H
+
+struct v86_regs {
+	__u32 ebx;
+	__u32 ecx;
+	__u32 edx;
+	__u32 esi;
+	__u32 edi;
+	__u32 ebp;
+	__u32 eax;
+	__u32 eip;
+	__u32 eflags;
+	__u32 esp;
+	__u16 cs;
+	__u16 ss;
+	__u16 es;
+	__u16 ds;
+	__u16 fs;
+	__u16 gs;
+};
+
+/* Task flags */
+#define TF_VBEIB    0x01
+#define TF_BUF_ESDI 0x02
+#define TF_BUF_ESBX 0x04
+#define TF_BUF_RET  0x08
+#define TF_EXIT		0x10
+
+struct uvesafb_task {
+	__u8 flags;
+	int buf_len;
+	struct v86_regs regs;
+};
+
+/* Constants for the capabilities field
+ * in vbe_ib */
+#define VBE_CAP_CAN_SWITCH_DAC	0x01
+#define VBE_CAP_VGACOMPAT	0x02
+
+/* The VBE Info Block */
+struct vbe_ib {
+	char  vbe_signature[4];
+	__u16 vbe_version;
+	__u32 oem_string_ptr;
+	__u32 capabilities;
+	__u32 mode_list_ptr;
+	__u16 total_memory;
+	__u16 oem_software_rev;
+	__u32 oem_vendor_name_ptr;
+	__u32 oem_product_name_ptr;
+	__u32 oem_product_rev_ptr;
+	__u8  reserved[222];
+	char  oem_data[256];
+	char  misc_data[512];
+} __attribute__ ((packed));
+
+#ifdef __KERNEL__
+
+/* VBE CRTC Info Block */
+struct vbe_crtc_ib {
+	u16 horiz_total;
+	u16 horiz_start;
+	u16 horiz_end;
+	u16 vert_total;
+	u16 vert_start;
+	u16 vert_end;
+	u8  flags;
+	u32 pixel_clock;
+	u16 refresh_rate;
+	u8  reserved[40];
+} __attribute__ ((packed));
+
+#define VBE_MODE_VGACOMPAT	0x20
+#define VBE_MODE_COLOR		0x08
+#define VBE_MODE_SUPPORTEDHW	0x01
+#define VBE_MODE_GRAPHICS	0x10
+#define VBE_MODE_LFB		0x80
+
+#define VBE_MODE_MASK		(VBE_MODE_COLOR | VBE_MODE_SUPPORTEDHW | \
+				VBE_MODE_GRAPHICS | VBE_MODE_LFB)
+
+/* VBE Mode Info Block */
+struct vbe_mode_ib {
+	/* for all VBE revisions */
+	u16 mode_attr;
+	u8  winA_attr;
+	u8  winB_attr;
+	u16 win_granularity;
+	u16 win_size;
+	u16 winA_seg;
+	u16 winB_seg;
+	u32 win_func_ptr;
+	u16 bytes_per_scan_line;
+
+	/* for VBE 1.2+ */
+	u16 x_res;
+	u16 y_res;
+	u8  x_char_size;
+	u8  y_char_size;
+	u8  planes;
+	u8  bits_per_pixel;
+	u8  banks;
+	u8  memory_model;
+	u8  bank_size;
+	u8  image_pages;
+	u8  reserved1;
+
+	/* Direct color fields for direct/6 and YUV/7 memory models. */
+	/* Offsets are bit positions of lsb in the mask. */
+	u8  red_len;
+	u8  red_off;
+	u8  green_len;
+	u8  green_off;
+	u8  blue_len;
+	u8  blue_off;
+	u8  rsvd_len;
+	u8  rsvd_off;
+	u8  direct_color_info;	/* direct color mode attributes */
+
+	/* for VBE 2.0+ */
+	u32 phys_base_ptr;
+	u8  reserved2[6];
+
+	/* for VBE 3.0+ */
+	u16 lin_bytes_per_scan_line;
+	u8  bnk_image_pages;
+	u8  lin_image_pages;
+	u8  lin_red_len;
+	u8  lin_red_off;
+	u8  lin_green_len;
+	u8  lin_green_off;
+	u8  lin_blue_len;
+	u8  lin_blue_off;
+	u8  lin_rsvd_len;
+	u8  lin_rsvd_off;
+	u32 max_pixel_clock;
+	u16 mode_id;
+	u8  depth;
+} __attribute__ ((packed));
+
+#define UVESAFB_DEFAULT_MODE "640x480-16"
+
+/* How long to wait for a reply from userspace [ms] */
+#define UVESAFB_TIMEOUT 5000
+
+/* Max number of concurrent tasks */
+#define UVESAFB_TASKS_MAX 16
+
+#define dac_reg	(0x3c8)
+#define dac_val	(0x3c9)
+
+struct uvesafb_pal_entry {
+	u_char blue, green, red, pad;
+} __attribute__ ((packed));
+
+struct uvesafb_ktask {
+	struct uvesafb_task t;
+	u8 *buf;
+	struct completion *done;
+	u32 ack;
+};
+
+#define uvesafb_free(task)					\
+do {								\
+	if (task) {						\
+		if (task->done)					\
+			kfree(task->done);			\
+		kfree(task);					\
+		task = NULL;					\
+	}							\
+} while(0)
+
+#define uvesafb_reset(task)					\
+do {								\
+	struct completion *cpl = task->done;			\
+	memset(task, 0, sizeof(struct uvesafb_ktask));		\
+	task->done = cpl;					\
+} while(0)							\
+
+static int uvesafb_exec(struct uvesafb_ktask *tsk);
+
+#define UVESAFB_NEED_EXACT_RES		1
+#define UVESAFB_NEED_EXACT_DEPTH	2
+
+struct uvesafb_par {
+	struct vbe_ib vbe_ib;		/* VBE Info Block */
+	struct vbe_mode_ib *vbe_modes;	/* list of supported VBE modes */
+	int vbe_modes_cnt;
+
+	u8 nocrtc;
+	u8 ypan;			/* 0 - nothing, 1 - ypan, 2 - ywrap */
+	u8 pmi_setpal;			/* pmi for palette changes */
+	u16  *pmi_base;			/* protected mode interface location */
+	void (*pmi_start)(void);
+	void (*pmi_pal)(void);
+
+	u8 *vbe_state_orig;		/* original hardware state, before the driver was loaded */
+	u8 *vbe_state_saved;		/* state saved by fb_save_state */
+	int vbe_state_size;
+	atomic_t ref_count;
+
+	int mode_idx;
+	struct vbe_crtc_ib crtc;
+};
+
+#endif
+#endif /* _UVESAFB_H */


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

* Re: [Linux-fbdev-devel] [PATCH 3/4] fbdev: uvesafb driver
  2007-06-23 10:52 [PATCH 3/4] fbdev: uvesafb driver Michal Januszewski
@ 2007-06-23 11:51 ` Bernhard Fischer
  2007-06-23 18:35 ` Andrew Morton
  2007-06-24  3:50 ` Akinobu Mita
  2 siblings, 0 replies; 7+ messages in thread
From: Bernhard Fischer @ 2007-06-23 11:51 UTC (permalink / raw)
  To: spock, linux-fbdev-devel; +Cc: linux-kernel

On Sat, Jun 23, 2007 at 12:52:43PM +0200, Michal Januszewski wrote:

>+static void uvesafb_cn_callback(void *data)
>+{
>+	struct cn_msg *msg = (struct cn_msg *)data;
>+	struct uvesafb_task *utask = (struct uvesafb_task *)msg->data;
>+	struct uvesafb_ktask *task;
>+
>+	if (msg->seq >= UVESAFB_TASKS_MAX)
>+		return;
>+
>+	task = uvfb_tasks[msg->seq];
>+
>+	if (!task || msg->ack != task->ack)
>+		return;

You need utask only here, so could spare setting it before the
checking above.
>+
>+	memcpy(&task->t, utask, sizeof(struct uvesafb_task));
>+
>+	if (task->t.buf_len && task->buf)
>+		memcpy(task->buf, ((u8*)utask) + sizeof(struct uvesafb_task),
>+						task->t.buf_len);
>+
>+	complete(task->done);
>+	return;
>+}

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

* Re: [PATCH 3/4] fbdev: uvesafb driver
  2007-06-23 10:52 [PATCH 3/4] fbdev: uvesafb driver Michal Januszewski
  2007-06-23 11:51 ` [Linux-fbdev-devel] " Bernhard Fischer
@ 2007-06-23 18:35 ` Andrew Morton
  2007-06-23 23:20   ` Michal Januszewski
  2007-06-24  3:50 ` Akinobu Mita
  2 siblings, 1 reply; 7+ messages in thread
From: Andrew Morton @ 2007-06-23 18:35 UTC (permalink / raw)
  To: spock; +Cc: linux-fbdev-devel, linux-kernel

On Sat, 23 Jun 2007 12:52:43 +0200 Michal Januszewski <spock@gentoo.org> wrote:

> Add the uvesafb driver, an enhanced version of vesafb that makes use of
> a userspace helper to run x86 Video BIOS code.
> 
> Signed-off-by: Michal Januszewski <spock@gentoo.org>
> ---
>  drivers/video/Kconfig   |   18 +
>  drivers/video/Makefile  |    1 +
>  drivers/video/uvesafb.c | 1902 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/video/Kbuild    |    2 +-
>  include/video/uvesafb.h |  208 ++++++
>  5 files changed, 2130 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 403dac7..5cc03f9 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -585,6 +585,24 @@ config FB_TGA
>  
>  	  Say Y if you have one of those.
>  
> +config FB_UVESA
> +	tristate "Userspace VESA VGA graphics support"
> +	depends on FB && CONNECTOR

These dependencies are insufficient.

> ...
>
> --- /dev/null
> +++ b/drivers/video/uvesafb.c
> @@ -0,0 +1,1902 @@
> +/*
> + * A framebuffer driver for VBE 2.0+ compliant video cards
> + *
> + * (c) 2007 Michal Januszewski <spock@gentoo.org>
> + *     Loosely based upon the vesafb driver.
> + *
> + */
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/skbuff.h>
> +#include <linux/timer.h>
> +#include <linux/completion.h>
> +#include <linux/connector.h>
> +#include <linux/random.h>
> +#include <linux/platform_device.h>
> +#include <linux/limits.h>
> +#include <linux/fb.h>
> +#include <video/edid.h>
> +#include <video/vga.h>
> +#include <video/uvesafb.h>
> +#include <asm/io.h>
> +#include <asm/mtrr.h>

Only x86 has mtrr.h: you just broke allmodconfig on all other
architectures.

> +#include "edid.h"
> +
> +static struct cb_id uvesafb_cn_id = {
> +	.idx = CN_IDX_V86D,
> +	.val = CN_VAL_V86D_UVESAFB
> +};
> +static struct sock *nls;
> +static char v86d_path[PATH_MAX] = "/sbin/v86d";

Remove the PATH_MAX, save some memory.

Oh, it gets set via sysfs.  hrm.


> +static char v86d_started = 0;	/* has v86d been started by uvesafb? */

Unneeded initialisation-to-zero.

scripts/checkpatch.pl would have reported this.  It in fact generates 93
warnings against this patch and from a quick scan, they all look
legitimate.

> +static void uvesafb_cn_callback(void *data)
> +{
> +	struct cn_msg *msg = (struct cn_msg *)data;

unneeded cast of void*

> +	struct uvesafb_task *utask = (struct uvesafb_task *)msg->data;
> +	struct uvesafb_ktask *task;
> +
> +	if (msg->seq >= UVESAFB_TASKS_MAX)
> +		return;
> +
> +	task = uvfb_tasks[msg->seq];
> +
> +	if (!task || msg->ack != task->ack)
> +		return;
> +
> +	memcpy(&task->t, utask, sizeof(struct uvesafb_task));
> +
> +	if (task->t.buf_len && task->buf)
> +		memcpy(task->buf, ((u8*)utask) + sizeof(struct uvesafb_task),
> +						task->t.buf_len);

Isn't this

	memcpy(task->buf, utask + 1, task->t.buf_len);
?

> +
> +	complete(task->done);
> +	return;
> +}
> +
> +static int uvesafb_helper_start(void)
> +{
> +	char *envp[] = {
> +		"HOME=/",
> +		"PATH=/sbin:/bin",
> +		NULL,
> +	};
> +
> +	char *argv[] = {
> +		v86d_path,
> +		NULL,
> +	};
> +
> +	return call_usermodehelper(v86d_path, argv, envp, 1);
> +}

Now I'm wondering what this code actually does.  It would be nice to have
an overall design and implementation description in the changelog so we
don't have to reverse-engineer your thoughts from your implementation...

The comment over uvesafb_exec() helps.

> +static struct uvesafb_ktask *uvesafb_prep(void)
> +{
> +	struct uvesafb_ktask *task;
> +
> +	task = kzalloc(sizeof(struct uvesafb_ktask), GFP_ATOMIC);
> +	if (task) {
> +		task->done = kzalloc(sizeof(struct completion), GFP_ATOMIC);
> +		if (!task->done) {
> +			kfree(task);
> +			task = NULL;
> +		}
> +	}
> +	return task;
> +}

GFP_ATOMIC is unreliable and should be strenuously avoided.  I suspect all
callers can use GFP_KERNEL here.  If not, we should at least pass the gfp_t
into this function as an argument so that we can use GFP_KERNEL from as
many callsites as possible.

> +/*
> + * Execute a uvesafb task.
> + *
> + * Returns 0 if the task is executed successfully.
> + *
> + * A message sent to the userspace consists of the uvesafb_task
> + * struct and (optionally) a buffer. The uvesafb_task struct is
> + * a simplified version of uvesafb_ktask (its kernel counterpart)
> + * containing only the register values, flags and the length of
> + * the buffer.
> + *
> + * Each message is assigned a sequence number (increased linearly)
> + * and a random ack number. The sequence number is used as a key
> + * for the uvfb_tasks array which holds pointers to uvesafb_ktask
> + * structs for all requests.
> + */
> +static int uvesafb_exec(struct uvesafb_ktask *task)
> +{
> +	static int seq = 0;
> +	struct cn_msg *m;
> +	int err;
> +	int len = sizeof(task->t) + task->t.buf_len;
> +
> +	/* Check whether the message isn't longer than the maximum
> +	 * allowed by connector */
> +	if (sizeof(*m) + len > CONNECTOR_MAX_MSG_SIZE) {
> +		printk(KERN_WARNING "uvesafb: message too long (%d), "
> +				"can't exec task\n", (int)(sizeof(*m) + len));
> +		return -E2BIG;
> +	}
> +
> +	m = kmalloc(sizeof(*m) + len, GFP_ATOMIC);

More GFP_ATOMIC badness

> +	if (!m)
> +		return -ENOMEM;
> +
> +	init_completion(task->done);
> +
> +	memset(m, 0, sizeof(*m) + len);

kzalloc()

> +	memcpy(&m->id, &uvesafb_cn_id, sizeof(m->id));
> +	m->seq = seq;
> +	m->len = len;
> +	m->ack = random32();
> +
> +	/* uvesafb_task structure */
> +	memcpy(m + 1, &task->t, sizeof(task->t));
> +
> +	/* Buffer */
> +	memcpy((u8*)m + sizeof(task->t) + sizeof(*m),
> +		task->buf, task->t.buf_len);
> +
> +	/* Save the message ack number so that we can find the kernel
> +	 * part of this task when a reply is received from userspace. */
> +	task->ack = m->ack;
> +
> +	/* If all slots are taken -- bail out. */
> +	if (uvfb_tasks[seq])
> +		return -EBUSY;
> +
> +	/* Save a pointer to the kernel part of the task struct. */
> +	uvfb_tasks[seq] = task;
> +
> +	err = cn_netlink_send(m, 0, gfp_any());
> +	if (err == -ESRCH) {
> +		/* Try to start the userspace helper if sending
> +		 * the request failed the first time. */
> +		err = uvesafb_helper_start();
> +		if (err) {
> +			printk(KERN_ERR "uvesafb: failed to execute %s\n",
> +					v86d_path);
> +			printk(KERN_ERR "uvesafb: make sure that the v86d"
> +					"helper is installed and executable\n");
> +		} else {
> +			v86d_started = 1;
> +			err = cn_netlink_send(m, 0, gfp_any());
> +		}
> +	}
> +	kfree(m);
> +
> +	if (!err && !(task->t.flags & TF_EXIT))
> +		err = !wait_for_completion_timeout(task->done,
> +				msecs_to_jiffies(UVESAFB_TIMEOUT));

If you can run wait_for_completion() here then you can use GFP_KERNEL up
there.

> +	uvfb_tasks[seq] = NULL;
> +
> +	seq++;
> +	if (seq >= UVESAFB_TASKS_MAX)
> +		seq = 0;
> +
> +	return err;
> +}
>
> ...
>
> +static int __devinit uvesafb_vbe_getinfo(struct uvesafb_ktask *task,
> +	struct uvesafb_par *par)
> +{
> +	int err;
> +
> +	task->t.regs.eax = 0x4f00;
> +	task->t.flags = TF_VBEIB;
> +	task->t.buf_len = sizeof(struct vbe_ib);
> +	task->buf = (u8*) &par->vbe_ib;
> +	strncpy(par->vbe_ib.vbe_signature, "VBE2", 4);
> +
> +	err = uvesafb_exec(task);
> +	if (err || (task->t.regs.eax & 0xffff) != 0x004f) {
> +		printk(KERN_ERR "uvesafb: Getting VBE info block failed "
> +				"(eax=0x%x, err=%x)\n", (u32)task->t.regs.eax,
> +				err);
> +		return -EINVAL;
> +	}
> +
> +	if (par->vbe_ib.vbe_version < 0x0200) {
> +		printk(KERN_ERR "uvesafb: Sorry, pre-VBE 2.0 cards are "
> +				"not supported.\n");
> +		return -EINVAL;
> +	}
> +
> +	if (!par->vbe_ib.mode_list_ptr) {
> +	    printk(KERN_ERR "uvesafb: Missing mode list!\n");
> +	    return -EINVAL;

whitespace went wonky here.

> +	}
> +
> +	printk(KERN_INFO "uvesafb: ");
> +
> +	/* Convert string pointers and the mode list pointer into
> +	 * usable addresses. Print informational messages about the
> +	 * video adapter and its vendor. */

Commenting style is

	/* Convert string pointers and the mode list pointer into
	 * usable addresses. Print informational messages about the
	 * video adapter and its vendor.
	 */

or

	/*
	 * Convert string pointers and the mode list pointer into
	 * usable addresses. Print informational messages about the
	 * video adapter and its vendor.
	 */

> +	if (par->vbe_ib.oem_vendor_name_ptr)
> +		printk("%s, ",
> +			(char*)(task->buf + par->vbe_ib.oem_vendor_name_ptr));
> +
> +	if (par->vbe_ib.oem_product_name_ptr)
> +		printk("%s, ",
> +			(char*)(task->buf + par->vbe_ib.oem_product_name_ptr));
> +
> +	if (par->vbe_ib.oem_product_rev_ptr)
> +		printk("%s, ",
> +			(char*)(task->buf + par->vbe_ib.oem_product_rev_ptr));
> +
> +	if (par->vbe_ib.oem_string_ptr)
> +		printk("OEM: %s, ",
> +			(char*)(task->buf + par->vbe_ib.oem_string_ptr));
> +
> +	printk("VBE v%d.%d\n", ((par->vbe_ib.vbe_version & 0xff00) >> 8),
> +			par->vbe_ib.vbe_version & 0xff);
> +
> +	return 0;
> +}
> +
> +static int __devinit uvesafb_vbe_getmodes(struct uvesafb_ktask *task,
> +		struct uvesafb_par *par)
> +{
> +	int off = 0, err;
> +	u16 *mode;
> +
> +	par->vbe_modes_cnt = 0;
> +
> +	/* Count available modes. */
> +	mode = (u16*) (((u8*)&par->vbe_ib) + par->vbe_ib.mode_list_ptr);
> +	while (*mode != 0xffff) {
> +		par->vbe_modes_cnt++;
> +		mode++;
> +	}
> +
> +	par->vbe_modes = kzalloc(sizeof(struct vbe_mode_ib) *
> +				par->vbe_modes_cnt, GFP_KERNEL);
> +	if (!par->vbe_modes)
> +		return -ENOMEM;
> +
> +	/* Get info about all available modes. */
> +	mode = (u16*) (((u8*)&par->vbe_ib) + par->vbe_ib.mode_list_ptr);
> +	while (*mode != 0xffff) {
> +		struct vbe_mode_ib *mib;
> +
> +		uvesafb_reset(task);
> +		task->t.regs.eax = 0x4f01;
> +		task->t.regs.ecx = (u32) *mode;
> +		task->t.flags = TF_BUF_RET | TF_BUF_ESDI;
> +		task->t.buf_len = sizeof(struct vbe_mode_ib);
> +		task->buf = (u8*) (par->vbe_modes + off);
> +
> +		err = uvesafb_exec(task);
> +		if (err || (task->t.regs.eax & 0xffff) != 0x004f) {
> +			printk(KERN_ERR "uvesafb: Getting mode info block "
> +				"for mode 0x%x failed (eax=0x%x, err=%x)\n",
> +				*mode, (u32)task->t.regs.eax, err);
> +			return -EINVAL;
> +		}
> +
> +		mib = (struct vbe_mode_ib*)task->buf;

Perhaps uvesafb_ktask.buf should be void* so we can lose some typecasts.

> +		mib->mode_id = *mode;
> +
> +		/* We only want modes that are supported with the current
> +		 * hardware configuration, color, graphics and that have
> +		 * support for the LFB. */
> +		if ((mib->mode_attr & VBE_MODE_MASK) == VBE_MODE_MASK &&
> +		     mib->bits_per_pixel >= 8) {
> +			off++;
> +		} else {
> +			par->vbe_modes_cnt--;
> +		}
> +		mode++;
> +		mib->depth = mib->red_len + mib->green_len + mib->blue_len;
> +
> +		/* Handle 8bpp modes and modes with broken color component
> +		 * lengths. */
> +		if (mib->depth == 0 || (mib->depth == 24 &&
> +					mib->bits_per_pixel == 32))
> +			mib->depth = mib->bits_per_pixel;
> +	}
> +
> +	return 0;
> +}
> +
> +#ifdef __i386__

CONFIG_X86 would be more typical.

Be aware that CONFIG_X86 is true for both i386 and x86_64.  You don't state
whether this code works on x86_64.  If it can, it should.

> +static int __devinit uvesafb_vbe_getpmi(struct uvesafb_ktask *task,
> +		struct uvesafb_par *par)
> +{
> +	int i, err;
> +
> +	uvesafb_reset(task);
> +	task->t.regs.eax = 0x4f0a;
> +	task->t.regs.ebx = 0x0;
> +	err = uvesafb_exec(task);
> +
> +	if ((task->t.regs.eax & 0xffff) != 0x4f || task->t.regs.es < 0xc000) {
> +		par->pmi_setpal = par->ypan = 0;
> +	} else {
> +		par->pmi_base = (u16*)phys_to_virt(((u32)task->t.regs.es << 4)
> +						+ task->t.regs.edi);
> +		par->pmi_start = (void*)((char*)par->pmi_base +
> +						par->pmi_base[1]);
> +		par->pmi_pal = (void*)((char*)par->pmi_base +
> +						par->pmi_base[2]);
> +		printk(KERN_INFO "uvesafb: protected mode interface info at "
> +				 "%04x:%04x\n",
> +				 (u16)task->t.regs.es, (u16)task->t.regs.edi);
> +		printk(KERN_INFO "uvesafb: pmi: set display start = %p, "
> +				 "set palette = %p\n", par->pmi_start,
> +				 par->pmi_pal);
> +
> +		if (par->pmi_base[3]) {
> +			printk(KERN_INFO "uvesafb: pmi: ports = ");
> +			for (i = par->pmi_base[3]/2;
> +					par->pmi_base[i] != 0xffff; i++)
> +				printk("%x ", par->pmi_base[i]);
> +			printk("\n");
> +
> +			if (par->pmi_base[i] != 0xffff) {
> +				printk(KERN_INFO "uvesafb: can't handle memory"
> +						 " requests, pmi disabled\n");
> +				par->ypan = par->pmi_setpal = 0;
> +			}
> +		}
> +	}
> +	return 0;
> +}
> +#endif

You might care to cc "H.  Peter Anvin" <hpa@zytor.com> on the next version
of this patch - he's a whizz at this sort of low-level x86 bios
communication stuff.

> +/* Check whether a video mode is supported by the Video BIOS and is
> + * compatible with the monitor limits. */
> +static int __devinit uvesafb_is_valid_mode(struct fb_videomode *mode,
> +		struct fb_info *info)
> +{
> +	if (info->monspecs.gtf) {
> +		fb_videomode_to_var(&info->var, mode);
> +		if (fb_validate_mode(&info->var, info))
> +			return -1;
> +	}
> +
> +	if (uvesafb_vbe_find_mode(info->par, mode->xres, mode->yres, 8,
> +				UVESAFB_NEED_EXACT_RES) == -1)
> +		return -1;
> +
> +	return 0;
> +}
> +
> +static int __devinit uvesafb_vbe_getedid(struct uvesafb_ktask *task,
> +		struct fb_info *info)
> +{
> +	struct uvesafb_par *par = (struct uvesafb_par *)info->par;

fb_info.par has type void* hence all casts such as the above can and should
be removed.

> +	int err = 0;
> +
> +	if (noedid || par->vbe_ib.vbe_version < 0x0300)
> +		return -EINVAL;
> +
> +	task->t.regs.eax = 0x4f15;
> +	task->t.regs.ebx = 0;
> +	task->t.regs.ecx = 0;
> +	task->t.buf_len = 0;
> +	task->t.flags = 0;
> +
> +	err = uvesafb_exec(task);
> +
> +	if ((task->t.regs.eax & 0xffff) != 0x004f || err)
> +		return -EINVAL;
> +
> +	if ((task->t.regs.ebx & 0x3) == 3) {
> +		printk(KERN_INFO "uvesafb: VBIOS/hardware supports both "
> +				 "DDC1 and DDC2 transfers\n");
> +	} else if ((task->t.regs.ebx & 0x3) == 2) {
> +		printk(KERN_INFO "uvesafb: VBIOS/hardware supports DDC2 "
> +				 "transfers\n");
> +	} else if ((task->t.regs.ebx & 0x3) == 1) {
> +		printk(KERN_INFO "uvesafb: VBIOS/hardware supports DDC1 "
> +				 "transfers\n");
> +	} else {
> +		printk(KERN_INFO "uvesafb: VBIOS/hardware doesn't support "
> +				 "DDC transfers\n");
> +		return -EINVAL;
> +	}
> +
> +	task->t.regs.eax = 0x4f15;
> +	task->t.regs.ebx = 1;
> +	task->t.regs.ecx = task->t.regs.edx = 0;
> +	task->t.flags = TF_BUF_RET | TF_BUF_ESDI;
> +	task->t.buf_len = EDID_LENGTH;
> +	task->buf = kzalloc(EDID_LENGTH, GFP_KERNEL);
> +
> +	err = uvesafb_exec(task);
> +
> +	if ((task->t.regs.eax & 0xffff) == 0x004f && !err) {
> +		fb_edid_to_monspecs(task->buf, &info->monspecs);
> +
> +		if (info->monspecs.vfmax && info->monspecs.hfmax) {
> +			/* If the maximum pixel clock wasn't specified in
> +			 * the EDID block, set it to 300 MHz. */
> +			if (info->monspecs.dclkmax == 0)
> +				info->monspecs.dclkmax = 300 * 1000000;
> +			info->monspecs.gtf = 1;
> +		}
> +	} else {
> +		err = -EINVAL;
> +	}
> +
> +	kfree(task->buf);
> +	return err;
> +}
> +
> +static void __devinit uvesafb_vbe_getmonspecs(struct uvesafb_ktask *task,
> +		struct fb_info *info)
> +{
> +	struct uvesafb_par *par = info->par;
> +	int i;
> +	memset(&info->monspecs, 0, sizeof(struct fb_monspecs));

Missing a newline above.

> +	/* If we don't get all necessary data from the EDID block,
> +	 * mark it as incompatible with the GTF. */
> +	if (uvesafb_vbe_getedid(task, info))
> +		info->monspecs.gtf = 0;
> +
> +	/* Kernel command line overrides. */
> +	if (maxclk)
> +		info->monspecs.dclkmax = maxclk * 1000000;
> +	if (maxvf)
> +		info->monspecs.vfmax = maxvf;
> +	if (maxhf)
> +		info->monspecs.hfmax = maxhf * 1000;
> +
> +	/* In case DDC transfers are not supported the user can provide
> +	 * monitor limits manually. Lower limits are set to "safe" values. */
> +	if (info->monspecs.gtf == 0 && maxclk && maxvf && maxhf) {
> +		info->monspecs.dclkmin = 0;
> +		info->monspecs.vfmin = 60;
> +		info->monspecs.hfmin = 29000;
> +		info->monspecs.gtf = 1;
> +	}
> +
> +	if (info->monspecs.gtf)
> +		printk(KERN_INFO
> +			"uvesafb: monitor limits: vf = %d Hz, hf = %d kHz, "
> +			"clk = %d MHz\n", info->monspecs.vfmax,
> +			(int)(info->monspecs.hfmax / 1000),
> +			(int)(info->monspecs.dclkmax / 1000000));
> +	else
> +		printk(KERN_INFO "uvesafb: no monitor limits have been set\n");
> +
> +	/* Add VBE modes to the modelist. */
> +	for (i = 0; i < par->vbe_modes_cnt; i++) {
> +		struct fb_var_screeninfo var;
> +		struct vbe_mode_ib *mode;
> +		struct fb_videomode vmode;
> +
> +		mode = &par->vbe_modes[i];
> +		memset(&var, 0, sizeof(var));
> +
> +		var.xres = mode->x_res;
> +		var.yres = mode->y_res;
> +
> +		fb_get_mode(FB_VSYNCTIMINGS | FB_IGNOREMON, 60, &var, info);
> +		fb_var_to_videomode(&vmode, &var);
> +		fb_add_videomode(&vmode, &info->modelist);
> +	}
> +
> +	/* Add valid VESA modes to our modelist. */
> +	for (i = 0; i < VESA_MODEDB_SIZE; i++) {
> +		if (!uvesafb_is_valid_mode((struct fb_videomode*)
> +						&vesa_modes[i], info))
> +			fb_add_videomode(&vesa_modes[i], &info->modelist);
> +	}
> +
> +	for (i = 0; i < info->monspecs.modedb_len; i++) {
> +		if (!uvesafb_is_valid_mode(&info->monspecs.modedb[i], info))
> +			fb_add_videomode(&info->monspecs.modedb[i],
> +					&info->modelist);
> +	}
> +
> +	return;
> +}
> +
> +static void __devinit uvesafb_vbe_getstatesize(struct uvesafb_ktask *task,
> +		struct uvesafb_par *par)
> +{
> +	int err;
> +	uvesafb_reset(task);

Here also.

> +	/* Get the VBE state buffer size. We want all available
> +	 * hardware state data (CL = 0x0f). */
> +	task->t.regs.eax = 0x4f04;
> +	task->t.regs.ecx = 0x000f;
> +	task->t.regs.edx = 0x0000;
> +	task->t.flags = 0;
> +
> +	err = uvesafb_exec(task);
> +
> +	if (err || (task->t.regs.eax & 0xffff) != 0x004f) {
> +		printk(KERN_WARNING "uvesafb: VBE state buffer size "
> +			"cannot be determined (eax=0x%x, err=%d)\n",
> +			task->t.regs.eax, err);
> +		par->vbe_state_size = 0;
> +		return;
> +	}
> +
> +	par->vbe_state_size = 64 * (task->t.regs.ebx & 0xffff);
> +}
> +
> +static int __devinit uvesafb_vbe_init(struct fb_info *info)
> +{
> +	struct uvesafb_ktask *task = NULL;
> +	struct uvesafb_par *par = info->par;
> +	int err;
> +
> +	task = uvesafb_prep();
> +	if (!task)
> +		return -ENOMEM;
> +
> +	if ((err = uvesafb_vbe_getinfo(task, par)))

checkpatch.pl will do my work here..

> +		goto out;
> +	if ((err = uvesafb_vbe_getmodes(task, par)))
> +		goto out;
> +	par->nocrtc = nocrtc;
> +#ifdef __i386__
> +	par->pmi_setpal = pmi_setpal;
> +	par->ypan = ypan;
> +
> +	if (par->pmi_setpal || par->ypan)
> +		uvesafb_vbe_getpmi(task, par);
> +#else
> +	/* The protected mode interface is not available on non-x86. */
> +	par->pmi_setpal = par->ypan = 0;
> +#endif
> +
> +	INIT_LIST_HEAD(&info->modelist);
> +	uvesafb_vbe_getmonspecs(task, info);
> +	uvesafb_vbe_getstatesize(task, par);
> +
> +out:	uvesafb_free(task);
> +	return err;
> +}
> +
> +static int __devinit uvesafb_vbe_init_mode(struct fb_info *info)
> +{
> +	struct list_head *pos;
> +	struct fb_modelist *modelist;
> +	struct fb_videomode *mode;
> +	struct uvesafb_par *par = info->par;
> +	int i, modeid;
> +
> +	/* Has the user requested a specific VESA mode? */
> +	if (vbemode) {
> +		for (i = 0; i < par->vbe_modes_cnt; i++) {
> +			if (par->vbe_modes[i].mode_id == vbemode) {
> +				fb_get_mode(FB_VSYNCTIMINGS | FB_IGNOREMON, 60,
> +							&info->var, info);
> +				/* With pixclock set to 0, the default BIOS
> +				 * timings will be used in set_par(). */
> +				info->var.pixclock = 0;
> +				modeid = i;
> +				goto gotmode;
> +			}
> +		}
> +		printk(KERN_INFO "uvesafb: requested VBE mode 0x%x is "
> +				 "unavailable\n", vbemode);
> +		vbemode = 0;
> +	}
> +
> +	i = 0;
> +	list_for_each(pos, &info->modelist) {
> +		i++;
> +	}

Unneeded braces

> +	mode = kzalloc(i * sizeof(*mode), GFP_KERNEL);

Check for and handle the allocation failure, please.

> +	i = 0;
> +	list_for_each(pos, &info->modelist) {
> +		modelist = list_entry(pos, struct fb_modelist, list);
> +		mode[i] = modelist->mode;
> +		i++;
> +	}
> +
> +	if (!mode_option)
> +		mode_option = UVESAFB_DEFAULT_MODE;
> +
> +	i = fb_find_mode(&info->var, info, mode_option, mode, i,
> +		NULL, 8);
> +
> +	kfree(mode);
> +
> +	/* fb_find_mode() failed */
> +	if (i == 0 || i >= 3) {
> +		info->var.xres = 640;
> +		info->var.yres = 480;
> +		mode = (struct fb_videomode*)
> +				fb_find_best_mode(&info->var, &info->modelist);
> +
> +		if (mode) {
> +			fb_videomode_to_var(&info->var, mode);
> +		} else {
> +			modeid = par->vbe_modes[0].mode_id;
> +			fb_get_mode(FB_VSYNCTIMINGS | FB_IGNOREMON, 60,
> +				    &info->var, info);
> +			goto gotmode;
> +		}
> +	}
> +
> +	/* Look for a matching VBE mode. */
> +	modeid = uvesafb_vbe_find_mode(par, info->var.xres, info->var.yres,
> +			info->var.bits_per_pixel, UVESAFB_NEED_EXACT_RES);
> +
> +	if (modeid == -1)
> +		return -EINVAL;
> +
> +gotmode:
> +	uvesafb_setup_var(&info->var, info, &par->vbe_modes[modeid]);
> +
> +	/* If we are not VBE3.0+ compliant, we're done -- the BIOS will
> +	 * ignore our mode timings anyway. */
> +	if (par->vbe_ib.vbe_version < 0x0300)
> +		fb_get_mode(FB_VSYNCTIMINGS | FB_IGNOREMON, 60,
> +					&info->var, info);
> +
> +	return modeid;
> +}
> +
> +static int uvesafb_setpalette(struct uvesafb_pal_entry *entries, int count,
> +			     int start, struct fb_info *info)
> +{
> +	struct uvesafb_ktask *task;
> +	struct uvesafb_par *par = info->par;
> +	int i = par->mode_idx;
> +	int err = 0;
> +
> +	/* We support palette modifications for 8 bpp modes only, so
> +	 * there can never be more than 256 entries. */
> +	if (start + count > 256)
> +		return -EINVAL;
> +
> +	/* Use VGA registers if mode is VGA-compatible. */
> +	if (i >= 0 && i < par->vbe_modes_cnt &&
> +	    par->vbe_modes[i].mode_attr & VBE_MODE_VGACOMPAT) {
> +		for (i = 0; i < count; i++) {
> +			outb_p(start + i,        dac_reg);
> +			outb_p(entries[i].red,   dac_val);
> +			outb_p(entries[i].green, dac_val);
> +			outb_p(entries[i].blue,  dac_val);
> +		}
> +	} else if (par->pmi_setpal) {
> +		__asm__ __volatile__(
> +		"call *(%%esi)"
> +		: /* no return value */
> +		: "a" (0x4f09),         /* EAX */
> +		  "b" (0),              /* EBX */
> +		  "c" (count),          /* ECX */
> +		  "d" (start),          /* EDX */
> +		  "D" (entries),        /* EDI */
> +		  "S" (&par->pmi_pal)); /* ESI */

uh, so we're CONFIG_X86_32-only, yes?

> +	} else {
> +		task = uvesafb_prep();
> +		if (!task)
> +			return -ENOMEM;
> +
> +		task->t.regs.eax = 0x4f09;
> +		task->t.regs.ebx = 0x0;
> +		task->t.regs.ecx = count;
> +		task->t.regs.edx = start;
> +		task->t.flags = TF_BUF_ESDI;
> +		task->t.buf_len = sizeof(struct uvesafb_pal_entry) * count;
> +		task->buf = (u8*) entries;
> +
> +		err = uvesafb_exec(task);
> +		if ((task->t.regs.eax & 0xffff) != 0x004f)
> +			err = 1;
> +
> +		uvesafb_free(task);
> +	}
> +	return err;
> +}
> +
>
> ...
>
> +
> +static struct device_attribute device_attrs[] = {
> +	__ATTR(vbe_version, S_IRUGO, uvesafb_show_vbe_ver, NULL),
> +	__ATTR(vbe_modes, S_IRUGO, uvesafb_show_vbe_modes, NULL),
> +	__ATTR(oem_vendor, S_IRUGO, uvesafb_show_vendor, NULL),
> +	__ATTR(oem_product_name, S_IRUGO, uvesafb_show_product_name, NULL),
> +	__ATTR(oem_product_rev, S_IRUGO, uvesafb_show_product_rev, NULL),
> +	__ATTR(oem_string, S_IRUGO, uvesafb_show_oem_string, NULL),
> +	__ATTR(nocrtc, S_IRUGO | S_IWUSR, uvesafb_show_nocrtc,
> +		uvesafb_store_nocrtc),
> +};

Can we use an attribute group here?

> +static void __devinit uvesafb_create_sysfs(struct device *dev,
> +		struct fb_info *info)
> +{
> +	int i, err = 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(device_attrs); i++) {
> +		err |= device_create_file(dev, &device_attrs[i]);
> +	}

Unneeded braces

> +	if (err != 0)
> +		printk(KERN_WARNING "fb%d: failed to register attributes\n",
> +			info->node);
> +}
> +
> +static void __devinit uvesafb_remove_sysfs(struct device *dev)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(device_attrs); i++) {
> +		device_remove_file(dev, &device_attrs[i]);
> +	}

ditto

> +}
> +
> +static int __devinit uvesafb_probe(struct platform_device *dev)
> +{
> +	struct fb_info *info;
> +	struct vbe_mode_ib *mode = NULL;
> +	struct uvesafb_par *par;
> +	int err = 0, i;
> +
> +	info = framebuffer_alloc(sizeof(*par) +	sizeof(u32) * 256, &dev->dev);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	par = info->par;
> +
> +	if ((err = uvesafb_vbe_init(info))) {
> +		printk(KERN_ERR "uvesafb: vbe_init() failed with %d\n", err);
> +		goto out;
> +	}
> +
> +	info->fbops = &uvesafb_ops;
> +
> +	i = uvesafb_vbe_init_mode(info);
> +	if (i < 0) {
> +		err = -EINVAL;
> +		goto out;
> +	} else {
> +		mode = &par->vbe_modes[i];
> +	}
> +
> +	if (fb_alloc_cmap(&info->cmap, 256, 0) < 0) {
> +		err = -ENXIO;
> +		goto out;
> +	}
> +
> +	uvesafb_init_info(info, mode);
> +
> +	if (!request_mem_region(info->fix.smem_start, info->fix.smem_len,
> +	    "uvesafb")) {
> +		printk(KERN_WARNING "uvesafb: cannot reserve video memory at "
> +		       "0x%lx\n", info->fix.smem_start);
> +		/* We cannot make this fatal. Sometimes this comes from magic
> +		   spaces our resource handlers simply don't know about. */

so...  what happens?  The driver starts altering mrmory regions which it
doesn't own?

> +	}
> +
> +	info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len);
> +
> +	if (!info->screen_base) {
> +		printk(KERN_ERR
> +		       "uvesafb: abort, cannot ioremap 0x%x bytes of video "
> +		       "memory at 0x%lx\n",
> +		       info->fix.smem_len, info->fix.smem_start);
> +		err = -EIO;
> +		goto out_mem;
> +	}
> +
> +	if (!request_region(0x3c0, 32, "uvesafb")) {
> +		printk(KERN_ERR "uvesafb: request region 0x3c0-0x3e0 failed\n");
> +		err = -EIO;
> +		goto out_unmap;
> +	}
> +
> +	uvesafb_init_mtrr(info);
> +	platform_set_drvdata(dev, info);
> +
> +	if (register_framebuffer(info) < 0) {
> +		printk(KERN_ERR
> +		       "uvesafb: failed to register framebuffer device\n");
> +		err = -EINVAL;
> +		goto out_reg;
> +	}
> +
> +	printk(KERN_INFO "uvesafb: framebuffer at 0x%lx, mapped to 0x%p, "
> +		       "using %dk, total %dk\n", info->fix.smem_start,
> +		       info->screen_base, info->fix.smem_len/1024,
> +		       par->vbe_ib.total_memory * 64);
> +	printk(KERN_INFO "fb%d: %s frame buffer device\n", info->node,
> +		       info->fix.id);
> +
> +	uvesafb_create_sysfs(&dev->dev, info);
> +	return 0;
> +
> +out_reg:
> +	release_region(0x3c0, 32);
> +out_unmap:
> +	iounmap(info->screen_base);
> +out_mem:
> +	release_mem_region(info->fix.smem_start, info->fix.smem_len);
> +	if (!list_empty(&info->modelist))
> +		fb_destroy_modelist(&info->modelist);
> +	fb_destroy_modedb(info->monspecs.modedb);
> +	fb_dealloc_cmap(&info->cmap);
> +out:
> +	if (par->vbe_modes)
> +		kfree(par->vbe_modes);
> +
> +	framebuffer_release(info);
> +	return err;
> +}
>
> ...
>
> +#ifndef MODULE
> +static int __devinit uvesafb_setup(char *options)
> +{
> +	char *this_opt;
> +
> +	if (!options || !*options)
> +		return 0;
> +
> +	while ((this_opt = strsep(&options, ",")) != NULL) {
> +		if (!*this_opt) continue;
> +
> +		if (!strcmp(this_opt, "redraw"))
> +			ypan = 0;
> +		else if (!strcmp(this_opt, "ypan"))
> +			ypan = 1;
> +		else if (!strcmp(this_opt, "ywrap"))
> +			ypan = 2;
> +		else if (!strcmp(this_opt, "vgapal"))
> +			pmi_setpal = 0;
> +		else if (!strcmp(this_opt, "pmipal"))
> +			pmi_setpal = 1;
> +		else if (!strncmp(this_opt, "mtrr:", 5))
> +			mtrr = simple_strtoul(this_opt+5, NULL, 0);
> +		else if (!strcmp(this_opt, "nomtrr"))
> +			mtrr = 0;
> +		else if (!strcmp(this_opt, "nocrtc"))
> +			nocrtc = 1;
> +		else if (!strcmp(this_opt, "noedid"))
> +			noedid = 1;
> +		else if (!strcmp(this_opt, "noblank"))
> +			blank = 0;
> +		else if (!strncmp(this_opt, "vtotal:", 7))
> +			vram_total = simple_strtoul(this_opt + 7, NULL, 0);
> +		else if (!strncmp(this_opt, "vremap:", 7))
> +			vram_remap = simple_strtoul(this_opt + 7, NULL, 0);
> +		else if (!strncmp(this_opt, "maxhf:", 6))
> +			maxhf = simple_strtoul(this_opt + 6, NULL, 0);
> +		else if (!strncmp(this_opt, "maxvf:", 6))
> +			maxvf = simple_strtoul(this_opt + 6, NULL, 0);
> +		else if (!strncmp(this_opt, "maxclk:", 7))
> +			maxclk = simple_strtoul(this_opt + 7, NULL, 0);
> +		else if (!strncmp(this_opt, "vbemode:", 8))
> +			vbemode = simple_strtoul(this_opt + 8, NULL,0);
> +		else if (this_opt[0] >= '0' && this_opt[0] <= '9') {
> +			mode_option = this_opt;
> +		} else {
> +			printk(KERN_WARNING
> +			       "uvesafb: unrecognized option %s\n", this_opt);
> +		}
> +	}
> +
> +	return 0;
> +}
> +#endif /* !MODULE */

The #ifdef MODULE is unpleasing.  Can we use module_param() here?  That'll
work for both modular and non-modular case.

> +#define uvesafb_free(task)					\
> +do {								\
> +	if (task) {						\
> +		if (task->done)					\
> +			kfree(task->done);			\
> +		kfree(task);					\
> +		task = NULL;					\
> +	}							\
> +} while(0)

I don't think this need to be a macro?  Code should be written in C unless
there is some reason why it _must_ be a macro.

Oh, it sneakily zeroes a variable within the caller's context.  Ow.  Can
we not do that please?  Someone who is reading this code will see a call to
a "function" called uvesafb_free() and the last thing they will expect is
that "function" magically zeroed out a local variable.

> +#define uvesafb_reset(task)					\
> +do {								\
> +	struct completion *cpl = task->done;			\
> +	memset(task, 0, sizeof(struct uvesafb_ktask));		\
> +	task->done = cpl;					\
> +} while(0)							\

This can be a regular C function.

> +static int uvesafb_exec(struct uvesafb_ktask *tsk);
> +
> +#define UVESAFB_NEED_EXACT_RES		1
> +#define UVESAFB_NEED_EXACT_DEPTH	2
> +
> +struct uvesafb_par {
> +	struct vbe_ib vbe_ib;		/* VBE Info Block */
> +	struct vbe_mode_ib *vbe_modes;	/* list of supported VBE modes */
> +	int vbe_modes_cnt;
> +
> +	u8 nocrtc;
> +	u8 ypan;			/* 0 - nothing, 1 - ypan, 2 - ywrap */
> +	u8 pmi_setpal;			/* pmi for palette changes */
> +	u16  *pmi_base;			/* protected mode interface location */
> +	void (*pmi_start)(void);
> +	void (*pmi_pal)(void);
> +
> +	u8 *vbe_state_orig;		/* original hardware state, before the driver was loaded */
> +	u8 *vbe_state_saved;		/* state saved by fb_save_state */
> +	int vbe_state_size;
> +	atomic_t ref_count;
> +
> +	int mode_idx;
> +	struct vbe_crtc_ib crtc;
> +};
> +
> +#endif

A comment on the endif would be nice.

> +#endif /* _UVESAFB_H */


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

* Re: [PATCH 3/4] fbdev: uvesafb driver
  2007-06-23 18:35 ` Andrew Morton
@ 2007-06-23 23:20   ` Michal Januszewski
  2007-06-23 23:36     ` Andrew Morton
  0 siblings, 1 reply; 7+ messages in thread
From: Michal Januszewski @ 2007-06-23 23:20 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-fbdev-devel, linux-kernel

On Sat, Jun 23, 2007 at 11:35:57AM -0700, Andrew Morton wrote:
> On Sat, 23 Jun 2007 12:52:43 +0200 Michal Januszewski <spock@gentoo.org> wrote:

> > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> > index 403dac7..5cc03f9 100644
> > --- a/drivers/video/Kconfig
> > +++ b/drivers/video/Kconfig
> > @@ -585,6 +585,24 @@ config FB_TGA
> >  
> >  	  Say Y if you have one of those.
> >  
> > +config FB_UVESA
> > +	tristate "Userspace VESA VGA graphics support"
> > +	depends on FB && CONNECTOR
> 
> These dependencies are insufficient.

What exactly is missing here?  A dep on X86?  This would indicate the
arches on which the driver has actually been tested.  But which arches
are supported and which aren't is, in the end, up to the userspace helper.
 
> > +static struct cb_id uvesafb_cn_id = {
> > +	.idx = CN_IDX_V86D,
> > +	.val = CN_VAL_V86D_UVESAFB
> > +};
> > +static struct sock *nls;
> > +static char v86d_path[PATH_MAX] = "/sbin/v86d";
> 
> Remove the PATH_MAX, save some memory.
> 
> Oh, it gets set via sysfs.  hrm.

Hmm, I guess I could make it a hard-coded value, but paths to userspace 
helpers should generally be configurable, right?

> Now I'm wondering what this code actually does.  It would be nice to have
> an overall design and implementation description in the changelog so we
> don't have to reverse-engineer your thoughts from your implementation...

OK, I'll include a more detailed description in the next round of the
patches.
 
> > +#ifdef __i386__
> 
> CONFIG_X86 would be more typical.
> 
> Be aware that CONFIG_X86 is true for both i386 and x86_64.  You don't state
> whether this code works on x86_64.  If it can, it should.

AFAIK, it can't.  PMI code is meant to be run in 32-bit protected mode.
I'll add a note about this to the patch and change __i386__ to
CONFIG_X86_32.
 
> You might care to cc "H.  Peter Anvin" <hpa@zytor.com> on the next version
> of this patch - he's a whizz at this sort of low-level x86 bios
> communication stuff.

Thanks, I'll do that.
 
> > +	if (!request_mem_region(info->fix.smem_start, info->fix.smem_len,
> > +	    "uvesafb")) {
> > +		printk(KERN_WARNING "uvesafb: cannot reserve video memory at "
> > +		       "0x%lx\n", info->fix.smem_start);
> > +		/* We cannot make this fatal. Sometimes this comes from magic
> > +		   spaces our resource handlers simply don't know about. */
> 
> so...  what happens?  The driver starts altering mrmory regions which it
> doesn't own?

Fixed.  This was a leftover from vesafb.c.  Are there any reasons for not
fixing it there as well?
 
> > +#ifndef MODULE
> > +static int __devinit uvesafb_setup(char *options)
> > +{
> > +	char *this_opt;
> > +
> > +	if (!options || !*options)
> > +		return 0;
> > +
> > +	while ((this_opt = strsep(&options, ",")) != NULL) {
> > +		if (!*this_opt) continue;
> > +
> > +		if (!strcmp(this_opt, "redraw"))
> > +			ypan = 0;
> > +		else if (!strcmp(this_opt, "ypan"))
> > +			ypan = 1;
> > +		else if (!strcmp(this_opt, "ywrap"))
> > +			ypan = 2;
> > +		else if (!strcmp(this_opt, "vgapal"))
> > +			pmi_setpal = 0;
> > +		else if (!strcmp(this_opt, "pmipal"))
> > +			pmi_setpal = 1;
> > +		else if (!strncmp(this_opt, "mtrr:", 5))
> > +			mtrr = simple_strtoul(this_opt+5, NULL, 0);
> > +		else if (!strcmp(this_opt, "nomtrr"))
> > +			mtrr = 0;
> > +		else if (!strcmp(this_opt, "nocrtc"))
> > +			nocrtc = 1;
> > +		else if (!strcmp(this_opt, "noedid"))
> > +			noedid = 1;
> > +		else if (!strcmp(this_opt, "noblank"))
> > +			blank = 0;
> > +		else if (!strncmp(this_opt, "vtotal:", 7))
> > +			vram_total = simple_strtoul(this_opt + 7, NULL, 0);
> > +		else if (!strncmp(this_opt, "vremap:", 7))
> > +			vram_remap = simple_strtoul(this_opt + 7, NULL, 0);
> > +		else if (!strncmp(this_opt, "maxhf:", 6))
> > +			maxhf = simple_strtoul(this_opt + 6, NULL, 0);
> > +		else if (!strncmp(this_opt, "maxvf:", 6))
> > +			maxvf = simple_strtoul(this_opt + 6, NULL, 0);
> > +		else if (!strncmp(this_opt, "maxclk:", 7))
> > +			maxclk = simple_strtoul(this_opt + 7, NULL, 0);
> > +		else if (!strncmp(this_opt, "vbemode:", 8))
> > +			vbemode = simple_strtoul(this_opt + 8, NULL,0);
> > +		else if (this_opt[0] >= '0' && this_opt[0] <= '9') {
> > +			mode_option = this_opt;
> > +		} else {
> > +			printk(KERN_WARNING
> > +			       "uvesafb: unrecognized option %s\n", this_opt);
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +#endif /* !MODULE */
> 
> The #ifdef MODULE is unpleasing.  Can we use module_param() here?  That'll
> work for both modular and non-modular case.

True, but it would also make uvesafb the only (?) fb driver that
doesn't support the "video=smthfb:<options>" way of setting options.
Unless it's considered deprecated, I think it would be best to leave
it as it is, for the sake of consistency.
 
Thanks for all your comments & suggestions, and I'm sorry for the screw-up
with checkpatch.pl.  I've fixed the code in my git tree and the updates
will be included in the next version of this patch.

Best regards,
Michal


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

* Re: [PATCH 3/4] fbdev: uvesafb driver
  2007-06-23 23:20   ` Michal Januszewski
@ 2007-06-23 23:36     ` Andrew Morton
  2007-06-24 11:15       ` Michal Januszewski
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Morton @ 2007-06-23 23:36 UTC (permalink / raw)
  To: spock; +Cc: linux-fbdev-devel, linux-kernel

On Sun, 24 Jun 2007 01:20:33 +0200 Michal Januszewski <spock@gentoo.org> wrote:

> > > +config FB_UVESA
> > > +	tristate "Userspace VESA VGA graphics support"
> > > +	depends on FB && CONNECTOR
> > 
> > These dependencies are insufficient.
> 
> What exactly is missing here?  A dep on X86?

Yes.  From your other comments it appears that a dependency on X86_32 is
needed.

>  This would indicate the
> arches on which the driver has actually been tested.  But which arches
> are supported and which aren't is, in the end, up to the userspace helper.

The other architectures won't compile: they don't have mtrr.h

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

* Re: [PATCH 3/4] fbdev: uvesafb driver
  2007-06-23 10:52 [PATCH 3/4] fbdev: uvesafb driver Michal Januszewski
  2007-06-23 11:51 ` [Linux-fbdev-devel] " Bernhard Fischer
  2007-06-23 18:35 ` Andrew Morton
@ 2007-06-24  3:50 ` Akinobu Mita
  2 siblings, 0 replies; 7+ messages in thread
From: Akinobu Mita @ 2007-06-24  3:50 UTC (permalink / raw)
  To: Michal Januszewski; +Cc: linux-fbdev-devel, linux-kernel

> +static int uvesafb_blank(int blank, struct fb_info *info)
> +{
> +	struct uvesafb_par *par = info->par;
> +	struct uvesafb_ktask *task;
> +	int err = 1;
> +
> +	if (par->vbe_ib.capabilities & VBE_CAP_VGACOMPAT) {
> +		int loop = 10000;
> +		u8 seq = 0, crtc17 = 0;
> +
> +		if (blank == FB_BLANK_POWERDOWN) {
> +			seq = 0x20;
> +			crtc17 = 0x00;
> +			err = 0;
> +		} else {
> +			seq = 0x00;
> +			crtc17 = 0x80;
> +			err = (blank == FB_BLANK_UNBLANK) ? 0 : -EINVAL;
> +		}
> +
> +		vga_wseq(NULL, 0x00, 0x01);
> +		seq |= vga_rseq(NULL, 0x01) & ~0x20;
> +		vga_wseq(NULL, 0x00, seq);
> +
> +		crtc17 |= vga_rcrt(NULL, 0x17) & ~0x80;
> +		while (loop--);
> +		vga_wcrt(NULL, 0x17, crtc17);
> +		vga_wseq(NULL, 0x00, 0x03);
> +	} else {
> +		task = uvesafb_prep();
> +		if (!task)
> +			return -ENOMEM;
> +
> +		task->t.regs.eax = 0x4f10;
> +		switch (blank) {
> +		case FB_BLANK_UNBLANK:
> +			task->t.regs.ebx = 0x0001;
> +			break;
> +		case FB_BLANK_NORMAL:
> +			task->t.regs.ebx = 0x0101;	/* standby */
> +			break;
> +		case FB_BLANK_POWERDOWN:
> +			task->t.regs.ebx = 0x0401;	/* powerdown */
> +			break;
> +		default:
> +			goto out;
> +		}
> +		task->t.flags = 0;
> +
> +		err = uvesafb_exec(task);
> +		if (!err && (task->t.regs.eax & 0xffff) == 0x004f)
> +			err = 0;

There is no effect to this assignment.

	if (!err && ... )
		err = 0;

> +out:		uvesafb_free(task);
> +	}
> +	return err;
> +}

[...]

> +static int __devinit uvesafb_init(void)
> +{
> +	int err;
> +
> +#ifndef MODULE
> +	char *option = NULL;
> +
> +	if (fb_get_options("uvesafb", &option))
> +		return -ENODEV;
> +	uvesafb_setup(option);
> +#endif
> +	err = cn_add_callback(&uvesafb_cn_id, "uvesafb", uvesafb_cn_callback);
> +	if (err)
> +		goto err_out;
> +
> +	err = platform_driver_register(&uvesafb_driver);
> +
> +	if (!err) {
> +		uvesafb_device = platform_device_alloc("uvesafb", 0);
> +		if (uvesafb_device)
> +			err = platform_device_add(uvesafb_device);
> +		else
> +			err = -ENOMEM;
> +
> +		if (err) {
> +			platform_device_put(uvesafb_device);
> +			platform_driver_unregister(&uvesafb_driver);

Initialization failed. It should return from this function here.
Otherwise it will attempt to call driver_create_file() with unregistered
driver below.

And you forgot to put:
			cn_del_callback(&uvesafb_cn_id);

> +		}
> +
> +		err = driver_create_file(&uvesafb_driver.driver,
> +				&driver_attr_v86d);
> +		if (err) {
> +			printk(KERN_WARNING "uvesafb: failed to register "
> +					"attributes\n");
> +			err = 0;
> +		}
> +	}
> +	return err;
> +
> +err_out:
> +	if (nls && nls->sk_socket)
> +		sock_release(nls->sk_socket);

I can't find any uses of nls in this driver.

> +	return err;
> +}


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

* Re: [PATCH 3/4] fbdev: uvesafb driver
  2007-06-23 23:36     ` Andrew Morton
@ 2007-06-24 11:15       ` Michal Januszewski
  0 siblings, 0 replies; 7+ messages in thread
From: Michal Januszewski @ 2007-06-24 11:15 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-fbdev-devel, linux-kernel

On Sat, Jun 23, 2007 at 04:36:39PM -0700, Andrew Morton wrote:
> On Sun, 24 Jun 2007 01:20:33 +0200 Michal Januszewski <spock@gentoo.org> wrote:
> 
> > > > +config FB_UVESA
> > > > +	tristate "Userspace VESA VGA graphics support"
> > > > +	depends on FB && CONNECTOR
> > > 
> > > These dependencies are insufficient.
> > 
> > What exactly is missing here?  A dep on X86?
> 
> Yes.  From your other comments it appears that a dependency on X86_32 is
> needed.

X86_32 is only needed for a bunch of specific operations that make use of
the PMI.  The driver can work without those (it has already been tested
on X86_64, where PMI cannot be used).
 
> >  This would indicate the
> > arches on which the driver has actually been tested.  But which arches
> > are supported and which aren't is, in the end, up to the userspace helper.
> 
> The other architectures won't compile: they don't have mtrr.h

That was my mistake of not putting the mtrr.h include inside a #ifdef
CONFIG_MTRR.  After fixing that and a few other things you pointed out
in your previous message, I was able to successfully compile uvesafb for
PPC (using a cross-compiler).

Best regards,
Michal


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

end of thread, other threads:[~2007-06-24 11:15 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-06-23 10:52 [PATCH 3/4] fbdev: uvesafb driver Michal Januszewski
2007-06-23 11:51 ` [Linux-fbdev-devel] " Bernhard Fischer
2007-06-23 18:35 ` Andrew Morton
2007-06-23 23:20   ` Michal Januszewski
2007-06-23 23:36     ` Andrew Morton
2007-06-24 11:15       ` Michal Januszewski
2007-06-24  3:50 ` Akinobu Mita

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