On 01/11/2019 16:39, Pierre-Louis Bossart wrote:
+static int qcom_swrm_prepare(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dai->dev);
+
+ if (!ctrl->sruntime[dai->id])
+ return -EINVAL;
+
+ return sdw_enable_stream(ctrl->sruntime[dai->id]);
So in hw_params you call sdw_prepare_stream() and in _prepare you
call sdw_enable_stream()?
Shouldn't this be handled in a .trigger operation as per the
documentation "From ASoC DPCM framework, this stream state is linked to
.trigger() start operation."
If I move sdw_enable/disable_stream() to trigger I get a big click
noise on my speakers at start and end of every playback. Tried
different things but nothing helped so far!. Enabling Speaker DACs
only after SoundWire ports are enabled is working for me!
There is nothing complicated on WSA881x codec side all the DACs are
enabled/disabled as part of DAPM.
that looks like a work-around to me? If you do a bank switch without
anything triggered, you are most likely sending a bunch of zeroes to
your amplifier and enabling click/pop removals somehow.
It'd be worth looking into this, maybe there's a missing digital
mute/unmute that's not done in the right order?
Digital mute does not help too, as they get unmuted before
sdw_enable_stream() call in trigger, I hit same click sound.
Same in the disable path too!
Also I noticed that there are more than 20+ register read/writes in the
sdw_enable_stream() path which took atleast 30 to 40 milliseconds.
I will try my luck checking the docs to see if I can find something
which talks about this.
--srini
It's also my understanding that .prepare will be called multiples times,
I agree, need to add some extra checks in the prepare to deal with this!
including for underflows and resume if you don't support INFO_RESUME.
the sdw_disable_stream() is in .hw_free, which is not necessarily
called by the core, so you may have a risk of not being able to recover?
Hmm, I thought hw_free is always called to release resources allocated
in hw_params.
In what cases does the core not call this?
yes, but prepare can be called without hw_free called first. that's why
we updated the state machine to allow for DISABLED|DEPREPARED ->
PREPARED transitions.
+static const struct dev_pm_ops qcom_swrm_dev_pm_ops = {
+ SET_RUNTIME_PM_OPS(qcom_swrm_runtime_suspend,
+ qcom_swrm_runtime_resume,
+ NULL
+ )
+};
Maybe define pm_runtime at a later time then? We've had a lot of race
conditions to deal with, and it's odd that you don't support plain
vanilla suspend first?
Trying to keep things simple for the first patchset! added this
dummies to keep the soundwire core happy!
If you are referring to the errors when pm_runtime is not enabled, we
fixed this is the series that's been out for review for 10 days now...
see '[PATCH 03/18] soundwire: bus: add PM/no-PM versions of read/write
functions', that should remove the need for dummy functions.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
