W dniu 19.03.2024 o 01:08, Andre Przywara pisze: > On Tue, 19 Mar 2024 00:28:43 +0100 > Kamil Kasperski <ressetkk@gmail.com> wrote: > > Hi Kamil, > >> W dniu 18.03.2024 o 12:42, Andre Przywara pisze: >>> On Sun, 17 Mar 2024 20:44:51 +0100 >>> Kamil Kasperski <ressetkk@gmail.com> wrote: >>> >>> Hi Kamil, >>> >>> thanks a lot for putting together those patches and sending them >>> for upstream inclusion! >>> >>>> Add dtsi file for T95 tv boxes and add initial support for T95 5G AXP313A >>>> variant with a board name H616-T95MAX-AXP313A-v3.0 Internal storage is not >>>> accessible due to lack of support for H616 NAND controller. >>> Them using raw NAND is really unfortunate. I think the original T95 box >>> used eMMC? >> I'm not sure. I don't have access to the other revision of this box. > That's alright, I was just curious. I think Jernej has such an early > box, and IIRC he used mostly the X96 Mate DT with it, so I was assuming > it has eMMC. > >>> >>>> Signed-off-by: Kamil Kasperski <ressetkk@gmail.com> >>>> --- >>>> arch/arm64/boot/dts/allwinner/Makefile | 1 + >>>> arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi | 109 +++++++++++++++++++++ >>>> .../dts/allwinner/sun50i-h616-t95max-axp313.dts | 85 ++++++++++++++++ >>>> 3 files changed, 195 insertions(+) >>>> >>>> diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile >>>> index 21149b346a60..294921f12b73 100644 >>>> --- a/arch/arm64/boot/dts/allwinner/Makefile >>>> +++ b/arch/arm64/boot/dts/allwinner/Makefile >>>> @@ -42,6 +42,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-tanix-tx6-mini.dtb >>>> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-bigtreetech-cb1-manta.dtb >>>> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-bigtreetech-pi.dtb >>>> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-orangepi-zero2.dtb >>>> +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-t95max-axp313.dtb >>>> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-x96-mate.dtb >>>> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-longanpi-3h.dtb >>>> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-orangepi-zero2w.dtb >>>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi >>>> new file mode 100644 >>>> index 000000000000..815cf2dac24b >>>> --- /dev/null >>>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi >>>> @@ -0,0 +1,109 @@ >>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) >>>> +/* >>>> + * Copyright (C) 2024 Kamil Kasperski <ressetkk@gmail.com> >>>> + * >>>> + * Common DT nodes for H616-based T95 TV boxes >>>> + * There are two versions reported with different PMIC variants. >>>> + */ >>>> + >>>> +#include "sun50i-h616.dtsi" >>>> + >>>> +#include <dt-bindings/gpio/gpio.h> >>>> +#include <dt-bindings/interrupt-controller/arm-gic.h> >>>> + >>>> +/ { >>>> + aliases { >>>> + ethernet1 = &sdio_wifi; >>>> + serial0 = &uart0; >>>> + }; >>>> + >>>> + chosen { >>>> + stdout-path = "serial0:115200n8"; >>>> + }; >>>> + >>>> + reg_vcc5v: vcc5v { >>>> + /* board wide 5V supply directly from the DC input */ >>>> + compatible = "regulator-fixed"; >>>> + regulator-name = "vcc-5v"; >>>> + regulator-min-microvolt = <5000000>; >>>> + regulator-max-microvolt = <5000000>; >>>> + regulator-always-on; >>>> + }; >>>> + >>>> + reg_vcc3v3: vcc3v3 { >>>> + /* discrete 3.3V regulator */ >>>> + compatible = "regulator-fixed"; >>>> + regulator-name = "vcc-3v3"; >>>> + regulator-min-microvolt = <3300000>; >>>> + regulator-max-microvolt = <3300000>; >>>> + regulator-always-on; >>>> + }; >>>> + >>>> + wifi_pwrseq: wifi-pwrseq { >>> Krzysztof recently sent a patch to just use "pwrseq" as the node name, >>> so can you do the same here? >>> >>> https://lore.kernel.org/linux-sunxi/20240317184130.157695-2-krzysztof.kozlowski@linaro.org/T/#u >> Sure. I'll send v4. >> >>> >>>> + compatible = "mmc-pwrseq-simple"; >>>> + clocks = <&rtc CLK_OSC32K_FANOUT>; >>>> + clock-names = "ext_clock"; >>>> + pinctrl-0 = <&x32clk_fanout_pin>; >>>> + pinctrl-names = "default"; >>>> + reset-gpios = <&pio 6 18 GPIO_ACTIVE_LOW>; /* PG18 */ >>>> + }; >>>> +}; >>>> + >>>> +&ehci0 { >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&ehci2 { >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&ir { >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&mmc0 { >>>> + cd-gpios = <&pio 8 16 GPIO_ACTIVE_LOW>; /* PI16 */ >>>> + bus-width = <4>; >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&mmc1 { >>>> + mmc-pwrseq = <&wifi_pwrseq>; >>>> + bus-width = <4>; >>>> + non-removable; >>>> + status = "okay"; >>>> + >>>> + sdio_wifi: wifi@1 { >>>> + reg = <1>; >>>> + }; >>> So does the WiFi work with mainline drivers? IIUC the BCM43342 is not >>> supported by the existing Broadcom drivers? >> It's actually BCM43342/1 It System doesn't detect this chip by default. >> The most relevant message from dmesg is: >> [ 14.042035] kernel: brcmfmac: brcmf_fw_alloc_request: Unknown chip BCM43342/1 >> I believe that it's only a matter of missing module. I don't think it is >> supported in mainline ATM. I left it to have a wi-fi node accessible >> and detectable by kernel. If you think that it's better to remove the >> node if it's not supported I can do it. > No, that's fine, I was just curious. But another thing to consider are > any GPIOs connected to the module, for reset and wakeup. Do you have a > dump of the vendor firmware DT somewhere? At least this information in > there seems reliable. > You should be able to access the loaded DTB via /sys/firmware/fdt, then > can decompile that with dtc. Look for properties ending in -gpios, > related to WiFi/WLAN. Yep, I do. I also have gpio dump from /sys/kernel/debug/gpio and some of the pins are present there gpio-207 ( |wlan_hostwake ) in lo gpio-208 ( |bt_hostwake ) in hi gpio-209 ( |bt_wake ) out hi gpio-210 ( |wlan_regon ) out lo gpio-211 ( |bt_rst ) out lo DTS doesn't contain any additional information apart from the string that defines hostwake and regon. wlan_hostwake = <0x53 0x06 0x0f 0x06 0xffffffff 0xffffffff 0x00>; wlan_regon = <0x53 0x06 0x12 0x01 0xffffffff 0xffffffff 0x00>; The only -gpios properties are for the sdcard. It's already included in dts. There are quite more pins allocated in /sys/kernel/debug/gpio but they are not labeled, or not relevant for this device. >> Somebody actually extracted modified precompiled module from custom >> 5.15.16 rockchip kernel, which implements support for this card. There's no >> patch for it that could be submitted to mainline unfortunately ATM. >> >> I've found a patch that adds chip id strings to brcmfmac, but I would like to >> test it beforehand. > Sure, using the SDIO ID it should autodetect it, and mainline Linux not > (yet) supporting it is strictly speaking not a reason to not describe > it in the DT - only that we would like to test it somehow, of course. > So if in doubt leave it as it is, that should not hurt in any case. Let's leave it. Right now I can pin-point that there is missing firmware blob. The firmware and patch for WIFI and BT is floating on the net, but it's not mainlined in firmware repo (yet, hopefully). Cheers, Kamil >>>> +}; >>>> + >>>> +&ohci0 { >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&ohci2 { >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&uart0 { >>>> + pinctrl-names = "default"; >>>> + pinctrl-0 = <&uart0_ph_pins>; >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&uart1 { >>>> + pinctrl-names = "default"; >>>> + pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>; >>>> + uart-has-rtscts; >>>> + status = "okay"; >>> Similar question here, what is the Bluetooth situation in mainline? I >>> guess it's not supported, since you didn't put a BT node in here? >> I haven't tested that yet. It's partially due to lack of experience with >> DTS. I would like to figure out mainline support eventually, but for now >> I would like to just have a working foundation for these TVB. >> >> If that's also not really a wanted option to leave those nodes, I can >> remove them for now. > No, again that's fine from a pure DT perspective. The information above > seems accurate. But again there might be GPIOs at play here, something > to look out for in the vendor DT. > In my experience the BT seem to be easier to support. Even for combined > modules you might have luck and see BT working, even when WiFi does not. > But again, just leave it as it is now, we can always add that later, > that should not block the submission. > >>>> +}; >>>> + >>>> +&usbotg { >>>> + dr_mode = "host"; /* USB A type receptable */ >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&usbphy { >>>> + status = "okay"; >>>> +}; >>>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-t95max-axp313.dts b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95max-axp313.dts >>>> new file mode 100644 >>>> index 000000000000..c8650aca2407 >>>> --- /dev/null >>>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95max-axp313.dts >>>> @@ -0,0 +1,85 @@ >>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) >>>> +/* >>>> + * Copyright (C) 2024 Kamil Kasperski <ressetkk@gmail.com> >>>> + * >>>> + * Configuration for T95 TV box with board label H616-T95MAX-AXP313A-v3.0 >>>> + */ >>>> + >>>> +/dts-v1/; >>>> + >>>> +#include "sun50i-h616-t95.dtsi" >>>> + >>>> +/ { >>>> + model = "T95 5G (AXP313)"; >>>> + compatible = "t95,t95max-axp313", "allwinner,sun50i-h616"; >>>> +}; >>>> + >>>> +&mmc0 { >>>> + vmmc-supply = <®_dldo1>; >>>> +}; >>>> + >>>> +&mmc1 { >>>> + vmmc-supply = <®_dldo1>; >>>> + vqmmc-supply = <®_aldo1>; >>>> +}; >>>> + >>>> +&r_i2c { >>>> + status = "okay"; >>>> + >>>> + axp313: pmic@36 { >>>> + compatible = "x-powers,axp313a"; >>>> + reg = <0x36>; >>>> + #interrupt-cells = <1>; >>>> + interrupt-controller; >>>> + interrupt-parent = <&pio>; >>> I don't think you need interrupt-parent unless you also actually specify >>> an interrupt line. >>> (But please keep #interrupt-cells and interrupt-controller.) >> I think you may be right. I based this part off of OPiZ3 dts, rather than >> transpeed. Will update in the v4. > Thanks! > > Cheers, > Andre > >>>> + >>>> + vin1-supply = <®_vcc5v>; >>>> + vin2-supply = <®_vcc5v>; >>>> + vin3-supply = <®_vcc5v>; >>>> + >>>> + regulators { >>>> + reg_aldo1: aldo1 { >>>> + regulator-always-on; >>>> + regulator-min-microvolt = <1800000>; >>>> + regulator-max-microvolt = <1800000>; >>>> + regulator-name = "vcc1v8"; >>>> + }; >>>> + >>>> + reg_dldo1: dldo1 { >>>> + regulator-always-on; >>>> + regulator-min-microvolt = <3300000>; >>>> + regulator-max-microvolt = <3300000>; >>>> + regulator-name = "vcc3v3"; >>>> + }; >>>> + >>>> + reg_dcdc1: dcdc1 { >>>> + regulator-always-on; >>>> + regulator-min-microvolt = <810000>; >>>> + regulator-max-microvolt = <990000>; >>>> + regulator-name = "vdd-gpu-sys"; >>>> + }; >>>> + >>>> + reg_dcdc2: dcdc2 { >>>> + regulator-always-on; >>>> + regulator-min-microvolt = <810000>; >>>> + regulator-max-microvolt = <1100000>; >>>> + regulator-name = "vdd-cpu"; >>>> + }; >>>> + >>>> + reg_dcdc3: dcdc3 { >>>> + regulator-always-on; >>>> + regulator-min-microvolt = <1500000>; >>>> + regulator-max-microvolt = <1500000>; >>>> + regulator-name = "vdd-dram"; >>>> + }; >>>> + }; >>>> + }; >>>> +}; >>>> + >>>> +&pio { >>>> + vcc-pc-supply = <®_aldo1>; >>>> + vcc-pf-supply = <®_dldo1>; >>>> + vcc-pg-supply = <®_dldo1>; >>> So if vqmmc-supply for MMC1 is at the 1.8V from ALDO1, that must mean that >>> the whole of PortG is at 1.8V, right? So I think this would need to be >>> reg_aldo1 here. >>> >>> Apart from those minor things it looks good to me. >>> >>> Cheers, >>> Andre >> Good catch. Will update in v4. >> >> Cheers, >> Kamil >> >>>> + vcc-ph-supply = <®_dldo1>; >>>> + vcc-pi-supply = <®_dldo1>; >>>> +}; >>>> >>
On Tue, 19 Mar 2024 00:28:43 +0100 Kamil Kasperski <ressetkk@gmail.com> wrote: Hi Kamil, > W dniu 18.03.2024 o 12:42, Andre Przywara pisze: > > On Sun, 17 Mar 2024 20:44:51 +0100 > > Kamil Kasperski <ressetkk@gmail.com> wrote: > > > > Hi Kamil, > > > > thanks a lot for putting together those patches and sending them > > for upstream inclusion! > > > >> Add dtsi file for T95 tv boxes and add initial support for T95 5G AXP313A > >> variant with a board name H616-T95MAX-AXP313A-v3.0 Internal storage is not > >> accessible due to lack of support for H616 NAND controller. > > Them using raw NAND is really unfortunate. I think the original T95 box > > used eMMC? > > I'm not sure. I don't have access to the other revision of this box. That's alright, I was just curious. I think Jernej has such an early box, and IIRC he used mostly the X96 Mate DT with it, so I was assuming it has eMMC. > > > >> Signed-off-by: Kamil Kasperski <ressetkk@gmail.com> > >> --- > >> arch/arm64/boot/dts/allwinner/Makefile | 1 + > >> arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi | 109 +++++++++++++++++++++ > >> .../dts/allwinner/sun50i-h616-t95max-axp313.dts | 85 ++++++++++++++++ > >> 3 files changed, 195 insertions(+) > >> > >> diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile > >> index 21149b346a60..294921f12b73 100644 > >> --- a/arch/arm64/boot/dts/allwinner/Makefile > >> +++ b/arch/arm64/boot/dts/allwinner/Makefile > >> @@ -42,6 +42,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-tanix-tx6-mini.dtb > >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-bigtreetech-cb1-manta.dtb > >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-bigtreetech-pi.dtb > >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-orangepi-zero2.dtb > >> +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-t95max-axp313.dtb > >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-x96-mate.dtb > >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-longanpi-3h.dtb > >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-orangepi-zero2w.dtb > >> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi > >> new file mode 100644 > >> index 000000000000..815cf2dac24b > >> --- /dev/null > >> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi > >> @@ -0,0 +1,109 @@ > >> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > >> +/* > >> + * Copyright (C) 2024 Kamil Kasperski <ressetkk@gmail.com> > >> + * > >> + * Common DT nodes for H616-based T95 TV boxes > >> + * There are two versions reported with different PMIC variants. > >> + */ > >> + > >> +#include "sun50i-h616.dtsi" > >> + > >> +#include <dt-bindings/gpio/gpio.h> > >> +#include <dt-bindings/interrupt-controller/arm-gic.h> > >> + > >> +/ { > >> + aliases { > >> + ethernet1 = &sdio_wifi; > >> + serial0 = &uart0; > >> + }; > >> + > >> + chosen { > >> + stdout-path = "serial0:115200n8"; > >> + }; > >> + > >> + reg_vcc5v: vcc5v { > >> + /* board wide 5V supply directly from the DC input */ > >> + compatible = "regulator-fixed"; > >> + regulator-name = "vcc-5v"; > >> + regulator-min-microvolt = <5000000>; > >> + regulator-max-microvolt = <5000000>; > >> + regulator-always-on; > >> + }; > >> + > >> + reg_vcc3v3: vcc3v3 { > >> + /* discrete 3.3V regulator */ > >> + compatible = "regulator-fixed"; > >> + regulator-name = "vcc-3v3"; > >> + regulator-min-microvolt = <3300000>; > >> + regulator-max-microvolt = <3300000>; > >> + regulator-always-on; > >> + }; > >> + > >> + wifi_pwrseq: wifi-pwrseq { > > Krzysztof recently sent a patch to just use "pwrseq" as the node name, > > so can you do the same here? > > > > https://lore.kernel.org/linux-sunxi/20240317184130.157695-2-krzysztof.kozlowski@linaro.org/T/#u > > Sure. I'll send v4. > > > > >> + compatible = "mmc-pwrseq-simple"; > >> + clocks = <&rtc CLK_OSC32K_FANOUT>; > >> + clock-names = "ext_clock"; > >> + pinctrl-0 = <&x32clk_fanout_pin>; > >> + pinctrl-names = "default"; > >> + reset-gpios = <&pio 6 18 GPIO_ACTIVE_LOW>; /* PG18 */ > >> + }; > >> +}; > >> + > >> +&ehci0 { > >> + status = "okay"; > >> +}; > >> + > >> +&ehci2 { > >> + status = "okay"; > >> +}; > >> + > >> +&ir { > >> + status = "okay"; > >> +}; > >> + > >> +&mmc0 { > >> + cd-gpios = <&pio 8 16 GPIO_ACTIVE_LOW>; /* PI16 */ > >> + bus-width = <4>; > >> + status = "okay"; > >> +}; > >> + > >> +&mmc1 { > >> + mmc-pwrseq = <&wifi_pwrseq>; > >> + bus-width = <4>; > >> + non-removable; > >> + status = "okay"; > >> + > >> + sdio_wifi: wifi@1 { > >> + reg = <1>; > >> + }; > > So does the WiFi work with mainline drivers? IIUC the BCM43342 is not > > supported by the existing Broadcom drivers? > > It's actually BCM43342/1 It System doesn't detect this chip by default. > The most relevant message from dmesg is: > [ 14.042035] kernel: brcmfmac: brcmf_fw_alloc_request: Unknown chip BCM43342/1 > I believe that it's only a matter of missing module. I don't think it is > supported in mainline ATM. I left it to have a wi-fi node accessible > and detectable by kernel. If you think that it's better to remove the > node if it's not supported I can do it. No, that's fine, I was just curious. But another thing to consider are any GPIOs connected to the module, for reset and wakeup. Do you have a dump of the vendor firmware DT somewhere? At least this information in there seems reliable. You should be able to access the loaded DTB via /sys/firmware/fdt, then can decompile that with dtc. Look for properties ending in -gpios, related to WiFi/WLAN. > Somebody actually extracted modified precompiled module from custom > 5.15.16 rockchip kernel, which implements support for this card. There's no > patch for it that could be submitted to mainline unfortunately ATM. > > I've found a patch that adds chip id strings to brcmfmac, but I would like to > test it beforehand. Sure, using the SDIO ID it should autodetect it, and mainline Linux not (yet) supporting it is strictly speaking not a reason to not describe it in the DT - only that we would like to test it somehow, of course. So if in doubt leave it as it is, that should not hurt in any case. > >> +}; > >> + > >> +&ohci0 { > >> + status = "okay"; > >> +}; > >> + > >> +&ohci2 { > >> + status = "okay"; > >> +}; > >> + > >> +&uart0 { > >> + pinctrl-names = "default"; > >> + pinctrl-0 = <&uart0_ph_pins>; > >> + status = "okay"; > >> +}; > >> + > >> +&uart1 { > >> + pinctrl-names = "default"; > >> + pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>; > >> + uart-has-rtscts; > >> + status = "okay"; > > Similar question here, what is the Bluetooth situation in mainline? I > > guess it's not supported, since you didn't put a BT node in here? > > I haven't tested that yet. It's partially due to lack of experience with > DTS. I would like to figure out mainline support eventually, but for now > I would like to just have a working foundation for these TVB. > > If that's also not really a wanted option to leave those nodes, I can > remove them for now. No, again that's fine from a pure DT perspective. The information above seems accurate. But again there might be GPIOs at play here, something to look out for in the vendor DT. In my experience the BT seem to be easier to support. Even for combined modules you might have luck and see BT working, even when WiFi does not. But again, just leave it as it is now, we can always add that later, that should not block the submission. > >> +}; > >> + > >> +&usbotg { > >> + dr_mode = "host"; /* USB A type receptable */ > >> + status = "okay"; > >> +}; > >> + > >> +&usbphy { > >> + status = "okay"; > >> +}; > >> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-t95max-axp313.dts b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95max-axp313.dts > >> new file mode 100644 > >> index 000000000000..c8650aca2407 > >> --- /dev/null > >> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95max-axp313.dts > >> @@ -0,0 +1,85 @@ > >> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > >> +/* > >> + * Copyright (C) 2024 Kamil Kasperski <ressetkk@gmail.com> > >> + * > >> + * Configuration for T95 TV box with board label H616-T95MAX-AXP313A-v3.0 > >> + */ > >> + > >> +/dts-v1/; > >> + > >> +#include "sun50i-h616-t95.dtsi" > >> + > >> +/ { > >> + model = "T95 5G (AXP313)"; > >> + compatible = "t95,t95max-axp313", "allwinner,sun50i-h616"; > >> +}; > >> + > >> +&mmc0 { > >> + vmmc-supply = <®_dldo1>; > >> +}; > >> + > >> +&mmc1 { > >> + vmmc-supply = <®_dldo1>; > >> + vqmmc-supply = <®_aldo1>; > >> +}; > >> + > >> +&r_i2c { > >> + status = "okay"; > >> + > >> + axp313: pmic@36 { > >> + compatible = "x-powers,axp313a"; > >> + reg = <0x36>; > >> + #interrupt-cells = <1>; > >> + interrupt-controller; > >> + interrupt-parent = <&pio>; > > I don't think you need interrupt-parent unless you also actually specify > > an interrupt line. > > (But please keep #interrupt-cells and interrupt-controller.) > > I think you may be right. I based this part off of OPiZ3 dts, rather than > transpeed. Will update in the v4. Thanks! Cheers, Andre > >> + > >> + vin1-supply = <®_vcc5v>; > >> + vin2-supply = <®_vcc5v>; > >> + vin3-supply = <®_vcc5v>; > >> + > >> + regulators { > >> + reg_aldo1: aldo1 { > >> + regulator-always-on; > >> + regulator-min-microvolt = <1800000>; > >> + regulator-max-microvolt = <1800000>; > >> + regulator-name = "vcc1v8"; > >> + }; > >> + > >> + reg_dldo1: dldo1 { > >> + regulator-always-on; > >> + regulator-min-microvolt = <3300000>; > >> + regulator-max-microvolt = <3300000>; > >> + regulator-name = "vcc3v3"; > >> + }; > >> + > >> + reg_dcdc1: dcdc1 { > >> + regulator-always-on; > >> + regulator-min-microvolt = <810000>; > >> + regulator-max-microvolt = <990000>; > >> + regulator-name = "vdd-gpu-sys"; > >> + }; > >> + > >> + reg_dcdc2: dcdc2 { > >> + regulator-always-on; > >> + regulator-min-microvolt = <810000>; > >> + regulator-max-microvolt = <1100000>; > >> + regulator-name = "vdd-cpu"; > >> + }; > >> + > >> + reg_dcdc3: dcdc3 { > >> + regulator-always-on; > >> + regulator-min-microvolt = <1500000>; > >> + regulator-max-microvolt = <1500000>; > >> + regulator-name = "vdd-dram"; > >> + }; > >> + }; > >> + }; > >> +}; > >> + > >> +&pio { > >> + vcc-pc-supply = <®_aldo1>; > >> + vcc-pf-supply = <®_dldo1>; > >> + vcc-pg-supply = <®_dldo1>; > > So if vqmmc-supply for MMC1 is at the 1.8V from ALDO1, that must mean that > > the whole of PortG is at 1.8V, right? So I think this would need to be > > reg_aldo1 here. > > > > Apart from those minor things it looks good to me. > > > > Cheers, > > Andre > > Good catch. Will update in v4. > > Cheers, > Kamil > > >> + vcc-ph-supply = <®_dldo1>; > >> + vcc-pi-supply = <®_dldo1>; > >> +}; > >> > >
W dniu 18.03.2024 o 12:42, Andre Przywara pisze: > On Sun, 17 Mar 2024 20:44:51 +0100 > Kamil Kasperski <ressetkk@gmail.com> wrote: > > Hi Kamil, > > thanks a lot for putting together those patches and sending them > for upstream inclusion! > >> Add dtsi file for T95 tv boxes and add initial support for T95 5G AXP313A >> variant with a board name H616-T95MAX-AXP313A-v3.0 Internal storage is not >> accessible due to lack of support for H616 NAND controller. > Them using raw NAND is really unfortunate. I think the original T95 box > used eMMC? I'm not sure. I don't have access to the other revision of this box. > >> Signed-off-by: Kamil Kasperski <ressetkk@gmail.com> >> --- >> arch/arm64/boot/dts/allwinner/Makefile | 1 + >> arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi | 109 +++++++++++++++++++++ >> .../dts/allwinner/sun50i-h616-t95max-axp313.dts | 85 ++++++++++++++++ >> 3 files changed, 195 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile >> index 21149b346a60..294921f12b73 100644 >> --- a/arch/arm64/boot/dts/allwinner/Makefile >> +++ b/arch/arm64/boot/dts/allwinner/Makefile >> @@ -42,6 +42,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-tanix-tx6-mini.dtb >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-bigtreetech-cb1-manta.dtb >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-bigtreetech-pi.dtb >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-orangepi-zero2.dtb >> +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-t95max-axp313.dtb >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-x96-mate.dtb >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-longanpi-3h.dtb >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-orangepi-zero2w.dtb >> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi >> new file mode 100644 >> index 000000000000..815cf2dac24b >> --- /dev/null >> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi >> @@ -0,0 +1,109 @@ >> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) >> +/* >> + * Copyright (C) 2024 Kamil Kasperski <ressetkk@gmail.com> >> + * >> + * Common DT nodes for H616-based T95 TV boxes >> + * There are two versions reported with different PMIC variants. >> + */ >> + >> +#include "sun50i-h616.dtsi" >> + >> +#include <dt-bindings/gpio/gpio.h> >> +#include <dt-bindings/interrupt-controller/arm-gic.h> >> + >> +/ { >> + aliases { >> + ethernet1 = &sdio_wifi; >> + serial0 = &uart0; >> + }; >> + >> + chosen { >> + stdout-path = "serial0:115200n8"; >> + }; >> + >> + reg_vcc5v: vcc5v { >> + /* board wide 5V supply directly from the DC input */ >> + compatible = "regulator-fixed"; >> + regulator-name = "vcc-5v"; >> + regulator-min-microvolt = <5000000>; >> + regulator-max-microvolt = <5000000>; >> + regulator-always-on; >> + }; >> + >> + reg_vcc3v3: vcc3v3 { >> + /* discrete 3.3V regulator */ >> + compatible = "regulator-fixed"; >> + regulator-name = "vcc-3v3"; >> + regulator-min-microvolt = <3300000>; >> + regulator-max-microvolt = <3300000>; >> + regulator-always-on; >> + }; >> + >> + wifi_pwrseq: wifi-pwrseq { > Krzysztof recently sent a patch to just use "pwrseq" as the node name, > so can you do the same here? > > https://lore.kernel.org/linux-sunxi/20240317184130.157695-2-krzysztof.kozlowski@linaro.org/T/#u Sure. I'll send v4. > >> + compatible = "mmc-pwrseq-simple"; >> + clocks = <&rtc CLK_OSC32K_FANOUT>; >> + clock-names = "ext_clock"; >> + pinctrl-0 = <&x32clk_fanout_pin>; >> + pinctrl-names = "default"; >> + reset-gpios = <&pio 6 18 GPIO_ACTIVE_LOW>; /* PG18 */ >> + }; >> +}; >> + >> +&ehci0 { >> + status = "okay"; >> +}; >> + >> +&ehci2 { >> + status = "okay"; >> +}; >> + >> +&ir { >> + status = "okay"; >> +}; >> + >> +&mmc0 { >> + cd-gpios = <&pio 8 16 GPIO_ACTIVE_LOW>; /* PI16 */ >> + bus-width = <4>; >> + status = "okay"; >> +}; >> + >> +&mmc1 { >> + mmc-pwrseq = <&wifi_pwrseq>; >> + bus-width = <4>; >> + non-removable; >> + status = "okay"; >> + >> + sdio_wifi: wifi@1 { >> + reg = <1>; >> + }; > So does the WiFi work with mainline drivers? IIUC the BCM43342 is not > supported by the existing Broadcom drivers? It's actually BCM43342/1 It System doesn't detect this chip by default. The most relevant message from dmesg is: [ 14.042035] kernel: brcmfmac: brcmf_fw_alloc_request: Unknown chip BCM43342/1 I believe that it's only a matter of missing module. I don't think it is supported in mainline ATM. I left it to have a wi-fi node accessible and detectable by kernel. If you think that it's better to remove the node if it's not supported I can do it. Somebody actually extracted modified precompiled module from custom 5.15.16 rockchip kernel, which implements support for this card. There's no patch for it that could be submitted to mainline unfortunately ATM. I've found a patch that adds chip id strings to brcmfmac, but I would like to test it beforehand. > >> +}; >> + >> +&ohci0 { >> + status = "okay"; >> +}; >> + >> +&ohci2 { >> + status = "okay"; >> +}; >> + >> +&uart0 { >> + pinctrl-names = "default"; >> + pinctrl-0 = <&uart0_ph_pins>; >> + status = "okay"; >> +}; >> + >> +&uart1 { >> + pinctrl-names = "default"; >> + pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>; >> + uart-has-rtscts; >> + status = "okay"; > Similar question here, what is the Bluetooth situation in mainline? I > guess it's not supported, since you didn't put a BT node in here? I haven't tested that yet. It's partially due to lack of experience with DTS. I would like to figure out mainline support eventually, but for now I would like to just have a working foundation for these TVB. If that's also not really a wanted option to leave those nodes, I can remove them for now. > >> +}; >> + >> +&usbotg { >> + dr_mode = "host"; /* USB A type receptable */ >> + status = "okay"; >> +}; >> + >> +&usbphy { >> + status = "okay"; >> +}; >> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-t95max-axp313.dts b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95max-axp313.dts >> new file mode 100644 >> index 000000000000..c8650aca2407 >> --- /dev/null >> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95max-axp313.dts >> @@ -0,0 +1,85 @@ >> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) >> +/* >> + * Copyright (C) 2024 Kamil Kasperski <ressetkk@gmail.com> >> + * >> + * Configuration for T95 TV box with board label H616-T95MAX-AXP313A-v3.0 >> + */ >> + >> +/dts-v1/; >> + >> +#include "sun50i-h616-t95.dtsi" >> + >> +/ { >> + model = "T95 5G (AXP313)"; >> + compatible = "t95,t95max-axp313", "allwinner,sun50i-h616"; >> +}; >> + >> +&mmc0 { >> + vmmc-supply = <®_dldo1>; >> +}; >> + >> +&mmc1 { >> + vmmc-supply = <®_dldo1>; >> + vqmmc-supply = <®_aldo1>; >> +}; >> + >> +&r_i2c { >> + status = "okay"; >> + >> + axp313: pmic@36 { >> + compatible = "x-powers,axp313a"; >> + reg = <0x36>; >> + #interrupt-cells = <1>; >> + interrupt-controller; >> + interrupt-parent = <&pio>; > I don't think you need interrupt-parent unless you also actually specify > an interrupt line. > (But please keep #interrupt-cells and interrupt-controller.) I think you may be right. I based this part off of OPiZ3 dts, rather than transpeed. Will update in the v4. > >> + >> + vin1-supply = <®_vcc5v>; >> + vin2-supply = <®_vcc5v>; >> + vin3-supply = <®_vcc5v>; >> + >> + regulators { >> + reg_aldo1: aldo1 { >> + regulator-always-on; >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <1800000>; >> + regulator-name = "vcc1v8"; >> + }; >> + >> + reg_dldo1: dldo1 { >> + regulator-always-on; >> + regulator-min-microvolt = <3300000>; >> + regulator-max-microvolt = <3300000>; >> + regulator-name = "vcc3v3"; >> + }; >> + >> + reg_dcdc1: dcdc1 { >> + regulator-always-on; >> + regulator-min-microvolt = <810000>; >> + regulator-max-microvolt = <990000>; >> + regulator-name = "vdd-gpu-sys"; >> + }; >> + >> + reg_dcdc2: dcdc2 { >> + regulator-always-on; >> + regulator-min-microvolt = <810000>; >> + regulator-max-microvolt = <1100000>; >> + regulator-name = "vdd-cpu"; >> + }; >> + >> + reg_dcdc3: dcdc3 { >> + regulator-always-on; >> + regulator-min-microvolt = <1500000>; >> + regulator-max-microvolt = <1500000>; >> + regulator-name = "vdd-dram"; >> + }; >> + }; >> + }; >> +}; >> + >> +&pio { >> + vcc-pc-supply = <®_aldo1>; >> + vcc-pf-supply = <®_dldo1>; >> + vcc-pg-supply = <®_dldo1>; > So if vqmmc-supply for MMC1 is at the 1.8V from ALDO1, that must mean that > the whole of PortG is at 1.8V, right? So I think this would need to be > reg_aldo1 here. > > Apart from those minor things it looks good to me. > > Cheers, > Andre Good catch. Will update in v4. Cheers, Kamil >> + vcc-ph-supply = <®_dldo1>; >> + vcc-pi-supply = <®_dldo1>; >> +}; >>
On Tue, 19 Mar 2024 00:41:33 +0530 Amit Singh Tomar <amitsinght@marvell.com> wrote: Hi, > >> > >> With the DT bindings now describing the format of the CPU OPP tables, we can include the OPP table in each board's .dts file, and specify the CPU power supply. > >> This allows to enable DVFS, and get up to 50% of performance benefit in the highest OPP, or up to 60% power savings in the lowest OPP, compared to the fixed 1GHz @ 1.0V OPP we are running in by default > >> at the moment. > >> [Amit] Could you please elaborate, what test were run to see 50 % performance benefits? > > > > Currently all H616 boards running mainline firmware and kernels run at a > > fixed 1GHz CPU clock frequency. If you happen to have a good SoC (bin 1 or > > 3), this patchset will allow you to run at 1.5 GHz, which is 50% faster. > > So anything that scales with CPU frequency should run much quicker. > > > Okay, it would be interesting to see results of some benchmark here. But why? This is not a performance optimisation, it's adding a missing feature, because the CPU was locked to 1 GHz before, for safety reasons, due to missing thermal and DVFS capability. Now it's able to run at up to 1.5 GHz, as specified. If you are upset about the bold claim, I can just remove it from the commit message, it was just a heads up that we were leaving a lot of performance on the table at the moment. Posting absolute performance numbers is a tricky subject, legally, so please run your own: http://www.netlib.org/benchmark/linpackc.new $ musl-gcc -s -static -Ofast -o linpack linpack.c -lm -march=native -fno-math-errno -funsafe-math-optimizations $ gcc --version gcc (Debian 12.2.0-14) 12.2.0 # echo userspace > /sys/devices/system/cpu/cpufreq/policy0/scaling_governor # echo 1008000 > /sys/devices/system/cpu/cpufreq/policy0/scaling_setspeed $ ./linpack # echo performance > /sys/devices/system/cpu/cpufreq/policy0/scaling_governor $ ./linpack $ dc -e '3k $high $low /p' 1.498 Cheers, Andre
>>
>> -----Original Message-----
>> From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org> On Behalf Of Andre Przywara
>> Sent: Monday, March 18, 2024 6:42 AM
>> To: Yangtao Li <tiny.windzz@gmail.com>; Viresh Kumar <vireshk@kernel.org>; Nishanth Menon <nm@ti.com>; Stephen Boyd <sboyd@kernel.org>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; Chen-Yu Tsai <wens@csie.org>; Jernej Skrabec <jernej.skrabec@gmail.com>; Samuel Holland <samuel@sholland.org>; Rafael J . Wysocki <rafael@kernel.org>
>> Cc: linux-pm@vger.kernel.org; devicetree@vger.kernel.org; linux-sunxi@lists.linux.dev; linux-arm-kernel@lists.infradead.org; Brandon Cheo Fusi <fusibrandon13@gmail.com>; Martin Botka <martin.botka@somainline.org>; Martin Botka <martin.botka1@gmail.com>
>> Subject: [EXTERNAL] [PATCH v2 8/8] arm64: dts: allwinner: h616: enable DVFS for all boards
>>
>>
>> With the DT bindings now describing the format of the CPU OPP tables, we can include the OPP table in each board's .dts file, and specify the CPU power supply.
>> This allows to enable DVFS, and get up to 50% of performance benefit in the highest OPP, or up to 60% power savings in the lowest OPP, compared to the fixed 1GHz @ 1.0V OPP we are running in by default
>> at the moment.
>> [Amit] Could you please elaborate, what test were run to see 50 % performance benefits?
>
> Currently all H616 boards running mainline firmware and kernels run at a
> fixed 1GHz CPU clock frequency. If you happen to have a good SoC (bin 1 or
> 3), this patchset will allow you to run at 1.5 GHz, which is 50% faster.
> So anything that scales with CPU frequency should run much quicker.
>
Okay, it would be interesting to see results of some benchmark here.
Thanks
-Amit
On Mon, Mar 18, 2024 at 02:49:47PM +0100, Maxime Ripard wrote:
> Hi,
>
> On Fri, Mar 15, 2024 at 10:22:05AM +0200, Ville Syrjälä wrote:
> > On Mon, Mar 11, 2024 at 03:49:48PM +0100, Maxime Ripard wrote:
> > > Infoframes in KMS is usually handled by a bunch of low-level helpers
> > > that require quite some boilerplate for drivers. This leads to
> > > discrepancies with how drivers generate them, and which are actually
> > > sent.
> > >
> > > Now that we have everything needed to generate them in the HDMI
> > > connector state, we can generate them in our common logic so that
> > > drivers can simply reuse what we precomputed.
> > >
> > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > ---
> > > drivers/gpu/drm/Kconfig | 1 +
> > > drivers/gpu/drm/drm_atomic_state_helper.c | 323 +++++++++++++++++++++
> > > drivers/gpu/drm/drm_connector.c | 14 +
> > > .../gpu/drm/tests/drm_atomic_state_helper_test.c | 1 +
> > > drivers/gpu/drm/tests/drm_connector_test.c | 12 +
> > > include/drm/drm_atomic_state_helper.h | 8 +
> > > include/drm/drm_connector.h | 133 +++++++++
> > > 7 files changed, 492 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > > index 872edb47bb53..ad9c467e20ce 100644
> > > --- a/drivers/gpu/drm/Kconfig
> > > +++ b/drivers/gpu/drm/Kconfig
> > > @@ -97,10 +97,11 @@ config DRM_KUNIT_TEST
> > > If in doubt, say "N".
> > >
> > > config DRM_KMS_HELPER
> > > tristate
> > > depends on DRM
> > > + select DRM_DISPLAY_HDMI_HELPER
> > > help
> > > CRTC helpers for KMS drivers.
> > >
> > > config DRM_DEBUG_DP_MST_TOPOLOGY_REFS
> > > bool "Enable refcount backtrace history in the DP MST helpers"
> > > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> > > index e66272c0d006..2bf53666fc9d 100644
> > > --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> > > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> > > @@ -36,10 +36,12 @@
> > > #include <drm/drm_plane.h>
> > > #include <drm/drm_print.h>
> > > #include <drm/drm_vblank.h>
> > > #include <drm/drm_writeback.h>
> > >
> > > +#include <drm/display/drm_hdmi_helper.h>
> > > +
> > > #include <linux/slab.h>
> > > #include <linux/dma-fence.h>
> > >
> > > /**
> > > * DOC: atomic state reset and initialization
> > > @@ -912,10 +914,143 @@ hdmi_compute_config(const struct drm_connector *connector,
> > > }
> > >
> > > return -EINVAL;
> > > }
> > >
> > > +static int hdmi_generate_avi_infoframe(const struct drm_connector *connector,
> > > + struct drm_connector_state *state)
> > > +{
> > > + const struct drm_display_mode *mode =
> > > + connector_state_get_mode(state);
> > > + struct drm_connector_hdmi_infoframe *infoframe =
> > > + &state->hdmi.infoframes.avi;
> > > + struct hdmi_avi_infoframe *frame =
> > > + &infoframe->data.avi;
> > > + bool is_full_range = state->hdmi.is_full_range;
> > > + enum hdmi_quantization_range rgb_quant_range =
> > > + is_full_range ? HDMI_QUANTIZATION_RANGE_FULL : HDMI_QUANTIZATION_RANGE_LIMITED;
> > > + int ret;
> > > +
> > > + ret = drm_hdmi_avi_infoframe_from_display_mode(frame, connector, mode);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + frame->colorspace = state->hdmi.output_format;
> > > +
> > > + drm_hdmi_avi_infoframe_quant_range(frame, connector, mode, rgb_quant_range);
> >
> > drm_hdmi_avi_infoframe_quant_range() doesn't handle YCbCr currently.
>
> I guess it's not really a problem anymore if we drop YUV422 selection,
> but I'll add a comment.
>
> > > + drm_hdmi_avi_infoframe_colorimetry(frame, state);
> > > + drm_hdmi_avi_infoframe_bars(frame, state);
> > > +
> > > + infoframe->set = true;
> > > +
> > > + return 0;
> > > +}
> > > +
> > <snip>
> > > +
> > > +#define UPDATE_INFOFRAME(c, os, ns, i) \
> > > + write_or_clear_infoframe(c, \
> > > + &(c)->hdmi.infoframes.i, \
> > > + &(os)->hdmi.infoframes.i, \
> > > + &(ns)->hdmi.infoframes.i)
> >
> > This macro feels like pointless obfuscation to me.
>
> I'll remove it then.
>
> > <snip>
> > > @@ -1984,20 +2063,73 @@ struct drm_connector {
> > >
> > > /**
> > > * @hdmi: HDMI-related variable and properties.
> > > */
> > > struct {
> > > +#define DRM_CONNECTOR_HDMI_VENDOR_LEN 8
> > > + /**
> > > + * @vendor: HDMI Controller Vendor Name
> > > + */
> > > + unsigned char vendor[DRM_CONNECTOR_HDMI_VENDOR_LEN] __nonstring;
> > > +
> > > +#define DRM_CONNECTOR_HDMI_PRODUCT_LEN 16
> > > + /**
> > > + * @product: HDMI Controller Product Name
> > > + */
> > > + unsigned char product[DRM_CONNECTOR_HDMI_PRODUCT_LEN] __nonstring;
> > > +
> > > /**
> > > * @supported_formats: Bitmask of @hdmi_colorspace
> > > * supported by the controller.
> > > */
> > > unsigned long supported_formats;
> > >
> > > /**
> > > * @funcs: HDMI connector Control Functions
> > > */
> > > const struct drm_connector_hdmi_funcs *funcs;
> > > +
> > > + /**
> > > + * @infoframes: Current Infoframes output by the connector
> > > + */
> > > + struct {
> > > + /**
> > > + * @lock: Mutex protecting against concurrent access to
> > > + * the infoframes, most notably between KMS and ALSA.
> > > + */
> > > + struct mutex lock;
> > > +
> > > + /**
> > > + * @audio: Current Audio Infoframes structure. Protected
> > > + * by @lock.
> > > + */
> > > + struct drm_connector_hdmi_infoframe audio;
> > > +
> > > + /**
> > > + * @avi: Current AVI Infoframes structure. Protected by
> > > + * @lock.
> > > + */
> > > + struct drm_connector_hdmi_infoframe avi;
> > > +
> > > + /**
> > > + * @hdr_drm: Current DRM (Dynamic Range and Mastering)
> > > + * Infoframes structure. Protected by @lock.
> > > + */
> > > + struct drm_connector_hdmi_infoframe hdr_drm;
> > > +
> > > + /**
> > > + * @spd: Current SPD Infoframes structure. Protected by
> > > + * @lock.
> > > + */
> > > + struct drm_connector_hdmi_infoframe spd;
> > > +
> > > + /**
> > > + * @vendor: Current HDMI Vendor Infoframes structure.
> > > + * Protected by @lock.
> > > + */
> > > + struct drm_connector_hdmi_infoframe hdmi;
> > > + } infoframes;
> > > } hdmi;
> >
> > What's the deal with this bloat? These are already tracked in the
> > connector's state so this looks entirely redundant.
>
> The next patch in this series is about adding debugfs entries to read
> the infoframes, and thus we need to care about concurrency between
> debugfs files accesses and commits. Copying the things we care about
> from the state to the entity is the typical solution for that, but I
> guess we could also take the proper locks and access the current
> connector state.
Yeah, just lock and dump the latest state. That is the only thing
that should of interest to anyone in userspace.
Also are you actually adding some kind of ad-hoc state dump things
just for these? Why not do whatever is needed to include them in
the normal .atomic_state_print() stuff?
--
Ville Syrjälä
Intel
[-- Attachment #1: Type: text/plain, Size: 6605 bytes --] Hi, On Fri, Mar 15, 2024 at 10:22:05AM +0200, Ville Syrjälä wrote: > On Mon, Mar 11, 2024 at 03:49:48PM +0100, Maxime Ripard wrote: > > Infoframes in KMS is usually handled by a bunch of low-level helpers > > that require quite some boilerplate for drivers. This leads to > > discrepancies with how drivers generate them, and which are actually > > sent. > > > > Now that we have everything needed to generate them in the HDMI > > connector state, we can generate them in our common logic so that > > drivers can simply reuse what we precomputed. > > > > Signed-off-by: Maxime Ripard <mripard@kernel.org> > > --- > > drivers/gpu/drm/Kconfig | 1 + > > drivers/gpu/drm/drm_atomic_state_helper.c | 323 +++++++++++++++++++++ > > drivers/gpu/drm/drm_connector.c | 14 + > > .../gpu/drm/tests/drm_atomic_state_helper_test.c | 1 + > > drivers/gpu/drm/tests/drm_connector_test.c | 12 + > > include/drm/drm_atomic_state_helper.h | 8 + > > include/drm/drm_connector.h | 133 +++++++++ > > 7 files changed, 492 insertions(+) > > > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > > index 872edb47bb53..ad9c467e20ce 100644 > > --- a/drivers/gpu/drm/Kconfig > > +++ b/drivers/gpu/drm/Kconfig > > @@ -97,10 +97,11 @@ config DRM_KUNIT_TEST > > If in doubt, say "N". > > > > config DRM_KMS_HELPER > > tristate > > depends on DRM > > + select DRM_DISPLAY_HDMI_HELPER > > help > > CRTC helpers for KMS drivers. > > > > config DRM_DEBUG_DP_MST_TOPOLOGY_REFS > > bool "Enable refcount backtrace history in the DP MST helpers" > > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c > > index e66272c0d006..2bf53666fc9d 100644 > > --- a/drivers/gpu/drm/drm_atomic_state_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c > > @@ -36,10 +36,12 @@ > > #include <drm/drm_plane.h> > > #include <drm/drm_print.h> > > #include <drm/drm_vblank.h> > > #include <drm/drm_writeback.h> > > > > +#include <drm/display/drm_hdmi_helper.h> > > + > > #include <linux/slab.h> > > #include <linux/dma-fence.h> > > > > /** > > * DOC: atomic state reset and initialization > > @@ -912,10 +914,143 @@ hdmi_compute_config(const struct drm_connector *connector, > > } > > > > return -EINVAL; > > } > > > > +static int hdmi_generate_avi_infoframe(const struct drm_connector *connector, > > + struct drm_connector_state *state) > > +{ > > + const struct drm_display_mode *mode = > > + connector_state_get_mode(state); > > + struct drm_connector_hdmi_infoframe *infoframe = > > + &state->hdmi.infoframes.avi; > > + struct hdmi_avi_infoframe *frame = > > + &infoframe->data.avi; > > + bool is_full_range = state->hdmi.is_full_range; > > + enum hdmi_quantization_range rgb_quant_range = > > + is_full_range ? HDMI_QUANTIZATION_RANGE_FULL : HDMI_QUANTIZATION_RANGE_LIMITED; > > + int ret; > > + > > + ret = drm_hdmi_avi_infoframe_from_display_mode(frame, connector, mode); > > + if (ret) > > + return ret; > > + > > + frame->colorspace = state->hdmi.output_format; > > + > > + drm_hdmi_avi_infoframe_quant_range(frame, connector, mode, rgb_quant_range); > > drm_hdmi_avi_infoframe_quant_range() doesn't handle YCbCr currently. I guess it's not really a problem anymore if we drop YUV422 selection, but I'll add a comment. > > + drm_hdmi_avi_infoframe_colorimetry(frame, state); > > + drm_hdmi_avi_infoframe_bars(frame, state); > > + > > + infoframe->set = true; > > + > > + return 0; > > +} > > + > <snip> > > + > > +#define UPDATE_INFOFRAME(c, os, ns, i) \ > > + write_or_clear_infoframe(c, \ > > + &(c)->hdmi.infoframes.i, \ > > + &(os)->hdmi.infoframes.i, \ > > + &(ns)->hdmi.infoframes.i) > > This macro feels like pointless obfuscation to me. I'll remove it then. > <snip> > > @@ -1984,20 +2063,73 @@ struct drm_connector { > > > > /** > > * @hdmi: HDMI-related variable and properties. > > */ > > struct { > > +#define DRM_CONNECTOR_HDMI_VENDOR_LEN 8 > > + /** > > + * @vendor: HDMI Controller Vendor Name > > + */ > > + unsigned char vendor[DRM_CONNECTOR_HDMI_VENDOR_LEN] __nonstring; > > + > > +#define DRM_CONNECTOR_HDMI_PRODUCT_LEN 16 > > + /** > > + * @product: HDMI Controller Product Name > > + */ > > + unsigned char product[DRM_CONNECTOR_HDMI_PRODUCT_LEN] __nonstring; > > + > > /** > > * @supported_formats: Bitmask of @hdmi_colorspace > > * supported by the controller. > > */ > > unsigned long supported_formats; > > > > /** > > * @funcs: HDMI connector Control Functions > > */ > > const struct drm_connector_hdmi_funcs *funcs; > > + > > + /** > > + * @infoframes: Current Infoframes output by the connector > > + */ > > + struct { > > + /** > > + * @lock: Mutex protecting against concurrent access to > > + * the infoframes, most notably between KMS and ALSA. > > + */ > > + struct mutex lock; > > + > > + /** > > + * @audio: Current Audio Infoframes structure. Protected > > + * by @lock. > > + */ > > + struct drm_connector_hdmi_infoframe audio; > > + > > + /** > > + * @avi: Current AVI Infoframes structure. Protected by > > + * @lock. > > + */ > > + struct drm_connector_hdmi_infoframe avi; > > + > > + /** > > + * @hdr_drm: Current DRM (Dynamic Range and Mastering) > > + * Infoframes structure. Protected by @lock. > > + */ > > + struct drm_connector_hdmi_infoframe hdr_drm; > > + > > + /** > > + * @spd: Current SPD Infoframes structure. Protected by > > + * @lock. > > + */ > > + struct drm_connector_hdmi_infoframe spd; > > + > > + /** > > + * @vendor: Current HDMI Vendor Infoframes structure. > > + * Protected by @lock. > > + */ > > + struct drm_connector_hdmi_infoframe hdmi; > > + } infoframes; > > } hdmi; > > What's the deal with this bloat? These are already tracked in the > connector's state so this looks entirely redundant. The next patch in this series is about adding debugfs entries to read the infoframes, and thus we need to care about concurrency between debugfs files accesses and commits. Copying the things we care about from the state to the entity is the typical solution for that, but I guess we could also take the proper locks and access the current connector state. Maxime [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --]
On Mon, Mar 18, 2024 at 01:05:22PM +0100, Maxime Ripard wrote: > Hi Ville, > > Thanks for your review ! > > On Fri, Mar 15, 2024 at 10:05:16AM +0200, Ville Syrjälä wrote: > > On Mon, Mar 11, 2024 at 03:49:42PM +0100, Maxime Ripard wrote: > > > +static bool > > > +sink_supports_format_bpc(const struct drm_connector *connector, > > > + const struct drm_display_info *info, > > > + const struct drm_display_mode *mode, > > > + unsigned int format, unsigned int bpc) > > > +{ > > > + struct drm_device *dev = connector->dev; > > > + u8 vic = drm_match_cea_mode(mode); > > > + > > > + if (vic == 1 && bpc != 8) { > > > + drm_dbg(dev, "VIC1 requires a bpc of 8, got %u\n", bpc); > > > > Use of drm_dbg() for kms stuff is surprising. > > > > > + return false; > > > + } > > > > I don't think we have this in i915. My original impression was that you > > can use higher color depth if you can determine the sink capabilities, > > but all sinks are required to accept 640x480@8bpc as a fallback. > > > > but CTA-861-H says: > > "5.4 Color Coding & Quantization > > Component Depth: The coding shall be N-bit, where N = 8, 10, 12, or 16 > > bits/component — except in the case of the default 640x480 Video Timing 1, > > where the value of N shall be 8." > > > > So that does seem to imply that you're supposed to use exactly 8bpc. > > Though the word "default" in there is confusing. Are they specifically > > using that to indicate that this is about the fallback behaviour, or > > is it just indicating that it is a "default mode that always has to > > be supported". Dunno. I guess no real harm in forcing 8bpc for 640x480 > > since no one is likely to use that for any high fidelity stuff. > > My understanding was that CTA-861 mandates that 640x480@60Hz is > supported, and mentions it being the default timing on a few occurences, > like in section 4 - Video Formats and Waveform Timings that states "This > section describes the default IT 640x480 Video Timing as well as all of > the standard CE Video Timings.", or Section 6.2 - Describing Video > Formats in EDID "The 640x480@60Hz flag, in the Established Timings area, > shall always be set, since the 640x480p format is a mandatory default > timing." > > So my understanding is that default here applies to the timing itself, > and not the bpc, and is thus the second interpretation you suggested. > > I'll add a comment to make it clearer. > > > > +static int > > > +hdmi_compute_format(const struct drm_connector *connector, > > > + struct drm_connector_state *state, > > > + const struct drm_display_mode *mode, > > > + unsigned int bpc) > > > +{ > > > + struct drm_device *dev = connector->dev; > > > + > > > + if (hdmi_try_format_bpc(connector, state, mode, bpc, HDMI_COLORSPACE_RGB)) { > > > + state->hdmi.output_format = HDMI_COLORSPACE_RGB; > > > + return 0; > > > + } > > > + > > > + if (hdmi_try_format_bpc(connector, state, mode, bpc, HDMI_COLORSPACE_YUV422)) { > > > + state->hdmi.output_format = HDMI_COLORSPACE_YUV422; > > > + return 0; > > > + } > > > > Looks like you're preferring YCbCr 4:2:2 over RGB 8bpc. Not sure > > if that's a good tradeoff to make. > > Yeah, indeed. I guess it's a judgement call on whether we prioritise > lowering the bpc over selecting YUV422, but I guess I can try all > available RGB bpc before falling back to YUV422. > > > In i915 we don't currently expose 4:2:2 at all because it doesn't > > help in getting a working display, and we have no uapi for the > > user to force it if they really want 4:2:2 over RGB. > > I guess if the priority is given to lowering bpc, then it indeed doesn't > make sense to support YUV422, since the limiting factor is likely to be > the TMDS char rate and YUV422 12 bpc is equivalent to RGB 8bpc there. > > dw-hdmi on the other hand will always put YUV422 and YUV444 before RGB > for a given bpc, which is weird to me: > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c#L2696 > > What is even weirder to me is that YUV422 is explicitly stated to be > 12bpc only, so there's some invalid configurations there (8 and 10 bpc). > > And given that it's order by decreasing order of preference, I'm pretty > sure it'll never actually pick any YUV or RGB > 8bpc format since RGB > 8bpc is super likely to be always available and thus picked first. 8bpc RGB is in fact mandatory. > > If we want to converge, I think we should amend this code to support > YUV420 for YUV420-only modes first, and then the RGB options like i915 > is doing. And then if someone is interested in more, we can always > expand it to other formats. > > > > + > > > + drm_dbg(dev, "Failed. No Format Supported for that bpc count.\n"); > > > + > > > + return -EINVAL; > > > +} > > > + > > > +static int > > > +hdmi_compute_config(const struct drm_connector *connector, > > > + struct drm_connector_state *state, > > > + const struct drm_display_mode *mode) > > > +{ > > > + struct drm_device *dev = connector->dev; > > > + unsigned int max_bpc = clamp_t(unsigned int, > > > + state->max_bpc, > > > + 8, connector->max_bpc); > > > + unsigned int bpc; > > > + int ret; > > > + > > > + for (bpc = max_bpc; bpc >= 8; bpc -= 2) { > > > + drm_dbg(dev, "Trying with a %d bpc output\n", bpc); > > > + > > > + ret = hdmi_compute_format(connector, state, mode, bpc); > > > > Hmm. Actually I'm not sure your 4:2:2 stuff even works since you > > check for bpc==12 in there and only call this based on the max_bpc. > > I'm not convinced max_bpc would actually be 12 for a sink that > > supports YCbCr 4:2:2 but not 12bpc RGB. > > It's another discussion we had in an earlier version, but yeah we lack > the infrastructure to support those for now. I still believe it would > require an increased max_bpc to select YUV422, otherwise things would be > pretty inconsistent with other YUV formats. Ideally I'd like to know the actual color depth of the panel independently of the supported signal color depths. Unfortunately I don't think EDID gives us that. Can't recall if DisplayID might have something a bit more sensible. Given how info->bpc works right now, I suppose it would make sense to bump it up to 12 when 4:2:2 is supported. But I've not thought through the actual implications such a change. > But yeah, we need to provide a hook to report we don't support RGB > > 8bpc for HDMI 1.4 devices. Which goes back to the previous question > actually, I believe it would still provide value to support YUV422 on > those devices, with something like: > > for (bpc = max_bpc; bpc >= 8; bpc -= 2) { > if (!connector->hdmi->funcs->validate_config(mode, RGB, bpc)) > continue; > > // Select RGB with bpc > ... > } > > if (connector->hdmi->funcs->validate_config(mode, YUV) && > hdmi_try_format_bpc(..., mode, 12, YUV422) { > // Select YUV422, 12 bpc > ... > } > > What do you think? Since 8bpc RGB must always be supported this looks like dead code to me. -- Ville Syrjälä Intel
[-- Attachment #1: Type: text/plain, Size: 6289 bytes --] Hi Ville, Thanks for your review ! On Fri, Mar 15, 2024 at 10:05:16AM +0200, Ville Syrjälä wrote: > On Mon, Mar 11, 2024 at 03:49:42PM +0100, Maxime Ripard wrote: > > +static bool > > +sink_supports_format_bpc(const struct drm_connector *connector, > > + const struct drm_display_info *info, > > + const struct drm_display_mode *mode, > > + unsigned int format, unsigned int bpc) > > +{ > > + struct drm_device *dev = connector->dev; > > + u8 vic = drm_match_cea_mode(mode); > > + > > + if (vic == 1 && bpc != 8) { > > + drm_dbg(dev, "VIC1 requires a bpc of 8, got %u\n", bpc); > > Use of drm_dbg() for kms stuff is surprising. > > > + return false; > > + } > > I don't think we have this in i915. My original impression was that you > can use higher color depth if you can determine the sink capabilities, > but all sinks are required to accept 640x480@8bpc as a fallback. > > but CTA-861-H says: > "5.4 Color Coding & Quantization > Component Depth: The coding shall be N-bit, where N = 8, 10, 12, or 16 > bits/component — except in the case of the default 640x480 Video Timing 1, > where the value of N shall be 8." > > So that does seem to imply that you're supposed to use exactly 8bpc. > Though the word "default" in there is confusing. Are they specifically > using that to indicate that this is about the fallback behaviour, or > is it just indicating that it is a "default mode that always has to > be supported". Dunno. I guess no real harm in forcing 8bpc for 640x480 > since no one is likely to use that for any high fidelity stuff. My understanding was that CTA-861 mandates that 640x480@60Hz is supported, and mentions it being the default timing on a few occurences, like in section 4 - Video Formats and Waveform Timings that states "This section describes the default IT 640x480 Video Timing as well as all of the standard CE Video Timings.", or Section 6.2 - Describing Video Formats in EDID "The 640x480@60Hz flag, in the Established Timings area, shall always be set, since the 640x480p format is a mandatory default timing." So my understanding is that default here applies to the timing itself, and not the bpc, and is thus the second interpretation you suggested. I'll add a comment to make it clearer. > > +static int > > +hdmi_compute_format(const struct drm_connector *connector, > > + struct drm_connector_state *state, > > + const struct drm_display_mode *mode, > > + unsigned int bpc) > > +{ > > + struct drm_device *dev = connector->dev; > > + > > + if (hdmi_try_format_bpc(connector, state, mode, bpc, HDMI_COLORSPACE_RGB)) { > > + state->hdmi.output_format = HDMI_COLORSPACE_RGB; > > + return 0; > > + } > > + > > + if (hdmi_try_format_bpc(connector, state, mode, bpc, HDMI_COLORSPACE_YUV422)) { > > + state->hdmi.output_format = HDMI_COLORSPACE_YUV422; > > + return 0; > > + } > > Looks like you're preferring YCbCr 4:2:2 over RGB 8bpc. Not sure > if that's a good tradeoff to make. Yeah, indeed. I guess it's a judgement call on whether we prioritise lowering the bpc over selecting YUV422, but I guess I can try all available RGB bpc before falling back to YUV422. > In i915 we don't currently expose 4:2:2 at all because it doesn't > help in getting a working display, and we have no uapi for the > user to force it if they really want 4:2:2 over RGB. I guess if the priority is given to lowering bpc, then it indeed doesn't make sense to support YUV422, since the limiting factor is likely to be the TMDS char rate and YUV422 12 bpc is equivalent to RGB 8bpc there. dw-hdmi on the other hand will always put YUV422 and YUV444 before RGB for a given bpc, which is weird to me: https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c#L2696 What is even weirder to me is that YUV422 is explicitly stated to be 12bpc only, so there's some invalid configurations there (8 and 10 bpc). And given that it's order by decreasing order of preference, I'm pretty sure it'll never actually pick any YUV or RGB > 8bpc format since RGB 8bpc is super likely to be always available and thus picked first. If we want to converge, I think we should amend this code to support YUV420 for YUV420-only modes first, and then the RGB options like i915 is doing. And then if someone is interested in more, we can always expand it to other formats. > > + > > + drm_dbg(dev, "Failed. No Format Supported for that bpc count.\n"); > > + > > + return -EINVAL; > > +} > > + > > +static int > > +hdmi_compute_config(const struct drm_connector *connector, > > + struct drm_connector_state *state, > > + const struct drm_display_mode *mode) > > +{ > > + struct drm_device *dev = connector->dev; > > + unsigned int max_bpc = clamp_t(unsigned int, > > + state->max_bpc, > > + 8, connector->max_bpc); > > + unsigned int bpc; > > + int ret; > > + > > + for (bpc = max_bpc; bpc >= 8; bpc -= 2) { > > + drm_dbg(dev, "Trying with a %d bpc output\n", bpc); > > + > > + ret = hdmi_compute_format(connector, state, mode, bpc); > > Hmm. Actually I'm not sure your 4:2:2 stuff even works since you > check for bpc==12 in there and only call this based on the max_bpc. > I'm not convinced max_bpc would actually be 12 for a sink that > supports YCbCr 4:2:2 but not 12bpc RGB. It's another discussion we had in an earlier version, but yeah we lack the infrastructure to support those for now. I still believe it would require an increased max_bpc to select YUV422, otherwise things would be pretty inconsistent with other YUV formats. But yeah, we need to provide a hook to report we don't support RGB > 8bpc for HDMI 1.4 devices. Which goes back to the previous question actually, I believe it would still provide value to support YUV422 on those devices, with something like: for (bpc = max_bpc; bpc >= 8; bpc -= 2) { if (!connector->hdmi->funcs->validate_config(mode, RGB, bpc)) continue; // Select RGB with bpc ... } if (connector->hdmi->funcs->validate_config(mode, YUV) && hdmi_try_format_bpc(..., mode, 12, YUV422) { // Select YUV422, 12 bpc ... } What do you think? Maxime [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --]
On Sun, 17 Mar 2024 19:41:27 +0100 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: Hi Krzysztof, > There is no "reg_gmac_3v3" device node in sun50i-h6-pine-h64.dts, > although there is "gmac-3v3" with "reg_gmac_3v3" label, so let's assume > author wanted to remove that node. Delete node via phandle, not via > full node path, to fix this. Ah, that's a good catch! Indeed that regulator node is still in the DTB right now. > Fixes: f33a91175029 ("arm64: dts: allwinner: add pineh64 model B") > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Successfully compiled the DTB and can confirm that the node is gone now. Reviewed-by: Andre Przywara <andre.przywara@arm.com> Cheers, Andre > --- > arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-model-b.dts | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-model-b.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-model-b.dts > index b710f1a0f53a..1b6e5595ac6e 100644 > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-model-b.dts > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-model-b.dts > @@ -5,12 +5,12 @@ > > #include "sun50i-h6-pine-h64.dts" > > +/delete-node/ ®_gmac_3v3; > + > / { > model = "Pine H64 model B"; > compatible = "pine64,pine-h64-model-b", "allwinner,sun50i-h6"; > > - /delete-node/ reg_gmac_3v3; > - > wifi_pwrseq: wifi_pwrseq { > compatible = "mmc-pwrseq-simple"; > reset-gpios = <&r_pio 1 3 GPIO_ACTIVE_LOW>; /* PM3 */
On Sun, 17 Mar 2024 20:44:51 +0100 Kamil Kasperski <ressetkk@gmail.com> wrote: Hi Kamil, thanks a lot for putting together those patches and sending them for upstream inclusion! > Add dtsi file for T95 tv boxes and add initial support for T95 5G AXP313A > variant with a board name H616-T95MAX-AXP313A-v3.0 Internal storage is not > accessible due to lack of support for H616 NAND controller. Them using raw NAND is really unfortunate. I think the original T95 box used eMMC? > > Signed-off-by: Kamil Kasperski <ressetkk@gmail.com> > --- > arch/arm64/boot/dts/allwinner/Makefile | 1 + > arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi | 109 +++++++++++++++++++++ > .../dts/allwinner/sun50i-h616-t95max-axp313.dts | 85 ++++++++++++++++ > 3 files changed, 195 insertions(+) > > diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile > index 21149b346a60..294921f12b73 100644 > --- a/arch/arm64/boot/dts/allwinner/Makefile > +++ b/arch/arm64/boot/dts/allwinner/Makefile > @@ -42,6 +42,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-tanix-tx6-mini.dtb > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-bigtreetech-cb1-manta.dtb > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-bigtreetech-pi.dtb > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-orangepi-zero2.dtb > +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-t95max-axp313.dtb > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-x96-mate.dtb > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-longanpi-3h.dtb > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-orangepi-zero2w.dtb > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi > new file mode 100644 > index 000000000000..815cf2dac24b > --- /dev/null > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi > @@ -0,0 +1,109 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +/* > + * Copyright (C) 2024 Kamil Kasperski <ressetkk@gmail.com> > + * > + * Common DT nodes for H616-based T95 TV boxes > + * There are two versions reported with different PMIC variants. > + */ > + > +#include "sun50i-h616.dtsi" > + > +#include <dt-bindings/gpio/gpio.h> > +#include <dt-bindings/interrupt-controller/arm-gic.h> > + > +/ { > + aliases { > + ethernet1 = &sdio_wifi; > + serial0 = &uart0; > + }; > + > + chosen { > + stdout-path = "serial0:115200n8"; > + }; > + > + reg_vcc5v: vcc5v { > + /* board wide 5V supply directly from the DC input */ > + compatible = "regulator-fixed"; > + regulator-name = "vcc-5v"; > + regulator-min-microvolt = <5000000>; > + regulator-max-microvolt = <5000000>; > + regulator-always-on; > + }; > + > + reg_vcc3v3: vcc3v3 { > + /* discrete 3.3V regulator */ > + compatible = "regulator-fixed"; > + regulator-name = "vcc-3v3"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + regulator-always-on; > + }; > + > + wifi_pwrseq: wifi-pwrseq { Krzysztof recently sent a patch to just use "pwrseq" as the node name, so can you do the same here? https://lore.kernel.org/linux-sunxi/20240317184130.157695-2-krzysztof.kozlowski@linaro.org/T/#u > + compatible = "mmc-pwrseq-simple"; > + clocks = <&rtc CLK_OSC32K_FANOUT>; > + clock-names = "ext_clock"; > + pinctrl-0 = <&x32clk_fanout_pin>; > + pinctrl-names = "default"; > + reset-gpios = <&pio 6 18 GPIO_ACTIVE_LOW>; /* PG18 */ > + }; > +}; > + > +&ehci0 { > + status = "okay"; > +}; > + > +&ehci2 { > + status = "okay"; > +}; > + > +&ir { > + status = "okay"; > +}; > + > +&mmc0 { > + cd-gpios = <&pio 8 16 GPIO_ACTIVE_LOW>; /* PI16 */ > + bus-width = <4>; > + status = "okay"; > +}; > + > +&mmc1 { > + mmc-pwrseq = <&wifi_pwrseq>; > + bus-width = <4>; > + non-removable; > + status = "okay"; > + > + sdio_wifi: wifi@1 { > + reg = <1>; > + }; So does the WiFi work with mainline drivers? IIUC the BCM43342 is not supported by the existing Broadcom drivers? > +}; > + > +&ohci0 { > + status = "okay"; > +}; > + > +&ohci2 { > + status = "okay"; > +}; > + > +&uart0 { > + pinctrl-names = "default"; > + pinctrl-0 = <&uart0_ph_pins>; > + status = "okay"; > +}; > + > +&uart1 { > + pinctrl-names = "default"; > + pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>; > + uart-has-rtscts; > + status = "okay"; Similar question here, what is the Bluetooth situation in mainline? I guess it's not supported, since you didn't put a BT node in here? > +}; > + > +&usbotg { > + dr_mode = "host"; /* USB A type receptable */ > + status = "okay"; > +}; > + > +&usbphy { > + status = "okay"; > +}; > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-t95max-axp313.dts b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95max-axp313.dts > new file mode 100644 > index 000000000000..c8650aca2407 > --- /dev/null > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95max-axp313.dts > @@ -0,0 +1,85 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +/* > + * Copyright (C) 2024 Kamil Kasperski <ressetkk@gmail.com> > + * > + * Configuration for T95 TV box with board label H616-T95MAX-AXP313A-v3.0 > + */ > + > +/dts-v1/; > + > +#include "sun50i-h616-t95.dtsi" > + > +/ { > + model = "T95 5G (AXP313)"; > + compatible = "t95,t95max-axp313", "allwinner,sun50i-h616"; > +}; > + > +&mmc0 { > + vmmc-supply = <®_dldo1>; > +}; > + > +&mmc1 { > + vmmc-supply = <®_dldo1>; > + vqmmc-supply = <®_aldo1>; > +}; > + > +&r_i2c { > + status = "okay"; > + > + axp313: pmic@36 { > + compatible = "x-powers,axp313a"; > + reg = <0x36>; > + #interrupt-cells = <1>; > + interrupt-controller; > + interrupt-parent = <&pio>; I don't think you need interrupt-parent unless you also actually specify an interrupt line. (But please keep #interrupt-cells and interrupt-controller.) > + > + vin1-supply = <®_vcc5v>; > + vin2-supply = <®_vcc5v>; > + vin3-supply = <®_vcc5v>; > + > + regulators { > + reg_aldo1: aldo1 { > + regulator-always-on; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + regulator-name = "vcc1v8"; > + }; > + > + reg_dldo1: dldo1 { > + regulator-always-on; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + regulator-name = "vcc3v3"; > + }; > + > + reg_dcdc1: dcdc1 { > + regulator-always-on; > + regulator-min-microvolt = <810000>; > + regulator-max-microvolt = <990000>; > + regulator-name = "vdd-gpu-sys"; > + }; > + > + reg_dcdc2: dcdc2 { > + regulator-always-on; > + regulator-min-microvolt = <810000>; > + regulator-max-microvolt = <1100000>; > + regulator-name = "vdd-cpu"; > + }; > + > + reg_dcdc3: dcdc3 { > + regulator-always-on; > + regulator-min-microvolt = <1500000>; > + regulator-max-microvolt = <1500000>; > + regulator-name = "vdd-dram"; > + }; > + }; > + }; > +}; > + > +&pio { > + vcc-pc-supply = <®_aldo1>; > + vcc-pf-supply = <®_dldo1>; > + vcc-pg-supply = <®_dldo1>; So if vqmmc-supply for MMC1 is at the 1.8V from ALDO1, that must mean that the whole of PortG is at 1.8V, right? So I think this would need to be reg_aldo1 here. Apart from those minor things it looks good to me. Cheers, Andre > + vcc-ph-supply = <®_dldo1>; > + vcc-pi-supply = <®_dldo1>; > +}; >
On Mon, 18 Mar 2024 04:39:46 +0000 Amit Singh Tomar <amitsinght@marvell.com> wrote: Hi Amit, can you please try to reply using the standard quoted line prefix ('>'), and cut the header? I almost missed your question in here. > Hi, > > -----Original Message----- > From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org> On Behalf Of Andre Przywara > Sent: Monday, March 18, 2024 6:42 AM > To: Yangtao Li <tiny.windzz@gmail.com>; Viresh Kumar <vireshk@kernel.org>; Nishanth Menon <nm@ti.com>; Stephen Boyd <sboyd@kernel.org>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; Chen-Yu Tsai <wens@csie.org>; Jernej Skrabec <jernej.skrabec@gmail.com>; Samuel Holland <samuel@sholland.org>; Rafael J . Wysocki <rafael@kernel.org> > Cc: linux-pm@vger.kernel.org; devicetree@vger.kernel.org; linux-sunxi@lists.linux.dev; linux-arm-kernel@lists.infradead.org; Brandon Cheo Fusi <fusibrandon13@gmail.com>; Martin Botka <martin.botka@somainline.org>; Martin Botka <martin.botka1@gmail.com> > Subject: [EXTERNAL] [PATCH v2 8/8] arm64: dts: allwinner: h616: enable DVFS for all boards > > > With the DT bindings now describing the format of the CPU OPP tables, we can include the OPP table in each board's .dts file, and specify the CPU power supply. > This allows to enable DVFS, and get up to 50% of performance benefit in the highest OPP, or up to 60% power savings in the lowest OPP, compared to the fixed 1GHz @ 1.0V OPP we are running in by default > at the moment. > [Amit] Could you please elaborate, what test were run to see 50 % performance benefits? Currently all H616 boards running mainline firmware and kernels run at a fixed 1GHz CPU clock frequency. If you happen to have a good SoC (bin 1 or 3), this patchset will allow you to run at 1.5 GHz, which is 50% faster. So anything that scales with CPU frequency should run much quicker. Cheers, Andre > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > .../boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi | 5 +++++ > arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts | 5 +++++ > arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts | 5 +++++ > .../arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts | 5 +++++ arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts | 5 +++++ > .../boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts | 5 +++++ > 6 files changed, 30 insertions(+) > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi > index 1fed2b46cfe87..86e58d1ed23ea 100644 > --- a/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi > @@ -6,6 +6,7 @@ > /dts-v1/; > > #include "sun50i-h616.dtsi" > +#include "sun50i-h616-cpu-opp.dtsi" > > #include <dt-bindings/gpio/gpio.h> > #include <dt-bindings/interrupt-controller/arm-gic.h> > @@ -62,6 +63,10 @@ wifi_pwrseq: wifi-pwrseq { > }; > }; > > +&cpu0 { > + cpu-supply = <®_dcdc2>; > +}; > + > &mmc0 { > vmmc-supply = <®_dldo1>; > /* Card detection pin is not connected */ diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts b/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts > index b5d713926a341..a360d8567f955 100644 > --- a/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts > @@ -6,12 +6,17 @@ > /dts-v1/; > > #include "sun50i-h616-orangepi-zero.dtsi" > +#include "sun50i-h616-cpu-opp.dtsi" > > / { > model = "OrangePi Zero2"; > compatible = "xunlong,orangepi-zero2", "allwinner,sun50i-h616"; }; > > +&cpu0 { > + cpu-supply = <®_dcdca>; > +}; > + > &emac0 { > allwinner,rx-delay-ps = <3100>; > allwinner,tx-delay-ps = <700>; > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts b/arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts > index 959b6fd18483b..26d25b5b59e0f 100644 > --- a/arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts > @@ -6,6 +6,7 @@ > /dts-v1/; > > #include "sun50i-h616.dtsi" > +#include "sun50i-h616-cpu-opp.dtsi" > > #include <dt-bindings/gpio/gpio.h> > #include <dt-bindings/interrupt-controller/arm-gic.h> > @@ -32,6 +33,10 @@ reg_vcc5v: vcc5v { > }; > }; > > +&cpu0 { > + cpu-supply = <®_dcdca>; > +}; > + > &ehci0 { > status = "okay"; > }; > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts b/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts > index 21ca1977055d9..6a4f0da972330 100644 > --- a/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts > @@ -6,6 +6,7 @@ > /dts-v1/; > > #include "sun50i-h616.dtsi" > +#include "sun50i-h616-cpu-opp.dtsi" > > #include <dt-bindings/gpio/gpio.h> > #include <dt-bindings/interrupt-controller/arm-gic.h> > @@ -53,6 +54,10 @@ reg_vcc3v3: vcc3v3 { > }; > }; > > +&cpu0 { > + cpu-supply = <®_dcdc2>; > +}; > + > &ehci1 { > status = "okay"; > }; > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts > index b3b1b8692125f..e1cd7572a14ce 100644 > --- a/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts > @@ -6,12 +6,17 @@ > /dts-v1/; > > #include "sun50i-h616-orangepi-zero.dtsi" > +#include "sun50i-h616-cpu-opp.dtsi" > > / { > model = "OrangePi Zero3"; > compatible = "xunlong,orangepi-zero3", "allwinner,sun50i-h618"; }; > > +&cpu0 { > + cpu-supply = <®_dcdc2>; > +}; > + > &emac0 { > allwinner,tx-delay-ps = <700>; > phy-mode = "rgmii-rxid"; > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts b/arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts > index 8ea1fd41aebaa..2dd178a164fbe 100644 > --- a/arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts > @@ -6,6 +6,7 @@ > /dts-v1/; > > #include "sun50i-h616.dtsi" > +#include "sun50i-h616-cpu-opp.dtsi" > > #include <dt-bindings/gpio/gpio.h> > #include <dt-bindings/interrupt-controller/arm-gic.h> > @@ -41,6 +42,10 @@ reg_vcc3v3: vcc3v3 { > }; > }; > > +&cpu0 { > + cpu-supply = <®_dcdc2>; > +}; > + > &ehci0 { > status = "okay"; > }; > -- > 2.35.8 > >
Hi, -----Original Message----- From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org> On Behalf Of Andre Przywara Sent: Monday, March 18, 2024 6:42 AM To: Yangtao Li <tiny.windzz@gmail.com>; Viresh Kumar <vireshk@kernel.org>; Nishanth Menon <nm@ti.com>; Stephen Boyd <sboyd@kernel.org>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; Chen-Yu Tsai <wens@csie.org>; Jernej Skrabec <jernej.skrabec@gmail.com>; Samuel Holland <samuel@sholland.org>; Rafael J . Wysocki <rafael@kernel.org> Cc: linux-pm@vger.kernel.org; devicetree@vger.kernel.org; linux-sunxi@lists.linux.dev; linux-arm-kernel@lists.infradead.org; Brandon Cheo Fusi <fusibrandon13@gmail.com>; Martin Botka <martin.botka@somainline.org>; Martin Botka <martin.botka1@gmail.com> Subject: [EXTERNAL] [PATCH v2 8/8] arm64: dts: allwinner: h616: enable DVFS for all boards With the DT bindings now describing the format of the CPU OPP tables, we can include the OPP table in each board's .dts file, and specify the CPU power supply. This allows to enable DVFS, and get up to 50% of performance benefit in the highest OPP, or up to 60% power savings in the lowest OPP, compared to the fixed 1GHz @ 1.0V OPP we are running in by default at the moment. [Amit] Could you please elaborate, what test were run to see 50 % performance benefits? Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- .../boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi | 5 +++++ arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts | 5 +++++ arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts | 5 +++++ .../arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts | 5 +++++ arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts | 5 +++++ .../boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts | 5 +++++ 6 files changed, 30 insertions(+) diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi index 1fed2b46cfe87..86e58d1ed23ea 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi @@ -6,6 +6,7 @@ /dts-v1/; #include "sun50i-h616.dtsi" +#include "sun50i-h616-cpu-opp.dtsi" #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/interrupt-controller/arm-gic.h> @@ -62,6 +63,10 @@ wifi_pwrseq: wifi-pwrseq { }; }; +&cpu0 { + cpu-supply = <®_dcdc2>; +}; + &mmc0 { vmmc-supply = <®_dldo1>; /* Card detection pin is not connected */ diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts b/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts index b5d713926a341..a360d8567f955 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts @@ -6,12 +6,17 @@ /dts-v1/; #include "sun50i-h616-orangepi-zero.dtsi" +#include "sun50i-h616-cpu-opp.dtsi" / { model = "OrangePi Zero2"; compatible = "xunlong,orangepi-zero2", "allwinner,sun50i-h616"; }; +&cpu0 { + cpu-supply = <®_dcdca>; +}; + &emac0 { allwinner,rx-delay-ps = <3100>; allwinner,tx-delay-ps = <700>; diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts b/arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts index 959b6fd18483b..26d25b5b59e0f 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts @@ -6,6 +6,7 @@ /dts-v1/; #include "sun50i-h616.dtsi" +#include "sun50i-h616-cpu-opp.dtsi" #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/interrupt-controller/arm-gic.h> @@ -32,6 +33,10 @@ reg_vcc5v: vcc5v { }; }; +&cpu0 { + cpu-supply = <®_dcdca>; +}; + &ehci0 { status = "okay"; }; diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts b/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts index 21ca1977055d9..6a4f0da972330 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts @@ -6,6 +6,7 @@ /dts-v1/; #include "sun50i-h616.dtsi" +#include "sun50i-h616-cpu-opp.dtsi" #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/interrupt-controller/arm-gic.h> @@ -53,6 +54,10 @@ reg_vcc3v3: vcc3v3 { }; }; +&cpu0 { + cpu-supply = <®_dcdc2>; +}; + &ehci1 { status = "okay"; }; diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts index b3b1b8692125f..e1cd7572a14ce 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts @@ -6,12 +6,17 @@ /dts-v1/; #include "sun50i-h616-orangepi-zero.dtsi" +#include "sun50i-h616-cpu-opp.dtsi" / { model = "OrangePi Zero3"; compatible = "xunlong,orangepi-zero3", "allwinner,sun50i-h618"; }; +&cpu0 { + cpu-supply = <®_dcdc2>; +}; + &emac0 { allwinner,tx-delay-ps = <700>; phy-mode = "rgmii-rxid"; diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts b/arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts index 8ea1fd41aebaa..2dd178a164fbe 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts @@ -6,6 +6,7 @@ /dts-v1/; #include "sun50i-h616.dtsi" +#include "sun50i-h616-cpu-opp.dtsi" #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/interrupt-controller/arm-gic.h> @@ -41,6 +42,10 @@ reg_vcc3v3: vcc3v3 { }; }; +&cpu0 { + cpu-supply = <®_dcdc2>; +}; + &ehci0 { status = "okay"; }; -- 2.35.8
With the DT bindings now describing the format of the CPU OPP tables, we can include the OPP table in each board's .dts file, and specify the CPU power supply. This allows to enable DVFS, and get up to 50% of performance benefit in the highest OPP, or up to 60% power savings in the lowest OPP, compared to the fixed 1GHz @ 1.0V OPP we are running in by default at the moment. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- .../boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi | 5 +++++ arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts | 5 +++++ arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts | 5 +++++ .../arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts | 5 +++++ arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts | 5 +++++ .../boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts | 5 +++++ 6 files changed, 30 insertions(+) diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi index 1fed2b46cfe87..86e58d1ed23ea 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi @@ -6,6 +6,7 @@ /dts-v1/; #include "sun50i-h616.dtsi" +#include "sun50i-h616-cpu-opp.dtsi" #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/interrupt-controller/arm-gic.h> @@ -62,6 +63,10 @@ wifi_pwrseq: wifi-pwrseq { }; }; +&cpu0 { + cpu-supply = <®_dcdc2>; +}; + &mmc0 { vmmc-supply = <®_dldo1>; /* Card detection pin is not connected */ diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts b/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts index b5d713926a341..a360d8567f955 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts @@ -6,12 +6,17 @@ /dts-v1/; #include "sun50i-h616-orangepi-zero.dtsi" +#include "sun50i-h616-cpu-opp.dtsi" / { model = "OrangePi Zero2"; compatible = "xunlong,orangepi-zero2", "allwinner,sun50i-h616"; }; +&cpu0 { + cpu-supply = <®_dcdca>; +}; + &emac0 { allwinner,rx-delay-ps = <3100>; allwinner,tx-delay-ps = <700>; diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts b/arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts index 959b6fd18483b..26d25b5b59e0f 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts @@ -6,6 +6,7 @@ /dts-v1/; #include "sun50i-h616.dtsi" +#include "sun50i-h616-cpu-opp.dtsi" #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/interrupt-controller/arm-gic.h> @@ -32,6 +33,10 @@ reg_vcc5v: vcc5v { }; }; +&cpu0 { + cpu-supply = <®_dcdca>; +}; + &ehci0 { status = "okay"; }; diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts b/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts index 21ca1977055d9..6a4f0da972330 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts @@ -6,6 +6,7 @@ /dts-v1/; #include "sun50i-h616.dtsi" +#include "sun50i-h616-cpu-opp.dtsi" #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/interrupt-controller/arm-gic.h> @@ -53,6 +54,10 @@ reg_vcc3v3: vcc3v3 { }; }; +&cpu0 { + cpu-supply = <®_dcdc2>; +}; + &ehci1 { status = "okay"; }; diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts index b3b1b8692125f..e1cd7572a14ce 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts @@ -6,12 +6,17 @@ /dts-v1/; #include "sun50i-h616-orangepi-zero.dtsi" +#include "sun50i-h616-cpu-opp.dtsi" / { model = "OrangePi Zero3"; compatible = "xunlong,orangepi-zero3", "allwinner,sun50i-h618"; }; +&cpu0 { + cpu-supply = <®_dcdc2>; +}; + &emac0 { allwinner,tx-delay-ps = <700>; phy-mode = "rgmii-rxid"; diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts b/arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts index 8ea1fd41aebaa..2dd178a164fbe 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts @@ -6,6 +6,7 @@ /dts-v1/; #include "sun50i-h616.dtsi" +#include "sun50i-h616-cpu-opp.dtsi" #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/interrupt-controller/arm-gic.h> @@ -41,6 +42,10 @@ reg_vcc3v3: vcc3v3 { }; }; +&cpu0 { + cpu-supply = <®_dcdc2>; +}; + &ehci0 { status = "okay"; }; -- 2.35.8
From: Martin Botka <martin.botka@somainline.org> Add an Operating Performance Points table for the CPU cores to enable Dynamic Voltage & Frequency Scaling (DVFS) on the H616. Also add the needed cpu_speed_grade nvmem cell. Signed-off-by: Martin Botka <martin.botka@somainline.org> [Andre: rework to only use single opp-microvolt] Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- .../dts/allwinner/sun50i-h616-cpu-opp.dtsi | 138 ++++++++++++++++++ .../arm64/boot/dts/allwinner/sun50i-h616.dtsi | 4 + 2 files changed, 142 insertions(+) create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h616-cpu-opp.dtsi diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-cpu-opp.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616-cpu-opp.dtsi new file mode 100644 index 0000000000000..b0ec24000dad9 --- /dev/null +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-cpu-opp.dtsi @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +// Copyright (C) 2023 Martin Botka <martin@somainline.org> + +/ { + cpu_opp_table: opp-table-cpu { + compatible = "allwinner,sun50i-h616-operating-points"; + nvmem-cells = <&cpu_speed_grade>; + opp-shared; + + opp-480000000 { + opp-hz = /bits/ 64 <480000000>; + opp-microvolt = <900000>; + clock-latency-ns = <244144>; /* 8 32k periods */ + opp-supported-hw = <0x1f>; + }; + + opp-600000000 { + opp-hz = /bits/ 64 <600000000>; + opp-microvolt = <900000>; + clock-latency-ns = <244144>; /* 8 32k periods */ + opp-supported-hw = <0x12>; + }; + + opp-720000000 { + opp-hz = /bits/ 64 <720000000>; + opp-microvolt = <900000>; + clock-latency-ns = <244144>; /* 8 32k periods */ + opp-supported-hw = <0x0d>; + }; + + opp-792000000-l { + opp-hz = /bits/ 64 <792000000>; + opp-microvolt = <900000>; + clock-latency-ns = <244144>; /* 8 32k periods */ + opp-supported-hw = <0x02>; + }; + + opp-792000000-h { + opp-hz = /bits/ 64 <792000000>; + opp-microvolt = <940000>; + clock-latency-ns = <244144>; /* 8 32k periods */ + opp-supported-hw = <0x10>; + }; + + opp-936000000 { + opp-hz = /bits/ 64 <936000000>; + opp-microvolt = <900000>; + clock-latency-ns = <244144>; /* 8 32k periods */ + opp-supported-hw = <0x0d>; + }; + + opp-1008000000-l { + opp-hz = /bits/ 64 <1008000000>; + opp-microvolt = <950000>; + clock-latency-ns = <244144>; /* 8 32k periods */ + opp-supported-hw = <0x0d>; + }; + + opp-1008000000-m { + opp-hz = /bits/ 64 <1008000000>; + opp-microvolt = <940000>; + clock-latency-ns = <244144>; /* 8 32k periods */ + opp-supported-hw = <0x02>; + }; + + opp-1008000000-h { + opp-hz = /bits/ 64 <1008000000>; + opp-microvolt = <1020000>; + clock-latency-ns = <244144>; /* 8 32k periods */ + opp-supported-hw = <0x10>; + }; + + opp-1104000000 { + opp-hz = /bits/ 64 <1104000000>; + opp-microvolt = <1000000>; + clock-latency-ns = <244144>; /* 8 32k periods */ + opp-supported-hw = <0x0d>; + }; + + opp-1200000000-l { + opp-hz = /bits/ 64 <1200000000>; + opp-microvolt = <1020000>; + clock-latency-ns = <244144>; /* 8 32k periods */ + opp-supported-hw = <0x02>; + }; + + opp-1200000000-m { + opp-hz = /bits/ 64 <1200000000>; + opp-microvolt = <1050000>; + clock-latency-ns = <244144>; /* 8 32k periods */ + opp-supported-hw = <0x0d>; + }; + + opp-1200000000-h { + opp-hz = /bits/ 64 <1200000000>; + opp-microvolt = <1100000>; + clock-latency-ns = <244144>; /* 8 32k periods */ + opp-supported-hw = <0x10>; + }; + + opp-1320000000 { + opp-hz = /bits/ 64 <1320000000>; + opp-microvolt = <1100000>; + clock-latency-ns = <244144>; /* 8 32k periods */ + opp-supported-hw = <0x1d>; + }; + + opp-1416000000 { + opp-hz = /bits/ 64 <1416000000>; + opp-microvolt = <1100000>; + clock-latency-ns = <244144>; /* 8 32k periods */ + opp-supported-hw = <0x0d>; + }; + + opp-1512000000 { + opp-hz = /bits/ 64 <1512000000>; + opp-microvolt = <1100000>; + clock-latency-ns = <244144>; /* 8 32k periods */ + opp-supported-hw = <0x0a>; + }; + }; +}; + +&cpu0 { + operating-points-v2 = <&cpu_opp_table>; +}; + +&cpu1 { + operating-points-v2 = <&cpu_opp_table>; +}; + +&cpu2 { + operating-points-v2 = <&cpu_opp_table>; +}; + +&cpu3 { + operating-points-v2 = <&cpu_opp_table>; +}; diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi index 08517f46de058..91f1874aa3b02 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi @@ -143,6 +143,10 @@ sid: efuse@3006000 { ths_calibration: thermal-sensor-calibration@14 { reg = <0x14 0x8>; }; + + cpu_speed_grade: cpu-speed-grade@0 { + reg = <0x0 2>; + }; }; watchdog: watchdog@30090a0 { -- 2.35.8
From: Martin Botka <martin.botka@somainline.org> The Allwinner H616/H618 SoCs have different OPP tables per SoC version and die revision. The SoC version is stored in NVMEM, as before, though encoded differently. The die revision is in a different register, in the SRAM controller. Firmware already exports that value in a standardised way, through the SMCCC SoCID mechanism. We need both values, as some chips have the same SoC version, but they don't support the same frequencies and they get differentiated by the die revision. Add the new compatible string and tie the new translation function to it. This mechanism not only covers the original H616 SoC, but also its very close sibling SoCs H618 and H700, so add them to the list as well. Signed-off-by: Martin Botka <martin.botka@somainline.org> Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- drivers/cpufreq/sun50i-cpufreq-nvmem.c | 53 ++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c index bd170611c7906..3f7051937ff2b 100644 --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c @@ -10,6 +10,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <linux/arm-smccc.h> #include <linux/cpu.h> #include <linux/module.h> #include <linux/nvmem-consumer.h> @@ -46,14 +47,63 @@ static u32 sun50i_h6_efuse_xlate(u32 speedbin) return 0; } +/* + * Judging by the OPP tables in the vendor BSP, the quality order of the + * returned speedbin index is 4 -> 0/2 -> 3 -> 1, from worst to best. + * 0 and 2 seem identical from the OPP tables' point of view. + */ +static u32 sun50i_h616_efuse_xlate(u32 speedbin) +{ + int ver_bits = arm_smccc_get_soc_id_revision(); + u32 value = 0; + + switch (speedbin & 0xffff) { + case 0x2000: + value = 0; + break; + case 0x2400: + case 0x7400: + case 0x2c00: + case 0x7c00: + if (ver_bits != SMCCC_RET_NOT_SUPPORTED && ver_bits <= 1) { + /* ic version A/B */ + value = 1; + } else { + /* ic version C and later version */ + value = 2; + } + break; + case 0x5000: + case 0x5400: + case 0x6000: + value = 3; + break; + case 0x5c00: + value = 4; + break; + case 0x5d00: + default: + value = 0; + } + + return value; +} + static struct sunxi_cpufreq_data sun50i_h6_cpufreq_data = { .efuse_xlate = sun50i_h6_efuse_xlate, }; +static struct sunxi_cpufreq_data sun50i_h616_cpufreq_data = { + .efuse_xlate = sun50i_h616_efuse_xlate, +}; + static const struct of_device_id cpu_opp_match_list[] = { { .compatible = "allwinner,sun50i-h6-operating-points", .data = &sun50i_h6_cpufreq_data, }, + { .compatible = "allwinner,sun50i-h616-operating-points", + .data = &sun50i_h616_cpufreq_data, + }, {} }; @@ -230,6 +280,9 @@ static struct platform_driver sun50i_cpufreq_driver = { static const struct of_device_id sun50i_cpufreq_match_list[] = { { .compatible = "allwinner,sun50i-h6" }, + { .compatible = "allwinner,sun50i-h616" }, + { .compatible = "allwinner,sun50i-h618" }, + { .compatible = "allwinner,sun50i-h700" }, {} }; MODULE_DEVICE_TABLE(of, sun50i_cpufreq_match_list); -- 2.35.8
The opp_supported_hw DT property allows the DT to specify a mask of chip revisions that a certain OPP is eligible for. This allows for easy limiting of maximum frequencies, for instance. Add support for that in the sun50i-cpufreq-nvmem driver. We support both the existing opp-microvolt suffix properties as well as the opp-supported-hw property, the generic code figures out which is needed automatically. However if none of the DT OPP nodes contain an opp-supported-hw property, the core code will ignore all OPPs and the driver will fail probing. So check the DT's eligibility first before using that feature. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- drivers/cpufreq/sun50i-cpufreq-nvmem.c | 62 ++++++++++++++++++++++---- 1 file changed, 54 insertions(+), 8 deletions(-) diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c index 7b44f3b13e7d2..bd170611c7906 100644 --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c @@ -57,6 +57,41 @@ static const struct of_device_id cpu_opp_match_list[] = { {} }; +/** + * dt_has_supported_hw() - Check if any OPPs use opp-supported-hw + * + * If we ask the cpufreq framework to use the opp-supported-hw feature, it + * will ignore every OPP node without that DT property. If none of the OPPs + * have it, the driver will fail probing, due to the lack of OPPs. + * + * Returns true if we have at least one OPP with the opp-supported-hw property. + */ +static bool dt_has_supported_hw(void) +{ + bool has_opp_supported_hw = false; + struct device_node *np, *opp; + struct device *cpu_dev; + + cpu_dev = get_cpu_device(0); + if (!cpu_dev) + return -ENODEV; + + np = dev_pm_opp_of_get_opp_desc_node(cpu_dev); + if (!np) + return -ENOENT; + + for_each_child_of_node(np, opp) { + if (of_find_property(opp, "opp-supported-hw", NULL)) { + has_opp_supported_hw = true; + break; + } + } + + of_node_put(np); + + return has_opp_supported_hw; +} + /** * sun50i_cpufreq_get_efuse() - Determine speed grade from efuse value * @@ -110,7 +145,8 @@ static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev) { int *opp_tokens; char name[MAX_NAME_LEN]; - unsigned int cpu; + unsigned int cpu, supported_hw; + struct dev_pm_opp_config config = {}; int speed; int ret; @@ -125,7 +161,18 @@ static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev) return speed; } + /* + * We need at least one OPP with the "opp-supported-hw" property, + * or else the upper layers will ignore every OPP and will bail out. + */ + if (dt_has_supported_hw()) { + supported_hw = 1U << speed; + config.supported_hw = &supported_hw; + config.supported_hw_count = 1; + } + snprintf(name, MAX_NAME_LEN, "speed%d", speed); + config.prop_name = name; for_each_possible_cpu(cpu) { struct device *cpu_dev = get_cpu_device(cpu); @@ -135,12 +182,11 @@ static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev) goto free_opp; } - opp_tokens[cpu] = dev_pm_opp_set_prop_name(cpu_dev, name); - if (opp_tokens[cpu] < 0) { - ret = opp_tokens[cpu]; - pr_err("Failed to set prop name\n"); + ret = dev_pm_opp_set_config(cpu_dev, &config); + if (ret < 0) goto free_opp; - } + + opp_tokens[cpu] = ret; } cpufreq_dt_pdev = platform_device_register_simple("cpufreq-dt", -1, @@ -155,7 +201,7 @@ static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev) free_opp: for_each_possible_cpu(cpu) - dev_pm_opp_put_prop_name(opp_tokens[cpu]); + dev_pm_opp_clear_config(opp_tokens[cpu]); kfree(opp_tokens); return ret; @@ -169,7 +215,7 @@ static void sun50i_cpufreq_nvmem_remove(struct platform_device *pdev) platform_device_unregister(cpufreq_dt_pdev); for_each_possible_cpu(cpu) - dev_pm_opp_put_prop_name(opp_tokens[cpu]); + dev_pm_opp_clear_config(opp_tokens[cpu]); kfree(opp_tokens); } -- 2.35.8
From: Brandon Cheo Fusi <fusibrandon13@gmail.com> Make converting the speed bin value into a speed grade generic and determined by a platform specific callback. Also change the prototypes involved to encode the speed bin directly in the return value. This allows to extend the driver more easily to support more SoCs. Signed-off-by: Brandon Cheo Fusi <fusibrandon13@gmail.com> [Andre: merge output into return value] Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- drivers/cpufreq/sun50i-cpufreq-nvmem.c | 74 +++++++++++++++++--------- 1 file changed, 49 insertions(+), 25 deletions(-) diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c index 32a9c88f8ff6d..7b44f3b13e7d2 100644 --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c @@ -25,19 +25,52 @@ static struct platform_device *cpufreq_dt_pdev, *sun50i_cpufreq_pdev; +struct sunxi_cpufreq_data { + u32 (*efuse_xlate)(u32 speedbin); +}; + +static u32 sun50i_h6_efuse_xlate(u32 speedbin) +{ + u32 efuse_value; + + efuse_value = (speedbin >> NVMEM_SHIFT) & NVMEM_MASK; + + /* + * We treat unexpected efuse values as if the SoC was from + * the slowest bin. Expected efuse values are 1-3, slowest + * to fastest. + */ + if (efuse_value >= 1 && efuse_value <= 3) + return efuse_value - 1; + else + return 0; +} + +static struct sunxi_cpufreq_data sun50i_h6_cpufreq_data = { + .efuse_xlate = sun50i_h6_efuse_xlate, +}; + +static const struct of_device_id cpu_opp_match_list[] = { + { .compatible = "allwinner,sun50i-h6-operating-points", + .data = &sun50i_h6_cpufreq_data, + }, + {} +}; + /** * sun50i_cpufreq_get_efuse() - Determine speed grade from efuse value - * @versions: Set to the value parsed from efuse * - * Returns 0 if success. + * Returns non-negative speed bin index on success, a negative error + * value otherwise. */ -static int sun50i_cpufreq_get_efuse(u32 *versions) +static int sun50i_cpufreq_get_efuse(void) { struct nvmem_cell *speedbin_nvmem; struct device_node *np; struct device *cpu_dev; - u32 *speedbin, efuse_value; - size_t len; + const struct of_device_id *match; + const struct sunxi_cpufreq_data *opp_data; + u32 *speedbin; int ret; cpu_dev = get_cpu_device(0); @@ -48,12 +81,12 @@ static int sun50i_cpufreq_get_efuse(u32 *versions) if (!np) return -ENOENT; - ret = of_device_is_compatible(np, - "allwinner,sun50i-h6-operating-points"); - if (!ret) { + match = of_match_node(cpu_opp_match_list, np); + if (!match) { of_node_put(np); return -ENOENT; } + opp_data = match->data; speedbin_nvmem = of_nvmem_cell_get(np, NULL); of_node_put(np); @@ -61,25 +94,16 @@ static int sun50i_cpufreq_get_efuse(u32 *versions) return dev_err_probe(cpu_dev, PTR_ERR(speedbin_nvmem), "Could not get nvmem cell\n"); - speedbin = nvmem_cell_read(speedbin_nvmem, &len); + speedbin = nvmem_cell_read(speedbin_nvmem, NULL); nvmem_cell_put(speedbin_nvmem); if (IS_ERR(speedbin)) return PTR_ERR(speedbin); - efuse_value = (*speedbin >> NVMEM_SHIFT) & NVMEM_MASK; - - /* - * We treat unexpected efuse values as if the SoC was from - * the slowest bin. Expected efuse values are 1-3, slowest - * to fastest. - */ - if (efuse_value >= 1 && efuse_value <= 3) - *versions = efuse_value - 1; - else - *versions = 0; + ret = opp_data->efuse_xlate(*speedbin); kfree(speedbin); - return 0; + + return ret; }; static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev) @@ -87,7 +111,7 @@ static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev) int *opp_tokens; char name[MAX_NAME_LEN]; unsigned int cpu; - u32 speed = 0; + int speed; int ret; opp_tokens = kcalloc(num_possible_cpus(), sizeof(*opp_tokens), @@ -95,10 +119,10 @@ static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev) if (!opp_tokens) return -ENOMEM; - ret = sun50i_cpufreq_get_efuse(&speed); - if (ret) { + speed = sun50i_cpufreq_get_efuse(); + if (speed < 0) { kfree(opp_tokens); - return ret; + return speed; } snprintf(name, MAX_NAME_LEN, "speed%d", speed); -- 2.35.8
From: Martin Botka <martin.botka@somainline.org> The Allwinner H616 uses a similar NVMEM based mechanism to determine the silicon revision, which is required to select the right frequency / voltage pair for the OPPs. However it limits the maximum frequency for some speedbins, which requires to introduce the opp-supported-hw property. Add this property to the list of allowed properties, also drop the requirement for the revision specific opp-microvolt properties, since they won't be needed if using opp-supported-hw. When using this property, we also might have multiple OPP nodes per frequency, so relax the OPP node naming to allow a single letter suffix. Also use to opportunity to adjust some wording, and drop a sentence referring to the Linux driver and the OPP subsystem. Shorten the existing example and add another example, showcasing the opp-supported-hw property. Signed-off-by: Martin Botka <martin.botka@somainline.org> Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- .../allwinner,sun50i-h6-operating-points.yaml | 89 ++++++++++--------- 1 file changed, 47 insertions(+), 42 deletions(-) diff --git a/Documentation/devicetree/bindings/opp/allwinner,sun50i-h6-operating-points.yaml b/Documentation/devicetree/bindings/opp/allwinner,sun50i-h6-operating-points.yaml index 51f62c3ae1947..d5439a3f696bc 100644 --- a/Documentation/devicetree/bindings/opp/allwinner,sun50i-h6-operating-points.yaml +++ b/Documentation/devicetree/bindings/opp/allwinner,sun50i-h6-operating-points.yaml @@ -13,25 +13,25 @@ maintainers: description: | For some SoCs, the CPU frequency subset and voltage value of each OPP varies based on the silicon variant in use. Allwinner Process - Voltage Scaling Tables defines the voltage and frequency value based - on the speedbin blown in the efuse combination. The - sun50i-cpufreq-nvmem driver reads the efuse value from the SoC to - provide the OPP framework with required information. + Voltage Scaling Tables define the voltage and frequency values based + on the speedbin blown in the efuse combination. allOf: - $ref: opp-v2-base.yaml# properties: compatible: - const: allwinner,sun50i-h6-operating-points + enum: + - allwinner,sun50i-h6-operating-points + - allwinner,sun50i-h616-operating-points nvmem-cells: description: | A phandle pointing to a nvmem-cells node representing the efuse - registers that has information about the speedbin that is used + register that has information about the speedbin that is used to select the right frequency/voltage value pair. Please refer - the for nvmem-cells bindings - Documentation/devicetree/bindings/nvmem/nvmem.txt and also + to the nvmem-cells bindings in + Documentation/devicetree/bindings/nvmem/nvmem.yaml and also the examples below. opp-shared: true @@ -41,21 +41,23 @@ required: - nvmem-cells patternProperties: - "^opp-[0-9]+$": + "^opp-[0-9]+(-[a-z])?$": type: object properties: opp-hz: true clock-latency-ns: true + opp-microvolt: true + opp-supported-hw: + description: | + A single 32 bit bitmap value, representing compatible HW, one + bit per speed bin index. patternProperties: "^opp-microvolt-speed[0-9]$": true required: - opp-hz - - opp-microvolt-speed0 - - opp-microvolt-speed1 - - opp-microvolt-speed2 unevaluatedProperties: false @@ -77,58 +79,61 @@ examples: opp-microvolt-speed2 = <800000>; }; - opp-720000000 { + opp-1080000000 { clock-latency-ns = <244144>; /* 8 32k periods */ - opp-hz = /bits/ 64 <720000000>; + opp-hz = /bits/ 64 <1080000000>; - opp-microvolt-speed0 = <880000>; - opp-microvolt-speed1 = <820000>; - opp-microvolt-speed2 = <800000>; + opp-microvolt-speed0 = <1060000>; + opp-microvolt-speed1 = <880000>; + opp-microvolt-speed2 = <840000>; }; - opp-816000000 { + opp-1488000000 { clock-latency-ns = <244144>; /* 8 32k periods */ - opp-hz = /bits/ 64 <816000000>; + opp-hz = /bits/ 64 <1488000000>; - opp-microvolt-speed0 = <880000>; - opp-microvolt-speed1 = <820000>; - opp-microvolt-speed2 = <800000>; + opp-microvolt-speed0 = <1160000>; + opp-microvolt-speed1 = <1000000>; + opp-microvolt-speed2 = <960000>; }; + }; + + - | + opp-table { + compatible = "allwinner,sun50i-h616-operating-points"; + nvmem-cells = <&speedbin_efuse>; + opp-shared; - opp-888000000 { + opp-480000000 { clock-latency-ns = <244144>; /* 8 32k periods */ - opp-hz = /bits/ 64 <888000000>; + opp-hz = /bits/ 64 <480000000>; - opp-microvolt-speed0 = <940000>; - opp-microvolt-speed1 = <820000>; - opp-microvolt-speed2 = <800000>; + opp-microvolt = <900000>; + opp-supported-hw = <0x1f>; }; - opp-1080000000 { + opp-792000000-l { clock-latency-ns = <244144>; /* 8 32k periods */ - opp-hz = /bits/ 64 <1080000000>; + opp-hz = /bits/ 64 <792000000>; - opp-microvolt-speed0 = <1060000>; - opp-microvolt-speed1 = <880000>; - opp-microvolt-speed2 = <840000>; + opp-microvolt = <900000>; + opp-supported-hw = <0x02>; }; - opp-1320000000 { + opp-792000000-h { clock-latency-ns = <244144>; /* 8 32k periods */ - opp-hz = /bits/ 64 <1320000000>; + opp-hz = /bits/ 64 <792000000>; - opp-microvolt-speed0 = <1160000>; - opp-microvolt-speed1 = <940000>; - opp-microvolt-speed2 = <900000>; + opp-microvolt = <940000>; + opp-supported-hw = <0x10>; }; - opp-1488000000 { + opp-1512000000 { clock-latency-ns = <244144>; /* 8 32k periods */ - opp-hz = /bits/ 64 <1488000000>; + opp-hz = /bits/ 64 <1512000000>; - opp-microvolt-speed0 = <1160000>; - opp-microvolt-speed1 = <1000000>; - opp-microvolt-speed2 = <960000>; + opp-microvolt = <1100000>; + opp-supported-hw = <0x0a>; }; }; -- 2.35.8
From: Martin Botka <martin.botka@somainline.org> The AllWinner H616 SoC will use the (extended) H6 OPP driver, so add them to the cpufreq-dt blocklist, to not create the device twice. This also affects the closely related sibling SoCs H618 and H700. Signed-off-by: Martin Botka <martin.botka@somainline.org> Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- drivers/cpufreq/cpufreq-dt-platdev.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c index bd1e1357cef8e..3f98c4b31647a 100644 --- a/drivers/cpufreq/cpufreq-dt-platdev.c +++ b/drivers/cpufreq/cpufreq-dt-platdev.c @@ -104,6 +104,9 @@ static const struct of_device_id allowlist[] __initconst = { */ static const struct of_device_id blocklist[] __initconst = { { .compatible = "allwinner,sun50i-h6", }, + { .compatible = "allwinner,sun50i-h616", }, + { .compatible = "allwinner,sun50i-h618", }, + { .compatible = "allwinner,sun50i-h700", }, { .compatible = "apple,arm-platform", }, -- 2.35.8
From: Martin Botka <martin.botka@somainline.org> The "SoC ID revision" as provided via the SMCCC SOCID interface can be valuable information for drivers, when certain functionality depends on a die revision, for instance. One example is the sun50i-cpufreq-nvmem driver, which needs this information to determine the speed bin of the SoC. Export the arm_smccc_get_soc_id_revision() function so that it can be called by any driver. Signed-off-by: Martin Botka <martin.botka@somainline.org> Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- drivers/firmware/smccc/smccc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/firmware/smccc/smccc.c b/drivers/firmware/smccc/smccc.c index db818f9dcb8ee..d670635914ecb 100644 --- a/drivers/firmware/smccc/smccc.c +++ b/drivers/firmware/smccc/smccc.c @@ -69,6 +69,7 @@ s32 arm_smccc_get_soc_id_revision(void) { return smccc_soc_id_revision; } +EXPORT_SYMBOL_GPL(arm_smccc_get_soc_id_revision); static int __init smccc_devices_init(void) { -- 2.35.8
This series adds cpufreq support for the Allwinner H616 SoC. This v2 is quite some rework compared to Martin's original series from about half a year ago[1]. The various H616 chips are rated into different speedbins at the factory, the bin index is then burned into the efuses. This is very similar to the H6, though the location of the speedbin fuse and its encoding differs. Also the die revision has a say here, we can derive this from the SoC ID, already provided by TF-A through the SMCCC SoC ID interface. On top of that not all chips are qualified to reach the full 1.5GHz, and the BSP kernel describes different OPPs for each speedbin. This requires to add support for the opp-supported-hw DT property, to be able to describe those requirements properly. Patch 1/8 exports the SoC ID function, so that we can call it from our driver. Patch 2/8 blocks the affected SoCs from the generic DT cpufreq driver, patch 3/8 adds the DT binding documentation. Patch 4/8 refactors the existing speedbin determination for the H6, to be able to plug in the H616 version later more easily. Patch 5/8 adds support for the opp-supported-hw property. This is done in a generic way, so it's usable for other SoCs as well, and the code will figure out if the current DT requires use of this feature. Patch 6/8 then eventually adds the H616 bits to the driver, and ties that to the new compatible string. Patch 7/8 add the CPU OPP table as a .dtsi to the DT directory, the values in there were taken from the BSP source. Patch 8/8 then enables the OPPs for all boards we have DTs for. Please have a look, especially patch 5/8 might need some discussion. This is based on v6.8, with the THS series on top, which should reach mainline in the next days. I plan to send a rebased version after -rc1, but wanted to start the discussion early. Cheers, Andre [1] https://lore.kernel.org/linux-sunxi/20230904-cpufreq-h616-v1-0-b8842e525c43@somainline.org/T/#u Andre Przywara (2): cpufreq: sun50i: Add support for opp_supported_hw arm64: dts: allwinner: h616: enable DVFS for all boards Brandon Cheo Fusi (1): cpufreq: sun50i: Refactor speed bin decoding Martin Botka (5): firmware: smccc: Export revision soc_id function cpufreq: dt-platdev: Blocklist Allwinner H616/618 SoCs dt-bindings: opp: Describe H616 OPPs and opp-supported-hw cpufreq: sun50i: Add H616 support arm64: dts: allwinner: h616: Add CPU OPPs table .../allwinner,sun50i-h6-operating-points.yaml | 89 +++++---- .../sun50i-h616-bigtreetech-cb1.dtsi | 5 + .../dts/allwinner/sun50i-h616-cpu-opp.dtsi | 138 +++++++++++++ .../allwinner/sun50i-h616-orangepi-zero2.dts | 5 + .../dts/allwinner/sun50i-h616-x96-mate.dts | 5 + .../arm64/boot/dts/allwinner/sun50i-h616.dtsi | 4 + .../allwinner/sun50i-h618-orangepi-zero2w.dts | 5 + .../allwinner/sun50i-h618-orangepi-zero3.dts | 5 + .../sun50i-h618-transpeed-8k618-t.dts | 5 + drivers/cpufreq/cpufreq-dt-platdev.c | 3 + drivers/cpufreq/sun50i-cpufreq-nvmem.c | 189 +++++++++++++++--- drivers/firmware/smccc/smccc.c | 1 + 12 files changed, 379 insertions(+), 75 deletions(-) create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h616-cpu-opp.dtsi -- 2.35.8
Add dtsi file for T95 tv boxes and add initial support for T95 5G AXP313A variant with a board name H616-T95MAX-AXP313A-v3.0 Internal storage is not accessible due to lack of support for H616 NAND controller. Signed-off-by: Kamil Kasperski <ressetkk@gmail.com> --- arch/arm64/boot/dts/allwinner/Makefile | 1 + arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi | 109 +++++++++++++++++++++ .../dts/allwinner/sun50i-h616-t95max-axp313.dts | 85 ++++++++++++++++ 3 files changed, 195 insertions(+) diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile index 21149b346a60..294921f12b73 100644 --- a/arch/arm64/boot/dts/allwinner/Makefile +++ b/arch/arm64/boot/dts/allwinner/Makefile @@ -42,6 +42,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-tanix-tx6-mini.dtb dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-bigtreetech-cb1-manta.dtb dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-bigtreetech-pi.dtb dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-orangepi-zero2.dtb +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-t95max-axp313.dtb dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-x96-mate.dtb dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-longanpi-3h.dtb dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-orangepi-zero2w.dtb diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi new file mode 100644 index 000000000000..815cf2dac24b --- /dev/null +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95.dtsi @@ -0,0 +1,109 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/* + * Copyright (C) 2024 Kamil Kasperski <ressetkk@gmail.com> + * + * Common DT nodes for H616-based T95 TV boxes + * There are two versions reported with different PMIC variants. + */ + +#include "sun50i-h616.dtsi" + +#include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/interrupt-controller/arm-gic.h> + +/ { + aliases { + ethernet1 = &sdio_wifi; + serial0 = &uart0; + }; + + chosen { + stdout-path = "serial0:115200n8"; + }; + + reg_vcc5v: vcc5v { + /* board wide 5V supply directly from the DC input */ + compatible = "regulator-fixed"; + regulator-name = "vcc-5v"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + regulator-always-on; + }; + + reg_vcc3v3: vcc3v3 { + /* discrete 3.3V regulator */ + compatible = "regulator-fixed"; + regulator-name = "vcc-3v3"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + + wifi_pwrseq: wifi-pwrseq { + compatible = "mmc-pwrseq-simple"; + clocks = <&rtc CLK_OSC32K_FANOUT>; + clock-names = "ext_clock"; + pinctrl-0 = <&x32clk_fanout_pin>; + pinctrl-names = "default"; + reset-gpios = <&pio 6 18 GPIO_ACTIVE_LOW>; /* PG18 */ + }; +}; + +&ehci0 { + status = "okay"; +}; + +&ehci2 { + status = "okay"; +}; + +&ir { + status = "okay"; +}; + +&mmc0 { + cd-gpios = <&pio 8 16 GPIO_ACTIVE_LOW>; /* PI16 */ + bus-width = <4>; + status = "okay"; +}; + +&mmc1 { + mmc-pwrseq = <&wifi_pwrseq>; + bus-width = <4>; + non-removable; + status = "okay"; + + sdio_wifi: wifi@1 { + reg = <1>; + }; +}; + +&ohci0 { + status = "okay"; +}; + +&ohci2 { + status = "okay"; +}; + +&uart0 { + pinctrl-names = "default"; + pinctrl-0 = <&uart0_ph_pins>; + status = "okay"; +}; + +&uart1 { + pinctrl-names = "default"; + pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>; + uart-has-rtscts; + status = "okay"; +}; + +&usbotg { + dr_mode = "host"; /* USB A type receptable */ + status = "okay"; +}; + +&usbphy { + status = "okay"; +}; diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-t95max-axp313.dts b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95max-axp313.dts new file mode 100644 index 000000000000..c8650aca2407 --- /dev/null +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-t95max-axp313.dts @@ -0,0 +1,85 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/* + * Copyright (C) 2024 Kamil Kasperski <ressetkk@gmail.com> + * + * Configuration for T95 TV box with board label H616-T95MAX-AXP313A-v3.0 + */ + +/dts-v1/; + +#include "sun50i-h616-t95.dtsi" + +/ { + model = "T95 5G (AXP313)"; + compatible = "t95,t95max-axp313", "allwinner,sun50i-h616"; +}; + +&mmc0 { + vmmc-supply = <®_dldo1>; +}; + +&mmc1 { + vmmc-supply = <®_dldo1>; + vqmmc-supply = <®_aldo1>; +}; + +&r_i2c { + status = "okay"; + + axp313: pmic@36 { + compatible = "x-powers,axp313a"; + reg = <0x36>; + #interrupt-cells = <1>; + interrupt-controller; + interrupt-parent = <&pio>; + + vin1-supply = <®_vcc5v>; + vin2-supply = <®_vcc5v>; + vin3-supply = <®_vcc5v>; + + regulators { + reg_aldo1: aldo1 { + regulator-always-on; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-name = "vcc1v8"; + }; + + reg_dldo1: dldo1 { + regulator-always-on; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-name = "vcc3v3"; + }; + + reg_dcdc1: dcdc1 { + regulator-always-on; + regulator-min-microvolt = <810000>; + regulator-max-microvolt = <990000>; + regulator-name = "vdd-gpu-sys"; + }; + + reg_dcdc2: dcdc2 { + regulator-always-on; + regulator-min-microvolt = <810000>; + regulator-max-microvolt = <1100000>; + regulator-name = "vdd-cpu"; + }; + + reg_dcdc3: dcdc3 { + regulator-always-on; + regulator-min-microvolt = <1500000>; + regulator-max-microvolt = <1500000>; + regulator-name = "vdd-dram"; + }; + }; + }; +}; + +&pio { + vcc-pc-supply = <®_aldo1>; + vcc-pf-supply = <®_dldo1>; + vcc-pg-supply = <®_dldo1>; + vcc-ph-supply = <®_dldo1>; + vcc-pi-supply = <®_dldo1>; +}; -- 2.34.1
Add T95 AXP313 TV Box variant to dt-bindings. Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Signed-off-by: Kamil Kasperski <ressetkk@gmail.com> --- Documentation/devicetree/bindings/arm/sunxi.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml index 09d835db6db5..6fe137605ba5 100644 --- a/Documentation/devicetree/bindings/arm/sunxi.yaml +++ b/Documentation/devicetree/bindings/arm/sunxi.yaml @@ -860,6 +860,11 @@ properties: - const: allwinner,sl631 - const: allwinner,sun8i-v3 + - description: T95 5G (AXP313) TV Box + items: + - const: t95,t95max-axp313 + - const: allwinner,sun50i-h616 + - description: Tanix TX6 items: - const: oranth,tanix-tx6 -- 2.34.1
Add vendor prefix for T95 tv boxes of unnamed brand. It's hard to determine the actual vendor, so let's assume t95 as default vendor for all t95-branded tv boxes. Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Signed-off-by: Kamil Kasperski <ressetkk@gmail.com> --- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index 04505cb0b640..65025ad4dcd5 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -1416,6 +1416,8 @@ patternProperties: "^synopsys,.*": description: Synopsys, Inc. (deprecated, use snps) deprecated: true + "^t95,.*": + description: T95 "^tbs,.*": description: TBS Technologies "^tbs-biometrics,.*": -- 2.34.1