Hello,
this is my attempt to address the review comments I got for v2:
- Gerd pointed out that cbvesa should only use the modes where memmodel == MM_DIRECT.
- Kevin noted that reading global variables need the GET_GLOBAL wrapper. (I would have expected that the compiler does the right thing here. Probably I'm still to wet behind the ears here :-)
Note this is only compile tested.
Uwe Kleine-König (2): cbvga: reuse svga modes definitions from svgamodes.c Add additional resolutions for 16:9 displays: 1600x900 and 2560x1440
vgasrc/cbvga.c | 80 ++++++---------------------------------------- vgasrc/svgamodes.c | 8 +++++ 2 files changed, 17 insertions(+), 71 deletions(-)
For cbvga only modes with MM_DIRECT are usable, so skip the other ones. This effectively adds the following modes:
{ 0x10D, { MM_DIRECT, 320, 200, 15, 8, 16, SEG_GRAPH } }, { 0x10E, { MM_DIRECT, 320, 200, 16, 8, 16, SEG_GRAPH } }, { 0x10F, { MM_DIRECT, 320, 200, 24, 8, 16, SEG_GRAPH } }, { 0x140, { MM_DIRECT, 320, 200, 32, 8, 16, SEG_GRAPH } }, --- vgasrc/cbvga.c | 80 ++++++-------------------------------------------- 1 file changed, 9 insertions(+), 71 deletions(-)
diff --git a/vgasrc/cbvga.c b/vgasrc/cbvga.c index 438d8fda6c6e..b919137bb183 100644 --- a/vgasrc/cbvga.c +++ b/vgasrc/cbvga.c @@ -13,76 +13,13 @@ #include "vgabios.h" // SET_VGA #include "vgafb.h" // handle_gfx_op #include "vgautil.h" // VBE_total_memory +#include "svgamodes.h" // svga_modes
static int CBmode VAR16; static struct vgamode_s CBmodeinfo VAR16; static struct vgamode_s CBemulinfo VAR16; static u32 CBlinelength VAR16;
-static struct cbvga_mode_s -{ - u16 mode; - struct vgamode_s info; -} cbvesa_modes[] VAR16 = { - /* VESA 1.0 modes */ - { 0x110, { MM_DIRECT, 640, 480, 15, 8, 16, SEG_GRAPH } }, - { 0x111, { MM_DIRECT, 640, 480, 16, 8, 16, SEG_GRAPH } }, - { 0x112, { MM_DIRECT, 640, 480, 24, 8, 16, SEG_GRAPH } }, - { 0x113, { MM_DIRECT, 800, 600, 15, 8, 16, SEG_GRAPH } }, - { 0x114, { MM_DIRECT, 800, 600, 16, 8, 16, SEG_GRAPH } }, - { 0x115, { MM_DIRECT, 800, 600, 24, 8, 16, SEG_GRAPH } }, - { 0x116, { MM_DIRECT, 1024, 768, 15, 8, 16, SEG_GRAPH } }, - { 0x117, { MM_DIRECT, 1024, 768, 16, 8, 16, SEG_GRAPH } }, - { 0x118, { MM_DIRECT, 1024, 768, 24, 8, 16, SEG_GRAPH } }, - { 0x119, { MM_DIRECT, 1280, 1024, 15, 8, 16, SEG_GRAPH } }, - { 0x11A, { MM_DIRECT, 1280, 1024, 16, 8, 16, SEG_GRAPH } }, - { 0x11B, { MM_DIRECT, 1280, 1024, 24, 8, 16, SEG_GRAPH } }, - { 0x11D, { MM_DIRECT, 1600, 1200, 15, 8, 16, SEG_GRAPH } }, - { 0x11E, { MM_DIRECT, 1600, 1200, 16, 8, 16, SEG_GRAPH } }, - { 0x11F, { MM_DIRECT, 1600, 1200, 24, 8, 16, SEG_GRAPH } }, - /* VESA 2.0 modes */ - { 0x141, { MM_DIRECT, 640, 400, 32, 8, 16, SEG_GRAPH } }, - { 0x142, { MM_DIRECT, 640, 480, 32, 8, 16, SEG_GRAPH } }, - { 0x143, { MM_DIRECT, 800, 600, 32, 8, 16, SEG_GRAPH } }, - { 0x144, { MM_DIRECT, 1024, 768, 32, 8, 16, SEG_GRAPH } }, - { 0x145, { MM_DIRECT, 1280, 1024, 32, 8, 16, SEG_GRAPH } }, - { 0x147, { MM_DIRECT, 1600, 1200, 32, 8, 16, SEG_GRAPH } }, - { 0x149, { MM_DIRECT, 1152, 864, 15, 8, 16, SEG_GRAPH } }, - { 0x14a, { MM_DIRECT, 1152, 864, 16, 8, 16, SEG_GRAPH } }, - { 0x14b, { MM_DIRECT, 1152, 864, 24, 8, 16, SEG_GRAPH } }, - { 0x14c, { MM_DIRECT, 1152, 864, 32, 8, 16, SEG_GRAPH } }, - { 0x175, { MM_DIRECT, 1280, 768, 16, 8, 16, SEG_GRAPH } }, - { 0x176, { MM_DIRECT, 1280, 768, 24, 8, 16, SEG_GRAPH } }, - { 0x177, { MM_DIRECT, 1280, 768, 32, 8, 16, SEG_GRAPH } }, - { 0x178, { MM_DIRECT, 1280, 800, 16, 8, 16, SEG_GRAPH } }, - { 0x179, { MM_DIRECT, 1280, 800, 24, 8, 16, SEG_GRAPH } }, - { 0x17a, { MM_DIRECT, 1280, 800, 32, 8, 16, SEG_GRAPH } }, - { 0x17b, { MM_DIRECT, 1280, 960, 16, 8, 16, SEG_GRAPH } }, - { 0x17c, { MM_DIRECT, 1280, 960, 24, 8, 16, SEG_GRAPH } }, - { 0x17d, { MM_DIRECT, 1280, 960, 32, 8, 16, SEG_GRAPH } }, - { 0x17e, { MM_DIRECT, 1440, 900, 16, 8, 16, SEG_GRAPH } }, - { 0x17f, { MM_DIRECT, 1440, 900, 24, 8, 16, SEG_GRAPH } }, - { 0x180, { MM_DIRECT, 1440, 900, 32, 8, 16, SEG_GRAPH } }, - { 0x181, { MM_DIRECT, 1400, 1050, 16, 8, 16, SEG_GRAPH } }, - { 0x182, { MM_DIRECT, 1400, 1050, 24, 8, 16, SEG_GRAPH } }, - { 0x183, { MM_DIRECT, 1400, 1050, 32, 8, 16, SEG_GRAPH } }, - { 0x184, { MM_DIRECT, 1680, 1050, 16, 8, 16, SEG_GRAPH } }, - { 0x185, { MM_DIRECT, 1680, 1050, 24, 8, 16, SEG_GRAPH } }, - { 0x186, { MM_DIRECT, 1680, 1050, 32, 8, 16, SEG_GRAPH } }, - { 0x187, { MM_DIRECT, 1920, 1200, 16, 8, 16, SEG_GRAPH } }, - { 0x188, { MM_DIRECT, 1920, 1200, 24, 8, 16, SEG_GRAPH } }, - { 0x189, { MM_DIRECT, 1920, 1200, 32, 8, 16, SEG_GRAPH } }, - { 0x18a, { MM_DIRECT, 2560, 1600, 16, 8, 16, SEG_GRAPH } }, - { 0x18b, { MM_DIRECT, 2560, 1600, 24, 8, 16, SEG_GRAPH } }, - { 0x18c, { MM_DIRECT, 2560, 1600, 32, 8, 16, SEG_GRAPH } }, - { 0x18d, { MM_DIRECT, 1280, 720, 16, 8, 16, SEG_GRAPH } }, - { 0x18e, { MM_DIRECT, 1280, 720, 24, 8, 16, SEG_GRAPH } }, - { 0x18f, { MM_DIRECT, 1280, 720, 32, 8, 16, SEG_GRAPH } }, - { 0x190, { MM_DIRECT, 1920, 1080, 16, 8, 16, SEG_GRAPH } }, - { 0x191, { MM_DIRECT, 1920, 1080, 24, 8, 16, SEG_GRAPH } }, - { 0x192, { MM_DIRECT, 1920, 1080, 32, 8, 16, SEG_GRAPH } }, -}; - struct vgamode_s *cbvga_find_mode(int mode) { if (mode == GET_GLOBAL(CBmode)) @@ -91,8 +28,8 @@ struct vgamode_s *cbvga_find_mode(int mode) return &CBemulinfo;
int i; - for (i = 0; i < ARRAY_SIZE(cbvesa_modes); i++) { - struct cbvga_mode_s *cbmode_g = &cbvesa_modes[i]; + for (i = 0; i < GET_GLOBAL(svga_mcount); i++) { + struct generic_svga_mode *cbmode_g = &svga_modes[i]; if (GET_GLOBAL(cbmode_g->mode) == 0xffff) continue; if (GET_GLOBAL(cbmode_g->mode) == mode) @@ -114,8 +51,8 @@ cbvga_list_modes(u16 seg, u16 *dest, u16 *last) * + 24 Bpp and 32 Bpp are supported */ int i; - for (i = 0; i < ARRAY_SIZE(cbvesa_modes) && dest < last; i++) { - struct cbvga_mode_s *cbmode_g = &cbvesa_modes[i]; + for (i = 0; i < GET_GLOBAL(svga_mcount) && dest < last; i++) { + struct generic_svga_mode *cbmode_g = &svga_modes[i]; u16 mode = GET_GLOBAL(cbmode_g->mode); if (mode == 0xffff) continue; @@ -265,10 +202,11 @@ cbvga_setup_modes(u64 addr, u8 bpp, u32 xlines, u32 ylines, u32 linelength) , get_global_seg(), &CBmodeinfo, sizeof(CBemulinfo));
// Validate modes - for (i = 0; i < ARRAY_SIZE(cbvesa_modes); i++) { - struct cbvga_mode_s *cbmode_g = &cbvesa_modes[i]; + for (i = 0; i < GET_GLOBAL(svga_mcount); i++) { + struct generic_svga_mode *cbmode_g = &svga_modes[i]; /* Skip VBE modes that doesn't fit into coreboot's framebuffer */ - if ((GET_GLOBAL(cbmode_g->info.height) > ylines) + if ((GET_GLOBAL(cbmode_g->info.memmodel) != MM_DIRECT) + || (GET_GLOBAL(cbmode_g->info.height) > ylines) || (GET_GLOBAL(cbmode_g->info.width) > xlines) || (GET_GLOBAL(cbmode_g->info.depth) != bpp)) { dprintf(3, "Removing mode %x\n", GET_GLOBAL(cbmode_g->mode));
This allows to have qemu run at the native screen resolution of my (physical) monitor.
This is inspired by a patch created by Andreas Dangel that I found on https://adangel.org/2015/09/11/qemu-kvm-custom-resolutions/ . --- vgasrc/svgamodes.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/vgasrc/svgamodes.c b/vgasrc/svgamodes.c index 013504c3e691..f89ebef950a8 100644 --- a/vgasrc/svgamodes.c +++ b/vgasrc/svgamodes.c @@ -76,5 +76,13 @@ struct generic_svga_mode svga_modes[] VAR16 = { { 0x190, { MM_DIRECT, 1920, 1080, 16, 8, 16, SEG_GRAPH } }, { 0x191, { MM_DIRECT, 1920, 1080, 24, 8, 16, SEG_GRAPH } }, { 0x192, { MM_DIRECT, 1920, 1080, 32, 8, 16, SEG_GRAPH } }, + + /* custom resolutions for 16:9 displays */ + { 0x193, { MM_DIRECT, 1600, 900, 16, 8, 16, SEG_GRAPH } }, + { 0x194, { MM_DIRECT, 1600, 900, 24, 8, 16, SEG_GRAPH } }, + { 0x195, { MM_DIRECT, 1600, 900, 32, 8, 16, SEG_GRAPH } }, + { 0x196, { MM_DIRECT, 2560, 1440, 16, 8, 16, SEG_GRAPH } }, + { 0x197, { MM_DIRECT, 2560, 1440, 24, 8, 16, SEG_GRAPH } }, + { 0x198, { MM_DIRECT, 2560, 1440, 32, 8, 16, SEG_GRAPH } }, }; unsigned int svga_mcount VAR16 = ARRAY_SIZE(svga_modes);
On 7/31/19 5:51 PM, Uwe Kleine-König wrote:
Hello,
this is my attempt to address the review comments I got for v2:
Gerd pointed out that cbvesa should only use the modes where memmodel == MM_DIRECT.
Kevin noted that reading global variables need the GET_GLOBAL wrapper. (I would have expected that the compiler does the right thing here. Probably I'm still to wet behind the ears here :-)
Note this is only compile tested.
Is there anything I have to do here to get these patches merged?
Best regards Uwe
On Thu, Oct 10, 2019 at 05:43:30PM +0200, Uwe Kleine-König wrote:
On 7/31/19 5:51 PM, Uwe Kleine-König wrote:
Hello,
this is my attempt to address the review comments I got for v2:
Gerd pointed out that cbvesa should only use the modes where memmodel == MM_DIRECT.
Kevin noted that reading global variables need the GET_GLOBAL wrapper. (I would have expected that the compiler does the right thing here. Probably I'm still to wet behind the ears here :-)
Note this is only compile tested.
Is there anything I have to do here to get these patches merged?
The patches look fine to me. The commits should have a signed-off-by line though. Gerd - do you have any further comments on this?
-Kevin
Hello Kevin,
On 10/13/19 4:07 AM, Kevin O'Connor wrote:
The commits should have a signed-off-by line though.
Not sure what is meant here. Do you miss an S-o-b by me or by Gerd?
If the former and assuming the intention is that its meaning is similar to that in the kernel: INAL, but I think if you want to have an at least small chance that this formalism has any juristic meaning you need to formalize it. Currently you ship GPLv3 and LGPLv3 in seabios.git but many source files don't have their own copyright notes that declare which should be effective[1]. Also there is no file that documents what Signed-off-by should actually mean.
Just my 0.02€ Uwe
[1] I only checked a few files (vgasrc/ramfb.c, vgasrc/svgamodes.c) by hand and assumed the other files to be in a similar situation. A quick glance using licensecheck from https://metacpan.org/release/App-Licensecheck:
seabios$ git ls-files | xargs licensecheck | sed "s/.*: //" | sort | uniq -c 2008 is > 1013 at /usr/share/perl5/String/Copyright.pm line 194. 1 BSD (2 clause) 10 GPL 4 GPL GENERATED FILE 16 GPL (v2.0) 1 GPL (v2) GENERATED FILE 102 LGPL (v3) 1 MIT/X11 (BSD like) 3 *No copyright* GENERATED FILE 1 *No copyright* GPL (v2.0) 7 *No copyright* GPL (v2 or later) (with incorrect FSF address) 3 *No copyright* LGPL (v3) 101 *No copyright* UNKNOWN 6 UNKNOWN
On Sat, Oct 12, 2019 at 10:07:23PM -0400, Kevin O'Connor wrote:
On Thu, Oct 10, 2019 at 05:43:30PM +0200, Uwe Kleine-König wrote:
On 7/31/19 5:51 PM, Uwe Kleine-König wrote:
Hello,
this is my attempt to address the review comments I got for v2:
Gerd pointed out that cbvesa should only use the modes where memmodel == MM_DIRECT.
Kevin noted that reading global variables need the GET_GLOBAL wrapper. (I would have expected that the compiler does the right thing here. Probably I'm still to wet behind the ears here :-)
Note this is only compile tested.
Is there anything I have to do here to get these patches merged?
The patches look fine to me. The commits should have a signed-off-by line though. Gerd - do you have any further comments on this?
No comments, looks all fine to me.
cheers, Gerd
On 10/16/19 12:27 PM, Gerd Hoffmann wrote:
On Sat, Oct 12, 2019 at 10:07:23PM -0400, Kevin O'Connor wrote:
On Thu, Oct 10, 2019 at 05:43:30PM +0200, Uwe Kleine-König wrote:
On 7/31/19 5:51 PM, Uwe Kleine-König wrote:
Hello,
this is my attempt to address the review comments I got for v2:
Gerd pointed out that cbvesa should only use the modes where memmodel == MM_DIRECT.
Kevin noted that reading global variables need the GET_GLOBAL wrapper. (I would have expected that the compiler does the right thing here. Probably I'm still to wet behind the ears here :-)
Note this is only compile tested.
Is there anything I have to do here to get these patches merged?
The patches look fine to me. The commits should have a signed-off-by line though. Gerd - do you have any further comments on this?
No comments, looks all fine to me.
Would it help if I prepared a pull-request for my patches? How else can I help getting these patches in? I'm still unclear about that Signed-of-by concern.
Best regards Uwe
On Thu, Oct 17, 2019 at 09:44:38AM +0200, Uwe Kleine-König wrote:
On 10/16/19 12:27 PM, Gerd Hoffmann wrote:
On Sat, Oct 12, 2019 at 10:07:23PM -0400, Kevin O'Connor wrote:
On Thu, Oct 10, 2019 at 05:43:30PM +0200, Uwe Kleine-König wrote:
On 7/31/19 5:51 PM, Uwe Kleine-König wrote:
Hello,
this is my attempt to address the review comments I got for v2:
Gerd pointed out that cbvesa should only use the modes where memmodel == MM_DIRECT.
Kevin noted that reading global variables need the GET_GLOBAL wrapper. (I would have expected that the compiler does the right thing here. Probably I'm still to wet behind the ears here :-)
Note this is only compile tested.
Is there anything I have to do here to get these patches merged?
The patches look fine to me. The commits should have a signed-off-by line though. Gerd - do you have any further comments on this?
No comments, looks all fine to me.
Would it help if I prepared a pull-request for my patches? How else can I help getting these patches in? I'm still unclear about that Signed-of-by concern.
Re-send patches with Signed-of-by added to the commit message. Git can do that automatically for you (-s switch for "git commit"). It's used to keep track of the patch workflow (who wrote the patch, who reviewed & committed, ...).
cheers, Gerd
On 10/17/19 10:51 AM, Gerd Hoffmann wrote:
On Thu, Oct 17, 2019 at 09:44:38AM +0200, Uwe Kleine-König wrote:
I'm still unclear about that Signed-of-by concern.
Re-send patches with Signed-of-by added to the commit message. Git can do that automatically for you (-s switch for "git commit"). It's used to keep track of the patch workflow (who wrote the patch, who reviewed & committed, ...).
I would have expected that the "who wrote" is obvious from the Author: line in the commit, "who reviewed" by something like "Reviewed-by:" and the committer from the Committer line.
For the projects I usually interact with Signed-off-by has a juristic meaning, see my mail from Monday.
If the Signed-off-by here really only has the purpose to show through which hands the patch went in, I can resend of course.
Best regards Uwe
'signed-off-by' is a linux kernel convention; by adding your Signed-off-by line to a patch, you are certifying that you have read and understood the Developer Certificate of Origin (DCO), originated in 2004 by the Linux foundation as an affirmation that the source code being submitted originated from the developer, or that the developer has permission to submit the code. Many projects follow the same conventions used by the linux kernel.
https://en.wikipedia.org/wiki/Developer_Certificate_of_Origin
On Thu, Oct 17, 2019 at 4:31 AM Uwe Kleine-König uwe@kleine-koenig.org wrote:
On 10/17/19 10:51 AM, Gerd Hoffmann wrote:
On Thu, Oct 17, 2019 at 09:44:38AM +0200, Uwe Kleine-König wrote:
I'm still unclear about that Signed-of-by concern.
Re-send patches with Signed-of-by added to the commit message. Git can do that automatically for you (-s switch for "git commit"). It's used to keep track of the patch workflow (who wrote the patch, who reviewed & committed, ...).
I would have expected that the "who wrote" is obvious from the Author: line in the commit, "who reviewed" by something like "Reviewed-by:" and the committer from the Committer line.
For the projects I usually interact with Signed-off-by has a juristic meaning, see my mail from Monday.
If the Signed-off-by here really only has the purpose to show through which hands the patch went in, I can resend of course.
Best regards Uwe
SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-leave@seabios.org
-
Gerd Hoffmann -
Kevin O'Connor -
Matt DeVillier -
Uwe Kleine-König