- Linaro-mm-sig - lists.linaro.org

Re: [PATCH 11/18] ASoC: mediatek: mt8365: Add platform driver

by Krzysztof Kozlowski

On 26/02/2024 15:01, Alexandre Mergnat wrote: > Add mt8365 platform driver. > > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> ... > + memif->substream = substream; > + > + snd_pcm_hw_constraint_step(substream->runtime, 0, > + SNDRV_PCM_HW_PARAM_BUFFER_BYTES, 16); > + > + snd_soc_set_runtime_hwparams(substream, mtk_afe_hardware); > + > + ret = snd_pcm_hw_constraint_integer(runtime, > + SNDRV_PCM_HW_PARAM_PERIODS); > + if (ret < 0) > + dev_err(afe->dev, "snd_pcm_hw_constraint_integer failed\n"); > + > + mt8365_afe_enable_main_clk(afe); > + return ret; > +} > + > +static void mt8365_afe_fe_shutdown(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); > + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai); > + int memif_num = snd_soc_rtd_to_cpu(rtd, 0)->id; > + struct mtk_base_afe_memif *memif = &afe->memif[memif_num]; > + > + dev_dbg(afe->dev, "%s %s\n", __func__, memif->data->name); Drop trace-like prints. Tracing is for this. Before upstreaming driver from vendor repository, such stuff should be cleaned. > + > + memif->substream = NULL; > + > + mt8365_afe_disable_main_clk(afe); > +} > + > +static int mt8365_afe_fe_hw_params(struct snd_pcm_substream *substream, > + struct snd_pcm_hw_params *params, struct snd_soc_dai *dai) > +{ > + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); > + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai); > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + int dai_id = snd_soc_rtd_to_cpu(rtd, 0)->id; > + struct mt8365_control_data *ctrl_data = &afe_priv->ctrl_data; > + struct mtk_base_afe_memif *memif = &afe->memif[dai_id]; > + struct mt8365_fe_dai_data *fe_data = &afe_priv->fe_data[dai_id]; > + const size_t request_size = params_buffer_bytes(params); > + unsigned int channels = params_channels(params); > + unsigned int rate = params_rate(params); > + int ret, fs = 0; > + unsigned int base_end_offset = 8; > + > + dev_info(afe->dev, > + "%s %s period = %d rate = %d channels = %d\n", > + __func__, memif->data->name, params_period_size(params), > + rate, channels); > + > + if (dai_id == MT8365_AFE_MEMIF_VUL2) { > + if (!ctrl_data->bypass_cm1) > + /* configure cm1 */ > + mt8365_afe_configure_cm(afe, > + MT8365_CM1, channels, rate); > + else > + regmap_update_bits(afe->regmap, AFE_CM1_CON0, > + CM_AFE_CM1_VUL_SEL, CM_AFE_CM1_VUL_SEL); > + } else if (dai_id == MT8365_AFE_MEMIF_TDM_IN) { > + if (!ctrl_data->bypass_cm2) > + /* configure cm2 */ > + mt8365_afe_configure_cm(afe, > + MT8365_CM2, channels, rate); > + else > + regmap_update_bits(afe->regmap, AFE_CM2_CON0, > + CM_AFE_CM2_TDM_SEL, ~CM_AFE_CM2_TDM_SEL); > + > + base_end_offset = 4; > + } > + > + if (request_size > fe_data->sram_size) { > + ret = snd_pcm_lib_malloc_pages(substream, request_size); > + if (ret < 0) { > + dev_err(afe->dev, > + "%s %s malloc pages %zu bytes failed %d\n", > + __func__, memif->data->name, request_size, ret); > + return ret; > + } > + > + fe_data->use_sram = false; > + > + mt8365_afe_emi_clk_on(afe); > + } else { > + struct snd_dma_buffer *dma_buf = &substream->dma_buffer; > + > + dma_buf->dev.type = SNDRV_DMA_TYPE_DEV; > + dma_buf->dev.dev = substream->pcm->card->dev; > + dma_buf->area = (unsigned char *)fe_data->sram_vir_addr; > + dma_buf->addr = fe_data->sram_phy_addr; > + dma_buf->bytes = request_size; > + snd_pcm_set_runtime_buffer(substream, dma_buf); > + > + fe_data->use_sram = true; > + } > + > + memif->phys_buf_addr = lower_32_bits(substream->runtime->dma_addr); > + memif->buffer_size = substream->runtime->dma_bytes; > + > + /* start */ > + regmap_write(afe->regmap, memif->data->reg_ofs_base, > + memif->phys_buf_addr); > + /* end */ > + regmap_write(afe->regmap, > + memif->data->reg_ofs_base + base_end_offset, > + memif->phys_buf_addr + memif->buffer_size - 1); > + > + /* set channel */ > + if (memif->data->mono_shift >= 0) { > + unsigned int mono = (params_channels(params) == 1) ? 1 : 0; > + > + if (memif->data->mono_reg < 0) > + dev_info(afe->dev, "%s mono_reg is NULL\n", __func__); > + else > + regmap_update_bits(afe->regmap, memif->data->mono_reg, > + 1 << memif->data->mono_shift, > + mono << memif->data->mono_shift); > + } > + > + /* set rate */ > + if (memif->data->fs_shift < 0) > + return 0; > + > + fs = afe->memif_fs(substream, params_rate(params)); > + > + if (fs < 0) > + return -EINVAL; > + > + if (memif->data->fs_reg < 0) > + dev_info(afe->dev, "%s fs_reg is NULL\n", __func__); > + else > + regmap_update_bits(afe->regmap, memif->data->fs_reg, > + memif->data->fs_maskbit << memif->data->fs_shift, > + fs << memif->data->fs_shift); > + > + return 0; > +} > + > +static int mt8365_afe_fe_hw_free(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); > + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai); > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + int dai_id = snd_soc_rtd_to_cpu(rtd, 0)->id; > + struct mtk_base_afe_memif *memif = &afe->memif[dai_id]; > + struct mt8365_fe_dai_data *fe_data = &afe_priv->fe_data[dai_id]; > + int ret = 0; > + > + dev_dbg(afe->dev, "%s %s\n", __func__, memif->data->name); Drop > + > + if (fe_data->use_sram) { > + snd_pcm_set_runtime_buffer(substream, NULL); > + } else { > + ret = snd_pcm_lib_free_pages(substream); > + > + mt8365_afe_emi_clk_off(afe); > + } > + > + return ret; > +} > + > +static int mt8365_afe_fe_prepare(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); > + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai); > + int dai_id = snd_soc_rtd_to_cpu(rtd, 0)->id; > + struct mtk_base_afe_memif *memif = &afe->memif[dai_id]; > + > + /* set format */ > + if (memif->data->hd_reg >= 0) { > + switch (substream->runtime->format) { > + case SNDRV_PCM_FORMAT_S16_LE: > + regmap_update_bits(afe->regmap, memif->data->hd_reg, > + 3 << memif->data->hd_shift, > + 0 << memif->data->hd_shift); > + break; > + case SNDRV_PCM_FORMAT_S32_LE: > + regmap_update_bits(afe->regmap, memif->data->hd_reg, > + 3 << memif->data->hd_shift, > + 3 << memif->data->hd_shift); > + > + if (dai_id == MT8365_AFE_MEMIF_TDM_IN) { > + regmap_update_bits(afe->regmap, > + memif->data->hd_reg, > + 3 << memif->data->hd_shift, > + 1 << memif->data->hd_shift); > + regmap_update_bits(afe->regmap, > + memif->data->hd_reg, > + 1 << memif->data->hd_align_mshift, > + 1 << memif->data->hd_align_mshift); > + } > + break; > + case SNDRV_PCM_FORMAT_S24_LE: > + regmap_update_bits(afe->regmap, memif->data->hd_reg, > + 3 << memif->data->hd_shift, > + 1 << memif->data->hd_shift); > + break; > + default: > + return -EINVAL; > + } > + } > + > + mt8365_afe_irq_direction_enable(afe, > + memif->irq_usage, > + MT8365_AFE_IRQ_DIR_MCU); > + > + return 0; > +} > + > +int mt8365_afe_fe_trigger(struct snd_pcm_substream *substream, int cmd, > + struct snd_soc_dai *dai) > +{ > + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); > + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai); > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + int dai_id = snd_soc_rtd_to_cpu(rtd, 0)->id; > + struct mt8365_control_data *ctrl_data = &afe_priv->ctrl_data; > + > + switch (cmd) { > + case SNDRV_PCM_TRIGGER_START: > + case SNDRV_PCM_TRIGGER_RESUME: > + /* enable channel merge */ > + if (dai_id == MT8365_AFE_MEMIF_VUL2 && > + !ctrl_data->bypass_cm1) { > + regmap_update_bits(afe->regmap, > + AFE_CM1_CON0, > + CM_AFE_CM_ON, CM_AFE_CM_ON); > + } else if (dai_id == MT8365_AFE_MEMIF_TDM_IN && > + !ctrl_data->bypass_cm2) { > + regmap_update_bits(afe->regmap, > + AFE_CM2_CON0, > + CM_AFE_CM_ON, CM_AFE_CM_ON); > + } > + break; > + case SNDRV_PCM_TRIGGER_STOP: > + case SNDRV_PCM_TRIGGER_SUSPEND: > + /* disable channel merge */ > + if (dai_id == MT8365_AFE_MEMIF_VUL2 && > + !ctrl_data->bypass_cm1) { > + regmap_update_bits(afe->regmap, > + AFE_CM1_CON0, > + CM_AFE_CM_ON, ~CM_AFE_CM_ON); > + } else if (dai_id == MT8365_AFE_MEMIF_TDM_IN && > + !ctrl_data->bypass_cm2) { > + regmap_update_bits(afe->regmap, > + AFE_CM2_CON0, > + CM_AFE_CM_ON, ~CM_AFE_CM_ON); > + } > + break; > + default: > + break; > + } > + > + return mtk_afe_fe_trigger(substream, cmd, dai); > +} > + > +static int mt8365_afe_hw_gain1_startup(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai); > + > + mt8365_afe_enable_main_clk(afe); > + return 0; > +} > + > +static void mt8365_afe_hw_gain1_shutdown(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai); > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + struct mt8365_be_dai_data *be = > + &afe_priv->be_data[dai->id - MT8365_AFE_BACKEND_BASE]; > + > + const unsigned int stream = substream->stream; Why do you const local scalars? What is the point of this variable anyway? > + > + if (be->prepared[stream]) { > + regmap_update_bits(afe->regmap, AFE_GAIN1_CON0, > + AFE_GAIN1_CON0_EN_MASK, 0); > + be->prepared[stream] = false; > + } > + mt8365_afe_disable_main_clk(afe); > +} > + > +static int mt8365_afe_hw_gain1_prepare(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct snd_pcm_runtime * const runtime = substream->runtime; > + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai); > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + struct mt8365_be_dai_data *be = > + &afe_priv->be_data[dai->id - MT8365_AFE_BACKEND_BASE]; > + > + const unsigned int rate = runtime->rate; > + const unsigned int stream = substream->stream; So this const is everywhere? How does it change it from code safety? No one expects to overwrite such variables anyway, so there is no increase in readability, either. > + int fs; > + unsigned int val1 = 0, val2 = 0; > + > + if (be->prepared[stream]) { > + dev_info(afe->dev, "%s prepared already\n", __func__); > + return 0; > + } > + > + fs = mt8365_afe_fs_timing(rate); > + regmap_update_bits(afe->regmap, AFE_GAIN1_CON0, > + AFE_GAIN1_CON0_MODE_MASK, (unsigned int)fs<<4); > + > + regmap_read(afe->regmap, AFE_GAIN1_CON1, &val1); > + regmap_read(afe->regmap, AFE_GAIN1_CUR, &val2); > + if ((val1 & AFE_GAIN1_CON1_MASK) != (val2 & AFE_GAIN1_CUR_MASK)) > + regmap_update_bits(afe->regmap, AFE_GAIN1_CUR, > + AFE_GAIN1_CUR_MASK, val1); > + > + regmap_update_bits(afe->regmap, AFE_GAIN1_CON0, > + AFE_GAIN1_CON0_EN_MASK, 1); > + be->prepared[stream] = true; > + > + return 0; > +} > + > +static const struct snd_pcm_hardware mt8365_hostless_hardware = { > + .info = (SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_INTERLEAVED | > + SNDRV_PCM_INFO_MMAP_VALID), > + .period_bytes_min = 256, > + .period_bytes_max = 4 * 48 * 1024, > + .periods_min = 2, > + .periods_max = 256, > + .buffer_bytes_max = 8 * 48 * 1024, > + .fifo_size = 0, > +}; > + > +/* dai ops */ > +static int mtk_dai_hostless_startup(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai); > + struct snd_pcm_runtime *runtime = substream->runtime; > + int ret; > + > + snd_soc_set_runtime_hwparams(substream, &mt8365_hostless_hardware); > + > + ret = snd_pcm_hw_constraint_integer(runtime, > + SNDRV_PCM_HW_PARAM_PERIODS); > + if (ret < 0) > + dev_info(afe->dev, "snd_pcm_hw_constraint_integer failed\n"); > + return ret; > +} > + > +/* FE DAIs */ > +static const struct snd_soc_dai_ops mt8365_afe_fe_dai_ops = { > + .startup = mt8365_afe_fe_startup, > + .shutdown = mt8365_afe_fe_shutdown, > + .hw_params = mt8365_afe_fe_hw_params, > + .hw_free = mt8365_afe_fe_hw_free, > + .prepare = mt8365_afe_fe_prepare, > + .trigger = mt8365_afe_fe_trigger, > +}; > + > +static const struct snd_soc_dai_ops mt8365_dai_hostless_ops = { > + .startup = mtk_dai_hostless_startup, > +}; > + ... > +static int mt8365_afe_cm2_io_input_mux_get(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) > +{ > + ucontrol->value.integer.value[0] = mCM2Input; > + > + return 0; > +} > + > +static int mt8365_afe_cm2_io_input_mux_put(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) Run checkpatch --strict > +{ > + struct snd_soc_dapm_context *dapm = > + snd_soc_dapm_kcontrol_dapm(kcontrol); > + struct snd_soc_component *comp = snd_soc_dapm_to_component(dapm); > + struct mtk_base_afe *afe = snd_soc_component_get_drvdata(comp); > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + int ret; > + > + mCM2Input = ucontrol->value.enumerated.item[0]; > + > + afe_priv->cm2_mux_input = mCM2Input; > + ret = snd_soc_dapm_put_enum_double(kcontrol, ucontrol); > + > + return ret; > +} > + ... > + > +static const struct regmap_config mt8365_afe_regmap_config = { > + .reg_bits = 32, > + .reg_stride = 4, > + .val_bits = 32, > + .max_register = MAX_REGISTER, > + .cache_type = REGCACHE_NONE, > +}; > + > +static irqreturn_t mt8365_afe_irq_handler(int irq, void *dev_id) > +{ > + struct mtk_base_afe *afe = dev_id; > + unsigned int reg_value; > + unsigned int mcu_irq_mask; > + int i, ret; > + > + ret = regmap_read(afe->regmap, AFE_IRQ_MCU_STATUS, &reg_value); > + if (ret) { > + dev_err(afe->dev, "%s irq status err\n", __func__); I can imagine happy user seeing flooded dmesg on each IRQ with these messages... > + reg_value = AFE_IRQ_STATUS_BITS; > + goto err_irq; > + } > + > + ret = regmap_read(afe->regmap, AFE_IRQ_MCU_EN, &mcu_irq_mask); > + if (ret) { > + dev_err(afe->dev, "%s irq mcu_en err\n", __func__); > + reg_value = AFE_IRQ_STATUS_BITS; > + goto err_irq; > + } > + > + /* only clr cpu irq */ > + reg_value &= mcu_irq_mask; > + > + for (i = 0; i < MT8365_AFE_MEMIF_NUM; i++) { > + struct mtk_base_afe_memif *memif = &afe->memif[i]; > + struct mtk_base_afe_irq *mcu_irq; > + > + if (memif->irq_usage < 0) > + continue; > + > + mcu_irq = &afe->irqs[memif->irq_usage]; > + > + if (!(reg_value & (1 << mcu_irq->irq_data->irq_clr_shift))) > + continue; > + > + snd_pcm_period_elapsed(memif->substream); > + } > + > +err_irq: > + /* clear irq */ > + regmap_write(afe->regmap, AFE_IRQ_MCU_CLR, > + reg_value & AFE_IRQ_STATUS_BITS); > + > + return IRQ_HANDLED; > +} > + > +static int __maybe_unused mt8365_afe_runtime_suspend(struct device *dev) > +{ > + return 0; > +} > + > +static int mt8365_afe_runtime_resume(struct device *dev) > +{ > + return 0; > +} > + > +static int __maybe_unused mt8365_afe_suspend(struct device *dev) > +{ > + struct mtk_base_afe *afe = dev_get_drvdata(dev); > + struct regmap *regmap = afe->regmap; > + int i; > + > + mt8365_afe_enable_main_clk(afe); > + > + if (!afe->reg_back_up) > + afe->reg_back_up = > + devm_kcalloc(dev, afe->reg_back_up_list_num, > + sizeof(unsigned int), GFP_KERNEL); > + > + for (i = 0; i < afe->reg_back_up_list_num; i++) > + regmap_read(regmap, afe->reg_back_up_list[i], > + &afe->reg_back_up[i]); > + > + mt8365_afe_disable_main_clk(afe); > + > + return 0; > +} > + > +static int __maybe_unused mt8365_afe_resume(struct device *dev) > +{ > + struct mtk_base_afe *afe = dev_get_drvdata(dev); > + struct regmap *regmap = afe->regmap; > + int i = 0; > + > + if (!afe->reg_back_up) { > + dev_dbg(dev, "%s no reg_backup\n", __func__); > + return 0; > + } > + > + mt8365_afe_enable_main_clk(afe); > + > + for (i = 0; i < afe->reg_back_up_list_num; i++) > + regmap_write(regmap, afe->reg_back_up_list[i], > + afe->reg_back_up[i]); > + > + mt8365_afe_disable_main_clk(afe); > + > + return 0; > +} > + > +static int __maybe_unused mt8365_afe_dev_runtime_suspend(struct device *dev) > +{ > + struct mtk_base_afe *afe = dev_get_drvdata(dev); > + > + dev_dbg(afe->dev, "%s suspend %d %d >>\n", __func__, > + pm_runtime_status_suspended(dev), afe->suspended); Drop silly trace-like prints. Tracing is for this. > + > + if (pm_runtime_status_suspended(dev) || afe->suspended) > + return 0; > + > + mt8365_afe_suspend(dev); > + > + afe->suspended = true; > + > + dev_dbg(afe->dev, "%s <<\n", __func__); Drop silly trace-like prints. Tracing is for this. > + > + return 0; > +} > + > +static int __maybe_unused mt8365_afe_dev_runtime_resume(struct device *dev) > +{ > + struct mtk_base_afe *afe = dev_get_drvdata(dev); > + > + dev_dbg(afe->dev, "%s suspend %d %d >>\n", __func__, > + pm_runtime_status_suspended(dev), afe->suspended); Drop silly trace-like prints. Tracing is for this. > + > + if (pm_runtime_status_suspended(dev) || !afe->suspended) > + return 0; > + > + mt8365_afe_resume(dev); > + > + afe->suspended = false; > + > + dev_dbg(afe->dev, "%s <<\n", __func__); Drop silly trace-like prints. Tracing is for this. > + > + return 0; > +} > + > +static int mt8365_afe_init_registers(struct mtk_base_afe *afe) > +{ > + size_t i; > + > + static struct { > + unsigned int reg; > + unsigned int mask; > + unsigned int val; > + } init_regs[] = { > + { AFE_CONN_24BIT, GENMASK(31, 0), GENMASK(31, 0) }, > + { AFE_CONN_24BIT_1, GENMASK(21, 0), GENMASK(21, 0) }, > + }; > + > + mt8365_afe_enable_main_clk(afe); > + > + for (i = 0; i < ARRAY_SIZE(init_regs); i++) > + regmap_update_bits(afe->regmap, init_regs[i].reg, > + init_regs[i].mask, init_regs[i].val); > + > + mt8365_afe_disable_main_clk(afe); > + > + return 0; > +} > + > +static int mt8365_afe_component_probe(struct snd_soc_component *component) > +{ > + return mtk_afe_add_sub_dai_control(component); > +} > + > +static const struct snd_soc_component_driver mt8365_afe_component = { > + .name = AFE_PCM_NAME, > + .probe = mt8365_afe_component_probe, > + .pointer = mtk_afe_pcm_pointer, > + .pcm_construct = mtk_afe_pcm_new, > +}; > + > +static const struct snd_soc_component_driver mt8365_afe_pcm_component = { > + .name = "mt8365-afe-pcm-dai", > +}; > + > +static int mt8365_dai_memif_register(struct mtk_base_afe *afe) > +{ > + struct mtk_base_afe_dai *dai; > + > + dai = devm_kzalloc(afe->dev, sizeof(*dai), GFP_KERNEL); > + if (!dai) > + return -ENOMEM; > + > + list_add(&dai->list, &afe->sub_dais); > + > + dai->dai_drivers = mt8365_memif_dai_driver; > + dai->num_dai_drivers = ARRAY_SIZE(mt8365_memif_dai_driver); > + > + dai->dapm_widgets = mt8365_memif_widgets; > + dai->num_dapm_widgets = ARRAY_SIZE(mt8365_memif_widgets); > + dai->dapm_routes = mt8365_memif_routes; > + dai->num_dapm_routes = ARRAY_SIZE(mt8365_memif_routes); > + return 0; > +} > + > +typedef int (*dai_register_cb)(struct mtk_base_afe *); > +static const dai_register_cb dai_register_cbs[] = { > + mt8365_dai_pcm_register, > + mt8365_dai_i2s_register, > + mt8365_dai_adda_register, > + mt8365_dai_dmic_register, > + mt8365_dai_memif_register, > +}; > + > +static int mt8365_afe_pcm_dev_probe(struct platform_device *pdev) > +{ > + struct mtk_base_afe *afe; > + struct mt8365_afe_private *afe_priv; > + struct device *dev; > + int ret, i, sel_irq; > + unsigned int irq_id; > + struct resource *res; > + > + afe = devm_kzalloc(&pdev->dev, sizeof(*afe), GFP_KERNEL); > + if (!afe) > + return -ENOMEM; > + platform_set_drvdata(pdev, afe); > + > + afe->platform_priv = devm_kzalloc(&pdev->dev, sizeof(*afe_priv), > + GFP_KERNEL); > + if (!afe->platform_priv) > + return -ENOMEM; > + > + afe_priv = afe->platform_priv; > + afe->dev = &pdev->dev; > + dev = afe->dev; > + > + spin_lock_init(&afe_priv->afe_ctrl_lock); > + mutex_init(&afe_priv->afe_clk_mutex); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + afe->base_addr = devm_ioremap_resource(&pdev->dev, res); Why not using helper combining these two? > + if (IS_ERR(afe->base_addr)) > + return PTR_ERR(afe->base_addr); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); > + if (res) { > + afe_priv->afe_sram_vir_addr = > + devm_ioremap_resource(&pdev->dev, res); > + if (!IS_ERR(afe_priv->afe_sram_vir_addr)) { > + afe_priv->afe_sram_phy_addr = res->start; > + afe_priv->afe_sram_size = resource_size(res); > + } > + } > + > + /* initial audio related clock */ > + ret = mt8365_afe_init_audio_clk(afe); > + if (ret) { > + dev_err(afe->dev, "mt8365_afe_init_audio_clk fail\n"); return dev_err_probe() > + return ret; > + } > + > + afe->regmap = devm_regmap_init_mmio_clk(&pdev->dev, "top_audio_sel", > + afe->base_addr, &mt8365_afe_regmap_config); > + if (IS_ERR(afe->regmap)) > + return PTR_ERR(afe->regmap); > + > + /* memif % irq initialize*/ > + afe->memif_size = MT8365_AFE_MEMIF_NUM; > + afe->memif = devm_kcalloc(afe->dev, afe->memif_size, > + sizeof(*afe->memif), GFP_KERNEL); > + if (!afe->memif) > + return -ENOMEM; > + > + afe->irqs_size = MT8365_AFE_IRQ_NUM; > + afe->irqs = devm_kcalloc(afe->dev, afe->irqs_size, > + sizeof(*afe->irqs), GFP_KERNEL); > + if (!afe->irqs) > + return -ENOMEM; > + > + for (i = 0; i < afe->irqs_size; i++) > + afe->irqs[i].irq_data = &irq_data[i]; > + > + irq_id = platform_get_irq(pdev, 0); > + if (!irq_id) { That does not look correct. 0 is valid. Look at help of this function. > + dev_err(afe->dev, "np %s no irq\n", afe->dev->of_node->name); return dev_err_probe() > + return -ENXIO; > + } > + ret = devm_request_irq(afe->dev, irq_id, mt8365_afe_irq_handler, > + 0, "Afe_ISR_Handle", (void *)afe); > + if (ret) { > + dev_err(afe->dev, "could not request_irq\n"); return dev_err_probe() > + return ret; > + } > + > + /* init sub_dais */ > + INIT_LIST_HEAD(&afe->sub_dais); > + > + for (i = 0; i < ARRAY_SIZE(dai_register_cbs); i++) { > + ret = dai_register_cbs[i](afe); > + if (ret) { > + dev_warn(afe->dev, "dai register i %d fail, ret %d\n", > + i, ret); > + return ret; > + } > + } > + > + /* init dai_driver and component_driver */ > + ret = mtk_afe_combine_sub_dai(afe); > + if (ret) { > + dev_warn(afe->dev, "mtk_afe_combine_sub_dai fail, ret %d\n", > + ret); > + return ret; > + } > + > + for (i = 0; i < afe->memif_size; i++) { > + afe->memif[i].data = &memif_data[i]; > + sel_irq = memif_specified_irqs[i]; > + if (sel_irq >= 0) { > + afe->memif[i].irq_usage = sel_irq; > + afe->memif[i].const_irq = 1; > + afe->irqs[sel_irq].irq_occupyed = true; > + } else { > + afe->memif[i].irq_usage = -1; > + } > + } > + > + afe->mtk_afe_hardware = &mt8365_afe_hardware; > + afe->memif_fs = mt8365_memif_fs; > + afe->irq_fs = mt8365_irq_fs; > + > + pm_runtime_enable(&pdev->dev); > + > + pm_runtime_get_sync(&pdev->dev); > + afe->reg_back_up_list = mt8365_afe_backup_list; > + afe->reg_back_up_list_num = ARRAY_SIZE(mt8365_afe_backup_list); > + afe->runtime_resume = mt8365_afe_runtime_resume; > + afe->runtime_suspend = mt8365_afe_runtime_suspend; > + > + /* open afe pdn for dapm read/write audio register */ > + mt8365_afe_enable_top_cg(afe, MT8365_TOP_CG_AFE); > + > + /* Set 26m parent clk */ > + mt8365_afe_set_clk_parent(afe, > + afe_priv->clocks[MT8365_CLK_TOP_AUD_SEL], > + afe_priv->clocks[MT8365_CLK_CLK26M]); > + > + ret = devm_snd_soc_register_component(&pdev->dev, > + &mt8365_afe_component, NULL, 0); > + if (ret) { > + dev_warn(dev, "err_platform\n"); > + return ret; > + } > + > + ret = devm_snd_soc_register_component(&pdev->dev, > + &mt8365_afe_pcm_component, > + afe->dai_drivers, > + afe->num_dai_drivers); > + if (ret) { > + dev_warn(dev, "err_dai_component\n"); > + return ret; > + } > + > + mt8365_afe_init_registers(afe); > + > + dev_info(&pdev->dev, "MT8365 AFE driver initialized.\n"); Drop simple probe success messages everywhere. They are not needed and they don't bring useful information. Best regards, Krzysztof

1 year, 11 months

1
0
0 0

Re: [PATCH 10/18] ASoc: mediatek: mt8365: Add a specific soundcard for EVK

by Krzysztof Kozlowski

On 26/02/2024 15:01, amergnat(a)baylibre.com wrote: > From: Nicolas Belin <nbelin(a)baylibre.com> > > Add a specific soundcard for mt8365-evk. It supports audio jack > in/out, dmics, the amic and lineout. > > Signed-off-by: Nicolas Belin <nbelin(a)baylibre.com> > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> ... > +static int mt8365_mt6357_dev_probe(struct platform_device *pdev) > +{ > + struct snd_soc_card *card = &mt8365_mt6357_card; > + struct device *dev = &pdev->dev; > + struct device_node *platform_node; > + struct mt8365_mt6357_priv *priv; > + int i, ret; > + > + card->dev = dev; > + ret = parse_dai_link_info(card); > + if (ret) > + goto err; > + > + platform_node = of_parse_phandle(dev->of_node, "mediatek,platform", 0); > + if (!platform_node) { > + dev_err(dev, "Property 'platform' missing or invalid\n"); > + return -EINVAL; > + } > + > + for (i = 0; i < card->num_links; i++) { > + if (mt8365_mt6357_dais[i].platforms->name) > + continue; > + mt8365_mt6357_dais[i].platforms->of_node = platform_node; > + } > + > + priv = devm_kzalloc(dev, sizeof(struct mt8365_mt6357_priv), > + GFP_KERNEL); > + if (!priv) { > + ret = -ENOMEM; > + dev_err(dev, "%s allocate card private data fail %d\n", > + __func__, ret); Don't print anything on memory allocations failures. Run coccinelle/coccicheck to detect such trivial issues. > + return ret; > + } > + > + snd_soc_card_set_drvdata(card, priv); > + > + mt8365_mt6357_gpio_probe(card); > + > + ret = devm_snd_soc_register_card(dev, card); > + if (ret) > + dev_err(dev, "%s snd_soc_register_card fail %d\n", > + __func__, ret); > +err: > + of_node_put(platform_node); > + clean_card_reference(card); > + return ret; > +} > + > +static const struct of_device_id mt8365_mt6357_dt_match[] = { > + { .compatible = "mediatek,mt8365-mt6357", }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, mt8365_mt6357_dt_match); > + > +static struct platform_driver mt8365_mt6357_driver = { > + .driver = { > + .name = "mt8365_mt6357", > + .of_match_table = mt8365_mt6357_dt_match, > + .pm = &snd_soc_pm_ops, > + }, > + .probe = mt8365_mt6357_dev_probe, > +}; > + > +module_platform_driver(mt8365_mt6357_driver); > + > +/* Module information */ > +MODULE_DESCRIPTION("MT8365 EVK SoC machine driver"); > +MODULE_AUTHOR("Nicolas Belin <nbelin(a)baylibre.com>"); > +MODULE_LICENSE("GPL"); > +MODULE_ALIAS("mt8365 mt6357 soc card"); This does not look like correct alias. Drop. > Best regards, Krzysztof

1 year, 11 months

1
0
0 0

Re: [PATCH] net: ethernet: ti: am65-cpsw: Add minimal XDP support

by Andrew Lunn

> +static struct sk_buff *am65_cpsw_alloc_skb(struct net_device *ndev, unsigned int len) > +{ > + struct page *page; > + struct sk_buff *skb; > + > + page = dev_alloc_pages(0); You are likely to get better performance if you use the page_pool. When FEC added XDP support, the first set of changes was to make use of page_pool. That improved the drivers performance. Then XDP was added on top. Maybe you can follow that pattern. Andrew

1 year, 11 months

1
0
0 0

Re: [PATCH 12/18] ASoC: codecs: mt6357: add MT6357 codec

by Mark Brown

On Mon, Feb 26, 2024 at 03:01:50PM +0100, amergnat(a)baylibre.com wrote: > index 000000000000..13e95c227114 > --- /dev/null > +++ b/sound/soc/codecs/mt6357.c > @@ -0,0 +1,1805 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * MT6357 ALSA SoC audio codec driver > + * Please use a C++ comment for the whole comment to make it clearer that this is intentional. > +static void set_playback_gpio(struct mt6357_priv *priv, bool enable) > +{ > + if (enable) { > + /* set gpio mosi mode */ > + regmap_write(priv->regmap, MT6357_GPIO_MODE2_CLR, GPIO_MODE2_CLEAR_ALL); > + regmap_write(priv->regmap, MT6357_GPIO_MODE2_SET, GPIO8_MODE_SET_AUD_CLK_MOSI | > + GPIO9_MODE_SET_AUD_DAT_MOSI0 | > + GPIO10_MODE_SET_AUD_DAT_MOSI1 | > + GPIO11_MODE_SET_AUD_SYNC_MOSI); This would be a lot more legible if you worked out the values to set and then had a single set of writes, currently the indentation makes it very hard to read. Similarly for other similar functions. > +static int mt6357_put_volsw(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) > +{ > + struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol); > + struct mt6357_priv *priv = snd_soc_component_get_drvdata(component); > + struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; > + unsigned int reg; > + int ret; > + > + ret = snd_soc_put_volsw(kcontrol, ucontrol); > + if (ret < 0) > + return ret; > + > + switch (mc->reg) { > + case MT6357_ZCD_CON2: > + regmap_read(priv->regmap, MT6357_ZCD_CON2, &reg); > + priv->ana_gain[ANALOG_VOLUME_HPOUTL] = > + (reg & AUD_HPL_GAIN_MASK) >> AUD_HPL_GAIN_SFT; > + priv->ana_gain[ANALOG_VOLUME_HPOUTR] = > + (reg & AUD_HPR_GAIN_MASK) >> AUD_HPR_GAIN_SFT; > + break; It would probably be less code and would definitely be clearer and simpler to just read the values when we need them rather than constatly keeping a cache separate to the register cache. > + /* ul channel swap */ > + SOC_SINGLE("UL LR Swap", MT6357_AFE_UL_DL_CON0, AFE_UL_LR_SWAP_SFT, 1, 0), On/off controls should end in Switch. > +static const char * const hslo_mux_map[] = { > + "Open", "DACR", "Playback", "Test mode" > +}; > + > +static int hslo_mux_map_value[] = { > + 0x0, 0x1, 0x2, 0x3, > +}; Why not just use a normal mux here, there's no missing values or reordering? Similarly for other muxes. > +static unsigned int mt6357_read(struct snd_soc_component *codec, unsigned int reg) > +{ > + struct mt6357_priv *priv = snd_soc_component_get_drvdata(codec); > + unsigned int val; > + > + pr_debug("%s() reg = 0x%x", __func__, reg); > + regmap_read(priv->regmap, reg, &val); > + return val; > +} Remove these, there are vastly more logging facilities as standard in the regmap core. > +/* Reg bit defines */ > +/* MT6357_GPIO_DIR0 */ > +#define GPIO8_DIR_MASK BIT(8) > +#define GPIO8_DIR_INPUT 0 Please namespace your defines, these look very generic.

1 year, 11 months

1
0
0 0

Re: [PATCH 02/18] ASoC: dt-bindings: mediatek,mt8365-mt6357: Add audio sound card document

by AngeloGioacchino Del Regno

Il 26/02/24 15:01, Alexandre Mergnat ha scritto: > Add soundcard bindings for the MT8365 SoC with the MT6357 audio codec. > > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> > --- > .../bindings/sound/mediatek,mt8365-mt6357.yaml | 127 +++++++++++++++++++++ > 1 file changed, 127 insertions(+) > > diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8365-mt6357.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8365-mt6357.yaml > new file mode 100644 > index 000000000000..f469611ec6b6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8365-mt6357.yaml > @@ -0,0 +1,127 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/sound/mediatek,mt8365-mt6357.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Mediatek MT8365 sound card with MT6357 sound codec. > + > +maintainers: > + - Alexandre Mergnat <amergnat(a)baylibre.com> > + > +description: > + This binding describes the MT8365 sound card. > + > +properties: > + compatible: > + const: mediatek,mt8365-mt6357 > + > + mediatek,hp-pull-down: > + description: > + Earphone driver positive output stage short to the > + audio reference ground. > + Default value is false. > + type: boolean > + > + mediatek,micbias0-microvolt: > + description: | description: Selects MIC Bias 0 output voltage > + Selects MIC Bias 0 output voltage. > + [1.7v, 1.8v, 1.9v, 2.0v, 2.1v, 2.5v, 2.6v, 2.7v] > + enum: [0, 1, 2, 3, 4, 5, 6, 7] No, you don't say 0 1 2 3 4 to a property that says "microvolt", that's simply wrong. mediatek,micbias0-microvolt = <2100000>; ...so you want a binding that says enum: [ 1700000, 1800000, this, that, 2700000] > + > + mediatek,micbias1-microvolt: > + description: | > + Selects MIC Bias 1 output voltage. > + [1.7v, 1.8v, 1.9v, 2.0v, 2.1v, 2.5v, 2.6v, 2.7v] > + enum: [0, 1, 2, 3, 4, 5, 6, 7] same here. > + > + mediatek,platform: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: The phandle of MT8365 ASoC platform. > + > + pinctrl-names: > + minItems: 1 > + items: > + - const: aud_default > + - const: aud_dmic > + - const: aud_miso_off > + - const: aud_miso_on > + - const: aud_mosi_off > + - const: aud_mosi_on > + > + vaud28-supply: > + description: > + 2.8 volt supply for the audio codec > + > +patternProperties: > + "^dai-link-[0-9]+$": > + type: object > + description: > + Container for dai-link level properties and CODEC sub-nodes. > + > + properties: > + codec: > + type: object > + description: Holds subnode which indicates codec dai. > + > + properties: > + sound-dai: > + maxItems: 1 > + description: phandle of the codec DAI > + > + additionalProperties: false > + > + link-name: > + description: > + This property corresponds to the name of the BE dai-link to which > + we are going to update parameters in this node. > + items: > + const: 2ND I2S BE > + > + sound-dai: > + maxItems: 1 > + description: phandle of the CPU DAI > + > + additionalProperties: false > + > + required: > + - link-name > + - sound-dai > + > +additionalProperties: false > + > +required: > + - compatible > + - mediatek,platform > + - pinctrl-names > + - vaud28-supply > + > +examples: > + - | > + sound { > + compatible = "mediatek,mt8365-mt6357"; > + mediatek,platform = <&afe>; Please: https://docs.kernel.org/devicetree/bindings/dts-coding-style.html Regards, Angelo > + pinctrl-names = "aud_default", > + "aud_dmic", > + "aud_miso_off", > + "aud_miso_on", > + "aud_mosi_off", > + "aud_mosi_on"; > + pinctrl-0 = <&aud_default_pins>; > + pinctrl-1 = <&aud_dmic_pins>; > + pinctrl-2 = <&aud_miso_off_pins>; > + pinctrl-3 = <&aud_miso_on_pins>; > + pinctrl-4 = <&aud_mosi_off_pins>; > + pinctrl-5 = <&aud_mosi_on_pins>; > + vaud28-supply = <&mt6357_vaud28_reg>; > + > + /* hdmi interface */ > + dai-link-0 { > + sound-dai = <&afe>; > + link-name = "2ND I2S BE"; > + > + codec { > + sound-dai = <&it66121hdmitx>; > + }; > + }; > + }; >

1 year, 11 months

1
0
0 0

Re: [PATCH 13/18] mfd: mt6397-core: register mt6357 sound codec

by AngeloGioacchino Del Regno

Il 26/02/24 15:01, amergnat(a)baylibre.com ha scritto: > From: Fabien Parent <fparent(a)baylibre.com> > > Add MT6357 codec entry in the MFD driver. > > Signed-off-by: Fabien Parent <fparent(a)baylibre.com> > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno(a)collabora.com> > --- > drivers/mfd/mt6397-core.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c > index 4449dde05021..4fd4a2da5ad7 100644 > --- a/drivers/mfd/mt6397-core.c > +++ b/drivers/mfd/mt6397-core.c > @@ -141,6 +141,9 @@ static const struct mfd_cell mt6357_devs[] = { > .num_resources = ARRAY_SIZE(mt6357_rtc_resources), > .resources = mt6357_rtc_resources, > .of_compatible = "mediatek,mt6357-rtc", > + }, { > + .name = "mt6357-sound", > + .of_compatible = "mediatek,mt6357-sound" > }, { > .name = "mtk-pmic-keys", > .num_resources = ARRAY_SIZE(mt6357_keys_resources), >

1 year, 11 months

1
0
0 0

Re: [PATCH 12/18] ASoC: codecs: mt6357: add MT6357 codec

by AngeloGioacchino Del Regno

Il 26/02/24 15:01, amergnat(a)baylibre.com ha scritto: > From: Nicolas Belin <nbelin(a)baylibre.com> > > Add the support of MT6357 PMIC audio codec. > > Signed-off-by: Nicolas Belin <nbelin(a)baylibre.com> > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> > --- > sound/soc/codecs/Kconfig | 7 + > sound/soc/codecs/Makefile | 2 + > sound/soc/codecs/mt6357.c | 1805 +++++++++++++++++++++++++++++++++++++++++++++ > sound/soc/codecs/mt6357.h | 674 +++++++++++++++++ > 4 files changed, 2488 insertions(+) > > diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig > index 59f9742e9ff4..9cf2b9b70472 100644 > --- a/sound/soc/codecs/Kconfig > +++ b/sound/soc/codecs/Kconfig > @@ -153,6 +153,7 @@ config SND_SOC_ALL_CODECS > imply SND_SOC_MC13783 > imply SND_SOC_ML26124 > imply SND_SOC_MT6351 > + imply SND_SOC_MT6357 > imply SND_SOC_MT6358 > imply SND_SOC_MT6359 > imply SND_SOC_MT6660 > @@ -2361,6 +2362,12 @@ config SND_SOC_ML26124 > config SND_SOC_MT6351 > tristate "MediaTek MT6351 Codec" > > +config SND_SOC_MT6357 > + tristate "MediaTek MT6357 Codec" > + help > + Enable support for the platform which uses MT6357 as > + external codec device. > + > config SND_SOC_MT6358 > tristate "MediaTek MT6358 Codec" > help > diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile > index f53baa2b9565..33e27612867e 100644 > --- a/sound/soc/codecs/Makefile > +++ b/sound/soc/codecs/Makefile > @@ -169,6 +169,7 @@ snd-soc-ml26124-objs := ml26124.o > snd-soc-msm8916-analog-objs := msm8916-wcd-analog.o > snd-soc-msm8916-digital-objs := msm8916-wcd-digital.o > snd-soc-mt6351-objs := mt6351.o > +snd-soc-mt6357-objs := mt6357.o > snd-soc-mt6358-objs := mt6358.o > snd-soc-mt6359-objs := mt6359.o > snd-soc-mt6359-accdet-objs := mt6359-accdet.o > @@ -554,6 +555,7 @@ obj-$(CONFIG_SND_SOC_ML26124) += snd-soc-ml26124.o > obj-$(CONFIG_SND_SOC_MSM8916_WCD_ANALOG) +=snd-soc-msm8916-analog.o > obj-$(CONFIG_SND_SOC_MSM8916_WCD_DIGITAL) +=snd-soc-msm8916-digital.o > obj-$(CONFIG_SND_SOC_MT6351) += snd-soc-mt6351.o > +obj-$(CONFIG_SND_SOC_MT6357) += snd-soc-mt6357.o > obj-$(CONFIG_SND_SOC_MT6358) += snd-soc-mt6358.o > obj-$(CONFIG_SND_SOC_MT6359) += snd-soc-mt6359.o > obj-$(CONFIG_SND_SOC_MT6359_ACCDET) += mt6359-accdet.o > diff --git a/sound/soc/codecs/mt6357.c b/sound/soc/codecs/mt6357.c > new file mode 100644 > index 000000000000..13e95c227114 > --- /dev/null > +++ b/sound/soc/codecs/mt6357.c > @@ -0,0 +1,1805 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * MT6357 ALSA SoC audio codec driver > + * > + * Copyright (c) 2024 Baylibre > + * Author: Nicolas Belin <nbelin(a)baylibre.com> > + */ > + > +#include <linux/dma-mapping.h> > +#include <sound/soc.h> > +#include <sound/tlv.h> > +#include <linux/mfd/mt6397/core.h> > +#include <linux/regulator/consumer.h> > + > +#include "mt6357.h" > + > +static void set_playback_gpio(struct mt6357_priv *priv, bool enable) > +{ you're anyway always doing CLEAR_ALL, so you can do it outside of the if branch. regmap_write( ... CLEAR_ALL); if (enable) { ... } else { ... } > + if (enable) { > + /* set gpio mosi mode */ > + regmap_write(priv->regmap, MT6357_GPIO_MODE2_CLR, GPIO_MODE2_CLEAR_ALL); > + regmap_write(priv->regmap, MT6357_GPIO_MODE2_SET, GPIO8_MODE_SET_AUD_CLK_MOSI | > + GPIO9_MODE_SET_AUD_DAT_MOSI0 | > + GPIO10_MODE_SET_AUD_DAT_MOSI1 | > + GPIO11_MODE_SET_AUD_SYNC_MOSI); Are you sure that you need to write to MODE2_SET *and* to MODE2?! > + regmap_write(priv->regmap, MT6357_GPIO_MODE2, GPIO8_MODE_AUD_CLK_MOSI | > + GPIO9_MODE_AUD_DAT_MOSI0 | > + GPIO10_MODE_AUD_DAT_MOSI1 | > + GPIO11_MODE_AUD_SYNC_MOSI); > + } else { > + /* set pad_aud_*_mosi to GPIO mode and dir input > + * reason: > + * pad_aud_dat_mosi*, because the pin is used as boot strap > + */ > + regmap_write(priv->regmap, MT6357_GPIO_MODE2_CLR, GPIO_MODE2_CLEAR_ALL); > + regmap_write(priv->regmap, MT6357_GPIO_MODE2, GPIO8_MODE_GPIO | > + GPIO9_MODE_GPIO | > + GPIO10_MODE_GPIO | > + GPIO11_MODE_GPIO); > + regmap_update_bits(priv->regmap, MT6357_GPIO_DIR0, GPIO8_DIR_MASK | > + GPIO9_DIR_MASK | > + GPIO10_DIR_MASK | > + GPIO11_DIR_MASK, > + GPIO8_DIR_INPUT | > + GPIO9_DIR_INPUT | > + GPIO10_DIR_INPUT | > + GPIO11_DIR_INPUT); > + } > +} > + > +static void set_capture_gpio(struct mt6357_priv *priv, bool enable) > +{ same comment applies here. > + if (enable) { > + /* set gpio miso mode */ > + regmap_write(priv->regmap, MT6357_GPIO_MODE3_CLR, GPIO_MODE3_CLEAR_ALL); > + regmap_write(priv->regmap, MT6357_GPIO_MODE3_SET, GPIO12_MODE_SET_AUD_CLK_MISO | > + GPIO13_MODE_SET_AUD_DAT_MISO0 | > + GPIO14_MODE_SET_AUD_DAT_MISO1 | > + GPIO15_MODE_SET_AUD_SYNC_MISO); > + regmap_write(priv->regmap, MT6357_GPIO_MODE3, GPIO12_MODE_AUD_CLK_MISO | > + GPIO13_MODE_AUD_DAT_MISO0 | > + GPIO14_MODE_AUD_DAT_MISO1 | > + GPIO15_MODE_AUD_SYNC_MISO); > + } else { > + /* set pad_aud_*_miso to GPIO mode and dir input > + * reason: > + * pad_aud_clk_miso, because when playback only the miso_clk > + * will also have 26m, so will have power leak > + * pad_aud_dat_miso*, because the pin is used as boot strap > + */ > + regmap_write(priv->regmap, MT6357_GPIO_MODE3_CLR, GPIO_MODE3_CLEAR_ALL); > + regmap_write(priv->regmap, MT6357_GPIO_MODE3, GPIO12_MODE_GPIO | > + GPIO13_MODE_GPIO | > + GPIO14_MODE_GPIO | > + GPIO15_MODE_GPIO); > + regmap_update_bits(priv->regmap, MT6357_GPIO_DIR0, GPIO12_DIR_MASK | > + GPIO13_DIR_MASK | > + GPIO14_DIR_MASK | > + GPIO15_DIR_MASK, > + GPIO12_DIR_INPUT | > + GPIO13_DIR_INPUT | > + GPIO14_DIR_INPUT | > + GPIO15_DIR_INPUT); > + } > +} > + > +static void hp_main_output_ramp(struct mt6357_priv *priv, bool up) > +{ > + int i = 0, stage = 0; > + int target = 7; > + /* Enable/Reduce HPL/R main output stage step by step */ > + for (i = 0; i <= target; i++) { double 'i` initialization. > + stage = up ? i : target - i; > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON1, > + HPLOUT_STG_CTRL_VAUDP15_MASK, > + stage << HPLOUT_STG_CTRL_VAUDP15_SFT); > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON1, > + HPROUT_STG_CTRL_VAUDP15_MASK, > + stage << HPROUT_STG_CTRL_VAUDP15_SFT); > + usleep_range(600, 700); > + } > +} > + > +static void hp_aux_feedback_loop_gain_ramp(struct mt6357_priv *priv, bool up) > +{ > + int i = 0, stage = 0; > + /* Reduce HP aux feedback loop gain step by step */ > + for (i = 0; i <= 0xf; i++) { same > + stage = up ? i : 0xf - i; > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON6, > + HP_AUX_LOOP_GAIN_MASK, stage << HP_AUX_LOOP_GAIN_SFT); > + usleep_range(600, 700); > + } > +} > + > +static void hp_pull_down(struct mt6357_priv *priv, bool enable) > +{ > + if (enable) > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON2, > + HPP_SHORT_2VCM_VAUDP15_MASK, HPP_SHORT_2VCM_VAUDP15_ENABLE); > + else > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON2, > + HPP_SHORT_2VCM_VAUDP15_MASK, HPP_SHORT_2VCM_VAUDP15_DISABLE); > +} > + > +static bool is_valid_hp_pga_idx(int reg_idx) > +{ > + return (reg_idx >= DL_GAIN_8DB && reg_idx <= DL_GAIN_N_12DB) || reg_idx == DL_GAIN_N_40DB; > +} > + > +static void volume_ramp(struct mt6357_priv *priv, int lfrom, int lto, > + int rfrom, int rto, unsigned int reg_addr) > +{ > + int lcount, rcount, sleep = 0; > + > + if (!is_valid_hp_pga_idx(lfrom) || !is_valid_hp_pga_idx(lto)) > + pr_debug("%s(), invalid left volume index, from %d, to %d\n", > + __func__, lfrom, lto); > + > + if (!is_valid_hp_pga_idx(rfrom) || !is_valid_hp_pga_idx(rto)) > + pr_debug("%s(), invalid right volume index, from %d, to %d\n", > + __func__, rfrom, rto); > + > + if (lto > lfrom) > + lcount = 1; > + else > + lcount = -1; > + > + if (rto > rfrom) > + rcount = 1; > + else > + rcount = -1; > + > + while ((lto != lfrom) || (rto != rfrom)) { > + if (lto != lfrom) { > + lfrom += lcount; > + if (is_valid_hp_pga_idx(lfrom)) { > + regmap_update_bits(priv->regmap, reg_addr, DL_GAIN_REG_LEFT_MASK, > + lfrom << DL_GAIN_REG_LEFT_SHIFT); > + sleep = 1; > + } > + } > + if (rto != rfrom) { > + rfrom += rcount; > + if (is_valid_hp_pga_idx(rfrom)) { > + regmap_update_bits(priv->regmap, reg_addr, DL_GAIN_REG_RIGHT_MASK, > + rfrom << DL_GAIN_REG_RIGHT_SHIFT); > + sleep = 1; > + } > + } > + if (sleep) > + usleep_range(200, 300); > + } > +} > + > +static void lo_volume_ramp(struct mt6357_priv *priv, int lfrom, int lto, int rfrom, int rto) > +{ > + volume_ramp(priv, lfrom, lto, rfrom, rto, MT6357_ZCD_CON1); > +} > + > +static void hp_volume_ramp(struct mt6357_priv *priv, int lfrom, int lto, int rfrom, int rto) > +{ > + volume_ramp(priv, lfrom, lto, rfrom, rto, MT6357_ZCD_CON2); > +} > + > +static void hs_volume_ramp(struct mt6357_priv *priv, int from, int to) > +{ > + volume_ramp(priv, from, to, 0, 0, MT6357_ZCD_CON3); > +} > + > +static int mt6357_put_volsw(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) > +{ > + struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol); > + struct mt6357_priv *priv = snd_soc_component_get_drvdata(component); > + struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; > + unsigned int reg; > + int ret; > + > + ret = snd_soc_put_volsw(kcontrol, ucontrol); > + if (ret < 0) > + return ret; > + regmap_read(priv->regmap, mc->reg, &reg) switch (mc->reg) { ... }; return 0; > + switch (mc->reg) { > + case MT6357_ZCD_CON2: > + regmap_read(priv->regmap, MT6357_ZCD_CON2, &reg); > + priv->ana_gain[ANALOG_VOLUME_HPOUTL] = > + (reg & AUD_HPL_GAIN_MASK) >> AUD_HPL_GAIN_SFT; > + priv->ana_gain[ANALOG_VOLUME_HPOUTR] = > + (reg & AUD_HPR_GAIN_MASK) >> AUD_HPR_GAIN_SFT; > + break; > + case MT6357_ZCD_CON1: > + regmap_read(priv->regmap, MT6357_ZCD_CON1, &reg); > + priv->ana_gain[ANALOG_VOLUME_LINEOUTL] = > + (reg & AUD_LOL_GAIN_MASK) >> AUD_LOL_GAIN_SFT; > + priv->ana_gain[ANALOG_VOLUME_LINEOUTR] = > + (reg & AUD_LOR_GAIN_MASK) >> AUD_LOR_GAIN_SFT; > + break; > + case MT6357_ZCD_CON3: > + regmap_read(priv->regmap, MT6357_ZCD_CON3, &reg); > + priv->ana_gain[ANALOG_VOLUME_HSOUT] = > + (reg & AUD_HS_GAIN_MASK) >> AUD_HS_GAIN_SFT; > + break; > + case MT6357_AUDENC_ANA_CON0: > + case MT6357_AUDENC_ANA_CON1: > + regmap_read(priv->regmap, MT6357_AUDENC_ANA_CON0, &reg); > + priv->ana_gain[ANALOG_VOLUME_MIC1] = > + (reg & AUDPREAMPLGAIN_MASK) >> AUDPREAMPLGAIN_SFT; > + regmap_read(priv->regmap, MT6357_AUDENC_ANA_CON1, &reg); > + priv->ana_gain[ANALOG_VOLUME_MIC2] = > + (reg & AUDPREAMPRGAIN_MASK) >> AUDPREAMPRGAIN_SFT; > + break; > + } > + > + return ret; > +} > + ..snip.. > + > +static int adc_enable_event(struct snd_soc_dapm_widget *w, > + struct snd_kcontrol *kcontrol, > + int event) > +{ > + struct snd_soc_component *cmpnt = snd_soc_dapm_to_component(w->dapm); > + struct mt6357_priv *priv = snd_soc_component_get_drvdata(cmpnt); > + > + dev_dbg(priv->dev, "%s(), event = 0x%x\n", __func__, event); > + switch (event) { > + case SND_SOC_DAPM_PRE_PMU: > + /* enable aud_pad TX fifos */ > + regmap_update_bits(priv->regmap, MT6357_AFE_AUD_PAD_TOP, > + AUD_PAD_TX_FIFO_NORMAL_PATH_MASK, > + AUD_PAD_TX_FIFO_NORMAL_PATH_ENABLE); > + /* UL turn on */ > + regmap_update_bits(priv->regmap, MT6357_AFE_UL_SRC_CON0_L, > + UL_SRC_ON_TMP_CTL_MASK, UL_SRC_ENABLE); > + /* Wait to avoid any pop noises */ > + msleep(100); > + //set the mic gains to the stored values Keep the comments style consistent please. > + regmap_update_bits(priv->regmap, MT6357_AUDENC_ANA_CON0, AUDPREAMPLGAIN_MASK, > + priv->ana_gain[ANALOG_VOLUME_MIC1] << AUDPREAMPLGAIN_SFT); > + regmap_update_bits(priv->regmap, MT6357_AUDENC_ANA_CON1, AUDPREAMPRGAIN_MASK, > + priv->ana_gain[ANALOG_VOLUME_MIC2] << AUDPREAMPRGAIN_SFT); > + break; > + case SND_SOC_DAPM_POST_PMD: > + /* UL turn off */ > + regmap_update_bits(priv->regmap, MT6357_AFE_UL_SRC_CON0_L, > + UL_SRC_ON_TMP_CTL_MASK, UL_SRC_DISABLE); > + /* disable aud_pad TX fifos */ > + regmap_update_bits(priv->regmap, MT6357_AFE_AUD_PAD_TOP, > + AUD_PAD_TX_FIFO_NORMAL_PATH_MASK, > + AUD_PAD_TX_FIFO_NORMAL_PATH_DISABLE); > + break; > + default: > + break; > + } > + > + return 0; > +} > + ..snip.. > + > +static int lol_mux_event(struct snd_soc_dapm_widget *w, > + struct snd_kcontrol *kcontrol, > + int event) > +{ > + struct snd_soc_component *cmpnt = snd_soc_dapm_to_component(w->dapm); > + struct mt6357_priv *priv = snd_soc_component_get_drvdata(cmpnt); > + > + dev_dbg(priv->dev, "%s(), event 0x%x\n", __func__, event); > + switch (event) { > + case SND_SOC_DAPM_POST_PMU: > + /* Set LO STB enhance circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON4, > + AUD_LOLOUT_STB_ENH_VAUDP15_MASK, > + AUD_LOLOUT_STB_ENH_VAUDP15_ENABLE); > + /* Enable LO driver bias circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON4, > + AUD_LOL_PWRUP_BIAS_VAUDP15_MASK, > + AUD_LOL_PWRUP_BIAS_VAUDP15_ENABLE); > + /* Enable LO driver core circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON4, > + AUD_LOL_PWRUP_VAUDP15_MASK, AUD_LOL_PWRUP_VAUDP15_ENABLE); > + /* Set LOL gain to normal gain step by step */ > + lo_volume_ramp(priv, DL_GAIN_N_40DB, priv->ana_gain[ANALOG_VOLUME_LINEOUTL], > + DL_GAIN_N_40DB, priv->ana_gain[ANALOG_VOLUME_LINEOUTR]); > + break; > + case SND_SOC_DAPM_PRE_PMD: > + Drop extra blank line. > + /* decrease LOL gain to minimum gain step by step */ > + lo_volume_ramp(priv, priv->ana_gain[ANALOG_VOLUME_LINEOUTL], DL_GAIN_N_40DB, > + priv->ana_gain[ANALOG_VOLUME_LINEOUTR], DL_GAIN_N_40DB); > + /* Disable LO driver core circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON4, > + AUD_LOL_PWRUP_VAUDP15_MASK, AUD_LOL_PWRUP_VAUDP15_DISABLE); > + /* Disable LO driver bias circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON4, > + AUD_LOL_PWRUP_BIAS_VAUDP15_MASK, > + AUD_LOL_PWRUP_BIAS_VAUDP15_DISABLE); > + /* Clear LO STB enhance circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON4, > + AUD_LOLOUT_STB_ENH_VAUDP15_MASK, > + AUD_LOLOUT_STB_ENH_VAUDP15_DISABLE); > + break; > + default: > + break; > + } > + > + return 0; > +} > + > +static int hs_mux_event(struct snd_soc_dapm_widget *w, > + struct snd_kcontrol *kcontrol, > + int event) > +{ > + struct snd_soc_component *cmpnt = snd_soc_dapm_to_component(w->dapm); > + struct mt6357_priv *priv = snd_soc_component_get_drvdata(cmpnt); > + > + dev_dbg(priv->dev, "%s(), event 0x%x\n", __func__, event); > + switch (event) { > + case SND_SOC_DAPM_POST_PMU: > + > + /* Set HS STB enhance circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON3, > + AUD_HSOUT_STB_ENH_VAUDP15_MASK, > + AUD_HSOUT_STB_ENH_VAUDP15_ENABLE); > + /* Enable HS driver bias circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON3, > + AUD_HS_PWRUP_BIAS_VAUDP15_MASK, > + AUD_HS_PWRUP_BIAS_VAUDP15_ENABLE); > + /* Enable HS driver core circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON3, > + AUD_HS_PWRUP_VAUDP15_MASK, AUD_HS_PWRUP_VAUDP15_ENABLE); > + /* Set HS gain to normal gain step by step */ > + hs_volume_ramp(priv, DL_GAIN_N_40DB, priv->ana_gain[ANALOG_VOLUME_HSOUT]); > + break; > + case SND_SOC_DAPM_PRE_PMD: > + same > + /* decrease HS gain to minimum gain step by step */ > + hs_volume_ramp(priv, priv->ana_gain[ANALOG_VOLUME_HSOUT], DL_GAIN_N_40DB); > + /* Disable HS driver core circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON3, > + AUD_HS_PWRUP_VAUDP15_MASK, AUD_HS_PWRUP_VAUDP15_DISABLE); > + /* Disable HS driver bias circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON3, > + AUD_HS_PWRUP_BIAS_VAUDP15_MASK, > + AUD_HS_PWRUP_BIAS_VAUDP15_ENABLE); > + /* Clear HS STB enhance circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON3, > + AUD_HSOUT_STB_ENH_VAUDP15_MASK, > + AUD_HSOUT_STB_ENH_VAUDP15_DISABLE); > + break; > + default: > + break; > + } > + > + return 0; > +} > + ..snip.. > + > +static unsigned int mt6357_read(struct snd_soc_component *codec, unsigned int reg) > +{ > + struct mt6357_priv *priv = snd_soc_component_get_drvdata(codec); > + unsigned int val; > + > + pr_debug("%s() reg = 0x%x", __func__, reg); I'm not sure that you really need that pr_debug().... > + regmap_read(priv->regmap, reg, &val); > + return val; > +} > + > +static int mt6357_write(struct snd_soc_component *codec, unsigned int reg, unsigned int value) > +{ > + struct mt6357_priv *priv = snd_soc_component_get_drvdata(codec); > + > + pr_debug("%s() reg = 0x%x value= 0x%x\n", __func__, reg, value); ...nor this one. > + regmap_update_bits(priv->regmap, reg, 0xffff, value); > + return 0; return regmap_update_bits(...); > +} > + > +static const struct snd_soc_component_driver mt6357_soc_component_driver = { > + .probe = mt6357_codec_probe, > + .read = mt6357_read, > + .write = mt6357_write, > + .controls = mt6357_controls, > + .num_controls = ARRAY_SIZE(mt6357_controls), > + .dapm_widgets = mt6357_dapm_widgets, > + .num_dapm_widgets = ARRAY_SIZE(mt6357_dapm_widgets), > + .dapm_routes = mt6357_dapm_routes, > + .num_dapm_routes = ARRAY_SIZE(mt6357_dapm_routes), > +}; > + > +static void mt6357_parse_dt(struct mt6357_priv *priv) > +{ > + int ret, voltage_index; > + u32 micbias_voltage_index = 0; > + struct device *dev = priv->dev; > + > + priv->pull_down_needed = false; > + if (of_property_read_bool(dev->of_node, "mediatek,hp-pull-down")) > + priv->pull_down_needed = true; > + > + micbias_voltage_index = MT6357_MICBIAS0_DEFAULT_VOLTAGE_INDEX; > + ret = of_property_read_u32(dev->of_node, "mediatek,micbias0-microvolt", &voltage_index); > + if (!ret) > + micbias_voltage_index = voltage_index; > + > + regmap_update_bits(priv->regmap, MT6357_AUDENC_ANA_CON8, AUD_MICBIAS0_VREF_MASK, > + micbias_voltage_index << AUD_MICBIAS0_VREF_SFT); > + > + micbias_voltage_index = MT6357_MICBIAS1_DEFAULT_VOLTAGE_INDEX; > + ret = of_property_read_u32(dev->of_node, "mediatek,micbias1-microvolt", &voltage_index); > + if (!ret) > + micbias_voltage_index = voltage_index; > + > + regmap_update_bits(priv->regmap, MT6357_AUDENC_ANA_CON9, AUD_MICBIAS1_VREF_MASK, > + micbias_voltage_index << AUD_MICBIAS1_VREF_SFT); > +} > + > +static int mt6357_platform_driver_probe(struct platform_device *pdev) > +{ > + struct mt6357_priv *priv; > + struct mt6397_chip *mt6397 = dev_get_drvdata(pdev->dev.parent); struct mt6397_chip *mt6397 = dev_get_drvdata(pdev->dev.parent); struct mt6357_priv *priv; int ret; looks better :-) > + int ret; > + > + ret = devm_regulator_get_enable(&pdev->dev, "vaud28"); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, "failed to enable vaud28 regulator\n"); > + > + priv = devm_kzalloc(&pdev->dev, > + sizeof(struct mt6357_priv), > + GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + dev_set_drvdata(&pdev->dev, priv); > + priv->dev = &pdev->dev; > + > + priv->regmap = mt6397->regmap; > + if (IS_ERR(priv->regmap)) > + return PTR_ERR(priv->regmap); > + > + mt6357_parse_dt(priv); > + > + pdev->dev.coherent_dma_mask = DMA_BIT_MASK(64); > + if (!pdev->dev.dma_mask) > + pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask; > + > + return devm_snd_soc_register_component(&pdev->dev, > + &mt6357_soc_component_driver, > + mtk_6357_dai_codecs, > + ARRAY_SIZE(mtk_6357_dai_codecs)); > +} > + > +static const struct of_device_id mt6357_of_match[] = { > + {.compatible = "mediatek,mt6357-sound",}, > + {} { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, mt6357_of_match); > + > +static struct platform_driver mt6357_platform_driver = { > + .driver = { > + .name = "mt6357-sound", > + .of_match_table = mt6357_of_match, > + }, > + .probe = mt6357_platform_driver_probe, > +}; > + > +module_platform_driver(mt6357_platform_driver) > + > +MODULE_DESCRIPTION("MT6357 ALSA SoC codec driver"); > +MODULE_AUTHOR("Nicolas Belin <nbelin(a)baylibre.com>"); > +MODULE_LICENSE("GPL"); Cheers, Angelo

1 year, 11 months

1
0
0 0

Re: [PATCH 10/18] ASoc: mediatek: mt8365: Add a specific soundcard for EVK

by AngeloGioacchino Del Regno

Il 26/02/24 15:01, amergnat(a)baylibre.com ha scritto: > From: Nicolas Belin <nbelin(a)baylibre.com> > > Add a specific soundcard for mt8365-evk. It supports audio jack > in/out, dmics, the amic and lineout. > > Signed-off-by: Nicolas Belin <nbelin(a)baylibre.com> > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> > --- > sound/soc/mediatek/mt8365/mt8365-mt6357.c | 379 ++++++++++++++++++++++++++++++ > 1 file changed, 379 insertions(+) > > diff --git a/sound/soc/mediatek/mt8365/mt8365-mt6357.c b/sound/soc/mediatek/mt8365/mt8365-mt6357.c > new file mode 100644 > index 000000000000..5192b35458e6 > --- /dev/null > +++ b/sound/soc/mediatek/mt8365/mt8365-mt6357.c > @@ -0,0 +1,379 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Mediatek MT8365 Sound Card driver > + * > + * Copyright (c) 2024 MediaTek Inc. > + * Authors: Nicolas Belin <nbelin(a)baylibre.com> > + */ > + > +#include <linux/module.h> > +#include <linux/of_gpio.h> > +#include <sound/soc.h> > +#include <sound/pcm_params.h> > +#include "mt8365-afe-common.h" > +#include <linux/pinctrl/consumer.h> > +#include "../common/mtk-soundcard-driver.h" > + > +enum PINCTRL_PIN_STATE { Aren't those supposed to be AFE pin states? Anyway, lower case name for enums please, and no zero initializer for the first entry. enum mt8365_pin_state { PIN_STATE_DEFAULT, .... }; > + PIN_STATE_DEFAULT = 0, > + PIN_STATE_DMIC, > + PIN_STATE_MISO_OFF, > + PIN_STATE_MISO_ON, > + PIN_STATE_MOSI_OFF, > + PIN_STATE_MOSI_ON, > + PIN_STATE_MAX > +}; > + > +static const char * const mt8365_mt6357_pin_str[PIN_STATE_MAX] = { > + "aud_default", > + "aud_dmic", > + "aud_miso_off", > + "aud_miso_on", > + "aud_mosi_off", > + "aud_mosi_on", > +}; > + > +struct mt8365_mt6357_priv { > + struct pinctrl *pinctrl; > + struct pinctrl_state *pin_states[PIN_STATE_MAX]; > +}; > + > +struct mt8365_dai_link_prop { > + char *name; > + unsigned int link_id; > +}; This structure is unused. > + > +enum { > + /* FE */ > + DAI_LINK_DL1_PLAYBACK = 0, > + DAI_LINK_DL2_PLAYBACK, > + DAI_LINK_AWB_CAPTURE, > + DAI_LINK_VUL_CAPTURE, > + /* BE */ > + DAI_LINK_2ND_I2S_INTF, > + DAI_LINK_DMIC, > + DAI_LINK_INT_ADDA, > + DAI_LINK_NUM > +}; > + > +static const struct snd_soc_dapm_widget mt8365_mt6357_widgets[] = { > + SND_SOC_DAPM_OUTPUT("HDMI Out"), > +}; > + > +static const struct snd_soc_dapm_route mt8365_mt6357_routes[] = { > + {"HDMI Out", NULL, "2ND I2S Playback"}, > + {"DMIC In", NULL, "MICBIAS0"}, > +}; > + > +static int mt8365_mt6357_int_adda_startup(struct snd_pcm_substream *substream) > +{ > + struct snd_soc_pcm_runtime *rtd = substream->private_data; > + struct mt8365_mt6357_priv *priv = snd_soc_card_get_drvdata(rtd->card); > + int ret = 0; > + > + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { > + if (IS_ERR(priv->pin_states[PIN_STATE_MOSI_ON])) > + return ret; > + > + ret = pinctrl_select_state(priv->pinctrl, > + priv->pin_states[PIN_STATE_MOSI_ON]); > + if (ret) > + dev_err(rtd->card->dev, "%s failed to select state %d\n", > + __func__, ret); > + } > + > + if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) { > + if (IS_ERR(priv->pin_states[PIN_STATE_MISO_ON])) > + return ret; > + > + ret = pinctrl_select_state(priv->pinctrl, > + priv->pin_states[PIN_STATE_MISO_ON]); > + if (ret) > + dev_err(rtd->card->dev, "%s failed to select state %d\n", > + __func__, ret); > + } > + > + return 0; > +} > + > +static void mt8365_mt6357_int_adda_shutdown(struct snd_pcm_substream *substream) > +{ > + struct snd_soc_pcm_runtime *rtd = substream->private_data; > + struct mt8365_mt6357_priv *priv = snd_soc_card_get_drvdata(rtd->card); > + int ret = 0; > + > + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { > + if (IS_ERR(priv->pin_states[PIN_STATE_MOSI_OFF])) > + return; > + > + ret = pinctrl_select_state(priv->pinctrl, > + priv->pin_states[PIN_STATE_MOSI_OFF]); > + if (ret) > + dev_err(rtd->card->dev, "%s failed to select state %d\n", > + __func__, ret); > + } > + > + if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) { > + if (IS_ERR(priv->pin_states[PIN_STATE_MISO_OFF])) > + return; > + > + ret = pinctrl_select_state(priv->pinctrl, > + priv->pin_states[PIN_STATE_MISO_OFF]); > + if (ret) > + dev_err(rtd->card->dev, "%s failed to select state %d\n", > + __func__, ret); > + } > + > +} > + > +static const struct snd_soc_ops mt8365_mt6357_int_adda_ops = { > + .startup = mt8365_mt6357_int_adda_startup, > + .shutdown = mt8365_mt6357_int_adda_shutdown, > +}; > + > +SND_SOC_DAILINK_DEFS(playback1, > + DAILINK_COMP_ARRAY(COMP_CPU("DL1")), > + DAILINK_COMP_ARRAY(COMP_DUMMY()), > + DAILINK_COMP_ARRAY(COMP_EMPTY())); > +SND_SOC_DAILINK_DEFS(playback2, > + DAILINK_COMP_ARRAY(COMP_CPU("DL2")), > + DAILINK_COMP_ARRAY(COMP_DUMMY()), > + DAILINK_COMP_ARRAY(COMP_EMPTY())); > +SND_SOC_DAILINK_DEFS(awb_capture, > + DAILINK_COMP_ARRAY(COMP_CPU("AWB")), > + DAILINK_COMP_ARRAY(COMP_DUMMY()), > + DAILINK_COMP_ARRAY(COMP_EMPTY())); > +SND_SOC_DAILINK_DEFS(vul, > + DAILINK_COMP_ARRAY(COMP_CPU("VUL")), > + DAILINK_COMP_ARRAY(COMP_DUMMY()), > + DAILINK_COMP_ARRAY(COMP_EMPTY())); > + > +SND_SOC_DAILINK_DEFS(i2s3, > + DAILINK_COMP_ARRAY(COMP_CPU("2ND I2S")), > + DAILINK_COMP_ARRAY(COMP_DUMMY()), > + DAILINK_COMP_ARRAY(COMP_EMPTY())); > +SND_SOC_DAILINK_DEFS(dmic, > + DAILINK_COMP_ARRAY(COMP_CPU("DMIC")), > + DAILINK_COMP_ARRAY(COMP_DUMMY()), > + DAILINK_COMP_ARRAY(COMP_EMPTY())); > +SND_SOC_DAILINK_DEFS(primary_codec, > + DAILINK_COMP_ARRAY(COMP_CPU("INT ADDA")), > + DAILINK_COMP_ARRAY(COMP_CODEC("mt6357-sound", "mt6357-snd-codec-aif1")), > + DAILINK_COMP_ARRAY(COMP_EMPTY())); > + > +/* Digital audio interface glue - connects codec <---> CPU */ > +static struct snd_soc_dai_link mt8365_mt6357_dais[] = { > + /* Front End DAI links */ > + [DAI_LINK_DL1_PLAYBACK] = { > + .name = "DL1_FE", > + .stream_name = "MultiMedia1_PLayback", > + .id = DAI_LINK_DL1_PLAYBACK, > + .trigger = { > + SND_SOC_DPCM_TRIGGER_POST, > + SND_SOC_DPCM_TRIGGER_POST > + }, > + .dynamic = 1, > + .dpcm_playback = 1, > + .dpcm_merged_rate = 1, > + SND_SOC_DAILINK_REG(playback1), > + }, > + [DAI_LINK_DL2_PLAYBACK] = { > + .name = "DL2_FE", > + .stream_name = "MultiMedia2_PLayback", > + .id = DAI_LINK_DL2_PLAYBACK, > + .trigger = { > + SND_SOC_DPCM_TRIGGER_POST, > + SND_SOC_DPCM_TRIGGER_POST > + }, > + .dynamic = 1, > + .dpcm_playback = 1, > + .dpcm_merged_rate = 1, > + SND_SOC_DAILINK_REG(playback2), > + }, > + [DAI_LINK_AWB_CAPTURE] = { > + .name = "AWB_FE", > + .stream_name = "DL1_AWB_Record", > + .id = DAI_LINK_AWB_CAPTURE, > + .trigger = { > + SND_SOC_DPCM_TRIGGER_POST, > + SND_SOC_DPCM_TRIGGER_POST > + }, > + .dynamic = 1, > + .dpcm_capture = 1, > + .dpcm_merged_rate = 1, > + SND_SOC_DAILINK_REG(awb_capture), > + }, > + [DAI_LINK_VUL_CAPTURE] = { > + .name = "VUL_FE", > + .stream_name = "MultiMedia1_Capture", > + .id = DAI_LINK_VUL_CAPTURE, > + .trigger = { > + SND_SOC_DPCM_TRIGGER_POST, > + SND_SOC_DPCM_TRIGGER_POST > + }, > + .dynamic = 1, > + .dpcm_capture = 1, > + .dpcm_merged_rate = 1, > + SND_SOC_DAILINK_REG(vul), > + }, > + /* Back End DAI links */ > + [DAI_LINK_2ND_I2S_INTF] = { > + .name = "2ND I2S BE", > + .no_pcm = 1, > + .id = DAI_LINK_2ND_I2S_INTF, > + .dai_fmt = SND_SOC_DAIFMT_I2S | > + SND_SOC_DAIFMT_NB_NF | > + SND_SOC_DAIFMT_CBS_CFS, > + .dpcm_playback = 1, > + .dpcm_capture = 1, > + SND_SOC_DAILINK_REG(i2s3), > + }, > + [DAI_LINK_DMIC] = { > + .name = "DMIC BE", > + .no_pcm = 1, > + .id = DAI_LINK_DMIC, > + .dpcm_capture = 1, > + SND_SOC_DAILINK_REG(dmic), > + }, > + [DAI_LINK_INT_ADDA] = { > + .name = "MTK Codec", > + .no_pcm = 1, > + .id = DAI_LINK_INT_ADDA, > + .dpcm_playback = 1, > + .dpcm_capture = 1, > + .ops = &mt8365_mt6357_int_adda_ops, > + SND_SOC_DAILINK_REG(primary_codec), > + }, > +}; > + > +static int pinctrl_setup(struct snd_soc_card *card, > + struct mt8365_mt6357_priv *priv, > + unsigned int pin_id) > +{ > + int ret = 0; > + /* if pin exist */ > + if (!IS_ERR(priv->pin_states[pin_id])) { > + ret = pinctrl_select_state(priv->pinctrl, > + priv->pin_states[pin_id]); > + if (ret) > + dev_err(card->dev, > + "%s failed to select state %d\n", > + __func__, ret); > + } > + return ret; > +} > + > +static int mt8365_mt6357_gpio_probe(struct snd_soc_card *card) > +{ > + struct mt8365_mt6357_priv *priv = snd_soc_card_get_drvdata(card); > + int ret = 0; > + int i; > + > + priv->pinctrl = devm_pinctrl_get(card->dev); > + if (IS_ERR(priv->pinctrl)) { > + ret = PTR_ERR(priv->pinctrl); > + dev_err(card->dev, "%s devm_pinctrl_get failed %d\n", > + __func__, ret); > + return ret; > + } > + > + for (i = 0 ; i < PIN_STATE_MAX ; i++) { > + priv->pin_states[i] = pinctrl_lookup_state(priv->pinctrl, > + mt8365_mt6357_pin_str[i]); > + if (IS_ERR(priv->pin_states[i])) { > + ret = PTR_ERR(priv->pin_states[i]); > + dev_err(card->dev, "%s Can't find pin state %s %d\n", > + __func__, mt8365_mt6357_pin_str[i], ret); > + } > + } > + > + /* Setup pins */ for (i = PIN_STATE_DEFAULT; i < PIN_STATE_MAX; i++) { /* Are you sure about that?!?! */ if (IS_ERR(priv->pin_states[i])) continue; ret = pinctrl_select_state(...) if (ret) { /* Should you go back to default? */ pinctrl_select_state(card, priv, PIN_STATE_DEFAULT); return dev_err_probe(card->dev, ret, "ARGH!\n"); /* :-) */ } }; > + ret |= pinctrl_setup(card, priv, PIN_STATE_DEFAULT); ....because OR'in return values is a big no. > + ret |= pinctrl_setup(card, priv, PIN_STATE_DMIC); > + ret |= pinctrl_setup(card, priv, PIN_STATE_MISO_OFF); > + ret |= pinctrl_setup(card, priv, PIN_STATE_MOSI_OFF); > + return 0; > + return ret; > +} > + > +static struct snd_soc_card mt8365_mt6357_card = { > + .name = "mt8365-evk", > + .owner = THIS_MODULE, > + .dai_link = mt8365_mt6357_dais, > + .num_links = ARRAY_SIZE(mt8365_mt6357_dais), > + .dapm_widgets = mt8365_mt6357_widgets, > + .num_dapm_widgets = ARRAY_SIZE(mt8365_mt6357_widgets), > + .dapm_routes = mt8365_mt6357_routes, > + .num_dapm_routes = ARRAY_SIZE(mt8365_mt6357_routes), > +}; > + > +static int mt8365_mt6357_dev_probe(struct platform_device *pdev) > +{ > + struct snd_soc_card *card = &mt8365_mt6357_card; > + struct device *dev = &pdev->dev; > + struct device_node *platform_node; > + struct mt8365_mt6357_priv *priv; > + int i, ret; > + > + card->dev = dev; > + ret = parse_dai_link_info(card); > + if (ret) > + goto err; > + > + platform_node = of_parse_phandle(dev->of_node, "mediatek,platform", 0); > + if (!platform_node) { > + dev_err(dev, "Property 'platform' missing or invalid\n"); > + return -EINVAL; > + } > + > + for (i = 0; i < card->num_links; i++) { > + if (mt8365_mt6357_dais[i].platforms->name) > + continue; > + mt8365_mt6357_dais[i].platforms->of_node = platform_node; > + } > + > + priv = devm_kzalloc(dev, sizeof(struct mt8365_mt6357_priv), > + GFP_KERNEL); > + if (!priv) { > + ret = -ENOMEM; > + dev_err(dev, "%s allocate card private data fail %d\n", > + __func__, ret); > + return ret; > + } > + > + snd_soc_card_set_drvdata(card, priv); > + > + mt8365_mt6357_gpio_probe(card); P.S.: with my cleanup this function would not need any parse_dai_link_info() call, nor phandle parsing, nor platform_node assignment, nor any call to devm_snd_soc_register_card(), nor.... ...practically you'd need to only allocate your priv (granted that you are not moving the gpio_probe() to the AFE driver) and the actual gpio_probe() call. But I'm not pressing to wait, anyway. Cheers, Angelo

1 year, 11 months

1
0
0 0

Re: [PATCH 00/18] Add audio support for the MediaTek Genio 350-evk board

by AngeloGioacchino Del Regno

Il 26/02/24 15:01, Alexandre Mergnat ha scritto: > This serie aim to add the following audio support for the Genio 350-evk: > - Playback > - 2ch Headset Jack (Earphone) > - 1ch Line-out Jack (Speaker) > - 8ch HDMI Tx > - Capture > - 1ch DMIC (On-board Digital Microphone) > - 1ch AMIC (On-board Analogic Microphone) > - 1ch Headset Jack (External Analogic Microphone) > > Of course, HDMI playback need the MT8365 display patches [1] and a DTS > change documented in "mediatek,mt8365-mt6357.yaml". > > [1]: https://lore.kernel.org/all/20231023-display-support-v1-0-5c860ed5c33b@bayl… > > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> Actually, I am cooking a series (I'm finishing the testing....) that brings quite a bit of cleanups in MTK ASoC, including the commonization of the machine driver probe, with the dai-link DT nodes, and which also modernizes most of the existing drivers to use that instead. If you wait for a day or two, your mt8365-mt6357.c driver's probe function can be shrunk to ~3 lines or something like that.. very easily :-) Cheers, Angelo > --- > Alexandre Mergnat (15): > ASoC: dt-bindings: mediatek,mt8365-afe: Add audio afe document > ASoC: dt-bindings: mediatek,mt8365-mt6357: Add audio sound card document > dt-bindings: mfd: mediatek: Add codec property for MT6357 PMIC > ASoC: mediatek: mt8365: Add common header > SoC: mediatek: mt8365: support audio clock control > ASoC: mediatek: mt8365: Add I2S DAI support > ASoC: mediatek: mt8365: Add ADDA DAI support > ASoC: mediatek: mt8365: Add DMIC DAI support > ASoC: mediatek: mt8365: Add PCM DAI support > ASoC: mediatek: mt8365: Add platform driver > ASoC: mediatek: Add MT8365 support > arm64: defconfig: enable mt8365 sound > arm64: dts: mediatek: add mt6357 audio codec support > arm64: dts: mediatek: add afe support for mt8365 SoC > arm64: dts: mediatek: add audio support for mt8365-evk > > Fabien Parent (1): > mfd: mt6397-core: register mt6357 sound codec > > Nicolas Belin (2): > ASoc: mediatek: mt8365: Add a specific soundcard for EVK > ASoC: codecs: mt6357: add MT6357 codec > > .../devicetree/bindings/mfd/mediatek,mt6357.yaml | 11 + > .../bindings/sound/mediatek,mt8365-afe.yaml | 164 ++ > .../bindings/sound/mediatek,mt8365-mt6357.yaml | 127 ++ > arch/arm64/boot/dts/mediatek/mt6357.dtsi | 6 +- > arch/arm64/boot/dts/mediatek/mt8365-evk.dts | 95 +- > arch/arm64/boot/dts/mediatek/mt8365.dtsi | 47 +- > arch/arm64/configs/defconfig | 2 + > drivers/mfd/mt6397-core.c | 3 + > sound/soc/codecs/Kconfig | 7 + > sound/soc/codecs/Makefile | 2 + > sound/soc/codecs/mt6357.c | 1805 +++++++++++++++ > sound/soc/codecs/mt6357.h | 674 ++++++ > sound/soc/mediatek/Kconfig | 20 + > sound/soc/mediatek/Makefile | 1 + > sound/soc/mediatek/mt8365/Makefile | 15 + > sound/soc/mediatek/mt8365/mt8365-afe-clk.c | 455 ++++ > sound/soc/mediatek/mt8365/mt8365-afe-clk.h | 55 + > sound/soc/mediatek/mt8365/mt8365-afe-common.h | 495 +++++ > sound/soc/mediatek/mt8365/mt8365-afe-pcm.c | 2347 ++++++++++++++++++++ > sound/soc/mediatek/mt8365/mt8365-dai-adda.c | 355 +++ > sound/soc/mediatek/mt8365/mt8365-dai-dmic.c | 475 ++++ > sound/soc/mediatek/mt8365/mt8365-dai-i2s.c | 901 ++++++++ > sound/soc/mediatek/mt8365/mt8365-dai-pcm.c | 298 +++ > sound/soc/mediatek/mt8365/mt8365-mt6357.c | 379 ++++ > sound/soc/mediatek/mt8365/mt8365-reg.h | 987 ++++++++ > 25 files changed, 9718 insertions(+), 8 deletions(-) > --- > base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d > change-id: 20240226-audio-i350-4e11da088e55 > > Best regards,

1 year, 11 months

1
0
0 0

Re: [PATCH 11/11] drm/mediatek: Rename "pending_needs_vblank" to "needs_vblank"

by AngeloGioacchino Del Regno

Il 26/02/24 09:50, Shawn Sung ha scritto: > From: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > > Rename "pending_needs_vblank" to "needs_vblank" to reduce the code size. > This decreases human readability though, so I would prefer that we omit this one. Regards, Angelo > Signed-off-by: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > --- > drivers/gpu/drm/mediatek/mtk_crtc.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.c b/drivers/gpu/drm/mediatek/mtk_crtc.c > index 7fe234de83a3..a1fb6c67681d 100644 > --- a/drivers/gpu/drm/mediatek/mtk_crtc.c > +++ b/drivers/gpu/drm/mediatek/mtk_crtc.c > @@ -42,7 +42,7 @@ struct mtk_crtc { > struct drm_crtc base; > bool enabled; > > - bool pending_needs_vblank; > + bool needs_vblank; > struct drm_pending_vblank_event *event; > > struct drm_plane *planes; > @@ -105,9 +105,9 @@ static void mtk_crtc_finish_page_flip(struct mtk_crtc *mtk_crtc) > static void mtk_drm_finish_page_flip(struct mtk_crtc *mtk_crtc) > { > drm_crtc_handle_vblank(&mtk_crtc->base); > - if (!mtk_crtc->config_updating && mtk_crtc->pending_needs_vblank) { > + if (!mtk_crtc->config_updating && mtk_crtc->needs_vblank) { > mtk_crtc_finish_page_flip(mtk_crtc); > - mtk_crtc->pending_needs_vblank = false; > + mtk_crtc->needs_vblank = false; > } > } > > @@ -571,7 +571,7 @@ static void mtk_crtc_update_config(struct mtk_crtc *mtk_crtc, bool needs_vblank) > mutex_lock(&mtk_crtc->hw_lock); > mtk_crtc->config_updating = true; > if (needs_vblank) > - mtk_crtc->pending_needs_vblank = true; > + mtk_crtc->needs_vblank = true; > > for (i = 0; i < mtk_crtc->layer_nr; i++) { > struct drm_plane *plane = &mtk_crtc->planes[i];

1 year, 11 months

1
0
0 0

Re: [PATCH 05/11] drm/mediatek: Rename "mtk_drm_hdmi" to "mtk_hdmi"

by AngeloGioacchino Del Regno

Il 26/02/24 09:50, Shawn Sung ha scritto: > From: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > > Rename all "mtk_drm_hdmi" to "mtk_hdmi": > - To align the naming rule > - To reduce the code size > > Signed-off-by: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> Reviewed-by: AngeloGiaocchino Del Regno <angelogioacchino.delregno(a)collabora.com>

1 year, 11 months

1
0
0 0

Re: [PATCH 01/11] drm/mediatek: Rename "mtk_drm_crtc" to "mtk_crtc"

by AngeloGioacchino Del Regno

Il 26/02/24 09:50, Shawn Sung ha scritto: > From: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > > Rename all "mtk_drm_crtc" to "mtk_crtc" due to the following benefits: > - Lower the matches when searching the native drm_crtc* codes > - Reduce the code size > > Signed-off-by: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> Reviewed-by: AngeloGiaocchino Del Regno <angelogioacchino.delregno(a)collabora.com>

1 year, 11 months

1
0
0 0

Re: [PATCH 02/11] drm/mediatek: Rename "mtk_drm_ddp_comp" to "mtk_ddp_comp"

by AngeloGioacchino Del Regno

Il 26/02/24 09:50, Shawn Sung ha scritto: > From: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > > Rename all "mtk_drm_ddp_comp" to "mtk_ddp_comp": > - To align the naming rule > - To reduce the code size > > Signed-off-by: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> Reviewed-by: AngeloGiaocchino Del Regno <angelogioacchino.delregno(a)collabora.com>

1 year, 11 months

1
0
0 0

Re: [PATCH 03/11] drm/mediatek: Rename "mtk_drm_plane" to "mtk_plane"

by AngeloGioacchino Del Regno

Il 26/02/24 09:50, Shawn Sung ha scritto: > From: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > > Rename all "mtk_drm_plane" to "mtk_plane": > - To align the naming rule > - To reduce the code size > > Signed-off-by: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> Reviewed-by: AngeloGiaocchino Del Regno <angelogioacchino.delregno(a)collabora.com>

1 year, 11 months

1
0
0 0

Re: [PATCH 04/11] drm/mediatek: Rename "mtk_drm_gem" to "mtk_gem"

by AngeloGioacchino Del Regno

Il 26/02/24 09:50, Shawn Sung ha scritto: > From: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > > Rename all "mtk_drm_gem" to "mtk_gem": > - To align the naming rule > - To reduce the code size > > Signed-off-by: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> Reviewed-by: AngeloGiaocchino Del Regno <angelogioacchino.delregno(a)collabora.com>

1 year, 11 months

1
0
0 0

Re: [PATCH 06/11] drm/mediatek: Rename files "mtk_drm_crtc.*" to "mtk_crtc.*"

by AngeloGioacchino Del Regno

Il 26/02/24 09:50, Shawn Sung ha scritto: > From: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > > Rename files mtk_drm_crtc.* to mtk_crtc.*. > > Signed-off-by: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> Reviewed-by: AngeloGiaocchino Del Regno <angelogioacchino.delregno(a)collabora.com>

1 year, 11 months

1
0
0 0

Re: [PATCH 07/11] drm/mediatek: Rename files "mtk_drm_ddp_comp.*" to "mtk_ddp_comp.*"

by AngeloGioacchino Del Regno

Il 26/02/24 09:50, Shawn Sung ha scritto: > From: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > > Rename files mtk_drm_ddp_comp.* to mtk_ddp_comp.*. > > Signed-off-by: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> Reviewed-by: AngeloGiaocchino Del Regno <angelogioacchino.delregno(a)collabora.com>

1 year, 11 months

1
0
0 0

Re: [PATCH 08/11] drm/mediatek: Rename files "mtk_drm_plane.*" to "mtk_plane.*"

by AngeloGioacchino Del Regno

Il 26/02/24 09:50, Shawn Sung ha scritto: > From: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > > Rename files mtk_drm_plane.* to mtk_plane.*. > > Signed-off-by: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> Reviewed-by: AngeloGiaocchino Del Regno <angelogioacchino.delregno(a)collabora.com>

1 year, 11 months

1
0
0 0

Re: [PATCH 09/11] drm/mediatek: Rename files "mtk_drm_gem.*" to "mtk_gem.*"

by AngeloGioacchino Del Regno

Il 26/02/24 09:50, Shawn Sung ha scritto: > From: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > > Rename files mtk_drm_gem.* to mtk_gem.*. > > Signed-off-by: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> Reviewed-by: AngeloGiaocchino Del Regno <angelogioacchino.delregno(a)collabora.com>

1 year, 11 months

1
0
0 0

Re: [PATCH 10/11] drm/mediatek: Rename mtk_ddp_comp functions

by AngeloGioacchino Del Regno

Il 26/02/24 09:50, Shawn Sung ha scritto: > From: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > > Rename functions of mtk_ddp_comp: > - To align the naming rule > - To reduce the code size > > Signed-off-by: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> Reviewed-by: AngeloGiaocchino Del Regno <angelogioacchino.delregno(a)collabora.com>

1 year, 11 months

1
0
0 0

Re: [PATCH v5] drm/test: add a test suite for GEM objects backed by shmem

by Guenter Roeck

On Thu, Feb 22, 2024 at 05:33:48PM +0100, Marco Pagani wrote: > > > > In this context, the TTM unit tests fail as well in qemu, with worse result: > > It seems there is some bad cleanup after a failed test case, causing list > > corruptions in the drm core and ultimately a crash. I don't know if this > > is also caused by the missing dma_mask initialization. > > > > That's interesting. Which --arch argument are you using to run the > tests with QEMU? Example (I am not sure if any of those parameters matters; it is just one of my tests): qemu-system-x86_64 -kernel arch/x86/boot/bzImage -M q35 -cpu IvyBridge \ -no-reboot -snapshot -smp 2 \ -device e1000,netdev=net0 -netdev user,id=net0 -m 512 \ -drive file=rootfs.ext2,format=raw,if=ide \ --append "earlycon=uart8250,io,0x3f8,9600n8 root=/dev/sda1 console=ttyS0" \ -d unimp,guest_errors -nographic -monitor none This results in: [ ... ] [ 5.989496] KTAP version 1 [ 5.989639] # Subtest: ttm_device [ 5.989711] # module: ttm_device_test [ 5.989760] 1..5 [ 6.002044] ok 1 ttm_device_init_basic [ 6.013557] ok 2 ttm_device_init_multiple ILLOPC: ffffffffb8ac9350: 0f 0b [ 6.022481] ok 3 ttm_device_fini_basic [ 6.026172] ------------[ cut here ]------------ [ 6.026315] WARNING: CPU: 1 PID: 1575 at drivers/gpu/drm/ttm/ttm_device.c:206 ttm_device_init+0x170/0x190 ... [ 6.135016] ok 3 Above the allocation limit [ 6.138759] ------------[ cut here ]------------ [ 6.138925] WARNING: CPU: 1 PID: 1595 at kernel/dma/mapping.c:503 dma_alloc_attrs+0xf6/0x100 ... [ 6.143850] # ttm_pool_alloc_basic: ASSERTION FAILED at drivers/gpu/drm/ttm/tests/ttm_pool_test.c:162 [ 6.143850] Expected err == 0, but [ 6.143850] err == -12 (0xfffffffffffffff4) [ 6.148824] not ok 4 One page, with coherent DMA mappings enabled From there things go downhill. [ 6.152821] list_add corruption. prev->next should be next (ffffffffbbd53950), but was 0000000000000000. (prev=ffff8af1c38f9e20). and so on until the emulation crashes. Guenter

1 year, 11 months

1
0
0 0

Re: [PATCH v5] drm/test: add a test suite for GEM objects backed by shmem

by Guenter Roeck

Hi Marco, On 2/22/24 07:32, Marco Pagani wrote: > > > On 2024-02-18 16:49, Guenter Roeck wrote: >> Hi, >> >> On Thu, Nov 30, 2023 at 06:14:16PM +0100, Marco Pagani wrote: >>> This patch introduces an initial KUnit test suite for GEM objects >>> backed by shmem buffers. >>> >>> Suggested-by: Javier Martinez Canillas <javierm(a)redhat.com> >>> Signed-off-by: Marco Pagani <marpagan(a)redhat.com> >> >> When running this in qemu, I get lots of warnings backtraces in the drm >> core. >> >> WARNING: CPU: 0 PID: 1341 at drivers/gpu/drm/drm_gem_shmem_helper.c:327 >> WARNING: CPU: 0 PID: 1341 at drivers/gpu/drm/drm_gem_shmem_helper.c:173 >> WARNING: CPU: 0 PID: 1341 at drivers/gpu/drm/drm_gem_shmem_helper.c:385 >> WARNING: CPU: 0 PID: 1341 at drivers/gpu/drm/drm_gem_shmem_helper.c:211 >> WARNING: CPU: 0 PID: 1345 at kernel/dma/mapping.c:194 >> WARNING: CPU: 0 PID: 1347 at drivers/gpu/drm/drm_gem_shmem_helper.c:429 >> WARNING: CPU: 0 PID: 1349 at drivers/gpu/drm/drm_gem_shmem_helper.c:445 >> >> It looks like dma_resv_assert_held() asserts each time it is executed. >> The backtrace in kernel/dma/mapping.c is triggered by >> if (WARN_ON_ONCE(!dev->dma_mask)) >> return 0; >> in __dma_map_sg_attrs(). >> >> Is this a possible problem in the test code, or can it be caused by >> some limitations or bugs in the qemu emulation ? If so, do you have any >> thoughts or ideas what those limitations / bugs might be ? > > Hi Guenter, > > Thanks for reporting this issue. As you correctly noted, the warnings appear to > be caused by the dma_mask in the mock device being uninitialized. I'll send a > patch to fix it soon. > Thanks a lot for the update. In this context, the TTM unit tests fail as well in qemu, with worse result: It seems there is some bad cleanup after a failed test case, causing list corruptions in the drm core and ultimately a crash. I don't know if this is also caused by the missing dma_mask initialization. Thanks, Guenter

1 year, 11 months

1
0
0 0

[PATCH v2] udmabuf: Fix a potential (and unlikely) access to unallocated memory

by Christophe JAILLET

If 'list_limit' is set to a very high value, 'lsize' computation could overflow if 'head.count' is big enough. In such a case, udmabuf_create() would access to memory beyond 'list'. Use memdup_array_user() which checks for overflow. While at it, include <linux/string.h>. Fixes: fbb0de795078 ("Add udmabuf misc device") Signed-off-by: Christophe JAILLET <christophe.jaillet(a)wanadoo.fr> --- v2: - Use memdup_array_user() [Kees Cook] - Use sizeof(*list) [Gustavo A. R. Silva] - Add include <linux/string.h> v1: https://lore.kernel.org/all/3e37f05c7593f1016f0a46de188b3357cbbd0c0b.169506… Signed-off-by: Christophe JAILLET <christophe.jaillet(a)wanadoo.fr> --- drivers/dma-buf/udmabuf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index c40645999648..5728948ea6f2 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -11,6 +11,7 @@ #include <linux/module.h> #include <linux/shmem_fs.h> #include <linux/slab.h> +#include <linux/string.h> #include <linux/udmabuf.h> #include <linux/vmalloc.h> #include <linux/iosys-map.h> @@ -314,14 +315,13 @@ static long udmabuf_ioctl_create_list(struct file *filp, unsigned long arg) struct udmabuf_create_list head; struct udmabuf_create_item *list; int ret = -EINVAL; - u32 lsize; if (copy_from_user(&head, (void __user *)arg, sizeof(head))) return -EFAULT; if (head.count > list_limit) return -EINVAL; - lsize = sizeof(struct udmabuf_create_item) * head.count; - list = memdup_user((void __user *)(arg + sizeof(head)), lsize); + list = memdup_array_user((void __user *)(arg + sizeof(head)), + sizeof(*list), head.count); if (IS_ERR(list)) return PTR_ERR(list); -- 2.43.2

1 year, 12 months

3
4
0 0

Re: [PATCH v5] drm/test: add a test suite for GEM objects backed by shmem

by Guenter Roeck

Hi, On Thu, Nov 30, 2023 at 06:14:16PM +0100, Marco Pagani wrote: > This patch introduces an initial KUnit test suite for GEM objects > backed by shmem buffers. > > Suggested-by: Javier Martinez Canillas <javierm(a)redhat.com> > Signed-off-by: Marco Pagani <marpagan(a)redhat.com> When running this in qemu, I get lots of warnings backtraces in the drm core. WARNING: CPU: 0 PID: 1341 at drivers/gpu/drm/drm_gem_shmem_helper.c:327 WARNING: CPU: 0 PID: 1341 at drivers/gpu/drm/drm_gem_shmem_helper.c:173 WARNING: CPU: 0 PID: 1341 at drivers/gpu/drm/drm_gem_shmem_helper.c:385 WARNING: CPU: 0 PID: 1341 at drivers/gpu/drm/drm_gem_shmem_helper.c:211 WARNING: CPU: 0 PID: 1345 at kernel/dma/mapping.c:194 WARNING: CPU: 0 PID: 1347 at drivers/gpu/drm/drm_gem_shmem_helper.c:429 WARNING: CPU: 0 PID: 1349 at drivers/gpu/drm/drm_gem_shmem_helper.c:445 It looks like dma_resv_assert_held() asserts each time it is executed. The backtrace in kernel/dma/mapping.c is triggered by if (WARN_ON_ONCE(!dev->dma_mask)) return 0; in __dma_map_sg_attrs(). Is this a possible problem in the test code, or can it be caused by some limitations or bugs in the qemu emulation ? If so, do you have any thoughts or ideas what those limitations / bugs might be ? Thanks, Guenter

1 year, 12 months

1
0
0 0

[PATCH] dma-buf: try to catch swiotlb bounce buffers

by Daniel Vetter

They rather fundamentally break the entire concept of zero copy, so if an exporter manages to hand these out things will break all over. Luckily there's not too many case that use swiotlb_sync_single_for_device/cpu(): - The generic iommu dma-api code in drivers/iommu/dma-iommu.c. We can catch that with sg_dma_is_swiotlb() reliably. - The generic direct dma code in kernel/dma/direct.c. We can mostly catch that with looking for a NULL dma_ops, except for some powerpc special cases. - Xen, which I don't bother to catch here. Implement these checks in dma_buf_map_attachment when CONFIG_DMA_API_DEBUG is enabled. Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: "Christian König" <christian.koenig(a)amd.com> Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: Paul Cercueil <paul(a)crapouillou.net> --- Entirely untested, but since I sent the mail with the idea I figured I might as well type it up after I realized there's a lot fewer cases to check. That is, if I haven't completely misread the dma-api and swiotlb code. -Sima --- drivers/dma-buf/dma-buf.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index d1e7f823fbdb..d6f95523f995 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -28,6 +28,12 @@ #include <linux/mount.h> #include <linux/pseudo_fs.h> +#ifdef CONFIG_DMA_API_DEBUG +#include <linux/dma-direct.h> +#include <linux/dma-map-ops.h> +#include <linux/swiotlb.h> +#endif + #include <uapi/linux/dma-buf.h> #include <uapi/linux/magic.h> @@ -1149,10 +1155,13 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, #ifdef CONFIG_DMA_API_DEBUG if (!IS_ERR(sg_table)) { struct scatterlist *sg; + struct device *dev = attach->dev; u64 addr; int len; int i; + bool is_direct_dma = !get_dma_ops(dev); + for_each_sgtable_dma_sg(sg_table, sg, i) { addr = sg_dma_address(sg); len = sg_dma_len(sg); @@ -1160,7 +1169,15 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, pr_debug("%s: addr %llx or len %x is not page aligned!\n", __func__, addr, len); } + + if (is_direct_dma) { + phys_addr_t paddr = dma_to_phys(dev, addr); + + WARN_ON_ONCE(is_swiotlb_buffer(dev, paddr)); + } } + + WARN_ON_ONCE(sg_dma_is_swiotlb(sg)); } #endif /* CONFIG_DMA_API_DEBUG */ return sg_table; -- 2.43.0

2 years

1
0
0 0

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig