Alsa-Devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Amadeusz Sławiński" <amadeuszx.slawinski@linux.intel.com>
To: Cezary Rojewski <cezary.rojewski@intel.com>, alsa-devel@alsa-project.org
Cc: andriy.shevchenko@intel.com, filip.kaczmarski@intel.com,
	harshapriya.n@intel.com, ppapierkowski@habana.ai,
	marcin.barlik@intel.com, zwisler@google.com,
	pierre-louis.bossart@linux.intel.com, lgirdwood@gmail.com,
	filip.proborszcz@intel.com, broonie@kernel.org,
	michal.wasko@intel.com, tiwai@suse.com,
	krzysztof.hejmowski@intel.com, cujomalainey@chromium.org,
	vamshi.krishna.gopal@intel.com
Subject: Re: [PATCH v2 06/13] ASoC: Intel: catpt: PCM operations
Date: Tue, 11 Aug 2020 14:17:29 +0200	[thread overview]
Message-ID: <e02fc6e7-20aa-4977-d2a3-a78e3e696df2@linux.intel.com> (raw)
In-Reply-To: <20200811100034.6875-7-cezary.rojewski@intel.com>



On 8/11/2020 12:00 PM, Cezary Rojewski wrote:
> DSP designed for Lynxpoint and Wildcat Point offers no dynamic topology
> i.e. all pipelines are already defined within firmware and host is
> relegated to allocing stream for predefined pins. This is represented by
> 'catpt_topology' member.
> 
> Implementation covers all available pin types:
> - system playback and capture
> - two offload streams
> - loopback (reference)
> - bluetooth playback and capture
> 
> PCM DAI operations differentiate between those pins as some (mainly
> offload) are to be handled differently - DSP expects wp updates on each
> notify_position notification.
> 
> System playback has no volume control capability as it is routed to
> mixer stream directly. Other primary streams - capture and two offloads
> - offer individual volume controls.
> 
> Compared to sound/soc/intel/haswell this configures SSP device format
> automatically on pcm creation.
> 
> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
> ---

...

> +
> +static int catpt_set_ctlvol(struct catpt_dev *cdev, u8 stream_id, long *ctlvol)
> +{
> +	u32 dspvol;
> +	int ret, i;
> +
> +	if (ctlvol[0] == ctlvol[1]) {

This check seems bit suspicious to me, as CATPT_CHANNELS_MAX is 4 and 
you only check 2 of possible values here, while in else part, you loop 
over all possible 4 to set them? So there is chance that ctlvol[0] is 
equal to ctlvol[1], but ctlvol[2] and ctlvol[3] are different, what 
happens then?

> +		dspvol = ctlvol_to_dspvol(ctlvol[0]);
> +
> +		ret = catpt_ipc_set_volume(cdev, stream_id,
> +					   CATPT_ALL_CHANNELS_MASK, dspvol,
> +					   0, CATPT_AUDIO_CURVE_NONE);
> +	} else {
> +		for (i = 0; i < CATPT_CHANNELS_MAX; i++) {
> +			dspvol = ctlvol_to_dspvol(ctlvol[i]);
> +
> +			ret = catpt_ipc_set_volume(cdev, stream_id,
> +						   i, dspvol,
> +						   0, CATPT_AUDIO_CURVE_NONE);
> +			if (ret)
> +				goto exit;
> +		}
> +	}
> +exit:
> +	if (ret)
> +		return CATPT_IPC_ERROR(ret);
> +	return 0;
> +}
> +
> +static int catpt_volume_info(struct snd_kcontrol *kcontrol,
> +			     struct snd_ctl_elem_info *uinfo)
> +{
> +	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
> +	uinfo->count = CATPT_CHANNELS_MAX;
> +	uinfo->value.integer.min = 0;
> +	uinfo->value.integer.max = ARRAY_SIZE(volume_map) - 1;
> +	return 0;
> +}
> +
> +static int catpt_mixer_volume_get(struct snd_kcontrol *kcontrol,
> +				  struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component =
> +		snd_soc_kcontrol_component(kcontrol);
> +	struct catpt_dev *cdev = dev_get_drvdata(component->dev);
> +	u32 dspvol;
> +	int i;
> +
> +	pm_runtime_get_sync(cdev->dev);
> +
> +	for (i = 0; i < CATPT_CHANNELS_MAX; i++) {
> +		dspvol = catpt_mixer_volume(cdev, &cdev->mixer, i);
> +		ucontrol->value.integer.value[i] = dspvol_to_ctlvol(dspvol);
> +	}
> +
> +	pm_runtime_mark_last_busy(cdev->dev);
> +	pm_runtime_put_autosuspend(cdev->dev);
> +
> +	return 0;
> +}
> +
> +static int catpt_mixer_volume_put(struct snd_kcontrol *kcontrol,
> +				  struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component =
> +		snd_soc_kcontrol_component(kcontrol);
> +	struct catpt_dev *cdev = dev_get_drvdata(component->dev);
> +	int ret;
> +
> +	pm_runtime_get_sync(cdev->dev);
> +
> +	ret = catpt_set_ctlvol(cdev, cdev->mixer.mixer_hw_id,
> +			       ucontrol->value.integer.value);
> +
> +	pm_runtime_mark_last_busy(cdev->dev);
> +	pm_runtime_put_autosuspend(cdev->dev);
> +
> +	return ret;
> +}
> +
> +static int catpt_stream_volume_get(struct snd_kcontrol *kcontrol,
> +				   struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component =
> +		snd_soc_kcontrol_component(kcontrol);
> +	struct catpt_dev *cdev = dev_get_drvdata(component->dev);
> +	struct catpt_stream_runtime *stream;
> +	long *ctlvol = (long *)kcontrol->private_value;
> +	u32 dspvol;
> +	int i;
> +
> +	stream = catpt_stream_find(cdev, kcontrol->id.device);
> +	if (!stream) {
> +		for (i = 0; i < CATPT_CHANNELS_MAX; i++)
> +			ucontrol->value.integer.value[i] = ctlvol[i];
> +		return 0;
> +	}
> +
> +	pm_runtime_get_sync(cdev->dev);
> +
> +	for (i = 0; i < CATPT_CHANNELS_MAX; i++) {
> +		dspvol = catpt_stream_volume(cdev, stream, i);
> +		ucontrol->value.integer.value[i] = dspvol_to_ctlvol(dspvol);
> +	}
> +
> +	pm_runtime_mark_last_busy(cdev->dev);
> +	pm_runtime_put_autosuspend(cdev->dev);
> +
> +	return 0;
> +}
> +
> +static int catpt_stream_volume_put(struct snd_kcontrol *kcontrol,
> +				   struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component =
> +		snd_soc_kcontrol_component(kcontrol);
> +	struct catpt_dev *cdev = dev_get_drvdata(component->dev);
> +	struct catpt_stream_runtime *stream;
> +	long *ctlvol = (long *)kcontrol->private_value;
> +	int ret, i;
> +
> +	stream = catpt_stream_find(cdev, kcontrol->id.device);
> +	if (!stream) {
> +		for (i = 0; i < CATPT_CHANNELS_MAX; i++)
> +			ctlvol[i] = ucontrol->value.integer.value[i];
> +		return 0;
> +	}
> +
> +	pm_runtime_get_sync(cdev->dev);
> +
> +	ret = catpt_set_ctlvol(cdev, stream->info.stream_hw_id,
> +			       ucontrol->value.integer.value);
> +
> +	pm_runtime_mark_last_busy(cdev->dev);
> +	pm_runtime_put_autosuspend(cdev->dev);
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	for (i = 0; i < CATPT_CHANNELS_MAX; i++)
> +		ctlvol[i] = ucontrol->value.integer.value[i];
> +	return 0;
> +}
> +
> +static int catpt_loopback_switch_get(struct snd_kcontrol *kcontrol,
> +				     struct snd_ctl_elem_value *ucontrol)
> +{
> +	ucontrol->value.integer.value[0] = *(bool *)kcontrol->private_value;
> +	return 0;
> +}
> +
> +static int catpt_loopback_switch_put(struct snd_kcontrol *kcontrol,
> +				     struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component =
> +		snd_soc_kcontrol_component(kcontrol);
> +	struct catpt_dev *cdev = dev_get_drvdata(component->dev);
> +	struct catpt_stream_runtime *stream;
> +	bool mute;
> +	int ret;
> +
> +	mute = (bool)ucontrol->value.integer.value[0];
> +
> +	stream = catpt_stream_find(cdev, kcontrol->id.device);
> +	if (!stream) {
> +		*(bool *)kcontrol->private_value = mute;
> +		return 0;
> +	}
> +
> +	pm_runtime_get_sync(cdev->dev);
> +
> +	ret = catpt_ipc_mute_loopback(cdev, stream->info.stream_hw_id, mute);
> +
> +	pm_runtime_mark_last_busy(cdev->dev);
> +	pm_runtime_put_autosuspend(cdev->dev);
> +
> +	if (ret)
> +		return CATPT_IPC_ERROR(ret);
> +
> +	*(bool *)kcontrol->private_value = mute;
> +	return 0;
> +}
> +
> +static const SNDRV_CTL_TLVD_DECLARE_DB_SCALE(catpt_volume_tlv, -9000, 300, 1);
> +
> +static int catpt_component_probe(struct snd_soc_component *component)
> +{
> +	struct snd_kcontrol_new templates[5];
> +	struct snd_kcontrol_new volctl = {
> +		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
> +		.access = SNDRV_CTL_ELEM_ACCESS_TLV_READ |
> +			  SNDRV_CTL_ELEM_ACCESS_READWRITE,
> +		.info = catpt_volume_info,
> +		.tlv.p = catpt_volume_tlv,
> +	};
> +	void *p;
> +	int i;
> +
> +	/* master volume (mixer stream) */
> +	templates[0] = volctl;
> +	templates[0].name = "Master Playback Volume";
> +	templates[0].get = catpt_mixer_volume_get;
> +	templates[0].put = catpt_mixer_volume_put;
> +
> +	/* individual volume controls for offload and capture */
> +	for (i = 1; i < 4; i++) {
> +		/* volume storage for each of CATPT_CHANNELS_MAX */
> +		p = devm_kcalloc(component->dev, CATPT_CHANNELS_MAX,
> +				 sizeof(long), GFP_KERNEL);
> +		if (!p)
> +			return -ENOMEM;
> +		templates[i] = volctl;
> +		templates[i].get = catpt_stream_volume_get;
> +		templates[i].put = catpt_stream_volume_put;
> +		templates[i].private_value = (unsigned long)p;
> +	}
> +
> +	templates[1].name = "Media0 Playback Volume";
> +	templates[1].device = CATPT_PIN_ID_OFFLOAD1;
> +	templates[2].name = "Media1 Playback Volume";
> +	templates[2].device = CATPT_PIN_ID_OFFLOAD2;
> +	templates[3].name = "Mic Capture Volume";
> +	templates[3].device = CATPT_PIN_ID_CAPTURE1;
> +
> +	/* reference stream mute */
> +	p = devm_kzalloc(component->dev, sizeof(bool), GFP_KERNEL);
> +	if (!p)
> +		return -ENOMEM;
> +	templates[4] = (struct snd_kcontrol_new)
> +		SOC_SINGLE_BOOL_EXT("Loopback Mute", (unsigned long)p,
> +				    catpt_loopback_switch_get,
> +				    catpt_loopback_switch_put);
> +	templates[4].device = CATPT_PIN_ID_REFERENCE;
> +
> +	return snd_soc_add_component_controls(component, templates,
> +					      ARRAY_SIZE(templates));
> +}
> +
> +static int catpt_waves_switch_get(struct snd_kcontrol *kcontrol,
> +				  struct snd_ctl_elem_value *ucontrol)
> +{
> +	return 0;
> +}
> +
> +static int catpt_waves_switch_put(struct snd_kcontrol *kcontrol,
> +				  struct snd_ctl_elem_value *ucontrol)
> +{
> +	return 0;
> +}
> +
> +static int catpt_waves_param_get(struct snd_kcontrol *kcontrol,
> +				 unsigned int __user *bytes,
> +				 unsigned int size)
> +{
> +	return 0;
> +}
> +
> +static int catpt_waves_param_put(struct snd_kcontrol *kcontrol,
> +				 const unsigned int __user *bytes,
> +				 unsigned int size)
> +{
> +	return 0;
> +}
> +
> +static const struct snd_kcontrol_new component_kcontrols[] = {
> +/* Enable or disable WAVES module */
> +SOC_SINGLE_BOOL_EXT("Waves Switch", 0,
> +		    catpt_waves_switch_get, catpt_waves_switch_put),
> +/* WAVES module parameter control */
> +SND_SOC_BYTES_TLV("Waves Set Param", 128,
> +		  catpt_waves_param_get, catpt_waves_param_put),
> +};
> +
> +static const struct snd_soc_dapm_widget component_widgets[] = {
> +	SND_SOC_DAPM_AIF_IN("SSP0 CODEC IN", NULL, 0, SND_SOC_NOPM, 0, 0),
> +	SND_SOC_DAPM_AIF_OUT("SSP0 CODEC OUT", NULL, 0, SND_SOC_NOPM, 0, 0),
> +	SND_SOC_DAPM_AIF_IN("SSP1 BT IN", NULL, 0, SND_SOC_NOPM, 0, 0),
> +	SND_SOC_DAPM_AIF_OUT("SSP1 BT OUT", NULL, 0, SND_SOC_NOPM, 0, 0),
> +
> +	SND_SOC_DAPM_MIXER("Playback VMixer", SND_SOC_NOPM, 0, 0, NULL, 0),
> +};
> +
> +static const struct snd_soc_dapm_route component_routes[] = {
> +	{"Playback VMixer", NULL, "System Playback"},
> +	{"Playback VMixer", NULL, "Offload0 Playback"},
> +	{"Playback VMixer", NULL, "Offload1 Playback"},
> +
> +	{"SSP0 CODEC OUT", NULL, "Playback VMixer"},
> +
> +	{"Analog Capture", NULL, "SSP0 CODEC IN"},
> +	{"Loopback Capture", NULL, "SSP0 CODEC IN"},
> +
> +	{"SSP1 BT OUT", NULL, "Bluetooth Playback"},
> +	{"Bluetooth Capture", NULL, "SSP1 BT IN"},
> +};
> +
> +static const struct snd_soc_component_driver catpt_comp_driver = {
> +	.name = "catpt-platform",
> +
> +	.probe = catpt_component_probe,
> +	.pcm_construct = catpt_component_pcm_construct,
> +	.open = catpt_component_open,
> +	.pointer = catpt_component_pointer,
> +
> +	.controls = component_kcontrols,
> +	.num_controls = ARRAY_SIZE(component_kcontrols),
> +	.dapm_widgets = component_widgets,
> +	.num_dapm_widgets = ARRAY_SIZE(component_widgets),
> +	.dapm_routes = component_routes,
> +	.num_dapm_routes = ARRAY_SIZE(component_routes),
> +};
> +
> +int catpt_arm_stream_templates(struct catpt_dev *cdev)
> +{
> +	struct resource *res;
> +	u32 scratch_size = 0;
> +	int i, j;
> +
> +	for (i = 0; i < ARRAY_SIZE(catpt_topology); i++) {
> +		struct catpt_stream_template *template;
> +		struct catpt_module_entry *entry;
> +		struct catpt_module_type *type;
> +
> +		template = catpt_topology[i];
> +		template->persistent_size = 0;
> +
> +		for (j = 0; j < template->num_entries; j++) {
> +			entry = &template->entries[j];
> +			type = &cdev->modules[entry->module_id];
> +
> +			if (!type->loaded)
> +				return -ENOENT;
> +
> +			entry->entry_point = type->entry_point;
> +			template->persistent_size += type->persistent_size;
> +			if (type->scratch_size > scratch_size)
> +				scratch_size = type->scratch_size;
> +		}
> +	}
> +
> +	if (scratch_size) {
> +		/* allocate single scratch area for all modules */
> +		res = catpt_request_region(&cdev->dram, scratch_size);
> +		if (!res)
> +			return -EBUSY;
> +		cdev->scratch = res;
> +	}
> +
> +	return 0;
> +}
> +
> +int catpt_register_plat_component(struct catpt_dev *cdev)
> +{
> +	struct snd_soc_component *component;
> +	int ret;
> +
> +	component = devm_kzalloc(cdev->dev, sizeof(*component), GFP_KERNEL);
> +	if (!component)
> +		return -ENOMEM;
> +
> +	ret = snd_soc_component_initialize(component, &catpt_comp_driver,
> +					   cdev->dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	component->name = catpt_comp_driver.name;
> +	return snd_soc_add_component(component, dai_drivers,
> +				     ARRAY_SIZE(dai_drivers));
> +}
> 

  reply	other threads:[~2020-08-11 12:20 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-11 10:00 [PATCH v2 00/13] ASoC: Intel: Catpt - Lynx and Wildcat point Cezary Rojewski
2020-08-11 10:00 ` [PATCH v2 01/13] ASoC: Intel: Add catpt device Cezary Rojewski
2020-08-11 12:17   ` Amadeusz Sławiński
2020-08-11 12:24     ` Mark Brown
2020-08-11 12:45     ` Cezary Rojewski
2020-08-11 10:00 ` [PATCH v2 02/13] ASoC: Intel: catpt: Define DSP operations Cezary Rojewski
2020-08-11 12:17   ` Amadeusz Sławiński
2020-08-11 12:49     ` Cezary Rojewski
2020-08-11 10:00 ` [PATCH v2 03/13] ASoC: Intel: catpt: Firmware loading and context restore Cezary Rojewski
2020-08-11 10:00 ` [PATCH v2 04/13] ASoC: Intel: catpt: Implement IPC protocol Cezary Rojewski
2020-08-11 10:00 ` [PATCH v2 05/13] ASoC: Intel: catpt: Add IPC messages Cezary Rojewski
2020-08-11 10:00 ` [PATCH v2 06/13] ASoC: Intel: catpt: PCM operations Cezary Rojewski
2020-08-11 12:17   ` Amadeusz Sławiński [this message]
2020-08-11 12:44     ` Cezary Rojewski
2020-08-11 10:00 ` [PATCH v2 07/13] ASoC: Intel: catpt: Event tracing Cezary Rojewski
2020-08-11 10:00 ` [PATCH v2 08/13] ASoC: Intel: catpt: Simple sysfs attributes Cezary Rojewski
2020-08-11 10:00 ` [PATCH v2 09/13] ASoC: Intel: Select catpt and deprecate haswell Cezary Rojewski
2020-08-11 13:08   ` kernel test robot
2020-08-11 13:38     ` Andy Shevchenko
2020-08-11 14:09       ` Cezary Rojewski
2020-08-11 14:41         ` Andy Shevchenko
2020-08-11 10:00 ` [PATCH v2 10/13] ASoC: Intel: haswell: Remove haswell-solution specific code Cezary Rojewski
2020-08-11 10:00 ` [PATCH v2 11/13] ASoC: Intel: broadwell: " Cezary Rojewski
2020-08-11 10:00 ` [PATCH v2 12/13] ASoC: Intel: bdw-5650: " Cezary Rojewski
2020-08-11 10:00 ` [PATCH v2 13/13] ASoC: Intel: bdw-5677: " 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=e02fc6e7-20aa-4977-d2a3-a78e3e696df2@linux.intel.com \
    --to=amadeuszx.slawinski@linux.intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=andriy.shevchenko@intel.com \
    --cc=broonie@kernel.org \
    --cc=cezary.rojewski@intel.com \
    --cc=cujomalainey@chromium.org \
    --cc=filip.kaczmarski@intel.com \
    --cc=filip.proborszcz@intel.com \
    --cc=harshapriya.n@intel.com \
    --cc=krzysztof.hejmowski@intel.com \
    --cc=lgirdwood@gmail.com \
    --cc=marcin.barlik@intel.com \
    --cc=michal.wasko@intel.com \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=ppapierkowski@habana.ai \
    --cc=tiwai@suse.com \
    --cc=vamshi.krishna.gopal@intel.com \
    --cc=zwisler@google.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox