linux-stable.git/drivers/soundwire/intel.c, branch linux-6.3.y Linux kernel stable tree soundwire: intel: don't save hw_params for use in prepare 2023-05-11T14:17:35+00:00 Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com 2023-03-21T02:26:42+00:00 4ddae1bef783a01dc099ec1dbeee0560bbe15f4a [ Upstream commit 0a0d1740bd8fd7dafb81fcb102fb5d0b83b1ce73 ] The existing code copies the hw_params pointer and reuses it later in .prepare, specifically to re-initialize the ALH DMA channel information that's lost in suspend-resume cycles. This is not needed, we can directly access the information from the substream/rtd - as done for the HDAudio DAIs in sound/soc/sof/intel/hda-dai.c In addition, using the saved pointer causes the suspend-resume test cases to fail on specific platforms, depending on which version of GCC is used. Péter Ujfalusi and I have spent long hours to root-cause this problem that was reported by the Intel CI first with 6.2-rc1 and again v6.3-rc1. In the latter case we were lucky that the problem was 100% reproducible on local test devices, and found out that adding a dev_dbg() or adding a call to usleep_range() just before accessing the saved pointer "fixed" the issue. With errors appearing just by changing the compiler version or minor changes in the code generated, clearly we have a memory management Heisenbug. The root-cause seems to be that the hw_params pointer is not persistent. The soc-pcm code allocates the hw_params structure on the stack, and passes it to the BE dailink hw_params and DAIs hw_params. Saving such a pointer and reusing it later during the .prepare stage cannot possibly work reliably, it's broken-by-design since v5.10. It's astonishing that the problem was not seen earlier. This simple fix will have to be back-ported to -stable, due to changes to avoid the use of the get/set_dmadata routines this patch will only apply on kernels older than v6.1. Fixes: a5a0239c27fe ("soundwire: intel: reinitialize IP+DSP in .prepare(), but only when resuming") Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> Link: https://lore.kernel.org/r/20230321022642.1426611-1-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org> 
[ Upstream commit 0a0d1740bd8fd7dafb81fcb102fb5d0b83b1ce73 ]

The existing code copies the hw_params pointer and reuses it later in
.prepare, specifically to re-initialize the ALH DMA channel
information that's lost in suspend-resume cycles.

This is not needed, we can directly access the information from the
substream/rtd - as done for the HDAudio DAIs in
sound/soc/sof/intel/hda-dai.c

In addition, using the saved pointer causes the suspend-resume test
cases to fail on specific platforms, depending on which version of GCC
is used. Péter Ujfalusi and I have spent long hours to root-cause this
problem that was reported by the Intel CI first with 6.2-rc1 and again
v6.3-rc1. In the latter case we were lucky that the problem was 100%
reproducible on local test devices, and found out that adding a
dev_dbg() or adding a call to usleep_range() just before accessing the
saved pointer "fixed" the issue. With errors appearing just by
changing the compiler version or minor changes in the code generated,
clearly we have a memory management Heisenbug.

The root-cause seems to be that the hw_params pointer is not
persistent. The soc-pcm code allocates the hw_params structure on the
stack, and passes it to the BE dailink hw_params and DAIs
hw_params. Saving such a pointer and reusing it later during the
.prepare stage cannot possibly work reliably, it's broken-by-design
since v5.10. It's astonishing that the problem was not seen earlier.

This simple fix will have to be back-ported to -stable, due to changes
to avoid the use of the get/set_dmadata routines this patch will only
apply on kernels older than v6.1.

Fixes: a5a0239c27fe ("soundwire: intel: reinitialize IP+DSP in .prepare(), but only when resuming")
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20230321022642.1426611-1-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
soundwire: intel: remove DAI startup/shutdown 2023-01-09T16:03:49+00:00 Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com 2022-12-15T08:54:36+00:00 5c2f4c9cfc79028079f0691899a93843827a00b7 The only thing these DAI startup/shutdown callbacks do is play with pm_runtime reference counts. This is not wrong, but it's not necessary at all. At the ASoC core level, only the component matters for pm_runtime. The ASoC core already calls pm_runtime_get_sync() in snd_soc_pcm_component_pm_runtime_get(), before the DAI startup callback is invoked. None of the SoundWire codec drivers rely on pm_runtime helpers in their DAI startup/shutdown either. This adds to the evidence that only the component, or more precisely the device specified when registering a component, should deal with pm_runtime transitions. Beyond the code cleanup, this move prepares for the addition of link power management in the auxiliary device startup/resume/suspend callbacks. The DAI callbacks can by-design assume that the device is already pm_runtime active. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Reviewed-by: Rander Wang <rander.wang@intel.com> Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> Link: https://lore.kernel.org/r/20221215085436.2001568-1-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org> 
The only thing these DAI startup/shutdown callbacks do is play with
pm_runtime reference counts.

This is not wrong, but it's not necessary at all. At the ASoC core level,
only the component matters for pm_runtime. The ASoC core already calls
pm_runtime_get_sync() in snd_soc_pcm_component_pm_runtime_get(),
before the DAI startup callback is invoked.

None of the SoundWire codec drivers rely on pm_runtime helpers in
their DAI startup/shutdown either. This adds to the evidence that only
the component, or more precisely the device specified when registering
a component, should deal with pm_runtime transitions.

Beyond the code cleanup, this move prepares for the addition of link
power management in the auxiliary device startup/resume/suspend
callbacks. The DAI callbacks can by-design assume that the device is
already pm_runtime active.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20221215085436.2001568-1-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
soundwire: intel: split auxdevice to different file 2022-11-23T14:41:49+00:00 Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com 2022-11-11T01:31:34+00:00 7cbf00bd4142cd88ac7ecbc4ea7b917a220cb721 The auxdevice layer is completely generic, it should be split from intel.c which is only geared to the 'cnl' hw_ops now. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> Link: https://lore.kernel.org/r/20221111013135.38289-8-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org> 
The auxdevice layer is completely generic, it should be split from
intel.c which is only geared to the 'cnl' hw_ops now.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20221111013135.38289-8-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
soundwire: intel: add in-band wake callbacks in hw_ops 2022-11-23T14:41:49+00:00 Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com 2022-11-11T01:31:33+00:00 36e3b385f35a33a10b792ec46350dd87d79e84dd No functionality change, only add indirection for in-band wake management helpers. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> Link: https://lore.kernel.org/r/20221111013135.38289-7-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org> 
No functionality change, only add indirection for in-band wake
management helpers.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20221111013135.38289-7-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
soundwire: intel: add link power management callbacks in hw_ops 2022-11-23T14:41:49+00:00 Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com 2022-11-11T01:31:32+00:00 49c9ff45991a5a62e040c8b43c89a9ab38a0a91f No functionality change, only add indirection for link power management helpers. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> Link: https://lore.kernel.org/r/20221111013135.38289-6-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org> 
No functionality change, only add indirection for link power
management helpers.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20221111013135.38289-6-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
soundwire: intel: add bus management callbacks in hw_ops 2022-11-23T14:41:48+00:00 Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com 2022-11-11T01:31:31+00:00 3db0c5a6a2832c7b4b40676299e4bbbe1a96bc8b No functionality change, only add indirection for bus management helpers. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> Link: https://lore.kernel.org/r/20221111013135.38289-5-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org> 
No functionality change, only add indirection for bus management
helpers.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20221111013135.38289-5-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
soundwire: intel: add register_dai callback in hw_ops 2022-11-23T14:41:48+00:00 Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com 2022-11-11T01:31:30+00:00 b6234bcc6589a0719ec91d810114c0b556a5b88b No functionality change, only add indirection for DAI registration helper. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> Link: https://lore.kernel.org/r/20221111013135.38289-4-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org> 
No functionality change, only add indirection for DAI registration
helper.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20221111013135.38289-4-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
soundwire: intel: add debugfs callbacks in hw_ops 2022-11-23T14:41:48+00:00 Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com 2022-11-11T01:31:29+00:00 fb2dc6a0a5f885233d632b1e92be9c0be977b0dc No functionality change, only add indirection for debugfs helpers. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> Link: https://lore.kernel.org/r/20221111013135.38289-3-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org> 
No functionality change, only add indirection for debugfs helpers.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20221111013135.38289-3-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
soundwire: intel: start using hw_ops 2022-11-23T14:41:48+00:00 Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com 2022-11-11T01:31:28+00:00 b3ad31f33982497dbc7a66a9d3013b1ac6985dfe Before introducing new hardware with completely different register spaces and programming sequences, we need to abstract some of the existing routines in hw_ops that will be platform-specific. For now we only use the 'cnl' ops - after the first Intel platform with SoundWire capabilities. Rather than one big intrusive patch, hw_ops are introduced in this patch so show the dependencies between drivers. Follow-up patches will introduce callbacks for debugfs, power and bus management. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Rander Wang <rander.wang@intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> Link: https://lore.kernel.org/r/20221111013135.38289-2-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org> 
Before introducing new hardware with completely different register
spaces and programming sequences, we need to abstract some of the
existing routines in hw_ops that will be platform-specific. For now we
only use the 'cnl' ops - after the first Intel platform with SoundWire
capabilities.

Rather than one big intrusive patch, hw_ops are introduced in this
patch so show the dependencies between drivers. Follow-up patches will
introduce callbacks for debugfs, power and bus management.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20221111013135.38289-2-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Merge branch 'fixes' into next 2022-11-23T14:21:09+00:00 Vinod Koul vkoul@kernel.org 2022-11-23T14:21:09+00:00 ca1c131ccf6e09f4976aeca035f12529c9a7d8a9 Merge fixes into next as Intel driver has a dependency 
Merge fixes into next as Intel driver has a dependency