linux-kernel - Re: [PATCH 5/5] dmaengine: dw: Add DMA-channels mask cell support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200730164146.GX3703480@smile.fi.intel.com>
Date:   Thu, 30 Jul 2020 19:41:46 +0300
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Serge Semin <Sergey.Semin@...kalelectronics.ru>
Cc:     Vinod Koul <vkoul@...nel.org>, Viresh Kumar <vireshk@...nel.org>,
        Dan Williams <dan.j.williams@...el.com>,
        Serge Semin <fancer.lancer@...il.com>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Pavel Parkhomenko <Pavel.Parkhomenko@...kalelectronics.ru>,
        Peter Ujfalusi <peter.ujfalusi@...com>,
        Rob Herring <robh+dt@...nel.org>, dmaengine@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/5] dmaengine: dw: Add DMA-channels mask cell support

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