drm/xe/guc_pc: Add _locked variant for min/max freq
authorLucas De Marchi <lucas.demarchi@intel.com>
Wed, 18 Jun 2025 18:49:58 +0000 (11:49 -0700)
committerLucas De Marchi <lucas.demarchi@intel.com>
Tue, 1 Jul 2025 20:53:16 +0000 (13:53 -0700)
There are places in which the getters/setters are called one after the
other causing a multiple lock()/unlock(). These are not currently a
problem since they are all happening from the same thread, but there's a
race possibility as calls are added outside of the early init when the
max/min and stashed values need to be correlated.

Add the _locked() variants to prepare for that.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://lore.kernel.org/r/20250618-wa-22019338487-v5-1-b888388477f2@intel.com
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
(cherry picked from commit 1beae9aa2b88d3a02eb666e7b777eb2d7bc645f4)
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
drivers/gpu/drm/xe/xe_guc_pc.c

index 3beaaa7b25c1b06661a2d7d5fb4516184e6518e2..2d9663acf2f2ca9a5702d27d3688989314f1975d 100644 (file)
@@ -5,6 +5,7 @@
 
 #include "xe_guc_pc.h"
 
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/ktime.h>
 
@@ -553,6 +554,25 @@ u32 xe_guc_pc_get_rpn_freq(struct xe_guc_pc *pc)
        return pc->rpn_freq;
 }
 
+static int xe_guc_pc_get_min_freq_locked(struct xe_guc_pc *pc, u32 *freq)
+{
+       int ret;
+
+       lockdep_assert_held(&pc->freq_lock);
+
+       /* Might be in the middle of a gt reset */
+       if (!pc->freq_ready)
+               return -EAGAIN;
+
+       ret = pc_action_query_task_state(pc);
+       if (ret)
+               return ret;
+
+       *freq = pc_get_min_freq(pc);
+
+       return 0;
+}
+
 /**
  * xe_guc_pc_get_min_freq - Get the min operational frequency
  * @pc: The GuC PC
@@ -562,27 +582,29 @@ u32 xe_guc_pc_get_rpn_freq(struct xe_guc_pc *pc)
  *         -EAGAIN if GuC PC not ready (likely in middle of a reset).
  */
 int xe_guc_pc_get_min_freq(struct xe_guc_pc *pc, u32 *freq)
+{
+       guard(mutex)(&pc->freq_lock);
+
+       return xe_guc_pc_get_min_freq_locked(pc, freq);
+}
+
+static int xe_guc_pc_set_min_freq_locked(struct xe_guc_pc *pc, u32 freq)
 {
        int ret;
 
-       xe_device_assert_mem_access(pc_to_xe(pc));
+       lockdep_assert_held(&pc->freq_lock);
 
-       mutex_lock(&pc->freq_lock);
-       if (!pc->freq_ready) {
-               /* Might be in the middle of a gt reset */
-               ret = -EAGAIN;
-               goto out;
-       }
+       /* Might be in the middle of a gt reset */
+       if (!pc->freq_ready)
+               return -EAGAIN;
 
-       ret = pc_action_query_task_state(pc);
+       ret = pc_set_min_freq(pc, freq);
        if (ret)
-               goto out;
+               return ret;
 
-       *freq = pc_get_min_freq(pc);
+       pc->user_requested_min = freq;
 
-out:
-       mutex_unlock(&pc->freq_lock);
-       return ret;
+       return 0;
 }
 
 /**
@@ -595,25 +617,29 @@ out:
  *         -EINVAL if value out of bounds.
  */
 int xe_guc_pc_set_min_freq(struct xe_guc_pc *pc, u32 freq)
+{
+       guard(mutex)(&pc->freq_lock);
+
+       return xe_guc_pc_set_min_freq_locked(pc, freq);
+}
+
+static int xe_guc_pc_get_max_freq_locked(struct xe_guc_pc *pc, u32 *freq)
 {
        int ret;
 
-       mutex_lock(&pc->freq_lock);
-       if (!pc->freq_ready) {
-               /* Might be in the middle of a gt reset */
-               ret = -EAGAIN;
-               goto out;
-       }
+       lockdep_assert_held(&pc->freq_lock);
 
-       ret = pc_set_min_freq(pc, freq);
+       /* Might be in the middle of a gt reset */
+       if (!pc->freq_ready)
+               return -EAGAIN;
+
+       ret = pc_action_query_task_state(pc);
        if (ret)
-               goto out;
+               return ret;
 
-       pc->user_requested_min = freq;
+       *freq = pc_get_max_freq(pc);
 
-out:
-       mutex_unlock(&pc->freq_lock);
-       return ret;
+       return 0;
 }
 
 /**
@@ -625,25 +651,29 @@ out:
  *         -EAGAIN if GuC PC not ready (likely in middle of a reset).
  */
 int xe_guc_pc_get_max_freq(struct xe_guc_pc *pc, u32 *freq)
+{
+       guard(mutex)(&pc->freq_lock);
+
+       return xe_guc_pc_get_max_freq_locked(pc, freq);
+}
+
+static int xe_guc_pc_set_max_freq_locked(struct xe_guc_pc *pc, u32 freq)
 {
        int ret;
 
-       mutex_lock(&pc->freq_lock);
-       if (!pc->freq_ready) {
-               /* Might be in the middle of a gt reset */
-               ret = -EAGAIN;
-               goto out;
-       }
+       lockdep_assert_held(&pc->freq_lock);
 
-       ret = pc_action_query_task_state(pc);
+       /* Might be in the middle of a gt reset */
+       if (!pc->freq_ready)
+               return -EAGAIN;
+
+       ret = pc_set_max_freq(pc, freq);
        if (ret)
-               goto out;
+               return ret;
 
-       *freq = pc_get_max_freq(pc);
+       pc->user_requested_max = freq;
 
-out:
-       mutex_unlock(&pc->freq_lock);
-       return ret;
+       return 0;
 }
 
 /**
@@ -657,24 +687,9 @@ out:
  */
 int xe_guc_pc_set_max_freq(struct xe_guc_pc *pc, u32 freq)
 {
-       int ret;
-
-       mutex_lock(&pc->freq_lock);
-       if (!pc->freq_ready) {
-               /* Might be in the middle of a gt reset */
-               ret = -EAGAIN;
-               goto out;
-       }
-
-       ret = pc_set_max_freq(pc, freq);
-       if (ret)
-               goto out;
+       guard(mutex)(&pc->freq_lock);
 
-       pc->user_requested_max = freq;
-
-out:
-       mutex_unlock(&pc->freq_lock);
-       return ret;
+       return xe_guc_pc_set_max_freq_locked(pc, freq);
 }
 
 /**