* [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support
@ 2025-06-23 6:17 Devendra K Verma
2025-06-26 22:06 ` Vinod Koul
2025-07-23 13:56 ` Manivannan Sadhasivam
0 siblings, 2 replies; 9+ messages in thread
From: Devendra K Verma @ 2025-06-23 6:17 UTC (permalink / raw)
To: devverma; +Cc: dmaengine, linux-kernel, mani, vkoul
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@amd.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;
};
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support
2025-06-23 6:17 [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support Devendra K Verma
@ 2025-06-26 22:06 ` Vinod Koul
2025-07-02 9:38 ` Verma, Devendra
2025-07-23 13:56 ` Manivannan Sadhasivam
1 sibling, 1 reply; 9+ messages in thread
From: Vinod Koul @ 2025-06-26 22:06 UTC (permalink / raw)
To: Devendra K Verma; +Cc: dmaengine, linux-kernel, mani
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@amd.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?
> };
>
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread* RE: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support
2025-06-26 22:06 ` Vinod Koul
@ 2025-07-02 9:38 ` Verma, Devendra
2025-07-17 6:39 ` Verma, Devendra
2025-07-23 7:20 ` Vinod Koul
0 siblings, 2 replies; 9+ messages in thread
From: Verma, Devendra @ 2025-07-02 9:38 UTC (permalink / raw)
To: Vinod Koul
Cc: dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
mani@kernel.org
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Vinod
Thanks for the review.
> -----Original Message-----
> From: Vinod Koul <vkoul@kernel.org>
> Sent: Friday, June 27, 2025 03:37
> To: Verma, Devendra <Devendra.Verma@amd.com>
> Cc: dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org; mani@kernel.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@amd.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
^ permalink raw reply [flat|nested] 9+ messages in thread* RE: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support
2025-07-02 9:38 ` Verma, Devendra
@ 2025-07-17 6:39 ` Verma, Devendra
2025-07-23 7:20 ` Vinod Koul
1 sibling, 0 replies; 9+ messages in thread
From: Verma, Devendra @ 2025-07-17 6:39 UTC (permalink / raw)
To: Vinod Koul
Cc: dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
mani@kernel.org
[AMD Official Use Only - AMD Internal Distribution Only]
Gentle reminder!
Regards,
Devendra
> -----Original Message-----
> From: Verma, Devendra
> Sent: Wednesday, July 2, 2025 15:09
> To: Vinod Koul <vkoul@kernel.org>
> Cc: dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org; mani@kernel.org
> Subject: RE: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support
>
> Hi Vinod
>
> Thanks for the review.
>
> > -----Original Message-----
> > From: Vinod Koul <vkoul@kernel.org>
> > Sent: Friday, June 27, 2025 03:37
> > To: Verma, Devendra <Devendra.Verma@amd.com>
> > Cc: dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org;
> > mani@kernel.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@amd.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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support
2025-07-02 9:38 ` Verma, Devendra
2025-07-17 6:39 ` Verma, Devendra
@ 2025-07-23 7:20 ` Vinod Koul
2025-07-24 9:45 ` Verma, Devendra
1 sibling, 1 reply; 9+ messages in thread
From: Vinod Koul @ 2025-07-23 7:20 UTC (permalink / raw)
To: Verma, Devendra
Cc: dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
mani@kernel.org
On 02-07-25, 09:38, Verma, Devendra wrote:
> > 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@amd.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.
That does not answer my question, what decides which mode should be
used?
--
~Vinod
^ permalink raw reply [flat|nested] 9+ messages in thread* RE: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support
2025-07-23 7:20 ` Vinod Koul
@ 2025-07-24 9:45 ` Verma, Devendra
0 siblings, 0 replies; 9+ messages in thread
From: Verma, Devendra @ 2025-07-24 9:45 UTC (permalink / raw)
To: Vinod Koul
Cc: dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
mani@kernel.org
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Vinod
Please check the response inline.
Regards,
Devendra
> -----Original Message-----
> From: Vinod Koul <vkoul@kernel.org>
> Sent: Wednesday, July 23, 2025 12:51
> To: Verma, Devendra <Devendra.Verma@amd.com>
> Cc: dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org; mani@kernel.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 02-07-25, 09:38, Verma, Devendra wrote:
> > > 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@amd.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.
>
> That does not answer my question, what decides which mode should be used?
>
Simple HDMA mode is useful when there is no need for scatter-gather and
one big chunk of data from contiguous location is to be moved. It saves the effort of
preparing a linked list of descriptors as DMA channel registers can be programmed
directly.
> --
> ~Vinod
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support
2025-06-23 6:17 [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support Devendra K Verma
2025-06-26 22:06 ` Vinod Koul
@ 2025-07-23 13:56 ` Manivannan Sadhasivam
2025-07-31 9:32 ` Verma, Devendra
1 sibling, 1 reply; 9+ messages in thread
From: Manivannan Sadhasivam @ 2025-07-23 13:56 UTC (permalink / raw)
To: Devendra K Verma; +Cc: dmaengine, linux-kernel, vkoul
On Mon, Jun 23, 2025 at 11:47:33AM GMT, 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@amd.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;
Who is allocating 'dw_edma_peripheral_config' and setting 'non_ll_en' flag? We
cannot introduce a flag without anyone using it in upstream.
- Mani
> 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;
> };
>
> 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
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 9+ messages in thread* RE: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support
2025-07-23 13:56 ` Manivannan Sadhasivam
@ 2025-07-31 9:32 ` Verma, Devendra
2025-07-31 13:31 ` Manivannan Sadhasivam
0 siblings, 1 reply; 9+ messages in thread
From: Verma, Devendra @ 2025-07-31 9:32 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
vkoul@kernel.org
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Manivannan
Please check my response inline.
Regards,
Devendra
> -----Original Message-----
> From: Manivannan Sadhasivam <mani@kernel.org>
> Sent: Wednesday, July 23, 2025 19:26
> To: Verma, Devendra <Devendra.Verma@amd.com>
> Cc: dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org; vkoul@kernel.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 Mon, Jun 23, 2025 at 11:47:33AM GMT, 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@amd.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;
>
> Who is allocating 'dw_edma_peripheral_config' and setting 'non_ll_en' flag? We
> cannot introduce a flag without anyone using it in upstream.
>
> - Mani
>
The caller of the DMA engine client APIs will be responsible for allocating
and setting the non_ll_en flag. This flag helps in setting the channel in to
non-Linked-List mode. This functionality is supported by the DMA controller.
The peripheral_config is just configuring the peripheral (DMA) in to the modes
it supports based on user discretion. As this functionality is used by the
clients to differentiate between the modes supported by the controller it
may not require the equivalent code in upstream.
> > 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;
> > };
> >
> > 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
> >
>
> --
> மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support
2025-07-31 9:32 ` Verma, Devendra
@ 2025-07-31 13:31 ` Manivannan Sadhasivam
0 siblings, 0 replies; 9+ messages in thread
From: Manivannan Sadhasivam @ 2025-07-31 13:31 UTC (permalink / raw)
To: Verma, Devendra
Cc: dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
vkoul@kernel.org
On Thu, Jul 31, 2025 at 09:32:12AM GMT, Verma, Devendra wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> Hi Manivannan
>
> Please check my response inline.
>
> Regards,
> Devendra
>
> > -----Original Message-----
> > From: Manivannan Sadhasivam <mani@kernel.org>
> > Sent: Wednesday, July 23, 2025 19:26
> > To: Verma, Devendra <Devendra.Verma@amd.com>
> > Cc: dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org; vkoul@kernel.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 Mon, Jun 23, 2025 at 11:47:33AM GMT, 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@amd.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;
> >
> > Who is allocating 'dw_edma_peripheral_config' and setting 'non_ll_en' flag? We
> > cannot introduce a flag without anyone using it in upstream.
> >
> > - Mani
> >
>
> The caller of the DMA engine client APIs will be responsible for allocating
> and setting the non_ll_en flag. This flag helps in setting the channel in to
> non-Linked-List mode. This functionality is supported by the DMA controller.
> The peripheral_config is just configuring the peripheral (DMA) in to the modes
> it supports based on user discretion. As this functionality is used by the
> clients to differentiate between the modes supported by the controller it
> may not require the equivalent code in upstream.
>
As I stated above, we *must not* have an unused interface in upstream drivers. I
understand that fact that your downstream driver is setting this flag and you
want to keep your driver downstream. But the flag would be unused in upstream
and that is not acceptable, sorry.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-07-31 13:31 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-23 6:17 [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support Devendra K Verma
2025-06-26 22:06 ` Vinod Koul
2025-07-02 9:38 ` Verma, Devendra
2025-07-17 6:39 ` Verma, Devendra
2025-07-23 7:20 ` Vinod Koul
2025-07-24 9:45 ` Verma, Devendra
2025-07-23 13:56 ` Manivannan Sadhasivam
2025-07-31 9:32 ` Verma, Devendra
2025-07-31 13:31 ` Manivannan Sadhasivam
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox