On start/pause_release/resume, when more than one FE is connected to
the same BE, it's possible that the trigger is sent more than
once. This is not desirable, we only want to trigger a BE once, which
is straightforward to implement with a refcount.
For stop/pause/suspend, the problem is more complicated: the check
implemented in snd_soc_dpcm_can_be_free_stop() may fail due to a
conceptual deadlock when we trigger the BE before the FE. In this
case, the FE states have not yet changed, so there are corner cases
where the TRIGGER_STOP is never sent - the dual case of start where
multiple triggers might be sent.
This patch suggests an unconditional trigger in all cases, without
checking the FE states, using a refcount protected by a spinlock.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Message-Id: <20210817164054.250028-3-pierre-louis.bossart@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
When more than one FE is connected to a BE, e.g. in a mixing use case,
the BE can be triggered multiple times when the FE are opened/started
concurrently. This race condition is problematic in the case of
SoundWire BE dailinks, and this is not desirable in a general
case. The code carefully checks when the BE can be stopped or
hw_free'ed, but the trigger code does not use any mutual exclusion.
Fix by using the same spinlock already used to check FE states, and
set the state before the trigger. In case of errors, the initial
state will be restored.
This patch does not change how the triggers are handled, it only makes
sure the states are handled in critical sections.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Message-Id: <20210817164054.250028-2-pierre-louis.bossart@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
This patch cleanups below cppcheck warning.
sound/soc/soc-pcm.c:1624:30: style: The scope of the variable 'codec_stream' can be reduced. [variableScope]
struct snd_soc_pcm_stream *codec_stream;
^
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87o8aozf28.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
This patch cleanups below cppcheck warning.
sound/soc/soc-pcm.c:1305:30: style: The scope of the variable 'widget' can be reduced. [variableScope]
struct snd_soc_dapm_widget *widget;
^
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87pmv4zf2c.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
This patch cleanups below cppcheck warning.
sound/soc/soc-pcm.c:2578:22: style: The scope of the variable 'codec_dai' can be reduced. [variableScope]
struct snd_soc_dai *codec_dai;
^
sound/soc/soc-pcm.c:2580:6: style: The scope of the variable 'stream' can be reduced. [variableScope]
int stream;
^
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87r1fkzf2g.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
This patch cleanups below cppcheck warning.
sound/soc/soc-pcm.c:631:9: style: The scope of the variable 'r' can be reduced. [variableScope]
int i, r, ret = 0;
^
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87sg00zf2l.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
This patch cleanups below cppcheck warning.
sound/soc/soc-pcm.c:446:29: style: The scope of the variable 'pcm_codec' can be reduced. [variableScope]
struct snd_soc_pcm_stream *pcm_codec, *pcm_cpu;
^
sound/soc/soc-pcm.c:446:41: style: The scope of the variable 'pcm_cpu' can be reduced. [variableScope]
struct snd_soc_pcm_stream *pcm_codec, *pcm_cpu;
^
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87tukgzf2p.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
On stream stop, currently CPU DAI stop sequence invoked first
followed by DMA. For Few platforms, it is required to stop the
DMA first before stopping CPU DAI.
Introduced new flag in dai_link structure for reordering stop sequence.
Based on flag check, ASoC core will re-order the stop sequence.
Fixes: 4378f1fbe9 ("ASoC: soc-pcm: Use different sequence for start/stop trigger")
Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
Link: https://lore.kernel.org/r/20210716123015.15697-1-vijendar.mukunda@amd.com
Signed-off-by: Mark Brown <broonie@kernel.org>
In case, where the loops are not executed for a reason, the uninitialized
variable 'err' is returned to the caller. Make code fully predictible
and assign zero in the declaration.
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
Cc: Mark Brown <broonie@kernel.org>
Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/20210614071746.1787072-1-perex@perex.cz
Signed-off-by: Mark Brown <broonie@kernel.org>
soc_pcm_params_symmetry() checks CPU / Codec symmetry.
Unfortunately there was bug on it (= A) which didn't check Codec.
But is back by (B).
A: v5.7: commit c840f7698d ("ASoC: soc-pcm: Merge for_each_rtd_cpu/codec_dais()")
B: v5.12: commit 3a90672111 ("ASoC: soc-pcm: cleanup soc_pcm_params_symmetry()")
In total,
old - v5.6 (= Generation-1):
symmetric_rate : DAI_Link / CPU / Codec
symmetric_channels : DAI_Link / CPU / Codec
symmetric_sample_bits : DAI_Link / CPU / Codec
v5.7 - v5.11 (= Generation-2): (= because of bug by (A))
symmetric_rate : DAI_Link / CPU
symmetric_channels : DAI_Link / CPU / Codec
symmetric_sample_bits : DAI_Link / CPU / Codec
v5.12 - (= Generation-3): (= back by (B))
symmetric_rate : DAI_Link / CPU / Codec
symmetric_channels : DAI_Link / CPU / Codec
symmetric_sample_bits : DAI_Link / CPU / Codec
OTOH, we can use DPCM which is configured by FE / BE.
Both FE / BE uses dummy-DAI.
FE: CPU <-> dummy-DAI
BE: dummy-DAI <-> Codec
One note is that we can use .be_hw_params_fixup in DPCM case.
This means BE settings might be fixuped/updated by FE.
This feature is used for example on MIXer case.
It can be happen not only for rate, but for channels/sample_bits too.
Because of these reasons, below issue happen on
Generation-1 / Generation-3, if...
1) Sound Card used DPCM
2) It exchanges rate to 48kHz by using .be_hw_params_fixup()
3) Codec had symmetric_rate = 1
I didn't confirm, but maybe same things happen
if it exchanged channels/sample_bits at Generation-1/2/3 too.
# aplay 44100.wav
# aplay 44100.wav
=> [kernel] be.ak4613-hifi: ASoC: unmatched rate symmetry: snd-soc-dummy-dai:44100 - soc_pcm_params_symmetry:48000
[kernel] be.ak4613-hifi: ASoC: hw_params BE failed -22
[kernel] fe.rsnd-dai.0: ASoC: hw_params BE failed -22
aplay: set_params:1407: Unable to install hw params:
ACCESS: RW_INTERLEAVED
FORMAT: S16_LE
SUBFORMAT: STD
SAMPLE_BITS: 16
FRAME_BITS: 32
CHANNELS: 2
RATE: 44100
PERIOD_TIME: (23219 23220)
PERIOD_SIZE: 1024
PERIOD_BYTES: 4096
PERIODS: 4
BUFFER_TIME: (92879 92880)
BUFFER_SIZE: 4096
BUFFER_BYTES: 16384
TICK_TIME: 0
soc_pcm_params_symmetry() checks by below
if (symmetry)
for_each_rtd_cpu_dais(rtd, i, cpu_dai)
if (cpu_dai->xxx && cpu_dai->xxx != d.xxx) {
dev_err(rtd->dev, "...");
return -EINVAL;
}
Because of above reason 3) (= Codec had symmetric_rate = 1)
BE can't ignore "if (symmetric)".
At 1st aplay, soc_pcm_params_symmetry() ignores it,
because dummy-DAI->rate is 0.
After this check, each DAI sets/keep settings.
In above sample case, BE gets 48000 and FE gets 44100,
and it happen BE -> FE order.
Because DPCM is sharing *same* dummy-DAI,
dummy-DAI sets as 48000 by BE, and is overwrote by 44100 by FE.
This settings never be cleaned (= a) after 1st aplay,
because dummy-DAI is used from FE/BE, never be last user (b).
static int soc_pcm_hw_clean(...)
{
...
for_each_rtd_dais(rtd, i, dai) {
...
(b) if (snd_soc_dai_active(dai) == 1)
(a) soc_pcm_set_dai_params(dai, NULL);
...
}
...
}
At 2nd aplay, BE gets 48000 but dummy-DAI is keeping 44100,
soc_pcm_params_symmetry() checks will fail.
To solve this issue, this patch ignores dummy-DAI
at soc_pcm_params_symmetry()
Link: https://lore.kernel.org/r/87a6q0z4xt.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87y2djxa2n.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
It indicates unmatched symmetry value, but not indicates on which DAI.
This patch indicates it.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/871rbbyono.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
__soc_pcm_params_symmetry() macro is using "name" as parameter
which will be exchanged to rate/channles/sample_bit, like below
dai->name => dai->rate
dai->name => dai->channels
dai->name => dai->sample_bit
But, dai itself has "name". This means
1) It is very confusable naming
2) It can't use dai->name
This patch use "xxx" instead of "name"
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/8735vryoob.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Add snd_soc_pcm_component_ack back, which can be used to get an
updated buffer pointer in the platform driver.
On Asymmetric multiprocessor, this pointer can be sent to Cortex-M
core for audio processing.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Link: https://lore.kernel.org/r/1615516725-4975-2-git-send-email-shengjiu.wang@nxp.com
Signed-off-by: Mark Brown <broonie@kernel.org>
All snd_soc_component_xxx() and snd_soc_pcm_component_xxx() itself
indicate error message if failed.
Its caller doesn't need to indicate duplicated error message.
This patch removes it.
All snd_soc_component_xxx() indicate error message if failed.
Its caller doesn't need to indicate duplicated error message.
This patch removes it.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/878s6puta6.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
All snd_soc_dai_xxx() and snd_soc_pcm_dai_xxx() itself
indicate error message if failed.
Its caller doesn't need to indicate duplicated error message.
This patch removes it.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87a6r5utaa.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
soc_pcm_hw_free() never fail, error message is not needed.
We can't use void function for it, because it is used
part of struct snd_pcm_ops :: hw_free.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87czw1utaj.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.
int function(...)
{
...
return ret;
}
int caller(...)
{
...
ret = function(...);
if (ret < 0)
dev_err(...)
...
}
This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.
If function() indicates error message, we can get same and
detail information without forgot.
int function(...)
{
...
if (ret < 0)
dev_err(...)
return ret;
}
int caller(...)
{
...
ret = function(...);
...
}
This patch follow above style at dpcm_fe/be_dai_prepare()
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87eeghutap.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.
int function(...)
{
...
return ret;
}
int caller(...)
{
...
ret = function(...);
if (ret < 0)
dev_err(...)
...
}
This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.
If function() indicates error message, we can get same and
detail information without forgot.
int function(...)
{
...
if (ret < 0)
dev_err(...)
return ret;
}
int caller(...)
{
...
ret = function(...);
...
}
This patch follow above style at dpcm_fe/be_dai_hw_params()
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87ft0xutat.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.
int function(...)
{
...
return ret;
}
int caller(...)
{
...
ret = function(...);
if (ret < 0)
dev_err(...)
...
}
This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.
If function() indicates error message, we can get same and
detail information without forgot.
int function(...)
{
...
if (ret < 0)
dev_err(...)
return ret;
}
int caller(...)
{
...
ret = function(...);
...
}
This patch follow above style at dpcm_fe/be_dai_startup().
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87h7ldutay.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.
int function(...)
{
...
return ret;
}
int caller(...)
{
...
ret = function(...);
if (ret < 0)
dev_err(...)
...
}
This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.
If function() indicates error message, we can get same and
detail information without forgot.
int function(...)
{
...
if (ret < 0)
dev_err(...)
return ret;
}
int caller(...)
{
...
ret = function(...);
...
}
This patch also
do below to dpcm_run_update_startup()
1) remove duplicated ret = -EINVAL
2) remove blank line
do below to dpcm_run_update_shutdown()
1) remove unused ret
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87im5tutb3.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.
int function(...)
{
...
return ret;
}
int caller(...)
{
...
ret = function(...);
if (ret < 0)
dev_err(...)
...
}
This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.
If function() indicates error message, we can get same and
detail information without forgot.
int function(...)
{
...
if (ret < 0)
dev_err(...)
return ret;
}
int caller(...)
{
...
ret = function(...);
...
}
This patch follow above style at dpcm_apply_symmetry(...)
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87k0q9utb9.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.
int function(...)
{
...
return ret;
}
int caller(...)
{
...
ret = function(...);
if (ret < 0)
dev_err(...)
...
}
This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.
If function() indicates error message, we can get same and
detail information without forgot.
int function(...)
{
...
if (ret < 0)
dev_err(...)
return ret;
}
int caller(...)
{
...
ret = function(...);
...
}
Now, dpcm_be_dai_trigger() user uses it like below.
err = dpcm_be_dai_trigger(...);
if (err < 0)
dev_err(..., "ASoC: trigger FE failed %d\n", err);
But we can get more detail information if dpcm_be_dai_trigger() itself
had dev_err(). And above error message is confusable,
failed is *BE*, not *FE*.
This patch indicates error message at dpcm_be_dai_trigger().
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87lfaputbe.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.
int function(...)
{
...
return ret;
}
int caller(...)
{
...
ret = function(...);
if (ret < 0)
dev_err(...)
...
}
This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.
If function() indicates error message, we can get same and
detail information without forgot.
int function(...)
{
...
if (ret < 0)
dev_err(...)
return ret;
}
int caller(...)
{
...
ret = function(...);
...
}
Now, many place uses dpcm_path_get() like below
ret = dpcm_path_get(...);
if (ret < 0)
goto error;
(A) else if (ret == 0)
dev_dbg(...)
But here, (A) part can be indicated at dpcm_path_get() not caller.
It is simple and readable code.
This patch do it.
Small detail behaviors will be exchanged by this patch.
1) indicates debug info (= path numbers) if path > 0 case only
(It was *always* indicated).
2) soc_dpcm_fe_runtime_update() is indicating error message
for paths < 0 case, but it is already done at dpcm_path_get().
Thus just remove it. but dev_dbg() vs dev_warn() is exchanged.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87mtv5utbj.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.
int function(...)
{
...
return ret;
}
int caller(...)
{
...
ret = function(...);
if (ret < 0)
dev_err(...)
...
}
This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.
If function() indicates error message, we can get same and
detail information without forgot.
int function(...)
{
...
if (ret < 0)
dev_err(...)
return ret;
}
int caller(...)
{
...
ret = function(...);
...
}
This patch follow above style at soc_pcm_prepare().
By this patch, dpcm_fe/be_dai_prepare(...)
temporary lacks FE/BE error info, but it will reborn soon.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87o8flutbn.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.
int function(...)
{
...
return ret;
}
int caller(...)
{
...
ret = function(...);
if (ret < 0)
dev_err(...)
...
}
This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.
If function() indicates error message, we can get same and
detail information without forgot.
int function(...)
{
...
if (ret < 0)
dev_err(...)
return ret;
}
int caller(...)
{
...
ret = function(...);
...
}
This patch follow above style at soc_pcm_hw_params().
By this patch, dpcm_fe/be_dai_hw_params(...)
temporary lacks FE/BE error info, but it will reborn soon.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87pn01utbt.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.
int function(...)
{
...
return ret;
}
int caller(...)
{
...
ret = function(...);
if (ret < 0)
dev_err(...)
...
}
This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.
If function() indicates error message, we can get same and
detail information without forgot.
int function(...)
{
...
if (ret < 0)
dev_err(...)
return ret;
}
int caller(...)
{
...
ret = function(...);
...
}
This patch follow above style at soc_pcm_open().
By this patch, dpcm_fe/be_dai_startup(...)
temporary lacks FE/BE error info, but it will reborn soon.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87r1khutby.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
soc-pcm has very similar but different DPCM BE DAI stop operation at
1) dpcm_be_dai_startup() error case rollback
2) dpcm_be_dai_startup_unwind()
3) dpcm_be_dai_shutdown()
The differences are
1) for rollback
2) Doesn't check by snd_soc_dpcm_be_can_update() (Is this bug ?)
3) Do soc_pcm_hw_free() if it was not !OPENed and !HW_FREEed,
and call soc_pcm_close().
We can share same code by
1) hw_free is not needed. Needs last dpcm as rollback.
2) hw_free is not needed.
3) hw_free is needed.
This patch adds new dpcm_be_dai_stop() and share these 3.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87a6rduoam.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
rtd->dai_link is setuped at soc_new_pcm_runtime(),
thus "rtd->dai_link == NULL" is never happen.
This patch removes unneeded !rtd->dai_link check
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87blbtuoar.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
At dpcm_be_dai_startup_unwind(), it indicates error message at (1)
if this function was called with no users.
But, it doesn't use "continue" here. Thus, users will be a
negative number at (2)
void dpcm_be_dai_startup_unwind(...)
{
...
for_each_dpcm_be(...) {
...
(1) if (be->dpcm[stream].users == 0)
dev_err(...);
(2) if (--be->dpcm[stream].users != 0)
continue;
At dpcm_be_dai_startup(), it indicates error message if
user reached to MAX USERS at (A).
But, it doesn't use "continue" here. Thus, it will be over
MAX USERS at (B).
int dpcm_be_dai_startup(...)
{
...
for_each_dpcm_be(...) {
...
(A) if (be->dpcm[stream].users == DPCM_MAX_BE_USERS)
dev_err(...);
(B) if (be->dpcm[stream].users++ != 0)
continue;
These are just bug. This patch fixup these.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87czw9uoav.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Current soc_pcm_open() is checking runtime->hw parameters, but having
such function is very helpful for reading code.
This patch adds new soc_hw_sanity_check() and checks runtime->hw
parameters there. And print its debug message there, too.
Debug message print out timing is exchanged after this patch,
but it is not a big deal, because it is for debug.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87eegpuob1.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Current soc-pcm has soc_pcm_has_symmetry() and using it as
if (soc_pcm_has_symmetry(substream))
substream->runtime->hw.info |= SNDRV_PCM_INFO_JOINT_DUPLEX;
We want to share same operation as same function.
This patch adds soc_pcm_update_symmetry() and pack above code in
one function.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87ft15uob6.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
snd_soc_set_runtime_hwparams() is called from each driver
to initialize hw parameters,
but coping each parameters one-by-one.
Current code is not copying all parameters, but no big effect
if we do it. This patch copies all parameters by simple code.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87h7lluoba.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
soc-pcm needs DAI name and it will be "multicpu/multicodec" if it has
many DAIs. But current code is using very verbose for it.
This patch uses macro and makes code simple.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87im61uobf.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
soc_pcm_apply_symmetry() is used like below in all cases.
if (snd_soc_dai_active(dai)) {
err = soc_pcm_apply_symmetry(fe_substream, dai);
...
}
Because of this style, the code is deep nested.
This patch checks it under soc_pcm_apply_symmetry(), and makes code simple.
static int soc_pcm_apply_symmetry(...)
{
...
=> if (!snd_soc_dai_active(...))
return 0;
...
}
=> ret = soc_pcm_apply_symmetry();
if (ret < 0)
...
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87k0qhuobl.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
cppcheck warning:
sound/soc/soc-pcm.c:1907:6: style: Variable 'err' is assigned a value
that is never used. [unreadVariable]
err = dpcm_be_dai_hw_free(fe, stream);
^
it's not clear why dpcm_be_dai_hw_free() is sometimes called without
testing the error status, and sometimes an error message is provided.
When in doubt, add an error message for consistency. This may have to
be revisited.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20210218221921.88991-5-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
cppcheck warning:
sound/soc/soc-pcm.c:2398:7: style: Variable 'ret' is reassigned a
value before the old one has been used. [redundantAssignment]
ret = -EINVAL;
^
sound/soc/soc-pcm.c:2395:7: note: ret is assigned
ret = -EINVAL;
^
sound/soc/soc-pcm.c:2398:7: note: ret is overwritten
ret = -EINVAL;
^
This looks like a copy/paste or git merge issue.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20210218221921.88991-3-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
dpcm_fe_dai_startup() (= A) calls dpcm_set_fe_runtime() (= B) to setup
DPCM runtime. From *naming point of view*, it sounds like setup function
for FE.
(A) static int dpcm_fe_dai_startup(...)
{
...
(B) dpcm_set_fe_runtime(...);
...
}
But in dpcm_set_fe_runtime() (= B),
It setups FE by dpcm_runtime_setup_fe() (= X),
and setups BE by dpcm_runtime_merge_xxx() (= Y).
(B) static void dpcm_set_fe_runtime(...)
{
...
(X) dpcm_runtime_setup_fe(...);
^ dpcm_runtime_setup_be_format(...);
(Y) dpcm_runtime_setup_be_chan(...);
v dpcm_runtime_setup_be_rate(...);
}
These means that the function which is called as xxx_fe_xxx()
is setups both FE and BE. This is confusable and can be hot bed for bug.
Now dpcm_set_fe_runtime() (= B) is simple enough and confusable naming,
let's unpack it at dpcm_fe_dai_startup().
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87mtvxvsgn.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
dpcm_fe_dai_startup() (= A) calls dpcm_set_fe_runtime() (= B) to setup
DPCM runtime. From *naming point of view*, it sounds like setup function
for FE.
(A) static int dpcm_fe_dai_startup(...)
{
...
(B) dpcm_set_fe_runtime(...);
...
}
But in dpcm_set_fe_runtime() (= B),
It setups FE by dpcm_runtime_setup_fe() (= X),
and setups BE by dpcm_runtime_merge_xxx() (= Y).
(B) static void dpcm_set_fe_runtime(...)
{
...
(X) dpcm_runtime_setup_fe(...);
^ dpcm_runtime_merge_format(...);
(Y) dpcm_runtime_merge_chan(...);
v dpcm_runtime_merge_rate(...);
}
These means that the function which is called as xxx_fe_xxx()
is setups both FE and BE. This is confusable and can be hot bed for bug.
This patch renames unclear dpcm_runtime_merge_xxx() (= Y) to
more clear dpcm_runtime_setup_be_xxx().
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87o8gdvsgr.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
dpcm_fe_dai_startup() (= A) calls dpcm_set_fe_runtime() (= B) to setup
DPCM runtime. From *naming point of view*, it sounds like setup function
for FE.
(A) static int dpcm_fe_dai_startup(...)
{
...
(B) dpcm_set_fe_runtime(...);
...
}
But in dpcm_set_fe_runtime() (= B),
It setups FE by for_each loop (= X),
and setups BE by dpcm_runtime_merge_xxx() (= Y).
(B) static void dpcm_set_fe_runtime(...)
{
...
^ for_each_rtd_cpu_dais(...) {
| ...
(X) soc_pcm_hw_update_rate(...);
| soc_pcm_hw_update_chan(...);
| soc_pcm_hw_update_format(...);
v }
^ dpcm_runtime_merge_format(...);
(Y) dpcm_runtime_merge_chan(...);
v dpcm_runtime_merge_rate(...);
}
These means that the function which is called as xxx_fe_xxx()
is setups both FE and BE. This is confusable and can be hot bed for bug.
To tidyup it, as 1st step, this patch adds new dpcm_runtime_setup_fe()
for (X).
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87pn0tvsgx.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
dpcm_init_runtime_hw() (= A) is used from dpcm_set_fe_runtime() (= B)
with for_each_rtd_cpu_dais() loop (= C), and it checks formats (= D).
(A) static void dpcm_init_runtime_hw(...)
{
...
^ if (runtime->hw.formats)
| (D1) runtime->hw.formats &= stream->formats;
(D) else
| (D2) runtime->hw.formats = stream->formats;
v }
(B) static void dpcm_set_fe_runtime(...)
{
...
(C) for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
...
(A) dpcm_init_runtime_hw(...);
}
}
If this for_each_rtd_cpu_dais() loop (= C) calls
dpcm_init_runtime_hw() (= A) multiple times, this means it is Multi-CPU.
If we focus to format operation at (D), using mask (= D1) is understandable
because it restricts unsupported format.
But, enabling format when zero format case (= D2) is very strange,
because it might enables unsupported format.
This runtime->hw.formats is initialized by ULLONG_MAX at soc_pcm_hw_init(),
thus becoming zero format means it can't use such format.
And doing this strange format operation is only here.
This patch removes strange format operation (= D2), and use standard
soc_pcm_hw_update_format() for it.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87sg5pvshq.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
In case DPCM runtime has multiple CPU DAIs, dpcm_init_runtime_hw() is
called multiple times, once for each CPU DAI. This will lead to
ignoring hw limits of all but the last DAI.
Fix this by moving soc_pcm_hw_init() up by one level to
dpcm_init_runtime_hw().
Fixes: 140f553d12 ("ASoC: soc-pcm: fix hwparams min/max init for dpcm")
Suggested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Link: https://lore.kernel.org/r/20210216172251.3023723-1-kai.vehmanen@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
When runtime is initialized with dpcm_init_runtime_hw(), some of the
min/max calculations assume that defaults are set. For example
calculation of channel min/max values may be done using zero-initialized
data and soc_pcm_hw_update_chan() will always return max-channels of 0
in this case. This will result in failure to open the PCM at all.
Fix the issue by calling soc_pcm_hw_init() before calling any
soc_pcm_hw_update_*() functions.
Remove the conditional code on runtime->hw.formats as this field
is anyways set in soc_pcm_hw_init().
Fixes: 6cb56a4549 ("ASoC: soc-pcm: add soc_pcm_hw_update_chan()")
Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Link: https://lore.kernel.org/r/20210214220414.2876690-1-kai.vehmanen@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
We have soc_pcm_hw_update_xxx() now.
This patch creates same function for format.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Link: https://lore.kernel.org/r/87pn1g90oa.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
We have soc_pcm_hw_update_rate() now.
This patch creates same function for chan.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Link: https://lore.kernel.org/r/87r1lw90oo.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
