Change in ...libgfxinit[master]: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake

[Nico Huber (Code Review)]

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/libgfxinit/+/31453

Change subject: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake
......................................................................

gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake

These seem to be 100% compatible to Kaby Lake wrt. modesetting. So
treat them as the latter for now.

Untested. Didn't look at documented workarounds, yet.

Change-Id: If01883ba95246f9bfd66049772597e0317e294d2
Signed-off-by: Nico Huber 
---
M common/hw-gfx-gma-config.ads.template
1 file changed, 18 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/53/31453/1

diff --git a/common/hw-gfx-gma-config.ads.template b/common/hw-gfx-gma-config.ads.template
index d91d9c7..c3f3a7c 100644
--- a/common/hw-gfx-gma-config.ads.template
+++ b/common/hw-gfx-gma-config.ads.template
@@ -390,6 +390,14 @@
       (Device_Id and 16#ffcf#) = 16#590b# or
       (Device_Id and 16#ffcf#) = 16#590d#);

+   function Is_Coffee_Lake_Y_AML (Device_Id : Word16) return Boolean is
+     (Device_Id = 16#87ca#);
+   -- Including Whiskey Lake:
+   function Is_Coffee_Lake_U (Device_Id : Word16) return Boolean is
+     ((Device_Id and 16#fff0#) = 16#3ea0#);
+   function Is_Coffee_Lake (Device_Id : Word16) return Boolean is
+     ((Device_Id and 16#fff0#) = 16#3e90#);
+
    function Is_GPU (Device_Id : Word16; CPU : CPU_Type; CPU_Var : CPU_Variant)
       return Boolean is
      (case CPU is
@@ -412,10 +420,16 @@
                                  when ULT    => Is_Skylake_U (Device_Id),
                                  when ULX    => Is_Skylake_Y (Device_Id)),
          when Kabylake     => (case CPU_Var is
-                                 when Normal => Is_Kaby_Lake (Device_Id),
-                                 when ULT    => Is_Kaby_Lake_U (Device_Id),
-                                 when ULX    => Is_Kaby_Lake_Y (Device_Id) or
-                                                Is_Kaby_Lake_Y_AML (Device_Id)));
+                                 when Normal =>
+                                    Is_Kaby_Lake (Device_Id) or
+                                    Is_Coffee_Lake (Device_Id),
+                                 when ULT    =>
+                                    Is_Kaby_Lake_U (Device_Id) or
+                                    Is_Coffee_Lake_U (Device_Id),
+                                 when ULX    =>
+                                    Is_Kaby_Lake_Y (Device_Id) or
+                                    Is_Kaby_Lake_Y_AML (Device_Id) or
+                                    Is_Coffee_Lake_Y_AML (Device_Id)));

function Compatible_GPU (Device_Id : Word16) return Boolean is
      (Is_GPU (Device_Id, CPU, CPU_Var));

[Thomas Heijligen (Code Review)]

Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/31453 )

Change subject: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake
......................................................................

Patch Set 3: Code-Review+1

Working on with <Linear "high-resolution" framebuffer> mode on Clevo n130bu (i7-7500U Kabylake) (clevo/kbl-u in gerrit)

[Lijian Zhao (Code Review)]

Lijian Zhao has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/31453 )

Change subject: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake
......................................................................

Patch Set 5:

GFX device id?src/soc/intel/common/block/graphics/graphics.c shall have the whole list.

[Nico Huber (Code Review)]

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

Change subject: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake
......................................................................

Patch Set 5:
> Patch Set 5:
>
> GFX device id?src/soc/intel/common/block/graphics/graphics.c shall have the whole list.

Thanks. Alas, this list is far from complete. It only contains
the IDs that are already used with upstream coreboot.

In the past, Intel had comprehensive lists in the Specification
Updates. This stopped at some point. But I've learned in the
meantime that there are "Stepping and IDs" User Guides now,
e.g. [1][2]. Those have 28 IDs for KBL+CFL+AML alone, the
coreboot source only 12 of them.

As these are NDA documents, I only add IDs that are publicly
available, e.g. in Linux source code. Maybe you know public
versions of these documents? I don't think they are kept under
wraps on purpose.

[1] 567957
[2] 573255

[Lijian Zhao (Code Review)]

Lijian Zhao has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/31453 )

Change subject: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake
......................................................................

Patch Set 5:
> Patch Set 5:
> > Patch Set 5:
> >
> > GFX device id?src/soc/intel/common/block/graphics/graphics.c shall have the whole list.
>
> Thanks. Alas, this list is far from complete. It only contains
> the IDs that are already used with upstream coreboot.
>
> In the past, Intel had comprehensive lists in the Specification
> Updates. This stopped at some point. But I've learned in the
> meantime that there are "Stepping and IDs" User Guides now,
> e.g. [1][2]. Those have 28 IDs for KBL+CFL+AML alone, the
> coreboot source only 12 of them.
>
> As these are NDA documents, I only add IDs that are publicly
> available, e.g. in Linux source code. Maybe you know public
> versions of these documents? I don't think they are kept under
> wraps on purpose.
>
> [1] 567957
> [2] 573255

But pci devices ID must be published anyway right? The thing is we can add the missing 16 IDs but that had not been tested which will make it a little bit tricky .....

[Nico Huber (Code Review)]

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

Change subject: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake
......................................................................

Patch Set 5:
> > > GFX device id?src/soc/intel/common/block/graphics/graphics.c shall have the whole list.
> >
> > Thanks. Alas, this list is far from complete. It only contains
> > the IDs that are already used with upstream coreboot.
> >
> > In the past, Intel had comprehensive lists in the Specification
> > Updates. This stopped at some point. But I've learned in the
> > meantime that there are "Stepping and IDs" User Guides now,
> > e.g. [1][2]. Those have 28 IDs for KBL+CFL+AML alone, the
> > coreboot source only 12 of them.
> >
> > As these are NDA documents, I only add IDs that are publicly
> > available, e.g. in Linux source code. Maybe you know public
> > versions of these documents? I don't think they are kept under
> > wraps on purpose.
> >
> > [1] 567957
> > [2] 573255
>
> But pci devices ID must be published anyway right?

Maybe they are and I'm looking at the wrong places. But so
far I couldn't find any official, public equivalent to these
"confidential" Stepping and IDs guides. Maybe a question for
your documentation team (if there is such a thing at Intel).
> The thing is we can add the missing 16 IDs but that had not been tested which will make it a little bit tricky .....

Yeah, I guess it's a tradeoff. If you'd add them it might seem
that you pretend it works, while you never tested. If you don't
add them, it might take a developer of a coreboot port some
time to realize if they are missing. No way to win :-/

[Nico Huber (Code Review)]

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

Change subject: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake
......................................................................

Patch Set 6: Verified+1

Change in libgfxinit[master]: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake

[Angel Pons (Code Review)]

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

Change subject: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake
......................................................................

Patch Set 6: Code-Review+2

(1 comment)

https://review.coreboot.org/c/libgfxinit/+/31453/6//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/libgfxinit/+/31453/6//COMMIT_MSG@9 
PS6, Line 9: wrt
very minor: w.r.t.

Change in libgfxinit[master]: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake

[Nico Huber (Code Review)]

Hello Angel Pons, Arthur Heymans, Thomas Heijligen, Patrick Georgi,

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

https://review.coreboot.org/c/libgfxinit/+/31453

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

Change subject: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake
......................................................................

gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake

These seem to be 100% compatible to Kaby Lake w.r.t. modesetting. So
treat them as the latter for now.

Untested. Didn't look at documented workarounds, yet.

Change-Id: If01883ba95246f9bfd66049772597e0317e294d2
Signed-off-by: Nico Huber 
---
M common/hw-gfx-gma-config.ads.template
1 file changed, 18 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/53/31453/7

Change in libgfxinit[master]: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake

[Nico Huber (Code Review)]

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

Change subject: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake
......................................................................

Patch Set 7:

(1 comment)

https://review.coreboot.org/c/libgfxinit/+/31453/6//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/libgfxinit/+/31453/6//COMMIT_MSG@9 
PS6, Line 9: wrt
> very minor: w.r.t.

Thanks

Change in libgfxinit[master]: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake

[Nico Huber (Code Review)]

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

Change subject: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake
......................................................................

Patch Set 8: Verified+1 Code-Review+2

This was just a rebase...

Change in libgfxinit[master]: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake

[Nico Huber (Code Review)]

Nico Huber has removed a vote from this change. ( https://review.coreboot.org/c/libgfxinit/+/31453 )

Change subject: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake
......................................................................

Removed Code-Review+2 by Nico Huber

Change in libgfxinit[master]: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake

[Nico Huber (Code Review)]

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

Change subject: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake
......................................................................

Patch Set 8: Code-Review+2

Change in libgfxinit[master]: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake

[Nico Huber (Code Review)]

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/libgfxinit/+/31453 )

Change subject: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake
......................................................................

gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake

These seem to be 100% compatible to Kaby Lake w.r.t. modesetting. So
treat them as the latter for now.

Untested. Didn't look at documented workarounds, yet.

Change-Id: If01883ba95246f9bfd66049772597e0317e294d2
Signed-off-by: Nico Huber 
Reviewed-on: https://review.coreboot.org/c/libgfxinit/+/31453
---
M common/hw-gfx-gma-config.ads.template
1 file changed, 18 insertions(+), 4 deletions(-)

Approvals:
  Nico Huber: Verified; Looks good to me, approved

diff --git a/common/hw-gfx-gma-config.ads.template b/common/hw-gfx-gma-config.ads.template
index fc14a46..2667333 100644
--- a/common/hw-gfx-gma-config.ads.template
+++ b/common/hw-gfx-gma-config.ads.template
@@ -396,6 +396,14 @@
       (Device_Id and 16#ffcf#) = 16#590b# or
       (Device_Id and 16#ffcf#) = 16#590d#);

+   function Is_Coffee_Lake_Y_AML (Device_Id : Word16) return Boolean is
+     (Device_Id = 16#87ca#);
+   -- Including Whiskey Lake:
+   function Is_Coffee_Lake_U (Device_Id : Word16) return Boolean is
+     ((Device_Id and 16#fff0#) = 16#3ea0#);
+   function Is_Coffee_Lake (Device_Id : Word16) return Boolean is
+     ((Device_Id and 16#fff0#) = 16#3e90#);
+
    function Is_GPU (Device_Id : Word16; CPU : CPU_Type; CPU_Var : CPU_Variant)
       return Boolean is
      (case CPU is
@@ -418,10 +426,16 @@
                                  when ULT    => Is_Skylake_U (Device_Id),
                                  when ULX    => Is_Skylake_Y (Device_Id)),
          when Kabylake     => (case CPU_Var is
-                                 when Normal => Is_Kaby_Lake (Device_Id),
-                                 when ULT    => Is_Kaby_Lake_U (Device_Id),
-                                 when ULX    => Is_Kaby_Lake_Y (Device_Id) or
-                                                Is_Kaby_Lake_Y_AML (Device_Id)));
+                                 when Normal =>
+                                    Is_Kaby_Lake (Device_Id) or
+                                    Is_Coffee_Lake (Device_Id),
+                                 when ULT    =>
+                                    Is_Kaby_Lake_U (Device_Id) or
+                                    Is_Coffee_Lake_U (Device_Id),
+                                 when ULX    =>
+                                    Is_Kaby_Lake_Y (Device_Id) or
+                                    Is_Kaby_Lake_Y_AML (Device_Id) or
+                                    Is_Coffee_Lake_Y_AML (Device_Id)));

function Compatible_GPU (Device_Id : Word16) return Boolean is
      (Is_GPU (Device_Id, CPU, CPU_Var));

Change in libgfxinit[master]: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake

[Angel Pons (Code Review)]

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

Change subject: gma: Add more PCI IDs for Coffee/Whiskey/Amber Lake
......................................................................

Patch Set 9: Code-Review+2

Still going to state this looks good