From patchwork Thu Sep 10 01:57:55 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Matt Roper X-Patchwork-Id: 7150031 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id A5062BF036 for ; Thu, 10 Sep 2015 01:58:45 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 8C9932097B for ; Thu, 10 Sep 2015 01:58:44 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 5CF6A20959 for ; Thu, 10 Sep 2015 01:58:43 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id F2FE972256; Wed, 9 Sep 2015 18:58:41 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 71FE8721A3 for ; Wed, 9 Sep 2015 18:58:40 -0700 (PDT) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP; 09 Sep 2015 18:58:40 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,501,1437462000"; d="scan'208";a="801303994" Received: from mdroper-hswdev.fm.intel.com (HELO mdroper-hswdev) ([10.1.134.215]) by orsmga002.jf.intel.com with ESMTP; 09 Sep 2015 18:58:41 -0700 Received: from mattrope by mdroper-hswdev with local (Exim 4.84) (envelope-from ) id 1ZZr8J-0005JT-RU; Wed, 09 Sep 2015 18:58:39 -0700 From: Matt Roper To: intel-gfx@lists.freedesktop.org Date: Wed, 9 Sep 2015 18:57:55 -0700 Message-Id: <1441850275-20353-5-git-send-email-matthew.d.roper@intel.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1441850275-20353-1-git-send-email-matthew.d.roper@intel.com> References: <1441850275-20353-1-git-send-email-matthew.d.roper@intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 4/4] drm/i915: Update modeset programming to use intermediate state (v2) X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP As suggested by Ville, the general flow should now roughly follow: // whatever the user wanted compute_final_atomic_state() // include all crtcs in the intermediate state which are // getting disabled (even temporarily to perform a modeset) compute_intermediate_atomic_state() ret = check_state_change(old, intermediate) ret = check_state_change(intermediate, new) // commit all planes in one go to make them pop out as // atomically as possible for_each_crtc_in(intermediate) { commit_planes() } for_each_crtc_in(intermediate) { disable_crtc() } for_each_crtc_in(new) { if (!currently_active) crtc_enable() } // commit all planes in one go to make them pop in as atomically // as possible for_each_crtc_in(new) { commit_planes() } v2: Because we're potentially performing two state swaps here, the actual set of FB's that need to be cleaned up at the end of the process may need to be fetched from the intermediate state rather than the final state, so use our own intel_cleanup_planes() rather than the helper version. Cc: Ville Syrjälä Cc: Maarten Lankhorst Signed-off-by: Matt Roper --- I know Maarten had some reservations about this approach, so hopefully providing an implementation will allow us to continue the discussion and come to an agreement on whether or not intermediate states are the way to go. drivers/gpu/drm/i915/intel_display.c | 104 +++++++++++++++++++++++++++-------- 1 file changed, 80 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5081a9e..d63ad1d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13027,6 +13027,36 @@ static int intel_atomic_check(struct drm_device *dev, return intel_atomic_check_planes(state->dev, state); } +/* + * An Intel-specific version of drm_atomic_helper_cleanup_planes(). + * Since we've potentially swapped to an intermediate state before + * swapping again to the final state, the actual framebuffers that + * need to be cleaned up will have been swapped into the intermediate + * state object for any CRTC's that are undergoing a modeset, so we + * need to grab them from there instead of the old_state object. + */ +static void +intel_cleanup_planes(struct drm_device *dev, + struct drm_atomic_state *old_state) +{ + struct drm_atomic_state *inter = + to_intel_atomic_state(old_state)->intermediate; + struct drm_plane *plane; + struct drm_plane_state *plane_state; + int i; + + for_each_plane_in_state(old_state, plane, plane_state, i) { + const struct drm_plane_helper_funcs *funcs; + + funcs = plane->helper_private; + + plane_state = inter->plane_states[i] ?: plane_state; + + if (funcs->cleanup_fb) + funcs->cleanup_fb(plane, plane_state); + } +} + /** * intel_atomic_commit - commit validated state object * @dev: DRM device @@ -13048,11 +13078,12 @@ static int intel_atomic_commit(struct drm_device *dev, bool async) { struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_atomic_state *istate = to_intel_atomic_state(state); + struct drm_atomic_state *inter = istate->intermediate; struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *old_crtc_state; int ret = 0; int i; - bool any_ms = false; if (async) { DRM_DEBUG_KMS("i915 does not yet support async commit\n"); @@ -13063,59 +13094,84 @@ static int intel_atomic_commit(struct drm_device *dev, if (ret) return ret; - drm_atomic_helper_swap_state(dev, state); + if (!istate->any_ms) { + /* + * If there's no modeset involved, just move to the final state + * and jump directly to the final plane updates. + */ + drm_atomic_helper_swap_state(dev, state); + intel_modeset_update_crtc_state(state); + goto nomodeset; + } - for_each_crtc_in_state(state, crtc, crtc_state, i) { - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + /* Swap in intermediate state */ + drm_atomic_helper_swap_state(dev, inter); - if (!needs_modeset(crtc->state)) - continue; + /* + * Commit all planes in one go to make them pop out as atomically + * as possible. + */ + for_each_crtc_in_state(inter, crtc, old_crtc_state, i) { + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - any_ms = true; intel_pre_plane_update(intel_crtc); + drm_atomic_helper_commit_planes_on_crtc(old_crtc_state); + intel_post_plane_update(intel_crtc); + } + + for_each_crtc_in_state(inter, crtc, old_crtc_state, i) { + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - if (crtc_state->active) { - intel_crtc_disable_planes(crtc, crtc_state->plane_mask); + if (old_crtc_state->active) { + intel_crtc_disable_planes(crtc, + old_crtc_state->plane_mask); dev_priv->display.crtc_disable(crtc); intel_crtc->active = false; intel_disable_shared_dpll(intel_crtc); } } + /* Commit final state to the underlying crtc/plane objects */ + drm_atomic_helper_swap_state(dev, state); + /* Only after disabling all output pipelines that will be changed can we * update the the output configuration. */ intel_modeset_update_crtc_state(state); + intel_shared_dpll_commit(state); - if (any_ms) { - intel_shared_dpll_commit(state); - - drm_atomic_helper_update_legacy_modeset_state(state->dev, state); - modeset_update_crtc_power_domains(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) { - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + /* Now enable the clocks, pipe, and connectors that we set up. */ + for_each_crtc_in_state(state, crtc, old_crtc_state, i) { bool modeset = needs_modeset(crtc->state); if (modeset && crtc->state->active) { update_scanline_offset(to_intel_crtc(crtc)); dev_priv->display.crtc_enable(crtc); } + } - if (!modeset) - intel_pre_plane_update(intel_crtc); +nomodeset: - drm_atomic_helper_commit_planes_on_crtc(crtc_state); + /* + * Commit all planes in one go to make them pop in as atomically as + * possible. + */ + for_each_crtc_in_state(state, crtc, old_crtc_state, i) { + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + + intel_pre_plane_update(intel_crtc); + drm_atomic_helper_commit_planes_on_crtc(old_crtc_state); intel_post_plane_update(intel_crtc); } /* FIXME: add subpixel order */ drm_atomic_helper_wait_for_vblanks(dev, state); - drm_atomic_helper_cleanup_planes(dev, state); + intel_cleanup_planes(dev, state); - if (any_ms) + if (istate->any_ms) intel_modeset_check_state(dev, state); drm_atomic_state_free(state);