[Intel-gfx] [PATCH 05/17] drm/i915/gem: Make caps.scheduler static

[Chris Wilson]

Quoting Andi Shyti (2019-08-05 18:08:31)
> Hi Chris,
> 
> >  /**
> > - * i915_gem_shrinker_register - Register the i915 shrinker
> > + * i915_gem_driver_register__shrinker - Register the i915 shrinker
> >   * @i915: i915 device
> >   *
> >   * This function registers and sets up the i915 shrinker and OOM handler.
> >   */
> > -void i915_gem_shrinker_register(struct drm_i915_private *i915)
> > +void i915_gem_driver_register__shrinker(struct drm_i915_private *i915)
> >  {
> >       i915->mm.shrinker.scan_objects = i915_gem_shrinker_scan;
> >       i915->mm.shrinker.count_objects = i915_gem_shrinker_count;
> > @@ -486,7 +486,7 @@ void i915_gem_shrinker_register(struct drm_i915_private *i915)
> >   *
> >   * This function unregisters the i915 shrinker and OOM handler.
> >   */
> > -void i915_gem_shrinker_unregister(struct drm_i915_private *i915)
> > +void i915_gem_driver_unregister__shrinker(struct drm_i915_private *i915)
> 
> what is the reason for the renaming? Can this land in a different
> patch?

Because I'm updating the names to fit the current role (for device
unbinding) and to fit the ongoing GT/GEM split.

> >       WARN_ON(unregister_vmap_purge_notifier(&i915->mm.vmap_notifier));
> >       WARN_ON(unregister_oom_notifier(&i915->mm.oom_notifier));
> > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> > index 01857c12f12f..50aa7e95124d 100644
> > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> > +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> > @@ -382,7 +382,7 @@ static bool assert_mmap_offset(struct drm_i915_private *i915,
> >  
> >  static void disable_retire_worker(struct drm_i915_private *i915)
> >  {
> > -     i915_gem_shrinker_unregister(i915);
> > +     i915_gem_driver_unregister__shrinker(i915);
> >  
> >       intel_gt_pm_get(&i915->gt);
> >  
> > @@ -398,7 +398,7 @@ static void restore_retire_worker(struct drm_i915_private *i915)
> >       igt_flush_test(i915, I915_WAIT_LOCKED);
> >       mutex_unlock(&i915->drm.struct_mutex);
> >  
> > -     i915_gem_shrinker_register(i915);
> > +     i915_gem_driver_register__shrinker(i915);
> >  }
> >  
> >  static void mmap_offset_lock(struct drm_i915_private *i915)
> > diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> > index 98c071fe532b..cdba6cd29327 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> > @@ -757,11 +757,8 @@ static void __intel_gt_set_wedged(struct intel_gt *gt)
> >       if (!INTEL_INFO(gt->i915)->gpu_reset_clobbers_display)
> >               __intel_gt_reset(gt, ALL_ENGINES);
> >  
> > -     for_each_engine(engine, gt->i915, id) {
> > +     for_each_engine(engine, gt->i915, id)
> >               engine->submit_request = nop_submit_request;
> > -             engine->schedule = NULL;
> > -     }
> > -     gt->i915->caps.scheduler = 0;
> 
> Maybe I haven't understood the patch, but is this the only part
> described in the commit log?

Aside from the knock on effects of making it static?

> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 65863e955f40..9b10b879a37f 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -1255,8 +1255,6 @@ int i915_gem_init_hw(struct drm_i915_private *i915)
> >  
> >       intel_mocs_init_l3cc_table(gt);
> >  
> > -     intel_engines_set_scheduler_caps(i915);
> > -
> 
> Is this really necessary necessary in this patch... can this also
> go in a different patch?

No, this is the "Make caps.scheduler static". We need to initialise the
static caps for the user interface only after we finish our probe of the
HW. As such the static caps is a part of driver registration for
userspace.
-Chris