> Date: Tue, 13 Aug 2013 07:13:43 +0200
> From: Egbert Eich <e...@freedesktop.org>
> 
> On Mon, Aug 12, 2013 at 08:58:37PM +0200, Mark Kettenis wrote:
> > > From: Egbert Eich <e...@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 <a...@redhat.com>
> > >   Date:   Tue Sep 20 18:12:29 2011 -0400
> > > 
> > >     vgahw: Port to pciaccess IO space routines
> > > 
> > >     Reviewed-by: Jeremy Huddleston <jerem...@apple.com>
> > >     Tested-by: Jeremy Huddleston <jerem...@apple.com>
> > >     Signed-off-by: Adam Jackson <a...@redhat.com>
> > >     Reviewed-by: Jamey Sharp <ja...@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__.
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to