[PATCH v5 0/1] mfd: arizona: Export functions to control subsystem DVFS

[PATCH v5 0/1] mfd: arizona: Export functions to control subsystem DVFS

[Richard Fitzgerald]

V4 of this patch was acked by Lee but this new version is significantly
different from V4 so I've removed the ack.
Main changes since V4 are around better control of the DVFS across
suspend/resume, and added handling for the WM8998 and WM1814 codecs.

Richard Fitzgerald (1):
  mfd: arizona: Export functions to control subsystem DVFS

 drivers/mfd/arizona-core.c       |  172 +++++++++++++++++++++++++++++++++++++-
 include/linux/mfd/arizona/core.h |   13 +++
 2 files changed, 183 insertions(+), 2 deletions(-)

-- 
1.7.2.5

[PATCH v5 1/1] mfd: arizona: Export functions to control subsystem DVFS

[Richard Fitzgerald]

The WM5102, WM8997, WM8998 and WM1814 codecs have an internal dynamic
clock booster. When this booster is active, the DCVDD voltage must be
increased. If all the currently active audio paths can run with the root
SYSCLK we can disable the booster, allowing us to turn down DCVDD voltage
to save power.

Previously this was being done by having the booster enable bit set
as a side-effect of the LDO1 regulator driver, which is unexpected
behaviour of a regulator and not compatible with using an external
regulator.

This patch exports functions to handle the booster enable and
DCVDD voltage, with each relevant subsystem flagging whether it can
currently run without the booster. Note that these subsystems are
stateless and none of them are nestable, so there's no need for
reference counting, we only need a simple boolean for each subsystem
of whether their current condition could require the booster or will
allow us to turn the codec down to lower operating power.

Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
---
 drivers/mfd/arizona-core.c       |  172 +++++++++++++++++++++++++++++++++++++-
 include/linux/mfd/arizona/core.h |   13 +++
 2 files changed, 183 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
index 16828cc..0586624 100644
--- a/drivers/mfd/arizona-core.c
+++ b/drivers/mfd/arizona-core.c
@@ -94,6 +94,125 @@ int arizona_clk32k_disable(struct arizona *arizona)
 }
 EXPORT_SYMBOL_GPL(arizona_clk32k_disable);
 
+static int arizona_dvfs_apply_boost(struct arizona *arizona)
+{
+	int ret;
+
+	ret = regulator_set_voltage(arizona->dcvdd, 1800000, 1800000);
+	if (ret != 0) {
+		dev_err(arizona->dev,
+			"Failed to boost DCVDD: %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_update_bits(arizona->regmap,
+				ARIZONA_DYNAMIC_FREQUENCY_SCALING_1,
+				ARIZONA_SUBSYS_MAX_FREQ, 1);
+	if (ret != 0) {
+		dev_err(arizona->dev,
+			"Failed to enable subsys max: %d\n", ret);
+
+		regulator_set_voltage(arizona->dcvdd, 1200000, 1800000);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int arizona_dvfs_remove_boost(struct arizona *arizona)
+{
+	int ret;
+
+	ret = regmap_update_bits(arizona->regmap,
+				ARIZONA_DYNAMIC_FREQUENCY_SCALING_1,
+				ARIZONA_SUBSYS_MAX_FREQ, 0);
+	if (ret != 0) {
+		dev_err(arizona->dev,
+			"Failed to disable subsys max: %d\n", ret);
+		return ret;
+	}
+
+	ret = regulator_set_voltage(arizona->dcvdd, 1200000, 1800000);
+	if (ret != 0) {
+		dev_err(arizona->dev,
+			"Failed to unboost DCVDD : %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+int arizona_dvfs_up(struct arizona *arizona, unsigned int flags)
+{
+	unsigned int old_flags;
+	int ret = 0;
+
+	mutex_lock(&arizona->subsys_max_lock);
+
+	old_flags = arizona->subsys_max_rq;
+	arizona->subsys_max_rq |= flags;
+
+	/* If currently caching the change will be applied in runtime resume */
+	if (arizona->subsys_max_cached) {
+		dev_dbg(arizona->dev, "subsys_max_cached (dvfs up)\n");
+		goto out;
+	}
+
+	if (arizona->subsys_max_rq != old_flags) {
+		switch (arizona->type) {
+		case WM5102:
+		case WM8997:
+		case WM8998:
+		case WM1814:
+			ret = arizona_dvfs_apply_boost(arizona);
+			break;
+
+		default:
+			break;
+		}
+
+	}
+out:
+	mutex_unlock(&arizona->subsys_max_lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(arizona_dvfs_up);
+
+int arizona_dvfs_down(struct arizona *arizona, unsigned int flags)
+{
+	unsigned int old_flags;
+	int ret = 0;
+
+	mutex_lock(&arizona->subsys_max_lock);
+
+	old_flags = arizona->subsys_max_rq;
+	arizona->subsys_max_rq &= ~flags;
+
+	/* If currently caching the change will be applied in runtime resume */
+	if (arizona->subsys_max_cached) {
+		dev_dbg(arizona->dev, "subsys_max_cached (dvfs down)\n");
+		goto out;
+	}
+
+	if ((old_flags != 0) && (arizona->subsys_max_rq == 0)) {
+		switch (arizona->type) {
+		case WM5102:
+		case WM8997:
+		case WM8998:
+		case WM1814:
+			ret = arizona_dvfs_remove_boost(arizona);
+			break;
+
+		default:
+			break;
+		}
+	}
+out:
+	mutex_unlock(&arizona->subsys_max_lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(arizona_dvfs_down);
+
 static irqreturn_t arizona_clkgen_err(int irq, void *data)
 {
 	struct arizona *arizona = data;
@@ -462,6 +581,31 @@ static int wm5102_clear_write_sequencer(struct arizona *arizona)
 }
 
 #ifdef CONFIG_PM
+static int arizona_restore_dvfs(struct arizona *arizona)
+{
+	int ret;
+
+	switch (arizona->type) {
+	default:
+		return 0;	/* no DVFS */
+
+	case WM5102:
+	case WM8997:
+	case WM8998:
+	case WM1814:
+		ret = 0;
+		mutex_lock(&arizona->subsys_max_lock);
+		if (arizona->subsys_max_rq != 0) {
+			dev_dbg(arizona->dev, "Restore subsys_max boost\n");
+			ret = arizona_dvfs_apply_boost(arizona);
+		}
+
+		arizona->subsys_max_cached = false;
+		mutex_unlock(&arizona->subsys_max_lock);
+		return ret;
+	}
+}
+
 static int arizona_runtime_resume(struct device *dev)
 {
 	struct arizona *arizona = dev_get_drvdata(dev);
@@ -590,6 +734,10 @@ static int arizona_runtime_resume(struct device *dev)
 		goto err;
 	}
 
+	ret = arizona_restore_dvfs(arizona);
+	if (ret < 0)
+		goto err;
+
 	return 0;
 
 err:
@@ -612,6 +760,21 @@ static int arizona_runtime_suspend(struct device *dev)
 		return ret;
 	}
 
+	switch (arizona->type) {
+	case WM5102:
+	case WM8997:
+	case WM8998:
+	case WM1814:
+		/* Must disable DVFS boost before powering down DCVDD */
+		mutex_lock(&arizona->subsys_max_lock);
+		arizona->subsys_max_cached = true;
+		arizona_dvfs_remove_boost(arizona);
+		mutex_unlock(&arizona->subsys_max_lock);
+		break;
+	default:
+		break;
+	}
+
 	if (arizona->external_dcvdd) {
 		ret = regmap_update_bits(arizona->regmap,
 					 ARIZONA_ISOLATION_CONTROL,
@@ -620,7 +783,7 @@ static int arizona_runtime_suspend(struct device *dev)
 		if (ret != 0) {
 			dev_err(arizona->dev, "Failed to isolate DCVDD: %d\n",
 				ret);
-			return ret;
+			goto err;
 		}
 	}
 
@@ -639,7 +802,7 @@ static int arizona_runtime_suspend(struct device *dev)
 		if (ret < 0) {
 			dev_err(arizona->dev,
 				"Failed to set suspend voltage: %d\n", ret);
-			return ret;
+			goto err;
 		}
 		break;
 	case WM5102:
@@ -675,6 +838,10 @@ static int arizona_runtime_suspend(struct device *dev)
 	}
 
 	return 0;
+
+err:
+	arizona_restore_dvfs(arizona);
+	return ret;
 }
 #endif
 
@@ -933,6 +1100,7 @@ int arizona_dev_init(struct arizona *arizona)
 
 	dev_set_drvdata(arizona->dev, arizona);
 	mutex_init(&arizona->clk_lock);
+	mutex_init(&arizona->subsys_max_lock);
 
 	if (dev_get_platdata(arizona->dev))
 		memcpy(&arizona->pdata, dev_get_platdata(arizona->dev),
diff --git a/include/linux/mfd/arizona/core.h b/include/linux/mfd/arizona/core.h
index b34a625..f18e4ca 100644
--- a/include/linux/mfd/arizona/core.h
+++ b/include/linux/mfd/arizona/core.h
@@ -136,6 +136,10 @@ struct arizona {
 	struct mutex clk_lock;
 	int clk32k_ref;
 
+	struct mutex subsys_max_lock;
+	unsigned int subsys_max_rq;
+	bool subsys_max_cached;
+
 	bool ctrlif_error;
 
 	struct snd_soc_dapm_context *dapm;
@@ -148,8 +152,17 @@ struct arizona {
 	struct mutex dac_comp_lock;
 };
 
+#define ARIZONA_DVFS_SR1_RQ          0x00000001
+#define ARIZONA_DVFS_SR2_RQ          0x00000002
+#define ARIZONA_DVFS_SR3_RQ          0x00000004
+#define ARIZONA_DVFS_ASR1_RQ         0x00000010
+#define ARIZONA_DVFS_ASR2_RQ         0x00000020
+#define ARIZONA_DVFS_ADSP1_RQ        0x00010000
+
 int arizona_clk32k_enable(struct arizona *arizona);
 int arizona_clk32k_disable(struct arizona *arizona);
+int arizona_dvfs_up(struct arizona *arizona, unsigned int mask);
+int arizona_dvfs_down(struct arizona *arizona, unsigned int mask);
 
 int arizona_request_irq(struct arizona *arizona, int irq, char *name,
 			irq_handler_t handler, void *data);
-- 
1.7.2.5

Re: [PATCH v5 1/1] mfd: arizona: Export functions to control subsystem DVFS

[Charles Keepax]

On Mon, May 18, 2015 at 03:15:05PM +0100, Richard Fitzgerald wrote:
> The WM5102, WM8997, WM8998 and WM1814 codecs have an internal dynamic
> clock booster. When this booster is active, the DCVDD voltage must be
> increased. If all the currently active audio paths can run with the root
> SYSCLK we can disable the booster, allowing us to turn down DCVDD voltage
> to save power.
> 
> Previously this was being done by having the booster enable bit set
> as a side-effect of the LDO1 regulator driver, which is unexpected
> behaviour of a regulator and not compatible with using an external
> regulator.
> 
> This patch exports functions to handle the booster enable and
> DCVDD voltage, with each relevant subsystem flagging whether it can
> currently run without the booster. Note that these subsystems are
> stateless and none of them are nestable, so there's no need for
> reference counting, we only need a simple boolean for each subsystem
> of whether their current condition could require the booster or will
> allow us to turn the codec down to lower operating power.
> 
> Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
> ---
>  drivers/mfd/arizona-core.c       |  172 +++++++++++++++++++++++++++++++++++++-
>  include/linux/mfd/arizona/core.h |   13 +++
>  2 files changed, 183 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
> index 16828cc..0586624 100644
> --- a/drivers/mfd/arizona-core.c
> +++ b/drivers/mfd/arizona-core.c
> @@ -94,6 +94,125 @@ int arizona_clk32k_disable(struct arizona *arizona)
>  }
>  EXPORT_SYMBOL_GPL(arizona_clk32k_disable);
>  
> +static int arizona_dvfs_apply_boost(struct arizona *arizona)
> +{
> +	int ret;
> +
> +	ret = regulator_set_voltage(arizona->dcvdd, 1800000, 1800000);
> +	if (ret != 0) {
> +		dev_err(arizona->dev,
> +			"Failed to boost DCVDD: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regmap_update_bits(arizona->regmap,
> +				ARIZONA_DYNAMIC_FREQUENCY_SCALING_1,
> +				ARIZONA_SUBSYS_MAX_FREQ, 1);
> +	if (ret != 0) {
> +		dev_err(arizona->dev,
> +			"Failed to enable subsys max: %d\n", ret);
> +
> +		regulator_set_voltage(arizona->dcvdd, 1200000, 1800000);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int arizona_dvfs_remove_boost(struct arizona *arizona)
> +{
> +	int ret;
> +
> +	ret = regmap_update_bits(arizona->regmap,
> +				ARIZONA_DYNAMIC_FREQUENCY_SCALING_1,
> +				ARIZONA_SUBSYS_MAX_FREQ, 0);
> +	if (ret != 0) {
> +		dev_err(arizona->dev,
> +			"Failed to disable subsys max: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regulator_set_voltage(arizona->dcvdd, 1200000, 1800000);
> +	if (ret != 0) {
> +		dev_err(arizona->dev,
> +			"Failed to unboost DCVDD : %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +int arizona_dvfs_up(struct arizona *arizona, unsigned int flags)
> +{
> +	unsigned int old_flags;
> +	int ret = 0;
> +
> +	mutex_lock(&arizona->subsys_max_lock);
> +
> +	old_flags = arizona->subsys_max_rq;
> +	arizona->subsys_max_rq |= flags;
> +
> +	/* If currently caching the change will be applied in runtime resume */
> +	if (arizona->subsys_max_cached) {
> +		dev_dbg(arizona->dev, "subsys_max_cached (dvfs up)\n");
> +		goto out;
> +	}
> +
> +	if (arizona->subsys_max_rq != old_flags) {

Don't we want a similar check here to dvfs_down? Might as well
skip apply_boost if we can.

> +		switch (arizona->type) {
> +		case WM5102:
> +		case WM8997:
> +		case WM8998:
> +		case WM1814:
> +			ret = arizona_dvfs_apply_boost(arizona);
> +			break;
> +
> +		default:
> +			break;
> +		}

Would it make more sense to check the chip type at the start of
the function outside the mutex? Then we can simple return and do
no work if it is a chip that doesn't have DVFS?

> +
> +	}
> +out:
> +	mutex_unlock(&arizona->subsys_max_lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(arizona_dvfs_up);
> +
> +int arizona_dvfs_down(struct arizona *arizona, unsigned int flags)
> +{
> +	unsigned int old_flags;
> +	int ret = 0;
> +
> +	mutex_lock(&arizona->subsys_max_lock);
> +
> +	old_flags = arizona->subsys_max_rq;
> +	arizona->subsys_max_rq &= ~flags;
> +
> +	/* If currently caching the change will be applied in runtime resume */
> +	if (arizona->subsys_max_cached) {
> +		dev_dbg(arizona->dev, "subsys_max_cached (dvfs down)\n");
> +		goto out;
> +	}
> +
> +	if ((old_flags != 0) && (arizona->subsys_max_rq == 0)) {
> +		switch (arizona->type) {
> +		case WM5102:
> +		case WM8997:
> +		case WM8998:
> +		case WM1814:
> +			ret = arizona_dvfs_remove_boost(arizona);
> +			break;
> +
> +		default:
> +			break;
> +		}

ditto here.

> +	}
> +out:
> +	mutex_unlock(&arizona->subsys_max_lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(arizona_dvfs_down);
> +
>  static irqreturn_t arizona_clkgen_err(int irq, void *data)
>  {
>  	struct arizona *arizona = data;
> @@ -462,6 +581,31 @@ static int wm5102_clear_write_sequencer(struct arizona *arizona)
>  }
>  
>  #ifdef CONFIG_PM
> +static int arizona_restore_dvfs(struct arizona *arizona)
> +{
> +	int ret;
> +
> +	switch (arizona->type) {
> +	default:
> +		return 0;	/* no DVFS */
> +
> +	case WM5102:
> +	case WM8997:
> +	case WM8998:
> +	case WM1814:
> +		ret = 0;
> +		mutex_lock(&arizona->subsys_max_lock);
> +		if (arizona->subsys_max_rq != 0) {
> +			dev_dbg(arizona->dev, "Restore subsys_max boost\n");
> +			ret = arizona_dvfs_apply_boost(arizona);
> +		}
> +
> +		arizona->subsys_max_cached = false;
> +		mutex_unlock(&arizona->subsys_max_lock);
> +		return ret;
> +	}
> +}
> +
>  static int arizona_runtime_resume(struct device *dev)
>  {
>  	struct arizona *arizona = dev_get_drvdata(dev);
> @@ -590,6 +734,10 @@ static int arizona_runtime_resume(struct device *dev)
>  		goto err;
>  	}
>  
> +	ret = arizona_restore_dvfs(arizona);
> +	if (ret < 0)
> +		goto err;
> +
>  	return 0;
>  
>  err:
> @@ -612,6 +760,21 @@ static int arizona_runtime_suspend(struct device *dev)
>  		return ret;
>  	}
>  
> +	switch (arizona->type) {
> +	case WM5102:
> +	case WM8997:
> +	case WM8998:
> +	case WM1814:
> +		/* Must disable DVFS boost before powering down DCVDD */
> +		mutex_lock(&arizona->subsys_max_lock);
> +		arizona->subsys_max_cached = true;
> +		arizona_dvfs_remove_boost(arizona);
> +		mutex_unlock(&arizona->subsys_max_lock);
> +		break;
> +	default:
> +		break;
> +	}
> +

I would be tempted to factor this out into a function as well
since we have a restore_dvfs for resume.

Thanks,
Charles