Skip to content

Use FFT-shifted buffer in all PHY receiver functions#240

Open
sakthivelvelumani wants to merge 2 commits into
duranta-project:developfrom
sakthivelvelumani:gnb-rxdataf-buf
Open

Use FFT-shifted buffer in all PHY receiver functions#240
sakthivelvelumani wants to merge 2 commits into
duranta-project:developfrom
sakthivelvelumani:gnb-rxdataf-buf

Conversation

@sakthivelvelumani

Copy link
Copy Markdown
Collaborator

Similar to the transmit path, this PR changes rxdataF to hold FFT shifted data. So all PRBs are contiguous starting from PRB 0.

@sakthivelvelumani sakthivelvelumani added 5G-NR Perform 5G Tests gNB and removed 5G-NR Perform 5G Tests labels Jun 26, 2026
@durantabot

Copy link
Copy Markdown
Collaborator

CI Build: #509 | Failed on the following stages:

@sgarg00 sgarg00 added BUILD-ONLY Execute build stages only for code improvements with no impact on 4G/5G functionality. retrigger-ci Re-run CI and removed retrigger-ci Re-run CI labels Jun 27, 2026
@github-actions github-actions Bot removed the retrigger-ci Re-run CI label Jun 27, 2026
@sgarg00

sgarg00 commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

@sakthivelvelumani sakthivelvelumani added retrigger-ci Re-run CI and removed BUILD-ONLY Execute build stages only for code improvements with no impact on 4G/5G functionality. labels Jun 28, 2026
@github-actions github-actions Bot removed the retrigger-ci Re-run CI label Jun 28, 2026
Comment thread openair1/PHY/MODULATION/ofdm_mod.c Outdated
{
const int half = nbins / 2; // half bw

c16_t tmp[fft_size / 2]; // holds the negative half

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a reason why this array does not have the size half? If so, I think it should be documented, because it is not obvious...

Suggested change
c16_t tmp[fft_size / 2]; // holds the negative half
c16_t tmp[half]; // holds the negative half

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I missed it. thanks.

Comment thread openair1/PHY/MODULATION/ofdm_mod.c Outdated
{
const int half = nbins / 2; // # negative-freq bins

c16_t tmp[fft_size / 2]; // holds the negative half

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same here...

Comment thread openair1/PHY/NR_TRANSPORT/srs_rx.c Outdated
Comment on lines 105 to 107

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

More wrap-around logic

Comment thread openair1/PHY/NR_TRANSPORT/srs_rx.c Outdated
Comment on lines 113 to 115

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wrap-around logic

Comment thread radio/fhi_72/oaioran.c Outdated
Comment on lines +736 to +740
// The purpose of following if is not clear? So adding assert in else to detect missed reads.
if ((startRB + numRB) == (start_totalRB + num_totalRB)) {
int pos_len = 0;
int neg_len = 0;

if (start_totalRB < (num_totalRB >> 1)) // there are PRBs left of DC
neg_len = min((num_totalRB * 6) - (start_totalRB * 12), num_totalRB * N_SC_PER_PRB);
pos_len = (num_totalRB * N_SC_PER_PRB) - neg_len;
// Calculation of the pointer for the section in the buffer.
// positive half
uint8_t *dst1 = (uint8_t *)(pos + (neg_len == 0 ? ((start_totalRB * N_SC_PER_PRB) - (num_totalRB * 6)) : 0));
// negative half
uint8_t *dst2 = (uint8_t *)(pos + (start_totalRB * N_SC_PER_PRB) + fftsize - (num_totalRB * 6));
memcpy((void *)dst2, (void *)local_dst, neg_len * 4);
memcpy((void *)dst1, (void *)&local_dst[neg_len], pos_len * 4);
memcpy((void *)pos, (void *)local_dst, num_totalRB * N_SC_PER_PRB * 4);
} else {
DevAssert(0);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You could avoid this strange part of the code if you changed the code above to write straight into the destination buffer.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks. I removed the temporary buffer.

make fft shift functions more generic. separate functions to do fft
shift inplace and inverse fft shift.

Signed-off-by: Sakthivel Velumani <s.velumani@northeastern.edu>
Same as txdataF, now rxdataF buffer is flat with first RB starting at index 0.

Signed-off-by: Sakthivel Velumani <s.velumani@northeastern.edu>
@RaymondKnopp

Copy link
Copy Markdown
Collaborator

In preparation for the full CAT-B support may I suggest that the final fftshift operation (both TX and RX) actually happen in the RU component. For 7.2 we actually undo the fftshift during compression/decompression and we don't actually need it at all. We can put the fftshift in the low-phy variants which are not 7.2. When we will add multiple layer support and multiple sections this will be required anyway since we are not going to transfer a full OFDM symbol between high-PHY and low-PHY.

@sakthivelvelumani

Copy link
Copy Markdown
Collaborator Author

@RaymondKnopp This PR does fftshift only for non 7.2 scenario and it does immediately after FFT operation inside nr_ofdm_demod_and_rx_rotation(). For 7.2, the PRBs are read from xran (which are shifted) and the high-phy processes it as is.

Speaking of multiple section, in #157 I made some preparation work to pass multiple section information (no of prbs, symbols, beam id) from high-phy to low-phy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants