Re: [PATCH v9 03/11] dmaengine: qcom: bam_dma: implement support for BAM locking

[Bartosz Golaszewski]

On Wed, Feb 25, 2026 at 10:52 AM Vinod Koul <vkoul@xxxxxxxxxx> wrote:
>
> > > >
> > > > Sorry for jumping late. But I disagree with the argument that the client drivers
> > > > have to set the LOCK/UNLOCK bit. These bits are specific to BAM DMA IP for
> > > > serializing the command descriptors from multiple entities. So DMA clients like
> > > > Crypto/NAND have no business in setting this flag. It is the job of the BAM
> > > > dmaengine driver to set/unset it at the start and end of the descriptor chain.
> > > >
> > >
> > > But what if a given client does not need locking? We don't want to enable it
> > > for everyone - as I explained before.
> > >
> >
> > That's not going to hurt. AFAIK, enabling locking wouldn't cause any notable
> > performance overhead.
>
> I was always skeptical on this one. I had never seen why locking should
> be pushed to clients. As Bjorn said it leads to more mess than worth it.
> Thanks Mnai
>

[snip]

> >
> > > >> > Reg Dmitry question above, this is dma hw capability, how will client
> > > >> > know if it has to lock on older rev of hardware or not...?
> > > >> >
> > > >> > > Also: only the crypto engine needs it for now, not all the other users
> > > >> > > of the BAM engine.
> > > >> >
> > > >>
> > > >> Trying to set the lock/unlock bits will make
> > > >> dmaengine_desc_attach_metadata() fail if HW does not support it.
> > > >>
> > > >
> > > > The BAM dmaengine driver *must* know based on the IP version whether it supports
> > > > the LOCK/UNLOCK bits or not, not the client drivers. How can the client drivers
> > > > know about the BAM DMA IP capability?
>
> Lock bits are on the BAM DMA IP or client? Can we not add this
> capability to BAM driver and lock for IPs that support
>

Lock bits are in the BAM command descriptors. We do in fact already
expose the layout of BAM command elements to consumers.

> >
> > > How do
> > > you handle the case where we need to lock the BAM, send an arbitrary number
> > > of descriptors from the client and then unlock it? How can the BAM know *when*
> > > to lock/unlock?
> > >
> >
> > BAM driver has to perform lock during issue_pending() and unlock while reporting
> > the completion using vchan_cookie_complete().
>
> Sounds good to me, thanks Mani
>

This sounds great in theory but submitting new descriptors while
you're starting the engines proves to be quite tricky. :(

Over the course of this week, I tried really hard to make this happen.
The fact that we have two descriptor chains - one with data and one
with command descriptors - prepared with two separate calls to
dmaengine_prep_slave_sg(), but which we want to wrap with a
lock/unlock means we'll most likely really need to find a way to
insert the dummy command descriptors the moment we want to flush the
queue. Though that also means that the consumers need to adjust - for
instance: they need to submit both the data and command descriptors
*before* calling dmaengine_issue_pending(). So there goes not
involving the clients in the locking logic.

I will give it another try on Monday, but there's also another
problem. The lock/unlock bits need to be attached to *real* command
descriptors. So we need an actual "dummy" register write. The HPG
evens says this is the way to do it and I verified that passing 0 in
addr and size fields of a command descriptor will result in an smmu
fault. In the current approach, we do a harmles write into the VERSION
register of the QCE. But the address of that register is not known to
the BAM driver. How do I tell the BAM driver which address to write
to? There's struct dma_slave_config that has dst_addr and src_addr
fields that could be reused but that's probably not really their
function.

Bart