Re: ASoC: tas2783-sdw: port prepare never completes after S0i3, no audio and no error (AMD ACP7.0, ASUS ProArt PX13)

[Andrey Golovko]

I have the root cause, and it is neither the host DMA nor the data port
itself: after the peripheral loses power in S0i3 its SDCA Power Domain
Entity PDE23 comes back at PS3, and a Data Port cannot complete channel
preparation while the Function is powered down. The only place the driver
powers PDE23 on is tas_sdw_hw_params(), which the resume path never calls.

Below is the evidence, and at the end a question about where the fix
belongs, which I do not think I should answer alone.

Test setup: ASUS ProArt PX13 HN7306EAC, AMD ACP7.0, two TAS2783 at unique
0x8/0xB plus an rt721-sdca on link 1. Kernel: broonie/sound for-next
(7.2.0-rc6 base) with b627da430357, Peter Ujfalusi's two tas2783
reg_defaults fixes, the regcache sort fixes, Antoine's stereo patch and
three unrelated display fixes. One s2idle cycle with 99.98 s of S0i3
residency out of 105 s of sleep, i.e. the ACP island really was
power-gated.

After resume, with no workaround service running: all three peripherals
Attached, both amps re-download firmware, MSI is re-disabled by
5893013efabb, suspend_stats clean, zero errors of any kind in the log --
and no audio, until the PCM is fully torn down and re-created.


The host side is not at fault, and I was wrong in July
=====================================================

I dumped the ACP and SoundWire manager registers in the silent state and
again after a card-profile cycle had restored audio (78 registers, mmap of
the PCI BAR). The playback stream configuration is identical in both:

  ACP_P1_AUDIO1_TX_RINGBUFADDR    0x048E0000   both
  ACP_P1_AUDIO1_TX_RINGBUFSIZE    0x00008000   both
  ACP_P1_AUDIO1_TX_FIFOADDR/SIZE  0x700/0x100  both
  ACP_P1_AUDIO1_TX_DMA_SIZE       0x00000040   both
  ACP_P1_AUDIO1_TX_INTR_WATERMARK 0x00001000   both
  ACP70_SW1_AUDIO1_TX_EN          0x00000001   both

The only meaningful difference is the linear position counter, and it
shows the DMA is running in the silent state too: 0x017F9440 = 25.1 MB,
which at 48 kHz/2ch/S16 is ~131 s, matching the time since resume.

This retracts what I suggested in July, that ACP loses its SoundWire DMA
ring-buffer configuration across S0i3 and would need it reprogrammed in
.prepare. That hypothesis is wrong: the registers are already correct
while there is no sound.


The peripheral reports the failure correctly
============================================

Slave-side registers, silent versus working, taken during playback:

  DP1 PortCtrl (0x102)      0x20   both
  DP1 BlockCtrl1 (0x103)    0x0f   both
  bank SampleCtrl/Offset/HCtrl (0x122..0x126) identical

  DPn_PrepareStatus (0x104)  amp 0x8: 0x1 silent, 0x0 working
                             amp 0xB: 0x2 silent, 0x0 working

Niranjan, this is the point you doubted on 28 July: the device does update
DPn_PrepareStatus, and it does so per channel -- each amp reports exactly
the channel it owns as not prepared, 0x1 for the one on channel 0 and 0x2
for the one on channel 1 (single-channel masks, per Antoine's patch). The
transport parameters are intact. Only the prepare state is lost.


Minimal reproduction, no ALSA involved
======================================

With a debug module that talks to the peripherals through
sdw_write_no_pm()/sdw_read_no_pm() only, in the silent state after resume:

  1. PrepareCtrl (0x105) already holds the channel mask, so writing the
     same value again changes nothing: PrepareStatus stays 0x1/0x2.

  2. PrepareCtrl <= 0 clears PrepareStatus to 0x0 immediately. De-prepare
     works, and the device is answering us.

  3. PrepareCtrl <= mask sets PrepareStatus to the mask again, and it
     never clears. I polled for 50 ms, the same way
     sdw_prep_deprep_slave_port() does.

  4. PDE23 Requested and Actual Power State both read 0x3, i.e. PS3.

  5. Write PDE23 Requested = PS0. Actual reads 0x0 immediately. Repeat
     step 3 and PrepareStatus clears within 1 ms. Audio is back, without
     touching the PCM, without hw_params, without re-enumeration.

So the port is not broken and the state machine is not stuck: it is
waiting for power that nobody restores.


Why nothing is logged
=====================

tas_sdw_hw_params() writes PDE23 Requested = PS0, with a retry loop whose
comment already states the dependency:

  /*
   * Sometimes, there is error returned during power on.
   * So added retry logic to ensure power on so that
   * port prepare succeeds
   */

and tas_sdw_pcm_hw_free() writes PS3 on the way out. After S0i3 the
peripheral is back at its register defaults, where PDE23 is PS3. Userspace
resumes the surviving PCM with TRIGGER_RESUME rather than tearing it down,
so hw_params never runs, so PDE23 is never powered up again.

The silence is total because tas2783 sets simple_ch_prep_sm, and
sdw_prep_deprep_slave_port() then skips both the PrepareCtrl write and the
NOT_PREPARED poll. The driver compensates with tas_port_prep(), whose
comment says the same thing -- "the port fails to enter the prepared state
resulting in no audio output" -- but that callback only runs during a real
prepare. Nobody ever asks the peripheral whether it is ready, so a port
that never prepares looks exactly like a healthy one.

Pierre-Louis, this also answers your question from 27 July about whether
playback is ongoing during suspend: it does not matter. What matters is
that a PCM stays open across the cycle, which PipeWire does by default on
an idle sink.


Where should this be fixed?
===========================

Three candidates, and I would rather hear TI and the maintainers than pick
one myself:

 (a) ACP stops advertising SNDRV_PCM_INFO_RESUME on the SoundWire DMA
     PCMs. That flag promises a resume with no stream re-setup, which
     SoundWire cannot honour when the peripheral loses power; Intel's
     SoundWire DMA does not set it. Userspace would then have to do a full
     hw_free/hw_params recovery, which restores PDE23 as a side effect. I
     have this patch and will report whether it is sufficient on its own
     on a tree that carries everything above.

 (b) tas2783 restores the SDCA power state itself, in
     tas2783_sdca_dev_resume() or on the uninitialised re-attach path,
     when a stream is active. This makes the codec track stream state that
     ASoC already owns, which I do not much like.

 (c) The SoundWire core stops letting a stream continue silently across a
     peripheral that went UNATTACHED and came back: mark the runtime as
     needing re-prepare, and fail loudly if it is not.

My own preference is (a), because the promise in the PCM flag is the thing
that is actually untrue. But (a) fixes it by making userspace do the right
thing, which is not the same as the kernel keeping its own state
consistent, so I may well be missing the intended design.

One more question for TI while we are here: given that tas2783 needs an
explicit PrepareCtrl write anyway, and has tas_port_prep() for exactly
that reason, is simple_ch_prep_sm the right property to declare? Dropping
it would put the core back in charge of the write and, more importantly,
of the NOT_PREPARED poll -- which would have turned this silent failure
into a "Chn prep failed for port 1" error line months ago.

Happy to test patches, and to run the register-level probe again on any
variant that is useful.

Thanks,
Andrey