Re: [PATCH v2 3/4] ASoC: davinci-mcasp: Constraint on the period and buffer size based on FIFO usage

From: Mark Brown <broonie@kernel.org>
To: Peter Ujfalusi <peter.ujfalusi@ti.com>
Cc: alsa-devel@alsa-project.org, Lars-Peter Clausen <lars@metafoo.de>,
	nsekhar@ti.com, Liam Girdwood <lgirdwood@gmail.com>,
	Jyri Sarha <jsarha@ti.com>,
	zonque@gmail.com
Subject: Re: [PATCH v2 3/4] ASoC: davinci-mcasp: Constraint on the period and buffer size based on FIFO usage
Date: Mon, 17 Mar 2014 16:52:14 +0000	[thread overview]
Message-ID: <20140317165214.GT11706@sirena.org.uk> (raw)
In-Reply-To: <5326F891.7090906@ti.com>

[-- Attachment #1.1: Type: text/plain, Size: 1740 bytes --]

On Mon, Mar 17, 2014 at 03:28:49PM +0200, Peter Ujfalusi wrote:
> On 03/16/2014 01:18 PM, Lars-Peter Clausen wrote:

> > Hm... the sound like there is still a bug somewhere. We constrain the buffer
> > size to be a multiple of the period size. If the period size is constraint to
> > be a multiple of a constant then the buffer size should automatically be
> > constrained to be a multiple of constant * period count. And just constraining
> > the buffer size to be a multiple of the burst size still allows buffer sizes
> > that are not a multiple of burst size * period count.

> Yeah, aplay, PA is fine. mplayer however does things in a different manner. I
> use mplayer mainly to test PA (with -ao pulse) but sometimes I use it through
> ALSA and this is where I noticed the issue.

> If I constrain only period_size mplayer will fail to set hw_params. If I only
> constrain the buffer_size than we can have period_size which does not allign
> with the FIFO settings.

> I also tried to have snd_pcm_hw_rule_add() and using the snd_interval_step() -
> which need to be exported but mplayer fails with that also. It fails even if I
> add the rule for period and buffer size.

> Also the constraint as it is currently is not optimal since it does not take
> into account the number of channels as you pointed out.

This is all sounding like the thing that needs to be looked at here is
mplayer so we understand what's going wrong with regard to the buffer
sizes.  It's sounding like if we should be doing it this is a general
thing which we should be constraining presumably it'd apply to all
drivers, not just this one, and so shouldn't be being fixed in the
driver but it's not obvious to me why the period constraint isn't
sufficient.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]