Re: [PATCH] ALSA: hda/hdmi: Add Intel silent stream support

[Takashi Iwai <tiwai@suse.de>]

On Wed, 24 Jun 2020 19:58:40 +0200,
N, Harshapriya wrote:
> 
> > On Wed, 24 Jun 2020 19:05:14 +0200,
> > Pierre-Louis Bossart wrote:
> > >
> > >
> > >
> > > On 6/24/20 11:43 AM, Takashi Iwai wrote:
> > > > On Wed, 24 Jun 2020 17:33:45 +0200,
> > > > Pierre-Louis Bossart wrote:
> > > >>
> > > >>
> > > >>>>> The silent stream is enabled with a Kconfig option, as well as a
> > > >>>>> kernel parameter should there be a need to override the build time
> > default.
> > > >>>>
> > > >>>> I'm not sure whether the module option is the best interface.
> > > >>>> An alternative is a mixer element that controls dynamically.
> > > >>>> Then it'll be per card unlike the module option.
> > > >>>
> > > >>> +1, kcontrol seems the appropriate way to control this.
> > > >>
> > > >> It was my suggestion to use Kconfig+kernel parameter for
> > > >> simplicity/overrides.
> > > >>
> > > >> The kcontrol is a nice idea, but in practice we typically only have
> > > >> one card dealing with HDMI.
> > > >
> > > > Not really.  There are systems with two HDMI outputs from both
> > > > integrated and discrete GPUs.  Most modern systems are only with
> > > > hybrid graphics, though.
> > >
> > > Ok, maybe I am mistaken, in most of the HDMI issues we've seen only
> > > one HDMI source.
> > >
> > > But it's a good point that this is only supposed to be used for Intel
> > > whether it's a kernel parameter or a kcontrol shouldn't this be
> > > dependent on a PCI ID being detected and a SKYLAKE flag being set?
> > > it's my understanding that this applies from Skylake to TigerLake, not
> > > before.
> > 
> > I guess we can check it from the codec ID?  Change the probe function for
> > Skylake+ codecs to patch_i915_skl_hdmi and co, and set the flag there.
> > 
> > > >> It also doesn't have a UCM representation so would force the use of
> > > >> amixer and manual configs, or the UCM file would always set the
> > > >> mode.
> > > >
> > > > But people usually use the distro kernels, so the situation is more
> > > > or less equivalent; you'd have to adjust a module option manually if
> > > > you want a different one from the default, and you'd have to be root
> > > > to change it.
> > > >
> > > > So, rather the question is how we should provide the setup of such
> > > > parameter.  It's supposed to be a part of power management stuff
> > > > that should be touched by either a smart PM tool or a manual
> > > > override such as runtime PM setup?  Or can it be seen as a more casual
> > tuning?
> > >
> > > I am not aware of such tools. The only thing I know is that some of
> > > the HDaudio power settings are already controlled by kernel
> > > parameters, e.g.
> > >
> > > /etc/modprobe.d/audio_powersave.conf
> > > options snd_hda_intel power_save=1
> > 
> > Yes, it's been the primary knob for years to turn on/off the runtime PM for HD-
> > audio and other legacy drivers.  This was used by powertop or some other
> > power-aware daemons and tools, to be toggled dynamically per the power
> > cable state or such.
> > 
> > And, how the silent stream feature should be seen?
> > Should it be a system-wide root-only setup or adjustable per user?
> > Would it be changed often?  Such questions and answers will lead us to the
> > right direction, I hope.
> I think this feature should not be adjustable by the user during runtime because,
> a)  It's based on the platform and OEM preference of having it (given it has power implications)

Well, the argument isn't equivalent with runtime PM.  The runtime PM
is supposed to keep the almost same functionality, hence it's
preferred to be enabled unless you hit a clear demerit (e.g. click
noise or such).  But this one is rather a trade-off between power-save
vs avoiding the drop of first samples.

The problem is that this feature blocks the whole runtime PM, even if
you don't use HDMI audio.  From that point, I guess many users prefer
this off for those who don't use HDMI audio.

> b) Changing it on the runtime will cause the issue of unbalanced power up/down sequence like you mentioned

This should be no problem if we do code right :)


thanks,

Takashi