It turned out that the recent simplification of HD-audio bus access helpers caused a regression on the virtual HD-audio device on QEMU with ARM platforms. The driver got a CORB/RIRB timeout and couldn't probe any codecs.

The essential difference that caused a problem was the enforced aligned MMIO accesses by simplification. Since snd-hda-tegra driver is enabled on ARM, it enables CONFIG_SND_HDA_ALIGNED_MMIO, which makes the all HD-audio drivers using the aligned MMIO accesses. While this is mandatory for snd-hda-tegra, it seems that snd-hda-intel on ARM gets broken by this access pattern.

For addressing the regression, this patch introduces a new flag, aligned_mmio, to hdac_bus object, and applies the aligned MMIO only when this flag is set. This change affects only platforms with CONFIG_SND_HDA_ALIGNED_MMIO set, i.e. mostly only for ARM platforms.

Unfortunately the patch became a big bigger than it should be, just because the former calls didn't take hdac_bus object in the argument, hence we had to extend the call patterns.

Fixes: 19abfefd4c76 ("ALSA: hda: Direct MMIO accesses") BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1161152 Cc: stable@vger.kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/hdaudio.h | 77 ++++++++++++++++++++++++++++++++--------------- sound/pci/hda/hda_tegra.c | 1 + 2 files changed, 54 insertions(+), 24 deletions(-)

diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h index e05b95e83d5a..fb9dce4c6928 100644 --- a/include/sound/hdaudio.h +++ b/include/sound/hdaudio.h @@ -8,6 +8,7 @@

#include <linux/device.h> #include <linux/interrupt.h> +#include <linux/io.h> #include <linux/pm_runtime.h> #include <linux/timecounter.h> #include <sound/core.h> @@ -330,6 +331,7 @@ struct hdac_bus { bool chip_init:1; /* h/w initialized */

/* behavior flags */ + bool aligned_mmio:1; /* aligned MMIO access */ bool sync_write:1; /* sync after verb write */ bool use_posbuf:1; /* use position buffer */ bool snoop:1; /* enable snooping */ @@ -405,34 +407,61 @@ void snd_hdac_bus_free_stream_pages(struct hdac_bus *bus); unsigned int snd_hdac_aligned_read(void __iomem *addr, unsigned int mask); void snd_hdac_aligned_write(unsigned int val, void __iomem *addr, unsigned int mask); -#define snd_hdac_reg_writeb(v, addr) snd_hdac_aligned_write(v, addr, 0xff) -#define snd_hdac_reg_writew(v, addr) snd_hdac_aligned_write(v, addr, 0xffff) -#define snd_hdac_reg_readb(addr) snd_hdac_aligned_read(addr, 0xff) -#define snd_hdac_reg_readw(addr) snd_hdac_aligned_read(addr, 0xffff) -#else /* CONFIG_SND_HDA_ALIGNED_MMIO */ -#define snd_hdac_reg_writeb(val, addr) writeb(val, addr) -#define snd_hdac_reg_writew(val, addr) writew(val, addr) -#define snd_hdac_reg_readb(addr) readb(addr) -#define snd_hdac_reg_readw(addr) readw(addr) -#endif /* CONFIG_SND_HDA_ALIGNED_MMIO */ -#define snd_hdac_reg_writel(val, addr) writel(val, addr) -#define snd_hdac_reg_readl(addr) readl(addr) +#define snd_hdac_aligned_mmio(bus) (bus)->aligned_mmio +#else +#define snd_hdac_aligned_mmio(bus) false +#define snd_hdac_aligned_read(addr, mask) 0 +#define snd_hdac_aligned_write(val, addr, mask) do {} while (0) +#endif + +static inline void snd_hdac_reg_writeb(struct hdac_bus *bus, void __iomem *addr, + u8 val) +{ + if (snd_hdac_aligned_mmio(bus)) + snd_hdac_aligned_write(val, addr, 0xff); + else + writeb(val, addr); +} + +static inline void snd_hdac_reg_writew(struct hdac_bus *bus, void __iomem *addr, + u16 val) +{ + if (snd_hdac_aligned_mmio(bus)) + snd_hdac_aligned_write(val, addr, 0xffff); + else + writew(val, addr); +} + +static inline u8 snd_hdac_reg_readb(struct hdac_bus *bus, void __iomem *addr) +{ + return snd_hdac_aligned_mmio(bus) ? + snd_hdac_aligned_read(addr, 0xff) : readb(addr); +} + +static inline u16 snd_hdac_reg_readw(struct hdac_bus *bus, void __iomem *addr) +{ + return snd_hdac_aligned_mmio(bus) ? + snd_hdac_aligned_read(addr, 0xffff) : readw(addr); +} + +#define snd_hdac_reg_writel(bus, addr, val) writel(val, addr) +#define snd_hdac_reg_readl(bus, addr) readl(addr)

/* * macros for easy use */ #define _snd_hdac_chip_writeb(chip, reg, value) \ - snd_hdac_reg_writeb(value, (chip)->remap_addr + (reg)) + snd_hdac_reg_writeb(chip, (chip)->remap_addr + (reg), value) #define _snd_hdac_chip_readb(chip, reg) \ - snd_hdac_reg_readb((chip)->remap_addr + (reg)) + snd_hdac_reg_readb(chip, (chip)->remap_addr + (reg)) #define _snd_hdac_chip_writew(chip, reg, value) \ - snd_hdac_reg_writew(value, (chip)->remap_addr + (reg)) + snd_hdac_reg_writew(chip, (chip)->remap_addr + (reg), value) #define _snd_hdac_chip_readw(chip, reg) \ - snd_hdac_reg_readw((chip)->remap_addr + (reg)) + snd_hdac_reg_readw(chip, (chip)->remap_addr + (reg)) #define _snd_hdac_chip_writel(chip, reg, value) \ - snd_hdac_reg_writel(value, (chip)->remap_addr + (reg)) + snd_hdac_reg_writel(chip, (chip)->remap_addr + (reg), value) #define _snd_hdac_chip_readl(chip, reg) \ - snd_hdac_reg_readl((chip)->remap_addr + (reg)) + snd_hdac_reg_readl(chip, (chip)->remap_addr + (reg))

/* read/write a register, pass without AZX_REG_ prefix */ #define snd_hdac_chip_writel(chip, reg, value) \ @@ -540,17 +569,17 @@ int snd_hdac_get_stream_stripe_ctl(struct hdac_bus *bus, */ /* read/write a register, pass without AZX_REG_ prefix */ #define snd_hdac_stream_writel(dev, reg, value) \ - snd_hdac_reg_writel(value, (dev)->sd_addr + AZX_REG_ ## reg) + snd_hdac_reg_writel((dev)->bus, (dev)->sd_addr + AZX_REG_ ## reg, value) #define snd_hdac_stream_writew(dev, reg, value) \ - snd_hdac_reg_writew(value, (dev)->sd_addr + AZX_REG_ ## reg) + snd_hdac_reg_writew((dev)->bus, (dev)->sd_addr + AZX_REG_ ## reg, value) #define snd_hdac_stream_writeb(dev, reg, value) \ - snd_hdac_reg_writeb(value, (dev)->sd_addr + AZX_REG_ ## reg) + snd_hdac_reg_writeb((dev)->bus, (dev)->sd_addr + AZX_REG_ ## reg, value) #define snd_hdac_stream_readl(dev, reg) \ - snd_hdac_reg_readl((dev)->sd_addr + AZX_REG_ ## reg) + snd_hdac_reg_readl((dev)->bus, (dev)->sd_addr + AZX_REG_ ## reg) #define snd_hdac_stream_readw(dev, reg) \ - snd_hdac_reg_readw((dev)->sd_addr + AZX_REG_ ## reg) + snd_hdac_reg_readw((dev)->bus, (dev)->sd_addr + AZX_REG_ ## reg) #define snd_hdac_stream_readb(dev, reg) \ - snd_hdac_reg_readb((dev)->sd_addr + AZX_REG_ ## reg) + snd_hdac_reg_readb((dev)->bus, (dev)->sd_addr + AZX_REG_ ## reg)

/* update a register, pass without AZX_REG_ prefix */ #define snd_hdac_stream_updatel(dev, reg, mask, val) \ diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c index 8350954b7986..e5191584638a 100644 --- a/sound/pci/hda/hda_tegra.c +++ b/sound/pci/hda/hda_tegra.c @@ -398,6 +398,7 @@ static int hda_tegra_create(struct snd_card *card, return err;

chip->bus.needs_damn_long_delay = 1; + chip->bus.core.aligned_mmio = 1;

err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops); if (err < 0) {