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 /* /** 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. > + 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 */