[Openpvrsgx-devgroup] S5PV210 Patches

[H. Nikolaus Schaller]

Hi Jonathan,

> Am 20.04.2020 um 21:42 schrieb Jonathan Bakker <xc-racer2 at live.ca>:
> 
> 
> 
> On 2020-04-20 12:11 a.m., H. Nikolaus Schaller wrote:
>> Fine. One question in light of discussion on lkml: is there a reason to call the clock-name = "sclk"?
>> It seems as if clock-name = "gpu" could fit better?
> 
> No real reason, other than that's what the vendor kernel called it :)

ok, I see.

> 
> Looking at the s5pv210 datasheet, there's two clocks allocated to the sgx, the core (which is controllable, that I've labelled sclk) and the bus clock (which doesn't appear to be controllable).

Ok, I see. This roughly corresponds to the functional clock "fclk" and interface
clock "iclk" concept of OMAP devices, although those are no longer explicitly
controlled by the sgx driver.

This is what may have confused Maxime for the A31, because he knows that the
driver should control 4 clocks there.

>  I have no real preference on what they are named.  "gpu" seem a little generic.

What about "core"?

Or could you please try if this works (or can be made working?):

		gpu-module at f3000000 {
			compatible = "simple-pm-bus";
			reg = <0xf3000000 0x10000>;
     			#address-cells = <1>;
			#size-cells = <1>;
			ranges;

			clock-names = "sclk";
			clocks = <&clocks CLK_G3D>;

			assigned-clocks = <&clocks MOUT_G3D>, <&clocks DOUT_G3D>;
			assigned-clock-rates = <0>, <66700000>;
			assigned-clock-parents = <&clocks MOUT_MPLL>;

			gpu: gpu at 0 {
				compatible = "samsung,s5pv210-sgx540-120";
				reg = <0x0 0x10000>;
				interrupt-parent = <&vic2>;
				interrupts = <10>;
			};
		};

This would allow to keep the clocks and their number completely outside
of the sgx node. Basically it looks to me that you have 3 clocks.

I am not exactly sure about ranges and reg entries, but here
it is described as exactly what we would need:

https://elixir.bootlin.com/linux/v5.7-rc2/source/Documentation/devicetree/bindings/bus/simple-pm-bus.yaml

But this would allow to use Tony's pvr_drv driver to work without
any explicit clock and power gating since it does the required pm
calls.

The more I think about, Maxime hasn't understood the concept we
are following here and it could easily solve his A31 definition
(but I am not sure if I have - and I haven't explained well :).

My goal is to separate the clock/pm definitions from the sgx core itself.

So we should take the chance to try/compare this approach on real
hardware before we submit a version with optional clocks for
the sgx bindings. Because this forces us to have a driver that
is capable of getting this clock. And then we have to agree on
a clock name.

BR,
Nikolaus