| Message ID | 1591098821-17910-1-git-send-email-macpaul.lin@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | sound: usb: pcm: fix incorrect power state when playing sound after PM_AUTO suspend | expand |
On Tue, 2020-06-02 at 19:53 +0800, Macpaul Lin wrote: > This patch fix incorrect power state changed by usb_audio_suspend() > when CONFIG_PM is enabled. > > After receiving suspend PM message with auto flag, usb_audio_suspend() > change card's power state to SNDRV_CTL_POWER_D3hot. Only when the other > resume PM message with auto flag can change power state to > SNDRV_CTL_POWER_D0 in __usb_audio_resume(). > > However, when system is not under auto suspend, resume PM message with > auto flag might not be able to receive on time which cause the power > state was incorrect. At this time, if a player starts to play sound, > will cause snd_usb_pcm_open() to access the card and setup_hw_info() will > resume the card. > > But even the card is back to work and all function normal, the power > state is still in SNDRV_CTL_POWER_D3hot. Which cause the infinite loop > happened in snd_power_wait() to check the power state. Thus the > successive setting ioctl cannot be passed to card. > > Hence we suggest to change power state to SNDRV_CTL_POWER_D0 when card > has been resumed successfully. > > Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> > --- > sound/usb/pcm.c | 11 +++++++++++linux-usb@vger.kernel.org, > 1 file changed, 11 insertions(+) > > diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c > index a4e4064..d667ecb 100644 > --- a/sound/usb/pcm.c > +++ b/sound/usb/pcm.c > @@ -1322,6 +1322,17 @@ static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substre > if (err < 0) > return err; > > + /* fix incorrect power state when resuming by open and later ioctls */ > + if (IS_ENABLED(CONFIG_PM) && > + snd_power_get_state(subs->stream->chip->card) > + == SNDRV_CTL_POWER_D3hot) { > + /* set these variables for power state correction */ > + subs->stream->chip->autosuspended = 0; > + subs->stream->chip->num_suspended_intf = 1; > + dev_info(&subs->dev->dev, > + "change power state from D3hot to D0\n"); > + } > + > return snd_usb_autoresume(subs->stream->chip); > } > The issue was found on kernel 4.14 (android tree). The test is to add debug log in sound/core/init.c to check if the power state is SNDRV_CTL_POWER_D3hot. diff --git a/sound/core/init.c b/sound/core/init.c index b02a997..a0bee76 100644 --- a/sound/core/init.c +++ b/sound/core/init.c @@ -1011,6 +1011,8 @@ int snd_power_wait(struct snd_card *card, unsigned int power_state) if (snd_power_get_state(card) == power_state) break; set_current_state(TASK_UNINTERRUPTIBLE); + pr_info("%s snd_power_get_state[%x]\n", __func__, + snd_power_get_state(card)); schedule_timeout(30 * HZ); } remove_wait_queue(&card->power_sleep, &wait); After applied a work around by forcing the power state, pcm related ioctl and parameter settings can be set to usb sound card correctly. Otherwise a infinite loop will happened in snd_power_wait(). Here is the origin work around for verifying this power state issue on kernel 4.14. diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 933adcd7af81..9acd50dd7155 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1274,6 +1274,16 @@ static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substre if (err < 0) return err; + /* avoid incorrect power state when executing IOCTL */ + if (IS_ENABLED(CONFIG_PM) && + snd_power_get_state(subs->stream->chip->card) + == SNDRV_CTL_POWER_D3hot) { + dev_info(&subs->dev->dev, + "change power state from D3hot to D0\n"); + snd_power_change_state(subs->stream->chip->card, + SNDRV_CTL_POWER_D0); + } + param_period_time_if_needed = SNDRV_PCM_HW_PARAM_PERIOD_TIME; if (subs->speed == USB_SPEED_FULL) /* full speed devices have fixed data packet interval */ However, the patch I've send is meant to make sure the power state will be corrected before snd_usb_autoresume(), It should be adapt to kernel 4.14 and later. Thanks. Macpaul Lin
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index a4e4064..d667ecb 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1322,6 +1322,17 @@ static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substre if (err < 0) return err; + /* fix incorrect power state when resuming by open and later ioctls */ + if (IS_ENABLED(CONFIG_PM) && + snd_power_get_state(subs->stream->chip->card) + == SNDRV_CTL_POWER_D3hot) { + /* set these variables for power state correction */ + subs->stream->chip->autosuspended = 0; + subs->stream->chip->num_suspended_intf = 1; + dev_info(&subs->dev->dev, + "change power state from D3hot to D0\n"); + } + return snd_usb_autoresume(subs->stream->chip); }
This patch fix incorrect power state changed by usb_audio_suspend() when CONFIG_PM is enabled. After receiving suspend PM message with auto flag, usb_audio_suspend() change card's power state to SNDRV_CTL_POWER_D3hot. Only when the other resume PM message with auto flag can change power state to SNDRV_CTL_POWER_D0 in __usb_audio_resume(). However, when system is not under auto suspend, resume PM message with auto flag might not be able to receive on time which cause the power state was incorrect. At this time, if a player starts to play sound, will cause snd_usb_pcm_open() to access the card and setup_hw_info() will resume the card. But even the card is back to work and all function normal, the power state is still in SNDRV_CTL_POWER_D3hot. Which cause the infinite loop happened in snd_power_wait() to check the power state. Thus the successive setting ioctl cannot be passed to card. Hence we suggest to change power state to SNDRV_CTL_POWER_D0 when card has been resumed successfully. Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> --- sound/usb/pcm.c | 11 +++++++++++ 1 file changed, 11 insertions(+)