Re: [PATCH 3/3] ASoC: omap-mcbsp: Add period size protection mode

[Peter Ujfalusi <peter.ujfalusi@ti.com>]

On 03/20/2012 06:20 PM, Jarkko Nikula wrote:
> On 03/20/2012 01:13 PM, Peter Ujfalusi wrote:
>> Certain application can experience underrun right after the playback start.
>> This is caused by the McBSP FIFO/sDMA integration:
>> The sDMA will push samples to the FIFO till it has threshold amount of free
>> slots available in the FIFO. If the application picks period size which is
>> smaller than the FIFO size, and it did not prepared multiple periods, or
>> it did not set the start_threshold for the stream to cover the FIFO size
>> the hw pointer will move forward, which is causing the underrun.
>>
>> Add a sysfs entry for McBSP ports: period_protection.
>> If this property is set the driver will place the constraint agains the
>> period size, and not for the buffer size. To ensure that we do not hit
>> underrun, the period size constraint will be increased with the requested
>> number of frames (the period size will be FIFO size + period_protection).
>>
>> As default the period_protection is disabled.
>>
> I don't think this is going to solve the actual problem here where
> custom asound.conf was required.

I also suggested to use custom asound.conf. For Pandora it does exist,
but the argument was that it need to be specifically created/copied to
any new OS being ported to the HW.

> IMHO custom sysfs is even worse option and very hard to remove afterwards.

Well, for system integration's point of view I would prefer the sysfs
file. If this is needed by the distribution/use case it can be enabled,
disabled in runtime. In my view it poses less hassle for distributions.

> And defaulting this new setting for Pandora might break e.g. MER or MeeGo on N900.

I would not default this behavior for the exact same reasons. It was
planed to be optional.

> I didn't check this but would it be possible to either put restriction
> to start-delay (I think Grazvydas said he has experimental code for
> that?) or make sure that minimum buffer size must be higher than FIFO +
> 1 period (or something like that)?

The start_threshold is sw_param, driver should not touch it. In fact it
is way above the driver layer.

-- 
Péter