Re: [PATCH RFC 1/3] drm/msm/adreno: Add support for ACD

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/11/2024 13:46, Akhil P Oommen wrote:
On 11/7/2024 2:25 PM, neil.armstrong@xxxxxxxxxx wrote:
On 06/11/2024 02:44, Akhil P Oommen wrote:
On 11/4/2024 9:14 PM, neil.armstrong@xxxxxxxxxx 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@xxxxxxxxxxx>
---
    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







[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux