| Submitter | Uwe Hermann |
|---|---|
| Date | 2009-10-26 20:14:33 |
| Message ID | <20091026201432.GX22827@greenwood> |
| Download | mbox | patch |
| Permalink | /patch/494/ |
| State | Accepted |
| Headers | show |
Comments
On Mon, Oct 26, 2009 at 2:14 PM, Uwe Hermann <uwe@hermann-uwe.de> wrote: > See patch. > I liked the correspondence between the devices menu and src/devices. Maybe some other name would be better for the top-level menu? I don't think miscellaneous is really what we want to say for src/devices. I think it's pretty ugly to have 512 be the value for CONFIG_VIDEO_MB that means 512K. I know you didn't do that, but... Maybe we could just say the 512K of RAM doesn't make that much difference and make the minimum 1MB? With the menu names fixed: Acked-by: Myles Watson <mylesgw@gmail.com> Thanks, Myles
On Mon, Oct 26, 2009 at 02:26:36PM -0600, Myles Watson wrote: > On Mon, Oct 26, 2009 at 2:14 PM, Uwe Hermann <uwe@hermann-uwe.de> wrote: > > > See patch. > > > I liked the correspondence between the devices menu and src/devices. Maybe > some other name would be better for the top-level menu? I don't think > miscellaneous is really what we want to say for src/devices. The best I could come up with so far is "Chipset" for the top-level menu and leaving the "Devices" subsection intact. I'm happy about suggestions for better names. > I think it's pretty ugly to have 512 be the value for CONFIG_VIDEO_MB that > means 512K. I know you didn't do that, but... Maybe we could just say the > 512K of RAM doesn't make that much difference and make the minimum 1MB? Yeah, the 512KB thing is not really nice, but there's not much we can do about it. We could turn VIDEO_MB into VIDEO_KB instead to have the same unit everywhere, not sure if that's nicer or uglier. I don't want to leave out valid options supported by the hardware though, so the 512KB setting should stay. I think it's not _that_ big of a problem as the uglyness is only visible to the developers of two chipsets I think (i810 and i830), but not to the other developers and especially not to the user (who sees nice menuconfig entries). We can leave it as is IMHO. > With the menu names fixed: > Acked-by: Myles Watson <mylesgw@gmail.com> Thanks, r4866. libv mentioned on IRC that cn400 and cn700 values may be wrong, can somebody with datasheets please verify/fix them? Or maybe they can be extracted from unichrome at git://people.freedesktop.org/~libv/xf86-video-unichrome but I wasn't able to find the values after a quick glance (more detailed checking is probably needed). Uwe.
Sweet! Great work Uwe, I really like this. Even though it is too late: Acked-by: Joseph Smith <joe@settoplinux.org> >> I think it's pretty ugly to have 512 be the value for CONFIG_VIDEO_MB that >> means 512K. I know you didn't do that, but... Maybe we could just say the >> 512K of RAM doesn't make that much difference and make the minimum 1MB? > > Yeah, the 512KB thing is not really nice, but there's not much we can do > about it. We could turn VIDEO_MB into VIDEO_KB instead to have the same > unit everywhere, not sure if that's nicer or uglier. > > I don't want to leave out valid options supported by the hardware > though, so the 512KB setting should stay. I think it's not _that_ big > of a problem as the uglyness is only visible to the developers of > two chipsets I think (i810 and i830), but not to the other developers > and especially not to the user (who sees nice menuconfig entries). > We can leave it as is IMHO. > Yes it is ugly but decimals would be uglier (0.5MB) especially in calculations. Anyways if it is supported by the chip it should be there.
silly question: why, in a 45-line high window, does it only let me see 6 mainboards at a time in menuconfig? Would be oh-so-cool when demo'ing this to have the huge number of vendors show up ... ron
> >> I think it's pretty ugly to have 512 be the value for CONFIG_VIDEO_MB > that > >> means 512K. I know you didn't do that, but... Maybe we could just say > the > >> 512K of RAM doesn't make that much difference and make the minimum 1MB? > > > > Yeah, the 512KB thing is not really nice, but there's not much we can do > > about it. We could turn VIDEO_MB into VIDEO_KB instead to have the same > > unit everywhere, not sure if that's nicer or uglier. > > > > I don't want to leave out valid options supported by the hardware > > though, so the 512KB setting should stay. I think it's not _that_ big > > of a problem as the uglyness is only visible to the developers of > > two chipsets I think (i810 and i830), but not to the other developers > > and especially not to the user (who sees nice menuconfig entries). > > We can leave it as is IMHO. > > > Yes it is ugly but decimals would be uglier (0.5MB) especially in > calculations. Anyways if it is supported by the chip it should be there. I don't think "supported by the chipset" is a strong enough argument. There are lots of horrible configurations supported by chips that aren't supported by coreboot. If there's a good argument why someone might want 512K of video memory instead of 1MB, then that's a reason to keep it. It is hard for me to believe that saving 512K of RAM matters to anyone. Thanks, Myles
On 10/26/2009 11:18 PM, Myles Watson wrote: >>>> I think it's pretty ugly to have 512 be the value for CONFIG_VIDEO_MB >> that >>>> means 512K. I know you didn't do that, but... Maybe we could just say >> the >>>> 512K of RAM doesn't make that much difference and make the minimum 1MB? >>> >>> Yeah, the 512KB thing is not really nice, but there's not much we can do >>> about it. We could turn VIDEO_MB into VIDEO_KB instead to have the same >>> unit everywhere, not sure if that's nicer or uglier. >>> >>> I don't want to leave out valid options supported by the hardware >>> though, so the 512KB setting should stay. I think it's not _that_ big >>> of a problem as the uglyness is only visible to the developers of >>> two chipsets I think (i810 and i830), but not to the other developers >>> and especially not to the user (who sees nice menuconfig entries). >>> We can leave it as is IMHO. >>> >> Yes it is ugly but decimals would be uglier (0.5MB) especially in >> calculations. Anyways if it is supported by the chip it should be there. > I don't think "supported by the chipset" is a strong enough argument. There > are lots of horrible configurations supported by chips that aren't supported > by coreboot. If there's a good argument why someone might want 512K of > video memory instead of 1MB, then that's a reason to keep it. > > It is hard for me to believe that saving 512K of RAM matters to anyone. > I does when you don't care when you just use a shell (lamp server etc). Also if your board only has 128MB on board memory, you may want to conserve as much as you can.....
On 10/26/2009 11:22 PM, Joseph Smith wrote: > On 10/26/2009 11:18 PM, Myles Watson wrote: >>>>> I think it's pretty ugly to have 512 be the value for CONFIG_VIDEO_MB >>> that >>>>> means 512K. I know you didn't do that, but... Maybe we could just say >>> the >>>>> 512K of RAM doesn't make that much difference and make the minimum >>>>> 1MB? >>>> >>>> Yeah, the 512KB thing is not really nice, but there's not much we >>>> can do >>>> about it. We could turn VIDEO_MB into VIDEO_KB instead to have the same >>>> unit everywhere, not sure if that's nicer or uglier. >>>> >>>> I don't want to leave out valid options supported by the hardware >>>> though, so the 512KB setting should stay. I think it's not _that_ big >>>> of a problem as the uglyness is only visible to the developers of >>>> two chipsets I think (i810 and i830), but not to the other developers >>>> and especially not to the user (who sees nice menuconfig entries). >>>> We can leave it as is IMHO. >>>> >>> Yes it is ugly but decimals would be uglier (0.5MB) especially in >>> calculations. Anyways if it is supported by the chip it should be there. >> I don't think "supported by the chipset" is a strong enough argument. >> There >> are lots of horrible configurations supported by chips that aren't >> supported >> by coreboot. If there's a good argument why someone might want 512K of >> video memory instead of 1MB, then that's a reason to keep it. >> >> It is hard for me to believe that saving 512K of RAM matters to anyone. >> > I does when you don't care when you just use a shell (lamp server etc). > Also if your board only has 128MB on board memory, you may want to > conserve as much as you can..... > > I remember when I used to run a clarkconnect sever on an i810. I turned the vga down to the lowest setting in the bios, because clarkconnect did not have a GUI.....
On Mon, 26 Oct 2009 20:12:33 -0700, ron minnich <rminnich@gmail.com> wrote: > silly question: > > why, in a 45-line high window, does it only let me see 6 mainboards at > a time in menuconfig? > not sure? > > Would be oh-so-cool when demo'ing this to have the huge number of > vendors show up ... > Yes very cool :-) It would show these vendors what they are missing out on :-)
Patch
Add kconfig menus for most chipset VIDEO_MB values. VIDEO_MB is a variable that defines how many MB of RAM will be used for onboard graphics frame buffer. It's northbridge-dependent which values for CONFIG_MB are valid (but not board-dependent). This patch adds choices for menuconfig to select the VIDEO_MB value for: - Intel 82810 - Intel 82830 - VIA CN400 - VIA CN700 Note: CN400 and CN700 are based on the CX700 datasheet, not sure if they're correct. If somebody has CN400 and CN700 datasheets, please verify. We drop all per-board VIDEO_MB variables in per-board Kconfig files as there's a northbridge-specific option/default now (plus the user can override the value if needed in menuconfig). As CONFIG_MB is chipset-specific but not board-specific (and never was), filter it in util/compareboard/compareboard, we don't need to match those values. Finally, put "CPU", "Northbridge", "Southbridge", "Super I/O", and "Miscellaneous" sections into the "Devices" menu, where NB-specific options will appear if you select a board using a certain NB, SB-specific options would appear in the "Southbridge" section etc. Signed-off-by: Uwe Hermann <uwe@hermann-uwe.de> Index: src/Kconfig =================================================================== --- src/Kconfig (Revision 4863) +++ src/Kconfig (Arbeitskopie) @@ -52,12 +52,22 @@ source src/mainboard/Kconfig source src/arch/i386/Kconfig source src/arch/ppc/Kconfig + +menu "Devices" + +comment "CPU" +source src/cpu/Kconfig +comment "Northbridge" source src/northbridge/Kconfig -source src/devices/Kconfig +comment "Southbridge" source src/southbridge/Kconfig +comment "Super I/O" source src/superio/Kconfig -source src/cpu/Kconfig +comment "Miscellaneous" +source src/devices/Kconfig +endmenu + config PCI_BUS_SEGN_BITS int default 0 @@ -211,9 +221,10 @@ bool default n +# TODO: Can probably be removed once all chipsets have kconfig options for it. config VIDEO_MB + int default 0 - int config USE_WATCHDOG_ON_BOOT bool Index: src/devices/Kconfig =================================================================== --- src/devices/Kconfig (Revision 4863) +++ src/devices/Kconfig (Arbeitskopie) @@ -18,8 +18,6 @@ ## Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA ## -menu "Devices" - config VGA_BRIDGE_SETUP bool "Setup bridges on path to VGA adapter" default y @@ -82,8 +80,6 @@ help See debug.h for values 0 is no debug output, 0x31ff is _verbose_. -endmenu - config CONSOLE_VGA_MULTI bool default n Index: src/mainboard/bcom/winnetp680/Kconfig =================================================================== --- src/mainboard/bcom/winnetp680/Kconfig (Revision 4863) +++ src/mainboard/bcom/winnetp680/Kconfig (Arbeitskopie) @@ -25,11 +25,6 @@ default 10 depends on BOARD_BCOM_WINNETP680 -config VIDEO_MB - int - default 32 - depends on BOARD_BCOM_WINNETP680 - config RAMBASE hex default 0x4000 Index: src/mainboard/thomson/ip1000/Kconfig =================================================================== --- src/mainboard/thomson/ip1000/Kconfig (Revision 4863) +++ src/mainboard/thomson/ip1000/Kconfig (Arbeitskopie) @@ -29,8 +29,3 @@ default 7 depends on BOARD_THOMSON_IP1000 -config VIDEO_MB - int - default 0 - depends on BOARD_THOMSON_IP1000 - Index: src/mainboard/kontron/kt690/Kconfig =================================================================== --- src/mainboard/kontron/kt690/Kconfig (Revision 4863) +++ src/mainboard/kontron/kt690/Kconfig (Arbeitskopie) @@ -44,11 +44,6 @@ default 0x0 depends on BOARD_KONTRON_KT690 -config VIDEO_MB - int - default 1 - depends on BOARD_KONTRON_KT690 - config LB_CKS_RANGE_END int default 122 Index: src/mainboard/mitac/6513wu/Kconfig =================================================================== --- src/mainboard/mitac/6513wu/Kconfig (Revision 4863) +++ src/mainboard/mitac/6513wu/Kconfig (Arbeitskopie) @@ -50,8 +50,3 @@ default 8 depends on BOARD_MITAC_6513WU -config VIDEO_MB - int - default 1 - depends on BOARD_MITAC_6513WU - Index: src/mainboard/nec/powermate2000/Kconfig =================================================================== --- src/mainboard/nec/powermate2000/Kconfig (Revision 4863) +++ src/mainboard/nec/powermate2000/Kconfig (Arbeitskopie) @@ -50,8 +50,3 @@ default 5 depends on BOARD_NEC_POWERMATE_2000 -config VIDEO_MB - int - default 1 - depends on BOARD_NEC_POWERMATE_2000 - Index: src/mainboard/rca/rm4100/Kconfig =================================================================== --- src/mainboard/rca/rm4100/Kconfig (Revision 4863) +++ src/mainboard/rca/rm4100/Kconfig (Arbeitskopie) @@ -30,8 +30,3 @@ default 7 depends on BOARD_RCA_RM4100 -config VIDEO_MB - int - default 0 - depends on BOARD_RCA_RM4100 - Index: src/mainboard/hp/e_vectra_p2706t/Kconfig =================================================================== --- src/mainboard/hp/e_vectra_p2706t/Kconfig (Revision 4863) +++ src/mainboard/hp/e_vectra_p2706t/Kconfig (Arbeitskopie) @@ -53,8 +53,3 @@ default 3 depends on BOARD_HP_E_VECTRA_P2706T -config VIDEO_MB - int - default 1 - depends on BOARD_HP_E_VECTRA_P2706T - Index: src/mainboard/via/epia-m700/Kconfig =================================================================== --- src/mainboard/via/epia-m700/Kconfig (Revision 4863) +++ src/mainboard/via/epia-m700/Kconfig (Arbeitskopie) @@ -27,11 +27,6 @@ default "EPIA-M700" depends on BOARD_VIA_EPIA_M700 -config VIDEO_MB - int - default 64 - depends on BOARD_VIA_EPIA_M700 - config RAMBASE hex default 0x4000 Index: src/mainboard/via/epia-n/Kconfig =================================================================== --- src/mainboard/via/epia-n/Kconfig (Revision 4863) +++ src/mainboard/via/epia-n/Kconfig (Arbeitskopie) @@ -31,11 +31,6 @@ default "EPIA-N" depends on BOARD_VIA_EPIA_N -config VIDEO_MB - int - default 32 - depends on BOARD_VIA_EPIA_N - config RAMBASE hex default 0x4000 Index: src/mainboard/via/epia-cn/Kconfig =================================================================== --- src/mainboard/via/epia-cn/Kconfig (Revision 4863) +++ src/mainboard/via/epia-cn/Kconfig (Arbeitskopie) @@ -28,11 +28,6 @@ default "EPIA-CN" depends on BOARD_VIA_EPIA_CN -config VIDEO_MB - int - default 32 - depends on BOARD_VIA_EPIA_CN - config RAMBASE hex default 0x4000 Index: src/mainboard/via/pc2500e/Kconfig =================================================================== --- src/mainboard/via/pc2500e/Kconfig (Revision 4863) +++ src/mainboard/via/pc2500e/Kconfig (Arbeitskopie) @@ -22,11 +22,6 @@ default "pc2500e" depends on BOARD_VIA_PC2500E -config VIDEO_MB - int - default 32 - depends on BOARD_VIA_PC2500E - config RAMBASE hex default 0x4000 Index: src/mainboard/msi/ms6178/Kconfig =================================================================== --- src/mainboard/msi/ms6178/Kconfig (Revision 4863) +++ src/mainboard/msi/ms6178/Kconfig (Arbeitskopie) @@ -49,8 +49,3 @@ default 4 depends on BOARD_MSI_MS_6178 -config VIDEO_MB - int - default 1 - depends on BOARD_MSI_MS_6178 - Index: src/mainboard/jetway/j7f24/Kconfig =================================================================== --- src/mainboard/jetway/j7f24/Kconfig (Revision 4863) +++ src/mainboard/jetway/j7f24/Kconfig (Arbeitskopie) @@ -29,11 +29,6 @@ default "J7f24" depends on BOARD_JETWAY_J7F24 -config VIDEO_MB - int - default 32 - depends on BOARD_JETWAY_J7F24 - config RAMBASE hex default 0x4000 Index: src/mainboard/technexion/tim5690/Kconfig =================================================================== --- src/mainboard/technexion/tim5690/Kconfig (Revision 4863) +++ src/mainboard/technexion/tim5690/Kconfig (Arbeitskopie) @@ -129,11 +129,6 @@ default 0x4000 depends on BOARD_TECHNEXION_TIM5690 -config VIDEO_MB - int - default 1 - depends on BOARD_TECHNEXION_TIM5690 - config HAVE_OPTION_TABLE bool default n Index: src/mainboard/asus/mew-am/Kconfig =================================================================== --- src/mainboard/asus/mew-am/Kconfig (Revision 4863) +++ src/mainboard/asus/mew-am/Kconfig (Arbeitskopie) @@ -50,8 +50,3 @@ default 8 depends on BOARD_ASUS_MEW_AM -config VIDEO_MB - int - default 1 - depends on BOARD_ASUS_MEW_AM - Index: src/mainboard/asus/mew-vm/Kconfig =================================================================== --- src/mainboard/asus/mew-vm/Kconfig (Revision 4863) +++ src/mainboard/asus/mew-vm/Kconfig (Arbeitskopie) @@ -45,8 +45,3 @@ default 11 depends on BOARD_ASUS_MEW_VM -config VIDEO_MB - int - default 1 - depends on BOARD_ASUS_MEW_VM - Index: src/northbridge/via/cn400/Kconfig =================================================================== --- src/northbridge/via/cn400/Kconfig (Revision 4863) +++ src/northbridge/via/cn400/Kconfig (Arbeitskopie) @@ -17,3 +17,36 @@ default n depends on NORTHBRIDGE_VIA_CN400 +# TODO: Values are from the CX700 datasheet, not sure if this matches CN400. +# TODO: What should be the per-chipset default value here? +choice + prompt "Onboard graphics" + default CN400_VIDEO_MB_32MB + depends on NORTHBRIDGE_VIA_CN400 + +# TODO: Disabling onboard graphics is not yet supported in the source code. +config CN400_VIDEO_MB_OFF + bool "Disabled, 0KB" +config CN400_VIDEO_MB_8MB + bool "Enabled, 8MB" +config CN400_VIDEO_MB_16MB + bool "Enabled, 16MB" +config CN400_VIDEO_MB_32MB + bool "Enabled, 32MB" +config CN400_VIDEO_MB_64MB + bool "Enabled, 64MB" +config CN400_VIDEO_MB_128MB + bool "Enabled, 128MB" + +endchoice + +config VIDEO_MB + int + default 0 if CN400_VIDEO_MB_OFF + default 8 if CN400_VIDEO_MB_8MB + default 16 if CN400_VIDEO_MB_16MB + default 32 if CN400_VIDEO_MB_32MB + default 64 if CN400_VIDEO_MB_64MB + default 128 if CN400_VIDEO_MB_128MB + depends on NORTHBRIDGE_VIA_CN400 + Index: src/northbridge/via/cn700/Kconfig =================================================================== --- src/northbridge/via/cn700/Kconfig (Revision 4863) +++ src/northbridge/via/cn700/Kconfig (Arbeitskopie) @@ -17,3 +17,37 @@ bool default n depends on NORTHBRIDGE_VIA_CN700 + +# TODO: Values are from the CX700 datasheet, not sure if this matches CN700. +# TODO: What should be the per-chipset default value here? +choice + prompt "Onboard graphics" + default CN700_VIDEO_MB_32MB + depends on NORTHBRIDGE_VIA_CN700 + +# TODO: Disabling onboard graphics is not yet supported in the code. +config CN700_VIDEO_MB_OFF + bool "Disabled, 0KB" +config CN700_VIDEO_MB_8MB + bool "Enabled, 8MB" +config CN700_VIDEO_MB_16MB + bool "Enabled, 16MB" +config CN700_VIDEO_MB_32MB + bool "Enabled, 32MB" +config CN700_VIDEO_MB_64MB + bool "Enabled, 64MB" +config CN700_VIDEO_MB_128MB + bool "Enabled, 128MB" + +endchoice + +config VIDEO_MB + int + default 0 if CN700_VIDEO_MB_OFF + default 8 if CN700_VIDEO_MB_8MB + default 16 if CN700_VIDEO_MB_16MB + default 32 if CN700_VIDEO_MB_32MB + default 64 if CN700_VIDEO_MB_64MB + default 128 if CN700_VIDEO_MB_128MB + depends on NORTHBRIDGE_VIA_CN700 + Index: src/northbridge/intel/i82810/Kconfig =================================================================== --- src/northbridge/intel/i82810/Kconfig (Revision 4863) +++ src/northbridge/intel/i82810/Kconfig (Arbeitskopie) @@ -22,3 +22,24 @@ bool select HAVE_HIGH_TABLES +choice + prompt "Onboard graphics" + default I810_VIDEO_MB_1MB + depends on NORTHBRIDGE_INTEL_I82810 + +config I810_VIDEO_MB_OFF + bool "Disabled, 0KB" +config I810_VIDEO_MB_512KB + bool "Enabled, 512KB" +config I810_VIDEO_MB_1MB + bool "Enabled, 1MB" + +endchoice + +config VIDEO_MB + int + default 0 if I810_VIDEO_MB_OFF + default 512 if I810_VIDEO_MB_512KB + default 1 if I810_VIDEO_MB_1MB + depends on NORTHBRIDGE_INTEL_I82810 + Index: src/northbridge/intel/i82830/Kconfig =================================================================== --- src/northbridge/intel/i82830/Kconfig (Revision 4863) +++ src/northbridge/intel/i82830/Kconfig (Arbeitskopie) @@ -2,3 +2,27 @@ bool select HAVE_HIGH_TABLES +choice + prompt "Onboard graphics" + default I830_VIDEO_MB_8MB + depends on NORTHBRIDGE_INTEL_I82830 + +config I830_VIDEO_MB_OFF + bool "Disabled, 0KB" +config I830_VIDEO_MB_512KB + bool "Enabled, 512KB" +config I830_VIDEO_MB_1MB + bool "Enabled, 1MB" +config I830_VIDEO_MB_8MB + bool "Enabled, 8MB" + +endchoice + +config VIDEO_MB + int + default 0 if I830_VIDEO_MB_OFF + default 512 if I830_VIDEO_MB_512KB + default 1 if I830_VIDEO_MB_1MB + default 8 if I830_VIDEO_MB_8MB + depends on NORTHBRIDGE_INTEL_I82830 + Index: util/compareboard/compareboard =================================================================== --- util/compareboard/compareboard (Revision 4863) +++ util/compareboard/compareboard (Arbeitskopie) @@ -44,6 +44,7 @@ -e "/^CONFIG_ROM_IMAGE_SIZE / d" \ -e "/^CONFIG_STACK_SIZE / d" \ -e "/^CONFIG_GDB_STUB / d" \ + -e "/^CONFIG_VIDEO_MB / d" \ $A/old > $A/old.filtered sed \ -e "/^CONFIG_VENDOR_/ d" \ @@ -67,6 +68,7 @@ -e "/^CONFIG_ROM_IMAGE_SIZE / d" \ -e "/^CONFIG_STACK_SIZE / d" \ -e "/^CONFIG_GDB_STUB / d" \ + -e "/^CONFIG_VIDEO_MB / d" \ $A/new > $A/new.filtered normalize $A/old.filtered > $A/old.normalized