[PATCH v2 0/2] ASoC: tas2783: prepare the port again on the resume path

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

  https://lore.kernel.org/all/[email protected]/

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.

Changes in v2:
  - new patch 1: prepare the stream on SNDRV_PCM_TRIGGER_RESUME, so that
    the ports are reprogrammed on the IOCTL_RESUME path too (Mark Brown,
    Vijendar Mukunda)
  - patch 2: unchanged except for a comment that no longer implies
    userspace prepare is the only way in

Andrey Golovko (2):
  ASoC: sdw_utils: prepare the stream again when resuming
  ASoC: tas2783-sdw: power the Function up before preparing the port

 sound/soc/codecs/tas2783-sdw.c      | 20 ++++++++++++++++++++
 sound/soc/sdw_utils/soc_sdw_utils.c | 17 ++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

--
2.53.0