On Fri, Apr 10, 2015 at 04:14:09PM +0800, Koro Chen wrote:
> + if (memif->use_sram) {
> + struct snd_dma_buffer *dma_buf = &substream->dma_buffer;
> + int size = params_buffer_bytes(params);
> +
> + memif->buffer_size = size;
> + memif->phys_buf_addr = afe->sram_phy_address;
> +
> + dma_buf->bytes = size;
> + dma_buf->area = (unsigned char *)afe->sram_address;
> + dma_buf->addr = afe->sram_phy_address;
> + dma_buf->dev.type = SNDRV_DMA_TYPE_DEV;
> + dma_buf->dev.dev = substream->pcm->card->dev;
> + snd_pcm_set_runtime_buffer(substream, dma_buf);
> + } else {
> + ret = snd_pcm_lib_malloc_pages(substream,
> + params_buffer_bytes(params));
> + if (ret < 0)
> + return ret;
> +
> + memif->phys_buf_addr = substream->runtime->dma_addr;
> + memif->buffer_size = substream->runtime->dma_bytes;
> + }
Ah, so the SRAM is directly memory mappable. Nice. But we have a
limited amount of it so need to allocate it to a device somehow based on
some factor I guess?
> +static int mtk_afe_set_adda_dac_out(struct mtk_afe *afe, uint32_t rate)
> +{
> + u32 audio_i2s_dac = 0;
> + u32 con0, con1;
> +
> + /* set dl src2 */
> + con0 = (mtk_afe_adda_fs(rate) << 28) | (0x03 << 24) | (0x03 << 11);
> +
> + /* 8k or 16k voice mode */
> + if (con0 == 0 || con0 == 3)
> + con0 |= 0x01 << 5;
This all looks a bit magic, some defines would not go amiss here.
> + /* SA suggests to apply -0.3db to audio/speech path */
> + con0 = con0 | (0x01 << 1);
> + con1 = 0xf74f0000;
More magic numbers! This also suggests that there is a volume control
lurking in here which could usefully be exposed to applications?
> +static void mtk_afe_pmic_shutdown(struct mtk_afe *afe,
> + struct snd_pcm_substream *substream)
> +{
> + /* output */
> + regmap_update_bits(afe->regmap, AFE_ADDA_DL_SRC2_CON0, 1, 0);
> + regmap_update_bits(afe->regmap, AFE_I2S_CON1, 1, 0);
> +
> + /* input */
> + regmap_update_bits(afe->regmap, AFE_ADDA_UL_SRC_CON0, 1, 0);
> + /* disable ADDA */
> + regmap_update_bits(afe->regmap, AFE_ADDA_UL_DL_CON0, 1, false);
> +}
This is looking like exposing the routing and using DAPM might save a
bunch of code? Overall my main thought looking at the code here and
what the hardware was described as doing is that it'd all be simpler if
it were a DPCM based thing using DAPM for power. I think I'd like to
see a strong reason for not using at least DPCM.
> + if (rate == MTK_AFE_I2S_RATE_8K)
> + voice_mode = 0;
> + else if (rate == MTK_AFE_I2S_RATE_16K)
> + voice_mode = 1;
> + else if (rate == MTK_AFE_I2S_RATE_32K)
> + voice_mode = 2;
> + else if (rate == MTK_AFE_I2S_RATE_48K)
> + voice_mode = 3;
This should be a switch statement.
Attachment:
signature.asc
Description: Digital signature
