Hi @Nathan Chancellor Probably fix is merged in drm-intel-next related patch: https://patchwork.freedesktop.org/series/134860/ Can you please check and suggest if this patch is merged ? Thanks, Mitul > -----Original Message----- > From: Nathan Chancellor <nathan@xxxxxxxxxx> > Sent: Wednesday, June 19, 2024 9:12 PM > To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@xxxxxxxxx> > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Nautiyal, Ankit K > <ankit.k.nautiyal@xxxxxxxxx>; intel-xe@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH v16 9/9] drm/i915: Compute CMRR and calculate vtotal > > Hi Mitul, > > On Mon, Jun 10, 2024 at 12:52:02PM +0530, Mitul Golani wrote: > ... > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c > > b/drivers/gpu/drm/i915/display/intel_vrr.c > > index 4ad99a54aa83..05f67dc9d98d 100644 > > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > > @@ -12,6 +12,9 @@ > > #include "intel_vrr_regs.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; > > @@ -107,6 +110,52 @@ 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) { > > + 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)) > > + 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, bool > > +video_mode_required) { > > + int multiplier_m = 1, multiplier_n = 1, vtotal, desired_refresh_rate; > > + long long adjusted_pixel_rate; > > + struct drm_display_mode *adjusted_mode = > > +&crtc_state->hw.adjusted_mode; > > + > > + desired_refresh_rate = drm_mode_vrefresh(adjusted_mode); > > + > > + if (video_mode_required) { > > + multiplier_m = 1001; > > + multiplier_n = 1000; > > + } > > + > > + crtc_state->cmrr.cmrr_n = > > + desired_refresh_rate * adjusted_mode->crtc_htotal * > multiplier_n; > > + vtotal = (adjusted_mode->crtc_clock * 1000 * multiplier_n) / > crtc_state->cmrr.cmrr_n; > > + adjusted_pixel_rate = adjusted_mode->crtc_clock * 1000 * > multiplier_m; > > + crtc_state->cmrr.cmrr_m = do_div(adjusted_pixel_rate, > > +crtc_state->cmrr.cmrr_n); > > + > > + return vtotal; > > +} > > This change is now in -next as commit 1676ecd303ac ("drm/i915: Compute > CMRR and calculate vtotal"), where it breaks the xe build for 32-bit platforms > with: > > $ make -skj"$(nproc)" ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- > allmodconfig drivers/gpu/drm/xe/i915-display/intel_vrr.o > In file included from arch/arm/include/asm/div64.h:107, > from include/linux/math.h:6, > from include/linux/kernel.h:27, > from include/linux/cpumask.h:11, > from include/linux/smp.h:13, > from include/linux/lockdep.h:14, > from include/linux/spinlock.h:63, > from include/linux/kref.h:16, > from include/drm/drm_device.h:5, > from include/drm/drm_drv.h:35, > from drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h:13, > from drivers/gpu/drm/i915/display/intel_vrr.c:7: > drivers/gpu/drm/i915/display/intel_vrr.c: In function 'cmrr_get_vtotal': > include/asm-generic/div64.h:222:35: error: comparison of distinct pointer > types lacks a cast [-Werror] > 222 | (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ > | ^~ > drivers/gpu/drm/i915/display/intel_vrr.c:155:35: note: in expansion of macro > 'do_div' > 155 | crtc_state->cmrr.cmrr_m = do_div(adjusted_pixel_rate, crtc_state- > >cmrr.cmrr_n); > | ^~~~~~ > cc1: all warnings being treated as errors > > Also, is do_div() correct here? It is different from the other div_() macros in that > the "return value" is the remainder, not the result of the division. > > Cheers, > Nathan
