[OpenBIOS] [PATCH] Add user resolutions to QEMU, VGA node's fb-modes property.

[G 3]

On Oct 9, 2016, at 10:02 AM, Mark Cave-Ayland wrote:

> On 07/10/16 21:55, Programmingkid wrote:
>
>> Implement the add-resolutions word. It will copy the user  
>> resolutions in the
>> options node's resolutions property to the QEMU,VGA node's fb- 
>> modes property.
>>
>> Signed-off-by: John Arbuckle <programmingkidx at gmail.com>
>> ---
>>  arch/ppc/qemu/init.c  |  2 +-
>>  arch/ppc/qemu/qemu.fs | 68 +++++++++++++++++++++++++++++++++++++++ 
>> ++++++++++++
>>  2 files changed, 69 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/ppc/qemu/init.c b/arch/ppc/qemu/init.c
>> index 6f21814..df873ce 100644
>> --- a/arch/ppc/qemu/init.c
>> +++ b/arch/ppc/qemu/init.c
>> @@ -631,7 +631,7 @@ go(void)
>>      if (find_dev("/rom/macos")) {
>>          fword("insert-copyright-property");
>>      }
>> -
>> +    feval("add-resolutions");
>>      feval("saved-program-state >sps.entry @");
>>      addr = POP();
>>
>> diff --git a/arch/ppc/qemu/qemu.fs b/arch/ppc/qemu/qemu.fs
>> index 3d99a34..10df618 100644
>> --- a/arch/ppc/qemu/qemu.fs
>> +++ b/arch/ppc/qemu/qemu.fs
>> @@ -138,3 +138,71 @@ variable keyboard-phandle 0 keyboard-phandle !
>>      3drop 0
>>    then
>>  ;
>> +
>> +\ Adds user's resolutions from /Options node's resolutions  
>> property to
>> +\ /pci/QEMU,VGA node's fb-modes property.
>> +\ Turns a string of "widthxheight,..." into an array of ints.
>> +
>> +: add-resolutions { ; fb-mode-addr fb-mode-len value res-addr res- 
>> len } ( -- )
>> +    decimal
>> +
>> +    " /options" find-device  ( -- )
>> +    " resolutions" active-package get-package-property  ( -- res- 
>> addr res-len false )
>> +    abort" "  \ Fail quietly because user resolutions are optional
>> +    -> res-len
>> +    -> res-addr
>> +
>> +    \ Add the first value to the array
>> +    0 0 res-addr res-len
>> +    >number
>> +    -> res-len -> res-addr
>> +    drop encode-int
>> +    -> fb-mode-len -> fb-mode-addr
>> +    res-addr res-len 1 /string
>> +    -> res-len -> res-addr
>> +
>> +    begin
>> +        res-len 0 = not
>> +    while
>> +        0 0
>> +        res-addr res-len
>> +        >number  ( ud1 c-addr1 u1 -- ud2 0 c-addr2 u2 )
>> +        -> res-len
>> +        -> res-addr
>> +        drop
>> +        -> value
>> +
>> +        \ go to the next number after 'x' or ','
>> +        res-addr res-len 1 /string
>> +        -> res-len
>> +        -> res-addr
>> +
>> +        \ Encode and add value to array
>> +        fb-mode-addr fb-mode-len value  ( -- fb-mode-addr fb-mode- 
>> len value )
>> +        encode-int  ( -- new-fb-mode-addr new-fb-mode-len )
>> +        encode+  ( -- new-fb-mode-addr new-fb-mode-len )
>> +        -> fb-mode-len
>> +        -> fb-mode-addr
>> +    repeat
>> +
>> +    \ Get the original resolutions
>> +    " /pci/QEMU,VGA" find-device  ( -- )
>> +    " fb-modes" active-package get-package-property  ( -- prop- 
>> addr prop-len false )
>> +    abort" Could not find fb-modes property in QEMU,VGA  
>> node!"  ( -- prop-addr prop-len )
>> +
>> +    \ Needed to fix a limit of encode+
>> +    encode-bytes  ( -- new-prop-addr new-prop-len )
>> +
>> +    fb-mode-addr fb-mode-len  ( -- new-prop-addr new-prop-len fb- 
>> mode-addr fb-mode-len )
>> +    2swap  ( --  fb-mode-addr fb-mode-len new-prop-addr new-prop- 
>> len )
>> +
>> +    \ Add user resolutions first then add built-in resolutions if  
>> space permits
>> +    encode+ ( -- new-addr new-len )
>> +    -> fb-mode-len
>> +    -> fb-mode-addr
>> +
>> +    \ Update fb-modes property
>> +    " /pci/QEMU,VGA" find-device
>> +    fb-mode-addr fb-mode-len
>> +    " fb-modes" property
>> +;
>>
>
> I've had a look at this, and I still don't see much change from my  
> last
> set of review comments. The main things here are that you need to flip
> this on its head so that the code to parse the textual /options  
> property
> is part of vga.fs and
> not part of the generic setup.

I did try to use my code in vga.fs, but none of the words I wanted to  
use were available.
The compiler would complain that words were not in the dictionary.

> Once the properties are encoded in the vga.fs node itself I don't  
> think
> there is a need for print-resolutions since you can just display the
> values using .properties.

The values printed with .properties do not help the user see what  
resolutions
are in the fb-modes property.

> Also I think you've missed a trick here in not making use of some  
> of the
> in-built Forth string functions e.g left-split to do some of the  
> parsing
> work so I'd suggest having a look around the Forth string functions to
> see if there is anything else that can help you.

Thank you for this advice. I will investigate these words shortly.

> Finally once the work has been moved into vga.fs I'm afraid you'll  
> need
> to remove all local variables since SPARC64 also makes use of the VGA
> driver and doesn't have local variables enabled.

That shouldn't be a big problem. I will see what I can do.