Re: [PATCH v8 19/22] ASoC: qdsp6: audioreach: add q6apm-dai support

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
	bjorn.andersson@linaro.org, broonie@kernel.org, robh@kernel.org
Cc: plai@codeaurora.org, tiwai@suse.de, devicetree@vger.kernel.org,
	perex@perex.cz, alsa-devel@alsa-project.org, lgirdwood@gmail.com,
	bgoswami@codeaurora.org
Subject: Re: [PATCH v8 19/22] ASoC: qdsp6: audioreach: add q6apm-dai support
Date: Tue, 28 Sep 2021 10:33:19 +0100	[thread overview]
Message-ID: <c12ac2f5-bc53-ac0c-6ed0-f748a62015ac@linaro.org> (raw)
In-Reply-To: <20605122-e6b4-1b5f-003a-96a74306f984@linux.intel.com>

On 27/09/2021 17:25, Pierre-Louis Bossart wrote:
> 
>> +static int q6apm_dai_prepare(struct snd_soc_component *component,
>> +			     struct snd_pcm_substream *substream)
>> +{
>> +	struct snd_pcm_runtime *runtime = substream->runtime;
>> +	struct q6apm_dai_rtd *prtd = runtime->private_data;
>> +	struct audioreach_module_config cfg;
>> +	struct device *dev = component->dev;
>> +	struct q6apm_dai_data *pdata;
>> +	int ret;
>> +
>> +	pdata = snd_soc_component_get_drvdata(component);
>> +	if (!pdata)
>> +		return -EINVAL;
>> +
>> +	if (!prtd || !prtd->graph) {
>> +		dev_err(dev, "%s: private data null or audio client freed\n", __func__);
>> +		return -EINVAL;
>> +	}
>> +
>> +	cfg.direction = substream->stream;
>> +	cfg.sample_rate = runtime->rate;
>> +	cfg.num_channels = runtime->channels;
>> +	cfg.bit_width = prtd->bits_per_sample;
>> +
>> +	prtd->pcm_count = snd_pcm_lib_period_bytes(substream);
>> +	prtd->pos = 0;
>> +	/* rate and channels are sent to audio driver */
>> +	ret = q6apm_graph_media_format_shmem(prtd->graph, &cfg);
>> +	if (ret < 0) {
>> +		dev_err(dev, "%s: q6apm_open_write failed\n", __func__);
>> +		return ret;
>> +	}
>> +
>> +	ret = q6apm_graph_media_format_pcm(prtd->graph, &cfg);
>> +	if (ret < 0)
>> +		dev_err(dev, "%s: CMD Format block failed\n", __func__);
>> +
>> +	ret = q6apm_map_memory_regions(prtd->graph, substream->stream, prtd->phys,
>> +				       (prtd->pcm_size / prtd->periods), prtd->periods);
>> +
>> +	if (ret < 0) {
>> +		dev_err(dev, "Audio Start: Buffer Allocation failed rc = %d\n",	ret);
>> +		return -ENOMEM;
>> +	}
>> +
>> +	ret = q6apm_graph_prepare(prtd->graph);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to prepare Graph %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = q6apm_graph_start(prtd->graph);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to Start Graph %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
>> +		int i;
>> +		/* Queue the buffers for Capture ONLY after graph is started */
>> +		for (i = 0; i < runtime->periods; i++)
>> +			q6apm_read(prtd->graph);
>> +
>> +	}
>> +
>> +	prtd->state = Q6APM_STREAM_RUNNING;
> 
> you should probably explain why a stream moves to the 'RUNNING' state in
> a .prepare() callback, instead of TRIGGER_START?

Sure, will add a comment,

--srini
> 
>> +
>> +	return 0;
>> +}
>> +
>> +static int q6apm_dai_trigger(struct snd_soc_component *component,
>> +			     struct snd_pcm_substream *substream, int cmd)
>> +{
>> +	struct snd_pcm_runtime *runtime = substream->runtime;
>> +	struct q6apm_dai_rtd *prtd = runtime->private_data;
>> +	int ret = 0;
>> +
>> +	switch (cmd) {
>> +	case SNDRV_PCM_TRIGGER_START:
>> +	case SNDRV_PCM_TRIGGER_RESUME:
>> +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
>> +		 /* start writing buffers for playback only as we already queued capture buffers */
>> +		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
>> +			ret = q6apm_write_async(prtd->graph, prtd->pcm_count, 0, 0, 0);
>> +		break;
>> +	case SNDRV_PCM_TRIGGER_STOP:
>> +		/* TODO support be handled via SoftPause Module */
>> +		prtd->state = Q6APM_STREAM_STOPPED;
>> +		break;
>> +	case SNDRV_PCM_TRIGGER_SUSPEND:
>> +	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>> +		break;
>> +	default:
>> +		ret = -EINVAL;
>> +		break;
>> +	}
>> +
>> +	return ret;
>> +}
> 