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

ad9912: mention lower f_ref, loop filter phase noise performance #2500

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Spaqin
Copy link
Collaborator

@Spaqin Spaqin commented Jul 18, 2024

ARTIQ Pull Request

Description of Changes

As discussed in #2489, documenting the consequences of using low f_ref and needing to make hardware changes to the loop filter for better performance.

Type of Changes

Type
📜 Docs

Steps (Choose relevant, delete irrelevant before submitting)

All Pull Requests

  • Use correct spelling and grammar.

Code Changes

  • Add and check docstrings and comments

Documentation Changes

  • Check, test, and update the documentation in doc/. Build documentation (cd doc/manual/; make html) to ensure no errors.

Git Logistics

  • Split your contribution into logically separate changes (git rebase --interactive). Merge/squash/fixup commits that just fix or amend previous commits. Remove unintended changes & cleanup. See tutorial.
  • Write short & meaningful commit messages. Review each commit for messages (git show). Format:
    topic: description. < 50 characters total.
    
    Longer description. < 70 characters per line
    

Licensing

See copyright & licensing for more info.
ARTIQ files that do not contain a license header are copyrighted by M-Labs Limited and are licensed under LGPLv3+.

@Spaqin Spaqin requested a review from architeuthidae July 18, 2024 04:57
:param pll_en: PLL enable bit, set to 0 to bypass PLL (default: 1).
Note that when bypassing the PLL the red front panel LED may remain on.

.. note:: For lower than default f_ref, onboard loop filter is not optimal
and may require hardware changes for better phase noise performance.
Copy link
Member

Choose a reason for hiding this comment

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

Is it "for better phase noise performance" or "for staying within specs at all"?

@Spaqin Spaqin force-pushed the ad9912_docs branch 2 times, most recently from 7177204 to 2104f8c Compare July 18, 2024 06:41
:param pll_en: PLL enable bit, set to 0 to bypass PLL (default: 1).
Note that when bypassing the PLL the red front panel LED may remain on.

.. note:: For lower than default f_ref, onboard loop filter is not optimal
Copy link
Member

Choose a reason for hiding this comment

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

Is it the case for all frequencies lower than the default? What is the tolerance?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And now that's something I couldn't get a straightforward answer to after much deliberation with @MorganTL, thus slightly vague wording.

The loop filter parameters on the 9912 Urukul board were chosen for a multiplier of 10, so 100MHz sysclk. Technically, for any different frequency it should be considered; and technically then it's not really designed for 125MHz we provide to the DDS either, but we consider it okay. From what I learned, using different sysclk than designed changes the phase margin and bandwidth. While phase margin is defined that it must be bigger than 45-50 degrees to keep the PLL stable (and for that extreme example of 10MHz mentioned earlier it is), but and there seems to be no exact guidelines on determining bandwidth, which determines output noise. Whole system performance depends on the quality of the provided clock, and whether adjusting the loop filter is necessary also depends on the use cases.

In such case maybe it would be better to just refer the user to the datasheet of AD9912.

Copy link
Collaborator

@architeuthidae architeuthidae left a comment

Choose a reason for hiding this comment

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

This is basically the same thing as solving the merge conflict, but for the record, I was putting f_ref and clk_div into double backticks.

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