| Message ID | 20180730092336.18741-5-jorge.sanjuan@codethink.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb-audio: Add UAC3 Power Domains | expand |
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 >
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); }
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(-)