Linaro-mm-sig May 2024

linaro-mm-sig@lists.linaro.org

27 participants
52 discussions

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

by Daniel Vetter

On Wed, May 22, 2024 at 03:34:52PM +0200, Maxime Ripard wrote: > Hi, > > On Mon, May 06, 2024 at 03:38:24PM GMT, 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. > > > > So you need more, either: > > > > - cgroups limits on dma-buf and dma-buf heaps. This has been bikeshedded > > for years and is just not really moving. > > For reference, are you talking about: > > https://lore.kernel.org/r/20220502231944.3891435-1-tjmercier@google.com > > Or has there been a new version of that recently? I think the design feedback from Tejun has changed to that system memory should be tracked with memcg instead (but that kinda leaves the open of what to do with cma), and only device memory be tracked with a separate cgroups controller. But I'm also not sure whether that would actually solve all the tracking/isolation requirements people tossed around or just gives us something that wont get the job done. Either way, yes I think that was the most recent code. -Sima -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch

1 week, 3 days

Re: [PATCH] dma-buf: handle testing kthreads creation failure

by T.J. Mercier

On Wed, May 22, 2024 at 5:24 AM Fedor Pchelkin <pchelkin(a)ispras.ru> wrote: > > kthread creation may possibly fail inside race_signal_callback(). In > such case stop the already started threads and return with error code. > > Found by Linux Verification Center (linuxtesting.org). > > Fixes: 2989f6451084 ("dma-buf: Add selftests for dma-fence") > Cc: stable(a)vger.kernel.org > Signed-off-by: Fedor Pchelkin <pchelkin(a)ispras.ru> > --- > drivers/dma-buf/st-dma-fence.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/dma-buf/st-dma-fence.c b/drivers/dma-buf/st-dma-fence.c > index b7c6f7ea9e0c..ab1ec4631578 100644 > --- a/drivers/dma-buf/st-dma-fence.c > +++ b/drivers/dma-buf/st-dma-fence.c > @@ -540,6 +540,12 @@ static int race_signal_callback(void *arg) > t[i].before = pass; > t[i].task = kthread_run(thread_signal_callback, &t[i], > "dma-fence:%d", i); > + if (IS_ERR(t[i].task)) { > + ret = PTR_ERR(t[i].task); > + while (--i >= 0) > + kthread_stop(t[i].task); This looks like it needs to be kthread_stop_put since get_task_struct was called for previous successful kthread_run calls. > + return ret; > + } > get_task_struct(t[i].task); > } > > -- > 2.39.2 >

1 week, 4 days

[PATCH v5 0/9] dma-buf: heaps: Add restricted heap

by Yong Wu

The purpose of this patchset is for MediaTek secure video playback, and also to enable other potential uses of this in the future. The 'restricted dma-heap' will be used to allocate dma_buf objects that reference memory in the secure world that is inaccessible/unmappable by the non-secure (i.e. kernel/userspace) world. That memory will be used by the secure/ trusted world to store secure information (i.e. decrypted media content). The dma_bufs allocated from the kernel will be passed to V4L2 for video decoding (as input and output). They will also be used by the drm system for rendering of the content. This patchset adds two MediaTek restricted heaps and they will be used in v4l2[1] and drm[2]. 1) restricted_mtk_cm: secure chunk memory for MediaTek SVP (Secure Video Path). The buffer is reserved for the secure world after bootup and it is used for vcodec's ES/working buffer; 2) restricted_mtk_cma: secure CMA memory for MediaTek SVP. This buffer is dynamically reserved for the secure world and will be got when we start playing secure videos. Once the security video playing is complete, the CMA will be released. This heap is used for the vcodec's frame buffer. [1] https://lore.kernel.org/linux-mediatek/20240412090851.24999-1-yunfei.dong@m… [2] https://lore.kernel.org/linux-mediatek/20240403102701.369-1-shawn.sung@medi… Change note: v5: 1) Reconstruct TEE commands to allow the kernel to obtain the PA of the TEE buffer to initialize a valid sg table. 2) Previously, PA was hidden from the kernel. Then the kernel checks if this is restricted buffer by "if (sg_page(sg) == NULL)". In this version, we will add a new explicit interface (sg_dma_is_restricted) for users to determine whether this is a restricted buffer. 3) some words improve, like using "rheap". Rebase on v6.9-rc7. v4: https://lore.kernel.org/linux-mediatek/20240112092014.23999-1-yong.wu@media… 1) Rename the heap name from "secure" to "restricted". suggested from Simon/Pekka. There are still several "secure" string in MTK file since we use ARM platform in which we call this "secure world"/ "secure command". v3: https://lore.kernel.org/linux-mediatek/20231212024607.3681-1-yong.wu@mediat… 1) Separate the secure heap to a common file(secure_heap.c) and mtk special file (secure_heap_mtk.c), and put all the tee related code into our special file. 2) About dt-binding, Add "mediatek," prefix since this is Mediatek TEE firmware definition. 3) Remove the normal CMA heap which is a draft for qcom. Rebase on v6.7-rc1. v2: https://lore.kernel.org/linux-mediatek/20231111111559.8218-1-yong.wu@mediat… 1) Move John's patches into the vcodec patchset since they use the new dma heap interface directly. https://lore.kernel.org/linux-mediatek/20231106120423.23364-1-yunfei.dong@m… 2) Reword the dt-binding description. 3) Rename the heap name from mtk_svp to secure_mtk_cm. This means the current vcodec/DRM upstream code doesn't match this. 4) Add a normal CMA heap. currently it should be a draft version. 5) Regarding the UUID, I still use hard code, but put it in a private data which allow the others could set their own UUID. What's more, UUID is necessary for the session with TEE. If we don't have it, we can't communicate with the TEE, including the get_uuid interface, which tries to make uuid more generic, not working. If there is other way to make UUID more general, please free to tell me. v1: https://lore.kernel.org/linux-mediatek/20230911023038.30649-1-yong.wu@media… Base on v6.6-rc1. Yong Wu (9): dt-bindings: reserved-memory: Add mediatek,dynamic-restricted-region scatterlist: Add a flag for the restricted memory lib/scatterlist: Add sg_dup_table dma-buf: heaps: Initialize a restricted heap dma-buf: heaps: restricted_heap: Add private heap ops dma-buf: heaps: restricted_heap: Add dma_ops dma-buf: heaps: restricted_heap: Add MediaTek restricted heap and heap_init dma-buf: heaps: restricted_heap_mtk: Add TEE memory service call dma_buf: heaps: restricted_heap_mtk: Add a new CMA heap .../mediatek,dynamic-restricted-region.yaml | 43 ++ drivers/dma-buf/heaps/Kconfig | 16 + drivers/dma-buf/heaps/Makefile | 4 +- drivers/dma-buf/heaps/restricted_heap.c | 219 +++++++++ drivers/dma-buf/heaps/restricted_heap.h | 45 ++ drivers/dma-buf/heaps/restricted_heap_mtk.c | 423 ++++++++++++++++++ drivers/dma-buf/heaps/system_heap.c | 27 +- include/linux/scatterlist.h | 36 ++ lib/scatterlist.c | 26 ++ 9 files changed, 812 insertions(+), 27 deletions(-) create mode 100644 Documentation/devicetree/bindings/reserved-memory/mediatek,dynamic-restricted-region.yaml create mode 100644 drivers/dma-buf/heaps/restricted_heap.c create mode 100644 drivers/dma-buf/heaps/restricted_heap.h create mode 100644 drivers/dma-buf/heaps/restricted_heap_mtk.c -- 2.18.0

1 week, 5 days

[PATCH] dma-buf/fence-array: Add flex array to struct dma_fence_array

by Christophe JAILLET

This is an effort to get rid of all multiplications from allocation functions in order to prevent integer overflows [1][2]. The "struct dma_fence_array" can be refactored to add a flex array in order to have the "callback structures allocated behind the array" be more explicit. Do so: - makes the code more readable and safer. - allows using __counted_by() for additional checks - avoids some pointer arithmetic in dma_fence_array_enable_signaling() Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-a… [1] Link: https://github.com/KSPP/linux/issues/160 [2] Signed-off-by: Christophe JAILLET <christophe.jaillet(a)wanadoo.fr> --- Compile tested only. Also, I don't think that 'cb' is a great name and the associated kernel-doc description could certainly be improved. Any proposal welcomed :) --- drivers/dma-buf/dma-fence-array.c | 10 ++++------ include/linux/dma-fence-array.h | 3 +++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-buf/dma-fence-array.c index 9b3ce8948351..9c55afaca607 100644 --- a/drivers/dma-buf/dma-fence-array.c +++ b/drivers/dma-buf/dma-fence-array.c @@ -70,7 +70,7 @@ static void dma_fence_array_cb_func(struct dma_fence *f, static bool dma_fence_array_enable_signaling(struct dma_fence *fence) { struct dma_fence_array *array = to_dma_fence_array(fence); - struct dma_fence_array_cb *cb = (void *)(&array[1]); + struct dma_fence_array_cb *cb = array->cb; unsigned i; for (i = 0; i < array->num_fences; ++i) { @@ -168,22 +168,20 @@ struct dma_fence_array *dma_fence_array_create(int num_fences, bool signal_on_any) { struct dma_fence_array *array; - size_t size = sizeof(*array); WARN_ON(!num_fences || !fences); - /* Allocate the callback structures behind the array. */ - size += num_fences * sizeof(struct dma_fence_array_cb); - array = kzalloc(size, GFP_KERNEL); + array = kzalloc(struct_size(array, cb, num_fences), GFP_KERNEL); if (!array) return NULL; + array->num_fences = num_fences; + spin_lock_init(&array->lock); dma_fence_init(&array->base, &dma_fence_array_ops, &array->lock, context, seqno); init_irq_work(&array->work, irq_dma_fence_array_work); - array->num_fences = num_fences; atomic_set(&array->num_pending, signal_on_any ? 1 : num_fences); array->fences = fences; diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h index ec7f25def392..a793f9d5c73b 100644 --- a/include/linux/dma-fence-array.h +++ b/include/linux/dma-fence-array.h @@ -33,6 +33,7 @@ struct dma_fence_array_cb { * @num_pending: fences in the array still pending * @fences: array of the fences * @work: internal irq_work function + * @cb: array of callback helpers */ struct dma_fence_array { struct dma_fence base; @@ -43,6 +44,8 @@ struct dma_fence_array { struct dma_fence **fences; struct irq_work work; + + struct dma_fence_array_cb cb[] __counted_by(num_fences); }; /** -- 2.45.1

1 week, 5 days

[RFC PATCH v3 12/21] dma-buf/dma-resv: Introduce dma_resv_trylock_ctx()

by Thomas Hellström

For the drm_exec_trylock() functionality, there is a need to be able to trylock a dma-resv object as part of a drm_exec transaction. Therefore expose a variant of dma_resv_trylock that also takes a struct ww_acquire_ctx parameter. Cc: Christian König <christian.koenig(a)amd.com> Cc: Somalapuram Amaranath <Amaranath.Somalapuram(a)amd.com> Cc: Matthew Brost <matthew.brost(a)intel.com> Cc: <dri-devel(a)lists.freedesktop.org> Cc: <linaro-mm-sig(a)lists.linaro.org> Signed-off-by: Thomas Hellström <thomas.hellstrom(a)linux.intel.com> --- include/linux/dma-resv.h | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h index 8d0e34dad446..68dae8f2a22c 100644 --- a/include/linux/dma-resv.h +++ b/include/linux/dma-resv.h @@ -405,6 +405,27 @@ static inline int dma_resv_lock_slow_interruptible(struct dma_resv *obj, return ww_mutex_lock_slow_interruptible(&obj->lock, ctx); } +/** + * dma_resv_trylock_ctx - trylock the reservation object + * @obj: the reservation object + * @ctx: The ww acquire context or NULL. + * + * Tries to lock the reservation object for exclusive access and modification. + * Note, that the lock is only against other writers, readers will run + * concurrently with a writer under RCU. The seqlock is used to notify readers + * if they overlap with a writer. The context parameter ensures that other + * ww transactions can perform deadlock backoff if necessary, and that + * subsequent attempts to dma_resv_lock() @obj for @ctx will return + * -EALREADY. + * + * Return: true if the lock was acquired, false otherwise. + */ +static inline bool __must_check +dma_resv_trylock_ctx(struct dma_resv *obj, struct ww_acquire_ctx *ctx) +{ + return ww_mutex_trylock(&obj->lock, ctx); +} + /** * dma_resv_trylock - trylock the reservation object * @obj: the reservation object @@ -421,7 +442,7 @@ static inline int dma_resv_lock_slow_interruptible(struct dma_resv *obj, */ static inline bool __must_check dma_resv_trylock(struct dma_resv *obj) { - return ww_mutex_trylock(&obj->lock, NULL); + return dma_resv_trylock_ctx(obj, NULL); } /** -- 2.44.0

1 week, 5 days

Re: [PATCH v4 14/16] arm64: defconfig: enable mt8365 sound

by AngeloGioacchino Del Regno

Il 26/04/24 19:22, Alexandre Mergnat ha scritto: > Enable the MediaTek MT8365-EVK sound support. > > The audio feature is handled by the MT8365 SoC and > the MT6357 PMIC codec audio. > > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno(a)collabora.com>

1 week, 6 days

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 week, 6 days

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 week, 6 days

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 week, 6 days

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 week, 6 days

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig May 2024