Skip to content

Conversation

@Lore0599
Copy link
Contributor

Cluster Adapter CDC

This PR introduces the possibility of enabling or disabling the CDC between the SoC and the cluster domains.
Additionally, when the CDC is enabled, it adjusts the parameter for the input FIFO depth to avoid bandwidth limitations between the two domains.

@Lore0599 Lore0599 requested review from Xeratec and micprog November 17, 2025 18:15
Copy link
Member

@Xeratec Xeratec left a comment

Choose a reason for hiding this comment

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

As far as I can tell, the EnAxiCdc options has no effect on what clock source is used on the cluster. In this case, it is possible that there is no CDC and the cluster is running at a different frequency than the host. Would this be a problem?

@Lore0599
Copy link
Contributor Author

Lore0599 commented Nov 20, 2025

@Xeratec Yes, the parameter is used only to select whether you want the CDC or not. However, the clock signal in the RTL is still different, which means that when building the SoC top level, you must ensure that the cluster clock runs at the same frequency as the SoC clock. If this is not ensured, the system will not work.

@Lore0599 Lore0599 requested a review from fischeti November 20, 2025 14:21
Copy link

@fischeti fischeti left a comment

Choose a reason for hiding this comment

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

LGTM!

@fischeti
Copy link

@Xeratec Yes, the parameter is used only to select whether you want the CDC or not. However, the clock signal in the RTL is still different, which means that when building the SoC top level, you must ensure that the cluster clock runs at the same frequency as the SoC clock. If this is not ensured, the system will not work.

You could also use the EnAxiCdc parameter in the testbench to assign the cluster clock to the soc clock

Copy link
Member

@Xeratec Xeratec left a comment

Choose a reason for hiding this comment

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

I agree with the comment from @fischeti. Additionally, there seems to be an accidental change and I have a question about the newly added TODO comment.

Copy link
Member

Choose a reason for hiding this comment

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

These changes seem unwanted.

~reg2hw.reset_cluster_0.q
};

// TODO: Double check that teh logic AND is sufficient!
Copy link
Member

Choose a reason for hiding this comment

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

What is this TODO about?

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.

3 participants