Re: [PATCH 1/2] arm64: dts: qcom: sc7180: Add gpu cooling support

From: Akhil P Oommen
Date: Wed Oct 14 2020 - 09:30:07 EST


On 10/9/2020 10:27 PM, Matthias Kaehlcke wrote:
On Fri, Oct 09, 2020 at 08:05:10AM -0700, Doug Anderson wrote:
Hi,

On Thu, Oct 8, 2020 at 10:10 AM Akhil P Oommen <akhilpo@xxxxxxxxxxxxxx> wrote:

Add cooling-cells property and the cooling maps for the gpu tzones
to support GPU cooling.

Signed-off-by: Akhil P Oommen <akhilpo@xxxxxxxxxxxxxx>
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 29 ++++++++++++++++++++++-------
1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index d46b383..40d6a28 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -2,7 +2,7 @@
/*
* SC7180 SoC device tree source
*
- * Copyright (c) 2019, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2019-20, The Linux Foundation. All rights reserved.
*/

#include <dt-bindings/clock/qcom,dispcc-sc7180.h>
@@ -1885,6 +1885,7 @@
iommus = <&adreno_smmu 0>;
operating-points-v2 = <&gpu_opp_table>;
qcom,gmu = <&gmu>;
+ #cooling-cells = <2>;

Presumably we should add this to the devicetree bindings, too?
Yes, thanks for catching this. Will update in the next patch.



interconnects = <&gem_noc MASTER_GFX3D &mc_virt SLAVE_EBI1>;
interconnect-names = "gfx-mem";
@@ -3825,16 +3826,16 @@
};

gpuss0-thermal {
- polling-delay-passive = <0>;
+ polling-delay-passive = <100>;

Why did you make this change? I'm pretty sure that we _don't_ want
this since we're using interrupts for the thermal sensor. See commit
22337b91022d ("arm64: dts: qcom: sc7180: Changed polling mode in
Thermal-zones node").

I was going to ask the same, this shouldn't be needed.

polling-delay = <0>;

thermal-sensors = <&tsens0 13>;

trips {
gpuss0_alert0: trip-point0 {
- temperature = <90000>;
+ temperature = <95000>;
hysteresis = <2000>;
- type = "hot";
+ type = "passive";

Matthias probably knows better, but I wonder if we should be making
two passive trip levels like we do with CPU. IIRC this is important
if someone wants to be able to use this with IPA.

Yes, please introduce a second trip point and make both of them
'passive'.

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Adding Manaf here.

-Akhil.