It seems appropriate for me. Although it will now run only if (intel_state->modeset) instead in all commit_tail it is apparently in sync with original motivation. And also be able to catch all unclaimed reg access anyways. So, Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> On Wed, Jun 7, 2017 at 1:04 PM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > Quoting Chris Wilson (2017-05-04 12:55:08) >> Replace the large comment about requiring the powerwell for >> intel_uncore_arm_unclaimed_mmio_detection() by moving the arming of the >> mmio error detection into the powerwell held for modesetting. Thereby >> also accomplishing the goal of only arming the mmio detection after a >> full modeset. >> >> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> >> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx> >> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Ping? > >> drivers/gpu/drm/i915/intel_display.c | 23 +++++++++-------------- >> 1 file changed, 9 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 85b9e2f521a0..14e12e46eda5 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -12912,8 +12912,16 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) >> >> drm_atomic_helper_commit_hw_done(state); >> >> - if (intel_state->modeset) >> + if (intel_state->modeset) { >> + /* As one of the primary mmio accessors, KMS has a high >> + * likelihood of triggering bugs in unclaimed access. After we >> + * finish modesetting, see if an error has been flagged, and if >> + * so enable debugging for the next modeset - and hope we catch >> + * the culprit. >> + */ >> + intel_uncore_arm_unclaimed_mmio_detection(dev_priv); >> intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET); >> + } >> >> mutex_lock(&dev->struct_mutex); >> drm_atomic_helper_cleanup_planes(dev, state); >> @@ -12923,19 +12931,6 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) >> >> drm_atomic_state_put(state); >> >> - /* As one of the primary mmio accessors, KMS has a high likelihood >> - * of triggering bugs in unclaimed access. After we finish >> - * modesetting, see if an error has been flagged, and if so >> - * enable debugging for the next modeset - and hope we catch >> - * the culprit. >> - * >> - * XXX note that we assume display power is on at this point. >> - * This might hold true now but we need to add pm helper to check >> - * unclaimed only when the hardware is on, as atomic commits >> - * can happen also when the device is completely off. >> - */ >> - intel_uncore_arm_unclaimed_mmio_detection(dev_priv); >> - >> intel_atomic_helper_free_state(dev_priv); >> } >> >> -- >> 2.11.0 >> > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx