| Message ID | 20200528222401.26941-1-Sergey.Semin@baikalelectronics.ru |
|---|---|
| Headers | show |
| Series | dmaengine: dw: Take Baikal-T1 SoC DW DMAC peculiarities into account | expand |
On Fri, May 29, 2020 at 01:23:59AM +0300, Serge Semin wrote: > According to the DW APB DMAC data book the minimum burst transaction > length is 1 and it's true for any version of the controller since > isn't parametrised in the coreAssembler so can't be changed at the > IP-core synthesis stage. Let's initialise the min_burst member of the > DMA controller descriptor so the DMA clients could use it to properly > optimize the DMA requests. > @@ -1229,6 +1229,7 @@ int do_dma_probe(struct dw_dma_chip *chip) > dw->dma.device_issue_pending = dwc_issue_pending; > > /* DMA capabilities */ > + dw->dma.min_burst = 1; Perhaps then relaxed maximum, like dw->dma.max_burst = 256; (channels will update this) ? > dw->dma.src_addr_widths = DW_DMA_BUSWIDTHS; > dw->dma.dst_addr_widths = DW_DMA_BUSWIDTHS; > dw->dma.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV) | > -- > 2.26.2 >
On Fri, May 29, 2020 at 01:24:00AM +0300, Serge Semin wrote: > IP core of the DW DMA controller may be synthesized with different > max burst length of the transfers per each channel. According to Synopsis > having the fixed maximum burst transactions length may provide some > performance gain. At the same time setting up the source and destination > multi size exceeding the max burst length limitation may cause a serious > problems. In our case the DMA transaction just hangs up. In order to fix > this lets introduce the max burst length platform config of the DW DMA > controller device and don't let the DMA channels configuration code > exceed the burst length hardware limitation. > > Note the maximum burst length parameter can be detected either in runtime > from the DWC parameter registers or from the dedicated DT property. > Depending on the IP core configuration the maximum value can vary from > channel to channel so by overriding the channel slave max_burst capability > we make sure a DMA consumer will get the channel-specific max burst > length. LGTM, but consider comment to previous patch (in that case perhaps definition of min and max should be moved there). Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: linux-mips@vger.kernel.org > Cc: devicetree@vger.kernel.org > > --- > > Changelog v2: > - Rearrange SoBs. > - Discard dwc_get_maxburst() accessor. It's enough to have a clamping > guard against exceeding the hardware max burst limitation. > > Changelog v3: > - Override the slave channel max_burst capability instead of calculating > the minimum value of max burst lengths and setting the DMA-device > generic capability. > > Changelog v4: > - Clamp the dst and src burst lengths in the generic dwc_config() method > instead of doing that in the encode_maxburst() callback. > - Define max_burst with u32 type in struct dw_dma_platform_data. > - Perform of_property_read_u32_array() directly into the platform data > max_burst member. > --- > drivers/dma/dw/core.c | 10 ++++++++++ > drivers/dma/dw/of.c | 5 +++++ > drivers/dma/dw/regs.h | 2 ++ > include/linux/platform_data/dma-dw.h | 4 ++++ > 4 files changed, 21 insertions(+) > > diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c > index a8cebb1dbb68..60ef779fc5e0 100644 > --- a/drivers/dma/dw/core.c > +++ b/drivers/dma/dw/core.c > @@ -791,6 +791,11 @@ static int dwc_config(struct dma_chan *chan, struct dma_slave_config *sconfig) > > memcpy(&dwc->dma_sconfig, sconfig, sizeof(*sconfig)); > > + dwc->dma_sconfig.src_maxburst = > + clamp(dwc->dma_sconfig.src_maxburst, 0U, dwc->max_burst); > + dwc->dma_sconfig.dst_maxburst = > + clamp(dwc->dma_sconfig.dst_maxburst, 0U, dwc->max_burst); > + > dw->encode_maxburst(dwc, &dwc->dma_sconfig.src_maxburst); > dw->encode_maxburst(dwc, &dwc->dma_sconfig.dst_maxburst); > > @@ -1051,7 +1056,9 @@ static void dwc_free_chan_resources(struct dma_chan *chan) > > static void dwc_caps(struct dma_chan *chan, struct dma_slave_caps *caps) > { > + struct dw_dma_chan *dwc = to_dw_dma_chan(chan); > > + caps->max_burst = dwc->max_burst; > } > > int do_dma_probe(struct dw_dma_chip *chip) > @@ -1194,9 +1201,12 @@ int do_dma_probe(struct dw_dma_chip *chip) > dwc->nollp = > (dwc_params >> DWC_PARAMS_MBLK_EN & 0x1) == 0 || > (dwc_params >> DWC_PARAMS_HC_LLP & 0x1) == 1; > + dwc->max_burst = > + (0x4 << (dwc_params >> DWC_PARAMS_MSIZE & 0x7)); > } else { > dwc->block_size = pdata->block_size; > dwc->nollp = !pdata->multi_block[i]; > + dwc->max_burst = pdata->max_burst[i] ?: DW_DMA_MAX_BURST; > } > } > > diff --git a/drivers/dma/dw/of.c b/drivers/dma/dw/of.c > index 9e27831dee32..1474b3817ef4 100644 > --- a/drivers/dma/dw/of.c > +++ b/drivers/dma/dw/of.c > @@ -98,6 +98,11 @@ struct dw_dma_platform_data *dw_dma_parse_dt(struct platform_device *pdev) > pdata->multi_block[tmp] = 1; > } > > + if (of_property_read_u32_array(np, "snps,max-burst-len", pdata->max_burst, > + nr_channels)) { > + memset32(pdata->max_burst, DW_DMA_MAX_BURST, nr_channels); > + } > + > if (!of_property_read_u32(np, "snps,dma-protection-control", &tmp)) { > if (tmp > CHAN_PROTCTL_MASK) > return NULL; > diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h > index 1ab840b06e79..76654bd13c1a 100644 > --- a/drivers/dma/dw/regs.h > +++ b/drivers/dma/dw/regs.h > @@ -126,6 +126,7 @@ struct dw_dma_regs { > /* Bitfields in DWC_PARAMS */ > #define DWC_PARAMS_MBLK_EN 11 /* multi block transfer */ > #define DWC_PARAMS_HC_LLP 13 /* set LLP register to zero */ > +#define DWC_PARAMS_MSIZE 16 /* max group transaction size */ > > /* bursts size */ > enum dw_dma_msize { > @@ -284,6 +285,7 @@ struct dw_dma_chan { > /* hardware configuration */ > unsigned int block_size; > bool nollp; > + u32 max_burst; > > /* custom slave configuration */ > struct dw_dma_slave dws; > diff --git a/include/linux/platform_data/dma-dw.h b/include/linux/platform_data/dma-dw.h > index f3eaf9ec00a1..29c484da2979 100644 > --- a/include/linux/platform_data/dma-dw.h > +++ b/include/linux/platform_data/dma-dw.h > @@ -12,6 +12,7 @@ > > #define DW_DMA_MAX_NR_MASTERS 4 > #define DW_DMA_MAX_NR_CHANNELS 8 > +#define DW_DMA_MAX_BURST 256 > > /** > * struct dw_dma_slave - Controller-specific information about a slave > @@ -42,6 +43,8 @@ struct dw_dma_slave { > * @data_width: Maximum data width supported by hardware per AHB master > * (in bytes, power of 2) > * @multi_block: Multi block transfers supported by hardware per channel. > + * @max_burst: Maximum value of burst transaction size supported by hardware > + * per channel (in units of CTL.SRC_TR_WIDTH/CTL.DST_TR_WIDTH). > * @protctl: Protection control signals setting per channel. > */ > struct dw_dma_platform_data { > @@ -56,6 +59,7 @@ struct dw_dma_platform_data { > unsigned char nr_masters; > unsigned char data_width[DW_DMA_MAX_NR_MASTERS]; > unsigned char multi_block[DW_DMA_MAX_NR_CHANNELS]; > + u32 max_burst[DW_DMA_MAX_NR_CHANNELS]; > #define CHAN_PROTCTL_PRIVILEGED BIT(0) > #define CHAN_PROTCTL_BUFFERABLE BIT(1) > #define CHAN_PROTCTL_CACHEABLE BIT(2) > -- > 2.26.2 >
On Fri, May 29, 2020 at 01:24:01AM +0300, Serge Semin wrote: > Multi-block support provides a way to map the kernel-specific SG-table so > the DW DMA device would handle it as a whole instead of handling the > SG-list items or so called LLP block items one by one. So if true LLP > list isn't supported by the DW DMA engine, then soft-LLP mode will be > utilized to load and execute each LLP-block one by one. The soft-LLP mode > of the DMA transactions execution might not work well for some DMA > consumers like SPI due to its Tx and Rx buffers inter-dependency. Let's > initialize the max_sg_nents DMA channels capability based on the nollp > flag state. If it's true, no hardware accelerated LLP is available and > max_sg_nents should be set with 1, which means that the DMA engine > can handle only a single SG list entry at a time. If noLLP is set to > false, then hardware accelerated LLP is supported and the DMA engine > can handle infinite number of SG entries in a single DMA transaction. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: linux-mips@vger.kernel.org > Cc: devicetree@vger.kernel.org > > --- > > Changelog v3: > - This is a new patch created as a result of the discussion with Vinud and > Andy in the framework of DW DMA burst and LLP capabilities. > > Changelog v4: > - Use explicit if-else statement when assigning the max_sg_nents field. > --- > drivers/dma/dw/core.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c > index 60ef779fc5e0..b76eee75fde8 100644 > --- a/drivers/dma/dw/core.c > +++ b/drivers/dma/dw/core.c > @@ -1059,6 +1059,18 @@ static void dwc_caps(struct dma_chan *chan, struct dma_slave_caps *caps) > struct dw_dma_chan *dwc = to_dw_dma_chan(chan); > > caps->max_burst = dwc->max_burst; > + > + /* > + * It might be crucial for some devices to have the hardware > + * accelerated multi-block transfers supported, aka LLPs in DW DMAC > + * notation. So if LLPs are supported then max_sg_nents is set to > + * zero which means unlimited number of SG entries can be handled in a > + * single DMA transaction, otherwise it's just one SG entry. > + */ > + if (dwc->nollp) > + caps->max_sg_nents = 1; > + else > + caps->max_sg_nents = 0; > } > > int do_dma_probe(struct dw_dma_chip *chip) > -- > 2.26.2 >
On Fri, May 29, 2020 at 01:25:15PM +0300, Andy Shevchenko wrote: > On Fri, May 29, 2020 at 01:23:59AM +0300, Serge Semin wrote: > > According to the DW APB DMAC data book the minimum burst transaction > > length is 1 and it's true for any version of the controller since > > isn't parametrised in the coreAssembler so can't be changed at the > > IP-core synthesis stage. Let's initialise the min_burst member of the > > DMA controller descriptor so the DMA clients could use it to properly > > optimize the DMA requests. ... > > /* DMA capabilities */ > > > + dw->dma.min_burst = 1; > > Perhaps then relaxed maximum, like > > dw->dma.max_burst = 256; > > (channels will update this) > > ? And forgot to mention that perhaps we need a definitions for both. > > dw->dma.src_addr_widths = DW_DMA_BUSWIDTHS; > > dw->dma.dst_addr_widths = DW_DMA_BUSWIDTHS;
On Fri, May 29, 2020 at 01:41:19PM +0300, Serge Semin wrote: > On Fri, May 29, 2020 at 01:29:02PM +0300, Andy Shevchenko wrote: > > On Fri, May 29, 2020 at 01:25:15PM +0300, Andy Shevchenko wrote: > > > On Fri, May 29, 2020 at 01:23:59AM +0300, Serge Semin wrote: ... > > > > /* DMA capabilities */ > > > > + dw->dma.min_burst = 1; > > > > > > Perhaps then relaxed maximum, like > > > > > > dw->dma.max_burst = 256; > > > > > > (channels will update this) > > > > > > ? > > > And forgot to mention that perhaps we need a definitions for both. > > By "definitions for both" do you mean a macro with corresponding parameter > definition like it's done for the max burst length in the next patch? > Something like this: > --- include/linux/platform_data/dma-dw.h > +++ include/linux/platform_data/dma-dw.h > +#define DW_DMA_MIN_BURST 1 > +#define DW_DMA_MAX_BURST 256 > > ? Yes!
On Fri, May 29, 2020 at 01:50:09PM +0300, Andy Shevchenko wrote: > On Fri, May 29, 2020 at 01:41:19PM +0300, Serge Semin wrote: > > On Fri, May 29, 2020 at 01:29:02PM +0300, Andy Shevchenko wrote: > > > On Fri, May 29, 2020 at 01:25:15PM +0300, Andy Shevchenko wrote: > > > > On Fri, May 29, 2020 at 01:23:59AM +0300, Serge Semin wrote: > > ... > > > > > > /* DMA capabilities */ > > > > > + dw->dma.min_burst = 1; > > > > > > > > Perhaps then relaxed maximum, like > > > > > > > > dw->dma.max_burst = 256; > > > > > > > > (channels will update this) > > > > > > > > ? > > > > > And forgot to mention that perhaps we need a definitions for both. > > > > By "definitions for both" do you mean a macro with corresponding parameter > > definition like it's done for the max burst length in the next patch? > > Something like this: > > --- include/linux/platform_data/dma-dw.h > > +++ include/linux/platform_data/dma-dw.h > > +#define DW_DMA_MIN_BURST 1 > > +#define DW_DMA_MAX_BURST 256 > > > > ? > > Yes! Ok. Good idea. I'll do that. Thanks. -Sergey > > -- > With Best Regards, > Andy Shevchenko > >
On Fri, May 29, 2020 at 01:23:53AM +0300, Serge Semin wrote: > Some hardware aside from default 0/1 may have greater minimum burst > transactions length constraints. Here we introduce the DMA device > and slave capability, which if required can be initialized by the DMA > engine driver with the device-specific value. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: linux-mips@vger.kernel.org > Cc: devicetree@vger.kernel.org > > --- > > Changelog v3: > - This is a new patch created as a result of the discussion with Vinud and > Andy in the framework of DW DMA burst and LLP capabilities. > --- > drivers/dma/dmaengine.c | 1 + > include/linux/dmaengine.h | 4 ++++ > 2 files changed, 5 insertions(+) > > diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c > index d31076d9ef25..b332ffe52780 100644 > --- a/drivers/dma/dmaengine.c > +++ b/drivers/dma/dmaengine.c > @@ -590,6 +590,7 @@ int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps) > caps->src_addr_widths = device->src_addr_widths; > caps->dst_addr_widths = device->dst_addr_widths; > caps->directions = device->directions; > + caps->min_burst = device->min_burst; > caps->max_burst = device->max_burst; > caps->residue_granularity = device->residue_granularity; > caps->descriptor_reuse = device->descriptor_reuse; > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h > index e1c03339918f..0c7403b27133 100644 > --- a/include/linux/dmaengine.h > +++ b/include/linux/dmaengine.h > @@ -465,6 +465,7 @@ enum dma_residue_granularity { > * Since the enum dma_transfer_direction is not defined as bit flag for > * each type, the dma controller should set BIT(<TYPE>) and same > * should be checked by controller as well > + * @min_burst: min burst capability per-transfer > * @max_burst: max burst capability per-transfer > * @cmd_pause: true, if pause is supported (i.e. for reading residue or > * for resume later) > @@ -478,6 +479,7 @@ struct dma_slave_caps { > u32 src_addr_widths; > u32 dst_addr_widths; > u32 directions; > + u32 min_burst; > u32 max_burst; > bool cmd_pause; > bool cmd_resume; > @@ -769,6 +771,7 @@ struct dma_filter { > * Since the enum dma_transfer_direction is not defined as bit flag for > * each type, the dma controller should set BIT(<TYPE>) and same > * should be checked by controller as well > + * @min_burst: min burst capability per-transfer > * @max_burst: max burst capability per-transfer > * @residue_granularity: granularity of the transfer residue reported > * by tx_status > @@ -839,6 +842,7 @@ struct dma_device { > u32 src_addr_widths; > u32 dst_addr_widths; > u32 directions; > + u32 min_burst; > u32 max_burst; > bool descriptor_reuse; > enum dma_residue_granularity residue_granularity; > -- > 2.26.2 >
On Fri, May 29, 2020 at 01:23:55AM +0300, Serge Semin wrote: > There are DMA devices (like ours version of Synopsys DW DMAC) which have > DMA capabilities non-uniformly redistributed amongst the device channels. > In order to provide a way of exposing the channel-specific parameters to > the DMA engine consumers, we introduce a new DMA-device callback. In case > if provided it gets called from the dma_get_slave_caps() method and is > able to override the generic DMA-device capabilities. I thought there is a pattern to return something, but it seems none. So, I have nothing against it to return void. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> But consider one comment below. > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: linux-mips@vger.kernel.org > Cc: devicetree@vger.kernel.org > > --- > > Changelog v3: > - This is a new patch created as a result of the discussion with Vinod and > Andy in the framework of DW DMA burst and LLP capabilities. > --- > drivers/dma/dmaengine.c | 3 +++ > include/linux/dmaengine.h | 2 ++ > 2 files changed, 5 insertions(+) > > diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c > index ad56ad58932c..edbb11d56cde 100644 > --- a/drivers/dma/dmaengine.c > +++ b/drivers/dma/dmaengine.c > @@ -599,6 +599,9 @@ int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps) > caps->cmd_resume = !!device->device_resume; > caps->cmd_terminate = !!device->device_terminate_all; > Perhaps a comment to explain that this is channel specific correction / override / you name it on top of device level capabilities? > + if (device->device_caps) > + device->device_caps(chan, caps); > + > return 0; > } > EXPORT_SYMBOL_GPL(dma_get_slave_caps); > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h > index a7e4d8dfdd19..b303e59929e5 100644 > --- a/include/linux/dmaengine.h > +++ b/include/linux/dmaengine.h > @@ -899,6 +899,8 @@ struct dma_device { > struct dma_chan *chan, dma_addr_t dst, u64 data, > unsigned long flags); > > + void (*device_caps)(struct dma_chan *chan, > + struct dma_slave_caps *caps); > int (*device_config)(struct dma_chan *chan, > struct dma_slave_config *config); > int (*device_pause)(struct dma_chan *chan); > -- > 2.26.2 >
On Fri, May 29, 2020 at 01:23:57AM +0300, Serge Semin wrote: > Maximum block size DW DMAC configuration corresponds to the max segment > size DMA parameter in the DMA core subsystem notation. Lets set it with a > value specific to the probed DW DMA controller. It shall help the DMA > clients to create size-optimized SG-list items for the controller. This in > turn will cause less dw_desc allocations, less LLP reinitializations, > better DMA device performance. Yes, something like that for time being, thanks! Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: linux-mips@vger.kernel.org > Cc: devicetree@vger.kernel.org > > --- > > Changelog v2: > - This is a new patch created in place of the dropped one: > "dmaengine: dw: Add LLP and block size config accessors". > > Changelog v3: > - Use the block_size found for the very first channel instead of looking for > the maximum of maximum block sizes. > - Don't define device-specific device_dma_parameters object, since it has > already been defined by the platform device core. > --- > drivers/dma/dw/core.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c > index 33e99d95b3d3..fb95920c429e 100644 > --- a/drivers/dma/dw/core.c > +++ b/drivers/dma/dw/core.c > @@ -1229,6 +1229,13 @@ int do_dma_probe(struct dw_dma_chip *chip) > BIT(DMA_MEM_TO_MEM); > dw->dma.residue_granularity = DMA_RESIDUE_GRANULARITY_BURST; > > + /* > + * For now there is no hardware with non uniform maximum block size > + * across all of the device channels, so we set the maximum segment > + * size as the block size found for the very first channel. > + */ > + dma_set_max_seg_size(dw->dma.dev, dw->chan[0].block_size); > + > err = dma_async_device_register(&dw->dma); > if (err) > goto err_dma_register; > -- > 2.26.2 >
On Fri, May 29, 2020 at 01:23:58AM +0300, Serge Semin wrote: > Since some DW DMA controllers (like one installed on Baikal-T1 SoC) may > have non-uniform DMA capabilities per device channels, let's add > the DW DMA specific device_caps callback to expose that specifics up to > the DMA consumer. It's a dummy function for now. We'll fill it in with > capabilities overrides in the next commits. This one I leave to Vinod to decide what to do. It is not harmful per se, but I consider better if it has a user already. Thus, no tag, sorry.
On Fri, May 29, 2020 at 03:12:03PM +0300, Andy Shevchenko wrote: > On Fri, May 29, 2020 at 01:23:55AM +0300, Serge Semin wrote: > > There are DMA devices (like ours version of Synopsys DW DMAC) which have > > DMA capabilities non-uniformly redistributed amongst the device channels. > > In order to provide a way of exposing the channel-specific parameters to > > the DMA engine consumers, we introduce a new DMA-device callback. In case > > if provided it gets called from the dma_get_slave_caps() method and is > > able to override the generic DMA-device capabilities. > > I thought there is a pattern to return something, but it seems none. > So, I have nothing against it to return void. > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > But consider one comment below. > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> > > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: linux-mips@vger.kernel.org > > Cc: devicetree@vger.kernel.org > > > > --- > > > > Changelog v3: > > - This is a new patch created as a result of the discussion with Vinod and > > Andy in the framework of DW DMA burst and LLP capabilities. > > --- > > drivers/dma/dmaengine.c | 3 +++ > > include/linux/dmaengine.h | 2 ++ > > 2 files changed, 5 insertions(+) > > > > diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c > > index ad56ad58932c..edbb11d56cde 100644 > > --- a/drivers/dma/dmaengine.c > > +++ b/drivers/dma/dmaengine.c > > @@ -599,6 +599,9 @@ int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps) > > caps->cmd_resume = !!device->device_resume; > > caps->cmd_terminate = !!device->device_terminate_all; > > > > Perhaps a comment to explain that this is channel specific correction / > override / you name it on top of device level capabilities? > > > + if (device->device_caps) > > + device->device_caps(chan, caps); > > + Agreed. I also forgot to add a doc-comment above the struct dma_device definition. -Sergey > > return 0; > > } > > EXPORT_SYMBOL_GPL(dma_get_slave_caps); > > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h > > index a7e4d8dfdd19..b303e59929e5 100644 > > --- a/include/linux/dmaengine.h > > +++ b/include/linux/dmaengine.h > > @@ -899,6 +899,8 @@ struct dma_device { > > struct dma_chan *chan, dma_addr_t dst, u64 data, > > unsigned long flags); > > > > + void (*device_caps)(struct dma_chan *chan, > > + struct dma_slave_caps *caps); > > int (*device_config)(struct dma_chan *chan, > > struct dma_slave_config *config); > > int (*device_pause)(struct dma_chan *chan); > > -- > > 2.26.2 > > > > -- > With Best Regards, > Andy Shevchenko > >
Baikal-T1 SoC has an DW DMAC on-board to provide a Mem-to-Mem, low-speed peripherals Dev-to-Mem and Mem-to-Dev functionality. Mostly it's compatible with currently implemented in the kernel DW DMAC driver, but there are some peculiarities which must be taken into account in order to have the device fully supported. First of all traditionally we replaced the legacy plain text-based dt-binding file with yaml-based one. Secondly Baikal-T1 DW DMA Controller provides eight channels, which alas have different max burst length configuration. In particular first two channels may burst up to 128 bits (16 bytes) at a time while the rest of them just up to 32 bits. We must make sure that the DMA subsystem doesn't set values exceeding these limitations otherwise the controller will hang up. In third currently we discovered the problem in using the DW APB SPI driver together with DW DMAC. The problem happens if there is no natively implemented multi-block LLP transfers support and the SPI-transfer length exceeds the max lock size. In this case due to asynchronous handling of Tx- and Rx- SPI transfers interrupt we might end up with Dw APB SSI Rx FIFO overflow. So if DW APB SSI (or any other DMAC service consumer) intends to use the DMAC to asynchronously execute the transfers we'd have to at least warn the user of the possible errors. In forth it's worth to set the DMA device max segment size with max block size config specific to the DW DMA controller. It shall help the DMA clients to create size-optimized SG-list items for the controller. This in turn will cause less dw_desc allocations, less LLP reinitializations, better DMA device performance. Finally there is a bug in the algorithm of the nollp flag detection. In particular even if DW DMAC parameters state the multi-block transfers support there is still HC_LLP (hardcode LLP) flag, which if set makes expected by the driver true multi-block LLP functionality unusable. This happens cause' if HC_LLP flag is set the LLP registers will be hardcoded to zero so the contiguous multi-block transfers will be only supported. We must take the flag into account when detecting the LLP support otherwise the driver just won't work correctly. This patchset is rebased and tested on the mainline Linux kernel 5.7-rc4: 0e698dfa2822 ("Linux 5.7-rc4") tag: v5.7-rc4 Changelog v2: - Rearrange SoBs. - Move $ref to the root level of the properties. So do do with the constraints in the DT binding. - Replace "additionalProperties: false" with "unevaluatedProperties: false" property in the DT binding file. - Discard default settings defined out of property enum constraint. - Set default max-burst-len to 256 TR-WIDTH words in the DT binding. - Discard noLLP and block_size accessors. - Set max segment size of the DMA device structure with the DW DMA block size config. - Print warning if noLLP flag is set. - Discard max burst length accessor. - Add comment about why hardware accelerated LLP list support depends on both MBLK_EN and HC_LLP configs setting. - Use explicit bits state comparison operator in noLLP flag setting. Link: https://lore.kernel.org/dmaengine/20200508105304.14065-1-Sergey.Semin@baikalelectronics.ru/ Changelog v3: - Use the block_size found for the very first channel instead of looking for the maximum of maximum block sizes. - Don't define device-specific device_dma_parameters object, since it has already been defined by the platform device core. - Add more details into the property description about what limitations snps,max-burst-len defines. - Move commit fb7e3bbfc830 ("dmaengine: dw: Take HC_LLP flag into account for noLLP auto-config") to the head of the series. - Add a new patch "dmaengine: Introduce min burst length capability" as a result of the discussion with Vinod and Andy regarding the burst length capability. - Add a new patch "dmaengine: Introduce max SG list entries capability" suggested by Andy. - Add a new patch "dmaengine: Introduce DMA-device device_caps callback" as a result of the discussion with Vinud and Andy in the framework of DW DMA burst and LLP capabilities. - Add a new patch "dmaengine: dw: Add dummy device_caps callback" as a preparation commit before setting the max_burst and max_sg_nents DW DMA capabilities. - Override the slave channel max_burst capability instead of calculating the minimum value of max burst lengths and setting the DMA-device generic capability. - Add a new patch "dmaengine: dw: Initialize max_sg_nents with nollp flag". This is required to fix the DW APB SSI issue of the Tx and Rx DMA channels de-synchronization. Link: https://lore.kernel.org/dmaengine/20200526225022.20405-1-Sergey.Semin@baikalelectronics.ru/ Changelog v4: - Use explicit if-else statement when assigning the max_sg_nents field. - Clamp the dst and src burst lengths in the generic dwc_config() method instead of doing that in the encode_maxburst() callback. - Define max_burst with u32 type in struct dw_dma_platform_data. - Perform of_property_read_u32_array() with the platform data max_burst member passed directly. - Add a new patch "dmaengine: dw: Initialize min_burst capability", which initializes the min_burst capability with 1. - Fix of->if typo. It should be definitely "of" in the max_sg_list capability description. Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru> Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru> Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru> Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru> Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru> Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Rob Herring <robh+dt@kernel.org> Cc: linux-mips@vger.kernel.org Cc: dmaengine@vger.kernel.org Cc: devicetree@vger.kernel.org Cc: linux-kernel@vger.kernel.org Serge Semin (11): dt-bindings: dma: dw: Convert DW DMAC to DT binding dt-bindings: dma: dw: Add max burst transaction length property dmaengine: Introduce min burst length capability dmaengine: Introduce max SG list entries capability dmaengine: Introduce DMA-device device_caps callback dmaengine: dw: Take HC_LLP flag into account for noLLP auto-config dmaengine: dw: Set DMA device max segment size parameter dmaengine: dw: Add dummy device_caps callback dmaengine: dw: Initialize min_burst capability dmaengine: dw: Introduce max burst length hw config dmaengine: dw: Initialize max_sg_nents capability .../bindings/dma/snps,dma-spear1340.yaml | 176 ++++++++++++++++++ .../devicetree/bindings/dma/snps-dma.txt | 69 ------- drivers/dma/dmaengine.c | 5 + drivers/dma/dw/core.c | 47 ++++- drivers/dma/dw/of.c | 5 + drivers/dma/dw/regs.h | 3 + include/linux/dmaengine.h | 14 ++ include/linux/platform_data/dma-dw.h | 4 + 8 files changed, 253 insertions(+), 70 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/snps,dma-spear1340.yaml delete mode 100644 Documentation/devicetree/bindings/dma/snps-dma.txt