Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MTL - Soundwire - clock stop prep/de-prep failed slave:15 #4619

Closed
udaymb opened this issue Oct 6, 2023 · 16 comments
Closed

MTL - Soundwire - clock stop prep/de-prep failed slave:15 #4619

udaymb opened this issue Oct 6, 2023 · 16 comments

Comments

@udaymb
Copy link

udaymb commented Oct 6, 2023

Below issue is observed
Codec - cs42l42 headset and max98363 speaker

In the Chrome MTL system - headset is connected, In UI, during youtube playback is on speaker, if switched to headset, below log is seen in dmesg

soundwire sdw-master-0: clock stop prep/de-prep failed slave:15
soundwire sdw-master-0: clock stop deprepare wait failed:-110

Above message has no effect on audio. Audio is working fine and heard on headset.

Logs - dmesg-clock-prep-deprep-15-110.log

@plbossart
Copy link
Member

-110 is ETIMEDOUT
slave:15 is a broadcast command to all peripherals.

I have no idea what setup you are using @udaymb, please add this file
sof-dyndbg.conf.txt as /etc/modprobe.d/sof-dyndbg.conf and re-attach the logs.

Also adding @rfvirgil and @charleskeepax for comments, IIRC there's something specific about clock stop with this CS42L42 device?

@rfvirgil
Copy link

rfvirgil commented Oct 6, 2023 via email

@plbossart
Copy link
Member

@rfvirgil my guess based on the limited logs is that the broadcast read fails

static int sdw_bus_wait_for_clk_prep_deprep(struct sdw_bus *bus, u16 dev_num)
{
	int retry = bus->clk_stop_timeout;
	int val;

	do {
		val = sdw_bread_no_pm(bus, dev_num, SDW_SCP_STAT);
		if (val < 0) {
			if (val != -ENODATA)
				dev_err(bus->dev, "SDW_SCP_STAT bread failed:%d\n", val);
			return val;
		}
		val &= SDW_SCP_STAT_CLK_STP_NF;
		if (!val) {
			dev_dbg(bus->dev, "clock stop prep/de-prep done slave:%d\n",
				dev_num);
			return 0;
		}

		usleep_range(1000, 1500);
		retry--;
	} while (retry);

	dev_err(bus->dev, "clock stop prep/de-prep failed slave:%d\n",
		dev_num);

	return -ETIMEDOUT;
}

We certainly need more logs but my money is on the "NotFinished" bit remaining set. If there was really a command issue we would not see the last dev_err() log.

@rfvirgil
Copy link

rfvirgil commented Oct 6, 2023 via email

@plbossart
Copy link
Member

That's not the port prep that's the issue here, it's the clock stop prep/deprep. Similar logic and wording but different steps. Ports are prepared/deprepared due to streaming actions, clock stop prep/deprep happens in suspend-resume steps.

@rfvirgil
Copy link

rfvirgil commented Oct 6, 2023 via email

@udaymb
Copy link
Author

udaymb commented Oct 9, 2023

-110 is ETIMEDOUT slave:15 is a broadcast command to all peripherals.

I have no idea what setup you are using @udaymb, please add this file sof-dyndbg.conf.txt as /etc/modprobe.d/sof-dyndbg.conf and re-attach the logs.

Also adding @rfvirgil and @charleskeepax for comments, IIRC there's something specific about clock stop with this CS42L42 device?

@plbossart - As suggested updated sof-dyndbg.conf.txt as /etc/modprobe.d/sof-dyndbg.conf

and updated below soundwire codec related in the same file
options snd_soc_cs42l42 dyndbg=+pmf
options snd_soc_cs42l42_sdw dyndbg=+pmf
options snd_soc_max98363 dyndbg=+pmf

Logs - dmesg-clock-prep-deprep-15-110-dyndbg.log

@rfvirgil
Copy link

rfvirgil commented Oct 9, 2023

@plbossart Does MTL issue bus-reset after clock stop?
In this log it looks like it it has not. There is a log print in the cs42l42 driver if unattach_request is set:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/sound/soc/codecs/cs42l42-sdw.c?h=linux-6.5.y#n440

But that isn't in the log where it is exiting clock stop (starting at 216.804360). If unattach_request is correctly 0, the CS42L42 has not been reset, so it will still be in clock stop. Looks like something has gone wrong with sending Clock_Stop_Prepare=0. But there aren't any "trf on Slave %d failed" messages so all the writes were acked.

@plbossart
Copy link
Member

I don't understand the sequence, there's just not enough information in the broken section

[  166.361093] snd_sof:sof_pcm_close: sof-audio-pci-intel-mtl 0000:00:1f.3: pcm: close stream 2 dir 0
[  166.853116] soundwire sdw-master-0: clock stop prep/de-prep failed slave:15
[  166.853132] soundwire sdw-master-0: clock stop deprepare wait failed:-110
[  166.854684] snd_soc_cs42l42_sdw:cs42l42_sdw_runtime_resume: cs42l42-sdw sdw:0:01fa:4242:00: Runtime resume
[  166.854707] snd_sof:sof_pcm_open: sof-audio-pci-intel-mtl 0000:00:1f.3: pcm: open stream 0 dir 0

@plbossart
Copy link
Member

@udaymb please try with this patch and re-attach the logs

0001-soundwire-bus-add-more-meaningful-logs-and-traces.patch.txt

@udaymb
Copy link
Author

udaymb commented Oct 9, 2023

@plbossart
Copy link
Member

Thanks @udaymb, that's what I suspected:

[  137.171017] snd_soc_cs42l42_sdw:cs42l42_sdw_runtime_suspend: cs42l42-sdw sdw:0:01fa:4242:00: Runtime suspend
[  137.171405] soundwire_bus:sdw_slave_clk_stop_prepare: cs42l42-sdw sdw:0:01fa:4242:00: sdw_slave_clk_stop_prepare: prepare done
[  137.171567] soundwire_bus:sdw_bus_wait_for_clk_prep_deprep: soundwire sdw-master-0: clock stop prepare done slave:15
[  144.661221] entering low_mem (avail RAM = 51012 kB, avail swap 0 kB, avail file 51012 kB, anon mem: 7268588 kB)
[  144.765862] soundwire_bus:sdw_slave_clk_stop_prepare: cs42l42-sdw sdw:0:01fa:4242:00: sdw_slave_clk_stop_prepare: deprepare done
[  144.766187] soundwire_bus:sdw_bus_wait_for_clk_prep_deprep: soundwire sdw-master-0: clock stop deprepare not complete slave:15, retry 300, STP_NF 0x1
...
[  145.291382] soundwire_bus:sdw_bus_wait_for_clk_prep_deprep: soundwire sdw-master-0: clock stop deprepare not complete slave:15, retry 1, STP_NF 0x1
[  145.292915] soundwire sdw-master-0: clock stop deprepare failed slave:15
[  145.292931] soundwire sdw-master-0: clock stop deprepare wait failed:-110

Somehow the CS42L42 does not clear the NotFinished bit. This doesn't seem correct, but it doesn't really matter in the end, the error is squelched into a dev_warn and the resume proceeds anyways.

I don't really know what to make of this. This happens only on the exit from clock-stop, but the NotFinished logic is the same for the entry in clock-stop. The same logic has been used on all SoundWire devices.

@rfvirgil any ideas?

plbossart added a commit to plbossart/sound that referenced this issue Oct 9, 2023
The same logic is used for clock stop prepare and deprepare, and
having different logs for the two steps helps identify problems.

In addition, when the "NotFinished" bit remains set, the error
handling is not quite right:

a) for the clock stop prepare, the error is handled at the caller
level, and the error is ignored: there's no good reason to prevent the
pm_runtime suspend from happening. Throwing an error that is later
ignored is confusing.

b) for the clock stop deprepare, the error is ignored in bus.c and a
dev_warn() log shown. Throwing an error is also alarming users for no
good reason.

For both cases, demoting the error to dev_dbg() makes more sense.

Link: thesofproject#4619
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
@rfvirgil
Copy link

@plbossart Do you have MTL + cs42l42 SoundWire in your Github automated test system?

@plbossart
Copy link
Member

@plbossart Do you have MTL + cs42l42 SoundWire in your Github automated test system?

No we do not. We only have basic RVPs with RT711-SDCA. We had a plan to introduce additional boards but it's also from Realtek.

bardliao pushed a commit that referenced this issue Oct 11, 2023
The same logic is used for clock stop prepare and deprepare, and
having different logs for the two steps helps identify problems.

In addition, when the "NotFinished" bit remains set, the error
handling is not quite right:

a) for the clock stop prepare, the error is handled at the caller
level, and the error is ignored: there's no good reason to prevent the
pm_runtime suspend from happening. Throwing an error that is later
ignored is confusing.

b) for the clock stop deprepare, the error is ignored in bus.c and a
dev_warn() log shown. Throwing an error is also alarming users for no
good reason.

For both cases, demoting the error to dev_dbg() makes more sense.

Link: #4619
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
plbossart added a commit that referenced this issue Oct 12, 2023
The same logic is used for clock stop prepare and deprepare, and
having different logs for the two steps helps identify problems.

In addition, when the "NotFinished" bit remains set, the error
handling is not quite right:

a) for the clock stop prepare, the error is handled at the caller
level, and the error is ignored: there's no good reason to prevent the
pm_runtime suspend from happening. Throwing an error that is later
ignored is confusing.

b) for the clock stop deprepare, the error is ignored in bus.c and a
dev_warn() log shown. Throwing an error is also alarming users for no
good reason.

For both cases, demoting the error to dev_dbg() makes more sense.

Link: #4619
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
akiyks pushed a commit to akiyks/linux that referenced this issue Oct 17, 2023
The same logic is used for clock stop prepare and deprepare, and
having different logs for the two steps helps identify problems.

In addition, when the "NotFinished" bit remains set, the error
handling is not quite right:

a) for the clock stop prepare, the error is handled at the caller
level, and the error is ignored: there's no good reason to prevent the
pm_runtime suspend from happening. Throwing an error that is later
ignored is confusing.

b) for the clock stop deprepare, the error is ignored in bus.c and a
dev_warn() log shown. Throwing an error is also alarming users for no
good reason.

For both cases, demoting the error to dev_dbg() makes more sense.

Link: thesofproject#4619
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20231013010812.114216-1-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
@plbossart
Copy link
Member

@udaymb can we close this issue? no input or follow-up since October.

@plbossart
Copy link
Member

no reply, closing

fadlyas07 pushed a commit to greenforce-project/chromeos-kernel-mirror that referenced this issue Aug 24, 2024
…re/deprepare

The same logic is used for clock stop prepare and deprepare, and
having different logs for the two steps helps identify problems.

In addition, when the "NotFinished" bit remains set, the error
handling is not quite right:

a) for the clock stop prepare, the error is handled at the caller
level, and the error is ignored: there's no good reason to prevent the
pm_runtime suspend from happening. Throwing an error that is later
ignored is confusing.

b) for the clock stop deprepare, the error is ignored in bus.c and a
dev_warn() log shown. Throwing an error is also alarming users for no
good reason.

For both cases, demoting the error to dev_dbg() makes more sense.

Link: thesofproject/linux#4619
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20231013010812.114216-1-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
(cherry picked from commit ff435da4cc8b6137be9f0b6b4a9af095590d4b8c
 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master)

BUG=b:326869955
TEST=Test Audio use cases.

Change-Id: Id2ef4f23517ad869e4dc7ffa0a645fe060f8f018
Signed-off-by: Debi sahoo <debix.prasad.sahoo@intel.com>
hubot pushed a commit to aosp-mirror/kernel_common that referenced this issue Oct 1, 2024
…re/deprepare

The same logic is used for clock stop prepare and deprepare, and
having different logs for the two steps helps identify problems.

In addition, when the "NotFinished" bit remains set, the error
handling is not quite right:

a) for the clock stop prepare, the error is handled at the caller
level, and the error is ignored: there's no good reason to prevent the
pm_runtime suspend from happening. Throwing an error that is later
ignored is confusing.

b) for the clock stop deprepare, the error is ignored in bus.c and a
dev_warn() log shown. Throwing an error is also alarming users for no
good reason.

For both cases, demoting the error to dev_dbg() makes more sense.

Link: thesofproject/linux#4619
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20231013010812.114216-1-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
(cherry picked from commit ff435da
 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master)

BUG=b:326869955
TEST=Test Audio use cases.

Change-Id: Id2ef4f23517ad869e4dc7ffa0a645fe060f8f018
Signed-off-by: Debi sahoo <debix.prasad.sahoo@intel.com>
Signed-off-by: Hubert Mazur <hmazur@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants