| Message ID | 20241012-gpu-acd-v1-0-1e5e91aa95b6@quicinc.com |
|---|---|
| Headers | show |
| Series | Support for GPU ACD feature on Adreno X1-85 | expand |
On Sat, Oct 12, 2024 at 01:59:30AM +0530, Akhil P Oommen wrote: > Update GPU node to include acd level values. > > Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com> > --- > arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi > index a36076e3c56b..e6c500480eb1 100644 > --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi > +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi > @@ -3323,60 +3323,69 @@ zap-shader { > }; > > gpu_opp_table: opp-table { > - compatible = "operating-points-v2"; > + compatible = "operating-points-v2-adreno"; This nicely breaks all existing users of this DTS. Sorry, no. We are way past initial bringup/development. One year past. Best regards, Krzysztof
On Mon, Oct 14, 2024 at 09:40:13AM +0200, Krzysztof Kozlowski wrote: > On Sat, Oct 12, 2024 at 01:59:30AM +0530, Akhil P Oommen wrote: > > Update GPU node to include acd level values. > > > > Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com> > > --- > > arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi > > index a36076e3c56b..e6c500480eb1 100644 > > --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi > > +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi > > @@ -3323,60 +3323,69 @@ zap-shader { > > }; > > > > gpu_opp_table: opp-table { > > - compatible = "operating-points-v2"; > > + compatible = "operating-points-v2-adreno"; > > This nicely breaks all existing users of this DTS. Sorry, no. We are way > past initial bringup/development. One year past. It is not obvious to me how it breaks backward compatibility. Could you please elaborate a bit? I am aware that drivers should be backward compatible with DT, but not the other way. Are we talking about kernels other than Linux? Also, does including "operating-points-v2" too here help? -Akhil. > > Best regards, > Krzysztof >
On Wed, Oct 16, 2024 at 09:50:04AM +0200, Krzysztof Kozlowski wrote: > On 15/10/2024 21:35, Akhil P Oommen wrote: > > On Mon, Oct 14, 2024 at 09:40:13AM +0200, Krzysztof Kozlowski wrote: > >> On Sat, Oct 12, 2024 at 01:59:30AM +0530, Akhil P Oommen wrote: > >>> Update GPU node to include acd level values. > >>> > >>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com> > >>> --- > >>> arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 ++++++++++- > >>> 1 file changed, 10 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi > >>> index a36076e3c56b..e6c500480eb1 100644 > >>> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi > >>> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi > >>> @@ -3323,60 +3323,69 @@ zap-shader { > >>> }; > >>> > >>> gpu_opp_table: opp-table { > >>> - compatible = "operating-points-v2"; > >>> + compatible = "operating-points-v2-adreno"; > >> > >> This nicely breaks all existing users of this DTS. Sorry, no. We are way > >> past initial bringup/development. One year past. How do I identify when devicetree is considered stable? An arbitrary time period doesn't sound like a good idea. Is there a general consensus on this? X1E chipset is still considered under development at least till the end of this year, right? > > > > It is not obvious to me how it breaks backward compatibility. Could you > > I did not say "backward compatibility". I said existing users. > > > please elaborate a bit? I am aware that drivers should be backward > > compatible with DT, but not the other way. Are we talking about kernels other > > than Linux? > > > > Boot OpenBSD with new DTS. Previously: worked fine. Now: works less fine. > > We had exact talk about this during LPC. > > > Also, does including "operating-points-v2" too here help? > > Fallback? Yes, assuming these are compatible. Not much is explained in > the commit msg, except duplicating diff. That's not what the commit msg > is for. Okay. We can keep the fallback compatible string. -Akhil. > > > Best regards, > Krzysztof >
On 17/10/2024 08:12, Akhil P Oommen wrote: > On Wed, Oct 16, 2024 at 09:50:04AM +0200, Krzysztof Kozlowski wrote: >> On 15/10/2024 21:35, Akhil P Oommen wrote: >>> On Mon, Oct 14, 2024 at 09:40:13AM +0200, Krzysztof Kozlowski wrote: >>>> On Sat, Oct 12, 2024 at 01:59:30AM +0530, Akhil P Oommen wrote: >>>>> Update GPU node to include acd level values. >>>>> >>>>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com> >>>>> --- >>>>> arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 ++++++++++- >>>>> 1 file changed, 10 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi >>>>> index a36076e3c56b..e6c500480eb1 100644 >>>>> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi >>>>> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi >>>>> @@ -3323,60 +3323,69 @@ zap-shader { >>>>> }; >>>>> >>>>> gpu_opp_table: opp-table { >>>>> - compatible = "operating-points-v2"; >>>>> + compatible = "operating-points-v2-adreno"; >>>> >>>> This nicely breaks all existing users of this DTS. Sorry, no. We are way >>>> past initial bringup/development. One year past. > > How do I identify when devicetree is considered stable? An arbitrary > time period doesn't sound like a good idea. Is there a general consensus > on this? > > X1E chipset is still considered under development at least till the end of this > year, right? Stable could be when people already get their consumer/final product with it. I got some weeks ago Lenovo T14s laptop and since yesterday working fine with Ubuntu: https://discourse.ubuntu.com/t/ubuntu-24-10-concept-snapdragon-x-elite/48800 All chipsets are under development, even old SM8450, but we avoid breaking it while doing that. Best regards, Krzysztof
On 11.10.2024 10:29 PM, Akhil P Oommen wrote: > ACD a.k.a Adaptive Clock Distribution is a feature which helps to reduce > the power consumption. In some chipsets, it is also a requirement to > support higher GPU frequencies. This patch adds support for GPU ACD by > sending necessary data to GMU and AOSS. The feature support for the > chipset is detected based on devicetree data. > > Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com> > --- [...] > + > + /* Initialize qmp node to talk to AOSS */ > + gmu->qmp = qmp_get(gmu->dev); > + if (IS_ERR(gmu->qmp)) { > + cmd->enable_by_level = 0; > + return dev_err_probe(gmu->dev, PTR_ERR(gmu->qmp), "Failed to initialize qmp\n"); > + } I'm still in favor of keeping qmp_get where it currently is, so that probe can fail/defer faster Konrad
On Mon, Oct 21, 2024 at 11:38:41AM +0200, Konrad Dybcio wrote: > On 11.10.2024 10:29 PM, Akhil P Oommen wrote: > > ACD a.k.a Adaptive Clock Distribution is a feature which helps to reduce > > the power consumption. In some chipsets, it is also a requirement to > > support higher GPU frequencies. This patch adds support for GPU ACD by > > sending necessary data to GMU and AOSS. The feature support for the > > chipset is detected based on devicetree data. > > > > Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com> > > --- > > [...] > > > + > > + /* Initialize qmp node to talk to AOSS */ > > + gmu->qmp = qmp_get(gmu->dev); > > + if (IS_ERR(gmu->qmp)) { > > + cmd->enable_by_level = 0; > > + return dev_err_probe(gmu->dev, PTR_ERR(gmu->qmp), "Failed to initialize qmp\n"); > > + } > > I'm still in favor of keeping qmp_get where it currently is, so that > probe can fail/defer faster Sorry, I somehow missed this email from you until now. If it fails, then it probably doesn't matter if it is a bit late. But for defer, isn't there some optimizations to track the dependency from devicetree data? I am not entirely sure! Since qmp node is related to ACD, I felt it is better to: 1. Keep all acd probe related code in a single place. 2. Be more opportunistic in skipping qmp_get() wherever possible. But if you still have strong opinion on this, I can move it back in the next revision (v3). -Akhil > > Konrad
On 22.10.2024 12:09 AM, Akhil P Oommen wrote: > On Mon, Oct 21, 2024 at 11:38:41AM +0200, Konrad Dybcio wrote: >> On 11.10.2024 10:29 PM, Akhil P Oommen wrote: >>> ACD a.k.a Adaptive Clock Distribution is a feature which helps to reduce >>> the power consumption. In some chipsets, it is also a requirement to >>> support higher GPU frequencies. This patch adds support for GPU ACD by >>> sending necessary data to GMU and AOSS. The feature support for the >>> chipset is detected based on devicetree data. >>> >>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com> >>> --- >> >> [...] >> >>> + >>> + /* Initialize qmp node to talk to AOSS */ >>> + gmu->qmp = qmp_get(gmu->dev); >>> + if (IS_ERR(gmu->qmp)) { >>> + cmd->enable_by_level = 0; >>> + return dev_err_probe(gmu->dev, PTR_ERR(gmu->qmp), "Failed to initialize qmp\n"); >>> + } >> >> I'm still in favor of keeping qmp_get where it currently is, so that >> probe can fail/defer faster > > Sorry, I somehow missed this email from you until now. > > If it fails, then it probably doesn't matter if it is a bit late. But for defer, isn't there > some optimizations to track the dependency from devicetree data? I am > not entirely sure! There's devlink for clocks/supplies etc, it doesn't apply universally for all phandle references IIUC. > > Since qmp node is related to ACD, I felt it is better to: > 1. Keep all acd probe related code in a single place. > 2. Be more opportunistic in skipping qmp_get() wherever possible. > > But if you still have strong opinion on this, I can move it back in the > next revision (v3). I suppose the answer is yes, I have a strong opinion :D Konrad
On 11/10/2024 22:29, Akhil P Oommen wrote: > ACD a.k.a Adaptive Clock Distribution is a feature which helps to reduce > the power consumption. In some chipsets, it is also a requirement to > support higher GPU frequencies. This patch adds support for GPU ACD by > sending necessary data to GMU and AOSS. The feature support for the > chipset is detected based on devicetree data. > > Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com> > --- > drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 81 ++++++++++++++++++++++++++++------- > drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 1 + > drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 36 ++++++++++++++++ > drivers/gpu/drm/msm/adreno/a6xx_hfi.h | 21 +++++++++ > 4 files changed, 124 insertions(+), 15 deletions(-) > <snip> > + > +static int a6xx_hfi_enable_acd(struct a6xx_gmu *gmu) > +{ > + struct a6xx_hfi_acd_table *acd_table = &gmu->acd_table; > + struct a6xx_hfi_msg_feature_ctrl msg = { > + .feature = HFI_FEATURE_ACD, > + .enable = 1, > + .data = 0, > + }; > + int ret; > + > + if (!acd_table->enable_by_level) > + return 0; > + > + /* Enable ACD feature at GMU */ > + ret = a6xx_hfi_send_msg(gmu, HFI_H2F_FEATURE_CTRL, &msg, sizeof(msg), NULL, 0); > + if (ret) { > + DRM_DEV_ERROR(gmu->dev, "Unable to enable ACD (%d)\n", ret); > + return ret; > + } > + > + /* Send ACD table to GMU */ > + ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, &msg, sizeof(msg), NULL, 0); This looks wrong, in this exact code, you never use the acd_table... perhaps it should be acd_table here > + if (ret) { > + DRM_DEV_ERROR(gmu->dev, "Unable to ACD table (%d)\n", ret); > + return ret; > + } > + > + return 0; > +} > + > static int a6xx_hfi_send_test(struct a6xx_gmu *gmu) > { > struct a6xx_hfi_msg_test msg = { 0 }; > @@ -756,6 +788,10 @@ int a6xx_hfi_start(struct a6xx_gmu *gmu, int boot_state) > if (ret) > return ret; > > + ret = a6xx_hfi_enable_acd(gmu); > + if (ret) > + return ret; > + > ret = a6xx_hfi_send_core_fw_start(gmu); > if (ret) > return ret; > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h b/drivers/gpu/drm/msm/adreno/a6xx_hfi.h > index 528110169398..51864c8ad0e6 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h > +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.h > @@ -151,12 +151,33 @@ struct a6xx_hfi_msg_test { > u32 header; > }; > > +#define HFI_H2F_MSG_ACD 7 > +#define MAX_ACD_STRIDE 2 > + > +struct a6xx_hfi_acd_table { > + u32 header; > + u32 version; > + u32 enable_by_level; > + u32 stride; > + u32 num_levels; > + u32 data[16 * MAX_ACD_STRIDE]; > +}; > + > #define HFI_H2F_MSG_START 10 > > struct a6xx_hfi_msg_start { > u32 header; > }; > > +#define HFI_H2F_FEATURE_CTRL 11 > + > +struct a6xx_hfi_msg_feature_ctrl { > + u32 header; > + u32 feature; > + u32 enable; > + u32 data; > +}; > + > #define HFI_H2F_MSG_CORE_FW_START 14 > > struct a6xx_hfi_msg_core_fw_start { > Thanks, Neil
On 11/4/2024 9:14 PM, neil.armstrong@linaro.org wrote: > On 11/10/2024 22:29, Akhil P Oommen wrote: >> ACD a.k.a Adaptive Clock Distribution is a feature which helps to reduce >> the power consumption. In some chipsets, it is also a requirement to >> support higher GPU frequencies. This patch adds support for GPU ACD by >> sending necessary data to GMU and AOSS. The feature support for the >> chipset is detected based on devicetree data. >> >> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com> >> --- >> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 81 +++++++++++++++++++++++++ >> +++------- >> drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 1 + >> drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 36 ++++++++++++++++ >> drivers/gpu/drm/msm/adreno/a6xx_hfi.h | 21 +++++++++ >> 4 files changed, 124 insertions(+), 15 deletions(-) >> > > <snip> > >> + >> +static int a6xx_hfi_enable_acd(struct a6xx_gmu *gmu) >> +{ >> + struct a6xx_hfi_acd_table *acd_table = &gmu->acd_table; >> + struct a6xx_hfi_msg_feature_ctrl msg = { >> + .feature = HFI_FEATURE_ACD, >> + .enable = 1, >> + .data = 0, >> + }; >> + int ret; >> + >> + if (!acd_table->enable_by_level) >> + return 0; >> + >> + /* Enable ACD feature at GMU */ >> + ret = a6xx_hfi_send_msg(gmu, HFI_H2F_FEATURE_CTRL, &msg, >> sizeof(msg), NULL, 0); >> + if (ret) { >> + DRM_DEV_ERROR(gmu->dev, "Unable to enable ACD (%d)\n", ret); >> + return ret; >> + } >> + >> + /* Send ACD table to GMU */ >> + ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, &msg, sizeof(msg), >> NULL, 0); > > This looks wrong, in this exact code, you never use the acd_table... > perhaps it should be acd_table here Whoops! Weirdly gmu didn't explode when I tested. Thanks for your keen eye. -Akhil. > >> + if (ret) { >> + DRM_DEV_ERROR(gmu->dev, "Unable to ACD table (%d)\n", ret); >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> static int a6xx_hfi_send_test(struct a6xx_gmu *gmu) >> { >> struct a6xx_hfi_msg_test msg = { 0 }; >> @@ -756,6 +788,10 @@ int a6xx_hfi_start(struct a6xx_gmu *gmu, int >> boot_state) >> if (ret) >> return ret; >> + ret = a6xx_hfi_enable_acd(gmu); >> + if (ret) >> + return ret; >> + >> ret = a6xx_hfi_send_core_fw_start(gmu); >> if (ret) >> return ret; >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h b/drivers/gpu/drm/ >> msm/adreno/a6xx_hfi.h >> index 528110169398..51864c8ad0e6 100644 >> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.h >> @@ -151,12 +151,33 @@ struct a6xx_hfi_msg_test { >> u32 header; >> }; >> +#define HFI_H2F_MSG_ACD 7 >> +#define MAX_ACD_STRIDE 2 >> + >> +struct a6xx_hfi_acd_table { >> + u32 header; >> + u32 version; >> + u32 enable_by_level; >> + u32 stride; >> + u32 num_levels; >> + u32 data[16 * MAX_ACD_STRIDE]; >> +}; >> + >> #define HFI_H2F_MSG_START 10 >> struct a6xx_hfi_msg_start { >> u32 header; >> }; >> +#define HFI_H2F_FEATURE_CTRL 11 >> + >> +struct a6xx_hfi_msg_feature_ctrl { >> + u32 header; >> + u32 feature; >> + u32 enable; >> + u32 data; >> +}; >> + >> #define HFI_H2F_MSG_CORE_FW_START 14 >> struct a6xx_hfi_msg_core_fw_start { >> > > Thanks, > Neil
On 06/11/2024 02:44, Akhil P Oommen wrote: > On 11/4/2024 9:14 PM, neil.armstrong@linaro.org wrote: >> On 11/10/2024 22:29, Akhil P Oommen wrote: >>> ACD a.k.a Adaptive Clock Distribution is a feature which helps to reduce >>> the power consumption. In some chipsets, it is also a requirement to >>> support higher GPU frequencies. This patch adds support for GPU ACD by >>> sending necessary data to GMU and AOSS. The feature support for the >>> chipset is detected based on devicetree data. >>> >>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com> >>> --- >>> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 81 +++++++++++++++++++++++++ >>> +++------- >>> drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 1 + >>> drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 36 ++++++++++++++++ >>> drivers/gpu/drm/msm/adreno/a6xx_hfi.h | 21 +++++++++ >>> 4 files changed, 124 insertions(+), 15 deletions(-) >>> >> >> <snip> >> >>> + >>> +static int a6xx_hfi_enable_acd(struct a6xx_gmu *gmu) >>> +{ >>> + struct a6xx_hfi_acd_table *acd_table = &gmu->acd_table; >>> + struct a6xx_hfi_msg_feature_ctrl msg = { >>> + .feature = HFI_FEATURE_ACD, >>> + .enable = 1, >>> + .data = 0, >>> + }; >>> + int ret; >>> + >>> + if (!acd_table->enable_by_level) >>> + return 0; >>> + >>> + /* Enable ACD feature at GMU */ >>> + ret = a6xx_hfi_send_msg(gmu, HFI_H2F_FEATURE_CTRL, &msg, >>> sizeof(msg), NULL, 0); >>> + if (ret) { >>> + DRM_DEV_ERROR(gmu->dev, "Unable to enable ACD (%d)\n", ret); >>> + return ret; >>> + } >>> + >>> + /* Send ACD table to GMU */ >>> + ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, &msg, sizeof(msg), >>> NULL, 0); >> >> This looks wrong, in this exact code, you never use the acd_table... >> perhaps it should be acd_table here > > Whoops! Weirdly gmu didn't explode when I tested. > > Thanks for your keen eye. You're welcome ! I've been trying to enable this on SM8650, but HFI_H2F_MSG_ACD fails. My changes: ================><================================ diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c index 7c96d6f8aaa9..bd9d586f245e 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c @@ -682,7 +682,7 @@ static int a6xx_hfi_enable_acd(struct a6xx_gmu *gmu) } /* Send ACD table to GMU */ - ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, &acd_table, sizeof(*acd_table), NULL, 0); + ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, &acd_table, sizeof(struct a6xx_hfi_acd_table), NULL, 0); if (ret) { DRM_DEV_ERROR(gmu->dev, "Unable to send ACD table (%d)\n", ret); return ret; ================><================================ with the appropriate qcom,opp-acd-level in DT taken from downstream, I get: [ 6.946184] platform 3d6a000.gmu: [drm:a6xx_hfi_send_msg.constprop.0 [msm]] *ERROR* Message (null) id 4 timed out waiting for response [ 6.958697] platform 3d6a000.gmu: [drm:a6xx_hfi_start [msm]] *ERROR* Unable to send ACD table (-110) is there something missing ? Neil > > -Akhil. > >> >>> + if (ret) { >>> + DRM_DEV_ERROR(gmu->dev, "Unable to ACD table (%d)\n", ret); >>> + return ret; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> static int a6xx_hfi_send_test(struct a6xx_gmu *gmu) >>> { >>> struct a6xx_hfi_msg_test msg = { 0 }; >>> @@ -756,6 +788,10 @@ int a6xx_hfi_start(struct a6xx_gmu *gmu, int >>> boot_state) >>> if (ret) >>> return ret; >>> + ret = a6xx_hfi_enable_acd(gmu); >>> + if (ret) >>> + return ret; >>> + >>> ret = a6xx_hfi_send_core_fw_start(gmu); >>> if (ret) >>> return ret; >>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h b/drivers/gpu/drm/ >>> msm/adreno/a6xx_hfi.h >>> index 528110169398..51864c8ad0e6 100644 >>> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h >>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.h >>> @@ -151,12 +151,33 @@ struct a6xx_hfi_msg_test { >>> u32 header; >>> }; >>> +#define HFI_H2F_MSG_ACD 7 >>> +#define MAX_ACD_STRIDE 2 >>> + >>> +struct a6xx_hfi_acd_table { >>> + u32 header; >>> + u32 version; >>> + u32 enable_by_level; >>> + u32 stride; >>> + u32 num_levels; >>> + u32 data[16 * MAX_ACD_STRIDE]; >>> +}; >>> + >>> #define HFI_H2F_MSG_START 10 >>> struct a6xx_hfi_msg_start { >>> u32 header; >>> }; >>> +#define HFI_H2F_FEATURE_CTRL 11 >>> + >>> +struct a6xx_hfi_msg_feature_ctrl { >>> + u32 header; >>> + u32 feature; >>> + u32 enable; >>> + u32 data; >>> +}; >>> + >>> #define HFI_H2F_MSG_CORE_FW_START 14 >>> struct a6xx_hfi_msg_core_fw_start { >>> >> >> Thanks, >> Neil >
On 11/7/2024 2:25 PM, neil.armstrong@linaro.org wrote: > On 06/11/2024 02:44, Akhil P Oommen wrote: >> On 11/4/2024 9:14 PM, neil.armstrong@linaro.org wrote: >>> On 11/10/2024 22:29, Akhil P Oommen wrote: >>>> ACD a.k.a Adaptive Clock Distribution is a feature which helps to >>>> reduce >>>> the power consumption. In some chipsets, it is also a requirement to >>>> support higher GPU frequencies. This patch adds support for GPU ACD by >>>> sending necessary data to GMU and AOSS. The feature support for the >>>> chipset is detected based on devicetree data. >>>> >>>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com> >>>> --- >>>> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 81 +++++++++++++++++++++++++ >>>> +++------- >>>> drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 1 + >>>> drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 36 ++++++++++++++++ >>>> drivers/gpu/drm/msm/adreno/a6xx_hfi.h | 21 +++++++++ >>>> 4 files changed, 124 insertions(+), 15 deletions(-) >>>> >>> >>> <snip> >>> >>>> + >>>> +static int a6xx_hfi_enable_acd(struct a6xx_gmu *gmu) >>>> +{ >>>> + struct a6xx_hfi_acd_table *acd_table = &gmu->acd_table; >>>> + struct a6xx_hfi_msg_feature_ctrl msg = { >>>> + .feature = HFI_FEATURE_ACD, >>>> + .enable = 1, >>>> + .data = 0, >>>> + }; >>>> + int ret; >>>> + >>>> + if (!acd_table->enable_by_level) >>>> + return 0; >>>> + >>>> + /* Enable ACD feature at GMU */ >>>> + ret = a6xx_hfi_send_msg(gmu, HFI_H2F_FEATURE_CTRL, &msg, >>>> sizeof(msg), NULL, 0); >>>> + if (ret) { >>>> + DRM_DEV_ERROR(gmu->dev, "Unable to enable ACD (%d)\n", ret); >>>> + return ret; >>>> + } >>>> + >>>> + /* Send ACD table to GMU */ >>>> + ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, &msg, sizeof(msg), >>>> NULL, 0); >>> >>> This looks wrong, in this exact code, you never use the acd_table... >>> perhaps it should be acd_table here >> >> Whoops! Weirdly gmu didn't explode when I tested. >> >> Thanks for your keen eye. > > You're welcome ! > > I've been trying to enable this on SM8650, but HFI_H2F_MSG_ACD fails. > > My changes: > ================><================================ > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c b/drivers/gpu/drm/ > msm/adreno/a6xx_hfi.c > index 7c96d6f8aaa9..bd9d586f245e 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c > @@ -682,7 +682,7 @@ static int a6xx_hfi_enable_acd(struct a6xx_gmu *gmu) > } > > /* Send ACD table to GMU */ > - ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, &acd_table, > sizeof(*acd_table), NULL, 0); > + ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, &acd_table, &acd_table -> acd_table here? -Akhil > sizeof(struct a6xx_hfi_acd_table), NULL, 0); > if (ret) { > DRM_DEV_ERROR(gmu->dev, "Unable to send ACD table > (%d)\n", ret); > return ret; > ================><================================ > > with the appropriate qcom,opp-acd-level in DT taken from downstream, I get: > [ 6.946184] platform 3d6a000.gmu: [drm:a6xx_hfi_send_msg.constprop.0 > [msm]] *ERROR* Message (null) id 4 timed out waiting for response > [ 6.958697] platform 3d6a000.gmu: [drm:a6xx_hfi_start [msm]] *ERROR* > Unable to send ACD table (-110) > > is there something missing ? > > Neil > >> >> -Akhil. >> >>> >>>> + if (ret) { >>>> + DRM_DEV_ERROR(gmu->dev, "Unable to ACD table (%d)\n", ret); >>>> + return ret; >>>> + } >>>> + >>>> + return 0; >>>> +} >>>> + >>>> static int a6xx_hfi_send_test(struct a6xx_gmu *gmu) >>>> { >>>> struct a6xx_hfi_msg_test msg = { 0 }; >>>> @@ -756,6 +788,10 @@ int a6xx_hfi_start(struct a6xx_gmu *gmu, int >>>> boot_state) >>>> if (ret) >>>> return ret; >>>> + ret = a6xx_hfi_enable_acd(gmu); >>>> + if (ret) >>>> + return ret; >>>> + >>>> ret = a6xx_hfi_send_core_fw_start(gmu); >>>> if (ret) >>>> return ret; >>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h b/drivers/gpu/drm/ >>>> msm/adreno/a6xx_hfi.h >>>> index 528110169398..51864c8ad0e6 100644 >>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h >>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.h >>>> @@ -151,12 +151,33 @@ struct a6xx_hfi_msg_test { >>>> u32 header; >>>> }; >>>> +#define HFI_H2F_MSG_ACD 7 >>>> +#define MAX_ACD_STRIDE 2 >>>> + >>>> +struct a6xx_hfi_acd_table { >>>> + u32 header; >>>> + u32 version; >>>> + u32 enable_by_level; >>>> + u32 stride; >>>> + u32 num_levels; >>>> + u32 data[16 * MAX_ACD_STRIDE]; >>>> +}; >>>> + >>>> #define HFI_H2F_MSG_START 10 >>>> struct a6xx_hfi_msg_start { >>>> u32 header; >>>> }; >>>> +#define HFI_H2F_FEATURE_CTRL 11 >>>> + >>>> +struct a6xx_hfi_msg_feature_ctrl { >>>> + u32 header; >>>> + u32 feature; >>>> + u32 enable; >>>> + u32 data; >>>> +}; >>>> + >>>> #define HFI_H2F_MSG_CORE_FW_START 14 >>>> struct a6xx_hfi_msg_core_fw_start { >>>> >>> >>> Thanks, >>> Neil >> >
On 07/11/2024 13:46, Akhil P Oommen wrote: > On 11/7/2024 2:25 PM, neil.armstrong@linaro.org wrote: >> On 06/11/2024 02:44, Akhil P Oommen wrote: >>> On 11/4/2024 9:14 PM, neil.armstrong@linaro.org wrote: >>>> On 11/10/2024 22:29, Akhil P Oommen wrote: >>>>> ACD a.k.a Adaptive Clock Distribution is a feature which helps to >>>>> reduce >>>>> the power consumption. In some chipsets, it is also a requirement to >>>>> support higher GPU frequencies. This patch adds support for GPU ACD by >>>>> sending necessary data to GMU and AOSS. The feature support for the >>>>> chipset is detected based on devicetree data. >>>>> >>>>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com> >>>>> --- >>>>> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 81 +++++++++++++++++++++++++ >>>>> +++------- >>>>> drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 1 + >>>>> drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 36 ++++++++++++++++ >>>>> drivers/gpu/drm/msm/adreno/a6xx_hfi.h | 21 +++++++++ >>>>> 4 files changed, 124 insertions(+), 15 deletions(-) >>>>> >>>> >>>> <snip> >>>> >>>>> + >>>>> +static int a6xx_hfi_enable_acd(struct a6xx_gmu *gmu) >>>>> +{ >>>>> + struct a6xx_hfi_acd_table *acd_table = &gmu->acd_table; >>>>> + struct a6xx_hfi_msg_feature_ctrl msg = { >>>>> + .feature = HFI_FEATURE_ACD, >>>>> + .enable = 1, >>>>> + .data = 0, >>>>> + }; >>>>> + int ret; >>>>> + >>>>> + if (!acd_table->enable_by_level) >>>>> + return 0; >>>>> + >>>>> + /* Enable ACD feature at GMU */ >>>>> + ret = a6xx_hfi_send_msg(gmu, HFI_H2F_FEATURE_CTRL, &msg, >>>>> sizeof(msg), NULL, 0); >>>>> + if (ret) { >>>>> + DRM_DEV_ERROR(gmu->dev, "Unable to enable ACD (%d)\n", ret); >>>>> + return ret; >>>>> + } >>>>> + >>>>> + /* Send ACD table to GMU */ >>>>> + ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, &msg, sizeof(msg), >>>>> NULL, 0); >>>> >>>> This looks wrong, in this exact code, you never use the acd_table... >>>> perhaps it should be acd_table here >>> >>> Whoops! Weirdly gmu didn't explode when I tested. >>> >>> Thanks for your keen eye. >> >> You're welcome ! >> >> I've been trying to enable this on SM8650, but HFI_H2F_MSG_ACD fails. >> >> My changes: >> ================><================================ >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c b/drivers/gpu/drm/ >> msm/adreno/a6xx_hfi.c >> index 7c96d6f8aaa9..bd9d586f245e 100644 >> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c >> @@ -682,7 +682,7 @@ static int a6xx_hfi_enable_acd(struct a6xx_gmu *gmu) >> } >> >> /* Send ACD table to GMU */ >> - ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, &acd_table, >> sizeof(*acd_table), NULL, 0); >> + ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, &acd_table, > > &acd_table -> acd_table here? Damn, good catch ! Ok so it didn't explode anymore, but still fails: ===== [ 7.083258] platform 3d6a000.gmu: [drm:a6xx_hfi_send_msg.constprop.0 [msm]] *ERROR* Message HFI_H2F_MSG_GX_BW_PERF_VOTE id 7 timed out waiting for response [ 7.149709] platform 3d6a000.gmu: [drm:a6xx_hfi_send_msg.constprop.0 [msm]] *ERROR* Unexpected message id 7 on the response queue [ 7.149744] platform 3d6a000.gmu: [drm:a6xx_hfi_send_msg.constprop.0 [msm]] *ERROR* The HFI response queue is unexpectedly empty [ 7.165163] platform 3d6a000.gmu: [drm:a6xx_hfi_send_msg.constprop.0 [msm]] *ERROR* Unexpected message id 8 on the response queue [ 7.165188] platform 3d6a000.gmu: [drm:a6xx_hfi_send_msg.constprop.0 [msm]] *ERROR* The HFI response queue is unexpectedly empty ==== Seems with ACD enabled, first vote can take up to 100ms, and downstream has 1s timeout, with a larger timeout I got it to work ! Thanks, Neil > > -Akhil > >> sizeof(struct a6xx_hfi_acd_table), NULL, 0); >> if (ret) { >> DRM_DEV_ERROR(gmu->dev, "Unable to send ACD table >> (%d)\n", ret); >> return ret; >> ================><================================ >> >> with the appropriate qcom,opp-acd-level in DT taken from downstream, I get: >> [ 6.946184] platform 3d6a000.gmu: [drm:a6xx_hfi_send_msg.constprop.0 >> [msm]] *ERROR* Message (null) id 4 timed out waiting for response >> [ 6.958697] platform 3d6a000.gmu: [drm:a6xx_hfi_start [msm]] *ERROR* >> Unable to send ACD table (-110) >> >> is there something missing ? >> >> Neil >> >>> >>> -Akhil. >>> >>>> >>>>> + if (ret) { >>>>> + DRM_DEV_ERROR(gmu->dev, "Unable to ACD table (%d)\n", ret); >>>>> + return ret; >>>>> + } >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> static int a6xx_hfi_send_test(struct a6xx_gmu *gmu) >>>>> { >>>>> struct a6xx_hfi_msg_test msg = { 0 }; >>>>> @@ -756,6 +788,10 @@ int a6xx_hfi_start(struct a6xx_gmu *gmu, int >>>>> boot_state) >>>>> if (ret) >>>>> return ret; >>>>> + ret = a6xx_hfi_enable_acd(gmu); >>>>> + if (ret) >>>>> + return ret; >>>>> + >>>>> ret = a6xx_hfi_send_core_fw_start(gmu); >>>>> if (ret) >>>>> return ret; >>>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h b/drivers/gpu/drm/ >>>>> msm/adreno/a6xx_hfi.h >>>>> index 528110169398..51864c8ad0e6 100644 >>>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h >>>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.h >>>>> @@ -151,12 +151,33 @@ struct a6xx_hfi_msg_test { >>>>> u32 header; >>>>> }; >>>>> +#define HFI_H2F_MSG_ACD 7 >>>>> +#define MAX_ACD_STRIDE 2 >>>>> + >>>>> +struct a6xx_hfi_acd_table { >>>>> + u32 header; >>>>> + u32 version; >>>>> + u32 enable_by_level; >>>>> + u32 stride; >>>>> + u32 num_levels; >>>>> + u32 data[16 * MAX_ACD_STRIDE]; >>>>> +}; >>>>> + >>>>> #define HFI_H2F_MSG_START 10 >>>>> struct a6xx_hfi_msg_start { >>>>> u32 header; >>>>> }; >>>>> +#define HFI_H2F_FEATURE_CTRL 11 >>>>> + >>>>> +struct a6xx_hfi_msg_feature_ctrl { >>>>> + u32 header; >>>>> + u32 feature; >>>>> + u32 enable; >>>>> + u32 data; >>>>> +}; >>>>> + >>>>> #define HFI_H2F_MSG_CORE_FW_START 14 >>>>> struct a6xx_hfi_msg_core_fw_start { >>>>> >>>> >>>> Thanks, >>>> Neil >>> >> >
On 11/7/2024 8:01 PM, neil.armstrong@linaro.org wrote: > On 07/11/2024 13:46, Akhil P Oommen wrote: >> On 11/7/2024 2:25 PM, neil.armstrong@linaro.org wrote: >>> On 06/11/2024 02:44, Akhil P Oommen wrote: >>>> On 11/4/2024 9:14 PM, neil.armstrong@linaro.org wrote: >>>>> On 11/10/2024 22:29, Akhil P Oommen wrote: >>>>>> ACD a.k.a Adaptive Clock Distribution is a feature which helps to >>>>>> reduce >>>>>> the power consumption. In some chipsets, it is also a requirement to >>>>>> support higher GPU frequencies. This patch adds support for GPU >>>>>> ACD by >>>>>> sending necessary data to GMU and AOSS. The feature support for the >>>>>> chipset is detected based on devicetree data. >>>>>> >>>>>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com> >>>>>> --- >>>>>> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 81 +++++++++++++++++++ >>>>>> ++++++ >>>>>> +++------- >>>>>> drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 1 + >>>>>> drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 36 ++++++++++++++++ >>>>>> drivers/gpu/drm/msm/adreno/a6xx_hfi.h | 21 +++++++++ >>>>>> 4 files changed, 124 insertions(+), 15 deletions(-) >>>>>> >>>>> >>>>> <snip> >>>>> >>>>>> + >>>>>> +static int a6xx_hfi_enable_acd(struct a6xx_gmu *gmu) >>>>>> +{ >>>>>> + struct a6xx_hfi_acd_table *acd_table = &gmu->acd_table; >>>>>> + struct a6xx_hfi_msg_feature_ctrl msg = { >>>>>> + .feature = HFI_FEATURE_ACD, >>>>>> + .enable = 1, >>>>>> + .data = 0, >>>>>> + }; >>>>>> + int ret; >>>>>> + >>>>>> + if (!acd_table->enable_by_level) >>>>>> + return 0; >>>>>> + >>>>>> + /* Enable ACD feature at GMU */ >>>>>> + ret = a6xx_hfi_send_msg(gmu, HFI_H2F_FEATURE_CTRL, &msg, >>>>>> sizeof(msg), NULL, 0); >>>>>> + if (ret) { >>>>>> + DRM_DEV_ERROR(gmu->dev, "Unable to enable ACD (%d)\n", ret); >>>>>> + return ret; >>>>>> + } >>>>>> + >>>>>> + /* Send ACD table to GMU */ >>>>>> + ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, &msg, sizeof(msg), >>>>>> NULL, 0); >>>>> >>>>> This looks wrong, in this exact code, you never use the acd_table... >>>>> perhaps it should be acd_table here >>>> >>>> Whoops! Weirdly gmu didn't explode when I tested. >>>> >>>> Thanks for your keen eye. >>> >>> You're welcome ! >>> >>> I've been trying to enable this on SM8650, but HFI_H2F_MSG_ACD fails. >>> >>> My changes: >>> ================><================================ >>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c b/drivers/gpu/drm/ >>> msm/adreno/a6xx_hfi.c >>> index 7c96d6f8aaa9..bd9d586f245e 100644 >>> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c >>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c >>> @@ -682,7 +682,7 @@ static int a6xx_hfi_enable_acd(struct a6xx_gmu *gmu) >>> } >>> >>> /* Send ACD table to GMU */ >>> - ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, &acd_table, >>> sizeof(*acd_table), NULL, 0); >>> + ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, &acd_table, >> >> &acd_table -> acd_table here? > > Damn, good catch ! > > Ok so it didn't explode anymore, but still fails: > ===== > [ 7.083258] platform 3d6a000.gmu: [drm:a6xx_hfi_send_msg.constprop.0 > [msm]] *ERROR* Message HFI_H2F_MSG_GX_BW_PERF_VOTE id 7 timed out > waiting for response > [ 7.149709] platform 3d6a000.gmu: [drm:a6xx_hfi_send_msg.constprop.0 > [msm]] *ERROR* Unexpected message id 7 on the response queue > [ 7.149744] platform 3d6a000.gmu: [drm:a6xx_hfi_send_msg.constprop.0 > [msm]] *ERROR* The HFI response queue is unexpectedly empty > [ 7.165163] platform 3d6a000.gmu: [drm:a6xx_hfi_send_msg.constprop.0 > [msm]] *ERROR* Unexpected message id 8 on the response queue > [ 7.165188] platform 3d6a000.gmu: [drm:a6xx_hfi_send_msg.constprop.0 > [msm]] *ERROR* The HFI response queue is unexpectedly empty > ==== > > Seems with ACD enabled, first vote can take up to 100ms, and downstream > has 1s timeout, with a larger timeout I got it to work ! Yes, there is an additional overhead during first perf vote. Thanks for the heads up. I am yet to test with fixes. -Akhil. > > Thanks, > Neil >> >> -Akhil >> >>> sizeof(struct a6xx_hfi_acd_table), NULL, 0); >>> if (ret) { >>> DRM_DEV_ERROR(gmu->dev, "Unable to send ACD table >>> (%d)\n", ret); >>> return ret; >>> ================><================================ >>> >>> with the appropriate qcom,opp-acd-level in DT taken from downstream, >>> I get: >>> [ 6.946184] platform 3d6a000.gmu: [drm:a6xx_hfi_send_msg.constprop.0 >>> [msm]] *ERROR* Message (null) id 4 timed out waiting for response >>> [ 6.958697] platform 3d6a000.gmu: [drm:a6xx_hfi_start [msm]] *ERROR* >>> Unable to send ACD table (-110) >>> >>> is there something missing ? >>> >>> Neil >>> >>>> >>>> -Akhil. >>>> >>>>> >>>>>> + if (ret) { >>>>>> + DRM_DEV_ERROR(gmu->dev, "Unable to ACD table (%d)\n", ret); >>>>>> + return ret; >>>>>> + } >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> static int a6xx_hfi_send_test(struct a6xx_gmu *gmu) >>>>>> { >>>>>> struct a6xx_hfi_msg_test msg = { 0 }; >>>>>> @@ -756,6 +788,10 @@ int a6xx_hfi_start(struct a6xx_gmu *gmu, int >>>>>> boot_state) >>>>>> if (ret) >>>>>> return ret; >>>>>> + ret = a6xx_hfi_enable_acd(gmu); >>>>>> + if (ret) >>>>>> + return ret; >>>>>> + >>>>>> ret = a6xx_hfi_send_core_fw_start(gmu); >>>>>> if (ret) >>>>>> return ret; >>>>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h b/drivers/gpu/drm/ >>>>>> msm/adreno/a6xx_hfi.h >>>>>> index 528110169398..51864c8ad0e6 100644 >>>>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h >>>>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.h >>>>>> @@ -151,12 +151,33 @@ struct a6xx_hfi_msg_test { >>>>>> u32 header; >>>>>> }; >>>>>> +#define HFI_H2F_MSG_ACD 7 >>>>>> +#define MAX_ACD_STRIDE 2 >>>>>> + >>>>>> +struct a6xx_hfi_acd_table { >>>>>> + u32 header; >>>>>> + u32 version; >>>>>> + u32 enable_by_level; >>>>>> + u32 stride; >>>>>> + u32 num_levels; >>>>>> + u32 data[16 * MAX_ACD_STRIDE]; >>>>>> +}; >>>>>> + >>>>>> #define HFI_H2F_MSG_START 10 >>>>>> struct a6xx_hfi_msg_start { >>>>>> u32 header; >>>>>> }; >>>>>> +#define HFI_H2F_FEATURE_CTRL 11 >>>>>> + >>>>>> +struct a6xx_hfi_msg_feature_ctrl { >>>>>> + u32 header; >>>>>> + u32 feature; >>>>>> + u32 enable; >>>>>> + u32 data; >>>>>> +}; >>>>>> + >>>>>> #define HFI_H2F_MSG_CORE_FW_START 14 >>>>>> struct a6xx_hfi_msg_core_fw_start { >>>>>> >>>>> >>>>> Thanks, >>>>> Neil >>>> >>> >> >
This series adds support for ACD feature for Adreno GPU which helps to lower the power consumption on GX rail and also sometimes is a requirement to enable higher GPU frequencies. At high level, following are the sequences required for ACD feature: 1. Identify the ACD level data for each regulator corner 2. Send a message to AOSS to switch voltage plan 3. Send a table with ACD level information to GMU during every gpu wake up For (1), it is better to keep ACD level data in devicetree because this value depends on the process node, voltage margins etc which are chipset specific. For instance, same GPU HW IP on a different chipset would have a different set of values. So, a new schema which extends opp-v2 is created to add a new property called "qcom,opp-acd-level". I would like to have more feedback on this, hence the RFC tag for this series. ACD support is dynamically detected based on the presence of "qcom,opp-acd-level" property in GPU's opp table. Also, qmp node should be present under GMU node in devicetree for communication with AOSS. The devicetree patch in this series adds the acd-level data for X1-85 GPU present in Snapdragon X1 Elite chipset. This series is rebased on top of drm-msm/msm-next. --- Akhil P Oommen (3): drm/msm/adreno: Add support for ACD dt-bindings: opp: Add v2-qcom-adreno vendor bindings arm64: dts: qcom: x1e80100: Add ACD levels for GPU .../bindings/opp/opp-v2-qcom-adreno.yaml | 84 ++++++++++++++++++++++ arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 ++- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 81 +++++++++++++++++---- drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 1 + drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 36 ++++++++++ drivers/gpu/drm/msm/adreno/a6xx_hfi.h | 21 ++++++ 6 files changed, 218 insertions(+), 16 deletions(-) --- base-commit: a20a91fb1bfac5d05ec5bcf9afe0c9363f6c8c93 change-id: 20240724-gpu-acd-6c1dc5dcf516 Best regards,