dmaengine: qcom: bam_dma: Add LOCK and UNLOCK flag bit support

PATCH In review Md Sadre Alam 4 participants 22 messages Dec 17, 2020 – Feb 11, 2021

Subsystems (3)

Documentation Arm Qualcomm mailing list dmaengine

lore

Share & tools

lore

Md Sadre Alam

author

2020-12-17 14:38 UTC #0 lore raw

PATCH 1/1 dmaengine: qcom: bam_dma: Add LOCK and UNLOCK flag bit support

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.

Signed-off-by Md Sadre Alam <[email protected]>

---
 Documentation/driver-api/dmaengine/provider.rst | 9 +++++++++
 drivers/dma/qcom/bam_dma.c                      | 9 +++++++++
 include/linux/dmaengine.h                       | 5 +++++
 3 files changed, 23 insertions(+)

Documentation/driver-api/dmaengine/provider.rst +9 -0

index ddb0a81..d7516e2 100644
@@ -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
 ====================

drivers/dma/qcom/bam_dma.c +9 -0

index 4eeb8bb..cdbe395 100644
@@ -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;

include/linux/dmaengine.h +5 -0

index dd357a7..79ccadb4 100644
@@ -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),
 };
 
 /**

Signature · 1 line

2.7.4

Thara Gopinath

2020-12-19 03:36 UTC #1 lore raw

Quoted 10 lines · Md Sadre Alam

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.

Hi,

This is a generic question. What is the point of LOCK/UNLOCK with allocating LOCK groups to the individual dma channels? By default doesn't all channels fall in the same group. This would mean that
a lock does not prevent the dma controller from not executing a
transaction on the other channels.

Signature · 2 lines

Warm Regards
Thara

Quoted 79 lines

Signed-off-by Md Sadre Alam <[email protected]>

---
  Documentation/driver-api/dmaengine/provider.rst | 9 +++++++++
  drivers/dma/qcom/bam_dma.c                      | 9 +++++++++
  include/linux/dmaengine.h                       | 5 +++++
  3 files changed, 23 insertions(+)

Documentation/driver-api/dmaengine/provider.rst +9 -0

index ddb0a81..d7516e2 100644
@@ -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
  ====================

drivers/dma/qcom/bam_dma.c +9 -0

index 4eeb8bb..cdbe395 100644
@@ -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;

include/linux/dmaengine.h +5 -0

index dd357a7..79ccadb4 100644
@@ -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),
  };
 
  /**

[email protected]

author

2020-12-21 07:36 UTC #2 lore raw

Quoted 11 lines · Thara Gopinath

Quoted 10 lines · Md Sadre Alam
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.

Quoted 8 lines

Hi,
This is a generic question. What is the point of LOCK/UNLOCK with allocating LOCK groups to the individual dma channels? By default doesn't all channels fall in the same group. This would mean that a lock does not prevent the dma controller from not executing a transaction on the other channels.

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.

The actual locking is done on the new descriptor fetching for publishing, i.e. locked pipe will not fetch new descriptors even if it got event/events adding more descriptors for this pipe (locked pipe).

The bam LOCKING mechanism is needed where different cores needs to share same hardware block which use bam for their transaction. So if both cores wanted to access the hardware block in parallel via bam, then locking mechanism is needed for bam pipes.

Quoted 86 lines

--
Warm Regards
Thara
Quoted 82 lines
Signed-off-by Md Sadre Alam <[email protected]>
---
  Documentation/driver-api/dmaengine/provider.rst | 9 +++++++++
  drivers/dma/qcom/bam_dma.c                      | 9 +++++++++
  include/linux/dmaengine.h                       | 5 +++++
  3 files changed, 23 insertions(+)
Documentation/driver-api/dmaengine/provider.rst +0 -0
index ddb0a81..d7516e2 100644
@@ -599,6 +599,15 @@ DMA_CTRL_REUSE
    - This flag is only supported if the channel reports the 
DMA_LOAD_EOT
capability.
+- DMA_PREP_LOCK
Documentation/driver-api/dmaengine/provider.rst +2 -0
+
+  - If set , the client driver tells DMA controller I am locking you 
for
Documentation/driver-api/dmaengine/provider.rst +5 -0
+    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
drivers/dma/qcom/bam_dma.c +9 -0
@@ -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;
include/linux/dmaengine.h +0 -0
index dd357a7..79ccadb4 100644
@@ -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),
include/linux/dmaengine.h +2 -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),
  };
    /**
 

Thara Gopinath

2020-12-21 18:10 UTC #3 lore raw

Quoted 21 lines · On 12/21/20 2:35 AM, [email protected] wrote:

Quoted 11 lines · Thara Gopinath
Quoted 10 lines · Md Sadre Alam
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.
Quoted 8 lines
Hi,
This is a generic question. What is the point of LOCK/UNLOCK with allocating LOCK groups to the individual dma channels? By default doesn't all channels fall in the same group. This would mean that a lock does not prevent the dma controller from not executing a transaction on the other channels.

Quoted 5 lines

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.

So unless you assign pipe groups, this will not work as intended right? So this patch is only half of the solution. There should also be a patch allowing pipe groups to be assigned. Without that extra bit this patch does nothing , right ?

Signature · 2 lines

Warm Regards
Thara

[email protected]

author

2020-12-22 12:19 UTC #4 lore raw

Quoted 28 lines · Thara Gopinath

Quoted 21 lines · On 12/21/20 2:35 AM, [email protected] wrote:
Quoted 11 lines · Thara Gopinath
Quoted 10 lines · Md Sadre Alam
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.
Quoted 8 lines
Hi,
This is a generic question. What is the point of LOCK/UNLOCK with allocating LOCK groups to the individual dma channels? By default doesn't all channels fall in the same group. This would mean that a lock does not prevent the dma controller from not executing a transaction on the other channels.
Quoted 5 lines
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.

Quoted 4 lines

So unless you assign pipe groups, this will not work as intended right? So this patch is only half of the solution. There should also be a patch allowing pipe groups to be assigned. Without that extra bit this patch does nothing , right ?

Yes you are right.
We are having some register which will configure the pipe lock group. But these registers are not exposed to non-secure world. These registers only accessible through secure world. Currently in IPQ5018 SoC we are configuring these register in secure world to configure pipe lock group.

[email protected]

author

2021-01-12 09:31 UTC #5 lore raw

Quoted 34 lines · On 2020-12-22 17:48, [email protected] wrote:

Quoted 28 lines · Thara Gopinath
Quoted 21 lines · On 12/21/20 2:35 AM, [email protected] wrote:
Quoted 11 lines · Thara Gopinath
Quoted 10 lines · Md Sadre Alam
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.
Quoted 8 lines
Hi,
This is a generic question. What is the point of LOCK/UNLOCK with allocating LOCK groups to the individual dma channels? By default doesn't all channels fall in the same group. This would mean that a lock does not prevent the dma controller from not executing a transaction on the other channels.
Quoted 5 lines
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.
Quoted 4 lines
So unless you assign pipe groups, this will not work as intended right? So this patch is only half of the solution. There should also be a patch allowing pipe groups to be assigned. Without that extra bit this patch does nothing , right ?

Quoted 6 lines

Yes you are right.
We are having some register which will configure the pipe lock group. But these registers are not exposed to non-secure world. These registers only accessible through secure world. Currently in IPQ5018 SoC we are configuring
these register in secure world to configure pipe lock group.

ping! Is there any update on this ? Do you need any further info ?

Vinod Koul

maintainer

2020-12-21 09:48 UTC #6 lore raw

In reply to Md Sadre Alam

Hello,

Quoted 10 lines · Md Sadre Alam

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..?

Quoted 80 lines

Signed-off-by Md Sadre Alam <[email protected]>

---
 Documentation/driver-api/dmaengine/provider.rst | 9 +++++++++
 drivers/dma/qcom/bam_dma.c                      | 9 +++++++++
 include/linux/dmaengine.h                       | 5 +++++
 3 files changed, 23 insertions(+)

Documentation/driver-api/dmaengine/provider.rst +9 -0

index ddb0a81..d7516e2 100644
@@ -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
 ====================

drivers/dma/qcom/bam_dma.c +9 -0

index 4eeb8bb..cdbe395 100644
@@ -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;

include/linux/dmaengine.h +5 -0

index dd357a7..79ccadb4 100644
@@ -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),
 };
 
 /**

Signature · 1 line

2.7.4

Signature · 1 line

~Vinod

[email protected]

author

2020-12-21 17:34 UTC #7 lore raw

Quoted 14 lines · Vinod Koul

Hello,
Quoted 10 lines · Md Sadre Alam
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.

Quoted 6 lines

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.

Quoted 89 lines

Signed-off-by Md Sadre Alam <[email protected]>

---
 Documentation/driver-api/dmaengine/provider.rst | 9 +++++++++
 drivers/dma/qcom/bam_dma.c                      | 9 +++++++++
 include/linux/dmaengine.h                       | 5 +++++
 3 files changed, 23 insertions(+)

Documentation/driver-api/dmaengine/provider.rst +0 -0

index ddb0a81..d7516e2 100644
@@ -599,6 +599,15 @@ DMA_CTRL_REUSE
   - This flag is only supported if the channel reports the

DMA_LOAD_EOT
capability.

+- DMA_PREP_LOCK

Documentation/driver-api/dmaengine/provider.rst +2 -0

+
+  - If set , the client driver tells DMA controller I am locking you

for

Documentation/driver-api/dmaengine/provider.rst +5 -0

+    this transcation.
+
+- DMA_PREP_UNLOCK
+
+  - If set, the client driver will tells DMA controller I am

releasing the lock

Documentation/driver-api/dmaengine/provider.rst +1 -0

+
 General Design Notes
 ====================

drivers/dma/qcom/bam_dma.c +9 -0

index 4eeb8bb..cdbe395 100644
@@ -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);

drivers/dma/qcom/bam_dma.c +0 -0

 		unsigned int curr_offset = 0;

include/linux/dmaengine.h +0 -0

index dd357a7..79ccadb4 100644
@@ -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 {

include/linux/dmaengine.h +2 -1

 	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

[email protected]

author

2021-01-12 09:32 UTC #8 lore raw

Quoted 43 lines · On 2020-12-21 23:03, [email protected] wrote:

Quoted 14 lines · Vinod Koul
Hello,
Quoted 10 lines · Md Sadre Alam
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.
Quoted 6 lines
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).

Quoted 3 lines

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 ?

Quoted 91 lines

Signed-off-by Md Sadre Alam <[email protected]>

---
 Documentation/driver-api/dmaengine/provider.rst | 9 +++++++++
 drivers/dma/qcom/bam_dma.c                      | 9 +++++++++
 include/linux/dmaengine.h                       | 5 +++++
 3 files changed, 23 insertions(+)

Documentation/driver-api/dmaengine/provider.rst +0 -0

index ddb0a81..d7516e2 100644
@@ -599,6 +599,15 @@ DMA_CTRL_REUSE
   - This flag is only supported if the channel reports the

DMA_LOAD_EOT
capability.

+- DMA_PREP_LOCK

Documentation/driver-api/dmaengine/provider.rst +2 -0

+
+  - If set , the client driver tells DMA controller I am locking you

for

Documentation/driver-api/dmaengine/provider.rst +5 -0

+    this transcation.
+
+- DMA_PREP_UNLOCK
+
+  - If set, the client driver will tells DMA controller I am

releasing the lock

Documentation/driver-api/dmaengine/provider.rst +1 -0

+
 General Design Notes
 ====================

drivers/dma/qcom/bam_dma.c +9 -0

index 4eeb8bb..cdbe395 100644
@@ -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);

drivers/dma/qcom/bam_dma.c +0 -0

 		unsigned int curr_offset = 0;

include/linux/dmaengine.h +0 -0

index dd357a7..79ccadb4 100644
@@ -190,6 +190,9 @@ struct dma_interleaved_template {
  *  transaction is marked with DMA_PREP_REPEAT will cause the new

include/linux/dmaengine.h +2 -1

 	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

Vinod Koul

maintainer

2021-01-12 10:11 UTC #9 lore raw

Quoted 37 lines · On 12-01-21, 15:01, [email protected] wrote:

Quoted 36 lines · On 2020-12-21 23:03, [email protected] wrote:
Quoted 16 lines · Vinod Koul
Hello,
Quoted 7 lines · Md Sadre Alam
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.
Quoted 5 lines
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.
Quoted 6 lines
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.

Quoted 2 lines

Quoted 2 lines
The Pipe Locking/Unlocking will be only on command-descriptor. Upon encountering a command descriptor

Can you explain what is a cmd descriptor?

Quoted 11 lines

Quoted 11 lines
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.

So IIUC, no api right? it would be internal to driver..?

Signature · 1 line

~Vinod

[email protected]

author

2021-01-13 19:51 UTC #10 lore raw

Quoted 38 lines · Vinod Koul

Quoted 37 lines · On 12-01-21, 15:01, [email protected] wrote:
Quoted 36 lines · On 2020-12-21 23:03, [email protected] wrote:
Quoted 16 lines · Vinod Koul
Hello,
Quoted 7 lines · Md Sadre Alam
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.
Quoted 5 lines
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.
Quoted 6 lines
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.

Quoted 4 lines

Quoted 2 lines
Quoted 2 lines
The Pipe Locking/Unlocking will be only on command-descriptor. Upon encountering a command descriptor
Can you explain what is a cmd descriptor?

In BAM pipe descriptor structure there is a field called CMD (Command
descriptor).
CMD allows the SW to create descriptors of type Command which does not
generate any data transmissions
but configures registers in the Peripheral (write operations, and read
registers operations ).
Using command descriptor enables the SW to queue new configurations
between data transfers in advance.

Quoted 9 lines

Quoted 9 lines
Quoted 9 lines
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).

Quoted 5 lines

Quoted 3 lines
Quoted 3 lines
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.
So IIUC, no api right? it would be internal to driver..?

Yes its totally internal to deriver.

Vinod Koul

maintainer

2021-01-15 05:58 UTC #11 lore raw

Quoted 44 lines · On 14-01-21, 01:20, [email protected] wrote:

Quoted 38 lines · Vinod Koul
Quoted 37 lines · On 12-01-21, 15:01, [email protected] wrote:
Quoted 36 lines · On 2020-12-21 23:03, [email protected] wrote:
Quoted 16 lines · Vinod Koul
Hello,
Quoted 7 lines · Md Sadre Alam
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.
Quoted 5 lines
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.
Quoted 6 lines
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.
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
The Pipe Locking/Unlocking will be only on command-descriptor. Upon encountering a command descriptor
Can you explain what is a cmd descriptor?

Quoted 8 lines

In BAM pipe descriptor structure there is a field called CMD (Command
descriptor).
CMD allows the SW to create descriptors of type Command which does not
generate any data transmissions
but configures registers in the Peripheral (write operations, and read
registers operations ).
Using command descriptor enables the SW to queue new configurations
between data transfers in advance.

What and when is the CMD descriptor used for..?

Quoted 9 lines

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
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).

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
Quoted 3 lines
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.
So IIUC, no api right? it would be internal to driver..?
Yes its totally internal to deriver.

So no need for this patch then, right?

Signature · 1 line

~Vinod

[email protected]

author

2021-01-18 03:52 UTC #12 lore raw

Quoted 57 lines · Vinod Koul

Quoted 44 lines · On 14-01-21, 01:20, [email protected] wrote:
Quoted 38 lines · Vinod Koul
Quoted 37 lines · On 12-01-21, 15:01, [email protected] wrote:
Quoted 36 lines · On 2020-12-21 23:03, [email protected] wrote:
Quoted 16 lines · Vinod Koul
Hello,
Quoted 7 lines · Md Sadre Alam
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.
Quoted 5 lines
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.
Quoted 6 lines
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.
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
The Pipe Locking/Unlocking will be only on command-descriptor. Upon encountering a command descriptor
Can you explain what is a cmd descriptor?
Quoted 11 lines
In BAM pipe descriptor structure there is a field called CMD
(Command
descriptor).
CMD allows the SW to create descriptors of type Command which does
not
generate any data transmissions
but configures registers in the Peripheral (write operations, and
read
registers operations ).
Using command descriptor enables the SW to queue new configurations
between data transfers in advance.

Quoted 1 line

What and when is the CMD descriptor used for..?

CMD descriptor is mainly used for configuring controller register. We can read/write controller register via BAM using CMD descriptor
only.
CMD descriptor use command pipe for the transaction.

Quoted 14 lines

Quoted 14 lines
Quoted 14 lines
Quoted 13 lines
Quoted 13 lines
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.

Quoted 5 lines

Quoted 3 lines
Quoted 1 line
So IIUC, no api right? it would be internal to driver..?
Yes its totally internal to deriver.
So no need for this patch then, right?

This patch is needed , because if some hardware will shared between
multiple core
like A53 and ubi32 for example. In IPQ5018 there is only one crypto
engine and this will
be shared between A53 core and ubi32 core and in A53 core & ubi32 core
there are different
drivers is getting used. So if encryption/decryption request come at
same time from both the
driver then things will get messed up. So here we need LOCKING
mechanism. If first request is
from A53 core driver then this driver should lock all the pipes other
than pipe dedicated to
A53 core. So while preparing CMD descriptor driver should used this
flag "DMA_PREP_LOCK",
Since LOCK and UNLOCK flag bit we can set only on CMD descriptor. Once
request processed then
driver will set UNLOCK flag on CMD descriptor. Driver should use this
flag "DMA_PREP_UNLOCK"
while preparing CMD descriptor. Same logic will be apply for ubi32
core driver as well.

Vinod Koul

maintainer

2021-01-19 18:45 UTC #13 lore raw

Quoted 58 lines · On 18-01-21, 09:21, [email protected] wrote:

Quoted 57 lines · Vinod Koul
Quoted 55 lines · On 14-01-21, 01:20, [email protected] wrote:
Quoted 38 lines · Vinod Koul
Quoted 37 lines · On 12-01-21, 15:01, [email protected] wrote:
Quoted 36 lines · On 2020-12-21 23:03, [email protected] wrote:
Quoted 16 lines · Vinod Koul
Hello,
Quoted 7 lines · Md Sadre Alam
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.
Quoted 5 lines
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.
Quoted 6 lines
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.
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
The Pipe Locking/Unlocking will be only on command-descriptor. Upon encountering a command descriptor
Can you explain what is a cmd descriptor?
In BAM pipe descriptor structure there is a field called CMD
(Command
descriptor).
CMD allows the SW to create descriptors of type Command which does
not
generate any data transmissions
but configures registers in the Peripheral (write operations, and
read
registers operations ).
Using command descriptor enables the SW to queue new configurations
between data transfers in advance.

Quoted 5 lines

Quoted 1 line
What and when is the CMD descriptor used for..?
CMD descriptor is mainly used for configuring controller register. We can read/write controller register via BAM using CMD descriptor only. CMD descriptor use command pipe for the transaction.

In which use cases would you need to issue cmd descriptors..?

Quoted 20 lines

Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
Quoted 13 lines
Quoted 13 lines
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.
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
So IIUC, no api right? it would be internal to driver..?
Yes its totally internal to deriver.
So no need for this patch then, right?

Quoted 14 lines

This patch is needed , because if some hardware will shared between multiple core like A53 and ubi32 for example. In IPQ5018 there is only one crypto engine and this will be shared between A53 core and ubi32 core and in A53 core & ubi32 core there are different drivers is getting used. So if encryption/decryption request come at same time from both the driver then things will get messed up. So here we need LOCKING mechanism. If first request is from A53 core driver then this driver should lock all the pipes other than pipe dedicated to A53 core. So while preparing CMD descriptor driver should used this flag "DMA_PREP_LOCK", Since LOCK and UNLOCK flag bit we can set only on CMD descriptor. Once request processed then driver will set UNLOCK flag on CMD descriptor. Driver should use this flag "DMA_PREP_UNLOCK" while preparing CMD descriptor. Same logic will be apply for ubi32 core driver as well.

Why cant this be applied at driver level, based on txn being issued it can lock issue the txn and then unlock when done. I am not convinced yet that this needs to be exported to users and can be managed by dmaengine driver.

Thanks

Signature · 1 line

~Vinod

[email protected]

author

2021-01-27 18:27 UTC #14 lore raw

Quoted 59 lines · Vinod Koul

Quoted 58 lines · On 18-01-21, 09:21, [email protected] wrote:
Quoted 57 lines · Vinod Koul
Quoted 55 lines · On 14-01-21, 01:20, [email protected] wrote:
Quoted 38 lines · Vinod Koul
Quoted 37 lines · On 12-01-21, 15:01, [email protected] wrote:
Quoted 36 lines · On 2020-12-21 23:03, [email protected] wrote:
Quoted 16 lines · Vinod Koul
Hello,
Quoted 7 lines · Md Sadre Alam
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.
Quoted 5 lines
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.
Quoted 6 lines
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.
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
The Pipe Locking/Unlocking will be only on command-descriptor. Upon encountering a command descriptor
Can you explain what is a cmd descriptor?
In BAM pipe descriptor structure there is a field called CMD
(Command
descriptor).
CMD allows the SW to create descriptors of type Command which does
not
generate any data transmissions
but configures registers in the Peripheral (write operations, and
read
registers operations ).
Using command descriptor enables the SW to queue new configurations
between data transfers in advance.

Quoted 8 lines

Quoted 6 lines
Quoted 1 line
What and when is the CMD descriptor used for..?
CMD descriptor is mainly used for configuring controller register. We can read/write controller register via BAM using CMD descriptor
only.
CMD descriptor use command pipe for the transaction.
In which use cases would you need to issue cmd descriptors..?

In IPQ5018 there is only one Crypto engine and it will get shared
between
UBI32 core & A53 core. So here we need to use command descriptor
in-order to
perform LOCKING/UNLOCKING mechanism. Since LOCK/ULOCK flag we can set
only on
CMD descriptor.

Quoted 36 lines

Quoted 21 lines
Quoted 15 lines
Quoted 15 lines
Quoted 15 lines
Quoted 14 lines
Quoted 14 lines
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.
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
So IIUC, no api right? it would be internal to driver..?
Yes its totally internal to deriver.
So no need for this patch then, right?
Quoted 14 lines
This patch is needed , because if some hardware will shared between multiple core like A53 and ubi32 for example. In IPQ5018 there is only one crypto engine and this will be shared between A53 core and ubi32 core and in A53 core & ubi32 core there are different drivers is getting used. So if encryption/decryption request come at same time from both the driver then things will get messed up. So here we need LOCKING mechanism. If first request is from A53 core driver then this driver should lock all the pipes other than pipe dedicated to A53 core. So while preparing CMD descriptor driver should used this flag "DMA_PREP_LOCK", Since LOCK and UNLOCK flag bit we can set only on CMD descriptor. Once request processed then driver will set UNLOCK flag on CMD descriptor. Driver should use this flag "DMA_PREP_UNLOCK" while preparing CMD descriptor. Same logic will be apply for ubi32 core driver as well.

Quoted 4 lines

Why cant this be applied at driver level, based on txn being issued it can lock issue the txn and then unlock when done. I am not convinced yet that this needs to be exported to users and can be managed by dmaengine driver.

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.

if (flags & DMA_PREP_CMD) { <== check for descriptor type

		if (flags & DMA_PREP_LOCK)
			desc->flags |= cpu_to_le16(DESC_FLAG_LOCK); <== Actual LOCK flag 
setting on HW descriptor.
		if (flags & DMA_PREP_UNLOCK)
			desc->flags |= cpu_to_le16(DESC_FLAG_UNLOCK); <== Actual UNLOCK flag 
setting on HW descriptor.
	}

Quoted 2 lines

Thanks

Vinod Koul

maintainer

2021-02-01 06:29 UTC #15 lore raw

Quoted 67 lines · On 27-01-21, 23:56, [email protected] wrote:

Quoted 66 lines · Vinod Koul
Quoted 64 lines · On 18-01-21, 09:21, [email protected] wrote:
Quoted 57 lines · Vinod Koul
Quoted 44 lines · On 14-01-21, 01:20, [email protected] wrote:
Quoted 38 lines · Vinod Koul
Quoted 37 lines · On 12-01-21, 15:01, [email protected] wrote:
Quoted 36 lines · On 2020-12-21 23:03, [email protected] wrote:
Quoted 16 lines · Vinod Koul
Hello,
Quoted 7 lines · Md Sadre Alam
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.
Quoted 5 lines
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.
Quoted 6 lines
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.
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
The Pipe Locking/Unlocking will be only on command-descriptor. Upon encountering a command descriptor
Can you explain what is a cmd descriptor?
Quoted 11 lines
In BAM pipe descriptor structure there is a field called CMD
(Command
descriptor).
CMD allows the SW to create descriptors of type Command which does
not
generate any data transmissions
but configures registers in the Peripheral (write operations, and
read
registers operations ).
Using command descriptor enables the SW to queue new configurations
between data transfers in advance.
Quoted 1 line
What and when is the CMD descriptor used for..?
CMD descriptor is mainly used for configuring controller register. We can read/write controller register via BAM using CMD descriptor
only.
CMD descriptor use command pipe for the transaction.

Quoted 6 lines

Quoted 1 line
In which use cases would you need to issue cmd descriptors..?
In IPQ5018 there is only one Crypto engine and it will get shared between UBI32 core & A53 core. So here we need to use command descriptor in-order to perform LOCKING/UNLOCKING mechanism. Since LOCK/ULOCK flag we can set only on CMD descriptor.

So when will lock/unlock be performed? Can you please explain that..

Quoted 41 lines

Quoted 36 lines
Quoted 21 lines
Quoted 15 lines
Quoted 15 lines
Quoted 15 lines
Quoted 14 lines
Quoted 14 lines
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.
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
So IIUC, no api right? it would be internal to driver..?
Yes its totally internal to deriver.
So no need for this patch then, right?
Quoted 14 lines
This patch is needed , because if some hardware will shared between multiple core like A53 and ubi32 for example. In IPQ5018 there is only one crypto engine and this will be shared between A53 core and ubi32 core and in A53 core & ubi32 core there are different drivers is getting used. So if encryption/decryption request come at same time from both the driver then things will get messed up. So here we need LOCKING mechanism. If first request is from A53 core driver then this driver should lock all the pipes other than pipe dedicated to A53 core. So while preparing CMD descriptor driver should used this flag "DMA_PREP_LOCK", Since LOCK and UNLOCK flag bit we can set only on CMD descriptor. Once request processed then driver will set UNLOCK flag on CMD descriptor. Driver should use this flag "DMA_PREP_UNLOCK" while preparing CMD descriptor. Same logic will be apply for ubi32 core driver as well.
Quoted 4 lines
Why cant this be applied at driver level, based on txn being issued it can lock issue the txn and then unlock when done. I am not convinced yet that this needs to be exported to users and can be managed by dmaengine driver.

Quoted 10 lines

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..

Quoted 11 lines

if (flags & DMA_PREP_CMD) { <== check for descriptor type

		if (flags & DMA_PREP_LOCK)
			desc->flags |= cpu_to_le16(DESC_FLAG_LOCK); <== Actual LOCK flag setting
on HW descriptor.
		if (flags & DMA_PREP_UNLOCK)
			desc->flags |= cpu_to_le16(DESC_FLAG_UNLOCK); <== Actual UNLOCK flag
setting on HW descriptor.
	}

Quoted 2 lines

Thanks

Signature · 1 line

~Vinod

[email protected]

author

2021-02-01 06:36 UTC #16 lore raw

Quoted 68 lines · Vinod Koul

Quoted 67 lines · On 27-01-21, 23:56, [email protected] wrote:
Quoted 66 lines · Vinod Koul
Quoted 64 lines · On 18-01-21, 09:21, [email protected] wrote:
Quoted 57 lines · Vinod Koul
Quoted 44 lines · On 14-01-21, 01:20, [email protected] wrote:
Quoted 38 lines · Vinod Koul
Quoted 37 lines · On 12-01-21, 15:01, [email protected] wrote:
Quoted 36 lines · On 2020-12-21 23:03, [email protected] wrote:
Quoted 16 lines · Vinod Koul
Hello,
Quoted 7 lines · Md Sadre Alam
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.
Quoted 5 lines
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.
Quoted 6 lines
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.
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
The Pipe Locking/Unlocking will be only on command-descriptor. Upon encountering a command descriptor
Can you explain what is a cmd descriptor?
Quoted 11 lines
In BAM pipe descriptor structure there is a field called CMD
(Command
descriptor).
CMD allows the SW to create descriptors of type Command which does
not
generate any data transmissions
but configures registers in the Peripheral (write operations, and
read
registers operations ).
Using command descriptor enables the SW to queue new configurations
between data transfers in advance.
Quoted 1 line
What and when is the CMD descriptor used for..?
CMD descriptor is mainly used for configuring controller register. We can read/write controller register via BAM using CMD descriptor
only.
CMD descriptor use command pipe for the transaction.

Quoted 8 lines

Quoted 6 lines
Quoted 1 line
In which use cases would you need to issue cmd descriptors..?
In IPQ5018 there is only one Crypto engine and it will get shared between UBI32 core & A53 core. So here we need to use command descriptor in-order to perform LOCKING/UNLOCKING mechanism. Since LOCK/ULOCK flag we can set only on CMD descriptor.
So when will lock/unlock be performed? Can you please explain that..

LOCK/UNLOCK will be performed when two different driver wanted to use
the
same HW. eg. In IPQ5018 there is only one Crypto engine and it will be
shared b/w
UBI32 core driver and A53 core driver.

When A53 core wanted to submit request to crypto engine via BAM then
first it has to
LOCK all other pipes (pipe dedicated to UBI32 core) and then trigger
the transaction start.
Once all the transaction will be completed the A53 core crypto driver
will release the LOCK
from all the pipes. Same sequence will be applicable for UBI32 core
crypto driver as well.
It depends whose request will come first to the crypto HW.

Quoted 58 lines

Quoted 42 lines
Quoted 37 lines
Quoted 22 lines
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
Quoted 15 lines
Quoted 15 lines
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.
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
So IIUC, no api right? it would be internal to driver..?
Yes its totally internal to deriver.
So no need for this patch then, right?
Quoted 14 lines
This patch is needed , because if some hardware will shared between multiple core like A53 and ubi32 for example. In IPQ5018 there is only one crypto engine and this will be shared between A53 core and ubi32 core and in A53 core & ubi32 core there are different drivers is getting used. So if encryption/decryption request come at same time from both the driver then things will get messed up. So here we need LOCKING mechanism. If first request is from A53 core driver then this driver should lock all the pipes other than pipe dedicated to A53 core. So while preparing CMD descriptor driver should used this flag "DMA_PREP_LOCK", Since LOCK and UNLOCK flag bit we can set only on CMD descriptor. Once request processed then driver will set UNLOCK flag on CMD descriptor. Driver should use this flag "DMA_PREP_UNLOCK" while preparing CMD descriptor. Same logic will be apply for ubi32 core driver as well.
Quoted 4 lines
Why cant this be applied at driver level, based on txn being issued it can lock issue the txn and then unlock when done. I am not convinced yet that this needs to be exported to users and can be managed by dmaengine driver.
Quoted 14 lines
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.

Quoted 3 lines

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.

Quoted 14 lines

if (flags & DMA_PREP_CMD) { <== check for descriptor type

		if (flags & DMA_PREP_LOCK)
			desc->flags |= cpu_to_le16(DESC_FLAG_LOCK); <== Actual LOCK flag 
setting
on HW descriptor.
		if (flags & DMA_PREP_UNLOCK)
			desc->flags |= cpu_to_le16(DESC_FLAG_UNLOCK); <== Actual UNLOCK 
flag
setting on HW descriptor.
	}

Quoted 2 lines

Thanks

Vinod Koul

maintainer

2021-02-01 06:48 UTC #17 lore raw

Quoted 23 lines · On 01-02-21, 11:52, [email protected] wrote:

Quoted 18 lines · Vinod Koul
Quoted 6 lines · On 27-01-21, 23:56, [email protected] 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
Quoted 8 lines
"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.
Quoted 3 lines
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..

Quoted 13 lines

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();

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

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

Thanks

Signature · 1 line

~Vinod

[email protected]

author

2021-02-01 15:51 UTC #18 lore raw

Quoted 44 lines · Vinod Koul

Quoted 34 lines · On 01-02-21, 11:52, [email protected] wrote:
Quoted 18 lines · Vinod Koul
Quoted 6 lines · On 27-01-21, 23:56, [email protected] 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
Quoted 8 lines
"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.
Quoted 3 lines
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
Quoted 8 lines
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.

Quoted 5 lines

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.

Quoted 4 lines

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.

Quoted 10 lines

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

[email protected]

author

2021-02-09 16:41 UTC #19 lore raw

Quoted 100 lines · On 2021-02-01 21:20, [email protected] wrote:

Quoted 44 lines · Vinod Koul
Quoted 34 lines · On 01-02-21, 11:52, [email protected] wrote:
Quoted 18 lines · Vinod Koul
Quoted 6 lines · On 27-01-21, 23:56, [email protected] 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
Quoted 8 lines
"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.
Quoted 3 lines
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
Quoted 8 lines
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.
Quoted 5 lines
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.
Quoted 4 lines
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();

Quoted 8 lines

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?

Quoted 10 lines

Quoted 10 lines
Also if we do this, it needs to be done for specific platforms..
Thanks

Bjorn Andersson

2021-02-09 17:38 UTC #20 lore raw

In reply to [email protected]

Quoted 48 lines · On Mon 01 Feb 09:50 CST 2021, [email protected] wrote:

Quoted 47 lines · Vinod Koul
Quoted 34 lines · On 01-02-21, 11:52, [email protected] wrote:
Quoted 18 lines · Vinod Koul
Quoted 6 lines · On 27-01-21, 23:56, [email protected] 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
Quoted 8 lines
"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.
Quoted 3 lines
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
Quoted 8 lines
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.

Quoted 7 lines

Quoted 2 lines
        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.

So you have a single piece of crypto hardware and you're using the BAM's LOCK/UNLOCK feature to implement a "mutex" on a particular BAM channel?

Quoted 30 lines

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);
}

Quoted 5 lines

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.

It sounds fairly clear that the actual descriptor modification must happen in the BAM driver, but the question in my mind is how this is exposed to the DMAengine clients (e.g. crypto, QPIC etc).

What is the life span of the locked state? Do you always provide a series of descriptors that starts with a LOCK and ends with an UNLOCK? Or do you envision that the crypto driver provides a LOCK descriptor and at some later point provides a UNLOCK descriptor?

Finally, this patch just adds the BAM part of things, where is the patch that actually makes use of this feature?

Regards,
Bjorn

Quoted 27 lines

Quoted 4 lines
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.
Quoted 10 lines
Also if we do this, it needs to be done for specific platforms..
Thanks

[email protected]

author

2021-02-11 04:02 UTC #21 lore raw

Quoted 53 lines · Bjorn Andersson

Quoted 51 lines · On Mon 01 Feb 09:50 CST 2021, [email protected] wrote:
Quoted 49 lines · Vinod Koul
Quoted 34 lines · On 01-02-21, 11:52, [email protected] wrote:
Quoted 18 lines · Vinod Koul
Quoted 6 lines · On 27-01-21, 23:56, [email protected] 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
Quoted 8 lines
"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.
Quoted 3 lines
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
Quoted 8 lines
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();

Quoted 7 lines

Quoted 4 lines
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.
So you have a single piece of crypto hardware and you're using the BAM's LOCK/UNLOCK feature to implement a "mutex" on a particular BAM channel?

Yes, In IPQ5018 SoC we are having only one Crypto and it will be
shared between
UBI32 core & A53 core, and these two cores are running different
driver to use Crypto.
The LOCK/UNLOCK flag can be set only on CMD descriptor.

Quoted 43 lines

Quoted 35 lines
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);
}
Quoted 7 lines
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.

Quoted 3 lines

It sounds fairly clear that the actual descriptor modification must happen in the BAM driver, but the question in my mind is how this is exposed to the DMAengine clients (e.g. crypto, QPIC etc).

I have added these two flags "DMA_PREP_LOCK" & "DMA_PREP_UNLOCK" In
enum dma_ctrl_flags.

  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),
};

So these flags we get passed while preparing CMD descriptor in Crypto
driver. Based on these
flags only i am setting LOCK/UNLOCK flags on actual hardware descriptor
in BAM driver.

  if (flags & DMA_PREP_CMD) {
     if (flags & DMA_PREP_LOCK)
         desc->flags |= cpu_to_le16(DESC_FLAG_LOCK);

Quoted 6 lines

What is the life span of the locked state? Do you always provide a series of descriptors that starts with a LOCK and ends with an UNLOCK? Or do you envision that the crypto driver provides a LOCK descriptor and at some later point provides a UNLOCK descriptor?

While preparing CMD descriptor we will use this LOCK/UNLOCK flags. So
if i wanted to write
some 20 registers of Crypto HW via BAM then i will prepare multiple
command descriptor
let's say 20 CMD descriptor so in the very first CMD descriptor I will
set the LOCK (DMA_PREP_LOCK ) flag and
in the the last CMD descriptor I will set the UNLOCK (DMA_PREP_UNLOCK
) flag.

Quoted 4 lines

Finally, this patch just adds the BAM part of things, where is the patch that actually makes use of this feature?

Yes , this patch will add BAM part of things. For Crypto i will push
another patch
which will use this feature.

Quoted 33 lines

Regards,
Bjorn
Quoted 30 lines
Quoted 4 lines
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.
Quoted 10 lines
Also if we do this, it needs to be done for specific platforms..
Thanks