mbox series

[00/12] 8550 fixups

Message ID 20231218-topic-8550_fixes-v1-0-ce1272d77540@linaro.org
Headers show
Series 8550 fixups | expand

Message

Konrad Dybcio Dec. 18, 2023, 4:02 p.m. UTC
I found a couple of sneaky bugs concerning 8550, ranging from icc and clk,
to some usual omissions in the dts. This series attempts to amend them to
mostly prevent UB due to misconfiguration.

Patches 1-2 for icc, rest for qcom

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
Konrad Dybcio (12):
      interconnect: qcom: sm8550: Remove bogus per-RSC BCMs and nodes
      interconnect: qcom: sm8550: Enable sync_state
      clk: qcom: gcc-sm8550: Add the missing RETAIN_FF_ENABLE GDSC flag
      clk: qcom: gcc-sm8550: Mark the PCIe GDSCs votable
      clk: qcom: gcc-sm8550: use collapse-voting for PCIe GDSCs
      clk: qcom: gcc-sm8550: Mark RCGs shared where applicable
      clk: qcom: gpucc-sm8550: Update GPU PLL settings
      clk: qcom: dispcc-sm8550: Update disp PLL settings
      clk: qcom: dispcc-sm8550: Use the correct PLL configuration function
      arm64: dts: qcom: sm8550: Switch UFS from opp-table-hz to opp-v2
      arm64: dts: qcom: sm8550: Separate out X3 idle state
      arm64: dts: qcom: sm8550: Update idle state time requirements

 arch/arm64/boot/dts/qcom/sm8550.dtsi |  82 +++--
 drivers/clk/qcom/dispcc-sm8550.c     |  12 +-
 drivers/clk/qcom/gcc-sm8550.c        | 110 +++----
 drivers/clk/qcom/gpucc-sm8550.c      |   6 +-
 drivers/interconnect/qcom/sm8550.c   | 575 +----------------------------------
 drivers/interconnect/qcom/sm8550.h   | 284 ++++++++---------
 6 files changed, 257 insertions(+), 812 deletions(-)
---
base-commit: ceb2fe0d438644e1de06b9a6468a1fb8e2199c70
change-id: 20231218-topic-8550_fixes-2bc63cdfe1fd

Best regards,

Comments

Nitin Rawat Dec. 18, 2023, 4:35 p.m. UTC | #1
On 12/18/2023 9:32 PM, Konrad Dybcio wrote:
> Now that the non-legacy form of OPP is supported within the UFS driver,
> go ahead and switch to it, adding support for more intermediate freq/power
> states.
> 
> In doing so, add the CX RPMhPD under GCC to make sure at least some of
> the power state requirements are *actually* propagated up the stack.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>   arch/arm64/boot/dts/qcom/sm8550.dtsi | 50 +++++++++++++++++++++++++++++-------
>   1 file changed, 41 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
> index d707d15cea5b..d6edd54f3ad3 100644
> --- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
> @@ -1930,6 +1930,7 @@ ufs_mem_hc: ufs@1d84000 {
>   			iommus = <&apps_smmu 0x60 0x0>;
>   			dma-coherent;
>   
> +			operating-points-v2 = <&ufs_opp_table>;
>   			interconnects = <&aggre1_noc MASTER_UFS_MEM 0 &mc_virt SLAVE_EBI1 0>,
>   					<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_UFS_MEM_CFG 0>;
>   
> @@ -1950,18 +1951,49 @@ ufs_mem_hc: ufs@1d84000 {
>   				 <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>,
>   				 <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>,
>   				 <&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>;
> -			freq-table-hz =
> -				<75000000 300000000>,
> -				<0 0>,
> -				<0 0>,
> -				<75000000 300000000>,
> -				<100000000 403000000>,
> -				<0 0>,
> -				<0 0>,
> -				<0 0>;
>   			qcom,ice = <&ice>;
>   
>   			status = "disabled";
> +
> +			ufs_opp_table: opp-table {
> +				compatible = "operating-points-v2";
> +
> +				opp-75000000 {
> +					opp-hz = /bits/ 64 <75000000>,
> +						 /bits/ 64 <0>,
> +						 /bits/ 64 <0>,
> +						 /bits/ 64 <75000000>,
> +						 /bits/ 64 <100000000>,
> +						 /bits/ 64 <0>,
> +						 /bits/ 64 <0>,
> +						 /bits/ 64 <0>;
> +					required-opps = <&rpmhpd_opp_low_svs>;
> +				};
> +
> +				opp-150000000 {
> +					opp-hz = /bits/ 64 <150000000>,
> +						 /bits/ 64 <0>,
> +						 /bits/ 64 <0>,
> +						 /bits/ 64 <150000000>,
> +						 /bits/ 64 <100000000> > +						 /bits/ 64 <0>,
> +						 /bits/ 64 <0>,
> +						 /bits/ 64 <0>;
> +					required-opps = <&rpmhpd_opp_svs>;
> +				};
> +
> +				opp-300000000 {
> +					opp-hz = /bits/ 64 <300000000>,
> +						 /bits/ 64 <0>,
> +						 /bits/ 64 <0>,
> +						 /bits/ 64 <300000000>,
> +						 /bits/ 64 <100000000>,
Hi Konrad,

This entry is for ICE clock ? Shouldn't the entry be 403000000 ?
Same for svs as well ?

> +						 /bits/ 64 <0>,
> +						 /bits/ 64 <0>,
> +						 /bits/ 64 <0>;
> +					required-opps = <&rpmhpd_opp_nom>;
> +				};
> +			};
>   		};
>   
>   		ice: crypto@1d88000 {
> 

Regards,
Nitin
Bjorn Andersson Dec. 19, 2023, 5:28 p.m. UTC | #2
On Mon, Dec 18, 2023 at 05:02:02PM +0100, Konrad Dybcio wrote:
> The downstream kernel has infrastructure for passing votes from different
> interconnect nodes onto different RPMh RSCs. This neither implemented, not
> is going to be implemented upstream (in favor of a different solution
> using ICC tags through the same node).
> 
> Unfortunately, as it happens, meaningless (in the upstream context) parts
> of the vendor driver were copied, ending up causing havoc - since all
> "per-RSC" (in quotes because they all point to the main APPS one) BCMs
> defined within the driver overwrite the value in RPMh on every
> aggregation.
> 
> To both avoid keeping bogus code around and possibly introducing
> impossible-to-track-down bugs (busses shutting down for no reason), get
> rid of the duplicated BCMs and their associated ICC nodes.
> 
> Fixes: e6f0d6a30f73 ("interconnect: qcom: Add SM8550 interconnect provider driver")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Reviewed-by: Bjorn Andersson <andersson@kernel.org>

Regards,
Bjorn
Bjorn Andersson Dec. 19, 2023, 7:33 p.m. UTC | #3
On Mon, 18 Dec 2023 17:02:01 +0100, Konrad Dybcio wrote:
> I found a couple of sneaky bugs concerning 8550, ranging from icc and clk,
> to some usual omissions in the dts. This series attempts to amend them to
> mostly prevent UB due to misconfiguration.
> 
> Patches 1-2 for icc, rest for qcom
> 
> 
> [...]

Applied, thanks!

[11/12] arm64: dts: qcom: sm8550: Separate out X3 idle state
        commit: 28b735232d5e16a34f98dbac1e7b5401c1c16d89
[12/12] arm64: dts: qcom: sm8550: Update idle state time requirements
        commit: ad6556fb45d4ab91ad786a2025cbe2b0f2e6cf77

Best regards,