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

[Verma, Devendra]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Frank Li <Frank.li@xxxxxxx>
> Sent: Friday, February 20, 2026 9:33 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
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> On Fri, Feb 20, 2026 at 11:47:59AM +0000, Verma, Devendra wrote:
> > [AMD Official Use Only - AMD Internal Distribution Only]
> >
> > Hi Frank
> > Please check my response inline
> >
> > Regards,
> > Devendra
> > > -----Original Message-----
> > > From: Frank Li <Frank.li@xxxxxxx>
> > > Sent: Thursday, February 19, 2026 10:38 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
> > > > > > > > > config->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.
> > > > >
> > > >
> > > > 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.
> > >
> >
> > Thanks! Will update in the next version.
> >
> > > >
> > > > > >
> > > > > > > > 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.
> > >
> >
> > I do not understand what you mean with the above statement.
> > But if it about writing a new function to check the LL mode support
> > then I think the current variable is good enough which provides good
> > readability and do not create any ambiguity compared to the ll region size
> comparison.
>
> It is not big deal,  use 'bool cap_non_ll: 1' in dw_edma_chip. So we add more
> cap flags in future.
>
> Frank
>

Hi Frank, could you elaborate what you mean by adding the cap flag? How it is going
To help identify the overall chip state?
I do not understand what is being implied here.

- Regards,
Devendra

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