Bug 4890

Here's the relevant comment from Makefile.am:

# The two assembly language files here could conceivably be
# resurrected by someone sufficiently motivated.

So Ferris, are you? =)

Yes, I saw the comment.  One way or the other, I, or some one, has to be
motivated enough, but these .s modules should never have been removed:  sunffb
cannot run without them (at least, it can't run without VISmoveImage.s).

And they are probably needed --- see davem's comment to the effect that if you
don't use 64-bit loads, under some circumstances you can hang an ffb2.

It's really just a matter of assembling them; I don't know what the problem is:
 There's nothing magic about it; e.g., xorg-x11 ends up with---
sparc-unknown-linux-gnu-gcc -c -x assembler-with-cpp -Wa,-Av9a VISmoveImage.s -o
VISmoveImage.o
sparc-unknown-linux-gnu-gcc -c -x assembler-with-cpp -Wa,-Av9a ffb_asm.s -o
ffb_asm.o
(I don't know why those unbuilt .c modules were omitted, either.)

This works for me and is right for Gentoo (but probably not in general):

1.  In ./src/Makefile.am, at the end of sunffb_drv_la_SOURCES, add the two
assembler routines ffb_asm.s, VISmoveImage.s
2.  Add these lines to configure.ac in the root directory:
CCASFLAGS="-x assembler-with-cpp -Wa,-Av9a"
AM_PROG_AS
(I put them right after AC_PROG_CC)
3.  aclocal ; autoconf ; automake ; libtoolize --copy --force

With Gentoo, I then just did 'ebuild xf86-video-sunffb-1.0.0.1.ebuild compile
install qmerge' (which does a normal build and install to running system).

After that, sunffb (and X-modular) appear to be healthy.

Just did a little research into this, and we'll need to allow for fbsd, sun or
linux for the assembler flags, same as monolithic did.

Here's a quick idea of the conditional stuff we need to look at:

$ grep -i VISOPTION config/cf/*
config/cf/FreeBSD.cf:#define AsVISOption                -Av9a
config/cf/linux.cf:# define AsVISOption         -Av9a
config/cf/sun.cf:#  ifndef AsVISOption
config/cf/sun.cf:#     define AsVISOption               -Av8plusa
config/cf/sun.cf:#     define AsVISOption               -xarch=v8plusa
config/cf/sun.cf:#     define AsVISOption               -Av9a
config/cf/sun.cf:#     define AsVISOption               -xarch=v9a

Looks like it depends more on compiler and 32 vs. 64-bit modes than on OS:

gcc 32-bit mode:      -v8plusa
gcc 64-bit mode:      -v9a
Sun cc 32-bit mode:   -xarch=v8plusa
Sun cc 64-bit mode:   -xarch=v9a

Yeah, that's right. But the flags do differ slightly on the OS level as well, so
there needs to be a way to figure out both.

Not quite.  With Linux/gcc, the -Wa,-Av9a is just an architecture flag; it has
nothing to do with 32-bit/64-bit mode.  The assembly source checks for
predefined symbols (such as __sparc_v9__) to determine 32/64 bit mode.  If you
just use the CFLAGS, such as -cpu=ultrasparc, -mv9, unfortunately you get both,
which results in a 64-bit userland assembler module.  This is right for solaris,
but wrong for linux.

These modules are pure assembler, and they won't build without -Av9a, because
they use instructions which ara available only on sparc-v9 architecture.  But
for linux, they will build incorrectly if you just use the CFLAGS, because linux
does not use 64-bit usrland.  That's why you need special Assembler flags:  You
need to tell the assembler to build for v9 architecture, but you have to tell
the compiler NOT actually to define __sparc_v9__.

Guess I misunderstood something in the discussion we just had. =)

Probably you didn't; more likely, I was unclear.  The reason we have to go
through all this is that the source of VISmoveImage.s uses this test to
determine 64-bitness:
#if defined(__sparc_v9__) || defined(__sparcv9) || defined(__arch64__)
That is not good enough for Linux, because linux/gcc does define __sparc_v9__
but usually wants 32-bit code + (for sunffb) v9 architecture.  Now, on linux
systems, gcc also defines __linux__, so naively, you might think the proper fix
would be something like:

#if !defined(__linux__) && (defined(__sparc_v9__) || defined(__sparcv9) ||
defined(__arch64__))

in the source code, and not bother with CCASFLAGS at all (in which case, it just
defaults to CFLAGS).  This works until you get to experimental 64-bit userland
linux builds (Debian, Gentoo both have people playing with this possibility). 
Then, it fails.  But of course, just going the CCASFLAGS route should always
fail in 64-bit userland on any system unless somehow __arch64__ gets defined in
the CCASFLAGS (or by some other mechanism).

So, what we really want is for the test just to be:
#ifdef __arch64__
and for the compiler (or alternatively, the user) to have -D__arch64__ (or
-U__arch64__) in the CFLAGS passed to the configure script (or otherwise set
correctly by the compiler).  With Gentoo, this is quite a good solution, and
could be controlled either with a USE option (like USE=sparc64bituserland) or as
a global CFLAG combined with a profile.  Unfortunately, for other linux
versions, for solaris, for BSD, and for whatever else, this probably does not
fit so naturally into the build process.

In summary, the difficulty is that different compilers define the same symbol
(__sparc_v9__) for the ultrasparc (or v9) architectures to mean different
things. For gcc, it just means the code is optimized for v9 architecture, which
sunffb must be. But evidently for other compilers (and for VISmoveImage.s) it
also means the code is 64-bit userland.  And the VISmoveImage.s source is set up
in such a way that linux must use the CCASFLAGS approach to get it to build
correctly.

I think the solution to this (at least for Linux) comes out as follows.  Sunffb
cannot run on anything but sparc_v9 of some flavor (Ultrasparc 1, 2, 3, ...), so
the conditional tests
#if defined(__sparc_v9__) || defined(__sparcv9) || defined(__arch64__)
don't make much sense.  With gcc, the test is just "#if defined(__arch64__)" and
that symbol is defined by specifying the '-m64' compiler flag.

Unfortunately, the VISmoveImage.s source file (1) is required (see the comments
in the source about how if you don't use 64-bit moves, you can hang the ffb
card) and (2) will not build without the "-x assembler-with-cpp -Wa,-Av9" bit
for flags to gcc (normal CFLAGS + -m32 fails, calling assembler directly fails,
...).

This does mean, however, that the build should be able to sort out the 32/64 bit
userland question by examining the CFLAGS for the presence of '-m64' (or with a
configure test program which interrogates "#ifdef __arch64__"). (As an aside,
after playing some with the 64-bit usrland compiler used to build the kernel, I
suspect that the 64-bit usrland version of sunffb will not build at all, but I
can't prove it yet one way or the other.)

Note further, that if you run configure with CCASFLAGS defined, the configure
and build sort everything out correctly, once the assembler sources are properly
reactivated.  My verification of this was very simple:
1. Add ffb_asm.s, VISmoveImage.s to the source file list in src/Makefile.am
2. Add AM_PROG_AS to configure.ac
3. aclocal; ...
4.  CFLAGS="-pipe -O2 -mcpu=ultrasparc" \
    CCASFLAGS="-x assembler-with-cpp -Wa,-Av9a" ./configure
5. make
And everything builds correctly.

So the solution might just be a matter of putting back the .s source files and
documenting that if you don't like CFLAGS for building them (for Linux, you
don't; for other Unix flavors, you might), define CCASFLAGS to be what you need.

This is cheating a bit, perhaps, because with Gentoo, we can just hide that in
the ebuild for sunffb, but it is consistent with how configure runs generally
(take relevant flags from the environment).

I am bumping the priority on this.  Releases of sunffb without this fixed are
pointless because they will not run at all on any system.  It's reasonable to
put in critical fixes of other things if they come up, but without the correct
inclusion of the assembly sources in the build process, it just creates extra
work to fix each release by hand, and if nothing major is happening to the
driver, that becomes very annoying.

It would help if you would at least include the two assembly routines in
src/Makefile.am's sunffb_drv_la_SOURCES list and AS_PROG_AM in configure.ac.  It
might not build as-is with that, but at least it makes the build process cleaner
(to make it build, define CCASFLAGS to whatever your system needs), and without
them, sunffb_drv might build, but there is NO system on which it can possibly run.

With those changes, it builds more or less normally (user must define CCASFLAGS
to make it work, but that's a big improvement over having to go in and fix
configuration files.).  I.e., do what Comment 11 above suggests.

so do you have a patch that implements this?  it's a bit unclear what you're
suggesting, you tend to cover every possible option.

(In reply to comment #13)
> so do you have a patch that implements this?  it's a bit unclear what you're
> suggesting, you tend to cover every possible option.

No, if I had a good patch, this would not be so frustrating.  In every case, the
two .s files have to be moved into the sunffb_drv_la_SOURCES list in
src/Makefile.am, and for this to work, you have to add the AM_PROG_AS macro to
configure.ac and rerun the appropriate autotools chain to get everything set up
right.

Now, it gets harder.  Those .s files are not really assembly code; they need to
be preprocessed by cpp before the assembler on any system can handle them.  That
is what the CCASFLAGS definition is to help with, and this varies, I suppose
from compiler to compiler.  For Gentoo, it's not really a problem because we can
force it in the build process to be '-x assembler-with-cpp -Wa,-Av9a ' which is
what any gnu gcc/Linux-32-bit-user-mode system will want.  It seems that the
(experimental) Linux-64-bit-user-mode systems will want to add -m64 to this. 
However, for other operating systems on sparc using X-modular, I do not know
what CCASFLAGS should be and don't have any good way to figure it out.

In any case, the first part of this (getting VISmoveImage.s, amd maybe
ffb_asm.s) into the build process is required in order to have any hope of
building a working ffb/afb driver.  Once that is done, I know how to build it on
Linux (either 32-bit or 64-bit userland), but not on anything else.

Created attachment 4060 [details] [review]
Patch to add Assembler routines back to sunffb and to setup configure.ac to know about them.

As requested, this patch should give a version of xf86-video-sunffb which will
build correctly for 32-bit userland sparc linux.  (It should work on any system
if the user correctly defines CCASFLAGS; I am building in the correct
definition for Linux).
I have verified that if you apply the patch (I used 
patch -p0 -b -z- < xf86-video-sunffb-asm.patch), then rerun the
aclocal ; autoconf ; automake ; libtoolize --copy --force
chain, then (for example),
CFLAGS='-O2 -pipe -mcpu=ultrasparc3' ./configure; make -j2
builds sunffb_drv.so with the assembler routines included and properly built.

applied, with some additional hack work to keep it distcheckable on non-sparc
machines.  thanks!

(In reply to comment #16)
> applied, with some additional hack work to keep it distcheckable on non-sparc
> machines.  thanks!

Uh, are there any non-sparc machines which can use this?  No matter what you are
on, you need the entry points provided by VISmoveImage.S, and that is sparc
assmebly code.  I suppose you could be cross-building it, but in the end, I
think it has to run on sparc.

yes, but the file needs to be explicitly listed to always be included in any
tarballs that are made, else the only tarballs it will compile from are ones
built *on* sparc.

Builds fine now on Gentoo sparc linux.  Thanks.