From patchwork Thu Dec 6 17:09:42 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matt Roper X-Patchwork-Id: 10716447 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 163FA1932 for ; Thu, 6 Dec 2018 17:10:17 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id F40F42F191 for ; Thu, 6 Dec 2018 17:10:16 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E85F52F193; Thu, 6 Dec 2018 17:10:16 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, 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 A335A2F194 for ; Thu, 6 Dec 2018 17:10:16 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 38DD86E622; Thu, 6 Dec 2018 17:10:16 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by gabe.freedesktop.org (Postfix) with ESMTPS id EA5DB6E622 for ; Thu, 6 Dec 2018 17:10:14 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Dec 2018 09:10:14 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,323,1539673200"; d="scan'208";a="123579473" Received: from mdroper-desk.fm.intel.com ([10.105.128.10]) by fmsmga002.fm.intel.com with ESMTP; 06 Dec 2018 09:10:14 -0800 From: Matt Roper To: intel-gfx@lists.freedesktop.org Date: Thu, 6 Dec 2018 09:09:42 -0800 Message-Id: <20181206170944.16539-2-matthew.d.roper@intel.com> X-Mailer: git-send-email 2.14.4 In-Reply-To: <20181206170944.16539-1-matthew.d.roper@intel.com> References: <20181206170944.16539-1-matthew.d.roper@intel.com> Subject: [Intel-gfx] [PATCH 1/3] drm/i915: Remove a very stale FIXME X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 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-Virus-Scanned: ClamAV using ClamSMTP SKL watermark calculations can and do trigger atomic transaction rejection if no valid set of watermarks can be found. This FIXME comment in the code hasn't been relevant for a very long time. Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/intel_pm.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index a26b4eddda25..9500bda64f26 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5548,10 +5548,6 @@ skl_compute_wm(struct drm_atomic_state *state) * Note that the DDB allocation above may have added more CRTC's that * weren't otherwise being modified (and set bits in dirty_pipes) if * pipe allocations had to change. - * - * FIXME: Now that we're doing this in the atomic check phase, we - * should allow skl_update_pipe_wm() to return failure in cases where - * no suitable watermark values can be found. */ for_each_new_crtc_in_state(state, crtc, cstate, i) { struct intel_crtc_state *intel_cstate = From patchwork Thu Dec 6 17:09:43 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matt Roper X-Patchwork-Id: 10716449 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A938818A7 for ; Thu, 6 Dec 2018 17:10:28 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8F5E42F1A3 for ; Thu, 6 Dec 2018 17:10:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 83D322F1B1; Thu, 6 Dec 2018 17:10:28 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, 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 8614E2F194 for ; Thu, 6 Dec 2018 17:10:27 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 9668A6E626; Thu, 6 Dec 2018 17:10:25 +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 F2BD76E623 for ; Thu, 6 Dec 2018 17:10:23 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Dec 2018 09:10:23 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,323,1539673200"; d="scan'208";a="123579522" Received: from mdroper-desk.fm.intel.com ([10.105.128.10]) by fmsmga002.fm.intel.com with ESMTP; 06 Dec 2018 09:10:23 -0800 From: Matt Roper To: intel-gfx@lists.freedesktop.org Date: Thu, 6 Dec 2018 09:09:43 -0800 Message-Id: <20181206170944.16539-3-matthew.d.roper@intel.com> X-Mailer: git-send-email 2.14.4 In-Reply-To: <20181206170944.16539-1-matthew.d.roper@intel.com> References: <20181206170944.16539-1-matthew.d.roper@intel.com> Subject: [Intel-gfx] [PATCH 2/3] drm/i915: Don't use DDB allocation when choosing gen9 watermark method X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 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-Virus-Scanned: ClamAV using ClamSMTP The bspec gives an if/else chain for choosing whether to use "method 1" or "method 2" for calculating the watermark "Selected Result Blocks" value for a plane. One of the branches of the if chain is: "Else If ('plane buffer allocation' is known and (plane buffer allocation / plane blocks per line) >=1)" Since our driver currently calculates DDB allocations first and the actual watermark values second, the plane buffer allocation is known at this point in our code and we include this test in our driver's logic. However we plan to soon move to a "watermarks first, ddb allocation second" sequence where we won't know the DDB allocation at this point. Let's drop this arm of the if/else statement (effectively considering the DDB allocation unknown) as an independent patch so that any regressions can be more accurately bisected to either the different watermark value (in this patch) or the new DDB allocation (in the next patch). Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/intel_pm.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 9500bda64f26..b09c2a257ff1 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4766,13 +4766,6 @@ static int skl_compute_plane_wm(const struct intel_crtc_state *cstate, wp->dbuf_block_size < 1) && (wp->plane_bytes_per_line / wp->dbuf_block_size < 1)) { selected_result = method2; - } else if (ddb_allocation >= - fixed16_to_u32_round_up(wp->plane_blocks_per_line)) { - if (IS_GEN9(dev_priv) && - !IS_GEMINILAKE(dev_priv)) - selected_result = min_fixed16(method1, method2); - else - selected_result = method2; } else if (latency >= wp->linetime_us) { if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv)) From patchwork Thu Dec 6 17:09:44 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matt Roper X-Patchwork-Id: 10716451 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A907C1923 for ; Thu, 6 Dec 2018 17:10:37 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8FF862F1AC for ; Thu, 6 Dec 2018 17:10:37 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 83ECF2F1B3; Thu, 6 Dec 2018 17:10:37 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, 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 753252F1AC for ; Thu, 6 Dec 2018 17:10:36 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id CBF536E624; Thu, 6 Dec 2018 17:10:35 +0000 (UTC) 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 ESMTPS id 4A0556E624 for ; Thu, 6 Dec 2018 17:10:34 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Dec 2018 09:10:33 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,323,1539673200"; d="scan'208";a="123579580" Received: from mdroper-desk.fm.intel.com ([10.105.128.10]) by fmsmga002.fm.intel.com with ESMTP; 06 Dec 2018 09:10:33 -0800 From: Matt Roper To: intel-gfx@lists.freedesktop.org Date: Thu, 6 Dec 2018 09:09:44 -0800 Message-Id: <20181206170944.16539-4-matthew.d.roper@intel.com> X-Mailer: git-send-email 2.14.4 In-Reply-To: <20181206170944.16539-1-matthew.d.roper@intel.com> References: <20181206170944.16539-1-matthew.d.roper@intel.com> Subject: [Intel-gfx] [PATCH 3/3] drm/i915: Switch to level-based DDB allocation algorithm X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 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-Virus-Scanned: ClamAV using ClamSMTP The DDB allocation algorithm currently used by the driver grants each plane a very small minimum allocation of DDB blocks and then divies up all of the remaining blocks based on the percentage of the total data rate that the plane makes up. It turns out that this proportional allocation approach is overly-generous with the larger planes and can leave very small planes wthout a big enough allocation to even hit their level 0 watermark requirements (especially on APL, which has a smaller DDB in general than other gen9 platforms). Or there can be situations where the smallest planes hit a lower watermark level than they should have been able to hit with a more equitable division of DDB blocks, thus limiting the overall system sleep state that can be achieved. The bspec now describes an alternate algorithm that can be used to overcome these types of issues. With the new algorithm, we calculate all plane watermark values for all wm levels first, then go back and partition a pipe's DDB space second. The DDB allocation will calculate what the highest watermark level that can be achieved on *all* active planes, and then grant the blocks necessary to hit that level to each plane. Any remaining blocks are then divided up proportionally according to data rate, similar to the old algorithm. There was a previous attempt to implement this algorithm a couple years ago in bb9d85f6e9d ("drm/i915/skl: New ddb allocation algorithm"), but some regressions were reported, the patch was reverted, and nobody ever got around to figuring out exactly where the bug was in that version. Our watermark code has evolved significantly in the meantime, but we're still getting bug reports caused by the unfair proportional algorithm, so let's give this another shot. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105458 Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/intel_pm.c | 345 ++++++++++++++-------------------------- 1 file changed, 121 insertions(+), 224 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index b09c2a257ff1..6c60a668c383 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4306,102 +4306,6 @@ icl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate, return total_data_rate; } -static uint16_t -skl_ddb_min_alloc(const struct drm_plane_state *pstate, const int plane) -{ - struct drm_framebuffer *fb = pstate->fb; - struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate); - uint32_t src_w, src_h; - uint32_t min_scanlines = 8; - uint8_t plane_bpp; - - if (WARN_ON(!fb)) - return 0; - - /* For packed formats, and uv-plane, return 0 */ - if (plane == 1 && fb->format->format != DRM_FORMAT_NV12) - return 0; - - /* For Non Y-tile return 8-blocks */ - if (fb->modifier != I915_FORMAT_MOD_Y_TILED && - fb->modifier != I915_FORMAT_MOD_Yf_TILED && - fb->modifier != I915_FORMAT_MOD_Y_TILED_CCS && - fb->modifier != I915_FORMAT_MOD_Yf_TILED_CCS) - return 8; - - /* - * Src coordinates are already rotated by 270 degrees for - * the 90/270 degree plane rotation cases (to match the - * GTT mapping), hence no need to account for rotation here. - */ - src_w = drm_rect_width(&intel_pstate->base.src) >> 16; - src_h = drm_rect_height(&intel_pstate->base.src) >> 16; - - /* Halve UV plane width and height for NV12 */ - if (plane == 1) { - src_w /= 2; - src_h /= 2; - } - - plane_bpp = fb->format->cpp[plane]; - - if (drm_rotation_90_or_270(pstate->rotation)) { - switch (plane_bpp) { - case 1: - min_scanlines = 32; - break; - case 2: - min_scanlines = 16; - break; - case 4: - min_scanlines = 8; - break; - case 8: - min_scanlines = 4; - break; - default: - WARN(1, "Unsupported pixel depth %u for rotation", - plane_bpp); - min_scanlines = 32; - } - } - - return DIV_ROUND_UP((4 * src_w * plane_bpp), 512) * min_scanlines/4 + 3; -} - -static void -skl_ddb_calc_min(const struct intel_crtc_state *cstate, int num_active, - uint16_t *minimum, uint16_t *uv_minimum) -{ - const struct drm_plane_state *pstate; - struct drm_plane *plane; - - drm_atomic_crtc_state_for_each_plane_state(plane, pstate, &cstate->base) { - enum plane_id plane_id = to_intel_plane(plane)->id; - struct intel_plane_state *plane_state = to_intel_plane_state(pstate); - - if (plane_id == PLANE_CURSOR) - continue; - - /* slave plane must be invisible and calculated from master */ - if (!pstate->visible || WARN_ON(plane_state->slave)) - continue; - - if (!plane_state->linked_plane) { - minimum[plane_id] = skl_ddb_min_alloc(pstate, 0); - uv_minimum[plane_id] = skl_ddb_min_alloc(pstate, 1); - } else { - enum plane_id y_plane_id = - plane_state->linked_plane->id; - - minimum[y_plane_id] = skl_ddb_min_alloc(pstate, 0); - minimum[plane_id] = skl_ddb_min_alloc(pstate, 1); - } - } - - minimum[PLANE_CURSOR] = skl_cursor_allocation(num_active); -} - static int skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, struct skl_ddb_allocation *ddb /* out */) @@ -4411,15 +4315,18 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, struct drm_i915_private *dev_priv = to_i915(crtc->dev); struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb; - uint16_t alloc_size, start; - uint16_t minimum[I915_MAX_PLANES] = {}; - uint16_t uv_minimum[I915_MAX_PLANES] = {}; + struct skl_ddb_entry *plane_alloc, *uv_plane_alloc; + struct skl_plane_wm *wm; + uint16_t alloc_size, start = 0; + uint16_t total[I915_MAX_PLANES] = {}; + uint16_t uv_total[I915_MAX_PLANES] = {}; u64 total_data_rate; enum plane_id plane_id; int num_active; u64 plane_data_rate[I915_MAX_PLANES] = {}; u64 uv_plane_data_rate[I915_MAX_PLANES] = {}; - uint16_t total_min_blocks = 0; + uint16_t blocks = 0; + int level; /* Clear the partitioning for disabled planes. */ memset(cstate->wm.skl.plane_ddb_y, 0, sizeof(cstate->wm.skl.plane_ddb_y)); @@ -4449,81 +4356,100 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, if (alloc_size == 0) return 0; - skl_ddb_calc_min(cstate, num_active, minimum, uv_minimum); + /* Allocate fixed number of blocks for cursor. */ + total[PLANE_CURSOR] = skl_cursor_allocation(num_active); + alloc_size -= total[PLANE_CURSOR]; /* - * 1. Allocate the mininum required blocks for each active plane - * and allocate the cursor, it doesn't require extra allocation - * proportional to the data rate. + * Find the highest watermark level for which we can satisfy the block + * requirement of active planes. */ + for (level = ilk_wm_max_level(dev_priv) - 1; level >= 0; level--) { + for_each_plane_id_on_crtc(intel_crtc, plane_id) { + wm = &cstate->wm.skl.optimal.planes[plane_id]; + blocks += wm->wm[level].plane_res_b; + blocks += wm->uv_wm[level].plane_res_b; + } - for_each_plane_id_on_crtc(intel_crtc, plane_id) { - total_min_blocks += minimum[plane_id]; - total_min_blocks += uv_minimum[plane_id]; + if (blocks < alloc_size) { + alloc_size -= blocks; + break; + } } - if (total_min_blocks > alloc_size) { + if (level < 0) { DRM_DEBUG_KMS("Requested display configuration exceeds system DDB limitations"); - DRM_DEBUG_KMS("minimum required %d/%d\n", total_min_blocks, - alloc_size); + DRM_DEBUG_KMS("minimum required %d/%d\n", blocks, + alloc_size); return -EINVAL; } - alloc_size -= total_min_blocks; - cstate->wm.skl.plane_ddb_y[PLANE_CURSOR].start = alloc->end - minimum[PLANE_CURSOR]; - cstate->wm.skl.plane_ddb_y[PLANE_CURSOR].end = alloc->end; - /* - * 2. Distribute the remaining space in proportion to the amount of - * data each plane needs to fetch from memory. - * - * FIXME: we may not allocate every single block here. + * If any non-cursor planes are turned on (i.e., total_data_rate != 0), + * grant each plane the blocks it requires at the highest achievable + * watermark level, plus an extra share of the leftover blocks + * proportional to its relative data rate. */ - if (total_data_rate == 0) - return 0; + if (total_data_rate) { + for_each_plane_id_on_crtc(intel_crtc, plane_id) { + u64 rate; - start = alloc->start; - for_each_plane_id_on_crtc(intel_crtc, plane_id) { - u64 data_rate, uv_data_rate; - uint16_t plane_blocks, uv_plane_blocks; - - if (plane_id == PLANE_CURSOR) - continue; + wm = &cstate->wm.skl.optimal.planes[plane_id]; - data_rate = plane_data_rate[plane_id]; + rate = plane_data_rate[plane_id]; + total[plane_id] = wm->wm[level].plane_res_b + + div64_u64(alloc_size * rate, total_data_rate); - /* - * allocation for (packed formats) or (uv-plane part of planar format): - * promote the expression to 64 bits to avoid overflowing, the - * result is < available as data_rate / total_data_rate < 1 - */ - plane_blocks = minimum[plane_id]; - plane_blocks += div64_u64(alloc_size * data_rate, total_data_rate); - - /* Leave disabled planes at (0,0) */ - if (data_rate) { - cstate->wm.skl.plane_ddb_y[plane_id].start = start; - cstate->wm.skl.plane_ddb_y[plane_id].end = start + plane_blocks; + rate = uv_plane_data_rate[plane_id]; + uv_total[plane_id] = wm->uv_wm[level].plane_res_b + + div64_u64(alloc_size * rate, total_data_rate); } + } - start += plane_blocks; + /* Set the actual DDB start/end points for each plane */ + start = alloc->start; + for_each_plane_id_on_crtc(intel_crtc, plane_id) { + plane_alloc = &cstate->wm.skl.plane_ddb_y[plane_id]; + uv_plane_alloc = &cstate->wm.skl.plane_ddb_uv[plane_id]; - /* Allocate DDB for UV plane for planar format/NV12 */ - uv_data_rate = uv_plane_data_rate[plane_id]; + /* Gen11+ uses a separate plane for UV watermarks */ + WARN_ON(INTEL_GEN(dev_priv) >= 11 && uv_total[plane_id]); - uv_plane_blocks = uv_minimum[plane_id]; - uv_plane_blocks += div64_u64(alloc_size * uv_data_rate, total_data_rate); + /* Leave disabled planes at (0,0) */ + if (total[plane_id]) { + plane_alloc->start = start; + plane_alloc->end = start += total[plane_id]; + } - /* Gen11+ uses a separate plane for UV watermarks */ - WARN_ON(INTEL_GEN(dev_priv) >= 11 && uv_plane_blocks); + if (uv_total[plane_id]) { + uv_plane_alloc->start = start; + uv_plane_alloc->end = start + uv_total[plane_id]; + } + } - if (uv_data_rate) { - cstate->wm.skl.plane_ddb_uv[plane_id].start = start; - cstate->wm.skl.plane_ddb_uv[plane_id].end = - start + uv_plane_blocks; + /* + * When we calculated watermark values we didn't know how high + * of a level we'd actually be able to hit, so we just marked + * all levels as "enabled." Go back now and disable the ones + * that aren't actually possible. + */ + for ( ; level < ilk_wm_max_level(dev_priv); level++) { + for_each_plane_id_on_crtc(intel_crtc, plane_id) { + wm = &cstate->wm.skl.optimal.planes[plane_id]; + wm->wm[level].plane_en = false; + wm->wm[level].plane_res_b = 0; + wm->wm[level].plane_res_l = 0; } + } - start += uv_plane_blocks; + /* + * Go back and disable the transition watermark if it turns out we + * don't have enough DDB blocks for it. + */ + for_each_plane_id_on_crtc(intel_crtc, plane_id) { + wm = &cstate->wm.skl.optimal.planes[plane_id]; + if (wm->trans_wm.plane_res_b > total[plane_id]) + wm->trans_wm.plane_en = false; } return 0; @@ -4720,17 +4646,15 @@ skl_compute_plane_wm_params(const struct intel_crtc_state *cstate, return 0; } -static int skl_compute_plane_wm(const struct intel_crtc_state *cstate, - const struct intel_plane_state *intel_pstate, - uint16_t ddb_allocation, - int level, - const struct skl_wm_params *wp, - const struct skl_wm_level *result_prev, - struct skl_wm_level *result /* out */) +static void skl_compute_plane_wm(const struct intel_crtc_state *cstate, + const struct intel_plane_state *intel_pstate, + int level, + const struct skl_wm_params *wp, + const struct skl_wm_level *result_prev, + struct skl_wm_level *result /* out */) { struct drm_i915_private *dev_priv = to_i915(intel_pstate->base.plane->dev); - const struct drm_plane_state *pstate = &intel_pstate->base; uint32_t latency = dev_priv->wm.skl_latency[level]; uint_fixed_16_16_t method1, method2; uint_fixed_16_16_t selected_result; @@ -4740,9 +4664,6 @@ static int skl_compute_plane_wm(const struct intel_crtc_state *cstate, bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state); uint32_t min_disp_buf_needed; - if (latency == 0) - return level == 0 ? -EINVAL : 0; - /* Display WA #1141: kbl,cfl */ if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) || IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) && @@ -4828,38 +4749,24 @@ static int skl_compute_plane_wm(const struct intel_crtc_state *cstate, min_disp_buf_needed = res_blocks; } - if ((level > 0 && res_lines > 31) || - res_blocks >= ddb_allocation || - min_disp_buf_needed >= ddb_allocation) { - /* - * If there are no valid level 0 watermarks, then we can't - * support this display configuration. - */ - if (level) { - return 0; - } else { - struct drm_plane *plane = pstate->plane; - - DRM_DEBUG_KMS("Requested display configuration exceeds system watermark limitations\n"); - DRM_DEBUG_KMS("[PLANE:%d:%s] blocks required = %u/%u, lines required = %u/31\n", - plane->base.id, plane->name, - res_blocks, ddb_allocation, res_lines); - return -EINVAL; - } - } - /* The number of lines are ignored for the level 0 watermark. */ + if (level > 0 && res_lines > 31) + return; + + /* + * If res_lines is valid, assume we can use this watermark level + * for now. We'll come back and disable it after we calculate the + * DDB allocation if it turns out we don't actually have enough + * blocks to satisfy it. + */ result->plane_res_b = res_blocks; result->plane_res_l = res_lines; result->plane_en = true; - - return 0; } -static int +static void skl_compute_wm_levels(const struct intel_crtc_state *cstate, const struct intel_plane_state *intel_pstate, - uint16_t ddb_blocks, const struct skl_wm_params *wm_params, struct skl_wm_level *levels) { @@ -4867,25 +4774,15 @@ skl_compute_wm_levels(const struct intel_crtc_state *cstate, to_i915(intel_pstate->base.plane->dev); int level, max_level = ilk_wm_max_level(dev_priv); struct skl_wm_level *result_prev = &levels[0]; - int ret; for (level = 0; level <= max_level; level++) { struct skl_wm_level *result = &levels[level]; - ret = skl_compute_plane_wm(cstate, - intel_pstate, - ddb_blocks, - level, - wm_params, - result_prev, - result); - if (ret) - return ret; + skl_compute_plane_wm(cstate, intel_pstate, level, wm_params, + result_prev, result); result_prev = result; } - - return 0; } static uint32_t @@ -4913,8 +4810,7 @@ skl_compute_linetime_wm(const struct intel_crtc_state *cstate) static void skl_compute_transition_wm(const struct intel_crtc_state *cstate, const struct skl_wm_params *wp, - struct skl_plane_wm *wm, - uint16_t ddb_allocation) + struct skl_plane_wm *wm) { struct drm_device *dev = cstate->base.crtc->dev; const struct drm_i915_private *dev_priv = to_i915(dev); @@ -4962,34 +4858,38 @@ static void skl_compute_transition_wm(const struct intel_crtc_state *cstate, } - res_blocks += 1; - - if (res_blocks < ddb_allocation) { - wm->trans_wm.plane_res_b = res_blocks; - wm->trans_wm.plane_en = true; - } + /* + * Just assume we can enable the transition watermark. After + * computing the DDB we'll come back and disable it if that + * assumption turns out to be false. + */ + wm->trans_wm.plane_res_b = res_blocks + 1; + wm->trans_wm.plane_en = true; } static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state, enum plane_id plane_id, int color_plane) { + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); struct skl_plane_wm *wm = &crtc_state->wm.skl.optimal.planes[plane_id]; - u16 ddb_blocks = skl_ddb_entry_size(&crtc_state->wm.skl.plane_ddb_y[plane_id]); struct skl_wm_params wm_params; int ret; + /* + * This can only happen if someone overrides the latency to an invalid + * value of 0 in debugfs. + */ + if (dev_priv->wm.skl_latency[0] == 0) + return -EINVAL; + ret = skl_compute_plane_wm_params(crtc_state, plane_state, &wm_params, color_plane); if (ret) return ret; - ret = skl_compute_wm_levels(crtc_state, plane_state, - ddb_blocks, &wm_params, wm->wm); - if (ret) - return ret; - - skl_compute_transition_wm(crtc_state, &wm_params, wm, ddb_blocks); + skl_compute_wm_levels(crtc_state, plane_state, &wm_params, wm->wm); + skl_compute_transition_wm(crtc_state, &wm_params, wm); return 0; } @@ -4999,7 +4899,6 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state, enum plane_id plane_id) { struct skl_plane_wm *wm = &crtc_state->wm.skl.optimal.planes[plane_id]; - u16 ddb_blocks = skl_ddb_entry_size(&crtc_state->wm.skl.plane_ddb_uv[plane_id]); struct skl_wm_params wm_params; int ret; @@ -5011,10 +4910,7 @@ static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state, if (ret) return ret; - ret = skl_compute_wm_levels(crtc_state, plane_state, - ddb_blocks, &wm_params, wm->uv_wm); - if (ret) - return ret; + skl_compute_wm_levels(crtc_state, plane_state, &wm_params, wm->uv_wm); return 0; } @@ -5532,13 +5428,9 @@ skl_compute_wm(struct drm_atomic_state *state) if (ret || !changed) return ret; - ret = skl_compute_ddb(state); - if (ret) - return ret; - /* * Calculate WM's for all pipes that are part of this transaction. - * Note that the DDB allocation above may have added more CRTC's that + * Note that skl_ddb_add_affected_pipes may have added more CRTC's that * weren't otherwise being modified (and set bits in dirty_pipes) if * pipe allocations had to change. */ @@ -5568,6 +5460,11 @@ skl_compute_wm(struct drm_atomic_state *state) intel_cstate->update_wm_pre = true; } + ret = skl_compute_ddb(state); + if (ret) + return ret; + + skl_print_wm_changes(intel_state); return 0;