| Message ID | 1463023735-16981-2-git-send-email-vinod.koul@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4446085d21e75dd6c0c45577f12db0bd7c7bf35f |
| Headers | show |
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 >
On Fri, 13 May 2016 11:34:46 +0200, Vinod Koul wrote: > > 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.. Acked-by: Takashi Iwai <tiwai@suse.de> Takashi
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);