From patchwork Tue Nov 3 16:54:57 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matt Roper X-Patchwork-Id: 7545701 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 56DBE9F399 for ; Tue, 3 Nov 2015 16:55:15 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4B5A820719 for ; Tue, 3 Nov 2015 16:55:14 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 77E792070E for ; Tue, 3 Nov 2015 16:55:12 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8C2A87209E; Tue, 3 Nov 2015 08:55:07 -0800 (PST) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTP id 74B03720CD for ; Tue, 3 Nov 2015 08:55:03 -0800 (PST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga103.jf.intel.com with ESMTP; 03 Nov 2015 08:55:01 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,239,1444719600"; d="scan'208";a="677534637" Received: from mdroper-hswdev.fm.intel.com (HELO mdroper-hswdev) ([10.1.134.207]) by orsmga003.jf.intel.com with ESMTP; 03 Nov 2015 08:55:01 -0800 Received: from mattrope by mdroper-hswdev with local (Exim 4.84) (envelope-from ) id 1ZterM-0004Xg-Am; Tue, 03 Nov 2015 08:55:00 -0800 From: Matt Roper To: intel-gfx@lists.freedesktop.org Date: Tue, 3 Nov 2015 08:54:57 -0800 Message-Id: <1446569697-17425-1-git-send-email-matthew.d.roper@intel.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <87k2pzo0yd.fsf@intel.com> References: <87k2pzo0yd.fsf@intel.com> Cc: Jani Nikula Subject: [Intel-gfx] [PATCH 3/4] drm/i915: Sanitize watermarks after hardware state readout (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: , MIME-Version: 1.0 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 Although we can do a good job of reading out hardware state, the graphics firmware may have programmed the watermarks in a creative way that doesn't match how i915 would have chosen to program them. We shouldn't trust the firmware's watermark programming, but should rather re-calculate how we think WM's should be programmed and then shove those values into the hardware. We can do this pretty easily by creating a dummy top-level state, running it through the check process to calculate all the values, and then just programming the watermarks for each CRTC. v2: Move watermark sanitization after our BIOS fb reconstruction; the watermark calculations that we do here need to look at pstate->fb, which isn't setup yet in intel_modeset_setup_hw_state(), even though we have an enabled & visible plane. Cc: Jani Nikula Cc: Maarten Lankhorst Signed-off-by: Matt Roper --- Jani, based on your logs it looks like the culprit is that we're calculating watermarks at startup time after we've read out preliminary hardware state (so we know the primary plane is enabled & visible), but before we reconstruct the BIOS fb (so pstate->fb is NULL). I think moving the watermark sanitization later in the process so that we'll have a proper fb setup should hopefully solve the issue. Can you test this version when you get a chance? I'll also send a rebased patch #4 since the code movement here means that the previous version won't apply cleanly. drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c | 55 ++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_pm.c | 14 +++++---- 3 files changed, 64 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 20cd6d8..09807c8 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -629,6 +629,7 @@ struct drm_i915_display_funcs { struct dpll *best_clock); int (*compute_pipe_wm)(struct intel_crtc *crtc, struct drm_atomic_state *state); + void (*program_watermarks)(struct intel_crtc_state *cstate); void (*update_wm)(struct drm_crtc *crtc); int (*modeset_calc_cdclk)(struct drm_atomic_state *state); void (*modeset_commit_cdclk)(struct drm_atomic_state *state); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7b3cfb6..e289311 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14936,6 +14936,54 @@ void intel_modeset_init_hw(struct drm_device *dev) intel_enable_gt_powersave(dev); } +/* + * Calculate what we think the watermarks should be for the state we've read + * out of the hardware and then immediately program those watermarks so that + * we ensure the hardware settings match our internal state. + */ +static void sanitize_watermarks(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = to_i915(dev); + struct drm_atomic_state *state; + struct drm_crtc *crtc; + struct drm_crtc_state *cstate; + int ret; + int i; + + /* Only supported on platforms that use atomic watermark design */ + if (!dev_priv->display.program_watermarks) + return; + + /* + * Calculate what we think WM's should be by creating a dummy state and + * running it through the atomic check code. + */ + state = drm_atomic_helper_duplicate_state(dev, + dev->mode_config.acquire_ctx); + if (WARN_ON(IS_ERR(state))) + return; + + ret = intel_atomic_check(dev, state); + if (ret) { + /* + * Just give up and leave watermarks untouched if we get an + * error back from 'check' + */ + DRM_DEBUG_KMS("Could not determine valid watermarks for inherited state\n"); + return; + } + + /* Write calculated watermark values back */ + to_i915(dev)->wm.config = to_intel_atomic_state(state)->wm_config; + for_each_crtc_in_state(state, crtc, cstate, i) { + struct intel_crtc_state *cs = to_intel_crtc_state(cstate); + + dev_priv->display.program_watermarks(cs); + } + + drm_atomic_state_free(state); +} + void intel_modeset_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -15059,6 +15107,13 @@ void intel_modeset_init(struct drm_device *dev) */ intel_find_initial_plane_obj(crtc, &plane_config); } + + /* + * Make sure hardware watermarks really match the state we read out. + * Note that we need to do this after reconstructing the BIOS fb's + * since the watermark calculation done here will use pstate->fb. + */ + sanitize_watermarks(dev); } static void intel_enable_pipe_a(struct drm_device *dev) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 180348b..fbcb072 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3611,15 +3611,19 @@ static void skl_update_wm(struct drm_crtc *crtc) dev_priv->wm.skl_hw = *results; } -static void ilk_program_watermarks(struct drm_i915_private *dev_priv) +static void ilk_program_watermarks(struct intel_crtc_state *cstate) { - struct drm_device *dev = dev_priv->dev; + struct drm_crtc *crtc = cstate->base.crtc; + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = to_i915(dev); struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm; struct ilk_wm_maximums max; struct intel_wm_config *config = &dev_priv->wm.config; struct ilk_wm_values results = {}; enum intel_ddb_partitioning partitioning; + to_intel_crtc(crtc)->wm.active.ilk = cstate->wm.optimal.ilk; + ilk_compute_wm_maximums(dev, 1, config, INTEL_DDB_PART_1_2, &max); ilk_wm_merge(dev, config, &max, &lp_wm_1_2); @@ -3644,7 +3648,6 @@ static void ilk_program_watermarks(struct drm_i915_private *dev_priv) static void ilk_update_wm(struct drm_crtc *crtc) { - struct drm_i915_private *dev_priv = to_i915(crtc->dev); struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state); @@ -3662,9 +3665,7 @@ static void ilk_update_wm(struct drm_crtc *crtc) intel_wait_for_vblank(crtc->dev, intel_crtc->pipe); } - intel_crtc->wm.active.ilk = cstate->wm.optimal.ilk; - - ilk_program_watermarks(dev_priv); + ilk_program_watermarks(cstate); } static void skl_pipe_wm_active_state(uint32_t val, @@ -6972,6 +6973,7 @@ void intel_init_pm(struct drm_device *dev) dev_priv->wm.spr_latency[0] && dev_priv->wm.cur_latency[0])) { dev_priv->display.update_wm = ilk_update_wm; dev_priv->display.compute_pipe_wm = ilk_compute_pipe_wm; + dev_priv->display.program_watermarks = ilk_program_watermarks; } else { DRM_DEBUG_KMS("Failed to read display plane latency. " "Disable CxSR\n");