From patchwork Fri Nov 11 16:57:40 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Zanoni, Paulo R" X-Patchwork-Id: 9423255 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id CB7B660484 for ; Fri, 11 Nov 2016 16:58:06 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B89E5293E1 for ; Fri, 11 Nov 2016 16:58:06 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id AD99229466; Fri, 11 Nov 2016 16:58:06 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 247DA293E1 for ; Fri, 11 Nov 2016 16:58:06 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C97186E939; Fri, 11 Nov 2016 16:58:04 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1BA466E935 for ; Fri, 11 Nov 2016 16:58:02 +0000 (UTC) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga105.jf.intel.com with ESMTP; 11 Nov 2016 08:58:02 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,621,1473145200"; d="scan'208";a="2939530" Received: from przanoni-mobl.amr.corp.intel.com ([10.254.182.134]) by orsmga002.jf.intel.com with ESMTP; 11 Nov 2016 08:58:00 -0800 From: Paulo Zanoni To: intel-gfx@lists.freedesktop.org Date: Fri, 11 Nov 2016 14:57:40 -0200 Message-Id: <1478883461-20201-7-git-send-email-paulo.r.zanoni@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1478883461-20201-1-git-send-email-paulo.r.zanoni@intel.com> References: <1478883461-20201-1-git-send-email-paulo.r.zanoni@intel.com> MIME-Version: 1.0 Cc: Paulo Zanoni Subject: [Intel-gfx] [PATCH 6/7] drm/i915/fbc: move from crtc_state->enable_fbc to plane_state->enable_fbc 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-Virus-Scanned: ClamAV using ClamSMTP Ville pointed out that intel_fbc_choose_crtc() is iterating over all planes instead of just the primary planes. There are no real consequences of this problem for HSW+, and for the other platforms it just means that in some obscure multi-screen cases we'll keep FBC disabled when we could have enabled it. Still, iterating over all planes doesn't seem to be the best thing to do. My initial idea was to just add a check for plane->type and be done, but then I realized that in commits not involving the primary plane we would reset crtc_state->enable_fbc back to false even when FBC is enabled. That also wouldn't result in a bug due to the way the enable_fbc variable is checked, but, still, our code can be better than this. So I went for the solution that involves tracking enable_fbc in the primary plane state instead of the CRTC state. This way, if a commit doesn't involve the primary plane for the CRTC we won't be resetting enable_fbc back to false, so the variable will always reflect the reality. And this change also makes more sense since FBC is actually tied to the single plane and not the full pipe. As a bonus, we only iterate over the CRTCs instead of iterating over all planes. Cc: Ville Syrjälä Reported-by: Ville Syrjälä Signed-off-by: Paulo Zanoni Reviewed-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_drv.h | 4 ++-- drivers/gpu/drm/i915/intel_fbc.c | 36 +++++++++++++++++++----------------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 003afb8..025cb74 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -403,6 +403,8 @@ struct intel_plane_state { int scaler_id; struct drm_intel_sprite_colorkey ckey; + + bool enable_fbc; }; struct intel_initial_plane_config { @@ -648,8 +650,6 @@ struct intel_crtc_state { bool ips_enabled; - bool enable_fbc; - bool double_wide; bool dp_encoder_is_mst; diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index b095175..fc4ac57 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -1055,16 +1055,17 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv, struct drm_atomic_state *state) { struct intel_fbc *fbc = &dev_priv->fbc; - struct drm_plane *plane; - struct drm_plane_state *plane_state; + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; bool crtc_chosen = false; int i; mutex_lock(&fbc->lock); - /* Does this atomic commit involve the CRTC currently tied to FBC? */ + /* Does this atomic commit involve the plane currently tied to FBC? */ if (fbc->crtc && - !drm_atomic_get_existing_crtc_state(state, &fbc->crtc->base)) + !drm_atomic_get_existing_plane_state(state, + fbc->crtc->base.primary)) goto out; if (!intel_fbc_can_enable(dev_priv)) @@ -1074,25 +1075,26 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv, * plane. We could go for fancier schemes such as checking the plane * size, but this would just affect the few platforms that don't tie FBC * to pipe or plane A. */ - for_each_plane_in_state(state, plane, plane_state, i) { - struct intel_plane_state *intel_plane_state = - to_intel_plane_state(plane_state); - struct intel_crtc_state *intel_crtc_state; - struct intel_crtc *crtc = to_intel_crtc(plane_state->crtc); + for_each_crtc_in_state(state, crtc, crtc_state, i) { + struct intel_plane_state *plane_state = to_intel_plane_state( + drm_atomic_get_existing_plane_state(state, + crtc->primary)); + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - if (!intel_plane_state->base.visible) + if (!plane_state) continue; - if (fbc_on_pipe_a_only(dev_priv) && crtc->pipe != PIPE_A) + if (!plane_state->base.visible) continue; - if (fbc_on_plane_a_only(dev_priv) && crtc->plane != PLANE_A) + if (fbc_on_pipe_a_only(dev_priv) && intel_crtc->pipe != PIPE_A) continue; - intel_crtc_state = to_intel_crtc_state( - drm_atomic_get_existing_crtc_state(state, &crtc->base)); + if (fbc_on_plane_a_only(dev_priv) && + intel_crtc->plane != PLANE_A) + continue; - intel_crtc_state->enable_fbc = true; + plane_state->enable_fbc = true; crtc_chosen = true; break; } @@ -1130,13 +1132,13 @@ void intel_fbc_enable(struct intel_crtc *crtc, if (fbc->enabled) { WARN_ON(fbc->crtc == NULL); if (fbc->crtc == crtc) { - WARN_ON(!crtc_state->enable_fbc); + WARN_ON(!plane_state->enable_fbc); WARN_ON(fbc->active); } goto out; } - if (!crtc_state->enable_fbc) + if (!plane_state->enable_fbc) goto out; WARN_ON(fbc->active);