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

[mdalam]

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.

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







> Thanks