Change in coreboot[master]: soc/intel/skylake: devicetrice: introduce PchHdaVcType parameter for FSP
Michael Niewöhner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35542 )
Change subject: soc/intel/skylake: devicetrice: introduce PchHdaVcType parameter for FSP ......................................................................
soc/intel/skylake: devicetrice: introduce PchHdaVcType parameter for FSP
Set the FSP Parameter PchHdaVcType from devicetree if needed for a variant.
Signed-off-by: Michael Niewöhner foss@mniewoehner.de Change-Id: Ibafc3b6bd2495658f2bd634218042ec413a89f5e --- M src/mainboard/asrock/h110m/devicetree.cb M src/mainboard/asrock/h110m/ramstage.c M src/mainboard/intel/kblrvp/ramstage.c M src/mainboard/intel/kblrvp/variants/baseboard/devicetree.cb M src/mainboard/supermicro/x11-lga1151-series/ramstage.c M src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-tf/overridetree.cb M src/soc/intel/skylake/chip.h M src/soc/intel/skylake/chip_fsp20.c 8 files changed, 7 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/35542/1
diff --git a/src/mainboard/asrock/h110m/devicetree.cb b/src/mainboard/asrock/h110m/devicetree.cb index 572cd6a..f6a0bcf 100644 --- a/src/mainboard/asrock/h110m/devicetree.cb +++ b/src/mainboard/asrock/h110m/devicetree.cb @@ -55,6 +55,7 @@ register "PmTimerDisabled" = "0" register "EnableAzalia" = "1" register "DspEnable" = "0" + register "PchHdaVcType" = "0x01"
register "pirqa_routing" = "PCH_IRQ11" register "pirqb_routing" = "PCH_IRQ10" diff --git a/src/mainboard/asrock/h110m/ramstage.c b/src/mainboard/asrock/h110m/ramstage.c index c93e84c..a247b72 100644 --- a/src/mainboard/asrock/h110m/ramstage.c +++ b/src/mainboard/asrock/h110m/ramstage.c @@ -24,6 +24,4 @@ gpio_configure_pads(gpio_table, ARRAY_SIZE(gpio_table));
params->CdClock = 3; - /* Enable Virtual Channel 1 */ - params->PchHdaVcType = 0x1; } diff --git a/src/mainboard/intel/kblrvp/ramstage.c b/src/mainboard/intel/kblrvp/ramstage.c index ad55c26..a19e96e 100644 --- a/src/mainboard/intel/kblrvp/ramstage.c +++ b/src/mainboard/intel/kblrvp/ramstage.c @@ -25,9 +25,6 @@ * dependencies during hardware initialization. */ gpio_configure_pads(gpio_table, ARRAY_SIZE(gpio_table)); params->CdClock = 3; - - /* Enable Virtual Channel 1 */ - params->PchHdaVcType = 0x1; }
static void ioexpander_init(void *unused) diff --git a/src/mainboard/intel/kblrvp/variants/baseboard/devicetree.cb b/src/mainboard/intel/kblrvp/variants/baseboard/devicetree.cb index 2a1cb8a..43c16e3 100644 --- a/src/mainboard/intel/kblrvp/variants/baseboard/devicetree.cb +++ b/src/mainboard/intel/kblrvp/variants/baseboard/devicetree.cb @@ -30,6 +30,7 @@ register "SkipExtGfxScan" = "1" register "Device4Enable" = "1" register "SaGv" = "SaGv_Enabled" + register "PchHdaVcType" = "0x01"
register "pirqa_routing" = "PCH_IRQ11" register "pirqb_routing" = "PCH_IRQ10" diff --git a/src/mainboard/supermicro/x11-lga1151-series/ramstage.c b/src/mainboard/supermicro/x11-lga1151-series/ramstage.c index 677a10f..a16678e 100644 --- a/src/mainboard/supermicro/x11-lga1151-series/ramstage.c +++ b/src/mainboard/supermicro/x11-lga1151-series/ramstage.c @@ -20,9 +20,4 @@ /* Configure pads prior to SiliconInit() in case there's any * dependencies during hardware initialization. */ gpio_configure_pads(gpio_table, ARRAY_SIZE(gpio_table)); - - // TODO: on x11ssm FSP does not crash as in x11ssh; even when explicitly setting to 0x0 - // probably can be removed here - /* This must be one, otherwise FSP crashes ... */ - params->PchHdaVcType = 0x1; } diff --git a/src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-tf/overridetree.cb b/src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-tf/overridetree.cb index 1039f7a..02db50c 100644 --- a/src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-tf/overridetree.cb +++ b/src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-tf/overridetree.cb @@ -30,6 +30,9 @@ register "PcieRpEnable[8]" = "1" register "PcieRpClkReqSupport[8]" = "0"
+ # FIXME: find out why FSP crashes without this + register "PchHdaVcType" = "0x01" + device domain 0 on device pci 01.0 on end # unused device pci 01.1 on # PCIE Slot (JPCIE1) diff --git a/src/soc/intel/skylake/chip.h b/src/soc/intel/skylake/chip.h index fee14d8..dc263a2 100644 --- a/src/soc/intel/skylake/chip.h +++ b/src/soc/intel/skylake/chip.h @@ -207,6 +207,7 @@ /* Audio related */ u8 EnableAzalia; u8 DspEnable; + u8 PchHdaVcType;
/* * I/O Buffer Ownership: diff --git a/src/soc/intel/skylake/chip_fsp20.c b/src/soc/intel/skylake/chip_fsp20.c index d1d7d6f..4fa34b0 100644 --- a/src/soc/intel/skylake/chip_fsp20.c +++ b/src/soc/intel/skylake/chip_fsp20.c @@ -361,6 +361,7 @@ params->PchIshEnable = dev ? dev->enabled : 0;
params->PchHdaEnable = config->EnableAzalia; + params->PchHdaVcType = config->PchHdaVcType; params->PchHdaIoBufferOwnership = config->IoBufferOwnership; params->PchHdaDspEnable = config->DspEnable; params->Device4Enable = config->Device4Enable;
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35542
to look at the new patch set (#5).
Change subject: soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter ......................................................................
soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter
Make the the FSP Parameter PchHdaVcType a devicetree setting.
Signed-off-by: Michael Niewöhner foss@mniewoehner.de Change-Id: Ibafc3b6bd2495658f2bd634218042ec413a89f5e --- M src/mainboard/asrock/h110m/devicetree.cb M src/mainboard/asrock/h110m/ramstage.c M src/mainboard/intel/kblrvp/ramstage.c M src/mainboard/intel/kblrvp/variants/baseboard/devicetree.cb M src/mainboard/supermicro/x11-lga1151-series/ramstage.c M src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-tf/overridetree.cb M src/soc/intel/skylake/chip.h M src/soc/intel/skylake/chip_fsp20.c 8 files changed, 7 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/35542/5
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35542
to look at the new patch set (#9).
Change subject: soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter ......................................................................
soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter
Make the the FSP Parameter PchHdaVcType a devicetree setting.
Signed-off-by: Michael Niewöhner foss@mniewoehner.de Change-Id: Ibafc3b6bd2495658f2bd634218042ec413a89f5e --- M src/mainboard/asrock/h110m/devicetree.cb M src/mainboard/asrock/h110m/ramstage.c M src/mainboard/intel/kblrvp/ramstage.c M src/mainboard/intel/kblrvp/variants/baseboard/devicetree.cb M src/mainboard/supermicro/x11-lga1151-series/ramstage.c M src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-tf/overridetree.cb M src/soc/intel/skylake/chip.h M src/soc/intel/skylake/chip_fsp20.c 8 files changed, 7 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/35542/9
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35542
to look at the new patch set (#11).
Change subject: soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter ......................................................................
soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter
Make the the FSP Parameter PchHdaVcType a devicetree setting.
Signed-off-by: Michael Niewöhner foss@mniewoehner.de Change-Id: Ibafc3b6bd2495658f2bd634218042ec413a89f5e --- M src/mainboard/asrock/h110m/devicetree.cb M src/mainboard/asrock/h110m/ramstage.c M src/mainboard/intel/kblrvp/ramstage.c M src/mainboard/intel/kblrvp/variants/baseboard/devicetree.cb M src/mainboard/supermicro/x11-lga1151-series/ramstage.c M src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-tf/overridetree.cb M src/soc/intel/skylake/chip.h M src/soc/intel/skylake/chip_fsp20.c 8 files changed, 7 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/35542/11
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35542
to look at the new patch set (#12).
Change subject: soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter ......................................................................
soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter
Make the the FSP Parameter PchHdaVcType a devicetree setting and make use of it in the devicetrees of all boards that currently set it.
Signed-off-by: Michael Niewöhner foss@mniewoehner.de Change-Id: Ibafc3b6bd2495658f2bd634218042ec413a89f5e --- M src/mainboard/asrock/h110m/devicetree.cb M src/mainboard/asrock/h110m/ramstage.c M src/mainboard/intel/kblrvp/ramstage.c M src/mainboard/intel/kblrvp/variants/baseboard/devicetree.cb M src/mainboard/supermicro/x11-lga1151-series/ramstage.c M src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-tf/overridetree.cb M src/soc/intel/skylake/chip.h M src/soc/intel/skylake/chip_fsp20.c 8 files changed, 7 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/35542/12
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35542 )
Change subject: soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter ......................................................................
Patch Set 12: Code-Review+1
(4 comments)
Thank you for this contribution
https://review.coreboot.org/c/coreboot/+/35542/12/src/mainboard/asrock/h110m... File src/mainboard/asrock/h110m/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/35542/12/src/mainboard/asrock/h110m... PS12, Line 58: 0x01 "VC1"
https://review.coreboot.org/c/coreboot/+/35542/12/src/mainboard/intel/kblrvp... File src/mainboard/intel/kblrvp/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/35542/12/src/mainboard/intel/kblrvp... PS12, Line 33: 0x01 "VC1"
https://review.coreboot.org/c/coreboot/+/35542/12/src/mainboard/supermicro/x... File src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-tf/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/35542/12/src/mainboard/supermicro/x... PS12, Line 34: register "PchHdaVcType" = "0x01" register "PchHdaVcType" = "VC1"
https://review.coreboot.org/c/coreboot/+/35542/12/src/soc/intel/skylake/chip... File src/soc/intel/skylake/chip.h:
https://review.coreboot.org/c/coreboot/+/35542/12/src/soc/intel/skylake/chip... PS12, Line 210: u8 PchHdaVcType I think it would be better /* Virtual Channel Type Select */ enum { VC0, VC1, } PchHdaVcType;
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35542 )
Change subject: soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter ......................................................................
Patch Set 12:
I'm still curious why it needs to be one on some boards, even if the HDA is disabled. For me it sounds like a bug that's triggered somehow.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35542 )
Change subject: soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter ......................................................................
Patch Set 12:
Patch Set 12:
I'm still curious why it needs to be one on some boards, even if the HDA is disabled. For me it sounds like a bug that's triggered somehow.
Yeah, I guess this is a bug in FSP, so we only should set this when needed; I've added more info to https://github.com/IntelFsp/FSP/issues/30
Hello Patrick Rudolph, Angel Pons, Maxim Polyakov, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35542
to look at the new patch set (#13).
Change subject: soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter ......................................................................
soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter
Make the the FSP Parameter PchHdaVcType a devicetree setting and make use of it in the devicetrees of all boards that currently set it.
Signed-off-by: Michael Niewöhner foss@mniewoehner.de Change-Id: Ibafc3b6bd2495658f2bd634218042ec413a89f5e --- M src/mainboard/asrock/h110m/devicetree.cb M src/mainboard/asrock/h110m/ramstage.c M src/mainboard/intel/kblrvp/ramstage.c M src/mainboard/intel/kblrvp/variants/baseboard/devicetree.cb M src/mainboard/supermicro/x11-lga1151-series/ramstage.c M src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-tf/overridetree.cb M src/soc/intel/skylake/chip.h M src/soc/intel/skylake/chip_fsp20.c 8 files changed, 12 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/35542/13
Hello Patrick Rudolph, Angel Pons, Maxim Polyakov, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35542
to look at the new patch set (#14).
Change subject: soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter ......................................................................
soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter
Make the the FSP Parameter PchHdaVcType a devicetree setting and make use of it in the devicetrees of all boards that currently set it.
Signed-off-by: Michael Niewöhner foss@mniewoehner.de Change-Id: Ibafc3b6bd2495658f2bd634218042ec413a89f5e --- M src/mainboard/asrock/h110m/devicetree.cb M src/mainboard/asrock/h110m/ramstage.c M src/mainboard/intel/kblrvp/ramstage.c M src/mainboard/intel/kblrvp/variants/baseboard/devicetree.cb M src/mainboard/supermicro/x11-lga1151-series/ramstage.c M src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-tf/overridetree.cb M src/soc/intel/skylake/chip.h M src/soc/intel/skylake/chip_fsp20.c 8 files changed, 12 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/35542/14
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35542 )
Change subject: soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter ......................................................................
Patch Set 14:
(4 comments)
https://review.coreboot.org/c/coreboot/+/35542/12/src/mainboard/asrock/h110m... File src/mainboard/asrock/h110m/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/35542/12/src/mainboard/asrock/h110m... PS12, Line 58: 0x01
"VC1"
Done
https://review.coreboot.org/c/coreboot/+/35542/12/src/mainboard/intel/kblrvp... File src/mainboard/intel/kblrvp/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/35542/12/src/mainboard/intel/kblrvp... PS12, Line 33: 0x01
"VC1"
Done
https://review.coreboot.org/c/coreboot/+/35542/12/src/mainboard/supermicro/x... File src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-tf/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/35542/12/src/mainboard/supermicro/x... PS12, Line 34: register "PchHdaVcType" = "0x01"
register "PchHdaVcType" = "VC1"
Done
https://review.coreboot.org/c/coreboot/+/35542/12/src/soc/intel/skylake/chip... File src/soc/intel/skylake/chip.h:
https://review.coreboot.org/c/coreboot/+/35542/12/src/soc/intel/skylake/chip... PS12, Line 210: u8 PchHdaVcType
I think it would be better […]
Done
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35542 )
Change subject: soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter ......................................................................
Patch Set 14: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35542 )
Change subject: soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter ......................................................................
soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter
Make the the FSP Parameter PchHdaVcType a devicetree setting and make use of it in the devicetrees of all boards that currently set it.
Signed-off-by: Michael Niewöhner foss@mniewoehner.de Change-Id: Ibafc3b6bd2495658f2bd634218042ec413a89f5e Reviewed-on: https://review.coreboot.org/c/coreboot/+/35542 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/mainboard/asrock/h110m/devicetree.cb M src/mainboard/asrock/h110m/ramstage.c M src/mainboard/intel/kblrvp/ramstage.c M src/mainboard/intel/kblrvp/variants/baseboard/devicetree.cb M src/mainboard/supermicro/x11-lga1151-series/ramstage.c M src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-tf/overridetree.cb M src/soc/intel/skylake/chip.h M src/soc/intel/skylake/chip_fsp20.c 8 files changed, 12 insertions(+), 8 deletions(-)
Approvals: build bot (Jenkins): Verified Maxim Polyakov: Looks good to me, approved
diff --git a/src/mainboard/asrock/h110m/devicetree.cb b/src/mainboard/asrock/h110m/devicetree.cb index 572cd6a..acb2a9e 100644 --- a/src/mainboard/asrock/h110m/devicetree.cb +++ b/src/mainboard/asrock/h110m/devicetree.cb @@ -55,6 +55,7 @@ register "PmTimerDisabled" = "0" register "EnableAzalia" = "1" register "DspEnable" = "0" + register "PchHdaVcType" = "Vc1"
register "pirqa_routing" = "PCH_IRQ11" register "pirqb_routing" = "PCH_IRQ10" diff --git a/src/mainboard/asrock/h110m/ramstage.c b/src/mainboard/asrock/h110m/ramstage.c index c93e84c..a247b72 100644 --- a/src/mainboard/asrock/h110m/ramstage.c +++ b/src/mainboard/asrock/h110m/ramstage.c @@ -24,6 +24,4 @@ gpio_configure_pads(gpio_table, ARRAY_SIZE(gpio_table));
params->CdClock = 3; - /* Enable Virtual Channel 1 */ - params->PchHdaVcType = 0x1; } diff --git a/src/mainboard/intel/kblrvp/ramstage.c b/src/mainboard/intel/kblrvp/ramstage.c index ad55c26..a19e96e 100644 --- a/src/mainboard/intel/kblrvp/ramstage.c +++ b/src/mainboard/intel/kblrvp/ramstage.c @@ -25,9 +25,6 @@ * dependencies during hardware initialization. */ gpio_configure_pads(gpio_table, ARRAY_SIZE(gpio_table)); params->CdClock = 3; - - /* Enable Virtual Channel 1 */ - params->PchHdaVcType = 0x1; }
static void ioexpander_init(void *unused) diff --git a/src/mainboard/intel/kblrvp/variants/baseboard/devicetree.cb b/src/mainboard/intel/kblrvp/variants/baseboard/devicetree.cb index 2a1cb8a..212721a 100644 --- a/src/mainboard/intel/kblrvp/variants/baseboard/devicetree.cb +++ b/src/mainboard/intel/kblrvp/variants/baseboard/devicetree.cb @@ -30,6 +30,7 @@ register "SkipExtGfxScan" = "1" register "Device4Enable" = "1" register "SaGv" = "SaGv_Enabled" + register "PchHdaVcType" = "Vc1"
register "pirqa_routing" = "PCH_IRQ11" register "pirqb_routing" = "PCH_IRQ10" diff --git a/src/mainboard/supermicro/x11-lga1151-series/ramstage.c b/src/mainboard/supermicro/x11-lga1151-series/ramstage.c index 694165a..a16678e 100644 --- a/src/mainboard/supermicro/x11-lga1151-series/ramstage.c +++ b/src/mainboard/supermicro/x11-lga1151-series/ramstage.c @@ -20,7 +20,4 @@ /* Configure pads prior to SiliconInit() in case there's any * dependencies during hardware initialization. */ gpio_configure_pads(gpio_table, ARRAY_SIZE(gpio_table)); - - /* This must be one, otherwise FSP crashes ... */ - params->PchHdaVcType = 0x1; } diff --git a/src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-tf/overridetree.cb b/src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-tf/overridetree.cb index 1039f7a..09aa8b5 100644 --- a/src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-tf/overridetree.cb +++ b/src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-tf/overridetree.cb @@ -30,6 +30,9 @@ register "PcieRpEnable[8]" = "1" register "PcieRpClkReqSupport[8]" = "0"
+ # FIXME: find out why FSP crashes without this + register "PchHdaVcType" = "Vc1" + device domain 0 on device pci 01.0 on end # unused device pci 01.1 on # PCIE Slot (JPCIE1) diff --git a/src/soc/intel/skylake/chip.h b/src/soc/intel/skylake/chip.h index 70fb045..944315b 100644 --- a/src/soc/intel/skylake/chip.h +++ b/src/soc/intel/skylake/chip.h @@ -208,6 +208,12 @@ u8 EnableAzalia; u8 DspEnable;
+ /* HDA Virtual Channel Type Select */ + enum { + Vc0, + Vc1, + } PchHdaVcType; + /* * I/O Buffer Ownership: * 0: HD-A Link diff --git a/src/soc/intel/skylake/chip_fsp20.c b/src/soc/intel/skylake/chip_fsp20.c index e46e52c..462285c2 100644 --- a/src/soc/intel/skylake/chip_fsp20.c +++ b/src/soc/intel/skylake/chip_fsp20.c @@ -361,6 +361,7 @@ params->PchIshEnable = dev ? dev->enabled : 0;
params->PchHdaEnable = config->EnableAzalia; + params->PchHdaVcType = config->PchHdaVcType; params->PchHdaIoBufferOwnership = config->IoBufferOwnership; params->PchHdaDspEnable = config->DspEnable; params->Device4Enable = config->Device4Enable;
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35542 )
Change subject: soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter ......................................................................
Patch Set 15:
Patch Set 12:
I'm still curious why it needs to be one on some boards, even if the HDA is disabled. For me it sounds like a bug that's triggered somehow.
I tried disabling HDA for my asrock board + I did not install VC1 (VC0 is installed by default in FSP):
--- a/src/mainboard/asrock/h110m/devicetree.cb +++ b/src/mainboard/asrock/h110m/devicetree.cb @@ -53,7 +53,7 @@ chip soc/intel/skylake register "Device4Enable" = "1" register "SaGv" = "SaGv_Enabled" register "PmTimerDisabled" = "0" - register "EnableAzalia" = "1" + register "EnableAzalia" = "0"
register "pirqa_routing" = "PCH_IRQ11" @@ -426,7 +426,7 @@ chip soc/intel/skylake end # LPC Interface device pci 1f.1 on end # P2SB device pci 1f.2 on end # Power Management Controller - device pci 1f.3 on end # Intel HDA + device pci 1f.3 off end # Intel HDA device pci 1f.4 on end # SMBus
params->CdClock = 3; - /* Enable Virtual Channel 1 */ - params->PchHdaVcType = 0x1;
The OS is loading, but there is no sound
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35542 )
Change subject: soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter ......................................................................
Patch Set 15:
Patch Set 15:
Patch Set 12:
I'm still curious why it needs to be one on some boards, even if the HDA is disabled. For me it sounds like a bug that's triggered somehow.
I tried disabling HDA for my asrock board + I did not install VC1 (VC0 is installed by default in FSP):
--- a/src/mainboard/asrock/h110m/devicetree.cb +++ b/src/mainboard/asrock/h110m/devicetree.cb @@ -53,7 +53,7 @@ chip soc/intel/skylake register "Device4Enable" = "1" register "SaGv" = "SaGv_Enabled" register "PmTimerDisabled" = "0"
register "EnableAzalia" = "1"
register "EnableAzalia" = "0" register "pirqa_routing" = "PCH_IRQ11"@@ -426,7 +426,7 @@ chip soc/intel/skylake end # LPC Interface device pci 1f.1 on end # P2SB device pci 1f.2 on end # Power Management Controller
device pci 1f.3 on end # Intel HDA
device pci 1f.3 off end # Intel HDA device pci 1f.4 on end # SMBus params->CdClock = 3;
/* Enable Virtual Channel 1 */params->PchHdaVcType = 0x1;The OS is loading, but there is no sound
Patrick meant the bug that FSP crashes on some boards with EnableAzalia=0, PchHdaVcType=0, while it does not with EnableAzalia=0, PchHdaVcType=1. See the github bug ;)
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35542 )
Change subject: soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter ......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35542/15/src/soc/intel/skylake/chip... File src/soc/intel/skylake/chip.h:
https://review.coreboot.org/c/coreboot/+/35542/15/src/soc/intel/skylake/chip... PS15, Line 213: Vc0, What is the default in various FSP binaries? SKL and KBL? And is Vc0 ok in practice?
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35542 )
Change subject: soc/intel/skylake: devicetree: introduce PchHdaVcType fsp parameter ......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35542/15/src/soc/intel/skylake/chip... File src/soc/intel/skylake/chip.h:
https://review.coreboot.org/c/coreboot/+/35542/15/src/soc/intel/skylake/chip... PS15, Line 213: Vc0,
What is the default in various FSP binaries? SKL and KBL? And is Vc0 ok in practice?
Vc0 is ok in pratice at least for boards that have HDA disabled and do not crash with Vc0 due to NULL pointer deref
-
Aaron Durbin (Code Review) -
Maxim Polyakov (Code Review)
-
Michael Niewöhner (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)