[PATCH 0/3] ASoC: mchp-spdiftx: add power saving features

newer
[PATCH] ASoC: Intel: sof_sdw_amp:...

Claudiu Beznea

17 Nov 2022 17 Nov '22

1:37 p.m.

Hi,

This series adds support for runtime PM and system suspend/resume for Microchip SPDIFTX (patches 2/3, 3/3). Along with it I took the chance and added a minor cleanup (patch 1/3).

Thank you, Claudiu Beznea

Claudiu Beznea (3): ASoC: mchp-spdiftx: simplify locking around ctrl->ch_stat ASoC: mchp-spdiftx: add runtime pm support ASoC: mchp-spdiftx: add support for system suspend/resume

sound/soc/atmel/mchp-spdiftx.c | 165 ++++++++++++++++++++++----------- 1 file changed, 113 insertions(+), 52 deletions(-)

-- 2.34.1

Show replies by date

Claudiu Beznea

17 Nov 17 Nov

1:37 p.m.

New subject: [PATCH 1/3] ASoC: mchp-spdiftx: simplify locking around ctrl->ch_stat

Use a temporary variable to keep the AES3 value. With this a spin_unlock_irqrestore() call has been removed from the final code.

Signed-off-by: Claudiu Beznea claudiu.beznea@microchip.com --- sound/soc/atmel/mchp-spdiftx.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/sound/soc/atmel/mchp-spdiftx.c b/sound/soc/atmel/mchp-spdiftx.c index ab2d7a791f39..4e231cec9045 100644 --- a/sound/soc/atmel/mchp-spdiftx.c +++ b/sound/soc/atmel/mchp-spdiftx.c @@ -355,6 +355,7 @@ static int mchp_spdiftx_hw_params(struct snd_pcm_substream *substream, struct mchp_spdiftx_mixer_control *ctrl = &dev->control; u32 mr; unsigned int bps = params_physical_width(params) / 8; + unsigned char aes3; int ret;

dev_dbg(dev->dev, "%s() rate=%u format=%#x width=%u channels=%u\n", @@ -440,48 +441,48 @@ static int mchp_spdiftx_hw_params(struct snd_pcm_substream *substream,

mr |= SPDIFTX_MR_BPS(bps);

- spin_lock_irqsave(&ctrl->lock, flags); - ctrl->ch_stat[3] &= ~IEC958_AES3_CON_FS; switch (params_rate(params)) { case 22050: - ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_22050; + aes3 = IEC958_AES3_CON_FS_22050; break; case 24000: - ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_24000; + aes3 = IEC958_AES3_CON_FS_24000; break; case 32000: - ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_32000; + aes3 = IEC958_AES3_CON_FS_32000; break; case 44100: - ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_44100; + aes3 = IEC958_AES3_CON_FS_44100; break; case 48000: - ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_48000; + aes3 = IEC958_AES3_CON_FS_48000; break; case 88200: - ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_88200; + aes3 = IEC958_AES3_CON_FS_88200; break; case 96000: - ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_96000; + aes3 = IEC958_AES3_CON_FS_96000; break; case 176400: - ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_176400; + aes3 = IEC958_AES3_CON_FS_176400; break; case 192000: - ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_192000; + aes3 = IEC958_AES3_CON_FS_192000; break; case 8000: case 11025: case 16000: case 64000: - ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_NOTID; + aes3 = IEC958_AES3_CON_FS_NOTID; break; default: dev_err(dev->dev, "unsupported sample frequency: %u\n", params_rate(params)); - spin_unlock_irqrestore(&ctrl->lock, flags); return -EINVAL; } + spin_lock_irqsave(&ctrl->lock, flags); + ctrl->ch_stat[3] &= ~IEC958_AES3_CON_FS; + ctrl->ch_stat[3] |= aes3; mchp_spdiftx_channel_status_write(dev); spin_unlock_irqrestore(&ctrl->lock, flags);

-- 2.34.1

Claudiu Beznea

1:37 p.m.

New subject: [PATCH 2/3] ASoC: mchp-spdiftx: add runtime pm support

Add runtime PM support for Microchip SPDIFTX driver. The runtime PM APIs disables/enables IP's clock and enables/disable caching for regmap.

Signed-off-by: Claudiu Beznea claudiu.beznea@microchip.com --- sound/soc/atmel/mchp-spdiftx.c | 116 ++++++++++++++++++++++++--------- 1 file changed, 86 insertions(+), 30 deletions(-)

diff --git a/sound/soc/atmel/mchp-spdiftx.c b/sound/soc/atmel/mchp-spdiftx.c index 4e231cec9045..ec454e64d85c 100644 --- a/sound/soc/atmel/mchp-spdiftx.c +++ b/sound/soc/atmel/mchp-spdiftx.c @@ -9,6 +9,7 @@ #include <linux/clk.h> #include <linux/io.h> #include <linux/module.h> +#include <linux/pm_runtime.h> #include <linux/spinlock.h>

#include <sound/asoundef.h> @@ -175,6 +176,7 @@ static const struct regmap_config mchp_spdiftx_regmap_config = { .readable_reg = mchp_spdiftx_readable_reg, .writeable_reg = mchp_spdiftx_writeable_reg, .precious_reg = mchp_spdiftx_precious_reg, + .cache_type = REGCACHE_FLAT, };

#define SPDIFTX_GCLK_RATIO 128 @@ -196,7 +198,6 @@ struct mchp_spdiftx_dev { struct clk *pclk; struct clk *gclk; unsigned int fmt; - unsigned int gclk_enabled:1; };

static inline int mchp_spdiftx_is_running(struct mchp_spdiftx_dev *dev) @@ -486,10 +487,9 @@ static int mchp_spdiftx_hw_params(struct snd_pcm_substream *substream, mchp_spdiftx_channel_status_write(dev); spin_unlock_irqrestore(&ctrl->lock, flags);

- if (dev->gclk_enabled) { - clk_disable_unprepare(dev->gclk); - dev->gclk_enabled = 0; - } + /* GCLK is enabled by runtime PM. */ + clk_disable_unprepare(dev->gclk); + ret = clk_set_rate(dev->gclk, params_rate(params) * SPDIFTX_GCLK_RATIO); if (ret) { @@ -503,7 +503,7 @@ static int mchp_spdiftx_hw_params(struct snd_pcm_substream *substream, dev_err(dev->dev, "unable to enable gclk: %d\n", ret); return ret; } - dev->gclk_enabled = 1; + dev_dbg(dev->dev, "%s(): GCLK set to %d\n", __func__, params_rate(params) * SPDIFTX_GCLK_RATIO);

@@ -523,10 +523,6 @@ static int mchp_spdiftx_hw_free(struct snd_pcm_substream *substream,

regmap_write(dev->regmap, SPDIFTX_IDR, SPDIFTX_IR_TXUDR | SPDIFTX_IR_TXOVR); - if (dev->gclk_enabled) { - clk_disable_unprepare(dev->gclk); - dev->gclk_enabled = 0; - }

return regmap_write(dev->regmap, SPDIFTX_CR, SPDIFTX_CR_SWRST | SPDIFTX_CR_FCLR); @@ -709,17 +705,9 @@ static struct snd_kcontrol_new mchp_spdiftx_ctrls[] = { static int mchp_spdiftx_dai_probe(struct snd_soc_dai *dai) { struct mchp_spdiftx_dev *dev = snd_soc_dai_get_drvdata(dai); - int ret;

snd_soc_dai_init_dma_data(dai, &dev->playback, NULL);

- ret = clk_prepare_enable(dev->pclk); - if (ret) { - dev_err(dev->dev, - "failed to enable the peripheral clock: %d\n", ret); - return ret; - } - /* Add controls */ snd_soc_add_dai_controls(dai, mchp_spdiftx_ctrls, ARRAY_SIZE(mchp_spdiftx_ctrls)); @@ -727,19 +715,9 @@ static int mchp_spdiftx_dai_probe(struct snd_soc_dai *dai) return 0; }

-static int mchp_spdiftx_dai_remove(struct snd_soc_dai *dai) -{ - struct mchp_spdiftx_dev *dev = snd_soc_dai_get_drvdata(dai); - - clk_disable_unprepare(dev->pclk); - - return 0; -} - static struct snd_soc_dai_driver mchp_spdiftx_dai = { .name = "mchp-spdiftx", .probe = mchp_spdiftx_dai_probe, - .remove = mchp_spdiftx_dai_remove, .playback = { .stream_name = "S/PDIF Playback", .channels_min = 1, @@ -763,6 +741,54 @@ static const struct of_device_id mchp_spdiftx_dt_ids[] = { }; MODULE_DEVICE_TABLE(of, mchp_spdiftx_dt_ids);

+static int mchp_spdiftx_runtime_suspend(struct device *dev) +{ + struct mchp_spdiftx_dev *spdiftx = dev_get_drvdata(dev); + + regcache_cache_only(spdiftx->regmap, true); + + clk_disable_unprepare(spdiftx->gclk); + clk_disable_unprepare(spdiftx->pclk); + + return 0; +} + +static int mchp_spdiftx_runtime_resume(struct device *dev) +{ + struct mchp_spdiftx_dev *spdiftx = dev_get_drvdata(dev); + int ret; + + ret = clk_prepare_enable(spdiftx->pclk); + if (ret) { + dev_err(spdiftx->dev, + "failed to enable the peripheral clock: %d\n", ret); + return ret; + } + ret = clk_prepare_enable(spdiftx->gclk); + if (ret) { + dev_err(spdiftx->dev, + "failed to enable generic clock: %d\n", ret); + goto disable_pclk; + } + + regcache_cache_only(spdiftx->regmap, false); + regcache_mark_dirty(spdiftx->regmap); + ret = regcache_sync(spdiftx->regmap); + if (ret) { + regcache_cache_only(spdiftx->regmap, true); + clk_disable_unprepare(spdiftx->gclk); +disable_pclk: + clk_disable_unprepare(spdiftx->pclk); + } + + return ret; +} + +static const struct dev_pm_ops mchp_spdiftx_pm_ops = { + RUNTIME_PM_OPS(mchp_spdiftx_runtime_suspend, mchp_spdiftx_runtime_resume, + NULL) +}; + static int mchp_spdiftx_probe(struct platform_device *pdev) { struct mchp_spdiftx_dev *dev; @@ -827,29 +853,59 @@ static int mchp_spdiftx_probe(struct platform_device *pdev) dev->regmap = regmap; platform_set_drvdata(pdev, dev);

+ pm_runtime_enable(dev->dev); + if (!pm_runtime_enabled(dev->dev)) { + err = mchp_spdiftx_runtime_resume(dev->dev); + if (err) + return err; + } + dev->playback.addr = (dma_addr_t)mem->start + SPDIFTX_CDR; dev->playback.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;

err = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0); if (err) { dev_err(&pdev->dev, "failed to register PMC: %d\n", err); - return err; + goto pm_runtime_suspend; }

err = devm_snd_soc_register_component(&pdev->dev, &mchp_spdiftx_component, &mchp_spdiftx_dai, 1); - if (err) + if (err) { dev_err(&pdev->dev, "failed to register component: %d\n", err); + goto pm_runtime_suspend; + } + + return 0; + +pm_runtime_suspend: + if (!pm_runtime_status_suspended(dev->dev)) + mchp_spdiftx_runtime_suspend(dev->dev); + pm_runtime_disable(dev->dev);

return err; }

+static int mchp_spdiftx_remove(struct platform_device *pdev) +{ + struct mchp_spdiftx_dev *dev = platform_get_drvdata(pdev); + + if (!pm_runtime_status_suspended(dev->dev)) + mchp_spdiftx_runtime_suspend(dev->dev); + + pm_runtime_disable(dev->dev); + + return 0; +} + static struct platform_driver mchp_spdiftx_driver = { .probe = mchp_spdiftx_probe, + .remove = mchp_spdiftx_remove, .driver = { .name = "mchp_spdiftx", .of_match_table = of_match_ptr(mchp_spdiftx_dt_ids), + .pm = pm_ptr(&mchp_spdiftx_pm_ops) }, };

-- 2.34.1

Claudiu Beznea

1:37 p.m.

New subject: [PATCH 3/3] ASoC: mchp-spdiftx: add support for system suspend/resume

Add support for system suspend/resume by moving the enable/disable of interrupts in mchp_spdiftx_trigger() on SNDRV_PCM_TRIGGER_SUSPEND/ SNDRV_PCM_TRIGGER_RESUME commands.

Signed-off-by: Claudiu Beznea claudiu.beznea@microchip.com --- sound/soc/atmel/mchp-spdiftx.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/sound/soc/atmel/mchp-spdiftx.c b/sound/soc/atmel/mchp-spdiftx.c index ec454e64d85c..dc96a6fbf514 100644 --- a/sound/soc/atmel/mchp-spdiftx.c +++ b/sound/soc/atmel/mchp-spdiftx.c @@ -198,6 +198,7 @@ struct mchp_spdiftx_dev { struct clk *pclk; struct clk *gclk; unsigned int fmt; + unsigned int suspend_irq; };

static inline int mchp_spdiftx_is_running(struct mchp_spdiftx_dev *dev) @@ -318,16 +319,25 @@ static int mchp_spdiftx_trigger(struct snd_pcm_substream *substream, int cmd, running = !!(mr & SPDIFTX_MR_TXEN_ENABLE);

switch (cmd) { - case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_RESUME: + case SNDRV_PCM_TRIGGER_START: + regmap_write(dev->regmap, SPDIFTX_IER, dev->suspend_irq | + SPDIFTX_IR_TXUDR | SPDIFTX_IR_TXOVR); + dev->suspend_irq = 0; + fallthrough; case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: if (!running) { mr &= ~SPDIFTX_MR_TXEN_MASK; mr |= SPDIFTX_MR_TXEN_ENABLE; } break; - case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_SUSPEND: + regmap_read(dev->regmap, SPDIFTX_IMR, &dev->suspend_irq); + fallthrough; + case SNDRV_PCM_TRIGGER_STOP: + regmap_write(dev->regmap, SPDIFTX_IDR, dev->suspend_irq | + SPDIFTX_IR_TXUDR | SPDIFTX_IR_TXOVR); + fallthrough; case SNDRV_PCM_TRIGGER_PAUSE_PUSH: if (running) { mr &= ~SPDIFTX_MR_TXEN_MASK; @@ -507,10 +517,6 @@ static int mchp_spdiftx_hw_params(struct snd_pcm_substream *substream, dev_dbg(dev->dev, "%s(): GCLK set to %d\n", __func__, params_rate(params) * SPDIFTX_GCLK_RATIO);

- /* Enable interrupts */ - regmap_write(dev->regmap, SPDIFTX_IER, - SPDIFTX_IR_TXUDR | SPDIFTX_IR_TXOVR); - regmap_write(dev->regmap, SPDIFTX_MR, mr);

return 0; @@ -521,9 +527,6 @@ static int mchp_spdiftx_hw_free(struct snd_pcm_substream *substream, { struct mchp_spdiftx_dev *dev = snd_soc_dai_get_drvdata(dai);

- regmap_write(dev->regmap, SPDIFTX_IDR, - SPDIFTX_IR_TXUDR | SPDIFTX_IR_TXOVR); - return regmap_write(dev->regmap, SPDIFTX_CR, SPDIFTX_CR_SWRST | SPDIFTX_CR_FCLR); } @@ -785,6 +788,7 @@ static int mchp_spdiftx_runtime_resume(struct device *dev) }

static const struct dev_pm_ops mchp_spdiftx_pm_ops = { + SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume) RUNTIME_PM_OPS(mchp_spdiftx_runtime_suspend, mchp_spdiftx_runtime_resume, NULL) };

-- 2.34.1

Mark Brown

18 Nov 18 Nov

4:25 p.m.

On Thu, 17 Nov 2022 14:37:47 +0200, Claudiu Beznea wrote:

...

This series adds support for runtime PM and system suspend/resume for Microchip SPDIFTX (patches 2/3, 3/3). Along with it I took the chance and added a minor cleanup (patch 1/3).

Thank you, Claudiu Beznea

[...]

Applied to

broonie/sound.git for-next

Thanks!

[1/3] ASoC: mchp-spdiftx: simplify locking around ctrl->ch_stat commit: 215450eb8b0fac000a42c1cd52c8966fb5159037 [2/3] ASoC: mchp-spdiftx: add runtime pm support commit: 4bf54ca60f99643cfaa3e5b532f139f6501f318e [3/3] ASoC: mchp-spdiftx: add support for system suspend/resume commit: abc7edb0329cd2eabc0b948f5e248c85f6268296

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks, Mark

766

Age (days ago)

767

Last active (days ago)

List overview

Download

4 comments

2 participants

tags (0)

participants (2)

Claudiu Beznea
Mark Brown