From patchwork Wed Feb 28 08:02:12 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stanislav Lisovskiy X-Patchwork-Id: 13574979 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 9990AC54E41 for ; Wed, 28 Feb 2024 08:02:20 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 9649210E765; Wed, 28 Feb 2024 08:02:18 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="P/v4cB4U"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.9]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5BE6210E5AA for ; Wed, 28 Feb 2024 08:02:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1709107337; x=1740643337; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=tQ8GNLaqnghthgrBj2/T0hNI2CRG2UObGSbk/UUh2gc=; b=P/v4cB4UOAiYO9Q6v5ixWMcmRR4el8LaUqxPu4Gbc9B/54GvHtVnrUXW 1qfhYdqhoI+TkwzQOFWupD7chKZDHj+jPcOpoDvuOBL+5ac29MpW3Ygs9 dCeJEKDa7FTFZvlL5bxKOzQHOMFPfl770aXPqsyr6AfxF0rN6Vxsumfit o6O8aWqQECi3G4IV5qeZ1v6QWGXr4JLyGoS+yl9wjFVx5PV65cYVSeX+R K/XkPbgkLTeyUusxI/i6anSpiBvE5vc+nWCsrLHnW0/okuSsSaNlFjDww fR9WukI1Yp4e/k6olngv9+rPfH7O9BrH3UM2Kt+hNbYZJHut1G/Dcf8LO A==; X-IronPort-AV: E=McAfee;i="6600,9927,10996"; a="14205675" X-IronPort-AV: E=Sophos;i="6.06,190,1705392000"; d="scan'208";a="14205675" Received: from fmviesa007.fm.intel.com ([10.60.135.147]) by fmvoesa103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Feb 2024 00:02:17 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.06,190,1705392000"; d="scan'208";a="7220646" Received: from unknown (HELO slisovsk-Lenovo-ideapad-720S-13IKB.fi.intel.com) ([10.237.72.65]) by fmviesa007.fm.intel.com with ESMTP; 28 Feb 2024 00:02:15 -0800 From: Stanislav Lisovskiy To: intel-gfx@lists.freedesktop.org Cc: jani.saarinen@intel.com, Stanislav.Lisovskiy@intel.com, ville.syrjala@linux.intel.com Subject: [PATCH 1/2] drm/i915: Update mbus in intel_dbuf_mbus_update and do it properly Date: Wed, 28 Feb 2024 10:02:12 +0200 Message-Id: <20240228080213.17441-2-stanislav.lisovskiy@intel.com> X-Mailer: git-send-email 2.37.3 In-Reply-To: <20240228080213.17441-1-stanislav.lisovskiy@intel.com> References: <20240228080213.17441-1-stanislav.lisovskiy@intel.com> MIME-Version: 1.0 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: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" According to BSpec we need to do correspondent MBUS updates before or after DBUF reallocation, depending on whether we are reducing or increasing amount of pipes(typical scenario is swithing between multiple and single displays). As of BSpec 49213 if we are swithing from multiple to single display MBUS registers should be updated with correspondent values _before_ Dbuf reallocation happens, however if we are switching from single display to multiple then it should happen _after_ DDB reallocation(i.e plane programming). Signed-off-by: Stanislav Lisovskiy Tested-by: Paz Zcharya --- drivers/gpu/drm/i915/display/skl_watermark.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index c6b9be80d83c4..606b7ba9db9ce 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -3534,7 +3534,7 @@ int intel_dbuf_init(struct drm_i915_private *i915) * Configure MBUS_CTL and all DBUF_CTL_S of each slice to join_mbus state before * update the request state of all DBUS slices. */ -static void update_mbus_pre_enable(struct intel_atomic_state *state) +static void intel_dbuf_mbus_update(struct intel_atomic_state *state) { struct drm_i915_private *i915 = to_i915(state->base.dev); u32 mbus_ctl, dbuf_min_tracker_val; @@ -3584,7 +3584,10 @@ void intel_dbuf_pre_plane_update(struct intel_atomic_state *state) WARN_ON(!new_dbuf_state->base.changed); - update_mbus_pre_enable(state); + if ((hweight8(new_dbuf_state->active_pipes) <= hweight8(old_dbuf_state->active_pipes)) + || (old_dbuf_state->active_pipes == 0)) + intel_dbuf_mbus_update(state); + gen9_dbuf_slices_update(i915, old_dbuf_state->enabled_slices | new_dbuf_state->enabled_slices); @@ -3605,6 +3608,9 @@ void intel_dbuf_post_plane_update(struct intel_atomic_state *state) WARN_ON(!new_dbuf_state->base.changed); + if (hweight8(new_dbuf_state->active_pipes) > hweight8(old_dbuf_state->active_pipes)) + intel_dbuf_mbus_update(state); + gen9_dbuf_slices_update(i915, new_dbuf_state->enabled_slices); } From patchwork Wed Feb 28 08:02:13 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stanislav Lisovskiy X-Patchwork-Id: 13574980 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 30A59C54E41 for ; Wed, 28 Feb 2024 08:02:24 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id AB4F510E82D; Wed, 28 Feb 2024 08:02:23 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="Uo2Mwg9K"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.9]) by gabe.freedesktop.org (Postfix) with ESMTPS id E5E0E10E82D for ; Wed, 28 Feb 2024 08:02:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1709107339; x=1740643339; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=jcArGVqfVBtYHbDeQ1t0YBVzUpEFnZ4HapbN6Ks0QMQ=; b=Uo2Mwg9KWiArRbjkElglYu8Q8YAbecT9cUPj3fIOpG/CIllrd8bWO0FO EloYRVSM2ovyoMMOS2b+maH2xyikwJ/Gr0wfiLvZBCLdWm8pj+lpXRQsb 5JR73an39f//waNqTbAene9RZ2azXQ5iHEnc3t6urPBcvFyARtmMq5KZ8 1Fp+j/IlFGjPHiwGOKDWUnkda266n4HBNOB2NscphEmG359LsXVSXOX7y NqljNgkunqIWIFh/4ixbOyYqyTWYdNEeZFlB+iEqBuNgKfIfMcrPf33qn mr+f2bpsQzs2KlTZAj+hFazgvIT0jTtnhZ3OVDG4CC42QuAJG2T6tHl1f g==; X-IronPort-AV: E=McAfee;i="6600,9927,10996"; a="14205679" X-IronPort-AV: E=Sophos;i="6.06,190,1705392000"; d="scan'208";a="14205679" Received: from fmviesa007.fm.intel.com ([10.60.135.147]) by fmvoesa103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Feb 2024 00:02:19 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.06,190,1705392000"; d="scan'208";a="7220655" Received: from unknown (HELO slisovsk-Lenovo-ideapad-720S-13IKB.fi.intel.com) ([10.237.72.65]) by fmviesa007.fm.intel.com with ESMTP; 28 Feb 2024 00:02:17 -0800 From: Stanislav Lisovskiy To: intel-gfx@lists.freedesktop.org Cc: jani.saarinen@intel.com, Stanislav.Lisovskiy@intel.com, ville.syrjala@linux.intel.com Subject: [PATCH 2/2] drm/i915: Implement vblank synchronized MBUS join changes Date: Wed, 28 Feb 2024 10:02:13 +0200 Message-Id: <20240228080213.17441-3-stanislav.lisovskiy@intel.com> X-Mailer: git-send-email 2.37.3 In-Reply-To: <20240228080213.17441-1-stanislav.lisovskiy@intel.com> References: <20240228080213.17441-1-stanislav.lisovskiy@intel.com> MIME-Version: 1.0 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: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" Currently we can't change MBUS join status without doing a modeset, because we are lacking mechanism to synchronize those with vblank. However then this means that we can't do a fastset, if there is a need to change MBUS join state. Fix that by implementing such change. We already call correspondent check and update at pre_plane dbuf update, so the only thing left is to have a non-modeset version of that. If active pipes stay the same then fastset is possible and only MBUS join state/ddb allocation updates would be committed. v2: Implement additional changes according to BSpec. Vblank wait is needed after MBus/Dbuf programming in case if no modeset is done and we are switching from single to multiple displays, i.e mbus join state switches from "joined" to "non-joined" state. Otherwise vblank wait is not needed according to spec. v3: Split mbus and dbox programming into to pre/post plane update parts, how it should be done according to BSpec. v4: - Place "single display to multiple displays scenario" MBUS/DBOX programming after DDB reallocation, but before crtc enabling(that is where is has to be according to spec). - Check if crtc is still active, not only the old state. - Do a vblank wait if MBUX DBOX register was modified. - And of course do vblank wait only if crtc was active. - Do vblank wait only if we are not doing a modeset, if we are doing something before *commit_modeset_enables, because all crtcs might be disabled at this moment, so we will get WARN if try waiting for vblank then. - Still getting FIFO underrun so try waiting for vblank in pre_plane update as well. - Write also pipe that we need to sync with to MBUS_CTL register. v5: - Do vblank wait only for the first pipe, if mbus is joined - Check also if new/old_dbuf_state is not NULL, before getting single pipe and active pipes. Signed-off-by: Stanislav Lisovskiy Tested-by: Paz Zcharya --- drivers/gpu/drm/i915/display/intel_display.c | 13 ++- drivers/gpu/drm/i915/display/skl_watermark.c | 104 +++++++++++++++---- drivers/gpu/drm/i915/display/skl_watermark.h | 1 + 3 files changed, 96 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 00ac65a140298..989818f5d342f 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -6906,6 +6906,17 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state) } } + /* + * Some MBUS/DBuf update scenarios(mbus join disable) require that + * changes are done _after_ DDB reallocation, but _before_ crtc enabling. + * Typically we are disabling resources in post_plane/crtc_enable hooks, + * however in that case BSpec explicitly states that this should be done + * before we enable additional displays. + * FIXME: Should we still call this also there(post_plane hook) + * for extra-safety? If so, how to make sure, we don't call it twice? + */ + intel_dbuf_mbus_post_ddb_update(state); + update_pipes = modeset_pipes; /* @@ -7148,9 +7159,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state) } intel_encoders_update_prepare(state); - intel_dbuf_pre_plane_update(state); - intel_mbus_dbox_update(state); for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { if (new_crtc_state->do_async_flip) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index 606b7ba9db9ce..f0604ede399f7 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -2628,13 +2628,6 @@ skl_compute_ddb(struct intel_atomic_state *state) if (ret) return ret; - if (old_dbuf_state->joined_mbus != new_dbuf_state->joined_mbus) { - /* TODO: Implement vblank synchronized MBUS joining changes */ - ret = intel_modeset_all_pipes_late(state, "MBUS joining change"); - if (ret) - return ret; - } - drm_dbg_kms(&i915->drm, "Enabled dbuf slices 0x%x -> 0x%x (total dbuf slices 0x%x), mbus joined? %s->%s\n", old_dbuf_state->enabled_slices, @@ -3539,8 +3532,9 @@ static void intel_dbuf_mbus_update(struct intel_atomic_state *state) struct drm_i915_private *i915 = to_i915(state->base.dev); u32 mbus_ctl, dbuf_min_tracker_val; enum dbuf_slice slice; - const struct intel_dbuf_state *dbuf_state = + const struct intel_dbuf_state *new_dbuf_state = intel_atomic_get_new_dbuf_state(state); + enum pipe pipe = ffs(new_dbuf_state->active_pipes) - 1; if (!HAS_MBUS_JOINING(i915)) return; @@ -3549,13 +3543,13 @@ static void intel_dbuf_mbus_update(struct intel_atomic_state *state) * TODO: Implement vblank synchronized MBUS joining changes. * Must be properly coordinated with dbuf reprogramming. */ - if (dbuf_state->joined_mbus) { + if (new_dbuf_state->joined_mbus) { mbus_ctl = MBUS_HASHING_MODE_1x4 | MBUS_JOIN | - MBUS_JOIN_PIPE_SELECT_NONE; + MBUS_JOIN_PIPE_SELECT(pipe); dbuf_min_tracker_val = DBUF_MIN_TRACKER_STATE_SERVICE(3); } else { mbus_ctl = MBUS_HASHING_MODE_2x2 | - MBUS_JOIN_PIPE_SELECT_NONE; + MBUS_JOIN_PIPE_SELECT(pipe); dbuf_min_tracker_val = DBUF_MIN_TRACKER_STATE_SERVICE(1); } @@ -3576,21 +3570,35 @@ void intel_dbuf_pre_plane_update(struct intel_atomic_state *state) intel_atomic_get_new_dbuf_state(state); const struct intel_dbuf_state *old_dbuf_state = intel_atomic_get_old_dbuf_state(state); + int new_num_active_pipes = 0; + int old_num_active_pipes = 0; if (!new_dbuf_state || (new_dbuf_state->enabled_slices == old_dbuf_state->enabled_slices && new_dbuf_state->joined_mbus == old_dbuf_state->joined_mbus)) return; + new_num_active_pipes = hweight8(new_dbuf_state->active_pipes); + + if (old_dbuf_state) + old_num_active_pipes = hweight8(old_dbuf_state->active_pipes); + WARN_ON(!new_dbuf_state->base.changed); - if ((hweight8(new_dbuf_state->active_pipes) <= hweight8(old_dbuf_state->active_pipes)) - || (old_dbuf_state->active_pipes == 0)) + /* + * Switching from multiple to single display scenario(enable mbus join). + * Also we put here "<=" instead of "<" for suboptimal cases, when + * we switch from single => single display, enabling mbus join. + */ + if (new_num_active_pipes <= old_num_active_pipes) intel_dbuf_mbus_update(state); gen9_dbuf_slices_update(i915, old_dbuf_state->enabled_slices | new_dbuf_state->enabled_slices); + + if (new_num_active_pipes <= old_num_active_pipes) + intel_mbus_dbox_update(state); } void intel_dbuf_post_plane_update(struct intel_atomic_state *state) @@ -3608,13 +3616,59 @@ void intel_dbuf_post_plane_update(struct intel_atomic_state *state) WARN_ON(!new_dbuf_state->base.changed); - if (hweight8(new_dbuf_state->active_pipes) > hweight8(old_dbuf_state->active_pipes)) - intel_dbuf_mbus_update(state); - gen9_dbuf_slices_update(i915, new_dbuf_state->enabled_slices); } +void intel_dbuf_mbus_post_ddb_update(struct intel_atomic_state *state) +{ + const struct intel_dbuf_state *new_dbuf_state = + intel_atomic_get_new_dbuf_state(state); + const struct intel_dbuf_state *old_dbuf_state = + intel_atomic_get_old_dbuf_state(state); + enum pipe pipe; + int new_num_active_pipes = 0; + int old_num_active_pipes = 0; + + if (!new_dbuf_state || + (new_dbuf_state->enabled_slices == old_dbuf_state->enabled_slices && + new_dbuf_state->joined_mbus == old_dbuf_state->joined_mbus)) + return; + + pipe = ffs(new_dbuf_state->active_pipes) - 1; + new_num_active_pipes = hweight8(new_dbuf_state->active_pipes); + + if (old_dbuf_state) + old_num_active_pipes = hweight8(old_dbuf_state->active_pipes); + + WARN_ON(!new_dbuf_state->base.changed); + + /* + * Switching from single to multiple display scenario + */ + if (new_num_active_pipes > old_num_active_pipes) { + struct intel_crtc *crtc; + struct intel_crtc_state *new_crtc_state; + int i; + intel_dbuf_mbus_update(state); + intel_mbus_dbox_update(state); + + if (state->modeset) + return; + + for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { + /* + * According to BSpec we should wait vblank on single display, + * before enabling additional displays + */ + if (!new_crtc_state->hw.active || (crtc->pipe != pipe)) + continue; + + intel_crtc_wait_for_next_vblank(crtc); + } + } +} + static bool xelpdp_is_only_pipe_per_dbuf_bank(enum pipe pipe, u8 active_pipes) { switch (pipe) { @@ -3638,8 +3692,8 @@ void intel_mbus_dbox_update(struct intel_atomic_state *state) { struct drm_i915_private *i915 = to_i915(state->base.dev); const struct intel_dbuf_state *new_dbuf_state, *old_dbuf_state; - const struct intel_crtc_state *new_crtc_state; - const struct intel_crtc *crtc; + const struct intel_crtc_state *new_crtc_state, *old_crtc_state; + struct intel_crtc *crtc; u32 val = 0; int i; @@ -3685,12 +3739,14 @@ void intel_mbus_dbox_update(struct intel_atomic_state *state) val |= MBUS_DBOX_B_CREDIT(8); } - for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { - u32 pipe_val = val; + for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { + u32 pipe_val = val, old_pipe_val; if (!new_crtc_state->hw.active) continue; + old_pipe_val = intel_de_read(i915, PIPE_MBUS_DBOX_CTL(crtc->pipe)); + if (DISPLAY_VER(i915) >= 14) { if (xelpdp_is_only_pipe_per_dbuf_bank(crtc->pipe, new_dbuf_state->active_pipes)) @@ -3700,6 +3756,14 @@ void intel_mbus_dbox_update(struct intel_atomic_state *state) } intel_de_write(i915, PIPE_MBUS_DBOX_CTL(crtc->pipe), pipe_val); + + /* + * BSpec instructs us to wait for vblank, if credits were changed. + * However we need to do that here only if crtc was active prior to change. + */ + if (old_pipe_val != pipe_val && + old_crtc_state->hw.active && !state->modeset) + intel_crtc_wait_for_next_vblank(crtc); } } diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h index e3d1d74a7b170..5a97556a68c2c 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.h +++ b/drivers/gpu/drm/i915/display/skl_watermark.h @@ -73,6 +73,7 @@ intel_atomic_get_dbuf_state(struct intel_atomic_state *state); int intel_dbuf_init(struct drm_i915_private *i915); void intel_dbuf_pre_plane_update(struct intel_atomic_state *state); void intel_dbuf_post_plane_update(struct intel_atomic_state *state); +void intel_dbuf_mbus_post_ddb_update(struct intel_atomic_state *state); void intel_mbus_dbox_update(struct intel_atomic_state *state); #endif /* __SKL_WATERMARK_H__ */