On Thu, May 12, 2016 at 08:58:53AM +0530, Vinod Koul wrote:

The HDA links can be switched off when not is use, similarly command DMA can be stopped as well. This calls for a reference counting mechanism on the link by it's users to manage the link power. The DMA can be turned off when all links are off

For this we add two APIs snd_hdac_ext_bus_link_get snd_hdac_ext_bus_link_put

They help users to turn up/down link and manage the DMA as well

Takashi,

Does this look fine to you? If so can we have you ACK please..

Signed-off-by: Jeeja KP jeeja.kp@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com

---

include/sound/hdaudio_ext.h | 13 ++++++++ sound/hda/ext/hdac_ext_bus.c | 3 ++ sound/hda/ext/hdac_ext_controller.c | 66 +++++++++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+)

diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h index 07fa59237feb..b9593b201599 100644 --- a/include/sound/hdaudio_ext.h +++ b/include/sound/hdaudio_ext.h @@ -14,6 +14,8 @@

• @gtscap: gts capabilities pointer
• @drsmcap: dma resume capabilities pointer
• @hlink_list: link list of HDA links

• • @lock: lock for link mgmt
• • @cmd_dma_state: state of cmd DMAs: CORB and RIRB
  */

struct hdac_ext_bus { struct hdac_bus bus; @@ -27,6 +29,9 @@ struct hdac_ext_bus { void __iomem *drsmcap;

struct list_head hlink_list;

• struct mutex lock;
• bool cmd_dma_state;

};

int snd_hdac_ext_bus_init(struct hdac_ext_bus *sbus, struct device *dev, @@ -142,6 +147,9 @@ struct hdac_ext_link { void __iomem *ml_addr; /* link output stream reg pointer */ u32 lcaps; /* link capablities */ u16 lsdiid; /* link sdi identifier */

• int ref_count;
• struct list_head list;

};

@@ -154,6 +162,11 @@ void snd_hdac_ext_link_set_stream_id(struct hdac_ext_link *link, void snd_hdac_ext_link_clear_stream_id(struct hdac_ext_link *link, int stream);

+int snd_hdac_ext_bus_link_get(struct hdac_ext_bus *ebus,

• 		struct hdac_ext_link *link);
  

+int snd_hdac_ext_bus_link_put(struct hdac_ext_bus *ebus,

• 		struct hdac_ext_link *link);
  

/* update register macro */ #define snd_hdac_updatel(addr, reg, mask, val) \ writel(((readl(addr + reg) & ~(mask)) | (val)), \ diff --git a/sound/hda/ext/hdac_ext_bus.c b/sound/hda/ext/hdac_ext_bus.c index 2433f7c81472..3b7ae24900fd 100644 --- a/sound/hda/ext/hdac_ext_bus.c +++ b/sound/hda/ext/hdac_ext_bus.c @@ -105,6 +105,9 @@ int snd_hdac_ext_bus_init(struct hdac_ext_bus *ebus, struct device *dev, INIT_LIST_HEAD(&ebus->hlink_list); ebus->idx = idx++;

• mutex_init(&ebus->lock);
• ebus->cmd_dma_state = true;
• return 0;

} EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_init); diff --git a/sound/hda/ext/hdac_ext_controller.c b/sound/hda/ext/hdac_ext_controller.c index 548cc1e4114b..860f8cad6602 100644 --- a/sound/hda/ext/hdac_ext_controller.c +++ b/sound/hda/ext/hdac_ext_controller.c @@ -186,6 +186,9 @@ int snd_hdac_ext_bus_get_ml_capabilities(struct hdac_ext_bus *ebus) hlink->lcaps = readl(hlink->ml_addr + AZX_REG_ML_LCAP); hlink->lsdiid = readw(hlink->ml_addr + AZX_REG_ML_LSDIID);

• /* since link in On, update the ref */
  

• hlink->ref_count = 1;
  

• list_add_tail(&hlink->list, &ebus->hlink_list); }

@@ -327,3 +330,66 @@ int snd_hdac_ext_bus_link_power_down_all(struct hdac_ext_bus *ebus) return 0; } EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_power_down_all);

+int snd_hdac_ext_bus_link_get(struct hdac_ext_bus *ebus,

• 		struct hdac_ext_link *link)
  

+{

• int ret = 0;
• mutex_lock(&ebus->lock);
• /*

• * if we move from 0 to 1, count will be 1 so power up this link
  

• * as well, also check the dma status and trigger that
  

• */
  

• if (++link->ref_count == 1) {

• if (!ebus->cmd_dma_state) {
  

• 	snd_hdac_bus_init_cmd_io(&ebus->bus);
  

• 	ebus->cmd_dma_state = true;
  

• }
  

• ret = snd_hdac_ext_bus_link_power_up(link);
  

• }
• mutex_unlock(&ebus->lock);
• return ret;

+} +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_get);

+int snd_hdac_ext_bus_link_put(struct hdac_ext_bus *ebus,

• 		struct hdac_ext_link *link)
  

+{

• int ret = 0;
• struct hdac_ext_link *hlink;
• bool link_up = false;
• mutex_lock(&ebus->lock);
• /*

• * if we move from 1 to 0, count will be 0
  

• * so power down this link as well
  

• */
  

• if (--link->ref_count == 0) {

• ret = snd_hdac_ext_bus_link_power_down(link);
  

• /*
  

•  * now check if all links are off, if so turn off
  

•  * cmd dma as well
  

•  */
  

• list_for_each_entry(hlink, &ebus->hlink_list, list) {
  

• 	if (hlink->ref_count) {
  

• 		link_up = true;
  

• 		break;
  

• 	}
  

• }
  

• if (!link_up) {
  

• 	snd_hdac_bus_stop_cmd_io(&ebus->bus);
  

• 	ebus->cmd_dma_state = false;
  

• }
  

• }
• mutex_unlock(&ebus->lock);
• return ret;

+}

+EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_put);

1.9.1

The patch

ALSA: hdac: add link pm and ref counting

has been applied to the asoc tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

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

From 4446085d21e75dd6c0c45577f12db0bd7c7bf35f Mon Sep 17 00:00:00 2001

From: Vinod Koul vinod.koul@intel.com Date: Thu, 12 May 2016 08:58:53 +0530 Subject: [PATCH] ALSA: hdac: add link pm and ref counting

The HDA links can be switched off when not is use, similarly command DMA can be stopped as well. This calls for a reference counting mechanism on the link by it's users to manage the link power. The DMA can be turned off when all links are off

For this we add two APIs snd_hdac_ext_bus_link_get snd_hdac_ext_bus_link_put

They help users to turn up/down link and manage the DMA as well

Signed-off-by: Jeeja KP jeeja.kp@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com Acked-by: Takashi Iwai tiwai@suse.de Signed-off-by: Mark Brown broonie@kernel.org --- include/sound/hdaudio_ext.h | 13 ++++++++ sound/hda/ext/hdac_ext_bus.c | 3 ++ sound/hda/ext/hdac_ext_controller.c | 66 +++++++++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+)

diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h index 07fa59237feb..b9593b201599 100644 --- a/include/sound/hdaudio_ext.h +++ b/include/sound/hdaudio_ext.h @@ -14,6 +14,8 @@ * @gtscap: gts capabilities pointer * @drsmcap: dma resume capabilities pointer * @hlink_list: link list of HDA links + * @lock: lock for link mgmt + * @cmd_dma_state: state of cmd DMAs: CORB and RIRB */ struct hdac_ext_bus { struct hdac_bus bus; @@ -27,6 +29,9 @@ struct hdac_ext_bus { void __iomem *drsmcap;

struct list_head hlink_list; + + struct mutex lock; + bool cmd_dma_state; };

int snd_hdac_ext_bus_init(struct hdac_ext_bus *sbus, struct device *dev, @@ -142,6 +147,9 @@ struct hdac_ext_link { void __iomem *ml_addr; /* link output stream reg pointer */ u32 lcaps; /* link capablities */ u16 lsdiid; /* link sdi identifier */ + + int ref_count; + struct list_head list; };

@@ -154,6 +162,11 @@ void snd_hdac_ext_link_set_stream_id(struct hdac_ext_link *link, void snd_hdac_ext_link_clear_stream_id(struct hdac_ext_link *link, int stream);

+int snd_hdac_ext_bus_link_get(struct hdac_ext_bus *ebus, + struct hdac_ext_link *link); +int snd_hdac_ext_bus_link_put(struct hdac_ext_bus *ebus, + struct hdac_ext_link *link); + /* update register macro */ #define snd_hdac_updatel(addr, reg, mask, val) \ writel(((readl(addr + reg) & ~(mask)) | (val)), \ diff --git a/sound/hda/ext/hdac_ext_bus.c b/sound/hda/ext/hdac_ext_bus.c index 2433f7c81472..3b7ae24900fd 100644 --- a/sound/hda/ext/hdac_ext_bus.c +++ b/sound/hda/ext/hdac_ext_bus.c @@ -105,6 +105,9 @@ int snd_hdac_ext_bus_init(struct hdac_ext_bus *ebus, struct device *dev, INIT_LIST_HEAD(&ebus->hlink_list); ebus->idx = idx++;

+ mutex_init(&ebus->lock); + ebus->cmd_dma_state = true; + return 0; } EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_init); diff --git a/sound/hda/ext/hdac_ext_controller.c b/sound/hda/ext/hdac_ext_controller.c index 548cc1e4114b..860f8cad6602 100644 --- a/sound/hda/ext/hdac_ext_controller.c +++ b/sound/hda/ext/hdac_ext_controller.c @@ -186,6 +186,9 @@ int snd_hdac_ext_bus_get_ml_capabilities(struct hdac_ext_bus *ebus) hlink->lcaps = readl(hlink->ml_addr + AZX_REG_ML_LCAP); hlink->lsdiid = readw(hlink->ml_addr + AZX_REG_ML_LSDIID);

+ /* since link in On, update the ref */ + hlink->ref_count = 1; + list_add_tail(&hlink->list, &ebus->hlink_list); }

@@ -327,3 +330,66 @@ int snd_hdac_ext_bus_link_power_down_all(struct hdac_ext_bus *ebus) return 0; } EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_power_down_all); + +int snd_hdac_ext_bus_link_get(struct hdac_ext_bus *ebus, + struct hdac_ext_link *link) +{ + int ret = 0; + + mutex_lock(&ebus->lock); + + /* + * if we move from 0 to 1, count will be 1 so power up this link + * as well, also check the dma status and trigger that + */ + if (++link->ref_count == 1) { + if (!ebus->cmd_dma_state) { + snd_hdac_bus_init_cmd_io(&ebus->bus); + ebus->cmd_dma_state = true; + } + + ret = snd_hdac_ext_bus_link_power_up(link); + } + + mutex_unlock(&ebus->lock); + return ret; +} +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_get); + +int snd_hdac_ext_bus_link_put(struct hdac_ext_bus *ebus, + struct hdac_ext_link *link) +{ + int ret = 0; + struct hdac_ext_link *hlink; + bool link_up = false; + + mutex_lock(&ebus->lock); + + /* + * if we move from 1 to 0, count will be 0 + * so power down this link as well + */ + if (--link->ref_count == 0) { + ret = snd_hdac_ext_bus_link_power_down(link); + + /* + * now check if all links are off, if so turn off + * cmd dma as well + */ + list_for_each_entry(hlink, &ebus->hlink_list, list) { + if (hlink->ref_count) { + link_up = true; + break; + } + } + + if (!link_up) { + snd_hdac_bus_stop_cmd_io(&ebus->bus); + ebus->cmd_dma_state = false; + } + } + + mutex_unlock(&ebus->lock); + return ret; +} +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_put);

The patch

ASoC: Intel: Skylake: add link management

has been applied to the asoc tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

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

From cce6c149eba3aabf678ffea91ac1e4103b9c185e Mon Sep 17 00:00:00 2001

From: Vinod Koul vinod.koul@intel.com Date: Thu, 12 May 2016 08:58:54 +0530 Subject: [PATCH] ASoC: Intel: Skylake: add link management

Use shiny new link APIs to manage the links. Also remove old link configuration logic from driver.

We need to keep link and cmd dma to off during active suspend to allow system to enter low power state and turn it on if the link and cmd dma was on before active suspend in active resume.

Signed-off-by: Jeeja KP jeeja.kp@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/skylake/skl-pcm.c | 1 - sound/soc/intel/skylake/skl.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c index 4494db6a05f5..15480234b20b 100644 --- a/sound/soc/intel/skylake/skl-pcm.c +++ b/sound/soc/intel/skylake/skl-pcm.c @@ -533,7 +533,6 @@ static int skl_link_pcm_prepare(struct snd_pcm_substream *substream, if (!link) return -EINVAL;

- snd_hdac_ext_bus_link_power_up(link); snd_hdac_ext_link_stream_reset(link_dev);

snd_hdac_ext_link_stream_setup(link_dev, format_val); diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c index 83e985c0c0c9..06d8c263c68f 100644 --- a/sound/soc/intel/skylake/skl.c +++ b/sound/soc/intel/skylake/skl.c @@ -229,7 +229,12 @@ static int skl_suspend(struct device *dev) * running, we need to save the state for these and continue */ if (skl->supend_active) { + /* turn off the links and stop the CORB/RIRB DMA if it is On */ snd_hdac_ext_bus_link_power_down_all(ebus); + + if (ebus->cmd_dma_state) + snd_hdac_bus_stop_cmd_io(&ebus->bus); + enable_irq_wake(bus->irq); pci_save_state(pci); pci_disable_device(pci); @@ -255,6 +260,7 @@ static int skl_resume(struct device *dev) struct hdac_ext_bus *ebus = pci_get_drvdata(pci); struct skl *skl = ebus_to_skl(ebus); struct hdac_bus *bus = ebus_to_hbus(ebus); + struct hdac_ext_link *hlink = NULL; int ret;

/* Turned OFF in HDMI codec driver after codec reconfiguration */ @@ -276,8 +282,29 @@ static int skl_resume(struct device *dev) ret = pci_enable_device(pci); snd_hdac_ext_bus_link_power_up_all(ebus); disable_irq_wake(bus->irq); + /* + * turn On the links which are On before active suspend + * and start the CORB/RIRB DMA if On before + * active suspend. + */ + list_for_each_entry(hlink, &ebus->hlink_list, list) { + if (hlink->ref_count) + snd_hdac_ext_bus_link_power_up(hlink); + } + + if (ebus->cmd_dma_state) + snd_hdac_bus_init_cmd_io(&ebus->bus); } else { ret = _skl_resume(ebus); + + /* turn off the links which are off before suspend */ + list_for_each_entry(hlink, &ebus->hlink_list, list) { + if (!hlink->ref_count) + snd_hdac_ext_bus_link_power_down(hlink); + } + + if (!ebus->cmd_dma_state) + snd_hdac_bus_stop_cmd_io(&ebus->bus); }

return ret; @@ -613,6 +640,7 @@ static int skl_probe(struct pci_dev *pci, struct skl *skl; struct hdac_ext_bus *ebus = NULL; struct hdac_bus *bus = NULL; + struct hdac_ext_link *hlink = NULL; int err;

/* we use ext core ops, so provide NULL for ops here */ @@ -679,6 +707,12 @@ static int skl_probe(struct pci_dev *pci, } }

+ /* + * we are done probling so decrement link counts + */ + list_for_each_entry(hlink, &ebus->hlink_list, list) + snd_hdac_ext_bus_link_put(ebus, hlink); + /*configure PM */ pm_runtime_put_noidle(bus->dev); pm_runtime_allow(bus->dev);