| Message ID | 20191120173642.70109-1-stuart.summers@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Extend reset modparam to domain resets | expand |
Quoting Stuart Summers (2019-11-20 17:36:42) > In the event a platform does not properly implement reset, > do not go through reset flows for engine domains to avoid > an unlikely situation where writes are accepted but register > values are never cleared, as this can result in GPU wedges > in these cases. Is there no way to store the HW details in the uncore or gt? Even if that is a big old parse modparams at probe, the flow here should follow HW. -Chris
Quoting Stuart Summers (2019-11-20 17:36:42) > In the event a platform does not properly implement reset, > do not go through reset flows for engine domains to avoid > an unlikely situation where writes are accepted but register > values are never cleared, as this can result in GPU wedges > in these cases. > > Signed-off-by: Stuart Summers <stuart.summers@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_reset.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c > index 0388f9375366..0454e01e063c 100644 > --- a/drivers/gpu/drm/i915/gt/intel_reset.c > +++ b/drivers/gpu/drm/i915/gt/intel_reset.c > @@ -270,6 +270,12 @@ static int gen6_hw_domain_reset(struct intel_gt *gt, u32 hw_domain_mask) > struct intel_uncore *uncore = gt->uncore; > int err; > > + if (!i915_modparams.reset) { > + DRM_DEBUG_DRIVER("Skipping 0x%08x engines reset\n", > + hw_domain_mask); > + return 0; And it should be -ENODEV. Factor that into your plans :) -Chris
On Wed, 2019-11-20 at 17:45 +0000, Chris Wilson wrote: > Quoting Stuart Summers (2019-11-20 17:36:42) > > In the event a platform does not properly implement reset, > > do not go through reset flows for engine domains to avoid > > an unlikely situation where writes are accepted but register > > values are never cleared, as this can result in GPU wedges > > in these cases. > > Is there no way to store the HW details in the uncore or gt? > Even if that is a big old parse modparams at probe, the flow here > should > follow HW. Hm.. that does make sense. That might take a rework of some of this reset logic though. Is that the route we want to go? Thanks, Stuart > -Chris
On Wed, 2019-11-20 at 17:45 +0000, Chris Wilson wrote: > Quoting Stuart Summers (2019-11-20 17:36:42) > > In the event a platform does not properly implement reset, > > do not go through reset flows for engine domains to avoid > > an unlikely situation where writes are accepted but register > > values are never cleared, as this can result in GPU wedges > > in these cases. > > > > Signed-off-by: Stuart Summers <stuart.summers@intel.com> > > --- > > drivers/gpu/drm/i915/gt/intel_reset.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c > > b/drivers/gpu/drm/i915/gt/intel_reset.c > > index 0388f9375366..0454e01e063c 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_reset.c > > +++ b/drivers/gpu/drm/i915/gt/intel_reset.c > > @@ -270,6 +270,12 @@ static int gen6_hw_domain_reset(struct > > intel_gt *gt, u32 hw_domain_mask) > > struct intel_uncore *uncore = gt->uncore; > > int err; > > > > + if (!i915_modparams.reset) { > > + DRM_DEBUG_DRIVER("Skipping 0x%08x engines reset\n", > > + hw_domain_mask); > > + return 0; > > And it should be -ENODEV. Factor that into your plans :) Heh, again, might require some rework although I can see the intention here. Thanks, Stuart > -Chris
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c index 0388f9375366..0454e01e063c 100644 --- a/drivers/gpu/drm/i915/gt/intel_reset.c +++ b/drivers/gpu/drm/i915/gt/intel_reset.c @@ -270,6 +270,12 @@ static int gen6_hw_domain_reset(struct intel_gt *gt, u32 hw_domain_mask) struct intel_uncore *uncore = gt->uncore; int err; + if (!i915_modparams.reset) { + DRM_DEBUG_DRIVER("Skipping 0x%08x engines reset\n", + hw_domain_mask); + return 0; + } + /* * GEN6_GDRST is not in the gt power well, no need to check * for fifo space for the write or forcewake the chip for @@ -494,6 +500,13 @@ static int gen8_engine_reset_prepare(struct intel_engine_cs *engine) return 0; } + if (!i915_modparams.reset) { + DRM_DEBUG_DRIVER("Skipping %s reset request {request: %08x, RESET_CTL: %08x}\n", + engine->name, request, + intel_uncore_read_fw(uncore, reg)); + return 0; + } + intel_uncore_write_fw(uncore, reg, _MASKED_BIT_ENABLE(request)); ret = __intel_wait_for_register_fw(uncore, reg, mask, ack, 700, 0, NULL);
In the event a platform does not properly implement reset, do not go through reset flows for engine domains to avoid an unlikely situation where writes are accepted but register values are never cleared, as this can result in GPU wedges in these cases. Signed-off-by: Stuart Summers <stuart.summers@intel.com> --- drivers/gpu/drm/i915/gt/intel_reset.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)