Re: [SeaBIOS] [PATCH 2/2] vgabios: Limit the range of the VBE number of pages parameter.

2013-09-14 Thread Paul Menzel
Am Freitag, den 13.09.2013, 16:24 -0400 schrieb Kevin O'Connor:
 Looking at the output of other VGA BIOS implementations, it appears
 that the number of available video pages reported is always between 1
 and 127.

That is including 1 and 127, right?

 Signed-off-by: Kevin O'Connor ke...@koconnor.net
 ---
  vgasrc/vbe.c | 4 
  1 file changed, 4 insertions(+)
 
 diff --git a/vgasrc/vbe.c b/vgasrc/vbe.c
 index d962333..2c08736 100644
 --- a/vgasrc/vbe.c
 +++ b/vgasrc/vbe.c
 @@ -144,6 +144,10 @@ vbe_104f01(struct bregs *regs)
  mode_attr |= VBE_MODE_ATTRIBUTE_LINEAR_FRAME_BUFFER_MODE;
  break;
  }
 +if (pages  128)
 +pages = 128;

As 1 gets subtracted (below in diff), in this case 127 is included.

 +if (pages  2)
 +pages++;

Can pages be 0? If yes, it would be 0 again after substracting 1. Should
it be `pages = 1` instead of `pages++`?

  SET_FARVAR(seg, info-mode_attributes, mode_attr);
  SET_FARVAR(seg, info-planes, planes);
  SET_FARVAR(seg, info-pages, pages - 1);


Thanks,

Paul


signature.asc
Description: This is a digitally signed message part
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 2/2] vgabios: Limit the range of the VBE number of pages parameter.

2013-09-14 Thread Peter Stuge
Paul Menzel wrote:
  +if (pages  2)
  +pages++;
 
 Can pages be 0? If yes, it would be 0 again after substracting 1. Should
 it be `pages = 1` instead of `pages++`?

pages=2;


//Peter

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


Re: [SeaBIOS] [PATCH 2/2] vgabios: Limit the range of the VBE number of pages parameter.

2013-09-14 Thread Kevin O'Connor
On Sat, Sep 14, 2013 at 09:11:02AM +0200, Paul Menzel wrote:
 Am Freitag, den 13.09.2013, 16:24 -0400 schrieb Kevin O'Connor:
  Looking at the output of other VGA BIOS implementations, it appears
  that the number of available video pages reported is always between 1
  and 127.
 
 That is including 1 and 127, right?
 
  Signed-off-by: Kevin O'Connor ke...@koconnor.net
  ---
   vgasrc/vbe.c | 4 
   1 file changed, 4 insertions(+)
  
  diff --git a/vgasrc/vbe.c b/vgasrc/vbe.c
  index d962333..2c08736 100644
  --- a/vgasrc/vbe.c
  +++ b/vgasrc/vbe.c
  @@ -144,6 +144,10 @@ vbe_104f01(struct bregs *regs)
   mode_attr |= VBE_MODE_ATTRIBUTE_LINEAR_FRAME_BUFFER_MODE;
   break;
   }
  +if (pages  128)
  +pages = 128;
 
 As 1 gets subtracted (below in diff), in this case 127 is included.
 
  +if (pages  2)
  +pages++;
 
 Can pages be 0? If yes, it would be 0 again after substracting 1. Should
 it be `pages = 1` instead of `pages++`?

The goal was to report 0 if there isn't enough memory for even a
single image.  Ideally that would not happen in practice as it
shouldn't even be presented as a valid video mode if there isn't
enough memory for it.

Otherwise, the goal is to report between 1-127 (inclusive) for the
number of pages.  So, if there is only enough ram for 1 image, then
the code would report 1 (which means 2 images).  If there is enough
ram for more than 128 images, then the code would report 127 (which
means 128).  This is what the AMD VGABIOS appears to be doing.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios