diff mbox series

[7/7] memory: tegra: Rework update_clock_tree_delay()

Message ID 20240409094632.62916-8-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
Further streamline this function by moving the delay post-processing
to the callers, leaving it only with the task of returing the measured
delay values.

Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
---
 drivers/memory/tegra/tegra210-emc-cc-r21021.c | 120 +++++++-----------
 1 file changed, 46 insertions(+), 74 deletions(-)

Comments

Krzysztof Kozlowski April 13, 2024, 8:08 a.m. UTC | #1
On 09/04/2024 11:46, Diogo Ivo wrote:
> Further streamline this function by moving the delay post-processing
> to the callers, leaving it only with the task of returing the measured
> delay values.
> 
> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> ---
>  drivers/memory/tegra/tegra210-emc-cc-r21021.c | 120 +++++++-----------
>  1 file changed, 46 insertions(+), 74 deletions(-)
> 
> diff --git a/drivers/memory/tegra/tegra210-emc-cc-r21021.c b/drivers/memory/tegra/tegra210-emc-cc-r21021.c
> index ec2f84758d55..5e2c84fc835c 100644
> --- a/drivers/memory/tegra/tegra210-emc-cc-r21021.c
> +++ b/drivers/memory/tegra/tegra210-emc-cc-r21021.c
> @@ -105,7 +105,7 @@ enum {
>  			  next->ptfv_list[w])) /			\
>  			(next->ptfv_list[w] + 1);			\
>  									\
> -		emc_dbg(emc, EMA_UPDATES, "%s: (s=%lu) EMA: %u\n",	\
> +		emc_dbg(emc, EMA_UPDATES, "%s: (s=%u) EMA: %u\n",	\

Does not look related.



Best regards,
Krzysztof
Diogo Ivo April 15, 2024, 11:22 a.m. UTC | #2
On Sat, Apr 13, 2024 at 10:08:40AM +0200, Krzysztof Kozlowski wrote:
> On 09/04/2024 11:46, Diogo Ivo wrote:
> > Further streamline this function by moving the delay post-processing
> > to the callers, leaving it only with the task of returing the measured
> > delay values.
> > 
> > Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> > ---
> >  drivers/memory/tegra/tegra210-emc-cc-r21021.c | 120 +++++++-----------
> >  1 file changed, 46 insertions(+), 74 deletions(-)
> > 
> > diff --git a/drivers/memory/tegra/tegra210-emc-cc-r21021.c b/drivers/memory/tegra/tegra210-emc-cc-r21021.c
> > index ec2f84758d55..5e2c84fc835c 100644
> > --- a/drivers/memory/tegra/tegra210-emc-cc-r21021.c
> > +++ b/drivers/memory/tegra/tegra210-emc-cc-r21021.c
> > @@ -105,7 +105,7 @@ enum {
> >  			  next->ptfv_list[w])) /			\
> >  			(next->ptfv_list[w] + 1);			\
> >  									\
> > -		emc_dbg(emc, EMA_UPDATES, "%s: (s=%lu) EMA: %u\n",	\
> > +		emc_dbg(emc, EMA_UPDATES, "%s: (s=%u) EMA: %u\n",	\
> 
> Does not look related.

This was necessary to avoid compiler warnings as before when we were calling
this macro from update_clock_tree_delay() the value we were passing in was
declared as an unsigned long and now it is declared as u32, which it still
big enough for the values we are dealing with here.

Best regards,
Diogo
Krzysztof Kozlowski April 18, 2024, 5:19 p.m. UTC | #3
On 15/04/2024 13:22, Diogo Ivo wrote:
> On Sat, Apr 13, 2024 at 10:08:40AM +0200, Krzysztof Kozlowski wrote:
>> On 09/04/2024 11:46, Diogo Ivo wrote:
>>> Further streamline this function by moving the delay post-processing
>>> to the callers, leaving it only with the task of returing the measured
>>> delay values.
>>>
>>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>>> ---
>>>  drivers/memory/tegra/tegra210-emc-cc-r21021.c | 120 +++++++-----------
>>>  1 file changed, 46 insertions(+), 74 deletions(-)
>>>
>>> diff --git a/drivers/memory/tegra/tegra210-emc-cc-r21021.c b/drivers/memory/tegra/tegra210-emc-cc-r21021.c
>>> index ec2f84758d55..5e2c84fc835c 100644
>>> --- a/drivers/memory/tegra/tegra210-emc-cc-r21021.c
>>> +++ b/drivers/memory/tegra/tegra210-emc-cc-r21021.c
>>> @@ -105,7 +105,7 @@ enum {
>>>  			  next->ptfv_list[w])) /			\
>>>  			(next->ptfv_list[w] + 1);			\
>>>  									\
>>> -		emc_dbg(emc, EMA_UPDATES, "%s: (s=%lu) EMA: %u\n",	\
>>> +		emc_dbg(emc, EMA_UPDATES, "%s: (s=%u) EMA: %u\n",	\
>>
>> Does not look related.
> 
> This was necessary to avoid compiler warnings as before when we were calling
> this macro from update_clock_tree_delay() the value we were passing in was
> declared as an unsigned long and now it is declared as u32, which it still
> big enough for the values we are dealing with here.

OK, that's difficult to see from the patch context but looks right.

Best regards,
Krzysztof
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 ec2f84758d55..5e2c84fc835c 100644
--- a/drivers/memory/tegra/tegra210-emc-cc-r21021.c
+++ b/drivers/memory/tegra/tegra210-emc-cc-r21021.c
@@ -105,7 +105,7 @@  enum {
 			  next->ptfv_list[w])) /			\
 			(next->ptfv_list[w] + 1);			\
 									\
-		emc_dbg(emc, EMA_UPDATES, "%s: (s=%lu) EMA: %u\n",	\
+		emc_dbg(emc, EMA_UPDATES, "%s: (s=%u) EMA: %u\n",	\
 			__stringify(dev), nval, next->ptfv_list[dqs]);	\
 	} while (0)
 
@@ -130,93 +130,51 @@  static bool tegra210_emc_compare_update_delay(struct tegra210_emc_timing *timing
 	return false;
 }
 
-static u32 update_clock_tree_delay(struct tegra210_emc *emc, int type)
+static void tegra210_emc_get_clktree_delay(struct tegra210_emc *emc,
+					  u32 delay[DRAM_CLKTREE_NUM])
 {
-	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;
-	bool dvfs_update = type == DVFS_UPDATE;
-	bool dvfs_pt1 = type == DVFS_PT1;
-	u32 temp[2][2], value, udelay;
-	unsigned long cval = 0;
+	struct tegra210_emc_timing *curr = emc->last;
+	u32 rate_mhz = curr->rate / 1000;
+	u32 msb, lsb, dqsosc, udelay;
+	unsigned long clocks = 0;
 	unsigned int c, d, idx;
-	bool over = false;
 
-	if (dvfs_pt1 || periodic_training_update) {
-		udelay = tegra210_emc_actual_osc_clocks(last->run_clocks);
-		udelay *= 1000;
-		udelay = 2 + (udelay / last->rate);
+	clocks = tegra210_emc_actual_osc_clocks(curr->run_clocks);
+	udelay = 2 + (clocks / rate_mhz);
 
-		tegra210_emc_start_periodic_compensation(emc);
-		udelay(udelay);
-	}
+	tegra210_emc_start_periodic_compensation(emc);
+	udelay(udelay);
 
 	for (d = 0; d < emc->num_devices; d++) {
-		if (dvfs_pt1 || periodic_training_update) {
-			/* Dev[d] MSB */
-			value = tegra210_emc_mrr_read(emc, 2 - d, 19);
-
-			for (c = 0; c < emc->num_channels; c++) {
-				temp[c][0] = (value & 0x00ff) << 8;
-				temp[c][1] = (value & 0xff00) << 0;
-				value >>= 16;
-			}
-
-			/* Dev[d] LSB */
-			value = tegra210_emc_mrr_read(emc, 2 - d, 18);
-
-			for (c = 0; c < emc->num_channels; c++) {
-				temp[c][0] |= (value & 0x00ff) >> 0;
-				temp[c][1] |= (value & 0xff00) >> 8;
-				value >>= 16;
-			}
-		}
+		/* Read DQSOSC from MRR18/19 */
+		msb = tegra210_emc_mrr_read(emc, 2 - d, 19);
+		lsb = tegra210_emc_mrr_read(emc, 2 - d, 18);
 
 		for (c = 0; c < emc->num_channels; c++) {
 			/* C[c]D[d]U[0] */
 			idx = c * 4 + d * 2;
 
-			if (dvfs_pt1 || periodic_training_update) {
-				cval = tegra210_emc_actual_osc_clocks(last->run_clocks);
-				cval *= 1000000;
-				cval /= last_timing_rate_mhz * 2 * temp[c][0];
-			}
-
-			if (dvfs_pt1)
-				__INCREMENT_PTFV(idx, cval);
-			else if (dvfs_update)
-				__AVERAGE_PTFV(idx);
-			else if (periodic_training_update)
-				__WEIGHTED_UPDATE_PTFV(idx, cval);
+			dqsosc = (msb & 0x00ff) << 8;
+			dqsosc |= (lsb & 0x00ff) >> 0;
 
-			if (dvfs_update || periodic_training_update)
-				over |= tegra210_emc_compare_update_delay(next,
-							__MOVAVG_AC(next, idx), idx);
+			/* Check for unpopulated channels */
+			if (dqsosc)
+				delay[idx] = clocks * 1000000 / rate_mhz * 2 * dqsosc;
 
 			/* C[c]D[d]U[1] */
 			idx++;
 
-			if (dvfs_pt1 || periodic_training_update) {
-				cval = tegra210_emc_actual_osc_clocks(last->run_clocks);
-				cval *= 1000000;
-				cval /= last_timing_rate_mhz * 2 * temp[c][1];
-			}
+			dqsosc = (msb & 0xff00) << 0;
+			dqsosc |= (lsb & 0xff00) >> 8;
 
-			if (dvfs_pt1)
-				__INCREMENT_PTFV(idx, cval);
-			else if (dvfs_update)
-				__AVERAGE_PTFV(idx);
-			else if (periodic_training_update)
-				__WEIGHTED_UPDATE_PTFV(idx, cval);
+			/* Check for unpopulated channels */
+			if (dqsosc)
+				delay[idx] = clocks * 1000000 / rate_mhz * 2 * dqsosc;
 
-			if (dvfs_update || periodic_training_update)
-				over |= tegra210_emc_compare_update_delay(next,
-							__MOVAVG_AC(next, idx), idx);
+			msb >>= 16;
+			lsb >>= 16;
 		}
 	}
-
-	return over;
 }
 
 static bool periodic_compensation_handler(struct tegra210_emc *emc, u32 type,
@@ -228,7 +186,7 @@  static bool periodic_compensation_handler(struct tegra210_emc *emc, u32 type,
 	   (nt)->ptfv_list[PTFV_DVFS_SAMPLES_INDEX]; })
 
 	u32 i, samples = next->ptfv_list[PTFV_DVFS_SAMPLES_INDEX];
-	u32 idx;
+	u32 delay[DRAM_CLKTREE_NUM], idx;
 	bool over = false;
 
 	if (!next->periodic_training)
@@ -252,16 +210,30 @@  static bool periodic_compensation_handler(struct tegra210_emc *emc, u32 type,
 
 			for (i = 0; i < samples; i++) {
 				/* Generate next sample of data. */
-				update_clock_tree_delay(emc, DVFS_PT1);
+				tegra210_emc_get_clktree_delay(emc, delay);
+
+				for (idx = 0; idx < DRAM_CLKTREE_NUM; idx++)
+					__INCREMENT_PTFV(idx, delay[idx]);
 			}
 		}
 
-		/* Do the division part of the moving average */
-		over = update_clock_tree_delay(emc, DVFS_UPDATE);
+		for (idx = 0; idx < DRAM_CLKTREE_NUM; idx++) {
+			/* Do the division part of the moving average */
+			__AVERAGE_PTFV(idx);
+			over |= tegra210_emc_compare_update_delay(next,
+						__MOVAVG_AC(next, idx), idx);
+		}
 	}
 
-	if (type == PERIODIC_TRAINING_SEQUENCE)
-		over = update_clock_tree_delay(emc, PERIODIC_TRAINING_UPDATE);
+	if (type == PERIODIC_TRAINING_SEQUENCE) {
+		tegra210_emc_get_clktree_delay(emc, delay);
+
+		for (idx = 0; idx < DRAM_CLKTREE_NUM; idx++) {
+			__WEIGHTED_UPDATE_PTFV(idx, delay[idx]);
+			over |= tegra210_emc_compare_update_delay(next,
+						__MOVAVG_AC(next, idx), idx);
+		}
+	}
 
 	return over;
 }