| Message ID | 1483771530-8545-4-git-send-email-appanad@xilinx.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Sat, Jan 07, 2017 at 12:15:30PM +0530, Kedareswara rao Appana wrote: > When driver is handling AXI DMA SoftIP > When user submits multiple descriptors back to back on the S2MM(recv) > side with the current driver flow the last buffer descriptor next bd > points to a invalid location resulting the invalid data or errors in the > DMA engine. Can you rephrase this, it a bit hard to understand. > > This patch fixes this issue by creating a BD Chain during whats a BD? > channel allocation itself and use those BD's. > > Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com> > --- > Changes for v5: > ---> None. > Changes for v4: > ---> None. > Changes for v3: > ---> None. > Changes for v2: > ---> None. > > drivers/dma/xilinx/xilinx_dma.c | 133 +++++++++++++++++++++++++--------------- > 1 file changed, 83 insertions(+), 50 deletions(-) > > diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c > index 0e9c02e..af2159d 100644 > --- a/drivers/dma/xilinx/xilinx_dma.c > +++ b/drivers/dma/xilinx/xilinx_dma.c > @@ -163,6 +163,7 @@ > #define XILINX_DMA_BD_SOP BIT(27) > #define XILINX_DMA_BD_EOP BIT(26) > #define XILINX_DMA_COALESCE_MAX 255 > +#define XILINX_DMA_NUM_DESCS 255 why 255? > #define XILINX_DMA_NUM_APP_WORDS 5 > > /* Multi-Channel DMA Descriptor offsets*/ > @@ -310,6 +311,7 @@ struct xilinx_dma_tx_descriptor { > * @pending_list: Descriptors waiting > * @active_list: Descriptors ready to submit > * @done_list: Complete descriptors > + * @free_seg_list: Free descriptors > * @common: DMA common channel > * @desc_pool: Descriptors pool > * @dev: The dma device > @@ -331,7 +333,9 @@ struct xilinx_dma_tx_descriptor { > * @desc_submitcount: Descriptor h/w submitted count > * @residue: Residue for AXI DMA > * @seg_v: Statically allocated segments base > + * @seg_p: Physical allocated segments base > * @cyclic_seg_v: Statically allocated segment base for cyclic transfers > + * @cyclic_seg_p: Physical allocated segments base for cyclic dma > * @start_transfer: Differentiate b/w DMA IP's transfer > */ > struct xilinx_dma_chan { > @@ -342,6 +346,7 @@ struct xilinx_dma_chan { > struct list_head pending_list; > struct list_head active_list; > struct list_head done_list; > + struct list_head free_seg_list; > struct dma_chan common; > struct dma_pool *desc_pool; > struct device *dev; > @@ -363,7 +368,9 @@ struct xilinx_dma_chan { > u32 desc_submitcount; > u32 residue; > struct xilinx_axidma_tx_segment *seg_v; > + dma_addr_t seg_p; > struct xilinx_axidma_tx_segment *cyclic_seg_v; > + dma_addr_t cyclic_seg_p; > void (*start_transfer)(struct xilinx_dma_chan *chan); > u16 tdest; > }; > @@ -569,17 +576,31 @@ static struct xilinx_axidma_tx_segment * > xilinx_axidma_alloc_tx_segment(struct xilinx_dma_chan *chan) > { > struct xilinx_axidma_tx_segment *segment; > - dma_addr_t phys; > - > - segment = dma_pool_zalloc(chan->desc_pool, GFP_ATOMIC, &phys); > - if (!segment) > - return NULL; > + unsigned long flags; > > - segment->phys = phys; > + spin_lock_irqsave(&chan->lock, flags); > + if (!list_empty(&chan->free_seg_list)) { > + segment = list_first_entry(&chan->free_seg_list, > + struct xilinx_axidma_tx_segment, > + node); > + list_del(&segment->node); > + } > + spin_unlock_irqrestore(&chan->lock, flags); > > return segment; > } > > +static void xilinx_dma_clean_hw_desc(struct xilinx_axidma_desc_hw *hw) > +{ > + u32 next_desc = hw->next_desc; > + u32 next_desc_msb = hw->next_desc_msb; > + > + memset(hw, 0, sizeof(struct xilinx_axidma_desc_hw)); > + > + hw->next_desc = next_desc; > + hw->next_desc_msb = next_desc_msb; > +} > + > /** > * xilinx_dma_free_tx_segment - Free transaction segment > * @chan: Driver specific DMA channel > @@ -588,7 +609,9 @@ xilinx_axidma_alloc_tx_segment(struct xilinx_dma_chan *chan) > static void xilinx_dma_free_tx_segment(struct xilinx_dma_chan *chan, > struct xilinx_axidma_tx_segment *segment) > { > - dma_pool_free(chan->desc_pool, segment, segment->phys); > + xilinx_dma_clean_hw_desc(&segment->hw); > + > + list_add_tail(&segment->node, &chan->free_seg_list); > } > > /** > @@ -713,16 +736,26 @@ static void xilinx_dma_free_descriptors(struct xilinx_dma_chan *chan) > static void xilinx_dma_free_chan_resources(struct dma_chan *dchan) > { > struct xilinx_dma_chan *chan = to_xilinx_chan(dchan); > + unsigned long flags; > > dev_dbg(chan->dev, "Free all channel resources.\n"); > > xilinx_dma_free_descriptors(chan); > + > if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) { > - xilinx_dma_free_tx_segment(chan, chan->cyclic_seg_v); > - xilinx_dma_free_tx_segment(chan, chan->seg_v); > + spin_lock_irqsave(&chan->lock, flags); > + INIT_LIST_HEAD(&chan->free_seg_list); > + spin_unlock_irqrestore(&chan->lock, flags); > + > + /* Free Memory that is allocated for cyclic DMA Mode */ > + dma_free_coherent(chan->dev, sizeof(*chan->cyclic_seg_v), > + chan->cyclic_seg_v, chan->cyclic_seg_p); > + } > + > + if (chan->xdev->dma_config->dmatype != XDMA_TYPE_AXIDMA) { > + dma_pool_destroy(chan->desc_pool); > + chan->desc_pool = NULL; > } > - dma_pool_destroy(chan->desc_pool); > - chan->desc_pool = NULL; > } > > /** > @@ -805,6 +838,7 @@ static void xilinx_dma_do_tasklet(unsigned long data) > static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan) > { > struct xilinx_dma_chan *chan = to_xilinx_chan(dchan); > + int i; > > /* Has this channel already been allocated? */ > if (chan->desc_pool) > @@ -815,11 +849,30 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan) > * for meeting Xilinx VDMA specification requirement. > */ > if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) { > - chan->desc_pool = dma_pool_create("xilinx_dma_desc_pool", > - chan->dev, > - sizeof(struct xilinx_axidma_tx_segment), > - __alignof__(struct xilinx_axidma_tx_segment), > - 0); > + /* Allocate the buffer descriptors. */ > + chan->seg_v = dma_zalloc_coherent(chan->dev, > + sizeof(*chan->seg_v) * > + XILINX_DMA_NUM_DESCS, > + &chan->seg_p, GFP_KERNEL); > + if (!chan->seg_v) { > + dev_err(chan->dev, > + "unable to allocate channel %d descriptors\n", > + chan->id); > + return -ENOMEM; > + } > + > + for (i = 0; i < XILINX_DMA_NUM_DESCS; i++) { > + chan->seg_v[i].hw.next_desc = > + lower_32_bits(chan->seg_p + sizeof(*chan->seg_v) * > + ((i + 1) % XILINX_DMA_NUM_DESCS)); > + chan->seg_v[i].hw.next_desc_msb = > + upper_32_bits(chan->seg_p + sizeof(*chan->seg_v) * > + ((i + 1) % XILINX_DMA_NUM_DESCS)); > + chan->seg_v[i].phys = chan->seg_p + > + sizeof(*chan->seg_v) * i; > + list_add_tail(&chan->seg_v[i].node, > + &chan->free_seg_list); > + } > } else if (chan->xdev->dma_config->dmatype == XDMA_TYPE_CDMA) { > chan->desc_pool = dma_pool_create("xilinx_cdma_desc_pool", > chan->dev, > @@ -834,7 +887,8 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan) > 0); > } > > - if (!chan->desc_pool) { > + if (!chan->desc_pool && > + (chan->xdev->dma_config->dmatype != XDMA_TYPE_AXIDMA)) { > dev_err(chan->dev, > "unable to allocate channel %d descriptor pool\n", > chan->id); > @@ -843,22 +897,20 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan) > > if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) { > /* > - * For AXI DMA case after submitting a pending_list, keep > - * an extra segment allocated so that the "next descriptor" > - * pointer on the tail descriptor always points to a > - * valid descriptor, even when paused after reaching taildesc. > - * This way, it is possible to issue additional > - * transfers without halting and restarting the channel. > - */ > - chan->seg_v = xilinx_axidma_alloc_tx_segment(chan); > - > - /* > * For cyclic DMA mode we need to program the tail Descriptor > * register with a value which is not a part of the BD chain > * so allocating a desc segment during channel allocation for > * programming tail descriptor. > */ > - chan->cyclic_seg_v = xilinx_axidma_alloc_tx_segment(chan); > + chan->cyclic_seg_v = dma_zalloc_coherent(chan->dev, > + sizeof(*chan->cyclic_seg_v), > + &chan->cyclic_seg_p, GFP_KERNEL); > + if (!chan->cyclic_seg_v) { > + dev_err(chan->dev, > + "unable to allocate desc segment for cyclic DMA\n"); > + return -ENOMEM; > + } > + chan->cyclic_seg_v->phys = chan->cyclic_seg_p; > } > > dma_cookie_init(dchan); > @@ -1198,7 +1250,7 @@ static void xilinx_cdma_start_transfer(struct xilinx_dma_chan *chan) > static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan) > { > struct xilinx_dma_tx_descriptor *head_desc, *tail_desc; > - struct xilinx_axidma_tx_segment *tail_segment, *old_head, *new_head; > + struct xilinx_axidma_tx_segment *tail_segment; > u32 reg; > > if (chan->err) > @@ -1217,21 +1269,6 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan) > tail_segment = list_last_entry(&tail_desc->segments, > struct xilinx_axidma_tx_segment, node); > > - if (chan->has_sg && !chan->xdev->mcdma) { > - old_head = list_first_entry(&head_desc->segments, > - struct xilinx_axidma_tx_segment, node); > - new_head = chan->seg_v; > - /* Copy Buffer Descriptor fields. */ > - new_head->hw = old_head->hw; > - > - /* Swap and save new reserve */ > - list_replace_init(&old_head->node, &new_head->node); > - chan->seg_v = old_head; > - > - tail_segment->hw.next_desc = chan->seg_v->phys; > - head_desc->async_tx.phys = new_head->phys; > - } > - > reg = dma_ctrl_read(chan, XILINX_DMA_REG_DMACR); > > if (chan->desc_pendingcount <= XILINX_DMA_COALESCE_MAX) { > @@ -1729,7 +1766,7 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg( > { > struct xilinx_dma_chan *chan = to_xilinx_chan(dchan); > struct xilinx_dma_tx_descriptor *desc; > - struct xilinx_axidma_tx_segment *segment = NULL, *prev = NULL; > + struct xilinx_axidma_tx_segment *segment = NULL; > u32 *app_w = (u32 *)context; > struct scatterlist *sg; > size_t copy; > @@ -1780,10 +1817,6 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg( > XILINX_DMA_NUM_APP_WORDS); > } > > - if (prev) > - prev->hw.next_desc = segment->phys; > - > - prev = segment; > sg_used += copy; > > /* > @@ -1797,7 +1830,6 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg( > segment = list_first_entry(&desc->segments, > struct xilinx_axidma_tx_segment, node); > desc->async_tx.phys = segment->phys; > - prev->hw.next_desc = segment->phys; > > /* For the last DMA_MEM_TO_DEV transfer, set EOP */ > if (chan->direction == DMA_MEM_TO_DEV) { > @@ -2341,6 +2373,7 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev, > INIT_LIST_HEAD(&chan->pending_list); > INIT_LIST_HEAD(&chan->done_list); > INIT_LIST_HEAD(&chan->active_list); > + INIT_LIST_HEAD(&chan->free_seg_list); > > /* Retrieve the channel properties from the device tree */ > has_dre = of_property_read_bool(node, "xlnx,include-dre"); > -- > 2.1.2 >
Hi Vinod, Thanks for the review... > On Sat, Jan 07, 2017 at 12:15:30PM +0530, Kedareswara rao Appana wrote: > > When driver is handling AXI DMA SoftIP When user submits multiple > > descriptors back to back on the S2MM(recv) side with the current > > driver flow the last buffer descriptor next bd points to a invalid > > location resulting the invalid data or errors in the DMA engine. > > Can you rephrase this, it a bit hard to understand. When DMA is receiving packets h/w expects the descriptors Should be in the form of a ring (I mean h/w buffer descriptor Next descriptor field should always point to valid address So that when DMA engine go and fetch that next descriptor it always Sees a valid address). But with the current driver implementation when user queues Multiple descriptors the last descriptor next descriptor field Pointing to an invalid location causing data corruption or Errors from the DMA h/w engine... To avoid this issue creating a Buffer descriptor Chain during Channel allocation and using those buffer descriptors for processing User requested data. Please let me know if the above explanation is not clear will explain in detail.... > > > > > This patch fixes this issue by creating a BD Chain during > > whats a BD? Buffer descriptor. > > > channel allocation itself and use those BD's. > > > > Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com> > > --- > > > > drivers/dma/xilinx/xilinx_dma.c | 133 > > +++++++++++++++++++++++++--------------- > > 1 file changed, 83 insertions(+), 50 deletions(-) > > > > diff --git a/drivers/dma/xilinx/xilinx_dma.c > > b/drivers/dma/xilinx/xilinx_dma.c index 0e9c02e..af2159d 100644 > > --- a/drivers/dma/xilinx/xilinx_dma.c > > +++ b/drivers/dma/xilinx/xilinx_dma.c > > @@ -163,6 +163,7 @@ > > #define XILINX_DMA_BD_SOP BIT(27) > > #define XILINX_DMA_BD_EOP BIT(26) > > #define XILINX_DMA_COALESCE_MAX 255 > > +#define XILINX_DMA_NUM_DESCS 255 > > why 255? It is not an h/w limitation Allocating 255 descriptors (Each descriptor is capable of sending 7MB data) So roughly using allocated descriptors DMA engine can transfer 1GB data And in the driver we are reusing the allocated descriptors when they are free. Regards, Kedar. -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 12, 2017 at 02:19:49PM +0000, Appana Durga Kedareswara Rao wrote: > Hi Vinod, > > Thanks for the review... > > > On Sat, Jan 07, 2017 at 12:15:30PM +0530, Kedareswara rao Appana wrote: > > > When driver is handling AXI DMA SoftIP When user submits multiple > > > descriptors back to back on the S2MM(recv) side with the current > > > driver flow the last buffer descriptor next bd points to a invalid > > > location resulting the invalid data or errors in the DMA engine. > > > > Can you rephrase this, it a bit hard to understand. > > When DMA is receiving packets h/w expects the descriptors > Should be in the form of a ring (I mean h/w buffer descriptor > Next descriptor field should always point to valid address > So that when DMA engine go and fetch that next descriptor it always > Sees a valid address). > > > But with the current driver implementation when user queues > Multiple descriptors the last descriptor next descriptor field > Pointing to an invalid location causing data corruption or > Errors from the DMA h/w engine... > > To avoid this issue creating a Buffer descriptor Chain during > Channel allocation and using those buffer descriptors for processing > User requested data. Is it not doable to to modify the next pointer to point to subsequent transaction. IOW you are modifying tail descriptor to point to subsequent descriptor. Btw how and when does DMA stop, assuming it is circular it never would, isn't there a valid/stop flag associated with a descriptor which tells DMA engine what to do next Btw there is something wrong with your MUA perhaps line are titlecased for no reason. This is typically behavious of non linux tool which may not be great tool for this work. > > Please let me know if the above explanation is not clear will explain in detail.... > > > > > > > > > This patch fixes this issue by creating a BD Chain during > > > > whats a BD? > > Buffer descriptor. Thats nowhere mentioned.. > > > > > > channel allocation itself and use those BD's. > > > > > > Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com> > > > --- > > > > > > drivers/dma/xilinx/xilinx_dma.c | 133 > > > +++++++++++++++++++++++++--------------- > > > 1 file changed, 83 insertions(+), 50 deletions(-) > > > > > > diff --git a/drivers/dma/xilinx/xilinx_dma.c > > > b/drivers/dma/xilinx/xilinx_dma.c index 0e9c02e..af2159d 100644 > > > --- a/drivers/dma/xilinx/xilinx_dma.c > > > +++ b/drivers/dma/xilinx/xilinx_dma.c > > > @@ -163,6 +163,7 @@ > > > #define XILINX_DMA_BD_SOP BIT(27) > > > #define XILINX_DMA_BD_EOP BIT(26) > > > #define XILINX_DMA_COALESCE_MAX 255 > > > +#define XILINX_DMA_NUM_DESCS 255 > > > > why 255? > > It is not an h/w limitation > Allocating 255 descriptors (Each descriptor is capable of sending 7MB data) > So roughly using allocated descriptors DMA engine can transfer 1GB data > And in the driver we are reusing the allocated descriptors when they are free. > > Regards, > Kedar.
Hi Vinod, Thanks for the review... [Snip] > > > > > On Sat, Jan 07, 2017 at 12:15:30PM +0530, Kedareswara rao Appana wrote: > > > > When driver is handling AXI DMA SoftIP When user submits multiple > > > > descriptors back to back on the S2MM(recv) side with the current > > > > driver flow the last buffer descriptor next bd points to a invalid > > > > location resulting the invalid data or errors in the DMA engine. > > > > > > Can you rephrase this, it a bit hard to understand. > > > > When DMA is receiving packets h/w expects the descriptors Should be in > > the form of a ring (I mean h/w buffer descriptor Next descriptor field > > should always point to valid address So that when DMA engine go and > > fetch that next descriptor it always Sees a valid address). > > > > > > But with the current driver implementation when user queues Multiple > > descriptors the last descriptor next descriptor field Pointing to an > > invalid location causing data corruption or Errors from the DMA h/w > > engine... > > > > To avoid this issue creating a Buffer descriptor Chain during Channel > > allocation and using those buffer descriptors for processing User > > requested data. > > Is it not doable to to modify the next pointer to point to subsequent transaction. > IOW you are modifying tail descriptor to point to subsequent descriptor. > > Btw how and when does DMA stop, assuming it is circular it never would, isn't > there a valid/stop flag associated with a descriptor which tells DMA engine what > to do next There are two registers that controls the DMA transfers. Current descriptor and tail descriptor register. When current descriptor reaches tail descriptor dma engine will pause. When reprogramming the tail descriptor the DMA engine will starts fetching descriptors again. But with the existing driver flow if we reprogram the tail descriptor The tail descriptor next descriptor field is pointing to an invalid location Causing data corruption... > > > Btw there is something wrong with your MUA perhaps line are titlecased for no > reason. This is typically behavious of non linux tool which may not be great tool > for this work. Thanks for pointing it out. I usually replies from outlook from a windows machine. Will check with others in my team how they configured their mail client. > > > > > Please let me know if the above explanation is not clear will explain in detail.... > > > > > > > > > > > > > This patch fixes this issue by creating a BD Chain during > > > > > > whats a BD? > > > > Buffer descriptor. > > Thats nowhere mentioned.. Yep sorry I should have been mentioned it... Regards, Kedar. -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 13, 2017 at 06:00:44AM +0000, Appana Durga Kedareswara Rao wrote: > Hi Vinod, > > Thanks for the review... > > [Snip] > > > > > > > On Sat, Jan 07, 2017 at 12:15:30PM +0530, Kedareswara rao Appana wrote: > > > > > When driver is handling AXI DMA SoftIP When user submits multiple > > > > > descriptors back to back on the S2MM(recv) side with the current > > > > > driver flow the last buffer descriptor next bd points to a invalid > > > > > location resulting the invalid data or errors in the DMA engine. > > > > > > > > Can you rephrase this, it a bit hard to understand. > > > > > > When DMA is receiving packets h/w expects the descriptors Should be in > > > the form of a ring (I mean h/w buffer descriptor Next descriptor field > > > should always point to valid address So that when DMA engine go and > > > fetch that next descriptor it always Sees a valid address). > > > > > > > > > But with the current driver implementation when user queues Multiple > > > descriptors the last descriptor next descriptor field Pointing to an > > > invalid location causing data corruption or Errors from the DMA h/w > > > engine... > > > > > > To avoid this issue creating a Buffer descriptor Chain during Channel > > > allocation and using those buffer descriptors for processing User > > > requested data. > > > > Is it not doable to to modify the next pointer to point to subsequent transaction. > > IOW you are modifying tail descriptor to point to subsequent descriptor. > > > > Btw how and when does DMA stop, assuming it is circular it never would, isn't > > there a valid/stop flag associated with a descriptor which tells DMA engine what > > to do next > > There are two registers that controls the DMA transfers. > Current descriptor and tail descriptor register. > When current descriptor reaches tail descriptor dma engine will pause. > > When reprogramming the tail descriptor the DMA engine will starts fetching descriptors again. > > But with the existing driver flow if we reprogram the tail descriptor > The tail descriptor next descriptor field is pointing to an invalid location > Causing data corruption... So the solution is..? > > Btw there is something wrong with your MUA perhaps line are titlecased for no > > reason. This is typically behavious of non linux tool which may not be great tool > > for this work. > > Thanks for pointing it out. > I usually replies from outlook from a windows machine. > Will check with others in my team how they configured their mail client. Yeah that isnt right tool for the job. See Documentation/process/email-clients.rst FWIW, I use mutt, vim as editor with exchange servers, seems to work well for me now.
Hi Vinod, Thanks for the review... [Snip] > > > Btw how and when does DMA stop, assuming it is circular it never > > > would, isn't there a valid/stop flag associated with a descriptor > > > which tells DMA engine what to do next > > > > There are two registers that controls the DMA transfers. > > Current descriptor and tail descriptor register. > > When current descriptor reaches tail descriptor dma engine will pause. > > > > When reprogramming the tail descriptor the DMA engine will starts fetching > descriptors again. > > > > But with the existing driver flow if we reprogram the tail descriptor > > The tail descriptor next descriptor field is pointing to an invalid > > location Causing data corruption... > > So the solution is..? This patch. I mean if we have a set of descriptors in chain (in circular manner last descriptor points to first descriptor) It always points to valid descriptors. Will update the patch commit description in the next version... > > > > Btw there is something wrong with your MUA perhaps line are > > > titlecased for no reason. This is typically behavious of non linux > > > tool which may not be great tool for this work. > > > > Thanks for pointing it out. > > I usually replies from outlook from a windows machine. > > Will check with others in my team how they configured their mail client. > > Yeah that isnt right tool for the job. See Documentation/process/email- > clients.rst > > FWIW, I use mutt, vim as editor with exchange servers, seems to work well for > me now. Thanks for pointing it out will go through it. Will install mutt in my Linux machine will start replying from that Regards, Kedar. > > -- > ~Vinod -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c index 0e9c02e..af2159d 100644 --- a/drivers/dma/xilinx/xilinx_dma.c +++ b/drivers/dma/xilinx/xilinx_dma.c @@ -163,6 +163,7 @@ #define XILINX_DMA_BD_SOP BIT(27) #define XILINX_DMA_BD_EOP BIT(26) #define XILINX_DMA_COALESCE_MAX 255 +#define XILINX_DMA_NUM_DESCS 255 #define XILINX_DMA_NUM_APP_WORDS 5 /* Multi-Channel DMA Descriptor offsets*/ @@ -310,6 +311,7 @@ struct xilinx_dma_tx_descriptor { * @pending_list: Descriptors waiting * @active_list: Descriptors ready to submit * @done_list: Complete descriptors + * @free_seg_list: Free descriptors * @common: DMA common channel * @desc_pool: Descriptors pool * @dev: The dma device @@ -331,7 +333,9 @@ struct xilinx_dma_tx_descriptor { * @desc_submitcount: Descriptor h/w submitted count * @residue: Residue for AXI DMA * @seg_v: Statically allocated segments base + * @seg_p: Physical allocated segments base * @cyclic_seg_v: Statically allocated segment base for cyclic transfers + * @cyclic_seg_p: Physical allocated segments base for cyclic dma * @start_transfer: Differentiate b/w DMA IP's transfer */ struct xilinx_dma_chan { @@ -342,6 +346,7 @@ struct xilinx_dma_chan { struct list_head pending_list; struct list_head active_list; struct list_head done_list; + struct list_head free_seg_list; struct dma_chan common; struct dma_pool *desc_pool; struct device *dev; @@ -363,7 +368,9 @@ struct xilinx_dma_chan { u32 desc_submitcount; u32 residue; struct xilinx_axidma_tx_segment *seg_v; + dma_addr_t seg_p; struct xilinx_axidma_tx_segment *cyclic_seg_v; + dma_addr_t cyclic_seg_p; void (*start_transfer)(struct xilinx_dma_chan *chan); u16 tdest; }; @@ -569,17 +576,31 @@ static struct xilinx_axidma_tx_segment * xilinx_axidma_alloc_tx_segment(struct xilinx_dma_chan *chan) { struct xilinx_axidma_tx_segment *segment; - dma_addr_t phys; - - segment = dma_pool_zalloc(chan->desc_pool, GFP_ATOMIC, &phys); - if (!segment) - return NULL; + unsigned long flags; - segment->phys = phys; + spin_lock_irqsave(&chan->lock, flags); + if (!list_empty(&chan->free_seg_list)) { + segment = list_first_entry(&chan->free_seg_list, + struct xilinx_axidma_tx_segment, + node); + list_del(&segment->node); + } + spin_unlock_irqrestore(&chan->lock, flags); return segment; } +static void xilinx_dma_clean_hw_desc(struct xilinx_axidma_desc_hw *hw) +{ + u32 next_desc = hw->next_desc; + u32 next_desc_msb = hw->next_desc_msb; + + memset(hw, 0, sizeof(struct xilinx_axidma_desc_hw)); + + hw->next_desc = next_desc; + hw->next_desc_msb = next_desc_msb; +} + /** * xilinx_dma_free_tx_segment - Free transaction segment * @chan: Driver specific DMA channel @@ -588,7 +609,9 @@ xilinx_axidma_alloc_tx_segment(struct xilinx_dma_chan *chan) static void xilinx_dma_free_tx_segment(struct xilinx_dma_chan *chan, struct xilinx_axidma_tx_segment *segment) { - dma_pool_free(chan->desc_pool, segment, segment->phys); + xilinx_dma_clean_hw_desc(&segment->hw); + + list_add_tail(&segment->node, &chan->free_seg_list); } /** @@ -713,16 +736,26 @@ static void xilinx_dma_free_descriptors(struct xilinx_dma_chan *chan) static void xilinx_dma_free_chan_resources(struct dma_chan *dchan) { struct xilinx_dma_chan *chan = to_xilinx_chan(dchan); + unsigned long flags; dev_dbg(chan->dev, "Free all channel resources.\n"); xilinx_dma_free_descriptors(chan); + if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) { - xilinx_dma_free_tx_segment(chan, chan->cyclic_seg_v); - xilinx_dma_free_tx_segment(chan, chan->seg_v); + spin_lock_irqsave(&chan->lock, flags); + INIT_LIST_HEAD(&chan->free_seg_list); + spin_unlock_irqrestore(&chan->lock, flags); + + /* Free Memory that is allocated for cyclic DMA Mode */ + dma_free_coherent(chan->dev, sizeof(*chan->cyclic_seg_v), + chan->cyclic_seg_v, chan->cyclic_seg_p); + } + + if (chan->xdev->dma_config->dmatype != XDMA_TYPE_AXIDMA) { + dma_pool_destroy(chan->desc_pool); + chan->desc_pool = NULL; } - dma_pool_destroy(chan->desc_pool); - chan->desc_pool = NULL; } /** @@ -805,6 +838,7 @@ static void xilinx_dma_do_tasklet(unsigned long data) static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan) { struct xilinx_dma_chan *chan = to_xilinx_chan(dchan); + int i; /* Has this channel already been allocated? */ if (chan->desc_pool) @@ -815,11 +849,30 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan) * for meeting Xilinx VDMA specification requirement. */ if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) { - chan->desc_pool = dma_pool_create("xilinx_dma_desc_pool", - chan->dev, - sizeof(struct xilinx_axidma_tx_segment), - __alignof__(struct xilinx_axidma_tx_segment), - 0); + /* Allocate the buffer descriptors. */ + chan->seg_v = dma_zalloc_coherent(chan->dev, + sizeof(*chan->seg_v) * + XILINX_DMA_NUM_DESCS, + &chan->seg_p, GFP_KERNEL); + if (!chan->seg_v) { + dev_err(chan->dev, + "unable to allocate channel %d descriptors\n", + chan->id); + return -ENOMEM; + } + + for (i = 0; i < XILINX_DMA_NUM_DESCS; i++) { + chan->seg_v[i].hw.next_desc = + lower_32_bits(chan->seg_p + sizeof(*chan->seg_v) * + ((i + 1) % XILINX_DMA_NUM_DESCS)); + chan->seg_v[i].hw.next_desc_msb = + upper_32_bits(chan->seg_p + sizeof(*chan->seg_v) * + ((i + 1) % XILINX_DMA_NUM_DESCS)); + chan->seg_v[i].phys = chan->seg_p + + sizeof(*chan->seg_v) * i; + list_add_tail(&chan->seg_v[i].node, + &chan->free_seg_list); + } } else if (chan->xdev->dma_config->dmatype == XDMA_TYPE_CDMA) { chan->desc_pool = dma_pool_create("xilinx_cdma_desc_pool", chan->dev, @@ -834,7 +887,8 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan) 0); } - if (!chan->desc_pool) { + if (!chan->desc_pool && + (chan->xdev->dma_config->dmatype != XDMA_TYPE_AXIDMA)) { dev_err(chan->dev, "unable to allocate channel %d descriptor pool\n", chan->id); @@ -843,22 +897,20 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan) if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) { /* - * For AXI DMA case after submitting a pending_list, keep - * an extra segment allocated so that the "next descriptor" - * pointer on the tail descriptor always points to a - * valid descriptor, even when paused after reaching taildesc. - * This way, it is possible to issue additional - * transfers without halting and restarting the channel. - */ - chan->seg_v = xilinx_axidma_alloc_tx_segment(chan); - - /* * For cyclic DMA mode we need to program the tail Descriptor * register with a value which is not a part of the BD chain * so allocating a desc segment during channel allocation for * programming tail descriptor. */ - chan->cyclic_seg_v = xilinx_axidma_alloc_tx_segment(chan); + chan->cyclic_seg_v = dma_zalloc_coherent(chan->dev, + sizeof(*chan->cyclic_seg_v), + &chan->cyclic_seg_p, GFP_KERNEL); + if (!chan->cyclic_seg_v) { + dev_err(chan->dev, + "unable to allocate desc segment for cyclic DMA\n"); + return -ENOMEM; + } + chan->cyclic_seg_v->phys = chan->cyclic_seg_p; } dma_cookie_init(dchan); @@ -1198,7 +1250,7 @@ static void xilinx_cdma_start_transfer(struct xilinx_dma_chan *chan) static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan) { struct xilinx_dma_tx_descriptor *head_desc, *tail_desc; - struct xilinx_axidma_tx_segment *tail_segment, *old_head, *new_head; + struct xilinx_axidma_tx_segment *tail_segment; u32 reg; if (chan->err) @@ -1217,21 +1269,6 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan) tail_segment = list_last_entry(&tail_desc->segments, struct xilinx_axidma_tx_segment, node); - if (chan->has_sg && !chan->xdev->mcdma) { - old_head = list_first_entry(&head_desc->segments, - struct xilinx_axidma_tx_segment, node); - new_head = chan->seg_v; - /* Copy Buffer Descriptor fields. */ - new_head->hw = old_head->hw; - - /* Swap and save new reserve */ - list_replace_init(&old_head->node, &new_head->node); - chan->seg_v = old_head; - - tail_segment->hw.next_desc = chan->seg_v->phys; - head_desc->async_tx.phys = new_head->phys; - } - reg = dma_ctrl_read(chan, XILINX_DMA_REG_DMACR); if (chan->desc_pendingcount <= XILINX_DMA_COALESCE_MAX) { @@ -1729,7 +1766,7 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg( { struct xilinx_dma_chan *chan = to_xilinx_chan(dchan); struct xilinx_dma_tx_descriptor *desc; - struct xilinx_axidma_tx_segment *segment = NULL, *prev = NULL; + struct xilinx_axidma_tx_segment *segment = NULL; u32 *app_w = (u32 *)context; struct scatterlist *sg; size_t copy; @@ -1780,10 +1817,6 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg( XILINX_DMA_NUM_APP_WORDS); } - if (prev) - prev->hw.next_desc = segment->phys; - - prev = segment; sg_used += copy; /* @@ -1797,7 +1830,6 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg( segment = list_first_entry(&desc->segments, struct xilinx_axidma_tx_segment, node); desc->async_tx.phys = segment->phys; - prev->hw.next_desc = segment->phys; /* For the last DMA_MEM_TO_DEV transfer, set EOP */ if (chan->direction == DMA_MEM_TO_DEV) { @@ -2341,6 +2373,7 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev, INIT_LIST_HEAD(&chan->pending_list); INIT_LIST_HEAD(&chan->done_list); INIT_LIST_HEAD(&chan->active_list); + INIT_LIST_HEAD(&chan->free_seg_list); /* Retrieve the channel properties from the device tree */ has_dre = of_property_read_bool(node, "xlnx,include-dre");
When driver is handling AXI DMA SoftIP When user submits multiple descriptors back to back on the S2MM(recv) side with the current driver flow the last buffer descriptor next bd points to a invalid location resulting the invalid data or errors in the DMA engine. This patch fixes this issue by creating a BD Chain during channel allocation itself and use those BD's. Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com> --- Changes for v5: ---> None. Changes for v4: ---> None. Changes for v3: ---> None. Changes for v2: ---> None. drivers/dma/xilinx/xilinx_dma.c | 133 +++++++++++++++++++++++++--------------- 1 file changed, 83 insertions(+), 50 deletions(-)