[alsa-devel] [PATCH] ALSA: hda - set intel audio clock to a properly value
From: Libin Yang libin.yang@intel.com
On some Intel platforms, the audio clock may not be set correctly with initial setting. This will cause the audio playback/capture rates wrong.
This patch checks the audio clock setting and will set it to a properly value if it is not set correct.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188411
Signed-off-by: Libin Yang libin.yang@intel.com --- include/sound/hda_register.h | 12 +++-- sound/hda/ext/hdac_ext_controller.c | 6 +-- sound/pci/hda/hda_intel.c | 91 +++++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 6 deletions(-)
diff --git a/include/sound/hda_register.h b/include/sound/hda_register.h index 0013063..7ea16cb 100644 --- a/include/sound/hda_register.h +++ b/include/sound/hda_register.h @@ -227,6 +227,8 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 }; #define AZX_REG_PPLCLLPU 0xC
/* registers for Multiple Links Capability Structure */ +/* Multiple Links Capability */ +#define AZX_REG_ML_CAP_BASE 0xc00 #define AZX_ML_CAP_ID 0x2 #define AZX_REG_ML_MLCH 0x00 #define AZX_REG_ML_MLCD 0x04 @@ -243,9 +245,13 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 }; #define AZX_REG_ML_LOUTPAY 0x20 #define AZX_REG_ML_LINPAY 0x30
-#define AZX_MLCTL_SPA (1<<16) -#define AZX_MLCTL_CPA 23 - +#define AZX_REG_ML_LCAPx(x) (AZX_REG_ML_CAP_BASE + (0x40 + 0x40 * x)) +#define AZX_REG_ML_LCTLx(x) (AZX_REG_ML_CAP_BASE + (0x44 + 0x40 * x)) +#define ML_LCTL_SCF_MASK 0xF +#define AZX_MLCTL_SPA (0x1 << 16) +#define AZX_MLCTL_CPA (0x1 << 23) +#define AZX_MLCTL_SPA_SHIFT 16 +#define AZX_MLCTL_CPA_SHIFT 23
/* registers for DMA Resume Capability Structure */ #define AZX_DRSM_CAP_ID 0x5 diff --git a/sound/hda/ext/hdac_ext_controller.c b/sound/hda/ext/hdac_ext_controller.c index 2614691..84f3b81 100644 --- a/sound/hda/ext/hdac_ext_controller.c +++ b/sound/hda/ext/hdac_ext_controller.c @@ -171,7 +171,7 @@ static int check_hdac_link_power_active(struct hdac_ext_link *link, bool enable) { int timeout; u32 val; - int mask = (1 << AZX_MLCTL_CPA); + int mask = (1 << AZX_MLCTL_CPA_SHIFT);
udelay(3); timeout = 150; @@ -179,10 +179,10 @@ static int check_hdac_link_power_active(struct hdac_ext_link *link, bool enable) do { val = readl(link->ml_addr + AZX_REG_ML_LCTL); if (enable) { - if (((val & mask) >> AZX_MLCTL_CPA)) + if (((val & mask) >> AZX_MLCTL_CPA_SHIFT)) return 0; } else { - if (!((val & mask) >> AZX_MLCTL_CPA)) + if (!((val & mask) >> AZX_MLCTL_CPA_SHIFT)) return 0; } udelay(3); diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index c8256a8..017f64f 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -539,6 +539,94 @@ static void bxt_reduce_dma_latency(struct azx *chip) azx_writel(chip, SKL_EM4L, val); }
+/* + * ML_LCAP bits: + * bit 0: 6 MHz Supported + * bit 1: 12 MHz Supported + * bit 2: 24 MHz Supported + * bit 3: 48 MHz Supported + * bit 4: 96 MHz Supported + * bit 5: 192 MHz Supported + */ +static int intel_get_lctl_scf(struct azx *chip) +{ + u32 val; + + val = azx_readl(chip, ML_LCAPx(0)); + + if (val & (1 << 2)) + return 2; + else if (val & (1 << 3)) + return 3; + else if (val & (1 << 1)) + return 1; + else if (val & (1 << 4)) + return 4; + else if (val & (1 << 5)) + return 5; + + dev_warn(chip->card->dev, "set audio clock frequency to 6MHz"); + return 0; +} + +static void intel_init_lctl(struct azx *chip) +{ + u32 val; + int timeout; + + /* 0. check lctl register value is correct or not */ + /* the codecs are sharing the first link setting by default */ + val = azx_readl(chip, ML_LCTLx(0)); + /* if SCF is already set, let's use it */ + if ((val & ML_LCTL_SCF_MASK) != 0) + return; + + /* + * Before operatiing on SPA, CPA must match SPA. + * Any deviation may result in undefined behavior. + */ + if (((val & AZX_MLCTL_SPA) >> AZX_MLCTL_SPA_SHIFT) ^ + ((val & AZX_MLCTL_CPA) >> AZX_MLCTL_CPA_SHIFT)) + return; + + /* 1. turn link down: set SPA to 0 and wait CPA to 0 */ + val = azx_readl(chip, ML_LCTLx(0)); + val &= ~AZX_MLCTL_SPA; + azx_writel(chip, ML_LCTLx(0), val); + /* wait for CPA */ + timeout = 50; + while (timeout) { + if ((azx_readl(chip, ML_LCTLx(0)) & AZX_MLCTL_CPA) == 0) + break; + timeout--; + udelay(10); + } + if (!timeout) + goto SET_SPA; + /* need add 100ns delay for hardware ready */ + udelay(100); + + /* 2. update SCF to select a properly audio clock*/ + val &= ~ML_LCTL_SCF_MASK; + val |= intel_get_lctl_scf(chip); + azx_writel(chip, ML_LCTLx(0), val); + +SET_SPA: + /* 4. turn link up: set SPA to 1 and wait CPA to 1 */ + val = azx_readl(chip, ML_LCTLx(0)); + val |= AZX_MLCTL_SPA; + azx_writel(chip, ML_LCTLx(0), val); + timeout = 50; + while (timeout) { + if ((azx_readl(chip, ML_LCTLx(0)) & AZX_MLCTL_CPA) != 0) + break; + timeout--; + udelay(10); + } + /* need add 100ns delay for hardware ready */ + udelay(100); +} + static void hda_intel_init_chip(struct azx *chip, bool full_reset) { struct hdac_bus *bus = azx_bus(chip); @@ -564,6 +652,9 @@ static void hda_intel_init_chip(struct azx *chip, bool full_reset) /* reduce dma latency to avoid noise */ if (IS_BXT(pci)) bxt_reduce_dma_latency(chip); + + if (IS_SKL_PLUS(pci)) + intel_init_lctl(chip); }
/* calculate runtime delay from LPIB */
On Tue, 07 Mar 2017 07:20:22 +0100, libin.yang@intel.com wrote:
From: Libin Yang libin.yang@intel.com
On some Intel platforms, the audio clock may not be set correctly with initial setting. This will cause the audio playback/capture rates wrong.
This patch checks the audio clock setting and will set it to a properly value if it is not set correct.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188411
Signed-off-by: Libin Yang libin.yang@intel.com
include/sound/hda_register.h | 12 +++-- sound/hda/ext/hdac_ext_controller.c | 6 +-- sound/pci/hda/hda_intel.c | 91 +++++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 6 deletions(-)
diff --git a/include/sound/hda_register.h b/include/sound/hda_register.h index 0013063..7ea16cb 100644 --- a/include/sound/hda_register.h +++ b/include/sound/hda_register.h @@ -227,6 +227,8 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 }; #define AZX_REG_PPLCLLPU 0xC
/* registers for Multiple Links Capability Structure */ +/* Multiple Links Capability */ +#define AZX_REG_ML_CAP_BASE 0xc00 #define AZX_ML_CAP_ID 0x2 #define AZX_REG_ML_MLCH 0x00 #define AZX_REG_ML_MLCD 0x04 @@ -243,9 +245,13 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 }; #define AZX_REG_ML_LOUTPAY 0x20 #define AZX_REG_ML_LINPAY 0x30
-#define AZX_MLCTL_SPA (1<<16) -#define AZX_MLCTL_CPA 23
+#define AZX_REG_ML_LCAPx(x) (AZX_REG_ML_CAP_BASE + (0x40 + 0x40 * x)) +#define AZX_REG_ML_LCTLx(x) (AZX_REG_ML_CAP_BASE + (0x44 + 0x40 * x)) +#define ML_LCTL_SCF_MASK 0xF +#define AZX_MLCTL_SPA (0x1 << 16) +#define AZX_MLCTL_CPA (0x1 << 23) +#define AZX_MLCTL_SPA_SHIFT 16 +#define AZX_MLCTL_CPA_SHIFT 23
/* registers for DMA Resume Capability Structure */ #define AZX_DRSM_CAP_ID 0x5 diff --git a/sound/hda/ext/hdac_ext_controller.c b/sound/hda/ext/hdac_ext_controller.c index 2614691..84f3b81 100644 --- a/sound/hda/ext/hdac_ext_controller.c +++ b/sound/hda/ext/hdac_ext_controller.c @@ -171,7 +171,7 @@ static int check_hdac_link_power_active(struct hdac_ext_link *link, bool enable) { int timeout; u32 val;
- int mask = (1 << AZX_MLCTL_CPA);
int mask = (1 << AZX_MLCTL_CPA_SHIFT);
udelay(3); timeout = 150;
@@ -179,10 +179,10 @@ static int check_hdac_link_power_active(struct hdac_ext_link *link, bool enable) do { val = readl(link->ml_addr + AZX_REG_ML_LCTL); if (enable) {
if (((val & mask) >> AZX_MLCTL_CPA))
if (((val & mask) >> AZX_MLCTL_CPA_SHIFT)) return 0;
if (!((val & mask) >> AZX_MLCTL_CPA))
if (!((val & mask) >> AZX_MLCTL_CPA_SHIFT)) return 0;
These changes are rather to fix the inconsistencies between CPA and SPA register definitions. Better to split as a preliminary cleanup patch (i.e. define AZX_MLCTL_SPA, *_CPA and *_SHIFT properly and adapt to them).
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index c8256a8..017f64f 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -539,6 +539,94 @@ static void bxt_reduce_dma_latency(struct azx *chip) azx_writel(chip, SKL_EM4L, val); }
+/*
- ML_LCAP bits:
- bit 0: 6 MHz Supported
- bit 1: 12 MHz Supported
- bit 2: 24 MHz Supported
- bit 3: 48 MHz Supported
- bit 4: 96 MHz Supported
- bit 5: 192 MHz Supported
- */
+static int intel_get_lctl_scf(struct azx *chip) +{
- u32 val;
- val = azx_readl(chip, ML_LCAPx(0));
- if (val & (1 << 2))
return 2;- else if (val & (1 << 3))
return 3;- else if (val & (1 << 1))
return 1;- else if (val & (1 << 4))
return 4;- else if (val & (1 << 5))
return 5;
I guess a loop is cleaner and error-prone, e.g.:
static int preferred_bits[] = { 2, 3, 1, 4, 5 }; int i;
for (i = 0; i < ARRAY_SIZE(preferred_bits); i++) if (val & (1 << i)) return i; return 0;
- dev_warn(chip->card->dev, "set audio clock frequency to 6MHz");
- return 0;
+}
+static void intel_init_lctl(struct azx *chip) +{
- u32 val;
- int timeout;
- /* 0. check lctl register value is correct or not */
- /* the codecs are sharing the first link setting by default */
- val = azx_readl(chip, ML_LCTLx(0));
- /* if SCF is already set, let's use it */
- if ((val & ML_LCTL_SCF_MASK) != 0)
return;- /*
* Before operatiing on SPA, CPA must match SPA.
operating.
* Any deviation may result in undefined behavior.*/- if (((val & AZX_MLCTL_SPA) >> AZX_MLCTL_SPA_SHIFT) ^
((val & AZX_MLCTL_CPA) >> AZX_MLCTL_CPA_SHIFT))
Should it be better with "==" instead of XOR here?
return;- /* 1. turn link down: set SPA to 0 and wait CPA to 0 */
- val = azx_readl(chip, ML_LCTLx(0));
- val &= ~AZX_MLCTL_SPA;
- azx_writel(chip, ML_LCTLx(0), val);
- /* wait for CPA */
- timeout = 50;
- while (timeout) {
if ((azx_readl(chip, ML_LCTLx(0)) & AZX_MLCTL_CPA) == 0)break;timeout--;udelay(10);- }
- if (!timeout)
goto SET_SPA;- /* need add 100ns delay for hardware ready */
- udelay(100);
- /* 2. update SCF to select a properly audio clock*/
- val &= ~ML_LCTL_SCF_MASK;
- val |= intel_get_lctl_scf(chip);
- azx_writel(chip, ML_LCTLx(0), val);
+SET_SPA:
Use lower letters for a label.
- /* 4. turn link up: set SPA to 1 and wait CPA to 1 */
- val = azx_readl(chip, ML_LCTLx(0));
- val |= AZX_MLCTL_SPA;
- azx_writel(chip, ML_LCTLx(0), val);
- timeout = 50;
- while (timeout) {
if ((azx_readl(chip, ML_LCTLx(0)) & AZX_MLCTL_CPA) != 0)break;timeout--;udelay(10);- }
- /* need add 100ns delay for hardware ready */
- udelay(100);
Well, both clearing and setting SPA are almost the same procedure, so better to factor out a small function for that, IMO.
thanks,
Takashi
Hi Takashi,
Thanks for review and please see my comments inline.
I'm OOO currently and I will send the updated one next week.
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, March 20, 2017 5:51 PM To: Yang, Libin libin.yang@intel.com Cc: alsa-devel@alsa-project.org; Lin, Mengdong mengdong.lin@intel.com; infernix@infernix.net Subject: Re: [alsa-devel] [PATCH] ALSA: hda - set intel audio clock to a properly value
On Tue, 07 Mar 2017 07:20:22 +0100, libin.yang@intel.com wrote:
From: Libin Yang libin.yang@intel.com
On some Intel platforms, the audio clock may not be set correctly with initial setting. This will cause the audio playback/capture rates wrong.
This patch checks the audio clock setting and will set it to a properly value if it is not set correct.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188411
Signed-off-by: Libin Yang libin.yang@intel.com
include/sound/hda_register.h | 12 +++-- sound/hda/ext/hdac_ext_controller.c | 6 +-- sound/pci/hda/hda_intel.c | 91
+++++++++++++++++++++++++++++++++++++
3 files changed, 103 insertions(+), 6 deletions(-)
diff --git a/include/sound/hda_register.h b/include/sound/hda_register.h index 0013063..7ea16cb 100644 --- a/include/sound/hda_register.h +++ b/include/sound/hda_register.h @@ -227,6 +227,8 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2,
SDO3 };
#define AZX_REG_PPLCLLPU 0xC
/* registers for Multiple Links Capability Structure */ +/* Multiple Links Capability */ +#define AZX_REG_ML_CAP_BASE 0xc00 #define AZX_ML_CAP_ID 0x2 #define AZX_REG_ML_MLCH 0x00 #define AZX_REG_ML_MLCD 0x04 @@ -243,9 +245,13 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2,
SDO3 };
#define AZX_REG_ML_LOUTPAY 0x20 #define AZX_REG_ML_LINPAY 0x30
-#define AZX_MLCTL_SPA (1<<16) -#define AZX_MLCTL_CPA 23
+#define AZX_REG_ML_LCAPx(x) (AZX_REG_ML_CAP_BASE + (0x40 +
0x40 * x))
+#define AZX_REG_ML_LCTLx(x) (AZX_REG_ML_CAP_BASE + (0x44 +
0x40 * x))
+#define ML_LCTL_SCF_MASK 0xF +#define AZX_MLCTL_SPA (0x1 << 16) +#define AZX_MLCTL_CPA (0x1 << 23) +#define AZX_MLCTL_SPA_SHIFT 16 +#define AZX_MLCTL_CPA_SHIFT 23
/* registers for DMA Resume Capability Structure */ #define AZX_DRSM_CAP_ID 0x5 diff --git a/sound/hda/ext/hdac_ext_controller.c b/sound/hda/ext/hdac_ext_controller.c index 2614691..84f3b81 100644 --- a/sound/hda/ext/hdac_ext_controller.c +++ b/sound/hda/ext/hdac_ext_controller.c @@ -171,7 +171,7 @@ static int check_hdac_link_power_active(struct hdac_ext_link *link, bool enable) { int timeout; u32 val;
- int mask = (1 << AZX_MLCTL_CPA);
int mask = (1 << AZX_MLCTL_CPA_SHIFT);
udelay(3); timeout = 150;
@@ -179,10 +179,10 @@ static int check_hdac_link_power_active(struct
hdac_ext_link *link, bool enable)
do { val = readl(link->ml_addr + AZX_REG_ML_LCTL); if (enable) {
if (((val & mask) >> AZX_MLCTL_CPA))
if (((val & mask) >> AZX_MLCTL_CPA_SHIFT)) return 0;
if (!((val & mask) >> AZX_MLCTL_CPA))
if (!((val & mask) >> AZX_MLCTL_CPA_SHIFT)) return 0;These changes are rather to fix the inconsistencies between CPA and SPA register definitions. Better to split as a preliminary cleanup patch (i.e. define AZX_MLCTL_SPA, *_CPA and *_SHIFT properly and adapt to them).
Yes. I will do it.
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index c8256a8..017f64f 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -539,6 +539,94 @@ static void bxt_reduce_dma_latency(struct azx
*chip)
azx_writel(chip, SKL_EM4L, val); }
+/*
- ML_LCAP bits:
- bit 0: 6 MHz Supported
- bit 1: 12 MHz Supported
- bit 2: 24 MHz Supported
- bit 3: 48 MHz Supported
- bit 4: 96 MHz Supported
- bit 5: 192 MHz Supported
- */
+static int intel_get_lctl_scf(struct azx *chip) {
- u32 val;
- val = azx_readl(chip, ML_LCAPx(0));
- if (val & (1 << 2))
return 2;- else if (val & (1 << 3))
return 3;- else if (val & (1 << 1))
return 1;- else if (val & (1 << 4))
return 4;- else if (val & (1 << 5))
return 5;I guess a loop is cleaner and error-prone, e.g.:
static int preferred_bits[] = { 2, 3, 1, 4, 5 }; int i;
for (i = 0; i < ARRAY_SIZE(preferred_bits); i++) if (val & (1 << i)) return i;
return 0;
OK. I will do it.
- dev_warn(chip->card->dev, "set audio clock frequency to 6MHz");
- return 0;
+}
+static void intel_init_lctl(struct azx *chip) {
- u32 val;
- int timeout;
- /* 0. check lctl register value is correct or not */
- /* the codecs are sharing the first link setting by default */
- val = azx_readl(chip, ML_LCTLx(0));
- /* if SCF is already set, let's use it */
- if ((val & ML_LCTL_SCF_MASK) != 0)
return;- /*
* Before operatiing on SPA, CPA must match SPA.operating.
Yes, my typo.
* Any deviation may result in undefined behavior.*/- if (((val & AZX_MLCTL_SPA) >> AZX_MLCTL_SPA_SHIFT) ^
((val & AZX_MLCTL_CPA) >> AZX_MLCTL_CPA_SHIFT))Should it be better with "==" instead of XOR here?
AZX_MLCTL_SPA bit value should be the same with AZX_MLCTL_CPA bit value before operation. If they are different, we should not touch this register. So if XOR is true, we should return directly.
return;- /* 1. turn link down: set SPA to 0 and wait CPA to 0 */
- val = azx_readl(chip, ML_LCTLx(0));
- val &= ~AZX_MLCTL_SPA;
- azx_writel(chip, ML_LCTLx(0), val);
- /* wait for CPA */
- timeout = 50;
- while (timeout) {
if ((azx_readl(chip, ML_LCTLx(0)) & AZX_MLCTL_CPA) == 0)break;timeout--;udelay(10);- }
- if (!timeout)
goto SET_SPA;- /* need add 100ns delay for hardware ready */
- udelay(100);
- /* 2. update SCF to select a properly audio clock*/
- val &= ~ML_LCTL_SCF_MASK;
- val |= intel_get_lctl_scf(chip);
- azx_writel(chip, ML_LCTLx(0), val);
+SET_SPA:
Use lower letters for a label.
OK.
- /* 4. turn link up: set SPA to 1 and wait CPA to 1 */
- val = azx_readl(chip, ML_LCTLx(0));
- val |= AZX_MLCTL_SPA;
- azx_writel(chip, ML_LCTLx(0), val);
- timeout = 50;
- while (timeout) {
if ((azx_readl(chip, ML_LCTLx(0)) & AZX_MLCTL_CPA) != 0)break;timeout--;udelay(10);- }
- /* need add 100ns delay for hardware ready */
- udelay(100);
Well, both clearing and setting SPA are almost the same procedure, so better to factor out a small function for that, IMO.
OK, I will do it. Thanks for suggestion
Best Regards, Libin
thanks,
Takashi
Hi Libin,
One generic comment.
This patch uses the enhanced capability structure and so instead of using IS_SKL_PLUS() to check the capability it is better to check the ppcap. All the SKL platforms may not be having the enhanced capability structure.
Regards, Rakesh
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa- project.org] On Behalf Of Yang, Libin Sent: Tuesday, March 21, 2017 9:11 AM To: Takashi Iwai tiwai@suse.de Cc: Lin, Mengdong mengdong.lin@intel.com; alsa-devel@alsa-project.org; infernix@infernix.net Subject: Re: [alsa-devel] [PATCH] ALSA: hda - set intel audio clock to a properly value
Hi Takashi,
Thanks for review and please see my comments inline.
I'm OOO currently and I will send the updated one next week.
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, March 20, 2017 5:51 PM To: Yang, Libin libin.yang@intel.com Cc: alsa-devel@alsa-project.org; Lin, Mengdong mengdong.lin@intel.com; infernix@infernix.net Subject: Re: [alsa-devel] [PATCH] ALSA: hda - set intel audio clock to a
properly
value
On Tue, 07 Mar 2017 07:20:22 +0100, libin.yang@intel.com wrote:
From: Libin Yang libin.yang@intel.com
On some Intel platforms, the audio clock may not be set correctly with initial setting. This will cause the audio playback/capture rates wrong.
This patch checks the audio clock setting and will set it to a properly value if it is not set correct.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188411
Signed-off-by: Libin Yang libin.yang@intel.com
include/sound/hda_register.h | 12 +++-- sound/hda/ext/hdac_ext_controller.c | 6 +-- sound/pci/hda/hda_intel.c | 91
+++++++++++++++++++++++++++++++++++++
3 files changed, 103 insertions(+), 6 deletions(-)
diff --git a/include/sound/hda_register.h b/include/sound/hda_register.h index 0013063..7ea16cb 100644 --- a/include/sound/hda_register.h +++ b/include/sound/hda_register.h @@ -227,6 +227,8 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2,
SDO3 };
#define AZX_REG_PPLCLLPU 0xC
/* registers for Multiple Links Capability Structure */ +/* Multiple Links Capability */ +#define AZX_REG_ML_CAP_BASE 0xc00 #define AZX_ML_CAP_ID 0x2 #define AZX_REG_ML_MLCH 0x00 #define AZX_REG_ML_MLCD 0x04 @@ -243,9 +245,13 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1,
SDO2,
SDO3 };
#define AZX_REG_ML_LOUTPAY 0x20 #define AZX_REG_ML_LINPAY 0x30
-#define AZX_MLCTL_SPA (1<<16) -#define AZX_MLCTL_CPA 23
+#define AZX_REG_ML_LCAPx(x) (AZX_REG_ML_CAP_BASE + (0x40 +
0x40 * x))
+#define AZX_REG_ML_LCTLx(x) (AZX_REG_ML_CAP_BASE + (0x44 +
0x40 * x))
+#define ML_LCTL_SCF_MASK 0xF +#define AZX_MLCTL_SPA (0x1 << 16) +#define AZX_MLCTL_CPA (0x1 << 23) +#define AZX_MLCTL_SPA_SHIFT 16 +#define AZX_MLCTL_CPA_SHIFT 23
/* registers for DMA Resume Capability Structure */ #define AZX_DRSM_CAP_ID 0x5 diff --git a/sound/hda/ext/hdac_ext_controller.c b/sound/hda/ext/hdac_ext_controller.c index 2614691..84f3b81 100644 --- a/sound/hda/ext/hdac_ext_controller.c +++ b/sound/hda/ext/hdac_ext_controller.c @@ -171,7 +171,7 @@ static int check_hdac_link_power_active(struct hdac_ext_link *link, bool enable) { int timeout; u32 val;
- int mask = (1 << AZX_MLCTL_CPA);
int mask = (1 << AZX_MLCTL_CPA_SHIFT);
udelay(3); timeout = 150;
@@ -179,10 +179,10 @@ static int check_hdac_link_power_active(struct
hdac_ext_link *link, bool enable)
do { val = readl(link->ml_addr + AZX_REG_ML_LCTL); if (enable) {
if (((val & mask) >> AZX_MLCTL_CPA))
if (((val & mask) >> AZX_MLCTL_CPA_SHIFT)) return 0;
if (!((val & mask) >> AZX_MLCTL_CPA))
if (!((val & mask) >> AZX_MLCTL_CPA_SHIFT)) return 0;These changes are rather to fix the inconsistencies between CPA and SPA register definitions. Better to split as a preliminary cleanup patch (i.e. define AZX_MLCTL_SPA, *_CPA and *_SHIFT properly and adapt to them).
Yes. I will do it.
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index c8256a8..017f64f 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -539,6 +539,94 @@ static void bxt_reduce_dma_latency(struct azx
*chip)
azx_writel(chip, SKL_EM4L, val); }
+/*
- ML_LCAP bits:
- bit 0: 6 MHz Supported
- bit 1: 12 MHz Supported
- bit 2: 24 MHz Supported
- bit 3: 48 MHz Supported
- bit 4: 96 MHz Supported
- bit 5: 192 MHz Supported
- */
+static int intel_get_lctl_scf(struct azx *chip) {
- u32 val;
- val = azx_readl(chip, ML_LCAPx(0));
- if (val & (1 << 2))
return 2;- else if (val & (1 << 3))
return 3;- else if (val & (1 << 1))
return 1;- else if (val & (1 << 4))
return 4;- else if (val & (1 << 5))
return 5;I guess a loop is cleaner and error-prone, e.g.:
static int preferred_bits[] = { 2, 3, 1, 4, 5 }; int i;
for (i = 0; i < ARRAY_SIZE(preferred_bits); i++) if (val & (1 << i)) return i;
return 0;
OK. I will do it.
- dev_warn(chip->card->dev, "set audio clock frequency to 6MHz");
- return 0;
+}
+static void intel_init_lctl(struct azx *chip) {
- u32 val;
- int timeout;
- /* 0. check lctl register value is correct or not */
- /* the codecs are sharing the first link setting by default */
- val = azx_readl(chip, ML_LCTLx(0));
- /* if SCF is already set, let's use it */
- if ((val & ML_LCTL_SCF_MASK) != 0)
return;- /*
* Before operatiing on SPA, CPA must match SPA.operating.
Yes, my typo.
* Any deviation may result in undefined behavior.*/- if (((val & AZX_MLCTL_SPA) >> AZX_MLCTL_SPA_SHIFT) ^
((val & AZX_MLCTL_CPA) >> AZX_MLCTL_CPA_SHIFT))Should it be better with "==" instead of XOR here?
AZX_MLCTL_SPA bit value should be the same with AZX_MLCTL_CPA bit value before operation. If they are different, we should not touch this register. So if XOR is true, we should return directly.
return;- /* 1. turn link down: set SPA to 0 and wait CPA to 0 */
- val = azx_readl(chip, ML_LCTLx(0));
- val &= ~AZX_MLCTL_SPA;
- azx_writel(chip, ML_LCTLx(0), val);
- /* wait for CPA */
- timeout = 50;
- while (timeout) {
if ((azx_readl(chip, ML_LCTLx(0)) & AZX_MLCTL_CPA) == 0)break;timeout--;udelay(10);- }
- if (!timeout)
goto SET_SPA;- /* need add 100ns delay for hardware ready */
- udelay(100);
- /* 2. update SCF to select a properly audio clock*/
- val &= ~ML_LCTL_SCF_MASK;
- val |= intel_get_lctl_scf(chip);
- azx_writel(chip, ML_LCTLx(0), val);
+SET_SPA:
Use lower letters for a label.
OK.
- /* 4. turn link up: set SPA to 1 and wait CPA to 1 */
- val = azx_readl(chip, ML_LCTLx(0));
- val |= AZX_MLCTL_SPA;
- azx_writel(chip, ML_LCTLx(0), val);
- timeout = 50;
- while (timeout) {
if ((azx_readl(chip, ML_LCTLx(0)) & AZX_MLCTL_CPA) != 0)break;timeout--;udelay(10);- }
- /* need add 100ns delay for hardware ready */
- udelay(100);
Well, both clearing and setting SPA are almost the same procedure, so better to factor out a small function for that, IMO.
OK, I will do it. Thanks for suggestion
Best Regards, Libin
thanks,
Takashi
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
HI Rakesh,
Get it. Thanks. I will check it.
Regards, Libin
-----Original Message----- From: Ughreja, Rakesh A Sent: Wednesday, March 22, 2017 12:35 AM To: Yang, Libin libin.yang@intel.com; Takashi Iwai tiwai@suse.de Cc: Lin, Mengdong mengdong.lin@intel.com; alsa-devel@alsa-project.org; infernix@infernix.net Subject: RE: [alsa-devel] [PATCH] ALSA: hda - set intel audio clock to a properly value
Hi Libin,
One generic comment.
This patch uses the enhanced capability structure and so instead of using IS_SKL_PLUS() to check the capability it is better to check the ppcap. All the SKL platforms may not be having the enhanced capability structure.
Regards, Rakesh
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa- project.org] On Behalf Of Yang, Libin Sent: Tuesday, March 21, 2017 9:11 AM To: Takashi Iwai tiwai@suse.de Cc: Lin, Mengdong mengdong.lin@intel.com; alsa-devel@alsa-project.org; infernix@infernix.net Subject: Re: [alsa-devel] [PATCH] ALSA: hda - set intel audio clock to a properly value
Hi Takashi,
Thanks for review and please see my comments inline.
I'm OOO currently and I will send the updated one next week.
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, March 20, 2017 5:51 PM To: Yang, Libin libin.yang@intel.com Cc: alsa-devel@alsa-project.org; Lin, Mengdong mengdong.lin@intel.com; infernix@infernix.net Subject: Re: [alsa-devel] [PATCH] ALSA: hda - set intel audio clock to a
properly
value
On Tue, 07 Mar 2017 07:20:22 +0100, libin.yang@intel.com wrote:
From: Libin Yang libin.yang@intel.com
On some Intel platforms, the audio clock may not be set correctly with initial setting. This will cause the audio playback/capture rates wrong.
This patch checks the audio clock setting and will set it to a properly value if it is not set correct.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188411
Signed-off-by: Libin Yang libin.yang@intel.com
include/sound/hda_register.h | 12 +++-- sound/hda/ext/hdac_ext_controller.c | 6 +-- sound/pci/hda/hda_intel.c | 91
+++++++++++++++++++++++++++++++++++++
3 files changed, 103 insertions(+), 6 deletions(-)
diff --git a/include/sound/hda_register.h b/include/sound/hda_register.h index 0013063..7ea16cb 100644 --- a/include/sound/hda_register.h +++ b/include/sound/hda_register.h @@ -227,6 +227,8 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2,
SDO3 };
#define AZX_REG_PPLCLLPU 0xC
/* registers for Multiple Links Capability Structure */ +/* Multiple Links Capability */ +#define AZX_REG_ML_CAP_BASE 0xc00 #define AZX_ML_CAP_ID 0x2 #define AZX_REG_ML_MLCH 0x00 #define AZX_REG_ML_MLCD 0x04 @@ -243,9 +245,13 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1,
SDO2,
SDO3 };
#define AZX_REG_ML_LOUTPAY 0x20 #define AZX_REG_ML_LINPAY 0x30
-#define AZX_MLCTL_SPA (1<<16) -#define AZX_MLCTL_CPA 23
+#define AZX_REG_ML_LCAPx(x) (AZX_REG_ML_CAP_BASE + (0x40 +
0x40 * x))
+#define AZX_REG_ML_LCTLx(x) (AZX_REG_ML_CAP_BASE + (0x44 +
0x40 * x))
+#define ML_LCTL_SCF_MASK 0xF +#define AZX_MLCTL_SPA (0x1 << 16) +#define AZX_MLCTL_CPA (0x1 << 23) +#define AZX_MLCTL_SPA_SHIFT 16 +#define AZX_MLCTL_CPA_SHIFT 23
/* registers for DMA Resume Capability Structure */ #define AZX_DRSM_CAP_ID 0x5 diff --git a/sound/hda/ext/hdac_ext_controller.c b/sound/hda/ext/hdac_ext_controller.c index 2614691..84f3b81 100644 --- a/sound/hda/ext/hdac_ext_controller.c +++ b/sound/hda/ext/hdac_ext_controller.c @@ -171,7 +171,7 @@ static int check_hdac_link_power_active(struct hdac_ext_link *link, bool enable) { int timeout; u32 val;
- int mask = (1 << AZX_MLCTL_CPA);
int mask = (1 << AZX_MLCTL_CPA_SHIFT);
udelay(3); timeout = 150;
@@ -179,10 +179,10 @@ static int check_hdac_link_power_active(struct
hdac_ext_link *link, bool enable)
do { val = readl(link->ml_addr + AZX_REG_ML_LCTL); if (enable) {
if (((val & mask) >> AZX_MLCTL_CPA))
if (((val & mask) >> AZX_MLCTL_CPA_SHIFT)) return 0;
if (!((val & mask) >> AZX_MLCTL_CPA))
if (!((val & mask) >> AZX_MLCTL_CPA_SHIFT)) return 0;These changes are rather to fix the inconsistencies between CPA and SPA register definitions. Better to split as a preliminary cleanup patch (i.e. define AZX_MLCTL_SPA, *_CPA and *_SHIFT properly and adapt
to them).
Yes. I will do it.
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index c8256a8..017f64f 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -539,6 +539,94 @@ static void bxt_reduce_dma_latency(struct azx
*chip)
azx_writel(chip, SKL_EM4L, val); }
+/*
- ML_LCAP bits:
- bit 0: 6 MHz Supported
- bit 1: 12 MHz Supported
- bit 2: 24 MHz Supported
- bit 3: 48 MHz Supported
- bit 4: 96 MHz Supported
- bit 5: 192 MHz Supported
- */
+static int intel_get_lctl_scf(struct azx *chip) {
- u32 val;
- val = azx_readl(chip, ML_LCAPx(0));
- if (val & (1 << 2))
return 2;- else if (val & (1 << 3))
return 3;- else if (val & (1 << 1))
return 1;- else if (val & (1 << 4))
return 4;- else if (val & (1 << 5))
return 5;I guess a loop is cleaner and error-prone, e.g.:
static int preferred_bits[] = { 2, 3, 1, 4, 5 }; int i;
for (i = 0; i < ARRAY_SIZE(preferred_bits); i++) if (val & (1 << i)) return i;
return 0;
OK. I will do it.
- dev_warn(chip->card->dev, "set audio clock frequency to 6MHz");
- return 0;
+}
+static void intel_init_lctl(struct azx *chip) {
- u32 val;
- int timeout;
- /* 0. check lctl register value is correct or not */
- /* the codecs are sharing the first link setting by default */
- val = azx_readl(chip, ML_LCTLx(0));
- /* if SCF is already set, let's use it */
- if ((val & ML_LCTL_SCF_MASK) != 0)
return;- /*
* Before operatiing on SPA, CPA must match SPA.operating.
Yes, my typo.
* Any deviation may result in undefined behavior.*/- if (((val & AZX_MLCTL_SPA) >> AZX_MLCTL_SPA_SHIFT) ^
((val & AZX_MLCTL_CPA) >> AZX_MLCTL_CPA_SHIFT))Should it be better with "==" instead of XOR here?
AZX_MLCTL_SPA bit value should be the same with AZX_MLCTL_CPA bit value before operation. If they are different, we should not touch this register. So if XOR is true, we should return directly.
return;- /* 1. turn link down: set SPA to 0 and wait CPA to 0 */
- val = azx_readl(chip, ML_LCTLx(0));
- val &= ~AZX_MLCTL_SPA;
- azx_writel(chip, ML_LCTLx(0), val);
- /* wait for CPA */
- timeout = 50;
- while (timeout) {
if ((azx_readl(chip, ML_LCTLx(0)) & AZX_MLCTL_CPA) == 0)break;timeout--;udelay(10);- }
- if (!timeout)
goto SET_SPA;- /* need add 100ns delay for hardware ready */
- udelay(100);
- /* 2. update SCF to select a properly audio clock*/
- val &= ~ML_LCTL_SCF_MASK;
- val |= intel_get_lctl_scf(chip);
- azx_writel(chip, ML_LCTLx(0), val);
+SET_SPA:
Use lower letters for a label.
OK.
- /* 4. turn link up: set SPA to 1 and wait CPA to 1 */
- val = azx_readl(chip, ML_LCTLx(0));
- val |= AZX_MLCTL_SPA;
- azx_writel(chip, ML_LCTLx(0), val);
- timeout = 50;
- while (timeout) {
if ((azx_readl(chip, ML_LCTLx(0)) & AZX_MLCTL_CPA) != 0)break;timeout--;udelay(10);- }
- /* need add 100ns delay for hardware ready */
- udelay(100);
Well, both clearing and setting SPA are almost the same procedure, so better to factor out a small function for that, IMO.
OK, I will do it. Thanks for suggestion
Best Regards, Libin
thanks,
Takashi
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Tue, 21 Mar 2017 17:11:27 +0100, Yang, Libin wrote:
* Any deviation may result in undefined behavior.*/- if (((val & AZX_MLCTL_SPA) >> AZX_MLCTL_SPA_SHIFT) ^
((val & AZX_MLCTL_CPA) >> AZX_MLCTL_CPA_SHIFT))Should it be better with "==" instead of XOR here?
AZX_MLCTL_SPA bit value should be the same with AZX_MLCTL_CPA bit value before operation. If they are different, we should not touch this register. So if XOR is true, we should return directly.
I meant that (A != B) is more understandable than (A ^ B).
Takashi
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, March 22, 2017 12:51 AM To: Yang, Libin libin.yang@intel.com Cc: alsa-devel@alsa-project.org; Lin, Mengdong mengdong.lin@intel.com; infernix@infernix.net Subject: Re: [alsa-devel] [PATCH] ALSA: hda - set intel audio clock to a properly value
On Tue, 21 Mar 2017 17:11:27 +0100, Yang, Libin wrote:
* Any deviation may result in undefined behavior.*/- if (((val & AZX_MLCTL_SPA) >> AZX_MLCTL_SPA_SHIFT) ^
((val & AZX_MLCTL_CPA) >> AZX_MLCTL_CPA_SHIFT))Should it be better with "==" instead of XOR here?
AZX_MLCTL_SPA bit value should be the same with AZX_MLCTL_CPA bit value before operation. If they are different, we should not touch this register. So if XOR is true, we should return directly.
I meant that (A != B) is more understandable than (A ^ B).
Get it. I will use (A != B)
Regards, Libin
Takashi
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel- bounces@alsa-project.org] On Behalf Of Yang, Libin Sent: Tuesday, March 7, 2017 11:50 AM To: alsa-devel@alsa-project.org; tiwai@suse.de Cc: Yang, Libin libin.yang@intel.com; Lin, Mengdong mengdong.lin@intel.com; infernix@infernix.net Subject: [alsa-devel] [PATCH] ALSA: hda - set intel audio clock to a properly value
From: Libin Yang libin.yang@intel.com
On some Intel platforms, the audio clock may not be set correctly with initial setting. This will cause the audio playback/capture rates wrong.
This patch checks the audio clock setting and will set it to a properly value if it is not set correct.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188411
Signed-off-by: Libin Yang libin.yang@intel.com
include/sound/hda_register.h | 12 +++-- sound/hda/ext/hdac_ext_controller.c | 6 +-- sound/pci/hda/hda_intel.c | 91 +++++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 6 deletions(-)
diff --git a/include/sound/hda_register.h b/include/sound/hda_register.h index 0013063..7ea16cb 100644 --- a/include/sound/hda_register.h +++ b/include/sound/hda_register.h @@ -227,6 +227,8 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 }; #define AZX_REG_PPLCLLPU 0xC
/* registers for Multiple Links Capability Structure */ +/* Multiple Links Capability */ +#define AZX_REG_ML_CAP_BASE 0xc00
Base is already available as part of bus, use bus->mlcap
+#define AZX_REG_ML_LCAPx(x) (AZX_REG_ML_CAP_BASE + (0x40 + 0x40 * x)) +#define AZX_REG_ML_LCTLx(x) (AZX_REG_ML_CAP_BASE + (0x44 + 0x40 * x))
This is specific to the had legacy driver as only link0 is used, I think this can be moved to driver As define. If it required to use all the link, then need to use ext functions to initialize the address per link.
+#define ML_LCTL_SCF_MASK 0xF +#define AZX_MLCTL_SPA (0x1 << 16) +#define AZX_MLCTL_CPA (0x1 << 23) +#define AZX_MLCTL_SPA_SHIFT 16 +#define AZX_MLCTL_CPA_SHIFT 23
static void hda_intel_init_chip(struct azx *chip, bool full_reset) { struct hdac_bus *bus = azx_bus(chip); @@ -564,6 +652,9 @@ static void hda_intel_init_chip(struct azx *chip, bool full_reset)
- if (IS_SKL_PLUS(pci))
intel_init_lctl(chip);
Use bus->mlcap to check for multilink capability is present.
Hi Jeeja,
-----Original Message----- From: Kp, Jeeja Sent: Thursday, March 23, 2017 9:11 PM To: Yang, Libin libin.yang@intel.com; alsa-devel@alsa-project.org; tiwai@suse.de Cc: Lin, Mengdong mengdong.lin@intel.com; infernix@infernix.net Subject: RE: [alsa-devel] [PATCH] ALSA: hda - set intel audio clock to a properly value
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel- bounces@alsa-project.org] On Behalf Of Yang, Libin Sent: Tuesday, March 7, 2017 11:50 AM To: alsa-devel@alsa-project.org; tiwai@suse.de Cc: Yang, Libin libin.yang@intel.com; Lin, Mengdong mengdong.lin@intel.com; infernix@infernix.net Subject: [alsa-devel] [PATCH] ALSA: hda - set intel audio clock to a properly value
From: Libin Yang libin.yang@intel.com
On some Intel platforms, the audio clock may not be set correctly with initial setting. This will cause the audio playback/capture rates wrong.
This patch checks the audio clock setting and will set it to a properly value if it is not set correct.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188411
Signed-off-by: Libin Yang libin.yang@intel.com
include/sound/hda_register.h | 12 +++-- sound/hda/ext/hdac_ext_controller.c | 6 +-- sound/pci/hda/hda_intel.c | 91 +++++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 6 deletions(-)
diff --git a/include/sound/hda_register.h b/include/sound/hda_register.h index 0013063..7ea16cb 100644 --- a/include/sound/hda_register.h +++ b/include/sound/hda_register.h @@ -227,6 +227,8 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 }; #define AZX_REG_PPLCLLPU 0xC
/* registers for Multiple Links Capability Structure */ +/* Multiple Links Capability */ +#define AZX_REG_ML_CAP_BASE 0xc00
Base is already available as part of bus, use bus->mlcap
+#define AZX_REG_ML_LCAPx(x) (AZX_REG_ML_CAP_BASE + (0x40 + 0x40 * x)) +#define AZX_REG_ML_LCTLx(x) (AZX_REG_ML_CAP_BASE + (0x44 + 0x40 * x))
This is specific to the had legacy driver as only link0 is used, I think this can be moved to driver As define. If it required to use all the link, then need to use ext functions to initialize the address per link.
I'm not very sure what's your meaning. Do you mean using +#define AZX_REG_ML_LCAP0 (AZX_REG_ML_CAP_BASE + 0x40)
+#define ML_LCTL_SCF_MASK 0xF +#define AZX_MLCTL_SPA (0x1 << 16) +#define AZX_MLCTL_CPA (0x1 << 23) +#define AZX_MLCTL_SPA_SHIFT 16 +#define AZX_MLCTL_CPA_SHIFT 23
static void hda_intel_init_chip(struct azx *chip, bool full_reset) { struct hdac_bus *bus = azx_bus(chip); @@ -564,6 +652,9 @@ static void hda_intel_init_chip(struct azx *chip, bool full_reset)
- if (IS_SKL_PLUS(pci))
intel_init_lctl(chip);Use bus->mlcap to check for multilink capability is present.
Get it.
Thanks, Libin
-----Original Message----- From: Kp, Jeeja Sent: Thursday, March 23, 2017 9:11 PM To: Yang, Libin libin.yang@intel.com; alsa-devel@alsa-project.org; tiwai@suse.de Cc: Lin, Mengdong mengdong.lin@intel.com; infernix@infernix.net Subject: RE: [alsa-devel] [PATCH] ALSA: hda - set intel audio clock to a properly value
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel- bounces@alsa-project.org] On Behalf Of Yang, Libin Sent: Tuesday, March 7, 2017 11:50 AM To: alsa-devel@alsa-project.org; tiwai@suse.de Cc: Yang, Libin libin.yang@intel.com; Lin, Mengdong mengdong.lin@intel.com; infernix@infernix.net Subject: [alsa-devel] [PATCH] ALSA: hda - set intel audio clock to a properly value
From: Libin Yang libin.yang@intel.com
On some Intel platforms, the audio clock may not be set correctly with initial setting. This will cause the audio playback/capture rates wrong.
This patch checks the audio clock setting and will set it to a properly value if it is not set correct.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188411
Signed-off-by: Libin Yang libin.yang@intel.com
include/sound/hda_register.h | 12 +++-- sound/hda/ext/hdac_ext_controller.c | 6 +-- sound/pci/hda/hda_intel.c | 91 +++++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 6 deletions(-)
diff --git a/include/sound/hda_register.h b/include/sound/hda_register.h index 0013063..7ea16cb 100644 --- a/include/sound/hda_register.h +++ b/include/sound/hda_register.h @@ -227,6 +227,8 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 }; #define AZX_REG_PPLCLLPU 0xC
/* registers for Multiple Links Capability Structure */ +/* Multiple Links Capability */ +#define AZX_REG_ML_CAP_BASE 0xc00
Base is already available as part of bus, use bus->mlcap
+#define AZX_REG_ML_LCAPx(x) (AZX_REG_ML_CAP_BASE + (0x40 + 0x40 * x)) +#define AZX_REG_ML_LCTLx(x) (AZX_REG_ML_CAP_BASE + (0x44 + 0x40 * x))
This is specific to the had legacy driver as only link0 is used, I think this can be moved to driver As define. If it required to use all the link, then need to use ext functions to initialize the address per link.
I'm not very sure what's your meaning. Do you mean using
Use bus->mlcap + 0x44 as you are always using link0 (add offset, no need to define explicitly - AZX_REG_ML_LCTLx(x)).
Hi Jeeja,
-----Original Message----- From: Kp, Jeeja Sent: Thursday, March 30, 2017 4:07 PM To: Yang, Libin libin.yang@intel.com; alsa-devel@alsa-project.org; tiwai@suse.de Cc: Lin, Mengdong mengdong.lin@intel.com; infernix@infernix.net Subject: RE: [alsa-devel] [PATCH] ALSA: hda - set intel audio clock to a properly value
-----Original Message----- From: Kp, Jeeja Sent: Thursday, March 23, 2017 9:11 PM To: Yang, Libin libin.yang@intel.com; alsa-devel@alsa-project.org; tiwai@suse.de Cc: Lin, Mengdong mengdong.lin@intel.com; infernix@infernix.net Subject: RE: [alsa-devel] [PATCH] ALSA: hda - set intel audio clock to a properly value
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel- bounces@alsa-project.org] On Behalf Of Yang, Libin Sent: Tuesday, March 7, 2017 11:50 AM To: alsa-devel@alsa-project.org; tiwai@suse.de Cc: Yang, Libin libin.yang@intel.com; Lin, Mengdong mengdong.lin@intel.com; infernix@infernix.net Subject: [alsa-devel] [PATCH] ALSA: hda - set intel audio clock to a properly value
From: Libin Yang libin.yang@intel.com
On some Intel platforms, the audio clock may not be set correctly with initial setting. This will cause the audio playback/capture rates wrong.
This patch checks the audio clock setting and will set it to a properly value if it is not set correct.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188411
Signed-off-by: Libin Yang libin.yang@intel.com
include/sound/hda_register.h | 12 +++-- sound/hda/ext/hdac_ext_controller.c | 6 +-- sound/pci/hda/hda_intel.c | 91 +++++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 6 deletions(-)
diff --git a/include/sound/hda_register.h b/include/sound/hda_register.h index 0013063..7ea16cb 100644 --- a/include/sound/hda_register.h +++ b/include/sound/hda_register.h @@ -227,6 +227,8 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 }; #define AZX_REG_PPLCLLPU 0xC
/* registers for Multiple Links Capability Structure */ +/* Multiple Links Capability */ +#define AZX_REG_ML_CAP_BASE 0xc00
Base is already available as part of bus, use bus->mlcap
+#define AZX_REG_ML_LCAPx(x) (AZX_REG_ML_CAP_BASE + (0x40 + 0x40 * x)) +#define AZX_REG_ML_LCTLx(x) (AZX_REG_ML_CAP_BASE + (0x44 + 0x40 * x))
This is specific to the had legacy driver as only link0 is used, I think this can be moved to driver As define. If it required to use all the link, then need to use ext functions to initialize the address per link.
I'm not very sure what's your meaning. Do you mean using
Use bus->mlcap + 0x44 as you are always using link0 (add offset, no need to define explicitly - AZX_REG_ML_LCTLx(x)).
OK. Thanks for suggestion. It can avoid new definition. Thanks for suggestion.
Regards, Libin
participants (5)
-
Kp, Jeeja -
libin.yang＠intel.com -
Takashi Iwai -
Ughreja, Rakesh A -
Yang, Libin