* [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).