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....@mediatek.com>
> ---
>  sound/usb/pcm.c |   11 +++++++++++linux-...@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

Reply via email to