| Message ID | 1357844826-30746-3-git-send-email-mporter@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jan 10, 2013 at 02:07:05PM -0500, Matt Porter wrote: > Implement device_channel_caps(). > > EDMA has a finite set of PaRAM slots available for linking > a multi-segment SG transfer. In order to prevent any one > channel from consuming all PaRAM slots to fulfill a large SG > transfer, the driver reports a static per-channel max number > of SG segments it will handle. > > The maximum size of SG segment is limited by the slave config > maxburst and addr_width for the channel in question. These values > are used from the current channel config to calculate and return > the max segment length cap. > > Signed-off-by: Matt Porter <mporter@ti.com> > --- > drivers/dma/edma.c | 27 +++++++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c > index 82c8672..fc4b9db 100644 > --- a/drivers/dma/edma.c > +++ b/drivers/dma/edma.c > @@ -70,6 +70,7 @@ struct edma_chan { > bool alloced; > int slot[EDMA_MAX_SLOTS]; > struct dma_slave_config cfg; > + struct dmaengine_chan_caps caps; > }; > > struct edma_cc { > @@ -462,6 +463,28 @@ static void edma_issue_pending(struct dma_chan *chan) > spin_unlock_irqrestore(&echan->vchan.lock, flags); > } > > +static struct dmaengine_chan_caps > +*edma_get_channel_caps(struct dma_chan *chan, enum dma_transfer_direction dir) > +{ > + struct edma_chan *echan; > + enum dma_slave_buswidth width = 0; > + u32 burst = 0; > + > + if (chan) { I think this check is redundant. > + echan = to_edma_chan(chan); > + if (dir == DMA_MEM_TO_DEV) { > + width = echan->cfg.dst_addr_width; > + burst = echan->cfg.dst_maxburst; Per you API example burst and width is not set yet, so this doesn't make sense > + } else if (dir == DMA_DEV_TO_MEM) { > + width = echan->cfg.src_addr_width; > + burst = echan->cfg.src_maxburst; > + } > + echan->caps.seg_len = (SZ_64K - 1) * width * burst; Also the defination of API is max, above computation doesn't make sense to me! -- ~Vinod -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Jan 20, 2013 at 01:03:21PM +0000, Vinod Koul wrote: > On Thu, Jan 10, 2013 at 02:07:05PM -0500, Matt Porter wrote: > > Implement device_channel_caps(). > > > > EDMA has a finite set of PaRAM slots available for linking > > a multi-segment SG transfer. In order to prevent any one > > channel from consuming all PaRAM slots to fulfill a large SG > > transfer, the driver reports a static per-channel max number > > of SG segments it will handle. > > > > The maximum size of SG segment is limited by the slave config > > maxburst and addr_width for the channel in question. These values > > are used from the current channel config to calculate and return > > the max segment length cap. > > > > Signed-off-by: Matt Porter <mporter@ti.com> > > --- > > drivers/dma/edma.c | 27 +++++++++++++++++++++++++++ > > 1 file changed, 27 insertions(+) > > > > diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c > > index 82c8672..fc4b9db 100644 > > --- a/drivers/dma/edma.c > > +++ b/drivers/dma/edma.c > > @@ -70,6 +70,7 @@ struct edma_chan { > > bool alloced; > > int slot[EDMA_MAX_SLOTS]; > > struct dma_slave_config cfg; > > + struct dmaengine_chan_caps caps; > > }; > > > > struct edma_cc { > > @@ -462,6 +463,28 @@ static void edma_issue_pending(struct dma_chan *chan) > > spin_unlock_irqrestore(&echan->vchan.lock, flags); > > } > > > > +static struct dmaengine_chan_caps > > +*edma_get_channel_caps(struct dma_chan *chan, enum dma_transfer_direction dir) > > +{ > > + struct edma_chan *echan; > > + enum dma_slave_buswidth width = 0; > > + u32 burst = 0; > > + > > + if (chan) { > I think this check is redundant. Yes, will remove. > > + echan = to_edma_chan(chan); > > + if (dir == DMA_MEM_TO_DEV) { > > + width = echan->cfg.dst_addr_width; > > + burst = echan->cfg.dst_maxburst; > Per you API example burst and width is not set yet, so this doesn't make sense The explanation in the cover letter mentions that dmaengine_slave_config() is required to be called prior to dmaengine_get_channel_caps(). If we switch to the alternative API, then that would go away including the dependency on direction. > > + } else if (dir == DMA_DEV_TO_MEM) { > > + width = echan->cfg.src_addr_width; > > + burst = echan->cfg.src_maxburst; > > + } > > + echan->caps.seg_len = (SZ_64K - 1) * width * burst; > Also the defination of API is max, above computation doesn't make sense to me! Ok, so in this case, the slave driver has informed the dmaengine driver that the max burst for the channel is foo. That's in units of addr_width. On the EDMA DMAC, we program burst transfers by setting ACNT to our per-transfer width (FIFO width in the slave SG case we are covering here) then BCNT gets the maxburst setting. We then have a CCNT field for EDMA that has a limit of SZ_64K-1 transfers. Thus, if a slave driver tells us the maxburst for the channel is foo and our width is bar, the maximum size of an SG segment in that configuration is: (SZ_64K - 1) * bar * foo. -Matt -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Jan 20, 2013 at 11:51:08AM -0500, Matt Porter wrote: > The explanation in the cover letter mentions that dmaengine_slave_config() is > required to be called prior to dmaengine_get_channel_caps(). If we > switch to the alternative API, then that would go away including the > dependency on direction. Nope you got that wrong! -- ~Vinod -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 21, 2013 at 03:16:32AM +0000, Vinod Koul wrote: > On Sun, Jan 20, 2013 at 11:51:08AM -0500, Matt Porter wrote: > > The explanation in the cover letter mentions that dmaengine_slave_config() is > > required to be called prior to dmaengine_get_channel_caps(). If we > > switch to the alternative API, then that would go away including the > > dependency on direction. > Nope you got that wrong! :) Yes, dropped the ball there, should have been for the api to make sense as implemented: 1. Allocate a DMA slave channel 2. Set slave and controller specific parameters 2a. [Optionally] Get channel capabilities 3. Get a descriptor for transaction 4. Submit the transaction 5. Issue pending requests and wait for callback notification FWIW, the implementation example in the davinci mmc client driver shows proper use as in the correct documentation above. -Matt -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" 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/edma.c b/drivers/dma/edma.c index 82c8672..fc4b9db 100644 --- a/drivers/dma/edma.c +++ b/drivers/dma/edma.c @@ -70,6 +70,7 @@ struct edma_chan { bool alloced; int slot[EDMA_MAX_SLOTS]; struct dma_slave_config cfg; + struct dmaengine_chan_caps caps; }; struct edma_cc { @@ -462,6 +463,28 @@ static void edma_issue_pending(struct dma_chan *chan) spin_unlock_irqrestore(&echan->vchan.lock, flags); } +static struct dmaengine_chan_caps +*edma_get_channel_caps(struct dma_chan *chan, enum dma_transfer_direction dir) +{ + struct edma_chan *echan; + enum dma_slave_buswidth width = 0; + u32 burst = 0; + + if (chan) { + echan = to_edma_chan(chan); + if (dir == DMA_MEM_TO_DEV) { + width = echan->cfg.dst_addr_width; + burst = echan->cfg.dst_maxburst; + } else if (dir == DMA_DEV_TO_MEM) { + width = echan->cfg.src_addr_width; + burst = echan->cfg.src_maxburst; + } + echan->caps.seg_len = (SZ_64K - 1) * width * burst; + return &echan->caps; + } + return NULL; +} + static size_t edma_desc_size(struct edma_desc *edesc) { int i; @@ -521,6 +544,9 @@ static void __init edma_chan_init(struct edma_cc *ecc, echan->ch_num = EDMA_CTLR_CHAN(ecc->ctlr, i); echan->ecc = ecc; echan->vchan.desc_free = edma_desc_free; + dma_cap_set(DMA_SLAVE, echan->caps.cap_mask); + dma_cap_set(DMA_SG, echan->caps.cap_mask); + echan->caps.seg_nr = MAX_NR_SG; vchan_init(&echan->vchan, dma); @@ -537,6 +563,7 @@ static void edma_dma_init(struct edma_cc *ecc, struct dma_device *dma, dma->device_alloc_chan_resources = edma_alloc_chan_resources; dma->device_free_chan_resources = edma_free_chan_resources; dma->device_issue_pending = edma_issue_pending; + dma->device_channel_caps = edma_get_channel_caps; dma->device_tx_status = edma_tx_status; dma->device_control = edma_control; dma->dev = dev;
Implement device_channel_caps(). EDMA has a finite set of PaRAM slots available for linking a multi-segment SG transfer. In order to prevent any one channel from consuming all PaRAM slots to fulfill a large SG transfer, the driver reports a static per-channel max number of SG segments it will handle. The maximum size of SG segment is limited by the slave config maxburst and addr_width for the channel in question. These values are used from the current channel config to calculate and return the max segment length cap. Signed-off-by: Matt Porter <mporter@ti.com> --- drivers/dma/edma.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)