-
Notifications
You must be signed in to change notification settings - Fork 140
SoundWire: revisit interrupt and lcount handling #3933
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SoundWire: revisit interrupt and lcount handling #3933
Conversation
c1275ab to
5c13a86
Compare
RanderWang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Going back to draft, I am not happy with the PR, it needs more work to move the startup and interrupt enabling. |
…pt thread 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>
5c13a86 to
9729667
Compare
|
let's give @bardliao a chance to review. |
|
changes since last version (comments from @ranj063 and @ujfalusi ) diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index bc32636c2014..944f95bd1e92 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -165,14 +165,9 @@ void hda_common_enable_sdw_irq(struct snd_sof_dev *sdev, bool enable)
if (!hdev->sdw)
return;
- val = snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_REG_ADSPIC2);
-
- if (enable)
- val |= HDA_DSP_REG_ADSPIC2_SNDW;
- else
- val &= ~HDA_DSP_REG_ADSPIC2_SNDW;
-
- snd_sof_dsp_write(sdev, HDA_DSP_BAR, HDA_DSP_REG_ADSPIC2, val);
+ 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)
@@ -935,6 +930,7 @@ static int hda_init_caps(struct snd_sof_dev *sdev)
struct hdac_bus *bus = sof_to_bus(sdev);
struct snd_sof_pdata *pdata = sdev->pdata;
struct sof_intel_hda_dev *hdev = pdata->hw_pdata;
+ bool i915_off = false;
u32 link_mask;
int ret;
@@ -975,6 +971,7 @@ static int hda_init_caps(struct snd_sof_dev *sdev)
ret = hda_sdw_probe(sdev);
if (ret < 0) {
dev_err(sdev->dev, "SoundWire probe error: %d\n", ret);
+ i915_off = true;
goto err;
}
@@ -984,7 +981,7 @@ static int hda_init_caps(struct snd_sof_dev *sdev)
hda_codec_probe_bus(sdev);
err:
- if (!HDA_IDISP_CODEC(bus->codec_mask))
+ if (!HDA_IDISP_CODEC(bus->codec_mask) || i915_off)
hda_codec_i915_display_power(sdev, false);
hda_bus_ml_put_all(bus);
diff --git a/sound/soc/sof/intel/mtl.c b/sound/soc/sof/intel/mtl.c
index d20f04a19ad6..904ae42534e1 100644
--- a/sound/soc/sof/intel/mtl.c
+++ b/sound/soc/sof/intel/mtl.c
@@ -576,7 +576,7 @@ static void mtl_ipc_dump(struct snd_sof_dev *sdev)
static int mtl_dsp_disable_interrupts(struct snd_sof_dev *sdev)
{
- hda_sdw_int_enable(sdev, false);
+ mtl_enable_sdw_irq(sdev, false);
mtl_disable_ipc_interrupts(sdev);
return mtl_enable_interrupts(sdev, false);
} |
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>
…tions 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>
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>
…tion 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>
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>
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>
715a7a3
9729667 to
715a7a3
Compare
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>
715a7a3 to
d0fdba0
Compare
|
dropped "ASoC: SOF: Intel: hda: improve error handling on SoundWire probe", no other change. |
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.
Tested on TGL SKU 0A32