Change in ...coreboot[master]: asus/am1i-a: set VGA_BIOS_ID to 1002,9830 instead of default 1002,9836
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 mikebdp2@gmail.com 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
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
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.
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 mikebdp2@gmail.com 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
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
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.
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.
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 mikebdp2@gmail.com 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
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)
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
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
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
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 mikebdp2@gmail.com Change-Id: I75c815b13934afcb5be316f85933f7c200d55bbd Reviewed-on: https://review.coreboot.org/c/coreboot/+/33777 Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- 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
-
Angel Pons (Code Review) -
HAOUAS Elyes (Code Review)
-
Mike Banon (Code Review)
-
Patrick Georgi (Code Review)