Skip to content

Conversation

@CyrilKoe
Copy link
Contributor

@CyrilKoe CyrilKoe commented Jul 4, 2023

The goal is to disable the default instantiation of the BUFGMUX, as it consumes lots of clock resources when it is not actually needed. It can also create long BUF chains that can fail DRCs.
This can often fail Vivado implementations.
When buffering is needed, who's instantiating the IP has to explicitly set it.

@luca-valente
Copy link
Contributor

@CyrilKoe We need to add the parameter also in the RTL target file so that it is actually possible to use it. Here as for the clock gating cell.

Copy link
Collaborator

@niwis niwis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CyrilKoe I'm not sure if IS_FUNCTIONAL makes sense here. A multiplexer is always functional. Why is the tc_clk_mux used for non-clock signals?

/// Using BUFGMUX on FPGA can allocate limited clock ressources
/// to non clock signals. It can be disabled with
/// IS_FUNCTIONAL = 0
parameter bit IS_FUNCTIONAL = 1'b0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default needs to remain 1'b1.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@niwis the goal is to disable the default instantiation of the BUFGMUX, as it consumes lots of clock resources when it is not actually needed. When needed, who's instantiating the IP has to explicitly set it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can find a different name for the parameter 😄

Copy link
Contributor Author

@CyrilKoe CyrilKoe Jul 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EN_BUF_FPGA ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand from the comment that the BUFGMUX is not wanted if you multiplex a non-clock signal. My question is: why do you use tc_clk_mux for these non-clock signals in the first place? Maybe the place to solve this would be one hierarchy level above (by not instantiating this module). But I might be missing something; happy to discuss this offline.

Copy link
Contributor

@luca-valente luca-valente Jul 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of "legacy" code using the tc_clk_mux... let's discuss it offline 😄

@CyrilKoe CyrilKoe force-pushed the fix/fpga_clk_mux branch from 6f43739 to 87b59bf Compare July 6, 2023 08:59
@CyrilKoe CyrilKoe force-pushed the fix/fpga_clk_mux branch from 87b59bf to 360fe59 Compare July 6, 2023 09:15
@CyrilKoe
Copy link
Contributor Author

CyrilKoe commented Jul 6, 2023

@niwis @luca-valente
Canged now, if you agree with using this parameter I will start a Carfield CI with the change and I will comment here if it passed.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants