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

Re: [PATCH 3/9] dt-bindings: mailbox: rockchip,rknn: Add bindings

by Conor Dooley

On Wed, Jun 12, 2024 at 03:52:56PM +0200, Tomeu Vizoso wrote: > Add the bindings for the Neural Processing Unit IP from Rockchip. > > Signed-off-by: Tomeu Vizoso <tomeu(a)tomeuvizoso.net> > --- > .../devicetree/bindings/npu/rockchip,rknn.yaml | 123 +++++++++++++++++++++ > 1 file changed, 123 insertions(+) > > diff --git a/Documentation/devicetree/bindings/npu/rockchip,rknn.yaml b/Documentation/devicetree/bindings/npu/rockchip,rknn.yaml > new file mode 100644 > index 000000000000..570a4889c11c > --- /dev/null > +++ b/Documentation/devicetree/bindings/npu/rockchip,rknn.yaml > @@ -0,0 +1,123 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/npu/rockchip,rknn.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Neural Processing Unit IP from Rockchip, based on NVIDIA's NVDLA > + > +maintainers: > + - Tomeu Vizoso <tomeu(a)tomeuvizoso.net> > + > +description: |+ The |+ chomping operator is not needed here. > + Rockchip IP for accelerating inference of neural networks, based on NVIDIA's open source NVDLA IP. > + > +properties: > + compatible: > + items: > + - enum: > + - rockchip,rk3588-rknn > + - const: rockchip,rknn > + > + reg: > + description: Base registers for NPU cores > + minItems: 1 > + maxItems: 20 For all of these properties, you need to describe the individual items. > + > + interrupts: > + minItems: 1 > + maxItems: 20 > + > + interrupt-names: > + minItems: 1 > + maxItems: 20 > + > + clocks: > + minItems: 1 > + maxItems: 20 > + > + clock-names: > + minItems: 1 > + maxItems: 20 > + > + assigned-clocks: > + maxItems: 1 > + > + assigned-clock-rates: > + maxItems: 1 > + > + resets: > + minItems: 1 > + maxItems: 20 > + > + reset-names: > + minItems: 1 > + maxItems: 20 > + > + power-domains: > + minItems: 1 > + maxItems: 20 > + > + power-domain-names: > + minItems: 1 > + maxItems: 20 > + > + iommus: > + items: > + - description: IOMMU for all cores > + > +required: > + - compatible > + - reg > + - interrupts > + - interrupt-names > + - clocks > + - clock-names > + - assigned-clocks > + - assigned-clock-rates > + - resets > + - reset-names > + - power-domains > + - power-domain-names > + - iommus > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + > + bus { > + #address-cells = <2>; > + #size-cells = <2>; > + > + rknn: npu@fdab0000 { Drop the label here, it's not used. > + compatible = "rockchip,rk3588-rknn", "rockchip,rknn"; > + reg = <0x0 0xfdab0000 0x0 0x9000>, > + <0x0 0xfdac0000 0x0 0x9000>, > + <0x0 0xfdad0000 0x0 0x9000>; > + interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH 0>, > + <GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH 0>, > + <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH 0>; > + interrupt-names = "npu0_irq", "npu1_irq", "npu2_irq"; > + clocks = <&scmi_clk 0>, <&cru 1>, > + <&cru 2>, <&cru 3>, > + <&cru 4>, <&cru 5>, > + <&cru 6>, <&cru 7>; > + clock-names = "clk_npu", > + "aclk0", "aclk1", "aclk2", > + "hclk0", "hclk1", "hclk2", > + "pclk"; > + assigned-clocks = <&scmi_clk 0>; > + assigned-clock-rates = <200000000>; > + resets = <&cru 0>, <&cru 1>, <&cru 2>, > + <&cru 3>, <&cru 4>, <&cru 5>; > + reset-names = "srst_a0", "srst_a1", "srst_a2", > + "srst_h0", "srst_h1", "srst_h2"; > + power-domains = <&power 0>, <&power 1>, <&power 2>; > + power-domain-names = "npu0", "npu1", "npu2"; > + iommus = <&rknpu_mmu>; > + status = "disabled"; A disabled example is useless. > + }; > + }; > +... > > -- > 2.45.2 >

12 months

1
0
0 0

Re: [PATCH] dma-buf/heaps: Correct the types of fd_flags and heap_flags

by John Stultz

On Wed, Jun 5, 2024 at 7:02 PM Barry Song <21cnbao(a)gmail.com> wrote: > > From: Barry Song <v-songbaohua(a)oppo.com> > > dma_heap_allocation_data defines the UAPI as follows: > > struct dma_heap_allocation_data { > __u64 len; > __u32 fd; > __u32 fd_flags; > __u64 heap_flags; > }; > > But dma heaps are casting both fd_flags and heap_flags into > unsigned long. This patch makes dma heaps - cma heap and > system heap have consistent types with UAPI. > > Signed-off-by: Barry Song <v-songbaohua(a)oppo.com> Thanks for submitting this additional cleanup! Acked-by: John Stultz <jstultz(a)google.com>

1 year

1
0
0 0

Re: [PATCH v2] dma-buf: align fd_flags and heap_flags with dma_heap_allocation_data

by Sumit Semwal

Hello Barry, On Wed, 5 Jun 2024 at 06:56, Barry Song <21cnbao(a)gmail.com> wrote: > > From: Barry Song <v-songbaohua(a)oppo.com> > > dma_heap_allocation_data defines the UAPI as follows: > > struct dma_heap_allocation_data { > __u64 len; > __u32 fd; > __u32 fd_flags; > __u64 heap_flags; > }; > > However, dma_heap_buffer_alloc() casts both fd_flags and heap_flags > into unsigned int. We're inconsistent with types in the non UAPI > arguments. This patch fixes it. Thank you for your patch; I've pushed it to drm-misc-next. Best, Sumit. > > Signed-off-by: Barry Song <v-songbaohua(a)oppo.com> > Acked-by: John Stultz <jstultz(a)google.com> > --- > -v2: > collect ack of John, thanks! > refine commit message; > > drivers/dma-buf/dma-heap.c | 4 ++-- > include/uapi/linux/dma-heap.h | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c > index 84ae708fafe7..2298ca5e112e 100644 > --- a/drivers/dma-buf/dma-heap.c > +++ b/drivers/dma-buf/dma-heap.c > @@ -50,8 +50,8 @@ static struct class *dma_heap_class; > static DEFINE_XARRAY_ALLOC(dma_heap_minors); > > static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len, > - unsigned int fd_flags, > - unsigned int heap_flags) > + u32 fd_flags, > + u64 heap_flags) > { > struct dma_buf *dmabuf; > int fd; > diff --git a/include/uapi/linux/dma-heap.h b/include/uapi/linux/dma-heap.h > index 6f84fa08e074..a4cf716a49fa 100644 > --- a/include/uapi/linux/dma-heap.h > +++ b/include/uapi/linux/dma-heap.h > @@ -19,7 +19,7 @@ > #define DMA_HEAP_VALID_FD_FLAGS (O_CLOEXEC | O_ACCMODE) > > /* Currently no heap flags */ > -#define DMA_HEAP_VALID_HEAP_FLAGS (0) > +#define DMA_HEAP_VALID_HEAP_FLAGS (0ULL) > > /** > * struct dma_heap_allocation_data - metadata passed from userspace for > -- > 2.34.1 > -- Thanks and regards, Sumit Semwal (he / him) Tech Lead - LCG, Vertical Technologies Linaro.org │ Open source software for ARM SoCs

1 year

1
0
0 0

Re: [RFC PATCH v1] dma-buf: heaps: move the verification of heap_flags to the corresponding heap

by John Stultz

On Mon, Jun 3, 2024 at 11:30 PM Hailong Liu <hailong.liu(a)oppo.com> wrote: > On 6/4/2024 2:06 AM, John Stultz wrote: > > On Mon, Jun 3, 2024 at 10:21 AM Hailong Liu <hailong.liu(a)oppo.com> wrote: > >> We now aim to improve priority dma-buf allocation. Consider android > >> animations scene: > >> > >> when device is in low memory, Allocating dma-buf as animation > >> buffers enter direct_reclaimation, longer allocation time result in a > >> laggy UI. But if we know the usage of the dma-buf, we can use some > >> mechanisms to boost, e.g. animation-memory-pool. > > > > Can you generalize this a bit further? When would userland know to use > > this new flag? > > If it is aware, would it make sense to just use a separate heap name instead? > > > > (Also: These other mechanisms you mention should probably also be > > submitted upstream, however for upstream there's also the requirement > > that we have open users and are not just enabling proprietary blob > > userspace, which makes any changes to dma-buf heaps for out of tree > > code quite difficult) > > > >> However, dma-buf usage identification becomes a challenge. A potential > >> solution could be heap_flags. the use of heap_flags seems ugly and > >> contrary to the intended design as you said, How aboult extending > >> dma_heap_allocation_data as follows? > >> > >> struct dma_heap_allocation_data { > >> __u64 len; > >> __u32 fd; > >> __u32 fd_flags; > >> __u64 heap_flags; > >> __u64 buf_flags: // buf usage > >> }; > > > > This would affect the ABI (forcing a new ioctl number). And it's > > unclear what flags you envision as buffer specific (rather than heap > > specific as this patch suggested). > > > > I think we need more details about the specific problem you're seeing > > and trying to resolve. > This patch mainly focuses on optimization for Android scenarios. Let’s > discuss it on the issue website. > Bug: 344501512 Ok, we can do that if you need. But if this is ever going to go upstream (and it's more and more important that we minimize out of tree technical debt), conversations about how to generalize this will need to happen on the list. thanks -john

1 year

1
0
0 0

Re: [RFC PATCH v1] dma-buf: heaps: move the verification of heap_flags to the corresponding heap

by John Stultz

On Mon, Jun 3, 2024 at 10:21 AM Hailong Liu <hailong.liu(a)oppo.com> wrote: > On Mon, 03. Jun 09:01, John Stultz wrote: > > On Mon, Jun 3, 2024 at 4:40 AM <hailong.liu(a)oppo.com> wrote: > > > > > > From: "Hailong.Liu" <hailong.liu(a)oppo.com> > > > > > > This help module use heap_flags to determine the type of dma-buf, > > > so that some mechanisms can be used to speed up allocation, such as > > > memory_pool, to optimize the allocation time of dma-buf. > > > > This feels like it's trying to introduce heap specific flags, but > > doesn't introduce any details about what those flags might be? > > > > This seems like it would re-allow the old opaque vendor specific heap > > flags that we saw in the ION days, which was problematic as different > > userspaces would use the same interface with potentially colliding > > heap flags with different meanings. Resulting in no way to properly > > move to an upstream solution. > > > > With the dma-heaps interface, we're trying to make sure it is well > > defined. One can register a number of heaps with different behaviors, > > and the heap name is used to differentiate the behavior. Any flags > > introduced will need to be well defined and behaviorally consistent > > between heaps. That way when an upstream solution lands, if necessary > > we can provide backwards compatibility via symlinks. > > > > So I don't think this is a good direction to go for dma-heaps. > > > > It would be better if you were able to clarify what flag requirements > > you need, so we can better understand how they might apply to other > > heaps, and see if it was something we would want to define as a flag > > (see the discussion here for similar thoughts: > > https://lore.kernel.org/lkml/CANDhNCoOKwtpstFE2VDcUvzdXUWkZ-Zx+fz6xrdPWTyci… > > ) > > > > But if your vendor heap really needs some sort of flags argument that > > you can't generalize, you can always implement your own dmabuf > > exporter driver with whatever ioctl interface you'd prefer. > > Thanks for your reply. Let’s continue our discussion here instead > of on android-review. We aim to enhance memory allocation on each > all heaps. Your pointer towards heap_flags used in /dev/ion for heap > identification was helpful. > > We now aim to improve priority dma-buf allocation. Consider android > animations scene: > > when device is in low memory, Allocating dma-buf as animation > buffers enter direct_reclaimation, longer allocation time result in a > laggy UI. But if we know the usage of the dma-buf, we can use some > mechanisms to boost, e.g. animation-memory-pool. Can you generalize this a bit further? When would userland know to use this new flag? If it is aware, would it make sense to just use a separate heap name instead? (Also: These other mechanisms you mention should probably also be submitted upstream, however for upstream there's also the requirement that we have open users and are not just enabling proprietary blob userspace, which makes any changes to dma-buf heaps for out of tree code quite difficult) > However, dma-buf usage identification becomes a challenge. A potential > solution could be heap_flags. the use of heap_flags seems ugly and > contrary to the intended design as you said, How aboult extending > dma_heap_allocation_data as follows? > > struct dma_heap_allocation_data { > __u64 len; > __u32 fd; > __u32 fd_flags; > __u64 heap_flags; > __u64 buf_flags: // buf usage > }; This would affect the ABI (forcing a new ioctl number). And it's unclear what flags you envision as buffer specific (rather than heap specific as this patch suggested). I think we need more details about the specific problem you're seeing and trying to resolve. thanks -john

1 year

1
0
0 0

Re: [RFC PATCH v1] dma-buf: heaps: move the verification of heap_flags to the corresponding heap

by John Stultz

On Mon, Jun 3, 2024 at 4:40 AM <hailong.liu(a)oppo.com> wrote: > > From: "Hailong.Liu" <hailong.liu(a)oppo.com> > > This help module use heap_flags to determine the type of dma-buf, > so that some mechanisms can be used to speed up allocation, such as > memory_pool, to optimize the allocation time of dma-buf. This feels like it's trying to introduce heap specific flags, but doesn't introduce any details about what those flags might be? This seems like it would re-allow the old opaque vendor specific heap flags that we saw in the ION days, which was problematic as different userspaces would use the same interface with potentially colliding heap flags with different meanings. Resulting in no way to properly move to an upstream solution. With the dma-heaps interface, we're trying to make sure it is well defined. One can register a number of heaps with different behaviors, and the heap name is used to differentiate the behavior. Any flags introduced will need to be well defined and behaviorally consistent between heaps. That way when an upstream solution lands, if necessary we can provide backwards compatibility via symlinks. So I don't think this is a good direction to go for dma-heaps. It would be better if you were able to clarify what flag requirements you need, so we can better understand how they might apply to other heaps, and see if it was something we would want to define as a flag (see the discussion here for similar thoughts: https://lore.kernel.org/lkml/CANDhNCoOKwtpstFE2VDcUvzdXUWkZ-Zx+fz6xrdPWTyci… ) But if your vendor heap really needs some sort of flags argument that you can't generalize, you can always implement your own dmabuf exporter driver with whatever ioctl interface you'd prefer. thanks -john

1 year

1
0
0 0

Re: [PATCH v6 0/7] Add mediate-drm secure flow for SVP

by mripard＠kernel.org

On Tue, May 28, 2024 at 07:15:34AM GMT, Jason-JH Lin (林睿祥) wrote: > Hi Maxime, > > On Mon, 2024-05-27 at 16:06 +0200, Maxime Ripard wrote: > > Hi, > > > > On Sun, May 26, 2024 at 07:29:21AM GMT, Jason-JH.Lin wrote: > > > From: Jason-jh Lin <jason-jh.lin(a)mediatek.corp-partner.google.com> > > > > > > Memory Definitions: > > > secure memory - Memory allocated in the TEE (Trusted Execution > > > Environment) which is inaccessible in the REE (Rich Execution > > > Environment, i.e. linux kernel/userspace). > > > secure handle - Integer value which acts as reference to 'secure > > > memory'. Used in communication between TEE and REE to reference > > > 'secure memory'. > > > secure buffer - 'secure memory' that is used to store decrypted, > > > compressed video or for other general purposes in the TEE. > > > secure surface - 'secure memory' that is used to store graphic > > > buffers. > > > > > > Memory Usage in SVP: > > > The overall flow of SVP starts with encrypted video coming in from > > > an > > > outside source into the REE. The REE will then allocate a 'secure > > > buffer' and send the corresponding 'secure handle' along with the > > > encrypted, compressed video data to the TEE. The TEE will then > > > decrypt > > > the video and store the result in the 'secure buffer'. The REE will > > > then allocate a 'secure surface'. The REE will pass the 'secure > > > handles' for both the 'secure buffer' and 'secure surface' into the > > > TEE for video decoding. The video decoder HW will then decode the > > > contents of the 'secure buffer' and place the result in the 'secure > > > surface'. The REE will then attach the 'secure surface' to the > > > overlay > > > plane for rendering of the video. > > > > > > Everything relating to ensuring security of the actual contents of > > > the > > > 'secure buffer' and 'secure surface' is out of scope for the REE > > > and > > > is the responsibility of the TEE. > > > > > > DRM driver handles allocation of gem objects that are backed by a > > > 'secure > > > surface' and for displaying a 'secure surface' on the overlay > > > plane. > > > This introduces a new flag for object creation called > > > DRM_MTK_GEM_CREATE_RESTRICTED which indicates it should be a > > > 'secure > > > surface'. All changes here are in MediaTek specific code. > > > --- > > > TODO: > > > 1) Drop MTK_DRM_IOCTL_GEM_CREATE and use DMA_HEAP_IOCTL_ALLOC in > > > userspace > > > 2) DRM driver use secure mailbox channel to handle normal and > > > secure flow > > > 3) Implement setting mmsys routing table in the secure world series > > > > I'm not sure what you mean here. Why are you trying to upstream > > something that still needs to be removed from your patch series? > > > Because their is too much patches need to be fixed in this series, so I > list down the remaining TODO items and send to review for the other > patches. > > Sorry for the bothering, I'll drop this at the next version. If you don't intend to use it, we just shouldn't add it. Removing the TODO item doesn't make sense, even more so if heaps should be the way you handle this. > > Also, I made some comments on the previous version that have been > > entirely ignored and still apply on this version: > > > https://lore.kernel.org/dri-devel/20240415-guppy-of-perpetual-current-3a797… > > > > I lost that mail in my mailbox, so I didn't reply at that time. > I have imported that mail and replied to you. Hope you don't mind :) I haven't received that answer Maxime

1 year

1
0
0 0

Re: [RFC PATCH] dma-buf: align fd_flags and heap_flags with dma_heap_allocation_data

by John Stultz

On Wed, May 22, 2024 at 2:02 AM Barry Song <21cnbao(a)gmail.com> wrote: > > From: Barry Song <v-songbaohua(a)oppo.com> > > dma_heap_allocation_data defines the UAPI as follows: > > struct dma_heap_allocation_data { > __u64 len; > __u32 fd; > __u32 fd_flags; > __u64 heap_flags; > }; > > However, dma_heap_buffer_alloc() casts them into unsigned int. It's unclear > whether this is intentional or what the purpose is, but it can be quite > confusing for users. > > Adding to the confusion, dma_heap_ops.allocate defines both of these as > unsigned long. Fortunately, since dma_heap_ops is not part of the UAPI, > it is less of a concern. > > struct dma_heap_ops { > struct dma_buf *(*allocate)(struct dma_heap *heap, > unsigned long len, > unsigned long fd_flags, > unsigned long heap_flags); > }; > > I am sending this RFC in hopes of clarifying these confusions. > > If the goal is to constrain both flags to 32 bits while ensuring the struct > is aligned to 64 bits, it would have been more suitable to define > dma_heap_allocation_data accordingly from the beginning, like so: > > struct dma_heap_allocation_data { > __u64 len; > __u32 fd; > __u32 fd_flags; > __u32 heap_flags; > __u32 padding; > }; So here, if I recall, the intent was to keep 64bits for potential future heap_flags. But your point above that we're inconsistent with types in the non UAPI arguments is valid. So I think your patch makes sense. Thanks for raising this issue! Acked-by: John Stultz <jstultz(a)google.com>

1 year

1
0
0 0

[PATCH] dma-buf: add a warning when drv try to reserve 0 fence slots

by Christian König

When dma_resv_reserve_fences() is called with num_fences=0 it usually means that a driver or other component messed up its calculation how many fences are needed. Warn in that situation. When no fence are needed the function shouldn't be called in the first place. Signed-off-by: Christian König <christian.koenig(a)amd.com> --- drivers/dma-buf/dma-resv.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index e2869fb31140..5f8d010516f0 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -186,6 +186,13 @@ int dma_resv_reserve_fences(struct dma_resv *obj, unsigned int num_fences) dma_resv_assert_held(obj); + /* Driver and component code should never call this function with + * num_fences=0. If they do it usually points to bugs when calculating + * the number of needed fences dynamically. + */ + if (WARN_ON(!num_fences)) + return -EINVAL; + old = dma_resv_fences_list(obj); if (old && old->max_fences) { if ((old->num_fences + num_fences) <= old->max_fences) -- 2.34.1

1 year

1
0
0 0

Re: [PATCH] udmabuf: add CONFIG_MMU dependency

by David Hildenbrand

Am 28.05.24 um 15:31 schrieb Arnd Bergmann: > From: Arnd Bergmann <arnd(a)arndb.de> > > There is no !CONFIG_MMU version of vmf_insert_pfn(): > > arm-linux-gnueabi-ld: drivers/dma-buf/udmabuf.o: in function `udmabuf_vm_fault': > udmabuf.c:(.text+0xaa): undefined reference to `vmf_insert_pfn' > > Fixes: f7254e043ff1 ("udmabuf: use vmf_insert_pfn and VM_PFNMAP for handling mmap") > Signed-off-by: Arnd Bergmann <arnd(a)arndb.de> > --- > drivers/dma-buf/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig > index e4dc53a36428..b46eb8a552d7 100644 > --- a/drivers/dma-buf/Kconfig > +++ b/drivers/dma-buf/Kconfig > @@ -35,6 +35,7 @@ config UDMABUF > default n > depends on DMA_SHARED_BUFFER > depends on MEMFD_CREATE || COMPILE_TEST > + depends on MMU > help > A driver to let userspace turn memfd regions into dma-bufs. > Qemu can use this to create host dmabufs for guest framebuffers. Acked-by: David Hildenbrand <david(a)redhat.com> -- Thanks, David / dhildenb

1 year

1
0
0 0

Re: [PATCH v4 1/3] dt-bindings: mailbox: Add mediatek,gce-props.yaml

by AngeloGioacchino Del Regno

Il 26/05/24 17:04, Jason-JH Lin (林睿祥) ha scritto: > Hi Angelo, Jassi, > > Could you help me apply this series? > Thanks! > That's not me, it's Jassi - green light from me, btw. Cheers, Angelo > Regards, > Jason-JH.Lin > > On Wed, 2024-01-24 at 09:57 +0100, AngeloGioacchino Del Regno wrote: >> Il 24/01/24 02:14, Jason-JH.Lin ha scritto: >>> Add mediatek,gce-props.yaml for common GCE properties that is used >>> for >>> both mailbox providers and consumers. We place the common property >>> "mediatek,gce-events" in this binding currently. >>> >>> The property "mediatek,gce-events" is used for GCE event ID >>> corresponding >>> to a hardware event signal sent by the hardware or a software >>> driver. >>> If the mailbox providers or consumers want to manipulate the value >>> of >>> the event ID, they need to know the specific event ID. >>> >>> Signed-off-by: Jason-JH.Lin <jason-jh.lin(a)mediatek.com> >>> Reviewed-by: Conor Dooley <conor.dooley(a)microchip.com> >> >> Reviewed-by: AngeloGioacchino Del Regno < >> angelogioacchino.delregno(a)collabora.com> >> >

1 year

1
0
0 0

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

by T.J. Mercier

On Wed, May 22, 2024 at 11:14 AM Fedor Pchelkin <pchelkin(a)ispras.ru> wrote: > > kthread creation may possibly fail inside race_signal_callback(). In > such a case stop the already started threads, put the already taken > references to them 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> Reviewed-by: T.J. Mercier <tjmercier(a)google.com> > --- > v2: use kthread_stop_put() to actually put the last reference as > T.J. Mercier noticed; > link to v1: https://lore.kernel.org/lkml/20240522122326.696928-1-pchelkin@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..6a1bfcd0cc21 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_put(t[i].task); > + return ret; > + } > get_task_struct(t[i].task); > } > > -- > 2.39.2 >

1 year

2
1
0 0

[PATCH v6 0/7] Add mediate-drm secure flow for SVP

by Jason-JH.Lin

From: Jason-jh Lin <jason-jh.lin(a)mediatek.corp-partner.google.com> Memory Definitions: secure memory - Memory allocated in the TEE (Trusted Execution Environment) which is inaccessible in the REE (Rich Execution Environment, i.e. linux kernel/userspace). secure handle - Integer value which acts as reference to 'secure memory'. Used in communication between TEE and REE to reference 'secure memory'. secure buffer - 'secure memory' that is used to store decrypted, compressed video or for other general purposes in the TEE. secure surface - 'secure memory' that is used to store graphic buffers. Memory Usage in SVP: The overall flow of SVP starts with encrypted video coming in from an outside source into the REE. The REE will then allocate a 'secure buffer' and send the corresponding 'secure handle' along with the encrypted, compressed video data to the TEE. The TEE will then decrypt the video and store the result in the 'secure buffer'. The REE will then allocate a 'secure surface'. The REE will pass the 'secure handles' for both the 'secure buffer' and 'secure surface' into the TEE for video decoding. The video decoder HW will then decode the contents of the 'secure buffer' and place the result in the 'secure surface'. The REE will then attach the 'secure surface' to the overlay plane for rendering of the video. Everything relating to ensuring security of the actual contents of the 'secure buffer' and 'secure surface' is out of scope for the REE and is the responsibility of the TEE. DRM driver handles allocation of gem objects that are backed by a 'secure surface' and for displaying a 'secure surface' on the overlay plane. This introduces a new flag for object creation called DRM_MTK_GEM_CREATE_RESTRICTED which indicates it should be a 'secure surface'. All changes here are in MediaTek specific code. --- TODO: 1) Drop MTK_DRM_IOCTL_GEM_CREATE and use DMA_HEAP_IOCTL_ALLOC in userspace 2) DRM driver use secure mailbox channel to handle normal and secure flow 3) Implement setting mmsys routing table in the secure world series --- Based on 3 series: [1] v3 dma-buf: heaps: Add MediaTek secure heap - https://patchwork.kernel.org/project/linux-mediatek/list/?series=809023 [2] v6 media: mediatek: add driver to support secure video decoder - https://patchwork.kernel.org/project/linux-mediatek/list/?series=853689 [3] v6 Add CMDQ secure driver for SVP - https://patchwork.kernel.org/project/linux-mediatek/list/?series=855884 --- Change in v6: 1. Rebase to linux-next then rebased on 3 series [1][2][3] 2. Remove some parameter and security settings in normal world 3. Drop secure port related patches 4. Fix some build error and warning Changes in v5: 1. Sync the local changes Changes in v4: 1. Rebase on mediatek-drm-next(278640d4d74cd) and fix the conflicts 2. This series is based on 20240129063025.29251-1-yunfei.dong(a)mediatek.com 3. This series is based on 20240322052829.9893-1-shawn.sung(a)mediatek.com 4. This series is based on 20240403065603.21920-1-shawn.sung(a)mediatek.com Changes in v3: 1. fix kerneldoc problems 2. fix typo in title and commit message 3. adjust naming for secure variable 4. add the missing part for is_suecure plane implementation 5. use BIT_ULL macro to replace bit shifting 6. move modification of ovl_adaptor part to the correct patch 7. add TODO list in commit message 8. add commit message for using share memory to store execute count Changes in v2: 1. remove the DRIVER_RDNDER flag for mtk_drm_ioctl 2. move cmdq_insert_backup_cookie into client driver 3. move secure gce node define from mt8195-cherry.dtsi to mt8195.dtsi --- CK Hu (1): drm/mediatek: Add interface to allocate MediaTek GEM buffer Jason-JH.Lin (6): drm/mediatek/uapi: Add DRM_MTK_GEM_CREATE_RESTRICTED flag drm/mediatek: Add secure buffer control flow to mtk_drm_gem drm/mediatek: Add secure identify flag and funcution to mtk_drm_plane drm/mediatek: Add mtk_ddp_sec_write() to config secure buffer info drm/mediatek: Add secure flow support to mediatek-drm drm/mediatek: Add cmdq_insert_backup_cookie before secure pkt finalize drivers/gpu/drm/mediatek/mtk_crtc.c | 263 +++++++++++++++++- drivers/gpu/drm/mediatek/mtk_crtc.h | 1 + drivers/gpu/drm/mediatek/mtk_ddp_comp.c | 14 + drivers/gpu/drm/mediatek/mtk_ddp_comp.h | 5 + drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 9 +- .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 1 + drivers/gpu/drm/mediatek/mtk_drm_drv.c | 13 +- drivers/gpu/drm/mediatek/mtk_gem.c | 114 ++++++++ drivers/gpu/drm/mediatek/mtk_gem.h | 12 + drivers/gpu/drm/mediatek/mtk_mdp_rdma.c | 8 +- drivers/gpu/drm/mediatek/mtk_mdp_rdma.h | 1 + drivers/gpu/drm/mediatek/mtk_plane.c | 25 ++ drivers/gpu/drm/mediatek/mtk_plane.h | 2 + include/uapi/drm/mediatek_drm.h | 47 ++++ 14 files changed, 500 insertions(+), 15 deletions(-) create mode 100644 include/uapi/drm/mediatek_drm.h -- 2.18.0

1 year

2
8
0 0

[PATCH] dma-buf/sw-sync: don't enable IRQ from sync_print_obj()

by Tetsuo Handa

Since commit a6aa8fca4d79 ("dma-buf/sw-sync: Reduce irqsave/irqrestore from known context") by error replaced spin_unlock_irqrestore() with spin_unlock_irq() for both sync_debugfs_show() and sync_print_obj() despite sync_print_obj() is called from sync_debugfs_show(), lockdep complains inconsistent lock state warning. Use plain spin_{lock,unlock}() for sync_print_obj(), for sync_debugfs_show() is already using spin_{lock,unlock}_irq(). Reported-by: syzbot <syzbot+a225ee3df7e7f9372dbe(a)syzkaller.appspotmail.com> Closes: https://syzkaller.appspot.com/bug?extid=a225ee3df7e7f9372dbe Fixes: a6aa8fca4d79 ("dma-buf/sw-sync: Reduce irqsave/irqrestore from known context") Signed-off-by: Tetsuo Handa <penguin-kernel(a)I-love.SAKURA.ne.jp> --- drivers/dma-buf/sync_debug.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/dma-buf/sync_debug.c b/drivers/dma-buf/sync_debug.c index 101394f16930..237bce21d1e7 100644 --- a/drivers/dma-buf/sync_debug.c +++ b/drivers/dma-buf/sync_debug.c @@ -110,12 +110,12 @@ static void sync_print_obj(struct seq_file *s, struct sync_timeline *obj) seq_printf(s, "%s: %d\n", obj->name, obj->value); - spin_lock_irq(&obj->lock); + spin_lock(&obj->lock); /* Caller already disabled IRQ. */ list_for_each(pos, &obj->pt_list) { struct sync_pt *pt = container_of(pos, struct sync_pt, link); sync_print_fence(s, &pt->base, false); } - spin_unlock_irq(&obj->lock); + spin_unlock(&obj->lock); } static void sync_print_sync_file(struct seq_file *s, -- 2.18.4

1 year

2
3
0 0

[PATCH v2] 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> Reviewed-by: Kees Cook <keescook(a)chromium.org> --- Compile tested only. Changes in v2: - Name the new field 'callbacks' instead of 'cb' [Christian König] v1: https://lore.kernel.org/all/d3204a5b4776553455c2cfb1def72f1dae0dba25.171605… --- 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..c74ac197d5fe 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->callbacks; 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, callbacks, 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..29c5650c1038 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 + * @callbacks: 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 callbacks[] __counted_by(num_fences); }; /** -- 2.45.1

1 year

1
0
0 0

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 year

1
0
0 0

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 year

1
0
0 0

[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 year

3
2
0 0

[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 year

1
0
0 0

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 year

1
0
0 0

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
0
0 0

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
0
0 0

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
0
0 0

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
0
0 0

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
0
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig