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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox