| Message ID | 1436252911-5703-15-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 07, 2015 at 09:08:25AM +0200, Maarten Lankhorst wrote: > Calculate all state using a normal transition, but afterwards fudge > crtc->state->active back to its old value. This should still allow > state restore in setup_hw_state to work properly. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90396 > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> What we could do here (just as an experiment really, I haven't thought through the implications for locking) is duplicate the state here _before_ we suspend everything. Then on force_restore we just use the saved state from here for resuming. Would get rid of the failures on resume. Otoh we need that code for the lid notifier still, so probably meh. -Daniel > --- > drivers/gpu/drm/i915/intel_display.c | 52 +++++++++++++++++++++++++++++++++--- > drivers/gpu/drm/i915/intel_drv.h | 2 +- > 2 files changed, 50 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index f7b1fc28142c..9c461ea84d77 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -6219,12 +6219,58 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc) > * turn all crtc's off, but do not adjust state > * This has to be paired with a call to intel_modeset_setup_hw_state. > */ > -void intel_display_suspend(struct drm_device *dev) > +int intel_display_suspend(struct drm_device *dev) > { > + struct drm_mode_config *config = &dev->mode_config; > + struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx; > + struct drm_atomic_state *state; > struct drm_crtc *crtc; > + unsigned crtc_mask = 0; > + int ret = 0; > + > + if (WARN_ON(!ctx)) > + return 0; > + > + lockdep_assert_held(&ctx->ww_ctx); > + state = drm_atomic_state_alloc(dev); > + if (WARN_ON(!state)) > + return -ENOMEM; > + > + state->acquire_ctx = ctx; > + state->allow_modeset = true; > + > + for_each_crtc(dev, crtc) { > + struct drm_crtc_state *crtc_state = > + drm_atomic_get_crtc_state(state, crtc); > > - for_each_crtc(dev, crtc) > - intel_crtc_disable_noatomic(crtc); > + ret = PTR_ERR_OR_ZERO(crtc_state); > + if (ret) > + goto free; > + > + if (!crtc_state->active) > + continue; > + > + crtc_state->active = false; > + crtc_mask |= 1 << drm_crtc_index(crtc); > + } > + > + if (crtc_mask) { > + ret = intel_set_mode(state); > + > + if (!ret) { > + for_each_crtc(dev, crtc) > + if (crtc_mask & (1 << drm_crtc_index(crtc))) > + crtc->state->active = true; > + > + return ret; > + } > + } > + > +free: > + if (ret) > + DRM_ERROR("Suspending crtc's failed with %i\n", ret); > + drm_atomic_state_free(state); > + return ret; > } > > /* Master function to enable/disable CRTC and corresponding power wells */ > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 525b4137521c..66e2016e0a96 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -992,7 +992,7 @@ int intel_pch_rawclk(struct drm_device *dev); > void intel_mark_busy(struct drm_device *dev); > void intel_mark_idle(struct drm_device *dev); > void intel_crtc_restore_mode(struct drm_crtc *crtc); > -void intel_display_suspend(struct drm_device *dev); > +int intel_display_suspend(struct drm_device *dev); > int intel_crtc_control(struct drm_crtc *crtc, bool enable); > void intel_crtc_update_dpms(struct drm_crtc *crtc); > void intel_encoder_destroy(struct drm_encoder *encoder); > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Op 07-07-15 om 11:48 schreef Daniel Vetter: > On Tue, Jul 07, 2015 at 09:08:25AM +0200, Maarten Lankhorst wrote: >> Calculate all state using a normal transition, but afterwards fudge >> crtc->state->active back to its old value. This should still allow >> state restore in setup_hw_state to work properly. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90396 >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > What we could do here (just as an experiment really, I haven't thought > through the implications for locking) is duplicate the state here _before_ > we suspend everything. Then on force_restore we just use the saved state > from here for resuming. Would get rid of the failures on resume. > > Otoh we need that code for the lid notifier still, so probably meh. > Or make a intel_duplicate_state helper and then swap the sw state back. I think that might be overkill and just patching up crtc_state->active afterwards is fine. The restore on resume performs a full recheck anyway. ~Maarten
On Tue, Jul 07, 2015 at 12:50:39PM +0200, Maarten Lankhorst wrote: > Op 07-07-15 om 11:48 schreef Daniel Vetter: > > On Tue, Jul 07, 2015 at 09:08:25AM +0200, Maarten Lankhorst wrote: > >> Calculate all state using a normal transition, but afterwards fudge > >> crtc->state->active back to its old value. This should still allow > >> state restore in setup_hw_state to work properly. > >> > >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90396 > >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > What we could do here (just as an experiment really, I haven't thought > > through the implications for locking) is duplicate the state here _before_ > > we suspend everything. Then on force_restore we just use the saved state > > from here for resuming. Would get rid of the failures on resume. > > > > Otoh we need that code for the lid notifier still, so probably meh. > > > Or make a intel_duplicate_state helper and then swap the sw state back. > I think that might be overkill and just patching up crtc_state->active afterwards > is fine. The restore on resume performs a full recheck anyway. Yeah was really just an idea, current patch looks fine. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f7b1fc28142c..9c461ea84d77 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6219,12 +6219,58 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc) * turn all crtc's off, but do not adjust state * This has to be paired with a call to intel_modeset_setup_hw_state. */ -void intel_display_suspend(struct drm_device *dev) +int intel_display_suspend(struct drm_device *dev) { + struct drm_mode_config *config = &dev->mode_config; + struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx; + struct drm_atomic_state *state; struct drm_crtc *crtc; + unsigned crtc_mask = 0; + int ret = 0; + + if (WARN_ON(!ctx)) + return 0; + + lockdep_assert_held(&ctx->ww_ctx); + state = drm_atomic_state_alloc(dev); + if (WARN_ON(!state)) + return -ENOMEM; + + state->acquire_ctx = ctx; + state->allow_modeset = true; + + for_each_crtc(dev, crtc) { + struct drm_crtc_state *crtc_state = + drm_atomic_get_crtc_state(state, crtc); - for_each_crtc(dev, crtc) - intel_crtc_disable_noatomic(crtc); + ret = PTR_ERR_OR_ZERO(crtc_state); + if (ret) + goto free; + + if (!crtc_state->active) + continue; + + crtc_state->active = false; + crtc_mask |= 1 << drm_crtc_index(crtc); + } + + if (crtc_mask) { + ret = intel_set_mode(state); + + if (!ret) { + for_each_crtc(dev, crtc) + if (crtc_mask & (1 << drm_crtc_index(crtc))) + crtc->state->active = true; + + return ret; + } + } + +free: + if (ret) + DRM_ERROR("Suspending crtc's failed with %i\n", ret); + drm_atomic_state_free(state); + return ret; } /* Master function to enable/disable CRTC and corresponding power wells */ diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 525b4137521c..66e2016e0a96 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -992,7 +992,7 @@ int intel_pch_rawclk(struct drm_device *dev); void intel_mark_busy(struct drm_device *dev); void intel_mark_idle(struct drm_device *dev); void intel_crtc_restore_mode(struct drm_crtc *crtc); -void intel_display_suspend(struct drm_device *dev); +int intel_display_suspend(struct drm_device *dev); int intel_crtc_control(struct drm_crtc *crtc, bool enable); void intel_crtc_update_dpms(struct drm_crtc *crtc); void intel_encoder_destroy(struct drm_encoder *encoder);
Calculate all state using a normal transition, but afterwards fudge crtc->state->active back to its old value. This should still allow state restore in setup_hw_state to work properly. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90396 Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 52 +++++++++++++++++++++++++++++++++--- drivers/gpu/drm/i915/intel_drv.h | 2 +- 2 files changed, 50 insertions(+), 4 deletions(-)