Re: [Qemu-devel] [PATCH 1/2] audio: honor QEMU_AUDIO_TIMER_PERIOD instead of waking up every *nano* second

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 10/10/2013 08:31 AM, Alex Bligh wrote:
>
> On 9 Oct 2013, at 20:42, Hans de Goede wrote:
>
>> Now that we no longer have MIN_REARM_TIMER_NS a bug in the audio subsys has
>> clearly shown it self by trying to make a timer fire every nano second.
>>
>> Note we have a similar problem in 1.6, 1.5 and older but there
>> MIN_REARM_TIMER_NS limits the wakeups caused by audio being active to
>> 4000 times / second. This still causes a host cpu load of 50 % for simply
>> playing audio, where as with this patch git master is at 13%, so we should
>> backport this to 1.5 and 1.6 too.
>>
>> Note this will not apply to 1.5 and 1.6 as is.
>>
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> audio/audio.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/audio/audio.c b/audio/audio.c
>> index af4cdf6..b3db679 100644
>> --- a/audio/audio.c
>> +++ b/audio/audio.c
>> @@ -1124,7 +1124,8 @@ static int audio_is_timer_needed (void)
>> static void audio_reset_timer (AudioState *s)
>> {
>>      if (audio_is_timer_needed ()) {
>> -        timer_mod (s->ts, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 1);
>> +        timer_mod (s->ts,
>> +            qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + conf.period.ticks);
>
> This assumes conf.period.ticks is in nanoseconds. That seems wrong.
> Suggest multiplying by SCALE_US or SCALE_MS.

Which it is, quoting from higher up in the same file:

         conf.period.ticks =
             muldiv64 (1, get_ticks_per_sec (), conf.period.hertz);

And get_ticks_per_sec () returns ns .

Regards,

Hans



>
> Alex
>
>>      }
>>      else {
>>          timer_del (s->ts);
>> --
>> 1.8.3.1
>>
>>
>>
>