AM625: Rerouting interrupt to DMA event and DMA_DEV_TO_MEM channel selection

Hi Hamish,

Sorry I won't be able to provide much information, since the kernel doesn't have a reference of using GPIO interrupt triggering DMA transfers. But looking at the AM62x DMA driver drivers/dma/ti/k3-udma.c, it does support DMA_DEV_TO_MEM.

Hi Hamish,

Hamish Guthrie said:

1. How do I configure the IRQ routing so that the GPIO1_15 line triggers a suitable DMA event to start the DMA transfer from this FPGA FIFO into the DMA buffer.

Actually, this feature is supported in AM62X, but we don't have this example in MCU+SDK.

So, I have created this example on the R5F core in AM62X, and I hope  it is similar to the Linux core as well.

Can you please tell me if you want to implement this requirement on R5F or A53?

 You have to follow the steps below:

1. Route GPIO Events to the L2G .

2. Next, convert the local GPIO event into a global event.

Software Changes : 

1. Configure GPIO as an input based on your requirements.

 In my example, I have configured GPIO1_23, which is available on EVM.

2. Route the Main GPIO Interrupt Router outputs from 22 to 31 to L2G. Since these routers only connect to L2G.

Please see the below image.

Unfortunately, in the software, these routers are not routed to L2G.

So, I have manually updated only the main GPIO router outputs from 22 to 31 in the sciclient_defaultBoardcfg_rm.c file.

Folder Path: 

C:\ti\mcu_plus_sdk_am62x_08_06_00_18\source\drivers\sciclient\sciclient_default_boardcfg\am62x

3. Compile the sciclient_defaultBoardcfg_rm.c file.

You have to give the below command to compile it.

gmake SOC=am62x

4. To convert GPIO events to Global events, we need to configure the register below. 

In the application, please check the below function for more details about converting a GPIO Event to a global event.

static void configure_intaggrL2G (uint32_t localEvent, uint32_t globalEvent)
{
uint64_t eventRegOffset = CSL_DMASS0_INTAGGR_L2G_BASE + (localEvent * 0x20U);
CSL_REG32_WR(eventRegOffset, (0U << 31U) | (globalEvent & 0xFFFFU) ); /* pulse event */
}

5. Compile SBL and Flash SBL with the updated board configuration file.

6. Run your Application from CCS.

Please let me know if you need any other help.

The below link can be helpful to configure GPIO Routers and other Routers .

https://software-dl.ti.com/tisci/esd/latest/5_soc_doc/am62x/interrupt_cfg.html

Regards,

S.Anil.

Hamish Guthrie said:

1. How do I configure the IRQ routing so that the GPIO1_15 line triggers a suitable DMA event to start the DMA transfer from this FPGA FIFO into the DMA buffer.

Hello Hamish,

For your requirement in the Software , you just need to update the below parameter in the function.

rmIrqReq.src_index = 180U; //GPIO1_15 and Bank 0

You can go through the below link to configure Router output's , which can be helpful.

https://software-dl.ti.com/tisci/esd/latest/5_soc_doc/am62x/interrupt_cfg.html

Regards,

S.Anil.

Hello Anil,

Thank you for your reply and for the examples.

We need to implement this requirement on the A53 cores under Linux, the problem is that I believe the infrastructure is not in place in the kernel yet to facilitate the re-routing, or alternatively, it may be in place, but even after studying the device tree bindings and the source code, I have not been able to figure out how to do it under Linux, so it is a very Linux-specific requirement, and if you could me with that it would be great. I do not find any documentation which relates to this anywhere.

Regards

Hamish

Hello Hamish,

Regards,

S.Anil.

Hi Hamish,

After discussed the details with our software dev team, it should be possible to implement gpio event routing to directly trigger DMA transfers. I am collecting information then will start the implementation. I will keep you posted on the progress.

Regarding the DMA device tree binding for DMA for accessing GPMC, here is the info in the kernel binding doc  <Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml>:

  "#dma-cells":                                                                 
    const: 3                                                                    
    description: |                                                              
      cell 1: type of the BCDMA channel to be used to service the peripheral:   
        0 - split channel                                                       
        1 - block copy channel using global trigger 1                           
        2 - block copy channel using global trigger 2                           
        3 - block copy channel using local trigger                              
                                                                                
      cell 2: parameter for the channel:                                        
        if cell 1 is 0 (split channel):                                         
          PSI-L thread ID of the remote (to BCDMA) end.                         
          Valid ranges for thread ID depends on the data movement direction:       
          for source thread IDs (rx): 0 - 0x7fff                                
          for destination thread IDs (tx): 0x8000 - 0xffff                      
                                                                                
          Please refer to the device documentation for the PSI-L thread map and 
          also the PSI-L peripheral chapter for the correct thread ID.          
        if cell 1 is 1 or 2 (block copy channel using global trigger):          
          Unused, ignored                                                       
                                                                                
          The trigger must be configured for the channel externally to BCDMA,   
          channels using global triggers should not be requested directly, but  
          via DMA event router.                                                 
        if cell 1 is 3 (block copy channel using local trigger):                
          bchan number of the locally triggered channel                         
                                                                                
      cell 3: ASEL value for the channel

though the information is difficult to understand, here is basically what needed in device tree:

dmas = <&main_bcdma 1 0 0>;

The second field would be '1' or '2', to use either "global trigger 1" or "global trigger 2". I will have details on this later.

Here I have a couple questions in your implementation on AM335x:

- The FPGA triggers the GPIO in pulse, not level, right? I don't see how AM335x (or AM62x) can clear the GPIO after DMA read if it was level triggered.

- You have a custom FPGA driver with dmaengine slave API to access GPMC interface, right? Did you have any EDMA driver modification? I'd like to understand if any GPMC driver modification is needed on AM62x.

Hi Bin,

In our existing implementation with the AM335x we are using level triggering, but what happens is that the FPGA raises the XDMA_EVENT0 line once data is ready in the FIFO, and that event launches the DMA transfer, and once the transfer has completed, the dma engine executes a callback which is in our driver, within that callback we send an interrupt acknowledge to the FPGA (we write a value to a specific register in the FPGA) and when the FPGA received this acknowledge, the FPGA deasserts the XDMA_EVENT0 line.

Yes, we have a custom FPGA driver and we are using the dmaengine slave API. We have no EDMA driver modification at all.

Here is the code we configure the dmaengine with:

static int setup_transfer(struct dma_dev *ddev)
{
        struct device *dev = &ddev->pdev->dev;
        struct fpga_dev *fdev = dev_get_drvdata(dev->parent);
        struct dma_slave_config conf;
        struct dma_async_tx_descriptor *tx;
        unsigned long flags;
        int ret;

        memset(&conf, 0, sizeof(conf));
        conf.direction = DMA_DEV_TO_MEM;
        conf.src_addr_width = ddev->bank_width;
        conf.src_maxburst = ddev->frame_size / 2;
        conf.src_addr = (dma_addr_t) (fdev->phys_base + FPGA_ADDR_DAQ_FIFO);
        ret = dmaengine_slave_config(ddev->dma, &conf);
        if (ret < 0)
                return ret;

        flags = DMA_PREP_INTERRUPT;
        tx = dmaengine_prep_dma_cyclic(ddev->dma, ddev->dma_phys,
                        ddev->frame_count * ddev->frame_size,
                        ddev->frame_size / 2,
                        DMA_DEV_TO_MEM, flags);

        if (!tx) {
                dev_err(dev, "dma preparation failed\n");
                return -ENODEV;
        }

        tx->callback = dma_tc_callback;
        tx->callback_param = ddev;
        ddev->cookie = dmaengine_submit(tx);
        dma_async_issue_pending(ddev->dma);

        return 0;
}

The callback referred to in the code above first verifies that we indeed received a valid frame (we have a verifyable header in the frame), then it increments the write pointer (and DMA buffer wrap), then acknowledges the interrupt to the FPGA, then notifies user-space that data is ready.

I doubt that any modification would be required to the AM62x drivers.

Hi Hamish,

Thanks for the details. I will create a test driver in this same way to validate the GPIO event routing.

Hi Bin,

Thank you very much for your very attentive assistance. I would just like to re-iterate that the GPIO event routing is probably lower priority than getting the DMA engine to work with DEV_TO_MEM transfers.

Kind regards
Hamish

Hi Hamish,

Sounds good, we can focus on CPU managing DMA transfers first.

I don't have a test driver to validate yet, but can you please try the following hack to see if it makes DMA DEV_TO_MEM working?

For this hack, the DTS setting for dma channels would be:

dmas = <&main_bcdma 1 0 0>;

diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
index 0d117c1c49fe..64c39ec30edd 100644
--- a/drivers/dma/ti/k3-udma.c
+++ b/drivers/dma/ti/k3-udma.c
@@ -3069,7 +3069,8 @@ udma_prep_slave_sg_triggered_tr(struct udma_chan *uc, struct scatterlist *sgl,
                cppi5_tr_init(&tr_req[tr_idx].flags, CPPI5_TR_TYPE15, false,
                              true, CPPI5_TR_EVENT_SIZE_COMPLETION, 0);
                cppi5_tr_csf_set(&tr_req[tr_idx].flags, CPPI5_TR_CSF_SUPR_EVT);
-               cppi5_tr_set_trigger(&tr_req[tr_idx].flags,
+               if (uc->config.tr_trigger_type != 1)
+                       cppi5_tr_set_trigger(&tr_req[tr_idx].flags,
                                     uc->config.tr_trigger_type,
                                     CPPI5_TR_TRIGGER_TYPE_ICNT2_DEC, 0, 0);
 
@@ -3116,7 +3117,8 @@ udma_prep_slave_sg_triggered_tr(struct udma_chan *uc, struct scatterlist *sgl,
                                      CPPI5_TR_EVENT_SIZE_COMPLETION, 0);
                        cppi5_tr_csf_set(&tr_req[tr_idx].flags,
                                         CPPI5_TR_CSF_SUPR_EVT);
-                       cppi5_tr_set_trigger(&tr_req[tr_idx].flags,
+                       if (uc->config.tr_trigger_type != 1)
+                               cppi5_tr_set_trigger(&tr_req[tr_idx].flags,
                                             uc->config.tr_trigger_type,
                                             CPPI5_TR_TRIGGER_TYPE_ICNT2_DEC,
                                             0, 0);

Hi Bin,

Unfortunately today was one of those days with permanent interruptions, but I did manage to get a DMA channel allocated, however, I am just a little concerned about some debug messages:

In my DT I have 

dmas = <&main_bcdma 1 0 0>;
dma-names = "rx";

And I applied your above patch. I also enabled debug in the k3-udma driver and I see the following when I load my driver:

[ 8.864572] sbfpgadma sbfpgadma: request dma channel
[ 8.864594] ti-udma 485c0100.dma-controller: chan1: triggered channel (type: 1)
[ 8.864604] ti-udma 485c0100.dma-controller: bcdma_alloc_chan_resources: chan1 as MEM-to-MEM
[ 8.865248] sbfpgadma sbfpgadma: channel: 1, name: dma:rx

The thing that concerns me is the fact that the udma driver reports that it has allocated chan1 as MEM-to-MEM.

Unfortunately with all of the interruptions today I was unable to try to trigger any DMA transfers from the GPIO interrupt context, but I will try it in the morning and report back.

Hi Hamish,

> [ 8.864604] ti-udma 485c0100.dma-controller: bcdma_alloc_chan_resources: chan1 as MEM-to-MEM

By reading k3-udma.c, this direction "MEM-to-MEM" is specified in another struct field, different from

> conf.direction = DMA_DEV_TO_MEM;

But I didn't have enough time today to figure out how to set this dir to DEV-to-MEM, or know if it matters. Please let me know how you test goes.

Hi Bin,

I managed to try to launch DMA transfers but as soon as I issue the dmaengine_prep_dma_cyclic command, I receive the following error message:

[ 121.941578] ti-udma 485c0100.dma-controller: udma_prep_dma_cyclic: chan1 is for MEM_TO_MEM, not supporting DEV_TO_MEM
[ 121.952200] sbfpgadma sbfpgadma: dma preparation failed

So it seems as though the channel is the wrong type for a DEV_TO_MEM transfer.

Hi Hamish,

According to my understanding of the udma driver, the channel needs direction configuration in two different struct fields. I will have to figure out how to set both to DEV_TO_MEM. I first need to create a dummy kernel driver to simulate DMA DEV_TO_MEM transfer to validate this. It is also needed for the GPIO event routing implementation.

I am fully loaded in these few days, I will start work on this very soon. I will keep you posted.

Hi Hamish,

I just wanted to give an update before my 2-weeks vacation.

I am first writing a kernel driver which uses DMA Engine slave API to simulate DEV_TO_MEM copy on AM335x (EDMA), which you have proved working on GPMC/FPGA. This driver will be used to test DMA DEV_TO_MEM copy on AM62x (UDMA), and further to test GPIO event triggering.

Along with my other daily work, the development has been slow but making progress. I will keep you post once I am back from vacation and reach to the next milestone.

Hi Bin,

Would it help if I sent you the code for my driver?

The other stupid thing is I clicked by mistake on the unsubscribe link for the thread and I do not see how to re-subscribe - is there any way that I can do this?

Hey Hamish,

The view is a bit different for TI employees than you, but check the right side of your screen above the "Similar topics" section (make sure you are logged into your e2e account before doing that). On my view, there is a "turn reply notifications off/on" option that you can toggle if you aren't getting email notifications:

Regards,

Nick

Thanks Nick for the hint.

Hi Hamish,

Thanks for the offer, I think I don't need it any this time. Your code snippet above already helped. My driver is more than half done now, next is to implement the DMA completion callback which will validate the DST buffer to ensure the DMA copy is done right. Then I can start debugging the entire use case.

Hi Hamish,

My apologies for the delay, I got pulled into a critical issue, and have been tied up to its debugging for the past few weeks. I will resume the driver implementation as soon as I have some bandwidth.

Hi Hamish,

Any reason your current driver uses dmaengine_prep_dma_cyclic() not dmaengine_prep_slave_single()?

Checking the udma driver, it seems udma_prep_dma_cyclic() only supports MEM_TO_MEM, but udma_prep_slave_sg() supports DEV_TO_MEM.

Hello Bin, yes there is a very good reason to use cyclic, and that is because we only have to set up the DMA descriptors once and the DMA engine handles wrapping of the destination DMA buffer, so using cyclic it is essentially a case of set it up once and then fire and forget, however (and please correct me if I am wrong), with udma_prep_slave_sg I understand that we would have to set up the DMA descriptors for every transfer, and we would also have manage the destination address of the transfer and wrapping prior to every DMA transfer. I guess we could do this, but using cyclic is so much simpler, and there is no overhead every DMA cycle.

Hi Hamish,

Thanks for the details. It makes sense.

I am checking with the sw dev team if the UDMA driver supports DEV_TO_MEM in cyclic. Meanwhile, I am trying to see if I can use get a DEV_TO_MEM channel with slave_simple. It does get a channel, but further resource allocation failed in the UDMA driver.

I will keep you posted.

Hi Hamish,

After struggling to get DEV_TO_MEM to read small simulated FIFO, I checked your example code again:

        tx = dmaengine_prep_dma_cyclic(ddev->dma, ddev->dma_phys,
                        ddev->frame_count * ddev->frame_size,
                        ddev->frame_size / 2,
                        DMA_DEV_TO_MEM, flags);

so your FPGA fifo size is ddev->frame_size/2 (as you have explained ping-pong buffers are used), it is entirely mmapped, right? Somehow I thought the fifo depth is frame_size / 2, but the fifo entry is just a few bytes, and the DMA has to repeatedly read the fifo few bytes at a time until the half frame_size is done. Now checking this function call again, the DMA reads the half frame_size, then repeatedly reading up to frame_count * frame_size driven by the gpio events, right?

Hi Hamish,

Never mind on my question above.

The DMA should be able to read conf.src_addr_width multiple times until the entire fifo.

Hi Bin,

This is correct, so in a typical transfer of ours, src_addr_width is set to 2, so the DMA engine will read 16-bits 4102 times to copy a single frame of data across to the DMA buffer.

Hi Hamish,

Just wanted to update the progress. With the kernel patch below, I am able to do DEV_TO_MEM transfer in my DMA test driver. 

diff --git a/arch/arm64/boot/dts/ti/k3-am625-sk.dts b/arch/arm64/boot/dts/ti/k3-am625-sk.dts
index b1737978103b..5a7b614b7b4c 100644
--- a/arch/arm64/boot/dts/ti/k3-am625-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am625-sk.dts
@@ -13,6 +13,12 @@ / {
 	compatible = "ti,am625-sk", "ti,am625";
 	model = "Texas Instruments AM625 SK";
 
+	dma_test {
+		compatible = "ti,dma_test";
+		dmas = <&main_bcdma 1 0 0>;
+		dma-names = "rx";
+	};
+
 	opp-table {
 		/* Add 1.4GHz OPP for am625-sk board. Requires VDD_CORE to be at 0.85V */
 		opp-1400000000 {
diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
index ff3dc91f826c..6f8af321bb3d 100644
--- a/drivers/dma/ti/k3-udma.c
+++ b/drivers/dma/ti/k3-udma.c
@@ -949,12 +949,17 @@ static int udma_start(struct udma_chan *uc)
 			       sizeof(uc->static_tr));
 		}
 
-		udma_rchanrt_write(uc, UDMA_CHAN_RT_CTL_REG,
+		if (!uc->config.tr_trigger_type)
+			udma_rchanrt_write(uc, UDMA_CHAN_RT_CTL_REG,
 				   UDMA_CHAN_RT_CTL_EN);
 
-		/* Enable remote */
-		udma_rchanrt_write(uc, UDMA_CHAN_RT_PEER_RT_EN_REG,
+		if (!uc->config.tr_trigger_type)
+			/* Enable remote */
+			udma_rchanrt_write(uc, UDMA_CHAN_RT_PEER_RT_EN_REG,
 				   UDMA_PEER_RT_EN_ENABLE);
+		else
+			udma_tchanrt_write(uc, UDMA_CHAN_RT_CTL_REG,
+				   UDMA_CHAN_RT_CTL_EN);
 
 		break;
 	case DMA_MEM_TO_DEV:
@@ -2827,7 +2832,7 @@ static struct udma_desc *udma_alloc_tr_desc(struct udma_chan *uc,
 	if (uc->cyclic)
 		reload_count = CPPI5_INFO0_TRDESC_RLDCNT_INFINITE;
 
-	if (dir == DMA_DEV_TO_MEM)
+	if (dir == DMA_DEV_TO_MEM && !uc->config.tr_trigger_type)
 		ring_id = k3_ringacc_get_ring_id(uc->rflow->r_ring);
 	else
 		ring_id = k3_ringacc_get_ring_id(uc->tchan->tc_ring);
@@ -3074,7 +3079,9 @@ udma_prep_slave_sg_triggered_tr(struct udma_chan *uc, struct scatterlist *sgl,
 		cppi5_tr_init(&tr_req[tr_idx].flags, CPPI5_TR_TYPE15, false,
 			      true, CPPI5_TR_EVENT_SIZE_COMPLETION, 0);
 		cppi5_tr_csf_set(&tr_req[tr_idx].flags, csf);
-		cppi5_tr_set_trigger(&tr_req[tr_idx].flags,
+
+		if (!uc->config.tr_trigger_type)
+			cppi5_tr_set_trigger(&tr_req[tr_idx].flags,
 				     uc->config.tr_trigger_type,
 				     CPPI5_TR_TRIGGER_TYPE_ICNT2_DEC, 0, 0);
 
@@ -3417,6 +3424,7 @@ udma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 	struct udma_desc *d;
 	u32 burst;
 
+#if 0
 	if (dir != uc->config.dir &&
 	    (uc->config.dir == DMA_MEM_TO_MEM && !uc->config.tr_trigger_type)) {
 		dev_err(chan->device->dev,
@@ -3426,6 +3434,7 @@ udma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 			dmaengine_get_direction_text(dir));
 		return NULL;
 	}
+#endif
 
 	if (dir == DMA_DEV_TO_MEM) {
 		dev_width = uc->cfg.src_addr_width;
@@ -3446,7 +3455,7 @@ udma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 	if (uc->config.pkt_mode)
 		d = udma_prep_slave_sg_pkt(uc, sgl, sglen, dir, tx_flags,
 					   context);
-	else if (is_slave_direction(uc->config.dir))
+	else if (is_slave_direction(uc->config.dir) && !uc->config.tr_trigger_type)
 		d = udma_prep_slave_sg_tr(uc, sgl, sglen, dir, tx_flags,
 					  context);
 	else

The DMA transfer starts automatically right after dma_async_issue_pending() is called. I am now trying to figure out how to do software trigger instead (then finally move to gpio hw trigger).

My DMA test driver calls dmaengine_prep_slave_single(), but I think cyclic should be supported too with some driver change.

tx = dmaengine_prep_slave_single(dma_ch, dma_buf_addr,
                fifo_size, DMA_DEV_TO_MEM,
                DMA_PREP_INTERRUPT | DMA_CTRL_ACK);

Hi Hamish,

With the kernel patch below, I am able to use software to trigger DEV_TO_MEM transfers.

diff --git a/arch/arm64/boot/dts/ti/k3-am625-sk.dts b/arch/arm64/boot/dts/ti/k3-am625-sk.dts
index b1737978103b..5a7b614b7b4c 100644
--- a/arch/arm64/boot/dts/ti/k3-am625-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am625-sk.dts
@@ -13,6 +13,12 @@ / {
 	compatible = "ti,am625-sk", "ti,am625";
 	model = "Texas Instruments AM625 SK";
 
+	dma_test {
+		compatible = "ti,dma_test";
+		dmas = <&main_bcdma 1 0 0>;
+		dma-names = "rx";
+	};
+
 	opp-table {
 		/* Add 1.4GHz OPP for am625-sk board. Requires VDD_CORE to be at 0.85V */
 		opp-1400000000 {
diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
index ff3dc91f826c..f06899367ebd 100644
--- a/drivers/dma/ti/k3-udma.c
+++ b/drivers/dma/ti/k3-udma.c
@@ -952,9 +952,14 @@ static int udma_start(struct udma_chan *uc)
 		udma_rchanrt_write(uc, UDMA_CHAN_RT_CTL_REG,
 				   UDMA_CHAN_RT_CTL_EN);
 
-		/* Enable remote */
-		udma_rchanrt_write(uc, UDMA_CHAN_RT_PEER_RT_EN_REG,
+		if (!uc->config.tr_trigger_type)
+			/* Enable remote */
+			udma_rchanrt_write(uc, UDMA_CHAN_RT_PEER_RT_EN_REG,
 				   UDMA_PEER_RT_EN_ENABLE);
+		else
+			udma_tchanrt_write(uc, UDMA_CHAN_RT_CTL_REG,
+				   UDMA_CHAN_RT_CTL_EN);
+
 
 		break;
 	case DMA_MEM_TO_DEV:
@@ -2827,7 +2832,7 @@ static struct udma_desc *udma_alloc_tr_desc(struct udma_chan *uc,
 	if (uc->cyclic)
 		reload_count = CPPI5_INFO0_TRDESC_RLDCNT_INFINITE;
 
-	if (dir == DMA_DEV_TO_MEM)
+	if (dir == DMA_DEV_TO_MEM && !uc->config.tr_trigger_type)
 		ring_id = k3_ringacc_get_ring_id(uc->rflow->r_ring);
 	else
 		ring_id = k3_ringacc_get_ring_id(uc->tchan->tc_ring);
@@ -2987,6 +2992,7 @@ udma_prep_slave_sg_triggered_tr(struct udma_chan *uc, struct scatterlist *sgl,
 	int num_tr = 0;
 	int tr_idx = 0;
 	u32 burst, trigger_size, port_window;
+	int dim1;
 	u64 asel;
 
 	if (dir == DMA_DEV_TO_MEM) {
@@ -3021,6 +3027,7 @@ udma_prep_slave_sg_triggered_tr(struct udma_chan *uc, struct scatterlist *sgl,
 		tr_cnt1 = burst;
 	}
 	trigger_size = tr_cnt0 * tr_cnt1;
+	dim1 = (uc->config.tr_trigger_type) ? 0 : (-1) * tr_cnt0;
 
 	/* estimate the number of TRs we will need */
 	for_each_sg(sgl, sgent, sglen, i) {
@@ -3085,7 +3092,7 @@ udma_prep_slave_sg_triggered_tr(struct udma_chan *uc, struct scatterlist *sgl,
 			tr_req[tr_idx].icnt1 = tr_cnt1;
 			tr_req[tr_idx].icnt2 = tr0_cnt2;
 			tr_req[tr_idx].icnt3 = tr0_cnt3;
-			tr_req[tr_idx].dim1 = (-1) * tr_cnt0;
+			tr_req[tr_idx].dim1 = dim1;
 
 			tr_req[tr_idx].daddr = sg_addr;
 			tr_req[tr_idx].dicnt0 = tr_cnt0;
@@ -3132,7 +3139,7 @@ udma_prep_slave_sg_triggered_tr(struct udma_chan *uc, struct scatterlist *sgl,
 				tr_req[tr_idx].icnt1 = tr_cnt1;
 				tr_req[tr_idx].icnt2 = tr1_cnt2;
 				tr_req[tr_idx].icnt3 = 1;
-				tr_req[tr_idx].dim1 = (-1) * tr_cnt0;
+				tr_req[tr_idx].dim1 = dim1;
 
 				tr_req[tr_idx].daddr = sg_addr;
 				tr_req[tr_idx].dicnt0 = tr_cnt0;
@@ -3417,6 +3424,7 @@ udma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 	struct udma_desc *d;
 	u32 burst;
 
+#if 0
 	if (dir != uc->config.dir &&
 	    (uc->config.dir == DMA_MEM_TO_MEM && !uc->config.tr_trigger_type)) {
 		dev_err(chan->device->dev,
@@ -3426,6 +3434,7 @@ udma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 			dmaengine_get_direction_text(dir));
 		return NULL;
 	}
+#endif
 
 	if (dir == DMA_DEV_TO_MEM) {
 		dev_width = uc->cfg.src_addr_width;
@@ -3446,7 +3455,7 @@ udma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 	if (uc->config.pkt_mode)
 		d = udma_prep_slave_sg_pkt(uc, sgl, sglen, dir, tx_flags,
 					   context);
-	else if (is_slave_direction(uc->config.dir))
+	else if (is_slave_direction(uc->config.dir) && !uc->config.tr_trigger_type)
 		d = udma_prep_slave_sg_tr(uc, sgl, sglen, dir, tx_flags,
 					  context);
 	else
@@ -3778,6 +3787,9 @@ static void udma_issue_pending(struct dma_chan *chan)
 	struct udma_chan *uc = to_udma_chan(chan);
 	unsigned long flags;
 
+	if (uc->config.tr_trigger_type && uc->bchan)
+		dev_info(NULL, "___chan%d: id %d\n", uc->id, uc->bchan->id);
+
 	spin_lock_irqsave(&uc->vc.lock, flags);
 
 	/* If we have something pending and no active descriptor, then */

When the DMA channel is allocated, this patch prints a debug message:

(NULL device *): ___chanX: id Y

then I use command 'devmem2 0x4c00Y008 w 1' to issue the sw trigger.

please note that each sw event triggers one DMA transfer for (conf.src_addr_width * conf.src_maxburst) bytes, which is a half frame size in your application. So you would have to issue the sw event multiple times until the DDR buffer is full, then the DMA completion callback() will be called.

Hi Hamish,

(Sorry, by accident I clicked "TI Thinks Resolved" instead of "Reply" button...)

I am glad the DMA is working (in a hacky way) in your project. Thanks for the update.

Yes, dmaengine API doesn't provide the physical DMA channel ID, which is needed here to calculate the register address for sw trigger. For a quick prototype, I added the printk in udma_issue_pending(), which got called in every DMA transfer. It probably could be moved to udma_slave_config() which is called once.

I actually didn't talk with our sw dev team about cyclic mode, since I have been making daily development progress in the last week or so. But now since I have made the sg_single mode working, I have more understanding of the UDMA driver and I believe the cyclic mode should work with GPMC too.

Is that my understanding correct that the cyclic mode has higher priority than GPIO hw trigger? If so, I will start to work on enabling cyclic mode from tomorrow.

I wan't aware of the GPMC 100MHz bus speed requirement. Is it tracked in an E2E thread? If not, please post it on this E2E forums. 

Hi Bin,

Yes I would say that the cyclic mode now has higher priority over the GPIO hw trigger.

The GPMC 100MHz was addressed in the Webex meeting we had and Anil made suggestions, and you said that you would show me how to change GPMC_CLK_SEL to 0 via the DT. Your rely in that email thread was on July 7 2023.

Hi Hamish,

Okay let me work on the cyclic mode then I will get to the GPMC clock. My apologies for lost tracking the emails.

Hi Bin,

Sorry to bug you, but do we see any progress on the cyclic mode?

Hi Hamish,

This cyclic mode for GPMC (which doesn't have PDMA) is a completely new implementation, it requires to a new function to use DMA TR15 for cyclic mode.

Until end of yesterday, I got the implementation prototype working - I can use sw trigger to start the DMA transfer repeatedly - cyclic. However some of the DMA configurations are not aligned with the existing cyclic mode implementation which uses DMA TR1. Those config misalignments should be understood to ensure a proper driver implementation. Then I will work on the DMA 4-dimensions parameter configuration in the new function to ensure correct data transfer. In summary, I probably need another week to deliver the final patch for you to integrate and validate. 

Hi Bin,

Thank you for your continued assistance, sounds good.

Hi Hamish,

I have made some great progress today, I can get dma cyclic transfer working correctly in my preliminary test.

Can you please confirm if your DDR buffer is less than 64KB, and the DDR buffer size is exact multiple of the FIFO size, no residue? My cyclic driver doesn't buffer > 64KB yet.

Hi Bin,

That sounds great!

The size of the DDR DMA buffer is variable and generally a lot larger than 64KB, our default is 4MB, but there are some applications where it is even bigger. The reason for this is that if an anomaly is detected in the data stream, a use-case is that the history leading up to the anomaly is critical.

Just to give you a concrete example, we do the following in our current setup: Firstly the FIFO we have in the FPGA is 20000 bytes, but the FPGA uses a max of 50% of that to store data and once the DMA is triggered, the FPGA writes to the other half of the FIFO, so what I refer to as the frame_size, must be less than 10000 bytes.

We allocate the max required size of the DMA buffer in the DT as a shared-dma-pool. Then we look at the frame size, so assume we have 8 channels with 256 'scans per frame', we have frame_size = header_size + sample_size * channels * scans_per_frame = 12 + 4 * 8 * 256 = 8204, we then see how many full frames we can fit in the DMA buffer frame_count = resource_size(dma_buffer) / frame_size = 4194304 / 8204 = 511, which means our DMA buffer size is 511 * 8204 = 4192244, meaning that we are not using 2060 bytes of the pre-reserved buffer space.

In the AM335x using the cyclic DMA, we then do the setup as follows:

conf.direction = DMA_DEV_TO_MEM;
conf.src_addr_width = ddev->bank_width; /* bank_width = 2 because we use 16-bit wide GPMC bus */
conf.src_maxburst = ddev->frame_size / ddev->bank_width;
conf.src_addr = (dma_addr_t) (FIFO_ADDRESS);
ret = dmaengine_slave_config(ddev->dma, &conf);

And then:

tx = dmaengine_prep_dma_cyclic(ddev->dma, ddev->dma_phys,
        ddev->geom.frame_count * ddev->frame_size,
        ddev->frame_size / ddev->bank_width,
        DMA_DEV_TO_MEM, flags);

I hope this explains more detail of our use-case?

Hi Hamish,

Appreciated the details. I believe my current cyclic mode driver should work for this use case. 

Let me confirm it and clear some understanding of the hardware behavior, then I likely should be able to provide you a preliminary patch some time next week.

Hi Bin,

Thank you very much - your persistence in this matter is highly appreciated. Hopefully we have a resolution soon!

Hi Hamish,

Sorry for the delay. I have been distracted multiple times while cleaning up the patches.

Please apply the following kernel patch on top of the previous patches. This adds the sw triggered cyclic mode support. The patch is preliminary, and I have not validated some corner cases, but I believe it should work in your src/dst configuration. Please let me if you run into any issue in cyclic mode.

diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
index cf653c2c8f37..bb19bf8009ae 100644
--- a/drivers/dma/ti/k3-udma.c
+++ b/drivers/dma/ti/k3-udma.c
@@ -2833,7 +2833,7 @@ static struct udma_desc *udma_alloc_tr_desc(struct udma_chan *uc,
 
 	tr_desc = hwdesc->cppi5_desc_vaddr;
 
-	if (uc->cyclic)
+	if (uc->cyclic && !uc->config.tr_trigger_type)
 		reload_count = CPPI5_INFO0_TRDESC_RLDCNT_INFINITE;
 
 	if (dir == DMA_DEV_TO_MEM && !uc->config.tr_trigger_type)
@@ -3559,6 +3559,70 @@ udma_prep_dma_cyclic_tr(struct udma_chan *uc, dma_addr_t buf_addr,
 	return d;
 }
 
+static struct udma_desc *
+udma_prep_dma_cyclic_triggered_tr(struct udma_chan *uc, dma_addr_t buf_addr,
+			size_t buf_len, size_t period_len,
+			enum dma_transfer_direction dir, unsigned long flags)
+{
+	struct udma_desc *d;
+	size_t tr_size;
+	dma_addr_t period_addr;
+	struct cppi5_tr_type15_t *tr_req;
+	unsigned int periods = buf_len / period_len;
+	u16 tr0_cnt0, tr0_cnt1, tr0_cnt2, tr0_cnt3, tr1_cnt0;
+	int num_tr;
+	int tr_idx = 0;
+
+	num_tr = udma_get_tr_counters(periods, 0, &tr0_cnt2, &tr0_cnt3,
+			&tr1_cnt0);
+	if (num_tr != 1 || !tr0_cnt2) {
+		dev_err(uc->ud->dev, "src size %zu and tgt size %zu are not supported\n",
+			period_len, buf_len);
+		return NULL;
+	}
+
+	tr0_cnt0 = uc->cfg.src_addr_width;
+	tr0_cnt1 = period_len / tr0_cnt0;
+
+	tr_size = sizeof(struct cppi5_tr_type15_t);
+	d = udma_alloc_tr_desc(uc, tr_size, periods * num_tr, dir);
+	if (!d)
+		return NULL;
+
+	tr_req = d->hwdesc[0].tr_req_base;
+	if (uc->ud->match_data->type == DMA_TYPE_UDMA)
+		period_addr = uc->cfg.src_addr;
+	else
+		period_addr = uc->cfg.src_addr |
+			((u64)uc->config.asel << K3_ADDRESS_ASEL_SHIFT);
+
+	cppi5_tr_init(&tr_req[tr_idx].flags, CPPI5_TR_TYPE15, false,
+		      true, CPPI5_TR_EVENT_SIZE_ICNT2_DEC, 0);
+	cppi5_tr_set_trigger(&tr_req[tr_idx].flags, uc->config.tr_trigger_type,
+			CPPI5_TR_TRIGGER_TYPE_ICNT2_DEC, 0, 0);
+
+	tr_req[tr_idx].addr = period_addr;
+	tr_req[tr_idx].icnt0 = tr0_cnt0;
+	tr_req[tr_idx].icnt1 = tr0_cnt1;
+	tr_req[tr_idx].icnt2 = periods;
+	tr_req[tr_idx].dim1 = 0;
+
+	tr_req[tr_idx].daddr = buf_addr |
+		((u64)uc->config.asel << K3_ADDRESS_ASEL_SHIFT);
+	tr_req[tr_idx].dicnt0 = tr0_cnt0;
+	tr_req[tr_idx].dicnt1 = tr0_cnt1;
+	tr_req[tr_idx].dicnt2 = periods;
+	tr_req[tr_idx].ddim1 = tr0_cnt0;
+	tr_req[tr_idx].ddim2 = period_len;
+
+	if (!(flags & DMA_PREP_INTERRUPT))
+		cppi5_tr_csf_set(&tr_req[tr_idx].flags, CPPI5_TR_CSF_SUPR_EVT);
+
+	cppi5_tr_csf_set(&tr_req[tr_idx].flags, CPPI5_TR_CSF_EOP);
+
+	return d;
+}
+
 static struct udma_desc *
 udma_prep_dma_cyclic_pkt(struct udma_chan *uc, dma_addr_t buf_addr,
 			 size_t buf_len, size_t period_len,
@@ -3637,6 +3701,7 @@ udma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
 	struct udma_desc *d;
 	u32 burst;
 
+#if 0
 	if (dir != uc->config.dir) {
 		dev_err(chan->device->dev,
 			"%s: chan%d is for %s, not supporting %s\n",
@@ -3645,6 +3710,7 @@ udma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
 			dmaengine_get_direction_text(dir));
 		return NULL;
 	}
+#endif
 
 	uc->cyclic = true;
 
@@ -3665,14 +3731,20 @@ udma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
 	if (uc->config.pkt_mode)
 		d = udma_prep_dma_cyclic_pkt(uc, buf_addr, buf_len, period_len,
 					     dir, flags);
-	else
+	else if (!uc->config.tr_trigger_type)
 		d = udma_prep_dma_cyclic_tr(uc, buf_addr, buf_len, period_len,
 					    dir, flags);
+	else
+		d = udma_prep_dma_cyclic_triggered_tr(uc, buf_addr, buf_len,
+					              period_len, dir, flags);
 
 	if (!d)
 		return NULL;
 
-	d->sglen = buf_len / period_len;
+	if (!uc->config.tr_trigger_type)
+		d->sglen = buf_len / period_len;
+	else
+		d->sglen = 1;
 
 	d->dir = dir;
 	d->residue = buf_len;
-- 
2.34.1

Hi Bin,

Thank you very much for the patch. I have tried it out, but I only get a single DMA transfer taking place, but I think I understand why.

So the way my hardware works is follows:
FPGA fills the FIFO with data and once the first frame of data is in the FIFO, it sends an interrupt to the SoC.
The FPGA then expects the SoC to transfer that frame of data from the FIFO to the DMA buffer and once that transfer is complete, it is expecting an interrupt acknowledge from the SoC. It also continues filling the next frame into the second half of the FIFO. If it has not received the interrupt acknowledge from the SoC when it finished filling the FIFO there is an error condition.

I believe what is happening with the new cyclic DMA is that I receive the interrupt from the FPGA and I write 1 to the trigger register and the DMA engine performs the first DMA transfer, but I do not see the callback being called by the DMA engine, which means that I never send the interrupt acknowledge to the FPGA so the FPGA overruns and stops generating interrupts.

I am going to continue investigating, but I thought I would give you some feedback as early as I could start drawing conclusions.

In our implementation with the am553x, the callback is called at the end of each and every cyclic transfer, which is a single frame of data.

Hi Mamish,

Thanks for the feedback.

Hamish Guthrie said:

but I only get a single DMA transfer taking place

Hamish Guthrie said:

but I do not see the callback being called by the DMA engine

What is the size of the single DMA transfer, one frame or multiple frames? If the DMA callback is not called, how do you know you got a single DMA transfer?

Here is how I tested it:

- the fake FIFO is 4 bytes, the DMA buffer is 8 bytes;

- when I send the sw trigger twice, the DMA engine callback is called, 

- keep sending sw trigger twice, the callback will be called, until dmaengine_terminate_async() is called in the callback function.

Please let me know what could be the issue to not triggering the DMA transfers repeatedly.

Hi Bin,

A single DMA transfer should be a single frame (8204 bytes)

I poked in the DMA buffer and I saw our standard header at the beginning of the buffer, and when I initialise my driver in debug mode, I ensure that I zero the DMA buffer, so I am pretty convinced there was a transfer.

My DMA buffer is 4192244 bytes, and a single frame is 8204 bytes, meaning that I have capacity for 511 frames in the DMA buffer.

So in my case, the FIFO is 8204 bytes, and I require the callback to be called after every frame is transferred in order to send my interrupt acknowledge to the FPGA because if I do not acknowledge after every frame, the FPGA will see and overflow and it will stop.

Hi Hamish,

Have you checked the DMA buffer has only 8204 bytes data, after 8204 bytes there are all zeros in the DMA buffer?

Hi Bin,

Yes, I checked the rest of the buffer is all 0's, and in fact we have a ramp generator which generates data in a ramp so every single word of data which is transmitted from the FPGA has a value

Hi Hamish,

Please remove the two lines below from the patch to see if it helps:

74 +       if (!(flags & DMA_PREP_INTERRUPT))
75 +               cppi5_tr_csf_set(&tr_req[tr_idx].flags, CPPI5_TR_CSF_SUPR_EVT);

Hi Bin,

I removed those two lines and it made no difference.

Actually, there is another thing I noticed, which I did not mention, after a bit of digging I had noticed that the amount of data in the DMA buffer was half the frame size, now when we set up the DMA transfer for the am335x, we first do dmaengine_slave_config, and importantly, for that we set src_addr_width to 2, because we are using a 16-bit wide GPMC bus, and then also we set src_maxburst = frame_size / 2. I am doing exactly the same for the am62x, then when I call dmaengine_prep_cyclic, I call it with buf_len = our calculated buffer size (from above 4192244 bytes) and period_len = frame_size / 2. This is where the difference is, if I have the period_len = frame_size / 2 on the am62x, I only get half of the frame transferred, whereas, if I use frame_len I get the complete frame transferred - so it appears in the am335x the period_len in the dmaengine_prep_dma_cyclic must be frame_len / src_addr_width.

The bottom line is that even with the 2 lines you asked me to remove not present, I still do not see the callback being called, and hence my interrupt acknowledge is not being sent to the FPGA

Hi Hamish,

Sorry for the delayed response.

Currently the AM62x DMA driver only requested DMA Ring IRQ for GPMC use case, but not the DMA channel IRQ.

The DMA Ring IRQ only triggers when the entire DMA transfer is done (filled up the multi-frame buffer in your case), but the DMA channel IRQ could be configured to trigger when transferred a single frame or multiple frames. Since the DMA channel IRQ is not requested in the current driver, we are not getting dma completion callback for every frame.

I am currently having issues when trying to request the DMA channel IRQ, and still working on it, but unlikely it will be resolved in this month, I all be taking time off from this Friday for the holidays.

Hi Hamish,

You mentioned offline that the GPMC bus clock runs as 66MHz on your board, I am wondering if the GPMC FCLK divider is configured as by 2. Can you please read the GPMC register 0x3B000060 to confirm this?

# devmem2 0x3b000060 w 

Hi Bin,

I read the value of that register and the value is 0, however, I also had a look in the TRM and this register is not described.