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

New: ddrForwardClock #2876

Merged
merged 1 commit into from
Jan 20, 2025
Merged

New: ddrForwardClock #2876

merged 1 commit into from
Jan 20, 2025

Conversation

DigitalBrains1
Copy link
Member

@DigitalBrains1 DigitalBrains1 commented Jan 16, 2025

This PR was inspired by the clash-cores PR #7 Add RGMII core which uses this construction but keeps the output clock as a Signal domDDR Bit. With the new ddrOutClock, we can correctly declare the thing to be a clock.

There's a qualitative difference between Clock and Signal _ Bit. In a nutshell, a signal is relative to a clock, it has setup and hold constraints and carries data. All of these don't apply to a clock. You could say a clock leads and a signal follows. The resulting output on the pin is no different, it's just about correctly declaring what something is in the Haskell world.

The new ddrForwardClock function gets a DDR output primitive as an argument. If you look at clash-prelude, the only directly fitting one is Clash.Explicit.DDR.ddrOut (if you apply the reset value first). However, in a PR that I will create soon, the API for Clash.Intel.DDR and Clash.Xilinx.DDR is changed to accomodate the signature introduced in this PR. Even with the current code, it's possible to adapt the vendor primitives to fit ddrForwardClock, but soon it will be a natural fit.

Still TODO:

  • Write a changelog entry (see changelog/README.md)
  • Check copyright notices are up to date in edited files

@DigitalBrains1
Copy link
Member Author

GHC 8.10.7 should get its priorities straight:

    • Expected kind ‘Nat’,
        but ‘DomainPeriod domIn ~ 2’ has kind ‘Constraint’
    • In the first argument of ‘(*)’, namely ‘DomainPeriod domIn ~ 2’
      In the type signature:
[...]
                        DomainPeriod domIn ~ 2 * DomainPeriod domDdr) =>
[...]

Why is this same expression no longer a problem in clash-cores PR #7 Add RGMII core? I couldn't get it to work here. I did notice that code had fewer KnownDomains than I expected; if you look at our docs, a use of DomainPeriod is documented as needing KnownDomain in its example, but apparently you don't?

I'll remove some KnownDomains and add parentheses to the expression.

@DigitalBrains1 DigitalBrains1 force-pushed the ddrclock branch 2 times, most recently from 3c1524c to d3b7b76 Compare January 16, 2025 16:55
@rowanG077
Copy link
Member

Yes this is very nice! Thank you!

@rowanG077
Copy link
Member

rowanG077 commented Jan 16, 2025

Perhaps ddrForwardClock is a more appropriate name as this technique is generally referred to a clock forwarding, or at least that's what I know it as and is googleable.

@DigitalBrains1 DigitalBrains1 changed the title New: ddrOutClock New: ddrForwardClock Jan 20, 2025
@DigitalBrains1
Copy link
Member Author

I've changed the name from ddrOutClock to ddrForwardClock.

@DigitalBrains1 DigitalBrains1 enabled auto-merge (squash) January 20, 2025 11:25
@DigitalBrains1 DigitalBrains1 merged commit d9e2a53 into master Jan 20, 2025
13 checks passed
@DigitalBrains1 DigitalBrains1 deleted the ddrclock branch January 20, 2025 21:45
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.

2 participants