[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<SA1PR12MB81208BAD2A5D264482333FF79540A@SA1PR12MB8120.namprd12.prod.outlook.com>
Date: Wed, 2 Jul 2025 09:38:48 +0000
From: "Verma, Devendra" <Devendra.Verma@....com>
To: Vinod Koul <vkoul@...nel.org>
CC: "dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"mani@...nel.org" <mani@...nel.org>
Subject: RE: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Vinod
Thanks for the review.
> -----Original Message-----
> From: Vinod Koul <vkoul@...nel.org>
> Sent: Friday, June 27, 2025 03:37
> To: Verma, Devendra <Devendra.Verma@....com>
> Cc: dmaengine@...r.kernel.org; linux-kernel@...r.kernel.org; mani@...nel.org
> Subject: Re: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On 23-06-25, 11:47, Devendra K Verma wrote:
> > The HDMA IP supports the simple mode (non-linked list).
> > In this mode the channel registers are configured to initiate a single
> > DMA data transfer. The channel can be configured in simple mode via
> > peripheral param of dma_slave_config param.
> >
> > Signed-off-by: Devendra K Verma <devverma@....com>
> > ---
> > drivers/dma/dw-edma/dw-edma-core.c | 10 +++++
> > drivers/dma/dw-edma/dw-edma-core.h | 2 +
> > drivers/dma/dw-edma/dw-hdma-v0-core.c | 53
> ++++++++++++++++++++++++++-
> > include/linux/dma/edma.h | 8 ++++
> > 4 files changed, 72 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c
> > b/drivers/dma/dw-edma/dw-edma-core.c
> > index c2b88cc99e5d..4dafd6554277 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -235,9 +235,19 @@ static int dw_edma_device_config(struct dma_chan
> *dchan,
> > struct dma_slave_config *config) {
> > struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> > + struct dw_edma_peripheral_config *pconfig = config->peripheral_config;
> > + unsigned long flags;
> > +
> > + if (WARN_ON(config->peripheral_config &&
> > + config->peripheral_size != sizeof(*pconfig)))
> > + return -EINVAL;
> >
> > + spin_lock_irqsave(&chan->vc.lock, flags);
> > memcpy(&chan->config, config, sizeof(*config));
> > +
> > + chan->non_ll_en = pconfig ? pconfig->non_ll_en : false;
> > chan->configured = true;
> > + spin_unlock_irqrestore(&chan->vc.lock, flags);
> >
> > return 0;
> > }
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.h
> > b/drivers/dma/dw-edma/dw-edma-core.h
> > index 71894b9e0b15..c0266976aa22 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.h
> > +++ b/drivers/dma/dw-edma/dw-edma-core.h
> > @@ -86,6 +86,8 @@ struct dw_edma_chan {
> > u8 configured;
> >
> > struct dma_slave_config config;
> > +
> > + bool non_ll_en;
>
> why do you need this? What is the decision to use non ll vs ll one?
>
The IP supports both the modes, LL mode and non-LL mode.
In the current driver code, the support for non-LL mode is not
present. This patch enables the non-LL aka simple mode support
by means of the peripheral_config option in the dmaengine_slave_config.
> > };
> >
> > struct dw_edma_irq {
> > diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > index e3f8db4fe909..3237c807a18e 100644
> > --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > @@ -225,7 +225,7 @@ static void dw_hdma_v0_sync_ll_data(struct
> dw_edma_chunk *chunk)
> > readl(chunk->ll_region.vaddr.io); }
> >
> > -static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool
> > first)
> > +static void dw_hdma_v0_ll_start(struct dw_edma_chunk *chunk, bool
> > +first)
> > {
> > struct dw_edma_chan *chan = chunk->chan;
> > struct dw_edma *dw = chan->dw;
> > @@ -263,6 +263,57 @@ static void dw_hdma_v0_core_start(struct
> dw_edma_chunk *chunk, bool first)
> > SET_CH_32(dw, chan->dir, chan->id, doorbell,
> > HDMA_V0_DOORBELL_START); }
> >
> > +static void dw_hdma_v0_non_ll_start(struct dw_edma_chunk *chunk) {
> > + struct dw_edma_chan *chan = chunk->chan;
> > + struct dw_edma *dw = chan->dw;
> > + struct dw_edma_burst *child;
> > + u32 val;
> > +
> > + list_for_each_entry(child, &chunk->burst->list, list) {
> > + SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
> > +
> > + /* Source address */
> > + SET_CH_32(dw, chan->dir, chan->id, sar.lsb, lower_32_bits(child->sar));
> > + SET_CH_32(dw, chan->dir, chan->id, sar.msb,
> > + upper_32_bits(child->sar));
> > +
> > + /* Destination address */
> > + SET_CH_32(dw, chan->dir, chan->id, dar.lsb, lower_32_bits(child->dar));
> > + SET_CH_32(dw, chan->dir, chan->id, dar.msb,
> > + upper_32_bits(child->dar));
> > +
> > + /* Transfer size */
> > + SET_CH_32(dw, chan->dir, chan->id, transfer_size,
> > + child->sz);
> > +
> > + /* Interrupt setup */
> > + val = GET_CH_32(dw, chan->dir, chan->id, int_setup) |
> > + HDMA_V0_STOP_INT_MASK |
> HDMA_V0_ABORT_INT_MASK |
> > + HDMA_V0_LOCAL_STOP_INT_EN |
> > + HDMA_V0_LOCAL_ABORT_INT_EN;
> > +
> > + if (!(dw->chip->flags & DW_EDMA_CHIP_LOCAL))
> > + val |= HDMA_V0_REMOTE_STOP_INT_EN |
> > + HDMA_V0_REMOTE_ABORT_INT_EN;
> > +
> > + SET_CH_32(dw, chan->dir, chan->id, int_setup, val);
> > +
> > + /* Channel control setup */
> > + val = GET_CH_32(dw, chan->dir, chan->id, control1);
> > + val &= ~HDMA_V0_LINKLIST_EN;
> > + SET_CH_32(dw, chan->dir, chan->id, control1, val);
> > +
> > + /* Ring the doorbell */
> > + SET_CH_32(dw, chan->dir, chan->id, doorbell,
> HDMA_V0_DOORBELL_START);
> > + }
> > +}
> > +
> > +static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool
> > +first) {
> > + struct dw_edma_chan *chan = chunk->chan;
> > +
> > + if (!chan->non_ll_en)
> > + dw_hdma_v0_ll_start(chunk, first);
> > + else
> > + dw_hdma_v0_non_ll_start(chunk); }
> > +
> > static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan) {
> > struct dw_edma *dw = chan->dw;
> > diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h index
> > 3080747689f6..82d808013a66 100644
> > --- a/include/linux/dma/edma.h
> > +++ b/include/linux/dma/edma.h
> > @@ -101,6 +101,14 @@ struct dw_edma_chip {
> > struct dw_edma *dw;
> > };
> >
> > +/**
> > + * struct dw_edma_peripheral_config - peripheral spicific configurations
> > + * @non_ll_en: enable non-linked list mode of operations
> > + */
> > +struct dw_edma_peripheral_config {
> > + bool non_ll_en;
> > +};
> > +
> > /* Export to the platform drivers */
> > #if IS_REACHABLE(CONFIG_DW_EDMA)
> > int dw_edma_probe(struct dw_edma_chip *chip);
> > --
> > 2.43.0
>
> --
> ~Vinod
Powered by blists - more mailing lists