From: Frank Li <Frank.li@oss.nxp.com>
To: Koichiro Den <den@valinux.co.jp>
Cc: "Manivannan Sadhasivam" <mani@kernel.org>,
"Vinod Koul" <vkoul@kernel.org>, "Frank Li" <Frank.Li@kernel.org>,
"Gustavo Pimentel" <Gustavo.Pimentel@synopsys.com>,
"Kees Cook" <kees@kernel.org>,
"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
"Kishon Vijay Abraham I" <kishon@kernel.org>,
"Bjorn Helgaas" <bhelgaas@google.com>,
"Christoph Hellwig" <hch@lst.de>,
"Serge Semin" <fancer.lancer@gmail.com>,
"Cai Huoqing" <cai.huoqing@linux.dev>,
"Niklas Cassel" <cassel@kernel.org>,
"Devendra K Verma" <devendra.verma@amd.com>,
dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 05/19] dmaengine: dw-edma: Add LL interrupt placement policy
Date: Fri, 24 Jul 2026 10:36:40 -0500 [thread overview]
Message-ID: <amOGiJe8fVRoLCMQ@SMW015318> (raw)
In-Reply-To: <dbnylap6togkuwbgrnt4oors56c4litp7ndvfnbvgg4wynigtx@z4mrupqrd5sg>
On Fri, Jul 24, 2026 at 02:46:23PM +0900, Koichiro Den wrote:
> On Thu, Jul 23, 2026 at 02:58:16PM -0500, Frank Li wrote:
> > On Thu, Jul 23, 2026 at 05:41:36PM +0900, Koichiro Den wrote:
> > > Move linked-list interrupt placement behind a core callback so eDMA and
> > > HDMA can use different policies.
> > >
> > > Keep the eDMA behavior. For HDMA, place watermarks at descriptor ends,
> > > before link entries, and every four entries when a descriptor exceeds the
> > > usable ring capacity or another issued descriptor follows. A later patch
> > > handles these watermarks as progress events.
> > >
> > > Four entries is an empirically chosen coalescing interval.
> > >
> > > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > > ---
> > > Changes in v2:
> > > - Fix the zero-based interval so the first watermark is on the fourth
> > > entry, not the fifth.
> > > - Revise the commit message and source comments.
> > >
> > > drivers/dma/dw-edma/dw-edma-core.c | 8 +++++++-
> > > drivers/dma/dw-edma/dw-edma-core.h | 2 ++
> > > drivers/dma/dw-edma/dw-edma-v0-core.c | 10 ++++++++++
> > > drivers/dma/dw-edma/dw-hdma-v0-core.c | 28 +++++++++++++++++++++++++++
> > > 4 files changed, 47 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > > index a9f25a65294e..78d1bb6302fb 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > > @@ -98,6 +98,12 @@ static u32 dw_edma_core_get_free_num(struct dw_edma_chan *chan)
> > > chan->ll_max;
> > > }
> > >
> > > +static bool dw_edma_core_enable_ll_irq(struct dw_edma_desc *desc, u32 i,
> > > + u32 free)
> > > +{
> > > + return desc->chan->dw->core->ll_irq(desc, i, free);
> > > +}
> > > +
> > > static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
> > > {
> > > struct dw_edma_chan *chan = desc->chan;
> > > @@ -120,7 +126,7 @@ static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
> > >
> > > dw_edma_core_ll_data(chan, &desc->burst[i],
> > > chan->ll_head, chan->cb,
> > > - i == desc->nburst - 1 || free == 1);
> > > + dw_edma_core_enable_ll_irq(desc, i, free));
> > >
> > > chan->ll_head++;
> > >
> > > diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> > > index f2c2d5af1fff..8ed37e8a12cb 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-core.h
> > > +++ b/drivers/dma/dw-edma/dw-edma-core.h
> > > @@ -162,6 +162,8 @@ struct dw_edma_core_ops {
> > > void (*ll_link)(struct dw_edma_chan *chan, u32 idx, bool cb, u64 addr);
> > > void (*ll_clear)(struct dw_edma_chan *chan, u32 idx);
> > > int (*ll_cur_idx)(struct dw_edma_chan *chan);
> > > + /* Called with chan->vc.lock held. */
> > > + bool (*ll_irq)(struct dw_edma_desc *desc, u32 i, u32 free);
> > > void (*ch_doorbell)(struct dw_edma_chan *chan);
> > > void (*ch_enable)(struct dw_edma_chan *chan);
> > > void (*ch_config)(struct dw_edma_chan *chan);
> > > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > index c31fff095b4f..62141aa32e50 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > @@ -649,6 +649,15 @@ static int dw_edma_v0_core_ll_cur_idx(struct dw_edma_chan *chan)
> > > return (val - base) / EDMA_LL_SZ;
> > > }
> > >
> > > +static bool dw_edma_v0_core_ll_irq(struct dw_edma_desc *desc, u32 i, u32 free)
> > > +{
> > > + /*
> > > + * eDMA reports LL interrupts through DONE. Keep them at
> > > + * descriptor ends, plus the last free slot to refill the ring.
> > > + */
> > > + return i == desc->nburst - 1 || free == 1;
> > > +}
> > > +
> > > /* eDMA debugfs callbacks */
> > > static void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
> > > {
> > > @@ -685,6 +694,7 @@ static const struct dw_edma_core_ops dw_edma_v0_core = {
> > > .ll_link = dw_edma_v0_core_ll_link,
> > > .ll_clear = dw_edma_v0_core_ll_clear,
> > > .ll_cur_idx = dw_edma_v0_core_ll_cur_idx,
> > > + .ll_irq = dw_edma_v0_core_ll_irq,
> > > .ch_doorbell = dw_edma_v0_core_ch_doorbell,
> > > .ch_enable = dw_edma_v0_core_ch_enable,
> > > .ch_config = dw_edma_v0_core_ch_config,
> > > diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > > index b2d35f0b7b6d..fb651a83f0f3 100644
> > > --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > > +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > > @@ -13,6 +13,9 @@
> > > #include "dw-hdma-v0-regs.h"
> > > #include "dw-hdma-v0-debugfs.h"
> > >
> > > +/* Empirically chosen watermark interval. */
> > > +#define HDMA_V0_WATERMARK_INTERVAL 4
> > > +
> > > enum dw_hdma_control {
> > > DW_HDMA_V0_CB = BIT(0),
> > > DW_HDMA_V0_TCB = BIT(1),
> > > @@ -417,6 +420,30 @@ static int dw_hdma_v0_core_ll_cur_idx(struct dw_edma_chan *chan)
> > > return (val - base) / EDMA_LL_SZ;
> > > }
> > >
> > > +static bool dw_hdma_v0_core_ll_irq(struct dw_edma_desc *desc, u32 i, u32 free)
> > > +{
> > > + struct dw_edma_chan *chan = desc->chan;
> > > + bool needs_progress;
> > > +
> > > + /*
> > > + * Always place a watermark at a descriptor end and before the link
> > > + * element. The first reports descriptor progress; the second provides
> > > + * one progress point per ring lap.
> > > + */
> > > + if (i == desc->nburst - 1 || chan->ll_head == chan->ll_max - 1)
> > > + return true;
> > > +
> > > + /*
> > > + * Use periodic watermarks when a descriptor exceeds the usable ring or
> > > + * when more issued work follows it.
> > > + */
> > > + needs_progress = desc->nburst > chan->ll_max - 1 ||
> > > + !list_is_last(&desc->vd.node, &chan->vc.desc_issued);
> > > +
> > > + return needs_progress &&
> > > + (chan->ll_head + 1) % HDMA_V0_WATERMARK_INTERVAL == 0;
> >
> > This is not good layer design. edma/hdma supposed just do action to operate
> > register, less software logic.
>
> Indeed, thanks for pointing that out.
>
> >
> > Can we move this code to core layer? introduce an 'interval' for difference
> > backend.
> >
> > eDMA: interval is number description.
> > hDMA: interval is HDMA_V0_WATERMARK_INTERVAL
>
> Sorry, by "eDMA: interval is number description", do you mean using desc->nburst
> as the "interval", so eDMA keeps one interrupt at the end of each descriptor?
Yes, of cource, you also can periodic interval check eDMA if want.
Frank
>
> >
> > So core layer can use unified logic to handle for both eDMA and hDMA?
>
> I'll move as much of the software policy as possible into the common core. I
> think even the periodic interval and descriptor-end placement can be shared.
> I'll rework this part and verify it with performance tests.
>
> Thanks for the review and suggestion. Much appreciated.
> Koichiro
>
> >
> > Frank
> >
> > > +}
> > > +
> > > /* HDMA debugfs callbacks */
> > > static void dw_hdma_v0_core_debugfs_on(struct dw_edma *dw)
> > > {
> > > @@ -441,6 +468,7 @@ static const struct dw_edma_core_ops dw_hdma_v0_core = {
> > > .ll_link = dw_hdma_v0_core_ll_link,
> > > .ll_clear = dw_hdma_v0_core_ll_clear,
> > > .ll_cur_idx = dw_hdma_v0_core_ll_cur_idx,
> > > + .ll_irq = dw_hdma_v0_core_ll_irq,
> > > .ch_doorbell = dw_hdma_v0_core_ch_doorbell,
> > > .ch_enable = dw_hdma_v0_core_ch_enable,
> > > .ch_config = dw_hdma_v0_core_ch_config,
> > > --
> > > 2.51.0
> > >
next prev parent reply other threads:[~2026-07-24 15:36 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
2026-07-23 8:41 ` [PATCH v2 01/19] dmaengine: dw-edma: Add dw_edma_core_ll_cur_idx() to get current LL entry index Koichiro Den
2026-07-23 16:25 ` Frank Li
2026-07-23 8:41 ` [PATCH v2 02/19] dmaengine: dw-edma: Add dw_edma_core_ll_clear() to clear LL control-word Koichiro Den
2026-07-23 18:53 ` Frank Li
2026-07-23 8:41 ` [PATCH v2 03/19] dmaengine: dw-edma: Factor out linked-list transfer start Koichiro Den
2026-07-23 16:31 ` Frank Li
2026-07-23 8:41 ` [PATCH v2 04/19] dmaengine: dw-edma: Make DMA link list work as a circular buffer Koichiro Den
2026-07-23 9:07 ` sashiko-bot
2026-07-23 16:43 ` Frank Li
2026-07-24 6:23 ` Koichiro Den
2026-07-23 8:41 ` [PATCH v2 05/19] dmaengine: dw-edma: Add LL interrupt placement policy Koichiro Den
2026-07-23 19:58 ` Frank Li
2026-07-24 5:46 ` Koichiro Den
2026-07-24 15:36 ` Frank Li [this message]
2026-07-23 8:41 ` [PATCH v2 06/19] dmaengine: dw-edma: Move callback result helper before LL helpers Koichiro Den
2026-07-23 16:51 ` Frank Li
2026-07-23 8:41 ` [PATCH v2 07/19] dmaengine: dw-edma: Dispatch DONE interrupts by channel request Koichiro Den
2026-07-23 8:55 ` sashiko-bot
2026-07-23 16:57 ` Frank Li
2026-07-24 4:50 ` Koichiro Den
2026-07-24 15:42 ` Frank Li
2026-07-23 8:41 ` [PATCH v2 08/19] dmaengine: dw-edma: Centralize LL doorbell decisions Koichiro Den
2026-07-23 9:09 ` sashiko-bot
2026-07-23 17:02 ` Frank Li
2026-07-24 4:53 ` Koichiro Den
2026-07-23 8:41 ` [PATCH v2 09/19] dmaengine: dw-edma: Reclaim issued descriptors from IRQ-paired LL progress Koichiro Den
2026-07-23 9:01 ` sashiko-bot
2026-07-23 20:56 ` Frank Li
2026-07-24 5:17 ` Koichiro Den
2026-07-24 15:42 ` Frank Li
2026-07-23 8:41 ` [PATCH v2 10/19] dmaengine: dw-edma: Use HDMA watermarks as progress events Koichiro Den
2026-07-23 8:41 ` [PATCH v2 11/19] dmaengine: dw-edma: Reconcile lost completions from a stopped LLP re-sample Koichiro Den
2026-07-23 8:41 ` [PATCH v2 12/19] dmaengine: dw-edma: Recover stopped channels from tx_status() Koichiro Den
2026-07-23 8:59 ` sashiko-bot
2026-07-23 8:41 ` [PATCH v2 13/19] dmaengine: dw-edma: Make the LL ring reset a full channel resync Koichiro Den
2026-07-23 9:10 ` sashiko-bot
2026-07-23 8:41 ` [PATCH v2 14/19] dmaengine: dw-edma: Reset LL state after terminate and abort Koichiro Den
2026-07-23 9:14 ` sashiko-bot
2026-07-23 8:41 ` [PATCH v2 15/19] dmaengine: dw-edma: Add engine reset and enable operations Koichiro Den
2026-07-23 9:11 ` sashiko-bot
2026-07-23 8:41 ` [PATCH v2 16/19] dmaengine: dw-edma: Add engine recovery infrastructure Koichiro Den
2026-07-23 9:14 ` sashiko-bot
2026-07-23 8:41 ` [PATCH v2 17/19] dmaengine: dw-edma: Detect and recover a stalled eDMA engine Koichiro Den
2026-07-23 9:12 ` sashiko-bot
2026-07-23 8:41 ` [PATCH v2 18/19] dmaengine: dw-edma: Dynamically append requests while running Koichiro Den
2026-07-23 8:41 ` [PATCH v2 19/19] dmaengine: dw-edma: Add trace support Koichiro Den
2026-07-23 9:11 ` sashiko-bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=amOGiJe8fVRoLCMQ@SMW015318 \
--to=frank.li@oss.nxp.com \
--cc=Frank.Li@kernel.org \
--cc=Gustavo.Pimentel@synopsys.com \
--cc=bhelgaas@google.com \
--cc=cai.huoqing@linux.dev \
--cc=cassel@kernel.org \
--cc=den@valinux.co.jp \
--cc=devendra.verma@amd.com \
--cc=dmaengine@vger.kernel.org \
--cc=fancer.lancer@gmail.com \
--cc=hch@lst.de \
--cc=kees@kernel.org \
--cc=kishon@kernel.org \
--cc=kwilczynski@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mani@kernel.org \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.