audio: new timer code for hda codec, fix audio_get_conf_int

-----BEGIN PGP SIGNATURE-----
 Version: GnuPG v2.0.22 (GNU/Linux)
 
 iQIcBAABAgAGBQJbMOpJAAoJEEy22O7T6HE46J4QAJFzQjX1trVObx7lqhVMiQiP
 C6IS6/FkqY22fylWQuvl2Sq/TfRpftXWvAIEYNybbYo2uT0oeZQDARP9Atq+WDjk
 yzlVHEsARmiXPnvyae3XsDAdnoEOfQne31kOE1qDbQxPHUHMigWSFw/6/zDqfV3P
 dObk1SJFrUfxVY6puECbtcli5SXsZG9DA7Vmy/LT0xsOAReOKxNNLN4/MipAwR3y
 iss21IGTSx157zF3nK5W7++BQVwI3+ATiTeOT7taCujzoKno0cKKQ/z9ItsFILGM
 eAyODn7X+lMSb2DTGmYi8mtzwBMxu5DsIvF07wevc/jD1zeeFayAj6+WT564MxDD
 t2sZ11q0h3rqm8MpPjje97BFjHpI296ZOLGBR1s6Z2z0V3YM76SsfmomEORfCaQ7
 3dJfFOGYoYpJQ/qvGF/uZSg5MwuqfpqpdMxwEFcl+zBLrIMcWAanTNNUtHs42rJV
 83klCGeZKe29X0U+CaDFMzXAAlUmRdjq9BFj4f0chzd8tx6hEQa8L2GJDKy32gHn
 xF6fwSInasP9p0rRI9bESd6J+1BOqhyvYS7G1zjx8feTIh3jVDFqxk9gjU7fFSmg
 RjPJgZxmYW6p6I762KundcsFzTxE7ZRTIeJ53XnDlQuQo8O1cj7ynTVV24QSkmA+
 1j+fLHHNNfQx4XhIR7cV
 =6DsA
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/kraxel/tags/audio-20180625-pull-request' into staging

audio: new timer code for hda codec, fix audio_get_conf_int

# gpg: Signature made Mon 25 Jun 2018 14:12:41 BST
# gpg:                using RSA key 4CB6D8EED3E87138
# gpg: Good signature from "Gerd Hoffmann (work) <kraxel@redhat.com>"
# gpg:                 aka "Gerd Hoffmann <gerd@kraxel.org>"
# gpg:                 aka "Gerd Hoffmann (private) <kraxel@gmail.com>"
# Primary key fingerprint: A032 8CFF B93A 17A7 9901  FE7D 4CB6 D8EE D3E8 7138

* remotes/kraxel/tags/audio-20180625-pull-request:
  audio: Convert use of atoi to qemu_strtoi
  audio/hda: enable new timer code by default.
  audio/hda: detect output buffer overruns
  audio/hda: tweak timer adjust logic
  audio/hda: turn some dprintfs into trace points
  audio/hda: create millisecond timers that handle IO

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

audio/audio.c

@@ -335,9 +335,8 @@ static int audio_get_conf_int (const char *key, int defval, int *defaultp)
     char *strval;
 
     strval = getenv (key);
-    if (strval) {
+    if (strval && !qemu_strtoi(strval, NULL, 10, &val)) {
         *defaultp = 0;
-        val = atoi (strval);
         return val;
     }
     else {

hw/audio/hda-codec.c

@@ -18,11 +18,13 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/atomic.h"
 #include "hw/hw.h"
 #include "hw/pci/pci.h"
 #include "intel-hda.h"
 #include "intel-hda-defs.h"
 #include "audio/audio.h"
+#include "trace.h"
 
 /* -------------------------------------------------------------------------- */
 
@@ -126,6 +128,10 @@ static void hda_codec_parse_fmt(uint32_t format, struct audsettings *as)
 #define   PARAM nomixemu
 #include  "hda-codec-common.h"
 
+#define HDA_TIMER_TICKS (SCALE_MS)
+#define B_SIZE sizeof(st->buf)
+#define B_MASK (sizeof(st->buf) - 1)
+
 /* -------------------------------------------------------------------------- */
 
 static const char *fmt2name[] = {
@@ -154,8 +160,13 @@ struct HDAAudioStream {
         SWVoiceIn *in;
         SWVoiceOut *out;
     } voice;
-    uint8_t buf[HDA_BUFFER_SIZE];
+    uint8_t compat_buf[HDA_BUFFER_SIZE];
-    uint32_t bpos;
+    uint32_t compat_bpos;
+    uint8_t buf[8192]; /* size must be power of two */
+    int64_t rpos;
+    int64_t wpos;
+    QEMUTimer *buft;
+    int64_t buft_start;
 };
 
 #define TYPE_HDA_AUDIO "hda-audio"
@@ -174,55 +185,217 @@ struct HDAAudioState {
     /* properties */
     uint32_t debug;
     bool     mixer;
+    bool     use_timer;
 };
 
+static inline int64_t hda_bytes_per_second(HDAAudioStream *st)
+{
+    return 2 * st->as.nchannels * st->as.freq;
+}
+
+static inline void hda_timer_sync_adjust(HDAAudioStream *st, int64_t target_pos)
+{
+    int64_t limit = B_SIZE / 8;
+    int64_t corr = 0;
+
+    if (target_pos > limit) {
+        corr = HDA_TIMER_TICKS;
+    }
+    if (target_pos < -limit) {
+        corr = -HDA_TIMER_TICKS;
+    }
+    if (corr == 0) {
+        return;
+    }
+
+    trace_hda_audio_adjust(st->node->name, target_pos);
+    atomic_fetch_add(&st->buft_start, corr);
+}
+
+static void hda_audio_input_timer(void *opaque)
+{
+    HDAAudioStream *st = opaque;
+
+    int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+
+    int64_t buft_start = atomic_fetch_add(&st->buft_start, 0);
+    int64_t wpos = atomic_fetch_add(&st->wpos, 0);
+    int64_t rpos = atomic_fetch_add(&st->rpos, 0);
+
+    int64_t wanted_rpos = hda_bytes_per_second(st) * (now - buft_start)
+                          / NANOSECONDS_PER_SECOND;
+    wanted_rpos &= -4; /* IMPORTANT! clip to frames */
+
+    if (wanted_rpos <= rpos) {
+        /* we already transmitted the data */
+        goto out_timer;
+    }
+
+    int64_t to_transfer = audio_MIN(wpos - rpos, wanted_rpos - rpos);
+    while (to_transfer) {
+        uint32_t start = (rpos & B_MASK);
+        uint32_t chunk = audio_MIN(B_SIZE - start, to_transfer);
+        int rc = hda_codec_xfer(
+                &st->state->hda, st->stream, false, st->buf + start, chunk);
+        if (!rc) {
+            break;
+        }
+        rpos += chunk;
+        to_transfer -= chunk;
+        atomic_fetch_add(&st->rpos, chunk);
+    }
+
+out_timer:
+
+    if (st->running) {
+        timer_mod_anticipate_ns(st->buft, now + HDA_TIMER_TICKS);
+    }
+}
+
 static void hda_audio_input_cb(void *opaque, int avail)
 {
     HDAAudioStream *st = opaque;
-    int recv = 0;
-    int len;
-    bool rc;
 
-    while (avail - recv >= sizeof(st->buf)) {
+    int64_t wpos = atomic_fetch_add(&st->wpos, 0);
-        if (st->bpos != sizeof(st->buf)) {
+    int64_t rpos = atomic_fetch_add(&st->rpos, 0);
-            len = AUD_read(st->voice.in, st->buf + st->bpos,
+
-                           sizeof(st->buf) - st->bpos);
+    int64_t to_transfer = audio_MIN(B_SIZE - (wpos - rpos), avail);
-            st->bpos += len;
+
-            recv += len;
+    hda_timer_sync_adjust(st, -((wpos - rpos) + to_transfer - (B_SIZE >> 1)));
-            if (st->bpos != sizeof(st->buf)) {
+
+    while (to_transfer) {
+        uint32_t start = (uint32_t) (wpos & B_MASK);
+        uint32_t chunk = (uint32_t) audio_MIN(B_SIZE - start, to_transfer);
+        uint32_t read = AUD_read(st->voice.in, st->buf + start, chunk);
+        wpos += read;
+        to_transfer -= read;
+        atomic_fetch_add(&st->wpos, read);
+        if (chunk != read) {
             break;
         }
     }
-        rc = hda_codec_xfer(&st->state->hda, st->stream, false,
+}
-                            st->buf, sizeof(st->buf));
+
+static void hda_audio_output_timer(void *opaque)
+{
+    HDAAudioStream *st = opaque;
+
+    int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+
+    int64_t buft_start = atomic_fetch_add(&st->buft_start, 0);
+    int64_t wpos = atomic_fetch_add(&st->wpos, 0);
+    int64_t rpos = atomic_fetch_add(&st->rpos, 0);
+
+    int64_t wanted_wpos = hda_bytes_per_second(st) * (now - buft_start)
+                          / NANOSECONDS_PER_SECOND;
+    wanted_wpos &= -4; /* IMPORTANT! clip to frames */
+
+    if (wanted_wpos <= wpos) {
+        /* we already received the data */
+        goto out_timer;
+    }
+
+    int64_t to_transfer = audio_MIN(B_SIZE - (wpos - rpos), wanted_wpos - wpos);
+    while (to_transfer) {
+        uint32_t start = (wpos & B_MASK);
+        uint32_t chunk = audio_MIN(B_SIZE - start, to_transfer);
+        int rc = hda_codec_xfer(
+                &st->state->hda, st->stream, true, st->buf + start, chunk);
         if (!rc) {
             break;
         }
-        st->bpos = 0;
+        wpos += chunk;
+        to_transfer -= chunk;
+        atomic_fetch_add(&st->wpos, chunk);
+    }
+
+out_timer:
+
+    if (st->running) {
+        timer_mod_anticipate_ns(st->buft, now + HDA_TIMER_TICKS);
     }
 }
 
 static void hda_audio_output_cb(void *opaque, int avail)
+{
+    HDAAudioStream *st = opaque;
+
+    int64_t wpos = atomic_fetch_add(&st->wpos, 0);
+    int64_t rpos = atomic_fetch_add(&st->rpos, 0);
+
+    int64_t to_transfer = audio_MIN(wpos - rpos, avail);
+
+    if (wpos - rpos == B_SIZE) {
+        /* drop buffer, reset timer adjust */
+        st->rpos = 0;
+        st->wpos = 0;
+        st->buft_start = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+        trace_hda_audio_overrun(st->node->name);
+        return;
+    }
+
+    hda_timer_sync_adjust(st, (wpos - rpos) - to_transfer - (B_SIZE >> 1));
+
+    while (to_transfer) {
+        uint32_t start = (uint32_t) (rpos & B_MASK);
+        uint32_t chunk = (uint32_t) audio_MIN(B_SIZE - start, to_transfer);
+        uint32_t written = AUD_write(st->voice.out, st->buf + start, chunk);
+        rpos += written;
+        to_transfer -= written;
+        atomic_fetch_add(&st->rpos, written);
+        if (chunk != written) {
+            break;
+        }
+    }
+}
+
+static void hda_audio_compat_input_cb(void *opaque, int avail)
+{
+    HDAAudioStream *st = opaque;
+    int recv = 0;
+    int len;
+    bool rc;
+
+    while (avail - recv >= sizeof(st->compat_buf)) {
+        if (st->compat_bpos != sizeof(st->compat_buf)) {
+            len = AUD_read(st->voice.in, st->compat_buf + st->compat_bpos,
+                           sizeof(st->compat_buf) - st->compat_bpos);
+            st->compat_bpos += len;
+            recv += len;
+            if (st->compat_bpos != sizeof(st->compat_buf)) {
+                break;
+            }
+        }
+        rc = hda_codec_xfer(&st->state->hda, st->stream, false,
+                            st->compat_buf, sizeof(st->compat_buf));
+        if (!rc) {
+            break;
+        }
+        st->compat_bpos = 0;
+    }
+}
+
+static void hda_audio_compat_output_cb(void *opaque, int avail)
 {
     HDAAudioStream *st = opaque;
     int sent = 0;
     int len;
     bool rc;
 
-    while (avail - sent >= sizeof(st->buf)) {
+    while (avail - sent >= sizeof(st->compat_buf)) {
-        if (st->bpos == sizeof(st->buf)) {
+        if (st->compat_bpos == sizeof(st->compat_buf)) {
             rc = hda_codec_xfer(&st->state->hda, st->stream, true,
-                                st->buf, sizeof(st->buf));
+                                st->compat_buf, sizeof(st->compat_buf));
             if (!rc) {
                 break;
             }
-            st->bpos = 0;
+            st->compat_bpos = 0;
         }
-        len = AUD_write(st->voice.out, st->buf + st->bpos,
+        len = AUD_write(st->voice.out, st->compat_buf + st->compat_bpos,
-                        sizeof(st->buf) - st->bpos);
+                        sizeof(st->compat_buf) - st->compat_bpos);
-        st->bpos += len;
+        st->compat_bpos += len;
         sent += len;
-        if (st->bpos != sizeof(st->buf)) {
+        if (st->compat_bpos != sizeof(st->compat_buf)) {
             break;
         }
     }
@@ -237,8 +410,18 @@ static void hda_audio_set_running(HDAAudioStream *st, bool running)
         return;
     }
     st->running = running;
-    dprint(st->state, 1, "%s: %s (stream %d)\n", st->node->name,
+    trace_hda_audio_running(st->node->name, st->stream, st->running);
-           st->running ? "on" : "off", st->stream);
+    if (st->state->use_timer) {
+        if (running) {
+            int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+            st->rpos = 0;
+            st->wpos = 0;
+            st->buft_start = now;
+            timer_mod_anticipate_ns(st->buft, now + HDA_TIMER_TICKS);
+        } else {
+            timer_del(st->buft);
+        }
+    }
     if (st->output) {
         AUD_set_active_out(st->voice.out, st->running);
     } else {
@@ -274,22 +457,36 @@ static void hda_audio_set_amp(HDAAudioStream *st)
 
 static void hda_audio_setup(HDAAudioStream *st)
 {
+    bool use_timer = st->state->use_timer;
+    audio_callback_fn cb;
+
     if (st->node == NULL) {
         return;
     }
 
-    dprint(st->state, 1, "%s: format: %d x %s @ %d Hz\n",
+    trace_hda_audio_format(st->node->name, st->as.nchannels,
-           st->node->name, st->as.nchannels,
                            fmt2name[st->as.fmt], st->as.freq);
 
     if (st->output) {
-        st->voice.out = AUD_open_out(&st->state->card, st->voice.out,
+        if (use_timer) {
-                                     st->node->name, st,
+            cb = hda_audio_output_cb;
-                                     hda_audio_output_cb, &st->as);
+            st->buft = timer_new_ns(QEMU_CLOCK_VIRTUAL,
+                                    hda_audio_output_timer, st);
         } else {
+            cb = hda_audio_compat_output_cb;
+        }
+        st->voice.out = AUD_open_out(&st->state->card, st->voice.out,
+                                     st->node->name, st, cb, &st->as);
+    } else {
+        if (use_timer) {
+            cb = hda_audio_input_cb;
+            st->buft = timer_new_ns(QEMU_CLOCK_VIRTUAL,
+                                    hda_audio_input_timer, st);
+        } else {
+            cb = hda_audio_compat_input_cb;
+        }
         st->voice.in = AUD_open_in(&st->state->card, st->voice.in,
-                                   st->node->name, st,
+                                   st->node->name, st, cb, &st->as);
-                                   hda_audio_input_cb, &st->as);
     }
 }
 
@@ -505,7 +702,7 @@ static int hda_audio_init(HDACodecDevice *hda, const struct desc_codec *desc)
                 /* unmute output by default */
                 st->gain_left = QEMU_HDA_AMP_STEPS;
                 st->gain_right = QEMU_HDA_AMP_STEPS;
-                st->bpos = sizeof(st->buf);
+                st->compat_bpos = sizeof(st->compat_buf);
                 st->output = true;
             } else {
                 st->output = false;
@@ -532,6 +729,9 @@ static void hda_audio_exit(HDACodecDevice *hda)
         if (st->node == NULL) {
             continue;
         }
+        if (a->use_timer) {
+            timer_del(st->buft);
+        }
         if (st->output) {
             AUD_close_out(&a->card, st->voice.out);
         } else {
@@ -581,6 +781,26 @@ static void hda_audio_reset(DeviceState *dev)
     }
 }
 
+static bool vmstate_hda_audio_stream_buf_needed(void *opaque)
+{
+    HDAAudioStream *st = opaque;
+    return st->state->use_timer;
+}
+
+static const VMStateDescription vmstate_hda_audio_stream_buf = {
+    .name = "hda-audio-stream/buffer",
+    .version_id = 1,
+    .needed = vmstate_hda_audio_stream_buf_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_BUFFER(buf, HDAAudioStream),
+        VMSTATE_INT64(rpos, HDAAudioStream),
+        VMSTATE_INT64(wpos, HDAAudioStream),
+        VMSTATE_TIMER_PTR(buft, HDAAudioStream),
+        VMSTATE_INT64(buft_start, HDAAudioStream),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_hda_audio_stream = {
     .name = "hda-audio-stream",
     .version_id = 1,
@@ -592,9 +812,13 @@ static const VMStateDescription vmstate_hda_audio_stream = {
         VMSTATE_UINT32(gain_right, HDAAudioStream),
         VMSTATE_BOOL(mute_left, HDAAudioStream),
         VMSTATE_BOOL(mute_right, HDAAudioStream),
-        VMSTATE_UINT32(bpos, HDAAudioStream),
+        VMSTATE_UINT32(compat_bpos, HDAAudioStream),
-        VMSTATE_BUFFER(buf, HDAAudioStream),
+        VMSTATE_BUFFER(compat_buf, HDAAudioStream),
         VMSTATE_END_OF_LIST()
+    },
+    .subsections = (const VMStateDescription * []) {
+        &vmstate_hda_audio_stream_buf,
+        NULL
     }
 };
 
@@ -615,6 +839,7 @@ static const VMStateDescription vmstate_hda_audio = {
 static Property hda_audio_properties[] = {
     DEFINE_PROP_UINT32("debug", HDAAudioState, debug,   0),
     DEFINE_PROP_BOOL("mixer", HDAAudioState, mixer,  true),
+    DEFINE_PROP_BOOL("use-timer", HDAAudioState, use_timer,  true),
     DEFINE_PROP_END_OF_LIST(),
 };
 

hw/audio/intel-hda.c

@@ -407,13 +407,6 @@ static bool intel_hda_xfer(HDACodecDevice *dev, uint32_t stnr, bool output,
     if (st->bpl == NULL) {
         return false;
     }
-    if (st->ctl & (1 << 26)) {
-        /*
-         * Wait with the next DMA xfer until the guest
-         * has acked the buffer completion interrupt
-         */
-        return false;
-    }
 
     left = len;
     s = st->bentries;

hw/audio/trace-events

@@ -17,3 +17,9 @@ milkymist_ac97_in_cb(int avail, uint32_t remaining) "avail %d remaining %u"
 milkymist_ac97_in_cb_transferred(int transferred) "transferred %d"
 milkymist_ac97_out_cb(int free, uint32_t remaining) "free %d remaining %u"
 milkymist_ac97_out_cb_transferred(int transferred) "transferred %d"
+
+# hw/audio/hda-codec.c
+hda_audio_running(const char *stream, int nr, bool running) "st %s, nr %d, run %d"
+hda_audio_format(const char *stream, int chan, const char *fmt, int freq) "st %s, %d x %s @ %d Hz"
+hda_audio_adjust(const char *stream, int pos) "st %s, pos %d"
+hda_audio_overrun(const char *stream) "st %s"

include/hw/compat.h

@@ -6,6 +6,10 @@
         .driver   = "migration",\
         .property = "decompress-error-check",\
         .value    = "off",\
+    },{\
+        .driver   = "hda-audio",\
+        .property = "use-timer",\
+        .value    = "false",\
     },
 
 #define HW_COMPAT_2_11 \