[pulseaudio-discuss] [PATCH] sbc_math.h: add explicit check for ARMv6 instructions

[Arun Raghavan]

Hi Paul,

On Tue, 2011-02-22 at 12:33 +0100, Paul Menzel wrote:
> Am Dienstag, den 22.02.2011, 09:48 +0000 schrieb Colin Guthrie:
> > 'Twas brillig, and Arun Raghavan at 21/02/11 20:31 did gyre and gimble:
> > > On Sun, 2011-02-20 at 19:20 +0100, Paul Menzel wrote:
> > >> Date: Sun, 20 Feb 2011 15:57:55 +0100
> > >>
> > >> Building PulseAudio 051d8213 [1] using OpenEmbedded with distribution `minimal-uclibc` for `MACHINE="om-gta01"` having an ARMv4T architecture (armv4t-oe-linux-uclibceabi) compilation fails with the following error.
> > > [...]
> > >> 	{standard input}:4099: Error: selected processor does not support Thumb mode `mla r3,r2,r6,r3'
> > >> 	{standard input}:4114: Error: selected processor does not support Thumb mode `mla r3,r2,ip,r3'
> > >> 	{standard input}:4125: Error: selected processor does not support Thumb mode `mla r3,r6,r2,r3'
> > >> 	make[3]: *** [libbluetooth_sbc_la-sbc.lo] Error 1
> > >>
> > >> Apply the same fix as in [2], which is similar to the patch applied in OpenEmbedded since commit 976ab4b5 [3].
> > >>
> > >> [1] http://git.0pointer.de/?p=pulseaudio.git;a=commit;h=051d82133f0ae6a57bf66fd200bc8e3591a7d5ca
> > >> [2] http://git.0pointer.de/?p=pulseaudio.git;a=commit;h=12b900858ae82d435c100d6eb94cb7bb22fe5e29
> > >> [3] http://cgit.openembedded.org/cgit.cgi/openembedded/commit/?id=976ab4b4587d548c0483a274058c5359cb72bf1b
> > >>
> > >> Signed-off-by: Paul Menzel <paulepanter at users.sourceforge.net>
> > >> CC: Marcel Holtmann <marcel at holtmann.org>
> > >> ---
> > >> I do not know if these files are just copied from linux-bluetooth upstream, so I am adding Marcel to CC anyway because he is mentioned in the copyright section.
> > > 
> > > It does appear that the files are a copy of the bluez package. If we're
> > > fixing this on the PulseAudio side, I think we should include something
> > > like diff below configure.ac, so that we can actually verify that the
> > > mla instruction is available for the arch being used.
> > > 
> > > -- Arun
> > > 
> > > ----
> > > diff --git a/configure.ac b/configure.ac
> > > index 08c947a..77e9823 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -257,13 +257,14 @@ case $host in
> > >               asm volatile ("ldr r0, %2 \n"
> > >                             "ldr r2, %3 \n"
> > >                             "ldr r3, %4 \n"
> > > +                           "mla r4, r1, r2, r3 \n"
> > >                             "ssat r1, #8, r0 \n"
> > >                             "str r1, %0 \n"
> > >                             "pkhbt r1, r3, r2, LSL #8 \n"
> > >                             "str r1, %1 \n"
> > >                             : "=m" (a), "=m" (b)
> > >                             : "m" (a), "m" (b), "m" (c)
> > > -                           : "r0", "r1", "r2", "r3", "cc");
> > > +                           : "r0", "r1", "r2", "r3", "r4", "cc");
> > >               return (a == -128 && b == 0xaabbdddd) ? 0 : -1;
> > >             ]]),
> > >           [pulseaudio_cv_support_armv6=yes],
> > 
> > Paul, can you confirm if this avoids the problem for you? If so, then
> > Arun, can you make a proper patch? You can? Awesome, thanks :)
> 
> Does not Arun’s patch just improve the check for `HAVE_ARMV6`? My patch
> just adds the check `if defined(HAVE_ARMV6)` to `sbc_math.h`. Without
> that addition all the configure checks are not evaluated at all in
> `sbc_math.h`.

For bits other than bluez, we need ARMv6 or above. For the MLA
instruction that's triggering the error you see, we need a version of
ARMv6 or above that also supports Thumb2 [1].

So your patch is incorrect in that it will fail on (e.g.) ARMv6K. My
patch above, on the other hand, will make PA not use v6 asm on ARMv6K
even if bluez support is disabled, which is also incorrect.

The correct fix for this, imo, is in bluez (there is a new
sbc_primitives_armv6.h that can probably be used at least as a
template). We need to do an sbc-udpate on the PA side anyway, and can
pull this when we do.

Cheers,
Arun

[1]
http://infocenter.arm.com/help/topic/com.arm.doc.qrc0001m/QRC0001_UAL.pdf