bootlin / mali-driver

GNU General Public License v2.0
16 stars 8 forks source link

Switched to use platform_get_irq_byname #5

Closed AustinSchuh closed 1 year ago

AustinSchuh commented 1 year ago

On my 6.0 kernel, platform_get_resource wasn't finding the IRQ. The panfrost driver uses platform_get_irq_byname successfully, so this patch switches over to that.

Tested on a Rock Pi 4b on 6.0.8

Signed-off-by: Austin Schuh austin.linux@gmail.com

giuliobenetti commented 1 year ago

Hi @AustinSchuh,

thank you for your contribution! I'm going to test it soon on RK3288. I ask you some change in the code.

AustinSchuh commented 1 year ago

Cool, done!

I wasn't sure how old a kernel this repo is supposed to support, or when the API changed. Maybe you've got a better idea there.

giuliobenetti commented 1 year ago

Cool, done!

I wasn't sure how old a kernel this repo is supposed to support, or when the API changed. Maybe you've got a better idea there.

platform_get_irq_byname() is the only API I see and I've already checked it changed through all the time onle once, it's been introduced with Linux 2.6.11 that is very old and very unlikely that this driver will ever build/run on it. To check this kind of stuff I suggest and encourage you to use Elixir: https://elixir.bootlin.com/linux/v2.6.11/A/ident/platform_get_irq_byname VS https://elixir.bootlin.com/linux/v2.6.10/A/ident/platform_get_irq_byname It's useful because once you find a symbol then you can change version and how ti changes. Another way is to use git blame and see how the prototype line changes with previous commits for example. If you want, for completeness you can modify this patch and add your new implementation on is Linux version >= 2.6.11 Of course you would create 2 different functions because they are very specific to the platform_get_irq_byname() use.

I'll let you know my testing.

giuliobenetti commented 1 year ago

@AustinSchuh thank you for the patch. During my testing on a hardware different than usual(because the usual has broken) I've found that you probably modify your .dts file by adding clk_mali clock-name to mali node and changing or adding the compatible "arm,mali-midgard". Is it like that? To avoid to specify clk_mali property I've just pushed another patch to master. Can you please give a try to the following branch without adding clk_mali property and using the "arm,mali-t860" compatible? https://github.com/bootlin/mali-driver/tree/test/add-t860-compatible This way we align with Panfrost.

I wait for yours.

Thank you!

AustinSchuh commented 1 year ago
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 9d5b0e8c9cca..aa80146bc870 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -2043,16 +2043,29 @@ edp_in_vopl: endpoint@1 {
        };

        gpu: gpu@ff9a0000 {
-               compatible = "rockchip,rk3399-mali", "arm,mali-t860";
+               compatible = "arm,malit860",
+                             "arm,malit86x",
+                             "arm,malit8xx",
+                             "arm,mali-midgard";
                reg = <0x0 0xff9a0000 0x0 0x10000>;
                interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH 0>,
                             <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH 0>,
                             <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH 0>;
                interrupt-names = "job", "mmu", "gpu";
                clocks = <&cru ACLK_GPU>;
+                clock-names = "clk_mali"; 
                #cooling-cells = <2>;
                power-domains = <&power RK3399_PD_GPU>;
+                power-off-delay-ms = <200>;
                status = "disabled";
+
+                gpu_power_model: power_model {
+                        compatible = "arm,mali-simple-power-model";
+                        static-coefficient = <411000>;
+                        dynamic-coefficient = <733>;
+                        ts = <32000 4700 (-80) 2>;
+                        thermal-zone = "gpu-thermal";
+                };
        };

        pinctrl: pinctrl {

Was what I had applied, so your assumption was correct :)

I was able to revert that change with your test branch and everything appears to be working. I ran clinfo successfully and got back reasonable performance numbers. I don't see a PR for that change or I'd post my results there too. Consider this a "Reviewed-By" from me if that is helpful.

giuliobenetti commented 1 year ago

@AustinSchuh thank you for testing, patch is now committed in master branch.