Re: [Intel-gfx] [PATCH 7/8] drm/i915/gt: Expose per-gt RPS defaults in sysfs

2022-05-26 Thread Dixit, Ashutosh
On Tue, 10 May 2022 03:58:13 -0700, Andi Shyti wrote:
>
> Hi Ashutosh,

Hi Andi,

> > > +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);
>
> I guess this is %u.

Fixed in v2.

> > > +}
> > > +
> > > +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
> > > +};
>
> Do you think this in the default group of kobj_gt_type like the
> gt_id?

gt_id I think fits well in the category of a "default" attribute for a
gt. I am not sure if these attributes similarly qualify as "default"
attributes (for the .defaults sysfs), what do you think? That is why I have
followed what is done in the engine sysfs which also does not use default
attributes. But we can revisit based on your comments.

>
> [...]
>
> > > +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;
>
> more of a matter of taste, but this looks natural to me to be in
> rps rather then in the gt.

In v2 I have changed the name of this from 'struct intel_rps_defaults' to
'struct gt_defaults'. So the idea is to have a central place in the gt
where any "module" (RPS, PM, ...) can store their defaults which are then
exposed via sysfs files in gt/gtN/.defaults directory. There are multiple
ways of doing this but this is what I've done in this series, basically to
keep things simple.

Thanks.
--
Ashutosh


Re: [Intel-gfx] [PATCH 7/8] drm/i915/gt: Expose per-gt RPS defaults in sysfs

2022-05-26 Thread Dixit, Ashutosh
On Tue, 10 May 2022 00:53:23 -0700, Tvrtko Ursulin wrote:
>
> On 29/04/2022 20:56, Ashutosh Dixit wrote:
> > Create a gt/gtN/.defaults directory (similar to
> > engine//.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?

This is actually the case at present (with boost also RP0). However, PVC
also sets min to RP0 (instead of RPn).

And then there is a completely different scheme being considered for GuC
where min = max = boost = "-1" where -1 denotes a "default managed by
firmware". For it is not clear why i915 should set these min/max in FW
(which is autonomous and can better manage the freq control algorithm)
especially when these controls are exposed to userspace in sysfs.

This patch was written because user space was asking a control to restore
all RPS parameters to their default values. Then Chris suggested doing the
same for gt sysfs as what we do for the engine sysfs, expose default values
in sysfs and have userspace restore them when needed.

> > Cc: Joonas Lahtinen 
> > Signed-off-by: Ashutosh Dixit 
> > Reviewed-by: Rodrigo Vivi 
> > ---
> >   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?

Child kobject's take a reference on their parents so child kobjects need a
kobject_put before the parent kobject_put to free the memory allocated for
the parent (i.e. doing a kobject_put on the parent will not automatically
free all the child kobjects).

> > 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 
> > #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

Re: [Intel-gfx] [PATCH 7/8] drm/i915/gt: Expose per-gt RPS defaults in sysfs

2022-05-10 Thread Andi Shyti
Hi Ashutosh,

> > +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);

I guess this is %u.

> > +}
> > +
> > +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
> > +};

Do you think this in the default group of kobj_gt_type like the
gt_id?

[...]

> > +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;

more of a matter of taste, but this looks natural to me to be in
rps rather then in the gt.

[...]

Andi


Re: [Intel-gfx] [PATCH 7/8] drm/i915/gt: Expose per-gt RPS defaults in sysfs

2022-05-10 Thread Tvrtko Ursulin



On 29/04/2022 20:56, Ashutosh Dixit wrote:

Create a gt/gtN/.defaults directory (similar to
engine//.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 
Signed-off-by: Ashutosh Dixit 
Reviewed-by: Rodrigo Vivi 
---
  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 
  
  #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_sysf

[Intel-gfx] [PATCH 7/8] drm/i915/gt: Expose per-gt RPS defaults in sysfs

2022-04-29 Thread Ashutosh Dixit
Create a gt/gtN/.defaults directory (similar to
engine//.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

Cc: Joonas Lahtinen 
Signed-off-by: Ashutosh Dixit 
Reviewed-by: Rodrigo Vivi 
---
 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);
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 
 
 #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",

[Intel-gfx] [PATCH 7/8] drm/i915/gt: Expose per-gt RPS defaults in sysfs

2022-04-13 Thread Ashutosh Dixit
Create a gt/gtN/.defaults directory (similar to
engine//.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

Cc: Rodrigo Vivi 
Cc: Andi Shyti 
Cc: Joonas Lahtinen 
Signed-off-by: Ashutosh Dixit 
---
 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 6f1b081ca5b7..7df32fc8b29d 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_gtn);
-}
-
 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_gtn);
+   if (!gt->sysfs_defaults)
+   goto exit_fail;
+
intel_gt_sysfs_pm_init(gt, >->sysfs_gtn);
 
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);
kobject_put(>->sysfs_gtn);
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h 
b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
index a99aa7e8b01a..fb5fd1bdab1f 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 
 
 #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_gtn);
+}
+
 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 2a3398003933..2b9024cf1d78 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",