-----Original Message----- From: Liam Girdwood [mailto:liam.r.girdwood@linux.intel.com] Sent: Tuesday, December 20, 2016 6:31 PM To: Keyon Jie yang.jie@linux.intel.com Cc: sound-open-firmware@alsa-project.org; Zhang, Keqiao keqiao.zhang@intel.com; Jie, Yang yang.jie@intel.com; Ingalsuo, Seppo seppo.ingalsuo@intel.com; Lin, Mengdong mengdong.lin@intel.com Subject: Re: [Sound-open-firmware] [PATCH 3/8] volume: add 16bit<==>24bit volume copy function and mapping
On Tue, 2016-12-20 at 15:46 +0800, Keyon Jie wrote:
add 16bit<==>24bit volume copy function and mapping, for 24 bits ssp output/input.
Signed-off-by: Keyon Jie yang.jie@linux.intel.com
src/audio/volume.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)
diff --git a/src/audio/volume.c b/src/audio/volume.c index f5528cd..5299edd 100644 --- a/src/audio/volume.c +++ b/src/audio/volume.c @@ -172,12 +172,55 @@ static void vol_s16_to_s16(struct comp_dev *dev,
struct comp_buffer *sink,
sink->w_ptr = dest; }
+/* copy and scale volume from 16 bit source buffer to 24 bit on 32 +bit boundary dest buffer */ static void vol_s16_to_s24(struct comp_dev *dev,
struct comp_buffer *sink,
- struct comp_buffer *source, uint32_t frames) {
- struct comp_data *cd = comp_get_drvdata(dev);
- int16_t *src = (int16_t*) source->r_ptr;
- int32_t val, i, *dest = (int32_t*) sink->w_ptr;
- /* buffer sizes are always divisible by period frames */
- for (i = 0; i < frames * source->params.channels; i++) {
val = (int32_t)*src;*dest = (val * cd->volume[i % source->params.channels]) >> 8;We must not use % or / in pipeline copy code since they are very expensive in terms of cycles.
OK, it do add numbers of instruction lines with this, thanks for pointing out, will change in next version.
Thanks, ~Keyon
What we should do here is have a version of each function, for 2 chan, 4, chan, etc (select at params time).
e.g. 2 channel volume
/* buffer sizes are always divisible by period frames */ for (i = 0; i < frames * 2; i += 2) { val[0] = (int32_t)src[i]; val[1] = (int32_t)src[i + 1];
dest[i] = (int16_t)(((val[0] >> 8) * cd->volume[0]) >> 16); dest[i + 1] = (int16_t)(((val[1] >> 8) * cd->volume[1]) >> 16);}
The above code does the same, but doesnt use % and is vectorisable by gcc (and also easier to add intrinsics).
Our goal is to keep the copy() functions as optimal as possible (even at the expense of adding a little more code for copy functions using a different number of channels).
Liam
dest++;src++;- }
- source->r_ptr = src;
- sink->w_ptr = dest;
+}
+/* copy and scale volume from 16 bit source buffer to 24 bit on 32 +bit boundary dest buffer */ static void vol_s24_to_s16(struct comp_dev *dev,
struct comp_buffer *sink,
- struct comp_buffer *source, uint32_t frames) {
- struct comp_data *cd = comp_get_drvdata(dev);
- int32_t val, i, *src = (int32_t*) source->r_ptr;
- int16_t *dest = (int16_t*) sink->w_ptr;
- /* buffer sizes are always divisible by period frames */
- for (i = 0; i < frames * source->params.channels; i++) {
val = (int32_t)*src;*dest = (int16_t)(((val >> 8) *cd->volume[i % source->params.channels]) >> 16);dest++;src++;- }
- source->r_ptr = src;
- sink->w_ptr = dest;
+}
/* map of source and sink buffer formats to volume function */ static const struct comp_func_map func_map[] = { {STREAM_FORMAT_S16_LE, STREAM_FORMAT_S16_LE,
vol_s16_to_s16},
{STREAM_FORMAT_S16_LE, STREAM_FORMAT_S32_LE,
vol_s16_to_s32},
{STREAM_FORMAT_S32_LE, STREAM_FORMAT_S16_LE,
vol_s32_to_s16},
{STREAM_FORMAT_S32_LE, STREAM_FORMAT_S32_LE,
vol_s32_to_s32},
- {STREAM_FORMAT_S16_LE, STREAM_FORMAT_S24_4LE,
vol_s16_to_s24},
- {STREAM_FORMAT_S24_4LE, STREAM_FORMAT_S16_LE,
vol_s24_to_s16},
};
static void vol_update(struct comp_data *cd, uint32_t chan)