[Chucko]

I was farting around with the 8061 documentation, and took a look at the patch in the GUFB strategy document that copies the 0-page (internal register) locations out to where the QH or TwEECer can observe them.

The existing code looks like this:

Code: Select all

84B2:
       ldw REG_42,#$84EE
84B6:
       ldw REG_14,[REG_42++]
       ldw REG_1C,[REG_14]
       stw REG_1C,$0D00[REG_14]
       cmpw Zero,[REG_42]
       jne $84B6

84C6:
       ...

I calculate that this takes 49 machine states, or roughly 9.8 microseconds, per word copied.

A slight reorganization makes this 3 bytes longer, but cuts the loop time to 42 machine states, or 8.4 us/word:

Code: Select all

84B2:
       ldw REG_42,#$84F2
       ldw REG_14,[REG_42++]
84B9:
       ldw REG_1C,[REG_14]
       stw REG_1C,$0D00[REG_14]
       ldw REG_14,[REG_42++]
       cmpw Zero,REG_14
       jne $84B9

84C9:
       ...

Could I simply drop this in as shown, moving the rest of the patch down 3 bytes and changing the start address of the list? Are other changes necessary to accommodate the slightly longer patch?

[cgrey8]

The main thing I see you doing differently is with the cmpw. Instead of comparing a pointed-to-value to zero, you compare a local register to zero, avoiding the pointer value lookup time. I haven't thought about what the values inside Reg14 and Reg84 would be and if they'd be different in your scenario or not. So I can't say for sure that this is a more efficient way of doing the same thing.

What I don't know is if there is any code immediately after this logic. As you show, this pushes the patch code into 0x84C6-84C8. I don't know if there's anything important there or not. If there isn't, why relocate the datalog list from 0x84EE to 0x84F2? If there is, what is it?

I'm at work and don't have a GUFB def to open and look at for myself. And I wasn't the original author of the patch code. So I'm not the best person to be asking if this is a good idea or not.

[Chucko]

The difference is the original code reads the next location from memory twice. I rearranged it to eliminate the duplicate read. The loop itself is shorter, but the initial read now happens outside the loop.

The code that follows simply initializes the 8 calibration ROM pointer registers at F0 - FE. Other than being moved by 3 bytes, it requires no changes. After that is the table of zero page locations to be copied.

What I worry about is the coordination with where BE puts the zero page list. Anyone seen Clint lately? I could email this to him.

[jsa]

moving the rest of the patch down 3 bytes..

to accommodate the slightly longer patch?

Your payload address space slots still have to end at 0x852F, though it appears you might be on to that by the first line of your revised patch.

[jsa]

What I worry about is the coordination with where BE puts the zero page list. Anyone seen Clint lately? I could email this to him.

BE will put the payload address space slots where ever you set in to be.

0x84F2 - 0x852F will be fine, however you could choose another location if desired.

Code: Select all

0x84F1  0xff      # filler
0x84F2  0000
.
.

[Chucko]

I'm pretty sure that DJNZ is a PC relative jump, and doesn't need adjusting.

I've been working from an A9L disassembly (thanks, fellow geeks!). The 0x852F restriction is the end of the calibration console routine that this patch overwrites. I know this is a hard limit.

[jsa]

Ok it should work.

[cgrey8]

That should be defined within the definition. I think it's the presence of 0000, which right now resides at 0x84EE. If that gets pushed to 0x84F2, as you showed, then I think BE will see that just like it see's that the payload begins at 0x84EE right now. At least that's my interpretation of how it works.

[sailorbob]

Your modification risks crashing the code as the check for the zero marker will be missed if there are no datalog payload items. You are also reducing the number of payload items.

[Chucko]

Your modification risks crashing the code as the check for the zero marker will be missed if there are no datalog payload items. You are also reducing the number of payload items.

The original code has the same behavior. It always copies at least one word unconditionally. It's straightforward to rearrange the loop to avoid this.

You're right that the change reduces the number of payload items from the internal registers. The original patch supports up to 32 words, and my proposed change cuts that to 30. The current GUFB def has a grand total of 21 payload items defined in the internal registers - and several of those are 1-byte fields that are adjacent to others - so I don't see that as a significant drawback.

The only question I have is, how does BE figure out where to put the payload list? Is there a hidden parameter somewhere in the definition that I'm overlooking, or does it look at that first LDW instruction in the patch?

[sailorbob]

The original code does not have this flaw, look at what you are loading into 0x0014 on the first pass through.

[86GT]

BE has a few things it looks for:

1) DatalogOffset parameter in cell D2 must be set to the address you have defined in the patch code. This is defined at B14 and B15
2) BE will place the parameters at 0000 which is at B62 in this example.
3) The items at B63-B127 must be defined in BE. This is what tells BE ho many parameters the patch code supports. If the PCM has the memory you can lengthen this if you so desired.

My take on this is:
if it's not broken don't fix it.

On the other hand if the parameters in the internal registers are not being updated at the rate of 8.4 us then there is no need to update the patch code. Parameters like temperature are slow reacting and do not require a fast update rate. PIP signals, on the other hand, are a different story.

[Chucko]

The original code does not have this flaw, look at what you are loading into 0x0014 on the first pass through.

It presumes the start of the payload list has been moved 4 bytes to make room for the 3-byte-longer patch.

Maybe I should have been a little clearer about this.

[Chucko]

Thanks for the detailed info, Clint. That's exactly what I wanted to know.

On the other hand if the parameters in the internal registers are not being updated at the rate of 8.4 us then there is no need to update the patch code. Parameters like temperature are slow reacting and do not require a fast update rate. PIP signals, on the other hand, are a different story.

My intent is not to update readings more frequently. It's for the patch code to use less time during the update.

Sorry, I'm a cycle counter by nature when I write code.

[Chucko]

My take on this is:
if it's not broken don't fix it.

I expected this response and I understand the reasoning. But c'mon, we're all hot-rodders!

[sailorbob]

The flaw has nothing to do with the location of the payload list.

[86GT]

I expected this response and I understand the reasoning. But c'mon, we're all hot-rodders!

LOL true. I am all for improvements if it is going to save processing time for other tasks.

I dont have a full understanding of the 8061 op code. I have not spent the time to learn it, so I leave that up to the experts .

[cgrey8]

...I dont have a full understanding of the 8061 op code. I have not spent the time to learn it, so I leave that up to the experts .

Ditto

But more to the point, the improvement in execution from a more optimized patch code logic just isn't going to resonate with anybody without detailing how that translates to a benefit to them. And I can't say that I have a solid grasp on this. Some of my knowledge may be inaccurate, but here's my stab at explaining it AND my opinion on how valuable I think it'd be.

Lets say you've update your tune's Spark table to differentiate between RPM Load values above 4000RPMs (as opposed to the last column being 4000) and you shift the rows around so your top row represent 160% and the 2nd to top row, maybe 110%. This way, you change the spark advance as RPMs go from 4000-6000 and as Load goes up to 160. Looks good on paper until you go datalogging and start noticing that some of the values you are getting for spark advance aren't what should be produced, but in other places it is. Then you start noticing that at high RPMs, you get these long stretches where the advance value just doesn't change, but it changes quite frequently at lower RPMs. What gives?

What you are seeing is the affects of background loop starvation. The less often the background loop gets to execute, the less often you get a new advance value. And the faster the engine spins, the more the higher priority code (aka ISRs) are getting run. At 5000RPMs, you might not get a complete background loop for an entire second on a GUFx era EEC!!! At 6500, they may not exist because the ISRs are dominating the processor. Somewhere around 6800 RPMs, the processor may not even keep up with the ISRs and then things like Spark and Injector fire calculations don't get calculated as fast as their results are needed...and the engine falls on its face with random misfires either due to lack of fuel or bad/no spark or both.

For N/A applications where advance doesn't need to change much and the engine stays below 6000RPMs, there's little harm done. It's not an ideal situation, but it's not horrible. However boosted applications where boost could be making big changes through this high load and rather critical region, you don't want to be commanding a spark advance associated with 110% Load as the instantaneous load is creeping into the 140s and 150s. Yet that's what you might get...and see in your datalog. So the only safe thing to do is sacrifice power in the lower RPM/Loads and set them to values that are safe for MAX RPM/Load so you don't command spark advance that's too high for the engine conditions.


All that said, I get the desire to minimize what all happens during background loops in order to make them more efficient and get more passes through during these high RPM crunch-times. What I'm doubting is the result. At idle, the background loop updates are numerous. It's when the RPMs are high that the efficiency begins to count for something. But it's not the background loop's efficiency that is really causing problems. And a minor optimization like this will likely not get noticed in ANY practical sense. It's simply that an 80s era microcontroller is undersized for the task of accurately and optimally running a V8 at really high RPMs.

So unless you are having a problem with getting the background loops you need during these time periods, I don't really see the practical benefits of messing with this unless it's just for giggles and something to do on your personal vehicle. But I'm not yet convinced it's going to improve anything enough to justify updating the defs for it.

And if you are running a race car and notice problems getting background loop execution consistency, then my recommendation would be to buy a modern aftermarket EEC setup like the Holley EFI setup they use to replace GM factory computers. They have modern day processors that are not even close to being challenged by a V8's real-time engine management requirements at 10,000 RPMs. At the very least, update to one of the 24MHz EEC-Vs. But my guess is the time and money it'd take to go from a GUFx era EEC to a 2000 era EEC just wouldn't be worth it over buying a complete aftermarket EEC replacement rig like a MegaSquirt or Holley Terminator/Dominator setup.

[86GT]

Totally agree Chris.

I have seen the 6800-7000 RPM issue in the GUFx and CBAZx series. At first I thought it was the TFI module. I then read that the TFI module was used on things like Mega Squirt without any RPM issues. What we found is that the RPM upper limit is an issue but so was the rate at which RPM climbed. We had a CBAZA that was seeing RPM go from being on the trans brake of 3800 to 7200 in less than a seconds and it would sputter and all kinds of bad things. We then did the same test but used less boost and the RPM climbed at about 2 seconds and it was okay all the way to 7200. Keep in mind we disabled all routines in the process that we could. No EGR or air pump and alto more.

If a patch code optimization can save some of this, then I am all for it. It might not be needed for most but it might help the datalogs look better and not so stair step!!!

[tvrfan]

On reading this thread, an idea occurs to me...

If you have an engine that can climb up the revs really fast (as you stated), then could it also be that the 'rolling average' calculations
need to change? have you already thought of that ?

Why -

On the A9L bins, (GUFB) the rolling average calcs actually use 'time elapsed' with a multiplication factor to limit the rate of change of the variables
(RPM is one) - so this is TIME based, not background loop or anything. So increasing those numbers will allow a faster rate of change per millisec,
for perhaps a slightly less stable idle state, or cruise or something. If you're running open loop anyway, it may not make much difference.....

Just my 2C worth, don't know if it's a valid idea, but thought I would suggest it anyway in case it helps.

A9L - rolling average subr at 3654, references out to the various factors at 0x97e8 to 9808

[86GT]

I have not looked into that but yes we had it in OL the entire time

[cgrey8]

It's become more and more apparent to me why car mfgs wanted more resolution about engine RPM than just PIP...enter the crank ring that has, what 36 points on it? So instead of getting a view of the crank speed 4 times/rev (less if it's a V6 or L4). It gets a view of the crank 36 times/rev so it can monitor changes in RPM during cylinder fires.

I can see this being very valuable at detecting which cylinders are more "powerful" than others or when the engine is stalling. So instead of having to allow multiple cylinder fires pass before positive detection, this can be detected before 1 cylinder fire has passed making the EEC far more responsive. But the same is true on fast-revving. If you have a very powerful engine that can fast-rev, the EEC can get a much more accurate assessment of instantaneous rotational acceleration which would definitely affect the cylinder air-charge calculation, which then affect transient enrichment, manifold filling modeling, and spark advance.

My guess is these older 80s era EECs just weren't fast enough to deal with 36 events/rotation at 6000RPMs. Heck, that would probably tax them a bit even at moderate RPMs.

[86GT]

Very true Chris. They also used the EDIS8 which handled all of the rpm
for sark related issues.

I have been working with strategy RAAD4 on my 67 and found rpm to be more stable because of the 36 to 1 reflector wheel. The processor can change spark per cylinder. I am working on adding the cylinder balance and power balance tests to be.

[Chucko]

What you are seeing is the affects of background loop starvation. The less often the background loop gets to execute, the less often you get a new advance value. And the faster the engine spins, the more the higher priority code (aka ISRs) are getting run. At 5000RPMs, you might not get a complete background loop for an entire second on a GUFx era EEC!!! At 6500, they may not exist because the ISRs are dominating the processor. Somewhere around 6800 RPMs, the processor may not even keep up with the ISRs and then things like Spark and Injector fire calculations don't get calculated as fast as their results are needed...and the engine falls on its face with random misfires either due to lack of fuel or bad/no spark or both.

This is a separate but related issue. My proposal doesn't help with this at all.

However I'm taking a good hard look at the GUFB disassembly, and may be making some tweaks in my own BINs.

Something like the MegaSquirt, with a faster, more modern microcontroller, is probably a better idea for a race car.

[cgrey8]

LOL, what you do to your own vehicle is your business. Feel free to experiment. The more you experiment, the more you learn.

My point was that even if you got this to work with your setup, I'm not sure I could justify putting it into a release version of GUFx as the benefits don't seem tangible enough to justify the change.

[Chucko]

Sailorbob was right, I was making a mistake. None of the loads or stores were setting the condition flags, so the JNE couldn't make the right decision. So I revised the patch:

Code: Select all

84B2:
       ldw REG_42,#$84EE
       ldw REG_14,[REG_42++]
84B9:
       ldw REG_1C,[REG_14]
       stw REG_1C,$0D00[REG_14]
       ad3w REG_14,Zero,[REG_42++]
       jnz $84B9

84C7:
       ...

The AD3W instruction adds the contents of the word pointed to by REG_42 to 0 (setting the zero flag if that word is 0) and puts the result in REG_14, then increments REG_42.

As a bonus, the revision cuts the loop time to 7.8 microseconds per word, faster than my first patch.

This loop takes up one additional byte compared to the original; but that byte can be made up later by replacing

Code: Select all

84CE:
    ldw REG_18,#$00F0

with

Code: Select all

84CF:
    ldzbw REG_18,#$F0

Which it turns out is 1 machine state quicker than the original LDW.

So the patch ends up with the same total length, AND is faster to boot.

I'll try this later today.

[Chucko]

It worked. I had a little glitch at first because I buggered the opcode for the AD3W. Got that sorted and it works fine.

[sailorbob]

The original flaw is still in there, look at what you are loading into 0x0014 on the first pass through.

[sailorbob]

The original code has the same behavior.

My apologies, the original has this same flaw too. Sloppy coding

BTW, jne and jnz are the same instruction.

[Chucko]

The original code has the same behavior.

My apologies, the original has this same flaw too. Sloppy coding

BTW, jne and jnz are the same instruction.

Hey, so my patch is bug-for-bug compatible!! That's the best outcome a maintenance coder can hope for.