Op 06-08-15 om 15:12 schreef Daniel Vetter: > On Wed, Aug 05, 2015 at 12:37:10PM +0200, Maarten Lankhorst wrote: >> The rest will be a noop anyway, since without modeset there will be >> no updated dplls and no modeset state to update. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> >> --- >> drivers/gpu/drm/i915/intel_display.c | 30 +++++++----------------------- >> 1 file changed, 7 insertions(+), 23 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 1fd8b7dec7da..aa444cbc2262 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -12181,33 +12181,15 @@ fail: >> return ret; >> } >> >> -static bool intel_crtc_in_use(struct drm_crtc *crtc) >> -{ >> - struct drm_encoder *encoder; >> - struct drm_device *dev = crtc->dev; >> - >> - list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) >> - if (encoder->crtc == crtc) >> - return true; >> - >> - return false; >> -} >> - >> static void >> -intel_modeset_update_state(struct drm_atomic_state *state) >> +intel_modeset_update_crtc_state(struct drm_atomic_state *state) >> { >> struct drm_crtc *crtc; >> struct drm_crtc_state *crtc_state; >> int i; >> >> - intel_shared_dpll_commit(state); > This should be right next to swap_state, and we should probably rename it > to be consistent. After some digging I think this could work if we no longer check pll->config.crtc_mask in intel_disable_shared_dpll. If we check crtc_mask in intel_enable_shared_dpll we can remove the one from disable and prepare, and let the hw checker deal with it. >> - >> - drm_atomic_helper_update_legacy_modeset_state(state->dev, state); > This helper should always work, why try to optimize things? I didn't see the point of going through the connector state, but shrug I guess the extra loops probably don't matter. >> - >> /* Double check state. */ >> for_each_crtc_in_state(state, crtc, crtc_state, i) { >> - WARN_ON(crtc->state->enable != intel_crtc_in_use(crtc)); > I guess this WARN_ON should be moved into the state checker? Or entirely > removed if redundant. It's removed because the atomic core does this for us. >> - >> to_intel_crtc(crtc)->config = to_intel_crtc_state(crtc->state); >> >> /* Update hwmode for vblank functions */ >> @@ -13110,12 +13092,14 @@ static int intel_atomic_commit(struct drm_device *dev, >> >> /* Only after disabling all output pipelines that will be changed can we >> * update the the output configuration. */ >> - intel_modeset_update_state(state); >> + intel_modeset_update_crtc_state(state); >> >> - /* The state has been swaped above, so state actually contains the >> - * old state now. */ >> - if (any_ms) >> + if (any_ms) { >> + intel_shared_dpll_commit(state); >> + >> + drm_atomic_helper_update_legacy_modeset_state(state->dev, state); >> modeset_update_crtc_power_domains(state); >> + } >> >> /* Now enable the clocks, plane, pipe, and connectors that we set up. */ >> for_each_crtc_in_state(state, crtc, crtc_state, i) { >> -- >> 2.1.0 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx