Hi Takashi,
Thanks for your review! The revised patches are submitted: [PATCH v2 1/2] drm/i915: provide interface for audio driver to query cdclk [PATCH v2 2/2] ALSA: hda - restore BCLK M/N value as per CDCLK for HSW/BDW display HDA controller
And we think these patches may have Cc to stable, since Haswell need them.
Hi Jani,
Your first patch was also submitted to Intel gfx mailing list. Thanks for your help on this issue!
Regards Mengdong
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, July 02, 2014 11:01 PM To: Lin, Mengdong Cc: alsa-devel@alsa-project.org; Nikula, Jani Subject: Re: [PATCH 3/3] ALSA: hda - restore BCLK M/N value as per CDCLK for HSW/BDW display HDA controller
At Wed, 2 Jul 2014 22:44:07 +0800, mengdong.lin@intel.com wrote:
From: Mengdong Lin mengdong.lin@intel.com
For HSW/BDW display HD-A controller, this patch restores BCLK by setting the M/N values as per the core display clock (CDCLK) queried from i915 display driver.
And the audio driver will also restore BCLK in azx_first_init() since the display driver can turn off the shared power in boot phase if only eDP is connected and M/N values will be lost and must be
reprogrammed.
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 25753db..c7dfd1d 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -299,10 +299,6 @@ static char *driver_short_names[] = {
struct hda_intel { struct azx chip;
- /* HSW/BDW display HDA controller to restore BCLK from CDCLK */
- unsigned int bclk_m;
- unsigned int bclk_n;
};
@@ -598,20 +594,41 @@ 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);
- int cdclk_freq;
- unsigned int bclk_m, bclk_n;
- cdclk_freq = hda_get_display_clk();
- switch (cdclk_freq) {
- case 337500:
bclk_m = 16;bclk_n = 225;break;- case 450000: /* default CDCLK 450MHz */
bclk_m = 4;bclk_n = 75;break;- case 540000:
bclk_m = 4;bclk_n = 90;break;- case 675000:
bclk_m = 8;bclk_n = 225;break;
- azx_writew(chip, EM4, hda->bclk_m);
- azx_writew(chip, EM5, hda->bclk_n);
- default:
bclk_m = 4;bclk_n = 75;You can put "default:" around "case 45000:", and move the comment about default CDCLK there.
- }
- azx_writew(chip, EM4, bclk_m);
- azx_writew(chip, EM5, bclk_n);
}
IMO, the whole function can be moved to hda_i915.c and provided as a function such as hda_set_display_clk(). Then you can drop hda_get_display_clk() but call get_cdclk() internally.
Also, all these patches may have Cc to stable, right?
thanks,
Takashi