* [PATCH 3/3] Code clean up: cache->folder removed
From: Dmitriy Paliy @ 2010-11-11 7:36 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Dmitriy Paliy
In-Reply-To: <1289460972-2971-1-git-send-email-dmitriy.paliy@nokia.com>
cache->folder is not used anywhere and therefore removed.
---
plugins/pbap.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/plugins/pbap.c b/plugins/pbap.c
index af60741..660b17d 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -125,7 +125,6 @@ struct aparam_header {
struct cache {
gboolean valid;
uint32_t index;
- char *folder;
GSList *entries;
};
@@ -219,7 +218,6 @@ static const char *cache_find(struct cache *cache, uint32_t handle)
static void cache_clear(struct cache *cache)
{
- g_free(cache->folder);
g_slist_foreach(cache->entries, (GFunc) cache_entry_free, NULL);
g_slist_free(cache->entries);
cache->entries = NULL;
--
1.7.0.4
^ permalink raw reply related
* [PATCH] Add iwmmxt optimization for sbc for pxa series cpu
From: Keith Mok @ 2010-11-11 8:05 UTC (permalink / raw)
To: linux-bluetooth
Hi all,
This patch add iwmmxt (Intel wireless mmx, pxa platform) optimzation
for sbc, based on the mmx code.
Have verified the encoded result against the mmx generated one.
Keith
Signed-off-by: Keith Mok <ek9852@gmail.com>
---
Makefile.am | 1 +
sbc/sbc_primitives.c | 4 +
sbc/sbc_primitives_iwmmxt.c | 361 +++++++++++++++++++++++++++++++++++++++++++
sbc/sbc_primitives_iwmmxt.h | 38 +++++
4 files changed, 404 insertions(+), 0 deletions(-)
create mode 100644 sbc/sbc_primitives_iwmmxt.c
create mode 100644 sbc/sbc_primitives_iwmmxt.h
diff --git a/Makefile.am b/Makefile.am
index da308a7..03a9bf2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -65,6 +65,7 @@ noinst_LTLIBRARIES += sbc/libsbc.la
sbc_libsbc_la_SOURCES = sbc/sbc.h sbc/sbc.c sbc/sbc_math.h sbc/sbc_tables.h \
sbc/sbc_primitives.h sbc/sbc_primitives.c \
sbc/sbc_primitives_mmx.h sbc/sbc_primitives_mmx.c \
+ sbc/sbc_primitives_iwmmxt.h sbc/sbc_primitives_iwmmxt.c \
sbc/sbc_primitives_neon.h sbc/sbc_primitives_neon.c \
sbc/sbc_primitives_armv6.h sbc/sbc_primitives_armv6.c
diff --git a/sbc/sbc_primitives.c b/sbc/sbc_primitives.c
index f87fb5a..ad780d0 100644
--- a/sbc/sbc_primitives.c
+++ b/sbc/sbc_primitives.c
@@ -33,6 +33,7 @@
#include "sbc_primitives.h"
#include "sbc_primitives_mmx.h"
+#include "sbc_primitives_iwmmxt.h"
#include "sbc_primitives_neon.h"
#include "sbc_primitives_armv6.h"
@@ -544,6 +545,9 @@ void sbc_init_primitives(struct sbc_encoder_state *state)
#ifdef SBC_BUILD_WITH_ARMV6_SUPPORT
sbc_init_primitives_armv6(state);
#endif
+#ifdef SBC_BUILD_WITH_IWMMXT_SUPPORT
+ sbc_init_primitives_iwmmxt(state);
+#endif
#ifdef SBC_BUILD_WITH_NEON_SUPPORT
sbc_init_primitives_neon(state);
#endif
diff --git a/sbc/sbc_primitives_iwmmxt.c b/sbc/sbc_primitives_iwmmxt.c
new file mode 100644
index 0000000..4825998
--- /dev/null
+++ b/sbc/sbc_primitives_iwmmxt.c
@@ -0,0 +1,361 @@
+/*
+ *
+ * Bluetooth low-complexity, subband codec (SBC) library
+ *
+ * Copyright (C) 2010 Keith Mok <ek9852@gmail.com>
+ * Based on sbc_primitives_mmx.c
+ *
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#include <stdint.h>
+#include <limits.h>
+#include "sbc.h"
+#include "sbc_math.h"
+#include "sbc_tables.h"
+
+#include "sbc_primitives_iwmmxt.h"
+
+/*
+ * IWMMXT optimizations
+ */
+
+#ifdef SBC_BUILD_WITH_IWMMXT_SUPPORT
+
+static inline void sbc_analyze_four_iwmmxt(const int16_t *in, int32_t *out,
+ const FIXED_T *consts)
+{
+ asm volatile (
+ "tbcstw wr4, %2\n"
+ "wldrd wr0, [%0]\n"
+ "wldrd wr1, [%0, #8]\n"
+ "wldrd wr2, [%1]\n"
+ "wldrd wr3, [%1, #8]\n"
+ "wmadds wr0, wr2, wr0\n"
+ "wmadds wr1, wr3, wr1\n"
+ "waddwss wr0, wr0, wr4\n"
+ "waddwss wr1, wr1, wr4\n"
+ "\n"
+ "wldrd wr2, [%0, #16]\n"
+ "wldrd wr3, [%0, #24]\n"
+ "wldrd wr4, [%1, #16]\n"
+ "wldrd wr5, [%1, #24]\n"
+ "wmadds wr2, wr4, wr2\n"
+ "wmadds wr3, wr5, wr3\n"
+ "waddwss wr0, wr2, wr0\n"
+ "waddwss wr1, wr3, wr1\n"
+ "\n"
+ "wldrd wr2, [%0, #32]\n"
+ "wldrd wr3, [%0, #40]\n"
+ "wldrd wr4, [%1, #32]\n"
+ "wldrd wr5, [%1, #40]\n"
+ "wmadds wr2, wr4, wr2\n"
+ "wmadds wr3, wr5, wr3\n"
+ "waddwss wr0, wr2, wr0\n"
+ "waddwss wr1, wr3, wr1\n"
+ "\n"
+ "wldrd wr2, [%0, #48]\n"
+ "wldrd wr3, [%0, #56]\n"
+ "wldrd wr4, [%1, #48]\n"
+ "wldrd wr5, [%1, #56]\n"
+ "wmadds wr2, wr4, wr2\n"
+ "wmadds wr3, wr5, wr3\n"
+ "waddwss wr0, wr2, wr0\n"
+ "waddwss wr1, wr3, wr1\n"
+ "\n"
+ "wldrd wr2, [%0, #64]\n"
+ "wldrd wr3, [%0, #72]\n"
+ "wldrd wr4, [%1, #64]\n"
+ "wldrd wr5, [%1, #72]\n"
+ "wmadds wr2, wr4, wr2\n"
+ "wmadds wr3, wr5, wr3\n"
+ "waddwss wr0, wr2, wr0\n"
+ "waddwss wr1, wr3, wr1\n"
+ "\n"
+ "tmcr wcgr0, %4\n"
+ "wsrawg wr0, wr0, wcgr0\n"
+ "wsrawg wr1, wr1, wcgr0\n"
+ "wpackwss wr0, wr0, wr0\n"
+ "wpackwss wr1, wr1, wr1\n"
+ "\n"
+ "wldrd wr4, [%1, #80]\n"
+ "wldrd wr5, [%1, #88]\n"
+ "wldrd wr6, [%1, #96]\n"
+ "wldrd wr7, [%1, #104]\n"
+ "wmadds wr2, wr5, wr0\n"
+ "wmadds wr0, wr4, wr0\n"
+ "\n"
+ "wmadds wr3, wr7, wr1\n"
+ "wmadds wr1, wr6, wr1\n"
+ "waddwss wr0, wr1, wr0\n"
+ "waddwss wr2, wr3, wr2\n"
+ "\n"
+ "wstrd wr0, [%3]\n"
+ "wstrd wr2, [%3, #8]\n"
+ :
+ : "r" (in), "r" (consts),
+ "r" (1 << (SBC_PROTO_FIXED4_SCALE - 1)), "r" (out),
+ "r" (SBC_PROTO_FIXED4_SCALE)
+ : "memory");
+}
+
+static inline void sbc_analyze_eight_iwmmxt(const int16_t *in, int32_t *out,
+ const FIXED_T *consts)
+{
+ asm volatile (
+ "tbcstw wr8, %2\n"
+ "wldrd wr0, [%0]\n"
+ "wldrd wr1, [%0, #8]\n"
+ "wldrd wr2, [%0, #16]\n"
+ "wldrd wr3, [%0, #24]\n"
+ "wldrd wr4, [%1]\n"
+ "wldrd wr5, [%1, #8]\n"
+ "wldrd wr6, [%1, #16]\n"
+ "wldrd wr7, [%1, #24]\n"
+ "wmadds wr0, wr0, wr4\n"
+ "wmadds wr1, wr1, wr5\n"
+ "wmadds wr2, wr2, wr6\n"
+ "wmadds wr3, wr3, wr7\n"
+ "waddwss wr0, wr0, wr8\n"
+ "waddwss wr1, wr1, wr8\n"
+ "waddwss wr2, wr2, wr8\n"
+ "waddwss wr3, wr3, wr8\n"
+ "\n"
+ "wldrd wr4, [%0, #32]\n"
+ "wldrd wr5, [%0, #40]\n"
+ "wldrd wr6, [%0, #48]\n"
+ "wldrd wr7, [%0, #56]\n"
+ "wldrd wr8, [%1, #32]\n"
+ "wldrd wr9, [%1, #40]\n"
+ "wldrd wr10, [%1, #48]\n"
+ "wldrd wr11, [%1, #56]\n"
+ "wmadds wr4, wr4, wr8\n"
+ "wmadds wr5, wr5, wr9\n"
+ "wmadds wr6, wr6, wr10\n"
+ "wmadds wr7, wr7, wr11\n"
+ "waddwss wr0, wr4, wr0\n"
+ "waddwss wr1, wr5, wr1\n"
+ "waddwss wr2, wr6, wr2\n"
+ "waddwss wr3, wr7, wr3\n"
+ "\n"
+ "wldrd wr4, [%0, #64]\n"
+ "wldrd wr5, [%0, #72]\n"
+ "wldrd wr6, [%0, #80]\n"
+ "wldrd wr7, [%0, #88]\n"
+ "wldrd wr8, [%1, #64]\n"
+ "wldrd wr9, [%1, #72]\n"
+ "wldrd wr10, [%1, #80]\n"
+ "wldrd wr11, [%1, #88]\n"
+ "wmadds wr4, wr4, wr8\n"
+ "wmadds wr5, wr5, wr9\n"
+ "wmadds wr6, wr6, wr10\n"
+ "wmadds wr7, wr7, wr11\n"
+ "waddwss wr0, wr4, wr0\n"
+ "waddwss wr1, wr5, wr1\n"
+ "waddwss wr2, wr6, wr2\n"
+ "waddwss wr3, wr7, wr3\n"
+ "\n"
+ "wldrd wr4, [%0, #96]\n"
+ "wldrd wr5, [%0, #104]\n"
+ "wldrd wr6, [%0, #112]\n"
+ "wldrd wr7, [%0, #120]\n"
+ "wldrd wr8, [%1, #96]\n"
+ "wldrd wr9, [%1, #104]\n"
+ "wldrd wr10, [%1, #112]\n"
+ "wldrd wr11, [%1, #120]\n"
+ "wmadds wr4, wr4, wr8\n"
+ "wmadds wr5, wr5, wr9\n"
+ "wmadds wr6, wr6, wr10\n"
+ "wmadds wr7, wr7, wr11\n"
+ "waddwss wr0, wr4, wr0\n"
+ "waddwss wr1, wr5, wr1\n"
+ "waddwss wr2, wr6, wr2\n"
+ "waddwss wr3, wr7, wr3\n"
+ "\n"
+ "wldrd wr4, [%0, #128]\n"
+ "wldrd wr5, [%0, #136]\n"
+ "wldrd wr6, [%0, #144]\n"
+ "wldrd wr7, [%0, #152]\n"
+ "wldrd wr8, [%1, #128]\n"
+ "wldrd wr9, [%1, #136]\n"
+ "wldrd wr10, [%1, #144]\n"
+ "wldrd wr11, [%1, #152]\n"
+ "wmadds wr4, wr4, wr8\n"
+ "wmadds wr5, wr5, wr9\n"
+ "wmadds wr6, wr6, wr10\n"
+ "wmadds wr7, wr7, wr11\n"
+ "waddwss wr0, wr4, wr0\n"
+ "waddwss wr1, wr5, wr1\n"
+ "waddwss wr2, wr6, wr2\n"
+ "waddwss wr3, wr7, wr3\n"
+ "\n"
+ "tmcr wcgr0, %4\n"
+ "wsrawg wr0, wr0, wcgr0\n"
+ "wsrawg wr1, wr1, wcgr0\n"
+ "wsrawg wr2, wr2, wcgr0\n"
+ "wsrawg wr3, wr3, wcgr0\n"
+ "\n"
+ "wpackwss wr0, wr0, wr0\n"
+ "wpackwss wr1, wr1, wr1\n"
+ "wpackwss wr2, wr2, wr2\n"
+ "wpackwss wr3, wr3, wr3\n"
+ "\n"
+ "wldrd wr4, [%1, #160]\n"
+ "wldrd wr5, [%1, #168]\n"
+ "wmadds wr4, wr4, wr0\n"
+ "wmadds wr5, wr5, wr0\n"
+ "\n"
+ "wldrd wr6, [%1, #192]\n"
+ "wldrd wr7, [%1, #200]\n"
+ "wmadds wr6, wr6, wr1\n"
+ "wmadds wr7, wr7, wr1\n"
+ "waddwss wr4, wr6, wr4\n"
+ "waddwss wr5, wr7, wr5\n"
+ "\n"
+ "wldrd wr6, [%1, #224]\n"
+ "wldrd wr7, [%1, #232]\n"
+ "wmadds wr6, wr6, wr2\n"
+ "wmadds wr7, wr7, wr2\n"
+ "waddwss wr4, wr6, wr4\n"
+ "waddwss wr5, wr7, wr5\n"
+ "\n"
+ "wldrd wr6, [%1, #256]\n"
+ "wldrd wr7, [%1, #264]\n"
+ "wmadds wr6, wr6, wr3\n"
+ "wmadds wr7, wr7, wr3\n"
+ "waddwss wr4, wr6, wr4\n"
+ "waddwss wr5, wr7, wr5\n"
+ "\n"
+ "wstrd wr4, [%3]\n"
+ "wstrd wr5, [%3, #8]\n"
+ "\n"
+ "wldrd wr4, [%1, #176]\n"
+ "wldrd wr5, [%1, #184]\n"
+ "wmadds wr5, wr5, wr0\n"
+ "wmadds wr0, wr4, wr0\n"
+ "\n"
+ "wldrd wr4, [%1, #208]\n"
+ "wldrd wr7, [%1, #216]\n"
+ "wmadds wr7, wr7, wr1\n"
+ "wmadds wr1, wr4, wr1\n"
+ "waddwss wr0, wr1, wr0\n"
+ "waddwss wr5, wr7, wr5\n"
+ "\n"
+ "wldrd wr4, [%1, #240]\n"
+ "wldrd wr7, [%1, #248]\n"
+ "wmadds wr7, wr7, wr2\n"
+ "wmadds wr2, wr4, wr2\n"
+ "waddwss wr0, wr2, wr0\n"
+ "waddwss wr5, wr7, wr5\n"
+ "\n"
+ "wldrd wr4, [%1, #272]\n"
+ "wldrd wr7, [%1, #280]\n"
+ "wmadds wr7, wr7, wr3\n"
+ "wmadds wr3, wr4, wr3\n"
+ "waddwss wr0, wr3, wr0\n"
+ "waddwss wr5, wr7, wr5\n"
+ "\n"
+ "wstrd wr0, [%3, #16]\n"
+ "wstrd wr5, [%3, #24]\n"
+ :
+ : "r" (in), "r" (consts),
+ "r" (1 << (SBC_PROTO_FIXED8_SCALE - 1)), "r" (out),
+ "r" (SBC_PROTO_FIXED8_SCALE)
+ : "memory");
+}
+
+static inline void sbc_analyze_4b_4s_iwmmxt(int16_t *x, int32_t *out,
+ int out_stride)
+{
+ /* Analyze blocks */
+ sbc_analyze_four_iwmmxt(x + 12, out, analysis_consts_fixed4_simd_odd);
+ out += out_stride;
+ sbc_analyze_four_iwmmxt(x + 8, out, analysis_consts_fixed4_simd_even);
+ out += out_stride;
+ sbc_analyze_four_iwmmxt(x + 4, out, analysis_consts_fixed4_simd_odd);
+ out += out_stride;
+ sbc_analyze_four_iwmmxt(x + 0, out, analysis_consts_fixed4_simd_even);
+}
+
+static inline void sbc_analyze_4b_8s_iwmmxt(int16_t *x, int32_t *out,
+ int out_stride)
+{
+ /* Analyze blocks */
+ sbc_analyze_eight_iwmmxt(x + 24, out, analysis_consts_fixed8_simd_odd);
+ out += out_stride;
+ sbc_analyze_eight_iwmmxt(x + 16, out, analysis_consts_fixed8_simd_even);
+ out += out_stride;
+ sbc_analyze_eight_iwmmxt(x + 8, out, analysis_consts_fixed8_simd_odd);
+ out += out_stride;
+ sbc_analyze_eight_iwmmxt(x + 0, out, analysis_consts_fixed8_simd_even);
+}
+
+static void sbc_calc_scalefactors_iwmmxt(
+ int32_t sb_sample_f[16][2][8],
+ uint32_t scale_factor[2][8],
+ int blocks, int channels, int subbands)
+{
+ int ch, sb;
+ intptr_t blk;
+ for (ch = 0; ch < channels; ch++) {
+ for (sb = 0; sb < subbands; sb += 2) {
+ int b;
+ blk = &sb_sample_f[0][ch][sb];
+ b = blocks;
+ asm volatile (
+ "tbcstw wr0, %4\n"
+ "1:\n"
+ "wldrd wr1, [%0], %2\n"
+ "wxor wr2, wr2, wr2\n"
+ "wcmpgtsw wr3, wr1, wr2\n"
+ "waddwss wr1, wr1, wr3\n"
+ "wcmpgtsw wr2, wr2, wr1\n"
+ "wxor wr1, wr1, wr2\n"
+
+ "wor wr0, wr0, wr1\n"
+
+ "subs %1, %1, #1\n"
+ "bne 1b\n"
+
+ "tmrrc %0, %1, wr0\n"
+ "clz %0, %0\n"
+ "rsb %0, %0, %5\n"
+ "str %0, [%3]\n"
+
+ "clz %1, %1\n"
+ "rsb %1, %1, %5\n"
+ "str %1, [%3, #4]\n"
+ : "+&r" (blk), "+&r" (b)
+ : "i" ((char *) &sb_sample_f[1][0][0] -
+ (char *) &sb_sample_f[0][0][0]),
+ "r" (&scale_factor[ch][sb]),
+ "r" (1 << SCALE_OUT_BITS),
+ "i" (SCALE_OUT_BITS+1)
+ : "memory");
+ }
+ }
+}
+
+void sbc_init_primitives_iwmmxt(struct sbc_encoder_state *state)
+{
+ state->sbc_analyze_4b_4s = sbc_analyze_4b_4s_iwmmxt;
+ state->sbc_analyze_4b_8s = sbc_analyze_4b_8s_iwmmxt;
+ state->sbc_calc_scalefactors = sbc_calc_scalefactors_iwmmxt;
+ state->implementation_info = "IWMMXT";
+}
+
+#endif
diff --git a/sbc/sbc_primitives_iwmmxt.h b/sbc/sbc_primitives_iwmmxt.h
new file mode 100644
index 0000000..827d811
--- /dev/null
+++ b/sbc/sbc_primitives_iwmmxt.h
@@ -0,0 +1,38 @@
+/*
+ *
+ * Bluetooth low-complexity, subband codec (SBC) library
+ *
+ * Based on sbc_primitives_mmx.c
+ *
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifndef __SBC_PRIMITIVES_IWMMXT_H
+#define __SBC_PRIMITIVES_IWMMXT_H
+
+#include "sbc_primitives.h"
+
+#if defined(__GNUC__) && defined(__IWMMXT__) && \
+ !defined(SBC_HIGH_PRECISION) && (SCALE_OUT_BITS == 15)
+
+#define SBC_BUILD_WITH_IWMMXT_SUPPORT
+
+void sbc_init_primitives_iwmmxt(struct sbc_encoder_state *encoder_state);
+
+#endif
+
+#endif
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH] Add a new configuration option to disable Low Energy support
From: Johan Hedberg @ 2010-11-11 9:18 UTC (permalink / raw)
To: Claudio Takahasi; +Cc: linux-bluetooth
In-Reply-To: <1289426161-10045-3-git-send-email-claudio.takahasi@openbossa.org>
Hi Claudio,
On Wed, Nov 10, 2010, Claudio Takahasi wrote:
> @@ -623,6 +624,9 @@ int attrib_server_init(void)
>
> sdp_handle = record->handle;
>
> + if (!main_opts.le)
> + return 0;
> +
If the option is called "le" it shouldn't affect GATT over BR/EDR, so I
think the check in attrib_server_init should only affect the LE socket
(or then rename the option to e.g. main.opts.attrib).
Johan
^ permalink raw reply
* Re: [PATCH] hcitool: Bring up device before sending commands over socket
From: Johan Hedberg @ 2010-11-11 9:26 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1289327397-21935-1-git-send-email-anderson.lizardo@openbossa.org>
Hi,
On Tue, Nov 09, 2010, Anderson Lizardo wrote:
> The Texas specific initialization code sends HCI commands over the
> bluetooth socket, but does not bring up the device. This gives these
> errors when running "hciattach /dev/ttyUSB0 texas":
>
> Found a Texas Instruments' chip!
> Firmware file : /lib/firmware/TIInit_XX.Y.ZZ.bts
> Loaded BTS script version 1
> Cannot send hci command to socket: Network is down
> Can't initialize device: Network is down
> ---
> tools/hciattach_ti.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
Since no one has objected to this I went ahead and pushed it upstream
(after doing s/hcitool/hciattach/ to the summary).
Johan
^ permalink raw reply
* Re: [PATCH] Check HealthApplication path before trying to destroy it
From: Johan Hedberg @ 2010-11-11 9:27 UTC (permalink / raw)
To: Elvis Pfützenreuter; +Cc: linux-bluetooth
In-Reply-To: <1289355108-4041-1-git-send-email-epx@signove.com>
Hi Elvis,
On Wed, Nov 10, 2010, Elvis Pfützenreuter wrote:
> ---
> health/hdp.c | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
Pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] Fix segfault in HDP during device re-creation
From: Johan Hedberg @ 2010-11-11 9:28 UTC (permalink / raw)
To: Jose Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <1289382461-10510-1-git-send-email-santoscadenas@gmail.com>
Hi,
On Wed, Nov 10, 2010, Jose Antonio Santos Cadenas wrote:
> ---
> health/hdp.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/health/hdp.c b/health/hdp.c
> index 1eba8e1..d361b27 100644
> --- a/health/hdp.c
> +++ b/health/hdp.c
> @@ -259,6 +259,7 @@ static void device_unref_mcl(struct hdp_device *hdp_device)
> if (!hdp_device->mcl)
> return;
>
> + mcap_close_mcl(hdp_device->mcl, FALSE);
> mcap_mcl_unref(hdp_device->mcl);
> hdp_device->mcl = NULL;
> hdp_device->mcl_conn = FALSE;
Pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] Fix pull phonebook reply if filter not set
From: Johan Hedberg @ 2010-11-11 9:29 UTC (permalink / raw)
To: Lukasz Pawlik; +Cc: linux-bluetooth
In-Reply-To: <1289389387-14308-1-git-send-email-lucas.pawlik@gmail.com>
Hi Lukasz,
On Wed, Nov 10, 2010, Lukasz Pawlik wrote:
> According to the PBAP specification if filter is not set or is set to
> 0x00000000 in the application parameters header all attributes of the vCard
> should be returned. Previously only mandatory attributes were returned in
> phonebook pull reply. This patch fix this and now all currently supported
> vCards attributes will be returned.
> ---
> plugins/vcard.c | 10 ++++++++--
> 1 files changed, 8 insertions(+), 2 deletions(-)
Pushed upstream. Thanks.
Johan
^ permalink raw reply
* [PATCH] sbc: added "cc" to the clobber list of mmx inline assembly
From: Siarhei Siamashka @ 2010-11-11 9:29 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Siarhei Siamashka
From: Siarhei Siamashka <siarhei.siamashka@nokia.com>
In the case of scale factors calculation optimizations, the inline
assembly code has instructions which update flags register, but
"cc" was not mentioned in the clobber list. When optimizing code,
gcc theoretically is allowed to do a comparison before the inline
assembly block, and a conditional branch after it which would lead
to a problem if the flags register gets clobbered. While this is
apparently not happening in practice with the current versions of
gcc, the clobber list needs to be corrected.
Regarding the other inline assembly blocks. While most likely it
is actually unnecessary based on quick review, "cc" is also added
there to the clobber list because it should have no impact on
performance in practice. It's kind of cargo cult, but relieves
us from the need to track the potential updates of flags register
in all these places.
---
sbc/sbc_primitives_mmx.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/sbc/sbc_primitives_mmx.c b/sbc/sbc_primitives_mmx.c
index 45c62ac..7f2fbc3 100644
--- a/sbc/sbc_primitives_mmx.c
+++ b/sbc/sbc_primitives_mmx.c
@@ -101,7 +101,7 @@ static inline void sbc_analyze_four_mmx(const int16_t *in, int32_t *out,
:
: "r" (in), "r" (consts), "r" (&round_c), "r" (out),
"i" (SBC_PROTO_FIXED4_SCALE)
- : "memory");
+ : "cc", "memory");
}
static inline void sbc_analyze_eight_mmx(const int16_t *in, int32_t *out,
@@ -243,7 +243,7 @@ static inline void sbc_analyze_eight_mmx(const int16_t *in, int32_t *out,
:
: "r" (in), "r" (consts), "r" (&round_c), "r" (out),
"i" (SBC_PROTO_FIXED8_SCALE)
- : "memory");
+ : "cc", "memory");
}
static inline void sbc_analyze_4b_4s_mmx(int16_t *x, int32_t *out,
@@ -323,7 +323,7 @@ static void sbc_calc_scalefactors_mmx(
"r" (&scale_factor[ch][sb]),
"r" (&consts),
"i" (SCALE_OUT_BITS)
- : "memory");
+ : "cc", "memory");
}
}
asm volatile ("emms\n");
--
1.7.2.2
^ permalink raw reply related
* Re: [PATCH 1/7] Fix proper type handling in contacts_query_all
From: Johan Hedberg @ 2010-11-11 9:32 UTC (permalink / raw)
To: Bartosz Szatkowski; +Cc: linux-bluetooth
In-Reply-To: <1289394930-6694-1-git-send-email-bulislaw@linux.com>
Hi Bartosz,
On Wed, Nov 10, 2010, Bartosz Szatkowski wrote:
> Previously all phone numbers, addresses and emails was considered to be "work".
> Now there are three working types for emails and addresses: "work", "home",
> "other" and four for phone numbers - these three as well as "cell".
> ---
> plugins/phonebook-tracker.c | 61 +++++++++++++++++++++++++++++++------------
> 1 files changed, 44 insertions(+), 17 deletions(-)
All seven patches have been pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH 1/7] Fix invalid memory access when EIR field length is zero
From: Johan Hedberg @ 2010-11-11 9:35 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth, Bruna Moreira
In-Reply-To: <1289411487-6113-1-git-send-email-anderson.lizardo@openbossa.org>
Hi,
On Wed, Nov 10, 2010, Anderson Lizardo wrote:
> - uint8_t *uuid16;
> - uint8_t *uuid32;
> - uint8_t *uuid128;
> + uint8_t *uuid16 = 0;
> + uint8_t *uuid32 = 0;
> + uint8_t *uuid128 = 0;
0 is for integers, NULL for pointers. However, in this case I don't see
why the inialization is needed at all since the rest of the patch seems
to take care of the bug by itself.
Johan
^ permalink raw reply
* [PATCH 1/2] Remove a2dp setup callbacks after they return
From: Luiz Augusto von Dentz @ 2010-11-11 9:40 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
Since the callback won't be ever called again it make no sense to keep
them, also this cause a2dp_cancel to assume there are still some pending
callbacks to be processed and do not abort when it should.
---
audio/a2dp.c | 55 +++++++++++++++++++++++++++++++++++--------------------
1 files changed, 35 insertions(+), 20 deletions(-)
diff --git a/audio/a2dp.c b/audio/a2dp.c
index 3aaf022..a55020d 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -160,15 +160,18 @@ static gboolean finalize_config(struct a2dp_setup *s)
struct avdtp_stream *stream = s->err ? NULL : s->stream;
setup_ref(s);
- for (l = s->cb; l != NULL; l = l->next) {
+ for (l = s->cb; l != NULL; ){
struct a2dp_setup_cb *cb = l->data;
+ l = l->next;
+
if (!cb->config_cb)
continue;
cb->config_cb(s->session, s->sep, stream, s->err,
cb->user_data);
- cb->config_cb = NULL;
+ s->cb = g_slist_remove(s->cb, cb);
+ g_free(cb);
setup_unref(s);
}
@@ -193,14 +196,18 @@ static gboolean finalize_resume(struct a2dp_setup *s)
setup_ref(s);
- for (l = s->cb; l != NULL; l = l->next) {
+ for (l = s->cb; l != NULL; ) {
struct a2dp_setup_cb *cb = l->data;
- if (cb && cb->resume_cb) {
- cb->resume_cb(s->session, s->err, cb->user_data);
- cb->resume_cb = NULL;
- setup_unref(s);
- }
+ l = l->next;
+
+ if (!cb->resume_cb)
+ continue;
+
+ cb->resume_cb(s->session, s->err, cb->user_data);
+ s->cb = g_slist_remove(s->cb, cb);
+ g_free(cb);
+ setup_unref(s);
}
setup_unref(s);
@@ -213,14 +220,18 @@ static gboolean finalize_suspend(struct a2dp_setup *s)
GSList *l;
setup_ref(s);
- for (l = s->cb; l != NULL; l = l->next) {
+ for (l = s->cb; l != NULL; ) {
struct a2dp_setup_cb *cb = l->data;
- if (cb->suspend_cb) {
- cb->suspend_cb(s->session, s->err, cb->user_data);
- cb->suspend_cb = NULL;
- setup_unref(s);
- }
+ l = l->next;
+
+ if (!cb->suspend_cb)
+ continue;
+
+ cb->suspend_cb(s->session, s->err, cb->user_data);
+ s->cb = g_slist_remove(s->cb, cb);
+ g_free(cb);
+ setup_unref(s);
}
setup_unref(s);
@@ -242,14 +253,18 @@ static gboolean finalize_select(struct a2dp_setup *s, GSList *caps)
GSList *l;
setup_ref(s);
- for (l = s->cb; l != NULL; l = l->next) {
+ for (l = s->cb; l != NULL; ) {
struct a2dp_setup_cb *cb = l->data;
- if (cb->select_cb) {
- cb->select_cb(s->session, s->sep, caps, cb->user_data);
- cb->select_cb = NULL;
- setup_unref(s);
- }
+ l = l->next;
+
+ if (!cb->select_cb)
+ continue;
+
+ cb->select_cb(s->session, s->sep, caps, cb->user_data);
+ s->cb = g_slist_remove(s->cb, cb);
+ g_free(cb);
+ setup_unref(s);
}
setup_unref(s);
--
1.7.1
^ permalink raw reply related
* [PATCH 2/2] Fix not aborting sink stream configuration on disconnect
From: Luiz Augusto von Dentz @ 2010-11-11 9:40 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1289468425-11997-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
If stream configuration is not complete it should be aborted so we can
proceed with disconnection process.
---
audio/sink.c | 28 +++++++++++++++++++++++++++-
1 files changed, 27 insertions(+), 1 deletions(-)
diff --git a/audio/sink.c b/audio/sink.c
index a9f6307..cb3ca74 100644
--- a/audio/sink.c
+++ b/audio/sink.c
@@ -351,6 +351,12 @@ static void discovery_complete(struct avdtp *session, GSList *seps, struct avdtp
struct pending_request *pending;
int id;
+ if (!sink->connect) {
+ avdtp_unref(sink->session);
+ sink->session = NULL;
+ return;
+ }
+
pending = sink->connect;
if (err) {
@@ -669,11 +675,31 @@ gboolean sink_new_stream(struct audio_device *dev, struct avdtp *session,
gboolean sink_shutdown(struct sink *sink)
{
- if (!sink->stream)
+ if (!sink->session)
return FALSE;
avdtp_set_device_disconnect(sink->session, TRUE);
+ /* cancel pending connect */
+ if (sink->connect) {
+ struct pending_request *pending = sink->connect;
+
+ if (pending->msg)
+ error_failed(pending->conn, pending->msg,
+ "Stream setup failed");
+ pending_request_free(sink->dev, pending);
+ sink->connect = NULL;
+
+ return TRUE;
+ }
+
+ /* disconnect already ongoing */
+ if (sink->disconnect)
+ return TRUE;
+
+ if (!sink->stream)
+ return FALSE;
+
if (avdtp_close(sink->session, sink->stream, FALSE) < 0)
return FALSE;
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 2/7] Refactor get_eir_uuids() to get EIR data length parameter
From: Johan Hedberg @ 2010-11-11 9:42 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1289411487-6113-2-git-send-email-anderson.lizardo@openbossa.org>
Hi,
On Wed, Nov 10, 2010, Anderson Lizardo wrote:
> get_eir_uuids() will be reused to parse LE advertising data as well, as
> they share the same format. But for Advertising, maximum data length is
> different (31 bytes vs. 240 bytes for EIR), and the radio is not
> required to send the non-significant (zero-filled) bytes.
>
> adapter_emit_device_found() now also accepts a EIR data length
> parameter, so it can be reused for LE and can propagate the exact data
> length.
> ---
> src/adapter.c | 17 ++++++++++-------
> src/adapter.h | 2 +-
> src/event.c | 2 +-
> 3 files changed, 12 insertions(+), 9 deletions(-)
This patch seems fine, but it doesn't apply anymore since the first
patch of the set had issues and I didn't apply it.
Johan
^ permalink raw reply
* Re: [PATCH 3/7] Refactoring adapter_update_found_devices() function
From: Johan Hedberg @ 2010-11-11 10:01 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth, Bruna Moreira
In-Reply-To: <1289411487-6113-3-git-send-email-anderson.lizardo@openbossa.org>
Hi,
On Wed, Nov 10, 2010, Anderson Lizardo wrote:
> +void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
> + int8_t rssi, uint32_t class, const char *name,
> + const char *alias, gboolean legacy,
> + name_status_t name_status, uint8_t *eir_data)
> +{
> + struct remote_dev_info *dev;
> +
> + if (!update_found_devices(adapter, bdaddr, rssi, &dev)) {
> + dev->class = class;
> + if (name)
> + dev->name = g_strdup(name);
> + if (alias)
> + dev->alias = g_strdup(alias);
> + dev->legacy = legacy;
> + dev->name_status = name_status;
> + } else if (dev->rssi == rssi)
> + return;
I find the name and signature of update_found_devices() a little bit
unintuitive which makes following the code that uses it a bit hard.
Additionally it seems you're not using the rssi anymore within the
update_found_devices function so there's no point in passing the rssi to
it. To make the code more readable, could you change the function so
that its usage would look something like:
struct remote_dev_info *dev;
gboolean new_dev;
dev = get_found_dev(adapter, bdaddr, &new_dev);
if (new_dev) {
<set the fields for the new device>
} else if (dev->rssi == rssi)
return;
dev->rssi = rssi;
...
Since the rest of the patches seem to depend on this one I'll stop the
review here and wait until you fix the issues I've mentioned so far.
Johan
^ permalink raw reply
* Re: [PATCH] Print LE link type on hcitool
From: Johan Hedberg @ 2010-11-11 10:08 UTC (permalink / raw)
To: Sheldon Demario; +Cc: linux-bluetooth
In-Reply-To: <1289415288-18470-1-git-send-email-sheldon.demario@openbossa.org>
Hi Sheldon,
On Wed, Nov 10, 2010, Sheldon Demario wrote:
> ---
> lib/hci.h | 1 +
> tools/hcitool.c | 2 ++
> 2 files changed, 3 insertions(+), 0 deletions(-)
Pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] Use reference counting of the device object while discovering services
From: Johan Hedberg @ 2010-11-11 10:08 UTC (permalink / raw)
To: Claudio Takahasi; +Cc: linux-bluetooth
In-Reply-To: <1289426161-10045-1-git-send-email-claudio.takahasi@openbossa.org>
Hi Claudio,
On Wed, Nov 10, 2010, Claudio Takahasi wrote:
> ---
> attrib/client.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
Pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] Fix possible memory leak of the GIOChannel in the attribute server
From: Johan Hedberg @ 2010-11-11 10:09 UTC (permalink / raw)
To: Claudio Takahasi; +Cc: linux-bluetooth
In-Reply-To: <1289426161-10045-2-git-send-email-claudio.takahasi@openbossa.org>
Hi Claudio,
On Wed, Nov 10, 2010, Claudio Takahasi wrote:
> ---
> src/attrib-server.c | 32 +++++++++++++++++++-------------
> 1 files changed, 19 insertions(+), 13 deletions(-)
Pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH 1/3] Split up pbap object and pbap session
From: Johan Hedberg @ 2010-11-11 10:25 UTC (permalink / raw)
To: Dmitriy Paliy; +Cc: linux-bluetooth
In-Reply-To: <1289460972-2971-1-git-send-email-dmitriy.paliy@nokia.com>
Hi Dmitriy,
On Thu, Nov 11, 2010, Dmitriy Paliy wrote:
> +static void *vobject_create(void *user_data)
There's no reason for the void * types here. Use specific types instead.
> @@ -718,10 +736,13 @@ static void *vobject_pull_open(const char *name, int oflag, mode_t mode,
> cb = query_result;
>
> ret = phonebook_pull(name, pbap->params, cb, pbap);
> +
> if (ret < 0)
> goto fail;
No need to add the empty line.
>
> - return pbap;
> + obj = vobject_create(pbap);
> +
> + return obj;
Just do "return vobject_create(pbap);"
> + obj = vobject_create(pbap);
> +
> + return obj;
Same here.
> + obj = vobject_create(pbap);
> +
> + return obj;
And here.
Johan
^ permalink raw reply
* Re: [PATCH 2/3] Code clean up: pbap->params = params removed
From: Johan Hedberg @ 2010-11-11 10:26 UTC (permalink / raw)
To: Dmitriy Paliy; +Cc: linux-bluetooth
In-Reply-To: <1289460972-2971-2-git-send-email-dmitriy.paliy@nokia.com>
Hi Dmitriy,
On Thu, Nov 11, 2010, Dmitriy Paliy wrote:
> pbap->params = params; removed due to the fact that this assignment is
> already used in the same function.
> ---
> plugins/pbap.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/plugins/pbap.c b/plugins/pbap.c
> index 7b9f1ff..af60741 100644
> --- a/plugins/pbap.c
> +++ b/plugins/pbap.c
> @@ -617,7 +617,6 @@ static int pbap_get(struct obex_session *os, obex_object_t *obj,
> if (path == NULL)
> return -EBADR;
>
> - pbap->params = params;
> ret = obex_get_stream_start(os, path);
>
> g_free(path);
Pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH 3/3] Code clean up: cache->folder removed
From: Johan Hedberg @ 2010-11-11 10:26 UTC (permalink / raw)
To: Dmitriy Paliy; +Cc: linux-bluetooth
In-Reply-To: <1289460972-2971-3-git-send-email-dmitriy.paliy@nokia.com>
Hi Dmitriy,
On Thu, Nov 11, 2010, Dmitriy Paliy wrote:
> cache->folder is not used anywhere and therefore removed.
> ---
> plugins/pbap.c | 2 --
> 1 files changed, 0 insertions(+), 2 deletions(-)
>
> diff --git a/plugins/pbap.c b/plugins/pbap.c
> index af60741..660b17d 100644
> --- a/plugins/pbap.c
> +++ b/plugins/pbap.c
> @@ -125,7 +125,6 @@ struct aparam_header {
> struct cache {
> gboolean valid;
> uint32_t index;
> - char *folder;
> GSList *entries;
> };
>
> @@ -219,7 +218,6 @@ static const char *cache_find(struct cache *cache, uint32_t handle)
>
> static void cache_clear(struct cache *cache)
> {
> - g_free(cache->folder);
> g_slist_foreach(cache->entries, (GFunc) cache_entry_free, NULL);
> g_slist_free(cache->entries);
> cache->entries = NULL;
This one is also now upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH 1/2] Remove a2dp setup callbacks after they return
From: Johan Hedberg @ 2010-11-11 10:36 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1289468425-11997-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
On Thu, Nov 11, 2010, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
>
> Since the callback won't be ever called again it make no sense to keep
> them, also this cause a2dp_cancel to assume there are still some pending
> callbacks to be processed and do not abort when it should.
> ---
> audio/a2dp.c | 55 +++++++++++++++++++++++++++++++++++--------------------
> 1 files changed, 35 insertions(+), 20 deletions(-)
Both patches have been pushed upstream. Thanks.
Johan
^ permalink raw reply
* Scripts which have been used to calculate the 'magic' constants for sbc encoder
From: Siarhei Siamashka @ 2010-11-11 10:47 UTC (permalink / raw)
To: linux-bluetooth
[-- Attachment #1.1: Type: Text/Plain, Size: 565 bytes --]
Hi,
The scripts which have been used to calculate the 'magic' constants introduced
in commit:
http://git.kernel.org/?p=bluetooth/bluez.git;a=commit;h=f70d1ada0aba5567fbf67cedfb4a4ba1a9f9852e
Remembered that these are not available in bluez repository so sending them
here basically for archival purposes. I also had scripts for finding a suitable
SIMD permutation, but IIRC these were totally unmaintainable (required several
scripts to run, with human intervention in between and tweaking intermediate
data).
--
Best regards,
Siarhei Siamashka
[-- Attachment #1.2: sbc-encoder-quality-tuning-scripts.tar.gz --]
[-- Type: application/x-compressed-tar, Size: 4686 bytes --]
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH] Add iwmmxt optimization for sbc for pxa series cpu
From: Siarhei Siamashka @ 2010-11-11 11:46 UTC (permalink / raw)
To: Keith Mok; +Cc: linux-bluetooth
In-Reply-To: <AANLkTikPiwr+OtsDVbgoya281Zncg0P5Joq_v0UCzZDi@mail.gmail.com>
On Thursday 11 November 2010 10:05:46 Keith Mok wrote:
> This patch add iwmmxt (Intel wireless mmx, pxa platform) optimzation
> for sbc, based on the mmx code.
> Have verified the encoded result against the mmx generated one.
Nice, I guess it should provide a noticeable performance improvement on this
hardware.
Did you run some benchmarks with these optimizations to measure how much they
are helping? The most interesting numbers are for the "44100kHz audio
with bitpool set to 53, 8 subbands, joint stereo" case, which is typically
used for A2DP. This can be done by running:
$ time ./sbcenc -b53 -s8 -j test.au > /dev/null
In my opinion, commit messages for the performance patches are more descriptive
in the following format:
http://git.kernel.org/?p=bluetooth/bluez.git;a=commit;h=e80454d08b4ec098024ddfbdffbd71e9d2f81bd0
And splitting the patch into parts, adding one optimization at a time may be a
good idea (for bisecting purposes).
A few other comments below.
I don't have any IWMMXT capable hardware to test/benchmark, but I checked the
following manuals:
http://download.intel.com/design/intelxscale/31451001.pdf
http://download.intel.com/design/intelxscale/27347302.pdf
> +static inline void sbc_analyze_four_iwmmxt(const int16_t *in, int32_t
> *out, + const FIXED_T *consts)
> +{
> + asm volatile (
> + "tbcstw wr4, %2\n"
> + "wldrd wr0, [%0]\n"
> + "wldrd wr1, [%0, #8]\n"
> + "wldrd wr2, [%1]\n"
> + "wldrd wr3, [%1, #8]\n"
Using back-to-back WLDRD instructions has some performance penalty
"D.3.2.3 Memory Control Pipeline
There is also an additional stall introduced by the core when 2 double word (64
bits) are issued back to back such as:
WLDRD or WSTRD
WLDR[B,H,W,D] or WSTR[B,H,W,D] <- 1 cycle stall.
Critical inner loop sequences can use non memory related instructions following
a WLDRD or WSTRD."
It's better to try rearranging the code so that load instructions are
interleaved with the others whenever it is possible.
> + "wmadds wr0, wr2, wr0\n"
> + "wmadds wr1, wr3, wr1\n"
> + "waddwss wr0, wr0, wr4\n"
> + "waddwss wr1, wr1, wr4\n"
> + "\n"
> + "wldrd wr2, [%0, #16]\n"
> + "wldrd wr3, [%0, #24]\n"
> + "wldrd wr4, [%1, #16]\n"
^^^^^^ (1)
> + "wldrd wr5, [%1, #24]\n"
> + "wmadds wr2, wr4, wr2\n"
^^^^^^^ (2)
It also makes sense to pay attention to instruction latencies. Here you use wr4
register (2) after loading (1) with only one unrelated instruction in between.
And according to "Table D-1. Issue Cycle and Result Latency of the Intel®
Wireless MMX™ 2 Coprocessor Instructions", WLDRD has result latency 3, so that
it works best if you insert 2 unrelated instruction in between.
> + "wmadds wr3, wr5, wr3\n"
> + "waddwss wr0, wr2, wr0\n"
> + "waddwss wr1, wr3, wr1\n"
> + "\n"
> + "wldrd wr2, [%0, #32]\n"
> + "wldrd wr3, [%0, #40]\n"
> + "wldrd wr4, [%1, #32]\n"
> + "wldrd wr5, [%1, #40]\n"
> + "wmadds wr2, wr4, wr2\n"
> + "wmadds wr3, wr5, wr3\n"
According to "Table D-3. Resource Availability Delay for the Multiplier
Pipeline", back-to-back WMADD instructions may have a performance penalty.
> + "waddwss wr0, wr2, wr0\n"
> + "waddwss wr1, wr3, wr1\n"
> + "\n"
> + "wldrd wr2, [%0, #48]\n"
> + "wldrd wr3, [%0, #56]\n"
> + "wldrd wr4, [%1, #48]\n"
> + "wldrd wr5, [%1, #56]\n"
> + "wmadds wr2, wr4, wr2\n"
> + "wmadds wr3, wr5, wr3\n"
> + "waddwss wr0, wr2, wr0\n"
> + "waddwss wr1, wr3, wr1\n"
> + "\n"
> + "wldrd wr2, [%0, #64]\n"
> + "wldrd wr3, [%0, #72]\n"
> + "wldrd wr4, [%1, #64]\n"
> + "wldrd wr5, [%1, #72]\n"
> + "wmadds wr2, wr4, wr2\n"
> + "wmadds wr3, wr5, wr3\n"
> + "waddwss wr0, wr2, wr0\n"
> + "waddwss wr1, wr3, wr1\n"
> + "\n"
> + "tmcr wcgr0, %4\n"
> + "wsrawg wr0, wr0, wcgr0\n"
> + "wsrawg wr1, wr1, wcgr0\n"
> + "wpackwss wr0, wr0, wr0\n"
> + "wpackwss wr1, wr1, wr1\n"
> + "\n"
> + "wldrd wr4, [%1, #80]\n"
> + "wldrd wr5, [%1, #88]\n"
> + "wldrd wr6, [%1, #96]\n"
> + "wldrd wr7, [%1, #104]\n"
> + "wmadds wr2, wr5, wr0\n"
> + "wmadds wr0, wr4, wr0\n"
> + "\n"
> + "wmadds wr3, wr7, wr1\n"
> + "wmadds wr1, wr6, wr1\n"
> + "waddwss wr0, wr1, wr0\n"
> + "waddwss wr2, wr3, wr2\n"
> + "\n"
> + "wstrd wr0, [%3]\n"
> + "wstrd wr2, [%3, #8]\n"
> + :
> + : "r" (in), "r" (consts),
> + "r" (1 << (SBC_PROTO_FIXED4_SCALE - 1)), "r" (out),
> + "r" (SBC_PROTO_FIXED4_SCALE)
> + : "memory");
> +}
> +static void sbc_calc_scalefactors_iwmmxt(
> + int32_t sb_sample_f[16][2][8],
> + uint32_t scale_factor[2][8],
> + int blocks, int channels, int subbands)
> +{
> + int ch, sb;
> + intptr_t blk;
> + for (ch = 0; ch < channels; ch++) {
> + for (sb = 0; sb < subbands; sb += 2) {
> + int b;
> + blk = &sb_sample_f[0][ch][sb];
> + b = blocks;
> + asm volatile (
> + "tbcstw wr0, %4\n"
> + "1:\n"
> + "wldrd wr1, [%0], %2\n"
> + "wxor wr2, wr2, wr2\n"
> + "wcmpgtsw wr3, wr1, wr2\n"
The MMX code was using PCMPGTD and the other instructions just because MMX
instruction set is very limited and did not have the needed instructions. But
you can use WABS and WMAX instructions to do this job better. You can refer to
the original C code and also to ARM NEON optimizations to get some ideas about
how to do this operation faster.
> + "waddwss wr1, wr1, wr3\n"
> + "wcmpgtsw wr2, wr2, wr1\n"
> + "wxor wr1, wr1, wr2\n"
> +
> + "wor wr0, wr0, wr1\n"
> +
> + "subs %1, %1, #1\n"
> + "bne 1b\n"
> +
> + "tmrrc %0, %1, wr0\n"
> + "clz %0, %0\n"
> + "rsb %0, %0, %5\n"
> + "str %0, [%3]\n"
> +
> + "clz %1, %1\n"
> + "rsb %1, %1, %5\n"
> + "str %1, [%3, #4]\n"
> + : "+&r" (blk), "+&r" (b)
> + : "i" ((char *) &sb_sample_f[1][0][0] -
> + (char *) &sb_sample_f[0][0][0]),
> + "r" (&scale_factor[ch][sb]),
> + "r" (1 << SCALE_OUT_BITS),
> + "i" (SCALE_OUT_BITS+1)
> + : "memory");
And this is actually a bug, which exists in the original MMX code too (my
fault). In order to fix it, "cc" needs to be added to the clobber list. I have
just sent a patch for MMX code here:
http://marc.info/?l=linux-bluetooth&m=128946780706187&w=2
Such bug is more dangerous on ARM, because it is up to the developer whether to
update flags in each particular instruction or not. So while almost every
arithmetic x86 instruction updates flags unconditionally, on ARM the flags can
easily survive long enough. That makes it possible for the compiler to
implement more clever optimizations related to setting and checking flags, and
fail if the clobber list does not contain correct information.
> + }
> + }
> +}
--
Best regards,
Siarhei Siamashka
^ permalink raw reply
* [PATCH 0/1 v2] Split up object and session in pbap.c
From: Dmitriy Paliy @ 2010-11-11 12:08 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1289460972-2971-1-git-send-email-dmitriy.paliy@nokia.com>
Hi,
This is updated version with some improvements to code as given
in comments to previous submission. Also one DBG printout is not
deleted as in previous version of the patch.
BR,
Dmitriy
^ permalink raw reply
* [PATCH 1/1 v2] Split up object and session in pbap.c
From: Dmitriy Paliy @ 2010-11-11 12:08 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Dmitriy Paliy
In-Reply-To: <1289460972-2971-1-git-send-email-dmitriy.paliy@nokia.com>
Object and session data is separated in PBAP plugin. Reason is that
when OBEX session firstly makes disconnect of service_data, which
corresponds to session in pbap.c, it than closes object, which also
corresponds to session in pbap.c.
Memory is deallocated after PBAP session is disconnected. When OBEX
driver closes the object, it is trying to dereference the deallocated
memory in order to free pbap->buffer data.
Here object and session are separated, while pointers are created to
make one-to-one mapping. pbap_object is created in vobject_..._open
functions after query to tracker submitted. Session and object are
handled separately when freed.
---
plugins/pbap.c | 76 +++++++++++++++++++++++++++++++++++--------------------
1 files changed, 48 insertions(+), 28 deletions(-)
diff --git a/plugins/pbap.c b/plugins/pbap.c
index c64377c..4e55c72 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -140,8 +140,13 @@ struct pbap_session {
struct apparam_field *params;
char *folder;
uint32_t find_handle;
- GString *buffer;
struct cache cache;
+ struct pbap_object *obj;
+};
+
+struct pbap_object {
+ GString *buffer;
+ struct pbap_session *session;
};
static const uint8_t PBAP_TARGET[TARGET_SIZE] = {
@@ -237,9 +242,9 @@ static void phonebook_size_result(const char *buffer, size_t bufsize,
hdr->len = PHONEBOOKSIZE_LEN;
memcpy(hdr->val, &phonebooksize, sizeof(phonebooksize));
- pbap->buffer = g_string_new_len(aparam, sizeof(aparam));
+ pbap->obj->buffer = g_string_new_len(aparam, sizeof(aparam));
- obex_object_set_io_flags(pbap, G_IO_IN, 0);
+ obex_object_set_io_flags(pbap->obj, G_IO_IN, 0);
}
static void query_result(const char *buffer, size_t bufsize, int vcards,
@@ -250,17 +255,17 @@ static void query_result(const char *buffer, size_t bufsize, int vcards,
DBG("");
if (vcards <= 0) {
- obex_object_set_io_flags(pbap, G_IO_ERR, -ENOENT);
+ obex_object_set_io_flags(pbap->obj, G_IO_ERR, -ENOENT);
return;
}
- if (!pbap->buffer)
- pbap->buffer = g_string_new_len(buffer, bufsize);
+ if (!pbap->obj->buffer)
+ pbap->obj->buffer = g_string_new_len(buffer, bufsize);
else
- pbap->buffer = g_string_append_len(pbap->buffer, buffer,
+ pbap->obj->buffer = g_string_append_len(pbap->obj->buffer, buffer,
bufsize);
- obex_object_set_io_flags(pbap, G_IO_IN, 0);
+ obex_object_set_io_flags(pbap->obj, G_IO_IN, 0);
}
static void cache_entry_notify(const char *id, uint32_t handle,
@@ -392,7 +397,7 @@ static void cache_ready_notify(void *user_data)
hdr->len = PHONEBOOKSIZE_LEN;
memcpy(hdr->val, &size, sizeof(size));
- pbap->buffer = g_string_new_len(aparam, sizeof(aparam));
+ pbap->obj->buffer = g_string_new_len(aparam, sizeof(aparam));
goto done;
}
@@ -406,29 +411,29 @@ static void cache_ready_notify(void *user_data)
if (sorted == NULL) {
pbap->cache.valid = TRUE;
- obex_object_set_io_flags(pbap, G_IO_ERR, -ENOENT);
+ obex_object_set_io_flags(pbap->obj, G_IO_ERR, -ENOENT);
return;
}
/* Computing offset considering first entry of the phonebook */
l = g_slist_nth(sorted, pbap->params->liststartoffset);
- pbap->buffer = g_string_new(VCARD_LISTING_BEGIN);
+ pbap->obj->buffer = g_string_new(VCARD_LISTING_BEGIN);
for (; l && max; l = l->next, max--) {
const struct cache_entry *entry = l->data;
- g_string_append_printf(pbap->buffer, VCARD_LISTING_ELEMENT,
+ g_string_append_printf(pbap->obj->buffer, VCARD_LISTING_ELEMENT,
entry->handle, entry->name);
}
- pbap->buffer = g_string_append(pbap->buffer, VCARD_LISTING_END);
+ pbap->obj->buffer = g_string_append(pbap->obj->buffer, VCARD_LISTING_END);
g_slist_free(sorted);
done:
if (!pbap->cache.valid) {
pbap->cache.valid = TRUE;
- obex_object_set_io_flags(pbap, G_IO_IN, 0);
+ obex_object_set_io_flags(pbap->obj, G_IO_IN, 0);
}
}
@@ -445,14 +450,14 @@ static void cache_entry_done(void *user_data)
id = cache_find(&pbap->cache, pbap->find_handle);
if (id == NULL) {
DBG("Entry %d not found on cache", pbap->find_handle);
- obex_object_set_io_flags(pbap, G_IO_ERR, -ENOENT);
+ obex_object_set_io_flags(pbap->obj, G_IO_ERR, -ENOENT);
return;
}
ret = phonebook_get_entry(pbap->folder, id, pbap->params,
query_result, pbap);
if (ret < 0)
- obex_object_set_io_flags(pbap, G_IO_ERR, ret);
+ obex_object_set_io_flags(pbap->obj, G_IO_ERR, ret);
}
static struct apparam_field *parse_aparam(const uint8_t *buffer, uint32_t hlen)
@@ -689,6 +694,17 @@ static struct obex_service_driver pbap = {
.chkput = pbap_chkput
};
+static struct pbap_object *vobject_create(struct pbap_session *pbap)
+{
+ struct pbap_object *obj;
+
+ obj = g_new0(struct pbap_object, 1);
+ obj->session = pbap;
+ pbap->obj = obj;
+
+ return obj;
+}
+
static void *vobject_pull_open(const char *name, int oflag, mode_t mode,
void *context, size_t *size, int *err)
{
@@ -718,7 +734,7 @@ static void *vobject_pull_open(const char *name, int oflag, mode_t mode,
if (ret < 0)
goto fail;
- return pbap;
+ return vobject_create(pbap);
fail:
if (err)
@@ -752,7 +768,7 @@ static void *vobject_list_open(const char *name, int oflag, mode_t mode,
* Valid cache and empty buffer mean that cache was already
* created within a single session, but no data is available.
*/
- if (!pbap->buffer) {
+ if (!pbap->obj->buffer) {
ret = -ENOENT;
goto fail;
}
@@ -768,7 +784,7 @@ static void *vobject_list_open(const char *name, int oflag, mode_t mode,
goto fail;
done:
- return pbap;
+ return vobject_create(pbap);
fail:
if (err)
@@ -817,7 +833,7 @@ done:
if (ret < 0)
goto fail;
- return pbap;
+ return vobject_create(pbap);
fail:
if (err)
@@ -829,12 +845,13 @@ fail:
static ssize_t vobject_pull_read(void *object, void *buf, size_t count,
uint8_t *hi)
{
- struct pbap_session *pbap = object;
+ struct pbap_object *obj = object;
+ struct pbap_session *pbap = obj->session;
- DBG("buffer %p maxlistcount %d", pbap->buffer,
+ DBG("buffer %p maxlistcount %d", obj->buffer,
pbap->params->maxlistcount);
- if (!pbap->buffer)
+ if (!obj->buffer)
return -EAGAIN;
/* PhoneBookSize */
@@ -844,13 +861,14 @@ static ssize_t vobject_pull_read(void *object, void *buf, size_t count,
/* Stream data */
*hi = OBEX_HDR_BODY;
- return string_read(pbap->buffer, buf, count);
+ return string_read(obj->buffer, buf, count);
}
static ssize_t vobject_list_read(void *object, void *buf, size_t count,
uint8_t *hi)
{
- struct pbap_session *pbap = object;
+ struct pbap_object *obj = object;
+ struct pbap_session *pbap = obj->session;
DBG("valid %d maxlistcount %d", pbap->cache.valid,
pbap->params->maxlistcount);
@@ -864,13 +882,13 @@ static ssize_t vobject_list_read(void *object, void *buf, size_t count,
else
*hi = OBEX_HDR_BODY;
- return string_read(pbap->buffer, buf, count);
+ return string_read(obj->buffer, buf, count);
}
static ssize_t vobject_vcard_read(void *object, void *buf, size_t count,
uint8_t *hi)
{
- struct pbap_session *pbap = object;
+ struct pbap_object *pbap = object;
DBG("buffer %p", pbap->buffer);
@@ -883,13 +901,15 @@ static ssize_t vobject_vcard_read(void *object, void *buf, size_t count,
static int vobject_close(void *object)
{
- struct pbap_session *pbap = object;
+ struct pbap_object *pbap = object;
if (pbap->buffer) {
string_free(pbap->buffer);
pbap->buffer = NULL;
}
+ g_free(pbap);
+
return 0;
}
--
1.7.0.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox