rcornwell / sims

Burroughs B5500, ICL1900, SEL32, IBM 360/370, IBM 7000 and DEC PDP10 KA10/KI10/KL10/KS10, PDP6 simulators for SimH
http://sky-visions.com
95 stars 20 forks source link

KA10: idling for DPY #148

Closed larsbrinkhoff closed 5 years ago

larsbrinkhoff commented 5 years ago

The current version of the Type 340 display DPY device doesn't do well with idling, causing the host CPU to run at 100%.

The svc routine runs every 50 microseconds. I tried replacing sim_activate_after with sim_clock_coschedule. While that was good for idling, it wasn't good for display updates.

If there is no way to reduce host CPU usage with an active display, it should still be possible to have it idling with a blank display.

CC @philbudne, @markpizz

larsbrinkhoff commented 5 years ago

The display code has a linked list of intensified points. I propose to add a function display_blank which returns true if the list is empty.

larsbrinkhoff commented 5 years ago

Something like this. Not tested yet.

larsbrinkhoff commented 5 years ago

@rcornwell wrote:

I am not sure it will be possible to get good display results with DPY and idle support. Since the display needs to be constantly refreshed.

The idea is to only idle when the display is blank. If the screen is all black (or greenish charcoal), it doesn't need to be refreshed.

larsbrinkhoff commented 5 years ago

I have now tested this. The display does indeed idle when inactive, and consume CPU when active.

However, it's not entirely correct. When I kill my program, the display keeps showing the same image, so aging doesn't work. I'll investigate.

larsbrinkhoff commented 5 years ago

The better method to fix the problem is to either shut the DPY down when it is not being used, or to have it detect a blank page and not schedule anything for some time.

This is exactly what this pull reqest is doing.

philbudne commented 5 years ago

Some thoughts:

someone could think display_blank means clear the display: call it display_is_blank so it's clear it's a predicate?

putting the predicate at the "display" level may be the only choice for the PDP-6 interface for the 340 display, because, unlike other "smart" display devices, the 340/PDP-6 combo doesn't DMA commands for itself. It might be a bad example of how to do this for smarter hardware (something like the VT11 which does do DMA AND can draw blinking text)

I can't help wondering: does ITS disable 340 data interrupts (the CONX_DC bit) if no one is using the display????

rcornwell commented 5 years ago

If the display is not being sent any more commands it should clear it's image and idle. As long as ITS continues to send commands the DPY should display them.

larsbrinkhoff commented 5 years ago

Thanks, those are good comments.

Yes, ITS does CONO DIS,0 when it's not in use.

BUT... some crafty program could still do DATAO DIS,X to feed it instructions without interrupts. I did this for my munching squares port, because running a display list at full speed turned out to be too quick!

rcornwell commented 5 years ago

If the DPY device activates the service routine when it has commands given to it. And if the service routine decides that there is no more pixels to age, then it can choose not to reschedule itself and the device will naturally go IDLE when there is nothing to display. This is sort of how IMP works.

larsbrinkhoff commented 5 years ago

I added this to display.c:

int
display_is_blank(void)
{
    return head->next == head;
}

I thought that an empty list here would mean that there are no visible pixels being displayed. But from my testing it seems this can return true even when there is graphics being displayed.

To kx10_dpy.c I added this whenever a CONO or DATAO is executed:

        if (!sim_activate_time(uptr))
            sim_activate_after(uptr, DPY_CYCLE_US);

This seems to work.

In dpy_svc I changed to:

    if (!display_blank() && !uptr->INT_COUNTDOWN == 0)
        sim_activate_after(uptr, DPY_CYCLE_US); /* requeue! */

It occurs to me this should be ||, not &&. That may explain why it doesn't work fully.

markpizz commented 5 years ago

FYI:

 if (!sim_activate_time(uptr))

should probably be:

if (!sim_is_active(uptr))

This is the correct since 1) it is asking the question you really want to ask and 2) it is VASTLY more efficient

larsbrinkhoff commented 5 years ago

Thanks Mark, yes that is what I really wanted.

I fixed the logic, idling works now.

rcornwell commented 5 years ago

Do we need the changes to display code since you don't appear to use them?

markpizz commented 5 years ago

Since, from Rich's point of view, the display code is part of SCP, submit generic changes directly to the simh repo.

rcornwell commented 5 years ago

From my point of view changes to display can come either through me or through the main simH repo. If there are changes that I have received I will push them when I do an update to fix bugs.

I would also make sure that changes to display do not effect the version of KA which is in simH.

philbudne commented 5 years ago

"idle" could be detected by the pattern of commands being sent to the display processor.

larsbrinkhoff commented 5 years ago

Well, I split the change in two commits. The first with the generic display changes, and the second with the KA10 part. Rich, if you accept that I'm fine with that.

I tested this with some standalone programs and with ITS.

rcornwell commented 5 years ago

It is fine the way it is. I am just asking about the need for the display change since you don't actually use the change.

larsbrinkhoff commented 5 years ago

I do use the change. dpy_svc calls display_is_blank.

rcornwell commented 5 years ago

Ok, see it now.

rcornwell commented 5 years ago

Updated.