Re: [PATCH 1/5] drm/msm/adreno: Use OPP for every GPU generation

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

 



On 23/02/2023 00:40, Konrad Dybcio wrote:


On 22.02.2023 23:38, Dmitry Baryshkov wrote:
On 22/02/2023 23:47, Konrad Dybcio wrote:
Some older GPUs (namely a2xx with no opp tables at all and a320 with
downstream-remnants gpu pwrlevels) used not to have OPP tables. They
both however had just one frequency defined, making it extremely easy
to construct such an OPP table from within the driver if need be.

Do so and switch all clk_set_rate calls on core_clk to their OPP
counterparts.

Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
---
   drivers/gpu/drm/msm/adreno/adreno_gpu.c | 94 +++++++++++++++------------------
   drivers/gpu/drm/msm/msm_gpu.c           |  4 +-
   drivers/gpu/drm/msm/msm_gpu_devfreq.c   |  2 +-
   3 files changed, 45 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index ce6b76c45b6f..9b940c0f063f 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -922,73 +922,50 @@ void adreno_wait_ring(struct msm_ringbuffer *ring, uint32_t ndwords)
               ring->id);
   }
   -/* Get legacy powerlevels from qcom,gpu-pwrlevels and populate the opp table */
-static int adreno_get_legacy_pwrlevels(struct device *dev)
-{
-    struct device_node *child, *node;
-    int ret;
-
-    node = of_get_compatible_child(dev->of_node, "qcom,gpu-pwrlevels");
-    if (!node) {
-        DRM_DEV_DEBUG(dev, "Could not find the GPU powerlevels\n");
-        return -ENXIO;
-    }
-
-    for_each_child_of_node(node, child) {
-        unsigned int val;
-
-        ret = of_property_read_u32(child, "qcom,gpu-freq", &val);
-        if (ret)
-            continue;
-
-        /*
-         * Skip the intentionally bogus clock value found at the bottom
-         * of most legacy frequency tables
-         */
-        if (val != 27000000)
-            dev_pm_opp_add(dev, val, 0);
-    }
-
-    of_node_put(node);
-
-    return 0;
-}
-
-static void adreno_get_pwrlevels(struct device *dev,
+static int adreno_get_pwrlevels(struct device *dev,
           struct msm_gpu *gpu)
   {
+    struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
       unsigned long freq = ULONG_MAX;
       struct dev_pm_opp *opp;
       int ret;
         gpu->fast_rate = 0;
   -    /* You down with OPP? */
-    if (!of_find_property(dev->of_node, "operating-points-v2", NULL))
-        ret = adreno_get_legacy_pwrlevels(dev);
-    else {
-        ret = devm_pm_opp_of_add_table(dev);
-        if (ret)
-            DRM_DEV_ERROR(dev, "Unable to set the OPP table\n");
-    }
-
-    if (!ret) {
+    /* devm_pm_opp_of_add_table may error out but will still create an OPP table */
+    ret = devm_pm_opp_of_add_table(dev);
+    if (ret == -ENODEV) {
+        /* Special cases for ancient hw with ancient DT bindings */
+        if (adreno_is_a2xx(adreno_gpu)) {
+            dev_warn(dev, "Unable to find the OPP table. Falling back to 200 MHz.\n");
+            dev_pm_opp_add(dev, 200000000, 0);
+            gpu->fast_rate = 200000000;

We can skip setting the fast_rate, dev_pm_opp_find_freq_floor below will get it from our freshly generated opp table.
It's not reached in this code path.

I see. I got lost in all the ifs. What do you think about turning it into the main code path, since after this code block we always have a valid OPP table?



+        } else if (adreno_is_a320(adreno_gpu)) {
+            dev_warn(dev, "Unable to find the OPP table. Falling back to 450 MHz.\n");
+            dev_pm_opp_add(dev, 450000000, 0);
+            gpu->fast_rate = 450000000;
+        } else {
+            DRM_DEV_ERROR(dev, "Unable to find the OPP table\n");
+            return -ENODEV;
+        }
+    } else if (ret) {
+        DRM_DEV_ERROR(dev, "Unable to set the OPP table\n");
+        return ret;
+    } else {
           /* Find the fastest defined rate */
           opp = dev_pm_opp_find_freq_floor(dev, &freq);
-        if (!IS_ERR(opp)) {
+
+        if (IS_ERR(opp))
+            return PTR_ERR(opp);
+        else {
               gpu->fast_rate = freq;
               dev_pm_opp_put(opp);
           }
       }
   -    if (!gpu->fast_rate) {
-        dev_warn(dev,
-            "Could not find a clock rate. Using a reasonable default\n");
-        /* Pick a suitably safe clock speed for any target */
-        gpu->fast_rate = 200000000;
-    }
-
       DBG("fast_rate=%u, slow_rate=27000000", gpu->fast_rate);
+
+    return 0;
   }
     int adreno_gpu_ocmem_init(struct device *dev, struct adreno_gpu *adreno_gpu,


--
With best wishes
Dmitry




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux