[PATCH v3 0/2] ASoC topology header parsing refinement

[PATCH v3 0/2] ASoC topology header parsing refinement

[Keyon Jie]

This small series is to optimize the header logging during the topology
parsing. This is verified work fine on both SOF and SST drivers.

Change History:
v3:
- Remove using the separated soc_pass_load() function and merge it to the
  soc_tplg_load_header() body.
- Add more Tested-by tags.

v2:
- Change the internal used array to be 'static' to fix the issue
  reported by: kbuild test robot <lkp@intel.com>
- Add testing coverage including Intel SST driver also.

v1:
- Initial version.

Keyon Jie (2):
  ASoC: topology: refine and log the header in the correct pass
  ASoC: topology: remove the redundant pass checks

 sound/soc/soc-topology.c | 96 ++++++++++++++++------------------------
 1 file changed, 38 insertions(+), 58 deletions(-)

-- 
2.25.1

[PATCH v3 1/2] ASoC: topology: refine and log the header in the correct pass

[Keyon Jie]

The check (tplg->pass == le32_to_cpu(hdr->type)) makes no sense as it is
comparing two different enums, refine the element loading functions, and
log the information when the header is being parsed in the corresponding
parsing pass.

Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Tested-by: Vamshi Kerishna Gopal <vamshi.krishna.gopal@intel.com>
Tested-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/soc-topology.c | 50 +++++++++++++++++++++++++++++-----------
 1 file changed, 36 insertions(+), 14 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 49875978a1ce..f0a1a47d9808 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -2685,12 +2685,6 @@ static int soc_valid_header(struct soc_tplg *tplg,
 		return -EINVAL;
 	}
 
-	if (tplg->pass == le32_to_cpu(hdr->type))
-		dev_dbg(tplg->dev,
-			"ASoC: Got 0x%x bytes of type %d version %d vendor %d at pass %d\n",
-			hdr->payload_size, hdr->type, hdr->version,
-			hdr->vendor_type, tplg->pass);
-
 	return 1;
 }
 
@@ -2698,6 +2692,10 @@ static int soc_valid_header(struct soc_tplg *tplg,
 static int soc_tplg_load_header(struct soc_tplg *tplg,
 	struct snd_soc_tplg_hdr *hdr)
 {
+	int (*elem_load)(struct soc_tplg *tplg,
+			 struct snd_soc_tplg_hdr *hdr);
+	unsigned int hdr_pass;
+
 	tplg->pos = tplg->hdr_pos + sizeof(struct snd_soc_tplg_hdr);
 
 	/* check for matching ID */
@@ -2711,24 +2709,48 @@ static int soc_tplg_load_header(struct soc_tplg *tplg,
 	case SND_SOC_TPLG_TYPE_MIXER:
 	case SND_SOC_TPLG_TYPE_ENUM:
 	case SND_SOC_TPLG_TYPE_BYTES:
-		return soc_tplg_kcontrol_elems_load(tplg, hdr);
+		hdr_pass = SOC_TPLG_PASS_MIXER;
+		elem_load = soc_tplg_kcontrol_elems_load;
+		break;
 	case SND_SOC_TPLG_TYPE_DAPM_GRAPH:
-		return soc_tplg_dapm_graph_elems_load(tplg, hdr);
+		hdr_pass = SOC_TPLG_PASS_GRAPH;
+		elem_load = soc_tplg_dapm_graph_elems_load;
+		break;
 	case SND_SOC_TPLG_TYPE_DAPM_WIDGET:
-		return soc_tplg_dapm_widget_elems_load(tplg, hdr);
+		hdr_pass = SOC_TPLG_PASS_WIDGET;
+		elem_load = soc_tplg_dapm_widget_elems_load;
+		break;
 	case SND_SOC_TPLG_TYPE_PCM:
-		return soc_tplg_pcm_elems_load(tplg, hdr);
+		hdr_pass = SOC_TPLG_PASS_PCM_DAI;
+		elem_load = soc_tplg_pcm_elems_load;
+		break;
 	case SND_SOC_TPLG_TYPE_DAI:
-		return soc_tplg_dai_elems_load(tplg, hdr);
+		hdr_pass = SOC_TPLG_PASS_BE_DAI;
+		elem_load = soc_tplg_dai_elems_load;
+		break;
 	case SND_SOC_TPLG_TYPE_DAI_LINK:
 	case SND_SOC_TPLG_TYPE_BACKEND_LINK:
 		/* physical link configurations */
-		return soc_tplg_link_elems_load(tplg, hdr);
+		hdr_pass = SOC_TPLG_PASS_LINK;
+		elem_load = soc_tplg_link_elems_load;
+		break;
 	case SND_SOC_TPLG_TYPE_MANIFEST:
-		return soc_tplg_manifest_load(tplg, hdr);
+		hdr_pass = SOC_TPLG_PASS_MANIFEST;
+		elem_load = soc_tplg_manifest_load;
+		break;
 	default:
 		/* bespoke vendor data object */
-		return soc_tplg_vendor_load(tplg, hdr);
+		hdr_pass = SOC_TPLG_PASS_VENDOR;
+		elem_load = soc_tplg_vendor_load;
+		break;
+	}
+
+	if (tplg->pass == hdr_pass) {
+		dev_dbg(tplg->dev,
+			"ASoC: Got 0x%x bytes of type %d version %d vendor %d at pass %d\n",
+			hdr->payload_size, hdr->type, hdr->version,
+			hdr->vendor_type, tplg->pass);
+		return elem_load(tplg, hdr);
 	}
 
 	return 0;
-- 
2.25.1

[PATCH v3 2/2] ASoC: topology: remove the redundant pass checks

[Keyon Jie]

As we have check the 'pass' in the soc_elem_pass_load(), so no need to
check it again in each specific elem_load function, at the same time,
the tplg->pos will be reset to the next header base when the pass is
mismatched, so the increasing of the tplg->pos in these cases made no
sense. Here remove all of them.

Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Tested-by: Vamshi Kerishna Gopal <vamshi.krishna.gopal@intel.com>
Tested-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/soc-topology.c | 46 ++--------------------------------------
 1 file changed, 2 insertions(+), 44 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index f0a1a47d9808..9e89633676b7 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -246,8 +246,8 @@ static inline void soc_control_err(struct soc_tplg *tplg,
 }
 
 /* pass vendor data to component driver for processing */
-static int soc_tplg_vendor_load_(struct soc_tplg *tplg,
-	struct snd_soc_tplg_hdr *hdr)
+static int soc_tplg_vendor_load(struct soc_tplg *tplg,
+				struct snd_soc_tplg_hdr *hdr)
 {
 	int ret = 0;
 
@@ -268,16 +268,6 @@ static int soc_tplg_vendor_load_(struct soc_tplg *tplg,
 	return ret;
 }
 
-/* pass vendor data to component driver for processing */
-static int soc_tplg_vendor_load(struct soc_tplg *tplg,
-	struct snd_soc_tplg_hdr *hdr)
-{
-	if (tplg->pass != SOC_TPLG_PASS_VENDOR)
-		return 0;
-
-	return soc_tplg_vendor_load_(tplg, hdr);
-}
-
 /* optionally pass new dynamic widget to component driver. This is mainly for
  * external widgets where we can assign private data/ops */
 static int soc_tplg_widget_load(struct soc_tplg *tplg,
@@ -1127,12 +1117,6 @@ static int soc_tplg_kcontrol_elems_load(struct soc_tplg *tplg,
 	int ret;
 	int i;
 
-	if (tplg->pass != SOC_TPLG_PASS_MIXER) {
-		tplg->pos += le32_to_cpu(hdr->size) +
-			le32_to_cpu(hdr->payload_size);
-		return 0;
-	}
-
 	dev_dbg(tplg->dev, "ASoC: adding %d kcontrols at 0x%lx\n", hdr->count,
 		soc_tplg_get_offset(tplg));
 
@@ -1204,14 +1188,6 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg,
 
 	count = le32_to_cpu(hdr->count);
 
-	if (tplg->pass != SOC_TPLG_PASS_GRAPH) {
-		tplg->pos +=
-			le32_to_cpu(hdr->size) +
-			le32_to_cpu(hdr->payload_size);
-
-		return 0;
-	}
-
 	if (soc_tplg_check_elem_count(tplg,
 		sizeof(struct snd_soc_tplg_dapm_graph_elem),
 		count, le32_to_cpu(hdr->payload_size), "graph")) {
@@ -1741,9 +1717,6 @@ static int soc_tplg_dapm_widget_elems_load(struct soc_tplg *tplg,
 
 	count = le32_to_cpu(hdr->count);
 
-	if (tplg->pass != SOC_TPLG_PASS_WIDGET)
-		return 0;
-
 	dev_dbg(tplg->dev, "ASoC: adding %d DAPM widgets\n", count);
 
 	for (i = 0; i < count; i++) {
@@ -2101,9 +2074,6 @@ static int soc_tplg_pcm_elems_load(struct soc_tplg *tplg,
 
 	count = le32_to_cpu(hdr->count);
 
-	if (tplg->pass != SOC_TPLG_PASS_PCM_DAI)
-		return 0;
-
 	/* check the element size and count */
 	pcm = (struct snd_soc_tplg_pcm *)tplg->pos;
 	size = le32_to_cpu(pcm->size);
@@ -2386,12 +2356,6 @@ static int soc_tplg_link_elems_load(struct soc_tplg *tplg,
 
 	count = le32_to_cpu(hdr->count);
 
-	if (tplg->pass != SOC_TPLG_PASS_LINK) {
-		tplg->pos += le32_to_cpu(hdr->size) +
-			le32_to_cpu(hdr->payload_size);
-		return 0;
-	}
-
 	/* check the element size and count */
 	link = (struct snd_soc_tplg_link_config *)tplg->pos;
 	size = le32_to_cpu(link->size);
@@ -2528,9 +2492,6 @@ static int soc_tplg_dai_elems_load(struct soc_tplg *tplg,
 
 	count = le32_to_cpu(hdr->count);
 
-	if (tplg->pass != SOC_TPLG_PASS_BE_DAI)
-		return 0;
-
 	/* config the existing BE DAIs */
 	for (i = 0; i < count; i++) {
 		dai = (struct snd_soc_tplg_dai *)tplg->pos;
@@ -2610,9 +2571,6 @@ static int soc_tplg_manifest_load(struct soc_tplg *tplg,
 	bool abi_match;
 	int ret = 0;
 
-	if (tplg->pass != SOC_TPLG_PASS_MANIFEST)
-		return 0;
-
 	manifest = (struct snd_soc_tplg_manifest *)tplg->pos;
 
 	/* check ABI version by size, create a new manifest if abi not match */
-- 
2.25.1

Re: [PATCH v3 0/2] ASoC topology header parsing refinement

[Ranjani Sridharan]

On Wed, 2020-05-27 at 10:27 +0800, Keyon Jie wrote:
> This small series is to optimize the header logging during the
> topology
> parsing. This is verified work fine on both SOF and SST drivers.
> 
> Change History:
> v3:
> - Remove using the separated soc_pass_load() function and merge it to
> the
>   soc_tplg_load_header() body.
> - Add more Tested-by tags.
> 
> v2:
> - Change the internal used array to be 'static' to fix the issue
>   reported by: kbuild test robot <lkp@intel.com>
> - Add testing coverage including Intel SST driver also.
> 
> v1:
> - Initial version.
> 
> Keyon Jie (2):
>   ASoC: topology: refine and log the header in the correct pass
>   ASoC: topology: remove the redundant pass checks
LGTM, thanks Keyon!

Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Re: [PATCH v3 0/2] ASoC topology header parsing refinement

[Mark Brown]

On Wed, 27 May 2020 10:27:59 +0800, Keyon Jie wrote:
> This small series is to optimize the header logging during the topology
> parsing. This is verified work fine on both SOF and SST drivers.
> 
> Change History:
> v3:
> - Remove using the separated soc_pass_load() function and merge it to the
>   soc_tplg_load_header() body.
> - Add more Tested-by tags.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/2] ASoC: topology: refine and log the header in the correct pass
      commit: 82ed7418736ded9c24529987d5aa9a39c185e4e9
[2/2] ASoC: topology: remove the redundant pass checks
      commit: c2cbd0a7194d05be501fe2cb48bcd10ff468aa87

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