Messages in this thread | | | Subject | Re: [PATCH v6 4/5] ASoC: qcom: Add support for lpass hdmi driver | From | Srinivasa Rao Mandadapu <> | Date | Wed, 23 Sep 2020 14:11:29 +0530 |
| |
Thanks for review Srinivas!!!
On 9/22/2020 2:50 AM, Srinivas Kandagatla wrote: > > > On 18/09/2020 07:04, Srinivasa Rao Mandadapu wrote: >> From: V Sujith Kumar Reddy <vsujithk@codeaurora.org> >> >> Upadate lpass cpu and platform driver to support audio over dp. >> Also add lpass-hdmi.c and lpass-hdmi.h. >> >> Signed-off-by: Srinivasa Rao <srivasam@codeaurora.org> >> Signed-off-by: V Sujith Kumar Reddy <vsujithk@codeaurora.org> >> --- >> sound/soc/qcom/Kconfig | 5 + >> sound/soc/qcom/Makefile | 2 + >> sound/soc/qcom/lpass-apq8016.c | 4 +- >> sound/soc/qcom/lpass-cpu.c | 37 ++- >> sound/soc/qcom/lpass-hdmi.c | 596 >> +++++++++++++++++++++++++++++++++++++++ >> sound/soc/qcom/lpass-hdmi.h | 129 +++++++++ >> sound/soc/qcom/lpass-ipq806x.c | 4 +- >> sound/soc/qcom/lpass-lpaif-reg.h | 52 +++- >> sound/soc/qcom/lpass-platform.c | 466 ++++++++++++++++++++++++------ >> sound/soc/qcom/lpass.h | 119 +++++++- >> 10 files changed, 1294 insertions(+), 120 deletions(-) >> create mode 100644 sound/soc/qcom/lpass-hdmi.c >> create mode 100644 sound/soc/qcom/lpass-hdmi.h >> >> diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig >> index a607ace..509584c 100644 >> --- a/sound/soc/qcom/Kconfig >> +++ b/sound/soc/qcom/Kconfig >> @@ -12,6 +12,10 @@ config SND_SOC_LPASS_CPU >> tristate >> select REGMAP_MMIO >> +config SND_SOC_LPASS_HDMI >> + tristate >> + select REGMAP_MMIO >> + >> config SND_SOC_LPASS_PLATFORM >> tristate >> select REGMAP_MMIO >> @@ -30,6 +34,7 @@ config SND_SOC_LPASS_SC7180 >> tristate >> select SND_SOC_LPASS_CPU >> select SND_SOC_LPASS_PLATFORM >> + select SND_SOC_LPASS_HDMI >> config SND_SOC_STORM >> tristate "ASoC I2S support for Storm boards" >> diff --git a/sound/soc/qcom/Makefile b/sound/soc/qcom/Makefile >> index 7972c94..0bd90d7 100644 >> --- a/sound/soc/qcom/Makefile >> +++ b/sound/soc/qcom/Makefile >> @@ -1,12 +1,14 @@ >> # SPDX-License-Identifier: GPL-2.0 >> # Platform >> snd-soc-lpass-cpu-objs := lpass-cpu.o >> +snd-soc-lpass-hdmi-objs := lpass-hdmi.o >> snd-soc-lpass-platform-objs := lpass-platform.o >> snd-soc-lpass-ipq806x-objs := lpass-ipq806x.o >> snd-soc-lpass-apq8016-objs := lpass-apq8016.o >> snd-soc-lpass-sc7180-objs := lpass-sc7180.o >> obj-$(CONFIG_SND_SOC_LPASS_CPU) += snd-soc-lpass-cpu.o >> +obj-$(CONFIG_SND_SOC_LPASS_HDMI) += snd-soc-lpass-hdmi.o >> obj-$(CONFIG_SND_SOC_LPASS_PLATFORM) += snd-soc-lpass-platform.o >> obj-$(CONFIG_SND_SOC_LPASS_IPQ806X) += snd-soc-lpass-ipq806x.o >> obj-$(CONFIG_SND_SOC_LPASS_APQ8016) += snd-soc-lpass-apq8016.o >> diff --git a/sound/soc/qcom/lpass-apq8016.c >> b/sound/soc/qcom/lpass-apq8016.c >> index 5c8ae22..0aedb3a 100644 >> --- a/sound/soc/qcom/lpass-apq8016.c >> +++ b/sound/soc/qcom/lpass-apq8016.c >> @@ -125,7 +125,7 @@ static struct snd_soc_dai_driver >> apq8016_lpass_cpu_dai_driver[] = { >> }; >> static int apq8016_lpass_alloc_dma_channel(struct lpass_data >> *drvdata, >> - int direction) >> + int direction, unsigned int dai_id) >> { >> struct lpass_variant *v = drvdata->variant; >> int chan = 0; >> @@ -151,7 +151,7 @@ static int apq8016_lpass_alloc_dma_channel(struct >> lpass_data *drvdata, >> return chan; >> } >> -static int apq8016_lpass_free_dma_channel(struct lpass_data >> *drvdata, int chan) >> +static int apq8016_lpass_free_dma_channel(struct lpass_data >> *drvdata, int chan, unsigned int dai_id) >> { >> clear_bit(chan, &drvdata->dma_ch_bit_map); >> diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c >> index 5d84f63..41a2470 100644 >> --- a/sound/soc/qcom/lpass-cpu.c >> +++ b/sound/soc/qcom/lpass-cpu.c >> @@ -589,13 +589,33 @@ int asoc_qcom_lpass_cpu_platform_probe(struct >> platform_device *pdev) >> variant->wrdma_channel_start); >> drvdata->lpaif_map = devm_regmap_init_mmio(dev, drvdata->lpaif, >> - &lpass_cpu_regmap_config); >> + &lpass_cpu_regmap_config); > > change not related to this patch! > > Have you run checkpatch on this patch > >> if (IS_ERR(drvdata->lpaif_map)) { >> dev_err(dev, "error initializing regmap: %ld\n", >> PTR_ERR(drvdata->lpaif_map)); >> return PTR_ERR(drvdata->lpaif_map); >> } >> +#ifdef CONFIG_SND_SOC_LPASS_SC7180 > No, this is not the right way to do it! > We can not have any ifdef with in the code, we should be able to cope > up with finding if hdmiif resources are available or not! > > You should proably look at variant data to see which resources are > required or not! That should work and also get rid of this #ifdef Okay. will remove this. > > > >> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, >> "lpass-hdmiif"); >> + drvdata->hdmiif = devm_ioremap_resource(dev, res); >> + if (IS_ERR((void const __force *)drvdata->hdmiif)) { >> + dev_err(dev, "error mapping reg resource: %ld\n", >> + PTR_ERR((void const __force *)drvdata->hdmiif)); >> + return PTR_ERR((void const __force *)drvdata->hdmiif); >> + } >> + >> + lpass_hdmi_regmap_config.max_register = >> LPAIF_HDMI_RDMAPER_REG(variant, >> + variant->hdmi_rdma_channels); >> + drvdata->hdmiif_map = devm_regmap_init_mmio(dev, drvdata->hdmiif, >> + &lpass_hdmi_regmap_config); >> + if (IS_ERR(drvdata->hdmiif_map)) { >> + dev_err(dev, "error initializing regmap: %ld\n", >> + PTR_ERR(drvdata->hdmiif_map)); >> + return PTR_ERR(drvdata->hdmiif_map); >> + } >> + >> +#endif >> if (variant->init) { >> ret = variant->init(pdev); >> if (ret) { >> @@ -606,6 +626,9 @@ int asoc_qcom_lpass_cpu_platform_probe(struct >> platform_device *pdev) >> for (i = 0; i < variant->num_dai; i++) { >> dai_id = variant->dai_driver[i].id; >> + if (dai_id == LPASS_DP_RX) >> + continue; >> + >> drvdata->mi2s_osr_clk[dai_id] = devm_clk_get(dev, >> variant->dai_osr_clk_names[i]); >> if (IS_ERR(drvdata->mi2s_osr_clk[dai_id])) { >> @@ -625,10 +648,10 @@ int asoc_qcom_lpass_cpu_platform_probe(struct >> platform_device *pdev) >> "error getting %s: %ld\n", >> variant->dai_bit_clk_names[i], >> PTR_ERR(drvdata->mi2s_bit_clk[dai_id])); >> + > > change not related to this patch! Okay. > >> return PTR_ERR(drvdata->mi2s_bit_clk[dai_id]); >> } >> } >> - > > change not related to this patch! Okay. > > >> /* Allocation for i2sctl regmap fields */ >> drvdata->i2sctl = devm_kzalloc(&pdev->dev, sizeof(struct >> lpaif_i2sctl), >> GFP_KERNEL); >> @@ -636,10 +659,18 @@ int asoc_qcom_lpass_cpu_platform_probe(struct >> platform_device *pdev) >> /* Initialize bitfields for dai I2SCTL register */ >> ret = lpass_cpu_init_i2sctl_bitfields(dev, drvdata->i2sctl, >> drvdata->lpaif_map); >> - if (ret) { >> + if (ret) >> dev_err(dev, "error init i2sctl field: %d\n", ret); >> + >> +#ifdef CONFIG_SND_SOC_LPASS_SC7180 >> + ret = lpass_hdmi_init_bitfields(dev, drvdata->hdmiif_map); >> + if (ret) { >> + dev_err(dev, "%s error hdmi init failed\n", __func__); >> return ret; >> } >> +#else >> + return -EINVAL; >> +#endif >> ret = devm_snd_soc_register_component(dev, >> &lpass_cpu_comp_driver, >> diff --git a/sound/soc/qcom/lpass-hdmi.c b/sound/soc/qcom/lpass-hdmi.c >> new file mode 100644 >> index 0000000..7cb2ba3 >> --- /dev/null >> +++ b/sound/soc/qcom/lpass-hdmi.c >> @@ -0,0 +1,596 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (c) 2020 The Linux Foundation. All rights reserved. >> + * >> + * lpass-hdmi.c -- ALSA SoC HDMI-CPU DAI driver for QTi LPASS HDMI >> + */ >> + >> +#include <linux/clk.h> > Do you need this header? > >> +#include <linux/kernel.h> >> +#include <linux/module.h> > <<< >> +#include <linux/of.h> >> +#include <linux/of_device.h> >> +#include <linux/platform_device.h> > >>> > Do you need these headers? Will remove redundant headers. > >> +#include <sound/pcm.h> >> +#include <sound/pcm_params.h> >> +#include <linux/regmap.h> >> +#include <sound/soc.h> >> +#include <sound/soc-dai.h> >> +#include <dt-bindings/sound/sc7180-lpass.h> >> +#include "lpass-lpaif-reg.h" >> +#include "lpass.h" >> +#include "lpass-hdmi.h" >> + >> +int lpass_hdmi_init_bitfields(struct device *dev, struct regmap *map) >> +{ >> + struct lpass_data *drvdata = dev_get_drvdata(dev); >> + struct lpass_variant *v = drvdata->variant; >> + unsigned int i; >> + struct lpass_hdmi_tx_ctl *tx_ctl; >> + struct lpass_hdmitx_legacy *legacy; >> + struct lpass_vbit_ctrl *vbit_ctl; >> + struct lpass_hdmi_tx_parity *tx_parity; >> + struct lpass_dp_metadata_ctl *meta_ctl; >> + struct lpass_sstream_ctl *sstream_ctl; >> + struct lpass_hdmi_tx_ch_msb *ch_msb; >> + struct lpass_hdmi_tx_ch_lsb *ch_lsb; >> + struct lpass_hdmitx_dmactl *hdmi_tx_dmactl; >> + >> + drvdata->tx_ctl = devm_kzalloc(dev, sizeof(struct >> lpass_hdmi_tx_ctl), >> + GFP_KERNEL); >> + if (drvdata->tx_ctl == NULL) >> + return -ENOMEM; >> + >> + tx_ctl = drvdata->tx_ctl; >> + tx_ctl->soft_reset = devm_regmap_field_alloc(dev, map, >> + v->soft_reset); >> + tx_ctl->force_reset = devm_regmap_field_alloc(dev, map, >> + v->force_reset); >> + if (IS_ERR(tx_ctl->soft_reset) || IS_ERR(tx_ctl->force_reset)) >> + return -EINVAL; >> + >> + drvdata->legacy = devm_kzalloc(dev, sizeof(struct >> lpass_hdmitx_legacy), > > consider doing something like this to save uncessary long lines here! > which makes code unreadable. okay. > > > legacy = devm_kzalloc(dev, sizeof(*legacy), GFP_KERNEL); > if (!legacy) > return -ENOMEM > > drvdata->legacy = legacy; > > > use sizeof(*legacy) and same comment to other parts of the code! > > >> + GFP_KERNEL); >> + if (drvdata->legacy == NULL) >> + return -ENOMEM; >> + >> + legacy = drvdata->legacy; >> + legacy->legacy_en = devm_regmap_field_alloc(dev, map, >> + v->legacy_en); >> + if (IS_ERR(legacy->legacy_en)) >> + return -EINVAL; >> + >> + drvdata->vbit_ctl = devm_kzalloc(dev, sizeof(struct >> lpass_vbit_ctrl), >> + GFP_KERNEL); >> + if (drvdata->vbit_ctl == NULL) >> + return -ENOMEM; >> + >> + vbit_ctl = drvdata->vbit_ctl; >> + vbit_ctl->replace_vbit = devm_regmap_field_alloc(dev, map, >> + v->replace_vbit); >> + vbit_ctl->vbit_stream = devm_regmap_field_alloc(dev, map, >> + v->vbit_stream); >> + if (IS_ERR(vbit_ctl->replace_vbit) || >> IS_ERR(vbit_ctl->vbit_stream)) >> + return -EINVAL; >> + >> + drvdata->tx_parity = devm_kzalloc(dev, >> + sizeof(struct lpass_hdmi_tx_parity), GFP_KERNEL); >> + >> + if (drvdata->tx_parity == NULL) >> + return -ENOMEM; >> + >> + tx_parity = drvdata->tx_parity; >> + tx_parity->calc_en = devm_regmap_field_alloc(dev, map, v->calc_en); >> + >> + if (IS_ERR(tx_parity->calc_en)) >> + return -EINVAL; >> + >> + drvdata->meta_ctl = devm_kzalloc(dev, >> + sizeof(struct lpass_dp_metadata_ctl), GFP_KERNEL); >> + >> + if (drvdata->meta_ctl == NULL) >> + return -ENOMEM; >> + >> + meta_ctl = drvdata->meta_ctl; >> + meta_ctl->mute = devm_regmap_field_alloc(dev, map, v->mute); >> + meta_ctl->as_sdp_cc = devm_regmap_field_alloc(dev, map, >> + v->as_sdp_cc); >> + meta_ctl->as_sdp_ct = devm_regmap_field_alloc(dev, map, >> + v->as_sdp_ct); >> + meta_ctl->aif_db4 = devm_regmap_field_alloc(dev, map, >> + v->aif_db4); >> + meta_ctl->frequency = devm_regmap_field_alloc(dev, map, >> + v->frequency); >> + meta_ctl->mst_index = devm_regmap_field_alloc(dev, map, >> + v->mst_index); > <<<<< >> + meta_ctl->dptx_index = devm_regmap_field_alloc(dev, map, >> + v->dptx_index); >> + >> + if (IS_ERR(meta_ctl->mute) || IS_ERR(meta_ctl->as_sdp_cc) || >> + IS_ERR(meta_ctl->as_sdp_ct) || IS_ERR(meta_ctl->aif_db4) || >> + IS_ERR(meta_ctl->frequency) || IS_ERR(meta_ctl->mst_index) || >> + IS_ERR(meta_ctl->dptx_index)) >> + return -EINVAL; > >>>>> > > These type of code is allover the place, why not just add a simple > macro to do this. > > something like > > #define QCOM_REGMAP_FILED_ALLOC(d, m, f) \ > do { \ > f = devm_regmap_field_alloc(d, m, f); \ > if (IS_ERR(f)) \ > return -EINVAL; \ > } while (0) > > and call > QCOM_REGMAP_FILED_ALLOC(dev, map, meta_ctl->dptx_index); > > this will make code much cleaner IMO. Okay. > > Also we can consider adding some bluk apis to allocate regmap field > or something > like!https://git.linaro.org/people/srinivas.kandagatla/linux.git/commit/?h=wip/regmap&id=03551a6e2941d71b99e2e4a5035a9de5f0df6d5d > > and call > devm_regmap_field_bulk_alloc(dev, map, &sstream_ctl->sstream_en, > &v->sstream_en, 9); > > this should work as long as order of reg_field and regmap_field are > maintained, which in this case is true! > Okay. > >> + >> + drvdata->sstream_ctl = devm_kzalloc(dev, >> + sizeof(struct lpass_sstream_ctl), GFP_KERNEL); >> + if (drvdata->sstream_ctl == NULL) >> + return -ENOMEM; >> + >> + sstream_ctl = drvdata->sstream_ctl; >> + sstream_ctl->sstream_en = devm_regmap_field_alloc(dev, map, >> + v->sstream_en); >> + sstream_ctl->dma_sel = devm_regmap_field_alloc(dev, map, >> + v->dma_sel); >> + sstream_ctl->auto_bbit_en = devm_regmap_field_alloc(dev, map, >> + v->auto_bbit_en); >> + sstream_ctl->layout = devm_regmap_field_alloc(dev, map, >> + v->layout); >> + sstream_ctl->layout_sp = devm_regmap_field_alloc(dev, map, >> + v->layout_sp); >> + sstream_ctl->dp_audio = devm_regmap_field_alloc(dev, map, >> + v->dp_audio); >> + sstream_ctl->set_sp_on_en = devm_regmap_field_alloc(dev, map, >> + v->set_sp_on_en); >> + sstream_ctl->dp_staffing_en = devm_regmap_field_alloc(dev, map, >> + v->dp_staffing_en); >> + sstream_ctl->dp_sp_b_hw_en = devm_regmap_field_alloc(dev, map, >> + v->dp_sp_b_hw_en); >> + >> + if (IS_ERR(sstream_ctl->sstream_en) || >> IS_ERR(sstream_ctl->dma_sel) || >> + IS_ERR(sstream_ctl->auto_bbit_en) || >> + IS_ERR(sstream_ctl->layout) || >> IS_ERR(sstream_ctl->layout_sp) || >> + IS_ERR(sstream_ctl->dp_audio) || >> + IS_ERR(sstream_ctl->set_sp_on_en) || >> + IS_ERR(sstream_ctl->dp_staffing_en) || >> + IS_ERR(sstream_ctl->dp_sp_b_hw_en)) >> + return -EINVAL; >> + >> + for (i = 0; i < LPASS_MAX_HDMI_DMA_CHANNELS; i++) { >> + drvdata->ch_msb[i] = devm_kzalloc(dev, >> + sizeof(struct lpass_hdmi_tx_ch_msb), GFP_KERNEL); >> + if (drvdata->ch_msb[i] == NULL) >> + return -ENOMEM; >> + >> + ch_msb = drvdata->ch_msb[i]; >> + >> + ch_msb->msb_bits = devm_regmap_field_alloc(dev, map, >> + v->msb_bits); >> + if (IS_ERR(ch_msb->msb_bits)) >> + return -EINVAL; >> + >> + drvdata->ch_lsb[i] = devm_kzalloc(dev, >> + sizeof(struct lpass_hdmi_tx_ch_lsb), GFP_KERNEL); >> + if (drvdata->ch_lsb[i] == NULL) >> + return -ENOMEM; >> + >> + ch_lsb = drvdata->ch_lsb[i]; >> + ch_lsb->lsb_bits = devm_regmap_field_alloc(dev, map, >> + v->lsb_bits); >> + if (IS_ERR(ch_lsb->lsb_bits)) >> + return -EINVAL; >> + >> + >> + drvdata->hdmi_tx_dmactl[i] = devm_kzalloc(dev, >> + sizeof(struct lpass_hdmitx_dmactl), GFP_KERNEL); >> + if (drvdata->hdmi_tx_dmactl[i] == NULL) >> + return -ENOMEM; >> + >> + hdmi_tx_dmactl = drvdata->hdmi_tx_dmactl[i]; >> + hdmi_tx_dmactl->use_hw_chs = devm_regmap_field_alloc(dev, map, >> + v->use_hw_chs); >> + hdmi_tx_dmactl->use_hw_usr = devm_regmap_field_alloc(dev, map, >> + v->use_hw_usr); >> + hdmi_tx_dmactl->hw_chs_sel = devm_regmap_field_alloc(dev, map, >> + v->hw_chs_sel); >> + hdmi_tx_dmactl->hw_usr_sel = devm_regmap_field_alloc(dev, map, >> + v->hw_usr_sel); >> + if (IS_ERR(hdmi_tx_dmactl->use_hw_chs) || >> + IS_ERR(hdmi_tx_dmactl->use_hw_usr) || >> + IS_ERR(hdmi_tx_dmactl->hw_chs_sel) || >> + IS_ERR(hdmi_tx_dmactl->hw_usr_sel)) >> + return -EINVAL; >> + } >> + return 0; >> + >> +} >> +EXPORT_SYMBOL(lpass_hdmi_init_bitfields); >> + >> +static int lpass_hdmi_daiops_hw_params(struct snd_pcm_substream >> *substream, >> + struct snd_pcm_hw_params *params, struct snd_soc_dai *dai) >> +{ >> + struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai); >> + snd_pcm_format_t format = params_format(params); >> + unsigned int rate = params_rate(params); >> + unsigned int channels = params_channels(params); >> + unsigned int ret; >> + unsigned int bitwidth; >> + unsigned int word_length; >> + unsigned int ch_sts_buf0; >> + unsigned int ch_sts_buf1; >> + unsigned int data_format; >> + unsigned int sampling_freq; >> + unsigned int ch = 0; >> + >> + bitwidth = snd_pcm_format_width(format); >> + if (bitwidth < 0) { >> + dev_err(dai->dev, "%s invalid bit width given : %d\n", >> + __func__, bitwidth); >> + return bitwidth; >> + } >> + >> + switch (bitwidth) { >> + case 16: >> + word_length = LPASS_DP_AUDIO_BITWIDTH16; >> + break; >> + case 24: >> + word_length = LPASS_DP_AUDIO_BITWIDTH24; >> + break; >> + default: >> + dev_err(dai->dev, "%s invalid bit width given : %d\n", >> + __func__, bitwidth); >> + return -EINVAL; >> + } >> + >> + switch (rate) { >> + case 32000: >> + sampling_freq = LPASS_SAMPLING_FREQ32; >> + break; >> + case 44100: >> + sampling_freq = LPASS_SAMPLING_FREQ44; >> + break; >> + case 48000: >> + sampling_freq = LPASS_SAMPLING_FREQ48; >> + break; >> + >> + default: >> + dev_err(dai->dev, "%s invalid bit width given : %d\n", >> + __func__, bitwidth); >> + return -EINVAL; >> + } >> + data_format = LPASS_DATA_FORMAT_LINEAR; >> + ch_sts_buf0 = (((data_format << LPASS_DATA_FORMAT_SHIFT) & >> LPASS_DATA_FORMAT_MASK) >> + | ((sampling_freq << LPASS_FREQ_BIT_SHIFT) & >> LPASS_FREQ_BIT_MASK)); >> + ch_sts_buf1 = (word_length) & LPASS_WORDLENGTH_MASK; >> + >> + ret = regmap_field_write(drvdata->tx_ctl->soft_reset, >> LPASS_TX_CTL_RESET); >> + if (ret) >> + return ret; >> + >> + ret = regmap_field_write(drvdata->tx_ctl->soft_reset, >> LPASS_TX_CTL_CLEAR); >> + if (ret) >> + return ret; >> + >> + ret = regmap_field_write(drvdata->legacy->legacy_en, >> + LPASS_HDMITX_LEGACY_DISABLE); >> + if (ret) >> + return ret; >> + >> + ret = regmap_field_write(drvdata->tx_parity->calc_en, >> + HDMITX_PARITY_CALC_EN); >> + if (ret) >> + return ret; >> + >> + ret = regmap_field_write(drvdata->vbit_ctl->replace_vbit, >> + REPLACE_VBIT); >> + if (ret) >> + >> + return ret; >> + >> + ret = regmap_field_write(drvdata->vbit_ctl->vbit_stream, >> + LINEAR_PCM_DATA); >> + if (ret) >> + >> + return ret; >> + >> + ret = regmap_field_write(drvdata->ch_msb[0]->msb_bits, >> ch_sts_buf1); >> + if (ret) >> + >> + return ret; >> + >> + ret = regmap_field_write(drvdata->ch_lsb[0]->lsb_bits, >> ch_sts_buf0); >> + if (ret) >> + >> + return ret; >> + >> + ret = regmap_field_write(drvdata->hdmi_tx_dmactl[0]->use_hw_chs, >> + HW_MODE); >> + if (ret) >> + >> + return ret; >> + >> + ret = regmap_field_write(drvdata->hdmi_tx_dmactl[0]->hw_chs_sel, >> + SW_MODE); >> + if (ret) >> + >> + return ret; >> + >> + ret = regmap_field_write(drvdata->hdmi_tx_dmactl[0]->use_hw_usr, >> + HW_MODE); >> + if (ret) >> + >> + return ret; >> + >> + ret = regmap_field_write(drvdata->hdmi_tx_dmactl[0]->hw_usr_sel, >> + SW_MODE); >> + if (ret) >> + >> + return ret; >> + >> + ret = regmap_field_write(drvdata->meta_ctl->mute, >> + LPASS_MUTE_ENABLE); >> + if (ret) >> + return ret; >> + >> + ret = regmap_field_write(drvdata->meta_ctl->as_sdp_cc, >> + channels - 1); >> + if (ret) >> + return ret; >> + >> + >> + ret = regmap_field_write(drvdata->meta_ctl->as_sdp_ct, >> + LPASS_META_DEFAULT_VAL); >> + if (ret) >> + return ret; >> + >> + ret = regmap_field_write(drvdata->meta_ctl->aif_db4, >> + LPASS_META_DEFAULT_VAL); >> + if (ret) >> + return ret; >> + >> + ret = regmap_field_write(drvdata->meta_ctl->frequency, >> sampling_freq); >> + if (ret) >> + return ret; >> + >> + ret = regmap_field_write(drvdata->meta_ctl->mst_index, >> + LPASS_META_DEFAULT_VAL); >> + if (ret) >> + return ret; >> + >> + ret = regmap_field_write(drvdata->meta_ctl->dptx_index, >> + LPASS_META_DEFAULT_VAL); >> + if (ret) >> + return ret; >> + >> + ret = regmap_field_write(drvdata->sstream_ctl->sstream_en, >> + LPASS_SSTREAM_DISABLE); >> + if (ret) >> + return ret; >> + >> + ret = regmap_field_write(drvdata->sstream_ctl->dma_sel, ch); >> + if (ret) >> + return ret; >> + >> + >> + ret = regmap_field_write(drvdata->sstream_ctl->auto_bbit_en, >> + LPASS_SSTREAM_DEFAULT_ENABLE); >> + if (ret) >> + return ret; >> + >> + >> + ret = regmap_field_write(drvdata->sstream_ctl->layout, >> + LPASS_SSTREAM_DEFAULT_DISABLE); >> + if (ret) >> + return ret; >> + >> + ret = regmap_field_write(drvdata->sstream_ctl->layout_sp, >> + LPASS_LAYOUT_SP_DEFAULT); >> + if (ret) >> + return ret; >> + >> + ret = regmap_field_write(drvdata->sstream_ctl->dp_audio, >> + LPASS_SSTREAM_DEFAULT_ENABLE); >> + if (ret) >> + return ret; >> + >> + ret = regmap_field_write(drvdata->sstream_ctl->set_sp_on_en, >> + LPASS_SSTREAM_DEFAULT_ENABLE); >> + if (ret) >> + return ret; >> + >> + ret = regmap_field_write(drvdata->sstream_ctl->dp_sp_b_hw_en, >> + LPASS_SSTREAM_DEFAULT_ENABLE); >> + if (ret) >> + return ret; >> + >> + ret = regmap_field_write(drvdata->sstream_ctl->dp_staffing_en, >> + LPASS_SSTREAM_DEFAULT_ENABLE); >> + if (ret) >> + >> + return ret; >> + >> + return ret; >> +} >> + >> + >> + > too many empty lines ! Okay. Will remove empty lines! > >> +static int lpass_hdmi_daiops_prepare(struct snd_pcm_substream >> *substream, >> + struct snd_soc_dai *dai) >> +{ >> + struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai); >> + int ret; >> + >> + ret = regmap_field_write(drvdata->sstream_ctl->sstream_en, >> + LPASS_SSTREAM_ENABLE); >> + if (ret) >> + return ret; >> + >> + ret = regmap_field_write(drvdata->meta_ctl->mute, >> + LPASS_MUTE_DISABLE); >> + if (ret) >> + return ret; >> + >> + return ret; >> +} >> + >> +static int lpass_hdmi_daiops_trigger(struct snd_pcm_substream >> *substream, >> + int cmd, struct snd_soc_dai *dai) >> +{ >> + struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai); >> + int ret = -EINVAL; >> + >> + switch (cmd) { >> + case SNDRV_PCM_TRIGGER_START: >> + case SNDRV_PCM_TRIGGER_RESUME: >> + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: >> + > unnecessary space! Okay. >> + ret = regmap_field_write(drvdata->sstream_ctl->sstream_en, >> + LPASS_SSTREAM_ENABLE); >> + if (ret) >> + return ret; >> + >> + ret = regmap_field_write(drvdata->meta_ctl->mute, >> + LPASS_MUTE_DISABLE); >> + if (ret) >> + return ret; >> + >> + break; >> + case SNDRV_PCM_TRIGGER_STOP: >> + case SNDRV_PCM_TRIGGER_SUSPEND: >> + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: >> + >> + ret = regmap_field_write(drvdata->sstream_ctl->sstream_en, >> + LPASS_SSTREAM_DISABLE); >> + if (ret) >> + return ret; >> + >> + >> + ret = regmap_field_write(drvdata->meta_ctl->mute, >> + LPASS_MUTE_ENABLE); >> + if (ret) >> + return ret; >> + >> + ret = regmap_field_write(drvdata->sstream_ctl->dp_audio, 0); >> + if (ret) >> + return ret; >> + >> + break; >> + } >> + return ret; >> +} >> + >> +const struct snd_soc_dai_ops asoc_qcom_lpass_hdmi_dai_ops = { >> + .hw_params = lpass_hdmi_daiops_hw_params, >> + .prepare = lpass_hdmi_daiops_prepare, >> + .trigger = lpass_hdmi_daiops_trigger, >> +}; >> +EXPORT_SYMBOL_GPL(asoc_qcom_lpass_hdmi_dai_ops); >> + >> + >> + > too many empty lines?? Okay. > > >> +static bool lpass_hdmi_regmap_writeable(struct device *dev, unsigned >> int reg) >> +{ >> + struct lpass_data *drvdata = dev_get_drvdata(dev); >> + struct lpass_variant *v = drvdata->variant; >> + int i; >> + >> + if (reg == LPASS_HDMI_TX_CTL_ADDR(v)) >> + return true; >> + if (reg == LPASS_HDMI_TX_LEGACY_ADDR(v)) >> + return true; >> + if (reg == LPASS_HDMI_TX_VBIT_CTL_ADDR(v)) >> + return true; >> + > switch case should make this much cleaner. Okay. > >> + for (i = 0; i < v->hdmi_rdma_channels; i++) { >> + if (reg == LPASS_HDMI_TX_CH_LSB_ADDR(v, i)) >> + return true; >> + if (reg == LPASS_HDMI_TX_CH_MSB_ADDR(v, i)) >> + return true; >> + if (reg == LPASS_HDMI_TX_DMA_ADDR(v, i)) >> + return true; >> + } >> + >> + if (reg == LPASS_HDMI_TX_PARITY_ADDR(v)) >> + return true; >> + if (reg == LPASS_HDMI_TX_DP_ADDR(v)) >> + return true; >> + if (reg == LPASS_HDMI_TX_SSTREAM_ADDR(v)) >> + return true; >> + >> + if (reg == LPASS_HDMITX_APP_IRQEN_REG(v)) >> + return true; >> + if (reg == LPASS_HDMITX_APP_IRQCLEAR_REG(v)) >> + return true; >> + >> + for (i = 0; i < v->rdma_channels; ++i) { >> + if (reg == LPAIF_HDMI_RDMACTL_REG(v, i)) >> + return true; >> + if (reg == LPAIF_HDMI_RDMABASE_REG(v, i)) >> + return true; >> + if (reg == LPAIF_HDMI_RDMABUFF_REG(v, i)) >> + return true; >> + if (reg == LPAIF_HDMI_RDMAPER_REG(v, i)) >> + return true; >> + >> + } >> + return false; >> +} >> + >> +static bool lpass_hdmi_regmap_readable(struct device *dev, unsigned >> int reg) >> +{ >> + struct lpass_data *drvdata = dev_get_drvdata(dev); >> + struct lpass_variant *v = drvdata->variant; >> + int i; >> + >> + if (reg == LPASS_HDMI_TX_CTL_ADDR(v)) >> + return true; >> + if (reg == LPASS_HDMI_TX_LEGACY_ADDR(v)) >> + return true; >> + if (reg == LPASS_HDMI_TX_VBIT_CTL_ADDR(v)) >> + return true; > > Same here, use switch case Okay. >> + >> + for (i = 0; i < v->hdmi_rdma_channels; i++) { >> + if (reg == LPASS_HDMI_TX_CH_LSB_ADDR(v, i)) >> + return true; >> + if (reg == LPASS_HDMI_TX_CH_MSB_ADDR(v, i)) >> + return true; >> + if (reg == LPASS_HDMI_TX_DMA_ADDR(v, i)) >> + return true; >> + } > >> + >> + if (reg == LPASS_HDMI_TX_PARITY_ADDR(v)) >> + return true; >> + if (reg == LPASS_HDMI_TX_DP_ADDR(v)) >> + return true; >> + if (reg == LPASS_HDMI_TX_SSTREAM_ADDR(v)) >> + return true; >> + >> + if (reg == LPASS_HDMITX_APP_IRQEN_REG(v)) >> + return true; >> + if (reg == LPASS_HDMITX_APP_IRQSTAT_REG(v)) >> + return true; >> + >> + for (i = 0; i < v->rdma_channels; ++i) { >> + if (reg == LPAIF_HDMI_RDMACTL_REG(v, i)) >> + return true; >> + if (reg == LPAIF_HDMI_RDMABASE_REG(v, i)) >> + return true; >> + if (reg == LPAIF_HDMI_RDMABUFF_REG(v, i)) >> + return true; >> + if (reg == LPAIF_HDMI_RDMAPER_REG(v, i)) >> + return true; >> + if (reg == LPAIF_HDMI_RDMACURR_REG(v, i)) >> + return true; >> + } >> + >> + return false; >> +} >> + >> +static bool lpass_hdmi_regmap_volatile(struct device *dev, unsigned >> int reg) >> +{ >> + return true; > > Are all the register range really volatile? will check this. > >> + >> +} >> +struct regmap_config lpass_hdmi_regmap_config = { >> + .reg_bits = 32, >> + .reg_stride = 4, >> + .val_bits = 32, >> + .writeable_reg = lpass_hdmi_regmap_writeable, >> + .readable_reg = lpass_hdmi_regmap_readable, >> + .volatile_reg = lpass_hdmi_regmap_volatile, >> + .cache_type = REGCACHE_FLAT, > > Any reason for using FLAT cache type ?? Will check this. > >> +}; >> +EXPORT_SYMBOL(lpass_hdmi_regmap_config); >> + >> +MODULE_DESCRIPTION("QTi LPASS HDMI Driver"); >> +MODULE_LICENSE("GPL v2"); >> diff --git a/sound/soc/qcom/lpass-hdmi.h b/sound/soc/qcom/lpass-hdmi.h >> new file mode 100644 >> index 0000000..f91f322 >> --- /dev/null >> +++ b/sound/soc/qcom/lpass-hdmi.h >> @@ -0,0 +1,129 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * Copyright (c) 2020 The Linux Foundation. All rights reserved. >> + * >> + * lpass_hdmi.h - Definitions for the QTi LPASS HDMI >> + */ >> + >> +#ifndef __LPASS_HDMI_H__ >> +#define __LPASS_HDMI_H__ >> + >> +#include <linux/clk.h> >> +#include <linux/compiler.h> >> +#include <linux/platform_device.h> >> +#include <linux/regmap.h> >> + >> +#define LPASS_HDMITX_LEGACY_DISABLE 0x0 >> +#define LPASS_HDMITX_LEGACY_ENABLE 0x1 >> +#define LPASS_DP_AUDIO_BITWIDTH16 0x0 >> +#define LPASS_DP_AUDIO_BITWIDTH24 0xb >> +#define LPASS_DATA_FORMAT_SHIFT 0x1 >> +#define LPASS_DATA_FORMAT_MASK 0x2 >> +#define LPASS_FREQ_BIT_SHIFT 24 >> +#define LPASS_FREQ_BIT_MASK 0xf000000 > > use GENMASK macro for all such masks. Okay. > >> +#define LPASS_DATA_FORMAT_LINEAR 0x0 >> +#define LPASS_DATA_FORMAT_NON_LINEAR 0x1 >> +#define LPASS_SAMPLING_FREQ32 0x3 >> +#define LPASS_SAMPLING_FREQ44 0x0 >> +#define LPASS_SAMPLING_FREQ48 0x2 >> +#define LPASS_TX_CTL_RESET 0x1 >> +#define LPASS_TX_CTL_CLEAR 0x0 >> +#define LPASS_SSTREAM_ENABLE 1 >> +#define LPASS_SSTREAM_DISABLE 0 >> +#define LPASS_LAYOUT_SP_DEFAULT 0xf >> +#define LPASS_SSTREAM_DEFAULT_ENABLE 1 >> +#define LPASS_SSTREAM_DEFAULT_DISABLE 0 >> +#define LPASS_WORDLENGTH_MASK 0xf >> +#define LPASS_MUTE_ENABLE 1 >> +#define LPASS_MUTE_DISABLE 0 >> +#define LPASS_META_DEFAULT_VAL 0 >> +#define HW_MODE 1 >> +#define SW_MODE 0 >> +#define LEGACY_LPASS_LPAIF 1 >> +#define LEGACY_LPASS_HDMI 0 >> +#define REPLACE_VBIT 0x1 >> +#define LINEAR_PCM_DATA 0x0 >> +#define NON_LINEAR_PCM_DATA 0x1 >> +#define HDMITX_PARITY_CALC_EN 0x1 >> +#define HDMITX_PARITY_CALC_DIS 0x0 >> + >> + >> +#define LPASS_HDMI_TX_CTL_ADDR(v) \ >> + (v->hdmi_tx_ctl_addr) >> + >> +#define LPASS_HDMI_TX_LEGACY_ADDR(v) \ >> + (v->hdmi_legacy_addr) >> +#define LPASS_HDMI_TX_VBIT_CTL_ADDR(v) \ >> + (v->hdmi_vbit_addr) > above three macros should be able to go in single lines!! Not sure why > you are adding an extra break, which makes it very odd for reading. Okay. > >> +#define LPASS_HDMI_TX_CH_LSB_ADDR(v, port) \ >> + (v->hdmi_ch_lsb_addr + \ >> + v->ch_stride * (port)) >> +#define LPASS_HDMI_TX_CH_MSB_ADDR(v, port) \ >> + (v->hdmi_ch_msb_addr + \ >> + v->ch_stride * (port)) >> +#define LPASS_HDMI_TX_DMA_ADDR(v, port) \ >> + (v->hdmi_dmactl_addr + \ >> + v->hdmi_dma_stride * (port)) >> +#define LPASS_HDMI_TX_PARITY_ADDR(v) \ >> + (v->hdmi_parity_addr) >> +#define LPASS_HDMI_TX_DP_ADDR(v) \ >> + (v->hdmi_DP_addr) >> +#define LPASS_HDMI_TX_SSTREAM_ADDR(v) \ >> + (v->hdmi_sstream_addr) >> + >> +struct lpass_sstream_ctl { >> + struct regmap_field *sstream_en; >> + struct regmap_field *dma_sel; >> + struct regmap_field *auto_bbit_en; >> + struct regmap_field *layout; >> + struct regmap_field *layout_sp; >> + struct regmap_field *set_sp_on_en; >> + struct regmap_field *dp_audio; >> + struct regmap_field *dp_staffing_en; >> + struct regmap_field *dp_sp_b_hw_en; >> +}; >> + >> +struct lpass_dp_metadata_ctl { >> + struct regmap_field *mute; >> + struct regmap_field *as_sdp_cc; >> + struct regmap_field *as_sdp_ct; >> + struct regmap_field *aif_db4; >> + struct regmap_field *frequency; >> + struct regmap_field *mst_index; >> + struct regmap_field *dptx_index; >> +}; >> + >> +struct lpass_hdmi_tx_ctl { >> + struct regmap_field *soft_reset; >> + struct regmap_field *force_reset; >> +}; >> + >> +struct lpass_hdmitx_dmactl { >> + struct regmap_field *use_hw_chs; >> + struct regmap_field *use_hw_usr; >> + struct regmap_field *hw_chs_sel; >> + struct regmap_field *hw_usr_sel; >> +}; >> + >> +struct lpass_vbit_ctrl { >> + struct regmap_field *replace_vbit; >> + struct regmap_field *vbit_stream; >> +}; >> + >> +struct lpass_hdmitx_legacy { >> + struct regmap_field *legacy_en; >> +}; >> + >> +struct lpass_hdmi_tx_parity { >> + struct regmap_field *calc_en; >> +}; >> + >> +struct lpass_hdmi_tx_ch_lsb { >> + struct regmap_field *lsb_bits; >> +}; >> + >> +struct lpass_hdmi_tx_ch_msb { >> + struct regmap_field *msb_bits; >> +}; >> + >> +#endif /* __LPASS_HDMI_H__ */ >> diff --git a/sound/soc/qcom/lpass-ipq806x.c >> b/sound/soc/qcom/lpass-ipq806x.c >> index 72f09b3..832a916 100644 >> --- a/sound/soc/qcom/lpass-ipq806x.c >> +++ b/sound/soc/qcom/lpass-ipq806x.c >> @@ -96,7 +96,7 @@ static int ipq806x_lpass_exit(struct >> platform_device *pdev) >> return 0; >> } >> -static int ipq806x_lpass_alloc_dma_channel(struct lpass_data >> *drvdata, int dir) >> +static int ipq806x_lpass_alloc_dma_channel(struct lpass_data >> *drvdata, int dir, unsigned int dai_id) >> { >> if (dir == SNDRV_PCM_STREAM_PLAYBACK) >> return IPQ806X_LPAIF_RDMA_CHAN_MI2S; >> @@ -104,7 +104,7 @@ static int ipq806x_lpass_alloc_dma_channel(struct >> lpass_data *drvdata, int dir) >> return -EINVAL; >> } >> -static int ipq806x_lpass_free_dma_channel(struct lpass_data >> *drvdata, int chan) >> +static int ipq806x_lpass_free_dma_channel(struct lpass_data >> *drvdata, int chan, unsigned int dai_id) >> { >> return 0; >> } >> diff --git a/sound/soc/qcom/lpass-lpaif-reg.h >> b/sound/soc/qcom/lpass-lpaif-reg.h >> index 5258e60..42aebd4 100644 >> --- a/sound/soc/qcom/lpass-lpaif-reg.h >> +++ b/sound/soc/qcom/lpass-lpaif-reg.h >> @@ -9,7 +9,8 @@ >> /* LPAIF I2S */ >> #define LPAIF_I2SCTL_REG_ADDR(v, addr, port) \ >> - (v->i2sctrl_reg_base + (addr) + v->i2sctrl_reg_stride * (port)) >> + (v->i2sctrl_reg_base + (addr) + \ >> + v->i2sctrl_reg_stride * (port)) >> #define LPAIF_I2SCTL_REG(v, port) LPAIF_I2SCTL_REG_ADDR(v, 0x0, >> (port)) >> @@ -70,6 +71,14 @@ >> #define LPAIF_IRQSTAT_REG(v, port) LPAIF_IRQ_REG_ADDR(v, 0x4, >> (port)) >> #define LPAIF_IRQCLEAR_REG(v, port) LPAIF_IRQ_REG_ADDR(v, 0xC, >> (port)) >> + >> +#define LPASS_HDMITX_APP_IRQ_REG_ADDR(v, addr) \ >> + ((v->hdmi_irq_reg_base) + (addr)) >> + >> +#define LPASS_HDMITX_APP_IRQEN_REG(v) >> LPASS_HDMITX_APP_IRQ_REG_ADDR(v, 0x4) >> +#define LPASS_HDMITX_APP_IRQSTAT_REG(v) >> LPASS_HDMITX_APP_IRQ_REG_ADDR(v, 0x8) >> +#define LPASS_HDMITX_APP_IRQCLEAR_REG(v) >> LPASS_HDMITX_APP_IRQ_REG_ADDR(v, 0xC) >> + >> #define LPAIF_IRQ_BITSTRIDE 3 >> #define LPAIF_IRQ_PER(chan) (1 << (LPAIF_IRQ_BITSTRIDE * >> (chan))) >> @@ -77,8 +86,22 @@ >> #define LPAIF_IRQ_ERR(chan) (4 << (LPAIF_IRQ_BITSTRIDE * >> (chan))) >> #define LPAIF_IRQ_ALL(chan) (7 << (LPAIF_IRQ_BITSTRIDE * >> (chan))) >> +#define LPAIF_IRQ_HDMI_REQ_ON_PRELOAD(chan) (1 << (14 + chan)) >> +#define LPAIF_IRQ_HDMI_SDEEP_AUD_DIS(chan) (1 << (24 + chan)) >> +#define LPAIF_IRQ_HDMI_METADONE BIT(23) >> /* LPAIF DMA */ >> +#define LPAIF_HDMI_RDMA_REG_ADDR(v, addr, chan) \ >> + (v->hdmi_rdma_reg_base + (addr) + v->hdmi_rdma_reg_stride * (chan)) >> + >> +#define LPAIF_HDMI_RDMACTL_AUDINTF(id) (id << >> LPAIF_RDMACTL_AUDINTF_SHIFT) >> + >> +#define LPAIF_HDMI_RDMACTL_REG(v, chan) LPAIF_HDMI_RDMA_REG_ADDR(v, >> 0x00, (chan)) >> +#define LPAIF_HDMI_RDMABASE_REG(v, chan) LPAIF_HDMI_RDMA_REG_ADDR(v, >> 0x04, (chan)) >> +#define LPAIF_HDMI_RDMABUFF_REG(v, chan) >> LPAIF_HDMI_RDMA_REG_ADDR(v, 0x08, (chan)) >> +#define LPAIF_HDMI_RDMACURR_REG(v, chan) LPAIF_HDMI_RDMA_REG_ADDR(v, >> 0x0C, (chan)) >> +#define LPAIF_HDMI_RDMAPER_REG(v, chan) >> LPAIF_HDMI_RDMA_REG_ADDR(v, 0x10, (chan)) >> +#define LPAIF_HDMI_RDMAPERCNT_REG(v, chan) >> LPAIF_HDMI_RDMA_REG_ADDR(v, 0x14, (chan)) >> #define LPAIF_RDMA_REG_ADDR(v, addr, chan) \ >> (v->rdma_reg_base + (addr) + v->rdma_reg_stride * (chan)) >> @@ -103,17 +126,22 @@ >> #define LPAIF_WRDMAPER_REG(v, chan) LPAIF_WRDMA_REG_ADDR(v, >> 0x10, (chan)) >> #define LPAIF_WRDMAPERCNT_REG(v, chan) LPAIF_WRDMA_REG_ADDR(v, >> 0x14, (chan)) >> -#define __LPAIF_DMA_REG(v, chan, dir, reg) \ >> - (dir == SNDRV_PCM_STREAM_PLAYBACK) ? \ >> - LPAIF_RDMA##reg##_REG(v, chan) : \ >> - LPAIF_WRDMA##reg##_REG(v, chan) >> - >> -#define LPAIF_DMACTL_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, dir, >> CTL) >> -#define LPAIF_DMABASE_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, >> dir, BASE) >> -#define LPAIF_DMABUFF_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, >> dir, BUFF) >> -#define LPAIF_DMACURR_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, >> dir, CURR) >> -#define LPAIF_DMAPER_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, >> dir, PER) >> -#define LPAIF_DMAPERCNT_REG(v, chan, dir) __LPAIF_DMA_REG(v, >> chan, dir, PERCNT) >> +#define LPAIF_INTFDMA_REG(v, chan, reg, dai_id) \ >> + ((v->dai_driver[dai_id].id == LPASS_DP_RX) ? \ >> + LPAIF_HDMI_RDMA##reg##_REG(v, chan) : \ >> + LPAIF_RDMA##reg##_REG(v, chan)) >> + >> +#define __LPAIF_DMA_REG(v, chan, dir, reg, dai_id) \ >> + ((dir == SNDRV_PCM_STREAM_PLAYBACK) ? \ >> + (LPAIF_INTFDMA_REG(v, chan, reg, dai_id)) : \ >> + LPAIF_WRDMA##reg##_REG(v, chan)) >> + >> +#define LPAIF_DMACTL_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, >> chan, dir, CTL, dai_id) >> +#define LPAIF_DMABASE_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, >> chan, dir, BASE, dai_id) >> +#define LPAIF_DMABUFF_REG(v, chan, dir, dai_id) >> __LPAIF_DMA_REG(v, chan, dir, BUFF, dai_id) >> +#define LPAIF_DMACURR_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, >> chan, dir, CURR, dai_id) >> +#define LPAIF_DMAPER_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, >> chan, dir, PER, dai_id) >> +#define LPAIF_DMAPERCNT_REG(v, chan, dir, dai_id) >> __LPAIF_DMA_REG(v, chan, dir, PERCNT, dai_id) >> #define LPAIF_DMACTL_BURSTEN_SINGLE 0 >> #define LPAIF_DMACTL_BURSTEN_INCR4 1 >> diff --git a/sound/soc/qcom/lpass-platform.c >> b/sound/soc/qcom/lpass-platform.c >> index 35aead1..e2d1f25 100644 >> --- a/sound/soc/qcom/lpass-platform.c >> +++ b/sound/soc/qcom/lpass-platform.c >> @@ -23,7 +23,7 @@ struct lpass_pcm_data { >> int i2s_port; >> }; >> -#define LPASS_PLATFORM_BUFFER_SIZE (16 * 1024) >> +#define LPASS_PLATFORM_BUFFER_SIZE (24 * 2 * 1024) >> #define LPASS_PLATFORM_PERIODS 2 >> static const struct snd_pcm_hardware lpass_platform_pcm_hardware = { >> @@ -62,41 +62,79 @@ static int >> lpass_platform_alloc_dmactl_fields(struct device *dev, >> if (drvdata->rd_dmactl == NULL) >> return -ENOMEM; >> - drvdata->wr_dmactl = devm_kzalloc(dev, sizeof(struct >> lpaif_dmactl), >> - GFP_KERNEL); >> - if (drvdata->wr_dmactl == NULL) >> - return -ENOMEM; >> - >> rd_dmactl = drvdata->rd_dmactl; >> - wr_dmactl = drvdata->wr_dmactl; >> rd_dmactl->bursten = devm_regmap_field_alloc(dev, map, >> v->rdma_bursten); >> rd_dmactl->wpscnt = devm_regmap_field_alloc(dev, map, >> v->rdma_wpscnt); >> rd_dmactl->fifowm = devm_regmap_field_alloc(dev, map, >> v->rdma_fifowm); >> - rd_dmactl->intf = devm_regmap_field_alloc(dev, map, v->rdma_intf); >> rd_dmactl->enable = devm_regmap_field_alloc(dev, map, >> v->rdma_enable); >> rd_dmactl->dyncclk = devm_regmap_field_alloc(dev, map, >> v->rdma_dyncclk); >> - >> + rd_dmactl->intf = devm_regmap_field_alloc(dev, map, v->rdma_intf); >> if (IS_ERR(rd_dmactl->bursten) || IS_ERR(rd_dmactl->wpscnt) || >> - IS_ERR(rd_dmactl->fifowm) || IS_ERR(rd_dmactl->intf) || >> - IS_ERR(rd_dmactl->enable) || IS_ERR(rd_dmactl->dyncclk)) >> + IS_ERR(rd_dmactl->fifowm) || IS_ERR(rd_dmactl->dyncclk) || >> + IS_ERR(rd_dmactl->enable) || IS_ERR(rd_dmactl->intf)) >> return -EINVAL; >> + drvdata->wr_dmactl = devm_kzalloc(dev, >> + sizeof(struct lpaif_dmactl), GFP_KERNEL); >> + if (drvdata->wr_dmactl == NULL) >> + return -ENOMEM; >> - wr_dmactl->bursten = devm_regmap_field_alloc(dev, map, >> v->wrdma_bursten); >> - wr_dmactl->wpscnt = devm_regmap_field_alloc(dev, map, >> v->wrdma_wpscnt); >> - wr_dmactl->fifowm = devm_regmap_field_alloc(dev, map, >> v->wrdma_fifowm); >> - wr_dmactl->intf = devm_regmap_field_alloc(dev, map, v->wrdma_intf); >> - wr_dmactl->enable = devm_regmap_field_alloc(dev, map, >> v->wrdma_enable); >> - wr_dmactl->dyncclk = devm_regmap_field_alloc(dev, map, >> v->wrdma_dyncclk); >> + wr_dmactl = drvdata->wr_dmactl; >> + >> + wr_dmactl->bursten = devm_regmap_field_alloc(dev, map, >> + v->wrdma_bursten); >> + wr_dmactl->wpscnt = devm_regmap_field_alloc(dev, map, >> + v->wrdma_wpscnt); >> + wr_dmactl->fifowm = devm_regmap_field_alloc(dev, map, >> + v->wrdma_fifowm); >> + wr_dmactl->intf = devm_regmap_field_alloc(dev, map, >> + v->wrdma_intf); >> + wr_dmactl->enable = devm_regmap_field_alloc(dev, map, >> + v->wrdma_enable); >> + wr_dmactl->dyncclk = devm_regmap_field_alloc(dev, map, >> + v->wrdma_dyncclk); > > Can you explain what is happening here, these chagnes just look like > spllitting one line into two! Will bring them in same line. As previous code has long lines and not taken care after modification. > > > I should stop reviewing at this point! > As I have seen some general issues with the patch like > - doing un-needed changes! > - removing #ifdef checks. > - try to come up with some api/macro to simplify field allocs as they > seems to be all over the place! > > thanks, > srini
-- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
| |