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

From: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
To: Matthew Auld <matthew.william.auld@gmail.com>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH v2 3/4] drm/i915: fix 64bit divide
Date: Mon, 13 Nov 2017 18:50:35 +0000	[thread overview]
Message-ID: <01d5ca0b-e087-913e-9fd2-5e1cd96bda89@intel.com> (raw)
In-Reply-To: <CAM0jSHNQ49H9vwmr2GA4P85yzB7jc5PzkRY+Lq+BjSSJb++U6Q@mail.gmail.com>

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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx