[SRU][B/OEM-B/D][PATCH 0/1] Intel HDMI audio print "Unable to sync register" errors

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU][B/OEM-B/D][PATCH 0/1] Intel HDMI audio print "Unable to sync register" errors

Hui Wang
BugLink: http://bugs.launchpad.net/bugs/1840394


This fix is in the v5.3-rc3, and EAON already has this patch by backporting stable, so
this patch is only for B/D.

[Impact]
After S3, the intel hdmi audio on a dell laptop print the error like below:
[ 2052.030247] snd_hda_codec_hdmi hdaudioC0D2: Unable to sync register 0x2f0d00. -5

And the hdmi audio can't work anymore after this.

[Fix]
For Intel hdmi audio codec, it doesn't need to resume forcibly, because the i915 side
probably is not ready to let hdmi codec work, since the intel hdmi audio uses component
framework, let i915 control the hdmi codec.

[Test Case]
Tested on that dell laptops, the hdmi audio doesn't have any issues anymore after
applying this patch.

[Regression Risk]
Low. We tested this patch on a couple of machines (dell and lenovo).


Takashi Iwai (1):
  ALSA: hda - Don't resume forcibly i915 HDMI/DP codec

 include/sound/hda_codec.h  | 2 ++
 sound/pci/hda/hda_codec.c  | 8 ++++++--
 sound/pci/hda/patch_hdmi.c | 6 +++++-
 3 files changed, 13 insertions(+), 3 deletions(-)

--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][D][PATCH 1/1] ALSA: hda - Don't resume forcibly i915 HDMI/DP codec

Hui Wang
From: Takashi Iwai <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1840394

We apply the codec resume forcibly at system resume callback for
updating and syncing the jack detection state that may have changed
during sleeping.  This is, however, superfluous for the codec like
Intel HDMI/DP, where the jack detection is managed via the audio
component notification; i.e. the jack state change shall be reported
sooner or later from the graphics side at mode change.

This patch changes the codec resume callback to avoid the forcible
resume conditionally with a new flag, codec->relaxed_resume, for
reducing the resume time.  The flag is set in the codec probe.

Although this doesn't fix the entire bug mentioned in the bugzilla
entry below, it's still a good optimization and some improvements are
seen.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201901
Cc: <[hidden email]>
Signed-off-by: Takashi Iwai <[hidden email]>
(cherry picked from commit 4914da2fb0c89205790503f20dfdde854f3afdd8)
Signed-off-by: Hui Wang <[hidden email]>
---
 include/sound/hda_codec.h  | 2 ++
 sound/pci/hda/hda_codec.c  | 8 ++++++--
 sound/pci/hda/patch_hdmi.c | 6 +++++-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h
index cc7c8d42d4fd..a0d9d788527d 100644
--- a/include/sound/hda_codec.h
+++ b/include/sound/hda_codec.h
@@ -262,6 +262,8 @@ struct hda_codec {
  unsigned int auto_runtime_pm:1; /* enable automatic codec runtime pm */
  unsigned int force_pin_prefix:1; /* Add location prefix */
  unsigned int link_down_at_suspend:1; /* link down at runtime suspend */
+ unsigned int relaxed_resume:1; /* don't resume forcibly for jack */
+
 #ifdef CONFIG_PM
  unsigned long power_on_acct;
  unsigned long power_off_acct;
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index b238e903b9d7..aeb847d452e1 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -2954,15 +2954,19 @@ static int hda_codec_runtime_resume(struct device *dev)
 #ifdef CONFIG_PM_SLEEP
 static int hda_codec_force_resume(struct device *dev)
 {
+ struct hda_codec *codec = dev_to_hda_codec(dev);
+ bool forced_resume = !codec->relaxed_resume;
  int ret;
 
  /* The get/put pair below enforces the runtime resume even if the
  * device hasn't been used at suspend time.  This trick is needed to
  * update the jack state change during the sleep.
  */
- pm_runtime_get_noresume(dev);
+ if (forced_resume)
+ pm_runtime_get_noresume(dev);
  ret = pm_runtime_force_resume(dev);
- pm_runtime_put(dev);
+ if (forced_resume)
+ pm_runtime_put(dev);
  return ret;
 }
 
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index af17265b829c..44ad63c5eb63 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -2293,8 +2293,10 @@ static void generic_hdmi_free(struct hda_codec *codec)
  struct hdmi_spec *spec = codec->spec;
  int pin_idx, pcm_idx;
 
- if (codec_has_acomp(codec))
+ if (codec_has_acomp(codec)) {
  snd_hdac_acomp_register_notifier(&codec->bus->core, NULL);
+ codec->relaxed_resume = 0;
+ }
 
  for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
  struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
@@ -2550,6 +2552,8 @@ static void register_i915_notifier(struct hda_codec *codec)
  spec->drm_audio_ops.pin_eld_notify = intel_pin_eld_notify;
  snd_hdac_acomp_register_notifier(&codec->bus->core,
  &spec->drm_audio_ops);
+ /* no need for forcible resume for jack check thanks to notifier */
+ codec->relaxed_resume = 1;
 }
 
 /* setup_stream ops override for HSW+ */
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][B/OEM-B][PATCH 1/1] ALSA: hda - Don't resume forcibly i915 HDMI/DP codec

Hui Wang
In reply to this post by Hui Wang
From: Takashi Iwai <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1840394

We apply the codec resume forcibly at system resume callback for
updating and syncing the jack detection state that may have changed
during sleeping.  This is, however, superfluous for the codec like
Intel HDMI/DP, where the jack detection is managed via the audio
component notification; i.e. the jack state change shall be reported
sooner or later from the graphics side at mode change.

This patch changes the codec resume callback to avoid the forcible
resume conditionally with a new flag, codec->relaxed_resume, for
reducing the resume time.  The flag is set in the codec probe.

Although this doesn't fix the entire bug mentioned in the bugzilla
entry below, it's still a good optimization and some improvements are
seen.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201901
Cc: <[hidden email]>
Signed-off-by: Takashi Iwai <[hidden email]>
(backported from commit 4914da2fb0c89205790503f20dfdde854f3afdd8)
Signed-off-by: Hui Wang <[hidden email]>
---
 sound/pci/hda/hda_codec.c  | 14 +++++++++-----
 sound/pci/hda/hda_codec.h  |  2 ++
 sound/pci/hda/patch_hdmi.c |  6 +++++-
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index cb27acd216ac..2d6829b12b77 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -2936,16 +2936,20 @@ static int hda_codec_runtime_resume(struct device *dev)
 #ifdef CONFIG_PM_SLEEP
 static int hda_codec_force_resume(struct device *dev)
 {
-       int ret;
+ struct hda_codec *codec = dev_to_hda_codec(dev);
+ bool forced_resume = !codec->relaxed_resume;
+ int ret;
 
        /* The get/put pair below enforces the runtime resume even if the
         * device hasn't been used at suspend time.  This trick is needed to
         * update the jack state change during the sleep.
         */
-       pm_runtime_get_noresume(dev);
-       ret = pm_runtime_force_resume(dev);
-       pm_runtime_put(dev);
-       return ret;
+ if (forced_resume)
+ pm_runtime_get_noresume(dev);
+ ret = pm_runtime_force_resume(dev);
+ if (forced_resume)
+ pm_runtime_put(dev);
+ return ret;
 }
 #endif /* CONFIG_PM_SLEEP */
 
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index a8b1b31f161c..1925d223214e 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -259,6 +259,8 @@ struct hda_codec {
  unsigned int auto_runtime_pm:1; /* enable automatic codec runtime pm */
  unsigned int force_pin_prefix:1; /* Add location prefix */
  unsigned int link_down_at_suspend:1; /* link down at runtime suspend */
+ unsigned int relaxed_resume:1;  /* don't resume forcibly for jack */
+
 #ifdef CONFIG_PM
  unsigned long power_on_acct;
  unsigned long power_off_acct;
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 34d7085dfca8..a2a76055b892 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -2294,8 +2294,10 @@ static void generic_hdmi_free(struct hda_codec *codec)
  struct hdmi_spec *spec = codec->spec;
  int pin_idx, pcm_idx;
 
- if (codec_has_acomp(codec))
+ if (codec_has_acomp(codec)) {
  snd_hdac_i915_register_notifier(NULL);
+ codec->relaxed_resume = 0;
+ }
 
  for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
  struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
@@ -2526,6 +2528,8 @@ static void register_i915_notifier(struct hda_codec *codec)
  wmb();
  spec->i915_audio_ops.pin_eld_notify = intel_pin_eld_notify;
  snd_hdac_i915_register_notifier(&spec->i915_audio_ops);
+ /* no need for forcible resume for jack check thanks to notifier */
+ codec->relaxed_resume = 1;
 }
 
 /* setup_stream ops override for HSW+ */
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/APPLIED[OEM-B]: [SRU][B/OEM-B/D][PATCH 0/1] Intel HDMI audio print "Unable to sync register" errors

AceLan Kao
In reply to this post by Hui Wang
Applied on Ubuntu-oem-4.15.0-1051.59
Acked-By: AceLan Kao <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][B/D][PATCH 0/1] Intel HDMI audio print "Unable to sync register" errors

Stefan Bader-2
In reply to this post by Hui Wang
On 16.08.19 03:39, Hui Wang wrote:

> BugLink: http://bugs.launchpad.net/bugs/1840394
>
>
> This fix is in the v5.3-rc3, and EAON already has this patch by backporting stable, so
> this patch is only for B/D.
>
> [Impact]
> After S3, the intel hdmi audio on a dell laptop print the error like below:
> [ 2052.030247] snd_hda_codec_hdmi hdaudioC0D2: Unable to sync register 0x2f0d00. -5
>
> And the hdmi audio can't work anymore after this.
>
> [Fix]
> For Intel hdmi audio codec, it doesn't need to resume forcibly, because the i915 side
> probably is not ready to let hdmi codec work, since the intel hdmi audio uses component
> framework, let i915 control the hdmi codec.
>
> [Test Case]
> Tested on that dell laptops, the hdmi audio doesn't have any issues anymore after
> applying this patch.
>
> [Regression Risk]
> Low. We tested this patch on a couple of machines (dell and lenovo).
>
>
> Takashi Iwai (1):
>   ALSA: hda - Don't resume forcibly i915 HDMI/DP codec
>
>  include/sound/hda_codec.h  | 2 ++
>  sound/pci/hda/hda_codec.c  | 8 ++++++--
>  sound/pci/hda/patch_hdmi.c | 6 +++++-
>  3 files changed, 13 insertions(+), 3 deletions(-)
>
Acked-by: Stefan Bader <[hidden email]>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][B/OEM-B/D][PATCH 0/1] Intel HDMI audio print "Unable to sync register" errors

Connor Kuehl
In reply to this post by Hui Wang
On 8/15/19 6:39 PM, Hui Wang wrote:

> BugLink: http://bugs.launchpad.net/bugs/1840394
>
>
> This fix is in the v5.3-rc3, and EAON already has this patch by backporting stable, so
> this patch is only for B/D.
>
> [Impact]
> After S3, the intel hdmi audio on a dell laptop print the error like below:
> [ 2052.030247] snd_hda_codec_hdmi hdaudioC0D2: Unable to sync register 0x2f0d00. -5
>
> And the hdmi audio can't work anymore after this.
>
> [Fix]
> For Intel hdmi audio codec, it doesn't need to resume forcibly, because the i915 side
> probably is not ready to let hdmi codec work, since the intel hdmi audio uses component
> framework, let i915 control the hdmi codec.
>
> [Test Case]
> Tested on that dell laptops, the hdmi audio doesn't have any issues anymore after
> applying this patch.
>
> [Regression Risk]
> Low. We tested this patch on a couple of machines (dell and lenovo).
>
>
> Takashi Iwai (1):
>    ALSA: hda - Don't resume forcibly i915 HDMI/DP codec
>
>   include/sound/hda_codec.h  | 2 ++
>   sound/pci/hda/hda_codec.c  | 8 ++++++--
>   sound/pci/hda/patch_hdmi.c | 6 +++++-
>   3 files changed, 13 insertions(+), 3 deletions(-)
>

Acked-by: Connor Kuehl <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED[B/D]: [SRU][B/OEM-B/D][PATCH 0/1] Intel HDMI audio print "Unable to sync register" errors

Kleber Souza
In reply to this post by Hui Wang
On 8/16/19 3:39 AM, Hui Wang wrote:

> BugLink: http://bugs.launchpad.net/bugs/1840394
>
>
> This fix is in the v5.3-rc3, and EAON already has this patch by backporting stable, so
> this patch is only for B/D.
>
> [Impact]
> After S3, the intel hdmi audio on a dell laptop print the error like below:
> [ 2052.030247] snd_hda_codec_hdmi hdaudioC0D2: Unable to sync register 0x2f0d00. -5
>
> And the hdmi audio can't work anymore after this.
>
> [Fix]
> For Intel hdmi audio codec, it doesn't need to resume forcibly, because the i915 side
> probably is not ready to let hdmi codec work, since the intel hdmi audio uses component
> framework, let i915 control the hdmi codec.
>
> [Test Case]
> Tested on that dell laptops, the hdmi audio doesn't have any issues anymore after
> applying this patch.
>
> [Regression Risk]
> Low. We tested this patch on a couple of machines (dell and lenovo).
>
>
> Takashi Iwai (1):
>   ALSA: hda - Don't resume forcibly i915 HDMI/DP codec
>
>  include/sound/hda_codec.h  | 2 ++
>  sound/pci/hda/hda_codec.c  | 8 ++++++--
>  sound/pci/hda/patch_hdmi.c | 6 +++++-
>  3 files changed, 13 insertions(+), 3 deletions(-)
>

Applied to bionic/master-next and disco/master-next branches.

Thanks,
Kleber

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team