Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Rework brightness level includes ......................................................................
drivers/intel/gma: Rework brightness level includes
Make GFX0 device independent from brightness controls by removing the includes for common.asl and configure_brightness_levels.asl from gma.asl and moving them into default_brightness_levels.asl, which is what most boards include directly.
Also include these two files directly for the two boards which do not use the default brightness levels, but define their own.
Change-Id: Ie71bd5fc7acd926b7ce7da17fbc108670fd453e0 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/drivers/intel/gma/acpi/default_brightness_levels.asl M src/drivers/intel/gma/acpi/gma.asl M src/mainboard/roda/rv11/variants/rv11/include/acpi/brightness_levels.asl M src/mainboard/roda/rv11/variants/rw11/include/acpi/brightness_levels.asl 4 files changed, 9 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/48862/1
diff --git a/src/drivers/intel/gma/acpi/default_brightness_levels.asl b/src/drivers/intel/gma/acpi/default_brightness_levels.asl index 9e81741..7dadc42 100644 --- a/src/drivers/intel/gma/acpi/default_brightness_levels.asl +++ b/src/drivers/intel/gma/acpi/default_brightness_levels.asl @@ -4,6 +4,9 @@
Scope (GFX0) { + #include "configure_brightness_levels.asl" + #include "common.asl" + Name (BRIG, Package (0x12) { 100, /* default AC */ diff --git a/src/drivers/intel/gma/acpi/gma.asl b/src/drivers/intel/gma/acpi/gma.asl index c4ee2db..ad47cea 100644 --- a/src/drivers/intel/gma/acpi/gma.asl +++ b/src/drivers/intel/gma/acpi/gma.asl @@ -23,7 +23,4 @@ Offset (CONFIG_INTEL_GMA_BCLM_OFFSET), BCLM, CONFIG_INTEL_GMA_BCLM_WIDTH } - -#include "configure_brightness_levels.asl" -#include "common.asl" } diff --git a/src/mainboard/roda/rv11/variants/rv11/include/acpi/brightness_levels.asl b/src/mainboard/roda/rv11/variants/rv11/include/acpi/brightness_levels.asl index 638e659..972bee8 100644 --- a/src/mainboard/roda/rv11/variants/rv11/include/acpi/brightness_levels.asl +++ b/src/mainboard/roda/rv11/variants/rv11/include/acpi/brightness_levels.asl @@ -4,6 +4,9 @@
Scope (GFX0) { + #include <drivers/intel/gma/acpi/configure_brightness_levels.asl> + #include <drivers/intel/gma/acpi/common.asl> + Name (BRIG, Package (13) { 74, /* default AC */ diff --git a/src/mainboard/roda/rv11/variants/rw11/include/acpi/brightness_levels.asl b/src/mainboard/roda/rv11/variants/rw11/include/acpi/brightness_levels.asl index 2ad4521..a50e97a 100644 --- a/src/mainboard/roda/rv11/variants/rw11/include/acpi/brightness_levels.asl +++ b/src/mainboard/roda/rv11/variants/rw11/include/acpi/brightness_levels.asl @@ -4,6 +4,9 @@
Scope (GFX0) { + #include <drivers/intel/gma/acpi/configure_brightness_levels.asl> + #include <drivers/intel/gma/acpi/common.asl> + Name (BRIG, Package (13) { 74, /* default AC */
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Rework brightness level includes ......................................................................
Patch Set 1:
I'm somehow missing, what the improvement here really is besides that the custom brightness table version is more explicit and more flexible.
So, for the case with default brightness nothing changes, right?
Example:
#include <drivers/intel/gma/acpi/gma.asl> #include <drivers/intel/gma/acpi/default_brightness_levels.asl>
When using a custom brightness table, one needs to include three files now instead of one:
Example:
#include <drivers/intel/gma/acpi/gma.asl>
Scope (GFX0) { Name (BRIG, Package() { ... }, },
Becomes this:
#include <drivers/intel/gma/acpi/gma.asl>
Scope (GFX0) { #include <drivers/intel/gma/acpi/configure_brightness_levels.asl> #include <drivers/intel/gma/acpi/common.asl>
Name (BRIG, Package() { ... }, }
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Rework brightness level includes ......................................................................
Patch Set 1:
Patch Set 1:
I'm somehow missing, what the improvement here really is besides that the custom brightness table version is more explicit and more flexible.
the improvement is that this allows gma.asl to be included to provide the GFX0 device without forcing the inclusion of unnecessary brightness controls for devices without an internal panel.
the use case driving this change is running MacOS on Chromeboxes where the GFX0 device is required
So, for the case with default brightness nothing changes, right?
correct.
I'm certainly open to alternative implementations to achieve the same decoupling
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Rework brightness level includes ......................................................................
Patch Set 1: Code-Review+2
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Rework brightness level includes ......................................................................
Patch Set 1: Code-Review+2
Patch Set 1:
Patch Set 1:
I'm somehow missing, what the improvement here really is besides that the custom brightness table version is more explicit and more flexible.
the improvement is that this allows gma.asl to be included to provide the GFX0 device without forcing the inclusion of unnecessary brightness controls for devices without an internal panel.
the use case driving this change is running MacOS on Chromeboxes where the GFX0 device is required
So, for the case with default brightness nothing changes, right?
correct.
I'm certainly open to alternative implementations to achieve the same decoupling
Got it, thanks! This way we can probably even get rid of the boilerplate version and just use gma.asl now: CB:48773
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Rework brightness level includes ......................................................................
Patch Set 1:
Why not rename `gma.asl` to `gma_with_panel.asl` or something like that? Then add one without the panel boilerplate for desktops etc. It would a) reduce unwanted boilerplate further and b) not need repeated includes in the mainboards.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Rework brightness level includes ......................................................................
Patch Set 1:
I'm somehow missing, what the improvement here really is besides that the custom brightness table version is more explicit and more flexible.
the improvement is that this allows gma.asl to be included to provide the GFX0 device without forcing the inclusion of unnecessary brightness controls for devices without an internal panel.
the use case driving this change is running MacOS on Chromeboxes where the GFX0 device is required
We had investigated the GFX0 requirement lately and only found two references in coreboot code (drivers/intel/gma and drivers/gfx/generic). Neither of them should be used for Chromeboxes AFAICS. So maybe there is some unwanted boilerplate left that depends on it? Or is the GFX0 hardcoded in a MacOS driver?
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Rework brightness level includes ......................................................................
Patch Set 1:
Patch Set 1:
Or is the GFX0 hardcoded in a MacOS driver?
MacOS actually wants `IGFX` IIRC, but MacOS bootloaders like clover and OpenCore expect the UEFI-standard `GFX0` in order to perform the necessary DSDT modifications to boot MacOS.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Rework brightness level includes ......................................................................
Patch Set 1:
Patch Set 1:
Why not rename `gma.asl` to `gma_with_panel.asl` or something like that? Then add one without the panel boilerplate for desktops etc. It would a) reduce unwanted boilerplate further and b) not need repeated includes in the mainboards.
I'm perfectly happy to do it that way as well
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Rework brightness level includes ......................................................................
Patch Set 1: Code-Review-1
putting on hold while I look at other approaches
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Rework brightness level includes ......................................................................
Patch Set 1: Code-Review-2
Patch Set 1: Code-Review-1
putting on hold while I look at other approaches
looks like I need to -2 to actually block it
Hello build bot (Jenkins), Nico Huber, Angel Pons, Michael Niewöhner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48862
to look at the new patch set (#2).
Change subject: drivers/intel/gma: Add stub for GFX0 device, include by default ......................................................................
drivers/intel/gma: Add stub for GFX0 device, include by default
Add a stub GFX0 device which includes only the PCI address, and include it by default for all platforms selecting INTEL_GMA_ACPI. Rework brightness level includes to utilize this new stub, and remove any duplicate device definitions in platform-level asl files.
Some OSes (eg, Windows, MacOS) require/prefer the ACPI device for the IGD to exist, even if ACPI brightness controls are not utilized. This change adds a GFX0 ACPI device for all boards whose platforms select INTEL_GMA_ACPI without requiring non-functional brightness controls to be added at the board level.
A subsequent change will eliminate the stub currently defined for platforms using the soc/intel/common implementation and migrate them to using this one.
Change-Id: Ie71bd5fc7acd926b7ce7da17fbc108670fd453e0 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/drivers/intel/gma/acpi/gma.asl A src/drivers/intel/gma/acpi/gma_stub.asl M src/northbridge/intel/gm45/acpi/gm45.asl M src/northbridge/intel/haswell/acpi/hostbridge.asl M src/northbridge/intel/i945/acpi/i945.asl M src/northbridge/intel/i945/acpi/igd.asl M src/northbridge/intel/ironlake/acpi/ironlake.asl M src/northbridge/intel/pineview/acpi/pineview.asl M src/northbridge/intel/sandybridge/acpi/sandybridge.asl M src/northbridge/intel/x4x/acpi/x4x.asl M src/soc/intel/baytrail/acpi/southcluster.asl M src/soc/intel/braswell/acpi/southcluster.asl M src/soc/intel/broadwell/acpi/hostbridge.asl 13 files changed, 36 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/48862/2
Matt DeVillier has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Add stub for GFX0 device, include by default ......................................................................
Removed Code-Review-2 by Matt DeVillier matt.devillier@gmail.com
Hello build bot (Jenkins), Nico Huber, Damien Zammit, Angel Pons, Michael Niewöhner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48862
to look at the new patch set (#3).
Change subject: drivers/intel/gma: Include gfx.asl by default for all platforms... ......................................................................
drivers/intel/gma: Include gfx.asl by default for all platforms...
which select INTEL_GMA_ACPI. Rework brightness level includes and platform-level asl files to avoid duplicate device definition for GFX0.
Some OSes (eg, Windows, MacOS) require/prefer the ACPI device for the IGD to exist, even if ACPI brightness controls are not utilized. This change adds a GFX0 ACPI device for all boards whose platforms select INTEL_GMA_ACPI without requiring non-functional brightness controls to be added at the board level.
Change-Id: Ie71bd5fc7acd926b7ce7da17fbc108670fd453e0 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/drivers/intel/gma/acpi/gma.asl M src/northbridge/intel/gm45/acpi/gm45.asl M src/northbridge/intel/haswell/acpi/hostbridge.asl M src/northbridge/intel/i945/acpi/i945.asl M src/northbridge/intel/i945/acpi/igd.asl M src/northbridge/intel/ironlake/acpi/ironlake.asl M src/northbridge/intel/pineview/acpi/pineview.asl M src/northbridge/intel/sandybridge/acpi/sandybridge.asl M src/northbridge/intel/x4x/acpi/x4x.asl M src/soc/intel/baytrail/acpi/southcluster.asl M src/soc/intel/braswell/acpi/southcluster.asl M src/soc/intel/broadwell/acpi/hostbridge.asl 12 files changed, 30 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/48862/3
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Include gfx.asl by default for all platforms... ......................................................................
Patch Set 3:
as discussed on IRC, I'm unsure if we really want a stub to be always included. I don't have a strong opinion about it, though
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Include gfx.asl by default for all platforms... ......................................................................
Patch Set 3: Code-Review+1
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Include gfx.asl by default for all platforms... ......................................................................
Patch Set 3:
as discussed on IRC, I'm unsure if we really want a stub to be always included. I don't have a strong opinion about it, though
I don't mind either. The only thing that comes to mind: Boards without IGD or the IGD strapped to be disabled get GFX0 too. But that shouldn't hurt.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Include gfx.asl by default for all platforms... ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48862/3/src/soc/intel/baytrail/acpi... File src/soc/intel/baytrail/acpi/southcluster.asl:
PS3: Not sure what southcluster means, but the IGD is usually north.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Include gfx.asl by default for all platforms... ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48862/3/src/soc/intel/baytrail/acpi... File src/soc/intel/baytrail/acpi/southcluster.asl:
PS3:
Not sure what southcluster means, but the IGD is usually north.
I realize this, but almost all of the soc/common platforms seem to throw all of the other includes in southcluster.asl
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Include gfx.asl by default for all platforms... ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48862/3/src/soc/intel/baytrail/acpi... File src/soc/intel/baytrail/acpi/southcluster.asl:
PS3:
I realize this, but almost all of the soc/common platforms seem to throw all of the other includes i […]
Ack. I guess these files are just very badly named (and structured worse).
Hello build bot (Jenkins), Nico Huber, Damien Zammit, Angel Pons, Michael Niewöhner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48862
to look at the new patch set (#5).
Change subject: drivers/intel/gma: Include gfx.asl by default for all platforms... ......................................................................
drivers/intel/gma: Include gfx.asl by default for all platforms...
which select INTEL_GMA_ACPI. Rework brightness level includes and platform-level asl files to avoid duplicate device definition for GFX0.
Include gfx.asl for Skylake/Kabylake, since all other soc/intel/common platforms already do. Adjust mb/51nb/x210 to prevent device redefinition.
Some OSes (eg, Windows, MacOS) require/prefer the ACPI device for the IGD to exist, even if ACPI brightness controls are not utilized. This change adds a GFX0 ACPI device for all boards whose platforms select INTEL_GMA_ACPI without requiring non-functional brightness controls to be added at the board level.
Change-Id: Ie71bd5fc7acd926b7ce7da17fbc108670fd453e0 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/drivers/intel/gma/acpi/gma.asl M src/mainboard/51nb/x210/acpi/graphics.asl M src/northbridge/intel/gm45/acpi/gm45.asl M src/northbridge/intel/haswell/acpi/hostbridge.asl M src/northbridge/intel/i945/acpi/i945.asl M src/northbridge/intel/i945/acpi/igd.asl M src/northbridge/intel/ironlake/acpi/ironlake.asl M src/northbridge/intel/pineview/acpi/pineview.asl M src/northbridge/intel/sandybridge/acpi/sandybridge.asl M src/northbridge/intel/x4x/acpi/x4x.asl M src/soc/intel/baytrail/acpi/southcluster.asl M src/soc/intel/braswell/acpi/southcluster.asl M src/soc/intel/broadwell/acpi/hostbridge.asl M src/soc/intel/skylake/acpi/pch.asl 14 files changed, 35 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/48862/5
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Include gfx.asl by default for all platforms... ......................................................................
Patch Set 5: Code-Review+2
(3 comments)
https://review.coreboot.org/c/coreboot/+/48862/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48862/5//COMMIT_MSG@7 PS5, Line 7: ... : : which nit: maybe move the ... in front of "which select" - just my opinion :)
https://review.coreboot.org/c/coreboot/+/48862/5//COMMIT_MSG@15 PS5, Line 15: eg e.g. Win...
https://review.coreboot.org/c/coreboot/+/48862/5/src/mainboard/51nb/x210/acp... File src/mainboard/51nb/x210/acpi/graphics.asl:
https://review.coreboot.org/c/coreboot/+/48862/5/src/mainboard/51nb/x210/acp... PS5, Line 5: nit: drop that newline?
Hello build bot (Jenkins), Nico Huber, Damien Zammit, Angel Pons, Michael Niewöhner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48862
to look at the new patch set (#6).
Change subject: drivers/intel/gma: Include gfx.asl by default for all platforms... ......................................................................
drivers/intel/gma: Include gfx.asl by default for all platforms...
which select INTEL_GMA_ACPI. Rework brightness level includes and platform-level asl files to avoid duplicate device definition for GFX0.
Include gfx.asl for Skylake/Kabylake, since all other soc/intel/common platforms already do. Adjust mb/51nb/x210 to prevent device redefinition.
Some OSes (e.g. Windows, MacOS) require/prefer the ACPI device for the IGD to exist, even if ACPI brightness controls are not utilized. This change adds a GFX0 ACPI device for all boards whose platforms select INTEL_GMA_ACPI without requiring non-functional brightness controls to be added at the board level.
Change-Id: Ie71bd5fc7acd926b7ce7da17fbc108670fd453e0 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/drivers/intel/gma/acpi/gma.asl M src/mainboard/51nb/x210/acpi/graphics.asl M src/northbridge/intel/gm45/acpi/gm45.asl M src/northbridge/intel/haswell/acpi/hostbridge.asl M src/northbridge/intel/i945/acpi/i945.asl M src/northbridge/intel/i945/acpi/igd.asl M src/northbridge/intel/ironlake/acpi/ironlake.asl M src/northbridge/intel/pineview/acpi/pineview.asl M src/northbridge/intel/sandybridge/acpi/sandybridge.asl M src/northbridge/intel/x4x/acpi/x4x.asl M src/soc/intel/baytrail/acpi/southcluster.asl M src/soc/intel/braswell/acpi/southcluster.asl M src/soc/intel/broadwell/acpi/hostbridge.asl M src/soc/intel/skylake/acpi/pch.asl 14 files changed, 35 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/48862/6
Hello build bot (Jenkins), Nico Huber, Damien Zammit, Angel Pons, Michael Niewöhner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48862
to look at the new patch set (#7).
Change subject: drivers/intel/gma: Include gfx.asl by default for all platforms... ......................................................................
drivers/intel/gma: Include gfx.asl by default for all platforms...
which select INTEL_GMA_ACPI. Rework brightness level includes and platform-level asl files to avoid duplicate device definition for GFX0.
Include gfx.asl for Skylake/Kabylake, since all other soc/intel/common platforms already do. Adjust mb/51nb/x210 to prevent device redefinition.
Some OSes (e.g. Windows, MacOS) require/prefer the ACPI device for the IGD to exist, even if ACPI brightness controls are not utilized. This change adds a GFX0 ACPI device for all boards whose platforms select INTEL_GMA_ACPI without requiring non-functional brightness controls to be added at the board level.
Change-Id: Ie71bd5fc7acd926b7ce7da17fbc108670fd453e0 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/drivers/intel/gma/acpi/gma.asl M src/mainboard/51nb/x210/acpi/graphics.asl M src/northbridge/intel/gm45/acpi/gm45.asl M src/northbridge/intel/haswell/acpi/hostbridge.asl M src/northbridge/intel/i945/acpi/i945.asl M src/northbridge/intel/i945/acpi/igd.asl M src/northbridge/intel/ironlake/acpi/ironlake.asl M src/northbridge/intel/pineview/acpi/pineview.asl M src/northbridge/intel/sandybridge/acpi/sandybridge.asl M src/northbridge/intel/x4x/acpi/x4x.asl M src/soc/intel/baytrail/acpi/southcluster.asl M src/soc/intel/braswell/acpi/southcluster.asl M src/soc/intel/broadwell/acpi/hostbridge.asl M src/soc/intel/skylake/acpi/pch.asl 14 files changed, 34 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/48862/7
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Include gfx.asl by default for all platforms... ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48862/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48862/5//COMMIT_MSG@7 PS5, Line 7: ... : : which
nit: maybe move the ... […]
I feel like it makes more sense to have in the subject line since without it it's unclear that the statement has an additional condition attached
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Include gfx.asl by default for all platforms... ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48862/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48862/5//COMMIT_MSG@15 PS5, Line 15: eg
e.g. Win...
Done
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Include gfx.asl by default for all platforms... ......................................................................
Patch Set 7: Code-Review+2
Michael Niewöhner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48862 )
Change subject: drivers/intel/gma: Include gfx.asl by default for all platforms... ......................................................................
drivers/intel/gma: Include gfx.asl by default for all platforms...
which select INTEL_GMA_ACPI. Rework brightness level includes and platform-level asl files to avoid duplicate device definition for GFX0.
Include gfx.asl for Skylake/Kabylake, since all other soc/intel/common platforms already do. Adjust mb/51nb/x210 to prevent device redefinition.
Some OSes (e.g. Windows, MacOS) require/prefer the ACPI device for the IGD to exist, even if ACPI brightness controls are not utilized. This change adds a GFX0 ACPI device for all boards whose platforms select INTEL_GMA_ACPI without requiring non-functional brightness controls to be added at the board level.
Change-Id: Ie71bd5fc7acd926b7ce7da17fbc108670fd453e0 Signed-off-by: Matt DeVillier matt.devillier@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/48862 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Michael Niewöhner foss@mniewoehner.de --- M src/drivers/intel/gma/acpi/gma.asl M src/mainboard/51nb/x210/acpi/graphics.asl M src/northbridge/intel/gm45/acpi/gm45.asl M src/northbridge/intel/haswell/acpi/hostbridge.asl M src/northbridge/intel/i945/acpi/i945.asl M src/northbridge/intel/i945/acpi/igd.asl M src/northbridge/intel/ironlake/acpi/ironlake.asl M src/northbridge/intel/pineview/acpi/pineview.asl M src/northbridge/intel/sandybridge/acpi/sandybridge.asl M src/northbridge/intel/x4x/acpi/x4x.asl M src/soc/intel/baytrail/acpi/southcluster.asl M src/soc/intel/braswell/acpi/southcluster.asl M src/soc/intel/broadwell/acpi/hostbridge.asl M src/soc/intel/skylake/acpi/pch.asl 14 files changed, 34 insertions(+), 8 deletions(-)
Approvals: build bot (Jenkins): Verified Michael Niewöhner: Looks good to me, approved
diff --git a/src/drivers/intel/gma/acpi/gma.asl b/src/drivers/intel/gma/acpi/gma.asl index 03b0491..2282110 100644 --- a/src/drivers/intel/gma/acpi/gma.asl +++ b/src/drivers/intel/gma/acpi/gma.asl @@ -1,9 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */
-Device (GFX0) +Scope (GFX0) { - Name (_ADR, 0x00020000) - OperationRegion (GFXC, PCI_Config, 0x00, 0x0100) Field (GFXC, DWordAcc, NoLock, Preserve) { diff --git a/src/mainboard/51nb/x210/acpi/graphics.asl b/src/mainboard/51nb/x210/acpi/graphics.asl index e703ba1..e577538 100644 --- a/src/mainboard/51nb/x210/acpi/graphics.asl +++ b/src/mainboard/51nb/x210/acpi/graphics.asl @@ -1,8 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */
-Device (GFX0) +Scope (GFX0) { - Name (_ADR, 0x00020000) Method (_DOS, 1, NotSerialized) { /* We never do anything in firmware, so _DOS is a noop */ diff --git a/src/northbridge/intel/gm45/acpi/gm45.asl b/src/northbridge/intel/gm45/acpi/gm45.asl index e4d8d66..f13133d 100644 --- a/src/northbridge/intel/gm45/acpi/gm45.asl +++ b/src/northbridge/intel/gm45/acpi/gm45.asl @@ -33,3 +33,6 @@
// PCIe graphics port 0:1.0 #include "peg.asl" + +// Integrated graphics 0:2.0 +#include <drivers/intel/gma/acpi/gfx.asl> diff --git a/src/northbridge/intel/haswell/acpi/hostbridge.asl b/src/northbridge/intel/haswell/acpi/hostbridge.asl index 28a33d8..a930afe 100644 --- a/src/northbridge/intel/haswell/acpi/hostbridge.asl +++ b/src/northbridge/intel/haswell/acpi/hostbridge.asl @@ -204,3 +204,6 @@ /* PCI Express Graphics */ #include "peg.asl" #endif + +/* Integrated graphics 0:2.0 */ +#include <drivers/intel/gma/acpi/gfx.asl> diff --git a/src/northbridge/intel/i945/acpi/i945.asl b/src/northbridge/intel/i945/acpi/i945.asl index cdc77b5f..c2142cc 100644 --- a/src/northbridge/intel/i945/acpi/i945.asl +++ b/src/northbridge/intel/i945/acpi/i945.asl @@ -77,4 +77,5 @@ #include "peg.asl"
// Integrated graphics 0:2.0 +#include <drivers/intel/gma/acpi/gfx.asl> #include "igd.asl" diff --git a/src/northbridge/intel/i945/acpi/igd.asl b/src/northbridge/intel/i945/acpi/igd.asl index 5258c52..4fc2da2 100644 --- a/src/northbridge/intel/i945/acpi/igd.asl +++ b/src/northbridge/intel/i945/acpi/igd.asl @@ -1,9 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */
-Device (GFX0) +Scope (GFX0) { - Name (_ADR, 0x00020000) - Name (BRIG, Package (0x12) { 0xf, diff --git a/src/northbridge/intel/ironlake/acpi/ironlake.asl b/src/northbridge/intel/ironlake/acpi/ironlake.asl index 3cf597d..4f9549c 100644 --- a/src/northbridge/intel/ironlake/acpi/ironlake.asl +++ b/src/northbridge/intel/ironlake/acpi/ironlake.asl @@ -36,3 +36,6 @@ Return(PDRS) } } + +/* Integrated graphics 0:2.0 */ +#include <drivers/intel/gma/acpi/gfx.asl> diff --git a/src/northbridge/intel/pineview/acpi/pineview.asl b/src/northbridge/intel/pineview/acpi/pineview.asl index 9515c31..5fb2b12 100644 --- a/src/northbridge/intel/pineview/acpi/pineview.asl +++ b/src/northbridge/intel/pineview/acpi/pineview.asl @@ -32,3 +32,6 @@
// PCIe graphics port 0:1.0 #include "peg.asl" + +// Integrated graphics 0:2.0 +#include <drivers/intel/gma/acpi/gfx.asl> diff --git a/src/northbridge/intel/sandybridge/acpi/sandybridge.asl b/src/northbridge/intel/sandybridge/acpi/sandybridge.asl index 728d8e3..1e47c1f 100644 --- a/src/northbridge/intel/sandybridge/acpi/sandybridge.asl +++ b/src/northbridge/intel/sandybridge/acpi/sandybridge.asl @@ -52,3 +52,6 @@ Return(PDRS) } } + +/* Integrated graphics 0:2.0 */ +#include <drivers/intel/gma/acpi/gfx.asl> diff --git a/src/northbridge/intel/x4x/acpi/x4x.asl b/src/northbridge/intel/x4x/acpi/x4x.asl index 5a3c0b6..b013da7 100644 --- a/src/northbridge/intel/x4x/acpi/x4x.asl +++ b/src/northbridge/intel/x4x/acpi/x4x.asl @@ -30,3 +30,6 @@
// PCIe graphics port 0:1.0 #include "peg.asl" + +// Integrated graphics 0:2.0 +#include <drivers/intel/gma/acpi/gfx.asl> diff --git a/src/soc/intel/baytrail/acpi/southcluster.asl b/src/soc/intel/baytrail/acpi/southcluster.asl index e3997d7..f59f240 100644 --- a/src/soc/intel/baytrail/acpi/southcluster.asl +++ b/src/soc/intel/baytrail/acpi/southcluster.asl @@ -256,3 +256,6 @@ // LPE Device #include "lpe.asl" } + +/* Integrated graphics 0:2.0 */ +#include <drivers/intel/gma/acpi/gfx.asl> diff --git a/src/soc/intel/braswell/acpi/southcluster.asl b/src/soc/intel/braswell/acpi/southcluster.asl index 2c31d40..d74647e 100644 --- a/src/soc/intel/braswell/acpi/southcluster.asl +++ b/src/soc/intel/braswell/acpi/southcluster.asl @@ -272,3 +272,6 @@ /* SCC Devices */ #include "scc.asl" } + +/* Integrated graphics 0:2.0 */ +#include <drivers/intel/gma/acpi/gfx.asl> diff --git a/src/soc/intel/broadwell/acpi/hostbridge.asl b/src/soc/intel/broadwell/acpi/hostbridge.asl index 3e7ced0..2b44c65 100644 --- a/src/soc/intel/broadwell/acpi/hostbridge.asl +++ b/src/soc/intel/broadwell/acpi/hostbridge.asl @@ -195,3 +195,6 @@
/* Configurable TDP */ #include "ctdp.asl" + +/* Integrated graphics 0:2.0 */ +#include <drivers/intel/gma/acpi/gfx.asl> diff --git a/src/soc/intel/skylake/acpi/pch.asl b/src/soc/intel/skylake/acpi/pch.asl index 6eea5bb..02e30f7 100644 --- a/src/soc/intel/skylake/acpi/pch.asl +++ b/src/soc/intel/skylake/acpi/pch.asl @@ -65,3 +65,6 @@ #if CONFIG(SOC_INTEL_COMMON_BLOCK_SGX) #include <soc/intel/common/acpi/sgx.asl> #endif + +/* Integrated graphics 0:2.0 */ +#include <drivers/intel/gma/acpi/gfx.asl>
-
Angel Pons (Code Review) -
Matt DeVillier (Code Review)
-
Michael Niewöhner (Code Review)
-
Nico Huber (Code Review)