[Intel-gfx] [PATCH 05/27] drm/i915/gem: Make caps.scheduler static
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Mon Jul 29 12:54:27 UTC 2019
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.
> * @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.
Regards,
Tvrtko
>
> /*
> * 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 8c277fbf9663..d960672ae550 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1711,7 +1711,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);
>
> /*
> @@ -1791,7 +1791,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 0e715b1266f0..0e27563030ab 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2479,6 +2479,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,
> @@ -2578,8 +2580,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 01dd0d1d9bf6..a84052f247f2 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1254,8 +1254,6 @@ int i915_gem_init_hw(struct drm_i915_private *i915)
>
> intel_mocs_init_l3cc_table(gt);
>
> - intel_engines_set_scheduler_caps(i915);
> -
> out:
> intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
> return ret;
> @@ -1596,6 +1594,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 */
> 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);
> }
> - rcu_read_unlock();
> i915_sw_fence_commit(&rq->submit);
> local_bh_enable(); /* Kick the execlists tasklet if just scheduled */
>
>
More information about the Intel-gfx
mailing list