On Fri, 22 Jan 2021 17:40:55 +0100, Ranjani Sridharan wrote: > > On Fri, 2021-01-22 at 15:12 +0100, Takashi Iwai wrote: > > On Fri, 22 Jan 2021 00:23:53 +0100, > > Ranjani Sridharan wrote: > > > Hi Takashi, > > > > > > While exploring some power optimizations on Intel platforms, I > > > noticed > > > that the hdac_ext_link ref_count is incremented during codec probe > > > in hdac_hda_codec_probe() and the ref_count is held until the codec > > > device is removed. > > > > > > I was wondering if it would be possible to call the get/put for the > > > hdac_ext_link in the codec runtime suspend/resume callbacks so that > > > the > > > link is powered up only when the link is in use. Are there any > > > downsides to doing this? > > > > Wouldn't the snd_hdac_ext_bus_link_power_up() / down() calls do the > > runtime PM stuff? Maybe we need to revisit those link power > > management. The ext stuff isn't well m > > and, I'm afraid. > Thanks, Takashi. > It looks like snd_hdac_ext_bus_link_power_up/down() are only called > during snd_hdac_ext_bus_link_get/put(). Actually, in my observation > disabling the CORB/RIRB buffer DMAs is what saves us power and this is > done only if snd_hdac_ext_bus_link_put() is called on all links. > > > > > The get() and put() are obviously for fully enabling and disabling > > the > > device, hence it's not suitable for the runtime PM suspend/resume. > > The power_up() / down() should be adjusted to fit with the runtime PM > > call, if any. > > The only additional thing that snd_hdac_ext_bus_link_get/put() does on > top of snd_hdac_ext_bus_link_power_up/down() is to stop the CORB/RIRB > DMA when all the link ref_counts are 0. Do you think it is not > advisable to stop the CORB/RIRB DMA during runtime PM? Why do you need to stop CORB/RIRB? For stopping the CORB/RIRB DMA, you need to disable the IRQ and other stuff at first, in anyway. Takashi