ASoC: tas2783: prepare the port again on the resume path

Andrey Golovko <andrey.golovko@gmail.com> says:

v1 was a single patch that powered the SDCA Function up in the port
PRE_PREP callback:

  https://lore.kernel.org/all/20260813001500.9218-1-andrey.golovko@gmail.com/

Mark asked whether that does the right thing when userspace restarts the
stream with SNDRV_PCM_IOCTL_RESUME instead of preparing it, and pointed
out that a stream can also be suspended while prepared; Vijendar agreed.

The answer to the first question is no, and it is reproducible on this
machine, so v2 adds a patch to fix that path.  The second case turns out
to be a wider gap that neither patch closes; it is described at the end.

Measured on an ASUS ProArt PX13 HN7306EAC (AMD ACP7.0, two TAS2783 on
SDW1).  A test program plays a 440/660 Hz tone straight to the hw
device, the machine goes through an s2idle cycle, and on -ESTRPIPE the
program calls snd_pcm_resume() only - never snd_pcm_prepare().  The tone
is measured through the internal microphone against the noise floor of
the same run, and the peripheral registers are read out of band.

With v1 alone (broonie/sound for-next + v1):

                       before suspend     after snd_pcm_resume()
  DP1 PrepareCtrl      0x1 / 0x2          0x0 / 0x0
  PDE23 req / act      0x0 / 0x0          0x3 / 0x3
  tone 440 / 660 Hz    +65.5 / +75.2 dB   -1.7 / +1.8 dB

snd_pcm_resume() returned 0 and the PCM stayed RUNNING, so nothing in
the stack reported a failure - the speakers were simply silent.  The
ports were never prepared again, so the PRE_PREP callback v1 relies on
was never reached.

With this series:

                       before suspend     after snd_pcm_resume()
  DP1 PrepareCtrl      0x1 / 0x2          0x1 / 0x2
  PDE23 req / act      0x0 / 0x0          0x0 / 0x0
  tone 440 / 660 Hz    +73.7 / +88.3 dB   +73.7 / +88.5 dB

ChannelEn also moved to the other bank across the cycle, as it should
when the ports are prepared afresh.

The ordinary path is unaffected.  The same test recovering with
snd_pcm_prepare() instead of snd_pcm_resume(), on the same build, gives
+79.8 / +86.9 dB after the cycle with the ports prepared and the bank
switched back.

This also explains the negative test Robin Everaars reported for v1 on
the same board earlier today: playback opened without an error, both
amplifiers stayed attached, and the speakers were silent - which is what
this path looks like from userspace.

The case this series does not cover
===================================

A stream that is suspended while merely PREPARED never sees a trigger at
all: snd_pcm_do_suspend() returns early when the stream is not running,
and snd_pcm_do_resume() returns early unless the suspended state was
RUNNING or DRAINING.  Userspace then calls snd_pcm_start(), the ports
are enabled, and the peripheral has lost its port state without anything
in the path noticing.  Patch 1 cannot help there - the SoundWire stream
is still SDW_STREAM_PREPARED, so sdw_prepare_stream() is a no-op by
design - and neither can the codec, which has no way to tell the core
that its ports went away.

The bus does know: the peripheral goes UNATTACHED and comes back
uninitialized.  Making that invalidate the prepared state of the streams
it takes part in looks like the right place to me, but it is a core
change and I have not written it.  Say if it belongs in this series.

Link: https://patch.msgid.link/20260813194000.10412-1-andrey.golovko@gmail.com

sound/soc/codecs/tas2783-sdw.c

@@ -1310,6 +1310,7 @@ static int tas_port_prep(struct sdw_slave *slave, struct sdw_prepare_ch *prep_ch
 			 enum sdw_port_prep_ops pre_ops)
 {
 	struct device *dev = &slave->dev;
+	struct tas2783_prv *tas_dev = dev_get_drvdata(dev);
 	struct sdw_dpn_prop *dpn_prop;
 	u32 addr;
 	int ret;
@@ -1321,6 +1322,25 @@ static int tas_port_prep(struct sdw_slave *slave, struct sdw_prepare_ch *prep_ch
 	addr = SDW_DPN_PREPARECTRL(prep_ch->num);
 	switch (pre_ops) {
 	case SDW_OPS_PORT_PRE_PREP:
+		/*
+		 * The Function has to be powered before the port can complete
+		 * channel preparation.  hw_params() does that when a stream is
+		 * set up, but a stream that is only re-prepared - as it is
+		 * after the peripheral lost power in S0i3 - does not go
+		 * through hw_params() again, and the peripheral is back at its
+		 * PS3 reset default.  Power it up here, where it is needed.
+		 */
+		scoped_guard(mutex, &tas_dev->pde_lock)
+			ret = regmap_write(tas_dev->regmap,
+					   SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_PDE23,
+							TAS2783_SDCA_CTL_REQ_POW_STATE, 0),
+					   TAS2783_SDCA_POW_STATE_ON);
+		if (ret) {
+			dev_err(dev, "power up failed for port %d, err=%d\n",
+				prep_ch->num, ret);
+			return ret;
+		}
+
 		ret = sdw_write_no_pm(slave, addr, prep_ch->ch_mask);
 		if (ret)
 			dev_err(dev, "prep failed for port %d, err=%d\n",

sound/soc/sdw_utils/soc_sdw_utils.c

@@ -1510,9 +1510,24 @@ int asoc_sdw_trigger(struct snd_pcm_substream *substream, int cmd)
 	}
 
 	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_RESUME:
+		/*
+		 * The peripherals lose their port configuration when the
+		 * controller is power-gated during system suspend, and an
+		 * application that restarts the stream with
+		 * SNDRV_PCM_IOCTL_RESUME - which platforms advertising
+		 * SNDRV_PCM_INFO_RESUME allow - never goes through
+		 * .prepare() again.  Prepare the stream here so that the
+		 * ports are reprogrammed before they are enabled;
+		 * sdw_prepare_stream() reapplies the parameters without
+		 * recomputing them when the stream is disabled.
+		 */
+		ret = sdw_prepare_stream(sdw_stream);
+		if (ret)
+			break;
+		fallthrough;
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-	case SNDRV_PCM_TRIGGER_RESUME:
 		ret = sdw_enable_stream(sdw_stream);
 		break;