* [Qemu-devel] [PATCH 0/2] HDA fixes for Windows 7 @ 2011-10-04 16:07 Marc-André Lureau 2011-10-04 16:07 ` [Qemu-devel] [PATCH 1/2] hda: do not mix output and input streams, RHBZ #740493 Marc-André Lureau 2011-10-04 16:07 ` [Qemu-devel] [PATCH 2/2] hda: do not mix output and input stream states, " Marc-André Lureau 0 siblings, 2 replies; 8+ messages in thread From: Marc-André Lureau @ 2011-10-04 16:07 UTC (permalink / raw) To: qemu-devel; +Cc: yhalperi, alevy, kraxel, 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 1/2] hda: do not mix output and input streams, RHBZ #740493 2011-10-04 16:07 [Qemu-devel] [PATCH 0/2] HDA fixes for Windows 7 Marc-André Lureau @ 2011-10-04 16:07 ` Marc-André Lureau 2011-10-04 16:07 ` [Qemu-devel] [PATCH 2/2] hda: do not mix output and input stream states, " Marc-André Lureau 1 sibling, 0 replies; 8+ messages in thread From: Marc-André Lureau @ 2011-10-04 16:07 UTC (permalink / raw) To: qemu-devel; +Cc: yhalperi, alevy, kraxel, 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 2/2] hda: do not mix output and input stream states, RHBZ #740493 2011-10-04 16:07 [Qemu-devel] [PATCH 0/2] HDA fixes for Windows 7 Marc-André Lureau 2011-10-04 16:07 ` [Qemu-devel] [PATCH 1/2] hda: do not mix output and input streams, RHBZ #740493 Marc-André Lureau @ 2011-10-04 16:07 ` Marc-André Lureau 2011-10-04 17:21 ` Juan Quintela 1 sibling, 1 reply; 8+ messages in thread From: Marc-André Lureau @ 2011-10-04 16:07 UTC (permalink / raw) To: qemu-devel; +Cc: yhalperi, alevy, kraxel, Marc-André Lureau Windows 7 may use the same stream number for input and output. Current code will confuse streams. NB: I wonder if this patch breaks migration code because of this change: - VMSTATE_BOOL_ARRAY(running, HDAAudioState, 16), + VMSTATE_BOOL_ARRAY(running, HDAAudioState, 2 * 16), --- hw/hda-audio.c | 15 +++++++++------ hw/intel-hda.c | 9 +++++---- hw/intel-hda.h | 2 +- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/hw/hda-audio.c b/hw/hda-audio.c index 03c0a24..dacf290 100644 --- a/hw/hda-audio.c +++ b/hw/hda-audio.c @@ -462,7 +462,7 @@ struct HDAAudioState { QEMUSoundCard card; const desc_codec *desc; HDAAudioStream st[4]; - bool running[16]; + bool running[2 * 16]; /* properties */ uint32_t debug; @@ -659,7 +659,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[st->output * 16 + st->stream]); hda_codec_response(hda, true, 0); break; case AC_VERB_GET_CONV: @@ -742,16 +742,19 @@ 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[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; } @@ -840,7 +843,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[st->output * 16 + st->stream]); } return 0; } @@ -870,7 +873,7 @@ static const VMStateDescription vmstate_hda_audio = { VMSTATE_STRUCT_ARRAY(st, HDAAudioState, 4, 0, vmstate_hda_audio_stream, HDAAudioStream), - VMSTATE_BOOL_ARRAY(running, HDAAudioState, 16), + VMSTATE_BOOL_ARRAY(running, HDAAudioState, 2 * 16), 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] hda: do not mix output and input stream states, RHBZ #740493 2011-10-04 16:07 ` [Qemu-devel] [PATCH 2/2] hda: do not mix output and input stream states, " Marc-André Lureau @ 2011-10-04 17:21 ` Juan Quintela 2011-10-05 1:43 ` Marc-André Lureau 0 siblings, 1 reply; 8+ messages in thread From: Juan Quintela @ 2011-10-04 17:21 UTC (permalink / raw) To: Marc-André Lureau Cc: alevy, yhalperi, qemu-devel, Marc-André Lureau, kraxel "Marc-André Lureau" <marcandre.lureau@gmail.com> wrote: > Windows 7 may use the same stream number for input and output. > Current code will confuse streams. > > NB: I wonder if this patch breaks migration code because of > this change: > - VMSTATE_BOOL_ARRAY(running, HDAAudioState, 16), > + VMSTATE_BOOL_ARRAY(running, HDAAudioState, 2 * 16), It does :-( Two questions to know how to handle this. a) My understanding of this patch is that we move from an array of 16 bools representing anything to one array where the 1st 16 represent if there are input and the 2nd 16's reprosenting if there are output for that channel. So, what we should do if we migrate from one old version that only has 16 bools? My understanding is that copying directly is not gonna work? b) Just in case it makes "compatibility" easier. Is the running array normally all false or similar? If so, we could just sent the other 16th values only if they are different of false. (if this is true, it is interesting to put in the 1st slots the ones that are more probable to be true). Later, Juan. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] hda: do not mix output and input stream states, RHBZ #740493 2011-10-04 17:21 ` Juan Quintela @ 2011-10-05 1:43 ` Marc-André Lureau 2011-10-14 11:38 ` Gerd Hoffmann 0 siblings, 1 reply; 8+ messages in thread From: Marc-André Lureau @ 2011-10-05 1:43 UTC (permalink / raw) To: quintela; +Cc: alevy, yhalperi, qemu-devel, Marc-André Lureau, kraxel Hi On Tue, Oct 4, 2011 at 7:21 PM, Juan Quintela <quintela@redhat.com> wrote: > "Marc-André Lureau" <marcandre.lureau@gmail.com> wrote: >> Windows 7 may use the same stream number for input and output. >> Current code will confuse streams. >> >> NB: I wonder if this patch breaks migration code because of >> this change: >> - VMSTATE_BOOL_ARRAY(running, HDAAudioState, 16), >> + VMSTATE_BOOL_ARRAY(running, HDAAudioState, 2 * 16), > > It does :-( > > Two questions to know how to handle this. > a) My understanding of this patch is that we move from an array of 16 > bools representing anything to one array where the 1st 16 represent if > there are input and the 2nd 16's reprosenting if there are output for > that channel. > > So, what we should do if we migrate from one old version that only has > 16 bools? My understanding is that copying directly is not gonna work? We might assume that the array is for output stream, and discard any running input stream. Or we can be slightly smarter and try to figure out the running HDAAudioStream (which by the way I don't know why there are 4 and not 8 or 2) and map this to a input and output arrays correctly, except in complex cases. For instance, if there is only running[1] == true but we have output st[0].running and input st[1].running we can map this correctly. But in complex cases that won't probably work. Just to be clear and honest, I am not sure how the HDA code works, I am new to all this, and I use a lot of guesses and skimming the spec. So for now, I don't think it will be a regression if we try to do a smart mapping. I suppose there is a hook after VM migration finish where we could apply this logic, but can we break the bitstrean and migrate to newer versions safely? Other possibility is to avoid the issue alltogether by ignoring running streams and stopping them. From my tests OS seems to deal with that situation just fine. Next time you play/record something it will run again. > b) Just in case it makes "compatibility" easier. > > Is the running array normally all false or similar? If so, we could > just sent the other 16th values only if they are different of false. > (if this is true, it is interesting to put in the 1st slots the ones > that are more probable to be true). Yes, they are usually all false but one, if playback or recording on going. But I am not follwing, could you give an example of how you would change the array? Wouldn't that break migration? regards -- Marc-André Lureau ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] hda: do not mix output and input stream states, RHBZ #740493 2011-10-05 1:43 ` Marc-André Lureau @ 2011-10-14 11:38 ` Gerd Hoffmann 0 siblings, 0 replies; 8+ messages in thread From: Gerd Hoffmann @ 2011-10-14 11:38 UTC (permalink / raw) To: Marc-André Lureau Cc: alevy, yhalperi, qemu-devel, Marc-André Lureau, quintela Hi, >> a) My understanding of this patch is that we move from an array of 16 >> bools representing anything to one array where the 1st 16 represent if >> there are input and the 2nd 16's reprosenting if there are output for >> that channel. Correct. >> So, what we should do if we migrate from one old version that only has >> 16 bools? My understanding is that copying directly is not gonna work? Yes. Putting output first increases the chance that it works as sound playback is used much more than sound recording. I think hda-audio doesn't need so safe any running state. intel-hda knows which streams are running and it can call intel_hda_notify_codecs() for each stream in intel_hda_post_load(). The only problem is that intel_hda_post_load() might get called before hda-audio state is loaded. Now how to handle compatibility? We can have a running_compat[] array with the current (broken) semantics, so we can write out the state which older qemu versions expect to see. Also on load running_compat[] will be filled, and any state already set by intel_hda_post_load in running_real[] (or however we'll name this) will be kept intact. cheers, Gerd ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 1/2] hda: do not mix output and input streams, RHBZ #740493 @ 2011-10-17 10:58 Marc-André Lureau 2011-10-17 10:58 ` [Qemu-devel] [PATCH 2/2] hda: do not mix output and input stream states, " Marc-André Lureau 0 siblings, 1 reply; 8+ messages in thread From: Marc-André Lureau @ 2011-10-17 10:58 UTC (permalink / raw) To: qemu-devel; +Cc: Marc-André Lureau, kraxel 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 2/2] hda: do not mix output and input stream states, RHBZ #740493 2011-10-17 10:58 [Qemu-devel] [PATCH 1/2] hda: do not mix output and input streams, " Marc-André Lureau @ 2011-10-17 10:58 ` Marc-André Lureau 0 siblings, 0 replies; 8+ messages in thread From: Marc-André Lureau @ 2011-10-17 10:58 UTC (permalink / raw) To: qemu-devel; +Cc: Marc-André Lureau, kraxel 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 1/2] hda: do not mix output and input streams, RHBZ #740493 @ 2011-10-25 14:53 Marc-André Lureau 2011-10-25 14:53 ` [Qemu-devel] [PATCH 2/2] hda: do not mix output and input stream states, " Marc-André Lureau 0 siblings, 1 reply; 8+ messages in thread From: Marc-André Lureau @ 2011-10-25 14:53 UTC (permalink / raw) To: qemu-devel; +Cc: Marc-André Lureau, kraxel 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 2/2] hda: do not mix output and input stream states, RHBZ #740493 2011-10-25 14:53 [Qemu-devel] [PATCH 1/2] hda: do not mix output and input streams, " Marc-André Lureau @ 2011-10-25 14:53 ` Marc-André Lureau 0 siblings, 0 replies; 8+ messages in thread From: Marc-André Lureau @ 2011-10-25 14:53 UTC (permalink / raw) To: qemu-devel; +Cc: Marc-André Lureau, kraxel 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-10-25 14:53 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-10-04 16:07 [Qemu-devel] [PATCH 0/2] HDA fixes for Windows 7 Marc-André Lureau 2011-10-04 16:07 ` [Qemu-devel] [PATCH 1/2] hda: do not mix output and input streams, RHBZ #740493 Marc-André Lureau 2011-10-04 16:07 ` [Qemu-devel] [PATCH 2/2] hda: do not mix output and input stream states, " Marc-André Lureau 2011-10-04 17:21 ` Juan Quintela 2011-10-05 1:43 ` Marc-André Lureau 2011-10-14 11:38 ` Gerd Hoffmann -- strict thread matches above, loose matches on Subject: below -- 2011-10-17 10:58 [Qemu-devel] [PATCH 1/2] hda: do not mix output and input streams, " Marc-André Lureau 2011-10-17 10:58 ` [Qemu-devel] [PATCH 2/2] hda: do not mix output and input stream states, " Marc-André Lureau 2011-10-25 14:53 [Qemu-devel] [PATCH 1/2] hda: do not mix output and input streams, " Marc-André Lureau 2011-10-25 14:53 ` [Qemu-devel] [PATCH 2/2] hda: do not mix output and input stream states, " Marc-André Lureau
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).