| Message ID | s5hh91eb0oc.wl-tiwai@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Apr 24, 2017 at 11:43:47AM +0200, Takashi Iwai wrote: > On Mon, 24 Apr 2017 11:12:14 +0200, > Vinod Koul wrote: > > > > On Mon, Apr 24, 2017 at 11:00:45AM +0200, Takashi Iwai wrote: > > > > > Removing ignore_suspend makes the PM succeeds. But it hits some other > > > ugly kernel bugs. > > > > Okay have you marked .nonatomic = true for the machine DAIs? > > Ah that's it. The patch below seems fixing the PM and the nonatomic > problems. I'm not sure about the nonatomic flag for the compress > stream, though. Well we dont have upstream decoders so it wont be used in this case. > Also I fiddled only with FE. Do we need the same flags for BE? The > others don't look setting like that, so I left so. I dont remember if BE needs or not FE should suffice. > > > thanks, > > Takashi > > --- > sound/soc/intel/boards/bytcr_rt5640.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > --- a/sound/soc/intel/boards/bytcr_rt5640.c > +++ b/sound/soc/intel/boards/bytcr_rt5640.c > @@ -646,7 +646,7 @@ static struct snd_soc_dai_link byt_rt564 > .codec_dai_name = "snd-soc-dummy-dai", > .codec_name = "snd-soc-dummy", > .platform_name = "sst-mfld-platform", > - .ignore_suspend = 1, > + .nonatomic = true, > .dynamic = 1, > .dpcm_playback = 1, > .dpcm_capture = 1, > @@ -659,7 +659,6 @@ static struct snd_soc_dai_link byt_rt564 > .codec_dai_name = "snd-soc-dummy-dai", > .codec_name = "snd-soc-dummy", > .platform_name = "sst-mfld-platform", > - .ignore_suspend = 1, > .nonatomic = true, > .dynamic = 1, > .dpcm_playback = 1, > @@ -672,6 +671,7 @@ static struct snd_soc_dai_link byt_rt564 > .codec_dai_name = "snd-soc-dummy-dai", > .codec_name = "snd-soc-dummy", > .platform_name = "sst-mfld-platform", > + .nonatomic = true, > }, > /* back ends */ > {
On Mon, 24 Apr 2017 11:52:44 +0200, Vinod Koul wrote: > > On Mon, Apr 24, 2017 at 11:43:47AM +0200, Takashi Iwai wrote: > > On Mon, 24 Apr 2017 11:12:14 +0200, > > Vinod Koul wrote: > > > > > > On Mon, Apr 24, 2017 at 11:00:45AM +0200, Takashi Iwai wrote: > > > > > > > Removing ignore_suspend makes the PM succeeds. But it hits some other > > > > ugly kernel bugs. > > > > > > Okay have you marked .nonatomic = true for the machine DAIs? > > > > Ah that's it. The patch below seems fixing the PM and the nonatomic > > problems. I'm not sure about the nonatomic flag for the compress > > stream, though. > > Well we dont have upstream decoders so it wont be used in this case. > > > Also I fiddled only with FE. Do we need the same flags for BE? The > > others don't look setting like that, so I left so. > > I dont remember if BE needs or not FE should suffice. OK then I leave it as is. When I submit the fix, I should put Cc to stable, and wonder which version we assure the nonatomic ops in SST driver. Did the code base support nonatomic ops from the beginning? thanks, Takashi > > > > > > > thanks, > > > > Takashi > > > > --- > > sound/soc/intel/boards/bytcr_rt5640.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > --- a/sound/soc/intel/boards/bytcr_rt5640.c > > +++ b/sound/soc/intel/boards/bytcr_rt5640.c > > @@ -646,7 +646,7 @@ static struct snd_soc_dai_link byt_rt564 > > .codec_dai_name = "snd-soc-dummy-dai", > > .codec_name = "snd-soc-dummy", > > .platform_name = "sst-mfld-platform", > > - .ignore_suspend = 1, > > + .nonatomic = true, > > .dynamic = 1, > > .dpcm_playback = 1, > > .dpcm_capture = 1, > > @@ -659,7 +659,6 @@ static struct snd_soc_dai_link byt_rt564 > > .codec_dai_name = "snd-soc-dummy-dai", > > .codec_name = "snd-soc-dummy", > > .platform_name = "sst-mfld-platform", > > - .ignore_suspend = 1, > > .nonatomic = true, > > .dynamic = 1, > > .dpcm_playback = 1, > > @@ -672,6 +671,7 @@ static struct snd_soc_dai_link byt_rt564 > > .codec_dai_name = "snd-soc-dummy-dai", > > .codec_name = "snd-soc-dummy", > > .platform_name = "sst-mfld-platform", > > + .nonatomic = true, > > }, > > /* back ends */ > > { > > -- > ~Vinod >
On Mon, Apr 24, 2017 at 11:54:06AM +0200, Takashi Iwai wrote: > On Mon, 24 Apr 2017 11:52:44 +0200, > Vinod Koul wrote: > > > > On Mon, Apr 24, 2017 at 11:43:47AM +0200, Takashi Iwai wrote: > > > On Mon, 24 Apr 2017 11:12:14 +0200, > > > Vinod Koul wrote: > > > > > > > > On Mon, Apr 24, 2017 at 11:00:45AM +0200, Takashi Iwai wrote: > > > > > > > > > Removing ignore_suspend makes the PM succeeds. But it hits some other > > > > > ugly kernel bugs. > > > > > > > > Okay have you marked .nonatomic = true for the machine DAIs? > > > > > > Ah that's it. The patch below seems fixing the PM and the nonatomic > > > problems. I'm not sure about the nonatomic flag for the compress > > > stream, though. > > > > Well we dont have upstream decoders so it wont be used in this case. > > > > > Also I fiddled only with FE. Do we need the same flags for BE? The > > > others don't look setting like that, so I left so. > > > > I dont remember if BE needs or not FE should suffice. > > OK then I leave it as is. > > When I submit the fix, I should put Cc to stable, and wonder which > version we assure the nonatomic ops in SST driver. Did the code base > support nonatomic ops from the beginning? 4.1 onwards. The PM was supported thru below commit which went into 4.1. The nonatomic was always a requirement for us due to nature of IPC. Non atomic support went into 3.18 so 4.1 onwards would make sense :) commit 4a8448d4289d7210053a43f9f21e42929beb159b Author: Vinod Koul <vinod.koul@intel.com> Date: Tue Feb 24 11:39:44 2015 +0530 ASoC: Intel: add pm support in sst ipc driver This adds support for system pm support. We need to save the dsp memory which gets lost on suspend and restore that on resume Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com> Signed-off-by: Vinod Koul <vinod.koul@intel.com> Signed-off-by: Mark Brown <broonie@kernel.org> Thanks
On 4/24/17 4:54 AM, Takashi Iwai wrote: > On Mon, 24 Apr 2017 11:52:44 +0200, > Vinod Koul wrote: >> >> On Mon, Apr 24, 2017 at 11:43:47AM +0200, Takashi Iwai wrote: >>> On Mon, 24 Apr 2017 11:12:14 +0200, >>> Vinod Koul wrote: >>>> >>>> On Mon, Apr 24, 2017 at 11:00:45AM +0200, Takashi Iwai wrote: >>>> >>>>> Removing ignore_suspend makes the PM succeeds. But it hits some other >>>>> ugly kernel bugs. >>>> >>>> Okay have you marked .nonatomic = true for the machine DAIs? >>> >>> Ah that's it. The patch below seems fixing the PM and the nonatomic >>> problems. I'm not sure about the nonatomic flag for the compress >>> stream, though. >> >> Well we dont have upstream decoders so it wont be used in this case. >> >>> Also I fiddled only with FE. Do we need the same flags for BE? The >>> others don't look setting like that, so I left so. >> >> I dont remember if BE needs or not FE should suffice. > > OK then I leave it as is. > > When I submit the fix, I should put Cc to stable, and wonder which > version we assure the nonatomic ops in SST driver. Did the code base > support nonatomic ops from the beginning? can we take this opportunity to align all drivers? The .nonatomic=true is set in all drivers for the BE, except for cht_bsw_max98090_ti.c It's either needed for all or not needed for all... > > > thanks, > > Takashi > >> >>> >>> >>> thanks, >>> >>> Takashi >>> >>> --- >>> sound/soc/intel/boards/bytcr_rt5640.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> --- a/sound/soc/intel/boards/bytcr_rt5640.c >>> +++ b/sound/soc/intel/boards/bytcr_rt5640.c >>> @@ -646,7 +646,7 @@ static struct snd_soc_dai_link byt_rt564 >>> .codec_dai_name = "snd-soc-dummy-dai", >>> .codec_name = "snd-soc-dummy", >>> .platform_name = "sst-mfld-platform", >>> - .ignore_suspend = 1, >>> + .nonatomic = true, >>> .dynamic = 1, >>> .dpcm_playback = 1, >>> .dpcm_capture = 1, >>> @@ -659,7 +659,6 @@ static struct snd_soc_dai_link byt_rt564 >>> .codec_dai_name = "snd-soc-dummy-dai", >>> .codec_name = "snd-soc-dummy", >>> .platform_name = "sst-mfld-platform", >>> - .ignore_suspend = 1, >>> .nonatomic = true, >>> .dynamic = 1, >>> .dpcm_playback = 1, >>> @@ -672,6 +671,7 @@ static struct snd_soc_dai_link byt_rt564 >>> .codec_dai_name = "snd-soc-dummy-dai", >>> .codec_name = "snd-soc-dummy", >>> .platform_name = "sst-mfld-platform", >>> + .nonatomic = true, >>> }, >>> /* back ends */ >>> { >> >> -- >> ~Vinod >> > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel >
On Mon, Apr 24, 2017 at 09:22:38AM -0500, Pierre-Louis Bossart wrote: > On 4/24/17 4:54 AM, Takashi Iwai wrote: > >On Mon, 24 Apr 2017 11:52:44 +0200, > >Vinod Koul wrote: > >> > >>On Mon, Apr 24, 2017 at 11:43:47AM +0200, Takashi Iwai wrote: > >>>On Mon, 24 Apr 2017 11:12:14 +0200, > >>>Vinod Koul wrote: > >>>> > >>>>On Mon, Apr 24, 2017 at 11:00:45AM +0200, Takashi Iwai wrote: > >>>> > >>>>>Removing ignore_suspend makes the PM succeeds. But it hits some other > >>>>>ugly kernel bugs. > >>>> > >>>>Okay have you marked .nonatomic = true for the machine DAIs? > >>> > >>>Ah that's it. The patch below seems fixing the PM and the nonatomic > >>>problems. I'm not sure about the nonatomic flag for the compress > >>>stream, though. > >> > >>Well we dont have upstream decoders so it wont be used in this case. > >> > >>>Also I fiddled only with FE. Do we need the same flags for BE? The > >>>others don't look setting like that, so I left so. > >> > >>I dont remember if BE needs or not FE should suffice. > > > >OK then I leave it as is. > > > >When I submit the fix, I should put Cc to stable, and wonder which > >version we assure the nonatomic ops in SST driver. Did the code base > >support nonatomic ops from the beginning? > > can we take this opportunity to align all drivers? > The .nonatomic=true is set in all drivers for the BE, except for > cht_bsw_max98090_ti.c ?? $ grep nonatomic sound/soc/intel/boards/cht_bsw_max98090_ti.c .nonatomic = true, .nonatomic = true, > It's either needed for all or not needed for all... For the record it is must have for all of the drivers :) $ grep -L nonatomic sound/soc/intel/boards/*.c sound/soc/intel/boards/bdw-rt5677.c sound/soc/intel/boards/broadwell.c sound/soc/intel/boards/byt-max98090.c sound/soc/intel/boards/byt-rt5640.c sound/soc/intel/boards/haswell.c sound/soc/intel/boards/mfld_machine.c So we should add the remaining one byt-max98090.c as Takashi fixed byt-rt5640.c one. I will send the patch for this one.
On Mon, 24 Apr 2017 18:27:38 +0200, Vinod Koul wrote: > > On Mon, Apr 24, 2017 at 09:22:38AM -0500, Pierre-Louis Bossart wrote: > > On 4/24/17 4:54 AM, Takashi Iwai wrote: > > >On Mon, 24 Apr 2017 11:52:44 +0200, > > >Vinod Koul wrote: > > >> > > >>On Mon, Apr 24, 2017 at 11:43:47AM +0200, Takashi Iwai wrote: > > >>>On Mon, 24 Apr 2017 11:12:14 +0200, > > >>>Vinod Koul wrote: > > >>>> > > >>>>On Mon, Apr 24, 2017 at 11:00:45AM +0200, Takashi Iwai wrote: > > >>>> > > >>>>>Removing ignore_suspend makes the PM succeeds. But it hits some other > > >>>>>ugly kernel bugs. > > >>>> > > >>>>Okay have you marked .nonatomic = true for the machine DAIs? > > >>> > > >>>Ah that's it. The patch below seems fixing the PM and the nonatomic > > >>>problems. I'm not sure about the nonatomic flag for the compress > > >>>stream, though. > > >> > > >>Well we dont have upstream decoders so it wont be used in this case. > > >> > > >>>Also I fiddled only with FE. Do we need the same flags for BE? The > > >>>others don't look setting like that, so I left so. > > >> > > >>I dont remember if BE needs or not FE should suffice. > > > > > >OK then I leave it as is. > > > > > >When I submit the fix, I should put Cc to stable, and wonder which > > >version we assure the nonatomic ops in SST driver. Did the code base > > >support nonatomic ops from the beginning? > > > > can we take this opportunity to align all drivers? > > The .nonatomic=true is set in all drivers for the BE, except for > > cht_bsw_max98090_ti.c > > ?? > > $ grep nonatomic sound/soc/intel/boards/cht_bsw_max98090_ti.c > .nonatomic = true, > .nonatomic = true, > > > It's either needed for all or not needed for all... > > For the record it is must have for all of the drivers :) > > $ grep -L nonatomic sound/soc/intel/boards/*.c > sound/soc/intel/boards/bdw-rt5677.c > sound/soc/intel/boards/broadwell.c > sound/soc/intel/boards/byt-max98090.c > sound/soc/intel/boards/byt-rt5640.c > sound/soc/intel/boards/haswell.c > sound/soc/intel/boards/mfld_machine.c > > So we should add the remaining one byt-max98090.c as Takashi fixed > byt-rt5640.c one. I will send the patch for this one. Or maybe we should replace these definitions with some macro to expand to the mostly same contents? The difference is just a few callback functions, basically. Takashi
On 4/24/17 11:27 AM, Vinod Koul wrote: > On Mon, Apr 24, 2017 at 09:22:38AM -0500, Pierre-Louis Bossart wrote: >> On 4/24/17 4:54 AM, Takashi Iwai wrote: >>> On Mon, 24 Apr 2017 11:52:44 +0200, >>> Vinod Koul wrote: >>>> >>>> On Mon, Apr 24, 2017 at 11:43:47AM +0200, Takashi Iwai wrote: >>>>> On Mon, 24 Apr 2017 11:12:14 +0200, >>>>> Vinod Koul wrote: >>>>>> >>>>>> On Mon, Apr 24, 2017 at 11:00:45AM +0200, Takashi Iwai wrote: >>>>>> >>>>>>> Removing ignore_suspend makes the PM succeeds. But it hits some other >>>>>>> ugly kernel bugs. >>>>>> >>>>>> Okay have you marked .nonatomic = true for the machine DAIs? >>>>> >>>>> Ah that's it. The patch below seems fixing the PM and the nonatomic >>>>> problems. I'm not sure about the nonatomic flag for the compress >>>>> stream, though. >>>> >>>> Well we dont have upstream decoders so it wont be used in this case. >>>> >>>>> Also I fiddled only with FE. Do we need the same flags for BE? The >>>>> others don't look setting like that, so I left so. >>>> >>>> I dont remember if BE needs or not FE should suffice. >>> >>> OK then I leave it as is. >>> >>> When I submit the fix, I should put Cc to stable, and wonder which >>> version we assure the nonatomic ops in SST driver. Did the code base >>> support nonatomic ops from the beginning? >> >> can we take this opportunity to align all drivers? >> The .nonatomic=true is set in all drivers for the BE, except for >> cht_bsw_max98090_ti.c > > ?? > > $ grep nonatomic sound/soc/intel/boards/cht_bsw_max98090_ti.c > .nonatomic = true, > .nonatomic = true, Yes, the .nonatomic is set for the two PCM frontends but not for the SSP2 backend. > >> It's either needed for all or not needed for all... > > For the record it is must have for all of the drivers :) > > $ grep -L nonatomic sound/soc/intel/boards/*.c > sound/soc/intel/boards/bdw-rt5677.c > sound/soc/intel/boards/broadwell.c > sound/soc/intel/boards/byt-max98090.c > sound/soc/intel/boards/byt-rt5640.c > sound/soc/intel/boards/haswell.c > sound/soc/intel/boards/mfld_machine.c > > So we should add the remaining one byt-max98090.c as Takashi fixed > byt-rt5640.c one. I will send the patch for this one. Takashi fixed the bytcr-rt5640 driver, I am not sure we should touch the old byt- driver since they are not using the same firmware and they are not enabled by default. Same for broadwell, different driver, different problems. I was also also talking about drivers that have .nonatomic field set but not everywhere consistently.
On Mon, Apr 24, 2017 at 08:32:14PM +0200, Takashi Iwai wrote: > On Mon, 24 Apr 2017 18:27:38 +0200, > > > > So we should add the remaining one byt-max98090.c as Takashi fixed > > byt-rt5640.c one. I will send the patch for this one. > > Or maybe we should replace these definitions with some macro to expand > to the mostly same contents? The difference is just a few callback > functions, basically. And while at it, I cant help but wonder but if we can do better and mark it in platform driver thus avoiding it replication in machines. Afterall the atomic trigger is a platform property. Thanks
On Mon, Apr 24, 2017 at 02:01:44PM -0500, Pierre-Louis Bossart wrote: > On 4/24/17 11:27 AM, Vinod Koul wrote: > >On Mon, Apr 24, 2017 at 09:22:38AM -0500, Pierre-Louis Bossart wrote: > >>On 4/24/17 4:54 AM, Takashi Iwai wrote: > >>>On Mon, 24 Apr 2017 11:52:44 +0200, > >>>Vinod Koul wrote: > >>>> > >>>>On Mon, Apr 24, 2017 at 11:43:47AM +0200, Takashi Iwai wrote: > >>>>>On Mon, 24 Apr 2017 11:12:14 +0200, > >>>>>Vinod Koul wrote: > >>>>>> > >>>>>>On Mon, Apr 24, 2017 at 11:00:45AM +0200, Takashi Iwai wrote: > >>>>>> > >>>>>>>Removing ignore_suspend makes the PM succeeds. But it hits some other > >>>>>>>ugly kernel bugs. > >>>>>> > >>>>>>Okay have you marked .nonatomic = true for the machine DAIs? > >>>>> > >>>>>Ah that's it. The patch below seems fixing the PM and the nonatomic > >>>>>problems. I'm not sure about the nonatomic flag for the compress > >>>>>stream, though. > >>>> > >>>>Well we dont have upstream decoders so it wont be used in this case. > >>>> > >>>>>Also I fiddled only with FE. Do we need the same flags for BE? The > >>>>>others don't look setting like that, so I left so. > >>>> > >>>>I dont remember if BE needs or not FE should suffice. > >>> > >>>OK then I leave it as is. > >>> > >>>When I submit the fix, I should put Cc to stable, and wonder which > >>>version we assure the nonatomic ops in SST driver. Did the code base > >>>support nonatomic ops from the beginning? > >> > >>can we take this opportunity to align all drivers? > >>The .nonatomic=true is set in all drivers for the BE, except for > >>cht_bsw_max98090_ti.c > > > >?? > > > >$ grep nonatomic sound/soc/intel/boards/cht_bsw_max98090_ti.c > > .nonatomic = true, > > .nonatomic = true, > > Yes, the .nonatomic is set for the two PCM frontends but not for the > SSP2 backend. Oh okay, sorry I misunderstood that, I dont think we need this for BE. > > > > >>It's either needed for all or not needed for all... > > > >For the record it is must have for all of the drivers :) > > > >$ grep -L nonatomic sound/soc/intel/boards/*.c > >sound/soc/intel/boards/bdw-rt5677.c > >sound/soc/intel/boards/broadwell.c > >sound/soc/intel/boards/byt-max98090.c > >sound/soc/intel/boards/byt-rt5640.c > >sound/soc/intel/boards/haswell.c > >sound/soc/intel/boards/mfld_machine.c > > > >So we should add the remaining one byt-max98090.c as Takashi fixed > >byt-rt5640.c one. I will send the patch for this one. > > Takashi fixed the bytcr-rt5640 driver, I am not sure we should touch > the old byt- driver since they are not using the same firmware and > they are not enabled by default. Same for broadwell, different > driver, different problems. on that not we should rename file to depict which driver they use older or newer :) > I was also also talking about drivers that have .nonatomic field set > but not everywhere consistently.
--- a/sound/soc/intel/boards/bytcr_rt5640.c +++ b/sound/soc/intel/boards/bytcr_rt5640.c @@ -646,7 +646,7 @@ static struct snd_soc_dai_link byt_rt564 .codec_dai_name = "snd-soc-dummy-dai", .codec_name = "snd-soc-dummy", .platform_name = "sst-mfld-platform", - .ignore_suspend = 1, + .nonatomic = true, .dynamic = 1, .dpcm_playback = 1, .dpcm_capture = 1, @@ -659,7 +659,6 @@ static struct snd_soc_dai_link byt_rt564 .codec_dai_name = "snd-soc-dummy-dai", .codec_name = "snd-soc-dummy", .platform_name = "sst-mfld-platform", - .ignore_suspend = 1, .nonatomic = true, .dynamic = 1, .dpcm_playback = 1, @@ -672,6 +671,7 @@ static struct snd_soc_dai_link byt_rt564 .codec_dai_name = "snd-soc-dummy-dai", .codec_name = "snd-soc-dummy", .platform_name = "sst-mfld-platform", + .nonatomic = true, }, /* back ends */ {