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

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)