[PATCH -mm] video: uvesafb: Add X86 dependency.

[PATCH -mm] video: uvesafb: Add X86 dependency.

[Paul Mundt]

uvesafb is x86-specific, reflect that in the Kconfig.

Signed-off-by: Paul Mundt <lethal@linux-sh.org>

--

 drivers/video/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-2.6.23-rc4-mm1.orig/drivers/video/Kconfig	2007-09-11 15:15:52.000000000 +0900
+++ linux-2.6.23-rc4-mm1/drivers/video/Kconfig	2007-09-11 17:01:04.000000000 +0900
@@ -593,7 +593,7 @@
 
 config FB_UVESA
 	tristate "Userspace VESA VGA graphics support"
-	depends on FB && CONNECTOR
+	depends on FB && CONNECTOR && X86
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT

Re: [PATCH -mm] video: uvesafb: Add X86 dependency.

[Paulo Marques]

Paul Mundt wrote:
> uvesafb is x86-specific, reflect that in the Kconfig.

Hummm... uvesafb _shouldn't_ be x86 specific. At least according to 
their page [1] where it says: "works on non-x86 systems".

Uvesafb uses a x86 emulator in userspace to run code from the video card 
ROM, so it should work on any PCI system where we can access the video 
card ROM and can emulate the hardware used by the ROM code.

Why do you say that it's x86 specific? Am I missing something?

-- 
Paulo Marques - www.grupopie.com

"You're just jealous because the voices only talk to me."

[1] http://dev.gentoo.org/~spock/projects/uvesafb/

Re: [PATCH -mm] video: uvesafb: Add X86 dependency.

[Paul Mundt]

On Tue, Sep 11, 2007 at 12:41:49PM +0100, Paulo Marques wrote:
> Paul Mundt wrote:
> >uvesafb is x86-specific, reflect that in the Kconfig.
> 
> Hummm... uvesafb _shouldn't_ be x86 specific. At least according to 
> their page [1] where it says: "works on non-x86 systems".
> 
> Uvesafb uses a x86 emulator in userspace to run code from the video card 
> ROM, so it should work on any PCI system where we can access the video 
> card ROM and can emulate the hardware used by the ROM code.
> 
> Why do you say that it's x86 specific? Am I missing something?
> 
The emulator it uses only runs on x86 and x86_64. Thus, it's x86
specific. The v86d and uvesafb pages seem to be in disagremeent, unless
by 'non-x86' it's only implying x86_64.

Additionally, it needs the vga I/O routines, as per vgacon. Most
platforms don't define these.

Re: [PATCH -mm] video: uvesafb: Add X86 dependency.

[Paulo Marques]

Paul Mundt wrote:
> On Tue, Sep 11, 2007 at 12:41:49PM +0100, Paulo Marques wrote:
>>[...]
>> Why do you say that it's x86 specific? Am I missing something?
>>
> The emulator it uses only runs on x86 and x86_64. Thus, it's x86
> specific. The v86d and uvesafb pages seem to be in disagremeent, unless
> by 'non-x86' it's only implying x86_64.
> 
> Additionally, it needs the vga I/O routines, as per vgacon. Most
> platforms don't define these.

I just saw the v86d emulator code, and you're right. It mmaps /dev/mem 
and assumes the video BIOS is mapped there.

We should try to change that to mmap something like 
"/sys/bus/pci/devices/0000:01:00.0/rom" instead so that it can work on 
any system with a video card plugged on a PCI bus.

It should also be possible for the emulator to translate in/out 
instructions to appropriate memory mapped I/O equivalents, no?

Anyway, I think it is up to Michal to decide if we should remove the 
kernel support for other archs, or let it stay a bit more while working 
on solving the v86d side of things. So I'll just step aside now....

-- 
Paulo Marques - www.grupopie.com

"667: The neighbor of the beast."

Re: [PATCH -mm] video: uvesafb: Add X86 dependency.

[Paul Mundt]

On Tue, Sep 11, 2007 at 01:19:57PM +0100, Paulo Marques wrote:
> Paul Mundt wrote:
> >On Tue, Sep 11, 2007 at 12:41:49PM +0100, Paulo Marques wrote:
> >>[...]
> >>Why do you say that it's x86 specific? Am I missing something?
> >>
> >The emulator it uses only runs on x86 and x86_64. Thus, it's x86
> >specific. The v86d and uvesafb pages seem to be in disagremeent, unless
> >by 'non-x86' it's only implying x86_64.
> >
> >Additionally, it needs the vga I/O routines, as per vgacon. Most
> >platforms don't define these.
> 
> I just saw the v86d emulator code, and you're right. It mmaps /dev/mem 
> and assumes the video BIOS is mapped there.
> 
> We should try to change that to mmap something like 
> "/sys/bus/pci/devices/0000:01:00.0/rom" instead so that it can work on 
> any system with a video card plugged on a PCI bus.
> 
> It should also be possible for the emulator to translate in/out 
> instructions to appropriate memory mapped I/O equivalents, no?
> 
> Anyway, I think it is up to Michal to decide if we should remove the 
> kernel support for other archs, or let it stay a bit more while working 
> on solving the v86d side of things. So I'll just step aside now....
> 
Once v86d is fixed up to get at the ROM directly and the driver uses MMIO
directly, I don't see a problem with unrestricting it. For the time being
however, the build is both broken, and the emulator it uses won't run on
anything but x86, so I see no reason not to add a Kconfig dependency that
reflects this until such a time where it's no longer true.

At least if there's a set of restrictions on something fairly generic,
they tend to be visible, and they also tend to get fixed up over time. We
should however not enable something generically which at the moment is
very much tied to a single platform. Later patches can remove the
dependency at such a time that that assertion no longer holds true.

Re: [PATCH -mm] video: uvesafb: Add X86 dependency.

[Michal Januszewski]

On Tue, Sep 11, 2007 at 09:31:59PM +0900, Paul Mundt wrote:

> > Anyway, I think it is up to Michal to decide if we should remove the 
> > kernel support for other archs, or let it stay a bit more while working 
> > on solving the v86d side of things. So I'll just step aside now....
> > 
> Once v86d is fixed up to get at the ROM directly and the driver uses MMIO
> directly, I don't see a problem with unrestricting it. For the time being
> however, the build is both broken, and the emulator it uses won't run on
> anything but x86, so I see no reason not to add a Kconfig dependency that
> reflects this until such a time where it's no longer true.
> 
> At least if there's a set of restrictions on something fairly generic,
> they tend to be visible, and they also tend to get fixed up over time. We
> should however not enable something generically which at the moment is
> very much tied to a single platform. Later patches can remove the
> dependency at such a time that that assertion no longer holds true.

Just to clear things up: yes, at the moment v86d supports only
x86 and amd64 (aka x86_64) and yes, supporting other arches is
possible and planned.  The main limiting factors as far as this
is concerned are the little amount of my free time and the fact
that I don't currently have access to non-x86 hardware.

Please note that the kernel part (i.e. uvesafb) is meant to be
generic (it currently uses VGA IO ports on non-x86, which is a
bug and for which a patch will follow) and support or lack thereof
for a specific arch should be dependent on v86d only.

That being said, I think that having a kernel dependency 
tracking the development status of userspace code is generally
a bad idea.

Best regards,
-- 
Michal Januszewski                              JID: spock@im.gentoo.org
Gentoo Linux Developer                    http://people.gentoo.org/spock

[PATCH -mm] uvesafb: Don't access VGA registers directly when running on non-x86

[Michal Januszewski]

The VGA registers are only available at their legacy IO locations on x86.
Don't try to access them when running on other arches.

Note that the code accessing them directly is just an optimization (limits
slow BIOS function calls).  We don't lose any functionality by using
BIOS calls instead of it on non-x86.

Signed-off-by: Michal Januszewski <spock@gentoo.org>
---
diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c
index 853323e..74fa7c7 100644
--- a/drivers/video/uvesafb.c
+++ b/drivers/video/uvesafb.c
@@ -935,6 +935,7 @@ static int uvesafb_setpalette(struct uvesafb_pal_entry *entries, int count,
 	if (start + count > 256)
 		return -EINVAL;
 
+#ifdef CONFIG_X86
 	/* 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) {
@@ -957,8 +958,10 @@ static int uvesafb_setpalette(struct uvesafb_pal_entry *entries, int count,
 		  "D" (entries),        /* EDI */
 		  "S" (&par->pmi_pal)); /* ESI */
 	}
-#endif
-	else {
+#endif /* CONFIG_X86_32 */
+	else
+#endif /* CONFIG_X86 */
+	{
 		task = uvesafb_prep();
 		if (!task)
 			return -ENOMEM;
@@ -1102,6 +1105,7 @@ static int uvesafb_blank(int blank, struct fb_info *info)
 	struct uvesafb_ktask *task;
 	int err = 1;
 
+#ifdef CONFIG_X86
 	if (par->vbe_ib.capabilities & VBE_CAP_VGACOMPAT) {
 		int loop = 10000;
 		u8 seq = 0, crtc17 = 0;
@@ -1124,7 +1128,9 @@ static int uvesafb_blank(int blank, struct fb_info *info)
 		while (loop--);
 		vga_wcrt(NULL, 0x17, crtc17);
 		vga_wseq(NULL, 0x00, 0x03);
-	} else {
+	} else
+#endif /* CONFIG_X86 */
+	{
 		task = uvesafb_prep();
 		if (!task)
 			return -ENOMEM;

Re: [PATCH -mm] uvesafb: Don't access VGA registers directly when running on non-x86

[Paul Mundt]

On Wed, Sep 12, 2007 at 01:09:59AM +0200, Michal Januszewski wrote:
> The VGA registers are only available at their legacy IO locations on x86.
> Don't try to access them when running on other arches.
> 
> Note that the code accessing them directly is just an optimization (limits
> slow BIOS function calls).  We don't lose any functionality by using
> BIOS calls instead of it on non-x86.
> 
If you do that, then you also have to #ifdef CONFIG_X86 around
video/vga.h, as that drags in asm/vga.h, which does not exist on all
platforms. I have little interest in adding a stub vga.h on my
architectures to support a driver that in practice works on nothing but
x86.

[PATCH -mm] uvesafb: Don't access VGA registers directly when running on non-x86

[Michal Januszewski]

The VGA registers are only available at their legacy IO locations on
x86. Don't try to access them when running on other arches.

Note that the code accessing them directly is just an optimization
(limits slow BIOS function calls).  We don't lose any functionality 
by using BIOS calls instead of it on non-x86.

Signed-off-by: Michal Januszewski <spock@gentoo.org>
---
diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c
index 853323e..b983d26 100644
--- a/drivers/video/uvesafb.c
+++ b/drivers/video/uvesafb.c
@@ -19,8 +19,10 @@
 #include <linux/io.h>
 #include <linux/mutex.h>
 #include <video/edid.h>
-#include <video/vga.h>
 #include <video/uvesafb.h>
+#ifdef CONFIG_X86
+#include <video/vga.h>
+#endif
 #ifdef CONFIG_MTRR
 #include <asm/mtrr.h>
 #endif
@@ -935,6 +937,7 @@ static int uvesafb_setpalette(struct uvesafb_pal_entry *entries, int count,
 	if (start + count > 256)
 		return -EINVAL;
 
+#ifdef CONFIG_X86
 	/* 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) {
@@ -957,8 +960,10 @@ static int uvesafb_setpalette(struct uvesafb_pal_entry *entries, int count,
 		  "D" (entries),        /* EDI */
 		  "S" (&par->pmi_pal)); /* ESI */
 	}
-#endif
-	else {
+#endif /* CONFIG_X86_32 */
+	else
+#endif /* CONFIG_X86 */
+	{
 		task = uvesafb_prep();
 		if (!task)
 			return -ENOMEM;
@@ -1102,6 +1107,7 @@ static int uvesafb_blank(int blank, struct fb_info *info)
 	struct uvesafb_ktask *task;
 	int err = 1;
 
+#ifdef CONFIG_X86
 	if (par->vbe_ib.capabilities & VBE_CAP_VGACOMPAT) {
 		int loop = 10000;
 		u8 seq = 0, crtc17 = 0;
@@ -1124,7 +1130,9 @@ static int uvesafb_blank(int blank, struct fb_info *info)
 		while (loop--);
 		vga_wcrt(NULL, 0x17, crtc17);
 		vga_wseq(NULL, 0x00, 0x03);
-	} else {
+	} else
+#endif /* CONFIG_X86 */
+	{
 		task = uvesafb_prep();
 		if (!task)
 			return -ENOMEM;

Re: [PATCH -mm] uvesafb: Don't access VGA registers directly when running on non-x86

[Paul Mundt]

On Wed, Sep 12, 2007 at 09:41:51PM +0200, Michal Januszewski wrote:
> The VGA registers are only available at their legacy IO locations on
> x86. Don't try to access them when running on other arches.
> 
> Note that the code accessing them directly is just an optimization
> (limits slow BIOS function calls).  We don't lose any functionality 
> by using BIOS calls instead of it on non-x86.
> 
> Signed-off-by: Michal Januszewski <spock@gentoo.org>

Looks fine, thanks Michal.