| Message ID | 1436797833-11493-17-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jul 13, 2015 at 04:30:29PM +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. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Ok merged up to this patch with the exception of patch 8. I think reworking that logic to use mode->private_flags to make sure we do a modeset when userspace changes the mode, but not earlier is a sound approach which will address all my concerns. -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 599af76d34f6..6e3df10a43b9 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -6224,12 +6224,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 217b773e0673..f4abce103221 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -988,7 +988,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
Hey, Op 14-07-15 om 18:14 schreef Daniel Vetter: > On Mon, Jul 13, 2015 at 04:30:29PM +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. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Ok merged up to this patch with the exception of patch 8. I think > reworking that logic to use mode->private_flags to make sure we do a > modeset when userspace changes the mode, but not earlier is a sound > approach which will address all my concerns. Patch 8 doesn't affect whether a modeset will happen and is safe to merge. It just fills in some more mode flags. What you just described was done in patch 11 you just merged. ;-) ~Maarten
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 599af76d34f6..6e3df10a43b9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6224,12 +6224,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 217b773e0673..f4abce103221 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -988,7 +988,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. 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(-)