From mboxrd@z Thu Jan 1 00:00:00 1970 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 1BB47CCD1A2 for ; Fri, 17 Oct 2025 12:06:09 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id AAA0B10EBCC; Fri, 17 Oct 2025 12:06:08 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="h9SsEDiY"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.19]) by gabe.freedesktop.org (Postfix) with ESMTPS id D0CB310EBCB; Fri, 17 Oct 2025 12:06:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1760702767; x=1792238767; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=t+Wz/W0pBcnwcWGdflsV4Dj1d1fbLd0AChS07XAvIXc=; b=h9SsEDiYUXSyJvuXJCZud++fv2S+Fe3LnMoztRIRBo3XV1CfdFCtAlgG ZbNvxpE1a6sFIxauZIHW3s0R1ap7SlCxfmi6JHzKiomh3to4L55d4ge3q HirzQNRs8Q4Y+5K+UkgTAM8oHQk6HA5ePH5r+bhnwFDJWzvZPrq1DMWqO fPeGspB7JK2O/SeJiecLyt+L1iyMl96FPAUSCC1WBEf4SJV+EOs+Ut13i 0CEtAvDSZ1naszpw4ITWkxpV6EV3gIYl+lRynVyf8YrGOB+edtT3RAOSt fLzRpDJKpvVDcnCM/kuHgCWlm8pfhbzRvi/0JoDkZmKBRS0QDG15t0gfn Q==; X-CSE-ConnectionGUID: Rd7zUIMLSaqEOFtRmC6NcQ== X-CSE-MsgGUID: jVJCqUXoTji9wrFxeArDfg== X-IronPort-AV: E=McAfee;i="6800,10657,11584"; a="62815160" X-IronPort-AV: E=Sophos;i="6.19,236,1754982000"; d="scan'208";a="62815160" Received: from orviesa001.jf.intel.com ([10.64.159.141]) by orvoesa111.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Oct 2025 05:06:06 -0700 X-CSE-ConnectionGUID: e77Nft2nSCid/9eJsIQdVA== X-CSE-MsgGUID: P+7/qxuASq2K/QUkbsWV8g== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.19,236,1754982000"; d="scan'208";a="219878369" Received: from klitkey1-mobl1.ger.corp.intel.com (HELO localhost) ([10.245.244.129]) by smtpauth.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Oct 2025 05:06:05 -0700 Date: Fri, 17 Oct 2025 15:06:02 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Ankit Nautiyal Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org Subject: Re: [PATCH 4/5] drm/i915/vrr: Use the min static optimized guardband Message-ID: References: <20251017050202.2211985-1-ankit.k.nautiyal@intel.com> <20251017050202.2211985-5-ankit.k.nautiyal@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20251017050202.2211985-5-ankit.k.nautiyal@intel.com> X-Patchwork-Hint: comment Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo 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" On Fri, Oct 17, 2025 at 10:32:01AM +0530, Ankit Nautiyal wrote: > In the current VRR implementation, vrr.vmin and vrr.guardband are set such > that they do not need to change when switching from fixed refresh rate to > variable refresh rate. Specifically, vrr.guardband is always set to match > the vblank length. This approach works for most cases, but not for LRR, > where the guardband would need to change while the VRR timing generator is > still active. > > With the VRR TG always active, live updates to guardband are unsafe and not > recommended. To ensure hardware safety, guardband was moved out of the > !fastset block, meaning any change now requires a full modeset. > This breaks seamless LRR switching, which was previously supported. > > Since the problem arises from guardband being matched to the vblank length, > solution is to use a minimal, sufficient static value, instead. So we use a > static guardband defined during mode-set that fits within the smallest > expected vblank and remains unchanged in case of features like LRR where > vtotal changes. To compute this minimum guardband we take into account > latencies/delays due to different features as mentioned in the Bspec. > > Introduce a helper to compute the minimal sufficient guardband. > On platforms where the VRR timing generator is always ON, we optimize the > guardband regardless of whether the display is operating in fixed or > variable refresh rate mode. > > v2: > - Use max of sagv latency and skl_wm_latency(1) for PM delay > computation. (Ville) > - Avoid guardband optimization for HDMI for now. (Ville) > - Allow guardband optimization only for platforms with > intel_vrr_always_use_vrr_tg = true. (Ville) > - Add comments for PM delay and a #TODO note for HDMI. > > Bspec: 70151 > Signed-off-by: Ankit Nautiyal > --- > drivers/gpu/drm/i915/display/intel_vrr.c | 62 +++++++++++++++++++++++- > 1 file changed, 60 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c > index 597008a6c744..cd7bed358984 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > @@ -6,12 +6,16 @@ > > #include > > +#include "intel_crtc.h" > #include "intel_de.h" > #include "intel_display_regs.h" > #include "intel_display_types.h" > #include "intel_dp.h" > +#include "intel_psr.h" > #include "intel_vrr.h" > #include "intel_vrr_regs.h" > +#include "skl_prefill.h" > +#include "skl_watermark.h" > > #define FIXED_POINT_PRECISION 100 > #define CMRR_PRECISION_TOLERANCE 10 > @@ -433,17 +437,71 @@ intel_vrr_max_guardband(struct intel_crtc_state *crtc_state) > intel_vrr_max_vblank_guardband(crtc_state)); > } > > +static > +int intel_vrr_compute_optimized_guardband(struct intel_crtc_state *crtc_state) > +{ > + struct intel_display *display = to_intel_display(crtc_state); > + struct skl_prefill_ctx prefill_ctx; > + int prefill_min_guardband; > + int prefill_latency_us; > + int guardband = 0; > + > + skl_prefill_init_worst(&prefill_ctx, crtc_state); > + > + /* > + * The SoC power controller runs SAGV mutually exclusive with package C states, > + * so the max of package C and SAGV latencies is used to compute the min prefill guardband. > + * PM delay = max(sagv_latency, pkgc_max_latency (highest enabled wm level 1 and up)) > + */ > + prefill_latency_us = max(display->sagv.block_time_us, > + skl_watermark_max_latency(display, 1)); > + prefill_min_guardband = > + skl_prefill_min_guardband(&prefill_ctx, > + crtc_state, > + prefill_latency_us); This could be just guardband = skl_prefill_min_guardband(...) and you can then get rid of the prefill_min_guardband variable as well. But in general lgtm Reviewed-by: Ville Syrjälä > + > + if (intel_crtc_has_dp_encoder(crtc_state)) { > + guardband = max(guardband, intel_psr_min_guardband(crtc_state)); > + guardband = max(guardband, intel_dp_sdp_min_guardband(crtc_state, true)); > + } > + > + guardband = max(guardband, prefill_min_guardband); > + > + return guardband; > +} > + > +static bool intel_vrr_use_optimized_guardband(const struct intel_crtc_state *crtc_state) > +{ > + struct intel_display *display = to_intel_display(crtc_state); > + > + /* > + * #TODO: Enable optimized guardband for HDMI > + * For HDMI lot of infoframes are transmitted a line or two after vsync. > + * Since with optimized guardband the double bufferring point is at delayed vblank, > + * we need to ensure that vsync happens after delayed vblank for the HDMI case. > + */ > + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) > + return false; > + > + return intel_vrr_always_use_vrr_tg(display); > +} > + > void intel_vrr_compute_guardband(struct intel_crtc_state *crtc_state) > { > struct intel_display *display = to_intel_display(crtc_state); > struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode; > struct drm_display_mode *pipe_mode = &crtc_state->hw.pipe_mode; > + int guardband; > > if (!intel_vrr_possible(crtc_state)) > return; > > - crtc_state->vrr.guardband = min(crtc_state->vrr.vmin - adjusted_mode->crtc_vdisplay, > - intel_vrr_max_guardband(crtc_state)); > + if (intel_vrr_use_optimized_guardband(crtc_state)) > + guardband = intel_vrr_compute_optimized_guardband(crtc_state); > + else > + guardband = crtc_state->vrr.vmin - adjusted_mode->crtc_vdisplay; > + > + crtc_state->vrr.guardband = min(guardband, intel_vrr_max_guardband(crtc_state)); > > if (intel_vrr_always_use_vrr_tg(display)) { > adjusted_mode->crtc_vblank_start = > -- > 2.45.2 -- Ville Syrjälä Intel