Activity for Olaf Seibert
-
Olaf Seibert posted a comment on ticket #25
Great, that seems to work and produce the same program files as before. I plan to package this version for pkgsrc.
-
Incompatible change
-
I just thought of a reason why "swap the order of the checks for interrupt vs traps" could be a bad idea. The GUI action "enter the monitor" creates a trap which enters the monitor. So if we swap the order, you'd be in the monitor before the stack operations of the interrupt. But the check for breakpoints (etc) is still at the end. So if you enter the monitor due to a breakpoint (at the same clock cycle), you would still be after the stack operations. That would create a new kind of confusion, I...
-
I'm not the fastest in replying either :) CPU-history Regarding how an interrupt is represented in the CPU history: as it is, the history just stores the executed instructions as the time, the instruction address, 3 bytes of instruction, and the register values. (And which cpu executed this, but we can mostly ignore that). And when shown to the user, the ordinary disassembler is used. Which means that if you want to show something that isn't an actual instruction, we're very limited, because there...
-
I'm not the fastest in replying either :) CPU-history Regarding how an interrupt is represented in the CPU history: as it is, the history just stores the executed instructions as the time, 3 bytes of instruction, and the register values. (And which cpu executed this, but we can mostly ignore that). And when shown to the user, the ordinary disassembler is used. Which means that if you want to show something that isn't an actual instruction, we're very limited, because there isn't really a single byte...
-
With both changes above, I ran a full testbench with x64sc and x64 and there were no regressions. The 0, 0, 0 in monitor_cpuhistory_store(...), it registers a BRK instruction at $FFFE or $FFFA. I guess it makes sense since an irq is really a brk. But I could fill in any instruction, if that would be clearer somehow. JMP ($FFFA) could also be a candidate, in a sense.
-
Algorithmix: what you describe matches closely to what I was seeing, by examining the source code. This DO_INTERRUPT macro I mentioned, when it detects that an interrupt should trigger, does indeed do the part of pushing the PC and fetching the interrupt vector. Then after that it maybe drops into the monitor. So in principle, when you're in the monitor, the PC should show that this happened, because the PC should already be the start of the interrupt routine. But it seems that this isn't very clear,...
-
Algorithmix: what you describe matches closely to what I was seeing, by examining the source code. This DO_INTERRUPT macro I mentioned, when it detects that an interrupt should trigger, does indeed do the part of pushing the PC and fetching the interrupt vector. Then after that it maybe drops into the monitor. So in principle, when you're in the monitor, the PC should show that this happened, because the PC should already be the start of the interrupt routine. But it seems that this isn't very clear,...
-
Yes I have the same hesitation... (also, having re-read the original problems, the case I described, and the expected incorrect behaviour I would expect from it, sounds a lot like number 2 instead of number 1)
-
Okay, so what do we with the potential changes I posted? I based them on the available test programs, not on the description in this report. Those are a bit hard to replicate by hand, and so any potential fix is a bit hard to check. We could commit the patch, if we agree it improves some things and doesn't break anything else, but I'd like to have opinions on that first. But reading the descriptions, I don't think that my patch would fix all of those. However, even if the cause of the described issues...
-
Okay, so what do we with the potential changes I posted? I based them on the available test programs, not on the description in this report. Those are a bit hard to replicate by hand, and so any potential fix is a bit hard to check. We could commit the patch, if we agree it improves some things and doesn't break anything else, but I'd like to have opinions on that first. But reading the descriptions, I don't think that my patch would fix all of those. However, even if the cause of the described issues...
-
I've got a theory.... it must be bunnies! (oops, wrong tv series) So I set a breakpoint in the monitor, and I found that it is called from this part in the macro DO_INTERRUPT: if (ik & (IK_TRAP | IK_RESET)) { \ if (ik & IK_TRAP) { \ EXPORT_REGISTERS(); \ interrupt_do_trap(CPU_INT_STATUS, (uint16_t)reg_pc); \ IMPORT_REGISTERS(); \ if (CPU_INT_STATUS->global_pending_int & IK_RESET) { \ ik |= IK_RESET; \ } \ } \ and the checking for steps, breakpoints and watchpoints happens lower in the same macro...
-
Okay, I committed the change and it is in r45310. As soon as a build is available from https://github.com/VICE-Team/svn-mirror/releases you can try it.
-
This should fix bug #2052.
-
@stephan64: can you build a local version with the patch from above, to verify that this fixes the problem for you? Or do you need a pre-built version? (I would prefer to have an acknowledgement that this fixes the original issue before committing, if possible)
-
I looked at the source of vice-1.13 (which was mentioned above that the bug was already there) (I got it from Zimmers.net). The function in question, interrupt_set_irq(), is already there in recognizable form. But it does not have the changes to irq_pending_clk and irq_delay_cycles yet. It does have the part just below it, with last_stolen_cycles_clk (and a simpler else-part). So since that version is reported as broken already, it seems likely or at least possible that that part independently contributes...
-
Here is the full diff of "before" and "after". diff -ur interrupt-fix-before/x128-result.txt interrupt-fix-after/x128-result.txt --- interrupt-fix-before/x128-result.txt 2024-08-14 23:29:03.950919099 +0200 +++ interrupt-fix-after/x128-result.txt 2024-08-14 19:37:10.032335177 +0200 @@ -831,7 +831,7 @@ ../c128/z80/c128z80timing/,cb1c-rr-h.prg,ok,exitcode,,,,-1,-1,-1 ../c128/z80/c128z80timing/,cb1d-rr-l.prg,ok,exitcode,,,,-1,-1,-1 ../c128/z80/c128z80timing/,cb1e-rr-hl.prg,ok,exitcode,,,,-1,-1,-1 -../c128/z80/c128z80timing/,cb1f-rr-a.prg,timeout,exitcode,,,,-1,-1,-1...
-
I guess my fix doesn't change this then - it doesn't break into the monitor. I ran the testbench on several emus, probably more than the minimum needed: x128, x128c64, x64, x64dtv, x64sc, xcbm2, xpet, xplus4, xscpu64, xvic. There were some diffs: the tests in irqnoack of course, and some others that seem not 100% stable because they passed when I re-ran them. More tests would always be good. One scenario that I fear could still be broken is if the only active irq source turns off and in the same...
-
I think I found how the old code broke. The thing it did wrong was to not dispatch an IRQ when it really should. Take this sequence of events: - second irc source becomes active - cs->irq_delay_cycles = 0; from the above code happens - Searching for occurrences of cs->irq_delay_cycles the only places where it is read are the various copies of the function interrupt_check_irq_delay(), which contains if (cs->irq_delay_cycles >= delay_cycles) { if (!OPINFO_ENABLES_IRQ(*cs->last_opcode_info_ptr)) { return...
-
I have a proposal for what could be the fix. It is based on the following reasoning: When the first IRQ source activates, it activates the CPU's IRQ input, and "stuff happens". When another IRQ source activates, the input doesn't change, since it is already active. So code in interrupt.h, interrupt_set_irq(), when called with value != 0, should only "do something global" on the first IRQ. When an extra source activates, it can remember that this happened (for counting purposes), but not really anything...
-
Monitor crashes VICE on m 8900 to 89FF
-
petreu: adjust comments. VIAs have only 16 registers.
-
r45276 should have some fixes. The description doesn't really seem to say if "$8900 is your port for the datalines." is mirrored all the way to $89FF, but I suspect so, for easier address decoding. Same with the VIAs. I wonder why they didn't use the low-order address bits which are available on the expansion connector. Then you could address a whole page of RAM at once, instead of just a single byte...
-
petmem: add the range 8800-8fff to mem_bank_peek().
-
petreu.c: fix bug #2061
-
I have no personal experience with this hardware, but googling gave me http://www.cbmhardware.de/show.php?r=14&id=73/PET%20REU%20(RAM%20AND%20EXPANSION%20UNIT) Examining the code, I guess you were lucky that it only crashed for the 2 MB size; it should crash for all sizes except the 128 KB one.... real_bank_value = (real_bank_value & ((petreu_size_kb >> 4) - 1)); is I think the problem. It should be >> 6, and with this it leaves too many address bits set. The bank are address bit 16 and higher, so...
-
Number of cycles / lines incorrect on PET/CBM 4032
-
Thanks for confirming! I'm closing now.
-
Use of uninitialized array in 6809 assembler
-
Number of cycles / lines incorrect on PET/CBM 4032
-
Virtual filesystem mishandles some dir patterns
-
Any feedback?
-
Use size_t to store the result from strlen().
-
In r45226 I fixed the matching of the NUL pattern (the one that matches no files). In r45227 I improved the actual matching, so that you can have text after the * wildcard like in the 1581. Only one star is supported. It allows A*B to properly match ABBBBB which previously would fail (it looked ahead at the first B after the A and never tried any others). Now we skip ahead to 1 character before the end of the name. This works for ? wildcards and normal characters alike.
-
fsdevice: fix file name pattern matching.
-
fsdevice: fix bug #2047.
-
About (int)(ted.cycles_per_line * (line - current_line))): both line and current_line are unsigned, hence the difference between them is unsigned, hence the product is unsigned. I think that we should fix the issue closest to the source, so both line and current_line should be cast to int, so that the difference is a proper int. Otherwise, if it's unsigned but should be negative, it's a very large number and casting that to int is (I think) not well defined or even Undefined Behaviour(TM).
-
CRTC: add explanatory comment about ycounter.
-
Nice! I'm fairly confident that the results from the real thing will match, but it is always good to be sure. In a few moments I plan to commit the cleaned up version, which should behave exactly the same. I got rid of the goto by re-arranging the order of the code, so things happen in a more natural order. I also got rid of that suspicious + 1 we mentioned before, and another + 1 that was just plain ugly. Edit: it's in r45221.
-
CRTC: Make the fix for bug #2054 nicer.
-
Nice! I'm fairly confident that the results from the real thing will match, but it is always good to be sure. In a few moments I plan to commit the cleaned up version, which should behave exactly the same. I got rid of the goto by re-arranging the order of the code, so things happen in a more natural order. I also got rid of that suspicious + 1 we mentioned before, and another + 1 that was just plain ugly.
-
De-spacify the endline.
-
Sometimes I hate those style checks...
-
Okay, it should be in r45219 when it's available. I was hoping to keep the lifetime of the hack as short as possible, but getting a confirmation from the original reporter is more important :-)
-
Ugly bug fix for bug #2054.
-
I have here a horrible hack for you, which indeed special cases when reg5 == 0. I think it gets all cases right, but the flow of control through the function is horrible. But it could work as a first version of the fix (showing the intention) and then I'd do some improvement afterwards. Note that I just committed a change that only un-indents a block of code that is between {} without need, so update your sources before trying this patch. Index: crtc.c ===================================================================...
-
Un-indent this needlessly indented code.
-
So, just to confirm: this off-by-one for the values of register 5 only occurs if you put 0 in it, right? Changing it to 1 makes no change, but then the timing behaves correctly. And when you set it to 2 the timing does change, and in the correct way. The code I'm looking at in crtc/crtc.c has a test if ((crtc.raster.ycounter + 1) >= crtc.regs[CRTC_REG_VTOTALADJ]) where one could think that the bug is off-by-one in the other direction (the end of frame triggers too soon). So it's a bit mysterious....
-
At least it is unlikely to be any of the CIA rewrites then, which is a relief (for me, since I made a lot of changes there)
-
I gave it a quick try and the 1581 seems to handle 1 * with following text, but not 2.
-
I see. And maybe it has the same incorrect algorithm that can be found in Vice? For example, if your pattern is F*C it doesn't match the filename fsdevice-flush.c because after the F* it looks for the next C which will be the one near the end of "device", and it never tries the next one. (Yes I tried the file system device on the directory that contains its source code...)
-
Virtual filesystem mishandles some dir patterns
-
I noticed another weird thing: LOAD"$0:NONEXISTENT",8 results in a FILE NOT FOUND ERROR which is incorrect. It should load an "empty" directory listing too. I propose the following patch. Can you try if it works for all cases you can think of? I tried - $ should list all - $: should match nothing - $0 should list all - $0: should match nothing - $:pattern - $0:pattern - $pattern I found that the pattern matching routine also tries to match patterns where the * isn't at the end. For example X*Y. But...
-
Any weirdness involving two shifts plus another key (or possibly other 3-or-more-key combinations) can also be messed up by your physical keyboard. Recently there was bug report 2038 https://sourceforge.net/p/vice-emu/bugs/2038/ where that seemed to be the case, at least when I tried it with my keyboard. So if you have tools on your machine to show keyboard events (such as xev for X), you can try and see if the keyboard is even generating the keypresses you want it to.
-
I tried in X with xev, and also in a text console (no X), I also don't get key presses from double-shift + ZXCV and some more letters), so for my case at least it must be the keyboard, and not some driver or keymap in X which suppresses these.
-
I see similar things, yes. I say "similar" because I tried it first on my $WORK-Mac (which is restricted in what I am allowed to install, but Homebrew packages are allowed) . All of the letters that I tried worked with two shifts, and after letting go of the shift keys, the shift key "stuck" until I pressed and released them again (it seemed unclear if I needed to do that left, or right, or both in some order). That was with gtk3. On my NetBSD machine, it worked more like you describe (with US key...
-
For the PETs, the sound output doesn't really count as "adapters", since it's built-in for 4xxx models and higher (even if the piezo speaker is crap). For the VIC-20, it does have a CB2 output on the user port, but it also has a real sound generator, so there is no reason to use the CB2 output. Conceivably one could use it for sound, but I have no evidence that anyone has done this.
-
Use of uninitialized array in 6809 assembler
-
(re previous remark: it's slightly more complicated, unfortunately). r45162 should fix that but I don't have gcc 14.1 at hand to test.
-
Use of uninitialized array in 6809 assembler
-
(re previous remark: it's slightly more complicated, unfortunately). r45162 should fix that but I don't have gcc 14.1 at hand to test.
-
Initialize opc[], and use post-byte with submode in the same cases as asm_addr_mode_get_size() does.
-
But I would say that the code in mon_assemble_6809.c above the quoted part already constructs the whole instruction, and therefore should know already what the length is, so it should not even need to call asm_addr_mode_get_size...
-
I think it's mostly the first case (apart from one small detail that I found while looking at it). In mon_assemble_6809.c the opc array is filled like this (with an optional prefix, always an opcode and maybe a post-byte): opc_offset = 0; if (prefix[j] != -1) { opc[opc_offset++] = prefix[j]; } opc[opc_offset++] = opcode; if (operand_mode == ASM_ADDR_MODE_INDEXED) { opc[opc_offset++] = (uint8_t)operand_submode; } len = (monitor_cpu_for_memspace[mem]->asm_addr_mode_get_size) ((unsigned int)operand_mode,...
-
I found the name of the first char value[]: Flexible array member. See https://en.wikipedia.org/wiki/Flexible_array_member which refers to http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf (C99 with Technical Corrigendum 1-3). It also already describes <inttypes.h> and <stdint.h>. These are so old by now, I would expect any C compiler to have them, and I would simply disregard any "C compilers" which do not provide those headers. That really simplifies the contents of <regina64.h> a lot....
-
Ah yes, I can see how the char value[4] is confusing the compiler. The official way to do that is, I think, still char value[1] or maybe char value[] (I think that has been changing over time). Unfortunately these warnings are so many, that they hide some other types of warning too. For example incorrect printf format specifiers: In file included from /usr/pkg/gcc12/lib/gcc/x86_64--netbsd/12.3.0/include-fixed/stdio.h:611, from ./rexx.h:222, from ./files.c:87: ./files.c: In function 'getstatus': ./files.c:4146:16:...
-
Ah yes, I can see how the char value[4] is confusing the compiler. The official way to do that is, I think, still char value[1] or maybe char value[] (I think that has been changing over time). Unfortunately these warnings are so many, that they hide some other types of warning too. For example incorrect printf format specifiers: In file included from /usr/pkg/gcc12/lib/gcc/x86_64--netbsd/12.3.0/include-fixed/stdio.h:611, from ./rexx.h:222, from ./files.c:87: ./files.c: In function 'getstatus': ./files.c:4146:16:...
-
32 and 64 bit madness
-
Regina seems to be full of buffer overflows
-
Handling of AM/PM wrong if time of day is already at 12 o'clock
-
Yes let's close it.
-
testbench: add option -power60 for vicii-ntsc
-
CIA/tod/stability-ntsc: keep the 50/60 Hz line input bit stable.
-
I'm committing change back to the old way of toggling the AM/PM bit. We're assuming for now that this does not depend on the CIA model (or other hardware variation) nor on the old state of the AM/PM bit. If tests turn up that contradict this we'll fix it.
-
CIA: always toggle AM/PM flag when writing 12 o'clock time
-
I just thought of another question. What happens if you write $12 to the hours register when it already contails 12 o'clock , and the AM/PM bit is the same as what was in it before. Does PM toggle? And what happens if you do this but the only changed bit is the PM bit?
-
I just added it to the testbench.
-
Add ampmtest.prg to the testbench
-
So now I have this patch so I can switch quicly between OLD and NEW versions of the code while testing. For now this assumes it isn't model-dependent, so it doesn't need to be a run time choice. Index: ciacore.c =================================================================== --- ciacore.c (revision 44986) +++ ciacore.c (working copy) @@ -859,6 +859,16 @@ if (addr == CIA_TOD_HR) { /* force bits 6-5 = 0 */ byte &= 0x9f; +#define OLD 1 +#define NEW (!OLD) +#if OLD + /* Flip AM/PM on hour 12 */ +...
-
Yes you have a point there. I was musing... suppose we must take "on the rising edge" as absolute truth (but I haven't re-looked at the schematic yet), could this be caused a write causing some glitch on the comparator output? And if so, could it be model-dependent? (old vs new CIA) Just wondering so far...
-
So looking at both variants of the code, it seems that the only difference is that in the new state, there is an extra condition, if (changed). That does match the remark /* Flip AM/PM on hour 12 on the rising edge of the comparator */. So there are 2 things to check: - if the schematics from http://forum.6502.org/viewtopic.php?f=4&t=7418&sid=38219c61eebb1ccd1190624aaf233042&start=30#p96981 really say that the AM/PM bit is flipped on exactly that condition - change the code to remove that condition...
-
xpet 3.7.1: Sample playback trick doesn't work (but worked in 3.6.1)
-
I'm closing this one as fixed...
-
A monitor error in x128 Bank1 c64mode
-
Great! I'll close this one then.
-
I committed the proposed fix in r44940. Please retest when the build is available from https://github.com/VICE-Team/svn-mirror/releases . Thanks!
-
Correct an address testing error; should fix bug #1974.
-
I was the last one to touch the monitor code that reads the memory for the D and M commands, but I think I traced this to something else. To trace what the monitor is reading, I set a breakpoint at c128mem.c:mem_bank_peek() -> case bank256_cpu: -> mem_peek_with_config(mem_config which is 7 -> no special case applies, so at the bottom -> mem_peek_with_config_c64(config -> ram_peek_c64(addr) uint8_t *b = mem_read_base_tab[mem_config][addr >> 8]; if ((addr & 0xff) == 0) { return zero_peek(addr); } else...
-
I was the last one to touch the monitor code that reads the memory for the D and M commands, but I think I traced this to something else. To trace what the monitor is reading, I set a breakpoint at c128mem.c:mem_bank_peek() -> case bank256_cpu: -> mem_peek_with_config(mem_config which is 7 -> no special case applies, so at the bottom -> mem_peek_with_config_c64(config -> ram_peek_c64(addr) uint8_t *b = mem_read_base_tab[mem_config][addr >> 8]; if ((addr & 0xff) == 0) { return zero_peek(addr); } else...
-
I was the last one to touch the monitor code that reads the memory for the D and M commands, but I think I traced this to something else. To trace what the monitor is reading, I set a breakpoint at c128mem.c:mem_bank_peek() -> case bank256_cpu: -> mem_peek_with_config(mem_config which is 7 -> no special case applies, so at the bottom -> mem_peek_with_config_c64(config -> ram_peek_c64(addr) which just reads bank 0 uint8_t *b = mem_read_base_tab[mem_config][addr >> 8]; if ((addr & 0xff) == 0) { return...
-
Yes, I can reproduce it if I run x128 -default b1c64mode.prg, then go into the monitor: Available banks (some may be equivalent to others): *default *cpu ram rom io ram00-01 intfunc extfunc cart c64rom vdc (C:$e5d1) d 3800 3800 .C:3800 2F 40 8D RLA $8D40 (C:$3803) bank ram01 (C:$3803) d 3800 3800 .C:3800 A9 40 LDA #$40 (C:$3802)
-
Change inefficient quadratic loop into a linear one.
-
Fix some warnings.
-
I found some small timing inaccuracies when generating the sound output and with low values of the timer they could easily turn a "nice" high tone into some distorted noise. That improved gridrunner a lot and I think your sample player should be improved somewhat by that too. You can try r44805 if you like.
-
Add "offset" parameter to via_context->set_cb2() callback.
-
Lower the default for CB2Lowpass to 8000
-
Supply accurate clock for VIA shift register.
-
Don't reset internal data when initing the PET sound.
-
Mention models with CRTC too (they were improved later than those without).
-
Fix texi2html a bit.
1 >