[PATCH 2/9] int10: Port to pciaccess' legacy IO API

[Adam Jackson]

On Wed, 2010-09-22 at 15:47 +0300, Tiago Vignatti wrote:
> On Wed, Sep 22, 2010 at 01:28:32AM +0200, ext Adam Jackson wrote:
> > --- 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.

Could, but I was trying to keep the ABI breakage confined to as few
commits as possible.  Doesn't really matter I suppose.

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

The vm86 backend needs to die.  End of story.  All the major Linux
distros have been shipping x86emu everywhere since mid-2007, and vm86
hasn't ever worked on any of the other OSes.

> > @@ -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"?

It's whatever my editor came up with.  It's not like we're consistent.

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

Just to make the typedef go away.  MEM_RW will return uint16_t so it's
not like there's a sign extension bug to worry about.

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

That's a fair point, actually.  The only implemented backend for
pci_open_legacy_io in libpciaccess is for linux sysfs.  I thought that
had been implemented for other OSes but apparently not yet.  I should
probably at least attempt to port that in for the rest of the world, it
won't be much more than open(/dev/port) or iopl.

- ajax
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg-devel/attachments/20100922/3f3008b8/attachment.pgp>