| Message ID | 20240930-z_cnt-v2-1-9d38aba149a2@linux.dev (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d35f40642904b017d1301340734b91aef69d1c0c |
| Headers | show |
| Series | [v2] dmaengine: ti: k3-udma: Set EOP for all TRs in cyclic BCDMA transfer | expand |
On 9/30/24 8:02 PM, Jai Luthra wrote: > From: Jai Luthra <j-luthra@ti.com> > > When receiving data in cyclic mode from PDMA peripherals, where reload > count is set to infinite, any TR in the set can potentially be the last > one of the overall transfer. In such cases, the EOP flag needs to be set > in each TR and PDMA's Static TR "Z" parameter should be set, matching > the size of the TR. > > This is required for the teardown to function properly and cleanup the > internal state memory. This only affects platforms using BCDMA and not > those using UDMA-P, which could set EOP flag in the teardown TR > automatically. > > Similarly when transmitting data in cyclic mode to PDMA peripherals, the > EOP flag needs to be set to get the teardown completion signal > correctly. Acked-by: Peter Ujfalusi <peter.ujfalusi@gmail.com> > Fixes: 017794739702 ("dmaengine: ti: k3-udma: Initial support for K3 BCDMA") > Tested-by: Francesco Dolcini <francesco.dolcini@toradex.com> # Toradex Verdin AM62 > Signed-off-by: Jai Luthra <j-luthra@ti.com> > Signed-off-by: Jai Luthra <jai.luthra@linux.dev> > --- > Changes in v2: > - Fix commit message and comments to make it clear that this change is > only needed for BCDMA > - Drop the redundant pkt_mode check > - Link to v1: https://lore.kernel.org/r/20240918-z_cnt-v1-1-2c58fbfb07d6@linux.dev > --- > drivers/dma/ti/k3-udma.c | 62 ++++++++++++++++++++++++++++++++++++------------ > 1 file changed, 47 insertions(+), 15 deletions(-) > > diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c > index 406ee199c2ac1cffc29edb475df574cf9f0cf222..b3f27b3f92098a65afe9656ca31f9b8027294c16 100644 > --- a/drivers/dma/ti/k3-udma.c > +++ b/drivers/dma/ti/k3-udma.c > @@ -3185,27 +3185,40 @@ static int udma_configure_statictr(struct udma_chan *uc, struct udma_desc *d, > > d->static_tr.elcnt = elcnt; > > - /* > - * PDMA must to close the packet when the channel is in packet mode. > - * For TR mode when the channel is not cyclic we also need PDMA to close > - * the packet otherwise the transfer will stall because PDMA holds on > - * the data it has received from the peripheral. > - */ > if (uc->config.pkt_mode || !uc->cyclic) { > + /* > + * PDMA must close the packet when the channel is in packet mode. > + * For TR mode when the channel is not cyclic we also need PDMA > + * to close the packet otherwise the transfer will stall because > + * PDMA holds on the data it has received from the peripheral. > + */ > unsigned int div = dev_width * elcnt; > > if (uc->cyclic) > d->static_tr.bstcnt = d->residue / d->sglen / div; > else > d->static_tr.bstcnt = d->residue / div; > + } else if (uc->ud->match_data->type == DMA_TYPE_BCDMA && > + uc->config.dir == DMA_DEV_TO_MEM && > + uc->cyclic) { > + /* > + * For cyclic mode with BCDMA we have to set EOP in each TR to > + * prevent short packet errors seen on channel teardown. So the > + * PDMA must close the packet after every TR transfer by setting > + * burst count equal to the number of bytes transferred. > + */ > + struct cppi5_tr_type1_t *tr_req = d->hwdesc[0].tr_req_base; > > - if (uc->config.dir == DMA_DEV_TO_MEM && > - d->static_tr.bstcnt > uc->ud->match_data->statictr_z_mask) > - return -EINVAL; > + d->static_tr.bstcnt = > + (tr_req->icnt0 * tr_req->icnt1) / dev_width; > } else { > d->static_tr.bstcnt = 0; > } > > + if (uc->config.dir == DMA_DEV_TO_MEM && > + d->static_tr.bstcnt > uc->ud->match_data->statictr_z_mask) > + return -EINVAL; > + > return 0; > } > > @@ -3450,8 +3463,9 @@ udma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl, > /* static TR for remote PDMA */ > if (udma_configure_statictr(uc, d, dev_width, burst)) { > dev_err(uc->ud->dev, > - "%s: StaticTR Z is limited to maximum 4095 (%u)\n", > - __func__, d->static_tr.bstcnt); > + "%s: StaticTR Z is limited to maximum %u (%u)\n", > + __func__, uc->ud->match_data->statictr_z_mask, > + d->static_tr.bstcnt); > > udma_free_hwdesc(uc, d); > kfree(d); > @@ -3476,6 +3490,7 @@ udma_prep_dma_cyclic_tr(struct udma_chan *uc, dma_addr_t buf_addr, > u16 tr0_cnt0, tr0_cnt1, tr1_cnt0; > unsigned int i; > int num_tr; > + u32 period_csf = 0; > > num_tr = udma_get_tr_counters(period_len, __ffs(buf_addr), &tr0_cnt0, > &tr0_cnt1, &tr1_cnt0); > @@ -3498,6 +3513,20 @@ udma_prep_dma_cyclic_tr(struct udma_chan *uc, dma_addr_t buf_addr, > period_addr = buf_addr | > ((u64)uc->config.asel << K3_ADDRESS_ASEL_SHIFT); > > + /* > + * For BCDMA <-> PDMA transfers, the EOP flag needs to be set on the > + * last TR of a descriptor, to mark the packet as complete. > + * This is required for getting the teardown completion message in case > + * of TX, and to avoid short-packet error in case of RX. > + * > + * As we are in cyclic mode, we do not know which period might be the > + * last one, so set the flag for each period. > + */ > + if (uc->config.ep_type == PSIL_EP_PDMA_XY && > + uc->ud->match_data->type == DMA_TYPE_BCDMA) { > + period_csf = CPPI5_TR_CSF_EOP; > + } > + > for (i = 0; i < periods; i++) { > int tr_idx = i * num_tr; > > @@ -3525,8 +3554,10 @@ udma_prep_dma_cyclic_tr(struct udma_chan *uc, dma_addr_t buf_addr, > } > > if (!(flags & DMA_PREP_INTERRUPT)) > - cppi5_tr_csf_set(&tr_req[tr_idx].flags, > - CPPI5_TR_CSF_SUPR_EVT); > + period_csf |= CPPI5_TR_CSF_SUPR_EVT; > + > + if (period_csf) > + cppi5_tr_csf_set(&tr_req[tr_idx].flags, period_csf); > > period_addr += period_len; > } > @@ -3655,8 +3686,9 @@ udma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len, > /* static TR for remote PDMA */ > if (udma_configure_statictr(uc, d, dev_width, burst)) { > dev_err(uc->ud->dev, > - "%s: StaticTR Z is limited to maximum 4095 (%u)\n", > - __func__, d->static_tr.bstcnt); > + "%s: StaticTR Z is limited to maximum %u (%u)\n", > + __func__, uc->ud->match_data->statictr_z_mask, > + d->static_tr.bstcnt); > > udma_free_hwdesc(uc, d); > kfree(d); > > --- > base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b > change-id: 20240918-z_cnt-3ca5daec76bf > > Best regards,
On Mon, 30 Sep 2024 13:02:54 -0400, Jai Luthra wrote: > When receiving data in cyclic mode from PDMA peripherals, where reload > count is set to infinite, any TR in the set can potentially be the last > one of the overall transfer. In such cases, the EOP flag needs to be set > in each TR and PDMA's Static TR "Z" parameter should be set, matching > the size of the TR. > > This is required for the teardown to function properly and cleanup the > internal state memory. This only affects platforms using BCDMA and not > those using UDMA-P, which could set EOP flag in the teardown TR > automatically. > > [...] Applied, thanks! [1/1] dmaengine: ti: k3-udma: Set EOP for all TRs in cyclic BCDMA transfer commit: d35f40642904b017d1301340734b91aef69d1c0c Best regards,
diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c index 406ee199c2ac1cffc29edb475df574cf9f0cf222..b3f27b3f92098a65afe9656ca31f9b8027294c16 100644 --- a/drivers/dma/ti/k3-udma.c +++ b/drivers/dma/ti/k3-udma.c @@ -3185,27 +3185,40 @@ static int udma_configure_statictr(struct udma_chan *uc, struct udma_desc *d, d->static_tr.elcnt = elcnt; - /* - * PDMA must to close the packet when the channel is in packet mode. - * For TR mode when the channel is not cyclic we also need PDMA to close - * the packet otherwise the transfer will stall because PDMA holds on - * the data it has received from the peripheral. - */ if (uc->config.pkt_mode || !uc->cyclic) { + /* + * PDMA must close the packet when the channel is in packet mode. + * For TR mode when the channel is not cyclic we also need PDMA + * to close the packet otherwise the transfer will stall because + * PDMA holds on the data it has received from the peripheral. + */ unsigned int div = dev_width * elcnt; if (uc->cyclic) d->static_tr.bstcnt = d->residue / d->sglen / div; else d->static_tr.bstcnt = d->residue / div; + } else if (uc->ud->match_data->type == DMA_TYPE_BCDMA && + uc->config.dir == DMA_DEV_TO_MEM && + uc->cyclic) { + /* + * For cyclic mode with BCDMA we have to set EOP in each TR to + * prevent short packet errors seen on channel teardown. So the + * PDMA must close the packet after every TR transfer by setting + * burst count equal to the number of bytes transferred. + */ + struct cppi5_tr_type1_t *tr_req = d->hwdesc[0].tr_req_base; - if (uc->config.dir == DMA_DEV_TO_MEM && - d->static_tr.bstcnt > uc->ud->match_data->statictr_z_mask) - return -EINVAL; + d->static_tr.bstcnt = + (tr_req->icnt0 * tr_req->icnt1) / dev_width; } else { d->static_tr.bstcnt = 0; } + if (uc->config.dir == DMA_DEV_TO_MEM && + d->static_tr.bstcnt > uc->ud->match_data->statictr_z_mask) + return -EINVAL; + return 0; } @@ -3450,8 +3463,9 @@ udma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl, /* static TR for remote PDMA */ if (udma_configure_statictr(uc, d, dev_width, burst)) { dev_err(uc->ud->dev, - "%s: StaticTR Z is limited to maximum 4095 (%u)\n", - __func__, d->static_tr.bstcnt); + "%s: StaticTR Z is limited to maximum %u (%u)\n", + __func__, uc->ud->match_data->statictr_z_mask, + d->static_tr.bstcnt); udma_free_hwdesc(uc, d); kfree(d); @@ -3476,6 +3490,7 @@ udma_prep_dma_cyclic_tr(struct udma_chan *uc, dma_addr_t buf_addr, u16 tr0_cnt0, tr0_cnt1, tr1_cnt0; unsigned int i; int num_tr; + u32 period_csf = 0; num_tr = udma_get_tr_counters(period_len, __ffs(buf_addr), &tr0_cnt0, &tr0_cnt1, &tr1_cnt0); @@ -3498,6 +3513,20 @@ udma_prep_dma_cyclic_tr(struct udma_chan *uc, dma_addr_t buf_addr, period_addr = buf_addr | ((u64)uc->config.asel << K3_ADDRESS_ASEL_SHIFT); + /* + * For BCDMA <-> PDMA transfers, the EOP flag needs to be set on the + * last TR of a descriptor, to mark the packet as complete. + * This is required for getting the teardown completion message in case + * of TX, and to avoid short-packet error in case of RX. + * + * As we are in cyclic mode, we do not know which period might be the + * last one, so set the flag for each period. + */ + if (uc->config.ep_type == PSIL_EP_PDMA_XY && + uc->ud->match_data->type == DMA_TYPE_BCDMA) { + period_csf = CPPI5_TR_CSF_EOP; + } + for (i = 0; i < periods; i++) { int tr_idx = i * num_tr; @@ -3525,8 +3554,10 @@ udma_prep_dma_cyclic_tr(struct udma_chan *uc, dma_addr_t buf_addr, } if (!(flags & DMA_PREP_INTERRUPT)) - cppi5_tr_csf_set(&tr_req[tr_idx].flags, - CPPI5_TR_CSF_SUPR_EVT); + period_csf |= CPPI5_TR_CSF_SUPR_EVT; + + if (period_csf) + cppi5_tr_csf_set(&tr_req[tr_idx].flags, period_csf); period_addr += period_len; } @@ -3655,8 +3686,9 @@ udma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len, /* static TR for remote PDMA */ if (udma_configure_statictr(uc, d, dev_width, burst)) { dev_err(uc->ud->dev, - "%s: StaticTR Z is limited to maximum 4095 (%u)\n", - __func__, d->static_tr.bstcnt); + "%s: StaticTR Z is limited to maximum %u (%u)\n", + __func__, uc->ud->match_data->statictr_z_mask, + d->static_tr.bstcnt); udma_free_hwdesc(uc, d); kfree(d);