From: Michal Wajdeczko <michal.wajdeczko@intel.com> To: Vinay Belgaumkar <vinay.belgaumkar@intel.com>, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>, Sujaritha Sundaresan <sujaritha.sundaresan@intel.com> Subject: Re: [PATCH 13/15] drm/i915/guc/slpc: Sysfs hooks for SLPC Date: Tue, 27 Jul 2021 18:59:30 +0200 [thread overview] Message-ID: <e8d838e6-9d90-0099-8fe3-666875af21ce@intel.com> (raw) In-Reply-To: <20210726190800.26762-14-vinay.belgaumkar@intel.com> On 26.07.2021 21:07, Vinay Belgaumkar wrote: > Update the get/set min/max freq hooks to work for > SLPC case as well. Consolidate helpers for requested/min/max > frequency get/set to intel_rps where the proper action can > be taken depending on whether SLPC is enabled. > > v2: Add wrappers for getting rp0/1/n frequencies, update > softlimits in set min/max SLPC functions. Also check for > boundary conditions before setting them. > > v3: Address review comments (Michal W) > > Acked-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_rps.c | 165 ++++++++++++++++++++ > drivers/gpu/drm/i915/gt/intel_rps.h | 11 ++ > drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 14 ++ > drivers/gpu/drm/i915/i915_pmu.c | 2 +- > drivers/gpu/drm/i915/i915_reg.h | 2 + > drivers/gpu/drm/i915/i915_sysfs.c | 77 ++------- > 6 files changed, 207 insertions(+), 64 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c > index e858eeb2c59d..48d4147165a9 100644 > --- a/drivers/gpu/drm/i915/gt/intel_rps.c > +++ b/drivers/gpu/drm/i915/gt/intel_rps.c > @@ -37,6 +37,13 @@ static struct intel_uncore *rps_to_uncore(struct intel_rps *rps) > return rps_to_gt(rps)->uncore; > } > > +static struct intel_guc_slpc *rps_to_slpc(struct intel_rps *rps) > +{ > + struct intel_gt *gt = rps_to_gt(rps); > + > + return >->uc.guc.slpc; > +} > + > static bool rps_uses_slpc(struct intel_rps *rps) > { > struct intel_gt *gt = rps_to_gt(rps); > @@ -1960,6 +1967,164 @@ u32 intel_rps_read_actual_frequency(struct intel_rps *rps) > return freq; > } > > +u32 intel_rps_read_punit_req(struct intel_rps *rps) > +{ > + struct intel_uncore *uncore = rps_to_uncore(rps); > + > + return intel_uncore_read(uncore, GEN6_RPNSWREQ); > +} > + > +u32 intel_rps_get_req(struct intel_rps *rps, u32 pureq) hmm, "rps" looks to be not needed here btw, shouldn't this function be static ? > +{ > + u32 req = pureq >> GEN9_SW_REQ_UNSLICE_RATIO_SHIFT; > + > + return req; > +} > + > +u32 intel_rps_read_punit_req_frequency(struct intel_rps *rps) > +{ > + u32 freq = intel_rps_get_req(rps, intel_rps_read_punit_req(rps)); > + > + return intel_gpu_freq(rps, freq); > +} > + > +u32 intel_rps_get_requested_frequency(struct intel_rps *rps) > +{ > + if (rps_uses_slpc(rps)) > + return intel_rps_read_punit_req_frequency(rps); > + else > + return intel_gpu_freq(rps, rps->cur_freq); > +} > + > +u32 intel_rps_get_max_frequency(struct intel_rps *rps) > +{ > + struct intel_guc_slpc *slpc = rps_to_slpc(rps); > + > + if (rps_uses_slpc(rps)) > + return slpc->max_freq_softlimit; > + else > + return intel_gpu_freq(rps, rps->max_freq_softlimit); > +} > + > +u32 intel_rps_get_rp0_frequency(struct intel_rps *rps) > +{ > + struct intel_guc_slpc *slpc = rps_to_slpc(rps); > + > + if (rps_uses_slpc(rps)) > + return slpc->rp0_freq; > + else > + return intel_gpu_freq(rps, rps->rp0_freq); > +} > + > +u32 intel_rps_get_rp1_frequency(struct intel_rps *rps) > +{ > + struct intel_guc_slpc *slpc = rps_to_slpc(rps); > + > + if (rps_uses_slpc(rps)) > + return slpc->rp1_freq; > + else > + return intel_gpu_freq(rps, rps->rp1_freq); > +} > + > +u32 intel_rps_get_rpn_frequency(struct intel_rps *rps) > +{ > + struct intel_guc_slpc *slpc = rps_to_slpc(rps); > + > + if (rps_uses_slpc(rps)) > + return slpc->min_freq; > + else > + return intel_gpu_freq(rps, rps->min_freq); > +} > + > +int intel_rps_set_max_frequency(struct intel_rps *rps, u32 val) > +{ > + struct drm_i915_private *i915 = rps_to_i915(rps); > + struct intel_guc_slpc *slpc = rps_to_slpc(rps); > + int ret = 0; > + > + if (rps_uses_slpc(rps)) > + return intel_guc_slpc_set_max_freq(slpc, val); few above functions are implemented as nice dispatcher if (rps_uses_slpc(rps)) return ... slpc stuff; else return ... gpu stuff; can we have something similar here ? likely just putting below code into helper will do the trick > + > + mutex_lock(&rps->lock); > + > + val = intel_freq_opcode(rps, val); > + if (val < rps->min_freq || > + val > rps->max_freq || > + val < rps->min_freq_softlimit) { > + ret = -EINVAL; > + goto unlock; > + } > + > + if (val > rps->rp0_freq) > + drm_dbg(&i915->drm, "User requested overclocking to %d\n", > + intel_gpu_freq(rps, val)); > + > + rps->max_freq_softlimit = val; > + > + val = clamp_t(int, rps->cur_freq, > + rps->min_freq_softlimit, > + rps->max_freq_softlimit); > + > + /* > + * We still need *_set_rps to process the new max_delay and > + * update the interrupt limits and PMINTRMSK even though > + * frequency request may be unchanged. > + */ > + intel_rps_set(rps, val); > + > +unlock: > + mutex_unlock(&rps->lock); > + > + return ret; > +} > + > +u32 intel_rps_get_min_frequency(struct intel_rps *rps) > +{ > + struct intel_guc_slpc *slpc = rps_to_slpc(rps); > + > + if (rps_uses_slpc(rps)) > + return slpc->min_freq_softlimit; > + else > + return intel_gpu_freq(rps, rps->min_freq_softlimit); > +} > + > +int intel_rps_set_min_frequency(struct intel_rps *rps, u32 val) > +{ > + struct intel_guc_slpc *slpc = rps_to_slpc(rps); > + int ret = 0; > + > + if (rps_uses_slpc(rps)) > + return intel_guc_slpc_set_min_freq(slpc, val); > + > + mutex_lock(&rps->lock); > + > + val = intel_freq_opcode(rps, val); > + if (val < rps->min_freq || > + val > rps->max_freq || > + val > rps->max_freq_softlimit) { > + ret = -EINVAL; > + goto unlock; > + } > + > + rps->min_freq_softlimit = val; > + > + val = clamp_t(int, rps->cur_freq, > + rps->min_freq_softlimit, > + rps->max_freq_softlimit); > + > + /* > + * We still need *_set_rps to process the new min_delay and > + * update the interrupt limits and PMINTRMSK even though > + * frequency request may be unchanged. > + */ > + intel_rps_set(rps, val); > + > +unlock: > + mutex_unlock(&rps->lock); > + > + return ret; > +} > + > /* External interface for intel_ips.ko */ > > static struct drm_i915_private __rcu *ips_mchdev; > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.h b/drivers/gpu/drm/i915/gt/intel_rps.h > index 1d2cfc98b510..6a66690dfb0f 100644 > --- a/drivers/gpu/drm/i915/gt/intel_rps.h > +++ b/drivers/gpu/drm/i915/gt/intel_rps.h > @@ -31,6 +31,17 @@ int intel_gpu_freq(struct intel_rps *rps, int val); > int intel_freq_opcode(struct intel_rps *rps, int val); > u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat1); > u32 intel_rps_read_actual_frequency(struct intel_rps *rps); > +u32 intel_rps_get_requested_frequency(struct intel_rps *rps); > +u32 intel_rps_get_min_frequency(struct intel_rps *rps); > +int intel_rps_set_min_frequency(struct intel_rps *rps, u32 val); > +u32 intel_rps_get_max_frequency(struct intel_rps *rps); > +int intel_rps_set_max_frequency(struct intel_rps *rps, u32 val); > +u32 intel_rps_get_rp0_frequency(struct intel_rps *rps); > +u32 intel_rps_get_rp1_frequency(struct intel_rps *rps); > +u32 intel_rps_get_rpn_frequency(struct intel_rps *rps); > +u32 intel_rps_read_punit_req(struct intel_rps *rps); > +u32 intel_rps_get_req(struct intel_rps *rps, u32 pureq); > +u32 intel_rps_read_punit_req_frequency(struct intel_rps *rps); > > void gen5_rps_irq_handler(struct intel_rps *rps); > void gen6_rps_irq_handler(struct intel_rps *rps, u32 pm_iir); > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > index a98cbf274862..03861eb913d1 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > @@ -316,6 +316,11 @@ int intel_guc_slpc_set_max_freq(struct intel_guc_slpc *slpc, u32 val) > intel_wakeref_t wakeref; > int ret; > > + if ((val < slpc->min_freq) || > + (val > slpc->rp0_freq) || > + (val < slpc->min_freq_softlimit)) > + return -EINVAL; shouldn't this be part of the earlier patch ? > + > with_intel_runtime_pm(&i915->runtime_pm, wakeref) { > ret = slpc_set_param(slpc, > SLPC_PARAM_GLOBAL_MAX_GT_UNSLICE_FREQ_MHZ, > @@ -328,6 +333,8 @@ int intel_guc_slpc_set_max_freq(struct intel_guc_slpc *slpc, u32 val) > } > } > > + slpc->max_freq_softlimit = val; why in this patch? > + > return ret; > } > > @@ -375,6 +382,11 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc *slpc, u32 val) > struct drm_i915_private *i915 = guc_to_gt(guc)->i915; > intel_wakeref_t wakeref; > > + if ((val < slpc->min_freq) || > + (val > slpc->rp0_freq) || > + (val > slpc->max_freq_softlimit)) > + return -EINVAL; same here > + > with_intel_runtime_pm(&i915->runtime_pm, wakeref) { > ret = slpc_set_param(slpc, > SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ, > @@ -387,6 +399,8 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc *slpc, u32 val) > } > } > > + slpc->min_freq_softlimit = val; > + > return ret; > } > > diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c > index eca92076f31d..0b488d49694c 100644 > --- a/drivers/gpu/drm/i915/i915_pmu.c > +++ b/drivers/gpu/drm/i915/i915_pmu.c > @@ -407,7 +407,7 @@ frequency_sample(struct intel_gt *gt, unsigned int period_ns) > > if (pmu->enable & config_mask(I915_PMU_REQUESTED_FREQUENCY)) { > add_sample_mult(&pmu->sample[__I915_SAMPLE_FREQ_REQ], > - intel_gpu_freq(rps, rps->cur_freq), > + intel_rps_get_requested_frequency(rps), > period_ns / 1000); > } > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index f3a445f79a36..b4527ca027e3 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -9229,6 +9229,8 @@ enum { > #define GEN9_FREQUENCY(x) ((x) << 23) > #define GEN6_OFFSET(x) ((x) << 19) > #define GEN6_AGGRESSIVE_TURBO (0 << 15) > +#define GEN9_SW_REQ_UNSLICE_RATIO_SHIFT 23 > + > #define GEN6_RC_VIDEO_FREQ _MMIO(0xA00C) > #define GEN6_RC_CONTROL _MMIO(0xA090) > #define GEN6_RC_CTL_RC6pp_ENABLE (1 << 16) > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c > index 873bf996ceb5..346646a0b43b 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.c > +++ b/drivers/gpu/drm/i915/i915_sysfs.c > @@ -272,7 +272,7 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev, > struct drm_i915_private *i915 = kdev_minor_to_i915(kdev); > struct intel_rps *rps = &i915->gt.rps; > > - return sysfs_emit(buf, "%d\n", intel_gpu_freq(rps, rps->cur_freq)); > + return sysfs_emit(buf, "%d\n", intel_rps_get_requested_frequency(rps)); > } > > static ssize_t gt_boost_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf) > @@ -326,9 +326,10 @@ static ssize_t vlv_rpe_freq_mhz_show(struct device *kdev, > static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf) > { > struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - struct intel_rps *rps = &dev_priv->gt.rps; > + struct intel_gt *gt = &dev_priv->gt; while here, maybe worth to s/dev_priv/i915 same below Michal > + struct intel_rps *rps = >->rps; > > - return sysfs_emit(buf, "%d\n", intel_gpu_freq(rps, rps->max_freq_softlimit)); > + return sysfs_emit(buf, "%d\n", intel_rps_get_max_frequency(rps)); > } > > static ssize_t gt_max_freq_mhz_store(struct device *kdev, > @@ -336,7 +337,8 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev, > const char *buf, size_t count) > { > struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - struct intel_rps *rps = &dev_priv->gt.rps; > + struct intel_gt *gt = &dev_priv->gt; > + struct intel_rps *rps = >->rps; > ssize_t ret; > u32 val; > > @@ -344,35 +346,7 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev, > if (ret) > return ret; > > - mutex_lock(&rps->lock); > - > - val = intel_freq_opcode(rps, val); > - if (val < rps->min_freq || > - val > rps->max_freq || > - val < rps->min_freq_softlimit) { > - ret = -EINVAL; > - goto unlock; > - } > - > - if (val > rps->rp0_freq) > - DRM_DEBUG("User requested overclocking to %d\n", > - intel_gpu_freq(rps, val)); > - > - rps->max_freq_softlimit = val; > - > - val = clamp_t(int, rps->cur_freq, > - rps->min_freq_softlimit, > - rps->max_freq_softlimit); > - > - /* > - * We still need *_set_rps to process the new max_delay and > - * update the interrupt limits and PMINTRMSK even though > - * frequency request may be unchanged. > - */ > - intel_rps_set(rps, val); > - > -unlock: > - mutex_unlock(&rps->lock); > + ret = intel_rps_set_max_frequency(rps, val); > > return ret ?: count; > } > @@ -380,9 +354,10 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev, > static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf) > { > struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - struct intel_rps *rps = &dev_priv->gt.rps; > + struct intel_gt *gt = &dev_priv->gt; > + struct intel_rps *rps = >->rps; > > - return sysfs_emit(buf, "%d\n", intel_gpu_freq(rps, rps->min_freq_softlimit)); > + return sysfs_emit(buf, "%d\n", intel_rps_get_min_frequency(rps)); > } > > static ssize_t gt_min_freq_mhz_store(struct device *kdev, > @@ -398,31 +373,7 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev, > if (ret) > return ret; > > - mutex_lock(&rps->lock); > - > - val = intel_freq_opcode(rps, val); > - if (val < rps->min_freq || > - val > rps->max_freq || > - val > rps->max_freq_softlimit) { > - ret = -EINVAL; > - goto unlock; > - } > - > - rps->min_freq_softlimit = val; > - > - val = clamp_t(int, rps->cur_freq, > - rps->min_freq_softlimit, > - rps->max_freq_softlimit); > - > - /* > - * We still need *_set_rps to process the new min_delay and > - * update the interrupt limits and PMINTRMSK even though > - * frequency request may be unchanged. > - */ > - intel_rps_set(rps, val); > - > -unlock: > - mutex_unlock(&rps->lock); > + ret = intel_rps_set_min_frequency(rps, val); > > return ret ?: count; > } > @@ -448,11 +399,11 @@ static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr > u32 val; > > if (attr == &dev_attr_gt_RP0_freq_mhz) > - val = intel_gpu_freq(rps, rps->rp0_freq); > + val = intel_rps_get_rp0_frequency(rps); > else if (attr == &dev_attr_gt_RP1_freq_mhz) > - val = intel_gpu_freq(rps, rps->rp1_freq); > + val = intel_rps_get_rp1_frequency(rps); > else if (attr == &dev_attr_gt_RPn_freq_mhz) > - val = intel_gpu_freq(rps, rps->min_freq); > + val = intel_rps_get_rpn_frequency(rps); > else > BUG(); > >
WARNING: multiple messages have this Message-ID (diff)
From: Michal Wajdeczko <michal.wajdeczko@intel.com> To: Vinay Belgaumkar <vinay.belgaumkar@intel.com>, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 13/15] drm/i915/guc/slpc: Sysfs hooks for SLPC Date: Tue, 27 Jul 2021 18:59:30 +0200 [thread overview] Message-ID: <e8d838e6-9d90-0099-8fe3-666875af21ce@intel.com> (raw) In-Reply-To: <20210726190800.26762-14-vinay.belgaumkar@intel.com> On 26.07.2021 21:07, Vinay Belgaumkar wrote: > Update the get/set min/max freq hooks to work for > SLPC case as well. Consolidate helpers for requested/min/max > frequency get/set to intel_rps where the proper action can > be taken depending on whether SLPC is enabled. > > v2: Add wrappers for getting rp0/1/n frequencies, update > softlimits in set min/max SLPC functions. Also check for > boundary conditions before setting them. > > v3: Address review comments (Michal W) > > Acked-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_rps.c | 165 ++++++++++++++++++++ > drivers/gpu/drm/i915/gt/intel_rps.h | 11 ++ > drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 14 ++ > drivers/gpu/drm/i915/i915_pmu.c | 2 +- > drivers/gpu/drm/i915/i915_reg.h | 2 + > drivers/gpu/drm/i915/i915_sysfs.c | 77 ++------- > 6 files changed, 207 insertions(+), 64 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c > index e858eeb2c59d..48d4147165a9 100644 > --- a/drivers/gpu/drm/i915/gt/intel_rps.c > +++ b/drivers/gpu/drm/i915/gt/intel_rps.c > @@ -37,6 +37,13 @@ static struct intel_uncore *rps_to_uncore(struct intel_rps *rps) > return rps_to_gt(rps)->uncore; > } > > +static struct intel_guc_slpc *rps_to_slpc(struct intel_rps *rps) > +{ > + struct intel_gt *gt = rps_to_gt(rps); > + > + return >->uc.guc.slpc; > +} > + > static bool rps_uses_slpc(struct intel_rps *rps) > { > struct intel_gt *gt = rps_to_gt(rps); > @@ -1960,6 +1967,164 @@ u32 intel_rps_read_actual_frequency(struct intel_rps *rps) > return freq; > } > > +u32 intel_rps_read_punit_req(struct intel_rps *rps) > +{ > + struct intel_uncore *uncore = rps_to_uncore(rps); > + > + return intel_uncore_read(uncore, GEN6_RPNSWREQ); > +} > + > +u32 intel_rps_get_req(struct intel_rps *rps, u32 pureq) hmm, "rps" looks to be not needed here btw, shouldn't this function be static ? > +{ > + u32 req = pureq >> GEN9_SW_REQ_UNSLICE_RATIO_SHIFT; > + > + return req; > +} > + > +u32 intel_rps_read_punit_req_frequency(struct intel_rps *rps) > +{ > + u32 freq = intel_rps_get_req(rps, intel_rps_read_punit_req(rps)); > + > + return intel_gpu_freq(rps, freq); > +} > + > +u32 intel_rps_get_requested_frequency(struct intel_rps *rps) > +{ > + if (rps_uses_slpc(rps)) > + return intel_rps_read_punit_req_frequency(rps); > + else > + return intel_gpu_freq(rps, rps->cur_freq); > +} > + > +u32 intel_rps_get_max_frequency(struct intel_rps *rps) > +{ > + struct intel_guc_slpc *slpc = rps_to_slpc(rps); > + > + if (rps_uses_slpc(rps)) > + return slpc->max_freq_softlimit; > + else > + return intel_gpu_freq(rps, rps->max_freq_softlimit); > +} > + > +u32 intel_rps_get_rp0_frequency(struct intel_rps *rps) > +{ > + struct intel_guc_slpc *slpc = rps_to_slpc(rps); > + > + if (rps_uses_slpc(rps)) > + return slpc->rp0_freq; > + else > + return intel_gpu_freq(rps, rps->rp0_freq); > +} > + > +u32 intel_rps_get_rp1_frequency(struct intel_rps *rps) > +{ > + struct intel_guc_slpc *slpc = rps_to_slpc(rps); > + > + if (rps_uses_slpc(rps)) > + return slpc->rp1_freq; > + else > + return intel_gpu_freq(rps, rps->rp1_freq); > +} > + > +u32 intel_rps_get_rpn_frequency(struct intel_rps *rps) > +{ > + struct intel_guc_slpc *slpc = rps_to_slpc(rps); > + > + if (rps_uses_slpc(rps)) > + return slpc->min_freq; > + else > + return intel_gpu_freq(rps, rps->min_freq); > +} > + > +int intel_rps_set_max_frequency(struct intel_rps *rps, u32 val) > +{ > + struct drm_i915_private *i915 = rps_to_i915(rps); > + struct intel_guc_slpc *slpc = rps_to_slpc(rps); > + int ret = 0; > + > + if (rps_uses_slpc(rps)) > + return intel_guc_slpc_set_max_freq(slpc, val); few above functions are implemented as nice dispatcher if (rps_uses_slpc(rps)) return ... slpc stuff; else return ... gpu stuff; can we have something similar here ? likely just putting below code into helper will do the trick > + > + mutex_lock(&rps->lock); > + > + val = intel_freq_opcode(rps, val); > + if (val < rps->min_freq || > + val > rps->max_freq || > + val < rps->min_freq_softlimit) { > + ret = -EINVAL; > + goto unlock; > + } > + > + if (val > rps->rp0_freq) > + drm_dbg(&i915->drm, "User requested overclocking to %d\n", > + intel_gpu_freq(rps, val)); > + > + rps->max_freq_softlimit = val; > + > + val = clamp_t(int, rps->cur_freq, > + rps->min_freq_softlimit, > + rps->max_freq_softlimit); > + > + /* > + * We still need *_set_rps to process the new max_delay and > + * update the interrupt limits and PMINTRMSK even though > + * frequency request may be unchanged. > + */ > + intel_rps_set(rps, val); > + > +unlock: > + mutex_unlock(&rps->lock); > + > + return ret; > +} > + > +u32 intel_rps_get_min_frequency(struct intel_rps *rps) > +{ > + struct intel_guc_slpc *slpc = rps_to_slpc(rps); > + > + if (rps_uses_slpc(rps)) > + return slpc->min_freq_softlimit; > + else > + return intel_gpu_freq(rps, rps->min_freq_softlimit); > +} > + > +int intel_rps_set_min_frequency(struct intel_rps *rps, u32 val) > +{ > + struct intel_guc_slpc *slpc = rps_to_slpc(rps); > + int ret = 0; > + > + if (rps_uses_slpc(rps)) > + return intel_guc_slpc_set_min_freq(slpc, val); > + > + mutex_lock(&rps->lock); > + > + val = intel_freq_opcode(rps, val); > + if (val < rps->min_freq || > + val > rps->max_freq || > + val > rps->max_freq_softlimit) { > + ret = -EINVAL; > + goto unlock; > + } > + > + rps->min_freq_softlimit = val; > + > + val = clamp_t(int, rps->cur_freq, > + rps->min_freq_softlimit, > + rps->max_freq_softlimit); > + > + /* > + * We still need *_set_rps to process the new min_delay and > + * update the interrupt limits and PMINTRMSK even though > + * frequency request may be unchanged. > + */ > + intel_rps_set(rps, val); > + > +unlock: > + mutex_unlock(&rps->lock); > + > + return ret; > +} > + > /* External interface for intel_ips.ko */ > > static struct drm_i915_private __rcu *ips_mchdev; > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.h b/drivers/gpu/drm/i915/gt/intel_rps.h > index 1d2cfc98b510..6a66690dfb0f 100644 > --- a/drivers/gpu/drm/i915/gt/intel_rps.h > +++ b/drivers/gpu/drm/i915/gt/intel_rps.h > @@ -31,6 +31,17 @@ int intel_gpu_freq(struct intel_rps *rps, int val); > int intel_freq_opcode(struct intel_rps *rps, int val); > u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat1); > u32 intel_rps_read_actual_frequency(struct intel_rps *rps); > +u32 intel_rps_get_requested_frequency(struct intel_rps *rps); > +u32 intel_rps_get_min_frequency(struct intel_rps *rps); > +int intel_rps_set_min_frequency(struct intel_rps *rps, u32 val); > +u32 intel_rps_get_max_frequency(struct intel_rps *rps); > +int intel_rps_set_max_frequency(struct intel_rps *rps, u32 val); > +u32 intel_rps_get_rp0_frequency(struct intel_rps *rps); > +u32 intel_rps_get_rp1_frequency(struct intel_rps *rps); > +u32 intel_rps_get_rpn_frequency(struct intel_rps *rps); > +u32 intel_rps_read_punit_req(struct intel_rps *rps); > +u32 intel_rps_get_req(struct intel_rps *rps, u32 pureq); > +u32 intel_rps_read_punit_req_frequency(struct intel_rps *rps); > > void gen5_rps_irq_handler(struct intel_rps *rps); > void gen6_rps_irq_handler(struct intel_rps *rps, u32 pm_iir); > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > index a98cbf274862..03861eb913d1 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > @@ -316,6 +316,11 @@ int intel_guc_slpc_set_max_freq(struct intel_guc_slpc *slpc, u32 val) > intel_wakeref_t wakeref; > int ret; > > + if ((val < slpc->min_freq) || > + (val > slpc->rp0_freq) || > + (val < slpc->min_freq_softlimit)) > + return -EINVAL; shouldn't this be part of the earlier patch ? > + > with_intel_runtime_pm(&i915->runtime_pm, wakeref) { > ret = slpc_set_param(slpc, > SLPC_PARAM_GLOBAL_MAX_GT_UNSLICE_FREQ_MHZ, > @@ -328,6 +333,8 @@ int intel_guc_slpc_set_max_freq(struct intel_guc_slpc *slpc, u32 val) > } > } > > + slpc->max_freq_softlimit = val; why in this patch? > + > return ret; > } > > @@ -375,6 +382,11 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc *slpc, u32 val) > struct drm_i915_private *i915 = guc_to_gt(guc)->i915; > intel_wakeref_t wakeref; > > + if ((val < slpc->min_freq) || > + (val > slpc->rp0_freq) || > + (val > slpc->max_freq_softlimit)) > + return -EINVAL; same here > + > with_intel_runtime_pm(&i915->runtime_pm, wakeref) { > ret = slpc_set_param(slpc, > SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ, > @@ -387,6 +399,8 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc *slpc, u32 val) > } > } > > + slpc->min_freq_softlimit = val; > + > return ret; > } > > diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c > index eca92076f31d..0b488d49694c 100644 > --- a/drivers/gpu/drm/i915/i915_pmu.c > +++ b/drivers/gpu/drm/i915/i915_pmu.c > @@ -407,7 +407,7 @@ frequency_sample(struct intel_gt *gt, unsigned int period_ns) > > if (pmu->enable & config_mask(I915_PMU_REQUESTED_FREQUENCY)) { > add_sample_mult(&pmu->sample[__I915_SAMPLE_FREQ_REQ], > - intel_gpu_freq(rps, rps->cur_freq), > + intel_rps_get_requested_frequency(rps), > period_ns / 1000); > } > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index f3a445f79a36..b4527ca027e3 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -9229,6 +9229,8 @@ enum { > #define GEN9_FREQUENCY(x) ((x) << 23) > #define GEN6_OFFSET(x) ((x) << 19) > #define GEN6_AGGRESSIVE_TURBO (0 << 15) > +#define GEN9_SW_REQ_UNSLICE_RATIO_SHIFT 23 > + > #define GEN6_RC_VIDEO_FREQ _MMIO(0xA00C) > #define GEN6_RC_CONTROL _MMIO(0xA090) > #define GEN6_RC_CTL_RC6pp_ENABLE (1 << 16) > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c > index 873bf996ceb5..346646a0b43b 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.c > +++ b/drivers/gpu/drm/i915/i915_sysfs.c > @@ -272,7 +272,7 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev, > struct drm_i915_private *i915 = kdev_minor_to_i915(kdev); > struct intel_rps *rps = &i915->gt.rps; > > - return sysfs_emit(buf, "%d\n", intel_gpu_freq(rps, rps->cur_freq)); > + return sysfs_emit(buf, "%d\n", intel_rps_get_requested_frequency(rps)); > } > > static ssize_t gt_boost_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf) > @@ -326,9 +326,10 @@ static ssize_t vlv_rpe_freq_mhz_show(struct device *kdev, > static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf) > { > struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - struct intel_rps *rps = &dev_priv->gt.rps; > + struct intel_gt *gt = &dev_priv->gt; while here, maybe worth to s/dev_priv/i915 same below Michal > + struct intel_rps *rps = >->rps; > > - return sysfs_emit(buf, "%d\n", intel_gpu_freq(rps, rps->max_freq_softlimit)); > + return sysfs_emit(buf, "%d\n", intel_rps_get_max_frequency(rps)); > } > > static ssize_t gt_max_freq_mhz_store(struct device *kdev, > @@ -336,7 +337,8 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev, > const char *buf, size_t count) > { > struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - struct intel_rps *rps = &dev_priv->gt.rps; > + struct intel_gt *gt = &dev_priv->gt; > + struct intel_rps *rps = >->rps; > ssize_t ret; > u32 val; > > @@ -344,35 +346,7 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev, > if (ret) > return ret; > > - mutex_lock(&rps->lock); > - > - val = intel_freq_opcode(rps, val); > - if (val < rps->min_freq || > - val > rps->max_freq || > - val < rps->min_freq_softlimit) { > - ret = -EINVAL; > - goto unlock; > - } > - > - if (val > rps->rp0_freq) > - DRM_DEBUG("User requested overclocking to %d\n", > - intel_gpu_freq(rps, val)); > - > - rps->max_freq_softlimit = val; > - > - val = clamp_t(int, rps->cur_freq, > - rps->min_freq_softlimit, > - rps->max_freq_softlimit); > - > - /* > - * We still need *_set_rps to process the new max_delay and > - * update the interrupt limits and PMINTRMSK even though > - * frequency request may be unchanged. > - */ > - intel_rps_set(rps, val); > - > -unlock: > - mutex_unlock(&rps->lock); > + ret = intel_rps_set_max_frequency(rps, val); > > return ret ?: count; > } > @@ -380,9 +354,10 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev, > static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf) > { > struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - struct intel_rps *rps = &dev_priv->gt.rps; > + struct intel_gt *gt = &dev_priv->gt; > + struct intel_rps *rps = >->rps; > > - return sysfs_emit(buf, "%d\n", intel_gpu_freq(rps, rps->min_freq_softlimit)); > + return sysfs_emit(buf, "%d\n", intel_rps_get_min_frequency(rps)); > } > > static ssize_t gt_min_freq_mhz_store(struct device *kdev, > @@ -398,31 +373,7 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev, > if (ret) > return ret; > > - mutex_lock(&rps->lock); > - > - val = intel_freq_opcode(rps, val); > - if (val < rps->min_freq || > - val > rps->max_freq || > - val > rps->max_freq_softlimit) { > - ret = -EINVAL; > - goto unlock; > - } > - > - rps->min_freq_softlimit = val; > - > - val = clamp_t(int, rps->cur_freq, > - rps->min_freq_softlimit, > - rps->max_freq_softlimit); > - > - /* > - * We still need *_set_rps to process the new min_delay and > - * update the interrupt limits and PMINTRMSK even though > - * frequency request may be unchanged. > - */ > - intel_rps_set(rps, val); > - > -unlock: > - mutex_unlock(&rps->lock); > + ret = intel_rps_set_min_frequency(rps, val); > > return ret ?: count; > } > @@ -448,11 +399,11 @@ static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr > u32 val; > > if (attr == &dev_attr_gt_RP0_freq_mhz) > - val = intel_gpu_freq(rps, rps->rp0_freq); > + val = intel_rps_get_rp0_frequency(rps); > else if (attr == &dev_attr_gt_RP1_freq_mhz) > - val = intel_gpu_freq(rps, rps->rp1_freq); > + val = intel_rps_get_rp1_frequency(rps); > else if (attr == &dev_attr_gt_RPn_freq_mhz) > - val = intel_gpu_freq(rps, rps->min_freq); > + val = intel_rps_get_rpn_frequency(rps); > else > BUG(); > > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2021-07-27 16:59 UTC|newest] Thread overview: 102+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-07-26 19:07 [PATCH v3 00/15] drm/i915/guc/slpc: Enable GuC based power management features Vinay Belgaumkar 2021-07-26 19:07 ` [Intel-gfx] " Vinay Belgaumkar 2021-07-26 19:07 ` [PATCH 01/15] drm/i915/guc: SQUASHED PATCH - DO NOT REVIEW Vinay Belgaumkar 2021-07-26 19:07 ` [Intel-gfx] " Vinay Belgaumkar 2021-07-26 19:07 ` [PATCH 02/15] drm/i915/guc/slpc: Initial definitions for SLPC Vinay Belgaumkar 2021-07-26 19:07 ` [Intel-gfx] " Vinay Belgaumkar 2021-07-27 13:43 ` Michal Wajdeczko 2021-07-27 13:43 ` [Intel-gfx] " Michal Wajdeczko 2021-07-27 18:47 ` Belgaumkar, Vinay 2021-07-27 18:47 ` [Intel-gfx] " Belgaumkar, Vinay 2021-07-26 19:07 ` [PATCH 03/15] drm/i915/guc/slpc: Gate Host RPS when SLPC is enabled Vinay Belgaumkar 2021-07-26 19:07 ` [Intel-gfx] " Vinay Belgaumkar 2021-07-27 22:44 ` Matthew Brost 2021-07-27 22:44 ` [Intel-gfx] " Matthew Brost 2021-07-27 22:48 ` Belgaumkar, Vinay 2021-07-27 22:48 ` [Intel-gfx] " Belgaumkar, Vinay 2021-07-27 22:50 ` Matthew Brost 2021-07-27 22:50 ` [Intel-gfx] " Matthew Brost 2021-07-26 19:07 ` [PATCH 04/15] drm/i915/guc/slpc: Adding SLPC communication interfaces Vinay Belgaumkar 2021-07-26 19:07 ` [Intel-gfx] " Vinay Belgaumkar 2021-07-27 13:59 ` Michal Wajdeczko 2021-07-27 13:59 ` [Intel-gfx] " Michal Wajdeczko 2021-07-27 19:03 ` Belgaumkar, Vinay 2021-07-27 19:03 ` [Intel-gfx] " Belgaumkar, Vinay 2021-07-26 19:07 ` [PATCH 05/15] drm/i915/guc/slpc: Allocate, initialize and release SLPC Vinay Belgaumkar 2021-07-26 19:07 ` [Intel-gfx] " Vinay Belgaumkar 2021-07-27 14:03 ` Michal Wajdeczko 2021-07-27 14:03 ` [Intel-gfx] " Michal Wajdeczko 2021-07-26 19:07 ` [PATCH 06/15] drm/i915/guc/slpc: Enable SLPC and add related H2G events Vinay Belgaumkar 2021-07-26 19:07 ` [Intel-gfx] " Vinay Belgaumkar 2021-07-27 15:12 ` Michal Wajdeczko 2021-07-27 15:12 ` Michal Wajdeczko 2021-07-27 20:00 ` Belgaumkar, Vinay 2021-07-27 20:00 ` Belgaumkar, Vinay 2021-07-27 20:19 ` Michal Wajdeczko 2021-07-27 20:19 ` Michal Wajdeczko 2021-07-27 20:52 ` Belgaumkar, Vinay 2021-07-27 20:52 ` Belgaumkar, Vinay 2021-07-26 19:07 ` [PATCH 07/15] drm/i915/guc/slpc: Remove BUG_ON in guc_submission_disable Vinay Belgaumkar 2021-07-26 19:07 ` [Intel-gfx] " Vinay Belgaumkar 2021-07-28 0:20 ` Matthew Brost 2021-07-28 0:20 ` [Intel-gfx] " Matthew Brost 2021-07-28 1:01 ` Belgaumkar, Vinay 2021-07-28 1:01 ` [Intel-gfx] " Belgaumkar, Vinay 2021-07-28 1:06 ` Matthew Brost 2021-07-28 1:06 ` [Intel-gfx] " Matthew Brost 2021-07-26 19:07 ` [PATCH 08/15] drm/i915/guc/slpc: Add methods to set min/max frequency Vinay Belgaumkar 2021-07-26 19:07 ` [Intel-gfx] " Vinay Belgaumkar 2021-07-27 15:24 ` Michal Wajdeczko 2021-07-27 15:24 ` Michal Wajdeczko 2021-07-27 22:35 ` Belgaumkar, Vinay 2021-07-27 22:35 ` Belgaumkar, Vinay 2021-07-28 4:03 ` Belgaumkar, Vinay 2021-07-28 4:03 ` Belgaumkar, Vinay 2021-07-26 19:07 ` [PATCH 09/15] drm/i915/guc/slpc: Add get max/min freq hooks Vinay Belgaumkar 2021-07-26 19:07 ` [Intel-gfx] " Vinay Belgaumkar 2021-07-27 15:32 ` Michal Wajdeczko 2021-07-27 15:32 ` Michal Wajdeczko 2021-07-27 23:10 ` Belgaumkar, Vinay 2021-07-27 23:10 ` Belgaumkar, Vinay 2021-07-26 19:07 ` [PATCH 10/15] drm/i915/guc/slpc: Add debugfs for SLPC info Vinay Belgaumkar 2021-07-26 19:07 ` [Intel-gfx] " Vinay Belgaumkar 2021-07-27 15:37 ` Michal Wajdeczko 2021-07-27 15:37 ` [Intel-gfx] " Michal Wajdeczko 2021-07-28 0:10 ` Belgaumkar, Vinay 2021-07-28 0:10 ` [Intel-gfx] " Belgaumkar, Vinay 2021-07-26 19:07 ` [PATCH 11/15] drm/i915/guc/slpc: Enable ARAT timer interrupt Vinay Belgaumkar 2021-07-26 19:07 ` [Intel-gfx] " Vinay Belgaumkar 2021-07-27 15:40 ` Matthew Brost 2021-07-27 15:40 ` [Intel-gfx] " Matthew Brost 2021-07-28 0:15 ` Belgaumkar, Vinay 2021-07-28 0:15 ` [Intel-gfx] " Belgaumkar, Vinay 2021-07-26 19:07 ` [PATCH 12/15] drm/i915/guc/slpc: Cache platform frequency limits Vinay Belgaumkar 2021-07-26 19:07 ` [Intel-gfx] " Vinay Belgaumkar 2021-07-27 16:00 ` Michal Wajdeczko 2021-07-27 16:00 ` [Intel-gfx] " Michal Wajdeczko 2021-07-28 1:27 ` Belgaumkar, Vinay 2021-07-28 1:27 ` [Intel-gfx] " Belgaumkar, Vinay 2021-07-26 19:07 ` [PATCH 13/15] drm/i915/guc/slpc: Sysfs hooks for SLPC Vinay Belgaumkar 2021-07-26 19:07 ` [Intel-gfx] " Vinay Belgaumkar 2021-07-27 16:59 ` Michal Wajdeczko [this message] 2021-07-27 16:59 ` Michal Wajdeczko 2021-07-28 15:29 ` Belgaumkar, Vinay 2021-07-28 15:29 ` [Intel-gfx] " Belgaumkar, Vinay 2021-07-26 19:07 ` [PATCH 14/15] drm/i915/guc/slpc: Add SLPC selftest Vinay Belgaumkar 2021-07-26 19:07 ` [Intel-gfx] " Vinay Belgaumkar 2021-07-27 19:16 ` Matthew Brost 2021-07-27 19:16 ` [Intel-gfx] " Matthew Brost 2021-07-27 22:25 ` Belgaumkar, Vinay 2021-07-27 22:25 ` [Intel-gfx] " Belgaumkar, Vinay 2021-07-26 19:08 ` [PATCH 15/15] drm/i915/guc/rc: Setup and enable GUCRC feature Vinay Belgaumkar 2021-07-26 19:08 ` [Intel-gfx] " Vinay Belgaumkar 2021-07-27 15:37 ` Matt Roper 2021-07-27 15:37 ` [Intel-gfx] " Matt Roper 2021-07-27 16:18 ` Belgaumkar, Vinay 2021-07-27 16:18 ` [Intel-gfx] " Belgaumkar, Vinay 2021-07-27 19:49 ` Matt Roper 2021-07-27 19:49 ` [Intel-gfx] " Matt Roper 2021-07-26 19:34 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/guc/slpc: Enable GuC based power management features Patchwork 2021-07-26 19:36 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork 2021-07-26 19:59 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork 2021-07-26 23:32 ` [Intel-gfx] ✓ Fi.CI.IGT: " 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=e8d838e6-9d90-0099-8fe3-666875af21ce@intel.com \ --to=michal.wajdeczko@intel.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=intel-gfx@lists.freedesktop.org \ --cc=sujaritha.sundaresan@intel.com \ --cc=tvrtko.ursulin@linux.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: 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.