MEGA65 / mega65-core

MEGA65 FPGA core
Other
237 stars 84 forks source link

Constraint file needs some love and attention #761

Open MJoergen opened 6 months ago

MJoergen commented 6 months ago

When building the bitstream for R5 (using make bin/mega65r5.bit) I notice in Vivado's synthesis log:

CRITICAL WARNING: [Common 17-165] Too many positional options when parsing 'eth_clock', please type 'create_clock -help' for usage info. [/home/mike/git/MEGA65/mega65-core/src/vhdl/mega65r5.xdc:544]
WARNING: [Vivado 12-627] No clocks matched 'eth_rx_clock'. [/home/mike/git/MEGA65/mega65-core/src/vhdl/mega65r5.xdc:545]

The reason seems to be the use of an incorrect character for the dash/hyphen in the constraint file. The line in the constraint file is

create_clock –name eth_rx_clock –period  20 –waveform  {0 10} [get_ports {eth_clock}]

However, it should instead be:

create_clock -name eth_rx_clock -period  20 -waveform  {0 10} [get_ports {eth_clock}]

Note the subtle change of the glyph used for the dash/hyphen!

When trying this change, I see the critical warning goes away as expected. However, instead I get a large number of timing violations. The biggest timing violation is a Clock Domain Crossing from eth_rx_clock to clock200, where the slack is -9 ns.

I suggest reviewing the various clocks used in the system, and the associated constraints. The clock summary from Vivado shows the following table:

Clock                 Waveform(ns)         Period(ns)      Frequency(MHz)
-----                 ------------         ----------      --------------
CLK_IN                {0.000 5.000}        10.000          100.000
  CLKFBOUT            {0.000 5.000}        10.000          100.000
  CLKOUT0             {0.000 6.736}        13.472          74.227
  CLKOUT1             {0.000 7.222}        14.444          69.231
    clk_fb_adjust1    {0.000 7.222}        14.444          69.231
    u_clock124mhz     {0.000 4.012}        8.025           124.615
      clk_fb_adjust2  {0.000 4.012}        8.025           124.615
      u_clock9969mhz  {0.000 5.015}        10.031          99.692
        clk_fb        {0.000 5.015}        10.031          99.692
        clk_fb_sdram  {0.000 5.015}        10.031          99.692
        clock163      {0.000 3.086}        6.173           162.000
        clock27       {0.000 18.519}       37.037          27.000
        clock270      {0.000 1.852}        3.704           270.000
        clock325      {0.000 1.543}        3.086           324.000
          hr_clk      {0.000 6.173}        12.346          81.000
        clock41       {0.000 12.346}       24.691          40.500
        clock81p      {0.000 6.173}        12.346          81.000
        u_clock163m   {3.086 6.173}        6.173           162.000
  clk_fb_eth          {0.000 5.000}        10.000          100.000
  clko_fb             {0.000 5.000}        10.000          100.000
  clock200            {0.000 2.500}        5.000           200.000
  clock50             {0.000 10.000}       20.000          50.000
  clock60             {0.000 8.333}        16.667          60.000
eth_rx_clock          {0.000 10.000}       20.000          50.000

This shows that the clock eth_rx_clock is independent from all the other clocks. However, that I think is wrong. The clock eth_rx_clock is connected to the top level port eth_clock, which is an output, i.e. generated by the FPGA. It therefore seems more reasonable that eth_rx_clock should be constrained using a create_generated_clock command.

lydon42 commented 6 months ago

Same is true for mega65r4.xdc