[PATCH 0/9] ASoC: Intel: sst: Fix Bay Trail suspend/resume issues

[PATCH 0/9] ASoC: Intel: sst: Fix Bay Trail suspend/resume issues

[Hans de Goede]

Hi All,

Here is a series fixing suspend/resume errors (and broken audio after
resume) on Bay Trail devices.

Cherry Trail devices also do not like being suspend while audio is
playing, but with different sypmtoms instead of SST_ERR_INVALID_STREAM_ID
errors, if a stream was playing (being allocated is fine, playing is a
problem) before suspend then after resume we get the following errors:

  [ 2312.389974] intel_sst_acpi 808622A8:00: sst: Busy wait failed, cant send this msg
  [ 2312.597976] intel_sst_acpi 808622A8:00: sst: Busy wait failed, cant send this msg
  ...

Until the stream is stopped and restarted after which things work fine
again. I've tried enabling the quirk this commit adds for Cherry Trail
devices too, but that does not help. If anyone has any clues about the CHT
problem, please let me know.

Regards,

Hans

[PATCH 1/9] ASoC: Intel: sst: Fix error-code check in sst_pause_stream()

[Hans de Goede]

The value returned by sst_prepare_and_post_msg() is a negated SST_ERR_*
value, so we must check for -SST_ERR_INVALID_STREAM_ID. Note that
sst_pause_resume() already has the correct check.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/atom/sst/sst_stream.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/intel/atom/sst/sst_stream.c b/sound/soc/intel/atom/sst/sst_stream.c
index 7ee6aeb7e0af..b082b0922a7a 100644
--- a/sound/soc/intel/atom/sst/sst_stream.c
+++ b/sound/soc/intel/atom/sst/sst_stream.c
@@ -253,7 +253,7 @@ int sst_pause_stream(struct intel_sst_drv *sst_drv_ctx, int str_id)
 		if (retval == 0) {
 			str_info->prev = str_info->status;
 			str_info->status = STREAM_PAUSED;
-		} else if (retval == SST_ERR_INVALID_STREAM_ID) {
+		} else if (retval == -SST_ERR_INVALID_STREAM_ID) {
 			retval = -EINVAL;
 			mutex_lock(&sst_drv_ctx->sst_lock);
 			sst_clean_stream(str_info);
-- 
2.14.3

[PATCH 2/9] ASoC: Intel: sst: Remove 2 unused members from stream_info struct

[Hans de Goede]

Remove the unused ops and str_id members from the stream_info struct.

While at it also remove some kernel-doc comments for members which have
already been removed in the past.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/atom/sst/sst.h     | 7 -------
 sound/soc/intel/atom/sst/sst_pvt.c | 1 -
 2 files changed, 8 deletions(-)

diff --git a/sound/soc/intel/atom/sst/sst.h b/sound/soc/intel/atom/sst/sst.h
index e02e2b4cc08f..1f3107909044 100644
--- a/sound/soc/intel/atom/sst/sst.h
+++ b/sound/soc/intel/atom/sst/sst.h
@@ -181,21 +181,15 @@ struct sst_block {
  *
  * @status : stream current state
  * @prev : stream prev state
- * @ops : stream operation pb/cp/drm...
- * @bufs: stream buffer list
  * @lock : stream mutex for protecting state
  * @pcm_substream : PCM substream
  * @period_elapsed : PCM period elapsed callback
  * @sfreq : stream sampling freq
- * @str_type : stream type
  * @cumm_bytes : cummulative bytes decoded
- * @str_type : stream type
- * @src : stream source
  */
 struct stream_info {
 	unsigned int		status;
 	unsigned int		prev;
-	unsigned int		ops;
 	struct mutex		lock;
 
 	void			*pcm_substream;
@@ -212,7 +206,6 @@ struct stream_info {
 
 	unsigned int		num_ch;
 	unsigned int		pipe_id;
-	unsigned int		str_id;
 	unsigned int		task_id;
 };
 
diff --git a/sound/soc/intel/atom/sst/sst_pvt.c b/sound/soc/intel/atom/sst/sst_pvt.c
index b1e6b8f34a6a..d7d38b0f68db 100644
--- a/sound/soc/intel/atom/sst/sst_pvt.c
+++ b/sound/soc/intel/atom/sst/sst_pvt.c
@@ -365,7 +365,6 @@ void sst_init_stream(struct stream_info *stream,
 {
 	stream->status = STREAM_INIT;
 	stream->prev = STREAM_UN_INIT;
-	stream->ops = ops;
 }
 
 int sst_validate_strid(
-- 
2.14.3

[PATCH 3/9] ASoC: Intel: sst: Remove unnecessary sst_init_stream() function

[Hans de Goede]

sst_init_stream() has only one caller and all its function arguments are
unused. Inline it on the one call site and remove it.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/atom/sst/sst.h        | 2 --
 sound/soc/intel/atom/sst/sst_pvt.c    | 7 -------
 sound/soc/intel/atom/sst/sst_stream.c | 5 ++---
 3 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/sound/soc/intel/atom/sst/sst.h b/sound/soc/intel/atom/sst/sst.h
index 1f3107909044..164b0f674c20 100644
--- a/sound/soc/intel/atom/sst/sst.h
+++ b/sound/soc/intel/atom/sst/sst.h
@@ -494,8 +494,6 @@ int sst_prepare_and_post_msg(struct intel_sst_drv *sst,
 
 void sst_process_pending_msg(struct work_struct *work);
 int sst_assign_pvt_id(struct intel_sst_drv *sst_drv_ctx);
-void sst_init_stream(struct stream_info *stream,
-		int codec, int sst_id, int ops, u8 slot);
 int sst_validate_strid(struct intel_sst_drv *sst_drv_ctx, int str_id);
 struct stream_info *get_stream_info(struct intel_sst_drv *sst_drv_ctx,
 		int str_id);
diff --git a/sound/soc/intel/atom/sst/sst_pvt.c b/sound/soc/intel/atom/sst/sst_pvt.c
index d7d38b0f68db..af93244b4868 100644
--- a/sound/soc/intel/atom/sst/sst_pvt.c
+++ b/sound/soc/intel/atom/sst/sst_pvt.c
@@ -360,13 +360,6 @@ int sst_assign_pvt_id(struct intel_sst_drv *drv)
 	return local;
 }
 
-void sst_init_stream(struct stream_info *stream,
-		int codec, int sst_id, int ops, u8 slot)
-{
-	stream->status = STREAM_INIT;
-	stream->prev = STREAM_UN_INIT;
-}
-
 int sst_validate_strid(
 		struct intel_sst_drv *sst_drv_ctx, int str_id)
 {
diff --git a/sound/soc/intel/atom/sst/sst_stream.c b/sound/soc/intel/atom/sst/sst_stream.c
index b082b0922a7a..7cf5dd1993bc 100644
--- a/sound/soc/intel/atom/sst/sst_stream.c
+++ b/sound/soc/intel/atom/sst/sst_stream.c
@@ -83,6 +83,8 @@ int sst_alloc_stream_mrfld(struct intel_sst_drv *sst_drv_ctx, void *params)
 
 	pipe_id = str_params->device_type;
 	task_id = str_params->task;
+	sst_drv_ctx->streams[str_id].status = STREAM_INIT;
+	sst_drv_ctx->streams[str_id].prev = STREAM_UN_INIT;
 	sst_drv_ctx->streams[str_id].pipe_id = pipe_id;
 	sst_drv_ctx->streams[str_id].task_id = task_id;
 	sst_drv_ctx->streams[str_id].num_ch = num_ch;
@@ -100,9 +102,6 @@ int sst_alloc_stream_mrfld(struct intel_sst_drv *sst_drv_ctx, void *params)
 			pipe_id, task_id);
 
 	/* allocate device type context */
-	sst_init_stream(&sst_drv_ctx->streams[str_id], alloc_param.codec_type,
-			str_id, alloc_param.operation, 0);
-
 	dev_dbg(sst_drv_ctx->dev, "Alloc for str %d pipe %#x\n",
 			str_id, pipe_id);
 	ret = sst_prepare_and_post_msg(sst_drv_ctx, task_id, IPC_CMD,
-- 
2.14.3

[PATCH 4/9] ASoC: Intel: sst: Remove unused STREAM_DECODE and STREAM_RESET states

[Hans de Goede]

STREAM_DECODE is completely unused, status == STREAM_RESET was checked
for, but never set, remove both.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/atom/sst/sst.h               |  4 +---
 sound/soc/intel/atom/sst/sst_drv_interface.c | 19 -------------------
 2 files changed, 1 insertion(+), 22 deletions(-)

diff --git a/sound/soc/intel/atom/sst/sst.h b/sound/soc/intel/atom/sst/sst.h
index 164b0f674c20..f4fd442080b2 100644
--- a/sound/soc/intel/atom/sst/sst.h
+++ b/sound/soc/intel/atom/sst/sst.h
@@ -65,9 +65,7 @@ enum sst_stream_states {
 	STREAM_UN_INIT	= 0,	/* Freed/Not used stream */
 	STREAM_RUNNING	= 1,	/* Running */
 	STREAM_PAUSED	= 2,	/* Paused stream */
-	STREAM_DECODE	= 3,	/* stream is in decoding only state */
-	STREAM_INIT	= 4,	/* stream init, waiting for data */
-	STREAM_RESET	= 5,	/* force reset on recovery */
+	STREAM_INIT	= 3,	/* stream init, waiting for data */
 };
 
 enum sst_ram_type {
diff --git a/sound/soc/intel/atom/sst/sst_drv_interface.c b/sound/soc/intel/atom/sst/sst_drv_interface.c
index 71af5449be90..6a8b253c58d2 100644
--- a/sound/soc/intel/atom/sst/sst_drv_interface.c
+++ b/sound/soc/intel/atom/sst/sst_drv_interface.c
@@ -238,16 +238,7 @@ static int sst_cdev_close(struct device *dev, unsigned int str_id)
 		return -EINVAL;
 	}
 
-	if (stream->status == STREAM_RESET) {
-		dev_dbg(dev, "stream in reset state...\n");
-		stream->status = STREAM_UN_INIT;
-
-		retval = 0;
-		goto put;
-	}
-
 	retval = sst_free_stream(ctx, str_id);
-put:
 	stream->compr_cb_param = NULL;
 	stream->compr_cb = NULL;
 
@@ -256,7 +247,6 @@ static int sst_cdev_close(struct device *dev, unsigned int str_id)
 
 	dev_dbg(dev, "End\n");
 	return retval;
-
 }
 
 static int sst_cdev_ack(struct device *dev, unsigned int str_id,
@@ -486,16 +476,7 @@ static int sst_close_pcm_stream(struct device *dev, unsigned int str_id)
 		return -EINVAL;
 	}
 
-	if (stream->status == STREAM_RESET) {
-		/* silently fail here as we have cleaned the stream earlier */
-		dev_dbg(ctx->dev, "stream in reset state...\n");
-
-		retval = 0;
-		goto put;
-	}
-
 	retval = free_stream_context(ctx, str_id);
-put:
 	stream->pcm_substream = NULL;
 	stream->status = STREAM_UN_INIT;
 	stream->period_elapsed = NULL;
-- 
2.14.3

[PATCH 5/9] ASoC: Intel: sst: Add sst_realloc_stream() function

[Hans de Goede]

Move the struct snd_sst_alloc_mrfld alloc parameters from the stack
into struct stream_info and add a new sst_realloc_stream() function which
can re-alloc a stream with the same parameters as before.

This is a preparation patch for fixing suspend/resume issues with some
SST / DSP firmware versions.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/atom/sst/sst.h        |  3 ++
 sound/soc/intel/atom/sst/sst_stream.c | 88 +++++++++++++++++++++--------------
 2 files changed, 56 insertions(+), 35 deletions(-)

diff --git a/sound/soc/intel/atom/sst/sst.h b/sound/soc/intel/atom/sst/sst.h
index f4fd442080b2..a357cd615b59 100644
--- a/sound/soc/intel/atom/sst/sst.h
+++ b/sound/soc/intel/atom/sst/sst.h
@@ -180,6 +180,7 @@ struct sst_block {
  * @status : stream current state
  * @prev : stream prev state
  * @lock : stream mutex for protecting state
+ * @alloc_param : parameters used for stream (re-)allocation
  * @pcm_substream : PCM substream
  * @period_elapsed : PCM period elapsed callback
  * @sfreq : stream sampling freq
@@ -189,6 +190,7 @@ struct stream_info {
 	unsigned int		status;
 	unsigned int		prev;
 	struct mutex		lock;
+	struct snd_sst_alloc_mrfld alloc_param;
 
 	void			*pcm_substream;
 	void (*period_elapsed)(void *pcm_substream);
@@ -429,6 +431,7 @@ struct intel_sst_ops {
 	void (*post_download)(struct intel_sst_drv *sst);
 };
 
+int sst_realloc_stream(struct intel_sst_drv *sst_drv_ctx, int str_id);
 int sst_pause_stream(struct intel_sst_drv *sst_drv_ctx, int id);
 int sst_resume_stream(struct intel_sst_drv *sst_drv_ctx, int id);
 int sst_drop_stream(struct intel_sst_drv *sst_drv_ctx, int id);
diff --git a/sound/soc/intel/atom/sst/sst_stream.c b/sound/soc/intel/atom/sst/sst_stream.c
index 7cf5dd1993bc..fcedaa237505 100644
--- a/sound/soc/intel/atom/sst/sst_stream.c
+++ b/sound/soc/intel/atom/sst/sst_stream.c
@@ -35,29 +35,31 @@
 
 int sst_alloc_stream_mrfld(struct intel_sst_drv *sst_drv_ctx, void *params)
 {
-	struct snd_sst_alloc_mrfld alloc_param;
+	struct snd_pcm_params *pcm_params;
 	struct snd_sst_params *str_params;
 	struct snd_sst_tstamp fw_tstamp;
 	struct stream_info *str_info;
-	struct snd_sst_alloc_response *response;
-	unsigned int str_id, pipe_id, task_id;
-	int i, num_ch, ret = 0;
-	void *data = NULL;
+	int i, num_ch, str_id;
 
 	dev_dbg(sst_drv_ctx->dev, "Enter\n");
 
 	str_params = (struct snd_sst_params *)params;
-	memset(&alloc_param, 0, sizeof(alloc_param));
-	alloc_param.operation = str_params->ops;
-	alloc_param.codec_type = str_params->codec;
-	alloc_param.sg_count = str_params->aparams.sg_count;
-	alloc_param.ring_buf_info[0].addr =
+	str_id = str_params->stream_id;
+	str_info = get_stream_info(sst_drv_ctx, str_id);
+	if (!str_info)
+		return -EINVAL;
+
+	memset(&str_info->alloc_param, 0, sizeof(str_info->alloc_param));
+	str_info->alloc_param.operation = str_params->ops;
+	str_info->alloc_param.codec_type = str_params->codec;
+	str_info->alloc_param.sg_count = str_params->aparams.sg_count;
+	str_info->alloc_param.ring_buf_info[0].addr =
 		str_params->aparams.ring_buf_info[0].addr;
-	alloc_param.ring_buf_info[0].size =
+	str_info->alloc_param.ring_buf_info[0].size =
 		str_params->aparams.ring_buf_info[0].size;
-	alloc_param.frag_size = str_params->aparams.frag_size;
+	str_info->alloc_param.frag_size = str_params->aparams.frag_size;
 
-	memcpy(&alloc_param.codec_params, &str_params->sparams,
+	memcpy(&str_info->alloc_param.codec_params, &str_params->sparams,
 			sizeof(struct snd_sst_stream_params));
 
 	/*
@@ -67,46 +69,62 @@ int sst_alloc_stream_mrfld(struct intel_sst_drv *sst_drv_ctx, void *params)
 	 * Currently hardcoding as per FW reqm.
 	 */
 	num_ch = sst_get_num_channel(str_params);
+	pcm_params = &str_info->alloc_param.codec_params.uc.pcm_params;
 	for (i = 0; i < 8; i++) {
 		if (i < num_ch)
-			alloc_param.codec_params.uc.pcm_params.channel_map[i] = i;
+			pcm_params->channel_map[i] = i;
 		else
-			alloc_param.codec_params.uc.pcm_params.channel_map[i] = 0xFF;
+			pcm_params->channel_map[i] = 0xff;
 	}
 
-	str_id = str_params->stream_id;
-	str_info = get_stream_info(sst_drv_ctx, str_id);
-	if (str_info == NULL) {
-		dev_err(sst_drv_ctx->dev, "get stream info returned null\n");
-		return -EINVAL;
-	}
-
-	pipe_id = str_params->device_type;
-	task_id = str_params->task;
 	sst_drv_ctx->streams[str_id].status = STREAM_INIT;
 	sst_drv_ctx->streams[str_id].prev = STREAM_UN_INIT;
-	sst_drv_ctx->streams[str_id].pipe_id = pipe_id;
-	sst_drv_ctx->streams[str_id].task_id = task_id;
+	sst_drv_ctx->streams[str_id].pipe_id = str_params->device_type;
+	sst_drv_ctx->streams[str_id].task_id = str_params->task;
 	sst_drv_ctx->streams[str_id].num_ch = num_ch;
 
 	if (sst_drv_ctx->info.lpe_viewpt_rqd)
-		alloc_param.ts = sst_drv_ctx->info.mailbox_start +
+		str_info->alloc_param.ts = sst_drv_ctx->info.mailbox_start +
 			sst_drv_ctx->tstamp + (str_id * sizeof(fw_tstamp));
 	else
-		alloc_param.ts = sst_drv_ctx->mailbox_add +
+		str_info->alloc_param.ts = sst_drv_ctx->mailbox_add +
 			sst_drv_ctx->tstamp + (str_id * sizeof(fw_tstamp));
 
 	dev_dbg(sst_drv_ctx->dev, "alloc tstamp location = 0x%x\n",
-			alloc_param.ts);
+			str_info->alloc_param.ts);
 	dev_dbg(sst_drv_ctx->dev, "assigned pipe id 0x%x to task %d\n",
-			pipe_id, task_id);
+			str_info->pipe_id, str_info->task_id);
+
+	return sst_realloc_stream(sst_drv_ctx, str_id);
+}
+
+/**
+ * sst_realloc_stream - Send msg for (re-)allocating a stream using the
+ * @sst_drv_ctx  intel_sst_drv context pointer
+ * @str_id:	 stream ID
+ *
+ * Send a msg for (re-)allocating a stream using the parameters previously
+ * passed to sst_alloc_stream_mrfld() for the same stream ID.
+ * Return: 0 or negative errno value.
+ */
+int sst_realloc_stream(struct intel_sst_drv *sst_drv_ctx, int str_id)
+{
+	struct snd_sst_alloc_response *response;
+	struct stream_info *str_info;
+	void *data = NULL;
+	int ret;
+
+	str_info = get_stream_info(sst_drv_ctx, str_id);
+	if (!str_info)
+		return -EINVAL;
 
-	/* allocate device type context */
 	dev_dbg(sst_drv_ctx->dev, "Alloc for str %d pipe %#x\n",
-			str_id, pipe_id);
-	ret = sst_prepare_and_post_msg(sst_drv_ctx, task_id, IPC_CMD,
-			IPC_IA_ALLOC_STREAM_MRFLD, pipe_id, sizeof(alloc_param),
-			&alloc_param, &data, true, true, false, true);
+		str_id, str_info->pipe_id);
+
+	ret = sst_prepare_and_post_msg(sst_drv_ctx, str_info->task_id, IPC_CMD,
+			IPC_IA_ALLOC_STREAM_MRFLD, str_info->pipe_id,
+			sizeof(str_info->alloc_param), &str_info->alloc_param,
+			&data, true, true, false, true);
 
 	if (ret < 0) {
 		dev_err(sst_drv_ctx->dev, "FW alloc failed ret %d\n", ret);
-- 
2.14.3

[PATCH 6/9] ASoC: Intel: sst: Free streams on suspend, re-alloc on resume

[Hans de Goede]

The Bay Trail SST-DSP firmware version looses track of all streams over a
suspend/resume, failing any attempts to resume and/or free streams, with
a SST_ERR_INVALID_STREAM_ID error.

This commit adds support for free-ing the streams on suspend and
re-allocating them on resume, fixing suspend/resume issues on devices
using this firmware version.

This new behavior gets triggered by a new flag in sst_platform_info which
only gets set on Bay Trail platforms.

This has been tested on the following devices:
-Asus T100TA,    Bay Trail    + ALC5642 codec
-Ployer MOMO7W,  Bay Trail CR + ALC5652 codec

Tested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/x86/include/asm/platform_sst_audio.h |  1 +
 sound/soc/intel/atom/sst/sst.c            | 24 +++++++++++++++++++++++-
 sound/soc/intel/atom/sst/sst.h            |  4 ++++
 sound/soc/intel/atom/sst/sst_acpi.c       |  3 ++-
 sound/soc/intel/atom/sst/sst_stream.c     | 24 +++++++++++++++++++++++-
 5 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/platform_sst_audio.h b/arch/x86/include/asm/platform_sst_audio.h
index 5973a2f3db3d..059823bb8af7 100644
--- a/arch/x86/include/asm/platform_sst_audio.h
+++ b/arch/x86/include/asm/platform_sst_audio.h
@@ -135,6 +135,7 @@ struct sst_platform_info {
 	const struct sst_res_info *res_info;
 	const struct sst_lib_dnld_info *lib_info;
 	const char *platform;
+	bool streams_lost_on_suspend;
 };
 int add_sst_platform_device(void);
 #endif
diff --git a/sound/soc/intel/atom/sst/sst.c b/sound/soc/intel/atom/sst/sst.c
index 8afdff457579..0962bc9adc62 100644
--- a/sound/soc/intel/atom/sst/sst.c
+++ b/sound/soc/intel/atom/sst/sst.c
@@ -449,6 +449,13 @@ static int intel_sst_suspend(struct device *dev)
 			dev_err(dev, "stream %d is running, can't suspend, abort\n", i);
 			return -EBUSY;
 		}
+
+		if (ctx->pdata->streams_lost_on_suspend) {
+			stream->resume_status = stream->status;
+			stream->resume_prev = stream->prev;
+			if (stream->status != STREAM_UN_INIT)
+				sst_free_stream(ctx, i);
+		}
 	}
 	synchronize_irq(ctx->irq_num);
 	flush_workqueue(ctx->post_msg_wq);
@@ -509,8 +516,8 @@ static int intel_sst_resume(struct device *dev)
 {
 	struct intel_sst_drv *ctx = dev_get_drvdata(dev);
 	struct sst_fw_save *fw_save = ctx->fw_save;
-	int ret = 0;
 	struct sst_block *block;
+	int i, ret = 0;
 
 	if (!fw_save)
 		return 0;
@@ -550,6 +557,21 @@ static int intel_sst_resume(struct device *dev)
 		sst_set_fw_state_locked(ctx, SST_FW_RUNNING);
 	}
 
+	if (ctx->pdata->streams_lost_on_suspend) {
+		for (i = 1; i <= ctx->info.max_streams; i++) {
+			struct stream_info *stream = &ctx->streams[i];
+
+			if (stream->resume_status != STREAM_UN_INIT) {
+				dev_dbg(ctx->dev, "Re-allocing stream %d status %d prev %d\n",
+					i, stream->resume_status,
+					stream->resume_prev);
+				sst_realloc_stream(ctx, i);
+				stream->status = stream->resume_status;
+				stream->prev = stream->resume_prev;
+			}
+		}
+	}
+
 	sst_free_block(ctx, block);
 	return ret;
 }
diff --git a/sound/soc/intel/atom/sst/sst.h b/sound/soc/intel/atom/sst/sst.h
index a357cd615b59..b2a705dc9304 100644
--- a/sound/soc/intel/atom/sst/sst.h
+++ b/sound/soc/intel/atom/sst/sst.h
@@ -179,6 +179,8 @@ struct sst_block {
  *
  * @status : stream current state
  * @prev : stream prev state
+ * @resume_status : stream current state to restore on resume
+ * @resume_prev : stream prev state to restore on resume
  * @lock : stream mutex for protecting state
  * @alloc_param : parameters used for stream (re-)allocation
  * @pcm_substream : PCM substream
@@ -189,6 +191,8 @@ struct sst_block {
 struct stream_info {
 	unsigned int		status;
 	unsigned int		prev;
+	unsigned int		resume_status;
+	unsigned int		resume_prev;
 	struct mutex		lock;
 	struct snd_sst_alloc_mrfld alloc_param;
 
diff --git a/sound/soc/intel/atom/sst/sst_acpi.c b/sound/soc/intel/atom/sst/sst_acpi.c
index 6cd481bec275..c90b04cc071d 100644
--- a/sound/soc/intel/atom/sst/sst_acpi.c
+++ b/sound/soc/intel/atom/sst/sst_acpi.c
@@ -143,10 +143,11 @@ static struct sst_platform_info byt_rvp_platform_data = {
 	.lib_info = &byt_lib_dnld_info,
 	.res_info = &byt_rvp_res_info,
 	.platform = "sst-mfld-platform",
+	.streams_lost_on_suspend = true,
 };
 
 /* Cherryview (Cherrytrail and Braswell) uses same mrfld dpcm fw as Baytrail,
- * so pdata is same as Baytrail.
+ * so pdata is same as Baytrail, minus the streams_lost_on_suspend quirk.
  */
 static struct sst_platform_info chv_platform_data = {
 	.probe_data = &byt_fwparse_info,
diff --git a/sound/soc/intel/atom/sst/sst_stream.c b/sound/soc/intel/atom/sst/sst_stream.c
index fcedaa237505..107271f7dd63 100644
--- a/sound/soc/intel/atom/sst/sst_stream.c
+++ b/sound/soc/intel/atom/sst/sst_stream.c
@@ -302,7 +302,29 @@ int sst_resume_stream(struct intel_sst_drv *sst_drv_ctx, int str_id)
 		return -EINVAL;
 	if (str_info->status == STREAM_RUNNING)
 		return 0;
-	if (str_info->status == STREAM_PAUSED) {
+
+	if (str_info->resume_status == STREAM_PAUSED &&
+	    str_info->resume_prev == STREAM_RUNNING) {
+		/*
+		 * Stream was running before suspend and re-created on resume,
+		 * start it to get back to running state.
+		 */
+		dev_dbg(sst_drv_ctx->dev, "restart recreated stream after resume\n");
+		str_info->status = STREAM_RUNNING;
+		str_info->prev = STREAM_PAUSED;
+		retval = sst_start_stream(sst_drv_ctx, str_id);
+		str_info->resume_status = STREAM_UN_INIT;
+	} else if (str_info->resume_status == STREAM_PAUSED &&
+		   str_info->resume_prev == STREAM_INIT) {
+		/*
+		 * Stream was idle before suspend and re-created on resume,
+		 * keep it as is.
+		 */
+		dev_dbg(sst_drv_ctx->dev, "leaving recreated stream idle after resume\n");
+		str_info->status = STREAM_INIT;
+		str_info->prev = STREAM_PAUSED;
+		str_info->resume_status = STREAM_UN_INIT;
+	} else if (str_info->status == STREAM_PAUSED) {
 		retval = sst_prepare_and_post_msg(sst_drv_ctx, str_info->task_id,
 				IPC_CMD, IPC_IA_RESUME_STREAM_MRFLD,
 				str_info->pipe_id, 0, NULL, NULL,
-- 
2.14.3

[PATCH 7/9] ASoc: rt5651: Fix regcache sync errors on resume

[Hans de Goede]

The ALC5651 does not like multi-write accesses, avoid them. This fixes:

rt5651 i2c-10EC5651:00: Unable to sync registers 0x27-0x28. -121

Errors on resume (and all registers after the registers in the error not
being synced).

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 831b297978a4..45a73049cf64 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1722,6 +1722,7 @@ static const struct regmap_config rt5651_regmap = {
 	.num_reg_defaults = ARRAY_SIZE(rt5651_reg),
 	.ranges = rt5651_ranges,
 	.num_ranges = ARRAY_SIZE(rt5651_ranges),
+	.use_single_rw = true,
 };
 
 #if defined(CONFIG_OF)
-- 
2.14.3

[PATCH 8/9] ASoC: Intel: bytcr_rt5640: Drop unwanted ignore_suspend settings

[Hans de Goede]

The ignore_suspend settings were added in an attempt to try and fix
suspend / resume issues. But they never fully fixed these and now we've
a proper fix, so lets remove these.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5640.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index b6a1cfeec830..c506a6e129ca 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -532,9 +532,6 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime)
 			return ret;
 	}
 
-	snd_soc_dapm_ignore_suspend(&card->dapm, "Headphone");
-	snd_soc_dapm_ignore_suspend(&card->dapm, "Speaker");
-
 	if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN) {
 		/*
 		 * The firmware might enable the clock at
@@ -691,7 +688,6 @@ static struct snd_soc_dai_link byt_rt5640_dais[] = {
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF
 						| SND_SOC_DAIFMT_CBS_CFS,
 		.be_hw_params_fixup = byt_rt5640_codec_fixup,
-		.ignore_suspend = 1,
 		.nonatomic = true,
 		.dpcm_playback = 1,
 		.dpcm_capture = 1,
-- 
2.14.3

[PATCH 9/9] ASoC: Intel: bytcr_rt5651: Drop unwanted ignore_suspend settings

[Hans de Goede]

The ignore_suspend settings were added in an attempt to try and fix
suspend / resume issues. But they never fully fixed these and now we've
a proper fix, so lets remove these.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 456526a93dd5..49c538f2770a 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -335,8 +335,6 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 		dev_err(card->dev, "unable to add card controls\n");
 		return ret;
 	}
-	snd_soc_dapm_ignore_suspend(&card->dapm, "Headphone");
-	snd_soc_dapm_ignore_suspend(&card->dapm, "Speaker");
 
 	if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN) {
 		/*
@@ -487,7 +485,6 @@ static struct snd_soc_dai_link byt_rt5651_dais[] = {
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF
 						| SND_SOC_DAIFMT_CBS_CFS,
 		.be_hw_params_fixup = byt_rt5651_codec_fixup,
-		.ignore_suspend = 1,
 		.nonatomic = true,
 		.dpcm_playback = 1,
 		.dpcm_capture = 1,
-- 
2.14.3

Re: [PATCH 1/9] ASoC: Intel: sst: Fix error-code check in sst_pause_stream()

[Andy Shevchenko]

On Mon, Feb 19, 2018 at 12:01 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> The value returned by sst_prepare_and_post_msg() is a negated SST_ERR_*
> value, so we must check for -SST_ERR_INVALID_STREAM_ID. Note that
> sst_pause_resume() already has the correct check.
>

Should it have Fixes tag?

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  sound/soc/intel/atom/sst/sst_stream.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sound/soc/intel/atom/sst/sst_stream.c b/sound/soc/intel/atom/sst/sst_stream.c
> index 7ee6aeb7e0af..b082b0922a7a 100644
> --- a/sound/soc/intel/atom/sst/sst_stream.c
> +++ b/sound/soc/intel/atom/sst/sst_stream.c
> @@ -253,7 +253,7 @@ int sst_pause_stream(struct intel_sst_drv *sst_drv_ctx, int str_id)
>                 if (retval == 0) {
>                         str_info->prev = str_info->status;
>                         str_info->status = STREAM_PAUSED;
> -               } else if (retval == SST_ERR_INVALID_STREAM_ID) {
> +               } else if (retval == -SST_ERR_INVALID_STREAM_ID) {
>                         retval = -EINVAL;
>                         mutex_lock(&sst_drv_ctx->sst_lock);
>                         sst_clean_stream(str_info);
> --
> 2.14.3
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 6/9] ASoC: Intel: sst: Free streams on suspend, re-alloc on resume

[Andy Shevchenko]

On Mon, Feb 19, 2018 at 12:01 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> The Bay Trail SST-DSP firmware version looses track of all streams over a
> suspend/resume, failing any attempts to resume and/or free streams, with
> a SST_ERR_INVALID_STREAM_ID error.
>
> This commit adds support for free-ing the streams on suspend and
> re-allocating them on resume, fixing suspend/resume issues on devices
> using this firmware version.
>
> This new behavior gets triggered by a new flag in sst_platform_info which
> only gets set on Bay Trail platforms.
>
> This has been tested on the following devices:
> -Asus T100TA,    Bay Trail    + ALC5642 codec
> -Ployer MOMO7W,  Bay Trail CR + ALC5652 codec


>  /* Cherryview (Cherrytrail and Braswell) uses same mrfld dpcm fw as Baytrail,
> - * so pdata is same as Baytrail.
> + * so pdata is same as Baytrail, minus the streams_lost_on_suspend quirk.
>   */

A nit, perhaps to fix multi line comment style as well?


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/9] ASoC: Intel: sst: Fix Bay Trail suspend/resume issues

[Andy Shevchenko]

On Mon, Feb 19, 2018 at 12:01 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi All,
>
> Here is a series fixing suspend/resume errors (and broken audio after
> resume) on Bay Trail devices.
>
> Cherry Trail devices also do not like being suspend while audio is
> playing, but with different sypmtoms instead of SST_ERR_INVALID_STREAM_ID
> errors, if a stream was playing (being allocated is fine, playing is a
> problem) before suspend then after resume we get the following errors:
>
>   [ 2312.389974] intel_sst_acpi 808622A8:00: sst: Busy wait failed, cant send this msg
>   [ 2312.597976] intel_sst_acpi 808622A8:00: sst: Busy wait failed, cant send this msg
>   ...
>
> Until the stream is stopped and restarted after which things work fine
> again. I've tried enabling the quirk this commit adds for Cherry Trail
> devices too, but that does not help. If anyone has any clues about the CHT
> problem, please let me know.

Good fix!

FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/9] ASoC: Intel: sst: Fix error-code check in sst_pause_stream()

[Hans de Goede]

Hi,

On 19-02-18 14:47, Andy Shevchenko wrote:
> On Mon, Feb 19, 2018 at 12:01 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> The value returned by sst_prepare_and_post_msg() is a negated SST_ERR_*
>> value, so we must check for -SST_ERR_INVALID_STREAM_ID. Note that
>> sst_pause_resume() already has the correct check.
>>
> 
> Should it have Fixes tag?

According to git blame this goes back to the original commit introducing
the file, so I don't think that that is helpful.

Regards,

Hans



> 
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   sound/soc/intel/atom/sst/sst_stream.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/intel/atom/sst/sst_stream.c b/sound/soc/intel/atom/sst/sst_stream.c
>> index 7ee6aeb7e0af..b082b0922a7a 100644
>> --- a/sound/soc/intel/atom/sst/sst_stream.c
>> +++ b/sound/soc/intel/atom/sst/sst_stream.c
>> @@ -253,7 +253,7 @@ int sst_pause_stream(struct intel_sst_drv *sst_drv_ctx, int str_id)
>>                  if (retval == 0) {
>>                          str_info->prev = str_info->status;
>>                          str_info->status = STREAM_PAUSED;
>> -               } else if (retval == SST_ERR_INVALID_STREAM_ID) {
>> +               } else if (retval == -SST_ERR_INVALID_STREAM_ID) {
>>                          retval = -EINVAL;
>>                          mutex_lock(&sst_drv_ctx->sst_lock);
>>                          sst_clean_stream(str_info);
>> --
>> 2.14.3
>>
> 
> 
> 

Re: [PATCH 6/9] ASoC: Intel: sst: Free streams on suspend, re-alloc on resume

[Hans de Goede]

Hi,

On 19-02-18 14:51, Andy Shevchenko wrote:
> On Mon, Feb 19, 2018 at 12:01 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> The Bay Trail SST-DSP firmware version looses track of all streams over a
>> suspend/resume, failing any attempts to resume and/or free streams, with
>> a SST_ERR_INVALID_STREAM_ID error.
>>
>> This commit adds support for free-ing the streams on suspend and
>> re-allocating them on resume, fixing suspend/resume issues on devices
>> using this firmware version.
>>
>> This new behavior gets triggered by a new flag in sst_platform_info which
>> only gets set on Bay Trail platforms.
>>
>> This has been tested on the following devices:
>> -Asus T100TA,    Bay Trail    + ALC5642 codec
>> -Ployer MOMO7W,  Bay Trail CR + ALC5652 codec
> 
> 
>>   /* Cherryview (Cherrytrail and Braswell) uses same mrfld dpcm fw as Baytrail,
>> - * so pdata is same as Baytrail.
>> + * so pdata is same as Baytrail, minus the streams_lost_on_suspend quirk.
>>    */
> 
> A nit, perhaps to fix multi line comment style as well?

The is the net/* style of multi-line comments which is used in quite a few
places in the sounds/soc dir. I think both styles are accepted under sound/soc?

Regards,

Hans

Re: [PATCH 6/9] ASoC: Intel: sst: Free streams on suspend, re-alloc on resume

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 267 bytes --]

On Mon, Feb 19, 2018 at 03:02:03PM +0100, Hans de Goede wrote:

> The is the net/* style of multi-line comments which is used in quite a few
> places in the sounds/soc dir. I think both styles are accepted under sound/soc?

I really don't care, whatever people want.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 6/9] ASoC: Intel: sst: Free streams on suspend, re-alloc on resume

[Hans de Goede]

Hi,

On 19-02-18 17:29, Mark Brown wrote:
> On Mon, Feb 19, 2018 at 03:02:03PM +0100, Hans de Goede wrote:
> 
>> The is the net/* style of multi-line comments which is used in quite a few
>> places in the sounds/soc dir. I think both styles are accepted under sound/soc?
> 
> I really don't care, whatever people want.

Ok, good to know, so then there is no need to fixup the
comment style here.

Mark, is there anything specific you're waiting for before
merging this series, or are you just waiting a bit to give
people a chance to review it?

Regards,

Hans

Re: [PATCH 6/9] ASoC: Intel: sst: Free streams on suspend, re-alloc on resume

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 330 bytes --]

On Mon, Feb 19, 2018 at 05:32:16PM +0100, Hans de Goede wrote:

> Mark, is there anything specific you're waiting for before
> merging this series, or are you just waiting a bit to give
> people a chance to review it?

It was sent less than 24 hours ago, please allow a reasonable time for
review including from the Intel people.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 8/9] ASoC: Intel: bytcr_rt5640: Drop unwanted ignore_suspend settings

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 409 bytes --]

On Sun, Feb 18, 2018 at 11:01:45PM +0100, Hans de Goede wrote:
> The ignore_suspend settings were added in an attempt to try and fix
> suspend / resume issues. But they never fully fixed these and now we've
> a proper fix, so lets remove these.

Do these systems not have the ability to continue to play audio through
suspend using the DSP?  That's the sort of thing that ignore_suspend is
normally used for.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 8/9] ASoC: Intel: bytcr_rt5640: Drop unwanted ignore_suspend settings

[Hans de Goede]

Hi,

On 01-03-18 17:22, Mark Brown wrote:
> On Sun, Feb 18, 2018 at 11:01:45PM +0100, Hans de Goede wrote:
>> The ignore_suspend settings were added in an attempt to try and fix
>> suspend / resume issues. But they never fully fixed these and now we've
>> a proper fix, so lets remove these.
> 
> Do these systems not have the ability to continue to play audio through
> suspend using the DSP?  That's the sort of thing that ignore_suspend is
> normally used for.

In theory yes, in practice if we keep ignore_suspend set then we get
audio for a couple of more seconds after suspend until the buffers
are empty, as we have no code keeping them filled, which is not really
useful, since we don't support this in a meaningful way it is better
to just do a normal full suspend.

Regards,

Hans

Re: [PATCH 8/9] ASoC: Intel: bytcr_rt5640: Drop unwanted ignore_suspend settings

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1052 bytes --]

On Thu, Mar 01, 2018 at 05:26:06PM +0100, Hans de Goede wrote:
> On 01-03-18 17:22, Mark Brown wrote:
> > On Sun, Feb 18, 2018 at 11:01:45PM +0100, Hans de Goede wrote:

> > > The ignore_suspend settings were added in an attempt to try and fix
> > > suspend / resume issues. But they never fully fixed these and now we've
> > > a proper fix, so lets remove these.

> > Do these systems not have the ability to continue to play audio through
> > suspend using the DSP?  That's the sort of thing that ignore_suspend is
> > normally used for.

> In theory yes, in practice if we keep ignore_suspend set then we get
> audio for a couple of more seconds after suspend until the buffers
> are empty, as we have no code keeping them filled, which is not really
> useful, since we don't support this in a meaningful way it is better
> to just do a normal full suspend.

That sounds like what's missing is hookup of whatever the DSP uses to
wake the system when it's getting to the bottom of the buffer?  That's
the normal way this stuff is implemented anyway.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 8/9] ASoC: Intel: bytcr_rt5640: Drop unwanted ignore_suspend settings

[Hans de Goede]

Hi,

On 01-03-18 17:48, Mark Brown wrote:
> On Thu, Mar 01, 2018 at 05:26:06PM +0100, Hans de Goede wrote:
>> On 01-03-18 17:22, Mark Brown wrote:
>>> On Sun, Feb 18, 2018 at 11:01:45PM +0100, Hans de Goede wrote:
> 
>>>> The ignore_suspend settings were added in an attempt to try and fix
>>>> suspend / resume issues. But they never fully fixed these and now we've
>>>> a proper fix, so lets remove these.
> 
>>> Do these systems not have the ability to continue to play audio through
>>> suspend using the DSP?  That's the sort of thing that ignore_suspend is
>>> normally used for.
> 
>> In theory yes, in practice if we keep ignore_suspend set then we get
>> audio for a couple of more seconds after suspend until the buffers
>> are empty, as we have no code keeping them filled, which is not really
>> useful, since we don't support this in a meaningful way it is better
>> to just do a normal full suspend.
> 
> That sounds like what's missing is hookup of whatever the DSP uses to
> wake the system when it's getting to the bottom of the buffer?  That's
> the normal way this stuff is implemented anyway.

I'm afraid there is a lot more missing, at least from a standard Linux
distro pov, just waking up is not enough, we need to also wakeup
userspace to get the mp3-player (or whatever) to refill the buffer,
but preferably without waking up the GPU, turning on the screen, etc.

AFAIK support for this is currently completely missing, standard Linux
userspace currently treats suspend-2-idle as a a full suspend and any
wakeup as a full wakeup.

So AFAICT currently the ignore-suspend flag is currently not useful
and as the commit message mentions IIRC there were added to fix some
issues with suspend/resume in the past.

Regards,

Hans

Re: [PATCH 8/9] ASoC: Intel: bytcr_rt5640: Drop unwanted ignore_suspend settings

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1073 bytes --]

On Thu, Mar 01, 2018 at 05:58:35PM +0100, Hans de Goede wrote:
> On 01-03-18 17:48, Mark Brown wrote:

> > That sounds like what's missing is hookup of whatever the DSP uses to
> > wake the system when it's getting to the bottom of the buffer?  That's
> > the normal way this stuff is implemented anyway.

> I'm afraid there is a lot more missing, at least from a standard Linux
> distro pov, just waking up is not enough, we need to also wakeup
> userspace to get the mp3-player (or whatever) to refill the buffer,
> but preferably without waking up the GPU, turning on the screen, etc.

> AFAIK support for this is currently completely missing, standard Linux
> userspace currently treats suspend-2-idle as a a full suspend and any
> wakeup as a full wakeup.

Sure, but do such userspaces exist - ChromeOS or something for example?

> So AFAICT currently the ignore-suspend flag is currently not useful
> and as the commit message mentions IIRC there were added to fix some
> issues with suspend/resume in the past.

Is it possible the issue was playback during suspend?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 8/9] ASoC: Intel: bytcr_rt5640: Drop unwanted ignore_suspend settings

[Hans de Goede]

Hi,

On 01-03-18 18:17, Mark Brown wrote:
> On Thu, Mar 01, 2018 at 05:58:35PM +0100, Hans de Goede wrote:
>> On 01-03-18 17:48, Mark Brown wrote:
> 
>>> That sounds like what's missing is hookup of whatever the DSP uses to
>>> wake the system when it's getting to the bottom of the buffer?  That's
>>> the normal way this stuff is implemented anyway.
> 
>> I'm afraid there is a lot more missing, at least from a standard Linux
>> distro pov, just waking up is not enough, we need to also wakeup
>> userspace to get the mp3-player (or whatever) to refill the buffer,
>> but preferably without waking up the GPU, turning on the screen, etc.
> 
>> AFAIK support for this is currently completely missing, standard Linux
>> userspace currently treats suspend-2-idle as a a full suspend and any
>> wakeup as a full wakeup.
> 
> Sure, but do such userspaces exist - ChromeOS or something for example?

ChromeOS has a more or less standard userspace I believe.

>> So AFAICT currently the ignore-suspend flag is currently not useful
>> and as the commit message mentions IIRC there were added to fix some
>> issues with suspend/resume in the past.
> 
> Is it possible the issue was playback during suspend?

There was an issue with suspend being blocked (so the machine not suspending)
if audio was playing because of one the suspend callbacks returning
an error. I believe the ignore_suspend settings where added as a workaround
for that and that is no longer a problem.

Pierre-Louis, do you have any input on this / on this patch?

Regards,

Hans

Re: [PATCH 8/9] ASoC: Intel: bytcr_rt5640: Drop unwanted ignore_suspend settings

[Hans de Goede]

Hi,

On 04-03-18 15:11, Hans de Goede wrote:
> Hi,
> 
> On 01-03-18 18:17, Mark Brown wrote:
>> On Thu, Mar 01, 2018 at 05:58:35PM +0100, Hans de Goede wrote:
>>> On 01-03-18 17:48, Mark Brown wrote:
>>
>>>> That sounds like what's missing is hookup of whatever the DSP uses to
>>>> wake the system when it's getting to the bottom of the buffer?  That's
>>>> the normal way this stuff is implemented anyway.
>>
>>> I'm afraid there is a lot more missing, at least from a standard Linux
>>> distro pov, just waking up is not enough, we need to also wakeup
>>> userspace to get the mp3-player (or whatever) to refill the buffer,
>>> but preferably without waking up the GPU, turning on the screen, etc.
>>
>>> AFAIK support for this is currently completely missing, standard Linux
>>> userspace currently treats suspend-2-idle as a a full suspend and any
>>> wakeup as a full wakeup.
>>
>> Sure, but do such userspaces exist - ChromeOS or something for example?
> 
> ChromeOS has a more or less standard userspace I believe.
> 
>>> So AFAICT currently the ignore-suspend flag is currently not useful
>>> and as the commit message mentions IIRC there were added to fix some
>>> issues with suspend/resume in the past.
>>
>> Is it possible the issue was playback during suspend?
> 
> There was an issue with suspend being blocked (so the machine not suspending)
> if audio was playing because of one the suspend callbacks returning
> an error. I believe the ignore_suspend settings where added as a workaround
> for that and that is no longer a problem.

To be precise (I just got a bugzilla email because someone added a
comment) I believe the ignore_suspend settings were added in an
attempt to fix / workaround this bug:

https://bugzilla.kernel.org/show_bug.cgi?id=111481

And not to allow audio to keep playing while suspended.

Regards,

Hans

Re: [PATCH 8/9] ASoC: Intel: bytcr_rt5640: Drop unwanted ignore_suspend settings

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 399 bytes --]

On Sun, Mar 04, 2018 at 03:11:09PM +0100, Hans de Goede wrote:
> On 01-03-18 18:17, Mark Brown wrote:

> > Sure, but do such userspaces exist - ChromeOS or something for example?

> ChromeOS has a more or less standard userspace I believe.

No, they've got their own sound server for historical reasons.  There's
Android too of course who definitely do have facilities for compressed
audio offload.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]