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

[Frank Li]

On Wed, Feb 18, 2026 at 11:32:57AM +0000, Verma, Devendra wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> > -----Original Message-----
> > From: Frank Li <Frank.li@xxxxxxx>
> > Sent: Tuesday, February 17, 2026 10:09 PM
> > 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
>
> ---[ Snipped some text to reduce mail size ]---
>
> > > > 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.
> >
> > It actaully hidden error. Your patch provide an option, which need't ll memory
> > to do DMA transfer. DMA consumer actaully don't know which backend used,
> > which is private information by DMA engine providor.
> >
> > dw-edma-pcie.c is only one of all edma users, which know DMA engine's
> > information because it creates this interface.
> >
> > PCIE-EP framework also create dmaegine, PCIE-EP function driver use DMA
> > standard interface to get dma-chan.
> >
> > if (config->peripheral_config) { /* only your specific dma consumer set it now
> > */
> >         /* optional config information */
> >         if (chan->dw->chip->mf != EDMA_MF_HDMA_NATIVE) {
> >                 dev_err("edma have not implmentent no-lll mode\n")
> >                 return -EINVAL
> >         }
> >
> >         ...
> > }
> >
> > Add EDMA support no-ll mode is quite easily in future.
> >
>
> This looks reasonable provided that HDMA got the support for this param.
> An error check can be added in the next revision.
> The addition may be slightly different as following:
> If (chan->dw->chip->mf == EDMA_MF_HDMA_NATIVE) {
> ...
> } else if (config->peripheral_config) {
>  /* error handling */
> }
>
> Using the above, if support needs to be added to EDMA then a check for correct 'mf'
> in the if() shall be sufficient.
>
> > >
> > > > > +             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.
> >
> > This is API's callback, you can't assume caller do all correct things.
> >
> > > 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.
> >
> > It is not true. DMA consumer don't know such detail information, which only
> > know which dma engineer providor.
> >
>
> For the DMA consumer the only option is LL mode as default mode. In order to
> use the non-LL mode it need to provide the parameter in the form of peripheral_config.
> Given the above statement, the consumer even if gives the 'non_ll = false', it is not going
> to change any behavior.
> Even if the user is not giving the option the assumption is that controller is in LL mode,
> unless the DMA engine provider provides the information regarding non-LL mode as default mode
> to the DMA consumer explicitly.
> In the case where chan->dw->chip->non_ll = true, following case may happen:
> - DMA consumer provides no peripheral_config param or simply config->peripheral_config = NULL,
>    in this case non_ll = false which is the current flow.
> - DMA consumer provides a valid peripheral_config (!= NULL) param but the value is '0', in this case
>   It is explicit but it would have the same effect as above case.
>
> DMA consumer is supposed to provide the only option non_ll as it was not available and LL mode
> is set as default for the DMA operations.
> When 'chan->dw->chip->non_ll = true' then this was added to make the chip usable when
> the LLP base addresses are not configured. Without this, user cannot use any of the modes
> be it LL or non-LL if the LLP base address is not configured.

little bit confuse, Maybe the same as you. I expected behavor

config->peripheral_config = NULL        choose hardware default one
					- 	    LL mode if hardware support
					-      none-LL mode if not ll list region

config->peripheral_config != NULL
EDMA: return false
HDMA:
		0			force to none_ll mode. (always success)
		1			force back to ll mode  (return false if no ll list region in chip)

DMA consumer decide if fall back to none_ll to continue.

>
> > > 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;
> > > > > +     }
> > > > >
> > ...
> > > > > 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)
> > ?
> >
> > Do you miss this questin?
> >
> > Frank
> >
>
> Yes, looks like I missed this question. Could you explain a little bit more? I am unable to understand the context.

you set chip->non_ll = non_ll in dw_edma_pcie_probe()

and only set ll_region->sz = ll_block->sz when !chip->non_ll.

Thats means ll_region->sz is 0 when chip->non_ll is true.

So non_ll have not bring new infomation into dw_edma_chip.

check !ll_region->sz, which should be equal to this non_ll.

dw_edma_chip is the exchange information between controller and dma core
driver. Needn't cache it here because you already save a copy in dma-chan.

Frank
>
> > > >
> > > > Frank
> > > > >  };
> > > > >
> > > > >  /* Export to the platform drivers */
> > > > > --
> > > > > 2.43.0
> > > > >