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

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

> @@ -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?

>   		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?

> +	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.

> +		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,
>   };
>