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@xxxxxxxxxxxxxxxxxx>
> > ---
> > 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
