Change in ...coreboot[master]: soc/skl/memmap: calculate mem size even if IGD undefined in devtree

[Maxim Polyakov (Code Review)]

Maxim Polyakov has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32614

Change subject: soc/skl/memmap: calculate mem size even if IGD undefined in devtree
......................................................................

soc/skl/memmap: calculate mem size even if IGD undefined in devtree

The DRAM base memory should be calculated even if IGD isn`t defined
in the board device tree

Tested on Asrock H110M-DVS

Change-Id: I3da51473e6c06da803bd969a4a6dff792c18f962
Signed-off-by: Maxim Polyakov 
---
M src/soc/intel/skylake/memmap.c
1 file changed, 8 insertions(+), 13 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/32614/1

diff --git a/src/soc/intel/skylake/memmap.c b/src/soc/intel/skylake/memmap.c
index fde916a..40c52fe 100644
--- a/src/soc/intel/skylake/memmap.c
+++ b/src/soc/intel/skylake/memmap.c
@@ -170,13 +170,13 @@
 }

/* Calculate Intel Traditional Memory size based on GSM, DSM, TSEG and DPR. */
-static size_t calculate_traditional_mem_size(uintptr_t dram_base,
-		const struct device *dev)
+static size_t calculate_traditional_mem_size(uintptr_t dram_base)
 {
+	const struct device *idg_dev = dev_find_slot(0, SA_DEVFN_IGD);
    uintptr_t traditional_mem_base = dram_base;
    size_t traditional_mem_size;

-	if (dev->enabled) {
+	if (idg_dev && idg_dev->enabled) {
    	/* Read BDSM from Host Bridge */
    	traditional_mem_base -= sa_get_dsm_size();

@@ -200,14 +200,14 @@
  * Calculate Intel Reserved Memory size based on
  * PRMRR size, Trace Hub config and PTT selection.
  */
-static size_t calculate_reserved_mem_size(uintptr_t dram_base,
-		const struct device *dev)
+static size_t calculate_reserved_mem_size(uintptr_t dram_base)
 {
+	const struct device *root_dev = dev_find_slot(0, SA_DEVFN_ROOT);
    uintptr_t reserve_mem_base = dram_base;
    size_t reserve_mem_size;
    const struct soc_intel_skylake_config *config;

-	config = dev->chip_info;
+	config = root_dev->chip_info;

/* Get PRMRR size */
    reserve_mem_base -= get_prmrr_size(reserve_mem_base, config);
@@ -259,20 +259,15 @@
 static uintptr_t calculate_dram_base(size_t *reserved_mem_size)
 {
    uintptr_t dram_base;
-	const struct device *dev;
-
-	dev = dev_find_slot(0, PCI_DEVFN(SA_DEV_SLOT_IGD, 0));
-	if (!dev)
-		die("ERROR - IGD device not found!");

/* Read TOLUD from Host Bridge offset */
    dram_base = sa_get_tolud_base();

/* Get Intel Traditional Memory Range Size */
-	dram_base -= calculate_traditional_mem_size(dram_base, dev);
+	dram_base -= calculate_traditional_mem_size(dram_base);

/* Get Intel Reserved Memory Range Size */
-	*reserved_mem_size = calculate_reserved_mem_size(dram_base, dev);
+	*reserved_mem_size = calculate_reserved_mem_size(dram_base);

dram_base -= *reserved_mem_size;

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32614 )

Change subject: soc/skl/memmap: calculate mem size even if IGD undefined in devtree
......................................................................

Patch Set 1:

(3 comments)

https://review.coreboot.org/#/c/32614/1/src/soc/intel/skylake/memmap.c 
File src/soc/intel/skylake/memmap.c:

https://review.coreboot.org/#/c/32614/1/src/soc/intel/skylake/memmap.c@175 
PS1, Line 175: dev_find_slot
Please use pcidev_path_on_root().

We try to get rid of dev_find_slot() because its semantics are not
well defined before device enumeration in ramstage (every bus is 0
before that).

https://review.coreboot.org/#/c/32614/1/src/soc/intel/skylake/memmap.c@205 
PS1, Line 205: root_dev
`root_dev` reads a little odd here, because it's not the root of the
devicetree. Not your fault, I find `SA_DEVFN_ROOT` irritating as well.

https://review.coreboot.org/#/c/32614/1/src/soc/intel/skylake/memmap.c@205 
PS1, Line 205: dev_find_slot
Please use pcidev_path_on_root().

[Maxim Polyakov (Code Review)]

Hello Patrick Rudolph, Angel Pons, Paul Menzel, build bot (Jenkins), Nico Huber, Patrick Georgi,

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

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

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

Change subject: soc/skl/memmap: calculate mem size even if IGD undefined in devtree
......................................................................

soc/skl/memmap: calculate mem size even if IGD undefined in devtree

The DRAM base memory should be calculated even if IGD isn`t defined
in the board device tree

Tested on Asrock H110M-DVS

Change-Id: I3da51473e6c06da803bd969a4a6dff792c18f962
Signed-off-by: Maxim Polyakov 
---
M src/soc/intel/skylake/memmap.c
1 file changed, 7 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/32614/2

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32614 )

Change subject: soc/skl/memmap: calculate mem size even if IGD undefined in devtree
......................................................................

Patch Set 2: Code-Review+2

[Maxim Polyakov (Code Review)]

Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32614 )

Change subject: soc/skl/memmap: calculate mem size even if IGD undefined in devtree
......................................................................

Patch Set 2:

(3 comments)

https://review.coreboot.org/#/c/32614/1/src/soc/intel/skylake/memmap.c 
File src/soc/intel/skylake/memmap.c:

https://review.coreboot.org/#/c/32614/1/src/soc/intel/skylake/memmap.c@175 
PS1, Line 175: dev_find_slot
> Please use pcidev_path_on_root(). […]

Thank you for the review. I will use pcidev_path_on_root() from now on.

https://review.coreboot.org/#/c/32614/1/src/soc/intel/skylake/memmap.c@205 
PS1, Line 205: dev_find_slot
> Please use pcidev_path_on_root().

Done

https://review.coreboot.org/#/c/32614/1/src/soc/intel/skylake/memmap.c@205 
PS1, Line 205: root_dev
> `root_dev` reads a little odd here, because it's not the root of the […]

Ok. I use neutral dev here

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32614 )

Change subject: soc/skl/memmap: calculate mem size even if IGD undefined in devtree
......................................................................

Patch Set 2: Code-Review+2

(1 comment)

https://review.coreboot.org/#/c/32614/2/src/soc/intel/skylake/memmap.c 
File src/soc/intel/skylake/memmap.c:

https://review.coreboot.org/#/c/32614/2/src/soc/intel/skylake/memmap.c@175 
PS2, Line 175: idg
igd?

[Maxim Polyakov (Code Review)]

Hello Patrick Rudolph, Angel Pons, Paul Menzel, build bot (Jenkins), Nico Huber, Furquan Shaikh, Patrick Georgi,

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

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

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

Change subject: soc/skl/memmap: calculate mem size even if IGD undefined in devtree
......................................................................

soc/skl/memmap: calculate mem size even if IGD undefined in devtree

The DRAM base memory should be calculated even if IGD isn`t defined
in the board device tree

Tested on Asrock H110M-DVS

Change-Id: I3da51473e6c06da803bd969a4a6dff792c18f962
Signed-off-by: Maxim Polyakov 
---
M src/soc/intel/skylake/memmap.c
1 file changed, 7 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/32614/3

[Maxim Polyakov (Code Review)]

Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32614 )

Change subject: soc/skl/memmap: calculate mem size even if IGD undefined in devtree
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/#/c/32614/2/src/soc/intel/skylake/memmap.c 
File src/soc/intel/skylake/memmap.c:

https://review.coreboot.org/#/c/32614/2/src/soc/intel/skylake/memmap.c@175 
PS2, Line 175: idg
> igd?

Thank you for review. Perhaps at that moment I was thinking about "Integrated Discrete Graphics" :) but igd_dev looks better.

I renamed it to igd_dev

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32614 )

Change subject: soc/skl/memmap: calculate mem size even if IGD undefined in devtree
......................................................................

Patch Set 3: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32614 )

Change subject: soc/skl/memmap: calculate mem size even if IGD undefined in devtree
......................................................................

soc/skl/memmap: calculate mem size even if IGD undefined in devtree

The DRAM base memory should be calculated even if IGD isn`t defined
in the board device tree

Tested on Asrock H110M-DVS

Change-Id: I3da51473e6c06da803bd969a4a6dff792c18f962
Signed-off-by: Maxim Polyakov 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32614
Tested-by: build bot (Jenkins) 
Reviewed-by: Furquan Shaikh 
---
M src/soc/intel/skylake/memmap.c
1 file changed, 7 insertions(+), 12 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Furquan Shaikh: Looks good to me, approved

diff --git a/src/soc/intel/skylake/memmap.c b/src/soc/intel/skylake/memmap.c
index fde916a..ff7edbc 100644
--- a/src/soc/intel/skylake/memmap.c
+++ b/src/soc/intel/skylake/memmap.c
@@ -170,13 +170,13 @@
 }

/* Calculate Intel Traditional Memory size based on GSM, DSM, TSEG and DPR. */
-static size_t calculate_traditional_mem_size(uintptr_t dram_base,
-		const struct device *dev)
+static size_t calculate_traditional_mem_size(uintptr_t dram_base)
 {
+	const struct device *igd_dev = pcidev_path_on_root(SA_DEVFN_IGD);
    uintptr_t traditional_mem_base = dram_base;
    size_t traditional_mem_size;

-	if (dev->enabled) {
+	if (igd_dev && igd_dev->enabled) {
    	/* Read BDSM from Host Bridge */
    	traditional_mem_base -= sa_get_dsm_size();

@@ -200,9 +200,9 @@
  * Calculate Intel Reserved Memory size based on
  * PRMRR size, Trace Hub config and PTT selection.
  */
-static size_t calculate_reserved_mem_size(uintptr_t dram_base,
-		const struct device *dev)
+static size_t calculate_reserved_mem_size(uintptr_t dram_base)
 {
+	const struct device *dev = pcidev_path_on_root(SA_DEVFN_ROOT);
    uintptr_t reserve_mem_base = dram_base;
    size_t reserve_mem_size;
    const struct soc_intel_skylake_config *config;
@@ -259,20 +259,15 @@
 static uintptr_t calculate_dram_base(size_t *reserved_mem_size)
 {
    uintptr_t dram_base;
-	const struct device *dev;
-
-	dev = dev_find_slot(0, PCI_DEVFN(SA_DEV_SLOT_IGD, 0));
-	if (!dev)
-		die("ERROR - IGD device not found!");

/* Read TOLUD from Host Bridge offset */
    dram_base = sa_get_tolud_base();

/* Get Intel Traditional Memory Range Size */
-	dram_base -= calculate_traditional_mem_size(dram_base, dev);
+	dram_base -= calculate_traditional_mem_size(dram_base);

/* Get Intel Reserved Memory Range Size */
-	*reserved_mem_size = calculate_reserved_mem_size(dram_base, dev);
+	*reserved_mem_size = calculate_reserved_mem_size(dram_base);

dram_base -= *reserved_mem_size;