[PATCH v2] Implement missing words for the ATI FCode ROM ati-vga emulation
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- drivers/pci.c | 89 ++++++++++++++++++++++++++++++++++++------- drivers/pci.fs | 41 ++++++++++++++++++++ drivers/vga.fs | 4 +- forth/device/other.fs | 6 +++ 4 files changed, 125 insertions(+), 15 deletions(-) diff --git a/drivers/pci.c b/drivers/pci.c index f30e427..787fa93 100644 --- a/drivers/pci.c +++ b/drivers/pci.c @@ -408,20 +408,68 @@ static void ob_pci_unmap(ucell virt, ucell size) { static void ob_pci_bus_map_in(int *idx) { - uint32_t ba; - ucell size; - ucell virt; + fword("pci-map-in"); +} + +static void +ob_pci_bus_map_out(int *idx) +{ + /* Should call pci-map-out but nothing to do in it yet */ + fword("2drop"); +} - PCI_DPRINTF("ob_pci_bar_map_in idx=%p\n", idx); +static void +ob_pci_config_read8(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint8_t val = pci_config_read8(addr, hi & 0xff); + PUSH(val); +} + +static void +ob_pci_config_write8(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write8(addr, hi & 0xff, val); +} + +static void +ob_pci_config_read16(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint16_t val = pci_config_read16(addr, hi & 0xff); + PUSH(val); +} - size = POP(); - POP(); - POP(); - ba = POP(); +static void +ob_pci_config_write16(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write16(addr, hi & 0xff, val); +} - virt = ob_pci_map(ba, size); +static void +ob_pci_config_read32(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint32_t val = pci_config_read32(addr, hi & 0xff); + PUSH(val); +} - PUSH(virt); +static void +ob_pci_config_write32(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write32(addr, hi & 0xff, val); } static void @@ -459,7 +507,14 @@ NODE_METHODS(ob_pci_bus_node) = { { "close", ob_pci_close }, { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, - { "pci-map-in", ob_pci_bus_map_in }, + { "map-in", ob_pci_bus_map_in }, + { "map-out", ob_pci_bus_map_out }, + { "config-b@", ob_pci_config_read8 }, + { "config-b!", ob_pci_config_write8 }, + { "config-w@", ob_pci_config_read16 }, + { "config-w!", ob_pci_config_write16 }, + { "config-l@", ob_pci_config_read32 }, + { "config-l!", ob_pci_config_write32 }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free }, { "dma-map-in", ob_pci_dma_map_in }, @@ -473,7 +528,14 @@ static void ob_pci_bridge_map_in(int *idx) { /* As per the IEEE-1275 PCI specification, chain up to the parent */ - call_parent_method("pci-map-in"); + call_parent_method("map-in"); +} + +static void +ob_pci_bridge_map_out(int *idx) +{ + /* As per the IEEE-1275 PCI specification, chain up to the parent */ + call_parent_method("map-out"); } NODE_METHODS(ob_pci_bridge_node) = { @@ -481,7 +543,8 @@ NODE_METHODS(ob_pci_bridge_node) = { { "close", ob_pci_close }, { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, - { "pci-map-in", ob_pci_bridge_map_in }, + { "map-in", ob_pci_bridge_map_in }, + { "map-out", ob_pci_bridge_map_out }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free }, { "dma-map-in", ob_pci_dma_map_in }, diff --git a/drivers/pci.fs b/drivers/pci.fs index a7b56e1..327c9c9 100644 --- a/drivers/pci.fs +++ b/drivers/pci.fs @@ -37,4 +37,45 @@ then ; +: pci-map-in ( addr.lo addr.mid addr.hi size -- virt ) + \ Ignore everything but addr.hi and find assigned addr for BAR + drop nip nip + \ Save my-self and set it to same as active package because otherwise + \ decode-phys below seems to be confused. Maybe this should not be needed + my-self swap active-package ihandle>phandle to my-self + " assigned-addresses" active-package get-package-property 0= if + begin + decode-phys ( bar prop prop-len phys.lo phys.mid phys.hi ) + dup 0fffffff and 6 pick 0fffffff and = if + 2drop -rot + decode-int drop decode-int drop 2drop + ( bar addr.lo ) + \ Now translate addr.lo, This may be wrong + \ maybe we should look at parent's ranges instead? + over 18 rshift 3 and 1 = if + " reg" active-package parent get-package-property 0= if + ( bar addr.lo prop prop-len reg.addr reg.size ) + decode-int -rot decode-int 3drop + + + then + then + nip swap to my-self + exit + else + 3drop \ no match, try next + then + \ Drop the size as we don't need it + decode-int drop decode-int drop + dup 0= + until + 3drop + to my-self + -1 \ not found + else + drop + to my-self + -1 \ could not find assigned-addresses + then + ; + [THEN] diff --git a/drivers/vga.fs b/drivers/vga.fs index 53dcff0..f3ffda7 100644 --- a/drivers/vga.fs +++ b/drivers/vga.fs @@ -146,14 +146,14 @@ defer vbe-iow! : map-fb ( -- ) cfg-bar0 pci-bar>pci-addr if \ ( pci-addr.lo pci-addr.mid pci-addr.hi size ) - " pci-map-in" $call-parent + " map-in" $call-parent to fb-addr then ; : map-mmio ( -- ) cfg-bar2 pci-bar>pci-addr if \ ( pci-addr.lo pci-addr.mid pci-addr.hi size ) - " pci-map-in" $call-parent + " map-in" $call-parent to mmio-addr then ; diff --git a/forth/device/other.fs b/forth/device/other.fs index 1bed9b8..0ff34b6 100644 --- a/forth/device/other.fs +++ b/forth/device/other.fs @@ -58,21 +58,27 @@ defer (poke) \ 5.3.7.2 Device-register access : rb@ ( addr -- byte ) + ioc@ ; : rw@ ( waddr -- w ) + iow@ ; : rl@ ( qaddr -- quad ) + iol@ ; : rb! ( byte addr -- ) + ioc! ; : rw! ( w waddr -- ) + iow! ; : rl! ( quad qaddr -- ) + iol! ; : rx@ ( oaddr - o ) -- 2.30.4
On 18/09/2022 21:55, BALATON Zoltan wrote:
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- drivers/pci.c | 89 ++++++++++++++++++++++++++++++++++++------- drivers/pci.fs | 41 ++++++++++++++++++++ drivers/vga.fs | 4 +- forth/device/other.fs | 6 +++ 4 files changed, 125 insertions(+), 15 deletions(-)
diff --git a/drivers/pci.c b/drivers/pci.c index f30e427..787fa93 100644 --- a/drivers/pci.c +++ b/drivers/pci.c @@ -408,20 +408,68 @@ static void ob_pci_unmap(ucell virt, ucell size) { static void ob_pci_bus_map_in(int *idx) { - uint32_t ba; - ucell size; - ucell virt; + fword("pci-map-in"); +} + +static void +ob_pci_bus_map_out(int *idx) +{ + /* Should call pci-map-out but nothing to do in it yet */ + fword("2drop"); +}
- PCI_DPRINTF("ob_pci_bar_map_in idx=%p\n", idx); +static void +ob_pci_config_read8(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint8_t val = pci_config_read8(addr, hi & 0xff); + PUSH(val); +} + +static void +ob_pci_config_write8(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write8(addr, hi & 0xff, val); +} + +static void +ob_pci_config_read16(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint16_t val = pci_config_read16(addr, hi & 0xff); + PUSH(val); +}
- size = POP(); - POP(); - POP(); - ba = POP(); +static void +ob_pci_config_write16(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write16(addr, hi & 0xff, val); +}
- virt = ob_pci_map(ba, size); +static void +ob_pci_config_read32(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint32_t val = pci_config_read32(addr, hi & 0xff); + PUSH(val); +}
- PUSH(virt); +static void +ob_pci_config_write32(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write32(addr, hi & 0xff, val); }
static void @@ -459,7 +507,14 @@ NODE_METHODS(ob_pci_bus_node) = { { "close", ob_pci_close }, { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, - { "pci-map-in", ob_pci_bus_map_in }, + { "map-in", ob_pci_bus_map_in }, + { "map-out", ob_pci_bus_map_out }, + { "config-b@", ob_pci_config_read8 }, + { "config-b!", ob_pci_config_write8 }, + { "config-w@", ob_pci_config_read16 }, + { "config-w!", ob_pci_config_write16 }, + { "config-l@", ob_pci_config_read32 }, + { "config-l!", ob_pci_config_write32 }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free }, { "dma-map-in", ob_pci_dma_map_in }, @@ -473,7 +528,14 @@ static void ob_pci_bridge_map_in(int *idx) { /* As per the IEEE-1275 PCI specification, chain up to the parent */ - call_parent_method("pci-map-in"); + call_parent_method("map-in"); +} + +static void +ob_pci_bridge_map_out(int *idx) +{ + /* As per the IEEE-1275 PCI specification, chain up to the parent */ + call_parent_method("map-out"); }
NODE_METHODS(ob_pci_bridge_node) = { @@ -481,7 +543,8 @@ NODE_METHODS(ob_pci_bridge_node) = { { "close", ob_pci_close }, { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, - { "pci-map-in", ob_pci_bridge_map_in }, + { "map-in", ob_pci_bridge_map_in }, + { "map-out", ob_pci_bridge_map_out }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free }, { "dma-map-in", ob_pci_dma_map_in }, diff --git a/drivers/pci.fs b/drivers/pci.fs index a7b56e1..327c9c9 100644 --- a/drivers/pci.fs +++ b/drivers/pci.fs @@ -37,4 +37,45 @@ then ;
+: pci-map-in ( addr.lo addr.mid addr.hi size -- virt ) + \ Ignore everything but addr.hi and find assigned addr for BAR + drop nip nip + \ Save my-self and set it to same as active package because otherwise + \ decode-phys below seems to be confused. Maybe this should not be needed + my-self swap active-package ihandle>phandle to my-self + " assigned-addresses" active-package get-package-property 0= if + begin + decode-phys ( bar prop prop-len phys.lo phys.mid phys.hi ) + dup 0fffffff and 6 pick 0fffffff and = if + 2drop -rot + decode-int drop decode-int drop 2drop + ( bar addr.lo ) + \ Now translate addr.lo, This may be wrong + \ maybe we should look at parent's ranges instead? + over 18 rshift 3 and 1 = if + " reg" active-package parent get-package-property 0= if + ( bar addr.lo prop prop-len reg.addr reg.size ) + decode-int -rot decode-int 3drop + + + then + then + nip swap to my-self + exit + else + 3drop \ no match, try next + then + \ Drop the size as we don't need it + decode-int drop decode-int drop + dup 0= + until + 3drop + to my-self + -1 \ not found + else + drop + to my-self + -1 \ could not find assigned-addresses + then + ; + [THEN] diff --git a/drivers/vga.fs b/drivers/vga.fs index 53dcff0..f3ffda7 100644 --- a/drivers/vga.fs +++ b/drivers/vga.fs @@ -146,14 +146,14 @@ defer vbe-iow!
: map-fb ( -- ) cfg-bar0 pci-bar>pci-addr if \ ( pci-addr.lo pci-addr.mid pci-addr.hi size ) - " pci-map-in" $call-parent + " map-in" $call-parent to fb-addr then ;
: map-mmio ( -- ) cfg-bar2 pci-bar>pci-addr if \ ( pci-addr.lo pci-addr.mid pci-addr.hi size ) - " pci-map-in" $call-parent + " map-in" $call-parent to mmio-addr then ; diff --git a/forth/device/other.fs b/forth/device/other.fs index 1bed9b8..0ff34b6 100644 --- a/forth/device/other.fs +++ b/forth/device/other.fs @@ -58,21 +58,27 @@ defer (poke) \ 5.3.7.2 Device-register access
: rb@ ( addr -- byte ) + ioc@ ;
: rw@ ( waddr -- w ) + iow@ ;
: rl@ ( qaddr -- quad ) + iol@ ;
: rb! ( byte addr -- ) + ioc! ;
: rw! ( w waddr -- ) + iow! ;
: rl! ( quad qaddr -- ) + iol! ;
: rx@ ( oaddr - o )
This mixes and matches and quite a few things in one patch that should be split out separately: I think you could do separate patches for i) the PCI config words and ii) the device register access words. What was the issue with the existing PCI map-in which causes it to fail on your FCode ROM? I'm trying to understand why the existing implementation needs to be replaced. ATB, Mark.
On Sun, 25 Sep 2022, Mark Cave-Ayland wrote:
On 18/09/2022 21:55, BALATON Zoltan wrote:
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- drivers/pci.c | 89 ++++++++++++++++++++++++++++++++++++------- drivers/pci.fs | 41 ++++++++++++++++++++ drivers/vga.fs | 4 +- forth/device/other.fs | 6 +++ 4 files changed, 125 insertions(+), 15 deletions(-)
diff --git a/drivers/pci.c b/drivers/pci.c index f30e427..787fa93 100644 --- a/drivers/pci.c +++ b/drivers/pci.c @@ -408,20 +408,68 @@ static void ob_pci_unmap(ucell virt, ucell size) { static void ob_pci_bus_map_in(int *idx) { - uint32_t ba; - ucell size; - ucell virt; + fword("pci-map-in"); +} + +static void +ob_pci_bus_map_out(int *idx) +{ + /* Should call pci-map-out but nothing to do in it yet */ + fword("2drop"); +} - PCI_DPRINTF("ob_pci_bar_map_in idx=%p\n", idx); +static void +ob_pci_config_read8(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint8_t val = pci_config_read8(addr, hi & 0xff); + PUSH(val); +} + +static void +ob_pci_config_write8(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write8(addr, hi & 0xff, val); +} + +static void +ob_pci_config_read16(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint16_t val = pci_config_read16(addr, hi & 0xff); + PUSH(val); +} - size = POP(); - POP(); - POP(); - ba = POP(); +static void +ob_pci_config_write16(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write16(addr, hi & 0xff, val); +} - virt = ob_pci_map(ba, size); +static void +ob_pci_config_read32(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint32_t val = pci_config_read32(addr, hi & 0xff); + PUSH(val); +} - PUSH(virt); +static void +ob_pci_config_write32(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write32(addr, hi & 0xff, val); } static void @@ -459,7 +507,14 @@ NODE_METHODS(ob_pci_bus_node) = { { "close", ob_pci_close }, { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, - { "pci-map-in", ob_pci_bus_map_in }, + { "map-in", ob_pci_bus_map_in }, + { "map-out", ob_pci_bus_map_out }, + { "config-b@", ob_pci_config_read8 }, + { "config-b!", ob_pci_config_write8 }, + { "config-w@", ob_pci_config_read16 }, + { "config-w!", ob_pci_config_write16 }, + { "config-l@", ob_pci_config_read32 }, + { "config-l!", ob_pci_config_write32 }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free }, { "dma-map-in", ob_pci_dma_map_in }, @@ -473,7 +528,14 @@ static void ob_pci_bridge_map_in(int *idx) { /* As per the IEEE-1275 PCI specification, chain up to the parent */ - call_parent_method("pci-map-in"); + call_parent_method("map-in"); +} + +static void +ob_pci_bridge_map_out(int *idx) +{ + /* As per the IEEE-1275 PCI specification, chain up to the parent */ + call_parent_method("map-out"); } NODE_METHODS(ob_pci_bridge_node) = { @@ -481,7 +543,8 @@ NODE_METHODS(ob_pci_bridge_node) = { { "close", ob_pci_close }, { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, - { "pci-map-in", ob_pci_bridge_map_in }, + { "map-in", ob_pci_bridge_map_in }, + { "map-out", ob_pci_bridge_map_out }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free }, { "dma-map-in", ob_pci_dma_map_in }, diff --git a/drivers/pci.fs b/drivers/pci.fs index a7b56e1..327c9c9 100644 --- a/drivers/pci.fs +++ b/drivers/pci.fs @@ -37,4 +37,45 @@ then ; +: pci-map-in ( addr.lo addr.mid addr.hi size -- virt ) + \ Ignore everything but addr.hi and find assigned addr for BAR + drop nip nip + \ Save my-self and set it to same as active package because otherwise + \ decode-phys below seems to be confused. Maybe this should not be needed + my-self swap active-package ihandle>phandle to my-self + " assigned-addresses" active-package get-package-property 0= if + begin + decode-phys ( bar prop prop-len phys.lo phys.mid phys.hi ) + dup 0fffffff and 6 pick 0fffffff and = if + 2drop -rot + decode-int drop decode-int drop 2drop + ( bar addr.lo ) + \ Now translate addr.lo, This may be wrong + \ maybe we should look at parent's ranges instead? + over 18 rshift 3 and 1 = if + " reg" active-package parent get-package-property 0= if + ( bar addr.lo prop prop-len reg.addr reg.size ) + decode-int -rot decode-int 3drop + + + then + then + nip swap to my-self + exit + else + 3drop \ no match, try next + then + \ Drop the size as we don't need it + decode-int drop decode-int drop + dup 0= + until + 3drop + to my-self + -1 \ not found + else + drop + to my-self + -1 \ could not find assigned-addresses + then + ; + [THEN] diff --git a/drivers/vga.fs b/drivers/vga.fs index 53dcff0..f3ffda7 100644 --- a/drivers/vga.fs +++ b/drivers/vga.fs @@ -146,14 +146,14 @@ defer vbe-iow! : map-fb ( -- ) cfg-bar0 pci-bar>pci-addr if \ ( pci-addr.lo pci-addr.mid pci-addr.hi size ) - " pci-map-in" $call-parent + " map-in" $call-parent to fb-addr then ; : map-mmio ( -- ) cfg-bar2 pci-bar>pci-addr if \ ( pci-addr.lo pci-addr.mid pci-addr.hi size ) - " pci-map-in" $call-parent + " map-in" $call-parent to mmio-addr then ; diff --git a/forth/device/other.fs b/forth/device/other.fs index 1bed9b8..0ff34b6 100644 --- a/forth/device/other.fs +++ b/forth/device/other.fs @@ -58,21 +58,27 @@ defer (poke) \ 5.3.7.2 Device-register access : rb@ ( addr -- byte ) + ioc@ ; : rw@ ( waddr -- w ) + iow@ ; : rl@ ( qaddr -- quad ) + iol@ ; : rb! ( byte addr -- ) + ioc! ; : rw! ( w waddr -- ) + iow! ; : rl! ( quad qaddr -- ) + iol! ; : rx@ ( oaddr - o )
This mixes and matches and quite a few things in one patch that should be split out separately: I think you could do separate patches for i) the PCI config words and ii) the device register access words.
These are fairly simple so I've originally submitted it as one patch to get comments on what to do with it next but then it got stuck because I never got reply to my questions on your first reply so I wasn't sure how to continue and it isn't important to me as I only used it to test some ATI ROMs. I've only returned to it now because somebody asked help with some ROMs which needed these. I can split it up if that helps, will do in the next version.
What was the issue with the existing PCI map-in which causes it to fail on your FCode ROM? I'm trying to understand why the existing implementation needs to be replaced.
I can't remember the details as this was almost 3 years ago but the existing implementation did not do what it should and did not return the values the ROM expected. Back then we've found it works with SLOF so I've looked at what it does and implemented it similarly here. It's not the same as in SLOF as the code is different but the idea was the same. AFAIR the map-in word despite its name does not really have to map anything as BARs are assigned by OpenBIOS when detecting devices (this is the same in SLOF) so this method just needs to return the assigned address for the BAR (maybe translated but most of the cases are 1 to 1 mapping). As this info is already in the appropriate property it's simplest to extract it from there in Forth. I don't understand what the pre-existing ob_pci_bus_map_in() tried to do and if it could be fixed but if you can provide an alternative in C I'm OK with that but I can't do it myself as that would take me too much time while this already works for me and others who tried it. Regards, BALATON Zoltan
participants (2)
-
BALATON Zoltan -
Mark Cave-Ayland