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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> 2 nit-picks that I missed in previous versions of this patch, sorry:
> 
> > diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index
> > 7ba542e45a3d..8804808410b3 100644
> > --- a/sound/pci/hda/Kconfig
> > +++ b/sound/pci/hda/Kconfig
> > @@ -232,4 +232,20 @@ config SND_HDA_POWER_SAVE_DEFAULT
> >
> >   endif
> >
> > +config SND_HDA_INTEL_HDMI_SILENT_STREAM
> > +	bool "Enable Silent Stream always for HDMI"
> > +	depends on SND_HDA
> 
> nit-pick: should this be 'depends on SND_HDA_INTEL'?
> if not, this 'depends on SND_HDA' is redundant, already within an 'if SND_HDA'
> block
ack.
> 
> > +	help
> > +	  Intel hardware has a feature called 'silent stream', that
> > +	  keeps external HDMI receiver's analog circuitry powered on
> > +	  avoiding 2-3 sec silence during playback start. This mechanism
> > +	  relies on an info packet and preventing the codec from going to
> > +	  D3. (increasing the platform static power consumption when a
> > +	  HDMI receiver is plugged-in). 2-3 sec silence at the playback
> > +	  start is expected whenever there is format change. (default is
> > +	  2 channel format).
> > +	  Say Y to enable Silent Stream feature.
> > +
> > +endif
> > +
> 
> [...]
> 
> >   /* update ELD and jack state via audio component */
> >   static void sync_eld_via_acomp(struct hda_codec *codec,
> >   			       struct hdmi_spec_per_pin *per_pin)
> >   {
> >   	struct hdmi_spec *spec = codec->spec;
> >   	struct hdmi_eld *eld = &spec->temp_eld;
> > +	bool monitor_prev, monitor_next;
> >
> >   	mutex_lock(&per_pin->lock);
> >   	eld->monitor_present = false;
> > +	monitor_prev = per_pin->sink_eld.monitor_present;
> >   	eld->eld_size = snd_hdac_acomp_get_eld(&codec->core, per_pin-
> >pin_nid,
> >   				      per_pin->dev_id, &eld->monitor_present,
> >   				      eld->eld_buffer, ELD_MAX_SIZE);
> >   	eld->eld_valid = (eld->eld_size > 0);
> >   	update_eld(codec, per_pin, eld, 0);
> > +	monitor_next = per_pin->sink_eld.monitor_present;
> >   	mutex_unlock(&per_pin->lock);
> > +
> > +	/*
> > +	 * Power-up will call hdmi_present_sense, so the PM calls
> > +	 * have to be done without mutex held.
> > +	 */
> > +
> > +	if (enable_silent_stream) {
> > +		if (!monitor_prev && monitor_next) {
> > +			snd_hda_power_up_pm(codec);
> 
> nit-pick: is there a need to test the return value? I see this in patch_hdmi.c
Since this parent function returns void, I could probably add a print stating the failure.
> 
> 	ret = snd_hda_power_up_pm(codec);
> 	if (ret < 0 && pm_runtime_suspended(hda_codec_dev(codec)))
> 		goto out;
> 
> > +			silent_stream_enable(codec, per_pin);
> > +		} else if (monitor_prev && !monitor_next)
> > +			snd_hda_power_down_pm(codec);
> > +	}
> >   }
> >
> >   static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin,
> > int repoll)
> >




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux