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

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-07-29 13:54:27)
> 
> On 26/07/2019 09:45, Chris Wilson wrote:
> > We do not notify userspace when the scheduler capabilities are changed
> > (due to wedging the driver) and as such userspace will expect the caps
> > to be static and unchanging. Make it so, and so we only need to compute
> > our caps once during driver registration.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/gem/i915_gem_shrinker.c       |  6 +++---
> >   drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c |  4 ++--
> >   drivers/gpu/drm/i915/gt/intel_reset.c              |  5 +----
> >   drivers/gpu/drm/i915/i915_drv.c                    |  4 ++--
> >   drivers/gpu/drm/i915/i915_drv.h                    |  6 ++++--
> >   drivers/gpu/drm/i915/i915_gem.c                    | 13 +++++++++++--
> >   drivers/gpu/drm/i915/i915_request.c                |  2 --
> >   7 files changed, 23 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> > index 3f4c6bdcc3c3..b186bb5bfb44 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> > @@ -460,12 +460,12 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
> >   }
> >   
> >   /**
> > - * i915_gem_shrinker_register - Register the i915 shrinker
> > + * i915_gem_driver_register__shrinker - Register the i915 shrinker
> 
> I am not sure of this naming change. If the implementation lived in GEM 
> it would make sense (one more step in GEM driver registration), but 
> since the code lives in i915_gem_shrinker.c then the existing name 
> sounds okay to me.

"driver_register" is the naming convention we now have for this phase.
It operates on the i915_gem "device" as part of the GEM infrastructure.

> 
> >    * @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)
> >   {
> >       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 0cb457538e62..c937fa80cc3e 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;
> 
> Scheduler caps is one thing but engine->schedule seems like a different 
> one. Feels like these should be separate patches and then the second one 
> would need to explain why we don't need engine->schedule to be NULL any 
> more.

Why do you think they are unrelated?
-Chris