From patchwork Fri Apr 10 12:26:29 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Stanislav Lisovskiy X-Patchwork-Id: 11483253 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 299DC14B4 for ; Fri, 10 Apr 2020 12:30:03 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1007920769 for ; Fri, 10 Apr 2020 12:30:03 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1007920769 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 5D56E6E267; Fri, 10 Apr 2020 12:30:01 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7461D6E267 for ; Fri, 10 Apr 2020 12:29:59 +0000 (UTC) IronPort-SDR: UAwqUz4sF/bhCOPIhGHuU0Gu2bLrdZpkAcStoQeafF3ybxW2XllqXd38JztDC3DzAubhhlbhbg W7luwHfvCzbA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Apr 2020 05:29:58 -0700 IronPort-SDR: 68YFD+bIxm96uGrtIdMn75NIDQDzm7rcHNTie+cPwC7lO7/NyutcpDQWazJ57T1fHUxmLXWZx3 0iXBWjHObl3w== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,366,1580803200"; d="scan'208";a="425872115" Received: from unknown (HELO slisovsk-Lenovo-ideapad-720S-13IKB.fi.intel.com) ([10.237.72.89]) by orsmga005.jf.intel.com with ESMTP; 10 Apr 2020 05:29:56 -0700 From: Stanislav Lisovskiy To: intel-gfx@lists.freedesktop.org Date: Fri, 10 Apr 2020 15:26:29 +0300 Message-Id: <20200410122629.9181-1-stanislav.lisovskiy@intel.com> X-Mailer: git-send-email 2.24.1.485.gad05a3d8e5 In-Reply-To: <20200409154730.18568-8-stanislav.lisovskiy@intel.com> References: <20200409154730.18568-8-stanislav.lisovskiy@intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH v23 07/13] drm/i915: Use bw state for per crtc SAGV evaluation X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: jani.nikula@intel.com Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" Future platforms require per-crtc SAGV evaluation and serializing global state when those are changed from different commits. v2: - Add has_sagv check to intel_crtc_can_enable_sagv so that it sets bit in reject mask. - Use bw_state in intel_pre/post_plane_enable_sagv instead of atomic state Signed-off-by: Stanislav Lisovskiy Cc: Ville Syrjälä Cc: James Ausmus --- drivers/gpu/drm/i915/display/intel_bw.h | 6 ++ drivers/gpu/drm/i915/intel_pm.c | 98 ++++++++++++++++++++----- drivers/gpu/drm/i915/intel_pm.h | 4 +- 3 files changed, 90 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h index ac004d6f4276..d6df91058223 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.h +++ b/drivers/gpu/drm/i915/display/intel_bw.h @@ -18,6 +18,12 @@ struct intel_crtc_state; struct intel_bw_state { struct intel_global_state base; + /* + * Contains a bit mask, used to determine, whether correspondent + * pipe allows SAGV or not. + */ + u8 pipe_sagv_reject; + unsigned int data_rate[I915_MAX_PIPES]; u8 num_active_planes[I915_MAX_PIPES]; }; diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 88637f006add..a3c8c0c7a503 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -43,6 +43,7 @@ #include "i915_fixed.h" #include "i915_irq.h" #include "i915_trace.h" +#include "display/intel_bw.h" #include "intel_pm.h" #include "intel_sideband.h" #include "../../../platform/x86/intel_ips.h" @@ -3634,7 +3635,7 @@ static bool skl_needs_memory_bw_wa(struct drm_i915_private *dev_priv) return IS_GEN9_BC(dev_priv) || IS_BROXTON(dev_priv); } -static bool +bool intel_has_sagv(struct drm_i915_private *dev_priv) { /* HACK! */ @@ -3760,8 +3761,23 @@ intel_disable_sagv(struct drm_i915_private *dev_priv) void intel_sagv_pre_plane_update(struct intel_atomic_state *state) { struct drm_i915_private *dev_priv = to_i915(state->base.dev); + const struct intel_bw_state *new_bw_state = NULL; - if (!intel_can_enable_sagv(state)) { + /* + * Just return if we can't control SAGV or don't have it. + * This is different from situation when we have SAGV but just can't + * afford it due to DBuf limitation - in case if SAGV is completely + * disabled in a BIOS, we are not even allowed to send a PCode request, + * as it will throw an error. So have to check it here. + */ + if (!intel_has_sagv(dev_priv)) + return; + + new_bw_state = intel_atomic_get_new_bw_state(state); + if (!new_bw_state) + return; + + if (!intel_can_enable_sagv(new_bw_state)) { intel_disable_sagv(dev_priv); return; } @@ -3770,8 +3786,23 @@ void intel_sagv_pre_plane_update(struct intel_atomic_state *state) void intel_sagv_post_plane_update(struct intel_atomic_state *state) { struct drm_i915_private *dev_priv = to_i915(state->base.dev); + const struct intel_bw_state *new_bw_state = NULL; - if (intel_can_enable_sagv(state)) { + /* + * Just return if we can't control SAGV or don't have it. + * This is different from situation when we have SAGV but just can't + * afford it due to DBuf limitation - in case if SAGV is completely + * disabled in a BIOS, we are not even allowed to send a PCode request, + * as it will throw an error. So have to check it here. + */ + if (!intel_has_sagv(dev_priv)) + return; + + new_bw_state = intel_atomic_get_new_bw_state(state); + if (!new_bw_state) + return; + + if (intel_can_enable_sagv(new_bw_state)) { intel_enable_sagv(dev_priv); return; } @@ -3779,15 +3810,26 @@ void intel_sagv_post_plane_update(struct intel_atomic_state *state) static bool intel_crtc_can_enable_sagv(const struct intel_crtc_state *crtc_state) { + struct intel_atomic_state *state = to_intel_atomic_state(crtc_state->uapi.state); struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev); struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); struct intel_plane *plane; const struct intel_plane_state *plane_state; int level, latency; + if (!intel_has_sagv(dev_priv)) + return false; + if (!crtc_state->hw.active) return true; + /* + * SKL+ workaround: bspec recommends we disable SAGV when we have + * more then one pipe enabled + */ + if (hweight8(state->active_pipes) > 1) + return false; + if (crtc_state->hw.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) { DRM_DEBUG_KMS("No SAGV for interlaced mode on pipe %c\n", pipe_name(crtc->pipe)); @@ -3826,29 +3868,47 @@ static bool intel_crtc_can_enable_sagv(const struct intel_crtc_state *crtc_state return true; } -bool intel_can_enable_sagv(struct intel_atomic_state *state) +bool intel_can_enable_sagv(const struct intel_bw_state *bw_state) +{ + return bw_state->pipe_sagv_reject == 0; +} + +static int intel_compute_sagv_mask(struct intel_atomic_state *state) { struct drm_i915_private *dev_priv = to_i915(state->base.dev); + int ret; struct intel_crtc *crtc; const struct intel_crtc_state *crtc_state; enum pipe pipe; + struct intel_crtc_state *new_crtc_state; + struct intel_bw_state *new_bw_state = NULL; + const struct intel_bw_state *old_bw_state = NULL; + int i; - if (!intel_has_sagv(dev_priv)) - return false; + for_each_new_intel_crtc_in_state(state, crtc, + new_crtc_state, i) { + new_bw_state = intel_atomic_get_bw_state(state); + if (IS_ERR(new_bw_state)) + return PTR_ERR(new_bw_state); - /* - * SKL+ workaround: bspec recommends we disable SAGV when we have - * more then one pipe enabled - */ - if (hweight8(state->active_pipes) > 1) - return false; + old_bw_state = intel_atomic_get_old_bw_state(state); - /* Since we're now guaranteed to only have one active CRTC... */ - pipe = ffs(state->active_pipes) - 1; - crtc = intel_get_crtc_for_pipe(dev_priv, pipe); - crtc_state = to_intel_crtc_state(crtc->base.state); + if (intel_crtc_can_enable_sagv(new_crtc_state)) + new_bw_state->pipe_sagv_reject &= ~BIT(crtc->pipe); + else + new_bw_state->pipe_sagv_reject |= BIT(crtc->pipe); + } + + if (!old_bw_state) + return 0; + + if (new_bw_state->pipe_sagv_reject != old_bw_state->pipe_sagv_reject) { + ret = intel_atomic_serialize_global_state(&new_bw_state->base); + if (ret) + return ret; + } - return intel_crtc_can_enable_sagv(crtc_state); + return 0; } /* @@ -5863,6 +5923,10 @@ skl_compute_wm(struct intel_atomic_state *state) if (ret) return ret; + ret = intel_compute_sagv_mask(state); + if (ret) + return ret; + /* * skl_compute_ddb() will have adjusted the final watermarks * based on how much ddb is available. Now we can actually diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h index 9a6036ab0f90..abefc4205d0b 100644 --- a/drivers/gpu/drm/i915/intel_pm.h +++ b/drivers/gpu/drm/i915/intel_pm.h @@ -9,6 +9,7 @@ #include #include "i915_reg.h" +#include "display/intel_bw.h" struct drm_device; struct drm_i915_private; @@ -41,7 +42,8 @@ void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc, struct skl_pipe_wm *out); void g4x_wm_sanitize(struct drm_i915_private *dev_priv); void vlv_wm_sanitize(struct drm_i915_private *dev_priv); -bool intel_can_enable_sagv(struct intel_atomic_state *state); +bool intel_has_sagv(struct drm_i915_private *dev_priv); +bool intel_can_enable_sagv(const struct intel_bw_state *bw_state); int intel_enable_sagv(struct drm_i915_private *dev_priv); int intel_disable_sagv(struct drm_i915_private *dev_priv); void intel_sagv_pre_plane_update(struct intel_atomic_state *state);