[PATCH 0/4] usb-audio: Add UAC3 Power Domains

[PATCH 0/4] usb-audio: Add UAC3 Power Domains

[Jorge Sanjuan]

This patchset add support for UAC3 Power Domains. This feature
of the USB audio class 3 allows the host to notify the device 
what it is making use of so power comsumption can be optimized.

This proposal implements this feature for Power Domains
that include an Input/Output Terminal associated to an
audio Streaming interface. This is the main usage of this
feature according to the spec. For that reason, the logic
for the Power Domain state change has been implemented
within the ALSA PCMs logic and the suspend/resume callbacks
of the usb_driver. The behaviour would be as follows:

* Power Domain State D0: A Power Domain will reach this state
  only when the audio substream associated to that domain is
  being used (i,e. Audio playback/capture is happening).
* Power Domain State D1: This is the Idle state where the driver
  is going to always want to be in order to reduce power
  consumption.
* Power Domain State D2: This state is only set when the usb driver
  asumes the device is not going to be used anymore and hence, it
  wont care about getting any interrupts from the device. This
  will only happen when power level is set to "auto" in sysfs
  so the usb driver gets suspended when the interfaces are not in use.
 
NOTE: The way this has been implemented will always try to put the 
Power Domain in state D1 if the Power Domain exists so there is not a 
way a user could disable this feature. It may be worth getting a control
exposed to userland that enables/disables this feature (?).

Power Domains affecting other units independently are required to be 
bypassed via a Selector Unit first before the host can change the 
power state. This sceneario is not covered in this patchset.

based on next-20180719

Jorge Sanjuan (4):
  ALSA: usb-audio: Initial Power Domain support
  ALSA: usb-audio: AudioStreaming Power Domain parsing
  ALSA: usb-audio: Operate UAC3 Power Domains in PCM callbacks
  ALSA: usb-audio: Add UAC3 Power Domains to suspend/resume

 include/linux/usb/audio-v3.h |   4 ++
 sound/usb/Makefile           |   1 +
 sound/usb/card.c             |   9 ++++
 sound/usb/card.h             |   2 +
 sound/usb/pcm.c              |  64 +++++++++++++++++++++--
 sound/usb/pcm.h              |   2 +
 sound/usb/power.c            | 117 +++++++++++++++++++++++++++++++++++++++++++
 sound/usb/power.h            |  19 +++++++
 sound/usb/stream.c           |  70 +++++++++++++++++++++++---
 9 files changed, 277 insertions(+), 11 deletions(-)
 create mode 100644 sound/usb/power.c

-- 
2.11.0

[PATCH 1/4] ALSA: usb-audio: Initial Power Domain support

[Jorge Sanjuan]

Thee USB Audio Class 3 (UAC3) introduces Power Domains as a new
feature to let a host turn individual parts of an audio function
to different power states via USB requests. This lets the device
get to know a bit amore about what the host is up to in order to
optimize power consumption efficiently.

The Power Domains are optional for UAC3 configuration but all
UAC3 devices shall include at least one BADD configuration where
the support for Power Domains is compulsory.

This patch adds a set of features/helpers to parse these power
domains and change their status.

Signed-off-by: Jorge Sanjuan <jorge.sanjuan@codethink.co.uk>
---
 include/linux/usb/audio-v3.h |   4 ++
 sound/usb/Makefile           |   1 +
 sound/usb/power.c            | 117 +++++++++++++++++++++++++++++++++++++++++++
 sound/usb/power.h            |  19 +++++++
 4 files changed, 141 insertions(+)
 create mode 100644 sound/usb/power.c

diff --git a/include/linux/usb/audio-v3.h b/include/linux/usb/audio-v3.h
index 334bfa6dfb47..786e5939d831 100644
--- a/include/linux/usb/audio-v3.h
+++ b/include/linux/usb/audio-v3.h
@@ -447,4 +447,8 @@ struct uac3_interrupt_data_msg {
 /* BADD sample rate is always fixed to 48kHz */
 #define UAC3_BADD_SAMPLING_RATE				48000
 
+/* BADD power domains recovery times */
+#define UAC3_BADD_PD_RECOVER_D1D0			0x0258
+#define UAC3_BADD_PD_RECOVER_D2D0			0x1770
+
 #endif /* __LINUX_USB_AUDIO_V3_H */
diff --git a/sound/usb/Makefile b/sound/usb/Makefile
index 05440e2df8d9..a4d69638b501 100644
--- a/sound/usb/Makefile
+++ b/sound/usb/Makefile
@@ -15,6 +15,7 @@ snd-usb-audio-objs := 	card.o \
 			pcm.o \
 			proc.o \
 			quirks.o \
+			power.o \
 			stream.o
 
 snd-usbmidi-lib-objs := midi.o
diff --git a/sound/usb/power.c b/sound/usb/power.c
new file mode 100644
index 000000000000..9f5d1875c4d3
--- /dev/null
+++ b/sound/usb/power.c
@@ -0,0 +1,117 @@
+/*
+ *   UAC3 Power Domain state management functions
+ *
+ *   This program is free software; you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation; either version 2 of the License, or
+ *   (at your option) any later version.
+ *
+ *   This program is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *   GNU General Public License for more details.
+ *
+ *   You should have received a copy of the GNU General Public License
+ *   along with this program; if not, write to the Free Software
+ *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
+ *
+ */
+
+#include <linux/usb.h>
+#include <linux/usb/audio.h>
+#include <linux/usb/audio-v2.h>
+#include <linux/usb/audio-v3.h>
+
+#include "usbaudio.h"
+#include "helper.h"
+#include "power.h"
+
+struct snd_usb_power_domain *
+snd_usb_find_power_domain(struct usb_host_interface *ctrl_iface,
+			  unsigned char id)
+{
+	struct snd_usb_power_domain *pd;
+	void *p;
+
+	pd = kzalloc(sizeof(*pd), GFP_KERNEL);
+	if (!pd)
+		return NULL;
+
+	p = NULL;
+	while ((p = snd_usb_find_csint_desc(ctrl_iface->extra,
+					    ctrl_iface->extralen,
+					    p, UAC3_POWER_DOMAIN)) != NULL) {
+		struct uac3_power_domain_descriptor *pd_desc = p;
+		int i;
+
+		for (i = 0; i < pd_desc->bNrEntities; i++) {
+			if (pd_desc->baEntityID[i] == id) {
+				pd->pd_id = pd_desc->bPowerDomainID;
+				pd->pd_d1d0_rec =
+					le16_to_cpu(pd_desc->waRecoveryTime1);
+				pd->pd_d2d0_rec =
+					le16_to_cpu(pd_desc->waRecoveryTime2);
+				return pd;
+			}
+		}
+	}
+
+	kfree(pd);
+	return NULL;
+}
+
+int snd_usb_power_domain_set(struct snd_usb_audio *chip,
+			     struct snd_usb_power_domain *pd,
+			     unsigned char state)
+{
+	struct usb_device *dev = chip->dev;
+	unsigned char current_state;
+	int err, idx;
+
+	idx = snd_usb_ctrl_intf(chip) | (pd->pd_id << 8);
+
+	err = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0),
+			      UAC2_CS_CUR,
+			      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
+			      UAC3_AC_POWER_DOMAIN_CONTROL << 8, idx,
+			      &current_state, sizeof(current_state));
+	if (err < 0) {
+		dev_err(&dev->dev, "Can't get UAC3 power state for id %d\n",
+			pd->pd_id);
+		return err;
+	}
+
+	if (current_state == state) {
+		dev_dbg(&dev->dev, "UAC3 power domain id %d already in state %d\n",
+			pd->pd_id, state);
+		return 0;
+	}
+
+	err = snd_usb_ctl_msg(chip->dev, usb_sndctrlpipe(chip->dev, 0), UAC2_CS_CUR,
+			      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_OUT,
+			      UAC3_AC_POWER_DOMAIN_CONTROL << 8, idx,
+			      &state, sizeof(state));
+	if (err < 0) {
+		dev_err(&dev->dev, "Can't set UAC3 power state to %d for id %d\n",
+			state, pd->pd_id);
+		return err;
+	}
+
+	if (state == UAC3_PD_STATE_D0) {
+		switch (current_state) {
+		case UAC3_PD_STATE_D2:
+			udelay(pd->pd_d2d0_rec * 50);
+			break;
+		case UAC3_PD_STATE_D1:
+			udelay(pd->pd_d1d0_rec * 50);
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	dev_dbg(&dev->dev, "UAC3 power domain id %d change to state %d\n",
+		pd->pd_id, state);
+
+	return 0;
+}
diff --git a/sound/usb/power.h b/sound/usb/power.h
index b2e25f60c5a2..6004231a7c75 100644
--- a/sound/usb/power.h
+++ b/sound/usb/power.h
@@ -2,6 +2,25 @@
 #ifndef __USBAUDIO_POWER_H
 #define __USBAUDIO_POWER_H
 
+struct snd_usb_power_domain {
+	int pd_id;              /* UAC3 Power Domain ID */
+	int pd_d1d0_rec;        /* D1 to D0 recovery time */
+	int pd_d2d0_rec;        /* D2 to D0 recovery time */
+};
+
+enum {
+	UAC3_PD_STATE_D0,
+	UAC3_PD_STATE_D1,
+	UAC3_PD_STATE_D2,
+};
+
+int snd_usb_power_domain_set(struct snd_usb_audio *chip,
+			     struct snd_usb_power_domain *pd,
+			     unsigned char state);
+struct snd_usb_power_domain *
+snd_usb_find_power_domain(struct usb_host_interface *ctrl_iface,
+			  unsigned char id);
+
 #ifdef CONFIG_PM
 int snd_usb_autoresume(struct snd_usb_audio *chip);
 void snd_usb_autosuspend(struct snd_usb_audio *chip);
-- 
2.11.0

Re: [PATCH 1/4] ALSA: usb-audio: Initial Power Domain support

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2680 bytes --]

Hi Jorge,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on sound/for-next]
[also build test ERROR on v4.18-rc5 next-20180719]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jorge-Sanjuan/usb-audio-Add-UAC3-Power-Domains/20180719-212009
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next
config: arm-multi_v7_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm 

All error/warnings (new ones prefixed by >>):

   sound/usb/power.c: In function 'snd_usb_find_power_domain':
>> sound/usb/power.c:36:7: error: implicit declaration of function 'kzalloc'; did you mean 'd_alloc'? [-Werror=implicit-function-declaration]
     pd = kzalloc(sizeof(*pd), GFP_KERNEL);
          ^~~~~~~
          d_alloc
>> sound/usb/power.c:36:5: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     pd = kzalloc(sizeof(*pd), GFP_KERNEL);
        ^
>> sound/usb/power.c:59:2: error: implicit declaration of function 'kfree' [-Werror=implicit-function-declaration]
     kfree(pd);
     ^~~~~
   cc1: some warnings being treated as errors

vim +36 sound/usb/power.c

    28	
    29	struct snd_usb_power_domain *
    30	snd_usb_find_power_domain(struct usb_host_interface *ctrl_iface,
    31				  unsigned char id)
    32	{
    33		struct snd_usb_power_domain *pd;
    34		void *p;
    35	
  > 36		pd = kzalloc(sizeof(*pd), GFP_KERNEL);
    37		if (!pd)
    38			return NULL;
    39	
    40		p = NULL;
    41		while ((p = snd_usb_find_csint_desc(ctrl_iface->extra,
    42						    ctrl_iface->extralen,
    43						    p, UAC3_POWER_DOMAIN)) != NULL) {
    44			struct uac3_power_domain_descriptor *pd_desc = p;
    45			int i;
    46	
    47			for (i = 0; i < pd_desc->bNrEntities; i++) {
    48				if (pd_desc->baEntityID[i] == id) {
    49					pd->pd_id = pd_desc->bPowerDomainID;
    50					pd->pd_d1d0_rec =
    51						le16_to_cpu(pd_desc->waRecoveryTime1);
    52					pd->pd_d2d0_rec =
    53						le16_to_cpu(pd_desc->waRecoveryTime2);
    54					return pd;
    55				}
    56			}
    57		}
    58	
  > 59		kfree(pd);
    60		return NULL;
    61	}
    62	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 43896 bytes --]

Re: [PATCH 1/4] ALSA: usb-audio: Initial Power Domain support

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2719 bytes --]

Hi Jorge,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on sound/for-next]
[also build test ERROR on v4.18-rc5 next-20180719]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jorge-Sanjuan/usb-audio-Add-UAC3-Power-Domains/20180719-212009
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=sparc64 

All errors (new ones prefixed by >>):

   sound//usb/power.c: In function 'snd_usb_find_power_domain':
   sound//usb/power.c:36:7: error: implicit declaration of function 'kzalloc'; did you mean 'd_alloc'? [-Werror=implicit-function-declaration]
     pd = kzalloc(sizeof(*pd), GFP_KERNEL);
          ^~~~~~~
          d_alloc
   sound//usb/power.c:36:5: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     pd = kzalloc(sizeof(*pd), GFP_KERNEL);
        ^
>> sound//usb/power.c:59:2: error: implicit declaration of function 'kfree'; did you mean 'irq_free'? [-Werror=implicit-function-declaration]
     kfree(pd);
     ^~~~~
     irq_free
   cc1: some warnings being treated as errors

vim +59 sound//usb/power.c

    28	
    29	struct snd_usb_power_domain *
    30	snd_usb_find_power_domain(struct usb_host_interface *ctrl_iface,
    31				  unsigned char id)
    32	{
    33		struct snd_usb_power_domain *pd;
    34		void *p;
    35	
  > 36		pd = kzalloc(sizeof(*pd), GFP_KERNEL);
    37		if (!pd)
    38			return NULL;
    39	
    40		p = NULL;
    41		while ((p = snd_usb_find_csint_desc(ctrl_iface->extra,
    42						    ctrl_iface->extralen,
    43						    p, UAC3_POWER_DOMAIN)) != NULL) {
    44			struct uac3_power_domain_descriptor *pd_desc = p;
    45			int i;
    46	
    47			for (i = 0; i < pd_desc->bNrEntities; i++) {
    48				if (pd_desc->baEntityID[i] == id) {
    49					pd->pd_id = pd_desc->bPowerDomainID;
    50					pd->pd_d1d0_rec =
    51						le16_to_cpu(pd_desc->waRecoveryTime1);
    52					pd->pd_d2d0_rec =
    53						le16_to_cpu(pd_desc->waRecoveryTime2);
    54					return pd;
    55				}
    56			}
    57		}
    58	
  > 59		kfree(pd);
    60		return NULL;
    61	}
    62	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54307 bytes --]

Re: [PATCH 1/4] ALSA: usb-audio: Initial Power Domain support

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2719 bytes --]

Hi Jorge,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on sound/for-next]
[also build test ERROR on v4.18-rc5 next-20180719]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jorge-Sanjuan/usb-audio-Add-UAC3-Power-Domains/20180719-212009
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=sparc64 

All errors (new ones prefixed by >>):

   sound//usb/power.c: In function 'snd_usb_find_power_domain':
   sound//usb/power.c:36:7: error: implicit declaration of function 'kzalloc'; did you mean 'd_alloc'? [-Werror=implicit-function-declaration]
     pd = kzalloc(sizeof(*pd), GFP_KERNEL);
          ^~~~~~~
          d_alloc
   sound//usb/power.c:36:5: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     pd = kzalloc(sizeof(*pd), GFP_KERNEL);
        ^
>> sound//usb/power.c:59:2: error: implicit declaration of function 'kfree'; did you mean 'irq_free'? [-Werror=implicit-function-declaration]
     kfree(pd);
     ^~~~~
     irq_free
   cc1: some warnings being treated as errors

vim +59 sound//usb/power.c

    28	
    29	struct snd_usb_power_domain *
    30	snd_usb_find_power_domain(struct usb_host_interface *ctrl_iface,
    31				  unsigned char id)
    32	{
    33		struct snd_usb_power_domain *pd;
    34		void *p;
    35	
  > 36		pd = kzalloc(sizeof(*pd), GFP_KERNEL);
    37		if (!pd)
    38			return NULL;
    39	
    40		p = NULL;
    41		while ((p = snd_usb_find_csint_desc(ctrl_iface->extra,
    42						    ctrl_iface->extralen,
    43						    p, UAC3_POWER_DOMAIN)) != NULL) {
    44			struct uac3_power_domain_descriptor *pd_desc = p;
    45			int i;
    46	
    47			for (i = 0; i < pd_desc->bNrEntities; i++) {
    48				if (pd_desc->baEntityID[i] == id) {
    49					pd->pd_id = pd_desc->bPowerDomainID;
    50					pd->pd_d1d0_rec =
    51						le16_to_cpu(pd_desc->waRecoveryTime1);
    52					pd->pd_d2d0_rec =
    53						le16_to_cpu(pd_desc->waRecoveryTime2);
    54					return pd;
    55				}
    56			}
    57		}
    58	
  > 59		kfree(pd);
    60		return NULL;
    61	}
    62	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54307 bytes --]

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

[PATCH 2/4] ALSA: usb-audio: AudioStreaming Power Domain parsing

[Jorge Sanjuan]

Power Domains in the UAC3 spec are mainly intended to be
associated to an Input or Output Terminal so the host
changes the power state of the entire capture or playback
path within the topology.

This patch adds support for finding Power Domains associated
to an Audio Streaming Interface (bTerminalLink) and adds a
reference to them in the usb audio substreams (snd_usb_substream).

Signed-off-by: Jorge Sanjuan <jorge.sanjuan@codethink.co.uk>
---
 sound/usb/card.h   |  2 ++
 sound/usb/stream.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/sound/usb/card.h b/sound/usb/card.h
index 9b41b7dda84f..ac785d15ced4 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -37,6 +37,7 @@ struct audioformat {
 
 struct snd_usb_substream;
 struct snd_usb_endpoint;
+struct snd_usb_power_domain;
 
 struct snd_urb_ctx {
 	struct urb *urb;
@@ -115,6 +116,7 @@ struct snd_usb_substream {
 	int interface;	/* current interface */
 	int endpoint;	/* assigned endpoint */
 	struct audioformat *cur_audiofmt;	/* current audioformat pointer (for hw_params callback) */
+	struct snd_usb_power_domain *str_pd;	/* UAC3 Power Domain for streaming path */
 	snd_pcm_format_t pcm_format;	/* current audio format (for hw_params callback) */
 	unsigned int channels;		/* current number of channels (for hw_params callback) */
 	unsigned int channels_max;	/* max channels in the all audiofmts */
diff --git a/sound/usb/stream.c b/sound/usb/stream.c
index 729afd808cc4..031878a2a481 100644
--- a/sound/usb/stream.c
+++ b/sound/usb/stream.c
@@ -37,6 +37,7 @@
 #include "format.h"
 #include "clock.h"
 #include "stream.h"
+#include "power.h"
 
 /*
  * free a substream
@@ -53,6 +54,7 @@ static void free_substream(struct snd_usb_substream *subs)
 		kfree(fp);
 	}
 	kfree(subs->rate_list.list);
+	kfree(subs->str_pd);
 }
 
 
@@ -82,7 +84,8 @@ static void snd_usb_audio_pcm_free(struct snd_pcm *pcm)
 
 static void snd_usb_init_substream(struct snd_usb_stream *as,
 				   int stream,
-				   struct audioformat *fp)
+				   struct audioformat *fp,
+				   struct snd_usb_power_domain *pd)
 {
 	struct snd_usb_substream *subs = &as->substream[stream];
 
@@ -107,6 +110,9 @@ static void snd_usb_init_substream(struct snd_usb_stream *as,
 	if (fp->channels > subs->channels_max)
 		subs->channels_max = fp->channels;
 
+	if (pd)
+		subs->str_pd = pd;
+
 	snd_usb_preallocate_buffer(subs);
 }
 
@@ -468,9 +474,11 @@ snd_pcm_chmap_elem *convert_chmap_v3(struct uac3_cluster_header_descriptor
  * fmt_list and will be freed on the chip instance release. do not free
  * fp or do remove it from the substream fmt_list to avoid double-free.
  */
-int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
-			     int stream,
-			     struct audioformat *fp)
+static int __snd_usb_add_audio_stream(struct snd_usb_audio *chip,
+				      int stream,
+				      struct audioformat *fp,
+				      struct snd_usb_power_domain *pd)
+
 {
 	struct snd_usb_stream *as;
 	struct snd_usb_substream *subs;
@@ -498,7 +506,7 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
 		err = snd_pcm_new_stream(as->pcm, stream, 1);
 		if (err < 0)
 			return err;
-		snd_usb_init_substream(as, stream, fp);
+		snd_usb_init_substream(as, stream, fp, pd);
 		return add_chmap(as->pcm, stream, subs);
 	}
 
@@ -526,7 +534,7 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
 	else
 		strcpy(pcm->name, "USB Audio");
 
-	snd_usb_init_substream(as, stream, fp);
+	snd_usb_init_substream(as, stream, fp, pd);
 
 	/*
 	 * Keep using head insertion for M-Audio Audiophile USB (tm) which has a
@@ -544,6 +552,21 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
 	return add_chmap(pcm, stream, &as->substream[stream]);
 }
 
+int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
+			     int stream,
+			     struct audioformat *fp)
+{
+	return __snd_usb_add_audio_stream(chip, stream, fp, NULL);
+}
+
+int snd_usb_add_audio_stream_v3(struct snd_usb_audio *chip,
+				int stream,
+				struct audioformat *fp,
+				struct snd_usb_power_domain *pd)
+{
+	return __snd_usb_add_audio_stream(chip, stream, fp, pd);
+}
+
 static int parse_uac_endpoint_attributes(struct snd_usb_audio *chip,
 					 struct usb_host_interface *alts,
 					 int protocol, int iface_no)
@@ -819,6 +842,7 @@ snd_usb_get_audioformat_uac12(struct snd_usb_audio *chip,
 static struct audioformat *
 snd_usb_get_audioformat_uac3(struct snd_usb_audio *chip,
 			     struct usb_host_interface *alts,
+			     struct snd_usb_power_domain **pd_out,
 			     int iface_no, int altset_idx,
 			     int altno, int stream)
 {
@@ -829,6 +853,7 @@ snd_usb_get_audioformat_uac3(struct snd_usb_audio *chip,
 	struct uac3_as_header_descriptor *as = NULL;
 	struct uac3_hc_descriptor_header hc_header;
 	struct snd_pcm_chmap_elem *chmap;
+	struct snd_usb_power_domain *pd;
 	unsigned char badd_profile;
 	u64 badd_formats = 0;
 	unsigned int num_channels;
@@ -1008,12 +1033,28 @@ snd_usb_get_audioformat_uac3(struct snd_usb_audio *chip,
 		fp->rate_max = UAC3_BADD_SAMPLING_RATE;
 		fp->rates = SNDRV_PCM_RATE_CONTINUOUS;
 
+		pd = kzalloc(sizeof(pd), GFP_KERNEL);
+		if (!pd) {
+			kfree(fp->rate_table);
+			kfree(fp);
+			return NULL;
+		}
+		pd->pd_id = (stream == SNDRV_PCM_STREAM_PLAYBACK) ?
+					UAC3_BADD_PD_ID10 : UAC3_BADD_PD_ID11;
+		pd->pd_d1d0_rec = UAC3_BADD_PD_RECOVER_D1D0;
+		pd->pd_d2d0_rec = UAC3_BADD_PD_RECOVER_D2D0;
+
 	} else {
 		fp->attributes = parse_uac_endpoint_attributes(chip, alts,
 							       UAC_VERSION_3,
 							       iface_no);
+
+		pd = snd_usb_find_power_domain(chip->ctrl_intf,
+					       as->bTerminalLink);
+
 		/* ok, let's parse further... */
 		if (snd_usb_parse_audio_format_v3(chip, fp, as, stream) < 0) {
+			kfree(pd);
 			kfree(fp->chmap);
 			kfree(fp->rate_table);
 			kfree(fp);
@@ -1021,6 +1062,9 @@ snd_usb_get_audioformat_uac3(struct snd_usb_audio *chip,
 		}
 	}
 
+	if (pd)
+		*pd_out = pd;
+
 	return fp;
 }
 
@@ -1032,6 +1076,7 @@ int snd_usb_parse_audio_interface(struct snd_usb_audio *chip, int iface_no)
 	struct usb_interface_descriptor *altsd;
 	int i, altno, err, stream;
 	struct audioformat *fp = NULL;
+	struct snd_usb_power_domain *pd = NULL;
 	int num, protocol;
 
 	dev = chip->dev;
@@ -1114,7 +1159,7 @@ int snd_usb_parse_audio_interface(struct snd_usb_audio *chip, int iface_no)
 			break;
 		}
 		case UAC_VERSION_3:
-			fp = snd_usb_get_audioformat_uac3(chip, alts,
+			fp = snd_usb_get_audioformat_uac3(chip, alts, &pd,
 						iface_no, i, altno, stream);
 			break;
 		}
@@ -1125,9 +1170,14 @@ int snd_usb_parse_audio_interface(struct snd_usb_audio *chip, int iface_no)
 			return PTR_ERR(fp);
 
 		dev_dbg(&dev->dev, "%u:%d: add audio endpoint %#x\n", iface_no, altno, fp->endpoint);
-		err = snd_usb_add_audio_stream(chip, stream, fp);
+		if (protocol == UAC_VERSION_3)
+			err = snd_usb_add_audio_stream_v3(chip, stream, fp, pd);
+		else
+			err = snd_usb_add_audio_stream(chip, stream, fp);
+
 		if (err < 0) {
 			list_del(&fp->list); /* unlink for avoiding double-free */
+			kfree(pd);
 			kfree(fp->rate_table);
 			kfree(fp->chmap);
 			kfree(fp);
-- 
2.11.0

Re: [PATCH 2/4] ALSA: usb-audio: AudioStreaming Power Domain parsing

[kbuild test robot]

Hi Jorge,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on sound/for-next]
[also build test WARNING on v4.18-rc5 next-20180719]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jorge-Sanjuan/usb-audio-Add-UAC3-Power-Domains/20180719-212009
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> sound/usb/stream.c:562:5: sparse: symbol 'snd_usb_add_audio_stream_v3' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[RFC PATCH] ALSA: usb-audio: snd_usb_add_audio_stream_v3() can be static

[kbuild test robot]

Fixes: 69abc040c878 ("ALSA: usb-audio: AudioStreaming Power Domain parsing")
Signed-off-by: kbuild test robot <fengguang.wu@intel.com>
---
 stream.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/usb/stream.c b/sound/usb/stream.c
index 031878a..c0567fa1 100644
--- a/sound/usb/stream.c
+++ b/sound/usb/stream.c
@@ -559,10 +559,10 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
 	return __snd_usb_add_audio_stream(chip, stream, fp, NULL);
 }
 
-int snd_usb_add_audio_stream_v3(struct snd_usb_audio *chip,
-				int stream,
-				struct audioformat *fp,
-				struct snd_usb_power_domain *pd)
+static int snd_usb_add_audio_stream_v3(struct snd_usb_audio *chip,
+				       int stream,
+				       struct audioformat *fp,
+				       struct snd_usb_power_domain *pd)
 {
 	return __snd_usb_add_audio_stream(chip, stream, fp, pd);
 }

[PATCH 3/4] ALSA: usb-audio: Operate UAC3 Power Domains in PCM callbacks

[Jorge Sanjuan]

Make use of UAC3 Power Domains associated to an Audio Streaming
path within the PCM's logic. This means, when there is no audio
being transferred (pcm is closed), the host will set the Power Domain
associated to that substream to state D1. When audio is being transferred
(from hw_params onwards), the Power Domain will be set to D0 state.

This is the way the host lets the device now which Terminal
is going to be actively used and it is for the device to
manage its own internal resources on that UAC3 Power Domain.

Signed-off-by: Jorge Sanjuan <jorge.sanjuan@codethink.co.uk>
---
 sound/usb/pcm.c    | 34 +++++++++++++++++++++++++++++++---
 sound/usb/stream.c |  6 +++++-
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 4b930fa47277..0ae5f539706d 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -711,6 +711,24 @@ static int configure_endpoint(struct snd_usb_substream *subs)
 	return ret;
 }
 
+static int snd_usb_pcm_change_state(struct snd_usb_substream *subs, int state)
+{
+	int ret;
+
+	if (!subs->str_pd)
+		return 0;
+
+	ret = snd_usb_power_domain_set(subs->stream->chip, subs->str_pd, state);
+	if (ret < 0) {
+		dev_err(&subs->dev->dev,
+			"Cannot change Power Domain ID: %d to state: %d. Err: %d\n",
+			subs->str_pd->pd_id, state, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 /*
  * hw_params callback
  *
@@ -755,16 +773,22 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream,
 	ret = snd_usb_lock_shutdown(subs->stream->chip);
 	if (ret < 0)
 		return ret;
+
 	ret = set_format(subs, fmt);
-	snd_usb_unlock_shutdown(subs->stream->chip);
 	if (ret < 0)
-		return ret;
+		goto unlock;
+
+	ret = snd_usb_pcm_change_state(subs, UAC3_PD_STATE_D0);
+	if (ret < 0)
+		goto unlock;
 
 	subs->interface = fmt->iface;
 	subs->altset_idx = fmt->altset_idx;
 	subs->need_setup_ep = true;
 
-	return 0;
+ unlock:
+	snd_usb_unlock_shutdown(subs->stream->chip);
+	return ret;
 }
 
 /*
@@ -1265,6 +1289,7 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)
 	int direction = substream->stream;
 	struct snd_usb_stream *as = snd_pcm_substream_chip(substream);
 	struct snd_usb_substream *subs = &as->substream[direction];
+	int ret;
 
 	stop_endpoints(subs, true);
 
@@ -1273,7 +1298,10 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)
 	    !snd_usb_lock_shutdown(subs->stream->chip)) {
 		usb_set_interface(subs->dev, subs->interface, 0);
 		subs->interface = -1;
+		ret = snd_usb_pcm_change_state(subs, UAC3_PD_STATE_D1);
 		snd_usb_unlock_shutdown(subs->stream->chip);
+		if (ret < 0)
+			return ret;
 	}
 
 	subs->pcm_substream = NULL;
diff --git a/sound/usb/stream.c b/sound/usb/stream.c
index 031878a2a481..96402ca87aa5 100644
--- a/sound/usb/stream.c
+++ b/sound/usb/stream.c
@@ -110,8 +110,12 @@ static void snd_usb_init_substream(struct snd_usb_stream *as,
 	if (fp->channels > subs->channels_max)
 		subs->channels_max = fp->channels;
 
-	if (pd)
+	if (pd) {
 		subs->str_pd = pd;
+		/* Initialize Power Domain to idle status D1 */
+		snd_usb_power_domain_set(subs->stream->chip, pd,
+					 UAC3_PD_STATE_D1);
+	}
 
 	snd_usb_preallocate_buffer(subs);
 }
-- 
2.11.0

[PATCH 4/4] ALSA: usb-audio: Add UAC3 Power Domains to suspend/resume

[Jorge Sanjuan]

Set the UAC3 Power Domain state for an Audio Streaming interface
to D2 state before suspending the device (usb_driver callback).
This lets the device know there is no intention to use any of the
Units in the Audio Function and that the host is not going to
even listen for wake-up events (interrupts) on the units.

If the usb_driver gets resumed, the state D1 (idle) will be
set.

Signed-off-by: Jorge Sanjuan <jorge.sanjuan@codethink.co.uk>
---
 sound/usb/card.c |  9 +++++++++
 sound/usb/pcm.c  | 30 ++++++++++++++++++++++++++++++
 sound/usb/pcm.h  |  2 ++
 3 files changed, 41 insertions(+)

diff --git a/sound/usb/card.c b/sound/usb/card.c
index a1ed798a1c6b..9c2a15b805c3 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -808,6 +808,7 @@ static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message)
 		snd_power_change_state(chip->card, SNDRV_CTL_POWER_D3hot);
 	if (!chip->num_suspended_intf++) {
 		list_for_each_entry(as, &chip->pcm_list, list) {
+			snd_usb_pcm_suspend(as);
 			snd_pcm_suspend_all(as->pcm);
 			as->substream[0].need_setup_ep =
 				as->substream[1].need_setup_ep = true;
@@ -824,6 +825,7 @@ static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message)
 static int __usb_audio_resume(struct usb_interface *intf, bool reset_resume)
 {
 	struct snd_usb_audio *chip = usb_get_intfdata(intf);
+	struct snd_usb_stream *as;
 	struct usb_mixer_interface *mixer;
 	struct list_head *p;
 	int err = 0;
@@ -834,6 +836,13 @@ static int __usb_audio_resume(struct usb_interface *intf, bool reset_resume)
 		return 0;
 
 	atomic_inc(&chip->active); /* avoid autopm */
+
+	list_for_each_entry(as, &chip->pcm_list, list) {
+		err = snd_usb_pcm_resume(as);
+		if (err < 0)
+			goto err_out;
+	}
+
 	/*
 	 * ALSA leaves material resumption to user space
 	 * we just notify and restart the mixers
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 0ae5f539706d..266f7028d01b 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -729,6 +729,36 @@ static int snd_usb_pcm_change_state(struct snd_usb_substream *subs, int state)
 	return 0;
 }
 
+int snd_usb_pcm_suspend(struct snd_usb_stream *as)
+{
+	int ret;
+
+	ret = snd_usb_pcm_change_state(&as->substream[0], UAC3_PD_STATE_D2);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_usb_pcm_change_state(&as->substream[1], UAC3_PD_STATE_D2);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+int snd_usb_pcm_resume(struct snd_usb_stream *as)
+{
+	int ret;
+
+	ret = snd_usb_pcm_change_state(&as->substream[0], UAC3_PD_STATE_D1);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_usb_pcm_change_state(&as->substream[1], UAC3_PD_STATE_D1);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 /*
  * hw_params callback
  *
diff --git a/sound/usb/pcm.h b/sound/usb/pcm.h
index f77ec58bf1a1..9833627c1eca 100644
--- a/sound/usb/pcm.h
+++ b/sound/usb/pcm.h
@@ -6,6 +6,8 @@ snd_pcm_uframes_t snd_usb_pcm_delay(struct snd_usb_substream *subs,
 				    unsigned int rate);
 
 void snd_usb_set_pcm_ops(struct snd_pcm *pcm, int stream);
+int snd_usb_pcm_suspend(struct snd_usb_stream *as);
+int snd_usb_pcm_resume(struct snd_usb_stream *as);
 
 int snd_usb_init_pitch(struct snd_usb_audio *chip, int iface,
 		       struct usb_host_interface *alts,
-- 
2.11.0

Re: [PATCH 0/4] usb-audio: Add UAC3 Power Domains

[Takashi Iwai]

On Thu, 19 Jul 2018 13:22:11 +0200,
Jorge Sanjuan wrote:
> 
> This patchset add support for UAC3 Power Domains. This feature
> of the USB audio class 3 allows the host to notify the device 
> what it is making use of so power comsumption can be optimized.
> 
> This proposal implements this feature for Power Domains
> that include an Input/Output Terminal associated to an
> audio Streaming interface. This is the main usage of this
> feature according to the spec. For that reason, the logic
> for the Power Domain state change has been implemented
> within the ALSA PCMs logic and the suspend/resume callbacks
> of the usb_driver. The behaviour would be as follows:
> 
> * Power Domain State D0: A Power Domain will reach this state
>   only when the audio substream associated to that domain is
>   being used (i,e. Audio playback/capture is happening).
> * Power Domain State D1: This is the Idle state where the driver
>   is going to always want to be in order to reduce power
>   consumption.
> * Power Domain State D2: This state is only set when the usb driver
>   asumes the device is not going to be used anymore and hence, it
>   wont care about getting any interrupts from the device. This
>   will only happen when power level is set to "auto" in sysfs
>   so the usb driver gets suspended when the interfaces are not in use.
>  
> NOTE: The way this has been implemented will always try to put the 
> Power Domain in state D1 if the Power Domain exists so there is not a 
> way a user could disable this feature. It may be worth getting a control
> exposed to userland that enables/disables this feature (?).

Can it be tied with runtime PM?

Need to read through your patchset at first...


thanks,

Takashi


> Power Domains affecting other units independently are required to be 
> bypassed via a Selector Unit first before the host can change the 
> power state. This sceneario is not covered in this patchset.
> 
> based on next-20180719
> 
> Jorge Sanjuan (4):
>   ALSA: usb-audio: Initial Power Domain support
>   ALSA: usb-audio: AudioStreaming Power Domain parsing
>   ALSA: usb-audio: Operate UAC3 Power Domains in PCM callbacks
>   ALSA: usb-audio: Add UAC3 Power Domains to suspend/resume
> 
>  include/linux/usb/audio-v3.h |   4 ++
>  sound/usb/Makefile           |   1 +
>  sound/usb/card.c             |   9 ++++
>  sound/usb/card.h             |   2 +
>  sound/usb/pcm.c              |  64 +++++++++++++++++++++--
>  sound/usb/pcm.h              |   2 +
>  sound/usb/power.c            | 117 +++++++++++++++++++++++++++++++++++++++++++
>  sound/usb/power.h            |  19 +++++++
>  sound/usb/stream.c           |  70 +++++++++++++++++++++++---
>  9 files changed, 277 insertions(+), 11 deletions(-)
>  create mode 100644 sound/usb/power.c
> 
> -- 
> 2.11.0
> 

Re: [PATCH 0/4] usb-audio: Add UAC3 Power Domains

[Jorge]

On 19/07/18 12:56, Takashi Iwai wrote:
> On Thu, 19 Jul 2018 13:22:11 +0200,
> Jorge Sanjuan wrote:
>>
>> This patchset add support for UAC3 Power Domains. This feature
>> of the USB audio class 3 allows the host to notify the device
>> what it is making use of so power comsumption can be optimized.
>>
>> This proposal implements this feature for Power Domains
>> that include an Input/Output Terminal associated to an
>> audio Streaming interface. This is the main usage of this
>> feature according to the spec. For that reason, the logic
>> for the Power Domain state change has been implemented
>> within the ALSA PCMs logic and the suspend/resume callbacks
>> of the usb_driver. The behaviour would be as follows:
>>
>> * Power Domain State D0: A Power Domain will reach this state
>>    only when the audio substream associated to that domain is
>>    being used (i,e. Audio playback/capture is happening).
>> * Power Domain State D1: This is the Idle state where the driver
>>    is going to always want to be in order to reduce power
>>    consumption.
>> * Power Domain State D2: This state is only set when the usb driver
>>    asumes the device is not going to be used anymore and hence, it
>>    wont care about getting any interrupts from the device. This
>>    will only happen when power level is set to "auto" in sysfs
>>    so the usb driver gets suspended when the interfaces are not in use.
>>   
>> NOTE: The way this has been implemented will always try to put the
>> Power Domain in state D1 if the Power Domain exists so there is not a
>> way a user could disable this feature. It may be worth getting a control
>> exposed to userland that enables/disables this feature (?).
> 
> Can it be tied with runtime PM?

Sure. I think that could work. So the snd-usb driver would only attempt 
to drop a Power Domain from D0 to D1 only if (dev->power.runtime_auto == 
true)? Is there any clean way to check for that? I couldn't find any 
helper function.

The change to D2 state is already wrapped in runtime PM as usb_driver 
.suspend callback only gets called when runtime PM is enabled from sysfs.

Thanks,

Jorge

> 
> Need to read through your patchset at first...
> 
> 
> thanks,
> 
> Takashi
> 
> 
>> Power Domains affecting other units independently are required to be
>> bypassed via a Selector Unit first before the host can change the
>> power state. This sceneario is not covered in this patchset.
>>
>> based on next-20180719
>>
>> Jorge Sanjuan (4):
>>    ALSA: usb-audio: Initial Power Domain support
>>    ALSA: usb-audio: AudioStreaming Power Domain parsing
>>    ALSA: usb-audio: Operate UAC3 Power Domains in PCM callbacks
>>    ALSA: usb-audio: Add UAC3 Power Domains to suspend/resume
>>
>>   include/linux/usb/audio-v3.h |   4 ++
>>   sound/usb/Makefile           |   1 +
>>   sound/usb/card.c             |   9 ++++
>>   sound/usb/card.h             |   2 +
>>   sound/usb/pcm.c              |  64 +++++++++++++++++++++--
>>   sound/usb/pcm.h              |   2 +
>>   sound/usb/power.c            | 117 +++++++++++++++++++++++++++++++++++++++++++
>>   sound/usb/power.h            |  19 +++++++
>>   sound/usb/stream.c           |  70 +++++++++++++++++++++++---
>>   9 files changed, 277 insertions(+), 11 deletions(-)
>>   create mode 100644 sound/usb/power.c
>>
>> -- 
>> 2.11.0
>>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [alsa-devel] [PATCH 0/4] usb-audio: Add UAC3 Power Domains

[Jorge]

On 19/07/18 12:56, Takashi Iwai wrote:
> On Thu, 19 Jul 2018 13:22:11 +0200,
> Jorge Sanjuan wrote:
>>
>> This patchset add support for UAC3 Power Domains. This feature
>> of the USB audio class 3 allows the host to notify the device
>> what it is making use of so power comsumption can be optimized.
>>
>> This proposal implements this feature for Power Domains
>> that include an Input/Output Terminal associated to an
>> audio Streaming interface. This is the main usage of this
>> feature according to the spec. For that reason, the logic
>> for the Power Domain state change has been implemented
>> within the ALSA PCMs logic and the suspend/resume callbacks
>> of the usb_driver. The behaviour would be as follows:
>>
>> * Power Domain State D0: A Power Domain will reach this state
>>    only when the audio substream associated to that domain is
>>    being used (i,e. Audio playback/capture is happening).
>> * Power Domain State D1: This is the Idle state where the driver
>>    is going to always want to be in order to reduce power
>>    consumption.
>> * Power Domain State D2: This state is only set when the usb driver
>>    asumes the device is not going to be used anymore and hence, it
>>    wont care about getting any interrupts from the device. This
>>    will only happen when power level is set to "auto" in sysfs
>>    so the usb driver gets suspended when the interfaces are not in use.
>>   
>> NOTE: The way this has been implemented will always try to put the
>> Power Domain in state D1 if the Power Domain exists so there is not a
>> way a user could disable this feature. It may be worth getting a control
>> exposed to userland that enables/disables this feature (?).
> 
> Can it be tied with runtime PM?
> 
> Need to read through your patchset at first...
> 
> 
> thanks,
> 
> Takashi

Hi,

I just realized I accidentally only replied to alsa-devel.. Sorry about 
that.

I think it should be possible to tie up the D1 state changes (low power 
but still interrupt capable) to runtime PM. Changes to D2 are already 
tied to PM in this patchset. Just need to find the way to cleanly access 
`dev->power.runtime_auto` before the driver attempts to set D1 state. We 
could also let the driver only do D2<->D0 changes for now.

I got some kbuild errors due to missing include. Should I re-send this 
patchset or try to go around getting it tied up to runtime PM first?

Regards,

Jorge

> 
> 
>> Power Domains affecting other units independently are required to be
>> bypassed via a Selector Unit first before the host can change the
>> power state. This sceneario is not covered in this patchset.
>>
>> based on next-20180719
>>
>> Jorge Sanjuan (4):
>>    ALSA: usb-audio: Initial Power Domain support
>>    ALSA: usb-audio: AudioStreaming Power Domain parsing
>>    ALSA: usb-audio: Operate UAC3 Power Domains in PCM callbacks
>>    ALSA: usb-audio: Add UAC3 Power Domains to suspend/resume
>>
>>   include/linux/usb/audio-v3.h |   4 ++
>>   sound/usb/Makefile           |   1 +
>>   sound/usb/card.c             |   9 ++++
>>   sound/usb/card.h             |   2 +
>>   sound/usb/pcm.c              |  64 +++++++++++++++++++++--
>>   sound/usb/pcm.h              |   2 +
>>   sound/usb/power.c            | 117 +++++++++++++++++++++++++++++++++++++++++++
>>   sound/usb/power.h            |  19 +++++++
>>   sound/usb/stream.c           |  70 +++++++++++++++++++++++---
>>   9 files changed, 277 insertions(+), 11 deletions(-)
>>   create mode 100644 sound/usb/power.c
>>
>> -- 
>> 2.11.0
>>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [alsa-devel] [PATCH 0/4] usb-audio: Add UAC3 Power Domains

[Takashi Iwai]

On Fri, 27 Jul 2018 12:44:18 +0200,
Jorge wrote:
> 
> 
> 
> On 19/07/18 12:56, Takashi Iwai wrote:
> > On Thu, 19 Jul 2018 13:22:11 +0200,
> > Jorge Sanjuan wrote:
> >>
> >> This patchset add support for UAC3 Power Domains. This feature
> >> of the USB audio class 3 allows the host to notify the device
> >> what it is making use of so power comsumption can be optimized.
> >>
> >> This proposal implements this feature for Power Domains
> >> that include an Input/Output Terminal associated to an
> >> audio Streaming interface. This is the main usage of this
> >> feature according to the spec. For that reason, the logic
> >> for the Power Domain state change has been implemented
> >> within the ALSA PCMs logic and the suspend/resume callbacks
> >> of the usb_driver. The behaviour would be as follows:
> >>
> >> * Power Domain State D0: A Power Domain will reach this state
> >>    only when the audio substream associated to that domain is
> >>    being used (i,e. Audio playback/capture is happening).
> >> * Power Domain State D1: This is the Idle state where the driver
> >>    is going to always want to be in order to reduce power
> >>    consumption.
> >> * Power Domain State D2: This state is only set when the usb driver
> >>    asumes the device is not going to be used anymore and hence, it
> >>    wont care about getting any interrupts from the device. This
> >>    will only happen when power level is set to "auto" in sysfs
> >>    so the usb driver gets suspended when the interfaces are not in use.
> >>   NOTE: The way this has been implemented will always try to put
> >> the
> >> Power Domain in state D1 if the Power Domain exists so there is not a
> >> way a user could disable this feature. It may be worth getting a control
> >> exposed to userland that enables/disables this feature (?).
> >
> > Can it be tied with runtime PM?
> >
> > Need to read through your patchset at first...
> >
> >
> > thanks,
> >
> > Takashi
> 
> Hi,
> 
> I just realized I accidentally only replied to alsa-devel.. Sorry
> about that.

Thanks for resending.

> I think it should be possible to tie up the D1 state changes (low
> power but still interrupt capable) to runtime PM. Changes to D2 are
> already tied to PM in this patchset. Just need to find the way to
> cleanly access `dev->power.runtime_auto` before the driver attempts to
> set D1 state. We could also let the driver only do D2<->D0 changes for
> now.

Hm, OK, so the partial coverage looks feasible with the runtime PM
framework, at least.

> I got some kbuild errors due to missing include. Should I re-send this
> patchset or try to go around getting it tied up to runtime PM first?

Let's fix the easy issues with kbuild and get them merged.
The proper power state support can be implemented later.


thanks,

Takashi

Re: [alsa-devel] [PATCH 0/4] usb-audio: Add UAC3 Power Domains

[Takashi Iwai]

On Fri, 27 Jul 2018 12:44:18 +0200,
Jorge wrote:
> 
> 
> 
> On 19/07/18 12:56, Takashi Iwai wrote:
> > On Thu, 19 Jul 2018 13:22:11 +0200,
> > Jorge Sanjuan wrote:
> >>
> >> This patchset add support for UAC3 Power Domains. This feature
> >> of the USB audio class 3 allows the host to notify the device
> >> what it is making use of so power comsumption can be optimized.
> >>
> >> This proposal implements this feature for Power Domains
> >> that include an Input/Output Terminal associated to an
> >> audio Streaming interface. This is the main usage of this
> >> feature according to the spec. For that reason, the logic
> >> for the Power Domain state change has been implemented
> >> within the ALSA PCMs logic and the suspend/resume callbacks
> >> of the usb_driver. The behaviour would be as follows:
> >>
> >> * Power Domain State D0: A Power Domain will reach this state
> >>    only when the audio substream associated to that domain is
> >>    being used (i,e. Audio playback/capture is happening).
> >> * Power Domain State D1: This is the Idle state where the driver
> >>    is going to always want to be in order to reduce power
> >>    consumption.
> >> * Power Domain State D2: This state is only set when the usb driver
> >>    asumes the device is not going to be used anymore and hence, it
> >>    wont care about getting any interrupts from the device. This
> >>    will only happen when power level is set to "auto" in sysfs
> >>    so the usb driver gets suspended when the interfaces are not in use.
> >>   NOTE: The way this has been implemented will always try to put
> >> the
> >> Power Domain in state D1 if the Power Domain exists so there is not a
> >> way a user could disable this feature. It may be worth getting a control
> >> exposed to userland that enables/disables this feature (?).
> >
> > Can it be tied with runtime PM?
> >
> > Need to read through your patchset at first...
> >
> >
> > thanks,
> >
> > Takashi
> 
> Hi,
> 
> I just realized I accidentally only replied to alsa-devel.. Sorry
> about that.

Thanks for resending.

> I think it should be possible to tie up the D1 state changes (low
> power but still interrupt capable) to runtime PM. Changes to D2 are
> already tied to PM in this patchset. Just need to find the way to
> cleanly access `dev->power.runtime_auto` before the driver attempts to
> set D1 state. We could also let the driver only do D2<->D0 changes for
> now.

Hm, OK, so the partial coverage looks feasible with the runtime PM
framework, at least.

> I got some kbuild errors due to missing include. Should I re-send this
> patchset or try to go around getting it tied up to runtime PM first?

Let's fix the easy issues with kbuild and get them merged.
The proper power state support can be implemented later.


thanks,

Takashi

[PATCH v2 0/4] usb-audio: Add UAC3 Power Domains

[Jorge Sanjuan]

This is what's new in this v2:
 - Fixes build error reported by kbuild for multiple configs 
   (missing include of linux/slab.h).
 - Makes function `snd_usb_add_audio_stream_v3` static for now
   as no one is using it outside sound/usb/stream.c (suggested
   by kbuild).
 - Re-organization of patches so the bit that is not tied up
   to usb's runtime PM (See patch: "ALSA: usb-audio: Operate
   UAC3 Power Domains in PCM callbacks") is the last patch of
   the series. This is likely to need feature improvements to
   tie it up to runtime PM.

This patchset adds support for UAC3 Power Domains. This feature
of the USB audio class 3 allows the host to notify the device 
what it is making use of so power comsumption can be optimized.

This proposal implements this feature for Power Domains
that include an Input/Output Terminal associated to an
audio Streaming interface. This is the main usage of this
feature according to the spec. For that reason, the logic
for the Power Domain state change has been implemented
within the ALSA PCMs logic and the suspend/resume callbacks
of the usb_driver. The behaviour would be as follows:

* Power Domain State D0: A Power Domain will reach this state
  only when the audio substream associated to that domain is
  being used (i,e. Audio playback/capture is happening).
* Power Domain State D1: This is the Idle state where the driver
  is going to always want to be in order to reduce power
  consumption.
* Power Domain State D2: This state is only set when the usb driver
  asumes the device is not going to be used anymore and hence, it
  wont care about getting any interrupts from the device. This
  will only happen when power level is set to "auto" in sysfs
  so the usb driver gets suspended when the interfaces are not in use.
 
NOTE: The way this has been implemented will always try to put the 
Power Domain in state D1 if the Power Domain exists. The patch
"ALSA: usb-audio: Operate UAC3 Power Domains in PCM callbacks"
puts the logic for doing so inside the PCM's logic. Something to
improve on that is to also tie up those D1<->D0 state changes
to runtime PM maybe.

Jorge Sanjuan (4):
  ALSA: usb-audio: Initial Power Domain support
  ALSA: usb-audio: AudioStreaming Power Domain parsing
  ALSA: usb-audio: Add UAC3 Power Domains to suspend/resume
  ALSA: usb-audio: Operate UAC3 Power Domains in PCM callbacks

 include/linux/usb/audio-v3.h |   4 ++
 sound/usb/Makefile           |   1 +
 sound/usb/card.c             |   9 ++++
 sound/usb/card.h             |   2 +
 sound/usb/pcm.c              |  64 +++++++++++++++++++++--
 sound/usb/pcm.h              |   2 +
 sound/usb/power.c            | 118 +++++++++++++++++++++++++++++++++++++++++++
 sound/usb/power.h            |  19 +++++++
 sound/usb/stream.c           |  70 ++++++++++++++++++++++---
 9 files changed, 278 insertions(+), 11 deletions(-)
 create mode 100644 sound/usb/power.c

-- 
2.11.0

[PATCH v2 1/4] ALSA: usb-audio: Initial Power Domain support

[Jorge Sanjuan]

Thee USB Audio Class 3 (UAC3) introduces Power Domains as a new
feature to let a host turn individual parts of an audio function
to different power states via USB requests. This lets the device
get to know a bit amore about what the host is up to in order to
optimize power consumption efficiently.

The Power Domains are optional for UAC3 configuration but all
UAC3 devices shall include at least one BADD configuration where
the support for Power Domains is compulsory.

This patch adds a set of features/helpers to parse these power
domains and change their status.

Signed-off-by: Jorge Sanjuan <jorge.sanjuan@codethink.co.uk>
---
 include/linux/usb/audio-v3.h |   4 ++
 sound/usb/Makefile           |   1 +
 sound/usb/power.c            | 118 +++++++++++++++++++++++++++++++++++++++++++
 sound/usb/power.h            |  19 +++++++
 4 files changed, 142 insertions(+)
 create mode 100644 sound/usb/power.c

diff --git a/include/linux/usb/audio-v3.h b/include/linux/usb/audio-v3.h
index 334bfa6dfb47..786e5939d831 100644
--- a/include/linux/usb/audio-v3.h
+++ b/include/linux/usb/audio-v3.h
@@ -447,4 +447,8 @@ struct uac3_interrupt_data_msg {
 /* BADD sample rate is always fixed to 48kHz */
 #define UAC3_BADD_SAMPLING_RATE				48000
 
+/* BADD power domains recovery times */
+#define UAC3_BADD_PD_RECOVER_D1D0			0x0258
+#define UAC3_BADD_PD_RECOVER_D2D0			0x1770
+
 #endif /* __LINUX_USB_AUDIO_V3_H */
diff --git a/sound/usb/Makefile b/sound/usb/Makefile
index 05440e2df8d9..a4d69638b501 100644
--- a/sound/usb/Makefile
+++ b/sound/usb/Makefile
@@ -15,6 +15,7 @@ snd-usb-audio-objs := 	card.o \
 			pcm.o \
 			proc.o \
 			quirks.o \
+			power.o \
 			stream.o
 
 snd-usbmidi-lib-objs := midi.o
diff --git a/sound/usb/power.c b/sound/usb/power.c
new file mode 100644
index 000000000000..ce3fea2bd030
--- /dev/null
+++ b/sound/usb/power.c
@@ -0,0 +1,118 @@
+/*
+ *   UAC3 Power Domain state management functions
+ *
+ *   This program is free software; you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation; either version 2 of the License, or
+ *   (at your option) any later version.
+ *
+ *   This program is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *   GNU General Public License for more details.
+ *
+ *   You should have received a copy of the GNU General Public License
+ *   along with this program; if not, write to the Free Software
+ *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
+ *
+ */
+
+#include <linux/slab.h>
+#include <linux/usb.h>
+#include <linux/usb/audio.h>
+#include <linux/usb/audio-v2.h>
+#include <linux/usb/audio-v3.h>
+
+#include "usbaudio.h"
+#include "helper.h"
+#include "power.h"
+
+struct snd_usb_power_domain *
+snd_usb_find_power_domain(struct usb_host_interface *ctrl_iface,
+			  unsigned char id)
+{
+	struct snd_usb_power_domain *pd;
+	void *p;
+
+	pd = kzalloc(sizeof(*pd), GFP_KERNEL);
+	if (!pd)
+		return NULL;
+
+	p = NULL;
+	while ((p = snd_usb_find_csint_desc(ctrl_iface->extra,
+					    ctrl_iface->extralen,
+					    p, UAC3_POWER_DOMAIN)) != NULL) {
+		struct uac3_power_domain_descriptor *pd_desc = p;
+		int i;
+
+		for (i = 0; i < pd_desc->bNrEntities; i++) {
+			if (pd_desc->baEntityID[i] == id) {
+				pd->pd_id = pd_desc->bPowerDomainID;
+				pd->pd_d1d0_rec =
+					le16_to_cpu(pd_desc->waRecoveryTime1);
+				pd->pd_d2d0_rec =
+					le16_to_cpu(pd_desc->waRecoveryTime2);
+				return pd;
+			}
+		}
+	}
+
+	kfree(pd);
+	return NULL;
+}
+
+int snd_usb_power_domain_set(struct snd_usb_audio *chip,
+			     struct snd_usb_power_domain *pd,
+			     unsigned char state)
+{
+	struct usb_device *dev = chip->dev;
+	unsigned char current_state;
+	int err, idx;
+
+	idx = snd_usb_ctrl_intf(chip) | (pd->pd_id << 8);
+
+	err = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0),
+			      UAC2_CS_CUR,
+			      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
+			      UAC3_AC_POWER_DOMAIN_CONTROL << 8, idx,
+			      &current_state, sizeof(current_state));
+	if (err < 0) {
+		dev_err(&dev->dev, "Can't get UAC3 power state for id %d\n",
+			pd->pd_id);
+		return err;
+	}
+
+	if (current_state == state) {
+		dev_dbg(&dev->dev, "UAC3 power domain id %d already in state %d\n",
+			pd->pd_id, state);
+		return 0;
+	}
+
+	err = snd_usb_ctl_msg(chip->dev, usb_sndctrlpipe(chip->dev, 0), UAC2_CS_CUR,
+			      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_OUT,
+			      UAC3_AC_POWER_DOMAIN_CONTROL << 8, idx,
+			      &state, sizeof(state));
+	if (err < 0) {
+		dev_err(&dev->dev, "Can't set UAC3 power state to %d for id %d\n",
+			state, pd->pd_id);
+		return err;
+	}
+
+	if (state == UAC3_PD_STATE_D0) {
+		switch (current_state) {
+		case UAC3_PD_STATE_D2:
+			udelay(pd->pd_d2d0_rec * 50);
+			break;
+		case UAC3_PD_STATE_D1:
+			udelay(pd->pd_d1d0_rec * 50);
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	dev_dbg(&dev->dev, "UAC3 power domain id %d change to state %d\n",
+		pd->pd_id, state);
+
+	return 0;
+}
diff --git a/sound/usb/power.h b/sound/usb/power.h
index b2e25f60c5a2..6004231a7c75 100644
--- a/sound/usb/power.h
+++ b/sound/usb/power.h
@@ -2,6 +2,25 @@
 #ifndef __USBAUDIO_POWER_H
 #define __USBAUDIO_POWER_H
 
+struct snd_usb_power_domain {
+	int pd_id;              /* UAC3 Power Domain ID */
+	int pd_d1d0_rec;        /* D1 to D0 recovery time */
+	int pd_d2d0_rec;        /* D2 to D0 recovery time */
+};
+
+enum {
+	UAC3_PD_STATE_D0,
+	UAC3_PD_STATE_D1,
+	UAC3_PD_STATE_D2,
+};
+
+int snd_usb_power_domain_set(struct snd_usb_audio *chip,
+			     struct snd_usb_power_domain *pd,
+			     unsigned char state);
+struct snd_usb_power_domain *
+snd_usb_find_power_domain(struct usb_host_interface *ctrl_iface,
+			  unsigned char id);
+
 #ifdef CONFIG_PM
 int snd_usb_autoresume(struct snd_usb_audio *chip);
 void snd_usb_autosuspend(struct snd_usb_audio *chip);
-- 
2.11.0

Re: [PATCH v2 1/4] ALSA: usb-audio: Initial Power Domain support

[Takashi Iwai]

On Mon, 30 Jul 2018 11:23:33 +0200,
Jorge Sanjuan wrote:
> 
> diff --git a/sound/usb/power.c b/sound/usb/power.c
> new file mode 100644
> index 000000000000..ce3fea2bd030
> --- /dev/null
> +++ b/sound/usb/power.c
> @@ -0,0 +1,118 @@
> +/*
> + *   UAC3 Power Domain state management functions
> + *
> + *   This program is free software; you can redistribute it and/or modify
> + *   it under the terms of the GNU General Public License as published by
> + *   the Free Software Foundation; either version 2 of the License, or
> + *   (at your option) any later version.
> + *
> + *   This program is distributed in the hope that it will be useful,
> + *   but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *   GNU General Public License for more details.
> + *
> + *   You should have received a copy of the GNU General Public License
> + *   along with this program; if not, write to the Free Software
> + *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
> + *
> + */

For a new code, let's use the simpler SPDX form.


thanks,

Takashi

Re: [PATCH v2 1/4] ALSA: usb-audio: Initial Power Domain support

[Takashi Iwai]

On Mon, 30 Jul 2018 11:23:33 +0200,
Jorge Sanjuan wrote:
> 
> diff --git a/include/linux/usb/audio-v3.h b/include/linux/usb/audio-v3.h
> index 334bfa6dfb47..786e5939d831 100644
> --- a/include/linux/usb/audio-v3.h
> +++ b/include/linux/usb/audio-v3.h
> @@ -447,4 +447,8 @@ struct uac3_interrupt_data_msg {
>  /* BADD sample rate is always fixed to 48kHz */
>  #define UAC3_BADD_SAMPLING_RATE				48000
>  
> +/* BADD power domains recovery times */
> +#define UAC3_BADD_PD_RECOVER_D1D0			0x0258
> +#define UAC3_BADD_PD_RECOVER_D2D0			0x1770

Please put the unit for these values.  I guess they don't need to be
hex?


Takashi

Re: [PATCH v2 1/4] ALSA: usb-audio: Initial Power Domain support

[Jorge]

On 30/07/18 14:03, Takashi Iwai wrote:
> On Mon, 30 Jul 2018 11:23:33 +0200,
> Jorge Sanjuan wrote:
>>
>> diff --git a/include/linux/usb/audio-v3.h b/include/linux/usb/audio-v3.h
>> index 334bfa6dfb47..786e5939d831 100644
>> --- a/include/linux/usb/audio-v3.h
>> +++ b/include/linux/usb/audio-v3.h
>> @@ -447,4 +447,8 @@ struct uac3_interrupt_data_msg {
>>  /* BADD sample rate is always fixed to 48kHz */
>>  #define UAC3_BADD_SAMPLING_RATE				48000
>>  
>> +/* BADD power domains recovery times */
>> +#define UAC3_BADD_PD_RECOVER_D1D0			0x0258
>> +#define UAC3_BADD_PD_RECOVER_D2D0			0x1770
> 
> Please put the unit for these values.  I guess they don't need to be
> hex?
> 

Hi!

The BADD spec defines these inferred values as hex (see section 6.2.2.13
of the spec). Should we keep them as per spec to avoid confusion? I'll
update comment there with the units (50 us increments).

Thanks for the reviews today. I'll update this series soon with the
suggested changes.

Jorge

> 
> Takashi
> 

Re: [PATCH v2 1/4] ALSA: usb-audio: Initial Power Domain support

[Takashi Iwai]

On Mon, 30 Jul 2018 18:05:47 +0200,
Jorge wrote:
> 
> 
> 
> On 30/07/18 14:03, Takashi Iwai wrote:
> > On Mon, 30 Jul 2018 11:23:33 +0200,
> > Jorge Sanjuan wrote:
> >>
> >> diff --git a/include/linux/usb/audio-v3.h b/include/linux/usb/audio-v3.h
> >> index 334bfa6dfb47..786e5939d831 100644
> >> --- a/include/linux/usb/audio-v3.h
> >> +++ b/include/linux/usb/audio-v3.h
> >> @@ -447,4 +447,8 @@ struct uac3_interrupt_data_msg {
> >>  /* BADD sample rate is always fixed to 48kHz */
> >>  #define UAC3_BADD_SAMPLING_RATE				48000
> >>  
> >> +/* BADD power domains recovery times */
> >> +#define UAC3_BADD_PD_RECOVER_D1D0			0x0258
> >> +#define UAC3_BADD_PD_RECOVER_D2D0			0x1770
> > 
> > Please put the unit for these values.  I guess they don't need to be
> > hex?
> > 
> 
> Hi!
> 
> The BADD spec defines these inferred values as hex (see section 6.2.2.13
> of the spec). Should we keep them as per spec to avoid confusion? I'll
> update comment there with the units (50 us increments).

Well, if they were defined in hex, it makes sense to keep the raw
values as is.  But it'd be also helpful to give a comment showing the
actual usecs.


thanks,

Takashi

[PATCH v2 2/4] ALSA: usb-audio: AudioStreaming Power Domain parsing

[Jorge Sanjuan]

Power Domains in the UAC3 spec are mainly intended to be
associated to an Input or Output Terminal so the host
changes the power state of the entire capture or playback
path within the topology.

This patch adds support for finding Power Domains associated
to an Audio Streaming Interface (bTerminalLink) and adds a
reference to them in the usb audio substreams (snd_usb_substream).

Signed-off-by: Jorge Sanjuan <jorge.sanjuan@codethink.co.uk>
---
 sound/usb/card.h   |  2 ++
 sound/usb/stream.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/sound/usb/card.h b/sound/usb/card.h
index 9b41b7dda84f..ac785d15ced4 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -37,6 +37,7 @@ struct audioformat {
 
 struct snd_usb_substream;
 struct snd_usb_endpoint;
+struct snd_usb_power_domain;
 
 struct snd_urb_ctx {
 	struct urb *urb;
@@ -115,6 +116,7 @@ struct snd_usb_substream {
 	int interface;	/* current interface */
 	int endpoint;	/* assigned endpoint */
 	struct audioformat *cur_audiofmt;	/* current audioformat pointer (for hw_params callback) */
+	struct snd_usb_power_domain *str_pd;	/* UAC3 Power Domain for streaming path */
 	snd_pcm_format_t pcm_format;	/* current audio format (for hw_params callback) */
 	unsigned int channels;		/* current number of channels (for hw_params callback) */
 	unsigned int channels_max;	/* max channels in the all audiofmts */
diff --git a/sound/usb/stream.c b/sound/usb/stream.c
index 729afd808cc4..c0567fa1e84b 100644
--- a/sound/usb/stream.c
+++ b/sound/usb/stream.c
@@ -37,6 +37,7 @@
 #include "format.h"
 #include "clock.h"
 #include "stream.h"
+#include "power.h"
 
 /*
  * free a substream
@@ -53,6 +54,7 @@ static void free_substream(struct snd_usb_substream *subs)
 		kfree(fp);
 	}
 	kfree(subs->rate_list.list);
+	kfree(subs->str_pd);
 }
 
 
@@ -82,7 +84,8 @@ static void snd_usb_audio_pcm_free(struct snd_pcm *pcm)
 
 static void snd_usb_init_substream(struct snd_usb_stream *as,
 				   int stream,
-				   struct audioformat *fp)
+				   struct audioformat *fp,
+				   struct snd_usb_power_domain *pd)
 {
 	struct snd_usb_substream *subs = &as->substream[stream];
 
@@ -107,6 +110,9 @@ static void snd_usb_init_substream(struct snd_usb_stream *as,
 	if (fp->channels > subs->channels_max)
 		subs->channels_max = fp->channels;
 
+	if (pd)
+		subs->str_pd = pd;
+
 	snd_usb_preallocate_buffer(subs);
 }
 
@@ -468,9 +474,11 @@ snd_pcm_chmap_elem *convert_chmap_v3(struct uac3_cluster_header_descriptor
  * fmt_list and will be freed on the chip instance release. do not free
  * fp or do remove it from the substream fmt_list to avoid double-free.
  */
-int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
-			     int stream,
-			     struct audioformat *fp)
+static int __snd_usb_add_audio_stream(struct snd_usb_audio *chip,
+				      int stream,
+				      struct audioformat *fp,
+				      struct snd_usb_power_domain *pd)
+
 {
 	struct snd_usb_stream *as;
 	struct snd_usb_substream *subs;
@@ -498,7 +506,7 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
 		err = snd_pcm_new_stream(as->pcm, stream, 1);
 		if (err < 0)
 			return err;
-		snd_usb_init_substream(as, stream, fp);
+		snd_usb_init_substream(as, stream, fp, pd);
 		return add_chmap(as->pcm, stream, subs);
 	}
 
@@ -526,7 +534,7 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
 	else
 		strcpy(pcm->name, "USB Audio");
 
-	snd_usb_init_substream(as, stream, fp);
+	snd_usb_init_substream(as, stream, fp, pd);
 
 	/*
 	 * Keep using head insertion for M-Audio Audiophile USB (tm) which has a
@@ -544,6 +552,21 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
 	return add_chmap(pcm, stream, &as->substream[stream]);
 }
 
+int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
+			     int stream,
+			     struct audioformat *fp)
+{
+	return __snd_usb_add_audio_stream(chip, stream, fp, NULL);
+}
+
+static int snd_usb_add_audio_stream_v3(struct snd_usb_audio *chip,
+				       int stream,
+				       struct audioformat *fp,
+				       struct snd_usb_power_domain *pd)
+{
+	return __snd_usb_add_audio_stream(chip, stream, fp, pd);
+}
+
 static int parse_uac_endpoint_attributes(struct snd_usb_audio *chip,
 					 struct usb_host_interface *alts,
 					 int protocol, int iface_no)
@@ -819,6 +842,7 @@ snd_usb_get_audioformat_uac12(struct snd_usb_audio *chip,
 static struct audioformat *
 snd_usb_get_audioformat_uac3(struct snd_usb_audio *chip,
 			     struct usb_host_interface *alts,
+			     struct snd_usb_power_domain **pd_out,
 			     int iface_no, int altset_idx,
 			     int altno, int stream)
 {
@@ -829,6 +853,7 @@ snd_usb_get_audioformat_uac3(struct snd_usb_audio *chip,
 	struct uac3_as_header_descriptor *as = NULL;
 	struct uac3_hc_descriptor_header hc_header;
 	struct snd_pcm_chmap_elem *chmap;
+	struct snd_usb_power_domain *pd;
 	unsigned char badd_profile;
 	u64 badd_formats = 0;
 	unsigned int num_channels;
@@ -1008,12 +1033,28 @@ snd_usb_get_audioformat_uac3(struct snd_usb_audio *chip,
 		fp->rate_max = UAC3_BADD_SAMPLING_RATE;
 		fp->rates = SNDRV_PCM_RATE_CONTINUOUS;
 
+		pd = kzalloc(sizeof(pd), GFP_KERNEL);
+		if (!pd) {
+			kfree(fp->rate_table);
+			kfree(fp);
+			return NULL;
+		}
+		pd->pd_id = (stream == SNDRV_PCM_STREAM_PLAYBACK) ?
+					UAC3_BADD_PD_ID10 : UAC3_BADD_PD_ID11;
+		pd->pd_d1d0_rec = UAC3_BADD_PD_RECOVER_D1D0;
+		pd->pd_d2d0_rec = UAC3_BADD_PD_RECOVER_D2D0;
+
 	} else {
 		fp->attributes = parse_uac_endpoint_attributes(chip, alts,
 							       UAC_VERSION_3,
 							       iface_no);
+
+		pd = snd_usb_find_power_domain(chip->ctrl_intf,
+					       as->bTerminalLink);
+
 		/* ok, let's parse further... */
 		if (snd_usb_parse_audio_format_v3(chip, fp, as, stream) < 0) {
+			kfree(pd);
 			kfree(fp->chmap);
 			kfree(fp->rate_table);
 			kfree(fp);
@@ -1021,6 +1062,9 @@ snd_usb_get_audioformat_uac3(struct snd_usb_audio *chip,
 		}
 	}
 
+	if (pd)
+		*pd_out = pd;
+
 	return fp;
 }
 
@@ -1032,6 +1076,7 @@ int snd_usb_parse_audio_interface(struct snd_usb_audio *chip, int iface_no)
 	struct usb_interface_descriptor *altsd;
 	int i, altno, err, stream;
 	struct audioformat *fp = NULL;
+	struct snd_usb_power_domain *pd = NULL;
 	int num, protocol;
 
 	dev = chip->dev;
@@ -1114,7 +1159,7 @@ int snd_usb_parse_audio_interface(struct snd_usb_audio *chip, int iface_no)
 			break;
 		}
 		case UAC_VERSION_3:
-			fp = snd_usb_get_audioformat_uac3(chip, alts,
+			fp = snd_usb_get_audioformat_uac3(chip, alts, &pd,
 						iface_no, i, altno, stream);
 			break;
 		}
@@ -1125,9 +1170,14 @@ int snd_usb_parse_audio_interface(struct snd_usb_audio *chip, int iface_no)
 			return PTR_ERR(fp);
 
 		dev_dbg(&dev->dev, "%u:%d: add audio endpoint %#x\n", iface_no, altno, fp->endpoint);
-		err = snd_usb_add_audio_stream(chip, stream, fp);
+		if (protocol == UAC_VERSION_3)
+			err = snd_usb_add_audio_stream_v3(chip, stream, fp, pd);
+		else
+			err = snd_usb_add_audio_stream(chip, stream, fp);
+
 		if (err < 0) {
 			list_del(&fp->list); /* unlink for avoiding double-free */
+			kfree(pd);
 			kfree(fp->rate_table);
 			kfree(fp->chmap);
 			kfree(fp);
-- 
2.11.0

[PATCH v2 3/4] ALSA: usb-audio: Add UAC3 Power Domains to suspend/resume

[Jorge Sanjuan]

Set the UAC3 Power Domain state for an Audio Streaming interface
to D2 state before suspending the device (usb_driver callback).
This lets the device know there is no intention to use any of the
Units in the Audio Function and that the host is not going to
even listen for wake-up events (interrupts) on the units.

When the usb_driver gets resumed, the state D0 (fully powered) will
be set. This ties up the UAC3 Power Domains to the runtime PM.

Signed-off-by: Jorge Sanjuan <jorge.sanjuan@codethink.co.uk>
---
 sound/usb/card.c |  9 +++++++++
 sound/usb/pcm.c  | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 sound/usb/pcm.h  |  2 ++
 3 files changed, 59 insertions(+)

diff --git a/sound/usb/card.c b/sound/usb/card.c
index a1ed798a1c6b..9c2a15b805c3 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -808,6 +808,7 @@ static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message)
 		snd_power_change_state(chip->card, SNDRV_CTL_POWER_D3hot);
 	if (!chip->num_suspended_intf++) {
 		list_for_each_entry(as, &chip->pcm_list, list) {
+			snd_usb_pcm_suspend(as);
 			snd_pcm_suspend_all(as->pcm);
 			as->substream[0].need_setup_ep =
 				as->substream[1].need_setup_ep = true;
@@ -824,6 +825,7 @@ static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message)
 static int __usb_audio_resume(struct usb_interface *intf, bool reset_resume)
 {
 	struct snd_usb_audio *chip = usb_get_intfdata(intf);
+	struct snd_usb_stream *as;
 	struct usb_mixer_interface *mixer;
 	struct list_head *p;
 	int err = 0;
@@ -834,6 +836,13 @@ static int __usb_audio_resume(struct usb_interface *intf, bool reset_resume)
 		return 0;
 
 	atomic_inc(&chip->active); /* avoid autopm */
+
+	list_for_each_entry(as, &chip->pcm_list, list) {
+		err = snd_usb_pcm_resume(as);
+		if (err < 0)
+			goto err_out;
+	}
+
 	/*
 	 * ALSA leaves material resumption to user space
 	 * we just notify and restart the mixers
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 4b930fa47277..99ec9d5caa58 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -711,6 +711,54 @@ static int configure_endpoint(struct snd_usb_substream *subs)
 	return ret;
 }
 
+static int snd_usb_pcm_change_state(struct snd_usb_substream *subs, int state)
+{
+	int ret;
+
+	if (!subs->str_pd)
+		return 0;
+
+	ret = snd_usb_power_domain_set(subs->stream->chip, subs->str_pd, state);
+	if (ret < 0) {
+		dev_err(&subs->dev->dev,
+			"Cannot change Power Domain ID: %d to state: %d. Err: %d\n",
+			subs->str_pd->pd_id, state, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+int snd_usb_pcm_suspend(struct snd_usb_stream *as)
+{
+	int ret;
+
+	ret = snd_usb_pcm_change_state(&as->substream[0], UAC3_PD_STATE_D2);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_usb_pcm_change_state(&as->substream[1], UAC3_PD_STATE_D2);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+int snd_usb_pcm_resume(struct snd_usb_stream *as)
+{
+	int ret;
+
+	ret = snd_usb_pcm_change_state(&as->substream[0], UAC3_PD_STATE_D0);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_usb_pcm_change_state(&as->substream[1], UAC3_PD_STATE_D0);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 /*
  * hw_params callback
  *
diff --git a/sound/usb/pcm.h b/sound/usb/pcm.h
index f77ec58bf1a1..9833627c1eca 100644
--- a/sound/usb/pcm.h
+++ b/sound/usb/pcm.h
@@ -6,6 +6,8 @@ snd_pcm_uframes_t snd_usb_pcm_delay(struct snd_usb_substream *subs,
 				    unsigned int rate);
 
 void snd_usb_set_pcm_ops(struct snd_pcm *pcm, int stream);
+int snd_usb_pcm_suspend(struct snd_usb_stream *as);
+int snd_usb_pcm_resume(struct snd_usb_stream *as);
 
 int snd_usb_init_pitch(struct snd_usb_audio *chip, int iface,
 		       struct usb_host_interface *alts,
-- 
2.11.0

Re: [PATCH v2 3/4] ALSA: usb-audio: Add UAC3 Power Domains to suspend/resume

[Takashi Iwai]

On Mon, 30 Jul 2018 11:23:35 +0200,
Jorge Sanjuan wrote:
> 
> Set the UAC3 Power Domain state for an Audio Streaming interface
> to D2 state before suspending the device (usb_driver callback).
> This lets the device know there is no intention to use any of the
> Units in the Audio Function and that the host is not going to
> even listen for wake-up events (interrupts) on the units.
> 
> When the usb_driver gets resumed, the state D0 (fully powered) will
> be set. This ties up the UAC3 Power Domains to the runtime PM.
> 
> Signed-off-by: Jorge Sanjuan <jorge.sanjuan@codethink.co.uk>
> ---
>  sound/usb/card.c |  9 +++++++++
>  sound/usb/pcm.c  | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  sound/usb/pcm.h  |  2 ++
>  3 files changed, 59 insertions(+)
> 
> diff --git a/sound/usb/card.c b/sound/usb/card.c
> index a1ed798a1c6b..9c2a15b805c3 100644
> --- a/sound/usb/card.c
> +++ b/sound/usb/card.c
> @@ -808,6 +808,7 @@ static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message)
>  		snd_power_change_state(chip->card, SNDRV_CTL_POWER_D3hot);
>  	if (!chip->num_suspended_intf++) {
>  		list_for_each_entry(as, &chip->pcm_list, list) {
> +			snd_usb_pcm_suspend(as);
>  			snd_pcm_suspend_all(as->pcm);

The order of the call is doubtful.  Supposing that
snd_usb_pcm_suspend() makes the stream in D2 state, it should be done
after actually stopping the stream.  That is, it should be done after
snd_pcm_suspend_all().


thanks,

Takashi

[PATCH v2 4/4] ALSA: usb-audio: Operate UAC3 Power Domains in PCM callbacks

[Jorge Sanjuan]

Make use of UAC3 Power Domains associated to an Audio Streaming
path within the PCM's logic. This means, when there is no audio
being transferred (pcm is closed), the host will set the Power Domain
associated to that substream to state D1. When audio is being transferred
(from hw_params onwards), the Power Domain will be set to D0 state.

This is the way the host lets the device know which Terminal
is going to be actively used and it is for the device to
manage its own internal resources on that UAC3 Power Domain.

Note the resume method now sets the Power Domain to D1 state as
resuming the device doesn't mean audio streaming will occur.

Signed-off-by: Jorge Sanjuan <jorge.sanjuan@codethink.co.uk>
---
 sound/usb/pcm.c    | 20 +++++++++++++++-----
 sound/usb/stream.c |  6 +++++-
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 99ec9d5caa58..266f7028d01b 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -748,11 +748,11 @@ int snd_usb_pcm_resume(struct snd_usb_stream *as)
 {
 	int ret;
 
-	ret = snd_usb_pcm_change_state(&as->substream[0], UAC3_PD_STATE_D0);
+	ret = snd_usb_pcm_change_state(&as->substream[0], UAC3_PD_STATE_D1);
 	if (ret < 0)
 		return ret;
 
-	ret = snd_usb_pcm_change_state(&as->substream[1], UAC3_PD_STATE_D0);
+	ret = snd_usb_pcm_change_state(&as->substream[1], UAC3_PD_STATE_D1);
 	if (ret < 0)
 		return ret;
 
@@ -803,16 +803,22 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream,
 	ret = snd_usb_lock_shutdown(subs->stream->chip);
 	if (ret < 0)
 		return ret;
+
 	ret = set_format(subs, fmt);
-	snd_usb_unlock_shutdown(subs->stream->chip);
 	if (ret < 0)
-		return ret;
+		goto unlock;
+
+	ret = snd_usb_pcm_change_state(subs, UAC3_PD_STATE_D0);
+	if (ret < 0)
+		goto unlock;
 
 	subs->interface = fmt->iface;
 	subs->altset_idx = fmt->altset_idx;
 	subs->need_setup_ep = true;
 
-	return 0;
+ unlock:
+	snd_usb_unlock_shutdown(subs->stream->chip);
+	return ret;
 }
 
 /*
@@ -1313,6 +1319,7 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)
 	int direction = substream->stream;
 	struct snd_usb_stream *as = snd_pcm_substream_chip(substream);
 	struct snd_usb_substream *subs = &as->substream[direction];
+	int ret;
 
 	stop_endpoints(subs, true);
 
@@ -1321,7 +1328,10 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)
 	    !snd_usb_lock_shutdown(subs->stream->chip)) {
 		usb_set_interface(subs->dev, subs->interface, 0);
 		subs->interface = -1;
+		ret = snd_usb_pcm_change_state(subs, UAC3_PD_STATE_D1);
 		snd_usb_unlock_shutdown(subs->stream->chip);
+		if (ret < 0)
+			return ret;
 	}
 
 	subs->pcm_substream = NULL;
diff --git a/sound/usb/stream.c b/sound/usb/stream.c
index c0567fa1e84b..8fe3b0e00e45 100644
--- a/sound/usb/stream.c
+++ b/sound/usb/stream.c
@@ -110,8 +110,12 @@ static void snd_usb_init_substream(struct snd_usb_stream *as,
 	if (fp->channels > subs->channels_max)
 		subs->channels_max = fp->channels;
 
-	if (pd)
+	if (pd) {
 		subs->str_pd = pd;
+		/* Initialize Power Domain to idle status D1 */
+		snd_usb_power_domain_set(subs->stream->chip, pd,
+					 UAC3_PD_STATE_D1);
+	}
 
 	snd_usb_preallocate_buffer(subs);
 }
-- 
2.11.0

Re: [PATCH v2 4/4] ALSA: usb-audio: Operate UAC3 Power Domains in PCM callbacks

[Takashi Iwai]

On Mon, 30 Jul 2018 11:23:36 +0200,
Jorge Sanjuan wrote:
> 
> Make use of UAC3 Power Domains associated to an Audio Streaming
> path within the PCM's logic. This means, when there is no audio
> being transferred (pcm is closed), the host will set the Power Domain
> associated to that substream to state D1. When audio is being transferred
> (from hw_params onwards), the Power Domain will be set to D0 state.
> 
> This is the way the host lets the device know which Terminal
> is going to be actively used and it is for the device to
> manage its own internal resources on that UAC3 Power Domain.
> 
> Note the resume method now sets the Power Domain to D1 state as
> resuming the device doesn't mean audio streaming will occur.

I guess we need the power state transition to D0 also in prepare
callback.  The recovery from suspend doesn't need hw_params call but
just prepare -> trigger.

One could think of implementing into the trigger, but since the
transition needs some delay, prepare callback would be a better
choice, as it seems.


thanks,

Takashi

Re: [PATCH v2 4/4] ALSA: usb-audio: Operate UAC3 Power Domains in PCM callbacks

[Jorge]

On 30/07/18 14:13, Takashi Iwai wrote:
> On Mon, 30 Jul 2018 11:23:36 +0200,
> Jorge Sanjuan wrote:
>>
>> Make use of UAC3 Power Domains associated to an Audio Streaming
>> path within the PCM's logic. This means, when there is no audio
>> being transferred (pcm is closed), the host will set the Power Domain
>> associated to that substream to state D1. When audio is being transferred
>> (from hw_params onwards), the Power Domain will be set to D0 state.
>>
>> This is the way the host lets the device know which Terminal
>> is going to be actively used and it is for the device to
>> manage its own internal resources on that UAC3 Power Domain.
>>
>> Note the resume method now sets the Power Domain to D1 state as
>> resuming the device doesn't mean audio streaming will occur.
> 
> I guess we need the power state transition to D0 also in prepare
> callback.  The recovery from suspend doesn't need hw_params call but
> just prepare -> trigger.

Right! Shouldn't it then be enough to just go to D0 on .prepare? I,e.
Move the state transition from .hw_params to .prepare.

Jorge

> 
> One could think of implementing into the trigger, but since the
> transition needs some delay, prepare callback would be a better
> choice, as it seems.
> 
> 
> thanks,
> 
> Takashi
> 

-- 
Jorge Sanjuan, Software Engineer                         Codethink Ltd
http://www.codethink.co.uk/
We respect your privacy.   See https://www.codethink.co.uk/privacy.html

Re: [PATCH v2 4/4] ALSA: usb-audio: Operate UAC3 Power Domains in PCM callbacks

[Takashi Iwai]

On Mon, 30 Jul 2018 18:09:38 +0200,
Jorge wrote:
> 
> 
> 
> On 30/07/18 14:13, Takashi Iwai wrote:
> > On Mon, 30 Jul 2018 11:23:36 +0200,
> > Jorge Sanjuan wrote:
> >>
> >> Make use of UAC3 Power Domains associated to an Audio Streaming
> >> path within the PCM's logic. This means, when there is no audio
> >> being transferred (pcm is closed), the host will set the Power Domain
> >> associated to that substream to state D1. When audio is being transferred
> >> (from hw_params onwards), the Power Domain will be set to D0 state.
> >>
> >> This is the way the host lets the device know which Terminal
> >> is going to be actively used and it is for the device to
> >> manage its own internal resources on that UAC3 Power Domain.
> >>
> >> Note the resume method now sets the Power Domain to D1 state as
> >> resuming the device doesn't mean audio streaming will occur.
> > 
> > I guess we need the power state transition to D0 also in prepare
> > callback.  The recovery from suspend doesn't need hw_params call but
> > just prepare -> trigger.
> 
> Right! Shouldn't it then be enough to just go to D0 on .prepare? I,e.
> Move the state transition from .hw_params to .prepare.

Does the power domain transition needed for setting the format, EP,
etc done in set_format()?  If yes, we need D0 in hw_parmas as well.


thanks,

Takashi

Re: [PATCH v2 4/4] ALSA: usb-audio: Operate UAC3 Power Domains in PCM callbacks

[Jorge]

On 30/07/18 17:12, Takashi Iwai wrote:
> On Mon, 30 Jul 2018 18:09:38 +0200,
> Jorge wrote:
>>
>>
>>
>> On 30/07/18 14:13, Takashi Iwai wrote:
>>> On Mon, 30 Jul 2018 11:23:36 +0200,
>>> Jorge Sanjuan wrote:
>>>>
>>>> Make use of UAC3 Power Domains associated to an Audio Streaming
>>>> path within the PCM's logic. This means, when there is no audio
>>>> being transferred (pcm is closed), the host will set the Power Domain
>>>> associated to that substream to state D1. When audio is being transferred
>>>> (from hw_params onwards), the Power Domain will be set to D0 state.
>>>>
>>>> This is the way the host lets the device know which Terminal
>>>> is going to be actively used and it is for the device to
>>>> manage its own internal resources on that UAC3 Power Domain.
>>>>
>>>> Note the resume method now sets the Power Domain to D1 state as
>>>> resuming the device doesn't mean audio streaming will occur.
>>>
>>> I guess we need the power state transition to D0 also in prepare
>>> callback.  The recovery from suspend doesn't need hw_params call but
>>> just prepare -> trigger.
>>
>> Right! Shouldn't it then be enough to just go to D0 on .prepare? I,e.
>> Move the state transition from .hw_params to .prepare.
> 
> Does the power domain transition needed for setting the format, EP,
> etc done in set_format()?  If yes, we need D0 in hw_parmas as well.

Ok. That's a good point. The Power Domain will affect the USB streaming
terminals so a device may implement this in a way so it is not capable
to set_formats properly. I'll make sure the Power Domain is always set
to D0 *before* attempting to set_format.

Thanks!

Jorge

> 
> 
> thanks,
> 
> Takashi
> 

[PATCH v3 0/4] usb-audio: Add UAC3 Power Domains

[Jorge Sanjuan]

This is what's new in this v3:
 - Add proper SPDX identifier for new source file.
 - Add delay unit in comment for BADD inferred recovery
   times and specify the actual delay time for them.
 - Suspend the usb stream *after* alsa has supended its stuff.
 - Make sure Power State changes to D0 happen *before* stream
   format is set.
 - Try to set the Power State to D0 on .prepare callback too as
   recovery from suspend state doesn't need .hw_params call.

This patchset adds support for UAC3 Power Domains. This feature
of the USB audio class 3 allows the host to notify the device 
what it is making use of so power comsumption can be optimized.

This proposal implements this feature for Power Domains
that include an Input/Output Terminal associated to an
audio Streaming interface. This is the main usage of this
feature according to the spec. For that reason, the logic
for the Power Domain state change has been implemented
within the ALSA PCMs logic and the suspend/resume callbacks
of the usb_driver. The behaviour would be as follows:

* Power Domain State D0: A Power Domain will reach this state
  only when the audio substream associated to that domain is
  being used (i,e. Audio playback/capture is happening).
* Power Domain State D1: This is the Idle state where the driver
  is going to always want to be in order to reduce power
  consumption.
* Power Domain State D2: This state is only set when the usb driver
  asumes the device is not going to be used anymore and hence, it
  wont care about getting any interrupts from the device. This
  will only happen when power level is set to "auto" in sysfs
  so the usb driver gets suspended when the interfaces are not in use.
 
NOTE: The way this has been implemented will always try to put the 
Power Domain in state D1 if the Power Domain exists. The patch
"ALSA: usb-audio: Operate UAC3 Power Domains in PCM callbacks"
puts the logic for doing so inside the PCM's logic. Something to
improve on that is to also tie up those D1<->D0 state changes
to runtime PM maybe.


Jorge Sanjuan (4):
  ALSA: usb-audio: Initial Power Domain support
  ALSA: usb-audio: AudioStreaming Power Domain parsing
  ALSA: usb-audio: Add UAC3 Power Domains to suspend/resume
  ALSA: usb-audio: Operate UAC3 Power Domains in PCM callbacks

 include/linux/usb/audio-v3.h |   4 ++
 sound/usb/Makefile           |   1 +
 sound/usb/card.c             |   9 ++++
 sound/usb/card.h             |   2 +
 sound/usb/pcm.c              |  68 ++++++++++++++++++++++++++--
 sound/usb/pcm.h              |   2 +
 sound/usb/power.c            | 104 +++++++++++++++++++++++++++++++++++++++++++
 sound/usb/power.h            |  19 ++++++++
 sound/usb/stream.c           |  70 +++++++++++++++++++++++++----
 9 files changed, 268 insertions(+), 11 deletions(-)
 create mode 100644 sound/usb/power.c

-- 
2.11.0

[PATCH v3 1/4] ALSA: usb-audio: Initial Power Domain support

[Jorge Sanjuan]

Thee USB Audio Class 3 (UAC3) introduces Power Domains as a new
feature to let a host turn individual parts of an audio function
to different power states via USB requests. This lets the device
get to know a bit amore about what the host is up to in order to
optimize power consumption efficiently.

The Power Domains are optional for UAC3 configuration but all
UAC3 devices shall include at least one BADD configuration where
the support for Power Domains is compulsory.

This patch adds a set of features/helpers to parse these power
domains and change their status.

Signed-off-by: Jorge Sanjuan <jorge.sanjuan@codethink.co.uk>
---
 include/linux/usb/audio-v3.h |   4 ++
 sound/usb/Makefile           |   1 +
 sound/usb/power.c            | 104 +++++++++++++++++++++++++++++++++++++++++++
 sound/usb/power.h            |  19 ++++++++
 4 files changed, 128 insertions(+)
 create mode 100644 sound/usb/power.c

diff --git a/include/linux/usb/audio-v3.h b/include/linux/usb/audio-v3.h
index 334bfa6dfb47..6b708434b7f9 100644
--- a/include/linux/usb/audio-v3.h
+++ b/include/linux/usb/audio-v3.h
@@ -447,4 +447,8 @@ struct uac3_interrupt_data_msg {
 /* BADD sample rate is always fixed to 48kHz */
 #define UAC3_BADD_SAMPLING_RATE				48000
 
+/* BADD power domains recovery times in 50us increments */
+#define UAC3_BADD_PD_RECOVER_D1D0			0x0258	/* 30ms */
+#define UAC3_BADD_PD_RECOVER_D2D0			0x1770	/* 300ms */
+
 #endif /* __LINUX_USB_AUDIO_V3_H */
diff --git a/sound/usb/Makefile b/sound/usb/Makefile
index 05440e2df8d9..d330f74c90e6 100644
--- a/sound/usb/Makefile
+++ b/sound/usb/Makefile
@@ -13,6 +13,7 @@ snd-usb-audio-objs := 	card.o \
 			mixer_scarlett.o \
 			mixer_us16x08.o \
 			pcm.o \
+			power.o \
 			proc.o \
 			quirks.o \
 			stream.o
diff --git a/sound/usb/power.c b/sound/usb/power.c
new file mode 100644
index 000000000000..bd303a1ba1b7
--- /dev/null
+++ b/sound/usb/power.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *   UAC3 Power Domain state management functions
+ */
+
+#include <linux/slab.h>
+#include <linux/usb.h>
+#include <linux/usb/audio.h>
+#include <linux/usb/audio-v2.h>
+#include <linux/usb/audio-v3.h>
+
+#include "usbaudio.h"
+#include "helper.h"
+#include "power.h"
+
+struct snd_usb_power_domain *
+snd_usb_find_power_domain(struct usb_host_interface *ctrl_iface,
+			  unsigned char id)
+{
+	struct snd_usb_power_domain *pd;
+	void *p;
+
+	pd = kzalloc(sizeof(*pd), GFP_KERNEL);
+	if (!pd)
+		return NULL;
+
+	p = NULL;
+	while ((p = snd_usb_find_csint_desc(ctrl_iface->extra,
+					    ctrl_iface->extralen,
+					    p, UAC3_POWER_DOMAIN)) != NULL) {
+		struct uac3_power_domain_descriptor *pd_desc = p;
+		int i;
+
+		for (i = 0; i < pd_desc->bNrEntities; i++) {
+			if (pd_desc->baEntityID[i] == id) {
+				pd->pd_id = pd_desc->bPowerDomainID;
+				pd->pd_d1d0_rec =
+					le16_to_cpu(pd_desc->waRecoveryTime1);
+				pd->pd_d2d0_rec =
+					le16_to_cpu(pd_desc->waRecoveryTime2);
+				return pd;
+			}
+		}
+	}
+
+	kfree(pd);
+	return NULL;
+}
+
+int snd_usb_power_domain_set(struct snd_usb_audio *chip,
+			     struct snd_usb_power_domain *pd,
+			     unsigned char state)
+{
+	struct usb_device *dev = chip->dev;
+	unsigned char current_state;
+	int err, idx;
+
+	idx = snd_usb_ctrl_intf(chip) | (pd->pd_id << 8);
+
+	err = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0),
+			      UAC2_CS_CUR,
+			      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
+			      UAC3_AC_POWER_DOMAIN_CONTROL << 8, idx,
+			      &current_state, sizeof(current_state));
+	if (err < 0) {
+		dev_err(&dev->dev, "Can't get UAC3 power state for id %d\n",
+			pd->pd_id);
+		return err;
+	}
+
+	if (current_state == state) {
+		dev_dbg(&dev->dev, "UAC3 power domain id %d already in state %d\n",
+			pd->pd_id, state);
+		return 0;
+	}
+
+	err = snd_usb_ctl_msg(chip->dev, usb_sndctrlpipe(chip->dev, 0), UAC2_CS_CUR,
+			      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_OUT,
+			      UAC3_AC_POWER_DOMAIN_CONTROL << 8, idx,
+			      &state, sizeof(state));
+	if (err < 0) {
+		dev_err(&dev->dev, "Can't set UAC3 power state to %d for id %d\n",
+			state, pd->pd_id);
+		return err;
+	}
+
+	if (state == UAC3_PD_STATE_D0) {
+		switch (current_state) {
+		case UAC3_PD_STATE_D2:
+			udelay(pd->pd_d2d0_rec * 50);
+			break;
+		case UAC3_PD_STATE_D1:
+			udelay(pd->pd_d1d0_rec * 50);
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	dev_dbg(&dev->dev, "UAC3 power domain id %d change to state %d\n",
+		pd->pd_id, state);
+
+	return 0;
+}
diff --git a/sound/usb/power.h b/sound/usb/power.h
index b2e25f60c5a2..6004231a7c75 100644
--- a/sound/usb/power.h
+++ b/sound/usb/power.h
@@ -2,6 +2,25 @@
 #ifndef __USBAUDIO_POWER_H
 #define __USBAUDIO_POWER_H
 
+struct snd_usb_power_domain {
+	int pd_id;              /* UAC3 Power Domain ID */
+	int pd_d1d0_rec;        /* D1 to D0 recovery time */
+	int pd_d2d0_rec;        /* D2 to D0 recovery time */
+};
+
+enum {
+	UAC3_PD_STATE_D0,
+	UAC3_PD_STATE_D1,
+	UAC3_PD_STATE_D2,
+};
+
+int snd_usb_power_domain_set(struct snd_usb_audio *chip,
+			     struct snd_usb_power_domain *pd,
+			     unsigned char state);
+struct snd_usb_power_domain *
+snd_usb_find_power_domain(struct usb_host_interface *ctrl_iface,
+			  unsigned char id);
+
 #ifdef CONFIG_PM
 int snd_usb_autoresume(struct snd_usb_audio *chip);
 void snd_usb_autosuspend(struct snd_usb_audio *chip);
-- 
2.11.0

[PATCH v3 2/4] ALSA: usb-audio: AudioStreaming Power Domain parsing

[Jorge Sanjuan]

Power Domains in the UAC3 spec are mainly intended to be
associated to an Input or Output Terminal so the host
changes the power state of the entire capture or playback
path within the topology.

This patch adds support for finding Power Domains associated
to an Audio Streaming Interface (bTerminalLink) and adds a
reference to them in the usb audio substreams (snd_usb_substream).

Signed-off-by: Jorge Sanjuan <jorge.sanjuan@codethink.co.uk>
---
 sound/usb/card.h   |  2 ++
 sound/usb/stream.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/sound/usb/card.h b/sound/usb/card.h
index 9b41b7dda84f..ac785d15ced4 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -37,6 +37,7 @@ struct audioformat {
 
 struct snd_usb_substream;
 struct snd_usb_endpoint;
+struct snd_usb_power_domain;
 
 struct snd_urb_ctx {
 	struct urb *urb;
@@ -115,6 +116,7 @@ struct snd_usb_substream {
 	int interface;	/* current interface */
 	int endpoint;	/* assigned endpoint */
 	struct audioformat *cur_audiofmt;	/* current audioformat pointer (for hw_params callback) */
+	struct snd_usb_power_domain *str_pd;	/* UAC3 Power Domain for streaming path */
 	snd_pcm_format_t pcm_format;	/* current audio format (for hw_params callback) */
 	unsigned int channels;		/* current number of channels (for hw_params callback) */
 	unsigned int channels_max;	/* max channels in the all audiofmts */
diff --git a/sound/usb/stream.c b/sound/usb/stream.c
index 729afd808cc4..c0567fa1e84b 100644
--- a/sound/usb/stream.c
+++ b/sound/usb/stream.c
@@ -37,6 +37,7 @@
 #include "format.h"
 #include "clock.h"
 #include "stream.h"
+#include "power.h"
 
 /*
  * free a substream
@@ -53,6 +54,7 @@ static void free_substream(struct snd_usb_substream *subs)
 		kfree(fp);
 	}
 	kfree(subs->rate_list.list);
+	kfree(subs->str_pd);
 }
 
 
@@ -82,7 +84,8 @@ static void snd_usb_audio_pcm_free(struct snd_pcm *pcm)
 
 static void snd_usb_init_substream(struct snd_usb_stream *as,
 				   int stream,
-				   struct audioformat *fp)
+				   struct audioformat *fp,
+				   struct snd_usb_power_domain *pd)
 {
 	struct snd_usb_substream *subs = &as->substream[stream];
 
@@ -107,6 +110,9 @@ static void snd_usb_init_substream(struct snd_usb_stream *as,
 	if (fp->channels > subs->channels_max)
 		subs->channels_max = fp->channels;
 
+	if (pd)
+		subs->str_pd = pd;
+
 	snd_usb_preallocate_buffer(subs);
 }
 
@@ -468,9 +474,11 @@ snd_pcm_chmap_elem *convert_chmap_v3(struct uac3_cluster_header_descriptor
  * fmt_list and will be freed on the chip instance release. do not free
  * fp or do remove it from the substream fmt_list to avoid double-free.
  */
-int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
-			     int stream,
-			     struct audioformat *fp)
+static int __snd_usb_add_audio_stream(struct snd_usb_audio *chip,
+				      int stream,
+				      struct audioformat *fp,
+				      struct snd_usb_power_domain *pd)
+
 {
 	struct snd_usb_stream *as;
 	struct snd_usb_substream *subs;
@@ -498,7 +506,7 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
 		err = snd_pcm_new_stream(as->pcm, stream, 1);
 		if (err < 0)
 			return err;
-		snd_usb_init_substream(as, stream, fp);
+		snd_usb_init_substream(as, stream, fp, pd);
 		return add_chmap(as->pcm, stream, subs);
 	}
 
@@ -526,7 +534,7 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
 	else
 		strcpy(pcm->name, "USB Audio");
 
-	snd_usb_init_substream(as, stream, fp);
+	snd_usb_init_substream(as, stream, fp, pd);
 
 	/*
 	 * Keep using head insertion for M-Audio Audiophile USB (tm) which has a
@@ -544,6 +552,21 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
 	return add_chmap(pcm, stream, &as->substream[stream]);
 }
 
+int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
+			     int stream,
+			     struct audioformat *fp)
+{
+	return __snd_usb_add_audio_stream(chip, stream, fp, NULL);
+}
+
+static int snd_usb_add_audio_stream_v3(struct snd_usb_audio *chip,
+				       int stream,
+				       struct audioformat *fp,
+				       struct snd_usb_power_domain *pd)
+{
+	return __snd_usb_add_audio_stream(chip, stream, fp, pd);
+}
+
 static int parse_uac_endpoint_attributes(struct snd_usb_audio *chip,
 					 struct usb_host_interface *alts,
 					 int protocol, int iface_no)
@@ -819,6 +842,7 @@ snd_usb_get_audioformat_uac12(struct snd_usb_audio *chip,
 static struct audioformat *
 snd_usb_get_audioformat_uac3(struct snd_usb_audio *chip,
 			     struct usb_host_interface *alts,
+			     struct snd_usb_power_domain **pd_out,
 			     int iface_no, int altset_idx,
 			     int altno, int stream)
 {
@@ -829,6 +853,7 @@ snd_usb_get_audioformat_uac3(struct snd_usb_audio *chip,
 	struct uac3_as_header_descriptor *as = NULL;
 	struct uac3_hc_descriptor_header hc_header;
 	struct snd_pcm_chmap_elem *chmap;
+	struct snd_usb_power_domain *pd;
 	unsigned char badd_profile;
 	u64 badd_formats = 0;
 	unsigned int num_channels;
@@ -1008,12 +1033,28 @@ snd_usb_get_audioformat_uac3(struct snd_usb_audio *chip,
 		fp->rate_max = UAC3_BADD_SAMPLING_RATE;
 		fp->rates = SNDRV_PCM_RATE_CONTINUOUS;
 
+		pd = kzalloc(sizeof(pd), GFP_KERNEL);
+		if (!pd) {
+			kfree(fp->rate_table);
+			kfree(fp);
+			return NULL;
+		}
+		pd->pd_id = (stream == SNDRV_PCM_STREAM_PLAYBACK) ?
+					UAC3_BADD_PD_ID10 : UAC3_BADD_PD_ID11;
+		pd->pd_d1d0_rec = UAC3_BADD_PD_RECOVER_D1D0;
+		pd->pd_d2d0_rec = UAC3_BADD_PD_RECOVER_D2D0;
+
 	} else {
 		fp->attributes = parse_uac_endpoint_attributes(chip, alts,
 							       UAC_VERSION_3,
 							       iface_no);
+
+		pd = snd_usb_find_power_domain(chip->ctrl_intf,
+					       as->bTerminalLink);
+
 		/* ok, let's parse further... */
 		if (snd_usb_parse_audio_format_v3(chip, fp, as, stream) < 0) {
+			kfree(pd);
 			kfree(fp->chmap);
 			kfree(fp->rate_table);
 			kfree(fp);
@@ -1021,6 +1062,9 @@ snd_usb_get_audioformat_uac3(struct snd_usb_audio *chip,
 		}
 	}
 
+	if (pd)
+		*pd_out = pd;
+
 	return fp;
 }
 
@@ -1032,6 +1076,7 @@ int snd_usb_parse_audio_interface(struct snd_usb_audio *chip, int iface_no)
 	struct usb_interface_descriptor *altsd;
 	int i, altno, err, stream;
 	struct audioformat *fp = NULL;
+	struct snd_usb_power_domain *pd = NULL;
 	int num, protocol;
 
 	dev = chip->dev;
@@ -1114,7 +1159,7 @@ int snd_usb_parse_audio_interface(struct snd_usb_audio *chip, int iface_no)
 			break;
 		}
 		case UAC_VERSION_3:
-			fp = snd_usb_get_audioformat_uac3(chip, alts,
+			fp = snd_usb_get_audioformat_uac3(chip, alts, &pd,
 						iface_no, i, altno, stream);
 			break;
 		}
@@ -1125,9 +1170,14 @@ int snd_usb_parse_audio_interface(struct snd_usb_audio *chip, int iface_no)
 			return PTR_ERR(fp);
 
 		dev_dbg(&dev->dev, "%u:%d: add audio endpoint %#x\n", iface_no, altno, fp->endpoint);
-		err = snd_usb_add_audio_stream(chip, stream, fp);
+		if (protocol == UAC_VERSION_3)
+			err = snd_usb_add_audio_stream_v3(chip, stream, fp, pd);
+		else
+			err = snd_usb_add_audio_stream(chip, stream, fp);
+
 		if (err < 0) {
 			list_del(&fp->list); /* unlink for avoiding double-free */
+			kfree(pd);
 			kfree(fp->rate_table);
 			kfree(fp->chmap);
 			kfree(fp);
-- 
2.11.0

[PATCH v3 3/4] ALSA: usb-audio: Add UAC3 Power Domains to suspend/resume

[Jorge Sanjuan]

Set the UAC3 Power Domain state for an Audio Streaming interface
to D2 state before suspending the device (usb_driver callback).
This lets the device know there is no intention to use any of the
Units in the Audio Function and that the host is not going to
even listen for wake-up events (interrupts) on the units.

When the usb_driver gets resumed, the state D0 (fully powered) will
be set. This ties up the UAC3 Power Domains to the runtime PM.

Signed-off-by: Jorge Sanjuan <jorge.sanjuan@codethink.co.uk>
---
 sound/usb/card.c |  9 +++++++++
 sound/usb/pcm.c  | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 sound/usb/pcm.h  |  2 ++
 3 files changed, 59 insertions(+)

diff --git a/sound/usb/card.c b/sound/usb/card.c
index a1ed798a1c6b..2bfe4e80a6b9 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -809,6 +809,7 @@ static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message)
 	if (!chip->num_suspended_intf++) {
 		list_for_each_entry(as, &chip->pcm_list, list) {
 			snd_pcm_suspend_all(as->pcm);
+			snd_usb_pcm_suspend(as);
 			as->substream[0].need_setup_ep =
 				as->substream[1].need_setup_ep = true;
 		}
@@ -824,6 +825,7 @@ static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message)
 static int __usb_audio_resume(struct usb_interface *intf, bool reset_resume)
 {
 	struct snd_usb_audio *chip = usb_get_intfdata(intf);
+	struct snd_usb_stream *as;
 	struct usb_mixer_interface *mixer;
 	struct list_head *p;
 	int err = 0;
@@ -834,6 +836,13 @@ static int __usb_audio_resume(struct usb_interface *intf, bool reset_resume)
 		return 0;
 
 	atomic_inc(&chip->active); /* avoid autopm */
+
+	list_for_each_entry(as, &chip->pcm_list, list) {
+		err = snd_usb_pcm_resume(as);
+		if (err < 0)
+			goto err_out;
+	}
+
 	/*
 	 * ALSA leaves material resumption to user space
 	 * we just notify and restart the mixers
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 4b930fa47277..99ec9d5caa58 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -711,6 +711,54 @@ static int configure_endpoint(struct snd_usb_substream *subs)
 	return ret;
 }
 
+static int snd_usb_pcm_change_state(struct snd_usb_substream *subs, int state)
+{
+	int ret;
+
+	if (!subs->str_pd)
+		return 0;
+
+	ret = snd_usb_power_domain_set(subs->stream->chip, subs->str_pd, state);
+	if (ret < 0) {
+		dev_err(&subs->dev->dev,
+			"Cannot change Power Domain ID: %d to state: %d. Err: %d\n",
+			subs->str_pd->pd_id, state, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+int snd_usb_pcm_suspend(struct snd_usb_stream *as)
+{
+	int ret;
+
+	ret = snd_usb_pcm_change_state(&as->substream[0], UAC3_PD_STATE_D2);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_usb_pcm_change_state(&as->substream[1], UAC3_PD_STATE_D2);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+int snd_usb_pcm_resume(struct snd_usb_stream *as)
+{
+	int ret;
+
+	ret = snd_usb_pcm_change_state(&as->substream[0], UAC3_PD_STATE_D0);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_usb_pcm_change_state(&as->substream[1], UAC3_PD_STATE_D0);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 /*
  * hw_params callback
  *
diff --git a/sound/usb/pcm.h b/sound/usb/pcm.h
index f77ec58bf1a1..9833627c1eca 100644
--- a/sound/usb/pcm.h
+++ b/sound/usb/pcm.h
@@ -6,6 +6,8 @@ snd_pcm_uframes_t snd_usb_pcm_delay(struct snd_usb_substream *subs,
 				    unsigned int rate);
 
 void snd_usb_set_pcm_ops(struct snd_pcm *pcm, int stream);
+int snd_usb_pcm_suspend(struct snd_usb_stream *as);
+int snd_usb_pcm_resume(struct snd_usb_stream *as);
 
 int snd_usb_init_pitch(struct snd_usb_audio *chip, int iface,
 		       struct usb_host_interface *alts,
-- 
2.11.0

[PATCH v3 4/4] ALSA: usb-audio: Operate UAC3 Power Domains in PCM callbacks

[Jorge Sanjuan]

Make use of UAC3 Power Domains associated to an Audio Streaming
path within the PCM's logic. This means, when there is no audio
being transferred (pcm is closed), the host will set the Power Domain
associated to that substream to state D1. When audio is being transferred
(from hw_params onwards), the Power Domain will be set to D0 state.

This is the way the host lets the device know which Terminal
is going to be actively used and it is for the device to
manage its own internal resources on that UAC3 Power Domain.

Note the resume method now sets the Power Domain to D1 state as
resuming the device doesn't mean audio streaming will occur.

Signed-off-by: Jorge Sanjuan <jorge.sanjuan@codethink.co.uk>
---
 sound/usb/pcm.c    | 24 +++++++++++++++++++-----
 sound/usb/stream.c |  6 +++++-
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 99ec9d5caa58..bbc7116c9543 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -748,11 +748,11 @@ int snd_usb_pcm_resume(struct snd_usb_stream *as)
 {
 	int ret;
 
-	ret = snd_usb_pcm_change_state(&as->substream[0], UAC3_PD_STATE_D0);
+	ret = snd_usb_pcm_change_state(&as->substream[0], UAC3_PD_STATE_D1);
 	if (ret < 0)
 		return ret;
 
-	ret = snd_usb_pcm_change_state(&as->substream[1], UAC3_PD_STATE_D0);
+	ret = snd_usb_pcm_change_state(&as->substream[1], UAC3_PD_STATE_D1);
 	if (ret < 0)
 		return ret;
 
@@ -803,16 +803,22 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream,
 	ret = snd_usb_lock_shutdown(subs->stream->chip);
 	if (ret < 0)
 		return ret;
+
+	ret = snd_usb_pcm_change_state(subs, UAC3_PD_STATE_D0);
+	if (ret < 0)
+		goto unlock;
+
 	ret = set_format(subs, fmt);
-	snd_usb_unlock_shutdown(subs->stream->chip);
 	if (ret < 0)
-		return ret;
+		goto unlock;
 
 	subs->interface = fmt->iface;
 	subs->altset_idx = fmt->altset_idx;
 	subs->need_setup_ep = true;
 
-	return 0;
+ unlock:
+	snd_usb_unlock_shutdown(subs->stream->chip);
+	return ret;
 }
 
 /*
@@ -869,6 +875,10 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
 	snd_usb_endpoint_sync_pending_stop(subs->sync_endpoint);
 	snd_usb_endpoint_sync_pending_stop(subs->data_endpoint);
 
+	ret = snd_usb_pcm_change_state(subs, UAC3_PD_STATE_D0);
+	if (ret < 0)
+		goto unlock;
+
 	ret = set_format(subs, subs->cur_audiofmt);
 	if (ret < 0)
 		goto unlock;
@@ -1313,6 +1323,7 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)
 	int direction = substream->stream;
 	struct snd_usb_stream *as = snd_pcm_substream_chip(substream);
 	struct snd_usb_substream *subs = &as->substream[direction];
+	int ret;
 
 	stop_endpoints(subs, true);
 
@@ -1321,7 +1332,10 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)
 	    !snd_usb_lock_shutdown(subs->stream->chip)) {
 		usb_set_interface(subs->dev, subs->interface, 0);
 		subs->interface = -1;
+		ret = snd_usb_pcm_change_state(subs, UAC3_PD_STATE_D1);
 		snd_usb_unlock_shutdown(subs->stream->chip);
+		if (ret < 0)
+			return ret;
 	}
 
 	subs->pcm_substream = NULL;
diff --git a/sound/usb/stream.c b/sound/usb/stream.c
index c0567fa1e84b..8fe3b0e00e45 100644
--- a/sound/usb/stream.c
+++ b/sound/usb/stream.c
@@ -110,8 +110,12 @@ static void snd_usb_init_substream(struct snd_usb_stream *as,
 	if (fp->channels > subs->channels_max)
 		subs->channels_max = fp->channels;
 
-	if (pd)
+	if (pd) {
 		subs->str_pd = pd;
+		/* Initialize Power Domain to idle status D1 */
+		snd_usb_power_domain_set(subs->stream->chip, pd,
+					 UAC3_PD_STATE_D1);
+	}
 
 	snd_usb_preallocate_buffer(subs);
 }
-- 
2.11.0

Re: [PATCH v3 0/4] usb-audio: Add UAC3 Power Domains

[Takashi Iwai]

On Tue, 31 Jul 2018 14:28:41 +0200,
Jorge Sanjuan wrote:
> 
> This is what's new in this v3:
>  - Add proper SPDX identifier for new source file.
>  - Add delay unit in comment for BADD inferred recovery
>    times and specify the actual delay time for them.
>  - Suspend the usb stream *after* alsa has supended its stuff.
>  - Make sure Power State changes to D0 happen *before* stream
>    format is set.
>  - Try to set the Power State to D0 on .prepare callback too as
>    recovery from suspend state doesn't need .hw_params call.
> 
> This patchset adds support for UAC3 Power Domains. This feature
> of the USB audio class 3 allows the host to notify the device 
> what it is making use of so power comsumption can be optimized.
> 
> This proposal implements this feature for Power Domains
> that include an Input/Output Terminal associated to an
> audio Streaming interface. This is the main usage of this
> feature according to the spec. For that reason, the logic
> for the Power Domain state change has been implemented
> within the ALSA PCMs logic and the suspend/resume callbacks
> of the usb_driver. The behaviour would be as follows:
> 
> * Power Domain State D0: A Power Domain will reach this state
>   only when the audio substream associated to that domain is
>   being used (i,e. Audio playback/capture is happening).
> * Power Domain State D1: This is the Idle state where the driver
>   is going to always want to be in order to reduce power
>   consumption.
> * Power Domain State D2: This state is only set when the usb driver
>   asumes the device is not going to be used anymore and hence, it
>   wont care about getting any interrupts from the device. This
>   will only happen when power level is set to "auto" in sysfs
>   so the usb driver gets suspended when the interfaces are not in use.
>  
> NOTE: The way this has been implemented will always try to put the 
> Power Domain in state D1 if the Power Domain exists. The patch
> "ALSA: usb-audio: Operate UAC3 Power Domains in PCM callbacks"
> puts the logic for doing so inside the PCM's logic. Something to
> improve on that is to also tie up those D1<->D0 state changes
> to runtime PM maybe.
> 
> 
> Jorge Sanjuan (4):
>   ALSA: usb-audio: Initial Power Domain support
>   ALSA: usb-audio: AudioStreaming Power Domain parsing
>   ALSA: usb-audio: Add UAC3 Power Domains to suspend/resume
>   ALSA: usb-audio: Operate UAC3 Power Domains in PCM callbacks

Now I merge all these four patches.  Thanks!


Takashi