From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 06/39] drm/i915: Always try to reset the GPU on takeover
Date: Wed, 02 Jan 2019 15:19:52 +0200 [thread overview]
Message-ID: <87muoj42o7.fsf@gaia.fi.intel.com> (raw)
In-Reply-To: <20190102094139.24014-6-chris@chris-wilson.co.uk>
Chris Wilson <chris@chris-wilson.co.uk> writes:
> When we first introduced the reset to sanitize the GPU on taking over
> from the BIOS and before returning control to third parties (the BIOS!),
> we restricted it to only systems utilizing HW contexts as we were
> uncertain of how stable our reset mechanism truly was. We now have
> reasonable coverage across all machines that expose a GPU reset method,
> and so we should be safe to sanitize the GPU state everywhere.
>
> v2: We _have_ to skip the reset if it would clobber the display.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 2 +-
> drivers/gpu/drm/i915/i915_gem.c | 11 ++---------
> drivers/gpu/drm/i915/i915_pci.c | 5 +++++
> drivers/gpu/drm/i915/intel_device_info.h | 1 +
> drivers/gpu/drm/i915/intel_display.c | 4 ++--
> drivers/gpu/drm/i915/intel_engine_cs.c | 14 +++++++++++++-
> drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +-
> drivers/gpu/drm/i915/selftests/i915_gem.c | 2 +-
> 8 files changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index dcb935338c63..a96169acdb07 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2174,7 +2174,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
>
> intel_power_domains_resume(dev_priv);
>
> - intel_engines_sanitize(dev_priv);
> + intel_engines_sanitize(dev_priv, true);
>
> enable_rpm_wakeref_asserts(dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index e46a25747ab7..0ebde13620cb 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3432,8 +3432,7 @@ bool i915_gem_unset_wedged(struct drm_i915_private *i915)
> i915_retire_requests(i915);
> GEM_BUG_ON(i915->gt.active_requests);
>
> - if (!intel_gpu_reset(i915, ALL_ENGINES))
> - intel_engines_sanitize(i915);
> + intel_engines_sanitize(i915, false);
>
> /*
> * Undo nop_submit_request. We prevent all new i915 requests from
> @@ -5037,8 +5036,6 @@ void __i915_gem_object_release_unless_active(struct drm_i915_gem_object *obj)
>
> void i915_gem_sanitize(struct drm_i915_private *i915)
> {
> - int err;
> -
> GEM_TRACE("\n");
>
> mutex_lock(&i915->drm.struct_mutex);
> @@ -5063,11 +5060,7 @@ void i915_gem_sanitize(struct drm_i915_private *i915)
> * it may impact the display and we are uncertain about the stability
> * of the reset, so this could be applied to even earlier gen.
> */
> - err = -ENODEV;
> - if (INTEL_GEN(i915) >= 5 && intel_has_gpu_reset(i915))
> - err = WARN_ON(intel_gpu_reset(i915, ALL_ENGINES));
> - if (!err)
> - intel_engines_sanitize(i915);
> + intel_engines_sanitize(i915, false);
>
> intel_uncore_forcewake_put(i915, FORCEWAKE_ALL);
> intel_runtime_pm_put(i915);
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 0d342f2b44a5..dd4aff2b256e 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -82,6 +82,7 @@
> .display.has_overlay = 1, \
> .display.overlay_needs_physical = 1, \
> .display.has_gmch_display = 1, \
> + .gpu_reset_clobbers_display = true, \
> .hws_needs_physical = 1, \
> .unfenced_needs_alignment = 1, \
> .ring_mask = RENDER_RING, \
> @@ -122,6 +123,7 @@ static const struct intel_device_info intel_i865g_info = {
> GEN(3), \
> .num_pipes = 2, \
> .display.has_gmch_display = 1, \
> + .gpu_reset_clobbers_display = true, \
> .ring_mask = RENDER_RING, \
> .has_snoop = true, \
> .has_coherent_ggtt = true, \
> @@ -198,6 +200,7 @@ static const struct intel_device_info intel_pineview_info = {
> .num_pipes = 2, \
> .display.has_hotplug = 1, \
> .display.has_gmch_display = 1, \
> + .gpu_reset_clobbers_display = true, \
> .ring_mask = RENDER_RING, \
> .has_snoop = true, \
> .has_coherent_ggtt = true, \
> @@ -228,6 +231,7 @@ static const struct intel_device_info intel_g45_info = {
> GEN4_FEATURES,
> PLATFORM(INTEL_G45),
> .ring_mask = RENDER_RING | BSD_RING,
> + .gpu_reset_clobbers_display = false,
> };
>
> static const struct intel_device_info intel_gm45_info = {
> @@ -237,6 +241,7 @@ static const struct intel_device_info intel_gm45_info = {
> .display.has_fbc = 1,
> .display.supports_tv = 1,
> .ring_mask = RENDER_RING | BSD_RING,
> + .gpu_reset_clobbers_display = false,
Why not explicitly set this on rest of gen >= 5?
-Mika
> };
>
> #define GEN5_FEATURES \
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index dd34f974a857..8fd683497956 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -89,6 +89,7 @@ enum intel_ppgtt {
> func(is_alpha_support); \
> /* Keep has_* in alphabetical order */ \
> func(has_64bit_reloc); \
> + func(gpu_reset_clobbers_display); \
> func(has_reset_engine); \
> func(has_fpga_dbg); \
> func(has_guc); \
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ab3b4d02d499..1679c04280eb 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3746,8 +3746,8 @@ __intel_display_resume(struct drm_device *dev,
>
> static bool gpu_reset_clobbers_display(struct drm_i915_private *dev_priv)
> {
> - return intel_has_gpu_reset(dev_priv) &&
> - INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv);
> + return (INTEL_INFO(dev_priv)->gpu_reset_clobbers_display &&
> + intel_has_gpu_reset(dev_priv));
> }
>
> void intel_prepare_reset(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 189a934a63e9..eefa55000818 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -1043,22 +1043,34 @@ void intel_engines_reset_default_submission(struct drm_i915_private *i915)
> engine->set_default_submission(engine);
> }
>
> +static bool reset_engines(struct drm_i915_private *i915)
> +{
> + if (INTEL_INFO(i915)->gpu_reset_clobbers_display)
> + return false;
> +
> + return intel_gpu_reset(i915, ALL_ENGINES) == 0;
> +}
> +
> /**
> * intel_engines_sanitize: called after the GPU has lost power
> * @i915: the i915 device
> + * @force: ignore a failed reset and sanitize engine state anyway
> *
> * Anytime we reset the GPU, either with an explicit GPU reset or through a
> * PCI power cycle, the GPU loses state and we must reset our state tracking
> * to match. Note that calling intel_engines_sanitize() if the GPU has not
> * been reset results in much confusion!
> */
> -void intel_engines_sanitize(struct drm_i915_private *i915)
> +void intel_engines_sanitize(struct drm_i915_private *i915, bool force)
> {
> struct intel_engine_cs *engine;
> enum intel_engine_id id;
>
> GEM_TRACE("\n");
>
> + if (!reset_engines(i915) && !force)
> + return;
> +
> for_each_engine(engine, i915, id) {
> if (engine->reset.reset)
> engine->reset.reset(engine, NULL);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 91ef00d34e91..a62f09ffcfc9 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -1019,7 +1019,7 @@ gen8_emit_ggtt_write(u32 *cs, u32 value, u32 gtt_offset)
> return cs;
> }
>
> -void intel_engines_sanitize(struct drm_i915_private *i915);
> +void intel_engines_sanitize(struct drm_i915_private *i915, bool force);
>
> bool intel_engine_is_idle(struct intel_engine_cs *engine);
> bool intel_engines_are_idle(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c b/drivers/gpu/drm/i915/selftests/i915_gem.c
> index d0aa19d17653..bdcc53e15e75 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
> @@ -121,7 +121,7 @@ static void pm_resume(struct drm_i915_private *i915)
> */
> intel_runtime_pm_get(i915);
>
> - intel_engines_sanitize(i915);
> + intel_engines_sanitize(i915, false);
> i915_gem_sanitize(i915);
> i915_gem_resume(i915);
>
> --
> 2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2019-01-02 13:21 UTC|newest]
Thread overview: 69+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-02 9:41 [PATCH 01/39] drm: Reorder set_property_atomic to avoid returning with an active ww_ctx Chris Wilson
2019-01-02 9:41 ` [PATCH 02/39] drm/i915/selftests: Take a breath during check_partial_mappings() Chris Wilson
2019-01-02 11:07 ` Mika Kuoppala
2019-01-02 11:14 ` Chris Wilson
2019-01-02 9:41 ` [PATCH 03/39] drm/i915: Return immediately if trylock fails for direct-reclaim Chris Wilson
2019-01-02 9:41 ` [PATCH 04/39] drm/i915/userptr: Avoid struct_mutex recursion for mmu_invalidate_range_start Chris Wilson
2019-01-02 9:41 ` [PATCH 05/39] drm/i915/userptr: Probe vma range before gup Chris Wilson
2019-01-02 9:41 ` [PATCH 06/39] drm/i915: Always try to reset the GPU on takeover Chris Wilson
2019-01-02 13:19 ` Mika Kuoppala [this message]
2019-01-02 13:47 ` Chris Wilson
2019-01-02 14:09 ` Mika Kuoppala
2019-01-02 9:41 ` [PATCH 07/39] drm/i915: Report the number of closed vma held by each context in debugfs Chris Wilson
2019-01-04 13:54 ` Mika Kuoppala
2019-01-02 9:41 ` [PATCH 08/39] drm/i915: Track all held rpm wakerefs Chris Wilson
2019-01-02 9:41 ` [PATCH 09/39] drm/i915: Markup paired operations on wakerefs Chris Wilson
2019-01-03 9:38 ` Jani Nikula
2019-01-03 10:28 ` Chris Wilson
2019-01-02 9:41 ` [PATCH 10/39] drm/i915: Syntatic sugar for using intel_runtime_pm Chris Wilson
2019-01-03 9:48 ` Jani Nikula
2019-01-03 10:35 ` Chris Wilson
2019-01-02 9:41 ` [PATCH 11/39] drm/i915: Markup paired operations on display power domains Chris Wilson
2019-01-02 9:41 ` [PATCH 12/39] drm/i915: Track the wakeref used to initialise " Chris Wilson
2019-01-02 9:41 ` [PATCH 13/39] drm/i915: Combined gt.awake/gt.power wakerefs Chris Wilson
2019-01-02 9:41 ` [PATCH 14/39] drm/i915/dp: Markup pps lock power well Chris Wilson
2019-01-02 9:41 ` [PATCH 15/39] drm/i915: Complain if hsw_get_pipe_config acquires the same power well twice Chris Wilson
2019-01-02 9:41 ` [PATCH 16/39] drm/i915: Mark up Ironlake ips with rpm wakerefs Chris Wilson
2019-01-02 9:41 ` [PATCH 17/39] drm/i915: Serialise concurrent calls to i915_gem_set_wedged() Chris Wilson
2019-01-02 9:41 ` [PATCH 18/39] drm/i915: Differentiate between ggtt->mutex and ppgtt->mutex Chris Wilson
2019-01-03 13:29 ` Mika Kuoppala
2019-01-03 13:43 ` Tvrtko Ursulin
2019-01-03 13:55 ` Chris Wilson
2019-01-02 9:41 ` [PATCH 19/39] drm/i915: Pull all the reset functionality together into i915_reset.c Chris Wilson
2019-01-02 9:41 ` [PATCH 20/39] drm/i915: Make all GPU resets atomic Chris Wilson
2019-01-02 9:41 ` [PATCH 21/39] drm/i915/guc: Disable global reset Chris Wilson
2019-01-07 18:31 ` Daniele Ceraolo Spurio
2019-01-07 18:50 ` Chris Wilson
2019-01-07 21:28 ` Daniele Ceraolo Spurio
2019-01-07 21:35 ` Chris Wilson
2019-01-07 21:45 ` Daniele Ceraolo Spurio
2019-01-02 9:41 ` [PATCH 22/39] drm/i915: Remove GPU reset dependence on struct_mutex Chris Wilson
2019-01-02 9:41 ` [PATCH 23/39] drm/i915: Issue engine resets onto idle engines Chris Wilson
2019-01-02 9:41 ` [PATCH 24/39] drm/i915: Stop tracking MRU activity on VMA Chris Wilson
2019-01-02 13:13 ` Tvrtko Ursulin
2019-01-02 13:48 ` Chris Wilson
2019-01-02 9:41 ` [PATCH 25/39] drm/i915: Pull VM lists under the VM mutex Chris Wilson
2019-01-02 13:21 ` Tvrtko Ursulin
2019-01-02 13:28 ` Chris Wilson
2019-01-02 9:41 ` [PATCH 26/39] drm/i915: Consolidate the bound/unbound vma lists into one Chris Wilson
2019-01-02 14:22 ` Tvrtko Ursulin
2019-01-02 9:41 ` [PATCH 27/39] drm/i915: Move vma lookup to its own lock Chris Wilson
2019-01-02 15:07 ` Tvrtko Ursulin
2019-01-02 9:41 ` [PATCH 28/39] drm/i915: Move intel_execlists_show_requests() aside Chris Wilson
2019-01-02 15:15 ` Tvrtko Ursulin
2019-01-02 9:41 ` [PATCH 29/39] drm/i915: Use b->irq_enable() as predicate for mock engine Chris Wilson
2019-01-02 15:21 ` Tvrtko Ursulin
2019-01-04 12:13 ` Chris Wilson
2019-01-04 13:24 ` Tvrtko Ursulin
2019-01-02 9:41 ` [PATCH 30/39] drm/i915/selftests: Allocate mock ring/timeline per context Chris Wilson
2019-01-02 9:41 ` [PATCH 31/39] drm/i915/selftests: Make evict tolerant of foreign objects Chris Wilson
2019-01-02 9:41 ` [PATCH 32/39] drm/i915: Remove the intel_engine_notify tracepoint Chris Wilson
2019-01-02 9:41 ` [PATCH 33/39] drm/i915: Move list of timelines under its own lock Chris Wilson
2019-01-02 9:41 ` [PATCH 34/39] drm/i915: Introduce concept of per-timeline (context) HWSP Chris Wilson
2019-01-02 9:41 ` [PATCH 35/39] drm/i915: Enlarge vma->pin_count Chris Wilson
2019-01-02 9:41 ` [PATCH 36/39] drm/i915: Allocate a status page for each timeline Chris Wilson
2019-01-02 9:41 ` [PATCH 37/39] drm/i915: Track the context's seqno in its own timeline HWSP Chris Wilson
2019-01-03 15:08 ` Chris Wilson
2019-01-02 9:41 ` [PATCH 38/39] drm/i915: Identify active requests Chris Wilson
2019-01-02 9:41 ` [PATCH 39/39] drm/i915: Replace global breadcrumbs with per-context interrupt tracking Chris Wilson
2019-01-02 12:09 ` ✗ Fi.CI.BAT: failure for series starting with [01/39] drm: Reorder set_property_atomic to avoid returning with an active ww_ctx Patchwork
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=87muoj42o7.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.