[Qemu-devel] [PATCH v2 0/6] intel-hda: bugfixes and msi support

[Qemu-devel] [PATCH v2 0/6] intel-hda: bugfixes and msi support

[Gerd Hoffmann]

  Hi,

This patch series brings a bunch of bugfixes for the intel-hda driver
and it also adds MSI support to reduce interrupt sharing.

Changes in v2:
  * Codestyle fixups.
  * Added one more bugfix.
  * Added MSI patch.

François Revol (1):
  intel-hda: Honor WAKEEN bits.

Gerd Hoffmann (5):
  intel-hda: exit cleanup
  hda-audio: exit cleanup
  intel-hda: update irq status on WAKEEN changes.
  intel-hda: add msi support
  intel-hda: fix codec addressing.

 hw/hda-audio.c |   24 +++++++++++++++++++
 hw/intel-hda.c |   68 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 hw/intel-hda.h |    1 +
 3 files changed, 87 insertions(+), 6 deletions(-)

[Qemu-devel] [PATCH v2 1/6] intel-hda: exit cleanup

[Gerd Hoffmann]

Add pci exit callback for the intel-hda device and cleanup properly.
Also add an exit callback to the HDA bus implementation and make sure
it is called on qdev_free().

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/intel-hda.c |   20 ++++++++++++++++++++
 hw/intel-hda.h |    1 +
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index ccb059d..78c32da 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -61,9 +61,20 @@ static int hda_codec_dev_init(DeviceState *qdev, DeviceInfo *base)
     return info->init(dev);
 }
 
+static int hda_codec_dev_exit(DeviceState *qdev)
+{
+    HDACodecDevice *dev = DO_UPCAST(HDACodecDevice, qdev, qdev);
+
+    if (dev->info->exit) {
+        dev->info->exit(dev);
+    }
+    return 0;
+}
+
 void hda_codec_register(HDACodecDeviceInfo *info)
 {
     info->qdev.init = hda_codec_dev_init;
+    info->qdev.exit = hda_codec_dev_exit;
     info->qdev.bus_info = &hda_codec_bus_info;
     qdev_register(&info->qdev);
 }
@@ -1137,6 +1148,14 @@ static int intel_hda_init(PCIDevice *pci)
     return 0;
 }
 
+static int intel_hda_exit(PCIDevice *pci)
+{
+    IntelHDAState *d = DO_UPCAST(IntelHDAState, pci, pci);
+
+    cpu_unregister_io_memory(d->mmio_addr);
+    return 0;
+}
+
 static int intel_hda_post_load(void *opaque, int version)
 {
     IntelHDAState* d = opaque;
@@ -1219,6 +1238,7 @@ static PCIDeviceInfo intel_hda_info = {
     .qdev.vmsd    = &vmstate_intel_hda,
     .qdev.reset   = intel_hda_reset,
     .init         = intel_hda_init,
+    .exit         = intel_hda_exit,
     .qdev.props   = (Property[]) {
         DEFINE_PROP_UINT32("debug", IntelHDAState, debug, 0),
         DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/intel-hda.h b/hw/intel-hda.h
index ba290ec..4e44e38 100644
--- a/hw/intel-hda.h
+++ b/hw/intel-hda.h
@@ -32,6 +32,7 @@ struct HDACodecDevice {
 struct HDACodecDeviceInfo {
     DeviceInfo qdev;
     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);
 };
-- 
1.7.1

[Qemu-devel] [PATCH v2 2/6] hda-audio: exit cleanup

[Gerd Hoffmann]

Add exit callback to the driver.  Unregister the sound card properly
on exit.

[ v2: codestyle: add braces ]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/hda-audio.c |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/hw/hda-audio.c b/hw/hda-audio.c
index 1035774..c699d6f 100644
--- a/hw/hda-audio.c
+++ b/hw/hda-audio.c
@@ -808,6 +808,28 @@ static int hda_audio_init(HDACodecDevice *hda, const struct desc_codec *desc)
     return 0;
 }
 
+static int hda_audio_exit(HDACodecDevice *hda)
+{
+    HDAAudioState *a = DO_UPCAST(HDAAudioState, hda, hda);
+    HDAAudioStream *st;
+    int i;
+
+    dprint(a, 1, "%s\n", __FUNCTION__);
+    for (i = 0; i < ARRAY_SIZE(a->st); i++) {
+        st = a->st + i;
+        if (st->node == NULL) {
+            continue;
+        }
+        if (st->output) {
+            AUD_close_out(&a->card, st->voice.out);
+        } else {
+            AUD_close_in(&a->card, st->voice.in);
+        }
+    }
+    AUD_remove_card(&a->card);
+    return 0;
+}
+
 static int hda_audio_post_load(void *opaque, int version)
 {
     HDAAudioState *a = opaque;
@@ -879,6 +901,7 @@ static HDACodecDeviceInfo hda_audio_info_output = {
     .qdev.vmsd    = &vmstate_hda_audio,
     .qdev.props   = hda_audio_properties,
     .init         = hda_audio_init_output,
+    .exit         = hda_audio_exit,
     .command      = hda_audio_command,
     .stream       = hda_audio_stream,
 };
@@ -890,6 +913,7 @@ static HDACodecDeviceInfo hda_audio_info_duplex = {
     .qdev.vmsd    = &vmstate_hda_audio,
     .qdev.props   = hda_audio_properties,
     .init         = hda_audio_init_duplex,
+    .exit         = hda_audio_exit,
     .command      = hda_audio_command,
     .stream       = hda_audio_stream,
 };
-- 
1.7.1

[Qemu-devel] [PATCH v2 3/6] intel-hda: Honor WAKEEN bits.

[Gerd Hoffmann]

From: François Revol <revol@free.fr>

HDA: Honor WAKEEN bits when deciding to raise an interrupt on codec
status change.  This prevents an interrupt storm with the Haiku HDA
driver which does not handle codec status changes in the irq handler.

Signed-off-by: François Revol <revol@free.fr>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/intel-hda.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index 78c32da..2c1ef12 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -246,7 +246,7 @@ static void intel_hda_update_int_sts(IntelHDAState *d)
     if (d->rirb_sts & ICH6_RBSTS_OVERRUN) {
         sts |= (1 << 30);
     }
-    if (d->state_sts) {
+    if (d->state_sts & d->wake_en) {
         sts |= (1 << 30);
     }
 
@@ -628,6 +628,7 @@ static const struct IntelHDAReg regtab[] = {
     [ ICH6_REG_WAKEEN ] = {
         .name     = "WAKEEN",
         .size     = 2,
+        .wmask    = 0x3fff,
         .offset   = offsetof(IntelHDAState, wake_en),
     },
     [ ICH6_REG_STATESTS ] = {
-- 
1.7.1

[Qemu-devel] [PATCH v2 4/6] intel-hda: update irq status on WAKEEN changes.

[Gerd Hoffmann]

When the guest updates the WAKEEN register we
must re-calculate the IRQ status.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/intel-hda.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index 2c1ef12..e478e67 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -508,6 +508,11 @@ static void intel_hda_set_g_ctl(IntelHDAState *d, const IntelHDAReg *reg, uint32
     }
 }
 
+static void intel_hda_set_wake_en(IntelHDAState *d, const IntelHDAReg *reg, uint32_t old)
+{
+    intel_hda_update_irq(d);
+}
+
 static void intel_hda_set_state_sts(IntelHDAState *d, const IntelHDAReg *reg, uint32_t old)
 {
     intel_hda_update_irq(d);
@@ -630,6 +635,7 @@ static const struct IntelHDAReg regtab[] = {
         .size     = 2,
         .wmask    = 0x3fff,
         .offset   = offsetof(IntelHDAState, wake_en),
+        .whandler = intel_hda_set_wake_en,
     },
     [ ICH6_REG_STATESTS ] = {
         .name     = "STATESTS",
-- 
1.7.1

[Qemu-devel] [PATCH v2 5/6] intel-hda: add msi support

[Gerd Hoffmann]

This patch adds MSI support to the intel hda audio driver.  It is
enabled by default, use '-device intel-hda,msi=0' to disable it.

[ v2: codestyle: add braces ]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/intel-hda.c |   33 +++++++++++++++++++++++++++++++--
 1 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index e478e67..5e13dc3 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -19,6 +19,7 @@
 
 #include "hw.h"
 #include "pci.h"
+#include "msi.h"
 #include "qemu-timer.h"
 #include "audiodev.h"
 #include "intel-hda.h"
@@ -188,6 +189,7 @@ struct IntelHDAState {
 
     /* properties */
     uint32_t debug;
+    uint32_t msi;
 };
 
 struct IntelHDAReg {
@@ -268,6 +270,7 @@ static void intel_hda_update_int_sts(IntelHDAState *d)
 
 static void intel_hda_update_irq(IntelHDAState *d)
 {
+    int msi = d->msi && msi_enabled(&d->pci);
     int level;
 
     intel_hda_update_int_sts(d);
@@ -276,8 +279,15 @@ static void intel_hda_update_irq(IntelHDAState *d)
     } else {
         level = 0;
     }
-    dprint(d, 2, "%s: level %d\n", __FUNCTION__, level);
-    qemu_set_irq(d->pci.irq[0], level);
+    dprint(d, 2, "%s: level %d [%s]\n", __FUNCTION__,
+           level, msi ? "msi" : "intx");
+    if (msi) {
+        if (level) {
+            msi_notify(&d->pci, 0);
+        }
+    } else {
+        qemu_set_irq(d->pci.irq[0], level);
+    }
 }
 
 static int intel_hda_send_command(IntelHDAState *d, uint32_t verb)
@@ -1148,6 +1158,9 @@ static int intel_hda_init(PCIDevice *pci)
                                           intel_hda_mmio_write, d);
     pci_register_bar(&d->pci, 0, 0x4000, PCI_BASE_ADDRESS_SPACE_MEMORY,
                      intel_hda_map);
+    if (d->msi) {
+        msi_init(&d->pci, 0x50, 1, true, false);
+    }
 
     hda_codec_bus_init(&d->pci.qdev, &d->codecs,
                        intel_hda_response, intel_hda_xfer);
@@ -1159,10 +1172,24 @@ static int intel_hda_exit(PCIDevice *pci)
 {
     IntelHDAState *d = DO_UPCAST(IntelHDAState, pci, pci);
 
+    if (d->msi) {
+        msi_uninit(&d->pci);
+    }
     cpu_unregister_io_memory(d->mmio_addr);
     return 0;
 }
 
+static void intel_hda_write_config(PCIDevice *pci, uint32_t addr,
+                                   uint32_t val, int len)
+{
+    IntelHDAState *d = DO_UPCAST(IntelHDAState, pci, pci);
+
+    pci_default_write_config(pci, addr, val, len);
+    if (d->msi) {
+        msi_write_config(pci, addr, val, len);
+    }
+}
+
 static int intel_hda_post_load(void *opaque, int version)
 {
     IntelHDAState* d = opaque;
@@ -1246,8 +1273,10 @@ static PCIDeviceInfo intel_hda_info = {
     .qdev.reset   = intel_hda_reset,
     .init         = intel_hda_init,
     .exit         = intel_hda_exit,
+    .config_write = intel_hda_write_config,
     .qdev.props   = (Property[]) {
         DEFINE_PROP_UINT32("debug", IntelHDAState, debug, 0),
+        DEFINE_PROP_UINT32("msi", IntelHDAState, msi, 1),
         DEFINE_PROP_END_OF_LIST(),
     }
 };
-- 
1.7.1

[Qemu-devel] [PATCH v2 6/6] intel-hda: fix codec addressing.

[Gerd Hoffmann]

The HDA bus supports up to 15 codecs, with addresses 0 ... 14.
We get that wrong in two places:

 * When handing out addresses we accept address 15 as valid.
 * The bitmasks for two registers (WAKEEN and STATESTS) don't
   have bit 14 set.

This patch fixes it.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/intel-hda.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index 5e13dc3..b34b140 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -56,7 +56,7 @@ static int hda_codec_dev_init(DeviceState *qdev, DeviceInfo *base)
     if (dev->cad == -1) {
         dev->cad = bus->next_cad;
     }
-    if (dev->cad > 15)
+    if (dev->cad >= 15)
         return -1;
     bus->next_cad = dev->cad + 1;
     return info->init(dev);
@@ -643,15 +643,15 @@ static const struct IntelHDAReg regtab[] = {
     [ ICH6_REG_WAKEEN ] = {
         .name     = "WAKEEN",
         .size     = 2,
-        .wmask    = 0x3fff,
+        .wmask    = 0x7fff,
         .offset   = offsetof(IntelHDAState, wake_en),
         .whandler = intel_hda_set_wake_en,
     },
     [ ICH6_REG_STATESTS ] = {
         .name     = "STATESTS",
         .size     = 2,
-        .wmask    = 0x3fff,
-        .wclear   = 0x3fff,
+        .wmask    = 0x7fff,
+        .wclear   = 0x7fff,
         .offset   = offsetof(IntelHDAState, state_sts),
         .whandler = intel_hda_set_state_sts,
     },
-- 
1.7.1

Re: [Qemu-devel] [PATCH v2 0/6] intel-hda: bugfixes and msi support

[malc]

On Tue, 9 Nov 2010, Gerd Hoffmann wrote:

>   Hi,
> 
> This patch series brings a bunch of bugfixes for the intel-hda driver
> and it also adds MSI support to reduce interrupt sharing.

Applied.

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH v2 6/6] intel-hda: fix codec addressing.

[malc]

On Tue, 9 Nov 2010, Gerd Hoffmann wrote:

> The HDA bus supports up to 15 codecs, with addresses 0 ... 14.
> We get that wrong in two places:
> 
>  * When handing out addresses we accept address 15 as valid.
>  * The bitmasks for two registers (WAKEEN and STATESTS) don't
>    have bit 14 set.
> 
> This patch fixes it.
> 

I have applied and the reverted this one.

> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/intel-hda.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/intel-hda.c b/hw/intel-hda.c
> index 5e13dc3..b34b140 100644
> --- a/hw/intel-hda.c
> +++ b/hw/intel-hda.c
> @@ -56,7 +56,7 @@ static int hda_codec_dev_init(DeviceState *qdev, DeviceInfo *base)
>      if (dev->cad == -1) {
>          dev->cad = bus->next_cad;
>      }
> -    if (dev->cad > 15)
> +    if (dev->cad >= 15)
>          return -1;

Since there are no braces here.

-- 
mailto:av1474@comtv.ru