From: "Dixit, Ashutosh" <ashutosh.dixit@intel.com>
To: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [Intel-gfx] [PATCH] i915/guc/slpc: Provide sysfs for efficient freq
Date: Fri, 31 Mar 2023 20:11:29 -0700 [thread overview]
Message-ID: <877cuwguu6.wl-ashutosh.dixit@intel.com> (raw)
In-Reply-To: <20230401020049.3843873-1-vinay.belgaumkar@intel.com>
On Fri, 31 Mar 2023 19:00:49 -0700, Vinay Belgaumkar wrote:
>
Hi Vinay,
> @@ -478,20 +507,15 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc *slpc, u32 val)
> val > slpc->max_freq_softlimit)
> return -EINVAL;
>
> + /* Ignore efficient freq if lower min freq is requested */
> + ret = intel_guc_slpc_set_ignore_eff_freq(slpc, val < slpc->rp1_freq);
> + if (ret)
> + goto out;
> +
I don't agree with this. If we are now providing an interface explicitly to
ignore RPe, that should be /only/ way to ignore RPe. There should be no
other "under the hood" ignoring of RPe. In other words, ignoring RPe should
be minimized unless explicitly requested.
I don't clearly understand why this was done previously but it makes even
less sense to me now after this patch.
Thanks.
--
Ashutosh
> /* Need a lock now since waitboost can be modifying min as well */
> mutex_lock(&slpc->lock);
> wakeref = intel_runtime_pm_get(&i915->runtime_pm);
>
> - /* Ignore efficient freq if lower min freq is requested */
> - ret = slpc_set_param(slpc,
> - SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY,
> - val < slpc->rp1_freq);
> - if (ret) {
> - guc_probe_error(slpc_to_guc(slpc), "Failed to toggle efficient freq: %pe\n",
> - ERR_PTR(ret));
> - goto out;
> - }
> -
> ret = slpc_set_param(slpc,
> SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
> val);
next prev parent reply other threads:[~2023-04-01 3:11 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-01 2:00 [Intel-gfx] [PATCH] i915/guc/slpc: Provide sysfs for efficient freq Vinay Belgaumkar
2023-04-01 2:26 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2023-04-01 2:26 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-04-01 2:39 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-04-01 3:11 ` Dixit, Ashutosh [this message]
2023-04-05 13:57 ` [Intel-gfx] [PATCH] " Rodrigo Vivi
2023-04-05 19:42 ` Dixit, Ashutosh
2023-04-05 20:12 ` Rodrigo Vivi
2023-04-06 5:08 ` Dixit, Ashutosh
2023-04-02 0:02 ` [Intel-gfx] ✓ Fi.CI.IGT: success for " Patchwork
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=877cuwguu6.wl-ashutosh.dixit@intel.com \
--to=ashutosh.dixit@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=rodrigo.vivi@intel.com \
--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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).