Project

General

Profile

Feature #1851

generalize power control and TA loop code

Added by laforge almost 5 years ago. Updated almost 2 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
osmo-bts-trx
Target version:
Start date:
11/18/2016
Due date:
% Done:

100%

Spec Reference:

Description

omso-bts-trx contains code for a power control and a TA loop. This code is not and should not be osmo-bts-trx specific but should be common code available to all BTS models.


Related issues

Related to OsmoBTS - Bug #2989: OsmoBTS reports ever-growing TA valueClosed02/23/2018

Related to OsmoBTS - Bug #3055: osmo-bts-trx: slow TA loop feedbackClosed03/10/2018

Related to OsmoBTS - Bug #1622: OsmoBTS power control incompliant to TS 05.08 and TS 08.58Resolved02/23/2016

Related to OsmoBTS - Support #4213: ms-power-controlRejected09/23/2019

Related to OsmoBSC - Bug #4244: Take MS power class into account to calculate appropiate MS Power levelResolved10/30/2019

Associated revisions

Revision e5e12263 (diff)
Added by pespin almost 2 years ago

Change gsm_bts_trx field to bool and rename it

Thies field is used to store and retrieve whether MS power needs to be
calculated and updated by osmo-bts software or autonomously by lower
layers. Previous name was not clear
and may have been understood as indicating whether MS Power Control loop
is done or not in general, and the responsible for that is located under
lchan's ms_power_ctrl.fixed.

Related: OS#1851
Change-Id: Ic690ab69866a7377f1597e24aa7b0214831c1cbe

Revision e717aec2 (diff)
Added by pespin almost 2 years ago

Move and rename gsm_lchan.ms_power field

Make it clear that it contains the maximum MS power level (TS 05.05) and
not the one to be used. The one aimed at is in ms_power_ctrl.current.
Since it's used in related code, move it inside the ms_power_ctrl struct
too.

Related: OS#1851
Change-Id: Ib264ec7dac87355cef6415461ed74bd8e9c8ca52

Revision f1967045 (diff)
Added by pespin almost 2 years ago

Drop default vty cmd 'ms-power-control dsp'

That's the default value for all BTS, so no need to have it there.
Furthermore, forthcoming osmo-bts patches will drop osmo-bts-trx support
for DSP/HW based MS Power control, which means BTS will reject this
setting. Let's drop it now and let osmo-bts binary to select the
preferred one.

Related: OS#1851
Change-Id: I0f69880a5028002a53736653735c11ae3cd53f07

Revision 2149b0ff (diff)
Added by pespin almost 2 years ago

power_control.c: Apply latests improvements from loops.c

Several improvements have been made lately to MS Power Control loop from
osmo-bts-trx in loops.c. Let's port these to the common algorithm.

Related: OS#1851
Change-Id: I579967cc8bb69dc76a315c6c9d3a351f5961d92f

Revision d0a2caa0 (diff)
Added by pespin almost 2 years ago

power_control.c: Fix ms pwr ctrl skipped if MS doesn't support announced MS Power Level

Related: OS#1851
Change-Id: I1a9c00fe4eb3fa1eaa7997a9ec20716ddfe180a7

Revision c693067b (diff)
Added by pespin almost 2 years ago

Introduce BTS feature BTS_FEAT_MS_PWR_CTRL_DSP

It indicates whether BTS model supports managing an MS Power Control
Loop over HW/DSP instead of using the software based osmocom algorithm
present in osmo-bts.
osmo-bts-trx own loop implementation is considered to be a "DSP/HW" one
since it acts on lower layers and interferes with osmocom algorithm
since it controls the same end variable "lchan->ms_power_ctrl.current",
this way we make sure both aren't enabled at the same time.
Old behavior in kept: if common upper-layer algo is not enabled
explicitly in VTY (ms-power-control osmo) and bts-trx specific lower
layer algo is neither enabled (osmotrx ms-power-loop <xyz>), then no
power control is done at all.

Related: OS#1851
Change-Id: I49706926b1e962b18791174627bc3cc0cd0cd9d5

Revision 522fc939 (diff)
Added by pespin almost 2 years ago

power_control.c: Don't use announced MS Power level as input for loop calculations

Use instead the received MS Power currently in use by the MS matching
the measured signal. This way there's no need to wait for the MS to
reach the announced MS power level or add checks in case the MS doesn't
support that specific power level. Furthermore, more fine grained
announced power level value can be obtained faster due to more input
iterations not being dropped while waiting.

osmo-bts-trx specific algo was not following this approach and using
announced MS power instead because it's wowrking at a lower level and
henche was not using the transmitted MS Power level value by the MS as
input for the calculation.

The "if (diff < 2 && diff > -2))" condition is dropped since equal
signal strength may still result in a different MS power level announced
(the one currently used by the MS during tx of last SACCH block).

Related: OS#1851
Change-Id: I4494dc27a295a3dca1d3331d4ff712d486643e13

Revision 1d9f6efc (diff)
Added by pespin almost 2 years ago

power_control.c: Limit speed of announced MS Power Level value changes

It's not a good idea to request big changes in MS Power based on
sporadic bad signal received, let's instead change announced MS power
levels more smoothly to avoid possible big signal strength fluctations,
similar to what is already done in osmo-bts-trx specific loop (loops.c).

Related: OS#1851
Change-Id: Iecc4ec7e21471ec853ad2d5659af4052aba5444c

Revision e3a45309 (diff)
Added by pespin almost 2 years ago

bts-trx: Drop low layer MS Power Control Loop algo

Let's drop it instead of having code duplication from common code in a
lower layer, and maintain only the one in l1sap for all BTS models.
As a result, osmo-bts-trx loses feature BTS_FEAT_MS_PWR_CTRL_DSP and
will only be able to use "ms-power-control osmo" in VTY, which will be
enabled by default (meaning: change of behavior, now MS Power Control is
enabled by default in osmo-bts-trx and can only by disabled by BSC).
Old bts-trx specific VTY command "(no) osmotrx ms-power-loop" is marked
as deprecated but still working for more usual case (1 TRX configured)
to avoid breaking backward compatibility.

TA low level loop is still kept in loops.c and will be moved to l1sap at
some point too.

Related: OS#1851
Change-Id: I0d8b0c981d9ead91d93999df6e45fb06e426aeb9

History

#1 Updated by laforge almost 5 years ago

#2 Updated by laforge over 4 years ago

#3 Updated by laforge over 4 years ago

  • Assignee set to sysmocom

#4 Updated by laforge over 3 years ago

  • Related to Bug #2989: OsmoBTS reports ever-growing TA value added

#5 Updated by laforge over 3 years ago

  • Related to Bug #3055: osmo-bts-trx: slow TA loop feedback added

#6 Updated by laforge over 3 years ago

see also: Timing_Advance

#7 Updated by laforge almost 3 years ago

  • Assignee changed from sysmocom to tnt

#8 Updated by laforge almost 3 years ago

  • Related to Bug #1622: OsmoBTS power control incompliant to TS 05.08 and TS 08.58 added

#9 Updated by laforge almost 2 years ago

  • Assignee changed from tnt to pespin

#10 Updated by laforge almost 2 years ago

#11 Updated by pespin almost 2 years ago

There's seems to be some kind of support in "common" dir with some overlapping with loops.c/.h in osmo-bts-trx.

There's src/common/power_ctrl.c implementing lchan_ms_pwr_ctrl(), which looks pretty similar to loops.c trx_loop_sacch_input() + ms_power_diff(). Function lchan_ms_pwr_ctrl() is used in l1sap.c upon receival of a SACCH block, similar to what's done in trx-scheduler.c for the bts-trx API counterparts.

There's no similar API to bts-trx trx_loop_sacch_clock (Called once every downlink SACCH block needs to be sent.) in common; l1sap.c sends directly lchan->ms_power_ctrl.current in l1sap_ph_rts_ind().

There's a common vty cmd "ms-power-control (dsp|osmo)" used by non bts-trx models, with values "dsp -> 0, osmo -> 1". By default it's not set, meaning DSP is in used (and power_ctrl.c does nothing). This function is used around to access whether to use it or not:

int trx_ms_pwr_ctrl_is_osmo(struct gsm_bts_trx *trx)
{
    return trx->ms_power_control == 1;
}

So probably what's needed here:
  • Move loops.c different APIs to common/power_ctrl.c, and call them through l1sap. Check how to glue that though trx_scheduler.c (data should be passed in the primitives?)
  • loops.c APIs need to be cleaned of references to phy_link/instance from osmo-bts-trx, not sure how yet (adding APIs from different models?)
  • l1sap.c's l1sap_ph_rts_ind() probably needs to call trx_loop_sacch_clock() before using lchan->ms_power_ctrl.current.
  • lchan_ms_pwr_ctrl() may need a similar patch to this one done for loops.c: https://gerrit.osmocom.org/c/osmo-bts/+/15877

#12 Updated by laforge almost 2 years ago

On Fri, Oct 25, 2019 at 04:08:34PM +0000, pespin [REDMINE] wrote:

So probably what's needed here:
  • Move loops.c different APIs to common/power_ctrl.c, and call them through l1sap. Check how to glue that though trx_scheduler.c (data should be passed in the primitives?)

ACK.

  • loops.c APIs need to be cleaned of references to phy_link/instance from osmo-bts-trx, not sure how yet (adding APIs from different models?)

The question is what kind of model-specific APIs are needed? There's
nothing really PHY/model specific in power control loops - assuming that
we don't use an automatic mechanism in the DSP of sysmo/oc2g/...

Conceptually we receive uplink measurement values (which are processed
above l1sap anyway) and also receive indication of the acutal L1
transmit power by the MS (in uplink SACCH L1 header), and we send
instructions for the new transmit power level of the MS in the downlink
SACCH L1 header. Nothing bts/phy/backend specific here, AFAICT.

In any case, it seems like currently it's broken for osmo-bts-trx and
our users deserve a very fast fix of the existing code, before we spent
time on the generalization part.

#13 Updated by pespin almost 2 years ago

laforge wrote:

On Fri, Oct 25, 2019 at 04:08:34PM +0000, pespin [REDMINE] wrote:

So probably what's needed here:
  • Move loops.c different APIs to common/power_ctrl.c, and call them through l1sap. Check how to glue that though trx_scheduler.c (data should be passed in the primitives?)

ACK.

  • loops.c APIs need to be cleaned of references to phy_link/instance from osmo-bts-trx, not sure how yet (adding APIs from different models?)

The question is what kind of model-specific APIs are needed? There's
nothing really PHY/model specific in power control loops - assuming that
we don't use an automatic mechanism in the DSP of sysmo/oc2g/...

The only bts-trx specific things I see in loops.c are parameters from the phy_link structure which are in bts-trx specific parts:
phy_link->u.osmotrx.trx_target_rssi (VTY "osmotrx ms-power-loop <-127-127>")
phy_link->u.osmotrx.trx_ms_power_loop (VTY "osmotrx ms-power-loop <-127-127>")
phy_link->u.osmotrx.trx_ta_loop (VTY "osmotrx timing-advance-loop")

So not sure how these parameters apply to bts-trx but not to other BTS?

In any case, it seems like currently it's broken for osmo-bts-trx and
our users deserve a very fast fix of the existing code, before we spent
time on the generalization part.

I'm looking now at what is broken exactly after my patch fixing TC_rsl_ms_pwr_dyn_max, any pointer welcome.

#14 Updated by pespin almost 2 years ago

Some more similarities/differences with what we already have in common and in bts-trx:
  • common has VTY command "uplink-power-target <-110-0>" vs bts-trx has command "osmotrx ms-power-loop <-127-127>". AFAIU the first is in dBm while the second is in dB. So I guess we should have some kind of VTY cmd to set up some kind of calibration data used to convert from dB<->dBm and then use either dB or dBm in a generic algo? (I'm not sure if I'm saying nonsense here, please instruct me if that's the case).

#15 Updated by pespin almost 2 years ago

loops.c seems to be (roughly, partially) implementing some of the ideas from the algo explained in TS 05.08 Annex A.3 "BSS pre-processing and threshold comparisons", like having an array of 32 last values, while the common power_control.c code only uses last value to calculate new power level. Still I'm not sure why the bts-trx specific code gets the lowest value from the array instead of an average or a median from it... that probably makes MS to increase its transmission power over time if there are sporadic bad receivals.

#16 Updated by laforge almost 2 years ago

On Mon, Oct 28, 2019 at 04:02:12PM +0000, pespin [REDMINE] wrote:

  • common has VTY command "uplink-power-target <-110-0>" vs bts-trx has command "osmotrx ms-power-loop <-127-127>".

ACK, this is definitely duplication.

AFAIU the first is in dBm while the second is in dB.

This difference due to the fact that the PHY on non-TRX BTSs is
calibrated and we get absolute dBm values from the PHY.

The unit of osmo-bts-trx specific command must be dBFS, AFAICT. "dB"
always only defines a relative amount of power, think of it as a
"logarithmic percentage". Without giving an indication of what
corresponds to "100%", you cannot convert it to an absolute value.

So I guess we should have some kind of VTY cmd to set up some kind of calibration data used to convert from dB<->dBm and then use either dB or dBm in a generic algo? (I'm not sure if I'm saying nonsense here, please instruct me if that's the case).

The generic part with dBm is fine. Indeed, some kind of calibration
data is needed for the osmo-bts-trx setup. In the SC5 scenario, the TRX
reports calibrated absolute values in the TRXD protocol, so the dBm value
equals the dBFS value.

We could now either define that as "normal" and implement support for
calibration values/table in osmo-trx (and hence make it also report dBm
in TRXD). Or we could add that code to osmo-bts-trx and make it
optional, i.e. assume dBm == dBFS in the absence of calibration data.

In fact, reading trx_if.adoc, the TRXD protocol actually states that the
RSSI values are in dBm, so we can simply make that assumption in
osmo-bts-trx.

#17 Updated by laforge almost 2 years ago

Please note that a lot of processing happens already in the computation of uplink measurement
results. I always assumed that we could simply use those values as input, rather than
computing another set of values?

[...] like having an array of 32 last values, while the common power_control.c code only uses last value to calculate new power level.

Using only the last value is AFAIK what some of the proprietary PHYs we support do, and
it is known to generate oscillations due to the delays involved. At least some kind of simple
filter is required to avoid those oscillations, AFAICT.

Still I'm not sure why the bts-trx specific code gets the lowest value from the array instead of an average or a median from it... that probably makes MS to increase its transmission power over time if there are sporadic bad receivals.

I don't know the details of the algorithm, but you could try to reach out to jolly, the original author. Maybe he remembers his thoughts.

#18 Updated by pespin almost 2 years ago

  • Related to Bug #4244: Take MS power class into account to calculate appropiate MS Power level added

#19 Updated by pespin almost 2 years ago

  • Status changed from New to In Progress
  • % Done changed from 0 to 20

First bunch of commits cleaning up some variables and locations and unifying the VTY parameters used by both current algorithms (common and bts-trx).

#20 Updated by pespin almost 2 years ago

remote: https://gerrit.osmocom.org/c/osmo-bts/+/16058 Unify common and bts-trx power control loop VTY command
remote: https://gerrit.osmocom.org/c/osmo-bts/+/16059 Change gsm_bts_trx field to bool and rename it
remote: https://gerrit.osmocom.org/c/osmo-bts/+/16060 Change gsm_lchan field fixed to bool
remote: https://gerrit.osmocom.org/c/osmo-bts/+/16061 rsl: Remove unneeded duplicate reset on some lchan fields
remote: https://gerrit.osmocom.org/c/osmo-bts/+/16062 Move and rename gsm_lchan.ms_power field

#21 Updated by pespin almost 2 years ago

More related work:

remote: New Changes:
remote: https://gerrit.osmocom.org/c/osmo-bts/+/16068 cosmetic: Fix trailing whitespace
remote: https://gerrit.osmocom.org/c/osmo-bts/+/16069 bts-trx: loops.c: Avoid always clamping MS power to MS power class 1
remote: https://gerrit.osmocom.org/c/osmo-bts/+/16070 Apply latests improvements from loops.c to power_control.c
remote: https://gerrit.osmocom.org/c/osmo-bts/+/16071 Introduce BTS feature BTS_FEAT_MS_PWR_CTRL_DSP
remote: https://gerrit.osmocom.org/c/osmo-bts/+/16072 bts-trx: Drop low layer MS Power Control Loop algo
remote: https://gerrit.osmocom.org/c/osmo-bts/+/16073 power_control.c: Log rx current and target signal levels
remote:
remote: Updated Changes:
remote: https://gerrit.osmocom.org/c/osmo-bts/+/16059 Change gsm_bts_trx field to bool and rename it
remote: https://gerrit.osmocom.org/c/osmo-bts/+/16060 Change gsm_lchan field fixed to bool
remote: https://gerrit.osmocom.org/c/osmo-bts/+/16061 rsl: Remove unneeded duplicate reset on some lchan fields
remote: https://gerrit.osmocom.org/c/osmo-bts/+/16062 Move and rename gsm_lchan.ms_power field

#22 Updated by pespin almost 2 years ago

New changes for osmo-bts:
https://gerrit.osmocom.org/c/osmo-bts/+/16141 power_control.c: Don't use announced MS Power level as input for loop ...
https://gerrit.osmocom.org/c/osmo-bts/+/16142 power_control.c: Limit speed of announced MS Power Level value changes

#23 Updated by pespin almost 2 years ago

  • Status changed from In Progress to Feedback
  • % Done changed from 20 to 90

dexter once comments for https://gerrit.osmocom.org/c/osmo-bts/+/16233 are applied and it gets merged we can close this ticket.

#24 Updated by pespin almost 2 years ago

  • Status changed from Feedback to Resolved
  • % Done changed from 90 to 100

Merged, closing.

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)