| Message ID | 20200529144054.4251-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 05:40:52PM +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. The maximum burst transaction can vary from > channel to channel and from controller to controller depending on a > IP-core parameter the system engineer activated during the IP-core > synthesis. Let's initialise both min_burst and max_burst members of the > DMA controller descriptor with extreme values so the DMA clients could > use them to properly optimize the DMA requests. The channels and > controller-specific max_burst length initialization will be introduced > by the follow-up patches. 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 v4: > - This is a new patch suggested by Andy. > > Changelog v5: > - Introduce macro with extreme min and max burst length supported by the > DW DMA controller. > - Initialize max_burst length capability with extreme burst length supported > by the DW DMAC IP-core. > --- > drivers/dma/dw/core.c | 2 ++ > include/linux/platform_data/dma-dw.h | 2 ++ > 2 files changed, 4 insertions(+) > > diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c > index ceded21537e2..4887aa2fc73c 100644 > --- a/drivers/dma/dw/core.c > +++ b/drivers/dma/dw/core.c > @@ -1229,6 +1229,8 @@ int do_dma_probe(struct dw_dma_chip *chip) > dw->dma.device_issue_pending = dwc_issue_pending; > > /* DMA capabilities */ > + dw->dma.min_burst = DW_DMA_MIN_BURST; > + dw->dma.max_burst = DW_DMA_MAX_BURST; > 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) | > diff --git a/include/linux/platform_data/dma-dw.h b/include/linux/platform_data/dma-dw.h > index f3eaf9ec00a1..369e41e9dcc9 100644 > --- a/include/linux/platform_data/dma-dw.h > +++ b/include/linux/platform_data/dma-dw.h > @@ -12,6 +12,8 @@ > > #define DW_DMA_MAX_NR_MASTERS 4 > #define DW_DMA_MAX_NR_CHANNELS 8 > +#define DW_DMA_MIN_BURST 1 > +#define DW_DMA_MAX_BURST 256 > > /** > * struct dw_dma_slave - Controller-specific information about a slave > -- > 2.26.2 >
Vinod, Viresh Andy's finished his review. So all the patches of the series (except one rather decorative, which we have different opinion of) are tagged by him. Since merge window is about to be opened please consider to merge the series in. I'll really need it to be in the kernel to provide the noLLP-problem fix for the Dw APB SSI in 5.8. -Sergey On Fri, May 29, 2020 at 05:40:43PM +0300, Serge Semin wrote: > 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. > > Link: https://lore.kernel.org/dmaengine/20200528222401.26941-1-Sergey.Semin@baikalelectronics.ru > Changelog v5: > - Introduce macro with extreme min and max burst lengths supported by the > DW DMA controller. Define them in the patch with default min and max burst > length iintializations. > - Initialize max_burst length capability with extreme burst length supported > by the DW DMAC IP-core. > - Move DW_DMA_MAX_BURST macro definition to the patch "dmaengine: dw: > Initialize min and max burst DMA device capability". > - Add in-line comment at the point of the device_caps callback invocation. > - Add doc-comment for the device_caps member of struct dma_device > > 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 and max burst DMA device 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 | 12 ++ > drivers/dma/dw/core.c | 48 ++++- > drivers/dma/dw/of.c | 5 + > drivers/dma/dw/regs.h | 3 + > include/linux/dmaengine.h | 16 ++ > include/linux/platform_data/dma-dw.h | 5 + > 8 files changed, 264 insertions(+), 70 deletions(-) > create mode 100644 Documentation/devicetree/bindings/dma/snps,dma-spear1340.yaml > delete mode 100644 Documentation/devicetree/bindings/dma/snps-dma.txt > > -- > 2.26.2 >
Hi Serge, On 02-06-20, 12:27, Serge Semin wrote: > Vinod, Viresh > > Andy's finished his review. So all the patches of the series (except one rather > decorative, which we have different opinion of) are tagged by him. Since merge > window is about to be opened please consider to merge the series in. I'll really > need it to be in the kernel to provide the noLLP-problem fix for the Dw APB SSI > in 5.8. Sorry it was too late for 5.8.. merge window is closed now, i will review it shortly
On Tue, Jun 16, 2020 at 10:02:42PM +0530, Vinod Koul wrote: > Hi Serge, > > On 02-06-20, 12:27, Serge Semin wrote: > > Vinod, Viresh > > > > Andy's finished his review. So all the patches of the series (except one rather > > decorative, which we have different opinion of) are tagged by him. Since merge > > window is about to be opened please consider to merge the series in. I'll really > > need it to be in the kernel to provide the noLLP-problem fix for the Dw APB SSI > > in 5.8. > > Sorry it was too late for 5.8.. merge window is closed now, i will > review it shortly Yeah, alas It's too late now. On the other hand this is probably for better since I've discovered that the noLLP-related problem is more complicated than I thought in the first place. Yes, the interrupt handling latency will cause the problem with Rx FIFO overflow, but the overflow might still happen due to several other reasons (mostly due to our APB bus being too slow, but it can still be partly fixed, and I am looking for a convenient solution at the moment). So could you please merge the series in without the next patches: [PATCH v5 04/11] dmaengine: Introduce max SG list entries capability [PATCH v5 11/11] dmaengine: dw: Initialize max_sg_nents capability If I manage to fix the problem in general then I'll send another series with those two patches being part of it. Otherwise they won't be much useful for my platform. Sorry for inconvenience and thanks for understanding. -Sergey > > -- > ~Vinod
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. Link: https://lore.kernel.org/dmaengine/20200528222401.26941-1-Sergey.Semin@baikalelectronics.ru Changelog v5: - Introduce macro with extreme min and max burst lengths supported by the DW DMA controller. Define them in the patch with default min and max burst length iintializations. - Initialize max_burst length capability with extreme burst length supported by the DW DMAC IP-core. - Move DW_DMA_MAX_BURST macro definition to the patch "dmaengine: dw: Initialize min and max burst DMA device capability". - Add in-line comment at the point of the device_caps callback invocation. - Add doc-comment for the device_caps member of struct dma_device 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 and max burst DMA device 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 | 12 ++ drivers/dma/dw/core.c | 48 ++++- drivers/dma/dw/of.c | 5 + drivers/dma/dw/regs.h | 3 + include/linux/dmaengine.h | 16 ++ include/linux/platform_data/dma-dw.h | 5 + 8 files changed, 264 insertions(+), 70 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/snps,dma-spear1340.yaml delete mode 100644 Documentation/devicetree/bindings/dma/snps-dma.txt