[PATCH] vgahw: Resurrect disabling of legacy VGA support

[Mark Kettenis]

> Date: Tue, 13 Aug 2013 07:13:43 +0200
> From: Egbert Eich <eich at freedesktop.org>
> 
> On Mon, Aug 12, 2013 at 08:58:37PM +0200, Mark Kettenis wrote:
> > > From: Egbert Eich <eich at freedesktop.org>
> > > Date: Mon, 12 Aug 2013 18:24:47 +0200
> > > 
> > > DACDelay() used to be defined differently on platforms
> > > that don't have legacy VGA support.
> > > 
> > > This distinction was removed with
> > > 
> > >   commit 6d9efdce0d06df6b85f0681bea306c0b1e851502
> > >   Author: Adam Jackson <ajax at redhat.com>
> > >   Date:   Tue Sep 20 18:12:29 2011 -0400
> > > 
> > >     vgahw: Port to pciaccess IO space routines
> > > 
> > >     Reviewed-by: Jeremy Huddleston <jeremyhu at apple.com>
> > >     Tested-by: Jeremy Huddleston <jeremyhu at apple.com>
> > >     Signed-off-by: Adam Jackson <ajax at redhat.com>
> > >     Reviewed-by: Jamey Sharp <jamey at minilop.net>
> > > 
> > > which has produced crashes at least on PowerPC.
> > 
> > Which driver uses DACDelay(), but doesn't actually need working VGA
> > legacy support?
> 
> Mark, this is exactly the question that I've asked the reporter 
> when he told me that this code was needed. According to him the 
> hardware is nv on ppc64. Apparently DACDelay() causes an MC when 
> the memory that provides access to the legacy PIO address of ST01 
> is read as there is no response.
> I've got the same feeling that this code is papering over an issue 
> that sits elsewhere. But there's nothing immediately obvious I can 
> find in this driver. Due to lack of hardware I'm unable to do
> further investigations myself. So the only thing I can do here is 
> to post the patch here and relay back that my concerns are shared 
> by others.

There defenitely is an issue with the driver here.  For one thing, the
following bit:

#if defined(__powerpc__)
    vgaHWSetMmioFuncs(VGAHWPTR(pScrn), (CARD8 *)pNv->IOAddress, 0);
#endif

in nv_driver.c doesn't really make any sense.  Shortly afterwards,
NVCommonSetup() gets called which overrides most of the VGA I/O
routines and hijacks MMIOBase.  What probably needs to be done is to
make sure NVCommonSetup() also overrides ->readST01.  However, there
are potential issues with making ->readST01 use MMIO, as the timing
will become different, and DACDelay() might therefore not have the
intended effect.  It's not clear whether NVIDIA hardware needs the
delays when accessing the palette registers.  So perhaps the safest
thing to do would be to override ->readST01 only #ifdef __powerpc__.