Re: [PATCH] dmaengine: qcom: bam_dma: Add LOCK and UNLOCK flag bit support

From: mdalam
Date: Tue Feb 09 2021 - 11:41:09 EST


On 2021-02-01 21:20, mdalam@xxxxxxxxxxxxxx wrote:
On 2021-02-01 12:13, Vinod Koul wrote:
On 01-02-21, 11:52, mdalam@xxxxxxxxxxxxxx wrote:
On 2021-02-01 11:35, Vinod Koul wrote:
> On 27-01-21, 23:56, mdalam@xxxxxxxxxxxxxx wrote:

> > The actual LOCK/UNLOCK flag should be set on hardware command
> > descriptor.
> > so this flag setting should be done in DMA engine driver. The user
> > of the
> > DMA
> > driver like (in case of IPQ5018) Crypto can use flag
> > "DMA_PREP_LOCK" &
> > "DMA_PREP_UNLOCK"
> > while preparing CMD descriptor before submitting to the DMA
> > engine. In DMA
> > engine driver
> > we are checking these flasgs on CMD descriptor and setting actual
> > LOCK/UNLOCK flag on hardware
> > descriptor.
>
>
> I am not sure I comprehend this yet.. when is that we would need to do
> this... is this for each txn submitted to dmaengine.. or something
> else..

Its not for each transaction submitted to dmaengine. We have to set this
only
once on CMD descriptor. So when A53 crypto driver need to change the crypto
configuration
then first it will lock the all other pipes using setting the LOCK flag bit
on CMD
descriptor and then it can start the transaction , on data descriptor this
flag will
not get set once all transaction will be completed the A53 crypto driver
release the lock on
all other pipes using UNLOCK flag on CMD descriptor. So LOCK/UNLOCK will be
only once and not for
the each transaction.

Okay so why cant the bam driver check cmd descriptor and do lock/unlock
as below, why do we need users to do this.

if (flags & DMA_PREP_CMD) {
do_lock_bam();

User will not decide to do this LOCK/UNLOCK mechanism. It depends on use case.
This LOCK/UNLOCK mechanism not required always. It needs only when
hardware will be shared
between different core with different driver.
The LOCK/UNLOCK flags provides SW to enter ordering between pipes execution.
(Generally, the BAM pipes are total independent from each other and
work in parallel manner).
This LOCK/UNLOCK flags are part of actual pipe hardware descriptor.

Pipe descriptor having the following flags:
INT : Interrupt
EOT: End of transfer
EOB: End of block
NWD: Notify when done
CMD: Command
LOCK: Lock
UNLOCK: Unlock
etc.

Here the BAM driver is common driver for (QPIC, Crypto, QUP etc. in IPQ5018)
So here only Crypto will be shared b/w multiple cores so For crypto
request only the LOCK/UNLOCK
mechanism required.
For other request like for QPIC driver, QUPT driver etc. its not
required. So Crypto driver has to raise the flag for
LOCK/UNLOCK while preparing CMD descriptor. The actual locking will
happen in BAM driver only using condition
if (flags & DMA_PREP_CMD) {
if (flags & DMA_PREP_LOCK)
desc->flags |= cpu_to_le16(DESC_FLAG_LOCK);
}

So Crypto driver should set this flag DMA_PREP_LOCK while preparing
CMD descriptor.
So LOCK should be set on actual hardware pipe descriptor with
descriptor type CMD.


The point here is that this seems to be internal to dma and should be
handled by dma driver.

This LOCK/UNLOK flags are part of actual hardware descriptor so this
should be handled by BAM driver only.
If we set condition like this
if (flags & DMA_PREP_CMD) {
do_lock_bam();
Then LOCK/UNLOCK will be applied for all the CMD descriptor
including (QPIC driver, QUP driver , Crypto driver etc.).
So this is not our intension. So we need to set this LOCK/UNLOCK
only for the drivers it needs. So Crypto driver needs
locking mechanism so we will set LOCK/UNLOCK flag on Crypto driver
request only for other driver request like QPIC driver,
QUP driver will not set this.


ping! Do you need any further info on this?

Also if we do this, it needs to be done for specific platforms..








Thanks