From e83f0b3efdb3c4abb82775301a51b62a36eb8254 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 1 Apr 2024 12:01:59 -0500 Subject: [PATCH 1/3] fixup! ALSA: hda: hdac_controller: Implement support for use_pio_for_commands mode When we use the PIO mode, we should not program anything related to CORB. Signed-off-by: Pierre-Louis Bossart --- sound/hda/hdac_controller.c | 43 +++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c index b5c833b9f8b9ca..d11cd26a2db5c4 100644 --- a/sound/hda/hdac_controller.c +++ b/sound/hda/hdac_controller.c @@ -45,25 +45,26 @@ void snd_hdac_bus_init_cmd_io(struct hdac_bus *bus) WARN_ON_ONCE(!bus->rb.area); spin_lock_irq(&bus->reg_lock); - /* CORB set up */ - bus->corb.addr = bus->rb.addr; - bus->corb.buf = (__le32 *)bus->rb.area; - snd_hdac_chip_writel(bus, CORBLBASE, (u32)bus->corb.addr); - snd_hdac_chip_writel(bus, CORBUBASE, upper_32_bits(bus->corb.addr)); - - /* set the corb size to 256 entries (ULI requires explicitly) */ - snd_hdac_chip_writeb(bus, CORBSIZE, 0x02); - /* set the corb write pointer to 0 */ - snd_hdac_chip_writew(bus, CORBWP, 0); - - /* reset the corb hw read pointer */ - snd_hdac_chip_writew(bus, CORBRP, AZX_CORBRP_RST); - if (!bus->corbrp_self_clear) - azx_clear_corbrp(bus); - - /* enable corb dma */ - if (!bus->use_pio_for_commands) + if (!bus->use_pio_for_commands) { + /* CORB set up */ + bus->corb.addr = bus->rb.addr; + bus->corb.buf = (__le32 *)bus->rb.area; + snd_hdac_chip_writel(bus, CORBLBASE, (u32)bus->corb.addr); + snd_hdac_chip_writel(bus, CORBUBASE, upper_32_bits(bus->corb.addr)); + + /* set the corb size to 256 entries (ULI requires explicitly) */ + snd_hdac_chip_writeb(bus, CORBSIZE, 0x02); + /* set the corb write pointer to 0 */ + snd_hdac_chip_writew(bus, CORBWP, 0); + + /* reset the corb hw read pointer */ + snd_hdac_chip_writew(bus, CORBRP, AZX_CORBRP_RST); + if (!bus->corbrp_self_clear) + azx_clear_corbrp(bus); + + /* enable corb dma */ snd_hdac_chip_writeb(bus, CORBCTL, AZX_CORBCTL_RUN); + } /* RIRB set up */ bus->rirb.addr = bus->rb.addr + 2048; @@ -100,6 +101,9 @@ static void hdac_wait_for_cmd_dmas(struct hdac_bus *bus) && time_before(jiffies, timeout)) udelay(10); + if (bus->use_pio_for_commands) + return; + timeout = jiffies + msecs_to_jiffies(100); while ((snd_hdac_chip_readb(bus, CORBCTL) & AZX_CORBCTL_RUN) && time_before(jiffies, timeout)) @@ -115,7 +119,8 @@ void snd_hdac_bus_stop_cmd_io(struct hdac_bus *bus) spin_lock_irq(&bus->reg_lock); /* disable ringbuffer DMAs */ snd_hdac_chip_writeb(bus, RIRBCTL, 0); - snd_hdac_chip_writeb(bus, CORBCTL, 0); + if (!bus->use_pio_for_commands) + snd_hdac_chip_writeb(bus, CORBCTL, 0); spin_unlock_irq(&bus->reg_lock); hdac_wait_for_cmd_dmas(bus); From e53d3127c8c1445bb64329ef1578bd454be01b5d Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 1 Apr 2024 12:23:26 -0500 Subject: [PATCH 2/3] [HACK] ASoC: SOF: Intel: disable interrupts last This doesn't seem to have any effect on the controller reset issue, but I wonder why we disable interrupts and then stop the commands. This sounds backwards, the commands first need to complete and then the interrupts should be disabled? Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/intel/hda-ctrl.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/sound/soc/sof/intel/hda-ctrl.c b/sound/soc/sof/intel/hda-ctrl.c index 18018233e23774..243da67ff1c2bf 100644 --- a/sound/soc/sof/intel/hda-ctrl.c +++ b/sound/soc/sof/intel/hda-ctrl.c @@ -284,10 +284,6 @@ void hda_dsp_ctrl_stop_chip(struct snd_sof_dev *sdev) snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_INTCTL, SOF_HDA_INT_ALL_STREAM, 0); - /* disable controller CIE and GIE */ - snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_INTCTL, - SOF_HDA_INT_CTRL_EN | SOF_HDA_INT_GLOBAL_EN, - 0); /* clear stream status */ list_for_each_entry(stream, &bus->stream_list, list) { @@ -317,6 +313,11 @@ void hda_dsp_ctrl_stop_chip(struct snd_sof_dev *sdev) SOF_HDA_ADSP_DPUBASE, 0); } + /* disable controller CIE and GIE */ + snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_INTCTL, + SOF_HDA_INT_CTRL_EN | SOF_HDA_INT_GLOBAL_EN, + 0); + bus->chip_init = false; } From fb332ed594faf01b0b60a9d78fd645dda4848e1e Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 1 Apr 2024 14:49:50 -0500 Subject: [PATCH 3/3] ASoC: SOF: Intel: hda-ctrl: add missing WAKE_STS clear For some reason, the programming sequences in the SOF driver do not include the required clear of the WAKE_STS bits when the controller is not in reset. Adding this sequence avoids a regression on LunarLake when using PIO commands. The correlation or causation is not clear, but there was no reason in the first place to deviate from the recommended programming sequence. Closes: https://github.com/thesofproject/linux/issues/4889 Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/intel/hda-ctrl.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sound/soc/sof/intel/hda-ctrl.c b/sound/soc/sof/intel/hda-ctrl.c index 243da67ff1c2bf..8003e8f5df3f46 100644 --- a/sound/soc/sof/intel/hda-ctrl.c +++ b/sound/soc/sof/intel/hda-ctrl.c @@ -188,6 +188,7 @@ int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev) struct hdac_bus *bus = sof_to_bus(sdev); struct hdac_stream *stream; int sd_offset, ret = 0; + u32 gctl; if (bus->chip_init) return 0; @@ -196,6 +197,12 @@ int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev) hda_dsp_ctrl_misc_clock_gating(sdev, false); + /* clear WAKE_STS if not in reset */ + gctl = snd_sof_dsp_read(sdev, HDA_DSP_HDA_BAR, SOF_HDA_GCTL); + if (gctl & SOF_HDA_GCTL_RESET) + snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, + SOF_HDA_WAKESTS, SOF_HDA_WAKESTS_INT_MASK); + /* reset HDA controller */ ret = hda_dsp_ctrl_link_reset(sdev, true); if (ret < 0) {