Skip to content

[Proposal] Implement better sync that allows back-to-back bus transfers #106

Description

@amykyta3

Background

Currently, all bus implementations have an optional sync argument that, when True, unconditionally awaits the next RisingEdge of the associated bus clock before starting the next transaction.

The reason for this is clear - to issue a proper transaction, you need to first ensure that the simulation is aligned with the associated bus clock to avoid a "late" initiation of the first cycle of the transfer. To avoid such issues, sync defaults to True so that users encounter safe bus behavior by default.

Why this is a problem

Unfortunately the side-effect of issuing transactions with sync=True all the time means that there will always be an idle cycle inserted between transactions. For example with AXI-4 Lite:

await Combine(
    cocotb.start_soon(axi.write(0x4, 0x1234, sync=True)),
    cocotb.start_soon(axi.write(0x8, 0x5678, sync=True)),
    cocotb.start_soon(axi.write(0xc, 0x9abc, sync=True)),
)
Image Despite `*ready` signals asserted from the DUT, the driver inserts idle cycles between each transfer.

At initial glance, a really simple workaround for this exists: Only sync the first transaction, and don't sync the rest:

await Combine(
    cocotb.start_soon(axi.write(0x4, 0x1234, sync=True)),
    cocotb.start_soon(axi.write(0x8, 0x5678, sync=False)),
    cocotb.start_soon(axi.write(0xc, 0x9abc, sync=False)),
)

.. but in reality this ends up being very impractical. Concurrent bus activities often get issued from different unrelated co-routines, which might be interleaved with arbitrary Timer delays, with little ability to coordinate.
Coordinating this is possible, but it becomes a programming anti-pattern.

The end result is that most often, people will use the default sync=True all the time since it is simply easier to deal with.
Unfortunately that results in back-to-back bus transfers to never get tested which is a pretty bad thing.

Proposed solution

Rather than implementing the sync mechanism via:

await RisingEdge(self.clock)

... replace it with a more intelligent scheme that awaits a rising edge only if necessary. If the current cocotb phase is already triggered due to a rising edge for that same clock (ie: it is already synced) then it is not necessary to wait again and insert an unnecessary cycle delay.

This is possible by querying current_gpi_trigger() via the cocotb API:

trigger = cocotb.triggers.current_gpi_trigger()
if isinstance(trigger, RisingEdge) and trigger.signal == self.clock:
    # Already in the phase of the time step where the desired signal has
    # experienced a rising-edge. Do nothing
    pass
else:
    # Not in-phase with a rising edge. Wait until next
    await self.clock.rising_edge

This is slightly imperfect since if the prior RisingEdge was awaited on a different, but equivalent clock signal, then it will incorrectly assume that it is not in-phase and unnecessarily advance simulation time. Even so, this is acceptable since it will still fall back to the safer behavior.

If this is something the cocotb-bus maintainers would be interested in, I'd be happy to submit a PR for this.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions