All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 01/22] drm/i915/gem: Make caps.scheduler static
Date: Tue, 06 Aug 2019 13:40:48 +0300	[thread overview]
Message-ID: <87ftmetw3z.fsf@gaia.fi.intel.com> (raw)
In-Reply-To: <20190806090535.14807-1-chris@chris-wilson.co.uk>

Chris Wilson <chris@chris-wilson.co.uk> 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 <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> ---
>  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
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2019-08-06 10:41 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-06  9:05 [PATCH 01/22] drm/i915/gem: Make caps.scheduler static Chris Wilson
2019-08-06  9:05 ` [PATCH 02/22] drm/i915/gt: Move the [class][inst] lookup for engines onto the GT Chris Wilson
2019-08-06  9:05 ` [PATCH 03/22] drm/i915: Defer final intel_wakeref_put to process context Chris Wilson
2019-08-06 16:29   ` Chris Wilson
2019-08-06  9:05 ` [PATCH 04/22] drm/i915/pmu: Use GT parked for estimating RC6 while asleep Chris Wilson
2019-08-06  9:05 ` [PATCH 05/22] drm/i915: Drop the fudge warning on ring restart for ctg/elk Chris Wilson
2019-08-06  9:05 ` [PATCH 06/22] drm/i915/selftests: Pass intel_context to mock_request Chris Wilson
2019-08-06  9:05 ` [PATCH 07/22] drm/i915/gt: Make deferred context allocation explicit Chris Wilson
2019-08-06  9:05 ` [PATCH 08/22] drm/i915: Push the ring creation flags to the backend Chris Wilson
2019-08-06  9:05 ` [PATCH 09/22] drm/i915: Lift timeline into intel_context Chris Wilson
2019-08-06  9:05 ` [PATCH 10/22] drm/i915: Use intel_engine_lookup_user for probing HAS_BSD etc Chris Wilson
2019-08-06  9:05 ` [PATCH 11/22] drm/i915: Isolate i915_getparam_ioctl() Chris Wilson
2019-08-06  9:05 ` [PATCH 12/22] drm/i915: Only include active engines in the capture state Chris Wilson
2019-08-06  9:05 ` [PATCH 13/22] drm/i915: Make debugfs/per_file_stats scale better Chris Wilson
2019-08-06  9:05 ` [PATCH 14/22] drm/i915/gt: Track timeline activeness in enter/exit Chris Wilson
2019-08-06  9:05 ` [PATCH 15/22] drm/i915/gt: Convert timeline tracking to spinlock Chris Wilson
2019-08-06  9:05 ` [PATCH 16/22] drm/i915/gt: Guard timeline pinning with its own mutex Chris Wilson
2019-08-06  9:05 ` [PATCH 17/22] drm/i915: Protect request retirement with timeline->mutex Chris Wilson
2019-08-06  9:05 ` [PATCH 18/22] drm/i915/gt: Mark context->active_count as protected by timeline->mutex Chris Wilson
2019-08-06  9:05 ` [PATCH 19/22] drm/i915: Forgo last_fence active request tracking Chris Wilson
2019-08-06  9:05 ` [PATCH 20/22] drm/i915/overlay: Switch to using i915_active tracking Chris Wilson
2019-08-06  9:05 ` [PATCH 21/22] drm/i915: Extract intel_frontbuffer active tracking Chris Wilson
2019-08-07 14:45   ` kbuild test robot
2019-08-06  9:05 ` [PATCH 22/22] drm/i915: Markup expected timeline locks for i915_active Chris Wilson
2019-08-06 10:40 ` Mika Kuoppala [this message]
2019-08-06 10:53   ` [PATCH 01/22] drm/i915/gem: Make caps.scheduler static Chris Wilson
2019-08-06 11:50     ` Mika Kuoppala
2019-08-06 11:19 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/22] " Patchwork
2019-08-06 11:30 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-08-06 11:43 ` ✓ Fi.CI.BAT: success " Patchwork
2019-08-06 20:14 ` ✗ Fi.CI.IGT: failure " Patchwork
2019-08-06 20:20   ` Chris Wilson
2019-08-06 21:24     ` Chris Wilson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ftmetw3z.fsf@gaia.fi.intel.com \
    --to=mika.kuoppala@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.