[PATCH] ASoC: wm_adsp: Improve handling of raw byte streams

* [PATCH] ASoC: wm_adsp: Improve handling of raw byte streams
@ 2020-12-16 11:25 Richard Fitzgerald
  2020-12-26 16:13 ` kernel test robot
  2020-12-28 16:03 ` Mark Brown
  0 siblings, 2 replies; 3+ messages in thread
From: Richard Fitzgerald @ 2020-12-16 11:25 UTC (permalink / raw)
  To: broonie; +Cc: patches, alsa-devel, linux-kernel, Richard Fitzgerald

As the register map is 16-bit or 32-bit big-endian, the 24-bit
DSP words appear padded and with the bytes swapped. When reading a
raw stream of bytes, the pad bytes must be removed and the data bytes
swapped back to their original order.

The previous implementation of this assumed that the be32_to_cpu() in
wm_adsp_read_data_block() would swap back to little-endian. But this is
obviously only true on a little-endian CPU. It also made two walks
through the data, once to endian-swap and again to strip the pad bytes.

This patch re-works the code so that the endian-swap and unpad are done
together in a single walk, and it is not dependent on the endianness of
the CPU. The data_word_size argument to wm_adsp_remove_padding() has been
dropped because currently this is always 3.

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
 sound/soc/codecs/wm_adsp.c | 55 +++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 25 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index bcf18bf15a02..7c52ada10af3 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -3662,12 +3662,12 @@ int wm_adsp_compr_get_caps(struct snd_soc_component *component,
 }
 EXPORT_SYMBOL_GPL(wm_adsp_compr_get_caps);
 
-static int wm_adsp_read_data_block(struct wm_adsp *dsp, int mem_type,
-				   unsigned int mem_addr,
-				   unsigned int num_words, u32 *data)
+static int wm_adsp_read_raw_data_block(struct wm_adsp *dsp, int mem_type,
+				       unsigned int mem_addr,
+				       unsigned int num_words, __be32 *data)
 {
 	struct wm_adsp_region const *mem = wm_adsp_find_region(dsp, mem_type);
-	unsigned int i, reg;
+	unsigned int reg;
 	int ret;
 
 	if (!mem)
@@ -3680,16 +3680,22 @@ static int wm_adsp_read_data_block(struct wm_adsp *dsp, int mem_type,
 	if (ret < 0)
 		return ret;
 
-	for (i = 0; i < num_words; ++i)
-		data[i] = be32_to_cpu(data[i]) & 0x00ffffffu;
-
 	return 0;
 }
 
 static inline int wm_adsp_read_data_word(struct wm_adsp *dsp, int mem_type,
 					 unsigned int mem_addr, u32 *data)
 {
-	return wm_adsp_read_data_block(dsp, mem_type, mem_addr, 1, data);
+	__be32 raw;
+	int ret;
+
+	ret = wm_adsp_read_raw_data_block(dsp, mem_type, mem_addr, 1, &raw);
+	if (ret)
+		return ret;
+
+	*data = be32_to_cpu(raw) & 0x00ffffffu;
+
+	return 0;
 }
 
 static int wm_adsp_write_data_word(struct wm_adsp *dsp, int mem_type,
@@ -3722,18 +3728,22 @@ static inline int wm_adsp_buffer_write(struct wm_adsp_compr_buf *buf,
 				       buf->host_buf_ptr + field_offset, data);
 }
 
-static void wm_adsp_remove_padding(u32 *buf, int nwords, int data_word_size)
+static void wm_adsp_remove_padding(u32 *buf, int nwords)
 {
-	u8 *pack_in = (u8 *)buf;
+	const __be32 *pack_in = (__be32 *)buf;
 	u8 *pack_out = (u8 *)buf;
-	int i, j;
+	int i;
 
-	/* Remove the padding bytes from the data read from the DSP */
+	/*
+	 * DSP words from the register map have pad bytes and the data bytes
+	 * are in swapped order. This swaps back to the original little-endian
+	 * order and strips the pad bytes.
+	 */
 	for (i = 0; i < nwords; i++) {
-		for (j = 0; j < data_word_size; j++)
-			*pack_out++ = *pack_in++;
-
-		pack_in += sizeof(*buf) - data_word_size;
+		u32 word = be32_to_cpu(*pack_in++);
+		*pack_out++ = (u8)word;
+		*pack_out++ = (u8)(word >> 8);
+		*pack_out++ = (u8)(word >> 16);
 	}
 }
 
@@ -3917,12 +3927,7 @@ static int wm_adsp_buffer_parse_coeff(struct wm_coeff_ctl *ctl)
 		return -EINVAL;
 	}
 
-	for (i = 0; i < ARRAY_SIZE(coeff_v1.name); i++)
-		coeff_v1.name[i] = be32_to_cpu(coeff_v1.name[i]);
-
-	wm_adsp_remove_padding((u32 *)&coeff_v1.name,
-			       ARRAY_SIZE(coeff_v1.name),
-			       WM_ADSP_DATA_WORD_SIZE);
+	wm_adsp_remove_padding((u32 *)&coeff_v1.name, ARRAY_SIZE(coeff_v1.name));
 
 	buf->name = kasprintf(GFP_KERNEL, "%s-dsp-%s", ctl->dsp->part,
 			      (char *)&coeff_v1.name);
@@ -4261,12 +4266,12 @@ static int wm_adsp_buffer_capture_block(struct wm_adsp_compr *compr, int target)
 		return 0;
 
 	/* Read data from DSP */
-	ret = wm_adsp_read_data_block(buf->dsp, mem_type, adsp_addr,
-				      nwords, compr->raw_buf);
+	ret = wm_adsp_read_raw_data_block(buf->dsp, mem_type, adsp_addr,
+					  nwords, compr->raw_buf);
 	if (ret < 0)
 		return ret;
 
-	wm_adsp_remove_padding(compr->raw_buf, nwords, WM_ADSP_DATA_WORD_SIZE);
+	wm_adsp_remove_padding(compr->raw_buf, nwords);
 
 	/* update read index to account for words read */
 	buf->read_index += nwords;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread