[SDW][BUG]IO transfer timed out occurred during multi-pipline stress test #1555

YvonneYang2 · 2019-11-27T07:14:09Z

Describe the bug
Occurred IO transfer timed out when run multi-pipline stress test.

To Reproduce

Boot up
Run with comman "./multiplex_pipeline-test_update.sh pcm0p pcm2p pcm5p pcm1c pcm4c"

script:
multiplex_pipeline-test_update.sh.txt

Reproduced rate
Round1 : Run 300 times, failed at 171th time.
Round2: Run 300 times, failed at 132th time.

Expected result
Run multi-pipline stress test successfully

Actual result
Occurred IO transfer timeed out during multi-pipline stress test

Dmesg

[ 1853.884397] intel-sdw sdw-master-0: intel_startup: SDW0 Pin2: start
[ 1853.884401] intel-sdw sdw-master-0: intel_startup: SDW0 Pin2: done
[ 1853.884403]  Headphone: ASoC: open FE Headphone
[ 1853.884406] sof-audio-pci 0000:00:1f.3: pcm: open stream 0 dir 0
[ 1853.884407] sof-audio-pci 0000:00:1f.3: period min 192 max 16384 bytes
[ 1853.884408] sof-audio-pci 0000:00:1f.3: period count 2 max 16
[ 1853.884408] sof-audio-pci 0000:00:1f.3: buffer max 65536 bytes
[ 1853.884532]  SDW0-Playback: ASoC: hw_params BE SDW0-Playback
[ 1853.884535] rt711 sdw:0:25d:711:0: rt711_pcm_hw_params rt711-aif1
[ 1855.910857] intel-sdw sdw-master-0: IO transfer timed out
[ 1857.959011] intel-sdw sdw-master-0: IO transfer timed out
[ 1857.959191] rt711 sdw:0:25d:711:0: [rt711_sdw_write] 7203 8283 <= 0031
[ 1860.007068] intel-sdw sdw-master-0: IO transfer timed out

Test recipe:
kernel: https://github.com/thesofproject/linux/tree/integration/soundwire-latest commit: d740a3e
FW: https://github.com/thesofproject/sof/commits/master commit: d740a3e
tplg: Same with FW branch, sof-cml-rt711-rt1308-mono-rt715.tplg
platform: CML Laptop with ALC711+ ALC1308+ ALC715 in SDW mode

Logs:
failed_multipipeline_test_171_dmesg.log
failed_multipipeline_test_171_all_logger.log
multipipeline_test_132_all_logger.log
multipipeline_test_132_dmesg.log

The text was updated successfully, but these errors were encountered:

bardliao · 2019-11-27T11:53:17Z

@YvonneYang2 Could you test it with bardliao@690b151 ?

YvonneYang2 · 2019-11-28T09:45:49Z

Tested it 300 times, issue can still be reproduced with latest integration/soundwire-latest + bardliao@690b151

Run 500 times, failed at 243th time.

Dmesg:

[ 2656.934491] intel-sdw sdw-master-1: intel_suspend_runtime start
[ 2656.934501] intel-sdw sdw-master-0: intel_suspend_runtime start
[ 2656.934638] intel-sdw sdw-master-1: intel_suspend_runtime done
[ 2656.935155] intel-sdw sdw-master-0: Link power down failed: -11
[ 2656.982207] intel-sdw sdw-master-3: intel_suspend_runtime start
[ 2656.982284] intel-sdw sdw-master-3: intel_suspend_runtime done
[ 2658.982463]  HDMI1: ASoC: pop wq checking: Playback status: inactive waiting: yes
[ 2658.982533]  Headphone: ASoC: pop wq checking: Playback status: inactive waiting: yes
[ 2658.982595]  SDW1-speakers: ASoC: pop wq checking: Playback status: inactive waiting: yes
[ 2661.140283]  Headphone: ASoC: found 0 audio playback paths
[ 2661.140285]  Headphone: ASoC: Headphone no valid playback route
[ 2661.140287] sdw_rt711_rt1308_rt715 sdw_rt711_rt1308_rt715: ASoC: find BE for widget ALH2.OUT
[ 2661.140288] sdw_rt711_rt1308_rt715 sdw_rt711_rt1308_rt715: ASoC: try BE : ALH2.OUT
[ 2661.140289]  Headphone: connected new DPCM playback path Headphone -> SDW0-Playback
[ 2661.140295]  Headphone: ASoC: found 1 new BE paths
[ 2661.140296]  SDW0-Playback: ASoC: open playback BE SDW0-Playback
[ 2661.140302] intel-sdw sdw-master-0: intel_startup: SDW0 Pin2: start
[ 2661.140306] intel-sdw sdw-master-0: intel_startup: SDW0 Pin2: done
[ 2661.140308]  Headphone: ASoC: open FE Headphone
[ 2661.140311] sof-audio-pci 0000:00:1f.3: pcm: open stream 0 dir 0
[ 2661.140312] sof-audio-pci 0000:00:1f.3: period min 192 max 16384 bytes
[ 2661.140313] sof-audio-pci 0000:00:1f.3: period count 2 max 16
[ 2661.140314] sof-audio-pci 0000:00:1f.3: buffer max 65536 bytes
[ 2661.140438]  SDW0-Playback: ASoC: hw_params BE SDW0-Playback
[ 2661.140440] rt711 sdw:0:25d:711:0: rt711_pcm_hw_params rt711-aif1
[ 2663.142438] intel-sdw sdw-master-0: IO transfer timed out
[ 2665.190452] intel-sdw sdw-master-0: IO transfer timed out
[ 2665.190473] rt711 sdw:0:25d:711:0: [rt711_sdw_write] 7203 8283 <= 0031
[ 2667.238473] intel-sdw sdw-master-0: IO transfer timed out
[ 2669.286264] intel-sdw sdw-master-0: IO transfer timed out
[ 2669.286285] rt711 sdw:0:25d:711:0: [rt711_sdw_write] 7209 8289 <= 0031

failed_multipipeline_test_243_all_logger.log
failed_multipipeline_test_243_dmesg.log

bardliao · 2019-11-29T00:29:50Z

Look like "Link power down failed: -11" can be seen on all the error dmesg log. Let's fix it first. @YvonneYang2 Could you test it with 0001-soundiwre-Intel-set-link_control-bits-again-when-it-.patch.txt?

patch

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 63931cda51ab..91bd6fe550fc 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -407,6 +407,7 @@ static int intel_link_power_down(struct sdw_intel *sdw)
 	unsigned int link_id = sdw->instance;
 	void __iomem *shim = sdw->link_res->shim;
 	u16 ioctl;
+	int retry = 3;
 
 	/* Glue logic */
 	ioctl = intel_readw(shim, SDW_SHIM_IOCTL(link_id));
@@ -424,6 +425,12 @@ static int intel_link_power_down(struct sdw_intel *sdw)
 	link_control &=  spa_mask;
 
 	ret = intel_clear_bit(shim, SDW_SHIM_LCTL, link_control, cpa_mask);
+	while (ret == -EAGAIN && retry >= 0) {
+		msleep(1);
+		ret = intel_clear_bit(shim, SDW_SHIM_LCTL,
+				      link_control, cpa_mask);
+		retry--;
+	}
 	if (ret < 0)
 		return ret;

YvonneYang2 · 2019-11-29T03:31:57Z

Tested it 500 times with latest integration/soundwire-latest (commit:d6078f9) +
0001-soundiwre-Intel-set-link_control-bits-again-when-it-.patch.txt
, IO transfer timed out issue and "Link power down failed: -11" both cannot be reproduced.

bardliao · 2019-11-29T08:15:38Z

@YvonneYang2 To double confirm. Could you check it with below patch instead?

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 63931cda51ab..686c409d66c3 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -150,7 +150,7 @@ static inline void intel_writew(void __iomem *base, int offset, u16 value)
 
 static int intel_clear_bit(void __iomem *base, int offset, u32 value, u32 mask)
 {
-	int timeout = 10;
+	int timeout = 100;
 	u32 reg_read;
 
 	writel(value, base + offset);

YvonneYang2 · 2019-11-29T09:54:10Z

@bardliao
Issue can still be reproduced with latest integration/soundwire-latest (commit:d6078f9) + patch as above.

Run 300 times, failed at 175th.

Dmesg:

[ 1878.026234] sof-audio-pci 0000:00:1f.3: pcm: open stream 0 dir 0
[ 1878.026235] sof-audio-pci 0000:00:1f.3: period min 192 max 16384 bytes
[ 1878.026236] sof-audio-pci 0000:00:1f.3: period count 2 max 16
[ 1878.026237] sof-audio-pci 0000:00:1f.3: buffer max 65536 bytes
[ 1878.026357]  SDW0-Playback: ASoC: hw_params BE SDW0-Playback
[ 1878.026360] rt711 sdw:0:25d:711:0: rt711_pcm_hw_params rt711-aif1
[ 1880.038362] intel-sdw sdw-master-0: IO transfer timed out
[ 1882.086346] intel-sdw sdw-master-0: IO transfer timed out
[ 1882.086368] rt711 sdw:0:25d:711:0: [rt711_sdw_write] 7203 8283 <= 0031

failed_multipipeline_test_175_all_logger.log
failed_multipipeline_test_175_dmesg.log

bardliao · 2019-12-02T11:55:36Z

@YvonneYang2 Could you test it with #1573?

plbossart · 2019-12-02T21:18:36Z

@bardliao what's interesting is that the only time when the error happens when the sequences for pm_runtime are interleaved:

[ 1873.951019] intel-sdw sdw-master-1: intel_suspend_runtime start
[ 1873.951031] intel-sdw sdw-master-0: intel_suspend_runtime start
[ 1873.951108] intel-sdw sdw-master-1: intel_suspend_runtime done
[ 1873.956341] intel-sdw sdw-master-0: Link power down failed: -11
[ 1873.963366] intel-sdw sdw-master-3: intel_suspend_runtime start
[ 1873.963433] intel-sdw sdw-master-3: intel_suspend_runtime done

so it's likely we need a mutual exclusion between links...

And looking at the code, it's pretty clear:

static int intel_link_power_up(struct sdw_intel *sdw)
{
	unsigned int link_id = sdw->instance;
	void __iomem *shim = sdw->link_res->shim;
	int spa_mask, cpa_mask;
	int link_control, ret;

	/* Link power up sequence */
	link_control = intel_readl(shim, SDW_SHIM_LCTL);
	spa_mask = (SDW_SHIM_LCTL_SPA << link_id);
	cpa_mask = (SDW_SHIM_LCTL_CPA << link_id);
	link_control |=  spa_mask;

	ret = intel_set_bit(shim, SDW_SHIM_LCTL, link_control, cpa_mask);
	if (ret < 0)
		return ret;

	sdw->cdns.link_up = true;
	return 0;
}

so what happens is that we have a common register which is accessed for different fields independently. that can't be good...

plbossart · 2019-12-02T21:20:24Z

it's even worse for power-down, we also touch the glue logic

static int intel_link_power_down(struct sdw_intel *sdw)
{
	int link_control, spa_mask, cpa_mask, ret;
	unsigned int link_id = sdw->instance;
	void __iomem *shim = sdw->link_res->shim;
	u16 ioctl;

	/* Glue logic */
	ioctl = intel_readw(shim, SDW_SHIM_IOCTL(link_id));
	ioctl |= SDW_SHIM_IOCTL_BKE;
	ioctl |= SDW_SHIM_IOCTL_COE;
	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);

	ioctl &= ~(SDW_SHIM_IOCTL_MIF);
	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);

	/* Link power down sequence */
	link_control = intel_readl(shim, SDW_SHIM_LCTL);
	spa_mask = ~(SDW_SHIM_LCTL_SPA << link_id);
	cpa_mask = (SDW_SHIM_LCTL_CPA << link_id);
	link_control &=  spa_mask;

	ret = intel_clear_bit(shim, SDW_SHIM_LCTL, link_control, cpa_mask);
	if (ret < 0)
		return ret;

	sdw->cdns.link_up = false;
	return 0;
}

…sters Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex at the controller level, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. GitHub issue: thesofproject#1555 Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

plbossart · 2019-12-02T22:11:51Z

@YvonneYang2 @bardliao can you look at #1577 ?
This uses some of Bard's ideas but explicitly used to prevent concurrent access to all SHIM registers.
It sounds obvious now that I think of it but since it wasn't in the initial code it wasn't straightforward to reverse-engineer.

bardliao · 2019-12-03T01:25:48Z

Thanks @plbossart You always think more than me. I believe that #1577 will fix the issue.

YvonneYang2 · 2019-12-03T07:13:01Z

Tested it 1000 times with #1577, issue cannot be reproduced.

Test recipe:
kernel: https://github.com/thesofproject/linux/tree/integration/soundwire-latest commit: b4f9bfa + #1577
FW: https://github.com/thesofproject/sof/commits/master commit: feb9e2b
tplg: Same with FW branch, sof-cml-rt711-rt1308-mono-rt715.tplg
platform: CML Laptop with ALC711+ ALC1308+ ALC715 in SDW mode

…sters Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex at the controller level, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. GitHub issue: thesofproject#1555 Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

plbossart · 2019-12-03T13:36:18Z

Thanks @plbossart You always think more than me. I believe that #1577 will fix the issue.

Nah, you did all the hard work and made it easy to connect the dots. Thanks @bardliao

plbossart · 2019-12-03T13:36:56Z

Tested it 1000 times with #1577, issue cannot be reproduced.

Test recipe:
kernel: https://github.com/thesofproject/linux/tree/integration/soundwire-latest commit: b4f9bfa + #1577
FW: https://github.com/thesofproject/sof/commits/master commit: feb9e2b
tplg: Same with FW branch, sof-cml-rt711-rt1308-mono-rt715.tplg
platform: CML Laptop with ALC711+ ALC1308+ ALC715 in SDW mode

very cool, thanks @YvonneYang2

…sters Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex at the controller level, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. GitHub issue: #1555 Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

YvonneYang2 · 2019-12-04T10:57:44Z

Since commit is merged into soundwire-latest, tested it 1000 times, passed for 300 times so far. Stress test is still on going, will check it tomorrow.

…sters Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex at the controller level, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. GitHub issue: #1555 Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: thesofproject#1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: #1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: thesofproject#1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: #1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

… SHIM registers Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. BugLink: thesofproject#1555 Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> (cherry picked from commit a24b7aa https://github.com/thesofproject/linux.git topic/sof-dev-rebase) BUG=b:145960362 TEST=Audio Playback via Speaker/Headset using Soundwire Daughter Card TOREVERT: Once upstream solution lands revert this. Change-Id: Ibb657fd226b3f5298412974b0991b668b6f669ee

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: #1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: thesofproject#1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: #1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: thesofproject#1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> BUG=b:145960362 TEST=Audio Playback via Speaker/Headset using Soundwire Daughter Card TOREVERT: Once upstream solution lands revert this. Change-Id: Id0c9a44836e088f7103fd960fa0a7b1d78c77b09

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: #1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: thesofproject#1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: #1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: thesofproject#1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> BUG=b:145960362 TEST=Audio Playback via Speaker/Headset using Soundwire Daughter Card TOREVERT: Once upstream solution lands revert this. Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: #1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: thesofproject#1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: #1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: thesofproject#1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: #1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: thesofproject#1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: #1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: #1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> Link: https://lore.kernel.org/r/20200716150947.22119-3-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>

Somehow the existing code is not aligned with the steps described in the documentation, refactor code and make sure the register programming sequences are correct. Also add missing power-up, power-down and wake capabilities (the last two are used in follow-up patches but introduced here for consistency). Some of the SHIM registers exposed fields that are link specific, and in addition some of the power-related registers (SPA/CPA) take time to be updated. Uncontrolled access leads to timeouts or errors. Add a mutex, shared by all links, so that all accesses to such registers are serialized, and follow a pattern of read-modify-write. This includes making sure SHIM_SYNC is programmed only once, before the first master is powered on. We use a 'shim_mask' field, shared between all links and protected by a mutex, to deal with power-up and power-down sequences. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. BugLink: thesofproject#1555 Signed-off-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> Link: https://lore.kernel.org/r/20200716150947.22119-3-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>

YvonneYang2 added CML Applies to Comet Lake platform SDW Applies to SoundWire bus for codec connection bug Something isn't working labels Nov 27, 2019

YvonneYang2 mentioned this issue Nov 27, 2019

[BUG][SDW] IPC timed out for 0x80010000 size 120 occurred during multipipline stress test. #1476

Closed

YvonneYang2 added the MSI Issues observed only in MSI mode label Nov 27, 2019

bardliao self-assigned this Nov 28, 2019

bardliao mentioned this issue Dec 2, 2019

soundiwre: Intel: clear link_control bits again when it return -EAGAIN #1568

Closed

plbossart mentioned this issue Dec 2, 2019

[TEST] soundwire: intel: add mutex to prevent concurrent access to SHIM regi… #1577

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SDW][BUG]IO transfer timed out occurred during multi-pipline stress test #1555

[SDW][BUG]IO transfer timed out occurred during multi-pipline stress test #1555

YvonneYang2 commented Nov 27, 2019 •

edited

Loading

bardliao commented Nov 27, 2019

YvonneYang2 commented Nov 28, 2019

bardliao commented Nov 29, 2019

YvonneYang2 commented Nov 29, 2019 •

edited

Loading

bardliao commented Nov 29, 2019

YvonneYang2 commented Nov 29, 2019

bardliao commented Dec 2, 2019

plbossart commented Dec 2, 2019 •

edited

Loading

plbossart commented Dec 2, 2019

plbossart commented Dec 2, 2019

bardliao commented Dec 3, 2019

YvonneYang2 commented Dec 3, 2019

plbossart commented Dec 3, 2019

plbossart commented Dec 3, 2019

YvonneYang2 commented Dec 4, 2019

[SDW][BUG]IO transfer timed out occurred during multi-pipline stress test #1555

[SDW][BUG]IO transfer timed out occurred during multi-pipline stress test #1555

Comments

YvonneYang2 commented Nov 27, 2019 • edited Loading

bardliao commented Nov 27, 2019

YvonneYang2 commented Nov 28, 2019

bardliao commented Nov 29, 2019

YvonneYang2 commented Nov 29, 2019 • edited Loading

bardliao commented Nov 29, 2019

YvonneYang2 commented Nov 29, 2019

bardliao commented Dec 2, 2019

plbossart commented Dec 2, 2019 • edited Loading

plbossart commented Dec 2, 2019

plbossart commented Dec 2, 2019

bardliao commented Dec 3, 2019

YvonneYang2 commented Dec 3, 2019

plbossart commented Dec 3, 2019

plbossart commented Dec 3, 2019

YvonneYang2 commented Dec 4, 2019

YvonneYang2 commented Nov 27, 2019 •

edited

Loading

YvonneYang2 commented Nov 29, 2019 •

edited

Loading

plbossart commented Dec 2, 2019 •

edited

Loading