Implement GPIO signalling for when transmitting/not. #983
Conversation
This feature is separate from the tx_mode which controls whether the tx
stream is started and stopped.
Configuration options:
* Pin to output on (gpio_index)
* Whether to output high or low on transmit (sense)
* How many microseconds to set the tx output early (to turn on external
PAs, switches, etc) (prelude)
* Whether to source from idle or config (source)
* Offset between PPS and real time (pps_time_offset_us)
* Offset in samples between sample time and everything else
(sample_offset)
TDD Config start/stop piped down to radio layer.
In lib/phy/lower/lower_phy_factory.cpp removed the adding of tx_time_offset to
the max delay as this was already handled by the processor. Processor
checks the max delay against it's version of timestamp, processes symbols
based on such timestamp, but then sends the symbols out tx_time_offset
late. Therefore the max delay is being between two timeframes offset by
tx_time_offset, so the tx_time_offset term is already accounted for.
Add sample offset to align RF and GPIO (and PA which uses GPIO). SDR
does not line up it's time for everything else (e.g. GPIO) with the
sample time (i.e. RF). Therefore the transmit time needs to be brought
up by a certain number of samples. Similarly the end needs to be
extended so the SDR doesn't shut off the PA on itself (it is truly
confused about time).
Stitch together start/end from both sources.
Fixups to transmit FSM to keep underflow restart working.
Add configuration for whether to use idle or config for TX and GPIO.
Add PPS time offset parameter to compensate for 101.3 us offset in N310.
There was a problem hiding this comment.
Many thanks for this PR.
My comments are:
- We need to find a way to keep the lower PHY flexible (for supporting future dynamic TDD patterns);
- Wrapping the GPIO code in a different class could increase code clarity; and
- Splitting the PR in two, first UHD changes and second PHY changes could improve the review.
| @@ -58,6 +58,7 @@ class realtime_timing_worker : public controller, public ota_symbol_boundary_not | |||
| std::vector<ota_symbol_boundary_notifier*> ota_notifiers; | |||
| task_executor& executor; | |||
| const subcarrier_spacing scs; | |||
| const cyclic_prefix cp; | |||
There was a problem hiding this comment.
I might miss something. Is this attribute necessary?
There was a problem hiding this comment.
Not sure where this comes from. Assuming it still compiles without this and the similar line in the .cpp then it should be good to kill those two changes.
| /// TDD Configuration | ||
| std::optional<tdd_ul_dl_config_common> tdd_config; |
There was a problem hiding this comment.
The current architecture (without TDD pattern knowledge) allows the lower PHY to support dynamic TDD patterns. I prefer keeping the lower PHY flexibility.
| if(is_tdd_full_dl_slot(cfg, slot_index)) { | ||
| if(symbol_index == get_nsymb_per_slot(cp) - 1) { | ||
| return !has_active_tdd_dl_symbols(cfg, slot_index+1); | ||
| } else { | ||
| return false; | ||
| } | ||
| } else { | ||
| return symbol_index+1 == | ||
| get_active_tdd_dl_symbols(cfg, slot_index, cp).stop(); | ||
| } | ||
|
|
||
| // Impossible |
There was a problem hiding this comment.
| if(is_tdd_full_dl_slot(cfg, slot_index)) { | |
| if(symbol_index == get_nsymb_per_slot(cp) - 1) { | |
| return !has_active_tdd_dl_symbols(cfg, slot_index+1); | |
| } else { | |
| return false; | |
| } | |
| } else { | |
| return symbol_index+1 == | |
| get_active_tdd_dl_symbols(cfg, slot_index, cp).stop(); | |
| } | |
| // Impossible | |
| if(is_tdd_full_dl_slot(cfg, slot_index)) { | |
| if(symbol_index == get_nsymb_per_slot(cp) - 1) { | |
| return !has_active_tdd_dl_symbols(cfg, slot_index+1); | |
| } | |
| return false; | |
| } | |
| return symbol_index+1 == | |
| get_active_tdd_dl_symbols(cfg, slot_index, cp).stop(); |
| } else { | ||
| if(!has_active_tdd_dl_symbols(cfg, slot_index)) { | ||
| return false; | ||
| } else { | ||
| // Previous slot wasn't a full DL slot | ||
| return nof_active_symbols(cfg, slot_index-1, cp, true) | ||
| < nof_slots_per_tdd_period(cfg); | ||
| } | ||
| } | ||
|
|
||
| // Impossible |
There was a problem hiding this comment.
| } else { | |
| if(!has_active_tdd_dl_symbols(cfg, slot_index)) { | |
| return false; | |
| } else { | |
| // Previous slot wasn't a full DL slot | |
| return nof_active_symbols(cfg, slot_index-1, cp, true) | |
| < nof_slots_per_tdd_period(cfg); | |
| } | |
| } | |
| // Impossible | |
| if(!has_active_tdd_dl_symbols(cfg, slot_index)) { | |
| return false; | |
| } | |
| // Previous slot wasn't a full DL slot | |
| return nof_active_symbols(cfg, slot_index-1, cp, true) | |
| < nof_slots_per_tdd_period(cfg); |
|
As far as splitting UHD and PHY changes I want to make sure I understand what you're suggesting. The split I can see is:
Is that in line with what you're suggesting as far as splitting the patch up? |
|
"We need to find a way to keep the lower PHY flexible (for supporting future dynamic TDD patterns);" |
|
The lower PHY tells the radio if there are baseband samples to transmit. When there is no DL, the lower PHY indicates there is no data to transmit and the transmitter can be switched off. What problems have you found using this mechanism? |
|
That indication is per buffer of samples. The radio and frontend need time to switch, so they need to know where the samples actually stop. Technically speaking best case, with the existing setup, there will actually be something like 13 us or similar of overlap where the radio has to transmit and receive at the same time, which of course isn't possible. This also prevents special subframes from having both transmit and receive symbols. Additionally, as it was before the patch there was an offset that caused a portion of one of the symbols to be repeated. If I recall correct this was bleeding the last symbol into the next buffer. This would cause a problem for a special subframe with no transmit, but some receive symbols if the last symbol of the DL is non-empty. Another problem is that there is no apparent way to provide requirements/assertions on the write buffer. If I understand correct, the length of this write buffer is defined elsewhere, meaning that there would be a requirement on the length of the buffer that might not be terribly visible where the length is actually chosen. This means that if, in the future, the processing size changed from a slot to a subframe or vice versa the assumptions of the empty/not-empty flag would also be affected, potentially causing problems for the correctness of the implementation. There are the tx_start and tx_end attributes, but it is unclear whether the processes involved in settings these values are designed to be accurate, or merely to indicate where to clear the memory (it appears their primary purpose is to feed the It is for these reasons that I chose to explicitly provide a copy of the TDD config to the lower layer of the phy in order to allow it to provide the information needed to restrict the period of time that transmission is occurring (i.e. SDR is transmitting and giving TX GPIO indication) to the period where it is possible for transmission to occur. There are actually two modes of operation, one that strictly uses the TDD config timing, and one that uses the tx_start/tx_end indication restricted to the TDD config TX interval. |
This feature is separate from the tx_mode which controls whether the tx stream is started and stopped.
Configuration options:
TDD Config start/stop piped down to radio layer.
In lib/phy/lower/lower_phy_factory.cpp removed the adding of tx_time_offset to
the max delay as this was already handled by the processor. Processor
checks the max delay against it's version of timestamp, processes symbols
based on such timestamp, but then sends the symbols out tx_time_offset
late. Therefore the max delay is being between two timeframes offset by
tx_time_offset, so the tx_time_offset term is already accounted for.
Add sample offset to align RF and GPIO (and PA which uses GPIO). SDR does not line up it's time for everything else (e.g. GPIO) with the sample time (i.e. RF). Therefore the transmit time needs to be brought up by a certain number of samples. Similarly the end needs to be extended so the SDR doesn't shut off the PA on itself (it is truly confused about time).
Stitch together start/end from both sources.
Fixups to transmit FSM to keep underflow restart working.
Add configuration for whether to use idle or config for TX and GPIO.
Add PPS time offset parameter to compensate for 101.3 us offset in N310.