Hi Vinay,

On Tue, Apr 25, 2023 at 05:39:41PM -0700, Vinay Belgaumkar wrote:
> SLPC enables use of efficient freq at init by default. It is
> possible for GuC to request frequencies that are higher than
> the 'software' max if user has set it lower than the efficient
> level.
> 
> Scenarios/tests that require strict fixing of freq below the efficient
> level will need to disable it through this interface.
> 
> v2: Keep just one interface to toggle sysfs. With this, user will
> be completely responsible for toggling efficient frequency if need
> be. There will be no implicit disabling when user sets min < RP1 (Ashutosh)
> 
> v3: Remove unused label, review comments (Ashutosh)
> 
> v4: Toggle efficient freq usage in SLPC selftest and checkpatch fixes
> 
> v5: Review comments (Andi) and add a separate patch for selftest updates
> 
> Fixes: 95ccf312a1e4 ("drm/i915/guc/slpc: Allow SLPC to use efficient 
> frequency")
> Signed-off-by: Vinay Belgaumkar <[email protected]>
> Reviewed-by: Rodrigo Vivi <[email protected]>
> Reviewed-by: Ashutosh Dixit <[email protected]>

Reviewed-by: Andi Shyti <[email protected]> 

Andi

> ---
>  drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c   | 35 +++++++++++++++++
>  drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c   | 38 +++++++++++++------
>  drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h   |  1 +
>  .../gpu/drm/i915/gt/uc/intel_guc_slpc_types.h |  1 +
>  4 files changed, 64 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c 
> b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> index 28f27091cd3b..ee2b44f896a2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> @@ -451,6 +451,33 @@ static ssize_t punit_req_freq_mhz_show(struct kobject 
> *kobj,
>       return sysfs_emit(buff, "%u\n", preq);
>  }
>  
> +static ssize_t slpc_ignore_eff_freq_show(struct kobject *kobj,
> +                                      struct kobj_attribute *attr,
> +                                      char *buff)
> +{
> +     struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> +     struct intel_guc_slpc *slpc = &gt->uc.guc.slpc;
> +
> +     return sysfs_emit(buff, "%u\n", slpc->ignore_eff_freq);
> +}
> +
> +static ssize_t slpc_ignore_eff_freq_store(struct kobject *kobj,
> +                                       struct kobj_attribute *attr,
> +                                       const char *buff, size_t count)
> +{
> +     struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> +     struct intel_guc_slpc *slpc = &gt->uc.guc.slpc;
> +     int err;
> +     u32 val;
> +
> +     err = kstrtou32(buff, 0, &val);
> +     if (err)
> +             return err;
> +
> +     err = intel_guc_slpc_set_ignore_eff_freq(slpc, val);
> +     return err ?: count;
> +}
> +
>  struct intel_gt_bool_throttle_attr {
>       struct attribute attr;
>       ssize_t (*show)(struct kobject *kobj, struct kobj_attribute *attr,
> @@ -663,6 +690,8 @@ static struct kobj_attribute attr_media_freq_factor_scale 
> =
>  INTEL_GT_ATTR_RO(media_RP0_freq_mhz);
>  INTEL_GT_ATTR_RO(media_RPn_freq_mhz);
>  
> +INTEL_GT_ATTR_RW(slpc_ignore_eff_freq);
> +
>  static const struct attribute *media_perf_power_attrs[] = {
>       &attr_media_freq_factor.attr,
>       &attr_media_freq_factor_scale.attr,
> @@ -744,6 +773,12 @@ void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct 
> kobject *kobj)
>       if (ret)
>               gt_warn(gt, "failed to create punit_req_freq_mhz sysfs (%pe)", 
> ERR_PTR(ret));
>  
> +     if (intel_uc_uses_guc_slpc(&gt->uc)) {
> +             ret = sysfs_create_file(kobj, &attr_slpc_ignore_eff_freq.attr);
> +             if (ret)
> +                     gt_warn(gt, "failed to create ignore_eff_freq sysfs 
> (%pe)", ERR_PTR(ret));
> +     }
> +
>       if (i915_mmio_reg_valid(intel_gt_perf_limit_reasons_reg(gt))) {
>               ret = sysfs_create_files(kobj, throttle_reason_attrs);
>               if (ret)
> 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 026d73855f36..56dbba1ef668 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -277,6 +277,7 @@ int intel_guc_slpc_init(struct intel_guc_slpc *slpc)
>  
>       slpc->max_freq_softlimit = 0;
>       slpc->min_freq_softlimit = 0;
> +     slpc->ignore_eff_freq = false;
>       slpc->min_is_rpmax = false;
>  
>       slpc->boost_freq = 0;
> @@ -457,6 +458,29 @@ int intel_guc_slpc_get_max_freq(struct intel_guc_slpc 
> *slpc, u32 *val)
>       return ret;
>  }
>  
> +int intel_guc_slpc_set_ignore_eff_freq(struct intel_guc_slpc *slpc, bool val)
> +{
> +     struct drm_i915_private *i915 = slpc_to_i915(slpc);
> +     intel_wakeref_t wakeref;
> +     int ret;
> +
> +     mutex_lock(&slpc->lock);
> +     wakeref = intel_runtime_pm_get(&i915->runtime_pm);
> +
> +     ret = slpc_set_param(slpc,
> +                          SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY,
> +                          val);
> +     if (ret)
> +             guc_probe_error(slpc_to_guc(slpc), "Failed to set efficient 
> freq(%d): %pe\n",
> +                             val, ERR_PTR(ret));
> +     else
> +             slpc->ignore_eff_freq = val;
> +
> +     intel_runtime_pm_put(&i915->runtime_pm, wakeref);
> +     mutex_unlock(&slpc->lock);
> +     return ret;
> +}
> +
>  /**
>   * intel_guc_slpc_set_min_freq() - Set min frequency limit for SLPC.
>   * @slpc: pointer to intel_guc_slpc.
> @@ -482,16 +506,6 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc 
> *slpc, u32 val)
>       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);
> @@ -499,7 +513,6 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc 
> *slpc, u32 val)
>       if (!ret)
>               slpc->min_freq_softlimit = val;
>  
> -out:
>       intel_runtime_pm_put(&i915->runtime_pm, wakeref);
>       mutex_unlock(&slpc->lock);
>  
> @@ -752,6 +765,9 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
>       /* Set cached media freq ratio mode */
>       intel_guc_slpc_set_media_ratio_mode(slpc, slpc->media_ratio_mode);
>  
> +     /* Set cached value of ignore efficient freq */
> +     intel_guc_slpc_set_ignore_eff_freq(slpc, slpc->ignore_eff_freq);
> +
>       return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h 
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> index 17ed515f6a85..597eb5413ddf 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> @@ -46,5 +46,6 @@ void intel_guc_slpc_boost(struct intel_guc_slpc *slpc);
>  void intel_guc_slpc_dec_waiters(struct intel_guc_slpc *slpc);
>  int intel_guc_slpc_unset_gucrc_mode(struct intel_guc_slpc *slpc);
>  int intel_guc_slpc_override_gucrc_mode(struct intel_guc_slpc *slpc, u32 
> mode);
> +int intel_guc_slpc_set_ignore_eff_freq(struct intel_guc_slpc *slpc, bool 
> val);
>  
>  #endif
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc_types.h 
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc_types.h
> index a6ef53b04e04..a88651331497 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc_types.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc_types.h
> @@ -31,6 +31,7 @@ struct intel_guc_slpc {
>       /* frequency softlimits */
>       u32 min_freq_softlimit;
>       u32 max_freq_softlimit;
> +     bool ignore_eff_freq;
>  
>       /* cached media ratio mode */
>       u32 media_ratio_mode;
> -- 
> 2.38.1

Reply via email to