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@xxxxxxxxxxxxxxxxxx>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
> > ---
> > 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
