2020-03-12 19:34:48

by Pierre-Louis Bossart

[permalink] [raw]
Subject: [PATCH 00/10] ASoC: SOF: Intel: add SoundWire support

This patchset provides the support for SoundWire support on Intel
CometLake, IcelLake and TigerLake RVP platforms and form-factor
devices to be released 'soon'.

The bulk of the code is about detecting a valid SoundWire
configuration from ACPI, and implementing the interfaces suggested in
'[PATCH 0/8] soundwire: remove platform devices, add SOF interfaces'
for interrupts, PCI wakes and clock-stop configurations.

Since that SoundWire series is stuck with no resolution, the build
support for SOF w/ SoundWire is not provided for now, and fall-back
functions will be used. This code is tested on a daily basis in the
SOF tree and is not expected to change. If audio maintainers will only
accept functional code, which isn't unreasonable, I would kindly ask
that they reach out to Vinod Koul.

Bard Liao (1):
ASoC: SOF: Intel: hda: merge IPC, stream and SoundWire interrupt
handlers

Pierre-Louis Bossart (7):
ASoC: soc-acpi: expand description of _ADR-based devices
ASoC: SOF: Intel: add SoundWire configuration interface
ASoC: SOF: IPC: dai-intel: move ALH declarations in header file
ASoC: SOF: Intel: hda: add SoundWire stream config/free callbacks
ASoC: SOF: Intel: hda: initial SoundWire machine driver autodetect
ASoC: SOF: Intel: hda: disable SoundWire interrupts on suspend
ASoC: SOF: Intel: hda: add parameter to control SoundWire clock stop
quirks

Rander Wang (2):
ASoC: SOF: Intel: hda: add WAKEEN interrupt support for SoundWire
Asoc: SOF: Intel: hda: check SoundWire wakeen interrupt in irq thread

include/sound/soc-acpi.h | 39 +-
include/sound/sof/dai-intel.h | 18 +-
.../intel/common/soc-acpi-intel-cml-match.c | 87 +++-
.../intel/common/soc-acpi-intel-icl-match.c | 97 ++++-
.../intel/common/soc-acpi-intel-tgl-match.c | 49 ++-
sound/soc/sof/intel/hda-dsp.c | 2 +
sound/soc/sof/intel/hda-loader.c | 31 ++
sound/soc/sof/intel/hda.c | 399 ++++++++++++++++++
sound/soc/sof/intel/hda.h | 66 +++
9 files changed, 728 insertions(+), 60 deletions(-)


base-commit: 101247a3b86e1cc0e382b7e887a56176290fc957
--
2.20.1


2020-03-12 19:35:12

by Pierre-Louis Bossart

[permalink] [raw]
Subject: [PATCH 01/10] ASoC: soc-acpi: expand description of _ADR-based devices

For SoundWire, we need to know if endpoints needs to be 'aggregated'
(MIPI parlance, meaning logically grouped), e.g. when two speaker
amplifiers need to be handled as a single logical output.

We don't necessarily have the information at the firmware (BIOS)
level, so add a notion of endpoints and specify if a device/endpoint
is part of a group, with a position.

This may be expanded in future solutions, for now only provide a group
and position information.

Since we modify the header file, change all existing upstream tables
as well to avoid breaking compilation/bisect.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
---
include/sound/soc-acpi.h | 39 ++++++--
.../intel/common/soc-acpi-intel-cml-match.c | 87 +++++++++++++----
.../intel/common/soc-acpi-intel-icl-match.c | 97 +++++++++++++++----
.../intel/common/soc-acpi-intel-tgl-match.c | 49 ++++++++--
4 files changed, 221 insertions(+), 51 deletions(-)

diff --git a/include/sound/soc-acpi.h b/include/sound/soc-acpi.h
index a217a87cae86..392e953d561e 100644
--- a/include/sound/soc-acpi.h
+++ b/include/sound/soc-acpi.h
@@ -75,18 +75,45 @@ struct snd_soc_acpi_mach_params {
};

/**
- * snd_soc_acpi_link_adr: ACPI-based list of _ADR, with a variable
- * number of devices per link
- *
+ * snd_soc_acpi_endpoint - endpoint descriptor
+ * @num: endpoint number (mandatory, unique per device)
+ * @aggregated: 0 (independent) or 1 (logically grouped)
+ * @group_position: zero-based order (only when @aggregated is 1)
+ * @group_id: platform-unique group identifier (only when @aggregrated is 1)
+ */
+struct snd_soc_acpi_endpoint {
+ u8 num;
+ u8 aggregated;
+ u8 group_position;
+ u8 group_id;
+};
+
+/**
+ * snd_soc_acpi_adr_device - descriptor for _ADR-enumerated device
+ * @adr: 64 bit ACPI _ADR value
+ * @num_endpoints: number of endpoints for this device
+ * @endpoints: array of endpoints
+ */
+struct snd_soc_acpi_adr_device {
+ const u64 adr;
+ const u8 num_endpoints;
+ const struct snd_soc_acpi_endpoint *endpoints;
+};
+
+/**
+ * snd_soc_acpi_link_adr - ACPI-based list of _ADR enumerated devices
* @mask: one bit set indicates the link this list applies to
- * @num_adr: ARRAY_SIZE of adr
- * @adr: array of _ADR (represented as u64).
+ * @num_adr: ARRAY_SIZE of devices
+ * @adr_d: array of devices
+ *
+ * The number of devices per link can be more than 1, e.g. in SoundWire
+ * multi-drop configurations.
*/

struct snd_soc_acpi_link_adr {
const u32 mask;
const u32 num_adr;
- const u64 *adr;
+ const struct snd_soc_acpi_adr_device *adr_d;
};

/**
diff --git a/sound/soc/intel/common/soc-acpi-intel-cml-match.c b/sound/soc/intel/common/soc-acpi-intel-cml-match.c
index f55634c4c2e8..3525da79c68a 100644
--- a/sound/soc/intel/common/soc-acpi-intel-cml-match.c
+++ b/sound/soc/intel/common/soc-acpi-intel-cml-match.c
@@ -59,42 +59,95 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_cml_machines[] = {
};
EXPORT_SYMBOL_GPL(snd_soc_acpi_intel_cml_machines);

-static const u64 rt711_0_adr[] = {
- 0x000010025D071100
+static const struct snd_soc_acpi_endpoint single_endpoint = {
+ .num = 0,
+ .aggregated = 0,
+ .group_position = 0,
+ .group_id = 0,
};

-static const u64 rt1308_1_adr[] = {
- 0x000110025D130800
+static const struct snd_soc_acpi_endpoint spk_l_endpoint = {
+ .num = 0,
+ .aggregated = 1,
+ .group_position = 0,
+ .group_id = 1,
};

-static const u64 rt1308_2_adr[] = {
- 0x000210025D130800
+static const struct snd_soc_acpi_endpoint spk_r_endpoint = {
+ .num = 0,
+ .aggregated = 1,
+ .group_position = 1,
+ .group_id = 1,
};

-static const u64 rt715_3_adr[] = {
- 0x000310025D071500
+static const struct snd_soc_acpi_adr_device rt711_0_adr[] = {
+ {
+ .adr = 0x000010025D071100,
+ .num_endpoints = 1,
+ .endpoints = &single_endpoint,
+ }
+};
+
+static const struct snd_soc_acpi_adr_device rt1308_1_adr[] = {
+ {
+ .adr = 0x000110025D130800,
+ .num_endpoints = 1,
+ .endpoints = &single_endpoint,
+ }
+};
+
+static const struct snd_soc_acpi_adr_device rt1308_2_adr[] = {
+ {
+ .adr = 0x000210025D130800,
+ .num_endpoints = 1,
+ .endpoints = &single_endpoint,
+ }
+};
+
+static const struct snd_soc_acpi_adr_device rt1308_1_group1_adr[] = {
+ {
+ .adr = 0x000110025D130800,
+ .num_endpoints = 1,
+ .endpoints = &spk_l_endpoint,
+ }
+};
+
+static const struct snd_soc_acpi_adr_device rt1308_2_group1_adr[] = {
+ {
+ .adr = 0x000210025D130800,
+ .num_endpoints = 1,
+ .endpoints = &spk_r_endpoint,
+ }
+};
+
+static const struct snd_soc_acpi_adr_device rt715_3_adr[] = {
+ {
+ .adr = 0x000310025D071500,
+ .num_endpoints = 1,
+ .endpoints = &single_endpoint,
+ }
};

static const struct snd_soc_acpi_link_adr cml_3_in_1_default[] = {
{
.mask = BIT(0),
.num_adr = ARRAY_SIZE(rt711_0_adr),
- .adr = rt711_0_adr,
+ .adr_d = rt711_0_adr,
},
{
.mask = BIT(1),
- .num_adr = ARRAY_SIZE(rt1308_1_adr),
- .adr = rt1308_1_adr,
+ .num_adr = ARRAY_SIZE(rt1308_1_group1_adr),
+ .adr_d = rt1308_1_group1_adr,
},
{
.mask = BIT(2),
- .num_adr = ARRAY_SIZE(rt1308_2_adr),
- .adr = rt1308_2_adr,
+ .num_adr = ARRAY_SIZE(rt1308_2_group1_adr),
+ .adr_d = rt1308_2_group1_adr,
},
{
.mask = BIT(3),
.num_adr = ARRAY_SIZE(rt715_3_adr),
- .adr = rt715_3_adr,
+ .adr_d = rt715_3_adr,
},
{}
};
@@ -103,17 +156,17 @@ static const struct snd_soc_acpi_link_adr cml_3_in_1_mono_amp[] = {
{
.mask = BIT(0),
.num_adr = ARRAY_SIZE(rt711_0_adr),
- .adr = rt711_0_adr,
+ .adr_d = rt711_0_adr,
},
{
.mask = BIT(1),
.num_adr = ARRAY_SIZE(rt1308_1_adr),
- .adr = rt1308_1_adr,
+ .adr_d = rt1308_1_adr,
},
{
.mask = BIT(3),
.num_adr = ARRAY_SIZE(rt715_3_adr),
- .adr = rt715_3_adr,
+ .adr_d = rt715_3_adr,
},
{}
};
diff --git a/sound/soc/intel/common/soc-acpi-intel-icl-match.c b/sound/soc/intel/common/soc-acpi-intel-icl-match.c
index 752733013d54..36e2d09cf58c 100644
--- a/sound/soc/intel/common/soc-acpi-intel-icl-match.c
+++ b/sound/soc/intel/common/soc-acpi-intel-icl-match.c
@@ -33,55 +33,112 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_icl_machines[] = {
};
EXPORT_SYMBOL_GPL(snd_soc_acpi_intel_icl_machines);

-static const u64 rt700_0_adr[] = {
- 0x000010025D070000
+static const struct snd_soc_acpi_endpoint single_endpoint = {
+ .num = 0,
+ .aggregated = 0,
+ .group_position = 0,
+ .group_id = 0,
+};
+
+static const struct snd_soc_acpi_endpoint spk_l_endpoint = {
+ .num = 0,
+ .aggregated = 1,
+ .group_position = 0,
+ .group_id = 1,
+};
+
+static const struct snd_soc_acpi_endpoint spk_r_endpoint = {
+ .num = 0,
+ .aggregated = 1,
+ .group_position = 1,
+ .group_id = 1,
+};
+
+static const struct snd_soc_acpi_adr_device rt700_0_adr[] = {
+ {
+ .adr = 0x000010025D070000,
+ .num_endpoints = 1,
+ .endpoints = &single_endpoint,
+ }
};

static const struct snd_soc_acpi_link_adr icl_rvp[] = {
{
.mask = BIT(0),
.num_adr = ARRAY_SIZE(rt700_0_adr),
- .adr = rt700_0_adr,
+ .adr_d = rt700_0_adr,
},
{}
};

-static const u64 rt711_0_adr[] = {
- 0x000010025D071100
+static const struct snd_soc_acpi_adr_device rt711_0_adr[] = {
+ {
+ .adr = 0x000010025D071100,
+ .num_endpoints = 1,
+ .endpoints = &single_endpoint,
+ }
+};
+
+static const struct snd_soc_acpi_adr_device rt1308_1_adr[] = {
+ {
+ .adr = 0x000110025D130800,
+ .num_endpoints = 1,
+ .endpoints = &single_endpoint,
+ }
};

-static const u64 rt1308_1_adr[] = {
- 0x000110025D130800
+static const struct snd_soc_acpi_adr_device rt1308_2_adr[] = {
+ {
+ .adr = 0x000210025D130800,
+ .num_endpoints = 1,
+ .endpoints = &single_endpoint,
+ }
};

-static const u64 rt1308_2_adr[] = {
- 0x000210025D130800
+static const struct snd_soc_acpi_adr_device rt1308_1_group1_adr[] = {
+ {
+ .adr = 0x000110025D130800,
+ .num_endpoints = 1,
+ .endpoints = &spk_l_endpoint,
+ }
};

-static const u64 rt715_3_adr[] = {
- 0x000310025D071500
+static const struct snd_soc_acpi_adr_device rt1308_2_group1_adr[] = {
+ {
+ .adr = 0x000210025D130800,
+ .num_endpoints = 1,
+ .endpoints = &spk_r_endpoint,
+ }
+};
+
+static const struct snd_soc_acpi_adr_device rt715_3_adr[] = {
+ {
+ .adr = 0x000310025D071500,
+ .num_endpoints = 1,
+ .endpoints = &single_endpoint,
+ }
};

static const struct snd_soc_acpi_link_adr icl_3_in_1_default[] = {
{
.mask = BIT(0),
.num_adr = ARRAY_SIZE(rt711_0_adr),
- .adr = rt711_0_adr,
+ .adr_d = rt711_0_adr,
},
{
.mask = BIT(1),
- .num_adr = ARRAY_SIZE(rt1308_1_adr),
- .adr = rt1308_1_adr,
+ .num_adr = ARRAY_SIZE(rt1308_1_group1_adr),
+ .adr_d = rt1308_1_adr,
},
{
.mask = BIT(2),
- .num_adr = ARRAY_SIZE(rt1308_2_adr),
- .adr = rt1308_2_adr,
+ .num_adr = ARRAY_SIZE(rt1308_2_group1_adr),
+ .adr_d = rt1308_2_adr,
},
{
.mask = BIT(3),
.num_adr = ARRAY_SIZE(rt715_3_adr),
- .adr = rt715_3_adr,
+ .adr_d = rt715_3_adr,
},
{}
};
@@ -90,17 +147,17 @@ static const struct snd_soc_acpi_link_adr icl_3_in_1_mono_amp[] = {
{
.mask = BIT(0),
.num_adr = ARRAY_SIZE(rt711_0_adr),
- .adr = rt711_0_adr,
+ .adr_d = rt711_0_adr,
},
{
.mask = BIT(1),
.num_adr = ARRAY_SIZE(rt1308_1_adr),
- .adr = rt1308_1_adr,
+ .adr_d = rt1308_1_adr,
},
{
.mask = BIT(3),
.num_adr = ARRAY_SIZE(rt715_3_adr),
- .adr = rt715_3_adr,
+ .adr_d = rt715_3_adr,
},
{}
};
diff --git a/sound/soc/intel/common/soc-acpi-intel-tgl-match.c b/sound/soc/intel/common/soc-acpi-intel-tgl-match.c
index 5984dd151f3e..885ef4f00385 100644
--- a/sound/soc/intel/common/soc-acpi-intel-tgl-match.c
+++ b/sound/soc/intel/common/soc-acpi-intel-tgl-match.c
@@ -14,20 +14,53 @@ static struct snd_soc_acpi_codecs tgl_codecs = {
.codecs = {"MX98357A"}
};

-static const u64 rt711_0_adr[] = {
- 0x000010025D071100
+static const struct snd_soc_acpi_endpoint single_endpoint = {
+ .num = 0,
+ .aggregated = 0,
+ .group_position = 0,
+ .group_id = 0,
};

-static const u64 rt1308_1_adr[] = {
- 0x000120025D130800,
- 0x000122025D130800
+static const struct snd_soc_acpi_endpoint spk_l_endpoint = {
+ .num = 0,
+ .aggregated = 1,
+ .group_position = 0,
+ .group_id = 1,
+};
+
+static const struct snd_soc_acpi_endpoint spk_r_endpoint = {
+ .num = 0,
+ .aggregated = 1,
+ .group_position = 1,
+ .group_id = 1,
+};
+
+static const struct snd_soc_acpi_adr_device rt711_0_adr[] = {
+ {
+ .adr = 0x000010025D071100,
+ .num_endpoints = 1,
+ .endpoints = &single_endpoint,
+ }
+};
+
+static const struct snd_soc_acpi_adr_device rt1308_1_adr[] = {
+ {
+ .adr = 0x000120025D130800,
+ .num_endpoints = 1,
+ .endpoints = &spk_l_endpoint,
+ },
+ {
+ .adr = 0x000122025D130800,
+ .num_endpoints = 1,
+ .endpoints = &spk_r_endpoint,
+ }
};

static const struct snd_soc_acpi_link_adr tgl_i2s_rt1308[] = {
{
.mask = BIT(0),
.num_adr = ARRAY_SIZE(rt711_0_adr),
- .adr = rt711_0_adr,
+ .adr_d = rt711_0_adr,
},
{}
};
@@ -36,12 +69,12 @@ static const struct snd_soc_acpi_link_adr tgl_rvp[] = {
{
.mask = BIT(0),
.num_adr = ARRAY_SIZE(rt711_0_adr),
- .adr = rt711_0_adr,
+ .adr_d = rt711_0_adr,
},
{
.mask = BIT(1),
.num_adr = ARRAY_SIZE(rt1308_1_adr),
- .adr = rt1308_1_adr,
+ .adr_d = rt1308_1_adr,
},
{}
};
--
2.20.1

2020-03-12 19:35:21

by Pierre-Louis Bossart

[permalink] [raw]
Subject: [PATCH 02/10] ASoC: SOF: Intel: add SoundWire configuration interface

Now that the SoundWire core supports the multi-step initialization,
call the relevant APIs.

The actual hardware enablement can be done in two places, ideally we'd
want to startup the SoundWire IP as soon as possible (while still
taking power rail dependencies into account)

However when suspend/resume is implemented, the DSP device will be
resumed first, and only when the DSP firmware is downloaded/booted
would the SoundWire child devices be resumed, so there are only
marginal benefits in starting the IP earlier for the first probe.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
---
sound/soc/sof/intel/hda-loader.c | 13 ++++
sound/soc/sof/intel/hda.c | 120 +++++++++++++++++++++++++++++++
sound/soc/sof/intel/hda.h | 44 ++++++++++++
3 files changed, 177 insertions(+)

diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c
index 03b05d7f66da..8a37a473f8aa 100644
--- a/sound/soc/sof/intel/hda-loader.c
+++ b/sound/soc/sof/intel/hda-loader.c
@@ -402,6 +402,19 @@ int hda_dsp_pre_fw_run(struct snd_sof_dev *sdev)
/* post fw run operations */
int hda_dsp_post_fw_run(struct snd_sof_dev *sdev)
{
+ int ret;
+
+ if (sdev->first_boot) {
+ ret = hda_sdw_startup(sdev);
+ if (ret < 0) {
+ dev_err(sdev->dev,
+ "error: could not startup SoundWire links\n");
+ return ret;
+ }
+ }
+
+ hda_sdw_int_enable(sdev, true);
+
/* re-enable clock gating and power gating */
return hda_dsp_ctrl_clock_power_gating(sdev, true);
}
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 7ca887041a34..36015dd6c31a 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -18,7 +18,9 @@
#include <sound/hdaudio_ext.h>
#include <sound/hda_register.h>

+#include <linux/acpi.h>
#include <linux/module.h>
+#include <linux/soundwire/sdw_intel.h>
#include <sound/intel-nhlt.h>
#include <sound/sof.h>
#include <sound/sof/xtensa.h>
@@ -34,6 +36,98 @@

#define EXCEPT_MAX_HDR_SIZE 0x400

+#if IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE)
+
+void hda_sdw_int_enable(struct snd_sof_dev *sdev, bool enable)
+{
+ sdw_intel_enable_irq(sdev->bar[HDA_DSP_BAR], enable);
+}
+
+static int hda_sdw_acpi_scan(struct snd_sof_dev *sdev)
+{
+ struct sof_intel_hda_dev *hdev;
+ acpi_handle handle;
+ int ret;
+
+ handle = ACPI_HANDLE(sdev->dev);
+
+ /* save ACPI info for the probe step */
+ hdev = sdev->pdata->hw_pdata;
+
+ ret = sdw_intel_acpi_scan(handle, &hdev->info);
+ if (ret < 0) {
+ dev_err(sdev->dev, "%s failed\n", __func__);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int hda_sdw_probe(struct snd_sof_dev *sdev)
+{
+ struct sof_intel_hda_dev *hdev;
+ struct sdw_intel_res res;
+ acpi_handle handle;
+ void *sdw;
+
+ handle = ACPI_HANDLE(sdev->dev);
+
+ hdev = sdev->pdata->hw_pdata;
+
+ memset(&res, 0, sizeof(res));
+
+ res.mmio_base = sdev->bar[HDA_DSP_BAR];
+ res.irq = sdev->ipc_irq;
+ res.handle = hdev->info.handle;
+ res.parent = sdev->dev;
+
+ /*
+ * ops and arg fields are not populated for now,
+ * they will be needed when the DAI callbacks are
+ * provided
+ */
+
+ /* we could filter links here if needed, e.g for quirks */
+ res.count = hdev->info.count;
+ res.link_mask = hdev->info.link_mask;
+
+ sdw = sdw_intel_probe(&res);
+ if (!sdw) {
+ dev_err(sdev->dev, "error: SoundWire probe failed\n");
+ return -EINVAL;
+ }
+
+ /* save context */
+ hdev->sdw = sdw;
+
+ return 0;
+}
+
+int hda_sdw_startup(struct snd_sof_dev *sdev)
+{
+ struct sof_intel_hda_dev *hdev;
+
+ hdev = sdev->pdata->hw_pdata;
+
+ return sdw_intel_startup(hdev->sdw);
+}
+
+static int hda_sdw_exit(struct snd_sof_dev *sdev)
+{
+ struct sof_intel_hda_dev *hdev;
+
+ hdev = sdev->pdata->hw_pdata;
+
+ hda_sdw_int_enable(sdev, false);
+
+ if (hdev->sdw)
+ sdw_intel_exit(hdev->sdw);
+ hdev->sdw = NULL;
+
+ return 0;
+}
+#endif
+
/*
* Debug
*/
@@ -347,9 +441,12 @@ static const char *fixup_tplg_name(struct snd_sof_dev *sdev,
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;
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
struct hdac_ext_link *hlink;
#endif
+ struct sof_intel_hda_dev *hdev = pdata->hw_pdata;
+ u32 link_mask;
int ret = 0;

device_disable_async_suspend(bus->dev);
@@ -366,6 +463,27 @@ static int hda_init_caps(struct snd_sof_dev *sdev)
return ret;
}

+ /* scan SoundWire capabilities exposed by DSDT */
+ ret = hda_sdw_acpi_scan(sdev);
+ if (ret < 0) {
+ dev_err(sdev->dev, "error: SoundWire ACPI scan error\n");
+ return ret;
+ }
+
+ link_mask = hdev->info.link_mask;
+ if (!link_mask) {
+ /*
+ * probe/allocated SoundWire resources.
+ * The hardware configuration takes place in hda_sdw_startup
+ * after power rails are enabled.
+ */
+ ret = hda_sdw_probe(sdev);
+ if (ret < 0) {
+ dev_err(sdev->dev, "error: SoundWire probe error\n");
+ return ret;
+ }
+ }
+
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
if (bus->mlcap)
snd_hdac_ext_bus_get_ml_capabilities(bus);
@@ -626,6 +744,8 @@ int hda_dsp_remove(struct snd_sof_dev *sdev)
snd_hdac_ext_bus_device_remove(bus);
#endif

+ hda_sdw_exit(sdev);
+
if (!IS_ERR_OR_NULL(hda->dmic_dev))
platform_device_unregister(hda->dmic_dev);

diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 537c0a930a15..3cf2fb5985b9 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -11,6 +11,8 @@
#ifndef __SOF_INTEL_HDA_H
#define __SOF_INTEL_HDA_H

+#include <linux/soundwire/sdw.h>
+#include <linux/soundwire/sdw_intel.h>
#include <sound/compress_driver.h>
#include <sound/hda_codec.h>
#include <sound/hdaudio_ext.h>
@@ -437,6 +439,12 @@ struct sof_intel_hda_dev {

/* delayed work to enter D0I3 opportunistically */
struct delayed_work d0i3_work;
+
+ /* ACPI information stored between scan and probe steps */
+ struct sdw_intel_acpi_info info;
+
+ /* sdw context allocated by SoundWire driver */
+ struct sdw_intel_ctx *sdw;
};

static inline struct hdac_bus *sof_to_bus(struct snd_sof_dev *s)
@@ -655,6 +663,42 @@ int hda_dsp_trace_init(struct snd_sof_dev *sdev, u32 *stream_tag);
int hda_dsp_trace_release(struct snd_sof_dev *sdev);
int hda_dsp_trace_trigger(struct snd_sof_dev *sdev, int cmd);

+/*
+ * SoundWire support
+ */
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE)
+
+int hda_sdw_startup(struct snd_sof_dev *sdev);
+void hda_sdw_int_enable(struct snd_sof_dev *sdev, bool enable);
+
+#else
+
+static inline int hda_sdw_acpi_scan(struct snd_sof_dev *sdev)
+{
+ return 0;
+}
+
+static inline int hda_sdw_probe(struct snd_sof_dev *sdev)
+{
+ return 0;
+}
+
+static inline int hda_sdw_startup(struct snd_sof_dev *sdev)
+{
+ return 0;
+}
+
+static inline int hda_sdw_exit(struct snd_sof_dev *sdev)
+{
+ return 0;
+}
+
+static inline void hda_sdw_int_enable(struct snd_sof_dev *sdev, bool enable)
+{
+}
+
+#endif
+
/* common dai driver */
extern struct snd_soc_dai_driver skl_dai[];

--
2.20.1

2020-03-12 19:35:44

by Pierre-Louis Bossart

[permalink] [raw]
Subject: [PATCH 04/10] ASoC: SOF: Intel: hda: add SoundWire stream config/free callbacks

These callbacks are invoked when a matching hw_params/hw_free() DAI
operation takes place, and will result in IPC operations with the SOF
firmware.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
---
sound/soc/sof/intel/hda.c | 71 +++++++++++++++++++++++++++++++++++++++
1 file changed, 71 insertions(+)

diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 36015dd6c31a..f257b21d2551 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -24,6 +24,7 @@
#include <sound/intel-nhlt.h>
#include <sound/sof.h>
#include <sound/sof/xtensa.h>
+#include "../sof-audio.h"
#include "../ops.h"
#include "hda.h"

@@ -38,6 +39,74 @@

#if IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE)

+static int sdw_params_stream(struct device *dev,
+ struct sdw_intel_stream_params_data *params_data)
+{
+ struct snd_sof_dev *sdev = dev_get_drvdata(dev);
+ struct snd_soc_dai *d = params_data->dai;
+ struct sof_ipc_dai_config config;
+ struct sof_ipc_reply reply;
+ int link_id = params_data->link_id;
+ int alh_stream_id = params_data->alh_stream_id;
+ int ret;
+ u32 size = sizeof(config);
+
+ memset(&config, 0, size);
+ config.hdr.size = size;
+ config.hdr.cmd = SOF_IPC_GLB_DAI_MSG | SOF_IPC_DAI_CONFIG;
+ config.type = SOF_DAI_INTEL_ALH;
+ config.dai_index = (link_id << 8) | (d->id);
+ config.alh.stream_id = alh_stream_id;
+
+ /* send message to DSP */
+ ret = sof_ipc_tx_message(sdev->ipc,
+ config.hdr.cmd, &config, size, &reply,
+ sizeof(reply));
+ if (ret < 0) {
+ dev_err(sdev->dev,
+ "error: failed to set DAI hw_params for link %d dai->id %d ALH %d\n",
+ link_id, d->id, alh_stream_id);
+ }
+
+ return ret;
+}
+
+static int sdw_free_stream(struct device *dev,
+ struct sdw_intel_stream_free_data *free_data)
+{
+ struct snd_sof_dev *sdev = dev_get_drvdata(dev);
+ struct snd_soc_dai *d = free_data->dai;
+ struct sof_ipc_dai_config config;
+ struct sof_ipc_reply reply;
+ int link_id = free_data->link_id;
+ int ret;
+ u32 size = sizeof(config);
+
+ memset(&config, 0, size);
+ config.hdr.size = size;
+ config.hdr.cmd = SOF_IPC_GLB_DAI_MSG | SOF_IPC_DAI_CONFIG;
+ config.type = SOF_DAI_INTEL_ALH;
+ config.dai_index = (link_id << 8) | d->id;
+ config.alh.stream_id = 0xFFFF; /* invalid value on purpose */
+
+ /* send message to DSP */
+ ret = sof_ipc_tx_message(sdev->ipc,
+ config.hdr.cmd, &config, size, &reply,
+ sizeof(reply));
+ if (ret < 0) {
+ dev_err(sdev->dev,
+ "error: failed to free stream for link %d dai->id %d\n",
+ link_id, d->id);
+ }
+
+ return ret;
+}
+
+static const struct sdw_intel_ops sdw_callback = {
+ .params_stream = sdw_params_stream,
+ .free_stream = sdw_free_stream,
+};
+
void hda_sdw_int_enable(struct snd_sof_dev *sdev, bool enable)
{
sdw_intel_enable_irq(sdev->bar[HDA_DSP_BAR], enable);
@@ -80,6 +149,8 @@ static int hda_sdw_probe(struct snd_sof_dev *sdev)
res.irq = sdev->ipc_irq;
res.handle = hdev->info.handle;
res.parent = sdev->dev;
+ res.ops = &sdw_callback;
+ res.dev = sdev->dev;

/*
* ops and arg fields are not populated for now,
--
2.20.1

2020-03-12 19:35:49

by Pierre-Louis Bossart

[permalink] [raw]
Subject: [PATCH 05/10] ASoC: SOF: Intel: hda: initial SoundWire machine driver autodetect

For now we have a limited number of machine driver configurations, and
we can detect them based on the link configuration returned after
checking hardware and firmware (BIOS) configurations.

The link configuration is checked with a link_mask as well as a list
of _ADR descriptors for each link.

There is a chance that in extreme cases where the BIOS contains too
much information we would need to detect which Slave devices actually
report as 'attached'. This would be more accurate than static
table-based solutions, but it also introduces timing dependencies
since we don't know when those devices might become attached, so will
only be only be looked at if we see limitations with static methods
and the usual quirks based e.g. on DMI information.

Signed-off-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
Signed-off-by: Pierre-Louis Bossart <[email protected]>
---
sound/soc/sof/intel/hda.c | 164 ++++++++++++++++++++++++++++++++++----
1 file changed, 149 insertions(+), 15 deletions(-)

diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index f257b21d2551..1892f612c11d 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -20,6 +20,7 @@

#include <linux/acpi.h>
#include <linux/module.h>
+#include <linux/soundwire/sdw.h>
#include <linux/soundwire/sdw_intel.h>
#include <sound/intel-nhlt.h>
#include <sound/sof.h>
@@ -136,11 +137,8 @@ static int hda_sdw_probe(struct snd_sof_dev *sdev)
{
struct sof_intel_hda_dev *hdev;
struct sdw_intel_res res;
- acpi_handle handle;
void *sdw;

- handle = ACPI_HANDLE(sdev->dev);
-
hdev = sdev->pdata->hw_pdata;

memset(&res, 0, sizeof(res));
@@ -180,6 +178,9 @@ int hda_sdw_startup(struct snd_sof_dev *sdev)

hdev = sdev->pdata->hw_pdata;

+ if (!hdev->sdw)
+ return 0;
+
return sdw_intel_startup(hdev->sdw);
}

@@ -537,24 +538,31 @@ static int hda_init_caps(struct snd_sof_dev *sdev)
/* scan SoundWire capabilities exposed by DSDT */
ret = hda_sdw_acpi_scan(sdev);
if (ret < 0) {
- dev_err(sdev->dev, "error: SoundWire ACPI scan error\n");
- return ret;
+ dev_dbg(sdev->dev, "skipping SoundWire, ACPI scan error\n");
+ goto skip_soundwire;
}

link_mask = hdev->info.link_mask;
if (!link_mask) {
- /*
- * probe/allocated SoundWire resources.
- * The hardware configuration takes place in hda_sdw_startup
- * after power rails are enabled.
- */
- ret = hda_sdw_probe(sdev);
- if (ret < 0) {
- dev_err(sdev->dev, "error: SoundWire probe error\n");
- return ret;
- }
+ dev_dbg(sdev->dev, "skipping SoundWire, no links enabled\n");
+ goto skip_soundwire;
}

+ /*
+ * probe/allocate SoundWire resources.
+ * The hardware configuration takes place in hda_sdw_startup
+ * after power rails are enabled.
+ * It's entirely possible to have a mix of I2S/DMIC/SoundWire
+ * devices, so we allocate the resources in all cases.
+ */
+ ret = hda_sdw_probe(sdev);
+ if (ret < 0) {
+ dev_err(sdev->dev, "error: SoundWire probe error\n");
+ return ret;
+ }
+
+skip_soundwire:
+
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
if (bus->mlcap)
snd_hdac_ext_bus_get_ml_capabilities(bus);
@@ -953,6 +961,122 @@ static int hda_generic_machine_select(struct snd_sof_dev *sdev)
}
#endif

+#if IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE)
+/* Check if all Slaves defined on the link can be found */
+static bool link_slaves_found(struct snd_sof_dev *sdev,
+ const struct snd_soc_acpi_link_adr *link,
+ struct sdw_intel_ctx *sdw)
+{
+ struct hdac_bus *bus = sof_to_bus(sdev);
+ struct sdw_intel_slave_id *ids = sdw->ids;
+ int num_slaves = sdw->num_slaves;
+ unsigned int part_id, link_id, unique_id, mfg_id;
+ int i, j;
+
+ for (i = 0; i < link->num_adr; i++) {
+ u64 adr = link->adr_d[i].adr;
+
+ mfg_id = SDW_MFG_ID(adr);
+ part_id = SDW_PART_ID(adr);
+ link_id = SDW_DISCO_LINK_ID(adr);
+ for (j = 0; j < num_slaves; j++) {
+ if (ids[j].link_id != link_id ||
+ ids[j].id.part_id != part_id ||
+ ids[j].id.mfg_id != mfg_id)
+ continue;
+ /*
+ * we have to check unique id
+ * if there is more than one
+ * Slave on the link
+ */
+ unique_id = SDW_UNIQUE_ID(adr);
+ if (link->num_adr == 1 ||
+ ids[j].id.unique_id == SDW_IGNORED_UNIQUE_ID ||
+ ids[j].id.unique_id == unique_id) {
+ dev_dbg(bus->dev,
+ "found %x at link %d\n",
+ part_id, link_id);
+ break;
+ }
+ }
+ if (j == num_slaves) {
+ dev_dbg(bus->dev,
+ "Slave %x not found\n",
+ part_id);
+ return false;
+ }
+ }
+ return true;
+}
+
+static int hda_sdw_machine_select(struct snd_sof_dev *sdev)
+{
+ struct snd_sof_pdata *pdata = sdev->pdata;
+ const struct snd_soc_acpi_link_adr *link;
+ struct hdac_bus *bus = sof_to_bus(sdev);
+ struct snd_soc_acpi_mach *mach;
+ struct sof_intel_hda_dev *hdev;
+ u32 link_mask;
+ int i;
+
+ hdev = pdata->hw_pdata;
+ link_mask = hdev->info.link_mask;
+
+ /*
+ * Select SoundWire machine driver if needed using the
+ * alternate tables. This case deals with SoundWire-only
+ * machines, for mixed cases with I2C/I2S the detection relies
+ * on the HID list.
+ */
+ if (link_mask && !pdata->machine) {
+ for (mach = pdata->desc->alt_machines;
+ mach && mach->link_mask; mach++) {
+ if (mach->link_mask != link_mask)
+ continue;
+
+ /* No need to match adr if there is no links defined */
+ if (!mach->links)
+ break;
+
+ link = mach->links;
+ for (i = 0; i < hdev->info.count; i++, link++) {
+ /*
+ * Try next machine if any expected Slaves
+ * are not found on this link.
+ */
+ if (!link_slaves_found(sdev, link, hdev->sdw))
+ break;
+ }
+ /* Found if all Slaves are checked */
+ if (i == hdev->info.count)
+ break;
+ }
+ if (mach && mach->link_mask) {
+ dev_dbg(bus->dev,
+ "SoundWire machine driver %s topology %s\n",
+ mach->drv_name,
+ mach->sof_tplg_filename);
+ pdata->machine = mach;
+ mach->mach_params.links = mach->links;
+ mach->mach_params.link_mask = mach->link_mask;
+ mach->mach_params.platform = dev_name(sdev->dev);
+ pdata->fw_filename = mach->sof_fw_filename;
+ pdata->tplg_filename = mach->sof_tplg_filename;
+ } else {
+ dev_info(sdev->dev,
+ "No SoundWire machine driver found\n");
+ }
+ }
+
+ return 0;
+}
+#else
+static int hda_sdw_machine_select(struct snd_sof_dev *sdev)
+{
+ return 0;
+}
+#endif
+
void hda_set_mach_params(const struct snd_soc_acpi_mach *mach,
struct device *dev)
{
@@ -972,8 +1096,18 @@ void hda_machine_select(struct snd_sof_dev *sdev)
if (mach) {
sof_pdata->tplg_filename = mach->sof_tplg_filename;
sof_pdata->machine = mach;
+
+ if (mach->link_mask) {
+ mach->mach_params.links = mach->links;
+ mach->mach_params.link_mask = mach->link_mask;
+ }
}

+ /*
+ * If I2S fails, try SoundWire
+ */
+ hda_sdw_machine_select(sdev);
+
/*
* Choose HDA generic machine driver if mach is NULL.
* Otherwise, set certain mach params.
--
2.20.1

2020-03-12 19:36:01

by Pierre-Louis Bossart

[permalink] [raw]
Subject: [PATCH 06/10] ASoC: SOF: Intel: hda: disable SoundWire interrupts on suspend

Doing this avoid conflicts and errors reported on the bus.

The interrupts are only re-enabled on resume after the firmware is
downloaded, so the behavior is not fully symmetric

Signed-off-by: Pierre-Louis Bossart <[email protected]>
---
sound/soc/sof/intel/hda-dsp.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index 79ce52c32ef1..225146c57f2e 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -555,6 +555,8 @@ static int hda_suspend(struct snd_sof_dev *sdev, bool runtime_suspend)
#endif
int ret;

+ hda_sdw_int_enable(sdev, false);
+
/* disable IPC interrupts */
hda_dsp_ipc_int_disable(sdev);

--
2.20.1

2020-03-12 19:36:21

by Pierre-Louis Bossart

[permalink] [raw]
Subject: [PATCH 09/10] ASoC: SOF: Intel: hda: add WAKEEN interrupt support for SoundWire

From: Rander Wang <[email protected]>

When a SoundWire link is in clock stop state, a Slave device may wake
up the Master for some events such as jack detection. The WAKEEN
interrupt will be triggered and processed by the audio pci device.

If audio device is in D3, the interrupt will be routed to PME, or
aggregated at cAVS level as interrupt when audio device is in D0. This
patch only supports D3 case, where the audio pci device will be
resumed by a PME event and the WAKEEN interrupt will be processed
after audio pci device is powered up and ROM is initialized
successfully.

The WAKEEN handling is only enabled after the first boot due to
dependencies on a shim_lock mutex being initialized.

Signed-off-by: Rander Wang <[email protected]>
Signed-off-by: Pierre-Louis Bossart <[email protected]>
---
sound/soc/sof/intel/hda-loader.c | 18 ++++++++++++++++++
sound/soc/sof/intel/hda.c | 11 +++++++++++
sound/soc/sof/intel/hda.h | 5 +++++
3 files changed, 34 insertions(+)

diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c
index 8a37a473f8aa..4549d60d8cf8 100644
--- a/sound/soc/sof/intel/hda-loader.c
+++ b/sound/soc/sof/intel/hda-loader.c
@@ -349,6 +349,24 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev)
goto cleanup;
}

+ /*
+ * When a SoundWire link is in clock stop state, a Slave
+ * device may trigger in-band wakes for events such as jack
+ * insertion or acoustic event detection. This event will lead
+ * to a WAKEEN interrupt, handled by the PCI device and routed
+ * to PME if the PCI device is in D3. The resume function in
+ * audio PCI driver will be invoked by ACPI for PME event and
+ * initialize the device and process WAKEEN interrupt.
+ *
+ * The WAKEEN interrupt should be processed ASAP to prevent an
+ * interrupt flood, otherwise other interrupts, such IPC,
+ * cannot work normally. The WAKEEN is handled after the ROM
+ * is initialized successfully, which ensures power rails are
+ * enabled before accessing the SoundWire SHIM registers
+ */
+ if (!sdev->first_boot)
+ hda_sdw_process_wakeen(sdev);
+
/*
* at this point DSP ROM has been initialized and
* should be ready for code loading and firmware boot
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 32b02d3b6536..9c7a816183b6 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -241,6 +241,17 @@ static irqreturn_t hda_dsp_sdw_thread(int irq, void *context)
return sdw_intel_thread(irq, context);
}

+void hda_sdw_process_wakeen(struct snd_sof_dev *sdev)
+{
+ struct sof_intel_hda_dev *hdev;
+
+ hdev = sdev->pdata->hw_pdata;
+ if (!hdev->sdw)
+ return;
+
+ sdw_intel_process_wakeen_event(hdev->sdw);
+}
+
#endif

/*
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 2b72eefe1635..69717a38913c 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -672,6 +672,7 @@ int hda_dsp_trace_trigger(struct snd_sof_dev *sdev, int cmd);

int hda_sdw_startup(struct snd_sof_dev *sdev);
void hda_sdw_int_enable(struct snd_sof_dev *sdev, bool enable);
+void hda_sdw_process_wakeen(struct snd_sof_dev *sdev);

#else

@@ -708,6 +709,10 @@ static inline irqreturn_t hda_dsp_sdw_thread(int irq, void *context)
{
return IRQ_HANDLED;
}
+
+static inline void hda_sdw_process_wakeen(struct snd_sof_dev *sdev)
+{
+}
#endif

/* common dai driver */
--
2.20.1

2020-03-12 19:36:33

by Pierre-Louis Bossart

[permalink] [raw]
Subject: [PATCH 10/10] Asoc: SOF: Intel: hda: check SoundWire wakeen interrupt in irq thread

From: Rander Wang <[email protected]>

If pci device is in D0, wakeen interrupt will be
aggregated at cAVS level as interrupt. This commit
check the wakeen status and process it in irq thread

Signed-off-by: Rander Wang <[email protected]>
Signed-off-by: Pierre-Louis Bossart <[email protected]>
---
sound/soc/sof/intel/hda.c | 16 ++++++++++++++++
sound/soc/sof/intel/hda.h | 6 ++++++
2 files changed, 22 insertions(+)

diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 9c7a816183b6..b5206dc4607a 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -241,6 +241,19 @@ static irqreturn_t hda_dsp_sdw_thread(int irq, void *context)
return sdw_intel_thread(irq, context);
}

+static bool hda_sdw_check_wakeen_irq(struct snd_sof_dev *sdev)
+{
+ struct sof_intel_hda_dev *hdev;
+
+ hdev = sdev->pdata->hw_pdata;
+ if (hdev->sdw &&
+ snd_sof_dsp_read(sdev, HDA_DSP_BAR,
+ HDA_DSP_REG_SNDW_WAKE_STS))
+ return true;
+
+ return false;
+}
+
void hda_sdw_process_wakeen(struct snd_sof_dev *sdev)
{
struct sof_intel_hda_dev *hdev;
@@ -685,6 +698,9 @@ static irqreturn_t hda_dsp_interrupt_thread(int irq, void *context)
if (hda_dsp_check_sdw_irq(sdev))
hda_dsp_sdw_thread(irq, hdev->sdw);

+ if (hda_sdw_check_wakeen_irq(sdev))
+ hda_sdw_process_wakeen(sdev);
+
/* enable GIE interrupt */
snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR,
SOF_HDA_INTCTL,
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 69717a38913c..53f563aa782d 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -234,6 +234,7 @@
#define HDA_DSP_REG_ADSPIS2 (HDA_DSP_GEN_BASE + 0x14)

#define HDA_DSP_REG_ADSPIS2_SNDW BIT(5)
+#define HDA_DSP_REG_SNDW_WAKE_STS 0x2C192

/* Intel HD Audio Inter-Processor Communication Registers */
#define HDA_DSP_IPC_BASE 0x40
@@ -710,6 +711,11 @@ static inline irqreturn_t hda_dsp_sdw_thread(int irq, void *context)
return IRQ_HANDLED;
}

+static inline bool hda_sdw_check_wakeen_irq(struct snd_sof_dev *sdev)
+{
+ return false;
+}
+
static inline void hda_sdw_process_wakeen(struct snd_sof_dev *sdev)
{
}
--
2.20.1

2020-03-12 19:36:41

by Pierre-Louis Bossart

[permalink] [raw]
Subject: [PATCH 03/10] ASoC: SOF: IPC: dai-intel: move ALH declarations in header file

ALH was inserted in the wrong place during integration, add after DMIC
to mirror the file used by SOF firmware.

No functional change, just text move in the same file to better track
changes, if any.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
---
include/sound/sof/dai-intel.h | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/sound/sof/dai-intel.h b/include/sound/sof/dai-intel.h
index 5f1ef5565be6..04e48227f542 100644
--- a/include/sound/sof/dai-intel.h
+++ b/include/sound/sof/dai-intel.h
@@ -87,6 +87,15 @@ struct sof_ipc_dai_hda_params {
uint32_t link_dma_ch;
} __packed;

+/* ALH Configuration Request - SOF_IPC_DAI_ALH_CONFIG */
+struct sof_ipc_dai_alh_params {
+ struct sof_ipc_hdr hdr;
+ uint32_t stream_id;
+
+ /* reserved for future use */
+ uint32_t reserved[15];
+} __packed;
+
/* DMIC Configuration Request - SOF_IPC_DAI_DMIC_CONFIG */

/* This struct is defined per 2ch PDM controller available in the platform.
@@ -179,13 +188,4 @@ struct sof_ipc_dai_dmic_params {
struct sof_ipc_dai_dmic_pdm_ctrl pdm[0];
} __packed;

-/* ALH Configuration Request - SOF_IPC_DAI_ALH_CONFIG */
-struct sof_ipc_dai_alh_params {
- struct sof_ipc_hdr hdr;
- uint32_t stream_id;
-
- /* reserved for future use */
- uint32_t reserved[15];
-} __packed;
-
#endif
--
2.20.1

2020-03-12 19:37:11

by Pierre-Louis Bossart

[permalink] [raw]
Subject: [PATCH 07/10] ASoC: SOF: Intel: hda: merge IPC, stream and SoundWire interrupt handlers

From: Bard Liao <[email protected]>

We have a single irq handler for SOF interrupts. We can further merge
SoundWire ones to completely remove MSI interrupts handling issues
leading to timeouts.

Signed-off-by: Bard Liao <[email protected]>
Signed-off-by: Pierre-Louis Bossart <[email protected]>
---
sound/soc/sof/intel/hda.c | 36 ++++++++++++++++++++++++++++++++++++
sound/soc/sof/intel/hda.h | 11 +++++++++++
2 files changed, 47 insertions(+)

diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 1892f612c11d..f7e5371789f2 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -198,6 +198,38 @@ static int hda_sdw_exit(struct snd_sof_dev *sdev)

return 0;
}
+
+static bool hda_dsp_check_sdw_irq(struct snd_sof_dev *sdev)
+{
+ struct sof_intel_hda_dev *hdev;
+ bool ret = false;
+ u32 irq_status;
+
+ hdev = sdev->pdata->hw_pdata;
+
+ if (!hdev->sdw)
+ return ret;
+
+ /* store status */
+ irq_status = snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_REG_ADSPIS2);
+
+ /* invalid message ? */
+ if (irq_status == 0xffffffff)
+ goto out;
+
+ /* SDW message ? */
+ if (irq_status & HDA_DSP_REG_ADSPIS2_SNDW)
+ ret = true;
+
+out:
+ return ret;
+}
+
+static irqreturn_t hda_dsp_sdw_thread(int irq, void *context)
+{
+ return sdw_intel_thread(irq, context);
+}
+
#endif

/*
@@ -619,6 +651,7 @@ static irqreturn_t hda_dsp_interrupt_handler(int irq, void *context)
static irqreturn_t hda_dsp_interrupt_thread(int irq, void *context)
{
struct snd_sof_dev *sdev = context;
+ struct sof_intel_hda_dev *hdev = sdev->pdata->hw_pdata;

/* deal with streams and controller first */
if (hda_dsp_check_stream_irq(sdev))
@@ -627,6 +660,9 @@ static irqreturn_t hda_dsp_interrupt_thread(int irq, void *context)
if (hda_dsp_check_ipc_irq(sdev))
sof_ops(sdev)->irq_thread(irq, sdev);

+ if (hda_dsp_check_sdw_irq(sdev))
+ hda_dsp_sdw_thread(irq, hdev->sdw);
+
/* enable GIE interrupt */
snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR,
SOF_HDA_INTCTL,
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 3cf2fb5985b9..2b72eefe1635 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -233,6 +233,8 @@
#define HDA_DSP_REG_ADSPIC2 (HDA_DSP_GEN_BASE + 0x10)
#define HDA_DSP_REG_ADSPIS2 (HDA_DSP_GEN_BASE + 0x14)

+#define HDA_DSP_REG_ADSPIS2_SNDW BIT(5)
+
/* Intel HD Audio Inter-Processor Communication Registers */
#define HDA_DSP_IPC_BASE 0x40
#define HDA_DSP_REG_HIPCT (HDA_DSP_IPC_BASE + 0x00)
@@ -697,6 +699,15 @@ static inline void hda_sdw_int_enable(struct snd_sof_dev *sdev, bool enable)
{
}

+static inline bool hda_dsp_check_sdw_irq(struct snd_sof_dev *sdev)
+{
+ return false;
+}
+
+static inline irqreturn_t hda_dsp_sdw_thread(int irq, void *context)
+{
+ return IRQ_HANDLED;
+}
#endif

/* common dai driver */
--
2.20.1

2020-03-12 19:37:21

by Pierre-Louis Bossart

[permalink] [raw]
Subject: [PATCH 08/10] ASoC: SOF: Intel: hda: add parameter to control SoundWire clock stop quirks

Add module parameter so that the different modes can be quickly tested.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
---
sound/soc/sof/intel/hda.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index f7e5371789f2..32b02d3b6536 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -40,6 +40,16 @@

#if IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE)

+/*
+ * The default for SoundWire clock stop quirks is to power gate the IP
+ * and do a Bus Reset, this will need to be modified when the DSP
+ * needs to remain in D0i3 so that the Master does not lose context
+ * and enumeration is not required on clock restart
+ */
+static int sdw_clock_stop_quirks = SDW_INTEL_CLK_STOP_BUS_RESET;
+module_param(sdw_clock_stop_quirks, int, 0444);
+MODULE_PARM_DESC(sdw_clock_stop_quirks, "SOF SoundWire clock stop quirks");
+
static int sdw_params_stream(struct device *dev,
struct sdw_intel_stream_params_data *params_data)
{
@@ -149,6 +159,7 @@ static int hda_sdw_probe(struct snd_sof_dev *sdev)
res.parent = sdev->dev;
res.ops = &sdw_callback;
res.dev = sdev->dev;
+ res.clock_stop_quirks = sdw_clock_stop_quirks;

/*
* ops and arg fields are not populated for now,
--
2.20.1

2020-03-13 08:10:47

by Amadeusz Sławiński

[permalink] [raw]
Subject: Re: [PATCH 01/10] ASoC: soc-acpi: expand description of _ADR-based devices



On 3/12/2020 8:33 PM, Pierre-Louis Bossart wrote:
> For SoundWire, we need to know if endpoints needs to be 'aggregated'
> (MIPI parlance, meaning logically grouped), e.g. when two speaker
> amplifiers need to be handled as a single logical output.
>
> We don't necessarily have the information at the firmware (BIOS)
> level, so add a notion of endpoints and specify if a device/endpoint
> is part of a group, with a position.
>
> This may be expanded in future solutions, for now only provide a group
> and position information.
>
> Since we modify the header file, change all existing upstream tables
> as well to avoid breaking compilation/bisect.
>
> Signed-off-by: Pierre-Louis Bossart <[email protected]>
> ---
> include/sound/soc-acpi.h | 39 ++++++--
> .../intel/common/soc-acpi-intel-cml-match.c | 87 +++++++++++++----
> .../intel/common/soc-acpi-intel-icl-match.c | 97 +++++++++++++++----
> .../intel/common/soc-acpi-intel-tgl-match.c | 49 ++++++++--
> 4 files changed, 221 insertions(+), 51 deletions(-)
>

(...)

> diff --git a/sound/soc/intel/common/soc-acpi-intel-icl-match.c b/sound/soc/intel/common/soc-acpi-intel-icl-match.c
> index 752733013d54..36e2d09cf58c 100644
> --- a/sound/soc/intel/common/soc-acpi-intel-icl-match.c
> +++ b/sound/soc/intel/common/soc-acpi-intel-icl-match.c
> @@ -33,55 +33,112 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_icl_machines[] = {
> };
> EXPORT_SYMBOL_GPL(snd_soc_acpi_intel_icl_machines);
>
> -static const u64 rt700_0_adr[] = {
> - 0x000010025D070000
> +static const struct snd_soc_acpi_endpoint single_endpoint = {
> + .num = 0,
> + .aggregated = 0,
> + .group_position = 0,
> + .group_id = 0,
> +};
> +
> +static const struct snd_soc_acpi_endpoint spk_l_endpoint = {
> + .num = 0,
> + .aggregated = 1,
> + .group_position = 0,
> + .group_id = 1,
> +};
> +
> +static const struct snd_soc_acpi_endpoint spk_r_endpoint = {
> + .num = 0,
> + .aggregated = 1,
> + .group_position = 1,
> + .group_id = 1,
> +};
> +
> +static const struct snd_soc_acpi_adr_device rt700_0_adr[] = {
> + {
> + .adr = 0x000010025D070000,
> + .num_endpoints = 1,
> + .endpoints = &single_endpoint,
> + }
> };
>
> static const struct snd_soc_acpi_link_adr icl_rvp[] = {
> {
> .mask = BIT(0),
> .num_adr = ARRAY_SIZE(rt700_0_adr),
> - .adr = rt700_0_adr,
> + .adr_d = rt700_0_adr,
> },
> {}
> };
>
> -static const u64 rt711_0_adr[] = {
> - 0x000010025D071100
> +static const struct snd_soc_acpi_adr_device rt711_0_adr[] = {
> + {
> + .adr = 0x000010025D071100,
> + .num_endpoints = 1,
> + .endpoints = &single_endpoint,
> + }
> +};
> +
> +static const struct snd_soc_acpi_adr_device rt1308_1_adr[] = {
> + {
> + .adr = 0x000110025D130800,
> + .num_endpoints = 1,
> + .endpoints = &single_endpoint,
> + }
> };
>
> -static const u64 rt1308_1_adr[] = {
> - 0x000110025D130800
> +static const struct snd_soc_acpi_adr_device rt1308_2_adr[] = {
> + {
> + .adr = 0x000210025D130800,
> + .num_endpoints = 1,
> + .endpoints = &single_endpoint,
> + }
> };
>
> -static const u64 rt1308_2_adr[] = {
> - 0x000210025D130800
> +static const struct snd_soc_acpi_adr_device rt1308_1_group1_adr[] = {
> + {
> + .adr = 0x000110025D130800,
> + .num_endpoints = 1,
> + .endpoints = &spk_l_endpoint,
> + }
> };
>
> -static const u64 rt715_3_adr[] = {
> - 0x000310025D071500
> +static const struct snd_soc_acpi_adr_device rt1308_2_group1_adr[] = {
> + {
> + .adr = 0x000210025D130800,
> + .num_endpoints = 1,
> + .endpoints = &spk_r_endpoint,
> + }
> +};
> +
> +static const struct snd_soc_acpi_adr_device rt715_3_adr[] = {
> + {
> + .adr = 0x000310025D071500,
> + .num_endpoints = 1,
> + .endpoints = &single_endpoint,
> + }
> };
>
> static const struct snd_soc_acpi_link_adr icl_3_in_1_default[] = {
> {
> .mask = BIT(0),
> .num_adr = ARRAY_SIZE(rt711_0_adr),
> - .adr = rt711_0_adr,
> + .adr_d = rt711_0_adr,
> },
> {
> .mask = BIT(1),
> - .num_adr = ARRAY_SIZE(rt1308_1_adr),
> - .adr = rt1308_1_adr,
> + .num_adr = ARRAY_SIZE(rt1308_1_group1_adr),
> + .adr_d = rt1308_1_adr,

Is this right, you use different struct in ARRAY_SIZE and assignment?

> },
> {
> .mask = BIT(2),
> - .num_adr = ARRAY_SIZE(rt1308_2_adr),
> - .adr = rt1308_2_adr,
> + .num_adr = ARRAY_SIZE(rt1308_2_group1_adr),
> + .adr_d = rt1308_2_adr,

Same here.

> },
> {
> .mask = BIT(3),
> .num_adr = ARRAY_SIZE(rt715_3_adr),
> - .adr = rt715_3_adr,
> + .adr_d = rt715_3_adr,
> },
> {}
> };
> @@ -90,17 +147,17 @@ static const struct snd_soc_acpi_link_adr icl_3_in_1_mono_amp[] = {
> {
> .mask = BIT(0),
> .num_adr = ARRAY_SIZE(rt711_0_adr),
> - .adr = rt711_0_adr,
> + .adr_d = rt711_0_adr,
> },
> {
> .mask = BIT(1),
> .num_adr = ARRAY_SIZE(rt1308_1_adr),
> - .adr = rt1308_1_adr,
> + .adr_d = rt1308_1_adr,
> },
> {
> .mask = BIT(3),
> .num_adr = ARRAY_SIZE(rt715_3_adr),
> - .adr = rt715_3_adr,
> + .adr_d = rt715_3_adr,
> },
> {}
> };

(...)

2020-03-13 17:28:56

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: [PATCH 01/10] ASoC: soc-acpi: expand description of _ADR-based devices


>>   static const struct snd_soc_acpi_link_adr icl_3_in_1_default[] = {
>>       {
>>           .mask = BIT(0),
>>           .num_adr = ARRAY_SIZE(rt711_0_adr),
>> -        .adr = rt711_0_adr,
>> +        .adr_d = rt711_0_adr,
>>       },
>>       {
>>           .mask = BIT(1),
>> -        .num_adr = ARRAY_SIZE(rt1308_1_adr),
>> -        .adr = rt1308_1_adr,
>> +        .num_adr = ARRAY_SIZE(rt1308_1_group1_adr),
>> +        .adr_d = rt1308_1_adr,
>
> Is this right, you use different struct in ARRAY_SIZE and assignment?
>
>>       },
>>       {
>>           .mask = BIT(2),
>> -        .num_adr = ARRAY_SIZE(rt1308_2_adr),
>> -        .adr = rt1308_2_adr,
>> +        .num_adr = ARRAY_SIZE(rt1308_2_group1_adr),
>> +        .adr_d = rt1308_2_adr,
>
> Same here.

it's of course an editing issue, thanks for spotting this.
it should be the exact same things as the structure used for cml:

static const struct snd_soc_acpi_link_adr cml_3_in_1_default[] = {
{
.mask = BIT(0),
.num_adr = ARRAY_SIZE(rt711_0_adr),
.adr_d = rt711_0_adr,
},
{
.mask = BIT(1),
.num_adr = ARRAY_SIZE(rt1308_1_group1_adr),
.adr_d = rt1308_1_group1_adr,
},
{
.mask = BIT(2),
.num_adr = ARRAY_SIZE(rt1308_2_group1_adr),
.adr_d = rt1308_2_group1_adr,
},
{
.mask = BIT(3),
.num_adr = ARRAY_SIZE(rt715_3_adr),
l .adr_d = rt715_3_adr,
},
{}
};