Chris Wilson <[email protected]> writes:

> 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 <[email protected]>
> Cc: Tvrtko Ursulin <[email protected]>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_shrinker.c     | 16 ++--------------
>  .../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, 22 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c 
> b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> index 5ab7df53c2a0..edd21d14e64f 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> @@ -459,13 +459,7 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, 
> unsigned long event, void *ptr
>       return NOTIFY_DONE;
>  }
>  
> -/**
> - * i915_gem_shrinker_register - 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;
> @@ -480,13 +474,7 @@ void i915_gem_shrinker_register(struct drm_i915_private 
> *i915)
>       WARN_ON(register_vmap_purge_notifier(&i915->mm.vmap_notifier));
>  }
>  
> -/**
> - * i915_gem_shrinker_unregister - Unregisters the i915 shrinker
> - * @i915: i915 device
> - *
> - * 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 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;
>  
>       /*
>        * Make sure no request can slip through without getting completed by
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index d348d22bf0cf..77dc66d6e88f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1719,7 +1719,7 @@ static void i915_driver_register(struct 
> drm_i915_private *dev_priv)
>  {
>       struct drm_device *dev = &dev_priv->drm;
>  
> -     i915_gem_shrinker_register(dev_priv);
> +     i915_gem_driver_register(dev_priv);
>       i915_pmu_register(dev_priv);
>  
>       /*
> @@ -1799,7 +1799,7 @@ static void i915_driver_unregister(struct 
> drm_i915_private *dev_priv)
>       i915_teardown_sysfs(dev_priv);
>       drm_dev_unplug(&dev_priv->drm);
>  
> -     i915_gem_shrinker_unregister(dev_priv);
> +     i915_gem_driver_unregister(dev_priv);
>  }
>  
>  static void i915_welcome_messages(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4fabf1a3df68..c241aae07b3e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2488,6 +2488,8 @@ static inline u32 i915_reset_engine_count(struct 
> i915_gpu_error *error,
>  void i915_gem_init_mmio(struct drm_i915_private *i915);
>  int __must_check i915_gem_init(struct drm_i915_private *dev_priv);
>  int __must_check i915_gem_init_hw(struct drm_i915_private *dev_priv);
> +void i915_gem_driver_register(struct drm_i915_private *i915);
> +void i915_gem_driver_unregister(struct drm_i915_private *i915);
>  void i915_gem_driver_remove(struct drm_i915_private *dev_priv);
>  void i915_gem_driver_release(struct drm_i915_private *dev_priv);
>  int i915_gem_wait_for_idle(struct drm_i915_private *dev_priv,
> @@ -2587,8 +2589,8 @@ unsigned long i915_gem_shrink(struct drm_i915_private 
> *i915,
>  #define I915_SHRINK_WRITEBACK        BIT(4)
>  
>  unsigned long i915_gem_shrink_all(struct drm_i915_private *i915);
> -void i915_gem_shrinker_register(struct drm_i915_private *i915);
> -void i915_gem_shrinker_unregister(struct drm_i915_private *i915);
> +void i915_gem_driver_register__shrinker(struct drm_i915_private *i915);
> +void i915_gem_driver_unregister__shrinker(struct drm_i915_private *i915);
>  void i915_gem_shrinker_taints_mutex(struct drm_i915_private *i915,
>                                   struct mutex *mutex);
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index eb34f3e5a74d..5ab1ddfef23c 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1249,8 +1249,6 @@ int i915_gem_init_hw(struct drm_i915_private *i915)
>  
>       intel_mocs_init(gt);
>  
> -     intel_engines_set_scheduler_caps(i915);
> -
>  out:
>       intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
>       return ret;
> @@ -1599,6 +1597,17 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
>       return ret;
>  }
>  
> +void i915_gem_driver_register(struct drm_i915_private *i915)
> +{
> +     i915_gem_driver_register__shrinker(i915);
> +     intel_engines_set_scheduler_caps(i915);
> +}
> +
> +void i915_gem_driver_unregister(struct drm_i915_private *i915)
> +{
> +     i915_gem_driver_unregister__shrinker(i915);
> +}
> +
>  void i915_gem_driver_remove(struct drm_i915_private *dev_priv)
>  {
>       GEM_BUG_ON(dev_priv->gt.awake);
> diff --git a/drivers/gpu/drm/i915/i915_request.c 
> b/drivers/gpu/drm/i915/i915_request.c
> index 8ac7d14ec8c9..81094f250bdb 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -1198,7 +1198,6 @@ struct i915_request *__i915_request_commit(struct 
> i915_request *rq)
>        */
>       local_bh_disable();
>       i915_sw_fence_commit(&rq->semaphore);
> -     rcu_read_lock(); /* RCU serialisation for set-wedged protection */

We don't need to protect against attr changes anymore so yes...

>       if (engine->schedule) {
>               struct i915_sched_attr attr = rq->gem_context->sched;
>  
> @@ -1228,7 +1227,6 @@ struct i915_request *__i915_request_commit(struct 
> i915_request *rq)
>  
>               engine->schedule(rq, &attr);

but will now schedule during wedged. Didn't notice anything that
would blowup on reordering but is this intentional?

-Mika

>       }
> -     rcu_read_unlock();
>       i915_sw_fence_commit(&rq->submit);
>       local_bh_enable(); /* Kick the execlists tasklet if just scheduled */
>  
> -- 
> 2.23.0.rc1
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to