| Message ID | 1348152226-13588-9-git-send-email-mporter@ti.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Thu, Sep 20, 2012 at 8:13 PM, Matt Porter <mporter@ti.com> wrote: > The EDMA DMAC has a hardware limitation that prevents supporting > scatter gather lists with any number of segments. Since the EDMA > DMA Engine driver sets the maximum segments to 16, we do the > same. > > Note: this can be removed once the DMA Engine API supports an > API to query the DMAC's segment limitations. > I wouldn't want to bind the properties of EDMA to omap_hsmmc as this patch suggests. Why don't we have a max_segs property, which when explicitly specified in DT, will override the default ?
On Fri, Sep 21, 2012 at 10:45 PM, S, Venkatraman <svenkatr@ti.com> wrote: > On Thu, Sep 20, 2012 at 8:13 PM, Matt Porter <mporter@ti.com> wrote: >> The EDMA DMAC has a hardware limitation that prevents supporting >> scatter gather lists with any number of segments. Since the EDMA >> DMA Engine driver sets the maximum segments to 16, we do the >> same. >> >> Note: this can be removed once the DMA Engine API supports an >> API to query the DMAC's segment limitations. >> > > I wouldn't want to bind the properties of EDMA to omap_hsmmc as this patch > suggests. Why don't we have a max_segs property, which when explicitly specified > in DT, will override the default ? If you are adventurous, this can be a generic mmc DT binding instead of restricting it to OMAP.
On Fri, Sep 21, 2012 at 10:47:30PM +0530, S, Venkatraman wrote: > On Fri, Sep 21, 2012 at 10:45 PM, S, Venkatraman <svenkatr@ti.com> wrote: > > On Thu, Sep 20, 2012 at 8:13 PM, Matt Porter <mporter@ti.com> wrote: > >> The EDMA DMAC has a hardware limitation that prevents supporting > >> scatter gather lists with any number of segments. Since the EDMA > >> DMA Engine driver sets the maximum segments to 16, we do the > >> same. > >> > >> Note: this can be removed once the DMA Engine API supports an > >> API to query the DMAC's segment limitations. > >> > > > > I wouldn't want to bind the properties of EDMA to omap_hsmmc as this patch > > suggests. Why don't we have a max_segs property, which when explicitly specified > > in DT, will override the default ? > > If you are adventurous, this can be a generic mmc DT binding instead > of restricting it to OMAP. I say if it's a limitation in the DMAC, then DMAC's driver should handle it, no ? Meaning that in this case you would copy from one multi-segment sg into a one-segment sg and when transfer is complete, before calling user's callback, copy data the other way around (?)
On Fri, Sep 21, 2012 at 10:48 PM, Felipe Balbi <balbi@ti.com> wrote: > On Fri, Sep 21, 2012 at 10:47:30PM +0530, S, Venkatraman wrote: >> On Fri, Sep 21, 2012 at 10:45 PM, S, Venkatraman <svenkatr@ti.com> wrote: >> > On Thu, Sep 20, 2012 at 8:13 PM, Matt Porter <mporter@ti.com> wrote: >> >> The EDMA DMAC has a hardware limitation that prevents supporting >> >> scatter gather lists with any number of segments. Since the EDMA >> >> DMA Engine driver sets the maximum segments to 16, we do the >> >> same. >> >> >> >> Note: this can be removed once the DMA Engine API supports an >> >> API to query the DMAC's segment limitations. >> >> >> > >> > I wouldn't want to bind the properties of EDMA to omap_hsmmc as this patch >> > suggests. Why don't we have a max_segs property, which when explicitly specified >> > in DT, will override the default ? >> >> If you are adventurous, this can be a generic mmc DT binding instead >> of restricting it to OMAP. > > I say if it's a limitation in the DMAC, then DMAC's driver should handle > it, no ? Meaning that in this case you would copy from one multi-segment > sg into a one-segment sg and when transfer is complete, before calling > user's callback, copy data the other way around (?) > Right ! So even if the property is defined for MMC, Matt will end up coding the limitation into every peripheral driver that uses EMAC, which doesn't scale. Your solution is better.
On Fri, Sep 21, 2012 at 10:47:30PM +0530, S, Venkatraman wrote: > On Fri, Sep 21, 2012 at 10:45 PM, S, Venkatraman <svenkatr@ti.com> wrote: > > On Thu, Sep 20, 2012 at 8:13 PM, Matt Porter <mporter@ti.com> wrote: > >> The EDMA DMAC has a hardware limitation that prevents supporting > >> scatter gather lists with any number of segments. Since the EDMA > >> DMA Engine driver sets the maximum segments to 16, we do the > >> same. > >> > >> Note: this can be removed once the DMA Engine API supports an > >> API to query the DMAC's segment limitations. > >> > > > > I wouldn't want to bind the properties of EDMA to omap_hsmmc as this patch > > suggests. Why don't we have a max_segs property, which when explicitly specified > > in DT, will override the default ? > > If you are adventurous, this can be a generic mmc DT binding instead > of restricting it to OMAP. I think it's bad practice to add something to a binding that is not part of the hardware definition for the device. In this case, the limitations comes strictly from the DMAC. As I noted, the proper fix for this is to have the DMA Engine API extended to allow querying the DMAC SG capabilities before setting up a DMA transfer. I brought this up previously in the thread where the actual EDMA dmaengine wrapper driver was discussed and Vinod indicated that he was open to dmaengine providing this information. It makes sense as DMA Engine should tell the slave driver everything it needs to know to set up a transfer...this is just a missing piece right now. FWIW, we have this issue in davinci_mmc.c as well...it just was already hardcoded with a value to satisfy the EDMA DMAC. However, I'd like to see that go away as well. -Matt
On Fri, Sep 21, 2012 at 10:45:29PM +0530, S, Venkatraman wrote: > On Thu, Sep 20, 2012 at 8:13 PM, Matt Porter <mporter@ti.com> wrote: > > The EDMA DMAC has a hardware limitation that prevents supporting > > scatter gather lists with any number of segments. Since the EDMA > > DMA Engine driver sets the maximum segments to 16, we do the > > same. > > > > Note: this can be removed once the DMA Engine API supports an > > API to query the DMAC's segment limitations. > > > > I wouldn't want to bind the properties of EDMA to omap_hsmmc as this patch > suggests. Why don't we have a max_segs property, which when explicitly specified > in DT, will override the default ? Why not have a generic way that DMA engine can export these kinds of properties?
On Fri, Sep 21, 2012 at 08:18:41PM +0300, Felipe Balbi wrote: > On Fri, Sep 21, 2012 at 10:47:30PM +0530, S, Venkatraman wrote: > > On Fri, Sep 21, 2012 at 10:45 PM, S, Venkatraman <svenkatr@ti.com> wrote: > > > On Thu, Sep 20, 2012 at 8:13 PM, Matt Porter <mporter@ti.com> wrote: > > >> The EDMA DMAC has a hardware limitation that prevents supporting > > >> scatter gather lists with any number of segments. Since the EDMA > > >> DMA Engine driver sets the maximum segments to 16, we do the > > >> same. > > >> > > >> Note: this can be removed once the DMA Engine API supports an > > >> API to query the DMAC's segment limitations. > > >> > > > > > > I wouldn't want to bind the properties of EDMA to omap_hsmmc as this patch > > > suggests. Why don't we have a max_segs property, which when explicitly specified > > > in DT, will override the default ? > > > > If you are adventurous, this can be a generic mmc DT binding instead > > of restricting it to OMAP. > > I say if it's a limitation in the DMAC, then DMAC's driver should handle > it, no ? Meaning that in this case you would copy from one multi-segment > sg into a one-segment sg and when transfer is complete, before calling > user's callback, copy data the other way around (?) With this DMAC, we would have to do a CPU-based copy or a series of smaller DMA-based 16 segment copies with completion interrupts in between. The reason the EDMA DMA Engine driver sets this limit is that we have a hardware limitation preventing setting up a large multi-segment transfer. The limitation is set by how many EDMA PaRaM slots are available (varies based on how the hwmod is instantiated) but on AM335x it's 256. You can't use all of those for just one slave device and so the EDMA dmaengine driver arbitrarily hardcodes (atm) 16 as the max any one channel can claim. Even if you could use all of them, it's common for an unrestricted scatter gather transfer to exceed even our best case hardware limitation. This is a case where asking the DMA Engine driver to handle any length SG is going to result in a big peformance hit, since the MMC subsystem provides this hook for a reason, we just need the proper DMA Engine API to find out how to set it. So I guess I'm going to need to write up an API proposal unless Vinod has already been thinking about this... -Matt
On Fri, Sep 21, 2012 at 07:47:21PM +0100, Russell King wrote: > On Fri, Sep 21, 2012 at 10:45:29PM +0530, S, Venkatraman wrote: > > On Thu, Sep 20, 2012 at 8:13 PM, Matt Porter <mporter@ti.com> wrote: > > > The EDMA DMAC has a hardware limitation that prevents supporting > > > scatter gather lists with any number of segments. Since the EDMA > > > DMA Engine driver sets the maximum segments to 16, we do the > > > same. > > > > > > Note: this can be removed once the DMA Engine API supports an > > > API to query the DMAC's segment limitations. > > > > > > > I wouldn't want to bind the properties of EDMA to omap_hsmmc as this patch > > suggests. Why don't we have a max_segs property, which when explicitly specified > > in DT, will override the default ? > > Why not have a generic way that DMA engine can export these kinds of > properties? That's exactly what my note above is suggesting... Something along the lines of: struct slave_sg_caps { int max_segs; /* <0 is no limit */ } struct slave_sg_cap * dmaengine_get_slave_sg_caps(struct dma_chan *chan); I'm sure there are or will be other characteristics worth providing to slave drivers. -Matt
On Fri, 2012-09-21 at 19:47 +0100, Russell King - ARM Linux wrote: > On Fri, Sep 21, 2012 at 10:45:29PM +0530, S, Venkatraman wrote: > > On Thu, Sep 20, 2012 at 8:13 PM, Matt Porter <mporter@ti.com> wrote: > > > The EDMA DMAC has a hardware limitation that prevents supporting > > > scatter gather lists with any number of segments. Since the EDMA > > > DMA Engine driver sets the maximum segments to 16, we do the > > > same. > > > > > > Note: this can be removed once the DMA Engine API supports an > > > API to query the DMAC's segment limitations. > > > > > > > I wouldn't want to bind the properties of EDMA to omap_hsmmc as this patch > > suggests. Why don't we have a max_segs property, which when explicitly specified > > in DT, will override the default ? > > Why not have a generic way that DMA engine can export these kinds of > properties? We discussed this at KS. I was of opinion that DMA engine should export controller and channel capabilities as part of the channel it returns. Some folks had an opinion that they already know how to use controller so may not be very helpful, but if it is going to help (which I think), i have a patch for this :)
On Thu, Sep 27, 2012 at 03:11:08PM +0530, Vinod Koul wrote: > On Fri, 2012-09-21 at 19:47 +0100, Russell King - ARM Linux wrote: > > On Fri, Sep 21, 2012 at 10:45:29PM +0530, S, Venkatraman wrote: > > > On Thu, Sep 20, 2012 at 8:13 PM, Matt Porter <mporter@ti.com> wrote: > > > > The EDMA DMAC has a hardware limitation that prevents supporting > > > > scatter gather lists with any number of segments. Since the EDMA > > > > DMA Engine driver sets the maximum segments to 16, we do the > > > > same. > > > > > > > > Note: this can be removed once the DMA Engine API supports an > > > > API to query the DMAC's segment limitations. > > > > > > > > > > I wouldn't want to bind the properties of EDMA to omap_hsmmc as this patch > > > suggests. Why don't we have a max_segs property, which when explicitly specified > > > in DT, will override the default ? > > > > Why not have a generic way that DMA engine can export these kinds of > > properties? > We discussed this at KS. I was of opinion that DMA engine should export > controller and channel capabilities as part of the channel it returns. > > Some folks had an opinion that they already know how to use controller > so may not be very helpful, but if it is going to help (which I think), > i have a patch for this :) Anything you can show at this point? ;) I'd be happy to drop the half-hack for a real API. If not, I'm going to carry that to v2 atm. -Matt
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index c82d0ab..61b54ee 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1885,6 +1885,16 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev) * as we want. */ mmc->max_segs = 1024; + /* Eventually we should get our max_segs limitation for EDMA by + * querying the dmaengine API */ + if (pdev->dev.of_node) { + struct device_node *parent = pdev->dev.of_node->parent; + struct device_node *node; + node = of_find_node_by_name(parent, "edma"); + if (node) + mmc->max_segs = 16; + } + mmc->max_blk_size = 512; /* Block Length at max can be 1024 */ mmc->max_blk_count = 0xFFFF; /* No. of Blocks is 16 bits */ mmc->max_req_size = mmc->max_blk_size * mmc->max_blk_count;
The EDMA DMAC has a hardware limitation that prevents supporting scatter gather lists with any number of segments. Since the EDMA DMA Engine driver sets the maximum segments to 16, we do the same. Note: this can be removed once the DMA Engine API supports an API to query the DMAC's segment limitations. Signed-off-by: Matt Porter <mporter@ti.com> --- drivers/mmc/host/omap_hsmmc.c | 10 ++++++++++ 1 file changed, 10 insertions(+)