diff mbox

[v2,3/4] drm/i915: fix 64bit divide

Message ID 20171113181902.12411-4-lionel.g.landwerlin@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lionel Landwerlin Nov. 13, 2017, 6:19 p.m. UTC
ERROR: "__udivdi3" [drivers/gpu/drm/i915/i915.ko] undefined!
ERROR: "__divdi3" [drivers/gpu/drm/i915/i915.ko] undefined!

We can also drop an if() as we divide by (value + 1) only if value is
not 0.

Fixes: dab9178333 ("drm/i915: expose command stream timestamp frequency to userspace")
Reported-by: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/intel_device_info.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Matthew Auld Nov. 13, 2017, 6:44 p.m. UTC | #1
On 13 November 2017 at 18:19, Lionel Landwerlin
<lionel.g.landwerlin@intel.com> wrote:
> ERROR: "__udivdi3" [drivers/gpu/drm/i915/i915.ko] undefined!
> ERROR: "__divdi3" [drivers/gpu/drm/i915/i915.ko] undefined!
>
> We can also drop an if() as we divide by (value + 1) only if value is
> not 0.
>
> Fixes: dab9178333 ("drm/i915: expose command stream timestamp frequency to userspace")
> Reported-by: Matthew Auld <matthew.auld@intel.com>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_device_info.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 78bf7374fbdd..992ae1bdfb3b 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -336,13 +336,12 @@ static u64 read_reference_ts_freq(struct drm_i915_private *dev_priv)
>
>         base_freq = ((ts_override & GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_MASK) >>
>                      GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_SHIFT) + 1;
> -       base_freq *= 1000000;
> +       base_freq *= 1000000ULL;
>
>         frac_freq = ((ts_override &
>                       GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_MASK) >>
>                      GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_SHIFT);
> -       if (frac_freq != 0)
> -               frac_freq = 1000000 / (frac_freq + 1);
> +       frac_freq = div_u64(1000000ULL, frac_freq + 1);
s/div_u64/div64_u64/ ?

>
>         return base_freq + frac_freq;
>  }
> @@ -360,7 +359,7 @@ static u64 read_timestamp_frequency(struct drm_i915_private *dev_priv)
>                  *      hclks." (through the “Clocking Configuration”
>                  *      (“CLKCFG”) MCHBAR register)
>                  */
> -               return (dev_priv->rawclk_freq * 1000) / 16;
> +               return div_u64(dev_priv->rawclk_freq * 1000ULL, 16);
Why do we need this?

>         } else if (INTEL_GEN(dev_priv) <= 8) {
>                 /* PRMs say:
>                  *
> --
> 2.15.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Lionel Landwerlin Nov. 13, 2017, 6:50 p.m. UTC | #2
On 13/11/17 18:44, Matthew Auld wrote:
> On 13 November 2017 at 18:19, Lionel Landwerlin
> <lionel.g.landwerlin@intel.com> wrote:
>> ERROR: "__udivdi3" [drivers/gpu/drm/i915/i915.ko] undefined!
>> ERROR: "__divdi3" [drivers/gpu/drm/i915/i915.ko] undefined!
>>
>> We can also drop an if() as we divide by (value + 1) only if value is
>> not 0.
>>
>> Fixes: dab9178333 ("drm/i915: expose command stream timestamp frequency to userspace")
>> Reported-by: Matthew Auld <matthew.auld@intel.com>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_device_info.c | 7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
>> index 78bf7374fbdd..992ae1bdfb3b 100644
>> --- a/drivers/gpu/drm/i915/intel_device_info.c
>> +++ b/drivers/gpu/drm/i915/intel_device_info.c
>> @@ -336,13 +336,12 @@ static u64 read_reference_ts_freq(struct drm_i915_private *dev_priv)
>>
>>          base_freq = ((ts_override & GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_MASK) >>
>>                       GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_SHIFT) + 1;
>> -       base_freq *= 1000000;
>> +       base_freq *= 1000000ULL;
>>
>>          frac_freq = ((ts_override &
>>                        GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_MASK) >>
>>                       GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_SHIFT);
>> -       if (frac_freq != 0)
>> -               frac_freq = 1000000 / (frac_freq + 1);
>> +       frac_freq = div_u64(1000000ULL, frac_freq + 1);
> s/div_u64/div64_u64/ ?

Oh right!

>
>>          return base_freq + frac_freq;
>>   }
>> @@ -360,7 +359,7 @@ static u64 read_timestamp_frequency(struct drm_i915_private *dev_priv)
>>                   *      hclks." (through the “Clocking Configuration”
>>                   *      (“CLKCFG”) MCHBAR register)
>>                   */
>> -               return (dev_priv->rawclk_freq * 1000) / 16;
>> +               return div_u64(dev_priv->rawclk_freq * 1000ULL, 16);
> Why do we need this?

It was just for consistency as we return a u64. I can drop it if you want.

>
>>          } else if (INTEL_GEN(dev_priv) <= 8) {
>>                  /* PRMs say:
>>                   *
>> --
>> 2.15.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjälä Nov. 13, 2017, 8:50 p.m. UTC | #3
On Mon, Nov 13, 2017 at 06:44:47PM +0000, Matthew Auld wrote:
> On 13 November 2017 at 18:19, Lionel Landwerlin
> <lionel.g.landwerlin@intel.com> wrote:
> > ERROR: "__udivdi3" [drivers/gpu/drm/i915/i915.ko] undefined!
> > ERROR: "__divdi3" [drivers/gpu/drm/i915/i915.ko] undefined!
> >
> > We can also drop an if() as we divide by (value + 1) only if value is
> > not 0.
> >
> > Fixes: dab9178333 ("drm/i915: expose command stream timestamp frequency to userspace")
> > Reported-by: Matthew Auld <matthew.auld@intel.com>
> > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_device_info.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> > index 78bf7374fbdd..992ae1bdfb3b 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > @@ -336,13 +336,12 @@ static u64 read_reference_ts_freq(struct drm_i915_private *dev_priv)
> >
> >         base_freq = ((ts_override & GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_MASK) >>
> >                      GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_SHIFT) + 1;
> > -       base_freq *= 1000000;
> > +       base_freq *= 1000000ULL;
> >
> >         frac_freq = ((ts_override &
> >                       GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_MASK) >>
> >                      GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_SHIFT);
> > -       if (frac_freq != 0)
> > -               frac_freq = 1000000 / (frac_freq + 1);
> > +       frac_freq = div_u64(1000000ULL, frac_freq + 1);
> s/div_u64/div64_u64/ ?

Rather s/u64 frac_freq/u32 frac_freq/
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 78bf7374fbdd..992ae1bdfb3b 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -336,13 +336,12 @@  static u64 read_reference_ts_freq(struct drm_i915_private *dev_priv)
 
 	base_freq = ((ts_override & GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_MASK) >>
 		     GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_SHIFT) + 1;
-	base_freq *= 1000000;
+	base_freq *= 1000000ULL;
 
 	frac_freq = ((ts_override &
 		      GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_MASK) >>
 		     GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_SHIFT);
-	if (frac_freq != 0)
-		frac_freq = 1000000 / (frac_freq + 1);
+	frac_freq = div_u64(1000000ULL, frac_freq + 1);
 
 	return base_freq + frac_freq;
 }
@@ -360,7 +359,7 @@  static u64 read_timestamp_frequency(struct drm_i915_private *dev_priv)
 		 *      hclks." (through the “Clocking Configuration”
 		 *      (“CLKCFG”) MCHBAR register)
 		 */
-		return (dev_priv->rawclk_freq * 1000) / 16;
+		return div_u64(dev_priv->rawclk_freq * 1000ULL, 16);
 	} else if (INTEL_GEN(dev_priv) <= 8) {
 		/* PRMs say:
 		 *