[PATCH 0/4] McBSP smart idle and DMA op mode updates for ASoC

[PATCH 0/4] McBSP smart idle and DMA op mode updates for ASoC

[Liam Girdwood]

This series expands the OMAP mcbsp driver to support changing it's DMA operating
mode and smart idle mode from client drivers. It's primarily aimed at lowering
the power consumption for OMAP ASoC drivers by providing methods to gate clocks
on the mcbsp interface at runtime.

I've also added a patch to remove the mcbsp DMA op mode sysfs set functionality.
I think DMA op mode is very specific to the mcbsp client driver _only_ and
shouldn't really be changed by userspace. Please let me know if you use this
feature and I'll drop this patch.

Tony, can I get your ack to upstream via ALSA if it's ok

Thanks

Liam

 arch/arm/plat-omap/include/plat/mcbsp.h |   17 +++
 arch/arm/plat-omap/mcbsp.c              |  183 ++++++++++++++++++++++++++-----
 sound/soc/omap/omap-mcbsp.c             |   22 ++++-
 sound/soc/omap/omap-mcbsp.h             |    2 +
 4 files changed, 194 insertions(+), 30 deletions(-)

[PATCH 1/4] OMAP: mcbsp - add omap_mcbsp_set_dma_op_mode()

[Liam Girdwood]

This adds a method to set the MCBSP DMA OP mode.

Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
---
 arch/arm/plat-omap/include/plat/mcbsp.h |    2 ++
 arch/arm/plat-omap/mcbsp.c              |   31 +++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/mcbsp.h b/arch/arm/plat-omap/include/plat/mcbsp.h
index 1bd7021..f8823f4 100644
--- a/arch/arm/plat-omap/include/plat/mcbsp.h
+++ b/arch/arm/plat-omap/include/plat/mcbsp.h
@@ -476,6 +476,7 @@ u16 omap_mcbsp_get_max_rx_threshold(unsigned int id);
 u16 omap_mcbsp_get_tx_delay(unsigned int id);
 u16 omap_mcbsp_get_rx_delay(unsigned int id);
 int omap_mcbsp_get_dma_op_mode(unsigned int id);
+int omap_mcbsp_set_dma_op_mode(unsigned int id, unsigned int mode);
 #else
 static inline void omap_mcbsp_set_tx_threshold(unsigned int id, u16 threshold)
 { }
@@ -486,6 +487,7 @@ static inline u16 omap_mcbsp_get_max_rx_threshold(unsigned int id) { return 0; }
 static inline u16 omap_mcbsp_get_tx_delay(unsigned int id) { return 0; }
 static inline u16 omap_mcbsp_get_rx_delay(unsigned int id) { return 0; }
 static inline int omap_mcbsp_get_dma_op_mode(unsigned int id) { return 0; }
+static inline int omap_mcbsp_set_dma_op_mode(unsigned int id, unsigned int mode) {return 0;}
 #endif
 int omap_mcbsp_request(unsigned int id);
 void omap_mcbsp_free(unsigned int id);
diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index 4820cab..cc2b73c 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -635,6 +635,37 @@ int omap_mcbsp_get_dma_op_mode(unsigned int id)
 }
 EXPORT_SYMBOL(omap_mcbsp_get_dma_op_mode);
 
+/*
+ * omap_mcbsp_set_dma_op_mode set the current DMA
+ * operating mode for the mcbsp channel
+ */
+int omap_mcbsp_set_dma_op_mode(unsigned int id, unsigned int mode)
+{
+	struct omap_mcbsp *mcbsp;
+	int ret = 0;
+
+	if (mode > MCBSP_DMA_MODE_FRAME)
+		return -EINVAL;
+
+	if (!omap_mcbsp_check_valid_id(id)) {
+		printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
+		return -ENODEV;
+	}
+	mcbsp = id_to_mcbsp_ptr(id);
+
+	spin_lock_irq(&mcbsp->lock);
+	if (!mcbsp->free) {
+		ret = -EBUSY;
+		goto unlock;
+	}
+	mcbsp->dma_op_mode = mode;
+
+unlock:
+	spin_unlock_irq(&mcbsp->lock);
+	return ret;
+}
+EXPORT_SYMBOL(omap_mcbsp_set_dma_op_mode);
+
 static inline void omap34xx_mcbsp_request(struct omap_mcbsp *mcbsp)
 {
 	/*
-- 
1.7.0.4

[PATCH 2/4] OMAP: mcbsp - add smart idle configuration API

[Liam Girdwood]

Add a small API to configure McBSP smart idle modes
to conserve power.

Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
---
 arch/arm/plat-omap/include/plat/mcbsp.h |   15 ++++
 arch/arm/plat-omap/mcbsp.c              |  122 +++++++++++++++++++++++++++++++
 2 files changed, 137 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/mcbsp.h b/arch/arm/plat-omap/include/plat/mcbsp.h
index f8823f4..3f9fb71 100644
--- a/arch/arm/plat-omap/include/plat/mcbsp.h
+++ b/arch/arm/plat-omap/include/plat/mcbsp.h
@@ -278,6 +278,15 @@
 #define ENAWAKEUP		0x0004
 #define SOFTRST			0x0002
 
+#define MCBSP_CLK_ACT_IOFF_POFF		0
+#define MCBSP_CLK_ACT_ION_POFF		1
+#define MCBSP_CLK_ACT_IOFF_PON		2
+#define MCBSP_CLK_ACT_ION_PON			3
+
+#define MCBSP_IDLE_FORCE	0
+#define MCBSP_IDLE_NONE		1
+#define MCBSP_IDLE_SMART	2
+
 /********************** McBSP SSELCR bit definitions ***********************/
 #define SIDETONEEN		0x0400
 
@@ -456,6 +465,7 @@ struct omap_mcbsp {
 #ifdef CONFIG_ARCH_OMAP3
 	struct omap_mcbsp_st_data *st_data;
 	int dma_op_mode;
+	int idle_mode;
 	u16 max_tx_thres;
 	u16 max_rx_thres;
 #endif
@@ -477,6 +487,11 @@ u16 omap_mcbsp_get_tx_delay(unsigned int id);
 u16 omap_mcbsp_get_rx_delay(unsigned int id);
 int omap_mcbsp_get_dma_op_mode(unsigned int id);
 int omap_mcbsp_set_dma_op_mode(unsigned int id, unsigned int mode);
+int omap_mcbsp_set_idle_smart(unsigned int id, unsigned int clk_activity,
+		unsigned int wake);
+int omap_mcbsp_set_idle_none(unsigned int id);
+int omap_mcbsp_set_idle_force(unsigned int id);
+int omap_mcbsp_get_idle_mode(unsigned int id);
 #else
 static inline void omap_mcbsp_set_tx_threshold(unsigned int id, u16 threshold)
 { }
diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index cc2b73c..7785050 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -1743,6 +1743,128 @@ static inline void __devexit omap34xx_device_exit(struct omap_mcbsp *mcbsp)
 			omap_st_remove(mcbsp);
 	}
 }
+
+/* assert standby requests when idle */
+int omap_mcbsp_set_idle_smart(unsigned int id, unsigned int clk_activity,
+		u32 wakeup)
+{
+	struct omap_mcbsp *mcbsp;
+	u16 syscon;
+	int ret = 0;
+
+	if (!omap_mcbsp_check_valid_id(id)) {
+		printk(KERN_ERR "%s: Invalid id (%u)\n", __func__, id + 1);
+		return -ENODEV;
+	}
+	mcbsp = id_to_mcbsp_ptr(id);
+
+	spin_lock_irq(&mcbsp->lock);
+	if (!mcbsp->free) {
+		ret = -EBUSY;
+		goto unlock;
+	}
+
+	syscon = MCBSP_READ(mcbsp, SYSCON) &
+			~(ENAWAKEUP | SIDLEMODE(0x03) | CLOCKACTIVITY(0x03));
+	MCBSP_WRITE(mcbsp, WAKEUPEN, wakeup);
+	MCBSP_WRITE(mcbsp, SYSCON,
+			syscon | SIDLEMODE(MCBSP_IDLE_SMART) |
+			CLOCKACTIVITY(clk_activity) | ENAWAKEUP);
+	mcbsp->idle_mode = MCBSP_IDLE_SMART;
+
+unlock:
+	spin_unlock_irq(&mcbsp->lock);
+	return ret;
+}
+EXPORT_SYMBOL(omap_mcbsp_set_idle_smart);
+
+/* never assert standby requests */
+int omap_mcbsp_set_idle_none(unsigned int id)
+{
+	struct omap_mcbsp *mcbsp;
+	u16 syscon;
+	int ret = 0;
+
+	if (!omap_mcbsp_check_valid_id(id)) {
+		printk(KERN_ERR "%s: Invalid id (%u)\n", __func__, id + 1);
+		return -ENODEV;
+	}
+	mcbsp = id_to_mcbsp_ptr(id);
+
+	spin_lock_irq(&mcbsp->lock);
+	if (!mcbsp->free) {
+		ret = -EBUSY;
+		goto unlock;
+	}
+
+	syscon = MCBSP_READ(mcbsp, SYSCON) &
+			~(ENAWAKEUP | SIDLEMODE(0x03) | CLOCKACTIVITY(0x03));
+
+	MCBSP_WRITE(mcbsp, SYSCON, syscon | SIDLEMODE(MCBSP_IDLE_NONE));
+	MCBSP_WRITE(mcbsp, WAKEUPEN, 0);
+	mcbsp->idle_mode = MCBSP_IDLE_NONE;
+
+unlock:
+	spin_unlock_irq(&mcbsp->lock);
+	return ret;
+}
+EXPORT_SYMBOL(omap_mcbsp_set_idle_none);
+
+/* unconditionally assert standby requests */
+int omap_mcbsp_set_idle_force(unsigned int id)
+{
+	struct omap_mcbsp *mcbsp;
+	u16 syscon;
+	int ret = 0;
+
+	if (!omap_mcbsp_check_valid_id(id)) {
+		printk(KERN_ERR "%s: Invalid id (%u)\n", __func__, id + 1);
+		return -ENODEV;
+	}
+	mcbsp = id_to_mcbsp_ptr(id);
+
+	spin_lock_irq(&mcbsp->lock);
+	if (!mcbsp->free) {
+		ret = -EBUSY;
+		goto unlock;
+	}
+
+	syscon = MCBSP_READ(mcbsp, SYSCON) &
+			~(ENAWAKEUP | SIDLEMODE(0x03) | CLOCKACTIVITY(0x03));
+	/*
+	 * HW bug workaround - If no_idle mode is taken, we need to
+	 * go to smart_idle before going to always_idle, or the
+	 * device will not hit retention anymore.
+	 */
+	syscon |= SIDLEMODE(MCBSP_IDLE_SMART);
+	MCBSP_WRITE(mcbsp, SYSCON, syscon);
+	syscon &= ~(SIDLEMODE(0x03));
+
+	MCBSP_WRITE(mcbsp, SYSCON, syscon | SIDLEMODE(MCBSP_IDLE_FORCE));
+	MCBSP_WRITE(mcbsp, WAKEUPEN, 0);
+	mcbsp->idle_mode = MCBSP_IDLE_FORCE;
+
+unlock:
+	spin_unlock_irq(&mcbsp->lock);
+	return ret;
+}
+EXPORT_SYMBOL(omap_mcbsp_set_idle_force);
+
+int omap_mcbsp_get_idle_mode(unsigned int id)
+{
+	struct omap_mcbsp *mcbsp;
+
+	if (!omap_mcbsp_check_valid_id(id)) {
+		printk(KERN_ERR "%s: Invalid id (%u)\n", __func__, id + 1);
+		return -ENODEV;
+	}
+	mcbsp = id_to_mcbsp_ptr(id);
+
+	return mcbsp->idle_mode;
+}
+EXPORT_SYMBOL(omap_mcbsp_get_idle_mode);
+
+
 #else
 static inline void __devinit omap34xx_device_init(struct omap_mcbsp *mcbsp) {}
 static inline void __devexit omap34xx_device_exit(struct omap_mcbsp *mcbsp) {}
-- 
1.7.0.4

[PATCH 3/4] ASoC: mcbsp - add machine threshold callback

[Liam Girdwood]

Add a mechanism to register a machine specific callback
to calculate and set the McBSP Tx/Rx threshold.

Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
---
 sound/soc/omap/omap-mcbsp.c |   22 +++++++++++++++++++++-
 sound/soc/omap/omap-mcbsp.h |    2 ++
 2 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c
index 6f44cb4..9a1583d 100644
--- a/sound/soc/omap/omap-mcbsp.c
+++ b/sound/soc/omap/omap-mcbsp.c
@@ -51,6 +51,9 @@ struct omap_mcbsp_data {
 	unsigned int			bus_id;
 	struct omap_mcbsp_reg_cfg	regs;
 	unsigned int			fmt;
+
+	/* optional machine set_threshold() sample value */
+	void (*mach_set_threshold)(struct snd_pcm_substream *substream);
 	/*
 	 * Flags indicating is the bus already activated and configured by
 	 * another substream
@@ -306,7 +309,11 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream,
 	} else if (cpu_is_omap343x()) {
 		dma = omap24xx_dma_reqs[bus_id][substream->stream];
 		port = omap34xx_mcbsp_port[bus_id][substream->stream];
-		omap_mcbsp_dai_dma_params[id][substream->stream].set_threshold =
+		if (mcbsp_data->mach_set_threshold)
+			omap_mcbsp_dai_dma_params[id][substream->stream].set_threshold =
+						mcbsp_data->mach_set_threshold;
+		else
+			omap_mcbsp_dai_dma_params[id][substream->stream].set_threshold =
 						omap_mcbsp_set_threshold;
 		/* TODO: Currently, MODE_ELEMENT == MODE_FRAME */
 		if (omap_mcbsp_get_dma_op_mode(bus_id) ==
@@ -835,6 +842,19 @@ int omap_mcbsp_st_add_controls(struct snd_soc_codec *codec, int mcbsp_id)
 }
 EXPORT_SYMBOL_GPL(omap_mcbsp_st_add_controls);
 
+int omap_bcbsp_set_threshold_func(struct snd_soc_dai *cpu_dai,
+		void (*mach_set_threshold)(struct snd_pcm_substream *substream))
+{
+	struct omap_mcbsp_data *mcbsp_data = to_mcbsp(cpu_dai->private_data);
+
+	if (!cpu_is_omap34xx())
+		return -ENODEV;
+
+	mcbsp_data->mach_set_threshold = mach_set_threshold;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(omap_bcbsp_set_threshold_func);
+
 static int __init snd_omap_mcbsp_init(void)
 {
 	return snd_soc_register_dais(omap_mcbsp_dai,
diff --git a/sound/soc/omap/omap-mcbsp.h b/sound/soc/omap/omap-mcbsp.h
index 6c363e5..f8d8044 100644
--- a/sound/soc/omap/omap-mcbsp.h
+++ b/sound/soc/omap/omap-mcbsp.h
@@ -58,5 +58,7 @@ enum omap_mcbsp_div {
 extern struct snd_soc_dai omap_mcbsp_dai[NUM_LINKS];
 
 int omap_mcbsp_st_add_controls(struct snd_soc_codec *codec, int mcbsp_id);
+int omap_bcbsp_set_threshold_func(struct snd_soc_dai *cpu_dai,
+		void (*mach_set_threshold)(struct snd_pcm_substream *substream));
 
 #endif
-- 
1.7.0.4

[PATCH 4/4] OMAP: mcbsp - remove sysfs set for DMA op mode.

[Liam Girdwood]

The mcbsp DMA op mode is tightly integrated with the
mcbsp client driver operation and hence is unsuitable
for userspace configuration via sysfs.

Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
---
 arch/arm/plat-omap/mcbsp.c |   30 +-----------------------------
 1 files changed, 1 insertions(+), 29 deletions(-)

diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index 7785050..b7b8b5e 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -1546,35 +1546,7 @@ static ssize_t dma_op_mode_show(struct device *dev,
 	return len;
 }
 
-static ssize_t dma_op_mode_store(struct device *dev,
-				struct device_attribute *attr,
-				const char *buf, size_t size)
-{
-	struct omap_mcbsp *mcbsp = dev_get_drvdata(dev);
-	const char * const *s;
-	int i = 0;
-
-	for (s = &dma_op_modes[i]; i < ARRAY_SIZE(dma_op_modes); s++, i++)
-		if (sysfs_streq(buf, *s))
-			break;
-
-	if (i == ARRAY_SIZE(dma_op_modes))
-		return -EINVAL;
-
-	spin_lock_irq(&mcbsp->lock);
-	if (!mcbsp->free) {
-		size = -EBUSY;
-		goto unlock;
-	}
-	mcbsp->dma_op_mode = i;
-
-unlock:
-	spin_unlock_irq(&mcbsp->lock);
-
-	return size;
-}
-
-static DEVICE_ATTR(dma_op_mode, 0644, dma_op_mode_show, dma_op_mode_store);
+static DEVICE_ATTR(dma_op_mode, 0644, dma_op_mode_show, NULL);
 
 static ssize_t st_taps_show(struct device *dev,
 			    struct device_attribute *attr, char *buf)
-- 
1.7.0.4

Re: [PATCH 3/4] ASoC: mcbsp - add machine threshold callback

[Mark Brown]

On Tue, May 18, 2010 at 09:13:13PM +0100, Liam Girdwood wrote:
> Add a mechanism to register a machine specific callback
> to calculate and set the McBSP Tx/Rx threshold.

It's a shame the default implementation can't work for everyone but
there we are.  :(

> Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>

All of these:

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [PATCH 2/4] OMAP: mcbsp - add smart idle configuration API

[Nishanth Menon]

Liam Girdwood had written, on 05/18/2010 03:13 PM, the following:
> Add a small API to configure McBSP smart idle modes
> to conserve power.
> 
> Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
> ---
>  arch/arm/plat-omap/include/plat/mcbsp.h |   15 ++++
>  arch/arm/plat-omap/mcbsp.c              |  122 +++++++++++++++++++++++++++++++
>  2 files changed, 137 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/plat/mcbsp.h b/arch/arm/plat-omap/include/plat/mcbsp.h
> index f8823f4..3f9fb71 100644
> --- a/arch/arm/plat-omap/include/plat/mcbsp.h
> +++ b/arch/arm/plat-omap/include/plat/mcbsp.h
> @@ -278,6 +278,15 @@
>  #define ENAWAKEUP		0x0004
>  #define SOFTRST			0x0002
>  
> +#define MCBSP_CLK_ACT_IOFF_POFF		0
> +#define MCBSP_CLK_ACT_ION_POFF		1
> +#define MCBSP_CLK_ACT_IOFF_PON		2
> +#define MCBSP_CLK_ACT_ION_PON			3
> +
> +#define MCBSP_IDLE_FORCE	0
> +#define MCBSP_IDLE_NONE		1
> +#define MCBSP_IDLE_SMART	2
> +
>  /********************** McBSP SSELCR bit definitions ***********************/
>  #define SIDETONEEN		0x0400
>  
> @@ -456,6 +465,7 @@ struct omap_mcbsp {
>  #ifdef CONFIG_ARCH_OMAP3
>  	struct omap_mcbsp_st_data *st_data;
>  	int dma_op_mode;
> +	int idle_mode;
>  	u16 max_tx_thres;
>  	u16 max_rx_thres;
>  #endif
> @@ -477,6 +487,11 @@ u16 omap_mcbsp_get_tx_delay(unsigned int id);
>  u16 omap_mcbsp_get_rx_delay(unsigned int id);
>  int omap_mcbsp_get_dma_op_mode(unsigned int id);
>  int omap_mcbsp_set_dma_op_mode(unsigned int id, unsigned int mode);
> +int omap_mcbsp_set_idle_smart(unsigned int id, unsigned int clk_activity,
> +		unsigned int wake);
> +int omap_mcbsp_set_idle_none(unsigned int id);
> +int omap_mcbsp_set_idle_force(unsigned int id);
> +int omap_mcbsp_get_idle_mode(unsigned int id);
>  #else
>  static inline void omap_mcbsp_set_tx_threshold(unsigned int id, u16 threshold)
>  { }
> diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
> index cc2b73c..7785050 100644
> --- a/arch/arm/plat-omap/mcbsp.c
> +++ b/arch/arm/plat-omap/mcbsp.c
> @@ -1743,6 +1743,128 @@ static inline void __devexit omap34xx_device_exit(struct omap_mcbsp *mcbsp)
>  			omap_st_remove(mcbsp);
>  	}
>  }
> +
> +/* assert standby requests when idle */
> +int omap_mcbsp_set_idle_smart(unsigned int id, unsigned int clk_activity,
> +		u32 wakeup)
> +{
> +	struct omap_mcbsp *mcbsp;
> +	u16 syscon;
> +	int ret = 0;
> +
> +	if (!omap_mcbsp_check_valid_id(id)) {
> +		printk(KERN_ERR "%s: Invalid id (%u)\n", __func__, id + 1);
> +		return -ENODEV;
> +	}
> +	mcbsp = id_to_mcbsp_ptr(id);
> +
> +	spin_lock_irq(&mcbsp->lock);
> +	if (!mcbsp->free) {
> +		ret = -EBUSY;
> +		goto unlock;
> +	}
> +
> +	syscon = MCBSP_READ(mcbsp, SYSCON) &
> +			~(ENAWAKEUP | SIDLEMODE(0x03) | CLOCKACTIVITY(0x03));
> +	MCBSP_WRITE(mcbsp, WAKEUPEN, wakeup);
> +	MCBSP_WRITE(mcbsp, SYSCON,
> +			syscon | SIDLEMODE(MCBSP_IDLE_SMART) |
> +			CLOCKACTIVITY(clk_activity) | ENAWAKEUP);
> +	mcbsp->idle_mode = MCBSP_IDLE_SMART;
> +
> +unlock:
> +	spin_unlock_irq(&mcbsp->lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL(omap_mcbsp_set_idle_smart);
> +
> +/* never assert standby requests */
> +int omap_mcbsp_set_idle_none(unsigned int id)
> +{
> +	struct omap_mcbsp *mcbsp;
> +	u16 syscon;
> +	int ret = 0;
> +
> +	if (!omap_mcbsp_check_valid_id(id)) {
> +		printk(KERN_ERR "%s: Invalid id (%u)\n", __func__, id + 1);
> +		return -ENODEV;
> +	}
> +	mcbsp = id_to_mcbsp_ptr(id);
> +
> +	spin_lock_irq(&mcbsp->lock);
> +	if (!mcbsp->free) {
> +		ret = -EBUSY;
> +		goto unlock;
> +	}
> +
> +	syscon = MCBSP_READ(mcbsp, SYSCON) &
> +			~(ENAWAKEUP | SIDLEMODE(0x03) | CLOCKACTIVITY(0x03));
> +
> +	MCBSP_WRITE(mcbsp, SYSCON, syscon | SIDLEMODE(MCBSP_IDLE_NONE));
> +	MCBSP_WRITE(mcbsp, WAKEUPEN, 0);
> +	mcbsp->idle_mode = MCBSP_IDLE_NONE;
> +
> +unlock:
> +	spin_unlock_irq(&mcbsp->lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL(omap_mcbsp_set_idle_none);
> +
> +/* unconditionally assert standby requests */
> +int omap_mcbsp_set_idle_force(unsigned int id)
> +{
> +	struct omap_mcbsp *mcbsp;
> +	u16 syscon;
> +	int ret = 0;
> +
> +	if (!omap_mcbsp_check_valid_id(id)) {
> +		printk(KERN_ERR "%s: Invalid id (%u)\n", __func__, id + 1);
> +		return -ENODEV;
> +	}
> +	mcbsp = id_to_mcbsp_ptr(id);
> +
> +	spin_lock_irq(&mcbsp->lock);
> +	if (!mcbsp->free) {
> +		ret = -EBUSY;
> +		goto unlock;
> +	}
> +
> +	syscon = MCBSP_READ(mcbsp, SYSCON) &
> +			~(ENAWAKEUP | SIDLEMODE(0x03) | CLOCKACTIVITY(0x03));
> +	/*
> +	 * HW bug workaround - If no_idle mode is taken, we need to
> +	 * go to smart_idle before going to always_idle, or the
> +	 * device will not hit retention anymore.
> +	 */
> +	syscon |= SIDLEMODE(MCBSP_IDLE_SMART);
> +	MCBSP_WRITE(mcbsp, SYSCON, syscon);
> +	syscon &= ~(SIDLEMODE(0x03));
> +
> +	MCBSP_WRITE(mcbsp, SYSCON, syscon | SIDLEMODE(MCBSP_IDLE_FORCE));
> +	MCBSP_WRITE(mcbsp, WAKEUPEN, 0);
> +	mcbsp->idle_mode = MCBSP_IDLE_FORCE;

Just a curious question - how does this work with hwmod infrastructure?

> +
> +unlock:
> +	spin_unlock_irq(&mcbsp->lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL(omap_mcbsp_set_idle_force);
> +
> +int omap_mcbsp_get_idle_mode(unsigned int id)
> +{
> +	struct omap_mcbsp *mcbsp;
> +
> +	if (!omap_mcbsp_check_valid_id(id)) {
> +		printk(KERN_ERR "%s: Invalid id (%u)\n", __func__, id + 1);
> +		return -ENODEV;
> +	}
> +	mcbsp = id_to_mcbsp_ptr(id);
> +
> +	return mcbsp->idle_mode;
> +}
> +EXPORT_SYMBOL(omap_mcbsp_get_idle_mode);
> +
> +
>  #else
>  static inline void __devinit omap34xx_device_init(struct omap_mcbsp *mcbsp) {}
>  static inline void __devexit omap34xx_device_exit(struct omap_mcbsp *mcbsp) {}


-- 
Regards,
Nishanth Menon

Re: [PATCH 3/4] ASoC: mcbsp - add machine threshold callback

[Candelaria Villarreal, Jorge]

alsa-devel-bounces@alsa-project.org wrote:
> Add a mechanism to register a machine specific callback
> to calculate and set the McBSP Tx/Rx threshold.
> 
> Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
> ---
>  sound/soc/omap/omap-mcbsp.c |   22 +++++++++++++++++++++-
>  sound/soc/omap/omap-mcbsp.h |    2 ++
>  2 files changed, 23 insertions(+), 1 deletions(-)
> 
> diff --git a/sound/soc/omap/omap-mcbsp.c
> b/sound/soc/omap/omap-mcbsp.c index 6f44cb4..9a1583d 100644 ---
> a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c
> @@ -51,6 +51,9 @@ struct omap_mcbsp_data {
>  	unsigned int			bus_id;
>  	struct omap_mcbsp_reg_cfg	regs;
>  	unsigned int			fmt;
> +
> +	/* optional machine set_threshold() sample value */
> +	void (*mach_set_threshold)(struct snd_pcm_substream *substream); 
>  	 /* * Flags indicating is the bus already activated and
> configured by
>  	 * another substream
> @@ -306,7 +309,11 @@ static int
> omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream,
>  	} else if (cpu_is_omap343x()) {
>  		dma = omap24xx_dma_reqs[bus_id][substream->stream];
>  		port = omap34xx_mcbsp_port[bus_id][substream->stream]; -
> omap_mcbsp_dai_dma_params[id][substream->stream].set_threshold =
> +		if (mcbsp_data->mach_set_threshold)
> +
> omap_mcbsp_dai_dma_params[id][substream->stream].set_threshold = +
> mcbsp_data->mach_set_threshold;
> +		else
> +
> omap_mcbsp_dai_dma_params[id][substream->stream].set_threshold =
> 
> omap_mcbsp_set_threshold;
>  		/* TODO: Currently, MODE_ELEMENT == MODE_FRAME */
>  		if (omap_mcbsp_get_dma_op_mode(bus_id) ==
> @@ -835,6 +842,19 @@ int omap_mcbsp_st_add_controls(struct
> snd_soc_codec *codec, int mcbsp_id)
>  }
>  EXPORT_SYMBOL_GPL(omap_mcbsp_st_add_controls);
> 
> +int omap_bcbsp_set_threshold_func(struct snd_soc_dai *cpu_dai,
> +		void (*mach_set_threshold)(struct
> snd_pcm_substream *substream))

I think you meant omap_mcbsp_set_threshold_func here.

> +{
> +	struct omap_mcbsp_data *mcbsp_data =
> to_mcbsp(cpu_dai->private_data);
> +
> +	if (!cpu_is_omap34xx())
> +		return -ENODEV;
> +
> +	mcbsp_data->mach_set_threshold = mach_set_threshold; +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(omap_bcbsp_set_threshold_func);
> +
>  static int __init snd_omap_mcbsp_init(void)
>  {
>  	return snd_soc_register_dais(omap_mcbsp_dai,
> diff --git a/sound/soc/omap/omap-mcbsp.h
> b/sound/soc/omap/omap-mcbsp.h index 6c363e5..f8d8044 100644 ---
> a/sound/soc/omap/omap-mcbsp.h +++ b/sound/soc/omap/omap-mcbsp.h
> @@ -58,5 +58,7 @@ enum omap_mcbsp_div {
>  extern struct snd_soc_dai omap_mcbsp_dai[NUM_LINKS];
> 
>  int omap_mcbsp_st_add_controls(struct snd_soc_codec *codec,
> int mcbsp_id);
> +int omap_bcbsp_set_threshold_func(struct snd_soc_dai *cpu_dai,
> +		void (*mach_set_threshold)(struct
> snd_pcm_substream *substream));

Also here.

> 
>  #endif

Re: [alsa-devel] [PATCH 2/4] OMAP: mcbsp - add smart idle configuration API

[Kevin Hilman]

Liam Girdwood <lrg@slimlogic.co.uk> writes:

> Add a small API to configure McBSP smart idle modes
> to conserve power.

Would be useful here to explain why client drivers need to change
idle modes.

Also, this patch is telling me that it's time the McBSP is converted
over to the omap_hwmod/omap_device infrastructure where this type
of functionality would already be provided.

Not necesssarily a reason to prevent this patch for now, but this
will all need to be changed when converted to hwmod.

Kevin

Re: [PATCH 0/4] McBSP smart idle and DMA op mode updates for ASoC

[Jarkko Nikula]

On Tue, 18 May 2010 21:13:10 +0100
Liam Girdwood <lrg@slimlogic.co.uk> wrote:

> I've also added a patch to remove the mcbsp DMA op mode sysfs set functionality.
> I think DMA op mode is very specific to the mcbsp client driver _only_ and
> shouldn't really be changed by userspace. Please let me know if you use this
> feature and I'll drop this patch.
> 
I've used to say that the DMA op mode is more like use-case not machine
specific but I'm not sure is my point valid onemore. I've used to think
that low-latency processing would need accurate DMA pointer (op
mode == element) while mp3 playback would need low power consumption
(op mode == threshold).

Peter: what's the status today how well can we do low-latency
processing with threshold mode? IRCC, with your FIFO delay query
patches, can we estimate the DMA pointer position with enough accuracy?


-- 
Jarkko

Re: [PATCH 2/4] OMAP: mcbsp - add smart idle configuration API

[Jarkko Nikula]

On Tue, 18 May 2010 21:13:12 +0100
Liam Girdwood <lrg@slimlogic.co.uk> wrote:

> Add a small API to configure McBSP smart idle modes
> to conserve power.
> 
> Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
> ---
>  arch/arm/plat-omap/include/plat/mcbsp.h |   15 ++++
>  arch/arm/plat-omap/mcbsp.c              |  122 +++++++++++++++++++++++++++++++
>  2 files changed, 137 insertions(+), 0 deletions(-)
> 
> +#define MCBSP_IDLE_FORCE	0
> +#define MCBSP_IDLE_NONE		1
> +#define MCBSP_IDLE_SMART	2
> +
...
> +int omap_mcbsp_set_idle_smart(unsigned int id, unsigned int clk_activity,
> +		unsigned int wake);
> +int omap_mcbsp_set_idle_none(unsigned int id);
> +int omap_mcbsp_set_idle_force(unsigned int id);
> +int omap_mcbsp_get_idle_mode(unsigned int id);

Only minor comment but IMO single set function looks better. E.g.

omap_mcbsp_set_idle_mode(unsigned int id, int mode);


-- 
Jarkko

Re: [PATCH 3/4] ASoC: mcbsp - add machine threshold callback

[Jarkko Nikula]

On Tue, 18 May 2010 15:56:46 -0500
"Candelaria Villarreal, Jorge" <jorge.candelaria@ti.com> wrote:

> alsa-devel-bounces@alsa-project.org wrote:
> > Add a mechanism to register a machine specific callback
> > to calculate and set the McBSP Tx/Rx threshold.
> > 
...
> > +int omap_bcbsp_set_threshold_func(struct snd_soc_dai *cpu_dai,
> > +		void (*mach_set_threshold)(struct
> > snd_pcm_substream *substream));
> 
> Also here.
> 
Same comments, no any other objections but couple lines in commit log
would be nice have when machine would benefit from an own callback.
I.e. there could be boards that benefit from this.


-- 
Jarkko

Re: [PATCH 1/4] OMAP: mcbsp - add omap_mcbsp_set_dma_op_mode()

[Jarkko Nikula]

On Tue, 18 May 2010 21:13:11 +0100
Liam Girdwood <lrg@slimlogic.co.uk> wrote:

> This adds a method to set the MCBSP DMA OP mode.
> 
> Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
> ---
>  arch/arm/plat-omap/include/plat/mcbsp.h |    2 ++
>  arch/arm/plat-omap/mcbsp.c              |   31 +++++++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/plat/mcbsp.h b/arch/arm/plat-omap/include/plat/mcbsp.h
> index 1bd7021..f8823f4 100644
> --- a/arch/arm/plat-omap/include/plat/mcbsp.h
> +++ b/arch/arm/plat-omap/include/plat/mcbsp.h
> @@ -476,6 +476,7 @@ u16 omap_mcbsp_get_max_rx_threshold(unsigned int id);
>  u16 omap_mcbsp_get_tx_delay(unsigned int id);
>  u16 omap_mcbsp_get_rx_delay(unsigned int id);
>  int omap_mcbsp_get_dma_op_mode(unsigned int id);
> +int omap_mcbsp_set_dma_op_mode(unsigned int id, unsigned int mode);

This should have been here even in private form when the get function
was introduced. Let's wait for Peter's comment about op mode sysfs
functionality will we go with patch 4/4 or convert it to use this
function.

But anyway this patch is good to have.

Acked-by: Jarkko Nikula <jhnikula@gmail.com>

Re: [PATCH 0/4] McBSP smart idle and DMA op mode updates for ASoC

[Peter Ujfalusi]

On Wednesday 19 May 2010 08:13:25 ext Jarkko Nikula wrote:
> On Tue, 18 May 2010 21:13:10 +0100
> 
> Liam Girdwood <lrg@slimlogic.co.uk> wrote:
> > I've also added a patch to remove the mcbsp DMA op mode sysfs set
> > functionality. I think DMA op mode is very specific to the mcbsp client
> > driver _only_ and shouldn't really be changed by userspace. Please let
> > me know if you use this feature and I'll drop this patch.
> 
> I've used to say that the DMA op mode is more like use-case not machine
> specific but I'm not sure is my point valid onemore. I've used to think
> that low-latency processing would need accurate DMA pointer (op
> mode == element) while mp3 playback would need low power consumption
> (op mode == threshold).

Yeah, it used to be so clear ;)
I see these:
element mode: if user want to have constant latency [1]
threshold mode: variable latency, but possibility to save power [2]

[1] The McBSP is kept full during playback (and empty during capture)
The DMA pointer moves word-by-word

[2] The McBSP FIFO fill rate changes (full, drain, refill, full, drain...)
The DMA pointer moves in bursts.
Between burst memories can relax, core, MPU also in theory.
Smart idle helps to conserve more power here

> Peter: what's the status today how well can we do low-latency
> processing with threshold mode? IRCC, with your FIFO delay query
> patches, can we estimate the DMA pointer position with enough accuracy?

The DMA pointer is easy, and it was know before as well, but according to my 
tests, the McBSP FIFO caused delay reporting is fairly accurate.
I'll ask my users, if they have done some additional tests.

-- 
Péter

Re: [PATCH 0/4] McBSP smart idle and DMA op mode updates for ASoC

[Peter Ujfalusi]

On Wednesday 19 May 2010 11:31:57 Ujfalusi Peter (Nokia-D/Tampere) wrote:
> On Wednesday 19 May 2010 08:13:25 ext Jarkko Nikula wrote:
> > On Tue, 18 May 2010 21:13:10 +0100
> > 
> > Liam Girdwood <lrg@slimlogic.co.uk> wrote:
> > > I've also added a patch to remove the mcbsp DMA op mode sysfs set
> > > functionality. I think DMA op mode is very specific to the mcbsp client
> > > driver _only_ and shouldn't really be changed by userspace. Please let
> > > me know if you use this feature and I'll drop this patch.
> > 
> > I've used to say that the DMA op mode is more like use-case not machine
> > specific but I'm not sure is my point valid onemore. I've used to think
> > that low-latency processing would need accurate DMA pointer (op
> > mode == element) while mp3 playback would need low power consumption
> > (op mode == threshold).
> 
> Yeah, it used to be so clear ;)
> I see these:
> element mode: if user want to have constant latency [1]
> threshold mode: variable latency, but possibility to save power [2]
> 
> [1] The McBSP is kept full during playback (and empty during capture)
> The DMA pointer moves word-by-word

So the latency is the length of the McBSP FIFO.

> 
> [2] The McBSP FIFO fill rate changes (full, drain, refill, full, drain...)
> The DMA pointer moves in bursts.
> Between burst memories can relax, core, MPU also in theory.
> Smart idle helps to conserve more power here

The latency depends on when you are asking.
When the FIFO is full, than you have maximum latency, right at the end of the 
drain phase, you will have the shortest latency.

> 
> > Peter: what's the status today how well can we do low-latency
> > processing with threshold mode? IRCC, with your FIFO delay query
> > patches, can we estimate the DMA pointer position with enough accuracy?
> 
> The DMA pointer is easy, and it was know before as well, but according to
> my tests, the McBSP FIFO caused delay reporting is fairly accurate.
> I'll ask my users, if they have done some additional tests.

So it is not that clear how to view them ;)

-- 
Péter

Re: [alsa-devel] [PATCH 3/4] ASoC: mcbsp - add machine threshold callback

[Liam Girdwood]

On Tue, 2010-05-18 at 15:56 -0500, Candelaria Villarreal, Jorge wrote:
> > 
> > +int omap_bcbsp_set_threshold_func(struct snd_soc_dai *cpu_dai,
> > +             void (*mach_set_threshold)(struct
> > snd_pcm_substream *substream))
> 
> I think you meant omap_mcbsp_set_threshold_func here. 

I did indeed !

Thanks

Liam
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 0/4] McBSP smart idle and DMA op mode updates for ASoC

[Eduardo Valentin]

On Tue, May 18, 2010 at 10:13:10PM +0200, Liam Girdwood wrote:
> This series expands the OMAP mcbsp driver to support changing it's DMA operating
> mode and smart idle mode from client drivers. It's primarily aimed at lowering
> the power consumption for OMAP ASoC drivers by providing methods to gate clocks
> on the mcbsp interface at runtime.
> 
> I've also added a patch to remove the mcbsp DMA op mode sysfs set functionality.
> I think DMA op mode is very specific to the mcbsp client driver _only_ and
> shouldn't really be changed by userspace. Please let me know if you use this
> feature and I'll drop this patch.

Yeah, I'm not sure if that would be strong enough argument though. Unfortunately,
this dma op mode also couples with the usage of mcbsp internal buffer. Which in the very
end will mean a compromise between delay vs. pm, as you are probably aware.

So, letting userspace to toggle this mode will also mean they can choose between pm friendly
(op mode with frame and so on) or short delay friendly (element mode).

Of course, this might not be the best way of choosing it though.

> 
> Tony, can I get your ack to upstream via ALSA if it's ok
> 
> Thanks
> 
> Liam
> 
>  arch/arm/plat-omap/include/plat/mcbsp.h |   17 +++
>  arch/arm/plat-omap/mcbsp.c              |  183 ++++++++++++++++++++++++++-----
>  sound/soc/omap/omap-mcbsp.c             |   22 ++++-
>  sound/soc/omap/omap-mcbsp.h             |    2 +
>  4 files changed, 194 insertions(+), 30 deletions(-)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] OMAP: mcbsp - add smart idle configuration API

[Liam Girdwood]

On Tue, 2010-05-18 at 16:01 -0700, Kevin Hilman wrote:
> Liam Girdwood <lrg@slimlogic.co.uk> writes:
> 
> > Add a small API to configure McBSP smart idle modes
> > to conserve power.
> 
> Would be useful here to explain why client drivers need to change
> idle modes.
> 

I think Peter has covered this in one of his replies.

> Also, this patch is telling me that it's time the McBSP is converted
> over to the omap_hwmod/omap_device infrastructure where this type
> of functionality would already be provided.
> 
> Not necesssarily a reason to prevent this patch for now, but this
> will all need to be changed when converted to hwmod.

Ok, I think I'll actually just convert it to hwmod rather than adding a
soon to be deprecated API.

Thanks

Liam
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 2/4] OMAP: mcbsp - add smart idle configuration API

[Eduardo Valentin]

Hello Lian,

On Tue, May 18, 2010 at 10:13:12PM +0200, Liam Girdwood wrote:
> Add a small API to configure McBSP smart idle modes
> to conserve power.

I'm sorry but I didn't get the point of this patch, as you didn't add any wider
description why you need to export this feature. Why do you think mcbsp clients
should be aware of these bit? Would it make sense to mask it behind other feature?
like the threshold size for instance..

> 
> Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
> ---
>  arch/arm/plat-omap/include/plat/mcbsp.h |   15 ++++
>  arch/arm/plat-omap/mcbsp.c              |  122 +++++++++++++++++++++++++++++++
>  2 files changed, 137 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/plat/mcbsp.h b/arch/arm/plat-omap/include/plat/mcbsp.h
> index f8823f4..3f9fb71 100644
> --- a/arch/arm/plat-omap/include/plat/mcbsp.h
> +++ b/arch/arm/plat-omap/include/plat/mcbsp.h
> @@ -278,6 +278,15 @@
>  #define ENAWAKEUP		0x0004
>  #define SOFTRST			0x0002
>  
> +#define MCBSP_CLK_ACT_IOFF_POFF		0
> +#define MCBSP_CLK_ACT_ION_POFF		1
> +#define MCBSP_CLK_ACT_IOFF_PON		2
> +#define MCBSP_CLK_ACT_ION_PON			3
> +
> +#define MCBSP_IDLE_FORCE	0
> +#define MCBSP_IDLE_NONE		1
> +#define MCBSP_IDLE_SMART	2
> +
>  /********************** McBSP SSELCR bit definitions ***********************/
>  #define SIDETONEEN		0x0400
>  
> @@ -456,6 +465,7 @@ struct omap_mcbsp {
>  #ifdef CONFIG_ARCH_OMAP3
>  	struct omap_mcbsp_st_data *st_data;
>  	int dma_op_mode;
> +	int idle_mode;
>  	u16 max_tx_thres;
>  	u16 max_rx_thres;
>  #endif
> @@ -477,6 +487,11 @@ u16 omap_mcbsp_get_tx_delay(unsigned int id);
>  u16 omap_mcbsp_get_rx_delay(unsigned int id);
>  int omap_mcbsp_get_dma_op_mode(unsigned int id);
>  int omap_mcbsp_set_dma_op_mode(unsigned int id, unsigned int mode);
> +int omap_mcbsp_set_idle_smart(unsigned int id, unsigned int clk_activity,
> +		unsigned int wake);
> +int omap_mcbsp_set_idle_none(unsigned int id);
> +int omap_mcbsp_set_idle_force(unsigned int id);
> +int omap_mcbsp_get_idle_mode(unsigned int id);
>  #else
>  static inline void omap_mcbsp_set_tx_threshold(unsigned int id, u16 threshold)
>  { }
> diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
> index cc2b73c..7785050 100644
> --- a/arch/arm/plat-omap/mcbsp.c
> +++ b/arch/arm/plat-omap/mcbsp.c
> @@ -1743,6 +1743,128 @@ static inline void __devexit omap34xx_device_exit(struct omap_mcbsp *mcbsp)
>  			omap_st_remove(mcbsp);
>  	}
>  }
> +
> +/* assert standby requests when idle */
> +int omap_mcbsp_set_idle_smart(unsigned int id, unsigned int clk_activity,
> +		u32 wakeup)
> +{
> +	struct omap_mcbsp *mcbsp;
> +	u16 syscon;
> +	int ret = 0;
> +
> +	if (!omap_mcbsp_check_valid_id(id)) {
> +		printk(KERN_ERR "%s: Invalid id (%u)\n", __func__, id + 1);
> +		return -ENODEV;
> +	}
> +	mcbsp = id_to_mcbsp_ptr(id);
> +
> +	spin_lock_irq(&mcbsp->lock);
> +	if (!mcbsp->free) {
> +		ret = -EBUSY;
> +		goto unlock;
> +	}
> +
> +	syscon = MCBSP_READ(mcbsp, SYSCON) &
> +			~(ENAWAKEUP | SIDLEMODE(0x03) | CLOCKACTIVITY(0x03));
> +	MCBSP_WRITE(mcbsp, WAKEUPEN, wakeup);
> +	MCBSP_WRITE(mcbsp, SYSCON,
> +			syscon | SIDLEMODE(MCBSP_IDLE_SMART) |
> +			CLOCKACTIVITY(clk_activity) | ENAWAKEUP);
> +	mcbsp->idle_mode = MCBSP_IDLE_SMART;
> +
> +unlock:
> +	spin_unlock_irq(&mcbsp->lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL(omap_mcbsp_set_idle_smart);
> +
> +/* never assert standby requests */
> +int omap_mcbsp_set_idle_none(unsigned int id)
> +{
> +	struct omap_mcbsp *mcbsp;
> +	u16 syscon;
> +	int ret = 0;
> +
> +	if (!omap_mcbsp_check_valid_id(id)) {
> +		printk(KERN_ERR "%s: Invalid id (%u)\n", __func__, id + 1);
> +		return -ENODEV;
> +	}
> +	mcbsp = id_to_mcbsp_ptr(id);
> +
> +	spin_lock_irq(&mcbsp->lock);
> +	if (!mcbsp->free) {
> +		ret = -EBUSY;
> +		goto unlock;
> +	}
> +
> +	syscon = MCBSP_READ(mcbsp, SYSCON) &
> +			~(ENAWAKEUP | SIDLEMODE(0x03) | CLOCKACTIVITY(0x03));
> +
> +	MCBSP_WRITE(mcbsp, SYSCON, syscon | SIDLEMODE(MCBSP_IDLE_NONE));
> +	MCBSP_WRITE(mcbsp, WAKEUPEN, 0);
> +	mcbsp->idle_mode = MCBSP_IDLE_NONE;
> +
> +unlock:
> +	spin_unlock_irq(&mcbsp->lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL(omap_mcbsp_set_idle_none);
> +
> +/* unconditionally assert standby requests */
> +int omap_mcbsp_set_idle_force(unsigned int id)
> +{
> +	struct omap_mcbsp *mcbsp;
> +	u16 syscon;
> +	int ret = 0;
> +
> +	if (!omap_mcbsp_check_valid_id(id)) {
> +		printk(KERN_ERR "%s: Invalid id (%u)\n", __func__, id + 1);
> +		return -ENODEV;
> +	}
> +	mcbsp = id_to_mcbsp_ptr(id);
> +
> +	spin_lock_irq(&mcbsp->lock);
> +	if (!mcbsp->free) {
> +		ret = -EBUSY;
> +		goto unlock;
> +	}
> +
> +	syscon = MCBSP_READ(mcbsp, SYSCON) &
> +			~(ENAWAKEUP | SIDLEMODE(0x03) | CLOCKACTIVITY(0x03));
> +	/*
> +	 * HW bug workaround - If no_idle mode is taken, we need to
> +	 * go to smart_idle before going to always_idle, or the
> +	 * device will not hit retention anymore.
> +	 */
> +	syscon |= SIDLEMODE(MCBSP_IDLE_SMART);
> +	MCBSP_WRITE(mcbsp, SYSCON, syscon);
> +	syscon &= ~(SIDLEMODE(0x03));
> +
> +	MCBSP_WRITE(mcbsp, SYSCON, syscon | SIDLEMODE(MCBSP_IDLE_FORCE));
> +	MCBSP_WRITE(mcbsp, WAKEUPEN, 0);
> +	mcbsp->idle_mode = MCBSP_IDLE_FORCE;
> +
> +unlock:
> +	spin_unlock_irq(&mcbsp->lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL(omap_mcbsp_set_idle_force);
> +
> +int omap_mcbsp_get_idle_mode(unsigned int id)
> +{
> +	struct omap_mcbsp *mcbsp;
> +
> +	if (!omap_mcbsp_check_valid_id(id)) {
> +		printk(KERN_ERR "%s: Invalid id (%u)\n", __func__, id + 1);
> +		return -ENODEV;
> +	}
> +	mcbsp = id_to_mcbsp_ptr(id);
> +
> +	return mcbsp->idle_mode;
> +}
> +EXPORT_SYMBOL(omap_mcbsp_get_idle_mode);
> +
> +
>  #else
>  static inline void __devinit omap34xx_device_init(struct omap_mcbsp *mcbsp) {}
>  static inline void __devexit omap34xx_device_exit(struct omap_mcbsp *mcbsp) {}
> -- 
> 1.7.0.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] OMAP: mcbsp - add omap_mcbsp_set_dma_op_mode()

[Eduardo Valentin]

On Tue, May 18, 2010 at 10:13:11PM +0200, Liam Girdwood wrote:
> This adds a method to set the MCBSP DMA OP mode.

OK. good. But again, why do you need set dma op mode other than inside mcbsp code?

Right, so, from what I have read briefly, the idea is to remove the user space ability to set the mode and
let mcbsp clients to do it. What is not clear to me is how mcbsp clients (inside kernel) will determine if they want
short delay or pm friendly. That usually is bound to upper use case though.

It would be really nice if you improve your patch descriptions.

> 
> Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
> ---
>  arch/arm/plat-omap/include/plat/mcbsp.h |    2 ++
>  arch/arm/plat-omap/mcbsp.c              |   31 +++++++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/plat/mcbsp.h b/arch/arm/plat-omap/include/plat/mcbsp.h
> index 1bd7021..f8823f4 100644
> --- a/arch/arm/plat-omap/include/plat/mcbsp.h
> +++ b/arch/arm/plat-omap/include/plat/mcbsp.h
> @@ -476,6 +476,7 @@ u16 omap_mcbsp_get_max_rx_threshold(unsigned int id);
>  u16 omap_mcbsp_get_tx_delay(unsigned int id);
>  u16 omap_mcbsp_get_rx_delay(unsigned int id);
>  int omap_mcbsp_get_dma_op_mode(unsigned int id);
> +int omap_mcbsp_set_dma_op_mode(unsigned int id, unsigned int mode);
>  #else
>  static inline void omap_mcbsp_set_tx_threshold(unsigned int id, u16 threshold)
>  { }
> @@ -486,6 +487,7 @@ static inline u16 omap_mcbsp_get_max_rx_threshold(unsigned int id) { return 0; }
>  static inline u16 omap_mcbsp_get_tx_delay(unsigned int id) { return 0; }
>  static inline u16 omap_mcbsp_get_rx_delay(unsigned int id) { return 0; }
>  static inline int omap_mcbsp_get_dma_op_mode(unsigned int id) { return 0; }
> +static inline int omap_mcbsp_set_dma_op_mode(unsigned int id, unsigned int mode) {return 0;}
>  #endif
>  int omap_mcbsp_request(unsigned int id);
>  void omap_mcbsp_free(unsigned int id);
> diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
> index 4820cab..cc2b73c 100644
> --- a/arch/arm/plat-omap/mcbsp.c
> +++ b/arch/arm/plat-omap/mcbsp.c
> @@ -635,6 +635,37 @@ int omap_mcbsp_get_dma_op_mode(unsigned int id)
>  }
>  EXPORT_SYMBOL(omap_mcbsp_get_dma_op_mode);
>  
> +/*
> + * omap_mcbsp_set_dma_op_mode set the current DMA
> + * operating mode for the mcbsp channel
> + */
> +int omap_mcbsp_set_dma_op_mode(unsigned int id, unsigned int mode)
> +{
> +	struct omap_mcbsp *mcbsp;
> +	int ret = 0;
> +
> +	if (mode > MCBSP_DMA_MODE_FRAME)
> +		return -EINVAL;
> +
> +	if (!omap_mcbsp_check_valid_id(id)) {
> +		printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
> +		return -ENODEV;
> +	}
> +	mcbsp = id_to_mcbsp_ptr(id);
> +
> +	spin_lock_irq(&mcbsp->lock);
> +	if (!mcbsp->free) {
> +		ret = -EBUSY;
> +		goto unlock;
> +	}
> +	mcbsp->dma_op_mode = mode;
> +
> +unlock:
> +	spin_unlock_irq(&mcbsp->lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL(omap_mcbsp_set_dma_op_mode);
> +
>  static inline void omap34xx_mcbsp_request(struct omap_mcbsp *mcbsp)
>  {
>  	/*
> -- 
> 1.7.0.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] McBSP smart idle and DMA op mode updates for ASoC

[Eduardo Valentin]

On Wed, May 19, 2010 at 10:31:57AM +0200, Ujfalusi Peter (Nokia-D/Tampere) wrote:
> On Wednesday 19 May 2010 08:13:25 ext Jarkko Nikula wrote:
> > On Tue, 18 May 2010 21:13:10 +0100
> > 
> > Liam Girdwood <lrg@slimlogic.co.uk> wrote:
> > > I've also added a patch to remove the mcbsp DMA op mode sysfs set
> > > functionality. I think DMA op mode is very specific to the mcbsp client
> > > driver _only_ and shouldn't really be changed by userspace. Please let
> > > me know if you use this feature and I'll drop this patch.
> > 
> > I've used to say that the DMA op mode is more like use-case not machine
> > specific but I'm not sure is my point valid onemore. I've used to think
> > that low-latency processing would need accurate DMA pointer (op
> > mode == element) while mp3 playback would need low power consumption
> > (op mode == threshold).
> 
> Yeah, it used to be so clear ;)
> I see these:
> element mode: if user want to have constant latency [1]
> threshold mode: variable latency, but possibility to save power [2]
> 
> [1] The McBSP is kept full during playback (and empty during capture)
> The DMA pointer moves word-by-word
> 
> [2] The McBSP FIFO fill rate changes (full, drain, refill, full, drain...)
> The DMA pointer moves in bursts.
> Between burst memories can relax, core, MPU also in theory.
> Smart idle helps to conserve more power here
> 
> > Peter: what's the status today how well can we do low-latency
> > processing with threshold mode? IRCC, with your FIFO delay query
> > patches, can we estimate the DMA pointer position with enough accuracy?
> 
> The DMA pointer is easy, and it was know before as well, but according to my 
> tests, the McBSP FIFO caused delay reporting is fairly accurate.
> I'll ask my users, if they have done some additional tests.

OK. fair enough. What if you really want to avoid the delay at all?

> 
> -- 
> Péter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] McBSP smart idle and DMA op mode updates for ASoC

[Liam Girdwood]

On Wed, 2010-05-19 at 13:42 +0300, Eduardo Valentin wrote:
> On Tue, May 18, 2010 at 10:13:10PM +0200, Liam Girdwood wrote:
> > This series expands the OMAP mcbsp driver to support changing it's DMA operating
> > mode and smart idle mode from client drivers. It's primarily aimed at lowering
> > the power consumption for OMAP ASoC drivers by providing methods to gate clocks
> > on the mcbsp interface at runtime.
> > 
> > I've also added a patch to remove the mcbsp DMA op mode sysfs set functionality.
> > I think DMA op mode is very specific to the mcbsp client driver _only_ and
> > shouldn't really be changed by userspace. Please let me know if you use this
> > feature and I'll drop this patch.
> 
> Yeah, I'm not sure if that would be strong enough argument though. Unfortunately,
> this dma op mode also couples with the usage of mcbsp internal buffer. Which in the very
> end will mean a compromise between delay vs. pm, as you are probably aware.
> 
> So, letting userspace to toggle this mode will also mean they can choose between pm friendly
> (op mode with frame and so on) or short delay friendly (element mode).
> 
> Of course, this might not be the best way of choosing it though.
> 

The sysfs set interface implies userspace having knowledge of driver
capabilities and configuration in order to safely toggle between the two
DMA modes. Imo, the mcbsp client driver should be the only entity
configuring it's DMA modes (in a safe manner) depending on the use case.

Liam
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [alsa-devel] [PATCH 2/4] OMAP: mcbsp - add smart idle configuration API

[Liam Girdwood]

On Wed, 2010-05-19 at 13:46 +0300, Eduardo Valentin wrote:
> Hello Lian,
> 
> On Tue, May 18, 2010 at 10:13:12PM +0200, Liam Girdwood wrote:
> > Add a small API to configure McBSP smart idle modes
> > to conserve power.
> 
> I'm sorry but I didn't get the point of this patch, as you didn't add any wider
> description why you need to export this feature. 

Sorry, I thought this would obvious from the patch and description.

> Why do you think mcbsp clients
> should be aware of these bit? Would it make sense to mask it behind other feature?
> like the threshold size for instance..

Why do you think it should be masked behind threshold ? 

Liam
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 0/4] McBSP smart idle and DMA op mode updates for ASoC

[Eduardo Valentin]

On Wed, May 19, 2010 at 01:07:11PM +0200, Liam Girdwood wrote:
> On Wed, 2010-05-19 at 13:42 +0300, Eduardo Valentin wrote:
> > On Tue, May 18, 2010 at 10:13:10PM +0200, Liam Girdwood wrote:
> > > This series expands the OMAP mcbsp driver to support changing it's DMA operating
> > > mode and smart idle mode from client drivers. It's primarily aimed at lowering
> > > the power consumption for OMAP ASoC drivers by providing methods to gate clocks
> > > on the mcbsp interface at runtime.
> > > 
> > > I've also added a patch to remove the mcbsp DMA op mode sysfs set functionality.
> > > I think DMA op mode is very specific to the mcbsp client driver _only_ and
> > > shouldn't really be changed by userspace. Please let me know if you use this
> > > feature and I'll drop this patch.
> > 
> > Yeah, I'm not sure if that would be strong enough argument though. Unfortunately,
> > this dma op mode also couples with the usage of mcbsp internal buffer. Which in the very
> > end will mean a compromise between delay vs. pm, as you are probably aware.
> > 
> > So, letting userspace to toggle this mode will also mean they can choose between pm friendly
> > (op mode with frame and so on) or short delay friendly (element mode).
> > 
> > Of course, this might not be the best way of choosing it though.
> > 
> 
> The sysfs set interface implies userspace having knowledge of driver
> capabilities and configuration in order to safely toggle between the two
> DMA modes. Imo, the mcbsp client driver should be the only entity
> configuring it's DMA modes (in a safe manner) depending on the use case.


OK. That is fair enough. But it isn't clear in your patch series/description, how the client will
determine which use case to use. I understand your point and I agree with it. However, the way
it's being done here, it does not explain how the same issue will be handled after the removal of this
interface. That's my main concern.

> 
> Liam
> -- 
> Freelance Developer, SlimLogic Ltd
> ASoC and Voltage Regulator Maintainer.
> http://www.slimlogic.co.uk
> 

Re: [PATCH 1/4] OMAP: mcbsp - add omap_mcbsp_set_dma_op_mode()

[Liam Girdwood]

On Wed, 2010-05-19 at 13:50 +0300, Eduardo Valentin wrote:
> On Tue, May 18, 2010 at 10:13:11PM +0200, Liam Girdwood wrote:
> > This adds a method to set the MCBSP DMA OP mode.
> 
> OK. good. But again, why do you need set dma op mode other than inside mcbsp code?
> 

I think Peter has answered this.

> Right, so, from what I have read briefly, the idea is to remove the user space ability to set the mode and
> let mcbsp clients to do it. What is not clear to me is how mcbsp clients (inside kernel) will determine if they want
> short delay or pm friendly. That usually is bound to upper use case though.
> 

An example would be an ASoC client. The ASoC driver would be configured
either for low latency, low power, etc. by userspace (using formal ALSA
calls) and would be able to configure the mcbsp to match this request.  


> It would be really nice if you improve your patch descriptions.

Sorry, I thought again this was obvious.

Liam
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 2/4] OMAP: mcbsp - add smart idle configuration API

[Eduardo Valentin]

On Wed, May 19, 2010 at 01:21:26PM +0200, Liam Girdwood wrote:
> On Wed, 2010-05-19 at 13:46 +0300, Eduardo Valentin wrote:
> > Hello Lian,
> > 
> > On Tue, May 18, 2010 at 10:13:12PM +0200, Liam Girdwood wrote:
> > > Add a small API to configure McBSP smart idle modes
> > > to conserve power.
> > 
> > I'm sorry but I didn't get the point of this patch, as you didn't add any wider
> > description why you need to export this feature. 
> 
> Sorry, I thought this would obvious from the patch and description.
> 
> > Why do you think mcbsp clients
> > should be aware of these bit? Would it make sense to mask it behind other feature?
> > like the threshold size for instance..
> 
> Why do you think it should be masked behind threshold ? 

Well, that was just an example. It could be masked there because, if client doesn't set the
threshold level, you could assume you don't need to set the smart idle.

Or maybe it could be bound to dma op mode. if element mode, then no need to smart idle.
I guess that was the way it was, at some point.

> 
> Liam
> -- 
> Freelance Developer, SlimLogic Ltd
> ASoC and Voltage Regulator Maintainer.
> http://www.slimlogic.co.uk
> 

Re: [PATCH 0/4] McBSP smart idle and DMA op mode updates for ASoC

[Peter Ujfalusi]

On Wednesday 19 May 2010 13:52:27 Valentin Eduardo (Nokia-D/Helsinki) wrote:
> > The DMA pointer is easy, and it was know before as well, but according to
> > my tests, the McBSP FIFO caused delay reporting is fairly accurate. I'll
> > ask my users, if they have done some additional tests.
> 
> OK. fair enough. What if you really want to avoid the delay at all?

You can't. the McBSP FIFO can not be bypassed.
The only thing one can do is to take it into consideration.

-- 
Péter

Re: [PATCH 0/4] McBSP smart idle and DMA op mode updates for ASoC

[Eduardo Valentin]

On Wed, May 19, 2010 at 01:36:51PM +0200, Ujfalusi Peter (Nokia-D/Tampere) wrote:
> On Wednesday 19 May 2010 13:52:27 Valentin Eduardo (Nokia-D/Helsinki) wrote:
> > > The DMA pointer is easy, and it was know before as well, but according to
> > > my tests, the McBSP FIFO caused delay reporting is fairly accurate. I'll
> > > ask my users, if they have done some additional tests.
> > 
> > OK. fair enough. What if you really want to avoid the delay at all?
> 
> You can't. the McBSP FIFO can not be bypassed.
> The only thing one can do is to take it into consideration.


hmmm well, you can still do not use the fifo completely. Use only partially depending
on latency requirement would still be possible.

> 
> -- 
> Péter

Re: [PATCH 0/4] McBSP smart idle and DMA op mode updates for ASoC

[Peter Ujfalusi]

On Wednesday 19 May 2010 14:46:40 Valentin Eduardo (Nokia-D/Helsinki) wrote:
> On Wed, May 19, 2010 at 01:36:51PM +0200, Ujfalusi Peter (Nokia-D/Tampere) 
wrote:
> > On Wednesday 19 May 2010 13:52:27 Valentin Eduardo (Nokia-D/Helsinki) wrote:
> > > > The DMA pointer is easy, and it was know before as well, but
> > > > according to my tests, the McBSP FIFO caused delay reporting is
> > > > fairly accurate. I'll ask my users, if they have done some
> > > > additional tests.
> > > 
> > > OK. fair enough. What if you really want to avoid the delay at all?
> > 
> > You can't. the McBSP FIFO can not be bypassed.
> > The only thing one can do is to take it into consideration.
> 
> hmmm well, you can still do not use the fifo completely. Use only partially
> depending on latency requirement would still be possible.

Ah, you mean the idea, where in DMA completion interrupt we configure the McBSP 
threshold to maximum, and on McBSP FIFO empty interrupt, we configurre back the 
threshold to what the level we want to use from McBSP, and start the DMA?
This all has to be done 'manually', and if there is _any_ latency in the kernel, 
than we have channel switch, glitch, pause, and whatnot..
I remember, we tried that, and well, did not worked that reliable.

-- 
Péter

Re: [PATCH 2/4] OMAP: mcbsp - add smart idle configuration API

[Peter Ujfalusi]

On Wednesday 19 May 2010 13:43:28 ext Liam Girdwood wrote:
> On Tue, 2010-05-18 at 16:01 -0700, Kevin Hilman wrote:
> > Liam Girdwood <lrg@slimlogic.co.uk> writes:
> > > Add a small API to configure McBSP smart idle modes
> > > to conserve power.
> > 
> > Would be useful here to explain why client drivers need to change
> > idle modes.
> 
> I think Peter has covered this in one of his replies.
> 
> > Also, this patch is telling me that it's time the McBSP is converted
> > over to the omap_hwmod/omap_device infrastructure where this type
> > of functionality would already be provided.
> > 
> > Not necesssarily a reason to prevent this patch for now, but this
> > will all need to be changed when converted to hwmod.
> 
> Ok, I think I'll actually just convert it to hwmod rather than adding a
> soon to be deprecated API.

Does the hwmon supported on OMAP1, 2, 3, and 4? Do we have the needed framework 
covering all OMAP versions?

> 
> Thanks
> 
> Liam

-- 
Péter
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 0/4] McBSP smart idle and DMA op mode updates for ASoC

[Peter Ujfalusi]

On Wednesday 19 May 2010 14:07:11 ext Liam Girdwood wrote:
> On Wed, 2010-05-19 at 13:42 +0300, Eduardo Valentin wrote:
> > On Tue, May 18, 2010 at 10:13:10PM +0200, Liam Girdwood wrote:
> > > This series expands the OMAP mcbsp driver to support changing it's DMA
> > > operating mode and smart idle mode from client drivers. It's primarily
> > > aimed at lowering the power consumption for OMAP ASoC drivers by
> > > providing methods to gate clocks on the mcbsp interface at runtime.
> > > 
> > > I've also added a patch to remove the mcbsp DMA op mode sysfs set
> > > functionality. I think DMA op mode is very specific to the mcbsp
> > > client driver _only_ and shouldn't really be changed by userspace.
> > > Please let me know if you use this feature and I'll drop this patch.
> > 
> > Yeah, I'm not sure if that would be strong enough argument though.
> > Unfortunately, this dma op mode also couples with the usage of mcbsp
> > internal buffer. Which in the very end will mean a compromise between
> > delay vs. pm, as you are probably aware.
> > 
> > So, letting userspace to toggle this mode will also mean they can choose
> > between pm friendly (op mode with frame and so on) or short delay
> > friendly (element mode).
> > 
> > Of course, this might not be the best way of choosing it though.
> 
> The sysfs set interface implies userspace having knowledge of driver
> capabilities and configuration in order to safely toggle between the two
> DMA modes. Imo, the mcbsp client driver should be the only entity
> configuring it's DMA modes (in a safe manner) depending on the use case.

Furthermore, if there is a need for 'Use Cases', than the machine driver can 
provide user control to switch between them.
The thing is that in most cases these are trivial, and mostly the same settings, 
but if you throw a codec like the tlv320dac33 into the mix, which has it's own 
FIFO, than things gets complicated.
The user (the real one, not the developer) has several settings scattered all 
around the place, and those has to be configured in harmony.
The only place is to do this, is in the machine driver, whihc than can build up 
'scenarios', and configure the things in synchronized manner.

> 
> Liam

-- 
Péter
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 0/4] McBSP smart idle and DMA op mode updates for ASoC

[Jarkko Nikula]

On Wed, 19 May 2010 15:27:35 +0300
Peter Ujfalusi <peter.ujfalusi@nokia.com> wrote:

> > The sysfs set interface implies userspace having knowledge of driver
> > capabilities and configuration in order to safely toggle between the two
> > DMA modes. Imo, the mcbsp client driver should be the only entity
> > configuring it's DMA modes (in a safe manner) depending on the use case.
> 
> Furthermore, if there is a need for 'Use Cases', than the machine driver can 
> provide user control to switch between them.
> The thing is that in most cases these are trivial, and mostly the same settings, 
> but if you throw a codec like the tlv320dac33 into the mix, which has it's own 
> FIFO, than things gets complicated.
> The user (the real one, not the developer) has several settings scattered all 
> around the place, and those has to be configured in harmony.
> The only place is to do this, is in the machine driver, whihc than can build up 
> 'scenarios', and configure the things in synchronized manner.
> 
Yes and I think only very few developers know what to do with those op
mode and threshold sysfs controls so most probably they are unused.
Then machine drivers setting them automatically/with some control could
give us more testing base.

But as those sysfs controls are there we must preserve them for a
release cycle or two in case if someone is using them. At least Linus
or Andrew may complain about removal of them.


-- 
Jarkko

Re: [PATCH 2/4] OMAP: mcbsp - add smart idle configuration API

[Kevin Hilman]

Liam Girdwood <lrg@slimlogic.co.uk> writes:

> On Tue, 2010-05-18 at 16:01 -0700, Kevin Hilman wrote:
>> Liam Girdwood <lrg@slimlogic.co.uk> writes:
>> 
>> > Add a small API to configure McBSP smart idle modes
>> > to conserve power.
>> 
>> Would be useful here to explain why client drivers need to change
>> idle modes.
>> 
>
> I think Peter has covered this in one of his replies.
>
>> Also, this patch is telling me that it's time the McBSP is converted
>> over to the omap_hwmod/omap_device infrastructure where this type
>> of functionality would already be provided.
>> 
>> Not necesssarily a reason to prevent this patch for now, but this
>> will all need to be changed when converted to hwmod.
>
> Ok, I think I'll actually just convert it to hwmod rather than adding a
> soon to be deprecated API.

Excellent, that would certainly be my preference.

For reference, you can check out the pm-wip/uart and pm-wip/mmc
branches of my tree[1] for some hwmod conversions for UART and MMC
that are in progress.

Kevin

[1] git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git