Re: [PATCH 1/2] ASoC: qcom: sdm845: handle soundwire stream

[Srinivas Kandagatla]

On 17/03/2020 13:07, Pierre-Louis Bossart wrote:
> > @@ -45,11 +48,20 @@ static int sdm845_slim_snd_hw_params(struct 
> > snd_pcm_substream *substream,
> > ÂÂÂÂÂ struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > ÂÂÂÂÂ struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> > ÂÂÂÂÂ struct snd_soc_dai *codec_dai;
> > +ÂÂÂ struct sdm845_snd_data *pdata = snd_soc_card_get_drvdata(rtd->card);
> > ÂÂÂÂÂ u32 rx_ch[SLIM_MAX_RX_PORTS], tx_ch[SLIM_MAX_TX_PORTS];
> > +ÂÂÂ struct sdw_stream_runtime *sruntime;
> > ÂÂÂÂÂ u32 rx_ch_cnt = 0, tx_ch_cnt = 0;
> > ÂÂÂÂÂ int ret = 0, i;
> > ÂÂÂÂÂ for_each_rtd_codec_dais(rtd, i, codec_dai) {
> > +ÂÂÂÂÂÂÂ sruntime = snd_soc_dai_get_sdw_stream(codec_dai,
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ substream->stream);
> > +ÂÂÂÂÂÂÂ if (sruntime != ERR_PTR(-ENOTSUPP))
> > +ÂÂÂÂÂÂÂÂÂÂÂ pdata->sruntime[cpu_dai->id] = sruntime;
> > +ÂÂÂÂÂÂÂ else
> > +ÂÂÂÂÂÂÂÂÂÂÂ pdata->sruntime[cpu_dai->id] = NULL;
> > +
>
> Can you explain this part?
> The get_sdw_stream() is supposed to return what was set by 
> set_sdw_stream(), so if it's not supported isn't this an error?

In this case its not an error because we have
totally 3 codecs in this path.
One wcd934x Slimbus codec and two wsa881x Soundwire Codec.

wcd934x codec side will return ENOTSUPP which is not an error.

> > ÂÂÂÂÂÂÂÂÂ ret = snd_soc_dai_get_channel_map(codec_dai,
> > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &tx_ch_cnt, tx_ch, &rx_ch_cnt, rx_ch);
> > @@ -425,8 +437,65 @@ static void sdm845_snd_shutdown(struct 
> > snd_pcm_substream *substream)
> > ÂÂÂÂÂ }
> > Â }
> > +static int sdm845_snd_prepare(struct snd_pcm_substream *substream)
> > +{
> > +ÂÂÂ struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > +ÂÂÂ struct sdm845_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
> > +ÂÂÂ struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> > +ÂÂÂ struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id];
> > +ÂÂÂ int ret;
> > +
> > +ÂÂÂ if (!sruntime)
> > +ÂÂÂÂÂÂÂ return 0;
>
> same here, isn't this an error?

These callbacks are used for other dais aswell in this case
HDMI, Slimbus and Soundwire, so sruntime being null is not treated as error.

> > +ÂÂÂ if (data->stream_prepared[cpu_dai->id]) {
> > +ÂÂÂÂÂÂÂ sdw_disable_stream(sruntime);
> > +ÂÂÂÂÂÂÂ sdw_deprepare_stream(sruntime);
> > +ÂÂÂÂÂÂÂ data->stream_prepared[cpu_dai->id] = false;
> > +ÂÂÂ }
> > +
> > +ÂÂÂ ret = sdw_prepare_stream(sruntime);
> > +ÂÂÂ if (ret)
> > +ÂÂÂÂÂÂÂ return ret;
> > +
> > +ÂÂÂ /**
> > +ÂÂÂÂ * NOTE: there is a strict hw requirement about the ordering of port
> > +ÂÂÂÂ * enables and actual WSA881x PA enable. PA enable should only 
> > happen
> > +ÂÂÂÂ * after soundwire ports are enabled if not DC on the line is
> > +ÂÂÂÂ * accumulated resulting in Click/Pop Noise
> > +ÂÂÂÂ * PA enable/mute are handled as part of codec DAPM and digital 
> > mute.
> > +ÂÂÂÂ */
> > +
> > +ÂÂÂ ret = sdw_enable_stream(sruntime);
> > +ÂÂÂ if (ret) {
> > +ÂÂÂÂÂÂÂ sdw_deprepare_stream(sruntime);
> > +ÂÂÂÂÂÂÂ return ret;
> > +ÂÂÂ }
> > +ÂÂÂ data->stream_prepared[cpu_dai->id] = true;
> > +
> > +ÂÂÂ return ret;
> > +}
> > +
> > +static int sdm845_snd_hw_free(struct snd_pcm_substream *substream)
> > +{
> > +ÂÂÂ struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > +ÂÂÂ struct sdm845_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
> > +ÂÂÂ struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> > +ÂÂÂ struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id];
> > +
> > +ÂÂÂ if (sruntime && data->stream_prepared[cpu_dai->id]) {
>
> and here?
>
> Really wondering where the stream is actually allocated and set.

Controller is allocating the runtime in this case.

> > +ÂÂÂÂÂÂÂ sdw_disable_stream(sruntime);
> > +ÂÂÂÂÂÂÂ sdw_deprepare_stream(sruntime);
> > +ÂÂÂÂÂÂÂ data->stream_prepared[cpu_dai->id] = false;
> > +ÂÂÂ }
> > +
> > +ÂÂÂ return 0;
> > +}
> > +
> > Â static const struct snd_soc_ops sdm845_be_ops = {
> > ÂÂÂÂÂ .hw_params = sdm845_snd_hw_params,
> > +ÂÂÂ .hw_free = sdm845_snd_hw_free,
> > +ÂÂÂ .prepare = sdm845_snd_prepare,
> > ÂÂÂÂÂ .startup = sdm845_snd_startup,
> > ÂÂÂÂÂ .shutdown = sdm845_snd_shutdown,
> > Â };