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