On 20/12/2019 15:45, Pierre-Louis Bossart wrote:
+
+ /**
+ * NOTE: there is a strict hw requirement about the ordering of port
+ * enables and actual PA enable. PA enable should only happen after
PA == power amplifiers?
Yes.
+ * soundwire ports are enabled if not DC on the line is accumlated
accumulated
+ * resulting in Click/Pop Noise
+ */
+
+ ret = sdw_enable_stream(wsa881x->sruntime);
I guess this answers to my question above, you are not using the 'usual'
mapping between ALSA states and SoundWire stream states. Enabling the
stream will cause a bank switch and (zero?) data to be transmitted, is
this intentional?
I guess Yes!
Myself and Vinod spent few weeks understanding the audio glitches if we
enable PA before soundwire ports, and finally hw guys came in with this
information, that PA has to be disabled before soundwire ports are enabled.
If this is due to the order with the PA, then where is the PA handled?
PA enable/mute are handled as part of DAPM and digital mute.
+ if (ret) {
+ sdw_deprepare_stream(wsa881x->sruntime);
+ return ret;
+ }
+ wsa881x->stream_prepared = true;
+
+ return ret;
+}
+
+static int wsa881x_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *dai)
+{
+ struct wsa881x_priv *wsa881x = dev_get_drvdata(dai->dev);
+ int i;
+
+ wsa881x->active_ports = 0;
+ for (i = 0; i < WSA881X_MAX_SWR_PORTS; i++) {
+ if (!wsa881x->port_enable[i])
+ continue;
+
+ wsa881x->port_config[wsa881x->active_ports] =
+ wsa881x_pconfig[i];
+ wsa881x->active_ports++;
+ }
+
+ return sdw_stream_add_slave(wsa881x->slave, &wsa881x->sconfig,
+ wsa881x->port_config, wsa881x->active_ports,
+ wsa881x->sruntime);
+}
+
+static int wsa881x_hw_free(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct wsa881x_priv *wsa881x = dev_get_drvdata(dai->dev);
+
+ sdw_disable_stream(wsa881x->sruntime);
+ sdw_deprepare_stream(wsa881x->sruntime);
This works if you do a hw_params->prepare->hw_free transition, but isn't
it possible to have hw_params->hw_free as well? In that case the stream
would not enabled/prepared, so shouldn't you have the same test as in
prepare?
Am not 100% sure if we would just have hw_params->hw_free, If that is
true, then yes we need the same check here too. However soundwire core
should throw invalid state error in such cases too.
if (wsa881x->stream_prepared) {
sdw_disable_stream(wsa881x->sruntime);
sdw_deprepare_stream(wsa881x->sruntime);
wsa881x->stream_prepared = false;
}
+ sdw_stream_remove_slave(wsa881x->slave, wsa881x->sruntime);
+ wsa881x->stream_prepared = false;
+
+ return 0;
+}
+
+static struct snd_soc_dai_driver wsa881x_dais[] = {
+ [0] = {
is that [0] needed?
Not really needed!
+ .name = "SPKR",
+ .id = 0,
+ .playback = {
+ .stream_name = "SPKR Playback",
+ .rate_max = 48000,
+ .rate_min = 48000,
+ .channels_min = 1,
+ .channels_max = 1,
+ },
+ .ops = &wsa881x_dai_ops,
+ },
+};
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
