Skip to contentSkip to conversation

video: uvesafb: Add X86 dependency.

PATCH In review Paul Mundt 3 participants 10 messages Sep 11, 2007 – Sep 13, 2007 Framebuffer layer

lore
Share & tools
lore

Paul Mundt

author
#0 lore raw

PATCH 1/1 video: uvesafb: Add X86 dependency.

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

--

 drivers/video/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/video/Kconfig +1 -1
@@ -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
#1 lore raw
Quoted 1 line · Paul Mundt

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?

Signature · 5 lines
Paulo Marques - www.grupopie.com

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

[1] http://dev.gentoo.org/~spock/projects/uvesafb/
author
#2 lore raw
Quoted 12 lines · Paulo Marques
Quoted 1 line · Paul Mundt

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.

#3 lore raw
Quoted 10 lines · Paul Mundt
Quoted 3 lines · Paulo Marques

[...]
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....

Signature · 3 lines
Paulo Marques - www.grupopie.com

"667: The neighbor of the beast."
author
#4 lore raw
Quoted 19 lines · Paulo Marques
Quoted 10 lines · Paul Mundt
Quoted 3 lines · Paulo Marques

[...]
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.

Quoted 7 lines

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.

#5 lore raw
Quoted 10 lines · Paul Mundt
Quoted 4 lines

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.

Quoted 5 lines

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,

Signature · 2 lines
Michal Januszewski                              JID: [email protected]
Gentoo Linux Developer                    http://people.gentoo.org/spock
#6 lore raw

PATCH 1/1 uvesafb: Don't access VGA registers directly when running on non-x86

In reply to Paul Mundt

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.

---

drivers/video/uvesafb.c +9 -3
index 853323e..74fa7c7 100644
@@ -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;
author
#7 lore raw

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

Quoted 7 lines · 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.

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.

#8 lore raw

PATCH 1/1 uvesafb: Don't access VGA registers directly when running on non-x86

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.

---

drivers/video/uvesafb.c +12 -4
index 853323e..b983d26 100644
@@ -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;
author
#9 lore raw

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

Quoted 8 lines · 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.

Looks fine, thanks Michal.