Re: [alsa-devel] [RFC PATCH 3/5] ASoC: SOF: Intel: hda: add SoundWire IP support

From: Pierre-Louis Bossart
Date: Wed Sep 04 2019 - 13:47:49 EST




On 9/4/19 11:51 AM, Vinod Koul wrote:
On 04-09-19, 08:25, Pierre-Louis Bossart wrote:
On 9/4/19 2:21 AM, Vinod Koul wrote:
On 21-08-19, 15:17, Pierre-Louis Bossart wrote:
The Core0 needs to be powered before the SoundWire IP is initialized.

Call sdw_intel_init/exit and store the context. We only have one
context, but depending on the hardware capabilities and BIOS settings
may enable multiple SoundWire links.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
---
sound/soc/sof/intel/hda.c | 40 +++++++++++++++++++++++++++++++++------
sound/soc/sof/intel/hda.h | 5 +++++
2 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index a968890d0754..e754058e3679 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -57,6 +57,8 @@ static int hda_sdw_init(struct snd_sof_dev *sdev)
{
acpi_handle handle;
struct sdw_intel_res res;
+ struct sof_intel_hda_dev *hdev;
+ void *sdw;
handle = ACPI_HANDLE(sdev->dev);
@@ -66,23 +68,32 @@ static int hda_sdw_init(struct snd_sof_dev *sdev)
res.irq = sdev->ipc_irq;
res.parent = sdev->dev;
- hda_sdw_int_enable(sdev, true);
-
- sdev->sdw = sdw_intel_init(handle, &res);
- if (!sdev->sdw) {
+ sdw = sdw_intel_init(handle, &res);

should this be called for platforms without sdw, I was hoping that some
checks would be performed.. For example how would skl deal with this?

Good point. For now we rely on CONFIG_SOUNDWIRE_INTEL to use a fallback, but
if the kernel defines this config and we run on an older platform the only
safety would be the hardware capabilities and BIOS dependencies, I need to
test if it works.

Yes I am not sure given the experience with BIOS relying on that is a
great idea ! But if that works, that would be better.

I don't think it's going to be that bad, first we need to find the ACPI description for the controller, then see which links are active, and even with all links disabled nothing bad will happen.

What I am more worried about are inconsistencies where e.g we have both I2C/I2S and SoundWire devices exposed at the same time. The BIOS deals with this with dynamic changes depending on user changes, and we are likely to see reports of problems due to BIOS configuration selection, not the BIOS itself.