Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v16 9/9] drm/i915: Compute CMRR and calculate vtotal
       [not found] ` <20240610072203.24956-10-mitulkumar.ajitkumar.golani@intel.com>
@ 2024-06-19 15:42   ` Nathan Chancellor
  2024-06-19 18:10     ` Golani, Mitulkumar Ajitkumar
  0 siblings, 1 reply; 6+ messages in thread
From: Nathan Chancellor @ 2024-06-19 15:42 UTC (permalink / raw)
  To: Mitul Golani; +Cc: intel-gfx, ankit.k.nautiyal, intel-xe

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v16 9/9] drm/i915: Compute CMRR and calculate vtotal
  2024-06-19 15:42   ` [PATCH v16 9/9] drm/i915: Compute CMRR and calculate vtotal Nathan Chancellor
@ 2024-06-19 18:10     ` Golani, Mitulkumar Ajitkumar
  2024-06-19 18:26       ` Nathan Chancellor
  0 siblings, 1 reply; 6+ messages in thread
From: Golani, Mitulkumar Ajitkumar @ 2024-06-19 18:10 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: intel-gfx@lists.freedesktop.org, Nautiyal, Ankit K,
	intel-xe@lists.freedesktop.org

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@kernel.org>
> Sent: Wednesday, June 19, 2024 9:12 PM
> To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Nautiyal, Ankit K
> <ankit.k.nautiyal@intel.com>; intel-xe@lists.freedesktop.org
> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v16 9/9] drm/i915: Compute CMRR and calculate vtotal
  2024-06-19 18:10     ` Golani, Mitulkumar Ajitkumar
@ 2024-06-19 18:26       ` Nathan Chancellor
  2024-06-20 10:05         ` Golani, Mitulkumar Ajitkumar
  0 siblings, 1 reply; 6+ messages in thread
From: Nathan Chancellor @ 2024-06-19 18:26 UTC (permalink / raw)
  To: Golani, Mitulkumar Ajitkumar
  Cc: intel-gfx@lists.freedesktop.org, Nautiyal, Ankit K,
	intel-xe@lists.freedesktop.org

On Wed, Jun 19, 2024 at 06:10:34PM +0000, Golani, Mitulkumar Ajitkumar wrote:
> 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 ?

This is still reproducible at commit 851de367dede ("drm/i915: Enable
plane/pipeDMC ATS fault interrupts on mtl") in drm-intel-next, which
includes that change as commit e2dc7cb72b25 ("drm/i915/display: Update
calculation to avoid overflow"). The issue is the dividend in do_div()
is required to be an unsigned 64-bit type but you used a signed type.
Updating adjusted_pixel_rate to be a u64 should resolve the issue and
match the return type of mul_u32_u32(). I just wasn't sure if that was
the only fix this code would need, as do_div() is not typically used
with an assignment.

Cheers,
Nathan

> > -----Original Message-----
> > From: Nathan Chancellor <nathan@kernel.org>
> > Sent: Wednesday, June 19, 2024 9:12 PM
> > To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; Nautiyal, Ankit K
> > <ankit.k.nautiyal@intel.com>; intel-xe@lists.freedesktop.org
> > 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v16 9/9] drm/i915: Compute CMRR and calculate vtotal
  2024-06-19 18:26       ` Nathan Chancellor
@ 2024-06-20 10:05         ` Golani, Mitulkumar Ajitkumar
  2024-06-20 12:59           ` Jani Nikula
  0 siblings, 1 reply; 6+ messages in thread
From: Golani, Mitulkumar Ajitkumar @ 2024-06-20 10:05 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: intel-gfx@lists.freedesktop.org, Nautiyal, Ankit K,
	intel-xe@lists.freedesktop.org, Kurmi, Suresh Kumar, Shankar, Uma,
	Nikula, Jani

Hi @Nathan Chancellor,

Yes, with do_div, we are expecting the remainder value. Regarding the warning related to the adjusted_pixel_rate type cast, I haven't been able to reproduce this locally, possibly due to differences in the cross-compiler. We should consider typecasting adjusted_pixel_rate or treating it as unsigned ?

Adding @Nikula, Jani to suggest.

Regards,
Mitul
> -----Original Message-----
> From: Nathan Chancellor <nathan@kernel.org>
> Sent: Wednesday, June 19, 2024 11:56 PM
> To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Nautiyal, Ankit K
> <ankit.k.nautiyal@intel.com>; intel-xe@lists.freedesktop.org
> Subject: Re: [PATCH v16 9/9] drm/i915: Compute CMRR and calculate vtotal
> 
> On Wed, Jun 19, 2024 at 06:10:34PM +0000, Golani, Mitulkumar Ajitkumar
> wrote:
> > 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 ?
> 
> This is still reproducible at commit 851de367dede ("drm/i915: Enable
> plane/pipeDMC ATS fault interrupts on mtl") in drm-intel-next, which includes
> that change as commit e2dc7cb72b25 ("drm/i915/display: Update calculation
> to avoid overflow"). The issue is the dividend in do_div() is required to be an
> unsigned 64-bit type but you used a signed type.
> Updating adjusted_pixel_rate to be a u64 should resolve the issue and match
> the return type of mul_u32_u32(). I just wasn't sure if that was the only fix
> this code would need, as do_div() is not typically used with an assignment.
> 
> Cheers,
> Nathan
> 
> > > -----Original Message-----
> > > From: Nathan Chancellor <nathan@kernel.org>
> > > Sent: Wednesday, June 19, 2024 9:12 PM
> > > To: Golani, Mitulkumar Ajitkumar
> > > <mitulkumar.ajitkumar.golani@intel.com>
> > > Cc: intel-gfx@lists.freedesktop.org; Nautiyal, Ankit K
> > > <ankit.k.nautiyal@intel.com>; intel-xe@lists.freedesktop.org
> > > 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v16 9/9] drm/i915: Compute CMRR and calculate vtotal
  2024-06-20 10:05         ` Golani, Mitulkumar Ajitkumar
@ 2024-06-20 12:59           ` Jani Nikula
  2024-06-20 14:15             ` Nathan Chancellor
  0 siblings, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2024-06-20 12:59 UTC (permalink / raw)
  To: Golani, Mitulkumar Ajitkumar, Nathan Chancellor
  Cc: intel-gfx@lists.freedesktop.org, Nautiyal, Ankit K,
	intel-xe@lists.freedesktop.org, Kurmi, Suresh Kumar, Shankar, Uma

On Thu, 20 Jun 2024, "Golani, Mitulkumar Ajitkumar" <mitulkumar.ajitkumar.golani@intel.com> wrote:
> Hi @Nathan Chancellor,
>
> Yes, with do_div, we are expecting the remainder value. Regarding the
> warning related to the adjusted_pixel_rate type cast, I haven't been
> able to reproduce this locally, possibly due to differences in the
> cross-compiler. We should consider typecasting adjusted_pixel_rate or
> treating it as unsigned ?

Please avoid top-posting on the mailing lists.

I'm guessing this will be enough.

diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 6430da25957d..5a0da64c7db3 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -137,7 +137,7 @@ 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;
+	u64 adjusted_pixel_rate;
 	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
 
 	desired_refresh_rate = drm_mode_vrefresh(adjusted_mode);


BR,
Jani.

>
> Adding @Nikula, Jani to suggest.
>
> Regards,
> Mitul
>> -----Original Message-----
>> From: Nathan Chancellor <nathan@kernel.org>
>> Sent: Wednesday, June 19, 2024 11:56 PM
>> To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org; Nautiyal, Ankit K
>> <ankit.k.nautiyal@intel.com>; intel-xe@lists.freedesktop.org
>> Subject: Re: [PATCH v16 9/9] drm/i915: Compute CMRR and calculate vtotal
>>
>> On Wed, Jun 19, 2024 at 06:10:34PM +0000, Golani, Mitulkumar Ajitkumar
>> wrote:
>> > 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 ?
>>
>> This is still reproducible at commit 851de367dede ("drm/i915: Enable
>> plane/pipeDMC ATS fault interrupts on mtl") in drm-intel-next, which includes
>> that change as commit e2dc7cb72b25 ("drm/i915/display: Update calculation
>> to avoid overflow"). The issue is the dividend in do_div() is required to be an
>> unsigned 64-bit type but you used a signed type.
>> Updating adjusted_pixel_rate to be a u64 should resolve the issue and match
>> the return type of mul_u32_u32(). I just wasn't sure if that was the only fix
>> this code would need, as do_div() is not typically used with an assignment.
>>
>> Cheers,
>> Nathan
>>
>> > > -----Original Message-----
>> > > From: Nathan Chancellor <nathan@kernel.org>
>> > > Sent: Wednesday, June 19, 2024 9:12 PM
>> > > To: Golani, Mitulkumar Ajitkumar
>> > > <mitulkumar.ajitkumar.golani@intel.com>
>> > > Cc: intel-gfx@lists.freedesktop.org; Nautiyal, Ankit K
>> > > <ankit.k.nautiyal@intel.com>; intel-xe@lists.freedesktop.org
>> > > 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

-- 
Jani Nikula, Intel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v16 9/9] drm/i915: Compute CMRR and calculate vtotal
  2024-06-20 12:59           ` Jani Nikula
@ 2024-06-20 14:15             ` Nathan Chancellor
  0 siblings, 0 replies; 6+ messages in thread
From: Nathan Chancellor @ 2024-06-20 14:15 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Golani, Mitulkumar Ajitkumar, intel-gfx@lists.freedesktop.org,
	Nautiyal, Ankit K, intel-xe@lists.freedesktop.org,
	Kurmi, Suresh Kumar, Shankar, Uma

On Thu, Jun 20, 2024 at 03:59:03PM +0300, Jani Nikula wrote:
> On Thu, 20 Jun 2024, "Golani, Mitulkumar Ajitkumar" <mitulkumar.ajitkumar.golani@intel.com> wrote:
> > Hi @Nathan Chancellor,
> >
> > Yes, with do_div, we are expecting the remainder value. Regarding the
> > warning related to the adjusted_pixel_rate type cast, I haven't been
> > able to reproduce this locally, possibly due to differences in the
> > cross-compiler. We should consider typecasting adjusted_pixel_rate or
> > treating it as unsigned ?
> 
> Please avoid top-posting on the mailing lists.
> 
> I'm guessing this will be enough.

Indeed, that works.

> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 6430da25957d..5a0da64c7db3 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -137,7 +137,7 @@ 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;
> +	u64 adjusted_pixel_rate;
>  	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
>  
>  	desired_refresh_rate = drm_mode_vrefresh(adjusted_mode);
> 
> 
> BR,
> Jani.
> 
> >
> > Adding @Nikula, Jani to suggest.
> >
> > Regards,
> > Mitul
> >> -----Original Message-----
> >> From: Nathan Chancellor <nathan@kernel.org>
> >> Sent: Wednesday, June 19, 2024 11:56 PM
> >> To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>
> >> Cc: intel-gfx@lists.freedesktop.org; Nautiyal, Ankit K
> >> <ankit.k.nautiyal@intel.com>; intel-xe@lists.freedesktop.org
> >> Subject: Re: [PATCH v16 9/9] drm/i915: Compute CMRR and calculate vtotal
> >>
> >> On Wed, Jun 19, 2024 at 06:10:34PM +0000, Golani, Mitulkumar Ajitkumar
> >> wrote:
> >> > 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 ?
> >>
> >> This is still reproducible at commit 851de367dede ("drm/i915: Enable
> >> plane/pipeDMC ATS fault interrupts on mtl") in drm-intel-next, which includes
> >> that change as commit e2dc7cb72b25 ("drm/i915/display: Update calculation
> >> to avoid overflow"). The issue is the dividend in do_div() is required to be an
> >> unsigned 64-bit type but you used a signed type.
> >> Updating adjusted_pixel_rate to be a u64 should resolve the issue and match
> >> the return type of mul_u32_u32(). I just wasn't sure if that was the only fix
> >> this code would need, as do_div() is not typically used with an assignment.
> >>
> >> Cheers,
> >> Nathan
> >>
> >> > > -----Original Message-----
> >> > > From: Nathan Chancellor <nathan@kernel.org>
> >> > > Sent: Wednesday, June 19, 2024 9:12 PM
> >> > > To: Golani, Mitulkumar Ajitkumar
> >> > > <mitulkumar.ajitkumar.golani@intel.com>
> >> > > Cc: intel-gfx@lists.freedesktop.org; Nautiyal, Ankit K
> >> > > <ankit.k.nautiyal@intel.com>; intel-xe@lists.freedesktop.org
> >> > > 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
> 
> -- 
> Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-06-20 14:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20240610072203.24956-1-mitulkumar.ajitkumar.golani@intel.com>
     [not found] ` <20240610072203.24956-10-mitulkumar.ajitkumar.golani@intel.com>
2024-06-19 15:42   ` [PATCH v16 9/9] drm/i915: Compute CMRR and calculate vtotal Nathan Chancellor
2024-06-19 18:10     ` Golani, Mitulkumar Ajitkumar
2024-06-19 18:26       ` Nathan Chancellor
2024-06-20 10:05         ` Golani, Mitulkumar Ajitkumar
2024-06-20 12:59           ` Jani Nikula
2024-06-20 14:15             ` Nathan Chancellor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox