Change in coreboot[master]: soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Ronak Kanabar (Code Review)

4 Mar 2020 4 Mar '20

10:44 p.m.

Ronak Kanabar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check ......................................................................

soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check

coreboot was programming DDI_BUF_CTL in case GOP/VBIOS is not executed. When RUN_FSP_GOP config is selected, GOP will be executed and we don't need to program DDI_BUF_CTL. Hence moving the check before DDI_BUF_CTL programming

In case display is not connected, it may cause hang since GOP is also programming DDI_BUF_CTL before coreboot graphics_init call.

BUG=None BRANCH=None TEST=checked jslrvp and tglrvp compilation and boot.

Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/tigerlake/graphics.c 1 file changed, 12 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/39313/1

diff --git a/src/soc/intel/tigerlake/graphics.c b/src/soc/intel/tigerlake/graphics.c index c215384..86e9fca 100644 --- a/src/soc/intel/tigerlake/graphics.c +++ b/src/soc/intel/tigerlake/graphics.c @@ -45,6 +45,18 @@ return;

/* + * GFX PEIM module inside FSP binary is taking care of graphics + * initialization based on RUN_FSP_GOP Kconfig + * option and input VBT file. Hence no need to load/execute legacy VGA + * OpROM in order to initialize GFX. + * + * In case of non-FSP solution, SoC need to select VGA_ROM_RUN + * Kconfig to perform GFX initialization through VGA OpRom. + */ + if (CONFIG(RUN_FSP_GOP)) + return; + + /* * Enable DDI-A (eDP) 4-lane operation if the link is not up yet. * This will allow the kernel to use 4-lane eDP links properly * if the VBIOS or GOP driver do not execute. @@ -56,18 +68,6 @@ graphics_gtt_write(DDI_BUF_CTL_A, ddi_buf_ctl); }

- /* - * GFX PEIM module inside FSP binary is taking care of graphics - * initialization based on RUN_FSP_GOP Kconfig - * option and input VBT file. Hence no need to load/execute legacy VGA - * OpROM in order to initialize GFX. - * - * In case of non-FSP solution, SoC need to select VGA_ROM_RUN - * Kconfig to perform GFX initialization through VGA OpRom. - */ - if (CONFIG(RUN_FSP_GOP)) - return; - /* IGD needs to Bus Master */ uint32_t reg32 = pci_read_config32(dev, PCI_COMMAND); reg32 |= PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO;

Attachments:

attachment.htm (text/html — 5.1 KB)

Show replies by date

build bot (Jenkins) (Code Review)

4 Mar 4 Mar

10:45 p.m.

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check ......................................................................

Patch Set 1:

(10 comments)

https://review.coreboot.org/c/coreboot/+/39313/1/src/soc/intel/tigerlake/gra... File src/soc/intel/tigerlake/graphics.c:

https://review.coreboot.org/c/coreboot/+/39313/1/src/soc/intel/tigerlake/gra... PS1, Line 48: * GFX PEIM module inside FSP binary is taking care of graphics code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/39313/1/src/soc/intel/tigerlake/gra... PS1, Line 49: * initialization based on RUN_FSP_GOP Kconfig code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/39313/1/src/soc/intel/tigerlake/gra... PS1, Line 50: * option and input VBT file. Hence no need to load/execute legacy VGA code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/39313/1/src/soc/intel/tigerlake/gra... PS1, Line 51: * OpROM in order to initialize GFX. code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/39313/1/src/soc/intel/tigerlake/gra... PS1, Line 52: * code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/39313/1/src/soc/intel/tigerlake/gra... PS1, Line 53: * In case of non-FSP solution, SoC need to select VGA_ROM_RUN code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/39313/1/src/soc/intel/tigerlake/gra... PS1, Line 54: * Kconfig to perform GFX initialization through VGA OpRom. code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/39313/1/src/soc/intel/tigerlake/gra... PS1, Line 55: */ code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/39313/1/src/soc/intel/tigerlake/gra... PS1, Line 57: return; code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/39313/1/src/soc/intel/tigerlake/gra... PS1, Line 57: return; please, no spaces at the start of a line

Ronak Kanabar (Code Review)

10:51 p.m.

Hello build bot (Jenkins), Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check ......................................................................

soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check

In case display is not connected, it may cause hang since GOP is also programming DDI_BUF_CTL before coreboot graphics_init call.

BUG=None BRANCH=None TEST=checked jslrvp and tglrvp compilation and boot.

Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/tigerlake/graphics.c 1 file changed, 12 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/39313/2

build bot (Jenkins) (Code Review)

10:51 p.m.

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check ......................................................................

Patch Set 2:

(10 comments)

https://review.coreboot.org/c/coreboot/+/39313/2/src/soc/intel/tigerlake/gra... File src/soc/intel/tigerlake/graphics.c:

https://review.coreboot.org/c/coreboot/+/39313/2/src/soc/intel/tigerlake/gra... PS2, Line 48: * GFX PEIM module inside FSP binary is taking care of graphics code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/39313/2/src/soc/intel/tigerlake/gra... PS2, Line 49: * initialization based on RUN_FSP_GOP Kconfig code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/39313/2/src/soc/intel/tigerlake/gra... PS2, Line 50: * option and input VBT file. Hence no need to load/execute legacy VGA code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/39313/2/src/soc/intel/tigerlake/gra... PS2, Line 51: * OpROM in order to initialize GFX. code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/39313/2/src/soc/intel/tigerlake/gra... PS2, Line 52: * code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/39313/2/src/soc/intel/tigerlake/gra... PS2, Line 53: * In case of non-FSP solution, SoC need to select VGA_ROM_RUN code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/39313/2/src/soc/intel/tigerlake/gra... PS2, Line 54: * Kconfig to perform GFX initialization through VGA OpRom. code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/39313/2/src/soc/intel/tigerlake/gra... PS2, Line 55: */ code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/39313/2/src/soc/intel/tigerlake/gra... PS2, Line 57: return; code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/39313/2/src/soc/intel/tigerlake/gra... PS2, Line 57: return; please, no spaces at the start of a line

Ronak Kanabar (Code Review)

10:55 p.m.

Hello build bot (Jenkins), Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check ......................................................................

soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check

In case display is not connected, it may cause hang since GOP is also programming DDI_BUF_CTL before coreboot graphics_init call.

BUG=None BRANCH=None TEST=checked jslrvp and tglrvp compilation and boot.

Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/tigerlake/graphics.c 1 file changed, 12 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/39313/3

Karthik Ramasubramanian (Code Review)

11:25 p.m.

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39313/3/src/soc/intel/tigerlake/gra... File src/soc/intel/tigerlake/graphics.c:

https://review.coreboot.org/c/coreboot/+/39313/3/src/soc/intel/tigerlake/gra... PS3, Line 69: } Is it applicable only for JSL. I see CNL is also doing the same.

Paul Menzel (Code Review)

5 Mar 5 Mar

6:25 a.m.

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check ......................................................................

Patch Set 3:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39313/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/39313/3//COMMIT_MSG@12 PS3, Line 12: moving move

https://review.coreboot.org/c/coreboot/+/39313/3//COMMIT_MSG@12 PS3, Line 12: Hence moving the check before DDI_BUF_CTL programming Dot/period at the end of sentences.

Ronak Kanabar (Code Review)

7:25 a.m.

Hello build bot (Jenkins), Furquan Shaikh, Nick Vaccaro, Aamir Bohra, Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check ......................................................................

soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check

In case display is not connected, it may cause hang since GOP is also programming DDI_BUF_CTL before coreboot graphics_init call.

BUG=None BRANCH=None TEST=checked jslrvp and tglrvp compilation and boot.

Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/tigerlake/graphics.c 1 file changed, 12 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/39313/4

Ronak Kanabar (Code Review)

7:25 a.m.

Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check ......................................................................

Patch Set 4:

(2 comments)

...

Patch Set 1:

(10 comments)

https://review.coreboot.org/c/coreboot/+/39313/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/39313/3//COMMIT_MSG@12 PS3, Line 12: Hence moving the check before DDI_BUF_CTL programming

...

Dot/period at the end of sentences.

Ack

https://review.coreboot.org/c/coreboot/+/39313/3//COMMIT_MSG@12 PS3, Line 12: moving

...

move

Ack

Ronak Kanabar (Code Review)

7:34 a.m.

Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39313/3/src/soc/intel/tigerlake/gra... File src/soc/intel/tigerlake/graphics.c:

https://review.coreboot.org/c/coreboot/+/39313/3/src/soc/intel/tigerlake/gra... PS3, Line 69: }

...

Is it applicable only for JSL. I see CNL is also doing the same.

we observed hang when Display is not connected in JSL. Because GOP is also programming "ddi_buf_ctl" we need to check that why we didn't face this problem in previous platforms.

But as description suggest this should execute if VBIOS or GOP do not execute.

Furquan Shaikh (Code Review)

6 Mar 6 Mar

8:01 a.m.

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

Change subject: soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39313/3/src/soc/intel/tigerlake/gra... File src/soc/intel/tigerlake/graphics.c:

https://review.coreboot.org/c/coreboot/+/39313/3/src/soc/intel/tigerlake/gra... PS3, Line 69: } I am not convinced this is correct. At least in the past, this had to be done to ensure that the kernel can use 4-lane eDP in verified boot mode i.e. when GOP does not execute. In fact, your comment above already says that: "This will allow the kernel to use 4-lane eDP links properly if the VBIOS or GOP driver do not execute.". I am not sure how the latest kernel behaves, but if the behavior is still the same on kernel side, then by making this change, you will be impacting verified mode where GOP does not execute.

...

we observed hang when Display is not connected in JSL.

Where does the hang occur? In coreboot? In FSP? Have you debugged what operation is causing that hang? And is this a hard hang? an assert?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 4 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 06 Mar 2020 16:01:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Ronak Kanabar ronak.kanabar@intel.com Comment-In-Reply-To: Karthik Ramasubramanian kramasub@google.com Gerrit-MessageType: comment

Ronak Kanabar (Code Review)

9:50 a.m.

Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39313/3/src/soc/intel/tigerlake/gra... File src/soc/intel/tigerlake/graphics.c:

https://review.coreboot.org/c/coreboot/+/39313/3/src/soc/intel/tigerlake/gra... PS3, Line 69: }

...

I am not convinced this is correct. […]

Where does the hang occur? In coreboot? In FSP? Have you debugged what operation is causing that hang? And is this a hard hang? an assert?

...

...
...
When EDP display in not connected we are facing CATERR at "PCI: 00:02.0 init" In coreboot.

We will come up with more data on this CATERR.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 4 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 06 Mar 2020 17:50:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Comment-In-Reply-To: Ronak Kanabar ronak.kanabar@intel.com Comment-In-Reply-To: Karthik Ramasubramanian kramasub@google.com Gerrit-MessageType: comment

Paul Menzel (Code Review)

20 Mar 20 Mar

9:36 a.m.

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check ......................................................................

Patch Set 6:

(3 comments)

https://review.coreboot.org/c/coreboot/+/39313/6//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/39313/6//COMMIT_MSG@11 PS6, Line 11: be executed and we don't need to program DDI_BUF_CTL. Why? Who programs it?

https://review.coreboot.org/c/coreboot/+/39313/6//COMMIT_MSG@12 PS6, Line 12: Hence move the check before DDI_BUF_CTL programming. Hence fits on the line above.

https://review.coreboot.org/c/coreboot/+/39313/6//COMMIT_MSG@14 PS6, Line 14: it Before or after?

Furquan Shaikh (Code Review)

1:09 p.m.

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

Change subject: soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check ......................................................................

Patch Set 6:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39313/3/src/soc/intel/tigerlake/gra... File src/soc/intel/tigerlake/graphics.c:

https://review.coreboot.org/c/coreboot/+/39313/3/src/soc/intel/tigerlake/gra... PS3, Line 69: }

...

Where does the hang occur? In coreboot? In FSP? Have you debugged what operation is causing that han […]

Any more data here? Also, is there a bug associated with this? Can you please update the bug with debug details?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 6 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 20 Mar 2020 20:09:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Comment-In-Reply-To: Ronak Kanabar ronak.kanabar@intel.com Comment-In-Reply-To: Karthik Ramasubramanian kramasub@google.com Gerrit-MessageType: comment

Ronak Kanabar (Code Review)

25 Mar 25 Mar

10:38 p.m.

Hello build bot (Jenkins), Furquan Shaikh, Maulik V Vaghela, Nick Vaccaro, Aamir Bohra, Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check ......................................................................

soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check

In case display is not connected, it may cause hang since GOP is also programming DDI_BUF_CTL before coreboot graphics_init call.

BUG=b:150788968 BRANCH=None TEST=checked jslrvp and tglrvp compilation and boot.

Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/tigerlake/graphics.c 1 file changed, 12 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/39313/8

Ronak Kanabar (Code Review)

26 Mar 26 Mar

12:26 a.m.

Hello build bot (Jenkins), Furquan Shaikh, Maulik V Vaghela, Nick Vaccaro, Aamir Bohra, Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check ......................................................................

soc/intel/tigerlake: Correct order of "RUN_FSP_GOP" check

In case display is not connected, it may cause hang since GOP is also programming DDI_BUF_CTL before coreboot graphics_init call.

BUG=b:150788968 BRANCH=None TEST=checked jslrvp and tglrvp compilation and boot.

Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/tigerlake/graphics.c 1 file changed, 0 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/39313/9

Ronak Kanabar (Code Review)

12:33 a.m.

New subject: Change in coreboot[master]: soc/intel/tigerlake: Remove redundant code from graphics.c

Hello build bot (Jenkins), Furquan Shaikh, Maulik V Vaghela, Nick Vaccaro, Aamir Bohra, Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: Remove redundant code from graphics.c ......................................................................

soc/intel/tigerlake: Remove redundant code from graphics.c

It looks like kernel was relying on BIOS setting DDI_A_4_LANES when it was doing the max lane calculation if platform has older graphics

With newer graphics, portA and portE no longer seem to share the lanes. Hence, kernel assumes that eDP is using 4-lanes. So kernel driver for these platforms relies on BIOS setting DDI_A_4_LANES for it.Hence removing this code from coreboot

BUG=b:150788968 BRANCH=None TEST=checked jslrvp and tglrvp compilation and boot.

Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/tigerlake/graphics.c 1 file changed, 0 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/39313/10

Ronak Kanabar (Code Review)

12:43 a.m.

New subject: Change in coreboot[master]: soc/intel/tigerlake: Remove redundant code from graphics.c

Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Maulik V Vaghela, Nick Vaccaro, Aamir Bohra, Srinidhi N Kaushik, Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: Remove redundant code from graphics.c ......................................................................

soc/intel/tigerlake: Remove redundant code from graphics.c

It looks like kernel was relying on BIOS setting DDI_A_4_LANES when it was doing the max lane calculation if platform has older graphics

BUG=b:150788968 BRANCH=None TEST=checked jslrvp and tglrvp compilation and boot.

Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/tigerlake/graphics.c 1 file changed, 0 insertions(+), 13 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/39313/12

Ronak Kanabar (Code Review)

1:06 a.m.

New subject: Change in coreboot[master]: soc/intel/tigerlake: Remove redundant code from graphics.c

Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Maulik V Vaghela, Nick Vaccaro, Aamir Bohra, Srinidhi N Kaushik, Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: Remove redundant code from graphics.c ......................................................................

soc/intel/tigerlake: Remove redundant code from graphics.c

For newer Intel graphics(>11), The DDI port max lanes are set to 4 by default. And kernel driver no longer relies on coreboot to provide information via DDI_BUF_CTL_A(for DDI port A) register programming. Hence removing this code.

BUG=b:150788968 BRANCH=None TEST=checked jslrvp and tglrvp compilation and boot.

Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/tigerlake/graphics.c 1 file changed, 0 insertions(+), 13 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/39313/13

Ronak Kanabar (Code Review)

4 a.m.

New subject: Change in coreboot[master]: soc/intel/tigerlake: Remove redundant code from graphics.c

Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/tigerlake: Remove redundant code from graphics.c ......................................................................

Patch Set 13:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39313/3/src/soc/intel/tigerlake/gra... File src/soc/intel/tigerlake/graphics.c:

https://review.coreboot.org/c/coreboot/+/39313/3/src/soc/intel/tigerlake/gra... PS3, Line 69: }

...

will update bug by end of day tomorrow. Discussion Going on with GOP team.

as per discussion i have removed redundant code.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 13 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 26 Mar 2020 11:00:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Comment-In-Reply-To: Ronak Kanabar ronak.kanabar@intel.com Comment-In-Reply-To: Karthik Ramasubramanian kramasub@google.com Gerrit-MessageType: comment

Ronak Kanabar (Code Review)

4:01 a.m.

New subject: Change in coreboot[master]: soc/intel/tigerlake: Remove redundant code from graphics.c

Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/tigerlake: Remove redundant code from graphics.c ......................................................................

Patch Set 13:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39313/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/39313/3//COMMIT_MSG@12 PS3, Line 12: moving

...

Ack

changed commit message according to new changes

https://review.coreboot.org/c/coreboot/+/39313/3//COMMIT_MSG@12 PS3, Line 12: Hence moving the check before DDI_BUF_CTL programming

...

Ack

changed commit message according to new changes

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 13 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 26 Mar 2020 11:01:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Comment-In-Reply-To: Ronak Kanabar ronak.kanabar@intel.com Gerrit-MessageType: comment

Ronak Kanabar (Code Review)

4:03 a.m.

New subject: Change in coreboot[master]: soc/intel/tigerlake: Remove redundant code from graphics.c

Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/tigerlake: Remove redundant code from graphics.c ......................................................................

Patch Set 13:

(3 comments)

https://review.coreboot.org/c/coreboot/+/39313/6//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/39313/6//COMMIT_MSG@11 PS6, Line 11: be executed and we don't need to program DDI_BUF_CTL.

...

Why? Who programs it?

changed commit message according to new changes

https://review.coreboot.org/c/coreboot/+/39313/6//COMMIT_MSG@12 PS6, Line 12: Hence move the check before DDI_BUF_CTL programming.

...

Hence fits on the line above.

changed commit message according to new changes

https://review.coreboot.org/c/coreboot/+/39313/6//COMMIT_MSG@14 PS6, Line 14: it

...

Before or after?

changed commit message according to new changes

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 13 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 26 Mar 2020 11:03:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: comment

Aamir Bohra (Code Review)

4:58 a.m.

New subject: Change in coreboot[master]: soc/intel/tigerlake: Remove redundant code from graphics.c

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/tigerlake: Remove redundant code from graphics.c ......................................................................

Patch Set 13:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39313/13//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/39313/13//COMMIT_MSG@7 PS13, Line 7: Remove redundant code from graphics.c Remove DDI A lane programming?

https://review.coreboot.org/c/coreboot/+/39313/13/src/soc/intel/tigerlake/gr... File src/soc/intel/tigerlake/graphics.c:

https://review.coreboot.org/c/coreboot/+/39313/13/src/soc/intel/tigerlake/gr... PS13, Line 28: #include <drivers/intel/gma/i915_reg.h> not needed now?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 13 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-Comment-Date: Thu, 26 Mar 2020 11:58:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Nico Huber (Code Review)

8:17 a.m.

New subject: Change in coreboot[master]: soc/intel/tigerlake: Remove redundant code from graphics.c

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

Change subject: soc/intel/tigerlake: Remove redundant code from graphics.c ......................................................................

Patch Set 13:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39313/13//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/39313/13//COMMIT_MSG@9 PS13, Line 9: (>11) Isn't this the case since Gen 10? Should this be changed in older copies of this code, too?

In the Graphics PRM for Ice Lake, the bit is must-be-zero. I don't know a reference for Cannon Lake, but the eDP-bifurcation chapter was removed from its EDS...

https://review.coreboot.org/c/coreboot/+/39313/13/src/soc/intel/tigerlake/gr... File src/soc/intel/tigerlake/graphics.c:

https://review.coreboot.org/c/coreboot/+/39313/13/src/soc/intel/tigerlake/gr... PS13, Line 41: /* Skip IGD GT programming */ : if (CONFIG(SKIP_GRAPHICS_ENABLING)) : return; As there is no more code below that would access GT registers, should this be dropped, too?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 13 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-Comment-Date: Thu, 26 Mar 2020 15:17:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Ronak Kanabar (Code Review)

27 Mar 27 Mar

4:36 a.m.

New subject: Change in coreboot[master]: soc/intel/tigerlake: Remove redundant code from graphics.c

Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/tigerlake: Remove redundant code from graphics.c ......................................................................

Patch Set 13:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39313/13//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/39313/13//COMMIT_MSG@9 PS13, Line 9: (>11)

...

Isn't this the case since Gen 10? Should this be changed in older […]

ACK. same applies for ICL will add it here

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 13 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-Comment-Date: Fri, 27 Mar 2020 11:36:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

Ronak Kanabar (Code Review)

4:37 a.m.

New subject: Change in coreboot[master]: soc/intel/tigerlake: Remove DDI A lane programming

Hello build bot (Jenkins), Nico Huber, Furquan Shaikh, Wonkyu Kim, Maulik V Vaghela, Nick Vaccaro, Aamir Bohra, Srinidhi N Kaushik, Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: Remove DDI A lane programming ......................................................................

soc/intel/tigerlake: Remove DDI A lane programming

BUG=b:150788968 BRANCH=None TEST=checked jslrvp and tglrvp compilation and boot.

Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/tigerlake/graphics.c 1 file changed, 0 insertions(+), 13 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/39313/14

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 14 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-MessageType: newpatchset

Ronak Kanabar (Code Review)

4:37 a.m.

New subject: Change in coreboot[master]: soc/intel/tigerlake: Remove DDI A lane programming

Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/tigerlake: Remove DDI A lane programming ......................................................................

Patch Set 14:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39313/13//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/39313/13//COMMIT_MSG@7 PS13, Line 7: Remove redundant code from graphics.c

...

Remove DDI A lane programming?

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 14 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-Comment-Date: Fri, 27 Mar 2020 11:37:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Aamir Bohra aamir.bohra@intel.com Gerrit-MessageType: comment

Ronak Kanabar (Code Review)

4:44 a.m.

New subject: Change in coreboot[master]: soc/intel/tigerlake: Remove DDI A lane programming

Hello build bot (Jenkins), Nico Huber, Furquan Shaikh, Wonkyu Kim, Maulik V Vaghela, Nick Vaccaro, Aamir Bohra, Srinidhi N Kaushik, Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: Remove DDI A lane programming ......................................................................

soc/intel/tigerlake: Remove DDI A lane programming

BUG=b:150788968 BRANCH=None TEST=checked jslrvp and tglrvp compilation and boot.

Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/tigerlake/graphics.c 1 file changed, 0 insertions(+), 19 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/39313/15

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 15 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-MessageType: newpatchset

Ronak Kanabar (Code Review)

4:45 a.m.

New subject: Change in coreboot[master]: soc/intel/tigerlake: Remove DDI A lane programming

Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/tigerlake: Remove DDI A lane programming ......................................................................

Patch Set 15:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39313/13/src/soc/intel/tigerlake/gr... File src/soc/intel/tigerlake/graphics.c:

https://review.coreboot.org/c/coreboot/+/39313/13/src/soc/intel/tigerlake/gr... PS13, Line 28: #include <drivers/intel/gma/i915_reg.h>

...

not needed now?

Done

https://review.coreboot.org/c/coreboot/+/39313/13/src/soc/intel/tigerlake/gr... PS13, Line 41: /* Skip IGD GT programming */ : if (CONFIG(SKIP_GRAPHICS_ENABLING)) : return;

...

As there is no more code below that would access GT registers, […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 15 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-Comment-Date: Fri, 27 Mar 2020 11:45:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Aamir Bohra aamir.bohra@intel.com Gerrit-MessageType: comment

Ronak Kanabar (Code Review)

4:54 a.m.

New subject: Change in coreboot[master]: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming

Hello build bot (Jenkins), Nico Huber, Furquan Shaikh, Wonkyu Kim, Maulik V Vaghela, Nick Vaccaro, Aamir Bohra, Srinidhi N Kaushik, Patrick Rudolph,

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

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

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

Change subject: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming ......................................................................

soc/intel/{icelake, tigerlake}: Remove DDI A lane programming

For newer Intel graphics(>10), The DDI port max lanes are set to 4 by default. And kernel driver no longer relies on coreboot to provide information via DDI_BUF_CTL_A(for DDI port A) register programming. Hence removing this code.

BUG=b:150788968 BRANCH=None TEST=checked jslrvp and tglrvp compilation and boot.

Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/icelake/graphics.c M src/soc/intel/tigerlake/graphics.c 2 files changed, 0 insertions(+), 38 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/39313/16

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 16 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-MessageType: newpatchset

Ronak Kanabar (Code Review)

4:55 a.m.

New subject: Change in coreboot[master]: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming

Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming ......................................................................

Patch Set 16:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39313/13//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/39313/13//COMMIT_MSG@9 PS13, Line 9: (>11)

...

ACK. […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 16 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-Comment-Date: Fri, 27 Mar 2020 11:55:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Ronak Kanabar ronak.kanabar@intel.com Gerrit-MessageType: comment

Ronak Kanabar (Code Review)

5:19 a.m.

New subject: Change in coreboot[master]: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming

Hello build bot (Jenkins), Nico Huber, Furquan Shaikh, Wonkyu Kim, Maulik V Vaghela, Nick Vaccaro, Aamir Bohra, Srinidhi N Kaushik, Patrick Rudolph,

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

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

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

Change subject: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming ......................................................................

soc/intel/{icelake, tigerlake}: Remove DDI A lane programming

For newer Intel graphics(>=11), The DDI port max lanes are set to 4 by default. And kernel driver no longer relies on coreboot to provide information via DDI_BUF_CTL_A(for DDI port A) register programming. Hence removing this code.

BUG=b:150788968 BRANCH=None TEST=checked jslrvp and tglrvp compilation and boot.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/39313/17

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 17 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-MessageType: newpatchset

Nico Huber (Code Review)

6:11 a.m.

New subject: Change in coreboot[master]: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming

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

Change subject: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming ......................................................................

Patch Set 17: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/39313/13/src/soc/intel/tigerlake/gr... File src/soc/intel/tigerlake/graphics.c:

https://review.coreboot.org/c/coreboot/+/39313/13/src/soc/intel/tigerlake/gr... PS13, Line 41: /* Skip IGD GT programming */ : if (CONFIG(SKIP_GRAPHICS_ENABLING)) : return;

...

Done

Thanks.

Now the definition in src/soc/intel/common/block/graphics/Kconfig:6 seems to be unused? I guess it should be removed, but there is another stale copy of this code in Gerrit for Jasper Lake. So this would need some coordination.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 17 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-Comment-Date: Fri, 27 Mar 2020 13:11:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Ronak Kanabar ronak.kanabar@intel.com Gerrit-MessageType: comment

Aamir Bohra (Code Review)

6:35 a.m.

New subject: Change in coreboot[master]: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming ......................................................................

Patch Set 17: Code-Review+2

Can you please add a topic as "jsl_port", will remove the common Kconfig once we remove it from Jasper Lake also.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 17 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-Comment-Date: Fri, 27 Mar 2020 13:35:32 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Wonkyu Kim (Code Review)

9:29 a.m.

New subject: Change in coreboot[master]: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming

Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming ......................................................................

Patch Set 17: Code-Review+1

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 17 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.corp-partner.google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-Comment-Date: Fri, 27 Mar 2020 16:29:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Nick Vaccaro (Code Review)

12:18 p.m.

New subject: Change in coreboot[master]: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming

Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming ......................................................................

Patch Set 17: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 17 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.corp-partner.google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-Comment-Date: Fri, 27 Mar 2020 19:18:05 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Maulik V Vaghela (Code Review)

30 Mar 30 Mar

12:14 a.m.

New subject: Change in coreboot[master]: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming

Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming ......................................................................

Patch Set 17: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 17 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.corp-partner.google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-Comment-Date: Mon, 30 Mar 2020 07:14:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Patrick Georgi (Code Review)

1:52 a.m.

New subject: Change in coreboot[master]: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming ......................................................................

Patch Set 17:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39313/13/src/soc/intel/tigerlake/gr... File src/soc/intel/tigerlake/graphics.c:

https://review.coreboot.org/c/coreboot/+/39313/13/src/soc/intel/tigerlake/gr... PS13, Line 41: /* Skip IGD GT programming */ : if (CONFIG(SKIP_GRAPHICS_ENABLING)) : return;

...

Thanks. […]

This seems to be an open issue still. Does this need discussion, a change to this commit, a follow up commit or is it good as is it?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 17 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.corp-partner.google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-Comment-Date: Mon, 30 Mar 2020 08:52:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Ronak Kanabar ronak.kanabar@intel.com Gerrit-MessageType: comment

Ronak Kanabar (Code Review)

2:19 a.m.

New subject: Change in coreboot[master]: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming

Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming ......................................................................

Patch Set 17:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39313/13/src/soc/intel/tigerlake/gr... File src/soc/intel/tigerlake/graphics.c:

https://review.coreboot.org/c/coreboot/+/39313/13/src/soc/intel/tigerlake/gr... PS13, Line 41: /* Skip IGD GT programming */ : if (CONFIG(SKIP_GRAPHICS_ENABLING)) : return;

...

This seems to be an open issue still. […]

Done. for Kconfig will push other CL is needed.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 17 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.corp-partner.google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-Comment-Date: Mon, 30 Mar 2020 09:19:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Patrick Georgi pgeorgi@google.com Comment-In-Reply-To: Ronak Kanabar ronak.kanabar@intel.com Gerrit-MessageType: comment

Aamir Bohra (Code Review)

2:29 a.m.

New subject: Change in coreboot[master]: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming ......................................................................

Patch Set 17:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39313/13/src/soc/intel/tigerlake/gr... File src/soc/intel/tigerlake/graphics.c:

https://review.coreboot.org/c/coreboot/+/39313/13/src/soc/intel/tigerlake/gr... PS13, Line 41: /* Skip IGD GT programming */ : if (CONFIG(SKIP_GRAPHICS_ENABLING)) : return;

...

Done. for Kconfig will push other CL is needed.

can you please add jsl_port topic.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 17 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.corp-partner.google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-Comment-Date: Mon, 30 Mar 2020 09:29:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Patrick Georgi pgeorgi@google.com Comment-In-Reply-To: Ronak Kanabar ronak.kanabar@intel.com Gerrit-MessageType: comment

Ronak Kanabar (Code Review)

2:31 a.m.

New subject: Change in coreboot[master]: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming

Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming ......................................................................

Patch Set 17:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39313/13/src/soc/intel/tigerlake/gr... File src/soc/intel/tigerlake/graphics.c:

https://review.coreboot.org/c/coreboot/+/39313/13/src/soc/intel/tigerlake/gr... PS13, Line 28: #include <drivers/intel/gma/i915_reg.h>

...

Done

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 17 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.corp-partner.google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-Comment-Date: Mon, 30 Mar 2020 09:31:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Ronak Kanabar ronak.kanabar@intel.com Comment-In-Reply-To: Aamir Bohra aamir.bohra@intel.com Gerrit-MessageType: comment

Patrick Georgi (Code Review)

2:42 a.m.

New subject: Change in coreboot[master]: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming ......................................................................

soc/intel/{icelake, tigerlake}: Remove DDI A lane programming

BUG=b:150788968 BRANCH=None TEST=checked jslrvp and tglrvp compilation and boot.

Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39313 Reviewed-by: Nico Huber nico.h@gmx.de Reviewed-by: Aamir Bohra aamir.bohra@intel.com Reviewed-by: Wonkyu Kim wonkyu.kim@intel.corp-partner.google.com Reviewed-by: Nick Vaccaro nvaccaro@google.com Reviewed-by: Maulik V Vaghela maulik.v.vaghela@intel.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/icelake/graphics.c M src/soc/intel/tigerlake/graphics.c 2 files changed, 0 insertions(+), 38 deletions(-)

Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, but someone else must approve Aamir Bohra: Looks good to me, approved Nick Vaccaro: Looks good to me, approved Maulik V Vaghela: Looks good to me, approved Wonkyu Kim: Looks good to me, but someone else must approve

diff --git a/src/soc/intel/icelake/graphics.c b/src/soc/intel/icelake/graphics.c index 4f5d573..0ee340c 100644 --- a/src/soc/intel/icelake/graphics.c +++ b/src/soc/intel/icelake/graphics.c @@ -19,7 +19,6 @@ #include <device/device.h> #include <device/pci.h> #include <device/pci_ops.h> -#include <drivers/intel/gma/i915_reg.h> #include <drivers/intel/gma/opregion.h> #include <intelblocks/graphics.h> #include <types.h> @@ -31,24 +30,6 @@

void graphics_soc_init(struct device *dev) { - uint32_t ddi_buf_ctl; - - /* Skip IGD GT programming */ - if (CONFIG(SKIP_GRAPHICS_ENABLING)) - return; - - /* - * Enable DDI-A (eDP) 4-lane operation if the link is not up yet. - * This will allow the kernel to use 4-lane eDP links properly - * if the VBIOS or GOP driver do not execute. - */ - ddi_buf_ctl = graphics_gtt_read(DDI_BUF_CTL_A); - if (!acpi_is_wakeup_s3() && !(ddi_buf_ctl & DDI_BUF_CTL_ENABLE)) { - ddi_buf_ctl |= (DDI_A_4_LANES | DDI_INIT_DISPLAY_DETECTED | - DDI_BUF_IS_IDLE); - graphics_gtt_write(DDI_BUF_CTL_A, ddi_buf_ctl); - } - /* * GFX PEIM module inside FSP binary is taking care of graphics * initialization based on RUN_FSP_GOP Kconfig diff --git a/src/soc/intel/tigerlake/graphics.c b/src/soc/intel/tigerlake/graphics.c index fef17e1..4054bd5 100644 --- a/src/soc/intel/tigerlake/graphics.c +++ b/src/soc/intel/tigerlake/graphics.c @@ -25,7 +25,6 @@ #include <device/device.h> #include <device/pci.h> #include <device/pci_ops.h> -#include <drivers/intel/gma/i915_reg.h> #include <drivers/intel/gma/opregion.h> #include <intelblocks/graphics.h> #include <types.h> @@ -37,24 +36,6 @@

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 18 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.corp-partner.google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-MessageType: merged

9elements QA (Code Review)

2:57 a.m.

New subject: Change in coreboot[master]: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39313 )

Change subject: soc/intel/{icelake, tigerlake}: Remove DDI A lane programming ......................................................................

Patch Set 18:

Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/1918 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1917 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1916

Please note: This test is under development and might not be accurate at all!

-- To view, visit https://review.coreboot.org/c/coreboot/+/39313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32692501b60f48a07b8fbb9bb3a755b18f4b3ea9 Gerrit-Change-Number: 39313 Gerrit-PatchSet: 18 Gerrit-Owner: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.com Gerrit-Reviewer: Wonkyu Kim wonkyu.kim@intel.corp-partner.google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: 9elements QA hardwaretestrobot@gmail.com Gerrit-CC: Karthik Ramasubramanian kramasub@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Subrata Banik subrata.banik@intel.com Gerrit-Comment-Date: Mon, 30 Mar 2020 09:57:53 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1692

days inactive

1717

days old

coreboot-gerrit@coreboot.org

42 comments

12 participants

tags (0)

participants (12)

9elements QA (Code Review)
Aamir Bohra (Code Review)
build bot (Jenkins) (Code Review)
Furquan Shaikh (Code Review)
Karthik Ramasubramanian (Code Review)
Maulik V Vaghela (Code Review)
Nick Vaccaro (Code Review)
Nico Huber (Code Review)
Patrick Georgi (Code Review)
Paul Menzel (Code Review)
Ronak Kanabar (Code Review)
Wonkyu Kim (Code Review)