[alsa-devel] [PATCH 4/4] ALSA: hda - Wake the codec up on pin/ELD notify events

[Takashi Iwai]

On Thu, 20 Aug 2015 11:41:42 +0200,
David Henningsson wrote:
> 
> 
> 
> On 2015-08-20 11:28, Takashi Iwai wrote:
> > On Wed, 19 Aug 2015 10:48:58 +0200,
> > David Henningsson wrote:
> >>
> >> Whenever there is an event from the i915 driver, wake the codec
> >> and recheck plug/unplug + ELD status.
> >>
> >> This fixes the issue with lost unsol events in power save mode,
> >> the codec and controller can now sleep in D3 and still know when
> >> the HDMI monitor has been connected.
> >>
> >> Signed-off-by: David Henningsson <david.henningsson at canonical.com>
> >
> > This addition looks fine, but then we'll get double notification for
> > the normal hotplug/unplug, one via component ops and another via unsol
> > event?
> 
> Right, in case the unsol event actually works...
> 
> I would argue that the normal case would be that the controller and 
> codec is in D3 which means that the unsol event never gets through - due 
> to hw limitations - which is what triggered this patch set in the first 
> place.
> 
> But yes, in some case we might get double notification, but this should 
> not cause any trouble in practice. The unsol event could be turned off, 
> but would it be okay to save that for a later patch set (so I don't miss 
> the upcoming merge window)?

In that case, it should be mentioned in the changelog at least.

This series came a bit too late for the merge window, so I'm not sure
whether this can get in.  I personally find it OK, so take my ack for
ALSA parts (patch 3/4), but the rest need review and ack from i915
guys.  And we don't know who to merge this, if any.  The changes are
almost even to i915 and hda.  I don't mind either way, via drm or
sound tree.

In anyway,
   Reviewed-by: Takashi Iwai <tiwai at suse.de>


thanks,

Takashi

> >
> >
> > thanks,
> >
> > Takashi
> >
> >> ---
> >>   sound/pci/hda/patch_hdmi.c | 22 +++++++++++++++++++++-
> >>   1 file changed, 21 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> >> index a97db5f..932292c 100644
> >> --- a/sound/pci/hda/patch_hdmi.c
> >> +++ b/sound/pci/hda/patch_hdmi.c
> >> @@ -37,6 +37,8 @@
> >>   #include <sound/jack.h>
> >>   #include <sound/asoundef.h>
> >>   #include <sound/tlv.h>
> >> +#include <sound/hdaudio.h>
> >> +#include <sound/hda_i915.h>
> >>   #include "hda_codec.h"
> >>   #include "hda_local.h"
> >>   #include "hda_jack.h"
> >> @@ -144,6 +146,9 @@ struct hdmi_spec {
> >>   	 */
> >>   	struct hda_multi_out multiout;
> >>   	struct hda_pcm_stream pcm_playback;
> >> +
> >> +	/* i915/powerwell (Haswell+/Valleyview+) specific */
> >> +	struct i915_audio_component_audio_ops i915_audio_ops;
> >>   };
> >>
> >>
> >> @@ -2191,6 +2196,9 @@ static void generic_hdmi_free(struct hda_codec *codec)
> >>   	struct hdmi_spec *spec = codec->spec;
> >>   	int pin_idx;
> >>
> >> +	if (is_haswell_plus(codec) || is_valleyview_plus(codec))
> >> +		snd_hdac_i915_register_notifier(NULL);
> >> +
> >>   	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
> >>   		struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
> >>
> >> @@ -2316,6 +2324,14 @@ static void haswell_set_power_state(struct hda_codec *codec, hda_nid_t fg,
> >>   	snd_hda_codec_set_power_to_all(codec, fg, power_state);
> >>   }
> >>
> >> +static void intel_pin_eld_notify(void *audio_ptr, int port, int port_mst_index)
> >> +{
> >> +	struct hda_codec *codec = audio_ptr;
> >> +	int pin_nid = port + 0x04;
> >> +
> >> +	check_presence_and_report(codec, pin_nid);
> >> +}
> >> +
> >>   static int patch_generic_hdmi(struct hda_codec *codec)
> >>   {
> >>   	struct hdmi_spec *spec;
> >> @@ -2342,8 +2358,12 @@ static int patch_generic_hdmi(struct hda_codec *codec)
> >>   	if (is_valleyview_plus(codec) || is_skylake(codec))
> >>   		codec->core.link_power_control = 1;
> >>
> >> -	if (is_haswell_plus(codec) || is_valleyview_plus(codec))
> >> +	if (is_haswell_plus(codec) || is_valleyview_plus(codec)) {
> >>   		codec->depop_delay = 0;
> >> +		spec->i915_audio_ops.audio_ptr = codec;
> >> +		spec->i915_audio_ops.pin_eld_notify = intel_pin_eld_notify;
> >> +		snd_hdac_i915_register_notifier(&spec->i915_audio_ops);
> >> +	}
> >>
> >>   	if (hdmi_parse_codec(codec) < 0) {
> >>   		codec->spec = NULL;
> >> --
> >> 1.9.1
> >>
> >
> 
> -- 
> David Henningsson, Canonical Ltd.
> https://launchpad.net/~diwic
>