Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

2 participants
2906 discussions

Re: [PATCH v4 05/16] SoC: mediatek: mt8365: support audio clock control

by AngeloGioacchino Del Regno

Il 26/04/24 19:22, Alexandre Mergnat ha scritto: > Add audio clock wrapper and audio tuner control. > > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> > --- > sound/soc/mediatek/mt8365/mt8365-afe-clk.c | 443 +++++++++++++++++++++++++++++ > sound/soc/mediatek/mt8365/mt8365-afe-clk.h | 49 ++++ > 2 files changed, 492 insertions(+) > > diff --git a/sound/soc/mediatek/mt8365/mt8365-afe-clk.c b/sound/soc/mediatek/mt8365/mt8365-afe-clk.c > new file mode 100644 > index 000000000000..3a525dae857c > --- /dev/null > +++ b/sound/soc/mediatek/mt8365/mt8365-afe-clk.c > @@ -0,0 +1,443 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Mediatek 8365 AFE clock control > + * > + * Copyright (c) 2024 MediaTek Inc. > + * Authors: Jia Zeng <jia.zeng(a)mediatek.com> > + * Alexandre Mergnat <amergnat(a)baylibre.com> > + */ > + > +#include "mt8365-afe-clk.h" > +#include "mt8365-afe-common.h" > +#include "mt8365-reg.h" > +#include "../common/mtk-base-afe.h" > +#include <linux/device.h> > +#include <linux/mfd/syscon.h> > + > +static const char *aud_clks[MT8365_CLK_NUM] = { > + [MT8365_CLK_TOP_AUD_SEL] = "top_audio_sel", > + [MT8365_CLK_AUD_I2S0_M] = "audio_i2s0_m", > + [MT8365_CLK_AUD_I2S1_M] = "audio_i2s1_m", > + [MT8365_CLK_AUD_I2S2_M] = "audio_i2s2_m", > + [MT8365_CLK_AUD_I2S3_M] = "audio_i2s3_m", > + [MT8365_CLK_ENGEN1] = "engen1", > + [MT8365_CLK_ENGEN2] = "engen2", > + [MT8365_CLK_AUD1] = "aud1", > + [MT8365_CLK_AUD2] = "aud2", > + [MT8365_CLK_I2S0_M_SEL] = "i2s0_m_sel", > + [MT8365_CLK_I2S1_M_SEL] = "i2s1_m_sel", > + [MT8365_CLK_I2S2_M_SEL] = "i2s2_m_sel", > + [MT8365_CLK_I2S3_M_SEL] = "i2s3_m_sel", > + [MT8365_CLK_CLK26M] = "top_clk26m_clk", > +}; > + > +int mt8365_afe_init_audio_clk(struct mtk_base_afe *afe) > +{ > + size_t i; > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + > + for (i = 0; i < ARRAY_SIZE(aud_clks); i++) { > + afe_priv->clocks[i] = devm_clk_get(afe->dev, aud_clks[i]); > + if (IS_ERR(afe_priv->clocks[i])) { > + dev_err(afe->dev, "%s devm_clk_get %s fail\n", > + __func__, aud_clks[i]); > + return PTR_ERR(afe_priv->clocks[i]); > + } > + } > + return 0; > +} > + > +int mt8365_afe_enable_clk(struct mtk_base_afe *afe, struct clk *clk) Do you really really really need those helper functions? I say that you can simply call the clk API from the users instead, as you are really just doing that in those functions... > +{ > + int ret; > + > + if (clk) { > + ret = clk_prepare_enable(clk); > + if (ret) { > + dev_err(afe->dev, "Failed to enable clk\n"); > + return ret; > + } > + } > + return 0; > +} > + > +void mt8365_afe_disable_clk(struct mtk_base_afe *afe, struct clk *clk) > +{ > + if (clk) > + clk_disable_unprepare(clk); > +} > + > +int mt8365_afe_set_clk_rate(struct mtk_base_afe *afe, struct clk *clk, > + unsigned int rate) > +{ > + int ret; > + > + if (clk) { > + ret = clk_set_rate(clk, rate); > + if (ret) { > + dev_err(afe->dev, "Failed to set rate\n"); > + return ret; > + } > + } > + return 0; > +} > + > +int mt8365_afe_set_clk_parent(struct mtk_base_afe *afe, struct clk *clk, > + struct clk *parent) > +{ > + int ret; > + > + if (clk && parent) { > + ret = clk_set_parent(clk, parent); > + if (ret) { > + dev_err(afe->dev, "Failed to set parent\n"); > + return ret; > + } > + } > + return 0; > +} > + ..snip.. > +int mt8365_afe_enable_apll_tuner_cfg(struct mtk_base_afe *afe, unsigned int apll) > +{ > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + > + mutex_lock(&afe_priv->afe_clk_mutex); > + > + afe_priv->apll_tuner_ref_cnt[apll]++; > + if (afe_priv->apll_tuner_ref_cnt[apll] != 1) { > + mutex_unlock(&afe_priv->afe_clk_mutex); > + return 0; > + } > + > + if (apll == MT8365_AFE_APLL1) { > + regmap_update_bits(afe->regmap, AFE_APLL_TUNER_CFG, > + AFE_APLL_TUNER_CFG_MASK, 0x432); > + regmap_update_bits(afe->regmap, AFE_APLL_TUNER_CFG, > + AFE_APLL_TUNER_CFG_EN_MASK, 0x1); > + } else { > + regmap_update_bits(afe->regmap, AFE_APLL_TUNER_CFG1, > + AFE_APLL_TUNER_CFG1_MASK, 0x434); > + regmap_update_bits(afe->regmap, AFE_APLL_TUNER_CFG1, > + AFE_APLL_TUNER_CFG1_EN_MASK, 0x1); > + } > + > + mutex_unlock(&afe_priv->afe_clk_mutex); > + return 0; > +} > + > +int mt8365_afe_disable_apll_tuner_cfg(struct mtk_base_afe *afe, unsigned int apll) There's a tabulation here, please use spaces.... > +{ > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + > + mutex_lock(&afe_priv->afe_clk_mutex); > + > + afe_priv->apll_tuner_ref_cnt[apll]--; > + if (afe_priv->apll_tuner_ref_cnt[apll] == 0) { > + if (apll == MT8365_AFE_APLL1) > + regmap_update_bits(afe->regmap, AFE_APLL_TUNER_CFG, > + AFE_APLL_TUNER_CFG_EN_MASK, 0x0); > + else > + regmap_update_bits(afe->regmap, AFE_APLL_TUNER_CFG1, > + AFE_APLL_TUNER_CFG1_EN_MASK, 0x0); > + > + } else if (afe_priv->apll_tuner_ref_cnt[apll] < 0) { > + afe_priv->apll_tuner_ref_cnt[apll] = 0; > + } > + > + mutex_unlock(&afe_priv->afe_clk_mutex); > + return 0; > +} > + > +int mt8365_afe_enable_apll_associated_cfg(struct mtk_base_afe *afe, unsigned int apll) > +{ > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + > + if (apll == MT8365_AFE_APLL1) { > + if (clk_prepare_enable(afe_priv->clocks[MT8365_CLK_ENGEN1])) { > + dev_info(afe->dev, "%s Failed to enable ENGEN1 clk\n", > + __func__); > + return 0; > + } > + mt8365_afe_enable_top_cg(afe, MT8365_TOP_CG_22M); > + mt8365_afe_hd_engen_enable(afe, true); > +#ifdef ENABLE_AFE_APLL_TUNER > + mt8365_afe_enable_top_cg(afe, MT8365_TOP_CG_APLL_TUNER); > + mt8365_afe_enable_apll_tuner_cfg(afe, MT8365_AFE_APLL1); > +#endif > + } else { > + if (clk_prepare_enable(afe_priv->clocks[MT8365_CLK_ENGEN2])) { > + dev_info(afe->dev, "%s Failed to enable ENGEN2 clk\n", > + __func__); > + return 0; > + } > + mt8365_afe_enable_top_cg(afe, MT8365_TOP_CG_24M); > + mt8365_afe_hd_engen_enable(afe, false); > +#ifdef ENABLE_AFE_APLL_TUNER Those ifdefs are ugly. If you're not using that code, please remove. > + mt8365_afe_enable_top_cg(afe, MT8365_TOP_CG_APLL2_TUNER); > + mt8365_afe_enable_apll_tuner_cfg(afe, MT8365_AFE_APLL2); > +#endif > + } > + > + return 0; > +} > + > +int mt8365_afe_disable_apll_associated_cfg(struct mtk_base_afe *afe, unsigned int apll) > +{ > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + > + if (apll == MT8365_AFE_APLL1) { > +#ifdef ENABLE_AFE_APLL_TUNER > + mt8365_afe_disable_apll_tuner_cfg(afe, MT8365_AFE_APLL1); > + mt8365_afe_disable_top_cg(afe, MT8365_TOP_CG_APLL_TUNER); > +#endif > + mt8365_afe_hd_engen_disable(afe, true); > + mt8365_afe_disable_top_cg(afe, MT8365_TOP_CG_22M); > + clk_disable_unprepare(afe_priv->clocks[MT8365_CLK_ENGEN1]); > + } else { > +#ifdef ENABLE_AFE_APLL_TUNER > + mt8365_afe_disable_apll_tuner_cfg(afe, MT8365_AFE_APLL2); > + mt8365_afe_disable_top_cg(afe, MT8365_TOP_CG_APLL2_TUNER); > +#endif > + mt8365_afe_hd_engen_disable(afe, false); > + mt8365_afe_disable_top_cg(afe, MT8365_TOP_CG_24M); > + clk_disable_unprepare(afe_priv->clocks[MT8365_CLK_ENGEN2]); > + } > + > + return 0; > +} > diff --git a/sound/soc/mediatek/mt8365/mt8365-afe-clk.h b/sound/soc/mediatek/mt8365/mt8365-afe-clk.h > new file mode 100644 > index 000000000000..14fca6ae2641 > --- /dev/null > +++ b/sound/soc/mediatek/mt8365/mt8365-afe-clk.h > @@ -0,0 +1,49 @@ > +/* SPDX-License-Identifier: GPL-2.0 > + * > + * Mediatek 8365 AFE clock control definitions > + * > + * Copyright (c) 2024 MediaTek Inc. > + * Authors: Jia Zeng <jia.zeng(a)mediatek.com> > + * Alexandre Mergnat <amergnat(a)baylibre.com> > + */ > + > +#ifndef _MT8365_AFE_UTILS_H_ > +#define _MT8365_AFE_UTILS_H_ > + > +struct mtk_base_afe; > +struct clk; > + > +int mt8365_afe_init_audio_clk(struct mtk_base_afe *afe); > + Please drop all those blank lines between function signatures, they're not needed. Cheers, Angelo > +int mt8365_afe_enable_clk(struct mtk_base_afe *afe, struct clk *clk); > + > +void mt8365_afe_disable_clk(struct mtk_base_afe *afe, struct clk *clk); > + > +int mt8365_afe_set_clk_rate(struct mtk_base_afe *afe, struct clk *clk, unsigned int rate); > + > +int mt8365_afe_set_clk_parent(struct mtk_base_afe *afe, struct clk *clk, struct clk *parent); > + > +int mt8365_afe_enable_top_cg(struct mtk_base_afe *afe, unsigned int cg_type); > + > +int mt8365_afe_disable_top_cg(struct mtk_base_afe *afe, unsigned int cg_type); > + > +int mt8365_afe_enable_main_clk(struct mtk_base_afe *afe); > + > +int mt8365_afe_disable_main_clk(struct mtk_base_afe *afe); > + > +int mt8365_afe_emi_clk_on(struct mtk_base_afe *afe); > + > +int mt8365_afe_emi_clk_off(struct mtk_base_afe *afe); > + > +int mt8365_afe_enable_afe_on(struct mtk_base_afe *afe); > + > +int mt8365_afe_disable_afe_on(struct mtk_base_afe *afe); > + > +int mt8365_afe_enable_apll_tuner_cfg(struct mtk_base_afe *afe, unsigned int apll); > + > +int mt8365_afe_disable_apll_tuner_cfg(struct mtk_base_afe *afe, unsigned int apll); > + > +int mt8365_afe_enable_apll_associated_cfg(struct mtk_base_afe *afe, unsigned int apll); > + > +int mt8365_afe_disable_apll_associated_cfg(struct mtk_base_afe *afe, unsigned int apll); > +#endif >

1 year, 1 month

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

by AngeloGioacchino Del Regno

Il 26/04/24 19:22, Alexandre Mergnat ha scritto: > Add mt8365 platform driver. Since you have to anyway send a v5: Add a driver for the Analog Front End (AFE) PCM blahblah MT8365 blahblah :-) after which Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno(a)collabora.com>

1 year, 1 month

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

by AngeloGioacchino Del Regno

Il 26/04/24 19:22, 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 | 348 ++++++++++++++++++++++++++++++ > 1 file changed, 348 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..a65f668103da > --- /dev/null > +++ b/sound/soc/mediatek/mt8365/mt8365-mt6357.c > @@ -0,0 +1,348 @@ > +// 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-soc-card.h" > +#include "../common/mtk-soundcard-driver.h" > + ..snip.. > + > +static const struct mtk_soundcard_pdata mt8365_mt6357_card = { > + .card_name = "mt8365-mt6357", > + .card_data = &(struct mtk_platform_card_data) { > + .card = &mt8365_mt6357_soc_card, > + }, > + .soc_probe = mt8365_mt6357_dev_probe > +}; > + > +static const struct of_device_id mt8365_mt6357_dt_match[] = { > + { > + .compatible = "mediatek,mt8365-mt6357", > + .data = &mt8365_mt6357_card, > + }, > + { } Please, compress and add sentinel { .compatible = "mediatek,mt8365-mt6357", .data = &mt8365_mt6357_card }, { /* sentinel */ } after which Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno(a)collabora.com>

1 year, 1 month

Re: [PATCH v4 09/16] ASoC: mediatek: mt8365: Add PCM DAI support

by AngeloGioacchino Del Regno

Il 26/04/24 19:22, Alexandre Mergnat ha scritto: > Add Pulse Code Modulation Device Audio Interface support for MT8365 SoC. > > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> > --- > sound/soc/mediatek/mt8365/mt8365-dai-pcm.c | 293 +++++++++++++++++++++++++++++ > 1 file changed, 293 insertions(+) > > diff --git a/sound/soc/mediatek/mt8365/mt8365-dai-pcm.c b/sound/soc/mediatek/mt8365/mt8365-dai-pcm.c > new file mode 100644 > index 000000000000..a5de47c69620 > --- /dev/null > +++ b/sound/soc/mediatek/mt8365/mt8365-dai-pcm.c > @@ -0,0 +1,293 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Mediatek 8365 ALSA SoC Audio DAI PCM Control > + * > + * Copyright (c) 2024 MediaTek Inc. > + * Authors: Jia Zeng <jia.zeng(a)mediatek.com> > + * Alexandre Mergnat <amergnat(a)baylibre.com> > + */ > + > +#include <linux/bitops.h> > +#include <linux/regmap.h> > +#include <sound/pcm_params.h> > +#include "mt8365-afe-clk.h" > +#include "mt8365-afe-common.h" > + > +struct mt8365_pcm_intf_data { > + bool slave_mode; > + bool lrck_inv; > + bool bck_inv; > + unsigned int format; > +}; > + > +/* DAI Drivers */ > + > +static void mt8365_dai_enable_pcm1(struct mtk_base_afe *afe) > +{ > + regmap_update_bits(afe->regmap, PCM_INTF_CON1, > + PCM_INTF_CON1_EN, PCM_INTF_CON1_EN); > +} > + > +static void mt8365_dai_disable_pcm1(struct mtk_base_afe *afe) > +{ > + regmap_update_bits(afe->regmap, PCM_INTF_CON1, > + PCM_INTF_CON1_EN, 0x0); > +} > + > +static int mt8365_dai_configure_pcm1(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_pcm_intf_data *pcm_priv = afe_priv->dai_priv[MT8365_AFE_IO_PCM1]; > + bool slave_mode = pcm_priv->slave_mode; > + bool lrck_inv = pcm_priv->lrck_inv; > + bool bck_inv = pcm_priv->bck_inv; > + unsigned int fmt = pcm_priv->format; > + unsigned int bit_width = dai->sample_bits; > + unsigned int val = 0; > + > + if (!slave_mode) { > + val |= PCM_INTF_CON1_MASTER_MODE | > + PCM_INTF_CON1_BYPASS_ASRC; > + > + if (lrck_inv) > + val |= PCM_INTF_CON1_SYNC_OUT_INV; > + if (bck_inv) > + val |= PCM_INTF_CON1_BCLK_OUT_INV; > + } else { > + val |= PCM_INTF_CON1_SLAVE_MODE; > + > + if (lrck_inv) > + val |= PCM_INTF_CON1_SYNC_IN_INV; > + if (bck_inv) > + val |= PCM_INTF_CON1_BCLK_IN_INV; > + > + // TODO: add asrc setting /* TODO ... */ after which: Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno(a)collabora.com>

1 year, 1 month

Re: [PATCH v4 07/16] ASoC: mediatek: mt8365: Add ADDA DAI support

by AngeloGioacchino Del Regno

Il 26/04/24 19:22, Alexandre Mergnat ha scritto: > Add ADDA Device Audio Interface support for MT8365 SoC. > > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> > --- > sound/soc/mediatek/mt8365/mt8365-dai-adda.c | 315 ++++++++++++++++++++++++++++ > 1 file changed, 315 insertions(+) > > diff --git a/sound/soc/mediatek/mt8365/mt8365-dai-adda.c b/sound/soc/mediatek/mt8365/mt8365-dai-adda.c > new file mode 100644 > index 000000000000..65d45010ac90 > --- /dev/null > +++ b/sound/soc/mediatek/mt8365/mt8365-dai-adda.c > @@ -0,0 +1,315 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Mediatek 8365 ALSA SoC Audio DAI ADDA Control > + * > + * Copyright (c) 2024 MediaTek Inc. > + * Authors: Jia Zeng <jia.zeng(a)mediatek.com> > + * Alexandre Mergnat <amergnat(a)baylibre.com> > + */ > + > +#include <linux/bitops.h> > +#include <linux/regmap.h> > +#include <sound/pcm_params.h> > +#include "mt8365-afe-clk.h" > +#include "mt8365-afe-common.h" > +#include "../common/mtk-dai-adda-common.h" > + > +static int adda_afe_on_ref_cnt; > + > +/* DAI Drivers */ > + > +static int mt8365_dai_set_adda_out(struct mtk_base_afe *afe, unsigned int rate) > +{ > + unsigned int val; > + if (rate == 8000 || rate == 16000) val = AFE_ADDA_DL_VOICE_DATA; else val = 0; > + switch (rate) { > + case 8000: > + val = AFE_ADDA_DL_VOICE_DATA; > + break; > + case 16000: > + val = AFE_ADDA_DL_VOICE_DATA; > + break; > + default: > + val = 0; > + } > + > + val |= FIELD_PREP(AFE_ADDA_DL_SAMPLING_RATE, > + mtk_adda_dl_rate_transform(afe, rate)); > + val |= AFE_ADDA_DL_8X_UPSAMPLE | > + AFE_ADDA_DL_MUTE_OFF_CH1 | > + AFE_ADDA_DL_MUTE_OFF_CH2 | > + AFE_ADDA_DL_DEGRADE_GAIN; > + > + regmap_update_bits(afe->regmap, AFE_ADDA_PREDIS_CON0, 0xffffffff, 0); > + regmap_update_bits(afe->regmap, AFE_ADDA_PREDIS_CON1, 0xffffffff, 0); > + regmap_update_bits(afe->regmap, AFE_ADDA_DL_SRC2_CON0, 0xffffffff, val); > + /* SA suggest apply -0.3db to audio/speech path */ > + regmap_update_bits(afe->regmap, AFE_ADDA_DL_SRC2_CON1, > + 0xffffffff, 0xf74f0000); > + /* SA suggest use default value for sdm */ > + regmap_update_bits(afe->regmap, AFE_ADDA_DL_SDM_DCCOMP_CON, > + 0xffffffff, 0x0700701e); > + > + return 0; > +} > + > +static int mt8365_dai_set_adda_in(struct mtk_base_afe *afe, unsigned int rate) > +{ > + unsigned int val; > + > + val = FIELD_PREP(AFE_ADDA_UL_SAMPLING_RATE, > + mtk_adda_ul_rate_transform(afe, rate)); > + regmap_update_bits(afe->regmap, AFE_ADDA_UL_SRC_CON0, > + AFE_ADDA_UL_SAMPLING_RATE, val); > + /* Using Internal ADC */ > + regmap_update_bits(afe->regmap, AFE_ADDA_TOP_CON0, 0x1, 0x0); > + > + return 0; > +} > + > +int mt8365_dai_enable_adda_on(struct mtk_base_afe *afe) > +{ > + unsigned long flags; > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + > + spin_lock_irqsave(&afe_priv->afe_ctrl_lock, flags); > + > + adda_afe_on_ref_cnt++; > + if (adda_afe_on_ref_cnt == 1) > + regmap_update_bits(afe->regmap, AFE_ADDA_UL_DL_CON0, > + AFE_ADDA_UL_DL_ADDA_AFE_ON, > + AFE_ADDA_UL_DL_ADDA_AFE_ON); > + > + spin_unlock_irqrestore(&afe_priv->afe_ctrl_lock, flags); > + > + return 0; > +} > + > +int mt8365_dai_disable_adda_on(struct mtk_base_afe *afe) > +{ > + unsigned long flags; > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + > + spin_lock_irqsave(&afe_priv->afe_ctrl_lock, flags); > + > + adda_afe_on_ref_cnt--; > + if (adda_afe_on_ref_cnt == 0) > + regmap_update_bits(afe->regmap, AFE_ADDA_UL_DL_CON0, > + AFE_ADDA_UL_DL_ADDA_AFE_ON, > + ~AFE_ADDA_UL_DL_ADDA_AFE_ON); > + else if (adda_afe_on_ref_cnt < 0) > + adda_afe_on_ref_cnt = 0; Should we dev_warn() here? The refcount should never be less than zero - if it is, then there's some issue somewhere that must be solved. > + > + spin_unlock_irqrestore(&afe_priv->afe_ctrl_lock, flags); > + > + return 0; > +} > + Cheers, Angelo

1 year, 1 month

Re: [PATCH v4 06/16] ASoC: mediatek: mt8365: Add I2S DAI support

by AngeloGioacchino Del Regno

Il 26/04/24 19:22, Alexandre Mergnat ha scritto: > Add I2S Device Audio Interface support for MT8365 SoC. > > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> > --- > sound/soc/mediatek/mt8365/mt8365-dai-i2s.c | 854 +++++++++++++++++++++++++++++ > 1 file changed, 854 insertions(+) > > diff --git a/sound/soc/mediatek/mt8365/mt8365-dai-i2s.c b/sound/soc/mediatek/mt8365/mt8365-dai-i2s.c > new file mode 100644 > index 000000000000..c12d75dfe215 > --- /dev/null > +++ b/sound/soc/mediatek/mt8365/mt8365-dai-i2s.c > @@ -0,0 +1,854 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Mediatek 8365 ALSA SoC Audio DAI I2S Control > + * > + * Copyright (c) 2024 MediaTek Inc. > + * Authors: Jia Zeng <jia.zeng(a)mediatek.com> > + * Alexandre Mergnat <amergnat(a)baylibre.com> > + */ > + > +#include <linux/bitops.h> > +#include <linux/regmap.h> > +#include <sound/pcm_params.h> > +#include "mt8365-afe-clk.h" > +#include "mt8365-afe-common.h" > + > +struct mtk_afe_i2s_priv { > + bool adda_link; > + int i2s_out_on_ref_cnt; > + int id; > + int low_jitter_en; > + int mclk_id; > + int share_i2s_id; > + unsigned int clk_id_in; > + unsigned int clk_id_in_m_sel; > + unsigned int clk_id_out; > + unsigned int clk_id_out_m_sel; > + unsigned int clk_in_mult; > + unsigned int clk_out_mult; > + unsigned int config_val_in; > + unsigned int config_val_out; > + unsigned int dynamic_bck; > + unsigned int reg_off_in; > + unsigned int reg_off_out; > +}; > + > +/* This enum is merely for mtk_afe_i2s_priv declare */ > +enum { > + DAI_I2S0 = 0, > + DAI_I2S3, > + DAI_I2S_NUM, > +}; > + > +static const struct mtk_afe_i2s_priv mt8365_i2s_priv[DAI_I2S_NUM] = { > + [DAI_I2S0] = { > + .id = MT8365_AFE_IO_I2S, > + .mclk_id = MT8365_I2S0_MCK, > + .share_i2s_id = -1, > + .clk_id_in = MT8365_CLK_AUD_I2S2_M, > + .clk_id_out = MT8365_CLK_AUD_I2S1_M, > + .clk_id_in_m_sel = MT8365_CLK_I2S2_M_SEL, > + .clk_id_out_m_sel = MT8365_CLK_I2S1_M_SEL, > + .clk_in_mult = 256, > + .clk_out_mult = 256, > + .adda_link = true, > + .config_val_out = AFE_I2S_CON1_I2S2_TO_PAD, > + .reg_off_in = AFE_I2S_CON2, > + .reg_off_out = AFE_I2S_CON1, > + }, > + [DAI_I2S3] = { > + .id = MT8365_AFE_IO_2ND_I2S, > + .mclk_id = MT8365_I2S3_MCK, > + .share_i2s_id = -1, > + .clk_id_in = MT8365_CLK_AUD_I2S0_M, > + .clk_id_out = MT8365_CLK_AUD_I2S3_M, > + .clk_id_in_m_sel = MT8365_CLK_I2S0_M_SEL, > + .clk_id_out_m_sel = MT8365_CLK_I2S3_M_SEL, > + .clk_in_mult = 256, > + .clk_out_mult = 256, > + .adda_link = false, > + .config_val_in = AFE_I2S_CON_FROM_IO_MUX, > + .reg_off_in = AFE_I2S_CON, > + .reg_off_out = AFE_I2S_CON3, > + }, > +}; > + > +static const u32 *get_iir_coef(unsigned int input_fs, > + unsigned int output_fs, unsigned int *count) > +{ > +#define RATIOVER 9 > +#define INV_COEF 10 > +#define NO_NEED 11 Can you please move those definitions to the top? In that case, you should also rename those as IIR_RATIOVER IIR_INV_COEF IIR_NO_NEED > + > + static const u32 IIR_COEF_48_TO_44p1[30] = { > + 0x061fb0, 0x0bd256, 0x061fb0, 0xe3a3e6, 0xf0a300, 0x000003, > + 0x0e416d, 0x1bb577, 0x0e416d, 0xe59178, 0xf23637, 0x000003, > + 0x0c7d72, 0x189060, 0x0c7d72, 0xe96f09, 0xf505b2, 0x000003, > + 0x126054, 0x249143, 0x126054, 0xe1fc0c, 0xf4b20a, 0x000002, > + 0x000000, 0x323c85, 0x323c85, 0xf76d4e, 0x000000, 0x000002, > + }; > + > + static const u32 IIR_COEF_44p1_TO_32[42] = { > + 0x0a6074, 0x0d237a, 0x0a6074, 0xdd8d6c, 0xe0b3f6, 0x000002, > + 0x0e41f8, 0x128d48, 0x0e41f8, 0xefc14e, 0xf12d7a, 0x000003, > + 0x0cfa60, 0x11e89c, 0x0cfa60, 0xf1b09e, 0xf27205, 0x000003, > + 0x15b69c, 0x20e7e4, 0x15b69c, 0xea799a, 0xe9314a, 0x000002, > + 0x0f79e2, 0x1a7064, 0x0f79e2, 0xf65e4a, 0xf03d8e, 0x000002, > + 0x10c34f, 0x1ffe4b, 0x10c34f, 0x0bbecb, 0xf2bc4b, 0x000001, > + 0x000000, 0x23b063, 0x23b063, 0x07335f, 0x000000, 0x000002, > + }; > + > + static const u32 IIR_COEF_48_TO_32[42] = { > + 0x0a2a9b, 0x0a2f05, 0x0a2a9b, 0xe73873, 0xe0c525, 0x000002, > + 0x0dd4ad, 0x0e765a, 0x0dd4ad, 0xf49808, 0xf14844, 0x000003, > + 0x18a8cd, 0x1c40d0, 0x18a8cd, 0xed2aab, 0xe542ec, 0x000002, > + 0x13e044, 0x1a47c4, 0x13e044, 0xf44aed, 0xe9acc7, 0x000002, > + 0x1abd9c, 0x2a5429, 0x1abd9c, 0xff3441, 0xe0fc5f, 0x000001, > + 0x0d86db, 0x193e2e, 0x0d86db, 0x1a6f15, 0xf14507, 0x000001, > + 0x000000, 0x1f820c, 0x1f820c, 0x0a1b1f, 0x000000, 0x000002, > + }; > + > + static const u32 IIR_COEF_32_TO_16[48] = { > + 0x122893, 0xffadd4, 0x122893, 0x0bc205, 0xc0ee1c, 0x000001, > + 0x1bab8a, 0x00750d, 0x1bab8a, 0x06a983, 0xe18a5c, 0x000002, > + 0x18f68e, 0x02706f, 0x18f68e, 0x0886a9, 0xe31bcb, 0x000002, > + 0x149c05, 0x054487, 0x149c05, 0x0bec31, 0xe5973e, 0x000002, > + 0x0ea303, 0x07f24a, 0x0ea303, 0x115ff9, 0xe967b6, 0x000002, > + 0x0823fd, 0x085531, 0x0823fd, 0x18d5b4, 0xee8d21, 0x000002, > + 0x06888e, 0x0acbbb, 0x06888e, 0x40b55c, 0xe76dce, 0x000001, > + 0x000000, 0x2d31a9, 0x2d31a9, 0x23ba4f, 0x000000, 0x000001, > + }; > + > + static const u32 IIR_COEF_96_TO_44p1[48] = { > + 0x08b543, 0xfd80f4, 0x08b543, 0x0e2332, 0xe06ed0, 0x000002, > + 0x1b6038, 0xf90e7e, 0x1b6038, 0x0ec1ac, 0xe16f66, 0x000002, > + 0x188478, 0xfbb921, 0x188478, 0x105859, 0xe2e596, 0x000002, > + 0x13eff3, 0xffa707, 0x13eff3, 0x13455c, 0xe533b7, 0x000002, > + 0x0dc239, 0x03d458, 0x0dc239, 0x17f120, 0xe8b617, 0x000002, > + 0x0745f1, 0x05d790, 0x0745f1, 0x1e3d75, 0xed5f18, 0x000002, > + 0x05641f, 0x085e2b, 0x05641f, 0x48efd0, 0xe3e9c8, 0x000001, > + 0x000000, 0x28f632, 0x28f632, 0x273905, 0x000000, 0x000001, > + }; > + > + static const u32 IIR_COEF_44p1_TO_16[48] = { > + 0x0998fb, 0xf7f925, 0x0998fb, 0x1e54a0, 0xe06605, 0x000002, > + 0x0d828e, 0xf50f97, 0x0d828e, 0x0f41b5, 0xf0a999, 0x000003, > + 0x17ebeb, 0xee30d8, 0x17ebeb, 0x1f48ca, 0xe2ae88, 0x000002, > + 0x12fab5, 0xf46ddc, 0x12fab5, 0x20cc51, 0xe4d068, 0x000002, > + 0x0c7ac6, 0xfbd00e, 0x0c7ac6, 0x2337da, 0xe8028c, 0x000002, > + 0x060ddc, 0x015b3e, 0x060ddc, 0x266754, 0xec21b6, 0x000002, > + 0x0407b5, 0x04f827, 0x0407b5, 0x52e3d0, 0xe0149f, 0x000001, > + 0x000000, 0x1f9521, 0x1f9521, 0x2ac116, 0x000000, 0x000001, > + }; > + > + static const u32 IIR_COEF_48_TO_16[48] = { > + 0x0955ff, 0xf6544a, 0x0955ff, 0x2474e5, 0xe062e6, 0x000002, > + 0x0d4180, 0xf297f4, 0x0d4180, 0x12415b, 0xf0a3b0, 0x000003, > + 0x0ba079, 0xf4f0b0, 0x0ba079, 0x1285d3, 0xf1488b, 0x000003, > + 0x12247c, 0xf1033c, 0x12247c, 0x2625be, 0xe48e0d, 0x000002, > + 0x0b98e0, 0xf96d1a, 0x0b98e0, 0x27e79c, 0xe7798a, 0x000002, > + 0x055e3b, 0xffed09, 0x055e3b, 0x2a2e2d, 0xeb2854, 0x000002, > + 0x01a934, 0x01ca03, 0x01a934, 0x2c4fea, 0xee93ab, 0x000002, > + 0x000000, 0x1c46c5, 0x1c46c5, 0x2d37dc, 0x000000, 0x000001, > + }; > + > + static const u32 IIR_COEF_96_TO_16[48] = { > + 0x0805a1, 0xf21ae3, 0x0805a1, 0x3840bb, 0xe02a2e, 0x000002, > + 0x0d5dd8, 0xe8f259, 0x0d5dd8, 0x1c0af6, 0xf04700, 0x000003, > + 0x0bb422, 0xec08d9, 0x0bb422, 0x1bfccc, 0xf09216, 0x000003, > + 0x08fde6, 0xf108be, 0x08fde6, 0x1bf096, 0xf10ae0, 0x000003, > + 0x0ae311, 0xeeeda3, 0x0ae311, 0x37c646, 0xe385f5, 0x000002, > + 0x044089, 0xfa7242, 0x044089, 0x37a785, 0xe56526, 0x000002, > + 0x00c75c, 0xffb947, 0x00c75c, 0x378ba3, 0xe72c5f, 0x000002, > + 0x000000, 0x0ef76e, 0x0ef76e, 0x377fda, 0x000000, 0x000001, > + }; > + > + static const struct { > + const u32 *coef; > + unsigned int cnt; > + } iir_coef_tbl_list[8] = { > + /* 0: 0.9188 */ > + { IIR_COEF_48_TO_44p1, ARRAY_SIZE(IIR_COEF_48_TO_44p1) }, > + /* 1: 0.7256 */ > + { IIR_COEF_44p1_TO_32, ARRAY_SIZE(IIR_COEF_44p1_TO_32) }, > + /* 2: 0.6667 */ > + { IIR_COEF_48_TO_32, ARRAY_SIZE(IIR_COEF_48_TO_32) }, > + /* 3: 0.5 */ > + { IIR_COEF_32_TO_16, ARRAY_SIZE(IIR_COEF_32_TO_16) }, > + /* 4: 0.4594 */ > + { IIR_COEF_96_TO_44p1, ARRAY_SIZE(IIR_COEF_96_TO_44p1) }, > + /* 5: 0.3628 */ > + { IIR_COEF_44p1_TO_16, ARRAY_SIZE(IIR_COEF_44p1_TO_16) }, > + /* 6: 0.3333 */ > + { IIR_COEF_48_TO_16, ARRAY_SIZE(IIR_COEF_48_TO_16) }, > + /* 7: 0.1667 */ > + { IIR_COEF_96_TO_16, ARRAY_SIZE(IIR_COEF_96_TO_16) }, > + }; > + > + static const u32 freq_new_index[16] = { > + 0, 1, 2, 99, 3, 4, 5, 99, 6, 7, 8, 9, 10, 11, 12, 99 > + }; > + > + static const u32 iir_coef_tbl_matrix[13][13] = { > + {/*0*/ > + NO_NEED, NO_NEED, NO_NEED, NO_NEED, NO_NEED, > + NO_NEED, NO_NEED, NO_NEED, NO_NEED, NO_NEED, > + NO_NEED, NO_NEED, NO_NEED > + }, > + {/*1*/ > + 1, NO_NEED, NO_NEED, NO_NEED, NO_NEED, > + NO_NEED, NO_NEED, NO_NEED, NO_NEED, > + NO_NEED, NO_NEED, NO_NEED, NO_NEED > + }, > + {/*2*/ > + 2, 0, NO_NEED, NO_NEED, NO_NEED, NO_NEED, > + NO_NEED, NO_NEED, NO_NEED, NO_NEED, > + NO_NEED, NO_NEED, NO_NEED > + }, > + {/*3*/ > + 3, INV_COEF, INV_COEF, NO_NEED, NO_NEED, > + NO_NEED, NO_NEED, NO_NEED, NO_NEED, > + NO_NEED, NO_NEED, NO_NEED, NO_NEED > + }, > + {/*4*/ > + 5, 3, INV_COEF, 2, NO_NEED, NO_NEED, > + NO_NEED, NO_NEED, NO_NEED, > + NO_NEED, NO_NEED, NO_NEED, NO_NEED > + }, > + {/*5*/ > + 6, 4, 3, 2, 0, NO_NEED, NO_NEED, NO_NEED, > + NO_NEED, NO_NEED, NO_NEED, > + NO_NEED, NO_NEED > + }, > + {/*6*/ > + INV_COEF, INV_COEF, INV_COEF, 3, INV_COEF, > + INV_COEF, NO_NEED, NO_NEED, NO_NEED, > + NO_NEED, NO_NEED, NO_NEED > + }, > + {/*7*/ > + INV_COEF, INV_COEF, INV_COEF, 5, 3, > + INV_COEF, 1, NO_NEED, NO_NEED, > + NO_NEED, NO_NEED, NO_NEED, NO_NEED > + }, > + {/*8*/ > + 7, INV_COEF, INV_COEF, 6, 4, 3, 2, 0, NO_NEED, > + NO_NEED, NO_NEED, NO_NEED, NO_NEED > + }, > + {/*9*/ > + INV_COEF, INV_COEF, INV_COEF, INV_COEF, > + INV_COEF, INV_COEF, 5, 3, INV_COEF, > + NO_NEED, NO_NEED, NO_NEED, NO_NEED > + }, > + {/*10*/ > + INV_COEF, INV_COEF, INV_COEF, 7, INV_COEF, > + INV_COEF, 6, 4, 3, 0, > + NO_NEED, NO_NEED, NO_NEED > + }, > + { /*11*/ > + RATIOVER, INV_COEF, INV_COEF, INV_COEF, > + INV_COEF, INV_COEF, INV_COEF, INV_COEF, > + INV_COEF, 3, INV_COEF, NO_NEED, NO_NEED > + }, > + {/*12*/ > + RATIOVER, RATIOVER, INV_COEF, INV_COEF, > + INV_COEF, INV_COEF, 7, INV_COEF, > + INV_COEF, 4, 3, 0, NO_NEED > + }, > + }; > + > + const u32 *coef = NULL; > + unsigned int cnt = 0; > + u32 i = freq_new_index[input_fs]; > + u32 j = freq_new_index[output_fs]; > + > + if (i >= 13 || j >= 13) { > + } else { if (i < 13 && j < 13) { u32 k ... } > + u32 k = iir_coef_tbl_matrix[i][j]; > + > + if (k >= NO_NEED) { > + } else if (k == RATIOVER) { > + } else if (k == INV_COEF) { > + } else { > + coef = iir_coef_tbl_list[k].coef; > + cnt = iir_coef_tbl_list[k].cnt; > + } > + } > + *count = cnt; > + return coef; > +} > + > +static int mt8365_dai_set_config(struct mtk_base_afe *afe, > + struct mtk_afe_i2s_priv *i2s_data, > + bool is_input, unsigned int rate, > + int bit_width) > +{ > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + struct mt8365_be_dai_data *be = > + &afe_priv->be_data[i2s_data->id - MT8365_AFE_BACKEND_BASE]; > + unsigned int val, reg_off; > + int fs = mt8365_afe_fs_timing(rate); > + > + if (fs < 0) > + return -EINVAL; > + > + val = AFE_I2S_CON_LOW_JITTER_CLK | > + FIELD_PREP(AFE_I2S_CON_RATE_MASK, fs) | > + AFE_I2S_CON_FORMAT_I2S; val = AFE_I2S_CON_LOW_JITTER_CLK | AFE_I2S_CON_FORMAT_I2S; val |= FIELD_PREP(AFE_I2S_CON_RATE_MASK, fs); looks a bit more readable to me, but no strong opinions. > + > + if (is_input) { > + reg_off = i2s_data->reg_off_in; > + if (i2s_data->adda_link) > + val |= i2s_data->config_val_in; > + } else { > + reg_off = i2s_data->reg_off_out; > + val |= i2s_data->config_val_in; > + } > + > + /* 1:bck=32lrck(16bit) or bck=64lrck(32bit) 0:fix bck=64lrck */ > + if (i2s_data->dynamic_bck) { > + if (bit_width > 16) > + val |= AFE_I2S_CON_WLEN_32BIT; > + else > + val &= ~(u32)AFE_I2S_CON_WLEN_32BIT; > + } else { > + val |= AFE_I2S_CON_WLEN_32BIT; > + } > + > + if ((be->fmt_mode & SND_SOC_DAIFMT_MASTER_MASK) == > + SND_SOC_DAIFMT_CBM_CFM) { > + val |= AFE_I2S_CON_SRC_SLAVE; > + val &= ~(u32)AFE_I2S_CON_FROM_IO_MUX;//from consys > + } > + > + regmap_update_bits(afe->regmap, reg_off, ~(u32)AFE_I2S_CON_EN, val); > + > + if (i2s_data->adda_link && is_input) > + regmap_update_bits(afe->regmap, AFE_ADDA_TOP_CON0, 0x1, 0x1); > + > + return 0; > +} > + ..snip.. > + > +static int mt8365_afe_2nd_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) > +{ > + 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]; > + > + be->fmt_mode = 0; > + > + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { > + case SND_SOC_DAIFMT_I2S: > + be->fmt_mode |= SND_SOC_DAIFMT_I2S; > + break; > + case SND_SOC_DAIFMT_LEFT_J: > + be->fmt_mode |= SND_SOC_DAIFMT_LEFT_J; > + break; > + default: > + dev_info(afe->dev, "invalid audio format for 2nd i2s!\n"); This is an error: dev_err() > + return -EINVAL; > + } > + Everything else looks good. Cheers, Angelo

1 year, 1 month

Re: [PATCH v4 08/16] ASoC: mediatek: mt8365: Add DMIC DAI support

by AngeloGioacchino Del Regno

Il 26/04/24 19:22, Alexandre Mergnat ha scritto: > Add Digital Micro Device Audio Interface support for MT8365 SoC. > > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> > --- > sound/soc/mediatek/mt8365/mt8365-dai-dmic.c | 347 ++++++++++++++++++++++++++++ > 1 file changed, 347 insertions(+) > > diff --git a/sound/soc/mediatek/mt8365/mt8365-dai-dmic.c b/sound/soc/mediatek/mt8365/mt8365-dai-dmic.c > new file mode 100644 > index 000000000000..0dd606274d8e > --- /dev/null > +++ b/sound/soc/mediatek/mt8365/mt8365-dai-dmic.c > @@ -0,0 +1,347 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Mediatek 8365 ALSA SoC Audio DAI DMIC Control > + * > + * Copyright (c) 2024 MediaTek Inc. > + * Authors: Jia Zeng <jia.zeng(a)mediatek.com> > + * Alexandre Mergnat <amergnat(a)baylibre.com> > + */ > + > +#include <linux/bitops.h> > +#include <linux/regmap.h> > +#include <sound/pcm_params.h> > +#include "mt8365-afe-clk.h" > +#include "mt8365-afe-common.h" > + > +struct mt8365_dmic_data { > + bool two_wire_mode; > + unsigned int clk_phase_sel_ch1; > + unsigned int clk_phase_sel_ch2; > + bool iir_on; > + unsigned int irr_mode; > + unsigned int dmic_mode; > + unsigned int dmic_channel; > +}; > + > +static int get_chan_reg(unsigned int channel) > +{ > + switch (channel) { > + case 8: > + fallthrough; > + case 7: > + return AFE_DMIC3_UL_SRC_CON0; > + case 6: > + fallthrough; > + case 5: > + return AFE_DMIC2_UL_SRC_CON0; > + case 4: > + fallthrough; > + case 3: > + return AFE_DMIC1_UL_SRC_CON0; > + case 2: > + fallthrough; > + case 1: > + return AFE_DMIC0_UL_SRC_CON0; > + default: > + return -EINVAL; > + } > +} > + > +/* DAI Drivers */ > + > +static void audio_dmic_adda_enable(struct mtk_base_afe *afe) > +{ > + mt8365_dai_enable_adda_on(afe); > + regmap_update_bits(afe->regmap, AFE_ADDA_UL_DL_CON0, > + AFE_ADDA_UL_DL_DMIC_CLKDIV_ON, > + AFE_ADDA_UL_DL_DMIC_CLKDIV_ON); > +} > + > +static void audio_dmic_adda_disable(struct mtk_base_afe *afe) > +{ > + regmap_update_bits(afe->regmap, AFE_ADDA_UL_DL_CON0, > + AFE_ADDA_UL_DL_DMIC_CLKDIV_ON, > + ~AFE_ADDA_UL_DL_DMIC_CLKDIV_ON); > + mt8365_dai_disable_adda_on(afe); > +} > + > +static void mt8365_dai_enable_dmic(struct mtk_base_afe *afe, > + struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + struct mt8365_dmic_data *dmic_data = afe_priv->dai_priv[MT8365_AFE_IO_DMIC]; > + unsigned int val_mask; > + int reg = get_chan_reg(dmic_data->dmic_channel); > + > + if (reg < 0) > + return; > + > + /* val and mask will be always same to enable */ > + val_mask = DMIC_TOP_CON_CH1_ON | > + DMIC_TOP_CON_CH2_ON | > + DMIC_TOP_CON_SRC_ON; > + > + regmap_update_bits(afe->regmap, reg, val_mask, val_mask); > +} > + > +static void mt8365_dai_disable_dmic(struct mtk_base_afe *afe, > + struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + struct mt8365_dmic_data *dmic_data = afe_priv->dai_priv[MT8365_AFE_IO_DMIC]; > + unsigned int mask; > + int reg = get_chan_reg(dmic_data->dmic_channel); > + > + if (reg < 0) > + return; > + > + dev_info(afe->dev, "%s dmic_channel %d\n", > + __func__, dmic_data->dmic_channel); Please, either remove this print, or use dev_dbg(). In case you're not removing it, fits in one line. > + > + mask = DMIC_TOP_CON_CH1_ON | > + DMIC_TOP_CON_CH2_ON | > + DMIC_TOP_CON_SRC_ON | > + DMIC_TOP_CON_SDM3_LEVEL_MODE; > + > + /* Set all masked values to 0 */ > + regmap_update_bits(afe->regmap, reg, mask, 0); > +} > + > +static const struct reg_sequence mt8365_dmic_iir_coeff[] = { > + { AFE_DMIC0_IIR_COEF_02_01, 0x00000000 }, > + { AFE_DMIC0_IIR_COEF_04_03, 0x00003FB8 }, > + { AFE_DMIC0_IIR_COEF_06_05, 0x3FB80000 }, > + { AFE_DMIC0_IIR_COEF_08_07, 0x3FB80000 }, > + { AFE_DMIC0_IIR_COEF_10_09, 0x0000C048 }, > + { AFE_DMIC1_IIR_COEF_02_01, 0x00000000 }, > + { AFE_DMIC1_IIR_COEF_04_03, 0x00003FB8 }, > + { AFE_DMIC1_IIR_COEF_06_05, 0x3FB80000 }, > + { AFE_DMIC1_IIR_COEF_08_07, 0x3FB80000 }, > + { AFE_DMIC1_IIR_COEF_10_09, 0x0000C048 }, > + { AFE_DMIC2_IIR_COEF_02_01, 0x00000000 }, > + { AFE_DMIC2_IIR_COEF_04_03, 0x00003FB8 }, > + { AFE_DMIC2_IIR_COEF_06_05, 0x3FB80000 }, > + { AFE_DMIC2_IIR_COEF_08_07, 0x3FB80000 }, > + { AFE_DMIC2_IIR_COEF_10_09, 0x0000C048 }, > + { AFE_DMIC3_IIR_COEF_02_01, 0x00000000 }, > + { AFE_DMIC3_IIR_COEF_04_03, 0x00003FB8 }, > + { AFE_DMIC3_IIR_COEF_06_05, 0x3FB80000 }, > + { AFE_DMIC3_IIR_COEF_08_07, 0x3FB80000 }, > + { AFE_DMIC3_IIR_COEF_10_09, 0x0000C048 }, > +}; > + > +static int mt8365_dai_load_dmic_iir_coeff_table(struct mtk_base_afe *afe) > +{ > + return regmap_multi_reg_write(afe->regmap, > + mt8365_dmic_iir_coeff, > + ARRAY_SIZE(mt8365_dmic_iir_coeff)); > +} > + > +static int mt8365_dai_configure_dmic(struct mtk_base_afe *afe, > + struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + struct mt8365_dmic_data *dmic_data = afe_priv->dai_priv[MT8365_AFE_IO_DMIC]; > + bool two_wire_mode = dmic_data->two_wire_mode; > + unsigned int clk_phase_sel_ch1 = dmic_data->clk_phase_sel_ch1; > + unsigned int clk_phase_sel_ch2 = dmic_data->clk_phase_sel_ch2; > + unsigned int val = 0; > + unsigned int mask = 0; > + unsigned int rate = dai->rate; > + int reg = get_chan_reg(dai->channels); > + > + if (reg < 0) > + return -EINVAL; > + > + dmic_data->dmic_channel = dai->channels; > + > + val |= DMIC_TOP_CON_SDM3_LEVEL_MODE; > + mask |= DMIC_TOP_CON_SDM3_LEVEL_MODE; > + > + if (two_wire_mode) { > + val |= DMIC_TOP_CON_TWO_WIRE_MODE; > + mask |= DMIC_TOP_CON_TWO_WIRE_MODE; > + } else { > + val |= FIELD_PREP(DMIC_TOP_CON_CK_PHASE_SEL_CH1, > + clk_phase_sel_ch1); > + val |= FIELD_PREP(DMIC_TOP_CON_CK_PHASE_SEL_CH2, > + clk_phase_sel_ch2); > + mask |= DMIC_TOP_CON_CK_PHASE_SEL_CH1; > + mask |= DMIC_TOP_CON_CK_PHASE_SEL_CH2; > + } > + > + switch (rate) { > + case 48000: > + val |= DMIC_TOP_CON_VOICE_MODE_48K; > + break; > + case 32000: > + val |= DMIC_TOP_CON_VOICE_MODE_32K; > + break; > + case 16000: > + val |= DMIC_TOP_CON_VOICE_MODE_16K; > + break; > + case 8000: > + val |= DMIC_TOP_CON_VOICE_MODE_8K; > + break; > + default: > + return -EINVAL; > + } > + mask |= DMIC_TOP_CON_VOICE_MODE_MASK; You initialize the mask... > + > + regmap_update_bits(afe->regmap, reg, DMIC_TOP_CON_CONFIG_MASK, val); ...but then you don't use it? :-) > + > + return 0; > +} Cheers, Angelo

1 year, 1 month

Re: Safety of opening up /dev/dma_heap/* to physically present users (udev uaccess tag) ?

by Daniel Vetter

On Mon, May 06, 2024 at 04:01:42PM +0200, Hans de Goede wrote: > Hi Sima, > > On 5/6/24 3:38 PM, Daniel Vetter wrote: > > On Mon, May 06, 2024 at 02:05:12PM +0200, Maxime Ripard wrote: > >> Hi, > >> > >> On Mon, May 06, 2024 at 01:49:17PM GMT, Hans de Goede wrote: > >>> Hi dma-buf maintainers, et.al., > >>> > >>> Various people have been working on making complex/MIPI cameras work OOTB > >>> with mainline Linux kernels and an opensource userspace stack. > >>> > >>> The generic solution adds a software ISP (for Debayering and 3A) to > >>> libcamera. Libcamera's API guarantees that buffers handed to applications > >>> using it are dma-bufs so that these can be passed to e.g. a video encoder. > >>> > >>> In order to meet this API guarantee the libcamera software ISP allocates > >>> dma-bufs from userspace through one of the /dev/dma_heap/* heaps. For > >>> the Fedora COPR repo for the PoC of this: > >>> https://hansdegoede.dreamwidth.org/28153.html > >> > >> For the record, we're also considering using them for ARM KMS devices, > >> so it would be better if the solution wasn't only considering v4l2 > >> devices. > >> > >>> I have added a simple udev rule to give physically present users access > >>> to the dma_heap-s: > >>> > >>> KERNEL=="system", SUBSYSTEM=="dma_heap", TAG+="uaccess" > >>> > >>> (and on Rasperry Pi devices any users in the video group get access) > >>> > >>> This was just a quick fix for the PoC. Now that we are ready to move out > >>> of the PoC phase and start actually integrating this into distributions > >>> the question becomes if this is an acceptable solution; or if we need some > >>> other way to deal with this ? > >>> > >>> Specifically the question is if this will have any negative security > >>> implications? I can certainly see this being used to do some sort of > >>> denial of service attack on the system (1). This is especially true for > >>> the cma heap which generally speaking is a limited resource. > >> > >> There's plenty of other ways to exhaust CMA, like allocating too much > >> KMS or v4l2 buffers. I'm not sure we should consider dma-heaps > >> differently than those if it's part of our threat model. > > > > So generally for an arm soc where your display needs cma, your render node > > doesn't. And user applications only have access to the later, while only > > the compositor gets a kms fd through logind. At least in drm aside from > > vc4 there's really no render driver that just gives you access to cma and > > allows you to exhaust that, you need to be a compositor with drm master > > access to the display. > > > > Which means we're mostly protected against bad applications, and that's > > not a threat the "user physically sits in front of the machine accounts > > for", and which giving cma access to everyone would open up. And with > > flathub/snaps/... this is very much an issue. > > I agree that bad applications are an issue, but not for the flathub / snaps > case. Flatpacks / snaps run sandboxed and don't have access to a full /dev > so those should not be able to open /dev/dma_heap/* independent of > the ACLs on /dev/dma_heap/*. The plan is for cameras using the > libcamera software ISP to always be accessed through pipewire and > the camera portal, so in this case pipewere is taking the place of > the compositor in your kms vs render node example. Yeah essentially if you clarify to "set the permissions such that pipewire can do allocations", then I think that makes sense. And is at the same level as e.g. drm kms giving compsitors (but _only_ compositors) special access rights. > So this reduces the problem to bad apps packaged by regular distributions > and if any of those misbehave the distros should fix that. > > So I think that for the denial of service side allowing physical > present users (but not sandboxed apps running as those users) to > access /dev/dma_heap/* should be ok. > > My bigger worry is if dma_heap (u)dma-bufs can be abused in other > ways then causing a denial of service. > > I guess that the answer there is that causing other security issues > should not be possible ? Well pinned memory exhaustion is a very useful tool to make all kinds of other kernel issues exploitable. Like if you have that you can weaponize all kinds of kmalloc error paths (and since it's untracked memory the oom killer will not get you of these issuees). I think for the pipewire based desktop it'd be best if you only allow pipewire to get at an fd for allocating from dma-heaps, kinda like logind furnishes the kms master fd ... Still has the issue that you can't nuke these buffers, but that's for another day. But at least from a "limit attack surface" design pov I think this would be better than just handing out access to the current user outright. But that's also not the worst option I guess, as long as snaps/flatpacks only go through the pipewire service. -Sima -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch

1 year, 1 month

Re: Safety of opening up /dev/dma_heap/* to physically present users (udev uaccess tag) ?

by Laurent Pinchart

Hi Nicolas, On Wed, May 15, 2024 at 01:43:58PM -0400, nicolas.dufresne(a)collabora.corp-partner.google.com wrote: > Le mardi 14 mai 2024 à 23:42 +0300, Laurent Pinchart a écrit : > > > You'll hit the same limitation as we hit in GStreamer, which is that KMS driver > > > only offer allocation for render buffers and most of them are missing allocators > > > for YUV buffers, even though they can import in these formats. (kms allocators, > > > except dumb, which has other issues, are format aware). > > > > My experience on Arm platforms is that the KMS drivers offer allocation > > for scanout buffers, not render buffers, and mostly using the dumb > > allocator API. If the KMS device can scan out YUV natively, YUV buffer > > allocation should be supported. Am I missing something here ? > > There is two APIs, Dumb is the legacy allocation API, only used by display Is it legacy only ? I understand the dumb buffers API to be officially supported, to allocate scanout buffers suitable for software rendering. > drivers indeed, and the API does not include a pixel format or a modifier. The > allocation of YUV buffer has been made through a small hack, > > bpp = number of bits per component (of luma plane if multiple planes) > width = width > height = height * X > > Where X will vary, "3 / 2" is used for 420 subsampling, "2" for 422 and "3" for > 444. It is far from idea, requires deep knowledge of each formats in the > application I'm not sure I see that as an issue, but our experiences and uses cases may vary :-) > and cannot allocate each planes seperatly. For semi-planar or planar formats, unless I'm mistaken, you can either allocate a single buffer and use it with appropriate offsets when constructing your framebuffer (with DRM_IOCTL_MODE_ADDFB2), or allocate one buffer per plane. > The second is to use the driver specific allocation API. This is then abstracted > by GBM. This allows allocating render buffers with notably modifiers and/or use > cases. But no support for YUV formats or multi-planar formats. GBM is the way to go for render buffers indeed. It has been designed with only graphics buffer management use cases in mind, so it's unfortunately not an option as a generic allocator, at least in its current form. -- Regards, Laurent Pinchart

1 year, 1 month

Re: Safety of opening up /dev/dma_heap/* to physically present users (udev uaccess tag) ?

by Laurent Pinchart

On Mon, May 13, 2024 at 11:10:00AM -0400, Nicolas Dufresne wrote: > Le lundi 13 mai 2024 à 11:34 +0300, Laurent Pinchart a écrit : > > On Mon, May 13, 2024 at 10:29:22AM +0200, Maxime Ripard wrote: > > > On Wed, May 08, 2024 at 10:36:08AM +0200, Daniel Vetter wrote: > > > > On Tue, May 07, 2024 at 04:07:39PM -0400, Nicolas Dufresne wrote: > > > > > Hi, > > > > > > > > > > Le mardi 07 mai 2024 à 21:36 +0300, Laurent Pinchart a écrit : > > > > > > Shorter term, we have a problem to solve, and the best option we have > > > > > > found so far is to rely on dma-buf heaps as a backend for the frame > > > > > > buffer allocatro helper in libcamera for the use case described above. > > > > > > This won't work in 100% of the cases, clearly. It's a stop-gap measure > > > > > > until we can do better. > > > > > > > > > > Considering the security concerned raised on this thread with dmabuf heap > > > > > allocation not be restricted by quotas, you'd get what you want quickly with > > > > > memfd + udmabuf instead (which is accounted already). > > > > > > > > > > It was raised that distro don't enable udmabuf, but as stated there by Hans, in > > > > > any cases distro needs to take action to make the softISP works. This > > > > > alternative is easy and does not interfere in anyway with your future plan or > > > > > the libcamera API. You could even have both dmabuf heap (for Raspbian) and the > > > > > safer memfd+udmabuf for the distro with security concerns. > > > > > > > > > > And for the long term plan, we can certainly get closer by fixing that issue > > > > > with accounting. This issue also applied to v4l2 io-ops, so it would be nice to > > > > > find common set of helpers to fix these exporters. > > > > > > > > Yeah if this is just for softisp, then memfd + udmabuf is also what I was > > > > about to suggest. Not just as a stopgap, but as the real official thing. > > > > > > > > udmabuf does kinda allow you to pin memory, but we can easily fix that by > > > > adding the right accounting and then either let mlock rlimits or cgroups > > > > kernel memory limits enforce good behavior. > > > > > > I think the main drawback with memfd is that it'll be broken for devices > > > without an IOMMU, and while you said that it's uncommon for GPUs, it's > > > definitely not for codecs and display engines. > > > > If the application wants to share buffers between the camera and a > > display engine or codec, it should arguably not use the libcamera > > FrameBufferAllocator, but allocate the buffers from the display or the > > encoder. memfd wouldn't be used in that case. > > > > We need to eat our own dogfood though. If we want to push the > > responsibility for buffer allocation in the buffer sharing case to the > > application, we need to modify the cam application to do so when using > > the KMS backend. > > Agreed, and the new dmabuf feedback on wayland can also be used on top of this. > > You'll hit the same limitation as we hit in GStreamer, which is that KMS driver > only offer allocation for render buffers and most of them are missing allocators > for YUV buffers, even though they can import in these formats. (kms allocators, > except dumb, which has other issues, are format aware). My experience on Arm platforms is that the KMS drivers offer allocation for scanout buffers, not render buffers, and mostly using the dumb allocator API. If the KMS device can scan out YUV natively, YUV buffer allocation should be supported. Am I missing something here ? -- Regards, Laurent Pinchart

1 year, 1 month

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig