Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

23 participants
2283 discussions

Re: [PATCH] dma-buf: add DMA_BUF_IOCTL_SYNC_PARTIAL support

by T.J. Mercier

On Tue, Apr 9, 2024 at 12:34 AM Rong Qianfeng <11065417(a)vivo.com> wrote: > > > 在 2024/4/8 15:58, Christian König 写道: > > Am 07.04.24 um 09:50 schrieb Rong Qianfeng: > >> [SNIP] > >>> Am 13.11.21 um 07:22 schrieb Jianqun Xu: > >>>> Add DMA_BUF_IOCTL_SYNC_PARTIAL support for user to sync dma-buf with > >>>> offset and len. > >>> > >>> You have not given an use case for this so it is a bit hard to > >>> review. And from the existing use cases I don't see why this should > >>> be necessary. > >>> > >>> Even worse from the existing backend implementation I don't even see > >>> how drivers should be able to fulfill this semantics. > >>> > >>> Please explain further, > >>> Christian. > >> Here is a practical case: > >> The user space can allocate a large chunk of dma-buf for > >> self-management, used as a shared memory pool. > >> Small dma-buf can be allocated from this shared memory pool and > >> released back to it after use, thus improving the speed of dma-buf > >> allocation and release. > >> Additionally, custom functionalities such as memory statistics and > >> boundary checking can be implemented in the user space. > >> Of course, the above-mentioned functionalities require the > >> implementation of a partial cache sync interface. > > > > Well that is obvious, but where is the code doing that? > > > > You can't send out code without an actual user of it. That will > > obviously be rejected. > > > > Regards, > > Christian. > > In fact, we have already used the user-level dma-buf memory pool in the > camera shooting scenario on the phone. > > From the test results, The execution time of the photo shooting > algorithm has been reduced from 3.8s to 3s. > For phones, the (out of tree) Android version of the system heap has a page pool connected to a shrinker. That allows you to skip page allocation without fully pinning the memory like you get when allocating a dma-buf that's way larger than necessary. If it's for a camera maybe you need physically contiguous memory, but it's also possible to set that up. https://android.googlesource.com/kernel/common/+/refs/heads/android14-6.1/d… > To be honest, I didn't understand your concern "...how drivers should be > able to fulfill this semantics." Can you please help explain it in more > detail? > > Thanks, > > Rong Qianfeng. > > > > >> > >> Thanks > >> Rong Qianfeng. > > >

1 month

Re: [PATCH net-next v8 3/3] net: ethernet: ti: am65-cpsw: Add minimal XDP support

by Jakub Kicinski

On Wed, 10 Apr 2024 16:02:00 +0200 Julien Panis wrote: > > You shouldn't build the skb upfront any more. Give the page to the HW, > > once HW sends you a completion - build the skbs. If build fails > > (allocation failure) just give the page back to HW. If it succeeds, > > however, you'll get a skb which is far more likely to be cache hot. > > Not sure I get this point. > > "Give the page to the HW" = Do you mean that I should dma_map_single() > a full page (|PAGE_SIZE|) in am65_cpsw_nuss_rx_push() ? Yes, I think so. I think that's what you effectively do now anyway, you just limit the len and wrap it in an skb. But am65_cpsw_nuss_rx_push() will effectively get that page back from skb->data and map it.

1 month

Re: [PATCH net-next v8 2/3] net: ethernet: ti: Add desc_infos member to struct k3_cppi_desc_pool

by Jakub Kicinski

On Wed, 10 Apr 2024 10:36:16 +0200 Julien Panis wrote: > Also, about mem alloc failures, shouldn't we free 'pool' on kstrdup_const() > error at the beginning of k3_cppi_desc_pool_create_name() ? > I mean, it's not visible in my patch but I now wonder if this was done > properly even before I modify the file. Yes, it uses managed memory (devm_*) but prueth (I didn't check other callers) calls it from .ndo_open. So while not technically a full leak we can accumulate infinite memory by repeatedly failing here :S

1 month

Re: [PATCH net-next v8 3/3] net: ethernet: ti: am65-cpsw: Add minimal XDP support

by Jakub Kicinski

On Mon, 08 Apr 2024 11:38:04 +0200 Julien Panis wrote: > +static struct sk_buff *am65_cpsw_alloc_skb(struct am65_cpsw_rx_chn *rx_chn, > + struct net_device *ndev, > + unsigned int len, > + int desc_idx, > + bool allow_direct) > +{ > + struct sk_buff *skb; > + struct page *page; > + > + page = page_pool_dev_alloc_pages(rx_chn->page_pool); > + if (unlikely(!page)) > + return NULL; > + > + len += AM65_CPSW_HEADROOM; > + > + skb = build_skb(page_address(page), len); You shouldn't build the skb upfront any more. Give the page to the HW, once HW sends you a completion - build the skbs. If build fails (allocation failure) just give the page back to HW. If it succeeds, however, you'll get a skb which is far more likely to be cache hot. > + if (unlikely(!skb)) { > + page_pool_put_full_page(rx_chn->page_pool, page, allow_direct); > + rx_chn->pages[desc_idx] = NULL; > + return NULL; > + }

1 month, 1 week

Re: [PATCH net-next v8 2/3] net: ethernet: ti: Add desc_infos member to struct k3_cppi_desc_pool

by Jakub Kicinski

On Mon, 08 Apr 2024 11:38:03 +0200 Julien Panis wrote: > goto gen_pool_create_fail; > } > > + pool->desc_infos = kcalloc(pool->num_desc, > + sizeof(*pool->desc_infos), GFP_KERNEL); > + if (!pool->desc_infos) { > + ret = -ENOMEM; > + dev_err(pool->dev, > + "pool descriptor infos alloc failed %d\n", ret); Please don't add errors on mem alloc failures. They just bloat the kernel, there will be a rather large OOM splat in the logs if GFP_KERNEL allocation fails. > + kfree_const(pool_name); > + goto gen_pool_desc_infos_alloc_fail; > + } > + > pool->gen_pool->name = pool_name; If you add the new allocation after this line, I think you wouldn't have to free pool_name under the if () explicitly. -- pw-bot: cr

1 month, 1 week

Re: [PATCH v3 18/18] arm64: dts: mediatek: add audio support for mt8365-evk

by Krzysztof Kozlowski

On 09/04/2024 15:42, Alexandre Mergnat wrote: > Add the sound node which is linked to the MT8365 SoC AFE and > the MT6357 audio codec. > > Update the file header. > > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> > --- > arch/arm64/boot/dts/mediatek/mt8365-evk.dts | 98 +++++++++++++++++++++++++++-- > 1 file changed, 94 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/boot/dts/mediatek/mt8365-evk.dts b/arch/arm64/boot/dts/mediatek/mt8365-evk.dts > index 50cbaefa1a99..eb0c5f076dd4 100644 > --- a/arch/arm64/boot/dts/mediatek/mt8365-evk.dts > +++ b/arch/arm64/boot/dts/mediatek/mt8365-evk.dts > @@ -1,9 +1,9 @@ > // SPDX-License-Identifier: GPL-2.0 > /* > - * Copyright (c) 2021-2022 BayLibre, SAS. > - * Authors: > - * Fabien Parent <fparent(a)baylibre.com> > - * Bernhard Rosenkränzer <bero(a)baylibre.com> > + * Copyright (c) 2024 BayLibre, SAS. What is happening with your copyrights? Why do you change existing ones? > + * Authors: Fabien Parent <fparent(a)baylibre.com> > + * Bernhard Rosenkränzer <bero(a)baylibre.com> > + * Alexandre Mergnat <amergnat(a)baylibre.com> > */ > > /dts-v1/; > @@ -86,6 +86,29 @@ optee_reserved: optee@43200000 { > reg = <0 0x43200000 0 0x00c00000>; > }; > }; > + > + sound: sound { > + compatible = "mediatek,mt8365-mt6357"; > + pinctrl-names = "default", > + "dmic", > + "miso_off", > + "miso_on", > + "mosi_off", > + "mosi_on"; > + pinctrl-0 = <&aud_default_pins>; > + pinctrl-1 = <&aud_dmic_pins>; > + pinctrl-2 = <&aud_miso_off_pins>; > + pinctrl-3 = <&aud_miso_on_pins>; > + pinctrl-4 = <&aud_mosi_off_pins>; > + pinctrl-5 = <&aud_mosi_on_pins>; > + mediatek,platform = <&afe>; > + status = "okay"; Where did you disable the node? > + }; > +}; > + > +&afe { > + mediatek,dmic-mode = <1>; > + status = "okay"; > }; > > &cpu0 { > @@ -174,6 +197,12 @@ &mmc1 { > status = "okay"; > }; > > +&mt6357_codec { > + mediatek,micbias0-microvolt = <1900000>; > + mediatek,micbias1-microvolt = <1700000>; > + mediatek,vaud28-supply = <&mt6357_vaud28_reg>; > +}; > + > &mt6357_pmic { > interrupts-extended = <&pio 145 IRQ_TYPE_LEVEL_HIGH>; > interrupt-controller; > @@ -181,6 +210,67 @@ &mt6357_pmic { > }; > > &pio { > + aud_default_pins: audiodefault-pins { > + pins { > + pinmux = <MT8365_PIN_72_CMDAT4__FUNC_I2S3_BCK>, > + <MT8365_PIN_73_CMDAT5__FUNC_I2S3_LRCK>, > + <MT8365_PIN_74_CMDAT6__FUNC_I2S3_MCK>, > + <MT8365_PIN_75_CMDAT7__FUNC_I2S3_DO>; You have broken indentation everywhere. Best regards, Krzysztof

1 month, 1 week

Re: [PATCH v3 16/18] arm64: dts: mediatek: add mt6357 audio codec support

by Krzysztof Kozlowski

On 09/04/2024 15:42, Alexandre Mergnat wrote: > Add audio codec support of MT6357 PMIC. > Update the file header. Why? > > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> > --- > arch/arm64/boot/dts/mediatek/mt6357.dtsi | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/mediatek/mt6357.dtsi b/arch/arm64/boot/dts/mediatek/mt6357.dtsi > index 3330a03c2f74..ade410851524 100644 > --- a/arch/arm64/boot/dts/mediatek/mt6357.dtsi > +++ b/arch/arm64/boot/dts/mediatek/mt6357.dtsi > @@ -1,7 +1,7 @@ > // SPDX-License-Identifier: (GPL-2.0 OR MIT) > /* > * Copyright (c) 2020 MediaTek Inc. > - * Copyright (c) 2023 BayLibre Inc. > + * Copyright (c) 2024 BayLibre Inc. That's not a reasonable change. The file was published on 2023, wasn't it? If this is not correct, please explain why/how and make it separate patch. > */ > > #include <dt-bindings/input/input.h> > @@ -10,6 +10,9 @@ &pwrap { > mt6357_pmic: pmic { > compatible = "mediatek,mt6357"; > > + mt6357_codec: codec { > + }; There is no single point of having empty nodes. NAK. Best regards, Krzysztof

1 month, 1 week

Re: [PATCH v3 04/18] dt-bindings: mfd: mediatek: Add codec property for MT6357 PMIC

by Krzysztof Kozlowski

On 09/04/2024 15:42, Alexandre Mergnat wrote: > Add the audio codec sub-device. This sub-device is used to set required > and optional voltage properties between the codec and the board. > > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> > --- > Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml > index 37423c2e0fdf..7c6a4a587b5f 100644 > --- a/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml > +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml > @@ -37,6 +37,11 @@ properties: > "#interrupt-cells": > const: 2 > > + codec: > + type: object > + $ref: /schemas/sound/mt6357.yaml > + unevaluatedProperties: false Just put the properties here, without the codec node. Also, your example is now incomplete. Best regards, Krzysztof

1 month, 1 week

Re: [PATCH v3 03/18] ASoC: dt-bindings: mt6357: Add audio codec document

by Krzysztof Kozlowski

On 09/04/2024 15:42, Alexandre Mergnat wrote: > Add MT8365 audio codec bindings to set required > and optional voltage properties between the codec and the board. > The properties are: > - phandle of the requiered power supply. typo > - Setup of microphone bias voltage. > - Setup of the speaker pin pull-down. > > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> > --- > .../devicetree/bindings/sound/mt6357.yaml | 54 ++++++++++++++++++++++ Filename using compatible syntax, so missing vendor prefix. > 1 file changed, 54 insertions(+) > > diff --git a/Documentation/devicetree/bindings/sound/mt6357.yaml b/Documentation/devicetree/bindings/sound/mt6357.yaml > new file mode 100644 > index 000000000000..381cb71b959f > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/mt6357.yaml > @@ -0,0 +1,54 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/sound/mt6357.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Mediatek MT6357 Codec > + > +maintainers: > + - Alexandre Mergnat <amergnat(a)baylibre.com> > + > +description: | Do not need '|' unless you need to preserve formatting. > + This is the required and optional voltage properties for this subdevice. > + The communication between MT6357 and SoC is through Mediatek PMIC wrapper. > + For more detail, please visit Mediatek PMIC wrapper documentation. > + Must be a child node of PMIC wrapper. Why? > + > +properties: > + Drop blank line > + mediatek,hp-pull-down: > + description: > + Earphone driver positive output stage short to > + the audio reference ground. > + type: boolean > + > + mediatek,micbias0-microvolt: > + description: Selects MIC Bias 0 output voltage. > + enum: [1700000, 1800000, 1900000, 2000000, > + 2100000, 2500000, 2600000, 2700000] > + default: 1700000 > + > + mediatek,micbias1-microvolt: > + description: Selects MIC Bias 1 output voltage. > + enum: [1700000, 1800000, 1900000, 2000000, > + 2100000, 2500000, 2600000, 2700000] > + default: 1700000 > + > + mediatek,vaud28-supply: > + description: 2.8 volt supply phandle for the audio codec Supplies go without vendor prefixes. > + > +required: > + - mediatek,vaud28-supply That's basically no-op schema. I do not understand what you are trying to achieve here. > + > +additionalProperties: false > + > +examples: > + - | > + codec { > + mediatek,micbias0-microvolt = <1900000>; > + mediatek,micbias1-microvolt = <1700000>; > + mediatek,vaud28-supply = <&mt6357_vaud28_reg>; Sorry, this does not work. Change voltage to 1111111 and check the results. Best regards, Krzysztof

1 month, 1 week

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

by Krzysztof Kozlowski

On 09/04/2024 15:42, Alexandre Mergnat wrote: > Add soundcard bindings for the MT8365 SoC with the MT6357 audio codec. > > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> > +patternProperties: > + "^dai-link-[0-9]+$": > + type: object > + description: > + Container for dai-link level properties and CODEC sub-nodes. > + > + properties: > + codec: > + type: object > + description: Holds subnode which indicates codec dai. > + > + properties: > + sound-dai: > + maxItems: 1 > + description: phandle of the codec DAI > + > + additionalProperties: false > + > + link-name: > + description: > + This property corresponds to the name of the BE dai-link to which > + we are going to update parameters in this node. > + items: > + const: 2ND_I2S_BE What is the type of link-name? Why is it fixed? How can you have here multiple dai links if all of them must have the same name? Best regards, Krzysztof

1 month, 1 week

← Newer
1
2
3
4
5
6
7
8
9
...
229
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig