linux-kernel - Re: [PATCH RFC 1/1] ASoC: fsl_ssi: Make fifo watermark and maxburst settings device tree options

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]

Date:	Thu, 14 Jan 2016 13:26:24 -0800
From:	Caleb Crome <caleb@...me.org>
To:	Nicolin Chen <nicoleotsuka@...il.com>
Cc:	Timur Tabi <timur@...i.org>, Xiubo Li <Xiubo.Lee@...il.com>,
	Liam Girdwood <lgirdwood@...il.com>,
	Mark Brown <broonie@...nel.org>,
	Jaroslav Kysela <perex@...ex.cz>,
	Takashi Iwai <tiwai@...e.com>, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	"alsa-devel@...a-project.org" <alsa-devel@...a-project.org>
Subject: Re: [PATCH RFC 1/1] ASoC: fsl_ssi: Make fifo watermark and maxburst
 settings device tree options

On Thu, Jan 14, 2016 at 12:18 PM, Nicolin Chen <nicoleotsuka@...il.com> wrote:
> On Thu, Jan 14, 2016 at 08:29:42AM -0800, Caleb Crome wrote:
>> Tuning the SSI fifo watermark & maxburst settings needs to be
>> optimized differently depending on the demands on the system.  The
>> current default of 2 is too low for high data-rate systems.  This
>> patch maintains exactly the same behavior by default (i.e defaults to
>> 2), but adds device tree options to set maxburst & fifo depth from the
>> device tree.  This is necessary because a setting of 2 simply doesn't
>> work at higher data rates.
>
>> @@ -61,6 +61,16 @@ Optional properties:
>>  - fsl,mode:         The operating mode for the AC97 interface only.
>>                      "ac97-slave" - AC97 mode, SSI is clock slave
>>                      "ac97-master" - AC97 mode, SSI is clock master
>> +- fsl,fifo-watermark: Sets the fifo watermark.  The default is
>> +                    fifo_depth-2 words, meaning 'initiate dma transfer
>> +                    when 2 words are left in the fifo'.  At higher
>> +                    data rates (48kHz, 16-channels for example), this
>> +                    causes silent but deadly DMA xruns and channel
>> +                    slips.  For 15 word FIFOs (like on MX5, MX6) 8 is
>> +                    a good value when running at high data rates
>> +- fsl,dma-maxburst: sets the max number of words to transfer in DMA.
>> +                    This defaults to the same value as
>> +                    fsl,fifo-watermark.
>
> I think DT maintainers may not give a consent towards these two
> properties as they are not to describe the hardware but to hack
> software configurations. (And it seems you haven't CCed them.)
>

Yeah, I thought I'd just ask alsa first, rather than send to DT
maintainers.  Is it preferable to just send to everybody that
get_maintainers spits out even for an RFC?


> I forgot which values you've figured out for these two properties,
> but I think those two values should work for normal cases as well:
> as SSI only has limited FIFO depth, it won't hurt (increasing too
> much latency) even if using a higher watermark configuration imo.
> So it could be a good idea to use optimized settings for all use
> cases and let other users test it.
>
> Nicolin


As for optimal settings, I finally came to a setting of 4 for depth &
maxburst, which will result in more DMA requests, but it's the only
way that works at 48kHz for me.  The default settings is 13 (15 - 2)
for the ones of the 15 item fifo, which is a pretty dramatic
difference.  I just don't know if other chips will behave badly in
that case.

I'd be happy to just submit a patch that sets it to 4 if we think
that's the right way to go.

-Caleb