All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Cezary Rojewski <cezary.rojewski@intel.com>
Cc: alsa-devel@alsa-project.org, Mark Brown <broonie@kernel.org>,
	tiwai@suse.com, lgirdwood@gmail.com,
	pierre-louis.bossart@linux.intel.com
Subject: Applied "ASoC: Intel: Skylake: Make MCPS and CPS params obsolete" to the asoc tree
Date: Wed, 24 Jul 2019 20:17:50 +0100 (BST)	[thread overview]
Message-ID: <20190724191750.C64902742B60@ypsilon.sirena.org.uk> (raw)
In-Reply-To: <20190723145854.8527-7-cezary.rojewski@intel.com>

The patch

   ASoC: Intel: Skylake: Make MCPS and CPS params obsolete

has been applied to the asoc tree at

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

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

>From 84b71067ea840fadee32588aa3967d0d8c4e0b9a Mon Sep 17 00:00:00 2001
From: Cezary Rojewski <cezary.rojewski@intel.com>
Date: Tue, 23 Jul 2019 16:58:53 +0200
Subject: [PATCH] ASoC: Intel: Skylake: Make MCPS and CPS params obsolete

As per FW Interface Modules Configuration, init instance IPC request
requires base initial module configuration. This configuration structure
is made of:
- cpc (chunks per cycle)
- ibs (input buffer size)
- obs (output buffer size)
- is_pages (memory pages required)
- audio_fmt (self explanatory)

Skylake topology accepts following tokens: MCPS, CPS and CPC. All of
these are directly connected. Moreover, assigning one of these allows
to calculate the remaining two. In simplest scenario and assuming 1ms
scheduling, following is true:

CPS = CPC times 1000
MCPS = CPS times 1000 000
Note: these calculations vary depending on scenario and scheduling
requirements.

Given the current implementation, userspace is allowed to provide
different values for all three causing informational chaos. On top of
that, struct skl_base_cfg which represents base module configuration,
incorrectly takes CPS param instead of CPC.

This ambiguity may lead to user unintentionally providing improper
values to DSP firmware and thus impacting module scheduling in
unexpected fashion. Fix by making MCPS and CPS topology params obsolete
and relying solely on CPC value.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Link: https://lore.kernel.org/r/20190723145854.8527-7-cezary.rojewski@intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/skylake/skl-debug.c    |  6 ++++--
 sound/soc/intel/skylake/skl-messages.c |  2 +-
 sound/soc/intel/skylake/skl-topology.c | 15 ++++++---------
 sound/soc/intel/skylake/skl-topology.h |  4 +---
 4 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-debug.c b/sound/soc/intel/skylake/skl-debug.c
index c43aa4081232..fb232428109f 100644
--- a/sound/soc/intel/skylake/skl-debug.c
+++ b/sound/soc/intel/skylake/skl-debug.c
@@ -66,6 +66,8 @@ static ssize_t module_read(struct file *file, char __user *user_buf,
 			   size_t count, loff_t *ppos)
 {
 	struct skl_module_cfg *mconfig = file->private_data;
+	struct skl_module *module = mconfig->module;
+	struct skl_module_res *res = &module->resources[mconfig->res_idx];
 	char *buf;
 	ssize_t ret;
 
@@ -79,8 +81,8 @@ static ssize_t module_read(struct file *file, char __user *user_buf,
 			mconfig->id.pvt_id);
 
 	ret += snprintf(buf + ret, MOD_BUF - ret,
-			"Resources:\n\tMCPS %#x\n\tIBS %#x\n\tOBS %#x\t\n",
-			mconfig->mcps, mconfig->ibs, mconfig->obs);
+			"Resources:\n\tCPC %#x\n\tIBS %#x\n\tOBS %#x\t\n",
+			res->cpc, res->ibs, res->obs);
 
 	ret += snprintf(buf + ret, MOD_BUF - ret,
 			"Module data:\n\tCore %d\n\tIn queue %d\n\t"
diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c
index 07762543fb13..e8cc710f092b 100644
--- a/sound/soc/intel/skylake/skl-messages.c
+++ b/sound/soc/intel/skylake/skl-messages.c
@@ -477,7 +477,7 @@ static void skl_set_base_module_format(struct skl_dev *skl,
 
 	base_cfg->audio_fmt.interleaving = format->interleaving_style;
 
-	base_cfg->cps = res->cps;
+	base_cfg->cpc = res->cpc;
 	base_cfg->ibs = res->ibs;
 	base_cfg->obs = res->obs;
 	base_cfg->is_pages = res->is_pages;
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index 53a024c0464d..118866cd5075 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -2205,10 +2205,6 @@ static int skl_tplg_fill_res_tkn(struct device *dev,
 		return -EINVAL;
 
 	switch (tkn_elem->token) {
-	case SKL_TKN_MM_U32_CPS:
-		res->cps = tkn_elem->value;
-		break;
-
 	case SKL_TKN_MM_U32_DMA_SIZE:
 		res->dma_buffer_size = tkn_elem->value;
 		break;
@@ -2229,10 +2225,6 @@ static int skl_tplg_fill_res_tkn(struct device *dev,
 		res->ibs = tkn_elem->value;
 		break;
 
-	case SKL_TKN_U32_MAX_MCPS:
-		res->cps = tkn_elem->value;
-		break;
-
 	case SKL_TKN_MM_U32_RES_PIN_ID:
 	case SKL_TKN_MM_U32_PIN_BUF:
 		ret = skl_tplg_manifest_pin_res_tkn(dev, tkn_elem, res,
@@ -2241,6 +2233,11 @@ static int skl_tplg_fill_res_tkn(struct device *dev,
 			return ret;
 		break;
 
+	case SKL_TKN_MM_U32_CPS:
+	case SKL_TKN_U32_MAX_MCPS:
+		/* ignore unused tokens */
+		break;
+
 	default:
 		dev_err(dev, "Not a res type token: %d", tkn_elem->token);
 		return -EINVAL;
@@ -2693,7 +2690,7 @@ static int skl_tplg_get_pvt_data_v4(struct snd_soc_tplg_dapm_widget *tplg_w,
 		return ret;
 	mconfig->id.module_id = -1;
 	mconfig->id.instance_id = dfw->instance_id;
-	mconfig->module->resources[0].cps = dfw->max_mcps;
+	mconfig->module->resources[0].cpc = dfw->max_mcps / 1000;
 	mconfig->module->resources[0].ibs = dfw->ibs;
 	mconfig->module->resources[0].obs = dfw->obs;
 	mconfig->core_id = dfw->core_id;
diff --git a/sound/soc/intel/skylake/skl-topology.h b/sound/soc/intel/skylake/skl-topology.h
index e2a2fc5c5545..99a0277191ca 100644
--- a/sound/soc/intel/skylake/skl-topology.h
+++ b/sound/soc/intel/skylake/skl-topology.h
@@ -101,7 +101,7 @@ struct skl_audio_data_format {
 } __packed;
 
 struct skl_base_cfg {
-	u32 cps;
+	u32 cpc;
 	u32 ibs;
 	u32 obs;
 	u32 is_pages;
@@ -343,7 +343,6 @@ struct skl_module_pin_resources {
 struct skl_module_res {
 	u8 id;
 	u32 is_pages;
-	u32 cps;
 	u32 ibs;
 	u32 obs;
 	u32 dma_buffer_size;
@@ -384,7 +383,6 @@ struct skl_module_cfg {
 	u8 out_queue_mask;
 	u8 in_queue;
 	u8 out_queue;
-	u32 mcps;
 	u32 ibs;
 	u32 obs;
 	u8 is_loadable;
-- 
2.20.1

  reply	other threads:[~2019-07-24 19:17 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-23 14:58 [RESEND PATCH v2 0/7] ASoC: Intel: Skylake: Driver fundaments overhaul Cezary Rojewski
2019-07-23 14:58 ` [RESEND PATCH v2 1/7] ASoC: Intel: Skylake: Merge skl_sst and skl into skl_dev struct Cezary Rojewski
2019-07-24 19:17   ` Applied "ASoC: Intel: Skylake: Merge skl_sst and skl into skl_dev struct" to the asoc tree Mark Brown
2019-07-23 14:58 ` [RESEND PATCH v2 2/7] ASoC: Intel: Skylake: Combine snd_soc_skl_ipc and snd_soc_skl Cezary Rojewski
2019-07-23 14:58 ` [RESEND PATCH v2 3/7] ASoC: Intel: Skylake: Remove MCPS available check Cezary Rojewski
2019-07-24 19:17   ` Applied "ASoC: Intel: Skylake: Remove MCPS available check" to the asoc tree Mark Brown
2019-07-23 14:58 ` [RESEND PATCH v2 4/7] ASoC: Intel: Skylake: Remove memory available check Cezary Rojewski
2019-07-24 19:17   ` Applied "ASoC: Intel: Skylake: Remove memory available check" to the asoc tree Mark Brown
2019-07-23 14:58 ` [RESEND PATCH v2 5/7] ASoC: Intel: Skylake: Do not disable FW notifications Cezary Rojewski
2019-07-23 14:58 ` [RESEND PATCH v2 6/7] ASoC: Intel: Skylake: Make MCPS and CPS params obsolete Cezary Rojewski
2019-07-24 19:17   ` Mark Brown [this message]
2019-07-23 14:58 ` [RESEND PATCH v2 7/7] ASoC: Intel: Skylake: Cleanup skl_module_cfg declaration Cezary Rojewski
2019-07-24 19:17   ` Applied "ASoC: Intel: Skylake: Cleanup skl_module_cfg declaration" to the asoc tree Mark Brown
2019-07-23 15:44 ` [RESEND PATCH v2 0/7] ASoC: Intel: Skylake: Driver fundaments overhaul Mark Brown
2019-07-23 18:07   ` Pierre-Louis Bossart
2019-07-24 16:50     ` Mark Brown
2019-07-24 17:14     ` Cezary Rojewski
2019-07-24 18:10       ` Mark Brown
2019-07-24 12:39 ` Vinod Koul
2019-07-24 12:42 ` Vinod Koul
2019-07-26 11:31   ` Cezary Rojewski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190724191750.C64902742B60@ypsilon.sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=cezary.rojewski@intel.com \
    --cc=lgirdwood@gmail.com \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.