qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 08/11] next-cube.c: move static old_scr2 variable to NeXTP


From: Mark Cave-Ayland
Subject: Re: [PATCH v2 08/11] next-cube.c: move static old_scr2 variable to NeXTPC
Date: Wed, 20 Dec 2023 19:36:27 +0000
User-agent: Mozilla Thunderbird

On 20/12/2023 19:20, Thomas Huth wrote:

Am Wed, 20 Dec 2023 13:16:38 +0000
schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>:

Move the old_scr2 variable to NeXTPC so that the old SCR2 register state is
stored along with the current SCR2 state.

Since the SCR2 register is 32-bits wide, convert old_scr2 to uint32_t and
update the SCR2 register access code to allow unaligned writes.

Note that this is a migration break, but as nothing will currently boot then
we do not need to worry about this now.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
  hw/m68k/next-cube.c | 12 +++++++++---
  1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
index f2222554fa..d53f73fb8b 100644
--- a/hw/m68k/next-cube.c
+++ b/hw/m68k/next-cube.c
@@ -91,6 +91,7 @@ struct NeXTPC {
uint32_t scr1;
      uint32_t scr2;
+    uint32_t old_scr2;
      uint32_t int_mask;
      uint32_t int_status;
      uint32_t led;
@@ -125,8 +126,7 @@ static const uint8_t rtc_ram2[32] = {
static void nextscr2_write(NeXTPC *s, uint32_t val, int size)
  {
-    static uint8_t old_scr2;
-    uint8_t scr2_2;
+    uint8_t old_scr2, scr2_2;
      NextRtc *rtc = &s->rtc;
if (size == 4) {
@@ -144,6 +144,8 @@ static void nextscr2_write(NeXTPC *s, uint32_t val, int 
size)
          }
      }
+ old_scr2 = (s->old_scr2 >> 8) & 0xff;
+
      if (scr2_2 & 0x1) {
          /* DPRINTF("RTC %x phase %i\n", scr2_2, rtc->phase); */
          if (rtc->phase == -1) {
@@ -252,7 +254,6 @@ static void nextscr2_write(NeXTPC *s, uint32_t val, int 
size)
      }
      s->scr2 = val & 0xFFFF00FF;
      s->scr2 |= scr2_2 << 8;

So s->scr2 is updated with the "val" at the end of nextscr2_write() ...

-    old_scr2 = scr2_2;
  }
static uint64_t next_mmio_read(void *opaque, hwaddr addr, unsigned size)
@@ -318,7 +319,10 @@ static void next_mmio_write(void *opaque, hwaddr addr, 
uint64_t val,
          break;
case 0xd000 ... 0xd003:
+        s->scr2 = deposit32(s->scr2, (4 - (addr - 0xd000) - size) << 3,
+                            size << 3, val);

... but here it is also updated before nextscr2_write() ? Looks somewhat
strange. Though I have to admit that I don't fully understand the logic
here anyway... Maybe we could peek at Previous to see how this register is
supposed to behave?

I'm fairly sure what's supposed to happen is that bits 8-15 of SCR2 are used to bit-bang the RTC and all the other values are preserved, hence the logic at the end of nextscr2_write(). What makes it slightly more confusing is that scr2_2 and old_scr2 in the current version of nextscr2_write() represent just bits 8-15 of SCR2 and not the entire register.

This is something I tried to improve in the following 2 commits by splitting out the LED logic into its own function, and then finally updating old_scr2 to a 32-bit value to match scr2 and using extract32()/deposit32() in next_scr2_rtc_update() to make this process clearer.


ATB,

Mark.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]