[Qemu-devel] [PATCH 1/2] hda: do not mix output and input streams, RHBZ #740493

[Qemu-devel] [PATCH 1/2] hda: do not mix output and input streams, RHBZ #740493

[Marc-André Lureau]

Windows 7 may use the same stream number for input and output.
That will result in lot of garbage on playback.

The hardcoded value of 4 needs to be in sync with GCAP streams
description and IN/OUT registers.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/intel-hda.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index 4272204..c6a3fec 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -389,14 +389,15 @@ static bool intel_hda_xfer(HDACodecDevice *dev, uint32_t stnr, bool output,
 {
     HDACodecBus *bus = DO_UPCAST(HDACodecBus, qbus, dev->qdev.parent_bus);
     IntelHDAState *d = container_of(bus, IntelHDAState, codecs);
-    IntelHDAStream *st = NULL;
     target_phys_addr_t addr;
     uint32_t s, copy, left;
+    IntelHDAStream *st;
     bool irq = false;
 
-    for (s = 0; s < ARRAY_SIZE(d->st); s++) {
-        if (stnr == ((d->st[s].ctl >> 20) & 0x0f)) {
-            st = d->st + s;
+    st = output ? d->st + 4 : d->st;
+    for (s = 0; s < 4; s++) {
+        if (stnr == ((st[s].ctl >> 20) & 0x0f)) {
+            st = st + s;
             break;
         }
     }
-- 
1.7.6.2

[Qemu-devel] [PATCH 2/2] hda: do not mix output and input stream states, RHBZ #740493

[Marc-André Lureau]

Windows 7 may use the same stream number for input and output.
Current code will confuse streams.

Changes since v1:
- keep running_compat[] for migration version 1
- add running_real[] for migration version 2

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/hda-audio.c |   26 +++++++++++++++++++-------
 hw/intel-hda.c |    9 +++++----
 hw/intel-hda.h |    2 +-
 3 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/hw/hda-audio.c b/hw/hda-audio.c
index 03c0a24..a72b721 100644
--- a/hw/hda-audio.c
+++ b/hw/hda-audio.c
@@ -462,7 +462,8 @@ struct HDAAudioState {
     QEMUSoundCard card;
     const desc_codec *desc;
     HDAAudioStream st[4];
-    bool running[16];
+    bool running_compat[16];
+    bool running_real[2 * 16];
 
     /* properties */
     uint32_t debug;
@@ -659,7 +660,7 @@ static void hda_audio_command(HDACodecDevice *hda, uint32_t nid, uint32_t data)
         st->channel = payload & 0x0f;
         dprint(a, 2, "%s: stream %d, channel %d\n",
                st->node->name, st->stream, st->channel);
-        hda_audio_set_running(st, a->running[st->stream]);
+        hda_audio_set_running(st, a->running_real[st->output * 16 + st->stream]);
         hda_codec_response(hda, true, 0);
         break;
     case AC_VERB_GET_CONV:
@@ -742,16 +743,20 @@ fail:
     hda_codec_response(hda, true, 0);
 }
 
-static void hda_audio_stream(HDACodecDevice *hda, uint32_t stnr, bool running)
+static void hda_audio_stream(HDACodecDevice *hda, uint32_t stnr, bool running, bool output)
 {
     HDAAudioState *a = DO_UPCAST(HDAAudioState, hda, hda);
     int s;
 
-    a->running[stnr] = running;
+    a->running_compat[stnr] = running;
+    a->running_real[output * 16 + stnr] = running;
     for (s = 0; s < ARRAY_SIZE(a->st); s++) {
         if (a->st[s].node == NULL) {
             continue;
         }
+        if (a->st[s].output != output) {
+            continue;
+        }
         if (a->st[s].stream != stnr) {
             continue;
         }
@@ -833,6 +838,12 @@ static int hda_audio_post_load(void *opaque, int version)
     int i;
 
     dprint(a, 1, "%s\n", __FUNCTION__);
+    if (version == 1) {
+        /* assume running_compat[] is for output streams */
+        for (i = 0; i < ARRAY_SIZE(a->running_compat); i++)
+            a->running_real[16 + i] = a->running_compat[i];
+    }
+
     for (i = 0; i < ARRAY_SIZE(a->st); i++) {
         st = a->st + i;
         if (st->node == NULL)
@@ -840,7 +851,7 @@ static int hda_audio_post_load(void *opaque, int version)
         hda_codec_parse_fmt(st->format, &st->as);
         hda_audio_setup(st);
         hda_audio_set_amp(st);
-        hda_audio_set_running(st, a->running[st->stream]);
+        hda_audio_set_running(st, a->running_real[st->output * 16 + st->stream]);
     }
     return 0;
 }
@@ -864,13 +875,14 @@ static const VMStateDescription vmstate_hda_audio_stream = {
 
 static const VMStateDescription vmstate_hda_audio = {
     .name = "hda-audio",
-    .version_id = 1,
+    .version_id = 2,
     .post_load = hda_audio_post_load,
     .fields = (VMStateField []) {
         VMSTATE_STRUCT_ARRAY(st, HDAAudioState, 4, 0,
                              vmstate_hda_audio_stream,
                              HDAAudioStream),
-        VMSTATE_BOOL_ARRAY(running, HDAAudioState, 16),
+        VMSTATE_BOOL_ARRAY(running_compat, HDAAudioState, 16),
+        VMSTATE_BOOL_ARRAY_V(running_real, HDAAudioState, 2 * 16, 2),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index c6a3fec..f97775c 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -485,7 +485,7 @@ static void intel_hda_parse_bdl(IntelHDAState *d, IntelHDAStream *st)
     st->bp    = 0;
 }
 
-static void intel_hda_notify_codecs(IntelHDAState *d, uint32_t stream, bool running)
+static void intel_hda_notify_codecs(IntelHDAState *d, uint32_t stream, bool running, bool output)
 {
     DeviceState *qdev;
     HDACodecDevice *cdev;
@@ -493,7 +493,7 @@ static void intel_hda_notify_codecs(IntelHDAState *d, uint32_t stream, bool runn
     QLIST_FOREACH(qdev, &d->codecs.qbus.children, sibling) {
         cdev = DO_UPCAST(HDACodecDevice, qdev, qdev);
         if (cdev->info->stream) {
-            cdev->info->stream(cdev, stream, running);
+            cdev->info->stream(cdev, stream, running, output);
         }
     }
 }
@@ -567,6 +567,7 @@ static void intel_hda_set_ics(IntelHDAState *d, const IntelHDAReg *reg, uint32_t
 
 static void intel_hda_set_st_ctl(IntelHDAState *d, const IntelHDAReg *reg, uint32_t old)
 {
+    bool output = reg->stream >= 4;
     IntelHDAStream *st = d->st + reg->stream;
 
     if (st->ctl & 0x01) {
@@ -582,11 +583,11 @@ static void intel_hda_set_st_ctl(IntelHDAState *d, const IntelHDAReg *reg, uint3
             dprint(d, 1, "st #%d: start %d (ring buf %d bytes)\n",
                    reg->stream, stnr, st->cbl);
             intel_hda_parse_bdl(d, st);
-            intel_hda_notify_codecs(d, stnr, true);
+            intel_hda_notify_codecs(d, stnr, true, output);
         } else {
             /* stop */
             dprint(d, 1, "st #%d: stop %d\n", reg->stream, stnr);
-            intel_hda_notify_codecs(d, stnr, false);
+            intel_hda_notify_codecs(d, stnr, false, output);
         }
     }
     intel_hda_update_irq(d);
diff --git a/hw/intel-hda.h b/hw/intel-hda.h
index 4e44e38..65fd2a8 100644
--- a/hw/intel-hda.h
+++ b/hw/intel-hda.h
@@ -34,7 +34,7 @@ struct HDACodecDeviceInfo {
     int (*init)(HDACodecDevice *dev);
     int (*exit)(HDACodecDevice *dev);
     void (*command)(HDACodecDevice *dev, uint32_t nid, uint32_t data);
-    void (*stream)(HDACodecDevice *dev, uint32_t stnr, bool running);
+    void (*stream)(HDACodecDevice *dev, uint32_t stnr, bool running, bool output);
 };
 
 void hda_codec_bus_init(DeviceState *dev, HDACodecBus *bus,
-- 
1.7.6.2

Re: [Qemu-devel] [PATCH 1/2] hda: do not mix output and input streams, RHBZ #740493

[malc]

On Tue, 25 Oct 2011, Marc-Andr? Lureau wrote:

> Windows 7 may use the same stream number for input and output.
> That will result in lot of garbage on playback.
> 
> The hardcoded value of 4 needs to be in sync with GCAP streams
> description and IN/OUT registers.
> 
> Signed-off-by: Marc-Andr? Lureau <marcandre.lureau@redhat.com>
> ---
>  hw/intel-hda.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)

Applied.

[..snip..]

-- 
mailto:av1474@comtv.ru

[Qemu-devel] [PATCH 1/2] hda: do not mix output and input streams, RHBZ #740493

[Marc-André Lureau]

Windows 7 may use the same stream number for input and output.
That will result in lot of garbage on playback.

The hardcoded value of 4 needs to be in sync with GCAP streams
description and IN/OUT registers.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/intel-hda.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index 4272204..c6a3fec 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -389,14 +389,15 @@ static bool intel_hda_xfer(HDACodecDevice *dev, uint32_t stnr, bool output,
 {
     HDACodecBus *bus = DO_UPCAST(HDACodecBus, qbus, dev->qdev.parent_bus);
     IntelHDAState *d = container_of(bus, IntelHDAState, codecs);
-    IntelHDAStream *st = NULL;
     target_phys_addr_t addr;
     uint32_t s, copy, left;
+    IntelHDAStream *st;
     bool irq = false;
 
-    for (s = 0; s < ARRAY_SIZE(d->st); s++) {
-        if (stnr == ((d->st[s].ctl >> 20) & 0x0f)) {
-            st = d->st + s;
+    st = output ? d->st + 4 : d->st;
+    for (s = 0; s < 4; s++) {
+        if (stnr == ((st[s].ctl >> 20) & 0x0f)) {
+            st = st + s;
             break;
         }
     }
-- 
1.7.6.2

[Qemu-devel] [PATCH 0/2] HDA fixes for Windows 7

[Marc-André Lureau]

Hi,

There are two related bugs in stream state and xfer handling due to the
conflict of stream number (on Windows 7, input stream and output stream
both use the full range of 1..15 values. on Linux, the handling is
different and it works fine because there is no clash)

The patches handle stream number for input and output stream
distinctively to avoid conflict, and it seems to be in accordance
to the spec.

Marc-André Lureau (2):
  hda: do not mix output and input streams, RHBZ #740493
  hda: do not mix output and input stream states, RHBZ #740493

 hw/hda-audio.c |   15 +++++++++------
 hw/intel-hda.c |   18 ++++++++++--------
 hw/intel-hda.h |    2 +-
 3 files changed, 20 insertions(+), 15 deletions(-)

-- 
1.7.6.2

[Qemu-devel] [PATCH 1/2] hda: do not mix output and input streams, RHBZ #740493

[Marc-André Lureau]

Windows 7 may use the same stream number for input and output.
That will result in lot of garbage on playback.

The hardcoded value of 4 needs to be in sync with GCAP streams
description and IN/OUT registers.
---
 hw/intel-hda.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index 4272204..c6a3fec 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -389,14 +389,15 @@ static bool intel_hda_xfer(HDACodecDevice *dev, uint32_t stnr, bool output,
 {
     HDACodecBus *bus = DO_UPCAST(HDACodecBus, qbus, dev->qdev.parent_bus);
     IntelHDAState *d = container_of(bus, IntelHDAState, codecs);
-    IntelHDAStream *st = NULL;
     target_phys_addr_t addr;
     uint32_t s, copy, left;
+    IntelHDAStream *st;
     bool irq = false;
 
-    for (s = 0; s < ARRAY_SIZE(d->st); s++) {
-        if (stnr == ((d->st[s].ctl >> 20) & 0x0f)) {
-            st = d->st + s;
+    st = output ? d->st + 4 : d->st;
+    for (s = 0; s < 4; s++) {
+        if (stnr == ((st[s].ctl >> 20) & 0x0f)) {
+            st = st + s;
             break;
         }
     }
-- 
1.7.6.2