[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3280558.aeNJFYEL58@steina-w>
Date: Wed, 06 Mar 2024 10:55:00 +0100
From: Alexander Stein <alexander.stein@...tq-group.com>
To: Vinod Koul <vkoul@...nel.org>, Shawn Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>, Fabio Estevam <festevam@...il.com>, NXP Linux Team <linux-imx@....com>, linux-arm-kernel@...ts.infradead.org, Frank Li <Frank.Li@....com>
Cc: dmaengine@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, imx@...ts.linux.dev, Nicolin Chen <b42378@...escale.com>, Shengjiu Wang <shengjiu.wang@....com>, Joy Zou <joy.zou@....com>, Frank Li <Frank.Li@....com>
Subject: Re: [PATCH 1/4] dmaengine: imx-sdma: Support allocate memory from internal SRAM (iram)
Hi Frank,
thanks for the patch.
Am Montag, 4. März 2024, 05:32:53 CET schrieb Frank Li:
> From: Nicolin Chen <b42378@...escale.com>
>
> Allocate memory from SoC internal SRAM to reduce DDR access and keep DDR in
> lower power state (such as self-referesh) longer.
>
> Check iram_pool before sdma_init() so that ccb/context could be allocated
> from iram because DDR maybe in self-referesh in lower power audio case
> while sdma still running.
>
> Reviewed-by: Shengjiu Wang <shengjiu.wang@....com>
> Signed-off-by: Nicolin Chen <b42378@...escale.com>
> Signed-off-by: Joy Zou <joy.zou@....com>
> Signed-off-by: Frank Li <Frank.Li@....com>
> ---
> drivers/dma/imx-sdma.c | 53 +++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 40 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 9b42f5e96b1e0..9a6d8f1e9ff63 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -24,6 +24,7 @@
> #include <linux/semaphore.h>
> #include <linux/spinlock.h>
> #include <linux/device.h>
> +#include <linux/genalloc.h>
> #include <linux/dma-mapping.h>
> #include <linux/firmware.h>
> #include <linux/slab.h>
> @@ -516,6 +517,7 @@ struct sdma_engine {
> void __iomem *regs;
> struct sdma_context_data *context;
> dma_addr_t context_phys;
> + dma_addr_t ccb_phys;
> struct dma_device dma_device;
> struct clk *clk_ipg;
> struct clk *clk_ahb;
> @@ -531,6 +533,7 @@ struct sdma_engine {
> /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
> bool clk_ratio;
> bool fw_loaded;
> + struct gen_pool *iram_pool;
> };
>
> static int sdma_config_write(struct dma_chan *chan,
> @@ -1358,8 +1361,14 @@ static int sdma_request_channel0(struct sdma_engine *sdma)
> {
> int ret = -EBUSY;
>
> - sdma->bd0 = dma_alloc_coherent(sdma->dev, PAGE_SIZE, &sdma->bd0_phys,
> - GFP_NOWAIT);
> + if (sdma->iram_pool)
> + sdma->bd0 = gen_pool_dma_alloc(sdma->iram_pool,
> + sizeof(struct sdma_buffer_descriptor),
> + &sdma->bd0_phys);
> + else
> + sdma->bd0 = dma_alloc_coherent(sdma->dev,
> + sizeof(struct sdma_buffer_descriptor),
> + &sdma->bd0_phys, GFP_NOWAIT);
> if (!sdma->bd0) {
> ret = -ENOMEM;
> goto out;
> @@ -1379,10 +1388,14 @@ static int sdma_request_channel0(struct sdma_engine *sdma)
> static int sdma_alloc_bd(struct sdma_desc *desc)
> {
> u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
> + struct sdma_engine *sdma = desc->sdmac->sdma;
> int ret = 0;
>
> - desc->bd = dma_alloc_coherent(desc->sdmac->sdma->dev, bd_size,
> - &desc->bd_phys, GFP_NOWAIT);
> + if (sdma->iram_pool)
> + desc->bd = gen_pool_dma_alloc(sdma->iram_pool, bd_size, &desc->bd_phys);
> + else
> + desc->bd = dma_alloc_coherent(sdma->dev, bd_size, &desc->bd_phys, GFP_NOWAIT);
> +
> if (!desc->bd) {
> ret = -ENOMEM;
> goto out;
> @@ -1394,9 +1407,12 @@ static int sdma_alloc_bd(struct sdma_desc *desc)
> static void sdma_free_bd(struct sdma_desc *desc)
> {
> u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
> + struct sdma_engine *sdma = desc->sdmac->sdma;
>
> - dma_free_coherent(desc->sdmac->sdma->dev, bd_size, desc->bd,
> - desc->bd_phys);
> + if (sdma->iram_pool)
> + gen_pool_free(sdma->iram_pool, (unsigned long)desc->bd, bd_size);
> + else
> + dma_free_coherent(desc->sdmac->sdma->dev, bd_size, desc->bd, desc->bd_phys);
> }
>
> static void sdma_desc_free(struct virt_dma_desc *vd)
> @@ -2066,8 +2082,8 @@ static int sdma_get_firmware(struct sdma_engine *sdma,
>
> static int sdma_init(struct sdma_engine *sdma)
> {
> + int ccbsize;
> int i, ret;
> - dma_addr_t ccb_phys;
What is the motivation to put ccb_phys to struct sdma_engine? AFAICS
this is only used in sdma_init. Also the following patches of this series
are not using the struct member.
Best regards,
Alexander
>
> ret = clk_enable(sdma->clk_ipg);
> if (ret)
> @@ -2083,10 +2099,15 @@ static int sdma_init(struct sdma_engine *sdma)
> /* Be sure SDMA has not started yet */
> writel_relaxed(0, sdma->regs + SDMA_H_C0PTR);
>
> - sdma->channel_control = dma_alloc_coherent(sdma->dev,
> - MAX_DMA_CHANNELS * sizeof(struct sdma_channel_control) +
> - sizeof(struct sdma_context_data),
> - &ccb_phys, GFP_KERNEL);
> + ccbsize = MAX_DMA_CHANNELS * (sizeof(struct sdma_channel_control)
> + + sizeof(struct sdma_context_data));
> +
> + if (sdma->iram_pool)
> + sdma->channel_control = gen_pool_dma_alloc(sdma->iram_pool,
> + ccbsize, &sdma->ccb_phys);
> + else
> + sdma->channel_control = dma_alloc_coherent(sdma->dev, ccbsize, &sdma->ccb_phys,
> + GFP_KERNEL);
>
> if (!sdma->channel_control) {
> ret = -ENOMEM;
> @@ -2095,7 +2116,7 @@ static int sdma_init(struct sdma_engine *sdma)
>
> sdma->context = (void *)sdma->channel_control +
> MAX_DMA_CHANNELS * sizeof(struct sdma_channel_control);
> - sdma->context_phys = ccb_phys +
> + sdma->context_phys = sdma->ccb_phys +
> MAX_DMA_CHANNELS * sizeof(struct sdma_channel_control);
>
> /* disable all channels */
> @@ -2121,7 +2142,7 @@ static int sdma_init(struct sdma_engine *sdma)
> else
> writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
>
> - writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
> + writel_relaxed(sdma->ccb_phys, sdma->regs + SDMA_H_C0PTR);
>
> /* Initializes channel's priorities */
> sdma_set_channel_priority(&sdma->channel[0], 7);
> @@ -2272,6 +2293,12 @@ static int sdma_probe(struct platform_device *pdev)
> vchan_init(&sdmac->vc, &sdma->dma_device);
> }
>
> + if (np) {
> + sdma->iram_pool = of_gen_pool_get(np, "iram", 0);
> + if (sdma->iram_pool)
> + dev_info(&pdev->dev, "alloc bd from iram.\n");
> + }
> +
> ret = sdma_init(sdma);
> if (ret)
> goto err_init;
>
>
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
Powered by blists - more mailing lists