From 8c7b5ccb729870e606321b3703e2c2e698c49a95 Mon Sep 17 00:00:00 2001 From: Ander Conselvan de Oliveira Date: Tue, 21 Apr 2015 17:13:19 +0300 Subject: [PATCH] drm/i915: Use atomic helpers for computing changed flags Replace the drivers own logic for computing mode_changed, active_changed and planes_changed flags with the check_modeset() atomic helper. Since that function needs to compare the crtc's new mode with the current, this patch also moves the set up of crtc_state->mode earlier in the call chain. Note that for the call to check_plane() to work properly, we need to check new plane state against new crtc state. But since we still use the plane update helper, which doesn't have a full atomic state, we need to hack around that in intel_plane_atomic_check(). Signed-off-by: Ander Conselvan de Oliveira Reviewed-by: Maarten Lankhorst Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_atomic_plane.c | 16 +- drivers/gpu/drm/i915/intel_display.c | 197 +++++----------------- 2 files changed, 52 insertions(+), 161 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index 77462e1360bc..86ba4b2c3a65 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -111,6 +111,7 @@ static int intel_plane_atomic_check(struct drm_plane *plane, { struct drm_crtc *crtc = state->crtc; struct intel_crtc *intel_crtc; + struct intel_crtc_state *crtc_state; struct intel_plane *intel_plane = to_intel_plane(plane); struct intel_plane_state *intel_state = to_intel_plane_state(state); @@ -126,6 +127,17 @@ static int intel_plane_atomic_check(struct drm_plane *plane, if (!crtc) return 0; + /* FIXME: temporary hack necessary while we still use the plane update + * helper. */ + if (state->state) { + crtc_state = + intel_atomic_get_crtc_state(state->state, intel_crtc); + if (IS_ERR(crtc_state)) + return PTR_ERR(crtc_state); + } else { + crtc_state = intel_crtc->config; + } + /* * The original src/dest coordinates are stored in state->base, but * we want to keep another copy internal to our driver that we can @@ -144,9 +156,9 @@ static int intel_plane_atomic_check(struct drm_plane *plane, intel_state->clip.x1 = 0; intel_state->clip.y1 = 0; intel_state->clip.x2 = - intel_crtc->active ? intel_crtc->config->pipe_src_w : 0; + crtc_state->base.active ? crtc_state->pipe_src_w : 0; intel_state->clip.y2 = - intel_crtc->active ? intel_crtc->config->pipe_src_h : 0; + crtc_state->base.active ? crtc_state->pipe_src_h : 0; /* * Disabling a plane is always okay; we just need to update diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 44b3b5a7ad34..db450772b4a5 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -82,7 +82,7 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc, static void ironlake_pch_clock_get(struct intel_crtc *crtc, struct intel_crtc_state *pipe_config); -static int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode, +static int intel_set_mode(struct drm_crtc *crtc, struct drm_atomic_state *state); static int intel_framebuffer_init(struct drm_device *dev, struct intel_framebuffer *ifb, @@ -9892,7 +9892,9 @@ retry: if (ret) goto fail; - if (intel_set_mode(crtc, mode, state)) { + drm_mode_copy(&crtc_state->base.mode, mode); + + if (intel_set_mode(crtc, state)) { DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n"); if (old->release_fb) old->release_fb->funcs->destroy(old->release_fb); @@ -9966,7 +9968,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector, if (ret) goto fail; - intel_set_mode(crtc, NULL, state); + intel_set_mode(crtc, state); drm_atomic_state_free(state); @@ -11476,7 +11478,6 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state) static int intel_modeset_pipe_config(struct drm_crtc *crtc, - struct drm_display_mode *mode, struct drm_atomic_state *state, struct intel_crtc_state *pipe_config) { @@ -11499,10 +11500,6 @@ intel_modeset_pipe_config(struct drm_crtc *crtc, clear_intel_crtc_state(pipe_config); - pipe_config->base.crtc = crtc; - drm_mode_copy(&pipe_config->base.adjusted_mode, mode); - drm_mode_copy(&pipe_config->base.mode, mode); - pipe_config->cpu_transcoder = (enum transcoder) to_intel_crtc(crtc)->pipe; pipe_config->shared_dpll = DPLL_ID_PRIVATE; @@ -12199,27 +12196,8 @@ static void update_scanline_offset(struct intel_crtc *crtc) crtc->scanline_offset = 1; } -static void -intel_atomic_modeset_compute_changed_flags(struct drm_atomic_state *state, - struct drm_crtc *modeset_crtc) -{ - struct drm_crtc_state *crtc_state; - struct drm_crtc *crtc; - int i; - - for_each_crtc_in_state(state, crtc, crtc_state, i) { - if (crtc_state->enable != crtc->state->enable) - crtc_state->mode_changed = true; - - /* FIXME: Do we need to always set mode_changed for - * modeset_crtc if it is enabled? modeset_affect_pipes() - * did that. */ - } -} - static struct intel_crtc_state * intel_modeset_compute_config(struct drm_crtc *crtc, - struct drm_display_mode *mode, struct drm_atomic_state *state) { struct intel_crtc_state *pipe_config; @@ -12229,7 +12207,9 @@ intel_modeset_compute_config(struct drm_crtc *crtc, if (ret) return ERR_PTR(ret); - intel_atomic_modeset_compute_changed_flags(state, crtc); + ret = drm_atomic_helper_check_modeset(state->dev, state); + if (ret) + return ERR_PTR(ret); /* * Note this needs changes when we start tracking multiple modes @@ -12244,7 +12224,7 @@ intel_modeset_compute_config(struct drm_crtc *crtc, if (!pipe_config->base.enable) return pipe_config; - ret = intel_modeset_pipe_config(crtc, mode, state, pipe_config); + ret = intel_modeset_pipe_config(crtc, state, pipe_config); if (ret) return ERR_PTR(ret); @@ -12262,6 +12242,10 @@ intel_modeset_compute_config(struct drm_crtc *crtc, intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,"[modeset]"); + ret = drm_atomic_helper_check_planes(state->dev, state); + if (ret) + return ERR_PTR(ret); + return pipe_config; } @@ -12337,7 +12321,6 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state) } static int __intel_set_mode(struct drm_crtc *modeset_crtc, - struct drm_display_mode *mode, struct intel_crtc_state *pipe_config) { struct drm_device *dev = modeset_crtc->dev; @@ -12380,7 +12363,7 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc, * single crtc and mode. */ if (pipe_config->base.enable && needs_modeset(&pipe_config->base)) { - modeset_crtc->mode = *mode; + modeset_crtc->mode = pipe_config->base.mode; /* mode_set/enable/disable functions rely on a correct pipe * config. */ intel_crtc_set_state(to_intel_crtc(modeset_crtc), pipe_config); @@ -12446,12 +12429,11 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc, } static int intel_set_mode_with_config(struct drm_crtc *crtc, - struct drm_display_mode *mode, struct intel_crtc_state *pipe_config) { int ret; - ret = __intel_set_mode(crtc, mode, pipe_config); + ret = __intel_set_mode(crtc, pipe_config); if (ret == 0) intel_modeset_check_state(crtc->dev); @@ -12460,19 +12442,18 @@ static int intel_set_mode_with_config(struct drm_crtc *crtc, } static int intel_set_mode(struct drm_crtc *crtc, - struct drm_display_mode *mode, struct drm_atomic_state *state) { struct intel_crtc_state *pipe_config; int ret = 0; - pipe_config = intel_modeset_compute_config(crtc, mode, state); + pipe_config = intel_modeset_compute_config(crtc, state); if (IS_ERR(pipe_config)) { ret = PTR_ERR(pipe_config); goto out; } - ret = intel_set_mode_with_config(crtc, mode, pipe_config); + ret = intel_set_mode_with_config(crtc, pipe_config); if (ret) goto out; @@ -12539,125 +12520,21 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc) } crtc_state->base.enable = intel_crtc->new_enabled; + + if (&intel_crtc->base == crtc) + drm_mode_copy(&crtc_state->base.mode, &crtc->mode); } intel_modeset_setup_plane_state(state, crtc, &crtc->mode, crtc->primary->fb, crtc->x, crtc->y); - intel_set_mode(crtc, &crtc->mode, state); + intel_set_mode(crtc, state); drm_atomic_state_free(state); } #undef for_each_intel_crtc_masked -static bool -is_crtc_connector_off(struct drm_mode_set *set) -{ - int i; - - if (set->num_connectors == 0) - return false; - - if (WARN_ON(set->connectors == NULL)) - return false; - - for (i = 0; i < set->num_connectors; i++) - if (set->connectors[i]->encoder && - set->connectors[i]->encoder->crtc == set->crtc && - set->connectors[i]->dpms != DRM_MODE_DPMS_ON) - return true; - - return false; -} - -static void -intel_set_config_compute_mode_changes(struct drm_mode_set *set, - struct intel_crtc_state *pipe_config) -{ - struct drm_atomic_state *state; - struct drm_connector *connector; - struct drm_connector_state *connector_state; - struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; - int i; - - /* We should be able to check here if the fb has the same properties - * and then just flip_or_move it */ - if (is_crtc_connector_off(set)) { - pipe_config->base.mode_changed = true; - } else if (set->crtc->primary->fb != set->fb) { - /* - * If we have no fb, we can only flip as long as the crtc is - * active, otherwise we need a full mode set. The crtc may - * be active if we've only disabled the primary plane, or - * in fastboot situations. - */ - if (set->crtc->primary->fb == NULL) { - struct intel_crtc *intel_crtc = - to_intel_crtc(set->crtc); - - if (intel_crtc->active) { - DRM_DEBUG_KMS("crtc has no fb, will flip\n"); - pipe_config->base.planes_changed = true; - } else { - DRM_DEBUG_KMS("inactive crtc, full mode set\n"); - pipe_config->base.mode_changed = true; - } - } else if (set->fb == NULL) { - pipe_config->base.mode_changed = true; - } else if (set->fb->pixel_format != - set->crtc->primary->fb->pixel_format) { - pipe_config->base.mode_changed = true; - } else { - pipe_config->base.planes_changed = true; - } - } - - if (set->fb && (set->x != set->crtc->x || set->y != set->crtc->y)) - pipe_config->base.planes_changed = true; - - if (set->mode && !drm_mode_equal(set->mode, &set->crtc->mode)) { - DRM_DEBUG_KMS("modes are different, full mode set\n"); - drm_mode_debug_printmodeline(&set->crtc->mode); - drm_mode_debug_printmodeline(set->mode); - pipe_config->base.mode_changed = true; - } - - state = pipe_config->base.state; - - for_each_connector_in_state(state, connector, connector_state, i) { - if (connector_state->best_encoder != - connector->state->best_encoder) { - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] encoder changed, full mode switch\n", - connector->base.id, - connector->name); - pipe_config->base.mode_changed = true; - } - - if (connector_state->crtc != connector->state->crtc) { - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] crtc changed, full mode switch\n", - connector->base.id, - connector->name); - pipe_config->base.mode_changed = true; - } - } - - for_each_crtc_in_state(state, crtc, crtc_state, i) { - if (crtc_state->enable == crtc->state->enable) - continue; - - DRM_DEBUG_KMS("[CRTC:%d] %sabled, full mode switch\n", - crtc->base.id, - crtc_state->enable ? "en" : "dis"); - pipe_config->base.mode_changed = true; - } - - DRM_DEBUG_KMS("computed changes for [CRTC:%d], mode_changed=%d, fb_changed=%d\n", - set->crtc->base.id, pipe_config->base.mode_changed, - pipe_config->base.planes_changed); -} - static bool intel_connector_in_mode_set(struct intel_connector *connector, struct drm_mode_set *set) { @@ -12774,6 +12651,21 @@ intel_modeset_stage_output_state(struct drm_device *dev, crtc_state->enable = drm_atomic_connectors_for_crtc(state, crtc); } + ret = intel_modeset_setup_plane_state(state, set->crtc, set->mode, + set->fb, set->x, set->y); + if (ret) + return ret; + + crtc_state = drm_atomic_get_crtc_state(state, set->crtc); + if (IS_ERR(crtc_state)) + return PTR_ERR(crtc_state); + + if (set->mode) + drm_mode_copy(&crtc_state->mode, set->mode); + + if (set->num_connectors) + crtc_state->active = true; + return 0; } @@ -12821,30 +12713,17 @@ static int intel_crtc_set_config(struct drm_mode_set *set) if (ret) goto out; - ret = intel_modeset_setup_plane_state(state, set->crtc, set->mode, - set->fb, set->x, set->y); - if (ret) - goto out; - - pipe_config = intel_modeset_compute_config(set->crtc, set->mode, - state); + pipe_config = intel_modeset_compute_config(set->crtc, state); if (IS_ERR(pipe_config)) { ret = PTR_ERR(pipe_config); goto out; } - /* Compute whether we need a full modeset, only an fb base update or no - * change at all. In the future we might also check whether only the - * mode changed, e.g. for LVDS where we only change the panel fitter in - * such cases. */ - intel_set_config_compute_mode_changes(set, pipe_config); - intel_update_pipe_size(to_intel_crtc(set->crtc)); primary_plane_was_visible = primary_plane_visible(set->crtc); - ret = intel_set_mode_with_config(set->crtc, set->mode, - pipe_config); + ret = intel_set_mode_with_config(set->crtc, pipe_config); if (ret == 0 && pipe_config->base.enable && -- 2.20.1