search

analogdevicesinc / hdl

HDL libraries and projects
https://wiki.analog.com/resources/fpga/docs/hdl
Other
1.5k stars 1.51k forks source link

[BUG] library: common: ad_dds: Incorrect Calculation of Phase Increment #1408

Open ekigwana opened 2 months ago

ekigwana commented 2 months ago

Describe the bug The formula to calculate the phase increment for the DDS is incorrect.

To Reproduce The formula https://github.com/analogdevicesinc/hdl/blob/main/docs/library/common/ad_dds/index.rst?plain=1#L191 should be:

INCR = \frac{f{out} * (2^{phaseDW} - 1)}{f{if} * clkratio}

For a frequency output of 1 Hz it is clear that 1 * ^{phaseDW} will overflow! With clock ratio multiplied in the numerator in the case of a clock ratio of two The output frequency of the DDS quadruples. So instead of getting 12.5 kHz out, the output is 50 kHz. With clock ratio multiplied by the interface frequency in the denominator the frequency is always what is expected in the simulation. The only difference in that there are clock ratio consecutive samples on every clock.

I wondered what the Linux kernel driver is doing and checked here https://github.com/analogdevicesinc/linux/blob/main/drivers/iio/frequency/cf_axi_dds.c#L886 and it appears to follow the spirit of what I mentioned above. However, using iio-oscope connected to an adrv9009-zu11eg SDR I set the frequency of TX1 to 39.999985 MHz and a DDS Mode of One CW Tone

Steps to reproduce the behavior:

  1. Using iio-oscope connect to an adrv9009-zu11eg SDR or any SDR using multiple JESD lanes for TX
  2. Set the frequency of TX1 to 39.999985 MHz on the ADRV9009 tab
  3. Go to the debug tab by clicking on it
  4. Select the 'axi-adrv9009-tx-hpc' device under Device Selection combo box
  5. Set the Source to 'AXI_CORE' under Register Map Settings
  6. Set the read address byte to 0x0404 according to https://wiki.analog.com/resources/fpga/docs/axi_dac_ip
  7. Click read and observe results
  8. DDS_INC_1[15:0] indicates 21333

Expected behavior I expect to see: DDS_INC_1[15:0] = (39.999985 MHz (2^16-1)/(122.88 MHz 2) = int(10 666.4999)

The value read back is is consistent with ignoring the clock ratio altogether DDS_INC_1[15:0] = (39.999985 MHz * (2^16-1)/(122.88 MHz) = int(21 332.9998) The kernel folks need to account for the clock ratio here otherwise the frequency is incorrect

The value that the documentation says it should be is: DDS_INC_1[15:0] = (39.999985 MHz 2^16) 2)/(122.88 MHz) = int(42 666.6507)

Desktop (please complete the following information):

AndreiGrozav commented 1 month ago

Hi.

Sorry for the delayed reply. The formula is correct, I guess you misinterpreted it because is not rendered. The frequency ration is a multiplying factor of the equation, it is not part of the divisor. Have a look at https://wiki.analog.com/resources/fpga/docs/dds#frequency_-_dds_incr

Andrei

ekigwana commented 1 month ago

Hi.

Sorry for the delayed reply. The formula is correct, I guess you misinterpreted it because is not rendered. The frequency ration is a multiplying factor of the equation, it is not part of the divisor. Have a look at https://wiki.analog.com/resources/fpga/docs/dds#frequency_-_dds_incr

Andrei

If that formula is correct, how come the register value does not match on the development board and yet the output frequency is correct?