Quoting Mika Kuoppala (2019-01-02 13:19:52) > Chris Wilson <ch...@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 <ch...@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?
This was to override the .gpu_reset_clobbers_display = true pulled in from GEN4_FEATURES. Despite appearances to the alternative, we are setting GEN2_FEATURES, GEN3_FEATURES and GEN4_FEATURES. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx