[PATCH 2/9] int10: Port to pciaccess' legacy IO API
Tiago Vignatti
tiago.vignatti at nokia.com
Wed Sep 22 05:47:11 PDT 2010
On Wed, Sep 22, 2010 at 01:28:32AM +0200, ext Adam Jackson wrote:
> Signed-off-by: Adam Jackson <ajax at redhat.com>
> ---
> hw/xfree86/int10/generic.c | 5 ++-
> hw/xfree86/int10/helper_exec.c | 36 ++++++++++++------------
> hw/xfree86/int10/xf86int10.c | 61 +++++++++++++++++++---------------------
> hw/xfree86/int10/xf86int10.h | 2 +-
> 4 files changed, 52 insertions(+), 52 deletions(-)
>
> diff --git a/hw/xfree86/int10/generic.c b/hw/xfree86/int10/generic.c
> index fe8bb69..aa39db5 100644
> --- a/hw/xfree86/int10/generic.c
> +++ b/hw/xfree86/int10/generic.c
> @@ -309,7 +309,7 @@ MapVRam(xf86Int10InfoPtr pInt)
> INTPriv(pInt)->vRam = xf86MapDomainMemory(pInt->scrnIndex, VIDMEM_MMIO,
> pInt->dev, V_RAM, size);
>
> - pInt->ioBase = xf86Screens[pInt->scrnIndex]->domainIOBase;
> + pInt->io = pci_legacy_open_io(pInt->dev, 0, 64 * 1024);
so you could already remove this field (domainIOBase) usage in xf86Bus.c
(inside xf86AddEntityToScreen), in this commit.
> }
>
> static void
> @@ -320,6 +320,9 @@ UnmapVRam(xf86Int10InfoPtr pInt)
> int size = ((VRAM_SIZE + pagesize - 1)/pagesize) * pagesize;
>
> xf86UnMapVidMem(screen, INTPriv(pInt)->vRam, size);
> +
> + pci_device_close_io(pInt->dev, pInt->io);
> + pInt->io = NULL;
vm86 does also some bizarre vram mapping. Should we port it to use
pci_legacy_open interface also? Or we're decided already to kill it forever:
http://lists.x.org/archives/xorg-devel/2010-July/011163.html
Even if x86emu wouldn't be covering all vm86 role (yet), I do really like
when, on that thread, mjg mentioned about the incentives and motivation to fix
it that tends to zero. So, I'd say I'm in favour to delete such backend and
concentrate on x86emu only from now.
(sorry to be offtopic a bit here)
> Bool
> diff --git a/hw/xfree86/int10/helper_exec.c b/hw/xfree86/int10/helper_exec.c
> index 8f47afe..da98da4 100644
> --- a/hw/xfree86/int10/helper_exec.c
> +++ b/hw/xfree86/int10/helper_exec.c
> @@ -330,7 +330,7 @@ x_inb(CARD16 port)
> }
> #endif /* __NOT_YET__ */
> } else if (!pciCfg1inb(port, &val)) {
> - val = inb(Int10Current->ioBase + port);
> + val = pci_io_read8(Int10Current->io, port);
> if (PRINT_PORT && DEBUG_IO_TRACE())
> ErrorF(" inb(%#x) = %2.2x\n", port, val);
> }
> @@ -352,7 +352,7 @@ x_inw(CARD16 port)
> X_GETTIMEOFDAY(&tv);
> val = (CARD16)(tv.tv_usec / 3);
> } else if (!pciCfg1inw(port, &val)) {
> - val = inw(Int10Current->ioBase + port);
> + val = pci_io_read16(Int10Current->io, port);
> if (PRINT_PORT && DEBUG_IO_TRACE())
> ErrorF(" inw(%#x) = %4.4x\n", port, val);
> }
> @@ -386,7 +386,7 @@ x_outb(CARD16 port, CARD8 val)
> } else if (!pciCfg1outb(port, val)) {
> if (PRINT_PORT && DEBUG_IO_TRACE())
> ErrorF(" outb(%#x, %2.2x)\n", port, val);
> - outb(Int10Current->ioBase + port, val);
> + pci_io_write8(Int10Current->io, port, val);
> }
> }
>
> @@ -397,7 +397,7 @@ x_outw(CARD16 port, CARD16 val)
> if (!pciCfg1outw(port, val)) {
> if (PRINT_PORT && DEBUG_IO_TRACE())
> ErrorF(" outw(%#x, %4.4x)\n", port, val);
> - outw(Int10Current->ioBase + port, val);
> + pci_io_write16(Int10Current->io, port, val);
is the indentation above "correct"?
> }
> }
>
> @@ -407,7 +407,7 @@ x_inl(CARD16 port)
> CARD32 val;
>
> if (!pciCfg1in(port, &val)) {
> - val = inl(Int10Current->ioBase + port);
> + val = pci_io_read32(Int10Current->io, port);
> if (PRINT_PORT && DEBUG_IO_TRACE())
> ErrorF(" inl(%#x) = %8.8lx\n", port, val);
> }
> @@ -420,7 +420,7 @@ x_outl(CARD16 port, CARD32 val)
> if (!pciCfg1out(port, val)) {
> if (PRINT_PORT && DEBUG_IO_TRACE())
> ErrorF(" outl(%#x, %8.8lx)\n", port, val);
> - outl(Int10Current->ioBase + port, val);
> + pci_io_write32(Int10Current->io, port, val);
> }
> }
>
> @@ -644,29 +644,29 @@ bios_checksum(const CARD8 *start, int size)
> void
> LockLegacyVGA(xf86Int10InfoPtr pInt, legacyVGAPtr vga)
> {
> - vga->save_msr = inb(pInt->ioBase + 0x03CC);
> - vga->save_vse = inb(pInt->ioBase + 0x03C3);
> + vga->save_msr = pci_io_read8(pInt->io, 0x03CC);
> + vga->save_vse = pci_io_read8(pInt->io, 0x03C3);
> #ifndef __ia64__
> - vga->save_46e8 = inb(pInt->ioBase + 0x46E8);
> + vga->save_46e8 = pci_io_read8(pInt->io, 0x46E8);
> #endif
> - vga->save_pos102 = inb(pInt->ioBase + 0x0102);
> - outb(pInt->ioBase + 0x03C2, ~(CARD8)0x03 & vga->save_msr);
> - outb(pInt->ioBase + 0x03C3, ~(CARD8)0x01 & vga->save_vse);
> + vga->save_pos102 = pci_io_read8(pInt->io, 0x0102);
> + pci_io_write8(pInt->io, 0x03C2, ~(CARD8)0x03 & vga->save_msr);
> + pci_io_write8(pInt->io, 0x03C3, ~(CARD8)0x01 & vga->save_vse);
> #ifndef __ia64__
> - outb(pInt->ioBase + 0x46E8, ~(CARD8)0x08 & vga->save_46e8);
> + pci_io_write8(pInt->io, 0x46E8, ~(CARD8)0x08 & vga->save_46e8);
> #endif
> - outb(pInt->ioBase + 0x0102, ~(CARD8)0x01 & vga->save_pos102);
> + pci_io_write8(pInt->io, 0x0102, ~(CARD8)0x01 & vga->save_pos102);
> }
>
> void
> UnlockLegacyVGA(xf86Int10InfoPtr pInt, legacyVGAPtr vga)
> {
> - outb(pInt->ioBase + 0x0102, vga->save_pos102);
> + pci_io_write8(pInt->io, 0x0102, vga->save_pos102);
> #ifndef __ia64__
> - outb(pInt->ioBase + 0x46E8, vga->save_46e8);
> + pci_io_write8(pInt->io, 0x46E8, vga->save_46e8);
> #endif
> - outb(pInt->ioBase + 0x03C3, vga->save_vse);
> - outb(pInt->ioBase + 0x03C2, vga->save_msr);
> + pci_io_write8(pInt->io, 0x03C3, vga->save_vse);
> + pci_io_write8(pInt->io, 0x03C2, vga->save_msr);
> }
>
> #if defined (_PC)
> diff --git a/hw/xfree86/int10/xf86int10.c b/hw/xfree86/int10/xf86int10.c
> index 51eb91f..dd00e54 100644
> --- a/hw/xfree86/int10/xf86int10.c
> +++ b/hw/xfree86/int10/xf86int10.c
> @@ -84,7 +84,7 @@ int42_handler(xf86Int10InfoPtr pInt)
> /* Leave: Nothing */
> /* Implemented (except for clearing the screen) */
> { /* Localise */
> - IOADDRESS ioport;
> + unsigned int ioport;
> int i;
> CARD16 int1d, regvals, tmp;
> CARD8 mode, cgamode, cgacolour;
> @@ -172,18 +172,15 @@ int42_handler(xf86Int10InfoPtr pInt)
> /* Rows */
> MEM_WB(pInt, 0x0484, (25 - 1));
>
> - /* Remap I/O port number into its domain */
> - ioport += pInt->ioBase;
> -
> - /* Programme the mode */
> - outb(ioport + 4, cgamode & 0x37); /* Turn off screen */
> + /* Program the mode */
> + pci_io_write8(pInt->io, ioport + 4, cgamode & 0x37); /* Turn off screen */
> for (i = 0; i < 0x10; i++) {
> tmp = MEM_RB(pInt, regvals + i);
> - outb(ioport, i);
> - outb(ioport + 1, tmp);
> + pci_io_write8(pInt->io, ioport, i);
> + pci_io_write8(pInt->io, ioport + 1, tmp);
> }
> - outb(ioport + 5, cgacolour); /* Select colour mode */
> - outb(ioport + 4, cgamode); /* Turn on screen */
> + pci_io_write8(pInt->io, ioport + 5, cgacolour); /* Select colour mode */
> + pci_io_write8(pInt->io, ioport + 4, cgamode); /* Turn on screen */
> }
> break;
>
> @@ -194,15 +191,15 @@ int42_handler(xf86Int10InfoPtr pInt)
> /* Leave: Nothing */
> /* Implemented */
> { /* Localise */
> - IOADDRESS ioport = MEM_RW(pInt, 0x0463) + pInt->ioBase;
> + unsigned int ioport = MEM_RW(pInt, 0x0463);
would be worth to point out (and comment on the code) why you're using
unsigned int type. It's not clear for me.
>
> MEM_WB(pInt, 0x0460, X86_CL);
> MEM_WB(pInt, 0x0461, X86_CH);
>
> - outb(ioport, 0x0A);
> - outb(ioport + 1, X86_CH);
> - outb(ioport, 0x0B);
> - outb(ioport + 1, X86_CL);
> + pci_io_write8(pInt->io, ioport, 0x0A);
> + pci_io_write8(pInt->io, ioport + 1, X86_CH);
> + pci_io_write8(pInt->io, ioport, 0x0B);
> + pci_io_write8(pInt->io, ioport + 1, X86_CL);
> }
> break;
>
> @@ -214,7 +211,7 @@ int42_handler(xf86Int10InfoPtr pInt)
> /* Leave: Nothing */
> /* Implemented */
> { /* Localise */
> - IOADDRESS ioport;
> + unsigned int ioport;
same here.
> CARD16 offset;
>
> MEM_WB(pInt, (X86_BH << 1) + 0x0450, X86_DL);
> @@ -226,11 +223,11 @@ int42_handler(xf86Int10InfoPtr pInt)
> offset = (X86_DH * MEM_RW(pInt, 0x044A)) + X86_DL;
> offset += MEM_RW(pInt, 0x044E) << 1;
>
> - ioport = MEM_RW(pInt, 0x0463) + pInt->ioBase;
> - outb(ioport, 0x0E);
> - outb(ioport + 1, offset >> 8);
> - outb(ioport, 0x0F);
> - outb(ioport + 1, offset & 0xFF);
> + ioport = MEM_RW(pInt, 0x0463);
> + pci_io_write8(pInt->io, ioport, 0x0E);
> + pci_io_write8(pInt->io, ioport + 1, offset >> 8);
> + pci_io_write8(pInt->io, ioport, 0x0F);
> + pci_io_write8(pInt->io, ioport + 1, offset & 0xFF);
> }
> break;
>
> @@ -276,7 +273,7 @@ int42_handler(xf86Int10InfoPtr pInt)
> /* Leave: Nothing */
> /* Implemented */
> { /* Localise */
> - IOADDRESS ioport = MEM_RW(pInt, 0x0463) + pInt->ioBase;
> + unsigned int ioport = MEM_RW(pInt, 0x0463);
> CARD16 start;
> CARD8 x, y;
>
> @@ -287,10 +284,10 @@ int42_handler(xf86Int10InfoPtr pInt)
> start <<= 1;
>
> /* Update start address */
> - outb(ioport, 0x0C);
> - outb(ioport + 1, start >> 8);
> - outb(ioport, 0x0D);
> - outb(ioport + 1, start & 0xFF);
> + pci_io_write8(pInt->io, ioport, 0x0C);
> + pci_io_write8(pInt->io, ioport + 1, start >> 8);
> + pci_io_write8(pInt->io, ioport, 0x0D);
> + pci_io_write8(pInt->io, ioport + 1, start & 0xFF);
>
> /* Switch cursor position */
> y = MEM_RB(pInt, (X86_AL << 1) + 0x0450);
> @@ -298,10 +295,10 @@ int42_handler(xf86Int10InfoPtr pInt)
> start += (y * MEM_RW(pInt, 0x044A)) + x;
>
> /* Update cursor position */
> - outb(ioport, 0x0E);
> - outb(ioport + 1, start >> 8);
> - outb(ioport, 0x0F);
> - outb(ioport + 1, start & 0xFF);
> + pci_io_write8(pInt->io, ioport, 0x0E);
> + pci_io_write8(pInt->io, ioport + 1, start >> 8);
> + pci_io_write8(pInt->io, ioport, 0x0F);
> + pci_io_write8(pInt->io, ioport + 1, start & 0xFF);
> }
> break;
>
> @@ -426,7 +423,7 @@ int42_handler(xf86Int10InfoPtr pInt)
> /* Leave: Nothing */
> /* Implemented */
> { /* Localise */
> - IOADDRESS ioport = MEM_RW(pInt, 0x0463) + 5 + pInt->ioBase;
> + unsigned int ioport = MEM_RW(pInt, 0x0463) + 5;
> CARD8 cgacolour = MEM_RB(pInt, 0x0466);
>
> if (X86_BH) {
> @@ -438,7 +435,7 @@ int42_handler(xf86Int10InfoPtr pInt)
> }
>
> MEM_WB(pInt, 0x0466, cgacolour);
> - outb(ioport, cgacolour);
> + pci_io_write8(pInt->io, ioport, cgacolour);
> }
> break;
>
> diff --git a/hw/xfree86/int10/xf86int10.h b/hw/xfree86/int10/xf86int10.h
> index ba9ee52..5bf326e 100644
> --- a/hw/xfree86/int10/xf86int10.h
> +++ b/hw/xfree86/int10/xf86int10.h
> @@ -41,7 +41,7 @@ typedef struct {
> int flags;
> int stackseg;
> struct pci_device *dev;
> - IOADDRESS ioBase;
> + struct pci_io_handle *io;
> } xf86Int10InfoRec, *xf86Int10InfoPtr;
>
> typedef struct _int10Mem {
I hope those new io pci interface are okay in the library side (last I check
they were, but seems not used so far). So for sure this kind of changes are
coming for good.
Reviewed-by: Tiago Vignatti <tiago.vignatti at nokia.com>
Anyway, I'd like to hear some feedback from you about my semi-nitpicking I
commented above.
Thanks,
Tiago
More information about the xorg-devel
mailing list