Hi Bin,
On Wed, 13 Feb 2019 at 02:39, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Jan 22, 2019 at 9:14 AM Simon Glass sjg@chromium.org wrote:
Add sound support for link, using the HDA codec implementation.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/cpu/ivybridge/Kconfig | 1 + arch/x86/cpu/ivybridge/northbridge.c | 27 ++++ arch/x86/dts/chromebook_link.dts | 91 ++++++++++++ .../include/asm/arch-ivybridge/sandybridge.h | 3 + configs/chromebook_link_defconfig | 2 + drivers/sound/Kconfig | 10 ++ drivers/sound/Makefile | 1 + drivers/sound/ivybridge_sound.c | 137 ++++++++++++++++++ 8 files changed, 272 insertions(+) create mode 100644 drivers/sound/ivybridge_sound.c
diff --git a/arch/x86/cpu/ivybridge/Kconfig b/arch/x86/cpu/ivybridge/Kconfig index 5f0e60837c..2f42393786 100644 --- a/arch/x86/cpu/ivybridge/Kconfig +++ b/arch/x86/cpu/ivybridge/Kconfig @@ -21,6 +21,7 @@ config NORTHBRIDGE_INTEL_IVYBRIDGE imply USB_EHCI_HCD imply USB_XHCI_HCD imply VIDEO_VESA
imply SOUND_IVYBRIDGEif NORTHBRIDGE_INTEL_IVYBRIDGE
diff --git a/arch/x86/cpu/ivybridge/northbridge.c b/arch/x86/cpu/ivybridge/northbridge.c index 39bab7bdf3..0ac0a34acc 100644 --- a/arch/x86/cpu/ivybridge/northbridge.c +++ b/arch/x86/cpu/ivybridge/northbridge.c @@ -177,6 +177,30 @@ static void sandybridge_setup_northbridge_bars(struct udevice *dev) dm_pci_write_config8(dev, PAM6, 0x33); }
+static void sandybridge_init_iommu(struct udevice *dev) +{
u32 capid0_a;dm_pci_read_config32(dev, 0xe4, &capid0_a);if (capid0_a & (1 << 23)) {log_debug("capid0_a not needed\n");return;}/* setup BARs */writel(IOMMU_BASE1 >> 32, MCHBAR_REG(0x5404));writel(IOMMU_BASE1 | 1, MCHBAR_REG(0x5400));writel(IOMMU_BASE2 >> 32, MCHBAR_REG(0x5414));writel(IOMMU_BASE2 | 1, MCHBAR_REG(0x5410));/* lock policies */writel(0x80000000, IOMMU_BASE1 + 0xff0);/* Enable azalia sound */writel(0x20000000, IOMMU_BASE2 + 0xff0);writel(0xa0000000, IOMMU_BASE2 + 0xff0);+}
The function name looks it has something to do with IOMMU, but why does enabling sound support need IOMMU which is mainly for hypervisor stuff? Better providing some comments here, and if possible use macros instead of hard-coded numbers.
I've looked very hard but cannot find any documentation for these registers. They seem to be in a reserved area. The coreboot patch that this code came from is at https://review.coreboot.org/c/coreboot/+/12194/2 but there are no comments.
static int bd82x6x_northbridge_early_init(struct udevice *dev) { const int chipset_type = SANDYBRIDGE_MOBILE; @@ -197,6 +221,9 @@ static int bd82x6x_northbridge_early_init(struct udevice *dev)
sandybridge_setup_northbridge_bars(dev);
/* Setup IOMMU BARs */sandybridge_init_iommu(dev);/* Device Enable */ dm_pci_write_config32(dev, DEVEN, DEVEN_HOST | DEVEN_IGD);diff --git a/arch/x86/dts/chromebook_link.dts b/arch/x86/dts/chromebook_link.dts index f9f0979730..7e0a615a60 100644 --- a/arch/x86/dts/chromebook_link.dts +++ b/arch/x86/dts/chromebook_link.dts @@ -1,6 +1,8 @@ /dts-v1/;
#include <dt-bindings/gpio/x86-gpio.h> +#include <dt-bindings/sound/azalia.h> +#include <pci_ids.h>
/include/ "skeleton.dtsi" /include/ "keyboard.dtsi" @@ -372,6 +374,30 @@ compatible = "ehci-pci"; };
hda@1b,0 {reg = <0x0000d800 0 0 0 0>;compatible = "intel,bd82x6x-hda";beep-verbs = <0x00170500 /* power up codec */0x00270500 /* power up DAC */0x00b70500 /* power up speaker */0x00b70740 /* enable speaker out */0x00b78d00 /* enable EAPD pin */0x00b70c02 /* set EAPD pin */0x0143b013>; /* beep volume */Are these verbs from the chipset datasheet?
They are actually using the HD audio spec. In the first one 705 is to set the power state and 1 is the node ID.
It would be possible to change these to defines but it is quite laborious and I'm not sure how useful it is. I could use the macros to separate out the fields if you think that would help a lot. But people still need to look up the spec to understand these values.
I'll add a comment mentioning the specification again.
codecs {creative_codec: creative-ca0132 {vendor-id = <PCI_VENDOR_ID_CREATIVE>;device-id = <PCI_DEVICE_ID_CREATIVE_CA01322>;};intel_hdmi: hdmi {vendor-id = <PCI_VENDOR_ID_INTEL>;device-id = <PCI_DEVICE_ID_INTEL_COUGARPOINT_HDMI>;};};};usb_0: usb@1d,0 { reg = <0x0000e800 0 0 0 0>; compatible = "ehci-pci";@@ -492,3 +518,68 @@ };
};
+&creative_codec {
verbs = </* Malcolm Setup */0x01570d09 0x01570c23 0x01570a01 0x01570df00x01570efe 0x01570775 0x015707d3 0x015707090x01570753 0x015707d4 0x015707ef 0x015707750x015707d3 0x01570709 0x01570702 0x015707370x01570778 0x01553cce 0x015575c9 0x01553dce0x0155b7c9 0x01570de8 0x01570efe 0x015707020x01570768 0x01570762 0x01553ace 0x015546c90x01553bce 0x0155e8c9 0x01570d49 0x01570c880x01570d20 0x01570e19 0x01570700 0x01571a050x01571b29 0x01571a04 0x01571b29 0x01570a01And here? Is this something one can easily get from the chipset datasheet? If not, better adding some comments to point out the sources of these magic numbers.
Again I'll add a comment. Let me know what you think.
[..]
Regards, Simon