[PATCH 1/2] ASoC: Intel: catpt: Wake up device before configuring SSP port

[PATCH 1/2] ASoC: Intel: catpt: Wake up device before configuring SSP port

[Cezary Rojewski]

catpt_dai_pcm_new() invoked during new PCM runtime creation configures
SSP by sending IPC to DSP firmware. For that to succeed device needs to
be up and running. While components default probing behavior -
snd_soc_catpt causing machine board module to load just after it - needs
no changes, machine board's module may be unloaded and re-loaded at a
different time e.g.: when catpt is already asleep.

Wake device explicitly in catpt_dai_pcm_new() to ensure communication is
established before sending any IPCs, enabling those advanced scenarios
in the process.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---

In short, this allows for following scenario (which user usually do not
touch):

1. modprobe snd_soc_catpt // core driver
2. modprobe snd_soc_sst_broadwell // machine board driver
3. wait for snd_soc_catpt to suspend (~2s)
4. rmmod snd_soc_sst_broadwell
5. modprobe snd_soc_sst_broadwell

Without the 3. everything works just fine but let's be more flexible
here.
Right now, -110 (timeout) will occur as device will simply be asleep. To
allow for this advanced scenario, pm_runtime manipulation is required.
memcmp check is to prevent unnecessary operations from occurring.

 sound/soc/intel/catpt/pcm.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/sound/soc/intel/catpt/pcm.c b/sound/soc/intel/catpt/pcm.c
index f78018c857b8..ba653ebea7d1 100644
--- a/sound/soc/intel/catpt/pcm.c
+++ b/sound/soc/intel/catpt/pcm.c
@@ -667,7 +667,17 @@ static int catpt_dai_pcm_new(struct snd_soc_pcm_runtime *rtm,
 		break;
 	}
 
+	/* see if this is a new configuration */
+	if (!memcmp(&cdev->devfmt[devfmt.iface], &devfmt, sizeof(devfmt)))
+		return 0;
+
+	pm_runtime_get_sync(cdev->dev);
+
 	ret = catpt_ipc_set_device_format(cdev, &devfmt);
+
+	pm_runtime_mark_last_busy(cdev->dev);
+	pm_runtime_put_autosuspend(cdev->dev);
+
 	if (ret)
 		return CATPT_IPC_ERROR(ret);
 
-- 
2.17.1

[PATCH 2/2] ASoC: Intel: catpt: Relax clock selection conditions

[Cezary Rojewski]

Stress tests show that DSP may occasionally be late with signaling WAIT
state when all pins are made use of simultaneously plus start/stop
(pause) gets involved. While this isn't tied to standard audio scenarios
where only System Pin (playback and capture) is involved, ensure user is
not hindered when playing with more advanced scenarios.

From DSP perspective, clock acts as a resource: low clock equals less
resources, high clock more resources. Relax clock selection procedure so
only low -> high switch is allowed when awaiting WAIT signal times out.
Once active stream count decreases, DSP will have more time internally to
adjust thus low clock selection becomes possible again.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---

TLDR:
While issue is connected to DSP hw/firmware, software may address this
by relaxing the conditions so audio remains stable when stress scenarios
are ongoing for several iterations. It is rare for user to start
system pb/offload0/offload1/system cp/loopback all at once and play with
them but let's account for that too.

 sound/soc/intel/catpt/dsp.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/catpt/dsp.c b/sound/soc/intel/catpt/dsp.c
index 7d2968571951..9e807b941732 100644
--- a/sound/soc/intel/catpt/dsp.c
+++ b/sound/soc/intel/catpt/dsp.c
@@ -267,9 +267,12 @@ static int catpt_dsp_select_lpclock(struct catpt_dev *cdev, bool lp, bool waiti)
 					    reg, (reg & CATPT_ISD_DCPWM),
 					    500, 10000);
 		if (ret) {
-			dev_err(cdev->dev, "await WAITI timeout\n");
-			mutex_unlock(&cdev->clk_mutex);
-			return ret;
+			dev_warn(cdev->dev, "await WAITI timeout\n");
+			/* no signal - only high clock selection allowed */
+			if (lp) {
+				mutex_unlock(&cdev->clk_mutex);
+				return 0;
+			}
 		}
 	}
 
-- 
2.17.1

Re: [PATCH 1/2] ASoC: Intel: catpt: Wake up device before configuring SSP port

[Mark Brown]

On Mon, 12 Oct 2020 12:32:20 +0200, Cezary Rojewski wrote:
> catpt_dai_pcm_new() invoked during new PCM runtime creation configures
> SSP by sending IPC to DSP firmware. For that to succeed device needs to
> be up and running. While components default probing behavior -
> snd_soc_catpt causing machine board module to load just after it - needs
> no changes, machine board's module may be unloaded and re-loaded at a
> different time e.g.: when catpt is already asleep.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/2] ASoC: Intel: catpt: Wake up device before configuring SSP port
      commit: 1d159edf19542793851a04202e5b0dd548a9415c
[2/2] ASoC: Intel: catpt: Relax clock selection conditions
      commit: 3d53c6df4299134525ad9e197f480e89bc8b06af

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark