Hello,
This patch series consists of a general cleanup of the Tegra210 EMC
frequency scaling code for revision 7.
Currently the code is relying heavily on a function, update_clock_tree_delay(),
that is responsible for too many things, making it long and confusing.
The general idea with these patches is to simplify this function and its
surrounding code, making it more modular.
The motivation behind these changes (besides improving readability and
maintainability) is to make it simpler to add support in the future for
frequency change revisions other than 7, where we can reuse a large
portion of the modularized code rather than essentially repeating 2k
lines of code with minimal changes.
There are no functional changes with this patch set, as it is only meant
as preparation for following patches where revision 6 support is added.
The second version of the series can be found in [1]. v3 contains
changes only in patch 02/07 where a variable is renamed in order to fix
a build error on some architectures.
[1]: https://lore.kernel.org/linux-tegra/[email protected]/
Diogo Ivo (7):
memory: tegra: Remove periodic compensation duplicate calls
memory: tegra: Move DQSOSC measurement to common place
memory: tegra: Reword and correct comments
memory: tegra: Change macros to interpret parameter as integer
memory: tegra: Loop update_clock_tree_delay()
memory: tegra: Move compare/update current delay values to a function
memory: tegra: Rework update_clock_tree_delay()
drivers/memory/tegra/tegra210-emc-cc-r21021.c | 427 ++++--------------
1 file changed, 84 insertions(+), 343 deletions(-)
--
2.44.0
Fix incorrect comment on periodic_compensation_handler() as the call
update_clock_tree_delay() with DVFS_UPDATE is responsible for dividing
the samples accumulated up to that point and comparing the computed
values with the currently programmed ones. While at it fix the
indentation of a nearby comment.
Signed-off-by: Diogo Ivo <[email protected]>
---
drivers/memory/tegra/tegra210-emc-cc-r21021.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/memory/tegra/tegra210-emc-cc-r21021.c b/drivers/memory/tegra/tegra210-emc-cc-r21021.c
index a3525f3b8145..a9e19dfa9856 100644
--- a/drivers/memory/tegra/tegra210-emc-cc-r21021.c
+++ b/drivers/memory/tegra/tegra210-emc-cc-r21021.c
@@ -451,18 +451,12 @@ static u32 periodic_compensation_handler(struct tegra210_emc *emc, u32 type,
__MOVAVG(next, C1D1U1) = 0;
for (i = 0; i < samples; i++) {
- /*
- * Generate next sample of data.
- */
+ /* Generate next sample of data. */
adel = update_clock_tree_delay(emc, DVFS_PT1);
}
}
- /*
- * Seems like it should be part of the
- * 'if (last_timing->periodic_training)' conditional
- * since is already done for the else clause.
- */
+ /* Do the division part of the moving average */
adel = update_clock_tree_delay(emc, DVFS_UPDATE);
}
--
2.44.0
Further streamline this function by moving the delay post-processing
to the callers, leaving it only with the task of returning the measured
delay values.
Signed-off-by: Diogo Ivo <[email protected]>
---
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 a8a217502f0c..277fca62f57e 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 bool 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, delay_us;
- unsigned long cval = 0;
+ struct tegra210_emc_timing *curr = emc->last;
+ u32 rate_mhz = curr->rate / 1000;
+ u32 msb, lsb, dqsosc, delay_us;
unsigned int c, d, idx;
- bool over = false;
+ unsigned long clocks;
- if (dvfs_pt1 || periodic_training_update) {
- delay_us = tegra210_emc_actual_osc_clocks(last->run_clocks);
- delay_us *= 1000;
- delay_us = 2 + (delay_us / last->rate);
+ clocks = tegra210_emc_actual_osc_clocks(curr->run_clocks);
+ delay_us = 2 + (clocks / rate_mhz);
- tegra210_emc_start_periodic_compensation(emc);
- udelay(delay_us);
- }
+ tegra210_emc_start_periodic_compensation(emc);
+ udelay(delay_us);
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,8 +186,8 @@ 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 delay[DRAM_CLKTREE_NUM], idx;
bool over = false;
- u32 idx;
if (!next->periodic_training)
return 0;
@@ -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;
}
--
2.44.0
On 27/05/2024 12:15, Diogo Ivo wrote:
> Hello,
>
> On Tue, May 07, 2024 at 10:30:43AM GMT, Diogo Ivo wrote:
>> Hello,
>>
>> This patch series consists of a general cleanup of the Tegra210 EMC
>> frequency scaling code for revision 7.
>>
>> Currently the code is relying heavily on a function, update_clock_tree_delay(),
>> that is responsible for too many things, making it long and confusing.
>> The general idea with these patches is to simplify this function and its
>> surrounding code, making it more modular.
>>
>> The motivation behind these changes (besides improving readability and
>> maintainability) is to make it simpler to add support in the future for
>> frequency change revisions other than 7, where we can reuse a large
>> portion of the modularized code rather than essentially repeating 2k
>> lines of code with minimal changes.
>
> Gentle ping on this patch series. I think this version addressed all of
> the review comments, is there anything missing?
I think I explained you the process. Merge window finished yesterday
(today for this timezone), so why pinging the same day? Give some time...
Best regards,
Krzysztof
Hello,
On Tue, May 07, 2024 at 10:30:43AM GMT, Diogo Ivo wrote:
> Hello,
>
> This patch series consists of a general cleanup of the Tegra210 EMC
> frequency scaling code for revision 7.
>
> Currently the code is relying heavily on a function, update_clock_tree_delay(),
> that is responsible for too many things, making it long and confusing.
> The general idea with these patches is to simplify this function and its
> surrounding code, making it more modular.
>
> The motivation behind these changes (besides improving readability and
> maintainability) is to make it simpler to add support in the future for
> frequency change revisions other than 7, where we can reuse a large
> portion of the modularized code rather than essentially repeating 2k
> lines of code with minimal changes.
Gentle ping on this patch series. I think this version addressed all of
the review comments, is there anything missing?
Best regards,
Diogo
On Mon, May 27, 2024 at 12:20:07PM GMT, Krzysztof Kozlowski wrote:
> On 27/05/2024 12:15, Diogo Ivo wrote:
> > Gentle ping on this patch series. I think this version addressed all of
> > the review comments, is there anything missing?
>
> I think I explained you the process. Merge window finished yesterday
> (today for this timezone), so why pinging the same day? Give some time...
Sorry, I didn't mean to spam you. In fact it's actually good that you
didn't merge this yet as I just noticed that there should be parenthesis
in patch 7 around (rate_mhz * 2 * dqsosc) so that we get the proper result
for the delay. I apologize for missing this and I will send a v4 once I
re-check the series for mistakes like this.
Best regards,
Diogo