> -----Original Message----- > From: Intel-gfx <intel-gfx-boun...@lists.freedesktop.org> On Behalf Of Mitul > Golani > Sent: Thursday, May 9, 2024 1:28 PM > To: intel-gfx@lists.freedesktop.org > Cc: Shankar, Uma <uma.shan...@intel.com>; Nikula, Jani > <jani.nik...@intel.com> > Subject: [PATCH v8 3/7] drm/i915: Compute CMRR and calculate vtotal > > Compute Fixed Average Vtotal/CMRR with resepect to userspace VRR > enablement. Also calculate required parameters in case of CMRR is enabled. > During intel_vrr_compute_config, CMRR is getting enabled based on userspace > has enabled Adaptive Sync Vtotal mode (Legacy VRR) or not. Make CMRR as > small subset of FAVT mode, when Panle is running on Fixed refresh rate and on > VRR framework then only enable CMRR to match with actual refresh rate. > > --v2: > - Update is_cmrr_frac_required function return as bool, not int. [Jani] > - Use signed int math instead of unsigned in cmrr_get_vtotal2. [Jani] > - Fix typo and usage of camel case in cmrr_get_vtotal. [Jani] > - Use do_div in cmrr_get_vtotalwhile calculating cmrr_m. [ Jani] > - Simplify cmrr and vrr compute config in intel_vrr_compute_config. [Jani] > - Correct valiable name usage in is_cmrr_frac_required. [Ville] > > --v3: > - Removing RFC tag. > > --v4: > - Added edp check to address edp usecase for now. (ville) > - Updated is_cmrr_fraction_required to more simplified calculation. > - on longterm goal to be worked upon uapi as suggestion from ville. > > --v5: > - Correct vtotal paramas accuracy and add 2 digit precision. > - Avoid using DIV_ROUND_UP and improve scanline precision. > > --v6: > - Make CMRR a small subset of FAVT mode. > > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.gol...@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 1 + > .../drm/i915/display/intel_display_device.h | 1 + > drivers/gpu/drm/i915/display/intel_vrr.c | 99 ++++++++++++++++--- > 3 files changed, 89 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index 4a5318ab017d..8a76db59c85f 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -5458,6 +5458,7 @@ intel_pipe_config_compare(const struct > intel_crtc_state *current_config, > PIPE_CONF_CHECK_I(vrr.vsync_end); > PIPE_CONF_CHECK_LLI(cmrr.cmrr_m); > PIPE_CONF_CHECK_LLI(cmrr.cmrr_n); > + PIPE_CONF_CHECK_BOOL(cmrr.enable); > } > > #undef PIPE_CONF_CHECK_X > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h > b/drivers/gpu/drm/i915/display/intel_display_device.h > index 17ddf82f0b6e..b372b1acc19b 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_device.h > +++ b/drivers/gpu/drm/i915/display/intel_display_device.h > @@ -71,6 +71,7 @@ struct drm_printer; > BIT(trans)) != 0) > #define HAS_VRR(i915) (DISPLAY_VER(i915) >= 11) > #define HAS_AS_SDP(i915) (DISPLAY_VER(i915) >= 13) > +#define HAS_CMRR(i915) (DISPLAY_VER(i915) >= 20) > #define INTEL_NUM_PIPES(i915) > (hweight8(DISPLAY_RUNTIME_INFO(i915)->pipe_mask)) > #define I915_HAS_HOTPLUG(i915) (DISPLAY_INFO(i915)- > >has_hotplug) > #define OVERLAY_NEEDS_PHYSICAL(i915) (DISPLAY_INFO(i915)- > >overlay_needs_physical) > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c > b/drivers/gpu/drm/i915/display/intel_vrr.c > index 83ae56d22b5f..f5ba87fa00fe 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > @@ -11,6 +11,9 @@ > #include "intel_vrr.h" > #include "intel_dp.h" > > +#define FIXED_POINT_PRECISION 100 > +#define CMRR_PRECISION_TOLERANCE 10 > + > bool intel_vrr_is_capable(struct intel_connector *connector) { > const struct drm_display_info *info = &connector->base.display_info; > @@ -106,6 +109,59 @@ int intel_vrr_vmax_vblank_start(const struct > intel_crtc_state *crtc_state) > return crtc_state->vrr.vmax - intel_vrr_vblank_exit_length(crtc_state); > } > > +static bool > +is_cmrr_frac_required(struct intel_crtc_state *crtc_state, bool is_edp) > +{ > + int calculated_refresh_k, actual_refresh_k, pixel_clock_per_line; > + struct drm_display_mode *adjusted_mode = &crtc_state- > >hw.adjusted_mode; > + struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev); > + > + if (!(HAS_CMRR(i915) && is_edp)) > + return false; > + > + actual_refresh_k = > + drm_mode_vrefresh(adjusted_mode) * > FIXED_POINT_PRECISION; > + pixel_clock_per_line = > + adjusted_mode->crtc_clock * 1000 / adjusted_mode- > >crtc_htotal; > + calculated_refresh_k = > + pixel_clock_per_line * FIXED_POINT_PRECISION / > +adjusted_mode->crtc_vtotal; > + > + if ((actual_refresh_k - calculated_refresh_k) < > CMRR_PRECISION_TOLERANCE) > + return false; > + > + return true; > +} > + > +static unsigned int > +cmrr_get_vtotal(struct intel_crtc_state *crtc_state) { > + int multiplier_m = 1, multiplier_n = 1, vtotal; multiplier_m and multiplier_n have constant value and are not changing. Can this be a macro?
Thanks and Regards, Arun R Murthy -------------------- > + int actual_refresh_rate, desired_refresh_rate; > + long long actual_pixel_rate, adjusted_pixel_rate, pixel_clock_per_line; > + struct drm_display_mode *adjusted_mode = > +&crtc_state->hw.adjusted_mode; > + > + pixel_clock_per_line = > + adjusted_mode->crtc_clock * 1000 / adjusted_mode- > >crtc_htotal; > + actual_refresh_rate = > + pixel_clock_per_line * FIXED_POINT_PRECISION / > adjusted_mode->crtc_vtotal; > + desired_refresh_rate = drm_mode_vrefresh(adjusted_mode); > + actual_pixel_rate = actual_refresh_rate * adjusted_mode->crtc_vtotal; > + actual_pixel_rate = > + (actual_pixel_rate * adjusted_mode->crtc_htotal) / > +FIXED_POINT_PRECISION; > + > + multiplier_m = 1001; > + multiplier_n = 1000; > + > + crtc_state->cmrr.cmrr_n = > + ((desired_refresh_rate * adjusted_mode->crtc_htotal * > multiplier_n) / multiplier_m); > + crtc_state->cmrr.cmrr_n *= multiplier_n; > + vtotal = (actual_pixel_rate * multiplier_n) / crtc_state->cmrr.cmrr_n; > + adjusted_pixel_rate = actual_pixel_rate * multiplier_m; > + crtc_state->cmrr.cmrr_m = do_div(adjusted_pixel_rate, > +crtc_state->cmrr.cmrr_n); > + > + return vtotal; > +} > + > void > intel_vrr_compute_config(struct intel_crtc_state *crtc_state, > struct drm_connector_state *conn_state) @@ -115,6 > +171,7 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state, > struct intel_connector *connector = > to_intel_connector(conn_state->connector); > struct intel_dp *intel_dp = intel_attached_dp(connector); > + bool is_edp = intel_dp_is_edp(intel_dp); > struct drm_display_mode *adjusted_mode = &crtc_state- > >hw.adjusted_mode; > const struct drm_display_info *info = &connector->base.display_info; > int vmin, vmax; > @@ -159,18 +216,10 @@ intel_vrr_compute_config(struct intel_crtc_state > *crtc_state, > crtc_state->vrr.flipline = crtc_state->vrr.vmin + 1; > > /* > - * For XE_LPD+, we use guardband and pipeline override > - * is deprecated. > + * When panel is VRR capable and userspace has > + * not enabled adaptive sync mode then Fixed Average > + * Vtotal mode should be enabled. > */ > - if (DISPLAY_VER(i915) >= 13) { > - crtc_state->vrr.guardband = > - crtc_state->vrr.vmin + 1 - adjusted_mode- > >crtc_vblank_start; > - } else { > - crtc_state->vrr.pipeline_full = > - min(255, crtc_state->vrr.vmin - adjusted_mode- > >crtc_vblank_start - > - crtc_state->framestart_delay - 1); > - } > - > if (crtc_state->uapi.vrr_enabled) { > crtc_state->vrr.enable = true; > crtc_state->mode_flags |= I915_MODE_FLAG_VRR; @@ -182,6 > +231,25 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state, > (crtc_state->hw.adjusted_mode.crtc_vtotal - > crtc_state- > >hw.adjusted_mode.vsync_end); > } > + } else if (is_cmrr_frac_required(crtc_state, is_edp)) { > + crtc_state->cmrr.enable = true; > + crtc_state->vrr.vmax = cmrr_get_vtotal(crtc_state); > + crtc_state->vrr.vmin = crtc_state->vrr.vmax; > + crtc_state->vrr.flipline = crtc_state->vrr.vmin; > + crtc_state->mode_flags |= I915_MODE_FLAG_VRR; > + } > + > + /* > + * For XE_LPD+, we use guardband and pipeline override > + * is deprecated. > + */ > + if (DISPLAY_VER(i915) >= 13) { > + crtc_state->vrr.guardband = > + crtc_state->vrr.vmin + 1 - adjusted_mode- > >crtc_vblank_start; > + } else { > + crtc_state->vrr.pipeline_full = > + min(255, crtc_state->vrr.vmin - adjusted_mode- > >crtc_vblank_start - > + crtc_state->framestart_delay - 1); > } > } > > @@ -317,7 +385,14 @@ void intel_vrr_get_config(struct intel_crtc_state > *crtc_state) > > trans_vrr_ctl = intel_de_read(dev_priv, > TRANS_VRR_CTL(cpu_transcoder)); > > - crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE; > + if (HAS_CMRR(dev_priv)) { > + crtc_state->cmrr.enable = (trans_vrr_ctl & > VRR_CTL_CMRR_ENABLE) && > + (trans_vrr_ctl & > VRR_CTL_VRR_ENABLE); > + crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE > && > + !(trans_vrr_ctl & > VRR_CTL_CMRR_ENABLE); > + } else { > + crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE; > + } > > if (crtc_state->cmrr.enable) { > crtc_state->cmrr.cmrr_n = > -- > 2.25.1