clash-lang / clash-prelude

CLaSH prelude library containing datatypes and functions for circuit design
http://www.clash-lang.org/
Other
31 stars 27 forks source link

TopEntity annotations don't support differential clock sources #33

Closed ggreif closed 8 years ago

ggreif commented 9 years ago

Creating a "wrapper" for the ClockWizard generated IP in Vivado I get

architecture STRUCTURE of design_1 is
  component design_1_clk_wiz_0_0 is
  port (
    clk_in1_p : in STD_LOGIC;
    clk_in1_n : in STD_LOGIC;
    clk_out1 : out STD_LOGIC;
    reset : in STD_LOGIC;
    locked : out STD_LOGIC
  );
  end component design_1_clk_wiz_0_0;

This is not supported by the current TopEntity generator.

ggreif commented 9 years ago

@christiaanb Can you give me a hint how do I wire up this component? Esp. how should I use the clockWizard function to create the annotation? Right now I can edit the generated VHDL, but it won't compile:

ERROR: [Synth 8-2032] formal clk_in1_p is not declared [/home/ggreif/klingon/vhdl/RS232/rs232_tx.vhdl:26]

My hand-edited file contains something like

architecture structural of rs232_tx is
  signal system1000       : std_logic;
  signal clk_wiz_0_locked : std_logic;
  signal system1000_rstn  : std_logic;
  signal input_0          : product0;
begin
  design_1_clk_wiz_0_0 : entity design_1
    port map
      (clk_in1_p => CLOCK(0)
      ,clk_in1_n => CLOCK(1)
      ,CLK_OUT1  => system1000
      ,RESET     => not KEY(0)
      ,LOCKED    => clk_wiz_0_locked);

  -- reset system1000_rstn is asynchronously asserted, but synchronously de-asserted
  resetSync_n_0 : block
    signal n_1 : std_logic;
    signal n_2 : std_logic;
  begin
...

Sorry for asking these nooby VHDL questions...

christiaanb commented 9 years ago

Can you give me the entity declaration of design_1.

ggreif commented 9 years ago

This seems to work:

architecture structural of rs232_tx is
  signal system1000       : std_logic;
  signal clk_wiz_0_locked : std_logic;
  signal system1000_rstn  : std_logic;
  signal input_0          : product0;

  component design_1_clk_wiz_0_0 is
    port (
      clk_in1_p : in STD_LOGIC;
      clk_in1_n : in STD_LOGIC;
      clk_out1 : out STD_LOGIC;
      reset : in STD_LOGIC;
      locked : out STD_LOGIC
    );
  end component design_1_clk_wiz_0_0;

begin
  design_1_inst : design_1_clk_wiz_0_0
    port map
      (clk_in1_p => CLOCK(0)
      ,clk_in1_n => CLOCK(1)
      ,CLK_OUT1  => system1000
      ,RESET     => not KEY(0)
      ,LOCKED    => clk_wiz_0_locked);

  -- reset system1000_rstn is asynchronously asserted, but synchronously de-asserted
...

The entity looks like this

entity design_1 is
  port (
    sys_diff_clock_clk_n : in STD_LOGIC;
    sys_diff_clock_clk_p : in STD_LOGIC
  );
  attribute CORE_GENERATION_INFO : string;
  attribute CORE_GENERATION_INFO of design_1 : entity is "design_1,IP_Integrator,{x_ipVendor=xilinx.com,x_ipLibrary=BlockDiagram,x_ipName=design_1,x_ipVersion=1.00.a,x_ipLanguage=VHDL,numBlks=1,numReposBlks=1,numNonXlnxBlks=0,numHierBlks=0,maxHierDepth=0,synth_mode=Global}";
  attribute HW_HANDOFF : string;
  attribute HW_HANDOFF of design_1 : entity is "design_1.hwdef";
end design_1;
ggreif commented 9 years ago

@christiaanb not sure why the entity only contains inputs, but no outputs...

christiaanb commented 9 years ago

Indeed, I do not really get the purpose of that generated design1, indeed, it seems you should use design_1_clk_wiz_0_0. I never use the Xilinx tools, so I don't really know what files are generated and why.

Anyhow, I'm glad you got it to work eventually.

ggreif commented 9 years ago

@christiaanb It compiled, but design_1_clk_wiz_0_0 is now considered a black box, so bitstream generation doesn't work :-(

ggreif commented 9 years ago

@christiaanb I managed to extend the entity:

entity design_1 is
  port (
    clk_out1 : out STD_LOGIC;
    locked : out STD_LOGIC;
    reset_rtl : in STD_LOGIC;
    sys_diff_clock_clk_n : in STD_LOGIC;
    sys_diff_clock_clk_p : in STD_LOGIC
  );
  attribute CORE_GENERATION_INFO : string;
  attribute CORE_GENERATION_INFO of design_1 : entity is "design_1,IP_Integrator,{x_ipVendor=xilinx.com,x_ipLibrary=BlockDiagram,x_ipName=design_1,x_ipVersion=1.00.a,x_ipLanguage=VHDL,numBlks=1,numReposBlks=1,numNonXlnxBlks=0,numHierBlks=0,maxHierDepth=0,da_board_cnt=1,synth_mode=Global}";
  attribute HW_HANDOFF : string;
  attribute HW_HANDOFF of design_1 : entity is "design_1.hwdef";
end design_1;
ggreif commented 9 years ago

@christiaanb I succeeded generating my first bitstream :-) Please do not act on this issue yet (unless you have good reasons to write clockWizardDifferential), maybe I can pass the bundled differential clock through. Then there would be no need to change anything. Will play around with my setup next week.

ggreif commented 9 years ago

@christiaanb I am working on this. Will need changes in clash-prelude and the clash-compiler packages clash-lib, clash-vhdl & co.

christiaanb commented 9 years ago

I think you will only need changes in clash-lib and clash-prelude. And thanks for working on a patch yourself.

ggreif commented 9 years ago

@christiaanb I have pushed stuff to my two cloned repos. It works as-is. I think I'll need to touch prelude, because I have to add a bit of information to ClockSignal, that it is differential. I need to index into an Identifier (being a BitVector) and correctly print it as VHDL. That's why I need to change the backends.

christiaanb commented 9 years ago

Wouldn't it have been easier to just change the type of the c_inp field from Maybe (String, String) to [(String, String)], and support the multiple clock inputs like that? Then there wouldn't be a need to update the code of the backend.

ggreif commented 9 years ago

@christiaanb that was a very good suggestion. c_inp is a list now. I'll play around with the Vivado tool a bit more to figure out whether the CLK_IN1_D_clk_n nomenclature is standard, and maybe I can eliminate another string input to clockWizardDifferential. I'll defer the corresponding Altera functionality to someone else...

ggreif commented 8 years ago

@christiaanb all should be in place now. I'd appreciate a quick review of my fork, then I'll squash and submit.

christiaanb commented 8 years ago

Looks OK, as long as the PR only includes the changes for clash-lib and clash-prelude, and not the ones to clash-bin and clash-vhdl. Also, if possible, the PR should have an updated https://github.com/clash-lang/clash-compiler/blob/master/stack.yaml pointing to the correct commit of clash-prelude.

ggreif commented 8 years ago

Got my act together: pull request #37 and for clash-compiler i is pull request #100.

ggreif commented 8 years ago

Okay, this is done!