[alsa-devel] [PATCH 0/4] usb-audio: Add UAC3 Power Domains
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
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, + ¤t_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);
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-Po... 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
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-Po... 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
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);
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); }
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-Po... 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
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); }
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,
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
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
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
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
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
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, + ¤t_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);
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
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
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
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
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);
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,
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
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); }
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
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
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
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
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
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, + ¤t_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);
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);
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,
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); }
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
participants (5)
-
Jorge -
Jorge Sanjuan
-
kbuild test robot -
kbuild test robot -
Takashi Iwai