[PATCH 0/8] ASoC/soundwire: revisit interrupt and lcount handling
The code in drivers/soundwire/intel_init.c is hardware-dependent and the code does not apply to new generations starting with MeteorLake. Refactor and clean-up the code to make this intel_init.c hardware-agnostic and move all hardware-dependencies in the SOF driver using chip descriptors.
The ASoC patches are dependent on some patches that are applied to ASoC tree recently. So, this series won't apply to SoundWire tree. @Vinod Could you Ack if it looks good to you, and lets go through ASoC tree?
Pierre-Louis Bossart (8): soundwire: intel_init: remove useless interrupt enablement in interrupt thread ASoC: SOF: Intel: hda: add per-chip enable_sdw_irq() callback ASoC: SOF: Intel: mtl: factor interrupt enable/disable interrupt functions ASoC: SOF: Intel: mtl: move SoundWire interrupt enabling to callback ASoC: SOF: Intel: hda: add callback to check SoundWire lcount information soundwire: intel_init: remove sdw_intel_enable_irq() soundwire: intel_init: remove check on number of links ASoC: SOF: Intel: hda: read multi-link capabilities earlier
drivers/soundwire/intel_init.c | 37 -------- include/linux/soundwire/sdw_intel.h | 2 - sound/soc/sof/intel/cnl.c | 4 + sound/soc/sof/intel/hda.c | 63 ++++++++++++- sound/soc/sof/intel/hda.h | 12 +++ sound/soc/sof/intel/icl.c | 2 + sound/soc/sof/intel/mtl.c | 131 +++++++++++----------------- sound/soc/sof/intel/shim.h | 2 + sound/soc/sof/intel/tgl.c | 8 ++ 9 files changed, 139 insertions(+), 122 deletions(-)
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
When the code reaches the SoundWire interrupt thread handling, the interrupt was enabled already, and there is no code that disables it -> this is a no-op sequence.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com --- drivers/soundwire/intel_init.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index d091513919df..8bd95c9cbcaf 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -157,7 +157,6 @@ irqreturn_t sdw_intel_thread(int irq, void *dev_id) list_for_each_entry(link, &ctx->link_list, list) sdw_cdns_irq(irq, link->cdns);
- sdw_intel_enable_irq(ctx->mmio_base, true); return IRQ_HANDLED; } EXPORT_SYMBOL_NS(sdw_intel_thread, SOUNDWIRE_INTEL_INIT);
On 11-11-22, 12:26, Bard Liao wrote:
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
When the code reaches the SoundWire interrupt thread handling, the interrupt was enabled already, and there is no code that disables it -> this is a no-op sequence.
Acked-By: Vinod Koul vkoul@kernel.org
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
Different generations of Intel hardware rely on different programming sequences to enable SoundWire IP. In existing hardware, the SoundWire interrupt is enabled with a register field in the DSP register space. With HDaudio multi-link extensions registers, the SoundWire interrupt will be enabled with a generic interrupt enable field in LCTL, without any dependency on the DSP being enabled.
Add a per-chip callback following the example of the check_sdw_irq() model already upstream.
Note that the callback is not populated yet for MeteorLake (MTL) since the interrupts are already enabled in the init. A follow-up patch will move the functionality to this callback after a couple of cleanups.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com --- sound/soc/sof/intel/cnl.c | 2 ++ sound/soc/sof/intel/hda.c | 20 +++++++++++++++++++- sound/soc/sof/intel/hda.h | 6 ++++++ sound/soc/sof/intel/icl.c | 1 + sound/soc/sof/intel/shim.h | 1 + sound/soc/sof/intel/tgl.c | 4 ++++ 6 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c index 0aaa44bd49eb..cbb6474d5c33 100644 --- a/sound/soc/sof/intel/cnl.c +++ b/sound/soc/sof/intel/cnl.c @@ -457,6 +457,7 @@ const struct sof_intel_dsp_desc cnl_chip_info = { .sdw_shim_base = SDW_SHIM_BASE, .sdw_alh_base = SDW_ALH_BASE, .d0i3_offset = SOF_HDA_VS_D0I3C, + .enable_sdw_irq = hda_common_enable_sdw_irq, .check_sdw_irq = hda_common_check_sdw_irq, .check_ipc_irq = hda_dsp_check_ipc_irq, .cl_init = cl_dsp_init, @@ -490,6 +491,7 @@ const struct sof_intel_dsp_desc jsl_chip_info = { .sdw_shim_base = SDW_SHIM_BASE, .sdw_alh_base = SDW_ALH_BASE, .d0i3_offset = SOF_HDA_VS_D0I3C, + .enable_sdw_irq = hda_common_enable_sdw_irq, .check_sdw_irq = hda_common_check_sdw_irq, .check_ipc_irq = hda_dsp_check_ipc_irq, .cl_init = cl_dsp_init, diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 348fbfb6a2c2..2dc828866b5b 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -155,9 +155,27 @@ struct sdw_intel_ops sdw_callback = { .free_stream = sdw_free_stream, };
+void hda_common_enable_sdw_irq(struct snd_sof_dev *sdev, bool enable) +{ + struct sof_intel_hda_dev *hdev; + + hdev = sdev->pdata->hw_pdata; + + if (!hdev->sdw) + return; + + snd_sof_dsp_update_bits(sdev, HDA_DSP_BAR, HDA_DSP_REG_ADSPIC2, + HDA_DSP_REG_ADSPIC2_SNDW, + enable ? HDA_DSP_REG_ADSPIC2_SNDW : 0); +} + void hda_sdw_int_enable(struct snd_sof_dev *sdev, bool enable) { - sdw_intel_enable_irq(sdev->bar[HDA_DSP_BAR], enable); + const struct sof_intel_dsp_desc *chip; + + chip = get_chip_info(sdev->pdata); + if (chip && chip->enable_sdw_irq) + chip->enable_sdw_irq(sdev, enable); }
static int hda_sdw_acpi_scan(struct snd_sof_dev *sdev) diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index ea73fd17ae28..79fccd7b5bf7 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -294,6 +294,7 @@ #define HDA_DSP_REG_ADSPIC2 (HDA_DSP_GEN_BASE + 0x10) #define HDA_DSP_REG_ADSPIS2 (HDA_DSP_GEN_BASE + 0x14)
+#define HDA_DSP_REG_ADSPIC2_SNDW BIT(5) #define HDA_DSP_REG_ADSPIS2_SNDW BIT(5)
/* Intel HD Audio Inter-Processor Communication Registers */ @@ -795,6 +796,7 @@ int hda_dsp_trace_trigger(struct snd_sof_dev *sdev, int cmd); #if IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE)
int hda_sdw_startup(struct snd_sof_dev *sdev); +void hda_common_enable_sdw_irq(struct snd_sof_dev *sdev, bool enable); void hda_sdw_int_enable(struct snd_sof_dev *sdev, bool enable); void hda_sdw_process_wakeen(struct snd_sof_dev *sdev); bool hda_common_check_sdw_irq(struct snd_sof_dev *sdev); @@ -806,6 +808,10 @@ static inline int hda_sdw_startup(struct snd_sof_dev *sdev) return 0; }
+static inline void hda_common_enable_sdw_irq(struct snd_sof_dev *sdev, bool enable) +{ +} + static inline void hda_sdw_int_enable(struct snd_sof_dev *sdev, bool enable) { } diff --git a/sound/soc/sof/intel/icl.c b/sound/soc/sof/intel/icl.c index 8dd51f489ba1..f3aff23a13d2 100644 --- a/sound/soc/sof/intel/icl.c +++ b/sound/soc/sof/intel/icl.c @@ -181,6 +181,7 @@ const struct sof_intel_dsp_desc icl_chip_info = { .sdw_shim_base = SDW_SHIM_BASE, .sdw_alh_base = SDW_ALH_BASE, .d0i3_offset = SOF_HDA_VS_D0I3C, + .enable_sdw_irq = hda_common_enable_sdw_irq, .check_sdw_irq = hda_common_check_sdw_irq, .check_ipc_irq = hda_dsp_check_ipc_irq, .cl_init = cl_dsp_init, diff --git a/sound/soc/sof/intel/shim.h b/sound/soc/sof/intel/shim.h index 3e777c500a56..2e78f27c4207 100644 --- a/sound/soc/sof/intel/shim.h +++ b/sound/soc/sof/intel/shim.h @@ -185,6 +185,7 @@ struct sof_intel_dsp_desc { u32 d0i3_offset; u32 quirks; enum sof_intel_hw_ip_version hw_ip_version; + void (*enable_sdw_irq)(struct snd_sof_dev *sdev, bool enable); bool (*check_sdw_irq)(struct snd_sof_dev *sdev); bool (*check_ipc_irq)(struct snd_sof_dev *sdev); int (*power_down_dsp)(struct snd_sof_dev *sdev); diff --git a/sound/soc/sof/intel/tgl.c b/sound/soc/sof/intel/tgl.c index 946044f440c9..dda89c8ea820 100644 --- a/sound/soc/sof/intel/tgl.c +++ b/sound/soc/sof/intel/tgl.c @@ -136,6 +136,7 @@ const struct sof_intel_dsp_desc tgl_chip_info = { .sdw_shim_base = SDW_SHIM_BASE, .sdw_alh_base = SDW_ALH_BASE, .d0i3_offset = SOF_HDA_VS_D0I3C, + .enable_sdw_irq = hda_common_enable_sdw_irq, .check_sdw_irq = hda_common_check_sdw_irq, .check_ipc_irq = hda_dsp_check_ipc_irq, .cl_init = cl_dsp_init, @@ -162,6 +163,7 @@ const struct sof_intel_dsp_desc tglh_chip_info = { .sdw_shim_base = SDW_SHIM_BASE, .sdw_alh_base = SDW_ALH_BASE, .d0i3_offset = SOF_HDA_VS_D0I3C, + .enable_sdw_irq = hda_common_enable_sdw_irq, .check_sdw_irq = hda_common_check_sdw_irq, .check_ipc_irq = hda_dsp_check_ipc_irq, .cl_init = cl_dsp_init, @@ -188,6 +190,7 @@ const struct sof_intel_dsp_desc ehl_chip_info = { .sdw_shim_base = SDW_SHIM_BASE, .sdw_alh_base = SDW_ALH_BASE, .d0i3_offset = SOF_HDA_VS_D0I3C, + .enable_sdw_irq = hda_common_enable_sdw_irq, .check_sdw_irq = hda_common_check_sdw_irq, .check_ipc_irq = hda_dsp_check_ipc_irq, .cl_init = cl_dsp_init, @@ -214,6 +217,7 @@ const struct sof_intel_dsp_desc adls_chip_info = { .sdw_shim_base = SDW_SHIM_BASE, .sdw_alh_base = SDW_ALH_BASE, .d0i3_offset = SOF_HDA_VS_D0I3C, + .enable_sdw_irq = hda_common_enable_sdw_irq, .check_sdw_irq = hda_common_check_sdw_irq, .check_ipc_irq = hda_dsp_check_ipc_irq, .cl_init = cl_dsp_init,
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
The offsets and sequences are identical for interrupt enabling and disabling, we can refactor the code with a single routine and a boolean.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com --- sound/soc/sof/intel/mtl.c | 120 +++++++++++++------------------------- 1 file changed, 40 insertions(+), 80 deletions(-)
diff --git a/sound/soc/sof/intel/mtl.c b/sound/soc/sof/intel/mtl.c index 7452a7dbb0e4..43ffcccec0be 100644 --- a/sound/soc/sof/intel/mtl.c +++ b/sound/soc/sof/intel/mtl.c @@ -134,112 +134,72 @@ static void mtl_disable_ipc_interrupts(struct snd_sof_dev *sdev) MTL_DSP_REG_HFIPCXCTL_BUSY | MTL_DSP_REG_HFIPCXCTL_DONE, 0); }
-static int mtl_enable_interrupts(struct snd_sof_dev *sdev) +static int mtl_enable_interrupts(struct snd_sof_dev *sdev, bool enable) { u32 hfintipptr; u32 irqinten; - u32 host_ipc; u32 hipcie; + u32 mask; + u32 val; int ret;
/* read Interrupt IP Pointer */ hfintipptr = snd_sof_dsp_read(sdev, HDA_DSP_BAR, MTL_HFINTIPPTR) & MTL_HFINTIPPTR_PTR_MASK;
- /* Enable Host IPC and SOUNDWIRE */ - snd_sof_dsp_update_bits(sdev, HDA_DSP_BAR, hfintipptr, - MTL_IRQ_INTEN_L_HOST_IPC_MASK | MTL_IRQ_INTEN_L_SOUNDWIRE_MASK, - MTL_IRQ_INTEN_L_HOST_IPC_MASK | MTL_IRQ_INTEN_L_SOUNDWIRE_MASK); + /* Enable/Disable Host IPC and SOUNDWIRE */ + mask = MTL_IRQ_INTEN_L_HOST_IPC_MASK | MTL_IRQ_INTEN_L_SOUNDWIRE_MASK; + if (enable) + val = mask; + else + val = 0; + + snd_sof_dsp_update_bits(sdev, HDA_DSP_BAR, hfintipptr, mask, val);
/* check if operation was successful */ - host_ipc = MTL_IRQ_INTEN_L_HOST_IPC_MASK | MTL_IRQ_INTEN_L_SOUNDWIRE_MASK; ret = snd_sof_dsp_read_poll_timeout(sdev, HDA_DSP_BAR, hfintipptr, irqinten, - (irqinten & host_ipc) == host_ipc, + (irqinten & mask) == val, HDA_DSP_REG_POLL_INTERVAL_US, HDA_DSP_RESET_TIMEOUT_US); if (ret < 0) { - dev_err(sdev->dev, "failed to enable Host IPC and/or SOUNDWIRE\n"); + dev_err(sdev->dev, "failed to %s Host IPC and/or SOUNDWIRE\n", + enable ? "enable" : "disable"); return ret; }
- /* Set Host IPC interrupt enable */ - snd_sof_dsp_update_bits(sdev, HDA_DSP_BAR, MTL_DSP_REG_HfHIPCIE, - MTL_DSP_REG_HfHIPCIE_IE_MASK, MTL_DSP_REG_HfHIPCIE_IE_MASK); + /* Enable/Disable Host IPC interrupt*/ + mask = MTL_DSP_REG_HfHIPCIE_IE_MASK; + if (enable) + val = mask; + else + val = 0; + + snd_sof_dsp_update_bits(sdev, HDA_DSP_BAR, MTL_DSP_REG_HfHIPCIE, mask, val);
/* check if operation was successful */ - host_ipc = MTL_DSP_REG_HfHIPCIE_IE_MASK; ret = snd_sof_dsp_read_poll_timeout(sdev, HDA_DSP_BAR, MTL_DSP_REG_HfHIPCIE, hipcie, - (hipcie & host_ipc) == host_ipc, + (hipcie & mask) == val, HDA_DSP_REG_POLL_INTERVAL_US, HDA_DSP_RESET_TIMEOUT_US); if (ret < 0) { - dev_err(sdev->dev, "failed to set Host IPC interrupt enable\n"); + dev_err(sdev->dev, "failed to set Host IPC interrupt %s\n", + enable ? "enable" : "disable"); return ret; }
- snd_sof_dsp_update_bits(sdev, HDA_DSP_BAR, MTL_DSP_REG_HfSNDWIE, - MTL_DSP_REG_HfSNDWIE_IE_MASK, MTL_DSP_REG_HfSNDWIE_IE_MASK); - host_ipc = MTL_DSP_REG_HfSNDWIE_IE_MASK; + /* Enable/Disable SoundWire interrupt */ + mask = MTL_DSP_REG_HfSNDWIE_IE_MASK; + if (enable) + val = mask; + else + val = 0; + + snd_sof_dsp_update_bits(sdev, HDA_DSP_BAR, MTL_DSP_REG_HfSNDWIE, mask, val); + + /* check if operation was successful */ ret = snd_sof_dsp_read_poll_timeout(sdev, HDA_DSP_BAR, MTL_DSP_REG_HfSNDWIE, hipcie, - (hipcie & host_ipc) == host_ipc, + (hipcie & mask) == val, HDA_DSP_REG_POLL_INTERVAL_US, HDA_DSP_RESET_TIMEOUT_US); if (ret < 0) - dev_err(sdev->dev, "failed to set SoundWire IPC interrupt enable\n"); - - return ret; -} - -static int mtl_disable_interrupts(struct snd_sof_dev *sdev) -{ - u32 hfintipptr; - u32 irqinten; - u32 host_ipc; - u32 hipcie; - int ret1; - int ret; - - /* read Interrupt IP Pointer */ - hfintipptr = snd_sof_dsp_read(sdev, HDA_DSP_BAR, MTL_HFINTIPPTR) & MTL_HFINTIPPTR_PTR_MASK; - - /* Disable Host IPC and SOUNDWIRE */ - snd_sof_dsp_update_bits(sdev, HDA_DSP_BAR, hfintipptr, - MTL_IRQ_INTEN_L_HOST_IPC_MASK | MTL_IRQ_INTEN_L_SOUNDWIRE_MASK, 0); - - /* check if operation was successful */ - host_ipc = MTL_IRQ_INTEN_L_HOST_IPC_MASK | MTL_IRQ_INTEN_L_SOUNDWIRE_MASK; - ret = snd_sof_dsp_read_poll_timeout(sdev, HDA_DSP_BAR, hfintipptr, irqinten, - (irqinten & host_ipc) == 0, - HDA_DSP_REG_POLL_INTERVAL_US, HDA_DSP_RESET_TIMEOUT_US); - /* Continue to disable other interrupts when error happens */ - if (ret < 0) - dev_err(sdev->dev, "failed to disable Host IPC and SoundWire\n"); - - /* Set Host IPC interrupt disable */ - snd_sof_dsp_update_bits(sdev, HDA_DSP_BAR, MTL_DSP_REG_HfHIPCIE, - MTL_DSP_REG_HfHIPCIE_IE_MASK, 0); - - /* check if operation was successful */ - host_ipc = MTL_DSP_REG_HfHIPCIE_IE_MASK; - ret1 = snd_sof_dsp_read_poll_timeout(sdev, HDA_DSP_BAR, MTL_DSP_REG_HfHIPCIE, hipcie, - (hipcie & host_ipc) == 0, - HDA_DSP_REG_POLL_INTERVAL_US, - HDA_DSP_RESET_TIMEOUT_US); - if (ret1 < 0) { - dev_err(sdev->dev, "failed to set Host IPC interrupt disable\n"); - if (!ret) - ret = ret1; - } - - /* Set SoundWire IPC interrupt disable */ - snd_sof_dsp_update_bits(sdev, HDA_DSP_BAR, MTL_DSP_REG_HfSNDWIE, - MTL_DSP_REG_HfSNDWIE_IE_MASK, 0); - host_ipc = MTL_DSP_REG_HfSNDWIE_IE_MASK; - ret1 = snd_sof_dsp_read_poll_timeout(sdev, HDA_DSP_BAR, MTL_DSP_REG_HfSNDWIE, hipcie, - (hipcie & host_ipc) == 0, - HDA_DSP_REG_POLL_INTERVAL_US, - HDA_DSP_RESET_TIMEOUT_US); - if (ret1 < 0) { - dev_err(sdev->dev, "failed to set SoundWire IPC interrupt disable\n"); - if (!ret) - ret = ret1; - } + dev_err(sdev->dev, "failed to set SoundWire IPC interrupt %s\n", + enable ? "enable" : "disable");
return ret; } @@ -473,7 +433,7 @@ static int mtl_dsp_cl_init(struct snd_sof_dev *sdev, int stream_tag, bool imr_bo chip->ipc_ack_mask);
/* step 4: enable interrupts */ - ret = mtl_enable_interrupts(sdev); + ret = mtl_enable_interrupts(sdev, true); if (ret < 0) { if (hda->boot_iteration == HDA_FW_BOOT_ATTEMPTS) dev_err(sdev->dev, "%s: failed to enable interrupts\n", __func__); @@ -609,7 +569,7 @@ static void mtl_ipc_dump(struct snd_sof_dev *sdev) static int mtl_dsp_disable_interrupts(struct snd_sof_dev *sdev) { mtl_disable_ipc_interrupts(sdev); - return mtl_disable_interrupts(sdev); + return mtl_enable_interrupts(sdev, false); }
/* Meteorlake ops */
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
There's no real rationale for enabling the SoundWire interrupt in the init, this can be done from the enable_sdw_irq() callback.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com --- sound/soc/sof/intel/mtl.c | 44 ++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 17 deletions(-)
diff --git a/sound/soc/sof/intel/mtl.c b/sound/soc/sof/intel/mtl.c index 43ffcccec0be..a0839804a6da 100644 --- a/sound/soc/sof/intel/mtl.c +++ b/sound/soc/sof/intel/mtl.c @@ -134,6 +134,31 @@ static void mtl_disable_ipc_interrupts(struct snd_sof_dev *sdev) MTL_DSP_REG_HFIPCXCTL_BUSY | MTL_DSP_REG_HFIPCXCTL_DONE, 0); }
+static void mtl_enable_sdw_irq(struct snd_sof_dev *sdev, bool enable) +{ + u32 hipcie; + u32 mask; + u32 val; + int ret; + + /* Enable/Disable SoundWire interrupt */ + mask = MTL_DSP_REG_HfSNDWIE_IE_MASK; + if (enable) + val = mask; + else + val = 0; + + snd_sof_dsp_update_bits(sdev, HDA_DSP_BAR, MTL_DSP_REG_HfSNDWIE, mask, val); + + /* check if operation was successful */ + ret = snd_sof_dsp_read_poll_timeout(sdev, HDA_DSP_BAR, MTL_DSP_REG_HfSNDWIE, hipcie, + (hipcie & mask) == val, + HDA_DSP_REG_POLL_INTERVAL_US, HDA_DSP_RESET_TIMEOUT_US); + if (ret < 0) + dev_err(sdev->dev, "failed to set SoundWire IPC interrupt %s\n", + enable ? "enable" : "disable"); +} + static int mtl_enable_interrupts(struct snd_sof_dev *sdev, bool enable) { u32 hfintipptr; @@ -184,23 +209,6 @@ static int mtl_enable_interrupts(struct snd_sof_dev *sdev, bool enable) return ret; }
- /* Enable/Disable SoundWire interrupt */ - mask = MTL_DSP_REG_HfSNDWIE_IE_MASK; - if (enable) - val = mask; - else - val = 0; - - snd_sof_dsp_update_bits(sdev, HDA_DSP_BAR, MTL_DSP_REG_HfSNDWIE, mask, val); - - /* check if operation was successful */ - ret = snd_sof_dsp_read_poll_timeout(sdev, HDA_DSP_BAR, MTL_DSP_REG_HfSNDWIE, hipcie, - (hipcie & mask) == val, - HDA_DSP_REG_POLL_INTERVAL_US, HDA_DSP_RESET_TIMEOUT_US); - if (ret < 0) - dev_err(sdev->dev, "failed to set SoundWire IPC interrupt %s\n", - enable ? "enable" : "disable"); - return ret; }
@@ -568,6 +576,7 @@ static void mtl_ipc_dump(struct snd_sof_dev *sdev)
static int mtl_dsp_disable_interrupts(struct snd_sof_dev *sdev) { + mtl_enable_sdw_irq(sdev, false); mtl_disable_ipc_interrupts(sdev); return mtl_enable_interrupts(sdev, false); } @@ -645,6 +654,7 @@ const struct sof_intel_dsp_desc mtl_chip_info = { .sdw_shim_base = SDW_SHIM_BASE_ACE, .sdw_alh_base = SDW_ALH_BASE_ACE, .d0i3_offset = MTL_HDA_VS_D0I3C, + .enable_sdw_irq = mtl_enable_sdw_irq, .check_sdw_irq = mtl_dsp_check_sdw_irq, .check_ipc_irq = mtl_dsp_check_ipc_irq, .cl_init = mtl_dsp_cl_init,
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
The number of links is stored in different registers depending on the IP version, add sdw_check_lcount() callback. This callback only checks that the number of links supported in hardware is compatible with the number of links exposed in ACPI _DSD properties.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com --- sound/soc/sof/intel/cnl.c | 2 ++ sound/soc/sof/intel/hda.c | 39 ++++++++++++++++++++++++++++++++++++++ sound/soc/sof/intel/hda.h | 6 ++++++ sound/soc/sof/intel/icl.c | 1 + sound/soc/sof/intel/mtl.c | 1 + sound/soc/sof/intel/shim.h | 1 + sound/soc/sof/intel/tgl.c | 4 ++++ 7 files changed, 54 insertions(+)
diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c index cbb6474d5c33..6b075bbe5bfb 100644 --- a/sound/soc/sof/intel/cnl.c +++ b/sound/soc/sof/intel/cnl.c @@ -457,6 +457,7 @@ const struct sof_intel_dsp_desc cnl_chip_info = { .sdw_shim_base = SDW_SHIM_BASE, .sdw_alh_base = SDW_ALH_BASE, .d0i3_offset = SOF_HDA_VS_D0I3C, + .read_sdw_lcount = hda_sdw_check_lcount_common, .enable_sdw_irq = hda_common_enable_sdw_irq, .check_sdw_irq = hda_common_check_sdw_irq, .check_ipc_irq = hda_dsp_check_ipc_irq, @@ -491,6 +492,7 @@ const struct sof_intel_dsp_desc jsl_chip_info = { .sdw_shim_base = SDW_SHIM_BASE, .sdw_alh_base = SDW_ALH_BASE, .d0i3_offset = SOF_HDA_VS_D0I3C, + .read_sdw_lcount = hda_sdw_check_lcount_common, .enable_sdw_irq = hda_common_enable_sdw_irq, .check_sdw_irq = hda_common_check_sdw_irq, .check_ipc_irq = hda_dsp_check_ipc_irq, diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 2dc828866b5b..2f9d69e64091 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -238,10 +238,45 @@ static int hda_sdw_probe(struct snd_sof_dev *sdev) return 0; }
+int hda_sdw_check_lcount_common(struct snd_sof_dev *sdev) +{ + struct sof_intel_hda_dev *hdev; + struct sdw_intel_ctx *ctx; + u32 caps; + + hdev = sdev->pdata->hw_pdata; + ctx = hdev->sdw; + + caps = snd_sof_dsp_read(sdev, HDA_DSP_BAR, ctx->shim_base + SDW_SHIM_LCAP); + caps &= SDW_SHIM_LCAP_LCOUNT_MASK; + + /* Check HW supported vs property value */ + if (caps < ctx->count) { + dev_err(sdev->dev, + "BIOS master count %d is larger than hardware capabilities %d\n", + ctx->count, caps); + return -EINVAL; + } + + return 0; +} + +static int hda_sdw_check_lcount(struct snd_sof_dev *sdev) +{ + const struct sof_intel_dsp_desc *chip; + + chip = get_chip_info(sdev->pdata); + if (chip && chip->read_sdw_lcount) + return chip->read_sdw_lcount(sdev); + + return 0; +} + int hda_sdw_startup(struct snd_sof_dev *sdev) { struct sof_intel_hda_dev *hdev; struct snd_sof_pdata *pdata = sdev->pdata; + int ret;
hdev = sdev->pdata->hw_pdata;
@@ -251,6 +286,10 @@ int hda_sdw_startup(struct snd_sof_dev *sdev) if (pdata->machine && !pdata->machine->mach_params.link_mask) return 0;
+ ret = hda_sdw_check_lcount(sdev); + if (ret < 0) + return ret; + return sdw_intel_startup(hdev->sdw); }
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 79fccd7b5bf7..022ce80968dd 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -795,6 +795,7 @@ int hda_dsp_trace_trigger(struct snd_sof_dev *sdev, int cmd); */ #if IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE)
+int hda_sdw_check_lcount_common(struct snd_sof_dev *sdev); int hda_sdw_startup(struct snd_sof_dev *sdev); void hda_common_enable_sdw_irq(struct snd_sof_dev *sdev, bool enable); void hda_sdw_int_enable(struct snd_sof_dev *sdev, bool enable); @@ -803,6 +804,11 @@ bool hda_common_check_sdw_irq(struct snd_sof_dev *sdev);
#else
+static inline int hda_sdw_check_lcount_common(struct snd_sof_dev *sdev) +{ + return 0; +} + static inline int hda_sdw_startup(struct snd_sof_dev *sdev) { return 0; diff --git a/sound/soc/sof/intel/icl.c b/sound/soc/sof/intel/icl.c index f3aff23a13d2..435941a1692f 100644 --- a/sound/soc/sof/intel/icl.c +++ b/sound/soc/sof/intel/icl.c @@ -181,6 +181,7 @@ const struct sof_intel_dsp_desc icl_chip_info = { .sdw_shim_base = SDW_SHIM_BASE, .sdw_alh_base = SDW_ALH_BASE, .d0i3_offset = SOF_HDA_VS_D0I3C, + .read_sdw_lcount = hda_sdw_check_lcount_common, .enable_sdw_irq = hda_common_enable_sdw_irq, .check_sdw_irq = hda_common_check_sdw_irq, .check_ipc_irq = hda_dsp_check_ipc_irq, diff --git a/sound/soc/sof/intel/mtl.c b/sound/soc/sof/intel/mtl.c index a0839804a6da..904ae42534e1 100644 --- a/sound/soc/sof/intel/mtl.c +++ b/sound/soc/sof/intel/mtl.c @@ -654,6 +654,7 @@ const struct sof_intel_dsp_desc mtl_chip_info = { .sdw_shim_base = SDW_SHIM_BASE_ACE, .sdw_alh_base = SDW_ALH_BASE_ACE, .d0i3_offset = MTL_HDA_VS_D0I3C, + .read_sdw_lcount = hda_sdw_check_lcount_common, .enable_sdw_irq = mtl_enable_sdw_irq, .check_sdw_irq = mtl_dsp_check_sdw_irq, .check_ipc_irq = mtl_dsp_check_ipc_irq, diff --git a/sound/soc/sof/intel/shim.h b/sound/soc/sof/intel/shim.h index 2e78f27c4207..48428ccbcfe0 100644 --- a/sound/soc/sof/intel/shim.h +++ b/sound/soc/sof/intel/shim.h @@ -185,6 +185,7 @@ struct sof_intel_dsp_desc { u32 d0i3_offset; u32 quirks; enum sof_intel_hw_ip_version hw_ip_version; + int (*read_sdw_lcount)(struct snd_sof_dev *sdev); void (*enable_sdw_irq)(struct snd_sof_dev *sdev, bool enable); bool (*check_sdw_irq)(struct snd_sof_dev *sdev); bool (*check_ipc_irq)(struct snd_sof_dev *sdev); diff --git a/sound/soc/sof/intel/tgl.c b/sound/soc/sof/intel/tgl.c index dda89c8ea820..30f2f49ee149 100644 --- a/sound/soc/sof/intel/tgl.c +++ b/sound/soc/sof/intel/tgl.c @@ -136,6 +136,7 @@ const struct sof_intel_dsp_desc tgl_chip_info = { .sdw_shim_base = SDW_SHIM_BASE, .sdw_alh_base = SDW_ALH_BASE, .d0i3_offset = SOF_HDA_VS_D0I3C, + .read_sdw_lcount = hda_sdw_check_lcount_common, .enable_sdw_irq = hda_common_enable_sdw_irq, .check_sdw_irq = hda_common_check_sdw_irq, .check_ipc_irq = hda_dsp_check_ipc_irq, @@ -163,6 +164,7 @@ const struct sof_intel_dsp_desc tglh_chip_info = { .sdw_shim_base = SDW_SHIM_BASE, .sdw_alh_base = SDW_ALH_BASE, .d0i3_offset = SOF_HDA_VS_D0I3C, + .read_sdw_lcount = hda_sdw_check_lcount_common, .enable_sdw_irq = hda_common_enable_sdw_irq, .check_sdw_irq = hda_common_check_sdw_irq, .check_ipc_irq = hda_dsp_check_ipc_irq, @@ -190,6 +192,7 @@ const struct sof_intel_dsp_desc ehl_chip_info = { .sdw_shim_base = SDW_SHIM_BASE, .sdw_alh_base = SDW_ALH_BASE, .d0i3_offset = SOF_HDA_VS_D0I3C, + .read_sdw_lcount = hda_sdw_check_lcount_common, .enable_sdw_irq = hda_common_enable_sdw_irq, .check_sdw_irq = hda_common_check_sdw_irq, .check_ipc_irq = hda_dsp_check_ipc_irq, @@ -217,6 +220,7 @@ const struct sof_intel_dsp_desc adls_chip_info = { .sdw_shim_base = SDW_SHIM_BASE, .sdw_alh_base = SDW_ALH_BASE, .d0i3_offset = SOF_HDA_VS_D0I3C, + .read_sdw_lcount = hda_sdw_check_lcount_common, .enable_sdw_irq = hda_common_enable_sdw_irq, .check_sdw_irq = hda_common_check_sdw_irq, .check_ipc_irq = hda_dsp_check_ipc_irq,
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
The functionality is implemented with per-chip callbacks, there are no users of this symbol, remove the code.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com --- drivers/soundwire/intel_init.c | 24 ------------------------ include/linux/soundwire/sdw_intel.h | 2 -- 2 files changed, 26 deletions(-)
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index 8bd95c9cbcaf..0df3cdd85793 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -125,30 +125,6 @@ static int sdw_intel_cleanup(struct sdw_intel_ctx *ctx) return 0; }
-#define HDA_DSP_REG_ADSPIC2 (0x10) -#define HDA_DSP_REG_ADSPIS2 (0x14) -#define HDA_DSP_REG_ADSPIC2_SNDW BIT(5) - -/** - * sdw_intel_enable_irq() - enable/disable Intel SoundWire IRQ - * @mmio_base: The mmio base of the control register - * @enable: true if enable - */ -void sdw_intel_enable_irq(void __iomem *mmio_base, bool enable) -{ - u32 val; - - val = readl(mmio_base + HDA_DSP_REG_ADSPIC2); - - if (enable) - val |= HDA_DSP_REG_ADSPIC2_SNDW; - else - val &= ~HDA_DSP_REG_ADSPIC2_SNDW; - - writel(val, mmio_base + HDA_DSP_REG_ADSPIC2); -} -EXPORT_SYMBOL_NS(sdw_intel_enable_irq, SOUNDWIRE_INTEL_INIT); - irqreturn_t sdw_intel_thread(int irq, void *dev_id) { struct sdw_intel_ctx *ctx = dev_id; diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index 2e9fd91572d4..d2f581feed67 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -286,8 +286,6 @@ int sdw_intel_startup(struct sdw_intel_ctx *ctx);
void sdw_intel_exit(struct sdw_intel_ctx *ctx);
-void sdw_intel_enable_irq(void __iomem *mmio_base, bool enable); - irqreturn_t sdw_intel_thread(int irq, void *dev_id);
#define SDW_INTEL_QUIRK_MASK_BUS_DISABLE BIT(1)
On 11-11-22, 12:26, Bard Liao wrote:
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
The functionality is implemented with per-chip callbacks, there are no users of this symbol, remove the code.
Acked-By: Vinod Koul vkoul@kernel.org
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
The number of links is checked with a chip-dependent helper in the caller, remove the check in drivers/soundwire/intel_init.c
This change makes intel_init.c hardware-agnostic - which is quite fitting for a layer that only creates auxiliary devices.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com --- drivers/soundwire/intel_init.c | 12 ------------ 1 file changed, 12 deletions(-)
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index 0df3cdd85793..d6842925de61 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -272,24 +272,12 @@ sdw_intel_startup_controller(struct sdw_intel_ctx *ctx) { struct acpi_device *adev = acpi_fetch_acpi_dev(ctx->handle); struct sdw_intel_link_dev *ldev; - u32 caps; u32 link_mask; int i;
if (!adev) return -EINVAL;
- /* Check SNDWLCAP.LCOUNT */ - caps = ioread32(ctx->mmio_base + ctx->shim_base + SDW_SHIM_LCAP); - caps &= SDW_SHIM_LCAP_LCOUNT_MASK; - - /* Check HW supported vs property value */ - if (caps < ctx->count) { - dev_err(&adev->dev, - "BIOS master count is larger than hardware capabilities\n"); - return -EINVAL; - } - if (!ctx->ldev) return -EINVAL;
On 11-11-22, 12:26, Bard Liao wrote:
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
The number of links is checked with a chip-dependent helper in the caller, remove the check in drivers/soundwire/intel_init.c
This change makes intel_init.c hardware-agnostic - which is quite fitting for a layer that only creates auxiliary devices.
Acked-By: Vinod Koul vkoul@kernel.org
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
There's no reason to delay the multi-link parsing, this can be done earlier before checking the SoundWire capabilities.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com --- sound/soc/sof/intel/hda.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 2f9d69e64091..14a2f8701350 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -944,6 +944,8 @@ static int hda_init_caps(struct snd_sof_dev *sdev) return ret; }
+ hda_bus_ml_get_capabilities(bus); + /* scan SoundWire capabilities exposed by DSDT */ ret = hda_sdw_acpi_scan(sdev); if (ret < 0) { @@ -972,8 +974,6 @@ static int hda_init_caps(struct snd_sof_dev *sdev)
skip_soundwire:
- hda_bus_ml_get_capabilities(bus); - /* create codec instances */ hda_codec_probe_bus(sdev);
On Fri, 11 Nov 2022 12:26:45 +0800, Bard Liao wrote:
The code in drivers/soundwire/intel_init.c is hardware-dependent and the code does not apply to new generations starting with MeteorLake. Refactor and clean-up the code to make this intel_init.c hardware-agnostic and move all hardware-dependencies in the SOF driver using chip descriptors.
The ASoC patches are dependent on some patches that are applied to ASoC tree recently. So, this series won't apply to SoundWire tree. @Vinod Could you Ack if it looks good to you, and lets go through ASoC tree?
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/8] soundwire: intel_init: remove useless interrupt enablement in interrupt thread commit: c5e5da1eb3d3009ed861f1514b41bec323c191d1 [2/8] ASoC: SOF: Intel: hda: add per-chip enable_sdw_irq() callback commit: 8ebc90741e96646af7320336ac4433eea175390a [3/8] ASoC: SOF: Intel: mtl: factor interrupt enable/disable interrupt functions commit: 00f4f3380745da4950de2bf65f15af767d54dfe1 [4/8] ASoC: SOF: Intel: mtl: move SoundWire interrupt enabling to callback commit: aa70a580930a42781f57ac0d8b281ed2f6b0d8ec [5/8] ASoC: SOF: Intel: hda: add callback to check SoundWire lcount information commit: 625339caaea15c0e69d833227652d2f5b6e365cc [6/8] soundwire: intel_init: remove sdw_intel_enable_irq() commit: 562bb228cebea475cc967c4a53df97ca62aa90b5 [7/8] soundwire: intel_init: remove check on number of links commit: 2cd24c318cc943b54cbd2d855cee798314619c4e [8/8] ASoC: SOF: Intel: hda: read multi-link capabilities earlier commit: 5e2cbc4a813e866885f812f1b64fdf33a9a16700
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (3)
-
Bard Liao -
Mark Brown -
Vinod Koul