Hi Siarhei, On 11 January 2015 at 18:04, Simon Glass <[email protected]> wrote: > Hi Siarhei, > > On 11 January 2015 at 16:28, Siarhei Siamashka > <[email protected]> wrote: >> On Sat, 10 Jan 2015 08:24:52 -0700 >> Simon Glass <[email protected]> wrote: >> >>> Hi, >>> >>> On 10 January 2015 at 03:50, Ian Campbell <[email protected]> wrote: >>> > On Fri, 2015-01-09 at 13:13 +0200, Siarhei Siamashka wrote: >>> >> > If yes then I think it is confusing to modify this comment, and the >>> >> > comment before the pre-console #defines should mention that the buffer >>> >> > is boottime only/short lived etc. >>> >> >>> >> Just in case if something goes really wrong (in theory it shouldn't, >>> >> but in practice you know...) it is still somewhat safer to keep the >>> >> buffer in its own dedicated area and keep everything else out. >>> > >>> > Nothing in those defines is protecting anything though, if the kernel is >>> > more than 15M it will still overwrite that area. >>> > >>> >> > Perhaps a better place for the pre-console buffer would be right before >>> >> > the framebuffer (or at the top of RAM if no video on the board), with >>> >> > modifications to bootm_size or not depending on the answer to the >>> >> > original question above. >>> >> >>> >> If this needs any kind of runtime address calculations instead of >>> >> compile time constants, then IMHO it becomes unnecessarily complicated. >>> > >>> > One for Hans I think, my understanding was that the framebuffer was at >>> > the top of RAM, but having bootm_size set to 0xf0000000 unconditionally >>> > doesn't match that. I suppose the idea is that it corresponds with the >>> > smallest board because it's not worth making it dynamic (I think I >>> > recall Hans saying something like that at the time). >>> > >>> > I think you could safely put the early buffer at 0xf000000-DELTA (and >>> > adjust bootm_size to match), rather than worrying about packing it up >>> > below the real framebuffer. >>> > >>> >> Anyway. The sunxi part of these changes just needs to assign some >>> >> memory area to the pre-console buffer. In the end it does not really >>> >> matter which one. The size also does not need to be too large. >>> >> For example 1920 * 1080 / (8 * 16) = 16200. It means that only ~16K >>> >> of the log buffer can fully cover the FullHD display using the 8x16 >>> >> font. And this is even assuming no line breaks. I picked 1M only >>> >> because it was the smallest unit of the address space allocation in >>> >> sunxi-common.h :-) >>> > >>> > I don't think it needs to be allocated in 1M chunks, it just happens to >>> > have been arbitrarily chosen that way so far. >>> > >>> > If you want to keep the early buffer down in that region then I think >>> > it'd be better to steal a few KB from the end of the fdt, script or pxe >>> > (all of which will never be anywhere near 1MB) than stealing 1M off the >>> > end of the kernel (it's not totally inconceivable that a kernel might be >>> > approaching ~15M in size) >>> >>> I don't think it is a good idea to use the 'pre-console buffer' after >>> the console exists. It is a misnomer. >> >> If one is fixated on the idea that "console" == "UART serial console", >> then yes, keeping the pre-console buffer after the UART console >> is initialized looks like a misnomer. However there are other >> consoles too. And in the case of tablets, the "LCD console" may >> be the only console that is (reasonably easily) accessible. IMHO, >> the "pre-vga console buffer" interpretation is just as valid as the >> "pre-serial console buffer".
[snip] Just to repeat I think this patch is a good feature. It's great to see it. Also, so far as the implementation goes I have no objections - my comments relate to plans to extend it further. Regards, Simon _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
