[PATCH 1/2] ASoC: meson: gx-formatter: prepare on attach

[Valerio Setti]

Instead of calling the formatter driver prepare and enable at the same
time when 'gx_stream_start' is called (which happens at trigger time),
split the operation in 2: prepare is called when the widget is powered
up, whereas enable is kept on 'gx_stream_start'.

This resolves a problem related to i2s playback in 24-bit mode. Commit
c7852d2dcf66 ("ASoC: meson: aiu: align I2S design to the AXG one") moved
the content of what was once called 'aiu_encoder_i2s_setup_desc' from
'aiu-encoder-i2s' to 'aiu-formatter-i2s'.

'aiu_encoder_i2s_setup_desc' was basically accomplishing two tasks:
- reset the i2s pipeline.
- configure number of channels and physical samples width.
Before being moved 'aiu_encoder_i2s_setup_desc' was called in the encoder
DAI 'hw_params()', whereas after the move it is called at trigger time
('aiu_encoder_i2s_trigger'->'gx_stream_start' -> 'gx_formatter_enable' ->
'aiu_formatter_i2s_prepare').

In parallel 'aiu-fifo-i2s' (DAI FE) already performs the very same reset
of the pipeline at trigger time in 'aiu_fifo_i2s_trigger' and then it
triggers the playback.

Since the DAI triggering order is the default one (FE before BE) this
means that the pipeline reset happens when the BE already did it and
started the playback. This cause the 24-bit playback mode to be
corrupted.

This commit re-orders operations so that 'aiu_formatter_i2s_prepare' is
called after DAIs' prepare, but before triggering them.

Signed-off-by: Valerio Setti <vsetti at baylibre.com>
---
 sound/soc/meson/gx-formatter.c | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/sound/soc/meson/gx-formatter.c b/sound/soc/meson/gx-formatter.c
index 311e63affb23..c7333f624b15 100644
--- a/sound/soc/meson/gx-formatter.c
+++ b/sound/soc/meson/gx-formatter.c
@@ -15,27 +15,38 @@ struct gx_formatter {
 	struct gx_stream *stream;
 	const struct gx_formatter_driver *drv;
 	bool enabled;
+	bool prepared;
 	struct regmap *map;
 };
 
-static int gx_formatter_enable(struct gx_formatter *formatter)
+static int gx_formatter_prepare(struct gx_formatter *formatter)
 {
 	int ret;
 
-	/* Do nothing if the formatter is already enabled */
-	if (formatter->enabled)
+	if (formatter->prepared)
 		return 0;
 
 	/* Setup the stream parameter in the formatter */
 	if (formatter->drv->ops->prepare) {
 		ret = formatter->drv->ops->prepare(formatter->map,
-					   formatter->drv->quirks,
-					   formatter->stream);
+						   formatter->drv->quirks,
+						   formatter->stream);
 		if (ret)
 			return ret;
 	}
 
-	/* Finally, actually enable the formatter */
+	formatter->prepared = true;
+
+	return 0;
+}
+
+static int gx_formatter_enable(struct gx_formatter *formatter)
+{
+	/* Do nothing if the formatter is already enabled */
+	if (formatter->enabled)
+		return 0;
+
+	/* Enable the formatter */
 	if (formatter->drv->ops->enable)
 		formatter->drv->ops->enable(formatter->map);
 
@@ -63,6 +74,12 @@ static int gx_formatter_attach(struct gx_formatter *formatter)
 
 	mutex_lock(&ts->lock);
 
+	ret = gx_formatter_prepare(formatter);
+	if (ret) {
+		pr_err("failed to prepare the formatter\n");
+		goto out;
+	}
+
 	/* Catch up if the stream is already running when we attach */
 	if (ts->ready) {
 		ret = gx_formatter_enable(formatter);
@@ -87,9 +104,9 @@ static void gx_formatter_detach(struct gx_formatter *formatter)
 
 	mutex_lock(&ts->lock);
 	list_del(&formatter->list);
-	mutex_unlock(&ts->lock);
-
 	gx_formatter_disable(formatter);
+	formatter->prepared = false;
+	mutex_unlock(&ts->lock);
 }
 
 static int gx_formatter_power_up(struct gx_formatter *formatter,

-- 
2.47.3