Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simulations with Instances #392

Open
rroohhh opened this issue May 28, 2020 · 17 comments
Open

Simulations with Instances #392

rroohhh opened this issue May 28, 2020 · 17 comments

Comments

@rroohhh
Copy link
Contributor

rroohhh commented May 28, 2020

To make simulations of designs tightly integrated with fpga specific resources (like the PS7 on zynq) easier it would be nice to have some way to overwrite signals driven from / into Instance's.

@whitequark
Copy link
Member

Yep, this is planned with the next round of simulator improvements. I'm not sure if replacing Instances with simulation models is the best way to address your specific problem (it could be), but it'll be possible.

@rroohhh
Copy link
Contributor Author

rroohhh commented May 28, 2020

For my specifc problem it would be helpful to drive Instance io's from the simulator (for example to initiate AXI reads / writes coming from the PS7) and being able substitue FHDL simulation models.

Furthermore it would be nice to be able to create clocks from the simulation models to make it posible to simulate things with PLL's.

@whitequark
Copy link
Member

For my specifc problem it would be helpful to drive Instance io's from the simulator (for example to initiate AXI reads / writes coming from the PS7) and being able substitue FHDL simulation models.

Ah, so essentially you'd like the simulator to treat the ports of an unknown Instance as toplevel inputs and outputs, right?

Furthermore it would be nice to be able to create clocks from the simulation models to make it posible to simulate things with PLL's.

You can already do that!

@rroohhh
Copy link
Contributor Author

rroohhh commented May 28, 2020

Ah, so essentially you'd like the simulator to treat the ports of an unknown Instance as toplevel inputs and outputs, right?

Correct. (or disconnect them, so the signals connected to the ports can be driven directly)

Furthermore it would be nice to be able to create clocks from the simulation models to make it posible to simulate things with PLL's.

You can already do that!

How?

@whitequark
Copy link
Member

(or disconnect them, so the signals connected to the ports can be driven directly)

That's the same thing. Toplevel ports currently are treated as disconnected.

@whitequark
Copy link
Member

How?

Can you describe an example of what you want to do with a PLL?

@rroohhh
Copy link
Contributor Author

rroohhh commented May 28, 2020

Say I have a instance like this:

Instance("MMCME2_ADV",
    p_BANDWIDTH="OPTIMIZED",
    i_RST=mmcm_reset, o_LOCKED=mmcm_locked,

    p_REF_JITTER1=0.01, p_CLKIN1_PERIOD=10.0,
    p_CLKFBOUT_MULT_F=30.0, p_CLKFBOUT_PHASE=0.000, p_DIVCLK_DIVIDE=2,
    i_CLKIN1=ClockSignal("clk100"), i_CLKFBIN=mmcm_fb, o_CLKFBOUT=mmcm_fb,

    p_CLKOUT0_DIVIDE_F=10.0, p_CLKOUT0_PHASE=0.000, o_CLKOUT0=ClockSignal("mmcm_clk1"),
    p_CLKOUT1_DIVIDE=2, p_CLKOUT1_PHASE=0.000, o_CLKOUT1=ClockSignal("mmcm_clk2"))

I would like the mmcm_clk{1,2} domains to be work (without manually doing add_clock in my testbench).

@whitequark
Copy link
Member

Are the derived clocks an integer fraction of the input clock?

@rroohhh
Copy link
Contributor Author

rroohhh commented May 28, 2020

No and they could even have phase shifts.

@whitequark
Copy link
Member

OK, so the only way this could possibly work (other than implementing an actual simulated PLL) is if the implementation of the PLL instance in nmigen would be able to magically request the clock rate at its input port and then drive the output ports appropriately. There's currently no provision for this in nMigen and I'm not aware of other simulators providing similar capabilities.

That doesn't mean though that we can't implement this, but I'm not sure what the API would look like.

@rroohhh
Copy link
Contributor Author

rroohhh commented May 28, 2020

Yes thats what I meant by

Furthermore it would be nice to be able to create clocks from the simulation models to make it posible to simulate things with PLL's.

sorry that that wasn't clear.

API wise, mabye a special signal type could work, like this:

clock = SimulatedClock(frequency=1e6, phase=0)
m.d.comb += self.clkout0.eq(clock)

@whitequark
Copy link
Member

I'm not really sure how that helps?

@rroohhh
Copy link
Contributor Author

rroohhh commented May 28, 2020

Then maybe I misunderstood this:

[...], but I'm not sure what the API would look like.

I am not sure what you have planned for the simulation models that would replace instance's in a simulation, but I was assuming they would just be normal nmigen modules, so you could write something like this:

class MMCMSimModel:
    def __init__(self, **params):
        self.clkout0 = Signal()
    def elaborate(self, plat):e
        clock = SimulatedClock(frequency=1e6, phase=0)
        m.d.comb += self.clkout0.eq(clock)

(Obviously a real simulation model for the MMCM would not be this simple)

@whitequark
Copy link
Member

I was assuming they would just be normal nmigen modules, so you could write something like this:

Ah right so if your Instance can be replaced with a normal nMigen module then absolutely nothing special needs to be done because you can wrap it with a module that, in its elaborate method, would return Instance for a real platform and some simulation-only code otherwise. So there isn't anything to be done for that case.

I am not sure what you have planned for the simulation models that would replace instance's in a simulation

The plan was to make it possible to attach processes to instances that would be automatically added to the simulator whenever that instance is used. I.e. black boxes implemented in Python.

        clock = SimulatedClock(frequency=1e6, phase=0)
        m.d.comb += self.clkout0.eq(clock)

This wouldn't really work because you have to get the frequency of the input clock somehow.

@rroohhh
Copy link
Contributor Author

rroohhh commented May 28, 2020

Ah right so if your Instance can be replaced with a normal nMigen module then absolutely nothing special needs to be done because you can wrap it with a module that, in its elaborate method, would return Instance for a real platform and some simulation-only code otherwise. So there isn't anything to be done for that case.

Hmm how would I create a clock (that might be faster / phase shifted compared to the input clock) from a normal nMigen module?

The plan was to make it possible to attach processes to instances that would be automatically added to the simulator whenever that instance is used. I.e. black boxes implemented in Python.

I see, then I am also not really sure how the API could look like.

This wouldn't really work because you have to get the frequency of the input clock somehow.

True, although in this specific case it probably would work, as you need to specify the input clock period (CLKIN1_PERIOD) for vivado's timing analysis I think.

@whitequark
Copy link
Member

Hmm how would I create a clock (that might be faster / phase shifted compared to the input clock) from a normal nMigen module?

You can't. I was only talking about the replacement of Instances with Modules for simulation here. It is a fundamental limitation of nMigen that you can't use a normal Module to create a faster clock from a slower clock and that's not going to change.

True, although in this specific case it probably would work, as you need to specify the input clock period (CLKIN1_PERIOD) for vivado's timing analysis I think.

Right, that makes this feature a lot simpler. We can start by adding support for just this sub-case (since it doesn't require much API design) and then expand it as needed.

@rroohhh
Copy link
Contributor Author

rroohhh commented May 28, 2020

You can't. I was only talking about the replacement of Instances with Modules for simulation here. It is a fundamental limitation of nMigen that you can't use a normal Module to create a faster clock from a slower clock and that's not going to change.

Ok, makes sense.

Right, that makes this feature a lot simpler. We can start by adding support for just this sub-case (since it doesn't require much API design) and then expand it as needed.

Sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants