Message ID | 20210720163732.23003-1-Vijendar.Mukunda@amd.com |
---|---|
Headers | show |
Series | Add Vangogh ACP ASoC driver | expand |
> +static int acp5x_audio_probe(struct platform_device *pdev) > +{ > + struct resource *res; > + struct i2s_dev_data *adata; > + int status; > + > + if (!pdev->dev.platform_data) { > + dev_err(&pdev->dev, "platform_data not retrieved\n"); > + return -ENODEV; > + } > + irqflags = *((unsigned int *)(pdev->dev.platform_data)); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) { > + dev_err(&pdev->dev, "IORESOURCE_MEM FAILED\n"); > + return -ENODEV; indentation is off? > + } > + > + adata = devm_kzalloc(&pdev->dev, sizeof(*adata), GFP_KERNEL); > + if (!adata) > + return -ENOMEM; > + > + adata->acp5x_base = devm_ioremap(&pdev->dev, res->start, > + resource_size(res)); > + if (!adata->acp5x_base) > + return -ENOMEM; > + dev_set_drvdata(&pdev->dev, adata); > + status = devm_snd_soc_register_component(&pdev->dev, > + &acp5x_i2s_component, > + NULL, 0); > + if (status) > + dev_err(&pdev->dev, "Fail to register acp i2s component\n"); > + > + return status; > +}
> diff --git a/sound/soc/amd/vangogh/acp5x.h b/sound/soc/amd/vangogh/acp5x.h > index 2300e63534e7..c94ed8795b9c 100644 > --- a/sound/soc/amd/vangogh/acp5x.h > +++ b/sound/soc/amd/vangogh/acp5x.h > @@ -74,9 +74,20 @@ > #define I2S_MASTER_MODE_ENABLE 0x01 > #define I2S_MASTER_MODE_DISABLE 0x00 > > +#define SLOT_WIDTH_8 0x08 > +#define SLOT_WIDTH_16 0x10 > +#define SLOT_WIDTH_24 0x18 > +#define SLOT_WIDTH_32 0x20 nit-pick: it's not incorrect but is the hex notation necessary? > +#define TDM_ENABLE 1 > +#define TDM_DISABLE 0 > +#define ACP5x_ITER_IRER_SAMP_LEN_MASK 0x38
On 7/20/21 11:37 AM, Vijendar Mukunda wrote: > This adds an ASoC driver for the ACP (Audio CoProcessor) > block on AMD Vangogh APU. Thanks for the update. I added a couple of minor comments on the v4, this looks mostly good now so feel free to add my tag in the following version Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
On 7/20/21 11:57 PM, Pierre-Louis Bossart wrote: > >> +static int acp5x_dma_open(struct snd_soc_component *component, >> + struct snd_pcm_substream *substream) >> +{ >> + struct snd_pcm_runtime *runtime; >> + struct snd_soc_pcm_runtime *prtd; >> + struct i2s_dev_data *adata; >> + struct i2s_stream_instance *i2s_data; >> + int ret; >> + >> + runtime = substream->runtime; >> + prtd = asoc_substream_to_rtd(substream); >> + component = snd_soc_rtdcom_lookup(prtd, DRV_NAME); >> + adata = dev_get_drvdata(component->dev); >> + >> + i2s_data = kzalloc(sizeof(*i2s_data), GFP_KERNEL); >> + if (!i2s_data) >> + return -EINVAL; > > return -ENOMEM? Will fix it. > >> + >> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) >> + runtime->hw = acp5x_pcm_hardware_playback; >> + else >> + runtime->hw = acp5x_pcm_hardware_capture; >> + >> + ret = snd_pcm_hw_constraint_integer(runtime, >> + SNDRV_PCM_HW_PARAM_PERIODS); >> + if (ret < 0) { >> + dev_err(component->dev, "set integer constraint failed\n"); >> + kfree(i2s_data); >> + return ret; >> + } >> + i2s_data->acp5x_base = adata->acp5x_base; >> + runtime->private_data = i2s_data; >> + return ret; >> +} >> + >> +static int acp5x_dma_hw_params(struct snd_soc_component *component, >> + struct snd_pcm_substream *substream, >> + struct snd_pcm_hw_params *params) >> +{ >> + struct i2s_stream_instance *rtd; >> + struct snd_soc_pcm_runtime *prtd; >> + struct snd_soc_card *card; >> + struct acp5x_platform_info *pinfo; >> + struct i2s_dev_data *adata; >> + u64 size; >> + >> + prtd = asoc_substream_to_rtd(substream); >> + card = prtd->card; >> + pinfo = snd_soc_card_get_drvdata(card); >> + adata = dev_get_drvdata(component->dev); >> + rtd = substream->runtime->private_data; >> + >> + if (!rtd) >> + return -EINVAL; >> + >> + if (pinfo) { >> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { >> + rtd->i2s_instance = pinfo->play_i2s_instance; >> + switch (rtd->i2s_instance) { >> + case I2S_HS_INSTANCE: >> + adata->play_stream = substream; >> + break; >> + case I2S_SP_INSTANCE: >> + default: >> + adata->i2ssp_play_stream = substream; >> + } >> + } else { >> + rtd->i2s_instance = pinfo->cap_i2s_instance; >> + switch (rtd->i2s_instance) { >> + case I2S_HS_INSTANCE: >> + adata->capture_stream = substream; >> + break; >> + case I2S_SP_INSTANCE: >> + default: >> + adata->i2ssp_capture_stream = substream; >> + } >> + } >> + } else { >> + dev_err(component->dev, "pinfo failed\n"); > > should you bail here with e.g. return -EINVAL? Will fix it and post the new version. > >> + } >> + size = params_buffer_bytes(params); >> + rtd->dma_addr = substream->dma_buffer.addr; >> + rtd->num_pages = (PAGE_ALIGN(size) >> PAGE_SHIFT); >> + config_acp5x_dma(rtd, substream->stream); >> + return 0; >> +} >