All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] ALSA: hda: New NHLT functions and cleanup
@ 2021-11-10 10:31 Cezary Rojewski
  2021-11-10 10:31 ` [PATCH v3 1/4] ALSA: hda: Follow ACPI convention in NHLT struct naming Cezary Rojewski
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Cezary Rojewski @ 2021-11-10 10:31 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, Cezary Rojewski, tiwai, hdegoede, broonie

Changes add two crucial functions: endpoint presence-check and
retrieval of endpoint's BLOB (hardware configuration) to NHLT API.

Few cleanups accompany the above:
Work is done to align NHLT-struct naming with other, commonly used
ACPI-structs. While cleaning up, don't forget about "is DMIC in NHLT?"
check. No need to check for channel count or anything DMIC-configuration
related, just straight up verify link_type presence.

Changes in v3:
- no code changes
- appended Mark's Acked-by tag for patch 4/4
- appended Pierre's Reviewed-by tag for all patches

Changes in v2:
- patch "ALSA hda: Drop device-argument in NHLT functions" has been
  dropped
- updated newly added declarations in intel-nhlt.h so warning:
  "no-previous-prototype-for-function" and error:
  "use-of-undeclared-identifier" are no longer observed when
  CONFIG_SND_INTEL_NHLT is not enabled
- added Mark's tag to the last patch of the series

Amadeusz Sławiński (4):
  ALSA: hda: Follow ACPI convention in NHLT struct naming
  ALSA: hda: Fill gaps in NHLT endpoint-interface
  ALSA: hda: Simplify DMIC-in-NHLT check
  ASoC: Intel: Skylake: Use NHLT API to search for blob

 include/sound/intel-nhlt.h             |  53 ++++++++----
 sound/hda/intel-dsp-config.c           |   4 +-
 sound/hda/intel-nhlt.c                 | 110 ++++++++++++++++++++++++-
 sound/soc/intel/skylake/skl-nhlt.c     | 108 +-----------------------
 sound/soc/intel/skylake/skl-pcm.c      |   3 +
 sound/soc/intel/skylake/skl-topology.c |  29 ++++---
 sound/soc/intel/skylake/skl-topology.h |   1 +
 sound/soc/intel/skylake/skl.h          |   6 +-
 sound/soc/sof/intel/hda.c              |   2 +-
 9 files changed, 171 insertions(+), 145 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 1/4] ALSA: hda: Follow ACPI convention in NHLT struct naming
  2021-11-10 10:31 [PATCH v3 0/4] ALSA: hda: New NHLT functions and cleanup Cezary Rojewski
@ 2021-11-10 10:31 ` Cezary Rojewski
  2021-11-15  9:19   ` Takashi Iwai
  2021-11-10 10:31 ` [PATCH v3 2/4] ALSA: hda: Fill gaps in NHLT endpoint-interface Cezary Rojewski
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Cezary Rojewski @ 2021-11-10 10:31 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, Cezary Rojewski, tiwai, hdegoede, broonie,
	Amadeusz Sławiński

From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>

All ACPI table structs except for NHLT ones are defined in postfix way.
Update NHLT structs naming to make code cohesive.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 include/sound/intel-nhlt.h         | 16 ++++++++--------
 sound/hda/intel-dsp-config.c       |  2 +-
 sound/hda/intel-nhlt.c             |  8 ++++----
 sound/soc/intel/skylake/skl-nhlt.c |  8 ++++----
 sound/soc/intel/skylake/skl.h      |  2 +-
 sound/soc/sof/intel/hda.c          |  2 +-
 6 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/include/sound/intel-nhlt.h b/include/sound/intel-nhlt.h
index d0574805865f..b0796225f82e 100644
--- a/include/sound/intel-nhlt.h
+++ b/include/sound/intel-nhlt.h
@@ -77,7 +77,7 @@ struct nhlt_endpoint {
 	struct nhlt_specific_cfg config;
 } __packed;
 
-struct nhlt_acpi_table {
+struct acpi_table_nhlt {
 	struct acpi_table_header header;
 	u8 endpoint_count;
 	struct nhlt_endpoint desc[];
@@ -126,27 +126,27 @@ enum {
 	NHLT_MIC_ARRAY_VENDOR_DEFINED = 0xf,
 };
 
-struct nhlt_acpi_table *intel_nhlt_init(struct device *dev);
+struct acpi_table_nhlt *intel_nhlt_init(struct device *dev);
 
-void intel_nhlt_free(struct nhlt_acpi_table *addr);
+void intel_nhlt_free(struct acpi_table_nhlt *addr);
 
-int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt);
+int intel_nhlt_get_dmic_geo(struct device *dev, struct acpi_table_nhlt *nhlt);
 
 #else
 
-struct nhlt_acpi_table;
+struct acpi_table_nhlt;
 
-static inline struct nhlt_acpi_table *intel_nhlt_init(struct device *dev)
+static inline struct acpi_table_nhlt *intel_nhlt_init(struct device *dev)
 {
 	return NULL;
 }
 
-static inline void intel_nhlt_free(struct nhlt_acpi_table *addr)
+static inline void intel_nhlt_free(struct acpi_table_nhlt *addr)
 {
 }
 
 static inline int intel_nhlt_get_dmic_geo(struct device *dev,
-					  struct nhlt_acpi_table *nhlt)
+					  struct acpi_table_nhlt *nhlt)
 {
 	return 0;
 }
diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c
index b9ac9e9e45a4..64b6c6ab591e 100644
--- a/sound/hda/intel-dsp-config.c
+++ b/sound/hda/intel-dsp-config.c
@@ -379,7 +379,7 @@ static const struct config_entry *snd_intel_dsp_find_config
 
 static int snd_intel_dsp_check_dmic(struct pci_dev *pci)
 {
-	struct nhlt_acpi_table *nhlt;
+	struct acpi_table_nhlt *nhlt;
 	int ret = 0;
 
 	nhlt = intel_nhlt_init(&pci->dev);
diff --git a/sound/hda/intel-nhlt.c b/sound/hda/intel-nhlt.c
index e2237239d922..e6baa7af5c5d 100644
--- a/sound/hda/intel-nhlt.c
+++ b/sound/hda/intel-nhlt.c
@@ -4,9 +4,9 @@
 #include <linux/acpi.h>
 #include <sound/intel-nhlt.h>
 
-struct nhlt_acpi_table *intel_nhlt_init(struct device *dev)
+struct acpi_table_nhlt *intel_nhlt_init(struct device *dev)
 {
-	struct nhlt_acpi_table *nhlt;
+	struct acpi_table_nhlt *nhlt;
 	acpi_status status;
 
 	status = acpi_get_table(ACPI_SIG_NHLT, 0,
@@ -20,13 +20,13 @@ struct nhlt_acpi_table *intel_nhlt_init(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(intel_nhlt_init);
 
-void intel_nhlt_free(struct nhlt_acpi_table *nhlt)
+void intel_nhlt_free(struct acpi_table_nhlt *nhlt)
 {
 	acpi_put_table((struct acpi_table_header *)nhlt);
 }
 EXPORT_SYMBOL_GPL(intel_nhlt_free);
 
-int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt)
+int intel_nhlt_get_dmic_geo(struct device *dev, struct acpi_table_nhlt *nhlt)
 {
 	struct nhlt_endpoint *epnt;
 	struct nhlt_dmic_array_config *cfg;
diff --git a/sound/soc/intel/skylake/skl-nhlt.c b/sound/soc/intel/skylake/skl-nhlt.c
index 64226072f0ee..3033c1bf279f 100644
--- a/sound/soc/intel/skylake/skl-nhlt.c
+++ b/sound/soc/intel/skylake/skl-nhlt.c
@@ -88,7 +88,7 @@ struct nhlt_specific_cfg
 	struct hdac_bus *bus = skl_to_bus(skl);
 	struct device *dev = bus->dev;
 	struct nhlt_specific_cfg *sp_config;
-	struct nhlt_acpi_table *nhlt = skl->nhlt;
+	struct acpi_table_nhlt *nhlt = skl->nhlt;
 	u16 bps = (s_fmt == 16) ? 16 : 32;
 	u8 j;
 
@@ -132,7 +132,7 @@ static void skl_nhlt_trim_space(char *trim)
 
 int skl_nhlt_update_topology_bin(struct skl_dev *skl)
 {
-	struct nhlt_acpi_table *nhlt = (struct nhlt_acpi_table *)skl->nhlt;
+	struct acpi_table_nhlt *nhlt = (struct acpi_table_nhlt *)skl->nhlt;
 	struct hdac_bus *bus = skl_to_bus(skl);
 	struct device *dev = bus->dev;
 
@@ -155,7 +155,7 @@ static ssize_t platform_id_show(struct device *dev,
 	struct pci_dev *pci = to_pci_dev(dev);
 	struct hdac_bus *bus = pci_get_drvdata(pci);
 	struct skl_dev *skl = bus_to_skl(bus);
-	struct nhlt_acpi_table *nhlt = (struct nhlt_acpi_table *)skl->nhlt;
+	struct acpi_table_nhlt *nhlt = (struct acpi_table_nhlt *)skl->nhlt;
 	char platform_id[32];
 
 	sprintf(platform_id, "%x-%.6s-%.8s-%d", skl->pci_id,
@@ -335,7 +335,7 @@ static void skl_get_mclk(struct skl_dev *skl, struct skl_ssp_clk *mclk,
 
 void skl_get_clks(struct skl_dev *skl, struct skl_ssp_clk *ssp_clks)
 {
-	struct nhlt_acpi_table *nhlt = (struct nhlt_acpi_table *)skl->nhlt;
+	struct acpi_table_nhlt *nhlt = (struct acpi_table_nhlt *)skl->nhlt;
 	struct nhlt_endpoint *epnt;
 	struct nhlt_fmt *fmt;
 	int i;
diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h
index 33ed274fc0cb..37195aafbf27 100644
--- a/sound/soc/intel/skylake/skl.h
+++ b/sound/soc/intel/skylake/skl.h
@@ -67,7 +67,7 @@ struct skl_dev {
 	struct snd_soc_component *component;
 	struct snd_soc_dai_driver *dais;
 
-	struct nhlt_acpi_table *nhlt; /* nhlt ptr */
+	struct acpi_table_nhlt *nhlt; /* nhlt ptr */
 
 	struct list_head ppl_list;
 	struct list_head bind_list;
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 568d351b7a4e..ce65bb089a13 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -648,7 +648,7 @@ static int hda_init(struct snd_sof_dev *sdev)
 
 static int check_nhlt_dmic(struct snd_sof_dev *sdev)
 {
-	struct nhlt_acpi_table *nhlt;
+	struct acpi_table_nhlt *nhlt;
 	int dmic_num;
 
 	nhlt = intel_nhlt_init(sdev->dev);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 2/4] ALSA: hda: Fill gaps in NHLT endpoint-interface
  2021-11-10 10:31 [PATCH v3 0/4] ALSA: hda: New NHLT functions and cleanup Cezary Rojewski
  2021-11-10 10:31 ` [PATCH v3 1/4] ALSA: hda: Follow ACPI convention in NHLT struct naming Cezary Rojewski
@ 2021-11-10 10:31 ` Cezary Rojewski
  2021-11-10 10:31 ` [PATCH v3 3/4] ALSA: hda: Simplify DMIC-in-NHLT check Cezary Rojewski
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Cezary Rojewski @ 2021-11-10 10:31 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, Cezary Rojewski, tiwai, hdegoede, broonie,
	Amadeusz Sławiński

From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>

Two key operations missings are: endpoint presence-check and retrieval
of matching endpoint hardware configuration (blob). Add operations for
both use cases.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---

Changes in v2:
- updated newly added declarations in intel-nhlt.h so warning:
  "no-previous-prototype-for-function" and error:
  "use-of-undeclared-identifier" are no longer observed when
  CONFIG_SND_INTEL_NHLT is not enabled

 include/sound/intel-nhlt.h |  37 +++++++++++---
 sound/hda/intel-nhlt.c     | 102 +++++++++++++++++++++++++++++++++++++
 2 files changed, 131 insertions(+), 8 deletions(-)

diff --git a/include/sound/intel-nhlt.h b/include/sound/intel-nhlt.h
index b0796225f82e..7a54d3801608 100644
--- a/include/sound/intel-nhlt.h
+++ b/include/sound/intel-nhlt.h
@@ -10,6 +10,14 @@
 
 #include <linux/acpi.h>
 
+enum nhlt_link_type {
+	NHLT_LINK_HDA = 0,
+	NHLT_LINK_DSP = 1,
+	NHLT_LINK_DMIC = 2,
+	NHLT_LINK_SSP = 3,
+	NHLT_LINK_INVALID
+};
+
 #if IS_ENABLED(CONFIG_ACPI) && IS_ENABLED(CONFIG_SND_INTEL_NHLT)
 
 struct wav_fmt {
@@ -33,14 +41,6 @@ struct wav_fmt_ext {
 	u8 sub_fmt[16];
 } __packed;
 
-enum nhlt_link_type {
-	NHLT_LINK_HDA = 0,
-	NHLT_LINK_DSP = 1,
-	NHLT_LINK_DMIC = 2,
-	NHLT_LINK_SSP = 3,
-	NHLT_LINK_INVALID
-};
-
 enum nhlt_device_type {
 	NHLT_DEVICE_BT = 0,
 	NHLT_DEVICE_DMIC = 1,
@@ -132,6 +132,12 @@ void intel_nhlt_free(struct acpi_table_nhlt *addr);
 
 int intel_nhlt_get_dmic_geo(struct device *dev, struct acpi_table_nhlt *nhlt);
 
+bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type);
+struct nhlt_specific_cfg *
+intel_nhlt_get_endpoint_blob(struct device *dev, struct acpi_table_nhlt *nhlt,
+			     u32 bus_id, u8 link_type, u8 vbps, u8 bps,
+			     u8 num_ch, u32 rate, u8 dir, u8 dev_type);
+
 #else
 
 struct acpi_table_nhlt;
@@ -150,6 +156,21 @@ static inline int intel_nhlt_get_dmic_geo(struct device *dev,
 {
 	return 0;
 }
+
+static inline bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt,
+						u8 link_type)
+{
+	return false;
+}
+
+static inline struct nhlt_specific_cfg *
+intel_nhlt_get_endpoint_blob(struct device *dev, struct acpi_table_nhlt *nhlt,
+			     u32 bus_id, u8 link_type, u8 vbps, u8 bps,
+			     u8 num_ch, u32 rate, u8 dir, u8 dev_type)
+{
+	return NULL;
+}
+
 #endif
 
 #endif
diff --git a/sound/hda/intel-nhlt.c b/sound/hda/intel-nhlt.c
index e6baa7af5c5d..2108be213d4f 100644
--- a/sound/hda/intel-nhlt.c
+++ b/sound/hda/intel-nhlt.c
@@ -110,3 +110,105 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct acpi_table_nhlt *nhlt)
 	return dmic_geo;
 }
 EXPORT_SYMBOL_GPL(intel_nhlt_get_dmic_geo);
+
+bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type)
+{
+	struct nhlt_endpoint *epnt;
+	int i;
+
+	if (!nhlt)
+		return false;
+
+	epnt = (struct nhlt_endpoint *)nhlt->desc;
+	for (i = 0; i < nhlt->endpoint_count; i++) {
+		if (epnt->linktype == link_type)
+			return true;
+
+		epnt = (struct nhlt_endpoint *)((u8 *)epnt + epnt->length);
+	}
+	return false;
+}
+EXPORT_SYMBOL(intel_nhlt_has_endpoint_type);
+
+static struct nhlt_specific_cfg *
+nhlt_get_specific_cfg(struct device *dev, struct nhlt_fmt *fmt, u8 num_ch,
+		      u32 rate, u8 vbps, u8 bps)
+{
+	struct nhlt_fmt_cfg *cfg = fmt->fmt_config;
+	struct wav_fmt *wfmt;
+	u16 _bps, _vbps;
+	int i;
+
+	dev_dbg(dev, "Endpoint format count=%d\n", fmt->fmt_count);
+
+	for (i = 0; i < fmt->fmt_count; i++) {
+		wfmt = &cfg->fmt_ext.fmt;
+		_bps = wfmt->bits_per_sample;
+		_vbps = cfg->fmt_ext.sample.valid_bits_per_sample;
+
+		dev_dbg(dev, "Endpoint format: ch=%d fmt=%d/%d rate=%d\n",
+			wfmt->channels, _vbps, _bps, wfmt->samples_per_sec);
+
+		if (wfmt->channels == num_ch && wfmt->samples_per_sec == rate &&
+		    vbps == _vbps && bps == _bps)
+			return &cfg->config;
+
+		cfg = (struct nhlt_fmt_cfg *)(cfg->config.caps + cfg->config.size);
+	}
+
+	return NULL;
+}
+
+static bool nhlt_check_ep_match(struct device *dev, struct nhlt_endpoint *epnt,
+				u32 bus_id, u8 link_type, u8 dir, u8 dev_type)
+{
+	dev_dbg(dev, "Endpoint: vbus_id=%d link_type=%d dir=%d dev_type = %d\n",
+		epnt->virtual_bus_id, epnt->linktype,
+		epnt->direction, epnt->device_type);
+
+	if ((epnt->virtual_bus_id != bus_id) ||
+	    (epnt->linktype != link_type) ||
+	    (epnt->direction != dir))
+		return false;
+
+	/* link of type DMIC bypasses device_type check */
+	return epnt->linktype == NHLT_LINK_DMIC ||
+	       epnt->device_type == dev_type;
+}
+
+struct nhlt_specific_cfg *
+intel_nhlt_get_endpoint_blob(struct device *dev, struct acpi_table_nhlt *nhlt,
+			     u32 bus_id, u8 link_type, u8 vbps, u8 bps,
+			     u8 num_ch, u32 rate, u8 dir, u8 dev_type)
+{
+	struct nhlt_specific_cfg *cfg;
+	struct nhlt_endpoint *epnt;
+	struct nhlt_fmt *fmt;
+	int i;
+
+	if (!nhlt)
+		return NULL;
+
+	dev_dbg(dev, "Looking for configuration:\n");
+	dev_dbg(dev, "  vbus_id=%d link_type=%d dir=%d, dev_type=%d\n",
+		bus_id, link_type, dir, dev_type);
+	dev_dbg(dev, "  ch=%d fmt=%d/%d rate=%d\n", num_ch, vbps, bps, rate);
+	dev_dbg(dev, "Endpoint count=%d\n", nhlt->endpoint_count);
+
+	epnt = (struct nhlt_endpoint *)nhlt->desc;
+
+	for (i = 0; i < nhlt->endpoint_count; i++) {
+		if (nhlt_check_ep_match(dev, epnt, bus_id, link_type, dir, dev_type)) {
+			fmt = (struct nhlt_fmt *)(epnt->config.caps + epnt->config.size);
+
+			cfg = nhlt_get_specific_cfg(dev, fmt, num_ch, rate, vbps, bps);
+			if (cfg)
+				return cfg;
+		}
+
+		epnt = (struct nhlt_endpoint *)((u8 *)epnt + epnt->length);
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL(intel_nhlt_get_endpoint_blob);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 3/4] ALSA: hda: Simplify DMIC-in-NHLT check
  2021-11-10 10:31 [PATCH v3 0/4] ALSA: hda: New NHLT functions and cleanup Cezary Rojewski
  2021-11-10 10:31 ` [PATCH v3 1/4] ALSA: hda: Follow ACPI convention in NHLT struct naming Cezary Rojewski
  2021-11-10 10:31 ` [PATCH v3 2/4] ALSA: hda: Fill gaps in NHLT endpoint-interface Cezary Rojewski
@ 2021-11-10 10:31 ` Cezary Rojewski
  2021-11-10 10:31 ` [PATCH v3 4/4] ASoC: Intel: Skylake: Use NHLT API to search for blob Cezary Rojewski
  2021-11-14  8:35 ` [PATCH v3 0/4] ALSA: hda: New NHLT functions and cleanup Takashi Iwai
  4 siblings, 0 replies; 9+ messages in thread
From: Cezary Rojewski @ 2021-11-10 10:31 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, Cezary Rojewski, tiwai, hdegoede, broonie,
	Amadeusz Sławiński

From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>

Only DMIC endpoint presence is relevant, not its configuration.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/hda/intel-dsp-config.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c
index 64b6c6ab591e..d97152a3a331 100644
--- a/sound/hda/intel-dsp-config.c
+++ b/sound/hda/intel-dsp-config.c
@@ -384,7 +384,7 @@ static int snd_intel_dsp_check_dmic(struct pci_dev *pci)
 
 	nhlt = intel_nhlt_init(&pci->dev);
 	if (nhlt) {
-		if (intel_nhlt_get_dmic_geo(&pci->dev, nhlt))
+		if (intel_nhlt_has_endpoint_type(nhlt, NHLT_LINK_DMIC))
 			ret = 1;
 		intel_nhlt_free(nhlt);
 	}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 4/4] ASoC: Intel: Skylake: Use NHLT API to search for blob
  2021-11-10 10:31 [PATCH v3 0/4] ALSA: hda: New NHLT functions and cleanup Cezary Rojewski
                   ` (2 preceding siblings ...)
  2021-11-10 10:31 ` [PATCH v3 3/4] ALSA: hda: Simplify DMIC-in-NHLT check Cezary Rojewski
@ 2021-11-10 10:31 ` Cezary Rojewski
  2021-11-10 14:18   ` Mark Brown
  2021-11-14  8:35 ` [PATCH v3 0/4] ALSA: hda: New NHLT functions and cleanup Takashi Iwai
  4 siblings, 1 reply; 9+ messages in thread
From: Cezary Rojewski @ 2021-11-10 10:31 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, Cezary Rojewski, tiwai, hdegoede, broonie,
	Amadeusz Sławiński

From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>

With NHLT enriched with new search functions, remove local code in
favour of them. This also fixes broken behaviour: search should be based
on significant bits count rather than container size.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Acked-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/skylake/skl-nhlt.c     | 102 -------------------------
 sound/soc/intel/skylake/skl-pcm.c      |   3 +
 sound/soc/intel/skylake/skl-topology.c |  29 ++++---
 sound/soc/intel/skylake/skl-topology.h |   1 +
 sound/soc/intel/skylake/skl.h          |   4 -
 5 files changed, 21 insertions(+), 118 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-nhlt.c b/sound/soc/intel/skylake/skl-nhlt.c
index 3033c1bf279f..16f70554b2a0 100644
--- a/sound/soc/intel/skylake/skl-nhlt.c
+++ b/sound/soc/intel/skylake/skl-nhlt.c
@@ -13,108 +13,6 @@
 #include "skl.h"
 #include "skl-i2s.h"
 
-static struct nhlt_specific_cfg *skl_get_specific_cfg(
-		struct device *dev, struct nhlt_fmt *fmt,
-		u8 no_ch, u32 rate, u16 bps, u8 linktype)
-{
-	struct nhlt_specific_cfg *sp_config;
-	struct wav_fmt *wfmt;
-	struct nhlt_fmt_cfg *fmt_config = fmt->fmt_config;
-	int i;
-
-	dev_dbg(dev, "Format count =%d\n", fmt->fmt_count);
-
-	for (i = 0; i < fmt->fmt_count; i++) {
-		wfmt = &fmt_config->fmt_ext.fmt;
-		dev_dbg(dev, "ch=%d fmt=%d s_rate=%d\n", wfmt->channels,
-			 wfmt->bits_per_sample, wfmt->samples_per_sec);
-		if (wfmt->channels == no_ch && wfmt->bits_per_sample == bps) {
-			/*
-			 * if link type is dmic ignore rate check as the blob is
-			 * generic for all rates
-			 */
-			sp_config = &fmt_config->config;
-			if (linktype == NHLT_LINK_DMIC)
-				return sp_config;
-
-			if (wfmt->samples_per_sec == rate)
-				return sp_config;
-		}
-
-		fmt_config = (struct nhlt_fmt_cfg *)(fmt_config->config.caps +
-						fmt_config->config.size);
-	}
-
-	return NULL;
-}
-
-static void dump_config(struct device *dev, u32 instance_id, u8 linktype,
-		u8 s_fmt, u8 num_channels, u32 s_rate, u8 dirn, u16 bps)
-{
-	dev_dbg(dev, "Input configuration\n");
-	dev_dbg(dev, "ch=%d fmt=%d s_rate=%d\n", num_channels, s_fmt, s_rate);
-	dev_dbg(dev, "vbus_id=%d link_type=%d\n", instance_id, linktype);
-	dev_dbg(dev, "bits_per_sample=%d\n", bps);
-}
-
-static bool skl_check_ep_match(struct device *dev, struct nhlt_endpoint *epnt,
-		u32 instance_id, u8 link_type, u8 dirn, u8 dev_type)
-{
-	dev_dbg(dev, "vbus_id=%d link_type=%d dir=%d dev_type = %d\n",
-			epnt->virtual_bus_id, epnt->linktype,
-			epnt->direction, epnt->device_type);
-
-	if ((epnt->virtual_bus_id == instance_id) &&
-			(epnt->linktype == link_type) &&
-			(epnt->direction == dirn)) {
-		/* do not check dev_type for DMIC link type */
-		if (epnt->linktype == NHLT_LINK_DMIC)
-			return true;
-
-		if (epnt->device_type == dev_type)
-			return true;
-	}
-
-	return false;
-}
-
-struct nhlt_specific_cfg
-*skl_get_ep_blob(struct skl_dev *skl, u32 instance, u8 link_type,
-			u8 s_fmt, u8 num_ch, u32 s_rate,
-			u8 dirn, u8 dev_type)
-{
-	struct nhlt_fmt *fmt;
-	struct nhlt_endpoint *epnt;
-	struct hdac_bus *bus = skl_to_bus(skl);
-	struct device *dev = bus->dev;
-	struct nhlt_specific_cfg *sp_config;
-	struct acpi_table_nhlt *nhlt = skl->nhlt;
-	u16 bps = (s_fmt == 16) ? 16 : 32;
-	u8 j;
-
-	dump_config(dev, instance, link_type, s_fmt, num_ch, s_rate, dirn, bps);
-
-	epnt = (struct nhlt_endpoint *)nhlt->desc;
-
-	dev_dbg(dev, "endpoint count =%d\n", nhlt->endpoint_count);
-
-	for (j = 0; j < nhlt->endpoint_count; j++) {
-		if (skl_check_ep_match(dev, epnt, instance, link_type,
-						dirn, dev_type)) {
-			fmt = (struct nhlt_fmt *)(epnt->config.caps +
-						 epnt->config.size);
-			sp_config = skl_get_specific_cfg(dev, fmt, num_ch,
-							s_rate, bps, link_type);
-			if (sp_config)
-				return sp_config;
-		}
-
-		epnt = (struct nhlt_endpoint *)((u8 *)epnt + epnt->length);
-	}
-
-	return NULL;
-}
-
 static void skl_nhlt_trim_space(char *trim)
 {
 	char *s = trim;
diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index 9ecaf6a1e847..34908af50046 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -317,6 +317,7 @@ static int skl_pcm_hw_params(struct snd_pcm_substream *substream,
 	dev_dbg(dai->dev, "dma_id=%d\n", dma_id);
 
 	p_params.s_fmt = snd_pcm_format_width(params_format(params));
+	p_params.s_cont = snd_pcm_format_physical_width(params_format(params));
 	p_params.ch = params_channels(params);
 	p_params.s_freq = params_rate(params);
 	p_params.host_dma_id = dma_id;
@@ -405,6 +406,7 @@ static int skl_be_hw_params(struct snd_pcm_substream *substream,
 	struct skl_pipe_params p_params = {0};
 
 	p_params.s_fmt = snd_pcm_format_width(params_format(params));
+	p_params.s_cont = snd_pcm_format_physical_width(params_format(params));
 	p_params.ch = params_channels(params);
 	p_params.s_freq = params_rate(params);
 	p_params.stream = substream->stream;
@@ -569,6 +571,7 @@ static int skl_link_hw_params(struct snd_pcm_substream *substream,
 		snd_soc_dai_set_tdm_slot(codec_dai, 0, stream_tag, 0, 0);
 
 	p_params.s_fmt = snd_pcm_format_width(params_format(params));
+	p_params.s_cont = snd_pcm_format_physical_width(params_format(params));
 	p_params.ch = params_channels(params);
 	p_params.s_freq = params_rate(params);
 	p_params.stream = substream->stream;
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index 89e4231304dd..9bdf020a2b64 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -285,7 +285,7 @@ static int skl_tplg_update_be_blob(struct snd_soc_dapm_widget *w,
 {
 	struct skl_module_cfg *m_cfg = w->priv;
 	int link_type, dir;
-	u32 ch, s_freq, s_fmt;
+	u32 ch, s_freq, s_fmt, s_cont;
 	struct nhlt_specific_cfg *cfg;
 	u8 dev_type = skl_tplg_be_dev_type(m_cfg->dev_type);
 	int fmt_idx = m_cfg->fmt_idx;
@@ -301,7 +301,8 @@ static int skl_tplg_update_be_blob(struct snd_soc_dapm_widget *w,
 		link_type = NHLT_LINK_DMIC;
 		dir = SNDRV_PCM_STREAM_CAPTURE;
 		s_freq = m_iface->inputs[0].fmt.s_freq;
-		s_fmt = m_iface->inputs[0].fmt.bit_depth;
+		s_fmt = m_iface->inputs[0].fmt.valid_bit_depth;
+		s_cont = m_iface->inputs[0].fmt.bit_depth;
 		ch = m_iface->inputs[0].fmt.channels;
 		break;
 
@@ -310,12 +311,14 @@ static int skl_tplg_update_be_blob(struct snd_soc_dapm_widget *w,
 		if (m_cfg->hw_conn_type == SKL_CONN_SOURCE) {
 			dir = SNDRV_PCM_STREAM_PLAYBACK;
 			s_freq = m_iface->outputs[0].fmt.s_freq;
-			s_fmt = m_iface->outputs[0].fmt.bit_depth;
+			s_fmt = m_iface->outputs[0].fmt.valid_bit_depth;
+			s_cont = m_iface->outputs[0].fmt.bit_depth;
 			ch = m_iface->outputs[0].fmt.channels;
 		} else {
 			dir = SNDRV_PCM_STREAM_CAPTURE;
 			s_freq = m_iface->inputs[0].fmt.s_freq;
-			s_fmt = m_iface->inputs[0].fmt.bit_depth;
+			s_fmt = m_iface->inputs[0].fmt.valid_bit_depth;
+			s_cont = m_iface->inputs[0].fmt.bit_depth;
 			ch = m_iface->inputs[0].fmt.channels;
 		}
 		break;
@@ -325,16 +328,17 @@ static int skl_tplg_update_be_blob(struct snd_soc_dapm_widget *w,
 	}
 
 	/* update the blob based on virtual bus_id and default params */
-	cfg = skl_get_ep_blob(skl, m_cfg->vbus_id, link_type,
-					s_fmt, ch, s_freq, dir, dev_type);
+	cfg = intel_nhlt_get_endpoint_blob(skl->dev, skl->nhlt, m_cfg->vbus_id,
+					   link_type, s_fmt, s_cont, ch,
+					   s_freq, dir, dev_type);
 	if (cfg) {
 		m_cfg->formats_config[SKL_PARAM_INIT].caps_size = cfg->size;
 		m_cfg->formats_config[SKL_PARAM_INIT].caps = (u32 *)&cfg->caps;
 	} else {
 		dev_err(skl->dev, "Blob NULL for id %x type %d dirn %d\n",
 					m_cfg->vbus_id, link_type, dir);
-		dev_err(skl->dev, "PCM: ch %d, freq %d, fmt %d\n",
-					ch, s_freq, s_fmt);
+		dev_err(skl->dev, "PCM: ch %d, freq %d, fmt %d/%d\n",
+					ch, s_freq, s_fmt, s_cont);
 		return -EIO;
 	}
 
@@ -1849,10 +1853,11 @@ static int skl_tplg_be_fill_pipe_params(struct snd_soc_dai *dai,
 		pipe_fmt = &pipe->configs[pipe->pipe_config_idx].in_fmt;
 
 	/* update the blob based on virtual bus_id*/
-	cfg = skl_get_ep_blob(skl, mconfig->vbus_id, link_type,
-					pipe_fmt->bps, pipe_fmt->channels,
-					pipe_fmt->freq, pipe->direction,
-					dev_type);
+	cfg = intel_nhlt_get_endpoint_blob(dai->dev, skl->nhlt,
+					mconfig->vbus_id, link_type,
+					pipe_fmt->bps, params->s_cont,
+					pipe_fmt->channels, pipe_fmt->freq,
+					pipe->direction, dev_type);
 	if (cfg) {
 		mconfig->formats_config[SKL_PARAM_INIT].caps_size = cfg->size;
 		mconfig->formats_config[SKL_PARAM_INIT].caps = (u32 *)&cfg->caps;
diff --git a/sound/soc/intel/skylake/skl-topology.h b/sound/soc/intel/skylake/skl-topology.h
index f0695b2ac5dd..22963634fbea 100644
--- a/sound/soc/intel/skylake/skl-topology.h
+++ b/sound/soc/intel/skylake/skl-topology.h
@@ -284,6 +284,7 @@ struct skl_pipe_params {
 	u32 ch;
 	u32 s_freq;
 	u32 s_fmt;
+	u32 s_cont;
 	u8 linktype;
 	snd_pcm_format_t format;
 	int link_index;
diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h
index 37195aafbf27..3b92dfc23731 100644
--- a/sound/soc/intel/skylake/skl.h
+++ b/sound/soc/intel/skylake/skl.h
@@ -165,10 +165,6 @@ struct skl_dsp_ops {
 int skl_platform_unregister(struct device *dev);
 int skl_platform_register(struct device *dev);
 
-struct nhlt_specific_cfg *skl_get_ep_blob(struct skl_dev *skl, u32 instance,
-					u8 link_type, u8 s_fmt, u8 num_ch,
-					u32 s_rate, u8 dirn, u8 dev_type);
-
 int skl_nhlt_update_topology_bin(struct skl_dev *skl);
 int skl_init_dsp(struct skl_dev *skl);
 int skl_free_dsp(struct skl_dev *skl);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 4/4] ASoC: Intel: Skylake: Use NHLT API to search for blob
  2021-11-10 10:31 ` [PATCH v3 4/4] ASoC: Intel: Skylake: Use NHLT API to search for blob Cezary Rojewski
@ 2021-11-10 14:18   ` Mark Brown
  0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2021-11-10 14:18 UTC (permalink / raw)
  To: Cezary Rojewski
  Cc: pierre-louis.bossart, alsa-devel, tiwai, hdegoede,
	Amadeusz Sławiński

[-- Attachment #1: Type: text/plain, Size: 386 bytes --]

On Wed, Nov 10, 2021 at 11:31:17AM +0100, Cezary Rojewski wrote:
> From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> 
> With NHLT enriched with new search functions, remove local code in
> favour of them. This also fixes broken behaviour: search should be based
> on significant bits count rather than container size.

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 0/4] ALSA: hda: New NHLT functions and cleanup
  2021-11-10 10:31 [PATCH v3 0/4] ALSA: hda: New NHLT functions and cleanup Cezary Rojewski
                   ` (3 preceding siblings ...)
  2021-11-10 10:31 ` [PATCH v3 4/4] ASoC: Intel: Skylake: Use NHLT API to search for blob Cezary Rojewski
@ 2021-11-14  8:35 ` Takashi Iwai
  4 siblings, 0 replies; 9+ messages in thread
From: Takashi Iwai @ 2021-11-14  8:35 UTC (permalink / raw)
  To: Cezary Rojewski
  Cc: pierre-louis.bossart, alsa-devel, tiwai, hdegoede, broonie

On Wed, 10 Nov 2021 11:31:13 +0100,
Cezary Rojewski wrote:
> 
> Changes add two crucial functions: endpoint presence-check and
> retrieval of endpoint's BLOB (hardware configuration) to NHLT API.
> 
> Few cleanups accompany the above:
> Work is done to align NHLT-struct naming with other, commonly used
> ACPI-structs. While cleaning up, don't forget about "is DMIC in NHLT?"
> check. No need to check for channel count or anything DMIC-configuration
> related, just straight up verify link_type presence.
> 
> Changes in v3:
> - no code changes
> - appended Mark's Acked-by tag for patch 4/4
> - appended Pierre's Reviewed-by tag for all patches
> 
> Changes in v2:
> - patch "ALSA hda: Drop device-argument in NHLT functions" has been
>   dropped
> - updated newly added declarations in intel-nhlt.h so warning:
>   "no-previous-prototype-for-function" and error:
>   "use-of-undeclared-identifier" are no longer observed when
>   CONFIG_SND_INTEL_NHLT is not enabled
> - added Mark's tag to the last patch of the series
> 
> Amadeusz Sławiński (4):
>   ALSA: hda: Follow ACPI convention in NHLT struct naming
>   ALSA: hda: Fill gaps in NHLT endpoint-interface
>   ALSA: hda: Simplify DMIC-in-NHLT check
>   ASoC: Intel: Skylake: Use NHLT API to search for blob

Merged to topic/for-5.16 branch now.

thanks,


Takashi

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 1/4] ALSA: hda: Follow ACPI convention in NHLT struct naming
  2021-11-10 10:31 ` [PATCH v3 1/4] ALSA: hda: Follow ACPI convention in NHLT struct naming Cezary Rojewski
@ 2021-11-15  9:19   ` Takashi Iwai
  2021-11-26 13:48     ` Cezary Rojewski
  0 siblings, 1 reply; 9+ messages in thread
From: Takashi Iwai @ 2021-11-15  9:19 UTC (permalink / raw)
  To: Cezary Rojewski
  Cc: pierre-louis.bossart, alsa-devel, tiwai, hdegoede, broonie,
	Amadeusz Sławiński

On Wed, 10 Nov 2021 11:31:14 +0100,
Cezary Rojewski wrote:
> 
> From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> 
> All ACPI table structs except for NHLT ones are defined in postfix way.
> Update NHLT structs naming to make code cohesive.
> 
> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

This turned out to be conflicting with the definition in
include/acpi/actbl2.h, which was newly introduced in 5.16-rc1.

So unfortunately I'll have to drop the patches for now.
Could you rework with 5.16-rc base and resubmit?


thanks,

Takashi

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 1/4] ALSA: hda: Follow ACPI convention in NHLT struct naming
  2021-11-15  9:19   ` Takashi Iwai
@ 2021-11-26 13:48     ` Cezary Rojewski
  0 siblings, 0 replies; 9+ messages in thread
From: Cezary Rojewski @ 2021-11-26 13:48 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: pierre-louis.bossart, alsa-devel, tiwai, hdegoede, broonie,
	Amadeusz Sławiński

On 2021-11-15 10:19 AM, Takashi Iwai wrote:
> On Wed, 10 Nov 2021 11:31:14 +0100,
> Cezary Rojewski wrote:
>>
>> From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
>>
>> All ACPI table structs except for NHLT ones are defined in postfix way.
>> Update NHLT structs naming to make code cohesive.
>>
>> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
>> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
>> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> 
> This turned out to be conflicting with the definition in
> include/acpi/actbl2.h, which was newly introduced in 5.16-rc1.
> 
> So unfortunately I'll have to drop the patches for now.
> Could you rework with 5.16-rc base and resubmit?

Hello,

Sorry for the delay in replying. I've addressed the naming conflicts in 
v4 - this patch has been dropped and all the rest updated. Additional 
work will be done in separate series to align sound/hda/ with recently 
added NHLT-structs in ACPI tree.


Regards,
Czarek

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-11-26 13:49 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-10 10:31 [PATCH v3 0/4] ALSA: hda: New NHLT functions and cleanup Cezary Rojewski
2021-11-10 10:31 ` [PATCH v3 1/4] ALSA: hda: Follow ACPI convention in NHLT struct naming Cezary Rojewski
2021-11-15  9:19   ` Takashi Iwai
2021-11-26 13:48     ` Cezary Rojewski
2021-11-10 10:31 ` [PATCH v3 2/4] ALSA: hda: Fill gaps in NHLT endpoint-interface Cezary Rojewski
2021-11-10 10:31 ` [PATCH v3 3/4] ALSA: hda: Simplify DMIC-in-NHLT check Cezary Rojewski
2021-11-10 10:31 ` [PATCH v3 4/4] ASoC: Intel: Skylake: Use NHLT API to search for blob Cezary Rojewski
2021-11-10 14:18   ` Mark Brown
2021-11-14  8:35 ` [PATCH v3 0/4] ALSA: hda: New NHLT functions and cleanup Takashi Iwai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.