Re: [PATCH v4] dmaengine: qcom: gpi: Add GPI immediate DMA support for SPI protocol

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 12/5/2024 10:46 PM, Dmitry Baryshkov wrote:
On Thu, Dec 05, 2024 at 10:36:11PM +0530, Jyothi Kumar Seerapu wrote:
The DMA TRE(Transfer ring element) buffer contains the DMA
buffer address. Accessing data from this address can cause
significant delays in SPI transfers, which can be mitigated to
some extent by utilizing immediate DMA support.

QCOM GPI DMA hardware supports an immediate DMA feature for data
up to 8 bytes, storing the data directly in the DMA TRE buffer
instead of the DMA buffer address. This enhancement enables faster
SPI data transfers.

This optimization reduces the average transfer time from 25 us to
16 us for a single SPI transfer of 8 bytes length, with a clock
frequency of 50 MHz.

Signed-off-by: Jyothi Kumar Seerapu <quic_jseerapu@xxxxxxxxxxx>
---

v3 -> v4:
    - Instead using extra variable(immediate_dma) for Immediate dma
      condition check, made it to inlined.
    - Removed the extra brackets around Immediate dma condition check.

    Link to v3:
	https://lore.kernel.org/lkml/20241204122059.24239-1-quic_jseerapu@xxxxxxxxxxx/

v2 -> v3:
    - When to enable Immediate DMA support, control is moved to GPI driver
      from SPI driver.
    - Optimizations are done in GPI driver related to immediate dma changes.
    - Removed the immediate dma supported changes in qcom-gpi-dma.h file
      and handled in GPI driver.

    Link to v2:
	https://lore.kernel.org/all/20241128133351.24593-2-quic_jseerapu@xxxxxxxxxxx/
	https://lore.kernel.org/all/20241128133351.24593-3-quic_jseerapu@xxxxxxxxxxx/

v1 -> v2:
    - Separated the patches to dmaengine and spi subsystems
    - Removed the changes which are not required for this feature from
      qcom-gpi-dma.h file.
    - Removed the type conversions used in gpi_create_spi_tre.

    Link to v1:
	https://lore.kernel.org/lkml/20241121115201.2191-2-quic_jseerapu@xxxxxxxxxxx/

  drivers/dma/qcom/gpi.c | 31 ++++++++++++++++++++++++++-----
  1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
index 52a7c8f2498f..9d4fc760bbe6 100644
--- a/drivers/dma/qcom/gpi.c
+++ b/drivers/dma/qcom/gpi.c
@@ -27,6 +27,7 @@
  #define TRE_FLAGS_IEOT		BIT(9)
  #define TRE_FLAGS_BEI		BIT(10)
  #define TRE_FLAGS_LINK		BIT(11)
+#define TRE_FLAGS_IMMEDIATE_DMA	BIT(16)
  #define TRE_FLAGS_TYPE		GENMASK(23, 16)
/* SPI CONFIG0 WD0 */
@@ -64,6 +65,7 @@
/* DMA TRE */
  #define TRE_DMA_LEN		GENMASK(23, 0)
+#define TRE_DMA_IMMEDIATE_LEN	GENMASK(3, 0)
/* Register offsets from gpi-top */
  #define GPII_n_CH_k_CNTXT_0_OFFS(n, k)	(0x20000 + (0x4000 * (n)) + (0x80 * (k)))
@@ -1711,6 +1713,7 @@ static int gpi_create_spi_tre(struct gchan *chan, struct gpi_desc *desc,
  	dma_addr_t address;
  	struct gpi_tre *tre;
  	unsigned int i;
+	int len;
/* first create config tre if applicable */
  	if (direction == DMA_MEM_TO_DEV && spi->set_config) {
@@ -1763,14 +1766,32 @@ static int gpi_create_spi_tre(struct gchan *chan, struct gpi_desc *desc,
  	tre_idx++;
address = sg_dma_address(sgl);
-	tre->dword[0] = lower_32_bits(address);
-	tre->dword[1] = upper_32_bits(address);
+	len = sg_dma_len(sgl);
- tre->dword[2] = u32_encode_bits(sg_dma_len(sgl), TRE_DMA_LEN);
+	/* Support Immediate dma for write transfers for data length up to 8 bytes */
+	if (direction == DMA_MEM_TO_DEV && len <= 2 * sizeof(tre->dword[0])) {
+		/*
+		 * For Immediate dma, data length may not always be length of 8 bytes,
+		 * it can be length less than 8, hence initialize both dword's with 0
+		 */
+		tre->dword[0] = 0;
+		tre->dword[1] = 0;
+		memcpy(&tre->dword[0], sg_virt(sgl), len);
+
+		tre->dword[2] = u32_encode_bits(len, TRE_DMA_IMMEDIATE_LEN);
+	} else {
+		tre->dword[0] = lower_32_bits(address);
+		tre->dword[1] = upper_32_bits(address);
+
+		tre->dword[2] = u32_encode_bits(len, TRE_DMA_LEN);
+	}
tre->dword[3] = u32_encode_bits(TRE_TYPE_DMA, TRE_FLAGS_TYPE);
-	if (direction == DMA_MEM_TO_DEV)
-		tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_IEOT);
+	tre->dword[3] |= u32_encode_bits(direction == DMA_MEM_TO_DEV &&
+					 len <= 2 * sizeof(tre->dword[0]),
+					 TRE_FLAGS_IMMEDIATE_DMA);

Don't repeat the condition, put it inside if.
Moving logic of setting "TRE_FLAGS_IMMEDIATE_DMA" to inside 'if' causes this flag(16th bit) to overwrite due to "u32_encode_bits(TRE_TYPE_DMA, TRE_FLAGS_TYPE)" operation.

And so, instead using "TRE_TYPE_DMA" + "TRE_FLAGS_IMMEDIATE_DMA" for immediate dma type, will define separate macro "TRE_TYPE_IMMEDIATE_DMA" with type 0x11 and use it for immediate dma operation and existing "TRE_TYPE_DMA" for non-immediate dma operation.
As per hardware programming guide, type 0x11 for immediate dma.

#define TRE_TYPE_DMA            0x10
#define TRE_TYPE_IMMEDIATE_DMA  0x11

Please let me know if it is fine or any improvements/suggestions here.


+	tre->dword[3] |= u32_encode_bits(direction == DMA_MEM_TO_DEV,
+					 TRE_FLAGS_IEOT);
for (i = 0; i < tre_idx; i++)
  		dev_dbg(dev, "TRE:%d %x:%x:%x:%x\n", i, desc->tre[i].dword[0],
--
2.17.1






[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux