ast2400 woes

ast2400 woes

[Benjamin Herrenschmidt]

Hi Dave !

So your AST driver in -next is blowing up on my power8 box :-)

There are several issues that I want to discuss here (YC Chen on CC
might also have some valuable input here).

* First, accessors. The first obvious cause for blowing up for me is
that you are using the "old style" PIO offsets for these guys:

#define AST_IO_AR_PORT_WRITE            (0x40)
#define AST_IO_MISC_PORT_WRITE          (0x42)
#define AST_IO_SEQ_PORT                 (0x44)
#define AST_DAC_INDEX_READ              (0x3c7)
#define AST_IO_DAC_INDEX_WRITE          (0x48)
#define AST_IO_DAC_DATA                 (0x49)
#define AST_IO_GR_PORT                  (0x4E)
#define AST_IO_CRTC_PORT                (0x54)
#define AST_IO_INPUT_STATUS1_READ       (0x5A)
#define AST_IO_MISC_PORT_READ           (0x4C)

(And accessing them via PIO)

And I don't have PIO on that platform at all :-)

I tried using MMIO, but the above offsets don't work.

Those things are accessible via the MMIO BAR which is much better via
these offsets:

#define AR_PORT_WRITE		(pAST->MMIOVirtualAddr + 0x3c0)
#define MISC_PORT_WRITE		(pAST->MMIOVirtualAddr + 0x3c2)
#define VGA_ENABLE_PORT		(pAST->MMIOVirtualAddr + 0x3c3)
#define SEQ_PORT		(pAST->MMIOVirtualAddr + 0x3c4)
#define DAC_INDEX_READ		(pAST->MMIOVirtualAddr + 0x3c7)
#define DAC_INDEX_WRITE		(pAST->MMIOVirtualAddr + 0x3c8)
#define DAC_DATA		(pAST->MMIOVirtualAddr + 0x3c9)
#define GR_PORT			(pAST->MMIOVirtualAddr + 0x3cE)
#define CRTC_PORT		(pAST->MMIOVirtualAddr + 0x3d4)
#define INPUT_STATUS1_READ	(pAST->MMIOVirtualAddr + 0x3dA)
#define MISC_PORT_READ		(pAST->MMIOVirtualAddr + 0x3cc)

(From the X driver).

The spec is pretty tricky to read but seems to indicate that the above
offset should also work for PIO if needed, however, it seems like the
X driver is pretty happy to use MMIO unconditionally for them.

Any objection on me sending you a patch to send (almost) everybody to
use the MMIO path ?

The only remaining "issues" with PIO is the EnableVGA / IsVGAEnabled
path which still uses PIO in X.

Now, at least on the AST2400, the register in question is also on MMIO
(3c3, aka VGA_ENABLE_PORT in the above list), but I don't know whether
that works on all the older chipsets. (YC Chen on CC might have an opinion).

* Then, I notice that you only POST the chip in the "thaw" path which
as far as I can tell is only called on resume from sleep, am I correct ?

We don't have a BIOS so we rely on the kernel driver doing the full init.
For example we come up with VGA disabled, so the driver must enable it
(via 3c3 MMIO !) first thing first. The current probe code will just blow
up even if adjusted to use the MMIO offsets due to trying to access the
CRTC registers when VGA is disabled.

This leads to the question, mostly for YC Chen I suppose: Is there a way
to detect that the chip has been POSTed already by the BIOS or not ?

Should we key that off VGA Enabled being 0 ?

Cheers,
Ben.

ast2400 woes

[Benjamin Herrenschmidt]

Hi Dave !

So your AST KMS driver in -next is blowing up on my power8 box :-)

There are several issues that I want to discuss here (YC Chen on CC
might also have some valuable input here) before I send you patches
to fix it :-)

* First, accessors. The first obvious cause for blowing up for me is
that you are using the "old style" PIO offsets for these guys:

#define AST_IO_AR_PORT_WRITE            (0x40)
#define AST_IO_MISC_PORT_WRITE          (0x42)
#define AST_IO_SEQ_PORT                 (0x44)
#define AST_DAC_INDEX_READ              (0x3c7)
#define AST_IO_DAC_INDEX_WRITE          (0x48)
#define AST_IO_DAC_DATA                 (0x49)
#define AST_IO_GR_PORT                  (0x4E)
#define AST_IO_CRTC_PORT                (0x54)
#define AST_IO_INPUT_STATUS1_READ       (0x5A)
#define AST_IO_MISC_PORT_READ           (0x4C)

(And accessing them via PIO)

And I don't have PIO on that platform at all :-)

I tried using MMIO, but the above offsets don't work.

Those things are accessible via the MMIO BAR which is much better via
these offsets:

#define AR_PORT_WRITE		(pAST->MMIOVirtualAddr + 0x3c0)
#define MISC_PORT_WRITE		(pAST->MMIOVirtualAddr + 0x3c2)
#define VGA_ENABLE_PORT		(pAST->MMIOVirtualAddr + 0x3c3)
#define SEQ_PORT		(pAST->MMIOVirtualAddr + 0x3c4)
#define DAC_INDEX_READ		(pAST->MMIOVirtualAddr + 0x3c7)
#define DAC_INDEX_WRITE		(pAST->MMIOVirtualAddr + 0x3c8)
#define DAC_DATA		(pAST->MMIOVirtualAddr + 0x3c9)
#define GR_PORT			(pAST->MMIOVirtualAddr + 0x3cE)
#define CRTC_PORT		(pAST->MMIOVirtualAddr + 0x3d4)
#define INPUT_STATUS1_READ	(pAST->MMIOVirtualAddr + 0x3dA)
#define MISC_PORT_READ		(pAST->MMIOVirtualAddr + 0x3cc)

(From the X driver).

The spec is pretty tricky to read but seems to indicate that the above
offset should also work for PIO if needed, however, it seems like the
X driver is pretty happy to use MMIO unconditionally for them.

Any objection on me sending you a patch to send (almost) everybody to
use the MMIO path ?

The only remaining "issues" with PIO is the EnableVGA / IsVGAEnabled
path which still uses PIO in X.

Now, at least on the AST2400, the register in question is also on MMIO
(3c3, aka VGA_ENABLE_PORT in the above list), but I don't know whether
that works on all the older chipsets. (YC Chen on CC might have an opinion).

* Then, I notice that you only POST the chip in the "thaw" path which
as far as I can tell is only called on resume from sleep, am I correct ?

We don't have a BIOS so we rely on the kernel driver doing the full init.
For example we come up with VGA disabled, so the driver must enable it
(via 3c3 MMIO !) first thing first. The current probe code will just blow
up even if adjusted to use the MMIO offsets due to trying to access the
CRTC registers when VGA is disabled.

This leads to the question, mostly for YC Chen I suppose: Is there a way
to detect that the chip has been POSTed already by the BIOS or not ?

Should we key that off VGA Enabled being 0 ?

Cheers,
Ben.

Re: ast2400 woes

[Benjamin Herrenschmidt]

On Fri, 2014-06-06 at 21:31 +1000, Benjamin Herrenschmidt wrote:

> The spec is pretty tricky to read but seems to indicate that the above
> offset should also work for PIO if needed, however, it seems like the
> X driver is pretty happy to use MMIO unconditionally for them.
> 
> Any objection on me sending you a patch to send (almost) everybody to
> use the MMIO path ?
> 
> The only remaining "issues" with PIO is the EnableVGA / IsVGAEnabled
> path which still uses PIO in X.
> 
> Now, at least on the AST2400, the register in question is also on MMIO
> (3c3, aka VGA_ENABLE_PORT in the above list), but I don't know whether
> that works on all the older chipsets. (YC Chen on CC might have an opinion).

Ok, I think we need YC Chen answers here. Basically from what I can
tell those old "IO" registers and those new "MMIO" ones only differ
by that offset of 0x340.

The question thus boils down to:

 - Are the "3xx" versions only MMIO or PIO as well ?

 - Are the "3xx" version always available on all chips ?

 - Is MMIO always available on all chips ?

IE. Is there a reason why bASTIsVGAEnabled() and vASTEnableVGAMMIO
use the IO ports ? The latter reads 0x43 and writes 0x43 and 0x42,
can it be made to always use MMIO 0x3c3 and write 0x3c3 and 0x3c2 ?

On my AST2400 at least, even when MMIO is disabled, 0x3c3 still
responds so it works but is that valid for all chips ? Or do I need
to favor the PIO path if PIO is available in that case for older
chipsets ?

Thanks !

Cheers,
Ben.

Re: ast2400 woes

[Benjamin Herrenschmidt]

On Sat, 2014-06-07 at 09:20 +1000, Benjamin Herrenschmidt wrote:

> IE. Is there a reason why bASTIsVGAEnabled() and vASTEnableVGAMMIO
> use the IO ports ? The latter reads 0x43 and writes 0x43 and 0x42,
> can it be made to always use MMIO 0x3c3 and write 0x3c3 and 0x3c2 ?
> 
> On my AST2400 at least, even when MMIO is disabled, 0x3c3 still
> responds so it works but is that valid for all chips ? Or do I need
> to favor the PIO path if PIO is available in that case for older
> chipsets ?

Note: I have it working now with a couple of patches that i'll send
when I've cleaned them up, though I still need answers to the earlier
questions so we can make sure we don't break earlier chipset support
on x86.

However, YC, the Endian control bits in extended CRTC register A2 seem
to have no effect at all. With a big endian kernel I get the wrong
endian on graphics regardless of the setting of that register !

Is endian swapping supported on the AST2400 ?

Also what is the exact effect of that register ? Does it affect access
from PCI to the framebuffer or does it affect the way the CRTC consumes
pixels from the framebuffer ? Is is supposed to have an effect on
register accesses ?

Cheers,
Ben.

RE: ast2400 woes

[YC Chen]

Hi Benjamin,
After confirm with our h/w designer, ast2400 did not support big-endian as you said. We support it in our previous product(PCI revision < 0x20) for frame buffer access. So, the possible solution is made in sw level for big-endian support. May we know the impact if we did not support it in h/w design? We will consider to support it in next generation.
The default MMIO is enabled since the products PCI revision >= 0x20. The possible solution is check the value of 0x3c3 through MMIO. If the value is 0xFF, then you must enable it through PCI IO.

Regards,

Y.C. Chen

-----Original Message-----
From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org] 
Sent: Saturday, June 07, 2014 3:16 PM
To: Dave Airlie
Cc: dri-devel@lists.freedesktop.org; YC Chen
Subject: Re: ast2400 woes

On Sat, 2014-06-07 at 09:20 +1000, Benjamin Herrenschmidt wrote:

> IE. Is there a reason why bASTIsVGAEnabled() and vASTEnableVGAMMIO use 
> the IO ports ? The latter reads 0x43 and writes 0x43 and 0x42, can it 
> be made to always use MMIO 0x3c3 and write 0x3c3 and 0x3c2 ?
> 
> On my AST2400 at least, even when MMIO is disabled, 0x3c3 still 
> responds so it works but is that valid for all chips ? Or do I need to 
> favor the PIO path if PIO is available in that case for older chipsets 
> ?

Note: I have it working now with a couple of patches that i'll send when I've cleaned them up, though I still need answers to the earlier questions so we can make sure we don't break earlier chipset support on x86.

However, YC, the Endian control bits in extended CRTC register A2 seem to have no effect at all. With a big endian kernel I get the wrong endian on graphics regardless of the setting of that register !

Is endian swapping supported on the AST2400 ?

Also what is the exact effect of that register ? Does it affect access from PCI to the framebuffer or does it affect the way the CRTC consumes pixels from the framebuffer ? Is is supposed to have an effect on register accesses ?

Cheers,
Ben.

Re: ast2400 woes

[Benjamin Herrenschmidt]

On Mon, 2014-06-09 at 02:41 +0000, YC Chen wrote:
> Hi Benjamin,
> After confirm with our h/w designer, ast2400 did not support
> big-endian as you said. We support it in our previous product(PCI
> revision < 0x20) for frame buffer access. So, the possible solution is
> made in sw level for big-endian support. May we know the impact if we
> did not support it in h/w design? We will consider to support it in
> next generation.

So the open power products are migrating toward mostly little endian
environments where the problem is not present. But we still have a
strong big endian legacy which would be nice to support.

SW swapping is not a simple thing to do. A large part of the problem is
that the X server (and the software stack above it mostly) have the
native pixel format defined at compile time and is always BE on BE
machines. So swapping requires hooks in various places and reduce
performance. The two main cases are:

 - "Modern" distros such as fedora don't use the old style X DDX. They
use the KMS Linux driver and the "modesetting" DDX which is
unaccelerated and uses ShadowFB. We can add a swapping hook into there,
but it will result in an even slower setup (and it's already pretty slow
as it is). 

 - The old DDX relies on the HW swapping. X will always draw in native
format and since the old DDX draws directly into the FB, all
non-accelerated accesses would have to be wrapped and swapped. It's even
harder to do than the KMS case since it's not a single point that needs
hooking and all fb ops must be wrapped.

My recommendation here for future HW if you want to consider bringing BE
support back in is to not swap at the PCIe interface level however.
Leave that alone..

Simply provide a way to define the pixel format to be BE on the output
pipe (CRTC) and 2D engine. IE. When the pixels are *consumed* from
memory by the chip, not on the path between PCI and memory.

PCIe-level swapping doesn't work well from experience, especially when
manipulating objects of different bit depth in the same fb such as
32-bit pixmaps and 8-bit alpha masks.

> The default MMIO is enabled since the products PCI revision >= 0x20.
> The possible solution is check the value of 0x3c3 through MMIO. If the
> value is 0xFF, then you must enable it through PCI IO.

Will the value be 0xff because that's what the chip responds or will the
chip not respond which *happens* to return 0xff on some x86 chipsets ?
(ie, target abort).

Because in the latter case, this will cause errors ranging from machine
checks to EEH freeze on powerpc platforms. In fact, some x86 platforms
are also configured to not ignore PCI errors and will fail in nasty ways
with such a procedure.

Can you confirm which specific bit of which register will enable MMIO ?
What I can do is do that whenever IO space has been properly allocated
(ie, is supported by the platform) and assume that MMIO is already
enabled on platforms that don't support IO.

Also, about the 0x380 difference in offset between IO and MMIO, is that
always like that ? The documentation doesn't mention this at all...

Cheers,
Ben.

> Regards,
> 
> Y.C. Chen
> 
> -----Original Message-----
> From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org] 
> Sent: Saturday, June 07, 2014 3:16 PM
> To: Dave Airlie
> Cc: dri-devel@lists.freedesktop.org; YC Chen
> Subject: Re: ast2400 woes
> 
> On Sat, 2014-06-07 at 09:20 +1000, Benjamin Herrenschmidt wrote:
> 
> > IE. Is there a reason why bASTIsVGAEnabled() and vASTEnableVGAMMIO
> use 
> > the IO ports ? The latter reads 0x43 and writes 0x43 and 0x42, can
> it 
> > be made to always use MMIO 0x3c3 and write 0x3c3 and 0x3c2 ?
> > 
> > On my AST2400 at least, even when MMIO is disabled, 0x3c3 still 
> > responds so it works but is that valid for all chips ? Or do I need
> to 
> > favor the PIO path if PIO is available in that case for older
> chipsets 
> > ?
> 
> Note: I have it working now with a couple of patches that i'll send
> when I've cleaned them up, though I still need answers to the earlier
> questions so we can make sure we don't break earlier chipset support
> on x86.
> 
> However, YC, the Endian control bits in extended CRTC register A2 seem
> to have no effect at all. With a big endian kernel I get the wrong
> endian on graphics regardless of the setting of that register !
> 
> Is endian swapping supported on the AST2400 ?
> 
> Also what is the exact effect of that register ? Does it affect access
> from PCI to the framebuffer or does it affect the way the CRTC
> consumes pixels from the framebuffer ? Is is supposed to have an
> effect on register accesses ?
> 
> Cheers,
> Ben.
> 
>