[alsa-devel] [PATCH v2] ALSA: hda - restore BCLK M/N values when resuming HSW/BDW display controller
From: Mengdong Lin mengdong.lin@intel.com
For Intel Haswell/Broadwell display HD-A controller, the 24MHz HD-A link BCLK is converted from Core Display Clock (CDCLK): BCLK = CDCLK * M / N And there are two registers EM4 and EM5 to program M, N value respectively. The EM4/EM5 values will be lost and when the display power well is disabled.
BIOS programs CDCLK selected by OEM and EM4/EM5, but BIOS has no idea about display power well on/off at runtime. So the M/N can be wrong if non-default CDCLK is used when the audio controller resumes, which results in an invalid BCLK and abnormal audio playback rate. So this patch saves and restores valid M/N values on controller suspend/resume.
And 'struct hda_intel' is defined to contain standard HD-A 'struct azx' and Intel specific fields, as Takashi suggested.
Signed-off-by: Mengdong Lin mengdong.lin@intel.com
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index bb65a124..ff9cacd 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -288,6 +288,24 @@ static char *driver_short_names[] = { [AZX_DRIVER_GENERIC] = "HD-Audio Generic", };
+ +/* Intel HSW/BDW display HDA controller Extended Mode registers. + * EM4 (M value) and EM5 (N Value) are used to convert CDClk (Core Display + * Clock) to 24MHz BCLK: BCLK = CDCLK * M / N + * The values will be lost when the display power well is disabled. + */ +#define ICH6_REG_EM4 0x100c +#define ICH6_REG_EM5 0x1010 + +struct hda_intel { + struct azx chip; + + /* HSW/BDW display HDA controller to restore BCLK from CDCLK */ + unsigned int bclk_m; + unsigned int bclk_n; +}; + + #ifdef CONFIG_X86 static void __mark_pages_wc(struct azx *chip, struct snd_dma_buffer *dmab, bool on) { @@ -580,6 +598,22 @@ static int param_set_xint(const char *val, const struct kernel_param *kp) #define azx_del_card_list(chip) /* NOP */ #endif /* CONFIG_PM */
+static void haswell_save_bclk(struct azx *chip) +{ + struct hda_intel *hda = container_of(chip, struct hda_intel, chip); + + hda->bclk_m = azx_readw(chip, EM4); + hda->bclk_n = azx_readw(chip, EM5); +} + +static void haswell_restore_bclk(struct azx *chip) +{ + struct hda_intel *hda = container_of(chip, struct hda_intel, chip); + + azx_writew(chip, EM4, hda->bclk_m); + azx_writew(chip, EM5, hda->bclk_n); +} + #if defined(CONFIG_PM_SLEEP) || defined(SUPPORT_VGA_SWITCHEROO) /* * power management @@ -606,6 +640,13 @@ static int azx_suspend(struct device *dev) free_irq(chip->irq, chip); chip->irq = -1; } + + /* Save BCLK M/N values before they become invalid in D3. + * Will test if display power well can be released now. + */ + if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) + haswell_save_bclk(chip); + if (chip->msi) pci_disable_msi(chip->pci); pci_disable_device(pci); @@ -625,8 +666,10 @@ static int azx_resume(struct device *dev) if (chip->disabled) return 0;
- if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) + if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) { hda_display_power(true); + haswell_restore_bclk(chip); + } pci_set_power_state(pci, PCI_D0); pci_restore_state(pci); if (pci_enable_device(pci) < 0) { @@ -670,8 +713,10 @@ static int azx_runtime_suspend(struct device *dev) azx_stop_chip(chip); azx_enter_link_reset(chip); azx_clear_irq_pending(chip); - if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) + if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) { + haswell_save_bclk(chip); hda_display_power(false); + } return 0; }
@@ -689,8 +734,10 @@ static int azx_runtime_resume(struct device *dev) if (!(chip->driver_caps & AZX_DCAPS_PM_RUNTIME)) return 0;
- if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) + if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) { hda_display_power(true); + haswell_restore_bclk(chip); + }
/* Read STATESTS before controller reset */ status = azx_readw(chip, STATESTS); @@ -883,6 +930,8 @@ static int register_vga_switcheroo(struct azx *chip) static int azx_free(struct azx *chip) { struct pci_dev *pci = chip->pci; + struct hda_intel *hda = container_of(chip, struct hda_intel, chip); + int i;
if ((chip->driver_caps & AZX_DCAPS_PM_RUNTIME) @@ -930,7 +979,7 @@ static int azx_free(struct azx *chip) hda_display_power(false); hda_i915_exit(); } - kfree(chip); + kfree(hda);
return 0; } @@ -1174,6 +1223,7 @@ static int azx_create(struct snd_card *card, struct pci_dev *pci, static struct snd_device_ops ops = { .dev_free = azx_dev_free, }; + struct hda_intel *hda; struct azx *chip; int err;
@@ -1183,13 +1233,14 @@ static int azx_create(struct snd_card *card, struct pci_dev *pci, if (err < 0) return err;
- chip = kzalloc(sizeof(*chip), GFP_KERNEL); - if (!chip) { - dev_err(card->dev, "Cannot allocate chip\n"); + hda = kzalloc(sizeof(*hda), GFP_KERNEL); + if (!hda) { + dev_err(card->dev, "Cannot allocate hda\n"); pci_disable_device(pci); return -ENOMEM; }
+ chip = &hda->chip; spin_lock_init(&chip->reg_lock); mutex_init(&chip->open_mutex); chip->card = card;
At Thu, 26 Jun 2014 18:45:16 +0800, mengdong.lin@intel.com wrote:
From: Mengdong Lin mengdong.lin@intel.com
For Intel Haswell/Broadwell display HD-A controller, the 24MHz HD-A link BCLK is converted from Core Display Clock (CDCLK): BCLK = CDCLK * M / N And there are two registers EM4 and EM5 to program M, N value respectively. The EM4/EM5 values will be lost and when the display power well is disabled.
BIOS programs CDCLK selected by OEM and EM4/EM5, but BIOS has no idea about display power well on/off at runtime. So the M/N can be wrong if non-default CDCLK is used when the audio controller resumes, which results in an invalid BCLK and abnormal audio playback rate. So this patch saves and restores valid M/N values on controller suspend/resume.
And 'struct hda_intel' is defined to contain standard HD-A 'struct azx' and Intel specific fields, as Takashi suggested.
Signed-off-by: Mengdong Lin mengdong.lin@intel.com
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index bb65a124..ff9cacd 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -288,6 +288,24 @@ static char *driver_short_names[] = { [AZX_DRIVER_GENERIC] = "HD-Audio Generic", };
+/* Intel HSW/BDW display HDA controller Extended Mode registers.
- EM4 (M value) and EM5 (N Value) are used to convert CDClk (Core Display
- Clock) to 24MHz BCLK: BCLK = CDCLK * M / N
- The values will be lost when the display power well is disabled.
- */
+#define ICH6_REG_EM4 0x100c +#define ICH6_REG_EM5 0x1010
Avoid ICH6 prefix. It's just confusing as these are only for HSW/BDW.
Other than that, looks good to me.
thanks,
Takashi
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Thursday, June 26, 2014 6:55 PM To: Lin, Mengdong Cc: alsa-devel@alsa-project.org Subject: Re: [PATCH v2] ALSA: hda - restore BCLK M/N values when resuming HSW/BDW display controller
At Thu, 26 Jun 2014 18:45:16 +0800, mengdong.lin@intel.com wrote:
From: Mengdong Lin mengdong.lin@intel.com
For Intel Haswell/Broadwell display HD-A controller, the 24MHz HD-A link BCLK is converted from Core Display Clock (CDCLK): BCLK = CDCLK * M / N And there are two registers EM4 and EM5 to program M, N value
respectively.
The EM4/EM5 values will be lost and when the display power well is
disabled.
BIOS programs CDCLK selected by OEM and EM4/EM5, but BIOS has no
idea
about display power well on/off at runtime. So the M/N can be wrong if non-default CDCLK is used when the audio controller resumes, which results in an invalid BCLK and abnormal audio playback rate. So this patch saves and restores valid M/N values on controller
suspend/resume.
And 'struct hda_intel' is defined to contain standard HD-A 'struct azx' and Intel specific fields, as Takashi suggested.
Signed-off-by: Mengdong Lin mengdong.lin@intel.com
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index bb65a124..ff9cacd 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -288,6 +288,24 @@ static char *driver_short_names[] = { [AZX_DRIVER_GENERIC] = "HD-Audio Generic", };
+/* Intel HSW/BDW display HDA controller Extended Mode registers.
- EM4 (M value) and EM5 (N Value) are used to convert CDClk (Core
+Display
- Clock) to 24MHz BCLK: BCLK = CDCLK * M / N
- The values will be lost when the display power well is disabled.
- */
+#define ICH6_REG_EM4 0x100c +#define ICH6_REG_EM5 0x1010
Avoid ICH6 prefix. It's just confusing as these are only for HSW/BDW.
Other than that, looks good to me.
So how about adding these?
+#define HDMI_REG_EM4 0x100c +#define HDMI_REG_EM5 0x1010
#define azx_hdmi_readw(chip, reg) \ ((chip)->ops->reg_readw((chip)->remap_addr + HDMI_REG_##reg))
#define azx_hdmi_writew(chip, reg, value) \ ((chip)->ops->reg_writew(value, (chip)->remap_addr + HDMI_REG_##reg))
static void haswell_save_bclk(struct azx *chip) { struct hda_intel *hda = container_of(chip, struct hda_intel, chip);
hda->bclk_m = azx_hdmi_readw(chip, EM4); hda->bclk_n = azx_hdmi_readw(chip, EM5); }
Or use "intel" instead of "hdmi", indicating it's Intel specific.
Thanks Mengdong
At Thu, 26 Jun 2014 11:16:46 +0000, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Thursday, June 26, 2014 6:55 PM To: Lin, Mengdong Cc: alsa-devel@alsa-project.org Subject: Re: [PATCH v2] ALSA: hda - restore BCLK M/N values when resuming HSW/BDW display controller
At Thu, 26 Jun 2014 18:45:16 +0800, mengdong.lin@intel.com wrote:
From: Mengdong Lin mengdong.lin@intel.com
For Intel Haswell/Broadwell display HD-A controller, the 24MHz HD-A link BCLK is converted from Core Display Clock (CDCLK): BCLK = CDCLK * M / N And there are two registers EM4 and EM5 to program M, N value
respectively.
The EM4/EM5 values will be lost and when the display power well is
disabled.
BIOS programs CDCLK selected by OEM and EM4/EM5, but BIOS has no
idea
about display power well on/off at runtime. So the M/N can be wrong if non-default CDCLK is used when the audio controller resumes, which results in an invalid BCLK and abnormal audio playback rate. So this patch saves and restores valid M/N values on controller
suspend/resume.
And 'struct hda_intel' is defined to contain standard HD-A 'struct azx' and Intel specific fields, as Takashi suggested.
Signed-off-by: Mengdong Lin mengdong.lin@intel.com
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index bb65a124..ff9cacd 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -288,6 +288,24 @@ static char *driver_short_names[] = { [AZX_DRIVER_GENERIC] = "HD-Audio Generic", };
+/* Intel HSW/BDW display HDA controller Extended Mode registers.
- EM4 (M value) and EM5 (N Value) are used to convert CDClk (Core
+Display
- Clock) to 24MHz BCLK: BCLK = CDCLK * M / N
- The values will be lost when the display power well is disabled.
- */
+#define ICH6_REG_EM4 0x100c +#define ICH6_REG_EM5 0x1010
Avoid ICH6 prefix. It's just confusing as these are only for HSW/BDW.
Other than that, looks good to me.
So how about adding these?
+#define HDMI_REG_EM4 0x100c +#define HDMI_REG_EM5 0x1010
#define azx_hdmi_readw(chip, reg) \ ((chip)->ops->reg_readw((chip)->remap_addr + HDMI_REG_##reg))
#define azx_hdmi_writew(chip, reg, value) \ ((chip)->ops->reg_writew(value, (chip)->remap_addr + HDMI_REG_##reg))
static void haswell_save_bclk(struct azx *chip) { struct hda_intel *hda = container_of(chip, struct hda_intel, chip);
hda->bclk_m = azx_hdmi_readw(chip, EM4); hda->bclk_n = azx_hdmi_readw(chip, EM5); }
Or use "intel" instead of "hdmi", indicating it's Intel specific.
I'd just open-code a few read and write lines.
Takashi
At Thu, 26 Jun 2014 14:01:10 +0200, Takashi Iwai wrote:
At Thu, 26 Jun 2014 11:16:46 +0000, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Thursday, June 26, 2014 6:55 PM To: Lin, Mengdong Cc: alsa-devel@alsa-project.org Subject: Re: [PATCH v2] ALSA: hda - restore BCLK M/N values when resuming HSW/BDW display controller
At Thu, 26 Jun 2014 18:45:16 +0800, mengdong.lin@intel.com wrote:
From: Mengdong Lin mengdong.lin@intel.com
For Intel Haswell/Broadwell display HD-A controller, the 24MHz HD-A link BCLK is converted from Core Display Clock (CDCLK): BCLK = CDCLK * M / N And there are two registers EM4 and EM5 to program M, N value
respectively.
The EM4/EM5 values will be lost and when the display power well is
disabled.
BIOS programs CDCLK selected by OEM and EM4/EM5, but BIOS has no
idea
about display power well on/off at runtime. So the M/N can be wrong if non-default CDCLK is used when the audio controller resumes, which results in an invalid BCLK and abnormal audio playback rate. So this patch saves and restores valid M/N values on controller
suspend/resume.
And 'struct hda_intel' is defined to contain standard HD-A 'struct azx' and Intel specific fields, as Takashi suggested.
Signed-off-by: Mengdong Lin mengdong.lin@intel.com
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index bb65a124..ff9cacd 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -288,6 +288,24 @@ static char *driver_short_names[] = { [AZX_DRIVER_GENERIC] = "HD-Audio Generic", };
+/* Intel HSW/BDW display HDA controller Extended Mode registers.
- EM4 (M value) and EM5 (N Value) are used to convert CDClk (Core
+Display
- Clock) to 24MHz BCLK: BCLK = CDCLK * M / N
- The values will be lost when the display power well is disabled.
- */
+#define ICH6_REG_EM4 0x100c +#define ICH6_REG_EM5 0x1010
Avoid ICH6 prefix. It's just confusing as these are only for HSW/BDW.
Other than that, looks good to me.
So how about adding these?
+#define HDMI_REG_EM4 0x100c +#define HDMI_REG_EM5 0x1010
#define azx_hdmi_readw(chip, reg) \ ((chip)->ops->reg_readw((chip)->remap_addr + HDMI_REG_##reg))
#define azx_hdmi_writew(chip, reg, value) \ ((chip)->ops->reg_writew(value, (chip)->remap_addr + HDMI_REG_##reg))
static void haswell_save_bclk(struct azx *chip) { struct hda_intel *hda = container_of(chip, struct hda_intel, chip);
hda->bclk_m = azx_hdmi_readw(chip, EM4); hda->bclk_n = azx_hdmi_readw(chip, EM5); }
Or use "intel" instead of "hdmi", indicating it's Intel specific.
I'd just open-code a few read and write lines.
Thinking of this again, maybe it's better to keep the patch smaller at this stage, just concentrating on the fix.
We can get rid of ICH6 prefix later with a series of cleanup patches.
I'm going to take your rev2 patch then.
thanks,
Takashi
Hi Takashi,
Sorry. This patch cannot handle the following case: If only embedded DP is connected on boot, the gfx driver will turn off the display power well on boot case before the audio driver probes and request this power. So the M/V values programmed by BIOS in EM4/5 registers will be lost at the very beginning. And so the audio driver will save invalid M/N values on suspend and restore these invalid values on resume. One phenomenon is: if HDMI or DP monitor is connected after boot, audio playback rate is ~20% faster than normal. And I guess it's the patch cannot fix the Haswell issue https://bugzilla.kernel.org/show_bug.cgi?id=74861
There are two possible solutions: (1) Follows Windows model: Windows, gfx driver notifies BIOS on LPSP (when only eDP is connected) exit and BIOS reprograms those EM4/5 registers. There is no such implementation in Linux gfx driver now. We need time to check the effort.
(2) Gfx driver notify audio driver on exiting LPSP and CDCLK value (by reading GPU registers), so audio driver can reprograms EM4/5 registers. The open source interface between audio/gfx driver is still not ready. Would an private API to get CDCLK from gfx driver will be a possible workaround atm?
Could you share your opinion on this issue?
Thanks Mengdong
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Thursday, June 26, 2014 9:47 PM
I'd just open-code a few read and write lines.
Thinking of this again, maybe it's better to keep the patch smaller at this stage, just concentrating on the fix.
We can get rid of ICH6 prefix later with a series of cleanup patches.
I'm going to take your rev2 patch then.
thanks,
Takashi
At Fri, 27 Jun 2014 06:38:47 +0000, Lin, Mengdong wrote:
Hi Takashi,
Sorry. This patch cannot handle the following case: If only embedded DP is connected on boot, the gfx driver will turn off the display power well on boot case before the audio driver probes and request this power. So the M/V values programmed by BIOS in EM4/5 registers will be lost at the very beginning. And so the audio driver will save invalid M/N values on suspend and restore these invalid values on resume. One phenomenon is: if HDMI or DP monitor is connected after boot, audio playback rate is ~20% faster than normal. And I guess it's the patch cannot fix the Haswell issue https://bugzilla.kernel.org/show_bug.cgi?id=74861
There are two possible solutions: (1) Follows Windows model: Windows, gfx driver notifies BIOS on LPSP (when only eDP is connected) exit and BIOS reprograms those EM4/5 registers. There is no such implementation in Linux gfx driver now. We need time to check the effort.
(2) Gfx driver notify audio driver on exiting LPSP and CDCLK value (by reading GPU registers), so audio driver can reprograms EM4/5 registers. The open source interface between audio/gfx driver is still not ready. Would an private API to get CDCLK from gfx driver will be a possible workaround atm?
Could you share your opinion on this issue?
To me, the former sounds like an easier solution. In the latter case, it can be still an issue of module loading order.
In anyway, I keep the former patch as is, since it fixes actually some cases although it's not perfect. Please work on top of it for further fixes.
thanks,
Takashi
Add Jani from Gfx driver team and audio team.
Hi Takashi,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, June 27, 2014 6:28 PM
At Fri, 27 Jun 2014 06:38:47 +0000, Lin, Mengdong wrote:
Hi Takashi,
Sorry. This patch cannot handle the following case: If only embedded DP is connected on boot, the gfx driver will turn off
the display power well on boot case before the audio driver probes and request this power.
So the M/V values programmed by BIOS in EM4/5 registers will be lost at
the very beginning. And so the audio driver will save invalid M/N values on suspend and restore these invalid values on resume.
One phenomenon is: if HDMI or DP monitor is connected after boot,
audio playback rate is ~20% faster than normal.
And I guess it's the patch cannot fix the Haswell issue https://bugzilla.kernel.org/show_bug.cgi?id=74861
There are two possible solutions: (1) Follows Windows model: Windows, gfx driver notifies BIOS on LPSP
(when only eDP is connected) exit and BIOS reprograms those EM4/5 registers.
There is no such implementation in Linux gfx driver now. We need
time to check the effort.
(2) Gfx driver notify audio driver on exiting LPSP and CDCLK value (by
reading GPU registers), so audio driver can reprograms EM4/5 registers.
The open source interface between audio/gfx driver is still not ready.
Would an private API to get CDCLK from gfx driver will be a possible workaround atm?
Could you share your opinion on this issue?
To me, the former sounds like an easier solution. In the latter case, it can be still an issue of module loading order.
Can we introduce a private i915 interface for the audio driver to get the CDCLK as a quick fix? Then the audio driver can reprogram the EM4/5 M/N values as per the CDCLK.
The interface can be like 'int i915_get_cdclk_freq(void)' and the audio driver will use symbol_request() to get this private API and avoid the issue of module loading order. We can put the relevant code in sound/pci/hda/ hda_i915.c
For Broadwell Jani has implemented a notification to BIOS, so BIOS can reprogram the EM4/5 value after restoring the power. http://patchwork.freedesktop.org/patch/28866/ http://patchwork.freedesktop.org/patch/28868/
But for Haswell, BIOS does not support the above notification and provide another heavy-weight notification which will enable/disable the HD-A controller at runtime and so need OS to re-enumerate the device stack. That causes system hang. And test on Haswell shows that reprogramming the EM4/5 as per the CDCLK is also enough to give good BCLK and audio rate. So we hope to avoid further dependency on the black-box BIOS.
Libin is working on the generic interface between audio and gfx now and we can clean up code in the future. But before the interface is ready, a private API to query CDCLK seems to be a quick and reliable fix for Haswell and Broadwell.
Thanks Mengdong
Hi Takashi,
I submitted patches, using the private API to query CDCLK from i915 and restore BCLK. Could you have a review? http://mailman.alsa-project.org/pipermail/alsa-devel/2014-July/078489.html
The 1st patch by Jani is actually for i915 to provide the interface for audio driver to query CDCLK.
Thanks Mengdong
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Lin, Mengdong Sent: Wednesday, July 02, 2014 6:58 PM To: Takashi Iwai Cc: Nikula, Jani; Yang, Libin; alsa-devel@alsa-project.org; Li, Jocelyn Subject: Re: [alsa-devel] [PATCH v2] ALSA: hda - restore BCLK M/N values when resuming HSW/BDW display controller
Add Jani from Gfx driver team and audio team.
Hi Takashi,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, June 27, 2014 6:28 PM
At Fri, 27 Jun 2014 06:38:47 +0000, Lin, Mengdong wrote:
Hi Takashi,
Sorry. This patch cannot handle the following case: If only embedded DP is connected on boot, the gfx driver will turn off
the display power well on boot case before the audio driver probes and request this power.
So the M/V values programmed by BIOS in EM4/5 registers will be lost at
the very beginning. And so the audio driver will save invalid M/N values on suspend and restore these invalid values on resume.
One phenomenon is: if HDMI or DP monitor is connected after boot,
audio playback rate is ~20% faster than normal.
And I guess it's the patch cannot fix the Haswell issue https://bugzilla.kernel.org/show_bug.cgi?id=74861
There are two possible solutions: (1) Follows Windows model: Windows, gfx driver notifies BIOS on LPSP
(when only eDP is connected) exit and BIOS reprograms those EM4/5 registers.
There is no such implementation in Linux gfx driver now. We need
time to check the effort.
(2) Gfx driver notify audio driver on exiting LPSP and CDCLK value (by
reading GPU registers), so audio driver can reprograms EM4/5 registers.
The open source interface between audio/gfx driver is still not
ready.
Would an private API to get CDCLK from gfx driver will be a possible workaround atm?
Could you share your opinion on this issue?
To me, the former sounds like an easier solution. In the latter case, it can be still an issue of module loading order.
Can we introduce a private i915 interface for the audio driver to get the CDCLK as a quick fix? Then the audio driver can reprogram the EM4/5 M/N values as per the CDCLK.
The interface can be like 'int i915_get_cdclk_freq(void)' and the audio driver will use symbol_request() to get this private API and avoid the issue of module loading order. We can put the relevant code in sound/pci/hda/ hda_i915.c
For Broadwell Jani has implemented a notification to BIOS, so BIOS can reprogram the EM4/5 value after restoring the power. http://patchwork.freedesktop.org/patch/28866/ http://patchwork.freedesktop.org/patch/28868/
But for Haswell, BIOS does not support the above notification and provide another heavy-weight notification which will enable/disable the HD-A controller at runtime and so need OS to re-enumerate the device stack. That causes system hang. And test on Haswell shows that reprogramming the EM4/5 as per the CDCLK is also enough to give good BCLK and audio rate. So we hope to avoid further dependency on the black-box BIOS.
Libin is working on the generic interface between audio and gfx now and we can clean up code in the future. But before the interface is ready, a private API to query CDCLK seems to be a quick and reliable fix for Haswell and Broadwell.
Thanks Mengdong
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
participants (3)
-
Lin, Mengdong -
mengdong.lin＠intel.com
-
Takashi Iwai