linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] spi: spi-omap2-mcspi: Use EOW interrupt for transfer completion
@ 2022-08-22  9:15 Vaishnav Achath
  2022-08-22  9:15 ` [PATCH 1/2] dmaengine: ti: k3-udma: Respond TX done if DMA_PREP_INTERRUPT is not requested Vaishnav Achath
  2022-08-22  9:15 ` [PATCH 2/2] spi: spi-omap2-mcspi: Use EOW interrupt for completion when DMA enabled Vaishnav Achath
  0 siblings, 2 replies; 9+ messages in thread
From: Vaishnav Achath @ 2022-08-22  9:15 UTC (permalink / raw)
  To: peter.ujfalusi, vkoul, broonie, dmaengine, linux-kernel, linux-spi
  Cc: vigneshr, kishon, vaishnav.a

When using MCSPI with DMA enabled in Master/Slave mode, real-time 
performance issues were observed which were root caused to the 
uncertain delays in the  TX completion calculation mechanism in
k3-udma driver.

This series updates the omap2-mcspi driver to use End of Word(EOW)
interrupt to identify transaction completion and remove the usage
of DMA rx_completion and tx_completion for identifying transaction
completion.

Tested on J721E SK (for both Master and Slave Mode) for Full Duplex,
TX Only and RX Only mode transactions.Also tested with ILI9225 based
SPI TFT Display.

Vaishnav Achath (2):
  dmaengine: ti: k3-udma: Respond TX done if DMA_PREP_INTERRUPT is not
    requested
  spi: spi-omap2-mcspi: Use EOW interrupt for completion when DMA
    enabled

 drivers/dma/ti/k3-udma.c      |   5 +-
 drivers/spi/spi-omap2-mcspi.c | 141 +++++++++-------------------------
 2 files changed, 40 insertions(+), 106 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] dmaengine: ti: k3-udma: Respond TX done if DMA_PREP_INTERRUPT is not requested
  2022-08-22  9:15 [PATCH 0/2] spi: spi-omap2-mcspi: Use EOW interrupt for transfer completion Vaishnav Achath
@ 2022-08-22  9:15 ` Vaishnav Achath
  2022-08-22 13:12   ` Péter Ujfalusi
  2022-08-22  9:15 ` [PATCH 2/2] spi: spi-omap2-mcspi: Use EOW interrupt for completion when DMA enabled Vaishnav Achath
  1 sibling, 1 reply; 9+ messages in thread
From: Vaishnav Achath @ 2022-08-22  9:15 UTC (permalink / raw)
  To: peter.ujfalusi, vkoul, broonie, dmaengine, linux-kernel, linux-spi
  Cc: vigneshr, kishon, vaishnav.a

When the DMA consumer driver does not expect the callback for TX done,
There is no need to perform the channel RT byte counter calculations 
and estimate the completion but return complete on first attempt itself.
This assumes that the consumer who did not request DMA_PREP_INTERRUPT 
has its own mechanism for understanding TX completion, example: MCSPI
EOW interrupt can be used as TX completion signal for a SPI transaction.

Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
---
 drivers/dma/ti/k3-udma.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
index 39b330ada200..03d579068453 100644
--- a/drivers/dma/ti/k3-udma.c
+++ b/drivers/dma/ti/k3-udma.c
@@ -263,6 +263,7 @@ struct udma_chan_config {
 	enum udma_tp_level channel_tpl; /* Channel Throughput Level */
 
 	u32 tr_trigger_type;
+	unsigned long tx_flags;
 
 	/* PKDMA mapped channel */
 	int mapped_channel_id;
@@ -1057,7 +1058,7 @@ static bool udma_is_desc_really_done(struct udma_chan *uc, struct udma_desc *d)
 
 	/* Only TX towards PDMA is affected */
 	if (uc->config.ep_type == PSIL_EP_NATIVE ||
-	    uc->config.dir != DMA_MEM_TO_DEV)
+	    uc->config.dir != DMA_MEM_TO_DEV || !(uc->config.tx_flags & DMA_PREP_INTERRUPT))
 		return true;
 
 	peer_bcnt = udma_tchanrt_read(uc, UDMA_CHAN_RT_PEER_BCNT_REG);
@@ -3418,6 +3419,8 @@ udma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 	if (!burst)
 		burst = 1;
 
+	uc->config.tx_flags = tx_flags;
+
 	if (uc->config.pkt_mode)
 		d = udma_prep_slave_sg_pkt(uc, sgl, sglen, dir, tx_flags,
 					   context);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] spi: spi-omap2-mcspi: Use EOW interrupt for completion when DMA enabled
  2022-08-22  9:15 [PATCH 0/2] spi: spi-omap2-mcspi: Use EOW interrupt for transfer completion Vaishnav Achath
  2022-08-22  9:15 ` [PATCH 1/2] dmaengine: ti: k3-udma: Respond TX done if DMA_PREP_INTERRUPT is not requested Vaishnav Achath
@ 2022-08-22  9:15 ` Vaishnav Achath
  2022-08-22 13:14   ` kernel test robot
  1 sibling, 1 reply; 9+ messages in thread
From: Vaishnav Achath @ 2022-08-22  9:15 UTC (permalink / raw)
  To: peter.ujfalusi, vkoul, broonie, dmaengine, linux-kernel, linux-spi
  Cc: vigneshr, kishon, vaishnav.a

In MCSPI controller EOW interrupt is triggered when the channel has
transmitted the set number of bytes in MCSPI_XFERLEVEL[31-16] WCNT,
this can be used to signal the completion of a TX/RX when the internal
FIFO is enabled, when DMA is enabled the internal FIFO is always enabled.
Waiting for the DMA completion adds unpredictable delays due to the
non-realtime completion calculation mechanism in k3-udma driver.

This commit removes the dma_tx_completion and dma_rx_completion and
relies on the MCSPI controller EOW interrupt to signal transaction
completion.This fixes the real-time performance issues in master and
slave mode when DMA was enabled which resulted from the DMA completion
calculation delays.

Since the MCSPI driver now uses internal mechanism to identify a transfer
completion we disable the TX and RX DMA completion callback and remove
DMA_PREP_INTERRUPT.

Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
---
 drivers/spi/spi-omap2-mcspi.c | 141 +++++++++-------------------------
 1 file changed, 36 insertions(+), 105 deletions(-)

diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
index c48d02bb7013..8680465533e0 100644
--- a/drivers/spi/spi-omap2-mcspi.c
+++ b/drivers/spi/spi-omap2-mcspi.c
@@ -91,10 +91,6 @@
 struct omap2_mcspi_dma {
 	struct dma_chan *dma_tx;
 	struct dma_chan *dma_rx;
-
-	struct completion dma_tx_completion;
-	struct completion dma_rx_completion;
-
 	char dma_rx_ch_name[14];
 	char dma_tx_ch_name[14];
 };
@@ -116,7 +112,7 @@ struct omap2_mcspi_regs {
 };
 
 struct omap2_mcspi {
-	struct completion	txdone;
+	struct completion	txrxdone;
 	struct spi_master	*master;
 	/* Virtual base address of the controller */
 	void __iomem		*base;
@@ -375,30 +371,6 @@ static int mcspi_wait_for_completion(struct  omap2_mcspi *mcspi,
 	return 0;
 }
 
-static void omap2_mcspi_rx_callback(void *data)
-{
-	struct spi_device *spi = data;
-	struct omap2_mcspi *mcspi = spi_master_get_devdata(spi->master);
-	struct omap2_mcspi_dma *mcspi_dma = &mcspi->dma_channels[spi->chip_select];
-
-	/* We must disable the DMA RX request */
-	omap2_mcspi_set_dma_req(spi, 1, 0);
-
-	complete(&mcspi_dma->dma_rx_completion);
-}
-
-static void omap2_mcspi_tx_callback(void *data)
-{
-	struct spi_device *spi = data;
-	struct omap2_mcspi *mcspi = spi_master_get_devdata(spi->master);
-	struct omap2_mcspi_dma *mcspi_dma = &mcspi->dma_channels[spi->chip_select];
-
-	/* We must disable the DMA TX request */
-	omap2_mcspi_set_dma_req(spi, 0, 0);
-
-	complete(&mcspi_dma->dma_tx_completion);
-}
-
 static void omap2_mcspi_tx_dma(struct spi_device *spi,
 				struct spi_transfer *xfer,
 				struct dma_slave_config cfg)
@@ -413,12 +385,9 @@ static void omap2_mcspi_tx_dma(struct spi_device *spi,
 	dmaengine_slave_config(mcspi_dma->dma_tx, &cfg);
 
 	tx = dmaengine_prep_slave_sg(mcspi_dma->dma_tx, xfer->tx_sg.sgl,
-				     xfer->tx_sg.nents,
-				     DMA_MEM_TO_DEV,
-				     DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+					xfer->tx_sg.nents, DMA_MEM_TO_DEV, DMA_CTRL_ACK);
+
 	if (tx) {
-		tx->callback = omap2_mcspi_tx_callback;
-		tx->callback_param = spi;
 		dmaengine_submit(tx);
 	} else {
 		/* FIXME: fall back to PIO? */
@@ -500,11 +469,9 @@ omap2_mcspi_rx_dma(struct spi_device *spi, struct spi_transfer *xfer,
 	}
 
 	tx = dmaengine_prep_slave_sg(mcspi_dma->dma_rx, sg_out[0],
-				     out_mapped_nents[0], DMA_DEV_TO_MEM,
-				     DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+				     out_mapped_nents[0], DMA_DEV_TO_MEM, DMA_CTRL_ACK);
+
 	if (tx) {
-		tx->callback = omap2_mcspi_rx_callback;
-		tx->callback_param = spi;
 		dmaengine_submit(tx);
 	} else {
 		/* FIXME: fall back to PIO? */
@@ -513,10 +480,10 @@ omap2_mcspi_rx_dma(struct spi_device *spi, struct spi_transfer *xfer,
 	dma_async_issue_pending(mcspi_dma->dma_rx);
 	omap2_mcspi_set_dma_req(spi, 1, 1);
 
-	ret = mcspi_wait_for_completion(mcspi, &mcspi_dma->dma_rx_completion);
+	ret = mcspi_wait_for_completion(mcspi, &mcspi->txrxdone);
+	omap2_mcspi_set_dma_req(spi, 1, 0);
 	if (ret || mcspi->slave_aborted) {
 		dmaengine_terminate_sync(mcspi_dma->dma_rx);
-		omap2_mcspi_set_dma_req(spi, 1, 0);
 		return 0;
 	}
 
@@ -587,8 +554,8 @@ omap2_mcspi_txrx_dma(struct spi_device *spi, struct spi_transfer *xfer)
 	enum dma_slave_buswidth width;
 	unsigned es;
 	void __iomem		*chstat_reg;
-	void __iomem            *irqstat_reg;
 	int			wait_res;
+	int ret;
 
 	mcspi = spi_master_get_devdata(spi->master);
 	mcspi_dma = &mcspi->dma_channels[spi->chip_select];
@@ -618,68 +585,36 @@ omap2_mcspi_txrx_dma(struct spi_device *spi, struct spi_transfer *xfer)
 	tx = xfer->tx_buf;
 
 	mcspi->slave_aborted = false;
-	reinit_completion(&mcspi_dma->dma_tx_completion);
-	reinit_completion(&mcspi_dma->dma_rx_completion);
-	reinit_completion(&mcspi->txdone);
-	if (tx) {
-		/* Enable EOW IRQ to know end of tx in slave mode */
-		if (spi_controller_is_slave(spi->master))
-			mcspi_write_reg(spi->master,
-					OMAP2_MCSPI_IRQENABLE,
-					OMAP2_MCSPI_IRQSTATUS_EOW);
+	reinit_completion(&mcspi->txrxdone);
+	mcspi_write_reg(spi->master, OMAP2_MCSPI_IRQENABLE,	OMAP2_MCSPI_IRQSTATUS_EOW);
+	if (tx)
 		omap2_mcspi_tx_dma(spi, xfer, cfg);
-	}
 
-	if (rx != NULL)
+	if (rx)
 		count = omap2_mcspi_rx_dma(spi, xfer, cfg, es);
 
-	if (tx != NULL) {
-		int ret;
-
-		ret = mcspi_wait_for_completion(mcspi, &mcspi_dma->dma_tx_completion);
-		if (ret || mcspi->slave_aborted) {
-			dmaengine_terminate_sync(mcspi_dma->dma_tx);
-			omap2_mcspi_set_dma_req(spi, 0, 0);
-			return 0;
-		}
-
-		if (spi_controller_is_slave(mcspi->master)) {
-			ret = mcspi_wait_for_completion(mcspi, &mcspi->txdone);
-			if (ret || mcspi->slave_aborted)
-				return 0;
-		}
+	ret = mcspi_wait_for_completion(mcspi, &mcspi->txrxdone);
+	omap2_mcspi_set_dma_req(spi, 0, 0);
+	if (ret || mcspi->slave_aborted)
+		return 0;
 
+	/* for TX_ONLY mode, be sure all words have shifted out */
+	if (tx && !rx) {
+		chstat_reg = cs->base + OMAP2_MCSPI_CHSTAT0;
 		if (mcspi->fifo_depth > 0) {
-			irqstat_reg = mcspi->base + OMAP2_MCSPI_IRQSTATUS;
-
-			if (mcspi_wait_for_reg_bit(irqstat_reg,
-						OMAP2_MCSPI_IRQSTATUS_EOW) < 0)
-				dev_err(&spi->dev, "EOW timed out\n");
-
-			mcspi_write_reg(mcspi->master, OMAP2_MCSPI_IRQSTATUS,
-					OMAP2_MCSPI_IRQSTATUS_EOW);
-		}
-
-		/* for TX_ONLY mode, be sure all words have shifted out */
-		if (rx == NULL) {
-			chstat_reg = cs->base + OMAP2_MCSPI_CHSTAT0;
-			if (mcspi->fifo_depth > 0) {
-				wait_res = mcspi_wait_for_reg_bit(chstat_reg,
-						OMAP2_MCSPI_CHSTAT_TXFFE);
-				if (wait_res < 0)
-					dev_err(&spi->dev, "TXFFE timed out\n");
-			} else {
-				wait_res = mcspi_wait_for_reg_bit(chstat_reg,
-						OMAP2_MCSPI_CHSTAT_TXS);
-				if (wait_res < 0)
-					dev_err(&spi->dev, "TXS timed out\n");
-			}
-			if (wait_res >= 0 &&
-				(mcspi_wait_for_reg_bit(chstat_reg,
-					OMAP2_MCSPI_CHSTAT_EOT) < 0))
-				dev_err(&spi->dev, "EOT timed out\n");
+			wait_res = mcspi_wait_for_reg_bit(chstat_reg, OMAP2_MCSPI_CHSTAT_TXFFE);
+			if (wait_res < 0)
+				dev_err(&spi->dev, "TXFFE timed out\n");
+		} else {
+			wait_res = mcspi_wait_for_reg_bit(chstat_reg, OMAP2_MCSPI_CHSTAT_TXS);
+			if (wait_res < 0)
+				dev_err(&spi->dev, "TXS timed out\n");
 		}
+		if (wait_res >= 0 && (mcspi_wait_for_reg_bit(chstat_reg,
+							     OMAP2_MCSPI_CHSTAT_EOT) < 0))
+			dev_err(&spi->dev, "EOT timed out\n");
 	}
+
 	return count;
 }
 
@@ -1010,9 +945,6 @@ static int omap2_mcspi_request_dma(struct omap2_mcspi *mcspi,
 		mcspi_dma->dma_rx = NULL;
 	}
 
-	init_completion(&mcspi_dma->dma_rx_completion);
-	init_completion(&mcspi_dma->dma_tx_completion);
-
 no_dma:
 	return ret;
 }
@@ -1102,8 +1034,10 @@ static irqreturn_t omap2_mcspi_irq_handler(int irq, void *data)
 
 	/* Disable IRQ and wakeup slave xfer task */
 	mcspi_write_reg(mcspi->master, OMAP2_MCSPI_IRQENABLE, 0);
-	if (irqstat & OMAP2_MCSPI_IRQSTATUS_EOW)
-		complete(&mcspi->txdone);
+	if (irqstat & OMAP2_MCSPI_IRQSTATUS_EOW) {
+		complete_all(&mcspi->txrxdone);
+		mcspi_write_reg(mcspi->master, OMAP2_MCSPI_IRQSTATUS, OMAP2_MCSPI_IRQSTATUS_EOW);
+	}
 
 	return IRQ_HANDLED;
 }
@@ -1111,12 +1045,9 @@ static irqreturn_t omap2_mcspi_irq_handler(int irq, void *data)
 static int omap2_mcspi_slave_abort(struct spi_master *master)
 {
 	struct omap2_mcspi *mcspi = spi_master_get_devdata(master);
-	struct omap2_mcspi_dma *mcspi_dma = mcspi->dma_channels;
 
 	mcspi->slave_aborted = true;
-	complete(&mcspi_dma->dma_rx_completion);
-	complete(&mcspi_dma->dma_tx_completion);
-	complete(&mcspi->txdone);
+	complete_all(&mcspi->txrxdone);
 
 	return 0;
 }
@@ -1516,7 +1447,7 @@ static int omap2_mcspi_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "no irq resource found\n");
 		goto free_master;
 	}
-	init_completion(&mcspi->txdone);
+	init_completion(&mcspi->txrxdone);
 	status = devm_request_irq(&pdev->dev, status,
 				  omap2_mcspi_irq_handler, 0, pdev->name,
 				  mcspi);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dmaengine: ti: k3-udma: Respond TX done if DMA_PREP_INTERRUPT is not requested
  2022-08-22  9:15 ` [PATCH 1/2] dmaengine: ti: k3-udma: Respond TX done if DMA_PREP_INTERRUPT is not requested Vaishnav Achath
@ 2022-08-22 13:12   ` Péter Ujfalusi
  2022-08-23  6:57     ` Vaishnav Achath
  0 siblings, 1 reply; 9+ messages in thread
From: Péter Ujfalusi @ 2022-08-22 13:12 UTC (permalink / raw)
  To: Vaishnav Achath, vkoul, broonie, dmaengine, linux-kernel, linux-spi
  Cc: vigneshr, kishon



On 22/08/2022 12:15, Vaishnav Achath wrote:
> When the DMA consumer driver does not expect the callback for TX done,
> There is no need to perform the channel RT byte counter calculations 
> and estimate the completion but return complete on first attempt itself.
> This assumes that the consumer who did not request DMA_PREP_INTERRUPT 
> has its own mechanism for understanding TX completion, example: MCSPI
> EOW interrupt can be used as TX completion signal for a SPI transaction.

The check is in place to make sure that we don't leave stale data in the
DMA fabric.
If you drop the check then it is going to be possible that some TX data
is going to be lost.
Could be one out of 10K transfers or 100K, but if that happens it is not
going to be easy to figure out.
Let's say we go the packet back, but PDMA is still have data to send and
the IP stops transmitting (externally clocked bus, some delay, etc).
Is it going to be OK to disable the channel?

> 
> Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
> ---
>  drivers/dma/ti/k3-udma.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
> index 39b330ada200..03d579068453 100644
> --- a/drivers/dma/ti/k3-udma.c
> +++ b/drivers/dma/ti/k3-udma.c
> @@ -263,6 +263,7 @@ struct udma_chan_config {
>  	enum udma_tp_level channel_tpl; /* Channel Throughput Level */
>  
>  	u32 tr_trigger_type;
> +	unsigned long tx_flags;
>  
>  	/* PKDMA mapped channel */
>  	int mapped_channel_id;
> @@ -1057,7 +1058,7 @@ static bool udma_is_desc_really_done(struct udma_chan *uc, struct udma_desc *d)
>  
>  	/* Only TX towards PDMA is affected */
>  	if (uc->config.ep_type == PSIL_EP_NATIVE ||
> -	    uc->config.dir != DMA_MEM_TO_DEV)
> +	    uc->config.dir != DMA_MEM_TO_DEV || !(uc->config.tx_flags & DMA_PREP_INTERRUPT))
>  		return true;
>  
>  	peer_bcnt = udma_tchanrt_read(uc, UDMA_CHAN_RT_PEER_BCNT_REG);
> @@ -3418,6 +3419,8 @@ udma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>  	if (!burst)
>  		burst = 1;
>  
> +	uc->config.tx_flags = tx_flags;
> +
>  	if (uc->config.pkt_mode)
>  		d = udma_prep_slave_sg_pkt(uc, sgl, sglen, dir, tx_flags,
>  					   context);

-- 
Péter

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] spi: spi-omap2-mcspi: Use EOW interrupt for completion when DMA enabled
  2022-08-22  9:15 ` [PATCH 2/2] spi: spi-omap2-mcspi: Use EOW interrupt for completion when DMA enabled Vaishnav Achath
@ 2022-08-22 13:14   ` kernel test robot
  0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2022-08-22 13:14 UTC (permalink / raw)
  To: Vaishnav Achath, peter.ujfalusi, vkoul, broonie, dmaengine,
	linux-kernel, linux-spi
  Cc: kbuild-all, vigneshr, kishon, vaishnav.a

Hi Vaishnav,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on vkoul-dmaengine/next]
[also build test WARNING on broonie-spi/for-next linus/master v6.0-rc2 next-20220822]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Vaishnav-Achath/spi-spi-omap2-mcspi-Use-EOW-interrupt-for-transfer-completion/20220822-171807
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git next
config: arc-randconfig-r043-20220821 (https://download.01.org/0day-ci/archive/20220822/202208222100.SKGSz2m9-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/492d819558f7cb9fb64d860042c6ca17a054c3f7
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Vaishnav-Achath/spi-spi-omap2-mcspi-Use-EOW-interrupt-for-transfer-completion/20220822-171807
        git checkout 492d819558f7cb9fb64d860042c6ca17a054c3f7
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash drivers/spi/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/spi/spi-omap2-mcspi.c: In function 'omap2_mcspi_txrx_dma':
>> drivers/spi/spi-omap2-mcspi.c:549:34: warning: variable 'mcspi_dma' set but not used [-Wunused-but-set-variable]
     549 |         struct omap2_mcspi_dma  *mcspi_dma;
         |                                  ^~~~~~~~~


vim +/mcspi_dma +549 drivers/spi/spi-omap2-mcspi.c

d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  543  
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  544  static unsigned
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  545  omap2_mcspi_txrx_dma(struct spi_device *spi, struct spi_transfer *xfer)
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  546  {
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  547  	struct omap2_mcspi	*mcspi;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  548  	struct omap2_mcspi_cs	*cs = spi->controller_state;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11 @549  	struct omap2_mcspi_dma  *mcspi_dma;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  550  	unsigned int		count;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  551  	u8			*rx;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  552  	const u8		*tx;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  553  	struct dma_slave_config	cfg;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  554  	enum dma_slave_buswidth width;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  555  	unsigned es;
e47a682ace0cd5 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-11-06  556  	void __iomem		*chstat_reg;
d33f473dcd8e69 drivers/spi/spi-omap2-mcspi.c Illia Smyrnov   2013-06-17  557  	int			wait_res;
492d819558f7cb drivers/spi/spi-omap2-mcspi.c Vaishnav Achath 2022-08-22  558  	int ret;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  559  
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  560  	mcspi = spi_master_get_devdata(spi->master);
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  561  	mcspi_dma = &mcspi->dma_channels[spi->chip_select];
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  562  
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  563  	if (cs->word_len <= 8) {
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  564  		width = DMA_SLAVE_BUSWIDTH_1_BYTE;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  565  		es = 1;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  566  	} else if (cs->word_len <= 16) {
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  567  		width = DMA_SLAVE_BUSWIDTH_2_BYTES;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  568  		es = 2;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  569  	} else {
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  570  		width = DMA_SLAVE_BUSWIDTH_4_BYTES;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  571  		es = 4;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  572  	}
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  573  
d33f473dcd8e69 drivers/spi/spi-omap2-mcspi.c Illia Smyrnov   2013-06-17  574  	count = xfer->len;
d33f473dcd8e69 drivers/spi/spi-omap2-mcspi.c Illia Smyrnov   2013-06-17  575  
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  576  	memset(&cfg, 0, sizeof(cfg));
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  577  	cfg.src_addr = cs->phys + OMAP2_MCSPI_RX0;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  578  	cfg.dst_addr = cs->phys + OMAP2_MCSPI_TX0;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  579  	cfg.src_addr_width = width;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  580  	cfg.dst_addr_width = width;
baf8b9f8d260c5 drivers/spi/spi-omap2-mcspi.c Vignesh R       2019-01-15  581  	cfg.src_maxburst = 1;
baf8b9f8d260c5 drivers/spi/spi-omap2-mcspi.c Vignesh R       2019-01-15  582  	cfg.dst_maxburst = 1;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  583  
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  584  	rx = xfer->rx_buf;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  585  	tx = xfer->tx_buf;
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  586  
89e8b9cb846515 drivers/spi/spi-omap2-mcspi.c Vignesh R       2018-10-15  587  	mcspi->slave_aborted = false;
492d819558f7cb drivers/spi/spi-omap2-mcspi.c Vaishnav Achath 2022-08-22  588  	reinit_completion(&mcspi->txrxdone);
492d819558f7cb drivers/spi/spi-omap2-mcspi.c Vaishnav Achath 2022-08-22  589  	mcspi_write_reg(spi->master, OMAP2_MCSPI_IRQENABLE,	OMAP2_MCSPI_IRQSTATUS_EOW);
492d819558f7cb drivers/spi/spi-omap2-mcspi.c Vaishnav Achath 2022-08-22  590  	if (tx)
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  591  		omap2_mcspi_tx_dma(spi, xfer, cfg);
d7b4394e780b02 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-09-11  592  
492d819558f7cb drivers/spi/spi-omap2-mcspi.c Vaishnav Achath 2022-08-22  593  	if (rx)
e47a682ace0cd5 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-11-06  594  		count = omap2_mcspi_rx_dma(spi, xfer, cfg, es);
e47a682ace0cd5 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-11-06  595  
492d819558f7cb drivers/spi/spi-omap2-mcspi.c Vaishnav Achath 2022-08-22  596  	ret = mcspi_wait_for_completion(mcspi, &mcspi->txrxdone);
89e8b9cb846515 drivers/spi/spi-omap2-mcspi.c Vignesh R       2018-10-15  597  	omap2_mcspi_set_dma_req(spi, 0, 0);
89e8b9cb846515 drivers/spi/spi-omap2-mcspi.c Vignesh R       2018-10-15  598  	if (ret || mcspi->slave_aborted)
89e8b9cb846515 drivers/spi/spi-omap2-mcspi.c Vignesh R       2018-10-15  599  		return 0;
d33f473dcd8e69 drivers/spi/spi-omap2-mcspi.c Illia Smyrnov   2013-06-17  600  
e47a682ace0cd5 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-11-06  601  	/* for TX_ONLY mode, be sure all words have shifted out */
492d819558f7cb drivers/spi/spi-omap2-mcspi.c Vaishnav Achath 2022-08-22  602  	if (tx && !rx) {
d33f473dcd8e69 drivers/spi/spi-omap2-mcspi.c Illia Smyrnov   2013-06-17  603  		chstat_reg = cs->base + OMAP2_MCSPI_CHSTAT0;
d33f473dcd8e69 drivers/spi/spi-omap2-mcspi.c Illia Smyrnov   2013-06-17  604  		if (mcspi->fifo_depth > 0) {
492d819558f7cb drivers/spi/spi-omap2-mcspi.c Vaishnav Achath 2022-08-22  605  			wait_res = mcspi_wait_for_reg_bit(chstat_reg, OMAP2_MCSPI_CHSTAT_TXFFE);
d33f473dcd8e69 drivers/spi/spi-omap2-mcspi.c Illia Smyrnov   2013-06-17  606  			if (wait_res < 0)
d33f473dcd8e69 drivers/spi/spi-omap2-mcspi.c Illia Smyrnov   2013-06-17  607  				dev_err(&spi->dev, "TXFFE timed out\n");
d33f473dcd8e69 drivers/spi/spi-omap2-mcspi.c Illia Smyrnov   2013-06-17  608  		} else {
492d819558f7cb drivers/spi/spi-omap2-mcspi.c Vaishnav Achath 2022-08-22  609  			wait_res = mcspi_wait_for_reg_bit(chstat_reg, OMAP2_MCSPI_CHSTAT_TXS);
d33f473dcd8e69 drivers/spi/spi-omap2-mcspi.c Illia Smyrnov   2013-06-17  610  			if (wait_res < 0)
e47a682ace0cd5 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-11-06  611  				dev_err(&spi->dev, "TXS timed out\n");
d33f473dcd8e69 drivers/spi/spi-omap2-mcspi.c Illia Smyrnov   2013-06-17  612  		}
492d819558f7cb drivers/spi/spi-omap2-mcspi.c Vaishnav Achath 2022-08-22  613  		if (wait_res >= 0 && (mcspi_wait_for_reg_bit(chstat_reg,
d33f473dcd8e69 drivers/spi/spi-omap2-mcspi.c Illia Smyrnov   2013-06-17  614  							     OMAP2_MCSPI_CHSTAT_EOT) < 0))
e47a682ace0cd5 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-11-06  615  			dev_err(&spi->dev, "EOT timed out\n");
e47a682ace0cd5 drivers/spi/spi-omap2-mcspi.c Shubhrajyoti D  2012-11-06  616  	}
492d819558f7cb drivers/spi/spi-omap2-mcspi.c Vaishnav Achath 2022-08-22  617  
ccdc7bf925731e drivers/spi/omap2_mcspi.c     Samuel Ortiz    2007-07-17  618  	return count;
ccdc7bf925731e drivers/spi/omap2_mcspi.c     Samuel Ortiz    2007-07-17  619  }
ccdc7bf925731e drivers/spi/omap2_mcspi.c     Samuel Ortiz    2007-07-17  620  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dmaengine: ti: k3-udma: Respond TX done if DMA_PREP_INTERRUPT is not requested
  2022-08-22 13:12   ` Péter Ujfalusi
@ 2022-08-23  6:57     ` Vaishnav Achath
  2022-09-02 14:20       ` Péter Ujfalusi
  0 siblings, 1 reply; 9+ messages in thread
From: Vaishnav Achath @ 2022-08-23  6:57 UTC (permalink / raw)
  To: Péter Ujfalusi, vkoul, broonie, dmaengine, linux-kernel, linux-spi
  Cc: vigneshr, kishon

Hi Peter,

On 22/08/22 18:42, Péter Ujfalusi wrote:
> 
> 
> On 22/08/2022 12:15, Vaishnav Achath wrote:
>> When the DMA consumer driver does not expect the callback for TX done,
>> There is no need to perform the channel RT byte counter calculations 
>> and estimate the completion but return complete on first attempt itself.
>> This assumes that the consumer who did not request DMA_PREP_INTERRUPT 
>> has its own mechanism for understanding TX completion, example: MCSPI
>> EOW interrupt can be used as TX completion signal for a SPI transaction.
> 
> The check is in place to make sure that we don't leave stale data in the
> DMA fabric.
> If you drop the check then it is going to be possible that some TX data
> is going to be lost.
> Could be one out of 10K transfers or 100K, but if that happens it is not
> going to be easy to figure out.
> Let's say we go the packet back, but PDMA is still have data to send and
> the IP stops transmitting (externally clocked bus, some delay, etc).
> Is it going to be OK to disable the channel?
> 
Thanks for the feedback, yes the check is necessary for most of the cases
but there needs to be  a way to disable the check for consumers which can
identify the end of transaction using some other internal mechanism/interrupt.

For example the MCSPI controller has an End of Word(EOW) interrupt when the
said number of bytes has been clocked out, in this case the EOW interrupt
being raised guarantees that there is no stale data in DMA fabric.Using
the EOW interrupt to identify the completion of a transaction significantly
improves the transaction speed since we need not now wait for the slower DMA
TX completion calculation.

This commit tries to bypass the check only if the consumer did not request
it by not passing the DMA_PREP_INTERRUPT flag, in other cases the check
should not be bypassed.

>>
>> Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
>> ---
>>  drivers/dma/ti/k3-udma.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
>> index 39b330ada200..03d579068453 100644
>> --- a/drivers/dma/ti/k3-udma.c
>> +++ b/drivers/dma/ti/k3-udma.c
>> @@ -263,6 +263,7 @@ struct udma_chan_config {
>>  	enum udma_tp_level channel_tpl; /* Channel Throughput Level */
>>  
>>  	u32 tr_trigger_type;
>> +	unsigned long tx_flags;
>>  
>>  	/* PKDMA mapped channel */
>>  	int mapped_channel_id;
>> @@ -1057,7 +1058,7 @@ static bool udma_is_desc_really_done(struct udma_chan *uc, struct udma_desc *d)
>>  
>>  	/* Only TX towards PDMA is affected */
>>  	if (uc->config.ep_type == PSIL_EP_NATIVE ||
>> -	    uc->config.dir != DMA_MEM_TO_DEV)
>> +	    uc->config.dir != DMA_MEM_TO_DEV || !(uc->config.tx_flags & DMA_PREP_INTERRUPT))
>>  		return true;
>>  
>>  	peer_bcnt = udma_tchanrt_read(uc, UDMA_CHAN_RT_PEER_BCNT_REG);
>> @@ -3418,6 +3419,8 @@ udma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>>  	if (!burst)
>>  		burst = 1;
>>  
>> +	uc->config.tx_flags = tx_flags;
>> +
>>  	if (uc->config.pkt_mode)
>>  		d = udma_prep_slave_sg_pkt(uc, sgl, sglen, dir, tx_flags,
>>  					   context);
> 

-- 
Regards,
Vaishnav

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dmaengine: ti: k3-udma: Respond TX done if DMA_PREP_INTERRUPT is not requested
  2022-08-23  6:57     ` Vaishnav Achath
@ 2022-09-02 14:20       ` Péter Ujfalusi
  2022-09-05  3:02         ` Vaishnav Achath
  0 siblings, 1 reply; 9+ messages in thread
From: Péter Ujfalusi @ 2022-09-02 14:20 UTC (permalink / raw)
  To: Vaishnav Achath, vkoul, broonie, dmaengine, linux-kernel, linux-spi
  Cc: vigneshr, kishon

Hi Achath,

On 23/08/2022 09:57, Vaishnav Achath wrote:
> Hi Peter,
> 
> On 22/08/22 18:42, Péter Ujfalusi wrote:
>>
>>
>> On 22/08/2022 12:15, Vaishnav Achath wrote:
>>> When the DMA consumer driver does not expect the callback for TX done,
>>> There is no need to perform the channel RT byte counter calculations
>>> and estimate the completion but return complete on first attempt itself.
>>> This assumes that the consumer who did not request DMA_PREP_INTERRUPT
>>> has its own mechanism for understanding TX completion, example: MCSPI
>>> EOW interrupt can be used as TX completion signal for a SPI transaction.
>>
>> The check is in place to make sure that we don't leave stale data in the
>> DMA fabric.
>> If you drop the check then it is going to be possible that some TX data
>> is going to be lost.
>> Could be one out of 10K transfers or 100K, but if that happens it is not
>> going to be easy to figure out.
>> Let's say we go the packet back, but PDMA is still have data to send and
>> the IP stops transmitting (externally clocked bus, some delay, etc).
>> Is it going to be OK to disable the channel?
>>
> Thanks for the feedback, yes the check is necessary for most of the cases
> but there needs to be  a way to disable the check for consumers which can
> identify the end of transaction using some other internal mechanism/interrupt.
> 
> For example the MCSPI controller has an End of Word(EOW) interrupt when the
> said number of bytes has been clocked out, in this case the EOW interrupt
> being raised guarantees that there is no stale data in DMA fabric.Using
> the EOW interrupt to identify the completion of a transaction significantly
> improves the transaction speed since we need not now wait for the slower DMA
> TX completion calculation.
> 
> This commit tries to bypass the check only if the consumer did not request
> it by not passing the DMA_PREP_INTERRUPT flag, in other cases the check
> should not be bypassed.

Let me think about over the weekend... Do you have performance numbers 
for this change?

If we make sure that this is only affecting non cyclic transfers with a 
in code comment to explain the expectations from the user I think this 
can be safe.

> 
>>>
>>> Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
>>> ---
>>>   drivers/dma/ti/k3-udma.c | 5 ++++-
>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
>>> index 39b330ada200..03d579068453 100644
>>> --- a/drivers/dma/ti/k3-udma.c
>>> +++ b/drivers/dma/ti/k3-udma.c
>>> @@ -263,6 +263,7 @@ struct udma_chan_config {
>>>   	enum udma_tp_level channel_tpl; /* Channel Throughput Level */
>>>   
>>>   	u32 tr_trigger_type;
>>> +	unsigned long tx_flags;
>>>   
>>>   	/* PKDMA mapped channel */
>>>   	int mapped_channel_id;
>>> @@ -1057,7 +1058,7 @@ static bool udma_is_desc_really_done(struct udma_chan *uc, struct udma_desc *d)
>>>   
>>>   	/* Only TX towards PDMA is affected */
>>>   	if (uc->config.ep_type == PSIL_EP_NATIVE ||
>>> -	    uc->config.dir != DMA_MEM_TO_DEV)
>>> +	    uc->config.dir != DMA_MEM_TO_DEV || !(uc->config.tx_flags & DMA_PREP_INTERRUPT))
>>>   		return true;
>>>   
>>>   	peer_bcnt = udma_tchanrt_read(uc, UDMA_CHAN_RT_PEER_BCNT_REG);
>>> @@ -3418,6 +3419,8 @@ udma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>>>   	if (!burst)
>>>   		burst = 1;
>>>   
>>> +	uc->config.tx_flags = tx_flags;
>>> +
>>>   	if (uc->config.pkt_mode)
>>>   		d = udma_prep_slave_sg_pkt(uc, sgl, sglen, dir, tx_flags,
>>>   					   context);
>>
> 

-- 
Péter

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dmaengine: ti: k3-udma: Respond TX done if DMA_PREP_INTERRUPT is not requested
  2022-09-02 14:20       ` Péter Ujfalusi
@ 2022-09-05  3:02         ` Vaishnav Achath
  2022-09-10 18:57           ` Péter Ujfalusi
  0 siblings, 1 reply; 9+ messages in thread
From: Vaishnav Achath @ 2022-09-05  3:02 UTC (permalink / raw)
  To: Péter Ujfalusi, vkoul, broonie, dmaengine, linux-kernel, linux-spi
  Cc: vigneshr, kishon, Vaishnav Achath

Hi Peter,

On 02/09/22 19:50, Péter Ujfalusi wrote:
> Hi Achath,
> 
> On 23/08/2022 09:57, Vaishnav Achath wrote:
>> Hi Peter,
>>
>> On 22/08/22 18:42, Péter Ujfalusi wrote:
>>>
>>>
>>> On 22/08/2022 12:15, Vaishnav Achath wrote:
>>>> When the DMA consumer driver does not expect the callback for TX done,
>>>> There is no need to perform the channel RT byte counter calculations
>>>> and estimate the completion but return complete on first attempt itself.
>>>> This assumes that the consumer who did not request DMA_PREP_INTERRUPT
>>>> has its own mechanism for understanding TX completion, example: MCSPI
>>>> EOW interrupt can be used as TX completion signal for a SPI transaction.
>>>
>>> The check is in place to make sure that we don't leave stale data in the
>>> DMA fabric.
>>> If you drop the check then it is going to be possible that some TX data
>>> is going to be lost.
>>> Could be one out of 10K transfers or 100K, but if that happens it is not
>>> going to be easy to figure out.
>>> Let's say we go the packet back, but PDMA is still have data to send and
>>> the IP stops transmitting (externally clocked bus, some delay, etc).
>>> Is it going to be OK to disable the channel?
>>>
>> Thanks for the feedback, yes the check is necessary for most of the cases
>> but there needs to be  a way to disable the check for consumers which can
>> identify the end of transaction using some other internal mechanism/interrupt.
>>
>> For example the MCSPI controller has an End of Word(EOW) interrupt when the
>> said number of bytes has been clocked out, in this case the EOW interrupt
>> being raised guarantees that there is no stale data in DMA fabric.Using
>> the EOW interrupt to identify the completion of a transaction significantly
>> improves the transaction speed since we need not now wait for the slower DMA
>> TX completion calculation.
>>
>> This commit tries to bypass the check only if the consumer did not request
>> it by not passing the DMA_PREP_INTERRUPT flag, in other cases the check
>> should not be bypassed.
> 
> Let me think about over the weekend... Do you have performance numbers for this
> change?
> 
Thank you, yes we tested mainly for the SPI cases(Master and Slave mode), there
we saw a peak delay of 400ms for transaction completion and this varied with CPU
load, after adding the patch to not wait for DMA TX completion and use EOW
interrupt the peak latency reduced to 2ms.
> If we make sure that this is only affecting non cyclic transfers with a in code
> comment to explain the expectations from the user I think this can be safe.
> \
Sure I will add this in the next revision.
>>
>>>>
>>>> Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
>>>> ---
>>>>   drivers/dma/ti/k3-udma.c | 5 ++++-
>>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
>>>> index 39b330ada200..03d579068453 100644
>>>> --- a/drivers/dma/ti/k3-udma.c
>>>> +++ b/drivers/dma/ti/k3-udma.c
>>>> @@ -263,6 +263,7 @@ struct udma_chan_config {
>>>>       enum udma_tp_level channel_tpl; /* Channel Throughput Level */
>>>>         u32 tr_trigger_type;
>>>> +    unsigned long tx_flags;
>>>>         /* PKDMA mapped channel */
>>>>       int mapped_channel_id;
>>>> @@ -1057,7 +1058,7 @@ static bool udma_is_desc_really_done(struct udma_chan
>>>> *uc, struct udma_desc *d)
>>>>         /* Only TX towards PDMA is affected */
>>>>       if (uc->config.ep_type == PSIL_EP_NATIVE ||
>>>> -        uc->config.dir != DMA_MEM_TO_DEV)
>>>> +        uc->config.dir != DMA_MEM_TO_DEV || !(uc->config.tx_flags &
>>>> DMA_PREP_INTERRUPT))
>>>>           return true;
>>>>         peer_bcnt = udma_tchanrt_read(uc, UDMA_CHAN_RT_PEER_BCNT_REG);
>>>> @@ -3418,6 +3419,8 @@ udma_prep_slave_sg(struct dma_chan *chan, struct
>>>> scatterlist *sgl,
>>>>       if (!burst)
>>>>           burst = 1;
>>>>   +    uc->config.tx_flags = tx_flags;
>>>> +
>>>>       if (uc->config.pkt_mode)
>>>>           d = udma_prep_slave_sg_pkt(uc, sgl, sglen, dir, tx_flags,
>>>>                          context);
>>>
>>
> 

-- 
Regards,
Vaishnav

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dmaengine: ti: k3-udma: Respond TX done if DMA_PREP_INTERRUPT is not requested
  2022-09-05  3:02         ` Vaishnav Achath
@ 2022-09-10 18:57           ` Péter Ujfalusi
  0 siblings, 0 replies; 9+ messages in thread
From: Péter Ujfalusi @ 2022-09-10 18:57 UTC (permalink / raw)
  To: Vaishnav Achath, vkoul, broonie, dmaengine, linux-kernel, linux-spi
  Cc: vigneshr, kishon



On 05/09/2022 06:02, Vaishnav Achath wrote:
>> Let me think about over the weekend... Do you have performance numbers for this
>> change?
>>
> Thank you, yes we tested mainly for the SPI cases(Master and Slave mode), there
> we saw a peak delay of 400ms for transaction completion and this varied with CPU
> load, after adding the patch to not wait for DMA TX completion and use EOW
> interrupt the peak latency reduced to 2ms.

Thank you for the details.

>> If we make sure that this is only affecting non cyclic transfers with a in code
>> comment to explain the expectations from the user I think this can be safe.
>> \
> Sure I will add this in the next revision.

You can add my Acked-by when you send the next version:
Acked-by: Peter Ujfalusi <peter.ujfalusi@gmail.com>

>>>
>>>>>
>>>>> Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
>>>>> ---
>>>>>    drivers/dma/ti/k3-udma.c | 5 ++++-
>>>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
>>>>> index 39b330ada200..03d579068453 100644
>>>>> --- a/drivers/dma/ti/k3-udma.c
>>>>> +++ b/drivers/dma/ti/k3-udma.c
>>>>> @@ -263,6 +263,7 @@ struct udma_chan_config {
>>>>>        enum udma_tp_level channel_tpl; /* Channel Throughput Level */
>>>>>          u32 tr_trigger_type;
>>>>> +    unsigned long tx_flags;
>>>>>          /* PKDMA mapped channel */
>>>>>        int mapped_channel_id;
>>>>> @@ -1057,7 +1058,7 @@ static bool udma_is_desc_really_done(struct udma_chan
>>>>> *uc, struct udma_desc *d)
>>>>>          /* Only TX towards PDMA is affected */
>>>>>        if (uc->config.ep_type == PSIL_EP_NATIVE ||
>>>>> -        uc->config.dir != DMA_MEM_TO_DEV)
>>>>> +        uc->config.dir != DMA_MEM_TO_DEV || !(uc->config.tx_flags &
>>>>> DMA_PREP_INTERRUPT))
>>>>>            return true;
>>>>>          peer_bcnt = udma_tchanrt_read(uc, UDMA_CHAN_RT_PEER_BCNT_REG);
>>>>> @@ -3418,6 +3419,8 @@ udma_prep_slave_sg(struct dma_chan *chan, struct
>>>>> scatterlist *sgl,
>>>>>        if (!burst)
>>>>>            burst = 1;
>>>>>    +    uc->config.tx_flags = tx_flags;
>>>>> +
>>>>>        if (uc->config.pkt_mode)
>>>>>            d = udma_prep_slave_sg_pkt(uc, sgl, sglen, dir, tx_flags,
>>>>>                           context);
>>>>
>>>
>>
> 

-- 
Péter

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-09-10 18:57 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-22  9:15 [PATCH 0/2] spi: spi-omap2-mcspi: Use EOW interrupt for transfer completion Vaishnav Achath
2022-08-22  9:15 ` [PATCH 1/2] dmaengine: ti: k3-udma: Respond TX done if DMA_PREP_INTERRUPT is not requested Vaishnav Achath
2022-08-22 13:12   ` Péter Ujfalusi
2022-08-23  6:57     ` Vaishnav Achath
2022-09-02 14:20       ` Péter Ujfalusi
2022-09-05  3:02         ` Vaishnav Achath
2022-09-10 18:57           ` Péter Ujfalusi
2022-08-22  9:15 ` [PATCH 2/2] spi: spi-omap2-mcspi: Use EOW interrupt for completion when DMA enabled Vaishnav Achath
2022-08-22 13:14   ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).