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

[Frank Li]

On Thu, Feb 19, 2026 at 09:55:49AM +0000, Verma, Devendra wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> > -----Original Message-----
> > From: Frank Li <Frank.li@xxxxxxx>
> > Sent: Wednesday, February 18, 2026 9:20 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
> > > > > > config->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.
> >
>
> Thank you for the elaboration!
> I have few questions, why shall a DMA consumer decide to enable LL mode when the
> default mode supported is LL mode only?

LL mode only is software driver implement. Hardware support both LL mode
and no-LL mode. Previous driver implement only support LL mode. You try
to add non-LL mode. Choose straightforward forward method.

One indicate hardware capacity,  one actually used. Like PCI INTX and MSI.
If support MSI, most case use MSI. But still support switch to use INTX.

My key point avoid hidden beavior. Every branch is clean and
straightforward.

>
> If DMA consumer is trying to enable the LL mode, then one must be knowing the configuration
> of the controller that controller is working in non-LL mode,
> as LLP base address is not configured,then why to try and enable the LL mode?

The DMA consumer don't know these informaiton.

>
> The user need to know, at least, one detail from the above two cases.
>
> The use for non-LL mode is useful in the following scenario:
> - When user want to utilize the LL regions also for DMA data transfers.
> - For single and big chunks non-LL mode is useful in both use-cases when non-LL mode is default or
>   user enables it via peripheral_config params.
> - This addition, inadvertently, makes the DMA controller usable, for AMD (Xilinx) only, when the LLP
>   base addresses are not configured; it can be used in non-LL mode.

LL regions may not visiable,  User can use non-ll to config LL-region and
switch back to use LL-region to continue transfer. User may use non-ll
as indirectly reg access.

> For Synopsys, DMA controller
>   cannot be used in any mode if LLP base address is not configured.

Does spec said it? It doesn't make sense. it should be controlled by LLE
of DMA_CH_CONTROL1_OFF_RDCH_0.

>
> Based on the above points, if user is trying to enable LL mode when default mode is LL mode, it looks
> Intentionally making the choice when user is aware of the mode DMA controller operating in.
> Please let me know if this clarifies the doubt.

No API to get mode, only use set and test to know it.

Actually Needn't consider so complex. like functions API(x)

We just consider input x,

	validate x's input ragion,

	if x is out of region, just return error.

>
> > >
> > > > > 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
>
> I understand the concern here but it does not look good to piggyback the
> non_ll related information on the existing variable.
> The use of bool readily points out the information related to what mode is being intended
> but using the ll_region->sz is an inference the user has to make.
>
> Having ll_region->sz == 0 does not really tell it is non_ll mode or not, it can also mean that
> the size of LL region is zero while in LL mode which could be an error.
> This does not translate to support for non-LL mode. This brings the ambiguity.
> The introduction of the non_ll provides clarity and easy comparison with the similar
> choice (non_ll) provided by the DMA consumer in the dmaengine_slave_config().
> I request we shall retain the clarity here.

You can use helper(dw_chip_is_support_ll()) macro to check chip's capatiblity.

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