Change in coreboot[master]: drivers/intel/gma: put controller in separate header

[Matt DeVillier (Code Review)]

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

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

drivers/intel/gma: put controller in separate header

Including i915.h just for the GMA/SSDT related functions means
dragging along all of i915_reg.h as well, which is problematic
since some platforms (like Apollolake) use overlapping symbols.
To avoid this conflict, break out the GMA/SSDT bits into their
own header which can be included without conflict.

Change-Id: I73fb7ef01abaafdcdbc44f1e3f5eb1883fc31616
Signed-off-by: Matt DeVillier 
---
M src/drivers/intel/gma/i915.h
A src/drivers/intel/gma/i915_gma.h
2 files changed, 25 insertions(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/40592/1

diff --git a/src/drivers/intel/gma/i915.h b/src/drivers/intel/gma/i915.h
index f8721a2..cede8ee 100644
--- a/src/drivers/intel/gma/i915.h
+++ b/src/drivers/intel/gma/i915.h
@@ -4,6 +4,7 @@
 #ifndef INTEL_I915_H
 #define INTEL_I915_H 1

+#include <drivers/intel/gma/i915_gma.h>
 #include <drivers/intel/gma/i915_reg.h>
 #include <drivers/intel/gma/drm_dp_helper.h>
 #include <edid.h>
@@ -75,21 +76,6 @@
 void gtt_write(u32 reg, u32 data);
 u32 gtt_read(u32 reg);

-struct i915_gpu_controller_info
-{
-	int use_spread_spectrum_clock;
-	int ndid;
-	u32 did[5];
-};
-
-#define GMA_STATIC_DISPLAYS(ssc) {			\
-	.use_spread_spectrum_clock = (ssc),		\
-	.ndid = 3, .did = { 0x0100, 0x0240, 0x0410, }	\
-}
-
-void
-drivers_intel_gma_displays_ssdt_generate(const struct i915_gpu_controller_info *conf);
-
 /* vbt.c */
 struct device;
 void
diff --git a/src/drivers/intel/gma/i915_gma.h b/src/drivers/intel/gma/i915_gma.h
new file mode 100644
index 0000000..4b4da95
--- /dev/null
+++ b/src/drivers/intel/gma/i915_gma.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* This file is part of the coreboot project. */
+
+#ifndef _I915_GMA_H_
+#define _I915_GMA_H_
+
+#include <stdint.h>
+
+struct i915_gpu_controller_info
+{
+	int use_spread_spectrum_clock;
+	int ndid;
+	u32 did[5];
+};
+
+#define GMA_STATIC_DISPLAYS(ssc) {			\
+	.use_spread_spectrum_clock = (ssc),		\
+	.ndid = 3, .did = { 0x0100, 0x0240, 0x0410, }	\
+}
+
+void
+drivers_intel_gma_displays_ssdt_generate(const struct i915_gpu_controller_info *conf);
+
+#endif

[build bot (Jenkins) (Code Review)]

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

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40592/1/src/drivers/intel/gma/i915_... 
File src/drivers/intel/gma/i915_gma.h:

https://review.coreboot.org/c/coreboot/+/40592/1/src/drivers/intel/gma/i915_... 
PS1, Line 10: {
open brace '{' following struct go on the same line

[Angel Pons (Code Review)]

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

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

Patch Set 1: Code-Review+1

(2 comments)

https://review.coreboot.org/c/coreboot/+/40592/1/src/drivers/intel/gma/i915_... 
File src/drivers/intel/gma/i915_gma.h:

https://review.coreboot.org/c/coreboot/+/40592/1/src/drivers/intel/gma/i915_... 
PS1, Line 10: {
> open brace '{' following struct go on the same line

I agree

https://review.coreboot.org/c/coreboot/+/40592/1/src/drivers/intel/gma/i915_... 
PS1, Line 22: drivers_intel_gma_displays_ssdt_generate(const struct i915_gpu_controller_info *conf);
Fiiits in 96 chars

[Matt DeVillier (Code Review)]

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

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

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

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

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

drivers/intel/gma: put controller in separate header

Including i915.h just for the GMA/SSDT related functions means
dragging along all of i915_reg.h as well, which is problematic
since some platforms (like Apollolake) use overlapping symbols.
To avoid this conflict, break out the GMA/SSDT bits into their
own header which can be included without conflict.

Change-Id: I73fb7ef01abaafdcdbc44f1e3f5eb1883fc31616
Signed-off-by: Matt DeVillier 
---
M src/drivers/intel/gma/i915.h
A src/drivers/intel/gma/i915_gma.h
2 files changed, 23 insertions(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/40592/2

[Matt DeVillier (Code Review)]

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

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/40592/1/src/drivers/intel/gma/i915_... 
File src/drivers/intel/gma/i915_gma.h:

https://review.coreboot.org/c/coreboot/+/40592/1/src/drivers/intel/gma/i915_... 
PS1, Line 10: {
> I agree

Done

https://review.coreboot.org/c/coreboot/+/40592/1/src/drivers/intel/gma/i915_... 
PS1, Line 22: drivers_intel_gma_displays_ssdt_generate(const struct i915_gpu_controller_info *conf);
> Fiiits in 96 chars

Done

[Paul Menzel (Code Review)]

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

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40592/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40592/2//COMMIT_MSG@11 
PS2, Line 11: Apollolake
Apollo Lake

[Matt DeVillier (Code Review)]

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

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

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

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

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

drivers/intel/gma: put controller in separate header

Including i915.h just for the GMA/SSDT related functions means
dragging along all of i915_reg.h as well, which is problematic
since some platforms (like Apollo Lake) use overlapping symbols.
To avoid this conflict, break out the GMA/SSDT bits into their
own header which can be included without conflict.

Change-Id: I73fb7ef01abaafdcdbc44f1e3f5eb1883fc31616
Signed-off-by: Matt DeVillier 
---
M src/drivers/intel/gma/i915.h
A src/drivers/intel/gma/i915_gma.h
2 files changed, 23 insertions(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/40592/3

[Matt DeVillier (Code Review)]

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

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40592/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40592/2//COMMIT_MSG@11 
PS2, Line 11: Apollolake
> Apollo Lake

Done

[Angel Pons (Code Review)]

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

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

Patch Set 3: Code-Review+2

[Matt DeVillier (Code Review)]

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

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

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

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

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

drivers/intel/gma: put controller in separate header

Including i915.h just for the GMA/SSDT related functions means
dragging along all of i915_reg.h as well, which is problematic
since some platforms (like Apollolake) use overlapping symbols.
To avoid this conflict, break out the GMA/SSDT bits into their
own header which can be included without conflict.

Change-Id: I73fb7ef01abaafdcdbc44f1e3f5eb1883fc31616
Signed-off-by: Matt DeVillier 
---
A src/drivers/intel/gma/gma.h
M src/drivers/intel/gma/i915.h
2 files changed, 25 insertions(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/40592/4

[build bot (Jenkins) (Code Review)]

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

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40592/4/src/drivers/intel/gma/gma.h 
File src/drivers/intel/gma/gma.h:

https://review.coreboot.org/c/coreboot/+/40592/4/src/drivers/intel/gma/gma.h... 
PS4, Line 10: {
open brace '{' following struct go on the same line

[Matt DeVillier (Code Review)]

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

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

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

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

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

drivers/intel/gma: put controller in separate header

Including i915.h just for the GMA/SSDT related functions means
dragging along all of i915_reg.h as well, which is problematic
since some platforms (like Apollolake) use overlapping symbols.
To avoid this conflict, break out the GMA/SSDT bits into their
own header which can be included without conflict.

Change-Id: I73fb7ef01abaafdcdbc44f1e3f5eb1883fc31616
Signed-off-by: Matt DeVillier 
---
A src/drivers/intel/gma/gma.h
M src/drivers/intel/gma/i915.h
2 files changed, 24 insertions(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/40592/5

[Nico Huber (Code Review)]

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

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

Patch Set 5: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/40592/5/src/drivers/intel/gma/gma.h 
File src/drivers/intel/gma/gma.h:

https://review.coreboot.org/c/coreboot/+/40592/5/src/drivers/intel/gma/gma.h... 
PS5, Line 4: I915_
nit, doesn't reflect path/filename

(I don't really care, just noticed)

[Angel Pons (Code Review)]

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

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

Patch Set 5: Code-Review+1

(2 comments)

https://review.coreboot.org/c/coreboot/+/40592/5//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40592/5//COMMIT_MSG@11 
PS5, Line 11: Apollolake
woups, was lost from PS3

https://review.coreboot.org/c/coreboot/+/40592/5/src/drivers/intel/gma/gma.h 
File src/drivers/intel/gma/gma.h:

https://review.coreboot.org/c/coreboot/+/40592/5/src/drivers/intel/gma/gma.h... 
PS5, Line 20: void
This was also undone

[Nico Huber (Code Review)]

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

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40592/5//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40592/5//COMMIT_MSG@10 
PS5, Line 10: along all of i915_reg.h
Actually something we could fix as well. why have all the indirect
includes?

[Matt DeVillier (Code Review)]

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

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

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

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

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

drivers/intel/gma: put controller in separate header

Including i915.h just for the GMA/SSDT related functions means
dragging along all of i915_reg.h as well, which is problematic
since some platforms (like Apollolake) use overlapping symbols.
To avoid this conflict, break out the GMA/SSDT bits into their
own header which can be included without conflict.

Change-Id: I73fb7ef01abaafdcdbc44f1e3f5eb1883fc31616
Signed-off-by: Matt DeVillier 
---
A src/drivers/intel/gma/gma.h
M src/drivers/intel/gma/i915.h
2 files changed, 23 insertions(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/40592/6

[Matt DeVillier (Code Review)]

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

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

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

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

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

drivers/intel/gma: put controller in separate header

Including i915.h just for the GMA/SSDT related functions means
dragging along all of i915_reg.h as well, which is problematic
since some platforms (like Apollo Lake) use overlapping symbols.
To avoid this conflict, break out the GMA/SSDT bits into their
own header which can be included without conflict.

Change-Id: I73fb7ef01abaafdcdbc44f1e3f5eb1883fc31616
Signed-off-by: Matt DeVillier 
---
A src/drivers/intel/gma/gma.h
M src/drivers/intel/gma/i915.h
2 files changed, 23 insertions(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/40592/7

[Matt DeVillier (Code Review)]

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

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

Patch Set 7:

(4 comments)

https://review.coreboot.org/c/coreboot/+/40592/5//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40592/5//COMMIT_MSG@10 
PS5, Line 10: along all of i915_reg.h
> Actually something we could fix as well. why have all the indirect […]

what do you propose here? or something for a follow on patch?

https://review.coreboot.org/c/coreboot/+/40592/5//COMMIT_MSG@11 
PS5, Line 11: Apollolake
> woups, was lost from PS3

Done

https://review.coreboot.org/c/coreboot/+/40592/5/src/drivers/intel/gma/gma.h 
File src/drivers/intel/gma/gma.h:

https://review.coreboot.org/c/coreboot/+/40592/5/src/drivers/intel/gma/gma.h... 
PS5, Line 4: I915_
> nit, doesn't reflect path/filename […]

Done

https://review.coreboot.org/c/coreboot/+/40592/5/src/drivers/intel/gma/gma.h... 
PS5, Line 20: void
> This was also undone

Done

[Nico Huber (Code Review)]

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

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

Patch Set 7: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/40592/5//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40592/5//COMMIT_MSG@10 
PS5, Line 10: along all of i915_reg.h
> what do you propose here? or something for a follow on patch?

Follow up, if you want. These cleanups never end... I only noticed
that we could as well have removed the `#include <i915_reg.h>` from
`i915.h`. The latter is just overall messy.

[Matt DeVillier (Code Review)]

Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40592 )

Change subject: drivers/intel/gma: put controller in separate header
......................................................................

drivers/intel/gma: put controller in separate header

Including i915.h just for the GMA/SSDT related functions means
dragging along all of i915_reg.h as well, which is problematic
since some platforms (like Apollo Lake) use overlapping symbols.
To avoid this conflict, break out the GMA/SSDT bits into their
own header which can be included without conflict.

Change-Id: I73fb7ef01abaafdcdbc44f1e3f5eb1883fc31616
Signed-off-by: Matt DeVillier 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40592
Tested-by: build bot (Jenkins) 
Reviewed-by: Nico Huber 
---
A src/drivers/intel/gma/gma.h
M src/drivers/intel/gma/i915.h
2 files changed, 23 insertions(+), 15 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved

diff --git a/src/drivers/intel/gma/gma.h b/src/drivers/intel/gma/gma.h
new file mode 100644
index 0000000..7d20e6b
--- /dev/null
+++ b/src/drivers/intel/gma/gma.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* This file is part of the coreboot project. */
+
+#ifndef _GMA_H_
+#define _GMA_H_
+
+#include <stdint.h>
+
+struct i915_gpu_controller_info {
+	int use_spread_spectrum_clock;
+	int ndid;
+	u32 did[5];
+};
+
+#define GMA_STATIC_DISPLAYS(ssc) {			\
+	.use_spread_spectrum_clock = (ssc),		\
+	.ndid = 3, .did = { 0x0100, 0x0240, 0x0410, }	\
+}
+
+void drivers_intel_gma_displays_ssdt_generate(const struct i915_gpu_controller_info *conf);
+
+#endif
diff --git a/src/drivers/intel/gma/i915.h b/src/drivers/intel/gma/i915.h
index f8721a2..e18c795 100644
--- a/src/drivers/intel/gma/i915.h
+++ b/src/drivers/intel/gma/i915.h
@@ -6,6 +6,7 @@

#include <drivers/intel/gma/i915_reg.h>
 #include <drivers/intel/gma/drm_dp_helper.h>
+#include <drivers/intel/gma/gma.h>
 #include <edid.h>

/* port types. We stick with the same defines as the kernel */
@@ -75,21 +76,6 @@
 void gtt_write(u32 reg, u32 data);
 u32 gtt_read(u32 reg);

-struct i915_gpu_controller_info
-{
-	int use_spread_spectrum_clock;
-	int ndid;
-	u32 did[5];
-};
-
-#define GMA_STATIC_DISPLAYS(ssc) {			\
-	.use_spread_spectrum_clock = (ssc),		\
-	.ndid = 3, .did = { 0x0100, 0x0240, 0x0410, }	\
-}
-
-void
-drivers_intel_gma_displays_ssdt_generate(const struct i915_gpu_controller_info *conf);
-
 /* vbt.c */
 struct device;
 void