On Tue, Sep 07, 2021 at 09:41:23PM +0200, Jan Kiszka wrote:
> On 02.09.21 08:36, Jan Kiszka wrote:
> > On 28.07.21 11:10, Jan Kiszka wrote:
> >> On 30.01.20 09:05, Roger Quadros wrote:
> >>> NB0 is bridge to SRAM and NB1 is bridge to DDR.
> >>>
> >>> To ensure that SRAM transfers are not stalled due to
> >>> delays during DDR refreshes, SRAM traffic should be higher
> >>> priority (threadmap=2) than DDR traffic (threadmap=0).
> >>>
> >>> This patch does just that.
> >>>
> >>> This is required to fix ICSSG TX lock-ups due to delays in
> >>> MSMC transfers due to incorrect Northbridge configuration.
> >>>
> >>> Signed-off-by: Roger Quadros <rog...@ti.com>
> >>> Acked-by: Andrew F. Davis <a...@ti.com>
> >>> Acked-by: Tomi Valkeinen <tomi.valkei...@ti.com>
> >>> Acked-by: Benoit Parrot <bpar...@ti.com>
> >>> ---
> >>> arch/arm/mach-k3/am6_init.c | 14 ++++++++++++++
> >>> arch/arm/mach-k3/include/mach/am6_hardware.h | 7 +++++++
> >>> 2 files changed, 21 insertions(+)
> >>>
> >>> diff --git a/arch/arm/mach-k3/am6_init.c b/arch/arm/mach-k3/am6_init.c
> >>> index 8d107b870b..9379b95bdb 100644
> >>> --- a/arch/arm/mach-k3/am6_init.c
> >>> +++ b/arch/arm/mach-k3/am6_init.c
> >>> @@ -86,6 +86,18 @@ static void store_boot_index_from_rom(void)
> >>> bootindex = *(u32 *)(CONFIG_SYS_K3_BOOT_PARAM_TABLE_INDEX);
> >>> }
> >>>
> >>> +static void setup_am654_navss_northbridge(void)
> >>> +{
> >>> + /*
> >>> + * NB0 is bridge to SRAM and NB1 is bridge to DDR.
> >>> + * To ensure that SRAM transfers are not stalled due to
> >>> + * delays during DDR refreshes, SRAM traffic should be higher
> >>> + * priority (threadmap=2) than DDR traffic (threadmap=0).
> >>> + */
> >>> + writel(0x2, NAVSS0_NBSS_NB0_CFG_BASE + NAVSS_NBSS_THREADMAP);
> >>> + writel(0x0, NAVSS0_NBSS_NB1_CFG_BASE + NAVSS_NBSS_THREADMAP);
> >>> +}
> >>> +
> >>> void board_init_f(ulong dummy)
> >>> {
> >>> #if defined(CONFIG_K3_LOAD_SYSFW) || defined(CONFIG_K3_AM654_DDRSS)
> >>> @@ -101,6 +113,8 @@ void board_init_f(ulong dummy)
> >>> /* Make all control module registers accessible */
> >>> ctrl_mmr_unlock();
> >>>
> >>> + setup_am654_navss_northbridge();
> >>> +
> >>> #ifdef CONFIG_CPU_V7R
> >>> disable_linefill_optimization();
> >>> setup_k3_mpu_regions();
> >>> diff --git a/arch/arm/mach-k3/include/mach/am6_hardware.h
> >>> b/arch/arm/mach-k3/include/mach/am6_hardware.h
> >>> index 6df7631545..45a5b31c52 100644
> >>> --- a/arch/arm/mach-k3/include/mach/am6_hardware.h
> >>> +++ b/arch/arm/mach-k3/include/mach/am6_hardware.h
> >>> @@ -47,4 +47,11 @@
> >>> /* MCU SCRATCHPAD usage */
> >>> #define TI_SRAM_SCRATCH_BOARD_EEPROM_START
> >>> CONFIG_SYS_K3_MCU_SCRATCHPAD_BASE
> >>>
> >>> +/* NAVSS Northbridge config */
> >>> +#define NAVSS0_NBSS_NB0_CFG_BASE 0x03802000
> >>> +#define NAVSS0_NBSS_NB1_CFG_BASE 0x03803000
> >>> +
> >>> +#define NAVSS_NBSS_PID 0x0
> >>> +#define NAVSS_NBSS_THREADMAP 0x10
> >>> +
> >>> #endif /* __ASM_ARCH_AM6_HARDWARE_H */
> >>>
> >>
> >> This was never merged, not even commented on (only apparently rejected
> >> in patchwork) - but it is crucial as we now found out:
> >>
> >> prueth will quickly stall when these priorities are not applied, at
> >> least with SR1.0-based AM65x designs. And you probably know what else
> >> could go wrong. Please clarify and merge, possibly reducing the scope to
> >> SR1.0 if you can confirm that SR2.0 cannot be affected by design (I can
> >> only say this based on few practical experiments here).
> >>
> >> If it was good for several TI SDK releases by now, at least something
> >> similar should be good for upstream as well, I believe.
> >>
> >
> > Ping. We need at least some confirmation on what is actually needed.
> > Then, if you do not like to add it to the generic path, it would be easy
> > for us to carry it in the IOT2050 board init only - with the appropriate
> > condition check.
>
> Did some more experiments on SR2 silicon, and while I'm not seeing
> stalls without this patch there, iperf tests with prueth show high retry
> rates and, thus, about 10% worse throughput. So it seems to be required
> for newer silicon as well.At this point, I'd really appreciate it if someone from within TI can chime in here as I expect someone must suspect something here, thanks! -- Tom
signature.asc
Description: PGP signature
