Re: [PATCH v2 1/3] ALSA: hdac: add link pm and ref counting

[Vinod Koul <vinod.koul@intel.com>]

On Thu, May 05, 2016 at 11:31:09AM +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

And I missed CCing Takashi, sorry about that. Takashi are you okay with this
patch and it going thu ASoC tree :)

> 
> 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..56fa65a1195d 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;
> +
> +	struct mutex 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..0f62b5498c3d 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_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..9b60624d3405 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;
> +
> +	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_io) {
> +			snd_hdac_bus_init_cmd_io(&ebus->bus);
> +			ebus->cmd_io = 1;
> +		}
> +
> +		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, state = 0;
> +	struct hdac_ext_link *hlink = NULL;
> +
> +	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)
> +				state++;
> +		}
> +		if (!state) {
> +			snd_hdac_bus_stop_cmd_io(&ebus->bus);
> +			ebus->cmd_io = 0;
> +		}
> +	}
> +
> +	mutex_unlock(&ebus->lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_put);
> -- 
> 1.9.1
> 

-- 
~Vinod