| Message ID | 20180913145256.12261-1-wsa+renesas@sang-engineering.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | [RFC] dmaengine: rcar-dmac: set scatter/gather max segment size | expand |
Hi Wolfram, On Thu, Sep 13, 2018 at 4:53 PM Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > Fix warning when running with CONFIG_DMA_API_DEBUG_SG=y by allocating a > device_dma_parameters structure and filling in the max segment size. > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > --- > > According to this discussion [1], this is the intended way of setting > dma_parms. I am not 100% sure about the value, though. I took the maximum value > of the DMA Transfer Count Registers which is 16M. I'd think the real maximum > needs to be multiplied with the data length which is varying, though? Geert, > what do you think? > > [1] https://www.spinics.net/lists/iommu/msg29861.html > > drivers/dma/sh/rcar-dmac.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c > index 48ee35e2bce6..bb3631f6c41c 100644 > --- a/drivers/dma/sh/rcar-dmac.c > +++ b/drivers/dma/sh/rcar-dmac.c > @@ -198,6 +198,7 @@ struct rcar_dmac { > struct dma_device engine; > struct device *dev; > void __iomem *iomem; > + struct device_dma_parameters parms; > > unsigned int n_channels; > struct rcar_dmac_chan *channels; > @@ -1792,6 +1793,8 @@ static int rcar_dmac_probe(struct platform_device *pdev) > > dmac->dev = &pdev->dev; > platform_set_drvdata(pdev, dmac); > + dmac->dev->dma_parms = &dmac->parms; > + dma_set_max_seg_size(dmac->dev, 0x01000000); That is one too much, cfr. drivers/dma/sh/rcar-dmac.c:#define RCAR_DMATCR_MASK 0x00ffffff And commit d716d9b702bb759d ("dmaengine: rcar-dmac: fix max_chunk_size for R-Car Gen3"), which did: - max_chunk_size = (RCAR_DMATCR_MASK + 1) << desc->xfer_shift; + max_chunk_size = RCAR_DMATCR_MASK << desc->xfer_shift; And yes, this depends on the data length, which is not known at probe time. Of course, dma_set_max_seg_size(dmac->dev, RCAR_DMATCR_MASK) should be safe, albeit (slightly) suboptimal. But I doubt anyone is really doing such large transfers (up to (16 Mi - 1) * 64 bytes!). > dma_set_mask_and_coherent(dmac->dev, DMA_BIT_MASK(40)); > > ret = rcar_dmac_parse_of(&pdev->dev, dmac); Gr{oetje,eeting}s, Geert
Hi Geert, > > + dmac->dev->dma_parms = &dmac->parms; > > + dma_set_max_seg_size(dmac->dev, 0x01000000); > > That is one too much, cfr. > > drivers/dma/sh/rcar-dmac.c:#define RCAR_DMATCR_MASK 0x00ffffff I see. Will fix and send an updated patch. Nice to see that I was not totally wrong but just off-by-one. > And yes, this depends on the data length, which is not known at probe time. > Of course, dma_set_max_seg_size(dmac->dev, RCAR_DMATCR_MASK) > should be safe, albeit (slightly) suboptimal. True. But still *way* better than the current default of 64K. Thanks for the heads up! Regards, Wolfram
diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c index 48ee35e2bce6..bb3631f6c41c 100644 --- a/drivers/dma/sh/rcar-dmac.c +++ b/drivers/dma/sh/rcar-dmac.c @@ -198,6 +198,7 @@ struct rcar_dmac { struct dma_device engine; struct device *dev; void __iomem *iomem; + struct device_dma_parameters parms; unsigned int n_channels; struct rcar_dmac_chan *channels; @@ -1792,6 +1793,8 @@ static int rcar_dmac_probe(struct platform_device *pdev) dmac->dev = &pdev->dev; platform_set_drvdata(pdev, dmac); + dmac->dev->dma_parms = &dmac->parms; + dma_set_max_seg_size(dmac->dev, 0x01000000); dma_set_mask_and_coherent(dmac->dev, DMA_BIT_MASK(40)); ret = rcar_dmac_parse_of(&pdev->dev, dmac);
Fix warning when running with CONFIG_DMA_API_DEBUG_SG=y by allocating a device_dma_parameters structure and filling in the max segment size. Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> --- According to this discussion [1], this is the intended way of setting dma_parms. I am not 100% sure about the value, though. I took the maximum value of the DMA Transfer Count Registers which is 16M. I'd think the real maximum needs to be multiplied with the data length which is varying, though? Geert, what do you think? [1] https://www.spinics.net/lists/iommu/msg29861.html drivers/dma/sh/rcar-dmac.c | 3 +++ 1 file changed, 3 insertions(+)