Change in coreboot[master]: soc/intel/apollolake: Hook up GMA ACPI brightness controls

[Matt DeVillier (Code Review)]

Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40593 )

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

soc/intel/apollolake: Hook up GMA ACPI brightness controls

Add struct i915_gpu_controller_info for boards to supply info needed
to generate ACPI backlight control SSDT. Hook into soc/common framework
by implementing intel_igd_get_controller_info().

Change-Id: Ia62a88b58e7efd90f550000fc5b2cef0cb5fade7
Signed-off-by: Matt DeVillier 
---
M src/soc/intel/apollolake/chip.h
M src/soc/intel/apollolake/graphics.c
2 files changed, 13 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/40593/1

diff --git a/src/soc/intel/apollolake/chip.h b/src/soc/intel/apollolake/chip.h
index ac36b70..f69ea11 100644
--- a/src/soc/intel/apollolake/chip.h
+++ b/src/soc/intel/apollolake/chip.h
@@ -17,6 +17,7 @@
 #define _SOC_APOLLOLAKE_CHIP_H_

#include <commonlib/helpers.h>
+#include <drivers/intel/gma/i915_gma.h>
 #include <intelblocks/cfg.h>
 #include <intelblocks/gspi.h>
 #include <soc/gpe.h>
@@ -192,6 +193,9 @@
     * 0:Enable (default), 1:Disable.
     */
    uint8_t disable_xhci_lfps_pm;
+
+	/* i915 struct for GMA backlight control */
+	struct i915_gpu_controller_info gfx;
 };

typedef struct soc_intel_apollolake_config config_t;
diff --git a/src/soc/intel/apollolake/graphics.c b/src/soc/intel/apollolake/graphics.c
index 797df73..1e3e390 100644
--- a/src/soc/intel/apollolake/graphics.c
+++ b/src/soc/intel/apollolake/graphics.c
@@ -18,6 +18,7 @@
 #include <bootmode.h>
 #include <cbmem.h>
 #include <console/console.h>
+#include <drivers/intel/gma/i915_gma.h>
 #include <fsp/util.h>
 #include <device/device.h>
 #include <device/pci.h>
@@ -27,6 +28,7 @@
 #include <drivers/intel/gma/libgfxinit.h>
 #include <types.h>
 #include <soc/nvs.h>
+#include "chip.h"

uintptr_t gma_get_gnvs_aslb(const void *gnvs)
@@ -88,3 +90,10 @@
    current += sizeof(igd_opregion_t);
    return acpi_align_current(current);
 }
+
+const struct i915_gpu_controller_info *
+intel_igd_get_controller_info(struct device *device)
+{
+	struct soc_intel_apollolake_config *chip = device->chip_info;
+	return &chip->gfx;
+}

[Patrick Rudolph (Code Review)]

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

Patch Set 1: Code-Review+2

[Angel Pons (Code Review)]

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

Patch Set 1: Code-Review+2

[Matt DeVillier (Code Review)]

Hello build bot (Jenkins), Nico Huber, Angel Pons, Andrey Petrov, Patrick Rudolph,

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

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

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

soc/intel/apollolake: Hook up GMA ACPI brightness controls

Add struct i915_gpu_controller_info for boards to supply info needed
to generate ACPI backlight control SSDT. Hook into soc/common framework
by implementing intel_igd_get_controller_info().

Change-Id: Ia62a88b58e7efd90f550000fc5b2cef0cb5fade7
Signed-off-by: Matt DeVillier 
---
M src/soc/intel/apollolake/chip.h
M src/soc/intel/apollolake/graphics.c
2 files changed, 13 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/40593/2

[Nico Huber (Code Review)]

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

Patch Set 2:

Needs an update to include `gma.h`.

Beside that, APL has different registers for the PWM configuration.
There are two sets, all 32 bits:

* panel 1: freq 0xc8254, duty 0xc8258
* panel 2: freq 0xc8354, duty 0xc8358

I suggest that we add a Kconfig INTEL_GMA_PANEL_2 and override things
as follows:

config INTEL_GMA_BCLV_OFFSET
            default 0xc8358 if INTEL_GMA_PANEL_2
            default 0xc8258

config INTEL_GMA_BCLV_WIDTH
            default 32

config INTEL_GMA_BCLM_OFFSET
            default 0xc8354 if INTEL_GMA_PANEL_2
            default 0xc8254

config INTEL_GMA_BCLM_WIDTH
            default 32

[Matt DeVillier (Code Review)]

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

Patch Set 2:
> Patch Set 2:
>
> Needs an update to include `gma.h`.
>
> Beside that, APL has different registers for the PWM configuration.
> There are two sets, all 32 bits:
> panel 1: freq 0xc8254, duty 0xc8258
> panel 2: freq 0xc8354, duty 0xc8358
>
> I suggest that we add a Kconfig INTEL_GMA_PANEL_2 and override things
> as follows:
> config INTEL_GMA_BCLV_OFFSET
>         default 0xc8358 if INTEL_GMA_PANEL_2
>         default 0xc8258
>
> config INTEL_GMA_BCLV_WIDTH
>         default 32
>
> config INTEL_GMA_BCLM_OFFSET
>         default 0xc8354 if INTEL_GMA_PANEL_2
>         default 0xc8254
>
> config INTEL_GMA_BCLM_WIDTH
>         default 32

what determines the use of panel 1 vs panel 2?

[Matt DeVillier (Code Review)]

Hello build bot (Jenkins), Nico Huber, Angel Pons, Andrey Petrov, Patrick Rudolph,

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

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

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

soc/intel/apollolake: Hook up GMA ACPI brightness controls

Add struct i915_gpu_controller_info for boards to supply info needed
to generate ACPI backlight control SSDT. Hook into soc/common framework
by implementing intel_igd_get_controller_info().

Change-Id: Ia62a88b58e7efd90f550000fc5b2cef0cb5fade7
Signed-off-by: Matt DeVillier 
---
M src/soc/intel/apollolake/chip.h
M src/soc/intel/apollolake/graphics.c
2 files changed, 13 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/40593/3

[Nico Huber (Code Review)]

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

Patch Set 3:
> > I suggest that we add a Kconfig INTEL_GMA_PANEL_2 and override things
> > as follows:
> > config INTEL_GMA_BCLV_OFFSET
> >         default 0xc8358 if INTEL_GMA_PANEL_2
> >         default 0xc8258
> >
> > config INTEL_GMA_BCLV_WIDTH
> >         default 32
> >
> > config INTEL_GMA_BCLM_OFFSET
> >         default 0xc8354 if INTEL_GMA_PANEL_2
> >         default 0xc8254
> >
> > config INTEL_GMA_BCLM_WIDTH
> >         default 32
>
> what determines the use of panel 1 vs panel 2?

Board design, mostly. The current gma/acpi code assumes that there
is only one panel, so the mainboard code would have to select one.
I'm not sure if i915 supports multiple panels yet. That might make
things harder to synchronize between our ACPI code and i915...

[Matt DeVillier (Code Review)]

Hello build bot (Jenkins), Nico Huber, Angel Pons, Andrey Petrov, Patrick Rudolph,

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

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

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

soc/intel/apollolake: Hook up GMA ACPI brightness controls

Add struct i915_gpu_controller_info for boards to supply info needed
to generate ACPI backlight control SSDT. Hook into soc/common framework
by implementing intel_igd_get_controller_info().

Change-Id: Ia62a88b58e7efd90f550000fc5b2cef0cb5fade7
Signed-off-by: Matt DeVillier 
---
M src/soc/intel/apollolake/chip.h
M src/soc/intel/apollolake/graphics.c
2 files changed, 10 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/40593/4

[Matt DeVillier (Code Review)]

Hello build bot (Jenkins), Nico Huber, Angel Pons, Andrey Petrov, Patrick Rudolph,

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

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

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

soc/intel/apollolake: Hook up GMA ACPI brightness controls

Add struct i915_gpu_controller_info for boards to supply info needed
to generate ACPI backlight control SSDT. Hook into soc/common framework
by implementing intel_igd_get_controller_info().

Change-Id: Ia62a88b58e7efd90f550000fc5b2cef0cb5fade7
Signed-off-by: Matt DeVillier 
---
M src/soc/intel/apollolake/chip.h
M src/soc/intel/apollolake/graphics.c
2 files changed, 10 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/40593/5

[Angel Pons (Code Review)]

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

Patch Set 5: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/40593/5/src/soc/intel/apollolake/ch... 
File src/soc/intel/apollolake/chip.h:

https://review.coreboot.org/c/coreboot/+/40593/5/src/soc/intel/apollolake/ch... 
PS5, Line 192: 	struct i915_gpu_controller_info gfx;
Why not place this next to panel_cfg?

[Matt DeVillier (Code Review)]

Hello build bot (Jenkins), Nico Huber, Angel Pons, Andrey Petrov, Patrick Rudolph,

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

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

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

soc/intel/apollolake: Hook up GMA ACPI brightness controls

Add struct i915_gpu_controller_info for boards to supply info needed
to generate ACPI backlight control SSDT. Hook into soc/common framework
by implementing intel_igd_get_controller_info().

Change-Id: Ia62a88b58e7efd90f550000fc5b2cef0cb5fade7
Signed-off-by: Matt DeVillier 
---
M src/soc/intel/apollolake/chip.h
M src/soc/intel/apollolake/graphics.c
2 files changed, 10 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/40593/6

[Matt DeVillier (Code Review)]

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

Patch Set 6:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40593/5/src/soc/intel/apollolake/ch... 
File src/soc/intel/apollolake/chip.h:

https://review.coreboot.org/c/coreboot/+/40593/5/src/soc/intel/apollolake/ch... 
PS5, Line 192: 	struct i915_gpu_controller_info gfx;
> Why not place this next to panel_cfg?

Done

[Angel Pons (Code Review)]

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

Patch Set 6: Code-Review+2

[Nico Huber (Code Review)]

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

Patch Set 6: Code-Review-1
> Needs an update to include `gma.h`.
>
> Beside that, APL has different registers for the PWM configuration.
> There are two sets, all 32 bits:
> panel 1: freq 0xc8254, duty 0xc8258
> panel 2: freq 0xc8354, duty 0xc8358
>
> I suggest that we add a Kconfig INTEL_GMA_PANEL_2 and override things
> as follows:
> config INTEL_GMA_BCLV_OFFSET
>         default 0xc8358 if INTEL_GMA_PANEL_2
>         default 0xc8258
>
> config INTEL_GMA_BCLV_WIDTH
>         default 32
>
> config INTEL_GMA_BCLM_OFFSET
>         default 0xc8354 if INTEL_GMA_PANEL_2
>         default 0xc8254
>
> config INTEL_GMA_BCLM_WIDTH
>         default 32

Giving this a bump as it seems it was forgotten.

[Angel Pons (Code Review)]

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

Patch Set 6: Code-Review-1

[Matt DeVillier (Code Review)]

Hello build bot (Jenkins), Nico Huber, Angel Pons, Andrey Petrov, Patrick Rudolph,

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

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

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

soc/intel/apollolake: Hook up GMA ACPI brightness controls

Add struct i915_gpu_controller_info for boards to supply info needed
to generate ACPI backlight control SSDT.

Hook into soc/common framework by implementing intel_igd_get_controller_info().

Add Kconfig entries to set the correct register offets for backlight
frequency and duty cycle.

Change-Id: Ia62a88b58e7efd90f550000fc5b2cef0cb5fade7
Signed-off-by: Matt DeVillier 
---
M src/soc/intel/apollolake/Kconfig
M src/soc/intel/apollolake/chip.h
M src/soc/intel/apollolake/graphics.c
3 files changed, 28 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/40593/7

[Nico Huber (Code Review)]

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

Patch Set 7: Code-Review+2

[Angel Pons (Code Review)]

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

Patch Set 8: Code-Review+2
> Patch Set 6: Code-Review-1
> > Needs an update to include `gma.h`.
> >
> > Beside that, APL has different registers for the PWM configuration.
> > There are two sets, all 32 bits:
> > panel 1: freq 0xc8254, duty 0xc8258
> > panel 2: freq 0xc8354, duty 0xc8358
> >
> > I suggest that we add a Kconfig INTEL_GMA_PANEL_2 and override things
> > as follows:
> > config INTEL_GMA_BCLV_OFFSET
> >         default 0xc8358 if INTEL_GMA_PANEL_2
> >         default 0xc8258
> >
> > config INTEL_GMA_BCLV_WIDTH
> >         default 32
> >
> > config INTEL_GMA_BCLM_OFFSET
> >         default 0xc8354 if INTEL_GMA_PANEL_2
> >         default 0xc8254
> >
> > config INTEL_GMA_BCLM_WIDTH
> >         default 32
>
> Giving this a bump as it seems it was forgotten.

This should be good enough until someone wants to port some APL thing with two panels, but we can always revisit this later.

[Angel Pons (Code Review)]

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

Patch Set 8:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40593/8//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40593/8//COMMIT_MSG@14 
PS8, Line 14: offets
argh, just noticed: off*s*ets

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40593 )

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

Patch Set 8: Code-Review+2

[Matt DeVillier (Code Review)]

Hello build bot (Jenkins), Nico Huber, Angel Pons, Michael Niewöhner, Andrey Petrov, Patrick Rudolph,

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

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

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

soc/intel/apollolake: Hook up GMA ACPI brightness controls

Add struct i915_gpu_controller_info for boards to supply info needed
to generate ACPI backlight control SSDT.

Hook into soc/common framework by implementing intel_igd_get_controller_info().

Add Kconfig entries to set the correct register offsets for backlight
frequency and duty cycle.

Change-Id: Ia62a88b58e7efd90f550000fc5b2cef0cb5fade7
Signed-off-by: Matt DeVillier 
---
M src/soc/intel/apollolake/Kconfig
M src/soc/intel/apollolake/chip.h
M src/soc/intel/apollolake/graphics.c
3 files changed, 28 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/40593/9

[Matt DeVillier (Code Review)]

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

Patch Set 9:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40593/8//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40593/8//COMMIT_MSG@14 
PS8, Line 14: offets
> argh, just noticed: off*s*ets

Done

[Michael Niewöhner (Code Review)]

Michael Niewöhner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40593 )

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

soc/intel/apollolake: Hook up GMA ACPI brightness controls

Add struct i915_gpu_controller_info for boards to supply info needed
to generate ACPI backlight control SSDT.

Hook into soc/common framework by implementing intel_igd_get_controller_info().

Add Kconfig entries to set the correct register offsets for backlight
frequency and duty cycle.

Change-Id: Ia62a88b58e7efd90f550000fc5b2cef0cb5fade7
Signed-off-by: Matt DeVillier 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40593
Reviewed-by: Angel Pons 
Reviewed-by: Michael Niewöhner 
Reviewed-by: Nico Huber 
Tested-by: build bot (Jenkins) 
---
M src/soc/intel/apollolake/Kconfig
M src/soc/intel/apollolake/chip.h
M src/soc/intel/apollolake/graphics.c
3 files changed, 28 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved
  Angel Pons: Looks good to me, approved
  Michael Niewöhner: Looks good to me, approved

diff --git a/src/soc/intel/apollolake/Kconfig b/src/soc/intel/apollolake/Kconfig
index 4f762a9..62049b5 100644
--- a/src/soc/intel/apollolake/Kconfig
+++ b/src/soc/intel/apollolake/Kconfig
@@ -413,4 +413,22 @@
    string
    default "pdpt pt"

+config INTEL_GMA_PANEL_2
+	bool
+	default n
+
+config INTEL_GMA_BCLV_OFFSET
+	default 0xc8358 if INTEL_GMA_PANEL_2
+	default 0xc8258
+
+config INTEL_GMA_BCLV_WIDTH
+	default 32
+
+config INTEL_GMA_BCLM_OFFSET
+	default 0xc8354 if INTEL_GMA_PANEL_2
+	default 0xc8254
+
+config INTEL_GMA_BCLM_WIDTH
+	default 32
+
 endif
diff --git a/src/soc/intel/apollolake/chip.h b/src/soc/intel/apollolake/chip.h
index c94b7ef..482b333 100644
--- a/src/soc/intel/apollolake/chip.h
+++ b/src/soc/intel/apollolake/chip.h
@@ -41,6 +41,9 @@
     */
    struct i915_gpu_panel_config panel_cfg[2];

+	/* i915 struct for GMA backlight control */
+	struct i915_gpu_controller_info gfx;
+
    /*
     * Mapping from PCIe root port to CLKREQ input on the SOC. The SOC has
     * four CLKREQ inputs, but six root ports. Root ports without an
diff --git a/src/soc/intel/apollolake/graphics.c b/src/soc/intel/apollolake/graphics.c
index c24ccdf..257222f 100644
--- a/src/soc/intel/apollolake/graphics.c
+++ b/src/soc/intel/apollolake/graphics.c
@@ -74,3 +74,10 @@
    for (i = 0; i < ARRAY_SIZE(conf->panel_cfg); ++i)
    	graphics_configure_backlight(&conf->panel_cfg[i], mmio, i);
 }
+
+const struct i915_gpu_controller_info *
+intel_igd_get_controller_info(const struct device *device)
+{
+	struct soc_intel_apollolake_config *chip = device->chip_info;
+	return &chip->gfx;
+}

[Mike Banon (Code Review)]

Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40593 )

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

Patch Set 10:

(1 comment)

Patchset:

PS10: 
These Intel-related Kconfig options aren't properly guarded, which results in seeing them even at the AMD boards' generated configs:

CONFIG_INTEL_GMA_BCLV_OFFSET=0xc8254
CONFIG_INTEL_GMA_BCLV_WIDTH=16
CONFIG_INTEL_GMA_BCLM_OFFSET=0xc8256
CONFIG_INTEL_GMA_BCLM_WIDTH=16

Please guard them properly

[Matt DeVillier (Code Review)]

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

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

Patch Set 10:

(1 comment)

Patchset:

PS10:
> These Intel-related Kconfig options aren't properly guarded, which results in seeing them even at th […]

how are they not guarded properly? they're set inside `if SOC_INTEL_APOLLOLAKE`

[Angel Pons (Code Review)]

Attention is currently required from: Matt DeVillier.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40593 )

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

Patch Set 10:

(1 comment)

Patchset:

PS10:
> how are they not guarded properly? they're set inside `if SOC_INTEL_APOLLOLAKE`

I guess this can happen because SOC_INTEL_GEMINILAKE has an explicit default.

[Nico Huber (Code Review)]

Attention is currently required from: Matt DeVillier.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40593 )

Change subject: soc/intel/apollolake: Hook up GMA ACPI brightness controls
......................................................................

Patch Set 10:

(1 comment)

Patchset:

PS10:
> I guess this can happen because SOC_INTEL_GEMINILAKE has an explicit default.

The numbers don't match this patch at all. It's likely the defaults in
`drivers/intel/gma/Kconfig` that leak. That file could use some refactoring
indeed.