Re: [PATCH v2 2/2] dmaengine: dw-edma: Do not enable watermark interrupts for HDMA

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

 




On 8/2/2024 6:04 AM, Serge Semin wrote:
On Tue, Jul 23, 2024 at 06:49:32PM +0530, Mrinmay Sarkar wrote:
DW_HDMA_V0_LIE and DW_HDMA_V0_RIE are initialized as BIT(3) and BIT(4)
respectively in dw_hdma_control enum. But as per HDMA register these
bits are corresponds to LWIE and RWIE bit i.e local watermark interrupt
enable and remote watermarek interrupt enable. In linked list mode LWIE
and RWIE bits only enable the local and remote watermark interrupt.

Since the watermark interrupts are not used but enabled, this leads to
spurious interrupts getting generated. So remove the code that enables
them to avoid generating spurious watermark interrupts.

And also rename DW_HDMA_V0_LIE to DW_HDMA_V0_LWIE and DW_HDMA_V0_RIE to
DW_HDMA_V0_RWIE as there is no LIE and RIE bits in HDMA and those bits
are corresponds to LWIE and RWIE bits.

Fixes: e74c39573d35 ("dmaengine: dw-edma: Add support for native HDMA")
cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Mrinmay Sarkar <quic_msarkar@xxxxxxxxxxx>
---
  drivers/dma/dw-edma/dw-hdma-v0-core.c | 17 +++--------------
  1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index fa89b3a..9ad2e28 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -17,8 +17,8 @@ enum dw_hdma_control {
  	DW_HDMA_V0_CB					= BIT(0),
  	DW_HDMA_V0_TCB					= BIT(1),
  	DW_HDMA_V0_LLP					= BIT(2),
-	DW_HDMA_V0_LIE					= BIT(3),
-	DW_HDMA_V0_RIE					= BIT(4),
+	DW_HDMA_V0_LWIE					= BIT(3),
+	DW_HDMA_V0_RWIE					= BIT(4),
  	DW_HDMA_V0_CCS					= BIT(8),
  	DW_HDMA_V0_LLE					= BIT(9),
  };
@@ -195,25 +195,14 @@ static void dw_hdma_v0_write_ll_link(struct dw_edma_chunk *chunk,
  static void dw_hdma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
  {
  	struct dw_edma_burst *child;
-	struct dw_edma_chan *chan = chunk->chan;
  	u32 control = 0, i = 0;
-	int j;
if (chunk->cb)
  		control = DW_HDMA_V0_CB;
- j = chunk->bursts_alloc;
-	list_for_each_entry(child, &chunk->burst->list, list) {
-		j--;
-		if (!j) {
-			control |= DW_HDMA_V0_LIE;
-			if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL))
-				control |= DW_HDMA_V0_RIE;
-		}
-
+	list_for_each_entry(child, &chunk->burst->list, list)
  		dw_hdma_v0_write_ll_data(chunk, i++, control, child->sz,
  					 child->sar, child->dar);
-	}
Hm, in case of DW EDMA the LIE/RIE flags of the LL entries gets to be
moved to the LIE/RIE flags of the channel context register by the
DMA-engine. In its turn the context register LIE/RIE flags determine
whether the Local and Remote Done/Abort IRQs being raised. So without
the LIE/RIE flags being set in the LL-entries the IRQs won't be raised
and the whole procedure won't work. I have doubts it works differently
in case of HDMA because changing the semantics would cause
implementing additional logic in the DW HDMA RTL-model. Seeing the DW
HDMA IP-core supports the eDMA compatibility mode it would needlessly
expand the controller size. What are the rest of the CONTROL1 register
fields? There must be LIE/RIE flags someplace there for the non-LL
transfers and to preserve the values retrieved from the LL-entries.

Moreover the DW eDMA HW manual has a dedicated chapter called
"Interrupts and Error Handling" with a very demonstrative figures
describing the way the flags work. Does the DW HDMA databook have
something like that?

Please also note, the DW _EDMA_ LIE and RIE flags can be also utilized
for the intermediate IRQ raising, to implement the runtime LL-entries
recycling pattern. The IRQ in that case is called as "watermark" IRQ
in the DW EDMA HW databook, but the flags are still called as just
LIE/RIE.

-Serge(y)
Yes, you are right LIE/RIE flags need to be set without that the IRQs
won't be raised in case of DW EDMA.
But in DW HDMA case there in no such LIE/RIE flags and these particular
bits has been mapped to LWIE and RWIE flags and these are used to enable
watermark interrupt only.
There is no LIE/RIE fields in HDMA_CONTROL1_OFF_WRCH register fields
the same is present in EDMA CONTROL1 register.

DW HDMA has INT_SETUP register and it has LSIE/RSIE, LAIE/RAIE fields
those are enabling Local and Remote Stop/Abort IRQs in LL mode.

yes DW HDMA data book also have figures in "Interrupts and Error Handling"
section and there is no LIE/RIE flags and it is replaced with LWIE/RWIE flags
as I mentioned above.

Thanks,
Mrinmay

control = DW_HDMA_V0_LLP | DW_HDMA_V0_TCB;
  	if (!chunk->cb)
--
2.7.4





[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux