Hi Bin,
On Wed, 13 Feb 2019 at 02:39, Bin Meng <bmeng...@gmail.com> wrote:
>
> Hi Simon,
>
> On Tue, Jan 22, 2019 at 9:14 AM Simon Glass <s...@chromium.org> wrote:
> >
> > Add sound support for link, using the HDA codec implementation.
> >
> > Signed-off-by: Simon Glass <s...@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_IVYBRIDGE
> >
> > if 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 0x01570df0
> > + 0x01570efe 0x01570775 0x015707d3 0x01570709
> > + 0x01570753 0x015707d4 0x015707ef 0x01570775
> > + 0x015707d3 0x01570709 0x01570702 0x01570737
> > + 0x01570778 0x01553cce 0x015575c9 0x01553dce
> > + 0x0155b7c9 0x01570de8 0x01570efe 0x01570702
> > + 0x01570768 0x01570762 0x01553ace 0x015546c9
> > + 0x01553bce 0x0155e8c9 0x01570d49 0x01570c88
> > + 0x01570d20 0x01570e19 0x01570700 0x01571a05
> > + 0x01571b29 0x01571a04 0x01571b29 0x01570a01
> > +
>
> And 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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
