All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] ASoC: wm_adsp: Print error when regmap reads/writes fail
@ 2013-10-30 16:54 Dimitris Papastamos
  2013-10-30 16:54 ` [PATCH 2/3] ASoC: wm_adsp: Add support for grouped ALSA binary controls Dimitris Papastamos
  0 siblings, 1 reply; 4+ messages in thread
From: Dimitris Papastamos @ 2013-10-30 16:54 UTC (permalink / raw)
  To: broonie; +Cc: alsa-devel, patches

Change-Id: I782f7684d9ef38adcbee6c6ed6809534dc7ff9b9
Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm_adsp.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index e09ac94..cd007cc 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -339,8 +339,8 @@ static int wm_coeff_write_control(struct snd_kcontrol *kcontrol,
 	ret = regmap_raw_write(adsp->regmap, reg, scratch,
 			       ctl->len);
 	if (ret) {
-		adsp_err(adsp, "Failed to write %zu bytes to %x\n",
-			 ctl->len, reg);
+		adsp_err(adsp, "Failed to write %zu bytes to %x: %d\n",
+			 ctl->len, reg, ret);
 		kfree(scratch);
 		return ret;
 	}
@@ -393,8 +393,8 @@ static int wm_coeff_read_control(struct snd_kcontrol *kcontrol,
 
 	ret = regmap_raw_read(adsp->regmap, reg, scratch, ctl->len);
 	if (ret) {
-		adsp_err(adsp, "Failed to read %zu bytes from %x\n",
-			 ctl->len, reg);
+		adsp_err(adsp, "Failed to read %zu bytes from %x: %d\n",
+			 ctl->len, reg, ret);
 		kfree(scratch);
 		return ret;
 	}
@@ -1227,8 +1227,8 @@ static int wm_adsp_load_coeff(struct wm_adsp *dsp)
 					       le32_to_cpu(blk->len));
 			if (ret != 0) {
 				adsp_err(dsp,
-					"%s.%d: Failed to write to %x in %s\n",
-					file, blocks, reg, region_name);
+					"%s.%d: Failed to write to %x in %s: %d\n",
+					file, blocks, reg, region_name, ret);
 			}
 
 			kfree(buf);
-- 
1.8.4.2

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

* [PATCH 2/3] ASoC: wm_adsp: Add support for grouped ALSA binary controls
  2013-10-30 16:54 [PATCH 1/3] ASoC: wm_adsp: Print error when regmap reads/writes fail Dimitris Papastamos
@ 2013-10-30 16:54 ` Dimitris Papastamos
  2013-10-31  7:55   ` Takashi Iwai
  0 siblings, 1 reply; 4+ messages in thread
From: Dimitris Papastamos @ 2013-10-30 16:54 UTC (permalink / raw)
  To: broonie; +Cc: alsa-devel, patches

Currently the ALSA ABI imposes a hard limit of 512 bytes per binary
control.  To support coefficient data blocks of larger sizes we carve up
this space into multiple alsa controls.  All of these controls are
identified by a common prefix and suffix of the form ":<blk id>".

Control groupings can also consist of a single block in which case the
suffix ":0" is used.

Change-Id: Ib8560bbd98425b1732b2ccf372a53102bdeb7d7f
Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm_adsp.c | 59 +++++++++++++++++++++++++++++++++++++++-------
 sound/soc/codecs/wm_adsp.h |  1 +
 2 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index cd007cc..b3fbe4a 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -714,7 +714,6 @@ static void wm_adsp_ctl_work(struct work_struct *work)
 
 static int wm_adsp_create_control(struct wm_adsp *dsp,
 				  const struct wm_adsp_alg_region *region)
-
 {
 	struct wm_coeff_ctl *ctl;
 	struct wmfw_ctl_work *ctl_work;
@@ -746,8 +745,8 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 		return -EINVAL;
 	}
 
-	snprintf(name, PAGE_SIZE, "DSP%d %s %x",
-		 dsp->num, region_name, region->alg);
+	snprintf(name, PAGE_SIZE, "DSP%d %s %x:%d",
+		 dsp->num, region_name, region->alg, region->block);
 
 	list_for_each_entry(ctl, &dsp->ctl_list,
 			    list) {
@@ -808,6 +807,50 @@ err_name:
 	return ret;
 }
 
+static int wm_adsp_create_grouped_control(struct wm_adsp *dsp,
+					  struct wm_adsp_alg_region *region)
+{
+	size_t len = region->len, offset = 0;
+	struct wm_adsp_alg_region *r;
+	int ret;
+
+	region->block = 0;
+	/* This is the quick case for control groups of a single block */
+	if (region->len <= 512)
+		return wm_adsp_create_control(dsp, region);
+
+	/* The passed `region' is already in the list
+	 * of algorithm regions so just create the control for it and don't
+	 * add it to the list */
+	region->len = 512;
+	ret = wm_adsp_create_control(dsp, region);
+	if (ret < 0)
+		return ret;
+	offset += 512;
+
+	/* Carve up the entire region into 512-byte chunks */
+	do {
+		r = kzalloc(sizeof(*r), GFP_KERNEL);
+		if (!r)
+			return -ENOMEM;
+		r->block = offset / 512;
+		r->type = region->type;
+		r->alg = region->alg;
+		r->base = region->base + offset;
+		if (len - offset > 512)
+			r->len = 512;
+		else
+			r->len = len - offset;
+		offset += r->len;
+		list_add_tail(&r->list, &dsp->alg_regions);
+		ret = wm_adsp_create_control(dsp, r);
+		if (ret < 0)
+			return ret;
+	} while (offset < len);
+
+	return 0;
+}
+
 static int wm_adsp_setup_algs(struct wm_adsp *dsp)
 {
 	struct regmap *regmap = dsp->regmap;
@@ -983,7 +1026,7 @@ static int wm_adsp_setup_algs(struct wm_adsp *dsp)
 			if (i + 1 < algs) {
 				region->len = be32_to_cpu(adsp1_alg[i + 1].dm);
 				region->len -= be32_to_cpu(adsp1_alg[i].dm);
-				wm_adsp_create_control(dsp, region);
+				wm_adsp_create_grouped_control(dsp, region);
 			} else {
 				adsp_warn(dsp, "Missing length info for region DM with ID %x\n",
 					  be32_to_cpu(adsp1_alg[i].alg.id));
@@ -1000,7 +1043,7 @@ static int wm_adsp_setup_algs(struct wm_adsp *dsp)
 			if (i + 1 < algs) {
 				region->len = be32_to_cpu(adsp1_alg[i + 1].zm);
 				region->len -= be32_to_cpu(adsp1_alg[i].zm);
-				wm_adsp_create_control(dsp, region);
+				wm_adsp_create_grouped_control(dsp, region);
 			} else {
 				adsp_warn(dsp, "Missing length info for region ZM with ID %x\n",
 					  be32_to_cpu(adsp1_alg[i].alg.id));
@@ -1029,7 +1072,7 @@ static int wm_adsp_setup_algs(struct wm_adsp *dsp)
 			if (i + 1 < algs) {
 				region->len = be32_to_cpu(adsp2_alg[i + 1].xm);
 				region->len -= be32_to_cpu(adsp2_alg[i].xm);
-				wm_adsp_create_control(dsp, region);
+				wm_adsp_create_grouped_control(dsp, region);
 			} else {
 				adsp_warn(dsp, "Missing length info for region XM with ID %x\n",
 					  be32_to_cpu(adsp2_alg[i].alg.id));
@@ -1046,7 +1089,7 @@ static int wm_adsp_setup_algs(struct wm_adsp *dsp)
 			if (i + 1 < algs) {
 				region->len = be32_to_cpu(adsp2_alg[i + 1].ym);
 				region->len -= be32_to_cpu(adsp2_alg[i].ym);
-				wm_adsp_create_control(dsp, region);
+				wm_adsp_create_grouped_control(dsp, region);
 			} else {
 				adsp_warn(dsp, "Missing length info for region YM with ID %x\n",
 					  be32_to_cpu(adsp2_alg[i].alg.id));
@@ -1063,7 +1106,7 @@ static int wm_adsp_setup_algs(struct wm_adsp *dsp)
 			if (i + 1 < algs) {
 				region->len = be32_to_cpu(adsp2_alg[i + 1].zm);
 				region->len -= be32_to_cpu(adsp2_alg[i].zm);
-				wm_adsp_create_control(dsp, region);
+				wm_adsp_create_grouped_control(dsp, region);
 			} else {
 				adsp_warn(dsp, "Missing length info for region ZM with ID %x\n",
 					  be32_to_cpu(adsp2_alg[i].alg.id));
diff --git a/sound/soc/codecs/wm_adsp.h b/sound/soc/codecs/wm_adsp.h
index 7603167..60a7482 100644
--- a/sound/soc/codecs/wm_adsp.h
+++ b/sound/soc/codecs/wm_adsp.h
@@ -27,6 +27,7 @@ struct wm_adsp_region {
 
 struct wm_adsp_alg_region {
 	struct list_head list;
+	unsigned int block;
 	unsigned int alg;
 	int type;
 	unsigned int base;
-- 
1.8.4.2

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

* Re: [PATCH 2/3] ASoC: wm_adsp: Add support for grouped ALSA binary controls
  2013-10-30 16:54 ` [PATCH 2/3] ASoC: wm_adsp: Add support for grouped ALSA binary controls Dimitris Papastamos
@ 2013-10-31  7:55   ` Takashi Iwai
  2013-11-01 10:46     ` Dimitris Papastamos
  0 siblings, 1 reply; 4+ messages in thread
From: Takashi Iwai @ 2013-10-31  7:55 UTC (permalink / raw)
  To: Dimitris Papastamos; +Cc: alsa-devel, broonie, patches

At Wed, 30 Oct 2013 16:54:29 +0000,
Dimitris Papastamos wrote:
> 
> Currently the ALSA ABI imposes a hard limit of 512 bytes per binary
> control.  To support coefficient data blocks of larger sizes we carve up
> this space into multiple alsa controls.  All of these controls are
> identified by a common prefix and suffix of the form ":<blk id>".
> 
> Control groupings can also consist of a single block in which case the
> suffix ":0" is used.

Why not using the control element index?

You can create each kctl element individually with a different index,
but also you can create multiple ctl elements in a single shot by
passing to snd_kcontrol_new.count field.  This will create a grouped 
object, thus it saves spaces, too, in comparison with individual
kctls.  The drawback is that you need to retrieve the real index via
snd_ctl_get_ioff*() in each control callback.


Takashi

> 
> Change-Id: Ib8560bbd98425b1732b2ccf372a53102bdeb7d7f
> Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
> ---
>  sound/soc/codecs/wm_adsp.c | 59 +++++++++++++++++++++++++++++++++++++++-------
>  sound/soc/codecs/wm_adsp.h |  1 +
>  2 files changed, 52 insertions(+), 8 deletions(-)
> 
> diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
> index cd007cc..b3fbe4a 100644
> --- a/sound/soc/codecs/wm_adsp.c
> +++ b/sound/soc/codecs/wm_adsp.c
> @@ -714,7 +714,6 @@ static void wm_adsp_ctl_work(struct work_struct *work)
>  
>  static int wm_adsp_create_control(struct wm_adsp *dsp,
>  				  const struct wm_adsp_alg_region *region)
> -
>  {
>  	struct wm_coeff_ctl *ctl;
>  	struct wmfw_ctl_work *ctl_work;
> @@ -746,8 +745,8 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
>  		return -EINVAL;
>  	}
>  
> -	snprintf(name, PAGE_SIZE, "DSP%d %s %x",
> -		 dsp->num, region_name, region->alg);
> +	snprintf(name, PAGE_SIZE, "DSP%d %s %x:%d",
> +		 dsp->num, region_name, region->alg, region->block);
>  
>  	list_for_each_entry(ctl, &dsp->ctl_list,
>  			    list) {
> @@ -808,6 +807,50 @@ err_name:
>  	return ret;
>  }
>  
> +static int wm_adsp_create_grouped_control(struct wm_adsp *dsp,
> +					  struct wm_adsp_alg_region *region)
> +{
> +	size_t len = region->len, offset = 0;
> +	struct wm_adsp_alg_region *r;
> +	int ret;
> +
> +	region->block = 0;
> +	/* This is the quick case for control groups of a single block */
> +	if (region->len <= 512)
> +		return wm_adsp_create_control(dsp, region);
> +
> +	/* The passed `region' is already in the list
> +	 * of algorithm regions so just create the control for it and don't
> +	 * add it to the list */
> +	region->len = 512;
> +	ret = wm_adsp_create_control(dsp, region);
> +	if (ret < 0)
> +		return ret;
> +	offset += 512;
> +
> +	/* Carve up the entire region into 512-byte chunks */
> +	do {
> +		r = kzalloc(sizeof(*r), GFP_KERNEL);
> +		if (!r)
> +			return -ENOMEM;
> +		r->block = offset / 512;
> +		r->type = region->type;
> +		r->alg = region->alg;
> +		r->base = region->base + offset;
> +		if (len - offset > 512)
> +			r->len = 512;
> +		else
> +			r->len = len - offset;
> +		offset += r->len;
> +		list_add_tail(&r->list, &dsp->alg_regions);
> +		ret = wm_adsp_create_control(dsp, r);
> +		if (ret < 0)
> +			return ret;
> +	} while (offset < len);
> +
> +	return 0;
> +}
> +
>  static int wm_adsp_setup_algs(struct wm_adsp *dsp)
>  {
>  	struct regmap *regmap = dsp->regmap;
> @@ -983,7 +1026,7 @@ static int wm_adsp_setup_algs(struct wm_adsp *dsp)
>  			if (i + 1 < algs) {
>  				region->len = be32_to_cpu(adsp1_alg[i + 1].dm);
>  				region->len -= be32_to_cpu(adsp1_alg[i].dm);
> -				wm_adsp_create_control(dsp, region);
> +				wm_adsp_create_grouped_control(dsp, region);
>  			} else {
>  				adsp_warn(dsp, "Missing length info for region DM with ID %x\n",
>  					  be32_to_cpu(adsp1_alg[i].alg.id));
> @@ -1000,7 +1043,7 @@ static int wm_adsp_setup_algs(struct wm_adsp *dsp)
>  			if (i + 1 < algs) {
>  				region->len = be32_to_cpu(adsp1_alg[i + 1].zm);
>  				region->len -= be32_to_cpu(adsp1_alg[i].zm);
> -				wm_adsp_create_control(dsp, region);
> +				wm_adsp_create_grouped_control(dsp, region);
>  			} else {
>  				adsp_warn(dsp, "Missing length info for region ZM with ID %x\n",
>  					  be32_to_cpu(adsp1_alg[i].alg.id));
> @@ -1029,7 +1072,7 @@ static int wm_adsp_setup_algs(struct wm_adsp *dsp)
>  			if (i + 1 < algs) {
>  				region->len = be32_to_cpu(adsp2_alg[i + 1].xm);
>  				region->len -= be32_to_cpu(adsp2_alg[i].xm);
> -				wm_adsp_create_control(dsp, region);
> +				wm_adsp_create_grouped_control(dsp, region);
>  			} else {
>  				adsp_warn(dsp, "Missing length info for region XM with ID %x\n",
>  					  be32_to_cpu(adsp2_alg[i].alg.id));
> @@ -1046,7 +1089,7 @@ static int wm_adsp_setup_algs(struct wm_adsp *dsp)
>  			if (i + 1 < algs) {
>  				region->len = be32_to_cpu(adsp2_alg[i + 1].ym);
>  				region->len -= be32_to_cpu(adsp2_alg[i].ym);
> -				wm_adsp_create_control(dsp, region);
> +				wm_adsp_create_grouped_control(dsp, region);
>  			} else {
>  				adsp_warn(dsp, "Missing length info for region YM with ID %x\n",
>  					  be32_to_cpu(adsp2_alg[i].alg.id));
> @@ -1063,7 +1106,7 @@ static int wm_adsp_setup_algs(struct wm_adsp *dsp)
>  			if (i + 1 < algs) {
>  				region->len = be32_to_cpu(adsp2_alg[i + 1].zm);
>  				region->len -= be32_to_cpu(adsp2_alg[i].zm);
> -				wm_adsp_create_control(dsp, region);
> +				wm_adsp_create_grouped_control(dsp, region);
>  			} else {
>  				adsp_warn(dsp, "Missing length info for region ZM with ID %x\n",
>  					  be32_to_cpu(adsp2_alg[i].alg.id));
> diff --git a/sound/soc/codecs/wm_adsp.h b/sound/soc/codecs/wm_adsp.h
> index 7603167..60a7482 100644
> --- a/sound/soc/codecs/wm_adsp.h
> +++ b/sound/soc/codecs/wm_adsp.h
> @@ -27,6 +27,7 @@ struct wm_adsp_region {
>  
>  struct wm_adsp_alg_region {
>  	struct list_head list;
> +	unsigned int block;
>  	unsigned int alg;
>  	int type;
>  	unsigned int base;
> -- 
> 1.8.4.2
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

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

* Re: [PATCH 2/3] ASoC: wm_adsp: Add support for grouped ALSA binary controls
  2013-10-31  7:55   ` Takashi Iwai
@ 2013-11-01 10:46     ` Dimitris Papastamos
  0 siblings, 0 replies; 4+ messages in thread
From: Dimitris Papastamos @ 2013-11-01 10:46 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, broonie, patches

On Thu, Oct 31, 2013 at 08:55:53AM +0100, Takashi Iwai wrote:
> At Wed, 30 Oct 2013 16:54:29 +0000,
> Dimitris Papastamos wrote:
> > 
> > Currently the ALSA ABI imposes a hard limit of 512 bytes per binary
> > control.  To support coefficient data blocks of larger sizes we carve up
> > this space into multiple alsa controls.  All of these controls are
> > identified by a common prefix and suffix of the form ":<blk id>".
> > 
> > Control groupings can also consist of a single block in which case the
> > suffix ":0" is used.
> 
> Why not using the control element index?
> 
> You can create each kctl element individually with a different index,
> but also you can create multiple ctl elements in a single shot by
> passing to snd_kcontrol_new.count field.  This will create a grouped 
> object, thus it saves spaces, too, in comparison with individual
> kctls.  The drawback is that you need to retrieve the real index via
> snd_ctl_get_ioff*() in each control callback.

I will give this a shot when I have some time and re-send the patch.

Thanks,
Dimitris

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

end of thread, other threads:[~2013-11-01 10:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-30 16:54 [PATCH 1/3] ASoC: wm_adsp: Print error when regmap reads/writes fail Dimitris Papastamos
2013-10-30 16:54 ` [PATCH 2/3] ASoC: wm_adsp: Add support for grouped ALSA binary controls Dimitris Papastamos
2013-10-31  7:55   ` Takashi Iwai
2013-11-01 10:46     ` Dimitris Papastamos

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.