Change in ...coreboot[master]: asus/am1i-a: set VGA_BIOS_ID to 1002,9830 instead of default 1002,9836

[Mike Banon (Code Review)]

Mike Banon has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33777


Change subject: asus/am1i-a: set VGA_BIOS_ID to 1002,9830 instead of default 1002,9836
......................................................................

asus/am1i-a: set VGA_BIOS_ID to 1002,9830 instead of default 1002,9836

The majority of Socket AM1 APUs [1] - three out of five (three Athlons, the
most powerful for this socket) - have the integrated VGA with 1002,9830 ID,
while only one Sempron has 1002,9836. Set the default to more common one.

[1] https://en.wikipedia.org/wiki/List_of_AMD_accelerated_processing_units#%22Ka...)

Signed-off-by: Mike Banon <[e-mail]>
Change-Id: I75c815b13934afcb5be316f85933f7c200d55bbd
---
M src/mainboard/asus/am1i-a/Kconfig
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/33777/1

diff --git a/src/mainboard/asus/am1i-a/Kconfig b/src/mainboard/asus/am1i-a/Kconfig
index d50edbe..42ab5c6 100644
--- a/src/mainboard/asus/am1i-a/Kconfig
+++ b/src/mainboard/asus/am1i-a/Kconfig
@@ -44,7 +44,7 @@
 
 config VGA_BIOS_ID
 	string
-	default "1002,9836"
+	default "1002,9830"
 
 config HUDSON_LEGACY_FREE
 	bool

-- 
To view, visit https://review.coreboot.org/c/coreboot/+/33777
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I75c815b13934afcb5be316f85933f7c200d55bbd
Gerrit-Change-Number: 33777
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Banon <[e-mail]>
Gerrit-MessageType: newchange

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33777 )

Change subject: asus/am1i-a: set VGA_BIOS_ID to 1002,9830 instead of default 1002,9836
......................................................................


Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/33777/1/src/mainboard/asus/am1i-a/Kconfig 
File src/mainboard/asus/am1i-a/Kconfig:

https://review.coreboot.org/#/c/33777/1/src/mainboard/asus/am1i-a/Kconfig@47 
PS1, Line 47: default "1002,9830"
I think it is done here src/northbridge/amd/agesa/family16kb/Kconfig



-- 
To view, visit https://review.coreboot.org/c/coreboot/+/33777
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I75c815b13934afcb5be316f85933f7c200d55bbd
Gerrit-Change-Number: 33777
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Banon <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: HAOUAS Elyes <[e-mail]>
Gerrit-Comment-Date: Tue, 25 Jun 2019 08:02:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Angel Pons (Code Review)]

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

Change subject: asus/am1i-a: set VGA_BIOS_ID to 1002,9830 instead of default 1002,9836
......................................................................


Patch Set 1:

(2 comments)

Have you thought of removing the default ID? As Elyes pointed out, one is already defined on src/northbridge/amd/agesa/family16kb/Kconfig

https://review.coreboot.org/#/c/33777/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/33777/1//COMMIT_MSG@7 
PS1, Line 7: set VGA_BIOS_ID to 1002,9830 instead of default 1002,9836
No need to mention the IDs here


https://review.coreboot.org/#/c/33777/1/src/mainboard/asus/am1i-a/Kconfig 
File src/mainboard/asus/am1i-a/Kconfig:

https://review.coreboot.org/#/c/33777/1/src/mainboard/asus/am1i-a/Kconfig@47 
PS1, Line 47: default "1002,9830"
> I think it is done here src/northbridge/amd/agesa/family16kb/Kconfig
> True, there is no need to replace it here.

Change in ...coreboot[master]: asus/am1i-a: remove default VGA_BIOS_ID to make it the same as family...

[Mike Banon (Code Review)]

Hello build bot (Jenkins), 

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

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

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

Change subject: asus/am1i-a: remove default VGA_BIOS_ID to make it the same as family16kb
......................................................................

asus/am1i-a: remove default VGA_BIOS_ID to make it the same as family16kb

The majority of Socket AM1 APUs [1] - three out of five (three Athlons, the
most powerful for this socket) - have the integrated VGA with 1002,9830 ID,
while only one Sempron has 1002,9836. Remove it to make it 9830 like family16kb.

[1] https://en.wikipedia.org/wiki/List_of_AMD_accelerated_processing_units#%22Ka...)

Signed-off-by: Mike Banon <[e-mail]>
Change-Id: I75c815b13934afcb5be316f85933f7c200d55bbd
---
M src/mainboard/asus/am1i-a/Kconfig
1 file changed, 0 insertions(+), 4 deletions(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/33777/2
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/33777
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I75c815b13934afcb5be316f85933f7c200d55bbd
Gerrit-Change-Number: 33777
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Banon <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Angel Pons <[e-mail]>
Gerrit-CC: HAOUAS Elyes <[e-mail]>
Gerrit-MessageType: newpatchset

Change in ...coreboot[master]: asus/am1i-a: remove default VGA_BIOS_ID to make it the same as family...

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33777 )

Change subject: asus/am1i-a: remove default VGA_BIOS_ID to make it the same as family16kb
......................................................................


Patch Set 2:

now, we probably need to add the ID 1002,9836 for some Sempron into this file src/northbridge/amd/agesa/family16kb/Kconfig


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/33777
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I75c815b13934afcb5be316f85933f7c200d55bbd
Gerrit-Change-Number: 33777
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Banon <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Angel Pons <[e-mail]>
Gerrit-CC: HAOUAS Elyes <[e-mail]>
Gerrit-Comment-Date: Wed, 26 Jun 2019 06:35:50 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Change in ...coreboot[master]: asus/am1i-a: remove default VGA_BIOS_ID to make it the same as family...

[Mike Banon (Code Review)]

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

Change subject: asus/am1i-a: remove default VGA_BIOS_ID to make it the same as family16kb
......................................................................


Patch Set 2:
> Patch Set 2:
>
> now, we probably need to add the ID 1002,9836 for some Sempron into this file src/northbridge/amd/agesa/family16kb/Kconfig

No, we don't need to change anything there, since there is already a good 1002,9830 ID. This Sempron is less popular and less powerful than these three Athlons, so Athlon ID should be preferred.


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/33777
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I75c815b13934afcb5be316f85933f7c200d55bbd
Gerrit-Change-Number: 33777
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Banon <[e-mail]>
Gerrit-Reviewer: Mike Banon <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Angel Pons <[e-mail]>
Gerrit-CC: HAOUAS Elyes <[e-mail]>
Gerrit-Comment-Date: Wed, 26 Jun 2019 06:45:58 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Change in ...coreboot[master]: asus/am1i-a: remove default VGA_BIOS_ID to make it the same as family...

[Angel Pons (Code Review)]

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

Change subject: asus/am1i-a: remove default VGA_BIOS_ID to make it the same as family16kb
......................................................................


Patch Set 2: Code-Review+1

(3 comments)
> Patch Set 2:
> > Patch Set 2:
> >
> > now, we probably need to add the ID 1002,9836 for some Sempron into this file src/northbridge/amd/agesa/family16kb/Kconfig
>
> No, we don't need to change anything there, since there is already a good 1002,9830 ID. This Sempron is less popular and less powerful than these three Athlons, so Athlon ID should be preferred.

I think 9830 is a good default (three out of five APUs have that id) but I would probably document the other two IDs somewhere. Maybe in Documentation?

https://review.coreboot.org/#/c/33777/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/33777/2//COMMIT_MSG@7 
PS2, Line 7: remove default VGA_BIOS_ID to make it the same as family16kb
remove unnecessary VGA_BIOS_ID default


https://review.coreboot.org/#/c/33777/2//COMMIT_MSG@9 
PS2, Line 9: (three Athlons, the
           : most powerful for this socket)
It doesn't matter much if they are the most powerful ones or not.


https://review.coreboot.org/#/c/33777/2//COMMIT_MSG@11 
PS2, Line 11: Remove it to make it 9830 like family16kb.
Since VGA_BIOS_ID is already defined in fam16kb Kconfig, drop the value here.



-- 
To view, visit https://review.coreboot.org/c/coreboot/+/33777
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I75c815b13934afcb5be316f85933f7c200d55bbd
Gerrit-Change-Number: 33777
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Banon <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Mike Banon <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: HAOUAS Elyes <[e-mail]>
Gerrit-Comment-Date: Thu, 27 Jun 2019 01:38:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...coreboot[master]: asus/am1i-a: remove unnecessary VGA_BIOS_ID default

[Mike Banon (Code Review)]

Hello Angel Pons, build bot (Jenkins), 

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

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

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

Change subject: asus/am1i-a: remove unnecessary VGA_BIOS_ID default
......................................................................

asus/am1i-a: remove unnecessary VGA_BIOS_ID default

The majority of Socket AM1 APUs [1] - three out of five - have the integrated
VGA with 1002,9830 ID, while only one Sempron has 1002,9836. Since VGA_BIOS_ID
is already defined in fam16kb Kconfig as 1002,9830 ID, drop the value here.

[1] https://en.wikipedia.org/wiki/List_of_AMD_accelerated_processing_units#%22Ka...)

Signed-off-by: Mike Banon <[e-mail]>
Change-Id: I75c815b13934afcb5be316f85933f7c200d55bbd
---
M src/mainboard/asus/am1i-a/Kconfig
1 file changed, 0 insertions(+), 4 deletions(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/33777/3
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/33777
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I75c815b13934afcb5be316f85933f7c200d55bbd
Gerrit-Change-Number: 33777
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Banon <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Mike Banon <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: HAOUAS Elyes <[e-mail]>
Gerrit-MessageType: newpatchset

Change in ...coreboot[master]: asus/am1i-a: remove unnecessary VGA_BIOS_ID default

[Mike Banon (Code Review)]

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

Change subject: asus/am1i-a: remove unnecessary VGA_BIOS_ID default
......................................................................


Patch Set 3:

(3 comments)

Thank you very much, improved a commit message.

https://review.coreboot.org/#/c/33777/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/33777/2//COMMIT_MSG@7 
PS2, Line 7: remove default VGA_BIOS_ID to make it the same as family16kb
> remove unnecessary VGA_BIOS_ID default
> Done.

https://review.coreboot.org/#/c/33777/2//COMMIT_MSG@9 
PS2, Line 9: (three Athlons, the
           : most powerful for this socket)
> It doesn't matter much if they are the most powerful ones or not.
> Done.

https://review.coreboot.org/#/c/33777/2//COMMIT_MSG@11 
PS2, Line 11: Remove it to make it 9830 like family16kb.
> Since VGA_BIOS_ID is already defined in fam16kb Kconfig, drop the value here.
> Done (with a small addition)

Change in ...coreboot[master]: asus/am1i-a: remove unnecessary VGA_BIOS_ID default

[Mike Banon (Code Review)]

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

Change subject: asus/am1i-a: remove unnecessary VGA_BIOS_ID default
......................................................................


Patch Set 3:
> I think 9830 is a good default (three out of five APUs have that id) but I would probably document the other two IDs somewhere. Maybe in Documentation?

It would take some time for me to reach it


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/33777
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I75c815b13934afcb5be316f85933f7c200d55bbd
Gerrit-Change-Number: 33777
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Banon <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Mike Banon <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: HAOUAS Elyes <[e-mail]>
Gerrit-Comment-Date: Thu, 27 Jun 2019 06:49:12 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Change in coreboot[master]: asus/am1i-a: remove unnecessary VGA_BIOS_ID default

[Mike Banon (Code Review)]

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

Change subject: asus/am1i-a: remove unnecessary VGA_BIOS_ID default
......................................................................


Patch Set 5:

(2 comments)

https://review.coreboot.org/c/coreboot/+/33777/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/33777/1//COMMIT_MSG@7 
PS1, Line 7: set VGA_BIOS_ID to 1002,9830 instead of default 1002,9836
> No need to mention the IDs here
> Resolved.

https://review.coreboot.org/c/coreboot/+/33777/1/src/mainboard/asus/am1i-a/K... 
File src/mainboard/asus/am1i-a/Kconfig:

https://review.coreboot.org/c/coreboot/+/33777/1/src/mainboard/asus/am1i-a/K... 
PS1, Line 47: default "1002,9830"
> True, there is no need to replace it here.
> Resolved

Change in coreboot[master]: asus/am1i-a: remove unnecessary VGA_BIOS_ID default

[Angel Pons (Code Review)]

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

Change subject: asus/am1i-a: remove unnecessary VGA_BIOS_ID default
......................................................................


Patch Set 6: Code-Review+2


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/33777
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I75c815b13934afcb5be316f85933f7c200d55bbd
Gerrit-Change-Number: 33777
Gerrit-PatchSet: 6
Gerrit-Owner: Mike Banon <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Mike Banon <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Tue, 31 Dec 2019 20:43:42 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in coreboot[master]: asus/am1i-a: remove unnecessary VGA_BIOS_ID default

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/33777 )

Change subject: asus/am1i-a: remove unnecessary VGA_BIOS_ID default
......................................................................

asus/am1i-a: remove unnecessary VGA_BIOS_ID default

The majority of Socket AM1 APUs [1] - three out of five - have the integrated
VGA with 1002,9830 ID, while only one Sempron has 1002,9836. Since VGA_BIOS_ID
is already defined in fam16kb Kconfig as 1002,9830 ID, drop the value here.

[1] https://en.wikipedia.org/wiki/List_of_AMD_accelerated_processing_units#%22Ka...)

Signed-off-by: Mike Banon <[e-mail]>
Change-Id: I75c815b13934afcb5be316f85933f7c200d55bbd
Reviewed-on: https://review.coreboot.org/c/coreboot/+/33777
Reviewed-by: Angel Pons <[e-mail]>
Tested-by: build bot (Jenkins) <[e-mail]>
---
M src/mainboard/asus/am1i-a/Kconfig
1 file changed, 0 insertions(+), 4 deletions(-)

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



diff --git a/src/mainboard/asus/am1i-a/Kconfig b/src/mainboard/asus/am1i-a/Kconfig
index 947c2c5..a0dae9f 100644
--- a/src/mainboard/asus/am1i-a/Kconfig
+++ b/src/mainboard/asus/am1i-a/Kconfig
@@ -43,10 +43,6 @@
 	bool
 	default y
 
-config VGA_BIOS_ID
-	string
-	default "1002,9836"
-
 config HUDSON_LEGACY_FREE
 	bool
 	default n

-- 
To view, visit https://review.coreboot.org/c/coreboot/+/33777
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I75c815b13934afcb5be316f85933f7c200d55bbd
Gerrit-Change-Number: 33777
Gerrit-PatchSet: 7
Gerrit-Owner: Mike Banon <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Mike Banon <[e-mail]>
Gerrit-Reviewer: Patrick Georgi <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-MessageType: merged