[Intel-gfx] [PATCH 7/8] drm/i915/gt: Expose per-gt RPS defaults in sysfs
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Tue May 10 07:53:23 UTC 2022
On 29/04/2022 20:56, Ashutosh Dixit wrote:
> Create a gt/gtN/.defaults directory (similar to
> engine/<engine-name>/.defaults) to expose default parameter values for each
> gt in sysfs. Populate the .defaults directory with RPS parameter default
> values in order to allow userspace to revert to default values when needed.
>
> This patch adds the following sysfs files to gt/gtN/.defaults:
> * default_min_freq_mhz
> * default_max_freq_mhz
> * default_boost_freq_mhz
Possibly an uninformed question - max will not be the existing rp0, min
rpN, and boost I don't know?
>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit at intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_gt_sysfs.c | 10 ++--
> drivers/gpu/drm/i915/gt/intel_gt_sysfs.h | 6 +++
> drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 51 +++++++++++++++++++++
> drivers/gpu/drm/i915/gt/intel_gt_types.h | 10 ++++
> drivers/gpu/drm/i915/gt/intel_rps.c | 3 ++
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 17 +++++--
> 6 files changed, 87 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
> index 9e4ebf53379b..d651ccd0ab20 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
> @@ -22,11 +22,6 @@ bool is_object_gt(struct kobject *kobj)
> return !strncmp(kobj->name, "gt", 2);
> }
>
> -static struct intel_gt *kobj_to_gt(struct kobject *kobj)
> -{
> - return container_of(kobj, struct intel_gt, sysfs_gt);
> -}
> -
> struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
> const char *name)
> {
> @@ -101,6 +96,10 @@ void intel_gt_sysfs_register(struct intel_gt *gt)
> gt->i915->sysfs_gt, "gt%d", gt->info.id))
> goto exit_fail;
>
> + gt->sysfs_defaults = kobject_create_and_add(".defaults", >->sysfs_gt);
> + if (!gt->sysfs_defaults)
> + goto exit_fail;
> +
> intel_gt_sysfs_pm_init(gt, >->sysfs_gt);
>
> return;
> @@ -113,5 +112,6 @@ void intel_gt_sysfs_register(struct intel_gt *gt)
>
> void intel_gt_sysfs_unregister(struct intel_gt *gt)
> {
> + kobject_put(gt->sysfs_defaults);
Is this needed - won't below clean it up?
And not sure I am liking the mix of embedded and allocated kobjects..
Why we couldn't have it uniform?
> kobject_put(>->sysfs_gt);
> }
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
> index a99aa7e8b01a..6232923a420d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
> @@ -10,6 +10,7 @@
> #include <linux/kobject.h>
>
> #include "i915_gem.h" /* GEM_BUG_ON() */
> +#include "intel_gt_types.h"
>
> struct intel_gt;
>
> @@ -22,6 +23,11 @@ intel_gt_create_kobj(struct intel_gt *gt,
> struct kobject *dir,
> const char *name);
>
> +static inline struct intel_gt *kobj_to_gt(struct kobject *kobj)
> +{
> + return container_of(kobj, struct intel_gt, sysfs_gt);
> +}
> +
> void intel_gt_sysfs_register(struct intel_gt *gt);
> void intel_gt_sysfs_unregister(struct intel_gt *gt);
> struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
> 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 ab91e9cf9deb..5a191973322e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> @@ -726,6 +726,51 @@ static const struct attribute *media_perf_power_attrs[] = {
> NULL
> };
>
> +static ssize_t
> +default_min_freq_mhz_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> +{
> + struct intel_gt *gt = kobj_to_gt(kobj->parent);
> +
> + return sysfs_emit(buf, "%d\n", gt->rps_defaults.min_freq);
> +}
> +
> +static struct kobj_attribute default_min_freq_mhz =
> +__ATTR(rps_min_freq_mhz, 0444, default_min_freq_mhz_show, NULL);
> +
> +static ssize_t
> +default_max_freq_mhz_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> +{
> + struct intel_gt *gt = kobj_to_gt(kobj->parent);
> +
> + return sysfs_emit(buf, "%d\n", gt->rps_defaults.max_freq);
> +}
> +
> +static struct kobj_attribute default_max_freq_mhz =
> +__ATTR(rps_max_freq_mhz, 0444, default_max_freq_mhz_show, NULL);
> +
> +static ssize_t
> +default_boost_freq_mhz_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> +{
> + struct intel_gt *gt = kobj_to_gt(kobj->parent);
> +
> + return sysfs_emit(buf, "%d\n", gt->rps_defaults.boost_freq);
> +}
> +
> +static struct kobj_attribute default_boost_freq_mhz =
> +__ATTR(rps_boost_freq_mhz, 0444, default_boost_freq_mhz_show, NULL);
> +
> +static const struct attribute * const rps_defaults_attrs[] = {
> + &default_min_freq_mhz.attr,
> + &default_max_freq_mhz.attr,
> + &default_boost_freq_mhz.attr,
> + NULL
> +};
> +
> +static int add_rps_defaults(struct intel_gt *gt)
> +{
> + return sysfs_create_files(gt->sysfs_defaults, rps_defaults_attrs);
> +}
> +
> static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj,
> const struct attribute * const *attrs)
> {
> @@ -775,4 +820,10 @@ void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj)
> "failed to create add gt%u media_perf_power_attrs sysfs (%pe)\n",
> gt->info.id, ERR_PTR(ret));
> }
> +
> + ret = add_rps_defaults(gt);
> + if (ret)
> + drm_warn(>->i915->drm,
> + "failed to add gt%u rps defaults (%pe)\n",
> + gt->info.id, ERR_PTR(ret));
> }
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> index edd7a3cf5f5f..8b696669b846 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> @@ -62,6 +62,12 @@ enum intel_steering_type {
> NUM_STEERING_TYPES
> };
>
> +struct intel_rps_defaults {
> + u32 min_freq;
> + u32 max_freq;
> + u32 boost_freq;
> +};
> +
> enum intel_submission_method {
> INTEL_SUBMISSION_RING,
> INTEL_SUBMISSION_ELSP,
> @@ -227,6 +233,10 @@ struct intel_gt {
>
> /* gt/gtN sysfs */
> struct kobject sysfs_gt;
> +
> + /* sysfs defaults per gt */
> + struct intel_rps_defaults rps_defaults;
> + struct kobject *sysfs_defaults;
> };
>
> enum intel_gt_scratch_field {
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> index 6b68b40ebff0..6f2461e12409 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -1976,7 +1976,9 @@ void intel_rps_init(struct intel_rps *rps)
>
> /* Derive initial user preferences/limits from the hardware limits */
> rps->max_freq_softlimit = rps->max_freq;
> + rps_to_gt(rps)->rps_defaults.max_freq = rps->max_freq_softlimit;
> rps->min_freq_softlimit = rps->min_freq;
> + rps_to_gt(rps)->rps_defaults.min_freq = rps->min_freq_softlimit;
>
> /* After setting max-softlimit, find the overclock max freq */
> if (GRAPHICS_VER(i915) == 6 || IS_IVYBRIDGE(i915) || IS_HASWELL(i915)) {
> @@ -1994,6 +1996,7 @@ void intel_rps_init(struct intel_rps *rps)
>
> /* Finally allow us to boost to max by default */
> rps->boost_freq = rps->max_freq;
> + rps_to_gt(rps)->rps_defaults.boost_freq = rps->boost_freq;
> rps->idle_freq = rps->min_freq;
>
> /* Start in the middle, from here we will autotune based on workload */
> 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 2df31af70d63..cefd864c84eb 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -547,20 +547,24 @@ static int slpc_set_softlimits(struct intel_guc_slpc *slpc)
> * unless they have deviated from defaults, in which case,
> * we retain the values and set min/max accordingly.
> */
> - if (!slpc->max_freq_softlimit)
> + if (!slpc->max_freq_softlimit) {
> slpc->max_freq_softlimit = slpc->rp0_freq;
> - else if (slpc->max_freq_softlimit != slpc->rp0_freq)
> + slpc_to_gt(slpc)->rps_defaults.max_freq = slpc->max_freq_softlimit;
> + } else if (slpc->max_freq_softlimit != slpc->rp0_freq) {
> ret = intel_guc_slpc_set_max_freq(slpc,
> slpc->max_freq_softlimit);
> + }
>
> if (unlikely(ret))
> return ret;
>
> - if (!slpc->min_freq_softlimit)
> + if (!slpc->min_freq_softlimit) {
> slpc->min_freq_softlimit = slpc->min_freq;
> - else if (slpc->min_freq_softlimit != slpc->min_freq)
> + slpc_to_gt(slpc)->rps_defaults.min_freq = slpc->min_freq_softlimit;
> + } else if (slpc->min_freq_softlimit != slpc->min_freq) {
> return intel_guc_slpc_set_min_freq(slpc,
> slpc->min_freq_softlimit);
> + }
>
> return 0;
> }
> @@ -606,8 +610,11 @@ static void slpc_get_rp_values(struct intel_guc_slpc *slpc)
> slpc->rp1_freq = intel_gpu_freq(rps, caps.rp1_freq);
> slpc->min_freq = intel_gpu_freq(rps, caps.min_freq);
>
> - if (!slpc->boost_freq)
> + /* Boost freq is RP0, unless already set */
> + if (!slpc->boost_freq) {
> slpc->boost_freq = slpc->rp0_freq;
> + slpc_to_gt(slpc)->rps_defaults.boost_freq = slpc->boost_freq;
> + }
Not liking that there are two places which set each of the default. Is
it that there are GuC and non-GuC paths which initialize the parent
struct? Is there a way to set the defaults at one common place after
either branch has run?
Regards,
Tvrtko
> }
>
> /*
More information about the Intel-gfx
mailing list