Change in ...coreboot[master]: vboot: communicate display requirements with vb2api_fw_phase1

[Joel Kitching (Code Review)]

Joel Kitching has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32324

Change subject: vboot: communicate display requirements with vb2api_fw_phase1
......................................................................

vboot: communicate display requirements with vb2api_fw_phase1

Input: tell vb2api_fw_phase1 if display unconditionally available
Output: vb2api_fw_phase1 tells coreboot if it shall declare
        display as being available, based on some internal request

Move the vboot_set_declares_display call into verstage_main.

BUG=b:124141368, b:124192753, chromium:948529
TEST=make clean && make test-abuild
CQ-DEPEND=CL:1564232
BRANCH=none

Change-Id: I81c82c46303564b63b8a32e7f80beb9d891a4628
Cq-Depend: chromium:1564232
Signed-off-by: Joel Kitching 
---
M src/security/vboot/vboot_handoff.c
M src/security/vboot/vboot_logic.c
2 files changed, 13 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/32324/1

diff --git a/src/security/vboot/vboot_handoff.c b/src/security/vboot/vboot_handoff.c
index 85b1611..ddf0a78 100644
--- a/src/security/vboot/vboot_handoff.c
+++ b/src/security/vboot/vboot_handoff.c
@@ -76,14 +76,11 @@
    	vb_sd->flags |= VBSD_BOOT_DEV_SWITCH_ON;
    	vb_sd->flags |= VBSD_LF_DEV_SWITCH_ON;
    }
-	/* Inform vboot if the display was requested by vboot kernel phase
-	   or enabled by dev/rec mode. */
+	/* TODO(chromium:948529): Remove these two flags after downstream
+	   vboot code longer reads them. */
    if (vboot_wants_oprom() || vb2_sd->recovery_reason ||
-	    vb2_sd->flags & VB2_SD_FLAG_DEV_MODE_ENABLED) {
-		vboot_get_working_data()->flags |= VBOOT_FLAG_DISPLAY_REQUESTED;
+	    vb2_sd->flags & VB2_SD_FLAG_DEV_MODE_ENABLED)
    	vb_sd->flags |= VBSD_OPROM_LOADED;
-	}
-	/* TODO: Remove when depthcharge no longer reads this flag. */
    if (CONFIG(VBOOT_MUST_REQUEST_DISPLAY))
    	vb_sd->flags |= VBSD_OPROM_MATTERS;

diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c
index 2a8e619..ef6b82c 100644
--- a/src/security/vboot/vboot_logic.c
+++ b/src/security/vboot/vboot_logic.c
@@ -336,6 +336,10 @@
    if (CONFIG(VBOOT_LID_SWITCH) && !get_lid_switch())
    	ctx.flags |= VB2_CONTEXT_NOFAIL_BOOT;

+	/* Mainboard/SoC always initializes display. */
+	if (!CONFIG(VBOOT_MUST_REQUEST_DISPLAY))
+		ctx.flags |= VB2_CONTEXT_DISPLAY_REQUESTED;
+
    /* Do early init (set up secdata and NVRAM, load GBB) */
    printk(BIOS_INFO, "Phase 1\n");
    rv = vb2api_fw_phase1(&ctx);
@@ -360,6 +364,12 @@
    	vboot_reboot();
    }

+	/* Is vboot declaring that display is available?  If so, we should mark
+	   it down, so that the mainboard/SoC knows to initialize display. */
+	if (ctx.flags & VB2_CONTEXT_DISPLAY_REQUESTED)
+		vboot_get_working_data()->flags
+			|= VBOOT_FLAGS_DISPLAY_REQUESTED;
+
    /* Determine which firmware slot to boot (based on NVRAM) */
    printk(BIOS_INFO, "Phase 2\n");
    rv = vb2api_fw_phase2(&ctx);

[Joel Kitching (Code Review)]

Hello Aaron Durbin,

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

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

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

Change subject: vboot: communicate display requirements with vb2api_fw_phase1
......................................................................

vboot: communicate display requirements with vb2api_fw_phase1

Input: tell vb2api_fw_phase1 if display unconditionally available
Output: vb2api_fw_phase1 may request coreboot to initialize
        display, if needed based on some internal request

Move setting the VBOOT_FLAG_DISPLAY_REQUESTED flag into
verstage_main.

BUG=b:124141368, b:124192753, chromium:948529
TEST=make clean && make test-abuild
CQ-DEPEND=CL:1564232
BRANCH=none

Change-Id: I81c82c46303564b63b8a32e7f80beb9d891a4628
Cq-Depend: chromium:1564232
Signed-off-by: Joel Kitching 
---
M src/security/vboot/vboot_handoff.c
M src/security/vboot/vboot_logic.c
2 files changed, 13 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/32324/2

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32324 )

Change subject: vboot: communicate display requirements with vb2api_fw_phase1
......................................................................

Patch Set 2: Code-Review+2

(2 comments)

https://review.coreboot.org/#/c/32324/2/src/security/vboot/vboot_logic.c 
File src/security/vboot/vboot_logic.c:

https://review.coreboot.org/#/c/32324/2/src/security/vboot/vboot_logic.c@369 
PS2, Line 369: DISPLAY_REQUESTED
Isn't it called DISPLAY_INIT now?

https://review.coreboot.org/#/c/32324/2/src/security/vboot/vboot_logic.c@371 
PS2, Line 371: DISPLAY_REQUESTED
If the context flag is called DISPLAY_INIT, I'd do the same here

[Joel Kitching (Code Review)]

Hello Aaron Durbin, Julius Werner, build bot (Jenkins),

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

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

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

Change subject: vboot: communicate display requirements with vb2api_fw_phase1
......................................................................

vboot: communicate display requirements with vb2api_fw_phase1

Input: tell vb2api_fw_phase1 if display unconditionally available
Output: vb2api_fw_phase1 may request coreboot to initialize
        display, if needed based on some internal request

Move setting the VBOOT_FLAG_DISPLAY_REQUESTED flag into
verstage_main.

BUG=b:124141368, b:124192753, chromium:948529
TEST=make clean && make test-abuild
CQ-DEPEND=CL:1564232
BRANCH=none

Change-Id: I81c82c46303564b63b8a32e7f80beb9d891a4628
Cq-Depend: chromium:1564232
Signed-off-by: Joel Kitching 
---
M src/security/vboot/vboot_handoff.c
M src/security/vboot/vboot_logic.c
2 files changed, 12 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/32324/3

[Joel Kitching (Code Review)]

Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32324 )

Change subject: vboot: communicate display requirements with vb2api_fw_phase1
......................................................................

Patch Set 3:

(2 comments)

https://review.coreboot.org/#/c/32324/2/src/security/vboot/vboot_logic.c 
File src/security/vboot/vboot_logic.c:

https://review.coreboot.org/#/c/32324/2/src/security/vboot/vboot_logic.c@369 
PS2, Line 369: DISPLAY_REQUESTED
> Isn't it called DISPLAY_INIT now?

Done

https://review.coreboot.org/#/c/32324/2/src/security/vboot/vboot_logic.c@371 
PS2, Line 371: DISPLAY_REQUESTED
> If the context flag is called DISPLAY_INIT, I'd do the same here

Done

[Joel Kitching (Code Review)]

Hello Aaron Durbin, Julius Werner, build bot (Jenkins),

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

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

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

Change subject: vboot: communicate display requirements with vb2api_fw_phase1
......................................................................

vboot: communicate display requirements with vb2api_fw_phase1

Input: tell vb2api_fw_phase1 if display unconditionally available
Output: vb2api_fw_phase1 may request coreboot to initialize
        display, if needed based on some internal request

Move setting the VBOOT_FLAG_DISPLAY_REQUESTED flag into
verstage_main.

BUG=b:124141368, b:124192753, chromium:948529
TEST=make clean && make test-abuild
CQ-DEPEND=CL:1564232
BRANCH=none

Change-Id: I81c82c46303564b63b8a32e7f80beb9d891a4628
Cq-Depend: chromium:1564232
Signed-off-by: Joel Kitching 
---
M src/security/vboot/vboot_handoff.c
M src/security/vboot/vboot_logic.c
2 files changed, 12 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/32324/5

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32324 )

Change subject: vboot: communicate display requirements with vb2api_fw_phase1
......................................................................

Patch Set 5: Code-Review+2

[Joel Kitching (Code Review)]

Hello Aaron Durbin, Julius Werner, build bot (Jenkins),

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

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

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

Change subject: vboot: communicate display requirements with vb2api_fw_phase1
......................................................................

vboot: communicate display requirements with vb2api_fw_phase1

Input: tell vb2api_fw_phase1 if display unconditionally available
Output: vb2api_fw_phase1 may request coreboot to initialize
        display, if needed based on some internal request

Move setting the VBOOT_FLAG_DISPLAY_REQUESTED flag into
verstage_main.

BUG=b:124141368, b:124192753, chromium:948529
TEST=make clean && make test-abuild
BRANCH=none

Change-Id: I81c82c46303564b63b8a32e7f80beb9d891a4628
Cq-Depend: chromium:1564232
Signed-off-by: Joel Kitching 
---
M src/security/vboot/vboot_handoff.c
M src/security/vboot/vboot_logic.c
2 files changed, 12 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/32324/8

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32324 )

Change subject: vboot: communicate display requirements with vb2api_fw_phase1
......................................................................

Patch Set 8: Code-Review+2

[Patrick Georgi (Code Review)]

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

Change subject: vboot: communicate display requirements with vb2api_fw_phase1
......................................................................

vboot: communicate display requirements with vb2api_fw_phase1

Input: tell vb2api_fw_phase1 if display unconditionally available
Output: vb2api_fw_phase1 may request coreboot to initialize
        display, if needed based on some internal request

Move setting the VBOOT_FLAG_DISPLAY_REQUESTED flag into
verstage_main.

BUG=b:124141368, b:124192753, chromium:948529
TEST=make clean && make test-abuild
BRANCH=none

Change-Id: I81c82c46303564b63b8a32e7f80beb9d891a4628
Cq-Depend: chromium:1564232
Signed-off-by: Joel Kitching 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32324
Tested-by: build bot (Jenkins) 
Reviewed-by: Julius Werner 
---
M src/security/vboot/vboot_handoff.c
M src/security/vboot/vboot_logic.c
2 files changed, 12 insertions(+), 6 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Julius Werner: Looks good to me, approved

diff --git a/src/security/vboot/vboot_handoff.c b/src/security/vboot/vboot_handoff.c
index 208663e..fccbdfc 100644
--- a/src/security/vboot/vboot_handoff.c
+++ b/src/security/vboot/vboot_handoff.c
@@ -68,14 +68,11 @@
    	vb_sd->flags |= VBSD_BOOT_DEV_SWITCH_ON;
    	vb_sd->flags |= VBSD_LF_DEV_SWITCH_ON;
    }
-	/* Inform vboot if the display was requested by vboot kernel phase
-	   or enabled by dev/rec mode. */
+	/* TODO(chromium:948529): Remove these two flags after downstream
+	   vboot code longer reads them. */
    if (vboot_wants_oprom() || vb2_sd->recovery_reason ||
-	    vb2_sd->flags & VB2_SD_FLAG_DEV_MODE_ENABLED) {
-		vboot_get_working_data()->flags |= VBOOT_WD_FLAG_DISPLAY_INIT;
+	    vb2_sd->flags & VB2_SD_FLAG_DEV_MODE_ENABLED)
    	vb_sd->flags |= VBSD_OPROM_LOADED;
-	}
-	/* TODO: Remove when depthcharge no longer reads this flag. */
    if (CONFIG(VBOOT_MUST_REQUEST_DISPLAY))
    	vb_sd->flags |= VBSD_OPROM_MATTERS;

diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c
index 2a8e619..df34490 100644
--- a/src/security/vboot/vboot_logic.c
+++ b/src/security/vboot/vboot_logic.c
@@ -336,6 +336,10 @@
    if (CONFIG(VBOOT_LID_SWITCH) && !get_lid_switch())
    	ctx.flags |= VB2_CONTEXT_NOFAIL_BOOT;

+	/* Mainboard/SoC always initializes display. */
+	if (!CONFIG(VBOOT_MUST_REQUEST_DISPLAY))
+		ctx.flags |= VB2_CONTEXT_DISPLAY_INIT;
+
    /* Do early init (set up secdata and NVRAM, load GBB) */
    printk(BIOS_INFO, "Phase 1\n");
    rv = vb2api_fw_phase1(&ctx);
@@ -360,6 +364,11 @@
    	vboot_reboot();
    }

+	/* Is vboot declaring that display is available?  If so, we should mark
+	   it down, so that the mainboard/SoC knows to initialize display. */
+	if (ctx.flags & VB2_CONTEXT_DISPLAY_INIT)
+		vboot_get_working_data()->flags |= VBOOT_WD_FLAG_DISPLAY_INIT;
+
    /* Determine which firmware slot to boot (based on NVRAM) */
    printk(BIOS_INFO, "Phase 2\n");
    rv = vb2api_fw_phase2(&ctx);