From: "Dixit, Ashutosh" <ashutosh.dixit@intel.com> To: "Belgaumkar, Vinay" <vinay.belgaumkar@intel.com> Cc: "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>, "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org> Subject: Re: [PATCH 1/3] drm/i915/guc/slpc: Define and initialize boost frequency Date: Mon, 01 Nov 2021 13:26:20 -0700 [thread overview] Message-ID: <87r1bz39n7.wl-ashutosh.dixit@intel.com> (raw) In-Reply-To: <20211101043937.35747-2-vinay.belgaumkar@intel.com> 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 */
WARNING: multiple messages have this Message-ID (diff)
From: "Dixit, Ashutosh" <ashutosh.dixit@intel.com> To: "Belgaumkar, Vinay" <vinay.belgaumkar@intel.com> Cc: "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>, "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org> Subject: Re: [Intel-gfx] [PATCH 1/3] drm/i915/guc/slpc: Define and initialize boost frequency Date: Mon, 01 Nov 2021 13:26:20 -0700 [thread overview] Message-ID: <87r1bz39n7.wl-ashutosh.dixit@intel.com> (raw) In-Reply-To: <20211101043937.35747-2-vinay.belgaumkar@intel.com> 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 */
next prev parent reply other threads:[~2021-11-01 20:35 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-11-01 4:39 [PATCH v2 0/3] drm/i915/guc/slpc: Implement waitboost for SLPC Vinay Belgaumkar 2021-11-01 4:39 ` [Intel-gfx] " Vinay Belgaumkar 2021-11-01 4:39 ` [PATCH 1/3] drm/i915/guc/slpc: Define and initialize boost frequency Vinay Belgaumkar 2021-11-01 4:39 ` [Intel-gfx] " Vinay Belgaumkar 2021-11-01 20:26 ` Dixit, Ashutosh [this message] 2021-11-01 20:26 ` Dixit, Ashutosh 2021-11-02 0:20 ` Belgaumkar, Vinay 2021-11-02 0:20 ` [Intel-gfx] " Belgaumkar, Vinay 2021-11-01 4:39 ` [PATCH 2/3] drm/i915/guc/slpc: Add waitboost functionality for SLPC Vinay Belgaumkar 2021-11-01 4:39 ` [Intel-gfx] " Vinay Belgaumkar 2021-11-01 20:28 ` Dixit, Ashutosh 2021-11-01 20:28 ` [Intel-gfx] " Dixit, Ashutosh 2021-11-02 0:19 ` Belgaumkar, Vinay 2021-11-02 0:19 ` [Intel-gfx] " Belgaumkar, Vinay 2021-11-01 4:39 ` [PATCH 3/3] drm/i915/guc/slpc: Update boost sysfs hooks " Vinay Belgaumkar 2021-11-01 4:39 ` [Intel-gfx] " Vinay Belgaumkar 2021-11-01 20:28 ` Dixit, Ashutosh 2021-11-01 20:28 ` [Intel-gfx] " Dixit, Ashutosh 2021-11-04 0:39 ` Dixit, Ashutosh 2021-11-04 0:39 ` [Intel-gfx] " Dixit, Ashutosh 2021-11-01 5:05 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/guc/slpc: Implement waitboost for SLPC (rev2) Patchwork 2021-11-01 5:37 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork 2021-11-01 6:53 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork 2021-11-01 20:24 ` [PATCH v2 0/3] drm/i915/guc/slpc: Implement waitboost for SLPC Dixit, Ashutosh 2021-11-01 20:24 ` [Intel-gfx] " Dixit, Ashutosh 2021-11-02 0:18 ` Belgaumkar, Vinay 2021-11-02 0:18 ` [Intel-gfx] " Belgaumkar, Vinay -- strict thread matches above, loose matches on Subject: below -- 2021-11-02 1:26 [PATCH v3 " Vinay Belgaumkar 2021-11-02 1:26 ` [PATCH 1/3] drm/i915/guc/slpc: Define and initialize boost frequency Vinay Belgaumkar 2021-11-04 0:23 ` Dixit, Ashutosh 2021-10-20 19:52 [Intel-gfx] [PATCH 0/3] drm/i915/guc/slpc: Implement waitboost for SLPC Vinay Belgaumkar 2021-10-20 19:52 ` [PATCH 1/3] drm/i915/guc/slpc: Define and initialize boost frequency Vinay Belgaumkar
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=87r1bz39n7.wl-ashutosh.dixit@intel.com \ --to=ashutosh.dixit@intel.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=intel-gfx@lists.freedesktop.org \ --cc=vinay.belgaumkar@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.