Re: Question about hdac_ext_link ref_count management

[Takashi Iwai <tiwai@suse.de>]

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