Attention is currently required from: Nico Huber, Paul Menzel, Nicholas Chin, Swift Geek (Sebastian Grzywna), Elyes Haouas.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34826 )
Change subject: nb/intel/gm45: Wedge DDR2 SPD support in
......................................................................
Patch Set 15:
(3 comments)
File src/northbridge/intel/gm45/raminit.c:
https://review.coreboot.org/c/coreboot/+/34826/comment/12a7bdbf_dd4315f1
PS15, Line 232: test_dimm(sysinfo, dimm, 6, 0xff, 0x40
> I think it's assuming that if the data width isn't 64 bits then it's probably 72 bits which would be […]
Seems likely, yes
https://review.coreboot.org/c/coreboot/+/34826/comment/8c8c6f96_f5e9cf60
PS15, Line 233: !test_dimm(sysinfo, dimm, 11, 0xff, 0x00))
: die("Chipset doesn't support ECC RAM\n");
> if (dimm_info.flags.is_ecc) […]
Does gm45 code have dimm_info?
https://review.coreboot.org/c/coreboot/+/34826/comment/2e43ae54_276635f6
PS15, Line 334: typedef struct {
: int dimm_mask;
: struct spd_dimminfo {
: unsigned int rows;
: unsigned int cols;
: unsigned int chip_capacity;
: unsigned int banks;
: unsigned int ranks;
: unsigned int cas_latencies;
: unsigned int tAAmin;
: unsigned int tCKmin;
: unsigned int width;
: unsigned int tRAS;
: unsigned int tRP;
: unsigned int tRCD;
: unsigned int tWR;
: unsigned int page_size;
: unsigned int raw_card;
: } channel[2];
: } spdinfo_t;
> [out of topic] […]
It's not needed in any other file.
--
To view, visit https://review.coreboot.org/c/coreboot/+/34826
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I265938d58c30264fd5d4f7b89da7b689058b8cf8
Gerrit-Change-Number: 34826
Gerrit-PatchSet: 15
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Swift Geek (Sebastian Grzywna) <swiftgeek(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Swift Geek (Sebastian Grzywna) <swiftgeek(a)gmail.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Sun, 01 Jan 2023 18:06:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>
Comment-In-Reply-To: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-MessageType: comment
Attention is currently required from: Bora Guvendik, Zhixing Ma, Anil Kumar K, Hannah Williams, Cliff Huang, Tarun Tuli, Nico Huber, Michał Żygowski, Jérémy Compostella, Paul Menzel, Arthur Heymans.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/70276 )
Change subject: drivers/intel/gma: Hook up libgfxinit in romstage
......................................................................
Patch Set 19:
(2 comments)
File src/drivers/intel/gma/Kconfig:
https://review.coreboot.org/c/coreboot/+/70276/comment/77614eea_b0dfd373
PS13, Line 95: SOC_INTEL_ALDERLAKE
> Agreed. Suggest separating cleanup of existing block from this patch train though as out of scope.
Let's keep it like this for now.
File src/drivers/intel/gma/Kconfig:
https://review.coreboot.org/c/coreboot/+/70276/comment/cc048285_350f6fbb
PS15, Line 105: If
: libgfxinit is used for both romstage and ramstage this
: address should be the same than the one allocated by the PCI
: resource allocator in ramstage.
> The issue is that libhwbase decides of which driver to build and use at build time based on HWBASE_S […]
For now, you can tell coreboot to report BAR0 as a fixed resource. This avoids surprises later on in case something regarding allocation changes, e.g. PCIe bridges at 00.01.x being populated or not.
https://gist.github.com/Th3Fanbus/2f3c65aaccc0e1002a852951eb6d682d should do the trick. A long-term solution could be to replace the `HWBASE_STATIC_MMIO` and `HWBASE_DYNAMIC_MMIO` Kconfig options with stage-dependent implementations of some function. For instance, have a function that returns the base address; the romstage implementation returns a fixed base address and the ramstage implementation reads the value from the PCI config space registers.
--
To view, visit https://review.coreboot.org/c/coreboot/+/70276
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3c2101de10dc5df54fe873e43bbe0f1c4dccff44
Gerrit-Change-Number: 70276
Gerrit-PatchSet: 19
Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Boris Mittelberg <bmbm(a)google.com>
Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: Zhixing Ma <zhixing.ma(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Zhixing Ma <zhixing.ma(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Sun, 01 Jan 2023 17:56:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tarun Tuli <taruntuli(a)google.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Bora Guvendik, Zhixing Ma, Anil Kumar K, Hannah Williams, Cliff Huang, Tarun Tuli, Nico Huber, Michał Żygowski, Jérémy Compostella, Paul Menzel, Arthur Heymans.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/70276 )
Change subject: drivers/intel/gma: Hook up libgfxinit in romstage
......................................................................
Patch Set 19:
(1 comment)
File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/70276/comment/51105b6e_20e48278
PS15, Line 64: cache-as-ram
: stage
> Marking this to leave it as EARLY for now unless anyone has very strong objections. […]
Sounds good.
--
To view, visit https://review.coreboot.org/c/coreboot/+/70276
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3c2101de10dc5df54fe873e43bbe0f1c4dccff44
Gerrit-Change-Number: 70276
Gerrit-PatchSet: 19
Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Boris Mittelberg <bmbm(a)google.com>
Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: Zhixing Ma <zhixing.ma(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Zhixing Ma <zhixing.ma(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Sun, 01 Jan 2023 17:50:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tarun Tuli <taruntuli(a)google.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Elyes Haouas.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/70575 )
Change subject: nb/intel/i440bx: Specify supported memory type
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
This patch seems a bit silly right now, but if the ultimate goal is to get rid of the `NO_` options, then it seems reasonable enough.
--
To view, visit https://review.coreboot.org/c/coreboot/+/70575
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If94037f2b010527440795e6920dd7a533c52f606
Gerrit-Change-Number: 70575
Gerrit-PatchSet: 4
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Sun, 01 Jan 2023 17:49:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Elyes Haouas.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71583 )
Change subject: nb/intel/haswell: Specify supported memory type
......................................................................
Patch Set 2:
(1 comment)
File src/northbridge/intel/haswell/Kconfig:
https://review.coreboot.org/c/coreboot/+/71583/comment/bb67531a_48ebd3e2
PS2, Line 18: select USE_DDR4
Haswell does *not* support DDR4. The server platforms do, but they're a completely different beast (not supported in coreboot yet).
--
To view, visit https://review.coreboot.org/c/coreboot/+/71583
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I885cc00c8bfcfaaabb2ce2b0269172d8d7a88db5
Gerrit-Change-Number: 71583
Gerrit-PatchSet: 2
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Sun, 01 Jan 2023 17:48:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment