Looks mostly good, couple of comments below.
+static int wsa881x_prepare(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct wsa881x_priv *wsa881x = dev_get_drvdata(dai->dev);
+ int ret;
+
+ if (wsa881x->stream_prepared) {
+ sdw_disable_stream(wsa881x->sruntime);
+ sdw_deprepare_stream(wsa881x->sruntime);
+ wsa881x->stream_prepared = false;
+ }
in what scenario would you have a transition from a stream active to
prepared?
+
+
+ ret = sdw_prepare_stream(wsa881x->sruntime);
+ if (ret)
+ return ret;
+
+ /**
+ * 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?
+ * 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?
If this is due to the order with the PA, then where is the PA handled?
+ 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?
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?
+ .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
