| Message ID | 20230329-topic-adreno_opp-v1-3-24d34ac6f007@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Improve some Adreno Device Tree nodes | expand |
On Wed, 29 Mar 2023 at 22:17, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > Remove the self-explanatory comment about opp-supported-hw contents, > add required-opps to ensure reasonable power domain levels are voted > for (currently we've been piggybacking off of miracles and MDP votes) > and add newlines between each subnode. I'm not sure this is 100% correct. The values that you add are correct for the voltage scaling case. However, based on the vendor kernel sources I think that MX should only be scaled if the voltage is scaled too. I might be wrong here. > > Fixes: 69cc3114ab0f ("arm64: dts: Add Adreno GPU definitions") > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > --- > arch/arm64/boot/dts/qcom/msm8996.dtsi | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi > index 4dd37f72e018..62ad30e94f40 100644 > --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi > +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi > @@ -1244,37 +1244,45 @@ gpu: gpu@b00000 { > gpu_opp_table: opp-table { > compatible = "operating-points-v2"; > > - /* > - * 624Mhz is only available on speed bins 0 and 3. > - * 560Mhz is only available on speed bins 0, 2 and 3. > - * All the rest are available on all bins of the hardware. > - */ > opp-624000000 { > opp-hz = /bits/ 64 <624000000>; > + required-opps = <&rpmpd_opp_turbo>; > opp-supported-hw = <0x09>; > }; > + > opp-560000000 { > opp-hz = /bits/ 64 <560000000>; > + required-opps = <&rpmpd_opp_turbo>; > opp-supported-hw = <0x0d>; > }; > + > opp-510000000 { > opp-hz = /bits/ 64 <510000000>; > + required-opps = <&rpmpd_opp_nom>; > opp-supported-hw = <0xff>; > }; > + > opp-401800000 { > opp-hz = /bits/ 64 <401800000>; > + required-opps = <&rpmpd_opp_nom>; > opp-supported-hw = <0xff>; > }; > + > opp-315000000 { > opp-hz = /bits/ 64 <315000000>; > + required-opps = <&rpmpd_opp_svs>; > opp-supported-hw = <0xff>; > }; > + > opp-214000000 { > opp-hz = /bits/ 64 <214000000>; > + required-opps = <&rpmpd_opp_svs>; > opp-supported-hw = <0xff>; > }; > + > opp-133000000 { > opp-hz = /bits/ 64 <133000000>; > + required-opps = <&rpmpd_opp_svs>; > opp-supported-hw = <0xff>; > }; > }; > > -- > 2.40.0 >
On 29.03.2023 23:32, Dmitry Baryshkov wrote: > On Wed, 29 Mar 2023 at 22:17, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >> >> Remove the self-explanatory comment about opp-supported-hw contents, >> add required-opps to ensure reasonable power domain levels are voted >> for (currently we've been piggybacking off of miracles and MDP votes) >> and add newlines between each subnode. > > I'm not sure this is 100% correct. The values that you add are correct > for the voltage scaling case. However, based on the vendor kernel > sources I think that MX should only be scaled if the voltage is scaled > too. I might be wrong here. MX must be >= CX (and GX), so this should bring no harm. (citation needed, but that seems to hold true..) Konrad > >> >> Fixes: 69cc3114ab0f ("arm64: dts: Add Adreno GPU definitions") >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >> --- >> arch/arm64/boot/dts/qcom/msm8996.dtsi | 18 +++++++++++++----- >> 1 file changed, 13 insertions(+), 5 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi >> index 4dd37f72e018..62ad30e94f40 100644 >> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi >> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi >> @@ -1244,37 +1244,45 @@ gpu: gpu@b00000 { >> gpu_opp_table: opp-table { >> compatible = "operating-points-v2"; >> >> - /* >> - * 624Mhz is only available on speed bins 0 and 3. >> - * 560Mhz is only available on speed bins 0, 2 and 3. >> - * All the rest are available on all bins of the hardware. >> - */ >> opp-624000000 { >> opp-hz = /bits/ 64 <624000000>; >> + required-opps = <&rpmpd_opp_turbo>; >> opp-supported-hw = <0x09>; >> }; >> + >> opp-560000000 { >> opp-hz = /bits/ 64 <560000000>; >> + required-opps = <&rpmpd_opp_turbo>; >> opp-supported-hw = <0x0d>; >> }; >> + >> opp-510000000 { >> opp-hz = /bits/ 64 <510000000>; >> + required-opps = <&rpmpd_opp_nom>; >> opp-supported-hw = <0xff>; >> }; >> + >> opp-401800000 { >> opp-hz = /bits/ 64 <401800000>; >> + required-opps = <&rpmpd_opp_nom>; >> opp-supported-hw = <0xff>; >> }; >> + >> opp-315000000 { >> opp-hz = /bits/ 64 <315000000>; >> + required-opps = <&rpmpd_opp_svs>; >> opp-supported-hw = <0xff>; >> }; >> + >> opp-214000000 { >> opp-hz = /bits/ 64 <214000000>; >> + required-opps = <&rpmpd_opp_svs>; >> opp-supported-hw = <0xff>; >> }; >> + >> opp-133000000 { >> opp-hz = /bits/ 64 <133000000>; >> + required-opps = <&rpmpd_opp_svs>; >> opp-supported-hw = <0xff>; >> }; >> }; >> >> -- >> 2.40.0 >> > >
On 30.03.2023 12:57, Konrad Dybcio wrote: > > > On 29.03.2023 23:32, Dmitry Baryshkov wrote: >> On Wed, 29 Mar 2023 at 22:17, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >>> >>> Remove the self-explanatory comment about opp-supported-hw contents, >>> add required-opps to ensure reasonable power domain levels are voted >>> for (currently we've been piggybacking off of miracles and MDP votes) >>> and add newlines between each subnode. >> >> I'm not sure this is 100% correct. The values that you add are correct >> for the voltage scaling case. However, based on the vendor kernel >> sources I think that MX should only be scaled if the voltage is scaled >> too. I might be wrong here. > MX must be >= CX (and GX), so this should bring no harm. > > (citation needed, but that seems to hold true..) With that in mind, would you ack these patches Dmitry? Konrad > > Konrad >> >>> >>> Fixes: 69cc3114ab0f ("arm64: dts: Add Adreno GPU definitions") >>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >>> --- >>> arch/arm64/boot/dts/qcom/msm8996.dtsi | 18 +++++++++++++----- >>> 1 file changed, 13 insertions(+), 5 deletions(-) >>> >>> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi >>> index 4dd37f72e018..62ad30e94f40 100644 >>> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi >>> @@ -1244,37 +1244,45 @@ gpu: gpu@b00000 { >>> gpu_opp_table: opp-table { >>> compatible = "operating-points-v2"; >>> >>> - /* >>> - * 624Mhz is only available on speed bins 0 and 3. >>> - * 560Mhz is only available on speed bins 0, 2 and 3. >>> - * All the rest are available on all bins of the hardware. >>> - */ >>> opp-624000000 { >>> opp-hz = /bits/ 64 <624000000>; >>> + required-opps = <&rpmpd_opp_turbo>; >>> opp-supported-hw = <0x09>; >>> }; >>> + >>> opp-560000000 { >>> opp-hz = /bits/ 64 <560000000>; >>> + required-opps = <&rpmpd_opp_turbo>; >>> opp-supported-hw = <0x0d>; >>> }; >>> + >>> opp-510000000 { >>> opp-hz = /bits/ 64 <510000000>; >>> + required-opps = <&rpmpd_opp_nom>; >>> opp-supported-hw = <0xff>; >>> }; >>> + >>> opp-401800000 { >>> opp-hz = /bits/ 64 <401800000>; >>> + required-opps = <&rpmpd_opp_nom>; >>> opp-supported-hw = <0xff>; >>> }; >>> + >>> opp-315000000 { >>> opp-hz = /bits/ 64 <315000000>; >>> + required-opps = <&rpmpd_opp_svs>; >>> opp-supported-hw = <0xff>; >>> }; >>> + >>> opp-214000000 { >>> opp-hz = /bits/ 64 <214000000>; >>> + required-opps = <&rpmpd_opp_svs>; >>> opp-supported-hw = <0xff>; >>> }; >>> + >>> opp-133000000 { >>> opp-hz = /bits/ 64 <133000000>; >>> + required-opps = <&rpmpd_opp_svs>; >>> opp-supported-hw = <0xff>; >>> }; >>> }; >>> >>> -- >>> 2.40.0 >>> >> >>
On Fri, 26 May 2023 at 16:30, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > > > On 30.03.2023 12:57, Konrad Dybcio wrote: > > > > > > On 29.03.2023 23:32, Dmitry Baryshkov wrote: > >> On Wed, 29 Mar 2023 at 22:17, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > >>> > >>> Remove the self-explanatory comment about opp-supported-hw contents, > >>> add required-opps to ensure reasonable power domain levels are voted > >>> for (currently we've been piggybacking off of miracles and MDP votes) > >>> and add newlines between each subnode. > >> > >> I'm not sure this is 100% correct. The values that you add are correct > >> for the voltage scaling case. However, based on the vendor kernel > >> sources I think that MX should only be scaled if the voltage is scaled > >> too. I might be wrong here. > > MX must be >= CX (and GX), so this should bring no harm. > > > > (citation needed, but that seems to hold true..) > With that in mind, would you ack these patches Dmitry? I remember that we should not overwolt the memory, it might cause memory cells degradation. So MX >= CX & MX <= CX + delta. I have pinged Jordan if he can provide feedback on my a530/540 voltage programming attempt ([1]), but got no response from him up to now. [1] https://git.linaro.org/people/dmitry.baryshkov/kernel.git/log/?h=msm8996-upstream I will try doing more experiments with the mentioned branch. Maybe it breaks because of the missing MX vote or because of something suchalike. I'm 80% sure in the voltage level (which were generated by the CPR3 on my db820c board) and 95% sure in the programming sequence. > > Konrad > > > > Konrad > >> > >>> > >>> Fixes: 69cc3114ab0f ("arm64: dts: Add Adreno GPU definitions") > >>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > >>> --- > >>> arch/arm64/boot/dts/qcom/msm8996.dtsi | 18 +++++++++++++----- > >>> 1 file changed, 13 insertions(+), 5 deletions(-) > >>> > >>> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi > >>> index 4dd37f72e018..62ad30e94f40 100644 > >>> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi > >>> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi > >>> @@ -1244,37 +1244,45 @@ gpu: gpu@b00000 { > >>> gpu_opp_table: opp-table { > >>> compatible = "operating-points-v2"; > >>> > >>> - /* > >>> - * 624Mhz is only available on speed bins 0 and 3. > >>> - * 560Mhz is only available on speed bins 0, 2 and 3. > >>> - * All the rest are available on all bins of the hardware. > >>> - */ > >>> opp-624000000 { > >>> opp-hz = /bits/ 64 <624000000>; > >>> + required-opps = <&rpmpd_opp_turbo>; > >>> opp-supported-hw = <0x09>; > >>> }; > >>> + > >>> opp-560000000 { > >>> opp-hz = /bits/ 64 <560000000>; > >>> + required-opps = <&rpmpd_opp_turbo>; > >>> opp-supported-hw = <0x0d>; > >>> }; > >>> + > >>> opp-510000000 { > >>> opp-hz = /bits/ 64 <510000000>; > >>> + required-opps = <&rpmpd_opp_nom>; > >>> opp-supported-hw = <0xff>; > >>> }; > >>> + > >>> opp-401800000 { > >>> opp-hz = /bits/ 64 <401800000>; > >>> + required-opps = <&rpmpd_opp_nom>; > >>> opp-supported-hw = <0xff>; > >>> }; > >>> + > >>> opp-315000000 { > >>> opp-hz = /bits/ 64 <315000000>; > >>> + required-opps = <&rpmpd_opp_svs>; > >>> opp-supported-hw = <0xff>; > >>> }; > >>> + > >>> opp-214000000 { > >>> opp-hz = /bits/ 64 <214000000>; > >>> + required-opps = <&rpmpd_opp_svs>; > >>> opp-supported-hw = <0xff>; > >>> }; > >>> + > >>> opp-133000000 { > >>> opp-hz = /bits/ 64 <133000000>; > >>> + required-opps = <&rpmpd_opp_svs>; > >>> opp-supported-hw = <0xff>; > >>> }; > >>> }; > >>> > >>> -- > >>> 2.40.0 > >>> > >> > >>
On 26.05.2023 15:30, Konrad Dybcio wrote: > > > On 30.03.2023 12:57, Konrad Dybcio wrote: >> >> >> On 29.03.2023 23:32, Dmitry Baryshkov wrote: >>> On Wed, 29 Mar 2023 at 22:17, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >>>> >>>> Remove the self-explanatory comment about opp-supported-hw contents, >>>> add required-opps to ensure reasonable power domain levels are voted >>>> for (currently we've been piggybacking off of miracles and MDP votes) >>>> and add newlines between each subnode. >>> >>> I'm not sure this is 100% correct. The values that you add are correct >>> for the voltage scaling case. However, based on the vendor kernel >>> sources I think that MX should only be scaled if the voltage is scaled >>> too. I might be wrong here. >> MX must be >= CX (and GX), so this should bring no harm. >> >> (citation needed, but that seems to hold true..) > With that in mind, would you ack these patches Dmitry? Bump? Konrad
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi index 4dd37f72e018..62ad30e94f40 100644 --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi @@ -1244,37 +1244,45 @@ gpu: gpu@b00000 { gpu_opp_table: opp-table { compatible = "operating-points-v2"; - /* - * 624Mhz is only available on speed bins 0 and 3. - * 560Mhz is only available on speed bins 0, 2 and 3. - * All the rest are available on all bins of the hardware. - */ opp-624000000 { opp-hz = /bits/ 64 <624000000>; + required-opps = <&rpmpd_opp_turbo>; opp-supported-hw = <0x09>; }; + opp-560000000 { opp-hz = /bits/ 64 <560000000>; + required-opps = <&rpmpd_opp_turbo>; opp-supported-hw = <0x0d>; }; + opp-510000000 { opp-hz = /bits/ 64 <510000000>; + required-opps = <&rpmpd_opp_nom>; opp-supported-hw = <0xff>; }; + opp-401800000 { opp-hz = /bits/ 64 <401800000>; + required-opps = <&rpmpd_opp_nom>; opp-supported-hw = <0xff>; }; + opp-315000000 { opp-hz = /bits/ 64 <315000000>; + required-opps = <&rpmpd_opp_svs>; opp-supported-hw = <0xff>; }; + opp-214000000 { opp-hz = /bits/ 64 <214000000>; + required-opps = <&rpmpd_opp_svs>; opp-supported-hw = <0xff>; }; + opp-133000000 { opp-hz = /bits/ 64 <133000000>; + required-opps = <&rpmpd_opp_svs>; opp-supported-hw = <0xff>; }; };
Remove the self-explanatory comment about opp-supported-hw contents, add required-opps to ensure reasonable power domain levels are voted for (currently we've been piggybacking off of miracles and MDP votes) and add newlines between each subnode. Fixes: 69cc3114ab0f ("arm64: dts: Add Adreno GPU definitions") Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> --- arch/arm64/boot/dts/qcom/msm8996.dtsi | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)