2011-03-28 15:41:22

by Luiz Augusto von Dentz

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

Hi,

On Fri, Mar 25, 2011 at 1:34 PM, Paul Menzel
<[email protected]> wrote:
> Dear Arun,
>
>
> Am Samstag, den 19.03.2011, 16:14 +0530 schrieb Arun Raghavan:
>
>> On Wed, 2011-02-23 at 01:07 +0530, Arun Raghavan wrote:
>> [...]
>> > 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.
>>
>> Could you see if the attached patch works for you? If it does, I can
>> push this to the bluez folks.
>
> My tag on the other thread [1] is ambiguous. The error is indeed caused
> by your patch. I will try to implement your recommended changes but will
> probably not get to it before Sunday.

Im including Siarhei Siamashka since he did most, if not all, of this
code and linux-bluetooth to cc.

Btw, the sbc subdir should be in sync with sbc subdir in BlueZ tree.


--
Luiz Augusto von Dentz
Computer Engineer


Attachments:
thumb2-fix.patch (1.33 kB)

2011-03-30 17:42:57

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] sbc_math.h: add explicit check for ARMv6 instructions

Hi Colin,

On Tue, Mar 29, 2011 at 1:21 PM, Colin Guthrie <[email protected]> wrote:
> 'Twas brillig, and Paul Menzel at 29/03/11 00:00 did gyre and gimble:
>> ? ? ? ? commit b676f89d8579c7ec1629892342a330f1e4c35657
>> ? ? ? ? Author: Colin Guthrie <[email protected]>
>> ? ? ? ? Date: ? Sun Mar 20 11:44:53 2011 +0000
>>
>> ? ? ? ? ? ? bluetooth: Run 'make update-sbc'
>>
>> ? ? ? ? ? ? Note that changes to ipc.h from 8f3ef04b had to be manually reapplied.
>>
>
> I ran this after checking with Luiz first.
>
> I've asked that the local changes to our ipc.h were pushed upstream
> after doing so to but not sure of the status of that, Luiz, I think I
> included a specific patch in the last mail... let me know if you want it
> again :)

I think it is better to add the a2dp-codecs.h header present on BlueZ
to PA too and remove the definitions Ive added to ipc.h, this should
make it easier to sync things. I just need some free time to make this
happen :D

--
Luiz Augusto von Dentz
Computer Engineer

2011-03-29 10:21:03

by Colin Guthrie

[permalink] [raw]
Subject: Re: [PATCH] sbc_math.h: add explicit check for ARMv6 instructions

'Twas brillig, and Paul Menzel at 29/03/11 00:00 did gyre and gimble:
> commit b676f89d8579c7ec1629892342a330f1e4c35657
> Author: Colin Guthrie <[email protected]>
> Date: Sun Mar 20 11:44:53 2011 +0000
>
> bluetooth: Run 'make update-sbc'
>
> Note that changes to ipc.h from 8f3ef04b had to be manually reapplied.
>

I ran this after checking with Luiz first.

I've asked that the local changes to our ipc.h were pushed upstream
after doing so to but not sure of the status of that, Luiz, I think I
included a specific patch in the last mail... let me know if you want it
again :)

Col

--

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
Tribalogic Limited [http://www.tribalogic.net/]
Open Source:
Mageia Contributor [http://www.mageia.org/]
PulseAudio Hacker [http://www.pulseaudio.org/]
Trac Hacker [http://trac.edgewall.org/]

2011-03-28 23:00:57

by Paul Menzel

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

Dear BlueZ folks,


Am Montag, den 28.03.2011, 18:41 +0300 schrieb Luiz Augusto von Dentz:

> On Fri, Mar 25, 2011 at 1:34 PM, Paul Menzel wrote:

> > Am Samstag, den 19.03.2011, 16:14 +0530 schrieb Arun Raghavan:
> >
> >> On Wed, 2011-02-23 at 01:07 +0530, Arun Raghavan wrote:
> >> [...]
> >> > 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.
> >>
> >> Could you see if the attached patch works for you? If it does, I can
> >> push this to the bluez folks.
> >
> > My tag on the other thread [1] is ambiguous. The error is indeed caused
> > by your patch. I will try to implement your recommended changes but will
> > probably not get to it before Sunday.
>
> Im including Siarhei Siamashka since he did most, if not all, of this
> code and linux-bluetooth to cc.

to fix [1] I tried to add `#include "sbc_tables.h"` to
`src/modules/bluetooth/sbc/sbc_primitives.h`

diff --git a/src/modules/bluetooth/sbc/sbc_primitives.h b/src/modules/bluetooth/
index 3fec8d5..9544826 100644
--- a/src/modules/bluetooth/sbc/sbc_primitives.h
+++ b/src/modules/bluetooth/sbc/sbc_primitives.h
@@ -24,6 +24,8 @@
*
*/

+#include "sbc_tables.h"
+
#ifndef __SBC_PRIMITIVES_H
#define __SBC_PRIMITIVES_H

But this seems to result in a need for `sbc_math.h` which leads to a
circular inclusion.

[…]
In file included from modules/bluetooth/sbc/sbc_primitives.h:27:0,
from modules/bluetooth/sbc/sbc_primitives_armv6.h:30,
from modules/bluetooth/sbc/sbc_math.h:27,
from modules/bluetooth/sbc/sbc.c:46:
modules/bluetooth/sbc/sbc_tables.h:50:2: warning: implicit declaration of function 'ASR' [-Wimplicit-function-declaration]
modules/bluetooth/sbc/sbc_tables.h:50:2: error: 'SCALE_SPROTO4_TBL' undeclared here (not in a function)
modules/bluetooth/sbc/sbc_tables.h:58:2: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:58:2: error: (near initialization for 'sbc_proto_4_40m1[0]')
modules/bluetooth/sbc/sbc_tables.h:58:2: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:58:2: error: (near initialization for 'sbc_proto_4_40m1[1]')
modules/bluetooth/sbc/sbc_tables.h:58:2: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:58:2: error: (near initialization for 'sbc_proto_4_40m1[2]')
modules/bluetooth/sbc/sbc_tables.h:58:2: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:58:2: error: (near initialization for 'sbc_proto_4_40m1[3]')
modules/bluetooth/sbc/sbc_tables.h:59:2: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:59:2: error: (near initialization for 'sbc_proto_4_40m1[4]')
modules/bluetooth/sbc/sbc_tables.h:59:2: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:59:2: error: (near initialization for 'sbc_proto_4_40m1[5]')
modules/bluetooth/sbc/sbc_tables.h:59:2: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:59:2: error: (near initialization for 'sbc_proto_4_40m1[6]')
modules/bluetooth/sbc/sbc_tables.h:59:2: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:59:2: error: (near initialization for 'sbc_proto_4_40m1[7]')
modules/bluetooth/sbc/sbc_tables.h:60:2: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:60:2: error: (near initialization for 'sbc_proto_4_40m1[8]')
modules/bluetooth/sbc/sbc_tables.h:60:2: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:60:2: error: (near initialization for 'sbc_proto_4_40m1[9]')
modules/bluetooth/sbc/sbc_tables.h:60:2: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:60:2: error: (near initialization for 'sbc_proto_4_40m1[10]')
modules/bluetooth/sbc/sbc_tables.h:60:2: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:60:2: error: (near initialization for 'sbc_proto_4_40m1[11]')
modules/bluetooth/sbc/sbc_tables.h:61:2: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:61:2: error: (near initialization for 'sbc_proto_4_40m1[12]')
modules/bluetooth/sbc/sbc_tables.h:61:2: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:61:2: error: (near initialization for 'sbc_proto_4_40m1[13]')
modules/bluetooth/sbc/sbc_tables.h:61:2: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:61:2: error: (near initialization for 'sbc_proto_4_40m1[14]')
modules/bluetooth/sbc/sbc_tables.h:61:2: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:61:2: error: (near initialization for 'sbc_proto_4_40m1[15]')
modules/bluetooth/sbc/sbc_tables.h:62:2: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:62:2: error: (near initialization for 'sbc_proto_4_40m1[16]')
modules/bluetooth/sbc/sbc_tables.h:62:2: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:62:2: error: (near initialization for 'sbc_proto_4_40m1[17]')
modules/bluetooth/sbc/sbc_tables.h:62:2: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:62:2: error: (near initialization for 'sbc_proto_4_40m1[18]')
modules/bluetooth/sbc/sbc_tables.h:63:1: error: initializer element is not constant
modules/bluetooth/sbc/sbc_tables.h:63:1: error: (near initialization for 'sbc_proto_4_40m1[19]')
[ goes on … ]

> Btw, the sbc subdir should be in sync with sbc subdir in BlueZ tree.

The question is how often should the updates be made. As far as I know
`make update-sbc` is only run once in a while.

commit b676f89d8579c7ec1629892342a330f1e4c35657
Author: Colin Guthrie <[email protected]>
Date: Sun Mar 20 11:44:53 2011 +0000

bluetooth: Run 'make update-sbc'

Note that changes to ipc.h from 8f3ef04b had to be manually reapplied.


Thanks,

Paul


[1] https://tango.0pointer.de/pipermail/pulseaudio-discuss/2011-March/009594.html


Attachments:
signature.asc (198.00 B)
This is a digitally signed message part

2011-03-28 16:42:55

by Siarhei Siamashka

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

On Mon, Mar 28, 2011 at 6:41 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi,
>
> On Fri, Mar 25, 2011 at 1:34 PM, Paul Menzel
> <[email protected]> wrote:
>> Dear Arun,
>>
>>
>> Am Samstag, den 19.03.2011, 16:14 +0530 schrieb Arun Raghavan:
>>
>>> On Wed, 2011-02-23 at 01:07 +0530, Arun Raghavan wrote:
>>> [...]
>>> > 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.
>>>
>>> Could you see if the attached patch works for you? If it does, I can
>>> push this to the bluez folks.

-#ifdef __arm__
+#ifdef SBC_HAVE_THUMB2

I think this check can be just changed to
#if defined(__arm__) && (!defined(__thumb__) || defined(__thumb2__))

MLA instruction is available on all ARM processors at least since
armv4, unless compiling code for thumb1. A similar modification can be
also added to armv6 sbc encoder optimizations. Right now armv6
assembly optimizations are disabled for both thumb1 and thumb2, while
they could be still used with thumb2.

--
Best regards,
Siarhei Siamashka