| Message ID | 1448293951-32071-4-git-send-email-vinod.koul@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1a7aaa58ec7aaa389cd6b200809908ec472d316b |
| Headers | show |
On 11/23/2015 04:52 PM, Vinod Koul wrote: > From: Jeeja KP <jeeja.kp@intel.com> > > For DAPM resume, we should first change the power state of the > card and then recheck the endpoints. This ensures the dapm is > resumed first and then userspace can resume the streams. The problem is that DAPM uses the userspace state to decide whether to power up or not. So this change won't work, it will keep the endpoints suspended.
On Mon, Nov 23, 2015 at 05:06:41PM +0100, Lars-Peter Clausen wrote: > On 11/23/2015 04:52 PM, Vinod Koul wrote: > > From: Jeeja KP <jeeja.kp@intel.com> > > > > For DAPM resume, we should first change the power state of the > > card and then recheck the endpoints. This ensures the dapm is > > resumed first and then userspace can resume the streams. > > The problem is that DAPM uses the userspace state to decide whether to power > up or not. So this change won't work, it will keep the endpoints suspended. Well our observation are bit different... When testing with aplay and suspending the system the DAPM was suspending fine and then alsa suspends PCMs, but on resume the DAPM is resumed last and the PCMs first, due to which the devices start underun! The DAPM should be resumed first and then we should allow userspace access (PCM resume from aplay) The change of order here helps in that
On 11/23/2015 06:28 PM, Vinod Koul wrote: > On Mon, Nov 23, 2015 at 05:06:41PM +0100, Lars-Peter Clausen wrote: >> On 11/23/2015 04:52 PM, Vinod Koul wrote: >>> From: Jeeja KP <jeeja.kp@intel.com> >>> >>> For DAPM resume, we should first change the power state of the >>> card and then recheck the endpoints. This ensures the dapm is >>> resumed first and then userspace can resume the streams. >> >> The problem is that DAPM uses the userspace state to decide whether to power >> up or not. So this change won't work, it will keep the endpoints suspended. > > Well our observation are bit different... > > When testing with aplay and suspending the system the DAPM was suspending > fine and then alsa suspends PCMs, but on resume the DAPM is resumed last and > the PCMs first, due to which the devices start underun! > The DAPM should be resumed first and then we should allow userspace access > (PCM resume from aplay) > > The change of order here helps in that Sorry, overlooked something. We go to D2 earlier on and DAPM only is suspended when the state is D3 and not only when it is not in D0. Patch looks good. Reviewed-by: Lars-Peter Clausen <lars@metafoo.de>
On Mon, Nov 23, 2015 at 07:13:12PM +0100, Lars-Peter Clausen wrote: > On 11/23/2015 06:28 PM, Vinod Koul wrote: > > On Mon, Nov 23, 2015 at 05:06:41PM +0100, Lars-Peter Clausen wrote: > >> On 11/23/2015 04:52 PM, Vinod Koul wrote: > >>> From: Jeeja KP <jeeja.kp@intel.com> > >>> > >>> For DAPM resume, we should first change the power state of the > >>> card and then recheck the endpoints. This ensures the dapm is > >>> resumed first and then userspace can resume the streams. > >> > >> The problem is that DAPM uses the userspace state to decide whether to power > >> up or not. So this change won't work, it will keep the endpoints suspended. > > > > Well our observation are bit different... > > > > When testing with aplay and suspending the system the DAPM was suspending > > fine and then alsa suspends PCMs, but on resume the DAPM is resumed last and > > the PCMs first, due to which the devices start underun! > > The DAPM should be resumed first and then we should allow userspace access > > (PCM resume from aplay) > > > > The change of order here helps in that > > Sorry, overlooked something. We go to D2 earlier on and DAPM only is > suspended when the state is D3 and not only when it is not in D0. No issues > Patch looks good. > > Reviewed-by: Lars-Peter Clausen <lars@metafoo.de> Thanks for the review
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 0a26ac0fb513..6dd704a6b76f 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -846,12 +846,12 @@ static void soc_resume_deferred(struct work_struct *work) dev_dbg(card->dev, "ASoC: resume work completed\n"); - /* userspace can access us now we are back as we were before */ - snd_power_change_state(card->snd_card, SNDRV_CTL_POWER_D0); - /* Recheck all endpoints too, their state is affected by suspend */ dapm_mark_endpoints_dirty(card); snd_soc_dapm_sync(&card->dapm); + + /* userspace can access us now we are back as we were before */ + snd_power_change_state(card->snd_card, SNDRV_CTL_POWER_D0); } /* powers up audio subsystem after a suspend */