[Open-graphics] Some small changes
wpmcnamara at yahoo.com
Tue Nov 2 22:44:12 EDT 2010
On 11/02/2010 07:13 AM, Mark Marshall wrote:
> First some good news. I've managed to produce images for both FPGA's.
> For the XP10 I've used "ispLever Starter FPGA 8.1.00.34.21.10". For
> the S3 I used version 12..00 of the xilinx tools (I forget which
> I chose, we seem to have every version from 9.1 to 12.3 installed at
Very cool. What timing errors did you get on the XP10? Assuming that
you used the constraints from the repo, you likely got errors on the
> We don;t seem to ahve an sipLevel project checked in at the moment?
> Shall I try this or does someone else want to have a go?
This was on my to do list. I have not yet divined which of the files in
the project directory are actually needed. Running "clean all" doesn't
actually seem to remove all the generated files. If you want to tackle
this, be my guest. It will remove something from my list. Let me know
if you want my thoughts on what I was planning to try and do with it.
> The assembler used the "strtol" function to read in values. On a
> 32-bit machine this means that the values must fit into a "signed
> long", and are truncated to [LONG_MIN, LONG_MAX]. This meant that you
> couldn't have data words that were >= 0x80000000. My fix is to use a
> "long long" and "stroll" - this might not be ultra portable?
It is C99 compliant, but not C++98 compliant. I don't know that we care
and if in the future we run into a system that legitimately has
problems, we can deal with it then.
> Here I've split out all of the common code from the oga1_diag tools.
> This should make it much easier to make modifications to each tool.
> I'm sorry that this means that the build process is now slightly more
> complex, but I think the make file should deal with everything (At
> some point, objections were raised about making this code too
> "obscure", as it is really useful to be able to build it without mauch
> infrastructure or knowledge - I hope I've not hampered that).
I'll take a look at your changes. I stopped in the middle of updating
some of the SPI code to be more generic. It handled the BIOS prom, but
not yet the S3 prom. I need to get back to that.
> 740 + 743
> Some wires were in the verilog but never used. Removed to get a few
> less warnings.
Thanks. There are a lot of warnings on the S3. At some point we need
to see how many of them we can get rid of. It is rather hard to sort
out the useful ones.
> 741 + 744
> Put defines of the PCI commands into the oga1_defs header file and use
> it in the PCI verilog. This makes things mucah easier to read
> (especially for those who do not know there way around the PCI spec yet).
> The only other thing I've changed is that there used to be lines like:
> wire mem_space_i = cbe_d[3:1] == 'b011 ||
> cbe_d == 'b1100 ||
> cbe_d[3:1] == 'b111;
> Which I have chnaged to this:
> wire mem_space_i = (cbe_d == PCI_COMMAND_MEM_READ ||
> cbe_d == PCI_COMMAND_MEM_WRITE ||
> cbe_d == PCI_COMMAND_MEM_READ_N ||
> cbe_d == PCI_COMMAND_MEM_READ_LINE ||
> cbe_d == PCI_COMMAND_MEM_WRITE_INV);
> As far as I can tell the tools produce the same output, and the second
> is much easier to understand. If there really is a good technical
> reason for having lines like the first then I'll change them back, but
> I couldn't see any.
As Peter mentioned, these should synthesize into the same thing. The
original version is just the "simplified" version of what you wrote. I
like yours better. :)
> I hope no one minds that I've chanked in these changes. I have tested
> them all before hand, and they all seem OK. I have a few further
> changes in the piepline, they are also small:
As we don't have subsystem maintainers at this point, there isn't a
formal change submission process so I don't think anyone cares. I
suppose at some point though, we should start thinking about how we
handle submissions, both from folks with SVN commit rights and for those
We probably should start think about bug trackers as well.
> A change to how interrupts are dealt with. I will add the PCI defined
> disable interrupt bit and the interrupt status bit to config space. I
> also plan to modify the way in which the clear_interrupt register will
> work. From a lot of experience with interrupts in embeded platforms
> this is how it should work:
> There is a set number of different interrupt sources, all multiplexed
> onto the same interrupt. We have a number of registers that have the
> same structure (one bit per source), these registers are called
> "int_status", "int_mask" and "int_clear".
> If any interrupt source triggers (there's a hotplug event of
> whatever), then the corisponding bit in "int_status" gets sets. This
> always happens.
> If "(int_status & int_mask) != 0" then we raise an interrupt to the host.
> If a value is written to "int_clear", then the ones in the value
> written clear those interrupt sources (and only those). (This is
> often written as W1C in hardware specs.)
> The host will perform exactly this sequence to ack the interrupts.
> temp = int_status;
> int_clear = temp;
> This scheme produces "race-free" operation. Almost every other scheme
Sounds sane to me. I haven't looked at how current PCI interrupt
handling is done in OGD1, so I'll defer to your suggestion.
> A readable version of HQ_CONTROL
> It would be very useful to be able to read HQ control. I would also
> like to be able to read the HQ data memory. I will probably add these
> together. Reading HQ data memory is very useful for debug and
> possibly also useful for adding simple accelerator functions. Reading
> HQ program memory is a pain (As the type of memory instance would have
> to be changed) and I'm not sure it's that useful.
Is HQ program memory currently a BRAM? Assuming it is, what about the
following? Put a register in XP10 address space. A write to that
register resets and internal address register to address 0. Subsequent
reads would read the HQ program word at that address and auto increment
the address. The address can simply wrap at the top of the memory space.
I agree with you though, I'm not sure that reading the program memory is
> As always, I'd apperciate any feedback.
> Peter Stuge's git mirror is very useful for looking at these changes,
> if you don't have the source installed yourself.
> Open-graphics mailing list
> Open-graphics at duskglow.com
> List service provided by Duskglow Consulting, LLC (www.duskglow.com)
More information about the Open-graphics