On Tue, 18 Aug 2015 07:44:22 +0800 Vishnu Patekar <[email protected]> wrote:
> Hello, > Please see in-lined. > > On Tue, Aug 18, 2015 at 2:52 AM, Siarhei Siamashka > <[email protected]> wrote: > > On Tue, 18 Aug 2015 01:53:23 +0800 > > Vishnu Patekar <[email protected]> wrote: > > > >> On allwinner SOCs A33,H3 MMU is not enabled by BROM, so don't exit > >> if MMU is not enabled by BROM and return NULL. > >> It was reported on A33 tablet and Orange Pi H3 Board. > >> I've tested it on A33 tablet. > >> > >> Signed-off-by: VishnuPatekar <[email protected]> > > > > Thanks. If some of the SoC variants don't have MMU enabled by the BROM > > in the first place, then there is no need for us to temporarily disable > > it during the SPL execution. > > > > Later we might want to try enabling MMU on A33 for performance reasons > > though. The USB driver in the BROM is apparently doing a memcpy loop as > > part of the data transfer. And tweaking the cacheability attributes of > > the memory sections via MMU can improve the transfer speed quite > > significantly (up to 3x speed boost on Allwinner A20): > > > > https://github.com/linux-sunxi/sunxi-tools/commit/e4b3da2b17ee9d7c5cab9b80e708b3a309fc4c96 > > > > You can benchmark the transfer speed to DRAM on A33 by running something > > like "fel -v write 0x42000000 uImage". > > benchmark result is: > Written 2944.6 KB in 15.3 sec (speed: 191.9 KB/s) Ugh, this is very slow. And very likely can be improved significantly by enabling the MMU (after the SPL execution has completed). Instead of adjusting the existing table we would need to create a new one, so it needs a little bit more work compared to the SoC variants, where the MMU is already enabled by the BROM right from the start. We would not get record breaking speeds, but waiting 15 seconds for just uploading the kernel is way too much. As a side note, for getting the best possible performance, implementing a USB driver and the FEL protocol handling (or some other protocol) in the SPL might be an option to consider as an experiment in a distant future. It is not necessarily a good idea though, because currently we are saving the SPL size by using the USB code from the BROM. > >> --- > >> fel.c | 14 +++++++++----- > >> 1 file changed, 9 insertions(+), 5 deletions(-) > >> > >> diff --git a/fel.c b/fel.c > >> index 41b19c9..c53be60 100644 > >> --- a/fel.c > >> +++ b/fel.c > >> @@ -502,7 +502,7 @@ uint32_t aw_get_sctlr(libusb_device_handle *usb) > >> > >> uint32_t *aw_backup_and_disable_mmu(libusb_device_handle *usb) > >> { > >> - uint32_t *tt = malloc(16 * 1024); > >> + uint32_t *tt = NULL; > >> uint32_t ttbr0 = aw_get_ttbr0(usb); > >> uint32_t sctlr = aw_get_sctlr(usb); > >> uint32_t i; > >> @@ -520,19 +520,20 @@ uint32_t > >> *aw_backup_and_disable_mmu(libusb_device_handle *usb) > >> > >> if (!(sctlr & 1)) { > >> fprintf(stderr, "MMU is not enabled by BROM\n"); > > > > If this is in fact not an error on some SoC variants (A33), then > > probably we should downgrade it from "fprintf(stderr, ...)" to > > "pr_info(...)" in order not to scare/confuse the users. > Okie. I'll change it to pr_info. > > > >> - exit(1); > >> + goto exit; > > > > Maybe just "return NULL" here would be cleaner without adding goto > > statements and labels? > Okie. > > > >> } > >> > >> if ((sctlr >> 28) & 1) { > >> fprintf(stderr, "TEX remap is enabled!\n"); > >> - exit(1); > >> + goto exit; > >> } > >> > >> if (ttbr0 & 0x3FFF) { > >> fprintf(stderr, "Unexpected TTBR0 (%08X)\n", ttbr0); > >> - exit(1); > >> + goto exit; > >> } > > > > I would not touch these two extra safety checks unless really necessary. > > They can only fail if you have MMU enabled, but configured in an > > unexpected way. > Okie. Thanks. -- Best regards, Siarhei Siamashka -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
