| Summary: | [BAT BDW] sporadic WARNING: "Device suspended during HW access" during kms_pipe_crc_basic on pipe B/C | ||
|---|---|---|---|
| Product: | DRI | Reporter: | Daniel Vetter <daniel> |
| Component: | DRM/Intel | Assignee: | Imre Deak <imre.deak> |
| Status: | CLOSED FIXED | QA Contact: | Intel GFX Bugs mailing list <intel-gfx-bugs> |
| Severity: | normal | ||
| Priority: | highest | CC: | intel-gfx-bugs |
| Version: | XOrg git | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
|
Description
Daniel Vetter
2016-01-13 15:11:08 UTC
I'd say it's the same problem as in bug #93698, namely that the modeset path calls intel_pre_plane_update() somewhat unconditionally. A quick glance suggests that we should just: --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13831,9 +13831,8 @@ static int intel_atomic_commit(struct drm_device *dev, if (!needs_modeset(crtc->state)) continue; - intel_pre_plane_update(intel_crtc); - if (crtc_state->active) { + intel_pre_plane_update(intel_crtc); intel_crtc_disable_planes(crtc, crtc_state->plane_mask); dev_priv->display.crtc_disable(crtc); intel_crtc->active = false; @@ -13885,12 +13884,12 @@ static int intel_atomic_commit(struct drm_device *dev, hw_check = true; } - if (!modeset) - intel_pre_plane_update(intel_crtc); - if (crtc->state->active && - (crtc->state->planes_changed || update_pipe)) + (crtc->state->planes_changed || update_pipe)) { + if (!modeset) + intel_pre_plane_update(intel_crtc); drm_atomic_helper_commit_planes_on_crtc(crtc_state); + } if (put_domains) modeset_put_power_domains(dev_priv, put_domains); but it's bit of mess all around so it's hard to be sure without more digging. Regression because it's a new WARNING causing all kinds of CI trouble. Imre, please revert/tune down to debug level asap. (In reply to Daniel Vetter from comment #2) > Regression because it's a new WARNING causing all kinds of CI trouble. > > Imre, please revert/tune down to debug level asap. The assert that checks if the device is suspended during HW has been always around. My wakeref check patches added an additional check that checks for the wakeref reference count, which is a separate thing. (In reply to Imre Deak from comment #3) > (In reply to Daniel Vetter from comment #2) > > Regression because it's a new WARNING causing all kinds of CI trouble. > > > > Imre, please revert/tune down to debug level asap. > > The assert that checks if the device is suspended during HW has been always > around. My wakeref check patches added an additional check that checks for > the wakeref reference count, which is a separate thing. Sorry, mixed it up with another bug. We still need to remedy this somehow, since it's causing piles of noise in CI due to the WARN_ONCE. I can't see this error any more on the CI results, so I assume something has fixed the actual problem. I consider a WARN or at least ERROR message still useful to catch regressions in the future, so until it causes problems for CI again I would leave it as-is and close this bug. Daniel, if you disagree please reopen, but also consider using error instead of debug level. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.