On Thu, Jul 30, 2020 at 06:45:45PM +0300, Serge Semin wrote: > DW DMA IP-core provides a way to synthesize the DMA controller with > channels having different parameters like maximum burst-length, > multi-block support, maximum data width, etc. Those parameters both > explicitly and implicitly affect the channels performance. Since DMA slave > devices might be very demanding to the DMA performance, let's provide a > functionality for the slaves to be assigned with DW DMA channels, which > performance according to the platform engineer fulfill their requirements. > After this patch is applied it can be done by passing the mask of suitable > DMA-channels either directly in the dw_dma_slave structure instance or as > a fifth cell of the DMA DT-property. If mask is zero or not provided, then > there is no limitation on the channels allocation. > > For instance Baikal-T1 SoC is equipped with a DW DMAC engine, which first > two channels are synthesized with max burst length of 16, while the rest > of the channels have been created with max-burst-len=4. It would seem that > the first two channels must be faster than the others and should be more > preferable for the time-critical DMA slave devices. In practice it turned > out that the situation is quite the opposite. The channels with > max-burst-len=4 demonstrated a better performance than the channels with > max-burst-len=16 even when they both had been initialized with the same > settings. The performance drop of the first two DMA-channels made them > unsuitable for the DW APB SSI slave device. No matter what settings they > are configured with, full-duplex SPI transfers occasionally experience the > Rx FIFO overflow. It means that the DMA-engine doesn't keep up with > incoming data pace even though the SPI-bus is enabled with speed of 25MHz > while the DW DMA controller is clocked with 50MHz signal. There is no such > problem has been noticed for the channels synthesized with > max-burst-len=4. ... > + if (dws->channels && !(dws->channels & dwc->mask)) You can drop the first check if... > + return false; ... > + if (dma_spec->args_count >= 4) > + slave.channels = dma_spec->args[3]; ...you apply sane default here or somewhere else. ... > + fls(slave.channels) > dw->pdata->nr_channels)) Does it really make sense? I think it can also be simplified to faster op, i.e. BIT(nr_channels) < slave.channels (but check for off-by-one errors) ... > + * @channels: mask of the channels permitted for allocation (zero > + * value means any) Perhaps on one line? -- With Best Regards, Andy Shevchenko
