[alsa-devel] [PATCH] ASoC: sigmadsp: safeload should be used from 4 bytes

[Lars-Peter Clausen]

On 08/22/2018 03:40 PM, Danny Smith wrote:
> 
> 
> On 08/22/2018 03:05 PM, Lars-Peter Clausen wrote:
>> On 08/22/2018 09:53 AM, Danny Smith wrote:
>>>
>>> On 08/22/2018 08:34 AM, Lars-Peter Clausen wrote:
>>>> On 08/22/2018 08:11 AM, Robert Rosengren wrote:
>>>>> From: Danny Smith <dannys at axis.com>
>>>>>
>>>>> Fixed range in safeload conditional to allow safeload to be used from
>>>>> 4 bytes.
>>>> Hi,
>>>>
>>>> Thanks for the patch. The reason why 4 bytes is excluded is that up to 4
>>>> bytes can be updated atomically with a single register write. But I could
>>>> see that if the firmware reads the same parameter multiple times during the
>>>> same run you could get inconsistent results.
>>>>
>>>> Can you explain a bit more why you need this?
>>>>
>>>> If we want to allow safeload for 4 byte parameters the same reasoning
>>>> applies for parameters with less than 4 bytes as well and so the check
>>>> should be removed completely.
>>>>
>>>> - Lars
>>> Hi,
>>>
>>> Thanks for the feedback. It is correct that up to 4 bytes can be updated
>>> atomically but does that also guarantee that the data is safeloaded, i.e.
>>> updated when the parameter is not in use? Maybe updating up to 4 byte
>>> parameters without safeload does not cause audio glitches?
>> The datasheet says:
>>
>> """
>> To update parameters in real time while avoiding pop and click
>> noises on the output, the ADAU1761 uses a software safeload
>> mechanism. The software safeload mechanism enables the
>> SigmaDSP core to load new parameters into RAM while guaranteeing
>> that the parameters are not in use. This prevents an
>> undesirable condition where an instruction could execute with
>> a mix of old and new parameters.
>> """
>>
>> The way I understand the last sentence is that there is no issue when
>> loading only 4 bytes or less since there is no risk of mixing old and new
>> data.
>>
>> I don't mind changing this, I was just curious if you had seen any issues
>> with a 4 byte parameter update.
>>
>> - Lars
> I have not seen any issues doing a 4byte parameter update without safeload
> but have not performed any extensive testing on the subject. The reason for
> the patch was that I noticed that sigmastudio uses safeload when doing 4byte
> writes but I cannot say if it is necessary or not.

Ok, then lets change the driver, but drop the test completely so that
anything up to 20 bytes uses safeload.