Patchwork [1/2] Allow 1366x768 as a valid VGA resolution

login
register
mail settings
Submitter John Baboval
Date Oct. 28, 2011, 7:24 p.m.
Message ID <4EAB0169.8090408@virtualcomputer.com>
Download mbox | patch
Permalink /patch/122486/
State New
Headers show

Comments

John Baboval - Oct. 28, 2011, 7:24 p.m.
760p TV panels have a 1366x768 resolution, and have been popular
recently as low-cost monitors. The 1366 resolution doesn't pass
the (xres & 7) == 0 test.

Signed-off-by: John V. Baboval <john.baboval@virtualcomputer.com>
---
  hw/vga.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

              break;
Gerd Hoffmann - Nov. 1, 2011, 8:58 a.m.
On 10/28/11 21:24, John Baboval wrote:
> 760p TV panels have a 1366x768 resolution, and have been popular
> recently as low-cost monitors. The 1366 resolution doesn't pass
> the (xres & 7) == 0 test.

Why is it save to simply remove the test?
Guess there is a reason why it is there in the first place?

cheers,
  Gerd
John Baboval - Nov. 1, 2011, 1:39 p.m.
I don't know of any reason for it. 

-John


On Nov 1, 2011, at 4:58 AM, "Gerd Hoffmann" <kraxel@redhat.com> wrote:

> On 10/28/11 21:24, John Baboval wrote:
>> 760p TV panels have a 1366x768 resolution, and have been popular
>> recently as low-cost monitors. The 1366 resolution doesn't pass
>> the (xres & 7) == 0 test.
> 
> Why is it save to simply remove the test?
> Guess there is a reason why it is there in the first place?
> 
> cheers,
>  Gerd
>
Gerd Hoffmann - Nov. 1, 2011, 4:57 p.m.
On 11/01/11 14:39, John Baboval wrote:
> I don't know of any reason for it. 

I'd guess it is alignment, probably not important for all color depts.

Maybe it is a good idea to do all sanity checks in the
VBE_DISPI_INDEX_ENABLE branch where the actual mode switch happens. Then
you already know xres, yres and depth when applying the checks.  You can
calculate the scanline length, then check the scanline alignment instead
of being overly strict on xres in high color modes to satisfy alignment
requirements in low color modes.

You can also simply calculate how much memory the video mode needs and
check that against the configured video ram instead of pulling xres and
yres limits out of thin air.

cheers,
  Gerd
John Baboval - Nov. 3, 2011, 3:03 p.m.
This is a good idea. I'm going to re-work the patch, but I have a lot of 
other stuff going on too, so it may be a week or so before I get back to it.

On 11/01/2011 12:57 PM, Gerd Hoffmann wrote:
> On 11/01/11 14:39, John Baboval wrote:
>> I don't know of any reason for it.
> I'd guess it is alignment, probably not important for all color depts.
>
> Maybe it is a good idea to do all sanity checks in the
> VBE_DISPI_INDEX_ENABLE branch where the actual mode switch happens. Then
> you already know xres, yres and depth when applying the checks.  You can
> calculate the scanline length, then check the scanline alignment instead
> of being overly strict on xres in high color modes to satisfy alignment
> requirements in low color modes.
>
> You can also simply calculate how much memory the video mode needs and
> check that against the configured video ram instead of pulling xres and
> yres limits out of thin air.
>
> cheers,
>    Gerd

Patch

diff --git a/hw/vga.c b/hw/vga.c
index 8003eda..f12a371 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -680,7 +680,7 @@  static void vbe_ioport_write_data(void *opaque, 
uint32_t addr, uint32_t val)
              }
              break;
          case VBE_DISPI_INDEX_XRES:
-            if ((val <= vs.max_xres) && ((val & 7) == 0)) {
+            if (val <= vs.max_xres) {
                  s->vbe_regs[s->vbe_index] = val;
              }