EttusResearch / uhd

The USRP™ Hardware Driver Repository
http://uhd.ettus.com
Other
975 stars 655 forks source link

fpga/usrp3: Fix DC truncation bias for B2xx by adding rounding to DDC chain #703

Closed ryanvolz closed 4 months ago

ryanvolz commented 1 year ago

Pull Request Details

Description

Currently, the non-RFNOC usrp3 (i.e. B2xx) DDC code truncates to 18 bits from 24 bits after applying the halfband filters and before applying a scaling and final rounding. This truncation introduces a DC bias that is noticeable with small signal levels (or lots of integration), especially with high decimation rates. Changing the truncation to a rounding removes the DC bias.

This is essentially a forward port of a similar prior fix to the usrp2 DDC chain: https://github.com/EttusResearch/fpga/pull/4. (Note: In order to not repeat the mistake of finding a similar issue in another part of the codebase ~8 years later =P, I checked the RFNOC usrp3's DDC to see what it does. Thankfully, it doesn't have this problem since it uses a wider multiplier and thus avoids the truncation.)

Related Issue

Which devices/areas does this affect?

USRP3-based non-RFNOC devices, i.e. B2xx and B2xxmini

Testing Done

I tested this with a B200mini using GNU Radio. Without the radio connected to anything, I tuned with an LO offset of 1 MHz, gain of 0, and high decimation factor (master clock of 40e6, sample rate of 200e3). Using the stock fpga, a DC bias is present: b200mini_fpga_stock

Using this modification, the DC bias is gone: b200mini_fpga_with_rounding

Checklist

github-actions[bot] commented 1 year ago

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

ryanvolz commented 1 year ago

I have read the CLA Document and I hereby sign the CLA

ryanvolz commented 1 year ago

recheck

mbr0wn commented 9 months ago

@ryanvolz FYI we're reviewing this internally. I think this change is OK.

mbr0wn commented 9 months ago

(Note: In order to not repeat the mistake of finding a similar issue in another part of the codebase ~8 years later =P, I checked the RFNOC usrp3's DDC to see what it does. Thankfully, it doesn't have this problem since it uses a wider multiplier and thus avoids the truncation.)

Oh, and we very much appreciate this ^^^ :+1: