Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> 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.
Ok. And the GEN5+ it will initialized to false.
Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
