stikonas has quit [Remote host closed the connection]
naoki has quit [Quit: naoki]
naoki1 has joined #linux-rockchip
naoki1 is now known as naoki
qschulz has quit [Remote host closed the connection]
qschulz has joined #linux-rockchip
<macromorgan>
yeah, in all seriousness I wonder if 256k is good enough or if I should go nuts for 512k, assuming it doesn't break anything else anywhere.
<macromorgan>
I was worried about some sram limitation or some stuff, but I honestly have very little idea what I'm doing this early in the boot process
<macromorgan>
the biggest tree I see today (in debian anyway) is a qualcomm tree weighing in at 145k... I feel like if we were to do 256k we'd have enough room for a while, at least with the rk3588...
System_Error has quit [Remote host closed the connection]
System_Error has joined #linux-rockchip
System_Error has quit [Remote host closed the connection]
System_Error has joined #linux-rockchip
hexdump0815 has quit [Ping timeout: 252 seconds]
hexdump0815 has joined #linux-rockchip
System_Error has quit [Remote host closed the connection]
System_Error has joined #linux-rockchip
System_Error has quit [Remote host closed the connection]
System_Error has joined #linux-rockchip
<naoki>
It's been a while since I last submitted a patch, so I've forgotten how to do it. Hopefully there won't be any warnings... ;)
franoosh has joined #linux-rockchip
raster has joined #linux-rockchip
ldevulder has joined #linux-rockchip
<qschulz>
macromorgan: I feel like 256K is too low. We need to handle DTB with symbols and a shit ton of overlays applied to it
<qschulz>
one of the worries I have with TF-A is I think we only reserve "very little" memory on DRAM
<qschulz>
where the symbols for the rock-5b are already generated by the overlay application test
<qschulz>
also, can't we have the overlays for the other boards upstreamed and using the overlay applicatin test like in the patch series I sent?
<naoki>
oh
<Kwiboo>
detlevc: do you know if 0xfdc40000 or 0xfdc48000 is correct base address for second rkvdec2 core on rk3588?, TRM and your tree mention the first address, downstream kernel use the second address in dtsi :-S
<Kwiboo>
I am working on a v2 of a u-boot rk3582 support patch and would like to try and predict the correct rkvdec/rkvenc node names that should be disabled/failed
<CounterPillow>
fwiw I've run into dtb size issues when enabling symbols on RK356x before as well. It'd be great if we could start some more general discussion somewhere where we look into whether this problem can be solved more generally. To my knowledge, at the TF-A stage, we already have all of DRAM to play with, so I'm not sure why each component makes the programmer manually play memory extent tetris.
<naoki>
qschulz: That is, should I prepare at least one overlay.dtbo for each board.dtb?
<qschulz>
naoki: I don't know what's mmind00 opinion on that, but I believe the more we have upstreamed, the better. This includes DT overlays
<qschulz>
so if there is at least one overlay available for each board and that it's for stuff one can actually buy from Radxa or third party vendors, then I believe it makes sense to send a patch for each
<qschulz>
and then have a DT overlay application test which replicates a possible HW combination
<qschulz>
and then, you magically get the symbols in the main DTB of the board + validation that the overlay can actually apply on top of the base DTB :)
<CounterPillow>
Also enabling symbols for dtbs of SBCs in the kernel but see my point, that'll break a lot of currently working systems due to the ballooning sizes
<qschulz>
CounterPillow: see second link I sent to naoki a few minutes ago and answer there
<qschulz>
I don't believe it'll break a lot of currently working systems
<qschulz>
because U-Boot doesn't use that DT with symbols
<qschulz>
for itself, therefore not for TF-A either
f476 has joined #linux-rockchip
<qschulz>
for TF-A, you only get what got reserved for TF-A (2MiB) by U-Boot or less if TF-A is compiled to use less (which seems to be the case for upstream, ~7xxMiB I believe)
<qschulz>
CounterPillow: now is the time to answer to that mail because once we merge it, we open the flood gates to DTB having symbols enabled
<mmind00>
qschulz: I think you meant the link you sent to macromorgan right?
<mmind00>
i.e. where I tried to go over the affected variants
<qschulz>
mmind00: yes to macromorgan sorry
<naoki>
How do I handle overlays that are not board specific (but may be SoC specific) like changing the otg dr_mode?
<mmind00>
naoki: that's not soc specific
<mmind00>
also why would you need to override dr-mode?
<mmind00>
i.e. ... if the _board_ only has a host connector, you need to set it in the main board dts; if the controller is broken on the soc, you set it in the soc dtsi
<naoki>
Some Radxa boards have a Type-A OTG port with no physical role-changing switch.
<qschulz>
mmind00: ah I think this may be a "revision 1.1 only has host mode, but all the others have host and peripheral?
<naoki>
I should have said common across multiple boards, not specific to a single board.
<naoki>
that's not soc specific.
<mmind00>
somehow I'd assume the usb subsystem should contain functionality to force device/host mode?
<naoki>
A similar issue is setting up a mux on a 40-pin header.
<naoki>
As far as I know, except for Type-C, the only way is to change the dr_mode in the DTS.
<naoki>
By the way, does anyone know why defining CONFIG_SPL_USB_DWC3_GENERIC=y in U-Boot creates objects for TPL as well?
<naoki>
Linking u-boot-tpl fails. In the first place, dwc3 itself should not be necessary for TPL...
<qschulz>
naoki: because XPL_ always resolves to SPL_ in any non-proper stage
<qschulz>
you want PHASE_ instead of XPL_ in drivers/usb/dwc3/Makefile I guess
<qschulz>
don't know why we have both XPL_ and PHASE_ and it seems they are used 50/50 (205 found in Makefiles for XPL_, 314 for PHASE_)
_whitelogger has joined #linux-rockchip
Stat_headcrabbed has joined #linux-rockchip
f476 has quit [Ping timeout: 260 seconds]
f476 has joined #linux-rockchip
raster has quit [Remote host closed the connection]
raster has joined #linux-rockchip
raster has quit [Remote host closed the connection]
raster has joined #linux-rockchip
Stat_headcrabbed has quit [Quit: Stat_headcrabbed]
Stat_headcrabbed has joined #linux-rockchip
<dsimic>
qschulz: no worries about the Cc
<dsimic>
qschulz: Kwiboo: I read a bit about it, but not as much as I wanted... will do for sure
Stat_headcrabbed has quit [Quit: Stat_headcrabbed]
<dsimic>
I must say that I like the idea about compressed .dtb files more
<naoki>
I'm trying to add DFU to SPL, it works on RK3308, RK3328, RK356x, and RK3588, but it doesn't work on RK3399 :(
<naoki>
oh I found error...
<qschulz>
dsimic: this would require to add support for that to TF-A, OP-TEE, whatever else
<qschulz>
and you still need to store the uncompressed data somewhere when you use it
<qschulz>
so that wouldn't fix much of TF-A's issues for example
<dsimic>
indeed, it would solve only the storage issue
<qschulz>
it depends what we're expecting TF-A and OP-TEE to do
<qschulz>
if it's expected they won't apply overlays themselves, and not rename or relocate nodes, then we can always strip the symbols before passing the full DTB to TF-A
<qschulz>
don't remember if BL31 pass back the DTB or not, acting as a proxy essentially, in which case this would prevent us from being able to apply overlays after the DTB is gotten back from TF-A
Stat_headcrabbed has joined #linux-rockchip
<dsimic>
hmm, I'll also revisit the editing of DT performed by TF-A on some Allwinner platforms
<dsimic>
I'll make sure to go through the ML thread(s) today
<dsimic>
and to possibly correlate all that to the Allwinner TF-A stuff, in an attempt to provide an even bigger picture
naoki has quit [Quit: naoki]
<mmind00>
dsimic Kwiboo et all: whie you go down the rabbit hole of what to do in the future, could you also check if you can poke holes into the "current software" argument ... i.e. the one about: any DT hard-coded on devices right now, won't be affected by us adding symbols to new builds
<mmind00>
I made in v2 of qschulz's series
<dsimic>
oh, sure, not breaking already existing stuff is the paramount
<mmind00>
dsimic: where the argument is that existing bootloader software+DTs won't be affected ... and we never guarantee running old SW with new DTs ... just new SW with old DTs
<dsimic>
of course, only the backward DT compatibility is what we must ensure, so we e.g. don't brick devices that provide the DTs through their firmware
<dsimic>
after a kernel upgrade
<qschulz>
i think this is all perfectly fine because we only modify the DTB, and according to the guarantees, new DTB only with new SW, so there may be required changes outside of the kernel to be able to use the new DTB
<qschulz>
but we should probably have some idea of what's about to break so we can either work on fixing that asap or let the appropriate people know we're aboput to break stuff
<detlevc>
Kwiboo: Probably an error in the downstream dts (or there is a 3rd hidden core), but the TRM specifies VDPU381_core0_base: 0xFDC38000, VDPU381_core1_base: 0xFDC40000 and I have been testing both. I also have a setup where I use mpp on the 1st core an rkvdec2 on the second core and all works well with the 0xfdc40000 base address
Stat_headcrabbed has quit [Quit: Stat_headcrabbed]
<macromorgan>
qschulz: I think the following statement is not true... "Considering that U-Boot passes the full DTB to TF-A". The default setting is SPL_ATF_NO_PLATFORM_PARAM=y which doesn't pass a pointer to A-TF of the device tree, right?
<qschulz>
SPL_ATF_NO_PLATFORM_PARAM=n
<qschulz>
I have two boards with SPL_ATF_NO_PLATFORM_PARAM=n
<macromorgan>
"imply SPL_ATF_NO_PLATFORM_PARAM if SPL_ATF" is set in the Kconfig
<qschulz>
it's an imply yes
<macromorgan>
okay, but the *default* option doesn't even hand things off I guess is what I'm saying
<qschulz>
I actually removed it for RK3399 boards and moved it to defconfigs
<qschulz>
so any new RK3399 board will need to explicitly set SPL_ATF_NO_PLATFORM_PARAM if they are using Rockchip's blob
<macromorgan>
okay... I'll test today with 256KB I guess and see if that breaks stuff, if not I'll push some upstream requests
<qschulz>
I guess I should have said that "U-Boot can pass" instead of "U-Boot passes"
<macromorgan>
any benefit to trying v2 of the parameters? I mean in for a penny in for a pound am I right?
<qschulz>
macromorgan: nah, the point is: are we correct in stating that we're safe today and have we missed any corner case or user of that kernel FDT?
<qschulz>
i am not trying to push anyone to fix other projects
<macromorgan>
okay
<qschulz>
we want to know which projects is using or could be using this kernel DTB
<macromorgan>
I'll try for 256KB, we can keep "imply SPL_ATF_NO_PLATFORM_PARAM if SPL_ATF" at the SoC level and have board specific overrides... then I can just add instructions into my board documentation of how to use all the mainline bits
<qschulz>
macromorgan: I would actually switch all SoCs supported by upstream TF-A to have SPL_ATF_NO_PLATFORM_PARAM in defconfigs now
<qschulz>
e.g. RK3588
<qschulz>
I don't think RK356x support is complete upstream though
<macromorgan>
it's not
<macromorgan>
at least as of a few weeks ago it still wasn't
<macromorgan>
I had some issues with the VOP's power domain still at startup and suspend
<macromorgan>
even after adding the clock and reset stuff to SCMI
<macromorgan>
doing a grep even when looking at the removal of "imply SPL_ATF_NO_PLATFORM_PARAM if SPL_ATF" for rk3399 I still see a lot of boards go back and set it to Y manually
<qschulz>
macromorgan: they are not going back, I just moved the symbol to defconfigs instead of the SoC
<qschulz>
I migrated my board to upstream TF-A and removed this SPL_ATF_NO_PLATFORM_PARAM
<macromorgan>
okay I get it
<qschulz>
it's up to board maintainers to remove it as well, but I cannot decide for them
<qschulz>
macromorgan: ok so let me rephrase what I was asking you wrt mail. (1) are you aware of any user other than TF-A of DTB (and specifically, the kernel DTB)?
<macromorgan>
Optee
<qschulz>
(2) do you agree with the statements mmind00 and I provided in replies to the patch
<qschulz>
(3) do you foresee any project we'll break by keeping symbols in DTB?
<qschulz>
s/we'll break/breakage/
<qschulz>
(4) out of answers to (3) which ones are an issue today and which are just theoretical or future-me issues?
<Kwiboo>
qschulz: macromorgan: last time I checked TF-A for rockchip basically only tried to get correct uart/serial information from the passed DT, so maybe we can supply a trimmed down fdt if TF-A only need limited information
<Kwiboo>
and if I understand optee will try to add reserved memory nodes into DT, so does it need the kernel DT?
<macromorgan>
Kwiboo: U-boot basically does this today
<macromorgan>
and no, it does not, though U-Boot is trying to get away from having its own trees and I support this initative
<qschulz>
we are years away from being able to do this, we still have bootph properties all over the place which aren't upstreamed to the Linux kernel tree
<macromorgan>
right
<macromorgan>
but to the extent we can not futher differentiate the better, at least in my view
<qschulz>
and then you enter a new kind of issue, with new kernel DTB with old U-Boot
<qschulz>
and that means forward AND backward compatibility
<qschulz>
Kwiboo: no not really, it also supposedly should be able to handle some GPIOs
<qschulz>
ah that may be using another way of passing info that isn't actually DT
<macromorgan>
so my first question... if we're basically hard coding the first 2MB of RAM to be off limits but we're really only using the first 1MB for A-TF, can we expand that to 2MB?
<macromorgan>
assume that'd be TZRAM_SIZE?
<qschulz>
I don't see why not\
<macromorgan>
if we go from 1MB to 2MB we should have plenty of room for a bigger DTB
<qschulz>
for TF-A yes
<macromorgan>
right
<qschulz>
512K or maybe even 1M
<macromorgan>
and we have 32MB for Optee too, which again ought to be enough
<macromorgan>
1M seems excessive, but would we really want 512K?
<qschulz>
how is the DT passed to TEE?
<macromorgan>
if I read correctly a pointer is passed to the TEE in the platform parameters
<qschulz>
macromorgan: I'm thinking about multi-display DTS with panel timings and that kind of things, can grow quite big quite fast no?
ungeskriptet has quit [Remote host closed the connection]
<macromorgan>
possibly, for now the single largest tree I found in mainline is ~170KB
<macromorgan>
but yeah, future growth potential would be nice
<qschulz>
macromorgan: without symbols, yes, think about **with symbols** :)
<macromorgan>
right
<qschulz>
actually maybe the issue is rather a tree with MANY nodes
<qschulz>
do you have some link to provide for how the DTB is given to TEE?
<macromorgan>
yeah somewhere, I think. It's the x2 register if I recall in the platform parameters
<qschulz>
and, do you know how TEE (upstream at least) parses that?
ungeskriptet has joined #linux-rockchip
<macromorgan>
sorry, x0
<macromorgan>
it looks like U-Boot (SPL) hands off to A-TF the address of Optee, the address of U-Boot (main), and the address of the FDT
<Kwiboo>
does tf-a really need to open the fdt into new memory? is it needed beyond init, else it could possible just: if (fdt_check_header((void *)param_from_bl2)) plat_rockchip_dt_process_fdt_uart((void *)param_from_bl2);
<macromorgan>
I don't know honestly... to my knowledge only Optee actually modifies the FDT
<macromorgan>
actually to be clear, I don't know most of what's been asked so far this morning :-p
<qschulz>
Kwiboo: I guess the issue is if you want to modify it you need to be able to increase its size and not overlap on other data?
<qschulz>
but considering the whole idea with this unique DTB stuff is to pass them from one stage to the other, at some point you need to provide this bigger modified DTB to the next stage
<Kwiboo>
qschulz: yeah, based on quick grep it look like other plat may modify fdt so they need a new buffer, cannot see anything like that for rockchip
<qschulz>
so keeping it in BL31 reserved memory is not going to be really a good idea
franoosh has joined #linux-rockchip
<qschulz>
Kwiboo: anything... yet! Don't know what Rockchip is doing in their blob that they haven;t upstreamed yet :D
<qschulz>
Kwiboo: ah maybe it's to make sure TF-A has a copy of the DT and if it somehow needs to read the DT later on, it doesn't reread again from some address that may have had its content changed
<Kwiboo>
hehe, true, that will not help with older tf-a blobs, but possible just a c/p of what other platforms does, that actually modified the fdt
<dsimic>
one of the troubles with Allwinner TF-A is that it patches the FDT that's passed around, and all those changes become lost when U-Boot loads another FDT from /boot, as instructed in extlinux.conf... thus, it would be best to have TF-A (and other FDT "patchers") actually (or additionally) produce FDT patches that can be applied wherever they're needed
<qschulz>
dsimic: I think you want the same thing macromorgan wants, have the same DTB passed from stage to stage
<qschulz>
Kwiboo: we need to know the size of the FDT though and we cannot increase its size safely, so that's a big limitation
<macromorgan>
#define FDT_BUFFER_SIZE 0x20000
<qschulz>
macromorgan: yeah it's 128KiB, I actually modified that code a year or so ago :D
<macromorgan>
right... checking to see if it's coded anywhere else too
<macromorgan>
I'm going to try playing with 256K first then 512K
<dsimic>
qschulz: the trouble is that the "patching" performed by TF-A needs (and should) be applied to the filesystem-provided .dtb as well
<macromorgan>
I can easily make my tree bigger than 128K for testing by just adding symbols... making it bigger than 256K will present a large challenge though
<dsimic>
for example, not having that currently in place makes CPUIDLE not working on the A64-based PinePhone
<macromorgan>
dsimic: A-TF doesn't patch it today; Optee does though and U-Boot has code in place already to do that
<macromorgan>
which is to say U-Boot today copies the reserved memory nodes today into the kernel tree if you meet certain conditions
<macromorgan>
(which is why I opened this giant can of worms in the first place...)
<qschulz>
dsimic: isn't what TF-A put in the DTB reproducible? Can't you make U-Boot proper fixup the kernel dtb based on that?
<dsimic>
macromorgan: hmm, has it changed? let me check, please...
<dsimic>
qschulz: it is, but it's uknown what TF-A did
<dsimic>
s/uknown/unknown/
<qschulz>
dsimic: save the DTB before passing to TF-A at loadaddr X and pass a copy to loadaddr Y, then after TF-A returns, manually diff with libfdt the whole tree between X and Y :D
<dsimic>
uh-oh, that would be brute force approach :)
<dsimic>
s/be/be a/
<macromorgan>
are we talking about what BSP A-TF did? I'm not seeing anything being done to the tree by A-TF in mainline (we only pull from it, not modify it)
<qschulz>
macromorgan: dsimic is also talking about Allwinner at the same time
<dsimic>
macromorgan: here's what I still see in TF-A's plat/allwinner/common/sunxi_prepare_dtb.c:
<macromorgan>
yeah, allwinner does change stuff
<dsimic>
39 if (sunxi_psci_is_scpi()) {
<dsimic>
40 ret = fdt_add_cpu_idle_states(fdt, sunxi_idle_states);
<dsimic>
41 if (ret < 0) {
<dsimic>
42 WARN("Failed to add idle states to DT: %d\n", ret);
<dsimic>
43 }
<dsimic>
44 }
<macromorgan>
I spent the past few weeks in Allwinner land working on the H616 mainlining efforts :-)
<dsimic>
nice, thanks! :)
<macromorgan>
I wrote some board detect code for U-Boot I'm trying to get tested before I push upstream... it's going to require refactoring of a bit of the code, since Allwinner likes to lump everything together in board/allwinner rather than specific board/vendor
<qschulz>
though SHARE_MEM_SIZE seems to be 4 * 15 K
<qschulz>
so 60KiB
<macromorgan>
ohh, yeah okay
<qschulz>
and 4KiB for SCMI_SHARE_MEM
<qschulz>
would be nice to know what this shared mem is used for
<qschulz>
like does it need to be absolutely there
<qschulz>
or can we move it around
<qschulz>
Kwiboo: I'll look into your answer for the pinctrl stuff tomorrow. I only need to remove the use of ctrl->nr_pins and I don't think it's actually useful, but I haven't read your patches yet :)
raster has quit [Remote host closed the connection]
raster has joined #linux-rockchip
Stat_headcrabbed has joined #linux-rockchip
Stat_headcrabbed has quit [Client Quit]
<macromorgan>
the device tree buffer is getting pulled in the BSS section... 512k is as large as it gets, and even then it's taking up most of the memory allocated to A-TF
<macromorgan>
I'd argue that since we have a whopping 16KB to spare with a 512k device tree buffer, we might want to go back to 384k and leave it there?
<Kwiboo>
qschulz: thanks, you have already sent a few reviwed-by and comments on my pinctrl old series. I put a pause on it due to tpl/spl code size growth and lack of time, your new series remided me of it :-)
<macromorgan>
I'm seeing a ton of memory addresses here, there, and everywhere if I try to go above 512k...
<macromorgan>
would 256k give us enough breathing room for a time, or should I test at 384k?
vagrantc has joined #linux-rockchip
stikonas has joined #linux-rockchip
raster has quit [Quit: Gettin' stinky!]
digetx has quit [Ping timeout: 246 seconds]
digetx has joined #linux-rockchip
naoki has joined #linux-rockchip
sjoerd1 has joined #linux-rockchip
sjoerd has quit [Ping timeout: 245 seconds]
sjoerd1 is now known as sjoerd
naoki has quit [Quit: naoki]
naoki has joined #linux-rockchip
<macromorgan>
okay have patches ready to go, now I just need to figure out how to submit them.
<macromorgan>
I'm going to 1) update A-TF to have a 256k buffer for device trees. Any larger and we start risking running out of space. 256k should be fine for now as the largest tree in the mainline kernel when I build everything with symbols enabled is 215K, and it's extremely unlikely you'd want to apply a ton of overlays before A-TF runs. After A-TF we don't care as much about the size anyway.
<macromorgan>
2) I'm going to update Optee to add CFG_AUTO_MAX_PA_BITS=y as the default for rk3588, add CFG_DTB_MAX_SIZE ?= 0x40000 to match A-TF changes, and change the TZDRAM_START and SHMEM_START to mirror the px30 and rk3399
<macromorgan>
3) I'm going to update Linux to remove the optee node. If Optee is enabled U-Boot will add it back along with the mem holes, if it's not enabled it doesn't need to be there anyway.
naoki has quit [Ping timeout: 246 seconds]
naoki has joined #linux-rockchip
System_Error has quit [Remote host closed the connection]
System_Error has joined #linux-rockchip
vagrantc has quit [Quit: leaving]
naoki has quit [Quit: naoki]
naoki1 has joined #linux-rockchip
naoki1 is now known as naoki
System_Error has quit [Remote host closed the connection]