[Intel-gfx] [PATCH 1/2] drm/i915/perf: Register sysctl path globally

[Lucas De Marchi]

On Fri, Dec 13, 2019 at 04:09:20PM +0000, Chris Wilson wrote:
>Quoting Venkata Sandeep Dhanalakota (2019-12-13 15:51:51)
>> We do not require to register the sysctl paths per instance,
>> so making registration global.
>>
>> v2: make sysctl path register and unregister function driver
>>     specific (Tvrtko and Lucas).
>>
>> Cc: Sudeep Dutt <sudeep.dutt at intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Jani Nikula <jani.nikula at intel.com>
>> Signed-off-by: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota at intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_pci.c        |  9 ++++++++-
>>  drivers/gpu/drm/i915/i915_perf.c       | 18 ++++++++++++++----
>>  drivers/gpu/drm/i915/i915_perf.h       |  2 ++
>>  drivers/gpu/drm/i915/i915_perf_types.h |  1 -
>>  4 files changed, 24 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>> index bba6b50e6beb..4b33128070da 100644
>> --- a/drivers/gpu/drm/i915/i915_pci.c
>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>> @@ -30,6 +30,7 @@
>>  #include "display/intel_fbdev.h"
>>
>>  #include "i915_drv.h"
>> +#include "i915_perf.h"
>>  #include "i915_globals.h"
>>  #include "i915_selftest.h"
>>
>> @@ -1051,7 +1052,12 @@ static int __init i915_init(void)
>>                 return 0;
>>         }
>>
>> -       return pci_register_driver(&i915_pci_driver);
>> +       err = pci_register_driver(&i915_pci_driver);
>> +       if (err)
>> +               return err;
>> +
>> +       i915_perf_sysctl_register();
>> +       return 0;
>>  }
>>
>>  static void __exit i915_exit(void)
>> @@ -1059,6 +1065,7 @@ static void __exit i915_exit(void)
>>         if (!i915_pci_driver.driver.owner)
>>                 return;
>>
>> +       i915_perf_sysctl_unregister();
>>         pci_unregister_driver(&i915_pci_driver);
>>         i915_globals_exit();
>>  }
>> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>> index 8d2e37949f46..4abd7623ef2d 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.c
>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>> @@ -387,6 +387,8 @@ struct i915_oa_config_bo {
>>         struct i915_vma *vma;
>>  };
>>
>> +static struct ctl_table_header *sysctl_header;
>> +
>>  static enum hrtimer_restart oa_poll_check_timer_cb(struct hrtimer *hrtimer);
>>
>>  void i915_oa_config_release(struct kref *ref)
>> @@ -4228,7 +4230,7 @@ static struct ctl_table dev_root[] = {
>>  };
>>
>>  /**
>> - * i915_perf_init - initialize i915-perf state on module load
>> + * i915_perf_init - initialize i915-perf state on module bind
>>   * @i915: i915 device instance
>>   *
>>   * Initializes i915-perf state without exposing anything to userspace.
>> @@ -4345,7 +4347,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>>
>>                 oa_sample_rate_hard_limit = 1000 *
>>                         (RUNTIME_INFO(i915)->cs_timestamp_frequency_khz / 2);
>> -               perf->sysctl_header = register_sysctl_table(dev_root);
>>
>>                 mutex_init(&perf->metrics_lock);
>>                 idr_init(&perf->metrics_idr);
>> @@ -4381,6 +4382,17 @@ static int destroy_config(int id, void *p, void *data)
>>         return 0;
>>  }
>>
>> +void i915_perf_sysctl_register(void)
>> +{
>> +       sysctl_header = register_sysctl_table(dev_root);
>> +}
>> +
>> +void i915_perf_sysctl_unregister(void)
>> +{
>> +       if (sysctl_header)
>> +               unregister_sysctl_table(sysctl_header);
>
>unregister_sysctl_table() is NULL-safe.

I had the same comment.

Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

Lucas De Marchi

>
>Other than that,
>Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
>-Chris
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx