diff mbox series

[6/7] memory: tegra: Move compare/update current delay values to a function

Message ID 20240409094632.62916-7-diogo.ivo@tecnico.ulisboa.pt
State Changes Requested
Headers show
Series Cleanup Tegra210 EMC frequency scaling | expand

Commit Message

Diogo Ivo April 9, 2024, 9:46 a.m. UTC
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(-)

Comments

Krzysztof Kozlowski April 13, 2024, 8:07 a.m. UTC | #1
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
Diogo Ivo April 15, 2024, 11:17 a.m. UTC | #2
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 mbox series

Patch

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;
 	}