Change in coreboot[master]: intel/gma: Only enable bus mastering if we are going to use it

[Nico Huber (Code Review)]

Hello Matt DeVillier, Angel Pons, Patrick Rudolph,

I'd like you to do a code review. Please visit

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

to review the following change.

Change subject: intel/gma: Only enable bus mastering if we are going to use it
......................................................................

intel/gma: Only enable bus mastering if we are going to use it

Also fix wrong 32-bit writes.

Change-Id: Ib038f0cd558223536da08ba2264774db11cd8357
Signed-off-by: Nico Huber 
---
M src/northbridge/intel/gm45/gma.c
M src/northbridge/intel/haswell/gma.c
M src/northbridge/intel/i945/gma.c
M src/northbridge/intel/ironlake/gma.c
M src/northbridge/intel/pineview/gma.c
M src/northbridge/intel/sandybridge/gma.c
M src/northbridge/intel/x4x/gma.c
M src/soc/intel/broadwell/igd.c
M src/soc/intel/common/block/graphics/graphics.c
9 files changed, 23 insertions(+), 55 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/40727/1

diff --git a/src/northbridge/intel/gm45/gma.c b/src/northbridge/intel/gm45/gma.c
index 45da91d..ea68deb 100644
--- a/src/northbridge/intel/gm45/gma.c
+++ b/src/northbridge/intel/gm45/gma.c
@@ -145,22 +145,18 @@

static void gma_func0_init(struct device *dev)
 {
-	u32 reg32;
    u8 *mmio;
    u8 edid_data_lvds[128];
    struct edid edid_lvds;
    const struct northbridge_intel_gm45_config *const conf = dev->chip_info;

-	/* IGD needs to be Bus Master */
-	reg32 = pci_read_config32(dev, PCI_COMMAND);
-	reg32 |= PCI_COMMAND_MASTER;
-	pci_write_config32(dev, PCI_COMMAND, reg32);
-
    gtt_res = find_resource(dev, PCI_BASE_ADDRESS_0);
    if (gtt_res == NULL)
    	return;
    mmio = res2mmio(gtt_res, 0, 0);

+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);

if (!CONFIG(MAINBOARD_USE_LIBGFXINIT)) {
    	/* PCI Init, will run VBIOS */
diff --git a/src/northbridge/intel/haswell/gma.c b/src/northbridge/intel/haswell/gma.c
index 4dcdfe3..772c730 100644
--- a/src/northbridge/intel/haswell/gma.c
+++ b/src/northbridge/intel/haswell/gma.c
@@ -459,12 +459,6 @@
 static void gma_func0_init(struct device *dev)
 {
    int lightup_ok = 0;
-	u32 reg32;
-
-	/* IGD needs to be Bus Master */
-	reg32 = pci_read_config32(dev, PCI_COMMAND);
-	reg32 |= PCI_COMMAND_MASTER;
-	pci_write_config32(dev, PCI_COMMAND, reg32);

/* Init graphics power management */
    gma_pm_init_pre_vbios(dev);
@@ -472,6 +466,9 @@
    /* Pre panel init */
    gma_setup_panel(dev);

+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);
+
    int vga_disable = (pci_read_config16(dev, GGC) & 2) >> 1;

if (CONFIG(MAINBOARD_USE_LIBGFXINIT)) {
diff --git a/src/northbridge/intel/i945/gma.c b/src/northbridge/intel/i945/gma.c
index 198e044..9811590 100644
--- a/src/northbridge/intel/i945/gma.c
+++ b/src/northbridge/intel/i945/gma.c
@@ -661,8 +661,6 @@

static void gma_func0_init(struct device *dev)
 {
-	u32 reg32;
-
    /* Unconditionally reset graphics */
    pci_write_config8(dev, GDRST, 1);
    udelay(50);
@@ -671,9 +669,8 @@
    while (pci_read_config8(dev, GDRST) & 1)
    	;

-	/* IGD needs to be Bus Master */
-	reg32 = pci_read_config32(dev, PCI_COMMAND);
-	pci_write_config32(dev, PCI_COMMAND, reg32 | PCI_COMMAND_MASTER);
+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);

if (CONFIG(MAINBOARD_DO_NATIVE_VGA_INIT)) {
    	int vga_disable = (pci_read_config16(dev, GGC) & 2) >> 1;
@@ -714,12 +711,10 @@

static void gma_func1_init(struct device *dev)
 {
-	u32 reg32;
    u8 val;

-	/* IGD needs to be Bus Master */
-	reg32 = pci_read_config32(dev, PCI_COMMAND);
-	pci_write_config32(dev, PCI_COMMAND, reg32 | PCI_COMMAND_MASTER);
+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);

if (get_option(&val, "tft_brightness") == CB_SUCCESS)
    	pci_write_config8(dev, 0xf4, val);
diff --git a/src/northbridge/intel/ironlake/gma.c b/src/northbridge/intel/ironlake/gma.c
index 325857f..3604501 100644
--- a/src/northbridge/intel/ironlake/gma.c
+++ b/src/northbridge/intel/ironlake/gma.c
@@ -136,12 +136,8 @@

static void gma_func0_init(struct device *dev)
 {
-	u32 reg32;
-
-	/* IGD needs to be Bus Master */
-	reg32 = pci_read_config32(dev, PCI_COMMAND);
-	reg32 |= PCI_COMMAND_MASTER;
-	pci_write_config32(dev, PCI_COMMAND, reg32);
+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);

gtt_res = find_resource(dev, PCI_BASE_ADDRESS_0);
    if (!gtt_res || !gtt_res->base)
diff --git a/src/northbridge/intel/pineview/gma.c b/src/northbridge/intel/pineview/gma.c
index 7189f95..0df9055 100644
--- a/src/northbridge/intel/pineview/gma.c
+++ b/src/northbridge/intel/pineview/gma.c
@@ -219,12 +219,8 @@

static void gma_func0_init(struct device *dev)
 {
-	u32 reg32;
-
-	/* IGD needs to be Bus Master */
-	reg32 = pci_read_config32(dev, PCI_COMMAND);
-	reg32 |= PCI_COMMAND_MASTER;
-	pci_write_config32(dev, PCI_COMMAND, reg32);
+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);

if (!CONFIG(MAINBOARD_DO_NATIVE_VGA_INIT)) {
    	/* PCI init, will run VBIOS */
diff --git a/src/northbridge/intel/sandybridge/gma.c b/src/northbridge/intel/sandybridge/gma.c
index 53578f2..ec45e30 100644
--- a/src/northbridge/intel/sandybridge/gma.c
+++ b/src/northbridge/intel/sandybridge/gma.c
@@ -586,16 +586,12 @@

static void gma_func0_init(struct device *dev)
 {
-	u32 reg32;
-
-	/* IGD needs to be Bus Master */
-	reg32 = pci_read_config32(dev, PCI_COMMAND);
-	reg32 |= PCI_COMMAND_MASTER;
-	pci_write_config32(dev, PCI_COMMAND, reg32);
-
    /* Init graphics power management */
    gma_pm_init_pre_vbios(dev);

+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);
+
    if (!CONFIG(MAINBOARD_USE_LIBGFXINIT))
    	/* PCI Init, will run VBIOS */
    	pci_dev_init(dev);
diff --git a/src/northbridge/intel/x4x/gma.c b/src/northbridge/intel/x4x/gma.c
index 1b3d316..bfec77a 100644
--- a/src/northbridge/intel/x4x/gma.c
+++ b/src/northbridge/intel/x4x/gma.c
@@ -23,12 +23,8 @@

static void gma_func0_init(struct device *dev)
 {
-	u32 reg32;
-
-	/* IGD needs to be Bus Master */
-	reg32 = pci_read_config32(dev, PCI_COMMAND);
-	reg32 |= PCI_COMMAND_MASTER;
-	pci_write_config32(dev, PCI_COMMAND, reg32);
+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);

/* configure GMBUSFREQ */
    pci_update_config16(dev, 0xcc, ~0x1ff, 0xbc);
diff --git a/src/soc/intel/broadwell/igd.c b/src/soc/intel/broadwell/igd.c
index f136d74..2e4f08f 100644
--- a/src/soc/intel/broadwell/igd.c
+++ b/src/soc/intel/broadwell/igd.c
@@ -495,15 +495,13 @@
    int is_broadwell = !!(cpu_family_model() == BROADWELL_FAMILY_ULT);
    u32 rp1_gfx_freq;

-	/* IGD needs to be Bus Master */
-	u32 reg32 = pci_read_config32(dev, PCI_COMMAND);
-	reg32 |= PCI_COMMAND_MASTER;
-	pci_write_config32(dev, PCI_COMMAND, reg32);
-
    gtt_res = find_resource(dev, PCI_BASE_ADDRESS_0);
    if (!gtt_res || !gtt_res->base)
    	return;

+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);
+
    /* Wait for any configured pre-graphics delay */
    if (!acpi_is_wakeup_s3()) {
 #if CONFIG(CHROMEOS)
diff --git a/src/soc/intel/common/block/graphics/graphics.c b/src/soc/intel/common/block/graphics/graphics.c
index 6e7d27e..7d7a49d 100644
--- a/src/soc/intel/common/block/graphics/graphics.c
+++ b/src/soc/intel/common/block/graphics/graphics.c
@@ -45,10 +45,8 @@
    /* SoC specific configuration. */
    graphics_soc_init(dev);

-	/* IGD needs to Bus Master */
-	u32 reg32 = pci_read_config32(dev, PCI_COMMAND);
-	reg32 |= PCI_COMMAND_MASTER;
-	pci_write_config32(dev, PCI_COMMAND, reg32);
+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);

/*
     * GFX PEIM module inside FSP binary is taking care of graphics

[Matt DeVillier (Code Review)]

Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40727 )

Change subject: intel/gma: Only enable bus mastering if we are going to use it
......................................................................

Patch Set 2:

need to check if this breaks using external GOP driver with Tianocore (in which NO_GFX_INIT is set)

[Nico Huber (Code Review)]

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

Change subject: intel/gma: Only enable bus mastering if we are going to use it
......................................................................

Patch Set 2:
> need to check if this breaks using external GOP driver with Tianocore (in which NO_GFX_INIT is set)

Same for SeaBIOS running a VBIOS. But if any of that doesn't work
the payload (driver) is broken and that would at least deserve a
comment why we handle that in coreboot :)

[Matt DeVillier (Code Review)]

Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40727 )

Change subject: intel/gma: Only enable bus mastering if we are going to use it
......................................................................

Patch Set 2: Code-Review+1

tested ok on google/chell (SKL) w/NO_GFX_INIT and external GOP driver

[Angel Pons (Code Review)]

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

Change subject: intel/gma: Only enable bus mastering if we are going to use it
......................................................................

Patch Set 4: Code-Review+1

(3 comments)

https://review.coreboot.org/c/coreboot/+/40727/4/src/northbridge/intel/gm45/... 
File src/northbridge/intel/gm45/gma.c:

https://review.coreboot.org/c/coreboot/+/40727/4/src/northbridge/intel/gm45/... 
PS4, Line 157: 	if (!CONFIG(NO_GFX_INIT))
This reordering seems justified, as we need the gtt_res

https://review.coreboot.org/c/coreboot/+/40727/4/src/northbridge/intel/haswe... 
File src/northbridge/intel/haswell/gma.c:

https://review.coreboot.org/c/coreboot/+/40727/4/src/northbridge/intel/haswe... 
PS4, Line 468: 	if (!CONFIG(NO_GFX_INIT))
Does this reordering matter?

https://review.coreboot.org/c/coreboot/+/40727/4/src/northbridge/intel/sandy... 
File src/northbridge/intel/sandybridge/gma.c:

https://review.coreboot.org/c/coreboot/+/40727/4/src/northbridge/intel/sandy... 
PS4, Line 591: 	if (!CONFIG(NO_GFX_INIT))
Does this reordering matter?

[Nico Huber (Code Review)]

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

Change subject: intel/gma: Only enable bus mastering if we are going to use it
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40727/4/src/northbridge/intel/haswe... 
File src/northbridge/intel/haswell/gma.c:

https://review.coreboot.org/c/coreboot/+/40727/4/src/northbridge/intel/haswe... 
PS4, Line 468: 	if (!CONFIG(NO_GFX_INIT))
> Does this reordering matter?

I wanted to group it with the gfxinit. The lines above are about general
silicon init.

[Nico Huber (Code Review)]

Hello build bot (Jenkins), Damien Zammit, Matt DeVillier, Angel Pons, Patrick Rudolph,

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

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

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

Change subject: intel/gma: Only enable bus mastering if we are going to use it
......................................................................

intel/gma: Only enable bus mastering if we are going to use it

Also fix wrong 32-bit writes.

Change-Id: Ib038f0cd558223536da08ba2264774db11cd8357
Signed-off-by: Nico Huber 
---
M src/northbridge/intel/gm45/gma.c
M src/northbridge/intel/haswell/gma.c
M src/northbridge/intel/i945/gma.c
M src/northbridge/intel/ironlake/gma.c
M src/northbridge/intel/pineview/gma.c
M src/northbridge/intel/sandybridge/gma.c
M src/northbridge/intel/x4x/gma.c
M src/soc/intel/broadwell/igd.c
M src/soc/intel/common/block/graphics/graphics.c
9 files changed, 23 insertions(+), 55 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/40727/5

[Angel Pons (Code Review)]

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

Change subject: intel/gma: Only enable bus mastering if we are going to use it
......................................................................

Patch Set 5: Code-Review+2

(2 comments)

https://review.coreboot.org/c/coreboot/+/40727/4/src/northbridge/intel/haswe... 
File src/northbridge/intel/haswell/gma.c:

https://review.coreboot.org/c/coreboot/+/40727/4/src/northbridge/intel/haswe... 
PS4, Line 468: 	if (!CONFIG(NO_GFX_INIT))
> I wanted to group it with the gfxinit. The lines above are about general […]

Ack

https://review.coreboot.org/c/coreboot/+/40727/4/src/northbridge/intel/sandy... 
File src/northbridge/intel/sandybridge/gma.c:

https://review.coreboot.org/c/coreboot/+/40727/4/src/northbridge/intel/sandy... 
PS4, Line 591: 	if (!CONFIG(NO_GFX_INIT))
> Does this reordering matter?

Ack

[Nico Huber (Code Review)]

Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40727 )

Change subject: intel/gma: Only enable bus mastering if we are going to use it
......................................................................

intel/gma: Only enable bus mastering if we are going to use it

Also fix wrong 32-bit writes.

Change-Id: Ib038f0cd558223536da08ba2264774db11cd8357
Signed-off-by: Nico Huber 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40727
Reviewed-by: Angel Pons 
Tested-by: build bot (Jenkins) 
---
M src/northbridge/intel/gm45/gma.c
M src/northbridge/intel/haswell/gma.c
M src/northbridge/intel/i945/gma.c
M src/northbridge/intel/ironlake/gma.c
M src/northbridge/intel/pineview/gma.c
M src/northbridge/intel/sandybridge/gma.c
M src/northbridge/intel/x4x/gma.c
M src/soc/intel/broadwell/igd.c
M src/soc/intel/common/block/graphics/graphics.c
9 files changed, 23 insertions(+), 55 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved

diff --git a/src/northbridge/intel/gm45/gma.c b/src/northbridge/intel/gm45/gma.c
index b48b3c3..6a51dae 100644
--- a/src/northbridge/intel/gm45/gma.c
+++ b/src/northbridge/intel/gm45/gma.c
@@ -144,7 +144,6 @@

static void gma_func0_init(struct device *dev)
 {
-	u32 reg32;
    u8 *mmio;
    u8 edid_data_lvds[128];
    struct edid edid_lvds;
@@ -152,16 +151,13 @@

intel_gma_init_igd_opregion();

-	/* IGD needs to be Bus Master */
-	reg32 = pci_read_config32(dev, PCI_COMMAND);
-	reg32 |= PCI_COMMAND_MASTER;
-	pci_write_config32(dev, PCI_COMMAND, reg32);
-
    gtt_res = find_resource(dev, PCI_BASE_ADDRESS_0);
    if (gtt_res == NULL)
    	return;
    mmio = res2mmio(gtt_res, 0, 0);

+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);

if (!CONFIG(MAINBOARD_USE_LIBGFXINIT)) {
    	/* PCI Init, will run VBIOS */
diff --git a/src/northbridge/intel/haswell/gma.c b/src/northbridge/intel/haswell/gma.c
index fa67fe1..19341d4 100644
--- a/src/northbridge/intel/haswell/gma.c
+++ b/src/northbridge/intel/haswell/gma.c
@@ -458,21 +458,18 @@
 static void gma_func0_init(struct device *dev)
 {
    int lightup_ok = 0;
-	u32 reg32;

intel_gma_init_igd_opregion();

-	/* IGD needs to be Bus Master */
-	reg32 = pci_read_config32(dev, PCI_COMMAND);
-	reg32 |= PCI_COMMAND_MASTER;
-	pci_write_config32(dev, PCI_COMMAND, reg32);
-
    /* Init graphics power management */
    gma_pm_init_pre_vbios(dev);

/* Pre panel init */
    gma_setup_panel(dev);

+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);
+
    int vga_disable = (pci_read_config16(dev, GGC) & 2) >> 1;

if (CONFIG(MAINBOARD_USE_LIBGFXINIT)) {
diff --git a/src/northbridge/intel/i945/gma.c b/src/northbridge/intel/i945/gma.c
index 71ed4c4..183c8f5 100644
--- a/src/northbridge/intel/i945/gma.c
+++ b/src/northbridge/intel/i945/gma.c
@@ -660,8 +660,6 @@

static void gma_func0_init(struct device *dev)
 {
-	u32 reg32;
-
    intel_gma_init_igd_opregion();

/* Unconditionally reset graphics */
@@ -672,9 +670,8 @@
    while (pci_read_config8(dev, GDRST) & 1)
    	;

-	/* IGD needs to be Bus Master */
-	reg32 = pci_read_config32(dev, PCI_COMMAND);
-	pci_write_config32(dev, PCI_COMMAND, reg32 | PCI_COMMAND_MASTER);
+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);

if (CONFIG(MAINBOARD_DO_NATIVE_VGA_INIT)) {
    	int vga_disable = (pci_read_config16(dev, GGC) & 2) >> 1;
@@ -713,12 +710,10 @@

static void gma_func1_init(struct device *dev)
 {
-	u32 reg32;
    u8 val;

-	/* IGD needs to be Bus Master */
-	reg32 = pci_read_config32(dev, PCI_COMMAND);
-	pci_write_config32(dev, PCI_COMMAND, reg32 | PCI_COMMAND_MASTER);
+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);

if (get_option(&val, "tft_brightness") == CB_SUCCESS)
    	pci_write_config8(dev, 0xf4, val);
diff --git a/src/northbridge/intel/ironlake/gma.c b/src/northbridge/intel/ironlake/gma.c
index 6ba95d3..5ccf8a6 100644
--- a/src/northbridge/intel/ironlake/gma.c
+++ b/src/northbridge/intel/ironlake/gma.c
@@ -135,14 +135,10 @@

static void gma_func0_init(struct device *dev)
 {
-	u32 reg32;
-
    intel_gma_init_igd_opregion();

-	/* IGD needs to be Bus Master */
-	reg32 = pci_read_config32(dev, PCI_COMMAND);
-	reg32 |= PCI_COMMAND_MASTER;
-	pci_write_config32(dev, PCI_COMMAND, reg32);
+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);

gtt_res = find_resource(dev, PCI_BASE_ADDRESS_0);
    if (!gtt_res || !gtt_res->base)
diff --git a/src/northbridge/intel/pineview/gma.c b/src/northbridge/intel/pineview/gma.c
index 7e5b236..2f4b629 100644
--- a/src/northbridge/intel/pineview/gma.c
+++ b/src/northbridge/intel/pineview/gma.c
@@ -218,14 +218,10 @@

static void gma_func0_init(struct device *dev)
 {
-	u32 reg32;
-
    intel_gma_init_igd_opregion();

-	/* IGD needs to be Bus Master */
-	reg32 = pci_read_config32(dev, PCI_COMMAND);
-	reg32 |= PCI_COMMAND_MASTER;
-	pci_write_config32(dev, PCI_COMMAND, reg32);
+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);

if (!CONFIG(MAINBOARD_DO_NATIVE_VGA_INIT)) {
    	/* PCI init, will run VBIOS */
diff --git a/src/northbridge/intel/sandybridge/gma.c b/src/northbridge/intel/sandybridge/gma.c
index 6e89b4e..150b61c 100644
--- a/src/northbridge/intel/sandybridge/gma.c
+++ b/src/northbridge/intel/sandybridge/gma.c
@@ -585,18 +585,14 @@

static void gma_func0_init(struct device *dev)
 {
-	u32 reg32;
-
    intel_gma_init_igd_opregion();

-	/* IGD needs to be Bus Master */
-	reg32 = pci_read_config32(dev, PCI_COMMAND);
-	reg32 |= PCI_COMMAND_MASTER;
-	pci_write_config32(dev, PCI_COMMAND, reg32);
-
    /* Init graphics power management */
    gma_pm_init_pre_vbios(dev);

+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);
+
    if (!CONFIG(MAINBOARD_USE_LIBGFXINIT))
    	/* PCI Init, will run VBIOS */
    	pci_dev_init(dev);
diff --git a/src/northbridge/intel/x4x/gma.c b/src/northbridge/intel/x4x/gma.c
index 69b6f71..ecbd63e 100644
--- a/src/northbridge/intel/x4x/gma.c
+++ b/src/northbridge/intel/x4x/gma.c
@@ -22,14 +22,10 @@

static void gma_func0_init(struct device *dev)
 {
-	u32 reg32;
-
    intel_gma_init_igd_opregion();

-	/* IGD needs to be Bus Master */
-	reg32 = pci_read_config32(dev, PCI_COMMAND);
-	reg32 |= PCI_COMMAND_MASTER;
-	pci_write_config32(dev, PCI_COMMAND, reg32);
+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);

/* configure GMBUSFREQ */
    pci_update_config16(dev, 0xcc, ~0x1ff, 0xbc);
diff --git a/src/soc/intel/broadwell/igd.c b/src/soc/intel/broadwell/igd.c
index 0f83937..41167b1 100644
--- a/src/soc/intel/broadwell/igd.c
+++ b/src/soc/intel/broadwell/igd.c
@@ -496,15 +496,13 @@

intel_gma_init_igd_opregion();

-	/* IGD needs to be Bus Master */
-	u32 reg32 = pci_read_config32(dev, PCI_COMMAND);
-	reg32 |= PCI_COMMAND_MASTER;
-	pci_write_config32(dev, PCI_COMMAND, reg32);
-
    gtt_res = find_resource(dev, PCI_BASE_ADDRESS_0);
    if (!gtt_res || !gtt_res->base)
    	return;

+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);
+
    /* Wait for any configured pre-graphics delay */
    if (!acpi_is_wakeup_s3()) {
 #if CONFIG(CHROMEOS)
diff --git a/src/soc/intel/common/block/graphics/graphics.c b/src/soc/intel/common/block/graphics/graphics.c
index 6b035bc..ba4bc85 100644
--- a/src/soc/intel/common/block/graphics/graphics.c
+++ b/src/soc/intel/common/block/graphics/graphics.c
@@ -45,10 +45,8 @@
    if (CONFIG(RUN_FSP_GOP))
    	return;

-	/* IGD needs to Bus Master */
-	u32 reg32 = pci_read_config32(dev, PCI_COMMAND);
-	reg32 |= PCI_COMMAND_MASTER;
-	pci_write_config32(dev, PCI_COMMAND, reg32);
+	if (!CONFIG(NO_GFX_INIT))
+		pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);

if (CONFIG(MAINBOARD_USE_LIBGFXINIT)) {
    	if (!acpi_is_wakeup_s3() && display_init_required()) {

[9elements QA (Code Review)]

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

Change subject: intel/gma: Only enable bus mastering if we are going to use it
......................................................................

Patch Set 7:

Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4
Emulation targets:
"QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/4234
"QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/4233
"QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/4232
"QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/4231

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