Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34531 )
Change subject: mb/purism/librem_skl: use SOC_INTEL_COMMON_BLOCK_HDA_VERB ......................................................................
mb/purism/librem_skl: use SOC_INTEL_COMMON_BLOCK_HDA_VERB
Remove old hda_verb.c code copied from intel/kblrvp7, as it's been superseded by the common block HDA implementation.
Fixes a null pointer error preventing the HDA codecs from being initialized.
Signed-off-by: Matt DeVillier matt.devillier@puri.sm Change-Id: I2fd5363aad027f215f93964bc6a85f00fea86c88 --- M src/mainboard/purism/librem_skl/Kconfig M src/mainboard/purism/librem_skl/hda_verb.c D src/mainboard/purism/librem_skl/hda_verb.h 3 files changed, 45 insertions(+), 131 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/34531/1
diff --git a/src/mainboard/purism/librem_skl/Kconfig b/src/mainboard/purism/librem_skl/Kconfig index 2372b9d..ecde9e4 100644 --- a/src/mainboard/purism/librem_skl/Kconfig +++ b/src/mainboard/purism/librem_skl/Kconfig @@ -5,6 +5,7 @@ select HAVE_ACPI_RESUME select HAVE_ACPI_TABLES select INTEL_LPSS_UART_FOR_CONSOLE + select SOC_INTEL_COMMON_BLOCK_HDA_VERB select SOC_INTEL_SKYLAKE select MAINBOARD_USES_FSP2_0 select SPD_READ_BY_WORD diff --git a/src/mainboard/purism/librem_skl/hda_verb.c b/src/mainboard/purism/librem_skl/hda_verb.c index 206af8d..ea89f00 100644 --- a/src/mainboard/purism/librem_skl/hda_verb.c +++ b/src/mainboard/purism/librem_skl/hda_verb.c @@ -1,8 +1,7 @@ /* * This file is part of the coreboot project. * - * Copyright (C) 2017 Intel Corporation - * (Written by Naresh G Solanki naresh.solanki@intel.com for Intel Corp.) + * Copyright (C) 2019 Purism SPC. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as @@ -14,70 +13,60 @@ * GNU General Public License for more details. */
-#include <bootstate.h> -#include <chip.h> -#include <console/console.h> #include <device/azalia_device.h> -#include <soc/intel/common/hda_verb.h> -#include <soc/pci_devs.h>
-#include "hda_verb.h" +const u32 cim_verb_data[] = { + /* coreboot specific header */ + 0x10ec0269, /* Codec Vendor / Device ID: Realtek ALC269 */ + 0x19910269, /* Subsystem ID */ + 0x0000000c, /* Number of jacks (NID entries) */
-static void codecs_init(u8 *base, u32 codec_mask) -{ - int i; + 0x0017ff00, /* Function Reset */ + 0x0017ff00, /* Double Function Reset */ + 0x0017ff00, + 0x0017ff00,
- /* Can support up to 4 codecs */ - for (i = 3; i >= 0; i--) { - if (codec_mask & (1 << i)) - hda_codec_init(base, i, cim_verb_data_size, - cim_verb_data); - } + /* Bits 31:28 - Codec Address */ + /* Bits 27:20 - NID */ + /* Bits 19:8 - Verb ID */ + /* Bits 7:0 - Payload */
- if (pc_beep_verbs_size) - hda_codec_write(base, pc_beep_verbs_size, pc_beep_verbs); -} + /* NID 0x01, HDA Codec Subsystem ID Verb Table: 0x19910269 */ + AZALIA_SUBVENDOR(0x0, 0x19910269),
-static void mb_hda_codec_init(void *unused) -{ - struct soc_intel_skylake_config *config; - u8 *base; - struct resource *res; - u32 codec_mask; - struct device *dev; + /* Pin Widget Verb Table */
- dev = SA_DEV_ROOT; - /* Check if HDA is enabled, else return */ - if (dev == NULL || dev->chip_info == NULL) - return; + /* Pin Complex (NID 0x12) */ + AZALIA_PIN_CFG(0x0, 0x12, 0x40000000),
- config = dev->chip_info; + /* Pin Complex (NID 0x14) */ + AZALIA_PIN_CFG(0x0, 0x14, 0x90170110),
- /* - * IoBufferOwnership 0:HD-A Link, 1:Shared HD-A Link and I2S Port, - * 3:I2S Ports. In HDA mode where codec need to be programmed with - * verb table - */ - if (config->IoBufferOwnership == 3) - return; + /* Pin Complex (NID 0x15) */ + AZALIA_PIN_CFG(0x0, 0x15, 0x04214020),
- /* Find base address */ - dev = pcidev_path_on_root(PCH_DEVFN_HDA); - if (dev == NULL) - return; - res = find_resource(dev, PCI_BASE_ADDRESS_0); - if (!res) - return; + /* Pin Complex (NID 0x17) */ + AZALIA_PIN_CFG(0x0, 0x17, 0x411111f0),
- base = res2mmio(res, 0, 0); - printk(BIOS_DEBUG, "HDA: base = %p\n", base); + /* Pin Complex (NID 0x18) */ + AZALIA_PIN_CFG(0x0, 0x18, 0x04a19040),
- codec_mask = hda_codec_detect(base); + /* Pin Complex (NID 0x19) */ + AZALIA_PIN_CFG(0x0, 0x19, 0x90a70130),
- if (codec_mask) { - printk(BIOS_DEBUG, "HDA: codec_mask = %02x\n", codec_mask); - codecs_init(base, codec_mask); - } -} + /* Pin Complex (NID 0x1A) */ + AZALIA_PIN_CFG(0x0, 0x1A, 0x411111f0),
-BOOT_STATE_INIT_ENTRY(BS_POST_DEVICE, BS_ON_EXIT, mb_hda_codec_init, NULL); + /* Pin Complex (NID 0x1B) */ + AZALIA_PIN_CFG(0x0, 0x1B, 0x411111f0), + + /* Pin Complex (NID 0x1D) */ + AZALIA_PIN_CFG(0x0, 0x1D, 0x40548505), + + /* Pin Complex (NID 0x1E) */ + AZALIA_PIN_CFG(0x0, 0x1E, 0x411111f0), +}; + +const u32 pc_beep_verbs[] = {}; + +AZALIA_ARRAY_SIZES; diff --git a/src/mainboard/purism/librem_skl/hda_verb.h b/src/mainboard/purism/librem_skl/hda_verb.h deleted file mode 100644 index 660ad0c..0000000 --- a/src/mainboard/purism/librem_skl/hda_verb.h +++ /dev/null @@ -1,76 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2016 Google Inc. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License as - * published by the Free Software Foundation; version 2 of the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#ifndef HDA_VERB_H -#define HDA_VERB_H - -#include <device/azalia_device.h> - -const u32 cim_verb_data[] = { - /* coreboot specific header */ - 0x10ec0269, /* Codec Vendor / Device ID: Realtek ALC269 */ - 0x19910269, /* Subsystem ID */ - 0x0000000c, /* Number of jacks (NID entries) */ - - 0x0017ff00, /* Function Reset */ - 0x0017ff00, /* Double Function Reset */ - 0x0017ff00, - 0x0017ff00, - - /* Bits 31:28 - Codec Address */ - /* Bits 27:20 - NID */ - /* Bits 19:8 - Verb ID */ - /* Bits 7:0 - Payload */ - - /* NID 0x01, HDA Codec Subsystem ID Verb Table: 0x19910269 */ - AZALIA_SUBVENDOR(0x0, 0x19910269), - - /* Pin Widget Verb Table */ - - /* Pin Complex (NID 0x12) */ - AZALIA_PIN_CFG(0x0, 0x12, 0x40000000), - - /* Pin Complex (NID 0x14) */ - AZALIA_PIN_CFG(0x0, 0x14, 0x90170110), - - /* Pin Complex (NID 0x15) */ - AZALIA_PIN_CFG(0x0, 0x15, 0x04214020), - - /* Pin Complex (NID 0x17) */ - AZALIA_PIN_CFG(0x0, 0x17, 0x411111f0), - - /* Pin Complex (NID 0x18) */ - AZALIA_PIN_CFG(0x0, 0x18, 0x04a19040), - - /* Pin Complex (NID 0x19) */ - AZALIA_PIN_CFG(0x0, 0x19, 0x90a70130), - - /* Pin Complex (NID 0x1A) */ - AZALIA_PIN_CFG(0x0, 0x1A, 0x411111f0), - - /* Pin Complex (NID 0x1B) */ - AZALIA_PIN_CFG(0x0, 0x1B, 0x411111f0), - - /* Pin Complex (NID 0x1D) */ - AZALIA_PIN_CFG(0x0, 0x1D, 0x40548505), - - /* Pin Complex (NID 0x1E) */ - AZALIA_PIN_CFG(0x0, 0x1E, 0x411111f0), -}; - -const u32 pc_beep_verbs[] = { -}; -AZALIA_ARRAY_SIZES; -#endif
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34531 )
Change subject: mb/purism/librem_skl: use SOC_INTEL_COMMON_BLOCK_HDA_VERB ......................................................................
Patch Set 1: Code-Review+1
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34531 )
Change subject: mb/purism/librem_skl: use SOC_INTEL_COMMON_BLOCK_HDA_VERB ......................................................................
Patch Set 1: Code-Review+2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34531 )
Change subject: mb/purism/librem_skl: use SOC_INTEL_COMMON_BLOCK_HDA_VERB ......................................................................
Patch Set 1: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34531 )
Change subject: mb/purism/librem_skl: use SOC_INTEL_COMMON_BLOCK_HDA_VERB ......................................................................
Patch Set 1: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/34531/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34531/1//COMMIT_MSG@14 PS1, Line 14: Could you please add the Coverity error line as a *Found-by* tag? Jacob, do you have it?
Tested how?
https://review.coreboot.org/c/coreboot/+/34531/1//COMMIT_MSG@16 PS1, Line 16: Change-Id: I2fd5363aad027f215f93964bc6a85f00fea86c88 Don’t the hooks add the Signed-off-by line to the end?
Hello Kyösti Mälkki, Angel Pons, Paul Menzel, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34531
to look at the new patch set (#2).
Change subject: mb/purism/librem_skl: use SOC_INTEL_COMMON_BLOCK_HDA_VERB ......................................................................
mb/purism/librem_skl: use SOC_INTEL_COMMON_BLOCK_HDA_VERB
Remove old hda_verb.c code copied from intel/kblrvp7, as it's been superseded by the common block HDA implementation.
Fixes a null pointer error preventing the HDA codecs from being initialized, as found in Coverity CID 1403651.
Test: build/boot Librem 13v2, verify functional audio
Signed-off-by: Matt DeVillier matt.devillier@puri.sm Change-Id: I2fd5363aad027f215f93964bc6a85f00fea86c88 --- M src/mainboard/purism/librem_skl/Kconfig M src/mainboard/purism/librem_skl/hda_verb.c D src/mainboard/purism/librem_skl/hda_verb.h 3 files changed, 45 insertions(+), 131 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/34531/2
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34531 )
Change subject: mb/purism/librem_skl: use SOC_INTEL_COMMON_BLOCK_HDA_VERB ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34531/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34531/1//COMMIT_MSG@14 PS1, Line 14:
Could you please add the Coverity error line as a *Found-by* tag? Jacob, do you have it? […]
Done
https://review.coreboot.org/c/coreboot/+/34531/1//COMMIT_MSG@16 PS1, Line 16: Change-Id: I2fd5363aad027f215f93964bc6a85f00fea86c88
Don’t the hooks add the Signed-off-by line to the end?
Everything was added via `git commit -s`
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34531 )
Change subject: mb/purism/librem_skl: use SOC_INTEL_COMMON_BLOCK_HDA_VERB ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34531/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34531/1//COMMIT_MSG@16 PS1, Line 16: Change-Id: I2fd5363aad027f215f93964bc6a85f00fea86c88
Everything was added via `git commit -s`
Done
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34531 )
Change subject: mb/purism/librem_skl: use SOC_INTEL_COMMON_BLOCK_HDA_VERB ......................................................................
mb/purism/librem_skl: use SOC_INTEL_COMMON_BLOCK_HDA_VERB
Remove old hda_verb.c code copied from intel/kblrvp7, as it's been superseded by the common block HDA implementation.
Fixes a null pointer error preventing the HDA codecs from being initialized, as found in Coverity CID 1403651.
Test: build/boot Librem 13v2, verify functional audio
Signed-off-by: Matt DeVillier matt.devillier@puri.sm Change-Id: I2fd5363aad027f215f93964bc6a85f00fea86c88 Reviewed-on: https://review.coreboot.org/c/coreboot/+/34531 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Nico Huber nico.h@gmx.de Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/purism/librem_skl/Kconfig M src/mainboard/purism/librem_skl/hda_verb.c D src/mainboard/purism/librem_skl/hda_verb.h 3 files changed, 45 insertions(+), 131 deletions(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Nico Huber: Looks good to me, approved Paul Menzel: Looks good to me, but someone else must approve Angel Pons: Looks good to me, but someone else must approve
diff --git a/src/mainboard/purism/librem_skl/Kconfig b/src/mainboard/purism/librem_skl/Kconfig index 2372b9d..ecde9e4 100644 --- a/src/mainboard/purism/librem_skl/Kconfig +++ b/src/mainboard/purism/librem_skl/Kconfig @@ -5,6 +5,7 @@ select HAVE_ACPI_RESUME select HAVE_ACPI_TABLES select INTEL_LPSS_UART_FOR_CONSOLE + select SOC_INTEL_COMMON_BLOCK_HDA_VERB select SOC_INTEL_SKYLAKE select MAINBOARD_USES_FSP2_0 select SPD_READ_BY_WORD diff --git a/src/mainboard/purism/librem_skl/hda_verb.c b/src/mainboard/purism/librem_skl/hda_verb.c index 206af8d..ea89f00 100644 --- a/src/mainboard/purism/librem_skl/hda_verb.c +++ b/src/mainboard/purism/librem_skl/hda_verb.c @@ -1,8 +1,7 @@ /* * This file is part of the coreboot project. * - * Copyright (C) 2017 Intel Corporation - * (Written by Naresh G Solanki naresh.solanki@intel.com for Intel Corp.) + * Copyright (C) 2019 Purism SPC. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as @@ -14,70 +13,60 @@ * GNU General Public License for more details. */
-#include <bootstate.h> -#include <chip.h> -#include <console/console.h> #include <device/azalia_device.h> -#include <soc/intel/common/hda_verb.h> -#include <soc/pci_devs.h>
-#include "hda_verb.h" +const u32 cim_verb_data[] = { + /* coreboot specific header */ + 0x10ec0269, /* Codec Vendor / Device ID: Realtek ALC269 */ + 0x19910269, /* Subsystem ID */ + 0x0000000c, /* Number of jacks (NID entries) */
-static void codecs_init(u8 *base, u32 codec_mask) -{ - int i; + 0x0017ff00, /* Function Reset */ + 0x0017ff00, /* Double Function Reset */ + 0x0017ff00, + 0x0017ff00,
- /* Can support up to 4 codecs */ - for (i = 3; i >= 0; i--) { - if (codec_mask & (1 << i)) - hda_codec_init(base, i, cim_verb_data_size, - cim_verb_data); - } + /* Bits 31:28 - Codec Address */ + /* Bits 27:20 - NID */ + /* Bits 19:8 - Verb ID */ + /* Bits 7:0 - Payload */
- if (pc_beep_verbs_size) - hda_codec_write(base, pc_beep_verbs_size, pc_beep_verbs); -} + /* NID 0x01, HDA Codec Subsystem ID Verb Table: 0x19910269 */ + AZALIA_SUBVENDOR(0x0, 0x19910269),
-static void mb_hda_codec_init(void *unused) -{ - struct soc_intel_skylake_config *config; - u8 *base; - struct resource *res; - u32 codec_mask; - struct device *dev; + /* Pin Widget Verb Table */
- dev = SA_DEV_ROOT; - /* Check if HDA is enabled, else return */ - if (dev == NULL || dev->chip_info == NULL) - return; + /* Pin Complex (NID 0x12) */ + AZALIA_PIN_CFG(0x0, 0x12, 0x40000000),
- config = dev->chip_info; + /* Pin Complex (NID 0x14) */ + AZALIA_PIN_CFG(0x0, 0x14, 0x90170110),
- /* - * IoBufferOwnership 0:HD-A Link, 1:Shared HD-A Link and I2S Port, - * 3:I2S Ports. In HDA mode where codec need to be programmed with - * verb table - */ - if (config->IoBufferOwnership == 3) - return; + /* Pin Complex (NID 0x15) */ + AZALIA_PIN_CFG(0x0, 0x15, 0x04214020),
- /* Find base address */ - dev = pcidev_path_on_root(PCH_DEVFN_HDA); - if (dev == NULL) - return; - res = find_resource(dev, PCI_BASE_ADDRESS_0); - if (!res) - return; + /* Pin Complex (NID 0x17) */ + AZALIA_PIN_CFG(0x0, 0x17, 0x411111f0),
- base = res2mmio(res, 0, 0); - printk(BIOS_DEBUG, "HDA: base = %p\n", base); + /* Pin Complex (NID 0x18) */ + AZALIA_PIN_CFG(0x0, 0x18, 0x04a19040),
- codec_mask = hda_codec_detect(base); + /* Pin Complex (NID 0x19) */ + AZALIA_PIN_CFG(0x0, 0x19, 0x90a70130),
- if (codec_mask) { - printk(BIOS_DEBUG, "HDA: codec_mask = %02x\n", codec_mask); - codecs_init(base, codec_mask); - } -} + /* Pin Complex (NID 0x1A) */ + AZALIA_PIN_CFG(0x0, 0x1A, 0x411111f0),
-BOOT_STATE_INIT_ENTRY(BS_POST_DEVICE, BS_ON_EXIT, mb_hda_codec_init, NULL); + /* Pin Complex (NID 0x1B) */ + AZALIA_PIN_CFG(0x0, 0x1B, 0x411111f0), + + /* Pin Complex (NID 0x1D) */ + AZALIA_PIN_CFG(0x0, 0x1D, 0x40548505), + + /* Pin Complex (NID 0x1E) */ + AZALIA_PIN_CFG(0x0, 0x1E, 0x411111f0), +}; + +const u32 pc_beep_verbs[] = {}; + +AZALIA_ARRAY_SIZES; diff --git a/src/mainboard/purism/librem_skl/hda_verb.h b/src/mainboard/purism/librem_skl/hda_verb.h deleted file mode 100644 index 660ad0c..0000000 --- a/src/mainboard/purism/librem_skl/hda_verb.h +++ /dev/null @@ -1,76 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2016 Google Inc. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License as - * published by the Free Software Foundation; version 2 of the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#ifndef HDA_VERB_H -#define HDA_VERB_H - -#include <device/azalia_device.h> - -const u32 cim_verb_data[] = { - /* coreboot specific header */ - 0x10ec0269, /* Codec Vendor / Device ID: Realtek ALC269 */ - 0x19910269, /* Subsystem ID */ - 0x0000000c, /* Number of jacks (NID entries) */ - - 0x0017ff00, /* Function Reset */ - 0x0017ff00, /* Double Function Reset */ - 0x0017ff00, - 0x0017ff00, - - /* Bits 31:28 - Codec Address */ - /* Bits 27:20 - NID */ - /* Bits 19:8 - Verb ID */ - /* Bits 7:0 - Payload */ - - /* NID 0x01, HDA Codec Subsystem ID Verb Table: 0x19910269 */ - AZALIA_SUBVENDOR(0x0, 0x19910269), - - /* Pin Widget Verb Table */ - - /* Pin Complex (NID 0x12) */ - AZALIA_PIN_CFG(0x0, 0x12, 0x40000000), - - /* Pin Complex (NID 0x14) */ - AZALIA_PIN_CFG(0x0, 0x14, 0x90170110), - - /* Pin Complex (NID 0x15) */ - AZALIA_PIN_CFG(0x0, 0x15, 0x04214020), - - /* Pin Complex (NID 0x17) */ - AZALIA_PIN_CFG(0x0, 0x17, 0x411111f0), - - /* Pin Complex (NID 0x18) */ - AZALIA_PIN_CFG(0x0, 0x18, 0x04a19040), - - /* Pin Complex (NID 0x19) */ - AZALIA_PIN_CFG(0x0, 0x19, 0x90a70130), - - /* Pin Complex (NID 0x1A) */ - AZALIA_PIN_CFG(0x0, 0x1A, 0x411111f0), - - /* Pin Complex (NID 0x1B) */ - AZALIA_PIN_CFG(0x0, 0x1B, 0x411111f0), - - /* Pin Complex (NID 0x1D) */ - AZALIA_PIN_CFG(0x0, 0x1D, 0x40548505), - - /* Pin Complex (NID 0x1E) */ - AZALIA_PIN_CFG(0x0, 0x1E, 0x411111f0), -}; - -const u32 pc_beep_verbs[] = { -}; -AZALIA_ARRAY_SIZES; -#endif
-
Angel Pons (Code Review) -
Kyösti Mälkki (Code Review)
-
Martin Roth (Code Review)
-
Matt DeVillier (Code Review)
-
Nico Huber (Code Review)
-
Paul Menzel (Code Review)