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