Hi Nikolaus, Hi Maxime,
TI SoC seem to be the broadest number of available users of sgx5xx in the past and nowadays. Others are more the exception.
And maybe TI has some complicated stuff around the GPU that others don't have?
Looks so.
I can only agree on this.
What I also assume is that developers of DTS know what they do. So the risk that there is different semantics is IMHO very low.
Well, they know what they do if you document the binding. Let's say I have two clocks now on my SoC, and you just document that you want a clocks property, with a generic name in clock-names like "gpu".
Yes, that is what I want to propose for v7:
clocks: maxItems: 1
clock-names: maxItems: 1 items: - const: gpu
If you agree I can add the clocks/clock-names property as an optional property. This should solve omap and all others.
With the above example, what clock should I put in there? In which order? This isn't some random example pulled out of nowhere. The Allwinner A31 has (at least) 4 clocks for the GPU, 1 reset line and 1 regulator, so I can only assume that the GPU actually needs at least that amount to be properly integrated into an SoC.
Ah, now I understand your motivation: you have access and experience with the A31 and you know that our proposal doesn't fit to it.
From what I know from your description is that the A31 is quite special with 4 GPU clocks... Are they all really for the GPU or 3 of them for the interface logic (like on OMAP which separates between "functional clocks" and "interface clocks")? Or are there 4 groups of GPU cores with a separate clock for each one?
So what would be your proposal for the A31 DT?
Then I get a chance to compare DT snippets and try to make a mixture for the bindings.
This is my current binding for the A83T, the A31 looks similar but there is still something missing, since some parts are not mainlined yet:
sun8i-a83t.dtsi: gpu: gpu@1c400000 { compatible = "allwinner,sun8i-a83t-sgx544-115", "img,sgx544-115", "img,sgx544"; reg = <0x01c40000 0x10000>; interrupts = <GIC_SPI 129 IRQ_TYPE_LEVEL_HIGH>; clocks = <&ccu CLK_BUS_GPU>, <&ccu CLK_GPU_CORE>, <&ccu CLK_GPU_MEMORY>, <&ccu CLK_GPU_HYD>; clock-names = "bus", "core", "memory", "hyd";
resets = <&ccu RST_BUS_GPU>; };
sun8i-a83t-bananapi-m3.dts: &gpu { gpu-supply = <®_dcdc4>; };
But given that the current state on the Allwinner SoCs (at least) is that you can't even read a register, it might be a good idea to delay the introduction of that binding until you have something that works to avoid drowning under the number of special cases to deal with backward compatibility.
Maxime is right. Even if you enable the regulator, write 0x0 to the GPU Power Off Gating Register, deassert the reset and enable the clocks you are not able to read the register. You must first run: pvrsrvctl --no-module --start (user space binaries) to access registers otherwise the system will stuck with the following message when accessing them:
./devmem2 0x01C40024 /dev/mem opened.
Philipp has something minimal working on the A83 which works with the proposed binding and enabled him to read out the sgx revision register.
This is not correct. In the other mail I talked about my reference system. This is an old 3.4.39 kernel, modified by allwinner to run on their SOC's which don't use the common kernel techniques. So it's very hacky, but they got the gpu running. I'm using this system for comparison, to read out registers and for reverse engineering.
My current kernel module behaves similar like the reference design, but right now I'm not able to run "pvrsrvctl --no-module --start" without errors. So the initialization never get's finalized and I see the issue described above.
So if you are a specialist for the A31 SGX, please make a proposal for a binding that covers all the A31 needs and all other SoC we know. Or define a separate bindings for the A31.
The A31 and the A31s have some additional clocks mentioned in their datasheet (@ System Control Register/SRAM Controler). Those are not available in the A83T datasheet, but might be there since the memory map looks similar and allwinner might use the same userspace binaries for their devices.
From the knowledge I gained the last 3 days, we should delay the patches for the A83T, A31 and A31s. The idea of the placeholder patches was to show that from this binding also other devices could benefit.
Cheers, Philipp