| Message ID | 20240409094632.62916-7-diogo.ivo@tecnico.ulisboa.pt |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Cleanup Tegra210 EMC frequency scaling | expand |
On 09/04/2024 11:46, Diogo Ivo wrote: > Separate the comparison/updating of the measured delay values with the > values currently programmed into a separate function to simplify the > code. > > Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt> > --- > drivers/memory/tegra/tegra210-emc-cc-r21021.c | 84 +++++++++---------- > 1 file changed, 38 insertions(+), 46 deletions(-) > > diff --git a/drivers/memory/tegra/tegra210-emc-cc-r21021.c b/drivers/memory/tegra/tegra210-emc-cc-r21021.c > index 566e5c65c854..ec2f84758d55 100644 > --- a/drivers/memory/tegra/tegra210-emc-cc-r21021.c > +++ b/drivers/memory/tegra/tegra210-emc-cc-r21021.c > @@ -113,19 +113,35 @@ enum { > #define __MOVAVG(timing, dev) \ > ((timing)->ptfv_list[dev]) > > +static bool tegra210_emc_compare_update_delay(struct tegra210_emc_timing *timing, > + u32 measured, u32 idx) > +{ > + u32 *curr = &timing->current_dram_clktree[idx]; > + u32 rate_mhz = timing->rate / 1000; > + u32 tmdel; > + > + tmdel = abs(*curr - measured); > + > + if (tmdel * 128 * rate_mhz / 1000000 > timing->tree_margin) { > + *curr = measured; > + return true; > + } > + > + return false; > +} > + > static u32 update_clock_tree_delay(struct tegra210_emc *emc, int type) > { > bool periodic_training_update = type == PERIODIC_TRAINING_UPDATE; > struct tegra210_emc_timing *last = emc->last; > struct tegra210_emc_timing *next = emc->next; > u32 last_timing_rate_mhz = last->rate / 1000; > - u32 next_timing_rate_mhz = next->rate / 1000; > bool dvfs_update = type == DVFS_UPDATE; > - s32 tdel = 0, tmdel = 0, adel = 0; > bool dvfs_pt1 = type == DVFS_PT1; > u32 temp[2][2], value, udelay; > unsigned long cval = 0; > unsigned int c, d, idx; > + bool over = false; > > if (dvfs_pt1 || periodic_training_update) { > udelay = tegra210_emc_actual_osc_clocks(last->run_clocks); > @@ -174,17 +190,9 @@ static u32 update_clock_tree_delay(struct tegra210_emc *emc, int type) > else if (periodic_training_update) > __WEIGHTED_UPDATE_PTFV(idx, cval); > > - if (dvfs_update || periodic_training_update) { > - tdel = next->current_dram_clktree[idx] - > - __MOVAVG_AC(next, idx); > - tmdel = (tdel < 0) ? -1 * tdel : tdel; > - adel = tmdel; > - > - if (tmdel * 128 * next_timing_rate_mhz / 1000000 > > - next->tree_margin) > - next->current_dram_clktree[idx] = > - __MOVAVG_AC(next, idx); > - } > + if (dvfs_update || periodic_training_update) > + over |= tegra210_emc_compare_update_delay(next, > + __MOVAVG_AC(next, idx), idx); > > /* C[c]D[d]U[1] */ > idx++; > @@ -202,35 +210,26 @@ static u32 update_clock_tree_delay(struct tegra210_emc *emc, int type) > else if (periodic_training_update) > __WEIGHTED_UPDATE_PTFV(idx, cval); > > - if (dvfs_update || periodic_training_update) { > - tdel = next->current_dram_clktree[idx] - > - __MOVAVG_AC(next, idx); > - tmdel = (tdel < 0) ? -1 * tdel : tdel; > - > - if (tmdel > adel) > - adel = tmdel; > - > - if (tmdel * 128 * next_timing_rate_mhz / 1000000 > > - next->tree_margin) > - next->current_dram_clktree[idx] = > - __MOVAVG_AC(next, idx); > - } > + if (dvfs_update || periodic_training_update) > + over |= tegra210_emc_compare_update_delay(next, > + __MOVAVG_AC(next, idx), idx); > } > } > > - return adel; > + return over; You are now returning always 0 or 1, while previously it was tmdel, which I suppose is not 0/1. This looks odd, especially that function prototype did not change. Best regards, Krzysztof
On Sat, Apr 13, 2024 at 10:07:44AM +0200, Krzysztof Kozlowski wrote: > On 09/04/2024 11:46, Diogo Ivo wrote: > > Separate the comparison/updating of the measured delay values with the > > values currently programmed into a separate function to simplify the > > code. > > > > Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt> > > --- > > drivers/memory/tegra/tegra210-emc-cc-r21021.c | 84 +++++++++---------- > > 1 file changed, 38 insertions(+), 46 deletions(-) > > > > diff --git a/drivers/memory/tegra/tegra210-emc-cc-r21021.c b/drivers/memory/tegra/tegra210-emc-cc-r21021.c > > index 566e5c65c854..ec2f84758d55 100644 > > --- a/drivers/memory/tegra/tegra210-emc-cc-r21021.c > > +++ b/drivers/memory/tegra/tegra210-emc-cc-r21021.c > > @@ -113,19 +113,35 @@ enum { > > #define __MOVAVG(timing, dev) \ > > ((timing)->ptfv_list[dev]) > > > > +static bool tegra210_emc_compare_update_delay(struct tegra210_emc_timing *timing, > > + u32 measured, u32 idx) > > +{ > > + u32 *curr = &timing->current_dram_clktree[idx]; > > + u32 rate_mhz = timing->rate / 1000; > > + u32 tmdel; > > + > > + tmdel = abs(*curr - measured); > > + > > + if (tmdel * 128 * rate_mhz / 1000000 > timing->tree_margin) { > > + *curr = measured; > > + return true; > > + } > > + > > + return false; > > +} > > + > > static u32 update_clock_tree_delay(struct tegra210_emc *emc, int type) > > { > > bool periodic_training_update = type == PERIODIC_TRAINING_UPDATE; > > struct tegra210_emc_timing *last = emc->last; > > struct tegra210_emc_timing *next = emc->next; > > u32 last_timing_rate_mhz = last->rate / 1000; > > - u32 next_timing_rate_mhz = next->rate / 1000; > > bool dvfs_update = type == DVFS_UPDATE; > > - s32 tdel = 0, tmdel = 0, adel = 0; > > bool dvfs_pt1 = type == DVFS_PT1; > > u32 temp[2][2], value, udelay; > > unsigned long cval = 0; > > unsigned int c, d, idx; > > + bool over = false; > > > > if (dvfs_pt1 || periodic_training_update) { > > udelay = tegra210_emc_actual_osc_clocks(last->run_clocks); > > @@ -174,17 +190,9 @@ static u32 update_clock_tree_delay(struct tegra210_emc *emc, int type) > > else if (periodic_training_update) > > __WEIGHTED_UPDATE_PTFV(idx, cval); > > > > - if (dvfs_update || periodic_training_update) { > > - tdel = next->current_dram_clktree[idx] - > > - __MOVAVG_AC(next, idx); > > - tmdel = (tdel < 0) ? -1 * tdel : tdel; > > - adel = tmdel; > > - > > - if (tmdel * 128 * next_timing_rate_mhz / 1000000 > > > - next->tree_margin) > > - next->current_dram_clktree[idx] = > > - __MOVAVG_AC(next, idx); > > - } > > + if (dvfs_update || periodic_training_update) > > + over |= tegra210_emc_compare_update_delay(next, > > + __MOVAVG_AC(next, idx), idx); > > > > /* C[c]D[d]U[1] */ > > idx++; > > @@ -202,35 +210,26 @@ static u32 update_clock_tree_delay(struct tegra210_emc *emc, int type) > > else if (periodic_training_update) > > __WEIGHTED_UPDATE_PTFV(idx, cval); > > > > - if (dvfs_update || periodic_training_update) { > > - tdel = next->current_dram_clktree[idx] - > > - __MOVAVG_AC(next, idx); > > - tmdel = (tdel < 0) ? -1 * tdel : tdel; > > - > > - if (tmdel > adel) > > - adel = tmdel; > > - > > - if (tmdel * 128 * next_timing_rate_mhz / 1000000 > > > - next->tree_margin) > > - next->current_dram_clktree[idx] = > > - __MOVAVG_AC(next, idx); > > - } > > + if (dvfs_update || periodic_training_update) > > + over |= tegra210_emc_compare_update_delay(next, > > + __MOVAVG_AC(next, idx), idx); > > } > > } > > > > - return adel; > > + return over; > > You are now returning always 0 or 1, while previously it was tmdel, > which I suppose is not 0/1. > > This looks odd, especially that function prototype did not change. I completely forgot to change the return type of update_clock_tree_delay(), it should now be bool instead of u32. Before, tmdel was the largest difference between the current measurements of delay values and the values that are programmed in the hardware. All the callers of this function were taking this tmdel return value and repeating the calculation that I moved to tegra210_emc_compare_update_delay(), so this return value now simply reflects if the largest measured delay is over the set margin or not to avoid this repetition. Best regards, Diogo
diff --git a/drivers/memory/tegra/tegra210-emc-cc-r21021.c b/drivers/memory/tegra/tegra210-emc-cc-r21021.c index 566e5c65c854..ec2f84758d55 100644 --- a/drivers/memory/tegra/tegra210-emc-cc-r21021.c +++ b/drivers/memory/tegra/tegra210-emc-cc-r21021.c @@ -113,19 +113,35 @@ enum { #define __MOVAVG(timing, dev) \ ((timing)->ptfv_list[dev]) +static bool tegra210_emc_compare_update_delay(struct tegra210_emc_timing *timing, + u32 measured, u32 idx) +{ + u32 *curr = &timing->current_dram_clktree[idx]; + u32 rate_mhz = timing->rate / 1000; + u32 tmdel; + + tmdel = abs(*curr - measured); + + if (tmdel * 128 * rate_mhz / 1000000 > timing->tree_margin) { + *curr = measured; + return true; + } + + return false; +} + static u32 update_clock_tree_delay(struct tegra210_emc *emc, int type) { bool periodic_training_update = type == PERIODIC_TRAINING_UPDATE; struct tegra210_emc_timing *last = emc->last; struct tegra210_emc_timing *next = emc->next; u32 last_timing_rate_mhz = last->rate / 1000; - u32 next_timing_rate_mhz = next->rate / 1000; bool dvfs_update = type == DVFS_UPDATE; - s32 tdel = 0, tmdel = 0, adel = 0; bool dvfs_pt1 = type == DVFS_PT1; u32 temp[2][2], value, udelay; unsigned long cval = 0; unsigned int c, d, idx; + bool over = false; if (dvfs_pt1 || periodic_training_update) { udelay = tegra210_emc_actual_osc_clocks(last->run_clocks); @@ -174,17 +190,9 @@ static u32 update_clock_tree_delay(struct tegra210_emc *emc, int type) else if (periodic_training_update) __WEIGHTED_UPDATE_PTFV(idx, cval); - if (dvfs_update || periodic_training_update) { - tdel = next->current_dram_clktree[idx] - - __MOVAVG_AC(next, idx); - tmdel = (tdel < 0) ? -1 * tdel : tdel; - adel = tmdel; - - if (tmdel * 128 * next_timing_rate_mhz / 1000000 > - next->tree_margin) - next->current_dram_clktree[idx] = - __MOVAVG_AC(next, idx); - } + if (dvfs_update || periodic_training_update) + over |= tegra210_emc_compare_update_delay(next, + __MOVAVG_AC(next, idx), idx); /* C[c]D[d]U[1] */ idx++; @@ -202,35 +210,26 @@ static u32 update_clock_tree_delay(struct tegra210_emc *emc, int type) else if (periodic_training_update) __WEIGHTED_UPDATE_PTFV(idx, cval); - if (dvfs_update || periodic_training_update) { - tdel = next->current_dram_clktree[idx] - - __MOVAVG_AC(next, idx); - tmdel = (tdel < 0) ? -1 * tdel : tdel; - - if (tmdel > adel) - adel = tmdel; - - if (tmdel * 128 * next_timing_rate_mhz / 1000000 > - next->tree_margin) - next->current_dram_clktree[idx] = - __MOVAVG_AC(next, idx); - } + if (dvfs_update || periodic_training_update) + over |= tegra210_emc_compare_update_delay(next, + __MOVAVG_AC(next, idx), idx); } } - return adel; + return over; } -static u32 periodic_compensation_handler(struct tegra210_emc *emc, u32 type, - struct tegra210_emc_timing *last, - struct tegra210_emc_timing *next) +static bool periodic_compensation_handler(struct tegra210_emc *emc, u32 type, + struct tegra210_emc_timing *last, + struct tegra210_emc_timing *next) { #define __COPY_EMA(nt, lt, dev) \ ({ __MOVAVG(nt, dev) = __MOVAVG(lt, dev) * \ (nt)->ptfv_list[PTFV_DVFS_SAMPLES_INDEX]; }) - u32 i, adel = 0, samples = next->ptfv_list[PTFV_DVFS_SAMPLES_INDEX]; + u32 i, samples = next->ptfv_list[PTFV_DVFS_SAMPLES_INDEX]; u32 idx; + bool over = false; if (!next->periodic_training) return 0; @@ -253,23 +252,23 @@ static u32 periodic_compensation_handler(struct tegra210_emc *emc, u32 type, for (i = 0; i < samples; i++) { /* Generate next sample of data. */ - adel = update_clock_tree_delay(emc, DVFS_PT1); + update_clock_tree_delay(emc, DVFS_PT1); } } /* Do the division part of the moving average */ - adel = update_clock_tree_delay(emc, DVFS_UPDATE); + over = update_clock_tree_delay(emc, DVFS_UPDATE); } if (type == PERIODIC_TRAINING_SEQUENCE) - adel = update_clock_tree_delay(emc, PERIODIC_TRAINING_UPDATE); + over = update_clock_tree_delay(emc, PERIODIC_TRAINING_UPDATE); - return adel; + return over; } static u32 tegra210_emc_r21021_periodic_compensation(struct tegra210_emc *emc) { - u32 emc_cfg, emc_cfg_o, emc_cfg_update, del, value; + u32 emc_cfg, emc_cfg_o, emc_cfg_update, value; static const u32 list[] = { EMC_PMACRO_OB_DDLL_LONG_DQ_RANK0_0, EMC_PMACRO_OB_DDLL_LONG_DQ_RANK0_1, @@ -327,15 +326,12 @@ static u32 tegra210_emc_r21021_periodic_compensation(struct tegra210_emc *emc) * 4. Check delta wrt previous values (save value if margin * exceeds what is set in table). */ - del = periodic_compensation_handler(emc, - PERIODIC_TRAINING_SEQUENCE, - last, last); - + if (periodic_compensation_handler(emc, PERIODIC_TRAINING_SEQUENCE, + last, last)) { /* * 5. Apply compensation w.r.t. trained values (if clock tree * has drifted more than the set margin). */ - if (last->tree_margin < ((del * 128 * (last->rate / 1000)) / 1000000)) { for (i = 0; i < items; i++) { value = tegra210_emc_compensate(last, list[i]); emc_dbg(emc, EMA_WRITES, "0x%08x <= 0x%08x\n", @@ -516,11 +512,7 @@ static void tegra210_emc_r21021_set_clock(struct tegra210_emc *emc, u32 clksrc) EMC_EMC_STATUS_DRAM_IN_SELF_REFRESH_MASK, 0); - value = periodic_compensation_handler(emc, DVFS_SEQUENCE, fake, - next); - value = (value * 128 * next->rate / 1000) / 1000000; - - if (next->periodic_training && value > next->tree_margin) + if (periodic_compensation_handler(emc, DVFS_SEQUENCE, fake, next)) compensate_trimmer_applicable = true; }
Separate the comparison/updating of the measured delay values with the values currently programmed into a separate function to simplify the code. Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt> --- drivers/memory/tegra/tegra210-emc-cc-r21021.c | 84 +++++++++---------- 1 file changed, 38 insertions(+), 46 deletions(-)