Skip to content

Commit

Permalink
CHROMIUM: MALI: platform: Revise get_step_volt()
Browse files Browse the repository at this point in the history
The original logic of getting step voltage works exclusively with
current MT8183 OPP table, but that does not guarantee to work with other
OPP tables, in addition it was confusing to always use step_volt[0] in
calculation (although it works).

Revising the part to make it more compatible to handle general cases.

BUG=b:162191647
TEST=log in guest mode on Krane, repeatedly switching between Chrome and
     home page to trigger GPU frequency transitioning from 3e8 to 8e8 and
     reverse
TEST=run webGL aquarium without issue
TEST=manually modify devfreq values to see freq transitioning
     works, and voltage bias is legit all the time

Change-Id: I88de6a396f91a080ca9bd448a504a62b7651b36c
Signed-off-by: Fei Shao <fshao@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2321889
Reviewed-by: Nicolas Boichat <drinkcat@chromium.org>
  • Loading branch information
Fei Shao authored and Commit Bot committed Jul 31, 2020
1 parent 7a17350 commit 5570ea5
Showing 1 changed file with 45 additions and 71 deletions.
116 changes: 45 additions & 71 deletions drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c
Original file line number Diff line number Diff line change
Expand Up @@ -353,82 +353,49 @@ static void voltage_range_check(struct kbase_device *kbdev,

#ifdef CONFIG_REGULATOR
static bool get_step_volt(unsigned long *step_volt, unsigned long *target_volt,
int count, bool inc)
int regulator_num, bool inc)
{
unsigned long regulator_min_volt;
unsigned long regulator_max_volt;
unsigned long current_bias;
long adjust_step;
int i;

if (inc) {
current_bias = target_volt[1] - step_volt[0];
adjust_step = MIN_VOLT_BIAS;
} else {
current_bias = step_volt[1] - target_volt[0];
adjust_step = -MIN_VOLT_BIAS;
}

for (i = 0; i < count; ++i)
for (i = 0; i < regulator_num; i++)
if (step_volt[i] != target_volt[i])
break;

if (i == count)
return 0;
if (i == regulator_num)
return false;

for (i = 0; i < count; i++) {
if (i) {
regulator_min_volt = VSRAM_GPU_MIN_VOLT;
regulator_max_volt = VSRAM_GPU_MAX_VOLT;
} else {
regulator_min_volt = VGPU_MIN_VOLT;
regulator_max_volt = VGPU_MAX_VOLT;
}

/* This is tricky but step_volt[0] somehow works for MT8183 opp
* table: the code combines voltage_range_check() and voltage
* adjustment together so the voltage transition is always safe.
*
* Note that step_volt[1] (VSRAM) is passively adjusted along
* with step_volt[0] (VGPU) i.e. to maintain minimum voltage
* bias, step_volt[1] is only increased when step_volt[0]
* aggressively increases.
*
* Consider an extreme case of freq 300000000 -> 800000000
* in MT8183 opp table:
* step_volt[0] 625000 -> 825000, step_volt[1] 850000 -> 925000
* As the bias can be too large during the transition
* (925000 - 625000 > 250000), step_volt[0] and step_volt[1]
* will be both set to 625000 + 100000, but soon step_volt[1]
* will be clamped into its valid range, resulting the first
* step to be:
* step_volt[0] 625000 -> 725000, step_volt[1] 850000 -> 850000
*
* And then the second step completes the full transition later:
* step_volt[0] 725000 -> 825000, step_volt[1] 850000 -> 925000
*
* Similar logic works for the reversed case of freq 800000000
* -> 300000000.
*
* The logic above may not apply to other opp tables and
* refinement may be needed by the time.
*/
if (current_bias > MAX_VOLT_BIAS) {
step_volt[i] = clamp_val(step_volt[0] + adjust_step,
regulator_min_volt,
regulator_max_volt);
} else {
step_volt[i] = target_volt[i];
}
/* Do one round of *caterpillar move* - shrink the tail as much to the
* head as possible, and then step ahead as far as possible.
* Depending on the direction of voltage transition, a reversed
* sequence of extend-and-shrink may apply, which leads to the same
* result in the end.
*/
if (inc) {
step_volt[0] = min(target_volt[0],
step_volt[1] - MIN_VOLT_BIAS);
step_volt[1] = min(target_volt[1],
step_volt[0] + MAX_VOLT_BIAS);
} else {
step_volt[0] = max(target_volt[0],
step_volt[1] - MAX_VOLT_BIAS);
step_volt[1] = max(target_volt[1],
step_volt[0] + MIN_VOLT_BIAS);
}
return 1;
return true;
}

static int set_voltages(struct kbase_device *kbdev, unsigned long *voltages,
bool inc)
{
unsigned long step_volt[KBASE_MAX_REGULATORS];
int first, step;
const unsigned long reg_min_volt[KBASE_MAX_REGULATORS] = {
VGPU_MIN_VOLT,
VSRAM_GPU_MIN_VOLT,
};
const unsigned long reg_max_volt[KBASE_MAX_REGULATORS] = {
VGPU_MAX_VOLT,
VSRAM_GPU_MAX_VOLT,
};
int i, err;

/* Nothing to do if the direction of voltage transition is incorrect. */
Expand All @@ -439,19 +406,26 @@ static int set_voltages(struct kbase_device *kbdev, unsigned long *voltages,
for (i = 0; i < kbdev->regulator_num; ++i)
step_volt[i] = kbdev->current_voltage[i];

if (inc) {
first = kbdev->regulator_num - 1;
step = -1;
} else {
first = 0;
step = 1;
}

while (get_step_volt(step_volt, voltages, kbdev->regulator_num, inc)) {
for (i = first; i >= 0 && i < kbdev->regulator_num; i += step) {
for (i = 0; i < kbdev->regulator_num; i++) {
if (kbdev->current_voltage[i] == step_volt[i])
continue;

/* Assuming valid max voltages are always positive. */
if (reg_max_volt[i] > 0 &&
(step_volt[i] < reg_min_volt[i] ||
step_volt[i] > reg_max_volt[i])) {
dev_warn(kbdev->dev, "Clamp invalid voltage: "
"%lu of regulator %d into [%lu, %lu]",
step_volt[i], i,
reg_min_volt[i],
reg_max_volt[i]);

step_volt[i] = clamp_val(step_volt[i],
reg_min_volt[i],
reg_max_volt[i]);
}

err = regulator_set_voltage(kbdev->regulator[i],
step_volt[i],
step_volt[i] + VOLT_TOL);
Expand Down

0 comments on commit 5570ea5

Please sign in to comment.