Change in coreboot[master]: nb/intel/i945: Refactor sdram_set_channel_mode()

List overview All Threads
Download

newer

older

Change in coreboot[master]: Revert...

Change in coreboot[master]:...

HAOUAS Elyes (Code Review)

14 Mar 2020 14 Mar '20

4:02 a.m.

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39537 )

Change subject: nb/intel/i945: Refactor sdram_set_channel_mode() ......................................................................

nb/intel/i945: Refactor sdram_set_channel_mode()

Change-Id: I7574bc4a1dbb55d12dec9be17e403a2f448c16c0 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/i945/raminit.c 1 file changed, 10 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39537/1

diff --git a/src/northbridge/intel/i945/raminit.c b/src/northbridge/intel/i945/raminit.c index dd7030e..972e71f 100644 --- a/src/northbridge/intel/i945/raminit.c +++ b/src/northbridge/intel/i945/raminit.c @@ -1572,21 +1572,19 @@ reg32 = MCHBAR32(DCC); reg32 &= ~(7 << 0);

- if (sysinfo->interleaved) { - /* Dual Channel Interleaved */ - printk(BIOS_DEBUG, "Dual Channel Interleaved.\n"); - reg32 |= (1 << 1); - } else if (sysinfo->dimm[0] == SYSINFO_DIMM_NOT_POPULATED && - sysinfo->dimm[1] == SYSINFO_DIMM_NOT_POPULATED) { + if (sdram_capabilities_dual_channel() { + if (sysinfo->interleaved) { + printk(BIOS_DEBUG, "Dual Channel Interleaved.\n"); + reg32 |= (1 << 1); + } else { + /* Dual Channel Asymmetric */ + printk(BIOS_DEBUG, "Dual Channel Asymmetric.\n"); + reg32 |= (1 << 0); + } + } else if (sysinfo->dimm[0] == SYSINFO_DIMM_NOT_POPULATED { /* Channel 1 only */ printk(BIOS_DEBUG, "Single Channel 1 only.\n"); reg32 |= (1 << 2); - } else if (sdram_capabilities_dual_channel() && - (sysinfo->dimm[2] != SYSINFO_DIMM_NOT_POPULATED || - sysinfo->dimm[3] != SYSINFO_DIMM_NOT_POPULATED)) { - /* Dual Channel Asymmetric */ - printk(BIOS_DEBUG, "Dual Channel Asymmetric.\n"); - reg32 |= (1 << 0); } else { /* All bits 0 means Single Channel 0 operation */ printk(BIOS_DEBUG, "Single Channel 0 only.\n");

Attachments:

attachment.htm (text/html — 4.4 KB)

Show replies by date

HAOUAS Elyes (Code Review)

14 Mar 14 Mar

4:15 a.m.

Hello build bot (Jenkins), Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39537

to look at the new patch set (#2).

Change subject: nb/intel/i945: Refactor sdram_set_channel_mode() ......................................................................

nb/intel/i945: Refactor sdram_set_channel_mode()

Change-Id: I7574bc4a1dbb55d12dec9be17e403a2f448c16c0 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/i945/raminit.c 1 file changed, 10 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39537/2

HAOUAS Elyes (Code Review)

4:26 a.m.

Hello build bot (Jenkins), Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39537

to look at the new patch set (#3).

Change subject: nb/intel/i945: Refactor sdram_set_channel_mode() ......................................................................

nb/intel/i945: Refactor sdram_set_channel_mode()

Change-Id: I7574bc4a1dbb55d12dec9be17e403a2f448c16c0 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/i945/raminit.c 1 file changed, 10 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39537/3

Angel Pons (Code Review)

4:29 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39537 )

Change subject: nb/intel/i945: Refactor sdram_set_channel_mode() ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39537/3/src/northbridge/intel/i945/... File src/northbridge/intel/i945/raminit.c:

https://review.coreboot.org/c/coreboot/+/39537/3/src/northbridge/intel/i945/... PS3, Line 1584: } else What if sysinfo->dimm[1] is populated?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7574bc4a1dbb55d12dec9be17e403a2f448c16c0 Gerrit-Change-Number: 39537 Gerrit-PatchSet: 3 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Angel Pons th3fanbus@gmail.com Gerrit-Comment-Date: Sat, 14 Mar 2020 23:29:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

11:52 p.m.

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39537 )

Change subject: nb/intel/i945: Refactor sdram_set_channel_mode() ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39537/3/src/northbridge/intel/i945/... File src/northbridge/intel/i945/raminit.c:

https://review.coreboot.org/c/coreboot/+/39537/3/src/northbridge/intel/i945/... PS3, Line 1584: } else

...

What if sysinfo->dimm[1] is populated?

Here we are in Single-Channel. so if DIMM0 is not populated, it means that DIMM1 is populated 😊 so here you are 😊

-- To view, visit https://review.coreboot.org/c/coreboot/+/39537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7574bc4a1dbb55d12dec9be17e403a2f448c16c0 Gerrit-Change-Number: 39537 Gerrit-PatchSet: 3 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Angel Pons th3fanbus@gmail.com Gerrit-Comment-Date: Sun, 15 Mar 2020 06:52:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Angel Pons (Code Review)

15 Mar 15 Mar

3:48 a.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39537 )

Change subject: nb/intel/i945: Refactor sdram_set_channel_mode() ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39537/3/src/northbridge/intel/i945/... File src/northbridge/intel/i945/raminit.c:

https://review.coreboot.org/c/coreboot/+/39537/3/src/northbridge/intel/i945/... PS3, Line 1584: } else

...

Here we are in Single-Channel. so if DIMM0 is not populated, it means that DIMM1 is populated 😊 […]

I think I did not express myself well.

What I mean is that, if we are not in dual-channel mode, and DIMM0 is empty, it is possible for DIMM1 to be the only populated slot. And DIMM1 is on Channel 0, sooo... 😊

-- To view, visit https://review.coreboot.org/c/coreboot/+/39537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7574bc4a1dbb55d12dec9be17e403a2f448c16c0 Gerrit-Change-Number: 39537 Gerrit-PatchSet: 3 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Angel Pons th3fanbus@gmail.com Gerrit-Comment-Date: Sun, 15 Mar 2020 10:48:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Comment-In-Reply-To: HAOUAS Elyes ehaouas@noos.fr Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

3:58 a.m.

Hello build bot (Jenkins), Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39537

to look at the new patch set (#4).

Change subject: nb/intel/i945: Refactor sdram_set_channel_mode() ......................................................................

nb/intel/i945: Refactor sdram_set_channel_mode()

Change-Id: I7574bc4a1dbb55d12dec9be17e403a2f448c16c0 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/i945/raminit.c 1 file changed, 12 insertions(+), 14 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39537/4

-- To view, visit https://review.coreboot.org/c/coreboot/+/39537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7574bc4a1dbb55d12dec9be17e403a2f448c16c0 Gerrit-Change-Number: 39537 Gerrit-PatchSet: 4 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: newpatchset

HAOUAS Elyes (Code Review)

4:05 a.m.

Hello build bot (Jenkins), Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39537

to look at the new patch set (#5).

Change subject: nb/intel/i945: Refactor sdram_set_channel_mode() ......................................................................

nb/intel/i945: Refactor sdram_set_channel_mode()

Change-Id: I7574bc4a1dbb55d12dec9be17e403a2f448c16c0 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/i945/raminit.c 1 file changed, 11 insertions(+), 13 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39537/5

-- To view, visit https://review.coreboot.org/c/coreboot/+/39537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7574bc4a1dbb55d12dec9be17e403a2f448c16c0 Gerrit-Change-Number: 39537 Gerrit-PatchSet: 5 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: newpatchset

HAOUAS Elyes (Code Review)

4:08 a.m.

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39537 )

Change subject: nb/intel/i945: Refactor sdram_set_channel_mode() ......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39537/3/src/northbridge/intel/i945/... File src/northbridge/intel/i945/raminit.c:

https://review.coreboot.org/c/coreboot/+/39537/3/src/northbridge/intel/i945/... PS3, Line 1584: } else

...

I think I did not express myself well. […]

Please see datasheet page 151 [Bit 2]: Single-Channel Selector (SCS): When in Single-channel mode, this is the populated channel. 0: "Channel 0" 1: "Channel 1"

-- To view, visit https://review.coreboot.org/c/coreboot/+/39537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7574bc4a1dbb55d12dec9be17e403a2f448c16c0 Gerrit-Change-Number: 39537 Gerrit-PatchSet: 5 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Angel Pons th3fanbus@gmail.com Gerrit-Comment-Date: Sun, 15 Mar 2020 11:08:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Comment-In-Reply-To: HAOUAS Elyes ehaouas@noos.fr Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

11:11 p.m.

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39537 )

Change subject: nb/intel/i945: Refactor sdram_set_channel_mode() ......................................................................

Patch Set 5:

This change is ready for review.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7574bc4a1dbb55d12dec9be17e403a2f448c16c0 Gerrit-Change-Number: 39537 Gerrit-PatchSet: 5 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Angel Pons th3fanbus@gmail.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-Comment-Date: Mon, 16 Mar 2020 06:11:21 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

11:11 p.m.

HAOUAS Elyes has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/39537 )

Change subject: nb/intel/i945: Refactor sdram_set_channel_mode() ......................................................................

Abandoned

-- To view, visit https://review.coreboot.org/c/coreboot/+/39537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7574bc4a1dbb55d12dec9be17e403a2f448c16c0 Gerrit-Change-Number: 39537 Gerrit-PatchSet: 5 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Angel Pons th3fanbus@gmail.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-MessageType: abandon

1828

days inactive

1830

days old

coreboot-gerrit@coreboot.org

10 comments

2 participants

tags (0)

participants (2)

Angel Pons (Code Review)
HAOUAS Elyes (Code Review)