* [PATCH 0/2] ASoC: Intel: Skylake: Add HDA link management @ 2016-04-28 14:00 Vinod Koul 2016-04-28 14:00 ` [PATCH 1/2] ALSA: hdac: add link pm and ref counting Vinod Koul 2016-04-28 14:00 ` [PATCH 2/2] ASoC: Intel: Skylake: add link mangement Vinod Koul 0 siblings, 2 replies; 5+ messages in thread From: Vinod Koul @ 2016-04-28 14:00 UTC (permalink / raw) To: alsa-devel; +Cc: liam.r.girdwood, patches.audio, broonie, Vinod Koul HDA links are always kept On, this patch adds link management routines in hdac extended core and it's use in Skylake driver. One more patch to add support in codec will follow this The aim of serial is to shut down the links when not in use and keep them active when a user is active. The user can activate link by taking reference to the link when required. When all links are shutdown, the command DMAs are turned off as well. On first link power up, the command DMAs are turned on first. First patch touches HDA core, and second one is dependent on it so Takashi if you are okay, pls ACK it and we cna merge thru ASoC tree Vinod Koul (2): ALSA: hdac: add link pm and ref counting ASoC: Intel: Skylake: add link mangement include/sound/hdaudio_ext.h | 13 ++++++++ sound/hda/ext/hdac_ext_bus.c | 3 ++ sound/hda/ext/hdac_ext_controller.c | 62 +++++++++++++++++++++++++++++++++++++ sound/soc/intel/skylake/skl-pcm.c | 1 - sound/soc/intel/skylake/skl.c | 34 ++++++++++++++++++++ 5 files changed, 112 insertions(+), 1 deletion(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] ALSA: hdac: add link pm and ref counting 2016-04-28 14:00 [PATCH 0/2] ASoC: Intel: Skylake: Add HDA link management Vinod Koul @ 2016-04-28 14:00 ` Vinod Koul 2016-04-29 9:05 ` Takashi Iwai 2016-04-28 14:00 ` [PATCH 2/2] ASoC: Intel: Skylake: add link mangement Vinod Koul 1 sibling, 1 reply; 5+ messages in thread From: Vinod Koul @ 2016-04-28 14:00 UTC (permalink / raw) To: alsa-devel; +Cc: liam.r.girdwood, patches.audio, broonie, Vinod Koul, Jeeja KP 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> --- include/sound/hdaudio_ext.h | 13 ++++++++ sound/hda/ext/hdac_ext_bus.c | 3 ++ sound/hda/ext/hdac_ext_controller.c | 62 +++++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+) diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h index 07fa59237feb..59f8a6eaac62 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_io: state of cmd_io */ struct hdac_ext_bus { struct hdac_bus bus; @@ -27,6 +29,9 @@ struct hdac_ext_bus { void __iomem *drsmcap; struct list_head hlink_list; + + spinlock_t lock; + int cmd_io; }; 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 64de0a3d6d93..43ad6f17771f 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++; + spin_lock_init(&ebus->lock); + ebus->cmd_io = 1; + 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..df7835178248 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,62 @@ 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; + + spin_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_io) { + snd_hdac_bus_init_cmd_io(&ebus->bus); + ebus->cmd_io = 1; + } + + ret = snd_hdac_ext_bus_link_power_up(link); + } + + spin_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, state = 0; + struct hdac_ext_link *hlink = NULL; + + spin_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) + state++; + } + if (!state) { + snd_hdac_bus_stop_cmd_io(&ebus->bus); + ebus->cmd_io = 0; + } + } + + spin_unlock(&ebus->lock); + return ret; +} +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_put); -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] ALSA: hdac: add link pm and ref counting 2016-04-28 14:00 ` [PATCH 1/2] ALSA: hdac: add link pm and ref counting Vinod Koul @ 2016-04-29 9:05 ` Takashi Iwai 2016-05-02 9:02 ` Vinod Koul 0 siblings, 1 reply; 5+ messages in thread From: Takashi Iwai @ 2016-04-29 9:05 UTC (permalink / raw) To: Vinod Koul; +Cc: liam.r.girdwood, patches.audio, alsa-devel, broonie, Jeeja KP On Thu, 28 Apr 2016 16:00:58 +0200, 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 > > 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 | 62 +++++++++++++++++++++++++++++++++++++ > 3 files changed, 78 insertions(+) > > diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h > index 07fa59237feb..59f8a6eaac62 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_io: state of cmd_io > */ > struct hdac_ext_bus { > struct hdac_bus bus; > @@ -27,6 +29,9 @@ struct hdac_ext_bus { > void __iomem *drsmcap; > > struct list_head hlink_list; > + > + spinlock_t lock; > + int cmd_io; > }; > > 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 64de0a3d6d93..43ad6f17771f 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++; > > + spin_lock_init(&ebus->lock); > + ebus->cmd_io = 1; > + > 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..df7835178248 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,62 @@ 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; > + > + spin_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_io) { > + snd_hdac_bus_init_cmd_io(&ebus->bus); Calling this function inside a spinlock is wrong. It should be mutex instead. Takashi > + ebus->cmd_io = 1; > + } > + > + ret = snd_hdac_ext_bus_link_power_up(link); > + } > + > + spin_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, state = 0; > + struct hdac_ext_link *hlink = NULL; > + > + spin_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) > + state++; > + } > + if (!state) { > + snd_hdac_bus_stop_cmd_io(&ebus->bus); > + ebus->cmd_io = 0; > + } > + } > + > + spin_unlock(&ebus->lock); > + return ret; > +} > +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_put); > -- > 1.9.1 > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] ALSA: hdac: add link pm and ref counting 2016-04-29 9:05 ` Takashi Iwai @ 2016-05-02 9:02 ` Vinod Koul 0 siblings, 0 replies; 5+ messages in thread From: Vinod Koul @ 2016-05-02 9:02 UTC (permalink / raw) To: Takashi Iwai Cc: liam.r.girdwood, patches.audio, alsa-devel, broonie, Jeeja KP On Fri, Apr 29, 2016 at 11:05:51AM +0200, Takashi Iwai wrote: > > +int snd_hdac_ext_bus_link_get(struct hdac_ext_bus *ebus, > > + struct hdac_ext_link *link) > > +{ > > + int ret = 0; > > + > > + spin_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_io) { > > + snd_hdac_bus_init_cmd_io(&ebus->bus); > > Calling this function inside a spinlock is wrong. > It should be mutex instead. Yes I looked at it's usage, we should do a mutex here, will send an update -- ~Vinod ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] ASoC: Intel: Skylake: add link mangement 2016-04-28 14:00 [PATCH 0/2] ASoC: Intel: Skylake: Add HDA link management Vinod Koul 2016-04-28 14:00 ` [PATCH 1/2] ALSA: hdac: add link pm and ref counting Vinod Koul @ 2016-04-28 14:00 ` Vinod Koul 1 sibling, 0 replies; 5+ messages in thread From: Vinod Koul @ 2016-04-28 14:00 UTC (permalink / raw) To: alsa-devel; +Cc: liam.r.girdwood, patches.audio, broonie, Vinod Koul, Jeeja KP 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> --- 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 c726937321ef..6e71bf34ea2b 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 3982f5536f2d..71ca098561db 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_io) + 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_io) + 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_io) + 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); -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-05-02 8:56 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-04-28 14:00 [PATCH 0/2] ASoC: Intel: Skylake: Add HDA link management Vinod Koul 2016-04-28 14:00 ` [PATCH 1/2] ALSA: hdac: add link pm and ref counting Vinod Koul 2016-04-29 9:05 ` Takashi Iwai 2016-05-02 9:02 ` Vinod Koul 2016-04-28 14:00 ` [PATCH 2/2] ASoC: Intel: Skylake: add link mangement Vinod Koul
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).