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@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.