[PATCH 1/2] ALSA: hdmi - poll eld at resume time

[PATCH 1/2] ALSA: hdmi - poll eld at resume time

[Wang Xingchao]

Hdmi driver may not receive intrinsic event from gfx side when
it's in runtime suspend mode. There's no ELD info when exit from
runtime suspend. This patch avoid missing ELD info.

Signed-off-by: Wang Xingchao <xingchao.wang@linux.intel.com>
---
 sound/pci/hda/patch_hdmi.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 7803ddd..cb8ac66 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1868,12 +1868,33 @@ static void generic_hdmi_free(struct hda_codec *codec)
 	kfree(spec);
 }
 
+#ifdef CONFIG_PM
+static int generic_hdmi_resume(struct hda_codec *codec)
+{
+	struct hdmi_spec *spec = codec->spec;
+	int pin_idx;
+
+	generic_hdmi_init(codec);
+	snd_hda_codec_resume_amp(codec);
+	snd_hda_codec_resume_cache(codec);
+
+	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
+		struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
+		hdmi_present_sense(per_pin, 1);
+	}
+	return 0;
+}
+#endif
+
 static const struct hda_codec_ops generic_hdmi_patch_ops = {
 	.init			= generic_hdmi_init,
 	.free			= generic_hdmi_free,
 	.build_pcms		= generic_hdmi_build_pcms,
 	.build_controls		= generic_hdmi_build_controls,
 	.unsol_event		= hdmi_unsol_event,
+#ifdef CONFIG_PM
+	.resume 		= generic_hdmi_resume,
+#endif
 };
 
 
-- 
1.8.1.2

Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time

[Wang, Xingchao]

Hi Takashi,

The two patches fix below issues:
1) controller/codec enter suspend mode, insert monitor, gfx side update eld info, but eld# has no valid eld info in audio driver side.
2) hdmi monitor connected,  eld# info keep valid, controller/codec enter suspend mode, remove hdmi monitor, eld# still exist.

thanks
--xingchao

> -----Original Message-----
> From: Wang Xingchao [mailto:xingchao.wang@linux.intel.com]
> Sent: Monday, June 24, 2013 7:45 PM
> To: tiwai@suse.de
> Cc: alsa-devel@alsa-project.org; Wang, Xingchao; Wang Xingchao
> Subject: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> 
> Hdmi driver may not receive intrinsic event from gfx side when it's in runtime
> suspend mode. There's no ELD info when exit from runtime suspend. This patch
> avoid missing ELD info.
> 
> Signed-off-by: Wang Xingchao <xingchao.wang@linux.intel.com>
> ---
>  sound/pci/hda/patch_hdmi.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index
> 7803ddd..cb8ac66 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -1868,12 +1868,33 @@ static void generic_hdmi_free(struct hda_codec
> *codec)
>  	kfree(spec);
>  }
> 
> +#ifdef CONFIG_PM
> +static int generic_hdmi_resume(struct hda_codec *codec) {
> +	struct hdmi_spec *spec = codec->spec;
> +	int pin_idx;
> +
> +	generic_hdmi_init(codec);
> +	snd_hda_codec_resume_amp(codec);
> +	snd_hda_codec_resume_cache(codec);
> +
> +	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
> +		struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
> +		hdmi_present_sense(per_pin, 1);
> +	}
> +	return 0;
> +}
> +#endif
> +
>  static const struct hda_codec_ops generic_hdmi_patch_ops = {
>  	.init			= generic_hdmi_init,
>  	.free			= generic_hdmi_free,
>  	.build_pcms		= generic_hdmi_build_pcms,
>  	.build_controls		= generic_hdmi_build_controls,
>  	.unsol_event		= hdmi_unsol_event,
> +#ifdef CONFIG_PM
> +	.resume 		= generic_hdmi_resume,
> +#endif
>  };
> 
> 
> --
> 1.8.1.2

Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time

[Takashi Iwai]

At Mon, 24 Jun 2013 07:45:23 -0400,
Wang Xingchao wrote:
> 
> Hdmi driver may not receive intrinsic event from gfx side when
> it's in runtime suspend mode. There's no ELD info when exit from
> runtime suspend. This patch avoid missing ELD info.

hda_call_codec_resume() sets the jack detection all dirty, thus each
jack detection callback should be called at resume.  Didn't it work as
expected?


Takashi

> 
> Signed-off-by: Wang Xingchao <xingchao.wang@linux.intel.com>
> ---
>  sound/pci/hda/patch_hdmi.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 7803ddd..cb8ac66 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -1868,12 +1868,33 @@ static void generic_hdmi_free(struct hda_codec *codec)
>  	kfree(spec);
>  }
>  
> +#ifdef CONFIG_PM
> +static int generic_hdmi_resume(struct hda_codec *codec)
> +{
> +	struct hdmi_spec *spec = codec->spec;
> +	int pin_idx;
> +
> +	generic_hdmi_init(codec);
> +	snd_hda_codec_resume_amp(codec);
> +	snd_hda_codec_resume_cache(codec);
> +
> +	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
> +		struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
> +		hdmi_present_sense(per_pin, 1);
> +	}
> +	return 0;
> +}
> +#endif
> +
>  static const struct hda_codec_ops generic_hdmi_patch_ops = {
>  	.init			= generic_hdmi_init,
>  	.free			= generic_hdmi_free,
>  	.build_pcms		= generic_hdmi_build_pcms,
>  	.build_controls		= generic_hdmi_build_controls,
>  	.unsol_event		= hdmi_unsol_event,
> +#ifdef CONFIG_PM
> +	.resume 		= generic_hdmi_resume,
> +#endif
>  };
>  
>  
> -- 
> 1.8.1.2
> 

Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time

[Wang, Xingchao]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Monday, June 24, 2013 7:33 PM
> To: Wang Xingchao
> Cc: alsa-devel@alsa-project.org; Wang, Xingchao
> Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> 
> At Mon, 24 Jun 2013 07:45:23 -0400,
> Wang Xingchao wrote:
> >
> > Hdmi driver may not receive intrinsic event from gfx side when it's in
> > runtime suspend mode. There's no ELD info when exit from runtime
> > suspend. This patch avoid missing ELD info.
> 
> hda_call_codec_resume() sets the jack detection all dirty, thus each jack
> detection callback should be called at resume.  Didn't it work as expected?

I would double check that. In my test, it doesnot work as expected.

--xingchao
> 
> 
> Takashi
> 
> >
> > Signed-off-by: Wang Xingchao <xingchao.wang@linux.intel.com>
> > ---
> >  sound/pci/hda/patch_hdmi.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> > index 7803ddd..cb8ac66 100644
> > --- a/sound/pci/hda/patch_hdmi.c
> > +++ b/sound/pci/hda/patch_hdmi.c
> > @@ -1868,12 +1868,33 @@ static void generic_hdmi_free(struct hda_codec
> *codec)
> >  	kfree(spec);
> >  }
> >
> > +#ifdef CONFIG_PM
> > +static int generic_hdmi_resume(struct hda_codec *codec) {
> > +	struct hdmi_spec *spec = codec->spec;
> > +	int pin_idx;
> > +
> > +	generic_hdmi_init(codec);
> > +	snd_hda_codec_resume_amp(codec);
> > +	snd_hda_codec_resume_cache(codec);
> > +
> > +	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
> > +		struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
> > +		hdmi_present_sense(per_pin, 1);
> > +	}
> > +	return 0;
> > +}
> > +#endif
> > +
> >  static const struct hda_codec_ops generic_hdmi_patch_ops = {
> >  	.init			= generic_hdmi_init,
> >  	.free			= generic_hdmi_free,
> >  	.build_pcms		= generic_hdmi_build_pcms,
> >  	.build_controls		= generic_hdmi_build_controls,
> >  	.unsol_event		= hdmi_unsol_event,
> > +#ifdef CONFIG_PM
> > +	.resume 		= generic_hdmi_resume,
> > +#endif
> >  };
> >
> >
> > --
> > 1.8.1.2
> >

Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time

[Takashi Iwai]

At Mon, 24 Jun 2013 12:19:42 +0000,
Wang, Xingchao wrote:
> 
> 
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Monday, June 24, 2013 7:33 PM
> > To: Wang Xingchao
> > Cc: alsa-devel@alsa-project.org; Wang, Xingchao
> > Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> > 
> > At Mon, 24 Jun 2013 07:45:23 -0400,
> > Wang Xingchao wrote:
> > >
> > > Hdmi driver may not receive intrinsic event from gfx side when it's in
> > > runtime suspend mode. There's no ELD info when exit from runtime
> > > suspend. This patch avoid missing ELD info.
> > 
> > hda_call_codec_resume() sets the jack detection all dirty, thus each jack
> > detection callback should be called at resume.  Didn't it work as expected?
> 
> I would double check that. In my test, it doesnot work as expected.

OK, I found the problem.  patch_hdmi.c enables the jack detection
stuff without the callback, so the resume code triggers the check of
jack detection but only updates the kcontrols.

How about the patch below instead?


Takashi

---
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 8428763..3059d69 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -950,14 +950,10 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec, int pin_idx,
  * Unsolicited events
  */
 
-static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll);
-
 static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
 {
-	struct hdmi_spec *spec = codec->spec;
 	int tag = res >> AC_UNSOL_RES_TAG_SHIFT;
 	int pin_nid;
-	int pin_idx;
 	struct hda_jack_tbl *jack;
 
 	jack = snd_hda_jack_tbl_get_from_tag(codec, tag);
@@ -970,12 +966,6 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
 		"HDMI hot plug event: Codec=%d Pin=%d Presence_Detect=%d ELD_Valid=%d\n",
 		codec->addr, pin_nid,
 		!!(res & AC_UNSOL_RES_PD), !!(res & AC_UNSOL_RES_ELDV));
-
-	pin_idx = pin_nid_to_pin_index(spec, pin_nid);
-	if (pin_idx < 0)
-		return;
-
-	hdmi_present_sense(get_pin(spec, pin_idx), 1);
 	snd_hda_jack_report_sync(codec);
 }
 
@@ -1358,6 +1348,14 @@ static void hdmi_repoll_eld(struct work_struct *work)
 	hdmi_present_sense(per_pin, per_pin->repoll_count);
 }
 
+static void hdmi_jack_detect_cb(struct hda_codec *codec, struct hda_jack_tbl *jack)
+{
+	struct hdmi_spec *spec = codec->spec;
+	int pin_idx = pin_nid_to_pin_index(spec, jack->nid);
+	if (pin_idx >= 0)
+		hdmi_present_sense(get_pin(spec, pin_idx), 1);
+}
+
 static void intel_haswell_fixup_connect_list(struct hda_codec *codec,
 					     hda_nid_t nid);
 
@@ -1827,7 +1825,8 @@ static int generic_hdmi_init(struct hda_codec *codec)
 		hda_nid_t pin_nid = per_pin->pin_nid;
 
 		hdmi_init_pin(codec, pin_nid);
-		snd_hda_jack_detect_enable(codec, pin_nid, pin_nid);
+		snd_hda_jack_detect_enable_callback(codec, pin_nid, pin_nid,
+						    hdmi_jack_detect_cb);
 	}
 	return 0;
 }

Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time

[Wang, Xingchao]

Hi Takashi,


> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Monday, June 24, 2013 8:50 PM
> To: Wang, Xingchao
> Cc: alsa-devel@alsa-project.org; Wang Xingchao
> Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> 
> At Mon, 24 Jun 2013 12:19:42 +0000,
> Wang, Xingchao wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > Sent: Monday, June 24, 2013 7:33 PM
> > > To: Wang Xingchao
> > > Cc: alsa-devel@alsa-project.org; Wang, Xingchao
> > > Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> > >
> > > At Mon, 24 Jun 2013 07:45:23 -0400,
> > > Wang Xingchao wrote:
> > > >
> > > > Hdmi driver may not receive intrinsic event from gfx side when
> > > > it's in runtime suspend mode. There's no ELD info when exit from
> > > > runtime suspend. This patch avoid missing ELD info.
> > >
> > > hda_call_codec_resume() sets the jack detection all dirty, thus each
> > > jack detection callback should be called at resume.  Didn't it work as
> expected?
> >
> > I would double check that. In my test, it doesnot work as expected.
> 
> OK, I found the problem.  patch_hdmi.c enables the jack detection stuff
> without the callback, so the resume code triggers the check of jack detection
> but only updates the kcontrols.

You patch did not resolve the issue.
I added some debug log, the callback wasnot called at all.
I will continue to check it.

Thanks
--xingchao
> 
> How about the patch below instead?
> 
> 
> Takashi
> 
> ---
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index
> 8428763..3059d69 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -950,14 +950,10 @@ static void hdmi_setup_audio_infoframe(struct
> hda_codec *codec, int pin_idx,
>   * Unsolicited events
>   */
> 
> -static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int
> repoll);
> -
>  static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
> {
> -	struct hdmi_spec *spec = codec->spec;
>  	int tag = res >> AC_UNSOL_RES_TAG_SHIFT;
>  	int pin_nid;
> -	int pin_idx;
>  	struct hda_jack_tbl *jack;
> 
>  	jack = snd_hda_jack_tbl_get_from_tag(codec, tag); @@ -970,12 +966,6
> @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int
> res)
>  		"HDMI hot plug event: Codec=%d Pin=%d Presence_Detect=%d
> ELD_Valid=%d\n",
>  		codec->addr, pin_nid,
>  		!!(res & AC_UNSOL_RES_PD), !!(res & AC_UNSOL_RES_ELDV));
> -
> -	pin_idx = pin_nid_to_pin_index(spec, pin_nid);
> -	if (pin_idx < 0)
> -		return;
> -
> -	hdmi_present_sense(get_pin(spec, pin_idx), 1);
>  	snd_hda_jack_report_sync(codec);
>  }
> 
> @@ -1358,6 +1348,14 @@ static void hdmi_repoll_eld(struct work_struct
> *work)
>  	hdmi_present_sense(per_pin, per_pin->repoll_count);  }
> 
> +static void hdmi_jack_detect_cb(struct hda_codec *codec, struct
> +hda_jack_tbl *jack) {
> +	struct hdmi_spec *spec = codec->spec;
> +	int pin_idx = pin_nid_to_pin_index(spec, jack->nid);
> +	if (pin_idx >= 0)
> +		hdmi_present_sense(get_pin(spec, pin_idx), 1); }
> +
>  static void intel_haswell_fixup_connect_list(struct hda_codec *codec,
>  					     hda_nid_t nid);
> 
> @@ -1827,7 +1825,8 @@ static int generic_hdmi_init(struct hda_codec
> *codec)
>  		hda_nid_t pin_nid = per_pin->pin_nid;
> 
>  		hdmi_init_pin(codec, pin_nid);
> -		snd_hda_jack_detect_enable(codec, pin_nid, pin_nid);
> +		snd_hda_jack_detect_enable_callback(codec, pin_nid, pin_nid,
> +						    hdmi_jack_detect_cb);
>  	}
>  	return 0;
>  }

Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time

[Takashi Iwai]

At Tue, 25 Jun 2013 04:54:05 +0000,
Wang, Xingchao wrote:
> 
> Hi Takashi,
> 
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Monday, June 24, 2013 8:50 PM
> > To: Wang, Xingchao
> > Cc: alsa-devel@alsa-project.org; Wang Xingchao
> > Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> > 
> > At Mon, 24 Jun 2013 12:19:42 +0000,
> > Wang, Xingchao wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > Sent: Monday, June 24, 2013 7:33 PM
> > > > To: Wang Xingchao
> > > > Cc: alsa-devel@alsa-project.org; Wang, Xingchao
> > > > Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> > > >
> > > > At Mon, 24 Jun 2013 07:45:23 -0400,
> > > > Wang Xingchao wrote:
> > > > >
> > > > > Hdmi driver may not receive intrinsic event from gfx side when
> > > > > it's in runtime suspend mode. There's no ELD info when exit from
> > > > > runtime suspend. This patch avoid missing ELD info.
> > > >
> > > > hda_call_codec_resume() sets the jack detection all dirty, thus each
> > > > jack detection callback should be called at resume.  Didn't it work as
> > expected?
> > >
> > > I would double check that. In my test, it doesnot work as expected.
> > 
> > OK, I found the problem.  patch_hdmi.c enables the jack detection stuff
> > without the callback, so the resume code triggers the check of jack detection
> > but only updates the kcontrols.
> 
> You patch did not resolve the issue.
> I added some debug log, the callback wasnot called at all.

Even if you unplugged while runtime suspend?

The callback is called only when the plug status (i.e. the jack
detection state) change is detected at the resume time -- i.e. the
state the driver holds differs from the state at the resume.


Takashi

> I will continue to check it.
> 
> Thanks
> --xingchao
> > 
> > How about the patch below instead?
> > 
> > 
> > Takashi
> > 
> > ---
> > diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index
> > 8428763..3059d69 100644
> > --- a/sound/pci/hda/patch_hdmi.c
> > +++ b/sound/pci/hda/patch_hdmi.c
> > @@ -950,14 +950,10 @@ static void hdmi_setup_audio_infoframe(struct
> > hda_codec *codec, int pin_idx,
> >   * Unsolicited events
> >   */
> > 
> > -static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int
> > repoll);
> > -
> >  static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
> > {
> > -	struct hdmi_spec *spec = codec->spec;
> >  	int tag = res >> AC_UNSOL_RES_TAG_SHIFT;
> >  	int pin_nid;
> > -	int pin_idx;
> >  	struct hda_jack_tbl *jack;
> > 
> >  	jack = snd_hda_jack_tbl_get_from_tag(codec, tag); @@ -970,12 +966,6
> > @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int
> > res)
> >  		"HDMI hot plug event: Codec=%d Pin=%d Presence_Detect=%d
> > ELD_Valid=%d\n",
> >  		codec->addr, pin_nid,
> >  		!!(res & AC_UNSOL_RES_PD), !!(res & AC_UNSOL_RES_ELDV));
> > -
> > -	pin_idx = pin_nid_to_pin_index(spec, pin_nid);
> > -	if (pin_idx < 0)
> > -		return;
> > -
> > -	hdmi_present_sense(get_pin(spec, pin_idx), 1);
> >  	snd_hda_jack_report_sync(codec);
> >  }
> > 
> > @@ -1358,6 +1348,14 @@ static void hdmi_repoll_eld(struct work_struct
> > *work)
> >  	hdmi_present_sense(per_pin, per_pin->repoll_count);  }
> > 
> > +static void hdmi_jack_detect_cb(struct hda_codec *codec, struct
> > +hda_jack_tbl *jack) {
> > +	struct hdmi_spec *spec = codec->spec;
> > +	int pin_idx = pin_nid_to_pin_index(spec, jack->nid);
> > +	if (pin_idx >= 0)
> > +		hdmi_present_sense(get_pin(spec, pin_idx), 1); }
> > +
> >  static void intel_haswell_fixup_connect_list(struct hda_codec *codec,
> >  					     hda_nid_t nid);
> > 
> > @@ -1827,7 +1825,8 @@ static int generic_hdmi_init(struct hda_codec
> > *codec)
> >  		hda_nid_t pin_nid = per_pin->pin_nid;
> > 
> >  		hdmi_init_pin(codec, pin_nid);
> > -		snd_hda_jack_detect_enable(codec, pin_nid, pin_nid);
> > +		snd_hda_jack_detect_enable_callback(codec, pin_nid, pin_nid,
> > +						    hdmi_jack_detect_cb);
> >  	}
> >  	return 0;
> >  }
> 

Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time

[Wang, Xingchao]

Hi Takashi,

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Tuesday, June 25, 2013 2:07 PM
> To: Wang, Xingchao
> Cc: alsa-devel@alsa-project.org; Wang Xingchao
> Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> 
> At Tue, 25 Jun 2013 04:54:05 +0000,
> Wang, Xingchao wrote:
> >
> > Hi Takashi,
> >
> >
> > > -----Original Message-----
> > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > Sent: Monday, June 24, 2013 8:50 PM
> > > To: Wang, Xingchao
> > > Cc: alsa-devel@alsa-project.org; Wang Xingchao
> > > Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> > >
> > > At Mon, 24 Jun 2013 12:19:42 +0000,
> > > Wang, Xingchao wrote:
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > > Sent: Monday, June 24, 2013 7:33 PM
> > > > > To: Wang Xingchao
> > > > > Cc: alsa-devel@alsa-project.org; Wang, Xingchao
> > > > > Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> > > > >
> > > > > At Mon, 24 Jun 2013 07:45:23 -0400, Wang Xingchao wrote:
> > > > > >
> > > > > > Hdmi driver may not receive intrinsic event from gfx side when
> > > > > > it's in runtime suspend mode. There's no ELD info when exit
> > > > > > from runtime suspend. This patch avoid missing ELD info.
> > > > >
> > > > > hda_call_codec_resume() sets the jack detection all dirty, thus
> > > > > each jack detection callback should be called at resume.  Didn't
> > > > > it work as
> > > expected?
> > > >
> > > > I would double check that. In my test, it doesnot work as expected.
> > >
> > > OK, I found the problem.  patch_hdmi.c enables the jack detection
> > > stuff without the callback, so the resume code triggers the check of
> > > jack detection but only updates the kcontrols.
> >
> > You patch did not resolve the issue.
> > I added some debug log, the callback wasnot called at all.
> 
> Even if you unplugged while runtime suspend?

Yes, the controller/codec suspended in runtime already.

> 
> The callback is called only when the plug status (i.e. the jack detection state)
> change is detected at the resume time -- i.e. the state the driver holds differs
> from the state at the resume.
> 

Do you assume jackpoll_interval be non-zero? If so the hda_jackpoll_work will continue to run periodically.
IMO the callback just need be called only once at resume time.

Thanks
--xingchao

Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time

[Takashi Iwai]

At Tue, 25 Jun 2013 06:34:49 +0000,
Wang, Xingchao wrote:
> 
> Hi Takashi,
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Tuesday, June 25, 2013 2:07 PM
> > To: Wang, Xingchao
> > Cc: alsa-devel@alsa-project.org; Wang Xingchao
> > Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> > 
> > At Tue, 25 Jun 2013 04:54:05 +0000,
> > Wang, Xingchao wrote:
> > >
> > > Hi Takashi,
> > >
> > >
> > > > -----Original Message-----
> > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > Sent: Monday, June 24, 2013 8:50 PM
> > > > To: Wang, Xingchao
> > > > Cc: alsa-devel@alsa-project.org; Wang Xingchao
> > > > Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> > > >
> > > > At Mon, 24 Jun 2013 12:19:42 +0000,
> > > > Wang, Xingchao wrote:
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > > > Sent: Monday, June 24, 2013 7:33 PM
> > > > > > To: Wang Xingchao
> > > > > > Cc: alsa-devel@alsa-project.org; Wang, Xingchao
> > > > > > Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> > > > > >
> > > > > > At Mon, 24 Jun 2013 07:45:23 -0400, Wang Xingchao wrote:
> > > > > > >
> > > > > > > Hdmi driver may not receive intrinsic event from gfx side when
> > > > > > > it's in runtime suspend mode. There's no ELD info when exit
> > > > > > > from runtime suspend. This patch avoid missing ELD info.
> > > > > >
> > > > > > hda_call_codec_resume() sets the jack detection all dirty, thus
> > > > > > each jack detection callback should be called at resume.  Didn't
> > > > > > it work as
> > > > expected?
> > > > >
> > > > > I would double check that. In my test, it doesnot work as expected.
> > > >
> > > > OK, I found the problem.  patch_hdmi.c enables the jack detection
> > > > stuff without the callback, so the resume code triggers the check of
> > > > jack detection but only updates the kcontrols.
> > >
> > > You patch did not resolve the issue.
> > > I added some debug log, the callback wasnot called at all.
> > 
> > Even if you unplugged while runtime suspend?
> 
> Yes, the controller/codec suspended in runtime already.
> 
> > 
> > The callback is called only when the plug status (i.e. the jack detection state)
> > change is detected at the resume time -- i.e. the state the driver holds differs
> > from the state at the resume.
> > 
> 
> Do you assume jackpoll_interval be non-zero? If so the hda_jackpoll_work will continue to run periodically.
> IMO the callback just need be called only once at resume time.

Hmm, the problem is that the callback updater isn't called in the
resume path as default.  The oneliner below will fix it.

But, looking through all changes, maybe your first patch is easier to
apply as is now.  I'll take it.

We need a bit more cleanups over init and resume codes after all.
The init callback is called before calling build_controls and
build_pcm, and the very same init callback is called also in the
resume path (if the resume callback is undefined).  This causes the
confusion, too.

In patch_hdmi.c, the init changes only the pin control and amp, and
can't touch anything else, because it cannot call any pin-related
events at that point since the necessary resources are added in the
later point, generic_hdmi_build_controls().


thanks,

Takashi



> 
> Thanks
> --xingchao
> 

Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time

[Takashi Iwai]

At Tue, 25 Jun 2013 09:06:32 +0200,
Takashi Iwai wrote:
> 
> At Tue, 25 Jun 2013 06:34:49 +0000,
> Wang, Xingchao wrote:
> > 
> > Hi Takashi,
> > 
> > > -----Original Message-----
> > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > Sent: Tuesday, June 25, 2013 2:07 PM
> > > To: Wang, Xingchao
> > > Cc: alsa-devel@alsa-project.org; Wang Xingchao
> > > Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> > > 
> > > At Tue, 25 Jun 2013 04:54:05 +0000,
> > > Wang, Xingchao wrote:
> > > >
> > > > Hi Takashi,
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > > Sent: Monday, June 24, 2013 8:50 PM
> > > > > To: Wang, Xingchao
> > > > > Cc: alsa-devel@alsa-project.org; Wang Xingchao
> > > > > Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> > > > >
> > > > > At Mon, 24 Jun 2013 12:19:42 +0000,
> > > > > Wang, Xingchao wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > > > > Sent: Monday, June 24, 2013 7:33 PM
> > > > > > > To: Wang Xingchao
> > > > > > > Cc: alsa-devel@alsa-project.org; Wang, Xingchao
> > > > > > > Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> > > > > > >
> > > > > > > At Mon, 24 Jun 2013 07:45:23 -0400, Wang Xingchao wrote:
> > > > > > > >
> > > > > > > > Hdmi driver may not receive intrinsic event from gfx side when
> > > > > > > > it's in runtime suspend mode. There's no ELD info when exit
> > > > > > > > from runtime suspend. This patch avoid missing ELD info.
> > > > > > >
> > > > > > > hda_call_codec_resume() sets the jack detection all dirty, thus
> > > > > > > each jack detection callback should be called at resume.  Didn't
> > > > > > > it work as
> > > > > expected?
> > > > > >
> > > > > > I would double check that. In my test, it doesnot work as expected.
> > > > >
> > > > > OK, I found the problem.  patch_hdmi.c enables the jack detection
> > > > > stuff without the callback, so the resume code triggers the check of
> > > > > jack detection but only updates the kcontrols.
> > > >
> > > > You patch did not resolve the issue.
> > > > I added some debug log, the callback wasnot called at all.
> > > 
> > > Even if you unplugged while runtime suspend?
> > 
> > Yes, the controller/codec suspended in runtime already.
> > 
> > > 
> > > The callback is called only when the plug status (i.e. the jack detection state)
> > > change is detected at the resume time -- i.e. the state the driver holds differs
> > > from the state at the resume.
> > > 
> > 
> > Do you assume jackpoll_interval be non-zero? If so the hda_jackpoll_work will continue to run periodically.
> > IMO the callback just need be called only once at resume time.
> 
> Hmm, the problem is that the callback updater isn't called in the
> resume path as default.  The oneliner below will fix it.

Forgot to attach...


Takashi

---
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 35090b3..86d4709 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -3942,6 +3942,7 @@ static void hda_call_codec_resume(struct hda_codec *codec)
 			codec->patch_ops.init(codec);
 		snd_hda_codec_resume_amp(codec);
 		snd_hda_codec_resume_cache(codec);
+		snd_hda_jack_poll_all(codec);
 	}
 
 	if (codec->jackpoll_interval)

Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time

[Wang, Xingchao]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Tuesday, June 25, 2013 3:09 PM
> To: Wang, Xingchao
> Cc: alsa-devel@alsa-project.org; Wang Xingchao
> Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> 
> At Tue, 25 Jun 2013 09:06:32 +0200,
> Takashi Iwai wrote:
> >
> > At Tue, 25 Jun 2013 06:34:49 +0000,
> > Wang, Xingchao wrote:
> > >
> > > Hi Takashi,
> > >
> > > > -----Original Message-----
> > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > Sent: Tuesday, June 25, 2013 2:07 PM
> > > > To: Wang, Xingchao
> > > > Cc: alsa-devel@alsa-project.org; Wang Xingchao
> > > > Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> > > >
> > > > At Tue, 25 Jun 2013 04:54:05 +0000, Wang, Xingchao wrote:
> > > > >
> > > > > Hi Takashi,
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > > > Sent: Monday, June 24, 2013 8:50 PM
> > > > > > To: Wang, Xingchao
> > > > > > Cc: alsa-devel@alsa-project.org; Wang Xingchao
> > > > > > Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> > > > > >
> > > > > > At Mon, 24 Jun 2013 12:19:42 +0000, Wang, Xingchao wrote:
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > > > > > Sent: Monday, June 24, 2013 7:33 PM
> > > > > > > > To: Wang Xingchao
> > > > > > > > Cc: alsa-devel@alsa-project.org; Wang, Xingchao
> > > > > > > > Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume
> > > > > > > > time
> > > > > > > >
> > > > > > > > At Mon, 24 Jun 2013 07:45:23 -0400, Wang Xingchao wrote:
> > > > > > > > >
> > > > > > > > > Hdmi driver may not receive intrinsic event from gfx
> > > > > > > > > side when it's in runtime suspend mode. There's no ELD
> > > > > > > > > info when exit from runtime suspend. This patch avoid missing
> ELD info.
> > > > > > > >
> > > > > > > > hda_call_codec_resume() sets the jack detection all dirty,
> > > > > > > > thus each jack detection callback should be called at
> > > > > > > > resume.  Didn't it work as
> > > > > > expected?
> > > > > > >
> > > > > > > I would double check that. In my test, it doesnot work as expected.
> > > > > >
> > > > > > OK, I found the problem.  patch_hdmi.c enables the jack
> > > > > > detection stuff without the callback, so the resume code
> > > > > > triggers the check of jack detection but only updates the kcontrols.
> > > > >
> > > > > You patch did not resolve the issue.
> > > > > I added some debug log, the callback wasnot called at all.
> > > >
> > > > Even if you unplugged while runtime suspend?
> > >
> > > Yes, the controller/codec suspended in runtime already.
> > >
> > > >
> > > > The callback is called only when the plug status (i.e. the jack
> > > > detection state) change is detected at the resume time -- i.e. the
> > > > state the driver holds differs from the state at the resume.
> > > >
> > >
> > > Do you assume jackpoll_interval be non-zero? If so the hda_jackpoll_work
> will continue to run periodically.
> > > IMO the callback just need be called only once at resume time.
> >
> > Hmm, the problem is that the callback updater isn't called in the
> > resume path as default.  The oneliner below will fix it.
> 
> Forgot to attach...

Yes, it works if call snd_hda_jack_poll_all(codec) directly, but sometimes read pin sense value is 0xc000,0000 even unplug monitor, and eld# will keep valid all the time.
Although sometimes it works(the ELD info refreshed correctly), you have to wake up controller/codec before check eld info(cat codec# or play a piece of audio).

Thanks
--xingchao

> 
> 
> Takashi
> 
> ---
> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index
> 35090b3..86d4709 100644
> --- a/sound/pci/hda/hda_codec.c
> +++ b/sound/pci/hda/hda_codec.c
> @@ -3942,6 +3942,7 @@ static void hda_call_codec_resume(struct
> hda_codec *codec)
>  			codec->patch_ops.init(codec);
>  		snd_hda_codec_resume_amp(codec);
>  		snd_hda_codec_resume_cache(codec);
> +		snd_hda_jack_poll_all(codec);
>  	}
> 
>  	if (codec->jackpoll_interval)

Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time

[Wang, Xingchao]

Hi Takashi,

> -----Original Message-----
> From: Wang, Xingchao
> Sent: Tuesday, June 25, 2013 4:30 PM
> To: Takashi Iwai
> Cc: alsa-devel@alsa-project.org; Wang Xingchao

[snip]

> > > >
> > > > Do you assume jackpoll_interval be non-zero? If so the
> > > > hda_jackpoll_work
> > will continue to run periodically.
> > > > IMO the callback just need be called only once at resume time.
> > >
> > > Hmm, the problem is that the callback updater isn't called in the
> > > resume path as default.  The oneliner below will fix it.
> >
> > Forgot to attach...
> 
> Yes, it works if call snd_hda_jack_poll_all(codec) directly, but sometimes read
> pin sense value is 0xc000,0000 even unplug monitor, and eld# will keep valid all
> the time.
> Although sometimes it works(the ELD info refreshed correctly), you have to
> wake up controller/codec before check eld info(cat codec# or play a piece of
> audio).

The fix is not enough.
When hotplug event happen and codec/controller in active state, hdmi_intrinsic_event() will be called.
In such scenario, jack event would be reported but not handled(hdmi_present_sense() was moved to callback).
So there's no eld info updated, have to trigger codec_resume to update the eld info.

If App depends on ELD# info to check external monitor status, it may cause confuse as eld# under /proc donot
Provide realtime information.

Thanks
--xingchao
> 
> Thanks
> --xingchao
> 
> >
> >
> > Takashi
> >
> > ---
> > diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> > index
> > 35090b3..86d4709 100644
> > --- a/sound/pci/hda/hda_codec.c
> > +++ b/sound/pci/hda/hda_codec.c
> > @@ -3942,6 +3942,7 @@ static void hda_call_codec_resume(struct
> > hda_codec *codec)
> >  			codec->patch_ops.init(codec);
> >  		snd_hda_codec_resume_amp(codec);
> >  		snd_hda_codec_resume_cache(codec);
> > +		snd_hda_jack_poll_all(codec);
> >  	}
> >
> >  	if (codec->jackpoll_interval)

Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time

[Wang, Xingchao]

> -----Original Message-----
> From: Wang, Xingchao
> Sent: Tuesday, June 25, 2013 12:54 PM
> To: 'Takashi Iwai'
> Cc: alsa-devel@alsa-project.org; Wang Xingchao
> Subject: RE: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> 
> Hi Takashi,
> 
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Monday, June 24, 2013 8:50 PM
> > To: Wang, Xingchao
> > Cc: alsa-devel@alsa-project.org; Wang Xingchao
> > Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> >
> > At Mon, 24 Jun 2013 12:19:42 +0000,
> > Wang, Xingchao wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > Sent: Monday, June 24, 2013 7:33 PM
> > > > To: Wang Xingchao
> > > > Cc: alsa-devel@alsa-project.org; Wang, Xingchao
> > > > Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
> > > >
> > > > At Mon, 24 Jun 2013 07:45:23 -0400, Wang Xingchao wrote:
> > > > >
> > > > > Hdmi driver may not receive intrinsic event from gfx side when
> > > > > it's in runtime suspend mode. There's no ELD info when exit from
> > > > > runtime suspend. This patch avoid missing ELD info.
> > > >
> > > > hda_call_codec_resume() sets the jack detection all dirty, thus
> > > > each jack detection callback should be called at resume.  Didn't
> > > > it work as
> > expected?
> > >
> > > I would double check that. In my test, it doesnot work as expected.
> >
> > OK, I found the problem.  patch_hdmi.c enables the jack detection
> > stuff without the callback, so the resume code triggers the check of
> > jack detection but only updates the kcontrols.
> 
> You patch did not resolve the issue.
> I added some debug log, the callback wasnot called at all.
> I will continue to check it.
> 
Seems jackpoll_interval be 0 by default, and the callback was not called.
I think this would be a common issue not limited to Haswell, so what about call had_jackpoll_work()
in snd_hda_codec_resume()?

Thanks
--xingchao

> Thanks
> --xingchao
> >
> > How about the patch below instead?
> >
> >
> > Takashi
> >
> > ---
> > diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> > index
> > 8428763..3059d69 100644
> > --- a/sound/pci/hda/patch_hdmi.c
> > +++ b/sound/pci/hda/patch_hdmi.c
> > @@ -950,14 +950,10 @@ static void hdmi_setup_audio_infoframe(struct
> > hda_codec *codec, int pin_idx,
> >   * Unsolicited events
> >   */
> >
> > -static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int
> > repoll);
> > -
> >  static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned
> > int res) {
> > -	struct hdmi_spec *spec = codec->spec;
> >  	int tag = res >> AC_UNSOL_RES_TAG_SHIFT;
> >  	int pin_nid;
> > -	int pin_idx;
> >  	struct hda_jack_tbl *jack;
> >
> >  	jack = snd_hda_jack_tbl_get_from_tag(codec, tag); @@ -970,12 +966,6
> > @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned
> > int
> > res)
> >  		"HDMI hot plug event: Codec=%d Pin=%d Presence_Detect=%d
> > ELD_Valid=%d\n",
> >  		codec->addr, pin_nid,
> >  		!!(res & AC_UNSOL_RES_PD), !!(res & AC_UNSOL_RES_ELDV));
> > -
> > -	pin_idx = pin_nid_to_pin_index(spec, pin_nid);
> > -	if (pin_idx < 0)
> > -		return;
> > -
> > -	hdmi_present_sense(get_pin(spec, pin_idx), 1);
> >  	snd_hda_jack_report_sync(codec);
> >  }
> >
> > @@ -1358,6 +1348,14 @@ static void hdmi_repoll_eld(struct work_struct
> > *work)
> >  	hdmi_present_sense(per_pin, per_pin->repoll_count);  }
> >
> > +static void hdmi_jack_detect_cb(struct hda_codec *codec, struct
> > +hda_jack_tbl *jack) {
> > +	struct hdmi_spec *spec = codec->spec;
> > +	int pin_idx = pin_nid_to_pin_index(spec, jack->nid);
> > +	if (pin_idx >= 0)
> > +		hdmi_present_sense(get_pin(spec, pin_idx), 1); }
> > +
> >  static void intel_haswell_fixup_connect_list(struct hda_codec *codec,
> >  					     hda_nid_t nid);
> >
> > @@ -1827,7 +1825,8 @@ static int generic_hdmi_init(struct hda_codec
> > *codec)
> >  		hda_nid_t pin_nid = per_pin->pin_nid;
> >
> >  		hdmi_init_pin(codec, pin_nid);
> > -		snd_hda_jack_detect_enable(codec, pin_nid, pin_nid);
> > +		snd_hda_jack_detect_enable_callback(codec, pin_nid, pin_nid,
> > +						    hdmi_jack_detect_cb);
> >  	}
> >  	return 0;
> >  }

[PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended

[Wang Xingchao]

when controller/codec in runtime suspended mode, monitor hotplug
would not trigger unsolicited event. This patch tries to power up
codec and hdmi driver would probe ELD info again.

Signed-off-by: Wang Xingchao <xingchao.wang@linux.intel.com>
---
 sound/pci/hda/hda_eld.c   | 6 ++++++
 sound/pci/hda/hda_local.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
index d0d7ac1..914712a 100644
--- a/sound/pci/hda/hda_eld.c
+++ b/sound/pci/hda/hda_eld.c
@@ -482,6 +482,7 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry,
 				struct snd_info_buffer *buffer)
 {
 	struct hdmi_eld *eld = entry->private_data;
+	struct hda_codec *codec = eld->codec;
 	struct parsed_hdmi_eld *e = &eld->info;
 	char buf[SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE];
 	int i;
@@ -500,11 +501,14 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry,
 		[4 ... 7] = "reserved"
 	};
 
+	snd_hda_power_up(codec);
+
 	mutex_lock(&eld->lock);
 	snd_iprintf(buffer, "monitor_present\t\t%d\n", eld->monitor_present);
 	snd_iprintf(buffer, "eld_valid\t\t%d\n", eld->eld_valid);
 	if (!eld->eld_valid) {
 		mutex_unlock(&eld->lock);
+		snd_hda_power_down(codec);
 		return;
 	}
 	snd_iprintf(buffer, "monitor_name\t\t%s\n", e->monitor_name);
@@ -529,6 +533,7 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry,
 	for (i = 0; i < e->sad_count; i++)
 		hdmi_print_sad_info(i, e->sad + i, buffer);
 	mutex_unlock(&eld->lock);
+	snd_hda_power_down(codec);
 }
 
 static void hdmi_write_eld_info(struct snd_info_entry *entry,
@@ -614,6 +619,7 @@ int snd_hda_eld_proc_new(struct hda_codec *codec, struct hdmi_eld *eld,
 	entry->c.text.write = hdmi_write_eld_info;
 	entry->mode |= S_IWUSR;
 	eld->proc_entry = entry;
+	eld->codec = codec;
 
 	return 0;
 }
diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
index e0bf753..1aaf8b6 100644
--- a/sound/pci/hda/hda_local.h
+++ b/sound/pci/hda/hda_local.h
@@ -747,6 +747,7 @@ struct hdmi_eld {
 #ifdef CONFIG_PROC_FS
 	struct snd_info_entry *proc_entry;
 #endif
+	struct hda_codec *codec;
 };
 
 int snd_hdmi_get_eld_size(struct hda_codec *codec, hda_nid_t nid);
-- 
1.8.1.2

Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended

[Takashi Iwai]

At Mon, 24 Jun 2013 07:45:24 -0400,
Wang Xingchao wrote:
> 
> when controller/codec in runtime suspended mode, monitor hotplug
> would not trigger unsolicited event. This patch tries to power up
> codec and hdmi driver would probe ELD info again.

I don't know whether this is the wanted behavior.
The proc file is supposed to read the driver's status, not to trigger
anything.


Takashi

> Signed-off-by: Wang Xingchao <xingchao.wang@linux.intel.com>
> ---
>  sound/pci/hda/hda_eld.c   | 6 ++++++
>  sound/pci/hda/hda_local.h | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
> index d0d7ac1..914712a 100644
> --- a/sound/pci/hda/hda_eld.c
> +++ b/sound/pci/hda/hda_eld.c
> @@ -482,6 +482,7 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry,
>  				struct snd_info_buffer *buffer)
>  {
>  	struct hdmi_eld *eld = entry->private_data;
> +	struct hda_codec *codec = eld->codec;
>  	struct parsed_hdmi_eld *e = &eld->info;
>  	char buf[SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE];
>  	int i;
> @@ -500,11 +501,14 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry,
>  		[4 ... 7] = "reserved"
>  	};
>  
> +	snd_hda_power_up(codec);
> +
>  	mutex_lock(&eld->lock);
>  	snd_iprintf(buffer, "monitor_present\t\t%d\n", eld->monitor_present);
>  	snd_iprintf(buffer, "eld_valid\t\t%d\n", eld->eld_valid);
>  	if (!eld->eld_valid) {
>  		mutex_unlock(&eld->lock);
> +		snd_hda_power_down(codec);
>  		return;
>  	}
>  	snd_iprintf(buffer, "monitor_name\t\t%s\n", e->monitor_name);
> @@ -529,6 +533,7 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry,
>  	for (i = 0; i < e->sad_count; i++)
>  		hdmi_print_sad_info(i, e->sad + i, buffer);
>  	mutex_unlock(&eld->lock);
> +	snd_hda_power_down(codec);
>  }
>  
>  static void hdmi_write_eld_info(struct snd_info_entry *entry,
> @@ -614,6 +619,7 @@ int snd_hda_eld_proc_new(struct hda_codec *codec, struct hdmi_eld *eld,
>  	entry->c.text.write = hdmi_write_eld_info;
>  	entry->mode |= S_IWUSR;
>  	eld->proc_entry = entry;
> +	eld->codec = codec;
>  
>  	return 0;
>  }
> diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
> index e0bf753..1aaf8b6 100644
> --- a/sound/pci/hda/hda_local.h
> +++ b/sound/pci/hda/hda_local.h
> @@ -747,6 +747,7 @@ struct hdmi_eld {
>  #ifdef CONFIG_PROC_FS
>  	struct snd_info_entry *proc_entry;
>  #endif
> +	struct hda_codec *codec;
>  };
>  
>  int snd_hdmi_get_eld_size(struct hda_codec *codec, hda_nid_t nid);
> -- 
> 1.8.1.2
> 

Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended

[Wang, Xingchao]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Monday, June 24, 2013 7:36 PM
> To: Wang Xingchao
> Cc: alsa-devel@alsa-project.org; Wang, Xingchao
> Subject: Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec
> suspended
> 
> At Mon, 24 Jun 2013 07:45:24 -0400,
> Wang Xingchao wrote:
> >
> > when controller/codec in runtime suspended mode, monitor hotplug would
> > not trigger unsolicited event. This patch tries to power up codec and
> > hdmi driver would probe ELD info again.
> 
> I don't know whether this is the wanted behavior.
> The proc file is supposed to read the driver's status, not to trigger anything.
> 

I explained in another mail what the patch fix. In the real case, when HDMI monitor removed,
Eld# still hold monitor information because the codec/controller is in suspend mode atm and has no chance to refresh eld.
Even when controller/codec waken up, it will not refresh eld# as gfx side handled hdmi hotplut when audio driver suspended.
Then user will always see eld valid info but in fact the monitor is removed already, obviously it's a bug.

Thanks
--xingchao

> 
> Takashi
> 
> > Signed-off-by: Wang Xingchao <xingchao.wang@linux.intel.com>
> > ---
> >  sound/pci/hda/hda_eld.c   | 6 ++++++
> >  sound/pci/hda/hda_local.h | 1 +
> >  2 files changed, 7 insertions(+)
> >
> > diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c index
> > d0d7ac1..914712a 100644
> > --- a/sound/pci/hda/hda_eld.c
> > +++ b/sound/pci/hda/hda_eld.c
> > @@ -482,6 +482,7 @@ static void hdmi_print_eld_info(struct
> snd_info_entry *entry,
> >  				struct snd_info_buffer *buffer)
> >  {
> >  	struct hdmi_eld *eld = entry->private_data;
> > +	struct hda_codec *codec = eld->codec;
> >  	struct parsed_hdmi_eld *e = &eld->info;
> >  	char buf[SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE];
> >  	int i;
> > @@ -500,11 +501,14 @@ static void hdmi_print_eld_info(struct
> snd_info_entry *entry,
> >  		[4 ... 7] = "reserved"
> >  	};
> >
> > +	snd_hda_power_up(codec);
> > +
> >  	mutex_lock(&eld->lock);
> >  	snd_iprintf(buffer, "monitor_present\t\t%d\n", eld->monitor_present);
> >  	snd_iprintf(buffer, "eld_valid\t\t%d\n", eld->eld_valid);
> >  	if (!eld->eld_valid) {
> >  		mutex_unlock(&eld->lock);
> > +		snd_hda_power_down(codec);
> >  		return;
> >  	}
> >  	snd_iprintf(buffer, "monitor_name\t\t%s\n", e->monitor_name); @@
> > -529,6 +533,7 @@ static void hdmi_print_eld_info(struct snd_info_entry
> *entry,
> >  	for (i = 0; i < e->sad_count; i++)
> >  		hdmi_print_sad_info(i, e->sad + i, buffer);
> >  	mutex_unlock(&eld->lock);
> > +	snd_hda_power_down(codec);
> >  }
> >
> >  static void hdmi_write_eld_info(struct snd_info_entry *entry, @@
> > -614,6 +619,7 @@ int snd_hda_eld_proc_new(struct hda_codec *codec,
> struct hdmi_eld *eld,
> >  	entry->c.text.write = hdmi_write_eld_info;
> >  	entry->mode |= S_IWUSR;
> >  	eld->proc_entry = entry;
> > +	eld->codec = codec;
> >
> >  	return 0;
> >  }
> > diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
> > index e0bf753..1aaf8b6 100644
> > --- a/sound/pci/hda/hda_local.h
> > +++ b/sound/pci/hda/hda_local.h
> > @@ -747,6 +747,7 @@ struct hdmi_eld {
> >  #ifdef CONFIG_PROC_FS
> >  	struct snd_info_entry *proc_entry;
> >  #endif
> > +	struct hda_codec *codec;
> >  };
> >
> >  int snd_hdmi_get_eld_size(struct hda_codec *codec, hda_nid_t nid);
> > --
> > 1.8.1.2
> >

Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended

[Takashi Iwai]

At Mon, 24 Jun 2013 11:56:21 +0000,
Wang, Xingchao wrote:
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Monday, June 24, 2013 7:36 PM
> > To: Wang Xingchao
> > Cc: alsa-devel@alsa-project.org; Wang, Xingchao
> > Subject: Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec
> > suspended
> > 
> > At Mon, 24 Jun 2013 07:45:24 -0400,
> > Wang Xingchao wrote:
> > >
> > > when controller/codec in runtime suspended mode, monitor hotplug would
> > > not trigger unsolicited event. This patch tries to power up codec and
> > > hdmi driver would probe ELD info again.
> > 
> > I don't know whether this is the wanted behavior.
> > The proc file is supposed to read the driver's status, not to trigger anything.
> > 
> 
> I explained in another mail what the patch fix. In the real case, when HDMI monitor removed,
> Eld# still hold monitor information because the codec/controller is in suspend mode atm and has no chance to refresh eld.

And this is no bug, per se.  The proc file shows the driver's current
status, not the hardware's raw status.

> Even when controller/codec waken up, it will not refresh eld# as gfx side hand
led hdmi hotplut when audio driver suspended.

... and this is actually the bug.
But, it's still unclear why the jack detection callback isn't called
at resume.


Takashi

> Then user will always see eld valid info but in fact the monitor is removed already, obviously it's a bug.
> 
> Thanks
> --xingchao
> 
> > 
> > Takashi
> > 
> > > Signed-off-by: Wang Xingchao <xingchao.wang@linux.intel.com>
> > > ---
> > >  sound/pci/hda/hda_eld.c   | 6 ++++++
> > >  sound/pci/hda/hda_local.h | 1 +
> > >  2 files changed, 7 insertions(+)
> > >
> > > diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c index
> > > d0d7ac1..914712a 100644
> > > --- a/sound/pci/hda/hda_eld.c
> > > +++ b/sound/pci/hda/hda_eld.c
> > > @@ -482,6 +482,7 @@ static void hdmi_print_eld_info(struct
> > snd_info_entry *entry,
> > >  				struct snd_info_buffer *buffer)
> > >  {
> > >  	struct hdmi_eld *eld = entry->private_data;
> > > +	struct hda_codec *codec = eld->codec;
> > >  	struct parsed_hdmi_eld *e = &eld->info;
> > >  	char buf[SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE];
> > >  	int i;
> > > @@ -500,11 +501,14 @@ static void hdmi_print_eld_info(struct
> > snd_info_entry *entry,
> > >  		[4 ... 7] = "reserved"
> > >  	};
> > >
> > > +	snd_hda_power_up(codec);
> > > +
> > >  	mutex_lock(&eld->lock);
> > >  	snd_iprintf(buffer, "monitor_present\t\t%d\n", eld->monitor_present);
> > >  	snd_iprintf(buffer, "eld_valid\t\t%d\n", eld->eld_valid);
> > >  	if (!eld->eld_valid) {
> > >  		mutex_unlock(&eld->lock);
> > > +		snd_hda_power_down(codec);
> > >  		return;
> > >  	}
> > >  	snd_iprintf(buffer, "monitor_name\t\t%s\n", e->monitor_name); @@
> > > -529,6 +533,7 @@ static void hdmi_print_eld_info(struct snd_info_entry
> > *entry,
> > >  	for (i = 0; i < e->sad_count; i++)
> > >  		hdmi_print_sad_info(i, e->sad + i, buffer);
> > >  	mutex_unlock(&eld->lock);
> > > +	snd_hda_power_down(codec);
> > >  }
> > >
> > >  static void hdmi_write_eld_info(struct snd_info_entry *entry, @@
> > > -614,6 +619,7 @@ int snd_hda_eld_proc_new(struct hda_codec *codec,
> > struct hdmi_eld *eld,
> > >  	entry->c.text.write = hdmi_write_eld_info;
> > >  	entry->mode |= S_IWUSR;
> > >  	eld->proc_entry = entry;
> > > +	eld->codec = codec;
> > >
> > >  	return 0;
> > >  }
> > > diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
> > > index e0bf753..1aaf8b6 100644
> > > --- a/sound/pci/hda/hda_local.h
> > > +++ b/sound/pci/hda/hda_local.h
> > > @@ -747,6 +747,7 @@ struct hdmi_eld {
> > >  #ifdef CONFIG_PROC_FS
> > >  	struct snd_info_entry *proc_entry;
> > >  #endif
> > > +	struct hda_codec *codec;
> > >  };
> > >
> > >  int snd_hdmi_get_eld_size(struct hda_codec *codec, hda_nid_t nid);
> > > --
> > > 1.8.1.2
> > >
> 

Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended

[David Henningsson]

On 06/24/2013 02:00 PM, Takashi Iwai wrote:
> At Mon, 24 Jun 2013 11:56:21 +0000,
> Wang, Xingchao wrote:
>>
>>> -----Original Message-----
>>> From: Takashi Iwai [mailto:tiwai@suse.de]
>>> Sent: Monday, June 24, 2013 7:36 PM
>>> To: Wang Xingchao
>>> Cc: alsa-devel@alsa-project.org; Wang, Xingchao
>>> Subject: Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec
>>> suspended
>>>
>>> At Mon, 24 Jun 2013 07:45:24 -0400,
>>> Wang Xingchao wrote:
>>>>
>>>> when controller/codec in runtime suspended mode, monitor hotplug would
>>>> not trigger unsolicited event.

And here's the original problem IMO: If you can't detect plug/unplug in 
runtime suspend mode, the runtime suspend mode is broken. Userspace 
relies on getting notification when a HDMI/DP connection is 
plugged/unplugged. If no unsol event is triggered, how is userspace 
notified?

This patch tries to power up codec and
>>>> hdmi driver would probe ELD info again.
>>>
>>> I don't know whether this is the wanted behavior.
>>> The proc file is supposed to read the driver's status, not to trigger anything.
>>>
>>
>> I explained in another mail what the patch fix. In the real case, when HDMI monitor removed,
>> Eld# still hold monitor information because the codec/controller is in suspend mode atm and has no chance to refresh eld.
>
> And this is no bug, per se.  The proc file shows the driver's current
> status, not the hardware's raw status.

Well, the codec proc file shows the hardware's raw status (which 
includes powering up the codec), perhaps it would be more consistent if 
the eld proc file did the same?

>> Even when controller/codec waken up, it will not refresh eld# as gfx side hand
> led hdmi hotplut when audio driver suspended.
>
> ... and this is actually the bug.
> But, it's still unclear why the jack detection callback isn't called
> at resume.

Well, I haven't double checked, but is there anyone actually calling it? 
Just setting a jack to dirty does not cause any action in itself.

And btw, even if the proc file might show old information, we also have 
the ELD bytes ctrl, which IMO should always correctly return 0 if 
there's nothing connected.

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended

[Takashi Iwai]

At Mon, 24 Jun 2013 14:47:01 +0200,
David Henningsson wrote:
> 
> On 06/24/2013 02:00 PM, Takashi Iwai wrote:
> > At Mon, 24 Jun 2013 11:56:21 +0000,
> > Wang, Xingchao wrote:
> >>
> >>> -----Original Message-----
> >>> From: Takashi Iwai [mailto:tiwai@suse.de]
> >>> Sent: Monday, June 24, 2013 7:36 PM
> >>> To: Wang Xingchao
> >>> Cc: alsa-devel@alsa-project.org; Wang, Xingchao
> >>> Subject: Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec
> >>> suspended
> >>>
> >>> At Mon, 24 Jun 2013 07:45:24 -0400,
> >>> Wang Xingchao wrote:
> >>>>
> >>>> when controller/codec in runtime suspended mode, monitor hotplug would
> >>>> not trigger unsolicited event.
> 
> And here's the original problem IMO: If you can't detect plug/unplug in 
> runtime suspend mode, the runtime suspend mode is broken. Userspace 
> relies on getting notification when a HDMI/DP connection is 
> plugged/unplugged. If no unsol event is triggered, how is userspace 
> notified?

It can't, so far.  The same thing happens for any other jack
detections when the power-saving is turned on.

There is a low power mode that allows the jack detection, but this is
different from the aggressive power-saving with runtime D3.

In theory, the audio driver can be resumed forcibly before unsolicited
event is sent, once if we have a layer controlling the power of both
graphics and audio devices.  This is the thing to be done in future,
together with the runtime PM control of the graphics side.


> This patch tries to power up codec and
> >>>> hdmi driver would probe ELD info again.
> >>>
> >>> I don't know whether this is the wanted behavior.
> >>> The proc file is supposed to read the driver's status, not to trigger anything.
> >>>
> >>
> >> I explained in another mail what the patch fix. In the real case, when HDMI monitor removed,
> >> Eld# still hold monitor information because the codec/controller is in suspend mode atm and has no chance to refresh eld.
> >
> > And this is no bug, per se.  The proc file shows the driver's current
> > status, not the hardware's raw status.
> 
> Well, the codec proc file shows the hardware's raw status (which 
> includes powering up the codec), perhaps it would be more consistent if 
> the eld proc file did the same?

Well, the concept is different.

The codec proc file shows the codec raw status, so it is.
The eld proc file shows the content of the data the driver currently
contains.  It's good so for debugging purpose; otherwise the driver
status would change just by reading this proc file.

> >> Even when controller/codec waken up, it will not refresh eld# as gfx side hand
> > led hdmi hotplut when audio driver suspended.
> >
> > ... and this is actually the bug.
> > But, it's still unclear why the jack detection callback isn't called
> > at resume.
> 
> Well, I haven't double checked, but is there anyone actually calling it? 
> Just setting a jack to dirty does not cause any action in itself.

The culprit was found, see my previous reply.

> And btw, even if the proc file might show old information, we also have 
> the ELD bytes ctrl, which IMO should always correctly return 0 if 
> there's nothing connected.

Right.


thanks,

Takashi

Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended

[David Henningsson]

On 06/24/2013 03:00 PM, Takashi Iwai wrote:
> At Mon, 24 Jun 2013 14:47:01 +0200,
> David Henningsson wrote:
>>
>> On 06/24/2013 02:00 PM, Takashi Iwai wrote:
>>> At Mon, 24 Jun 2013 11:56:21 +0000,
>>> Wang, Xingchao wrote:
>>>>
>>>>> -----Original Message-----
>>>>> From: Takashi Iwai [mailto:tiwai@suse.de]
>>>>> Sent: Monday, June 24, 2013 7:36 PM
>>>>> To: Wang Xingchao
>>>>> Cc: alsa-devel@alsa-project.org; Wang, Xingchao
>>>>> Subject: Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec
>>>>> suspended
>>>>>
>>>>> At Mon, 24 Jun 2013 07:45:24 -0400,
>>>>> Wang Xingchao wrote:
>>>>>>
>>>>>> when controller/codec in runtime suspended mode, monitor hotplug would
>>>>>> not trigger unsolicited event.
>>
>> And here's the original problem IMO: If you can't detect plug/unplug in
>> runtime suspend mode, the runtime suspend mode is broken. Userspace
>> relies on getting notification when a HDMI/DP connection is
>> plugged/unplugged. If no unsol event is triggered, how is userspace
>> notified?
>
> It can't, so far.  The same thing happens for any other jack
> detections when the power-saving is turned on.
>
> There is a low power mode that allows the jack detection, but this is
> different from the aggressive power-saving with runtime D3.

If "aggressive power-saving with runtime D3" is the same as 
AZX_DCAPS_PM_RUNTIME, this is also enabled for analog codecs connected 
to a Lynx point controller.

It looks like userspace have problems getting notifications for e g 
headphone insertion on Lynx point controllers, so this is not only an 
HDMI/DP problem?

Trying to read up a little on this, there seem to be an option to set 
the WAKEEN register to have jack detection working even when the 
controller is in D3. (refer HDA specification 4.5.9.2:
Codec Wake From System S0, Controller D3.)
But it seems we do not (yet) use this feature. Is this something that 
could/should be implemented to fix the jack detection problems that 
seems to be happening otherwise?


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended

[Takashi Iwai]

At Tue, 25 Jun 2013 09:45:04 +0200,
David Henningsson wrote:
> 
> On 06/24/2013 03:00 PM, Takashi Iwai wrote:
> > At Mon, 24 Jun 2013 14:47:01 +0200,
> > David Henningsson wrote:
> >>
> >> On 06/24/2013 02:00 PM, Takashi Iwai wrote:
> >>> At Mon, 24 Jun 2013 11:56:21 +0000,
> >>> Wang, Xingchao wrote:
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Takashi Iwai [mailto:tiwai@suse.de]
> >>>>> Sent: Monday, June 24, 2013 7:36 PM
> >>>>> To: Wang Xingchao
> >>>>> Cc: alsa-devel@alsa-project.org; Wang, Xingchao
> >>>>> Subject: Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec
> >>>>> suspended
> >>>>>
> >>>>> At Mon, 24 Jun 2013 07:45:24 -0400,
> >>>>> Wang Xingchao wrote:
> >>>>>>
> >>>>>> when controller/codec in runtime suspended mode, monitor hotplug would
> >>>>>> not trigger unsolicited event.
> >>
> >> And here's the original problem IMO: If you can't detect plug/unplug in
> >> runtime suspend mode, the runtime suspend mode is broken. Userspace
> >> relies on getting notification when a HDMI/DP connection is
> >> plugged/unplugged. If no unsol event is triggered, how is userspace
> >> notified?
> >
> > It can't, so far.  The same thing happens for any other jack
> > detections when the power-saving is turned on.
> >
> > There is a low power mode that allows the jack detection, but this is
> > different from the aggressive power-saving with runtime D3.
> 
> If "aggressive power-saving with runtime D3" is the same as 
> AZX_DCAPS_PM_RUNTIME, this is also enabled for analog codecs connected 
> to a Lynx point controller.
> 
> It looks like userspace have problems getting notifications for e g 
> headphone insertion on Lynx point controllers, so this is not only an 
> HDMI/DP problem?

Yes.

> Trying to read up a little on this, there seem to be an option to set 
> the WAKEEN register to have jack detection working even when the 
> controller is in D3. (refer HDA specification 4.5.9.2:
> Codec Wake From System S0, Controller D3.)
> But it seems we do not (yet) use this feature. Is this something that 
> could/should be implemented to fix the jack detection problems that 
> seems to be happening otherwise?

It sounds feasible, at least for traditional jack detection of analog
pins.  But I'm not sure whether this would help for the Intel graphics
case.  Just need testing.


Takashi

Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended

[David Henningsson]

On 06/25/2013 09:55 AM, Takashi Iwai wrote:
> At Tue, 25 Jun 2013 09:45:04 +0200,
> David Henningsson wrote:
>>> There is a low power mode that allows the jack detection, but this is
>>> different from the aggressive power-saving with runtime D3.
>>
>> If "aggressive power-saving with runtime D3" is the same as
>> AZX_DCAPS_PM_RUNTIME, this is also enabled for analog codecs connected
>> to a Lynx point controller.
>>
>> It looks like userspace have problems getting notifications for e g
>> headphone insertion on Lynx point controllers, so this is not only an
>> HDMI/DP problem?
>
> Yes.
>
>> Trying to read up a little on this, there seem to be an option to set
>> the WAKEEN register to have jack detection working even when the
>> controller is in D3. (refer HDA specification 4.5.9.2:
>> Codec Wake From System S0, Controller D3.)
>> But it seems we do not (yet) use this feature. Is this something that
>> could/should be implemented to fix the jack detection problems that
>> seems to be happening otherwise?
>
> It sounds feasible, at least for traditional jack detection of analog
> pins.  But I'm not sure whether this would help for the Intel graphics
> case.  Just need testing.

Xingchao, what are your thoughts about using WAKEEN to wakeup both Lynx 
point and Haswell HDMI?


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended

[Wang, Xingchao]

Hi David,


> -----Original Message-----
> From: alsa-devel-bounces@alsa-project.org
> [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of David Henningsson
> Sent: Tuesday, June 25, 2013 5:02 PM
> To: Wang, Xingchao
> Cc: Takashi Iwai; alsa-devel@alsa-project.org
> Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info when
> codec suspended
> 
> On 06/25/2013 09:55 AM, Takashi Iwai wrote:
> > At Tue, 25 Jun 2013 09:45:04 +0200,
> > David Henningsson wrote:
> >>> There is a low power mode that allows the jack detection, but this
> >>> is different from the aggressive power-saving with runtime D3.
> >>
> >> If "aggressive power-saving with runtime D3" is the same as
> >> AZX_DCAPS_PM_RUNTIME, this is also enabled for analog codecs
> >> connected to a Lynx point controller.
> >>
> >> It looks like userspace have problems getting notifications for e g
> >> headphone insertion on Lynx point controllers, so this is not only an
> >> HDMI/DP problem?
> >
> > Yes.
> >
> >> Trying to read up a little on this, there seem to be an option to set
> >> the WAKEEN register to have jack detection working even when the
> >> controller is in D3. (refer HDA specification 4.5.9.2:
> >> Codec Wake From System S0, Controller D3.) But it seems we do not
> >> (yet) use this feature. Is this something that could/should be
> >> implemented to fix the jack detection problems that seems to be
> >> happening otherwise?
> >
> > It sounds feasible, at least for traditional jack detection of analog
> > pins.  But I'm not sure whether this would help for the Intel graphics
> > case.  Just need testing.
> 
> Xingchao, what are your thoughts about using WAKEEN to wakeup both Lynx
> point and Haswell HDMI?

That's okay for me, I would do some test on that. do you have some test cases?
That would help me verify them when enable WAKEEN feature.

Thanks
--xingchao
> 
> 
> --
> David Henningsson, Canonical Ltd.
> https://launchpad.net/~diwic
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended

[David Henningsson]

On 06/25/2013 11:33 AM, Wang, Xingchao wrote:
> Hi David,
>
>
>> -----Original Message-----
>> From: alsa-devel-bounces@alsa-project.org
>> [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of David Henningsson
>> Sent: Tuesday, June 25, 2013 5:02 PM
>> To: Wang, Xingchao
>> Cc: Takashi Iwai; alsa-devel@alsa-project.org
>> Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info when
>> codec suspended
>>
>> On 06/25/2013 09:55 AM, Takashi Iwai wrote:
>>> At Tue, 25 Jun 2013 09:45:04 +0200,
>>> David Henningsson wrote:
>>>>> There is a low power mode that allows the jack detection, but this
>>>>> is different from the aggressive power-saving with runtime D3.
>>>>
>>>> If "aggressive power-saving with runtime D3" is the same as
>>>> AZX_DCAPS_PM_RUNTIME, this is also enabled for analog codecs
>>>> connected to a Lynx point controller.
>>>>
>>>> It looks like userspace have problems getting notifications for e g
>>>> headphone insertion on Lynx point controllers, so this is not only an
>>>> HDMI/DP problem?
>>>
>>> Yes.
>>>
>>>> Trying to read up a little on this, there seem to be an option to set
>>>> the WAKEEN register to have jack detection working even when the
>>>> controller is in D3. (refer HDA specification 4.5.9.2:
>>>> Codec Wake From System S0, Controller D3.) But it seems we do not
>>>> (yet) use this feature. Is this something that could/should be
>>>> implemented to fix the jack detection problems that seems to be
>>>> happening otherwise?
>>>
>>> It sounds feasible, at least for traditional jack detection of analog
>>> pins.  But I'm not sure whether this would help for the Intel graphics
>>> case.  Just need testing.
>>
>> Xingchao, what are your thoughts about using WAKEEN to wakeup both Lynx
>> point and Haswell HDMI?
>
> That's okay for me, I would do some test on that. do you have some test cases?
> That would help me verify them when enable WAKEEN feature.

Since we still enable the legacy jack feature through /dev/input/event*, 
the easiest way to test would probably to run evtest (which is in the 
Ubuntu repositories). Find the correct /dev/input/event file by checking 
dmesg (or just trying them one by one), then run "sudo evtest 
/dev/input/<filename>".

Now check if you get events correctly on HDMI/DP/Headphone/Mic/etc plug 
and unplug, even if the controller is runtime suspended.

Also note that WAKEEN should probably be disabled during system S3, 
because we don't want to wake up the entire computer just because 
somebody unplugs his headphone, right?

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended

[Wang Xingchao]

[-- Attachment #1: Type: text/plain, Size: 4230 bytes --]

Hi David/Takashi,

Here's some update on this topic.
I used evtest to monitor hotplug input event for Headphone, it doesnot
report the hotplug event when audio controller/codec in runtime
suspend mode.
if it's during audio playback(both controller and codec are active),
the events could be monitored correctly.
However even the controller/codec in runtime suspend mode, the hotplug
event was not missed when waken up from suspend mode. After exit from
suspend mode, the hotplug event would be reported asap.
So userspace will not receive the event notification from driver when
controller/codec in suspend mode, but it will get them once audio
controller/codec become active.
I think it's acceptable for audio playback functinality, it will not
harm audio routing in fact. i'm not sure there's other potential risk,
i.e. user space will show/hide the devices in UI according to the
event, in that case , user will never see the Headphone device for
playback before audio controller/codec was waken up in another way.

Meanwhile i tested attached patch to monitor WAKEEN events, it doesnot
work well as Spec said. it would not wake up audio controller/codec
when plug in/out headphone in runtime suspend mode, and the status
register always be 0.

thanks
--xingchao


2013/6/25 David Henningsson <david.henningsson@canonical.com>:
> On 06/25/2013 11:33 AM, Wang, Xingchao wrote:
>>
>> Hi David,
>>
>>
>>> -----Original Message-----
>>> From: alsa-devel-bounces@alsa-project.org
>>> [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of David
>>> Henningsson
>>> Sent: Tuesday, June 25, 2013 5:02 PM
>>> To: Wang, Xingchao
>>> Cc: Takashi Iwai; alsa-devel@alsa-project.org
>>> Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info
>>> when
>>> codec suspended
>>>
>>> On 06/25/2013 09:55 AM, Takashi Iwai wrote:
>>>>
>>>> At Tue, 25 Jun 2013 09:45:04 +0200,
>>>> David Henningsson wrote:
>>>>>>
>>>>>> There is a low power mode that allows the jack detection, but this
>>>>>> is different from the aggressive power-saving with runtime D3.
>>>>>
>>>>>
>>>>> If "aggressive power-saving with runtime D3" is the same as
>>>>> AZX_DCAPS_PM_RUNTIME, this is also enabled for analog codecs
>>>>> connected to a Lynx point controller.
>>>>>
>>>>> It looks like userspace have problems getting notifications for e g
>>>>> headphone insertion on Lynx point controllers, so this is not only an
>>>>> HDMI/DP problem?
>>>>
>>>>
>>>> Yes.
>>>>
>>>>> Trying to read up a little on this, there seem to be an option to set
>>>>> the WAKEEN register to have jack detection working even when the
>>>>> controller is in D3. (refer HDA specification 4.5.9.2:
>>>>> Codec Wake From System S0, Controller D3.) But it seems we do not
>>>>> (yet) use this feature. Is this something that could/should be
>>>>> implemented to fix the jack detection problems that seems to be
>>>>> happening otherwise?
>>>>
>>>>
>>>> It sounds feasible, at least for traditional jack detection of analog
>>>> pins.  But I'm not sure whether this would help for the Intel graphics
>>>> case.  Just need testing.
>>>
>>>
>>> Xingchao, what are your thoughts about using WAKEEN to wakeup both Lynx
>>> point and Haswell HDMI?
>>
>>
>> That's okay for me, I would do some test on that. do you have some test
>> cases?
>> That would help me verify them when enable WAKEEN feature.
>
>
> Since we still enable the legacy jack feature through /dev/input/event*, the
> easiest way to test would probably to run evtest (which is in the Ubuntu
> repositories). Find the correct /dev/input/event file by checking dmesg (or
> just trying them one by one), then run "sudo evtest /dev/input/<filename>".
>
> Now check if you get events correctly on HDMI/DP/Headphone/Mic/etc plug and
> unplug, even if the controller is runtime suspended.
>
> Also note that WAKEEN should probably be disabled during system S3, because
> we don't want to wake up the entire computer just because somebody unplugs
> his headphone, right?
>
>
> --
> David Henningsson, Canonical Ltd.
> https://launchpad.net/~diwic
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[-- Attachment #2: 0001-ALSA-hda-Enable-WAKEEN-for-codecs-blindly-for-test.patch --]
[-- Type: application/octet-stream, Size: 1588 bytes --]

From c70566e01ba2ed8a0e7089b7e981d7800d42ea99 Mon Sep 17 00:00:00 2001
From: Wang Xingchao <xingchao.wang@linux.intel.com>
Date: Thu, 11 Jul 2013 18:54:20 -0400
Subject: [PATCH] ALSA: hda - Enable WAKEEN for codecs blindly for test

As HDA Spec(4.5.9.2) said, WAKEEN bits enable waking up capability
when controller in D3 or system in S3. This patch enabled all WAKEEN
bits blindly for test.

Signed-off-by: Wang Xingchao <xingchao.wang@linux.intel.com>
---
 sound/pci/hda/hda_intel.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index cc548e00..f9a2518 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1290,6 +1290,9 @@ static void azx_init_chip(struct azx *chip, int full_reset)
 	azx_writel(chip, DPLBASE, (u32)chip->posbuf.addr);
 	azx_writel(chip, DPUBASE, upper_32_bits(chip->posbuf.addr));
 
+	/* enable controller Wake Up event*/
+	azx_writeb(chip, WAKEEN, azx_readb(chip, WAKEEN) |
+		  0xFF);
 	chip->initialized = 1;
 }
 
@@ -1374,7 +1377,7 @@ static irqreturn_t azx_interrupt(int irq, void *dev_id)
 {
 	struct azx *chip = dev_id;
 	struct azx_dev *azx_dev;
-	u32 status;
+	u32 status, event;
 	u8 sd_status;
 	int i, ok;
 
@@ -1383,6 +1386,12 @@ static irqreturn_t azx_interrupt(int irq, void *dev_id)
 		return IRQ_NONE;
 #endif
 
+	status = azx_readb(chip, STATESTS);
+	if (status) {
+		event = azx_readb(chip, WAKEEN);
+		snd_printdd("wake up event %x status %x\n", event, status);
+	}
+
 	spin_lock(&chip->reg_lock);
 
 	if (chip->disabled) {
-- 
1.8.3.2


[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended

[David Henningsson]

On 07/12/2013 08:13 AM, Wang Xingchao wrote:
> Hi David/Takashi,
>
> Here's some update on this topic.
> I used evtest to monitor hotplug input event for Headphone, it doesnot
> report the hotplug event when audio controller/codec in runtime
> suspend mode.
> if it's during audio playback(both controller and codec are active),
> the events could be monitored correctly.
> However even the controller/codec in runtime suspend mode, the hotplug
> event was not missed when waken up from suspend mode. After exit from
> suspend mode, the hotplug event would be reported asap.
> So userspace will not receive the event notification from driver when
> controller/codec in suspend mode, but it will get them once audio
> controller/codec become active.
> I think it's acceptable for audio playback functinality, it will not
> harm audio routing in fact. i'm not sure there's other potential risk,
> i.e. user space will show/hide the devices in UI according to the
> event, in that case , user will never see the Headphone device for
> playback before audio controller/codec was waken up in another way.
>
> Meanwhile i tested attached patch to monitor WAKEEN events, it doesnot
> work well as Spec said. it would not wake up audio controller/codec
> when plug in/out headphone in runtime suspend mode, and the status
> register always be 0.

Hi Xingchao,

Maybe you're giving up too easily on the WAKEEN stuff? I can spot a few 
places where your patch is incomplete:

  - First, WAKEEN and STATETS are word registers, you should probably 
use azx_writew when you access them

  - Second, and this is definitely a problem, azx_runtime_suspend calls 
azx_stop_chip -> azx_int_disable. So we disable interrupts, that's why 
we don't get them from the wake events.

  - Third, first in azx_interrupt, we check for 
chip->pci->dev.power.runtime_status - maybe this is no longer true if 
the device is in D3?

Do you know how and if Windows has solved this?

>
> thanks
> --xingchao
>
>
> 2013/6/25 David Henningsson <david.henningsson@canonical.com>:
>> On 06/25/2013 11:33 AM, Wang, Xingchao wrote:
>>>
>>> Hi David,
>>>
>>>
>>>> -----Original Message-----
>>>> From: alsa-devel-bounces@alsa-project.org
>>>> [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of David
>>>> Henningsson
>>>> Sent: Tuesday, June 25, 2013 5:02 PM
>>>> To: Wang, Xingchao
>>>> Cc: Takashi Iwai; alsa-devel@alsa-project.org
>>>> Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info
>>>> when
>>>> codec suspended
>>>>
>>>> On 06/25/2013 09:55 AM, Takashi Iwai wrote:
>>>>>
>>>>> At Tue, 25 Jun 2013 09:45:04 +0200,
>>>>> David Henningsson wrote:
>>>>>>>
>>>>>>> There is a low power mode that allows the jack detection, but this
>>>>>>> is different from the aggressive power-saving with runtime D3.
>>>>>>
>>>>>>
>>>>>> If "aggressive power-saving with runtime D3" is the same as
>>>>>> AZX_DCAPS_PM_RUNTIME, this is also enabled for analog codecs
>>>>>> connected to a Lynx point controller.
>>>>>>
>>>>>> It looks like userspace have problems getting notifications for e g
>>>>>> headphone insertion on Lynx point controllers, so this is not only an
>>>>>> HDMI/DP problem?
>>>>>
>>>>>
>>>>> Yes.
>>>>>
>>>>>> Trying to read up a little on this, there seem to be an option to set
>>>>>> the WAKEEN register to have jack detection working even when the
>>>>>> controller is in D3. (refer HDA specification 4.5.9.2:
>>>>>> Codec Wake From System S0, Controller D3.) But it seems we do not
>>>>>> (yet) use this feature. Is this something that could/should be
>>>>>> implemented to fix the jack detection problems that seems to be
>>>>>> happening otherwise?
>>>>>
>>>>>
>>>>> It sounds feasible, at least for traditional jack detection of analog
>>>>> pins.  But I'm not sure whether this would help for the Intel graphics
>>>>> case.  Just need testing.
>>>>
>>>>
>>>> Xingchao, what are your thoughts about using WAKEEN to wakeup both Lynx
>>>> point and Haswell HDMI?
>>>
>>>
>>> That's okay for me, I would do some test on that. do you have some test
>>> cases?
>>> That would help me verify them when enable WAKEEN feature.
>>
>>
>> Since we still enable the legacy jack feature through /dev/input/event*, the
>> easiest way to test would probably to run evtest (which is in the Ubuntu
>> repositories). Find the correct /dev/input/event file by checking dmesg (or
>> just trying them one by one), then run "sudo evtest /dev/input/<filename>".
>>
>> Now check if you get events correctly on HDMI/DP/Headphone/Mic/etc plug and
>> unplug, even if the controller is runtime suspended.
>>
>> Also note that WAKEEN should probably be disabled during system S3, because
>> we don't want to wake up the entire computer just because somebody unplugs
>> his headphone, right?
>>
>>
>> --
>> David Henningsson, Canonical Ltd.
>> https://launchpad.net/~diwic
>> _______________________________________________
>> Alsa-devel mailing list
>> Alsa-devel@alsa-project.org
>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended

[Wang, Xingchao]

> -----Original Message-----
> From: David Henningsson [mailto:david.henningsson@canonical.com]
> Sent: Monday, July 15, 2013 12:29 PM
> To: Wang Xingchao
> Cc: Takashi Iwai; Wang, Xingchao; alsa-devel@alsa-project.org
> Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info when
> codec suspended
> 
> On 07/12/2013 08:13 AM, Wang Xingchao wrote:
> > Hi David/Takashi,
> >
> > Here's some update on this topic.
> > I used evtest to monitor hotplug input event for Headphone, it doesnot
> > report the hotplug event when audio controller/codec in runtime
> > suspend mode.
> > if it's during audio playback(both controller and codec are active),
> > the events could be monitored correctly.
> > However even the controller/codec in runtime suspend mode, the hotplug
> > event was not missed when waken up from suspend mode. After exit from
> > suspend mode, the hotplug event would be reported asap.
> > So userspace will not receive the event notification from driver when
> > controller/codec in suspend mode, but it will get them once audio
> > controller/codec become active.
> > I think it's acceptable for audio playback functinality, it will not
> > harm audio routing in fact. i'm not sure there's other potential risk,
> > i.e. user space will show/hide the devices in UI according to the
> > event, in that case , user will never see the Headphone device for
> > playback before audio controller/codec was waken up in another way.
> >
> > Meanwhile i tested attached patch to monitor WAKEEN events, it doesnot
> > work well as Spec said. it would not wake up audio controller/codec
> > when plug in/out headphone in runtime suspend mode, and the status
> > register always be 0.
> 
> Hi Xingchao,
> 
> Maybe you're giving up too easily on the WAKEEN stuff? I can spot a few places
> where your patch is incomplete:
> 
>   - First, WAKEEN and STATETS are word registers, you should probably use
> azx_writew when you access them

Thanks for clarify this. We support 8 codec at most, so I think it's safe to use writeb/readb here.
 
> 
>   - Second, and this is definitely a problem, azx_runtime_suspend calls
> azx_stop_chip -> azx_int_disable. So we disable interrupts, that's why we don't
> get them from the wake events.
[Wang, Xingchao]  yes, moreover I think it's not enough only enabling "controller interrupt" bits in this case, 
PCI interrupt bits must be anbled too. I'm still checking the setting in runtime_suspend.

> 
>   - Third, first in azx_interrupt, we check for
> chip->pci->dev.power.runtime_status - maybe this is no longer true if
> the device is in D3?
[Wang, Xingchao] yes, it's a risk here. Just move the lines before the "if" condition check.
> 
> Do you know how and if Windows has solved this?
> 
[Wang, Xingchao] I have wrote emails for windows team guys, has no response yet. I will update here when I have update.

thanks
--xingchao

Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended

[Wang, Xingchao]

> -----Original Message-----
> From: alsa-devel-bounces@alsa-project.org
> [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Wang, Xingchao
> Sent: Monday, July 15, 2013 4:54 PM
> To: David Henningsson; Wang Xingchao
> Cc: Takashi Iwai; alsa-devel@alsa-project.org
> Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info when
> codec suspended
> 
> 
> 
> > -----Original Message-----
> > From: David Henningsson [mailto:david.henningsson@canonical.com]
> > Sent: Monday, July 15, 2013 12:29 PM
> > To: Wang Xingchao
> > Cc: Takashi Iwai; Wang, Xingchao; alsa-devel@alsa-project.org
> > Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD
> > info when codec suspended
> >
> > On 07/12/2013 08:13 AM, Wang Xingchao wrote:
> > > Hi David/Takashi,
> > >
> > > Here's some update on this topic.
> > > I used evtest to monitor hotplug input event for Headphone, it
> > > doesnot report the hotplug event when audio controller/codec in
> > > runtime suspend mode.
> > > if it's during audio playback(both controller and codec are active),
> > > the events could be monitored correctly.
> > > However even the controller/codec in runtime suspend mode, the
> > > hotplug event was not missed when waken up from suspend mode. After
> > > exit from suspend mode, the hotplug event would be reported asap.
> > > So userspace will not receive the event notification from driver
> > > when controller/codec in suspend mode, but it will get them once
> > > audio controller/codec become active.
> > > I think it's acceptable for audio playback functinality, it will not
> > > harm audio routing in fact. i'm not sure there's other potential
> > > risk, i.e. user space will show/hide the devices in UI according to
> > > the event, in that case , user will never see the Headphone device
> > > for playback before audio controller/codec was waken up in another way.
> > >
> > > Meanwhile i tested attached patch to monitor WAKEEN events, it
> > > doesnot work well as Spec said. it would not wake up audio
> > > controller/codec when plug in/out headphone in runtime suspend mode,
> > > and the status register always be 0.
> >
> > Hi Xingchao,
> >
> > Maybe you're giving up too easily on the WAKEEN stuff? I can spot a
> > few places where your patch is incomplete:
> >
> >   - First, WAKEEN and STATETS are word registers, you should probably
> > use azx_writew when you access them

If the controller is active while codec suspended, "evtest" can catch headphone hotplug event.
I made some changes in azx_runtime_suspend(), which in brief leave interrupt enabled, then "evtest" cannot catch input event.
Also I added some debug log shows, when controller enter suspend mode, the WAKEEN bits were set(value 0x3), after headphone hotplug,
Audio controller was not waken up. When audio controller exit, in azx_runtime_resume(), the debug shows WAKEEN bits value no change, STATETS
has value 0. The controller did not detect changes from codec side, so it donot set the bits in STATETS.

I read the spec, and found something different:
- Controller should be in reset state while In D3?(Chapter 4.5.9.2 in  HD-A spec.)
Azx_runtime-suspend() did not reset the controller.
- codec will use power state change request on the link to let controller know.
Not sure it's hardware operation or software layer.
- codec discovery ( chapter 4.3)
"When the link is enabled by the assertion of CRST, the codecs will detect the de-assertion of the RESET# signal and request a status change and enumeration by the controller. 
 As the controller hardware detects these requests, it will provide the codecs with their unique addresses and set the controller STATESTS bits
 to indicate that a Status Change event was detected on the appropriate SDATA_INx signals.  "
I'm afraid something was not configured correctly, i.e. should let controller in reset state when enter runtime suspend mode?

Thanks
--xingchao
> 
> Thanks for clarify this. We support 8 codec at most, so I think it's safe to use
> writeb/readb here.
> 
> >
> >   - Second, and this is definitely a problem, azx_runtime_suspend
> > calls azx_stop_chip -> azx_int_disable. So we disable interrupts,
> > that's why we don't get them from the wake events.
> [Wang, Xingchao]  yes, moreover I think it's not enough only enabling
> "controller interrupt" bits in this case, PCI interrupt bits must be anbled too.
> I'm still checking the setting in runtime_suspend.
> 
> >
> >   - Third, first in azx_interrupt, we check for
> > chip->pci->dev.power.runtime_status - maybe this is no longer true if
> > the device is in D3?
> [Wang, Xingchao] yes, it's a risk here. Just move the lines before the "if"
> condition check.
> >
> > Do you know how and if Windows has solved this?
> >
> [Wang, Xingchao] I have wrote emails for windows team guys, has no response
> yet. I will update here when I have update.
> 
> thanks
> --xingchao
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended

[Wang, Xingchao]

> -----Original Message-----
> From: Wang, Xingchao
> Sent: Wednesday, July 17, 2013 6:15 PM
> To: Wang, Xingchao; David Henningsson
> Cc: Takashi Iwai; alsa-devel@alsa-project.org
> Subject: RE: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info when
> codec suspended
> 
> 
> 
> > -----Original Message-----
> > From: alsa-devel-bounces@alsa-project.org
> > [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Wang,
> > Xingchao
> > Sent: Monday, July 15, 2013 4:54 PM
> > To: David Henningsson; Wang Xingchao
> > Cc: Takashi Iwai; alsa-devel@alsa-project.org
> > Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD
> > info when codec suspended
> >
> >
> >
> > > -----Original Message-----
> > > From: David Henningsson [mailto:david.henningsson@canonical.com]
> > > Sent: Monday, July 15, 2013 12:29 PM
> > > To: Wang Xingchao
> > > Cc: Takashi Iwai; Wang, Xingchao; alsa-devel@alsa-project.org
> > > Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD
> > > info when codec suspended
> > >
> > > On 07/12/2013 08:13 AM, Wang Xingchao wrote:
> > > > Hi David/Takashi,
> > > >
> > > > Here's some update on this topic.
> > > > I used evtest to monitor hotplug input event for Headphone, it
> > > > doesnot report the hotplug event when audio controller/codec in
> > > > runtime suspend mode.
> > > > if it's during audio playback(both controller and codec are
> > > > active), the events could be monitored correctly.
> > > > However even the controller/codec in runtime suspend mode, the
> > > > hotplug event was not missed when waken up from suspend mode.
> > > > After exit from suspend mode, the hotplug event would be reported asap.
> > > > So userspace will not receive the event notification from driver
> > > > when controller/codec in suspend mode, but it will get them once
> > > > audio controller/codec become active.
> > > > I think it's acceptable for audio playback functinality, it will
> > > > not harm audio routing in fact. i'm not sure there's other
> > > > potential risk, i.e. user space will show/hide the devices in UI
> > > > according to the event, in that case , user will never see the
> > > > Headphone device for playback before audio controller/codec was waken
> up in another way.
> > > >
> > > > Meanwhile i tested attached patch to monitor WAKEEN events, it
> > > > doesnot work well as Spec said. it would not wake up audio
> > > > controller/codec when plug in/out headphone in runtime suspend
> > > > mode, and the status register always be 0.
> > >
> > > Hi Xingchao,
> > >
> > > Maybe you're giving up too easily on the WAKEEN stuff? I can spot a
> > > few places where your patch is incomplete:
> > >
> > >   - First, WAKEEN and STATETS are word registers, you should
> > > probably use azx_writew when you access them
> 
> If the controller is active while codec suspended, "evtest" can catch headphone
> hotplug event.
> I made some changes in azx_runtime_suspend(), which in brief leave interrupt
> enabled, then "evtest" cannot catch input event.
> Also I added some debug log shows, when controller enter suspend mode, the
> WAKEEN bits were set(value 0x3), after headphone hotplug, Audio controller
> was not waken up. When audio controller exit, in azx_runtime_resume(), the
> debug shows WAKEEN bits value no change, STATETS has value 0. The controller
> did not detect changes from codec side, so it donot set the bits in STATETS.
> 
> I read the spec, and found something different:
> - Controller should be in reset state while In D3?(Chapter 4.5.9.2 in  HD-A
> spec.)
> Azx_runtime-suspend() did not reset the controller.
> - codec will use power state change request on the link to let controller know.
> Not sure it's hardware operation or software layer.
> - codec discovery ( chapter 4.3)
> "When the link is enabled by the assertion of CRST, the codecs will detect the
> de-assertion of the RESET# signal and request a status change and
> enumeration by the controller.
>  As the controller hardware detects these requests, it will provide the codecs
> with their unique addresses and set the controller STATESTS bits  to indicate
> that a Status Change event was detected on the appropriate SDATA_INx
> signals.  "
> I'm afraid something was not configured correctly, i.e. should let controller in
> reset state when enter runtime suspend mode?

After some test, I found the controller should be in "reset" state, and CIE interrupt should be enabled,
Otherwise the wake-up event from codec could not wake up system.

I'm writing a patch to fix this.

--xingchao


 
> Thanks
> --xingchao
> >
> > Thanks for clarify this. We support 8 codec at most, so I think it's
> > safe to use writeb/readb here.
> >
> > >
> > >   - Second, and this is definitely a problem, azx_runtime_suspend
> > > calls azx_stop_chip -> azx_int_disable. So we disable interrupts,
> > > that's why we don't get them from the wake events.
> > [Wang, Xingchao]  yes, moreover I think it's not enough only enabling
> > "controller interrupt" bits in this case, PCI interrupt bits must be anbled too.
> > I'm still checking the setting in runtime_suspend.
> >
> > >
> > >   - Third, first in azx_interrupt, we check for
> > > chip->pci->dev.power.runtime_status - maybe this is no longer true
> > > chip->pci->if
> > > the device is in D3?
> > [Wang, Xingchao] yes, it's a risk here. Just move the lines before the "if"
> > condition check.
> > >
> > > Do you know how and if Windows has solved this?
> > >
> > [Wang, Xingchao] I have wrote emails for windows team guys, has no
> > response yet. I will update here when I have update.
> >
> > thanks
> > --xingchao
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel@alsa-project.org
> > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended

[Takashi Iwai]

At Thu, 18 Jul 2013 05:47:30 +0000,
Wang, Xingchao wrote:
> 
> 
> 
> > -----Original Message-----
> > From: Wang, Xingchao
> > Sent: Wednesday, July 17, 2013 6:15 PM
> > To: Wang, Xingchao; David Henningsson
> > Cc: Takashi Iwai; alsa-devel@alsa-project.org
> > Subject: RE: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info when
> > codec suspended
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: alsa-devel-bounces@alsa-project.org
> > > [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Wang,
> > > Xingchao
> > > Sent: Monday, July 15, 2013 4:54 PM
> > > To: David Henningsson; Wang Xingchao
> > > Cc: Takashi Iwai; alsa-devel@alsa-project.org
> > > Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD
> > > info when codec suspended
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: David Henningsson [mailto:david.henningsson@canonical.com]
> > > > Sent: Monday, July 15, 2013 12:29 PM
> > > > To: Wang Xingchao
> > > > Cc: Takashi Iwai; Wang, Xingchao; alsa-devel@alsa-project.org
> > > > Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD
> > > > info when codec suspended
> > > >
> > > > On 07/12/2013 08:13 AM, Wang Xingchao wrote:
> > > > > Hi David/Takashi,
> > > > >
> > > > > Here's some update on this topic.
> > > > > I used evtest to monitor hotplug input event for Headphone, it
> > > > > doesnot report the hotplug event when audio controller/codec in
> > > > > runtime suspend mode.
> > > > > if it's during audio playback(both controller and codec are
> > > > > active), the events could be monitored correctly.
> > > > > However even the controller/codec in runtime suspend mode, the
> > > > > hotplug event was not missed when waken up from suspend mode.
> > > > > After exit from suspend mode, the hotplug event would be reported asap.
> > > > > So userspace will not receive the event notification from driver
> > > > > when controller/codec in suspend mode, but it will get them once
> > > > > audio controller/codec become active.
> > > > > I think it's acceptable for audio playback functinality, it will
> > > > > not harm audio routing in fact. i'm not sure there's other
> > > > > potential risk, i.e. user space will show/hide the devices in UI
> > > > > according to the event, in that case , user will never see the
> > > > > Headphone device for playback before audio controller/codec was waken
> > up in another way.
> > > > >
> > > > > Meanwhile i tested attached patch to monitor WAKEEN events, it
> > > > > doesnot work well as Spec said. it would not wake up audio
> > > > > controller/codec when plug in/out headphone in runtime suspend
> > > > > mode, and the status register always be 0.
> > > >
> > > > Hi Xingchao,
> > > >
> > > > Maybe you're giving up too easily on the WAKEEN stuff? I can spot a
> > > > few places where your patch is incomplete:
> > > >
> > > >   - First, WAKEEN and STATETS are word registers, you should
> > > > probably use azx_writew when you access them
> > 
> > If the controller is active while codec suspended, "evtest" can catch headphone
> > hotplug event.
> > I made some changes in azx_runtime_suspend(), which in brief leave interrupt
> > enabled, then "evtest" cannot catch input event.
> > Also I added some debug log shows, when controller enter suspend mode, the
> > WAKEEN bits were set(value 0x3), after headphone hotplug, Audio controller
> > was not waken up. When audio controller exit, in azx_runtime_resume(), the
> > debug shows WAKEEN bits value no change, STATETS has value 0. The controller
> > did not detect changes from codec side, so it donot set the bits in STATETS.
> > 
> > I read the spec, and found something different:
> > - Controller should be in reset state while In D3?(Chapter 4.5.9.2 in  HD-A
> > spec.)
> > Azx_runtime-suspend() did not reset the controller.
> > - codec will use power state change request on the link to let controller know.
> > Not sure it's hardware operation or software layer.
> > - codec discovery ( chapter 4.3)
> > "When the link is enabled by the assertion of CRST, the codecs will detect the
> > de-assertion of the RESET# signal and request a status change and
> > enumeration by the controller.
> >  As the controller hardware detects these requests, it will provide the codecs
> > with their unique addresses and set the controller STATESTS bits  to indicate
> > that a Status Change event was detected on the appropriate SDATA_INx
> > signals.  "
> > I'm afraid something was not configured correctly, i.e. should let controller in
> > reset state when enter runtime suspend mode?
> 
> After some test, I found the controller should be in "reset" state, and CIE interrupt should be enabled,
> Otherwise the wake-up event from codec could not wake up system.
> 
> I'm writing a patch to fix this.

Great, thanks for checking it.


Takashi

> 
> --xingchao
> 
> 
>  
> > Thanks
> > --xingchao
> > >
> > > Thanks for clarify this. We support 8 codec at most, so I think it's
> > > safe to use writeb/readb here.
> > >
> > > >
> > > >   - Second, and this is definitely a problem, azx_runtime_suspend
> > > > calls azx_stop_chip -> azx_int_disable. So we disable interrupts,
> > > > that's why we don't get them from the wake events.
> > > [Wang, Xingchao]  yes, moreover I think it's not enough only enabling
> > > "controller interrupt" bits in this case, PCI interrupt bits must be anbled too.
> > > I'm still checking the setting in runtime_suspend.
> > >
> > > >
> > > >   - Third, first in azx_interrupt, we check for
> > > > chip->pci->dev.power.runtime_status - maybe this is no longer true
> > > > chip->pci->if
> > > > the device is in D3?
> > > [Wang, Xingchao] yes, it's a risk here. Just move the lines before the "if"
> > > condition check.
> > > >
> > > > Do you know how and if Windows has solved this?
> > > >
> > > [Wang, Xingchao] I have wrote emails for windows team guys, has no
> > > response yet. I will update here when I have update.
> > >
> > > thanks
> > > --xingchao
> > > _______________________________________________
> > > Alsa-devel mailing list
> > > Alsa-devel@alsa-project.org
> > > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>