[PATCH] drm/i915: Explicitly cast divisor to fix Coccinelle warning

[PATCH] drm/i915: Explicitly cast divisor to fix Coccinelle warning

[Thorsten Blum]

As the comment explains, the if check ensures that the divisor oa_period
is a u32. Explicitly cast oa_period to u32 to remove the following
Coccinelle/coccicheck warning reported by do_div.cocci:

  WARNING: do_div() does a 64-by-32 division, please consider using div64_u64 instead

Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com>
---
 drivers/gpu/drm/i915/i915_perf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 0b1cd4c7a525..24722e758aaf 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -4103,7 +4103,7 @@ static int read_properties_unlocked(struct i915_perf *perf,
 			 */
 			if (oa_period <= NSEC_PER_SEC) {
 				u64 tmp = NSEC_PER_SEC;
-				do_div(tmp, oa_period);
+				do_div(tmp, (u32)oa_period);
 				oa_freq_hz = tmp;
 			} else
 				oa_freq_hz = 0;
-- 
2.45.2

Re: [PATCH] drm/i915: Explicitly cast divisor to fix Coccinelle warning

[Ville Syrjälä]

On Wed, Jul 10, 2024 at 09:46:51AM +0200, Thorsten Blum wrote:
> As the comment explains, the if check ensures that the divisor oa_period
> is a u32. Explicitly cast oa_period to u32 to remove the following
> Coccinelle/coccicheck warning reported by do_div.cocci:
> 
>   WARNING: do_div() does a 64-by-32 division, please consider using div64_u64 instead
> 
> Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com>
> ---
>  drivers/gpu/drm/i915/i915_perf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 0b1cd4c7a525..24722e758aaf 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -4103,7 +4103,7 @@ static int read_properties_unlocked(struct i915_perf *perf,
>  			 */
>  			if (oa_period <= NSEC_PER_SEC) {
>  				u64 tmp = NSEC_PER_SEC;
> -				do_div(tmp, oa_period);
> +				do_div(tmp, (u32)oa_period);

Why is this code even using do_div() when it doesn't need the
remainder?

>  				oa_freq_hz = tmp;
>  			} else
>  				oa_freq_hz = 0;
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

Re: [PATCH] drm/i915: Explicitly cast divisor to fix Coccinelle warning

[Thorsten Blum]

On 10. Jul 2024, at 13:38, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Jul 10, 2024 at 09:46:51AM +0200, Thorsten Blum wrote:
>> As the comment explains, the if check ensures that the divisor oa_period
>> is a u32. Explicitly cast oa_period to u32 to remove the following
>> Coccinelle/coccicheck warning reported by do_div.cocci:
>> 
>>  WARNING: do_div() does a 64-by-32 division, please consider using div64_u64 instead
>> 
>> Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com>
>> ---
>> drivers/gpu/drm/i915/i915_perf.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>> index 0b1cd4c7a525..24722e758aaf 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.c
>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>> @@ -4103,7 +4103,7 @@ static int read_properties_unlocked(struct i915_perf *perf,
>>  */
>> if (oa_period <= NSEC_PER_SEC) {
>> u64 tmp = NSEC_PER_SEC;
>> - do_div(tmp, oa_period);
>> + do_div(tmp, (u32)oa_period);
> 
> Why is this code even using do_div() when it doesn't need the
> remainder?

do_div() is an optimized 64-by-32 division and the compiler should
automatically remove the remainder if it's not used.

Re: [PATCH] drm/i915: Explicitly cast divisor to fix Coccinelle warning

[Ville Syrjälä]

On Wed, Jul 10, 2024 at 01:55:32PM +0200, Thorsten Blum wrote:
> On 10. Jul 2024, at 13:38, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Wed, Jul 10, 2024 at 09:46:51AM +0200, Thorsten Blum wrote:
> >> As the comment explains, the if check ensures that the divisor oa_period
> >> is a u32. Explicitly cast oa_period to u32 to remove the following
> >> Coccinelle/coccicheck warning reported by do_div.cocci:
> >> 
> >>  WARNING: do_div() does a 64-by-32 division, please consider using div64_u64 instead
> >> 
> >> Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com>
> >> ---
> >> drivers/gpu/drm/i915/i915_perf.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> >> index 0b1cd4c7a525..24722e758aaf 100644
> >> --- a/drivers/gpu/drm/i915/i915_perf.c
> >> +++ b/drivers/gpu/drm/i915/i915_perf.c
> >> @@ -4103,7 +4103,7 @@ static int read_properties_unlocked(struct i915_perf *perf,
> >>  */
> >> if (oa_period <= NSEC_PER_SEC) {
> >> u64 tmp = NSEC_PER_SEC;
> >> - do_div(tmp, oa_period);
> >> + do_div(tmp, (u32)oa_period);
> > 
> > Why is this code even using do_div() when it doesn't need the
> > remainder?
> 
> do_div() is an optimized 64-by-32 division and the compiler should
> automatically remove the remainder if it's not used.

The point is that do_div() is a bad API because it magically
changes the divided in place. There are more sensible 64bit
division helpers in math64.h that can be used instead.

oa_exponent_to_ns() also hand rolls a DIV_ROUND_UP_ULL()
for some reason...

-- 
Ville Syrjälä
Intel

✗ Fi.CI.BAT: failure for drm/i915: Explicitly cast divisor to fix Coccinelle warning

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 2486 bytes --]

== Series Details ==

Series: drm/i915: Explicitly cast divisor to fix Coccinelle warning
URL   : https://patchwork.freedesktop.org/series/135942/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_15058 -> Patchwork_135942v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_135942v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_135942v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_135942v1/index.html

Participating hosts (36 -> 34)
------------------------------

  Missing    (2): fi-snb-2520m fi-kbl-8809g 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_135942v1:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@migrate:
    - bat-dg2-8:          [PASS][1] -> [ABORT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15058/bat-dg2-8/igt@i915_selftest@live@migrate.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_135942v1/bat-dg2-8/igt@i915_selftest@live@migrate.html

  
Known issues
------------

  Here are the changes found in Patchwork_135942v1 that come from known issues:

### IGT changes ###

#### Possible fixes ####

  * igt@kms_frontbuffer_tracking@basic:
    - bat-arls-2:         [DMESG-WARN][3] ([i915#7507]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15058/bat-arls-2/igt@kms_frontbuffer_tracking@basic.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_135942v1/bat-arls-2/igt@kms_frontbuffer_tracking@basic.html

  
  [i915#7507]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/7507


Build changes
-------------

  * Linux: CI_DRM_15058 -> Patchwork_135942v1

  CI-20190529: 20190529
  CI_DRM_15058: 28c3b18581a110660a7c42bb988a12a3b1f6d402 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7921: f547de980dca43c6630ced36e98af7f2a9c70ae7 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_135942v1: 28c3b18581a110660a7c42bb988a12a3b1f6d402 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_135942v1/index.html

[-- Attachment #2: Type: text/html, Size: 3113 bytes --]