Messages in this thread | | | Date | Tue, 09 Feb 2021 22:09:20 +0530 | From | mdalam@codeauro ... | Subject | Re: [PATCH] dmaengine: qcom: bam_dma: Add LOCK and UNLOCK flag bit support |
| |
On 2021-02-01 21:20, mdalam@codeaurora.org wrote: > On 2021-02-01 12:13, Vinod Koul wrote: >> On 01-02-21, 11:52, mdalam@codeaurora.org wrote: >>> On 2021-02-01 11:35, Vinod Koul wrote: >>> > On 27-01-21, 23:56, mdalam@codeaurora.org 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
| |