Quoting Mika Kuoppala (2019-01-02 13:19:52)
> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
