On Thu, 20 Aug 2015, Takashi Iwai <tiwai@xxxxxxx> wrote:
> 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.
Personally I'm fine with this still going for v4.3 since to me it looks
like any regressions this might cause will be on your side. *grin*.
BR,
Jani.
>
> 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
>>
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
