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@xxxxxxxxxxxxx>
> >
> > 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@xxxxxxx>
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
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
