On 11/1/2021 1:26 PM, Dixit, Ashutosh wrote:
On Sun, 31 Oct 2021 21:39:35 -0700, Belgaumkar, Vinay wrote:

Define helpers and struct members required to record boost info.
Boost frequency is initialized to RP0 at SLPC init. Also define num_waiters
which can track the pending boost requests.

Boost will be done by scheduling a worker thread. This will allow
us to make H2G calls inside an interrupt context. Initialize the

"to not make H2G calls from interrupt context" is probably better.

+static int slpc_force_min_freq(struct intel_guc_slpc *slpc, u32 freq)
+{
+       struct drm_i915_private *i915 = slpc_to_i915(slpc);
+       intel_wakeref_t wakeref;
+       int ret = 0;
+
+       lockdep_assert_held(&slpc->lock);
+
+       /**

nit: this I believe should just be

        /*

ok.


/** I believe shows up in kerneldoc so shouldn't be used unless we want
something in kerneldoc.

+        * This function is a little different as compared to
+        * intel_guc_slpc_set_min_freq(). Softlimit will not be updated
+        * here since this is used to temporarily change min freq,
+        * for example, during a waitboost. Caller is responsible for
+        * checking bounds.
+        */
+
+       with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
+               ret = slpc_set_param(slpc,
+                                    SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
+                                    freq);
+               if (ret)
+                       drm_err(&i915->drm, "Unable to force min freq to %u: 
%d",

Probably drm_err_ratelimited since it's called at run time not only at
init? Not sure if drm_err_once suffizes, probably not.

Keeping it drm_err as discussed offline.


+                               freq, ret);
+       }
+
+       return ret;
+}
+
+static void slpc_boost_work(struct work_struct *work)
+{
+       struct intel_guc_slpc *slpc = container_of(work, typeof(*slpc), 
boost_work);
+
+       /* Raise min freq to boost. It's possible that
+        * this is greater than current max. But it will
+        * certainly be limited by RP0. An error setting
+        * the min param is not fatal.
+        */

nit: do we follow the following format for multi-line comments,
Documentation/process/coding-style.rst mentions this:

/*
  * Line 1
  * Line 2
  */

Ok.

Thanks,
Vinay.

Reply via email to