RE: [PATCH RESEND v10 2/2] dmaengine: dw-edma: Add non-LL mode

[Verma, Devendra]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Frank

Please check my responses inline.

Regards,
Devendra
> -----Original Message-----
> From: Frank Li <Frank.li@xxxxxxx>
> Sent: Tuesday, February 17, 2026 1:16 AM
> To: Verma, Devendra <Devendra.Verma@xxxxxxx>
> Cc: bhelgaas@xxxxxxxxxx; mani@xxxxxxxxxx; vkoul@xxxxxxxxxx;
> dmaengine@xxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; Simek, Michal <michal.simek@xxxxxxx>
> Subject: Re: [PATCH RESEND v10 2/2] dmaengine: dw-edma: Add non-LL
> mode
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> On Mon, Feb 16, 2026 at 04:25:46PM +0530, Devendra K Verma wrote:
> > AMD MDB IP supports Linked List (LL) mode as well as non-LL mode.
> > The current code does not have the mechanisms to enable the DMA
> > transactions using the non-LL mode. The following two cases are added
> > with this patch:
> > - For the AMD (Xilinx) only, when a valid physical base address of
> >   the device side DDR is not configured, then the IP can still be
> >   used in non-LL mode. For all the channels DMA transactions will
> >   be using the non-LL mode only. This, the default non-LL mode,
> >   is not applicable for Synopsys IP with the current code addition.
> >
> > - If the default mode is LL-mode, for both AMD (Xilinx) and Synosys,
> >   and if user wants to use non-LL mode then user can do so via
> >   configuring the peripheral_config param of dma_slave_config.
> >
> > Signed-off-by: Devendra K Verma <devendra.verma@xxxxxxx>
> > ---
> > Changes in v10
> >   Added the peripheral_config check only for HDMA IP in
> >   dw_edma_device_config().
> >   Replaced the loop with single entry retrieval for non-LL
> >   mode.
> >   Addressed review comments and handled the burst allocation
> >   by defining 'bursts_max' as per suggestions.
> >
> > Changes in v9
> >   Fixed compilation errors related to macro name mismatch.
> >
> > Changes in v8
> >   Cosmetic change related to comment and code.
> >
> > Changes in v7
> >   No change
> >
> > Changes in v6
> >   Gave definition to bits used for channel configuration.
> >   Removed the comment related to doorbell.
> >
> > Changes in v5
> >   Variable name 'nollp' changed to 'non_ll'.
> >   In the dw_edma_device_config() WARN_ON replaced with dev_err().
> >   Comments follow the 80-column guideline.
> >
> > Changes in v4
> >   No change
> >
> > Changes in v3
> >   No change
> >
> > Changes in v2
> >   Reverted the function return type to u64 for
> >   dw_edma_get_phys_addr().
> >
> > Changes in v1
> >   Changed the function return type for dw_edma_get_phys_addr().
> >   Corrected the typo raised in review.
> > ---
> >  drivers/dma/dw-edma/dw-edma-core.c    | 35 ++++++++++++++-
> >  drivers/dma/dw-edma/dw-edma-core.h    |  1 +
> >  drivers/dma/dw-edma/dw-edma-pcie.c    | 44 ++++++++++++------
> >  drivers/dma/dw-edma/dw-hdma-v0-core.c | 65
> > ++++++++++++++++++++++++++-  drivers/dma/dw-edma/dw-hdma-v0-
> regs.h |  1 +
> >  include/linux/dma/edma.h              |  1 +
> >  6 files changed, 132 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c
> > b/drivers/dma/dw-edma/dw-edma-core.c
> > index b43255f914f3..ef3d79a9f88d 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -223,6 +223,31 @@ static int dw_edma_device_config(struct
> dma_chan *dchan,
> >                                struct dma_slave_config *config)  {
> >       struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> > +     int non_ll = 0;
> > +
> > +     chan->non_ll = false;
> > +     if (chan->dw->chip->mf == EDMA_MF_HDMA_NATIVE) {
>
> Need handle EMDA case. if mf is EDMA, need return error when
> config->peripheral_config is not NULL. Or remove above check to make
> code work for both EDMA or HDMA.
>

For the case of EDMA, the behavior is unchanged.
Even if the config->peripheral_config param is set then it would be simply ignored.
This is retention of the previous behavior. This is done because device_slave_config
has other params which affect the behavior of the DMA transactions, we do not check
all those params and return any error. The error is returned only in the cases where
particular parameter from dma_slave_config is used and if the parameter is not as expected
or in the expected form. The parameter used from dma_slave_config for the particular
IP type need to be known first then it can be checked for its correctness. This is behavior
for the peripheral_config which is used for HDMA and thus error checked.

> > +             if (config->peripheral_config &&
> > +                 config->peripheral_size != sizeof(int)) {
> > +                     dev_err(dchan->device->dev,
> > +                             "config param peripheral size mismatch\n");
> > +                     return -EINVAL;
> > +             }
> > +
> > +             /*
> > +              * When there is no valid LLP base address available then the
> > +              * default DMA ops will use the non-LL mode.
> > +              *
> > +              * Cases where LL mode is enabled and client wants to use the
> > +              * non-LL mode then also client can do so via providing the
> > +              * peripheral_config param.
> > +              */
> > +             if (config->peripheral_config)
> > +                     non_ll = *(int *)config->peripheral_config;
> > +
> > +             if (chan->dw->chip->non_ll || (!chan->dw->chip->non_ll
> > + && non_ll))
>
> if chan->dw->chip->non_ll is true, It should return error if you set non_ll false
> because no LLP base available.
>

In case the 'chan->dw->chip->non_ll' is true, then the default mode becomes
non-LL for HDMA ONLY. There is no option to the user to configure the LL mode
by giving 'non_ll = false' as part of the config->peripheral_config.
The configuration of default non-LL mode depends on how the IP is programmed
by the user. The user is aware of the IP configuration. The check for non-LL option
provided by the user is useful when LL-mode is default. That is why the value of
non_ll == false is not even evaluated.

> > +                     chan->non_ll = true;
> > +     }
> >
> >       memcpy(&chan->config, config, sizeof(*config));
> >       chan->configured = true;
> > @@ -358,6 +383,7 @@ dw_edma_device_transfer(struct
> dw_edma_transfer *xfer)
> >       struct dw_edma_desc *desc;
> >       u64 src_addr, dst_addr;
> >       size_t fsz = 0;
> > +     u32 bursts_max;
> >       u32 cnt = 0;
> >       int i;
> >
> > @@ -415,6 +441,13 @@ dw_edma_device_transfer(struct
> dw_edma_transfer *xfer)
> >               return NULL;
> >       }
> >
> > +     /*
> > +      * For non-LL mode, only a single burst can be handled
> > +      * in a single chunk unlike LL mode where multiple bursts
> > +      * can be configured in a single chunk.
> > +      */
> > +     bursts_max = chan->non_ll ? 1 : chan->ll_max;
> > +
> >       desc = dw_edma_alloc_desc(chan);
> >       if (unlikely(!desc))
> >               goto err_alloc;
> > @@ -450,7 +483,7 @@ dw_edma_device_transfer(struct
> dw_edma_transfer *xfer)
> >               if (xfer->type == EDMA_XFER_SCATTER_GATHER && !sg)
> >                       break;
> >
> > -             if (chunk->bursts_alloc == chan->ll_max) {
> > +             if (chunk->bursts_alloc == bursts_max) {
> >                       chunk = dw_edma_alloc_chunk(desc);
> >                       if (unlikely(!chunk))
> >                               goto err_alloc; diff --git
> > a/drivers/dma/dw-edma/dw-edma-core.h
> > b/drivers/dma/dw-edma/dw-edma-core.h
> > index 71894b9e0b15..c8e3d196a549 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.h
> > +++ b/drivers/dma/dw-edma/dw-edma-core.h
> > @@ -86,6 +86,7 @@ struct dw_edma_chan {
> >       u8                              configured;
> >
> >       struct dma_slave_config         config;
> > +     bool                            non_ll;
> >  };
> >
> >  struct dw_edma_irq {
> > diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c
> > b/drivers/dma/dw-edma/dw-edma-pcie.c
> > index 3aefc48f8e0a..94621b0f87df 100644
> > --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> > +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> > @@ -295,6 +295,15 @@ static void
> dw_edma_pcie_get_xilinx_dma_data(struct pci_dev *pdev,
> >       pdata->devmem_phys_off = off;
> >  }
> >
> > +static u64 dw_edma_get_phys_addr(struct pci_dev *pdev,
> > +                              struct dw_edma_pcie_data *pdata,
> > +                              enum pci_barno bar) {
> > +     if (pdev->vendor == PCI_VENDOR_ID_XILINX)
> > +             return pdata->devmem_phys_off;
> > +     return pci_bus_address(pdev, bar); }
> > +
> >  static int dw_edma_pcie_probe(struct pci_dev *pdev,
> >                             const struct pci_device_id *pid)  { @@
> > -304,6 +313,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> >       struct dw_edma_chip *chip;
> >       int err, nr_irqs;
> >       int i, mask;
> > +     bool non_ll = false;
> >
> >       vsec_data = kmalloc(sizeof(*vsec_data), GFP_KERNEL);
> >       if (!vsec_data)
> > @@ -329,21 +339,24 @@ static int dw_edma_pcie_probe(struct pci_dev
> > *pdev,
> >
> >               /*
> >                * There is no valid address found for the LL memory
> > -              * space on the device side.
> > +              * space on the device side. In the absence of LL base
> > +              * address use the non-LL mode or simple mode supported by
> > +              * the HDMA IP.
> >                */
> >               if (vsec_data->devmem_phys_off ==
> DW_PCIE_XILINX_MDB_INVALID_ADDR)
> > -                     return -ENOMEM;
> > +                     non_ll = true;
> >
> >               /*
> >                * Configure the channel LL and data blocks if number of
> >                * channels enabled in VSEC capability are more than the
> >                * channels configured in xilinx_mdb_data.
> >                */
> > -             dw_edma_set_chan_region_offset(vsec_data, BAR_2, 0,
> > -                                            DW_PCIE_XILINX_MDB_LL_OFF_GAP,
> > -                                            DW_PCIE_XILINX_MDB_LL_SIZE,
> > -                                            DW_PCIE_XILINX_MDB_DT_OFF_GAP,
> > -                                            DW_PCIE_XILINX_MDB_DT_SIZE);
> > +             if (!non_ll)
> > +                     dw_edma_set_chan_region_offset(vsec_data, BAR_2, 0,
> > +                                                    DW_PCIE_XILINX_MDB_LL_OFF_GAP,
> > +                                                    DW_PCIE_XILINX_MDB_LL_SIZE,
> > +                                                    DW_PCIE_XILINX_MDB_DT_OFF_GAP,
> > +
> > + DW_PCIE_XILINX_MDB_DT_SIZE);
> >       }
> >
> >       /* Mapping PCI BAR regions */
> > @@ -391,6 +404,7 @@ static int dw_edma_pcie_probe(struct pci_dev
> *pdev,
> >       chip->mf = vsec_data->mf;
> >       chip->nr_irqs = nr_irqs;
> >       chip->ops = &dw_edma_pcie_plat_ops;
> > +     chip->non_ll = non_ll;
> >
> >       chip->ll_wr_cnt = vsec_data->wr_ch_cnt;
> >       chip->ll_rd_cnt = vsec_data->rd_ch_cnt; @@ -399,7 +413,7 @@
> > static int dw_edma_pcie_probe(struct pci_dev *pdev,
> >       if (!chip->reg_base)
> >               return -ENOMEM;
> >
> > -     for (i = 0; i < chip->ll_wr_cnt; i++) {
> > +     for (i = 0; i < chip->ll_wr_cnt && !non_ll; i++) {
> >               struct dw_edma_region *ll_region = &chip->ll_region_wr[i];
> >               struct dw_edma_region *dt_region = &chip->dt_region_wr[i];
> >               struct dw_edma_block *ll_block = &vsec_data->ll_wr[i];
> > @@ -410,7 +424,8 @@ static int dw_edma_pcie_probe(struct pci_dev
> *pdev,
> >                       return -ENOMEM;
> >
> >               ll_region->vaddr.io += ll_block->off;
> > -             ll_region->paddr = pci_bus_address(pdev, ll_block->bar);
> > +             ll_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
> > +                                                      ll_block->bar);
> >               ll_region->paddr += ll_block->off;
> >               ll_region->sz = ll_block->sz;
> >
> > @@ -419,12 +434,13 @@ static int dw_edma_pcie_probe(struct pci_dev
> *pdev,
> >                       return -ENOMEM;
> >
> >               dt_region->vaddr.io += dt_block->off;
> > -             dt_region->paddr = pci_bus_address(pdev, dt_block->bar);
> > +             dt_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
> > +                                                      dt_block->bar);
> >               dt_region->paddr += dt_block->off;
> >               dt_region->sz = dt_block->sz;
> >       }
> >
> > -     for (i = 0; i < chip->ll_rd_cnt; i++) {
> > +     for (i = 0; i < chip->ll_rd_cnt && !non_ll; i++) {
> >               struct dw_edma_region *ll_region = &chip->ll_region_rd[i];
> >               struct dw_edma_region *dt_region = &chip->dt_region_rd[i];
> >               struct dw_edma_block *ll_block = &vsec_data->ll_rd[i];
> > @@ -435,7 +451,8 @@ static int dw_edma_pcie_probe(struct pci_dev
> *pdev,
> >                       return -ENOMEM;
> >
> >               ll_region->vaddr.io += ll_block->off;
> > -             ll_region->paddr = pci_bus_address(pdev, ll_block->bar);
> > +             ll_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
> > +                                                      ll_block->bar);
> >               ll_region->paddr += ll_block->off;
> >               ll_region->sz = ll_block->sz;
> >
> > @@ -444,7 +461,8 @@ static int dw_edma_pcie_probe(struct pci_dev
> *pdev,
> >                       return -ENOMEM;
> >
> >               dt_region->vaddr.io += dt_block->off;
> > -             dt_region->paddr = pci_bus_address(pdev, dt_block->bar);
> > +             dt_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
> > +                                                      dt_block->bar);
> >               dt_region->paddr += dt_block->off;
> >               dt_region->sz = dt_block->sz;
> >       }
> > diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > index e3f8db4fe909..a1b04fec6310 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_core_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,69 @@ 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_core_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;
> > +
> > +     child = list_first_entry_or_null(&chunk->burst->list,
> > +                                      struct dw_edma_burst, list);
> > +     if (!child)
> > +             return;
> > +
> > +     SET_CH_32(dw, chan->dir, chan->id, ch_en, HDMA_V0_CH_EN);
> > +
> > +     /* 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);
> > +
> > +     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)
> > +             dw_hdma_v0_core_non_ll_start(chunk);
> > +     else
> > +             dw_hdma_v0_core_ll_start(chunk, first); }
> > +
> >  static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)  {
> >       struct dw_edma *dw = chan->dw;
> > diff --git a/drivers/dma/dw-edma/dw-hdma-v0-regs.h
> > b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
> > index eab5fd7177e5..7759ba9b4850 100644
> > --- a/drivers/dma/dw-edma/dw-hdma-v0-regs.h
> > +++ b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
> > @@ -12,6 +12,7 @@
> >  #include <linux/dmaengine.h>
> >
> >  #define HDMA_V0_MAX_NR_CH                    8
> > +#define HDMA_V0_CH_EN                                BIT(0)
> >  #define HDMA_V0_LOCAL_ABORT_INT_EN           BIT(6)
> >  #define HDMA_V0_REMOTE_ABORT_INT_EN          BIT(5)
> >  #define HDMA_V0_LOCAL_STOP_INT_EN            BIT(4)
> > diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h index
> > 3080747689f6..78ce31b049ae 100644
> > --- a/include/linux/dma/edma.h
> > +++ b/include/linux/dma/edma.h
> > @@ -99,6 +99,7 @@ struct dw_edma_chip {
> >       enum dw_edma_map_format mf;
> >
> >       struct dw_edma          *dw;
> > +     bool                    non_ll;
>
> Can you check ll_region directly? it should be equal to (ll_region->sz == 0) ?
>
> Frank
> >  };
> >
> >  /* Export to the platform drivers */
> > --
> > 2.43.0
> >