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

[mdalam@xxxxxxxxxxxxxx]

On 2020-12-21 23:03, mdalam@xxxxxxxxxxxxxx wrote:
> On 2020-12-21 14:53, Vinod Koul wrote:
> > Hello,
> >
> > On 17-12-20, 20:07, Md Sadre Alam wrote:
> > > This change will add support for LOCK & UNLOCK flag bit support
> > > on CMD descriptor.
> > >
> > > If DMA_PREP_LOCK flag passed in prep_slave_sg then requester of this
> > > transaction wanted to lock the DMA controller for this transaction so
> > > BAM driver should set LOCK bit for the HW descriptor.
> > >
> > > If DMA_PREP_UNLOCK flag passed in prep_slave_sg then requester of 
> > > this
> > > transaction wanted to unlock the DMA controller.so BAM driver should 
> > > set
> > > UNLOCK bit for the HW descriptor.
> >
> > Can you explain why would we need to first lock and then unlock..? How
> > would this be used in real world.
> >
> > I have read a bit of documentation but is unclear to me. Also should
> > this be exposed as an API to users, sounds like internal to driver..?
>
> IPQ5018 SoC having only one Crypto Hardware Engine. This Crypto 
> Hardware Engine
> will be shared between A53 core & ubi32 core. There is two separate
> driver dedicated
> to A53 core and ubi32 core. So to use Crypto Hardware Engine
> parallelly for encryption/description
> we need bam locking mechanism. if one driver will submit the request
> for encryption/description
> to Crypto then first it has to set LOCK flag bit on command descriptor
> so that other pipes will
> get locked.
>
> The Pipe Locking/Unlocking will be only on command-descriptor. Upon
> encountering a command descriptor
> with LOCK bit set, The BAM will lock all other pipes not related to
> the current pipe group, and keep
> handling the current pipe only until it sees the UNLOCK set then it
> will release all locked pipes.
> locked pipe will not fetch new descriptors even if it got event/events
> adding more descriptors for
> this pipe (locked pipe).
>
> No need to expose as an API to user because its internal to driver, so
> while preparing command descriptor
> just we have to update the LOCK/UNLOCK flag.


ping! Is there any update on this ? Do you need any further info ?
> > > Signed-off-by: Md Sadre Alam <mdalam@xxxxxxxxxxxxxx>
> > > ---
> > >  Documentation/driver-api/dmaengine/provider.rst | 9 +++++++++
> > >  drivers/dma/qcom/bam_dma.c                      | 9 +++++++++
> > >  include/linux/dmaengine.h                       | 5 +++++
> > >  3 files changed, 23 insertions(+)
> > >
> > > diff --git a/Documentation/driver-api/dmaengine/provider.rst 
> > > b/Documentation/driver-api/dmaengine/provider.rst
> > > index ddb0a81..d7516e2 100644
> > > --- a/Documentation/driver-api/dmaengine/provider.rst
> > > +++ b/Documentation/driver-api/dmaengine/provider.rst
> > > @@ -599,6 +599,15 @@ DMA_CTRL_REUSE
> > >    - This flag is only supported if the channel reports the 
> > > DMA_LOAD_EOT
> > >      capability.
> > >
> > > +- DMA_PREP_LOCK
> > > +
> > > +  - If set , the client driver tells DMA controller I am locking you 
> > > for
> > > +    this transcation.
> > > +
> > > +- DMA_PREP_UNLOCK
> > > +
> > > +  - If set, the client driver will tells DMA controller I am 
> > > releasing the lock
> > > +
> > >  General Design Notes
> > >  ====================
> > >
> > > diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> > > index 4eeb8bb..cdbe395 100644
> > > --- a/drivers/dma/qcom/bam_dma.c
> > > +++ b/drivers/dma/qcom/bam_dma.c
> > > @@ -58,6 +58,8 @@ struct bam_desc_hw {
> > >  #define DESC_FLAG_EOB BIT(13)
> > >  #define DESC_FLAG_NWD BIT(12)
> > >  #define DESC_FLAG_CMD BIT(11)
> > > +#define DESC_FLAG_LOCK BIT(10)
> > > +#define DESC_FLAG_UNLOCK BIT(9)
> > >
> > >  struct bam_async_desc {
> > >  	struct virt_dma_desc vd;
> > > @@ -644,6 +646,13 @@ static struct dma_async_tx_descriptor 
> > > *bam_prep_slave_sg(struct dma_chan *chan,
> > >
> > >  	/* fill in temporary descriptors */
> > >  	desc = async_desc->desc;
> > > +	if (flags & DMA_PREP_CMD) {
> > > +		if (flags & DMA_PREP_LOCK)
> > > +			desc->flags |= cpu_to_le16(DESC_FLAG_LOCK);
> > > +		if (flags & DMA_PREP_UNLOCK)
> > > +			desc->flags |= cpu_to_le16(DESC_FLAG_UNLOCK);
> > > +	}
> > > +
> > >  	for_each_sg(sgl, sg, sg_len, i) {
> > >  		unsigned int remainder = sg_dma_len(sg);
> > >  		unsigned int curr_offset = 0;
> > > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> > > index dd357a7..79ccadb4 100644
> > > --- a/include/linux/dmaengine.h
> > > +++ b/include/linux/dmaengine.h
> > > @@ -190,6 +190,9 @@ struct dma_interleaved_template {
> > >   *  transaction is marked with DMA_PREP_REPEAT will cause the new 
> > > transaction
> > >   *  to never be processed and stay in the issued queue forever. The 
> > > flag is
> > >   *  ignored if the previous transaction is not a repeated 
> > > transaction.
> > > + * @DMA_PREP_LOCK: tell the driver that DMA HW engine going to be 
> > > locked for this
> > > + *  transaction , until not seen DMA_PREP_UNLOCK flag set.
> > > + * @DMA_PREP_UNLOCK: tell the driver to unlock the DMA HW engine.
> > >   */
> > >  enum dma_ctrl_flags {
> > >  	DMA_PREP_INTERRUPT = (1 << 0),
> > > @@ -202,6 +205,8 @@ enum dma_ctrl_flags {
> > >  	DMA_PREP_CMD = (1 << 7),
> > >  	DMA_PREP_REPEAT = (1 << 8),
> > >  	DMA_PREP_LOAD_EOT = (1 << 9),
> > > +	DMA_PREP_LOCK = (1 << 10),
> > > +	DMA_PREP_UNLOCK = (1 << 11),
> > >  };
> > >
> > >  /**
> > > --
> > > 2.7.4